From c7914c2ca5556c23442e5d33ad8cde544464df15 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sun, 5 Jul 2026 20:14:23 -0400 Subject: [PATCH] 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