Repository navigation
tech-debt(ledger): a review stamp pins the head SHA, so a PR body edited after approval is invisible to it #16874
Description
Activity
👋 Auto-triage could not confidently classify this issue (no confident signal). Could a maintainer add the appropriate labels (
frontend,backend,infrastructure,docs,testingandgood-first-issue,intermediate,advanced)?Root cause of the recurring
## Model Usedloss, found on #16853 — the PR it has now regressed on three times. It is more specific than this issue's framing, and it has a one-line fix.CodeRabbit rewrites the region between its markers wholesale, and anything placed inside that region is deleted on its next update.
The PR body's edit history (
userContentEdits) shows the pattern with no ambiguity — every human edit contains the heading, and every CodeRabbit edit that follows drops it:Edited Editor ## Model Usedin body09-17 04:37 mrveiss no (original, never had it) 09-17 05:17 mrveiss yes (restored) 09-17 11:34 coderabbitai no 09-17 13:34 mrveiss yes (restored again) 09-17 16:49 coderabbitai no The 13:34 body shows exactly why:
59: <!-- This is an auto-generated comment: release notes by coderabbit.ai --> 60: ## Model Used <- inside CodeRabbit's region 64: ## Summary by CodeRabbit 77: <!-- end of auto-generated comment: release notes by coderabbit.ai -->The bug was mine. My restore script inserted the heading immediately before
## Summary by CodeRabbit— an anchor that looks like the start of CodeRabbit's section but sits two lines inside it, after the opening marker. So each restore placed the heading where CodeRabbit would regenerate over it, and each "fix" armed the next regression.The fix is to anchor on the marker, not the heading: insert before
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->. Applied to #16853 — heading now at line 57, marker at 61 — and the template gate passes locally against the live body.I audited my other open PRs (#16854, #16444, #16911, #16916, #16922): all five have the heading outside the region. #16853 was the only one where the restore ran after CodeRabbit's region already existed.
What this means for this issue
This issue's premise — a stamp pins the head SHA, so a body edited after approval is invisible — is still true and still worth fixing; a stamp should not survive a body change. But it is not what drove the recurrence. The body was not edited after approval by a person; it was rewritten by a bot, deterministically, every time CodeRabbit updated its summary. Even a stamp that tracked body content would have been invalidated three times without anyone making a decision.
Two things worth taking from it:
- Anyone restoring a PR-body heading by anchoring on
## Summary by CodeRabbitwill hit this. That is the natural anchor to reach for and it is the wrong one.scripts/validate_pr_body.py(ci(process): PR body requirements are undiscoverable before push — gh --body-file never loads the template, and the gate's author failed it twice #16859) would be the place to warn if a required heading is found between CodeRabbit's markers — a heading inside that region passes the template gate today and will be deleted by the next bot update, so it is a pass that will not last. - The recurrence looked like a people problem ("authors keep forgetting") and was a placement problem. It was a pass that could not persist, which reads as a regression nobody can explain.
- Anyone restoring a PR-body heading by anchoring on
A ledger stamp pins a head SHA. A PR body is editable without moving it.
Found while sweeping the merge queue: three PRs had bodies missing content a reviewer had confirmed
present at approval time — #16328, #16854 and #16856. In each case the ledger stamp still matched the
head SHA, so every SHA-based check said the approval was current.
A
ledger review approve@<sha>verdict covers the tree at that SHA. It covers nothing about the PRbody, which GitHub lets anyone — or any bot — edit without producing a new commit. So the two can
diverge silently, and the gates that read the body (
Validate PR template sections,Check same-scope batching,Check PR links to its issue) fail on a PR whose stamp looks perfectlycurrent.
The mechanism, caught in the act
On #16856 the body ends with
## Summary by CodeRabbitand## Model Usedis gone. The botappended its section and the required heading did not survive. The PR is 42's fix to the batching
gate's wording — so a bot edit silently removed the heading from the very PR whose commit message
describes that heading being missed twice already.
#16854 lost its issue-linking keyword the same way:
#16843appears in prose with noRefs/Closes, soCheck PR links to its issuefails while the work itself is fine.Why this is not just "re-read the body"
A reviewer did read these bodies and confirm them. The verdict was correct when made. What is missing
is any mechanism for the verdict to notice it has been invalidated — the same shape as a green CI run
that describes a tree the PR no longer has, except here nothing moves at all, so no freshness check
fires.
It also defeats the cheap re-verification the fleet has been relying on: "the PR's own files are
sha256-identical to what I reviewed, so this is a diff-verify" is sound about the tree and says nothing
about the body.
Options
ledger review, soledger pr <n>can reportbody-changed-since-approvalthe way it already reports a stale head. Cheapest and it makes the gapvisible rather than requiring vigilance.
memory, and is exactly what failed here — the reviewer had already read it.
constantly, which trades a silent failure for a noisy one.
Option 1 looks right: the ledger already distinguishes stamped from stale, and this is the same
distinction applied to a second surface. Worth noting the failure is not hypothetical — it cost
three PRs a red apiece today, and two of them were blocked on the gate that the third PR exists to fix.
Bodies restored on #16856 and #16328; #16854's keyword is still missing and belongs to its author.
Found during the v0.9.0 merge-queue sweep. Refs #16855, #16859.