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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| 2026-08-14 | claude/rag-zod-hardening-tranche1 | 1fb98a174e815ce2d7e105d9339d766d3f54b02b | PR #1946 review-and-fix | fixed PR-introduced maintainability blocker; strengthened retrieval provenance and visual shape validation; merged latest main | offline: git diff --check; Prettier changed files; check:maintainability-budgets; branch-review-ledger; ledger-write-discipline; docs:check-links; CI pending |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| 2026-08-14 | claude/rag-zod-hardening-tranche1 | 0fded0878b6f8e35ef50b8c97a63af6e78e1f716 | rag.ts retrieval RPC row Zod shape contract (ledger #212 tranche 1) | Approved — 4 unchecked as SearchResult[] casts on retrieval RPC rows replaced with assertRetrievalRows; no ranking/ordering/scoring logic touched | verify:pr-local (all steps green except pre-existing check:medication-lexicon-report staleness on main); test 602 files/6517 passed; typecheck exit 0; eval:rag:offline 36 golden cases 23 suites, 579 passed |
129 changes: 129 additions & 0 deletions src/lib/rag/rag-row-contracts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { z } from "zod";
import { logger } from "@/lib/logger";
import { normalizeOptionalSourceMetadata } from "@/lib/source-metadata";
import type { SearchResult } from "@/lib/types";

/**
* Runtime shape contract for a row returned by a retrieval RPC.
*
* Retrieval rows are untrusted external data, not a compile-time guarantee. The RPCs are
* versioned (`*_v2` with a legacy fallback) and the live database has been observed to
* drift from the migrations in this repo — `docs/outstanding-issues.md` `#316` records ten
* retrieval RPC bodies diverging, with weekly live-drift red since 2026-07-26. Before this
* contract existed, `rag.ts` asserted those rows straight into the ranking pipeline with a
* bare `as SearchResult[]`, so a renamed column or a numeric returned as a string did not
* fail — it misranked or mis-cited silently, which is the worst failure mode for a clinical
* reference surface.
*
* The schema is deliberately asymmetric:
*
* - **Strict on the ranking, citation, and evidence fields.** The required chunk identity,
* provenance, and visual fields are `not null` in `supabase/schema.sql`, so requiring them
* cannot reject a row that works today. The four score fields are `.nullish()` — absent or
* null already flows through the downstream `?? 0` handling unchanged — but a *string where
* a number belongs* is rejected, which is precisely the silent-misranking case this exists
* to catch.
* - **Loose about everything else.** Column sets genuinely differ between RPC versions
* (`retrieval_synopsis` is absent from the older base hybrid function; `document_labels`
* and `document_summary` only appear on `match_document_chunks_v2`). `z.looseObject`
* preserves unknown keys rather than stripping them, so a harmless schema difference
* never becomes an outage or silent data loss.
*/
const retrievalImageSchema = z.looseObject({
id: z.string().min(1),
page_number: z.number().int().nullable(),
storage_path: z.string(),
caption: z.string(),
});

const retrievalRowSchema = z.looseObject({
id: z.string().min(1),
document_id: z.string().min(1),
title: z.string(),
file_name: z.string(),
page_number: z.number().int().nullable(),
chunk_index: z.number().int(),
section_heading: z.string().nullable(),
content: z.string(),
image_ids: z.array(z.string()),
source_metadata: z.record(z.string(), z.unknown()).nullable(),
images: z.array(retrievalImageSchema),
similarity: z.number().nullish(),
text_rank: z.number().nullish(),
hybrid_score: z.number().nullish(),
rrf_score: z.number().nullish(),
});

const retrievalRowsSchema = z.array(retrievalRowSchema);

/** Cap on reported issues; a wholesale shape change would otherwise report one per row. */
const MAX_REPORTED_ISSUES = 5;

