From bb8a009e0270b5cc015eae60bcb5dc71e02c330f Mon Sep 17 00:00:00 2001 From: siddharthd Date: Thu, 13 Aug 2026 12:23:01 +1000 Subject: [PATCH] frollo: don't re-import what the ledger already has, and stop hiding the queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first import wrote 550 rows on 2026-08-13. 422 of them (77%) were second copies of transactions the ledger already held from statements — $1,023,824.63 of movement counted twice. The owner found it by opening the transactions view and seeing one HDR Global salary listed twice, once as A$15,518.53 from the statement and once as US$10,782.00 from the feed. The currency was never the defect. toLedgerRow already left amount_aud NULL and named the currency in foreign_currency_code, which is the documented contract for a row whose AUD value is unknown, and the transactions page labels it. What made it look wrong was the duplicate sitting beside it. Two changes. Upstream, ingestFrolloCsv now drops rows the ledger already holds, matching on amount + direction within LEDGER_MATCH_DAYS (3). Three things had to be right and the first two were not, each caught only by rehearsing against real data rather than fixtures: - pg returns a DATE as a JS Date while Prisma and the CSV give strings. String(date).slice(0,10) is "Wed Mar 10", which parses to NaN, so the first dry run reported 550 to insert and zero duplicates. dayMs() takes both. - Direction has to be in the key. This ledger is full of internal transfers between the owner's own accounts and the feed carries both legs: 2026-05-18 has +3076.04 into ANZ and -3076.04 out of AMP. Matching on amount alone let the credit leg consume the ledger's debit row, so the real duplicate was written — 20 rows got in that way. - 'refund' is money in. The feed calls a reversed account fee a credit and the statement importer types it 'refund'; classifying it as an outflow left every ANZ servicing-fee reversal behind. Downstream, awaitsStatementLine() is removed. Its premise — feed rows never await a statement line — was asserted, never tested, and false for almost every account. Worse is how it got there: the reconcile queue jumped 8 -> 558 when the feed landed, that jump was read as noise and filtered away, and filtering it removed the only mechanism that would ever have collapsed the duplicates. The queue was right. A feed row IS a row awaiting its statement line. Re-imported: 375 dropped as already-on-ledger, 175 inserted. Residual duplicates 4 rows / $15.01, all sub-$5 account fees where several identical amounts fall in overlapping windows and greedy consumption picks the wrong one; not chased further at this scale. The CLI prints the already-on-ledger count even when zero — a number you have to go looking for is a number nobody looks at. --- scripts/import-frollo.mts | 6 +- src/__tests__/unit/frollo-ingest.test.ts | 189 +++++++++++++++++++++++ src/lib/frollo-ingest.ts | 123 ++++++++++++++- src/lib/queries.ts | 38 +++-- 4 files changed, 337 insertions(+), 19 deletions(-) create mode 100644 src/__tests__/unit/frollo-ingest.test.ts diff --git a/scripts/import-frollo.mts b/scripts/import-frollo.mts index 45f98c0..ff05fdd 100644 --- a/scripts/import-frollo.mts +++ b/scripts/import-frollo.mts @@ -25,7 +25,7 @@ */ import { readFileSync } from "node:fs"; import pg from "pg"; -import { ingestFrolloCsv, type SqlExecutor } from "../src/lib/frollo-ingest.ts"; +import { ingestFrolloCsv, LEDGER_MATCH_DAYS, type SqlExecutor } from "../src/lib/frollo-ingest.ts"; function arg(name: string, fallback?: string): string | undefined { const i = process.argv.indexOf(`--${name}`); @@ -99,6 +99,10 @@ console.log(); console.log("LEDGER"); console.log(` already imported : ${String(report.alreadyImported).padStart(5)}`); +// Printed even when zero. This is the count that was missing on 2026-08-13, +// when the whole file was written on top of a ledger that already held 77% of +// it; a number you have to go looking for is a number nobody looks at. +console.log(` already on ledger: ${String(report.ledgerDuplicates).padStart(5)} (statement twin within ${LEDGER_MATCH_DAYS} days)`); console.log(` new to insert : ${String(report.toInsert).padStart(5)}`); if (report.anomalies.length > 0) { diff --git a/src/__tests__/unit/frollo-ingest.test.ts b/src/__tests__/unit/frollo-ingest.test.ts new file mode 100644 index 0000000..a551409 --- /dev/null +++ b/src/__tests__/unit/frollo-ingest.test.ts @@ -0,0 +1,189 @@ +import { describe, it, expect } from "vitest"; +import { ingestFrolloCsv, LEDGER_MATCH_DAYS, type SqlExecutor } from "@/lib/frollo-ingest"; + +/** + * Tests for the ledger-duplicate guard. + * + * This is the check whose absence caused the 2026-08-13 first import to write + * 422 duplicate rows out of 550 — every one of them a second copy of a payment + * the ledger already held from a statement. It passed 29 unit tests and a clean + * dry run at the time, because every one of those tests compared the CSV against + * itself. Nothing compared it against the database. + */ + +const HEADER = + "transaction_id,description,user_description,amount,currency,transaction_date,posted_date," + + "account_number,account_name,credit_debit,transaction_type,provider_name,merchant_name," + + "budget_category,category_name,user_tags,notes,included"; + +function line(o: { id: string; desc?: string; amount: string; date: string }): string { + return [ + o.id, + o.desc ?? "SOME MERCHANT", + "", + o.amount, + "AUD", + o.date, + "", + "xxx-xxx xxxx1910", + "Smart Access", + o.amount.startsWith("-") ? "debit" : "credit", + "payment", + "CommBank", + "Some Merchant", + "lifestyle", + "Groceries", + "", + "", + "true", + ].join(","); +} + +const csv = (...lines: string[]) => [HEADER, ...lines].join("\n"); + +/** + * Stands in for the database. The two queries are told apart by their text, + * which is crude but keeps the test free of a live connection — and the guard + * is pure logic over what comes back. + */ +function exec(prior: { transaction_date: string; amount: string; transaction_type?: string }[]): SqlExecutor { + return (async (sql: string) => { + if (sql.includes("WHERE source = $1")) return []; + if (sql.includes("superseded_by_id IS NULL")) return prior.map((p) => ({ transaction_type: "debit", ...p })); + return []; + }) as SqlExecutor; +} + +describe("ledger-duplicate guard", () => { + it("drops a row the ledger already holds on the same day", async () => { + const r = await ingestFrolloCsv( + csv(line({ id: "1", amount: "-42.50", date: "2026-03-10" })), + exec([{ transaction_date: "2026-03-10", amount: "42.50" }]) + ); + expect(r.ledgerDuplicates).toBe(1); + expect(r.toInsert).toBe(0); + }); + + it("drops a row dated within the match window", async () => { + // The feed dates a transaction when the provider posted it and a statement + // when the bank did; a weekend puts two days between them. + const r = await ingestFrolloCsv( + csv(line({ id: "1", amount: "-42.50", date: "2026-03-10" })), + exec([{ transaction_date: "2026-03-13", amount: "42.50" }]) + ); + expect(LEDGER_MATCH_DAYS).toBe(3); + expect(r.ledgerDuplicates).toBe(1); + expect(r.toInsert).toBe(0); + }); + + it("keeps a row beyond the match window", async () => { + const r = await ingestFrolloCsv( + csv(line({ id: "1", amount: "-42.50", date: "2026-03-10" })), + exec([{ transaction_date: "2026-03-14", amount: "42.50" }]) + ); + expect(r.ledgerDuplicates).toBe(0); + expect(r.toInsert).toBe(1); + }); + + it("keeps a row whose amount differs", async () => { + const r = await ingestFrolloCsv( + csv(line({ id: "1", amount: "-42.50", date: "2026-03-10" })), + exec([{ transaction_date: "2026-03-10", amount: "42.51" }]) + ); + expect(r.ledgerDuplicates).toBe(0); + expect(r.toInsert).toBe(1); + }); + + it("consumes each ledger row once, so a genuine repeat survives", async () => { + // Two real charges of the same amount in the same week, one of which the + // ledger already has. Matching without consuming would drop both and lose a + // transaction that never existed anywhere else. + const r = await ingestFrolloCsv( + csv( + line({ id: "1", desc: "COFFEE ONE", amount: "-5.00", date: "2026-03-10" }), + line({ id: "2", desc: "COFFEE TWO", amount: "-5.00", date: "2026-03-11" }) + ), + exec([{ transaction_date: "2026-03-10", amount: "5.00" }]) + ); + expect(r.ledgerDuplicates).toBe(1); + expect(r.toInsert).toBe(1); + }); + + it("matches when the driver returns Date objects, not strings", async () => { + // `pg` maps a Postgres DATE to a JS Date; Prisma and the CSV give strings. + // The first cut of this guard read one as the other, and because the failure + // is a silent NaN it reported 550 rows to insert against a ledger holding + // 422 of them. Both shapes are tested because both drivers are in use: the + // CLI runs on pg, the API route on Prisma. + const asDate = await ingestFrolloCsv( + csv(line({ id: "1", amount: "-42.50", date: "2026-03-10" })), + exec([{ transaction_date: new Date(2026, 2, 10) as unknown as string, amount: "42.50" }]) + ); + expect(asDate.ledgerDuplicates).toBe(1); + expect(asDate.toInsert).toBe(0); + }); + + it("never matches a credit against a debit", async () => { + // Internal transfers between the owner's own accounts put both legs in the + // feed: 2026-05-18 carries +3076.04 into ANZ and -3076.04 out of AMP. On + // amount alone the credit leg consumed the ledger's debit row and the real + // duplicate was written. 20 rows got in this way before direction was part + // of the key. + const r = await ingestFrolloCsv( + csv( + line({ id: "1", desc: "PAYMENT FROM SELF", amount: "3076.04", date: "2026-05-18" }), + line({ id: "2", desc: "Transfer to Self", amount: "-3076.04", date: "2026-05-18" }) + ), + exec([{ transaction_date: "2026-05-18", amount: "3076.04", transaction_type: "debit" }]) + ); + // The debit leg is the duplicate; the credit leg is a genuinely new row. + expect(r.ledgerDuplicates).toBe(1); + expect(r.toInsert).toBe(1); + }); + + it("treats a statement 'refund' as money in", async () => { + // The feed calls a reversed account fee a credit; the statement importer + // types it 'refund'. Classifying refund as an outflow left every ANZ + // servicing-fee reversal in the file as a duplicate. + const r = await ingestFrolloCsv( + csv(line({ id: "1", desc: "REVERSAL OF ACCOUNT SERVICING FEE", amount: "5.00", date: "2026-02-27" })), + exec([{ transaction_date: "2026-02-27", amount: "5.00", transaction_type: "refund" }]) + ); + expect(r.ledgerDuplicates).toBe(1); + expect(r.toInsert).toBe(0); + }); + + it("treats a statement 'fee' as money out", async () => { + const r = await ingestFrolloCsv( + csv(line({ id: "1", desc: "ACCOUNT SERVICING FEE", amount: "-5.00", date: "2026-03-31" })), + exec([{ transaction_date: "2026-03-31", amount: "5.00", transaction_type: "fee" }]) + ); + expect(r.ledgerDuplicates).toBe(1); + expect(r.toInsert).toBe(0); + }); + + it("keeps everything when the ledger is empty", async () => { + const r = await ingestFrolloCsv( + csv( + line({ id: "1", desc: "A", amount: "-5.00", date: "2026-03-10" }), + line({ id: "2", desc: "B", amount: "-6.00", date: "2026-03-11" }) + ), + exec([]) + ); + expect(r.ledgerDuplicates).toBe(0); + expect(r.toInsert).toBe(2); + }); + + it("reports duplicates rather than silently discarding them", async () => { + // The owner's standing requirement: a row that disappears must be counted, + // because a missing transaction can mean several things and one of them is + // worth a follow-up. + const r = await ingestFrolloCsv( + csv(line({ id: "1", amount: "-42.50", date: "2026-03-10" })), + exec([{ transaction_date: "2026-03-10", amount: "42.50" }]) + ); + expect(r).toHaveProperty("ledgerDuplicates"); + expect(r.inScope).toBe(1); + expect(r.ledgerDuplicates + r.toInsert).toBe(r.inScope); + }); +}); diff --git a/src/lib/frollo-ingest.ts b/src/lib/frollo-ingest.ts index ef27865..e8dff85 100644 --- a/src/lib/frollo-ingest.ts +++ b/src/lib/frollo-ingest.ts @@ -41,6 +41,46 @@ export const SOURCE = "frollo"; */ export const LARGE_BATCH = 200; +/** + * How far a Frollo row may sit from a ledger row and still be the same event. + * + * The feed dates a transaction when the provider posted it; a statement dates it + * when the bank did. Those differ by a day or two around weekends. Three days is + * wide enough to catch that and narrow enough that two genuinely different + * charges for the identical amount in the same week are rare — and when they do + * collide, the cost is one missing row in a feed whose whole job is provisional + * visibility, against a permanent double-count the other way. + */ +export const LEDGER_MATCH_DAYS = 3; + +/** + * A date column, as midnight UTC, from whatever the driver handed back. + * + * `pg` maps a Postgres DATE to a JS `Date` in local time, while Prisma and the + * CSV both yield `"YYYY-MM-DD"` strings. Reading one as the other is silent: + * `String(new Date()).slice(0, 10)` is `"Wed Mar 10"`, which parses to NaN, and + * a guard built on it skips every row it was meant to compare — the first dry + * run of this code reported 550 rows to insert and zero duplicates against a + * ledger holding 422 of them. + */ +/** + * Ledger transaction types that mean money arriving. + * + * The whole set in use is debit | credit | payment | fee | interest | refund. + * `refund` is the one that bites: the feed calls a reversed account fee a + * `credit` while the statement importer types it `refund`, so classifying it as + * an outflow left five real duplicates behind — every ANZ servicing-fee reversal + * in the file. Small money, but it is the same shape of mismatch that would + * matter on a large refund. + */ +const INFLOW_TYPES = new Set(["credit", "refund"]); + +function dayMs(v: unknown): number { + if (v instanceof Date) return Date.UTC(v.getFullYear(), v.getMonth(), v.getDate()); + const m = /^(\d{4})-(\d{2})-(\d{2})/.exec(String(v)); + return m ? Date.UTC(Number(m[1]), Number(m[2]) - 1, Number(m[3])) : NaN; +} + export interface IngestOptions { ownerId?: number; /** Write. Without it, everything is computed and nothing is inserted. */ @@ -60,6 +100,12 @@ export interface IngestReport { duplicatesDropped: number; suspectRepeats: { description: string; transactionDate: string; amount: number; idGap: number }[]; alreadyImported: number; + /** + * Rows dropped because the ledger already holds the same event from a + * statement. Counted and reported, never silent: a Frollo row vanishing can + * mean several things and one of them is worth following up. + */ + ledgerDuplicates: number; toInsert: number; inserted: number; /** Non-empty means an automatic run should stop and a human should look. */ @@ -147,7 +193,79 @@ export async function ingestFrolloCsv( [SOURCE] ); const seen = new Set(existing.map((r) => r.source_ref)); - const fresh = ledger.filter((r) => !seen.has(r.sourceRef)); + const notYetImported = ledger.filter((r) => !seen.has(r.sourceRef)); + + // Drop rows the ledger already holds from a statement. + // + // This guard is the whole lesson of the 2026-08-13 first import. The feed was + // scoped to "accounts that issue no monthly statement", but that was asserted, + // never tested against the ledger — and it was wrong for almost every account: + // 422 of the 550 rows imported (77%) had a statement twin within three days, + // $1,023,824.63 of movement counted twice. The HDR Global salary showed it + // most plainly, appearing as both A$15,518.53 (statement, converted) and + // US$10,782.00 (feed, native) for the same July payment. + // + // The check that would have caught it is one query and takes a second. The + // check that was run instead compared each row's date against the statement's + // min-max window, which for accounts whose statements span 182 to 460 days + // swallows a year and cannot distinguish covered from uncovered at all. + const dates = notYetImported.map((r) => dayMs(r.transactionDate)).filter(Number.isFinite); + let ledgerDuplicates = 0; + let fresh = notYetImported; + if (dates.length > 0) { + const pad = LEDGER_MATCH_DAYS * 86_400_000; + const from = new Date(Math.min(...dates) - pad).toISOString().slice(0, 10); + const to = new Date(Math.max(...dates) + pad).toISOString().slice(0, 10); + + // Superseded rows are excluded: one has already been replaced by the row + // that supersedes it, so matching against both would hide a genuine gap. + const priorRows = await exec<{ transaction_date: string; amount: string; transaction_type: string }>( + `SELECT transaction_date, amount, transaction_type + FROM transactions + WHERE (source IS NULL OR source <> $1) + AND superseded_by_id IS NULL + AND transaction_date BETWEEN $2::date AND $3::date`, + [SOURCE, from, to] + ); + + // Keyed on amount AND direction. + // + // Amount is the field both sides agree on exactly — descriptions do not + // survive the trip (the feed writes "HDR Global Services (Bermuda)" where + // the statement writes "Received money from HDR Global Services (Bermuda) + // with reference ...") and dates drift by a day or two. + // + // Direction has to be in the key because this ledger is full of internal + // transfers between the owner's own accounts, and both legs are in the feed: + // 2026-05-18 carries a +3076.04 credit into ANZ and a -3076.04 debit out of + // AMP. On amount alone the credit leg consumed the ledger's debit row and + // the genuine duplicate was written — 20 rows got in this way on the first + // corrected run. A credit is never the duplicate of a debit. + const key = (amount: number, inflow: boolean) => `${Math.abs(amount).toFixed(2)}:${inflow ? "in" : "out"}`; + const byAmount = new Map(); + for (const p of priorRows) { + const k = key(Number(p.amount), INFLOW_TYPES.has(p.transaction_type)); + const t = dayMs(p.transaction_date); + if (!Number.isFinite(t)) continue; + const list = byAmount.get(k); + if (list) list.push(t); + else byAmount.set(k, [t]); + } + + fresh = notYetImported.filter((r) => { + const candidates = byAmount.get(key(r.amount, r.transactionType === "credit")); + if (!candidates) return true; + const t = dayMs(r.transactionDate); + if (!Number.isFinite(t)) return true; + const hit = candidates.findIndex((c) => Math.abs(c - t) <= pad); + if (hit === -1) return true; + // Consume the match so two feed rows cannot both claim one ledger row — + // a real pair of identical charges must not collapse into one. + candidates.splice(hit, 1); + ledgerDuplicates += 1; + return false; + }); + } const base = { totalRows: rows.length, @@ -158,7 +276,8 @@ export async function ingestFrolloCsv( unknownAccounts: unknown, duplicatesDropped: dropped.length, suspectRepeats, - alreadyImported: ledger.length - fresh.length, + alreadyImported: ledger.length - notYetImported.length, + ledgerDuplicates, toInsert: fresh.length, }; const anomalies = findAnomalies(base, opts.largeBatch); diff --git a/src/lib/queries.ts b/src/lib/queries.ts index 8608cf3..619b294 100644 --- a/src/lib/queries.ts +++ b/src/lib/queries.ts @@ -733,23 +733,31 @@ export const needsCardMatch = (alias = "t") => `(${alias}.payment_method IS NULL OR ${alias}.payment_method NOT IN ('cash', 'credits'))`; /** - * Rows for which a statement line could still arrive. + * REMOVED 2026-08-13, and the reason is worth more than the code was. * - * A row imported from an account feed is that account's own ledger entry, not a - * receipt waiting to be matched against one. The Frollo importer deliberately - * covers only accounts whose statements are *not* imported — credit cards are - * excluded from it precisely because theirs are — so no statement line is coming - * for these, ever. Same shape as a credits-funded order, different reason. + * There used to be an `awaitsStatementLine()` predicate here excluding + * account-feed rows from the pending-reconciliation queue. Its premise — "the + * Frollo importer deliberately covers only accounts whose statements are *not* + * imported, so no statement line is coming for these, ever" — was asserted and + * never tested. It was false for almost every account: of the 550 rows the first + * import wrote, 422 already had a statement twin. * - * Without this, roughly 600 rows a year would sit in the pending-reconciliation - * queue forever and bury the receipts that genuinely need a decision. Scoped to - * feeds rather than to `source IS NOT NULL`, so a future source that *does* await - * a statement is not silently swept up by it. + * What makes this worth recording is *how* it was introduced. The queue jumped + * from 8 to 558 the moment the feed landed, and that jump was read as noise and + * filtered away. The queue was right. Those 550 rows genuinely were provisional + * entries awaiting their statement lines, and suppressing the count removed the + * only mechanism that would ever have collapsed them — so the duplicates became + * permanent instead of transient, and stayed invisible until a human noticed the + * same salary payment listed twice in two currencies. + * + * A feed row IS a row awaiting a statement line. It belongs in the queue. The + * volume problem is solved upstream, by not importing rows the ledger already + * holds (`LEDGER_MATCH_DAYS` in frollo-ingest.ts) — not downstream by hiding + * the ones that are. + * + * If a genuinely statement-less feed is ever added, give it a predicate of its + * own and prove the premise with a query first. */ -export const ACCOUNT_FEED_SOURCES = ["frollo"] as const; - -export const awaitsStatementLine = (alias = "t") => - `(${alias}.source IS NULL OR ${alias}.source NOT IN (${ACCOUNT_FEED_SOURCES.map((s) => `'${s}'`).join(", ")}))`; /** * Bank label for a transaction. A row with no statement was not imported from @@ -803,7 +811,6 @@ export async function getPendingReconciliations(ownerId: number): Promise