fix(mcp): SQL-side filtering and pagination for list_verifikat_without_documents (#872)

The tool applied min_amount in memory after the PostgREST .range() page:
total_count ignored the filter, and next_offset advanced by the filtered
row count while the DB page consumed 'limit' rows — consecutive pages
overlapped and full backlog coverage could not be proven (reported via
agent.feedback).

gross_amount is an aggregate over journal_entry_lines that PostgREST
cannot filter on, so filtering/counting/pagination move into a new
verifikat_without_documents RPC (SECURITY DEFINER with the PR #625
tenant-guard pattern; id tiebreak for total ordering; filter-respecting
total_count). Also indexes document_attachments.journal_entry_id, which
the anti-join and the link tools both hit.

pg-real invariants: disjoint + complete pages under min_amount,
filter-respecting totals, since filter, doc/draft exclusion, tenant
guard both ways.

Part of dev_docs/mcp_optimization_plan.md (P0-2).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

extensions/general/mcp-server/server.ts

@@ -2777,62 +2777,42 @@ export const tools: McpTool[] = [
       const offset = Math.max(0, Number(args.offset) || 0)
       const since = typeof args.since === 'string' ? args.since : null
       const minAmount = typeof args.min_amount === 'number' && Number.isFinite(args.min_amount)
-        ? args.min_amount
+        ? Math.max(0, args.min_amount)
         : 0
 
-      // PostgREST left-join filter: journal_entries left-joined to
-      // document_attachments and filtered to rows where the join produced
-      // no document. The filter syntax `document_attachments.id=is.null`
-      // applies the predicate post-join (Supabase: foreign-table is-null).
-      let query = supabase
-        .from('journal_entries')
-        .select(
-          'id, voucher_series, voucher_number, entry_date, description, source_type, document_attachments!left(id), journal_entry_lines(debit_amount)',
-          { count: 'exact' },
-        )
-        .eq('company_id', companyId)
-        .eq('status', 'posted')
-        .is('document_attachments.id', null)
-      if (since) query = query.gte('entry_date', since)
-
-      const { data, error, count } = await query
-        .order('entry_date', { ascending: false })
-        .order('voucher_number', { ascending: false })
-        .range(offset, offset + limit - 1)
+      // gross_amount is an aggregate over journal_entry_lines, which PostgREST
+      // cannot filter on — filtering it in memory after .range() made
+      // total_count ignore min_amount and consecutive pages overlap. The RPC
+      // filters, counts and paginates in SQL so the total respects the filter
+      // and next_offset advances by exactly the rows consumed.
+      const { data, error } = await supabase.rpc('verifikat_without_documents', {
+        p_company_id: companyId,
+        p_since: since,
+        p_min_amount: minAmount,
+        p_limit: limit,
+        p_offset: offset,
+      })
       if (error) throw new Error(`Database error: ${error.message}`)
 
-      const rows = ((data ?? []) as Array<{
-        id: string
-        voucher_series: string
-        voucher_number: number
-        entry_date: string
-        description: string
-        source_type: string
-        journal_entry_lines: { debit_amount: number | string }[] | null
-      }>).map((e) => {
-        const lines = e.journal_entry_lines ?? []
-        const gross = lines.reduce((sum, l) => sum + (Number(l.debit_amount) || 0), 0)
-        return {
-          journal_entry_id: e.id,
-          voucher_series: e.voucher_series,
-          voucher_number: e.voucher_number,
-          entry_date: e.entry_date,
-          description: e.description,
-          source_type: e.source_type,
-          gross_amount: Math.round(gross * 100) / 100,
-        }
-      })
+      const result = data as {
+        ok: boolean
+        code?: string
+        total_count?: number
+        verifikat?: unknown[]
+      } | null
+      if (!result?.ok) {
+        throw new Error(`verifikat_without_documents failed: ${result?.code ?? 'unknown error'}`)
+      }
 
-      const filtered = minAmount > 0 ? rows.filter((r) => r.gross_amount >= minAmount) : rows
-
-      const total = count ?? 0
-      const hasMore = total > offset + filtered.length
+      const rows = result.verifikat ?? []
+      const total = result.total_count ?? 0
+      const hasMore = offset + rows.length < total
       return {
-        verifikat: filtered,
-        count: filtered.length,
+        verifikat: rows,
+        count: rows.length,
         total_count: total,
         has_more: hasMore,
-        ...(hasMore ? { next_offset: offset + filtered.length } : {}),
+        ...(hasMore ? { next_offset: offset + rows.length } : {}),
       }
     },
   },

