statements: derive account identity, fix the new-bank alert (0033)
ci / lint-test (push) Successful in 42s
ci / lint-test (push) Successful in 42s
account_number fragments exactly like bank_name did, from the same cause: ANZ's Access Advantage arrived as both '4085-56264' (4 statements) and '408556264' (1). uq_statement_identity keyed on it, so re-importing one period under the other spelling evaded the duplicate check. account_number_key is GENERATED ALWAYS — the number with separators stripped — and the unique index moves onto it. 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. The Amex mask and case are preserved; 'X' records which digits were redacted. Not the cause of the documented 31-row / $42,040.68 ANZ duplication — 107/142/143 overlap on different end dates, which that index cannot catch at any spelling. Adds statement_identity_drift: one account under two bank names, or one bank under two spellings of one number. Non-empty means fragmentation the normaliser could not know about. Currently one row, the ANZ pair above, now unified by key. Also documents the N8N regression this series caused. Check Known Bank compared the raw Gemini name against the canonical column before the insert, so nothing ever matched and every Zip statement was tagged pending and held for Slack approval — a stall, not just a noisy alert. Fixed in workflow FysADdFwEtwONQl4 by comparing like with like through normalize_bank_name(), passing currency so a genuinely new national entity still alerts. Verified live: 12s after vs 30-53s before.
This commit is contained in:
@@ -898,20 +898,76 @@ print.
|
|||||||
Wise is deliberately unmapped: `Wise Australia Pty Ltd.` is a single spelling, so
|
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.
|
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
|
**One institution's national entities stay apart (migration 0032).** A prefix
|
||||||
*new* bank are one institution. That always shows up the same way, so check it
|
generalises past the evidence it was built on: every 0031 merge was provably one
|
||||||
rather than trusting the map:
|
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
|
Currency alone cannot be the discriminator: **Wise holds AUD, EUR and USD
|
||||||
SELECT account_number, array_agg(DISTINCT bank_name)
|
accounts under one provider**, so "non-AUD means a different bank" would shatter
|
||||||
FROM statements WHERE account_number IS NOT NULL
|
it into three. That is why Wise must stay unmapped, and why mapping any
|
||||||
GROUP BY 1 HAVING count(DISTINCT bank_name) > 1;
|
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
|
`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.
|
UNIQUE constraint, so any future rename must update it too or strand its rows.
|
||||||
It is empty today; the migration handles it anyway.
|
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 = '<raw Gemini 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
|
### Loans
|
||||||
|
|
||||||
A loan repayment is **not** an expense. It is part principal (equity, a
|
A loan repayment is **not** an expense. It is part principal (equity, a
|
||||||
|
|||||||
@@ -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.';
|
||||||
@@ -135,6 +135,11 @@ model statements {
|
|||||||
card_name String?
|
card_name String?
|
||||||
account_type String?
|
account_type String?
|
||||||
account_number 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_start_date DateTime? @db.Date
|
||||||
billing_end_date DateTime? @db.Date
|
billing_end_date DateTime? @db.Date
|
||||||
total_amount_due Decimal? @db.Decimal(12, 2)
|
total_amount_due Decimal? @db.Decimal(12, 2)
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { describe, it, expect, beforeAll } from "vitest";
|
import { describe, it, expect, beforeAll } from "vitest";
|
||||||
|
import type { NextRequest } from "next/server";
|
||||||
import { queryRaw, queryRow } from "../../lib/db";
|
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.
|
* so the same 50/50 rows flip sides untouched.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
type Res = { status: number; json: () => Promise<Record<string, unknown>> };
|
type Ctx = { params: Promise<{ id: string }> };
|
||||||
|
type Handler = (req: NextRequest, ctx: Ctx) => Promise<Response>;
|
||||||
|
|
||||||
let txPATCH: (req: unknown, ctx: { params: Promise<{ id: string }> }) => Promise<Res>;
|
let txPATCH: Handler;
|
||||||
let stmtPATCH: (req: unknown, ctx: { params: Promise<{ id: string }> }) => Promise<Res>;
|
let stmtPATCH: Handler;
|
||||||
|
|
||||||
const req = (email: string | null, body: unknown) => ({
|
/** Enough of a NextRequest for these routes: the auth header and the body. */
|
||||||
headers: { get: (h: string) => (h.toLowerCase() === "x-forwarded-user" ? email : null) },
|
const req = (email: string | null, body: unknown) =>
|
||||||
json: async () => body,
|
({
|
||||||
});
|
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) }) });
|
const ctx = (id: number) => ({ params: Promise.resolve({ id: String(id) }) });
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user