perf(inbox): batch preceding-context assembly and bound-agent route validation

[bestony]

Closes #1724 (PERF-061).

Inbox context assembly and bound-agent validation both fanned out into sequential single-row queries. This replaces both with set-based statements.

What was slow

Preceding context (services/inbox.ts) issued one query per claimed trigger plus one per chat, all inside the delivery transaction. A 50-entry drain held its row locks across 50+ sequential round-trips.

Route validation (api/agent/ws-client.ts) looked up one agents row per bound agent on every inbox:ack, and did it twice per heartbeat. A client holding N bindings cost 2N queries per heartbeat, and there is no hard per-client agent cap — so the cost scaled with both fleet size and heartbeat frequency.

What changed

One statement for preceding context. Each trigger's lower bound is statically derivable: within a chat it is the previous trigger's id, and only the first trigger has to look outside the batch. Nothing depends on a previous iteration's result, so the whole loop collapses into jsonb_to_recordset (batch input, matching the idiom already in claimBacklogForPushFair) → lag() (in-batch bound) → short-circuiting COALESCE scalar subquery (out-of-batch bound) → CROSS JOIN LATERAL (per-trigger window).

The lock stays inside the LATERAL. PostgreSQL rejects FOR UPDATE at any query level that also carries a window function, and on the nullable side of an outer join — this is the only shape that keeps FOR UPDATE OF e SKIP LOCKED intact. Per-trigger ranges (trigger[i-1].id, trigger[i].id) are disjoint by construction, so one statement locks exactly the rows the loop would have.

One query for route validation. ensureAgentsStillRoutedHere resolves the whole set at once and applies the unchanged per-agent judgement, including the runtime-switch claim branch that parks a binding rather than dropping it. The single-agent helper delegates to it, so there is exactly one implementation of the judgement and its drop side effects.

The heartbeat's second route check is removed, not batched. restoredAgentIds is already a subset of what the first check returned (it is filtered by an EXISTS on the same client + active status), and the repair path re-validates anyway: maybeRepairInboxBacklog gates on the in-memory route, then drainBacklogForAgent opens with its own ensureAgentStillRoutedHere — a strictly later, and therefore fresher, authoritative check. Removing it closes no race. The one residual effect: an agent that just moved away can still consume a repair-throttle slot, which is meaningless for a binding that is going away.

Why the index changed

This is the part worth reviewing closely. Batching alone made the query slower, and the measurements are what drove the index change.

idx_inbox_chat_silent did not include id. In the old loop the window bounds were constants in each statement, so the planner walked inbox_entries_pkey over a narrow id range. Inside a LATERAL those same bounds are correlated values — the planner cannot estimate their width and falls back to scanning every silent row in the chat before discarding almost all of them:

Bitmap Index Scan on idx_inbox_chat_silent (actual rows=3636 loops=50)
Rows Removed by Filter: 3627        <- scanned 3636, kept 9
Heap Blocks: exact=131710

With id reachable by the scan the range collapses into it (rows=26, Heap Blocks: exact=909).

Partial, not a wider composite

The obvious fix — appending id to the existing four-column key — works but is expensive, because it defeats B-tree deduplication. The four-column key has only a few hundred distinct combinations per inbox, so near-duplicate keys compress; a key ending in the unique id makes every key distinct and the compression disappears:

index	size (240k rows)	bytes/row
(inbox_id, chat_id, notify, status)	1776 kB	7.6
(inbox_id, chat_id, notify, status, id)	11 MB	49.3
partial pair, both (inbox_id, chat_id, id)	2.6 MB	—

(7.6 bytes/row is below a bare index tuple, which is what confirms deduplication is active.)

So status/notify move from the key into predicates instead. That also bounds growth: inbox_entries is append-only, and the silent index now covers only rows still pending rather than all history. The notify-cursor index is deliberately not filtered on status — an already-acked trigger still closes a preceding-context window, and ACK-through has to walk acked rows to prove its prefix has no gap.

Measured effect

PG17, 240k inbox_entries, 40k in the hot inbox across 60 chats, production-like 80% acked mix, one 50-trigger drain. DB-side only — no round-trips, which is the measurement most favourable to the old loop. Medians of repeated runs:

	50-trigger drain
old loop, current index	11.5 ms
new statement, current index	51 ms — slower than the loop
new statement, composite +id	2.9 ms
new statement, partial pair	2.7 ms

The other (inbox_id, chat_id) shapes in this service were checked for regression:

query	current index	partial pair
chat-scoped claim	0.116 ms	0.115 ms
claim-prefix scan	0.102 ms	0.120 ms
ACK-through prefix walk	0.200 ms	0.052 ms
ACK-through silent drain	0.307 ms	0.069 ms

