fix(import): stop a generic bank signature from claiming, and misreading, a richer file

Review finding on #330, two defects with one root cause.

Desjardins' fingerprint is date/description/montant/solde — four labels any
Canadian bank could emit — and MIN_SIGNATURE_LABELS = 4 did not deliver the
property it promised, because matching was by SUBSET. Any
Date;Description;Montant;Solde file was announced 'Format Desjardins reconnu'.
A variant built only from generic labels must now describe the header exactly;
one carrying a discriminating label (chequenumber, categorie, memo) keeps
subset matching, so extra columns stay fine once something identifies the bank.

Worse, a matched signature's single amount column short-circuited the
sparse-complementary scan instead of being arbitrated against it. A
Date;Description;Debit;Credit;Montant;Solde file reads correctly as debit/credit
before #330 and became one unsigned column after, importing every deposit as an
expense. The scan now runs first; a signature's amount column only wins when the
pair contains it — which is what RBC's genuine Cheque Number / CAD$ case needs,
and it still passes.

Refs #330

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/utils/bankSignatures.test.ts

@@ -222,21 +222,90 @@ describe("what the signatures actually buy (#330)", () => {
     expect(configOf(readCsvFixture("bank-rbc")).amountMode).toBe("single");
   });
 
-  it("keeps Tangerine's direction column out of the description", () => {
+  it("keeps Tangerine's direction column out of the description, signature or not", () => {
     // `Transaction` is a description keyword of the generic dictionary, and
     // Tangerine's `Transaction` column holds DEBIT / CREDIT. Read as the
     // description, every transaction of the file is labelled `DEBIT`.
+    //
+    // This used to be rescued by the Tangerine signature alone, leaving any
+    // unrecognised file with a `Transaction` column broken. The cardinality
+    // veto in `detectDescriptionColumn` fixes the cause instead: a labelled
+    // column that behaves like an enum is not the description, whether or not a
+    // bank was identified.
     const anonymised = withHeader(
       readCsvFixture("bank-tangerine"),
       "Date,Transaction,Nom,Note,Montant"
     );
     expect(bankOf(anonymised)).toBeNull();
-    expect(configOf(anonymised).columnMapping.description).toBe(1);
+    expect(configOf(anonymised).columnMapping.description).toBe(2);
 
     expect(configOf(readCsvFixture("bank-tangerine")).columnMapping.description).toBe(
       2
     );
   });
+
+  it("does not let a generic variant claim a richer header", () => {
+    // Review finding on #340: Desjardins' fingerprint is four labels any
+    // Canadian bank could emit, so matching them as a SUBSET announced
+    // "Format Desjardins reconnu" over files that have nothing to do with it.
+    // A variant made only of generic labels now has to describe the header
+    // exactly.
+    const richer =
+      "Date;Description;Débit;Crédit;Montant;Solde\n" +
+      "05/01/2025;EPICERIE;84,32;;-84,32;1000,00\n" +
+      "15/01/2025;DEPOT PAIE;;1250,00;1250,00;2250,00\n" +
+      "20/01/2025;LOYER;900,00;;-900,00;1350,00\n" +
+      "25/01/2025;REMBOURSEMENT;;45,00;45,00;1395,00\n";
+    expect(bankOf(richer)).toBeNull();
+
+    // The exact header still is Desjardins.
+    const exact =
+      "Date;Description;Montant;Solde\n" +
+      "05/01/2025;EPICERIE;-84,32;1000,00\n" +
+      "15/01/2025;DEPOT PAIE;1250,00;2250,00\n";
+    expect(bankOf(exact)).toBe("desjardins");
+  });
+
+  it("does not let a signature's amount column displace a pair it is not in", () => {
+    // The measured regression: with the false Desjardins match above, the
+    // signature's `Montant` short-circuited the sparse-complementary scan, so a
+    // file that reads correctly as debit/credit became one unsigned column and
+    // every deposit imported as an expense. The scan now runs first and the
+    // signature only wins when the pair contains its column — which is what
+    // RBC's genuine `Cheque Number` / `CAD$` case needs.
+    const richer =
+      "Date;Description;Débit;Crédit;Montant;Solde\n" +
+      "05/01/2025;EPICERIE;84,32;;-84,32;1000,00\n" +
+      "15/01/2025;DEPOT PAIE;;1250,00;1250,00;2250,00\n" +
+      "20/01/2025;LOYER;900,00;;-900,00;1350,00\n" +
+      "25/01/2025;REMBOURSEMENT;;45,00;45,00;1395,00\n";
+    const config = configOf(richer);
+    expect(config.amountMode).toBe("debit_credit");
+    expect(config.columnMapping.debitAmount).toBe(2);
+    expect(config.columnMapping.creditAmount).toBe(3);
+
+    // RBC keeps its override: there the declared amount column IS in the pair.
+    expect(configOf(readCsvFixture("bank-rbc")).amountMode).toBe("single");
+  });
+
+  it("holds even for a legitimately recognised bank whose file grew columns", () => {
+    // The guard above is only reachable through a signature that really matches,
+    // so it needs a discriminating one. Tangerine is identified by `memo`, so it
+    // still matches as a subset when the export gains Débit/Crédit columns — and
+    // then its declared `Amount` must not displace that genuine pair either.
+    const grown =
+      "Date,Transaction,Name,Memo,Amount,Débit,Crédit\n" +
+      "05/01/2025,DEBIT,EPICERIE METRO,,-84.32,84.32,\n" +
+      "15/01/2025,CREDIT,DEPOT PAIE,Paie,1250.00,,1250.00\n" +
+      "20/01/2025,DEBIT,LOYER,,-900.00,900.00,\n" +
+      "25/01/2025,CREDIT,REMBOURSEMENT,,45.00,,45.00\n";
+    expect(bankOf(grown)).toBe("tangerine");
+
+    const config = configOf(grown);
+    expect(config.amountMode).toBe("debit_credit");
+    expect(config.columnMapping.debitAmount).toBe(5);
+    expect(config.columnMapping.creditAmount).toBe(6);
+  });
 });
 
 describe("an unknown file falls back to the generic dictionary (#330)", () => {

src/utils/bankSignatures.ts

@@ -85,6 +85,30 @@ export interface BankSignature {
  */
 export const MIN_SIGNATURE_LABELS = 4;
 
+/**
+ * Labels any Canadian bank could emit. A variant built only from these
+ * identifies no bank in particular, so the count alone does not deliver the
+ * property `MIN_SIGNATURE_LABELS` promises — such a variant has to match the
+ * header exactly (see `matchBankSignature`).
+ */
+const GENERIC_LABELS: ReadonlySet<string> = new Set([
+  "date",
+  "description",
+  "libelle",
+  "detail",
+  "transaction",
+  "montant",
+  "amount",
+  "solde",
+  "balance",
+  "debit",
+  "credit",
+  "retrait",
+  "depot",
+  "withdrawal",
+  "deposit",
+]);
+
 /**
  * The table. Written from the banks' documented export layouts; none of it has
  * been verified against a real statement (see the file header). Adding a bank
@@ -288,6 +312,20 @@ export function matchBankSignature(
 
     for (const variant of signature.variants) {
       if (!variant.labels.every((label) => index.has(label))) continue;
+      // A variant made only of generic labels must describe the header EXACTLY,
+      // not merely be contained in it. Desjardins is
+      // `date;description;montant;solde` — four labels any Canadian bank could
+      // emit — so accepting them as a SUBSET let a
+      // `Date;Description;Débit;Crédit;Montant;Solde` file claim to be
+      // Desjardins. A variant carrying a discriminating label (`chequenumber`,
+      // `categorie`, `memo`, …) keeps subset matching: extra columns are fine
+      // once something actually identifies the bank.
+      if (
+        variant.labels.every((label) => GENERIC_LABELS.has(label)) &&
+        index.size !== variant.labels.length
+      ) {
+        continue;
+      }
       return { signature, variant, roles: rolesOf(variant, index) };
     }
   }

src/utils/csvAutoDetect.ts

@@ -707,6 +707,37 @@ function detectBalanceColumns(
  * labelling its amount column `Détail du montant` therefore cannot end up with
  * its amounts in the description.
  */
+/**
+ * Does this column behave like an enumeration rather than free text?
+ *
+ * A label alone is not enough to pick the description: Tangerine exports
+ * `Date,Transaction,Name,Memo,Amount`, where `Transaction` holds DEBIT/CREDIT
+ * and `Name` holds the merchant. Honouring the label there moves the merchant
+ * out of the description and kills keyword categorisation.
+ *
+ * Cardinality separates the two — a description repeats almost nothing, an enum
+ * repeats almost everything. Average length does NOT: `Note` and `Libellé` are
+ * both short, and vetoing on length would reject legitimate columns.
+ */
+function looksLikeEnumColumn(rows: string[][], col: number): boolean {
+  const distinct = new Set<string>();
+  let filled = 0;
+
+  for (const row of rows) {
+    const cell = row[col]?.trim();
+    if (!cell) continue;
+    filled++;
+    distinct.add(cell.toLowerCase());
+  }
+
+  // A labelled but entirely empty column is never the description either.
+  if (filled === 0) return true;
+  // Too few rows to read anything into the cardinality.
+  if (filled < 4) return false;
+
+  return distinct.size <= Math.max(2, Math.floor(filled / 4));
+}
+
 function detectDescriptionColumn(
   rows: string[][],
   colCount: number,
@@ -719,7 +750,8 @@ function detectDescriptionColumn(
     preferred !== null &&
     preferred < colCount &&
     preferred !== dateCol &&
-    !numericCols.has(preferred)
+    !numericCols.has(preferred) &&
+    !looksLikeEnumColumn(rows, preferred)
   ) {
     return preferred;
   }
@@ -779,6 +811,26 @@ type AmountModeResult = SingleAmountResult | DebitCreditResult;
  * nothing numeric is dropped here and the generic path decides, exactly like a
  * mismatched label.
  */
+/**
+ * The first sparse-complementary pair among the candidates, in column order, or
+ * null. Extracted from `detectAmountMode` so a bank signature can be arbitrated
+ * AGAINST the pair the shape scan would have found, instead of short-circuiting
+ * a scan that never ran.
+ */
+function findSparseComplementaryPair(
+  rows: string[][],
+  amountCandidates: number[]
+): [number, number] | null {
+  for (let a = 0; a < amountCandidates.length; a++) {
+    for (let b = a + 1; b < amountCandidates.length; b++) {
+      const colA = amountCandidates[a];
+      const colB = amountCandidates[b];
+      if (isSparseComplementary(rows, colA, colB)) return [colA, colB];
+    }
+  }
+  return null;
+}
+
 function detectAmountMode(
   rows: string[][],
   amountCandidates: number[],
@@ -787,6 +839,8 @@ function detectAmountMode(
 ): AmountModeResult | null {
   if (amountCandidates.length === 0) return null;
 
+  const pair = findSparseComplementaryPair(rows, amountCandidates);
+
   if (signature) {
     const { debit, credit, amount } = signature.roles;
     if (
@@ -797,7 +851,17 @@ function detectAmountMode(
     ) {
       return { mode: "debit_credit", debitCol: debit, creditCol: credit };
     }
-    if (amount !== null && amountCandidates.includes(amount)) {
+    // A signature's SINGLE amount column may not silently displace a genuine
+    // debit/credit pair it has no part in. RBC needs the override — its
+    // near-empty `Cheque Number` really is sparse-complementary with `CAD$` —
+    // but there the pair contains the declared amount column. On a
+    // `Date;Description;Débit;Crédit;Montant;Solde` file the pair does not, and
+    // taking `Montant` unsigned imported every deposit as an expense.
+    if (
+      amount !== null &&
+      amountCandidates.includes(amount) &&
+      (!pair || pair.includes(amount))
+    ) {
       return detectSingleAmount(rows, amount);
     }
   }
@@ -806,16 +870,8 @@ function detectAmountMode(
     return detectSingleAmount(rows, amountCandidates[0]);
   }
 
-  // Check for sparse-complementary pair (debit/credit pattern)
-  for (let a = 0; a < amountCandidates.length; a++) {
-    for (let b = a + 1; b < amountCandidates.length; b++) {
-      const colA = amountCandidates[a];
-      const colB = amountCandidates[b];
-
-      if (isSparseComplementary(rows, colA, colB)) {
-        return orderDebitCredit(colA, colB, lexical);
-      }
-    }
+  if (pair) {
+    return orderDebitCredit(pair[0], pair[1], lexical);
   }
 
   // No complementary pair found — the labelled amount column if there is one,

src/utils/importFormat.test.ts

@@ -416,6 +416,35 @@ describe("mapRow — debit/credit is a subtraction, not a nullity test (#325)",
     ).toBe(60);
   });
 
+  it("errors on an unreadable cell even when its sibling parses", () => {
+    // The review finding on #336: testing `isNaN(debit) && isNaN(credit)` only
+    // caught the case where BOTH sides fell. But the unused column carries
+    // `0,00` in exactly the files this rule exists to fix, so an unreadable
+    // debit beside a `0,00` credit computed 0 − 0 = 0 and imported silently —
+    // the very bug, one cell over.
+    expect(
+      outcome(mapRow(["05/01/2025", "EPICERIE", "n/a", "0,00"], DC_FORMAT))
+    ).toBe(ROW_ERROR_KEYS.invalidAmount);
+    expect(
+      outcome(mapRow(["05/01/2025", "EPICERIE", "84,32 CAD", "0,00"], DC_FORMAT))
+    ).toBe(ROW_ERROR_KEYS.invalidAmount);
+    // Symmetric: an unreadable credit beside a readable debit.
+    expect(
+      outcome(mapRow(["15/01/2025", "PAIE", "0,00", "1 250 $ CAD"], DC_FORMAT))
+    ).toBe(ROW_ERROR_KEYS.invalidAmount);
+  });
+
+  it("still treats an EMPTY cell as absent, not as unreadable", () => {
+    // The distinction the fix rests on: empty means "this column does not apply
+    // to this row", which is the normal shape of a debit/credit file.
+    expect(
+      outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", ""], DC_FORMAT))
+    ).toBe(-84.32);
+    expect(
+      outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", "   "], DC_FORMAT))
+    ).toBe(-84.32);
+  });
+
   it("errors when NEITHER column is readable, instead of importing 0", () => {
     expect(outcome(mapRow(["05/01/2025", "X", "", ""], DC_FORMAT))).toBe(
       ROW_ERROR_KEYS.invalidAmount

src/utils/importFormat.ts

@@ -277,8 +277,12 @@ export interface MapRowOptions {
  * convention (both columns hold positive numbers) even when an export negates
  * its debits.
  *
- * A row whose amount is unreadable in BOTH columns is an error, never a 0: an
- * amount nobody could read must not enter the ledger as a free transaction.
+ * A mapped cell that is NOT EMPTY but does not parse is an error, whatever its
+ * sibling holds. Testing `isNaN(debit) && isNaN(credit)` was not enough: the
+ * unused column carries `0,00` in exactly the files this rule exists to fix, so
+ * an unreadable debit beside a `0,00` credit parsed as 0 − 0 = 0 and imported
+ * silently — the very bug, one cell over. An EMPTY cell is different: it means
+ * the column does not apply to this row, and contributes zero.
  */
 export function mapRow(
   raw: string[],
@@ -304,6 +308,22 @@ export function mapRow(
       decimalSeparator: options.decimalSeparators?.get(col),
     });
 
+  /**
+   * Read one side of a debit/credit pair. `present` separates "the column does
+   * not apply to this row" (empty cell, contributes zero) from "the column says
+   * something we cannot read" (an error) — a distinction `isNaN` alone cannot
+   * make once the other side parses.
+   */
+  const readSide = (
+    col: number | undefined
+  ): { present: boolean; readable: boolean; value: number } => {
+    if (col === undefined) return { present: false, readable: true, value: 0 };
+    const text = raw[col]?.trim() ?? "";
+    if (!text) return { present: false, readable: true, value: 0 };
+    const value = readAmount(col);
+    return { present: true, readable: !isNaN(value), value };
+  };
+
   // 1. Configuration. An unmapped amount column used to fall back to `?? 0`,
   //    reading column 0 — usually the date — for every row of the file. That is
   //    a format error affecting the whole import, so it is reported before any
@@ -325,14 +345,20 @@ export function mapRow(
   // 3. Amount.
   let amount: number;
   if (format.amountMode === "debit_credit") {
-    const debit = debitMapped ? readAmount(mapping.debitAmount!) : NaN;
-    const credit = creditMapped ? readAmount(mapping.creditAmount!) : NaN;
-    if (isNaN(debit) && isNaN(credit)) {
+    const debit = readSide(debitMapped ? mapping.debitAmount : undefined);
+    const credit = readSide(creditMapped ? mapping.creditAmount : undefined);
+    // A cell that holds something we cannot read fails the row even when its
+    // sibling parses — otherwise the `0,00` filler silently answers for it.
+    if (!debit.readable || !credit.readable) {
+      return fail(ROW_ERROR_KEYS.invalidAmount);
+    }
+    // Both empty: the row states no amount at all.
+    if (!debit.present && !credit.present) {
       return fail(ROW_ERROR_KEYS.invalidAmount);
     }
     amount =
-      (isNaN(credit) ? 0 : Math.abs(credit)) -
-      (isNaN(debit) ? 0 : Math.abs(debit));
+      (credit.present ? Math.abs(credit.value) : 0) -
+      (debit.present ? Math.abs(debit.value) : 0);
   } else {
     amount = readAmount(mapping.amount!);
     if (isNaN(amount)) return fail(ROW_ERROR_KEYS.invalidAmount);