maximus/Simpl-Resultat
1
0
Fork 0
Code Issues 5 Pull requests Projects Releases 29 Packages Wiki Activity Actions

fix(import): read amounts with an anchored parser and a real debit/credit rule #336

Closed
maximus wants to merge 1 commit from issue-325-amount-parsing into issue-324-persist-import-format
pull from: issue-325-amount-parsing
merge into: maximus:issue-324-persist-import-format
Conversation 3 Commits 1 Files changed 11 +858 -251
11 changed files with 858 additions and 251 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 7b6f063094 - Show all commits

7
src/components/import/FilePreviewTable.tsx
View file

@ -1,6 +1,7 @@
import { useTranslation } from "react-i18next";
import { AlertCircle } from "lucide-react";
import type { ParsedRow } from "../../shared/types";
import { isRowErrorKey } from "../../utils/importFormat";
interface FilePreviewTableProps {
rows: ParsedRow[];
@ -21,6 +22,10 @@ export default function FilePreviewTable({
const errorCount = rows.filter((r) => r.error).length;
// Row errors are i18n keys (#325); anything else reaches us verbatim.
const errorText = (error: string) =>
isRowErrorKey(error) ? t(error) : error;
return (
<div>
<div className="flex items-center justify-between mb-4">
@ -77,7 +82,7 @@ export default function FilePreviewTable({
<td className="px-3 py-2">
{row.parsed?.date || (
<span className="text-[var(--negative)] text-xs">
{row.error || "—"}
{row.error ? errorText(row.error) : "—"}
</span>
)}
</td>

7
src/components/import/ImportReportPanel.tsx
View file

@ -7,6 +7,7 @@ import {
FileText,
} from "lucide-react";
import type { ImportReport } from "../../shared/types";
import { isRowErrorKey } from "../../utils/importFormat";
interface ImportReportPanelProps {
report: ImportReport;
@ -104,7 +105,11 @@ export default function ImportReportPanel({
{report.errors.map((err, i) => (
<tr key={i}>
<td className="px-3 py-2">{err.rowIndex + 1}</td>
<td className="px-3 py-2 text-[var(--negative)]">{err.message}</td>
<td className="px-3 py-2 text-[var(--negative)]">
{/* Row errors are i18n keys (#325); an exception message
that reached this list is NOT one and stays verbatim. */}
{isRowErrorKey(err.message) ? t(err.message) : err.message}
</td>
</tr>
))}
</tbody>

74
src/hooks/useImportWizard.ts
View file

@ -39,16 +39,17 @@ import {
updateTemplate,
deleteTemplate as deleteTemplateService,
} from "../services/importConfigTemplateService";
import { parseDate } from "../utils/dateParser";
import { parseFrenchAmount } from "../utils/amountParser";
import {
preprocessQuotedCSV,
autoDetectConfig as runAutoDetect,
} from "../utils/csvAutoDetect";
import {
detectAmountSeparators,
formatFromRow,
formatToRow,
ImportFormatError,
mapRow,
ROW_ERROR_KEYS,
} from "../utils/importFormat";
/** Error text for the banner: an i18n key when we have one, the raw message otherwise. */
@ -510,66 +511,32 @@ export function useImportWizard() {
headers = firstDataRow.map((_, i) => `Col ${i}`);
}
// Data rows of THIS file, kept aside so the decimal separator of each
// amount column is arbitrated over the whole column before any row is
// read. `1.234` alone is ambiguous; the column is not.
const dataRows: string[][] = [];
for (let i = startIdx; i < data.length; i++) {
const raw = data[i];
if (raw.length <= 1 && raw[0]?.trim() === "") continue;
dataRows.push(raw);
}
const decimalSeparators = detectAmountSeparators(dataRows, config);
for (const raw of dataRows) {
try {
const date = parseDate(
raw[config.columnMapping.date]?.trim() || "",
config.dateFormat
allRows.push(
mapRow(raw, config, {
rowIndex: allRows.length,
sourceFilename: file.filename,
decimalSeparators,
})
);
const description =
raw[config.columnMapping.description]?.trim() || "";
let amount: number;
if (config.amountMode === "debit_credit") {
const debit = parseFrenchAmount(
raw[config.columnMapping.debitAmount ?? 0] || ""
);
const credit = parseFrenchAmount(
raw[config.columnMapping.creditAmount ?? 0] || ""
);
amount = isNaN(credit) ? -(isNaN(debit) ? 0 : debit) : credit;
} else {
amount = parseFrenchAmount(
raw[config.columnMapping.amount ?? 0] || ""
);
if (config.signConvention === "positive_expense" && !isNaN(amount)) {
amount = -amount;
}
}
if (!date) {
allRows.push({
rowIndex: allRows.length,
raw,
parsed: null,
error: "Invalid date",
sourceFilename: file.filename,
});
} else if (isNaN(amount)) {
allRows.push({
rowIndex: allRows.length,
raw,
parsed: null,
error: "Invalid amount",
sourceFilename: file.filename,
});
} else {
allRows.push({
rowIndex: allRows.length,
raw,
parsed: { date, description, amount },
sourceFilename: file.filename,
});
}
} catch {
allRows.push({
rowIndex: allRows.length,
raw,
parsed: null,
error: "Parse error",
error: ROW_ERROR_KEYS.parseError,
sourceFilename: file.filename,
});
}
@ -842,7 +809,10 @@ export function useImportWizard() {
// Count errors from parsing
const parseErrors = state.parsedPreview.filter((r) => r.error);
for (const err of parseErrors) {
errors.push({ rowIndex: err.rowIndex, message: err.error || "Parse error" });
errors.push({
rowIndex: err.rowIndex,
message: err.error || ROW_ERROR_KEYS.parseError,
});
}
const report: ImportReport = {

41
src/hooks/useSnapshotEditor.ts
View file

@ -161,7 +161,9 @@ export function holdingsFromServiceHoldings(
* column indices from `analyzeHoldingsCsv`. Behavior:
* - Symbols are normalized (UPPER/TRIM) like manual entry (SecurityPicker) so
* an imported title collapses onto the same `balance_securities` row.
* - Numbers are parsed with `parseFrenchAmount` (handles `1 234,56`, `1,234.56`).
* - Numbers are parsed with `parseFrenchAmount` (handles `1 234,56`, `1,234.56`,
* `(140,10)`); a cell it cannot read is left EMPTY, or kept verbatim for the
* quantity, so validation refuses the row instead of storing a wrong number.
* - unit_price + book_cost are OPTIONAL: when the mapping's column is null (no
* price/cost column) the field stays empty; the user fetches/types it later.
* - Duplicate symbols WITHIN the CSV are merged into one draft to respect the
@ -179,7 +181,13 @@ export function holdingsFromCsvRows(
const order: string[] = [];
const bySymbol = new Map<
string,
{ symbol: string; qty: number; book: number | null; price: string }
{
symbol: string;
qty: number | null;
qtyRaw: string;
book: number | null;
price: string;
}
>();
for (const row of rows) {
@ -188,8 +196,14 @@ export function holdingsFromCsvRows(
const symbol = normalizeSecuritySymbol(rawSymbol);
if (!symbol) continue;
const qtyParsed = parseFrenchAmount((row[mapping.quantity] ?? "").trim());
const qty = isNaN(qtyParsed) ? 0 : qtyParsed;
// An unreadable quantity used to be coerced to 0, which SAVED a
// zero-value position without a word (#325). It stays `null` now and the
// draft keeps the offending text, so `buildDetailedLines` raises the
// existing `snapshot_priced_quantity_required` and the user sees the cell
// to correct.
const qtyRaw = (row[mapping.quantity] ?? "").trim();
const qtyParsed = parseFrenchAmount(qtyRaw);
const qty = isNaN(qtyParsed) ? null : qtyParsed;
let price = "";
if (mapping.unit_price !== null) {
@ -205,12 +219,25 @@ export function holdingsFromCsvRows(
const existing = bySymbol.get(symbol);
if (existing) {
existing.qty += qty;
// One unreadable lot taints the merged quantity: summing it as 0 would
// hide the bad cell behind a plausible total.
if (existing.qty === null || qty === null) {
existing.qty = null;
if (!existing.qtyRaw) existing.qtyRaw = qtyRaw;
} else {
existing.qty += qty;
}
if (book !== null) existing.book = (existing.book ?? 0) + book;
if (!existing.price && price) existing.price = price; // first non-empty
} else {
order.push(symbol);
bySymbol.set(symbol, { symbol, qty, book, price });
bySymbol.set(symbol, {
symbol,
qty,
qtyRaw: qty === null ? qtyRaw : "",
book,
price,
});
}
}
@ -219,7 +246,7 @@ export function holdingsFromCsvRows(
return {
...makeEmptyHolding(defaultAssetType),
symbol: a.symbol,
quantity: String(a.qty),
quantity: a.qty !== null ? String(a.qty) : a.qtyRaw,
unit_price: a.price,
book_cost: a.book !== null ? String(a.book) : "",
};

6
src/i18n/locales/en.json
View file

@ -191,6 +191,12 @@
"unsupportedSignConvention": "The sign convention saved for this source is not recognized. Reconfigure the source before importing.",
"invalidColumnMapping": "The column mapping saved for this source cannot be read. Reconfigure the source before importing."
},
"rowErrors": {
"invalidDate": "Unreadable date",
"invalidAmount": "Unreadable amount",
"amountColumnNotMapped": "Amount column not mapped",
"parseError": "Unreadable row"
},
"help": {
"title": "How to import bank statements",
"tips": [

6
src/i18n/locales/fr.json
View file

@ -191,6 +191,12 @@
"unsupportedSignConvention": "La convention de signe enregistrée pour cette source n'est pas reconnue. Reconfigurez la source avant d'importer.",
"invalidColumnMapping": "Le mapping de colonnes enregistré pour cette source est illisible. Reconfigurez la source avant d'importer."
},
"rowErrors": {
"invalidDate": "Date illisible",
"invalidAmount": "Montant illisible",
"amountColumnNotMapped": "Colonne de montant non mappée",
"parseError": "Ligne illisible"
},
"help": {
"title": "Comment importer des relevés bancaires",
"tips": [

233
src/utils/amountParser.test.ts
View file

@ -1,34 +1,27 @@
// amountParser — characterization tests (issue #326).
// amountParser — characterization tests (#326), hardened (#325).
//
// `parseFrenchAmount` had no test at all, on a codebase of 871. It is called
// from 11 sites (8 in `csvAutoDetect.ts`, 3 in `useSnapshotEditor.ts`) and sits
// under every imported amount, so this file pins what it does TODAY, before
// issue #325 hardens it.
// under every imported amount, so #326 pinned what it did before #325 touched
// it. The `KNOWN DEFECT` blocks #326 left here have since flipped: the
// expectations were updated in place and the markers dropped, per the standing
// rule (update, never delete).
//
// ┌── HOW TO READ THIS FILE ─────────────────────────────────────────────────┐
// │ Two kinds of test live here, and the difference is deliberate: │
// │ │
// │ `describe("… contract")` — behaviour that must SURVIVE the rewrite. │
// │ Breaking one of these is a regression. │
// │ │
// │ `describe("… KNOWN DEFECT")` — behaviour that is WRONG today and is │
// │ expected to change in #325. Every such │
// │ assertion states the right answer in a │
// │ comment. When #325 lands, these tests are │
// │ meant to fail; the fix is to update the │
// │ expectation, not to delete the test. │
// └──────────────────────────────────────────────────────────────────────────┘
//
// The defect that motivated the whole chantier: `parseFrenchAmount` ends on
// The defect that motivated the whole chantier: `parseFrenchAmount` ended on
// `parseFloat`, which stops at the first invalid character instead of rejecting
// the string. A trailing unit or sign therefore yields a magnitude that is off
// by a factor of 100 — and it passes `isNaN`, so it is counted as a VALID row
// everywhere downstream. `"100,00 CAD"` does not fail; it imports as 10 000.
// the string. A trailing unit or sign therefore yielded a magnitude off by a
// factor of 100 — and it passed `isNaN`, so it counted as a VALID row
// everywhere downstream. `"100,00 CAD"` did not fail; it imported as 10 000.
// Validation is anchored now and such a cell is NaN, i.e. a visible row error.
import { describe, it, expect } from "vitest";
import { parseFrenchAmount } from "./amountParser";
import { detectDecimalSeparator, parseFrenchAmount } from "./amountParser";
import { autoDetectConfig } from "./csvAutoDetect";
import { holdingsFromCsvRows } from "../hooks/useSnapshotEditor";
import {
buildDetailedLines,
holdingsFromCsvRows,
} from "../hooks/useSnapshotEditor";
import { BalanceServiceError } from "../services/balance.service";
import { readCsvFixture } from "../__fixtures__/csv";
describe("parseFrenchAmount — separator contract (#326)", () => {
@ -76,99 +69,141 @@ describe("parseFrenchAmount — separator contract (#326)", () => {
});
});
describe("parseFrenchAmount — KNOWN DEFECT: parseFloat prefix scan (#326, fixed by #325)", () => {
// `parseFloat` returns the longest valid PREFIX instead of rejecting the
// whole string. Every expectation below is the bug, not the intent.
describe("parseFrenchAmount — anchored validation (#325, was a KNOWN DEFECT of #326)", () => {
// FIXED. `parseFloat` used to return the longest valid PREFIX instead of
// rejecting the string; validation is anchored over the whole normalized
// value now, so any residual character yields NaN.
//
// #325 must anchor the validation (regex over the whole normalized string)
// and return NaN on any residual character. All of these then become NaN,
// except the accounting-parenthesis and trailing-sign forms, which #325
// explicitly adds support for.
// Note on the two currency/indicator cases: link 1 wrote "should be 1234.56"
// and "should be 100" in the titles, while the block header it wrote just
// above said "all of these then become NaN, except the accounting-parenthesis
// and trailing-sign forms". The header is what #325 implements, for two
// reasons the titles missed. `CR`/`DB` carry a DIRECTION, so returning a
// magnitude for both would replace a loud failure with a silent SIGN error —
// and the D/C-indicator shape is refused upstream by design (#328). And
// "100,00 CAD" is structurally identical to "2025 Montant": rescuing the
// first by whitelisting a trailing word re-blinds `detectHeader` on the
// second, which is the very defect measured below.
it("returns a x100 magnitude on a trailing sign — should be -50", () => {
it("reads a trailing sign as a negative amount", () => {
// "50,00-" is the trailing-minus convention of several bank exports.
// The comma stops looking like a decimal separator once "-" trails it
// (the /,\d{1,2}$/ probe fails), so the comma is dropped as a thousands
// separator and "5000-" parses as 5000.
expect(parseFrenchAmount("50,00-")).toBe(5000);
expect(parseFrenchAmount("50,00-")).toBe(-50);
expect(parseFrenchAmount("1 234,56-")).toBe(-1234.56);
expect(parseFrenchAmount("50,00+")).toBe(50);
});
it("returns a x100 magnitude on a trailing indicator — should be 1234.56", () => {
expect(parseFrenchAmount("1 234,56 CR")).toBe(123456);
expect(parseFrenchAmount("1 234,56 DB")).toBe(123456);
it("rejects a trailing direction indicator instead of guessing a sign", () => {
expect(parseFrenchAmount("1 234,56 CR")).toBeNaN();
expect(parseFrenchAmount("1 234,56 DB")).toBeNaN();
});
it("returns a x100 magnitude on a trailing currency code — should be 100", () => {
expect(parseFrenchAmount("100,00 CAD")).toBe(10000);
expect(parseFrenchAmount("84,32 USD")).toBe(8432);
it("rejects a trailing currency code", () => {
expect(parseFrenchAmount("100,00 CAD")).toBeNaN();
expect(parseFrenchAmount("84,32 USD")).toBeNaN();
});
it("accepts a numeric prefix of a text label — should be NaN", () => {
// This is what blinds `detectHeader`: a header cell that STARTS with
// digits reads as a number, so the header row is taken for data.
expect(parseFrenchAmount("2024 Montant")).toBe(2024);
expect(parseFrenchAmount("5%")).toBe(5);
it("rejects the numeric prefix of a text label", () => {
// This is what used to blind `detectHeader`: a header cell STARTING with
// digits read as a number, so the header row was taken for data.
expect(parseFrenchAmount("2024 Montant")).toBeNaN();
expect(parseFrenchAmount("5%")).toBeNaN();
});
it("accepts JavaScript number literals a bank never emits — should be NaN", () => {
expect(parseFrenchAmount("1e3")).toBe(1000);
expect(parseFrenchAmount("Infinity")).toBe(Infinity);
it("rejects JavaScript number literals a bank never emits", () => {
expect(parseFrenchAmount("1e3")).toBeNaN();
expect(parseFrenchAmount("Infinity")).toBeNaN();
expect(parseFrenchAmount("0x1F")).toBeNaN();
});
it("keeps only the first two groups of a malformed number — should be NaN", () => {
expect(parseFrenchAmount("1,2,3")).toBe(1.2);
it("rejects a malformed number instead of keeping its first two groups", () => {
expect(parseFrenchAmount("1,2,3")).toBeNaN();
expect(parseFrenchAmount("1.2.3")).toBeNaN();
expect(parseFrenchAmount("1,23,456")).toBeNaN(); // groups must be 3 digits
});
it("rejects two signs, wherever they sit", () => {
expect(parseFrenchAmount("-50,00-")).toBeNaN();
expect(parseFrenchAmount("(-50,00)")).toBeNaN();
});
});
describe("parseFrenchAmount — KNOWN DEFECT: unsupported accounting forms (#326, fixed by #325)", () => {
it("rejects accounting parentheses — should be -50", () => {
// "(50,00)" is the standard accounting notation for a negative amount.
// Today it is NaN, so the row is dropped as "Invalid amount" rather than
// being imported with the wrong sign — a loud failure, unlike the ones
// above, but still a failure.
expect(parseFrenchAmount("(50,00)")).toBeNaN();
expect(parseFrenchAmount("(1 234,56)")).toBeNaN();
describe("parseFrenchAmount — accounting forms (#325, was a KNOWN DEFECT of #326)", () => {
it("reads accounting parentheses as a negative amount", () => {
expect(parseFrenchAmount("(50,00)")).toBe(-50);
expect(parseFrenchAmount("(1 234,56)")).toBe(-1234.56);
expect(parseFrenchAmount("(1,234.56)")).toBe(-1234.56);
});
it("still rejects an unbalanced parenthesis", () => {
expect(parseFrenchAmount("(50,00")).toBeNaN();
expect(parseFrenchAmount("50,00)")).toBeNaN();
});
});
describe("parseFrenchAmount — KNOWN DEFECT: separator arbitrated per cell (#326, fixed by #325)", () => {
// The French-vs-English decision is taken on each cell in isolation
// (`/,\d{1,2}$/`), never at column level. Two cells of the SAME column can
// therefore be read under two different conventions.
//
// #325 arbitrates the separator per column, which resolves both cases.
describe("parseFrenchAmount — column-level arbitration (#325, was a KNOWN DEFECT of #326)", () => {
// The French-vs-English decision used to be taken on each cell in isolation
// and NOTHING else, so two cells of the same column could be read under two
// different conventions. The isolated readings below are unchanged — they
// are the best a lone cell allows — but a caller that knows the column now
// passes its verdict and settles the ambiguity.
it("reads a 3-digit group after the comma as a thousands separator", () => {
it("keeps the documented reading when no column context is given", () => {
// "12,345" is 12.345 in a column of decimals, 12345 in a column of
// thousands. Nothing in the cell alone can tell.
// thousands. Nothing in the cell ALONE can tell.
expect(parseFrenchAmount("12,345")).toBe(12345);
});
it("reads a dot as a decimal separator when no comma is present", () => {
// In a French column, "1.234" means 1234. Read alone it yields 1.234 —
// a x1000 error in the opposite direction from the cases above.
expect(parseFrenchAmount("1.234")).toBe(1.234);
expect(parseFrenchAmount("1.234,56")).toBe(1234.56); // ...unless a comma follows
});
it("obeys the column verdict when there is one", () => {
expect(parseFrenchAmount("12,345", { decimalSeparator: "," })).toBe(12.345);
expect(parseFrenchAmount("12,345", { decimalSeparator: "." })).toBe(12345);
expect(parseFrenchAmount("1.234", { decimalSeparator: "," })).toBe(1234);
expect(parseFrenchAmount("1.234", { decimalSeparator: "." })).toBe(1.234);
});
it("still rejects a value that contradicts the column verdict", () => {
// A grouping separator groups by three, always.
expect(parseFrenchAmount("1.23", { decimalSeparator: "," })).toBeNaN();
expect(parseFrenchAmount("1,23", { decimalSeparator: "." })).toBeNaN();
});
it("arbitrates a column from a decisive sibling cell", () => {
// "1.234" alone is ambiguous; "84,32" in the same column is not.
expect(detectDecimalSeparator(["1.234", "84,32", "-6,95"])).toBe(",");
expect(detectDecimalSeparator(["1,234", "84.32", "-6.95"])).toBe(".");
// Mixed notation settles on the last separator of each decisive cell.
expect(detectDecimalSeparator(["1.234,56"])).toBe(",");
expect(detectDecimalSeparator(["1,234.56"])).toBe(".");
});
it("returns no verdict when the column gives no evidence", () => {
expect(detectDecimalSeparator([])).toBeUndefined();
expect(detectDecimalSeparator(["1.234", "5.678"])).toBeUndefined();
expect(detectDecimalSeparator(["", " ", "N/A"])).toBeUndefined();
// One vote each way is a tie, not a majority.
expect(detectDecimalSeparator(["84,32", "84.32"])).toBeUndefined();
});
});
describe("parseFrenchAmount — call-site fallout (#326)", () => {
describe("parseFrenchAmount — call-site fallout (#326, hardened by #325)", () => {
// The /review-spec revision of #326 asks the corpus to reach the holdings
// call sites too, because #325 hardens the parser GLOBALLY. These pin what
// the shared parser does to its two most exposed consumers.
it("blinds detectHeader when a header cell starts with digits", () => {
it("no longer blinds detectHeader when a header cell starts with digits", () => {
// `detectHeader` (csvAutoDetect.ts:224) treats "parses as a number" as
// proof of a data row. "2025 Montant" parses as 2025, so the header is
// taken for data.
// proof of a data row. "2025 Montant" used to parse as 2025, so the header
// was taken for data; anchoring makes it NaN and the header is recognised.
const cfg = autoDetectConfig(readCsvFixture("header-numeric-label"))!;
expect(cfg.hasHeader).toBe(false); // DEFECT — should be true
expect(cfg.hasHeader).toBe(true);
});
it("leaks the x100 magnitude into the holdings CSV import (#245)", () => {
it("no longer leaks a x100 magnitude into the holdings CSV import (#245)", () => {
// `holdingsFromCsvRows` (useSnapshotEditor.ts:191-202) shares the parser.
// A price column carrying its currency code silently multiplies every
// position by 100 — a $150.25 share is stored at $15,025.
// A price column carrying its currency code used to multiply every
// position by 100 — a $150.25 share stored at $15,025. It is refused now,
// and an empty price is what `buildDetailedLines` rejects on save.
const drafts = holdingsFromCsvRows(
[
["AAPL", "10", "150,25 CAD", "1 200,00"],
@ -176,19 +211,43 @@ describe("parseFrenchAmount — call-site fallout (#326)", () => {
],
{ symbol: 0, quantity: 1, unit_price: 2, book_cost: 3 }
);
expect(drafts[0].unit_price).toBe("15025"); // DEFECT — should be "150.25"
expect(drafts[0].book_cost).toBe("1200"); // clean cell, correct today
expect(drafts[1].unit_price).toBe("300.5"); // clean cell, correct today
expect(drafts[1].book_cost).toBe("140000"); // DEFECT — should be "1400"
expect(drafts[0].unit_price).toBe(""); // refused, not 15025
expect(drafts[0].book_cost).toBe("1200"); // clean cell
expect(drafts[1].unit_price).toBe("300.5"); // clean cell
expect(drafts[1].book_cost).toBe(""); // refused, not 140000
});
it("drops an accounting-parenthesis price instead of reading it", () => {
it("reads an accounting-parenthesis price instead of dropping it", () => {
const drafts = holdingsFromCsvRows(
[["GOOG", "2", "(140,10)", "280,20"]],
{ symbol: 0, quantity: 1, unit_price: 2, book_cost: 3 }
);
// NaN price is swallowed to an empty string — no error surfaces.
expect(drafts[0].unit_price).toBe(""); // DEFECT — should be "-140.10"
expect(drafts[0].unit_price).toBe("-140.1");
expect(drafts[0].quantity).toBe("2");
});
it("keeps an unreadable quantity verbatim so the save refuses it", () => {
// It used to be coerced to 0, which SAVED a zero-value position in
// silence. `buildDetailedLines` throws on the raw text instead.
const drafts = holdingsFromCsvRows(
[["GOOG", "2 parts", "140,10", "280,20"]],
{ symbol: 0, quantity: 1, unit_price: 2, book_cost: 3 }
);
expect(drafts[0].quantity).toBe("2 parts");
expect(() =>
buildDetailedLines({ 7: drafts }, new Set([7]))
).toThrowError(BalanceServiceError);
});
it("taints the merged quantity when one lot of a symbol is unreadable", () => {
const drafts = holdingsFromCsvRows(
[
["AAPL", "6", "150,00", "700"],
["AAPL", "quatre", "151,00", "500"],
],
{ symbol: 0, quantity: 1, unit_price: 2, book_cost: 3 }
);
expect(drafts).toHaveLength(1);
expect(drafts[0].quantity).toBe("quatre"); // NOT "6"
});
});

200
src/utils/amountParser.ts
View file

@ -1,26 +1,192 @@
/**
* Parse a French-formatted amount string to a number.
* Handles formats like: 1.234,56 / 1234,56 / -1 234.56 / 1 234,56
* Amount parsing for imported files (#325).
*
* The function used to end on `parseFloat`, which returns the longest valid
* PREFIX of a string instead of rejecting it. `"100,00 CAD"` therefore came out
* as 10 000 and `"1 234,56 CR"` as 123 456 — a factor-100 error that passes
* `isNaN`, so the row counted as VALID everywhere downstream. Validation is
* ANCHORED now: after normalisation the whole remaining string must match a
* numeric grammar, and any residual character yields `NaN`.
*
* `NaN` is the only safe answer for a cell we cannot read with certainty. A
* trailing `CR` / `DB` carries a DIRECTION, not noise, so guessing a magnitude
* for it would trade a loud failure for a silent sign error — the exact bug
* class this chantier exists to remove. The absolute-amount + D/C-indicator
* shape is refused, by design, upstream (#328).
*
* Two accounting forms ARE legitimate and supported: parentheses `(50,00)` and
* a trailing sign `50,00-`, both meaning a negative amount.
*
* Separator arbitration: `1.234` means 1234 in a French column and 1.234 in an
* English one, and nothing INSIDE the cell can tell. Callers that know the
* column pass `decimalSeparator` (see `detectDecimalSeparator`); callers that
* do not get the documented per-cell default.
*/
export function parseFrenchAmount(raw: string): number {
/** Which character separates the decimals in a given column. */
export type DecimalSeparator = "," | ".";
export interface AmountParseOptions {
/**
* Decimal separator arbitrated at COLUMN level. When set, the other character
* is read as a grouping separator, whatever a single cell looks like.
*/
decimalSeparator?: DecimalSeparator;
}
/** Currency symbols and every flavour of space are noise, never data. */
const NOISE = /[€$£\s\u00A0]/g;
/** Digits, optionally grouped in threes by `sep`. Anchored. */
function groupedRe(sep: "," | "."): RegExp {
const s = sep === "." ? "\\." : ",";
return new RegExp(`^\\d{1,3}(?:${s}\\d{3})+$`);
}
/** Digits, one `sep`, then decimals. Anchored. `limit` caps the decimal count. */
function decimalRe(sep: "," | ".", limit?: number): RegExp {
const s = sep === "." ? "\\." : ",";
const tail = limit === undefined ? "+" : `{1,${limit}}`;
return new RegExp(`^\\d+${s}\\d${tail}$`);
}
/** Grouped digits then decimals, e.g. `1.234,56`. Anchored. */
function groupedDecimalRe(group: "," | ".", dec: "," | "."): RegExp {
const g = group === "." ? "\\." : ",";
const d = dec === "." ? "\\." : ",";
return new RegExp(`^\\d{1,3}(?:${g}\\d{3})+${d}\\d+$`);
}
/**
* Read a fully normalised body (digits plus `.` and `,` only, no sign, no
* spaces) under a known decimal separator. Returns `NaN` when the body does not
* match the grammar exactly.
*/
function readWithSeparator(body: string, decimal: DecimalSeparator): number {
const group: DecimalSeparator = decimal === "," ? "." : ",";
const hasDecimal = body.includes(decimal);
const hasGroup = body.includes(group);
let normalized: string | null = null;
if (hasDecimal && hasGroup) {
if (groupedDecimalRe(group, decimal).test(body)) normalized = body;
} else if (hasDecimal) {
if (decimalRe(decimal).test(body)) normalized = body;
} else if (hasGroup) {
if (groupedRe(group).test(body)) normalized = body;
} else if (/^\d+$/.test(body)) {
normalized = body;
}
if (normalized === null) return NaN;
const stripped = normalized.split(group).join("");
return Number(decimal === "," ? stripped.replace(",", ".") : stripped);
}
/**
* Read a body with no column context. The rules reproduce the documented
* per-cell behaviour, now anchored:
* - both separators present -> the LAST one is the decimal;
* - a single `,` followed by one or two digits -> French decimal;
* - a single `.` -> English decimal (`1.234` reads as 1.234 alone);
* - otherwise the separator must group digits in perfect threes.
*/
function readPerCell(body: string): number {
const lastComma = body.lastIndexOf(",");
const lastDot = body.lastIndexOf(".");
if (lastComma >= 0 && lastDot >= 0) {
return readWithSeparator(body, lastComma > lastDot ? "," : ".");
}
if (lastComma >= 0) {
if (decimalRe(",", 2).test(body)) return readWithSeparator(body, ",");
return groupedRe(",").test(body) ? readWithSeparator(body, ".") : NaN;
}
if (lastDot >= 0) {
if (decimalRe(".").test(body)) return readWithSeparator(body, ".");
return groupedRe(".").test(body) ? readWithSeparator(body, ",") : NaN;
}
return /^\d+$/.test(body) ? Number(body) : NaN;
}
/**
* Parse an amount cell to a number, or `NaN` when it cannot be read with
* certainty. Handles `1.234,56`, `1 234,56`, `1,234.56`, `-84,32`, `(50,00)`
* and `50,00-`.
*/
export function parseFrenchAmount(
raw: string,
options: AmountParseOptions = {}
): number {
if (!raw || typeof raw !== "string") return NaN;
let cleaned = raw.trim();
let negative = false;
// Remove currency symbols and whitespace
cleaned = cleaned.replace(/[€$£\s\u00A0]/g, "");
// Detect if comma is decimal separator (French style)
// Pattern: digits followed by comma followed by exactly 1-2 digits at end
const frenchPattern = /,\d{1,2}$/;
if (frenchPattern.test(cleaned)) {
// French format: remove dots (thousand sep), replace comma with dot (decimal)
cleaned = cleaned.replace(/\./g, "").replace(",", ".");
} else {
// English format or no decimal: remove commas (thousand sep)
cleaned = cleaned.replace(/,/g, "");
// Accounting parentheses. Only a balanced, outermost pair counts; `(50,00`
// falls through and fails the anchored grammar below.
const parens = /^\((.*)\)$/.exec(cleaned);
if (parens) {
negative = true;
cleaned = parens[1].trim();
}
const result = parseFloat(cleaned);
return isNaN(result) ? NaN : result;
cleaned = cleaned.replace(NOISE, "");
if (!cleaned) return NaN;
// One sign, leading or trailing, never both, never inside parentheses.
const leading = /^[+-]/.test(cleaned);
const trailing = /[+-]$/.test(cleaned);
if (leading && trailing) return NaN;
if (leading || trailing) {
if (negative) return NaN; // `(-50,00)` is not a form any bank emits
negative = cleaned[leading ? 0 : cleaned.length - 1] === "-";
cleaned = leading ? cleaned.slice(1) : cleaned.slice(0, -1);
}
// Anchored gate: nothing but digits and separators may remain. This is what
// rejects `2024Montant`, `1e3`, `Infinity`, `5%` and `100,00CAD`.
if (!/^[\d.,]+$/.test(cleaned) || !/\d/.test(cleaned)) return NaN;
const value = options.decimalSeparator
? readWithSeparator(cleaned, options.decimalSeparator)
: readPerCell(cleaned);
if (!Number.isFinite(value)) return NaN;
return negative ? -value : value;
}
/**
* Arbitrate the decimal separator of a COLUMN from all of its cells.
*
* A cell is decisive when it carries both separators (the last one is the
* decimal) or exactly one separator followed by one or two digits — a grouping
* separator is always followed by three. Ambiguous cells such as `1.234` vote
* for nothing. Returns `undefined` when the column gives no verdict, in which
* case callers keep the per-cell default.
*/
export function detectDecimalSeparator(
cells: Iterable<string>
): DecimalSeparator | undefined {
let comma = 0;
let dot = 0;
for (const cell of cells) {
if (!cell || typeof cell !== "string") continue;
const body = cell.trim().replace(NOISE, "").replace(/^[+-]|[+-]$/g, "");
if (!body) continue;
const lastComma = body.lastIndexOf(",");
const lastDot = body.lastIndexOf(".");
if (lastComma >= 0 && lastDot >= 0) {
if (lastComma > lastDot) comma++;
else dot++;
continue;
}
if (lastComma >= 0 && decimalRe(",", 2).test(body)) comma++;
else if (lastDot >= 0 && decimalRe(".", 2).test(body)) dot++;
}
if (comma === dot) return undefined;
return comma > dot ? "," : ".";
}

153
src/utils/csvAutoDetect.test.ts
View file

@ -20,8 +20,8 @@ import {
autoDetectConfig,
preprocessQuotedCSV,
} from "./csvAutoDetect";
import { parseFrenchAmount } from "./amountParser";
import { parseDate } from "./dateParser";
import type { DecimalSeparator } from "./amountParser";
import { detectAmountSeparators, mapRow } from "./importFormat";
import {
CSV_FIXTURE_NAMES,
readCsvFixture,
@ -157,44 +157,25 @@ describe("analyzeHoldingsCsv (#245)", () => {
// └──────────────────────────────────────────────────────────────────────────┘
/**
* MIRROR of the row-mapping rule in `useImportWizard.parseFilesInternal`
* (`useImportWizard.ts:489-542`), copied verbatim.
* The row-mapping rule is `mapRow` (`src/utils/importFormat.ts`) since #325.
*
* It is duplicated here rather than imported because the rule currently lives
* inside a `useCallback` of a React hook and the repository has no jsdom.
* Issue #325 extracts it as a pure `mapRow(raw, format)` in
* `src/utils/importFormat.ts` — AT THAT POINT THIS MIRROR MUST BE DELETED and
* the tests re-pointed at the real `mapRow`. Until then, the guard test
* "production rule still matches the mirror" below fails the build if the
* production expression drifts, so the mirror cannot silently start lying.
* It used to be duplicated here as a hand copy called `mapCorpusRow`, because
* the rule lived inside a `useCallback` of a React hook and the repository has
* no jsdom; a static guard test pinned the five production expressions so the
* copy could not silently start lying. #325 lifted the rule out as a pure
* function, so the mirror and its guard are gone and these tests run the real
* thing — which is the whole point of the extraction.
*
* The shape below keeps the corpus assertions readable: a row is either its
* three parsed fields or its error.
*/
function mapCorpusRow(
raw: string[],
cfg: NonNullable<ReturnType<typeof autoDetectConfig>>
cfg: NonNullable<ReturnType<typeof autoDetectConfig>>,
decimalSeparators?: ReadonlyMap<number, DecimalSeparator>
): { date: string; description: string; amount: number } | { error: string } {
const date = parseDate(
raw[cfg.columnMapping.date]?.trim() || "",
cfg.dateFormat
);
const description = raw[cfg.columnMapping.description]?.trim() || "";
let amount: number;
if (cfg.amountMode === "debit_credit") {
const debit = parseFrenchAmount(raw[cfg.columnMapping.debitAmount ?? 0] || "");
const credit = parseFrenchAmount(
raw[cfg.columnMapping.creditAmount ?? 0] || ""
);
amount = isNaN(credit) ? -(isNaN(debit) ? 0 : debit) : credit;
} else {
amount = parseFrenchAmount(raw[cfg.columnMapping.amount ?? 0] || "");
if (cfg.signConvention === "positive_expense" && !isNaN(amount)) {
amount = -amount;
}
}
if (!date) return { error: "Invalid date" };
if (isNaN(amount)) return { error: "Invalid amount" };
return { date, description, amount };
const row = mapRow(raw, { ...cfg, encoding: "utf-8" }, { decimalSeparators });
return row.parsed ?? { error: row.error! };
}
/** Full pipeline: raw file text -> signed amounts, as the wizard runs it. */
@ -202,6 +183,7 @@ function parseFixtureEndToEnd(name: CsvFixtureName) {
const rawContent = readCsvFixture(name);
const cfg = autoDetectConfig(rawContent);
if (!cfg) throw new Error(`autoDetectConfig returned null for ${name}`);
const format = { ...cfg, encoding: "utf-8" };
const data = Papa.parse(preprocessQuotedCSV(rawContent), {
delimiter: cfg.delimiter,
@ -209,12 +191,14 @@ function parseFixtureEndToEnd(name: CsvFixtureName) {
}).data as string[][];
const startIdx = cfg.skipLines + (cfg.hasHeader ? 1 : 0);
const rows = [];
const dataRows: string[][] = [];
for (let i = startIdx; i < data.length; i++) {
const raw = data[i];
if (raw.length <= 1 && raw[0]?.trim() === "") continue;
rows.push(mapCorpusRow(raw, cfg));
dataRows.push(raw);
}
const decimalSeparators = detectAmountSeparators(dataRows, format);
const rows = dataRows.map((raw) => mapCorpusRow(raw, cfg, decimalSeparators));
return { config: cfg, rows };
}
@ -242,27 +226,21 @@ describe("corpus integrity (#326)", () => {
}
});
it("production rule still matches the mirror in mapCorpusRow", () => {
// Static contract, in the style of __integration__/transactions-transfer-icon.
// `mapCorpusRow` is a hand copy of a rule that lives inside a React hook;
// this is what stops it drifting. #325 extracts the rule as `mapRow` —
// when it does, these matchers fail, and that IS the signal to delete the
// mirror and import the real function.
it("runs the production rule, with no mirror left to drift (#325)", () => {
// The guard that used to live here pinned five expressions of
// `parseFilesInternal` so the hand-written mirror could not drift. Both are
// gone: the wizard delegates to `mapRow` and these tests call it directly.
const SRC = readFileSync(
resolve(import.meta.dirname, "..", "hooks", "useImportWizard.ts"),
"utf-8"
);
expect(SRC).toContain(
"amount = isNaN(credit) ? -(isNaN(debit) ? 0 : debit) : credit;"
);
expect(SRC).toContain(
'if (config.signConvention === "positive_expense" && !isNaN(amount)) {'
);
// The `?? 0` fallbacks silently read column 0 when a mapping is missing.
// #325 replaces them with an explicit row error.
expect(SRC).toContain("raw[config.columnMapping.debitAmount ?? 0]");
expect(SRC).toContain("raw[config.columnMapping.creditAmount ?? 0]");
expect(SRC).toContain("raw[config.columnMapping.amount ?? 0]");
expect(SRC).toContain("mapRow(raw, config, {");
// The `?? 0` fallbacks silently read column 0 — usually the date — when a
// mapping was incomplete. They must never come back.
expect(SRC).not.toContain("columnMapping.debitAmount ?? 0");
expect(SRC).not.toContain("columnMapping.creditAmount ?? 0");
expect(SRC).not.toContain("columnMapping.amount ?? 0");
expect(SRC).not.toContain("isNaN(credit)");
});
});
@ -358,15 +336,16 @@ describe("autoDetectConfig — KNOWN DEFECT: debit/credit order guessed by posit
});
});
describe("autoDetectConfig — KNOWN DEFECT: unused column filled with 0,00 (#326, fixed by #325)", () => {
// When the unused half of a debit/credit pair carries "0,00" instead of an
// empty cell, detection still gets the mode right — `isSparseComplementary`
// treats a 0 as absent. The PARSING is what breaks: the rule branches on
// `isNaN(credit)` (useImportWizard.ts:509), and "0,00" parses to 0, not NaN.
// Every debit row therefore imports as 0,00 and the expense vanishes.
describe("autoDetectConfig — unused column filled with 0,00 (#326, fixed by #325)", () => {
// FIXED. When the unused half of a debit/credit pair carries "0,00" instead
// of an empty cell, detection always got the mode right —
// `isSparseComplementary` treats a 0 as absent. The PARSING was what broke:
// the rule branched on `isNaN(credit)`, and "0,00" parses to 0, not NaN, so
// every debit row imported as 0,00 and the expense vanished — silently, since
// 0 passes validation.
//
// #325 replaces the nullity comparison with `amount = credit - debit` on
// magnitudes and treats a 0,00 cell as absent.
// The rule is `credit - debit` on magnitudes now, for which a 0,00 cell needs
// no special case: zero is the identity of the subtraction.
it("still detects the debit_credit mode correctly", () => {
const cfg = autoDetectConfig(readCsvFixture("unused-column-zero"))!;
@ -375,17 +354,21 @@ describe("autoDetectConfig — KNOWN DEFECT: unused column filled with 0,00 (#32
expect(cfg.columnMapping.creditAmount).toBe(3);
});
it("swallows every debit to zero end to end", () => {
expect(amountsOf("unused-column-zero")).toEqual(
// DEFECT — should be REFERENCE_AMOUNTS
[0, 1250, 0, 0, 300, 0]
it("parses end to end to the reference amounts", () => {
expect(amountsOf("unused-column-zero")).toEqual(REFERENCE_AMOUNTS);
});
it("keeps every expense, and none of them is zero", () => {
const amounts = amountsOf("unused-column-zero");
expect(amounts.filter((a) => a === 0)).toHaveLength(0);
expect(amounts.filter((a) => typeof a === "number" && a < 0)).toHaveLength(
4
);
});
it("keeps the credits intact, so only the expenses disappear", () => {
const amounts = amountsOf("unused-column-zero");
expect(amounts.filter((a) => a === 0)).toHaveLength(4);
expect(amounts.filter((a) => a !== 0)).toEqual([1250, 300]);
it("reads the same file identically to its empty-cell twin", () => {
// Same six transactions, written with empty cells instead of 0,00.
expect(amountsOf("unused-column-zero")).toEqual(amountsOf("debit-credit"));
});
});
@ -431,29 +414,27 @@ describe("autoDetectConfig — header carrying a number (#326)", () => {
});
});
describe("autoDetectConfig — KNOWN DEFECT: header cell starting with digits (#326, fixed by #328)", () => {
// "2025 Montant" normalizes to "2025Montant"; parseFloat returns the 2025
// prefix, `hasNumber` flips true, and the header row is taken for data.
describe("autoDetectConfig — header cell starting with digits (#326, fixed by #325)", () => {
// FIXED, by the other of the two routes link 1 named. "2025 Montant"
// normalizes to "2025Montant"; `parseFloat` returned the 2025 prefix,
// `hasNumber` flipped true, and the header row was taken for data.
//
// #328 adds a lexical signal to `detectHeader`, and #325 anchors the parser
// so a numeric prefix no longer reads as a number. Either fix closes this.
// Link 1's note: "#328 adds a lexical signal to `detectHeader`, and #325
// anchors the parser so a numeric prefix no longer reads as a number. Either
// fix closes this." #325 landed first — the anchored parser returns NaN for
// that cell, so `detectHeader` sees a row with no number and no date.
// #328 keeps its lexical signal for header cells that carry a BARE number.
it("takes the header row for a data row", () => {
it("recognises the header row", () => {
const cfg = autoDetectConfig(readCsvFixture("header-numeric-label"))!;
expect(cfg.hasHeader).toBe(false); // DEFECT — should be true
// The column mapping survives because the six real rows outvote the header.
expect(cfg.hasHeader).toBe(true);
expect(cfg.columnMapping).toEqual({ date: 0, description: 1, amount: 2 });
});
it("emits the header row as an error row instead of skipping it", () => {
it("skips the header instead of emitting it as an error row", () => {
const { rows } = parseFixtureEndToEnd("header-numeric-label");
expect(rows).toHaveLength(7); // DEFECT — should be 6
// The header survives as far as the date check, which is the only reason
// it fails loudly rather than importing 2025,00 as a transaction.
expect(rows[0]).toEqual({ error: "Invalid date" });
expect(amountsOf("header-numeric-label").slice(1)).toEqual(
REFERENCE_AMOUNTS
);
expect(rows).toHaveLength(6);
expect(amountsOf("header-numeric-label")).toEqual(REFERENCE_AMOUNTS);
});
});

215
src/utils/importFormat.test.ts
View file

@ -17,10 +17,14 @@ import {
AMOUNT_MODES,
FORMAT_FIELD_PAIRS,
ImportFormatError,
ROW_ERROR_KEYS,
SIGN_CONVENTIONS,
clearMappingForMode,
detectAmountSeparators,
formatFromRow,
formatToRow,
isRowErrorKey,
mapRow,
} from "./importFormat";
// ---------------------------------------------------------------------------
@ -304,6 +308,217 @@ describe("clearMappingForMode", () => {
});
});
// ---------------------------------------------------------------------------
// mapRow — the row rule, lifted out of the wizard (#325)
// ---------------------------------------------------------------------------
const DC_FORMAT: ImportFormat = {
delimiter: ";",
encoding: "utf-8",
dateFormat: "DD/MM/YYYY",
skipLines: 0,
hasHeader: true,
columnMapping: { date: 0, description: 1, debitAmount: 2, creditAmount: 3 },
amountMode: "debit_credit",
signConvention: "negative_expense",
};
const SINGLE_FORMAT: ImportFormat = {
...DC_FORMAT,
columnMapping: { date: 0, description: 1, amount: 2 },
amountMode: "single",
};
/** The amount of a row that parsed, or the error key of one that did not. */
function outcome(row: ReturnType<typeof mapRow>): number | string {
return row.parsed ? row.parsed.amount : row.error!;
}
describe("mapRow — debit/credit is a subtraction, not a nullity test (#325)", () => {
it("reads a debit as negative and a credit as positive", () => {
expect(
outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", ""], DC_FORMAT))
).toBe(-84.32);
expect(
outcome(mapRow(["15/01/2025", "PAIE", "", "1250,00"], DC_FORMAT))
).toBe(1250);
});
it("treats a 0,00 cell in the unused column as absent", () => {
// The bug: `isNaN(credit)` was false for "0,00", so the credit won and
// every debit imported as zero.
expect(
outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", "0,00"], DC_FORMAT))
).toBe(-84.32);
expect(
outcome(mapRow(["15/01/2025", "PAIE", "0,00", "1250,00"], DC_FORMAT))
).toBe(1250);
});
it("takes both columns as magnitudes, whatever sign they carry", () => {
expect(
outcome(mapRow(["05/01/2025", "EPICERIE", "-84,32", ""], DC_FORMAT))
).toBe(-84.32);
expect(
outcome(mapRow(["15/01/2025", "PAIE", "", "-1250,00"], DC_FORMAT))
).toBe(1250);
});
it("nets a row that fills both columns", () => {
expect(
outcome(mapRow(["05/01/2025", "AJUST", "40,00", "100,00"], DC_FORMAT))
).toBe(60);
});
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
);
expect(outcome(mapRow(["05/01/2025", "X", "n/a", "-"], DC_FORMAT))).toBe(
ROW_ERROR_KEYS.invalidAmount
);
});
it("ignores the sign convention, which only applies to a single column", () => {
const flipped: ImportFormat = {
...DC_FORMAT,
signConvention: "positive_expense",
};
expect(
outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", ""], flipped))
).toBe(-84.32);
});
});
describe("mapRow — an unmapped amount column is an error, not column 0 (#325)", () => {
// The `?? 0` fallbacks read column 0 — usually the date — for every row.
it("refuses a single-amount format with no amount column", () => {
const { amount: _drop, ...mapping } = SINGLE_FORMAT.columnMapping;
const row = mapRow(["05/01/2025", "EPICERIE", "-84,32"], {
...SINGLE_FORMAT,
columnMapping: mapping,
});
expect(row.parsed).toBeNull();
expect(row.error).toBe(ROW_ERROR_KEYS.amountColumnNotMapped);
});
it("refuses a debit/credit format with neither column mapped", () => {
const row = mapRow(["05/01/2025", "EPICERIE", "84,32", ""], {
...DC_FORMAT,
columnMapping: { date: 0, description: 1 },
});
expect(row.error).toBe(ROW_ERROR_KEYS.amountColumnNotMapped);
});
it("accepts a debit/credit format with only ONE column mapped", () => {
// Half a pair is a legitimate shape (a card statement with debits only).
expect(
outcome(
mapRow(["05/01/2025", "EPICERIE", "84,32"], {
...DC_FORMAT,
columnMapping: { date: 0, description: 1, debitAmount: 2 },
})
)
).toBe(-84.32);
});
it("reports the format error before any per-row problem", () => {
const row = mapRow(["not-a-date", "EPICERIE", "-84,32"], {
...SINGLE_FORMAT,
columnMapping: { date: 0, description: 1 },
});
expect(row.error).toBe(ROW_ERROR_KEYS.amountColumnNotMapped);
});
});
describe("mapRow — the rest of the contract (#325)", () => {
it("keeps the date check ahead of the amount check", () => {
expect(outcome(mapRow(["nope", "X", "abc"], SINGLE_FORMAT))).toBe(
ROW_ERROR_KEYS.invalidDate
);
expect(outcome(mapRow(["05/01/2025", "X", "abc"], SINGLE_FORMAT))).toBe(
ROW_ERROR_KEYS.invalidAmount
);
});
it("applies positive_expense to a single amount column", () => {
expect(
outcome(
mapRow(["05/01/2025", "EPICERIE", "84,32"], {
...SINGLE_FORMAT,
signConvention: "positive_expense",
})
)
).toBe(-84.32);
});
it("carries rowIndex, raw and sourceFilename through", () => {
const raw = ["05/01/2025", "EPICERIE", "-84,32"];
const row = mapRow(raw, SINGLE_FORMAT, {
rowIndex: 41,
sourceFilename: "releve.csv",
});
expect(row.rowIndex).toBe(41);
expect(row.raw).toBe(raw);
expect(row.sourceFilename).toBe("releve.csv");
expect(row.parsed).toEqual({
date: "2025-01-05",
description: "EPICERIE",
amount: -84.32,
});
});
it("defaults rowIndex to 0 and omits sourceFilename when not given", () => {
const row = mapRow(["05/01/2025", "X", "-1,00"], SINGLE_FORMAT);
expect(row.rowIndex).toBe(0);
expect("sourceFilename" in row).toBe(false);
});
it("reports every failure as an i18n key", () => {
for (const raw of [
["nope", "X", "-1,00"],
["05/01/2025", "X", "abc"],
]) {
const row = mapRow(raw, SINGLE_FORMAT);
expect(isRowErrorKey(row.error!)).toBe(true);
}
expect(isRowErrorKey("boom: sqlite is locked")).toBe(false);
});
});
describe("detectAmountSeparators — the column decides (#325)", () => {
const rows = [
["05/01/2025", "EPICERIE", "1.234", ""],
["15/01/2025", "PAIE", "", "84,32"],
];
it("arbitrates each amount column the format declares", () => {
const seps = detectAmountSeparators(rows, DC_FORMAT);
// Column 2 holds only the ambiguous "1.234" — no verdict of its own.
expect(seps.get(2)).toBeUndefined();
expect(seps.get(3)).toBe(",");
});
it("turns an ambiguous cell into the column's reading", () => {
const french = [
["05/01/2025", "A", "1.234"],
["15/01/2025", "B", "84,32"],
];
const seps = detectAmountSeparators(french, SINGLE_FORMAT);
expect(seps.get(2)).toBe(",");
expect(
outcome(mapRow(french[0], SINGLE_FORMAT, { decimalSeparators: seps }))
).toBe(1234);
// Without the column verdict, the same cell reads as 1.234.
expect(outcome(mapRow(french[0], SINGLE_FORMAT))).toBe(1.234);
});
it("looks at no column the format does not use", () => {
expect(detectAmountSeparators(rows, SINGLE_FORMAT).has(3)).toBe(false);
});
});
// ---------------------------------------------------------------------------
// Static guards on useImportWizard.
//

167
src/utils/importFormat.ts
View file

@ -23,8 +23,15 @@ import type {
ImportFormat,
ImportFormatRow,
ImportFormatRowInput,
ParsedRow,
SignConvention,
} from "../shared/types";
import {
detectDecimalSeparator,
parseFrenchAmount,
type DecimalSeparator,
} from "./amountParser";
import { parseDate } from "./dateParser";
/**
* Values the application can actually map. The database `CHECK` on
@ -185,3 +192,163 @@ export function clearMappingForMode(
...(mapping.amount !== undefined ? { amount: mapping.amount } : {}),
};
}
// ---------------------------------------------------------------------------
// Row mapping — one CSV row + one format -> one `ParsedRow` (#325)
// ---------------------------------------------------------------------------
/**
* i18n keys for the ways a single row can fail. They used to be raw English
* literals rendered straight into the preview table; they are keys now so the
* strings live in the locale files like every other displayed text. Resolve
* them with `isRowErrorKey` before calling `t()` — an `ImportReport` also
* carries raw exception messages, which must never be fed to the translator.
*/
export const ROW_ERROR_KEYS = {
invalidDate: "import.rowErrors.invalidDate",
invalidAmount: "import.rowErrors.invalidAmount",
amountColumnNotMapped: "import.rowErrors.amountColumnNotMapped",
parseError: "import.rowErrors.parseError",
} as const;
export type RowErrorKey = (typeof ROW_ERROR_KEYS)[keyof typeof ROW_ERROR_KEYS];
const ROW_ERROR_VALUES: readonly string[] = Object.values(ROW_ERROR_KEYS);
/** True when the string is one of ours and can safely be translated. */
export function isRowErrorKey(value: string): value is RowErrorKey {
return ROW_ERROR_VALUES.includes(value);
}
export interface MapRowOptions {
/** Position of the row in the whole import. Defaults to 0. */
rowIndex?: number;
/** File the row came from, carried through to the report. */
sourceFilename?: string;
/**
* Decimal separator arbitrated at COLUMN level, keyed by column index — see
* `detectAmountSeparators`. A column absent from the map keeps the per-cell
* default.
*/
decimalSeparators?: ReadonlyMap<number, DecimalSeparator>;
}
/**
* Map one raw CSV row to a `ParsedRow` under a given format. PURE: no React, no
* I/O, no throw — every failure comes back as `error` on the row.
*
* This rule used to live inside a `useCallback` of `useImportWizard`, which is
* why the corpus tests had to keep a hand-written mirror of it and why the
* detection score (#328) and the signed preview (#329) would each have had to
* re-implement it. One implementation, three consumers.
*
* The two-column rule is `credit - debit` on MAGNITUDES. It used to be
* `isNaN(credit) ? -debit : credit`, so a bank writing `0,00` in the unused
* column — which is common — made every debit import as 0,00 and pass
* validation, because `isNaN(0)` is false. Subtraction needs no special case
* for a `0,00` cell: zero is the identity. `Math.abs` implements the documented
* 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.
*/
export function mapRow(
raw: string[],
format: ImportFormat,
options: MapRowOptions = {}
): ParsedRow {
const mapping = format.columnMapping;
const base = {
rowIndex: options.rowIndex ?? 0,
raw,
...(options.sourceFilename !== undefined
? { sourceFilename: options.sourceFilename }
: {}),
};
const fail = (error: RowErrorKey): ParsedRow => ({
...base,
parsed: null,
error,
});
const readAmount = (col: number): number =>
parseFrenchAmount(raw[col]?.trim() ?? "", {
decimalSeparator: options.decimalSeparators?.get(col),
});
// 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
// per-row problem.
const debitMapped = mapping.debitAmount !== undefined;
const creditMapped = mapping.creditAmount !== undefined;
if (format.amountMode === "debit_credit") {
if (!debitMapped && !creditMapped) {
return fail(ROW_ERROR_KEYS.amountColumnNotMapped);
}
} else if (mapping.amount === undefined) {
return fail(ROW_ERROR_KEYS.amountColumnNotMapped);
}
// 2. Date. Kept ahead of the amount value, as it has always been.
const date = parseDate(raw[mapping.date]?.trim() || "", format.dateFormat);
if (!date) return fail(ROW_ERROR_KEYS.invalidDate);
// 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)) {
return fail(ROW_ERROR_KEYS.invalidAmount);
}
amount =
(isNaN(credit) ? 0 : Math.abs(credit)) -
(isNaN(debit) ? 0 : Math.abs(debit));
} else {
amount = readAmount(mapping.amount!);
if (isNaN(amount)) return fail(ROW_ERROR_KEYS.invalidAmount);
if (format.signConvention === "positive_expense") amount = -amount;
}
return {
...base,
parsed: {
date,
description: raw[mapping.description]?.trim() || "",
amount,
},
};
}
/**
* Arbitrate the decimal separator of every amount column the format declares,
* over the whole set of data rows.
*
* `1.234` is 1234 in a French column and 1.234 in an English one, and no rule
* applied to that cell ALONE can tell — but a sibling cell reading `84,32`
* settles it for the column. Detection deliberately stays out of this (it runs
* before a column is known to be an amount column at all); the verdict is
* applied where the value actually becomes a transaction.
*/
export function detectAmountSeparators(
rows: readonly string[][],
format: ImportFormat
): Map<number, DecimalSeparator> {
const mapping = format.columnMapping;
const columns =
format.amountMode === "debit_credit"
? [mapping.debitAmount, mapping.creditAmount]
: [mapping.amount];
const result = new Map<number, DecimalSeparator>();
for (const col of columns) {
if (col === undefined) continue;
const verdict = detectDecimalSeparator(
rows.map((row) => row[col] ?? "")
);
if (verdict) result.set(col, verdict);
}
return result;
}