From 1d0443669a63bf775665d1d599e940e04f487472 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Jul 2026 20:19:45 +0000 Subject: [PATCH 1/2] feat(governance): RAG behaviour memory + enforced ranking safeguards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs/rag-behaviour/: durable evidence-backed memory of the 2026-07-20 cycle — behaviour map (imputation sites, comparator chains, gate ladder, second-stage engagement), the two live-refuted approaches with binding third-attempt constraints, and the safeguard stack reference - AGENTS.md 'RAG ranking protection': standing rules every agent session inherits — flag RAG impact before editing, canary pair for behaviour changes, never insert comparator keys above relevance - pr-policy: new blocking gate — PRs touching RAG-ranking protected surfaces fail without an explicit 'RAG impact:' declaration (no-change reason or canary pair); self-test covers undeclared/vague/no-change/ canary cases; workflow failure message generalized - tests/rag-imputation-contract.test.ts: source-text pins on the imputation formulas (app + SQL) and the release comparator key ORDER (score -> similarity -> relevance -> id) — red-proven against a mutated formula; failure message routes editors to the required protocol Restoration context: canary #56 (run 29774459706) confirmed reverted main green 36/36, closing the #55-regression -> revert -> restore arc. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 --- .github/workflows/pr-policy.yml | 8 +- AGENTS.md | 28 ++++++ docs/rag-behaviour/README.md | 32 +++++++ docs/rag-behaviour/behaviour-map.md | 86 +++++++++++++++++ docs/rag-behaviour/refuted-approaches.md | 71 ++++++++++++++ docs/rag-behaviour/safeguards.md | 75 +++++++++++++++ scripts/pr-policy.mjs | 112 +++++++++++++++++++++-- tests/rag-imputation-contract.test.ts | 71 ++++++++++++++ 8 files changed, 474 insertions(+), 9 deletions(-) create mode 100644 docs/rag-behaviour/README.md create mode 100644 docs/rag-behaviour/behaviour-map.md create mode 100644 docs/rag-behaviour/refuted-approaches.md create mode 100644 docs/rag-behaviour/safeguards.md create mode 100644 tests/rag-imputation-contract.test.ts diff --git a/.github/workflows/pr-policy.yml b/.github/workflows/pr-policy.yml index 21fc06a51..554199bb6 100644 --- a/.github/workflows/pr-policy.yml +++ b/.github/workflows/pr-policy.yml @@ -94,13 +94,15 @@ jobs: .write(); // Advisory metadata nudges (title/summary/verification/UI/risk) are - // surfaced but never fail the check. Only a clinical-risk PR with an - // incomplete Clinical Governance Preflight blocks the merge. + // surfaced but never fail the check. Two conditions block the merge: a + // clinical-risk PR with an incomplete Clinical Governance Preflight, and a + // RAG-ranking-surface PR without an explicit `RAG impact:` declaration + // (docs/rag-behaviour/safeguards.md). for (const warning of warnings) core.warning(warning); if (!result.ok) { for (const error of result.errors) core.error(error); core.setFailed( - `PR policy found ${result.errors.length} blocking clinical-governance issue(s). Complete the Clinical Governance Preflight and rerun.`, + `PR policy found ${result.errors.length} blocking issue(s) — see the errors above (Clinical Governance Preflight and/or the RAG impact declaration), fix the PR body, and rerun.`, ); } diff --git a/AGENTS.md b/AGENTS.md index f3327fd55..baf21a7c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,6 +185,34 @@ When a branch or PR review completes, record the reviewed branch/ref, HEAD SHA, + + +# RAG ranking protection + +Retrieval/ranking behaviour is live-validated and safeguarded. Before touching any protected +surface, read `docs/rag-behaviour/` (README → behaviour-map → refuted-approaches → safeguards). + +- **Flag it.** Any task that will touch `src/lib/rag/**`, clinical-search, retrieval-selection, + released-search-order, ranking-config, evidence/result-sort/answer-ranking, the eval harness + (`scripts/eval-retrieval.ts`, `scripts/lib/clinical-aliases.ts`, ranking-tuning/snapshot + tooling), the golden fixture/snapshot, or the retrieval RPCs must say so to the user BEFORE + editing, even when the change looks incidental (refactor, rename, "just a comment"). +- **PR gate.** PRs touching those surfaces fail `pr-policy` without an explicit `RAG impact:` + line in the body — either `RAG impact: no retrieval behaviour change — ` or + `RAG impact: behaviour change — canary pair -> `. The source-pin contract + test (`tests/rag-imputation-contract.test.ts`) additionally goes red on any edit to the + imputation formulas or release-comparator key order. +- **Canary for behaviour.** Any retrieval/ranking/ordering behaviour change requires a live + eval-canary before/after pair (doc/content recall pinned 1.0, zero per-case rr regressions) + before it is trusted; regression → immediate single-commit revert + confirmation run. + Dispatches are provider-backed (~$1–2) and always need explicit user approval. +- **Never** insert a comparator key above the relevance score, bulk-merge the wide + captured-case alias tier into the strict golden tier, relax the clamped-score contract, or + adopt tuner recommendations without a measured live gain. Offline-green + review-approved + was proven insufficient for this surface on 2026-07-20 (see refuted-approaches). + + + # Railway project safety diff --git a/docs/rag-behaviour/README.md b/docs/rag-behaviour/README.md new file mode 100644 index 000000000..2f12cea5e --- /dev/null +++ b/docs/rag-behaviour/README.md @@ -0,0 +1,32 @@ +# RAG behaviour memory + +Durable, evidence-backed knowledge about how this repo's retrieval/ranking stack actually +behaves — created 2026-07-20 after a full measure → tune → structural-fix → refute cycle +(ADDENDUM 4; canary runs #49–#56). Read this BEFORE touching any retrieval, ranking, +selection, release-ordering, or eval-ground-truth surface. + +| File | What it holds | +| ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `behaviour-map.md` | The verified mechanics: score imputation sites, the release comparator chains, the gate/threshold ladder, second-stage engagement rules, and which live cases exercise which path. | +| `refuted-approaches.md` | The two live-refuted improvement attempts with their numbers, the root-cause post-mortems, and the binding constraints any third attempt must satisfy. | +| `safeguards.md` | The protection stack: protected-surface list, the pr-policy `RAG impact:` gate, the source-pin contract test, the canary-pair protocol, and the regeneration procedures. | + +Standing rules (mirrored in `AGENTS.md` so every agent session inherits them): + +1. **Flag RAG impact.** Any task touching a protected surface (list in `safeguards.md`) must say + so explicitly before editing, and its PR must carry a `RAG impact:` line (enforced by + `scripts/pr-policy.mjs` — the check fails without it). +2. **Canary for behaviour changes.** Any retrieval/ranking/ordering behaviour change requires a + live eval-canary pair — baseline + post — with doc/content recall pinned at 1.0 and zero + per-case regressions, before the change is trusted. `workflow_dispatch` on + `eval-canary.yml` (provider-backed → explicit user approval per run). +3. **Offline green is necessary, never sufficient.** The Phase C regression passed 121/121 + offline tests and an adversarial code review, then failed 3/36 live within one run. The live + corpus is the only authority on ordering behaviour. +4. **Ties are load-bearing.** Pools of equal scores are resolved by the boost/title/subject-aware + relevance rank — that resolution is correct behaviour, not noise. Never insert a new + comparator key above the relevance score. + +Related: `docs/observability-slos.md` §3.1 (boundary-case & metric-interpretation policy), +`docs/branch-review-ledger.md` (2026-07-20 rows = the full audit trail), +`docs/rag-hybrid-findings-and-todo.md` (older findings; RC-numbered history). diff --git a/docs/rag-behaviour/behaviour-map.md b/docs/rag-behaviour/behaviour-map.md new file mode 100644 index 000000000..e753f4263 --- /dev/null +++ b/docs/rag-behaviour/behaviour-map.md @@ -0,0 +1,86 @@ +# RAG behaviour map (verified 2026-07-20) + +Everything below was verified against live canary evidence (runs #49–#56) and direct source +inspection during the ADDENDUM-4 cycle. Line numbers drift with refactors — search by symbol. + +## 1. Score imputation on the embedding-free text fast path + +When a query resolves without embeddings (`strategy=text_fast_path` and friends), candidates +carry **imputed** primaries derived only from Postgres `text_rank`: + +| Site | Where | Formula (current, reviewed state) | Saturation | +| ------------------ | ----------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | +| S1 text chunks | SQL `match_document_chunks_text(_v2)` | `similarity = 0` (truthful contract — no fabricated cosine); `hybrid = least(0.5, 0.18 + least(text_rank,1)·0.3)`; honest signal in `lexical_score` | tr ≥ 1 → hybrid exactly 0.48 | +| S2 table facts | `rag-candidate-sources.ts` (`runTableFacts` grouping) | `similarity = min(0.94, 0.62 + min(tr,1)·0.3)`; `hybrid = min(0.97, 0.66 + min(tr,1)·0.3)` | tr ≥ 1 → byte-identical 0.92 / 0.96 | +| S3 document lookup | `rag-candidate-sources.ts` (documentScore path) | partial discrimination via `chunkScore·0.08`; doc rank clamps at 0.34, sim cap 0.92 reachable | high-tr + high-sim pools tie | +| S4 memory cards | `rag-candidate-sources.ts` | `sim = min(0.92, 0.58 + confidence·0.28)` | equal-confidence cards tie | + +`text_rank` itself is unbounded above 1 (ts_rank_cd + title-weighted + trigram/term bonuses) +and the SQL orders by it — so the RPCs discriminate candidates that the `min(tr,1)` clamps then +collapse to byte-identical app-side primaries. **This saturation is why unrelated documents +matching the same terms tie exactly.** + +## 2. What resolves ties (the comparator chains) + +- **Selection** (`retrieval-selection.ts`): clamped score → lexicalScore → rerankScore → + contentCoverageScore (#987) → chunkId. The clamped-score contract is sacred (measured + golden doc-recall 1.0→0.76 when violated — the #118 lesson). Coverage is carried ONLY as a + late tie-break, never added to score. +- **Release without second stage** (`released-search-order.ts`): hybrid → similarity → + relevance.score → id. +- **Release with second stage**: releaseRankScore (= max(hybrid, finalScore + position + adjustment)) → similarity → relevance.score → id. Position adjustments launder the selection + order into release order, so for engaged pools the selection comparator is what matters. +- **rankClinicalResults** (`clinical-search.ts`): unbounded rankScore → similarity → id. + +**The critical property: in all-saturated pools, hybrid AND similarity tie, so ordering falls +to `relevance.score` — the boost/title/subject-aware clinical rank. That fallback is doing +correct clinical work.** (Proven live: spreading similarity/hybrid above it caused the Phase C +regression — see `refuted-approaches.md`.) + +## 3. Second-stage engagement (`shouldUseSecondStageRerank`) + +- `table_threshold` / `medication_dose_risk`: engages on visual evidence OR `topScoresClose` + (|top1−top2| ≤ 0.04) — saturated pools always engage. +- `comparison`: engages on overflow or closeness. +- Everything else: needs closeness AND visual evidence — plain lexical pools do NOT engage. + +Live mapping (canary #54): engaged — clozapine, alcohol-ciwa-threshold; not engaged — +patient-safety-plan, opioid-withdrawal, flowchart-next-step. Fixes that only touch release-time +tie-breaks can never move engaged pools. + +## 4. The gate/threshold ladder (why score bands matter) + +All functional gates read `max(hybrid ?? similarity)` and live at ≤ 0.82: fast-path acceptance +0.62/0.64/0.66, coverage gates 0.48–0.62, answer routing 0.32/0.48/0.64/0.76, confidence bars +0.5/0.68, conflict/high-confidence 0.82. The bands (0.92, 0.94] (table-fact sim), (0.96, 0.97] +(table-fact hybrid) and (0.48, 0.5) (lexical hybrid) contain **no gates** — but they are NOT +free real estate for ordering keys (see §2's critical property). + +## 5. Live case ↔ path map (canary #53/#54 evidence) + +- Perfect rank-1 under current behaviour: 27+ of 36 cases. +- Known rank-depth headroom, all `text_fast_path`, all still PASSING their gates: + flowchart-next-step rr 0.20, alcohol-ciwa-threshold 0.25 (second-stage-engaged), + patient-safety-plan 0.33, opioid-withdrawal 0.33. +- `lithium-therapy-monitoring` rr was a hardcoded 0.00 until 2026-07-20 (no document + expectation); now gated on `["Lithium"]` and measuring 1.0. Treat 2026-07-20 as an mrr@10 + baseline step (+~0.028 from de-noising). +- `irrelevant_source_rate@10` ≈ 0.108 is dominated by broad/vector cases pulling topically + adjacent sibling guidelines — audit labels before treating as ranking debt + (`docs/observability-slos.md` §3.1). + +## 6. Eval measurement mechanics + +- Golden gates are zero-tolerance top-5 per-case checks; `--fail-on-threshold` fails the run on + any miss. The canary log's per-case lines + human summary are what humans and the + failure-issue analyzer read; `--json-out` writes the machine artifact independently. +- `textContainsClinicalTerm` is whitespace-delimited: it cannot match punctuation-joined tokens + (`CIWA-Ar`, `treatment,`, `(opioid`). Sanctioned aliases (`scripts/lib/clinical-aliases.ts`, + the STRICT tier) absorb known cases; a word-boundary matcher change is a proposed, + separately-reviewed follow-up. The WIDER captured-case tier in + `src/lib/eval-document-matching.ts` must never be bulk-merged into the strict tier. +- Fixture and ranking snapshot move in lockstep (test-pinned); the snapshot carries + `generatedAt` provenance with an active 30-day freshness gate; regenerate from the latest + `eval-canary-output` artifact via `npm run build:ranking-snapshot`. +- Run-over-run trends: `npm run eval:trend -- `. diff --git a/docs/rag-behaviour/refuted-approaches.md b/docs/rag-behaviour/refuted-approaches.md new file mode 100644 index 000000000..22c9467b3 --- /dev/null +++ b/docs/rag-behaviour/refuted-approaches.md @@ -0,0 +1,71 @@ +# Refuted ranking-improvement approaches (2026-07-20) + +Two approaches to the fast-path rank-depth headroom were implemented, live-tested, and refuted +in one evening. They are recorded here so no future task re-attempts them in the same shape. +Full audit trail: `docs/branch-review-ledger.md` (2026-07-20 rows), PRs #1003–#1006. + +## Refutation 1 — per-class feature-weight tuning (Phase B): live no-op + +- **Shape:** offline tuner on the provenance-stamped ranking snapshot recommended three + constrained per-class `featureFusion` nudges (document_lookup titleSectionRelevance→0.9, + table_threshold clinicalEvidence→0.95, comparison hybridRelevance→0.95; proxy comparison-mrr + 0.833→1.0). Staged live via the `rag_ranking_config` dispatch input — no code defaults + touched. +- **Live pair (#53 baseline vs #54 tuned, override proven active in the run log):** + mrr@10 0.8922 → 0.8921; irrelevant@10 0.1083 → 0.1083; every headroom case byte-identical. +- **Why it failed:** the 5-candidate linear proxy space saturates — the proxy's predicted gains + were already realized live, and the real headroom lives in saturated-tie structure that + feature weights cannot express. +- **Standing consequence:** tuner recommendations are hypotheses only; adoption requires a live + pair with measured gain (none → not adopted; nothing was rolled back because staging is + per-run). + +## Refutation 2 — saturation-tail spread of comparator keys (Phase C): live regression + +- **Shape:** a pure, monotone, set-independent tail spreading tied imputed primaries inside + "dead" score bands — table-fact `similarity` into (0.92, 0.94), lexical-chunk `hybrid` into + (0.48, 0.5). Passed a red-proven discriminating test, tie-conservation guard, 121/121 + targeted offline tests, full-suite, AND an adversarial code review (APPROVE-WITH-NITS — + the envelope math was correct: no gate crossed). +- **Live pair (#54 baseline vs #55 on the merged change):** FAILED 3/36 — doc_recall + 1.0 → 0.9167, mrr@10 0.8921 → 0.8138; patient-property rr 1.00 → 0.11, schizophrenia-overview + 1.00 → 0.14, patient-safety-plan 0.33 → 0.14. Reverted within the hour (#1005); restoration + confirmed by canary #56. +- **Why it failed:** hybrid (S1) and similarity (S2) sort ABOVE `relevance.score` in the + release comparators. Spreading them — even inside gate-free value bands — moved tie + resolution from the boost/title/subject-aware relevance rank to raw ts_rank order. + Lexically-loud chunks leapfrogged title-boosted correct documents: the #118 burial mechanism, + reproduced live. The "arbitrary" ties were not arbitrary — relevance was already resolving + them correctly. +- **Why offline missed it:** the discriminating fixtures used identical-content candidates + (every key tied), which structurally cannot expose an ordering flip between + differently-relevant candidates. The adversarial review audited values and gates, not the + comparator _precedence_ semantics. + +## Binding constraints for any third attempt + +1. **Position:** a text-rank discriminator may only be inserted strictly BELOW + `relevance.score` in the release comparators (i.e. between relevance and the id fallback), + or as a bounded term INSIDE the relevance rank itself — never as/above a primary key. +2. **Fixtures:** the discriminating offline test must use differently-relevant candidates + (different boosts/titles/coverage) and prove the OLD code orders them correctly by + relevance while the id-fallback case improves — identical-content fixtures are disallowed + as sole proof. +3. **Live pair:** dedicated baseline + post canary with doc/content recall pinned at 1.0 and + zero per-case rr regressions; any regression = immediate single-commit revert (both + directions proven cheap tonight). +4. **Approval:** separate explicit user approval; provider-backed runs are never automatic. +5. **Honest sizing:** the prize is rank depth on 3–4 already-passing cases (~0.03–0.08 mrr). + Weigh against the demonstrated regression risk before attempting at all. + +## Related follow-up plans (documented, not yet implemented) + +- **Word-boundary content matcher:** `textContainsClinicalTerm` misses ordinary prose + punctuation (`treatment,` `(opioid` `ptsd.[35]`) — 4 live occurrences found in top-5 + previews. A matcher-wide change to word-boundary semantics is a measurement-layer behaviour + shift: own PR, full 36-case impact audit offline (artifact replay), then one canary confirm. +- **irrelevant@10 labeling audit:** before any ranking work aimed at the 0.108 rate, audit the + broad/vector cases' top-10 for under-labeled relevant siblings (the alias-tier lesson); the + fix may be sanctioned labels, not ranking. +- **Phase E (answer-side quality):** untouched by this cycle; requires its own approval and + spend (~$2–5/run) per the master plan. diff --git a/docs/rag-behaviour/safeguards.md b/docs/rag-behaviour/safeguards.md new file mode 100644 index 000000000..dc83c2acf --- /dev/null +++ b/docs/rag-behaviour/safeguards.md @@ -0,0 +1,75 @@ +# RAG ranking safeguards + +The protection stack that keeps retrieval/ranking behaviour from being changed casually — by +any task, session, or agent. Added 2026-07-20 after the Phase C live regression proved that +offline-green + review-approved is not sufficient for this surface. + +## Protected surfaces + +Code and ground truth whose edits change (or re-measure) retrieval/ranking behaviour: + +- `src/lib/rag/**` — the retrieval waterfall, candidate sources, release pipeline +- `src/lib/clinical-search.ts`, `src/lib/retrieval-selection.ts`, + `src/lib/released-search-order.ts`, `src/lib/ranking-config.ts`, `src/lib/evidence.ts`, + `src/lib/result-sort.ts`, `src/lib/answer-ranking.ts`, `src/lib/evidence-relevance.ts`, + `src/lib/semantic-rerank.ts`, `src/lib/eval-document-matching.ts` +- `scripts/eval-retrieval.ts`, `scripts/lib/clinical-aliases.ts`, + `scripts/lib/ranking-tuning.ts`, `scripts/lib/ranking-snapshot-builder.ts`, + `scripts/build-ranking-snapshot.ts`, `scripts/tune-search-weights.ts` +- `scripts/fixtures/rag-retrieval-golden.json`, + `scripts/fixtures/rag-ranking-candidate-snapshot.v1.json` +- The contract-pinning tests: `tests/rag-fast-path-ordering.test.ts`, + `tests/ranking-tuning.test.ts`, `tests/retrieval-selection.test.ts`, + `tests/rag-second-stage-ranking.test.ts`, `tests/eval-retrieval.test.ts` +- Retrieval RPCs in `supabase/schema.sql` / migrations (covered by the clinical-risk gate) + +## Layer 1 — PR-body gate (enforced, blocking) + +`scripts/pr-policy.mjs` classifies changed files; a PR touching a protected surface **fails the +PR-policy check** unless its body carries an explicit acknowledgment line: + +``` +RAG impact: + RAG impact: no retrieval behaviour change — + RAG impact: behaviour change — canary pair -> +``` + +The line must state either **no behaviour change** (with a reason) or reference the **canary** +pair. This makes RAG impact a conscious declaration on every PR, from every session. + +## Layer 2 — source-pin contract test (enforced, offline) + +`tests/rag-imputation-contract.test.ts` pins the exact imputation formulas and the release +comparator key ORDER as source text. Any edit — including by a task that never read this folder +— goes red in unit tests with a failure message pointing here and to the process in +`refuted-approaches.md`. Updating the pins is allowed only alongside the full protocol +(design + differently-relevant fixtures + canary pair + user approval). + +## Layer 3 — behaviour guards (pre-existing, standing) + +- Zero-tolerance golden gates (36 cases, weekly scheduled canary + on-demand dispatch) +- Fast-path ordering suite (production-pipeline replays of the 2026-07-19 failure shapes) +- Snapshot gates: golden-regression quartet at production weights, full-snapshot high-risk + hard-negative floor, fixture↔snapshot lockstep pin, 30-day freshness gate +- Clinical Governance Preflight (blocking) for all clinical-risk paths + +## Layer 4 — process (agent memory) + +`AGENTS.md` § "RAG ranking protection" mirrors these rules so every agent session loads them: +flag RAG impact before editing, canary pair for behaviour changes, never insert comparator +keys above relevance, read this folder first. The eval-canary pair protocol: + +1. Baseline: latest green canary on current main (or one dispatch). +2. Change merges (or runs from a branch via the `ref` dispatch input). +3. Post: one dispatch; gates = recall 1.0/1.0, zero per-case rr regressions. +4. Regression → immediate single-commit revert + one confirmation dispatch. + +Provider-backed dispatches always need explicit user approval (~$1–2 each). + +## Rollback proof (2026-07-20) + +The full cycle was exercised live: merge (#1004) → regression detected by canary #55 within +~35 minutes → revert (#1005) → restoration confirmed 36/36 (canary #56). Total main exposure +under the regression: eval traffic only. diff --git a/scripts/pr-policy.mjs b/scripts/pr-policy.mjs index 825642648..9a17bde35 100644 --- a/scripts/pr-policy.mjs +++ b/scripts/pr-policy.mjs @@ -65,6 +65,21 @@ const operationalRiskPatterns = [ /^(?:Dockerfile|railway(?:\.[^.]+)?\.json|nixpacks\.toml)$/, ]; +// RAG-ranking protected surfaces (docs/rag-behaviour/safeguards.md). Narrower than +// clinicalRiskPatterns: these files change (or re-measure) retrieval/ranking ORDERING +// behaviour, which is live-validated — offline-green plus review-approved was proven +// insufficient here on 2026-07-20 (Phase C regression, canary #55). A PR touching any of +// them must carry an explicit `RAG impact:` declaration (see evaluatePullRequestPolicy). +const ragRankingPatterns = [ + /^src\/lib\/rag\//, + /^src\/lib\/(?:clinical-search|retrieval-selection|released-search-order|ranking-config|evidence|result-sort|answer-ranking|evidence-relevance|semantic-rerank|eval-document-matching)\.ts$/, + /^scripts\/(?:eval-retrieval|build-ranking-snapshot|tune-search-weights)\.ts$/, + /^scripts\/lib\/(?:clinical-aliases|ranking-tuning|ranking-snapshot-builder)\.ts$/, + /^scripts\/fixtures\/(?:rag-retrieval-golden|rag-ranking-candidate-snapshot\.v1)\.json$/, + // The contract-pinning tests are protected too: weakening them is the evasion route. + /^tests\/(?:rag-fast-path-ordering|ranking-tuning|retrieval-selection|rag-second-stage-ranking|eval-retrieval|rag-imputation-contract)\.test\.ts$/, +]; + const uiPatterns = [ /^src\/app\/(?!api\/)/, /^src\/(?:components|styles)\//, @@ -204,15 +219,30 @@ export function classifyPullRequestFiles(files) { files: normalized, clinicalRisk: normalized.some((file) => clinicalRiskPatterns.some((pattern) => pattern.test(file))), operationalRisk: normalized.some((file) => operationalRiskPatterns.some((pattern) => pattern.test(file))), + ragRanking: normalized.some((file) => ragRankingPatterns.some((pattern) => pattern.test(file))), ui: normalized.some((file) => uiPatterns.some((pattern) => pattern.test(file))), }; } +// The `RAG impact:` declaration a ragRanking PR must carry: either an explicit +// no-behaviour-change statement (with a reason) or a canary-pair reference. Matched anywhere +// in the body, list-marker tolerant, case-insensitive. +export function ragImpactDeclared(body) { + const match = String(body ?? "").match(/^[\s>*-]*\*{0,2}RAG impact\*{0,2}:\s*(\S[^\n]*)/im); + if (!match) return { declared: false, satisfied: false }; + const detail = match[1].trim(); + const noChange = /no\s+(?:retrieval\s+|ranking\s+)?behaviou?r\s+change/i.test(detail); + const canary = /canar/i.test(detail); + return { declared: true, satisfied: (noChange || canary) && detail.length >= 12 }; +} + export function evaluatePullRequestPolicy({ title, body, headRef, files }) { - // Only genuine clinical-risk governance gaps block the PR (hard failure). - // Every other metadata expectation (title, summary, verification, UI, risk - // and rollout) is advisory: it is surfaced as a warning so authors still get - // the nudge, but it never fails the check or blocks a merge. + // Two conditions block the PR (hard failure): a clinical-risk diff without a + // complete Clinical Governance Preflight, and a RAG-ranking-surface diff + // without an explicit `RAG impact:` declaration. Every other metadata + // expectation (title, summary, verification, UI, risk and rollout) is + // advisory: it is surfaced as a warning so authors still get the nudge, but + // it never fails the check or blocks a merge. const errors = []; const warnings = []; const classification = classifyPullRequestFiles(files); @@ -249,8 +279,25 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { ); } + // Blocking gate: a PR touching RAG-ranking protected surfaces must declare its impact + // explicitly (docs/rag-behaviour/safeguards.md — layer 1). Ordering behaviour is + // live-validated; the declaration forces every session/task to consciously state either + // "no behaviour change" (with a reason) or the canary pair covering the change. + if (classification.ragRanking) { + const ragImpact = ragImpactDeclared(body); + if (!ragImpact.declared) { + errors.push( + "This PR touches RAG-ranking protected surfaces. Add a `RAG impact:` line to the body — either `RAG impact: no retrieval behaviour change — ` or `RAG impact: behaviour change — canary pair -> ` (see docs/rag-behaviour/safeguards.md).", + ); + } else if (!ragImpact.satisfied) { + errors.push( + "The `RAG impact:` line must state `no retrieval behaviour change — ` or reference the validating canary pair (see docs/rag-behaviour/safeguards.md).", + ); + } + } + // Blocking gate: a clinical-risk PR must carry a complete Clinical Governance - // Preflight. This is the only condition that fails the check. + // Preflight. This is the only other condition that fails the check. if (classification.clinicalRisk) { if (!meaningfulText(governance)) { errors.push("Clinical-risk paths require the `## Clinical Governance Preflight` section."); @@ -282,7 +329,7 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { function selfTest() { const completeGovernance = requiredClinicalGovernanceItems.map((item) => `- [x] ${item}`).join("\n"); - const completeBody = `## Summary\n\n- Add a trusted PR policy.\n\n## Verification\n\n- [x] \`npm run verify:pr-local\`\n- [x] \`npm run verify:ui\`\n\n## Risk and rollout\n\n- Risk: low; metadata-only validation.\n- Rollback: revert the workflow commit.\n\n## Clinical Governance Preflight\n\n${completeGovernance}`; + const completeBody = `## Summary\n\n- Add a trusted PR policy.\n\nRAG impact: no retrieval behaviour change — metadata-only validation.\n\n## Verification\n\n- [x] \`npm run verify:pr-local\`\n- [x] \`npm run verify:ui\`\n\n## Risk and rollout\n\n- Risk: low; metadata-only validation.\n- Rollback: revert the workflow commit.\n\n## Clinical Governance Preflight\n\n${completeGovernance}`; assert.equal( evaluatePullRequestPolicy({ title: "ci: enforce pull request evidence", @@ -420,6 +467,7 @@ function selfTest() { files: ["src/app/api/search/route.ts"], clinicalRisk: true, operationalRisk: false, + ragRanking: false, ui: false, }); // Narrowed classification: presentation-only UI under the clinically-named @@ -429,8 +477,60 @@ function selfTest() { files: ["src/components/clinical-dashboard/settings-dialog.tsx"], clinicalRisk: false, operationalRisk: false, + ragRanking: false, ui: true, }); + // RAG-ranking protected surfaces (docs/rag-behaviour/safeguards.md): a PR touching them + // without a `RAG impact:` declaration is hard-blocked... + const ragUndeclared = evaluatePullRequestPolicy({ + title: "fix: adjust release ordering tie-break", + body: completeBody.replace(/^RAG impact:.*\n\n/m, ""), + headRef: "codex/ordering-fix", + files: ["src/lib/released-search-order.ts"], + }); + assert.equal(ragUndeclared.ok, false, "RAG-surface PRs without a RAG impact line must block"); + assert.match(ragUndeclared.errors.join(" "), /RAG impact/); + // ...a vague declaration (neither no-change nor canary) also blocks... + assert.match( + evaluatePullRequestPolicy({ + title: "fix: adjust release ordering tie-break", + body: completeBody.replace(/^RAG impact:.*$/m, "RAG impact: probably fine"), + headRef: "codex/ordering-fix", + files: ["src/lib/released-search-order.ts"], + }).errors.join(" "), + /RAG impact/, + ); + // ...an explicit no-behaviour-change declaration passes (with governance complete)... + assert.equal( + evaluatePullRequestPolicy({ + title: "docs: clarify release-order comments", + body: completeBody, + headRef: "codex/ordering-docs", + files: ["src/lib/released-search-order.ts"], + }).ok, + true, + "a declared no-behaviour-change RAG PR must pass", + ); + // ...and a canary-pair declaration passes for behaviour changes. + assert.equal( + evaluatePullRequestPolicy({ + title: "feat: retrieval ordering change", + body: completeBody.replace( + /^RAG impact:.*$/m, + "RAG impact: behaviour change — canary pair run 54 -> post-merge dispatch planned", + ), + headRef: "codex/ordering-change", + files: ["src/lib/rag/rag-candidate-sources.ts", "tests/rag-fast-path-ordering.test.ts"], + }).ok, + true, + "a canary-pair-declared RAG behaviour change must pass", + ); + // The golden fixture and contract tests are protected surfaces too. + assert.equal(classifyPullRequestFiles(["scripts/fixtures/rag-retrieval-golden.json"]).ragRanking, true); + assert.equal(classifyPullRequestFiles(["tests/ranking-tuning.test.ts"]).ragRanking, true); + // Answer synthesis is clinical-risk but NOT rag-ranking (retrieval ordering is the + // protected axis here; generation keeps the governance gate only). + assert.equal(classifyPullRequestFiles(["src/lib/answer-synthesis.ts"]).ragRanking, false); // ...but privacy/access-control UI surfaces stay gated. assert.equal(classifyPullRequestFiles(["src/components/privacy-input-notice.tsx"]).clinicalRisk, true); // ...and clinical behavior in the library layer stays gated. diff --git a/tests/rag-imputation-contract.test.ts b/tests/rag-imputation-contract.test.ts new file mode 100644 index 000000000..9d3aafea0 --- /dev/null +++ b/tests/rag-imputation-contract.test.ts @@ -0,0 +1,71 @@ +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { describe, expect, it } from "vitest"; + +// RAG-ranking contract pins (docs/rag-behaviour/safeguards.md — layer 2). +// +// These are deliberate SOURCE-TEXT pins on the score-imputation formulas and the release +// comparator key order. They exist because retrieval ORDERING behaviour is live-validated: +// on 2026-07-20 a change that passed 121/121 offline tests and an adversarial code review +// still regressed the live golden eval 3/36 within one run (canary #55) and was reverted. +// +// If an edit turns one of these pins red, that is the safeguard working — do NOT simply +// update the expected strings. The required protocol (docs/rag-behaviour/refuted-approaches.md) +// is: dedicated design, discriminating offline tests with DIFFERENTLY-relevant candidates, +// an explicit `RAG impact:` PR declaration, user approval, and a live canary before/after +// pair (doc/content recall pinned 1.0, zero per-case rr regressions) before the change is +// trusted. Update these pins in the same PR as that evidence, never alone. + +const read = (path: string) => readFileSync(resolve(path), "utf8"); + +describe("RAG imputation and release-order contract", () => { + it("pins the table-fact imputed-primary formulas (S2)", () => { + const source = read("src/lib/rag/rag-candidate-sources.ts"); + expect(source).toContain("similarity: Math.min(0.94, 0.62 + Math.min(textRank, 1) * 0.3)"); + expect(source).toContain("hybridScore: Math.min(0.97, 0.66 + Math.min(textRank, 1) * 0.3)"); + }); + + it("pins the lexical-chunk truthful-score contract in SQL (S1: similarity 0, hybrid capped at 0.48)", () => { + const schema = read("supabase/schema.sql"); + // Both text-RPC generations carry the cap; spacing differs between them. + const matches = schema.match(/least\(0\.5,\s*0\.18 \+ \(least\(ranked\.text_rank, 1\) \* 0\.3\)\)/g) ?? []; + expect(matches.length).toBeGreaterThanOrEqual(2); + }); + + it("pins the release comparator key order: score -> similarity -> relevance -> id", () => { + const source = read("src/lib/released-search-order.ts"); + // Key order is the load-bearing contract: in all-saturated pools the score keys tie and + // ordering falls to relevance.score — the boost/title/subject-aware clinical rank. A new + // key inserted ABOVE relevance lets raw text-rank override clinical ranking (the proven + // Phase C regression mechanism). Assert relative positions of the four keys in both + // comparators by stripping to their comparison lines. + const hybridKey = source.indexOf("rightHybrid - leftHybrid"); + const releaseKey = source.indexOf("rightReleaseScore - leftReleaseScore"); + const similarityKeys = [...source.matchAll(/rightSimilarity - leftSimilarity/g)].map((match) => match.index ?? -1); + const relevanceKeys = [ + ...source.matchAll(/right\.relevance\?\.score \?\? 0\) - \(left\.relevance\?\.score \?\? 0\)/g), + ].map((match) => match.index ?? -1); + const idKeys = [...source.matchAll(/left\.id\.localeCompare\(right\.id\)/g)].map((match) => match.index ?? -1); + + expect(hybridKey).toBeGreaterThan(-1); + expect(releaseKey).toBeGreaterThan(-1); + expect(similarityKeys).toHaveLength(2); + expect(relevanceKeys).toHaveLength(2); + expect(idKeys.length).toBeGreaterThanOrEqual(2); + // First comparator: hybrid < similarity < relevance < id. + expect(hybridKey).toBeLessThan(similarityKeys[0]); + expect(similarityKeys[0]).toBeLessThan(relevanceKeys[0]); + expect(relevanceKeys[0]).toBeLessThan(idKeys[0]); + // Second comparator: releaseScore < similarity < relevance < id. + expect(releaseKey).toBeLessThan(similarityKeys[1]); + expect(similarityKeys[1]).toBeLessThan(relevanceKeys[1]); + expect(relevanceKeys[1]).toBeLessThan(idKeys[1]); + }); + + it("pins the selection tie-break contract: coverage stays a late tie-break, never added to score", () => { + const source = read("src/lib/retrieval-selection.ts"); + expect(source).toContain("contentCoverageScore"); + // The clamped-score contract comment and the coverage comparator must both survive. + expect(source).toMatch(/clamp/i); + }); +}); From 7b1bffa8e672a84c48530d24ce62887a2d48835e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Jul 2026 20:19:45 +0000 Subject: [PATCH 2/2] docs(ledger): record RAG memory + safeguards review row Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 --- docs/branch-review-ledger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index ca9a2a208..1df733ae8 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -657,3 +657,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (restarted; PR: Phase C saturation-tail primaries) | 7572c7f | ADDENDUM 4 Phase C (user-authorized): per-candidate discriminative primaries for saturated fast-path ties — design-agent planned (consumer map + dead-band envelope proof), red-proven, tie-conservation guarded | Mechanism: min(text_rank,1) collapses all tr≥1 candidates to byte-identical imputed primaries; ordering fell to chunk id at release. Fix: saturationTailUnit (pure, monotone, SET-INDEPENDENT — rejected per-query min-max + rank-tier designs for set-dependence/#118 authority risk; rejected full-range log rescale for moving sub-knee values across the 0.62-0.82 gate ladder) scales the excess into DEAD cap bands only: S2 table-fact similarity (0.92, 0.94) with hybrid byte-identical (gates/triggers/selection provably unchanged; similarity = the release tie-break key), S1 lexical-chunk hybrid (0.48, 0.5) behind the truthful-contract signature (sub-0.5 bars hold). Sub-knee byte-identical (fixtures now DERIVE from the helper; 0.45→0.755/0.795 pinned). Discriminating test verified RED on old formulas (2 fail: discriminating + envelope) → green with tail; equal-tr tie-conservation pins the #987 coverage comparator; second-stage-engaged pools documented out of scope (position-derived releaseRankScore sorts first there) — matches live evidence that non-engaged pools (patient-safety, opioid, flowchart) are where id-order decided. S3/S4 = C-PR-2 candidates, evidence-gated on the post-merge canary vs #54 baseline (doc/content recall MUST stay 1.0, zero per-case regressions; success signal = rr lift on the headroom cases). Rollback: single revert (helpers + 2 expression sites + 1 map call; no schema/config/cache surface). | Targeted vitest 121/121 (fast-path 11/11 incl. 6 new, retrieval-selection, rag-routing, rag-answer-fallback, ranking-tuning, second-stage); npm run test 3025 passed / 1 known container pdf-budget artifact; lint + typecheck + prettier clean; red-proof executed and recorded; live validation = post-merge canary dispatch (~$1-2) | | 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (PR: revert Phase C pending review) | revert of f96217c | User-requested revert of #1004 (Phase C saturation-tail primaries): auto-merge fired before the adversarial rag-retrieval-reviewer pass landed; retrieval code returns to the last reviewed state until that verdict is in | Clean single-commit revert (the documented rollback path — helpers + 2 expression sites + 1 map call; no schema/config/cache surface). Ledger history rows from f96217c retained (docs are append-only record, not behavior). The 19:44Z post-merge canary dispatched on f96217c completes regardless and stands as Phase C's live validation datapoint; re-land decision = reviewer verdict + that pair result together. Offline state of the reverted change remains fully proven (red-proof + 121/121 + envelope tests). | Revert verified by vitest fast-path suite returning to pre-C 5/5 shape expected in CI; ladder on the revert = hosted pr-required | | 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (Phase C live verdict — no code change; main stays reverted at 0a498e6) | canary run 29773198933 (#55, on f96217c) | ADDENDUM 4 Phase C LIVE-REFUTED: canary on the merged tail code FAILED 3/36 (doc_recall 1.0→0.9167, mrr@10 0.8921→0.8138) — the user-ordered revert (#1005) was correct and STANDS | Failures: patient-safety-plan-include (PtSafetyPlan out of top-5, rr 0.33→0.14), patient-property-visual-table (rr 1.00→0.11) and schizophrenia-overview (rr 1.00→0.14) — two previously rank-1 cases destroyed. Root cause (post-hoc): the S1 lexical-chunk lift spreads hybrid_score, which is the PRIMARY release sort key — inside the dead (0.48,0.5) band it still PREEMPTS every downstream key, so raw ts_rank order overrode the boost/title/subject-aware relevance order that previously decided all-tied-at-0.48 pools. Lexically-loud chunks leapfrogged title-boosted correct documents = the #118 mechanism, reproduced live. The offline S1 test used identical-content candidates (coverage tie) and could not see it. The S2 similarity tail (tie-break-position key) remains reviewer-verified safe in isolation — retrieval reviewer verdict on the full diff: APPROVE-WITH-NITS, P3 only (0.49-lowering proven unreachable via the SQL 0.48 cap; no gate crossings in (0.92,0.94); hardening nits recorded). DISPOSITION: no re-land as-is. Any retry = S2-only + S1 redesigned to a key BELOW relevance in the comparator chain, new design + fresh pair. The staged-rollback discipline (canary pair + instant revert) worked exactly as designed. Spend ≈$4-8 of ≤$20. | Canary #55 read from job log (3 FAIL lines + summary); reviewer verdict from subagent report; main verified reverted (0 saturationTailUnit refs at 0a498e6) | +| 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (PR: RAG memory + safeguards) | see PR head | User-directed close-out: durable RAG behaviour memory (docs/rag-behaviour/, 4 files) + AGENTS.md standing protection rules + ENFORCED safeguards — pr-policy blocking `RAG impact:` gate on protected surfaces (self-tested: undeclared/vague blocked, no-change/canary declarations pass) + source-pin contract test on imputation formulas and release comparator key order (red-proven vs a mutated constant). Confirmation canary #56 (29774459706, reverted main 0a498e6): SUCCESS — 36/36 restored, closing the #55-regression→revert→restore arc live. Phase D complete (D-1 ref input #1003, D-2 eval:trend #1006, D-3 policy §3.1 #1006, D-4 latency in trend rows); remaining documented plans: word-boundary matcher (own PR), irrelevant@10 labeling audit, Phase E (separate approval). Spend ≈$5-10 of ≤$20. | pr-policy self-test + workflow guard PASS; contract test 4/4 + red-proof; npm run test 3025 passed / 1 known container pdf-budget artifact; lint+typecheck+prettier clean; docs:check-links 1030 refs PASS; check:github-actions PASS |