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

Author	SHA1	Message	Date
le king fu	7b6f063094	fix(import): read amounts with an anchored parser and a real debit/credit rule All checks were successful PR Check — Frontend / frontend (pull_request) Successful in 1m44s Details Three ways an imported amount could be silently wrong, all of them passing validation, all of them fixed here. The two-column rule was `isNaN(credit) ? -debit : credit`, so the credit always won. Many banks write `0,00` in the unused column rather than leaving it empty, and `isNaN(0)` is false -- every debit of such a file imported as 0,00 and the expense simply vanished, with no error anywhere. The rule is `credit - debit` on magnitudes now, which needs no special case for a `0,00` cell (zero is the identity of the subtraction) and implements the documented convention even when an export negates its debits. A row unreadable in BOTH columns is an error instead of a free 0,00 transaction. `parseFrenchAmount` ended on `parseFloat`, which returns the longest valid PREFIX instead of rejecting. Measured before the fix: `"50,00-"` -> 5000, `"1 234,56 CR"` -> 123456, `"100,00 CAD"` -> 10000. A factor-100 error, and it passes `isNaN`, so those rows counted as VALID everywhere downstream -- which would have defeated the signed preview (#329), the safety net of the whole chantier. Validation is anchored over the whole normalized string now and any residual character yields NaN. NaN, not a rescued magnitude, for a trailing `CR`/`DB` or currency code. Two reasons: `CR`/`DB` carry a DIRECTION, so returning a magnitude for both would trade a loud failure for a silent SIGN error (the D/C-indicator shape is refused upstream by design, #328); and `"100,00 CAD"` is structurally identical to `"2025 Montant"`, so whitelisting a trailing word to rescue the first re-blinds `detectHeader` on the second. Two accounting forms ARE legitimate and supported: parentheses `(50,00)` and a trailing sign `50,00-`. The `?? 0` fallbacks read column 0 -- usually the date -- when the mapping was incomplete. An unmapped amount column is an explicit row error now, reported ahead of any per-row problem since it is a format error affecting every row. `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. `detectDecimalSeparator` arbitrates from the decisive siblings of the column and `parseFrenchAmount` takes the verdict as an option. Detection deliberately stays out of it: it runs before a column is known to be an amount column at all, so the verdict is applied where the value actually becomes a transaction. The rule itself moves out of the hook as a pure `mapRow(raw, format)` in `importFormat.ts`. That is what lets the corpus tests run the REAL rule -- the hand-written mirror in `csvAutoDetect.test.ts` and the static guard pinning five `parseFilesInternal` expressions are both deleted -- and what stops the detection score (#328) and the signed preview (#329) each re-implementing it. Hardening is global: the parser is shared by 11 call sites, 8 in `csvAutoDetect.ts` and 3 in the holdings CSV import (#245), where a price cell `150,25 CAD` used to store 15025. It is refused now and `buildDetailedLines` raises on the empty price. An unreadable QUANTITY was worse -- coerced to 0, so a zero-value position saved in silence; the draft keeps the offending text instead and the existing `snapshot_priced_quantity_required` fires. Row errors become i18n keys (`import.rowErrors.*`) rather than the raw English literals rendered straight into the preview table, since this adds a user-visible string. The report table also carries raw exception messages, so both render sites resolve through `isRowErrorKey` and never feed `t()` anything that is not ours. Test churn, per link 1's handoff (update the expectation, drop the marker, never delete the test): the three `#325` KNOWN DEFECT blocks in `amountParser.test.ts` flip, plus `unused-column-zero` in `csvAutoDetect.test.ts`. One block tagged `#328` flips too -- `header-numeric-label`, whose own comment reads "#328 adds a lexical signal to detectHeader, and #325 anchors the parser [...] either fix closes this". The anchored parser landed first. The other `#328` blocks (`debit-credit-reversed`, `absolute-indicator`) and the `#329` block are verified unchanged. CHANGELOG and docs stay centralized in the last link of the stack, as the plan specifies. 989 vitest (963 before), tsc + vite build clean, cargo check clean. No DB migration. Resolves #325	2026-08-13 13:37:06 -04:00

Author

SHA1

Message

Date

le king fu

7b6f063094

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

PR Check — Frontend / frontend (pull_request) Successful in 1m44s

Details

Three ways an imported amount could be silently wrong, all of them passing
validation, all of them fixed here.

The two-column rule was `isNaN(credit) ? -debit : credit`, so the credit always
won. Many banks write `0,00` in the unused column rather than leaving it empty,
and `isNaN(0)` is false -- every debit of such a file imported as 0,00 and the
expense simply vanished, with no error anywhere. The rule is `credit - debit` on
magnitudes now, which needs no special case for a `0,00` cell (zero is the
identity of the subtraction) and implements the documented convention even when
an export negates its debits. A row unreadable in BOTH columns is an error
instead of a free 0,00 transaction.

`parseFrenchAmount` ended on `parseFloat`, which returns the longest valid
PREFIX instead of rejecting. Measured before the fix: `"50,00-"` -> 5000,
`"1 234,56 CR"` -> 123456, `"100,00 CAD"` -> 10000. A factor-100 error, and it
passes `isNaN`, so those rows counted as VALID everywhere downstream -- which
would have defeated the signed preview (#329), the safety net of the whole
chantier. Validation is anchored over the whole normalized string now and any
residual character yields NaN.

NaN, not a rescued magnitude, for a trailing `CR`/`DB` or currency code. Two
reasons: `CR`/`DB` carry a DIRECTION, so returning a magnitude for both would
trade a loud failure for a silent SIGN error (the D/C-indicator shape is refused
upstream by design, #328); and `"100,00 CAD"` is structurally identical to
`"2025 Montant"`, so whitelisting a trailing word to rescue the first re-blinds
`detectHeader` on the second. Two accounting forms ARE legitimate and supported:
parentheses `(50,00)` and a trailing sign `50,00-`.

The `?? 0` fallbacks read column 0 -- usually the date -- when the mapping was
incomplete. An unmapped amount column is an explicit row error now, reported
ahead of any per-row problem since it is a format error affecting every row.

`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. `detectDecimalSeparator` arbitrates from
the decisive siblings of the column and `parseFrenchAmount` takes the verdict as
an option. Detection deliberately stays out of it: it runs before a column is
known to be an amount column at all, so the verdict is applied where the value
actually becomes a transaction.

The rule itself moves out of the hook as a pure `mapRow(raw, format)` in
`importFormat.ts`. That is what lets the corpus tests run the REAL rule -- the
hand-written mirror in `csvAutoDetect.test.ts` and the static guard pinning five
`parseFilesInternal` expressions are both deleted -- and what stops the
detection score (#328) and the signed preview (#329) each re-implementing it.

Hardening is global: the parser is shared by 11 call sites, 8 in
`csvAutoDetect.ts` and 3 in the holdings CSV import (#245), where a price cell
`150,25 CAD` used to store 15025. It is refused now and `buildDetailedLines`
raises on the empty price. An unreadable QUANTITY was worse -- coerced to 0, so a
zero-value position saved in silence; the draft keeps the offending text instead
and the existing `snapshot_priced_quantity_required` fires.

Row errors become i18n keys (`import.rowErrors.*`) rather than the raw English
literals rendered straight into the preview table, since this adds a
user-visible string. The report table also carries raw exception messages, so
both render sites resolve through `isRowErrorKey` and never feed `t()` anything
that is not ours.

Test churn, per link 1's handoff (update the expectation, drop the marker, never
delete the test): the three `#325` KNOWN DEFECT blocks in `amountParser.test.ts`
flip, plus `unused-column-zero` in `csvAutoDetect.test.ts`. One block tagged
`#328` flips too -- `header-numeric-label`, whose own comment reads "#328 adds a
lexical signal to detectHeader, and #325 anchors the parser [...] either fix
closes this". The anchored parser landed first. The other `#328` blocks
(`debit-credit-reversed`, `absolute-indicator`) and the `#329` block are
verified unchanged. CHANGELOG and docs stay centralized in the last link of the
stack, as the plan specifies.

989 vitest (963 before), tsc + vite build clean, cargo check clean. No DB
migration.

Resolves #325

2026-08-13 13:37:06 -04:00