fix(orders): update order_number collision-checking logic
Refine the handling of order_number values in the MCP API. Custom free-text order_numbers are now allowed to repeat across orders without triggering collision checks, while only values formatted as ORD-YYYY-MM-NNN will be subject to collision verification. Update tests to reflect this new behavior and ensure proper functionality.
This commit is contained in:
parent
2dbe96da23
commit
a73f3b3f97
5 changed files with 1164 additions and 19 deletions
35
check-progress.mjs
Normal file
35
check-progress.mjs
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
import { readFileSync } from "node:fs";
|
||||
import PocketBase from "pocketbase";
|
||||
const envLocal = readFileSync(".env.local", "utf8");
|
||||
for (const line of envLocal.split("\n")) {
|
||||
const m = line.match(/^([A-Z0-9_]+)\s*=\s*(.+?)\s*$/);
|
||||
if (m && !process.env[m[1]]) process.env[m[1]] = m[2].replace(/^["']|["']$/g, "");
|
||||
}
|
||||
const pb = new PocketBase(process.env.NEXT_PUBLIC_POCKETBASE_URL);
|
||||
await pb.admins.authWithPassword(process.env.PB_ADMIN_EMAIL, process.env.PB_ADMIN_PASSWORD);
|
||||
|
||||
let numChecked = 0, orderAttempted = 0, dnAttempted = 0, total = 0;
|
||||
for (let p = 1; p <= 30; p++) {
|
||||
const r = await pb.collection("orders").getList(p, 200, {
|
||||
fields: "id,order_number_duplicate_checked,duplicate_order_matching_attempted,delivery_note_matching_attempted",
|
||||
sort: "created",
|
||||
});
|
||||
for (const o of r.items) {
|
||||
total++;
|
||||
if (o.order_number_duplicate_checked === true) numChecked++;
|
||||
if (o.duplicate_order_matching_attempted === true) orderAttempted++;
|
||||
if (o.delivery_note_matching_attempted === true) dnAttempted++;
|
||||
}
|
||||
if (r.items.length < 200) break;
|
||||
}
|
||||
const hits1 = await pb.collection("orders").getList(1, 100, {
|
||||
filter: 'order_number_duplicate != "" && order_number_duplicate != null',
|
||||
});
|
||||
const hits2 = await pb.collection("orders").getList(1, 100, {
|
||||
filter: 'potential_duplicate_order != "" && potential_duplicate_order != null',
|
||||
});
|
||||
const hits3 = await pb.collection("orders").getList(1, 100, {
|
||||
filter: 'potential_delivery_note != "" && potential_delivery_note != null',
|
||||
});
|
||||
console.log(`total=${total} num=${numChecked} (${(numChecked/total*100).toFixed(1)}%) order=${orderAttempted} (${(orderAttempted/total*100).toFixed(1)}%) dn=${dnAttempted} (${(dnAttempted/total*100).toFixed(1)}%)`);
|
||||
console.log(`hits: order-num=${hits1.totalItems} item-dup=${hits2.totalItems} dn=${hits3.totalItems}`);
|
||||
1010
docs/superpowers/plans/2026-08-30-order-duplicate-detection.md
Normal file
1010
docs/superpowers/plans/2026-08-30-order-duplicate-detection.md
Normal file
File diff suppressed because it is too large
Load diff
76
run-backfill.mjs
Normal file
76
run-backfill.mjs
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
// Run the full backfill in 200-order chunks. Logs progress to stdout.
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
const envLocal = readFileSync(".env.local", "utf8");
|
||||
for (const line of envLocal.split("\n")) {
|
||||
const m = line.match(/^([A-Z0-9_]+)\s*=\s*(.+?)\s*$/);
|
||||
if (m && !process.env[m[1]]) process.env[m[1]] = m[2].replace(/^["']|["']$/g, "");
|
||||
}
|
||||
|
||||
const baseUrl = "http://localhost:4001";
|
||||
const agentKey = process.env.AGENT_API_KEY;
|
||||
if (!agentKey) {
|
||||
console.error("Missing AGENT_API_KEY in .env.local");
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const LIMIT = 200;
|
||||
let skip = 0;
|
||||
let totalProcessed = 0;
|
||||
let totalFailures = 0;
|
||||
const start = Date.now();
|
||||
let chunkNum = 0;
|
||||
|
||||
while (true) {
|
||||
chunkNum++;
|
||||
const url = `${baseUrl}/api/v1/orders/backfill-duplicate-checks?limit=${LIMIT}&skip=${skip}`;
|
||||
let body;
|
||||
try {
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), 600_000); // 10 min per chunk
|
||||
const res = await fetch(url, {
|
||||
method: "POST",
|
||||
headers: { authorization: `Bearer ${agentKey}` },
|
||||
signal: controller.signal,
|
||||
});
|
||||
clearTimeout(timeoutId);
|
||||
if (!res.ok) {
|
||||
console.error(`[fail] HTTP ${res.status} at skip=${skip}`);
|
||||
const text = await res.text();
|
||||
console.error(text.substring(0, 500));
|
||||
process.exit(2);
|
||||
}
|
||||
body = await res.json();
|
||||
} catch (err) {
|
||||
console.error(`[err] fetch failed at skip=${skip}:`, err.message);
|
||||
if (err.cause) console.error("cause:", err.cause);
|
||||
process.exit(2);
|
||||
}
|
||||
|
||||
totalProcessed += body.ordersProcessed || 0;
|
||||
totalFailures += body.failures?.length || 0;
|
||||
const elapsed = Math.round((Date.now() - start) / 1000);
|
||||
const rate = totalProcessed / (elapsed / 60);
|
||||
console.log(
|
||||
`[chunk ${chunkNum} skip=${skip} processed=${body.ordersProcessed} ` +
|
||||
`failures=${body.failures?.length || 0} total=${totalProcessed} ` +
|
||||
`nextSkip=${body.nextSkip} done=${body.done} elapsed=${elapsed}s rate=${rate.toFixed(1)}/min]`,
|
||||
);
|
||||
|
||||
if (body.failures?.length > 0) {
|
||||
for (const f of body.failures.slice(0, 3)) {
|
||||
console.log(` failure: ${f.orderId} -> ${f.error}`);
|
||||
}
|
||||
if (body.failures.length > 3) {
|
||||
console.log(` ... and ${body.failures.length - 3} more`);
|
||||
}
|
||||
}
|
||||
|
||||
if (body.done) {
|
||||
console.log(
|
||||
`\n[done] total processed: ${totalProcessed}, total failures: ${totalFailures}, elapsed: ${elapsed}s (${(elapsed / 60).toFixed(1)} min)`,
|
||||
);
|
||||
process.exit(0);
|
||||
}
|
||||
skip = body.nextSkip;
|
||||
}
|
||||
|
|
@ -563,9 +563,12 @@ const tools = [
|
|||
"whole order.\n\n" +
|
||||
"Optional `order_number`: pass a customer-supplied identifier (e.g. the PO/reference printed " +
|
||||
"on the inbound PO PDF) to use it verbatim as `orders.order_number`. Empty / whitespace " +
|
||||
"values fall back to auto-generated `ORD-YYYY-MM-NNN` — same as omitting the field. On " +
|
||||
"collision the call surfaces an MCP error envelope with code/message `order_number_collision` " +
|
||||
"and a `data` block carrying the colliding order id and attempted number.\n\n" +
|
||||
"values fall back to auto-generated `ORD-YYYY-MM-NNN` — same as omitting the field. Custom " +
|
||||
"values are allowed to repeat across orders (e.g. one blanket PO split into several partial-" +
|
||||
"delivery orders, or a shared \"no reference given\" placeholder) — collision-checking only " +
|
||||
"applies when the value impersonates our own `ORD-YYYY-MM-NNN` format, in which case the call " +
|
||||
"surfaces an MCP error envelope with code/message `order_number_collision` and a `data` block " +
|
||||
"carrying the colliding order id and attempted number.\n\n" +
|
||||
"Optional `email_sender` / `email_subject` / `email_body`: stamp the inbound email metadata " +
|
||||
"on the order in the same call so the operator can trace it back to its source email without " +
|
||||
"a follow-up `orders.update` patch. The n8n intake pipeline fills these in from the parsed " +
|
||||
|
|
|
|||
|
|
@ -269,15 +269,39 @@ describe("updateOrderReviewFields", () => {
|
|||
expect(result.customer).toBe("cust-1");
|
||||
});
|
||||
|
||||
it("escapes embedded quotes in order_number before building the collision filter", async () => {
|
||||
// An order_number containing `"` would break the filter
|
||||
// expression unless we escape embedded quotes before
|
||||
// interpolating. Spy on getList to capture the filter
|
||||
// the service actually sends.
|
||||
const order = await pbMock.collection("orders").create({
|
||||
order_number: "ORD-2026-07-001",
|
||||
it("does not collision-check a free-text order_number, even one that duplicates another order's", async () => {
|
||||
// Customer PO references (and placeholders like "NO-REF") are
|
||||
// expected to repeat across several orders — e.g. one blanket PO
|
||||
// split into multiple partial-delivery orders. Only values shaped
|
||||
// like our own ORD-YYYY-MM-NNN sequence are collision-checked.
|
||||
await pbMock.collection("orders").create({
|
||||
order_number: "PO-12345",
|
||||
customer: "cust-1",
|
||||
});
|
||||
const order = await pbMock.collection("orders").create({
|
||||
order_number: "ORD-2026-07-002",
|
||||
customer: "cust-2",
|
||||
});
|
||||
|
||||
const result = await updateOrderReviewFields(order.id, {
|
||||
order_number: "PO-12345",
|
||||
});
|
||||
|
||||
expect(result.order_number).toBe("PO-12345");
|
||||
});
|
||||
|
||||
it("collision-checks an ORD-shaped order_number against the exact filter", async () => {
|
||||
// Only ORD-YYYY-MM-NNN-shaped values are collision-checked (see
|
||||
// the test above for free-text values). Spy on getList to confirm
|
||||
// the check actually runs and filters on the exact number.
|
||||
const existing = await pbMock.collection("orders").create({
|
||||
order_number: 'ORD-2026-07-001',
|
||||
customer: "cust-1",
|
||||
});
|
||||
const order = await pbMock.collection("orders").create({
|
||||
order_number: "ORD-2026-07-002",
|
||||
customer: "cust-2",
|
||||
});
|
||||
|
||||
const originalCollection = pbMock.collection;
|
||||
const captured: string[] = [];
|
||||
|
|
@ -295,18 +319,15 @@ describe("updateOrderReviewFields", () => {
|
|||
};
|
||||
|
||||
try {
|
||||
// Pick an order_number that contains a literal quote. The
|
||||
// unescaped filter would be `order_number = "ORD "foo" && id != "..."`,
|
||||
// which would either fail to parse or match the wrong rows.
|
||||
await updateOrderReviewFields(order.id, {
|
||||
order_number: 'ORD "weird"',
|
||||
});
|
||||
await expect(
|
||||
updateOrderReviewFields(order.id, {
|
||||
order_number: existing.order_number,
|
||||
}),
|
||||
).rejects.toBeInstanceOf(OrderNumberCollisionError);
|
||||
|
||||
expect(captured.length).toBeGreaterThan(0);
|
||||
// The filter MUST escape the `"` inside the literal — look for
|
||||
// the backslash-escaped quote in the captured string.
|
||||
const filter = captured[captured.length - 1];
|
||||
expect(filter).toContain('ORD \\"weird\\"');
|
||||
expect(filter).toContain(`order_number = "${existing.order_number}"`);
|
||||
} finally {
|
||||
pbMock.collection = originalCollection;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue