From a764ae0d3832546c9fb2a80d98dc158095c1fabc Mon Sep 17 00:00:00 2001
From: medic-bot <medic-bot@lacompagniemaximus.com>
Date: Wed, 11 Mar 2026 08:05:34 -0400
Subject: [PATCH] fix: address reviewer feedback (#34)

Fix sign bug in previous year actuals column: transaction amounts are
stored with sign in the DB (expenses negative) but budget entries are
always positive. Apply Math.abs() when building the previousYearTotal
map so the display-time sign multiplier works correctly.

Add unit tests for the normalization logic verifying that both expense
(negative in DB) and income (positive in DB) amounts are correctly
handled.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/hooks/useBudget.test.ts | 62 +++++++++++++++++++++++++++++++++++++
 src/hooks/useBudget.ts      |  4 ++-
 2 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 src/hooks/useBudget.test.ts

diff --git a/src/hooks/useBudget.test.ts b/src/hooks/useBudget.test.ts
new file mode 100644
index 0000000..baa88af
--- /dev/null
+++ b/src/hooks/useBudget.test.ts
@@ -0,0 +1,62 @@
+import { describe, it, expect } from "vitest";
+
+/**
+ * Unit tests for the previous-year actuals normalization logic used in useBudget.
+ *
+ * Transaction amounts in the database use signed values (expenses are negative).
+ * Budget entries are always stored as positive values, with sign applied at display time.
+ * When building the previousYearTotal map from raw actuals, we must normalize with
+ * Math.abs() to match the budget convention. Without this, expenses would show
+ * inverted signs (negative × -1 = positive instead of positive × -1 = negative).
+ */
+
+describe("previous year actuals normalization", () => {
+  // Simulates the normalization logic from useBudget.ts
+  function buildPrevYearTotalMap(
+    actuals: Array<{ category_id: number | null; actual: number }>
+  ): Map<number, number> {
+    const prevYearTotalMap = new Map<number, number>();
+    for (const a of actuals) {
+      if (a.category_id != null)
+        prevYearTotalMap.set(a.category_id, Math.abs(a.actual));
+    }
+    return prevYearTotalMap;
+  }
+
+  it("should store absolute values for expense actuals (negative in DB)", () => {
+    const actuals = [{ category_id: 1, actual: -500 }]; // expense: negative in DB
+    const map = buildPrevYearTotalMap(actuals);
+    expect(map.get(1)).toBe(500);
+  });
+
+  it("should store absolute values for income actuals (positive in DB)", () => {
+    const actuals = [{ category_id: 2, actual: 3000 }]; // income: positive in DB
+    const map = buildPrevYearTotalMap(actuals);
+    expect(map.get(2)).toBe(3000);
+  });
+
+  it("should skip null category_id entries", () => {
+    const actuals = [{ category_id: null, actual: -100 }];
+    const map = buildPrevYearTotalMap(actuals);
+    expect(map.size).toBe(0);
+  });
+
+  it("should handle zero actuals", () => {
+    const actuals = [{ category_id: 3, actual: 0 }];
+    const map = buildPrevYearTotalMap(actuals);
+    expect(map.get(3)).toBe(0);
+  });
+
+  it("should display correctly with sign multiplier applied", () => {
+    // Simulate the display logic: previousYearTotal * sign
+    const signFor = (type: string) => (type === "expense" ? -1 : 1);
+
+    // Expense category: actual was -500 in DB → normalized to 500 → displayed as 500 * -1 = -500
+    const expenseActual = Math.abs(-500);
+    expect(expenseActual * signFor("expense")).toBe(-500);
+
+    // Income category: actual was 3000 in DB → normalized to 3000 → displayed as 3000 * 1 = 3000
+    const incomeActual = Math.abs(3000);
+    expect(incomeActual * signFor("income")).toBe(3000);
+  });
+});
diff --git a/src/hooks/useBudget.ts b/src/hooks/useBudget.ts
index efa99c4..aa04a99 100644
--- a/src/hooks/useBudget.ts
+++ b/src/hooks/useBudget.ts
@@ -90,9 +90,11 @@ export function useBudget() {
       }
 
       // Build a map for previous year actuals: categoryId -> annual actual total
+      // Use Math.abs because transaction amounts are stored with sign (expenses negative),
+      // but budget entries are always positive — the sign is applied at display time.
       const prevYearTotalMap = new Map<number, number>();
       for (const a of prevYearActuals) {
-        if (a.category_id != null) prevYearTotalMap.set(a.category_id, a.actual);
+        if (a.category_id != null) prevYearTotalMap.set(a.category_id, Math.abs(a.actual));
       }
 
       // Helper: build months array from entryMap