Skip to content

fix(rag): stop the source-backed review fallback relabelling a rejected answer as trustworthy (#ZK460W) - #2721

Open
BigSimmo wants to merge 4 commits into
mainfrom
claude/rag-review-fallback-grounded-flip
Open

fix(rag): stop the source-backed review fallback relabelling a rejected answer as trustworthy (#ZK460W)#2721
BigSimmo wants to merge 4 commits into
mainfrom
claude/rag-review-fallback-grounded-flip

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • The source-backed review fallback is entered because the candidate answer failed its own quality gate. It then set grounded: true and re-derived confidence from retrieval similarity, which told every downstream consumer the opposite of what the gate had just decided: deriveTrust resolved to high or medium, quote cards unlocked, the source-gap warning was suppressed, and assessAndEnforceClaimSupport ran its high-risk enforcement over claims this route force-classifies as routine, so it passed vacuously.
  • Ledger #ZK460W named one site. There are three, all under SOURCE_BACKED_REVIEW_FALLBACK_REASON: the extractive review fallback, the generation fallback, and the post-generation claim quality gate. All three now keep the gate's verdict, and their citations carry provenance: "review_only" so nothing renders them as accepted claim support.
  • finalizeRagAnswerQualityCore replaces any ungrounded, unsupported answer with "No current source ... was found". That is written for model prose. On this route it printed a "nothing found" message directly above the sources that were in fact found, and avoiding it is why the route flagged itself grounded in the first place. The route now returns before those gates — safe because its prose is fixed text built in that module and each routing site already strips the rejected candidate's sections — with its display mode forced to evidence_gap so a plan built for the rejected candidate cannot ask for a threshold-table or comparison shape it has no rows to fill.
  • The fallback prose asserted "The uploaded documents contain relevant guidance on {subject}", where {subject} was rewritten from the clinician's own query. That claimed something this route had just failed to establish, and it carried query text into the delivered answer. Measured, not theorised: with the echo still in place, the offline adversarial case scope-other-owner-document put its planted patient-name canary into the answer body. The pointer is now fixed text taking no query at all, so the guarantee is structural rather than a property of the current wording.

Test changes, each recorded in the test with its reasoning:

  • Three assertions in rag-answer-fallback pinned the defect rather than the behaviour their own comments described — including one satisfied only by the echoed query.
  • Two prose tests in answer-responsiveness-gate constrained how the query was rewritten into the pointer; they are replaced by the stronger contract that nothing from the query reaches it.
  • Two round-trip budget non-vacuity checks used grounded === true to prove the answer path ran. Offline that flag only ever proved the relabelling ran. Citations are the honest signal, and the measured budgets are unchanged.
  • Five adversarial answer_from_evidence cases appeared to pass because the flag, not an answer, satisfied the expectation. They join KNOWN_DIVERGENCES with the observed shape pinned; the fixture stays normative and the entries self-expire.

Three immutable ledger inbox requests ride along (done #ZK460W, update #NTAV3D, add a row for the five newly divergent cases). They are pending only — reconcile after this lands.

Verification

  • npm run verify:pr-local — heavy scope, 39/39 checks, failed: (none), not reached: (none), exit 0.
  • Full offline unit suite: Test Files 1266 passed | 1 skipped (1267), Tests 18011 passed | 2 expected fail | 3 skipped.
  • Mutation-proved: each of the three routing sites reverted individually (site 1 fired 8 tests across two suites, site 2 fired the #231 case, site 3 fired the admission/discharge case), and the prose reverted to the old guidance claim fired 3 tests. Restored after each.
  • UI verification not run: no UI file changed.
  • Verification not run: npm run eval:retrieval:quality, npm run eval:rag, npm run eval:quality are provider-backed and this session has no live keys. Retrieval, ranking, selection, chunking and scoring are untouched by this diff; the live signal is the canary below.

Risk and rollout

  • Risk: medium. Answers on the source-backed review fallback now present as unsupported rather than grounded. That is the point of the change, but it is a visible difference: quote cards no longer render on this route, trust reads "unsupported", and the source-gap warning appears. Citations survive and are labelled "Added for source review; not accepted as claim support." This diff does not change how often the route is entered, only what it reports about itself.
  • Rollback: single-commit revert of 78e520914. No migration, no schema, no config.
  • Provider or production effects: None from this PR itself. The post-merge canary dispatch is provider-backed and was explicitly authorised by the owner.
  • RAG impact: behaviour change — canary pair baseline run 34105499999 (main, head c8cc72fa, dispatched 2026-09-07 09:20 UTC) -> one post-merge eval-canary dispatch, compared with npm run eval:retrieval:compare -- <baseline> <post> --fail-on-regression.

Read the baseline honestly before merging. Run 34105499999 FAILED, as has every canary since 2026-09-02 (runs 90-97). Its "Golden retrieval eval (live corpus)" step passed; the failure is in "Answer-quality subset (live generation)" and predates this branch by weeks. Retrieval recall — the metric the ranking safeguard exists to protect — is therefore green on main and is what the comparison must show unchanged. The answer-quality half of the pair cannot go from red to green here and is not claimed to.

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 three of those boxes, since this PR moves them rather than merely leaving them intact:

  • Linked-source verification is strengthened: this route no longer presents unverified content as claim support.
  • The patient-identifiable box is improved: the fallback no longer copies query text, which is how a planted patient-name canary reached the answer body in the offline harness.
  • TGA SaMD: clinical decision-support behaviour did change, in the conservative direction only — an answer the pipeline had rejected is no longer presented as trustworthy.

Notes

  • #VXB8XA (cite-mismatched-attribution) is unaffected and still diverges on its original shape.
  • #NTAV3D and #C2D9JF now have identical observed shapes: both point at in-scope evidence rather than refusing. That is one remaining question, not two, and the queued update says so.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GjUFJhAsyACuyJBvdqt4KF


Note

Medium Risk
Changes how rejected RAG answers are labeled and displayed (trust, warnings, quote cards) on an existing fallback path; behaviour is more conservative but user-visible.

Overview
Fixes ledger #ZK460W: when the pipeline degrades to the source-backed review fallback (candidate failed quality gates), answers are no longer marked grounded: true with confidence re-derived from retrieval. All three routing sites under SOURCE_BACKED_REVIEW_FALLBACK_REASON now keep grounded: false, confidence: "unsupported", and citations tagged provenance: "review_only" so UI does not treat them as accepted claim support.

The timeout/review pointer prose is fixed text (no query argument): it no longer claims documents contain “relevant guidance on {rewritten query}” or echoes clinician input into the answer. finalizeRagAnswerQualityCore short-circuits for this route so model-prose gates do not replace the pointer with “no source found” above citations that were retrieved; responseMode is forced to evidence_gap.

Tests and the adversarial harness are updated to assert the honest shape (including five offline answer_from_evidence cases pinned in KNOWN_DIVERGENCES). Outstanding-issues inbox/snapshot JSON records done #ZK460W, related updates, and a new P2 row for the newly exposed divergences; a P3 UI flake note is added separately.

Reviewed by Cursor Bugbot for commit 7ca2214. Configure here.

…ed answer as trustworthy (#ZK460W)

The source-backed review fallback is entered because the candidate answer failed its own
quality gate. It then set `grounded: true` and re-derived `confidence` from retrieval
similarity, which told every downstream consumer the opposite of what the gate had just
decided: `deriveTrust` resolved to high or medium, quote cards unlocked, the source-gap
warning was suppressed, and `assessAndEnforceClaimSupport` ran its high-risk enforcement
over claims this route force-classifies as routine, so it passed vacuously.

The ledger row named one site. There are three, all under
SOURCE_BACKED_REVIEW_FALLBACK_REASON: the extractive review fallback, the generation
fallback, and the post-generation claim quality gate. All three now keep the gate's
verdict, and their citations carry `provenance: "review_only"` so nothing renders them as
accepted claim support.

Two consequences had to be handled rather than worked around.

`finalizeRagAnswerQualityCore` replaces any ungrounded, unsupported answer with
"No current source ... was found". That is written for model prose. On this route it
printed a "nothing found" message directly above the sources that were in fact found, and
avoiding it is why the route flagged itself grounded in the first place. The route now
returns before those gates, which is safe because its prose is fixed text built in that
module and the rejected candidate's sections are already stripped at each routing site.
Its display mode is forced to `evidence_gap` so a plan built for the rejected candidate
cannot ask for a threshold-table or comparison shape it has no rows to fill.

The fallback prose asserted "The uploaded documents contain relevant guidance on
{subject}", where {subject} was rewritten from the clinician's own query. That claimed
something this route had just failed to establish, and it carried query text into the
delivered answer. Measured, not theorised: with the prose still echoing the query, the
offline adversarial harness case `scope-other-owner-document` put its planted patient-name
canary into the answer body. The pointer is now fixed text taking no query at all, so the
guarantee is structural.

Test changes, each recorded in the test with its reasoning:

- Three assertions in rag-answer-fallback pinned the defect rather than the behaviour
  their own comments described, including one satisfied only by the echoed query.
- Two prose tests in answer-responsiveness-gate constrained how the query was rewritten
  into the pointer; they are replaced by the stronger contract that nothing from the query
  reaches it.
- Two round-trip budget non-vacuity checks used `grounded === true` to prove the answer
  path ran; offline that flag only ever proved the relabelling ran. Citations are the
  honest signal and the measured budgets are unchanged.
- Five adversarial `answer_from_evidence` cases appeared to pass because the flag, not an
  answer, satisfied the expectation. They join the KNOWN_DIVERGENCES register with the
  observed shape pinned; the fixture stays normative and the entries self-expire.

Offline suite green: 1266 files, 18011 tests. Each of the three sites was mutation-proved
individually, as was the prose.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GjUFJhAsyACuyJBvdqt4KF
…ape, and the five newly divergent adversarial cases

Three immutable inbox requests, to be applied by a later `npm run issues:reconcile`
from a dedicated branch:

- done #ZK460W, with the outcome recording that the defect was at three sites rather
  than the one the row named, and the two consequences the row did not anticipate
  (the final quality gate's replacement prose, and the query echo in the pointer).
- update #NTAV3D, whose observed shape changed: still divergent, but now on the
  citation clause alone rather than on grounded.
- add a P2 row for the five adversarial `answer_from_evidence` cases that were
  passing on the grounded flag rather than on an answer, and now carry pins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GjUFJhAsyACuyJBvdqt4KF
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 6737a0f8-8ec3-4e3a-a408-05eab44378b5


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@supabase

supabase Bot commented Sep 7, 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 ↗︎.

@cursor

cursor Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_aacb64e4-277b-442f-8db7-fc8abf77bc55)

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T10:13:47.939834Z df6ec2f PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df6ec2f8cd

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

Comment thread src/lib/rag/rag.ts
Comment thread src/lib/rag/rag.ts
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Production UI (3)needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #16283 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

BigSimmo commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Two red checks on df6ec2f, both now handled

PR policy — fixed, and it was mine.

The three Clinical Governance Preflight boxes I had annotated inline ("— strengthened: ...") no longer matched the canonical item text, and collectSatisfiedGovernanceItems compares those seven lines exactly, so it counted 4 of 7. Reproduced offline against evaluatePullRequestPolicy with the old body:

ok: false
errors: [ "Check every Clinical Governance Preflight item before marking the PR ready (all 7 boxes checked, none left unchecked)." ]

and with the corrected body:

ok: true
errors: []
warnings: []

The seven boxes are now verbatim and the notes moved to a paragraph beneath them, so nothing was lost. The body is updated; no code changed. (The bolded - **RAG impact:** line was NOT a cause — ragImpactDeclared accepts it; I checked rather than assumed.)

Production UI (3) — did not reproduce, and is not reachable from this diff.

Failing test: tests/ui-tools.spec.ts:2928 "differentials compare queue launches presentation comparison". The compare-queue click never navigated:

Expected pattern: /\/differentials\/presentations\/acute-confusion-encephalopathy/
Received string:  "http://localhost:4598/differentials/compare?ids=wernicke-encephalopathy"
Timeout: 30000ms

Measured locally, four runs:

Run Result
this branch, full shard 3 1 failed, 206 passed (8.5m) — reproduced
this branch, full shard 3, second run 207 passed (7.7m)
this branch, that test alone passed (6.4s)
origin/main (0177bed1), full shard 3 207 passed (7.6m)
origin/main, that test alone passed (6.1s)

So it reproduced once and not again on identical code, and the base branch is green. It is a client-side navigation timeout in the differentials compare queue. This PR changes src/lib/rag/rag.ts and src/lib/rag/rag-extractive-answer.ts only — the answer pipeline. That journey makes no answer call, and the UI e2e suite serves demo/mocked answers regardless, so there is no path from this diff to it.

I am not calling that a root cause and not touching the test: quarantine here needs three reproductions on one SHA, and editing an unrelated differentials spec would widen this PR into someone else's flake. I have spent the one permitted re-run on the failed jobs of run 34109215397. If it goes red a second time on the same test, that is real and I will treat it as this PR's to root-cause.

Note for whoever picks this up: Production UI runs on this PR because scripts/ci-change-scope.mjs classifies any src/lib/** change as ui_changed=true (fail-closed). That is correct behaviour, not a misfire — it just means answer-pipeline PRs carry the full browser suite and its variance.


Generated by Claude Code

…-queue navigation timeout

Seen once in CI on this branch's Production UI shard 3, then cleared four ways
locally: a second full shard run on the same code passed, the test alone passed
on this branch and on origin/main, and origin/main's full shard passed. One
reproduction is well under the three-on-one-SHA bar for the flake ledger, so
this is a record rather than a quarantine — a second sighting should be read as
a pattern instead of being re-diagnosed from scratch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GjUFJhAsyACuyJBvdqt4KF
@cursor

cursor Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_2752026f-862d-470c-beab-1155e2fb4b58)

…n did not reach the reader (#ZK460W)

Both raised by Codex review on PR #2721, both verified in source before fixing.

**The source rail re-promoted the evidence.** `collectSourceCandidates` gathers
`smartApiPlan.coreSourceLinks` before `citations`, and `dedupeSourceLinks` is first-wins,
so on this route the core link shadowed the `review_only` citation for the same passage.
A core link is a bare `{chunk_id, document_id, title, file_name, page_number}` synthesised
from the answer plan and carries no provenance, so the rendered row came back reading
"Selected by the canonical answer source plan." with the plan's own source strength. The
demotion applied at the routing site never reached the source rail at all — the same defect
this branch exists to fix, surviving through a second channel.

`review_only` now takes precedence over any other candidate for the same citation identity,
which is the general fix rather than one scoped to this route. It is bounded to passages the
answer itself demoted: a core link for a chunk with no review-only citation keeps the answer
plan's wording, and there is a test for that so this cannot drift into a blanket downgrade.

The reviewer's report named the label "This page states the claim directly." That string
lives only in `answer-chat-perfected-v2-mockups.tsx`, not the production render path, so the
specific wording was wrong. The substance was right, which is what mattered.

**Cached pre-fix answers would have outlived the deploy.** The extractive arm of this route
carries no `generation_fallback:` marker, so `getSharedCachedAnswer` does not evict it, and
nothing else in the cache key changed. A repeat query would have kept serving the exact
answer this branch demotes — grounded, with deterministic-support citations — for the whole
`RAG_ANSWER_CACHE_TTL_MS` window after rollout. `ragCacheDependencyVersion` is bumped to
v22, which is the mechanism that constant exists for; the cost is one cold answer cache.

Offline suite green: 1266 files, 18013 tests. The provenance override was mutation-proved:
removed, the new test fails; restored, it passes.

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

BigSimmo commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Both Codex P1 findings verified and fixed in 103731b4

Posting here rather than on the two threads: this session's PR-follow guard blocks the API read that would give me the review-comment ids, so I cannot address them inline. Both are real, both are fixed, and both now have a test.

1. "Demote Smart API links with review-only citations" — correct.

Verified in source rather than taken on trust: collectSourceCandidates pushes smartApiPlan.coreSourceLinks before citations (answer-render-policy.ts:291), and dedupeSourceLinks is first-wins (if (seen.has(key)) continue, :333). candidateFromCoreSourceLink synthesises a bare {chunk_id, document_id, title, file_name, page_number} with no provenance, and sourceLinkFromCandidate copies provenance: citation.provenance — so the rendered row carried undefined and the reason "Selected by the canonical answer source plan." The demotion applied at the routing site never reached the source rail. That is this branch's own defect surviving through a second channel, so thank you.

Fixed by making review_only take precedence over any other candidate for the same citation identity — the general fix you offered as the alternative, not one scoped to this route. It is bounded to passages the answer itself demoted: a core link for a chunk with no review-only citation keeps the plan's wording, and there is a second test pinning that so it cannot drift into a blanket downgrade.

One correction to the report, for the record rather than to argue with it: the label "This page states the claim directly." appears only in src/components/answer-chat-perfected-v2-mockups.tsx, not the production render path, so that specific wording was not what a reader saw. The mechanism you described was right, which is the part that mattered.

2. "Invalidate pre-fix source-review cache entries" — correct.

Confirmed the gap is real and specific to the extractive arm. GENERATION_FALLBACK_MARKER is /(?:^|;\s*)generation_fallback(?::|$)/i, and the measured routing reason on that arm is high_confidence_extractive_retrieval; final_quality_gate:source_gap; source_backed_review_fallback; extractive_quality_gate:source_gap — no marker, so getSharedCachedAnswer does not evict it, and nothing else in the key changed. A repeat query would have kept serving the exact answer this branch demotes for the whole RAG_ANSWER_CACHE_TTL_MS window after rollout.

ragCacheDependencyVersion is bumped rag-cache-v21v22, which is what that constant exists for. Cost is one cold answer cache after deploy.

Verification on 103731b4

  • Offline suite: Test Files 1266 passed | 1 skipped (1267), Tests 18013 passed | 2 expected fail | 3 skipped.
  • npm run verify:pr-local: heavy scope, 41 checks, failed: (none), not reached: (none), exit 0.
  • Mutation-proved the provenance override: removed it, the new test fails on the exact assertion; restored, it passes.

Generated by Claude Code

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