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>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
ff01640f60
commit
331ae11867
@@ -22,7 +22,7 @@ import { registerEndpoint } from '@/lib/api/v1/registry'
|
||||
import { withApiV1 } from '@/lib/api/v1/with-api-v1'
|
||||
import { v1ErrorResponse, v1ErrorResponseFromCode } from '@/lib/api/v1/errors'
|
||||
import { ingestTransactions } from '@/lib/transactions/ingest'
|
||||
import { contentDedupKey } from '@/lib/transactions/external-id'
|
||||
import { contentBucketKey, descriptionsBridge, normalizeImportedDescription } from '@/lib/transactions/external-id'
|
||||
import type { RawTransaction } from '@/types'
|
||||
|
||||
const RawTx = z.object({
|
||||
@@ -70,10 +70,10 @@ registerEndpoint({
|
||||
'Single ad-hoc transactions (use the dashboard). Documents/receipts (use the documents endpoint). Manually-created journal entries (Phase 4).',
|
||||
pitfalls: [
|
||||
'external_id is the primary dedup key — make it stable for the same physical transaction across reruns.',
|
||||
'Content-based dedup (date+amount) runs in addition: a CSV row that matches an already-booked transaction by date+amount is skipped even if external_id differs.',
|
||||
'Content-based dedup runs in addition: a row matching an already-booked transaction by date, amount AND description (prefix-containment, to survive PSD2 title enrichment) is skipped even if external_id differs.',
|
||||
'raw_insert_only=true skips ALL post-insert pipeline steps (matching, categorization). Use for viewer-only imports.',
|
||||
'Max 500 items per call. For larger imports, split into pages of 500.',
|
||||
'Dry-run runs both dedup checks (external_id AND content-based date+amount against booked rows), matching the live pipeline. Numbers should agree barring concurrent imports between preview and commit.',
|
||||
'Dry-run previews external_id + content dedup against BOOKED rows only; the live pipeline also dedups against unbooked bank-synced rows, so preview skips are a lower bound on the live skip count.',
|
||||
],
|
||||
example: {
|
||||
request: {
|
||||
@@ -153,25 +153,52 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
|
||||
|
||||
const { data: bookedInRange } = await ctx.supabase
|
||||
.from('transactions')
|
||||
.select('date, amount, description')
|
||||
.select('date, amount, original_description, description')
|
||||
.eq('company_id', ctx.companyId!)
|
||||
.not('journal_entry_id', 'is', null)
|
||||
.gte('date', dateFrom)
|
||||
.lte('date', dateTo)
|
||||
// Build the content-dedup key with the SAME helper the live pipeline uses
|
||||
// (lib/transactions/ingest.ts), so the preview's content-match decision
|
||||
// matches the eventual ingest exactly: öre-normalized amount (handles a
|
||||
// PostgREST numeric returned as a string) plus the description prefix.
|
||||
const bookedKeys = new Set(
|
||||
(bookedInRange ?? []).map((r) => {
|
||||
const row = r as { date: string; amount: number | string; description: string | null }
|
||||
return contentDedupKey(row.date, row.amount, row.description)
|
||||
}),
|
||||
)
|
||||
// Mirror the live pipeline's content-dedup bridge (lib/transactions/ingest.ts):
|
||||
// bucket booked rows by (date, öre) and match by description prefix-containment
|
||||
// (keyed off the immutable original_description), consumed with the SAME
|
||||
// longest-match + counting semantics — so a batch of N copies against M booked
|
||||
// twins previews M skips and N−M imports, not N. Scope note: like the live
|
||||
// pipeline's booked map this previews ONLY booked rows; the unbooked
|
||||
// enable_banking overlap check is not modelled here.
|
||||
const bookedBuckets = new Map<string, string[]>()
|
||||
for (const r of bookedInRange ?? []) {
|
||||
const row = r as { date: string; amount: number | string; original_description: string | null; description: string | null }
|
||||
const key = contentBucketKey(row.date, row.amount)
|
||||
const desc = normalizeImportedDescription(row.original_description ?? row.description).toLowerCase().trim()
|
||||
const bucket = bookedBuckets.get(key)
|
||||
if (bucket) bucket.push(desc)
|
||||
else bookedBuckets.set(key, [desc])
|
||||
}
|
||||
// Consume the longest bridging stored description (counting semantics) — the
|
||||
// same logic as ingest.ts consumeBridgingTwin, on this preview's mutable copy.
|
||||
const consumeBookedTwin = (date: string, amount: number, desc: string): boolean => {
|
||||
const descs = bookedBuckets.get(contentBucketKey(date, amount))
|
||||
if (!descs || descs.length === 0) return false
|
||||
let bestIdx = -1
|
||||
let bestLen = -1
|
||||
for (let i = 0; i < descs.length; i++) {
|
||||
if (descriptionsBridge(desc, descs[i]) && descs[i].length > bestLen) {
|
||||
bestIdx = i
|
||||
bestLen = descs[i].length
|
||||
}
|
||||
}
|
||||
if (bestIdx === -1) return false
|
||||
descs.splice(bestIdx, 1)
|
||||
return true
|
||||
}
|
||||
|
||||
const previewRows = body.transactions.map((tx) => {
|
||||
const extIdHit = knownExtIds.has(tx.external_id)
|
||||
const contentHit = bookedKeys.has(contentDedupKey(tx.date, tx.amount, tx.description))
|
||||
// external_id precedence mirrors the live pipeline: a row caught by
|
||||
// external_id must NOT consume a booked twin (so it stays available for a
|
||||
// genuine content-only duplicate later in the batch).
|
||||
const contentHit =
|
||||
!extIdHit && consumeBookedTwin(tx.date, tx.amount, normalizeImportedDescription(tx.description))
|
||||
const wouldSkip = extIdHit || contentHit
|
||||
const reason = extIdHit
|
||||
? 'external_id_match'
|
||||
|
||||
Reference in New Issue
Block a user