diff --git a/scripts/pb-add-dn-staged-ingestion.py b/scripts/pb-add-dn-staged-ingestion.py index c05ff79..cfa21ad 100644 --- a/scripts/pb-add-dn-staged-ingestion.py +++ b/scripts/pb-add-dn-staged-ingestion.py @@ -399,6 +399,11 @@ def main(): relation("supplier", supplier_id), field("supplier_name_raw"), field("match_method"), field("match_evidence", "json", maxSize=2000000), field("candidate_key"), field("posted_at", "date"), + # What a person set by hand, kept apart from what the model read. + # Every run rebuilds a candidate from its page reads, so without + # this an operator's corrected order number is overwritten by the + # original misread on the very next run. + field("manual_overrides", "json", maxSize=100000), ]) add_indexes(token, "dn_batch_notes", [ "CREATE UNIQUE INDEX `idx_dn_batch_notes_candidate_key` " diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/route.test.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/route.test.ts index ebd50ca..967ef07 100644 --- a/src/app/api/dn-batches/[id]/notes/[noteId]/route.test.ts +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/route.test.ts @@ -317,7 +317,9 @@ describe("PATCH /api/dn-batches/[id]/notes/[noteId] — ownership", () => { params: Promise.resolve({ id: BATCH_ID, noteId: NOTE_ID }), }); expect(dnBatchNotesGetOne).toHaveBeenCalledWith(NOTE_ID, { - fields: "id,batch,status", + // manual_overrides too: a correction is merged into what is already + // recorded rather than replacing it. + fields: "id,batch,status,manual_overrides", }); }); }); @@ -381,9 +383,41 @@ describe("PATCH /api/dn-batches/[id]/notes/[noteId] — write & response", () => page_from: 1, page_to: 3, delivery_note_number: "DN-77", + // The document number is rebuilt from the page reads on every run, + // so correcting it has to be recorded as a decision or the original + // misread returns on the next one. Quantity, unit and the page range + // are not rebuilt, so they need no such record. + manual_overrides: { delivery_note_number: "DN-77" }, }); }); + it("merges a new correction into the ones already recorded", async () => { + dnBatchNotesGetOne.mockResolvedValueOnce({ + id: NOTE_ID, batch: BATCH_ID, status: "blocked", + manual_overrides: { delivery_note_number: "DN-77" }, + }); + + const res = await PATCH(patch({ po_number_raw: "PO-9165" }), { + params: Promise.resolve({ id: BATCH_ID, noteId: NOTE_ID }), + }); + + expect(res.status).toBe(200); + expect(allUpdates()[0].data).toEqual({ + po_number_raw: "PO-9165", + manual_overrides: { delivery_note_number: "DN-77", po_number_raw: "PO-9165" }, + }); + await expect(res.json()).resolves.toEqual(expect.objectContaining({ overridden: ["po_number_raw"] })); + }); + + it("records nothing extra when the edited field is not rebuilt by a run", async () => { + const res = await PATCH(patch({ quantity: 12 }), { + params: Promise.resolve({ id: BATCH_ID, noteId: NOTE_ID }), + }); + + expect(res.status).toBe(200); + expect(allUpdates()[0].data).toEqual({ quantity: 12 }); + }); + it("returns 500 with the error message when PB update throws", async () => { dnBatchNotesUpdate.mockRejectedValueOnce(new Error("PB write failed")); const res = await PATCH(patch({ quantity: 5 }), { diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/route.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/route.ts index 3b1315c..38d208c 100644 --- a/src/app/api/dn-batches/[id]/notes/[noteId]/route.ts +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/route.ts @@ -33,6 +33,17 @@ const EDITABLE = new Set([ const NUMERIC = new Set(["quantity", "gross_mass", "tare_mass", "nett_mass", "page_from", "page_to"]); +/** + * Fields a re-run rebuilds from the page reads, and would therefore + * overwrite. Correcting one has to be recorded as a decision rather than + * a value, or the original misread simply returns on the next run. + * + * Page ranges are deliberately absent: which pages belong to a document + * is the assembler's answer, corrected through the assembly route so the + * whole batch stays consistent. + */ +const OVERRIDABLE = new Set(["po_number_raw", "delivery_note_number"]); + export async function PATCH( request: Request, { params }: { params: Promise<{ id: string; noteId: string }> }, @@ -87,7 +98,7 @@ export async function PATCH( try { const note = await pb .collection("dn_batch_notes") - .getOne(noteId, { fields: "id,batch,status" }); + .getOne(noteId, { fields: "id,batch,status,manual_overrides" }); if (note.batch !== id) { return NextResponse.json({ error: "note_not_in_batch" }, { status: 400 }); } @@ -99,8 +110,17 @@ export async function PATCH( { status: 409 }, ); } - await pb.collection("dn_batch_notes").update(noteId, patch); - return NextResponse.json({ ok: true, updated: Object.keys(patch), rejected }); + const overridden = Object.keys(patch).filter((key) => OVERRIDABLE.has(key)); + const write = { ...patch }; + if (overridden.length > 0) { + const existing = note.manual_overrides && typeof note.manual_overrides === "object" && !Array.isArray(note.manual_overrides) + ? note.manual_overrides as Record + : {}; + write.manual_overrides = { ...existing, ...Object.fromEntries(overridden.map((key) => [key, patch[key]])) }; + } + + await pb.collection("dn_batch_notes").update(noteId, write); + return NextResponse.json({ ok: true, updated: Object.keys(patch), overridden, rejected }); } catch (err) { return NextResponse.json( { error: err instanceof Error ? err.message : "update_failed" }, diff --git a/src/lib/dn-pipeline/run-staged.test.ts b/src/lib/dn-pipeline/run-staged.test.ts index 31615ff..9e503b1 100644 --- a/src/lib/dn-pipeline/run-staged.test.ts +++ b/src/lib/dn-pipeline/run-staged.test.ts @@ -14,7 +14,7 @@ vi.mock("./extract", async (importOriginal) => { return { ...actual, extractPage: extractPageMock }; }); -import { aggregateGroupReads, candidateKey, pocketBaseFileUrl, runStagedBatch } from "./run-staged"; +import { aggregateGroupReads, applyManualOverrides, candidateKey, pocketBaseFileUrl, runStagedBatch } from "./run-staged"; import { evaluateRequiredChecks } from "./readiness"; const BATCH_ID = "abcdefghijklmno"; @@ -248,6 +248,43 @@ describe("runStagedBatch", () => { ]); }); + /* ---------------------------------------------------------------- */ + /* Manual corrections */ + /* ---------------------------------------------------------------- */ + + it("keeps a corrected order number and resolves the PO from it", async () => { + const fixture = stagedPb(); + normaliseSourceMock.mockResolvedValue([{ sourcePage: 1, png: Buffer.from("one"), mime: "image/png", width: 100, height: 200, rotation: 0 }]); + // The slip reads a letter O as a zero, so nothing matches on its own. + extractPageMock.mockResolvedValue(read(1, "DN-1", "PO-l")); + + await runStagedBatch(fixture.pb as never, BATCH_ID); + expect([...fixture.candidates.values()][0]).toEqual(expect.objectContaining({ purchase_order: "" })); + + // The operator picks the real order; the route records it as a decision. + const candidateId = [...fixture.candidates.keys()][0]; + fixture.candidates.get(candidateId)!.manual_overrides = { po_number_raw: "PO-1" }; + + await runStagedBatch(fixture.pb as never, BATCH_ID); + + const corrected = fixture.candidates.get(candidateId)!; + expect(corrected.po_number_raw).toBe("PO-1"); + expect(corrected.purchase_order).toBe("po-1"); + }); + + it("ignores an override that carries no value", () => { + // An override says a value IS something. Blank means the operator + // cleared the box, not that the document has no number. + const note = aggregateGroupReads({ key: "DN-1", pages: [1], reason: "single", confidence: 1 }, [read(1, "DN-1", "PO-1")]); + applyManualOverrides(note, { po_number_raw: " ", delivery_note_number: "" }); + expect(note.po_number_raw).toBe("PO-1"); + expect(note.delivery_note_number).toBe("DN-1"); + + applyManualOverrides(note, null); + applyManualOverrides(note, ["po_number_raw"]); + expect(note.po_number_raw).toBe("PO-1"); + }); + it("stops at Check page grouping only while a page is genuinely unplaced", async () => { const fixture = stagedPb(); normaliseSourceMock.mockResolvedValue([ diff --git a/src/lib/dn-pipeline/run-staged.ts b/src/lib/dn-pipeline/run-staged.ts index 219b724..5f5a780 100644 --- a/src/lib/dn-pipeline/run-staged.ts +++ b/src/lib/dn-pipeline/run-staged.ts @@ -41,6 +41,7 @@ type ExistingCandidate = { status?: string; stage?: string; posted_at?: string; + manual_overrides?: unknown; }; /** @@ -132,6 +133,17 @@ async function runClaimedStagedBatch(pb: PocketBase, batchId: string, ownership: const first = groupReads[0]; if (!first) continue; const note = aggregateGroupReads(group, groupReads); + // Match the stored candidate before resolving, not after: an operator + // who corrected a misread order number needs that correction to drive + // the resolution, or the run resolves the misread value again and + // hands back the same failure they just answered. + const pageIdentity = persistedPages.get(group.pages[0]); + if (!pageIdentity) continue; + const proposedKey = candidateKey(batchId, pageIdentity, note.delivery_note_number); + const matched = reconcileCandidate(existing, usedExisting, proposedKey, index, group, first); + if (matched) usedExisting.add(matched.id); + applyManualOverrides(note, matched?.manual_overrides); + const resolution = resolveEntities({ siteId: batch.site, poNumber: note.po_number_raw, @@ -166,11 +178,6 @@ async function runClaimedStagedBatch(pb: PocketBase, batchId: string, ownership: po.site === batch.site && poSupplierId && poSupplierId === resolution.supplierId && allLinesLinked && evaluateRequiredChecks(noteChecks).ready); if (isReady) ready++; else review++; - const pageIdentity = persistedPages.get(group.pages[0]); - if (!pageIdentity) continue; - const proposedKey = candidateKey(batchId, pageIdentity, note.delivery_note_number); - const matched = reconcileCandidate(existing, usedExisting, proposedKey, index, group, first); - if (matched) usedExisting.add(matched.id); // Once a candidate has an identity, it stays with that row even if a // recovered earlier page changes the group's first persisted page. const key = matched?.candidate_key || proposedKey; @@ -448,7 +455,9 @@ async function upsertCandidate( pages: group.pages, page_from: Math.min(...group.pages), page_to: Math.max(...group.pages), - delivery_note_number: group.key, + // The note's own number, not the group key, so a corrected document + // number reaches both fields rather than only doc_number below. + delivery_note_number: note.delivery_note_number || group.key, doc_type: note.doc_type, doc_number: note.delivery_note_number, doc_number_alts: note.doc_number_alts, @@ -469,10 +478,27 @@ async function upsertCandidate( else await ownership.create("dn_batch_notes", fields); } +/** + * Replace what the model read with what a person decided. + * + * Only the fields an operator can correct through the note route, and + * only when they hold a non-empty value: an override is a decision that + * something is so, never a decision that something is blank. + */ +export function applyManualOverrides(note: NoteSegment, overrides: unknown): NoteSegment { + if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) return note; + const values = overrides as Record; + const poNumber = typeof values.po_number_raw === "string" ? values.po_number_raw.trim() : ""; + const documentNumber = typeof values.delivery_note_number === "string" ? values.delivery_note_number.trim() : ""; + if (poNumber) note.po_number_raw = poNumber; + if (documentNumber) note.delivery_note_number = documentNumber; + return note; +} + async function loadCandidates(pb: PocketBase, batchId: string): Promise { const records = (await pb.collection("dn_batch_notes").getFullList({ filter: batchFilter(pb, batchId), - fields: "id,candidate_key,note_index,pages,delivery_note_number,doc_number,delivery_note,status,stage,posted_at", + fields: "id,candidate_key,note_index,pages,delivery_note_number,doc_number,delivery_note,status,stage,posted_at,manual_overrides", })) as unknown as ExistingCandidate[]; return records; }