fix(rag): honor preformatted answers, render high-yield bold, guard safety caveats + retrieval telemetry#514
Conversation
…afety caveats + retrieval telemetry Client display fixes (offline-proven, no retrieval-ranking impact): - F1: the client now honors the server `preformatted` flag, so document-support-list and table/visual answers are no longer re-mangled by the prose sanitizer on display (document names and facility codes were being deleted). Adds a lossless normalizePreformattedDisplayText and threads the flag through the display chain. - F3: high-yield **bold** now reaches the rendered answer via SafeBoldText (display-only preserveBold; the shared server answer-gen path is unchanged), and the copy/paste clinical draft no longer leaks literal `**`. - F4: the compact primary-answer cap never drops a safety-signal fragment (withhold/stop/threshold/escalate/...); byte-identical when none is present. Retrieval observability (additive, retrieval results byte-identical): - F8: lexical RPC errors (match_document_chunks_text, match_document_table_facts_text, match_documents_for_query) now record hybrid_rpc_errors telemetry before returning empty, instead of silently degrading to zero candidates with no signal. Infra: - F7: replay-safe migration documenting the retrieval owner-filter drift with a verified, owner-runnable byte-perfect codification procedure. Live already gates all 8 primary retrieval RPCs with the fail-closed retrieval_owner_matches; the gap is committed-source reproducibility only. Verification: typecheck, lint, format, full vitest (1655 passed), eval:rag:offline, and the @critical demo answer-flow E2E all green. Live eval:retrieval:quality 36/36. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Updates to Preview Branch (claude/rag-review-improvements-f1bf84) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPreformatted answer handling now preserves structured content during sanitization and rendering, supports controlled bold-marker retention, protects safety fragments during compaction, strips bold markers from copy text, and records hybrid retrieval RPC failures through search telemetry. A migration adds ownership verification for retrieval RPC codification. ChangesAnswer display handling
Retrieval RPC telemetry
Retrieval ownership migration
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant ClinicalDashboard
participant sanitizeAnswerDisplayText
participant NaturalLanguageAnswer
ClinicalDashboard->>sanitizeAnswerDisplayText: sanitize grounded preformatted text
ClinicalDashboard->>NaturalLanguageAnswer: pass raw answer and preformatted state
NaturalLanguageAnswer->>sanitizeAnswerDisplayText: format with preformatted and preserveBold options
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e87c446854
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/clinical-dashboard/answer-content.tsx (1)
231-292: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHead fragments can still lose safety cues
primaryAnswerDisplayTextonly rescuessafetyTail; if a safety cue appears in one of the first 3 fragments after word 85, truncation still drops it. Add a regression test intests/rag-answer-text.test.tsfor a long head fragment containingwithhold/do not, and preserve safety-bearing head content when applying the cap.🤖 Prompt for 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. In `@src/components/clinical-dashboard/answer-content.tsx` around lines 231 - 292, The 85-word truncation in primaryAnswerDisplayText can remove safety signals from the first three fragments. Update the head-cap logic to preserve safety-bearing head content, including fragments matching primaryAnswerSafetySignalPattern, while retaining the existing compact behavior for non-safety text; add a regression case in rag-answer-text.test.ts covering a long head fragment containing “withhold” or “do not”.
🧹 Nitpick comments (1)
tests/rag-offline-answer.test.ts (1)
165-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the new telemetry contract directly.
This test verifies fail-closed behavior, but it would still pass if
recordHybridRpcErrorstopped recordingmatch_document_chunks_text. Add an assertion at the telemetry or structured-logging boundary for RPCmatch_document_chunks_textand code42P01.🤖 Prompt for 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. In `@tests/rag-offline-answer.test.ts` around lines 165 - 185, Extend the test around answerOffline to assert the telemetry or structured-logging boundary records an RPC error for match_document_chunks_text with code 42P01. Keep the existing fail-closed assertions unchanged, and use the test’s existing telemetry capture or spy mechanism rather than only checking the returned answer.
🤖 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 `@src/components/clinical-dashboard/display-text.ts`:
- Around line 212-218: Update the preformatted branch in
sanitizeAnswerDisplayText so its handling of **bold** markers matches
options.preserveBold, preventing literal Markdown markers when
PriorAnswerTurnSurface renders collapsed text. Pass the preserveBold setting
through normalizePreformattedDisplayText or explicitly strip markers as
appropriate, and add a regression test covering preformatted text with bold
enabled.
In `@src/lib/source-text-sanitizer.ts`:
- Around line 568-571: Update the blood-value guard patterns in source-text
sanitization to recognize bold markers around values, including line-start forms
such as “o **RhD negative**” and label-prefixed forms such as “group/type: o
**Negative**”. Preserve existing non-bold matching and add regression coverage
for both bold cases.
In
`@supabase/migrations/20260712000000_forward_codify_retrieval_owner_matches.sql`:
- Around line 51-55: Append the exact live CREATE OR REPLACE FUNCTION
definitions for all eight retrieval RPCs after the completion marker in the
migration, preserving their current tenancy and sentinel-handling behavior.
Verify the migration contains all eight function definitions before merging.
- Around line 56-65: Update the commented psql query that builds rpc_bodies.sql
so string_agg orders its inputs by p.proname and p.oid, and remove the trailing
outer ORDER BY p.proname. Keep the existing function filtering and aggregation
behavior unchanged.
---
Outside diff comments:
In `@src/components/clinical-dashboard/answer-content.tsx`:
- Around line 231-292: The 85-word truncation in primaryAnswerDisplayText can
remove safety signals from the first three fragments. Update the head-cap logic
to preserve safety-bearing head content, including fragments matching
primaryAnswerSafetySignalPattern, while retaining the existing compact behavior
for non-safety text; add a regression case in rag-answer-text.test.ts covering a
long head fragment containing “withhold” or “do not”.
---
Nitpick comments:
In `@tests/rag-offline-answer.test.ts`:
- Around line 165-185: Extend the test around answerOffline to assert the
telemetry or structured-logging boundary records an RPC error for
match_document_chunks_text with code 42P01. Keep the existing fail-closed
assertions unchanged, and use the test’s existing telemetry capture or spy
mechanism rather than only checking the returned answer.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7474c87f-ee2b-4c19-bea5-60ac243a0764
📒 Files selected for processing (16)
src/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/answer-content.tsxsrc/components/clinical-dashboard/answer-result-surface.tsxsrc/components/clinical-dashboard/display-text.tssrc/components/clinical-dashboard/evidence-panels.tsxsrc/components/clinical-dashboard/output-panel.tsxsrc/lib/answer-render-policy.tssrc/lib/rag-answer-text.tssrc/lib/rag.tssrc/lib/source-text-sanitizer.tssupabase/migrations/20260712000000_forward_codify_retrieval_owner_matches.sqltests/answer-render-policy.test.tstests/display-text.test.tstests/rag-answer-text.test.tstests/rag-offline-answer.test.tstests/source-text-sanitizer.test.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
Note Docstrings generation - SKIPPED |
|
@codex diagnose and fix failing CI test |
Docstrings generation was requested by @BigSimmo. * #514 (comment) The following files were modified: * `src/components/ClinicalDashboard.tsx` * `src/components/clinical-dashboard/answer-content.tsx` * `src/components/clinical-dashboard/answer-result-surface.tsx` * `src/components/clinical-dashboard/display-text.ts` * `src/components/clinical-dashboard/evidence-panels.tsx` * `src/components/clinical-dashboard/output-panel.tsx` * `src/lib/answer-render-policy.ts` * `src/lib/rag-answer-text.ts` * `src/lib/rag.ts` * `src/lib/source-text-sanitizer.ts`
Summary
Testing
|
# Conflicts: # src/components/clinical-dashboard/answer-content.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71bd4c6cd0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af8d86117e
ℹ️ 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".
The compact-answer safety loop only rescued fragments that survived the `.map`/`.filter` usefulness gate, so a short caveat like "Contraindicated in pregnancy" (7 words, not flagged "useful") was dropped before it could be preserved. Make the map/filter safety-aware so safety-bearing fragments pass through untouched, and add regression tests for short and full-word escalation/contraindication caveats beyond the compact head. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eca2c84f1d
ℹ️ 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".
…it unformatted) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Skip whole-text clinicalProseUsefulness in primaryAnswerDisplayText so short safety sentences (Stop/Cease/held) survive to the fragment bypass - Add held to the safety signal matcher for passive withhold caveats - Strip catalogue/source artifacts on a de-bolded copy when preserveBold is set, then restore surviving emphasis spans - Restore the 20260712000000 migration stub so the Supabase preview branch matches local migration history (verify guard only; RPC bodies deferred) - Add regression tests for stop/held caveats and bolded catalogue stripping Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
@codex resolve all comments |
Summary
Testing
|
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Simplifying code... This may take up to 20 minutes. |
|
✅ Created PR with simplified code: #534 |
|
Ping-pong health check failed |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3016507b23
ℹ️ 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".
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ebd2bf717
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dac19c935b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99beaecdcb
ℹ️ 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".
- Add 'avoid' to the safety-signal matcher so short avoidance directives
('Avoid lithium in pregnancy.') are preserved through usefulness/length
filtering and the compact cap (Codex review).
- Test the safety matcher against a de-bolded copy of the fragment via a new
isPrimaryAnswerSafetyFragment helper, so server bold markers inside a phrase
('Do **not** administer', 'red **zone**') on the preserveBold path can no
longer defeat the match and let a caveat be dropped (Codex P1).
- Add regression tests for the avoid directive and the bolded-keyword caveat.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…l directives
Cover must not / should not / will not, discontinue, suspend, never, and 'not
recommended' so short contraindication-style caveats ('Clozapine should not be
used.') are preserved through usefulness/length filtering and the compact cap
(Codex P1). Kept deliberately broad — matching a non-safety fragment only
preserves it verbatim. Adds a should-not regression test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
RAG review pass — safe, offline-provable improvements to how answers are formatted/rendered, plus retrieval observability. No retrieval-ranking, scoring, cache-key, or answer-generation logic was changed.
preformatted. The server exempts deterministic document-support-list and table/visual answers from the prose sanitizer, but the client re-ran it unconditionally and deleted their document names / facility codes on display. The client now mirrors the server contract (answer.preformatted && answer.grounded) via a losslessnormalizePreformattedDisplayText, threaded through the display chain (display-text.ts,answer-content.tsx,ClinicalDashboard.tsx,answer-result-surface.tsx,output-panel.tsx,evidence-panels.tsx).<SafeBoldText>, butpolishClinicalAnswerProsestripped**first, so server emphasis (and the verified-vs-unverified number signal) never rendered. Added a display-onlypreserveBold(the shared server answer-gen path is unchanged) and stopped the copy/paste clinical draft from leaking literal**.match_document_chunks_text,match_document_table_facts_text,match_documents_for_query) swallowed RPC errors with no signal; they now recordhybrid_rpc_errorsbefore the unchangedreturn [](retrieval results byte-identical).retrieval_owner_matches— no live hole; the gap is committed-source reproducibility (fresh rebuilds/previews/DR get the stale fail-open bodies). Because live has diverged forward and this env has no Postgres DSN, the migration is a replay-safe, verified, owner-runnable byte-perfect codification spec (runnable fail-closed truth-table guard + exactpg_get_functiondefdump command). It also flags two non-primary functions still carrying a fail-open inline predicate (match_document_embedding_fields_text,get_related_document_metadata).Verification
typecheck,lint,format:check, fullvitest(1655 passed, 1 skipped),eval:rag:offline(59). (verify:pr-localconstituents run individually.)@criticaldemo answer-flow Playwright test green (exercises the modifiedplain-answer-responserender path).npm run eval:retrieval:quality— 36/36 (belt-and-suspenders; doc_recall@5=1, content_recall@5=1, hit@k=1, content_mrr@10=0.9255, force-embedding failures=0). Confirms F8 did not disturb retrieval.npm run eval:quality -- --rag-only --limit 8— 7/8 grounded (0.875). The one miss (discharge-documentation) is pre-existing and unrelated to this PR: it fails on grounding (route=extractive, model=none), produced entirely by retrieval-selection/grounding code this PR does not touch (therag.tsdiff is 3 additive telemetry lines that fire only on RPC error, of which there were none). ItsexpectedFilesis an intentional synthetic alias (eval-document-matching.ts), so it is not a stale-fixture issue. Tracked as a separate clinical-governance calibration follow-up.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy) — verified viacheck:supabase-project.Notes
🤖 Generated with Claude Code