Skip to content

test(4/7): provider conformance suites, and a prototype bug they found - #30

Merged
Tyler-R-Kendrick merged 17 commits into
mainfrom
claude/test-contract
Aug 30, 2026
Merged

test(4/7): provider conformance suites, and a prototype bug they found#30
Tyler-R-Kendrick merged 17 commits into
mainfrom
claude/test-contract

Conversation

@Tyler-R-Kendrick

@Tyler-R-Kendrick Tyler-R-Kendrick commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Stacked on #29.

Why contract tests matter here specifically

The provider-neutral design is the architectural centerpiece: TrainingEngine, ImplementationExecutor, TrainingLoop, PromotionApplier and TrainingStore are injected so any structurally compatible implementation works. That was only ever checked against the implementations shipped here, through whatever paths happened to exercise them — which is not a claim about anyone else's, and not a stated contract at all.

The suites ship in ts-autocode-training so an implementer can run them against their own provider. Framework-agnostic on purpose — named checks that throw on violation:

import { trainingStoreContract } from "ts-autocode-training";

for (const check of trainingStoreContract) {
  it(check.name, () => check.run(() => new MyStore()));
}

They state what types cannot: a store preserves append order and doesn't alias internal state; an executor surfaces a throwing body as a rejection; a loop returns the winning round last, because the runtime activates rounds.at(-1); an applier refuses a decision bound to a different candidate.

A vacuous check of my own, caught and fixed

The abort check originally asserted rejected || resolvedalways true. It now counts proposals, so it genuinely rejects a loop that keeps calling the engine after an abort, and a test proves it does.

That's the failure mode this whole stack is about: a check that cannot fail is worse than no check, because it reads as coverage.

The defined() __proto__ fix that this PR originally carried has moved to #29, where the property test that found it lives. Leaving it here would have left #29 intermittently red. It is present in this branch via the merge from its base.

Keeping the kit honest

  • test/contract.test.ts runs every shipped provider through the suites, plus a second, deliberately different store — a suite that only ever sees one shape is describing that shape, not a contract.
  • packages/training/test/conformance.test.ts proves each suite rejects an implementation violating the rule it names, so the kit can't pass everything.

That split also fixed a coverage artifact worth knowing: root tests exercise siblings' built dist/, so the kit showed 6% covered until it had a package-internal test importing ../src/.

Verification

npm run check green. 500 → 573 tests. Coverage rose to 91.16% statements / 80.6% branches; thresholds ratcheted to 91/80/93/93.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb

claude added 14 commits August 28, 2026 17:17
Reviews the consumer-facing surface of ts-autocode and its four sibling
packages against the API-design rules already stated in CONTRIBUTING.md,
and records the remediation plan.

Covers eight defects (README code that does not compile, grounding codegen
targeting a nonexistent training.define, an inaccurate sideEffects
declaration, a silently ignored fanOut, documented-but-unexported symbols,
a placeholder threshold reaching the judge, a fail-open evolve kill switch,
and an unreachable execution timeout), two missing capabilities (model
selection and a CLI), and the consistency, error-model, and boilerplate
backlog behind them.

The remediation is additive: renamed or reshaped APIs are added alongside
the existing ones, which keep working and are marked deprecated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
Each of these was reachable by a user following the documentation.

- README snippets did not compile: `activation.promotion.snapshot.candidateId`
  does not exist on `Activation`, and the quickstart referenced an undefined
  `deploymentPolicy` and used its token before defining it. Every TypeScript
  block in all four READMEs is now self-contained and compiled by
  test/docs.test.ts.
- The grounding package generated `training.define(...)`, which `Training` has
  no such method for, so every generated registration file failed to compile.
  Codegen now emits `defineGrounding` from the package that owns the concept,
  and the scan test typechecks the generated source instead of string-matching
  it.
- `sideEffects: false` was wrong: importing the root package wires the engine,
  executor, loop and promotion applier at import time, so a tree-shaking
  bundler could legally drop that and leave a consumer with "no training engine
  is configured" after importing the package that configures it.
- `TrainInput.fanOut` was documented but silently ignored by the default
  harness loop, whose judge/adversary/rubric sequence is serial. It now refuses
  a fan-out above 1 and names the loop that supports one.
