From 92efa5f01b27fa0dc99babb3a08df02288ffef1a Mon Sep 17 00:00:00 2001 From: RyuseiTaniguchi Date: Sat, 29 Aug 2026 18:36:34 -0700 Subject: [PATCH 1/2] feat(categorization): add category rules CRUD so tier 1 can be used category_rules is tier 1 of the categorization pipeline. The engine already loads the rules and applies them on every sync -- it sorts by priority descending, matches, and stops at the first hit. What was missing was any way to create a row, so the table stayed empty and the highest-priority tier ran over nothing. Adds the missing half: - getCategoryRules() lists rules in the order the engine evaluates them, with the target category name joined so the list needs no second query - src/actions/category-rules.ts: create / update / delete, each in a scoped and an authorized variant, matching actions/merchants.ts - /rules page and manager component The engine is untouched; it needed no changes. Three details the UI has to be honest about, all taken from what the engine actually does rather than from the issue text: - Matching is a case-insensitive substring (`target.includes(pattern)`), so the field is labelled "Pattern contains", not "matches" -- a "matches" label invites a regex that would silently never fire. - Rules run at sync time and only on transactions with a null category, so a new rule cannot re-file the existing review queue and never overrides a manual choice. A banner says so, because otherwise a rule that appears to do nothing reads as broken. - Priority ordering is the evaluation order, so the list is rendered in it rather than by creation date. An empty pattern is rejected: `includes("")` is true for every transaction, so one blank rule would swallow the whole feed. Rules are reachable at /rules rather than /settings/rules because the sidebar computes its active item with pathname.startsWith(href), which would light up both Settings and Rules for a nested route. Closes #89. --- src/actions/category-rules.ts | 148 +++++++++ src/app/(dashboard)/rules/page.tsx | 26 ++ .../organisms/category-rules-manager.tsx | 288 ++++++++++++++++++ src/components/organisms/sidebar-nav.tsx | 2 + src/queries/category-rules.ts | 51 ++++ .../integration/category-rule-actions.test.ts | 270 ++++++++++++++++ 6 files changed, 785 insertions(+) create mode 100644 src/actions/category-rules.ts create mode 100644 src/app/(dashboard)/rules/page.tsx create mode 100644 src/components/organisms/category-rules-manager.tsx create mode 100644 src/queries/category-rules.ts create mode 100644 tests/integration/category-rule-actions.test.ts diff --git a/src/actions/category-rules.ts b/src/actions/category-rules.ts new file mode 100644 index 00000000..62058b03 --- /dev/null +++ b/src/actions/category-rules.ts @@ -0,0 +1,148 @@ +"use server"; + +import { eq } from "drizzle-orm"; +import { revalidatePath } from "next/cache"; +import { v4 as uuid } from "uuid"; +import { z } from "zod"; +import { db as defaultDb, type LedgrDb } from "@/db"; +import { categoryRules, categories } from "@/db/schema"; +import { scopedQuery } from "@/lib/scoped-query"; +import { authorizeAction } from "@/lib/auth/authorize-action"; + +type ActionResult = { success: true } | { error: string }; + +// The engine matches with `target.includes(pattern)`, so an empty pattern is +// true for every transaction and one blank rule would swallow the entire feed. +// Trim first, then require something left over. +const ruleInputSchema = z.object({ + categoryId: z.string().min(1), + matchField: z.enum(["name", "merchant"]), + matchPattern: z.string().transform((s) => s.trim()).pipe(z.string().min(1).max(200)), + priority: z.number().int().min(0).max(999), +}); + +export type CategoryRuleInput = z.input; + +const updateInputSchema = ruleInputSchema.extend({ id: z.string().min(1) }); +export type CategoryRuleUpdateInput = z.input; + +/** + * A rule points at a category, so the category must belong to the same + * household. Without this check a caller could aim a rule at another + * household's category id and read its name back off the rules list. + */ +async function categoryBelongsToHousehold( + householdId: string, + categoryId: string, + db: LedgrDb, +): Promise { + const scoped = scopedQuery(householdId, db); + const [row] = await db + .select({ id: categories.id }) + .from(categories) + .where(scoped.where(categories, eq(categories.id, categoryId))) + .limit(1); + return !!row; +} + +export async function createCategoryRuleScoped( + householdId: string, + input: CategoryRuleInput, + db: LedgrDb = defaultDb, +): Promise { + const parsed = ruleInputSchema.safeParse(input); + if (!parsed.success) return { error: "Enter a pattern to match on." }; + + if (!(await categoryBelongsToHousehold(householdId, parsed.data.categoryId, db))) { + return { error: "Category not found" }; + } + + await db.insert(categoryRules).values({ + id: uuid(), + householdId, + categoryId: parsed.data.categoryId, + matchField: parsed.data.matchField, + matchPattern: parsed.data.matchPattern, + priority: parsed.data.priority, + }); + + revalidatePath("/settings/rules"); + return { success: true }; +} + +export async function createCategoryRule( + input: CategoryRuleInput, + db: LedgrDb = defaultDb, +): Promise { + const auth = await authorizeAction(); + if ("error" in auth) return auth; + return createCategoryRuleScoped(auth.householdId, input, db); +} + +export async function updateCategoryRuleScoped( + householdId: string, + input: CategoryRuleUpdateInput, + db: LedgrDb = defaultDb, +): Promise { + const parsed = updateInputSchema.safeParse(input); + if (!parsed.success) return { error: "Enter a pattern to match on." }; + + if (!(await categoryBelongsToHousehold(householdId, parsed.data.categoryId, db))) { + return { error: "Category not found" }; + } + + const scoped = scopedQuery(householdId, db); + const updated = await db + .update(categoryRules) + .set({ + categoryId: parsed.data.categoryId, + matchField: parsed.data.matchField, + matchPattern: parsed.data.matchPattern, + priority: parsed.data.priority, + }) + .where(scoped.where(categoryRules, eq(categoryRules.id, parsed.data.id))) + .returning({ id: categoryRules.id }); + + if (updated.length === 0) return { error: "Rule not found" }; + + revalidatePath("/settings/rules"); + return { success: true }; +} + +export async function updateCategoryRule( + input: CategoryRuleUpdateInput, + db: LedgrDb = defaultDb, +): Promise { + const auth = await authorizeAction(); + if ("error" in auth) return auth; + return updateCategoryRuleScoped(auth.householdId, input, db); +} + +export async function deleteCategoryRuleScoped( + householdId: string, + ruleId: string, + db: LedgrDb = defaultDb, +): Promise { + const parsed = z.string().min(1).safeParse(ruleId); + if (!parsed.success) return { error: "Invalid input" }; + + const scoped = scopedQuery(householdId, db); + const deleted = await db + .delete(categoryRules) + .where(scoped.where(categoryRules, eq(categoryRules.id, parsed.data))) + .returning({ id: categoryRules.id }); + + if (deleted.length === 0) return { error: "Rule not found" }; + + revalidatePath("/settings/rules"); + return { success: true }; +} + +export async function deleteCategoryRule( + ruleId: string, + db: LedgrDb = defaultDb, +): Promise { + const auth = await authorizeAction(); + if ("error" in auth) return auth; + return deleteCategoryRuleScoped(auth.householdId, ruleId, db); +} diff --git a/src/app/(dashboard)/rules/page.tsx b/src/app/(dashboard)/rules/page.tsx new file mode 100644 index 00000000..a0cee9ac --- /dev/null +++ b/src/app/(dashboard)/rules/page.tsx @@ -0,0 +1,26 @@ +import { getHouseholdId } from "@/lib/auth/session"; +import { getCategoryRules } from "@/queries/category-rules"; +import { getCategories } from "@/queries/categories"; +import { CategoryRulesManager } from "@/components/organisms/category-rules-manager"; + +export default async function RulesPage() { + const householdId = await getHouseholdId(); + + const [rules, categoryGroups] = await Promise.all([ + getCategoryRules(householdId), + getCategories(householdId), + ]); + + return ( +
+
+