Adding one round-trip back in (~1.5 ms on managed PG), lock-hold time for a 50-entry drain goes from ~87 ms to ~4 ms.

Migration safety

inbox_entries sits in the delivery hot path, and this migration drops an index — DROP INDEX takes ACCESS EXCLUSIVE, a plain CREATE INDEX takes SHARE. The migration file is hand-annotated with an operator runbook and IF NOT EXISTS / IF EXISTS guards, following the pattern in 0025_inbox_silent_entries.sql, 0026, 0064, and 0065 (drizzle wraps migrations in a transaction, so CONCURRENTLY cannot appear in the file itself).

The guards change re-execution behaviour only, not the resulting schema. Re-running drizzle-kit generate on this branch reports "No schema changes, nothing to migrate" and leaves the working tree clean, so the snapshot still matches src/db/schema/inbox-entries.ts — the invariant behind "never hand-edit migrations" holds. The runbook itself was executed verbatim against PG17 to confirm it works: both CREATE INDEX CONCURRENTLY IF NOT EXISTS statements succeed, indisvalid is true for each, and DROP INDEX CONCURRENTLY IF EXISTS then removes the old index.

The new indexes take new names specifically so that runbook can create them before dropping the old one. Reusing idx_inbox_chat_silent would force a window where the silent-row lookup has no index behind it.

Behaviour held fixed

• Per-trigger top-N, not a global LIMIT — the ORDER BY messages.created_at DESC + LIMIT stays inside the LATERAL, so the rows nearest each trigger survive the cap.
• Ordering is explicit at the outer level. A subquery's ORDER BY only decides which rows LIMIT keeps; it is not carried outward. The outer query orders on the raw timestamp so the prompt block reads oldest → newest under any plan.
• Sort key stays messages.created_at, not inbox_entries.created_at — addParticipant's backfill writes its rows in one INSERT VALUES (...) sharing a single statement_timestamp(). The 24h floor still compares inbox_entries.created_at; that asymmetry predates this change and is preserved.
• The previous-notify bound has no status filter — an already-acked notify row still closes the window.
• Window start is computed in TypeScript, not as a SQL interval, so the 24h boundary is bit-identical to before.
• Timestamps are rendered to ISO 8601 in SQL, because a raw statement returns PostgreSQL's own timestamp text (2026-07-20 00:01:00.123456+00) rather than a Date. to_char(... AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"') matches Date#toISOString() byte for byte — checked against the boundaries that could diverge:

input	to_char	toISOString()
.123456+00	2026-07-20T00:01:00.123Z	same
.999999+00	...00.999Z	same
.0005+00 (sub-ms truncation)	...00.000Z	same
no fractional part	...00.000Z	same
.5+00 (single digit padded)	...00.500Z	same
23:59:59.9999 (carry boundary)	...59.999Z	same
year 0999 (padded)	0999-01-02T03:04:05.060Z	same
12:00:00.123456+08 (non-UTC input)	2026-07-20T04:00:00.123Z	same

The explicit AT TIME ZONE 'UTC' is what keeps this independent of session timezone.

Verification

pnpm check · pnpm typecheck · pnpm test — 247 files / 2700 tests, all passing (testcontainers PG17, migration applied).

New inbox-preceding-context-batch.test.ts covers what batching can break while single-trigger tests stay green:

• each trigger in one drain gets its own window (a shared lower bound would leak earlier silent rows forward)
• per-chat windows stay separate when one drain spans several chats
• ordering follows message time, not inbox row order, and timestamps stay ISO 8601
• an already-acked notify row closes the window
• null-chat rows are skipped without breaking the batch
• query count is identical for a 2-trigger and an 8-trigger drain, with execute at exactly 1 — the O(N)→O(1) claim as an assertion rather than a comment. Both statement kinds are counted, so reintroducing a per-trigger select next to the batched one would still fail it
• a silent row held under FOR UPDATE by another transaction is skipped, not blocked on

inbox-delivery-indexes.test.ts gains an assertion on the new index shapes, so id in the key and the absent status filter on the notify cursor are contracts rather than incidental.

The cap behaviour (keep the rows nearest the trigger when candidates exceed PRECEDING_CONTEXT_MAX_ENTRIES) is already pinned by the existing inbox-ws-push.test.ts case and stays green.

One test-harness change: ws-client-branch-fake.test.ts's activeAgentRow() now supplies uuid. Batched validation needs it to map each returned row back to its agent; a single-row lookup left that mapping implicit. The real agents table has always returned the column.

QA

Touches the WS/inbox path, so formal QA is warranted. packages/qa/cases/cross-surface/authenticated-ws-inbox-delivery.md already covers this loop end-to-end; no new case needed.