maximus/Simpl-Resultat

Fork 0

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

maximus commented

2026-08-13 17:37:48 +00:00

Owner

Link 4 of the ten-link import-format stack. Based on issue-324-persist-import-format, not on main.

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

The debit/credit rule

useImportWizard.ts:509 was amount = isNaN(credit) ? -(isNaN(debit) ? 0 : debit) : credit — the credit always won. Many banks write 0,00 in the unused column rather than leaving it empty, and isNaN(0) is false, so every debit of such a file imported as 0,00 and the expense simply vanished with no error anywhere.

It is credit - debit on magnitudes now. That needs no special case for a 0,00 cell (zero is the identity of the subtraction) and Math.abs implements the documented convention even when an export negates its debits. A row unreadable in both columns is an error rather than a free 0,00 transaction.

The anchored parser

parseFrenchAmount ended on parseFloat, which returns the longest valid prefix instead of rejecting. Measured before the fix:

input	before	after
`50,00-`	5000	-50
`1 234,56 CR`	123456	NaN
`100,00 CAD`	10000	NaN
`(50,00)`	NaN	-50
`2024 Montant`	2024	NaN

A factor-100 error that passes isNaN, so those rows counted as VALID everywhere downstream — which would have defeated the signed preview of #329, the safety net of the whole chantier.

Why NaN and not the rescued magnitude for CR / CAD, where link 1's test titles said "should be 1234.56" / "should be 100" (its own block header said NaN, and so do the issue and the plan):

CR/DB carry a direction, not noise. Returning a magnitude for both 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 upstream by design (#328).
"100,00 CAD" and "2025 Montant" are structurally identical (digits, space, letters). Whitelisting a trailing word to rescue the first re-blinds detectHeader on the second — the defect link 1 measured.

Parentheses (50,00) and a trailing sign 50,00- are supported as the two legitimate accounting forms.

The `?? 0` fallbacks

They 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 identically.

Column-level separator

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; parseFrenchAmount takes the verdict as an option; detectAmountSeparators runs it once per file over the columns the format declares. 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.

`mapRow` extracted

The rule leaves the hook as a pure mapRow(raw, format, options?) in importFormat.ts. The prescribed two-argument call works as specified; the optional third carries what only the caller knows (rowIndex, sourceFilename, the column separators). This is what lets the corpus tests run the real rule — the hand-written mapCorpusRow mirror and the static guard pinning five parseFilesInternal expressions are both deleted, as link 1 and link 3 instructed — and what stops #328's score and #329's preview each re-implementing it.

Global hardening, including holdings

The parser is shared by 11 call sites: 8 in csvAutoDetect.ts, 3 in the #245 holdings CSV import. 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 typed snapshot_priced_quantity_required fires; one unreadable lot taints the merged quantity rather than being summed as 0. Covered by two new tests.

i18n

Row errors were raw English literals rendered straight into the preview table. Since this adds a user-visible string ("amount column not mapped"), all four become import.rowErrors.* keys in both locales. 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:

Flipped, tagged #325: parseFloat prefix scan, unsupported accounting forms, separator arbitrated per cell (amountParser.test.ts); unused column filled with 0,00 (csvAutoDetect.test.ts).
Flipped, tagged #328: header cell starting with digits. Its own comment reads "#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." The anchored parser landed first; the block records that and leaves #328 its lexical signal for header cells carrying a bare number.
Verified unchanged: debit-credit-reversed and absolute-indicator (#328), all-positive (#329), and link 3's useImportWizard source guards in importFormat.test.ts (no callback was renamed or reordered).

989 vitest (963 before), tsc + vite build clean, cargo check clean. No DB migration. CHANGELOG and docs/ stay centralized in the last link of the stack, as spec-plan-import-csv-format.md specifies.

Resolves #325

Generated autonomously by /autopilot run of 2026-08-13

Link 4 of the ten-link import-format stack. **Based on `issue-324-persist-import-format`, not on `main`.** Three ways an imported amount could be silently wrong, all of them passing validation. ## The debit/credit rule `useImportWizard.ts:509` was `amount = isNaN(credit) ? -(isNaN(debit) ? 0 : debit) : credit` — the credit always won. Many banks write `0,00` in the unused column rather than leaving it empty, and `isNaN(0)` is false, so **every debit of such a file imported as 0,00** and the expense simply vanished with no error anywhere. It is `credit - debit` on magnitudes now. That needs no special case for a `0,00` cell (zero is the identity of the subtraction) and `Math.abs` implements the documented convention even when an export negates its debits. A row unreadable in **both** columns is an error rather than a free 0,00 transaction. ## The anchored parser `parseFrenchAmount` ended on `parseFloat`, which returns the longest valid **prefix** instead of rejecting. Measured before the fix: | input | before | after | |---|---|---| | `50,00-` | 5000 | **-50** | | `1 234,56 CR` | 123456 | **NaN** | | `100,00 CAD` | 10000 | **NaN** | | `(50,00)` | NaN | **-50** | | `2024 Montant` | 2024 | **NaN** | A factor-100 error that passes `isNaN`, so those rows counted as VALID everywhere downstream — which would have defeated the signed preview of #329, the safety net of the whole chantier. **Why NaN and not the rescued magnitude** for `CR` / `CAD`, where link 1's test titles said "should be 1234.56" / "should be 100" (its own block header said NaN, and so do the issue and the plan): 1. `CR`/`DB` carry a **direction**, not noise. Returning a magnitude for both 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 upstream by design (#328). 2. `"100,00 CAD"` and `"2025 Montant"` are structurally identical (digits, space, letters). Whitelisting a trailing word to rescue the first **re-blinds `detectHeader`** on the second — the defect link 1 measured. Parentheses `(50,00)` and a trailing sign `50,00-` are supported as the two legitimate accounting forms. ## The `?? 0` fallbacks They 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 identically. ## Column-level separator `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; `parseFrenchAmount` takes the verdict as an option; `detectAmountSeparators` runs it once per file over the columns the format declares. 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. ## `mapRow` extracted The rule leaves the hook as a pure `mapRow(raw, format, options?)` in `importFormat.ts`. The prescribed two-argument call works as specified; the optional third carries what only the caller knows (`rowIndex`, `sourceFilename`, the column separators). This is what lets the corpus tests run the **real** rule — the hand-written `mapCorpusRow` mirror and the static guard pinning five `parseFilesInternal` expressions are both deleted, as link 1 and link 3 instructed — and what stops #328's score and #329's preview each re-implementing it. ## Global hardening, including holdings The parser is shared by 11 call sites: 8 in `csvAutoDetect.ts`, 3 in the #245 holdings CSV import. 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 typed `snapshot_priced_quantity_required` fires; one unreadable lot taints the merged quantity rather than being summed as 0. Covered by two new tests. ## i18n Row errors were raw English literals rendered straight into the preview table. Since this adds a user-visible string ("amount column not mapped"), all four become `import.rowErrors.*` keys in both locales. 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: - **Flipped, tagged `#325`:** `parseFloat prefix scan`, `unsupported accounting forms`, `separator arbitrated per cell` (`amountParser.test.ts`); `unused column filled with 0,00` (`csvAutoDetect.test.ts`). - **Flipped, tagged `#328`:** `header cell starting with digits`. Its own comment reads *"#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.**"* The anchored parser landed first; the block records that and leaves #328 its lexical signal for header cells carrying a **bare** number. - **Verified unchanged:** `debit-credit-reversed` and `absolute-indicator` (`#328`), `all-positive` (`#329`), and link 3's `useImportWizard` source guards in `importFormat.test.ts` (no callback was renamed or reordered). **989 vitest** (963 before), `tsc` + `vite build` clean, `cargo check` clean. No DB migration. CHANGELOG and `docs/` stay centralized in the last link of the stack, as `spec-plan-import-csv-format.md` specifies. Resolves #325 --- Generated autonomously by /autopilot run of 2026-08-13

maximus added 1 commit 2026-08-13 17:37:49 +00:00

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

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

Details

7b6f063094

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

maximus added the

autopilot:pending-human

label 2026-08-13 17:38:03 +00:00

maximus commented

2026-08-14 15:29:39 +00:00

Author

Owner

Revue adversariale — PR #336

Verdict : REQUEST_CHANGES — 1 blocage, 6 suggestions.

Résumé

Le fond de la PR est juste et bien argumenté. J'ai rejoué le parseur ancré et mapRow hors du dépôt (bundle esbuild des fichiers de la branche) plutôt que de lire le diff : les trois erreurs facteur-100 annoncées sont fermées ("50,00-" → -50, "1 234,56 CR" → NaN, "100,00 CAD" → NaN), les parenthèses comptables et le signe traînant marchent, credit − debit sur des magnitudes rend le cas 0,00 trivial, et l'extraction de mapRow en fonction pure est le bon geste — les tests corpus tournent enfin la vraie règle.

Sur la décision contestée (CR / CAD → NaN plutôt que la magnitude sauvée), je tranche pour le worker. Les deux arguments tiennent à la vérification :

CR/DB porte une direction. Sauver la magnitude échangerait un échec bruyant contre une erreur de signe silencieuse — la classe de bug que ce chantier existe pour retirer.
"100,00 CAD" et "2025 Montant" sont structurellement identiques, et c'est mesurable : le même ancrage qui refuse le premier fait passer la fixture header-numeric-label de hasHeader: false à true. Une whitelist de mot traînant ré-aveuglerait detectHeader.

Et le refus est découvrable : ligne surlignée en rouge dans FilePreviewTable, message traduit, ligne brute affichée à côté, errorCount puis la liste complète dans ImportReportPanel. Les titres de test du maillon 1 avaient tort, l'en-tête du même bloc avait raison ; la PR le documente correctement.

Vérifié sans reproche par ailleurs : le flux holdings dégrade proprement (le texte fautif reste dans la cellule, le total live saute la ligne au lieu d'afficher NaN, buildDetailedLines lève un BalanceServiceError typé attrapé par save() et traduit dans les deux locales) ; les gardes statiques du maillon 3 dans importFormat.test.ts sont intactes ; la garde supprimée est remplacée par quatre assertions négatives plus fortes ; 4 clés i18n présentes en FR et EN ; aucun skip/only ; aucune migration touchée ; CI frontend verte.

Un seul blocage, mais il porte sur l'invariant central de la PR.

Blocages

1. src/utils/importFormat.ts:303-308 — une cellule illisible à côté du remplissage 0,00 importe 0,00 sans aucune erreur.

En mode debit_credit, la règle n'échoue que si les deux cellules sont illisibles :

if (isNaN(debit) && isNaN(credit)) {
  return fail(ROW_ERROR_KEYS.invalidAmount);
}
amount =
  (isNaN(credit) ? 0 : Math.abs(credit)) -
  (isNaN(debit)  ? 0 : Math.abs(debit));

Quand une seule des deux est illisible, elle est traitée en silence comme absente. Or la colonne inutilisée porte 0,00 — c'est précisément la forme de fichier que cette PR existe pour corriger — donc la sœur parse toujours, et la ligne sort à amount = 0, parsed non nul, comptée VALIDE.

Rejoué sur la fixture unused-column-zero.csv de la PR, seul changement : un suffixe de devise sur la colonne utilisée (une forme que cette PR vient elle-même de rendre illisible).

05/01/2025;EPICERIE METRO SAINTE-FOY;84,32 CAD;0,00
15/01/2025;DEPOT PAIE EMPLOYEUR;0,00;1250,00 CAD
...
amounts : [0, 0, 0, 0, 0, 0]
lignes signalées en erreur : 0 / 6
attendu : [-84.32, 1250, -142.18, -56.75, 300, -6.95]

Six transactions réelles entrent au grand livre à 0,00 $, sans une seule erreur. C'est le symptôme exact décrit dans le corps de la PR — « every debit of such a file imported as 0,00 and the expense simply vanished with no error anywhere » — sur la même forme de fichier, par une autre porte. Le docstring de mapRow (importFormat.ts:253) énonce d'ailleurs le bon principe, plus large que le code : « an amount nobody could read must not enter the ledger as a free transaction ».

Cette PR amplifie le trou au lieu de le subir. L'arbitrage par colonne qu'elle introduit fabrique des NaN qui n'existaient pas :

verdicts de colonne : [ [2, ','], [3, ','] ]
  EPICERIE  debit=84,32      credit=0,00     -> -84.32
  LOYER     debit=12,50      credit=0,00     -> -12.5
  AUTO      debit=1,234.56   credit=0,00     ->  0      <-- avalée
  PAIE      debit=0,00       credit=1250,00  ->  1250

La même ligne avec une cellule vide au lieu de 0,00 échoue correctement (ERR import.rowErrors.invalidAmount). Seul le remplissage 0,00 masque la panne. Autres déclencheurs mesurés, tous à 0 erreur : marqueur de note 84,32*, indicateur 84,32 DB, code de devise.

Correctif : une cellule mappée non vide qui ne parse pas est une erreur, quelle que soit sa sœur. Une cellule vide reste « absente », donc le correctif 0,00 de la PR et les fixtures debit-credit / unused-column-zero passent inchangés.

const rawDebit  = debitMapped  ? (raw[mapping.debitAmount!]  ?? "").trim() : "";
const rawCredit = creditMapped ? (raw[mapping.creditAmount!] ?? "").trim() : "";
if ((rawDebit && isNaN(debit)) || (rawCredit && isNaN(credit))) {
  return fail(ROW_ERROR_KEYS.invalidAmount);
}
if (!rawDebit && !rawCredit) return fail(ROW_ERROR_KEYS.invalidAmount);

Test de régression à ajouter — il échoue sur la branche actuelle :

it("errors when ONE column is unreadable and the other is a 0,00 filler", () => {
  expect(outcome(mapRow(["05/01/2025", "X", "84,32 CAD", "0,00"], DC_FORMAT)))
    .toBe(ROW_ERROR_KEYS.invalidAmount);
  expect(outcome(mapRow(["05/01/2025", "X", "0,00", "1250,00 CR"], DC_FORMAT)))
    .toBe(ROW_ERROR_KEYS.invalidAmount);
});

Le bloc mapRow — debit/credit is a subtraction ne couvre aujourd'hui que ["", ""] et ["n/a", "-"] : les deux cas où les deux cellules tombent. Le cas mixte n'est testé nulle part, ce qui explique que la CI soit verte.

Suggestions (non bloquantes)

1. importFormat.ts:340 — arbitrer débit et crédit ensemble.
Les deux colonnes sont la même grandeur, écrite par la même banque, dans la même notation ; elles reçoivent pourtant deux verdicts indépendants. Une colonne débit entièrement ambiguë (1.234, 2.500) ne reçoit aucun verdict alors que la colonne crédit voisine prouve la convention française — le débit se lit alors 1,234 au lieu de 1234, silencieusement. Mutualiser les cellules des deux colonnes en un seul vote ferme ce cas.

2. amountParser.ts:102 vs :106 — asymétrie du nombre de décimales.
La branche virgule est plafonnée à 2 décimales (decimalRe(",", 2)), la branche point ne l'est pas (decimalRe(".")). Conséquence : "1,2345" → NaN mais "1.2345" → 1.2345. Même plafond dans detectDecimalSeparator (:186), donc une colonne française à 3+ décimales ne vote pour rien et chacune de ses cellules est refusée. Ça mord sur l'import de titres (#245) : quantités de parts de fonds et de crypto sont couramment à 3-8 décimales. Le comportement précédent était pire (silencieusement faux), donc pas de régression, mais le plafond mériterait d'être symétrique.

3. Fichier à convention réellement mixte.
Le verdict majoritaire s'applique en silence et les cellules minoritaires deviennent des erreurs de ligne isolées ; rien ne dit à l'utilisateur qu'un verdict de colonne a été rendu ni lequel. Une mention dans l'aperçu (« colonne lue en notation française ») rendrait la sévérité lisible. Ex. : 3 cellules 84,32 + 1 cellule 1,234.56 → la dernière refusée sans explication.

4. useSnapshotEditor.ts:634 — libellé du refus de quantité.
Le code réutilisé est snapshot_priced_quantity_required, rendu « La quantité est obligatoire pour les comptes cotés. » alors que le cas est « quantité présente mais illisible ». Le texte fautif reste visible dans la cellule, donc c'est actionnable, mais un code dédié serait plus clair.

5. Documentation du refus.
Un relevé dont les montants portent un suffixe de devise devient entièrement non importable, sans échappatoire dans l'UI. C'est le bon défaut, mais il faut que le CHANGELOG et le guide utilisateur du dernier maillon le disent explicitement — et un suivi mériterait d'être ouvert pour une option de colonne « suffixe à ignorer », ou pour le rattachement au support d'indicateur D/C de #328.

6. Ligne fantôme.
debit="0,00", credit="" importe une transaction à 0,00 sans erreur. Préexistant et hors périmètre du blocage ci-dessus, mais c'est une ligne qui n'a aucune raison d'entrer.

Revue adversariale — parseur et mapRow rejoués hors dépôt sur les fichiers de la branche, pas seulement lus.

## Revue adversariale — PR #336 **Verdict : REQUEST_CHANGES** — 1 blocage, 6 suggestions. ### Résumé Le fond de la PR est juste et bien argumenté. J'ai rejoué le parseur ancré et `mapRow` hors du dépôt (bundle esbuild des fichiers de la branche) plutôt que de lire le diff : les trois erreurs facteur-100 annoncées sont fermées (`"50,00-"` → -50, `"1 234,56 CR"` → NaN, `"100,00 CAD"` → NaN), les parenthèses comptables et le signe traînant marchent, `credit − debit` sur des magnitudes rend le cas `0,00` trivial, et l'extraction de `mapRow` en fonction pure est le bon geste — les tests corpus tournent enfin la vraie règle. **Sur la décision contestée (`CR` / `CAD` → NaN plutôt que la magnitude sauvée), je tranche pour le worker.** Les deux arguments tiennent à la vérification : - `CR`/`DB` porte une direction. Sauver la magnitude échangerait un échec bruyant contre une erreur de **signe** silencieuse — la classe de bug que ce chantier existe pour retirer. - `"100,00 CAD"` et `"2025 Montant"` sont structurellement identiques, et c'est mesurable : le même ancrage qui refuse le premier fait passer la fixture `header-numeric-label` de `hasHeader: false` à `true`. Une whitelist de mot traînant ré-aveuglerait `detectHeader`. Et le refus est **découvrable** : ligne surlignée en rouge dans `FilePreviewTable`, message traduit, ligne brute affichée à côté, `errorCount` puis la liste complète dans `ImportReportPanel`. Les titres de test du maillon 1 avaient tort, l'en-tête du même bloc avait raison ; la PR le documente correctement. Vérifié sans reproche par ailleurs : le flux holdings dégrade proprement (le texte fautif reste dans la cellule, le total live saute la ligne au lieu d'afficher `NaN`, `buildDetailedLines` lève un `BalanceServiceError` typé attrapé par `save()` et traduit dans les deux locales) ; les gardes statiques du maillon 3 dans `importFormat.test.ts` sont intactes ; la garde supprimée est remplacée par quatre assertions négatives plus fortes ; 4 clés i18n présentes en FR **et** EN ; aucun `skip`/`only` ; aucune migration touchée ; CI frontend verte. Un seul blocage, mais il porte sur l'invariant central de la PR. --- ### Blocages **1. `src/utils/importFormat.ts:303-308` — une cellule illisible à côté du remplissage `0,00` importe 0,00 sans aucune erreur.** En mode `debit_credit`, la règle n'échoue que si les **deux** cellules sont illisibles : ```ts if (isNaN(debit) && isNaN(credit)) { return fail(ROW_ERROR_KEYS.invalidAmount); } amount = (isNaN(credit) ? 0 : Math.abs(credit)) - (isNaN(debit) ? 0 : Math.abs(debit)); ``` Quand une seule des deux est illisible, elle est traitée en silence comme **absente**. Or la colonne inutilisée porte `0,00` — c'est précisément la forme de fichier que cette PR existe pour corriger — donc la sœur parse toujours, et la ligne sort à `amount = 0`, `parsed` non nul, comptée **VALIDE**. Rejoué sur la fixture `unused-column-zero.csv` de la PR, seul changement : un suffixe de devise sur la colonne utilisée (une forme que cette PR vient elle-même de rendre illisible). ``` 05/01/2025;EPICERIE METRO SAINTE-FOY;84,32 CAD;0,00 15/01/2025;DEPOT PAIE EMPLOYEUR;0,00;1250,00 CAD ... amounts : [0, 0, 0, 0, 0, 0] lignes signalées en erreur : 0 / 6 attendu : [-84.32, 1250, -142.18, -56.75, 300, -6.95] ``` Six transactions réelles entrent au grand livre à 0,00 $, sans une seule erreur. C'est le symptôme exact décrit dans le corps de la PR — « every debit of such a file imported as 0,00 and the expense simply vanished with no error anywhere » — sur la même forme de fichier, par une autre porte. Le docstring de `mapRow` (`importFormat.ts:253`) énonce d'ailleurs le bon principe, plus large que le code : « an amount nobody could read must not enter the ledger as a free transaction ». **Cette PR amplifie le trou au lieu de le subir.** L'arbitrage par colonne qu'elle introduit *fabrique* des NaN qui n'existaient pas : ``` verdicts de colonne : [ [2, ','], [3, ','] ] EPICERIE debit=84,32 credit=0,00 -> -84.32 LOYER debit=12,50 credit=0,00 -> -12.5 AUTO debit=1,234.56 credit=0,00 -> 0 <-- avalée PAIE debit=0,00 credit=1250,00 -> 1250 ``` La même ligne avec une cellule **vide** au lieu de `0,00` échoue correctement (`ERR import.rowErrors.invalidAmount`). Seul le remplissage `0,00` masque la panne. Autres déclencheurs mesurés, tous à 0 erreur : marqueur de note `84,32*`, indicateur `84,32 DB`, code de devise. Correctif : une cellule mappée **non vide** qui ne parse pas est une erreur, quelle que soit sa sœur. Une cellule vide reste « absente », donc le correctif `0,00` de la PR et les fixtures `debit-credit` / `unused-column-zero` passent inchangés. ```ts const rawDebit = debitMapped ? (raw[mapping.debitAmount!] ?? "").trim() : ""; const rawCredit = creditMapped ? (raw[mapping.creditAmount!] ?? "").trim() : ""; if ((rawDebit && isNaN(debit)) || (rawCredit && isNaN(credit))) { return fail(ROW_ERROR_KEYS.invalidAmount); } if (!rawDebit && !rawCredit) return fail(ROW_ERROR_KEYS.invalidAmount); ``` Test de régression à ajouter — il échoue sur la branche actuelle : ```ts it("errors when ONE column is unreadable and the other is a 0,00 filler", () => { expect(outcome(mapRow(["05/01/2025", "X", "84,32 CAD", "0,00"], DC_FORMAT))) .toBe(ROW_ERROR_KEYS.invalidAmount); expect(outcome(mapRow(["05/01/2025", "X", "0,00", "1250,00 CR"], DC_FORMAT))) .toBe(ROW_ERROR_KEYS.invalidAmount); }); ``` Le bloc `mapRow — debit/credit is a subtraction` ne couvre aujourd'hui que `["", ""]` et `["n/a", "-"]` : les deux cas où les **deux** cellules tombent. Le cas mixte n'est testé nulle part, ce qui explique que la CI soit verte. --- ### Suggestions (non bloquantes) **1. `importFormat.ts:340` — arbitrer débit et crédit ensemble.** Les deux colonnes sont la même grandeur, écrite par la même banque, dans la même notation ; elles reçoivent pourtant deux verdicts indépendants. Une colonne débit entièrement ambiguë (`1.234`, `2.500`) ne reçoit aucun verdict alors que la colonne crédit voisine prouve la convention française — le débit se lit alors 1,234 au lieu de 1234, silencieusement. Mutualiser les cellules des deux colonnes en un seul vote ferme ce cas. **2. `amountParser.ts:102` vs `:106` — asymétrie du nombre de décimales.** La branche virgule est plafonnée à 2 décimales (`decimalRe(",", 2)`), la branche point ne l'est pas (`decimalRe(".")`). Conséquence : `"1,2345"` → NaN mais `"1.2345"` → 1.2345. Même plafond dans `detectDecimalSeparator` (`:186`), donc une colonne française à 3+ décimales ne vote pour rien et chacune de ses cellules est refusée. Ça mord sur l'import de titres (#245) : quantités de parts de fonds et de crypto sont couramment à 3-8 décimales. Le comportement précédent était pire (silencieusement faux), donc pas de régression, mais le plafond mériterait d'être symétrique. **3. Fichier à convention réellement mixte.** Le verdict majoritaire s'applique en silence et les cellules minoritaires deviennent des erreurs de ligne isolées ; rien ne dit à l'utilisateur qu'un verdict de colonne a été rendu ni lequel. Une mention dans l'aperçu (« colonne lue en notation française ») rendrait la sévérité lisible. Ex. : 3 cellules `84,32` + 1 cellule `1,234.56` → la dernière refusée sans explication. **4. `useSnapshotEditor.ts:634` — libellé du refus de quantité.** Le code réutilisé est `snapshot_priced_quantity_required`, rendu « La quantité est obligatoire pour les comptes cotés. » alors que le cas est « quantité présente mais illisible ». Le texte fautif reste visible dans la cellule, donc c'est actionnable, mais un code dédié serait plus clair. **5. Documentation du refus.** Un relevé dont les montants portent un suffixe de devise devient entièrement non importable, sans échappatoire dans l'UI. C'est le bon défaut, mais il faut que le CHANGELOG et le guide utilisateur du dernier maillon le disent explicitement — et un suivi mériterait d'être ouvert pour une option de colonne « suffixe à ignorer », ou pour le rattachement au support d'indicateur D/C de #328. **6. Ligne fantôme.** `debit="0,00"`, `credit=""` importe une transaction à 0,00 sans erreur. Préexistant et hors périmètre du blocage ci-dessus, mais c'est une ligne qui n'a aucune raison d'entrer. --- *Revue adversariale — parseur et `mapRow` rejoués hors dépôt sur les fichiers de la branche, pas seulement lus.*

maximus commented

2026-08-14 16:03:37 +00:00

Author

Owner

Blocage levé — corrigé sur la tête de pile

Une cellule mappée non vide qui ne parse pas fait désormais échouer la ligne, quelle que soit sa sœur. Une cellule vide garde son sens (« cette colonne ne s'applique pas à cette ligne ») et contribue zéro.

Le correctif est le commit 484c4be sur issue-332-docs-adr-changelog, la tête de la pile, plutôt que sur cette branche : la chaîne se merge en --ff-only d'un seul tenant, et corriger ici aurait imposé un rebase en cascade de toute la descendance. L'attribution reste lisible — le commit porte Refs #325.

Vérifié par mutation : annuler le correctif fait rougir un test dédié. Un garde qui ne peut pas échouer ne garde rien.

Suite complète verte : 1181 vitest, build tsc + vite, cargo check.

## Blocage levé — corrigé sur la tête de pile Une cellule mappée non vide qui ne parse pas fait désormais échouer la ligne, quelle que soit sa sœur. Une cellule **vide** garde son sens (« cette colonne ne s'applique pas à cette ligne ») et contribue zéro. Le correctif est le commit `484c4be` sur `issue-332-docs-adr-changelog`, la tête de la pile, plutôt que sur cette branche : la chaîne se merge en `--ff-only` d'un seul tenant, et corriger ici aurait imposé un rebase en cascade de toute la descendance. L'attribution reste lisible — le commit porte `Refs #325`. **Vérifié par mutation** : annuler le correctif fait rougir un test dédié. Un garde qui ne peut pas échouer ne garde rien. Suite complète verte : 1181 vitest, build tsc + vite, `cargo check`.

maximus commented

2026-08-14 16:13:58 +00:00

Author

Owner

Mergée dans main en fast-forward avec le reste de la pile (tip 37b832e).

Forgejo ne détecte pas un merge local comme merged — la PR est donc fermée à la main, et l'issue liée s'est fermée automatiquement via son Resolves #N.

Tip cumulé validé avant push : 1181 vitest, 111 tests Rust, build tsc + vite. La CI ne tourne pas sur push main, cette validation locale était donc le seul filet.

Mergée dans `main` en fast-forward avec le reste de la pile (tip `37b832e`). Forgejo ne détecte pas un merge local comme *merged* — la PR est donc fermée à la main, et l'issue liée s'est fermée automatiquement via son `Resolves #N`. Tip cumulé validé avant push : **1181 vitest**, **111 tests Rust**, build tsc + vite. La CI ne tourne pas sur push `main`, cette validation locale était donc le seul filet.

maximus closed this pull request

2026-08-14 16:13:58 +00:00

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

Details