supabase/migrations/20260703130000_verifikat_without_documents_rpc.sql

@@ -0,0 +1,116 @@
+-- RPC: verifikat_without_documents — SQL-side filtering + pagination for the
+-- MCP tool gnubok_list_verifikat_without_documents.
+--
+-- Bug fix (dev_docs/mcp_optimization_plan.md P0-2): the tool applied
+-- min_amount IN MEMORY after the PostgREST .range() page, because
+-- gross_amount is an aggregate (sum of debit lines) PostgREST cannot filter
+-- on. Consequences: total_count ignored the filter, and next_offset advanced
+-- by the filtered row count while the DB page consumed `limit` rows — so the
+-- next page overlapped the previous page's tail. Agents paging a backlog got
+-- duplicates and could not prove full coverage.
+--
+-- This function computes gross_amount, applies since/min_amount, counts and
+-- paginates all in SQL, returning a filter-respecting total alongside the
+-- page. Ordering carries an id tiebreak so pagination is total and stable.
+--
+-- Tenant guard (mirrors 20260615120000): anon/authenticated callers must be
+-- members of p_company_id; service_role bypasses (MCP tools already scope by
+-- company_id).
+--
+-- pg-test: tests/pg/verifikat-without-documents-rpc.pg.test.ts (pagination
+-- invariants: disjoint pages, complete union, filter-respecting total, since
+-- filter, tenant guard).
+
+CREATE OR REPLACE FUNCTION public.verifikat_without_documents(
+  p_company_id uuid,
+  p_since date DEFAULT NULL,
+  p_min_amount numeric DEFAULT 0,
+  p_limit integer DEFAULT 20,
+  p_offset integer DEFAULT 0
+)
+RETURNS jsonb
+LANGUAGE plpgsql
+STABLE
+SECURITY DEFINER
+SET search_path TO 'public'
+AS $$
+DECLARE
+  v_jwt_role text := coalesce(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'role', '');
+  v_limit integer := least(greatest(coalesce(p_limit, 20), 1), 100);
+  v_offset integer := greatest(coalesce(p_offset, 0), 0);
+  v_min numeric := greatest(coalesce(p_min_amount, 0), 0);
+  v_result jsonb;
+BEGIN
+  IF v_jwt_role IN ('anon', 'authenticated') THEN
+    -- NOT IN (subquery) evaluates to UNKNOWN when either side yields NULL,
+    -- which would silently skip this deny branch — use NOT EXISTS and reject
+    -- a NULL company id outright.
+    IF p_company_id IS NULL OR NOT EXISTS (
+      SELECT 1 FROM public.user_company_ids() AS c(id) WHERE c.id = p_company_id
+    ) THEN
+      RETURN jsonb_build_object('ok', false, 'code', 'VERIFIKAT_WITHOUT_DOCUMENTS_FORBIDDEN');
+    END IF;
+  END IF;
+
+  WITH candidates AS (
+    SELECT
+      je.id,
+      je.voucher_series,
+      je.voucher_number,
+      je.entry_date,
+      je.description,
+      je.source_type,
+      round(coalesce(sum(l.debit_amount), 0), 2) AS gross_amount
+    FROM journal_entries je
+    LEFT JOIN journal_entry_lines l ON l.journal_entry_id = je.id
+    WHERE je.company_id = p_company_id
+      AND je.status = 'posted'
+      AND NOT EXISTS (
+        SELECT 1 FROM document_attachments d WHERE d.journal_entry_id = je.id
+      )
+      AND (p_since IS NULL OR je.entry_date >= p_since)
+    GROUP BY je.id
+    HAVING round(coalesce(sum(l.debit_amount), 0), 2) >= v_min
+  ),
+  total AS (
+    SELECT count(*) AS n FROM candidates
+  ),
+  page AS (
+    SELECT * FROM candidates
+    ORDER BY entry_date DESC, voucher_number DESC, id DESC
+    LIMIT v_limit OFFSET v_offset
+  )
+  SELECT jsonb_build_object(
+    'ok', true,
+    'total_count', (SELECT n FROM total),
+    'verifikat', coalesce(
+      (SELECT jsonb_agg(
+         jsonb_build_object(
+           'journal_entry_id', p.id,
+           'voucher_series', p.voucher_series,
+           'voucher_number', p.voucher_number,
+           'entry_date', p.entry_date,
+           'description', p.description,
+           'source_type', p.source_type,
+           'gross_amount', p.gross_amount
+         )
+         ORDER BY p.entry_date DESC, p.voucher_number DESC, p.id DESC
+       ) FROM page p),
+      '[]'::jsonb
+    )
+  )
+  INTO v_result;
+
+  RETURN v_result;
+END;
+$$;
+
+REVOKE ALL ON FUNCTION public.verifikat_without_documents(uuid, date, numeric, integer, integer) FROM PUBLIC, anon;
+GRANT EXECUTE ON FUNCTION public.verifikat_without_documents(uuid, date, numeric, integer, integer) TO authenticated, service_role;
+
+-- Anti-join + link lookups both hit document_attachments by journal_entry_id;
+-- the FK carries no index by default.
+CREATE INDEX IF NOT EXISTS idx_document_attachments_journal_entry_id
+  ON public.document_attachments (journal_entry_id);
+
+NOTIFY pgrst, 'reload schema';

