maximus/Simpl-Resultat

Fork 0

feat(import): make the preview mandatory and show what the amounts mean #339

Closed

maximus wants to merge 1 commit from issue-329-mandatory-preview into issue-328-confidence-score

maximus commented

2026-08-13 18:32:29 +00:00

Owner

Link 7 of the 10-link import-format stack. Based on issue-328-confidence-score, not on main.

Link 6 shipped a test proving a 100 % confidence score says nothing about sign: the all-positive fixture scores 1.0 while every credit imports as an expense. The score measures readability, not meaning. This link builds the control that reads meaning.

What changed

The file-preview step exists. It had been declared in ImportWizardStep since the beginning with no dispatch ever aiming at it — the optional modal had supplanted it. This is a state-machine change:

parseAndPreview parses and stops at the preview. It replaces parsePreview and parseAndCheckDuplicates (the one whose comment read "skips preview step").
checkDuplicates, dead code until now, is the preview's next button — so the rows the user validated are the rows that get checked, with no second parse in between.
ImportPage renders the step; the Aperçu / Vérifier-doublons button pair is a WizardNavigation, and duplicate-check walks back through the preview.
FilePreviewModal is removed. Editing FilePreviewTable alone would have mutated a still-live copy of it.
The step is gated by nothing, in particular not by the 0.9 threshold: a perfect score is exactly the case this catches.

The signed recap (summarizeParsedRows, pure and tested): outflows and their total, inflows and theirs, rows in error. Totals stay signed, as the amounts that would reach the ledger — magnitudes would hide the one thing the recap exposes. Computed over the whole file, never over the twenty rows displayed. A zero-amount row is filed under neither direction, deliberately; the row count printed beside the recap keeps that visible.

"Inverser les signes" (flipSignFormat) acts on the CONFIGURATION, so the correction is persisted with the source and the next file from that bank reads right on its own. In single mode it toggles the convention; in debit_credit it swaps the two column indices instead, because mapRow computes credit - debit on magnitudes and never reads the convention there — a toggle would have been inert (review finding). A half-mapped format swaps too; an unmapped column stays unmapped, never materialised as an undefined key.

ImportConfirmation now states the amount mode, the sign convention (only in the mode that applies it) and the column mapping named by header, on top of the four settings it listed.

The two inherited test obligations

The all-positive KNOWN DEFECT block: marker dropped, nothing deleted. Its three expectations still hold — an unsigned file carries no direction and detection cannot invent one — and two cases were added: the tell (six outflows, zero inflows, zero errors) and the honest limit, that flipping this file only produces its mirror image. What the flip does repair is asserted on debit-credit and signed-amount.
The score test naming #329 as its owner: updated in place. It now asserts the recap that sees what the score cannot, on the same rows, instead of promising it in a comment.

Guards

