fix: address reviewer feedback (#41)

- Default categoryType filter to "expense" so top-N ranking is not
  skewed by mixing income and expense amounts
- Remove redundant showFilterPanel condition (hasCategories already
  covers the overTime tab)
- Add unit tests for getCategoryOverTime query construction and output

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/hooks/useReports.ts

@@ -61,7 +61,7 @@ const initialState: ReportsState = {
   customDateFrom: monthStartStr,
   customDateTo: todayStr,
   sourceId: null,
-  categoryType: null,
+  categoryType: "expense",
   monthlyTrends: [],
   categorySpending: [],
   categoryOverTime: { categories: [], data: [], colors: {}, categoryIds: {} },

src/pages/ReportsPage.tsx

@@ -69,7 +69,7 @@ export default function ReportsPage() {
   const monthOptions = useMemo(() => buildMonthOptions(i18n.language), [i18n.language]);
 
   const hasCategories = ["byCategory", "overTime"].includes(state.tab) && filterCategories.length > 0;
-  const showFilterPanel = hasCategories || state.tab === "overTime" || (state.tab === "trends" && sources.length > 1);
+  const showFilterPanel = hasCategories || (state.tab === "trends" && sources.length > 1);
 
   return (
     <div className={state.isLoading ? "opacity-50 pointer-events-none" : ""}>

src/services/reportService.test.ts

@@ -0,0 +1,146 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { getCategoryOverTime } from "./reportService";
+
+// Mock the db module
+vi.mock("./db", () => ({
+  getDb: vi.fn(),
+}));
+
+import { getDb } from "./db";
+
+const mockSelect = vi.fn();
+const mockDb = { select: mockSelect };
+
+beforeEach(() => {
+  vi.mocked(getDb).mockResolvedValue(mockDb as never);
+  mockSelect.mockReset();
+});
+
+describe("getCategoryOverTime", () => {
+  it("builds query without WHERE clause when no filters are provided", async () => {
+    // First call: top categories, second call: monthly breakdown
+    mockSelect
+      .mockResolvedValueOnce([]) // topCategories
+      .mockResolvedValueOnce([]); // monthlyRows
+
+    await getCategoryOverTime();
+
+    const topCatSQL = mockSelect.mock.calls[0][0] as string;
+    // No WHERE clause should appear (no filters at all)
+    expect(topCatSQL).not.toContain("WHERE");
+  });
+
+  it("applies typeFilter via category type when provided", async () => {
+    mockSelect
+      .mockResolvedValueOnce([]) // topCategories
+      .mockResolvedValueOnce([]); // monthlyRows
+
+    await getCategoryOverTime(undefined, undefined, 50, undefined, "expense");
+
+    const topCatSQL = mockSelect.mock.calls[0][0] as string;
+    const topCatParams = mockSelect.mock.calls[0][1] as unknown[];
+    expect(topCatSQL).toContain("COALESCE(c.type, 'expense')");
+    expect(topCatSQL).toContain("WHERE");
+    expect(topCatParams[0]).toBe("expense");
+  });
+
+  it("applies income typeFilter correctly", async () => {
+    mockSelect
+      .mockResolvedValueOnce([]) // topCategories
+      .mockResolvedValueOnce([]); // monthlyRows
+
+    await getCategoryOverTime("2025-01-01", "2025-06-30", 50, undefined, "income");
+
+    const topCatSQL = mockSelect.mock.calls[0][0] as string;
+    const topCatParams = mockSelect.mock.calls[0][1] as unknown[];
+    expect(topCatSQL).toContain("COALESCE(c.type, 'expense') = $1");
+    expect(topCatParams[0]).toBe("income");
+    // Date params follow
+    expect(topCatParams[1]).toBe("2025-01-01");
+    expect(topCatParams[2]).toBe("2025-06-30");
+  });
+
+  it("applies date range filters", async () => {
+    mockSelect
+      .mockResolvedValueOnce([]) // topCategories
+      .mockResolvedValueOnce([]); // monthlyRows
+
+    await getCategoryOverTime("2025-01-01", "2025-12-31");
+
+    const topCatSQL = mockSelect.mock.calls[0][0] as string;
+    const topCatParams = mockSelect.mock.calls[0][1] as unknown[];
+    expect(topCatSQL).toContain("t.date >= $1");
+    expect(topCatSQL).toContain("t.date <= $2");
+    expect(topCatParams).toEqual(["2025-01-01", "2025-12-31", 50]);
+  });
+
+  it("applies sourceId filter", async () => {
+    mockSelect
+      .mockResolvedValueOnce([]) // topCategories
+      .mockResolvedValueOnce([]); // monthlyRows
+
+    await getCategoryOverTime(undefined, undefined, 50, 3);
+
+    const topCatSQL = mockSelect.mock.calls[0][0] as string;
+    const topCatParams = mockSelect.mock.calls[0][1] as unknown[];
+    expect(topCatSQL).toContain("t.source_id");
+    expect(topCatParams[0]).toBe(3);
+  });
+
+  it("combines typeFilter, date range, and sourceId", async () => {
+    mockSelect
+      .mockResolvedValueOnce([]) // topCategories
+      .mockResolvedValueOnce([]); // monthlyRows
+
+    await getCategoryOverTime("2025-01-01", "2025-06-30", 10, 2, "expense");
+
+    const topCatSQL = mockSelect.mock.calls[0][0] as string;
+    const topCatParams = mockSelect.mock.calls[0][1] as unknown[];
+    expect(topCatSQL).toContain("COALESCE(c.type, 'expense') = $1");
+    expect(topCatSQL).toContain("t.date >= $2");
+    expect(topCatSQL).toContain("t.date <= $3");
+    expect(topCatSQL).toContain("t.source_id = $4");
+    expect(topCatParams).toEqual(["expense", "2025-01-01", "2025-06-30", 2, 10]);
+  });
+
+  it("returns correct structure with categories, data, colors, and categoryIds", async () => {
+    mockSelect
+      .mockResolvedValueOnce([
+        { category_id: 1, category_name: "Food", category_color: "#ff0000", total: 500 },
+        { category_id: 2, category_name: "Transport", category_color: "#00ff00", total: 200 },
+      ])
+      .mockResolvedValueOnce([
+        { month: "2025-01", category_id: 1, category_name: "Food", total: 300 },
+        { month: "2025-01", category_id: 2, category_name: "Transport", total: 100 },
+        { month: "2025-02", category_id: 1, category_name: "Food", total: 200 },
+        { month: "2025-02", category_id: 2, category_name: "Transport", total: 100 },
+      ]);
+
+    const result = await getCategoryOverTime("2025-01-01", "2025-02-28", 50, undefined, "expense");
+
+    expect(result.categories).toEqual(["Food", "Transport"]);
+    expect(result.colors).toEqual({ Food: "#ff0000", Transport: "#00ff00" });
+    expect(result.categoryIds).toEqual({ Food: 1, Transport: 2 });
+    expect(result.data).toHaveLength(2);
+    expect(result.data[0]).toEqual({ month: "2025-01", Food: 300, Transport: 100 });
+    expect(result.data[1]).toEqual({ month: "2025-02", Food: 200, Transport: 100 });
+  });
+
+  it("groups non-top-N categories into Other", async () => {
+    mockSelect
+      .mockResolvedValueOnce([
+        { category_id: 1, category_name: "Food", category_color: "#ff0000", total: 500 },
+      ])
+      .mockResolvedValueOnce([
+        { month: "2025-01", category_id: 1, category_name: "Food", total: 300 },
+        { month: "2025-01", category_id: 2, category_name: "Transport", total: 100 },
+        { month: "2025-01", category_id: 3, category_name: "Entertainment", total: 50 },
+      ]);
+
+    const result = await getCategoryOverTime("2025-01-01", "2025-01-31", 1, undefined, "expense");
+
+    expect(result.categories).toEqual(["Food", "Other"]);
+    expect(result.colors["Other"]).toBe("#9ca3af");
+    expect(result.data[0]).toEqual({ month: "2025-01", Food: 300, Other: 150 });
+  });
+});