/**
* Thrown when a retrieval RPC returns rows that do not satisfy the ranking contract.
*
* The message carries only Zod issue paths and codes — never a row value. Retrieval rows
* contain clinical document text, so echoing one into a log or an error response would
* leak source content past the privacy boundary that `query-privacy.ts` maintains.
*/
export class RetrievalRowShapeError extends Error {
readonly rpc: string;
readonly issues: string[];

constructor(rpc: string, issues: string[]) {
super(`Retrieval RPC "${rpc}" returned rows that do not match the ranking contract: ${issues.join("; ")}`);
this.name = "RetrievalRowShapeError";
this.rpc = rpc;
this.issues = issues;
}
}

function describeIssues(error: z.ZodError): string[] {
const described = error.issues
.slice(0, MAX_REPORTED_ISSUES)
.map((issue) => `${issue.path.join(".") || "<root>"}: ${issue.message}`);
const remaining = error.issues.length - described.length;
return remaining > 0 ? [...described, `and ${remaining} more`] : described;
}

/**
* Validate retrieval RPC rows before they enter the ranking pipeline.
*
* This asserts rather than transforms: on success the caller keeps the original array and
* row objects, so object identity, key order, and nested `images` / `source_metadata`
* references are unchanged from what the RPC returned. That matters because ranking is a
* live-validated protected surface (`docs/rag-behaviour/`) — validation must be observable
* only when the data is already wrong.
*
* Logs before throwing so drift stays visible even where a caller degrades: the vector
* fallback in `rag.ts` catches retrieval failures and falls back to lexical results when it
* has them, which would otherwise swallow the signal entirely.
*/
export function assertRetrievalRows(rows: unknown, rpc: string): asserts rows is SearchResult[] {
const parsed = retrievalRowsSchema.safeParse(rows);
if (parsed.success) return;
const issues = describeIssues(parsed.error);
logger.error("retrieval_row_shape_mismatch", {
rpc,
issues,
rowCount: Array.isArray(rows) ? rows.length : null,
});
throw new RetrievalRowShapeError(rpc, issues);
}

