feat(mcp): unify missing-document surfaces on one predicate (#876)
* feat(mcp): unify missing-document surfaces on one predicate (P1-3) The two MCP surfaces told different truths: the transactions tool keyed 'has underlag' on transactions.document_id while the verifikat tool keyed on document_attachments — and neither respected the source-type semantics, version chains, or journal_entry_no_doc_required waivers that lib/worklist's canonical count applies. Measured on prod: 22,046 waived verifikat still listed to agents, 2,370 doc-exempt source types listed, ~87 docs attached to transactions but never propagated to the verifikat, 1,100 transactions flagged missing-receipt although their verifikat HAS the underlag. One predicate now lives in SQL — posted, needs-doc source type (mirrors NEEDS_DOC_SOURCE_TYPES), no current-version doc, no waiver: - verifikat_without_documents RPC v2 adopts the canonical predicate. - New transactions_without_documents RPC: the bank-driven subset of the same predicate, joined through transactions.journal_entry_id — a strict subset of the verifikat surface by construction. Rows expose qualified transaction_id (P1-2 forward-compat); bare id deprecated. - Both tools become thin RPC wrappers; descriptions state the actual set relationship. - lib/worklist countVerifikatMissingDocument delegates to the RPC (previously three full-table pulls set-differenced client-side) — badge count and agent surfaces can no longer drift. - Backfill: propagate transaction-attached docs to their verifikat where the attachment was never linked (open periods only; never steals a doc linked to another verifikat). pg-real: fixture matrix (no-doc/with-doc/waived/stale-version/ doc-exempt-source/import), strict-subset assertion, per-source-type pin of the SQL list against the TS constant, tenant guard. Part of dev_docs/mcp_optimization_plan.md (P1-3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(mcp): explicit grants restated + count-call comment (#876 review) - Restate REVOKE/GRANT on verifikat_without_documents so the migration is self-contained (CREATE OR REPLACE preserves the 20260703130000 grants — verified on prod: authenticated + service_role only). - Comment on the p_limit:1 count call: total_count is computed over the full filtered set, independent of page size. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Fable 5
Claude Fable 5
parent
250cc7c450
commit
21512db81a
+75
-95
@@ -2,149 +2,129 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||
import { createQueuedMockSupabase } from '@/tests/helpers'
|
||||
import { tools } from '../server'
|
||||
|
||||
/**
|
||||
* The tool is a thin wrapper over the transactions_without_documents RPC —
|
||||
* the bank-driven subset of the verifikat surface, keyed on the SAME document
|
||||
* truth (document_attachments + waivers), never transactions.document_id.
|
||||
* Predicate semantics are pinned by
|
||||
* tests/pg/document-surfaces-unification.pg.test.ts; these tests cover the
|
||||
* wrapper contract (envelope unwrap, pagination math, error paths).
|
||||
*/
|
||||
const tool = tools.find((t) => t.name === 'gnubok_list_transactions_without_documents')!
|
||||
|
||||
function envelope(transactions: unknown[], totalCount: number) {
|
||||
return { data: { ok: true, total_count: totalCount, transactions }, error: null }
|
||||
}
|
||||
|
||||
const row = (id: string, jeId: string) => ({
|
||||
id,
|
||||
transaction_id: id,
|
||||
date: '2026-04-12',
|
||||
description: 'HOTELL ANGLAIS',
|
||||
amount: -1247,
|
||||
currency: 'SEK',
|
||||
merchant_name: null,
|
||||
reference: null,
|
||||
is_business: null,
|
||||
category: null,
|
||||
journal_entry_id: jeId,
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('gnubok_list_transactions_without_documents', () => {
|
||||
it('is registered as a read-only paginated tool', () => {
|
||||
it('is registered as a read-only paginated tool with qualified transaction_id', () => {
|
||||
expect(tool).toBeDefined()
|
||||
expect(tool.annotations?.readOnlyHint).toBe(true)
|
||||
const schema = tool.outputSchema as Record<string, unknown>
|
||||
expect((schema.properties as Record<string, unknown>).transactions).toBeDefined()
|
||||
expect((schema.properties as Record<string, unknown>).total_count).toBeDefined()
|
||||
const schema = tool.outputSchema as { properties: Record<string, unknown> }
|
||||
expect(schema.properties.transactions).toBeDefined()
|
||||
expect(schema.properties.total_count).toBeDefined()
|
||||
const items = (schema.properties.transactions as { items: { properties: Record<string, unknown> } })
|
||||
.items
|
||||
expect(items.properties.transaction_id).toBeDefined()
|
||||
expect(items.properties.journal_entry_id).toBeDefined()
|
||||
})
|
||||
|
||||
it('returns booked transactions that have no document attached', async () => {
|
||||
const rows = [
|
||||
{
|
||||
id: 't1',
|
||||
date: '2026-04-12',
|
||||
description: 'HOTELL ANGLAIS',
|
||||
amount: -1247,
|
||||
currency: 'SEK',
|
||||
merchant_name: 'Hotell Anglais',
|
||||
reference: null,
|
||||
is_business: true,
|
||||
category: 'travel',
|
||||
journal_entry_id: 'je-1',
|
||||
},
|
||||
]
|
||||
it('unwraps the RPC envelope and passes filters through', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: null, error: null, count: 1 }) // count query
|
||||
enqueue({ data: rows, error: null }) // data query
|
||||
enqueue(envelope([row('t1', 'je-1')], 1))
|
||||
|
||||
const result = (await tool.execute(
|
||||
{ limit: 20 },
|
||||
{ limit: 20, since: '2026-01-01' },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never
|
||||
supabase as never,
|
||||
)) as {
|
||||
transactions: typeof rows
|
||||
transactions: Array<{ id: string; transaction_id: string; journal_entry_id: string }>
|
||||
count: number
|
||||
total_count: number
|
||||
has_more: boolean
|
||||
}
|
||||
|
||||
expect(supabase.rpc).toHaveBeenCalledWith('transactions_without_documents', {
|
||||
p_company_id: 'company-1',
|
||||
p_since: '2026-01-01',
|
||||
p_limit: 20,
|
||||
p_offset: 0,
|
||||
})
|
||||
expect(result.count).toBe(1)
|
||||
expect(result.total_count).toBe(1)
|
||||
expect(result.has_more).toBe(false)
|
||||
expect(result.transactions[0].id).toBe('t1')
|
||||
expect(result.transactions[0].transaction_id).toBe('t1')
|
||||
expect(result.transactions[0].journal_entry_id).toBe('je-1')
|
||||
})
|
||||
|
||||
it('returns rows when DB has null merchant_name, reference, is_business, category (MCP structured output)', async () => {
|
||||
const rows = [
|
||||
{
|
||||
id: 't-no-doc-1',
|
||||
date: '2026-03-17',
|
||||
description: 'Nolla skuld',
|
||||
amount: -2745,
|
||||
currency: 'SEK',
|
||||
merchant_name: null,
|
||||
reference: null,
|
||||
is_business: null,
|
||||
category: null,
|
||||
journal_entry_id: 'je-nolla-1',
|
||||
},
|
||||
]
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: null, error: null, count: 1 })
|
||||
enqueue({ data: rows, error: null })
|
||||
|
||||
const result = (await tool.execute(
|
||||
{ limit: 20 },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never
|
||||
)) as {
|
||||
transactions: typeof rows
|
||||
count: number
|
||||
total_count: number
|
||||
has_more: boolean
|
||||
}
|
||||
|
||||
expect(result.count).toBe(1)
|
||||
expect(result.transactions[0].journal_entry_id).toBe('je-nolla-1')
|
||||
expect(result.transactions[0].merchant_name).toBeNull()
|
||||
expect(result.transactions[0].reference).toBeNull()
|
||||
expect(result.transactions[0].is_business).toBeNull()
|
||||
expect(result.transactions[0].category).toBeNull()
|
||||
})
|
||||
|
||||
it('returns empty result when nothing matches', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: null, error: null, count: 0 })
|
||||
enqueue({ data: [], error: null })
|
||||
enqueue(envelope([], 0))
|
||||
|
||||
const result = (await tool.execute(
|
||||
{},
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never
|
||||
)) as { count: number; total_count: number; has_more: boolean }
|
||||
const result = (await tool.execute({}, 'company-1', 'user-1', supabase as never)) as {
|
||||
count: number
|
||||
total_count: number
|
||||
has_more: boolean
|
||||
}
|
||||
|
||||
expect(result.count).toBe(0)
|
||||
expect(result.total_count).toBe(0)
|
||||
expect(result.has_more).toBe(false)
|
||||
})
|
||||
|
||||
it('signals more pages with next_offset when total exceeds the page', async () => {
|
||||
const page = Array.from({ length: 20 }, (_, i) => ({
|
||||
id: `t${i}`,
|
||||
date: '2026-04-01',
|
||||
description: 'tx',
|
||||
amount: -100,
|
||||
currency: 'SEK',
|
||||
merchant_name: null,
|
||||
reference: null,
|
||||
is_business: true,
|
||||
category: null,
|
||||
journal_entry_id: `je-${i}`,
|
||||
}))
|
||||
it('signals more pages with next_offset advancing by rows consumed', async () => {
|
||||
const page = Array.from({ length: 20 }, (_, i) => row(`t${i}`, `je-${i}`))
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: null, error: null, count: 50 })
|
||||
enqueue({ data: page, error: null })
|
||||
enqueue(envelope(page, 45))
|
||||
|
||||
const result = (await tool.execute(
|
||||
{ limit: 20, offset: 0 },
|
||||
{ limit: 20 },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase as never
|
||||
)) as { has_more: boolean; next_offset?: number }
|
||||
supabase as never,
|
||||
)) as {
|
||||
has_more: boolean
|
||||
next_offset?: number
|
||||
}
|
||||
|
||||
expect(result.has_more).toBe(true)
|
||||
expect(result.next_offset).toBe(20)
|
||||
})
|
||||
|
||||
it('throws on database errors', async () => {
|
||||
it('throws on an RPC error', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: null, error: { message: 'connection refused' }, count: null })
|
||||
enqueue({ data: null, error: { message: 'connection refused' } })
|
||||
|
||||
await expect(
|
||||
tool.execute({}, 'company-1', 'user-1', supabase as never)
|
||||
).rejects.toThrow(/connection refused/)
|
||||
await expect(tool.execute({}, 'company-1', 'user-1', supabase as never)).rejects.toThrow(
|
||||
/connection refused/,
|
||||
)
|
||||
})
|
||||
|
||||
it('throws on a not-ok envelope (tenant guard)', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { ok: false, code: 'TRANSACTIONS_WITHOUT_DOCUMENTS_FORBIDDEN' }, error: null })
|
||||
|
||||
await expect(tool.execute({}, 'company-1', 'user-1', supabase as never)).rejects.toThrow(
|
||||
/TRANSACTIONS_WITHOUT_DOCUMENTS_FORBIDDEN/,
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2663,7 +2663,7 @@ export const tools: McpTool[] = [
|
||||
{
|
||||
name: 'gnubok_list_transactions_without_documents',
|
||||
title: 'List Transactions Missing Receipts',
|
||||
description: 'List BANK TRANSACTIONS booked without an attached underlag. For imported/manual verifikat (no bank tx row) call gnubok_list_verifikat_without_documents — this tool only covers bank-driven entries.',
|
||||
description: 'List booked bank transactions whose verifikat lacks an underlag. Strict subset of gnubok_list_verifikat_without_documents (same document truth, waivers respected) — use that tool for full coverage incl. imported/manual verifikat.',
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
additionalProperties: false,
|
||||
@@ -2677,7 +2677,8 @@ export const tools: McpTool[] = [
|
||||
type: 'object',
|
||||
additionalProperties: false,
|
||||
properties: {
|
||||
id: { type: 'string' },
|
||||
id: { type: 'string', description: 'Deprecated — read transaction_id instead' },
|
||||
transaction_id: { type: 'string' },
|
||||
date: { type: 'string' },
|
||||
description: { type: 'string' },
|
||||
amount: { type: 'number' },
|
||||
@@ -2700,42 +2701,39 @@ export const tools: McpTool[] = [
|
||||
const offset = Math.max(0, Number(args.offset) || 0)
|
||||
const since = typeof args.since === 'string' ? args.since : null
|
||||
|
||||
let countQuery = supabase
|
||||
.from('transactions')
|
||||
.select('id', { count: 'exact', head: true })
|
||||
.eq('company_id', companyId)
|
||||
.not('journal_entry_id', 'is', null)
|
||||
.is('document_id', null)
|
||||
if (since) countQuery = countQuery.gte('date', since)
|
||||
|
||||
const { count: totalCount, error: countError } = await countQuery
|
||||
if (countError) throw new Error(`Database error: ${countError.message}`)
|
||||
|
||||
let dataQuery = supabase
|
||||
.from('transactions')
|
||||
.select(
|
||||
'id, date, description, amount, currency, merchant_name, reference, is_business, category, journal_entry_id'
|
||||
)
|
||||
.eq('company_id', companyId)
|
||||
.not('journal_entry_id', 'is', null)
|
||||
.is('document_id', null)
|
||||
if (since) dataQuery = dataQuery.gte('date', since)
|
||||
|
||||
const { data, error } = await dataQuery
|
||||
.order('date', { ascending: false })
|
||||
.range(offset, offset + limit - 1)
|
||||
|
||||
// Same document truth as the verifikat surface: the RPC keys "has
|
||||
// underlag" on document_attachments (current version) + waivers, never
|
||||
// transactions.document_id — the two columns diverged historically
|
||||
// (P1-3, dev_docs/mcp_optimization_plan.md) and this surface is the
|
||||
// bank-driven SUBSET of gnubok_list_verifikat_without_documents by
|
||||
// construction.
|
||||
const { data, error } = await supabase.rpc('transactions_without_documents', {
|
||||
p_company_id: companyId,
|
||||
p_since: since,
|
||||
p_limit: limit,
|
||||
p_offset: offset,
|
||||
})
|
||||
if (error) throw new Error(`Database error: ${error.message}`)
|
||||
|
||||
const total = totalCount ?? 0
|
||||
const hasMore = total > offset + (data?.length ?? 0)
|
||||
const result = data as {
|
||||
ok: boolean
|
||||
code?: string
|
||||
total_count?: number
|
||||
transactions?: unknown[]
|
||||
} | null
|
||||
if (!result?.ok) {
|
||||
throw new Error(`transactions_without_documents failed: ${result?.code ?? 'unknown error'}`)
|
||||
}
|
||||
|
||||
const rows = result.transactions ?? []
|
||||
const total = result.total_count ?? 0
|
||||
const hasMore = offset + rows.length < total
|
||||
return {
|
||||
transactions: data,
|
||||
count: data?.length ?? 0,
|
||||
transactions: rows,
|
||||
count: rows.length,
|
||||
total_count: total,
|
||||
has_more: hasMore,
|
||||
...(hasMore ? { next_offset: offset + (data?.length ?? 0) } : {}),
|
||||
...(hasMore ? { next_offset: offset + rows.length } : {}),
|
||||
}
|
||||
},
|
||||
},
|
||||
@@ -2743,7 +2741,7 @@ export const tools: McpTool[] = [
|
||||
{
|
||||
name: 'gnubok_list_verifikat_without_documents',
|
||||
title: 'List Verifikat Missing Documents',
|
||||
description: 'List POSTED journal entries (verifikat) that have no document_attachments row. Covers SIE-imported, manual and salary vouchers that the transactions-based tool misses. Newest first, paginated.',
|
||||
description: 'List posted verifikat that genuinely lack an underlag: needs-doc source types only, current document versions, user waivers respected. Superset of gnubok_list_transactions_without_documents (covers imported/manual too). Newest first, paginated.',
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
additionalProperties: false,
|
||||
|
||||
Reference in New Issue
Block a user