diff --git a/.github/workflows/eval-canary.yml b/.github/workflows/eval-canary.yml index 9bb720697..c6c6e39f0 100644 --- a/.github/workflows/eval-canary.yml +++ b/.github/workflows/eval-canary.yml @@ -184,7 +184,10 @@ jobs: run: | mkdir -p .local/eval-canary set -o pipefail - npm run eval:answer-quality 2>&1 | tee .local/eval-canary/answer-targeting.log + # --dump-answers captures per-case answer TEXTS (not retained anywhere at rest) so + # targeting misses are diagnosable offline; rides the existing artifact upload. + # Content sensitivity matches the tee'd log: all-public corpus, no env values. + npm run eval:answer-quality -- --dump-answers .local/eval-canary/answer-targeting-answers.json 2>&1 | tee .local/eval-canary/answer-targeting.log # Uploaded on success AND failure: the JSON artifact regenerates the offline ranking # snapshot (download it, then `npm run build:ranking-snapshot -- --input --output diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index a5612feed..cc05fc6e5 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -668,3 +668,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-21 | claude/clinical-kb-pwa-review-asi3wb (restarted from c308fdc; PR: labeling widen) | see PR head | USER-APPROVED ground-truth widening (AskUserQuestion 2026-07-21: "Widen to accept siblings") for the two admission-discharge cases: the WIDER alias tier's AdmissionCommunityPts entry gains the two NMHS "Admission to Discharge" titles (Community Mental Health + Mental Health Inpatients). Deliberate exclusions documented in-code: discharge-only docs must not satisfy the admission slot; the MHHITH programme policy is too narrow. STRICT golden tier untouched (bulk-merge prohibition respected). Verified by replaying run #57's actual top-5 lists through expectedFileCoverage: admission-discharge-coverage-paraphrase now passes; admission-discharge-comparison STILL FAILS honestly (its top-5 carries no admission-side doc at all — dup discharge-planning + Falls Prevention) and is retained as a genuine comparison-class retrieval-coverage signal, folded into the same investigation as the neuroleptic case. | typecheck PASS; prettier clean; probe script replay recorded (paraphrase allHit true / comparison allHit false, missing admission slot); no retrieval code touched; protected-file edit (eval-document-matching.ts) per explicit user authorization | | 2026-07-21 | claude/clinical-kb-pwa-review-asi3wb (PR: E-3b budget-aware generation deadlines) | 314d03f | Clinical-governance review of E-3b diff `git diff origin/main...HEAD` (commits 1078264 + 314d03f): answer-side generation timing/gating + telemetry — reserve-aware generation timeout (`generationRequestTimeoutMs`), truncation self-heal budget gate (`deadlineAllowsGenerationRetry`), `route_budget_exhausted_by_retrieval` telemetry, and the cross-region eval carve-out in scripts/eval-quality.ts. | APPROVE-WITH-NITS. No P0/P1/P2. (a) Conservative failure preserved: reserve-aware timeout fires ~2s early producing the SAME error type — an internal SDK timeout is mapped by mapOpenAIError to PublicApiError(openai_timeout) (openai.ts:525-529), NOT a bare DOMException, so the rag.ts:4636 re-throw guard is not tripped and the existing source-backed fallback/extractive recovery is reached; truncation-skip falls through to the terminal throw (rag.ts:4309-4313) into the same catch. No new answer-producing path. (b) Safety gates intact: all recovery answers finalize through finalizeAnswer→finalizeRagAnswerQuality and isSafeExtractiveFallbackCandidate (grounded/confidence/quality/numeric) — none touched. (c) Eval carve-out env-gated: crossRegionRunner && budgetExhaustedByRetrieval && generationMs===0; EVAL_LATENCY_CONTEXT set only in eval-canary.yml:164, prod caller (eval-quality.ts:1095) passes no options → inert in release/local; generationMs===0 requirement means a generation-side failure (generationMs>0) is never suppressed. (d)/(e) Telemetry additions non-PHI (boolean + mechanical retry-reason strings); no privacy/query-privacy/cross-border/verification source files touched; no new provider call. Nits (P3, non-blocking): carve-out also excuses fast/strong routes when generationMs===0 (sound — provably no generation ran); `requestTimeoutMs` now prod-dead (test-only); report label "retrieval-exhausted" (routeDeadlineExceeded && flag) is broader than actual gate suppression (audit label only, gate stays strict). | 88 offline unit tests PASS (tests/rag-route-budget.test.ts, tests/eval-quality.test.ts, tests/rag-offline-answer.test.ts, tests/rag-answer-fallback.test.ts); no provider/Supabase/OpenAI calls; no files mutated except this ledger row | | 2026-07-21 | claude/clinical-kb-pwa-review-asi3wb (PR: E-3b budget-aware generation deadlines) | 314d03f (+eb08ec5 review row) | ADDENDUM 5 wave E-3b implemented per the design-agent plan: generation attempts clamped to route budget minus a measured 2s recovery reserve (single call-site, all four attempt kinds); truncation self-heal gated on retry viability (reserve+5s floor) with observable truncation_retry_skipped_budget_reserve marker; additive route_budget_exhausted_by_retrieval runtime flag; eval route-ceiling gains the triple-condition cross-region carve-out (context + runtime flag + zero generation) with retrieval-exhausted audit cells — local/release gates provably strict. Fixes I3 (54ms budget overrun after 22.6s provider timeout), I5 (82s truncation waste class), resolves I2 (clozapine 13.3s retrieval vs 12s runtime budget = geography, now suppressed ONLY in the sanctioned cross-region context with full auditability). Reviewer verdicts: rag-retrieval-reviewer APPROVE-WITH-NITS (2 P3: prod-dead requestTimeoutMs retained for symmetry; report-cell coupling cosmetic; cached-replay invariant PROVEN — budget-exhausted answers never cached, carve-out unreachable via replay; marker isolation proven — SLO counters key on fallback_reason not answer_retry_reasons); clinical-governance-reviewer APPROVE-WITH-NITS (prior row) — internal-timeout→PublicApiError→existing-fallback path verified, all safety gates still applied to recovery answers. ALSO BANKED — E-2 targeting baseline (canary run #58, 29788404357, all-green incl. first execution of the !cancelled()-fixed instrument, ~$1-2): metric_rates relevance 0.6 / readability 1.0 / artifact_leaks 1.0 / intent_coverage 0.9333 / fail_closed 0.9; targeting_rate 0.5909 (13/22); by intent: document_lookup 5/5, red_result_action 3/3, contraindication 2/2, dose 1/5, monitoring_schedule 1/5, pathway_referral 1/2; all 9 misses = missing dose figure/schedule-interval (answer lengths 73-232 chars) → E-3c co-primary target alongside the wasted-generation class. Phase E spend ≈$3-6 of ≤$20. | Red-proofs: reserve pinned 3 independent ways (exact 23000ms grant, deadline flag clear, total under budget); self-heal skip pins exact marker + single provider call; offline flag pinned true/false. Focused: route-budget 9/9, eval-quality 27/27, fallback+offline 52/52, parser/abort regressions 15/15. Full suite 3043 passed / 1 known container pdf artifact. typecheck+lint+prettier clean. No provider calls; live proof = E-4 paired run | +| 2026-07-21 | claude/clinical-kb-pwa-review-asi3wb (restarted from e02ba3d; PR: E-3c PR-A instrument) | see PR head | ADDENDUM 5 wave E-3c design accepted (Plan agent, full report in session record) and PR-A delivered: eval-answer-quality gains --dump-answers (per-case answer TEXT + sections + targeting verdict JSON for the canary artifact — answers are not retained at rest by privacy design, so miss diagnosis needs eval-time capture), parseArgs/buildAnswerDumpRecord exported behind an import.meta main-guard (eval-quality precedent), canary targeting step wired with the flag. Design highlights for the record: DEFECT 1 root confirmed as fast-attempt-doomed-then-discarded on strong_routine_retrieval procedural shapes — PR-B generalizes the EXISTING validated-extractive short-circuit pattern (LAI + blocked-recovery precedents, hasValidatedExtractiveCandidate) to the measured shape via a new rag-extractive-first.ts module (net ~-110 rag.ts lines against the 5030 budget); DEFECT 2 rank-1 root = extractive lead-slot selection prefers shortest sentence and admits figure-less leads (sort at rag-extractive-answer.ts:917, 1-slot monitoring leads) — PR-C adds intent-figure-aware lead promotion with a claim-support atom-corpus nuke-proofing guard + dose/threshold fallback candidate preference (find(safe && figure) ?? find(safe)); answer-verification CLEARED as direct cause (whole-answer gap or unbold only, never per-figure deletion). H2 (strong-route comparison/complex residual) explicitly deferred as the named E-3d candidate. E-4 metrics set: discarded-generation <20% from ~47%, dose ≥3/5, monitoring ≥3/5, no intent below #58, recalls pinned 1.0, relevance/fail_closed/readability/artifact_leaks ≥ #58. | PR-A gates: new tests/eval-answer-quality.test.ts 4/4; typecheck, check:github-actions, check:ci-scope, eval-canary-workflow test, prettier all clean; no provider calls | diff --git a/scripts/eval-answer-quality.ts b/scripts/eval-answer-quality.ts index fdee44b36..6d51153cf 100644 --- a/scripts/eval-answer-quality.ts +++ b/scripts/eval-answer-quality.ts @@ -5,6 +5,9 @@ // This is a SEPARATE, opt-in script (npm run eval:answer-quality) — it does not touch eval:quality // or add spend to any existing run. All metrics are reported informationally; the script exits 0 // unless --fail-on-threshold is passed with an explicit floor, so it can be calibrated safely. +import { mkdir, writeFile } from "node:fs/promises"; +import { dirname } from "node:path"; +import { pathToFileURL } from "node:url"; import { loadEnvConfig } from "@next/env"; import { answerQualityEvalCases, @@ -27,9 +30,10 @@ type Args = { intent?: string; json: boolean; targetingFloor?: number; + dumpAnswers?: string; }; -function parseArgs(argv: string[]): Args { +export function parseArgs(argv: string[]): Args { const args: Args = { ownerEmail: process.env.RAG_EVAL_OWNER_EMAIL, ownerId: process.env.RAG_EVAL_OWNER_ID ?? process.env.LOCAL_NO_AUTH_OWNER_ID, @@ -48,10 +52,40 @@ function parseArgs(argv: string[]): Args { if (token === "--intent") args.intent = value; if (token === "--limit") args.limit = Number.parseInt(value, 10); if (token === "--targeting-floor") args.targetingFloor = Number.parseFloat(value); + if (token === "--dump-answers") args.dumpAnswers = value; } return args; } +// Per-case answer-text record for the --dump-answers artifact (E-3c instrument): answers are +// not retained anywhere at rest (privacy design), so diagnosing targeting misses (e.g. "no +// dose figure") needs the texts captured at eval time. Sections included because +// scoreAnswerTargeting scores over answer + sections. +export function buildAnswerDumpRecord( + testCase: AnswerQualityEvalCase, + answer: RagAnswer, + targeting: ReturnType, +) { + return { + id: testCase.id, + question: testCase.question, + intent: testCase.expectedIntent, + route: answer.routingMode ?? null, + routing_reason: answer.routingReason ?? null, + query_class: answer.queryClass ?? null, + grounded: answer.grounded, + confidence: answer.confidence, + citation_count: answer.citations.length, + answer_length: answer.answer?.length ?? 0, + targeting: { applicable: targeting.applicable, score: targeting.score, reason: targeting.reason }, + answer: answer.answer ?? "", + answer_sections: (answer.answerSections ?? []).map((section) => ({ + heading: section.heading, + body: section.body, + })), + }; +} + const METRICS: AnswerQualityMetric[] = ["relevance", "readability", "artifact_leaks", "intent_coverage", "fail_closed"]; async function main() { @@ -81,6 +115,7 @@ async function main() { let targetingHit = 0; const targetingMisses: Array<{ id: string; intent: string; reason: string; answer_length: number }> = []; const caseResults: Array> = []; + const dumpRecords: Array> = []; for (const testCase of cases) { const answer = (await withProviderBackoff(`answer-quality:${testCase.id}`, () => @@ -93,6 +128,7 @@ async function main() { } const targeting = scoreAnswerTargeting(testCase, answer); + if (args.dumpAnswers) dumpRecords.push(buildAnswerDumpRecord(testCase, answer, targeting)); const bucket = targetingByIntent.get(testCase.expectedIntent) ?? { applicable: 0, hit: 0 }; if (targeting.applicable) { bucket.applicable += 1; @@ -170,15 +206,29 @@ async function main() { } } + if (args.dumpAnswers) { + await mkdir(dirname(args.dumpAnswers), { recursive: true }); + await writeFile( + args.dumpAnswers, + JSON.stringify({ generated_at: new Date().toISOString(), case_count: caseCount, cases: dumpRecords }, null, 2), + ); + // Non-JSON mode only, mirroring the --json-out no-stdout-interference convention. + if (!args.json) console.log(` answer dump written: ${args.dumpAnswers} (${dumpRecords.length} case(s))`); + } + if (args.targetingFloor !== undefined && targetingRate !== null && targetingRate < args.targetingFloor) { console.error(`FAIL: targeting_rate ${targetingRate} < floor ${args.targetingFloor}`); process.exit(1); } } -main() - .then(() => process.exit(0)) - .catch((error) => { - console.error(error); - process.exit(1); - }); +// Main-guard (eval-quality.ts precedent) so tests can import parseArgs/buildAnswerDumpRecord +// without executing the provider-backed eval. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main() + .then(() => process.exit(0)) + .catch((error) => { + console.error(error); + process.exit(1); + }); +} diff --git a/tests/eval-answer-quality.test.ts b/tests/eval-answer-quality.test.ts new file mode 100644 index 000000000..9b4c17c1c --- /dev/null +++ b/tests/eval-answer-quality.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; + +import { buildAnswerDumpRecord, parseArgs } from "../scripts/eval-answer-quality"; +import type { RagAnswer } from "../src/lib/types"; +import { answerQualityEvalCases, scoreAnswerTargeting } from "../src/lib/rag/rag-eval-cases"; + +describe("eval-answer-quality argument parsing", () => { + it("leaves dump-answers off by default and keeps --json independent", () => { + const args = parseArgs(["--json"]); + expect(args.dumpAnswers).toBeUndefined(); + expect(args.json).toBe(true); + }); + + it("captures the --dump-answers path alongside other flags", () => { + const args = parseArgs(["--dump-answers", "/tmp/out.json", "--limit", "5"]); + expect(args.dumpAnswers).toBe("/tmp/out.json"); + expect(args.limit).toBe(5); + expect(args.json).toBe(false); + }); +}); + +describe("answer dump records", () => { + const testCase = answerQualityEvalCases[0]; + const answer = { + answer: "Check lithium levels at baseline and every 3 months.", + grounded: true, + confidence: "medium", + routingMode: "extractive", + routingReason: "high_confidence_extractive_retrieval", + queryClass: "medication_dose_risk", + citations: [{ chunk_id: "chunk-1" }], + answerSections: [{ heading: "Monitoring", body: "Baseline then 3-monthly.", citation_chunk_ids: ["chunk-1"] }], + sources: [], + } as unknown as RagAnswer; + + it("captures the answer text, sections, and targeting verdict", () => { + const targeting = scoreAnswerTargeting(testCase, answer); + const record = buildAnswerDumpRecord(testCase, answer, targeting); + expect(record.id).toBe(testCase.id); + expect(record.question).toBe(testCase.question); + expect(record.answer).toContain("lithium levels"); + expect(record.answer_sections).toEqual([{ heading: "Monitoring", body: "Baseline then 3-monthly." }]); + expect(record.citation_count).toBe(1); + expect(record.targeting).toEqual({ + applicable: targeting.applicable, + score: targeting.score, + reason: targeting.reason, + }); + expect(record.answer_length).toBe(answer.answer?.length ?? 0); + }); + + it("tolerates missing answer text and sections", () => { + const bare = { grounded: false, confidence: "unsupported", citations: [], sources: [] } as unknown as RagAnswer; + const targeting = scoreAnswerTargeting(testCase, bare); + const record = buildAnswerDumpRecord(testCase, bare, targeting); + expect(record.answer).toBe(""); + expect(record.answer_length).toBe(0); + expect(record.answer_sections).toEqual([]); + expect(record.route).toBeNull(); + }); +});