From 9f4b53d37ac23b6c006c90c83aff403b4b963ff1 Mon Sep 17 00:00:00 2001 From: null Date: Fri, 3 Jul 2026 18:25:55 -0500 Subject: [PATCH] perf(tracker): batch getTracker per-bill queries (kill N+1) (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getTracker ran a payments query per bill plus computeAmountSuggestion per bill (up to 12 queries each: 6 months x 2) inside bills.map — ~70-450 queries for a 35-bill account per load. Now one query fetches all cycle payments (grouped in JS per bill range) and two compute all amount suggestions (computeAmountSuggestionsBatch). Behavior-preserving. Test: tests/amountSuggestionService.test.js pins batch == per-bill output. Co-Authored-By: Claude Opus 4.8 --- HISTORY.md | 4 + services/amountSuggestionService.js | 120 +++++++++++++++++++++----- services/trackerService.js | 50 ++++++++++- tests/amountSuggestionService.test.js | 74 ++++++++++++++++ 4 files changed, 225 insertions(+), 23 deletions(-) create mode 100644 tests/amountSuggestionService.test.js diff --git a/HISTORY.md b/HISTORY.md index 2ac6395..4492a2d 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,10 @@ # Bill Tracker — Changelog ## v0.41.0 +### ⚡ Tracker performance + +- **[Tracker] Killed the getTracker N+1 (was ~2–3 DB round-trips × N bills every home-page load)** — inside `bills.map`, `getTracker` ran a payments query per bill (`fetchPaymentsForBillCycle`) plus `computeAmountSuggestion` per bill, and the suggestion alone fired up to 12 queries per bill (6 months × 2) — roughly 70–450 queries for a 35-bill account on every Tracker load. Now one query fetches all bills' cycle payments (grouped in JS by each bill's own range) and two queries compute all amount suggestions (`computeAmountSuggestionsBatch`), replacing the per-bill loops. Behavior-preserving — `tests/amountSuggestionService.test.js` pins the batched suggestion to be byte-identical to the per-bill function, and the `trackerService` tests still pass unchanged. (Tracker P1) + ### 🐛 Tracker & bill-modal hardening - **[Tracker] Route error handling + autopay write atomicity** — `routes/tracker.js` had no try/catch and returned a plain `{error}` shape unlike the rest of the API; its three GET handlers now wrap in try/catch and return `standardizeError` (`{error, message, field, code}`), including the invalid-month validation path. Separately, `applyAutopaySuggestions` (which runs on a `GET /tracker`) inserted the auto-mark-paid payment and applied the balance delta as two un-transactional steps — a mid-way failure could leave a payment without its balance adjustment. Wrapped the INSERT + `applyBalanceDelta` in a single `db.transaction`. Covered by new cases in `tests/trackerService.test.js` (auto-mark creates one payment + drops the balance, idempotent on re-load; route returns the standardized error). (Tracker T2) diff --git a/services/amountSuggestionService.js b/services/amountSuggestionService.js index 0de2ea0..e31b927 100644 --- a/services/amountSuggestionService.js +++ b/services/amountSuggestionService.js @@ -3,6 +3,35 @@ const { accountingActiveSql } = require('./paymentAccountingService'); const { fromCents } = require('../utils/money'); +// The 6 calendar months immediately preceding (year, month), newest first, each +// as { y, m, key: 'YYYY-MM' }. +function priorMonths(year, month, count = 6) { + const list = []; + let y = year; + let m = month; + for (let i = 0; i < count; i++) { + m -= 1; + if (m === 0) { m = 12; y -= 1; } + list.push({ y, m, key: `${y}-${String(m).padStart(2, '0')}` }); + } + return list; +} + +// Rolling median of a bill's monthly amounts → a suggestion object (or null). +function medianSuggestion(amounts) { + if (amounts.length === 0) return null; + const sorted = [...amounts].sort((a, b) => a - b); + const mid = Math.floor(sorted.length / 2); + const median = sorted.length % 2 === 0 + ? (sorted[mid - 1] + sorted[mid]) / 2 + : sorted[mid]; + return { + suggestion: fromCents(Math.round(median)), + months_used: amounts.length, + confidence: amounts.length >= 3 ? 'high' : 'low', + }; +} + /** * Computes a suggested expected amount for a bill based on the rolling median * of the last 6 months of actual data. Prefers monthly_bill_state.actual_amount @@ -10,13 +39,8 @@ const { fromCents } = require('../utils/money'); */ function computeAmountSuggestion(db, billId, year, month) { const amounts = []; - let y = year; - let m = month; - - for (let i = 0; i < 6; i++) { - m -= 1; - if (m === 0) { m = 12; y -= 1; } + for (const { y, m } of priorMonths(year, month, 6)) { const mbs = db.prepare( 'SELECT actual_amount FROM monthly_bill_state WHERE bill_id = ? AND year = ? AND month = ?' ).get(billId, y, m); @@ -39,19 +63,75 @@ function computeAmountSuggestion(db, billId, year, month) { if (result.total > 0) amounts.push(result.total); } - if (amounts.length === 0) return null; - - const sorted = [...amounts].sort((a, b) => a - b); - const mid = Math.floor(sorted.length / 2); - const median = sorted.length % 2 === 0 - ? (sorted[mid - 1] + sorted[mid]) / 2 - : sorted[mid]; - - return { - suggestion: fromCents(Math.round(median)), - months_used: amounts.length, - confidence: amounts.length >= 3 ? 'high' : 'low', - }; + return medianSuggestion(amounts); } -module.exports = { computeAmountSuggestion }; +/** + * Batched form of computeAmountSuggestion for many bills at once: two queries + * total (monthly_bill_state + grouped payment sums) instead of up to 12 per + * bill. Returns a Map. Behavior-identical to calling + * computeAmountSuggestion per bill (guarded by amountSuggestionService.test.js). + */ +function computeAmountSuggestionsBatch(db, billIds, year, month) { + const result = new Map(); + if (!billIds || billIds.length === 0) return result; + + const months = priorMonths(year, month, 6); + const monthKeys = new Set(months.map(mm => mm.key)); + const minKey = months[months.length - 1].key; // earliest (month − 6) + const maxKey = months[0].key; // latest (month − 1) + // Broad window that fully spans the 6 months; the monthKeys filter below is + // the exact gate, so a '-01'…'-31' string window is safe for ISO dates. + const windowStart = `${minKey}-01`; + const windowEnd = `${maxKey}-31`; + + const placeholders = billIds.map(() => '?').join(','); + + // User-corrected monthly amounts. + const mbsMap = new Map(); // billId → { 'YYYY-MM': actual_amount } + const mbsRows = db.prepare(` + SELECT bill_id, year, month, actual_amount + FROM monthly_bill_state + WHERE bill_id IN (${placeholders}) + `).all(...billIds); + for (const r of mbsRows) { + if (r.actual_amount == null) continue; + const key = `${r.year}-${String(r.month).padStart(2, '0')}`; + if (!monthKeys.has(key)) continue; + if (!mbsMap.has(r.bill_id)) mbsMap.set(r.bill_id, {}); + mbsMap.get(r.bill_id)[key] = r.actual_amount; + } + + // Payment sums grouped by bill + month. + const payMap = new Map(); // billId → { 'YYYY-MM': total } + const payRows = db.prepare(` + SELECT bill_id, strftime('%Y-%m', paid_date) AS ym, COALESCE(SUM(amount), 0) AS total + FROM payments + WHERE bill_id IN (${placeholders}) + AND deleted_at IS NULL + AND ${accountingActiveSql()} + AND paid_date BETWEEN ? AND ? + GROUP BY bill_id, ym + `).all(...billIds, windowStart, windowEnd); + for (const r of payRows) { + if (!monthKeys.has(r.ym)) continue; + if (!payMap.has(r.bill_id)) payMap.set(r.bill_id, {}); + payMap.get(r.bill_id)[r.ym] = r.total; + } + + for (const billId of billIds) { + const amounts = []; + const mbsFor = mbsMap.get(billId) || {}; + const payFor = payMap.get(billId) || {}; + for (const { key } of months) { + if (mbsFor[key] != null) { amounts.push(mbsFor[key]); continue; } + const total = payFor[key] || 0; + if (total > 0) amounts.push(total); + } + result.set(billId, medianSuggestion(amounts)); + } + + return result; +} + +module.exports = { computeAmountSuggestion, computeAmountSuggestionsBatch }; diff --git a/services/trackerService.js b/services/trackerService.js index bedab3b..b833902 100644 --- a/services/trackerService.js +++ b/services/trackerService.js @@ -4,7 +4,7 @@ const { getDb } = require('../db/database'); const { buildTrackerRow, getCycleRange, resolveDueDate } = require('./statusService'); const { getUserSettings } = require('./userSettings'); const { computeBalanceDelta, applyBalanceDelta } = require('./billsService'); -const { computeAmountSuggestion } = require('./amountSuggestionService'); +const { computeAmountSuggestionsBatch } = require('./amountSuggestionService'); const { accountingActiveSql } = require('./paymentAccountingService'); const { normalizeMerchant } = require('./subscriptionService'); const { localDateString } = require('../utils/dates'); @@ -223,6 +223,46 @@ function fetchPaymentsForBillCycle(db, bill, year, month) { `).all(bill.id, range.start, range.end); } +// Batched form of fetchPaymentsForBillCycle: one query for every bill's cycle +// payments (grouped in JS by each bill's own range) instead of one query per +// bill. Returns Map with each list still ordered paid_date +// DESC — identical to calling fetchPaymentsForBillCycle per bill. +function fetchPaymentsForBillsInMonth(db, bills, year, month) { + const ranges = new Map(); // billId → { start, end } (only bills due this month) + let minStart = null; + let maxEnd = null; + for (const bill of bills) { + const range = getCycleRange(year, month, bill); + if (!range) continue; + ranges.set(bill.id, range); + if (minStart === null || range.start < minStart) minStart = range.start; + if (maxEnd === null || range.end > maxEnd) maxEnd = range.end; + } + + const byBill = new Map(); + if (minStart === null) return byBill; + + const ids = [...ranges.keys()]; + const placeholders = ids.map(() => '?').join(','); + const rows = db.prepare(` + SELECT bill_id, id, amount, paid_date, method, notes, payment_source, transaction_id, created_at, updated_at + FROM payments + WHERE bill_id IN (${placeholders}) AND paid_date BETWEEN ? AND ? + AND deleted_at IS NULL + AND ${accountingActiveSql()} + ORDER BY paid_date DESC + `).all(...ids, minStart, maxEnd); + + for (const row of rows) { + const range = ranges.get(row.bill_id); + // The union window may be wider than this bill's own cycle — filter to it. + if (row.paid_date < range.start || row.paid_date > range.end) continue; + if (!byBill.has(row.bill_id)) byBill.set(row.bill_id, []); + byBill.get(row.bill_id).push(row); + } + return byBill; +} + function fetchPreviousMonthPaid(db, billIds, range) { if (billIds.length === 0) return {}; const placeholders = billIds.map(() => '?').join(','); @@ -519,13 +559,17 @@ function getTracker(userId, query = {}, now = new Date()) { const dismissedSuggestions = fetchDismissedSuggestions(db, userId, billIds, year, month); const sparklines = fetchSparklines(db, billIds); const autopayStatsMap = fetchAutopayStats(db, billIds); + // Batched to avoid an N+1 across bills (was ~2–3 queries × N bills every load): + // one query for all cycle payments, two for all amount suggestions. + const paymentsByBill = fetchPaymentsForBillsInMonth(db, bills, year, month); + const amountSuggestions = computeAmountSuggestionsBatch(db, billIds, year, month); const rows = bills.map(bill => { bill.sparkline = sparklines[bill.id] ?? null; bill.autopay_stats = autopayStatsMap[bill.id] ?? null; if (!resolveDueDate(bill, year, month)) return null; - const payments = fetchPaymentsForBillCycle(db, bill, year, month); + const payments = paymentsByBill.get(bill.id) || []; const mbs = monthlyStates[bill.id]; const autopaySuggestion = applyAutopaySuggestions( db, @@ -551,7 +595,7 @@ function getTracker(userId, query = {}, now = new Date()) { row.snoozed_until = mbs?.snoozed_until ?? null; if (autopaySuggestion) row.autopay_suggestion = autopaySuggestion; row.previous_month_paid = fromCents(prevMonthPayments[bill.id] || 0); - row.amount_suggestion = computeAmountSuggestion(db, bill.id, year, month); + row.amount_suggestion = amountSuggestions.get(bill.id) ?? null; return row; }).filter(Boolean); diff --git a/tests/amountSuggestionService.test.js b/tests/amountSuggestionService.test.js new file mode 100644 index 0000000..6d377d0 --- /dev/null +++ b/tests/amountSuggestionService.test.js @@ -0,0 +1,74 @@ +'use strict'; + +// P1 — computeAmountSuggestionsBatch replaces up to 12 per-bill queries with two +// batched queries in getTracker. This pins that the batched form is byte-identical +// to calling the single-bill computeAmountSuggestion per bill (the behavior we +// rely on for the N+1 fix to be safe). +const test = require('node:test'); +const assert = require('node:assert/strict'); +const os = require('node:os'); +const path = require('node:path'); +const fs = require('node:fs'); + +const dbPath = path.join(os.tmpdir(), `bill-tracker-amountsug-${process.pid}.sqlite`); +process.env.DB_PATH = dbPath; + +const { getDb, closeDb } = require('../db/database'); +const { computeAmountSuggestion, computeAmountSuggestionsBatch } = require('../services/amountSuggestionService'); + +let db, userId, bills; +test.before(() => { + db = getDb(); + userId = db.prepare("INSERT INTO users (username, password_hash, role, active) VALUES ('as-user','x','user',1)").run().lastInsertRowid; + const insBill = db.prepare("INSERT INTO bills (user_id, name, due_day, expected_amount, active) VALUES (?, ?, 1, 10000, 1)"); + bills = { + payments: insBill.run(userId, 'Payments only').lastInsertRowid, // suggestion from payment sums + mbs: insBill.run(userId, 'Corrected').lastInsertRowid, // user-corrected actual_amount wins + mixed: insBill.run(userId, 'Mixed').lastInsertRowid, // some months mbs, some payments + empty: insBill.run(userId, 'No history').lastInsertRowid, // → null suggestion + }; + + const pay = db.prepare("INSERT INTO payments (bill_id, amount, paid_date, payment_source) VALUES (?, ?, ?, 'manual')"); + // For query month = 2026-06: prior months are May, Apr, Mar, Feb, Jan 2026, Dec 2025. + pay.run(bills.payments, 10000, '2026-05-15'); + pay.run(bills.payments, 12000, '2026-04-15'); + pay.run(bills.payments, 11000, '2026-03-15'); + // Two payments in one month should sum. + pay.run(bills.mixed, 3000, '2026-05-10'); + pay.run(bills.mixed, 2000, '2026-05-20'); // May total 5000 + pay.run(bills.mixed, 9000, '2026-04-10'); + + const mbs = db.prepare("INSERT INTO monthly_bill_state (bill_id, year, month, actual_amount) VALUES (?, ?, ?, ?)"); + mbs.run(bills.mbs, 2026, 5, 8000); + mbs.run(bills.mbs, 2026, 4, 8500); + mbs.run(bills.mbs, 2026, 3, 7500); + // Mixed: March has a corrected amount that should win over any payment sum. + mbs.run(bills.mixed, 2026, 3, 4200); +}); +test.after(() => { + closeDb(); + for (const s of ['', '-wal', '-shm']) { try { fs.rmSync(dbPath + s); } catch {} } +}); + +test('batch output equals per-bill computeAmountSuggestion for every bill', () => { + const ids = Object.values(bills); + const batch = computeAmountSuggestionsBatch(db, ids, 2026, 6); + for (const id of ids) { + const single = computeAmountSuggestion(db, id, 2026, 6); + assert.deepEqual(batch.get(id), single, `bill ${id} batch matches single`); + } +}); + +test('empty bill list returns an empty map; no-history bill → null', () => { + assert.equal(computeAmountSuggestionsBatch(db, [], 2026, 6).size, 0); + const batch = computeAmountSuggestionsBatch(db, [bills.empty], 2026, 6); + assert.equal(batch.get(bills.empty), null); +}); + +test('corrected monthly amount wins over payment sums (median of mbs values)', () => { + const s = computeAmountSuggestion(db, bills.mbs, 2026, 6); + // median of [8000, 8500, 7500] = 8000 → $80 + assert.equal(s.suggestion, 80); + assert.equal(s.months_used, 3); + assert.equal(s.confidence, 'high'); +});