From 37ee125b9bc153e4388407595291c67afec21818 Mon Sep 17 00:00:00 2001 From: Jonas Hagberg Date: Mon, 29 Jun 2026 22:49:10 +0200 Subject: [PATCH] fix(mcp): paginate trial-balance and VAT aggregations (1000-row truncation) (#806) * fix(mcp): paginate trial-balance and VAT aggregations (1000-row truncation) The gnubok_get_trial_balance tool and computeVatReport each ran an unbounded journal_entry_lines aggregation. PostgREST caps an unpaginated .select() at 1000 rows, so any period with >1000 entry lines silently truncated: wrong per-account sums and a false "not balanced" trial balance, and an under-reported momsdeklaration for yearly or busy quarterly VAT periods. - get_trial_balance now delegates to the canonical generateTrialBalance (lib/reports), which paginates via fetchAllRows and rolls opening balances forward, also fixing a latent bug where the tool ignored IB. - computeVatReport now paginates its line fetch via fetchAllRows. The library fixed this class of bug in #79; these two MCP paths kept their own copies that were never updated. Signed-off-by: Jonas Hagberg * fix(mcp): non-null assert periodId in generateTrialBalance call Fixes the core-only TS build error (string | undefined not assignable to string). periodId is guaranteed defined by the !period guard above; mirrors the existing periodId! call later in the file. Co-Authored-By: Claude Opus 4.8 (1M context) * test(mcp): support .range() in computeVatReport mock for paginated query computeVatReport now fetches journal_entry_lines via fetchAllRows (.range), but the hand-rolled mock terminated at .lte(). Move the terminal to .range() so the 8 VAT-aggregation tests exercise the paginated path. Test-only. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Signed-off-by: Jonas Hagberg Co-authored-by: Jakob Wennberg Co-authored-by: Claude Opus 4.8 (1M context) --- .../__tests__/vat-report-compute.test.ts | 10 ++- extensions/general/mcp-server/server.ts | 78 ++++++++----------- 2 files changed, 39 insertions(+), 49 deletions(-) diff --git a/extensions/general/mcp-server/__tests__/vat-report-compute.test.ts b/extensions/general/mcp-server/__tests__/vat-report-compute.test.ts index daff5b0b..9b0411d9 100644 --- a/extensions/general/mcp-server/__tests__/vat-report-compute.test.ts +++ b/extensions/general/mcp-server/__tests__/vat-report-compute.test.ts @@ -17,12 +17,14 @@ interface MockLine { function mockSupabaseWithLines(lines: MockLine[]) { // Build a chain that matches the call path in computeVatReport: - // .from('journal_entry_lines').select(...).eq(...).in(...).gte(...).lte(...) - // The terminal `.lte()` returns `{ data, error }`. + // .from('journal_entry_lines').select(...).eq(...).in(...).gte(...).lte(...).range(from, to) + // computeVatReport now paginates via fetchAllRows, so the terminal call is + // `.range(from, to)`. Returning all lines on the first page (always < the + // 1000-row PAGE_SIZE for these fixtures) makes fetchAllRows stop after one page. const terminal = { data: lines, error: null } const chain: Record unknown> = {} - // Terminal awaitable: vitest awaits the last call; .lte() returns the data. - chain.lte = () => terminal + chain.range = () => terminal + chain.lte = () => chain chain.gte = () => chain chain.in = () => chain chain.eq = () => chain diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index a7c746b9..a537ba3a 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -26,6 +26,7 @@ import { calculateVatLiability, } from '@/lib/reports/kpi' import { generateTrialBalance } from '@/lib/reports/trial-balance' +import { fetchAllRows } from '@/lib/supabase/fetch-all' import { generateARLedger } from '@/lib/reports/ar-ledger' import { generateMonthlyBreakdown } from '@/lib/reports/monthly-breakdown' import { uiWidgets, findUiWidget, WIDGET_MIME_TYPE } from './widgets' @@ -1035,18 +1036,26 @@ export async function computeVatReport( endDate = `${year}-12-31` } - const { data: lines, error } = await supabase - .from('journal_entry_lines') - .select('account_number, debit_amount, credit_amount, journal_entries!inner(entry_date, status, user_id)') - .eq('journal_entries.company_id', companyId) - .in('journal_entries.status', ['posted', 'reversed']) - .gte('journal_entries.entry_date', startDate) - .lte('journal_entries.entry_date', endDate) - - if (error) throw new Error(`Database error: ${error.message}`) + // Paginate. An unbounded .select() caps at PostgREST's 1000-row default, + // which silently truncates a yearly (or busy quarterly) VAT period with + // >1000 entry lines and under-reports the momsdeklaration. + const lines = await fetchAllRows<{ + account_number: string + debit_amount: number + credit_amount: number + }>(({ from, to }) => + supabase + .from('journal_entry_lines') + .select('account_number, debit_amount, credit_amount, journal_entries!inner(entry_date, status, user_id)') + .eq('journal_entries.company_id', companyId) + .in('journal_entries.status', ['posted', 'reversed']) + .gte('journal_entries.entry_date', startDate) + .lte('journal_entries.entry_date', endDate) + .range(from, to) + ) const accountTotals = new Map() - for (const line of lines ?? []) { + for (const line of lines) { const acc = line.account_number const existing = accountTotals.get(acc) ?? { debit: 0, credit: 0 } existing.debit += Number(line.debit_amount) || 0 @@ -3396,47 +3405,26 @@ export const tools: McpTool[] = [ if (!period) throw new Error('Fiscal period not found.') - // Aggregate journal entry lines - const { data: lines, error } = await supabase - .from('journal_entry_lines') - .select('account_number, debit_amount, credit_amount, journal_entries!inner(status, user_id, fiscal_period_id)') - .eq('journal_entries.company_id', companyId) - .eq('journal_entries.fiscal_period_id', periodId) - .in('journal_entries.status', ['posted', 'reversed']) + // Delegate to the canonical, paginated trial-balance builder. The + // previous inline query had no pagination, so PostgREST's 1000-row + // default silently truncated any period with >1000 entry lines (wrong + // sums, false "not balanced"), and it ignored opening balances. + // generateTrialBalance paginates and rolls IB forward. + const trialBalance = await generateTrialBalance(supabase, companyId, periodId!) - if (error) throw new Error(`Database error: ${error.message}`) - - // Get account names - const { data: accounts } = await supabase - .from('chart_of_accounts') - .select('account_number, account_name') - .eq('company_id', companyId) - - const accountMap = new Map((accounts ?? []).map((a: { account_number: string; account_name: string }) => [a.account_number, a.account_name])) - - // Aggregate by account - const totals = new Map() - for (const line of lines ?? []) { - const acc = line.account_number - const existing = totals.get(acc) ?? { debit: 0, credit: 0 } - existing.debit += Number(line.debit_amount) || 0 - existing.credit += Number(line.credit_amount) || 0 - totals.set(acc, existing) - } - - const rows = Array.from(totals.entries()) - .sort(([a], [b]) => a.localeCompare(b)) - .map(([accNum, t]) => { - const net = Math.round((t.debit - t.credit) * 100) / 100 + const rows = trialBalance.rows + .map((r) => { + const net = Math.round((r.closing_debit - r.closing_credit) * 100) / 100 return { - account_number: accNum, - account_name: accountMap.get(accNum) ?? accNum, - period_debit: Math.round(t.debit * 100) / 100, - period_credit: Math.round(t.credit * 100) / 100, + account_number: r.account_number, + account_name: r.account_name, + period_debit: r.period_debit, + period_credit: r.period_credit, closing_debit: net > 0 ? net : 0, closing_credit: net < 0 ? Math.abs(net) : 0, } }) + .sort((a, b) => a.account_number.localeCompare(b.account_number)) const totalDebit = Math.round(rows.reduce((s, r) => s + r.closing_debit, 0) * 100) / 100 const totalCredit = Math.round(rows.reduce((s, r) => s + r.closing_credit, 0) * 100) / 100