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
10 changed files with 842 additions and 161 deletions
Showing only changes of commit ce19efd476 - Show all commits

View file

@ -1,61 +0,0 @@
import { useEffect } from "react";
import { createPortal } from "react-dom";
import { useTranslation } from "react-i18next";
import { X } from "lucide-react";
import FilePreviewTable from "./FilePreviewTable";
import type { ParsedRow } from "../../shared/types";
interface FilePreviewModalProps {
rows: ParsedRow[];
totalCount: number;
onClose: () => void;
}
export default function FilePreviewModal({
rows,
totalCount,
onClose,
}: FilePreviewModalProps) {
const { t } = useTranslation();
useEffect(() => {
function handleEscape(e: KeyboardEvent) {
if (e.key === "Escape") onClose();
}
document.addEventListener("keydown", handleEscape);
return () => document.removeEventListener("keydown", handleEscape);
}, [onClose]);
return createPortal(
<div
className="fixed inset-0 z-[200] flex items-center justify-center bg-black/50"
onClick={(e) => { if (e.target === e.currentTarget) onClose(); }}
>
<div className="bg-[var(--card)] rounded-xl border border-[var(--border)] shadow-2xl w-full max-w-4xl max-h-[85vh] flex flex-col mx-4">
{/* Header */}
<div className="flex items-center justify-between px-6 py-4 border-b border-[var(--border)]">
<h2 className="text-lg font-semibold">{t("import.preview.title")}</h2>
<button
onClick={onClose}
className="p-1 rounded-lg hover:bg-[var(--muted)] transition-colors"
>
<X size={18} />
</button>
</div>
{/* Body */}
<div className="flex-1 overflow-auto p-6">
<FilePreviewTable rows={rows} />
{totalCount > rows.length && (
<p className="text-sm text-[var(--muted-foreground)] text-center mt-4">
{t("import.preview.moreRows", {
count: totalCount - rows.length,
})}
</p>
)}
</div>
</div>
</div>,
document.body
);
}

View file

