test: mutate the consent and confinement decisions too - #38
Merged
Tyler-R-Kendrick merged 11 commits intoSep 4, 2026
Merged
Conversation
Mutation testing scored 98.27 with six mutants standing, every one of them inside `formatImplementation`. They stood because no test ever promoted the shape a model actually returns: the existing cases use single-line bodies, or multi-line ones with no leading indent, so `Math.min` returned 0 whatever the de-dent logic did. Two of the six were equivalent -- `/^\s*/` always matches, so neither the `^` anchor nor the `?. ... ?? 0` guarding `exec` could ever be observed. That is dead defensive code, so it is gone, replaced by `leadingWhitespace`, which says what the expression was for. The new cases pin the rest: the de-dent is measured from the code lines and ignores the blank ones between them, a whitespace-only line counts as blank rather than as the shallowest indent, and a file that uses tabs supplies the indent unit for a method whose own indentation does not. Writing them surfaced a fault. Blank lines inside a promoted body were indented like any other, so promoting a multi-line candidate wrote trailing whitespace into the user's source file -- which their own lint step then rejects. Blank lines now emit empty. `digest` gets the same treatment: `isRecord`'s `typeof` guard is what keeps `undefined` away from `Object.getPrototypeOf`, which throws on it, and optional fields do reach the digest undefined -- candidate `metadata` is one. Nothing covered that. Mutation score is now 100 across the mutated scope, and the break threshold ratchets to match, with a note on excluding a genuinely equivalent mutant at the line rather than by lowering the threshold again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
A coverage sweep found whole decisions with no test behind them. Each of these is a public entry point or a guard that decides something consequential, not a line-count gap. **`defineGrounding`** — the runtime every emitted registration calls, and the only thing between generated source and a grounding naming no method, describing no intent, or pointing at no contract. `scan.test.ts` checked that codegen *emits* a call to it; nothing ever called it. All three validations, the freeze, and the ordering that keeps a blank methodRef out of the message that would interpolate it are now pinned. **`createSandboxPolicy`** — only `version` was ever asserted. The rest is the whole of the default confinement story: workspace-only writes, the outbound allowlist, the readonly paths, the UI lockdown. The allowlist matters most and now has the case that would be worst to get wrong — an allowlist that filters down to nothing fails closed rather than reading as "no restrictions". The policy also copies the caller's arrays rather than aliasing them, which nothing checked. **`CandidateEngine`'s request validation** — the guards keeping one trainable's records and evaluations out of another's optimization. A request assembled with the wrong evidence trains a method on traffic it never served; both guards were unexecuted. **`rewritePromotion`'s hot swap** — the applier writes the source rewrite *and* installs a live implementation so a running process picks the candidate up without a restart. The second half had no test at all, though it is the half that changes an application already serving traffic. Covered now: the swap installs for an async target, forwards arguments and the receiver to the executor, comes back out on rollback, and never installs for a sync target (whose calling convention it would change) or without an executor. **`wrapTrainable`'s wrapper** — the load-time half of the zero-config flow, for a `"use training"` function rather than a method. Every test of that flow installs a stub `wrap` handler, so the real wrapper was built and never called. **The Ax engine's examples and metric** — the branch turning captured traffic into examples is the zero-config path the README leads with, and it never ran; neither did the metric's failure modes. A metric that scores everything 1 optimizes nothing, so an empty body, a body that throws, and a wrong answer now each have a case. **The CLI** — `status`'s counting loop only ever ran against zero records, and a discovery failure the library does not model was never shown to propagate rather than turning into a tidy exit code. Two faults surfaced while writing these: - `inputValue` substring-matched the declared parameter type independently of `fieldType`, so the two disagreed wherever a type merely contains a primitive name. `Record<string, unknown>` and `string[]` were declared to Ax as a `json` field and a string array, then handed a JSON string. It now derives from `fieldType`, so the value matches the field. - `test/promotion-applier.test.ts` needed explicit teardown: the swap registry is module-global and keyed by trainable id, so "does not swap" assertions would otherwise pass or fail on declaration order. Coverage: statements 93.22 -> 96.15, branches 83.29 -> 88.60, functions 95.48 -> 97.18, lines 95.88 -> 98.20. 726 -> 803 tests. Mutation score holds at 100. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
Stryker mutated seven modules. Its own stated criterion -- a module where a surviving mutant is alarming rather than merely untidy -- covers three more, each deciding something the library does to a user's machine rather than something it merely computes: - `src/evolve.ts`, the kill switch deciding whether the library may rewrite the user's source without being asked, which must fail closed on a value it does not recognize; - `packages/harness/src/policy.ts`, the sandbox's filesystem and network confinement, where an allowlist failing open would be the worst outcome available; - `packages/rewrite/src/emit.ts`, which appends generated code to the user's own module, where a name that is not an identifier turns a valid file into a syntax error at load time. Adding them left thirteen mutants standing. Four were equivalent: the printer's container name, text, parent linkage and newline kind are all unobservable from the printed string, verified by construction. They are hoisted into named constants and excluded at the line with the reason, which is the escape hatch the threshold note describes -- not a lowered threshold. The other nine were real, and each names a test that was asserting less than it looked like it was: - `isInstrumentable` checked only the method name, so a class name that does not scan as an identifier would be emitted as `owner: () => Not-A-Class`. - `isIdentifierName`'s scanner skipped trivia, so `" normalize"` scanned as a clean identifier. Leading and trailing whitespace, reserved words, member expressions and the empty name now each have a case. - The emitted setter is how a promoted candidate replaces a directive-marked free function, and the test evaluating it used a `wrap` handler returning its own argument -- so it passed whether or not the setter worked. It returns a different function now, and the module's binding is asserted to change. - The evolve switch's message was asserted to match /must be one of/, leaving the list of accepted values -- the entire reason the message exists -- unchecked. Chasing the last of them found a defect repeated six times across two packages. `z.number().int().positive(message)` attaches the message to the positivity check alone, so `minTraces: 2.5`, `fanOut: 2.5`, `maxRounds: 1.5` and `timeoutMs: 1.5` all failed with zod's "Invalid input: expected int, received number", naming neither the setting nor its rule. A `positiveIntegerSetting` helper in each package now carries one message across every constraint, matching what `executionTimeout` already did by hand. Mutation scope: 7 modules -> 10, 348 -> 469 mutants, still under two minutes. Score holds at 100. 813 -> 827 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 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. Comment |
The mutation job in CI ran `stryker run` against a tree that had never been built. Several of the tests it runs import a sibling package by name -- `packages/training/test/promotion.test.ts` imports ts-autocode-rewrite -- which resolves through that package's `dist/`. Without it those files fail to collect, and Stryker scores every mutant they would have killed as a survivor rather than reporting an error. So the promotion gate, the module whose whole reason for being in the mutated set is that it decides whether generated code reaches a user's file, was being graded in CI with its own test file not running. Four mutants stood there, including one that blanks the message telling a user what range `minScore` accepts. At the old threshold of 95 the shortfall to 98.85 was invisible, and had been. Raising it to 100 is what surfaced it -- which is the other thing a threshold of 100 buys: it notices a test file quietly dropping out of the run. `test:mutation` now builds what it needs, rather than CI having to remember to. `build:packages` factors the four sibling builds that `build` and `typecheck` already ran in sequence. Verified from a clean tree with every `dist/` removed: 100, no survivors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
…verage-review-vqvobz-2-unit-gaps
…aude/test-coverage-review-vqvobz-3-mutation
`check (20)` failed on a corpus that was doing its job: "produces modules that discovery actually finds targets in" drew 79 against a threshold of 80. The three tests at the bottom of the fuzz suite measure the *generator* rather than the code, but drew unseeded, so each run saw a different corpus. `markedModule` can emit a class of only unmarked methods and no marked free function, which puts the hit rate at a binomial around 90 in 100 -- measured across seeds it ranges 83 to 95, so a threshold of 80 sits about three standard deviations out and fails roughly once in a few hundred runs. It has been that way since the corpus tests were written; this run is the first to draw the tail. Raising the threshold would trade one flake for a looser assertion. A fixed seed measures the same corpus every time and on every machine, and still fails loudly if the generators change such that the corpus stops reaching real code -- which is the entire point of these three. The properties above them are untouched and still explore. The damaged-module case gains an upper bound while it is here: it asserts the corpus is a genuine mix, and only had the lower half of that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
…verage-review-vqvobz-2-unit-gaps
…aude/test-coverage-review-vqvobz-3-mutation
Self-review: `positiveIntegerSetting` went in between `parseSetting`'s JSDoc and `parseSetting` itself, so a public export lost its documentation and an orphaned comment block sat above the new helper. Both copies now also say they are copies, matching how `attempt`/`errorMessage` document the same deliberate duplication. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
Base automatically changed from
claude/test-coverage-review-vqvobz-2-unit-gaps
to
main
September 4, 2026 19:10
…review-vqvobz-3-mutation # Conflicts: # packages/harness/test/harness.test.ts
Tyler-R-Kendrick
deleted the
claude/test-coverage-review-vqvobz-3-mutation
branch
September 4, 2026 19:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #37, which is stacked on #36. Review those first; this diff is
against #37.
Why these three modules
Stryker mutated seven modules. Its own stated criterion — a module where a
surviving mutant is alarming rather than merely untidy — covers three more.
Each decides something the library does to a user's machine, not something
it merely computes:
src/evolve.ts— the kill switch deciding whether the library may rewritethe user's source without being asked. It must fail closed on a value it does
not recognize.
packages/harness/src/policy.ts— the sandbox's filesystem and networkconfinement, where an allowlist failing open would be the worst outcome
available.
packages/rewrite/src/emit.ts— appends generated code to the user's ownmodule, where a name that is not an identifier turns a valid file into a
syntax error at load time.
What survived
Thirteen mutants. Four were equivalent — the printer's container name, text,
parent linkage and newline kind are all unobservable from the printed string
(verified by construction). They are hoisted into named constants and excluded
at the line with the reason, which is the escape hatch #36's threshold note
describes, not a lowered threshold.
The other nine were real, and each names a test that was asserting less than
it looked like it was:
isInstrumentablechecked only the method name, so a class name that does notscan as an identifier would be emitted as
owner: () => Not-A-Class.isIdentifierName's scanner skipped trivia, so" normalize"scanned as aclean identifier. Leading/trailing whitespace, reserved words, member
expressions and the empty name now each have a case.
free function — and the test evaluating it used a
wraphandler that returnedits own argument, so it passed whether or not the setter worked. It returns a
different function now, and the module's binding is asserted to change.
/must be one of/, leavingthe list of accepted values — the entire reason the message exists —
unchecked.
The defect this turned up
Chasing the last mutant found the same bug written six times across two
packages.
z.number().int().positive(message)attaches the message to thepositivity check alone, so every one of these failed with zod's generic
"Invalid input: expected int, received number", naming neither the setting norits rule:
minTraces2.5fanOut2.5maxRounds(loop + harness)1.5timeoutMs1.5sequence1.5A
positiveIntegerSettinghelper in each package now carries one message acrossevery constraint — matching what
executionTimeoutalready did by hand.Result
npm run checkandnpm run test:mutationboth pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115
Generated by Claude Code