harden(worker): validate claimed job rows and enrichment read-backs against Zod row contracts (#212 T4) - #2037
Conversation
…gainst Zod row contracts (#212 T4) Replace the two inbound as-unknown-as surfaces in worker/main.ts with assertion contracts in a new worker-local row-contracts module: the claim_ingestion_jobs result is validated per row (fail-soft partition; a malformed row is failed terminally instead of throwing outside the job lifecycle), and loadEnrichmentRows asserts its chunk/image read-backs, letting both deep-memory parameter casts drop. Outbound insert-payload casts are deliberately untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWVin3ToF7qvWjnCB9Ti3X
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:49 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 95 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
Comment |
…re, and record the branch review Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWVin3ToF7qvWjnCB9Ti3X
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #11389 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…nche-4-worker-q3y6i4 # Conflicts: # docs/rag-improvement/HANDOVER.md
… cancellations CI's docs:check-links (and every other planRequestBatch consumer) fails on the merge ref because tranche 3's #212 update request and this PR's #212 done request are both pending. Cancel both and queue one closure request that carries the T3 row correction and the T4 outcome, leaving exactly one pending mutation for #212. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWVin3ToF7qvWjnCB9Ti3X
Uh oh!
There was an error while loading. Please reload this page.
Summary
#212tranche 4 (final tranche): audit of all 12as unknown ascasts inworker/**(11 inworker/main.ts, 1 inworker/index.ts), replacing the inbound ones with Zod-backed assertion contracts. Tranches 1–3 coveredsrc/lib/rag/rag.ts(harden(rag): validate retrieval RPC rows against a Zod shape contract #1946),rag-candidate-sources.ts(harden(rag): validate signal rows from the candidate-source RPCs #1981), andsrc/app/api/**(harden(api): validate inbound DB and RPC payloads against Zod row contracts #2023).claim_ingestion_jobsRPC result inclaimJobs(worker/main.ts:407), whosedocumentsfield isto_jsonb(d.*)— untyped jsonb asserted intoJobRowwith nothing checking it. Two further sites (main.ts:1805-1806) were parameter casts papering over unvalidated read-backs fromdocument_chunks/document_images. The remaining nine are outbound insert payloads, aJson-shape cast, or CJS interop, and are deliberately untouched — the same "most sites needed nothing" finding as tranches 2 and 3.worker/row-contracts.ts: worker-local counterpart ofsrc/lib/validation/row-contracts.ts/src/lib/rag/rag-row-contracts.ts, deliberately duplicating their small validate-log-throw core (z.looseObjectonly;assertshelpers oversafeParse;WorkerRowShapeErrorcarrying Zod issue paths only, never row values; pins cited tosupabase/schema.sqlconstraints). Worker-local because the esbuild worker bundle has no@/alias; import-pure so Vitest can exercise it (worker/main.tsitself has zero executable coverage).claimJobsnow validates per row, fail-soft viapartitionClaimedJobRows: valid rows proceed by the same references in the same order; a row failing the contract is failed terminally (retry: false) through the existingfail_or_retry_ingestion_jobpath, and a row whose ids are themselves untrustworthy is logged and left to the bounded stale reclaim. No shape problem can throw out ofclaimJobs.loadEnrichmentRowsasserts its chunk/image read-backs, which lets bothupsertDocumentDeepMemoryparameter casts drop with no replacement cast. A mismatch there is contained by the existing inline-enrichment try: enrichment is marked failed, the job still completes, repair is queued.failOrRetryJob'sjobparameter is structurally narrowed (Pick<JobRow, ...>) so a contract-rejected row whosedocumentspayload cannot be trusted can still be failed; the ordinary full-JobRowcaller is unchanged.tests/worker-row-contract.test.ts(28 cases) mirroring the tranche-3 test shape: no-mutation/reference-identity, unknown-column preservation at both nesting levels, schema-backed rejections, no row values in errors, issue cap, and the partition semantics (sibling survival, failable identity, indexed-status preservation, unusable-id fallback).Per-site disposition of all 12 casts:
main.ts:407claim RPC resultmain.ts:1805/1806deep-memory paramsloadEnrichmentRows; casts removedmain.ts:532document_index_qualityupsertsanitizeJsonbRecordappliedmain.ts:808caption-cache metadataas Jsonmain.ts:1399/1496/1578document_embedding_fieldsinsertsembedding: number[]vs pgvector string;assertEmbeddingDimvalidates the risky fieldmain.ts:1478document_chunksinsertsatisfies IndexedChunkRow[]+assertEmbeddingDim+sanitizeJsonbRecordmain.ts:1517document_table_factsinsertmain.ts:1546document_index_unitsinsertworker/index.ts:8@next/envinteropVerification
npm run verify:pr-localnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsUI verification not run: no UI, routing, styling, or browser surface changed (worker + tests + docs only).
Live eval gates not run: no retrieval, ranking, selection, chunking-order, or answer-generation behavior changed; no file under
src/lib/rag/**or any RAG-ranking-protected surface is touched.npm run check:production-readiness(ingestion domain change; offline capability gaps reported as expected without provider credentials)npx vitest run tests/worker-row-contract.test.ts— 28 passed; adjacent suites (worker-run-loop,worker-behavior,worker-safe-logging,worker-bundle,architecture-boundaries,api-row-contract,rag-retrieval-row-contract) — 72 passed.npm run typecheck,npm run lint(--max-warnings 0), fullnpm run test(640 files, 6846 passed | 4 skipped), andnode scripts/build-worker.mjs(esbuild bundle proof for the new relative import) all green.verify:pr-localsummary: completed lint, typecheck, test, build, RAG fixture checks, ledger checks —failed: (none).Risk and rollout
TypeErrorbeforeprocessJob's try/catch, so the job is never marked failed and sits leased until the 45-minute stale reclaim while silently burning attempts; with this change it is terminally failed at claim time with the Zod issue paths inerror_message. A contract mismatch is deterministic (retrying reruns the sameto_jsonbover the same columns), so terminal fail cannot create a poison-pill retry loop — and per-row partition means one bad row can no longer take down claimed siblings or the--onceprocess. Enrichment-side mismatches degrade exactly as enrichment failures already do (job completes, repair queued). Residual false-rejection risk is confined to the two documented beyond-constraint pins (jsonb record-ness, document-status enum), both of which are claims the deleted casts were already making silently.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
error_messagecolumns and Sentry.as unknown asinventory): unguarded bareascasts on extractor output (tableMetadata.tableRows,main.ts:693/1004) and on the OpenAI visionclassificationwidening (main.ts:798-800/1100/1115). Recorded in the#212closure note rather than bundled here — each needs its own reader-by-reader pass.#212: the closure is queued as an immutable inbox request (npm run issues:done), not a canonical-ledger edit, and the#212 T4row indocs/rag-improvement/HANDOVER.md§2 is updated in this PR.Generated by Claude Code