From f9638cc5e5e2fab68488d14aaf9f1a7204ef25b9 Mon Sep 17 00:00:00 2001 From: siddharthd Date: Fri, 21 Aug 2026 17:52:47 +1000 Subject: [PATCH] line-items: one metadata row, agreed on by the write and the read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects on one surface, all invisible while a transaction has exactly one expense_metadata row — which is almost always. 1. ORDER BY (transaction_id = $1) DESC did the opposite of its comment. Postgres orders DESC as NULLS FIRST, and the row that merely POINTS at the transaction has a NULL transaction_id, so the clause written to prefer the owned row preferred the other one — in exactly the case it exists for. matchTransaction already spelled out NULLS LAST; now so do this and GET /api/transactions/[id]/order, which had no ORDER BY at all and could read a different row from the one an enrichment had just written. 2. replaceLineItems takes an optional metadataId. The match resolves a row through a platform-filtered join; re-picking independently here can land the write on a row the match ruled out. 3. previous_line_items on intake_batches is the undo baseline, recorded once per batch under COALESCE and read back. Returning whatever was just read hands a pantry retry its own write as the "previous" state, so undo restores the thing it undid. Migration 0034 is unapplied anywhere, so the column is added there rather than in a 0035. --- .../0034_intake_batches/migration.sql | 10 +++ src/app/api/transactions/[id]/order/route.ts | 8 ++ src/lib/line-items.ts | 74 +++++++++++++++---- 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/prisma/migrations/0034_intake_batches/migration.sql b/prisma/migrations/0034_intake_batches/migration.sql index 3fc9375..3b11b9c 100644 --- a/prisma/migrations/0034_intake_batches/migration.sql +++ b/prisma/migrations/0034_intake_batches/migration.sql @@ -32,10 +32,20 @@ CREATE TABLE IF NOT EXISTS intake_batches ( status text NOT NULL, transaction_id integer, capture_id integer, + -- The undo baseline: what `expense_metadata.line_items` held before this + -- batch first replaced it. Recorded here rather than returned from the write, + -- because pantry re-calls the enrich endpoint on every confirm and after a + -- timeout — a caller that hands back whatever it just read gives a retry its + -- OWN write as the "previous" state, and undo then restores the thing it was + -- meant to reverse. Written under COALESCE, so only the first snapshot sticks. + previous_line_items jsonb, created_at timestamptz NOT NULL DEFAULT now(), resolved_at timestamptz ); +-- For a database that already ran an earlier draft of this file. +ALTER TABLE intake_batches ADD COLUMN IF NOT EXISTS previous_line_items jsonb; + COMMENT ON TABLE intake_batches IS 'One row per upload batch through POST /api/intake. Persisted before ' 'extraction so Slack taps can act on it later and re-shares are detected. ' diff --git a/src/app/api/transactions/[id]/order/route.ts b/src/app/api/transactions/[id]/order/route.ts index 757b1f5..ab3fa42 100644 --- a/src/app/api/transactions/[id]/order/route.ts +++ b/src/app/api/transactions/[id]/order/route.ts @@ -34,6 +34,14 @@ export async function GET( -- through matched_transaction_id. Both directions have to resolve or the -- detail is missing on exactly the orders that were paid by card. WHERE transaction_id = $1 OR matched_transaction_id = $1 + -- The same order as replaceLineItems in line-items.ts and matchTransaction + -- in intake-router.ts. Both directions can resolve at once — a statement + -- line owning its own row while a delivery receipt points at it — and an + -- unordered LIMIT 1 then reads a different row from the one an enrichment + -- wrote, so a PUT that reported success showed nothing on the screen. + -- NULLS LAST because DESC is NULLS FIRST and the pointing row's + -- transaction_id is NULL. + ORDER BY (transaction_id = $1) DESC NULLS LAST, id LIMIT 1`, [Number(id)] ); diff --git a/src/lib/line-items.ts b/src/lib/line-items.ts index b94c846..e8594ee 100644 --- a/src/lib/line-items.ts +++ b/src/lib/line-items.ts @@ -15,6 +15,12 @@ import { queryRow, queryRaw } from "@/lib/db"; * transaction has no row at all (a manual transaction, a statement line), one * is created so the items have somewhere to live; a transaction is NEVER * created here. + * + * `opts.metadataId` pins WHICH row when the caller already resolved one. A + * statement transaction can own its own `expense_metadata` row AND be the + * `matched_transaction_id` of a delivery row: the match query and this write + * then re-pick independently and can land on different rows, so the enrichment + * reports success against a row nobody reads. */ export type LineItemsSource = "intake" | "pantry"; @@ -55,10 +61,35 @@ export type ReplaceResult = { line_items: LineItemRecord[]; }; +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * The undo baseline, recorded once per batch. + * + * Returning the row that was just read makes a RETRY destroy the thing undo + * exists to restore: pantry re-calls this on every confirm and after a timeout, + * and the second call would hand back its own write as the "previous" state. + * The first snapshot a batch ever takes is the only one worth keeping, so it is + * written under COALESCE and read back rather than trusted. + */ +async function undoBaseline(batchId: string | null | undefined, snapshot: unknown[]): Promise { + // Pantry passes back whatever batch id it was given; only an intake batch has + // a row to record against, and a non-uuid would fail the cast. + if (!batchId || !UUID.test(batchId)) return snapshot; + const row = await queryRow<{ previous_line_items: unknown }>( + `UPDATE intake_batches + SET previous_line_items = COALESCE(previous_line_items, $2::jsonb) + WHERE id = $1::uuid + RETURNING previous_line_items`, + [batchId, JSON.stringify(snapshot)] + ); + return Array.isArray(row?.previous_line_items) ? row.previous_line_items : snapshot; +} + export async function replaceLineItems( transactionId: number, lineItems: LineItemRecord[], - opts: { source: LineItemsSource; platform?: string | null; batchId?: string | null } + opts: { source: LineItemsSource; platform?: string | null; batchId?: string | null; metadataId?: number | null } ): Promise { const txn = await queryRow<{ id: number; @@ -73,25 +104,42 @@ export async function replaceLineItems( ); if (!txn) throw new TransactionNotFoundError(`transaction ${transactionId} does not exist`); - // Prefer the row that IS this transaction over one that merely points at it — - // a statement line can be the `matched_transaction_id` of a receipt row and - // carry its own row too, and the items belong on the receipt. - const existing = await queryRow<{ id: number; line_items: unknown }>( - `SELECT id, line_items FROM expense_metadata - WHERE transaction_id = $1 OR matched_transaction_id = $1 - ORDER BY (transaction_id = $1) DESC, id - LIMIT 1`, - [transactionId] - ); + // The caller's row wins where it still exists; otherwise prefer the row that + // IS this transaction over one that merely points at it — a statement line + // can be the `matched_transaction_id` of a receipt row and carry its own row + // too, and the items belong on the receipt. + const pinned = opts.metadataId + ? await queryRow<{ id: number; line_items: unknown }>( + `SELECT id, line_items FROM expense_metadata + WHERE id = $1 AND (transaction_id = $2 OR matched_transaction_id = $2)`, + [opts.metadataId, transactionId] + ) + : null; + const existing = + pinned ?? + (await queryRow<{ id: number; line_items: unknown }>( + `SELECT id, line_items FROM expense_metadata + WHERE transaction_id = $1 OR matched_transaction_id = $1 + -- NULLS LAST is load-bearing, and its absence inverted this clause. + -- Postgres orders DESC as NULLS FIRST, and the row that merely POINTS + -- at the transaction has a NULL transaction_id — so the comparison + -- meant to prefer the owned row preferred the other one, silently and + -- in exactly the case it was written for. matchTransaction spells it + -- out; this did not, and the two therefore disagreed. + ORDER BY (transaction_id = $1) DESC NULLS LAST, id + LIMIT 1`, + [transactionId] + )); const json = JSON.stringify(lineItems); if (existing) { + const previous = await undoBaseline(opts.batchId, Array.isArray(existing.line_items) ? existing.line_items : []); await queryRaw(`UPDATE expense_metadata SET line_items = $2::jsonb WHERE id = $1`, [existing.id, json]); return { transaction_id: transactionId, metadata_id: existing.id, created_metadata: false, - previous_line_items: Array.isArray(existing.line_items) ? existing.line_items : [], + previous_line_items: previous, line_items: lineItems, }; } @@ -118,7 +166,7 @@ export async function replaceLineItems( transaction_id: transactionId, metadata_id: created!.id, created_metadata: true, - previous_line_items: [], + previous_line_items: await undoBaseline(opts.batchId, []), line_items: lineItems, }; }