Skip to content

test(search): pin a route-level Supabase round-trip budget for /api/search - #1950

Merged
BigSimmo merged 6 commits into
mainfrom
codex/search-round-trip-budget
Aug 14, 2026
Merged

test(search): pin a route-level Supabase round-trip budget for /api/search#1950
BigSimmo merged 6 commits into
mainfrom
codex/search-round-trip-budget

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

  • Ledger chore(deps-dev): bump prettier from 3.9.0 to 3.9.1 #98 (residual): the offline round-trip budget harness pinned the answer path and the search retrieval core (searchChunksWithTelemetry, in tests/search-round-trip-budget.test.ts), but not the /api/search route itself — its auth, rate limiting, scope resolution, related-document enrichment, and telemetry write were invisible to any budget test. Add tests/search-route-round-trip-budget.test.ts, driving POST from src/app/api/search/route.ts with a counted Supabase client, following the tests/answer-route-preamble.test.ts pattern of importing the route fresh per scenario with vi.doMock. A round trip added anywhere in the route preamble or post-processing is now a red gate instead of something a reviewer has to notice.
  • Ledger docs: mark main branch-protection debt as resolved #189: pin the route-level budget from "the harness exists" to an explicit measured count (16 round trips) and exact breakdown for one representative anonymous, filter-free, non-cached search, plus a companion test pinning that a rate-limit denial costs exactly the limiter check (1 round trip) and nothing past it. Separately, disposition the three text-RPC probes: intentional, keep as-is. match_document_chunks_text_v2 and match_document_table_facts_text_v2 are each issued up to 3 times per search — src/lib/rag/rag-candidate-sources.ts fans out maxTextRpcQueryVariants (3) lexical query-variant phrasings (primary + up to two siblings) per text surface, to rescue recall when the primary phrasing misses. This already has a cost-reducing early exit: firstVariantPoolIsStrong (src/lib/rag/rag-retrieval-variants.ts, the "PT-02" mechanism) skips the sibling RPCs once the primary variant's pool is deep and precisely anchored, so a real, well-matched corpus query pays less than ×3 in practice — this test's single-chunk fixture never clears that depth bar, so it measures the worst-case fan-out, not the typical cost. Collapsing the fan-out further (e.g. to a single variant) would be a retrieval-behaviour change and needs the RAG-flag + live-canary process this PR is explicitly out of scope for; no such change is made here.

Three commits, each independently revertible: (1) the route-level harness + non-vacuity checks, (2) pinning the exact count/shape + the ×3 disposition, (3) a small PromiseLike<T> generics fix caught by npm run typecheck (the test harness's mocked query builder needed a two-type-parameter then() to satisfy lib.es5's PromiseLike<T> when a caller passes both onfulfilled and onrejected).

