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
12 changes: 12 additions & 0 deletions docs/development/review-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,15 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b
## The tree behind a head

49. **A green run measures the tree it ran on, not the commit you cite — every instrument in the loop reads the working tree, so a head can carry a message describing a fix whose bytes are not under it while each check passes.** `git status` is not a step in the loop: jest, `tsc`, `grep` and your own mutation script all read files from disk, and none of them reads a commit. So the green is a fact about a tree, the citation is a fact about a commit, and nothing in the run ties the two together. Three ways they separate, and the middle one is this rule's incident: edits left unstaged while the suite runs green over them; `git commit --amend -F <message>` run with those edits STILL unstaged, which rewrites the MESSAGE and keeps the old tree (without `-a` or `--only` the index is untouched); and a worktree whose HEAD is not the sha you push. The check is the one a consumer runs rather than the one the author ran — **at the moment of the run, `git status --porcelain` is empty and `git rev-parse HEAD` is the sha you will cite** — and after the fact, `git show <head>:<file> | grep '<the change>'`, which is how this rule's incident was refuted from outside the author's own instruments. Where both trees exist as objects the identity assertion is one line and stronger: `git rev-parse <head>^{tree}` compared against the tree you measured. Identical trees mean the artifact really is the one you tested, which is also why a message-only amend is a legitimate no-re-gate move — rule 32's carry by identity, and a different question from the merge-fidelity claim it says tree equality cannot answer once the base has moved. Neighbours: 44 asks whether a clearance EXISTS at the pressed head and cannot see that an author's green was taken somewhere else; 32 compares what landed against what was gated; 39 and 46 read what a run's aggregate says. Its own entry rather than a rider on 39 or 46, by rule 41's test: every check those send you to run passes here — the invocation is named, the count is read, the total matches its base — and the defect survives all of them, because they ask what the run said and this asks what the run enumerated. *(Earned: 2026-09-30, TASK-193/#2019, row B of the five. Two gate findings were edited in the worktree and the FULL frontend suite run over them — 119 suites / 1073 tests green — and then `git commit --amend -F <message>` was run with those edits unstaged, so the pushed head `8a692533` kept the PRE-FIX tree under a message describing both fixes, and "both findings fixed at 8a692533" was published on the PR, in the pod, and on the row. sprint-review refuted it from the ref — `gh pr view`, `git ls-remote refs/pull/2019/head`, and the PR-scoped diff — while every instrument on this side had read the working tree and agreed with the claim, which is the whole reason the class needs its own rule: nothing on the author's side can see it. The remedy is mechanical — stage before amending (`git add -A`, or `git commit --amend --only <file>`), and cite a head only after reading that head. The mirror case was measured the same day on #2049, where the same verb over a CLEAN index left tree `537f3d8394ed0434173ad647cfea54e00ada04b9` identical across a head move: the verb was never the defect, the unstaged index was.)*

## The input that makes them differ

50. **A fixture that supplies only the input where two things are EQUAL greens every mutation that erases the difference between them — so a mutation report on that hunk is vacuous, not clean.** Rule 17 says a mutation only reports which terms the chosen input shape *reaches*; this is the failure one level in, where the shape is reached in every exercised case and only the *distinguishing* case is missing. The mutant and the original agree on the fixture's value, so the arm is green, the line looks unpinned-but-harmless, and nothing in the run says which input was never tried. **The review move is to name the input that makes the two sides differ, before running anything: for a clamp, the value on the far side of the clamp; for a comparison, one where the sides are unequal; for an extraction, a case where the extracted string and its source diverge.** If the suite has no such input, say so — *no distinguishing input, result vacuous* — rather than reporting the hunk as covered or as dead code. And the corollary that costs the most: **when both sides of a comparison can be ABSENT, `null == null` and `None == None` read as agreement**, so assert that each side resolved to a real value before comparing them at all.

Measured on the incident, at `faf43bb20`, and the demonstration is two arms rather than one. `localizeWindow` (`frontend/src/v2/utils/localizeRelativeTime.ts:101`) clamps with `Math.max(1, Math.round(windowMs / 60_000))`. Arm A, clamp removed with the pinning test as it stands: **1 failed / 3 total**, failing at `localizeRelativeTime.test.ts:31` — the `minutes(1)` assertion. Arm B, same mutant with only that one line deleted so the sole sub-minute input is 30s: **3 passed / 3 total, green with the clamp gone.** 30s is exactly 0.5 minutes and `Math.round(0.5) === 1`, so clamped and unclamped return the same string and the arm cannot see the deletion. Totals hold at 3 in every arm, so neither result is rule 46's collapse.

Cited, with the numbers checked rather than recalled: the clamp landed in **#1981** (commit `d331c11ea`, TASK-164) — *not* #1980, which is a CSS sizing PR and carries none of this. The killing case proposed in #1981's code gate (`5332296881`) was 30s and it was **wrong for exactly the reason this rule names**; the fix's author measured 1ms instead and said so in the commit message (`79f5cb48d`). The same shape reached a real gate with one PASS stamped on it in **#1982** (`targetLabel` in `V2ConnectorTools.tsx`, TASK-179): with no seat-target grant in the suite, a row's location and the grant's subject were the same string in every exercised case. ux-lead's render gate FAILed the same head because its fixture carried a seat grant. **The input caught it, not the instrument**: the seat-grant test added in the fix (`83408e61`) reds on the reverted line, 1 failed / 21 passed at `faf43bb2`. It is not React-specific — a zh catalog parity check reported *173 of 173 added keys identical* because every path failed to resolve in **both** dictionaries, so `None == None` read as "untranslated in zh" (its author's account, on the TASK-181 row at 2026-09-27 23:37Z). The real answer was 1 of 178.

A sharper variant, earned the same hour this rule was written and by its own author: **the discriminating input can be PRESENT and get explained away.** Gating #2054 I tested whether nine landed squash subjects equalled `PR title + " (#N)"`, got 8 of 9, and reported the ninth as a presser overwriting the subject box. The rival hypothesis I never ran was the repo's own setting, `COMMIT_OR_PR_TITLE`: a one-commit PR defaults to its commit's subject and a multi-commit PR to the PR title. Adding that one column splits the nine perfectly by commit count — 4 of 4 and 5 of 5 — and across the 60 consecutive squash commits on `main` ending at `faf43bb2` (a positional sample, not a number range — #1988–#2053 holds 66 merged PRs, and the six it adds sit at first-parent positions 61–69), where the two texts diverge on a one-commit PR the commit subject wins 8 of 8 and the title 0 of 8. Eight rows agreed with both readings because both named the same string — the PR title on all five multi-commit rows, and on three of the four one-commit rows a subject identical to the title; **#2031 was the only row carrying any information, and I filed it as the anomaly.** So the move is not only *does the fixture contain a distinguishing input* but *am I discarding the one case that distinguishes* — a lone disagreeing row in an otherwise uniform result is the measurement, not the noise. Caught by @ux-lead, who ran the column I skipped.

Its own entry rather than a rider on rule 17, by rule 41's test: every check rule 17 sends you to run passes here. 17 asks whether the fixture constructs the shape inline or mocks the middleware away — `localizeWindow`'s test calls the real function with a real catalog, and #1982's suite renders the real component. The shape is genuine in both; only the discriminating value is absent, and 17's closing grep cannot see that.
Loading