The static guards from links 3 and 6 all still pass unchanged (mapRow(raw, config, { in parseFilesInternal, one runAutoDetect(, two await detectFormatForFile(, none in the restore arm, the checkDuplicatesInternal / executeImport callback slices). New guards pin the step's transition, the absence of any callback that skips it, the flip's re-parse under the flipped config, and the deleted modal.

Checks

1088 vitest (1054 after link 6), 58 files
npx tsc --noEmit and npm run build clean
cargo check exit 0 (no Rust work)
No DB migration (v1→v17 as link 1 left it)

Deliberately not in scope

CHANGELOG.md / CHANGELOG.fr.md, docs/architecture.md, docs/guide-utilisateur.md and the docs.* keys: spec-plan-import-csv-format.md centralises them in issue 10 to keep the linear stack conflict-free, and links 1-6 touched none of them. One item for that link: docs/architecture.md:35 and CLAUDE.md both say "13 composants" for src/components/import/, which is 12 now that the modal is gone.

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

Link 7 of the 10-link import-format stack. Based on `issue-328-confidence-score`, **not** on `main`. Link 6 shipped a test proving a 100 % confidence score says nothing about sign: the `all-positive` fixture scores 1.0 while every credit imports as an expense. The score measures readability, not meaning. This link builds the control that reads meaning. ## What changed **The `file-preview` step exists.** It had been declared in `ImportWizardStep` since the beginning with no dispatch ever aiming at it — the optional modal had supplanted it. This is a state-machine change: - `parseAndPreview` parses and stops at the preview. It replaces `parsePreview` and `parseAndCheckDuplicates` (the one whose comment read "skips preview step"). - `checkDuplicates`, dead code until now, is the preview's next button — so the rows the user validated are the rows that get checked, with no second parse in between. - `ImportPage` renders the step; the Aperçu / Vérifier-doublons button pair is a `WizardNavigation`, and `duplicate-check` walks back through the preview. - `FilePreviewModal` is removed. Editing `FilePreviewTable` alone would have mutated a still-live copy of it. - The step is gated by nothing, in particular not by the 0.9 threshold: a perfect score is exactly the case this catches. **The signed recap** (`summarizeParsedRows`, pure and tested): outflows and their total, inflows and theirs, rows in error. Totals stay signed, as the amounts that would reach the ledger — magnitudes would hide the one thing the recap exposes. Computed over the whole file, never over the twenty rows displayed. A zero-amount row is filed under neither direction, deliberately; the row count printed beside the recap keeps that visible. **"Inverser les signes"** (`flipSignFormat`) acts on the CONFIGURATION, so the correction is persisted with the source and the next file from that bank reads right on its own. In `single` mode it toggles the convention; in `debit_credit` it swaps the two column indices instead, because `mapRow` computes `credit - debit` on magnitudes and never reads the convention there — a toggle would have been inert (review finding). A half-mapped format swaps too; an unmapped column stays unmapped, never materialised as an `undefined` key. **`ImportConfirmation`** now states the amount mode, the sign convention (only in the mode that applies it) and the column mapping named by header, on top of the four settings it listed. ## The two inherited test obligations - The `all-positive` **KNOWN DEFECT** block: marker dropped, nothing deleted. Its three expectations still hold — an unsigned file carries no direction and detection cannot invent one — and two cases were added: the tell (six outflows, zero inflows, zero errors) and the honest limit, that flipping *this* file only produces its mirror image. What the flip does repair is asserted on `debit-credit` and `signed-amount`. - The score test naming #329 as its owner: updated in place. It now asserts the recap that sees what the score cannot, on the same rows, instead of promising it in a comment. ## Guards The static guards from links 3 and 6 all still pass unchanged (`mapRow(raw, config, {` in `parseFilesInternal`, one `runAutoDetect(`, two `await detectFormatForFile(`, none in the restore arm, the `checkDuplicatesInternal` / `executeImport` callback slices). New guards pin the step's transition, the absence of any callback that skips it, the flip's re-parse under the flipped config, and the deleted modal. ## Checks - **1088 vitest** (1054 after link 6), 58 files - `npx tsc --noEmit` and `npm run build` clean - `cargo check` exit 0 (no Rust work) - No DB migration (v1→v17 as link 1 left it) ## Deliberately not in scope `CHANGELOG.md` / `CHANGELOG.fr.md`, `docs/architecture.md`, `docs/guide-utilisateur.md` and the `docs.*` keys: `spec-plan-import-csv-format.md` centralises them in issue 10 to keep the linear stack conflict-free, and links 1-6 touched none of them. One item for that link: `docs/architecture.md:35` and `CLAUDE.md` both say "13 composants" for `src/components/import/`, which is 12 now that the modal is gone. Generated autonomously by /autopilot run of 2026-08-13

maximus added 1 commit 2026-08-13 18:32:30 +00:00

feat(import): make the preview mandatory and show what the amounts mean

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

Details

ce19efd476

A confidence score reports how many rows were READ, never what they say:
the `all-positive` fixture scores a perfect 100 % while every credit is
imported as an expense, because each of those rows is perfectly readable.
Nothing between that score and the database looked at the signs — the
preview was an optional modal of 20 rows with no totals, and the final
confirmation listed the delimiter and the date format but neither the
amount mode nor the sign convention.

The `file-preview` step had been declared in `ImportWizardStep` since the
beginning and no dispatch ever aimed at it. It is a real step now,
traversed at every import and gated by nothing — in particular not by the
detection score, which is sign-blind by construction.

- `useImportWizard`: `parseAndPreview` parses and stops at the preview,
  replacing `parsePreview` and the `parseAndCheckDuplicates` that jumped
  straight to the duplicates ("skips preview step"); `checkDuplicates`,
  dead code until now, is the preview's next button, so the rows the user
  validated are the rows that get checked.
- `summarizeParsedRows`: the recap, pure and tested — outflows and their
  total, inflows and theirs, rows in error. Totals stay SIGNED, since
  magnitudes would hide the one thing the recap exists to expose. Computed
  over the whole file, never over the twenty rows displayed.
- `flipSignFormat` + "Inverser les signes": the correction lands on the
  CONFIGURATION, so it is persisted with the source and the next file from
  that bank reads right on its own. In debit/credit mode it swaps the two
  column indices rather than toggling a convention `mapRow` ignores there,
  where a toggle would have been inert.
- `ImportConfirmation` states the amount mode, the sign convention (in the
  mode that applies it) and the column mapping, named by header.
- `FilePreviewModal` removed: it was the redundant surface, and editing
  the table alone would have mutated a still-live copy of it.

The `all-positive` KNOWN DEFECT marker is dropped rather than deleted. Its
three original expectations still hold — an unsigned file carries no
direction and detection cannot invent one — and two cases were added: the
recap tell (six outflows, zero inflows) and the honest limit, that
flipping this particular file only produces its mirror image.

1088 vitest (1054 before), tsc and vite build clean. No DB migration.

Resolves #329

maximus added the

autopilot:pending-human

label 2026-08-13 18:32:37 +00:00

maximus commented

2026-08-14 15:41:20 +00:00

Author

Owner

APPROVE

L'étape file-preview est réellement obligatoire — un seul chemin y mène, un seul en sort, et le retour arrière la retraverse. Le récap est une fonction pure testée, ses totaux sont exacts au centime près, et l'inversion agit bien sur la configuration (swap de colonnes en debit_credit, donc non inerte). Aucun blocage.

Ce qui a été vérifié

Non-contournabilité. SET_STEP: "duplicate-check" n'est dispatché qu'en src/hooks/useImportWizard.ts:762, dans checkDuplicatesInternal, appelé depuis un seul site (:776, checkDuplicates), lui-même câblé au seul bouton Suivant de l'aperçu (src/pages/ImportPage.tsx:132). parseAndCheckDuplicates a disparu de la source. Le seul autre goToStep("duplicate-check") est le retour depuis confirm (src/pages/ImportPage.tsx:169), donc en aval de l'aperçu, jamais un saut par-dessus. Retour arrière couvert : duplicate-check → file-preview (src/pages/ImportPage.tsx:150). Il n'existe aucun chemin file-preview → confirm : après une inversion faite depuis l'aperçu, atteindre la confirmation repasse forcément par checkDuplicates, donc duplicateResult ne peut pas rester périmé sur les anciens montants.

Totaux. summarizeParsedRows (src/utils/importFormat.ts:392-422) compte les erreurs sur !row.parsed, qui est exactement l'ensemble des lignes porteuses d'un error : mapRow ne pose error que via fail(), qui pose parsed: null dans le même objet (src/utils/importFormat.ts:269-273), et le chemin nominal ne pose jamais error. Le compteur d'erreurs et les montants affichés à côté lisent donc bien les mêmes lignes. Arithmétique recalculée hors test : -84,32 + -56,75 === -141,07 exactement — le toEqual du test n'est pas un faux positif de flottant ; all-positive → -1840,20 ; signed-amount → -290,20 / +1550,00. Ligne à montant nul classée dans aucune direction, documenté, testé, et le nombre de lignes affiché à côté garde l'écart visible.

Inversion. flipSignFormat (src/utils/importFormat.ts:450-473) échange debitAmount/creditAmount en mode debit_credit. Comme mapRow calcule |credit| - |debit| (src/utils/importFormat.ts:306-308), le swap est une négation exacte, y compris pour un format à moitié mappé (la seule colonne mappée change de rôle, -|v| devient +|v|), et une colonne non mappée n'est jamais matérialisée en clé undefined. Le test bout-en-bout sur la fixture debit-credit l'assert, pas seulement le mapping. La correction est bien mémorisée : executeImport sérialise state.sourceConfig via formatToRow (src/hooks/useImportWizard.ts:838) et écrit à l'unique point d'écriture (src/hooks/useImportWizard.ts:842), garde await updateSource( ×1 toujours verte.

Suppression du modal. Aucune référence résiduelle dans src/, aucune route. import.preview.title et import.preview.moreRows sont repris par FilePreviewTable — aucune clé i18n morte. Les 12 clés import.config.* que la confirmation consomme (amountMode, signConvention, columnMapping, les 5 colonnes, etc.) existent déjà dans fr.json et en.json ; les 6 nouvelles sont dans les deux fichiers et couvertes par un test.

Gardes héritées. mapRow(raw, config, { ×1, runAutoDetect( ×1, await detectFormatForFile( ×2, await updateSource( ×1 : inchangées. Tous les callbackBody(a, b) restent ordonnés dans la nouvelle source (parseAndPreview:663 → checkDuplicatesInternal:689, checkDuplicatesInternal:689 → checkDuplicates:771, flipSignConvention:798 → executeImport:825, executeImport:825 → goToStep:988). Bloc all-positive : les 3 it d'origine conservés mot pour mot, marqueur retiré du titre seulement, +2 cas dont la limite honnête (l'inversion ne produit que l'image miroir) — 84 → 89 it dans csvAutoDetect.test.ts, aucune suppression, aucun skip/only. Récap extrait en fonction pure et testé (5 cas dédiés + 3 sur fixtures), conforme à l'absence de jsdom. CI frontend verte sur ce19efd.

Suggestions non bloquantes

src/hooks/useImportWizard.ts:804 — SET_SOURCE_CONFIG est dispatché avant le await parseFilesInternal(flipped) de la ligne 812. Si ce re-parse échoue (fichier déplacé ou verrouillé entre les deux lectures), le wizard reste avec une configuration inversée et un parsedPreview d'avant l'inversion : « Suivant » importerait les lignes non inversées tout en persistant le format inversé sur la source — exactement le décalage de signe que l'étape existe pour empêcher. Rare, mais silencieux : restaurer la config précédente dans le catch, ou vider l'aperçu pour bloquer « Suivant ».
src/components/import/FilePreviewTable.tsx:23-29 — le retour anticipé sur rows.length === 0 précède le récap et le bouton d'inversion, et src/pages/ImportPage.tsx:135 laisse « Suivant » actif : un fichier sans ligne de données traverse l'aperçu sans rien montrer, puis crée quand même la source et un enregistrement imported_files à 0 ligne. Comportement préexistant, mais l'étape désormais obligatoire est l'endroit naturel pour le refuser.
src/utils/importFormat.test.ts:796-801 — la garde « aucun contournement » est une absence de chaîne sur l'ancien nom. Elle n'attraperait pas un futur callback qui dispatcherait payload: "duplicate-check" sous un autre nom. Un compteur positif dans le style de la garde await updateSource( — expect(WIZARD_SRC.match(/payload: "duplicate-check"/g)).toHaveLength(1) — épinglerait l'invariant plutôt que le symptôme d'hier.
src/utils/importFormat.ts:411-418 — un montant non fini tomberait dans aucune des trois cases (ni sortie, ni entrée, ni erreur) : il disparaîtrait du récap tout en restant importable. Inatteignable aujourd'hui, mapRow rejetant NaN avant de produire un parsed. Un else sur !Number.isFinite(amount) compté en erreur rendrait le dernier contrôle total-safe par construction plutôt que par dépendance.
Doc différée au maillon 10, cohérent avec les maillons 1-6 qui n'ont touché ni CHANGELOG ni docs/ — le guide utilisateur dit déjà « assistant d'import multi-étapes avec aperçu des données », rien de périmé n'y traîne. Reste le décompte signalé dans le corps de la PR : docs/architecture.md:35 et CLAUDE.md annoncent « 13 composants » dans src/components/import/, désormais 12.

## APPROVE L'étape `file-preview` est réellement obligatoire — un seul chemin y mène, un seul en sort, et le retour arrière la retraverse. Le récap est une fonction pure testée, ses totaux sont exacts au centime près, et l'inversion agit bien sur la configuration (swap de colonnes en `debit_credit`, donc non inerte). Aucun blocage. ### Ce qui a été vérifié **Non-contournabilité.** `SET_STEP: "duplicate-check"` n'est dispatché qu'en `src/hooks/useImportWizard.ts:762`, dans `checkDuplicatesInternal`, appelé depuis un seul site (`:776`, `checkDuplicates`), lui-même câblé au seul bouton Suivant de l'aperçu (`src/pages/ImportPage.tsx:132`). `parseAndCheckDuplicates` a disparu de la source. Le seul autre `goToStep("duplicate-check")` est le retour depuis `confirm` (`src/pages/ImportPage.tsx:169`), donc en aval de l'aperçu, jamais un saut par-dessus. Retour arrière couvert : `duplicate-check` → `file-preview` (`src/pages/ImportPage.tsx:150`). Il n'existe aucun chemin `file-preview` → `confirm` : après une inversion faite depuis l'aperçu, atteindre la confirmation repasse forcément par `checkDuplicates`, donc `duplicateResult` ne peut pas rester périmé sur les anciens montants. **Totaux.** `summarizeParsedRows` (`src/utils/importFormat.ts:392-422`) compte les erreurs sur `!row.parsed`, qui est exactement l'ensemble des lignes porteuses d'un `error` : `mapRow` ne pose `error` que via `fail()`, qui pose `parsed: null` dans le même objet (`src/utils/importFormat.ts:269-273`), et le chemin nominal ne pose jamais `error`. Le compteur d'erreurs et les montants affichés à côté lisent donc bien les mêmes lignes. Arithmétique recalculée hors test : `-84,32 + -56,75 === -141,07` exactement — le `toEqual` du test n'est pas un faux positif de flottant ; `all-positive` → -1840,20 ; `signed-amount` → -290,20 / +1550,00. Ligne à montant nul classée dans aucune direction, documenté, testé, et le nombre de lignes affiché à côté garde l'écart visible. **Inversion.** `flipSignFormat` (`src/utils/importFormat.ts:450-473`) échange `debitAmount`/`creditAmount` en mode `debit_credit`. Comme `mapRow` calcule `|credit| - |debit|` (`src/utils/importFormat.ts:306-308`), le swap est une négation exacte, y compris pour un format à moitié mappé (la seule colonne mappée change de rôle, `-|v|` devient `+|v|`), et une colonne non mappée n'est jamais matérialisée en clé `undefined`. Le test bout-en-bout sur la fixture `debit-credit` l'assert, pas seulement le mapping. La correction est bien mémorisée : `executeImport` sérialise `state.sourceConfig` via `formatToRow` (`src/hooks/useImportWizard.ts:838`) et écrit à l'unique point d'écriture (`src/hooks/useImportWizard.ts:842`), garde `await updateSource(` ×1 toujours verte. **Suppression du modal.** Aucune référence résiduelle dans `src/`, aucune route. `import.preview.title` et `import.preview.moreRows` sont repris par `FilePreviewTable` — aucune clé i18n morte. Les 12 clés `import.config.*` que la confirmation consomme (`amountMode`, `signConvention`, `columnMapping`, les 5 colonnes, etc.) existent déjà dans `fr.json` et `en.json` ; les 6 nouvelles sont dans les deux fichiers et couvertes par un test. **Gardes héritées.** `mapRow(raw, config, {` ×1, `runAutoDetect(` ×1, `await detectFormatForFile(` ×2, `await updateSource(` ×1 : inchangées. Tous les `callbackBody(a, b)` restent ordonnés dans la nouvelle source (`parseAndPreview`:663 → `checkDuplicatesInternal`:689, `checkDuplicatesInternal`:689 → `checkDuplicates`:771, `flipSignConvention`:798 → `executeImport`:825, `executeImport`:825 → `goToStep`:988). Bloc `all-positive` : les 3 `it` d'origine conservés mot pour mot, marqueur retiré du titre seulement, +2 cas dont la limite honnête (l'inversion ne produit que l'image miroir) — 84 → 89 `it` dans `csvAutoDetect.test.ts`, aucune suppression, aucun `skip`/`only`. Récap extrait en fonction pure et testé (5 cas dédiés + 3 sur fixtures), conforme à l'absence de jsdom. CI frontend verte sur `ce19efd`. ### Suggestions non bloquantes 1. **`src/hooks/useImportWizard.ts:804`** — `SET_SOURCE_CONFIG` est dispatché avant le `await parseFilesInternal(flipped)` de la ligne 812. Si ce re-parse échoue (fichier déplacé ou verrouillé entre les deux lectures), le wizard reste avec une configuration inversée et un `parsedPreview` d'avant l'inversion : « Suivant » importerait les lignes non inversées tout en persistant le format inversé sur la source — exactement le décalage de signe que l'étape existe pour empêcher. Rare, mais silencieux : restaurer la config précédente dans le `catch`, ou vider l'aperçu pour bloquer « Suivant ». 2. **`src/components/import/FilePreviewTable.tsx:23-29`** — le retour anticipé sur `rows.length === 0` précède le récap et le bouton d'inversion, et `src/pages/ImportPage.tsx:135` laisse « Suivant » actif : un fichier sans ligne de données traverse l'aperçu sans rien montrer, puis crée quand même la source et un enregistrement `imported_files` à 0 ligne. Comportement préexistant, mais l'étape désormais obligatoire est l'endroit naturel pour le refuser. 3. **`src/utils/importFormat.test.ts:796-801`** — la garde « aucun contournement » est une absence de chaîne sur l'ancien nom. Elle n'attraperait pas un futur callback qui dispatcherait `payload: "duplicate-check"` sous un autre nom. Un compteur positif dans le style de la garde `await updateSource(` — `expect(WIZARD_SRC.match(/payload: "duplicate-check"/g)).toHaveLength(1)` — épinglerait l'invariant plutôt que le symptôme d'hier. 4. **`src/utils/importFormat.ts:411-418`** — un montant non fini tomberait dans aucune des trois cases (ni sortie, ni entrée, ni erreur) : il disparaîtrait du récap tout en restant importable. Inatteignable aujourd'hui, `mapRow` rejetant `NaN` avant de produire un `parsed`. Un `else` sur `!Number.isFinite(amount)` compté en erreur rendrait le dernier contrôle total-safe par construction plutôt que par dépendance. 5. **Doc différée au maillon 10**, cohérent avec les maillons 1-6 qui n'ont touché ni CHANGELOG ni `docs/` — le guide utilisateur dit déjà « assistant d'import multi-étapes avec aperçu des données », rien de périmé n'y traîne. Reste le décompte signalé dans le corps de la PR : `docs/architecture.md:35` et `CLAUDE.md` annoncent « 13 composants » dans `src/components/import/`, désormais 12.

maximus commented

2026-08-14 16:14:00 +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:14:00 +00:00

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

Details