Verification

  • npm run test:focused -- --files tests/search-route-round-trip-budget.test.ts — declined, exactly as designed ("Focused test selection is unsafe: test or configuration paths changed"); it fails closed for test-file changes and prints its own follow-up instruction. Ran the full suite instead, per that instruction.
  • npm run verify:pr-local -- --files tests/search-route-round-trip-budget.test.ts — full run (this diff is executable/test scope, so it escalates past the docs/workflow fast path). Decisive output:
    • check:runtime — PASS (Node 24.19.0, npm 11.17.0)
    • check:installed-lock-parity — PASS
    • format:changed — "All matched files use Prettier code style!"
    • lint — clean (0 warnings/errors across src tests scripts worker supabase playwright + configs)
    • typecheck — clean (after the fix above)
    • testTest Files 602 passed (602) / Tests 6511 passed | 4 skipped (6515)
    • check:rag:fixtures — passed
    • check:medication-interactions — "data/medication-interaction-index.json is up to date (523 rows)."
    • One pre-existing, diff-unrelated failure: check:medication-lexicon-report reports docs/medication-interaction-lexicon-review.md as stale. This diff touches exactly one file (tests/search-route-round-trip-budget.test.ts — confirmed via git diff origin/main --name-only); the lexicon doc and its generator were last touched by fix(medications): reach lithium, retire the dead z-drugs term, make lexicon review flags evidence-bearing #1923, well before this branch's point (7a157f4). This is a pre-existing staleness on main, not something this PR introduced.
  • npm run verify:ui — not applicable, no UI/routing/styling/browser change.
  • npm run verify:release — not run; not a release/handoff-confidence request.
  • npm run eval:retrieval:quality — not applicable; no retrieval, ranking, selection, chunking, or scoring behaviour change (this PR only observes existing behaviour through mocked round-trip counting; it changes no production code under src/lib/rag/** or the route itself).
  • npm run eval:rag / npm run eval:quality — not applicable; no answer-generation or synthesis-prompt change.
  • npm run check:production-readiness — not applicable; no clinical workflow, privacy, environment, Supabase, or source-governance behaviour change (test-only diff).

Risk and rollout

  • Risk: none to production behaviour — this PR adds one new test file only; no application, config, or dependency code changed. Worst case if the pinned counts are wrong is a false-positive/false-negative test failure, not a runtime regression.
  • Rollback: revert any of the three commits independently — each stands on its own (harness → pinning/disposition → a mechanical type-signature fix to the harness).
  • Provider or production effects: None. Fully offline/mocked — RAG_PROVIDER_MODE=offline, a stubbed @/lib/supabase/admin client, and stubbed @/lib/openai. No network, database, or provider calls.

Notes

  • This PR touches only tests/search-route-round-trip-budget.test.ts — it does not modify src/app/api/search/route.ts, src/lib/rag/**, or any other production/ranking surface, so no Clinical Governance Preflight or RAG impact: line applies (confirmed against scripts/pr-policy.mjs's classifyPullRequestFiles: the changed path matches neither clinicalRiskPatterns nor ragRankingPatterns).

Generated by Claude Code

claude added 3 commits August 14, 2026 11:32
Ledger #98 residual: the offline round-trip budget harness pinned the
answer path and the search retrieval core (searchChunksWithTelemetry)
but left the /api/search route itself invisible — auth, rate limiting,
scope resolution, related-document enrichment, and the telemetry write
were untested. Drive POST directly with a counted Supabase client,
following the tests/answer-route-preamble.test.ts pattern of importing
the route fresh per scenario with vi.doMock, so a round trip added
anywhere in the route preamble or post-processing becomes a red gate.

This establishes the harness and non-vacuity checks only; pinning the
exact count/shape is ledger #189's follow-up commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
…he ×3 text-RPC fan-out

Ledger #189: tighten tests/search-route-round-trip-budget.test.ts from
"the harness exists and observes traffic" to an explicit pinned count
(16) and exact breakdown, so a route round trip added or removed is a
red gate rather than something a reviewer has to notice.

Also traced why match_document_chunks_text_v2 and
match_document_table_facts_text_v2 are each issued 3 times per search
(already pinned separately at the retrieval-core layer by
tests/search-round-trip-budget.test.ts): rag-candidate-sources.ts fans
out up to maxTextRpcQueryVariants (3) lexical query-variant phrasings —
primary plus up to two siblings — per text surface, to rescue recall
when the primary phrasing misses. A PT-02 early exit
(firstVariantPoolIsStrong in rag-retrieval-variants.ts) already skips
the sibling RPCs when the primary pool is deep and precisely anchored,
so this ×3 is the fixture's worst-case shape (a single mocked chunk
never clears the early-exit depth bar), not the typical cost against a
well-matched corpus. Disposition: intentional, keep as-is — no code
change in this PR. Full reasoning in the PR body per ledger #189's
scope (decide-and-record, not collapse-without-canary).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
npm run typecheck caught TS2416: RouteQuery.then() reused a single
type parameter for both the fulfilled and rejected branches, which is
narrower than lib.es5's PromiseLike<T>.then<TResult1, TResult2>(...).
Match the base interface's two-type-parameter signature so `.insert().then(undefined, onRejected)`
(the pattern logWeakSearch uses) type-checks.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
@supabase

supabase Bot commented Aug 14, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

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: 43 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: 9d45f101-340a-4d2a-917d-2484ec418931

📥 Commits

Reviewing files that changed from the base of the PR and between c07ebc1 and c9bd584.

📒 Files selected for processing (2)
  • docs/branch-review-records/daeebafbd027baeaf3c0cb7684f3d6e92386ed33e1fe2592147351ebb10efec8.record.md
  • tests/search-route-round-trip-budget.test.ts

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

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
BigSimmo pushed a commit that referenced this pull request Aug 14, 2026
Three independent branches (PR #1947, #1949, #1950) hit
check:medication-lexicon-report failing on a file with zero diff
against origin/main. Queues a P2 ledger request to investigate the
staleness-detection logic itself rather than let it keep recurring
unaddressed across unrelated PRs.
@BigSimmo
BigSimmo enabled auto-merge August 14, 2026 12:35
@BigSimmo
BigSimmo disabled auto-merge August 14, 2026 13:01
@BigSimmo
BigSimmo enabled auto-merge (squash) August 14, 2026 13:13
@BigSimmo
BigSimmo merged commit 563ce41 into main Aug 14, 2026
24 checks passed
@BigSimmo
BigSimmo deleted the codex/search-round-trip-budget branch August 14, 2026 13:18
BigSimmo pushed a commit that referenced this pull request Aug 14, 2026
…merged

#215, #210, and #293 were queued as `done` in this same PR, each citing a PR
(#1949, #1953, #1962) that turned out still to be open, not merged — verified
by checking whether each PR's actual file change is present on origin/main
(none are). Reconciling the original `done` requests would have closed these
rows while their fixes exist only on unmerged branches, one of which (#1949)
currently has failing required CI.

Converts all three to `update` requests documenting the correction and the
real current state, so reconciling this PR now cannot close a row before its
fix has actually landed. Leaves #98, #189, and #194 as `done` unchanged —
those cite PR #1950 and PR #1947, both confirmed merged into main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants