chore(mutation): shrink to report-only — drop the ratchet, baseline and graduation (#1457, #1781) - #1828
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
thymikee
left a comment
There was a problem hiding this comment.
Adversarial review of #1828 (report-only mutation harness). Verified in a detached worktree at 271d0ce12: ran pnpm mutation:test (36/36 green, matches the PR body), traced every exit path in run.ts, grepped the whole tree for leftover ratchet/baseline/gating references, and diffed mutation-affected.yml/mutation-weekly.yml against the claims in the PR body.
Findings, ranked
1. Moderate — a scoring-behavior test was deleted and never replaced.
scripts/mutation/ratchet.test.ts (deleted) carried the only assertions on summarizeReport's status classification, e.g.:
test('statuses outside the score (Ignored, CompileError) leave the denominator', () => {
const [score] = summarizeReport(mutants(['Killed', 'Ignored', 'CompileError']), ['kernel-errors']);
assert.equal(score?.total, 1);
assert.equal(score?.score, 100);
});
The new scripts/mutation/report.test.ts exercises Killed/Timeout/Survived/NoCoverage through renderReport, but nothing anywhere in scripts/mutation/*.test.ts now asserts that Ignored/CompileError/RuntimeError are excluded from the denominator — that behavior lives only in a comment at scripts/mutation/score.ts:34-39. Failure scenario: a future edit to tally() in score.ts that starts counting Ignored/CompileError toward total (silently deflating every reported score) would pass pnpm mutation:test clean. For a PR whose whole thesis is "the report is the part that pays," losing coverage of the report's own arithmetic is the one regression that would go unnoticed by the harness watching itself. Suggest porting that one assertion into score.test.ts/report.test.ts before merge.
2. Minor — the job-summary score table is skipped on the "incomplete shard set" failure path, contradicting the workflow comments.
mutation-affected.yml:161 and mutation-weekly.yml:114-115 both state "[the score table] lands in $GITHUB_STEP_SUMMARY" for either the success or the harness-failure branch. But in scoreModules() (scripts/mutation/run.ts:387-396), assertShardsCoverModules() throws before sweep() reaches emit() (line 411 vs 416) whenever a shard's report is missing/incomplete. On that exact "harness failure, not a score" day, the kernels that did complete successfully still don't get their scores published to the summary — only the raw envelope JSON and an error string do. This predates the PR (the ordering isn't touched by this diff) so it's not a regression this PR introduces, but the PR's own comment updates assert a stronger guarantee ("Either invocation writes the score table") than the code delivers. Worth a doc/comment correction, not a blocker.
3. Nit — LOC claim is off by one. PR body says "816 lines deleted, 281 added (net −535)"; gh pr view reports 817 deletions / 282 additions (net −535, same). Immaterial.
Checks that came back clean
- No leftover
ratchet/mutation-baseline/graduation/gating/stableRuns/--updatereferences anywhere in the tree (scripts/, workflows, docs, AGENTS.md, CONTEXT.md, README, package.json,.oxfmtrc.json,scripts/gate/declarations.ts,scripts/check-affected/checks.ts) — every hit from a repo-wide grep belongs to unrelated systems (layering ratchets, replay--update). run.tstraced end to end:sweep()alwaysreturn 0; the only non-zero exits are--fail-envelope(explicit harness-failure declaration) and thrownErrors (missing report, bad--shard, incomplete shard set, bad arg). No path returns non-zero on a score..oxfmtrc.jsonedit is justified — it only drops the now-nonexistentmutation-baselines/**ignore entry;fallow-baselines/**(unrelated, still live) is untouched.mutation-affected.yml'sselectjob works with no baseline path (--list-affected→affectedMatrix(base)→affectedMatrixFor(changedFiles(base)), nogatingarg); kernel-source path triggers (packages/kernel/src/errors.ts,src/daemon/ref-frame.ts, etc.) are all preserved, onlymutation-baselines/**was dropped as claimed.docs/agents/testing.mdreads coherently as report-only; no stray graduation/ratchet language remains in that section.- "Closes #1457" is present in the PR body.
pnpm mutation:testpasses 36/36 locally on this branch, matching the PR's test-plan claim.
VERDICT: needs-changes — restore test coverage for the status-classification behavior in score.ts (finding 1) before merging; finding 2 is a nice-to-fix comment correction.
…ore judging the shard set Review of #1828: - `report.test.ts` re-asserts that Ignored/CompileError/RuntimeError leave the denominator — the one behaviour `ratchet.test.ts` covered and nothing replaced. A `tally()` edit that counted tool noise would have deflated every published score with a green `mutation:test`. - `assertShardsCoverModules` now runs after `emit()`, so an incomplete shard set still publishes the kernels that completed instead of only an error string. This makes the workflow comments' claim about the job summary true rather than re-wording them down.
|
All three addressed in 1 (moderate, denominator coverage) — ported into 2 (job summary skipped on the harness-failure path) — took the code fix rather than the comment fix, since it is one moved line and strictly better: 3 (LOC nit) — PR body now reads 818 deletions / 314 additions (net −504), matching
|
|
P2: the report-only envelope still drops all partial results in the common missing-shard case. The weekly workflow always passes |
|
|
|
Right, and my earlier reorder missed exactly that route — |
|
CI note: Layering Guard is red on main, not from this PR. main HEAD |
|
Re-reviewed exact head Not ready yet: Layering is red at stale 47/46 and 17/16 ceilings, so rebase onto main/#1838 and rerun. Also refresh body scope arithmetic: current diff is 21 files, +402/-880 (net -478), not +348/-861. |
The mutation harness's two real catches (#1474, #1475) both came from humans reading the weekly score report. The ratchet half never operated: the baseline was committed exactly twice (8cce0ef, 60400d0), both times with `stableRuns: 0, gating: false`, and was never updated after the very fixes it triggered — the weekly job computed a new baseline and then `git checkout --`d it, uploading a proposal nobody applied in 3+ weeks. A gate nobody arms is harness weight; the report is the part that paid. Deletes ratchet.ts + ratchet.test.ts, mutation-baselines/, and every baseline/graduation/gating path in run.ts (`--update`, `mutation:baseline`). run.ts now exits non-zero only on a harness failure, never on a score. The report renders the per-kernel table (kernel, score, killed, survived, total, timeouts) plus the surviving mutants a strengthening PR works from. Kernel scoping stays: stryker.config.json and KERNEL_MODULES are untouched.
…ore judging the shard set Review of #1828: - `report.test.ts` re-asserts that Ignored/CompileError/RuntimeError leave the denominator — the one behaviour `ratchet.test.ts` covered and nothing replaced. A `tally()` edit that counted tool noise would have deflated every published score with a green `mutation:test`. - `assertShardsCoverModules` now runs after `emit()`, so an incomplete shard set still publishes the kernels that completed instead of only an error string. This makes the workflow comments' claim about the job summary true rather than re-wording them down.
…can select mutants The PR lane returns an empty matrix unless the diff touches the harness, so the kernel-source and `**/*.test.ts` triggers only bought a 1-4 min no-op job on ~96% of PRs. `on.pull_request.paths` is now exactly `LANE_TOOLING` plus the workflow file, asserted in both directions by workflow.test.ts against the exported constant — a missing path would let a harness change merge unproven, an extra one starts a job that can only answer `[]`. Also drops the workflow header's contradictory scope paragraph: it claimed the lane selects on kernel sources and any test reaching one, which has not been true since the ratchet went.
…the count The expected-count check ran inside readShardedReports, before anything was summarized, so on the weekly's real `--expect-shards 10` one dead shard threw away the nine that had reported — the earlier reorder only moved the zero-mutants check. The merge now returns the shard count, and both verdicts run after emit() with the same exit code and `score` stage. Regression uses the weekly argument shape (`--expect-shards 10`, one shard present) and asserts the reporting kernel's row reaches stdout while the run still fails.
79ab53f to
e843a9a
Compare
|
Rebased onto main ( |
|
CI green on the rebased head (33/33; the one Android smoke red was an emulator/adb boot flake on the hosted runner, rerun passed). Marking ready. |
|
Re-reviewed exact e843a9a: clean and merge-ready. The rebase is bounded to the 21-file mutation/workflow/docs scope (+402/-880), body arithmetic matches, and the missing-shard fix still emits partial scores before failing expected-count with an exact |
Implements the #1781 wave-2 decision for #1457: shrink the mutation harness to report-only.
Why
The harness's only two real catches — #1474 (scroll-edge-state, 976-line behavioural suite) and #1475 (kernel-errors 55.19% → 86.26%) — both came from a human reading the score report. The ratchet/graduation/gating half never operated:
mutation-baselines/decision-kernels.jsonwas committed exactly twice (8cce0ef6b07-27,60400d04b08-02), both times readingstableRuns: 0, gating: false.mutation-weekly.ymlcomputed a new baseline with--update, copied it aside, thengit checkout --'d it and uploaded a proposal. Three weekly runs (08-02, 08-09, 08-16) all succeeded; nobody ranpnpm mutation:baselinein 3+ weeks.A gate nobody arms is harness weight. The report is the part that paid.
What changed
scripts/mutation/ratchet.ts+ratchet.test.ts(448 LOC),mutation-baselines/(71 LOC), and every baseline/graduation/gating path inrun.ts(--update,readBaseline/writeBaseline,recordRun, thegatingargument toaffectedMatrixFor, the baseline fields in the lane envelope), plus themutation:baselinepackage script and themutation-baselines/**entries in.oxfmtrc.jsonand the affected-workflow path filter. 880 deletions, 402 additions across 21 files (net −478).run.tsnever exits non-zero on a score. The only non-zero exits are harness failures: a missing Stryker report, an incomplete shard set, a bad argument. Envelope stageratchet→score;resultnow states whether the lane produced a report at all.--no-run/mutation:check: verified it is not baseline-only — it is a plain alias for--report <default path>, i.e. "score an existing report", which both workflows still use through--report-dir.report.tsnow renders fromModuleScore[]: kernel · score · killed · survived · total · timeout, plus the surviving mutants per kernel (capped at 20) — the list a test-strengthening PR actually works from.score.tsgained atimeoutcount (timeouts are counted as killed by Stryker, so a score propped up by slow mutants was previously invisible).mutation-weekly.yml: theratchetjob is nowreport— no--update, no proposed-baseline copy/restore dance, no proposed-baseline artifact. The weekly sweep, the--expect-shardscompleteness check, the$GITHUB_STEP_SUMMARYtable and the report artifact all stay.mutation-affected.yml: advisory as before, step summary kept, no PR-comment bot added. Itson.pull_request.pathsis now exactlyLANE_TOOLING(scripts/mutation/**,scripts/lib/**,stryker.config.json) plus the workflow file — the only diffs that can produce a non-empty matrix, asserted in both directions byworkflow.test.tsagainst the exported constant. The kernel-source and**/*.test.tstriggers were only starting a 1–4 min no-opselectjob on ~96% of PRs.stryker.config.jsonandmodules.ts(KERNEL_MODULES) are unchanged — kernel scoping is kept; only the ratchet goes.docs/agents/testing.md"Mutation ratchet over decision kernels" → "Mutation report over decision kernels", with the three-sentence statement (report-only over the seven kernels, weekly full sweep + per-PR affected sweep; never gates; a low score is an input for a human-authored PR). Allpnpm mutation:baseline/graduation mentions removed repo-wide (grepoverdocs/,AGENTS.md,CONTEXT.md,README.md,scripts/gate/,scripts/check-affected/checks.ts).One judgement call worth reviewing
Deleting
gatingforced a decision about what the PR lane selects. Selecting on derived kernel ownership (the pre-existing--affectedbehaviour, unlocked oncegatingis gone) would have raised the lane's cost roughly 15x rather than shrinking it: I measured the last 40 merged PRs throughderivedAffectedModulesand 24 of 40 select a non-empty matrix, 23 of those selecting all seven kernels — a full ten-shard sweep, up to 30 min per shard, on ~60% of PRs for a report nobody gates on. (Only 1 of 40 touches a kernel source.) So the PR lane keeps its current effective behaviour — it spends mutants only when the diff touches the lane's own tooling, where a harness change has to prove itself against real mutants — and the rationale is now stated as a cost rule instead of "until the baseline graduates". Widening it is a separate, cost-bearing decision.The lane as it stands after this change
scripts/mutation/(down from 2,453, plus the 71-line baseline JSON), one weekly 10-shard ubuntu sweep, and an advisory affected sweep on harness diffs only — after the path narrowing the other ~96% of PRs start no job at all, rather than a 1–4 min no-op one.workflow_dispatch.#1412's open question
Answered: downscope. Changed-line mutation over arbitrary PR diffs stays a bounded wave-3 experiment, not a lane.
Test plan
pnpm mutation:test— 37/37 pass (ratchet.test.tsdeleted;report.test.tscovers the table columns, the survivor list, and the status classification — Ignored/CompileError/RuntimeError leave the denominator — replacing the coverageratchet.test.tscarried).pnpm format:check,pnpm lint,pnpm typecheck,pnpm check:layering,pnpm check:gate-manifest(47 checks / 33 lanes),pnpm check:gate-manifest:test,pnpm check:fallow— all green.pnpm check:production-exportsreports the same 17 pre-existing findings asmain, none inscripts/mutation/.CLI verified without a real sweep (too slow locally):
run.ts --helpparses;--affected --base HEADtakes the no-op path ("no decision-kernel modules affected", exit 0, envelopepass/complete);--list-affected --base HEADprints[];--updateis now rejected as an unknown option and still writes a failure envelope.Report rendering and the "a 0% kernel still exits zero" invariant are covered by
report.test.tsandenvelope.test.tsrespectively, both through the real CLI where the envelope is involved.Proven end-to-end on real mutants in CI: because this PR edits the harness, the affected lane ran its canary sweep on this very branch —
Select affected kernels→[kernel-errors]→ 185 real mutants → the new table:That number is also the last piece of evidence for this change: the committed baseline this PR deletes still claimed 55.19% for the same kernel, three weeks after test(kernel): cover all six errors.ts exports, kernel-errors mutation 55.19% → 86.26% #1475 raised it. The ratchet was comparing against a fiction.
Closes #1457