fix(dn): settle one product line at a time, and keep the link

Answering the second product question returned batch_locked. Resolving a
line fired a full re-run, which holds the batch lease for some twenty
seconds, so the next answer arrived while the lease was still taken. The
re-run achieved nothing either: resolve-item already links the line,
re-evaluates the document and writes its stage.

It was also quietly destructive. A run rebuilds `items` from the page
reads, and only a choice saved as an alias can be re-derived from the
descriptions — so a link made without ticking "remember this alias" was
discarded by the next run, including the tick a minute later. Manual
links now persist in manual_overrides alongside the order number, and are
honoured only while the linked item still belongs to the resolved order,
so correcting the order cannot leave lines pointing at the previous one.

Each line now has its own Save rather than committing the moment the
dropdown changes, and an answered line leaves the list instead of
reloading onto the same screen with the same questions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
DEls 2026-09-21 00:15:13 +02:00
parent 14be7e7e21
commit b8dcf20146
4 changed files with 152 additions and 31 deletions

View file

@ -26,7 +26,7 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
const leased = await withBatchLease(pb, id, "resolve-item", async (lease) => {
const [batch, note] = await Promise.all([
pb.collection("dn_batches").getOne(id, { fields: "id,site,page_count,assembly_revision,revision" }).catch(() => null),
pb.collection("dn_batch_notes").getOne(noteId, { fields: "id,batch,status,stage,posted_at,delivery_note,purchase_order,supplier,items,checks,pages,doc_number,delivery_note_number,assembly_revision,candidate_revision" }).catch(() => null),
pb.collection("dn_batch_notes").getOne(noteId, { fields: "id,batch,status,stage,posted_at,delivery_note,purchase_order,supplier,items,checks,pages,doc_number,delivery_note_number,assembly_revision,candidate_revision,manual_overrides" }).catch(() => null),
]);
if (!batch) return NextResponse.json({ error: "batch_not_found" }, { status: 404 });
if (!note) return NextResponse.json({ error: "note_not_found" }, { status: 404 });
@ -59,8 +59,19 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
const ready = readiness.ready;
const after = { lineIndex, purchase_order_item: String(purchaseOrderItemId), stage: ready ? "ready_to_post" : "needs_review", status: ready ? "candidate" : "blocked", reasons: readiness.reasons };
const reason = String(body.reason).trim();
// Record the link as a decision, not just a value. Every run rebuilds
// `items` from the page reads, and only an aliased choice can be
// re-derived from the descriptions — so without this, a link made
// without ticking "remember this alias" is gone on the next run.
const priorOverrides = object(note.manual_overrides) ? note.manual_overrides as Record<string, unknown> : {};
const priorItems = object(priorOverrides.items) ? priorOverrides.items as Record<string, unknown> : {};
const manual_overrides = {
...priorOverrides,
items: { ...priorItems, [String(lineIndex)]: String(purchaseOrderItemId) },
};
const mutations: BatchMutation[] = [
{ collection: "dn_batch_notes", method: "update", id: noteId, data: { items, stage: after.stage, status: after.status } },
{ collection: "dn_batch_notes", method: "update", id: noteId, data: { items, stage: after.stage, status: after.status, manual_overrides } },
{ collection: "dn_review_events", method: "create", data: { batch: id, note: noteId, action: "resolve_item", before, after, reason, actor: "web" } },
];
if (body.rememberAlias) {

View file

@ -302,13 +302,26 @@ function ProductDecisions({ note }: { note: DocumentNoteView }) {
const [saving, setSaving] = React.useState("");
const [error, setError] = React.useState("");
const [applyAll, setApplyAll] = React.useState(false);
const [resolved, setResolved] = React.useState<Record<number, string>>({});
const [picked, setPicked] = React.useState<Record<number, string>>({});
const check = note.checks.find((c) => c.id === "product_matched");
const decisions = (Array.isArray(check?.detail) ? check!.detail : []) as ItemDecision[];
const outstanding = decisions.filter((decision) => !(decision.lineIndex in resolved));
if (decisions.length === 0) return null;
async function choose(lineIndex: number, poItemId: string) {
if (!poItemId) return;
/**
* One line at a time, and no re-run.
*
* resolve-item already links the line, re-evaluates the document and
* writes its stage, so re-running afterwards achieved nothing except to
* hold the batch lease for twenty seconds — long enough that answering
* the second question returned batch_locked. It was also destructive:
* a re-run rebuilds `items` from the page reads, discarding any link
* that no saved alias could reproduce.
*/
async function choose(lineIndex: number, poItemId: string, label: string) {
if (!poItemId || saving) return;
setError("");
setSaving(String(lineIndex));
try {
@ -319,11 +332,16 @@ function ProductDecisions({ note }: { note: DocumentNoteView }) {
});
if (!res.ok) {
const body = await res.json().catch(() => ({}));
setError(body.detail || body.error || `Failed (${res.status})`);
setError(body.error === "batch_locked"
? "Another change to this batch is still running. Wait a moment and try again."
: body.detail || body.error || `Failed (${res.status})`);
return;
}
await fetch(`/api/dn-batches/${note.batchId}/rerun`, { method: "POST" });
window.location.reload();
// Answered lines leave the list so the remaining questions are the
// ones still to answer, rather than reloading onto the same screen.
setResolved((current) => ({ ...current, [lineIndex]: label }));
const body = await res.json().catch(() => ({}));
if (body.readyToPost) window.location.reload();
} finally {
setSaving("");
}
@ -332,35 +350,67 @@ function ProductDecisions({ note }: { note: DocumentNoteView }) {
return (
<div className="space-y-2 rounded-md border p-3">
<p className="text-xs text-muted-foreground">
These lines need a product decision. Your choice is remembered for next time.
{outstanding.length === 0
? "Every line is linked."
: `${outstanding.length} line${outstanding.length === 1 ? "" : "s"} need a product decision. Your choice is remembered for next time.`}
</p>
{decisions.map((decision, index) => (
<div key={`${index}-${decision.lineIndex}-${decision.description}`} className="space-y-1">
<p className="text-sm">{decision.description}</p>
<select
className="h-8 w-full rounded-md border bg-background px-2 text-sm"
defaultValue=""
disabled={saving === String(decision.lineIndex)}
onChange={(event) => choose(decision.lineIndex, event.target.value)}
>
<option value="">
{decision.kind === "ambiguous"
? `Pick one of ${decision.candidates.length} likely lines…`
: `No match found — pick from ${decision.candidates.length} PO lines…`}
</option>
{decision.candidates.map((candidate) => (
<option key={candidate.id} value={candidate.id}>
{candidate.description}{candidate.material_code ? ` (${candidate.material_code})` : ""}
</option>
))}
</select>
</div>
{Object.entries(resolved).map(([lineIndex, label]) => (
<p key={`done-${lineIndex}`} className="text-xs text-muted-foreground">✓ linked to {label}</p>
))}
{outstanding.map((decision) => {
const choice = picked[decision.lineIndex] ?? "";
const busy = saving === String(decision.lineIndex);
return (
<div key={`${decision.lineIndex}-${decision.description}`} className="space-y-1">
<p className="text-sm">{decision.description}</p>
<div className="flex flex-wrap items-center gap-2">
<select
className="h-8 min-w-0 flex-1 rounded-md border bg-background px-2 text-sm"
value={choice}
disabled={saving !== ""}
onChange={(event) => {
setPicked((current) => ({ ...current, [decision.lineIndex]: event.target.value }));
setError("");
}}
>
<option value="">
{decision.kind === "ambiguous"
? `Pick one of ${decision.candidates.length} likely lines…`
: `No match found — pick from ${decision.candidates.length} PO lines…`}
</option>
{decision.candidates.map((candidate) => (
<option key={candidate.id} value={candidate.id}>
{candidate.description}{candidate.material_code ? ` (${candidate.material_code})` : ""}
</option>
))}
</select>
{/* Saved per line, so answering one question settles that
one rather than committing every pick at once. */}
<Button
size="sm"
disabled={!choice || saving !== ""}
onClick={() => {
const option = decision.candidates.find((candidate) => candidate.id === choice);
if (option) choose(decision.lineIndex, option.id, option.description);
}}
>
{busy ? "Saving…" : "Save"}
</Button>
</div>
</div>
);
})}
<label className="flex items-center gap-2 text-xs text-muted-foreground">
<input type="checkbox" checked={applyAll} onChange={(event) => setApplyAll(event.target.checked)} />
Remember this supplier-specific alias
</label>
{error && <p className="text-xs text-destructive">{error}</p>}
{outstanding.length === 0 && Object.keys(resolved).length > 0 && (
<Button size="sm" onClick={() => window.location.reload()}>Refresh this document</Button>
)}
</div>
);
}

View file

@ -14,7 +14,7 @@ vi.mock("./extract", async (importOriginal) => {
return { ...actual, extractPage: extractPageMock };
});
import { aggregateGroupReads, applyManualOverrides, candidateKey, pocketBaseFileUrl, runStagedBatch } from "./run-staged";
import { aggregateGroupReads, applyManualOverrides, candidateKey, manualItemOverrides, pocketBaseFileUrl, runStagedBatch } from "./run-staged";
import { evaluateRequiredChecks } from "./readiness";
const BATCH_ID = "abcdefghijklmno";
@ -272,6 +272,35 @@ describe("runStagedBatch", () => {
expect(corrected.purchase_order).toBe("po-1");
});
it("keeps a line an operator linked by hand", async () => {
const fixture = stagedPb();
normaliseSourceMock.mockResolvedValue([{ sourcePage: 1, png: Buffer.from("one"), mime: "image/png", width: 100, height: 200, rotation: 0 }]);
// A description no rule matches, so nothing re-derives the link.
extractPageMock.mockResolvedValue({
...read(1, "DN-1", "PO-1"),
items: [{ description: "Unrecognisable scrawl", qty: 1, unit: "ton", unit_price: null, line_amount: null, product_code: null }],
});
await runStagedBatch(fixture.pb as never, BATCH_ID);
const candidateId = [...fixture.candidates.keys()][0];
expect((fixture.candidates.get(candidateId)!.items as Array<{ purchase_order_item: string }>)[0].purchase_order_item).toBe("");
fixture.candidates.get(candidateId)!.manual_overrides = { items: { "0": "poi-1" } };
await runStagedBatch(fixture.pb as never, BATCH_ID);
expect((fixture.candidates.get(candidateId)!.items as Array<{ purchase_order_item: string }>)[0].purchase_order_item).toBe("poi-1");
});
it("drops a hand-linked line that no longer belongs to the resolved order", () => {
// Correcting the order must not leave lines pointing at items on the
// order it replaced.
expect(manualItemOverrides({ items: { "0": "poi-1" } }, [{ id: "poi-1" }]).get(0)).toBe("poi-1");
expect(manualItemOverrides({ items: { "0": "poi-old" } }, [{ id: "poi-1" }]).size).toBe(0);
expect(manualItemOverrides({ items: { "-1": "poi-1" } }, [{ id: "poi-1" }]).size).toBe(0);
expect(manualItemOverrides(null, [{ id: "poi-1" }]).size).toBe(0);
});
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.

View file

@ -170,7 +170,13 @@ async function runClaimedStagedBatch(pb: PocketBase, batchId: string, ownership:
const noteChecks = runNoteChecks(note, checkContext);
if (deliveredError) noteChecks.push({ id: "delivered_totals_available", passed: false, reason: deliveredError });
const lineMatches = resolveNoteLines(note, checkContext);
const linkedIds = lineMatches.map((entry) => entry.match.kind === "matched" ? entry.match.poItem.id : "");
// A line an operator linked by hand keeps that link. Only aliased
// choices can be re-derived from the descriptions, so without this a
// link made without ticking "remember this alias" is discarded by the
// next run — including the tick a minute later.
const manualItemLinks = manualItemOverrides(matched?.manual_overrides, poItems);
const linkedIds = lineMatches.map((entry, itemIndex) =>
manualItemLinks.get(itemIndex) ?? (entry.match.kind === "matched" ? entry.match.poItem.id : ""));
const noteItems = note.items.map((item, itemIndex) => ({ ...item, purchase_order_item: linkedIds[itemIndex] || "" }));
const allLinesLinked = note.items.length > 0 && linkedIds.every(Boolean) && new Set(linkedIds).size === linkedIds.length;
const poSupplierId = relationId(po?.supplier_ref) || relationId(po?.supplier_id);
@ -495,6 +501,31 @@ export function applyManualOverrides(note: NoteSegment, overrides: unknown): Not
return note;
}
/**
* Line index to purchase-order item id, for lines linked by hand.
*
* A link is only honoured when the id still belongs to the order now
* resolved for this document: correcting the order must not leave lines
* pointing at items on the previous one.
*/
export function manualItemOverrides(
overrides: unknown,
poItems: readonly { id?: unknown }[],
): Map<number, string> {
const links = new Map<number, string>();
if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) return links;
const items = (overrides as Record<string, unknown>).items;
if (!items || typeof items !== "object" || Array.isArray(items)) return links;
const available = new Set(poItems.map((item) => String(item.id ?? "")).filter(Boolean));
for (const [key, value] of Object.entries(items as Record<string, unknown>)) {
const index = Number(key);
if (!Number.isInteger(index) || index < 0) continue;
if (typeof value !== "string" || !available.has(value)) continue;
links.set(index, value);
}
return links;
}
async function loadCandidates(pb: PocketBase, batchId: string): Promise<ExistingCandidate[]> {
const records = (await pb.collection("dn_batch_notes").getFullList({
filter: batchFilter(pb, batchId),