mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(documents): let index-all rasterise PDFs for image-input providers
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
This commit is contained in:
1 parent
3254e44281
commit
592d1e9aa8
4 files changed
+154
-20
No files matched your search
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<ContentIndexBackfillSummary> {
|
||||
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 };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -75,7 +75,9 @@ export const JOB_FACT_ALLOWLIST: ReadonlySet<string> = new Set([
|
||||
"dispatched_period_soon",
|
||||
"document_purge_deleted",
|
||||
"documents",
|
||||
"documents_failed",
|
||||
"documents_indexed",
|
||||
"documents_skipped",
|
||||
"dose_changes_migrated",
|
||||
"downstream_failed",
|
||||
"drained",
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user