From e5f1d7a916d5bb2b5a476c8328df019612b0de03 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Mon, 15 Jun 2026 20:46:41 +0200 Subject: [PATCH] fix(reports): count reversed entries in bank/supplier/AR reconciliation (#734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reversed (storno-corrected) journal entry must be summed together with its storno + correction, exactly as the trial balance and balance sheet already do (.in('status', ['posted','reversed'])). The reconciliation paths used posted-only and manufactured phantom differences. - bank-reconciliation getReconciliationStatus: gl_1930_balance now counts posted+reversed (so it equals the balansräkning for the account). Dropped the correction_adjustment subtraction and the reversed-linked-tx drop — a corrected/amount-corrected bank receipt now reconciles. The prior model broke once correctEntry began re-pointing the bank transaction to the live correction (the two changes were mutually inconsistent and produced a difference equal to the corrected amount). - supplier-reconciliation (2440) and ar-reconciliation (1510/1513): same posted -> posted+reversed fix. Removes the false "Ej avstämd" gap a fully paid, fully corrected company shows — the books net to 0 over posted+reversed while a posted-only query double-counted the payment legs. Also in this change: - MCP gnubok_get_reconciliation_status: add account_number param (was hardcoded to 1930; the lib already supported per-account reconciliation). - counterparty-templates normalizeCounterpartyName: strip trailing month / personal-initials tokens so "ngrok JW" / "Ngrok Mars" learn as one merchant. - pending-operation reject 409 (route + MCP tool): clarify a resolved op was approved explicitly (no auto-commit path exists) instead of a bare "already committed". - create_voucher: normalize description with String().trim() for consistency with line_description. - migration 20260625120000: backfill stripped diacritics on ~830 companies' seeded chart-of-accounts names (Foretagskonto -> Företagskonto, etc.); the seed function was fixed for new companies in 20260516130000 but never backfilled. UPDATE-only and idempotent. Already applied to production. Tests: 899 passing across the touched suites; lint + typecheck clean. Co-authored-by: Claude Opus 4.8 (1M context) --- .../pending-operations/[id]/reject/route.ts | 15 +- components/reports/BankReconciliationView.tsx | 4 +- extensions/general/mcp-server/server.ts | 56 +++++++- .../__tests__/counterparty-templates.test.ts | 17 +++ lib/bookkeeping/counterparty-templates.ts | 47 +++++- .../__tests__/bank-reconciliation.test.ts | 135 ++++++++++++------ lib/reconciliation/bank-reconciliation.ts | 102 +++++++------ .../__tests__/ar-reconciliation.test.ts | 44 +++++- .../__tests__/supplier-reconciliation.test.ts | 47 +++++- lib/reports/ar-reconciliation.ts | 19 ++- lib/reports/supplier-reconciliation.ts | 12 +- ..._backfill_chart_of_accounts_diacritics.sql | 56 ++++++++ 12 files changed, 445 insertions(+), 109 deletions(-) create mode 100644 supabase/migrations/20260625120000_backfill_chart_of_accounts_diacritics.sql diff --git a/app/api/pending-operations/[id]/reject/route.ts b/app/api/pending-operations/[id]/reject/route.ts index d90d7ae2..cd47bf85 100644 --- a/app/api/pending-operations/[id]/reject/route.ts +++ b/app/api/pending-operations/[id]/reject/route.ts @@ -73,8 +73,21 @@ export async function POST( } if (op.status !== 'pending') { + // There is no auto-commit path (removed in 20260505190027), so a non-pending + // status here means the op was resolved explicitly — almost always the user + // pressed Godkänn in the /pending (Att göra) UI in parallel, or another + // client already rejected it. Spell that out so an agent doesn't read the + // generic 409 as "the system committed it behind my back". + const explained = + op.status === 'rejected' + ? 'Operation already rejected.' + : op.status === 'expired' + ? 'Operation already expired and can no longer be rejected.' + : `Operation already ${op.status} — it was approved explicitly (most likely via the ` + + 'Att göra / pending UI in parallel), not auto-committed. It can no longer be rejected; ' + + 'reverse or correct the resulting verifikat instead.' return NextResponse.json( - { error: `Operation already ${op.status}` }, + { error: explained, status: op.status }, { status: 409 } ) } diff --git a/components/reports/BankReconciliationView.tsx b/components/reports/BankReconciliationView.tsx index 514a42ae..da3efc0f 100644 --- a/components/reports/BankReconciliationView.tsx +++ b/components/reports/BankReconciliationView.tsx @@ -618,9 +618,9 @@ export function BankReconciliationView() { )} {status.gl_1930_correction_adjustment !== 0 && (

- Rättelser och stornon på i perioden:{' '} + Varav rättelser och stornon på i perioden:{' '} {formatCurrency(status.gl_1930_correction_adjustment)} - {' '}— bokföringsmässiga rättelser utan motsvarande bankhändelse, räknas inte i avstämningen. + {' '}— ingår i det bokförda beloppet och i avstämningen, precis som i balansräkningen.

)}
diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index c6c6e57e..2aa725ff 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -5808,13 +5808,17 @@ export const tools: McpTool[] = [ { name: 'gnubok_get_reconciliation_status', title: 'Bank Reconciliation Status', - description: 'Bank reconciliation status: matched/unmatched counts, match rate, bank vs ledger balance, difference. Optional date range.', + description: 'Bank reconciliation for one cash account: matched/unmatched counts, bank vs ledger balance, difference. Defaults to 1930; pass account_number for 1940/1932 etc. Optional date range.', inputSchema: { type: 'object', additionalProperties: false, properties: { date_from: { type: 'string', description: 'Start date YYYY-MM-DD' }, date_to: { type: 'string', description: 'End date YYYY-MM-DD' }, + account_number: { + type: 'string', + description: 'Cash-account BAS code to reconcile, e.g. "1940". Defaults to "1930".', + }, }, }, outputSchema: { type: 'object' }, @@ -5827,7 +5831,37 @@ export const tools: McpTool[] = [ async execute(args, companyId, userId, supabase) { const dateFrom = args.date_from as string | undefined const dateTo = args.date_to as string | undefined - return await getReconciliationStatus(supabase, companyId, dateFrom, dateTo) + const accountNumber = (args.account_number as string | undefined) || '1930' + + // Pair the bank account with its currency + cash_account_id so EUR GL + // movements aren't compared against SEK transactions, and so a secondary + // same-currency account doesn't pool the primary's unassigned rows. Mirrors + // app/api/reconciliation/bank/status/route.ts. + const { data: cashAccount } = await supabase + .from('cash_accounts') + .select('id, currency, is_primary') + .eq('company_id', companyId) + .eq('ledger_account', accountNumber) + .maybeSingle() + + if (!cashAccount && accountNumber !== '1930') { + throw new Error(`Okänt kassakonto ${accountNumber} för det här företaget`) + } + + const currency = (cashAccount?.currency as string | undefined) ?? 'SEK' + const cashAccountId = cashAccount?.id as string | undefined + const includeUnassigned = cashAccount ? Boolean(cashAccount.is_primary) : true + + return await getReconciliationStatus( + supabase, + companyId, + dateFrom, + dateTo, + accountNumber, + currency, + cashAccountId, + includeUnassigned, + ) }, }, @@ -8413,7 +8447,10 @@ export const tools: McpTool[] = [ annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, async execute(args, companyId, userId, supabase, actor) { const entryDate = args.entry_date as string - const description = args.description as string + // Normalize like line_description (and gnubok_create_transactions) — coerce + // to string and trim, so a non-string or whitespace-only description is + // caught by the guard below instead of slipping into the preview/voucher. + const description = String(args.description ?? '').trim() const rawLines = args.lines as Array> | undefined if (!entryDate || !description || !Array.isArray(rawLines) || rawLines.length < 2) { @@ -9436,7 +9473,18 @@ export const tools: McpTool[] = [ .single() if (fetchError || !op) throw new Error('Pending operation not found') - if (op.status !== 'pending') throw new Error(`Operation already ${op.status}`) + if (op.status !== 'pending') { + // No auto-commit path exists (removed in 20260505190027). A non-pending + // status means the op was resolved explicitly — usually the user + // approved it in the Att göra / pending UI in parallel. Make that + // explicit so the agent doesn't read it as a silent auto-commit. + throw new Error( + op.status === 'rejected' + ? 'Operation already rejected.' + : `Operation already ${op.status} — approved explicitly (likely via the pending UI), ` + + 'not auto-committed. Reverse or correct the resulting verifikat instead.', + ) + } // Atomic claim — flips pending → rejected only when the row is still // pending AND in the caller's tenant (V8.3.1, CC6.3 tenant isolation). diff --git a/lib/bookkeeping/__tests__/counterparty-templates.test.ts b/lib/bookkeeping/__tests__/counterparty-templates.test.ts index 151790d9..fe667b23 100644 --- a/lib/bookkeeping/__tests__/counterparty-templates.test.ts +++ b/lib/bookkeeping/__tests__/counterparty-templates.test.ts @@ -69,6 +69,23 @@ describe('counterparty-templates', () => { it('preserves meaningful content', () => { expect(normalizeCounterpartyName('HEMKÖP LINNÉ')).toBe('hemköp linné') }) + + it('collapses trailing personal initials and month labels to one merchant', () => { + // Regression: three identical ngrok bookings ("ngrok JW", "Ngrok Mars", + // "ngrok JW") splintered into separate counterparty names, so matching + // never learned. They must all normalize to the same canonical merchant. + expect(normalizeCounterpartyName('ngrok JW')).toBe('ngrok') + expect(normalizeCounterpartyName('Ngrok Mars')).toBe('ngrok') + expect(normalizeCounterpartyName('SPOTIFY januari')).toBe('spotify') + expect(normalizeCounterpartyName('ICA MAXI AK')).toBe('ica maxi') + }) + + it('does not strip multi-letter trailing words or 3+ letter brands', () => { + // Conservative guard: only 1–2 char all-caps initials and month tokens go. + expect(normalizeCounterpartyName('SWISH ANDERS JOHANSSON')).toBe('anders johansson') + expect(normalizeCounterpartyName('NORDEA SEB')).toBe('nordea seb') // SEB is 3 chars — kept + expect(normalizeCounterpartyName('KLARNA')).toBe('klarna') // single token kept + }) }) // ── Confidence ───────────────────────────────────────────── diff --git a/lib/bookkeeping/counterparty-templates.ts b/lib/bookkeeping/counterparty-templates.ts index 45748ae6..503dd0b4 100644 --- a/lib/bookkeeping/counterparty-templates.ts +++ b/lib/bookkeeping/counterparty-templates.ts @@ -22,12 +22,49 @@ import type { SIEVoucher } from '@/lib/import/types' // ── Normalization ────────────────────────────────────────────── +/** + * Month tokens (Swedish + English, abbreviated and full) that show up as a + * trailing period label on a bank-feed description ("Ngrok Mars", "Spotify + * januari") rather than as part of the merchant's identity. + */ +const TRAILING_MONTH_TOKENS = new Set([ + 'jan', 'feb', 'mar', 'apr', 'maj', 'may', 'jun', 'jul', 'aug', 'sep', 'sept', + 'okt', 'oct', 'nov', 'dec', + 'januari', 'februari', 'mars', 'april', 'juni', 'juli', 'augusti', + 'september', 'oktober', 'november', 'december', +]) + +/** + * Strip trailing tokens that label *when/who* rather than *what merchant*: + * a month name, or a 1–2 letter all-caps personal initial ("ngrok JW", + * "ngrok JW", "Ngrok Mars" all describe the same merchant). Without this, one + * merchant splinters into many un-learnable variants and counterparty matching + * never fires (the reported ngrok bug: three prior bookings, zero matches). + * + * Conservative by design: only acts on a TRAILING token, only on 1–2 char + * all-caps initials (so 3-letter brands like SEB/ICA and any lowercased word + * survive), and always keeps at least one core token (never strips to empty). + */ +function stripTrailingNoiseTokens(s: string): string { + const tokens = s.trim().split(/\s+/).filter(Boolean) + while (tokens.length > 1) { + const last = tokens[tokens.length - 1] + const isMonth = TRAILING_MONTH_TOKENS.has(last.toLowerCase()) + // Personal initials: 1–2 letters, all-caps in the ORIGINAL casing (run + // before normalizeMerchantName lowercases everything). + const isInitials = /^[A-ZÅÄÖ]{1,2}$/.test(last) + if (!isMonth && !isInitials) break + tokens.pop() + } + return tokens.join(' ') +} + /** * Normalize a transaction description to a canonical counterparty name. * - * Strips bank transfer prefixes, trailing dates, invoice references, - * and trailing digit sequences, then delegates to normalizeMerchantName() - * for Swedish company suffix removal and lowercasing. + * Strips bank transfer prefixes, trailing dates, invoice references, trailing + * digit sequences, and trailing period/initials tokens, then delegates to + * normalizeMerchantName() for Swedish company suffix removal and lowercasing. */ export function normalizeCounterpartyName(raw: string): string { const cleaned = raw @@ -42,7 +79,9 @@ export function normalizeCounterpartyName(raw: string): string { .replace(/\s+\d{4,}\s*$/g, '') .trim() - return normalizeMerchantName(cleaned) + // Drop trailing month/initials tokens before merchant-name normalization so + // "ngrok JW" and "Ngrok Mars" collapse to the same canonical "ngrok". + return normalizeMerchantName(stripTrailingNoiseTokens(cleaned)) } // ── Confidence ───────────────────────────────────────────────── diff --git a/lib/reconciliation/__tests__/bank-reconciliation.test.ts b/lib/reconciliation/__tests__/bank-reconciliation.test.ts index 46f75b44..b64bfc05 100644 --- a/lib/reconciliation/__tests__/bank-reconciliation.test.ts +++ b/lib/reconciliation/__tests__/bank-reconciliation.test.ts @@ -828,60 +828,91 @@ describe('getReconciliationStatus', () => { expect(status.gl_1930_period_movement).toBe(200) }) - it('nets a book-only correction (storno + rättelse) to a reconciled period', async () => { - // The reported bug: a correction made via the storno flow puts a posted - // storno AND a posted correction on 1930, while the original flips to - // 'reversed'. Both posted vouchers have no bank-feed counterpart. Before the - // fix they inflated the period movement and showed as omatchade - // verifikationer, manufacturing a phantom diff. They must be excluded from - // the movement so a fully-matched period reconciles. + it('reconciles a corrected bank receipt and keeps gl_1930_balance equal to the balance sheet', async () => { + // A +25000 deposit was booked to the wrong counter-account, then corrected + // via the storno flow: the original flips to 'reversed', a storno (credit + // 25000) and a correction (debit 25000) are posted, and correctEntry + // re-points the bank transaction to the live correction (je-corr). + // + // Reconciliation now sums posted+reversed on 1930 — exactly as the trial + // balance / balance sheet do — so the cluster nets to the true +25000 and + // the period reconciles. gl_1930_balance must equal what the balansräkning + // shows for 1930 (the bug this widget used to have was the two disagreeing). const { supabase, enqueue } = createQueueMockSupabase() - // 1) transactions: one real matched outflow of -9908.75 + // 1) transactions: the +25000 deposit, re-pointed to the correction enqueue({ - data: [{ amount: -9908.75, journal_entry_id: 'je-others', reconciliation_method: 'auto_exact' }], + data: [{ amount: 25000, journal_entry_id: 'je-corr', reconciliation_method: 'manual' }], }) - // 2) GL lines on 1930: the matched outflow, plus the correction cluster. - // Original (credit 25000) is status='reversed'; storno (debit 25000) and - // correction (debit 25000) are posted. None of the cluster is linked. - enqueue({ - data: [ - { debit_amount: 0, credit_amount: 9908.75, journal_entries: { id: 'je-others', status: 'posted', source_type: 'bank_import' } }, - { debit_amount: 0, credit_amount: 25000, journal_entries: { id: 'je-orig', status: 'reversed', source_type: 'manual' } }, - { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-storno', status: 'posted', source_type: 'storno' } }, - { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-corr', status: 'posted', source_type: 'correction' } }, - ], - }) - // 3) RPC: empty (migration excludes storno/correction; reversed isn't posted) + // 2) GL lines on 1930: reversed original (debit 25000), storno (credit + // 25000), correction (debit 25000). All three are summed. + const lines = [ + { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-orig', status: 'reversed', source_type: 'bank_transaction' } }, + { debit_amount: 0, credit_amount: 25000, journal_entries: { id: 'je-storno', status: 'posted', source_type: 'storno' } }, + { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-corr', status: 'posted', source_type: 'correction' } }, + ] + enqueue({ data: lines }) + // 3) RPC: empty enqueue({ data: [] }) const status = await getReconciliationStatus(supabase as never, 'company-1') - // Posted balance still includes the storno/correction (+50000) and the - // matched outflow (-9908.75); the reversed original is not posted. - expect(status.gl_1930_balance).toBe(40091.25) - // …but those +50000 are book-only and excluded from the period movement. - expect(status.gl_1930_correction_adjustment).toBe(50000) - expect(status.gl_1930_period_movement).toBe(-9908.75) - expect(status.bank_transaction_total).toBe(-9908.75) + // Balance-sheet-equivalent: posted+reversed summed = 25000 - 25000 + 25000. + const balanceSheet1930 = lines.reduce( + (s, l) => s + l.debit_amount - l.credit_amount, + 0, + ) + expect(status.gl_1930_balance).toBe(balanceSheet1930) // 25000 — matches BS + expect(status.gl_1930_correction_adjustment).toBe(0) // storno + correction net + expect(status.gl_1930_period_movement).toBe(25000) + expect(status.bank_transaction_total).toBe(25000) expect(status.difference).toBe(0) expect(status.is_reconciled).toBe(true) }) - it('does not create a new phantom when a matched deposit is later corrected', async () => { - // Case 2: a +25000 deposit was matched to an entry, then that entry was - // corrected. The deposit's link stays on the now-'reversed' original. The - // movement excludes the storno/correction, so to stay symmetric the deposit - // (linked to a reversed entry) must drop off the bank side too — otherwise - // we'd swap the old -50000 phantom for a +25000 one. + it('reconciles an amount correction even though the correction adjustment is non-zero', async () => { + // Regression for the de-reconcile bug: a 25000 receipt was booked as 24000, + // then corrected to 25000. The storno (credit 24000) and correction (debit + // 25000) net to +1000 on 1930, and correctEntry re-points the real 25000 + // feed transaction to the correction. The OLD code subtracted that +1000 + // correction bucket from the movement while still counting the re-pointed + // 25000 transaction → a phantom 1000 diff. The unified inclusion rule nets + // it correctly: gl_balance = 25000 = the feed, difference = 0. + const { supabase, enqueue } = createQueueMockSupabase() + + enqueue({ + data: [{ amount: 25000, journal_entry_id: 'je-corr', reconciliation_method: 'manual' }], + }) + enqueue({ + data: [ + { debit_amount: 24000, credit_amount: 0, journal_entries: { id: 'je-orig', status: 'reversed', source_type: 'bank_transaction' } }, + { debit_amount: 0, credit_amount: 24000, journal_entries: { id: 'je-storno', status: 'posted', source_type: 'storno' } }, + { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-corr', status: 'posted', source_type: 'correction' } }, + ], + }) + enqueue({ data: [] }) + + const status = await getReconciliationStatus(supabase as never, 'company-1') + + expect(status.gl_1930_balance).toBe(25000) + expect(status.gl_1930_correction_adjustment).toBe(1000) // -24000 + 25000 + expect(status.gl_1930_period_movement).toBe(25000) + expect(status.bank_transaction_total).toBe(25000) + expect(status.difference).toBe(0) + expect(status.is_reconciled).toBe(true) + }) + + it('reconciles a legacy deposit still linked to the reversed original (no special-case drop)', async () => { + // Pre-relink data: the +25000 deposit was matched, the entry corrected, but + // the transaction was never re-pointed and still references the reversed + // original. With posted+reversed summed on the GL side and NO reversed-link + // dropping on the bank side, this still nets to zero — symmetric without any + // special case. const { supabase, enqueue } = createQueueMockSupabase() - // 1) transactions: the +25000 deposit, still linked to the reversed original enqueue({ data: [{ amount: 25000, journal_entry_id: 'je-orig', reconciliation_method: 'manual' }], }) - // 2) GL lines: reversed original (debit 25000), storno (credit 25000), - // correction (debit 25000) enqueue({ data: [ { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-orig', status: 'reversed', source_type: 'bank_transaction' } }, @@ -889,15 +920,37 @@ describe('getReconciliationStatus', () => { { debit_amount: 25000, credit_amount: 0, journal_entries: { id: 'je-corr', status: 'posted', source_type: 'correction' } }, ], }) - // 3) RPC: empty enqueue({ data: [] }) const status = await getReconciliationStatus(supabase as never, 'company-1') - // Deposit excluded (linked to a reversed entry); cluster excluded from movement. - expect(status.bank_transaction_total).toBe(0) - expect(status.gl_1930_period_movement).toBe(0) + expect(status.bank_transaction_total).toBe(25000) // counted, not dropped + expect(status.gl_1930_period_movement).toBe(25000) expect(status.difference).toBe(0) expect(status.is_reconciled).toBe(true) }) + + it('flags a book-only entry that moves the bank balance with no feed counterpart', async () => { + // Intentional behaviour: a manual posting that moves 1930 without a matching + // bank-feed transaction (e.g. interest the feed import missed, booked debit + // 1930 / credit 8310) is a genuine reconciliation break — the GL balance no + // longer matches the statement. It must surface as a difference, not be + // silently swept under a "correction" exclusion. + const { supabase, enqueue } = createQueueMockSupabase() + + enqueue({ data: [] }) // no bank-feed transactions + enqueue({ + data: [ + { debit_amount: 500, credit_amount: 0, journal_entries: { id: 'je-manual', status: 'posted', source_type: 'manual' } }, + ], + }) + enqueue({ data: [] }) + + const status = await getReconciliationStatus(supabase as never, 'company-1') + + expect(status.gl_1930_period_movement).toBe(500) + expect(status.bank_transaction_total).toBe(0) + expect(status.difference).toBe(-500) + expect(status.is_reconciled).toBe(false) + }) }) diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index 555165c9..9044c5e3 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -41,24 +41,25 @@ export interface ReconciliationRunResult { export interface ReconciliationStatus { bank_transaction_total: number /** - * @deprecated Use `gl_1930_period_movement` for the reconciliation diff. This - * field is preserved for back-compat with persisted status snapshots produced - * before the IB-exclusion change; new consumers reading this to compute the - * "real" difference will be off by the IB amount whenever a SIE-imported - * opening balance exists on 1930. The `difference` field on this interface - * is computed against `gl_1930_period_movement`, not this. + * The real ledger balance on the bank account, incl. IB — computed from the + * SAME `['posted','reversed']` lines the trial balance and balance sheet sum, + * so this value is identical to what the balansräkning reports for this + * account. (Use `gl_1930_period_movement` for the reconciliation diff, since + * this figure still includes the opening balance.) */ gl_1930_balance: number - /** Ledger movement on 1930 excluding opening_balance AND storno/correction - * lines — i.e. only movements that have a bank-feed counterpart. */ + /** Ledger movement on the bank account excluding only opening_balance — i.e. + * the ledger balance minus IB. Storno/correction lines ARE included here + * (they're part of the balance), so a corrected bank line reconciles against + * its re-pointed feed transaction. This is what `difference` compares against. */ gl_1930_period_movement: number - /** IB on 1930 within the date range — surfaced separately so reconciliation - * doesn't treat it as an unmatched bank transaction. */ + /** IB on the bank account within the date range — surfaced separately so + * reconciliation doesn't treat it as an unmatched bank transaction. */ gl_1930_opening_balance: number - /** Net of posted storno/correction lines on 1930 within the date range. - * Excluded from gl_1930_period_movement (and from the unmatched-voucher set) - * because a book-only correction has no counterpart in the bank feed. Surfaced - * separately so the UI can explain why a corrected period still reconciles. */ + /** Net of posted storno/correction lines on the bank account within the date + * range. INFORMATIONAL ONLY — it is part of the ledger balance and is included + * in gl_1930_period_movement, not subtracted from it. Surfaced so the UI can + * show how much of the period's movement came from corrections. */ gl_1930_correction_adjustment: number /** bankTotal − gl_1930_period_movement. Zero when every period transaction is matched. */ difference: number @@ -363,13 +364,16 @@ export async function getReconciliationStatus( const { data: transactions } = await txQuery - // Get GL bank account lines. Pull id/status/source_type from the join so we - // can (a) split out lines that have no bank-feed counterpart — opening_balance - // (prior year's closing balance) and storno/correction (book-only corrections) - // — and (b) identify reversed originals, whose still-linked bank transactions - // are superseded by the correction and must drop off the bank side too. - // 'reversed' is fetched alongside 'posted' precisely to resolve those links; - // reversed lines are NOT counted in any movement total. + // Get GL bank-account lines. We fetch posted AND reversed entries and count + // them TOGETHER — the exact inclusion rule the trial balance and balance sheet + // use (see lib/reports/trial-balance.ts, which sums `['posted','reversed']`). + // A reversed original stays in the ledger and is cancelled by its storno, so + // both legs must be summed; counting only the storno would leave a dangling + // half-correction. Using the identical rule here is what guarantees + // gl_1930_balance can never disagree with the balansräkning for this account — + // the headline bug this widget had (a corrected bank receipt showed one figure + // here and a different one on the balance sheet). source_type is still pulled + // so we can split out the opening balance and surface correction activity. let glQuery = supabase .from('journal_entry_lines') .select('debit_amount, credit_amount, journal_entries!inner(id, company_id, entry_date, status, source_type)') @@ -399,41 +403,47 @@ export async function getReconciliationStatus( return (Number(line.debit_amount) || 0) - (Number(line.credit_amount) || 0) } - const allLines = (glLines || []) as GlLineRow[] - const postedLines = allLines.filter((l) => entryOf(l)?.status === 'posted') + // posted + reversed = the ledger balance, exactly as the trial balance counts + // it. The .in() filter on the query already excludes draft/cancelled. + const countedLines = (glLines || []) as GlLineRow[] - // Reversed originals retain their bank-transaction link (the storno flow never - // re-points it), so a transaction pointing at one is a superseded booking — - // drop it from the bank side to keep the comparison symmetric with the - // movement, which excludes the matching storno/correction below. - const reversedEntryIds = new Set( - allLines - .filter((l) => entryOf(l)?.status === 'reversed') - .map((l) => entryOf(l)?.id) - .filter((id): id is string => Boolean(id)) + // Bank side: every feed transaction in range, full stop. We deliberately do + // NOT special-case rows linked to a reversed entry any more. Because the GL + // side now counts the reversed original, its storno AND the correction + // together (just like the balance sheet), a corrected bank line nets to its + // true amount on both sides and reconciles on its own — whether correctEntry + // re-pointed the transaction to the live corrected entry or a legacy row still + // points at the reversed original, the result is identical. The previous + // "drop reversed-linked transactions" rule paired with the now-removed + // correction subtraction below; together with correctEntry re-pointing the + // transaction they manufactured a difference equal to a corrected amount. + const bankTotal = (transactions || []).reduce( + (sum, tx) => sum + (Number(tx.amount) || 0), + 0 ) - // Calculate totals. Exclude transactions whose linked entry was reversed — - // their booking lives on in the correction, which is itself excluded from the - // movement, so counting the transaction would resurrect a phantom diff. - const bankTotal = (transactions || []).reduce((sum, tx) => { - if (tx.journal_entry_id && reversedEntryIds.has(tx.journal_entry_id)) return sum - return sum + (Number(tx.amount) || 0) - }, 0) - - // gl_1930_balance keeps its historical meaning: the posted balance incl. IB. - const glBalance = postedLines.reduce((sum, line) => sum + lineAmount(line), 0) - const glOpeningBalance = postedLines + // gl_1930_balance: the real ledger balance on this account incl. IB — + // byte-for-byte the figure the balansräkning / saldobalans report. + const glBalance = countedLines.reduce((sum, line) => sum + lineAmount(line), 0) + // IB is last year's closing position, not a movement with a bank-feed + // counterpart — surfaced separately and excluded from the period movement. + const glOpeningBalance = countedLines .filter((l) => entryOf(l)?.source_type === 'opening_balance') .reduce((sum, line) => sum + lineAmount(line), 0) - const glCorrectionAdjustment = postedLines + // Net storno/correction activity on the account this period. Surfaced for + // transparency ONLY — it is part of the ledger balance and is INCLUDED in the + // movement, never subtracted. (Subtracting it while still counting the + // re-pointed bank transaction is exactly what produced the old phantom diff.) + const glCorrectionAdjustment = countedLines .filter((l) => { const st = entryOf(l)?.source_type return st === 'storno' || st === 'correction' }) .reduce((sum, line) => sum + lineAmount(line), 0) - // Period movement = only the lines that have a bank-feed counterpart. - const glPeriodMovement = glBalance - glOpeningBalance - glCorrectionAdjustment + // Period movement = the ledger balance minus the opening balance. Everything + // else (real bookings, stornos and corrections alike) has — or should have — + // a bank-feed counterpart, so it stays in. + const glPeriodMovement = glBalance - glOpeningBalance const matchedCount = (transactions || []).filter( (tx) => tx.journal_entry_id !== null diff --git a/lib/reports/__tests__/ar-reconciliation.test.ts b/lib/reports/__tests__/ar-reconciliation.test.ts index 954af3b7..4539ceab 100644 --- a/lib/reports/__tests__/ar-reconciliation.test.ts +++ b/lib/reports/__tests__/ar-reconciliation.test.ts @@ -6,11 +6,15 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' let resultIdx: number let results: Array<{ data?: unknown; error?: unknown }> +let calls: Array<{ method: string; args: unknown[] }> function makeBuilder() { const b: Record = {} for (const m of ['select', 'eq', 'in']) { - b[m] = vi.fn().mockReturnValue(b) + b[m] = vi.fn().mockImplementation((...args: unknown[]) => { + calls.push({ method: m, args }) + return b + }) } b.single = vi.fn().mockImplementation(async () => results[resultIdx++] ?? { data: null, error: null }) b.then = (resolve: (v: unknown) => void) => resolve(results[resultIdx++] ?? { data: null, error: null }) @@ -32,6 +36,7 @@ beforeEach(() => { vi.clearAllMocks() resultIdx = 0 results = [] + calls = [] supabase = makeClient() }) @@ -232,6 +237,43 @@ describe('generateARReconciliation', () => { expect(result.is_reconciled).toBe(true) }) + it('counts posted AND reversed 1510 lines (corrected invoice nets correctly)', async () => { + // Same fix as supplier-reconciliation: a corrected customer invoice flips its + // original to status='reversed'. The reversed leg must be summed with the + // posted storno/correction or a corrected, settled invoice shows a phantom + // gap against the kundreskontra. + results = [ + // 0: invoices — single 5 000 SEK invoice still open + { + data: [{ total: 5000, paid_amount: 0, currency: 'SEK', exchange_rate: null }], + error: null, + }, + // 1: 1510 lines as returned by posted+reversed: original (reversed debit + // 5000), storno (credit 5000), correction (debit 5000). Net = 5000. + { + data: [ + { debit_amount: 5000, credit_amount: 0, journal_entry_id: 'reg-reversed' }, + { debit_amount: 0, credit_amount: 5000, journal_entry_id: 'storno' }, + { debit_amount: 5000, credit_amount: 0, journal_entry_id: 'correction' }, + ], + error: null, + }, + ] + + const result = await generateARReconciliation(supabase, 'company-1', 'period-1') + + expect(result.ar_ledger_total).toBe(5000) + expect(result.account_1510_balance).toBe(5000) + expect(result.difference).toBe(0) + expect(result.is_reconciled).toBe(true) + + // Guard the actual fix: the 1510/1513 query must include reversed entries. + const statusFilter = calls.find( + (c) => c.method === 'in' && c.args[0] === 'journal_entries.status', + ) + expect(statusFilter?.args[1]).toEqual(['posted', 'reversed']) + }) + it('uses Math.round for monetary precision', async () => { results = [ { diff --git a/lib/reports/__tests__/supplier-reconciliation.test.ts b/lib/reports/__tests__/supplier-reconciliation.test.ts index 2cae83ee..e15ffe8b 100644 --- a/lib/reports/__tests__/supplier-reconciliation.test.ts +++ b/lib/reports/__tests__/supplier-reconciliation.test.ts @@ -6,11 +6,15 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' let resultIdx: number let results: Array<{ data?: unknown; error?: unknown }> +let calls: Array<{ method: string; args: unknown[] }> function makeBuilder() { const b: Record = {} for (const m of ['select', 'eq', 'in']) { - b[m] = vi.fn().mockReturnValue(b) + b[m] = vi.fn().mockImplementation((...args: unknown[]) => { + calls.push({ method: m, args }) + return b + }) } b.single = vi.fn().mockImplementation(async () => results[resultIdx++] ?? { data: null, error: null }) b.then = (resolve: (v: unknown) => void) => resolve(results[resultIdx++] ?? { data: null, error: null }) @@ -32,6 +36,7 @@ beforeEach(() => { vi.clearAllMocks() resultIdx = 0 results = [] + calls = [] supabase = makeClient() }) @@ -208,6 +213,46 @@ describe('generateReconciliation', () => { expect(result.is_reconciled).toBe(false) }) + it('counts posted AND reversed 2440 lines (corrected invoice nets correctly)', async () => { + // Regression for the Arcim Technology AB false "Ej avstämd" gap: two supplier + // invoices were registered, corrected via the storno flow, and fully paid. + // The corrected registrations flip to status='reversed'. The leverantörs- + // reskontra shows 0 outstanding, and over posted+reversed the 2440 balance is + // 0 too — but a posted-only query saw only the storno + correction + payment + // legs and reported a phantom −41 121,25 kr debit. The query must include the + // reversed registration leg so both reconcile. + results = [ + // 0: supplier_invoices — both paid, nothing outstanding + { data: [], error: null }, + // 1: 2440 lines as returned by the posted+reversed query for one corrected, + // paid invoice of 11 231,25: registration (reversed credit), storno + // (debit), correction (credit), payment (debit). Net credit−debit = 0. + { + data: [ + { debit_amount: 0, credit_amount: 11231.25, journal_entry_id: 'reg-reversed' }, + { debit_amount: 11231.25, credit_amount: 0, journal_entry_id: 'storno' }, + { debit_amount: 0, credit_amount: 11231.25, journal_entry_id: 'correction' }, + { debit_amount: 11231.25, credit_amount: 0, journal_entry_id: 'payment' }, + ], + error: null, + }, + ] + + const result = await generateReconciliation(supabase, 'company-1', 'period-1') + + expect(result.supplier_ledger_total).toBe(0) + expect(result.account_2440_balance).toBe(0) + expect(result.difference).toBe(0) + expect(result.is_reconciled).toBe(true) + + // Guard the actual fix: the 2440 query must include reversed entries, not + // filter to posted-only (which excluded the reversed registration leg). + const statusFilter = calls.find( + (c) => c.method === 'in' && c.args[0] === 'journal_entries.status', + ) + expect(statusFilter?.args[1]).toEqual(['posted', 'reversed']) + }) + it('uses Math.round for monetary precision', async () => { results = [ { diff --git a/lib/reports/ar-reconciliation.ts b/lib/reports/ar-reconciliation.ts index 211455ae..a1f87ea2 100644 --- a/lib/reports/ar-reconciliation.ts +++ b/lib/reports/ar-reconciliation.ts @@ -62,12 +62,17 @@ export async function generateARReconciliation( return Math.round((sum + sek) * 100) / 100 }, 0) - // Get AR receivable balance from posted journal entry lines in this period. - // We sum 1510 (Kundfordringar) AND 1513 (Kundfordringar – delad faktura) so - // the comparison stays correct under ROT/RUT fakturamodellen, where the - // customer portion sits on 1510 and the Skatteverket claim on 1513 — both - // are open AR receivable from the company's perspective. 1513 is zero today - // (no fakturamodellen postings yet) so this is a forward-looking defense. + // Get AR receivable balance from the ledger in this period. We sum 1510 + // (Kundfordringar) AND 1513 (Kundfordringar – delad faktura) so the comparison + // stays correct under ROT/RUT fakturamodellen, where the customer portion sits + // on 1510 and the Skatteverket claim on 1513 — both are open AR receivable + // from the company's perspective. 1513 is zero today (no fakturamodellen + // postings yet) so this is a forward-looking defense. + // + // We count posted AND reversed entries together — the SAME inclusion rule the + // trial balance / balance sheet use. A corrected invoice flips its original to + // status='reversed'; that reversed leg is cancelled by the posted storno, so + // both must be summed or a corrected invoice manufactures a phantom gap. const { data: journalLines } = await supabase .from('journal_entry_lines') .select(` @@ -82,7 +87,7 @@ export async function generateARReconciliation( .in('account_number', ['1510', '1513']) .eq('journal_entries.company_id', companyId) .eq('journal_entries.fiscal_period_id', periodId) - .eq('journal_entries.status', 'posted') + .in('journal_entries.status', ['posted', 'reversed']) // Both 1510 and 1513 are debit-normal assets: balance = debits - credits let account1510Balance = 0 diff --git a/lib/reports/supplier-reconciliation.ts b/lib/reports/supplier-reconciliation.ts index a9f8c787..ee737239 100644 --- a/lib/reports/supplier-reconciliation.ts +++ b/lib/reports/supplier-reconciliation.ts @@ -59,7 +59,15 @@ export async function generateReconciliation( return Math.round((sum + sek) * 100) / 100 }, 0) - // Get account 2440 balance from posted journal entry lines in this period + // Get account 2440 balance from the ledger in this period. We count posted + // AND reversed entries together — the SAME inclusion rule the trial balance / + // balance sheet use. A corrected supplier invoice flips its original + // registration to status='reversed' (storno-service.ts); that reversed credit + // on 2440 is cancelled by the posted storno's debit, so BOTH legs must be + // summed or the report double-counts the payment debit and shows a phantom + // debit balance. (This is exactly the false −41 121,25 kr "Ej avstämd" gap a + // fully-paid, fully-corrected company hit: posted-only = −41 121,25, but + // posted+reversed = 0, matching the leverantörsreskontra.) const { data: journalLines } = await supabase .from('journal_entry_lines') .select(` @@ -74,7 +82,7 @@ export async function generateReconciliation( .eq('account_number', '2440') .eq('journal_entries.company_id', companyId) .eq('journal_entries.fiscal_period_id', periodId) - .eq('journal_entries.status', 'posted') + .in('journal_entries.status', ['posted', 'reversed']) // Account 2440 is a liability: credit normal balance // Balance = credits - debits diff --git a/supabase/migrations/20260625120000_backfill_chart_of_accounts_diacritics.sql b/supabase/migrations/20260625120000_backfill_chart_of_accounts_diacritics.sql new file mode 100644 index 00000000..ac0d65bb --- /dev/null +++ b/supabase/migrations/20260625120000_backfill_chart_of_accounts_diacritics.sql @@ -0,0 +1,56 @@ +-- Backfill: restore Swedish diacritics (å/ä/ö) on seeded chart-of-accounts names. +-- +-- The seed_chart_of_accounts() helper shipped between 2026-03-30 +-- (20260330130000_multi_tenant_company_refactor.sql) and 2026-05-16 with its +-- account-name string literals stripped of å/ä/ö — e.g. 'Foretagskonto / +-- checkkonto', 'Leverantorsskulder', 'Arets resultat', 'Loner'. The function was +-- repaired for NEW companies by 20260516130000_seed_chart_of_accounts_restore_ +-- swedish_chars.sql, but the ~830 companies seeded in that window were never +-- backfilled, so their books still render bank/expense/VAT accounts without +-- diacritics (the reported "Foretagskonto / checkkonto" / "Ovriga bankkonton"). +-- +-- This restores the diacritics on the AFFECTED rows only. It maps each corrupted +-- name back to the diacritic form OF THAT SAME ACCOUNT (same account_number and +-- same wording) — it deliberately does NOT adopt the later seed's restructured +-- VAT accounts (2610/2611/2612 stay as-is with their names fixed; they are not +-- renumbered to 2611/2621/2631 — that is a structural change, out of scope for a +-- charset repair). +-- +-- Safe + idempotent: the join matches on account_number AND the exact corrupted +-- account_name, so a company that renamed an account, or a row already carrying +-- the correct name, is left untouched. Re-running is a no-op. + +UPDATE public.chart_of_accounts AS coa +SET account_name = fix.correct_name +FROM ( + VALUES + ('1930', 'Foretagskonto / checkkonto', 'Företagskonto / checkkonto'), + ('1940', 'Ovriga bankkonton', 'Övriga bankkonton'), + ('2013', 'Ovriga egna uttag', 'Övriga egna uttag'), + ('2018', 'Ovriga egna insattningar', 'Övriga egna insättningar'), + ('2099', 'Arets resultat', 'Årets resultat'), + ('2440', 'Leverantorsskulder', 'Leverantörsskulder'), + ('2610', 'Utgaende moms 25%', 'Utgående moms 25%'), + ('2611', 'Utgaende moms 12%', 'Utgående moms 12%'), + ('2612', 'Utgaende moms 6%', 'Utgående moms 6%'), + ('2641', 'Debiterad ingaende moms', 'Debiterad ingående moms'), + ('2650', 'Redovisningskonto for moms', 'Redovisningskonto för moms'), + ('2731', 'Avrakning socialavgifter', 'Avräkning socialavgifter'), + ('2893', 'Skuld till aktieagare', 'Skuld till aktieägare'), + ('3001', 'Forsaljning tjanster 25%', 'Försäljning tjänster 25%'), + ('3002', 'Forsaljning varor 25%', 'Försäljning varor 25%'), + ('3100', 'Momsfri forsaljning', 'Momsfri försäljning'), + ('3900', 'Ovriga rorelseintakter', 'Övriga rörelseintäkter'), + ('4000', 'Varuinkop', 'Varuinköp'), + ('5410', 'Forbrukningsinventarier', 'Förbrukningsinventarier'), + ('5460', 'Forbrukningsmaterial', 'Förbrukningsmaterial'), + ('6530', 'Redovisningstjanster', 'Redovisningstjänster'), + ('6991', 'Ovriga avdragsgilla kostnader', 'Övriga avdragsgilla kostnader'), + ('7010', 'Loner', 'Löner'), + ('7210', 'Semesterloner', 'Semesterlöner'), + ('7960', 'Valutakursforluster', 'Valutakursförluster'), + ('8310', 'Ranteintakter', 'Ränteintäkter'), + ('8410', 'Rantekostnader', 'Räntekostnader') +) AS fix(account_number, corrupted_name, correct_name) +WHERE coa.account_number = fix.account_number + AND coa.account_name = fix.corrupted_name;