perf(orders): bulk-fetch candidate items in suggestOrderDuplicate (kill N+1)

src/lib/server/order-order-duplicate-link-service.test.ts

@@ -1,8 +1,9 @@
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 
-const { store, pbMock, clearStore } = vi.hoisted(() => {
+const { store, pbMock, clearStore, getFullListCalls } = vi.hoisted(() => {
   type StoreMap = Map<string, Record<string, unknown>>;
   const store: Record<string, StoreMap> = {};
+  const getFullListCalls: Record<string, number> = {};
   let seq = 0;
   const nextId = () => `mock-${++seq}`;
 
@@ -11,29 +12,40 @@ const { store, pbMock, clearStore } = vi.hoisted(() => {
     return store[name];
   };
 
+  const evalClause = (item: Record<string, unknown>, clause: string): boolean => {
+    // Mock-only filter parser: accepts the four operators the
+    // duplicate-matcher uses (=, !=, >=, plus the lex variants
+    // <, <=, >). Anything else silently passes through so a
+    // typo in a clause doesn't 500 the test.
+    const m = clause.match(/^(\w+)\s*([!=]+|>=|<=|>|<)\s*"?([^"]+)"?$/);
+    if (!m) return true;
+    const [, field, op, raw] = m;
+    const value = raw.replace(/^"|"$/g, "");
+    const actual = String(item[field] ?? "");
+    if (op === "=") return actual === value;
+    if (op === "!=") return actual !== value;
+    // `created >= "2026-..."` clause — compare lexicographically.
+    // Safe for the YYYY-MM-DD HH:MM:SS.sssZ format PB uses, which
+    // sorts correctly as a string. Without this, every candidate
+    // would pass and the 60-day window would be inert.
+    if (op === ">=") return actual >= value;
+    return true;
+  };
+
   const where = (name: string, filter: string): Record<string, unknown>[] => {
     const items = Array.from(getOrCreateMap(name).values());
     if (!filter || !filter.trim()) return items;
-    const clauses = filter.split("&&").map((c) => c.trim());
+    // PB filter syntax: `||` is OR (lower precedence), `&&` is AND.
+    // The bulk-fetch in suggestOrderDuplicate joins candidate ids
+    // with `||`, so we must honor that — otherwise the regex would
+    // miss the multi-clause filter and silently return everything,
+    // which would still pass behavioural tests but mask the
+    // optimization we're pinning in the new bulk-fetch test below.
+    const orGroups = filter.split("||").map((g) => g.trim()).filter(Boolean);
     return items.filter((item) =>
-      clauses.every((clause) => {
-        // Mock-only filter parser: accepts the four operators the
-        // duplicate-matcher uses (=, !=, >=, plus the lex variants
-        // <, <=, >). Anything else silently passes through so a
-        // typo in a clause doesn't 500 the test.
-        const m = clause.match(/^(\w+)\s*([!=]+|>=|<=|>|<)\s*"?([^"]+)"?$/);
-        if (!m) return true;
-        const [, field, op, raw] = m;
-        const value = raw.replace(/^"|"$/g, "");
-        const actual = String(item[field] ?? "");
-        if (op === "=") return actual === value;
-        if (op === "!=") return actual !== value;
-        // `created >= "2026-..."` clause — compare lexicographically.
-        // Safe for the YYYY-MM-DD HH:MM:SS.sssZ format PB uses, which
-        // sorts correctly as a string. Without this, every candidate
-        // would pass and the 60-day window would be inert.
-        if (op === ">=") return actual >= value;
-        return true;
+      orGroups.some((group) => {
+        const andClauses = group.split("&&").map((c) => c.trim()).filter(Boolean);
+        return andClauses.every((clause) => evalClause(item, clause));
       }),
     );
   };
@@ -51,6 +63,7 @@ const { store, pbMock, clearStore } = vi.hoisted(() => {
         return { items, totalItems: items.length, page: 1, perPage: _perPage };
       },
       async getFullList(opts: { filter?: string; sort?: string } = {}) {
+        getFullListCalls[name] = (getFullListCalls[name] ?? 0) + 1;
         const items = where(name, opts.filter || "");
         // Honor `sort` so tests can pin behaviour that depends on row
         // order. The real PB SDK sorts by the requested field
@@ -120,10 +133,11 @@ const { store, pbMock, clearStore } = vi.hoisted(() => {
 
   const clearStore = () => {
     for (const key of Object.keys(store)) delete store[key];
+    for (const key of Object.keys(getFullListCalls)) delete getFullListCalls[key];
     seq = 0;
   };
 
-  return { store, pbMock, clearStore };
+  return { store, pbMock, clearStore, getFullListCalls };
 });
 
 vi.mock("pocketbase", () => ({ default: vi.fn().mockImplementation(() => pbMock) }));
@@ -343,4 +357,44 @@ describe("suggestOrderDuplicate", () => {
     const updated = await pbMock.collection("orders").getOne("order-no-company");
     expect(updated.duplicate_order_matching_attempted).toBeFalsy();
   });
+
+  it("bulk-fetches candidate items in a single query (kills the N+1)", async () => {
+    // Three candidates for the same company, all with identical items.
+    // The source order also has identical items. With the old per-candidate
+    // loop, the matcher would have made 1 (source items) + 3 (one per
+    // candidate) = 4 getFullList calls against `order_items`. The new
+    // bulk-fetch path makes 1 (source) + 1 (all candidates in one OR query)
+    // = 2. This test pins the optimization — a regression that reverts to
+    // the per-candidate loop will push the count back up to 4.
+    await seedOrderWithItems(
+      "order-cand-1",
+      { order_number: "ORD-C1", company: "comp-1", created: "2026-08-05 00:00:00.000Z" },
+      [{ description: "Same Item", quantity: 5 }],
+    );
+    await seedOrderWithItems(
+      "order-cand-2",
+      { order_number: "ORD-C2", company: "comp-1", created: "2026-08-10 00:00:00.000Z" },
+      [{ description: "Same Item", quantity: 5 }],
+    );
+    await seedOrderWithItems(
+      "order-cand-3",
+      { order_number: "ORD-C3", company: "comp-1", created: "2026-08-15 00:00:00.000Z" },
+      [{ description: "Same Item", quantity: 5 }],
+    );
+    await seedOrderWithItems(
+      "order-source",
+      { order_number: "ORD-SRC", company: "comp-1", created: "2026-08-30 00:00:00.000Z" },
+      [{ description: "Same Item", quantity: 5 }],
+    );
+
+    const before = getFullListCalls["order_items"] ?? 0;
+    const result = await suggestOrderDuplicate("order-source");
+    const after = getFullListCalls["order_items"] ?? 0;
+
+    expect(result.duplicateOrderId).toBe("order-cand-1"); // oldest match wins
+    // 1 fetch for the source order's own items + 1 bulk fetch for ALL
+    // candidates' items. The OLD per-candidate loop would have made
+    // 1 + 3 = 4 calls; the new bulk-fetch makes exactly 2.
+    expect(after - before).toBe(2);
+  });
 });

src/lib/server/order-order-duplicate-link-service.ts

@@ -86,21 +86,39 @@ export async function suggestOrderDuplicate(
     });
 
   let match: { id: string; order_number: string } | null = null;
-  for (const candidate of candidates) {
-    const candidateItemRows = await pb
+
+  // Bulk-fetch ALL items for ALL candidates in one query, instead of
+  // looping with N per-candidate getFullList calls. With many recent
+  // candidates this is the difference between O(N) and O(1) PB calls.
+  if (candidates.length > 0) {
+    const candidateItems = await pb
       .collection(ORDER_ITEMS_COLLECTION)
-      .getFullList<{ description?: string; quantity?: number }>({
-        filter: `order = "${esc(candidate.id)}"`,
-        fields: "description,quantity",
-        batch: 1000,
+      .getFullList<{ order?: string; description?: string; quantity?: number }>({
+        filter: candidates.map((c) => `order = "${esc(c.id)}"`).join(" || "),
+        fields: "order,description,quantity",
+        batch: 5000,
       });
-    const candidateLines: DeliveryLine[] = candidateItemRows.map((r) => ({
-      description: r.description || "",
-      quantity: Number(r.quantity) || 0,
-    }));
-    if (isExactItemMatch(orderLines, candidateLines)) {
-      match = candidate;
-      break; // candidates sorted oldest-first — first hit is the original
+
+    // Group items by candidate order id so we can compare each
+    // candidate's lines to the source order's lines in memory.
+    const itemsByOrder = new Map<string, Array<{ description?: string; quantity?: number }>>();
+    for (const row of candidateItems) {
+      if (!row.order) continue;
+      const bucket = itemsByOrder.get(row.order);
+      if (bucket) bucket.push(row);
+      else itemsByOrder.set(row.order, [row]);
+    }
+
+    for (const candidate of candidates) {
+      const candidateItemRows = itemsByOrder.get(candidate.id) ?? [];
+      const candidateLines: DeliveryLine[] = candidateItemRows.map((r) => ({
+        description: r.description || "",
+        quantity: Number(r.quantity) || 0,
+      }));
+      if (isExactItemMatch(orderLines, candidateLines)) {
+        match = candidate;
+        break; // candidates sorted oldest-first — first hit is the original
+      }
     }
   }