diff --git a/docs/branch-review-records/daeebafbd027baeaf3c0cb7684f3d6e92386ed33e1fe2592147351ebb10efec8.record.md b/docs/branch-review-records/daeebafbd027baeaf3c0cb7684f3d6e92386ed33e1fe2592147351ebb10efec8.record.md new file mode 100644 index 000000000..5c8531ad9 --- /dev/null +++ b/docs/branch-review-records/daeebafbd027baeaf3c0cb7684f3d6e92386ed33e1fe2592147351ebb10efec8.record.md @@ -0,0 +1 @@ +| 2026-08-14 | codex/search-round-trip-budget | fa37b9473a6835a6364dae45d791d7f193152bbb | tests/search-route-round-trip-budget.test.ts (PR #1950, ledger #098 residual + #189) | Author handoff: offline test-only diff adding a pinned Supabase round-trip budget for POST /api/search, no production/RAG code changed | verify:pr-local full run (lint/typecheck clean; test 602 files, 6511 passed, 4 skipped; check:rag:fixtures + check:medication-interactions passed; check:medication-lexicon-report pre-existing unrelated staleness) | diff --git a/tests/search-route-round-trip-budget.test.ts b/tests/search-route-round-trip-budget.test.ts new file mode 100644 index 000000000..4e9d19512 --- /dev/null +++ b/tests/search-route-round-trip-budget.test.ts @@ -0,0 +1,278 @@ +/** + * Route-level Supabase round-trip budget for `POST /api/search` (ledger `#098` + * residual, `#189`). + * + * `tests/search-round-trip-budget.test.ts` pins the offline **retrieval core** + * (`searchChunksWithTelemetry`) but says so explicitly: everything the route + * does *around* retrieval — auth, rate limiting, scope resolution, + * related-document enrichment, and the telemetry write — is invisible there. This + * file drives `POST` itself 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 instead of something a + * reviewer has to notice by reading a diff. + * + * **The numbers here are measured, not derived.** They record what the route + * does today for one representative anonymous, filter-free, non-cached search. + * A failure means "the traffic changed" — information, not automatically a + * defect. Read the printed breakdown, decide whether the change was intended, + * and move the budget in the same commit as the change that moved it. + * + * The pinned breakdown below issues `match_document_chunks_text_v2` and + * `match_document_table_facts_text_v2` three times each. That is not + * accidental fan-out: `rag-candidate-sources.ts` probes 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; this fixture's single mocked chunk never + * clears that depth bar, so the full ×3 fan-out fires here — the worst-case + * shape, not the typical one for a well-matched corpus. Disposition recorded + * in this PR's body rather than duplicated here so it stays in one place. + * + * Scope: what travels through the mocked `@/lib/supabase/admin` client for this + * one request shape. A different request shape (authenticated caller, active + * filters, documents/differentials mode, a cache hit, a rate-limit denial) pays + * a different bill and is not claimed here. Traffic via another client, a + * direct `fetch`, or a provider SDK is invisible to this counter. + * + * Provider-free and DB-free. No retrieval, ranking, or selection behaviour is + * changed by this file; it only observes. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { countSupabaseRoundTrips } from "./helpers/supabase-round-trip-counter"; +import type { SearchResult } from "../src/lib/types"; + +/** Minimal indexed chunk; only the fields retrieval reads are populated. Mirrors + * the fixture in `tests/search-round-trip-budget.test.ts` so the retrieval-core + * traffic underneath this route budget matches that pinned shape. */ +function source(overrides: Partial = {}): SearchResult { + return { + id: "clozapine-chunk-1", + document_id: "clozapine-doc", + title: "Clozapine Prescribing Administration Monitoring", + file_name: "CG.MHSP.ClozapinePresAdminMonitor.pdf", + page_number: 11, + chunk_index: 0, + section_heading: "Monitoring", + content: + "Withhold clozapine if the absolute neutrophil count (ANC) falls below 1.5 x10^9/L. Mandatory FBC monitoring is weekly for the first 18 weeks of clozapine treatment.", + image_ids: [], + similarity: 0.95, + hybrid_score: 0.95, + text_rank: 1.2, + table_facts: [], + source_metadata: { + source_title: "Clozapine source", + publisher: "Local service", + jurisdiction: "Australia/WA", + version: "1", + publication_date: null, + review_date: null, + uploaded_at: null, + indexed_at: null, + uploaded_by: null, + document_status: "current", + clinical_validation_status: "approved", + extraction_quality: "good", + }, + images: [], + ...overrides, + } as SearchResult; +} + +function rateLimitRow(limited: boolean) { + return { + limited, + limit_value: 60, + remaining: limited ? 0 : 59, + retry_after_seconds: limited ? 60 : 0, + reset_at: new Date(Date.now() + 60_000).toISOString(), + }; +} + +/** + * Fluent no-op builder for every `.from()` call the route's own preamble and + * post-processing make: `resolveSearchScope`'s (unreached here) document/label + * paging, `fetchRelatedDocuments`' metadata/visual enrichment, and the + * fire-and-forget `rag_retrieval_logs` / `rag_queries` / `rag_query_misses` + * inserts. Every chained method returns `this`; execution (`then`) yields an + * empty, error-free result — including for `.insert()`, which the real client + * also returns as an awaitable builder rather than a bare promise. + */ +class RouteQuery implements PromiseLike<{ data: unknown[]; error: null }> { + select() { + return this; + } + in() { + return this; + } + eq() { + return this; + } + neq() { + return this; + } + order() { + return this; + } + limit() { + return this; + } + is() { + return this; + } + or() { + return this; + } + abortSignal() { + return this; + } + maybeSingle() { + return this; + } + insert() { + return this; + } + then( + onfulfilled?: ((value: { data: unknown[]; error: null }) => TResult1 | PromiseLike) | null, + onrejected?: ((reason: unknown) => TResult2 | PromiseLike) | null, + ): PromiseLike { + return Promise.resolve({ data: [], error: null }).then(onfulfilled, onrejected); + } +} + +/** + * Drives `POST /api/search` for one anonymous request through a counted + * Supabase client, returning the parsed response and the round-trip counter. + * + * Reset before mocking, not only in `afterEach`: a test that calls this twice + * would otherwise get the *first* run's mocked `createAdminClient` on the + * second call (`tests/search-round-trip-budget.test.ts` hit this first). + */ +async function postSearchWithCountedClient( + body: Record, + textSources: SearchResult[], + options: { limited?: boolean } = {}, +) { + vi.resetModules(); + vi.stubEnv("RAG_PROVIDER_MODE", "offline"); + vi.stubEnv("RAG_SEARCH_CACHE_TTL_MS", "0"); + + const rpc = vi.fn(async (name: string) => { + if (name === "consume_api_subject_rate_limit") { + return { data: [rateLimitRow(options.limited ?? false)], error: null }; + } + if (name === "match_document_chunks_text_v2" || name === "match_document_chunks_text") { + return { data: textSources, error: null }; + } + return { data: [], error: null }; + }); + + const { client, counter } = countSupabaseRoundTrips({ rpc, from: vi.fn(() => new RouteQuery()) }); + + vi.doMock("@/lib/env", async (importOriginal) => ({ + ...(await importOriginal()), + isDemoMode: () => false, + isLocalNoAuthMode: () => false, + })); + vi.doMock("@/lib/supabase/admin", () => ({ createAdminClient: () => client })); + vi.doMock("@/lib/openai", () => ({ + embedTextWithTelemetry: vi.fn(), + generateStructuredTextResult: vi.fn(), + })); + + const { POST } = await import("../src/app/api/search/route"); + const response = await POST( + new Request("http://localhost/api/search", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }), + ); + const payload = (await response.json()) as Record; + + // logRetrievalDiagnostics / logSearchObservation are fire-and-forget: the + // route neither awaits nor blocks on them, so their Supabase inserts land in + // the microtask queue after `await POST(...)` already returned. A macrotask + // tick flushes every microtask queued by then, so this reads the client's + // traffic after telemetry has actually landed rather than mid-flight. + await new Promise((resolve) => setTimeout(resolve, 0)); + + return { response, payload, counter }; +} + +afterEach(() => { + // `doMock` registrations survive both `restoreAllMocks` (which targets spies) + // and `resetModules` (which clears the module cache, not the mock registry), + // so they are dropped explicitly — the pattern the sibling suites already use + // (`tests/rag-variant-early-exit.test.ts:110`, `tests/search-round-trip-budget.test.ts`). + vi.doUnmock("@/lib/env"); + vi.doUnmock("@/lib/supabase/admin"); + vi.doUnmock("@/lib/openai"); + vi.restoreAllMocks(); + vi.resetModules(); + vi.unstubAllEnvs(); +}); + +describe("Supabase round-trip budget on the /api/search route", () => { + it("pins the route-level round-trip count for an anonymous, filter-free clinical search", async () => { + const { response, payload, counter } = await postSearchWithCountedClient( + { query: "What ANC threshold should withhold clozapine?" }, + [source()], + ); + + // Non-vacuity before the budget: a budget asserted against a request that + // never actually reached the retrieval core would pass while proving + // nothing (the failure mode `#120` was filed from). + expect(response.status, "the route must actually answer, or the budget proves nothing").toBe(200); + expect( + Array.isArray(payload.results) ? payload.results.length : 0, + "the route must actually retrieve, or the budget proves nothing", + ).toBeGreaterThan(0); + expect( + counter.total(), + `the route must issue Supabase traffic, or a zero budget is meaningless — ${JSON.stringify(counter.breakdown())}`, + ).toBeGreaterThan(0); + + // Measured, not derived — this is what the route does today, pinned so a + // change (in the preamble, retrieval core, enrichment, or telemetry write) + // has to be deliberate. Update in the commit that moves it. + expect(counter.total(), `search route round trips changed — ${JSON.stringify(counter.breakdown())}`).toBe(16); + + // The shape matters as much as the total: a refactor that removed one probe + // and added an unrelated query would keep 16 while changing the traffic. + // See the file header for why the two text-RPC surfaces below are ×3, not ×1. + expect(counter.breakdown(), "search route round-trip shape changed").toEqual({ + "rpc:consume_api_subject_rate_limit": 1, + "from:rag_aliases": 1, + "rpc:match_document_chunks_text_v2": 3, + "rpc:match_document_table_facts_text_v2": 3, + "rpc:get_related_document_metadata_v2": 2, + "from:document_index_quality": 1, + "from:document_images": 3, + "from:rag_retrieval_logs": 1, + "from:rag_queries": 1, + }); + }); + + it("spends no rate-limit-denied request on retrieval traffic beyond the limiter check", async () => { + // A route budget also has to say what a *denied* caller costs: the limiter + // check itself, and nothing past it. This is the route-level analogue of + // `tests/answer-route-preamble.test.ts`'s "dispatches no scope query at all + // when the limiter denies" — same claim, this route's own preamble. + const { response, counter } = await postSearchWithCountedClient( + { query: "What ANC threshold should withhold clozapine?" }, + [source()], + { limited: true }, + ); + + expect(response.status).toBe(429); + expect( + counter.total(), + `a rate-limited request must cost only the limiter check — ${JSON.stringify(counter.breakdown())}`, + ).toBe(1); + expect(counter.countOf("consume_api_subject_rate_limit")).toBe(1); + }); +});