- The root package re-exported a hand-maintained subset of its siblings that
  had drifted, leaving README-documented `trainingRounds` and `sequentialLoop`
  unreachable along with `defaultPromotionGates`. test/surface.test.ts now
  enforces exhaustiveness. The harness's colliding `defaultMaxRounds` is
  renamed `defaultHarnessRounds`, keeping the old name as a deprecated alias.
- The promotion rubric handed to the judge printed the literal string
  "evaluation default" instead of the threshold. The defaults are now exported
  as `defaultMinScore` and `defaultMinPassRate` and the rubric resolves them.
- `TS_AUTOCODE_EVOLVE` failed open: only "0", "false" and "off" disabled
  source-rewriting evolution, so "no" enabled it. It now fails closed and
  throws on an unrecognized value.
- Candidate execution timeout had no path through settings; added
  `TrainingSettings.execution.timeoutMs`, distinct from the retrying
  `resilience.evaluate` policy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
…ream

Errors were bare Error/TypeError/SyntaxError at ~40 sites, carrying a good
message and nothing else, so the only way to tell "not enough traces" from
"no engine configured" from "gate rejected" was to match on message text --
which is what the tests had to do.

Adds TsAutocodeError with a `code` discriminant and concrete subclasses that
carry the facts a caller would otherwise re-derive: InsufficientTracesError
holds `required`/`found`, PromotionRejectedError holds the decision and its
failures, TrainingIncompleteError holds the outcome.

Nothing breaks. Every message string is preserved byte for byte, so existing
catch blocks and substring assertions keep working. Errors that were TypeError
or SyntaxError still are: family membership is decided by a brand rather than
the prototype chain, so `instanceof TsAutocodeError` recognizes them without
changing their existing type. Zod failures are wrapped as InvalidSettingsError
instead of escaping as a schema-library type.

Also:

- TrainingRun.canActivate() reports whether the final candidate can be applied
  without provoking an exception. `outcome` already distinguished "stalled"
  from "exhausted"; a caller should not have needed try/catch to read it.
- TrainingSettings.onEvent reports background work as a discriminated union,
  including evolution.started/applied/skipped/failed, which had no observable
  signal at all. onError is retained, deprecated, and implemented as a
  projection of the same stream, so both can be configured without a failure
  being delivered twice to one handler.
- The evolution sad path documented in the README now has a test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
Four conventions disagreed with themselves; each is now settled additively,
with the old spelling kept, deprecated, and covered by test/deprecated.test.ts
so the compatibility promise is enforced rather than asserted.

- Runtime scoping. `configureTraining` configured a module singleton and
  replaced it wholesale, so a second call silently discarded the first's
  settings and nothing could hold an isolated runtime. Adds
  `createTrainingRuntime(settings)`, which registers nothing globally, and
  `resetTraining()` for tests. `configureTraining` keeps replacing by default
  -- silently carrying settings between unrelated calls would be a worse
  surprise than the one it fixes -- and takes `{ merge: true }` to opt in.
  Named apart from `createTraining`, which training.test.ts deliberately
  asserts the package must not export.

- Grouped train options. `TrainInput` grouped `evaluation` but flattened six
  round and gate options, and `policy` was a `PromotionGate` in disguise --
  the evaluator wrapped it into one -- so two spellings expressed one concept.
  Adds `rounds: { max, fanOut }` and `promotion: { minScore, minPassRate,
  gates }`. Both forms are honored; gates from both run rather than one
  shadowing the other.

- Opt-in that reads as opt-in. `evolution.enabled` was the only opt-in switch
  among three identically named ones, and it is the one that rewrites your
  source. Adds `evolution.auto`; `enabled` still works.

- Name collisions. Grounding's `digest` hashed normalized text while rewrite's
  canonicalizes an arbitrary value; both emit a `sha256:` prefix, so swapping
  them silently changes every hash. Renamed `textDigest`. Grounding's three
  SCREAMING_SNAKE exports, unique in this workspace, gain camelCase aliases.

- Root surface. Adds `ts-autocode/internal` for the author-level seams
  (`captureTrainable`, `provideTrainingDefaults`, the rewrite primitives), so
  what an application imports is what an application needs. All of it stays
  exported from the root.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
Choosing a model is the first thing most users change, and it was the one
thing the zero-config path could not do. The default engine hardcoded OpenAI;
using anything else meant importing createAxEngine from the ts-autocode/ax
subpath -- mentioned once in the README with no example anywhere in the repo --
constructing an AxAIService, and handing a whole replacement engine to
configureTraining.

Adds TrainingSettings.model, a provider-neutral ModelSelection carrying
provider, model name, an optional apiKey, and an optional stronger teacher
model. ts-autocode-training stays provider-agnostic: it forwards the descriptor
to whatever engine is configured through EngineContext, exactly as it already
forwards secrets and variables, and the default Ax engine interprets provider
as an Ax provider name.

Credentials resolve in order: an explicit model.apiKey, the configured secret
provider, then the environment variable conventional for that provider.
Previously only OPENAI_API_KEY was ever consulted, so a user who named another
provider would have been told to set the wrong variable; an unlisted provider
falls back to <PROVIDER>_API_KEY rather than failing.

Also documents ts-autocode/ax with a real example for the Ax-specific tuning
the neutral slot does not cover, which had none.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
The product is "instrument your app and let it rewrite itself", but inspecting
what is trainable, what has been captured, or what a run would change required
writing a script that imports discoverTrainables. No package declared a bin.

`ts-autocode discover` lists every marked method with its signature and, more
importantly, the exact identity to pass to defineTrainable. That is the one
place an otherwise type-safe design falls back to a string -- a typo in
`defineTrainable("Router.route")` silently yields a different symbol -- so
printing real ids is what makes the marker design usable without reading the
source scanner.

`ts-autocode status` reports captured traces per trainable, which is what
background evolution counts against evolution.minTraces. Both take --cwd,
--project, --file, --output-dir and --json.

The CLI is a function returning {code, stdout, stderr} with a thin bin wrapper,
so it is tested without spawning a process, and library failures are reported
by message rather than as a stack trace.

Also makes examples/optimize.ts real, per CONTRIBUTING's own rule. It imported
"../src/index.js" rather than the package name, exported rather than ran, and
was referenced by no test or script, so nothing would have noticed it breaking.
It now imports by package name (tsconfig.test.json maps the specifier to src/),
runs directly under node, and executes on every check against a stub engine so
CI needs no provider key.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
Five things a consumer had to work around.

- Types they had to supply but could not construct. Implementing a custom
  TrainingLoop means returning a CandidateReview containing a
  TrainableEvalRun, which only the internals could produce -- so this repo's
  own tests wrote `{...} as unknown as TrainableEvalRun` and `{} as never`, and
  a consumer had no better option. Adds createEvalRun,
  createPromotionDecision and createCandidateReview, and uses them in the two
  tests that needed the casts, which now have none.

- Uninferrable generics. defineTrainingHarness takes three type parameters but
  its settings mention only TCandidate, so a bare call inferred `unknown`
  three times and every documented call site wrote them all out. TChallenge was
  already scoped to `run` and inferred correctly; `inferringHarness()` gives
  the other two the same treatment.

- A lossy argument guess with no way out. Eval inputs were JSON.parsed and
  spread as arguments, so a trainable taking the literal string "[1,2]"
  received two numbers. Adds ExecutionSettings.decodeArgs, with the previous
  behavior exported as `evaluationArgs` and still the default.

- `...(x === undefined ? {} : { x })`, written out about twenty-five times
  because exactOptionalPropertyTypes forbids assigning an explicit undefined,
  plus a one-off maybeSignal() doing the same for one field. Adds
  `optional(key, value)` and `defined(values)` and applies them.

- Effect as a root runtime dependency to express two try/catch statements.
  attempt/attemptAsync are now plain try/catch and `effect` is dropped from the
  root package. It stays where it earns its place: resilience.ts, whose
  timeout/retry/interruption composition is genuinely hard by hand.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
Adds the packages/grounding README its own package.json homepage has always
linked to and that never existed, and extends the documentation typecheck to
cover it and docs/architecture.md, so every fenced TypeScript block in the repo
now compiles.

