From 27bd83948f32ffdbf1e298332d90fd16ec6f5955 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:20:51 +0000 Subject: [PATCH] fix(answer): fit the evidence preview under the stream cap instead of losing it at the boundary The wait showed no source cards on every non-fast answer. The preview builder sized its unit by source count alone (up to 12), while the stream contract bounds it by JSON size (64,000 chars). A real trimmed source is ~7,000 chars, not the ~900 the cap assumed, so a twelve-source unit was ~83,000 chars: built, sent to the route boundary, and dropped there as contract_rejected with nothing on screen. Fast routine answers select four passages and never hit the cap, which is why the browser proof stayed green. The builder now validates the exact unit it will emit with the same contract function and shrinks from the tail until it fits, in retrieval order. A source the contract rejects on its own is excluded individually. When nothing can ship, the wait records the new `undeliverable` reason for the setup-status diagnostic instead of emitting a unit that will be thrown away. selectedContextCount still names what governance let through. Adds a production-sized fixture to the offline contract proof so the builder's output is checked against the boundary it must cross. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Ron4eHAhoccBetbyqAJVvP --- ...fied-answer-incremental-delivery-design.md | 5 +- src/lib/answer-preview.ts | 53 ++++- src/lib/answer-stream-contract.ts | 7 +- tests/answer-incremental-delivery.test.ts | 188 ++++++++++++++++++ 4 files changed, 243 insertions(+), 10 deletions(-) diff --git a/docs/verified-answer-incremental-delivery-design.md b/docs/verified-answer-incremental-delivery-design.md index de464514c8..64eb8bc250 100644 --- a/docs/verified-answer-incremental-delivery-design.md +++ b/docs/verified-answer-incremental-delivery-design.md @@ -130,7 +130,10 @@ conflicts, and conclusions that depend on later sections are not independently e ### Phase 0 — offline contract proof - Add schema validation for `verifiedUnit`, sequence monotonicity, bounded payload size, and rejection - of `token` / `revising`. + of `token` / `revising`. (Amended 2026-09-05: the builder must fit the unit under the boundary's + size cap itself, by dropping sources from the tail, and never emit a unit the boundary will reject. + Until then a twelve-source unit of real-sized sources was ~83,000 characters against a 64,000 cap, + so every non-fast answer's rail was built and then discarded as `contract_rejected`.) - Add reconciliation tests proving every preview is an exact subset of `final` and is discarded on error, cancellation, retry, unknown schema version, or mismatch. - Add a source-governance fixture proving an outdated or poorly extracted danger-level source is diff --git a/src/lib/answer-preview.ts b/src/lib/answer-preview.ts index 4a8ffaf9b8..c82644b1c1 100644 --- a/src/lib/answer-preview.ts +++ b/src/lib/answer-preview.ts @@ -10,7 +10,11 @@ import { trimSourceForClient } from "@/lib/answer-client-payload"; import { env } from "@/lib/env"; import { hasDangerSourceGovernanceWarning, sourceGovernanceWarnings } from "@/lib/source-governance"; -import type { VerifiedEvidencePreviewUnit, VerifiedUnit } from "@/lib/answer-stream-contract"; +import { + isDeliverableVerifiedUnit, + type VerifiedEvidencePreviewUnit, + type VerifiedUnit, +} from "@/lib/answer-stream-contract"; import type { EvidenceRelevance, SearchResult } from "@/lib/types"; export type { VerifiedUnit }; @@ -39,6 +43,7 @@ export const evidencePreviewReasons = [ "empty_intersection_relaxed", "answer_level_danger", "all_sources_danger", + "undeliverable", "contract_rejected", ] as const; @@ -207,7 +212,8 @@ export function evaluateEvidencePreview(args: { const dangerDocumentIds = new Set(args.results.filter(isDangerLevelSource).map((result) => result.document_id)); const survivors = args.results.filter((result) => !dangerDocumentIds.has(result.document_id)); if (!survivors.length) return withheld("all_sources_danger"); - return { unit: buildUnit(survivors), reason: "ok" }; + const unit = buildUnit(survivors); + return unit ? { unit, reason: "ok" } : withheld("undeliverable"); } /** The unit alone, for callers that do not need to explain an absence. */ @@ -218,18 +224,51 @@ export function buildEvidencePreviewUnit(args: { return evaluateEvidencePreview(args).unit; } -function buildUnit(results: SearchResult[]): VerifiedEvidencePreviewUnit { - const selected = results.slice(0, evidencePreviewMaxSources); - return { +/** + * Build the unit so that it is guaranteed to pass the stream contract it is about to meet. + * + * **The builder used to size the unit by source count alone, while the contract bounds it by + * JSON size — and nothing checked one against the other.** `isDeliverableVerifiedUnit` caps a + * unit at 64,000 characters, sized on the assumption that "≤900 chars/source, ≤12 sources" + * leaves headroom. A real trimmed source is ~7,000 characters, not 900: the snippet is carried + * twice (`content` and `retrieval_synopsis`), and the score explanation, six generated labels, + * the indexing-quality record and the relevance chips all ride along by policy. Twelve of them + * is ~83,000 characters. Fast routine answers select four passages and stayed well under the + * cap, which is why the browser proof kept passing; every other route selects all of its + * candidates, so on precisely the long strong-route waits the rail exists for, the unit was + * built, sent to the boundary, and thrown away there as `contract_rejected`. The wait then + * showed no sources at all. + * + * The contract stays where it is — the boundary is the last line, not the only one — and the + * builder now shrinks to fit it: the same validator, applied here, on the exact unit that will + * be sent. Sources are dropped from the tail so what remains is the top of retrieval in + * retrieval order, which is what the rail draws. A source the contract would reject on its own + * (a malformed field, say) is excluded individually rather than taking the rail down with it. + * `selectedContextCount` keeps naming what governance let through, so the count never claims + * the shipped subset was the whole selection. + * + * Returns null only when not even one source can be delivered. + */ +function buildUnit(results: SearchResult[]): VerifiedEvidencePreviewUnit | null { + const unitOf = (sources: SearchResult[]): VerifiedEvidencePreviewUnit => ({ schemaVersion: 1, kind: "evidence_preview", sequence: 0, - sources: selected.map(trimSourceForClient), + sources, // Counts what survived governance, never the wider retrieval set: the stream contract // requires selectedContextCount >= sources.length, and a count drawn from passages that // were excluded would describe evidence the preview is deliberately not showing. selectedContextCount: results.length, - }; + }); + const deliverable = results + .slice(0, evidencePreviewMaxSources) + .map(trimSourceForClient) + .filter((source) => isDeliverableVerifiedUnit(unitOf([source]))); + for (let count = deliverable.length; count > 0; count -= 1) { + const unit = unitOf(deliverable.slice(0, count)); + if (isDeliverableVerifiedUnit(unit)) return unit; + } + return null; } /** Keep the ranking event small and keep final-path reconciliation out of the RAG monolith. diff --git a/src/lib/answer-stream-contract.ts b/src/lib/answer-stream-contract.ts index daf0853a67..88fd4ee502 100644 --- a/src/lib/answer-stream-contract.ts +++ b/src/lib/answer-stream-contract.ts @@ -26,8 +26,11 @@ export type VerifiedAnswerSectionUnit = { export type VerifiedUnit = VerifiedEvidencePreviewUnit | VerifiedAnswerSectionUnit; -// A unit is a bounded preview, never a transport for full documents. Sized to the -// client-source snippet policy (≤900 chars/source, ≤12 sources) with headroom. +// A unit is a bounded preview, never a transport for full documents. This cap is a ceiling +// the builder must fit under, not a size it can assume: a real trimmed source is ~7,000 JSON +// characters (the ≤900-char snippet is carried twice, plus scoring, labels, indexing quality +// and relevance), so twelve of them overrun it. `answer-preview.ts` shrinks the unit to fit +// this exact check before emitting; the check here is the boundary's own last line. const verifiedUnitMaxJsonChars = 64_000; const evidencePreviewMaxSources = 12; const clientSourceSnippetMaxChars = 900; diff --git a/tests/answer-incremental-delivery.test.ts b/tests/answer-incremental-delivery.test.ts index 2bd806329b..9d7e2653eb 100644 --- a/tests/answer-incremental-delivery.test.ts +++ b/tests/answer-incremental-delivery.test.ts @@ -308,8 +308,196 @@ describe("evidence preview builder (#100 Phase 1 server gate)", () => { expect(unit!.selectedContextCount).toBe(20); expect(isDeliverableVerifiedUnit(unit)).toBe(true); }); + + it("shrinks a production-sized preview to the contract's size cap instead of shipping a unit the boundary rejects", () => { + // The unit the builder emitted for twelve real-sized sources was ~83,000 JSON characters — + // the cap is 64,000 — so `toPublicAnswerProgressEvent` dropped it as `contract_rejected` + // and the wait showed no sources at all, on exactly the strong-route answers where the + // wait is longest. Fast routine answers select four passages and never hit it, which is + // why the browser proof (small synthetic sources) stayed green throughout. + const results = Array.from({ length: 12 }, (_, index) => makeProductionSizedSource(index)); + const oversized = JSON.stringify({ + schemaVersion: 1, + kind: "evidence_preview", + sequence: 0, + sources: results.map(trimSourceForClient), + selectedContextCount: results.length, + }).length; + expect(oversized).toBeGreaterThan(64_000); + + const fields = buildEvidencePreviewProgress({ normalResults: results, fallbackResults: results }); + expect(fields.previewReason).toBe("ok"); + const unit = fields.verifiedUnit!; + expect(unit).toBeDefined(); + expect(isDeliverableVerifiedUnit(unit)).toBe(true); + // Shrunk from the tail in retrieval order — the leading sources are the ones the rail + // draws — and the count still names what was selected, never what was shipped. + expect(unit.sources.length).toBeGreaterThanOrEqual(6); + expect(unit.sources.length).toBeLessThan(12); + expect(unit.sources.map((source) => source.id)).toEqual(results.slice(0, unit.sources.length).map((s) => s.id)); + expect(unit.selectedContextCount).toBe(12); + + // And the route boundary now keeps it, which is the whole point. + const publicEvent = toPublicAnswerProgressEvent({ stage: "ranking", message: "Selecting.", ...fields }, null); + expect(publicEvent?.verifiedUnit).toBeDefined(); + expect(publicEvent?.previewReason).toBe("ok"); + }); + + it("drops the one source the contract rejects and keeps the deliverable ones beside it", () => { + // A NaN similarity serialises as null and fails `isClientSource`. Before, one such source + // took the whole rail down at the boundary; now it is excluded on its own and the rest ship. + const broken = makeSource({ id: "chunk-broken", document_id: "doc-broken", similarity: Number.NaN }); + const unit = buildEvidencePreviewUnit({ results: [makeSource(), broken, makeSource({ id: "chunk-3" })] }); + expect(unit).not.toBeNull(); + expect(unit!.sources.map((source) => source.id)).toEqual(["chunk-1", "chunk-3"]); + expect(isDeliverableVerifiedUnit(unit)).toBe(true); + }); + + it("withholds with its own reason when no source can be delivered, rather than emitting a unit that will be rejected", () => { + const broken = makeSource({ similarity: Number.NaN }); + const fields = buildEvidencePreviewProgress({ normalResults: [broken], fallbackResults: [broken] }); + expect(fields.verifiedUnit).toBeUndefined(); + expect(fields.previewReason).toBe("undeliverable"); + expect(readLastEvidencePreviewReason()?.reason).toBe("undeliverable"); + }); }); +/** A source shaped and sized like the ones the live corpus actually returns: every client + * field populated, a 900-character snippet duplicated into `retrieval_synopsis`, a full score + * explanation, six generated labels and an indexing-quality record. Trimmed, it is ~7,000 JSON + * characters, against a 64,000-character unit cap. */ +function makeProductionSizedSource(index: number): SearchResult { + const document_id = `2f1d4c1e-9b8a-4a6e-8b0e-0f5b6f2d${String(index).padStart(4, "0")}`; + const snippet = + "Serum lithium should be measured 5 to 7 days after initiation or any dose change, then weekly until stable, then every 3 to 6 months. ".repeat( + 8, + ); + return { + id: `c9a5d1a2-7e6f-4d3c-9b2a-1e0f8d7c${String(index).padStart(4, "0")}`, + document_id, + title: `Lithium Prescribing and Monitoring Guideline - North Metropolitan Health Service Mental Health (${index})`, + file_name: `NMHS_MH_Lithium_Prescribing_Monitoring_Guideline_v3.2_2024_${index}.pdf`, + page_number: 12 + index, + chunk_index: 40 + index, + section_heading: "6.2 Serum lithium monitoring and dose adjustment in adults", + section_path: ["6 Monitoring", "6.2 Serum lithium monitoring and dose adjustment in adults", "6.2.1 Frequency"], + heading_level: 3, + parent_heading: "6 Monitoring", + anchor_id: "sec-6-2-1-frequency", + content: snippet, + retrieval_synopsis: snippet, + image_ids: ["img-1", "img-2"], + similarity: 0.8123456789012345, + similarity_origin: "cosine", + text_rank: 0.0612345678, + hybrid_score: 0.7345678901234567, + lexical_score: 0.4212345678, + rrf_score: 0.0323456789, + score_explanation: { + vectorScore: 0.8123456789012345, + textRank: 0.0612345678, + lexicalCoverageScore: 0.6666666666666666, + metadataMatchScore: 0.5, + sectionTitleMatchBoost: 0.04, + freshnessRecencyBoost: 0.02, + weightedHybridScore: 0.7345678901234567, + rrfScore: 0.0323456789, + rrfBoost: 0.0123456789, + memoryBoost: 0.05, + titleBoost: 0.08, + metadataBoost: 0.03, + clinicalSignalBoost: 0.06, + penalty: 0, + rawPenalty: 0, + rankScore: 1.0234567890123456, + releaseRankScore: 1.0234567890123456, + finalScore: 0.9345678901234567, + finalRank: index + 1, + preClampFinalScore: 1.0234567890123456, + fusionSignals: { + hybridRelevance: 0.7345678901234567, + lexicalCoverage: 0.6666666666666666, + reciprocalRankFusion: 0.0323456789, + titleSectionRelevance: 0.12, + metadataRelevance: 0.5, + clinicalEvidence: 0.06, + fixedAdjustment: 0, + }, + strategy: "weighted_hybrid_rrf_blend", + }, + source_strength: "strong", + source_metadata: { + source_kind: "document", + source_title: "Lithium Prescribing and Monitoring Guideline", + publisher: "North Metropolitan Health Service Mental Health, Public Health and Dental Services", + publisher_code: "NMHS", + jurisdiction: "Western Australia", + version: "3.2", + publication_date: "2024-03-01", + review_date: "2027-03-01", + uploaded_at: "2026-06-12T03:14:15.926Z", + indexed_at: "2026-06-12T03:19:26.535Z", + uploaded_by: "3b7f2c9e-1d4a-4f6b-8c2d-5e9a0b1c2d3e", + document_status: "current", + clinical_validation_status: "locally_reviewed", + clinical_validation_evidence: { + reviewer: "3b7f2c9e-1d4a-4f6b-8c2d-5e9a0b1c2d3e", + reviewed_at: "2026-06-13T01:00:00.000Z", + note: "Checked against the current intranet version.", + }, + extraction_quality: "good", + }, + document_labels: ["lithium", "mood stabiliser", "monitoring", "bipolar disorder", "renal function", "thyroid"].map( + (label, labelIndex) => ({ + id: `lbl-${index}-${labelIndex}`, + document_id, + owner_id: "3b7f2c9e-1d4a-4f6b-8c2d-5e9a0b1c2d3e", + label, + label_type: "topic", + source: "generated", + confidence: 0.91, + metadata: { model: "labeller", generated_at: "2026-06-12T03:19:26.535Z" }, + created_at: "2026-06-12T03:19:26.535Z", + updated_at: "2026-06-12T03:19:26.535Z", + }), + ), + memory_score: 0.12, + relevance: { + verdict: "direct", + label: "Directly addresses the question", + matchedTerms: ["lithium", "monitoring", "serum", "level", "dose"], + missingTerms: [], + directSourceCount: 1, + weakSourceCount: 0, + score: 0.91, + supportReason: "The passage directly covers lithium serum level monitoring frequency.", + isSourceBacked: true, + coverageScore: 1, + rankScore: 1.0234567890123456, + titleMatchedTerms: ["lithium", "monitoring"], + contentMatchedTerms: ["lithium", "monitoring", "serum", "level", "dose"], + metadataMatchedTerms: ["lithium"], + chips: ["Direct match", "WA guidance", "Current", "Locally reviewed"], + }, + match_explanation: { titleHit: true, labelHit: true, sectionHit: true, contentHit: true, tableHit: false }, + indexing_quality: { + document_id, + owner_id: "3b7f2c9e-1d4a-4f6b-8c2d-5e9a0b1c2d3e", + quality_score: 0.87, + extraction_quality: "good", + metrics: { pages: 48, chunks: 212, ocr_pages: 3, table_count: 9, avg_chunk_chars: 812, heading_coverage: 0.93 }, + issues: [], + updated_at: "2026-06-12T03:19:26.535Z", + }, + document_summary: "SERVER-ONLY summary", + adjacent_context: "SERVER-ONLY adjacent context", + memory_cards: [], + table_facts: [], + index_unit: null, + images: [], + } as unknown as SearchResult; +} + describe("public progress DTO passthrough", () => { it("passes a valid verified unit through the ranking stage", () => { const event = toPublicAnswerProgressEvent({ stage: "ranking", resultCount: 3, verifiedUnit: previewUnit() });