/** Build and validate the locally retrieved rows used as document-summary context. */
export function buildDocumentSummaryResults(
chunks: unknown[],
document: { title: string; file_name: string; metadata?: unknown },
): SearchResult[] {
const results = chunks.map((chunk) => ({
...(chunk as object),
title: document.title,
file_name: document.file_name,
source_metadata: normalizeOptionalSourceMetadata(document.metadata),
similarity: 1,
images: [],
}));
assertRetrievalRows(results, "document_summary_context");
return results;
}
27 changes: 13 additions & 14 deletions src/lib/rag/rag.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createAdminClient } from "@/lib/supabase/admin";
import { loadDocumentSummaryContext } from "@/lib/rag/rag-document-summary-context";
import { generationFailureDetailToken } from "@/lib/rag/rag-generation-failure-diagnostics";
import { assertRetrievalRows, buildDocumentSummaryResults } from "@/lib/rag/rag-row-contracts";
import { answerInstructions } from "@/lib/rag/rag-answer-instructions";
import { retrievalAccessScopeForArgs, retrievalRpcScopeArgs } from "@/lib/owner-scope";
import {
Expand Down Expand Up @@ -2179,13 +2180,17 @@ export async function searchChunksWithTelemetry(

const { data: hybridData, error: hybridError } = hybridResult;
if (hybridError) recordHybridRpcError(telemetry, "match_document_chunks_hybrid", hybridError);
telemetry.vector_candidate_count = hybridData?.length ?? 0;
recordRetrievalLayer(telemetry, "hybrid_vector", hybridData?.length ?? 0, {
// On a hybrid RPC error `hybridData` is null, so this validates an empty array and the
// existing hybrid-error -> vector-fallback path below is reached unchanged.
const hybridRows = hybridData ?? [];
assertRetrievalRows(hybridRows, "match_document_chunks_hybrid");
telemetry.vector_candidate_count = hybridRows.length;
recordRetrievalLayer(telemetry, "hybrid_vector", hybridRows.length, {
latencyMs: hybridResult.latencyMs,
topScore: layerTopScore((hybridData ?? []) as SearchResult[]),
topScore: layerTopScore(hybridRows),
});
const vectorCandidates = mergeSearchResults(
mergeSearchResults((hybridData ?? []) as SearchResult[], embeddingFieldCandidates),
mergeSearchResults(hybridRows, embeddingFieldCandidates),
indexUnitCandidates,
);

Expand Down Expand Up @@ -2257,7 +2262,9 @@ export async function searchChunksWithTelemetry(
);

if (error) throw new Error(error.message);
return (data ?? []) as SearchResult[];
const rows = data ?? [];
assertRetrievalRows(rows, "match_document_chunks");
return rows;
}),
).catch((error) => {
if (!args.forceEmbedding && textFastResults.length > 0) return [] as SearchResult[][];
Expand Down Expand Up @@ -4304,15 +4311,7 @@ export async function summarizeDocument(documentId: string, ownerId?: string, op
} satisfies RagAnswer;
}

const documentMetadata = (document as { metadata?: unknown }).metadata;
const results = committedChunks.map((chunk) => ({
...chunk,
title: document.title,
file_name: document.file_name,
source_metadata: normalizeOptionalSourceMetadata(documentMetadata),
similarity: 1,
images: [],
})) as SearchResult[];
const results = buildDocumentSummaryResults(committedChunks, document);

const summaryInstructions = `Summarize a clinical document for practical psychiatric use in Perth, Australia.
Use only the excerpts provided. Use a layered response: make the answer field a plain high-yield clinical paragraph,
Expand Down
146 changes: 146 additions & 0 deletions tests/rag-retrieval-row-contract.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { describe, expect, it, vi } from "vitest";
import { RetrievalRowShapeError, assertRetrievalRows, buildDocumentSummaryResults } from "@/lib/rag/rag-row-contracts";

vi.mock("@/lib/logger", () => ({
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
}));

// A realistic `match_document_chunks_hybrid_v2` row. Column list mirrors the RPC's
// `returns table (...)` in supabase/migrations/20260713020000_owner_plus_public_retrieval.sql.
/** A row with one column dropped, standing in for an RPC whose shape has drifted. */
function withoutColumn(column: string, overrides: Record<string, unknown> = {}) {
const row: Record<string, unknown> = hybridRow(overrides);
delete row[column];
return row;
}

function hybridRow(overrides: Record<string, unknown> = {}) {
return {
id: "3f1a2b6c-1111-4aaa-8bbb-000000000001",
document_id: "3f1a2b6c-2222-4aaa-8bbb-000000000002",
title: "RANZCP Mood Disorders Guideline",
file_name: "ranzcp-mood.pdf",
page_number: 14,
chunk_index: 7,
section_heading: "Lithium monitoring",
content: "Check serum lithium 5 days after any dose change.",
retrieval_synopsis: null,
image_ids: [],
source_metadata: { document_status: "current" },
similarity: 0.91,
text_rank: 0.42,
hybrid_score: 0.88,
rrf_score: 0.031,
images: [],
...overrides,
};
}

describe("retrieval row shape contract", () => {
it("accepts a realistic hybrid RPC row without mutating it", () => {
const rows: unknown = [hybridRow()];
const before = structuredClone(rows);
const firstRowReference = (rows as unknown[])[0];

expect(() => assertRetrievalRows(rows, "match_document_chunks_hybrid")).not.toThrow();

// Assert, do not transform: ranking is a live-validated protected surface, so a valid
// row must reach it byte-identical and by the same reference.
expect(rows).toEqual(before);
expect((rows as unknown[])[0]).toBe(firstRowReference);
});

it("builds valid document-summary rows outside the retrieval monolith", () => {
const chunk = hybridRow({ title: "stale", file_name: "stale.pdf", similarity: 0.2 });

const rows = buildDocumentSummaryResults([chunk], {
title: "Current title",
file_name: "current.pdf",
metadata: { document_status: "current" },
});

expect(rows[0]).toMatchObject({
id: chunk.id,
title: "Current title",
file_name: "current.pdf",
similarity: 1,
source_metadata: expect.objectContaining({ document_status: "current" }),
});
});

it("preserves unknown columns so an RPC version difference is not data loss", () => {
const rows: unknown = [hybridRow({ document_labels: [{ id: "l1" }], a_future_column: 42 })];

assertRetrievalRows(rows, "match_document_chunks_v2");

expect(rows[0]).toMatchObject({ document_labels: [{ id: "l1" }], a_future_column: 42 });
});

it("rejects a numeric score returned as a string", () => {
const rows: unknown = [hybridRow({ similarity: "0.91" })];

expect(() => assertRetrievalRows(rows, "match_document_chunks_hybrid")).toThrow(RetrievalRowShapeError);
});

it("rejects a row missing its chunk identity", () => {
const rows: unknown = [withoutColumn("id")];

expect(() => assertRetrievalRows(rows, "match_document_chunks_hybrid")).toThrow(RetrievalRowShapeError);
expect(() => assertRetrievalRows([hybridRow({ document_id: "" })], "x")).toThrow(RetrievalRowShapeError);
});

it("rejects malformed provenance and visual fields before they can miscite or crash rendering", () => {
expect(() => assertRetrievalRows([withoutColumn("title")], "match_document_chunks_hybrid")).toThrow(
RetrievalRowShapeError,
);
expect(() =>
assertRetrievalRows([hybridRow({ image_ids: "not-an-array" })], "match_document_chunks_hybrid"),
).toThrow(RetrievalRowShapeError);
expect(() => assertRetrievalRows([hybridRow({ images: [{}] })], "match_document_chunks_hybrid")).toThrow(
RetrievalRowShapeError,
);
});

it("accepts absent or null scores, which downstream already coalesces to 0", () => {
expect(() => assertRetrievalRows([withoutColumn("text_rank")], "match_document_chunks")).not.toThrow();
expect(() => assertRetrievalRows([hybridRow({ rrf_score: null })], "match_document_chunks")).not.toThrow();
expect(() => assertRetrievalRows([], "match_document_chunks")).not.toThrow();
});

it("names the RPC and leaks no row content in the error", () => {
const secret = "Check serum lithium 5 days after any dose change.";
let thrown: RetrievalRowShapeError | null = null;
try {
assertRetrievalRows([hybridRow({ hybrid_score: "0.88" })], "match_document_chunks_hybrid");
} catch (error) {
thrown = error as RetrievalRowShapeError;
}

expect(thrown).toBeInstanceOf(RetrievalRowShapeError);
expect(thrown?.rpc).toBe("match_document_chunks_hybrid");
expect(thrown?.message).toContain("match_document_chunks_hybrid");
expect(thrown?.message).toContain("hybrid_score");
// Retrieval rows carry clinical document text; it must never reach a log or a response.
expect(thrown?.message).not.toContain(secret);
expect(thrown?.message).not.toContain("ranzcp-mood.pdf");
});

it("caps the reported issues so a wholesale shape change stays readable", () => {
const rows = Array.from({ length: 20 }, () => hybridRow({ similarity: "nope" }));

let thrown: RetrievalRowShapeError | null = null;
try {
assertRetrievalRows(rows, "match_document_chunks_hybrid");
} catch (error) {
thrown = error as RetrievalRowShapeError;
}

expect(thrown?.issues).toHaveLength(6);
expect(thrown?.issues.at(-1)).toBe("and 15 more");
});

it("rejects a payload that is not an array of rows", () => {
expect(() => assertRetrievalRows({ rows: [] }, "match_document_chunks_hybrid")).toThrow(RetrievalRowShapeError);
expect(() => assertRetrievalRows(null, "match_document_chunks_hybrid")).toThrow(RetrievalRowShapeError);
});
});
Loading