diff --git a/CLAUDE.md b/CLAUDE.md index c350c9f..d6a6cea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -898,20 +898,76 @@ print. Wise is deliberately unmapped: `Wise Australia Pty Ltd.` is a single spelling, so shortening it would be a rename nobody asked for rather than a merge. -**The residual gap and its tell.** The function cannot know that two names for a -*new* bank are one institution. That always shows up the same way, so check it -rather than trusting the map: +**One institution's national entities stay apart (migration 0032).** A prefix +generalises past the evidence it was built on: every 0031 merge was provably one +account, but `citi%` would have flattened Citibank India into Citibank Australia +on arrival. `hsbc%` and `%american express%` had the same defect. The country now +comes from the name when the name states it and from the currency otherwise, with +AUD as home taking no suffix — so nothing renamed (`UPDATE 0`). -```sql -SELECT account_number, array_agg(DISTINCT bank_name) -FROM statements WHERE account_number IS NOT NULL -GROUP BY 1 HAVING count(DISTINCT bank_name) > 1; -``` +Currency alone cannot be the discriminator: **Wise holds AUD, EUR and USD +accounts under one provider**, so "non-AUD means a different bank" would shatter +it into three. That is why Wise must stay unmapped, and why mapping any +multi-currency provider through this function would be wrong. + +`N.A.` is deliberately not a country marker — it means "National Association", a +US legal form printed on Citibank letterhead worldwide including India. + +**The residual gap and its tell.** The function cannot know that two names for a +*new* bank are one institution. That always shows up the same way, so check the +`statement_identity_drift` view (migration 0033) rather than trusting the map. +Non-empty means an account is fragmented, by name or by number. + +### Account identity is derived, not the raw number (migration 0033) + +`account_number` fragments exactly like `bank_name` did, from the same cause — +Gemini copies what the PDF prints. ANZ's Access Advantage arrived as both +`4085-56264` (4 statements) and `408556264` (1). Since `uq_statement_identity` +keyed on it, re-importing one period under the other spelling evaded the +duplicate check. + +`account_number_key` is a **GENERATED ALWAYS** column — `account_number` with +separators stripped — and the unique index is now +`(bank_name, account_number_key, billing_end_date)`. **Never write to it; +display `account_number`.** + +Derived rather than rewritten because the raw number is the readable one and +some of it is structure: Up stores `633-123 / 176540052`, a BSB *and* an account +number, and flattening it would lose a distinction a human reads at a glance to +fix a machine problem. Case and the Amex mask (`XXXX-XXXXXX-01000`) are +preserved — `X` records which digits were redacted. + +This is **not** what caused the documented 31-row / $42,040.68 ANZ duplication. +Statements 107/142/143 overlap on *different* end dates, which that index cannot +catch at any spelling. Different problem, same table. `account_owner_mappings` also keys on `(bank_name, account_number)` with its own UNIQUE constraint, so any future rename must update it too or strand its rows. It is empty today; the migration handles it anyway. +**Normalising `bank_name` broke the N8N new-bank alert, and the fix lives in the +workflow.** `Check Known Bank` ran +`SELECT COUNT(id) FROM statements WHERE bank_name = ''`, +comparing the raw extraction against the now-canonical column *before* the insert +— so the trigger had not fired yet and nothing ever matched. That branch tags the +document **pending and holds it for Slack approval**, so every Zip statement +stalled awaiting a manual click, not merely a noisy alert. It was a regression +made worse by normalisation: previously a repeat of the same spelling at least +matched. + +Fixed 2026-08-15 by comparing like with like: + +```sql +SELECT COUNT(id) as count FROM statements +WHERE bank_name = normalize_bank_name('{{ ...summary.bank_name }}', '{{ ...summary.currency }}') +``` + +Currency is passed so a genuinely new national entity (Citibank India) still +alerts. Verified live: the run after the fix took 12s against 30–53s for the +approval-branch runs before it. Workflow `FysADdFwEtwONQl4` in the smarthome +repo — **any future change to `normalize_bank_name()` must keep that node in +step**, since it is the one caller outside this codebase. + ### Loans A loan repayment is **not** an expense. It is part principal (equity, a diff --git a/prisma/migrations/0033_account_number_key/migration.sql b/prisma/migrations/0033_account_number_key/migration.sql new file mode 100644 index 0000000..8697049 --- /dev/null +++ b/prisma/migrations/0033_account_number_key/migration.sql @@ -0,0 +1,71 @@ +-- One account, one identity — without destroying how the number reads. +-- +-- `account_number` carries the same fragmentation `bank_name` had, from the +-- same cause: Gemini copies whatever the PDF prints, and the formatting varies +-- per document. +-- +-- ANZ '4085-56264' 4 statements ANZ ACCESS ADVANTAGE +-- ANZ '408556264' 1 statement ACCESS ADVANTAGE <- same account +-- +-- That matters because `uq_statement_identity` is +-- (bank_name, account_number, billing_end_date). Re-importing one period under +-- the other spelling evades the duplicate check entirely. +-- +-- (It is NOT what caused the documented 31-row / $42,040.68 ANZ duplication. +-- Statements 107/142/143 overlap on *different* end dates, which that index +-- cannot catch at any spelling. Different problem, same table.) +-- +-- WHY A GENERATED COLUMN RATHER THAN REWRITING THE VALUE: the raw number is the +-- readable one and some of it is structure, not noise. Up stores +-- '633-123 / 176540052' — a BSB and an account number — and flattening that to +-- '633123176540052' would lose a distinction a human reads at a glance, to fix +-- a machine problem. So the raw text stays exactly as extracted and the +-- comparison key is derived beside it. Same reason the app derives trip +-- participation instead of storing it: two records of one fact drift. +-- +-- Separators only. Case and the Amex mask ('XXXX-XXXXXX-01000') are preserved, +-- because 'X' is real information about which digits were redacted, and +-- case-folding an alphanumeric account id could merge two genuinely different +-- ones. +-- +-- Verified collision-free before rebuilding the index: no two statements share +-- (bank_name, stripped account_number, billing_end_date). +-- +-- Idempotent: safe to re-run. + +ALTER TABLE statements + ADD COLUMN IF NOT EXISTS account_number_key TEXT + GENERATED ALWAYS AS ( + NULLIF(regexp_replace(coalesce(account_number, ''), '[^0-9A-Za-z]', '', 'g'), '') + ) STORED; + +COMMENT ON COLUMN statements.account_number_key IS + 'Derived from account_number, separators stripped. The identity used by ' + 'uq_statement_identity so one account under two formats is one account. ' + 'Never write to it — it is GENERATED. Display account_number instead.'; + +-- Rebuild the identity index on the derived key. Dropped and recreated rather +-- than added alongside: leaving the old one in place would keep admitting the +-- duplicate it exists to stop. +DROP INDEX IF EXISTS uq_statement_identity; +CREATE UNIQUE INDEX uq_statement_identity + ON statements (bank_name, account_number_key, billing_end_date) + WHERE bank_name IS NOT NULL AND billing_end_date IS NOT NULL; + +-- The fragmentation detector, now able to see both halves: one account under +-- two bank names, or one bank under two spellings of one number. +CREATE OR REPLACE VIEW statement_identity_drift AS + SELECT account_number_key, + array_agg(DISTINCT bank_name) AS bank_names, + array_agg(DISTINCT account_number) AS account_numbers, + count(*) AS statements + FROM statements + WHERE account_number_key IS NOT NULL + GROUP BY account_number_key + HAVING count(DISTINCT bank_name) > 1 + OR count(DISTINCT account_number) > 1; + +COMMENT ON VIEW statement_identity_drift IS + 'Non-empty means an account is fragmented: normalize_bank_name() cannot know ' + 'that two names for a bank it has never seen are one institution, and this is ' + 'how that always shows up. Check it after loading a new bank.'; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 4653e44..c06757a 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -135,6 +135,11 @@ model statements { card_name String? account_type String? account_number String + /// GENERATED ALWAYS (migration 0033) — account_number with separators + /// stripped, so '4085-56264' and '408556264' are one account. Backs + /// uq_statement_identity. Read-only: never write to it, display + /// account_number instead. + account_number_key String? billing_start_date DateTime? @db.Date billing_end_date DateTime? @db.Date total_amount_due Decimal? @db.Decimal(12, 2) diff --git a/src/__tests__/integration/transaction-owner.test.ts b/src/__tests__/integration/transaction-owner.test.ts index d628ad7..51453e2 100644 --- a/src/__tests__/integration/transaction-owner.test.ts +++ b/src/__tests__/integration/transaction-owner.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, beforeAll } from "vitest"; +import type { NextRequest } from "next/server"; import { queryRaw, queryRow } from "../../lib/db"; /** @@ -20,15 +21,18 @@ import { queryRaw, queryRow } from "../../lib/db"; * so the same 50/50 rows flip sides untouched. */ -type Res = { status: number; json: () => Promise> }; +type Ctx = { params: Promise<{ id: string }> }; +type Handler = (req: NextRequest, ctx: Ctx) => Promise; -let txPATCH: (req: unknown, ctx: { params: Promise<{ id: string }> }) => Promise; -let stmtPATCH: (req: unknown, ctx: { params: Promise<{ id: string }> }) => Promise; +let txPATCH: Handler; +let stmtPATCH: Handler; -const req = (email: string | null, body: unknown) => ({ - headers: { get: (h: string) => (h.toLowerCase() === "x-forwarded-user" ? email : null) }, - json: async () => body, -}); +/** Enough of a NextRequest for these routes: the auth header and the body. */ +const req = (email: string | null, body: unknown) => + ({ + headers: { get: (h: string) => (h.toLowerCase() === "x-forwarded-user" ? email : null) }, + json: async () => body, + }) as unknown as NextRequest; const ctx = (id: number) => ({ params: Promise.resolve({ id: String(id) }) });