Files
accounted/lib/transactions/external-id.ts
T
Jakob WennbergandClaude Opus 4.8 331ae11867 fix(transactions): stop Enable Banking re-sync from re-importing duplicates (#630)
* fix(transactions): make content-dedup bridge resilient to PSD2 description drift

Enable Banking re-syncs were re-importing every overlapping transaction as a
duplicate. Two changes in the June 1 deploy combined to defeat both dedup layers
at once: the external_id format changed (old rows' stored ids no longer match the
new scheme, so the exact-match layer misses) AND PSD2 descriptions were enriched
("TIC" -> "TIC  BG 0000005786439 Bg-bet. via internet"), so the content-dedup
bridge — which compared a fixed 24-char description prefix for equality — also
missed. Result: a full re-import (observed: 53 of 54 "new" rows were dupes).

The external_id format has changed several times historically and 7,120 of 9,393
old rows have no reconstructable canonical id, so a backfill is not viable; the
content bridge is the mechanism meant to survive id-scheme changes. Harden it:

- Split the bridge into bucketing (date, öre) and matching (description), and
  match by prefix-containment instead of fixed-prefix equality. PSD2 enrichment
  is prefix-preserving, so the enriched re-import bridges its stored original,
  while genuinely-distinct same-(date,amount) rows (distinct descriptions) are
  kept apart. Consumed with counting semantics + longest-match, so N stored twins
  dedup exactly N incoming.
- Replace contentDedupKey with contentBucketKey + descriptionsBridge; update the
  live pipeline (ingest.ts) and the v1 dry-run preview to the same logic.
- Freeze the external_id format with a regression test + a header warning: any
  future format change must ship a coordinated backfill.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test/refactor: address PR review — sanitize fixtures, mirror preview counting

- Replace real customer names ("Carl Bennet AB", "Brorsan AB") and prod-derived
  reference strings in tests with clearly fictional stand-ins (compliance A.8.33).
- v1 dry-run preview: use the same longest-match + counting/consume semantics as
  the live pipeline so a batch of N copies against M booked twins previews M skips,
  not N (greptile P2). Update stale pitfall docs: content dedup is now
  date+amount+description (prefix-containment), and the preview is booked-only so
  its skip count is a lower bound on the live skip count.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(transactions): scope content-dedup bridge by cash account

The content-dedup bridge buckets by (date, öre) company-wide, with no account
scope — while bank reconciliation IS account-scoped (cash_account_id). For a
company with multiple bank accounts, a transaction on account A could therefore
deduplicate a genuinely-different transaction on account B that shares the same
date, amount, and a prefix-bridging description (round-number fees/transfers are
the realistic trigger), dropping a real row before it reaches reconciliation.

Layer 1 (external_id) is already account-safe because the account IBAN is
embedded in the id; only the fuzzy content bridge was account-blind.

Add an account guard: store cash_account_id alongside each bucket entry and only
bridge when BOTH the incoming batch and the stored entry have a known
cash_account_id that matches. A null on either side falls back to bridge-allowed,
so single-account companies and legacy (un-backfilled) rows are unchanged — and
CSV-vs-PSD2 dedup for the same account still works. Affects the 11 multi-account
companies; everyone else is behaviourally identical.

Note: external_id format heterogeneity (old entry_reference / old date+amount /
new öre+index schemes) is harmless downstream — nothing parses the id; it is an
opaque exact-match dedup key and a display string. Reconciliation, invoice/
supplier/payment matching, and reporting all key off real transaction columns,
never external_id.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* harden(transactions): blank description never wildcards a described row

PR review (OWASP V8.2.1 + Swedish compliance) flagged that descriptionsBridge
returned true whenever either side was empty, so a blank stored/incoming title
could wildcard-match any same-(date,öre) transaction and silently consume a real
one. Every live caller normalizes blanks to FALLBACK_DESCRIPTION upstream, so the
branch was unreachable in production — but make the function safe in isolation:
a blank now bridges only another blank (date+öre identity), never a described row.
No live behaviour change; removes the footgun.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-02 15:59:10 +02:00

167 lines
8.4 KiB
TypeScript

/**
* Shared helpers for deriving stable bank-transaction `external_id`s and for
* normalizing monetary amounts used in dedup keys.
*
* Why this exists
* ---------------
* The transactions table is deduplicated on `(company_id, external_id)` (a
* partial unique index, see migration 20260330130000). The dedup is therefore
* only as good as the stability of `external_id` across re-syncs.
*
* For Enable Banking (PSD2 / Berlin Group) the previous scheme keyed
* `external_id` off the bank's `entry_reference` / `transaction_id`
* (`eb_{account}_{tx.id}`). Many Swedish ASPSPs do NOT return those fields
* stably across requests — a later "synka nu" can return the same underlying
* transaction with a different id, which produced a *new* `external_id` and
* therefore a duplicate row (including for transactions the user had already
* booked). See `buildStableExternalIds` for the content-derived replacement.
*/
/**
* Normalize a monetary amount to integer öre (hundredths) for stable,
* representation-agnostic comparison.
*
* PostgREST may return a `numeric` column as a JS number OR as a string
* (preserving precision), so `1234.5` and the string `"1234.50"` can describe
* the same amount. Interpolating either directly into a dedup key yields
* different strings (`"1234.5"` vs `"1234.50"`), silently breaking content
* dedup. Rounding to integer öre collapses both to `123450`.
*
* Uses the project-standard `Math.round(x * 100)` (never `toFixed`).
*/
export function amountToOre(amount: number | string): number {
return Math.round(Number(amount) * 100)
}
/**
* Swedish-first placeholder for transactions a bank/import source gives no
* usable title. Centralized so every import path and the tests agree.
*/
export const FALLBACK_DESCRIPTION = 'Okänd transaktion'
/**
* Normalize an imported transaction title for storage and display.
*
* Maps both an empty/whitespace title AND the legacy English 'Unknown'
* sentinel — still emitted by the bank-file format parsers and as the Enable
* Banking converter's last resort — to a Swedish-first neutral. Applied once at
* the ingest boundary so every source (PSD2 sync + CSV/CAMT import) inherits
* it; the bank's verbatim text is preserved separately in
* `transactions.original_description`. Match on the exact 'unknown' sentinel
* (case-insensitive) so a real description that merely contains the word is
* never clobbered.
*/
export function normalizeImportedDescription(raw: string | null | undefined): string {
const trimmed = (raw ?? '').trim()
if (!trimmed || trimmed.toLowerCase() === 'unknown') return FALLBACK_DESCRIPTION
return trimmed
}
/**
* Build stable, collision-safe `external_id`s for a batch of bank transactions
* whose provider does not supply a reliable stable id (e.g. Enable Banking).
*
* The id is derived from content — `{prefix}_{accountScope}_{date}_{öre}_{n}`
* — where `n` is an occurrence index that disambiguates genuinely identical
* transactions (same account, date and amount) within the batch.
*
* ⚠️ THE FORMAT STRING IS A STORED KEY. It is persisted to
* `transactions.external_id` and dedup compares incoming ids against the stored
* ones byte-for-byte. Changing this template silently orphans every prior row
* (its stored id no longer matches the new scheme) and re-imports them all on
* the next sync — this is exactly what happened in the June 2026 fleet-wide
* incident. Any format change MUST ship a coordinated backfill of existing rows
* and is locked by a frozen-format test (see `external-id.test.ts`).
*
* Properties this guarantees:
* - **Re-sync dedupe**: the same set of transactions produces the same *set*
* of ids regardless of the order the ASPSP returns them in, so a repeat sync
* collides with the existing rows on `(company_id, external_id)` and is
* skipped — even after the user has booked them. (The id *set* is what the
* unique index enforces; which physical row maps to `..._0` vs `..._1` need
* not be stable, only the set.)
* - **No false dedupe**: two legitimately distinct transactions that share a
* date and amount get different ids (`..._0`, `..._1`) and are both kept.
* This is the safeguard the bank-file importer already relies on via its
* `rowIndex` component (see `lib/import/bank-file/parser.ts`).
*
* Why description is NOT an input here (but IS in the content bridge): the
* `external_id` must be a *stable unique key*, so it cannot depend on a field
* that drifts — PSD2 enriches/reorders descriptions between a transaction's
* pending and booked states. The occurrence index gives uniqueness without
* that fragility. The content bridge (`contentBucketKey` + `descriptionsBridge`)
* has the opposite job — it is a best-effort *bridge* that must avoid dropping
* real transactions — so it keeps the description (see those for the trade-off).
*
* @param prefix Source tag, e.g. `'eb'` for Enable Banking.
* @param accountScope Stable per-account scope (prefer IBAN, fall back to the
* provider account uid). Keeps ids unique across accounts.
* Callers should pass a whitespace/case-normalized IBAN so
* formatting variants ("SE45 5000…" vs "SE455000…") don't
* produce different ids for the same account.
* @param txns Batch in provider order; each needs `date` + `amount`.
*/
export function buildStableExternalIds(
prefix: string,
accountScope: string,
txns: Array<{ date: string; amount: number | string }>
): string[] {
const occurrences = new Map<string, number>()
return txns.map((tx) => {
const fingerprint = `${tx.date}_${amountToOre(tx.amount)}`
const n = occurrences.get(fingerprint) ?? 0
occurrences.set(fingerprint, n + 1)
return `${prefix}_${accountScope}_${fingerprint}_${n}`
})
}
/**
* Bucket key for the content-dedup bridge: `{date}|{öre}` — deliberately NO
* description. Transactions that share a date and amount fall into the same
* bucket; `descriptionsBridge` then decides, per pair, whether two rows in that
* bucket are the same transaction. Splitting bucketing (date+öre) from matching
* (description) is what lets the bridge survive description drift while still
* keeping genuinely-distinct same-(date,amount) transactions apart.
*
* öre via `amountToOre` so a JS number (`-250`) and a PostgREST numeric string
* (`"-250.00"`) collapse to the same bucket, otherwise dedup silently misses.
*/
export function contentBucketKey(date: string, amount: number | string): string {
return `${date}|${amountToOre(amount)}`
}
/**
* Decide whether two normalized descriptions (same date+öre bucket) describe the
* same underlying transaction — the matching half of the content-dedup bridge.
*
* Returns true when either description is a prefix of the other. PSD2 enrichment
* is **prefix-preserving**: the same transaction's title grows between syncs
* ("TIC" → "TIC BG 0000005786439 Bg-bet. via internet", "UTBETALNING" →
* "UTBETALNING Insättning"), so prefix-containment bridges the two where a
* fixed-length prefix *equality* check (the pre-June-2026 scheme) missed and
* re-imported. A blank description carries no signal, so it never bridges a
* *described* row — otherwise an empty title would wildcard-match any
* same-(date,öre) transaction and could silently consume a real one; only two
* blanks bridge each other (date+öre identity). In practice every caller
* normalizes blanks to FALLBACK_DESCRIPTION upstream (see
* normalizeImportedDescription), so the blank path is defense-in-depth.
* Genuinely distinct descriptions ("Coffee" vs "Lunch", or two different
* reference codes that share a date and amount) are NOT prefixes of one another
* and never bridge, so two real same-(date,amount) transactions are kept apart.
*
* This is a *best-effort* signal, consumed with COUNTING semantics in the ingest
* pipeline (N existing matches consume N incoming): its job is to skip re-imports
* WITHOUT ever dropping a real transaction. The asymmetry favours keeping a
* visible, user-deletable duplicate over silently losing a row.
*/
export function descriptionsBridge(
a: string | null | undefined,
b: string | null | undefined
): boolean {
const x = (a ?? '').toLowerCase().trim()
const y = (b ?? '').toLowerCase().trim()
// A blank never wildcards a described row; only two blanks bridge each other.
if (x === '' || y === '') return x === y
return x.startsWith(y) || y.startsWith(x)
}