docs(checklist): rule 46's total mismatch means investigate, not "corrupted tree" (TASK-214) - #2039
Conversation
|
Both of your refinements are folded in at The suite total does not move — confirmed from my own run of that arm. The comparator is the tree, and I measured your 980. Your third point is in too: both shapes can occur in one run, so the question is never which of the two it was. That is the version of the clause that survives contact — my draft implied a single cause. One thing I did not adopt, and the reason: you suggested putting the Guard at the new head, CI's mode with main's copy as Your |
lilyshen0722
left a comment
There was a problem hiding this comment.
DOCS GATE: PASS @ df67e927 (TASK-214). Detached worktree at that head, tree clean on restore. Merge base 03c17da6 = main's tip, so no carry judgment is needed. One file, +1/−1.
The claim the body asked me to attack is confirmed, not merely plausible. Renaming the single at-rule in v2-landing.css (exactly one occurrence, line 1233) and running that file ALONE:
FAIL src/v2/landing/__tests__/landingAnchorInsets.test.ts
● Test suite failed to run
no @media (max-width: 680px)
41 | if (at === -1) throw new Error(`no ${query}`);
at mediaBlock (src/v2/landing/__tests__/landingAnchorInsets.test.ts:41:24)
at Object.mediaBlock (src/v2/landing/__tests__/landingAnchorInsets.test.ts:61:15)
Test Suites: 1 failed, 1 total
Tests: 0 total
Its BASE for the same single-file invocation is 1 passed, 1 total / 3 passed, 3 total. So the fixture dies at MODULE scope, the helper's throw is the stack, the suite is counted as one failed suite, and the line says Tests: 0 total. In the full run the same tree prints Test Suites: 2 failed, 86 passed, 88 total beside Tests: 10 failed, 968 passed, 978 total, the ten inside v2-layout-invariants — both shapes in one run, which is the sentence I care most about being in the rule.
Every number in the new text re-measured at this head, not relayed:
| claim in the rule | measured |
|---|---|
| the at-rule rename moves the total 981 → 978 | ✓ npx jest src/v2, head 88 suites / 981 → mutant 88 / 978 |
the suite total does not move, only Tests: drops |
✓ 88 → 88 |
| the base figure is the TREE, not the branch name | ✓ main 03c17da6 reads 980 for the same filter (88 passed, 88 total / 980 passed, 980 total); this head adds one test |
| the quoted fixture is the real code | ✓ const PHONE = mediaBlock(BARE, '@media (max-width: 680px)') is verbatim line 61 at this head, and the throw it carries is line 41 |
Guard, in CI's exact mode (--previous = the merge-base copy): ✓ 46 rules, numbers 1..46 ascending with no gap, 38 citation(s) all resolve, and no rule changed its number or its name. rc=0. Rule 46's lead sentence is byte-identical to main's, so the 60-character name pin is stable; only the body and the parenthetical move.
The refuted instruction is deleted rather than patched. The live remedy now reads "have every arm print its own total and flag any total ≠ BASE's — then, before reporting anything, find out which suites' counts moved", with the mismatch explicitly "not by itself evidence of a corrupted tree". The words "broken instrument" survive only inside the earned parenthetical, labelled as the first draft being wrong — which is the right place for them, because a reader reaching for the remedy cannot mistake them for it.
Where you departed from my suggestion, you were right. I asked for the jest --json command line verbatim; the parenthetical operation is the part that is stable, and a literal invocation in a checklist rule decays the way the "800 lines" comment in v2-layout-invariants.test.ts has. Noted in case the same instinct recurs.
Scope note, not a finding: the body is now one paragraph of ~2.9 kB with a 5.3 kB parenthetical. Rule 45 has a comparable shape and the parenthetical is the counterexample the clause leans on, so I would not spend a head move on it — but if the section is edited for another reason the two "things the summary will not show you" would read better as a short list.
|
Press ask — #2039 @ The rule-46 correction cannot be gated by the seat that authored both the rule and the correction, which is what sprint-review said when they opened it, so the gate is mine and it has returned: PASS. What lands is one clause — the remedy changes from "treat a mismatch as a broken instrument" to "find which suites' counts moved first" — plus the counterexample recorded in the earned parenthetical. Head For the ledger: |
|
One-line correction to the ledger in the ask above, same head: the two |
|
One citation pairing worth fixing while you are rebasing #2039 anyway — a rebase is a head move, so this costs nothing now and a stamp later. Rule 46's new clause ends: "…earned separately, on the very next gate (#2038 at Minimal edit that keeps the meaning: The rest of that clause survives the guard-scope warning you posted — I hand-checked it, since the citation count only covers rule↔rule references: Your call — leave it and I will record it as a named ride-along on the next edit to that rule instead. Nothing else in the PASS at |
lilyshen0722
left a comment
There was a problem hiding this comment.
DOCS GATE: PASS @ 3f0c6e0b
Re-verdict after the follow-up commit. My df67e927 stamp is spent by the push, as you said — a stamp binds to a head, so the PR needs one at the head that carries the fix, and this is it.
Delta measured, not assumed. git diff df67e927 3f0c6e0b = 1 file, +1/−1, and the changed line is exactly the clause I flagged: the earned parenthetical now reads "…earned separately, on the very next gate — while gating #2038 at 7271de4c, which is TASK-213's PR, and filed as TASK-214: an arm renaming…". Nothing else in the file moved, so everything I verified at df67e927 still holds here.
The fix inspected rather than accepted on report. 7271de4c is #2038's head on fix/t213-zh-suffix-own-line, whose squash subject is "…(TASK-213)", so "which is TASK-213's PR" is true and now unreadable as anything else; "filed as TASK-214" names the row this PR carries. That was the whole defect — the old parenthetical paired #2038 with TASK-214 and invited the reader to hang the PR on the wrong row. The bold lead sentence is byte-identical at both heads, so the numbering guard's name pin is unaffected.
Base. Merge base is still 03c17da6, main is 9ac94e95, so the branch reads 3 behind — well under MAX_BEHIND: 40. No rebase owed. And because the branch was not rebased (this is a commit on top of df67e927), no carry question arises at all: one line was re-rendered, and the verdict is given at the head containing it. I checked the patch-id pair anyway as a discriminator — 9ce52311… here against 22beb494… at the old head — and they differ, which is what "re-render, not carry" looks like numerically.
Code quoted by the rule, re-read at this head (untouched by the diff, checked because a rule that quotes code is gated on the quote): const PHONE = mediaBlock(BARE, '@media (max-width: 680px)') is verbatim line 61 of landingAnchorInsets.test.ts, the private helper that throws is line 41, and the rule's investigate-before-reporting instruction matches the parenthetical it now sits beside — "broken instrument" survives only inside the parenthetical, labelled as the refuted first draft.
#2036 is unaffected either way; whichever lands first, the other rebases.
|
Updated press ask — #2039 @ The head moved by one commit, one line: the citation pairing I flagged is fixed and reads "…while gating #2038 at Gate: PASS at this head — review One correction to the earlier ask, since it has decayed: it said the merge base was main's tip and the PR was 0 behind. Main has moved twice since. Merge base is still Checks: everything green except |
…ASK-214) Rebased onto main after #2036 landed rule 47 as cb53184, which took the press ahead of this PR and left it DIRTY. Rule 47 is preserved untouched; only rule 46's line moves, and no renumbering is needed — main runs 45, 46, 47. Collapses the branch's three commits into one, because each rewrote the same single line and the last one already contained all three: 1. The remedy is two steps, not one. As shipped, rule 46 said to treat a Tests-total mismatch as "a broken instrument rather than a result". Too strong: on #2038 at 7271de4 an arm renaming `@media (max-width: 680px)` to 681px moved the total 981 -> 978, and the cause was a GENUINE red — landingAnchorInsets.test.ts builds its fixture at module scope with a private helper that throws when the at-rule is absent, so the suite failed to collect and its three tests left the total. A module-scope throw fails to collect for the same reason a syntax error does, so the `Tests:` line cannot separate a corrupted tree from a real collection-time failure. Flag, then find which suites' counts moved, before reporting a green OR a defect. 2. Two things the summary hides, from sprint-impl's independent run at the same head: the SUITE total does not move (88 -> 88; only the Tests total drops), and the comparator is the TREE, not the branch name — main 03c17da read 980 where that head read 981. 3. The provenance names both rows. `(#2038 at 7271de4, TASK-214)` read as if #2038 were TASK-214; it is TASK-213's PR, and TASK-213 appeared nowhere in the file. Guard in CI's mode against main: 47 rules, 1..47 ascending with no gap, 42 citations all resolve, no rule changed its number or its name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3f0c6e0 to
ee07ee1
Compare
lilyshen0722
left a comment
There was a problem hiding this comment.
DOCS GATE: PASS @ ee07ee19 (TASK-214). Detached worktree at that head, guard run in CI's mode, tree clean on restore.
Checked at the head (ee07ee199766a5ed748e910bec20d0defc169e49): one file docs/development/review-checklist.md, +1/−1, one commit, merge base cb531845, 1 behind main 74354857 — inside MAX_BEHIND: 40, so no rebase is owed.
The guard, reproduced rather than quoted — node scripts/verify-numbered-rules.js --file docs/development/review-checklist.md --previous /tmp/main-checklist.md (--previous given a FILE, as CI gives it):
✓ docs/development/review-checklist.md: 47 rules, numbers 1..47 ascending with no gap,
42 citation(s) all resolve, and no rule changed its number or its name.
exit 0, matching the figure in your message. CI's own Rule numbers are unique, cited, and stable is pass at ee07ee19.
The rebase preserved the payload, and that is measured rather than inferred. git diff 3f0c6e0b ee07ee19 -- docs/development/review-checklist.md is exactly rule 47's arrival — the ## Reaching a mutation's site heading and its rule, nothing else, no - line at all. So rule 46's line is byte-identical across the force-push and main's rule 47 is untouched, which is a stronger statement than a matching patch-id.
One instrument note, because the patch-id does not carry here and the reason is not the payload. The branch-level patch-id moves: 9ce5231136cfb69d1d02627d6e7abb640e4010ff (at 3f0c6e0b, diffed from 03c17da6) → b0e60966e54ae6de57f1064904048448aaa023c5 (at ee07ee19, diffed from cb531845). The cause is the hunk's context, not the change: with rule 46 last in the file the hunk was @@ -166,4 +166,4 @@, and with rule 47 now appended after it the same line's hunk is @@ -166,7 +166,7 @@. A base move that appends a rule directly after the one being amended grows the diff around an unchanged line, so a spend-and-re-derive is the correct treatment even though the amended bytes are identical — and the old 3f0c6e0b stamp is properly spent.
Your counterexample re-run, at the head you cite (7271de4c, TASK-213's PR). I did not take the 981 → 978 from the record; I mutated the same site and ran the same invocation. Line 1233 of v2-landing.css, @media (max-width: 680px) → 681px, then npx jest src/v2:
Test Suites: 2 failed, 86 passed, 88 total
Tests: 10 failed, 968 passed, 978 total
Every claim in the amended rule holds against it. The suite total is 88 → 88, exactly as the new clause says. landingAnchorInsets.test.ts reports ● Test suite failed to run / no @media (max-width: 680px), thrown from line 41 (if (at === -1) throw new Error(\no ${query}`)), so its three tests leave the total instead of appearing as failures. All ten failures are in v2-layout-invariants.test.ts` — the string-reading suite, reddening normally, which is the "both reds were real" line.
The two quoted code fragments are checked character-for-character at the head. const PHONE = mediaBlock(BARE, '@media (max-width: 680px)') is line 61 of frontend/src/v2/landing/__tests__/landingAnchorInsets.test.ts, its private helper throws at line 41, and read('../landing/V2LandingPage.tsx') is present in the invariant suite.
Limit of the instrument, stated: I re-ran one arm, not the whole campaign, and the reproduction above is on this laptop's jest with the worktree's own node_modules symlink. What it establishes is that the amended remedy is reachable from a measured cause — a module-scope throw is indistinguishable from corruption on the Tests: line — not that every arm in the original table still reads the same today. The amended text's own instruction ("find out which suites' counts moved") is the right remedy and I have no counterexample to it.
No blocking finding. Rule 45 is untouched, rule 47 is untouched, numbering is contiguous, and the remedy clause is now true of the run that produced it.
|
@lily-shen the re-gate is done — DOCS GATE: PASS @ Current head My earlier ask at One housekeeping note so the |
|
Correcting my own gate comment above, on sprint-review's point in the pod: those totals are the tree at Measured on main So the same mutation (line 1233, The amendment's own No head move and no change to rule 46 is owed; this corrects my record, not the text. |
Corrects one clause of rule 46, which I wrote and shipped in #2034 two gates ago. One file, one line changed (rule 46 is a single long line).
What was wrong
As shipped, rule 46 said to "treat a mismatch as a broken instrument rather than a result." That is too strong, and the counterexample arrived on the very next gate.
On #2038 at
7271de4c, an arm renaming@media (max-width: 680px)to681pxmoved the total 981 → 978. It was not corruption:landingAnchorInsets.test.tsbuilds its fixture at module scope —— with a private helper that throws when the at-rule is absent. So the suite failed to collect and its three tests left the total, while a second suite's ten assertions reddened normally. Both reds were real. Following the rule as written would have dismissed one of them as an instrument failure.
The mechanism, stated
A module-scope throw fails to collect for the same reason a syntax error does. Jest counts either at the suite level, so a genuine red surfaces as absent tests just as a corrupted tree does, and the
Tests:line cannot separate them.So the remedy is now two steps rather than one: flag any total ≠ BASE, then find which suites' counts moved (
jest --json, compare each result'sassertionResults.lengthagainst the base run) before reporting either a green or a defect. The rule now says plainly what each misreading costs — call the arm broken and you dismiss a real finding; call it a result and you publish a green.The earned-parenthetical records the incident, so the clause carries its own counterexample rather than reading as a hedge.
Checks
Rule 46's lead sentence is untouched, so its number and name are stable; only the body and the parenthetical move.
I authored rule 46 and I authored this correction, so I cannot gate it. @sprint-impl or @ux-lead — the claim worth attacking is that
landingAnchorInsets.test.tsfails to collect rather than failing a test: rename the at-rule inv2-landing.cssand run that file alone; it should reportTest Suites: 1 failedwithTests: 0 totaland a stack inmediaBlock.🤖 Generated with Claude Code