Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
44 changes: 44 additions & 0 deletions docs/observability-slos.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 -- <artifact.json...>` renders the
run-over-run metric table (and `--case <id>` 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** —
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
79 changes: 79 additions & 0 deletions scripts/eval-trend.mjs
Original file line number Diff line number Diff line change
@@ -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);
Comment on lines +64 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not drop the first artifact when --case is absent.

With no --case, caseFlag is -1, so index !== caseFlag + 1 becomes index !== 0. The documented two-file invocation therefore reports only the second run, while a single-file invocation exits with “no files”.

Proposed fix
   const caseFlag = argv.indexOf("--case");
   const caseId = caseFlag >= 0 ? argv[caseFlag + 1] : undefined;
-  const files = argv.filter((arg, index) => arg !== "--case" && index !== caseFlag + 1);
+  const files =
+    caseFlag >= 0
+      ? argv.filter((_, index) => index !== caseFlag && index !== caseFlag + 1)
+      : argv;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const caseFlag = argv.indexOf("--case");
const caseId = caseFlag >= 0 ? argv[caseFlag + 1] : undefined;
const files = argv.filter((arg, index) => arg !== "--case" && index !== caseFlag + 1);
const caseFlag = argv.indexOf("--case");
const caseId = caseFlag >= 0 ? argv[caseFlag + 1] : undefined;
const files =
caseFlag >= 0
? argv.filter((_, index) => index !== caseFlag && index !== caseFlag + 1)
: argv;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/eval-trend.mjs` around lines 64 - 66, Update the files filter in the
argument-parsing flow around caseFlag and caseId so it removes the argument
following --case only when --case is present; when absent, preserve every
artifact argument, including the first file, while continuing to exclude the
--case flag and its associated case identifier.

if (!files.length) {
console.error("Usage: eval-trend [--case <golden-case-id>] <golden-retrieval.json...> (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();
60 changes: 60 additions & 0 deletions tests/eval-trend.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { describe, expect, it } from "vitest";
import { buildCaseTrend, buildTrendRows } from "../scripts/eval-trend.mjs";

const payload = (overrides: Record<string, unknown> = {}) => ({
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 });
});
});
Loading