fix(dn): over-PO quantities need an explicit accept, never from the assistant

Co-Authored-By: MiniMax (Pinchy)
This commit is contained in:
DEls 2026-10-26 19:27:29 +02:00
parent e7e8a2ff3d
commit bc138bcae7
6 changed files with 137 additions and 18 deletions

View file

@ -62,6 +62,18 @@ it("writes nothing when one edit in the batch fails", async () => {
expect(m.batch).not.toHaveBeenCalled();
});
it("books a quantity over the order only when the office ticks it", async () => {
// The order has 70 m of pipe left; 11 × 10 m is 110 m.
const over = { lines: [0], poItemId: "itemabcdefghijk", slipQty: 11, slipUnit: "ea", factor: 10, price: null, usePoRate: true, reason: "The site ordered extra pipe." };
const refused = await POST(req({ edits: [over], snapshotHash: p.snapshotHash, decisionHash: decisionHash(p) }), params);
expect(refused.status).toBe(409);
expect((await refused.json()).detail).toContain('Line 1: 40 m more than the PO has left. Tick "accept over PO" or change the quantity.');
expect(m.batch).not.toHaveBeenCalled();
const accepted = await POST(req({ edits: [{ ...over, acceptExcess: true }], snapshotHash: p.snapshotHash, decisionHash: decisionHash(p) }), params);
expect(accepted.status).toBe(200);
});
it("never reaches for the posting function", async () => {
const source = await import("node:fs/promises").then((fs) => fs.readFile(new URL("./route.ts", import.meta.url), "utf8"));
expect(source).not.toContain("postLineWithinLease");

View file

@ -119,7 +119,7 @@ describe("runLineAssistant", () => {
expect(saveLineEdits).toHaveBeenCalledWith(
expect.anything(),
expect.anything(),
[expect.objectContaining({ lines: [2, 3], poItemId: KERB_ITEM, slipQty: 4, slipUnit: "roll", factor: 15, usePoRate: true })],
[expect.objectContaining({ lines: [2, 3], poItemId: KERB_ITEM, slipQty: 4, slipUnit: "roll", factor: 15, usePoRate: true, acceptExcess: false })],
"operator",
);
// The rows the office sees now carry the combined line.
@ -164,6 +164,18 @@ describe("runLineAssistant", () => {
expect(rows[0].numbers).toEqual([1]);
});
it("never lets the model push a quantity over the purchase order", async () => {
const model = vi.fn(async () => ({ proposal: proposal([
{ lines: [3], poItemId: KERB_ITEM, slipQty: 4, slipUnit: "roll", factor: 30, price: null, reason: "a guess" },
]), model: "test-model" }));
const result = await runLineAssistant(pb as never, BATCH, NOTE, "operator", "book 120 m of kerb", { model, images: async () => [{ mediaType: "image/jpeg", data: "" }] });
// The office has to say yes on the line; the assistant cannot.
expect(result.reply).toMatch(/accept over PO/i);
expect(state.mutations.filter((mutation) => mutation.collection === "dn_batch_notes")).toHaveLength(0);
});
it("never reaches for the posting function", async () => {
const source = await readFile(new URL("./assistant.ts", import.meta.url), "utf8");
expect(source).not.toContain("postLineWithinLease");
@ -201,4 +213,10 @@ describe("LineAssistantProposalSchema", () => {
it("keeps a proposal with no edits at all", () => {
expect(LineAssistantProposalSchema.safeParse({ reply: "ok", question: null }).success).toBe(true);
});
it("refuses a proposal that tries to accept an excess itself", () => {
expect(LineAssistantProposalSchema.safeParse({ reply: "ok", question: null, edits: [
{ lines: [1], poItemId: null, slipQty: 1, slipUnit: "ea", factor: 1, price: null, reason: "because", acceptExcess: true },
] }).success).toBe(false);
});
});

View file

@ -134,6 +134,9 @@ export async function assistantHistory(pb: PocketBase, batchId: string, noteId:
function plainReason(error: unknown): string {
const detail = error instanceof Error ? error.message : "The lines could not be changed.";
if (/does not belong to this purchase order/i.test(detail)) return "That order line is not on this purchase order.";
// The office cannot tick a box on a line it cannot see: say where the
// decision has to be made.
if (/more than the PO has left/i.test(detail)) return detail.replace('Tick "accept over PO"', 'Tick "accept over PO" on that line');
return detail;
}
@ -222,6 +225,9 @@ export async function runLineAssistant(
price: edit.price,
// No edited price means the order rate is used.
usePoRate: edit.price === null,
// The assistant never accepts a quantity over the order. That is the
// office's decision, on the line, where the number is visible.
acceptExcess: false,
reason: edit.reason,
}));

View file

@ -26,6 +26,8 @@ export interface LineEdit {
/** The edited unit price, or null to keep the order rate. */
price: number | null;
usePoRate: boolean;
/** A quantity over what the order has left. Only a person may accept it. */
acceptExcess?: boolean;
reason: string;
}
@ -34,7 +36,7 @@ export interface LineRow {
numbers: number[];
allocationIndex: number;
slipText: string; slipQty: number | null; slipUnit: string | null; slipPrice: number | null; slipAmount: number | null;
poItemId: string | null; poUnit: string | null; poRate: number | null; ordered: number | null; priorReceived: number;
poItemId: string | null; poUnit: string | null; poRate: number | null; ordered: number | null; priorReceived: number; booked: number;
qty: number | null; unit: string | null; factor: number | null; poQty: number | null;
price: number | null; value: number | null; leftOnPo: number | null;
flags: Array<"unit_differs" | "factor_missing" | "price_differs" | "over_po" | "guard_conflict" | "not_on_po">;
@ -61,6 +63,36 @@ function priorReceivedForItem(context: PreviewContext, poItemId: string | null):
return context.priorItems.filter((row) => row.purchase_order_item === poItemId).reduce((sum, row) => sum + (finiteNumber(row.qty_received) ?? 0), 0);
}
/**
* The order line's own numbers.
*
* An edit may put a slip line on another order line, and the numbers that
* decide whether a quantity is over the order belong to the line it is
* going on — not to the one it came from.
*/
export function poLineNumbers(context: PreviewContext, poItemId: string | null): Pick<LineRow, "poItemId" | "poUnit" | "poRate" | "ordered" | "priorReceived" | "booked"> {
const item = context.poItems.find((candidate) => candidate.id === poItemId);
return {
poItemId: poItemId ?? null,
poUnit: item && typeof item.unit === "string" ? item.unit : null,
poRate: item ? (finiteNumber(item.rate) ?? null) : null,
ordered: item ? (finiteNumber(item.qty) ?? null) : null,
priorReceived: priorReceivedForItem(context, poItemId ?? null),
booked: bookedForItem(context, poItemId ?? null),
};
}
/**
* How much more than the order has left this quantity would book.
*
* The order line, what other notes have already delivered and what this note
* has already booked — the same three numbers the receipt itself checks.
*/
export function overPo(row: LineRow, qty: number | null): number {
if (qty === null || row.ordered === null) return 0;
return Math.max(0, round2(row.priorReceived + row.booked + qty - row.ordered));
}
function buildRow(
context: PreviewContext,
preview: ProcessingPreview,
@ -72,12 +104,8 @@ function buildRow(
.map((id) => context.lines.find((line) => line.sourceLineId === id))
.filter((line): line is PreviewLine => Boolean(line));
const first = source[0];
const item = context.poItems.find((candidate) => candidate.id === allocation.purchaseOrderItemId);
const poItemId = allocation.purchaseOrderItemId ?? null;
const poUnit = item && typeof item.unit === "string" ? item.unit : null;
const poRate = item ? (finiteNumber(item.rate) ?? null) : null;
const ordered = item ? (finiteNumber(item.qty) ?? null) : null;
const priorReceived = priorReceivedForItem(context, poItemId);
const { poUnit, poRate, ordered, priorReceived, booked } = poLineNumbers(context, poItemId);
const slipQty = finiteNumber(first?.qty) ?? null;
const slipUnit = first?.unit ?? null;
@ -130,7 +158,7 @@ function buildRow(
allocationIndex,
slipText: source.map((line) => line.description ?? "Unreadable line").join(" + "),
slipQty, slipUnit, slipPrice, slipAmount,
poItemId, poUnit, poRate, ordered, priorReceived,
poItemId, poUnit, poRate, ordered, priorReceived, booked,
qty, unit, factor, poQty,
price: slipPrice, value, leftOnPo,
flags,
@ -151,9 +179,13 @@ export function buildLineRows(context: PreviewContext, preview: ProcessingPrevie
*
* The quantity booked is the slip's quantity times the factor, in the
* order's unit. A factor of one with a different unit means the order's
* unit label is wrong, and the slip's own unit is booked instead. The
* reason is never empty: the arithmetic is written down so the next person
* can see how the number was reached.
* unit label is wrong, and the slip's own unit is booked instead.
*
* A quantity over what the order has left is only booked when the edit says
* so, and then the reason has to show that it was accepted — the office
* cannot see an excess nobody agreed to. The reason is never empty: the
* arithmetic is written down so the next person can see how the number was
* reached.
*/
export function editToConfirmation(row: LineRow, edit: LineEdit, poUnit: string | null): LineConfirmation {
const targetUnit = poUnit ?? row.poUnit ?? null;
@ -169,11 +201,18 @@ export function editToConfirmation(row: LineRow, edit: LineEdit, poUnit: string
const unitOverride = differs && factor === 1;
const unit = differs && !unitOverride ? targetUnit! : slipUnit;
const priceEdited = edit.price !== null && edit.price !== undefined && edit.usePoRate !== true;
const acceptExcess = edit.acceptExcess === true;
const over = overPo(row, poQty);
const reason = edit.reason?.trim() || [
const written = edit.reason?.trim() || [
`${edit.slipQty} ${slipUnit} × ${factor} = ${poQty} ${unit}`,
...(priceEdited ? [`Price R${edit.price} instead of PO R${row.poRate ?? "?"}`] : []),
].join("; ");
// An accepted excess is written down, so nobody reads the line later
// without knowing that it went over the order.
const reason = acceptExcess && over > 0 && !/over po/i.test(written)
? `${written}; Over PO by ${over} ${targetUnit ?? "unit"}, accepted`
: written;
return {
confirmed: true,
@ -182,6 +221,7 @@ export function editToConfirmation(row: LineRow, edit: LineEdit, poUnit: string
unit,
...(unitOverride ? { unitOverride: true } : {}),
...(priceEdited ? { netAmount: round2(poQty * edit.price!) } : { usePoRate: true }),
...(acceptExcess ? { acceptExcess: true } : {}),
reason,
};
}

View file

@ -16,7 +16,7 @@ const context = (): PreviewContext => ({
supplier: { id: "supplierabcdefg", name: "Vula" },
po: { id: "poabcdefghijklm", purchase_order_number: "MEGCPO9448", total_value: 11748.7 },
poItems: [
{ id: MESH_ITEM, description: "MESH DIAMOND 600 x 2.5 ROLL", unit: "m", qty: 2, rate: 1124.35, disc_percent: 0, vat_percent: 15 },
{ id: MESH_ITEM, description: "MESH DIAMOND 600 x 2.5 ROLL", unit: "m", qty: 100, rate: 1124.35, disc_percent: 0, vat_percent: 15 },
{ id: KERB_ITEM, description: "KERB 150", unit: "m", qty: 100, rate: 150, disc_percent: 0, vat_percent: 15 },
{ id: BAILING_ITEM, description: "1.6mm Bailing Wire", unit: "ea", qty: 100, rate: 95, disc_percent: 0, vat_percent: 15 },
],
@ -106,6 +106,37 @@ describe("saveLineEdits", () => {
allocation.purchaseOrderItemId === KERB_ITEM && allocation.sourceLineIds.length === 2)).toBe(true);
});
it("refuses a quantity over what the order has left unless the office accepts it", () => {
const c = context();
const before = preview(c);
const untouched = structuredClone(before);
// KERB has 100 m left on the order; 4 × 30 m is 120 m.
expect(() => saveLineEdits(c, before, [edit({
lines: [1],
poItemId: KERB_ITEM,
slipQty: 4,
slipUnit: "ea",
factor: 30,
reason: "One kerb is 30 m.",
})], "operator")).toThrow('Line 2: 20 m more than the PO has left. Tick "accept over PO" or change the quantity.');
expect(before).toEqual(untouched);
});
it("books a quantity over the order when the office accepts it, and says so", () => {
const c = context();
const result = saveLineEdits(c, preview(c), [edit({
lines: [1],
poItemId: KERB_ITEM,
slipQty: 4,
slipUnit: "ea",
factor: 30,
acceptExcess: true,
reason: "The site ordered extra kerb.",
})], "operator");
const allocation = result.preview.allocations.find((candidate) => candidate.purchaseOrderItemId === KERB_ITEM);
expect(allocation?.operatorConfirmation?.reason).toContain("Over PO");
});
it("saves nothing when one edit in the batch fails", () => { const c = context();
const before = preview(c);
const untouched = structuredClone(before);

View file

@ -14,11 +14,18 @@ import { resolveMatch } from "@/lib/dn-pipeline/matching";
import { rememberConnection } from "@/lib/dn-pipeline/remembered-match";
import { stableSourceKey } from "@/lib/dn-pipeline/line-booking";
import { readyIndexes } from "@/lib/inbox-three/readiness";
import { buildLineRows, editToConfirmation, type LineEdit } from "./line-rows";
import { buildLineRows, editToConfirmation, overPo, poLineNumbers, type LineEdit, type LineRow } from "./line-rows";
const sameLines = (a: readonly string[], b: readonly string[]) =>
a.length === b.length && [...a].sort().join("|") === [...b].sort().join("|");
/** What the office has to decide: this line is over what the order has left. */
function excessMessage(lineIndex: number, over: number, poUnit: string | null): string {
const line = lineIndex + 1;
if (over > 0) return `Line ${line}: ${over} ${poUnit ?? "unit"} more than the PO has left. Tick "accept over PO" or change the quantity.`;
return `Line ${line}: this line is worth more than the purchase order has left. Tick "accept over PO" or change the quantity.`;
}
/** Has this slip line already been booked? */
function isPosted(context: PreviewContext, preview: ProcessingPreview, sourceLineId: string): boolean {
if (preview.allocations.some((allocation) => allocation.posted && allocation.sourceLineIds.includes(sourceLineId))) return true;
@ -84,15 +91,20 @@ export function saveLineEdits(
}
const row = buildLineRows(working, next)[index];
const confirmation = editToConfirmation(row, edit, edit.poItemId ? row.poUnit : null);
// An edit may put the line on another order line. The numbers that
// decide whether a quantity is over the order belong to the line it is
// going on, not to the one it came from.
const target = poLineNumbers(working, edit.poItemId ?? row.poItemId);
const effectiveRow: LineRow = { ...row, ...target };
const confirmation = editToConfirmation(effectiveRow, edit, target.poUnit);
try {
current = resolveMatch(working, next, { index, sourceLineIds, snapshotHash: next.snapshotHash, confirmation });
} catch (error) {
// The clerk confirmed a quantity that is over what the order has left.
// Their reason is the acknowledgement; without it the line stays open.
// More than the order has left. The system does not decide this and
// neither does the assistant: a person has to accept it on the line.
if (!(error instanceof Error) || !/Acknowledge the cumulative excess/.test(error.message)) throw error;
current = resolveMatch(working, next, { index, sourceLineIds, snapshotHash: next.snapshotHash, confirmation: { ...confirmation, acceptExcess: true } });
throw Error(excessMessage(indexes[0], overPo(effectiveRow, confirmation.quantity ?? null), target.poUnit));
}
// A remembered receiving connection is the supplier's rule, not the