perf(tracker): batch getTracker per-bill queries (kill N+1) (P1)
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 <noreply@anthropic.com>
This commit is contained in:
null
parent
73631ab812
commit
9f4b53d37a
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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<billId, suggestion|null>. 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 };
|
||||
|
|
|
|||
|
|
@ -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<billId, payments[]> 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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
Loading…
Reference in New Issue