Updates prose the preceding commits made stale: onEvent alongside the
deprecated onError, rounds.fanOut and promotion.gates, the fail-closed evolve
switch, model selection as a neutral descriptor rather than a provider-specific
option, and the consumer/author surface split.

Records in docs/dx-review.md what shipped, plus two places the remediation
deliberately departed from the plan -- configureTraining still replaces by
default, and evolution's opt-in polarity was renamed rather than flipped --
and corrects one claim the review got wrong about test/wiring.ts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
The review claimed a bundler honoring `sideEffects: false` could drop the
package's import-time wiring and leave a consumer with "no training engine is
configured". Bundling a trivial consumer with esbuild --tree-shaking=true
produces byte-identical output with the flag set either way, so that failure
was asserted rather than observed.

The fix stands and is still correct: the declaration was factually untrue,
since importing the root module registers four providers and configures rewrite
capture. But the finding now says what it is -- a latent correctness bug in a
promise made to bundlers -- and records that esbuild does not collect on it,
rather than implying a reproduced breakage.

Also drops the planned tree-shaking bundle test, which would pass either way
and prove nothing. The manifest assertion in test/tier1.test.ts is what
actually guards the declaration.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
CI was red on every commit in this branch while `npm run check` passed
locally. Two causes, both mine, plus a real pre-existing bug the failure
uncovered.

**examples/optimize.ts could not resolve `ts-autocode`.** It imports by
package name, as a consumer would, but the package entry points at `dist/`,
which `npm run check` does not build until *after* the tests run, and Vitest
does not read tsconfig `paths`. It passed locally only because a stale `dist/`
happened to be lying around -- exactly the works-on-my-machine trap. Mirrors
the tsconfig paths as Vitest resolve aliases so the example resolves from
source, deterministically.

**test/tier1.test.ts imported src/register.ts** to reach a string-parsing
function, and importing that module installs a load hook. That surfaced the
real bug:

**`ts-autocode/register` crashed on Node 20.** `module.registerHooks` is the
synchronous in-thread loader API, added in Node 22.15. `engines` declares
`node >= 20`, and the README's headline zero-config command is
`node --import ts-autocode/register`, so the flagship feature was broken on the
minimum supported version -- and failed with an internal
`TypeError: registerHooks is not a function` rather than anything actionable.

Nothing had ever imported that module in a test: test/register.test.ts
exercises only the pure `augmentSource`, so the side-effecting entry was never
loaded under test on any Node. The guard now names the requirement, says the
rest of the package still works on Node 20, and points at the decorator, which
needs no load hook. The README says so too.

Also splits the evolve kill switch (src/evolve.ts) and the load-hook guard
(src/load-hook.ts) out of the side-effecting entry, so both are testable
without installing anything.

Verified with a clean dist/ on Node 20.20.2 -- the exact version CI failed on
-- and on Node 22: 218 tests pass on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
The suite had 218 tests and no coverage measurement at all, which made "we
have tests" unfalsifiable. Measuring it first showed 85.35% statements and
73.14% branches -- and that the three files added in the previous PR were
among the worst covered in the workspace: errors.ts at 16.66% branch,
builders.ts at 55%, optional.ts at 75%.

Adds @vitest/coverage-v8, wires coverage into `npm run check`, and sets
thresholds as a ratchet (90/78/93/93 -- what is actually achieved now, to be
raised as later suites land, never lowered).

New atomic unit suites, 184 tests:

- errors.ts: every constructor, static factory, payload accessor and message
  string; both directions of the brand-based `hasInstance`, including that a
  hand-rolled look-alike is not admitted and that subclass `instanceof` stays
  exact; and the Zod boundary.
- optional.ts / defined(): asserts key *presence*, not deep equality --
  `{a: undefined}` and `{}` compare equal under toEqual, so the distinction the
  helpers exist for would otherwise go unchecked.
- builders.ts: every defaulting rule a consumer will rely on without reading
  the source, plus that supplied evaluations are copied rather than aliased.
- token.ts: the normalization and rejection rules standing between a typo and
  a silently different identity.
- canonical.ts, component.ts: pre-existing gaps at 46% and 50% branch. The
  class-instance case matters -- if `isRecord` wrongly accepted a Date, every
  Date would hash identically.
- attempt.ts (both copies), and the CLI's status and option paths.

