fix(dn): keep an operator's correction through the next run
Picking the right order did nothing. The route stored the corrected number, the re-run rebuilt the candidate from its page reads, and po_number_raw went back to the original misread — so the operator answered the same question again, having been told it was saved. Nothing in dn_batch_notes distinguished a value a person chose from one the model read, so the runner could not tell a correction from stale OCR and overwrote both. A manual_overrides field now records the decision, and the runner applies it before resolving rather than after, so the corrected number is what the PO is matched on. Only fields a run rebuilds are recorded this way. Quantity, unit and the masses survive on their own; page ranges are the assembler's answer and are corrected through the assembly route, which keeps the whole batch consistent. An override must carry a value — clearing a box is not a decision that a document has no number. Verified end to end on a slip whose order number was read with a zero for the letter O: the correction survived the run, resolved the order, moved the match from fuzzy to exact, and left the document ready to post. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
7a02ab9bb7
commit
14be7e7e21
5 changed files with 134 additions and 12 deletions
|
|
@ -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` "
|
||||
|
|
|
|||
|
|
@ -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 }), {
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>
|
||||
: {};
|
||||
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" },
|
||||
|
|
|
|||
|
|
@ -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([
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>;
|
||||
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<ExistingCandidate[]> {
|
||||
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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue