From 592d1e9aa803d29de66ac74d449ed2be21b7e969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 12 Aug 2026 21:12:53 +0200 Subject: [PATCH] fix(documents): let index-all rasterise PDFs for image-input providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content-index backfill built its candidate MIME list from the provider's native PDF support, so a provider that reads images but not PDFs never saw a single PDF in "index all documents" — while the per-document path already rasterises a PDF to page images for exactly that provider class. The prefilter was stricter than the authority it fronts. PDFs are always candidates now; prepareVisionInput stays the per document authority (native pass-through or rasterised pages). The run summary also stops folding outcomes into one number: skipped (nothing billable happened, retried later) and failed (a provider call missed) are counted separately, carried through the wide event and the worker log, and reruns stay idempotent because indexed documents drop out of the candidate set. Refs #776 --- .../document-content-index-backfill.test.ts | 108 +++++++++++++++++- .../jobs/document-content-index-backfill.ts | 48 +++++--- src/lib/jobs/job-outcome.ts | 2 + src/lib/jobs/reminder/register-maintenance.ts | 16 ++- 4 files changed, 154 insertions(+), 20 deletions(-) diff --git a/src/lib/jobs/__tests__/document-content-index-backfill.test.ts b/src/lib/jobs/__tests__/document-content-index-backfill.test.ts index dff362c26..b0817c6ff 100644 --- a/src/lib/jobs/__tests__/document-content-index-backfill.test.ts +++ b/src/lib/jobs/__tests__/document-content-index-backfill.test.ts @@ -18,6 +18,9 @@ vi.mock("@/lib/documents/store", () => ({ vi.mock("@/lib/labs/ocr-upload", () => ({ detectOcrMimeType: vi.fn(() => "image/png"), })); +vi.mock("@/lib/documents/rasterize-pdf", () => ({ + rasterizePdf: vi.fn(), +})); vi.mock("@/lib/documents/describe", () => ({ transcribeDocument: vi.fn().mockResolvedValue({ text: "glucose 90" }), DocumentDescribeError: class DocumentDescribeError extends Error {}, @@ -42,6 +45,9 @@ vi.mock("@/lib/jobs/boss-instance", () => ({ getGlobalBoss: vi.fn() })); import { runContentIndexBackfillForUser } from "../document-content-index-backfill"; import { prisma } from "@/lib/db"; +import { detectOcrMimeType } from "@/lib/labs/ocr-upload"; +import { rasterizePdf } from "@/lib/documents/rasterize-pdf"; +import { transcribeDocument } from "@/lib/documents/describe"; import { resolveDocumentVisionProvider } from "@/lib/documents/provider-order"; import { assertDocumentEgressConsent, @@ -86,7 +92,12 @@ describe("runContentIndexBackfillForUser", () => { pick: null, } as never); const result = await runContentIndexBackfillForUser("user-1"); - expect(result).toEqual({ indexed: 0, reason: "no-provider" }); + expect(result).toEqual({ + indexed: 0, + skipped: 0, + failed: 0, + reason: "no-provider", + }); expect(upsertContentIndex).not.toHaveBeenCalled(); }); @@ -96,7 +107,12 @@ describe("runContentIndexBackfillForUser", () => { new ConsentRequiredError("insights" as never), ); const result = await runContentIndexBackfillForUser("user-1"); - expect(result).toEqual({ indexed: 0, reason: "no-consent" }); + expect(result).toEqual({ + indexed: 0, + skipped: 0, + failed: 0, + reason: "no-consent", + }); expect(upsertContentIndex).not.toHaveBeenCalled(); }); @@ -110,10 +126,89 @@ describe("runContentIndexBackfillForUser", () => { ] as never); const result = await runContentIndexBackfillForUser("user-1"); - expect(result).toEqual({ indexed: 2, reason: "ok" }); + expect(result).toEqual({ + indexed: 2, + skipped: 0, + failed: 0, + reason: "ok", + }); expect(upsertContentIndex).toHaveBeenCalledTimes(2); }); + it("includes PDFs as candidates for an image-input provider and rasterises them", async () => { + // The per-document authority (`prepareVisionInput`) renders a PDF to page + // images when the provider cannot take PDFs natively. The candidate MIME + // prefilter must not be stricter than that, or "index all documents" + // silently leaves every PDF out for exactly those providers. + vi.mocked(resolveDocumentVisionProvider).mockResolvedValue({ + chain: [{ providerType: "codex", instance: {} }], + pick: { + entry: { providerType: "codex", instance: {} }, + providerType: "codex", + pdfSupported: false, + }, + } as never); + vi.mocked(prisma.inboundDocument.findMany).mockResolvedValue([ + { id: "pdf-1" }, + ] as never); + vi.mocked(detectOcrMimeType).mockReturnValueOnce( + "application/pdf" as never, + ); + vi.mocked(rasterizePdf).mockResolvedValue({ + ok: true, + images: [{ mediaType: "image/png", dataBase64: "cGFnZQ==" }], + } as never); + + const result = await runContentIndexBackfillForUser("user-1"); + + // The discovery walk asked for PDFs despite pdfSupported=false… + const where = vi.mocked(prisma.inboundDocument.findMany).mock.calls[0]?.[0] + ?.where as { mimeType: { in: string[] } }; + expect(where.mimeType.in).toContain("application/pdf"); + // …and the document reached `prepareVisionInput`, which rasterised it. + expect(transcribeDocument).toHaveBeenCalledWith( + expect.objectContaining({ + images: [{ mediaType: "image/png", dataBase64: "cGFnZQ==" }], + documents: [], + }), + ); + expect(result).toEqual({ + indexed: 1, + skipped: 0, + failed: 0, + reason: "ok", + }); + }); + + it("counts a PDF the rasteriser cannot render as skipped, not failed", async () => { + vi.mocked(resolveDocumentVisionProvider).mockResolvedValue({ + chain: [{ providerType: "codex", instance: {} }], + pick: { + entry: { providerType: "codex", instance: {} }, + providerType: "codex", + pdfSupported: false, + }, + } as never); + vi.mocked(prisma.inboundDocument.findMany).mockResolvedValue([ + { id: "pdf-1" }, + ] as never); + vi.mocked(detectOcrMimeType).mockReturnValueOnce( + "application/pdf" as never, + ); + vi.mocked(rasterizePdf).mockResolvedValue({ ok: false } as never); + + const result = await runContentIndexBackfillForUser("user-1"); + // No provider call happened, so this is a skip (retryable later), and the + // summary says so instead of folding it into silence. + expect(transcribeDocument).not.toHaveBeenCalled(); + expect(result).toEqual({ + indexed: 0, + skipped: 1, + failed: 0, + reason: "ok", + }); + }); + it("stops when the daily budget is reached (resumable)", async () => { vi.mocked(resolveDocumentVisionProvider).mockResolvedValue(PICK as never); vi.mocked(prisma.inboundDocument.findMany).mockResolvedValueOnce([ @@ -128,7 +223,12 @@ describe("runContentIndexBackfillForUser", () => { .mockResolvedValue({ allowed: true, reserved: 1 } as never); const result = await runContentIndexBackfillForUser("user-1"); - expect(result).toEqual({ indexed: 1, reason: "budget-reached" }); + expect(result).toEqual({ + indexed: 1, + skipped: 0, + failed: 0, + reason: "budget-reached", + }); expect(upsertContentIndex).toHaveBeenCalledTimes(1); }); }); diff --git a/src/lib/jobs/document-content-index-backfill.ts b/src/lib/jobs/document-content-index-backfill.ts index fa5f17f05..8ddd81594 100644 --- a/src/lib/jobs/document-content-index-backfill.ts +++ b/src/lib/jobs/document-content-index-backfill.ts @@ -15,10 +15,12 @@ * * Bounded + resumable: an id-cursor forward walk over the not-yet-indexed set, * capped at `MAX_DOCS_PER_RUN` provider calls per job and stopped the moment the - * daily AI budget is reached. Only vision-indexable MIME types (image + PDF when - * the provider reads PDFs) are candidates, so the walk converges — an - * un-indexable file never re-queues work. A re-run picks up where budget / - * the cap left off because already-indexed documents drop out of the set. + * daily AI budget is reached. Only vision-indexable MIME types (images + PDFs — + * `prepareVisionInput` passes a PDF natively when the provider reads PDFs and + * rasterises it to page images otherwise) are candidates, so the walk + * converges — an un-indexable file never re-queues work. A re-run picks up + * where budget / the cap left off because already-indexed documents drop out + * of the set. * * The queue MUST be registered in the maintenance registrar * (`src/lib/jobs/reminder/register-maintenance.ts`) so pg-boss provisions it. @@ -66,6 +68,14 @@ export interface ContentIndexBackfillPayload { export interface ContentIndexBackfillSummary { indexed: number; + /** + * Documents visited but not indexable this run (gone by load time, + * undecryptable, or a PDF no rasteriser could render). Refunded, never + * billed, and left un-indexed for a later run. + */ + skipped: number; + /** Documents whose provider call failed. Refunded; a later run retries. */ + failed: number; reason: "ok" | "no-provider" | "no-consent" | "budget-reached"; } @@ -78,7 +88,8 @@ export async function runContentIndexBackfillForUser( userId: string, ): Promise { const { pick } = await resolveDocumentVisionProvider(userId); - if (!pick) return { indexed: 0, reason: "no-provider" }; + if (!pick) + return { indexed: 0, skipped: 0, failed: 0, reason: "no-provider" }; try { await assertDocumentEgressConsent({ userId, @@ -87,17 +98,23 @@ export async function runContentIndexBackfillForUser( }); } catch (err) { if (err instanceof ConsentRequiredError) { - return { indexed: 0, reason: "no-consent" }; + return { indexed: 0, skipped: 0, failed: 0, reason: "no-consent" }; } throw err; } - const candidateMimes = pick.pdfSupported - ? [...IMAGE_MIMES, "application/pdf"] - : [...IMAGE_MIMES]; + // PDFs are always candidates: `prepareVisionInput` passes them natively + // when the provider reads PDFs and rasterises them to page images + // otherwise — the same path the per-document index route takes. The + // prefilter here only trims the walk; it must never be stricter than + // that per-document authority, or an image-input provider silently + // leaves every PDF out of "index all documents". + const candidateMimes = [...IMAGE_MIMES, "application/pdf"]; const dailyCap = resolveDailyCap([{ providerType: pick.entry.providerType }]); let indexed = 0; + let skipped = 0; + let failed = 0; let reason: ContentIndexBackfillSummary["reason"] = "ok"; let cursor: string | null = null; @@ -135,13 +152,17 @@ export async function runContentIndexBackfillForUser( const document = await loadOwnedDocument(userId, id); if (!document) { await reconcileSpend(userId, reservation.reserved, 0, dateKey); + skipped += 1; continue; } const vision = await prepareVisionInput(document, pick.pdfSupported); if (!vision.ok) { - // Not vision-indexable (should be filtered by MIME, but guard) — refund - // and move the cursor past it so the walk keeps converging. + // Not vision-indexable after all (undecryptable, unreadable bytes, + // or a PDF the rasteriser could not render) — refund and move on so + // the walk keeps converging. Counted as skipped, not failed: no + // provider call happened. await reconcileSpend(userId, reservation.reserved, 0, dateKey); + skipped += 1; continue; } @@ -170,6 +191,7 @@ export async function runContentIndexBackfillForUser( // A provider miss on one document must not abort the batch — refund and // leave it un-indexed (a later run retries it). await reconcileSpend(userId, reservation.reserved, 0, dateKey); + failed += 1; } } @@ -179,9 +201,9 @@ export async function runContentIndexBackfillForUser( annotate({ action: { name: "documents.contentIndex.backfill" }, - meta: { indexed, reason }, + meta: { indexed, skipped, failed, reason }, }); - return { indexed, reason }; + return { indexed, skipped, failed, reason }; } /** diff --git a/src/lib/jobs/job-outcome.ts b/src/lib/jobs/job-outcome.ts index 72e390f2b..8120255e9 100644 --- a/src/lib/jobs/job-outcome.ts +++ b/src/lib/jobs/job-outcome.ts @@ -75,7 +75,9 @@ export const JOB_FACT_ALLOWLIST: ReadonlySet = new Set([ "dispatched_period_soon", "document_purge_deleted", "documents", + "documents_failed", "documents_indexed", + "documents_skipped", "dose_changes_migrated", "downstream_failed", "drained", diff --git a/src/lib/jobs/reminder/register-maintenance.ts b/src/lib/jobs/reminder/register-maintenance.ts index 4a815dc03..b12a8207f 100644 --- a/src/lib/jobs/reminder/register-maintenance.ts +++ b/src/lib/jobs/reminder/register-maintenance.ts @@ -1148,6 +1148,8 @@ export async function registerMaintenanceQueues( let users = 0; let skipped = 0; let documentsIndexed = 0; + let documentsSkipped = 0; + let documentsFailed = 0; for (const job of jobs) { const { userId } = job.data; if (!userId) { @@ -1155,13 +1157,19 @@ export async function registerMaintenanceQueues( continue; } try { - const { indexed, reason } = - await runContentIndexBackfillForUser(userId); + const { + indexed, + skipped: docsSkipped, + failed: docsFailed, + reason, + } = await runContentIndexBackfillForUser(userId); users++; documentsIndexed += indexed; + documentsSkipped += docsSkipped; + documentsFailed += docsFailed; workerLog( "info", - `[document-content-index-backfill] user=${userId} indexed=${indexed} reason=${reason}`, + `[document-content-index-backfill] user=${userId} indexed=${indexed} skipped=${docsSkipped} failed=${docsFailed} reason=${reason}`, ); } catch (err) { recordError(); @@ -1178,6 +1186,8 @@ export async function registerMaintenanceQueues( users, skipped, documents_indexed: documentsIndexed, + documents_skipped: documentsSkipped, + documents_failed: documentsFailed, }); }, );