From 918f73ea2539c7541dcaf04d6e4c5fac7e325530 Mon Sep 17 00:00:00 2001 From: Dave Date: Wed, 14 Oct 2026 12:42:46 +0200 Subject: [PATCH] =?UTF-8?q?fix(search):=20delivered=5Fat=20is=20never=20po?= =?UTF-8?q?pulated=20=E2=80=94=204,360=20orders=20were=20invisible?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asked what else search excludes, and measured it rather than reading the code. The terminal-state window filtered on: (item_status = "delivered" && delivered_at >= "<90d cutoff>") Live PB, 2026-10-14: 12,957 delivered items, and NOT ONE carries a `delivered_at` value. That clause matches exactly zero rows. Every delivered order was invisible to search — 4,360 of them. `cancelled` was unaffected because it filters on `updated`, which is always populated, which is the entire reason the two states behaved differently. Fix: each terminal state gets two alternatives — the semantically correct timestamp when it exists, and `updated` as the fallback. `updated` is always populated and is >= the delivery moment for a delivered row, so it can only ever be slightly INCLUSIVE, which is the safe direction for a "must come up" guarantee. `removed` (struck-from-order lines) now joins the terminal group too; an order whose only line is `removed` is in effect a cancelled order, which is how /orders already groups it. Coverage gain, measured live: "tiaan" orders 96 -> 107 "450NB" orders 1 -> 2 COST, and it is real Widening the window made the candidate set 5x bigger — 1,075 -> 5,361 orders, 3,5xx -> 16,5xx line items — and the snapshot rebuild is O(scope), so cold went from ~6s to a measured 17-22s. At the old 60s TTL that was 17-22s of PB work every minute per instance, which is not shippable. Mitigated for now by raising both the scope-id and snapshot TTLs to 5 minutes (5x less exposure) and relying on the page's mount pre-warm, now that the pre-warm is actually authenticated and stops being a no-op. Warm searches remain 0ms. Stated plainly: this is a mitigation, not a fix. The architecture is still O(scope) when it should be O(matches) — the quotes search already does it correctly by pushing `~` filters down to PB. That rewrite is the proper follow-up and should happen before the data grows further. Accepted trade: a status flip or new item can take up to 5 minutes to appear in search. Fine for "find the thing I half remember"; not fine if this ever backs anything needing immediate consistency. --- src/lib/server/universal-search-service.ts | 53 +++++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/src/lib/server/universal-search-service.ts b/src/lib/server/universal-search-service.ts index 6f4e8a1..67a37d5 100644 --- a/src/lib/server/universal-search-service.ts +++ b/src/lib/server/universal-search-service.ts @@ -209,7 +209,27 @@ export function orderGroups(groups: UniversalGroup[]): UniversalGroup[] { * ========================================================================== */ let scopedOrderIdsValue: { ids: string[]; expiresAt: number } | null = null; -const SCOPED_ORDER_IDS_TTL_MS = 60_000; +/** + * TTL for BOTH the scope-id list and the order snapshot. + * + * Raised from 60s to 5 minutes on 2026-10-14, after the scope fix below + * widened the candidate set from 1,075 to 5,361 orders (16.5k line + * items) and the cold rebuild went from ~6s to a measured 17-22s. The + * rebuild is O(scope), so at 60s it was re-running 17-22s of PB work + * every minute per instance. At 5 minutes that cost is amortised 5x, + * and the /search page pre-warms on mount, so the rebuild normally + * happens while nobody is waiting on it. + * + * The trade: a status flip or new item can take up to 5 minutes to + * appear in search results. Acceptable for a "find the thing I half + * remember" tool; NOT acceptable if this snapshot ever backs anything + * that must be immediately consistent. + * + * Long term this wants pushdown (let PB answer `~` queries directly, + * making cost O(matches) instead of O(scope)) — see the note in + * `fetchScopedOrderIds`. + */ +const SCOPED_ORDER_IDS_TTL_MS = 300_000; /** * Materialised order snapshot: headers, line items and supplier names @@ -220,10 +240,9 @@ const SCOPED_ORDER_IDS_TTL_MS = 60_000; * the actual matching accounting for ~10ms. The snapshot turns that * round-trip into a one-off amortised over a burst of typing. * - * TTL is deliberately short (60s) and matches the scope-id cache: a - * status flip or a new item should show up promptly, but nobody types - * 40 characters faster than once a minute. Cache-busting on write is - * left to that TTL — see clearUniversalSearchCaches for tests. + * Shares its TTL with the scope-id cache — see the note there for why + * that is 5 minutes rather than 60s. Cache-busting on write is left to + * the TTL; clearUniversalSearchCaches exists for tests. */ interface OrderSnapshot { orders: Record[]; @@ -233,7 +252,7 @@ interface OrderSnapshot { } let orderSnapshotValue: OrderSnapshot | null = null; -const ORDER_SNAPSHOT_TTL_MS = 60_000; +const ORDER_SNAPSHOT_TTL_MS = 300_000; /** * Reference collections (suppliers, companies, customers, users, stock @@ -280,8 +299,30 @@ async function fetchScopedOrderIds(pb: AdminPocketBase): Promise { const cutoff = daysAgoIso(UNIVERSAL_SEARCH_WINDOW_DAYS); const filter = [ ...NON_TERMINAL_STATUSES.map((s) => `item_status = "${s}"`), + // Terminal states inside the window. + // + // `delivered_at` IS NEVER POPULATED on this dataset — measured + // 2026-10-14: 12,957 delivered items, zero of them carrying a + // `delivered_at` value. Filtering on it alone therefore matches + // exactly zero rows and renders every delivered order invisible to + // search (4,360 orders on live PB). + // + // So each terminal state gets TWO alternatives: the semantically + // correct timestamp when it exists, and `updated` as the fallback. + // `updated` is always populated and is >= the moment of delivery for + // a delivered row, so it can only ever be slightly INCLUSIVE — the + // safe direction for a "must come up" guarantee. Cancelled already + // worked this way, which is why it returned rows and delivered + // didn't. + // + // `removed` (struck-from-order lines) is treated with the terminal + // group: an order whose only line is `removed` is, in effect, a + // cancelled order, and the /orders cancelled tab already groups them + // together. `(item_status = "delivered" && delivered_at >= "${cutoff}")`, + `(item_status = "delivered" && updated >= "${cutoff}")`, `(item_status = "cancelled" && updated >= "${cutoff}")`, + `(item_status = "removed" && updated >= "${cutoff}")`, ].join(" || "); const rows = await pb