Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/development/review-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b
## Where a mutation landed

45. **A mutation arm has to assert WHERE it landed, not that it landed — a count proves the edit happened and says nothing about its site.** A file-scoped `String.replace` takes the FIRST match, and in a stylesheet or module of ordinary size the declaration you are mutating is not unique. The failure is silent in the worst direction: the edit applies, an occurrence guard confirms it applied, the suite stays green because the code under test was never touched, and that green reads as *this arm is unpinned* — a finding, filed against a guard that was correct all along. Note which way the error runs: an arm that reds by accident gets investigated, an arm that greens by accident gets believed. So scope the replacement to the span you mean — the rule's own `{…}`, the function's own body — and print the site beside the result: file, line, and the enclosing selector or symbol. An occurrence count is a necessary check that the edit was not a no-op and not a sufficient one that the edit was *the* edit; it is also inert for a mutation that rewrites a body rather than a call, where the count does not move at all. Sibling trap in the same family: **a slice with a start and no end is not a scope** — `css.slice(css.indexOf('@media …'))` reads to the end of the file, so a pin written against one block silently accepts any rule below it (#2019 at `d4db0887`: the file holds **five** `prefers-reduced-motion` blocks — a bare grep answers six, the sixth being a prose comment, which is this entry's other half showing up in its own evidence — and the first opens at 506 and closes at 539 in a 1,341-line file (`wc -l`), so the slice reaches three unrelated `flex-wrap: wrap` rules and the fallback it pinned could be deleted outright with the suite green). Bound a scope at both ends, and make the helper refuse a scope that matches zero blocks or more than one. *(Earned: 2026-09-29, TASK-199/#2024, and it caught both seats in one file on one afternoon. Reviewing an sr-only census, the reviewer dropped `white-space: nowrap;` file-wide: at `bec75f1b`, the head these were measured at, that exact pattern occurs 5 times in `demo-workspace.css`, the first is line 188 in `.v2-demo__pod-name`, and `.v2-demo__sr` — the rule under test — opens at line 400, 212 lines later. The occurrence guard was counting a BROADER pattern than the mutation used — the bare string `white-space`, which matches 6 times because line 393 is a comment quoting the declaration — so it printed 6 → 5 where the quoted pattern would have printed 5 → 4; that mismatch is this rule's own subject one level down. The census stayed green because nothing it checks had changed, and the green was published as "the one declaration the comment calls load-bearing is the one the census does not pin". The author's own `border: 0;` arm had failed the same way on line 142, in a third rule, and their first account of the reviewer's failure named the wrong site too. Re-run scoped to `.v2-demo__sr`'s own braces: all five declarations red, four of them naming the rule. Its own entry rather than a rider on the mutation-discipline rules, by rule 41's test — every check those send you to run passes here, because the mutation was well chosen, applied, and confirmed applied.)*

## Reading a mutation's result

46. **An arm's verdict is its test TOTAL compared against BASE, not the presence of the word `failed` — a suite that fails to COMPILE reports zero failures and simply stops contributing tests.** Jest counts a collection failure at the *suite* level only: `Test Suites:` names it, and `Tests:` is a sum over the suites that actually ran, so a corrupted tree prints a line with no failures on it and a total that is quietly smaller. The number stays plausible, which is what makes this worse than the empty case our existing discipline already covers — `Tests: 0 total` or a missing `Tests:` line announces itself, and `875 passed, 875 total` does not. Worse still, **the suite you are watching is usually not one of the casualties.** An invariant suite that reads the module as a *string* (`read('../landing/V2LandingPage.tsx')`) never imports it, so it collects, runs and passes over the corruption; the suites that die are the ones that `import` the module, and they are elsewhere in the tree. So the arm you are pointed at goes green while a tenth of the run vanishes. The defence costs one line: **have every arm print its own total and flag any total ≠ BASE's**, and treat a mismatch as a broken instrument rather than a result. Grepping `^Tests:` alone is not enough — that is the line engineered to look fine. The generalisation is worth holding onto beyond jest: **any per-test aggregate is blind to a suite that produced no tests**, which is also how a coverage threshold, a snapshot count or a "N tests added" check can be satisfied by a tree that does not build. *(Earned: 2026-09-29, TASK-209, during the #2022 gate, and it had already been live in two of that day's gate harnesses before anyone noticed. Reproduced at main `abe19fe0` with `npx jest` in `frontend/`: BASE is `Test Suites: 120 passed, 120 total` and `Tests: 1108 passed, 1108 total`; corrupting **one** line — line 48 of `V2LandingPage.tsx`, a broken JSX tag, which is what a mis-quoted mutation regex produces — gives `Test Suites: 5 failed, 115 passed, 120 total` beside `Tests: 1005 passed, 1005 total`. Zero failed on the line a harness reads, and **103 tests absent**. Name the invocation whenever you quote a pair like this, because the totals are a property of the filter: the same corruption under `npx jest src/v2` reads `5 failed, 83 passed, 88 total` and `875 passed, 875 total` — different totals, the **same 103**, because the absent count belongs to the corrupted module’s importers and not to the filter that happened to be running. The five are exactly the suites that import the module — `landingBarInset`, `landingInstallCopy`, `landingHeroContent`, `V2LandingPage.stats`, `V2Login` — each a `SyntaxError` at collection; `v2-layout-invariants`, the suite nearly every CSS and TSX arm is aimed at, is **not** among them, because it reads the file rather than importing it. The reviewer's own harness printed only the `Tests:` line, so the first run of this corruption was recorded as `870 passed, 870 total` and read as a clean green; the arm that produced it was reported as a real result for several minutes before the suite-level line was looked at. Its own entry rather than a rider on 45, by rule 41's test: 45 sends you to confirm the edit's SITE, and every one of those checks passes here — the site was right, printed, and inside the intended braces — while the run it produced was not a run.)*
Loading