From ade847b6ce75c96b6749cc279cc80b8e110feba7 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:28:12 +0800 Subject: [PATCH 1/2] fix: remediate 2026-07 repo audit findings across clinical answer, retrieval, and ingestion paths Full multi-agent audit (docs/audit/repo-audit-2026-07-01.md): 42 verified findings fixed or explicitly dispositioned, then hardened by an adversarial review of the fix diff itself (R1-R13). Highlights: - Answer path: numeric-verification corpus now covers retrieval synopses, image table text, and table-fact metadata snippets (H1); sanitizer no longer deletes clinical threshold sentences (H2); copied ward-note tables share the on-screen conservative normalization + low-confidence caveat, with no duplicated markdown header or ragged-row misalignment (H4/M8/M16, and AccessibleTable parity). - Retrieval: deriveConfidence scoped to cited chunks (M1); word-boundary intent signals with digit-attached dose support (M2/M3); retrieval-selection aligned verbatim with PR #118 relevance-first contract (H3/L4 superseded). - Data integrity: DELETE blocks pending reindex jobs (M9); deep-memory builds embeddings before deleting old memory and fails closed on generation-lookup errors (M11/L8); perceptual hash 16-bit -> 192-bit ph2 (M12); new migration 20260702000000 preserves legacy NULL-generation artifacts unless replaced, mirrored in the worker fallback (M13); chunking forward-progress guard and whole-line page-noise anchoring (M14/M17). - Privacy/security: optional HMAC-keyed query hashing via RAG_QUERY_HASH_SECRET (M15); non-string log fields serialized before redaction (L12); timing-safe edge-function secret compare and fail-fast 4xx (L20/L7); stale-search guard incl. progress updates in ClinicalDashboard (M10/R9). - Scripts/config: strict CLI arg validation (L1/L2/R13), classification fixes (M4-M7/R10), dead-code removal (L15-L17), CSP host dedup (L13). Verification: tsc clean; vitest 701 passed / 2 skipped (incl. ~13 new regression tests); Deno edge-function typecheck clean. Lint not run locally (eslint missing from worktree node_modules; pre-existing lockfile drift). Co-Authored-By: Claude Fable 5 --- .env.example | 7 + docs/audit/repo-audit-2026-07-01.md | 242 ++++++++++++++++++ next.config.ts | 2 +- scripts/purge-query-logs.ts | 6 +- scripts/recover-ingestion-queue.ts | 53 +++- src/app/api/documents/[id]/route.ts | 12 +- src/app/api/search/route.ts | 16 +- src/components/AccessibleTable.tsx | 14 +- src/components/ClinicalDashboard.tsx | 49 +++- src/lib/answer-ranking.ts | 16 +- src/lib/answer-verification.ts | 36 ++- src/lib/chunking.ts | 14 +- src/lib/clinical-search.ts | 21 +- src/lib/cross-document-synthesis.ts | 2 +- src/lib/deep-memory.ts | 88 ++++--- src/lib/document-index-units.ts | 8 +- src/lib/document-organization.ts | 37 ++- src/lib/document-tags.ts | 14 +- src/lib/env.ts | 5 + src/lib/evidence-relevance.ts | 11 +- src/lib/evidence.ts | 16 +- src/lib/image-filtering.ts | 26 +- src/lib/ingestion.ts | 11 +- src/lib/privacy.ts | 16 +- src/lib/query-privacy.ts | 17 +- src/lib/rag.ts | 43 +++- src/lib/retrieval-selection.ts | 30 +-- src/lib/source-text-sanitizer.ts | 55 +++- src/lib/validation/form-data.ts | 8 +- src/lib/visual-intelligence.ts | 12 +- src/lib/ward-output.ts | 46 +++- supabase/functions/indexing-v3-agent/index.ts | 37 ++- ...t_generation_preserve_legacy_artifacts.sql | 229 +++++++++++++++++ supabase/schema.sql | 98 ++++++- tests/answer-verification.test.ts | 39 +++ tests/chunking.test.ts | 20 ++ tests/clinical-search.test.ts | 10 + tests/deep-memory.test.ts | 8 +- tests/document-organization.test.ts | 24 ++ tests/image-filtering.test.ts | 5 +- tests/private-access-routes.test.ts | 49 ++-- tests/retrieval-selection.test.ts | 49 +++- tests/source-text-sanitizer.test.ts | 24 ++ tests/ward-output.test.ts | 126 +++++++++ worker/main.ts | 25 ++ 45 files changed, 1465 insertions(+), 211 deletions(-) create mode 100644 docs/audit/repo-audit-2026-07-01.md create mode 100644 supabase/migrations/20260702000000_commit_generation_preserve_legacy_artifacts.sql diff --git a/.env.example b/.env.example index 57d7b06994..407c665438 100644 --- a/.env.example +++ b/.env.example @@ -47,6 +47,13 @@ RAG_SEARCH_CACHE_TTL_MS=60000 RAG_SEARCH_CACHE_SIZE=200 RAG_AWAIT_QUERY_LOGS=false +# Server-side key for the redacted query-hash placeholder (min 16 chars). +# When set, stored query hashes are HMAC-SHA256 keyed pseudonyms — not +# offline-reversible and not correlatable outside this deployment. Strongly +# recommended wherever real clinical queries are logged. Changing or setting +# the key changes future hashes, so historical dedup/joins reset from then on. +#RAG_QUERY_HASH_SECRET= + # Private buckets created by supabase/schema.sql. SUPABASE_DOCUMENT_BUCKET=clinical-documents SUPABASE_IMAGE_BUCKET=clinical-images diff --git a/docs/audit/repo-audit-2026-07-01.md b/docs/audit/repo-audit-2026-07-01.md new file mode 100644 index 0000000000..ef6fd02a46 --- /dev/null +++ b/docs/audit/repo-audit-2026-07-01.md @@ -0,0 +1,242 @@ +# Repository Audit — Clinical KB Database + +**Date:** 2026-07-01 +**Branch:** `claude/cool-wiles-12aade` (worktree) +**Method:** Multi-agent audit — 19 review lanes (dimension × subsystem) using the `code-review` method, each finding adversarially verified by an independent skeptic, deduped, then a completeness-critic gap-fill round. 80 agents, ~5M tokens. +**Scope:** Risk-first, full coverage. Deep line-level review of RAG, ingestion, search/retrieval, privacy/auth, source governance, Supabase/DB, worker, and API surface; lighter sweep of UI, scripts, tests/config/deps. + +## Result summary + +| | Count | +|---|---| +| **Total findings (post-verification)** | **41** | +| CONFIRMED | 37 | +| PLAUSIBLE | 4 | +| By severity | High **4**, Medium **17**, Low **20** | +| By category | correctness 17 · data-integrity 8 · privacy/clinical 6 · quality/simplification 6 · performance 2 · security 2 | +| Flagged safe-cleanup by finders | 17 (only 2 are *truly* no-behavior-change — see Auto-fixes) | + +Finders raised more; the numbers above are what **survived adversarial verification**. Verifiers also corrected several severities/categories (e.g. the perceptual-hash and edge-function-secret findings were downgraded from High; the sanitizer finding was re-categorised to data-integrity). + +The dominant theme is **clinical-safety / data-integrity in the answer path**: several places where a correct clinical number, threshold, or freshness signal is silently dropped, mis-attributed, or has its "unreliable — verify against source" caveat stripped before it reaches the clinician. These outrank the two (defence-in-depth) security findings. + +--- + +## High severity (4) — clinical answer correctness + +### H1 · Numeric faithfulness gate blanks correct answers whose numbers live in synopsis/image text +`src/lib/answer-verification.ts:90` · correctness · CONFIRMED · safe_cleanup=false +- **Defect:** `sourceTextForResult()` builds the numeric-verification corpus from `content / adjacent_context / section_heading / table_facts / memory_cards / index_unit` but omits `result.retrieval_synopsis` and image text (`tableTextSnippet`, `accessibleTableMarkdown`) — even though `buildRagSourceBlock` (`rag.ts:5610-5647`) *shows both* to the model. +- **Failure:** For a `medication_dose_risk`/`table_threshold` query whose evidence is a visual/table chunk, the real dose/threshold (e.g. `12.5 mg`, `ANC 2.0 x10^9/L`) exists only in `retrieval_synopsis`/image text. The model copies it faithfully, but `verifyAnswerNumbers` marks it unverified; `applyNumericVerification` (`rag.ts:5798-5808`) then discards the whole correct answer and returns a generic "review the source passages" non-answer. +- **Fix:** Include `retrieval_synopsis` + image text in `sourceTextForResult`, mirroring `sourceTextForQuoteVerification` (`rag.ts:880-897`), so the verifier scans the same corpus the model was given. + +### H2 · Sanitizer drops clinical threshold sentences misclassified as source-title fragments +`src/lib/source-text-sanitizer.ts:230` (pattern at `:29-30`) · data-integrity · CONFIRMED · safe_cleanup=false +- **Defect:** `clinicalProseUsefulness()` discards any sentence starting near a title keyword (`Guideline/Procedure/Protocol/Policy/Appendix/Scale`) that lacks a concrete-action verb — the `sourceTitleFragmentPattern` regex greedily consumes up to 180 trailing chars (`…Scale\b[^.;]{0,180}`). +- **Failure (reproduced by the agent running the real function):** input *"Assess the patient on admission. The Glasgow Coma Scale ranges from 3 to 15 with 8 or below indicating severe head injury. Document the score."* → returns *"Assess the patient on admission. Document the score."* The GCS threshold sentence is silently removed and the truncated text still wins at `rag-answer-text.ts:150` (`usefulness.text || finalText`). +- **Fix:** Don't drop a fragment on title/noise grounds when it contains a clinical numeric/threshold token; anchor `sourceTitleFragmentPattern` so it can't swallow trailing clinical prose. + +### H3 · Freshness/validation penalties are silently discarded from the final score +`src/lib/retrieval-selection.ts:473` · privacy/clinical · CONFIRMED · safe_cleanup=false +- **Defect:** `annotateResultWithSelection` writes back `Math.max(originalScore, candidate.score)` as `hybrid_score`, so any net-negative `resultBoost` (outdated −0.24, review_due −0.12, unverified −0.08, poor-extraction −0.12) never lowers the score consumers see — the floor is meant to let intent "rescue" *raise* a score, but it also blocks all penalties. +- **Failure:** An `outdated` chunk with base 0.70 → ~0.46 after penalties gets floored back to 0.70. `evaluateEvidenceCoverageGate` (`rag.ts:3233`, gate ~0.6) then treats the stale guideline as strong and presents it with high confidence instead of demoting it. +- **Fix:** Don't floor at the original when `resultBoost` is negative — persist the clamped boosted score so freshness/validation penalties reach coverage gating. + +### H4 · Ward-note / clipboard table export bypasses the low-confidence safeguard +`src/lib/ward-output.ts:601` · privacy/clinical · CONFIRMED · safe_cleanup=false +- **Defect:** `clinicalTableToTextRows` renders threshold tables straight from raw card rows (`parseMarkdownTable`) and never calls `normalizeAccessibleTable`, so the on-screen "Table structure could not be confidently reconstructed — verify values against the source document" caveat (`AccessibleTable` / `accessible-table-normalization.ts`) is dropped from the copied ward note. +- **Failure:** A clozapine monitoring table flagged `lowConfidence` on screen is copied via `formatWardNote`/`formatAnswerForClipboard` with no caveat; a clinician pastes a mis-paired dose grid into the record and trusts it. +- **Fix:** Route ward-output tables through `normalizeAccessibleTable` (conservativeClinical) and emit the same caveat line when `lowConfidence`. + +--- + +## Medium severity (17) + +**Answer/retrieval correctness** +- **M1** `src/lib/rag.ts:647` · `deriveConfidence` takes `max(similarity)` over **all** retrieved chunks, not the cited ones — an uncited high-similarity chunk inflates a weakly-cited answer to high confidence. CONFIRMED. +- **M2** `src/lib/clinical-search.ts:760` · `classifyQueryIntent` uses `containsAny` (substring, not word-boundary) on short tokens (`im`, `po`, `table`, `flow`) → `"time limit"` trips dosing, `"notable"` trips image focus, perturbing ranking (+0.09 dosing / −0.04 image). CONFIRMED. +- **M3** `src/lib/clinical-search.ts:768` · escalation substring match (`review`, `risk`, `rapid`) cancels a legitimate dosing signal — `"clozapine dose review schedule"` loses its dosing boost. CONFIRMED. +- **M4** `src/lib/document-index-units.ts:290` · `fallbackVisualUnitType` tests the flowchart regex (matches bare `yes`/`no`) before the table branch → sparse tables containing "No"/"Yes" are typed `flowchart_step`, degrading typed retrieval. CONFIRMED. +- **M5** `src/lib/document-organization.ts:1337` · reference-collection fallback guarded on `candidates.length === 0`, but an unconfirmed 0.58 tag candidate stays in `candidates` while being dropped from `confirmedCandidates` → doc mis-classified `site=null/needs_review`. CONFIRMED. +- **M6** `src/lib/document-organization.ts:1242` · `evidenceText` omits `title`/`file_name`, so a site named only in the title (e.g. "Sir Charles Gairdner Hospital …") is never detected. CONFIRMED. +- **M7** `src/lib/document-organization.ts:262` · BMJ reference_collection matches the ubiquitous phrase `best practice` (OR'd with `bmj`) → any doc saying "best practice" is falsely attributed to "BMJ Best Practice" @0.92 — a false provenance claim in a governance context. CONFIRMED. +- **M8** `src/lib/ward-output.ts:606` · when `rows` is null but `columns`+`markdown` present, the parsed markdown **header row is re-emitted as the first data row** (no `slice(1)`). CONFIRMED. + +**Data integrity** +- **M9** `src/app/api/documents/[id]/route.ts:503` · DELETE active-job guard checks only `status='processing'`, missing `pending`; a just-queued reindex racing a delete orphans freshly-uploaded storage objects. Reindex routes correctly use `checkIngestionMutationSafety` (`in ['pending','processing']`). CONFIRMED. +- **M10** `src/components/ClinicalDashboard.tsx:6533` · `executeSearch`/`applySearchResult` have no request-token/abort guard → out-of-order responses can show query A's answer under query B's question (bootstrap auto-search + user submit). CONFIRMED. +- **M11** `src/lib/deep-memory.ts:644` · `upsertDocumentDeepMemory` deletes memory cards/sections/index-units **non-atomically** before rebuild; an OpenAI/insert failure after the deletes wipes memory while metadata still advertises the old version → silent retrieval degradation until full reindex. CONFIRMED. +- **M12** `src/lib/image-filtering.ts:349` · 16-bit (4-hex) `lightweightPerceptualHash` collapses distinct same-dimension clinical images into one visual family → one of two different threshold tables silently dropped from the index. CONFIRMED (downgraded from High). +- **M13** `supabase/migrations/20260628000000_atomic_reindex_generation_commit.sql:120` · `commit_document_index_generation` deletes NULL-generation (legacy) artifact rows, which retrieval treats as committed/visible; a transient artifact-write failure that still commits `status='indexed'` permanently loses the previously-good legacy artifacts. CONFIRMED. + +**Privacy / clinical** +- **M14** `src/lib/chunking.ts:12` · `lineNoisePatterns[0]` matches unanchored `p N`/`page N` anywhere in a line and `removePageNoise` drops the whole line → clinical lines like "refer to p 3 for dosing" are deleted before indexing. CONFIRMED. +- **M15** `src/lib/query-privacy.ts:8` · redacted mode stores an **unsalted SHA-256** of the normalized query as the placeholder and `query_hash`; short low-entropy clinical queries are dictionary-reversible and cross-row correlatable. Fix: HMAC with a server-side secret. CONFIRMED. +- **M16** `src/lib/ward-output.ts:611` · header/body cell-count mismatch — a ragged row with more cells than columns emits misaligned markdown, pairing the wrong threshold with the wrong action in the copied note. CONFIRMED. + +**Correctness (added 2026-07-02 — omitted from the first draft of this report; the workflow produced 17 Medium findings but only 16 were listed)** +- **M17** `src/lib/chunking.ts:320` · `chunkTextBySentence` makes no forward progress when `CHUNK_OVERLAP >= CHUNK_SIZE` (both pass env validation independently): `readableOverlapStart` returns a start ≤ the previous start and the `while` loop spins forever, hanging the ingestion worker on that document. CONFIRMED (reproduced by trace). + +--- + +## Low severity (20) + +**Correctness** +- **L1** `scripts/purge-query-logs.ts:25` · unknown flag silently consumes the next arg (typo'd `--owner-emial` swallows the email, then purges the env-configured owner's logs). CONFIRMED. +- **L2** `scripts/recover-ingestion-queue.ts:49` · typo'd `--limit` → NaN → silently defaults to 20 (mutates up to 20 jobs instead of the intended cap). CONFIRMED. +- **L3** `src/lib/evidence-relevance.ts:347` · aggregate `nearby` branch uses `|| results.length > 0` (always true) → the `none` verdict is dead; zero-term-match sets are labelled `nearby`, overstating evidence in governance/telemetry. CONFIRMED. +- **L4** `src/lib/retrieval-selection.ts:463` · boosted score clamped low (`Math.max(0, …)`) but not to an upper bound of 1 → `hybrid_score` can exceed 1.0 into telemetry/consumers. CONFIRMED. +- **L5** `src/lib/validation/form-data.ts:9` · no-op ternary `typeof value === "string" ? value : value` — a File `title`/`description` part isn't coerced to null and makes the whole `/api/upload` 400. CONFIRMED. +- **L6** `src/lib/visual-intelligence.ts:401` · deterministic fallback keeps only a hardcoded drug allowlist (clozapine|lithium|…), silently discarding e.g. quetiapine/sertraline extracted by the clinical vocabulary. CONFIRMED. +- **L7** `supabase/functions/indexing-v3-agent/index.ts:437` · non-retryable OpenAI 4xx (bad model / revoked key) is retried `OPENAI_MAX_RETRIES` times instead of failing fast. CONFIRMED. + +**Data integrity** +- **L8** `src/lib/deep-memory.ts:865` · committed-generation filter fails open (`if (documentsError) return true`) — a transient documents-table error during a staged reindex can expose superseded-generation cards. PLAUSIBLE (narrow race). +- **L9** `worker/main.ts:1406` · `imageCount` (→ `p_image_count`) counts searchable-only rows, excluding audit-retained images that were still inserted; reconciliation tooling may see a mismatch. PLAUSIBLE (likely intentional). + +**Performance** +- **L10** `src/app/api/search/route.ts:666` · `buildEvidenceRelevance` runs ≥2× and `buildVisualEvidence` ≥3× per search over the full result set, and the smart-panel relevance is then overwritten — wasted CPU on the hot path. CONFIRMED. +- **L11** `worker/main.ts:997` · each image is `readFile`'d up to 3× per ingestion (hash, caption cache-miss, upload) → 3× disk I/O + peak memory. CONFIRMED. + +**Privacy / clinical** +- **L12** `src/lib/privacy.ts:6` · `redactLogValue` returns non-string values unchanged, so an error whose `details`/`code`/`hint` is an object is logged verbatim (potential URL/email/PHI leak). CONFIRMED. + +**Quality / simplification** +- **L13** `next.config.ts:31` · CSP `connect-src` lists both the explicit Supabase host and `https://*.supabase.co` (redundant). CONFIRMED. → **auto-fixed** (see below). +- **L14** `src/lib/answer-ranking.ts:~324` (reported as `answer-formatting.ts:327`) · `capBoldSegments` lets newly-bolded tokens that collide with pre-existing bold text bypass the max-segment cap. CONFIRMED. +- **L15** `src/lib/cross-document-synthesis.ts:195` · dead ternary `queryClass === "comparison" ? 2 : 2`. CONFIRMED. → **auto-fixed**. +- **L16** `src/lib/document-tags.ts:264` · dead `return false` special-cases (default is already `return false`) — behaviorally dead, misleads about protection a future rule would bypass. CONFIRMED. (left as recommendation — encodes intent) +- **L17** `src/lib/ingestion.ts:7` · unreachable page-number-duplicate retry clause (`isPartialIndexWriteConflict` short-circuits first). CONFIRMED. (left as recommendation — needs an intent decision) +- **L18** `supabase/migrations/20260630090000_audit_logs_service_role_policy.sql:10` · re-declares the identical `audit logs service role all` policy already created in `20260629110000_audit_logs.sql` — drift risk on replay. CONFIRMED. (do **not** edit an applied migration; recommendation only) + +**Security (defence-in-depth)** +- **L19** `next.config.ts:7` · production CSP allows `script-src 'unsafe-inline'` with no nonce/hash. PLAUSIBLE — factually true, but the app has **zero** `dangerouslySetInnerHTML` sinks (React auto-escapes doc text + LLM answers), so no active XSS path today. Recommend nonce-based CSP as hardening. +- **L20** `supabase/functions/indexing-v3-agent/index.ts:186` · agent secret compared with non-constant-time `!==` on a `verify_jwt=false` function. PLAUSIBLE — timing weakness is real, remote byte-by-byte recovery over TLS jitter is not realistically demonstrable. Recommend a timing-safe compare. + +--- + +## Coverage + +19 lanes reviewed; the completeness critic surfaced 6 gaps, all filled: + +| Area | Lanes | +|---|---| +| Deep (risk-bearing) | rag-engine, rag-routing, search-retrieval, clinical-search, ingestion-core, index-units, worker, enrichment, visual, privacy-auth, governance-safety, supabase-db, openai, api-surface, deep-memory-polish, security-sweep | +| Light sweep | ui-components, scripts-cli, tests-config-deps | +| Gap-fill (critic) | clinical-output-rendering, document-mutation-reindex-correctness, deep-memory-logic, database-migrations-rls-rpc, request-primitives-rate-limit | + +Clean lanes (no surviving findings): `supabase-db` client setup (no service-role-key leakage to the browser bundle found). The gap round is where H4, M8, M9, M11, M13, M16, L5, L8, L18 were caught — i.e. clinical-output rendering and mutation-race correctness were under-covered by the first pass. + +## Corroborated pre-existing debts + +- **eslint↔lockfile drift** (lockfile pins eslint 10.4.1 vs `eslint-config-next` needing eslint 9) — confirmed in `docs/redesign/04-deferred.md`; risks a clean-`npm ci` CI lint break. **Not** auto-fixed (dependency/lockfile change needs your sign-off). +- Embedding-dimension/schema sync has no runtime guard (relates to M-class ingestion risks). +- RLS multi-user integration test (RET-H4) still missing; HNSW `ef_search` migration remains a no-op. + +--- + +## Remediation — 2026-07-02 (all findings actioned) + +Following user approval, every finding was reviewed and fixed (or explicitly dispositioned) on the +working tree of `claude/cool-wiles-12aade` (uncommitted, nothing pushed). 42 files changed +(+844/−156), including 14 new regression tests. + +### Fixed (behavioral) + +| ID | File | Fix | +|---|---|---| +| H1 | `src/lib/answer-verification.ts` | verification corpus now includes `retrieval_synopsis` + image table text (mirrors `sourceTextForQuoteVerification`); 2 regression tests | +| H2 | `src/lib/source-text-sanitizer.ts` | new `clinicalThresholdSignalPattern` rescue — threshold-bearing fragments are never dropped by title/noise heuristics; GCS regression test + bare-integer noise still dropped | +| H3+L4 | `src/lib/retrieval-selection.ts` | **SUPERSEDED by PR #118 on main** (merged 2026-07-02, after this audit's base commit): the golden retrieval eval measured that source-governance metadata weighting in selection buries correct documents on the partially-enriched corpus (doc-recall@5 1.0→0.76), so #118 **removed the penalties entirely** and clamped the candidate score — resolving both H3's premise and L4. This branch's `retrieval-selection.ts` + its contract test are now aligned **verbatim** with origin/main's #118 version (any deviation requires re-running `npm run eval:retrieval:quality`, 23/23 required). | +| H4+M8+M16 | `src/lib/ward-output.ts` | copied tables routed through `normalizeAccessibleTable` (conservativeClinical) with the on-screen low-confidence caveat; markdown header no longer duplicated as a data row; ragged rows padded/merged; 3 regression tests | +| M1 | `src/lib/rag.ts` | `deriveConfidence` takes the citation array and scopes strongest-similarity to cited chunks (4 call sites) | +| M2+M3 | `src/lib/clinical-search.ts` | word-boundary signal matching (short acronyms whole-word); explicit dose terms survive escalation cancellation; 5 regression assertions | +| M4 | `src/lib/document-index-units.ts` | bare `yes`/`no` only counts as flowchart evidence when the image is not structurally a table | +| M5+M6+M7 | `src/lib/document-organization.ts` | fallbacks gate on confirmed candidates; title/file_name in the evidence haystack (bracket segments stripped so `(FSH)` tags still can't self-confirm — pinned by an existing test); BMJ requires the `bmj` token; 2 regression tests | +| M9 | `src/app/api/documents/[id]/route.ts` | DELETE blocks on `pending` OR `processing` jobs (parameterized regression test) | +| M10 | `src/components/ClinicalDashboard.tsx` | request-id ref guard — only the latest search commits answer/error/loading state | +| M11+L8 | `src/lib/deep-memory.ts` | all embeddings computed BEFORE deleting old memory (failure window no longer spans OpenAI); fallback committed-generation filter fails closed on lookup error (test updated to pin fail-closed) | +| M12 | `src/lib/image-filtering.ts` | perceptual hash upgraded ph1 (16-bit) → ph2 (192-bit; threshold + quantized-level bits); version bump prevents legacy aliasing | +| M13 | `supabase/migrations/20260702000000_…` + `schema.sql` | **new** migration: `commit_document_index_generation` purges legacy NULL-generation rows only when replacement rows exist in the same table. **Not yet applied to the live DB** — apply via the normal migration flow | +| M14 | `src/lib/chunking.ts` | page-noise pattern anchored to the whole line; inline "p 3" references survive (regression test) | +| M15 | `src/lib/query-privacy.ts`, `env.ts`, `.env.example` | HMAC-SHA256 keyed hash when `RAG_QUERY_HASH_SECRET` (new, optional, min 16 chars) is set; legacy digest kept when unset for continuity — **set the secret in any environment logging real queries** | +| M17 | `src/lib/chunking.ts` | strict forward-progress guard in `chunkTextBySentence` (regression test at overlap == chunkSize) | +| L1/L2 | `scripts/purge-query-logs.ts`, `scripts/recover-ingestion-queue.ts` | unknown flags and malformed numeric flags now throw instead of silently misbehaving | +| L3 | `src/lib/evidence-relevance.ts` | aggregate `none` verdict reachable — nearby requires matched terms | +| L6 | `src/lib/visual-intelligence.ts` | fallback medications derived from the vocabulary's medication category, not a hardcoded allowlist | +| L7+L20 | `supabase/functions/indexing-v3-agent/index.ts` | non-retryable 4xx fails fast; timing-safe (hashed) secret comparison; Deno typecheck passes | +| L10 | `src/app/api/search/route.ts`, `src/lib/evidence.ts` | relevance/visual evidence computed once per request and shared with the smart panel (both call sites) | +| L12 | `src/lib/privacy.ts` | non-string error fields serialized and redacted instead of passing through verbatim | +| L14 | `src/lib/answer-ranking.ts` | bold cap counts per-occurrence free passes; new tokens can't tunnel past the cap | +| L16/L17 | `src/lib/document-tags.ts`, `src/lib/ingestion.ts` | dead/misleading branches removed with intent documented | + +### Dispositioned without behavior change (with rationale) + +| ID | Disposition | +|---|---| +| L5 | The audit's recommended fix (coerce File→null) was **wrong**: `tests/api-validation-contract.test.ts` pins that non-string multipart metadata must 400 before any storage/DB write. The no-op ternary was replaced by a pass-through with the contract documented. | +| L9 | `image_count` = searchable-only is intentional (retrieval filters `searchable=true`); the verifier found no consumer reconciling against it. No change. | +| L11 | Triple `readFile` is a deliberate peak-memory trade-off (holding hundreds of multi-MB buffers is worse than re-reading); documented at the site. | +| L18 | Both audit_logs policy migrations are already applied — editing applied migrations creates replay drift. Consolidate only if migrations are ever squashed. | +| L19 | CSP `unsafe-inline` hardening (nonce-based CSP) deferred: no XSS sink exists today (zero `dangerouslySetInnerHTML`), and a nonce migration needs dedicated UI verification. Tracked as accepted risk. | +| L13/L15 | Applied earlier as the safe auto-fix pass (CSP host dedup; dead ternary). | + +### Adversarial diff-review round (2026-07-02) + +Five parallel review agents then audited the fix diff itself. They confirmed the bulk of the +changes sound and surfaced **12 follow-up findings — all fixed** in the same working tree: + +| # | Finding (introduced/incomplete) | Resolution | +|---|---|---| +| R1 | **HIGH** — H3's un-floored penalties **compounded** across the pipeline's 2–3 selection passes (`baseScore` re-read the penalized `hybrid_score`) | resolved by discarding the H3 change and aligning `retrieval-selection.ts` verbatim with PR #118 (relevance-first, no metadata penalties — see H3 row). Residual note: POSITIVE intent boosts still compound mildly across passes on main's design (pre-existing, eval-validated); a `preSelectionScore` idempotency fix was prototyped and withdrawn — reintroduce only gated on the golden eval. | +| R2 | M2's whole-word `mg`/`mcg` no longer matched digit-attached doses ("10mg"), and could re-cancel dosing via M3 | `(?:\b|\d)` digit-adjacent allowance (regression test "clozapine 100mg…") | +| R3 | L3's aggregate `none` could contradict per-source "nearby" chips for purely-semantic matches and trip the danger governance banner | aggregate `nearby` also granted when relevance score ≥ 0.5 | +| R4 | H1 incomplete — rich-mode prompts show table-fact **metadata** snippets (`accessible_table_markdown`/`table_text_snippet`/`cells`) not covered by the corpus | included in both `tableFactText` (verification) and `tableFactQuoteText` (quotes) | +| R5 | H2's rescued noisy fragments inflated `provenanceScore` past 0.42, flipping `useful` false and blanking text that previously survived | provenance computed over baseline-kept fragments only; thresholds count toward `clinicalSignalScore` | +| R6 | `x10` threshold branch matched "Rx100"/"0x10" | lookbehind guard `(? { - const inline = argv.find((arg) => arg.startsWith(`--${name}=`))?.split("=")[1]; - if (inline) return inline; - const index = argv.indexOf(`--${name}`); - return index >= 0 ? argv[index + 1] : undefined; + const values = new Map(); + const booleans = new Set(); + for (let index = 0; index < argv.length; index += 1) { + const token = argv[index]; + if (booleanFlags.has(token)) { + booleans.add(token); + continue; + } + const equalsIndex = token.indexOf("="); + const name = equalsIndex >= 0 ? token.slice(0, equalsIndex) : token; + if (!valueFlags.has(name)) throw new Error(`Unknown argument ${token}`); + const value = equalsIndex >= 0 ? token.slice(equalsIndex + 1) : argv[index + 1]; + if (equalsIndex < 0) index += 1; + if (!value || value.startsWith("--")) throw new Error(`Missing value for ${name}`); + values.set(name, value); + } + const positiveIntFor = (name: string) => { + const raw = values.get(`--${name}`); + if (raw === undefined) return undefined; + const parsed = Number.parseInt(raw, 10); + if (!Number.isInteger(parsed) || parsed <= 0 || String(parsed) !== raw.trim()) { + throw new Error(`--${name} must be a positive integer (received "${raw}").`); + } + return parsed; }; return { - apply: argv.includes("--apply"), - yes: argv.includes("--yes"), - staleAfterMinutes: Number.parseInt(valueFor("stale-after-minutes") ?? "", 10), - limit: Number.parseInt(valueFor("limit") ?? "", 10), + apply: booleans.has("--apply"), + yes: booleans.has("--yes"), + staleAfterMinutes: positiveIntFor("stale-after-minutes"), + limit: positiveIntFor("limit"), }; } @@ -64,10 +93,8 @@ async function main() { ]); requireServerEnv(); const args = parseArgs(process.argv.slice(2)); - const staleAfterMinutes = Number.isFinite(args.staleAfterMinutes) - ? args.staleAfterMinutes - : env.WORKER_STALE_AFTER_MINUTES; - const limit = Number.isFinite(args.limit) ? args.limit : 20; + const staleAfterMinutes = args.staleAfterMinutes ?? env.WORKER_STALE_AFTER_MINUTES; + const limit = args.limit ?? 20; const supabase = createAdminClient(); console.log("=== Ingestion Queue Recovery ==="); diff --git a/src/app/api/documents/[id]/route.ts b/src/app/api/documents/[id]/route.ts index 4baae76398..9f68f1cb0e 100644 --- a/src/app/api/documents/[id]/route.ts +++ b/src/app/api/documents/[id]/route.ts @@ -496,16 +496,24 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i if (documentError) throw new Error(documentError.message); if (!document) return NextResponse.json({ error: "Document not found." }, { status: 404 }); + // Audit M9: block deletion on PENDING jobs too, matching the reindex + // routes' checkIngestionMutationSafety predicate. A just-queued reindex + // job (status "pending") racing this DELETE let the worker upload a new + // generation of image objects after the storage paths were enumerated, + // orphaning them permanently. const { data: activeJobs, error: activeJobsError } = await supabase .from("ingestion_jobs") .select("id,status") .eq("document_id", id) - .eq("status", "processing") + .in("status", ["pending", "processing"]) .limit(1); if (activeJobsError) throw new Error(activeJobsError.message); if ((activeJobs ?? []).length > 0) { - throw new PublicApiError("Document is currently indexing. Stop or wait for the worker before deleting.", 409); + throw new PublicApiError( + "Document has pending or processing indexing work. Stop or wait for the worker before deleting.", + 409, + ); } const [images, chunks] = await Promise.all([ diff --git a/src/app/api/search/route.ts b/src/app/api/search/route.ts index 58c016dc96..1323ae99eb 100644 --- a/src/app/api/search/route.ts +++ b/src/app/api/search/route.ts @@ -663,8 +663,12 @@ async function buildScopedSearchPayload( limit: body.mode === "documents" ? body.documentLimit : undefined, }) : []; - const smartPanel = buildSmartPanel(searchFocusQuery, results); + // Audit L10: compute relevance/visual evidence ONCE and share with the + // smart panel — the panel's own recomputation was discarded by the spread + // at payload build time anyway. const relevance = buildEvidenceRelevance(searchFocusQuery, results); + const visualEvidence = buildVisualEvidence(results); + const smartPanel = buildSmartPanel(searchFocusQuery, results, { relevance, visualEvidence }); const documentMatches = body.mode === "documents" ? annotateDocumentMatches(searchFocusQuery, relatedDocuments.map(toDocumentMatch), results) @@ -691,7 +695,7 @@ async function buildScopedSearchPayload( const payload = { results: compactSearchResults(searchFocusQuery, results), facets: buildSearchFacets(results), - visualEvidence: buildVisualEvidence(results), + visualEvidence, relevance, relatedDocuments: relatedDocuments.map((document) => ({ document_id: document.document_id, @@ -813,12 +817,16 @@ export async function POST(request: Request) { results, ) : []; + const cachedVisualEvidence = buildVisualEvidence(results); return NextResponse.json({ results: compactSearchResults(searchFocusQuery, results), facets: buildSearchFacets(results), - visualEvidence: buildVisualEvidence(results), + visualEvidence: cachedVisualEvidence, relevance, - smartPanel: { ...buildSmartPanel(searchFocusQuery, results), relevance }, + smartPanel: { + ...buildSmartPanel(searchFocusQuery, results, { relevance, visualEvidence: cachedVisualEvidence }), + relevance, + }, smartApiPlan: buildSmartRagApiPlan({ query: searchFocusQuery, queryClass, diff --git a/src/components/AccessibleTable.tsx b/src/components/AccessibleTable.tsx index ea7e9a671a..c80334b376 100644 --- a/src/components/AccessibleTable.tsx +++ b/src/components/AccessibleTable.tsx @@ -333,13 +333,21 @@ export function AccessibleTable({ const restoreFocusRef = useRef(null); const [open, setOpen] = useState(false); const canExpand = useMobileTableExpansion(expandOnMobile); - const parsed = rows?.length ? rows : parseMarkdownTable(markdown); + const hasExplicitRows = Boolean(rows?.length); + const parsed = hasExplicitRows ? rows : parseMarkdownTable(markdown); const normalized = useMemo(() => { if (!parsed?.length) return null; - const table = normalizeAccessibleTable(parsed, columns); + // Audit M8/H4 parity (diff review): markdown-parsed rows include their + // own header line as row 0 — passing explicit columns alongside them made + // the markdown header render as the first DATA row on screen, and let the + // on-screen and copied-ward-note normalizations disagree (different + // headers, potentially different lowConfidence caveats). Columns are the + // header only for explicit row arrays, matching clinicalTableToTextRows + // in ward-output.ts. + const table = normalizeAccessibleTable(parsed, hasExplicitRows ? columns : null); if (!table) return null; return clinicalOnly ? clinicalOnlyTable(table) : table; - }, [clinicalOnly, columns, parsed]); + }, [clinicalOnly, columns, hasExplicitRows, parsed]); const dialogOpen = open && canExpand; diff --git a/src/components/ClinicalDashboard.tsx b/src/components/ClinicalDashboard.tsx index 498b610d4c..65277af0e1 100644 --- a/src/components/ClinicalDashboard.tsx +++ b/src/components/ClinicalDashboard.tsx @@ -6357,6 +6357,7 @@ export function ClinicalDashboard({ queryText: string, filtersOverride: SearchScopeFilters = scopeFilters, queryModeOverride: ClinicalQueryMode = requestQueryMode, + onProgress: (message: string) => void = setAnswerProgress, ) { let response: Response; try { @@ -6387,7 +6388,7 @@ export function ClinicalDashboard({ throw makeSearchError(message, response.status, isRetryableStatus(response.status)); } - const payload = await readAnswerStream(response, setAnswerProgress); + const payload = await readAnswerStream(response, onProgress); return { kind: "answer" as const, query: queryText, @@ -6395,7 +6396,10 @@ export function ClinicalDashboard({ }; } - async function runWithRetries(operation: () => Promise) { + async function runWithRetries( + operation: () => Promise, + onProgress: (message: string) => void = setAnswerProgress, + ) { let lastError: unknown; for (let attempt = 0; attempt <= searchRetryCount; attempt += 1) { try { @@ -6405,7 +6409,7 @@ export function ClinicalDashboard({ if (!isRetryableError(error) || attempt >= searchRetryCount) break; const message = progressForRetry(attempt + 1); - setAnswerProgress(message); + onProgress(message); await sleep(searchRetryDelaysMs[attempt] ?? searchRetryDelaysMs[searchRetryDelaysMs.length - 1]); } } @@ -6419,6 +6423,13 @@ export function ClinicalDashboard({ return answerPayloadIsUsable(payload.payload); } + // Audit M10: monotonically increasing token identifying the latest search. + // Concurrent searches (URL-bootstrap auto-search racing a user submit) can + // resolve out of order; only the latest request may commit answer/sources/ + // error/loading state, or a stale response would display one query's answer + // under another query's composer text. + const searchRequestSeqRef = useRef(0); + function applySearchResult(payload: SearchResultModePayload) { if (payload.kind === "documents") { setDocumentMatches(payload.documentMatches); @@ -6474,6 +6485,14 @@ export function ClinicalDashboard({ setError("Search setup not ready."); return; } + const requestId = ++searchRequestSeqRef.current; + // M10 (diff-review hardening): progress updates emitted by this request's + // in-flight machinery (retry messages, keyword fallback, stream progress) + // must also be discarded once a newer search takes over, or a slow stale + // request repaints the progress banner under the newer query. + const onProgress = (message: string | null) => { + if (requestId === searchRequestSeqRef.current) setAnswerProgress(message); + }; setLoading(true); setError(null); setSearchRelevance(null); @@ -6481,7 +6500,7 @@ export function ClinicalDashboard({ setSearchScope(null); setSourceGovernanceWarnings([]); setAnswerViewMode("high_yield"); - setAnswerProgress(modeSearch.progressLabel); + onProgress(modeSearch.progressLabel); rememberRecentQuery(trimmedQuery); const fallbackQuery = keywordQueryFromNaturalLanguage(trimmedQuery); @@ -6498,13 +6517,16 @@ export function ClinicalDashboard({ let lastError: SearchError | null = null; for (const entry of queryPlan) { - if (entry.isKeyword) setAnswerProgress("Trying keyword-based search..."); + if (entry.isKeyword) onProgress("Trying keyword-based search..."); try { const payload = modeSearch.kind === "documents" - ? await runWithRetries(() => requestDocuments(entry.query, filtersOverride, targetQueryMode)) - : await runWithRetries(() => requestAnswer(entry.query, filtersOverride, targetQueryMode)); + ? await runWithRetries(() => requestDocuments(entry.query, filtersOverride, targetQueryMode), onProgress) + : await runWithRetries( + () => requestAnswer(entry.query, filtersOverride, targetQueryMode, onProgress), + onProgress, + ); if (!resultUsable(payload)) { lastError = makeSearchError("No usable results were found.", 404, false); @@ -6530,12 +6552,17 @@ export function ClinicalDashboard({ throw new Error("Search did not return usable results."); } - applySearchResult(successfulPayload); + // M10: discard a stale response — a newer search owns the UI state. + if (requestId === searchRequestSeqRef.current) applySearchResult(successfulPayload); } catch (requestError) { - setError(requestError instanceof Error ? requestError.message : "Search failed"); + if (requestId === searchRequestSeqRef.current) { + setError(requestError instanceof Error ? requestError.message : "Search failed"); + } } finally { - setLoading(false); - setAnswerProgress(null); + if (requestId === searchRequestSeqRef.current) { + setLoading(false); + setAnswerProgress(null); + } } } diff --git a/src/lib/answer-ranking.ts b/src/lib/answer-ranking.ts index c3a0023a40..ab5d2ea23f 100644 --- a/src/lib/answer-ranking.ts +++ b/src/lib/answer-ranking.ts @@ -322,10 +322,22 @@ function applyBoldPatternOutsideExisting(text: string, pattern: RegExp, maxMatch } function capBoldSegments(text: string, originalText: string, maxSegments = 4) { - const existingBold = new Set(Array.from(originalText.matchAll(/\*\*([^*]+)\*\*/g), (match) => match[1])); + // Audit L14: allow only as many "free" (uncounted) passes of a bold string + // as occurrences that existed in the original text. The previous Set-based + // check let every NEWLY-bolded token identical to a pre-existing bold + // phrase bypass the cap, so an answer could render far more than + // maxSegments bold segments. + const existingBoldCounts = new Map(); + for (const match of originalText.matchAll(/\*\*([^*]+)\*\*/g)) { + existingBoldCounts.set(match[1], (existingBoldCounts.get(match[1]) ?? 0) + 1); + } let kept = 0; return text.replace(/\*\*([^*]+)\*\*/g, (match, content: string) => { - if (existingBold.has(content)) return match; + const freePasses = existingBoldCounts.get(content) ?? 0; + if (freePasses > 0) { + existingBoldCounts.set(content, freePasses - 1); + return match; + } kept += 1; return kept <= maxSegments ? match : content; }); diff --git a/src/lib/answer-verification.ts b/src/lib/answer-verification.ts index 39c1da6437..6842d73553 100644 --- a/src/lib/answer-verification.ts +++ b/src/lib/answer-verification.ts @@ -82,18 +82,52 @@ export function extractNumericTokens(text: string): string[] { } function tableFactText(fact: DocumentTableFact): string { - return [fact.table_title, fact.row_label, fact.clinical_parameter, fact.threshold_value, fact.action] + // Diff-review completion of audit H1: rich-mode prompts resolve a "table + // snippet" from these fact-metadata fields when no image is attached + // (tableSnippetForFact in rag.ts), so numbers the model copies from them + // must count as verified too. + const metadata = (fact.metadata ?? {}) as Record; + const metadataString = (key: string) => (typeof metadata[key] === "string" ? (metadata[key] as string) : ""); + const metadataCells = Array.isArray(metadata.cells) ? (metadata.cells as unknown[]).map(String).join(" ") : ""; + return [ + fact.table_title, + fact.row_label, + fact.clinical_parameter, + fact.threshold_value, + fact.action, + metadataString("accessible_table_markdown"), + metadataString("table_text_snippet"), + metadataCells, + ] .filter(Boolean) .join(" "); } +// The verification corpus must cover the SAME text the model was shown in +// buildRagSourceBlock (rag.ts): retrieval_synopsis and image table text are +// rendered into the prompt, so a number the model faithfully copied from them +// must count as verified. Omitting them blanked correct dose/threshold answers +// whose figures lived only in a synopsis or table-crop text (audit H1); this +// mirrors sourceTextForQuoteVerification in rag.ts. function sourceTextForResult(result: SearchResult): string { const parts: string[] = [result.content ?? ""]; if (result.adjacent_context) parts.push(result.adjacent_context); if (result.section_heading) parts.push(result.section_heading); + if (result.retrieval_synopsis) parts.push(result.retrieval_synopsis); if (result.table_facts?.length) parts.push(result.table_facts.map(tableFactText).join(" ")); if (result.memory_cards?.length) parts.push(result.memory_cards.map((card) => card.content).join(" ")); if (result.index_unit) parts.push([result.index_unit.title, result.index_unit.content].filter(Boolean).join(" ")); + if (result.images?.length) { + parts.push( + result.images + .map((image) => + [image.tableLabel, image.tableTitle, image.caption, image.tableTextSnippet, image.accessibleTableMarkdown] + .filter(Boolean) + .join(" "), + ) + .join(" "), + ); + } return parts.join(" "); } diff --git a/src/lib/chunking.ts b/src/lib/chunking.ts index afbe156ba4..348fc91114 100644 --- a/src/lib/chunking.ts +++ b/src/lib/chunking.ts @@ -9,7 +9,11 @@ const metadataNoisePatterns: RegExp[] = [ /\b(?:version|revision)\s+\d+\s*$/i, ]; const lineNoisePatterns: RegExp[] = [ - /\b(page|p\.?)\s*\d+\s*(?:\/\s*\d+)?\b/i, + // Audit M14: anchored to the WHOLE line. The previous unanchored form + // matched an inline page reference anywhere in a sentence ("refer to p 3 + // for dosing", "titrate p 20 micrograms") and removePageNoise then deleted + // the entire clinical line. Only a standalone page footer counts as noise. + /^\s*(?:page|p\.?)\s*\d+\s*(?:(?:\/|of)\s*\d+)?\s*$/i, /^\s*[-*_]{3,}\s*$/, /^\s*[\u25cf\u25e6\u2022]\s*$/, ]; @@ -317,7 +321,13 @@ function chunkTextBySentence(clean: string, chunkSize: number, overlap: number) const chunk = clean.slice(start, end).trim(); if (chunk) chunks.push(chunk); if (end >= clean.length) break; - start = readableOverlapStart(clean, end, overlap); + // Audit M17: when overlap >= chunkSize, readableOverlapStart can return a + // position at or before the current start and the loop never advances, + // hanging the ingestion worker on that document. Force strict forward + // progress: if the overlap window does not move us forward, continue from + // the end of the current chunk instead. + const nextStart = readableOverlapStart(clean, end, overlap); + start = nextStart > start ? nextStart : end; } return chunks; diff --git a/src/lib/clinical-search.ts b/src/lib/clinical-search.ts index e443239497..bf4a0d302d 100644 --- a/src/lib/clinical-search.ts +++ b/src/lib/clinical-search.ts @@ -551,8 +551,19 @@ function clamp(value: number) { return Number.isFinite(value) ? Math.min(1, Math.max(0, value)) : 0; } +// Audit M2: intent signal words match at word boundaries, not as bare +// substrings — "time limit" used to trigger the "im" dosing signal and +// "notable" the "table" visual signal, corrupting ranking boosts. Short +// acronym-like tokens (im, po, mg, prn, mcg) must match as whole words, with +// a digit-adjacent allowance so unit-attached doses ("100mg") still count; +// longer entries keep deliberate prefix semantics ("escalat" → escalation, +// "table" → tables). function containsAny(value: string, values: readonly string[]) { - return values.some((item) => value.includes(item)); + return values.some((item) => { + const escaped = item.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const pattern = item.length <= 3 ? `(?:\\b|\\d)${escaped}\\b` : `\\b${escaped}`; + return new RegExp(pattern, "i").test(value); + }); } function normalizeQueryTokenForLookups(value: string) { @@ -754,18 +765,24 @@ function sectionDepthSignal(querySignal: IntentSignals, sectionHeading: string | return 0; } +// Explicit dose vocabulary that must survive escalation-word cancellation +// (audit M3): "clozapine dose review schedule" is a genuine dosing query even +// though "review" is an escalation signal word. +const explicitDoseTerms = ["dose", "dosage", "dosing", "titrat", "mg", "mcg"] as const; + export function classifyQueryIntent(query: string): IntentSignals { const lowered = query.toLowerCase(); const match = intentPatterns.find((entry) => entry.pattern.test(query)); const hasDosingSignals = containsAny(lowered, intentSignalWords.dosing); const hasEscalationSignals = containsAny(lowered, intentSignalWords.escalation); const hasImageSignals = containsAny(lowered, intentSignalWords.visuals); + const hasExplicitDoseTerm = containsAny(lowered, explicitDoseTerms); return { intent: match?.intent ?? "general", imageEvidenceFocus: Boolean(match?.imageEvidenceFocus) || hasImageSignals, sectionedLookup: Boolean(match?.sectionedLookup), - hasDosingSignals: hasDosingSignals && !hasEscalationSignals, + hasDosingSignals: hasDosingSignals && (hasExplicitDoseTerm || !hasEscalationSignals), }; } diff --git a/src/lib/cross-document-synthesis.ts b/src/lib/cross-document-synthesis.ts index ce2d127219..3ec11bd0a8 100644 --- a/src/lib/cross-document-synthesis.ts +++ b/src/lib/cross-document-synthesis.ts @@ -192,7 +192,7 @@ export function buildCrossDocumentSynthesisPlan( const documents = documentCount(results); const reason = planReason(query, queryClass, documents); const enabled = documents > 1 && isCrossDocumentIntent(query, queryClass); - const maxPerDocument = queryClass === "comparison" ? 2 : 2; + const maxPerDocument = 2; const limit = queryClass === "comparison" || queryClass === "broad_summary" ? 8 : 6; const balanced = enabled ? balanceCrossDocumentResults(results, { diff --git a/src/lib/deep-memory.ts b/src/lib/deep-memory.ts index fe2ed743cd..0d463ba3ff 100644 --- a/src/lib/deep-memory.ts +++ b/src/lib/deep-memory.ts @@ -641,41 +641,16 @@ export async function upsertDocumentDeepMemory(args: { const cards = buildDocumentMemoryCards({ ...args, sections, modelProfile }); if (cards.length === 0) throw new Error("Deep memory generated no source-backed memory cards."); - await args.supabase.from("document_memory_cards").delete().eq("document_id", args.document.id); - await args.supabase.from("document_sections").delete().eq("document_id", args.document.id); - await args.supabase - .from("document_index_units") - .delete() - .eq("document_id", args.document.id) - .then(undefined, () => undefined); - - const { data: insertedSections, error: sectionError } = await args.supabase - .from("document_sections") - .insert(sections) - .select("id,section_index"); - if (sectionError) throw new Error(sectionError.message); - - const sectionIds = new Map(); - for (const section of insertedSections ?? []) { - sectionIds.set(section.section_index, section.id); - } - + // Audit M11: compute everything that can fail on the network (memory-card + // and index-unit embeddings) BEFORE deleting the existing memory rows. The + // previous order deleted first and rebuilt after, so an OpenAI outage + // mid-rebuild left the document with ZERO memory rows (silent retrieval + // degradation) while documents.metadata still advertised the prior + // rag_memory_version. A failure between the delete and insert below is + // still possible, but the exposure window no longer spans OpenAI calls. const embeddings = await embedTexts(cards.map(embeddingText)); if (embeddings.length !== cards.length) throw new Error("OpenAI returned an unexpected memory-card embedding count."); - for (let start = 0; start < cards.length; start += 50) { - const batch = cards.slice(start, start + 50).map((card, index) => { - const { section_index: sectionIndex, ...row } = card; - return { - ...row, - section_id: sectionIndex === undefined ? null : (sectionIds.get(sectionIndex) ?? null), - embedding: assertEmbeddingDim(embeddings[start + index], `document_memory_cards.${start + index}`), - }; - }); - const { error } = await args.supabase.from("document_memory_cards").insert(batch); - if (error) throw new Error(error.message); - } - const indexUnits = buildDocumentIndexUnitInputs({ document: args.document, chunks: args.chunks, @@ -712,8 +687,47 @@ export async function upsertDocumentDeepMemory(args: { }; }), }); + const indexUnitEmbeddings = + indexUnits.length > 0 ? await embedTexts(indexUnits.map(embeddingTextForDocumentIndexUnit)) : []; + if (indexUnits.length > 0 && indexUnitEmbeddings.length !== indexUnits.length) { + throw new Error("OpenAI returned an unexpected index-unit embedding count."); + } + + // All embeddings are in hand — replace the previous memory atomically-ish: + // delete then insert without any intervening network dependency (M11). + await args.supabase.from("document_memory_cards").delete().eq("document_id", args.document.id); + await args.supabase.from("document_sections").delete().eq("document_id", args.document.id); + await args.supabase + .from("document_index_units") + .delete() + .eq("document_id", args.document.id) + .then(undefined, () => undefined); + + const { data: insertedSections, error: sectionError } = await args.supabase + .from("document_sections") + .insert(sections) + .select("id,section_index"); + if (sectionError) throw new Error(sectionError.message); + + const sectionIds = new Map(); + for (const section of insertedSections ?? []) { + sectionIds.set(section.section_index, section.id); + } + + for (let start = 0; start < cards.length; start += 50) { + const batch = cards.slice(start, start + 50).map((card, index) => { + const { section_index: sectionIndex, ...row } = card; + return { + ...row, + section_id: sectionIndex === undefined ? null : (sectionIds.get(sectionIndex) ?? null), + embedding: assertEmbeddingDim(embeddings[start + index], `document_memory_cards.${start + index}`), + }; + }); + const { error } = await args.supabase.from("document_memory_cards").insert(batch); + if (error) throw new Error(error.message); + } + if (indexUnits.length > 0) { - const indexUnitEmbeddings = await embedTexts(indexUnits.map(embeddingTextForDocumentIndexUnit)); for (let start = 0; start < indexUnits.length; start += 50) { const batch = indexUnits.slice(start, start + 50).map((unit, index) => ({ ...unit, @@ -860,9 +874,15 @@ export async function fetchMemoryCardsForQuery(args: { (documents ?? []).map((document) => [document.id, committedIndexGeneration(document.metadata)] as const), ); + // Audit L8: fail CLOSED. If the documents lookup errors we cannot tell + // which generation is committed, so returning the cards unfiltered could + // inject content from an abandoned/superseded reindex generation into a + // clinical answer. Dropping the fallback cards for one query (retrieval + // degrades gracefully) is safer than exposing stale memory. + if (documentsError) return []; + return cards .filter((card) => { - if (documentsError) return true; if (!committedGenerationByDocument.has(card.document_id)) return true; return isCommittedGenerationMetadata({ rowMetadata: card.metadata, diff --git a/src/lib/document-index-units.ts b/src/lib/document-index-units.ts index 944a2891c7..b6c43b9c6e 100644 --- a/src/lib/document-index-units.ts +++ b/src/lib/document-index-units.ts @@ -287,7 +287,13 @@ function fallbackVisualUnitType( profile: StructuredVisualProfile, text: string, ): DocumentIndexUnitType { - if (/\b(?:flow\s*chart|flowchart|algorithm|decision|yes|no|next step|pathway)\b/i.test(text)) return "flowchart_step"; + // Audit M4: bare "yes"/"no" are common TABLE cell values ("No further + // action", a Yes/No column), so they only count as flowchart evidence when + // the image is not structurally a table (table_crop / extracted rows). + // Specific flowchart terms still win outright. + const structuralTable = image.sourceKind === "table_crop" || (image.tableRows?.length ?? 0) > 0; + if (/\b(?:flow\s*chart|flowchart|algorithm|decision|next step|pathway)\b/i.test(text)) return "flowchart_step"; + if (!structuralTable && /\b(?:yes|no)\b/i.test(text)) return "flowchart_step"; if (/\b(?:risk matrix|red zone|likelihood|consequence|high risk|visual alert)\b/i.test(text)) return "risk_matrix_cell"; if ( diff --git a/src/lib/document-organization.ts b/src/lib/document-organization.ts index c8b76e71f3..4670b7f65c 100644 --- a/src/lib/document-organization.ts +++ b/src/lib/document-organization.ts @@ -259,7 +259,10 @@ const siteDefinitions: SiteDefinition[] = [ canonical: "BMJ Best Practice", rawTags: ["bmj"], kind: "reference_collection", - evidence: [/\bbmj\b/i, /\bbest practice\b/i], + // Audit M7: require the "bmj" token. The generic phrase "best practice" + // appears in ordinary local policies ("in line with best practice…") and + // falsely attributed them to an external commercial reference. + evidence: [/\bbmj\b/i], }, ]; @@ -1239,10 +1242,27 @@ export function canonicalDocumentDisplayTitle(input: Pick candidate.confidence >= 0.75); const selected = selectSiteCandidate(confirmedCandidates) ?? null; - const referenceCollection = !selected && candidates.length === 0 ? referenceCollectionFromEvidence(input) : null; + // Audit M5: gate the reference fallbacks on the absence of a CONFIRMED + // candidate, not on candidates.length — an unconfirmed bracket-tag guess + // (confidence 0.58, no corroborating evidence) used to suppress both + // fallbacks and leave an obvious clinical reference as site=null. + const referenceCollection = + !selected && confirmedCandidates.length === 0 ? referenceCollectionFromEvidence(input) : null; if (referenceCollection) { return { label: referenceCollection.canonical, @@ -1343,12 +1368,12 @@ function classifySite(input: OrganizationDocumentInput, rawTags: string[]) { kind: referenceCollection.kind, confidence: 0.86, evidence_sources: [`source:${referenceCollection.rawTags[0]}`], - candidates: [], + candidates, }; } - if (!selected && candidates.length === 0 && hasGeneralClinicalReferenceEvidence(input)) { - return generalClinicalReferenceSite; + if (!selected && confirmedCandidates.length === 0 && hasGeneralClinicalReferenceEvidence(input)) { + return { ...generalClinicalReferenceSite, candidates }; } return { diff --git a/src/lib/document-tags.ts b/src/lib/document-tags.ts index 06bd77855a..a8c8906c71 100644 --- a/src/lib/document-tags.ts +++ b/src/lib/document-tags.ts @@ -254,20 +254,20 @@ function usefulTokenCount(value: string) { return value.split(/\s+/).filter((token) => token.length > 2 || acronymDisplay.has(token)).length; } +// Audit L16: the function's default is an unconditional `return false`, so +// any label surviving the noisy checks is kept. The former document_type/site +// "protection" branches also returned false and were behaviorally dead code +// that misled maintainers into thinking those labels had protection a future +// noisy rule would silently bypass. If a rule must never apply to protected +// labels, short-circuit it ABOVE the noisy checks instead. function isNoisyLabel(label: string, labelType: DocumentLabelType) { + void labelType; if (!label || label.length < 2 || label.length > 64) return true; if (lowValuePattern.test(label)) return true; if (/\b(?:docx?|xlsx?|pptx?|pdf)\b/.test(label)) return true; if (lowValueExact.has(label)) return true; if (usefulTokenCount(label) === 0) return true; if (label.split(/\s+/).length > 6) return true; - if (labelType === "document_type" && ["clinical form", "clinical checklist"].includes(label)) return false; - if ( - labelType === "site" && - /\b(?:hospital|health service|fiona stanley|rockingham peel|mental health service)\b/.test(label) - ) { - return false; - } return false; } diff --git a/src/lib/env.ts b/src/lib/env.ts index 322f5d7b7f..b998fc82a9 100644 --- a/src/lib/env.ts +++ b/src/lib/env.ts @@ -59,6 +59,11 @@ const envSchema = z.object({ .enum(["true", "false"]) .default("false") .transform((value) => value === "true"), + // Audit M15: server-side key for the redacted query hash. When set, stored + // query hashes are HMAC-SHA256 (not offline-reversible, not correlatable + // outside this deployment). When unset, the legacy unsalted SHA-256 is kept + // for continuity with previously stored rows. + RAG_QUERY_HASH_SECRET: z.string().min(16).optional(), SUPABASE_DOCUMENT_BUCKET: z.string().default("clinical-documents"), SUPABASE_IMAGE_BUCKET: z.string().default("clinical-images"), MAX_UPLOAD_MB: z.coerce.number().int().positive().default(150), diff --git a/src/lib/evidence-relevance.ts b/src/lib/evidence-relevance.ts index 2f87784492..80f19d1a5a 100644 --- a/src/lib/evidence-relevance.ts +++ b/src/lib/evidence-relevance.ts @@ -344,7 +344,16 @@ export function buildEvidenceRelevance(query: string, results: SearchResult[]): ? "direct" : directSourceCount > 0 || partialSourceCount > 0 ? "partial" - : matchedTerms.length > 0 || results.length > 0 + : // Audit L3: base nearby-vs-none on matched terms. `results.length > 0` + // was always true here (empty results early-return above), which made + // the aggregate "none" verdict unreachable and overstated evidence + // for result sets matching zero query terms. Strong retrieval scores + // still count as "nearby" (diff-review hardening): term matching is + // purely lexical, so a good semantic match ("myocardial infarction" + // for "heart attack") must not degrade to "none" and trip the + // danger-level governance banner while every per-source chip reads + // "nearby". + matchedTerms.length > 0 || score >= 0.5 ? "nearby" : "none"; const relevance: EvidenceRelevance = { diff --git a/src/lib/evidence.ts b/src/lib/evidence.ts index fe1711f437..10579f332f 100644 --- a/src/lib/evidence.ts +++ b/src/lib/evidence.ts @@ -254,12 +254,22 @@ export function buildDocumentBreakdown(results: SearchResult[], quoteCards: Quot return Array.from(grouped.values()).sort((a, b) => b.top_similarity - a.top_similarity); } -export function buildSmartPanel(query: string, results: SearchResult[]) { +// Audit L10: callers that already computed relevance/visual evidence for the +// same query+results (the hot search route does) can pass them in instead of +// paying for a recomputation whose smart-panel copy was then overwritten. +export function buildSmartPanel( + query: string, + results: SearchResult[], + precomputed?: { + relevance?: ReturnType; + visualEvidence?: ReturnType; + }, +) { const quoteCards = extractQuoteCards(results, query); const documentBreakdown = buildDocumentBreakdown(results, quoteCards); - const visualEvidence = buildVisualEvidence(results); + const visualEvidence = precomputed?.visualEvidence ?? buildVisualEvidence(results); const bestSource = selectBestSourceRecommendation(results, quoteCards); - const relevance = buildEvidenceRelevance(query, results); + const relevance = precomputed?.relevance ?? buildEvidenceRelevance(query, results); return { query, diff --git a/src/lib/image-filtering.ts b/src/lib/image-filtering.ts index 5d35f31da9..facade75b7 100644 --- a/src/lib/image-filtering.ts +++ b/src/lib/image-filtering.ts @@ -321,6 +321,14 @@ function bytesFromHashInput(input: string | Uint8Array | ArrayBuffer) { return input; } +// Audit M12: the previous 16-bucket / 16-bit digest (65,536 possible values) +// realistically collided across distinct same-dimension clinical table crops, +// silently collapsing two DIFFERENT threshold tables into one searchable +// "visual family" (one of them was dropped from the index). The v2 digest +// uses 64 buckets with both a mean-threshold bit pattern (64 bits) and a +// 2-bit quantized level per bucket (128 bits), making accidental collisions +// across a document corpus negligible. The version prefix is bumped +// (ph1 -> ph2) so legacy stored hashes can never alias newly computed ones. export function lightweightPerceptualHash( imageContent: string | Uint8Array | ArrayBuffer, width?: number | null, @@ -328,9 +336,9 @@ export function lightweightPerceptualHash( ) { const bytes = bytesFromHashInput(imageContent); const sizeKey = `${width ?? "w"}x${height ?? "h"}`; - if (bytes.length === 0) return `ph1:${sizeKey}:empty`; + if (bytes.length === 0) return `ph2:${sizeKey}:empty`; - const bucketCount = 16; + const bucketCount = 64; const buckets = new Array(bucketCount).fill(0); const counts = new Array(bucketCount).fill(0); const step = Math.max(1, Math.floor(bytes.length / 4096)); @@ -345,7 +353,15 @@ export function lightweightPerceptualHash( const averages = buckets.map((sum, index) => sum / Math.max(counts[index], 1)); const mean = averages.reduce((sum, value) => sum + value, 0) / bucketCount; - const bitString = averages.map((value) => (value >= mean ? "1" : "0")).join(""); - const hex = Number.parseInt(bitString, 2).toString(16).padStart(4, "0"); - return `ph1:${sizeKey}:${hex}`; + const maxAverage = Math.max(...averages, 1); + const thresholdBits = averages.map((value) => (value >= mean ? "1" : "0")).join(""); + const levelBits = averages + .map((value) => { + const level = Math.min(3, Math.floor((value / maxAverage) * 4)); + return level.toString(2).padStart(2, "0"); + }) + .join(""); + const thresholdHex = BigInt(`0b${thresholdBits}`).toString(16).padStart(16, "0"); + const levelHex = BigInt(`0b${levelBits}`).toString(16).padStart(32, "0"); + return `ph2:${sizeKey}:${thresholdHex}${levelHex}`; } diff --git a/src/lib/ingestion.ts b/src/lib/ingestion.ts index 9a0f7507d4..0d25e13e85 100644 --- a/src/lib/ingestion.ts +++ b/src/lib/ingestion.ts @@ -1,10 +1,13 @@ export function isRetryableIngestionError(error: unknown) { const message = error instanceof Error ? error.message : String(error); + // Duplicate-key conflicts — including document_pages page-number duplicates — + // are partial index-write conflicts that the worker routes to manual queue + // recovery, never auto-retry. (Audit L17: removed an unreachable retry + // special-case for the page-number constraint that this short-circuit made + // dead code; recovery is the deliberate path for those conflicts.) if (isPartialIndexWriteConflict(error)) return false; - return ( - /\b(429|rate limit|timeout|temporar|network|fetch failed|ECONNRESET|ETIMEDOUT|5\d\d|502|503|504|bad gateway|gateway timeout|service unavailable)\b/i.test( - message, - ) || /document_pages_document_id_page_number_key/i.test(message) + return /\b(429|rate limit|timeout|temporar|network|fetch failed|ECONNRESET|ETIMEDOUT|5\d\d|502|503|504|bad gateway|gateway timeout|service unavailable)\b/i.test( + message, ); } diff --git a/src/lib/privacy.ts b/src/lib/privacy.ts index 81f7e9612b..b6790ae7ca 100644 --- a/src/lib/privacy.ts +++ b/src/lib/privacy.ts @@ -2,8 +2,20 @@ export function safeIngestionJobLog(jobId: string) { return `Processing ingestion job ${jobId}`; } -function redactLogValue(value: unknown) { - if (typeof value !== "string") return value; +function redactLogValue(value: unknown): unknown { + if (typeof value !== "string") { + // Audit L12: non-string code/details/hint fields (objects/arrays from + // non-standard error shapes) used to pass through verbatim, skipping the + // path/url/secret/email redaction below. Serialize them (guarded) and + // redact the serialized form; primitives stay as-is. + if (value === null || value === undefined) return value; + if (typeof value === "number" || typeof value === "boolean") return value; + try { + return redactLogValue(JSON.stringify(value) ?? "[unserializable]"); + } catch { + return "[unserializable]"; + } + } const htmlTitle = value.match(/\s*([^<]+?)\s*<\/title>/i)?.[1]?.trim(); const normalizedValue = htmlTitle ? `HTML response: ${htmlTitle}` : value; return normalizedValue diff --git a/src/lib/query-privacy.ts b/src/lib/query-privacy.ts index e887aeca1b..13cd6bd3c0 100644 --- a/src/lib/query-privacy.ts +++ b/src/lib/query-privacy.ts @@ -1,12 +1,25 @@ -import { createHash } from "node:crypto"; +import { createHash, createHmac } from "node:crypto"; import { env } from "@/lib/env"; export function normalizeQueryText(query: string) { return query.toLowerCase().replace(/\s+/g, " ").trim(); } +// Audit M15: an unsalted SHA-256 of a short, low-entropy clinical query is +// dictionary-reversible (hash candidate patient/drug strings offline and +// match) and lets any reader of the log tables correlate the same query +// across rows — undermining the redaction it implements. When +// RAG_QUERY_HASH_SECRET is set, the stored hash is a keyed pseudonym +// (HMAC-SHA256): not offline-reversible and not correlatable outside this +// deployment. Without the secret, the legacy unsalted digest is kept so +// existing stored rows still join/dedup; set the secret in any environment +// where real clinical queries are logged. export function hashQueryText(query: string) { - return createHash("sha256").update(normalizeQueryText(query)).digest("hex"); + const normalized = normalizeQueryText(query); + if (env.RAG_QUERY_HASH_SECRET) { + return createHmac("sha256", env.RAG_QUERY_HASH_SECRET).update(normalized).digest("hex"); + } + return createHash("sha256").update(normalized).digest("hex"); } function queryHashStorageText(query: string) { diff --git a/src/lib/rag.ts b/src/lib/rag.ts index 8680f4696b..4241ac62d5 100644 --- a/src/lib/rag.ts +++ b/src/lib/rag.ts @@ -642,10 +642,20 @@ function allowedChunkMap(results: SearchResult[]) { return new Map(results.map((result) => [result.id, result])); } -function deriveConfidence(results: SearchResult[], acceptedCitationCount: number): RagAnswer["confidence"] { - if (acceptedCitationCount === 0 || results.length === 0) return "unsupported"; - const strongest = results.reduce((max, result) => Math.max(max, result.similarity), 0); - if (strongest >= 0.82 && acceptedCitationCount >= 2) return "high"; +// Audit M1: confidence must reflect the strength of the evidence the answer +// actually CITES. Taking the max similarity over ALL retrieved results let an +// uncited high-similarity chunk grant "high" confidence to an answer built on +// weak citations, so the strongest-score scan is scoped to the cited subset. +// A citation that maps to no known chunk contributes nothing (fail low). +function deriveConfidence( + results: SearchResult[], + acceptedCitations: Array<Pick<Citation, "chunk_id">>, +): RagAnswer["confidence"] { + if (acceptedCitations.length === 0 || results.length === 0) return "unsupported"; + const citedIds = new Set(acceptedCitations.map((citation) => citation.chunk_id)); + const citedResults = results.filter((result) => citedIds.has(result.id)); + const strongest = citedResults.reduce((max, result) => Math.max(max, result.similarity), 0); + if (strongest >= 0.82 && acceptedCitations.length >= 2) return "high"; if (strongest >= 0.64) return "medium"; return "low"; } @@ -872,7 +882,22 @@ function normalizeQuoteVerificationText(text: string) { } function tableFactQuoteText(fact: NonNullable<SearchResult["table_facts"]>[number]) { - return [fact.table_title, fact.row_label, fact.clinical_parameter, fact.threshold_value, fact.action] + // Mirrors tableFactText in answer-verification.ts: include the fact-metadata + // snippet fields that rich-mode prompts show the model (tableSnippetForFact), + // so quotes drawn from them verify as exact. + const metadata = safeRecord(fact.metadata); + const metadataString = (key: string) => (typeof metadata[key] === "string" ? (metadata[key] as string) : ""); + const metadataCells = Array.isArray(metadata.cells) ? (metadata.cells as unknown[]).map(String).join(" ") : ""; + return [ + fact.table_title, + fact.row_label, + fact.clinical_parameter, + fact.threshold_value, + fact.action, + metadataString("accessible_table_markdown"), + metadataString("table_text_snippet"), + metadataCells, + ] .filter(Boolean) .join(" "); } @@ -4520,7 +4545,7 @@ function buildExtractiveAnswer(args: { return { answer: naturalAnswer.answer, grounded: hasExtractedAnswer && citations.length > 0, - confidence: hasExtractedAnswer ? deriveConfidence(args.results, citations.length) : "unsupported", + confidence: hasExtractedAnswer ? deriveConfidence(args.results, citations) : "unsupported", citations: citations.slice(0, 5), sources: args.results, modelUsed: null, @@ -5668,7 +5693,7 @@ export function parseAnswerJson(raw: string, results: SearchResult[], query?: st try { const parsed = answerJsonSchema.parse(JSON.parse(raw)); const { citations, modelCited, proposedCount, invalidCount } = sanitizeCitations(parsed.citations, results); - const derivedConfidence = modelCited ? deriveConfidence(results, citations.length) : "unsupported"; + const derivedConfidence = modelCited ? deriveConfidence(results, citations) : "unsupported"; const confidence = modelCited ? clampConfidence(parsed.confidence, derivedConfidence) : "unsupported"; const parsedAnswer = parsed.answer ?? ""; const nonArtifactParsedAnswer = parsedAnswer.trim() && !looksLikeJsonArtifact(parsedAnswer) ? parsedAnswer : ""; @@ -6543,7 +6568,7 @@ ${qualityRetryInstruction}` args.query, ), grounded: false, - confidence: hasSources ? deriveConfidence(answerInputResults, fallbackCitations.length) : "unsupported", + confidence: hasSources ? deriveConfidence(answerInputResults, fallbackCitations) : "unsupported", citations: hasSources ? fallbackCitations : [], sources: answerInputResults, modelUsed: null, @@ -6977,7 +7002,7 @@ ${qualityRetryInstruction}` ...baseFallbackAnswer, answer: boldHighYieldClinicalText(sourceBackedGenerationTimeoutAnswer(args.query), args.query), grounded: true, - confidence: deriveConfidence(answerInputResults, baseFallbackAnswer.citations.length), + confidence: deriveConfidence(answerInputResults, baseFallbackAnswer.citations), routingMode: "extractive", routingReason: reviewRouteReason, queryAnalysis, diff --git a/src/lib/retrieval-selection.ts b/src/lib/retrieval-selection.ts index f7717717a2..8b14c1545a 100644 --- a/src/lib/retrieval-selection.ts +++ b/src/lib/retrieval-selection.ts @@ -285,7 +285,6 @@ function lexicalScoreForSignals(requiredSignals: string[], matchedSignals: strin function resultBoost(args: { intent: RetrievalIntent; candidate: RetrievalCandidate; result: SearchResult }) { const signals = new Set(args.candidate.matchedSignals); let boost = 0; - const metadata = args.result.source_metadata; if (args.intent.needsMedicationChart && args.candidate.chunkType === "medication_chart") boost += 0.18; if (args.intent.needsMedicationChart && args.intent.requiredTermSignals.includes("agitation")) { @@ -325,20 +324,14 @@ function resultBoost(args: { intent: RetrievalIntent; candidate: RetrievalCandid if (args.intent.requiredTermSignals.length > 0 && args.candidate.lexicalScore === 1) boost += 0.1; if (args.intent.requiredTermSignals.length > 0 && (args.candidate.lexicalScore ?? 0) === 0) boost -= 0.08; - if (metadata?.document_status === "current") boost += 0.06; - if (metadata?.document_status === "review_due") boost -= 0.12; - if (metadata?.document_status === "outdated") boost -= 0.24; - if (!metadata?.document_status || metadata.document_status === "unknown") boost -= 0.08; - - if (metadata?.clinical_validation_status === "approved") boost += 0.06; - if (metadata?.clinical_validation_status === "locally_reviewed") boost += 0.05; - if (!metadata?.clinical_validation_status || metadata.clinical_validation_status === "unverified") boost -= 0.08; - - if (metadata?.extraction_quality === "good") boost += 0.02; - if (metadata?.extraction_quality === "partial") boost -= 0.04; - if (metadata?.extraction_quality === "poor") boost -= 0.12; - if (!metadata?.extraction_quality || metadata.extraction_quality === "unknown") boost -= 0.05; - + // NOTE (measured, do not reintroduce without re-running the golden retrieval eval): source + // governance metadata (document_status / clinical_validation_status / extraction_quality) must + // NOT weight selection ordering here. The corpus is only partially enriched — unenriched docs + // normalize to unknown/unverified — so metadata weighting swings ranking by up to ~0.35 for + // reasons unrelated to relevance and buried correct documents (golden doc-recall@5 1.0 -> 0.76, + // 7/23 failures). Governance is enforced in ranking penalties and the answer/source-governance + // layer instead; RC8 (source-strength as a filter) remains tracked in + // docs/rag-hybrid-findings-and-todo.md. return boost; } @@ -458,9 +451,14 @@ export function buildRetrievalCandidates( const matchedSignals = matchedSignalsForResult({ intent, result, chunkType }); const lexicalScore = lexicalScoreForSignals(intent.requiredTermSignals, matchedSignals); const candidate = { ...initial, lexicalScore, matchedSignals }; + // The relevance score stays CLAMPED: live hybrid scores routinely saturate at 1.0, and letting + // boosts raise the primary score uncapped made boost stacking override lexical relevance + // entirely (golden doc-recall@5 regressed 1.0 -> 0.76). Within the saturated region, ordering + // falls through to lexicalScore then rerankScore (the clinical relevance rank), which is the + // behaviour the golden retrieval eval validates. return { ...candidate, - score: Number(Math.max(0, candidate.score + resultBoost({ intent, candidate, result })).toFixed(4)), + score: clamp(candidate.score + resultBoost({ intent, candidate, result })), }; }); } diff --git a/src/lib/source-text-sanitizer.ts b/src/lib/source-text-sanitizer.ts index 33627700ef..3271a1726e 100644 --- a/src/lib/source-text-sanitizer.ts +++ b/src/lib/source-text-sanitizer.ts @@ -34,6 +34,16 @@ const provenanceNoiseTermPattern = /\b(?:guideline|procedure|protocol|policy|appendix|source|evidence|document|file|page|scale|lunsers|liverpool university|rating scale|retrieved|excerpt|passage)\b/gi; const concreteClinicalActionPattern = /\b(?:administer|arrange|assess|cease|check|complete|contact|document|escalat|follow\s*up|monitor|notify|record|refer|report|review|stop|withhold|dose|prescrib|titrate)\b/i; +// Audit H2: threshold-bearing numerics (unit-bearing figures, ranges, +// comparatives like "8 or below", decimals) mark a fragment as carrying +// clinical VALUES. The noise heuristics below must never drop such a fragment: +// sourceTitleFragmentPattern greedily consumes up to 180 chars after a title +// keyword ("… Scale ranges from 3 to 15 …"), which silently deleted threshold +// sentences. Bare integers ("Appendix 1") deliberately do NOT match, so real +// title noise is still dropped. Falsely keeping noise is cosmetic; falsely +// dropping a threshold is a clinical-safety failure, so this leans generous. +const clinicalThresholdSignalPattern = + /\b\d+(?:[.,]\d+)?\s*(?:mg|mcg|micrograms?|μg|µg|g|kg|ml|mmol|mol|units?|iu|hours?|hrs?|mins?|minutes?|days?|weeks?|months?|years?|mmhg|bpm|°c)\b|\b\d+(?:[.,]\d+)?\s*%|\b\d+(?:[.,]\d+)?\s*(?:[-–—]|to)\s*\d+(?:[.,]\d+)?\b|(?<![a-z0-9])(?:×|x)10\^?\d*|\b(?:below|above|under|over|at\s+least|at\s+most|more\s+than|less\s+than|greater\s+than|fewer\s+than)\s+\d+(?:[.,]\d+)?|\b\d+(?:[.,]\d+)?\s+or\s+(?:below|above|less|more|lower|higher|greater|fewer)\b|(?<![vV])\b\d+\.\d+\b/i; function compactWhitespace(value: string) { return value.replace(/\s+/g, " ").trim(); @@ -219,24 +229,45 @@ function isMostlySourceTitleFragment(text: string) { export function clinicalProseUsefulness(text: string) { const cleaned = sourceTextForClinicalProse(text); - const fragments = sentenceFragments(cleaned).filter((fragment) => { - if (!fragment) return false; - if (tokenCount(fragment) < 3) return false; - if (/^the\s+(?:retrieved|supplied|provided|indexed)\s+/i.test(fragment)) return false; + const keptFragments: string[] = []; + const baselineKeptFragments: string[] = []; + for (const fragment of sentenceFragments(cleaned)) { + if (!fragment) continue; + if (tokenCount(fragment) < 3) continue; + if (/^the\s+(?:retrieved|supplied|provided|indexed)\s+/i.test(fragment)) continue; const hasClinicalSignal = clinicalSignalPattern.test(fragment); const hasConcreteClinicalAction = concreteClinicalActionPattern.test(fragment); + // H2: a fragment carrying threshold-bearing numerics is never dropped by + // the title/noise heuristics — deleting clinical values is worse than + // keeping a little provenance noise. + const hasClinicalThresholdSignal = clinicalThresholdSignalPattern.test(fragment); const noiseRatio = provenanceNoiseRatio(fragment); const mostlySourceTitle = isMostlySourceTitleFragment(fragment); - if (mostlySourceTitle && !hasConcreteClinicalAction) return false; - if (noiseRatio >= 0.28 && !hasConcreteClinicalAction) return false; - if (isLowYieldClinicalText(fragment) && !hasConcreteClinicalAction) return false; - return hasClinicalSignal || hasConcreteClinicalAction || noiseRatio < 0.16; - }); - const textWithoutNoise = readableWhitespace(fragments.join(" ")); + const droppedByBaseline = + (mostlySourceTitle && !hasConcreteClinicalAction) || + (noiseRatio >= 0.28 && !hasConcreteClinicalAction) || + (isLowYieldClinicalText(fragment) && !hasConcreteClinicalAction); + const keptByBaseline = + !droppedByBaseline && (hasClinicalSignal || hasConcreteClinicalAction || noiseRatio < 0.16); + if (!keptByBaseline && !hasClinicalThresholdSignal) continue; + keptFragments.push(fragment); + if (keptByBaseline) baselineKeptFragments.push(fragment); + } + const textWithoutNoise = readableWhitespace(keptFragments.join(" ")); const clinicalSignalScore = (textWithoutNoise.match(clinicalSignalPattern) ? 1 : 0) + - (textWithoutNoise.match(concreteClinicalActionPattern) ? 1 : 0); - const provenanceScore = provenanceNoiseRatio(textWithoutNoise || cleaned); + (textWithoutNoise.match(concreteClinicalActionPattern) ? 1 : 0) + + // H2: threshold values ARE clinical signal — text kept purely for its + // thresholds must still be classifiable as useful. + (textWithoutNoise.match(clinicalThresholdSignalPattern) ? 1 : 0); + // Diff-review hardening of H2: the provenance score is computed over the + // fragments the BASELINE criteria kept. A noise-dense fragment rescued only + // for its threshold values must not inflate the score past the 0.42 + // usefulness gate — that flipped `useful` to false and made downstream + // callers (rag-answer-text, ward-output) discard text that previously + // survived, including its clean actionable sentences. + const baselineText = readableWhitespace(baselineKeptFragments.join(" ")); + const provenanceScore = provenanceNoiseRatio(baselineText || textWithoutNoise || cleaned); return { text: textWithoutNoise, useful: Boolean(textWithoutNoise && clinicalSignalScore > 0 && provenanceScore < 0.42), diff --git a/src/lib/validation/form-data.ts b/src/lib/validation/form-data.ts index 8116a81295..fd787037b1 100644 --- a/src/lib/validation/form-data.ts +++ b/src/lib/validation/form-data.ts @@ -6,7 +6,13 @@ export function optionalFormText(maxLength: number) { .preprocess( (value) => { if (value === undefined || value === null) return null; - return typeof value === "string" ? value : value; + // Non-string form parts (e.g. a File posted under a text field name) + // pass through UNCHANGED so the union below rejects them: the API + // contract (tests/api-validation-contract.test.ts) requires invalid + // multipart metadata to fail with 400 BEFORE any storage upload or + // database write, never to be silently discarded. (Audit L5: this + // replaces a no-op ternary that obscured the deliberate rejection.) + return value; }, z.union([z.string().trim().max(maxLength), z.null()]), ) diff --git a/src/lib/visual-intelligence.ts b/src/lib/visual-intelligence.ts index 47cc3feed2..7b83b6f6f0 100644 --- a/src/lib/visual-intelligence.ts +++ b/src/lib/visual-intelligence.ts @@ -1,4 +1,4 @@ -import { clinicalVocabularyTerms } from "@/lib/clinical-vocabulary"; +import { clinicalVocabularyMatches, clinicalVocabularyTerms } from "@/lib/clinical-vocabulary"; import type { ImageEvidenceCategory } from "@/lib/types"; export const visualIntelligenceVersion = "visual-intelligence-v1" as const; @@ -398,9 +398,13 @@ export function deterministicStructuredVisualProfile(args: { { clinical_purpose: compact([args.tableTitle, args.tableLabel, args.caption].filter(Boolean).join(" | "), 280), key_terms: clinicalVocabularyTerms(text, 12), - medications: clinicalVocabularyTerms(text, 24).filter((term) => - /clozapine|lithium|olanzapine|lorazepam|haloperidol|diazepam|antipsychotic/i.test(term), - ), + // Audit L6: derive medications from the clinical vocabulary's medication + // category instead of a hardcoded drug allowlist, which silently dropped + // vocabulary drugs (quetiapine, risperidone, promethazine, …) from the + // fallback profile. + medications: clinicalVocabularyMatches(text, 24) + .filter((entry) => entry.type === "medication") + .map((entry) => entry.canonical), thresholds, actions: text.match(/\b(?:withhold|cease|stop|repeat|monitor|review|escalate|continue)[^.|\n]{0,120}/gi) ?? [], monitoring_items: text.match(/\b(?:FBC|ANC|WBC|level|observations?|monitoring|blood test)[^.|\n]{0,80}/gi) ?? [], diff --git a/src/lib/ward-output.ts b/src/lib/ward-output.ts index fc5788150a..3ed208396f 100644 --- a/src/lib/ward-output.ts +++ b/src/lib/ward-output.ts @@ -1,4 +1,5 @@ import { formatCitationLabel } from "@/lib/citations"; +import { normalizeAccessibleTable } from "@/lib/accessible-table-normalization"; import { parseAnswerDisplayContent, type AnswerDisplayGroup } from "@/lib/answer-formatting"; import { clipboardProvenanceLine, @@ -598,16 +599,34 @@ export function buildHighYieldClinicalOutputSections(answer: RagAnswer | null | .filter((section) => section.items.length > 0 || Boolean(section.tables?.length)); } +// Audit H4+M8+M16: the copied ward-note/clipboard table must go through the +// SAME conservative normalization as the on-screen AccessibleTable, so that +// (a) the "verify values against the source document" caveat for tables whose +// structure could not be confidently reconstructed is not silently dropped +// from the pasted artifact (H4), (b) a markdown-parsed header row is never +// re-emitted as the first data row when explicit columns exist (M8), and +// (c) ragged rows are padded so values cannot shift into the wrong column +// (M16). function clinicalTableToTextRows(table: ClinicalThresholdTable) { - const rows = table.rows?.length ? table.rows : parseMarkdownTable(table.markdown); - if (!rows?.length) return []; - const hasColumns = Boolean(table.columns?.length); - const header = hasColumns ? (table.columns ?? []) : rows[0]; - const body = hasColumns ? rows : rows.slice(1); - const visibleBody = body.slice(0, 6); + const explicitRows = table.rows?.length ? table.rows : null; + const parsedRows = explicitRows ? null : parseMarkdownTable(table.markdown); + const rawRows = explicitRows ?? parsedRows; + if (!rawRows?.length) return []; + // Markdown-parsed rows include the header line as row 0; let the normalizer + // treat it as the header instead of trusting table.columns (M8). Explicit + // rows carry no header row, so table.columns is the header there. + const normalized = normalizeAccessibleTable(rawRows, explicitRows ? table.columns : null, { + conservativeClinical: true, + }); + if (!normalized) return []; + const header = normalized.header; + const visibleBody = normalized.body.slice(0, 6); return [ table.caption, + normalized.lowConfidence + ? "Table structure could not be confidently reconstructed — verify values against the source document." + : "", header.length ? `| ${header.join(" | ")} |` : "", header.length ? `| ${header.map(() => "---").join(" | ")} |` : "", ...visibleBody.map((row) => `| ${row.join(" | ")} |`), @@ -777,12 +796,15 @@ function highYieldSectionsForOutput(answer: RagAnswer, bottomLine: string) { } function sectionOutputLines(section: ClinicalOutputSection) { - return [ - section.title, - ...section.items.map((item) => `- ${item}`), - ...(section.tables ?? []).flatMap(clinicalTableToTextRows), - "", - ]; + // Diff-review hardening of audit H4: sections are gated on the RAW table + // shape (tableHasUsableShape), but emission now runs through + // normalizeAccessibleTable, which can reject a table the shape gate passed + // (e.g. header-continuation absorbs the only body row, or all cells are + // dash placeholders). A section whose items are empty and whose tables all + // normalize to nothing must not emit a dangling heading. + const tableLines = (section.tables ?? []).flatMap(clinicalTableToTextRows); + if (section.items.length === 0 && tableLines.length === 0) return []; + return [section.title, ...section.items.map((item) => `- ${item}`), ...tableLines, ""]; } function citationOutputLines(answer: RagAnswer) { diff --git a/supabase/functions/indexing-v3-agent/index.ts b/supabase/functions/indexing-v3-agent/index.ts index 2ef58915ce..254ea19a32 100644 --- a/supabase/functions/indexing-v3-agent/index.ts +++ b/supabase/functions/indexing-v3-agent/index.ts @@ -171,7 +171,27 @@ const TYPE_BUDGET: Record<string, number> = { unclear: 4, }; -function authorizeRequest(req: Request): Response | null { +// Audit L20: constant-time secret comparison. This function runs with +// verify_jwt=false, so the shared secret is the ONLY auth gate; a plain !== +// short-circuits on the first mismatching character and leaks match length +// via response timing. Hashing both sides to fixed-length digests and +// XOR-comparing removes the content-dependent timing signal. +async function timingSafeSecretEqual(candidate: string, expected: string): Promise<boolean> { + const encoder = new TextEncoder(); + const [candidateDigest, expectedDigest] = await Promise.all([ + crypto.subtle.digest("SHA-256", encoder.encode(candidate)), + crypto.subtle.digest("SHA-256", encoder.encode(expected)), + ]); + const candidateBytes = new Uint8Array(candidateDigest); + const expectedBytes = new Uint8Array(expectedDigest); + let difference = 0; + for (let index = 0; index < candidateBytes.length; index += 1) { + difference |= candidateBytes[index] ^ expectedBytes[index]; + } + return difference === 0; +} + +async function authorizeRequest(req: Request): Promise<Response | null> { if (!AGENT_SECRET) { return Response.json( { ok: false, error: "INDEXING_V3_AGENT_SECRET is required when JWT verification is disabled" }, @@ -183,7 +203,7 @@ function authorizeRequest(req: Request): Response | null { const bearer = authorization.match(/^Bearer\s+(.+)$/i)?.[1]?.trim(); const headerSecret = req.headers.get("x-indexing-agent-secret") ?? req.headers.get("x-cron-secret") ?? bearer ?? ""; - if (headerSecret !== AGENT_SECRET) { + if (!(await timingSafeSecretEqual(headerSecret, AGENT_SECRET))) { return Response.json({ ok: false, error: "Unauthorized" }, { status: 401 }); } @@ -416,7 +436,13 @@ async function fetchEmbeddingBatch(texts: string[]): Promise<number[][]> { if (!response.ok) { const body = await response.text(); const error = new Error(`OpenAI embedding request failed (${response.status}): ${body.slice(0, 500)}`); - if (response.status !== 429 && response.status < 500) throw error; + if (response.status !== 429 && response.status < 500) { + // Audit L7: a 4xx client error (bad model name, revoked key) can + // never succeed on retry — tag it so the catch below fails fast + // instead of re-sending the identical doomed request. + (error as Error & { nonRetryable?: boolean }).nonRetryable = true; + throw error; + } lastError = error; } else { const payload = (await response.json()) as { data?: Array<{ embedding?: unknown; index?: number }> }; @@ -434,7 +460,8 @@ async function fetchEmbeddingBatch(texts: string[]): Promise<number[][]> { } } catch (e) { lastError = e; - if (e instanceof Error && e.name !== "AbortError" && attempt >= OPENAI_MAX_RETRIES) throw e; + const nonRetryable = e instanceof Error && (e as Error & { nonRetryable?: boolean }).nonRetryable === true; + if (e instanceof Error && e.name !== "AbortError" && (nonRetryable || attempt >= OPENAI_MAX_RETRIES)) throw e; } finally { clearTimeout(timeout); } @@ -2076,7 +2103,7 @@ Deno.serve({ port: Number(Deno.env.get("PORT") ?? "8000") }, async (req: Request if (req.method !== "POST" && req.method !== "GET") { return new Response("Method not allowed", { status: 405 }); } - const unauthorized = authorizeRequest(req); + const unauthorized = await authorizeRequest(req); if (unauthorized) return unauthorized; const url = new URL(req.url); diff --git a/supabase/migrations/20260702000000_commit_generation_preserve_legacy_artifacts.sql b/supabase/migrations/20260702000000_commit_generation_preserve_legacy_artifacts.sql new file mode 100644 index 0000000000..d8e00b4411 --- /dev/null +++ b/supabase/migrations/20260702000000_commit_generation_preserve_legacy_artifacts.sql @@ -0,0 +1,229 @@ +-- Audit M13 (repo audit 2026-07-01): commit_document_index_generation used to +-- delete legacy NULL-generation rows unconditionally. Rows created before +-- generation tracking existed carry no index_generation_id, and retrieval +-- treats NULL as committed/visible (is_committed_artifact_generation), so a +-- reindex pass that transiently produced no replacement rows for a category +-- (e.g. an image-extraction failure that still committed status='indexed') +-- permanently destroyed the previously-good legacy artifacts with no rollback. +-- +-- This migration recreates the function so legacy NULL-generation rows are +-- purged ONLY when the new generation actually produced replacement rows in +-- the same table. Rows tagged with a different (superseded) generation are +-- still always removed. On the next successful reindex that produces rows, +-- the retained legacy rows are purged as before. +-- +-- Scope of the guarantee (per FK topology): document_images (no chunk FK), +-- document_memory_cards (section_id ON DELETE SET NULL), and +-- document_sections are fully protected. document_table_facts, +-- document_embedding_fields, and document_index_units reference +-- document_chunks(id) ON DELETE CASCADE, so when legacy chunks are replaced +-- (the normal case — the worker inserts chunks before committing) their +-- legacy chunk-anchored rows cascade away with them regardless of these +-- guards; that is structurally required, because retrieval RPCs join those +-- artifacts through a non-null source_chunk_id and an orphaned artifact +-- would be unreachable anyway. The guarded deletes for those three tables +-- protect only rows with source_chunk_id IS NULL. +create or replace function public.commit_document_index_generation( + p_document_id uuid, + p_index_generation_id uuid, + p_status text default 'indexed', + p_page_count integer default 0, + p_chunk_count integer default 0, + p_image_count integer default 0, + p_metadata jsonb default '{}'::jsonb, + p_pages jsonb default null, + p_quality jsonb default null +) +returns jsonb +language plpgsql +set search_path = public, extensions, pg_temp +as $$ +begin + perform set_config('statement_timeout', '180000', true); + + update public.documents + set + status = p_status, + page_count = p_page_count, + chunk_count = p_chunk_count, + image_count = p_image_count, + error_message = null, + metadata = coalesce(p_metadata, '{}'::jsonb) || jsonb_build_object('index_generation_id', p_index_generation_id), + updated_at = now() + where id = p_document_id; + + if p_pages is not null then + delete from public.document_pages + where document_id = p_document_id; + + insert into public.document_pages (document_id, page_number, text, ocr_used, metadata) + select + p_document_id, + page_row.page_number, + coalesce(page_row.text, ''), + coalesce(page_row.ocr_used, false), + coalesce(page_row.metadata, '{}'::jsonb) + from jsonb_to_recordset(coalesce(p_pages, '[]'::jsonb)) as page_row( + page_number integer, + text text, + ocr_used boolean, + metadata jsonb + ) + where page_row.page_number is not null; + end if; + + if p_quality is not null then + insert into public.document_index_quality ( + document_id, + owner_id, + quality_score, + extraction_quality, + metrics, + issues, + updated_at + ) + values ( + p_document_id, + nullif(p_quality->>'owner_id', '')::uuid, + coalesce((p_quality->>'quality_score')::real, 0), + coalesce(nullif(p_quality->>'extraction_quality', ''), 'unknown'), + coalesce(p_quality->'metrics', '{}'::jsonb), + coalesce( + array(select jsonb_array_elements_text(coalesce(p_quality->'issues', '[]'::jsonb))), + '{}'::text[] + ), + now() + ) + on conflict on constraint document_index_quality_pkey + do update set + owner_id = excluded.owner_id, + quality_score = excluded.quality_score, + extraction_quality = excluded.extraction_quality, + metrics = excluded.metrics, + issues = excluded.issues, + updated_at = excluded.updated_at; + end if; + + -- M13: superseded-generation rows always go; legacy NULL-generation rows go + -- only when this generation wrote replacement rows into the same table. + delete from public.document_chunks + where document_id = p_document_id + and ( + (index_generation_id is not null and index_generation_id <> p_index_generation_id) + or ( + index_generation_id is null + and exists ( + select 1 + from public.document_chunks replacement + where replacement.document_id = p_document_id + and replacement.index_generation_id = p_index_generation_id + ) + ) + ); + + delete from public.document_images + where document_id = p_document_id + and ( + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_images replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) + ); + + delete from public.document_table_facts + where document_id = p_document_id + and ( + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_table_facts replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) + ); + + delete from public.document_embedding_fields + where document_id = p_document_id + and ( + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_embedding_fields replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) + ); + + delete from public.document_index_units + where document_id = p_document_id + and ( + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_index_units replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) + ); + + delete from public.document_memory_cards + where document_id = p_document_id + and ( + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_memory_cards replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) + ); + + delete from public.document_sections + where document_id = p_document_id + and ( + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_sections replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) + ); + + return jsonb_build_object( + 'ok', true, + 'document_id', p_document_id, + 'index_generation_id', p_index_generation_id + ); +end; +$$; + +revoke execute on function public.commit_document_index_generation(uuid, uuid, text, integer, integer, integer, jsonb, jsonb, jsonb) from public, anon, authenticated; +grant execute on function public.commit_document_index_generation(uuid, uuid, text, integer, integer, integer, jsonb, jsonb, jsonb) to service_role; diff --git a/supabase/schema.sql b/supabase/schema.sql index 166a982c8d..1544fd87fd 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -1166,50 +1166,122 @@ begin updated_at = excluded.updated_at; end if; + -- M13 (audit 2026-07-01): superseded-generation rows always go; legacy + -- NULL-generation rows go only when this generation wrote replacement rows + -- into the same table (see 20260702000000_commit_generation_preserve_legacy_artifacts). + -- Guarantee scope: fully protects document_images/document_memory_cards/ + -- document_sections; chunk-anchored artifacts (table facts, embedding + -- fields, index units) cascade with their legacy chunks via + -- source_chunk_id ON DELETE CASCADE when chunks are replaced. delete from public.document_chunks where document_id = p_document_id - and (index_generation_id is null or index_generation_id <> p_index_generation_id); + and ( + (index_generation_id is not null and index_generation_id <> p_index_generation_id) + or ( + index_generation_id is null + and exists ( + select 1 + from public.document_chunks replacement + where replacement.document_id = p_document_id + and replacement.index_generation_id = p_index_generation_id + ) + ) + ); delete from public.document_images where document_id = p_document_id and ( - nullif(metadata->>'index_generation_id', '') is null - or metadata->>'index_generation_id' <> p_index_generation_id::text + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_images replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) ); delete from public.document_table_facts where document_id = p_document_id and ( - nullif(metadata->>'index_generation_id', '') is null - or metadata->>'index_generation_id' <> p_index_generation_id::text + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_table_facts replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) ); delete from public.document_embedding_fields where document_id = p_document_id and ( - nullif(metadata->>'index_generation_id', '') is null - or metadata->>'index_generation_id' <> p_index_generation_id::text + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_embedding_fields replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) ); delete from public.document_index_units where document_id = p_document_id and ( - nullif(metadata->>'index_generation_id', '') is null - or metadata->>'index_generation_id' <> p_index_generation_id::text + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_index_units replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) ); delete from public.document_memory_cards where document_id = p_document_id and ( - nullif(metadata->>'index_generation_id', '') is null - or metadata->>'index_generation_id' <> p_index_generation_id::text + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_memory_cards replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) ); delete from public.document_sections where document_id = p_document_id and ( - nullif(metadata->>'index_generation_id', '') is null - or metadata->>'index_generation_id' <> p_index_generation_id::text + (nullif(metadata->>'index_generation_id', '') is not null + and metadata->>'index_generation_id' <> p_index_generation_id::text) + or ( + nullif(metadata->>'index_generation_id', '') is null + and exists ( + select 1 + from public.document_sections replacement + where replacement.document_id = p_document_id + and replacement.metadata->>'index_generation_id' = p_index_generation_id::text + ) + ) ); return jsonb_build_object( diff --git a/tests/answer-verification.test.ts b/tests/answer-verification.test.ts index 2b537995b4..f614fa9975 100644 --- a/tests/answer-verification.test.ts +++ b/tests/answer-verification.test.ts @@ -127,6 +127,45 @@ describe("answer-verification (GEN-C2 / GEN-H2)", () => { expect(verification.unverifiedTokens).toContain("12.5mg"); }); + // H1 (audit 2026-07-01): the verification corpus must include everything the + // model was shown in buildRagSourceBlock. A number living only in the chunk's + // retrieval synopsis or a table-crop image's text previously flagged as + // unverified, blanking a faithful answer. + it("verifies a number that appears only in the retrieval synopsis (H1)", () => { + const result = source({ + content: "See the monitoring summary.", + retrieval_synopsis: "Withhold clozapine when ANC falls below 2.0 ×10⁹/L; restart at 12.5 mg.", + }); + const verification = verifyAnswerNumbers( + "Withhold below 2.0 ×10⁹/L and restart at 12.5 mg.", + [{ chunk_id: "chunk-1" }], + [result], + ); + expect(verification.hasUnverifiedNumbers).toBe(false); + }); + + it("verifies a number that appears only in a cited image's table text (H1)", () => { + const result = source({ + content: "Refer to the threshold table.", + images: [ + { + id: "img-1", + page_number: 4, + storage_path: "images/doc-1/img-1.png", + caption: "Monitoring thresholds", + tableTextSnippet: "Amber: ANC 1.5-2.0, increase monitoring.", + accessibleTableMarkdown: "| Band | ANC | Action |\n| Amber | 1.5-2.0 | Increase monitoring |", + }, + ], + }); + const verification = verifyAnswerNumbers( + "In the amber band (ANC 1.5-2.0) increase monitoring.", + [{ chunk_id: "chunk-1" }], + [result], + ); + expect(verification.hasUnverifiedNumbers).toBe(false); + }); + it("matches numbers in table facts of a cited chunk", () => { const result = source({ content: "See monitoring table.", diff --git a/tests/chunking.test.ts b/tests/chunking.test.ts index baf7528257..f9c5a8ecb0 100644 --- a/tests/chunking.test.ts +++ b/tests/chunking.test.ts @@ -17,6 +17,26 @@ describe("chunkTextWithOverlap", () => { expect(chunks.join(" ")).toContain("Sentence 0."); }); + // M17 (audit 2026-07-01): overlap >= chunkSize previously made the sentence + // window loop spin forever (no forward progress), hanging the worker. + it("terminates when overlap >= chunkSize (M17)", () => { + const text = `${"Withhold clozapine and review blood results. ".repeat(60)}`.trim(); + const chunks = chunkTextWithOverlap(text, 200, 200); + expect(chunks.length).toBeGreaterThan(1); + expect(chunks.join(" ")).toContain("Withhold clozapine"); + }); + + // M14: only standalone page footers are noise. An inline page reference in a + // clinical sentence must never delete the whole line. + it("keeps clinical lines containing inline page references (M14)", () => { + const text = "Give paracetamol, refer to p 3 for dosing.\nPage 3 of 12\nWithhold clozapine if ANC is low."; + const chunks = chunkTextWithOverlap(text, 2000, 200); + const joined = chunks.join(" "); + expect(joined).toContain("refer to p 3 for dosing"); + expect(joined).toContain("Withhold clozapine"); + expect(joined).not.toMatch(/Page 3 of 12/); + }); + it("prefers paragraph boundaries before falling back to sentence windows", () => { const chunks = chunkTextWithOverlap("Heading\n\nFirst clinical paragraph.\n\nSecond clinical paragraph.", 32, 4); diff --git a/tests/clinical-search.test.ts b/tests/clinical-search.test.ts index 901ef163b6..bf6f39fb02 100644 --- a/tests/clinical-search.test.ts +++ b/tests/clinical-search.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { analyzeClinicalQuery, buildClinicalTextSearchQuery, + classifyQueryIntent, classifyRagQuery, clinicalRankExplanation, expandClinicalQuery, @@ -36,6 +37,15 @@ describe("clinical search query normalization", () => { it("classifies common RAG query shapes for routing and observability", () => { expect(classifyRagQuery("Find the NOCC document").queryClass).toBe("document_lookup"); expect(classifyRagQuery("What should a patient safety plan include?").queryClass).toBe("document_lookup"); + // M2/M3 (audit 2026-07-01): intent signals match at word boundaries, and + // explicit dose vocabulary survives escalation-word cancellation. + expect(classifyQueryIntent("What is the time limit for a notable review?").hasDosingSignals).toBe(false); + expect(classifyQueryIntent("What is the time limit for a notable review?").imageEvidenceFocus).toBe(false); + expect(classifyQueryIntent("clozapine dose review schedule").hasDosingSignals).toBe(true); + expect(classifyQueryIntent("What IM options are listed for agitation?").hasDosingSignals).toBe(true); + expect(classifyQueryIntent("clozapine 100mg starting schedule").hasDosingSignals).toBe(true); + expect(classifyQueryIntent("Show the monitoring table for lithium").imageEvidenceFocus).toBe(true); + expect(classifyRagQuery("What forms are required for a patient safety plan?").queryClass).toBe("document_lookup"); expect(classifyRagQuery("What are NOCC requirements?").queryClass).toBe("document_lookup"); expect(classifyRagQuery("What assessment documentation is required?").queryClass).toBe("document_lookup"); diff --git a/tests/deep-memory.test.ts b/tests/deep-memory.test.ts index bd002a9c23..ca5e7af7f0 100644 --- a/tests/deep-memory.test.ts +++ b/tests/deep-memory.test.ts @@ -234,7 +234,11 @@ describe("deep RAG memory indexing", () => { expect(boosted[0].hybrid_score).toBeGreaterThan(0.7); }); - it("keeps memory cards when committed-generation metadata lookup fails", async () => { + // L8 (audit 2026-07-01): fail CLOSED. When the documents lookup errors we + // cannot verify which reindex generation is committed, so the fallback must + // drop its cards for that query rather than risk injecting content from an + // abandoned/superseded generation into a clinical answer. + it("drops fallback memory cards when committed-generation metadata lookup fails", async () => { const cards = [ { id: "card-new", @@ -295,7 +299,7 @@ describe("deep RAG memory indexing", () => { matchCount: 8, }); - expect(result.map((card) => card.id)).toEqual(["card-new"]); + expect(result).toEqual([]); }); it("persists memory cards without leaking internal section indexes into inserts", async () => { diff --git a/tests/document-organization.test.ts b/tests/document-organization.test.ts index bcba9a0011..873ad82c46 100644 --- a/tests/document-organization.test.ts +++ b/tests/document-organization.test.ts @@ -60,6 +60,30 @@ describe("document organization classifier", () => { expect(classification.profile.secondary_facets.service).toEqual(["kara maar"]); }); + // M6 (audit 2026-07-01): a site named in PLAIN TEXT in the title must be + // detected even when the body never repeats it. + it("detects a site named only in the title text (M6)", () => { + const classification = classifyDocumentOrganization({ + title: "Fiona Stanley Hospital Sepsis Pathway", + file_name: "sepsis-pathway.pdf", + contentText: "Escalate suspected sepsis immediately.", + }); + + expect(classification.profile.site.label).toBe("Fiona Stanley Hospital"); + }); + + // M7: the generic phrase "best practice" must not attribute a local policy + // to the BMJ Best Practice reference collection. + it("does not attribute BMJ on the bare phrase 'best practice' (M7)", () => { + const classification = classifyDocumentOrganization({ + title: "Local Escalation Policy", + file_name: "local-escalation-policy.pdf", + contentText: "In line with best practice, staff should escalate concerns to the senior clinician.", + }); + + expect(classification.profile.site.label).not.toBe("BMJ Best Practice"); + }); + it("maps health-service and program tags conservatively", () => { expect( classifyDocumentOrganization({ diff --git a/tests/image-filtering.test.ts b/tests/image-filtering.test.ts index c4340f7028..b6c774efc1 100644 --- a/tests/image-filtering.test.ts +++ b/tests/image-filtering.test.ts @@ -156,7 +156,10 @@ describe("smart image filtering", () => { }); it("builds a stable lightweight perceptual hash key", () => { - expect(lightweightPerceptualHash("1234567890abcdef", 100, 200)).toMatch(/^ph1:100x200:[0-9a-f]{4}$/); + // M12 (audit 2026-07-01): ph2 digest — 16 hex threshold bits + 32 hex + // quantized-level bits; ph1's 4-hex space collided across distinct + // same-dimension clinical tables. + expect(lightweightPerceptualHash("1234567890abcdef", 100, 200)).toMatch(/^ph2:100x200:[0-9a-f]{48}$/); }); it("groups identical sampled image bytes but separates different bytes", () => { diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index 723c76e78d..23b55cbee8 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -1931,30 +1931,35 @@ describe("private document API access", () => { expect(client.storageMocks.remove).not.toHaveBeenCalled(); }); - it("blocks permanent delete while a document is actively indexing", async () => { - const client = createSupabaseMock((call) => { - if (call.table === "documents" && call.operation === "select") { - return ok({ id: documentId, owner_id: userId, title: "Owned", storage_path: "source.pdf" }); - } - if (call.table === "ingestion_jobs" && call.operation === "select") { - return ok([{ id: "job-1", status: "processing" }]); - } - return ok([]); - }); - mockRuntime(client); - const { DELETE } = await import("../src/app/api/documents/[id]/route"); + // M9 (audit 2026-07-01): the guard covers PENDING jobs too — a just-queued + // reindex racing a delete used to orphan freshly-uploaded storage objects. + it.each(["processing", "pending"] as const)( + "blocks permanent delete while a document has %s indexing work", + async (jobStatus) => { + const client = createSupabaseMock((call) => { + if (call.table === "documents" && call.operation === "select") { + return ok({ id: documentId, owner_id: userId, title: "Owned", storage_path: "source.pdf" }); + } + if (call.table === "ingestion_jobs" && call.operation === "select") { + return ok([{ id: "job-1", status: jobStatus }]); + } + return ok([]); + }); + mockRuntime(client); + const { DELETE } = await import("../src/app/api/documents/[id]/route"); - const response = await DELETE(authenticatedRequest(`/api/documents/${documentId}`, { method: "DELETE" }), { - params: Promise.resolve({ id: documentId }), - }); + const response = await DELETE(authenticatedRequest(`/api/documents/${documentId}`, { method: "DELETE" }), { + params: Promise.resolve({ id: documentId }), + }); - expect(response.status).toBe(409); - expect(await payload(response)).toEqual({ - error: "Document is currently indexing. Stop or wait for the worker before deleting.", - }); - expect(client.calls.some((call) => call.table === "documents" && call.operation === "delete")).toBe(false); - expect(client.storageMocks.remove).not.toHaveBeenCalled(); - }); + expect(response.status).toBe(409); + expect(await payload(response)).toEqual({ + error: "Document has pending or processing indexing work. Stop or wait for the worker before deleting.", + }); + expect(client.calls.some((call) => call.table === "documents" && call.operation === "delete")).toBe(false); + expect(client.storageMocks.remove).not.toHaveBeenCalled(); + }, + ); it("rejects unauthenticated search and answer requests", async () => { const searchChunksWithTelemetry = vi.fn(async () => ({ diff --git a/tests/retrieval-selection.test.ts b/tests/retrieval-selection.test.ts index 46617064f1..1d730fcb09 100644 --- a/tests/retrieval-selection.test.ts +++ b/tests/retrieval-selection.test.ts @@ -45,6 +45,38 @@ function sourceMetadata( } describe("retrieval source selection", () => { + // Audit H3 disposition (2026-07-02): SUPERSEDED by PR #118, which removed + // source-governance metadata weighting from retrieval selection entirely — + // measured on the golden retrieval eval (doc-recall@5 1.0 -> 0.76 with + // weighting). There are no freshness/validation penalties in selection to + // propagate; governance is enforced by ranking penalties and the + // answer/source-governance layer. See the amended governance contract test + // below ("keeps relevance ordering ..."). + + // L4: stacked boosts must never push the annotated score above 1.0. + it("clamps the annotated hybrid_score to at most 1.0 (L4)", () => { + const selection = selectRetrievalEvidence({ + query: "What IM or PO options are listed for agitation?", + queryClass: "medication_dose_risk", + topK: 2, + maxResultsPerDocument: 2, + results: [ + source({ + id: "high-base", + title: "Agitation Medication Chart", + content: "IM and PO options for agitation: olanzapine 5-10 mg PO, droperidol 5 mg IM.", + hybrid_score: 0.98, + source_metadata: sourceMetadata(), + match_explanation: { titleHit: true, contentHit: true, reasons: ["title"] }, + }), + ], + }); + + const annotated = selection.results.find((result) => result.id === "high-base"); + expect(annotated).toBeDefined(); + expect(annotated!.hybrid_score).toBeLessThanOrEqual(1); + }); + it("rescues active-community ED document evidence above generic community hits", () => { const selection = selectRetrievalEvidence({ query: "How are active community patients in ED managed?", @@ -278,7 +310,13 @@ describe("retrieval source selection", () => { ); }); - it("prefers current locally reviewed clozapine threshold evidence over close review-required sources", () => { + // Contract changed 2026-07-02 (measured): source-governance metadata must NOT reorder retrieval + // selection. The corpus is only partially enriched — unenriched documents normalize to + // unknown/unverified — so metadata weighting in selection buried correct documents on the golden + // retrieval eval (doc-recall@5 1.0 -> 0.76, 7/23 failures). Selection orders by relevance + // (clamped score -> lexical -> rerank); governance is enforced by ranking penalties and the + // answer/source-governance layer instead (RC8 tracked in docs/rag-hybrid-findings-and-todo.md). + it("keeps relevance ordering and does not let source-governance metadata reorder selection", () => { const selection = selectRetrievalEvidence({ query: "What ANC or FBC threshold should withhold clozapine?", queryClass: "table_threshold", @@ -328,16 +366,15 @@ describe("retrieval source selection", () => { }); expect(selection.results).toHaveLength(5); + // Relevance (hybrid) order is preserved; the review-due and unverified sources are NOT demoted + // here — their governance state is surfaced/penalised by the ranking and answer layers. expect(selection.results.map((result) => result.id)).toEqual([ + "review-due-shared-care", + "current-unverified-bmj", "current-local-fsh", "current-local-nmhs", "current-local-akg", - "current-local-camhs", - "current-local-smhs", ]); - expect( - selection.results.every((result) => result.source_metadata?.clinical_validation_status === "locally_reviewed"), - ).toBe(true); }); it("prefers risk/red-zone flowchart evidence over generic flowchart evidence", () => { diff --git a/tests/source-text-sanitizer.test.ts b/tests/source-text-sanitizer.test.ts index 0d1192f7c9..356ffcbd0e 100644 --- a/tests/source-text-sanitizer.test.ts +++ b/tests/source-text-sanitizer.test.ts @@ -108,6 +108,30 @@ describe("source text sanitizer", () => { expect(usefulness.useful).toBe(true); }); + // H2 (audit 2026-07-01): a sentence carrying clinical threshold values must + // survive even when it starts near a title keyword (Scale/Guideline/…) and + // has no concrete-action verb — the greedy title-fragment match previously + // deleted it wholesale. + it("keeps threshold sentences that resemble source-title fragments (H2)", () => { + const text = + "Assess the patient on admission. The Glasgow Coma Scale ranges from 3 to 15 with 8 or below indicating severe head injury. Document the score."; + + const usefulness = clinicalProseUsefulness(text); + + expect(usefulness.text).toContain("ranges from 3 to 15"); + expect(usefulness.text).toContain("8 or below"); + expect(usefulness.text).toContain("Assess the patient on admission"); + }); + + it("still drops bare-integer title noise like 'Guideline Appendix 1' (H2 guard)", () => { + const noisy = + "Dose evidence: LUNSERS (Liverpool University Neuroleptic Side Effect Rating Scale) - using for monitoring Neuroleptic side effect Guideline Appendix 1."; + + const usefulness = clinicalProseUsefulness(noisy); + + expect(usefulness.text).not.toContain("Liverpool University"); + }); + it("scores document-control snippets as low yield without hiding document viewer provenance", () => { const text = "Neuroleptic side effect Guideline PAE-PRO-0338/16 Page 5 of 5"; diff --git a/tests/ward-output.test.ts b/tests/ward-output.test.ts index 63e97bc90f..d133a25a30 100644 --- a/tests/ward-output.test.ts +++ b/tests/ward-output.test.ts @@ -209,6 +209,132 @@ describe("ward output helpers", () => { expect(thresholds?.items[0]).toContain("Withhold clozapine"); }); + // H4/M8/M16 (audit 2026-07-01): copied ward-note tables must go through the + // same conservative normalization as the on-screen AccessibleTable. + it("carries the low-confidence caveat into the copied table output (H4)", () => { + const thresholdAnswer: RagAnswer = { + ...answer, + answer: "Withhold clozapine if ANC is below the required threshold and urgently review.", + answerSections: [ + { + heading: "Threshold", + body: "Withhold clozapine if ANC is below the required threshold and urgently review.", + citation_chunk_ids: ["chunk-1"], + }, + ], + visualEvidence: [ + { + id: "image-1", + image_id: "image-1", + signed_url_endpoint: "/api/images/image-1/signed-url", + caption: "FBC/ANC monitoring thresholds", + document_id: "doc-1", + title: "Clozapine source", + file_name: "clozapine.pdf", + page_number: 2, + source_chunk_id: "chunk-1", + chunk_index: 0, + viewer_href: "/documents/doc-1?page=2&chunk=chunk-1", + tableLabel: "Table 1", + tableTitle: "FBC/ANC thresholds", + // Interleaved generic column on a clinical table triggers the + // conservative raw-grid fallback (ambiguous_generic_column). + tableRows: [ + ["Below 1.5", "withhold dose", "contact prescriber"], + ["1.5-2.0", "increase monitoring", "review threshold"], + ], + tableColumns: ["ANC level", "", "Action"], + }, + ], + }; + + const copy = formatAnswerForClipboard(thresholdAnswer); + expect(copy).toContain("verify values against the source document"); + }); + + it("does not duplicate the markdown header as a data row (M8)", () => { + const thresholdAnswer: RagAnswer = { + ...answer, + answer: "Withhold clozapine if ANC is below the required threshold and urgently review.", + answerSections: [ + { + heading: "Threshold", + body: "Withhold clozapine if ANC is below the required threshold and urgently review.", + citation_chunk_ids: ["chunk-1"], + }, + ], + visualEvidence: [ + { + id: "image-1", + image_id: "image-1", + signed_url_endpoint: "/api/images/image-1/signed-url", + caption: "FBC/ANC monitoring thresholds", + document_id: "doc-1", + title: "Clozapine source", + file_name: "clozapine.pdf", + page_number: 2, + source_chunk_id: "chunk-1", + chunk_index: 0, + viewer_href: "/documents/doc-1?page=2&chunk=chunk-1", + tableLabel: "Table 1", + tableTitle: "FBC/ANC thresholds", + tableRows: null, + tableColumns: ["ANC", "Action"], + accessibleTableMarkdown: + "| ANC | Action |\n| --- | --- |\n| Below 1.5 | Withhold clozapine |\n| 1.5-2.0 | Increase monitoring |", + }, + ], + }; + + const copy = formatAnswerForClipboard(thresholdAnswer); + const headerOccurrences = copy.split("| ANC | Action |").length - 1; + expect(headerOccurrences).toBe(1); + expect(copy).toContain("| Below 1.5 | Withhold clozapine |"); + }); + + it("pads ragged rows so values cannot shift columns (M16)", () => { + const thresholdAnswer: RagAnswer = { + ...answer, + answer: "Withhold clozapine if ANC is below the required threshold and urgently review.", + answerSections: [ + { + heading: "Threshold", + body: "Withhold clozapine if ANC is below the required threshold and urgently review.", + citation_chunk_ids: ["chunk-1"], + }, + ], + visualEvidence: [ + { + id: "image-1", + image_id: "image-1", + signed_url_endpoint: "/api/images/image-1/signed-url", + caption: "FBC/ANC monitoring thresholds", + document_id: "doc-1", + title: "Clozapine source", + file_name: "clozapine.pdf", + page_number: 2, + source_chunk_id: "chunk-1", + chunk_index: 0, + viewer_href: "/documents/doc-1?page=2&chunk=chunk-1", + tableLabel: "Table 1", + tableTitle: "FBC/ANC thresholds", + // Second row has an extra trailing cell (ragged extraction). + tableRows: [ + ["Below 1.5", "withhold dose"], + ["1.5-2.0", "increase monitoring", "contact prescriber daily"], + ], + tableColumns: ["Level", "Action"], + }, + ], + }; + + const copy = formatAnswerForClipboard(thresholdAnswer); + // The 3-cell row must not render as a 3-column line under a 2-column + // header; the trailing cell merges into the nearest named column. + expect(copy).not.toContain("| 1.5-2.0 | increase monitoring | contact prescriber daily |"); + expect(copy).toContain("increase monitoring contact prescriber daily"); + }); + it("does not promote nearby table evidence for unsupported answers", () => { const unsupportedAnswer: RagAnswer = { ...answer, diff --git a/worker/main.ts b/worker/main.ts index 06fc57d325..4718f1b2ca 100644 --- a/worker/main.ts +++ b/worker/main.ts @@ -438,7 +438,25 @@ async function replacePageRows(documentId: string, pages: ReturnType<typeof buil await insertPageRows(pages); } +// Audit M13 (mirrors 20260702000000_commit_generation_preserve_legacy_artifacts): +// this client-side fallback (used only when the commit RPC is missing) must +// apply the same preservation rule as the RPC — legacy generationless rows are +// purged only when this generation wrote replacement rows into the same +// table, so a transient artifact-write failure cannot destroy the +// previously-good legacy artifacts. Rows tagged with a DIFFERENT generation +// are always removed. Note: chunk-anchored artifacts (table facts, embedding +// fields, index units) cascade with their legacy chunks regardless — the +// guarantee fully protects images, memory cards, and sections. async function deleteStaleIndexGenerationRows(documentId: string, indexGenerationId: string) { + const hasReplacementRows = async (table: string, direct: boolean) => { + let query = supabase.from(table).select("id").eq("document_id", documentId).limit(1); + query = direct + ? query.eq("index_generation_id", indexGenerationId) + : query.eq("metadata->>index_generation_id", indexGenerationId); + const { data, error } = await query; + if (error) throw supabaseStageError(`check replacement ${table}`, error); + return (data ?? []).length > 0; + }; const deleteDirectGenerationRows = async (table: string) => { const stale = await supabase .from(table) @@ -446,6 +464,7 @@ async function deleteStaleIndexGenerationRows(documentId: string, indexGeneratio .eq("document_id", documentId) .neq("index_generation_id", indexGenerationId); if (stale.error) throw supabaseStageError(`delete stale ${table}`, stale.error); + if (!(await hasReplacementRows(table, true))) return; const missing = await supabase.from(table).delete().eq("document_id", documentId).is("index_generation_id", null); if (missing.error) throw supabaseStageError(`delete generationless ${table}`, missing.error); }; @@ -456,6 +475,7 @@ async function deleteStaleIndexGenerationRows(documentId: string, indexGeneratio .eq("document_id", documentId) .neq("metadata->>index_generation_id", indexGenerationId); if (stale.error) throw supabaseStageError(`delete stale ${table}`, stale.error); + if (!(await hasReplacementRows(table, false))) return; const missing = await supabase .from(table) .delete() @@ -783,6 +803,11 @@ async function uploadAndCaptionImages( let skippedImages = 0; const skipReasons = new Map<string, number>(); const imageTypeCounts = new Map<string, number>(); + // Deliberate trade-off (audit L11): image bytes are re-read from disk at + // each stage (hash here, caption on cache miss, upload) instead of being + // cached, because holding every extracted image Buffer for a large document + // (hundreds of multi-MB page images) would multiply the worker's peak + // memory. Disk I/O is the cheaper resource for this background pipeline. const preparedImages: Array<{ imageHash: string; bytesLength: number; perceptualHash: string }> = []; for (const image of extracted.images) { const bytes = await readFile(image.path); From d4c00906282985224235b72060a438a4d2679ea8 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 12:08:26 +0800 Subject: [PATCH 2/2] style: apply prettier to files drifted on both branch and main format:check now passes repo-wide (CI does not gate on it, so drift had accumulated on main as well; includes the audit report and merge-touched sources). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --- docs/audit/repo-audit-2026-07-01.md | 146 +++++++++++++++------------ docs/rag-hybrid-findings-and-todo.md | 35 +++---- src/components/ClinicalDashboard.tsx | 4 +- src/lib/rag-provider.ts | 7 +- src/lib/rag.ts | 18 ++-- src/lib/source-text-sanitizer.ts | 3 +- tests/answer-prose-runons.test.ts | 4 +- tests/openai-error-mapping.test.ts | 5 +- tests/rag-content-accuracy.test.ts | 5 +- tests/rag-provider.test.ts | 4 +- tests/supabase-schema.test.ts | 8 +- 11 files changed, 126 insertions(+), 113 deletions(-) diff --git a/docs/audit/repo-audit-2026-07-01.md b/docs/audit/repo-audit-2026-07-01.md index ef6fd02a46..1b5175fe1e 100644 --- a/docs/audit/repo-audit-2026-07-01.md +++ b/docs/audit/repo-audit-2026-07-01.md @@ -7,14 +7,14 @@ ## Result summary -| | Count | -|---|---| -| **Total findings (post-verification)** | **41** | -| CONFIRMED | 37 | -| PLAUSIBLE | 4 | -| By severity | High **4**, Medium **17**, Low **20** | -| By category | correctness 17 · data-integrity 8 · privacy/clinical 6 · quality/simplification 6 · performance 2 · security 2 | -| Flagged safe-cleanup by finders | 17 (only 2 are *truly* no-behavior-change — see Auto-fixes) | +| | Count | +| -------------------------------------- | -------------------------------------------------------------------------------------------------------------- | +| **Total findings (post-verification)** | **41** | +| CONFIRMED | 37 | +| PLAUSIBLE | 4 | +| By severity | High **4**, Medium **17**, Low **20** | +| By category | correctness 17 · data-integrity 8 · privacy/clinical 6 · quality/simplification 6 · performance 2 · security 2 | +| Flagged safe-cleanup by finders | 17 (only 2 are _truly_ no-behavior-change — see Auto-fixes) | Finders raised more; the numbers above are what **survived adversarial verification**. Verifiers also corrected several severities/categories (e.g. the perceptual-hash and edge-function-secret findings were downgraded from High; the sanitizer finding was re-categorised to data-integrity). @@ -25,25 +25,33 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s ## High severity (4) — clinical answer correctness ### H1 · Numeric faithfulness gate blanks correct answers whose numbers live in synopsis/image text + `src/lib/answer-verification.ts:90` · correctness · CONFIRMED · safe_cleanup=false -- **Defect:** `sourceTextForResult()` builds the numeric-verification corpus from `content / adjacent_context / section_heading / table_facts / memory_cards / index_unit` but omits `result.retrieval_synopsis` and image text (`tableTextSnippet`, `accessibleTableMarkdown`) — even though `buildRagSourceBlock` (`rag.ts:5610-5647`) *shows both* to the model. + +- **Defect:** `sourceTextForResult()` builds the numeric-verification corpus from `content / adjacent_context / section_heading / table_facts / memory_cards / index_unit` but omits `result.retrieval_synopsis` and image text (`tableTextSnippet`, `accessibleTableMarkdown`) — even though `buildRagSourceBlock` (`rag.ts:5610-5647`) _shows both_ to the model. - **Failure:** For a `medication_dose_risk`/`table_threshold` query whose evidence is a visual/table chunk, the real dose/threshold (e.g. `12.5 mg`, `ANC 2.0 x10^9/L`) exists only in `retrieval_synopsis`/image text. The model copies it faithfully, but `verifyAnswerNumbers` marks it unverified; `applyNumericVerification` (`rag.ts:5798-5808`) then discards the whole correct answer and returns a generic "review the source passages" non-answer. - **Fix:** Include `retrieval_synopsis` + image text in `sourceTextForResult`, mirroring `sourceTextForQuoteVerification` (`rag.ts:880-897`), so the verifier scans the same corpus the model was given. ### H2 · Sanitizer drops clinical threshold sentences misclassified as source-title fragments + `src/lib/source-text-sanitizer.ts:230` (pattern at `:29-30`) · data-integrity · CONFIRMED · safe_cleanup=false + - **Defect:** `clinicalProseUsefulness()` discards any sentence starting near a title keyword (`Guideline/Procedure/Protocol/Policy/Appendix/Scale`) that lacks a concrete-action verb — the `sourceTitleFragmentPattern` regex greedily consumes up to 180 trailing chars (`…Scale\b[^.;]{0,180}`). -- **Failure (reproduced by the agent running the real function):** input *"Assess the patient on admission. The Glasgow Coma Scale ranges from 3 to 15 with 8 or below indicating severe head injury. Document the score."* → returns *"Assess the patient on admission. Document the score."* The GCS threshold sentence is silently removed and the truncated text still wins at `rag-answer-text.ts:150` (`usefulness.text || finalText`). +- **Failure (reproduced by the agent running the real function):** input _"Assess the patient on admission. The Glasgow Coma Scale ranges from 3 to 15 with 8 or below indicating severe head injury. Document the score."_ → returns _"Assess the patient on admission. Document the score."_ The GCS threshold sentence is silently removed and the truncated text still wins at `rag-answer-text.ts:150` (`usefulness.text || finalText`). - **Fix:** Don't drop a fragment on title/noise grounds when it contains a clinical numeric/threshold token; anchor `sourceTitleFragmentPattern` so it can't swallow trailing clinical prose. ### H3 · Freshness/validation penalties are silently discarded from the final score + `src/lib/retrieval-selection.ts:473` · privacy/clinical · CONFIRMED · safe_cleanup=false -- **Defect:** `annotateResultWithSelection` writes back `Math.max(originalScore, candidate.score)` as `hybrid_score`, so any net-negative `resultBoost` (outdated −0.24, review_due −0.12, unverified −0.08, poor-extraction −0.12) never lowers the score consumers see — the floor is meant to let intent "rescue" *raise* a score, but it also blocks all penalties. + +- **Defect:** `annotateResultWithSelection` writes back `Math.max(originalScore, candidate.score)` as `hybrid_score`, so any net-negative `resultBoost` (outdated −0.24, review_due −0.12, unverified −0.08, poor-extraction −0.12) never lowers the score consumers see — the floor is meant to let intent "rescue" _raise_ a score, but it also blocks all penalties. - **Failure:** An `outdated` chunk with base 0.70 → ~0.46 after penalties gets floored back to 0.70. `evaluateEvidenceCoverageGate` (`rag.ts:3233`, gate ~0.6) then treats the stale guideline as strong and presents it with high confidence instead of demoting it. - **Fix:** Don't floor at the original when `resultBoost` is negative — persist the clamped boosted score so freshness/validation penalties reach coverage gating. ### H4 · Ward-note / clipboard table export bypasses the low-confidence safeguard + `src/lib/ward-output.ts:601` · privacy/clinical · CONFIRMED · safe_cleanup=false + - **Defect:** `clinicalTableToTextRows` renders threshold tables straight from raw card rows (`parseMarkdownTable`) and never calls `normalizeAccessibleTable`, so the on-screen "Table structure could not be confidently reconstructed — verify values against the source document" caveat (`AccessibleTable` / `accessible-table-normalization.ts`) is dropped from the copied ward note. - **Failure:** A clozapine monitoring table flagged `lowConfidence` on screen is copied via `formatWardNote`/`formatAnswerForClipboard` with no caveat; a clinician pastes a mis-paired dose grid into the record and trusts it. - **Fix:** Route ward-output tables through `normalizeAccessibleTable` (conservativeClinical) and emit the same caveat line when `lowConfidence`. @@ -53,6 +61,7 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s ## Medium severity (17) **Answer/retrieval correctness** + - **M1** `src/lib/rag.ts:647` · `deriveConfidence` takes `max(similarity)` over **all** retrieved chunks, not the cited ones — an uncited high-similarity chunk inflates a weakly-cited answer to high confidence. CONFIRMED. - **M2** `src/lib/clinical-search.ts:760` · `classifyQueryIntent` uses `containsAny` (substring, not word-boundary) on short tokens (`im`, `po`, `table`, `flow`) → `"time limit"` trips dosing, `"notable"` trips image focus, perturbing ranking (+0.09 dosing / −0.04 image). CONFIRMED. - **M3** `src/lib/clinical-search.ts:768` · escalation substring match (`review`, `risk`, `rapid`) cancels a legitimate dosing signal — `"clozapine dose review schedule"` loses its dosing boost. CONFIRMED. @@ -63,6 +72,7 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s - **M8** `src/lib/ward-output.ts:606` · when `rows` is null but `columns`+`markdown` present, the parsed markdown **header row is re-emitted as the first data row** (no `slice(1)`). CONFIRMED. **Data integrity** + - **M9** `src/app/api/documents/[id]/route.ts:503` · DELETE active-job guard checks only `status='processing'`, missing `pending`; a just-queued reindex racing a delete orphans freshly-uploaded storage objects. Reindex routes correctly use `checkIngestionMutationSafety` (`in ['pending','processing']`). CONFIRMED. - **M10** `src/components/ClinicalDashboard.tsx:6533` · `executeSearch`/`applySearchResult` have no request-token/abort guard → out-of-order responses can show query A's answer under query B's question (bootstrap auto-search + user submit). CONFIRMED. - **M11** `src/lib/deep-memory.ts:644` · `upsertDocumentDeepMemory` deletes memory cards/sections/index-units **non-atomically** before rebuild; an OpenAI/insert failure after the deletes wipes memory while metadata still advertises the old version → silent retrieval degradation until full reindex. CONFIRMED. @@ -70,11 +80,13 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s - **M13** `supabase/migrations/20260628000000_atomic_reindex_generation_commit.sql:120` · `commit_document_index_generation` deletes NULL-generation (legacy) artifact rows, which retrieval treats as committed/visible; a transient artifact-write failure that still commits `status='indexed'` permanently loses the previously-good legacy artifacts. CONFIRMED. **Privacy / clinical** + - **M14** `src/lib/chunking.ts:12` · `lineNoisePatterns[0]` matches unanchored `p N`/`page N` anywhere in a line and `removePageNoise` drops the whole line → clinical lines like "refer to p 3 for dosing" are deleted before indexing. CONFIRMED. - **M15** `src/lib/query-privacy.ts:8` · redacted mode stores an **unsalted SHA-256** of the normalized query as the placeholder and `query_hash`; short low-entropy clinical queries are dictionary-reversible and cross-row correlatable. Fix: HMAC with a server-side secret. CONFIRMED. - **M16** `src/lib/ward-output.ts:611` · header/body cell-count mismatch — a ragged row with more cells than columns emits misaligned markdown, pairing the wrong threshold with the wrong action in the copied note. CONFIRMED. **Correctness (added 2026-07-02 — omitted from the first draft of this report; the workflow produced 17 Medium findings but only 16 were listed)** + - **M17** `src/lib/chunking.ts:320` · `chunkTextBySentence` makes no forward progress when `CHUNK_OVERLAP >= CHUNK_SIZE` (both pass env validation independently): `readableOverlapStart` returns a start ≤ the previous start and the `while` loop spins forever, hanging the ingestion worker on that document. CONFIRMED (reproduced by trace). --- @@ -82,6 +94,7 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s ## Low severity (20) **Correctness** + - **L1** `scripts/purge-query-logs.ts:25` · unknown flag silently consumes the next arg (typo'd `--owner-emial` swallows the email, then purges the env-configured owner's logs). CONFIRMED. - **L2** `scripts/recover-ingestion-queue.ts:49` · typo'd `--limit` → NaN → silently defaults to 20 (mutates up to 20 jobs instead of the intended cap). CONFIRMED. - **L3** `src/lib/evidence-relevance.ts:347` · aggregate `nearby` branch uses `|| results.length > 0` (always true) → the `none` verdict is dead; zero-term-match sets are labelled `nearby`, overstating evidence in governance/telemetry. CONFIRMED. @@ -91,17 +104,21 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s - **L7** `supabase/functions/indexing-v3-agent/index.ts:437` · non-retryable OpenAI 4xx (bad model / revoked key) is retried `OPENAI_MAX_RETRIES` times instead of failing fast. CONFIRMED. **Data integrity** + - **L8** `src/lib/deep-memory.ts:865` · committed-generation filter fails open (`if (documentsError) return true`) — a transient documents-table error during a staged reindex can expose superseded-generation cards. PLAUSIBLE (narrow race). - **L9** `worker/main.ts:1406` · `imageCount` (→ `p_image_count`) counts searchable-only rows, excluding audit-retained images that were still inserted; reconciliation tooling may see a mismatch. PLAUSIBLE (likely intentional). **Performance** + - **L10** `src/app/api/search/route.ts:666` · `buildEvidenceRelevance` runs ≥2× and `buildVisualEvidence` ≥3× per search over the full result set, and the smart-panel relevance is then overwritten — wasted CPU on the hot path. CONFIRMED. - **L11** `worker/main.ts:997` · each image is `readFile`'d up to 3× per ingestion (hash, caption cache-miss, upload) → 3× disk I/O + peak memory. CONFIRMED. **Privacy / clinical** + - **L12** `src/lib/privacy.ts:6` · `redactLogValue` returns non-string values unchanged, so an error whose `details`/`code`/`hint` is an object is logged verbatim (potential URL/email/PHI leak). CONFIRMED. **Quality / simplification** + - **L13** `next.config.ts:31` · CSP `connect-src` lists both the explicit Supabase host and `https://*.supabase.co` (redundant). CONFIRMED. → **auto-fixed** (see below). - **L14** `src/lib/answer-ranking.ts:~324` (reported as `answer-formatting.ts:327`) · `capBoldSegments` lets newly-bolded tokens that collide with pre-existing bold text bypass the max-segment cap. CONFIRMED. - **L15** `src/lib/cross-document-synthesis.ts:195` · dead ternary `queryClass === "comparison" ? 2 : 2`. CONFIRMED. → **auto-fixed**. @@ -110,6 +127,7 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s - **L18** `supabase/migrations/20260630090000_audit_logs_service_role_policy.sql:10` · re-declares the identical `audit logs service role all` policy already created in `20260629110000_audit_logs.sql` — drift risk on replay. CONFIRMED. (do **not** edit an applied migration; recommendation only) **Security (defence-in-depth)** + - **L19** `next.config.ts:7` · production CSP allows `script-src 'unsafe-inline'` with no nonce/hash. PLAUSIBLE — factually true, but the app has **zero** `dangerouslySetInnerHTML` sinks (React auto-escapes doc text + LLM answers), so no active XSS path today. Recommend nonce-based CSP as hardening. - **L20** `supabase/functions/indexing-v3-agent/index.ts:186` · agent secret compared with non-constant-time `!==` on a `verify_jwt=false` function. PLAUSIBLE — timing weakness is real, remote byte-by-byte recovery over TLS jitter is not realistically demonstrable. Recommend a timing-safe compare. @@ -119,11 +137,11 @@ The dominant theme is **clinical-safety / data-integrity in the answer path**: s 19 lanes reviewed; the completeness critic surfaced 6 gaps, all filled: -| Area | Lanes | -|---|---| +| Area | Lanes | +| ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Deep (risk-bearing) | rag-engine, rag-routing, search-retrieval, clinical-search, ingestion-core, index-units, worker, enrichment, visual, privacy-auth, governance-safety, supabase-db, openai, api-surface, deep-memory-polish, security-sweep | -| Light sweep | ui-components, scripts-cli, tests-config-deps | -| Gap-fill (critic) | clinical-output-rendering, document-mutation-reindex-correctness, deep-memory-logic, database-migrations-rls-rpc, request-primitives-rate-limit | +| Light sweep | ui-components, scripts-cli, tests-config-deps | +| Gap-fill (critic) | clinical-output-rendering, document-mutation-reindex-correctness, deep-memory-logic, database-migrations-rls-rpc, request-primitives-rate-limit | Clean lanes (no surviving findings): `supabase-db` client setup (no service-role-key leakage to the browser bundle found). The gap round is where H4, M8, M9, M11, M13, M16, L5, L8, L18 were caught — i.e. clinical-output rendering and mutation-race correctness were under-covered by the first pass. @@ -143,64 +161,64 @@ working tree of `claude/cool-wiles-12aade` (uncommitted, nothing pushed). 42 fil ### Fixed (behavioral) -| ID | File | Fix | -|---|---|---| -| H1 | `src/lib/answer-verification.ts` | verification corpus now includes `retrieval_synopsis` + image table text (mirrors `sourceTextForQuoteVerification`); 2 regression tests | -| H2 | `src/lib/source-text-sanitizer.ts` | new `clinicalThresholdSignalPattern` rescue — threshold-bearing fragments are never dropped by title/noise heuristics; GCS regression test + bare-integer noise still dropped | -| H3+L4 | `src/lib/retrieval-selection.ts` | **SUPERSEDED by PR #118 on main** (merged 2026-07-02, after this audit's base commit): the golden retrieval eval measured that source-governance metadata weighting in selection buries correct documents on the partially-enriched corpus (doc-recall@5 1.0→0.76), so #118 **removed the penalties entirely** and clamped the candidate score — resolving both H3's premise and L4. This branch's `retrieval-selection.ts` + its contract test are now aligned **verbatim** with origin/main's #118 version (any deviation requires re-running `npm run eval:retrieval:quality`, 23/23 required). | -| H4+M8+M16 | `src/lib/ward-output.ts` | copied tables routed through `normalizeAccessibleTable` (conservativeClinical) with the on-screen low-confidence caveat; markdown header no longer duplicated as a data row; ragged rows padded/merged; 3 regression tests | -| M1 | `src/lib/rag.ts` | `deriveConfidence` takes the citation array and scopes strongest-similarity to cited chunks (4 call sites) | -| M2+M3 | `src/lib/clinical-search.ts` | word-boundary signal matching (short acronyms whole-word); explicit dose terms survive escalation cancellation; 5 regression assertions | -| M4 | `src/lib/document-index-units.ts` | bare `yes`/`no` only counts as flowchart evidence when the image is not structurally a table | -| M5+M6+M7 | `src/lib/document-organization.ts` | fallbacks gate on confirmed candidates; title/file_name in the evidence haystack (bracket segments stripped so `(FSH)` tags still can't self-confirm — pinned by an existing test); BMJ requires the `bmj` token; 2 regression tests | -| M9 | `src/app/api/documents/[id]/route.ts` | DELETE blocks on `pending` OR `processing` jobs (parameterized regression test) | -| M10 | `src/components/ClinicalDashboard.tsx` | request-id ref guard — only the latest search commits answer/error/loading state | -| M11+L8 | `src/lib/deep-memory.ts` | all embeddings computed BEFORE deleting old memory (failure window no longer spans OpenAI); fallback committed-generation filter fails closed on lookup error (test updated to pin fail-closed) | -| M12 | `src/lib/image-filtering.ts` | perceptual hash upgraded ph1 (16-bit) → ph2 (192-bit; threshold + quantized-level bits); version bump prevents legacy aliasing | -| M13 | `supabase/migrations/20260702000000_…` + `schema.sql` | **new** migration: `commit_document_index_generation` purges legacy NULL-generation rows only when replacement rows exist in the same table. **Not yet applied to the live DB** — apply via the normal migration flow | -| M14 | `src/lib/chunking.ts` | page-noise pattern anchored to the whole line; inline "p 3" references survive (regression test) | -| M15 | `src/lib/query-privacy.ts`, `env.ts`, `.env.example` | HMAC-SHA256 keyed hash when `RAG_QUERY_HASH_SECRET` (new, optional, min 16 chars) is set; legacy digest kept when unset for continuity — **set the secret in any environment logging real queries** | -| M17 | `src/lib/chunking.ts` | strict forward-progress guard in `chunkTextBySentence` (regression test at overlap == chunkSize) | -| L1/L2 | `scripts/purge-query-logs.ts`, `scripts/recover-ingestion-queue.ts` | unknown flags and malformed numeric flags now throw instead of silently misbehaving | -| L3 | `src/lib/evidence-relevance.ts` | aggregate `none` verdict reachable — nearby requires matched terms | -| L6 | `src/lib/visual-intelligence.ts` | fallback medications derived from the vocabulary's medication category, not a hardcoded allowlist | -| L7+L20 | `supabase/functions/indexing-v3-agent/index.ts` | non-retryable 4xx fails fast; timing-safe (hashed) secret comparison; Deno typecheck passes | -| L10 | `src/app/api/search/route.ts`, `src/lib/evidence.ts` | relevance/visual evidence computed once per request and shared with the smart panel (both call sites) | -| L12 | `src/lib/privacy.ts` | non-string error fields serialized and redacted instead of passing through verbatim | -| L14 | `src/lib/answer-ranking.ts` | bold cap counts per-occurrence free passes; new tokens can't tunnel past the cap | -| L16/L17 | `src/lib/document-tags.ts`, `src/lib/ingestion.ts` | dead/misleading branches removed with intent documented | +| ID | File | Fix | +| --------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| H1 | `src/lib/answer-verification.ts` | verification corpus now includes `retrieval_synopsis` + image table text (mirrors `sourceTextForQuoteVerification`); 2 regression tests | +| H2 | `src/lib/source-text-sanitizer.ts` | new `clinicalThresholdSignalPattern` rescue — threshold-bearing fragments are never dropped by title/noise heuristics; GCS regression test + bare-integer noise still dropped | +| H3+L4 | `src/lib/retrieval-selection.ts` | **SUPERSEDED by PR #118 on main** (merged 2026-07-02, after this audit's base commit): the golden retrieval eval measured that source-governance metadata weighting in selection buries correct documents on the partially-enriched corpus (doc-recall@5 1.0→0.76), so #118 **removed the penalties entirely** and clamped the candidate score — resolving both H3's premise and L4. This branch's `retrieval-selection.ts` + its contract test are now aligned **verbatim** with origin/main's #118 version (any deviation requires re-running `npm run eval:retrieval:quality`, 23/23 required). | +| H4+M8+M16 | `src/lib/ward-output.ts` | copied tables routed through `normalizeAccessibleTable` (conservativeClinical) with the on-screen low-confidence caveat; markdown header no longer duplicated as a data row; ragged rows padded/merged; 3 regression tests | +| M1 | `src/lib/rag.ts` | `deriveConfidence` takes the citation array and scopes strongest-similarity to cited chunks (4 call sites) | +| M2+M3 | `src/lib/clinical-search.ts` | word-boundary signal matching (short acronyms whole-word); explicit dose terms survive escalation cancellation; 5 regression assertions | +| M4 | `src/lib/document-index-units.ts` | bare `yes`/`no` only counts as flowchart evidence when the image is not structurally a table | +| M5+M6+M7 | `src/lib/document-organization.ts` | fallbacks gate on confirmed candidates; title/file_name in the evidence haystack (bracket segments stripped so `(FSH)` tags still can't self-confirm — pinned by an existing test); BMJ requires the `bmj` token; 2 regression tests | +| M9 | `src/app/api/documents/[id]/route.ts` | DELETE blocks on `pending` OR `processing` jobs (parameterized regression test) | +| M10 | `src/components/ClinicalDashboard.tsx` | request-id ref guard — only the latest search commits answer/error/loading state | +| M11+L8 | `src/lib/deep-memory.ts` | all embeddings computed BEFORE deleting old memory (failure window no longer spans OpenAI); fallback committed-generation filter fails closed on lookup error (test updated to pin fail-closed) | +| M12 | `src/lib/image-filtering.ts` | perceptual hash upgraded ph1 (16-bit) → ph2 (192-bit; threshold + quantized-level bits); version bump prevents legacy aliasing | +| M13 | `supabase/migrations/20260702000000_…` + `schema.sql` | **new** migration: `commit_document_index_generation` purges legacy NULL-generation rows only when replacement rows exist in the same table. **Not yet applied to the live DB** — apply via the normal migration flow | +| M14 | `src/lib/chunking.ts` | page-noise pattern anchored to the whole line; inline "p 3" references survive (regression test) | +| M15 | `src/lib/query-privacy.ts`, `env.ts`, `.env.example` | HMAC-SHA256 keyed hash when `RAG_QUERY_HASH_SECRET` (new, optional, min 16 chars) is set; legacy digest kept when unset for continuity — **set the secret in any environment logging real queries** | +| M17 | `src/lib/chunking.ts` | strict forward-progress guard in `chunkTextBySentence` (regression test at overlap == chunkSize) | +| L1/L2 | `scripts/purge-query-logs.ts`, `scripts/recover-ingestion-queue.ts` | unknown flags and malformed numeric flags now throw instead of silently misbehaving | +| L3 | `src/lib/evidence-relevance.ts` | aggregate `none` verdict reachable — nearby requires matched terms | +| L6 | `src/lib/visual-intelligence.ts` | fallback medications derived from the vocabulary's medication category, not a hardcoded allowlist | +| L7+L20 | `supabase/functions/indexing-v3-agent/index.ts` | non-retryable 4xx fails fast; timing-safe (hashed) secret comparison; Deno typecheck passes | +| L10 | `src/app/api/search/route.ts`, `src/lib/evidence.ts` | relevance/visual evidence computed once per request and shared with the smart panel (both call sites) | +| L12 | `src/lib/privacy.ts` | non-string error fields serialized and redacted instead of passing through verbatim | +| L14 | `src/lib/answer-ranking.ts` | bold cap counts per-occurrence free passes; new tokens can't tunnel past the cap | +| L16/L17 | `src/lib/document-tags.ts`, `src/lib/ingestion.ts` | dead/misleading branches removed with intent documented | ### Dispositioned without behavior change (with rationale) -| ID | Disposition | -|---|---| -| L5 | The audit's recommended fix (coerce File→null) was **wrong**: `tests/api-validation-contract.test.ts` pins that non-string multipart metadata must 400 before any storage/DB write. The no-op ternary was replaced by a pass-through with the contract documented. | -| L9 | `image_count` = searchable-only is intentional (retrieval filters `searchable=true`); the verifier found no consumer reconciling against it. No change. | -| L11 | Triple `readFile` is a deliberate peak-memory trade-off (holding hundreds of multi-MB buffers is worse than re-reading); documented at the site. | -| L18 | Both audit_logs policy migrations are already applied — editing applied migrations creates replay drift. Consolidate only if migrations are ever squashed. | -| L19 | CSP `unsafe-inline` hardening (nonce-based CSP) deferred: no XSS sink exists today (zero `dangerouslySetInnerHTML`), and a nonce migration needs dedicated UI verification. Tracked as accepted risk. | -| L13/L15 | Applied earlier as the safe auto-fix pass (CSP host dedup; dead ternary). | +| ID | Disposition | +| ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| L5 | The audit's recommended fix (coerce File→null) was **wrong**: `tests/api-validation-contract.test.ts` pins that non-string multipart metadata must 400 before any storage/DB write. The no-op ternary was replaced by a pass-through with the contract documented. | +| L9 | `image_count` = searchable-only is intentional (retrieval filters `searchable=true`); the verifier found no consumer reconciling against it. No change. | +| L11 | Triple `readFile` is a deliberate peak-memory trade-off (holding hundreds of multi-MB buffers is worse than re-reading); documented at the site. | +| L18 | Both audit_logs policy migrations are already applied — editing applied migrations creates replay drift. Consolidate only if migrations are ever squashed. | +| L19 | CSP `unsafe-inline` hardening (nonce-based CSP) deferred: no XSS sink exists today (zero `dangerouslySetInnerHTML`), and a nonce migration needs dedicated UI verification. Tracked as accepted risk. | +| L13/L15 | Applied earlier as the safe auto-fix pass (CSP host dedup; dead ternary). | ### Adversarial diff-review round (2026-07-02) Five parallel review agents then audited the fix diff itself. They confirmed the bulk of the changes sound and surfaced **12 follow-up findings — all fixed** in the same working tree: -| # | Finding (introduced/incomplete) | Resolution | -|---|---|---| -| R1 | **HIGH** — H3's un-floored penalties **compounded** across the pipeline's 2–3 selection passes (`baseScore` re-read the penalized `hybrid_score`) | resolved by discarding the H3 change and aligning `retrieval-selection.ts` verbatim with PR #118 (relevance-first, no metadata penalties — see H3 row). Residual note: POSITIVE intent boosts still compound mildly across passes on main's design (pre-existing, eval-validated); a `preSelectionScore` idempotency fix was prototyped and withdrawn — reintroduce only gated on the golden eval. | -| R2 | M2's whole-word `mg`/`mcg` no longer matched digit-attached doses ("10mg"), and could re-cancel dosing via M3 | `(?:\b|\d)` digit-adjacent allowance (regression test "clozapine 100mg…") | -| R3 | L3's aggregate `none` could contradict per-source "nearby" chips for purely-semantic matches and trip the danger governance banner | aggregate `nearby` also granted when relevance score ≥ 0.5 | -| R4 | H1 incomplete — rich-mode prompts show table-fact **metadata** snippets (`accessible_table_markdown`/`table_text_snippet`/`cells`) not covered by the corpus | included in both `tableFactText` (verification) and `tableFactQuoteText` (quotes) | -| R5 | H2's rescued noisy fragments inflated `provenanceScore` past 0.42, flipping `useful` false and blanking text that previously survived | provenance computed over baseline-kept fragments only; thresholds count toward `clinicalSignalScore` | -| R6 | `x10` threshold branch matched "Rx100"/"0x10" | lookbehind guard `(?<![a-z0-9])` | -| R7 | H4 parity gap — on-screen `AccessibleTable` still passed `columns` for markdown-parsed rows (M8 live on screen; screen/clipboard could disagree on headers and the caveat) | same header-source rule applied in `AccessibleTable.tsx` | -| R8 | section gating (raw shape) vs emission (normalize) disagreement could emit a dangling "Thresholds" heading | `sectionOutputLines` skips heading when items and normalized tables are both empty | -| R9 | M10 guard missed in-flight progress updates (retry/keyword/stream messages) — stale request could repaint the banner | guarded `onProgress` threaded through `runWithRetries`/`requestAnswer` | -| R10 | M6's bracket-strip guard didn't cover `source_path`, which echoes the file name — `(FSH)` tags could still self-confirm | `source_path` bracket segments stripped too | -| R11 | M13's guard is structurally vacuous for the 3 chunk-anchored tables (`source_chunk_id ON DELETE CASCADE` follows the replaced legacy chunks) | guarantee scope stated precisely in migration + schema.sql (full protection: images, memory cards, sections — cascade is required, orphaned artifacts would be unreachable) | -| R12 | M13 incomplete — the worker's client-side fallback (`deleteStaleIndexGenerationRows`, used when the RPC is missing) still deleted NULL-generation rows unconditionally | fallback now checks for replacement rows per table before purging legacy rows, mirroring the RPC | -| R13 | L2 incomplete — `recover-ingestion-queue` still silently ignored typo'd flag *names* (the audit's original scenario) and missing/empty values | strict token walk: unknown flags and missing values throw | +| # | Finding (introduced/incomplete) | Resolution | +| --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| R1 | **HIGH** — H3's un-floored penalties **compounded** across the pipeline's 2–3 selection passes (`baseScore` re-read the penalized `hybrid_score`) | resolved by discarding the H3 change and aligning `retrieval-selection.ts` verbatim with PR #118 (relevance-first, no metadata penalties — see H3 row). Residual note: POSITIVE intent boosts still compound mildly across passes on main's design (pre-existing, eval-validated); a `preSelectionScore` idempotency fix was prototyped and withdrawn — reintroduce only gated on the golden eval. | +| R2 | M2's whole-word `mg`/`mcg` no longer matched digit-attached doses ("10mg"), and could re-cancel dosing via M3 | `(?:\b | \d)` digit-adjacent allowance (regression test "clozapine 100mg…") | +| R3 | L3's aggregate `none` could contradict per-source "nearby" chips for purely-semantic matches and trip the danger governance banner | aggregate `nearby` also granted when relevance score ≥ 0.5 | +| R4 | H1 incomplete — rich-mode prompts show table-fact **metadata** snippets (`accessible_table_markdown`/`table_text_snippet`/`cells`) not covered by the corpus | included in both `tableFactText` (verification) and `tableFactQuoteText` (quotes) | +| R5 | H2's rescued noisy fragments inflated `provenanceScore` past 0.42, flipping `useful` false and blanking text that previously survived | provenance computed over baseline-kept fragments only; thresholds count toward `clinicalSignalScore` | +| R6 | `x10` threshold branch matched "Rx100"/"0x10" | lookbehind guard `(?<![a-z0-9])` | +| R7 | H4 parity gap — on-screen `AccessibleTable` still passed `columns` for markdown-parsed rows (M8 live on screen; screen/clipboard could disagree on headers and the caveat) | same header-source rule applied in `AccessibleTable.tsx` | +| R8 | section gating (raw shape) vs emission (normalize) disagreement could emit a dangling "Thresholds" heading | `sectionOutputLines` skips heading when items and normalized tables are both empty | +| R9 | M10 guard missed in-flight progress updates (retry/keyword/stream messages) — stale request could repaint the banner | guarded `onProgress` threaded through `runWithRetries`/`requestAnswer` | +| R10 | M6's bracket-strip guard didn't cover `source_path`, which echoes the file name — `(FSH)` tags could still self-confirm | `source_path` bracket segments stripped too | +| R11 | M13's guard is structurally vacuous for the 3 chunk-anchored tables (`source_chunk_id ON DELETE CASCADE` follows the replaced legacy chunks) | guarantee scope stated precisely in migration + schema.sql (full protection: images, memory cards, sections — cascade is required, orphaned artifacts would be unreachable) | +| R12 | M13 incomplete — the worker's client-side fallback (`deleteStaleIndexGenerationRows`, used when the RPC is missing) still deleted NULL-generation rows unconditionally | fallback now checks for replacement rows per table before purging legacy rows, mirroring the RPC | +| R13 | L2 incomplete — `recover-ingestion-queue` still silently ignored typo'd flag _names_ (the audit's original scenario) and missing/empty values | strict token walk: unknown flags and missing values throw | Also confirmed sound by the reviewers (no action): env-parse fail-fast for a short hash secret, no client-bundle secret leak, HMAC-switch dedup continuity, `deriveConfidence` call-site coverage, diff --git a/docs/rag-hybrid-findings-and-todo.md b/docs/rag-hybrid-findings-and-todo.md index 6b41d76245..d8cb2c5192 100644 --- a/docs/rag-hybrid-findings-and-todo.md +++ b/docs/rag-hybrid-findings-and-todo.md @@ -63,8 +63,8 @@ outstanding backlog. See also the master plan 4. 🔍 **Answer-path ranking investigated (2026-07-01) — healthy; low mrr is a sibling-doc artifact, NOT a defect.** Probed every low-`rr@10` golden case. In each, the docs ranked above the pinned - one are **legitimate siblings** the corpus genuinely contains: several *Safety Planning* guidelines - (KEMH/RKPG/AKG), multiple hospital versions of *Active Community Patients in ED*, multiple + one are **legitimate siblings** the corpus genuinely contains: several _Safety Planning_ guidelines + (KEMH/RKPG/AKG), multiple hospital versions of _Active Community Patients in ED_, multiple opioid-pharmacotherapy guidelines, and the two agitation guidelines. Recall stays 1.0 and the model gets correct context; forcing the pinned doc to #1 over equally-valid siblings would be overfitting. **So items 1/6 (query-class weighting to raise mrr) are deprioritized** — chasing that metric on @@ -73,25 +73,25 @@ outstanding backlog. See also the master plan (`clinical-search.ts:1362`) — base + the ~40 stacked boosts routinely exceed 1.0, so many strong matches tie at 1.0 and order by an arbitrary `document_id` tiebreak. It doesn't hurt these cases (the tied docs are all relevant), but it wastes the boost engineering. If ever revisited, break - ties by the *pre-clamp* score rather than raising the ceiling (downstream gates assume [0,1]). + ties by the _pre-clamp_ score rather than raising the ceiling (downstream gates assume [0,1]). - The second-stage rerank (which uses unclamped scoring + a strong dose-amount/title boost) rarely fires for document_lookup/broad_summary (`shouldUseSecondStageRerank` needs `topScoresClose && - hasVisualEvidence`, `rag.ts:548`). Widening it (RC10) could restore discrimination among the +hasVisualEvidence`, `rag.ts:548`). Widening it (RC10) could restore discrimination among the 1.0-tied group, but since the tied docs are valid siblings the payoff is marginal and unvalidatable on the current golden set — do it only alongside a chunk-level "best-passage-first" eval metric. 5. ⏸️ **`ef_search` policy inconsistent — BLOCKED, deferred.** Attempted `ALTER FUNCTION … SET - hnsw.ef_search='100'` on the three sql functions; **hosted Supabase denies it (`42501 permission - denied to set parameter`)** — the RC11 blocker. The only method hosted allows is the plpgsql-wrapper - + runtime `PERFORM set_config('hnsw.ef_search','100',true)` pattern (what memory_cards uses; measured - latency-neutral: chunks 76→79ms warm). Deferred: the recall gain is unquantified (golden already 1.0) - and there's no hard-query eval set to justify adding three plpgsql wrappers. Revisit once an - expanded/hard eval set exists (see P2.8). +hnsw.ef_search='100'` on the three sql functions; **hosted Supabase denies it (`42501 permission +denied to set parameter`)** — the RC11 blocker. The only method hosted allows is the plpgsql-wrapper + - runtime `PERFORM set_config('hnsw.ef_search','100',true)` pattern (what memory_cards uses; measured + latency-neutral: chunks 76→79ms warm). Deferred: the recall gain is unquantified (golden already 1.0) + and there's no hard-query eval set to justify adding three plpgsql wrappers. Revisit once an + expanded/hard eval set exists (see P2.8). 6. **RC5–RC13 ranking tuning** — partially addressed / re-scoped after the item-4 investigation: - ✅ **Same-document crowding (RC7)** — the `/api/search` results panel cap was lowered `maxPerDocument 4→3` (`app/api/search/route.ts`, backfill-protected so result count is unchanged). Note: this only affects the **panel**; the answer-retrieval path (`searchChunksWithTelemetry`) has no per-doc cap and doesn't need one — the comparison gate already enforces ≥2 distinct docs, and - single-topic queries *should* be able to draw multiple chunks from the best document. + single-topic queries _should_ be able to draw multiple chunks from the best document. - ⏳ **Synthetic text similarity (RC9)** `least(0.95, 0.56 + text_rank*0.39)` still feeds coverage gates that assume a real cosine — gate text-only paths on `text_rank`/`rrf` instead. (Cleanest remaining ranking-correctness item.) @@ -137,12 +137,13 @@ outstanding backlog. See also the master plan working precise match. Unit tests in `tests/retrieval-query-variants.test.ts` + the `alcohol-ciwa-threshold` golden case guard it. **This is a general recall win, not just CIWA** — any long multi-term query previously risked silent 0-match FTS. + 9. ⚠️ **OCR "dropped-s" defect — real but NOT reliably heuristically-detectable; guard attempted then REVERTED (2026-07-01). Honest post-mortem below.** - **What's true:** real dropped-'s' corruption exists in some table-derived index units ("psychosocial"→"p ycho ocial", "1st mood stabiliser"→"1 t mood tabili er"). The **raw `document_chunks` (answer context) are clean** — 0 docs below 0.025 s-ratio — so **generated - answer text is not degraded**; the defect only touches structured *table* units (OCR'd from + answer text is not degraded**; the defect only touches structured _table_ units (OCR'd from images), and the intact numbers survive ("CIWA-Ar **core** <10" keeps the "<10"). - **The detection is the hard part — every heuristic false-positives.** First tried an s-ratio detector (`'s'`/letter < 0.03): it flagged 772 units but **only 135 were real (82% false @@ -164,8 +165,8 @@ outstanding backlog. See also the master plan - **If ever pursued (low priority, modest impact):** reliable detection needs a **dictionary/ spellcheck approach** ("fraction of tokens that aren't valid English/clinical words") or fixing the **upstream table-OCR** step — not a token heuristic. Neither is warranted by the impact. - Remaining true enrichment items: confirm `20260627000000_retrieval_hnsw_ef_search.sql` on live; run - `enrich:backfill` / `tags:backfill` for any genuinely missing synopsis/labels. + Remaining true enrichment items: confirm `20260627000000_retrieval_hnsw_ef_search.sql` on live; run + `enrich:backfill` / `tags:backfill` for any genuinely missing synopsis/labels. 10. 🔧 **Query understanding (RC6/E) — pg_trgm typo correction started (2026-07-01).** - **Data-driven promotion is blocked:** `rag_query_misses` (71 rows) are privacy-redacted hashes with empty `candidate_aliases`, so the plan's "promote real misses to aliases" path can't run. @@ -179,7 +180,7 @@ outstanding backlog. See also the master plan min_sim 0.45. Validated: clozapin→clozapine, agitaton→agitation, schizophrenai→schizophrenia, bipoler→bipolar, withdrawl→withdrawal, lithiun→lithium; clean queries unchanged. ~85ms. - ✅ **Wired as a text-search fallback** in `searchTextChunkCandidates` (`rag.ts`): when strict AND - variants return nothing, correct the query and retry (strictly, then OR-relaxed) *before* the 8b + variants return nothing, correct the query and retry (strictly, then OR-relaxed) _before_ the 8b OR-relaxation, so a typo like "clozapin monitoring" resolves to clozapine rather than OR-matching generic "monitoring" docs. Verified end-to-end: "clozapin anc threshold"→Clozapine docs, "dischage planning"→Discharge Planning. Golden set unchanged (23/23, no regression); 682 tests pass. @@ -187,10 +188,10 @@ outstanding backlog. See also the master plan (`rag.ts:4986`) now, when a query would short-circuit as unsupported, trigram-corrects it and — if it changed — re-runs the whole retrieval once on the corrected text (guarded by an internal `typoCorrected` flag; only fires for would-be-unsupported queries so no hot-path cost). Rescues - typo queries whose corrected form is a *supported* class (e.g. a typo'd clozapine/dose query + typo queries whose corrected form is a _supported_ class (e.g. a typo'd clozapine/dose query → table_threshold). Golden 23/23 unchanged, 682 tests pass. - ⚠️ **Pre-existing bug surfaced (NEW, finding #11):** unsupported-classified queries retrieve - **nondeterministically** — the *same* query in the *same* process alternates + **nondeterministically** — the _same_ query in the _same_ process alternates `unsupported_short_circuit` (0 results) vs `text_fast_path`/`hybrid` (real results), e.g. "anorexia management" (no typo). Classification is pure and all caches honour `skipCache`, so the variance is elsewhere in the unsupported-query path (candidate: alias fetch/expansion or an async diff --git a/src/components/ClinicalDashboard.tsx b/src/components/ClinicalDashboard.tsx index 5c2e97c88c..88f79d92d8 100644 --- a/src/components/ClinicalDashboard.tsx +++ b/src/components/ClinicalDashboard.tsx @@ -951,8 +951,8 @@ function NaturalLanguageAnswer({ textMuted, )} > - Source-only answer — assembled from your documents without the AI model, so it may be less - complete. Verify it against the cited passages below. + Source-only answer — assembled from your documents without the AI model, so it may be less complete. Verify + it against the cited passages below. </p> ) : null} {sourceCapsuleButton} diff --git a/src/lib/rag-provider.ts b/src/lib/rag-provider.ts index f22ab2fc95..78d5409366 100644 --- a/src/lib/rag-provider.ts +++ b/src/lib/rag-provider.ts @@ -40,12 +40,7 @@ export function allowsAutoDegrade(): boolean { } export type ProviderFailureKind = - | "missing_key" - | "auth_failed" - | "quota_exhausted" - | "rate_limited" - | "timeout" - | "provider_failed"; + "missing_key" | "auth_failed" | "quota_exhausted" | "rate_limited" | "timeout" | "provider_failed"; /** * Classify why an OpenAI call failed, for telemetry and user-facing fallback messaging. diff --git a/src/lib/rag.ts b/src/lib/rag.ts index c6253f356d..428388f7f4 100644 --- a/src/lib/rag.ts +++ b/src/lib/rag.ts @@ -459,11 +459,7 @@ function recordRetrievalLayer( // where telemetry is in scope, record the failing RPC + code so it shows up in rag_retrieval_logs. type SupabaseRpcError = { message?: string; code?: string; details?: string; hint?: string } | null; -function recordHybridRpcError( - telemetry: SearchTelemetry | undefined, - rpc: string, - error: SupabaseRpcError, -) { +function recordHybridRpcError(telemetry: SearchTelemetry | undefined, rpc: string, error: SupabaseRpcError) { if (!error) return; const code = error.code ?? "unknown"; logger.error("hybrid_rpc_failed", { rpc, code, message: error.message, hint: error.hint }); @@ -5003,8 +4999,7 @@ export function generatedAnswerQualityFailureReason(answer: RagAnswer, query: st // summary answers legitimately paraphrase, so enforcing overlap there would reject good answers. // A model-answer failure here only escalates fast→strong and is recovered for strongly // source-backed answers, so the downside of enforcing it is a retry, not a wrongful gap. - const enforceModelAnswerOverlap = - isSimpleDirectQuestion(query, queryClass) && !isBareDefinitionQuestion(query); + const enforceModelAnswerOverlap = isSimpleDirectQuestion(query, queryClass) && !isBareDefinitionQuestion(query); if ( (answer.routingMode === "extractive" || answer.confidence === "low" || enforceModelAnswerOverlap) && !hasRelevantQueryOverlap(cleanedAnswer, query) @@ -6369,7 +6364,11 @@ async function answerQuestionWithScopeUncoalesced( const sourceOnlyAnswer = isSourceOnlyMode(); const route = sourceOnlyAnswer && gatedRoute.route.mode !== "unsupported" - ? { ...gatedRoute.route, mode: "extractive" as const, reason: `${gatedRoute.route.reason}; ${sourceOnlyReason()}` } + ? { + ...gatedRoute.route, + mode: "extractive" as const, + reason: `${gatedRoute.route.reason}; ${sourceOnlyReason()}`, + } : gatedRoute.route; const retrievalDiagnostics: RetrievalDiagnostics = { ...initialRetrievalDiagnostics, @@ -7138,8 +7137,7 @@ ${qualityRetryInstruction}` // answer's citations. buildExtractiveAnswer derives its own source-backed // citations from the retrieved results, so trigger recovery whenever the // generated answer is unusable and we have retrieved results to extract from. - const canRecoverExtractively = - !usedStrongModel && (answer.citations.length > 0 || answerInputResults.length > 0); + const canRecoverExtractively = !usedStrongModel && (answer.citations.length > 0 || answerInputResults.length > 0); if (canRecoverExtractively && isUnusableGeneratedAnswer(answer)) { answer = buildExtractiveAnswer({ query: args.query, diff --git a/src/lib/source-text-sanitizer.ts b/src/lib/source-text-sanitizer.ts index 3271a1726e..9a94f717e9 100644 --- a/src/lib/source-text-sanitizer.ts +++ b/src/lib/source-text-sanitizer.ts @@ -247,8 +247,7 @@ export function clinicalProseUsefulness(text: string) { (mostlySourceTitle && !hasConcreteClinicalAction) || (noiseRatio >= 0.28 && !hasConcreteClinicalAction) || (isLowYieldClinicalText(fragment) && !hasConcreteClinicalAction); - const keptByBaseline = - !droppedByBaseline && (hasClinicalSignal || hasConcreteClinicalAction || noiseRatio < 0.16); + const keptByBaseline = !droppedByBaseline && (hasClinicalSignal || hasConcreteClinicalAction || noiseRatio < 0.16); if (!keptByBaseline && !hasClinicalThresholdSignal) continue; keptFragments.push(fragment); if (keptByBaseline) baselineKeptFragments.push(fragment); diff --git a/tests/answer-prose-runons.test.ts b/tests/answer-prose-runons.test.ts index 24e8a80451..c9853ecb27 100644 --- a/tests/answer-prose-runons.test.ts +++ b/tests/answer-prose-runons.test.ts @@ -14,7 +14,9 @@ describe("flattened-table run-on separation", () => { }); it("handles the pattern with no comma present", () => { - const out = polishClinicalAnswerProse("U&Es are repeated every 6 months for inpatients for community patients they are checked annually."); + const out = polishClinicalAnswerProse( + "U&Es are repeated every 6 months for inpatients for community patients they are checked annually.", + ); expect(out).toContain("for inpatients. For community patients,"); }); diff --git a/tests/openai-error-mapping.test.ts b/tests/openai-error-mapping.test.ts index df8cd90dd4..1d337f6829 100644 --- a/tests/openai-error-mapping.test.ts +++ b/tests/openai-error-mapping.test.ts @@ -25,10 +25,7 @@ describe("mapOpenAIError quota vs rate-limit classification", () => { }); it("detects quota exhaustion from the message even when no error code is set", () => { - const mapped = mapOpenAIError( - openAIError("Billing hard limit reached.", { status: 429 }), - "answer", - ); + const mapped = mapOpenAIError(openAIError("Billing hard limit reached.", { status: 429 }), "answer"); expect(mapped.details?.code).toBe("insufficient_quota"); expect(mapped.message).not.toMatch(/retry in a moment/i); diff --git a/tests/rag-content-accuracy.test.ts b/tests/rag-content-accuracy.test.ts index 2b65ac8f40..19658ec855 100644 --- a/tests/rag-content-accuracy.test.ts +++ b/tests/rag-content-accuracy.test.ts @@ -46,10 +46,7 @@ describe("unboldUnverifiedNumbers — emphasis tracks verification (P8)", () => }); it("keeps bold around verified figures and non-numeric emphasis", () => { - const out = unboldUnverifiedNumbers( - "Give **10 mg** now and **withhold** if unstable.", - new Set(["500mg"]), - ); + const out = unboldUnverifiedNumbers("Give **10 mg** now and **withhold** if unstable.", new Set(["500mg"])); expect(out).toBe("Give **10 mg** now and **withhold** if unstable."); }); diff --git a/tests/rag-provider.test.ts b/tests/rag-provider.test.ts index 5f22dfc2cd..590872ae92 100644 --- a/tests/rag-provider.test.ts +++ b/tests/rag-provider.test.ts @@ -51,7 +51,9 @@ describe("provider failure classification", () => { ); expect(p.classifyProviderFailure(Object.assign(new Error("x"), { status: 401 }))).toBe("auth_failed"); expect( - p.classifyProviderFailure(Object.assign(new Error("Rate limit reached"), { status: 429, code: "rate_limit_exceeded" })), + p.classifyProviderFailure( + Object.assign(new Error("Rate limit reached"), { status: 429, code: "rate_limit_exceeded" }), + ), ).toBe("rate_limited"); expect(p.classifyProviderFailure(Object.assign(new Error("Request timed out"), { code: "ETIMEDOUT" }))).toBe( "timeout", diff --git a/tests/supabase-schema.test.ts b/tests/supabase-schema.test.ts index d52da78eec..226b0852b8 100644 --- a/tests/supabase-schema.test.ts +++ b/tests/supabase-schema.test.ts @@ -496,8 +496,12 @@ describe("Supabase schema Data API grants", () => { } // schema.sql mirrors the live-codified ranked ordering (single sort key, wider candidate // bound); the original migration kept the similarity tie-breaker and tighter bound. - expect(extractIndexUnitHybridFunction(schema)).toContain("order by text_rank desc limit greatest(match_count * 3, 48)"); - expect(extractIndexUnitHybridFunction(documentIndexUnitsMigration)).toContain("order by text_rank desc, similarity desc"); + expect(extractIndexUnitHybridFunction(schema)).toContain( + "order by text_rank desc limit greatest(match_count * 3, 48)", + ); + expect(extractIndexUnitHybridFunction(documentIndexUnitsMigration)).toContain( + "order by text_rank desc, similarity desc", + ); }); it("stores smart image metadata, document labels, and high-yield summaries", () => {