docs: record the landed X3 coverage-gate review and capture its two follow-ups - #1461
Conversation
Appended with npm run ledger:append (never hand-written). The row was held back during PR #1454 because a ledger-only tip would have cancelled the in-flight CI run and marked every other open PR behind; now that the PR has squash-merged as 102bb1f, recording it is harmless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS
Two follow-ups surfaced by the #86 coverage-gate extraction (PR #1454): - #86 updated in place rather than duplicated: records that the coverage gate shipped, and names rag-hydration.ts as the next X3 unit because it re-homes the five rag.ts-only symbols that forced prepareCoverageGateResults to stay behind. - #145 (new): an operator branch sync landing on a head whose CI is still running cancels a near-complete run. Observed three times in ~20 minutes on PR #1454. The existing anti-churn rule covers only your own pushes, not syncs, which is the case that recurs. Distinct from #95, #129 and #116. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS
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:4 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 (2)
Comment |
Resolves the docs/outstanding-issues.md conflict with PR #1453. That file deliberately carries no merge driver (#133), so overlapping appends conflict loudly rather than being silently concatenated. Resolved as the issues skill requires: rebuilt the file from origin/main and re-applied only the two rows this branch owns (the #86 in-place update and the new #145), so none of #1453's rows were dropped. Verified #140-#144 all still present and #144's cell content byte-identical to main. Re-checked that #145 was still free on main before reusing the id — main's next-id marker was untouched at 145, so there was no id collision to reallocate around. docs/branch-review-ledger.md auto-resolved through its merge=ledger driver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:2d47dcb6df
ℹ️ 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.
Codex was right on PR #1461. The previous wording claimed extracting the hydration cluster re-homes all five rag.ts-only symbols that prepareCoverageGateResults depends on, so that it could then move cleanly. It re-homes only two. Verified by symbol location on 102bb1f: the hydration cluster is rag.ts:1487-1718 and holds attachDocumentRankingMetadata and attachPageVisualEvidence. The other three are outside it — selectRankedRetrievalResults (1825), applySecondStageRerankIfNeeded (679), and measureSearchPhase (1975), the last being a shared pipeline timing wrapper with 21 references of which only two are hydration phases. The row now separates the actual hydration symbols from the remaining orchestration/ranking seam and states that hydration alone is not sufficient, so a future contributor is not sent at a false module boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS
Uh oh!
There was an error while loading. Please reload this page.
Fourth id collision on this one row. `main` (#1461) claimed `#145` for "A branch sync during in-flight CI cancels a near-complete run, and nothing warns" — which is, as it happens, the same CI-cancellation behaviour this branch hit an hour ago. Resolved by taking main's `docs/outstanding-issues.md` wholesale and re-applying this capture at `#146`, rather than hand-editing the conflict region: hand-editing a 144-row table around a conflict marker is how the duplicate and wrong-width rows got in before. Marker bumped to 147.
Main's #1461 reflowed the open-items table while this branch appended #17's live Web-Vitals verdict, so the two sides conflicted for real (`git merge-tree` dirty, not staleness). Resolved row by row rather than by taking either side: kept this branch's #17 BREACH verdict and #105 preconnect evidence, took main's #86 X3 progress and its new #145 row. #105 closes. Its remaining half — the `LoadingPanel` fallbacks — is now verified, and by a different method than the row prescribed. It claimed the fallback renders solely while the client chunk is in flight, so only a throttled-network run could observe it. That is wrong: the installed Next 16 loader wraps an `ssr:false` import in Suspense whenever a `loading` element is supplied, and `BailoutToCSR` throws on the server, so the fallback is emitted in the server response HTML. Confirmed against the running dev server — `role="status" aria-label="Loading"` appears 1x on `/`, 2x on `/dsm`, 2x on `/forms`. No throttled run and no `verify:ui` were needed. Finding raised by Codex on PR #1459. The two recommended-queue entries now match their detail rows: #105 is dropped from the queue, and #17 no longer directs a reader to capture evidence that has already been captured and graded — its next action is ranking the mobile findings by measured contribution and cross-checking the raw Lighthouse JSON artifact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Fifth docs/outstanding-issues.md conflict. Applying the default announced to the user after the fourth: drop the #86 "Hydration SHIPPED (#101)" row from this PR rather than keep re-resolving it. That file has no merge driver by design (#133) and main lands issue-ledger commits continuously, so every sync collided on it — five conflicts, each costing a full CI cycle, for one documentation line unrelated to the extraction. This branch now takes origin/main's copy verbatim and no longer modifies the file at all, making the PR immune to that churn. Nothing else changes: rag-hydration.ts, rag.ts at 4543, the budget ratchet, the codebase-index row and the X3 work-order entry all remain. The #86 row will be recorded in a separate follow-up PR after this merges — the same pattern used for #1454 via #1461. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS
* issues: capture the unreadable-CI token, at-risk worktree work, and the unpushed hook fix Three findings from the 2026-07-30 organisation session that were recorded nowhere durable: - #149 the session GitHub PAT lacks Checks: Read, so no agent can confirm a PR is green. The endpoint that does work returns an empty result rather than an error, so it reads like an absence of checks rather than an absence of permission. - #150 four worktrees on already-merged branches hold uncommitted work that exists in no branch and no PR, the largest being +395/-200 across 19 files including CI config. - #151 the pre-commit fail-open for #143 lives only on a never-pushed local branch, which is also 17 behind main and conflicts on the file whose count sentence main's new docs:update generator now owns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the session-followup capture review for PR #1490 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record #143/#151/#149 reconciliation for PR #1490 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): supersede PR #1490 reconciliation after remote sync Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * issues: record the worktree snapshots and redirect #151 to PR #1494#150 — the four at-risk worktrees were snapshotted onto their own already-merged branches (748ef018f, 5dbd9f965, b7eae51a4, d949859c3), so the work survives a worktree reclaim. All four are clean now. None is pushed or reviewed; the next action is per-snapshot promote-or-reset. #151 — the never-pushed branch is superseded rather than salvageable: its script and hook reached main by other routes, so the fail-open guard was applied to main's committed hook in PR #1494 instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: remove credential metadata and correct audit dates * docs: consolidate session follow-up findings * docs: record consolidated follow-up review * issues: record that #101 hydration shipped PR #1463 merged as dba7356, so #86's "Next X3 unit — rag-hydration.ts" is now stale. The row records the extraction as shipped and keeps the corrected boundary: hydration re-homed only two of prepareCoverageGateResults's five rag.ts-only dependencies, so it did not unblock that function — exactly as the Codex review on PR #1461 predicted. This row was deliberately dropped from #1463 itself (commit 6290d02) after docs/outstanding-issues.md conflicted on five consecutive main syncs. Recording it separately here is the same pattern used for #1454 via #1461. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS * docs(ledger): record the landed X3 hydration review Appended with npm run ledger:append (never hand-written), keyed to the squash commit dba7356 so ledger:lookup can resolve it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS * docs: fix the #101 mislabel and key the ledger row to a resolvable ref Both defects were raised by Codex on PR #1495 and both are real; verified against the files before accepting. 1. #101 is NOT this extraction. docs/outstanding-issues.md:138 shows #101 is "Canary-gated retrieval parallelisation candidates" (P3, rec) — a separate, still-open recommendation gated on a live canary pair. Calling the hydration extraction "#101" marked that unrelated work as shipped and could have caused the live-evaluation work to be skipped. The label came from the original task brief and was propagated without checking it against the ledger. Both the #86 row and the X3 work-order entry now identify the change as the X3 hydration unit (PR #1463) instead. #101's own row is untouched and still open. 2. The ledger row did not resolve. `npm run ledger:lookup -- dba7356` returned NOT REVIEWED, because the ref cell held only the slash-form branch token and that branch no longer resolves locally, so the throttling record could not prevent a repeat review. Appended a superseding record keyed to the landed SHA; the same lookup now returns ALREADY REVIEWED. The original row is retained, per the ledger's append-only rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS * docs: record consolidated PR reviews * docs: record ingestion recovery review * docs(visual): document the platform-scoped baseline layout and how to seed it `playwright.visual.config.ts` records snapshots under `__screenshots__/{platform}/`, so a baseline taken on Windows lands in `win32/` and is never consulted by the `ubuntu-24.04` CI job, which reads `linux/`. Nothing said so, and committing `win32/` images looks like protection while providing none. Records the constraint, names the CI artifact as the supported recorder for `linux/` baselines, and notes that comparison stays advisory until the jobs come off `continue-on-error`. Also creates the tracked directory `.gitignore` already claims exists, which sets `ui_changed=true` (`scripts/ci-change-scope.mjs`) so the visual job can run and produce that first artifact. No baselines are added here — they cannot be produced on this platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: correct visual baseline adoption steps * docs: record visual baseline guidance review * fix(ui): repair mockup accent token references * docs: record token-reference repair review * docs: archive advisory UI scoping task * docs: record advisory UI closure review * issues: archive #151 after #1494 and mark #143 fully resolved PR #1494 landed the fail-open guard on main, so close the open salvage row and update the #143 archive from PARTIAL to resolved across #1442 and #1494. Also carries the merge of origin/main that cleared the GitHub DIRTY mergeability state. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1490 main-sync and #151 closeout Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record #1496 id-collision renumber for PR #1490 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * issues: record the withdrawn live-region finding as #151 so it is not re-filed Archive-only row. There is no defect and no work to do — the row exists purely as a guard rail against repeating a misreading that already happened once. search-results-header-band.tsx sets aria-live={faulted ? "off" : "polite"} on its count/status span, which reads like a silenced failure announcement. It is not: the band mounts a separate fault panel with role="alert" carrying the failure title, body and Retry, and the mute is deliberate so the two do not both speak. The reasoning is in a comment directly above the attribute, and tests/search-results-header-band.dom.test.tsx pins it with singular role queries that throw on duplicates. During session 2026-07-30 (PR #1481) this was filed as a real P2 defect on the strength of the attribute alone, and the proposed fix — escalating the count span to role="alert"/aria-live="assertive" — would have produced a duplicate announcement and a red test, making it worse than no change. Codex caught it. An earlier withdrawal row was then lost to the squash that merged #1481, which is the row-deletion shape #148 now guards against. Also records that the mockup's escalation is correct in the mockup and must not be ported: search-refine-adaptive-mockups.tsx has no fault panel, so there the count span is the only announcement channel. #148 needed no work — the merge-base deletion check landed on main independently, and its output now reports the base it compared against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r * docs(rag): record refuted lexical probe collapse (#98) * issues: capture the residual id-allocation hazard as #151#133 is resolved: #1444 removed merge=union and #1479 excluded the ledger from Prettier, which together fixed conflict frequency. Neither changes id allocation, which is still read-modify-write against the next-id marker, so concurrent branches still claim the same number. Measured on PR #1451: one row was renumbered #135 -> #141 -> #145 -> #147 -> #149 across four sync cycles. The sharper finding is that GitHub's Update-branch button resolved one such collision into duplicate #141 rows with the marker left below main's highest id — git reported success and only check:outstanding-issues caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(issues): attribute the mobile CLS breach — a 128px reserve round trip #147 asked which elements shift. Driving Chromium against the same offline production build with a PerformanceObserver on layout-shift (Lighthouse mobile emulation, reading entry.sources[].node) gives one dominant cause on all four breaching routes: the entire main content region moves down 128px and straight back up 128px within 15-60ms. Both moves score, so it is pure cost with zero net movement — 100% of /documents/search's 0.220 and about 75% of /dsm's. The shifting element is the max-sm:pt-[var(--phone-overlay-chrome-h)] wrapper around <main>. A MutationObserver timeline on the root style attribute pins the mechanism rather than inferring it: the property goes CSS seed -> 200px -> 72px, and the 200px is written when the header stack ALREADY measures 72px (t=1552ms reserve=200px stack=72, corrected at t=1612ms). usePhoneOverlayChromeReserve reads stack.offsetHeight while the stack is transiently tall, publishes a value that is stale by the time it lands, and its ResizeObserver then corrects it. The CSS seed at globals.css:375 is correct for the settled stack, which corrects the mechanism recorded on the now-archived #130 — that framed the defect as the seed under-reserving by 0-8px. Measured, the driver is a 128px transient over-reserve written by the hook, not the seed. / is the control: it never writes the property and is the one clean route. Variance is stated rather than smoothed: /dsm measured 0.363 and 0.219 across two runs, and this harness has no network throttling so /forms and /therapy-compass run high locally. Only /dsm, /documents/search and / reproduced the live dispatch exactly. Also recorded: attaching a MutationObserver to document.documentElement inside a Playwright addInitScript throws before the document element exists, silently killing the CLS observer and reporting a uniform CLS=0.000 — a false clean bill that voided one run of this harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF * docs(ledger): record the #151 capture review for PR #1506 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(review): clarify snapshot branch state * docs(ledger): record PR #1490 main sync after snapshot wording Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: archive rendered style contract task * docs: record style contract closure review * docs: record synced style contract review * docs: record post-121 style closure review * docs: normalize style review ledger after sync * docs: record post-1490 style closure review * docs: record consolidated PR 1490 review * docs: record replacement consolidation review * docs: record reconciled consolidation review * docs: record post-1511 consolidation review * docs: normalize PR 1510 ledger after main sync * docs: record PR 1510 post-sync review * docs: correct false #98 canary evidence and NOTES triage Remove the incorrect probe-collapse canary attribution from #98 and point the unread --med-accent-soft note at #157 without breaking the seven-token TOKENS_MISSING accounting. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1510 evidence-correction review Supersede the prior approve-with-no-findings row after correcting the false #98 canary attribution and NOTES triage drift. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: keep concurrency note inside issue table * docs: record post-1513 consolidation review * docs: address CodeRabbit notes on PR #1510 Fix the computed-value-time wording in design-sync notes, give #33 a unique recommended-queue order, and drop the duplicated #98 Done block. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1510 CodeRabbit fix review Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
Two append-only memory records left over from the X3/#86 coverage-gate extraction (PR #1454, squashed as
102bb1f). Bundled into one PR per AGENTS.md § "PR bundling" — both are low-risk docs-only records, each in its own separately revertible commit.docs(ledger): record the landed X3 coverage-gate review. Appended withnpm run ledger:append(never hand-written), keyed to the squash commit102bb1fsoledger:lookupcan resolve it. This row was deliberately held back while PR refactor(rag): extract evidence coverage gate #1454 was open: pushing a ledger-only tip would have cancelled that PR's in-flight CI run and marked every other open PR behind, which AGENTS.md prohibits. Now that refactor(rag): extract evidence coverage gate #1454 has merged, recording it is harmless.issues: capture the two follow-ups the extraction surfaced.#086updated in place, not duplicated — records that the coverage gate shipped, and namesrag-hydration.tsas the next X3 unit because it re-homes the fiverag.ts-only runtime symbols (measureSearchPhase,attachDocumentRankingMetadata,selectRankedRetrievalResults,attachPageVisualEvidence,applySecondStageRerankIfNeeded) that forcedprepareCoverageGateResultsto stay behind. That dependency was the one piece of engineering judgement from refactor(rag): extract evidence coverage gate #1454 not written down anywhere.#145(new) — an operator branch sync landing on a head whose CI is still running cancels a near-complete run. Observed three times in ~20 minutes on PR refactor(rag): extract evidence coverage gate #1454 (d49ac0f,6de2bf5,d9598b7), cancellingBuild+Unit coverageeach time so no run finished and armed auto-merge never fired. The existing anti-churn rule covers only your own pushes, not syncs — which is the case that actually recurs. Explicitly distinguished from#095(cancelled-job reporting, which worked correctly here),#129and#116.No source, test, config, or workflow file is touched.
Verification
npm run verify:cheapDuring development, use
npm run verify:cheapas the faster iteration gate before the final PR-local preflight.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically. Run the offline command directly during iteration before spending a live eval.npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0)npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedUI verification not run: docs-only diff, no UI, routing, styling, browser, reduced-motion, or forced-colors surface touched.
Verification not run (provider-backed):
eval:retrieval:quality,eval:rag,eval:quality --rag-only,check:production-readiness,check:deployment-readiness— all call live OpenAI/Supabase, none is relevant to a docs-only diff, and none was run. No live evaluation, canary, ingestion, deployment, or release operation occurred.Gates run
npm run check:outstanding-issuesOutstanding-issues guard passed: 143 rows (70 open, 73 archived), unique ids, next-id=146 above the highest, no merge driver.npm run check:branch-review-ledgerBranch review ledger guard passed: 124 live table records + 1206 archived …, ledger merge active, six cells each, no conflict markers, mojibake, heading records, or duplicates.npm run format:checkAll matched files use Prettier code style!npm run verify:cheapTest Files 436 passed (436)/Tests 4574 passed | 4 skipped (4578);docs link check passed: 1406 repo path references resolve;docs script-ref check passed: 397 npm-run reference(s) resolve to real scriptsgit diff --checkRisk and rollout
docs/outstanding-issues.mdhaving no merge driver by design (#133), so an overlapping edit conflicts loudly — resolve by rebuilding fromorigin/mainand re-applying only these rows, never by taking one side wholesale.git reverteither commit independently; they share no file.main, push this branch, and open this PR.Notes
docs/branch-review-ledger.mdremains append-only: no existing row was edited or deleted, and the new row was generated byledger:appendrather than hand-written, so it carries a resolvable 40-char HEAD.mainrather than stacking on merged history. The branch was recreated fromorigin/mainat102bb1fand carries only these two commits.#145deliberately does not recommend disablingcancel-in-progress— superseded runs should still be cancelled; the cost is in when a sync lands, not in the cancellation policy.RAG impact: no retrieval behaviour change — docs-only records, no
src/lib/rag/**file touchedGenerated by Claude Code