fix: unignore transactions during categorization (#1683)

Fixes #1660

lib/bookkeeping/__tests__/cancel-orphaned-entry.test.ts

@@ -1,8 +1,19 @@
-import { describe, it, expect, vi } from 'vitest'
+import { beforeEach, describe, it, expect, vi } from 'vitest'
+
+const { reverseEntryMock } = vi.hoisted(() => ({
+  reverseEntryMock: vi.fn(),
+}))
+
+vi.mock('@/lib/bookkeeping/engine', () => ({
+  reverseEntry: reverseEntryMock,
+}))
+
 import {
   cancelOrphanedPaymentEntry,
   recordVoucherGapExplanation,
+  reverseOrphanedJournalEntry,
 } from '../cancel-orphaned-entry'
+import { withUnusedVoucherAllocation } from '../errors'
 
 // The real voucher_gap_explanations column set (supabase/migrations/
 // 20260402100100_voucher_gap_explanations.sql). company_id, user_id,
@@ -55,6 +66,11 @@ function createMockSupabase(opts: {
   return { supabase, updates, inserts }
 }
 
+beforeEach(() => {
+  reverseEntryMock.mockReset()
+  reverseEntryMock.mockResolvedValue(undefined)
+})
+
 describe('recordVoucherGapExplanation', () => {
   it('writes exactly the real NOT NULL columns, with the single voucher as a closed gap range', async () => {
     const { supabase, inserts } = createMockSupabase({})
@@ -143,6 +159,77 @@ describe('recordVoucherGapExplanation', () => {
   })
 })
 
+describe('reverseOrphanedJournalEntry', () => {
+  it('routes posted-orphan compensation through engine storno', async () => {
+    const { supabase, inserts, updates } = createMockSupabase({})
+
+    await reverseOrphanedJournalEntry(
+      supabase as never,
+      'company-1',
+      'user-1',
+      'je-1',
+      'Manuell avstämning krävs.',
+    )
+
+    expect(reverseEntryMock).toHaveBeenCalledWith(
+      supabase,
+      'company-1',
+      'user-1',
+      'je-1',
+    )
+    expect(updates).toEqual([])
+    expect(inserts['voucher_gap_explanations']).toBeUndefined()
+  })
+
+  it('documents only the exact unused reversal voucher exposed by the engine', async () => {
+    const { supabase, inserts } = createMockSupabase({})
+    reverseEntryMock.mockRejectedValueOnce(
+      withUnusedVoucherAllocation(new Error('account lookup failed'), {
+        fiscalPeriodId: 'fp-1',
+        voucherSeries: 'B',
+        voucherNumber: 67,
+      }),
+    )
+
+    await reverseOrphanedJournalEntry(
+      supabase as never,
+      'company-1',
+      'user-1',
+      'je-1',
+      'Manuell avstämning krävs.',
+    )
+
+    expect(inserts['voucher_gap_explanations']).toEqual([
+      {
+        company_id: 'company-1',
+        user_id: 'user-1',
+        fiscal_period_id: 'fp-1',
+        voucher_series: 'B',
+        gap_start: 67,
+        gap_end: 67,
+        explanation: 'Manuell avstämning krävs.',
+      },
+    ])
+  })
+
+  it('does not mislabel the original posted voucher when storno failure has no unused allocation', async () => {
+    const { supabase, inserts } = createMockSupabase({
+      orphan: { fiscal_period_id: 'fp-1', voucher_series: 'B', voucher_number: 66 },
+    })
+    reverseEntryMock.mockRejectedValueOnce(new Error('period locked'))
+
+    await reverseOrphanedJournalEntry(
+      supabase as never,
+      'company-1',
+      'user-1',
+      'je-1',
+      'Manuell avstämning krävs.',
+    )
+
+    expect(inserts['voucher_gap_explanations']).toBeUndefined()
+  })
+})
+
 describe('cancelOrphanedPaymentEntry', () => {
   it('cancels the voucher and records a gap explanation', async () => {
     const { supabase, updates, inserts } = createMockSupabase({

lib/bookkeeping/__tests__/engine.test.ts

@@ -1,6 +1,11 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { validateBalance, getSwedishLocalDate, createDraftEntry, reverseEntry } from '../engine'
-import { BookkeepingDatabaseError, AccountsNotInChartError, CannotReverseStornoError } from '../errors'
+import {
+  AccountsNotInChartError,
+  BookkeepingDatabaseError,
+  CannotReverseStornoError,
+  getUnusedVoucherAllocation,
+} from '../errors'
 import type { CreateJournalEntryLineInput, JournalEntryStatus } from '@/types'
 
 // Mock Supabase client for createDraftEntry/reverseEntry tests
@@ -612,6 +617,141 @@ describe('reverseEntry: entry_date defaults to original entry date', () => {
   })
 })
 
+describe('reverseEntry: unused voucher allocation', () => {
+  it('exposes the exact allocated number when account resolution fails before the reversal insert', async () => {
+    const original = {
+      id: 'entry-1',
+      company_id: 'company-1',
+      status: 'posted',
+      fiscal_period_id: 'period-1',
+      voucher_series: 'B',
+      voucher_number: 41,
+      entry_date: '2024-11-15',
+      description: 'Hyra november',
+      source_type: 'manual',
+      source_id: null,
+      lines: [
+        { account_number: '5010', debit_amount: 10000, credit_amount: 0 },
+        { account_number: '1930', debit_amount: 0, credit_amount: 10000 },
+      ],
+    }
+    const supabase = {
+      rpc: vi.fn().mockResolvedValue({ data: 42, error: null }),
+      from: vi.fn().mockImplementation((table: string) => {
+        if (table === 'journal_entries') {
+          const chain = createMockChain({ singleData: original })
+          return chain
+        }
+        if (table === 'chart_of_accounts') {
+          const chain: Record<string, unknown> = {}
+          for (const method of ['select', 'eq', 'in']) {
+            chain[method] = vi.fn().mockReturnValue(chain)
+          }
+          chain.then = (resolve: (value: unknown) => void) =>
+            resolve({ data: null, error: { message: 'account lookup failed' } })
+          return chain
+        }
+        return createMockChain()
+      }),
+    }
+
+    let caught: unknown
+    try {
+      await reverseEntry(supabase as never, 'company-1', 'user-1', 'entry-1')
+    } catch (error) {
+      caught = error
+    }
+
+    expect(caught).toBeInstanceOf(BookkeepingDatabaseError)
+    expect(getUnusedVoucherAllocation(caught)).toEqual({
+      fiscalPeriodId: 'period-1',
+      voucherSeries: 'B',
+      voucherNumber: 42,
+    })
+  })
+
+  it('does not label a preserved cancelled reversal header as an unused voucher', async () => {
+    const original = {
+      id: 'entry-1',
+      company_id: 'company-1',
+      status: 'posted',
+      fiscal_period_id: 'period-1',
+      voucher_series: 'B',
+      voucher_number: 41,
+      entry_date: '2024-11-15',
+      description: 'Hyra november',
+      source_type: 'manual',
+      source_id: null,
+      lines: [
+        { account_number: '5010', debit_amount: 10000, credit_amount: 0 },
+        { account_number: '1930', debit_amount: 0, credit_amount: 10000 },
+      ],
+    }
+    const reversal = { id: 'reversal-1', reverses_id: 'entry-1' }
+    const cancelUpdate = vi.fn()
+    const deleteLines = vi.fn()
+    let journalEntryCall = 0
+    let journalLineCall = 0
+
+    const supabase = {
+      rpc: vi.fn().mockResolvedValue({ data: 42, error: null }),
+      from: vi.fn().mockImplementation((table: string) => {
+        if (table === 'journal_entries') {
+          journalEntryCall += 1
+          if (journalEntryCall === 1) return createMockChain({ singleData: original })
+          if (journalEntryCall === 2) return createMockChain({ singleData: reversal })
+          return {
+            update: cancelUpdate.mockReturnValue({
+              eq: vi.fn().mockResolvedValue({ data: null, error: null }),
+            }),
+          }
+        }
+        if (table === 'chart_of_accounts') {
+          const chain: Record<string, unknown> = {}
+          for (const method of ['select', 'eq', 'in']) {
+            chain[method] = vi.fn().mockReturnValue(chain)
+          }
+          chain.then = (resolve: (value: unknown) => void) =>
+            resolve({
+              data: [
+                { id: 'acc-5010', account_number: '5010' },
+                { id: 'acc-1930', account_number: '1930' },
+              ],
+              error: null,
+            })
+          return chain
+        }
+        if (table === 'journal_entry_lines') {
+          journalLineCall += 1
+          if (journalLineCall === 1) {
+            return {
+              insert: vi.fn().mockResolvedValue({ error: { message: 'line insert failed' } }),
+            }
+          }
+          return {
+            delete: deleteLines.mockReturnValue({
+              eq: vi.fn().mockResolvedValue({ data: null, error: null }),
+            }),
+          }
+        }
+        return createMockChain()
+      }),
+    }
+
+    let caught: unknown
+    try {
+      await reverseEntry(supabase as never, 'company-1', 'user-1', 'entry-1')
+    } catch (error) {
+      caught = error
+    }
+
+    expect(caught).toBeInstanceOf(BookkeepingDatabaseError)
+    expect(getUnusedVoucherAllocation(caught)).toBeNull()
+    expect(cancelUpdate).toHaveBeenCalledWith({ status: 'cancelled' })
+    expect(deleteLines).toHaveBeenCalled()
+  })
+})
+
 describe('reverseEntry: storno guard', () => {
   // BFL 5 kap 5§: a storno-of-a-storno makes the original verifikat's
   // cancellation chain ambiguous, so stornos are never reversible. A

lib/bookkeeping/cancel-orphaned-entry.ts

@@ -1,5 +1,7 @@
 import type { SupabaseClient } from '@supabase/supabase-js'
 import { createLogger } from '@/lib/logger'
+import { reverseEntry } from '@/lib/bookkeeping/engine'
+import { getUnusedVoucherAllocation } from '@/lib/bookkeeping/errors'
 
 const log = createLogger('cancel-orphaned-entry')
 
@@ -71,6 +73,49 @@ export async function recordVoucherGapExplanation(
   }
 }
 
+/**
+ * Storno a posted journal entry that could not be linked to its transaction.
+ *
+ * The bookkeeping engine posts entries before the transaction CAS runs. When
+ * that CAS definitively fails, the entry is immutable and must be reversed,
+ * never edited or cancelled in place. Compensation is best-effort so the
+ * caller can preserve the original conflict response.
+ */
+export async function reverseOrphanedJournalEntry(
+  supabase: SupabaseClient,
+  companyId: string,
+  userId: string,
+  journalEntryId: string,
+  gapExplanation: string,
+): Promise<void> {
+  let unusedVoucher: ReturnType<typeof getUnusedVoucherAllocation> = null
+  try {
+    await reverseEntry(supabase, companyId, userId, journalEntryId)
+    return
+  } catch (reverseError) {
+    unusedVoucher = getUnusedVoucherAllocation(reverseError)
+    log.error('failed to storno orphaned journal entry', reverseError as Error, {
+      companyId,
+      journalEntryId,
+      unusedVoucher,
+    })
+  }
+
+  // The original posted voucher is live accounting evidence, never a gap.
+  // Only the engine can identify an exact reversal number that its durable
+  // sequence allocated before a reversal row existed.
+  if (!unusedVoucher) return
+
+  await recordVoucherGapExplanation(supabase, {
+    companyId,
+    userId,
+    fiscalPeriodId: unusedVoucher.fiscalPeriodId,
+    voucherSeries: unusedVoucher.voucherSeries,
+    voucherNumber: unusedVoucher.voucherNumber,
+    explanation: gapExplanation,
+  })
+}
+
 /**
  * Compensation for the payment-flow CAS guard: a payment voucher was posted,
  * but the invoice row was settled by a concurrent request between our read

lib/bookkeeping/engine.ts

@@ -11,6 +11,7 @@ import {
   EntryAlreadyReversedError,
   EntryDateOutsideFiscalPeriodError,
   FiscalPeriodNotFoundError,
+  withUnusedVoucherAllocation,
   JournalEntryNotBalancedError,
   JournalEntryNotFoundError,
 } from '@/lib/bookkeeping/errors'
@@ -1122,18 +1123,33 @@ export async function reverseEntry(
     original.fiscal_period_id,
     original.voucher_series || 'A'
   )
+  const unusedVoucherAllocation = {
+    fiscalPeriodId: original.fiscal_period_id,
+    voucherSeries: original.voucher_series || 'A',
+    voucherNumber,
+  }
 
   // Resolve account IDs: include inactive rows. The accounts on the
   // original committed entry were active at commit time; if the user has
   // since toggled one off, the storno must still be allowed to go through
   // (BFL 5 kap 5§). Only a truly missing chart row (rare: would require
   // the row to have been deleted) still throws AccountsNotInChartError.
-  const accountIdMap = await resolveAccountIds(supabase, companyId, reversedLines, { includeInactive: true })
+  let accountIdMap: Map<string, string>
+  try {
+    accountIdMap = await resolveAccountIds(supabase, companyId, reversedLines, { includeInactive: true })
+  } catch (resolveError) {
+    // The sequence RPC committed, but no reversal row exists yet. Carry the
+    // exact unused number so the caller can document this real gap.
+    throw withUnusedVoucherAllocation(resolveError, unusedVoucherAllocation)
+  }
 
   const reversalAccountNumbers = [...new Set(reversedLines.map(l => l.account_number))]
   const missingReversalAccounts = reversalAccountNumbers.filter(num => !accountIdMap.has(num))
   if (missingReversalAccounts.length > 0) {
-    throw new AccountsNotInChartError(missingReversalAccounts)
+    throw withUnusedVoucherAllocation(
+      new AccountsNotInChartError(missingReversalAccounts),
+      unusedVoucherAllocation,
+    )
   }
 
   // Create reversal entry with reverses_id link
@@ -1155,8 +1171,18 @@ export async function reverseEntry(
     .select()
     .single()
 
-  if (reversalError || !reversalEntry) {
-    throw new BookkeepingDatabaseError('create_reversal_entry', reversalError?.message)
+  if (reversalError) {
+    // PostgreSQL rejected the insert, so the allocated number is confirmed
+    // unused and can be explained without guessing from the original entry.
+    throw withUnusedVoucherAllocation(
+      new BookkeepingDatabaseError('create_reversal_entry', reversalError.message),
+      unusedVoucherAllocation,
+    )
+  }
+  if (!reversalEntry) {
+    // No database error means the outcome is ambiguous. Do not label the
+    // number unused unless the engine has a confirmed failure state.
+    throw new BookkeepingDatabaseError('create_reversal_entry', undefined)
   }
 
   // Insert reversal lines with dimensions
@@ -1167,6 +1193,9 @@ export async function reverseEntry(
     .insert(lineInserts)
 
   if (linesError) {
+    // Keep the reversal header as cancelled bookkeeping evidence. The gap
+    // detector counts every non-draft header, including cancelled rows, so
+    // this allocation is still used and must not be labelled as a gap.
     await supabase.from('journal_entries').update({ status: 'cancelled' }).eq('id', reversalEntry.id)
     await supabase.from('journal_entry_lines').delete().eq('journal_entry_id', reversalEntry.id)
     throw new BookkeepingDatabaseError('create_reversal_lines', linesError.message)
@@ -1179,6 +1208,8 @@ export async function reverseEntry(
     .eq('id', reversalEntry.id)
 
   if (postError) {
+    // As above, cleanup preserves the allocated voucher on the cancelled
+    // header. A failed or ambiguous cleanup also cannot prove it unused.
     await supabase.from('journal_entries').update({ status: 'cancelled' }).eq('id', reversalEntry.id)
     await supabase.from('journal_entry_lines').delete().eq('journal_entry_id', reversalEntry.id)
     throw new BookkeepingDatabaseError('post_reversal_entry', postError.message)
@@ -1198,6 +1229,7 @@ export async function reverseEntry(
   if (casError || !updatedOriginal || updatedOriginal.length === 0) {
     // Another concurrent reversal already changed the status: mark the orphaned
     // reversal as cancelled so it's excluded from reports but remains traceable.
+    // Its header still occupies the voucher number, so no gap metadata applies.
     await supabase.from('journal_entries').update({ status: 'cancelled' }).eq('id', reversalEntry.id)
     await supabase.from('journal_entry_lines').delete().eq('journal_entry_id', reversalEntry.id)
     throw new EntryAlreadyReversedError()

lib/bookkeeping/errors.ts

@@ -341,6 +341,39 @@ export class BookkeepingDatabaseError extends Error {
   }
 }
 
+export interface UnusedVoucherAllocation {
+  fiscalPeriodId: string
+  voucherSeries: string
+  voucherNumber: number
+}
+
+const UNUSED_VOUCHER_ALLOCATION = Symbol('unused-voucher-allocation')
+
+/**
+ * Preserve the exact durable sequence allocation when an engine operation
+ * fails before any journal-entry row uses the number. The original error type
+ * is retained so existing API mappings remain unchanged.
+ */
+export function withUnusedVoucherAllocation<T>(
+  error: T,
+  allocation: UnusedVoucherAllocation,
+): T {
+  if (error instanceof Error) {
+    Object.defineProperty(error, UNUSED_VOUCHER_ALLOCATION, {
+      value: allocation,
+      enumerable: false,
+    })
+  }
+  return error
+}
+
+export function getUnusedVoucherAllocation(error: unknown): UnusedVoucherAllocation | null {
+  if (!(error instanceof Error)) return null
+  return (
+    error as Error & { [UNUSED_VOUCHER_ALLOCATION]?: UnusedVoucherAllocation }
+  )[UNUSED_VOUCHER_ALLOCATION] ?? null
+}
+
 // ============================================================================
 // Type guard
 // ============================================================================