fix(scripts): correct the silent-revert incident corpus attribution - #2832
Merged
Conversation
The canary's own detector prints `content from bfb66be (#2644)` and `content from eda5ae5 (#2642)` for the #2633 squash, while its incident corpus recorded that same commit as dropping "#2632's rollups ... unfiled until now". Every part of that note was wrong. #2632 never merged: mergeCommit null, closed unmerged 2026-08-14T23:19:00Z, head 2b9391b not an ancestor of main, zero occurrences of "rollup" in its diff. Blaming the 1165 lines cc58cbc deleted attributes 853 to bfb66be (#2644, finding rollups) and 301 to eda5ae5 (#2642, aliased GraphQL merge evidence) -- two victims, not one, and neither of them #2632. The 853 figure was correct but unqualified: it is a blame attribution, not the squash's diffstat (1165) and not what #2644 added (947). Both figures now say what they measure. The "unfiled until now" claim was false. #2656 recorded this merge event, pinned to the same commits, ~23 hours before the canary merged -- from the test-coverage angle rather than as a silent revert, so the canary's real contribution is detection speed, not discovery. Comments and fixture note text only. No non-comment line of check-silent-revert.sh changes, no threshold or window moves, and --verify-known-incidents still passes against the unchanged shipped settings. Refs: #2831 Refs: #2808 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 37s —— View job Security review
No security issues found. This PR touches only comments in |
|
Claude finished @kyle-sexton's task in 2m 52s —— View job Code review —
|
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29d5a90bfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…sized counts Review of the previous commit caught it overreaching: "Every line count above is a BLAME ATTRIBUTION" swept in the 2178 -> 1700 and 1643 -> 1424 figures, which are whole-file line counts, not blame attributions. In a change whose whole point is saying what a number measures, that sentence had to be exact. Also restores "through all three" in the workflow header, dropped in the previous commit only to fit the wrap. Comments only; the non-comment diff against origin/main is still empty. Refs: #2831 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 41s —— View job Code review —
|
|
Claude finished @kyle-sexton's task in 1m 43s —— View job Security review
No security issues found. This PR (now at head Independently verified for this head:
No logic, permissions, triggers, or trust boundaries changed — this is a documentation/attribution correction only, consistent with the skip gate (criterion 2: no security-relevant surface). |
This was referenced Aug 16, 2026
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
…proves Verification of the previous commit caught the setup paragraph still asserting the thesis the conclusion now demolishes: "The highest verified-legitimate commit scores well below the threshold below. The separation is what makes the canary livable." Both halves are false at the shipped settings. The highest verified-legitimate fire is 390 (#2640) against a threshold of 200 -- nearly double it, not well below. And there is no separation to rest on: the smallest true finding is 301, under both legitimate fires (340, 390), so the populations overlap. Replaced with what the measurements actually support -- content-based detection fires on all three incidents where the two rejected designs exonerate them, and it is the disposition path plus the non-blocking posture, not any threshold, that keeps the canary livable. Both sentences predate #2832 and were already false on main (340 > 200 there too). They are repaired here because this branch rewrote the paragraph they sit in, and because leaving the file self-contradictory on its central claim is the same defect class this PR exists to close. Comments only. The non-comment body of check-silent-revert.sh remains byte-identical to origin/main, both fixtures' parsed fields are unchanged, and 27/27 detector tests plus --verify-known-incidents pass at the unchanged shipped settings. Refs: #2846 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
…proves Verification of the previous commit caught the setup paragraph still asserting the thesis the conclusion now demolishes: "The highest verified-legitimate commit scores well below the threshold below. The separation is what makes the canary livable." Both halves are false at the shipped settings. The highest verified-legitimate fire is 390 (#2640) against a threshold of 200 -- nearly double it, not well below. And there is no separation to rest on: the smallest true finding is 301, under both legitimate fires (340, 390), so the populations overlap. Replaced with what the measurements actually support -- content-based detection fires on all three incidents where the two rejected designs exonerate them, and it is the disposition path plus the non-blocking posture, not any threshold, that keeps the canary livable. Both sentences predate #2832 and were already false on main (340 > 200 there too). They are repaired here because this branch rewrote the paragraph they sit in, and because leaving the file self-contradictory on its central claim is the same defect class this PR exists to close. Comments only. The non-comment body of check-silent-revert.sh remains byte-identical to origin/main, both fixtures' parsed fields are unchanged, and 27/27 detector tests plus --verify-known-incidents pass at the unchanged shipped settings. Refs: #2846 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 17, 2026
…ributions (#2843) Closes #2837. Closes #2833. ## Summary Three defects in the silent-revert canary, plus one unfiled harness-safety fix in a file this change already owns. Branched off `2de57a379` (#2832), which had already merged. ## Fix ## 1. `declares_removal()` could not read the only revert subject that merges here (#2837) The detector accepted three intent forms. This repo is squash-only with `squash_merge_commit_title: PR_TITLE`, so the squash subject is the PR title, and `.github/workflows/pr-title.yml` gates every title through a required Conventional-Commits check whose default type list is all-lowercase — it admits `revert:` and contains nothing a `Revert "…"` subject could match. So a deliberate revert reached `main` wearing a subject the detector could not read. `declares_removal()` now also accepts the Conventional-Commits revert type, anchored at the start of the **subject**, requiring the literal lowercase token, its optional `(scope)` and/or `!`, its colon, and a non-empty description: ``` ^revert(\([^()]+\))?!?:[[:space:]]*[^[:space:]] ``` Never a substring search for "revert" — kept exactly as constrained as the three forms beside it. ### Before / after on the repo's one real deliberate revert `1d1fca6e8` — `revert: remove Cursor dual-target marketplace manifests (#1835) (#1839)` Before (at `2de57a379`, shipped detector, thresholds unmodified): ``` $ bash scripts/check-silent-revert.sh --commit 1d1fca6 SILENT REVERT SUSPECTED removed by 1d1fca6 revert: remove Cursor dual-target marketplace manifests (#1835) (#1839) 2026-07-30 19:41:47 -0400 content from b6c4b58 feat: add Cursor dual-target marketplace manifests (#1835) 2026-07-30 18:17:36 -0400 (1 commit(s) earlier on main) lines lost 3361 (threshold 200, window 40 commits) [... 102 files, sample block and "What to do" block elided; 130 lines total ...] EXIT=1 ``` After: ``` $ bash scripts/check-silent-revert.sh --commit 1d1fca6 declared 1d1fca6 revert: remove Cursor dual-target marketplace manifests (#1835) (#1839) removal is declared: the subject carries the Conventional-Commits revert type EXIT=0 ``` ### Suppression is not widened over anything the corpus records - No recorded incident has a `revert`-prefixed subject — `f603880da` `fix(disk-hygiene): …`, `9239f1541` `feat(disk-hygiene): …`, `cc58cbc53` `fix(repo-fleet-hygiene): …`, `c8470efd0` `docs(conventions): …`. All four rows still hold (green run below). - Neither acknowledgment-file commit is revert-prefixed either — `6f0a31109` `fix(repo-fleet-hygiene): …`, `91e77fc16` `fix(hook-utils): …` — so no ack row goes dead now that `declares_removal()` short-circuits ahead of `ack_reason()`. - The header's "fires on 5 commits — 1%" calibration figure is therefore unchanged; none of those five is revert-prefixed. New tests pin both directions. `revert:`, `revert(scope):`, `revert!:` and `revert(scope)!:` suppress; `feat: do not revert the alpha guard (#99)`, `reverted: drop the alpha guard (#99)`, `Revert: drop the alpha guard (#99)` and a bare `revert:` with no description all still fire. The pre-existing case only covered a *body* mention of "revert"; the subject is what the new form reads, so that is where a substring bug would widen. ## 2. `verify_known_incidents` asserted only "something fired" (#2833) A `fires` row passed on `scan_commit`'s exit status while the note beside it named a specific culprit and a specific line count that nothing checked. On `cc58cbc53`, whose deletions trace to two culprits, losing the `eda5ae5ed` attribution entirely would still have printed `ok` on the surviving `bfb66beb8` finding — the canary announcing a reproduction it did not perform. A `fires` row may now carry a bracketed attribution expectation after its sha: ``` fires <sha> [<culprit-full-sha>=<blamed-lines>,<culprit-full-sha>=<blamed-lines>] <note> ``` and the replay asserts the run's findings are **exactly** that set — same culprits, same per-culprit counts, no extras, no omissions. Full 40-character culprit shas only, the same discipline `silent-revert-acknowledged.txt` uses. Counts come from a new `FINDINGS_SINK` file that `report_finding` appends `<full-culprit-sha> <count>` to, not from scraping the human report — the report prints a 9-character abbreviation, which is not enough sha to assert on. Nothing else sets `FINDINGS_SINK`, so `--commit` and range mode are byte-identical. Every recorded figure was **measured, not transcribed from the notes** — the sink was wired first and the observed values recorded: ``` f603880 -> a95f240 346 9239f15 -> f603880 451 cc58cbc -> bfb66be 853 eda5ae5 298 ``` Three of the four agree with the notes #2832 corrected; the fourth is 298 rather than 301, for the reason in section 5. A malformed field is exit 2 (cannot run), never a FAIL and never a pass — a silently misread expectation is the same false green this file exists to remove. The field is optional so a row can be pinned before its attribution is measured, but a new well-formedness assertion requires every *shipped* `fires` row to carry one. ## 3. The workflow header stated a reason that was not true (unfiled) `.github/workflows/silent-revert-canary.yml` asserted *"There is no `pull_request` trigger, so it can never gate a PR"* — while its own `on:` block has a paths-filtered `pull_request` trigger, added by #2808 to run the detector's unit tests, and explained at length 20 lines further down in the same file. The conclusion holds for a different reason: both scan steps are gated `if: github.event_name != 'pull_request'`, and the lane sits outside `ci.yml` and its `ci-status` aggregate. Wording corrected to the actual reason. **No trigger and no `if:` changed.** `git diff` on that file is comment lines only — one hunk, `@@ -13,6 +13,9 @@`, entirely inside the `#` header. ## 4. The detector's line counts depended on ambient git config (unfiled, found by #2833's new assertion) The first CI run of this branch went red, and the failure is the most valuable thing in this PR. The runner reported the `eda5ae5ed` attribution as **298** lines where my machine measured **301**: ``` FAIL cc58cbc fires, but NOT as recorded recorded attribution: eda5ae5… 301 what the detector reported: eda5ae5… 298 ``` Cause: `attribute_file` called bare `git diff --unified=0`, so it inherited whatever `diff.algorithm` the caller's config carried. I have `diff.algorithm = histogram` set globally; CI has nothing set and therefore uses git's default `myers`. The algorithm changes which lines a hunk calls deleted, so it changes the per-culprit counts this canary **thresholds on**. Reproduced directly: ``` $ GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=diff.algorithm GIT_CONFIG_VALUE_0=myers \ FINDINGS_SINK=… scripts/check-silent-revert.sh --commit cc58cbc… bfb66be… 853 eda5ae5… 298 # 301 under histogram ``` Three lines is harmless in itself. The principle is not: the same drift can carry a count across the 200-line threshold, so a commit could fire on one machine and stay silent on another, and the header's "fires on 5 commits over 500 — 1%" calibration only ever described one algorithm. `attribute_file` now pins `--diff-algorithm=myers -M` explicitly, and the file enumeration pins `-M` too. **Both are git's defaults, so this does not change what CI detects today** — CI already had no `diff.algorithm` set. It makes a local run match CI, not the reverse. `-M` covers the same exposure for rename detection, which the existing header calls "load-bearing rather than incidental": `diff.renames = false` in a developer's config would decompose a `git mv` into delete + add and make relocating a large recent file fire. The recorded figure is now **298**, and the prose figures in the script header and the corpus are corrected with the refutation attached, so nobody re-measuring on a histogram machine "corrects" it back to 301. Worth stating plainly: nothing asked for this. #2833's exact-count assertion turned a silent, config-dependent divergence into a red build on its first run — which is precisely the argument for asserting attributions instead of exit status. ## 5. The test harness could commit the developer's work as `test <t@t.test>` (unfiled) Found the hard way while developing this. `mk_repo` is called as `repo="$(mk_repo)"`, so a `return 1` inside the command substitution cannot abort the suite — the caller just gets `""`. And `""` is not inert: `git -C ""` is documented as a no-op, so the next `add -A` + `commit` staged and committed my uncommitted work into the checkout, authored `test <t@t.test>`. Such a commit cannot be pushed here — it fails `required_signatures` with `no_user`. `mk_repo` now yields a path derived from `SELF_DIR` that does not exist, so every git call against it fails loudly and the assertions go red — fail-closed, which is what a harness that cannot build its fixture should do. Scoped to this file only; if the same `repo="$(mk_repo)"` shape exists in sibling harnesses that is a separate follow-up. ## Verification `scripts/check-silent-revert.sh --verify-known-incidents`, run on a machine with `diff.algorithm = histogram` set globally — it now agrees with CI exactly, because the flags are pinned: ``` ok f603880 fires as recorded, 1 attribution(s) reproduced exactly (#2639 dropped #2635 (346 blamed lines), 13 minutes later) ok 9239f15 fires as recorded, 1 attribution(s) reproduced exactly (#2641 dropped #2639 (451 blamed lines), 10 minutes later) ok cc58cbc fires as recorded, 2 attribution(s) reproduced exactly (#2633 dropped #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (298); already recorded in #2656) ok c8470ef stays clean as recorded (docs(conventions) rewrote 129 lines of a doc #2679 had just added) Canary reproduces every recorded incident at the shipped settings. ``` Injected failure against the **shipped** corpus (`853` changed to `852` in a copy) — the mechanism is proven on real rows, not only on synthetic fixtures: ``` $ SILENT_REVERT_INCIDENTS=<copy with 298 -> 297> scripts/check-silent-revert.sh --verify-known-incidents ok f603880 fires as recorded, 1 attribution(s) reproduced exactly (...) ok 9239f15 fires as recorded, 1 attribution(s) reproduced exactly (...) FAIL cc58cbc fires, but NOT as recorded (#2633 dropped #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (298); already recorded in #2656) recorded attribution: bfb66be 853 eda5ae5 297 what the detector reported: bfb66be 853 eda5ae5 298 A row that fires for the wrong reason is not a reproduction. Do NOT edit the row to match; find out why the attribution moved. ok c8470ef stays clean as recorded (...) The canary no longer reproduces the incidents it was built for. Do not relax the recorded expectations to make this pass. EXIT=1 ``` The commit still fires — exit status alone would have passed this row. Note the row is red on a **one-line** discrepancy in one of two attributions, which is exactly the regression #2833 describes. `scripts/check-silent-revert.test.sh`: **49 passed, 0 failed**, including `replay fails when the finding is attributed to a different culprit`, `replay fails when the recorded line count no longer reproduces`, and `replay fails when one of two recorded attributions stops reproducing`. So the assertion is proven by permanent tests, not only by a one-off injection. ## 6. Review follow-up: an unterminated attribution field read as *absent* Both automated review lanes independently flagged the same real gap, and they were right. A `fires` row whose field opened with `[` but never closed it failed the `[[ "$rest" == \[*\]* ]]` glob, so `attribution` stayed empty, the remainder became free-text `note`, and the row fell back to passing on exit status alone — reintroducing the exact pre-#2833 gap by the one route nobody would look at, and contradicting the contract documented directly above it. A leading `[` now COMMITS the row to carrying an attribution; unterminated takes the malformed path. Reproduced against the real corpus with the closing bracket stripped from the `f603880da` row: ``` check-silent-revert: unterminated attribution field for f603880… (no closing ']'): [a95f240…=346 #2639 dropped #2635 … EXIT=2 ``` The four malformed shapes already pinned all carried a closing `]`, so this one was untested; `[<sha>=40` and a bare `[` are now pinned too. The shipped corpus was never at risk — `t_shipped_data_files_are_wellformed`'s regex covers it — but that is a separate layer and does not hold for a custom `SILENT_REVERT_INCIDENTS`. ## 7. Verification follow-up: `git blame` was still ambient-config dependent Fresh-context verification of section 4 found that fix was only half of one. Pinning the diff flags left `attribute_file`'s `git blame` call bare, so `blame.ignoreRevsFile` — an ordinary setting in any repo carrying a bulk-reformat commit — still decided the per-culprit counts the replay now asserts on. On the real corpus, with that setting naming `bfb66beb8`: | culprit | pinned | with `blame.ignoreRevsFile` | | --- | ---: | ---: | | `bfb66beb8…` | 853 | **259** | | `eda5ae5ed…` | 298 | **322** | On a synthetic fixture it is worse than a wrong number. With the pin absent and that config present, the detector reports **no finding at all** on a genuine silent revert: | | clean config | hostile config | | --- | --- | --- | | pinned | `culprit 40` | `culprit 40` | | unpinned | `culprit 40` | **(nothing — the canary goes silent)** | That is a false green reached through the developer's own gitconfig — the precise failure this canary exists to remove. **The obvious fix does not work, and the comment says so.** `-c blame.ignoreRevsFile=` does *not* clear it: the documented "an empty file name resets the list" applies to the **option**, and the `-c` form was measured leaving the hostile value fully in effect (853 → 259 with the reset supposedly applied). Only `--no-ignore-revs-file` actually resets. The symmetry with the `-c` pins above is wrong here and is deliberately not used. `t_counts_are_immune_to_ambient_git_config` pins the property: the same fixture scanned twice, once under a hostile `GIT_CONFIG_GLOBAL` setting `blame.ignoreRevsFile`, `diff.algorithm` and `diff.renames`, asserting identical exit status and identical per-culprit findings. **Confirmed discriminating** — with the blame pin stripped it fails, and it fails because the hostile run reports nothing at all. `shellcheck`, `actionlint`, `typos`, `bash -n` and `scripts/check-shell-portability.sh` all pass. ## Test plan Run from a clean checkout of this branch, with `unset GIT_DIR GIT_WORK_TREE`: 1. `bash scripts/check-silent-revert.test.sh` — the detector's own unit suite. Expect **49 passed, 0 failed**. Covers all four `revert:` spellings, the four negative subject cases (`feat: do not revert ...`, `reverted:`, `Revert:`, bare `revert:`), the wrong-culprit / wrong-count / missing- attribution replay regressions, all six malformed-field shapes, and `t_counts_are_immune_to_ambient_git_config`. 2. `bash scripts/check-silent-revert.sh --verify-known-incidents` — the real corpus. Expect exit 0 with all three `fires` rows reporting `attribution(s) reproduced exactly` and the `clean` row staying clean. 3. Negative control for step 2: edit `scripts/silent-revert-incidents.txt` to inject a wrong culprit sha (leaving the count correct, so the commit still fires) and, separately, a wrong line count. Each must exit **1** with `fires, but NOT as recorded`. Restore the file afterwards. 4. Set `diff.algorithm = histogram` in global git config and repeat steps 1-2. The numbers must not move — that is what the new flag pins buy. 5. `git diff origin/main...HEAD -- .github/workflows/silent-revert-canary.yml` must show comment-only changes; no executable YAML line may differ. ## Related - Refs #2808 — the PR that merged the canary and its three intent forms. - Refs #2832 / #2831 — the corpus-attribution correction this branches off; #2833 was raised in its review. - Refs #2691 — the original silent-revert audit the corpus is built from. - Refs #1839 — the deliberate revert (`1d1fca6e8`) used as the real-history fixture for #2837. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 17, 2026
…proves Verification of the previous commit caught the setup paragraph still asserting the thesis the conclusion now demolishes: "The highest verified-legitimate commit scores well below the threshold below. The separation is what makes the canary livable." Both halves are false at the shipped settings. The highest verified-legitimate fire is 390 (#2640) against a threshold of 200 -- nearly double it, not well below. And there is no separation to rest on: the smallest true finding is 301, under both legitimate fires (340, 390), so the populations overlap. Replaced with what the measurements actually support -- content-based detection fires on all three incidents where the two rejected designs exonerate them, and it is the disposition path plus the non-blocking posture, not any threshold, that keeps the canary livable. Both sentences predate #2832 and were already false on main (340 > 200 there too). They are repaired here because this branch rewrote the paragraph they sit in, and because leaving the file self-contradictory on its central claim is the same defect class this PR exists to close. Comments only. The non-comment body of check-silent-revert.sh remains byte-identical to origin/main, both fixtures' parsed fields are unchanged, and 27/27 detector tests plus --verify-known-incidents pass at the unchanged shipped settings. Refs: #2846 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 17, 2026
…tor's counts (#2847) Closes #2846. ## Summary The calibration comments in `scripts/check-silent-revert.sh` carry the argument that justifies the 200-line threshold. Two of their sentences depended on which finding is the smallest, and #2832 had already falsified both by recording a fourth true finding. This PR repairs them — and re-derives every figure they rest on against the detector as PR #2843 pins it, because three of those figures move under the pins. Found by fresh-context verification of #2832, after it had merged. ## Why the numbers moved PR #2843 pins `attribute_file`'s git invocations to git's own defaults (`--diff-algorithm=myers`, `--no-ext-diff`, `--no-textconv`, `--no-ignore-revs-file`, `-M`). The original calibration was taken on a machine carrying `diff.algorithm = histogram`, and the algorithm choice changes which lines a hunk calls deleted. Every figure below was re-measured against the pinned detector and reproduced byte-identically with `GIT_CONFIG_GLOBAL` emptied, which is the property `t_counts_are_immune_to_ambient_git_config` asserts. | commit | PR | pre-pin | pinned | class | | --- | --- | ---: | ---: | --- | | `cc58cbc53` | #2633 | 853 | **853** | incident | | `cc58cbc53` | #2633 (#2642's share) | 301 | **298** | incident | | `9239f1541` | #2641 | 451 | **451** | incident | | `f603880da` | #2639 | 346 | **346** | incident | | `6f0a31109` | #2640 | 390 | **447** | cleared | | `91e77fc16` | #2135 | 340 | **323** | cleared | ## Fix **The overlap argument survives; every sentence stating it was re-derived.** The smallest true finding is 298 and both cleared fires score above it, at 323 and 447. The relationship is an inversion, not a narrow gap — so `NO THRESHOLD SEPARATES THEM` is now true by a wider margin than the six-line version it replaces. THRESHOLD stays 200. **The 200-vs-300 sentence inverted and was rewritten from measurement, not patched.** The old text said "at 300 the 301-line finding survives by a single line". Under the pins that finding is 298, so at 300 it VANISHES. The COMMIT set at 200 and 300 is still identical — `cc58cbc53` keeps its 853-line finding — but the FINDING set is not, and that is the sharper argument against tuning. Measured: ``` $ SILENT_REVERT_THRESHOLD=300 scripts/check-silent-revert.sh --verify-known-incidents FAIL cc58cbc fires, but NOT as recorded recorded attribution: bfb66be 853 eda5ae5 298 what the detector reported: bfb66be 853 EXIT=1 ``` Without #2833's attribution expectations that row would have passed on the surviving 853-line finding and announced a reproduction it never performed. The passage now says that, and cites `t_replay_asserts_the_recorded_attribution`, which pins the same two-culprit shape. **Detection and disposition are now distinguished.** The corpus sentence said the canary "fires on 5 commits" and left a reader to assume that is what CI shows. It is not: `6f0a31109` and `91e77fc16` are in `scripts/silent-revert-acknowledged.txt`, so `scan_commit` clears each before it attributes a line. Five commits cross the threshold; three print `SILENT REVERT SUSPECTED`. The prose now states both and says which one the threshold is calibrated against. **The corpus endpoint is pinned.** "the last 500 first-parent commits of main" is a moving window that falsifies itself on the next merge — the same defect class this PR closes. It now reads "the 500 first-parent commits of main ending at `738791c45`". **Two recall gaps are acknowledged rather than implied.** - `attribute_file` swallows `git` stderr on both commands that produce a count, so a failed diff or blame is indistinguishable from nothing-to-attribute and can only subtract. Every corpus enumeration is therefore a floor, and the prose is worded so the caveat is structural rather than an appended qualifier (#2880). - Paths the repository's `.gitattributes` marks `-diff` or `binary` produce no hunks, so their deletions attribute to zero on every machine including CI (#2883). This is a RECALL gap, not a calibration one: no path of that class appears in any commit whose figure is quoted, and the largest such deletion anywhere in the sweep was 72 lines from a `package-lock.json` — well under the threshold. **Also corrects a mischaracterization of #2656** that #2832 introduced, which said that issue recorded the event "rather than as a silent revert". Its Evidence section opens with *"#2633 was a stale-base squash that silently reverted two merged features."* It recorded it exactly as a silent revert — what was coverage-shaped was what it **asked for**. Now quoted rather than paraphrased. ## Not fixed here The fresh-context verifier confirmed each of these; every one sits in prose this PR does not own, and each needs a rewording rather than a renumber. - **"main's 1527-commit history"** (twice). Reproduces at no named endpoint — measured 1546 at `738791c45`, 1549 at `origin/main`. The claims it supports are unaffected and do reproduce: `Revert "` = 0, `revert:` = 1, that one being `1d1fca6e8` (#1839). Renumbering it would be falsified by the next merge, which is the same moving-window defect this PR removes elsewhere. - **"the replay exited 0 for 31h28m"**. The duration reproduces exactly, but it is the content-absence window; the replay itself only existed for about 6h13m of it. #2873's prose, and the same conflation appears once in `silent-revert-incidents.txt`. - **"a four-row corpus"**. There are 5 `marker` rows, and 3 `fires` + 1 `clean` expectation rows; the sentence's own unit is one read per marker, so 5. #2873's prose. - **The repo-wide-grep counterfactual.** Its present-tense half holds, but at the tree where both markers were actually missing, a repo-wide grep would have falsely cleared only the README half — the CHANGELOG copy that makes the claim true today was added by the restore commit itself. #2873's prose. - **The `clean` row's "129 lines"** reads 136 under the pins. Issue #2865 owns that row; correcting it here would collide. ## Related - PR #2843 — merged ahead of this one; it added the pins that move three of the figures here, and its pin table records the pre-pin and pinned columns side by side. This PR layers prose on top of it and changes no pin. - PR #2873 — merged ahead of both; added the restoration markers and the `marker) continue ;;` arm. Untouched here and verified intact after the rebase. - Refs #2880 — `attribute_file` swallows git stderr, which is why the corpus figures are worded as floors rather than exact counts. Acknowledged here, not fixed. - Refs #2883 — paths marked `-diff` or `binary` contribute zero to attribution. Acknowledged here as a recall gap, not fixed. - Refs #2865 — owns the `clean` row whose "129 lines" reads 136 under the pins. Deliberately left to that lane. - Refs #2832 — the corpus attribution correction that added the fourth finding and falsified the two sentences this PR repairs. ## Verification - Every figure re-measured against the shipped detector on this branch, twice — once inheriting ambient config and once with `GIT_CONFIG_GLOBAL` emptied — with identical results. - `scripts/check-silent-revert.test.sh` (101 passed, 0 failed) and both replay modes run green against the merged content. - An independent fresh-context verifier re-measured every number in the calibration comments without access to this reasoning, running the full 500-commit corpus sweep rather than per-commit checks alone. Every figure this PR states reproduced. It raised two defects in the new prose, both fixed here: "the number a reader sees in CI is 3" read as findings when it means commits (three commits, four findings between them), and "roughly once a month" was 12-19x off — the corpus spans 7.9 days with four of its five crossings inside 76 minutes, so that rate claim was removed rather than renumbered, because the corpus measures a burst and no per-month figure is defensible from it. Its full verdict, including drift it confirmed in prose this PR does not own, is recorded in the PR comments. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 17, 2026
…sub-threshold miss (#2905) Closes #2865. ## What this fixes `scripts/silent-revert-incidents.txt` pins a `clean` row that is supposed to be the closest a non-incident got to the 200-line threshold without crossing it — the row that breaks first if a threshold change starts taxing ordinary development. The pinned commit was not that, and its note named a number that is not a pull request. **Defect 1 (the re-pin).** Measured over the file's own 500-commit corpus (`7b47d2253~500..7b47d22`) at the pinned invocations (#2843, `GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1`), the pinned `c8470efd0` scores **136** blamed lines — sixth-closest of the eight commits in the 100–199 band. The true closest miss is **`9a2307c43` (#2189) at 195 lines** from `3584ae1fa` (#2183) — a margin of 5 lines, not the ~71 the old row implied. `9a2307c43` is now the lead `clean` row. **Defect 2 (the wrong PR number).** The old note credited the deleted content to #2679, which is a closed issue in this repository, not a pull request (`gh pr view 2679` cannot resolve it). The blamed lines trace to `6370a44e7`, the squash commit that landed #2715 — `gh api .../commits/6370a44e7/pulls` returns only #2715, and the commit's own body says `Builds on merged #2715 → #2692 → #2690`. The row is kept as a second guard with its note corrected, rather than dropped: it is still a verified-legitimate quiet commit, and keeping it costs a few lines of prose. **Defect 3 (the "once a month" rate)** was already fixed by #2847, which removed the rate claim from `scripts/check-silent-revert.sh` entirely. Nothing in this PR touches it. ## What this does NOT do Neither clean-row figure is CI-asserted, before or after this change. `clean` rows carry no bracketed attribution field (#2879) — their assertion is the absence of findings, which has no per-culprit count to pin — so the 195 and 136 are hand-measured prose, not watched numbers. The old row's recorded 129 drifting to a measured 136 under the pinned invocations without anything going red is exactly that gap, and the section comment now states it so a reader does not mistake the re-pin for an assertion. Both counts are written as floors ("no fewer than") because `attribute_file` drops `git blame`'s stderr (#2880), so any line blame fails on is silently not counted. ## Verification - Spot-checked both figures against PR #2843's pinned invocations before editing: `9a2307c43` reproduces **195** (75 lines `song-forms-examples.md`, 67 `box-model.md`, culprit `3584ae1fa`), `c8470efd0` reproduces **136** (109 lines `persist-findings.md`, culprit `6370a44e7`). - `bash scripts/check-silent-revert.test.sh`: **101 passed, 0 failed** on this branch. - `scripts/check-silent-revert.sh --verify-known-incidents`: exit 0 — all three `fires` rows reproduce their attributions exactly, and both `clean` rows stay quiet. - `scripts/check-silent-revert.sh --verify-restoration`: exit 0 — all 5 markers present. - A fresh-context verifier independently swept all 500 corpus commits twice (complete coverage: 487 `ok` + 2 `acknowledged` + 11 finding commits = 500) and reproduced every figure in the file. Its verdict: **195 at `9a2307c43` is the highest sub-threshold score** — the next highest is 188 (`3d69448cb`) — so the lead `clean` row pins the true closest miss. The two acknowledged commits were re-run with the ack file disabled and score 447 and 323, both above the threshold, so neither could displace it. ## Related - #2843, #2847, #2873 — the three PRs that reshaped the canary and this file ahead of this change; the figures here are measured under #2843's pinned invocations. - #2879 — records that `clean` rows carry no bracketed attribution field, which is why neither figure in this PR is CI-asserted. - #2880 — records that `attribute_file` drops `git blame`'s stderr, which is why both counts are written as floors. - #2831 / #2832 — the same wrong-PR-number defect shape, corrected earlier on the `fires` rows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LwdkpWf6bptu3AqTMoeg2H --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #2831.
The silent-revert canary merged in #2808 ships an incident corpus that
misattributes one of its three incidents. Since
--verify-known-incidentsreplays that corpus on every
push: main, the corpus is the guard's ownhonesty proof, and a wrong row costs it the credibility it exists to earn.
The guard already contradicts its own fixture. Running the shipped detector on
the incident commit prints:
while the corpus recorded that commit as
#2633 dropped #2632's rollups (853 lines) -- unfiled until now.What was wrong, and what it is now
The named victim never merged. PR #2632 is CLOSED, not merged:
mergeCommitnull,mergedAtnull, closed unmerged 2026-08-14T23:19:00Z, head2b9391benot an ancestor oforigin/main, and zero occurrences ofrollupin its diff.
There were two victims, and neither was #2632. Attributing each of the 1165
lines
cc58cbc53(#2633) deleted to the commitgit blamecredits it to:bfb66beb8eda5ae5ed1165 matches the squash diffstat exactly
(
6 files changed, 549 insertions(+), 1165 deletions(-)). #2642 was notmentioned in the corpus at all, despite the detector reporting it as a separate
finding. Corroboration:
plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.shwent 2178lines / 8
graphql/ 9rollupatbfb66beb8→ 1700 / 0 / 0 atcc58cbc53→2348 with both restored at
6f0a31109(#2640) → 2933 on currentorigin/main.853 was correct but unqualified. It is a blame attribution against one
culprit — not the deleting squash's diffstat total (1165) and not what #2644
added (942 insertions in its own squash commit, per
git diff --shortstat d55ffbf5 bfb66beb8). Both 853 and 301 now say what theymeasure, in the fixture and in the script header.
Worth noting for anyone re-measuring:
gh pr view 2644 --json additionssays947, not 942, because the PR-level count is three-dot against the merge base
while the commit diffstat is two-dot against the parent, and main moved under
the branch in between. #2640 diverges the same way (
+1281/−463as a commit,1269/451as a PR). The branch text avoids the trap by citing nolines-added figure at all — it just says the blame counts are not that.
"Unfiled until now" was false. #2656 recorded this merge event, pinned to
the same commits, roughly 23 hours before #2808 merged — from the test-coverage
angle rather than as a silent revert. The corpus now cites it and frames the
canary's contribution as detection speed and automation rather than discovery.
Two judgement calls worth reviewing
The
#2632string still appears twice, in a new note recording that therow previously named it and why that was wrong. That is a refutation, not an
attribution; it is there so the error is not silently reintroduced. Say the
word if you would rather it be dropped entirely.
check-silent-revert.shsaid the canary "fires 5 times" over 500 commits.Measured per commit:
f603880da1 finding,9239f15411,cc58cbc532,6f0a311091 (pre-acknowledgment, 390 lines),91e77fc161(pre-acknowledgment, 340 lines) — 5 commits, 6 findings. The number was
counting commits, so the count is unchanged; the units are now stated so the
6-vs-5 gap is not read as an error.
Blast radius
Comments and free-text fixture notes only.
diff <(git show HEAD~1:scripts/check-silent-revert.sh | grep -v '^\s*#') <(grep -v '^\s*#' scripts/check-silent-revert.sh)is empty — no non-comment line of the detector changed.
expect sha note; onlynotetext changed, and bothexpectandshaareuntouched.
shellcheck,bash -n,actionlint, andtyposall pass.scripts/check-silent-revert.sh --verify-known-incidentsafter the change:Related
corpus row.
two incidents (fix(disk-hygiene): session-honest belt, read-only allowlist, Recycle Bin detection #2639/feat(disk-hygiene): prioritize tidiness over reclaimable bytes in reports #2635 and feat(disk-hygiene): verify redundant checkout evidence #2641/fix(disk-hygiene): session-honest belt, read-only allowlist, Recycle Bin detection #2639) were already recorded correctly
and are untouched here.
corpus in place of the "unfiled until now" claim. Stays CLOSED/COMPLETED.
which is not one of them: it never merged, which is the whole point.
recorded commit still fires, never which culprit or how many lines, so a row
can keep printing a reproduction it no longer performs. Pre-existing, and out
of scope for a change constrained to leave the detector untouched.
🤖 Generated with Claude Code
https://claude.ai/code/session_018S8a1S71VxhLTRWBtMuEvp