refactor(rag): extract evidence coverage gate - #1454
Conversation
Move the evidence coverage gate out of the rag.ts monolith into a dedicated module. `evaluateEvidenceCoverageGate` and the `applyCoverageGateTelemetry` helper it owns move byte-for-byte; only the `export` keyword was added where a declaration now crosses the module boundary. The shared `visualEvidenceUnitTypes` taxonomy moves to `rag-evidence-gates.ts` because rag.ts's second-stage rerank still reads it — that keeps the new module free of any back-edge to rag.ts. `rag.ts` re-exports `evaluateEvidenceCoverageGate` so the public `@/lib/rag/rag` API is unchanged. `prepareCoverageGateResults` deliberately stays in rag.ts: it is pipeline orchestration calling the metadata/visual hydration and second-stage rerank helpers, so moving it would require a runtime back-edge. The hydration cluster is a separate later extraction. rag.ts 5030 -> 4780 lines; maintainability budget ratcheted 5030 -> 4780. RAG impact: no retrieval behaviour change — pure module extraction 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:29 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 (6)
Comment |
BigSimmo
commented
Jul 30, 2026
Zero jobs failed. The aggregate's own output: The Update-branch merge ( Deliberately not pushing anything. A commit now would cancel that in-flight run the same way and race the armed auto-merge, so the fix for this red check is to let the current run finish. I re-verified the extraction survived the If the run on Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
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
src/lib/rag/rag.tsmonolith into a newsrc/lib/rag/rag-coverage-gate.ts.evaluateEvidenceCoverageGate(exported) and theapplyCoverageGateTelemetryhelper it owns move byte-for-byte; the only edit to either body is theexportkeyword added toapplyCoverageGateTelemetry, which now crosses a module boundary.git diff --color-moved=zebraclassifies 250 of the 275 added lines as moved, and the only three non-moved removals are the two declaration lines that gainedexportplus one blank line.(query, results, queryClass)that returns an accept/reject verdict, a reason string, and the source-image flags, andapplyCoverageGateTelemetrywrites exactly those fields onto the telemetry record. Nothing else reads or writes them. Every predicate the gate calls already lives in a stable sibling (rag-evidence-gates.ts,clinical-search.ts), so the module needs no back-edge torag.ts.visualEvidenceUnitTypestaxonomy moved torag-evidence-gates.tsrather than into the new module, becauserag.ts's second-stage rerank (shouldUseSecondStageRerank,secondStageScore) still reads it. Keeping it inrag.tswould have forced a runtime back-edge; putting it in the coverage-gate module would have made ranking code depend on the gate.rag-evidence-gates.tsalready owns the neighbouring evidence predicates, so the constant is byte-identical there and both consumers import it.rag.tsexplicitly re-exportsevaluateEvidenceCoverageGate(export { evaluateEvidenceCoverageGate } from "@/lib/rag/rag-coverage-gate"). This was chosen deliberately to preserve the existing public@/lib/rag/ragAPI and avoid consumer churn —tests/retrieval-query-variants.test.tskeeps its current import unchanged.npm run check:knipis clean with the re-export in place, and no production consumer imports the symbol.prepareCoverageGateResultsstays inrag.ts. Despite its name it is pipeline orchestration — it callsmeasureSearchPhase,attachDocumentRankingMetadata,selectRankedRetrievalResults,attachPageVisualEvidence, andapplySecondStageRerankIfNeeded, five runtime symbols that exist only insiderag.ts, plus the module-privateSearchTimingandDocumentRankingMetadataCachetypes. Moving it would require either a runtime back-edge torag.tsor dragging the hydration cluster along, and a signature change to inject those helpers was not acceptable. Its body is verified byte-identical and untouched in place.src/lib/rag/rag.tsfrom 5030 down to the new measured 4780, preserving the existing "keep the reclaimed budget" intent so it cannot silently drift back.Files changed
src/lib/rag/rag-coverage-gate.tsevaluateEvidenceCoverageGate+applyCoverageGateTelemetry(+261)src/lib/rag/rag.tssrc/lib/rag/rag-evidence-gates.tsvisualEvidenceUnitTypestaxonomy (+11)scripts/check-maintainability-budgets.mjsrag.tsbudget 5030 → 4780 (+3 / −1)docs/maturity-backlog-workorders.mddocs/codebase-index.mdLine counts and movement evidence
src/lib/rag/rag.ts: 5030 → 4780 lines (−250), measured withsourceLineCountfromscripts/check-maintainability-budgets.mjs.src/lib/rag/rag.ts: 5030/5030; it now reportssrc/lib/rag/rag.ts: 4780/4780.git diff --stat:6 files changed, 299 insertions(+), 264 deletions(-).git diff --numstat:261/0new module,11/0evidence-gates,3/253rag.ts,3/1budget script,10/0+11/10docs.src/lib/rag/**implementation LOC: 18119 → 18141 (+22) — the new module's 24-line import block and 4-line header comment, less the 8 import lines pruned fromrag.ts. This is movement, not duplication: each moved function is defined exactly once repo-wide, and no pipeline body was copied.origin/mainby extracting the original byte ranges (rag.ts:1864-2077,2119-2134,514-523) and diffing them against the new locations: all three blocks are byte-identical modulo the addedexportkeyword.prepareCoverageGateResults(rag.ts:2079-2117on main) is likewise byte-identical in place.Verification
npm run verify:pr-localDuring 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: no UI, routing, styling, browser, reduced-motion, or forced-colors surface is touched by this change — the diff is confined to two
src/lib/ragmodules, a budget script, and two docs.Verification not run (provider-backed, deliberately withheld):
npm run eval:retrieval:quality,npm run eval:rag,npm run eval:quality --rag-only,npm run check:production-readiness, andnpm run check:deployment-readinessall call live OpenAI/Supabase. They were prohibited for this task and were not run. No live evaluation and no RAG canary was run, because this change is a zero-behavior-change structural extraction: the moved function bodies are byte-identical, no comparator, threshold, default, fallback, ordering rule, orawaitchanged, and the byte-range diff againstorigin/mainis the evidence. A reviewer who disputes that classification can refute it cheaply by re-running the byte-range comparison described above; if it is disputed, a live canary pair should be required before merge.Gates run
npm run workflow:rag-lab -- --write-evidenceRisk classes: retrieval, clinical— planner selected the offline checks; approval-gated live evals left unrunnpm run test -- tests/retrieval-query-variants.test.ts tests/rag-tail-latency.test.ts tests/rag-shared-cache.test.ts tests/rag-variant-early-exit.test.tsTest Files 4 passed (4)/Tests 81 passed (81)npm run check:maintainability-budgets[maintainability] src/lib/rag/rag.ts: 4780/4780 lines/Maintainability hotspot budgets passed.npm run check:rag:fixturesOffline RAG fixture and manifest validation passed (36 golden cases, 21 suites).npm run eval:rag:offlineTest Files 21 passed (21)/Tests 567 passed (567)/Offline RAG fixture and production-contract checks passed.npm run typechecktsc --noEmitcompleted with no diagnosticsnpm run lint--max-warnings 0completed with no outputnpm run check:knip.cssconfiguration hint)npm run format:checkAll matched files use Prettier code style!npm run verify:cheapTest Files 435 passed (435)/Tests 4569 passed | 4 skipped (4573)npm run test(full local unit suite)Test Files 435 passed (435)/Tests 4569 passed | 4 skipped (4573)npm run verify:pr-local -- --dry-run --files …npm run verify:pr-local✓ Compiled successfully in 56s,Client bundle secret surface check passed.,Offline RAG fixture and manifest validation passed (36 golden cases, 21 suites).git diff --checkEach provider-safety decision was made by reading the package script and the scripts it invokes first:
eval:rag:offlineresolves tocheck-rag-fixtures.mjs+test-rag-offline.mjs, both of which read local JSON fixtures and spawn Vitest with nofetch, no HTTP, and no OpenAI/Supabase/Railway reference;npm run buildonly reads a Supabase hostname string for thenext/imageallowlist and makes no network call.Risk and rollout
src/lib/rag/**), so the surface is high-consequence even though the diff is a mechanical move. The residual risk is a mis-transcribed body; that is directly refuted by the byte-range comparison againstorigin/mainand by 4569 green unit tests including the full 36-case offline golden fixture set.git revertof this commit restoresrag.tsand the 5030 budget exactly; there is no data migration, no cache-format change, and no persisted state involved.origin/main, push this feature branch, and open this pull request.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)RAG impact: no retrieval behaviour change — pure module extraction
Notes
rag.tsstill ownsprepareCoverageGateResults, so the gate's input preparation and the gate itself now live in different modules. That is the correct seam today — the preparation step is hydration/rerank orchestration, not gate logic — but it means a future reader must follow one import to see the full call sequence.rag-coverage-gate.tshas no back-edge torag.ts; its only dependencies are@/lib/types,@/lib/clinical-search,@/lib/rag/rag-contracts(type-only), and@/lib/rag/rag-evidence-gates.src/lib/rag/rag-hydration.ts(historically aroundrag.ts:1505-1736— the document-ranking-metadata cache,attachDocumentRankingMetadata,withCachedIndexQuality,attachIndexQualityMetadata, andattachPageVisualEvidence); it was deliberately excluded from this PR, and it is the extraction that would letprepareCoverageGateResultsmove cleanly afterwards. docs: record phase 6 re-audit #100 is also out of scope.Generated by Claude Code