1 Commits
Author SHA1 Message Date
siddharthd 669f9119b9 enrichment readers: resolve one metadata row, deterministically (board #253)
ci / lint-test (push) Successful in 53s
Four readers of expense_metadata that only the 1:1 UNIQUE constraints keep
correct, found while designing wallet-first ingestion. Each fails silently
rather than erroring, so none of them would announce itself.

merchantVerdict (order-reviews.ts) was a genuine fan-out: a plain JOIN on
transaction_id OR matched_transaction_id, no per-transaction cap, feeding
`for (const r of rows) counts[r.rating] += 1`. One review joined to N
metadata rows is counted N times, inflating the loved/never tallies that
drive the pre-order warning — and its LIMIT 50 applies to the fanned-out
set, so duplicates would also push older reviews out of the window. Now a
LATERAL, so the merchant matched is the merchant the UI shows.

nudgeState (slack/interactive) had no LIMIT and no ORDER BY, feeding
queryRow, which returns rows[0]. Postgres guarantees no ordering, so the
merchant and currency on every Slack card refresh could flip at random.
It was the only "resolve both links" site missing the convention its
siblings all carry.

getSharedTransactions had LATERAL LIMIT 1 with no ORDER BY — bounded, but
arbitrary, and free to disagree with the main transaction list about the
same transaction.

transaction_search (migration 0035): NOT the fan-out I first claimed. It
joined only on em.transaction_id, which is UNIQUE, so it could not
multiply rows and sums over it were never inflated. The real defect is
narrower and worse: it OMITTED matched_transaction_id, so every
card-settled order was invisible to it — 148 transactions (Glovo Milan,
Uber Eats, all source='order-bridge') reporting NULL payment_method /
order_reference / line_items while the data sat in the table. This view is
the agent read surface (Hermes' MCP selects those columns from it) and
home-dashboard reads it via dash_ro, so the omission was estate-wide.
Broadening the join is what introduces fan-out risk — transaction_id and
matched_transaction_id are two SEPARATE unique indexes, so two different
rows can legally point at one transaction — hence LATERAL rather than a
second OR. Verified before applying: 4760 rows unchanged, debit sum
unchanged at 1514412.89, only the 148 rows differ. Grants re-granted and
re-verified, since CREATE OR REPLACE VIEW drops them.

Also fixes an integration assertion 258fe3b left stale: it changed the
needs-header card text and updated the unit tests but not intake.test.ts.
The new assertion checks both halves of needsHeaderCard — that it names
what is actually missing, and that a non-in-store source gets the
share-more-of-the-screen advice.

npm run test:all: 182 unit + 276 integration, all passing.
2026-08-22 15:57:31 +10:00