fix: stop dropping VAT on MCP inbox-converted supplier invoices (#896)
gnubok_create_supplier_invoice_from_inbox sourced the invoice's header vat_amount from the OCR-extracted totals.vat field instead of summing the per-line vat_amount values. That header field is never reconciled with the line items, so per-line VAT customization (or a mis-extracted document total) could leave it at a stale or zero value. createSupplierInvoiceRegistrationEntry (and the cash/privately-paid variants) then gated the entire 2641 ingående moms posting on invoice.vat_amount > 0, so a stale header silently suppressed a correct per-line VAT split with no error. Confirmed via the ledger: this exact defect hit Glesys 623884, DNB 9664449205, and ComputerSalg 500265968 (all already manually corrected via storno+correction). Fix: derive vat_amount from summed lineItems in the MCP tool, and switch the three registration-entry gates from the header field to itemsHaveVat(items), so the engine itself can no longer be fooled by an unreconciled aggregate regardless of which caller populates it. Signed-off-by: Jonas Flodén <jonas@floden.nu> Co-authored-by: Jakob Wennberg <jakob.wennberg@gmail.com>
This commit is contained in:
co-authored by
Jakob Wennberg
parent
a1fad3193e
commit
31b244acf5
@@ -15,3 +15,4 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-07-05] PR #894 bot triage: accepted the delete-after-update reorder (destructive op last) and the manual-filing warning in confirm_unapprove_agi; declined soft-cancel status for unfiled AGI drafts and preserving approved_by on recall — a never-filed generated AGI is regenerable working data derived entirely from retained run data (not räkenskapsinformation; unapprove 409s once anything is filed), and the approval with legal weight is the one in force at booking, which unapprove can never touch (paid/booked runs are locked out).
|
||||
[2026-07-06] Migration 20260706100000 adds profiles.deleted_at/anonymized_at (ADD COLUMN IF NOT EXISTS) alongside committing anonymize_user_account verbatim: the prod function writes those columns but no repo migration ever created them, so without the columns the drift capture would ship a function that fails on every from-scratch database (CI replay, self-hosted). No-op on prod.
|
||||
[2026-07-06] v1 reconciliation run: confidence_threshold has NO server-side default when omitted (existing API consumers keep current behavior; only the unattended enable-banking sync callers pass DEFAULT_UNATTENDED_CONFIDENCE_THRESHOLD=0.9); registry pitfalls recommend 0.9 to integrators. Revisit if telemetry shows API callers auto-applying fuzzy matches.
|
||||
[2026-07-05] Fixed supplier-invoice VAT silently dropped via MCP inbox conversion: gnubok_create_supplier_invoice_from_inbox now derives vat_amount from summed lineItems instead of the unreconciled OCR totals.vat field, and createSupplierInvoiceRegistrationEntry/CashEntry/PrivatelyPaidEntry gate the 2641 posting on itemsHaveVat(items) instead of invoice.vat_amount > 0. Chose to fix both the immediate source (server.ts) and the downstream gate (supplier-invoice-entries.ts) rather than just one: the header field is inherently a redundant, independently-sourced aggregate that can drift again from a different call site in the future, so the engine itself should never trust it as a gate.
|
||||
|
||||
@@ -203,6 +203,41 @@ describe('gnubok_create_supplier_invoice_from_inbox: execute', () => {
|
||||
expect(result.preview.total).toBe(1250)
|
||||
})
|
||||
|
||||
it('derives vat_amount from summed line items, not the OCR totals block (regression: per-line VAT customization silently dropped VAT)', async () => {
|
||||
// Reported bug: totals.vat is an independent OCR-extracted field that was
|
||||
// never reconciled with lineItems. A stale/mis-extracted document total
|
||||
// (or a per-line VAT customization that didn't also fix up totals.vat)
|
||||
// used to flow straight through as the staged vat_amount, which becomes
|
||||
// the invoice header field that gates the WHOLE 2641 posting downstream
|
||||
// in createSupplierInvoiceRegistrationEntry. Deriving it from the items
|
||||
// instead keeps the header honest regardless of what the header block said.
|
||||
const staleTotals = {
|
||||
...baseExtracted,
|
||||
totals: { subtotal: 1000, vat: 0, total: 1000 }, // stale: doesn't match the line below
|
||||
lineItems: [
|
||||
{ description: 'Konsulttimmar', quantity: 10, unit_price: 100, line_total: 1000, vat_rate: 25, vat_amount: 250 },
|
||||
],
|
||||
}
|
||||
const supabase = makeMock({
|
||||
inbox: {
|
||||
id: 'inbox-10',
|
||||
status: 'received',
|
||||
extracted_data: staleTotals,
|
||||
matched_supplier_id: 'supplier-1',
|
||||
created_supplier_invoice_id: null,
|
||||
document_id: 'doc-10',
|
||||
},
|
||||
})
|
||||
const tool = tools.find((t) => t.name === 'gnubok_create_supplier_invoice_from_inbox')!
|
||||
const result = (await tool.execute(
|
||||
{ inbox_item_id: 'inbox-10', dry_run: true },
|
||||
'company-1', 'user-1', supabase,
|
||||
)) as { preview: { vat_amount: number } }
|
||||
|
||||
// Must reflect the item's real VAT (250), not the stale header total (0).
|
||||
expect(result.preview.vat_amount).toBe(250)
|
||||
})
|
||||
|
||||
it('falls through to org_number lookup when no matched supplier', async () => {
|
||||
const supabase = makeMock({
|
||||
inbox: {
|
||||
|
||||
@@ -7771,7 +7771,6 @@ export const tools: McpTool[] = [
|
||||
|
||||
const total = Number(totalsExt?.total) || 0
|
||||
const subtotal = Number(totalsExt?.subtotal) || 0
|
||||
const vatAmount = Number(totalsExt?.vat) || 0
|
||||
|
||||
// VAT treatment: explicit override wins, else heuristic from extracted data
|
||||
const vatTreatment = (args.vat_treatment_override as string | undefined)
|
||||
@@ -7831,6 +7830,16 @@ export const tools: McpTool[] = [
|
||||
}
|
||||
})
|
||||
|
||||
// Derive from the actual per-line VAT rather than trusting
|
||||
// totalsExt.vat: that header figure comes straight from OCR/agent-
|
||||
// supplied extracted_data and is never reconciled against lineItems.
|
||||
// Per-line VAT customization (edited rate/amount on a line without
|
||||
// also fixing up the document totals) used to leave this at a stale
|
||||
// or zero value, and createSupplierInvoiceRegistrationEntry gates the
|
||||
// whole 2641 posting on invoice.vat_amount > 0: a stale header meant
|
||||
// the correct per-line VAT was silently never booked.
|
||||
const vatAmount = lineItems.reduce((sum, li) => sum + li.vat_amount, 0)
|
||||
|
||||
const params = {
|
||||
inbox_item_id: inboxItemId,
|
||||
supplier_id: supplierId,
|
||||
|
||||
@@ -250,6 +250,47 @@ describe('createSupplierInvoiceRegistrationEntry', () => {
|
||||
assertBalanced(input)
|
||||
})
|
||||
|
||||
it('posts 2641 from the items even when invoice.vat_amount is stale/zero (regression: MCP inbox-conversion header/line mismatch)', async () => {
|
||||
// Reported bug: gnubok_create_supplier_invoice_from_inbox sourced the
|
||||
// header vat_amount from the OCR-extracted document totals (totalsExt.vat)
|
||||
// instead of summing the line items. Customizing per-line VAT rates (or a
|
||||
// mis-extracted document total) left invoice.vat_amount at 0 while the
|
||||
// items still carried correct non-zero vat_rate/vat_amount. The engine
|
||||
// used to gate the whole VAT branch on `invoice.vat_amount > 0`, so a
|
||||
// stale header silently suppressed a correct per-line VAT posting (e.g.
|
||||
// Glesys 623884: 1 001 kr booked with zero VAT instead of 800.54 + 200.14).
|
||||
// The fix drives the gate off the items themselves (itemsHaveVat), so the
|
||||
// header field can never again suppress a correct posting.
|
||||
const invoice = makeSupplierInvoice({
|
||||
subtotal: 800.54,
|
||||
vat_amount: 0, // stale header: never reconciled against the customized line
|
||||
total: 1001,
|
||||
})
|
||||
const items = [
|
||||
makeItem({ line_total: 800.54, account_number: '6540', vat_rate: 0.25, vat_amount: 200.14 }),
|
||||
]
|
||||
|
||||
await createSupplierInvoiceRegistrationEntry(
|
||||
null as never, 'company-1', 'user-1', invoice, items, 'swedish_business'
|
||||
)
|
||||
|
||||
const input = mockedCreateEntry.mock.calls[0][3]
|
||||
|
||||
const debit6540 = findByAccount(input.lines, '6540')
|
||||
expect(debit6540[0].debit_amount).toBe(800.54)
|
||||
|
||||
const debit2641 = findByAccount(input.lines, '2641')
|
||||
expect(debit2641).toHaveLength(1)
|
||||
expect(debit2641[0].debit_amount).toBe(200.14)
|
||||
|
||||
// 2440 must reflect the TRUE gross total (800.54 + 200.14), not the stale
|
||||
// header total: the engine derives it as totalDebits - totalCredits.
|
||||
const credit2440 = findByAccount(input.lines, '2440')
|
||||
expect(credit2440[0].credit_amount).toBe(1000.68)
|
||||
|
||||
assertBalanced(input)
|
||||
})
|
||||
|
||||
it('aggregates manual VAT overrides per rate group on mixed-rate invoice', async () => {
|
||||
// Restaurangkvitto med två olika momsöverskridningar pga representation-
|
||||
// tak och egen avrundning. 25%-raden får manuell 100 kr, 12%-raden får
|
||||
|
||||
@@ -172,7 +172,7 @@ export async function createSupplierInvoiceRegistrationEntry(
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if (invoice.vat_amount > 0) {
|
||||
} else if (itemsHaveVat(items)) {
|
||||
// Domestic standard: Debit ingående moms per rate group
|
||||
const vatByRate = groupVatByRate(items, invoice.currency, invoice.exchange_rate)
|
||||
for (const [rate, amount] of vatByRate) {
|
||||
@@ -430,7 +430,7 @@ export async function createSupplierInvoiceCashEntry(
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if (invoice.vat_amount > 0) {
|
||||
} else if (itemsHaveVat(items)) {
|
||||
// Domestic standard: Debit ingående moms per rate group (at the payment-
|
||||
// date rate when settling a foreign invoice, see effectiveRate above).
|
||||
const vatByRate = groupVatByRate(items, invoice.currency, effectiveRate)
|
||||
@@ -544,7 +544,7 @@ export async function createSupplierInvoicePrivatelyPaidEntry(
|
||||
}
|
||||
|
||||
// Debit: Ingående moms per rate group (mixed-rate kvitto support)
|
||||
if (invoice.vat_amount > 0) {
|
||||
if (itemsHaveVat(items)) {
|
||||
const vatByRate = groupVatByRate(items, invoice.currency, invoice.exchange_rate)
|
||||
for (const [rate, amount] of vatByRate) {
|
||||
if (amount > 0) {
|
||||
@@ -751,6 +751,23 @@ export async function createSupplierCreditNoteEntry(
|
||||
* Reverse-charge fiktiv moms doesn't use this: see groupBaseByRate, which
|
||||
* derives the basis directly so fiktiv VAT is always base × statutory rate.
|
||||
*/
|
||||
/**
|
||||
* True if any item would produce a non-zero ingående moms line: mirrors the
|
||||
* same stored-vs-computed fallback groupVatByRate uses (stored vat_amount,
|
||||
* else line_total × rate). Callers must gate the VAT branch on this instead
|
||||
* of invoice.vat_amount: that header field can come from a source (e.g. the
|
||||
* MCP inbox-conversion tool's OCR-extracted totals) that is never
|
||||
* reconciled against the items, so a stale or zero header would otherwise
|
||||
* silently suppress a correct per-line VAT posting.
|
||||
*/
|
||||
function itemsHaveVat(items: SupplierInvoiceItem[]): boolean {
|
||||
return items.some((item) => {
|
||||
if ((item.vat_amount ?? 0) > 0) return true
|
||||
const rate = item.vat_rate ?? 0.25
|
||||
return rate > 0 && (item.line_total ?? 0) > 0
|
||||
})
|
||||
}
|
||||
|
||||
function groupVatByRate(
|
||||
items: SupplierInvoiceItem[],
|
||||
currency: string,
|
||||
|
||||
Reference in New Issue
Block a user