tests/pg/verifikat-without-documents-rpc.pg.test.ts

@@ -0,0 +1,212 @@
+import { randomUUID } from 'crypto'
+import { beforeAll, describe, expect, it } from 'vitest'
+import { getPool, withUserContext } from './setup'
+import {
+  seedCompany,
+  insertAuthUser,
+  insertDraftJournalEntry,
+  insertBalancedLines,
+} from './fixtures'
+
+/**
+ * Invariants for the verifikat_without_documents RPC (mcp_optimization_plan
+ * P0-2). The predecessor applied min_amount in memory after the DB page:
+ * total_count ignored the filter and consecutive pages overlapped. These
+ * tests pin the SQL-side behavior: filter-respecting total, disjoint and
+ * complete pages, since filter, doc/draft exclusion, tenant guard.
+ */
+
+type RpcRow = {
+  journal_entry_id: string
+  voucher_number: number
+  gross_amount: number
+}
+type RpcResult = {
+  ok: boolean
+  code?: string
+  total_count?: number
+  verifikat?: RpcRow[]
+}
+
+async function callRpc(params: {
+  companyId: string
+  since?: string | null
+  minAmount?: number
+  limit?: number
+  offset?: number
+}): Promise<RpcResult> {
+  const { rows } = await getPool().query<{ result: RpcResult }>(
+    `SELECT public.verifikat_without_documents($1, $2, $3, $4, $5) AS result`,
+    [
+      params.companyId,
+      params.since ?? null,
+      params.minAmount ?? 0,
+      params.limit ?? 20,
+      params.offset ?? 0,
+    ],
+  )
+  return rows[0].result
+}
+
+async function attachDocument(params: {
+  userId: string
+  companyId: string
+  journalEntryId: string
+}): Promise<void> {
+  await getPool().query(
+    `INSERT INTO public.document_attachments
+       (id, user_id, company_id, journal_entry_id, file_name, mime_type,
+        file_size_bytes, storage_path, sha256_hash, upload_source)
+     VALUES ($1, $2, $3, $4, 'underlag.pdf', 'application/pdf', 1024, $5, $6, 'file_upload')`,
+    [
+      randomUUID(),
+      params.userId,
+      params.companyId,
+      params.journalEntryId,
+      `documents/${params.companyId}/underlag.pdf`,
+      randomUUID().replace(/-/g, '').padEnd(64, '0'),
+    ],
+  )
+}
+
+describe('verifikat_without_documents RPC', () => {
+  let userId: string
+  let companyId: string
+  let fiscalPeriodId: string
+  // voucher_number → { id, amount }; amounts chosen so min_amount splits the set
+  const seeded = new Map<number, { id: string; amount: number }>()
+
+  beforeAll(async () => {
+    const s = await seedCompany()
+    userId = s.userId
+    companyId = s.companyId
+    fiscalPeriodId = s.fiscalPeriodId
+
+    // 6 posted entries without documents: amounts 100..600, dates ascending
+    for (let i = 1; i <= 6; i++) {
+      const id = await insertDraftJournalEntry({
+        userId,
+        companyId,
+        fiscalPeriodId,
+        status: 'posted',
+        voucherNumber: i,
+        entryDate: `2026-06-0${i}`,
+        description: `no-doc ${i}`,
+      })
+      await insertBalancedLines(id, i * 100)
+      seeded.set(i, { id, amount: i * 100 })
+    }
+
+    // Posted entry WITH a document — must never appear
+    const withDoc = await insertDraftJournalEntry({
+      userId,
+      companyId,
+      fiscalPeriodId,
+      status: 'posted',
+      voucherNumber: 7,
+      entryDate: '2026-06-07',
+      description: 'has doc',
+    })
+    await insertBalancedLines(withDoc, 700)
+    await attachDocument({ userId, companyId, journalEntryId: withDoc })
+
+    // Draft entry — must never appear
+    const draft = await insertDraftJournalEntry({
+      userId,
+      companyId,
+      fiscalPeriodId,
+      status: 'draft',
+      voucherNumber: 8,
+      entryDate: '2026-06-08',
+      description: 'draft',
+    })
+    await insertBalancedLines(draft, 800)
+  })
+
+  it('lists posted no-doc entries only, newest first, with gross amounts', async () => {
+    const res = await callRpc({ companyId })
+    expect(res.ok).toBe(true)
+    expect(res.total_count).toBe(6)
+    const rows = res.verifikat ?? []
+    expect(rows.map((r) => r.voucher_number)).toEqual([6, 5, 4, 3, 2, 1])
+    expect(rows[0].gross_amount).toBe(600)
+  })
+
+  it('total_count respects min_amount', async () => {
+    const res = await callRpc({ companyId, minAmount: 350 })
+    expect(res.ok).toBe(true)
+    // amounts 400, 500, 600 pass the filter
+    expect(res.total_count).toBe(3)
+    expect((res.verifikat ?? []).map((r) => r.voucher_number)).toEqual([6, 5, 4])
+  })
+
+  it('paginates a filtered set with disjoint, complete pages', async () => {
+    const collected: string[] = []
+    let offset = 0
+    for (;;) {
+      const res = await callRpc({ companyId, minAmount: 250, limit: 2, offset })
+      expect(res.ok).toBe(true)
+      expect(res.total_count).toBe(4) // amounts 300..600
+      const rows = res.verifikat ?? []
+      if (rows.length === 0) break
+      collected.push(...rows.map((r) => r.journal_entry_id))
+      offset += rows.length
+      if (offset >= (res.total_count ?? 0)) break
+    }
+    // No duplicates (the old in-memory filter overlapped pages) …
+    expect(new Set(collected).size).toBe(collected.length)
+    // … and no gaps: exactly the 4 entries ≥ 250.
+    const expected = [3, 4, 5, 6].map((n) => seeded.get(n)!.id).sort()
+    expect([...collected].sort()).toEqual(expected)
+  })
+
+  it('respects the since filter', async () => {
+    const res = await callRpc({ companyId, since: '2026-06-04' })
+    expect(res.ok).toBe(true)
+    expect(res.total_count).toBe(3)
+    expect((res.verifikat ?? []).map((r) => r.voucher_number)).toEqual([6, 5, 4])
+  })
+
+  it('offset past the end returns an empty page with a truthful total', async () => {
+    const res = await callRpc({ companyId, minAmount: 250, limit: 2, offset: 10 })
+    expect(res.ok).toBe(true)
+    expect(res.total_count).toBe(4)
+    expect(res.verifikat).toEqual([])
+  })
+
+  it('blocks an authenticated caller from another tenant', async () => {
+    const strangerId = await insertAuthUser()
+    const res = await withUserContext(strangerId, async (client) => {
+      const { rows } = await client.query<{ result: RpcResult }>(
+        `SELECT public.verifikat_without_documents($1, NULL, 0, 20, 0) AS result`,
+        [companyId],
+      )
+      return rows[0].result
+    })
+    expect(res.ok).toBe(false)
+    expect(res.code).toBe('VERIFIKAT_WITHOUT_DOCUMENTS_FORBIDDEN')
+  })
+
+  it('rejects a NULL company id for authenticated callers (NOT IN → UNKNOWN bypass)', async () => {
+    const res = await withUserContext(userId, async (client) => {
+      const { rows } = await client.query<{ result: RpcResult }>(
+        `SELECT public.verifikat_without_documents(NULL, NULL, 0, 20, 0) AS result`,
+      )
+      return rows[0].result
+    })
+    expect(res.ok).toBe(false)
+    expect(res.code).toBe('VERIFIKAT_WITHOUT_DOCUMENTS_FORBIDDEN')
+  })
+
+  it('allows an authenticated member of the company', async () => {
+    const res = await withUserContext(userId, async (client) => {
+      const { rows } = await client.query<{ result: RpcResult }>(
+        `SELECT public.verifikat_without_documents($1, NULL, 0, 20, 0) AS result`,
+        [companyId],
+      )
+      return rows[0].result
+    })
+    expect(res.ok).toBe(true)
+    expect(res.total_count).toBe(6)
+  })
+})