Also adds test/digest-protocol.test.ts. Training and rewrite each implement the
body digest and never import each other, so guarded application depends on two
independent implementations agreeing; that agreement was assumed, and is now
asserted.

Boy-scout fix: test/docs.test.ts built one ts.Program per snippet, taking ~36s
and relying on the default 5s per-test timeout. Under coverage instrumentation
it blew that timeout and failed 15 of 26. It now builds one program for all
snippets: 3s, and robust under instrumentation. Verified it still fails when a
README snippet breaks rather than having gone vacuous.

218 -> 402 tests; 85.35 -> 90.15% statements, 73.14 -> 79% branches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
The workspace had no snapshot tests at all. For a library whose product is
rewritten source, the generated text *is* the product, and a diff of it is the
only review that shows what actually changed -- `toContain("return input")`
says almost nothing about an emitted module.

test/support/verify.ts follows the Verify model rather than Vitest's inline
snapshots: one named file per subject under test/snapshots/, committed and
reviewed like any other artifact. An inline .snap blob keyed by test name is
hard to read in a diff and a rename silently orphans it. `scrub()` removes
digests, UUIDs, timestamps and absolute paths, because a snapshot that churns
is one everyone learns to re-approve without reading.

Approved: discovered source targets (the whole contract handed to an
optimizer), emitted instrumentation and the augmented module, the synthetic
candidate declaration for sync and async targets, applied rewrites, grounding
codegen, promotion decisions, CLI usage/discover/status, the export surface of
all seven entry points with each export's kind, the error message catalogue,
the promotion rubric read by the judge, and the Ax program signature.

The last two are the ones nothing else could pin: both are read by a model
rather than by code, and neither has a natural assertion. The rubric is where
the literal string "evaluation default" once shipped in place of a threshold.

Verified the snapshots actually fail on a real change rather than passing
vacuously -- which also surfaced that the root suite runs against siblings'
built dist/, so a sibling source edit needs a rebuild before it is visible.

Boy-scout fix, found by the Ax snapshot: a parameter with a literal default and
no annotation (`retries = 2`) was reported as type `unknown`, which the Ax field
mapper turned into `json` -- so the optimizer was told a plainly numeric
argument had an opaque shape. Source discovery now infers string, number,
boolean, bigint and uniform array types from a literal initializer, exactly
where TypeScript would. Anything non-literal stays `unknown`. The snapshot now
shows `{"name": "number"}` where it showed `{"name": "json"}`.

402 -> 443 tests; branches 79 -> 80.29%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
The workspace had neither. Properties state the law and let fast-check hunt for
a counterexample; fuzzing feeds the parsers input they were not written for.
Both found real defects on the first run.

Properties cover identity round-trips, digest canonicalization (load-bearing:
guarded rewriting refuses a candidate whose body digest changed), the spread
helpers, evaluation-argument decoding, and promotion-gate aggregation.

Fuzzing covers source discovery, the register load hook, ambient declaration
scanning, the evolve kill switch, and the CLI.

Four findings, all fixed here:

1. `toTrainableToken` was not exported from either barrel. It takes the public
   `TrainableIdentity` type and is the canonical validator, so anyone
   implementing a loop, engine or store needs it. Now exported; the surface
   snapshot shows the single added line.

2. `minScore: Infinity` reported "expected number, received number" -- Zod's
   base schema rejected it before the .finite() message could apply, so a user
   who passed a bad threshold was told nothing useful. Every rejection now
   reports the range.

3. Discovery could report `bodyEnd` past the end of the source. TypeScript's
   error recovery synthesizes a body for an unterminated block whose `end` sits
   past EOF, so a truncated file produced a target claiming offsets outside its
   own source. Slicing clamps, so nothing was corrupted, but publishing an
   out-of-range range is malformed data crossing a public boundary. Now clamped
   -- a no-op for source that parses.

4. `TrainableTarget`'s body fields had undocumented and subtly different
   relationships to the source: `implementation` is trimmed, `bodyDigest`
   hashes the raw slice, and guarded application depends on the digest side.
   Nothing said so until a property asked. Now documented and pinned.

