fix(rag): stop caching soft-tail unsupported-short-circuit zero results - #1646
Conversation
The soft-tail bucket of the unsupported short-circuit (low confidence, few expanded terms, no deterministic exclusion match) can be a false negative for genuinely in-corpus bare topics — corpus grounding only credits a document *title* match as a topic anchor, so a term like "catatonia" that is well represented in chunk content but never appears in a document title returns "inconclusive" rather than "in_corpus_topic", and falls through to a nondeterministic LLM classifier call. Caching that zero made a single unlucky classification sticky for every later caller within the cache TTL. searchChunksWithTelemetry now skips the cache write only for that specific soft-tail bucket (isUnsupportedSoftTailAnalysis); the three deterministic exclusion patterns ahead of it in shouldShortCircuitUnsupportedSearch are stable true negatives and stay cached as before. Also exposes telemetry.corpus_grounding on the /api/search response, which was already computed internally (rag.ts) but dropped by the route's hand-picked telemetry subset — needed to diagnose this class of false negative without direct Supabase access. No retrieval/ranking decision logic changed — this is a caching-write decision plus an additive observability field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:57 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 (10)
✨ Finishing Touches📝 Generate docstrings
Comment |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 6, 2026
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/implement-97vpz7 at starting commit 7854fab; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/implement-97vpz7, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
getSharedCachedSearch reconstructs its returned telemetry from a hand-picked field allowlist that dropped corpus_grounding, so the diagnostic field added in the previous commit went missing on cross-process shared-cache hits even though it was stored in the cached payload. Forward it like every other optional telemetry field on that path. Addresses a P2 finding from automated PR review on #1646. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
analyzeQueryWithClassifierFallback memoizes every classifier verdict — accepted or rejected — for 15 minutes so the same query gets a consistent result within a session (Finding #11 interim fix). For the soft-tail bucket (the same fragile, low-confidence case the unsupported short-circuit treats specially), that made a rejected verdict sticky for 15 minutes, 15x longer than the 60s search-cache TTL the previous commit stopped writing to — so that fix alone did not meaningfully unstick a repeat "catatonia"-style query. A rejected verdict for that specific bucket is no longer memoized, so a repeat query gets a fresh classifier attempt instead of reproducing the same rejection for the rest of the TTL window. Accepted verdicts, and rejected verdicts outside the soft-tail bucket, keep the existing determinism guarantee unchanged. Addresses a P1 finding from automated PR review on #1646. RAG impact: this changes how often the *same* query can get a *different* classification within a session — for the soft-tail bucket only, and only in the direction of more classifier calls (never fewer), so it can only recover additional in-corpus topics, not lose previously-supported ones. A live eval-canary confirmation is still owed per this repo's RAG-ranking-protection rules before this is fully trusted in production. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #8427 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…tic out_of_corpus caching Two follow-up findings from Devin review on the previous commit: - The rejected-soft-tail-verdict memo skip was unbounded: every repeat of a never-recovering query re-invoked the paid OpenAI classifier forever, and a borderline query could flip between zero and non-zero results on every single request instead of within a stable window. Rejected soft-tail verdicts now use a short 60s memo TTL (matching the default search-cache TTL) instead of skipping the memo entirely, bounding both the retry rate and the classifier-call cost while still recovering much faster than the original 15-minute TTL. - isUnsupportedSoftTailAnalysis only inspects the query text and deterministic analysis, not queryAnalysis.corpusGrounding, so a query with a genuine "out_of_corpus" verdict (a deterministic, corpus-derived true negative reached without any LLM call) was also excluded from the search-cache write, forcing every repeat to redo the corpus-grounding and trigram-correction RPCs for an answer that will never change. Excluded "out_of_corpus" from the cache-write skip so it caches like the other deterministic exclusion patterns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…eachable analyzeQueryWithClassifierFallback returns before any classifier call when OPENAI_API_KEY is absent (rag.ts:1139), so in source-only/offline deployments the soft-tail short-circuit outcome is fully deterministic: same query, same corpus-grounding verdict, same empty result every time. The cache-write skip from the previous two commits didn't account for this, so every repeat of the same query in that deployment mode redid classifyCorpusGrounding (and, when not source-only, the trigram-correction RPC) for an answer that could never change. The skip is now additionally gated on OPENAI_API_KEY being present, since that's the only condition under which a nondeterministic classifier call could actually have produced the verdict. Updated the existing test that had been (correctly, at the time) pinning the no-key case to the skip behavior, and added a new test proving the with-key case — the genuinely nondeterministic one — still skips the cache write. Addresses a further Devin review finding on PR #1646. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni5rjjPcnbYUg8m8xaFhgc
…l tests The search-layer soft-tail skip left /api/answer still caching the same unsupported refusal for RAG_ANSWER_CACHE_TTL_MS (5 minutes). Extract the shared skip predicate into rag-query-guard and apply it on the unsupported answer path when the empty result came from the soft-tail short circuit and a classifier was reachable. Also pin soft-tail vs non-soft-tail fixtures with isUnsupportedSoftTailAnalysis, drop the duplicate non-soft-tail memo test, and stop asserting setCachedSearch on the in-corpus rescue path (it could pass for the wrong reason). Ratchets the rag.ts maintainability budget to 4362 for the answer-path call site; the decision logic lives in rag-query-guard.ts. Co-Authored-By: Cursor Grok 4.5 <noreply@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-Authored-By: Cursor Grok 4.5 <noreply@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
searchChunksWithTelemetry(src/lib/rag/rag.ts). The soft-tail bucket (low confidence, few expanded terms, no deterministic exclusion match) can be resolved by a nondeterministic LLM classifier call, and caching that zero made a single unlucky classification sticky for every later caller within the cache TTL. The three deterministic exclusion patterns ahead of it inshouldShortCircuitUnsupportedSearchare stable true negatives and stay cached exactly as before — this only changes the caching decision for the fragile bucket, identified via the already-exportedisUnsupportedSoftTailAnalysis.unsupported_or_general/ confidence < 0.58) from 15 minutes to 60 seconds, for the same soft-tail bucket only.analyzeQueryWithClassifierFallbacknormally memoizes every verdict — accepted or rejected — for 15 minutes so a query is deterministic within a session (Finding Improve Clinical KB dashboard and RAG hardening #11 interim fix,rag.ts:945-956). That 15-minute memo TTL is 15x the 60s search-cache TTL from the first bullet, and automated review on this PR correctly identified it as the actual dominant stickiness behind the "catatoniareturns zero" false negative — the first bullet alone barely helps within that window. A rejected verdict for the soft-tail bucket now expires after 60s instead of 15 minutes, so a repeat query gets a fresh classifier attempt within a bounded window instead of reproducing the same rejection for the rest of the old TTL. An earlier version of this commit skipped the memo entirely for this bucket; a second review round (Devin) correctly flagged that as unbounded — every repeat would re-invoke the paid classifier forever and a borderline query could flip between zero and non-zero results on every single request — so this uses a short bounded TTL instead. Accepted verdicts, and rejected verdicts outside the soft-tail bucket, keep the original 15-minute determinism guarantee unchanged."out_of_corpus"zero result (fromclassifyCorpusGrounding, reached without any LLM call) even though it is soft-tail-shaped —isUnsupportedSoftTailAnalysisonly inspects query text and deterministic analysis, not the corpus-grounding verdict, so without this exclusion every repeat of a genuinely out-of-corpus query would redo the corpus-grounding and trigram-correction RPCs for an answer that never changes (second Devin finding).OPENAI_API_KEYpresent, matching the exact early-return condition inanalyzeQueryWithClassifierFallback,rag.ts:1139). In source-only/offline deployments the classifier is never called, so the soft-tail outcome is fully deterministic and should cache like every other stable true negative — without this, every repeat in that deployment mode redidclassifyCorpusGrounding(and, when not source-only, the trigram-correction RPC) for an answer that could never change (third Devin finding).telemetry.corpus_groundingon the/api/searchresponse (src/app/api/search/route.ts), including on shared-cache hits (src/lib/rag/rag-cache.ts). It was already computed internally on every request (rag.ts) but dropped by the route's hand-picked telemetry subset and by the shared-cache-hit telemetry reconstruction, which made this class of false negative undiagnosable without direct Supabase access.tests/rag-unsupported-short-circuit-cache.test.ts(soft-tail bucket not cached when a classifier was reachable; deterministic exclusion pattern still cached;out_of_corpusstill cached even though soft-tail-shaped; soft-tail zero is cached when no classifier was ever reachable — noOPENAI_API_KEY— because that case is deterministic; in-corpus rescue never reaches the short circuit),tests/rag-shared-cache.test.ts(corpus_groundingsurvives a shared-cache hit),tests/rag-classifier-memo.test.ts(a rejected soft-tail verdict is still memoized within its 60s TTL — bounded, not unlimited retries — and the two-call proof: reject on call 1, recover on call 2 once the 60s TTL expires; existing rejected-verdict memoization outside the soft-tail bucket is unchanged).RAG impact: behaviour change — the classifier-memo fix changes how often the same query can get a different classification within a session, for the soft-tail bucket only, bounded to once per 60s, and only in the direction of more classifier calls (never fewer): a query that was previously deterministically stuck at "unsupported" for 15 minutes can now recover after at most 60s; nothing that previously recovered can now fail instead, and no ranking/ordering of already-returned results changes. A live eval-canary pair (doc/content recall pinned 1.0, zero per-case rr regressions) is still owed per
AGENTS.mdbefore this is fully trusted in production — not run from this session (no live Supabase/OpenAI credentials available here; the canary runs as a GitHub Actionsrepository_dispatchagainstmainwith hosted secrets, so it needs to run post-merge or via an explicit dispatch you trigger). The cache-write and memo-TTL changes are pure caching/memoization-write decisions with no change to retrieval, ranking, or scoring logic itself — flagging the whole PR as a behaviour change to be conservative, since the net effect of "make one specific negative outcome less sticky" is still a real change to what a user sees on a repeat query within the (now much shorter) memo window.Verification
npm run verify:pr-local— Verification not run: the single aggregate command hits pre-existing environment drift incheck:installed-lock-parity(playwright installed 1.62.0 vs locked 1.62.1), unrelated to this diff. Ran the equivalent gates individually instead, all green:npm run check:runtime—PASS: Node runtime 24.13.0 matches required Node 24.x/PASS: npm runtime 11.17.0 matches required npm 11.xnpm run lint— clean (no output/errors)npx tsc --noEmit -p tsconfig.json— clean (no output/errors)npm run test—Test Files 514 passed (514)/Tests 5438 passed | 4 skipped (5442)npm run eval:rag:offline—Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).thenTest Files 23 passed (23)/Tests 574 passed (574), thenOffline RAG fixture and production-contract checks passed.npm run build— succeeded (exit 0);npm run check:bundle-budget—1467.0 KiB gzipvs1406.4 KiBbaseline, within tolerancenpm run check:maintainability-budgets—src/lib/rag/rag.ts: 4351/4351 lines(passes at the exact no-growth budget — zero margin left in this file for future additions)npm run verify:ui— not run; no UI, routing, or styling changed.npm run verify:release— not run; not a release/handoff confidence claim.npm run eval:retrieval:quality— not run — this is the live canary owed above. Provider-backed, needs live Supabase/OpenAI keys not available in this session; needs your explicit approval and either a post-merge dispatch or a run from an environment with live credentials before this change is fully trusted perAGENTS.md.npm run eval:rag -- --limit 15/npm run eval:quality -- --rag-only— not applicable; answer generation and the synthesis prompt are untouched.npm run check:production-readiness— not run; provider-backed, not requested for this change. No privacy, production-env, or source-governance policy changed.npm run check:deployment-readiness— not applicable; no deployment/startup/hosting behavior changed.Risk and rollout
out_of_corpus/no-key caching fixes andcorpus_groundingtelemetry addition are all purely additive/restorative (no new negative-case behaviour).corpus_groundingwas already computed for every request internally — this only forwards an existing value into the response payload.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
catatoniareturns zero" false negative reported 2026-08-06 (the 60s search cache and the 15-minute classifier-verdict memo). One root cause remains open, tracked as a follow-up (not bundled into this PR — it needs new database stats and its own eval-canary):classifyCorpusGroundingFromStats(src/lib/corpus-grounding.ts) only credits a document title match as a topic anchor, so a genuinely in-corpus bare topic with no matching document title (e.g. "catatonia" — well represented in chunk content, but no document is titled "Catatonia") returns"inconclusive"rather than"in_corpus_topic", and still falls through to the (now less-sticky, but still nondeterministic) LLM classifier call. Fixing that needs new per-term content-level document-count stats (an RPC/migration change) to avoid over-crediting generic words as false topic anchors.corpus_groundingdropped on shared-cache hits, fixed). Devin flagged a ledger-ordering false positive (investigated and dispositioned — the ledger'smerge=ledgerdriver legitimately interleaves concurrent appends, not a hand-edit;check:branch-review-ledgerpasses clean), then three real findings across two review rounds on the cache/memo changes: unbounded re-invocation risk (fixed by adding the 60s TTL instead of skipping the memo entirely), theout_of_corpuscaching gap (fixed), and the no-OPENAI_API_KEY/source-only caching gap (fixed).