diff --git a/src/app/api/import/route.ts b/src/app/api/import/route.ts index 6048895b..4de31dcb 100644 --- a/src/app/api/import/route.ts +++ b/src/app/api/import/route.ts @@ -183,7 +183,7 @@ export async function POST(request: Request) { originalName: row.originalName, name: row.name, amount: row.amount, - normalizedAmount: normalizeAmount(row.amount, account.type), + normalizedAmount: normalizeAmount(row.amount), externalId: row.externalId, }); } diff --git a/src/lib/money.test.ts b/src/lib/money.test.ts index bb3fb92f..b8509dbe 100644 --- a/src/lib/money.test.ts +++ b/src/lib/money.test.ts @@ -56,39 +56,22 @@ describe("money utilities", () => { }); describe("normalizeAmount", () => { - it("flips sign for checking expense (positive → negative)", () => { - expect(normalizeAmount(1250, "checking")).toBe(-1250); + // Normalization is account-type independent: Plaid uses one sign + // convention across every account type, so a credit-card expense and a + // checking expense normalize identically. That used to be asserted once + // per account type; the function no longer takes a type at all, so the + // cases below cover the three behaviours that actually differ. + it("flips an expense positive → negative", () => { + expect(normalizeAmount(1250)).toBe(-1250); }); - it("flips sign for checking income (negative → positive)", () => { - expect(normalizeAmount(-5000, "checking")).toBe(5000); + it("flips income negative → positive", () => { + expect(normalizeAmount(-5000)).toBe(5000); }); - it("flips sign for credit card expense (positive → negative)", () => { - expect(normalizeAmount(5000, "credit")).toBe(-5000); - }); - it("flips sign for credit card payment (negative → positive)", () => { - expect(normalizeAmount(-20000, "credit")).toBe(20000); - }); - it("flips sign for investment accounts", () => { - expect(normalizeAmount(100000, "investment")).toBe(-100000); - }); - it("flips sign for depository accounts", () => { - expect(normalizeAmount(1250, "depository")).toBe(-1250); - }); - it("returns 0 (not -0) for zero amount on checking", () => { - expect(Object.is(normalizeAmount(0, "checking"), -0)).toBe(false); - expect(normalizeAmount(0, "checking")).toBe(0); - }); - it("returns 0 (not -0) for zero amount on credit", () => { - expect(Object.is(normalizeAmount(0, "credit"), -0)).toBe(false); - }); - it("flips sign for other account types", () => { - expect(normalizeAmount(1250, "other")).toBe(-1250); - }); - it("flips sign for savings", () => { - expect(normalizeAmount(1250, "savings")).toBe(-1250); - }); - it("flips sign for loan", () => { - expect(normalizeAmount(-5000, "loan")).toBe(5000); + it("returns 0, not -0, for a zero amount", () => { + // -0 breaks equality comparisons downstream; see the -0 gotcha in + // CLAUDE.md. + expect(normalizeAmount(0)).toBe(0); + expect(Object.is(normalizeAmount(0), -0)).toBe(false); }); }); @@ -187,21 +170,17 @@ describe("money property-based tests", () => { ); test.prop([fc.integer({ min: -9999999, max: 9999999 })])( - "normalizeAmount flips sign for all account types", + "normalizeAmount flips sign for any amount", (amount) => { - for (const type of ["checking", "savings", "credit", "loan", "investment", "other"]) { - expect(normalizeAmount(amount, type)).toBe(amount === 0 ? 0 : -amount); - } + expect(normalizeAmount(amount)).toBe(amount === 0 ? 0 : -amount); } ); test.prop([fc.integer({ min: -9999999, max: 9999999 })])( - "normalizeAmount sign symmetry for all account types", + "normalizeAmount is sign-symmetric", (amount) => { - for (const type of ["checking", "credit", "loan"]) { - const left = normalizeAmount(amount, type); - const right = -normalizeAmount(-amount, type); - expect(left).toBe(amount === 0 ? 0 : right); - } + const left = normalizeAmount(amount); + const right = -normalizeAmount(-amount); + expect(left).toBe(amount === 0 ? 0 : right); } ); diff --git a/src/lib/money.ts b/src/lib/money.ts index 211638fb..cd176835 100644 --- a/src/lib/money.ts +++ b/src/lib/money.ts @@ -25,9 +25,11 @@ function trimZero(s: string): string { return s.endsWith(".0") ? s.slice(0, -2) : s; } -// Plaid convention: positive = money out, negative = money in (all account types). -// We flip universally so: negative = expense, positive = income. -export function normalizeAmount(amountCents: number, _accountType: string): number { +// Plaid convention: positive = money out, negative = money in, for every +// account type alike. We flip universally so: negative = expense, positive = +// income. Deliberately takes no account type -- normalization does not depend +// on one, and a parameter implying otherwise is misleading. +export function normalizeAmount(amountCents: number): number { return amountCents === 0 ? 0 : -amountCents; } diff --git a/src/lib/plaid/sync.test.ts b/src/lib/plaid/sync.test.ts index 64e88e95..312a692d 100644 --- a/src/lib/plaid/sync.test.ts +++ b/src/lib/plaid/sync.test.ts @@ -20,20 +20,12 @@ function makeTxn(overrides: Partial = {}): PlaidTransaction { }; } -const accountTypeMap = new Map([ - ["acc-checking", "checking"], - ["acc-credit", "credit"], - ["acc-investment", "investment"], -]); - describe("processBatch", () => { it("converts Plaid float amounts to integer cents", () => { const result = processBatch( [makeTxn({ amount: 12.5 })], [], [], - "hh-1", - accountTypeMap, ); expect(result.inserts[0].amount).toBe(1250); @@ -44,8 +36,6 @@ describe("processBatch", () => { [makeTxn({ account_id: "acc-checking", amount: 12.5 })], [], [], - "hh-1", - accountTypeMap, ); // checking is depository-type → sign flips: 1250 → -1250 @@ -57,8 +47,6 @@ describe("processBatch", () => { [makeTxn({ account_id: "acc-credit", amount: 12.5 })], [], [], - "hh-1", - accountTypeMap, ); // credit → sign flipped: 1250 → -1250 (expense) @@ -70,8 +58,6 @@ describe("processBatch", () => { [makeTxn({ merchant_name: "WHOLE FOODS MARKET" })], [], [], - "hh-1", - accountTypeMap, ); expect(result.merchantUpserts).toHaveLength(1); @@ -86,8 +72,6 @@ describe("processBatch", () => { [makeTxn({ merchant_name: null })], [], [], - "hh-1", - accountTypeMap, ); expect(result.merchantUpserts).toHaveLength(0); @@ -104,8 +88,6 @@ describe("processBatch", () => { ], [], [], - "hh-1", - accountTypeMap, ); expect(result.pendingToRemove).toContain("txn-pending-old"); @@ -118,7 +100,7 @@ describe("processBatch", () => { name: "UPDATED NAME", }); - const result = processBatch([], [modified], [], "hh-1", accountTypeMap); + const result = processBatch([], [modified], []); expect(result.inserts).toHaveLength(0); expect(result.upserts).toHaveLength(1); @@ -144,8 +126,6 @@ describe("processBatch", () => { ], [], [], - "hh-1", - accountTypeMap, ); expect(result.merchantUpserts).toHaveLength(1); diff --git a/src/lib/plaid/sync.ts b/src/lib/plaid/sync.ts index f3fda209..568fda81 100644 --- a/src/lib/plaid/sync.ts +++ b/src/lib/plaid/sync.ts @@ -149,8 +149,6 @@ export function processBatch( added: PlaidTransaction[], modified: PlaidTransaction[], removed: PlaidRemovedTransaction[], - householdId: string, - accountTypeMap: Map, ): ProcessedBatch { // Build merchant upserts (deduplicated by normalized name) const merchantMap = new Map(); @@ -177,8 +175,7 @@ export function processBatch( function toRow(txn: PlaidTransaction): TransactionRow { const amountCents = plaidAmountToCents(txn.amount)!; - const accountType = accountTypeMap.get(txn.account_id) ?? "other"; - const normalizedAmt = normalizeAmount(amountCents, accountType); + const normalizedAmt = normalizeAmount(amountCents); return { externalId: txn.transaction_id, plaidAccountId: txn.account_id, @@ -596,9 +593,9 @@ async function doSync( const now = new Date(); try { - // Read bank_connections row + build accountTypeMap. Kept as its own - // short-lived transaction — not held open across the Plaid API round - // trip (with retries) that follows. + // Read the bank_connections row. Kept as its own short-lived + // transaction — not held open across the Plaid API round trip (with + // retries) that follows. const initial = await withHousehold(householdId, async (tx) => { const [row] = await tx .select() @@ -610,32 +607,13 @@ async function doSync( if (!row) return null; - const accountRows = await tx - .select({ - externalAccountId: accounts.externalAccountId, - type: accounts.type, - }) - .from(accounts) - .where( - and( - eq(accounts.householdId, householdId), - eq(accounts.bankConnectionId, itemId), - isNull(accounts.deletedAt), - ), - ); - - const map = new Map(); - for (const r of accountRows) { - if (r.externalAccountId) map.set(r.externalAccountId, r.type); - } - - return { item: row, accountTypeMap: map }; + return { item: row }; }, db); if (!initial) { return { success: false, error: `Plaid item ${itemId} not found` }; } - const { item, accountTypeMap } = initial; + const { item } = initial; // Decrypt access token const accessToken = decrypt(item.credential); @@ -650,8 +628,6 @@ async function doSync( fetchResult.added, fetchResult.modified, fetchResult.removed, - householdId, - accountTypeMap, ); // Apply to DB diff --git a/tests/integration/import.test.ts b/tests/integration/import.test.ts index e0c6adf0..73da7e55 100644 --- a/tests/integration/import.test.ts +++ b/tests/integration/import.test.ts @@ -67,7 +67,7 @@ describe("import pipeline integration", () => { originalName: row.originalName, name: row.name, amount: row.amount, - normalizedAmount: normalizeAmount(row.amount, "checking"), + normalizedAmount: normalizeAmount(row.amount), externalId: row.externalId, }); }