The fuzz corpus is itself tested. An early version used random punctuation;
instrumenting it showed 1 input in 3000 produced a discovered target, so every
property about offsets and rewriting was passing vacuously.
test/support/sources.ts generates structurally plausible marked modules and
then damages them, and the suite asserts the corpus still reaches real work.

One property documents a limitation rather than a bug: `-0` cannot round-trip
through an eval input, because JSON.stringify(-0) is "0".

443 -> 500 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
…ound

The provider-neutral design says any structurally compatible implementation
works. That was only ever checked against the implementations shipped here,
through whatever paths happened to exercise them -- which is not a claim about
anyone else's, and not a stated contract at all.

Adds conformance suites for all five injected seams, shipped in
ts-autocode-training so an implementer can run them against their own provider.
They are framework-agnostic on purpose: a list of named checks that throw on
violation, driven by whatever runner the consumer has.

They state what types cannot: a store preserves append order and does not alias
its internal state; an executor surfaces a throwing body as a rejection; a loop
returns the winning round last, because the runtime activates `rounds.at(-1)`;
an applier refuses a decision bound to a different candidate.

test/contract.test.ts runs every provider this repo ships through them, plus a
second, deliberately different store -- a suite that only ever sees one shape
describes that shape rather than a contract.
packages/training/test/conformance.test.ts proves each suite *rejects* an
implementation violating the rule it names, so the kit cannot pass everything.

Two defects fixed, both found while writing this:

1. `defined()` built its result with `result[key] = value`, which goes through
   the `__proto__` setter on Object.prototype. A `__proto__` key was silently
   dropped, and with an object value it replaced the result's prototype instead
   of adding a key. Now built with Object.fromEntries, which defines own
   properties. Found by a property test over arbitrary dictionaries;
   `optional()` was already safe because a computed key in an object literal
   defines rather than assigns.

2. One conformance check was written vacuously -- it asserted
   `rejected || resolved`, which is always true. It now counts proposals, so it
   genuinely rejects a loop that keeps calling the engine after an abort, and a
   test proves it does.

Also corrects an assertion in the property suite: `key in spread` walks the
prototype chain, so any key named after an Object.prototype member read as
present whether or not it was added. fast-check found that mistake in my own
test within a few hundred runs.

500 -> 573 tests; thresholds raised to 91/80/93/93.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 210cfc2b-25f2-47d7-b46f-949c7a44e897


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

claude added 2 commits August 28, 2026 19:18
CI on this branch failed with `Counterexample: ["toString", undefined]`,
seed 1960220664. Two separate defects, both here:

1. The assertion was wrong. `key in spread` walks the prototype chain, so any
   key named after an Object.prototype member -- "toString", "constructor",
   "valueOf" -- read as present whether or not it had been added. It takes
   fast-check about 87 runs to find one, which is why it passed locally and
   failed in CI: the seed is random per run. Now `Object.hasOwn`.

2. `defined()` was genuinely broken for the same class of key. It built its
   result with `result[key] = value`, an assignment that goes through the
   `__proto__` setter on Object.prototype: a `__proto__` key was silently
   dropped, and with an object value it replaced the result's prototype
   instead of adding a key. Now built with Object.fromEntries, which defines
   own properties. `optional()` was already safe, because a computed key in an
   object literal defines rather than assigns.

The second is the reason this belongs in this PR rather than a later one: the
dictionary property that finds it was introduced here, so leaving the fix
downstream would leave this branch intermittently red.

Reproduced the original failure with CI's exact seed before fixing, then
confirmed 2000 runs of that seed and 5000 of the dictionary property pass, plus
eight full suite runs on random seeds.

Adds regression tests for every key that shadows Object.prototype, including
that Object.prototype itself is never polluted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QchTdrzVJeexx1sjWjkJVb
The  __proto__ fix and both hasOwn assertion corrections moved to
the PR whose properties found them, so this branch takes them from its base
rather than carrying them itself.
Base automatically changed from claude/test-property-fuzz to main August 30, 2026 00:28
@Tyler-R-Kendrick
Tyler-R-Kendrick merged commit 550bdfe into main Aug 30, 2026
2 of 3 checks passed
@Tyler-R-Kendrick
Tyler-R-Kendrick deleted the claude/test-contract branch August 30, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants