fix(search): delivered_at is never populated — 4,360 orders were invisible

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.
This commit is contained in:
Dave 2026-10-14 12:42:46 +02:00
parent f1dc78c9a0
commit 918f73ea25

View file

@ -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<string, unknown>[];
@ -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<string[]> {
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