From f45845e408159c3cdeb2e3b5594570a11e916dd9 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sun, 5 Jul 2026 18:36:57 -0400 Subject: [PATCH 1/3] feat(reports): real-vs-real compare as an income statement + previous-month default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "actual vs actual" compare was expenses-only (WHERE amount < 0), so revenue categories were invisible — you could not tell a surplus from a deficit. Make it an income statement (the product's namesake "résultat"): - COMPARE_DELTA_SQL: broaden the WHERE and per-type CASE to include `income` as a signed SUM (like `transfer`), so revenue credits survive the outflow filter. Expenses stay ABS-of-outflows, byte-identical. Transfer netting (#243) intact. - Section order is now income -> expense -> transfer (COMPARE_TYPE_ORDER). - ComparePeriodTable: replace the now-meaningless flat grand total with two result lines — "result before transfers" (revenues - expenses) and the net "result" (after transfers) — extracted into a pure, tested compareResults module. Delta colours are direction-aware (income/result up = green, spending up = red); result amounts are coloured by sign (surplus/deficit). - ComparePeriodChart stays a spending view: filter to expense leaves so revenue bars don't mix into the same axis. - useCompare: skip the initial period-sync so the compare opens on the previous (last complete) month instead of the civil-year December that useReportsPeriod yields by default. Also anchor the `reports/` gitignore to `/reports/` — the unanchored rule was silently ignoring new files under src/components/reports/. Tests: new compareResults.test.ts (result roll-up, unbalanced transfer, deficit, subtotal exclusion); reportService.test.ts updated for the broadened SQL, the income bucket rule, and income-first section order. Build + 685 vitest green. Resolves #253 Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 5 +- CHANGELOG.fr.md | 5 + CHANGELOG.md | 5 + src/components/reports/ComparePeriodChart.tsx | 13 +- src/components/reports/ComparePeriodTable.tsx | 442 +++++++++--------- src/components/reports/compareResults.test.ts | 87 ++++ src/components/reports/compareResults.ts | 90 ++++ src/hooks/useCompare.ts | 10 + src/i18n/locales/en.json | 4 +- src/i18n/locales/fr.json | 4 +- src/services/reportService.test.ts | 35 +- src/services/reportService.ts | 40 +- 12 files changed, 478 insertions(+), 262 deletions(-) create mode 100644 src/components/reports/compareResults.test.ts create mode 100644 src/components/reports/compareResults.ts diff --git a/.gitignore b/.gitignore index a506128..7f440a3 100644 --- a/.gitignore +++ b/.gitignore @@ -63,8 +63,9 @@ src-tauri/icons/ios/ .claude/worktrees/ decisions-log.md -# Autopilot scratch + daily reports -reports/ +# Autopilot scratch + daily reports (repo root only — must not match +# src/components/reports/, which is real tracked source) +/reports/ # Spec scratch (committed only when promoted to docs/archive/) spec-decisions-*.md diff --git a/CHANGELOG.fr.md b/CHANGELOG.fr.md index 41fa9b9..719c4c6 100644 --- a/CHANGELOG.fr.md +++ b/CHANGELOG.fr.md @@ -2,6 +2,11 @@ ## [Non publié] +### Modifié + +- Rapports → Comparaison (réel vs réel) : le rapport est désormais une **analyse de résultat**. Les catégories de revenu apparaissent aux côtés des dépenses, avec deux lignes de résultat — **résultat avant transferts** (revenus − dépenses) et **résultat net** (après annulation des transferts) — pour voir d'un coup d'œil si la période dégage un surplus ou un déficit. Les catégories sont regroupées Revenus → Dépenses → Transferts, les couleurs suivent le sens (plus de revenu = vert, plus de dépense = rouge), et un montant de résultat passe au vert pour un surplus, au rouge pour un déficit (#253). +- Rapports : les rapports comparables s'ouvrent désormais par défaut sur le **mois précédent (dernier mois complet)** au lieu de décembre de l'année civile courante (#253). + ## [0.11.0] - 2026-07-05 ### Ajouté diff --git a/CHANGELOG.md b/CHANGELOG.md index 55adb26..7eeedea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Changed + +- Reports → Compare (real vs real): the report is now an **income statement**. Revenue categories are shown alongside expenses, with two result lines — **result before transfers** (revenues − expenses) and the **net result** (after netting transfers) — so you can see at a glance whether the period ran a surplus or a deficit. Categories are grouped Income → Expenses → Transfers, colours follow direction (more income is green, more spending is red), and a result amount turns green for a surplus, red for a deficit (#253). +- Reports: the comparable reports now open on the **previous (last complete) month** by default instead of the current civil year's December (#253). + ## [0.11.0] - 2026-07-05 ### Added diff --git a/src/components/reports/ComparePeriodChart.tsx b/src/components/reports/ComparePeriodChart.tsx index a539166..719ae91 100644 --- a/src/components/reports/ComparePeriodChart.tsx +++ b/src/components/reports/ComparePeriodChart.tsx @@ -40,13 +40,14 @@ export default function ComparePeriodChart({ ); } - // Sort by current-period amount (largest spending first) so the user's eye - // lands on the biggest categories, then reverse so the biggest appears at - // the top of the vertical bar chart. The table view groups by parent/child - // (Issue #247); the chart stays flat, so drop subtotal (is_parent) rows to - // avoid double-counting a group against its own leaves. + // The chart stays a spending view: the income-statement result (revenues + + // result lines, Issue #253) lives in the table, so keep only expense leaves + // here — mixing revenue bars into the same axis would misread. Drop subtotal + // (is_parent) rows too so a group is not double-counted against its own + // leaves. Sorted by current amount (largest spending first) for a top-down + // read of the biggest movers. const chartData = rows - .filter((r) => !r.is_parent) + .filter((r) => !r.is_parent && (r.category_type ?? "expense") === "expense") .sort((a, b) => b.currentAmount - a.currentAmount) .map((r) => ({ name: r.categoryName, diff --git a/src/components/reports/ComparePeriodTable.tsx b/src/components/reports/ComparePeriodTable.tsx index 568813d..24e6236 100644 --- a/src/components/reports/ComparePeriodTable.tsx +++ b/src/components/reports/ComparePeriodTable.tsx @@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next"; import { ArrowUpDown } from "lucide-react"; import type { CategoryDelta } from "../../shared/types"; import { reorderRows } from "../../utils/reorderRows"; +import { type SectionType, type Totals, sumLeaves, pct, computeResults } from "./compareResults"; export interface ComparePeriodTableProps { rows: CategoryDelta[]; @@ -33,57 +34,30 @@ function formatSignedCurrency(amount: number, language: string): string { }).format(amount); } -function formatPct(pct: number | null, language: string): string { - if (pct === null) return "—"; +function formatPct(pctValue: number | null, language: string): string { + if (pctValue === null) return "—"; return new Intl.NumberFormat(language === "fr" ? "fr-CA" : "en-CA", { style: "percent", maximumFractionDigits: 1, signDisplay: "always", - }).format(pct / 100); + }).format(pctValue / 100); } -function variationColor(value: number): string { - // Compare report deals with expenses (abs values): spending more is negative - // for the user, spending less is positive. Mirror that colour convention - // consistently so the eye parses the delta sign at a glance. - if (value > 0) return "var(--negative, #ef4444)"; - if (value < 0) return "var(--positive, #10b981)"; - return ""; +/** + * Delta colour, direction-aware (Issue #253). Expenses/transfers keep the + * spending convention (increase → red, decrease → green); income and the + * result lines invert it (higher is better → green). Also used to colour a + * result *amount* by sign: a surplus is green, a deficit red. + */ +function deltaColor(value: number, higherIsBetter: boolean): string { + if (value === 0) return ""; + const good = higherIsBetter ? value > 0 : value < 0; + return good ? "var(--positive, #10b981)" : "var(--negative, #ef4444)"; } const STORAGE_KEY = "compare-subtotals-position"; -type SectionType = "expense" | "income" | "transfer"; - -/** Aggregate of the 6 comparable figures across a set of leaf rows. */ -interface Totals { - monthCurrent: number; - monthPrevious: number; - monthDelta: number; - ytdCurrent: number; - ytdPrevious: number; - ytdDelta: number; -} - -function sumLeaves(rows: CategoryDelta[]): Totals { - return rows - .filter((r) => !r.is_parent) - .reduce( - (acc, r) => ({ - monthCurrent: acc.monthCurrent + r.currentAmount, - monthPrevious: acc.monthPrevious + r.previousAmount, - monthDelta: acc.monthDelta + r.deltaAbs, - ytdCurrent: acc.ytdCurrent + r.cumulativeCurrentAmount, - ytdPrevious: acc.ytdPrevious + r.cumulativePreviousAmount, - ytdDelta: acc.ytdDelta + r.cumulativeDeltaAbs, - }), - { monthCurrent: 0, monthPrevious: 0, monthDelta: 0, ytdCurrent: 0, ytdPrevious: 0, ytdDelta: 0 }, - ); -} - -function pct(delta: number, previous: number): number | null { - return previous !== 0 ? (delta / Math.abs(previous)) * 100 : null; -} +const COL_COUNT = 9; export default function ComparePeriodTable({ rows, @@ -112,7 +86,8 @@ export default function ComparePeriodTable({ const ytdPrevLabel = cumulativePreviousLabel ?? previousLabel; const ytdCurrLabel = cumulativeCurrentLabel ?? currentLabel; - // Group rows into contiguous type sections (the service already type-sorts). + // Group rows into contiguous type sections (the service already type-sorts: + // income → expense → transfer). const sectionLabels: Record = { expense: t("reports.compare.sections.expenses"), income: t("reports.compare.sections.income"), @@ -134,8 +109,197 @@ export default function ComparePeriodTable({ sections[sections.length - 1].rows.push(row); } - // Grand totals across every leaf. - const totals = sumLeaves(rows); + // Income-statement result lines (Issue #253): revenues − expenses, then the + // net after transfers. Replaces the old flat grand total, which is meaningless + // once revenues and (ABS) expenses share one table. + const results = computeResults(rows); + const nonTransferSections = sections.filter((s) => s.type !== "transfer"); + const transferSection = sections.find((s) => s.type === "transfer"); + + const renderSection = (section: { type: SectionType; rows: CategoryDelta[] }) => { + // Income section: an increase is good (green); expenses/transfers keep the + // spending convention (increase → red). + const higherIsBetter = section.type === "income"; + const sectionTotals = sumLeaves(section.rows); + return ( + + + + {sectionLabels[section.type]} + + + {reorderRows(section.rows, subtotalsOnTop).map((row) => { + const isParent = row.is_parent ?? false; + const depth = row.depth ?? 0; + const isTopParent = isParent && depth === 0; + const isIntermediateParent = isParent && depth >= 1; + const paddingClass = + depth >= 3 ? "pl-20" : depth === 2 ? "pl-14" : depth === 1 ? "pl-8" : "px-3"; + return ( + + + + + {row.categoryName} + + + {/* Monthly block */} + + {formatCurrency(row.currentAmount, lang)} + + + {formatCurrency(row.previousAmount, lang)} + + + {formatSignedCurrency(row.deltaAbs, lang)} + + + {formatPct(row.deltaPct, lang)} + + {/* Cumulative YTD block */} + + {formatCurrency(row.cumulativeCurrentAmount, lang)} + + + {formatCurrency(row.cumulativePreviousAmount, lang)} + + + {formatSignedCurrency(row.cumulativeDeltaAbs, lang)} + + + {formatPct(row.cumulativeDeltaPct, lang)} + + + ); + })} + {/* Section net total */} + + + {t(sectionTotalKeys[section.type])} + + + {formatCurrency(sectionTotals.monthCurrent, lang)} + + + {formatCurrency(sectionTotals.monthPrevious, lang)} + + + {formatSignedCurrency(sectionTotals.monthDelta, lang)} + + + {formatPct(pct(sectionTotals.monthDelta, sectionTotals.monthPrevious), lang)} + + + {formatCurrency(sectionTotals.ytdCurrent, lang)} + + + {formatCurrency(sectionTotals.ytdPrevious, lang)} + + + {formatSignedCurrency(sectionTotals.ytdDelta, lang)} + + + {formatPct(pct(sectionTotals.ytdDelta, sectionTotals.ytdPrevious), lang)} + + + + ); + }; + + // A result line (before-transfers subtotal or the net total). Higher is always + // better here: amounts are coloured by sign (surplus green / deficit red) and + // deltas by improvement. + const renderResultRow = (labelKey: string, tot: Totals, strong: boolean) => { + const rowBg = strong + ? "bg-[color-mix(in_srgb,var(--muted)_20%,var(--card))]" + : "bg-[color-mix(in_srgb,var(--muted)_10%,var(--card))]"; + const rowClass = strong + ? `border-t-2 border-[var(--border)] font-bold text-sm ${rowBg}` + : `border-b border-[var(--border)] font-semibold text-sm ${rowBg}`; + const cell = "text-right px-3 py-3 tabular-nums"; + return ( + + {t(labelKey)} + + {formatCurrency(tot.monthCurrent, lang)} + + + {formatCurrency(tot.monthPrevious, lang)} + + + {formatSignedCurrency(tot.monthDelta, lang)} + + + {formatPct(pct(tot.monthDelta, tot.monthPrevious), lang)} + + + {formatCurrency(tot.ytdCurrent, lang)} + + + {formatCurrency(tot.ytdPrevious, lang)} + + + {formatSignedCurrency(tot.ytdDelta, lang)} + + + {formatPct(pct(tot.ytdDelta, tot.ytdPrevious), lang)} + + + ); + }; return (
@@ -205,187 +369,25 @@ export default function ComparePeriodTable({ {rows.length === 0 ? ( - + {t("reports.empty.noData")} ) : ( <> - {sections.map((section) => { - const sectionTotals = sumLeaves(section.rows); - return ( - - - - {sectionLabels[section.type]} - - - {reorderRows(section.rows, subtotalsOnTop).map((row) => { - const isParent = row.is_parent ?? false; - const depth = row.depth ?? 0; - const isTopParent = isParent && depth === 0; - const isIntermediateParent = isParent && depth >= 1; - const paddingClass = - depth >= 3 ? "pl-20" : depth === 2 ? "pl-14" : depth === 1 ? "pl-8" : "px-3"; - return ( - - - - - {row.categoryName} - - - {/* Monthly block */} - - {formatCurrency(row.currentAmount, lang)} - - - {formatCurrency(row.previousAmount, lang)} - - - {formatSignedCurrency(row.deltaAbs, lang)} - - - {formatPct(row.deltaPct, lang)} - - {/* Cumulative YTD block */} - - {formatCurrency(row.cumulativeCurrentAmount, lang)} - - - {formatCurrency(row.cumulativePreviousAmount, lang)} - - - {formatSignedCurrency(row.cumulativeDeltaAbs, lang)} - - - {formatPct(row.cumulativeDeltaPct, lang)} - - - ); - })} - {/* Section net total */} - - - {t(sectionTotalKeys[section.type])} - - - {formatCurrency(sectionTotals.monthCurrent, lang)} - - - {formatCurrency(sectionTotals.monthPrevious, lang)} - - - {formatSignedCurrency(sectionTotals.monthDelta, lang)} - - - {formatPct(pct(sectionTotals.monthDelta, sectionTotals.monthPrevious), lang)} - - - {formatCurrency(sectionTotals.ytdCurrent, lang)} - - - {formatCurrency(sectionTotals.ytdPrevious, lang)} - - - {formatSignedCurrency(sectionTotals.ytdDelta, lang)} - - - {formatPct(pct(sectionTotals.ytdDelta, sectionTotals.ytdPrevious), lang)} - - - - ); - })} - {/* Grand totals row */} - - - {t("reports.compare.totalRow")} - - - {formatCurrency(totals.monthCurrent, lang)} - - - {formatCurrency(totals.monthPrevious, lang)} - - - {formatSignedCurrency(totals.monthDelta, lang)} - - - {formatPct(pct(totals.monthDelta, totals.monthPrevious), lang)} - - - {formatCurrency(totals.ytdCurrent, lang)} - - - {formatCurrency(totals.ytdPrevious, lang)} - - - {formatSignedCurrency(totals.ytdDelta, lang)} - - - {formatPct(pct(totals.ytdDelta, totals.ytdPrevious), lang)} - - + {nonTransferSections.map(renderSection)} + {/* Operating result (revenues − expenses), shown before the + transfers section only when transfers exist — otherwise it + equals the net total below and would just be noise. */} + {transferSection && + renderResultRow( + "reports.compare.resultBeforeTransfers", + results.resultBefore, + false, + )} + {transferSection && renderSection(transferSection)} + {/* Bottom line: result after netting transfers. */} + {renderResultRow("reports.compare.resultNet", results.resultNet, true)} )} diff --git a/src/components/reports/compareResults.test.ts b/src/components/reports/compareResults.test.ts new file mode 100644 index 0000000..aa1ef2e --- /dev/null +++ b/src/components/reports/compareResults.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from "vitest"; +import type { CategoryDelta } from "../../shared/types"; +import { computeResults, sumLeaves } from "./compareResults"; + +// Minimal leaf factory. Amounts arrive from COMPARE_DELTA_SQL already in the +// per-type sign convention (expenses = positive ABS, income/transfer = signed). +function leaf( + type: "expense" | "income" | "transfer", + mc: number, + mp: number, + cc = mc, + cp = mp, +): CategoryDelta { + return { + categoryId: 1, + categoryName: type, + categoryColor: "#000", + previousAmount: mp, + currentAmount: mc, + deltaAbs: mc - mp, + deltaPct: mp !== 0 ? ((mc - mp) / mp) * 100 : null, + cumulativePreviousAmount: cp, + cumulativeCurrentAmount: cc, + cumulativeDeltaAbs: cc - cp, + cumulativeDeltaPct: cp !== 0 ? ((cc - cp) / cp) * 100 : null, + category_type: type, + is_parent: false, + }; +} + +describe("computeResults — income-statement roll-up (Issue #253)", () => { + it("resultBefore = revenues − expenses; resultNet adds net transfers", () => { + const r = computeResults([ + leaf("income", 4350, 4500), + leaf("expense", 3900, 3700), + leaf("transfer", 0, 0), + ]); + // Operating result: 4350 − 3900 = 450 current; 4500 − 3700 = 800 previous. + expect(r.resultBefore.monthCurrent).toBe(450); + expect(r.resultBefore.monthPrevious).toBe(800); + expect(r.resultBefore.monthDelta).toBe(-350); + // Balanced transfer (0) → net equals the operating result. + expect(r.resultNet.monthCurrent).toBe(450); + expect(r.hasTransfers).toBe(true); + }); + + it("an unbalanced (one-leg) transfer moves resultNet away from resultBefore", () => { + const r = computeResults([ + leaf("income", 4000, 4000), + leaf("expense", 3000, 3000), + leaf("transfer", -200, 0), // a single categorised leg + ]); + expect(r.resultBefore.monthCurrent).toBe(1000); + // Net folds the residual transfer in: 1000 + (−200) = 800. + expect(r.resultNet.monthCurrent).toBe(800); + }); + + it("a deficit yields a negative net result and no transfer line", () => { + const r = computeResults([leaf("income", 2000, 2000), leaf("expense", 2500, 2400)]); + expect(r.resultNet.monthCurrent).toBe(-500); + expect(r.hasTransfers).toBe(false); + }); + + it("ignores subtotal (is_parent) rows so a group is not double-counted", () => { + const parent = { ...leaf("expense", 999, 999), is_parent: true }; + const r = computeResults([parent, leaf("expense", 100, 80)]); + expect(r.expense.monthCurrent).toBe(100); + }); + + it("carries the YTD figures through the same arithmetic", () => { + const r = computeResults([ + leaf("income", 100, 100, 1200, 1150), + leaf("expense", 60, 60, 700, 650), + ]); + expect(r.resultBefore.ytdCurrent).toBe(500); + expect(r.resultBefore.ytdPrevious).toBe(500); + }); + + it("sumLeaves adds only leaves", () => { + const t = sumLeaves([ + leaf("expense", 100, 50), + { ...leaf("expense", 999, 999), is_parent: true }, + ]); + expect(t.monthCurrent).toBe(100); + expect(t.monthPrevious).toBe(50); + }); +}); diff --git a/src/components/reports/compareResults.ts b/src/components/reports/compareResults.ts new file mode 100644 index 0000000..57af50e --- /dev/null +++ b/src/components/reports/compareResults.ts @@ -0,0 +1,90 @@ +import type { CategoryDelta } from "../../shared/types"; + +export type SectionType = "expense" | "income" | "transfer"; + +/** Aggregate of the 6 comparable figures across a set of leaf rows. */ +export interface Totals { + monthCurrent: number; + monthPrevious: number; + monthDelta: number; + ytdCurrent: number; + ytdPrevious: number; + ytdDelta: number; +} + +const ZERO: Totals = { + monthCurrent: 0, + monthPrevious: 0, + monthDelta: 0, + ytdCurrent: 0, + ytdPrevious: 0, + ytdDelta: 0, +}; + +/** Sum every non-subtotal (leaf) row into a single Totals. */ +export function sumLeaves(rows: CategoryDelta[]): Totals { + return rows + .filter((r) => !r.is_parent) + .reduce( + (acc, r) => ({ + monthCurrent: acc.monthCurrent + r.currentAmount, + monthPrevious: acc.monthPrevious + r.previousAmount, + monthDelta: acc.monthDelta + r.deltaAbs, + ytdCurrent: acc.ytdCurrent + r.cumulativeCurrentAmount, + ytdPrevious: acc.ytdPrevious + r.cumulativePreviousAmount, + ytdDelta: acc.ytdDelta + r.cumulativeDeltaAbs, + }), + { ...ZERO }, + ); +} + +export function pct(delta: number, previous: number): number | null { + return previous !== 0 ? (delta / Math.abs(previous)) * 100 : null; +} + +/** Component-wise a + sign*b across the 6 figures. */ +function combine(a: Totals, b: Totals, sign: 1 | -1): Totals { + return { + monthCurrent: a.monthCurrent + sign * b.monthCurrent, + monthPrevious: a.monthPrevious + sign * b.monthPrevious, + monthDelta: a.monthDelta + sign * b.monthDelta, + ytdCurrent: a.ytdCurrent + sign * b.ytdCurrent, + ytdPrevious: a.ytdPrevious + sign * b.ytdPrevious, + ytdDelta: a.ytdDelta + sign * b.ytdDelta, + }; +} + +export interface CompareResults { + income: Totals; + expense: Totals; + transfer: Totals; + /** Revenues − expenses: the operating result, before transfers. */ + resultBefore: Totals; + /** resultBefore + net transfers: the bottom-line total. */ + resultNet: Totals; + hasTransfers: boolean; +} + +/** + * Income-statement roll-up for the real-vs-real compare (Issue #253). + * + * Sign convention flowing in from COMPARE_DELTA_SQL: expenses are positive + * magnitudes (ABS of outflows), income is a signed SUM (positive), transfers a + * signed net (~0 when balanced). So the operating result is a straight + * `income − expense`, and the net adds the (usually ~0) transfer total. When a + * transfer is unbalanced (a single leg categorised) its residual shows up as + * the gap between resultBefore and resultNet. + */ +export function computeResults(rows: CategoryDelta[]): CompareResults { + const byType = (type: SectionType): Totals => + sumLeaves(rows.filter((r) => (r.category_type ?? "expense") === type)); + const income = byType("income"); + const expense = byType("expense"); + const transfer = byType("transfer"); + const resultBefore = combine(income, expense, -1); + const resultNet = combine(resultBefore, transfer, 1); + const hasTransfers = rows.some( + (r) => (r.category_type ?? "expense") === "transfer" && !r.is_parent, + ); + return { income, expense, transfer, resultBefore, resultNet, hasTransfers }; +} diff --git a/src/hooks/useCompare.ts b/src/hooks/useCompare.ts index 965dd81..0055468 100644 --- a/src/hooks/useCompare.ts +++ b/src/hooks/useCompare.ts @@ -122,7 +122,17 @@ export function useCompare() { // When the URL period changes, align the reference month with `to`. // The explicit dropdown remains the primary selector — this effect only // keeps the two in sync when the user navigates via PeriodSelector. + // + // Skip the FIRST run (mount): useReportsPeriod defaults to the civil-year + // range, whose `to` (Dec 31) would otherwise snap the reference month to a + // future, empty December and clobber the previous-month default carried by + // initialState (Issue #253). Only user-driven period changes sync afterwards. + const didInitPeriodSync = useRef(false); useEffect(() => { + if (!didInitPeriodSync.current) { + didInitPeriodSync.current = true; + return; + } const [y, m] = to.split("-").map(Number); if (!Number.isFinite(y) || !Number.isFinite(m)) return; if (y !== state.year || m !== state.month) { diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 4ed8412..c7f4647 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -435,7 +435,9 @@ }, "totalExpenses": "Total Expenses", "totalIncome": "Total Income", - "totalTransfers": "Total Transfers" + "totalTransfers": "Total Transfers", + "resultBeforeTransfers": "Result before transfers", + "resultNet": "Net result" }, "cartes": { "kpiSectionAria": "Key indicators for the reference month", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 792aca3..93e585e 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -435,7 +435,9 @@ }, "totalExpenses": "Total des dépenses", "totalIncome": "Total des revenus", - "totalTransfers": "Total des transferts" + "totalTransfers": "Total des transferts", + "resultBeforeTransfers": "Résultat avant transferts", + "resultNet": "Résultat net" }, "cartes": { "kpiSectionAria": "Indicateurs clés du mois de référence", diff --git a/src/services/reportService.test.ts b/src/services/reportService.test.ts index 8968390..a4373bf 100644 --- a/src/services/reportService.test.ts +++ b/src/services/reportService.test.ts @@ -487,8 +487,9 @@ describe("real-vs-real compare — transfer netting (Issue #243)", () => { // compare. Ordinary categories keep the expense-report behavior (only // outflows, summed as positive magnitudes). The netting lives entirely in // the SQL (COMPARE_DELTA_SQL): a signed SUM(t.amount) for `transfer`, - // ABS(t.amount) filtered on amount < 0 for everything else, with the WHERE - // broadened so transfer credits are not dropped before they can cancel. + // ABS(t.amount) filtered on amount < 0 for plain expenses. Income, like + // transfers, is a signed SUM (Issue #253); the WHERE is broadened so + // income/transfer credits are not dropped before they can count or cancel. // // There is no runnable SQLite in these unit tests (tauri-plugin-sql only // exists inside the Tauri WebView, and CI runs Node 20 → no node:sqlite), so @@ -499,11 +500,12 @@ describe("real-vs-real compare — transfer netting (Issue #243)", () => { /** Per-row contribution to a bucket — the exact rule COMPARE_DELTA_SQL encodes. */ function bucketContribution(amount: number, type: string | null): number { - return (type ?? "expense") === "transfer" - ? amount // signed → a debit and its matching credit cancel + const t = type ?? "expense"; + return t === "transfer" || t === "income" + ? amount // signed → transfer debit/credit cancel; income credits kept as revenue (#253) : amount < 0 ? Math.abs(amount) // expense outflow as a positive magnitude - : 0; // non-transfer credits dropped → no revenues surfaced + : 0; // other (expense) credits dropped — the report stays outflow-only for them } it("nets a balanced transfer to ~0 while an expense keeps its sum of outflows", () => { @@ -518,6 +520,11 @@ describe("real-vs-real compare — transfer netting (Issue #243)", () => { // Uncategorized (c.type NULL) defaults to expense behavior. const uncategorized = [-40, 15].reduce((s, a) => s + bucketContribution(a, null), 0); expect(uncategorized).toBe(40); + + // Income (paie + a small correction) is a signed SUM — credits are kept as + // revenue (Issue #253), not dropped like non-transfer expense credits. + const income = [4200, 150, -50].reduce((s, a) => s + bucketContribution(a, "income"), 0); + expect(income).toBe(4300); }); it("getCompareMonthOverMonth SQL nets transfers (signed) but keeps ABS for other types", async () => { @@ -528,11 +535,11 @@ describe("real-vs-real compare — transfer netting (Issue #243)", () => { const sql = mockSelect.mock.calls[0][0] as string; // transfer → signed t.amount; every other type → ABS(t.amount) expect(sql).toContain( - "COALESCE(c.type, 'expense') = 'transfer' THEN t.amount ELSE ABS(t.amount)", + "COALESCE(c.type, 'expense') IN ('transfer', 'income') THEN t.amount ELSE ABS(t.amount)", ); // WHERE broadened so transfer credits survive the expense (amount < 0) filter expect(sql).toContain( - "WHERE (t.amount < 0 OR COALESCE(c.type, 'expense') = 'transfer')", + "WHERE (t.amount < 0 OR COALESCE(c.type, 'expense') IN ('transfer', 'income'))", ); // still four date buckets expect(sql).toContain("month_current_total"); @@ -546,10 +553,10 @@ describe("real-vs-real compare — transfer netting (Issue #243)", () => { const sql = mockSelect.mock.calls[0][0] as string; expect(sql).toContain( - "COALESCE(c.type, 'expense') = 'transfer' THEN t.amount ELSE ABS(t.amount)", + "COALESCE(c.type, 'expense') IN ('transfer', 'income') THEN t.amount ELSE ABS(t.amount)", ); expect(sql).toContain( - "WHERE (t.amount < 0 OR COALESCE(c.type, 'expense') = 'transfer')", + "WHERE (t.amount < 0 OR COALESCE(c.type, 'expense') IN ('transfer', 'income'))", ); }); @@ -678,6 +685,8 @@ describe("buildCompareTree — hierarchical real-vs-real (Issue #247)", () => { { id: 24, name: "Restaurant", color: "#f97316", type: "expense", parent_id: 2 }, { id: 5, name: "Placements", color: null, type: "transfer", parent_id: null }, { id: 50, name: "REER", color: null, type: "transfer", parent_id: 5 }, + { id: 8, name: "Revenus", color: null, type: "income", parent_id: null }, + { id: 80, name: "Paie", color: null, type: "income", parent_id: 8 }, ] as Parameters[1]; it("nests leaves under a parent subtotal equal to the sum of its children", () => { @@ -723,14 +732,14 @@ describe("buildCompareTree — hierarchical real-vs-real (Issue #247)", () => { expect(parent.deltaAbs).toBe(0); }); - it("orders sections expense → income → transfer and keeps subtrees contiguous", () => { + it("orders sections income → expense → transfer and keeps subtrees contiguous", () => { const rows = buildCompareTree( - [leaf(22, "Épicerie", 500, 400), leaf(50, "REER", 0, 0)], + [leaf(80, "Paie", 4200, 4000), leaf(22, "Épicerie", 500, 400), leaf(50, "REER", 0, 0)], cats, ); const types = rows.map((r) => r.category_type); - // All expense rows precede all transfer rows. - expect(types).toEqual(["expense", "expense", "transfer", "transfer"]); + // Income section first, then expenses, then transfers (Issue #253). + expect(types).toEqual(["income", "income", "expense", "expense", "transfer", "transfer"]); }); it("preserves grand-total invariance vs the flat leaves", () => { diff --git a/src/services/reportService.ts b/src/services/reportService.ts index c24413d..a832332 100644 --- a/src/services/reportService.ts +++ b/src/services/reportService.ts @@ -447,7 +447,9 @@ interface CompareCatMeta { const COMPARE_CATEGORIES_SQL = `SELECT id, name, color, type, parent_id FROM categories`; -const COMPARE_TYPE_ORDER: Record = { expense: 0, income: 1, transfer: 2 }; +// Income statement reading order: revenues first, then expenses, then the +// (netted) transfers — the two result lines are injected around them in the UI. +const COMPARE_TYPE_ORDER: Record = { income: 0, expense: 1, transfer: 2 }; /** Depth cap mirroring CATEGORY_TREE_CTE — guards a cyclic parent_id chain. */ const MAX_TREE_DEPTH = 5; @@ -625,7 +627,7 @@ export function buildCompareTree( rows.push({ ...d, parent_id: null, is_parent: false, depth: 0, category_type: d.category_type ?? "expense" }); } - // Stable sort by type so sections (expense → income → transfer) are + // Stable sort by type so sections (income → expense → transfer) are // contiguous; magnitude/tree order within a type is preserved via the index. const order = new Map(); rows.forEach((r, i) => order.set(r, i)); @@ -649,33 +651,33 @@ function previousMonth(year: number, month: number): { year: number; month: numb * exact same query — only the eight date bounds differ). Four date buckets * ($1..$8): monthly current/previous and cumulative current/previous. * - * Per-category sign convention (Issue #243): - * - `transfer` categories NET via a signed SUM(t.amount), so a balanced - * debit/credit pair (e.g. "Paiement CC": -500 out, +500 in) cancels to ~0. - * A transfer is a money move, not spending, and must not inflate the - * expense figures. + * This report is an INCOME STATEMENT ("analyse de résultat", Issue #253), not a + * pure expense report: revenues are surfaced alongside expenses so the compare + * can show a result (revenues − expenses). Per-category sign convention: + * - `income` and `transfer` categories use a signed SUM(t.amount). Income + * credits stay positive; a `transfer` debit/credit pair (e.g. "Paiement CC": + * -500 out, +500 in) nets to ~0 (Issue #243 — a money move, not spending). * - Every other type keeps the expense-report behavior: only outflows * (amount < 0) count, summed as positive magnitudes via ABS. * - * The WHERE is broadened with `OR COALESCE(c.type, 'expense') = 'transfer'` so - * transfer *credits* survive the `amount < 0` expense filter and can actually - * cancel their debits — without it the credit leg would be dropped and the - * category could never net to zero. For non-transfer rows the WHERE still - * admits only outflows, so their totals are byte-identical to the previous - * behavior (no revenues surfaced, no zero-rows for pure-income categories). - * Uncategorized rows (LEFT JOIN → c.type NULL) default to 'expense'. + * The WHERE admits outflows OR any income/transfer row, so income and transfer + * *credits* survive the `amount < 0` filter — without it a pure-income category + * would be dropped entirely and a transfer could never net to zero. Expense + * rows still contribute only their outflows, byte-identical to before. The two + * result lines (before/after transfers) are computed in ComparePeriodTable from + * the per-type subtotals, not here. Uncategorized rows (c.type NULL) → 'expense'. */ const COMPARE_DELTA_SQL = `SELECT t.category_id, COALESCE(c.name, 'Uncategorized') AS category_name, COALESCE(c.color, '#9ca3af') AS category_color, - COALESCE(SUM(CASE WHEN t.date >= $1 AND t.date <= $2 THEN CASE WHEN COALESCE(c.type, 'expense') = 'transfer' THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS month_current_total, - COALESCE(SUM(CASE WHEN t.date >= $3 AND t.date <= $4 THEN CASE WHEN COALESCE(c.type, 'expense') = 'transfer' THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS month_previous_total, - COALESCE(SUM(CASE WHEN t.date >= $5 AND t.date <= $6 THEN CASE WHEN COALESCE(c.type, 'expense') = 'transfer' THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS cumulative_current_total, - COALESCE(SUM(CASE WHEN t.date >= $7 AND t.date <= $8 THEN CASE WHEN COALESCE(c.type, 'expense') = 'transfer' THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS cumulative_previous_total + COALESCE(SUM(CASE WHEN t.date >= $1 AND t.date <= $2 THEN CASE WHEN COALESCE(c.type, 'expense') IN ('transfer', 'income') THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS month_current_total, + COALESCE(SUM(CASE WHEN t.date >= $3 AND t.date <= $4 THEN CASE WHEN COALESCE(c.type, 'expense') IN ('transfer', 'income') THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS month_previous_total, + COALESCE(SUM(CASE WHEN t.date >= $5 AND t.date <= $6 THEN CASE WHEN COALESCE(c.type, 'expense') IN ('transfer', 'income') THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS cumulative_current_total, + COALESCE(SUM(CASE WHEN t.date >= $7 AND t.date <= $8 THEN CASE WHEN COALESCE(c.type, 'expense') IN ('transfer', 'income') THEN t.amount ELSE ABS(t.amount) END ELSE 0 END), 0) AS cumulative_previous_total FROM transactions t LEFT JOIN categories c ON t.category_id = c.id - WHERE (t.amount < 0 OR COALESCE(c.type, 'expense') = 'transfer') + WHERE (t.amount < 0 OR COALESCE(c.type, 'expense') IN ('transfer', 'income')) AND ( (t.date >= $1 AND t.date <= $2) OR (t.date >= $3 AND t.date <= $4) From d57b2563af18868e1b1b1103c450609d414ecb7e Mon Sep 17 00:00:00 2001 From: le king fu Date: Sun, 5 Jul 2026 19:37:22 -0400 Subject: [PATCH 2/3] fix(reports): StrictMode-safe period sync + review polish (#253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses /pr-review REQUEST_CHANGES on #255. - useCompare: the "skip first sync" boolean was not StrictMode-safe — the dev double-invoke of effects flipped the flag on setup #1, so setup #2 re-synced the reference month to the civil-year December, re-introducing the very bug Changement 2 fixes (dev only; prod has no double-invoke). Replace it with a value-change guard: a ref seeded with the initial `to` plus a pure syncReferenceOnPeriodChange() that only dispatches when `to` actually changes. Idempotent across the double-invoke, and now unit-tested (5 cases) since the decision is a pure function (the project has no renderHook harness). - Remove the now-orphaned reports.compare.totalRow i18n key (both locales) — the flat grand total it labelled was replaced by the result lines. - ComparePeriodTable: gate the "before transfers" line on results.hasTransfers (previously computed/tested but unused). - ComparePeriodChart: show the no-data empty state when the expense filter leaves nothing (a pure income/transfer period) instead of bare axes. Build + 690 vitest green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/components/reports/ComparePeriodChart.tsx | 18 +++--- src/components/reports/ComparePeriodTable.tsx | 2 +- src/hooks/useCompare.test.ts | 43 +++++++++++++- src/hooks/useCompare.ts | 58 +++++++++++++------ src/i18n/locales/en.json | 1 - src/i18n/locales/fr.json | 1 - 6 files changed, 93 insertions(+), 30 deletions(-) diff --git a/src/components/reports/ComparePeriodChart.tsx b/src/components/reports/ComparePeriodChart.tsx index 719ae91..290e333 100644 --- a/src/components/reports/ComparePeriodChart.tsx +++ b/src/components/reports/ComparePeriodChart.tsx @@ -32,14 +32,6 @@ export default function ComparePeriodChart({ }: ComparePeriodChartProps) { const { t, i18n } = useTranslation(); - if (rows.length === 0) { - return ( -
- {t("reports.empty.noData")} -
- ); - } - // The chart stays a spending view: the income-statement result (revenues + // result lines, Issue #253) lives in the table, so keep only expense leaves // here — mixing revenue bars into the same axis would misread. Drop subtotal @@ -56,6 +48,16 @@ export default function ComparePeriodChart({ color: r.categoryColor, })); + // Empty when there is no data at all, or when the period is pure + // income/transfers (no expense bars to draw) — avoid rendering bare axes. + if (chartData.length === 0) { + return ( +
+ {t("reports.empty.noData")} +
+ ); + } + const previousFill = "var(--muted-foreground)"; const currentFill = "var(--primary)"; diff --git a/src/components/reports/ComparePeriodTable.tsx b/src/components/reports/ComparePeriodTable.tsx index 24e6236..acb7a01 100644 --- a/src/components/reports/ComparePeriodTable.tsx +++ b/src/components/reports/ComparePeriodTable.tsx @@ -379,7 +379,7 @@ export default function ComparePeriodTable({ {/* Operating result (revenues − expenses), shown before the transfers section only when transfers exist — otherwise it equals the net total below and would just be noise. */} - {transferSection && + {results.hasTransfers && renderResultRow( "reports.compare.resultBeforeTransfers", results.resultBefore, diff --git a/src/hooks/useCompare.test.ts b/src/hooks/useCompare.test.ts index 1258b3f..500d293 100644 --- a/src/hooks/useCompare.test.ts +++ b/src/hooks/useCompare.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from "vitest"; -import { previousMonth, defaultReferencePeriod, comparisonMeta } from "./useCompare"; +import { + previousMonth, + defaultReferencePeriod, + comparisonMeta, + syncReferenceOnPeriodChange, +} from "./useCompare"; describe("useCompare helpers", () => { describe("previousMonth", () => { @@ -44,4 +49,40 @@ describe("useCompare helpers", () => { expect(comparisonMeta("yoy", 2026, 1)).toEqual({ previousYear: 2025, previousMonth: 1 }); }); }); + + describe("syncReferenceOnPeriodChange", () => { + // The ref is seeded with the initial `to`, so the mount call has to===last. + it("no-ops on mount, preserving the previous-month default", () => { + const r = syncReferenceOnPeriodChange("2026-12-31", "2026-12-31", 2026, 6); + expect(r.next).toBeNull(); + expect(r.lastSyncedTo).toBe("2026-12-31"); + }); + + // StrictMode double-invokes effects in dev: calling twice with the same `to` + // must not sync (a fire-once boolean would wrongly snap to December on run 2). + it("is idempotent across a repeated `to` (StrictMode-safe)", () => { + const a = syncReferenceOnPeriodChange("2026-12-31", "2026-12-31", 2026, 6); + const b = syncReferenceOnPeriodChange(a.lastSyncedTo, "2026-12-31", 2026, 6); + expect(a.next).toBeNull(); + expect(b.next).toBeNull(); + }); + + it("syncs the reference month when `to` changes (user navigation)", () => { + const r = syncReferenceOnPeriodChange("2026-12-31", "2026-06-30", 2026, 12); + expect(r.next).toEqual({ year: 2026, month: 6 }); + expect(r.lastSyncedTo).toBe("2026-06-30"); + }); + + it("records a changed `to` but does not dispatch when it already matches state", () => { + const r = syncReferenceOnPeriodChange("2026-12-31", "2026-06-30", 2026, 6); + expect(r.next).toBeNull(); + expect(r.lastSyncedTo).toBe("2026-06-30"); + }); + + it("ignores an unparseable `to`", () => { + const r = syncReferenceOnPeriodChange("2026-12-31", "garbage", 2026, 6); + expect(r.next).toBeNull(); + expect(r.lastSyncedTo).toBe("garbage"); + }); + }); }); diff --git a/src/hooks/useCompare.ts b/src/hooks/useCompare.ts index 0055468..23e73f7 100644 --- a/src/hooks/useCompare.ts +++ b/src/hooks/useCompare.ts @@ -60,6 +60,32 @@ export function comparisonMeta( return { previousYear: year - 1, previousMonth: month }; } +/** + * Pure decision for the URL-period → reference-month sync effect. + * + * Gates on a *change* of `to` (the period's upper bound) rather than a + * fire-once flag: seeded with the initial `to`, the mount is a no-op, so the + * previous-month default from initialState survives instead of snapping to the + * civil-year December that useReportsPeriod yields by default (Issue #253). + * Because it keys off a value change, it is idempotent under React StrictMode's + * dev double-invoke of effects — called twice with the same `to` it dispatches + * nothing the second time (a fire-once boolean would wrongly sync on run #2). + * + * Returns the `to` to remember plus an optional reference period to dispatch. + */ +export function syncReferenceOnPeriodChange( + lastSyncedTo: string, + to: string, + currentYear: number, + currentMonth: number, +): { lastSyncedTo: string; next: { year: number; month: number } | null } { + if (to === lastSyncedTo) return { lastSyncedTo, next: null }; + const [y, m] = to.split("-").map(Number); + if (!Number.isFinite(y) || !Number.isFinite(m)) return { lastSyncedTo: to, next: null }; + if (y === currentYear && m === currentMonth) return { lastSyncedTo: to, next: null }; + return { lastSyncedTo: to, next: { year: y, month: m } }; +} + const defaultRef = defaultReferencePeriod(); const initialState: State = { mode: "actual", @@ -119,25 +145,21 @@ export function useCompare() { fetch(state.mode, state.subMode, state.year, state.month); }, [fetch, state.mode, state.subMode, state.year, state.month]); - // When the URL period changes, align the reference month with `to`. - // The explicit dropdown remains the primary selector — this effect only - // keeps the two in sync when the user navigates via PeriodSelector. - // - // Skip the FIRST run (mount): useReportsPeriod defaults to the civil-year - // range, whose `to` (Dec 31) would otherwise snap the reference month to a - // future, empty December and clobber the previous-month default carried by - // initialState (Issue #253). Only user-driven period changes sync afterwards. - const didInitPeriodSync = useRef(false); + // Keep the reference month in sync with the URL period when the user navigates + // via PeriodSelector — but not on mount. The ref is seeded with the initial + // `to`, so syncReferenceOnPeriodChange only fires on an actual change of `to`, + // leaving the previous-month default from initialState intact (Issue #253) and + // staying idempotent under StrictMode's dev double-invoke. + const lastSyncedToRef = useRef(to); useEffect(() => { - if (!didInitPeriodSync.current) { - didInitPeriodSync.current = true; - return; - } - const [y, m] = to.split("-").map(Number); - if (!Number.isFinite(y) || !Number.isFinite(m)) return; - if (y !== state.year || m !== state.month) { - dispatch({ type: "SET_REFERENCE_PERIOD", payload: { year: y, month: m } }); - } + const { lastSyncedTo, next } = syncReferenceOnPeriodChange( + lastSyncedToRef.current, + to, + state.year, + state.month, + ); + lastSyncedToRef.current = lastSyncedTo; + if (next) dispatch({ type: "SET_REFERENCE_PERIOD", payload: next }); // eslint-disable-next-line react-hooks/exhaustive-deps }, [to]); diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index c7f4647..937e58c 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -427,7 +427,6 @@ "referenceMonth": "Reference month", "currentAmount": "Current", "previousAmount": "Previous", - "totalRow": "Total", "sections": { "expenses": "Expenses", "income": "Income", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 93e585e..109f0e3 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -427,7 +427,6 @@ "referenceMonth": "Mois de référence", "currentAmount": "Courant", "previousAmount": "Précédent", - "totalRow": "Total", "sections": { "expenses": "Dépenses", "income": "Revenus", From c7914c2ca5556c23442e5d33ad8cde544464df15 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sun, 5 Jul 2026 20:14:23 -0400 Subject: [PATCH 3/3] fix(reports): exclude income from Cartes top movers (#253 regression) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /pr-review on #255 caught a cross-consumer regression: broadening COMPARE_DELTA_SQL to surface income (this PR) also feeds getCartesSnapshot, whose "top movers" card is a spending view (up = red). A salary rise would land under "biggest increases" coloured red — inverted meaning. Filter significantMovers to expense leaves ((category_type ?? "expense") === "expense"), mirroring ComparePeriodChart. The surviving expense output is byte-identical to pre-#253. Add a Cartes regression test (an income category with the biggest delta must not top the movers list) — the existing test mocked the compare SQL without category types, so it stayed silently green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/services/reportService.cartes.test.ts | 27 +++++++++++++++++++++++ src/services/reportService.ts | 18 ++++++++++----- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/services/reportService.cartes.test.ts b/src/services/reportService.cartes.test.ts index 54cda9a..ab39758 100644 --- a/src/services/reportService.cartes.test.ts +++ b/src/services/reportService.cartes.test.ts @@ -235,6 +235,33 @@ describe("getCartesSnapshot", () => { expect(snapshot.topMoversDown[0].categoryName).toBe("D3"); }); + it("excludes income from top movers so a salary rise is not a red 'increase' (#253)", async () => { + // Post-#253 COMPARE_DELTA_SQL surfaces income as a signed positive SUM, so + // revenue rows now reach getCartesSnapshot. Top movers is a spending view + // (up = red), so income must be filtered out — otherwise a salary jump tops + // "biggest increases" in red. The category metadata query types id 99 as + // income; the two expense movers are absent from it and default to expense. + const momRows = [ + { category_id: 99, category_name: "Salaire", category_color: "#000", month_current_total: 5000, month_previous_total: 100, cumulative_current_total: 5000, cumulative_previous_total: 100 }, + { category_id: 1, category_name: "Épicerie", category_color: "#000", month_current_total: 300, month_previous_total: 100, cumulative_current_total: 300, cumulative_previous_total: 100 }, + { category_id: 2, category_name: "Resto", category_color: "#000", month_current_total: 100, month_previous_total: 400, cumulative_current_total: 100, cumulative_previous_total: 400 }, + ]; + routeSelect([ + { match: "strftime('%Y-%m', date)", rows: [{ month: "2026-03", income: 5000, expenses: 400 }] }, + { match: "ORDER BY ABS(month_current_total - month_previous_total) DESC", rows: momRows }, + { match: "parent_id FROM categories", rows: [{ id: 99, name: "Salaire", color: null, type: "income", parent_id: null }] }, + ]); + + const snapshot = await getCartesSnapshot(2026, 3); + const upNames = snapshot.topMoversUp.map((m) => m.categoryName); + // Income excluded despite the biggest delta (+4900). + expect(upNames).not.toContain("Salaire"); + // The biggest EXPENSE increase leads instead. + expect(snapshot.topMoversUp[0].categoryName).toBe("Épicerie"); + // Expense decrease still surfaces. + expect(snapshot.topMoversDown[0].categoryName).toBe("Resto"); + }); + it("computes YTD KPIs correctly when mode=ytd (sums Jan→refMonth of refYear)", async () => { // Reference = 2026-03, YTD = Jan + Feb + Mar of 2026. routeSelect([ diff --git a/src/services/reportService.ts b/src/services/reportService.ts index a832332..49b2e07 100644 --- a/src/services/reportService.ts +++ b/src/services/reportService.ts @@ -1170,12 +1170,20 @@ export async function getCartesSnapshot( // Top movers: biggest MoM increases / decreases. `momRows` now carries the // compare hierarchy (Issue #247) — skip the subtotal (`is_parent`) rows so a - // parent group can't double-count against its own leaves. The surviving leaves - // are byte-identical to the previous flat output; the sort/slice below is - // unchanged. `momRows` are sorted by absolute delta already; filter out - // near-zero noise and split by sign. + // parent group can't double-count against its own leaves. Keep only EXPENSE + // leaves: since Issue #253 broadened COMPARE_DELTA_SQL to surface income (and + // net transfers), momRows now also carries revenue rows — and this card is a + // spending view whose colours read "up = red" (more spending is bad), so a + // salary rise must not appear under "biggest increases" in red. Mirror the + // expense-only filter ComparePeriodChart uses. The surviving expense leaves + // are byte-identical to the pre-#253 flat output. `momRows` are sorted by + // absolute delta already; filter out near-zero noise and split by sign. const significantMovers = momRows.filter( - (r) => !r.is_parent && r.deltaAbs !== 0 && (r.previousAmount > 0 || r.currentAmount > 0), + (r) => + !r.is_parent && + (r.category_type ?? "expense") === "expense" && + r.deltaAbs !== 0 && + (r.previousAmount > 0 || r.currentAmount > 0), ); // Project the richer CategoryDelta shape down to the narrower CartesTopMover // shape so the Cartes dashboard keeps its stable contract regardless of how