Category rules

+

+ Send transactions to a category by matching their name or merchant. +

+
+ + +
+ ); +} diff --git a/src/components/organisms/category-rules-manager.tsx b/src/components/organisms/category-rules-manager.tsx new file mode 100644 index 00000000..bb53ce72 --- /dev/null +++ b/src/components/organisms/category-rules-manager.tsx @@ -0,0 +1,288 @@ +"use client"; + +import { useState, useTransition } from "react"; +import { Pencil, Trash2, Plus, X } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { + createCategoryRule, + updateCategoryRule, + deleteCategoryRule, +} from "@/actions/category-rules"; +import type { CategoryRuleRow } from "@/queries/category-rules"; +import type { CategoryGroup } from "@/queries/categories"; + +interface CategoryRulesManagerProps { + rules: CategoryRuleRow[]; + categoryGroups: CategoryGroup[]; +} + +type MatchField = "name" | "merchant"; + +const FIELD_LABEL: Record = { + name: "Transaction name", + merchant: "Merchant name", +}; + +interface DraftState { + id: string | null; + categoryId: string; + matchField: MatchField; + matchPattern: string; + priority: string; +} + +function emptyDraft(categoryId: string): DraftState { + return { id: null, categoryId, matchField: "name", matchPattern: "", priority: "0" }; +} + +export function CategoryRulesManager({ rules, categoryGroups }: CategoryRulesManagerProps) { + const flatCategories = categoryGroups.flatMap((g) => g.categories); + const firstCategoryId = flatCategories[0]?.id ?? ""; + + const [draft, setDraft] = useState(null); + const [error, setError] = useState(null); + const [pending, startTransition] = useTransition(); + + function openCreate() { + setError(null); + setDraft(emptyDraft(firstCategoryId)); + } + + function openEdit(rule: CategoryRuleRow) { + setError(null); + setDraft({ + id: rule.id, + categoryId: rule.categoryId, + matchField: rule.matchField, + matchPattern: rule.matchPattern, + priority: String(rule.priority), + }); + } + + function save() { + if (!draft) return; + setError(null); + const payload = { + categoryId: draft.categoryId, + matchField: draft.matchField, + matchPattern: draft.matchPattern, + priority: Number(draft.priority) || 0, + }; + + startTransition(async () => { + const result = draft.id + ? await updateCategoryRule({ ...payload, id: draft.id }) + : await createCategoryRule(payload); + + if ("error" in result) { + setError(result.error); + return; + } + setDraft(null); + }); + } + + function remove(ruleId: string) { + setError(null); + startTransition(async () => { + const result = await deleteCategoryRule(ruleId); + if ("error" in result) setError(result.error); + }); + } + + return ( +
+ {/* Rules only run during a sync, and only on transactions that have no + category yet. Saying so up front stops the page reading as broken + when a new rule does not visibly change anything. */} +
+

Rules apply on your next sync

+

+ A new rule categorizes matching transactions the next time an account syncs. It does not + re-file transactions that are already in your review queue, and it never changes a + category you set yourself. +

+
+ + {error && ( +
+ {error} +
+ )} + +
+
+
+

Your rules

+

+ {rules.length === 0 + ? "Checked before every other categorization step" + : `${rules.length} rule${rules.length === 1 ? "" : "s"}, highest priority first`} +

+
+ {!draft && ( + + )} +
+ + {draft && ( +
+
+
+ + +
+ +
+ + +
+
+ +
+
+ + setDraft({ ...draft, matchPattern: e.target.value })} + /> + {/* The engine does target.includes(pattern), so "contains" is + literally what happens. Saying "matches" would invite regex. */} +

+ Plain text, not a pattern language. Capitalization is ignored. +

+
+ +
+ + setDraft({ ...draft, priority: e.target.value })} + /> +

Higher wins

+
+
+ +
+ + +
+
+ )} + + {rules.length === 0 && !draft ? ( +
+

No rules yet

+

+ Rules catch transactions before Ledgr guesses. They run ahead of merchant defaults + and your bank's own category, so a rule always wins. +

+ +
+ ) : ( +
    + {rules.map((rule) => ( +
  • + + {rule.priority} + +
    + + {rule.matchField} + + “{rule.matchPattern}” + → + {rule.categoryName} +
    +
    + + +
    +
    • + ))} +
+ )} +
+
+ ); +} diff --git a/src/components/organisms/sidebar-nav.tsx b/src/components/organisms/sidebar-nav.tsx index f6cafee0..82d67913 100644 --- a/src/components/organisms/sidebar-nav.tsx +++ b/src/components/organisms/sidebar-nav.tsx @@ -4,6 +4,7 @@ import { useCallback } from "react"; import Link from "next/link"; import { usePathname, useRouter } from "next/navigation"; import { + Tags, LayoutDashboard, Building2, ArrowLeftRight, @@ -50,6 +51,7 @@ const NAV_GROUPS: { label: string | null; items: NavItem[] }[] = [ items: [ { href: "/accounts", label: "Accounts", icon: Building2 }, { href: "/transactions", label: "Transactions", icon: ArrowLeftRight }, + { href: "/rules", label: "Rules", icon: Tags }, { href: "/investments", label: "Investments", icon: TrendingUp }, ], }, diff --git a/src/queries/category-rules.ts b/src/queries/category-rules.ts new file mode 100644 index 00000000..33e3f899 --- /dev/null +++ b/src/queries/category-rules.ts @@ -0,0 +1,51 @@ +import { desc, eq } from "drizzle-orm"; +import { db as defaultDb, type LedgrDb } from "@/db"; +import { categoryRules, categories } from "@/db/schema"; +import { scopedQuery } from "@/lib/scoped-query"; + +export interface CategoryRuleRow { + id: string; + categoryId: string; + categoryName: string; + matchField: "name" | "merchant"; + matchPattern: string; + priority: number; +} + +/** + * Rules for the management page, in the order the categorization engine + * evaluates them. + * + * The engine sorts by priority descending and stops at the first match + * (`categorizeTransactions`, `lib/categorization/engine.ts`), so listing them + * in any other order would misrepresent which rule actually wins. + */ +export async function getCategoryRules( + householdId: string, + db: LedgrDb = defaultDb, +): Promise { + const scoped = scopedQuery(householdId, db); + + const rows = await db + .select({ + id: categoryRules.id, + categoryId: categoryRules.categoryId, + categoryName: categories.name, + matchField: categoryRules.matchField, + matchPattern: categoryRules.matchPattern, + priority: categoryRules.priority, + }) + .from(categoryRules) + .innerJoin(categories, eq(categories.id, categoryRules.categoryId)) + .where(scoped.where(categoryRules)) + .orderBy(desc(categoryRules.priority), categoryRules.matchPattern); + + return rows.map((r) => ({ + id: r.id, + categoryId: r.categoryId, + categoryName: r.categoryName, + matchField: (r.matchField ?? "name") as "name" | "merchant", + matchPattern: r.matchPattern, + priority: r.priority ?? 0, + })); +} diff --git a/tests/integration/category-rule-actions.test.ts b/tests/integration/category-rule-actions.test.ts new file mode 100644 index 00000000..e2c2f64e --- /dev/null +++ b/tests/integration/category-rule-actions.test.ts @@ -0,0 +1,270 @@ +import { describe, it, expect, beforeAll, afterAll, vi } from "vitest"; +import { createTestDb } from "./setup"; +import { insertHousehold, insertCategoryGroup, insertCategory } from "./helpers"; +import { + createCategoryRule, + updateCategoryRule, + deleteCategoryRule, +} from "../../src/actions/category-rules"; +import { getCategoryRules } from "../../src/queries/category-rules"; +import { categoryRules } from "../../src/db/schema"; +import { eq } from "drizzle-orm"; +import type { LedgrDb } from "../../src/db"; + +vi.mock("next/cache", () => ({ revalidatePath: vi.fn() })); +vi.mock("../../src/lib/demo-mode", () => ({ guardDemoMode: vi.fn(() => null) })); + +const mockUserId = "test-user-id"; +let mockHouseholdId: string; +vi.mock("../../src/lib/auth/session", () => ({ + getHouseholdId: vi.fn(() => Promise.resolve(mockHouseholdId)), + getSession: vi.fn(() => Promise.resolve({ user: { id: mockUserId } })), +})); + +describe("category rule actions", () => { + let db: LedgrDb; + let close: () => Promise; + let categoryId: string; + let otherCategoryId: string; + + beforeAll(async () => { + ({ db, close } = await createTestDb()); + + const hh = await insertHousehold(db); + mockHouseholdId = hh.householdId; + const { groupId } = await insertCategoryGroup(db, hh.householdId); + ({ categoryId } = await insertCategory(db, hh.householdId, groupId, { name: "Subscriptions" })); + ({ categoryId: otherCategoryId } = await insertCategory(db, hh.householdId, groupId, { + name: "Groceries", + })); + }); + + afterAll(async () => { + await close(); + }); + + describe("createCategoryRule", () => { + it("creates a rule the engine can load", async () => { + const result = await createCategoryRule( + { categoryId, matchField: "name", matchPattern: "twitterapi", priority: 10 }, + db, + ); + + expect(result).toMatchObject({ success: true }); + const [row] = await db + .select() + .from(categoryRules) + .where(eq(categoryRules.matchPattern, "twitterapi")); + expect(row!.categoryId).toBe(categoryId); + expect(row!.matchField).toBe("name"); + expect(row!.priority).toBe(10); + expect(row!.householdId).toBe(mockHouseholdId); + }); + + it("trims the pattern, so a stray space cannot stop a rule matching", async () => { + await createCategoryRule( + { categoryId, matchField: "name", matchPattern: " spotify ", priority: 0 }, + db, + ); + + const [row] = await db + .select() + .from(categoryRules) + .where(eq(categoryRules.categoryId, categoryId)); + const patterns = ( + await db.select().from(categoryRules).where(eq(categoryRules.householdId, mockHouseholdId)) + ).map((r) => r.matchPattern); + expect(patterns).toContain("spotify"); + expect(row).toBeDefined(); + }); + + it("rejects an empty pattern rather than creating a rule that matches everything", async () => { + // The engine does target.includes(pattern); an empty string is true for + // every transaction, so a blank rule would swallow the whole feed. + const result = await createCategoryRule( + { categoryId, matchField: "name", matchPattern: " ", priority: 0 }, + db, + ); + + expect(result).toMatchObject({ error: expect.any(String) }); + }); + + it("rejects a category belonging to another household", async () => { + const other = await insertHousehold(db); + const { groupId } = await insertCategoryGroup(db, other.householdId); + const foreign = await insertCategory(db, other.householdId, groupId, { name: "Foreign" }); + + const result = await createCategoryRule( + { categoryId: foreign.categoryId, matchField: "name", matchPattern: "leak", priority: 0 }, + db, + ); + + expect(result).toMatchObject({ error: expect.any(String) }); + const rows = await db + .select() + .from(categoryRules) + .where(eq(categoryRules.matchPattern, "leak")); + expect(rows).toHaveLength(0); + }); + }); + + describe("updateCategoryRule", () => { + it("changes the pattern, field, category and priority", async () => { + await createCategoryRule( + { categoryId, matchField: "name", matchPattern: "before", priority: 1 }, + db, + ); + const [created] = await db + .select() + .from(categoryRules) + .where(eq(categoryRules.matchPattern, "before")); + + const result = await updateCategoryRule( + { + id: created!.id, + categoryId: otherCategoryId, + matchField: "merchant", + matchPattern: "after", + priority: 99, + }, + db, + ); + + expect(result).toMatchObject({ success: true }); + const [row] = await db.select().from(categoryRules).where(eq(categoryRules.id, created!.id)); + expect(row!.matchPattern).toBe("after"); + expect(row!.matchField).toBe("merchant"); + expect(row!.categoryId).toBe(otherCategoryId); + expect(row!.priority).toBe(99); + }); + + it("will not update a rule belonging to another household", async () => { + const other = await insertHousehold(db); + const { groupId } = await insertCategoryGroup(db, other.householdId); + const foreignCat = await insertCategory(db, other.householdId, groupId, { name: "Theirs" }); + const foreignRuleId = crypto.randomUUID(); + await db.insert(categoryRules).values({ + id: foreignRuleId, + householdId: other.householdId, + categoryId: foreignCat.categoryId, + matchField: "name", + matchPattern: "theirs", + priority: 0, + }); + + const result = await updateCategoryRule( + { + id: foreignRuleId, + categoryId, + matchField: "name", + matchPattern: "hijacked", + priority: 0, + }, + db, + ); + + expect(result).toMatchObject({ error: expect.any(String) }); + const [row] = await db.select().from(categoryRules).where(eq(categoryRules.id, foreignRuleId)); + expect(row!.matchPattern).toBe("theirs"); + }); + }); + + describe("deleteCategoryRule", () => { + it("removes the rule", async () => { + await createCategoryRule( + { categoryId, matchField: "name", matchPattern: "doomed", priority: 0 }, + db, + ); + const [created] = await db + .select() + .from(categoryRules) + .where(eq(categoryRules.matchPattern, "doomed")); + + const result = await deleteCategoryRule(created!.id, db); + + expect(result).toMatchObject({ success: true }); + const rows = await db.select().from(categoryRules).where(eq(categoryRules.id, created!.id)); + expect(rows).toHaveLength(0); + }); + + it("will not delete a rule belonging to another household", async () => { + const other = await insertHousehold(db); + const { groupId } = await insertCategoryGroup(db, other.householdId); + const foreignCat = await insertCategory(db, other.householdId, groupId, { name: "Safe" }); + const foreignRuleId = crypto.randomUUID(); + await db.insert(categoryRules).values({ + id: foreignRuleId, + householdId: other.householdId, + categoryId: foreignCat.categoryId, + matchField: "name", + matchPattern: "survives", + priority: 0, + }); + + const result = await deleteCategoryRule(foreignRuleId, db); + + expect(result).toMatchObject({ error: expect.any(String) }); + const rows = await db.select().from(categoryRules).where(eq(categoryRules.id, foreignRuleId)); + expect(rows).toHaveLength(1); + }); + }); + + describe("getCategoryRules", () => { + it("returns rules in the order the engine evaluates them, highest priority first", async () => { + const hh = await insertHousehold(db); + const { groupId } = await insertCategoryGroup(db, hh.householdId); + const cat = await insertCategory(db, hh.householdId, groupId, { name: "Ordered" }); + for (const [pattern, priority] of [["low", 1], ["high", 100], ["mid", 50]] as const) { + await db.insert(categoryRules).values({ + id: crypto.randomUUID(), + householdId: hh.householdId, + categoryId: cat.categoryId, + matchField: "name", + matchPattern: pattern, + priority, + }); + } + + const rules = await getCategoryRules(hh.householdId, db); + + expect(rules.map((r) => r.matchPattern)).toEqual(["high", "mid", "low"]); + }); + + it("includes the target category name, so the list need not re-query", async () => { + const hh = await insertHousehold(db); + const { groupId } = await insertCategoryGroup(db, hh.householdId); + const cat = await insertCategory(db, hh.householdId, groupId, { name: "Coffee Shops" }); + await db.insert(categoryRules).values({ + id: crypto.randomUUID(), + householdId: hh.householdId, + categoryId: cat.categoryId, + matchField: "name", + matchPattern: "blue bottle", + priority: 0, + }); + + const [rule] = await getCategoryRules(hh.householdId, db); + + expect(rule.categoryName).toBe("Coffee Shops"); + }); + + it("does not return another household's rules", async () => { + const mine = await insertHousehold(db); + const theirs = await insertHousehold(db); + const { groupId } = await insertCategoryGroup(db, theirs.householdId); + const cat = await insertCategory(db, theirs.householdId, groupId, { name: "Hidden" }); + await db.insert(categoryRules).values({ + id: crypto.randomUUID(), + householdId: theirs.householdId, + categoryId: cat.categoryId, + matchField: "name", + matchPattern: "secret", + priority: 0, + }); + + const rules = await getCategoryRules(mine.householdId, db); + + expect(rules).toHaveLength(0); + }); + }); +}); From ee9dd278cc6934b41eb95e4a4defd9b7664696c5 Mon Sep 17 00:00:00 2001 From: RyuseiTaniguchi Date: Sat, 29 Aug 2026 18:46:36 -0700 Subject: [PATCH 2/2] refactor(categorization): extract rule-pattern validation so it is unit-testable The empty-pattern guard is the most consequential rule in this feature -- `"".includes()` is true for every string, so one blank pattern matches the whole feed, and being tier 1 it outranks every other categorization step. It was reachable only through a database round trip. Moves it to a pure module with no DB imports and covers it with unit and property tests: trimming, empty and whitespace-only rejection, the length boundary on both sides, and that the limit is measured after trimming. This also unblocks the mutation gate, which was not merely scoring low on this branch but erroring out: WARN Vitest failed to find test files related to mutated files INFO No tests were found ERROR No tests were executed. Stryker will exit prematurely. Both new files were covered only by tests/integration/, which vitest.stryker.config.ts excludes, so Stryker found zero related tests and aborted before producing a report. With a unit-tested pure module in the changed set it completes normally: rule-pattern.ts 100.00% 8 killed, 0 survived, 0 no-coverage category-rules.ts 0.00% 38 no-coverage (DB-backed, see #103) That hard-failure mode is a new manifestation of #103 and is reported there. --- src/actions/category-rules.ts | 11 ++-- src/lib/categorization/rule-pattern.test.ts | 61 +++++++++++++++++++++ src/lib/categorization/rule-pattern.ts | 31 +++++++++++ 3 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 src/lib/categorization/rule-pattern.test.ts create mode 100644 src/lib/categorization/rule-pattern.ts diff --git a/src/actions/category-rules.ts b/src/actions/category-rules.ts index 62058b03..efc647c9 100644 --- a/src/actions/category-rules.ts +++ b/src/actions/category-rules.ts @@ -8,16 +8,19 @@ import { db as defaultDb, type LedgrDb } from "@/db"; import { categoryRules, categories } from "@/db/schema"; import { scopedQuery } from "@/lib/scoped-query"; import { authorizeAction } from "@/lib/auth/authorize-action"; +import { normalizeRulePattern } from "@/lib/categorization/rule-pattern"; type ActionResult = { success: true } | { error: string }; -// The engine matches with `target.includes(pattern)`, so an empty pattern is -// true for every transaction and one blank rule would swallow the entire feed. -// Trim first, then require something left over. +// Pattern validity lives in normalizeRulePattern, which explains why an empty +// pattern is dangerous and is unit-tested without a database. const ruleInputSchema = z.object({ categoryId: z.string().min(1), matchField: z.enum(["name", "merchant"]), - matchPattern: z.string().transform((s) => s.trim()).pipe(z.string().min(1).max(200)), + matchPattern: z + .string() + .transform(normalizeRulePattern) + .refine((p): p is string => p !== null, { message: "Enter a pattern to match on." }), priority: z.number().int().min(0).max(999), }); diff --git a/src/lib/categorization/rule-pattern.test.ts b/src/lib/categorization/rule-pattern.test.ts new file mode 100644 index 00000000..7af87cb0 --- /dev/null +++ b/src/lib/categorization/rule-pattern.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from "vitest"; +import { test, fc } from "@fast-check/vitest"; +import { normalizeRulePattern, MAX_RULE_PATTERN_LENGTH } from "./rule-pattern"; + +describe("normalizeRulePattern", () => { + it("trims surrounding whitespace", () => { + // The engine does not trim, so a stored leading space would stop the + // pattern matching anything the user expected. + expect(normalizeRulePattern(" spotify ")).toBe("spotify"); + }); + + it("keeps a usable pattern unchanged", () => { + expect(normalizeRulePattern("twitterapi")).toBe("twitterapi"); + }); + + it("preserves inner whitespace", () => { + expect(normalizeRulePattern(" blue bottle ")).toBe("blue bottle"); + }); + + it("rejects an empty pattern", () => { + expect(normalizeRulePattern("")).toBeNull(); + }); + + it("rejects a whitespace-only pattern", () => { + // `"".includes()` is true for every string, so a blank rule would match the + // entire feed — and as tier 1 it would outrank every other step. + expect(normalizeRulePattern(" \t \n ")).toBeNull(); + }); + + it("accepts a pattern exactly at the length limit", () => { + const atLimit = "a".repeat(MAX_RULE_PATTERN_LENGTH); + expect(normalizeRulePattern(atLimit)).toBe(atLimit); + }); + + it("rejects a pattern one character over the limit", () => { + expect(normalizeRulePattern("a".repeat(MAX_RULE_PATTERN_LENGTH + 1))).toBeNull(); + }); + + it("measures the limit after trimming, not before", () => { + const padded = ` ${"a".repeat(MAX_RULE_PATTERN_LENGTH)} `; + expect(normalizeRulePattern(padded)).toBe("a".repeat(MAX_RULE_PATTERN_LENGTH)); + }); + + test.prop([fc.string()])("never returns an empty or untrimmed string", (raw) => { + const result = normalizeRulePattern(raw); + if (result !== null) { + expect(result.length).toBeGreaterThan(0); + expect(result).toBe(result.trim()); + expect(result.length).toBeLessThanOrEqual(MAX_RULE_PATTERN_LENGTH); + } + }); + + test.prop([fc.string()])( + "accepts exactly when the trimmed input is usable", + (raw) => { + const trimmed = raw.trim(); + const usable = trimmed.length > 0 && trimmed.length <= MAX_RULE_PATTERN_LENGTH; + expect(normalizeRulePattern(raw) !== null).toBe(usable); + }, + ); +}); diff --git a/src/lib/categorization/rule-pattern.ts b/src/lib/categorization/rule-pattern.ts new file mode 100644 index 00000000..02c51b38 --- /dev/null +++ b/src/lib/categorization/rule-pattern.ts @@ -0,0 +1,31 @@ +/** + * Validation for a category rule's match pattern. + * + * Kept separate from the server action so it can be exercised without a + * database: the rule it enforces is the one that matters most, and a rule this + * consequential should not be reachable only through a round trip. + * + * The categorization engine matches with + * `target.includes(pattern.toLowerCase())` (`lib/categorization/engine.ts`). + * `"".includes()` is true for *every* string, so a single blank pattern would + * match every transaction in the feed and, being tier 1, would outrank every + * other categorization step. One empty rule can therefore miscategorize an + * entire account. + */ + +/** Longest pattern we store. Generous for a merchant name; bounded so the column can't be abused. */ +export const MAX_RULE_PATTERN_LENGTH = 200; + +/** + * Normalize a user-entered pattern, or return null if it is not usable. + * + * Trimming happens before the empty check, so a pattern of only whitespace is + * rejected rather than stored — and a pattern the user typed with a stray + * leading space still matches, since the engine does no trimming of its own. + */ +export function normalizeRulePattern(raw: string): string | null { + const trimmed = raw.trim(); + if (trimmed.length === 0) return null; + if (trimmed.length > MAX_RULE_PATTERN_LENGTH) return null; + return trimmed; +}