test(search): pin search retrieval-core round-trip budgets (#098), and retract a wrong #121 claim

[BigSimmo]

Summary

• Budget the search retrieval core (#098). tests/search-round-trip-budget.test.ts pins Supabase round-trip counts for searchChunksWithTelemetry — the function src/app/api/search/route.ts calls to retrieve — registered in both scripts/fixtures/rag-offline-contract-tests.json and scripts/rag-offline-contract.mjs so it runs inside the offline contract.

  Scope, corrected after review — this is not a budget for the /api/search endpoint. Earlier versions of this PR, of ledger #098, and of the contract-runner comment all claimed it was. It is not: the route's authentication, rate limiting, scope resolution, related-document enrichment and telemetry write are invisible to this suite, so a round trip added to any of them leaves it green. The refusal scenario likewise says retrieval spends nothing on an adversarial query — an adversarial HTTP request still pays the route preamble. The file header, describe, both test names, the refusal assertion message, the #098 row and the contract-runner comment all now say retrieval core and name the exclusions. Driving POST with counted clients is filed as #098's next step, naming tests/answer-route-preamble.test.ts as the pattern to copy.

  The measured shape is itself a finding. One retrieval costs 11 round trips: rag_aliases 1, match_document_chunks_text_v2 3, match_document_table_facts_text_v2 3, get_related_document_metadata_v2 1, document_index_quality 1, document_images 2 — both text RPCs firing three times. Pinned by total and breakdown, since a refactor swapping one probe for an unrelated query would hold the total at 11 while changing the traffic. Whether 3+3 is intended is not established here — a retrieval question needing the RAG gate, carried on #098 as an open decision rather than a guess.

• Retract a claim I published earlier today on #121. An earlier version of this PR recorded that a sandboxed remote session has only two options for the Chromium 1194-vs-1234 mismatch — obtain /opt write permission, or decline to claim browser evidence. That was wrong, an over-generalisation from a single blocked mkdir. PR fix: Playwright browser preflight prevents #120 false greens #1432's preflight names the route I missed: PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH, read by playwright.config.ts:11, needing no filesystem write. Verified by launching rather than by reading the flag — the container's existing 1194 headless_shell drives fine under the repo's Playwright 1.62 client (version 141.0.7390.37, page rendered, boundingBox measured). The wrong sentence is kept on the row with the retraction beside it, because which claim failed and why is the part worth carrying forward.

• Mark #130's recorded blocker LIFTED. The pre-paint/cold-load guard was recorded as unbuildable in a remote session; on the corrected facts it is buildable. Also records PR fix: Playwright browser preflight prevents #120 false greens #1432 itself, which no ledger row referenced — assertPlaywrightBrowsersReady now runs inside scripts/run-playwright.mjs, so a missing binary exits 1 with one explicit message instead of surfacing as N tests "failing" at launch, the misdiagnosis that produced #120.

Verification

• npm run verify:cheap on the final tip — exit 0, Test Files 443 passed (443), Tests 4627 passed | 4 skipped (4631).
• The new suite was confirmed inside that run rather than assumed: the node project's include is tests/**/*.test.ts and vitest list reports the file with both tests. "443 passed" alone does not prove a specific file ran.
• npm run check:rag:fixtures — Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).
• npm run check:outstanding-issues — 146 rows (57 open, 89 archived), unique ids, next-id=149 above the highest.
• npm run format:check whole tree, npm run typecheck, eslint on touched files — clean. Prettier reformatted the new test after its first commit, so that commit was amended rather than pushed unformatted; CI checks the pushed blob, not the working tree.
• Budgets are deterministic — three consecutive runs, Tests 2 passed (2) each, before any number was pinned.
• The refusal guard was proven against the broken shape. With a non-refused query it fails on the round-trip assertion (expected 11 to be +0). A first attempt failed on the results assertion instead, which would have proved only that the test catches the regression, not that the budget can fail — so the counter assertion is deliberately ordered first.
• A harness defect was caught rather than shipped: the in-test control initially spent 0 trips because the module registry was cached between runs, so the second call used the first mock. vi.resetModules() now runs inside the harness, with the reason commented at the call site.
• Review findings fixed and re-verified together: Test Files 3 passed (3), Tests 9 passed (9) across this suite, the answer-path budget, and tests/rag-variant-early-exit.test.ts — the neighbouring mock-sensitive suite, run because the vi.doUnmock change concerns exactly that risk.

UI verification not run: no UI, routing, styling, or motion content. Browser evidence is now possible here via PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH — that is what this PR retracts — but nothing in this diff requires it.

Verification not run: every provider-backed gate (eval:*, verify:release, check:supabase-project, test:live) is unimplicated and unauthorized; no OpenAI, Supabase, or hosted-CI call was made. The suite is provider-free and DB-free, counting traffic through a mocked @/lib/supabase/admin client.

Risk and rollout

• Risk: Low. The only executable additions are a test and its contract registration. No src/lib/rag/** file is touched — the search surface is observed, not modified.
• The real risk in a budget guard is a number that looks authoritative but was never exercised, which is why non-vacuity assertions run first, numbers were measured across repeated runs, and the guard was proven against the broken shape. The second risk is a guard cited for more than it covers — raised in review, now fixed in the test, the contract runner, the ledger and this body.
• Known blind spot, documented in the file: it sees only traffic through the wrapped client. A trip via another client instance, a direct fetch, or a provider SDK is invisible.
• Rollback: git revert the relevant commit. Reverting the test removes a gate; it changes no product behaviour.
• Provider or production effects: None.

RAG impact: no retrieval behaviour change — this PR adds a test that observes searchChunksWithTelemetry through a mocked Supabase client, plus its registration in the offline contract list. No retrieval, ranking, selection, alias, scoring or citation code is modified, no index is applied, and no fixture case or comparator ordering is changed. The recorded 3+3 RPC observation is a measurement of existing behaviour, left as an open question on #098 rather than acted on.

Clinical Governance Preflight

• Source-backed claims still require linked source verification before clinical use
• No patient-identifiable document workflow was introduced or expanded without explicit governance approval
• Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
• Service-role keys and private document access remain server-only
• Demo/synthetic content remains clearly separated from real clinical sources
• Source metadata, review status, and outdated/unknown-source behavior remain conservative
• Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Notes on the above, stated plainly rather than implying an inspection that did not happen: every item is satisfied by construction, because this PR adds a test and ledger prose and modifies no product code. scripts/pr-policy.mjs classifies the diff clinicalRisk: false, so this section is not required by the repo's own gate; it is included because the change sits adjacent to retrieval and an automated description check asked for it. No answer generation, citation, verification, source-governance or document-access code is touched. The only Supabase client involved is a mock inside the test; no credential, key or project value appears in the diff, and the pinned project ref is referenced only as unchanged context. Clinical decision-support behaviour is unaffected, so the SaMD classification is unchanged. The one clinically-relevant effect is indirect and protective: an added round trip on the search retrieval path now fails a gate instead of going unnoticed.

Notes

• Two review findings were fixed in c24116a1, both verified against the code before being accepted. The contract-runner comment was a third copy of the /api/search overstatement I had missed. The vi.doUnmock cleanup is the established pattern here, not a generic suggestion — five sibling suites already do it, and the added comment cites tests/rag-variant-early-exit.test.ts:110.
• The branch carries several main syncs plus a merge of a concurrent push by another actor. Four produced real conflicts in docs/outstanding-issues.md, each resolved by rebuilding the edits on main's table and diffing the row-id sets against main to prove nothing was lost — never by taking one side wholesale. Nothing was force-pushed. main's own Stop Prettier padding the issues ledger table, closing #133 #1479 has since stopped Prettier padding that table, which should make such conflicts rarer.

🤖 Generated with Claude Code

https://claude.ai/code/session_018K7sEKH35KZkWxvCnQcNN2

[supabase]

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 17 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 74f973f2-6610-430c-aa34-7e5d5a3ae523

📥 Commits

Reviewing files that changed from the base of the PR and between 1590aff and affacf8.

📒 Files selected for processing (3)

• docs/outstanding-issues.md
• scripts/rag-offline-contract.mjs
• tests/search-round-trip-budget.test.ts

📝 Walkthrough

Walkthrough

Adds an offline search round-trip budget suite, registers it as a required contract test, and updates the outstanding-issues ledger with revised evidence, gate details, and deletion-detection requirements.

Changes

Offline search contract

Layer / File(s)	Summary
Search round-trip harness and assertions tests/search-round-trip-budget.test.ts	Mocks Supabase retrieval, counts calls through searchChunksWithTelemetry, verifies 11 lexical-query round trips with an exact breakdown, and confirms adversarial queries perform zero retrieval calls.
Required offline contract registration scripts/rag-offline-contract.mjs, scripts/fixtures/rag-offline-contract-tests.json	Adds the search budget suite to the required offline contract tests and fixture list under Ledger #098.
Outstanding-issues evidence and governance updates docs/outstanding-issues.md	Revises round-trip, browser, device, baseline, ledger-governance, and validation notes, including detection of deleted ledger rows.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• BigSimmo/Database#666: Changes the adversarial early-exit behavior and retrieval path tested by this round-trip suite.
• BigSimmo/Database#705: Introduces the offline RAG fixture-checking logic that consumes the required contract test list.
• BigSimmo/Database#1410: Updates the same outstanding-issues validation and CI gating mechanism.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the main changes: search round-trip budgets and a claim retraction.
Description check	✅ Passed	The description matches the required template with Summary, Verification, Risk and rollout, Clinical Governance Preflight, and Notes sections.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66f4872d5a

ℹ️ 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".

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@scripts/rag-offline-contract.mjs`:
- Around line 26-28: Update the comment near
tests/search-round-trip-budget.test.ts to describe the guard as covering the
retrieval core, specifically searchChunksWithTelemetry, rather than /api/search.
State that it verifies adversarial queries avoid Supabase queries, without
implying coverage of endpoint authentication, rate limiting, scope resolution,
enrichment, or telemetry writes.

In `@tests/search-round-trip-budget.test.ts`:
- Around line 139-143: Update the afterEach cleanup in
search-round-trip-budget.test.ts to explicitly remove the vi.doMock
registrations for "`@/lib/supabase/admin`" and "`@/lib/openai`" using vi.doUnmock,
alongside the existing restoreAllMocks, resetModules, and unstubAllEnvs calls.

🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

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: 410f9849-dae3-4307-a258-bc68217f73f2

📥 Commits

Reviewing files that changed from the base of the PR and between 00ddb22 and 1590aff.

📒 Files selected for processing (4)

• docs/outstanding-issues.md
• scripts/fixtures/rag-offline-contract-tests.json
• scripts/rag-offline-contract.mjs
• tests/search-round-trip-budget.test.ts

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found.