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 <noreply@anthropic.com>
This commit is contained in:
parent
d4cdde7384
commit
515c9d4cd2
2 changed files with 119 additions and 19 deletions
|
|
@ -12,6 +12,8 @@ const writes: { op: string; coll: string; id: string; data: Record<string, unkno
|
|||
const events: string[] = [];
|
||||
let currentActor: Actor | null = null;
|
||||
let orderItemFields = ["description", "ai_check"];
|
||||
let stockItemFields = ["description", "ai_check"];
|
||||
let failFindingsUpdate = false;
|
||||
let schemaChecks = 0;
|
||||
let idSeq = 0;
|
||||
let failSkuFill = false;
|
||||
|
|
@ -152,7 +154,8 @@ const pb = {
|
|||
collections: {
|
||||
getOne: async (name: string) => {
|
||||
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<string, unknown>) => {
|
||||
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 () => {
|
||||
|
|
|
|||
|
|
@ -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<boolean> {
|
||||
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<string, ReadFields>();
|
||||
/** Hashes whose AI read failed in THIS run: copies aren't re-sent. */
|
||||
const failedByHash = new Map<string, string>();
|
||||
const queued: Array<{ file: FileRef; fields: ReadFields; replaceId: string | null }> = [];
|
||||
|
||||
const readOneFile = async (file: FileRef): Promise<void> => {
|
||||
|
|
@ -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) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue