diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index cb2245c7d..ca9a2a208 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -656,3 +656,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (Phase B live pair — no code change; recorded for eval-history completeness) | canary runs 29763761133 (#53 baseline) + 29769050798 (#54 tuned) | ADDENDUM 4 Phase B verdict: staged 3-class tuner weights produced ZERO live movement — NOT adopted per the measured-gain rule | Offline tuner recommendations (document_lookup titleSection→0.9, table_threshold clinicalEvidence→0.95, comparison hybridRelevance→0.95; proxy comparison-mrr 0.833→1.0) staged via the B-PR-1 rag_ranking_config input; #54 log proves the override active (validation step echo). Result vs re-gated baseline: mrr@10 0.8921 vs 0.8922, irrelevant@10 0.1083 vs 0.1083, recalls 1.0/1.0 both, 36/36 both, all four headroom cases byte-identical rr (flowchart 0.20, ciwa-threshold 0.25, patient-safety 0.33, opioid 0.33). Learning: the 5-candidate linear proxy saturates; real headroom lives in fast-path saturated-tie structure (Phase C trigger condition mrr < 0.90 formally met; user authorized C). Nothing to roll back (per-run override). Spend ≈$2-4 of user-raised ≤$20 cap. | Both canary runs green (36/36 + answer-quality); override-active proof in #54 job log; no adoption = no code diff | | 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) | diff --git a/docs/observability-slos.md b/docs/observability-slos.md index c4053cda2..e6a501bab 100644 --- a/docs/observability-slos.md +++ b/docs/observability-slos.md @@ -179,6 +179,50 @@ Operational notes: `workflow_dispatch` run and confirm it goes green before trusting the weekly cadence (repo gate for this workflow). +### 3.1 Boundary-case and metric-interpretation policy (2026-07-20) + +Standing rules distilled from the ADDENDUM-4 eval/tuning cycle so the next +regression triage does not relearn them: + +- **Top-5 boundary cases are a human decision, never a silent retry.** When a + golden case's expected document sits at the top-5 boundary among multiple + legitimate sources (the alcohol-ciwa history), the fix is alias coverage or + a deliberately widened gate — chosen by a human with the run artifact open, + not by rerunning until green or by ranking nudges aimed at one fixture. +- **Fixture and snapshot move together.** The ranking snapshot's case count is + pinned to the golden fixture (tests/ranking-tuning.test.ts) and the snapshot + carries `generatedAt` provenance with a 30-day freshness gate. Regenerate + from the latest `eval-canary-output` artifact via + `npm run build:ranking-snapshot` — never hand-edit either file. +- **mrr@10 has a baseline step at 2026-07-20.** The lithium case was the only + ungated case (its rr was a hardcoded 0); gating it lifted measured mrr@10 by + ~+0.028 with zero retrieval change. Trend readers must treat that date as a + baseline reset, not an improvement. +- **`irrelevant_source_rate@10` is a labeling question before it is a ranking + question.** The rate is dominated by broad/vector cases whose extra top-10 + documents are topically adjacent (e.g. sibling guidelines). Before treating + the metric as ranking debt, audit whether those documents are under-labeled + relevant (the alias-tier lesson) — widening sanctioned labels may be the + correct fix and ranking changes aimed at the raw number may be optimizing + against mislabeled ground truth. +- **Ordering-headroom changes need a live pair, and two shapes are refuted.** + The residual rank-depth headroom on passing fast-path cases resisted both + per-class feature-weight tuning (live no-op, 2026-07-20 pair #53/#54) and a + saturation-tail spread of the primary/near-primary sort keys (live + regression, pair #54/#55 — spreading any comparator key ABOVE the + relevance score lets raw ts_rank override boost/title/subject-aware + ordering; see the branch-review ledger for the full post-mortems). Any third + attempt must (a) insert strictly BELOW the relevance key in the release + comparators, (b) carry a discriminating offline test that fails on the old + code with differently-relevant candidates (identical-content fixtures hide + this failure mode), and (c) run a dedicated before/after canary pair with + doc/content recall pinned at 1.0. +- **Artifact-based trends.** Every canary run uploads `eval-canary-output` + (30-day retention). `npm run eval:trend -- ` renders the + run-over-run metric table (and `--case ` a per-case rr trend) from + downloaded artifacts — the durable trend record without any new + infrastructure. + ## 4. Degradation counters on `/api/health` (shipped) The §2 reliability SQL is now also a scrape. An **authorized deep probe** — diff --git a/package.json b/package.json index be3434be1..979f266b3 100644 --- a/package.json +++ b/package.json @@ -152,6 +152,7 @@ "warm:retrieval": "node scripts/run-tsx.mjs scripts/warm-retrieval-cache.ts", "tune:search-weights": "node scripts/run-tsx.mjs scripts/tune-search-weights.ts", "build:ranking-snapshot": "node scripts/run-tsx.mjs scripts/build-ranking-snapshot.ts", + "eval:trend": "node scripts/eval-trend.mjs", "cleanup:storage": "node scripts/run-tsx.mjs scripts/cleanup-storage.ts", "purge:query-logs": "node scripts/run-tsx.mjs scripts/purge-query-logs.ts", "audit:tables": "node scripts/run-tsx.mjs scripts/audit-tables.ts", diff --git a/scripts/eval-trend.mjs b/scripts/eval-trend.mjs new file mode 100644 index 000000000..7ce6aa51f --- /dev/null +++ b/scripts/eval-trend.mjs @@ -0,0 +1,79 @@ +// Render a run-over-run metric trend from downloaded eval-canary artifacts. +// +// Every canary run uploads `eval-canary-output` (golden-retrieval.json, 30-day +// retention). Download the artifacts you want to compare, then: +// +// npm run eval:trend -- runA/golden-retrieval.json runB/golden-retrieval.json +// npm run eval:trend -- --case lithium-therapy-monitoring runA.json runB.json +// +// Files are ordered as given (pass oldest first). Offline and read-only: no +// providers, no repo state — the durable trend record without new infrastructure +// (docs/observability-slos.md §3.1). +import { readFileSync } from "node:fs"; + +/** One trend row per artifact payload; exported for tests. */ +export function buildTrendRows(payloads) { + return payloads.map(({ label, payload }) => { + const summary = payload?.summary ?? {}; + const results = Array.isArray(payload?.results) ? payload.results : []; + return { + label, + cases: results.length, + failed: Array.isArray(summary.failed_cases) ? summary.failed_cases.length : (summary.failed_cases ?? 0), + doc_recall_at_5: summary.document_recall_at_5 ?? null, + content_recall_at_5: summary.content_recall_at_5 ?? null, + mrr_at_10: summary.mrr_at_10 ?? null, + content_mrr_at_10: summary.content_mrr_at_10 ?? null, + irrelevant_at_10: summary.irrelevant_source_rate_at_10 ?? null, + p50_ms: summary.median_latency_ms ?? null, + p90_ms: summary.p90_latency_ms ?? null, + }; + }); +} + +/** Per-case reciprocal-rank trend across payloads; exported for tests. */ +export function buildCaseTrend(payloads, caseId) { + return payloads.map(({ label, payload }) => { + const match = (Array.isArray(payload?.results) ? payload.results : []).find((result) => result.id === caseId); + return { + label, + found: Boolean(match), + rr_at_10: match?.reciprocalRankAt10 ?? null, + content_rr_at_10: match?.contentReciprocalRankAt10 ?? null, + passed: match ? match.failures?.length === 0 || match.failures === undefined : null, + strategy: match?.retrievalStrategy ?? null, + }; + }); +} + +function formatTable(rows) { + if (!rows.length) return "(no rows)"; + const keys = Object.keys(rows[0]); + const cell = (value) => (typeof value === "number" ? Number(value.toFixed(4)).toString() : String(value ?? "-")); + const widths = keys.map((key) => Math.max(key.length, ...rows.map((row) => cell(row[key]).length))); + const line = (values) => values.map((value, i) => value.padEnd(widths[i])).join(" "); + return [ + line(keys), + line(widths.map((w) => "-".repeat(w))), + ...rows.map((row) => line(keys.map((k) => cell(row[k])))), + ].join("\n"); +} + +function main() { + const argv = process.argv.slice(2); + const caseFlag = argv.indexOf("--case"); + const caseId = caseFlag >= 0 ? argv[caseFlag + 1] : undefined; + const files = argv.filter((arg, index) => arg !== "--case" && index !== caseFlag + 1); + if (!files.length) { + console.error("Usage: eval-trend [--case ] (oldest first)"); + process.exit(2); + } + const payloads = files.map((file) => ({ label: file, payload: JSON.parse(readFileSync(file, "utf8")) })); + console.log(formatTable(buildTrendRows(payloads))); + if (caseId) { + console.log(`\nCase trend: ${caseId}`); + console.log(formatTable(buildCaseTrend(payloads, caseId))); + } +} + +if (process.argv[1] && process.argv[1].endsWith("eval-trend.mjs")) main(); diff --git a/tests/eval-trend.test.ts b/tests/eval-trend.test.ts new file mode 100644 index 000000000..b07cfa53d --- /dev/null +++ b/tests/eval-trend.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; +import { buildCaseTrend, buildTrendRows } from "../scripts/eval-trend.mjs"; + +const payload = (overrides: Record = {}) => ({ + label: "run-a.json", + payload: { + summary: { + document_recall_at_5: 1, + content_recall_at_5: 1, + mrr_at_10: 0.8921, + content_mrr_at_10: 0.9228, + irrelevant_source_rate_at_10: 0.1083, + median_latency_ms: 11895, + p90_latency_ms: 34045, + failed_cases: [], + }, + results: [ + { + id: "lithium-therapy-monitoring", + reciprocalRankAt10: 1, + contentReciprocalRankAt10: 0.75, + failures: [], + retrievalStrategy: "text_fast_path", + }, + { + id: "flowchart-next-step", + reciprocalRankAt10: 0.2, + contentReciprocalRankAt10: 0.78, + failures: [], + retrievalStrategy: "text_fast_path", + }, + ], + ...overrides, + }, +}); + +describe("eval-trend aggregation", () => { + it("builds one summary row per artifact in input order", () => { + const failing = payload({ + summary: { + document_recall_at_5: 0.9167, + content_recall_at_5: 0.9653, + mrr_at_10: 0.8138, + failed_cases: ["a", "b", "c"], + }, + }); + failing.label = "run-b.json"; + const rows = buildTrendRows([payload(), failing]); + expect(rows.map((row: { label: string }) => row.label)).toEqual(["run-a.json", "run-b.json"]); + expect(rows[0]).toMatchObject({ cases: 2, failed: 0, mrr_at_10: 0.8921, doc_recall_at_5: 1 }); + expect(rows[1]).toMatchObject({ failed: 3, mrr_at_10: 0.8138, doc_recall_at_5: 0.9167 }); + }); + + it("tracks a single case's reciprocal rank across runs and flags absences", () => { + const trend = buildCaseTrend([payload()], "flowchart-next-step"); + expect(trend[0]).toMatchObject({ found: true, rr_at_10: 0.2, strategy: "text_fast_path", passed: true }); + const missing = buildCaseTrend([payload()], "not-a-case"); + expect(missing[0]).toMatchObject({ found: false, rr_at_10: null, passed: null }); + }); +});