fix(ledger): repair the branch review ledger and close the loop that corrupted it - #1310
Conversation
…atch Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…corrupted it The review ledger exists to stop repeat branch/PR reviews, but it had drifted far enough that its own throttle no longer worked, and the guard was too weak to notice. Found in docs/branch-review-ledger.md (1113 records): - 146 mojibake lines from an append written through a non-UTF-8 shell, which turned em/en dashes, arrows and ellipses into "???" and U+FFFD. - 46 duplicate records. The exact-duplicate guard missed them precisely because the mojibake made the corrupted twin differ from the clean original. - 21 rows that were not six cells: unescaped prose pipes (up to 12 cells) or a merged Scope+Outcome cell (5 cells). Every consumer reads cells by index. - 4 records written as a "## <date>" heading plus bullet list inside the table, which splits the table and hides the record from every row parser. - 137 HEADs recorded as "see PR head", "pending pushed head", or 8-character abbreviations. No git rev-parse lookup can match those, so the throttle silently never fired and branches were re-reviewed. Repair: 140 corrupted lines restored byte-exact from git history rather than guessed (an arrow and an en dash are not interchangeable in these records), 6 residual separators repaired, the 4 block records converted to rows, exact duplicates dropped, all 1067 rows normalised to six cells. No record's content or verdict was changed. Root cause: rows were hand-written. scripts/branch-review-ledger.mjs adds `npm run ledger:lookup` (resolves HEAD, matches abbreviated SHAs and origin/x, `x`, "PR #N / x" spellings, prints an explicit verdict) and `npm run ledger:append` (stamps the date, resolves the HEAD to a full SHA, escapes pipes, writes UTF-8, refuses duplicates). check-branch-review-ledger.mjs now also fails on mojibake, wrong cell width, heading-style records, impossible dates, and blank lines splitting the table; records dated 2026-07-29 or later must additionally carry a resolvable HEAD and must not repeat a ref/HEAD/scope. The cutoff is the day after the repair so no historical record is retroactively invalidated. sweep-branch-ledger.mjs split rows on raw "|", so an escaped prose pipe shifted every later column, and it compared HEADs by exact string, so an abbreviated record never matched. Both fixed. AGENTS.md, docs/codex-review-protocol.md, docs/branch-cleanup-guide.md, docs/README.md and 12 skill/agent files now point at the two commands instead of "search this file" and a Select-String regex that could not match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also corrects the #85 knip row: the finding was first seen in a worktree with no node_modules of its own, which is the known condition under which knip misreports, so it must be re-run after npm ci before anyone acts on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ledger-fixes-42575f # Conflicts: # docs/outstanding-issues.md
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:18 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
Comment |
BigSimmo
commented
Jul 28, 2026
@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers. Also run Bugbot if not already done on this PR yet. |
PR #1310 is mergeable with required CI green on the fix tip; all 3 Codex review threads are fixed and resolved. Root causes
Changes ( |
bugbot run |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:3e405e8db9
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Jul 28, 2026
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Summary
Testing
|
…pend Codex P2s on PR #1310: lookup used substring scope matching (so branch-cleanup-deletion-pending skipped cleanup), append --supersede could write a ref/HEAD/scope the guard rejects, and 40-char hex was accepted without git rev-parse. Also stop treating parenthetical prose or n/a-embedded hex as ref/HEAD matches. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
cursor review |
Append-only review record for the Codex P2 fix pass and green hosted PR-required aggregate. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Why
docs/branch-review-ledger.mdexists to stop repeat branch/PR reviews. It had drifted far enough that its own throttle no longer worked, and the guard was too weak to notice — every defect below landed CI-green.What was wrong
Found across the ledger's 1113 records:
???and U+FFFD.## <date>heading + bullet list inside the table, which splits the table and hides the record from every row parser.see PR head,pending pushed head, or 8-character abbreviations. Nogit rev-parselookup can match those, so the throttle silently never fired and branches were re-reviewed.Tooling gaps behind it:
sweep-branch-ledger.mjssplit rows on a raw|(an escaped prose pipe shifted every later column) and compared HEADs by exact string;docs/branch-cleanup-guide.mdprescribed aSelect-Stringregex on the full SHA that could never match an abbreviated row.What changed
Repair. 140 corrupted lines restored byte-exact from git history rather than guessed — an arrow and an en dash are not interchangeable in these records. 6 residual separators repaired, the 4 block records converted to rows, exact duplicates dropped, all 1067 rows normalised to six cells. No record's content or verdict was changed; no record removed except exact duplicates, which this file's contract has always allowed.
Root cause. Rows were hand-written.
scripts/branch-review-ledger.mjsadds:npm run ledger:lookup -- <ref> [--head <sha>] [--scope <text>]— resolves the HEAD, matches abbreviated SHAs andorigin/x/`x`/PR #N / xspellings, printsALREADY REVIEWEDorNOT REVIEWED at this HEAD.npm run ledger:append -- --ref --head --scope --outcome --checks— stamps the date, resolves the HEAD to a full SHA, escapes pipes, writes UTF-8, refuses exact and ref/HEAD/scope duplicates.Guard.
check-branch-review-ledger.mjsnow also fails on mojibake, wrong cell width, heading-style records, impossible dates, and blank lines splitting the table. Records dated 2026-07-29 or later must additionally carry a resolvable HEAD and must not repeat a ref/HEAD/scope. The cutoff is the day after the repair, so no historical record is retroactively invalidated.Docs.
AGENTS.md,docs/codex-review-protocol.md,docs/branch-cleanup-guide.md,docs/README.mdand 12 skill/agent files now point at the two commands instead of "search this file".Risk
The repair rewrites 226 historical lines mid-file. Open PRs that merge
origin/mainafterwards take the repaired lines by ordinary 3-way merge; only a same-hunk conflict would letmerge=unionkeep both copies. Tracked as#088indocs/outstanding-issues.md— runnpm run check:branch-review-ledgeron the first few post-repair merges.This branch already exercised it: merging
origin/main(25 commits, ~30 new ledger records) auto-merged the ledger with no conflict. The merge did surface two blank lines inside the record table that main already carried — they split it into separate markdown tables, the old guard could not see them, and the new one blocked the merge until both were closed. Whitespace only; no record touched.No retrieval, ranking, clinical, privacy, auth, or schema surface is touched.
Checks
npm run verify:pr-localPASS on the merged tree: runtime, installed-lock parity,format:changed,lint,typecheck,test(405 files / 4126 passed, 3 skipped),build(compiled in 3.7min),check:rag:fixtures.npm run check:branch-review-ledgerPASS — 1092 records, six cells each, union merge active, no conflict markers, mojibake, heading records, or duplicates. Twelve new unit tests intests/repo-hygiene.test.ts.No provider-backed check was run (no OpenAI, Supabase, or hosted-CI call).
🤖 Generated with Claude Code