issues: archive five completed ledger rows, delete none - #1428
Conversation
Triage pass over all 58 open items found five rows whose work is finished and whose next action is empty. Each moves from Open items to Resolved / archive with its fix evidence and the 2026-07-30 date: - #95 the pr-required aggregate now routes a cancelled result through a shared cancelled_error helper; guarded by seven cases that execute the extracted script, three mutation-proven. The red is deliberately retained, since GitHub counts a skipped required check as passing. - #96 every PR #1316 sub-item is dispositioned: the adoption-gate root-path gap closed on PR #1394, four findings were fixed independently, and the Therapy Compass retry-waiter finding was corrected to not-a-live-defect. - #104 a correction row with no next action - the worker's triple image read is an accepted peak-memory trade-off documented at worker/main.ts:866-869, not debt. Archived so a fourth audit does not re-file it. - #109 the branch sweep refuses on a shallow clone, an indeterminate result is its own failure, and the guard moved into the exported collector so the evidence-pack path fails closed too. - #115 the band adoption gate was redesigned to walk a real reachability graph rather than asking whether a file mentions the band. Nothing is deleted. The ledger contract is archive-only (SKILL.md:44 "Archive, never delete"; this file's own conventions: "rows are archived, not deleted, so the history stays auditable"), so no row qualifies for deletion. Row total is unchanged at 120: 58 -> 53 open, 62 -> 67 archived. Prettier widened the archive Outcome column to fit the new evidence, which repads the other archive rows; git diff --ignore-all-space is 7 insertions / 7 deletions, i.e. the five moved rows plus both separators. Verified: node scripts/check-outstanding-issues.mjs --self-test && node scripts/check-outstanding-issues.mjs -> "Outstanding-issues guard passed: 120 rows (53 open, 67 archived), unique ids, next-id=126 above the highest". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe branch review ledger gains one append-only approval record for ChangesReview ledger update
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
main's #1418 (ledger merge dedupe + L4 quarterly archive rotation) and #1413 both edited docs/outstanding-issues.md, so this was a real content conflict rather than staleness: git merge-tree --write-tree confirmed CONFLICT before any resolution was attempted. Resolved by taking main's version of the ledger wholesale and re-applying this branch's five-row archive move on top, so neither side's work is lost: - from main: #88 and #97 archived, new open row #126 (quarterly ledger rotation) with queue order 35, the #23 "When" update (release-browser-matrix no longer blocked by pr-required), the #86 detail update, and the issues:next-id bump to 127. - from this branch: #95, #96, #104, #109 and #115 moved from Open items to Resolved / archive. No row from either side was dropped, and no id appears in both tables. Verified: 121 rows (52 open, 69 archived), marker next-id=127 above the highest; each of #88, #97, #95, #96, #104, #109, #115 resolves to exactly one archive row and #126 to one open row; zero conflict markers remain. npm run verify:cheap -> EXIT=0; "Gate-manifest OK: all 29 verify:cheap gates are enforced in CI"; "Test Files 431 passed (431)"; "Tests 4496 passed | 4 skipped (4500)". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:be27fbbe50
ℹ️ 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.
Two things in one commit because the merge and the fix are on the same file and the PR's CI cancels in-progress runs on every push. Merge: main advanced 28 commits to b1f5718 and had again edited docs/outstanding-issues.md, so git merge-tree confirmed a real conflict rather than staleness. Resolved as before by taking main's ledger wholesale and re-applying this branch's five-row archive move; all five were verified still open on main first, so the move is still the correct change. 125 rows (56 open, 69 archived), marker next-id=128. Correction (Codex P2 on PR #1428, docs/outstanding-issues.md:224): the #95 archive record described the fix inaccurately, and the claim was verified against .github/workflows/ci.yml before being accepted: - It named "a shared cancelled_error helper". No such helper exists at any commit on this branch - grep finds nothing at be27fbb or at HEAD. The real implementation is a single record() collector (ci.yml:739) that reads each job's own result into failures/cancellations arrays, with require_success and require_skipped_or_success as thin wrappers passing skipped_ok false/true. - It claimed the error "names the newest run for the head". It does not; it points the reader at a newer PR required run on the current head SHA and says that if there is none the run was hand-cancelled and must be re-run. Both were carried over from an obsolete sentence in the open row, which described an earlier revision; the row's later text described the shipped two-pass behaviour, and condensing it for the archive kept the stale half and dropped the accurate one. An archive record that misdescribes its own fix defeats the purpose of archiving it. The rewritten record also fixes an inherited count: the row said "seven cases" in tests/ci-cache-safety.test.ts, but ten of that file's thirteen tests execute the extracted aggregate (lines 95-200); the first three cover caching and Playwright deps. It additionally records the failures-win refinement from PR #1409, which the original condensation omitted entirely. Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)"; "Tests 4562 passed | 4 skipped (4566)"; "Outstanding-issues guard passed: 125 rows (56 open, 69 archived), unique ids, next-id=128 above the highest". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
Third main sync on this branch. The new `PR mergeability` check on main caught the drift, which is #116's fix working as designed. The replay script now carries the review-corrected #95 text. Re-running it against a fresh main would otherwise have reintroduced the exact `cancelled_error` description Codex flagged, since the script held the pre-review wording. main's .gitattributes now sets `merge=union` for this file and SKILL.md adds: "Never resolve a conflict by taking one side wholesale — that drops the other agent's rows." Proven not to happen here, by ID-set comparison against origin/main rather than by assertion: origin/main : 60 open, 65 archived, 125 total resolved : 55 open, 70 archived, 125 total IDs lost from main: NONE IDs added: NONE main's open rows preserved except the 5 moved: True main's archived rows all still archived: True So the result is a true union: main's 60 open minus the five archived equals 55, and main's 65 archived plus those five equals 70. Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)"; "Tests 4563 passed | 4 skipped (4567)"; "Outstanding-issues guard passed: 125 rows (55 open, 70 archived), unique ids, next-id=128 above the highest, union merge active". npx prettier --check . -> "All matched files use Prettier code style!". Zero conflict markers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
GitHub reported this PR dirty, but `git merge-tree --write-tree origin/main HEAD` exited 0: clean. That is the anti-churn case, not a content fight — GitHub's server-side merge does not read custom merge drivers, which is already recorded as ledger #127. So the documented remedy applied: merge locally and push. `git merge origin/main` then produced a CORRUPT tree and it must not be pushed as-is. `.gitattributes` sets `merge=union` for this file (added by #1418), and union keeps both sides of every differing line. Prettier pads these tables to the widest cell, so a single row's edit re-pads every other row — both sides' copies of all 128 rows are textually distinct, and union kept both. Result: every row duplicated, both table headers duplicated, 133 insertions. `npm run check:outstanding-issues` caught it, failing with 128 duplicate-id errors, both separator rows reported as non-canonical ids, and all five archived rows "in BOTH the open and archive tables". The gate #112 added is what stopped a silently doubled ledger from reaching the PR — it failed closed exactly as designed. This is the outcome ledger #127 predicted: it records that `merge=union` is the wrong fix for this file, with evidence, "so nobody re-proposes it". #1418 enabled it anyway. Union is safe for append-only single-line records like docs/branch-review-ledger.md; it is actively destructive on a width-padded table, because padding turns every unrelated row into a conflicting line. Recovered without a destructive reset: the merge commit and both parents are kept, only its tree was rebuilt — take origin/main's ledger, re-apply the five-row archive move, reformat. Proven lossless against origin/main rather than assumed: origin/main : 63 open + 65 archived = 128, 0 duplicates rebuilt : 58 open + 70 archived = 128, 0 duplicates IDs lost from main: NONE IDs invented: NONE open == main's open minus the five moved: True Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)"; "Tests 4563 passed | 4 skipped (4567)"; "Outstanding-issues guard passed: 128 rows (58 open, 70 archived), unique ids, next-id=131 above the highest, union merge active". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/outstanding-issues.md`:
- Line 172: Update the `#109` row in docs/outstanding-issues.md so the final
Resolved-column date matches the stated resolution date of 2026-07-29; only
retain 2026-07-30 if the table explicitly distinguishes it as a separate archive
date.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8736614b-886c-4c72-9385-599af23fbb1c
📒 Files selected for processing (2)
docs/branch-review-ledger.mddocs/outstanding-issues.md
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
My push of dd3a1b6 reported "[new branch]", which was the warning sign. The remote ref was absent at push time, so git created it instead of rejecting a non-fast-forward, and the branch tip moved from 1831cd2 back to a commit built on a14d9e7 — silently dropping three commits authored from a codex/review-pr1428 session between 09:57 and 10:07: 8d2710f Merge remote-tracking branch 'origin/main' into codex/review-pr1428 6157021 docs: record PR 1428 review 1831cd2 docs: align issue 109 resolution date They were recoverable: the objects still fetch by SHA and refs/pull/1428/head still pointed at 1831cd2, so nothing was lost permanently. Saved as recovered/pr1428-head and merged back here. Both of their real changes are kept: - 6157021 appends one record to docs/branch-review-ledger.md, which is append-only — losing it would have meant the PR #1428 review was never recorded and would be run again. - 1831cd2 corrects the Resolved date on the #109 archive row I wrote, from 2026-07-30 to 2026-07-29, aligning the column with the "RESOLVED 2026-07-29" in its own Outcome text. Their version is kept; mine was inconsistent. Resolved by taking their ledger wholesale — it already contained a14d9e7, so it carries every earlier commit of mine — and re-applying only the #122 move on top. Proven against their head rather than assumed: their head : 61 open + 71 archived = 132 merged : 60 open + 72 archived = 132 IDs lost from their head: NONE IDs invented: NONE duplicates: 0 #122 archived: True the five earlier archives still archived: True their #109 date fix kept: True The #122 replay script now refuses when #122 is already archived, rather than appending a second row, so a later merge cannot duplicate it. Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)"; "Tests 4563 passed | 4 skipped (4567)"; "Outstanding-issues guard passed: 132 rows (60 open, 72 archived), unique ids, next-id=135 above the highest, union merge active". npm run check:branch-review-ledger -> "113 live table records + 1206 archived ... no conflict markers, mojibake, heading records, or duplicates". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
…#1440) * docs(issues): re-apply #102's canary-gated correction lost to a merge Codex raised this as a P2 on PR #1423: with the runbook corrected, ledger #102 still told an operator the `(status,id)` use was ordering-safe, so the two operator entry points contradicted each other. A reply on that thread claimed the fix landed in 2bdc245. It did not reach `main`. Verified by content rather than by SHA, which matters here: #1423 was squash-merged, so `git merge-base --is-ancestor` returns NO for every original commit and proves nothing either way. Reading `docs/outstanding-issues.md` at `origin/main` (0310c6a) shows the retracted sentence still present, while the runbook correction and the test change from the same PR both landed. Most likely cause, and it is a documented hazard: #1428 ("archive five completed ledger rows") touched this same file on `main`, and #112 records that `docs/outstanding-issues.md` conflicts present as ordinary content conflicts a hurried resolution can settle by dropping one side. Union merge has since been added to this file, but it would not have saved this case — both sides edited the same sentence. The row now mirrors the runbook's classification and names the runbook as the authority if the two ever diverge again, so a future reader has a tie-breaker rather than two equal-looking claims. Gates: verify:cheap exit 0, Test Files 434 passed (434), Tests 4563 passed | 4 skipped. Outstanding-issues guard, branch-review-ledger guard, docs:check-links, docs:check-scripts, prettier all pass. No SQL applied; no index created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF * docs(issues): correct #102 within the existing table width Addresses the Codex P2 on this PR and fixes my own mistake in the first attempt. The correction itself is unchanged in substance: `documents_status_id_idx` is canary-gated, not ordering-safe, because `rag-candidate-sources.ts:482` pairs `.eq("status","indexed")` with an unordered `.limit(12)` and `(status, id)` serves that equality. The reasoning stays in the runbook, which is the authority; the ledger row now just points at it. What changed is the shape. My first version added prose to #102's Detail cell, which is the widest cell in that column — so Prettier repadded every row and the diff became 63 insertions / 63 deletions. Row #133 records that this exact whole-table behaviour has already caused mechanical conflicts that silently stopped all CI, and it is very likely how the original correction was lost between #1423 and main in the first place. Three attempts established the rule empirically, since my first two hypotheses were both wrong: growing the cell reflows (68 lines), *shrinking* it also reflows (68 lines, because the column width tracks this cell), and trailing whitespace padding does not help because Prettier trims it. The replacement is therefore exactly the same length as the text it replaces — 97 characters — and the diff is now 1 insertion / 1 deletion with Prettier reporting the file unchanged. Verified on an untouched row: before this, #100's Detail cell lost exactly one padding space (3533 -> 3532 chars), which is what proved the column width, not my cell's content, was driving the churn. Gates: outstanding-issues guard 137 rows / unique ids; branch-review-ledger 117 live + 1206 archived, no duplicates; prettier clean. No SQL applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF * docs: record issue correction review --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
docs/outstanding-issues.mdand archived the five whose work is finished and whose next action is empty. Each moves from Open items to Resolved / archive carrying its fix evidence and the2026-07-30date.#095— thepr-requiredaggregate now routes acancelledresult through a sharedcancelled_errorhelper instead of reporting it as a plain failure. Guarded by seven cases intests/ci-cache-safety.test.tsthat execute the extracted aggregate script, three of them mutation-proven against the pre-fix version. The red is deliberately retained, because GitHub counts a skipped required check as passing.#096— every PR Search results band: truthful failure state, approved design, and adoption gate #1316 sub-item is dispositioned: the band adoption gate's root-path gap closed on PR test(search): close the band adoption gate's route-coverage holes #1394, four findings were fixed independently, and the Therapy Compass retry-waiter finding was corrected to not-a-live-defect.#104— a correction row that carried no next action. The worker's triple image read is an accepted peak-memory trade-off documented atworker/main.ts:866-869, not debt. Archived so a fourth audit does not re-file it.#109— the branch sweep refuses outright on a shallow clone, an indeterminate result is refused as its own failure, and the guard moved into the exportedcollectReconciliationStateso the evidence-pack path fails closed too.#115— the band adoption gate was redesigned to walk a real reachability graph rather than asking whether a file mentions the band..claude/skills/issues/SKILL.md:44"Archive, never delete", and the file's own conventions: "rows are archived, not deleted, so the history stays auditable"). Row total is unchanged at 120 — 58 → 53 open, 62 → 67 archived.Prettier widened the archive
Outcomecolumn to fit the new evidence, which repads the other archive rows.git diff --ignore-all-spaceis 7 insertions / 7 deletions: the five moved rows plus both table separators. The alternative was trimming genuine fix evidence to fit the old column width, which would have defeated the point of archiving it.No IDs were allocated, so the
issues:next-id=126marker is untouched.Verification
npm run verify:cheap—EXIT=0;Gate-manifest OK: all 28 verify:cheap gates are enforced in CI (static-pr + mapped jobs);Test Files 431 passed (431);Tests 4493 passed | 4 skipped (4497). Includes this file's own structural gate:Outstanding-issues guard passed: 120 rows (53 open, 67 archived), unique ids, next-id=126 above the highest.npx prettier --check .—All matched files use Prettier code style!(repository-wide, not per-file).node scripts/check-outstanding-issues.mjs --self-test—outstanding-issues self-test passed.Verification not run: npm run verify:pr-local— beyondverify:cheapit adds format (run separately above, repo-wide), a conditional production build / client-bundle scan, and RAG fixture validation. This diff is one markdown file with no source, config, test, or fixture change, so none of those selectors apply.UI verification not run:no UI, routing, styling, reduced-motion, or forced-colors behaviour changed.While reading the log I confirmed the
check:function-grants: FAILlines are that gate's own negative self-test fixtures (/tmp/fn-grants-*/leaky.sql,reopened.sql,no-blanket.sql— deliberately-bad inputs it asserts do fail). The gate's real verdict ischeck:function-grants: OK — all 33 SECURITY DEFINER public function(s) are revoked from PUBLIC.Risk and rollout
scripts/pr-policy.mjsclassifyPullRequestFiles(['docs/outstanding-issues.md'])returnsclinicalRisk: false, operationalRisk: false, ragRanking: false, ui: false.git revertthis single commit. The five rows return to Open items unchanged, since the archive move preserves each row's text.Clinical Governance Preflight
Not applicable — this change touches none of ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output. Confirmed against
classifyPullRequestFiles, which reportsclinicalRisk: false. Left unchecked deliberately rather than ticked as vacuously true.Notes
Two findings from the triage that are not addressed here, recorded so they are not lost:
#120,#122,#094,#095and#097are one theme in five rows: a check that passes, or fails, without testing what it claims to.#120(verify:phone-chromeexits 0 while its own output ends13 failed, all 13 failing at browser launch) and#122(ci/circleci: verifyred on every branch including a docs-only diff) are still open, and both should be fixed before#055or#105bank browser or release proof.#120and#122. Adding them was scoped out of this PR to keep it a pure archive move.Generated by Claude Code
Summary by CodeRabbit