From 515c9d4cd2a7b752427631967a7d633ac44ab638 Mon Sep 17 00:00:00 2001 From: Dave Date: Sat, 17 Oct 2026 10:18:47 +0200 Subject: [PATCH] fix(collection-ai): one AI call per failed photo, card PO always paired, findings write is soft, guard both collections I7: a hash whose read failed this run is recorded as a failed copy with no second AI call. M4: the card's own PO lines are always offered for pairing alongside the POs the document names; the fallback finding still applies when none resolve. M5: a failed findings write no longer leaves the lines unchecked. M6: the schema guard requires ai_check on order_items and stock_purchase_items. Co-Authored-By: Claude Opus 5.5 --- src/lib/server/collection-ai-service.test.ts | 78 +++++++++++++++++++- src/lib/server/collection-ai-service.ts | 60 ++++++++++----- 2 files changed, 119 insertions(+), 19 deletions(-) diff --git a/src/lib/server/collection-ai-service.test.ts b/src/lib/server/collection-ai-service.test.ts index ea436eb..ab6afa1 100644 --- a/src/lib/server/collection-ai-service.test.ts +++ b/src/lib/server/collection-ai-service.test.ts @@ -12,6 +12,8 @@ const writes: { op: string; coll: string; id: string; data: Record { schemaChecks += 1; - return { name, fields: (name === "order_items" ? orderItemFields : []).map((f) => ({ name: f })) }; + const fields = name === "order_items" ? orderItemFields : name === "stock_purchase_items" ? stockItemFields : []; + return { name, fields: fields.map((f) => ({ name: f })) }; }, }, files: { @@ -186,6 +189,14 @@ const pb = { return structuredClone(row); }, update: async (id: string, patch: Record) => { + if ( + failFindingsUpdate && + name === "collection_doc_reads" && + Object.keys(patch).length === 1 && + "findings" in patch + ) { + throw new Error("findings write down"); + } const r = table(name).get(id); if (!r) throw new Error(`${name}/${id} not found`); Object.assign(r, structuredClone(patch)); @@ -453,6 +464,8 @@ beforeEach(() => { stockSkuCalls.length = 0; currentActor = null; orderItemFields = ["description", "ai_check"]; + stockItemFields = ["description", "ai_check"]; + failFindingsUpdate = false; schemaChecks = 0; idSeq = 0; failSkuFill = false; @@ -486,12 +499,73 @@ describe("checkCollectionCard", () => { expect(deps.readDocument).not.toHaveBeenCalled(); }); + it("refuses to run when stock_purchase_items has no ai_check field", async () => { + seedNss(); + stockItemFields = ["description"]; + const { deps } = makeDeps(); + + const r = await checkCollectionCard(PO_CEM, { deps }); + + expect(r.error).toBe(COLLECTION_AI_NOT_SET_UP_ERROR); + expect(writes).toEqual([]); + }); + + it("reads identical unreadable photos once: one AI call, a failed row for each copy", async () => { + seedNss({ attachments: ["a.jpg", "b.jpg", "c.jpg"] }); + const { deps, contents } = makeDeps({ + readDocument: vi.fn(async () => { + throw new Error("AI response was cut off (max_tokens)"); + }), + }); + contents["a.jpg"] = contents["b.jpg"] = contents["c.jpg"] = "blurry photo"; + + const r = await checkCollectionCard(PO_CEM, { deps }); + + expect(deps.readDocument).toHaveBeenCalledTimes(1); + expect(r.files_read).toBe(1); + const rows = reads(); + expect(rows.map((x) => x.filename).sort()).toEqual(["a.jpg", "b.jpg", "c.jpg"]); + expect(rows.every((x) => x.status === "failed" && x.error === "AI response was cut off (max_tokens)")).toBe(true); + const hash = createHash("sha256").update(Buffer.from("blurry photo")).digest("hex"); + expect(rows.every((x) => x.content_hash === hash)).toBe(true); + }); + + it("still writes the lines' checks when saving the document findings fails", async () => { + seedNss(); + failFindingsUpdate = true; + const { deps } = makeDeps(); + + const r = await checkCollectionCard(PO_CEM, { deps }); + + expect(r.error).toMatch(/findings write down/); + expect(aiCheck("order_items", "l1")?.doc_line?.code).toBe("SM293306030"); + expect(aiCheck("order_items", "l3")?.doc_line?.code).toBe("SM2NUT9B506"); + }); + + it("always pairs against the card's own PO as well as the POs the document names", async () => { + seedNss(); + const { deps } = makeDeps({ + // The document only prints the other PO number. + readDocument: vi.fn(async () => nssRead({ po_refs: [PO_CAS] })), + }); + + await checkCollectionCard(PO_CEM, { deps }); + + const poLines = deps.pairLines.mock.calls[0][1] as PairPoLine[]; + expect(poLines.map((l) => l.id).sort()).toEqual(["l1", "l2", "l3"]); + expect(aiCheck("order_items", "l1")?.doc_line?.code).toBe("SM293306030"); + const findings = reads()[0].findings as string[]; + expect(findings.some((f) => f.startsWith("No PO number on the document matched"))).toBe(false); + expect(findings.some((f) => f.includes("covers 2 POs"))).toBe(false); + }); + it("caches a positive schema check for the life of the process", async () => { seedNss(); const { deps } = makeDeps(); await checkCollectionCard(PO_CEM, { deps }); await checkCollectionCard(PO_CEM, { deps }); - expect(schemaChecks).toBe(1); + // One look at each line collection (order_items, stock_purchase_items), then cached. + expect(schemaChecks).toBe(2); }); it("returns the AI-disabled error and writes nothing when the read reports AI off", async () => { diff --git a/src/lib/server/collection-ai-service.ts b/src/lib/server/collection-ai-service.ts index 8aa9fec..32cd6fe 100644 --- a/src/lib/server/collection-ai-service.ts +++ b/src/lib/server/collection-ai-service.ts @@ -165,17 +165,21 @@ let aiCheckFieldConfirmed = false; /** * PocketBase silently drops unknown fields on update. Refuse to run until - * the migration has added `ai_check`. A positive answer is cached. + * the migration has added `ai_check` to BOTH line collections. A positive + * answer is cached. */ async function hasAiCheckField(pb: AdminPb): Promise { if (aiCheckFieldConfirmed) return true; - const collection = (await pb.collections.getOne(getOrderItemsCollectionName())) as { - fields?: Array<{ name: string }>; - schema?: Array<{ name: string }>; - }; - const fields = collection.fields ?? collection.schema ?? []; - aiCheckFieldConfirmed = fields.some((f) => f.name === "ai_check"); - return aiCheckFieldConfirmed; + for (const name of [getOrderItemsCollectionName(), STOCK_LINES]) { + const collection = (await pb.collections.getOne(name)) as { + fields?: Array<{ name: string }>; + schema?: Array<{ name: string }>; + }; + const fields = collection.fields ?? collection.schema ?? []; + if (!fields.some((f) => f.name === "ai_check")) return false; + } + aiCheckFieldConfirmed = true; + return true; } /** Test hook: forget the cached schema check. */ @@ -541,6 +545,8 @@ async function checkCard( const files = filesForLines(cardLines, stockPurchases); const readsByFile = await loadReadsForFiles(pb, files); const byHash = new Map(); + /** Hashes whose AI read failed in THIS run: copies aren't re-sent. */ + const failedByHash = new Map(); const queued: Array<{ file: FileRef; fields: ReadFields; replaceId: string | null }> = []; const readOneFile = async (file: FileRef): Promise => { @@ -567,6 +573,16 @@ async function checkCard( return; } const hash = createHash("sha256").update(fetched.bytes).digest("hex"); + const failedError = failedByHash.get(hash); + if (failedError !== undefined) { + // Same bytes already failed this run: record the copy, no AI call. + queued.push({ + file, + fields: { ...emptyRead(file, now), content_hash: hash, status: "failed", error: failedError }, + replaceId, + }); + return; + } let known = byHash.get(hash) ?? null; if (!known) { const found = await findReadByHash(pb, hash); @@ -595,6 +611,7 @@ async function checkCard( } else { fields.status = "failed"; fields.error = msg.slice(0, 2000); + failedByHash.set(hash, fields.error); fail(`read failed for ${file.filename}: ${msg}`); result.files_read += 1; } @@ -650,17 +667,21 @@ async function checkCard( for (const doc of docs) { try { - let resolved: PoRef[] = []; + let fromRefs: PoRef[] = []; if (doc.po_refs.length > 0) { poIndex ??= await loadPoIndex(pb); - resolved = resolveRefs(poIndex, doc.po_refs); + fromRefs = resolveRefs(poIndex, doc.po_refs); } - let fellBackToPo: string | null = null; - if (resolved.length === 0) { - resolved = cardPos; - fellBackToPo = poNumber; - } - const poLines = await loadLinesForPos(pb, resolved, false); + // The findings speak about the POs the document names (or the card's + // PO when none resolved) ... + const fellBackToPo: string | null = fromRefs.length === 0 ? poNumber : null; + const resolved = fromRefs.length > 0 ? fromRefs : cardPos; + // ... but the photo is on this card, so its own PO lines are always + // offered for pairing too. + const pairPos = [ + ...new Map([...fromRefs, ...cardPos].map((p) => [`${p.kind}:${p.id}`, p])).values(), + ]; + const poLines = await loadLinesForPos(pb, pairPos, false); const pairInput: PairPoLine[] = poLines.map((l) => ({ id: l.id, po_number: l.poNumber, @@ -705,7 +726,12 @@ async function checkCard( po_line_id: isRealPair(p.same_item) ? p.po_line_id : null, })), }); - await pb.collection(READS).update(doc.id, { findings }); + try { + await pb.collection(READS).update(doc.id, { findings }); + } catch (e) { + // The pairing worked: the lines still get their checks. + fail(`saving the findings of ${doc.document_no || doc.filename} failed: ${errMessage(e)}`); + } checkedDocs.push(doc); for (const l of poLines) {