@ -1,16 +1,24 @@
import { useTranslation } from "react-i18next";
import { AlertCircle } from "lucide-react";
import { AlertCircle, ArrowDownLeft, ArrowUpRight, RefreshCw } from "lucide-react";
import type { ParsedRow } from "../../shared/types";
import { isRowErrorKey } from "../../utils/importFormat";
import { isRowErrorKey, summarizeParsedRows } from "../../utils/importFormat";
/** Rows rendered in the table. The recap above it always covers the whole file. */
const DISPLAYED_ROWS = 20;
interface FilePreviewTableProps {
/** ALL parsed rows — the recap is meaningless on a truncated sample. */
rows: ParsedRow[];
onFlipSigns?: () => void;
isFlipping?: boolean;
}
export default function FilePreviewTable({
rows,
onFlipSigns,
isFlipping = false,
}: FilePreviewTableProps) {
const { t } = useTranslation();
const { t, i18n } = useTranslation();
if (rows.length === 0) {
return (
@ -20,7 +28,13 @@ export default function FilePreviewTable({
);
}
const errorCount = rows.filter((r) => r.error).length;
const totals = summarizeParsedRows(rows);
const displayedRows = rows.slice(0, DISPLAYED_ROWS);
const currency = new Intl.NumberFormat(
i18n.language === "fr" ? "fr-CA" : "en-CA",
{ style: "currency", currency: "CAD" }
);
// Row errors are i18n keys (#325); anything else reaches us verbatim.
const errorText = (error: string) =>
@ -36,15 +50,77 @@ export default function FilePreviewTable({
<span className="text-[var(--muted-foreground)]">
{t("import.preview.rowCount", { count: rows.length })}
</span>
{errorCount > 0 && (
{totals.errorCount > 0 && (
<span className="flex items-center gap-1 text-[var(--negative)]">
<AlertCircle size={14} />
{t("import.preview.errorCount", { count: errorCount })}
{t("import.preview.errorCount", { count: totals.errorCount })}
</span>
)}
</div>
</div>
{/*
The signed recap (#329) the last control before the database, and the
only one that looks at what the amounts MEAN. A statement showing zero
inflows next to a payroll line is wrong on its face, whatever produced
it. Computed over every row, never over the twenty displayed below.
*/}
<div className="mb-4 p-4 rounded-xl bg-[var(--card)] border border-[var(--border)]">
<div className="flex items-start justify-between gap-4 flex-wrap">
<div className="grid grid-cols-1 sm:grid-cols-3 gap-4 flex-1 min-w-[16rem]">
<div>
<p className="flex items-center gap-1 text-xs text-[var(--muted-foreground)]">
<ArrowDownLeft size={14} className="text-[var(--negative)]" />
{t("import.preview.outflowCount", { count: totals.outflowCount })}
</p>
<p className="font-mono text-sm text-[var(--negative)]">
{currency.format(totals.outflowTotal)}
</p>
</div>
<div>
<p className="flex items-center gap-1 text-xs text-[var(--muted-foreground)]">
<ArrowUpRight size={14} className="text-[var(--positive)]" />
{t("import.preview.inflowCount", { count: totals.inflowCount })}
</p>
<p className="font-mono text-sm text-[var(--positive)]">
{currency.format(totals.inflowTotal)}
</p>
</div>
<div>
<p className="flex items-center gap-1 text-xs text-[var(--muted-foreground)]">
<AlertCircle size={14} />
{t("import.preview.errorRows")}
</p>
<p
className={`font-mono text-sm ${
totals.errorCount > 0
? "text-[var(--negative)]"
: "text-[var(--muted-foreground)]"
}`}
>
{totals.errorCount}
</p>
</div>
</div>
{onFlipSigns && (
<div className="text-right max-w-xs">
<button
onClick={onFlipSigns}
disabled={isFlipping}
className="flex items-center gap-1 px-3 py-2 text-sm rounded-lg border border-[var(--border)] text-[var(--foreground)] hover:bg-[var(--muted)] transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
>
<RefreshCw size={14} />
{t("import.preview.flipSigns")}
</button>
<p className="mt-1 text-xs text-[var(--muted-foreground)]">
{t("import.preview.flipSignsHint")}
</p>
</div>
)}
</div>
</div>
<div className="overflow-x-auto rounded-xl border border-[var(--border)]">
<table className="w-full text-sm">
<thead>
@ -67,7 +143,7 @@ export default function FilePreviewTable({
</tr>
</thead>
<tbody className="divide-y divide-[var(--border)]">
{rows.map((row) => (
{displayedRows.map((row) => (
<tr
key={row.rowIndex}
className={
@ -109,6 +185,14 @@ export default function FilePreviewTable({
</tbody>
</table>
</div>
{rows.length > displayedRows.length && (
<p className="text-sm text-[var(--muted-foreground)] text-center mt-4">
{t("import.preview.moreRows", {
count: rows.length - displayedRows.length,
})}
</p>
)}
</div>
);
}

View file

@ -5,6 +5,8 @@ import type { SourceConfig, ScannedFile, DuplicateCheckResult } from "../../shar
interface ImportConfirmationProps {
sourceName: string;
config: SourceConfig;
/** Header labels of the parsed file, used to name the mapped columns. */
headers?: string[];
selectedFiles: ScannedFile[];
duplicateResult: DuplicateCheckResult;
excludedCount: number;
@ -13,6 +15,7 @@ interface ImportConfirmationProps {
export default function ImportConfirmation({
sourceName,
config,
headers = [],
selectedFiles,
duplicateResult,
excludedCount,
@ -22,6 +25,32 @@ export default function ImportConfirmation({
const rowsToImport =
duplicateResult.newRows.length + duplicateResult.duplicateRows.length - excludedCount;
/**
* A mapped column, named by its header when the file has one. An index alone
* ("2") tells the reader nothing about whether the right column was picked.
*/
const columnLabel = (index: number | undefined): string => {
if (index === undefined) return t("import.confirm.columnUnmapped");
const header = headers[index]?.trim();
return header ? `${index}${header}` : String(index);
};
// The mapping as the amount mode actually reads it: naming a debit column on
// a single-amount format would describe an import that is not happening.
const mappedColumns: Array<[string, number | undefined]> =
config.amountMode === "debit_credit"
? [
["import.config.dateColumn", config.columnMapping.date],
["import.config.descriptionColumn", config.columnMapping.description],
["import.config.debitColumn", config.columnMapping.debitAmount],
["import.config.creditColumn", config.columnMapping.creditAmount],
]
: [
["import.config.dateColumn", config.columnMapping.date],
["import.config.descriptionColumn", config.columnMapping.description],
["import.config.amountColumn", config.columnMapping.amount],
];
return (
<div className="space-y-6">
<h2 className="text-lg font-semibold">
@ -73,6 +102,47 @@ export default function ImportConfirmation({
<span className="font-medium">{t("import.config.skipLines")}:</span>{" "}
{config.skipLines}
</div>
{/*
The amount mode and the sign convention (#329). They decide what
every amount MEANS, and they were the two settings this last
checkpoint did not show the most fragile pair, invisible.
*/}
<div>
<span className="font-medium">{t("import.config.amountMode")}:</span>{" "}
{config.amountMode === "debit_credit"
? t("import.config.debitCredit")
: t("import.config.singleAmount")}
</div>
{/*
Single-amount mode only: `mapRow` computes `credit - debit` on
magnitudes in debit/credit mode and never reads the convention
there, so stating one would describe a rule that is not applied.
*/}
{config.amountMode === "single" && (
<div>
<span className="font-medium">
{t("import.config.signConvention")}:
</span>{" "}
{config.signConvention === "positive_expense"
? t("import.config.positiveExpense")
: t("import.config.negativeExpense")}
</div>
)}
</div>
</div>
{/* Column mapping */}
<div className="p-4">
<p className="text-sm font-medium mb-2">
{t("import.config.columnMapping")}
</p>
<div className="grid grid-cols-2 md:grid-cols-4 gap-2 text-xs text-[var(--muted-foreground)]">
{mappedColumns.map(([labelKey, index]) => (
<div key={labelKey}>
<span className="font-medium">{t(labelKey)}:</span>{" "}
{columnLabel(index)}
</div>
))}
</div>
</div>

View file

@ -46,6 +46,7 @@ import {
} from "../utils/csvAutoDetect";
import {
detectAmountSeparators,
flipSignFormat,
formatFromRow,
formatToRow,
ImportFormatError,
@ -580,9 +581,16 @@ export function useImportWizard() {
}
}, [state.selectedSource, state.importedFilesBySource]);
// Internal helper: parses selected files and returns rows + headers
const parseFilesInternal = useCallback(async (): Promise<{ rows: ParsedRow[]; headers: string[] }> => {
const config = state.sourceConfig;
// Internal helper: parses selected files and returns rows + headers.
//
// `configOverride` exists for the sign flip: it re-parses under a format that
// has just been dispatched and is therefore not yet readable in `state`.
// Reading the stale one would redisplay the exact table the user asked to
// correct, which reads as "the button did nothing".
const parseFilesInternal = useCallback(async (
configOverride?: SourceConfig
): Promise<{ rows: ParsedRow[]; headers: string[] }> => {
const config = configOverride ?? state.sourceConfig;
const allRows: ParsedRow[] = [];
let headers: string[] = [];
@ -644,8 +652,15 @@ export function useImportWizard() {
return { rows: allRows, headers };
}, [state.selectedFiles, state.sourceConfig]);
// Parse files and store preview (does NOT change wizard step)
const parsePreview = useCallback(async () => {
// Parse the selected files and STOP at the preview.
//
// Every import goes through that step now (#329). It used to be an optional
// modal nobody had to open, next to a button that went straight to the
// duplicate check — so a file read under a wrong convention reached the
// database without anyone ever seeing a total. The step is unconditional on
// purpose: the detection score does not gate it, because a perfect score says
// nothing about the sign of what was read.
const parseAndPreview = useCallback(async () => {
if (state.selectedFiles.length === 0) return;
dispatch({ type: "SET_LOADING", payload: true });
@ -657,6 +672,7 @@ export function useImportWizard() {
type: "SET_PARSED_PREVIEW",
payload: result,
});
dispatch({ type: "SET_STEP", payload: "file-preview" });
} catch (e) {
dispatch({
type: "SET_ERROR",
@ -746,7 +762,12 @@ export function useImportWizard() {
dispatch({ type: "SET_STEP", payload: "duplicate-check" });
}, [state.selectedFiles]);
// Check duplicates using already-parsed preview data
// Leave the preview for the duplicate check, on the rows it just displayed.
//
// Dead code until #329: the wizard jumped from the configuration straight to
// the duplicates, re-parsing on the way. It is the preview step's "next"
// button now, so the rows the user validated are the rows that get checked —
// no second parse can quietly read the file differently in between.
const checkDuplicates = useCallback(async () => {
dispatch({ type: "SET_LOADING", payload: true });
dispatch({ type: "SET_ERROR", payload: null });
@ -761,27 +782,45 @@ export function useImportWizard() {
}
}, [state.parsedPreview, checkDuplicatesInternal]);
// Parse files then check duplicates in one step (skips preview step)
const parseAndCheckDuplicates = useCallback(async () => {
if (state.selectedFiles.length === 0) return;
/**
* Read the file the other way round, and remember it.
*
* The correction lands on the CONFIGURATION (`flipSignFormat`, which knows
* that debit/credit flips by swapping columns rather than by toggling a
* convention `mapRow` ignores there), so it is persisted with the source at
* import time and the next file from that bank reads right on its own.
*
* Headers are deliberately NOT reloaded: a flip touches neither delimiter,
* encoding, skipped lines nor header flag, and `loadHeadersWithConfig`
* dispatches an empty row list racing it against this re-parse is a coin
* toss between the corrected table and a blank one.
*/
const flipSignConvention = useCallback(async () => {
const flipped: SourceConfig = {
...state.sourceConfig,
...flipSignFormat(state.sourceConfig),
};
dispatch({ type: "SET_SOURCE_CONFIG", payload: flipped });
// Detection did not produce this format. Keeping its badge would vouch for
// a configuration it never measured — the same rule as a manual edit.
dispatch({ type: "SET_DETECTION_SCORE", payload: null });
dispatch({ type: "SET_LOADING", payload: true });
dispatch({ type: "SET_ERROR", payload: null });
try {
const result = await parseFilesInternal();
const result = await parseFilesInternal(flipped);
dispatch({
type: "SET_PARSED_PREVIEW",
payload: result,
});
await checkDuplicatesInternal(result.rows);
} catch (e) {
dispatch({
type: "SET_ERROR",
payload: e instanceof Error ? e.message : String(e),
});
}
}, [state.selectedFiles, parseFilesInternal, checkDuplicatesInternal]);
}, [state.sourceConfig, parseFilesInternal]);
const executeImport = useCallback(async () => {
if (!state.duplicateResult) return;
@ -1063,9 +1102,9 @@ export function useImportWizard() {
updateConfig,
toggleFile,
selectAllFiles,
parsePreview,
parseAndPreview,
checkDuplicates,
parseAndCheckDuplicates,
flipSignConvention,
executeImport,
goToStep,
reset,

View file

@ -127,7 +127,12 @@
"description": "Description",
"amount": "Amount",
"raw": "Raw data",
"moreRows": "... and {{count}} more row(s)"
"moreRows": "... and {{count}} more row(s)",
"outflowCount": "{{count}} outflow(s)",
"inflowCount": "{{count}} inflow(s)",
"errorRows": "Rows in error",
"flipSigns": "Flip the signs",
"flipSignsHint": "Corrects the source configuration, not just this preview: the correction is remembered for the next imports."
},
"duplicates": {
"title": "Duplicate Detection",
@ -149,7 +154,8 @@
"files": "Files",
"settings": "Settings",
"rowsToImport": "Rows to import",
"rowsSummary": "{{count}} row(s) to import, {{skipped}} duplicate(s) skipped"
"rowsSummary": "{{count}} row(s) to import, {{skipped}} duplicate(s) skipped",
"columnUnmapped": "not mapped"
},
"progress": {
"title": "Import in Progress",

View file

@ -127,7 +127,12 @@
"description": "Description",
"amount": "Montant",
"raw": "Données brutes",
"moreRows": "... et {{count}} ligne(s) supplémentaire(s)"
"moreRows": "... et {{count}} ligne(s) supplémentaire(s)",
"outflowCount": "{{count}} sortie(s)",
"inflowCount": "{{count}} entrée(s)",
"errorRows": "Lignes en erreur",
"flipSigns": "Inverser les signes",
"flipSignsHint": "Corrige la configuration de la source, pas seulement cet aperçu : la correction est mémorisée pour les prochains imports."
},
"duplicates": {
"title": "Détection des doublons",
@ -149,7 +154,8 @@
"files": "Fichiers",
"settings": "Paramètres",
"rowsToImport": "Lignes à importer",
"rowsSummary": "{{count}} ligne(s) à importer, {{skipped}} doublon(s) ignoré(s)"
"rowsSummary": "{{count}} ligne(s) à importer, {{skipped}} doublon(s) ignoré(s)",
"columnUnmapped": "non associée"
},
"progress": {
"title": "Import en cours",

View file

@ -1,17 +1,16 @@
import { useState, useCallback } from "react";
import { useTranslation } from "react-i18next";
import { useImportWizard } from "../hooks/useImportWizard";
import ImportFolderConfig from "../components/import/ImportFolderConfig";
import SourceList from "../components/import/SourceList";
import SourceConfigPanel from "../components/import/SourceConfigPanel";
import FilePreviewTable from "../components/import/FilePreviewTable";
import DuplicateCheckPanel from "../components/import/DuplicateCheckPanel";
import ImportConfirmation from "../components/import/ImportConfirmation";
import ImportProgress from "../components/import/ImportProgress";
import ImportReportPanel from "../components/import/ImportReportPanel";
import WizardNavigation from "../components/import/WizardNavigation";
import ImportHistoryPanel from "../components/import/ImportHistoryPanel";
import FilePreviewModal from "../components/import/FilePreviewModal";
import { AlertCircle, Eye, X, ChevronLeft } from "lucide-react";
import { AlertCircle } from "lucide-react";
import { PageHelp } from "../components/shared/PageHelp";
export default function ImportPage() {
@ -24,8 +23,9 @@ export default function ImportPage() {
updateConfig,
toggleFile,
selectAllFiles,
parsePreview,
parseAndCheckDuplicates,
parseAndPreview,
checkDuplicates,
flipSignConvention,
executeImport,
goToStep,
reset,
@ -38,13 +38,6 @@ export default function ImportPage() {
setSkipAllDuplicates,
} = useImportWizard();
const [showPreviewModal, setShowPreviewModal] = useState(false);
const handlePreview = useCallback(async () => {
await parsePreview();
setShowPreviewModal(true);
}, [parsePreview]);
const nextDisabled = state.selectedFiles.length === 0 || !state.sourceConfig.name;
return (
@ -111,41 +104,36 @@ export default function ImportPage() {
detectionScore={state.detectionScore}
isLoading={state.isLoading}
/>
<div className="flex items-center justify-between pt-6 border-t border-[var(--border)]">
<div>
<button
onClick={reset}
className="flex items-center gap-1 px-4 py-2 text-sm text-[var(--muted-foreground)] hover:text-[var(--foreground)] transition-colors"
>
<X size={16} />
{t("common.cancel")}
</button>
</div>
<div className="flex items-center gap-3">
<button
onClick={() => goToStep("source-list")}
className="flex items-center gap-1 px-4 py-2 text-sm rounded-lg border border-[var(--border)] text-[var(--foreground)] hover:bg-[var(--muted)] transition-colors"
>
<ChevronLeft size={16} />
{t("import.wizard.back")}
</button>
<button
onClick={handlePreview}
disabled={nextDisabled || state.isLoading}
className="flex items-center gap-1 px-4 py-2 text-sm rounded-lg border border-[var(--border)] text-[var(--foreground)] hover:bg-[var(--muted)] transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
>
<Eye size={16} />
{t("import.wizard.preview")}
</button>
<button
onClick={parseAndCheckDuplicates}
disabled={nextDisabled || state.isLoading}
className="flex items-center gap-1 px-4 py-2 text-sm rounded-lg bg-[var(--primary)] text-white hover:opacity-90 transition-opacity disabled:opacity-50 disabled:cursor-not-allowed"
>
{t("import.wizard.checkDuplicates")}
</button>
</div>
</div>
{/*
One way forward, and it goes through the preview (#329). The pair of
buttons this replaces offered "Aperçu" as an optional detour and
"Vérifier les doublons" as the real path, so the totals were the one
screen an import never had to show.
*/}
<WizardNavigation
onBack={() => goToStep("source-list")}
onNext={parseAndPreview}
onCancel={reset}
nextLabel={t("import.wizard.preview")}
nextDisabled={nextDisabled || state.isLoading}
/>
</div>
)}
{state.step === "file-preview" && (
<div className="space-y-6">
<FilePreviewTable
rows={state.parsedPreview}
onFlipSigns={flipSignConvention}
isFlipping={state.isLoading}
/>
<WizardNavigation
onBack={() => goToStep("source-config")}
onNext={checkDuplicates}
onCancel={reset}
nextLabel={t("import.wizard.checkDuplicates")}
nextDisabled={state.isLoading}
/>
</div>
)}
@ -159,7 +147,7 @@ export default function ImportPage() {
onIncludeAll={() => setSkipAllDuplicates(false)}
/>
<WizardNavigation
onBack={() => goToStep("source-config")}
onBack={() => goToStep("file-preview")}
onNext={() => goToStep("confirm")}
onCancel={reset}
nextLabel={t("import.wizard.confirm")}
@ -172,6 +160,7 @@ export default function ImportPage() {
<ImportConfirmation
sourceName={state.sourceConfig.name}
config={state.sourceConfig}
headers={state.previewHeaders}
selectedFiles={state.selectedFiles}
duplicateResult={state.duplicateResult}
excludedCount={state.excludedDuplicateIndices.size}
@ -197,15 +186,6 @@ export default function ImportPage() {
{state.step === "report" && state.importReport && (
<ImportReportPanel report={state.importReport} onDone={reset} />
)}
{/* Preview modal */}
{showPreviewModal && state.parsedPreview.length > 0 && (
<FilePreviewModal
rows={state.parsedPreview.slice(0, 20)}
totalCount={state.parsedPreview.length}
onClose={() => setShowPreviewModal(false)}
/>
)}
</div>
);
}

View file

@ -23,7 +23,12 @@ import {
preprocessQuotedCSV,
} from "./csvAutoDetect";
import type { DecimalSeparator } from "./amountParser";
import { detectAmountSeparators, mapRow } from "./importFormat";
import {
detectAmountSeparators,
flipSignFormat,
mapRow,
summarizeParsedRows,
} from "./importFormat";
import {
CSV_FIXTURE_NAMES,
readCsvFixture,
@ -182,11 +187,24 @@ function mapCorpusRow(
return row.parsed ?? { error: row.error! };
}
/** Full pipeline: raw file text -> signed amounts, as the wizard runs it. */
function parseFixtureEndToEnd(name: CsvFixtureName) {
/** The shape `autoDetectConfig` returns: a format minus the encoding. */
type DetectedConfig = NonNullable<ReturnType<typeof autoDetectConfig>>;
/**
* Full pipeline: raw file text -> signed amounts, as the wizard runs it.
*
* `configure` rewrites the detected configuration before anything is read, so a
* correction the user makes in the preview the sign flip (#329) can be
* replayed over the whole file exactly as the wizard replays it.
*/
function parseFixtureEndToEnd(
name: CsvFixtureName,
configure?: (config: DetectedConfig) => DetectedConfig
) {
const rawContent = readCsvFixture(name);
const cfg = autoDetectConfig(rawContent);
if (!cfg) throw new Error(`autoDetectConfig returned null for ${name}`);
const detected = autoDetectConfig(rawContent);
if (!detected) throw new Error(`autoDetectConfig returned null for ${name}`);
const cfg = configure ? configure(detected) : detected;
const format = { ...cfg, encoding: "utf-8" };
const data = Papa.parse(preprocessQuotedCSV(rawContent), {
@ -207,12 +225,36 @@ function parseFixtureEndToEnd(name: CsvFixtureName) {
}
/** Signed amounts only — the number that actually reaches the database. */
function amountsOf(name: CsvFixtureName): (number | string)[] {
return parseFixtureEndToEnd(name).rows.map((r) =>
function amountsOf(
name: CsvFixtureName,
configure?: (config: DetectedConfig) => DetectedConfig
): (number | string)[] {
return parseFixtureEndToEnd(name, configure).rows.map((r) =>
"error" in r ? r.error : r.amount
);
}
/** The recap the mandatory preview step displays for a fixture (#329). */
function recapOf(
name: CsvFixtureName,
configure?: (config: DetectedConfig) => DetectedConfig
) {
const { rows } = parseFixtureEndToEnd(name, configure);
return summarizeParsedRows(
rows.map((r, rowIndex) =>
"error" in r
? { rowIndex, raw: [], parsed: null, error: r.error }
: { rowIndex, raw: [], parsed: r }
)
);
}
/** The sign flip as the preview's button applies it: onto the configuration. */
const flipped = (config: DetectedConfig): DetectedConfig => ({
...config,
...flipSignFormat(config),
});
/** The amounts every well-detected fixture in the corpus must produce. */
const REFERENCE_AMOUNTS = [-84.32, 1250, -142.18, -56.75, 300, -6.95];
@ -478,15 +520,19 @@ describe("autoDetectConfig — no header row (#326)", () => {
});
});
describe("autoDetectConfig — KNOWN DEFECT: all-positive amounts (#326, fixed by #329)", () => {
describe("autoDetectConfig — all-positive amounts: detected wrong, now SURFACED (#326, #329)", () => {
// `detectSingleAmount` (csvAutoDetect.ts:507-530) infers the convention from
// the share of negative values alone: no negatives -> `positive_expense` ->
// every amount is negated at parse time. On a file that mixes expenses and
// income and signs neither, the income is imported as an expense.
//
// Nothing in the file distinguishes the two cases, so detection cannot be
// "fixed" here. #329 surfaces it instead: a confidence score, a mandatory
// preview showing signed totals, and a "flip the signs" button.
// fixed here — the three expectations below still hold and are meant to. What
// #329 changed is that the mistake can no longer reach the database unseen:
// the preview step is traversed at every import, its recap states the two
// directions, and the flip button corrects the CONFIGURATION. The last two
// cases are that repair, and they are the reason this block no longer carries
// a KNOWN DEFECT marker.
it("infers positive_expense from the absence of negatives", () => {
const cfg = autoDetectConfig(readCsvFixture("all-positive"))!;
@ -497,21 +543,81 @@ describe("autoDetectConfig — KNOWN DEFECT: all-positive amounts (#326, fixed b
it("negates the income rows along with the expenses", () => {
expect(amountsOf("all-positive")).toEqual([
-84.32, // expense — correct
-1250, // DEFECT — payroll deposit, should be +1250
-1250, // WRONG — payroll deposit, should be +1250
-142.18, // expense — correct
-56.75, // expense — correct
-300, // DEFECT — incoming transfer, should be +300
-300, // WRONG — incoming transfer, should be +300
-6.95, // expense — correct
]);
});
it("produces a total that is plainly wrong, which the preview will expose", () => {
it("produces a total that is plainly wrong, which the preview exposes", () => {
const total = amountsOf("all-positive").reduce<number>(
(a, b) => a + (typeof b === "number" ? b : 0),
0
);
expect(total).toBeCloseTo(-1840.2, 2); // truth is +1259.80
});
it("shows a bank statement with SIX outflows and ZERO inflows", () => {
// The tell, and the whole reason the step is mandatory: no month of a real
// account has zero inflows. The score sees nothing here — every row reads.
const recap = recapOf("all-positive");
expect(recap.outflowCount).toBe(6);
expect(recap.inflowCount).toBe(0);
expect(recap.inflowTotal).toBe(0);
expect(recap.outflowTotal).toBeCloseTo(-1840.2, 2);
expect(recap.errorCount).toBe(0);
});
it("flips to the mirror image — this file needs more than a flip", () => {
// Stated plainly, because it is the limit of the control: the file carries
// no direction at all, so NO convention can split it correctly. Flipping
// turns six outflows into six inflows and the recap stays implausible,
// which is the honest outcome — the preview tells the user the file is
// unreadable as it stands, in both directions. What the flip does repair is
// a file that is uniformly reversed (the debit/credit case below).
expect(amountsOf("all-positive", flipped)).toEqual([
84.32, 1250, 142.18, 56.75, 300, 6.95,
]);
const recap = recapOf("all-positive", flipped);
expect(recap.inflowCount).toBe(6);
expect(recap.outflowCount).toBe(0);
});
});
describe("the sign flip on a whole fixture (#329)", () => {
it("reverses a correctly-read signed file, and back again", () => {
expect(amountsOf("signed-amount", flipped)).toEqual(
REFERENCE_AMOUNTS.map((a) => -a)
);
expect(
amountsOf("signed-amount", (cfg) => flipped(flipped(cfg)))
).toEqual(REFERENCE_AMOUNTS);
});
it("is NOT inert in debit/credit mode", () => {
// The review finding this branch answers: `mapRow` computes `credit - debit`
// on magnitudes and never reads the sign convention there, so a button that
// toggled the convention would leave a two-column file mapped backwards
// exactly as wrong as it found it. The columns swap instead.
const cfg = autoDetectConfig(readCsvFixture("debit-credit"))!;
expect(cfg.amountMode).toBe("debit_credit");
expect(amountsOf("debit-credit")).toEqual(REFERENCE_AMOUNTS);
expect(amountsOf("debit-credit", flipped)).toEqual(
REFERENCE_AMOUNTS.map((a) => -a)
);
});
it("leaves the recap of a well-read file plausible", () => {
const recap = recapOf("signed-amount");
expect(recap.outflowCount).toBe(4);
expect(recap.inflowCount).toBe(2);
expect(recap.inflowTotal).toBeCloseTo(1550, 2);
expect(recap.outflowTotal).toBeCloseTo(-290.2, 2);
expect(recap.errorCount).toBe(0);
});
});
describe("preprocessQuotedCSV + autoDetectConfig — whole-line-quoted file (#326)", () => {
@ -867,14 +973,21 @@ describe("detectImportFormat — confidence score (#328)", () => {
expect(scoreOf("no-header").totalRows).toBe(6);
});
it("says nothing about the SIGN of what it read (#329)", () => {
// `all-positive` is the corpus' known defect: unsigned magnitudes, detected
// as `negative_expense`, so every credit imports as an expense. Every row
it("says nothing about the SIGN of what it read — the recap does (#329)", () => {
// `all-positive` carries unsigned magnitudes, detected as
// `positive_expense`, so every credit imports as an expense. Every row
// still PARSES, so the score is a perfect 100 % — which is precisely why the
// threshold blocks nothing and the preview step stays mandatory.
// threshold gates nothing and the preview step is traversed unconditionally.
const score = scoreOf("all-positive");
expect(score.ratio).toBe(1);
expect(score.confident).toBe(true);
// The control that DOES see it, on the same file and the same rows: the
// signed recap of the mandatory preview step. This is no longer a promise
// written in a comment — it is the assertion below.
const recap = recapOf("all-positive");
expect(recap.inflowCount).toBe(0);
expect(recap.outflowCount).toBe(score.readRows);
});
it("carries no score on a refused format", () => {

View file

@ -6,12 +6,13 @@
// wizard restores what was saved instead of re-deriving it.
import { describe, it, expect } from "vitest";
import { readFileSync } from "node:fs";
import { existsSync, readFileSync } from "node:fs";
import { resolve } from "node:path";
import type {
ImportFormat,
ImportFormatRow,
ImportFormatRowInput,
ParsedRow,
} from "../shared/types";
import {
AMOUNT_MODES,
@ -21,11 +22,15 @@ import {
SIGN_CONVENTIONS,
clearMappingForMode,
detectAmountSeparators,
flipSignFormat,
formatFromRow,
formatToRow,
isRowErrorKey,
mapRow,
summarizeParsedRows,
} from "./importFormat";
import fr from "../i18n/locales/fr.json";
import en from "../i18n/locales/en.json";
// ---------------------------------------------------------------------------
// Samples. Every field deliberately differs from the wizard's default config,
@ -601,3 +606,323 @@ describe("useImportWizard — the config write point (#324)", () => {
expect(WIZARD_SRC.match(/await updateSource\(/g)).toHaveLength(1);
});
});
// ---------------------------------------------------------------------------
// The signed recap and the sign flip (#329)
// ---------------------------------------------------------------------------
/** A parsed row carrying an amount. */
function row(rowIndex: number, amount: number): ParsedRow {
return {
rowIndex,
raw: ["01/01/2025", "LABEL", String(amount)],
parsed: { date: "2025-01-01", description: "LABEL", amount },
};
}
/** A row that produced no amount at all. */
function errorRow(rowIndex: number): ParsedRow {
return {
rowIndex,
raw: ["", "LABEL", "x"],
parsed: null,
error: ROW_ERROR_KEYS.invalidDate,
};
}
describe("summarizeParsedRows — the recap that reads MEANING (#329)", () => {
it("counts and totals the two directions separately", () => {
expect(
summarizeParsedRows([row(0, -84.32), row(1, 1250), row(2, -56.75)])
).toEqual({
outflowCount: 2,
outflowTotal: -141.07,
inflowCount: 1,
inflowTotal: 1250,
errorCount: 0,
});
});
it("keeps the totals SIGNED, as they will reach the ledger", () => {
// Magnitudes would hide the one thing the recap exists to expose: a file
// read backwards looks identical once the signs are dropped.
const totals = summarizeParsedRows([row(0, -10), row(1, 40)]);
expect(totals.outflowTotal).toBeLessThan(0);
expect(totals.inflowTotal).toBeGreaterThan(0);
});
it("counts rows with no parsed amount as errors, and nothing else", () => {
const totals = summarizeParsedRows([errorRow(0), row(1, -10), errorRow(2)]);
expect(totals.errorCount).toBe(2);
expect(totals.outflowCount).toBe(1);
expect(totals.inflowCount).toBe(0);
});
it("files a zero amount under neither direction", () => {
// Deliberate: a 0,00 row is its own anomaly. Filing it silently under
// outflows or inflows would be a small lie of the same family as the big
// one this recap exists to catch.
const totals = summarizeParsedRows([row(0, 0), row(1, -10)]);
expect(totals.outflowCount).toBe(1);
expect(totals.inflowCount).toBe(0);
expect(totals.errorCount).toBe(0);
});
it("returns all zeroes on an empty file", () => {
expect(summarizeParsedRows([])).toEqual({
outflowCount: 0,
outflowTotal: 0,
inflowCount: 0,
inflowTotal: 0,
errorCount: 0,
});
});
});
describe("flipSignFormat — the correction lands on the FORMAT (#329)", () => {
const SINGLE = {
...SAMPLE_FORMAT,
amountMode: "single" as const,
signConvention: "negative_expense" as const,
columnMapping: { date: 0, description: 1, amount: 2 },
};
const DEBIT_CREDIT = {
...SAMPLE_FORMAT,
amountMode: "debit_credit" as const,
columnMapping: { date: 0, description: 1, debitAmount: 2, creditAmount: 3 },
};
it("toggles the convention in single-amount mode", () => {
expect(flipSignFormat(SINGLE).signConvention).toBe("positive_expense");
expect(
flipSignFormat({ ...SINGLE, signConvention: "positive_expense" })
.signConvention
).toBe("negative_expense");
});
it("is its own inverse in single-amount mode", () => {
const once = { ...SINGLE, ...flipSignFormat(SINGLE) };
expect({ ...once, ...flipSignFormat(once) }).toEqual(SINGLE);
});
it("leaves the mapping alone in single-amount mode", () => {
expect(flipSignFormat(SINGLE).columnMapping).toEqual(SINGLE.columnMapping);
});
it("swaps the two columns in debit/credit mode", () => {
// `mapRow` ignores the sign convention there — toggling it would be inert,
// which is the review finding this branch answers.
expect(flipSignFormat(DEBIT_CREDIT).columnMapping).toEqual({
date: 0,
description: 1,
debitAmount: 3,
creditAmount: 2,
});
});
it("does not touch the convention in debit/credit mode", () => {
expect(flipSignFormat(DEBIT_CREDIT).signConvention).toBe(
DEBIT_CREDIT.signConvention
);
});
it("moves a half-mapped column to the other role", () => {
const half = {
...DEBIT_CREDIT,
columnMapping: { date: 0, description: 1, debitAmount: 2 },
};
expect(flipSignFormat(half).columnMapping).toEqual({
date: 0,
description: 1,
creditAmount: 2,
});
});
it("never materializes an unmapped column as a key", () => {
// `mapRow` distinguishes "not mapped" from "mapped to column 0"; an
// explicit `undefined` would make that distinction unreachable.
const half = {
...DEBIT_CREDIT,
columnMapping: { date: 0, description: 1, creditAmount: 3 },
};
const mapping = flipSignFormat(half).columnMapping;
expect(Object.keys(mapping)).not.toContain("creditAmount");
expect(mapping.debitAmount).toBe(3);
});
it("flips the sign of every row it is applied to (single)", () => {
const raw = ["05/01/2025", "EPICERIE", "84.32"];
const before = mapRow(raw, { ...SINGLE, dateFormat: "DD/MM/YYYY" });
const after = mapRow(raw, {
...SINGLE,
dateFormat: "DD/MM/YYYY",
...flipSignFormat(SINGLE),
});
expect(before.parsed!.amount).toBe(84.32);
expect(after.parsed!.amount).toBe(-84.32);
});
it("flips the sign of every row it is applied to (debit/credit)", () => {
const raw = ["05/01/2025", "EPICERIE", "84.32", ""];
const format = { ...DEBIT_CREDIT, dateFormat: "DD/MM/YYYY" };
expect(mapRow(raw, format).parsed!.amount).toBe(-84.32);
expect(
mapRow(raw, { ...format, ...flipSignFormat(format) }).parsed!.amount
).toBe(84.32);
});
});
// ---------------------------------------------------------------------------
// Static guards: the preview step is traversed at EVERY import (#329).
//
// The step lives in a React page and the repository has no jsdom, so the wiring
// is asserted on the source — same technique as the guards above.
// ---------------------------------------------------------------------------
const PAGE_SRC = readFileSync(
resolve(import.meta.dirname, "..", "pages", "ImportPage.tsx"),
"utf-8"
);
describe("the preview step is mandatory (#329)", () => {
it("has a transition that targets it", () => {
// `file-preview` was declared in `ImportWizardStep` from the beginning and
// no dispatch ever aimed at it — the optional modal had supplanted it.
expect(callbackBody("parseAndPreview", "checkDuplicatesInternal")).toContain(
'dispatch({ type: "SET_STEP", payload: "file-preview" })'
);
});
it("leaves no callback that parses straight to the duplicate check", () => {
// `parseAndCheckDuplicates` jumped from the configuration to the duplicates
// ("skips preview step"). Keeping it exported would leave a live bypass.
expect(WIZARD_SRC).not.toContain("parseAndCheckDuplicates");
expect(WIZARD_SRC).not.toContain("skips preview step");
});
it("renders the step and reaches it from the configuration", () => {
expect(PAGE_SRC).toContain('state.step === "file-preview"');
expect(PAGE_SRC).toContain("onNext={parseAndPreview}");
expect(PAGE_SRC).toContain("onNext={checkDuplicates}");
});
it("walks back through the preview from the duplicate step", () => {
expect(PAGE_SRC).toContain('onBack={() => goToStep("file-preview")}');
});
it("shows the recap over the WHOLE file, not the displayed sample", () => {
// The table truncates its rows; a recap computed on the truncation would
// state a total that is not the file's.
expect(PAGE_SRC).toContain("rows={state.parsedPreview}");
expect(PAGE_SRC).not.toContain("state.parsedPreview.slice(");
});
it("no longer carries the optional preview modal", () => {
expect(PAGE_SRC).not.toContain("FilePreviewModal");
expect(
existsSync(
resolve(
import.meta.dirname,
"..",
"components",
"import",
"FilePreviewModal.tsx"
)
)
).toBe(false);
});
});
describe("the sign flip is wired to the configuration (#329)", () => {
const FLIP = () => callbackBody("flipSignConvention", "executeImport");
it("rewrites the config and re-parses under the new one", () => {
const body = FLIP();
expect(body).toContain("flipSignFormat(state.sourceConfig)");
expect(body).toContain('dispatch({ type: "SET_SOURCE_CONFIG", payload: flipped })');
// The flipped format is PASSED to the parse: `state` has not re-rendered
// yet, so re-reading it would redisplay the table the user asked to fix.
expect(body).toContain("parseFilesInternal(flipped)");
});
it("drops the detection score, which never measured this format", () => {
expect(FLIP()).toContain(
'dispatch({ type: "SET_DETECTION_SCORE", payload: null })'
);
});
it("does not reload headers, which a flip cannot change", () => {
// `loadHeadersWithConfig` dispatches an empty row list; racing it against
// the re-parse is a coin toss between the corrected table and a blank one.
expect(FLIP()).not.toContain("loadHeadersWithConfig");
});
});
describe("the confirmation states what decides the amounts (#329)", () => {
const CONFIRM_SRC = readFileSync(
resolve(
import.meta.dirname,
"..",
"components",
"import",
"ImportConfirmation.tsx"
),
"utf-8"
);
it("shows the amount mode and the column mapping", () => {
expect(CONFIRM_SRC).toContain("import.config.amountMode");
expect(CONFIRM_SRC).toContain("import.config.columnMapping");
});
it("shows the sign convention, in the mode that applies it", () => {
expect(CONFIRM_SRC).toContain("import.config.signConvention");
expect(CONFIRM_SRC).toContain('config.amountMode === "single" && (');
});
it("names the columns the amount mode actually reads", () => {
// Naming a debit column on a single-amount format would describe an import
// that is not happening, so the list is chosen by mode.
const mapping = CONFIRM_SRC.slice(
CONFIRM_SRC.indexOf("const mappedColumns"),
CONFIRM_SRC.indexOf("return (")
);
expect(mapping).toContain('config.amountMode === "debit_credit"');
for (const key of [
"dateColumn",
"descriptionColumn",
"amountColumn",
"debitColumn",
"creditColumn",
]) {
expect(mapping, key).toContain(`import.config.${key}`);
}
});
});
describe("every new preview string exists in both languages (#329)", () => {
it("carries the recap and the flip button", () => {
for (const key of [
"outflowCount",
"inflowCount",
"errorRows",
"flipSigns",
"flipSignsHint",
] as const) {
expect(fr.import.preview[key].length, `fr.${key}`).toBeGreaterThan(0);
expect(en.import.preview[key].length, `en.${key}`).toBeGreaterThan(0);
}
});
it("interpolates the two counts in both languages", () => {
for (const key of ["outflowCount", "inflowCount"] as const) {
expect(fr.import.preview[key], `fr.${key}`).toContain("{{count}}");
expect(en.import.preview[key], `en.${key}`).toContain("{{count}}");
}
});
it("carries the unmapped-column label of the confirmation", () => {
expect(fr.import.confirm.columnUnmapped.length).toBeGreaterThan(0);
expect(en.import.confirm.columnUnmapped.length).toBeGreaterThan(0);
});
});

View file

@ -352,3 +352,122 @@ export function detectAmountSeparators(
}
return result;
}
// ---------------------------------------------------------------------------
// The signed recap and the sign flip (#329)
// ---------------------------------------------------------------------------
/**
* What a whole parsed file amounts to, before a single row is written.
*
* This is the chantier's last control, and the only one that looks at MEANING.
* A confidence score reports how many rows were read, never what they say: the
* `all-positive` fixture scores a perfect 100 % while every credit imports as
* an expense (#328), because each of those rows is perfectly readable. A
* statement whose recap shows six outflows and zero inflows is wrong on its
* face, whatever produced it a bad detection, a stale template, a bank that
* changed its export.
*
* Totals are SIGNED, exactly as the amounts that would reach the ledger:
* `outflowTotal` is negative or zero, `inflowTotal` positive or zero. Showing
* magnitudes would hide the one thing the recap exists to expose.
*
* A row whose amount is exactly zero is neither an outflow nor an inflow, so
* the two counts do not have to add up to the row count displayed beside them.
* That is deliberate: a zero amount is its own anomaly and filing it silently
* under one direction would be a small lie of the same family as the big one.
*/
export interface PreviewTotals {
outflowCount: number;
/** Negative or zero. */
outflowTotal: number;
inflowCount: number;
/** Positive or zero. */
inflowTotal: number;
/** Rows that produced no amount at all — they will not be imported. */
errorCount: number;
}
/** Add up a parsed file into the recap the preview step displays. PURE. */
export function summarizeParsedRows(
rows: readonly ParsedRow[]
): PreviewTotals {
const totals: PreviewTotals = {
outflowCount: 0,
outflowTotal: 0,
inflowCount: 0,
inflowTotal: 0,
errorCount: 0,
};
for (const row of rows) {
// No parsed amount is exactly the set of rows carrying an `error`; counting
// the absence of a number rather than the presence of a message keeps this
// count and the amounts it sits next to reading the same rows.
if (!row.parsed) {
totals.errorCount++;
continue;
}
const amount = row.parsed.amount;
if (amount < 0) {
totals.outflowCount++;
totals.outflowTotal += amount;
} else if (amount > 0) {
totals.inflowCount++;
totals.inflowTotal += amount;
}
}
return totals;
}
/**
* Flip the direction a file's amounts are read in.
*
* It acts on the FORMAT, never on the parsed rows: the correction has to be
* memorised with the source, so the next import of the same bank reads right on
* its own. Flipping the rows alone would repair one import and let the next one
* reintroduce the same reversal.
*
* The operation differs by mode, and getting that wrong makes the button inert:
* - `single` the sign convention is what decides, so it toggles.
* - `debit_credit` `mapRow` computes `credit - debit` on magnitudes and
* never reads the sign convention there (#325); the direction is carried by
* WHICH column holds which role. Toggling the convention would change
* nothing at all, so the two column indices swap instead.
*
* A half-mapped debit/credit format swaps too: moving the single mapped column
* to the other role is precisely the correction a file whose one amount column
* was taken for the wrong side needs. Unmapped stays unmapped `mapRow`
* distinguishes "not mapped" from "mapped to column 0", so the key must not
* materialize as `undefined`.
*
* It returns the two fields a flip decides rather than a whole format, so the
* caller spreads it onto whatever carries them (`SourceConfig` in the wizard,
* a detected configuration in the tests) and keeps its own extra fields. Both
* fields always come back, so the pair can never be applied half way.
*/
export function flipSignFormat(
format: Readonly<
Pick<ImportFormat, "amountMode" | "signConvention" | "columnMapping">
>
): Pick<ImportFormat, "signConvention" | "columnMapping"> {
if (format.amountMode === "debit_credit") {
const { debitAmount, creditAmount, ...rest } = format.columnMapping;
return {
signConvention: format.signConvention,
columnMapping: {
...rest,
...(creditAmount !== undefined ? { debitAmount: creditAmount } : {}),
...(debitAmount !== undefined ? { creditAmount: debitAmount } : {}),
},
};
}
return {
columnMapping: format.columnMapping,
signConvention:
format.signConvention === "negative_expense"
? "positive_expense"
: "negative_expense",
};
}