fix(quote-assistant): exact-SKU rows always shortlisted; per-line shortlist errors
- With a SKU on the line, shortlistStockItems runs a second narrow query (sku = raw || sku ~ normalised) and merges by id, so the capped broad pull can't crowd out the exact SKU row. Quotes and backslashes are stripped from the raw SKU before any filter is built. - fillQuote catches a shortlist failure per line and marks only that line error instead of abandoning the quote. - Supplier names load once per fillQuote and are passed to each shortlist call (new optional parameter; default unchanged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
dd3ab2ad9a
commit
ee5c3bcbb3
4 changed files with 150 additions and 17 deletions
|
|
@ -8,6 +8,7 @@ const store = {
|
|||
itemFilters: [] as string[],
|
||||
updates: [] as { id: string; patch: Record<string, unknown> }[],
|
||||
failUpdateFor: new Set<string>(),
|
||||
supplierLoads: 0,
|
||||
};
|
||||
const events: string[] = [];
|
||||
const actors: { name?: string; source?: string; ignoreRequestUser?: boolean }[] = [];
|
||||
|
|
@ -59,6 +60,14 @@ const pb = {
|
|||
},
|
||||
};
|
||||
}
|
||||
if (name === "suppliers") {
|
||||
return {
|
||||
getFullList: async () => {
|
||||
store.supplierLoads += 1;
|
||||
return [{ id: "sup1", suppliername: "Bolt Co" }];
|
||||
},
|
||||
};
|
||||
}
|
||||
throw new Error(`unexpected collection ${name}`);
|
||||
},
|
||||
};
|
||||
|
|
@ -191,6 +200,7 @@ beforeEach(() => {
|
|||
store.itemFilters = [];
|
||||
store.updates = [];
|
||||
store.failUpdateFor.clear();
|
||||
store.supplierLoads = 0;
|
||||
events.length = 0;
|
||||
actors.length = 0;
|
||||
seq = 0;
|
||||
|
|
@ -477,6 +487,44 @@ describe("fillQuote", () => {
|
|||
expect(forced.decide.mock.calls[0][0].lines.map((l) => l.quote_item_id)).toEqual(["a"]);
|
||||
});
|
||||
|
||||
it("marks only the line whose shortlist failed as error; the rest still run", async () => {
|
||||
addQuote("q1");
|
||||
const a = addItem("q1", { id: "a", description: "breaks the shortlist" });
|
||||
const b = addItem("q1", { id: "b", description: "fine" });
|
||||
const { deps, decide, shortlist } = makeDeps();
|
||||
shortlist.mockImplementation(async (_pb: unknown, line: { description: string }) => {
|
||||
if (line.description === "breaks the shortlist") throw new Error("PB filter error");
|
||||
return [candidate()];
|
||||
});
|
||||
|
||||
const r = await fillQuote("q1", { deps });
|
||||
|
||||
expect(r.filled).toBe(1);
|
||||
expect(r.error).toContain("PB filter error");
|
||||
expect(decide.mock.calls[0][0].lines.map((l) => l.quote_item_id)).toEqual([b]);
|
||||
const failed = store.items.get(a)!;
|
||||
expect((failed.ai_fill as { status: string }).status).toBe("error");
|
||||
expect(failed.pricing_notes).toBe("");
|
||||
expect((store.items.get(b)!.ai_fill as { status: string }).status).toBe("filled");
|
||||
});
|
||||
|
||||
it("loads the supplier names once per quote and hands them to every shortlist call", async () => {
|
||||
addQuote("q1");
|
||||
addItem("q1");
|
||||
addItem("q1");
|
||||
addItem("q1");
|
||||
const { deps, shortlist } = makeDeps();
|
||||
|
||||
await fillQuote("q1", { deps });
|
||||
|
||||
expect(store.supplierLoads).toBe(1);
|
||||
expect(shortlist).toHaveBeenCalledTimes(3);
|
||||
const maps = shortlist.mock.calls.map((c) => (c as unknown[])[3]);
|
||||
expect(maps[0]).toBeInstanceOf(Map);
|
||||
expect((maps[0] as Map<string, string>).get("sup1")).toBe("Bolt Co");
|
||||
expect(maps.every((m) => m === maps[0])).toBe(true);
|
||||
});
|
||||
|
||||
it("does not throw when a single write fails; reports it and continues", async () => {
|
||||
addQuote("q1");
|
||||
const a = addItem("q1", { id: "a" });
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import {
|
|||
type StockCandidate,
|
||||
} from "@/lib/server/quote-assistant-shortlist";
|
||||
import { escapeForPbString } from "@/lib/server/price-list-match";
|
||||
import { loadSuppliersById } from "@/lib/server/item-intel-service";
|
||||
import { isAiFillPending } from "@/lib/quote-assistant-review";
|
||||
import type { Quote, QuoteAssistantRunSummary, QuoteItem } from "@/lib/types";
|
||||
|
||||
|
|
@ -122,18 +123,21 @@ export async function fillQuote(
|
|||
.slice(0, maxLines);
|
||||
if (lines.length === 0) return result;
|
||||
|
||||
// 1. Shortlist POS candidates per line (deterministic, no AI).
|
||||
// 1. Shortlist POS candidates per line (deterministic, no AI). One
|
||||
// line's lookup failing marks only that line as an error.
|
||||
const supplierNameById = await loadSuppliersById(pb);
|
||||
const candidatesById = new Map<string, StockCandidate[]>();
|
||||
try {
|
||||
for (const item of lines) {
|
||||
const shortlistFailed = new Set<string>();
|
||||
for (const item of lines) {
|
||||
try {
|
||||
candidatesById.set(
|
||||
item.id,
|
||||
await shortlist(pb, { description: item.description, sku: item.sku }),
|
||||
await shortlist(pb, { description: item.description, sku: item.sku }, undefined, supplierNameById),
|
||||
);
|
||||
} catch (e) {
|
||||
shortlistFailed.add(item.id);
|
||||
if (!result.error) result.error = `shortlist failed for line ${item.id}: ${errMessage(e)}`;
|
||||
}
|
||||
} catch (e) {
|
||||
result.error = `shortlist failed: ${errMessage(e)}`;
|
||||
return result;
|
||||
}
|
||||
|
||||
// 2. One AI call for the lines that have candidates.
|
||||
|
|
@ -186,8 +190,13 @@ export async function fillQuote(
|
|||
minConfidence: settings.quote_assistant_min_confidence,
|
||||
model,
|
||||
now,
|
||||
status:
|
||||
candidates.length === 0 ? "no_candidates" : aiFailed || noAnswer ? "error" : undefined,
|
||||
status: shortlistFailed.has(line.id)
|
||||
? "error"
|
||||
: candidates.length === 0
|
||||
? "no_candidates"
|
||||
: aiFailed || noAnswer
|
||||
? "error"
|
||||
: undefined,
|
||||
});
|
||||
await pb.collection("quote_items").update(line.id, patch);
|
||||
const status = (patch.ai_fill as { status?: string } | undefined)?.status;
|
||||
|
|
|
|||
|
|
@ -88,10 +88,61 @@ describe("shortlistStockItems", () => {
|
|||
expect(out.map((r) => r.id)).toContain("s1");
|
||||
});
|
||||
|
||||
it("includes the escaped SKU in the PB filter", async () => {
|
||||
it("strips quotes and backslashes from the raw SKU before building filters", async () => {
|
||||
const { pb, getList } = makePb([]);
|
||||
await shortlistStockItems(pb, { description: "", sku: 'AB"123' });
|
||||
expect(getList.mock.calls[0][2].filter).toContain('sku ~ "AB\\"123"');
|
||||
await shortlistStockItems(pb, { description: "", sku: 'AB"12\\3' });
|
||||
for (const call of getList.mock.calls) {
|
||||
const filter = call[2].filter!;
|
||||
expect(filter).not.toContain("\\");
|
||||
expect(filter).not.toMatch(/AB"|AB\\"/);
|
||||
}
|
||||
expect(getList.mock.calls[0][2].filter).toContain('sku ~ "AB123"');
|
||||
});
|
||||
|
||||
it("runs a narrow SKU query and ranks its exact hit first even when the broad pull missed it", async () => {
|
||||
// The broad token pull is capped (500 rows, unsorted): simulate it
|
||||
// filling up with lookalikes that don't include the SKU row.
|
||||
const decoys: Row[] = Array.from({ length: 500 }, (_, i) => ({
|
||||
id: `d${i}`, sku: `X-${i}`, name: "hex bolt zinc", description: "hex bolt zinc",
|
||||
}));
|
||||
const skuRow: Row = { id: "exact", sku: "hb-10/50", name: "Bolt", description: "something else", cost_price: 4 };
|
||||
const { pb, getList } = makePb([]);
|
||||
getList.mockImplementation(async (_p, _n, opts) =>
|
||||
({ items: opts.filter!.includes('sku = "') ? [skuRow] : decoys }),
|
||||
);
|
||||
|
||||
const out = await shortlistStockItems(pb, { description: "hex bolt zinc", sku: "HB 10 50" });
|
||||
|
||||
expect(getList).toHaveBeenCalledTimes(2);
|
||||
const narrow = getList.mock.calls[1][2].filter!;
|
||||
expect(narrow).toBe('is_active = true && (sku = "HB 10 50" || sku ~ "HB1050")');
|
||||
expect(out[0].id).toBe("exact");
|
||||
expect(out[0].score).toBe(1);
|
||||
});
|
||||
|
||||
it("merges the narrow and broad results by id (no duplicates)", async () => {
|
||||
const skuRow: Row = { id: "sku1", sku: "NUT-M18", name: "M18 NUT 8 PLN", description: "M18 NUT 8 PLN" };
|
||||
const { pb } = makePb([skuRow, NUT]);
|
||||
const out = await shortlistStockItems(pb, { description: "M18 hex nut grade 8", sku: "NUT-M18" });
|
||||
expect(out.map((r) => r.id).filter((id) => id === "sku1")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("makes no narrow query without a SKU", async () => {
|
||||
const { pb, getList } = makePb([NUT]);
|
||||
await shortlistStockItems(pb, { description: "M18 hex nut grade 8" });
|
||||
expect(getList).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("uses a supplier map passed in instead of loading suppliers", async () => {
|
||||
const { pb, getFullList } = makePb([NUT]);
|
||||
const [row] = await shortlistStockItems(
|
||||
pb,
|
||||
{ description: "M18 hex nut grade 8" },
|
||||
8,
|
||||
new Map([["sup1", "Preloaded Supplier"]]),
|
||||
);
|
||||
expect(row.supplier_name).toBe("Preloaded Supplier");
|
||||
expect(getFullList).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("drops rows scoring below 0.2", async () => {
|
||||
|
|
|
|||
|
|
@ -7,6 +7,10 @@
|
|||
* the top informative tokens, then in-app Jaccard/overlap scoring. Differences:
|
||||
* only active items, name + description + sku are searched, and an exact
|
||||
* (normalised) SKU hit always wins with score 1.
|
||||
*
|
||||
* The broad pull is capped and unsorted, so with a SKU on the line a second
|
||||
* narrow query (exact / normalised SKU) makes sure the SKU row is never
|
||||
* crowded out; both result sets are merged by id before scoring.
|
||||
*/
|
||||
import type { getAdminPocketBase } from "@/lib/server/pocketbase-admin";
|
||||
import {
|
||||
|
|
@ -35,6 +39,8 @@ export interface StockCandidate {
|
|||
export type AdminPb = Awaited<ReturnType<typeof getAdminPocketBase>>;
|
||||
|
||||
const FETCH_LIMIT = 500;
|
||||
const SKU_FETCH_LIMIT = 50;
|
||||
const STOCK_FIELDS = "id,sku,name,description,notes,unit,cost_price,selling_price,markup,supplier";
|
||||
const MIN_SCORE = 0.2;
|
||||
const MAX_TOKENS = 4;
|
||||
|
||||
|
|
@ -51,13 +57,20 @@ type StockRow = {
|
|||
supplier?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* `supplierNameById`: pass a preloaded supplier map when shortlisting many
|
||||
* lines in a row; when omitted the suppliers are loaded here.
|
||||
*/
|
||||
export async function shortlistStockItems(
|
||||
pb: AdminPb,
|
||||
line: { description: string; sku?: string },
|
||||
limit = 8,
|
||||
supplierNameById?: Map<string, string>,
|
||||
): Promise<StockCandidate[]> {
|
||||
const tokens = informativeTokens(line.description || "", MAX_TOKENS);
|
||||
const rawSku = (line.sku || "").trim();
|
||||
// `"` and `\` can't be escaped reliably in a PB filter string and never
|
||||
// matter to a SKU match (normaliseSku drops them anyway).
|
||||
const rawSku = (line.sku || "").replace(/[\\"]/g, "").trim();
|
||||
const wantedSku = normaliseSku(rawSku);
|
||||
|
||||
const clauses: string[] = [];
|
||||
|
|
@ -70,11 +83,23 @@ export async function shortlistStockItems(
|
|||
|
||||
const res = await pb.collection("stock_items").getList<StockRow>(1, FETCH_LIMIT, {
|
||||
filter: `is_active = true && (${clauses.join(" || ")})`,
|
||||
fields: "id,sku,name,description,notes,unit,cost_price,selling_price,markup,supplier",
|
||||
fields: STOCK_FIELDS,
|
||||
});
|
||||
|
||||
const rowsById = new Map<string, StockRow>();
|
||||
if (rawSku) {
|
||||
const skuClauses = [`sku = "${escapeForPbString(rawSku)}"`];
|
||||
if (wantedSku) skuClauses.push(`sku ~ "${escapeForPbString(wantedSku)}"`);
|
||||
const narrow = await pb.collection("stock_items").getList<StockRow>(1, SKU_FETCH_LIMIT, {
|
||||
filter: `is_active = true && (${skuClauses.join(" || ")})`,
|
||||
fields: STOCK_FIELDS,
|
||||
});
|
||||
for (const row of narrow.items) rowsById.set(row.id, row);
|
||||
}
|
||||
for (const row of res.items) if (!rowsById.has(row.id)) rowsById.set(row.id, row);
|
||||
|
||||
const lineTokens = tokenizeIntel(line.description || "");
|
||||
const scored = res.items
|
||||
const scored = [...rowsById.values()]
|
||||
.map((row) => {
|
||||
const skuHit = wantedSku !== "" && normaliseSku(row.sku) === wantedSku;
|
||||
const score = skuHit
|
||||
|
|
@ -91,7 +116,7 @@ export async function shortlistStockItems(
|
|||
|
||||
if (scored.length === 0) return [];
|
||||
|
||||
const supplierNameById = await loadSuppliersById(pb);
|
||||
const suppliers = supplierNameById ?? (await loadSuppliersById(pb));
|
||||
return scored.map(({ row, score }) => ({
|
||||
id: row.id,
|
||||
name: row.name || "",
|
||||
|
|
@ -103,7 +128,7 @@ export async function shortlistStockItems(
|
|||
selling_price: row.selling_price || 0,
|
||||
markup: row.markup || 0,
|
||||
supplier_id: row.supplier || "",
|
||||
supplier_name: supplierNameById.get(row.supplier || "") || "",
|
||||
supplier_name: suppliers.get(row.supplier || "") || "",
|
||||
score: +score.toFixed(3),
|
||||
}));
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue