PR 2 of 2: fix/inbox-retry-extraction-rematch (#1631)

* fix(inbox): match suppliers on VAT number, not just org-nr and name An extracted document auto-links to a supplier by org_number, then by an exact (case-insensitive) name match. The extractor deliberately leaves orgNumber null unless the document carries a real Swedish organisationsnummer, so for every foreign supplier the name was the only key left: "ADOBE SYSTEMS SOFTWARE IRELAND LTD" prints nothing but a momsregistreringsnummer (IE6364992H), which the suppliers table already stores in vat_number and which no code path looked at. Adds vat_number as a match key between org_number and name, and collapses the five inlined copies of the lookup into lib/suppliers/match-supplier.ts: - invoice-inbox upload (sync and deferred worker) - invoice-inbox PUT /items/:id/extracted-data - MCP createDocumentInboxItem (org-nr only until now: gains VAT and name) - MCP gnubok_set_inbox_extracted_data - MCP gnubok_create_supplier_invoice_from_inbox, which additionally read supplierExt.organizationNumber, a key the extraction schema never writes, so its org-nr lookup could not fire at all VAT numbers are compared on a canonical key (uppercased alphanumerics), so formatting variants match and a prefix-less "556012579001" still matches "SE556012579001"; two different country prefixes never do. Also escapes LIKE metacharacters in the name lookup, so a supplier named "100 % Solutions" is no longer a wildcard pattern. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(inbox): re-run the supplier match when the extraction is re-run "Tolka om" (POST /items/:id/retry-extraction) rewrote extracted_data and left matched_supplier_id untouched. It is the one affordance a user reaches for precisely when auto-linking failed, and it could not produce a link no matter how many times they pressed it: the match ran once, at first extraction, and never again. An item that arrived before its supplier existed stayed unlinked forever. The retry now runs the same shared matcher the other extraction paths use. Only a positive match is written, unlike those paths which also write null: a supplier the user picked by hand must survive a retry that finds nothing, which is the likelier case on a document that already failed to match once. Stacked on fix/supplier-match-vat-number for matchSupplierId(); merge that one first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(inbox): write the retry-extraction update as two literal payloads The conditional spread for matched_supplier_id was one more dynamic payload than the phantom-column guard's ceiling allows (380 > 379), and a spread is exactly the shape that guard cannot check. Two inline literal update calls keep every written column checkable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
2026-08-17 15:06:53 +02:00
parent 3841ab9f54
commit 1cd33d9d97
3 changed files with 115 additions and 12 deletions
@@ -1031,6 +1031,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
 [2026-08-16] Row exit animation for dry-table <tr> rows collapses via td padding/line-height/font-size transitions plus a numeric max-height (.row-collapsible) on the fixed-height cell spans, not grid-template-rows 0fr (the AttGoraSection pattern): table cells cannot host the grid wrapper without restructuring every td, and max-height needs a numeric rest value because auto/none does not interpolate. prefers-reduced-motion hides the exiting row instantly (display: none) while the 350ms timer does the state cleanup.
 [2026-08-16] Restyled QuickReviewDialog's inbox-picker trigger to the same full-width dropzone-footer row as TransactionBookingDialog even though it did not share the orphan-button layout: both surfaces come from #1620 and should present the same underlag affordance; the alternative (leaving a small outline button in one dialog and a footer row in the other) would split the visual language of one control. Presentation only, disabled-while-booking kept (PR #1628).
 [2026-08-17] Supplier auto-match now keys on vat_number between org_number and name, in one shared lib/suppliers/match-supplier.ts instead of five inlined copies: extraction deliberately leaves orgNumber null for non-Swedish entities, so momsregistreringsnumret is the only exact key a foreign supplier has and an exact name match was the sole remaining fallback. Normalisation is done in JS over the suppliers that carry a vat_number at all, because PostgREST cannot normalise in SQL; a prefix-vs-no-prefix pair (SE556012579001 vs 556012579001) matches, two different country prefixes never do.
+[2026-08-17] retry-extraction re-runs the supplier match but writes matched_supplier_id only on a positive match, unlike the other extraction paths which write null too: "Tolka om" is the affordance a user reaches for precisely when auto-linking failed, and clobbering a supplier they then picked by hand would be a worse regression than leaving a stale link.
 [2026-08-17] Batch "Bokför valda" ships ONE aggregate toast with an "Ångra alla" action instead of dropping undo from batch toasts: per-row undo is feasible today (each booked row storno-reverses via POST /uncategorize, the same endpoint as the per-row Ångra), so the aggregate action just pools it over every booked row. Silent mode suppresses the per-row success toast and the generic failure toast only; interactive escalations (SI/CI match suggestions, duplicate warning, activate-account) keep their dialogs because they are the only way forward for those rows.
 [2026-08-17] /pending: a FAILED fetch for a tab whose rows are not on screen HOLDS the loading state (spinner + error toast) rather than clearing rows to the empty state: "Inget att granska" after a failed load would be indistinguishable from a genuinely empty list, and BFL-relevant pending work must not look done when it is unknown.
 [2026-08-17] Full-archive direct download resurfaced as an ImportRow + small centered dialog on /import's Exportera tab (row "Komplett arkiv", hash #full-archive), not by re-mounting the orphaned components/settings/BackupDownloadForm.tsx: the form was pre-frame card styling with a duplicate cloud-backup section, while the export tab's existing SIE dialog sets the house pattern (ImportRow -> sm:max-w-md dialog). Its logic (estimate, 413 handling, last-download stamp) ported into components/import/FullArchiveDialog.tsx; the orphan and its dead settings_backup_download i18n namespace deleted. The dialog reuses FiscalYearSelector despite design.md's "legacy, no new uses" line: FyPicker is a toolbar context chip, and the SIE dialog in the same file already uses FiscalYearSelector for the identical dialog-form slot, so matching it beats introducing a third pattern. Row gated to owner/admin because GET /api/reports/full-archive enforces that role server-side; showing members a download that can only 403 helps nobody.
@@ -163,6 +163,7 @@ describe('POST /items/:id/retry-extraction', () => {
      data: { storage_path: 'path/to.pdf', mime_type: 'application/pdf', file_name: 'invoice.pdf' },
      error: null,
    })
+    enqueue({ data: null, error: null }) // supplier name lookup: no match
    enqueue({ data: null, error: null }) // inbox update on success

    serviceDownloadMock.mockResolvedValue({
@@ -178,6 +179,80 @@ describe('POST /items/:id/retry-extraction', () => {
    expect(body.data.extracted_data.totals.total).toBe(125)
  })

+  it('re-links the supplier on retry, matching a foreign supplier by VAT number', async () => {
+    // Re-running the extraction used to rewrite extracted_data and leave
+    // matched_supplier_id untouched, so "Tolka om" could never repair a link
+    // that failed the first time (e.g. because the supplier was created after
+    // the document arrived).
+    const { supabase, enqueue, findCall } = createQueuedMockSupabase()
+    enqueue({
+      data: { id: 'item-1', document_id: 'doc-1', correlation_id: null, created_supplier_invoice_id: null },
+      error: null,
+    })
+    enqueue({ data: { is_sandbox: false }, error: null }) // sandbox check
+    enqueue({
+      data: { storage_path: 'path/to.pdf', mime_type: 'application/pdf', file_name: 'invoice.pdf' },
+      error: null,
+    })
+    enqueue({
+      data: [{ id: 'adobe-supplier', vat_number: 'IE6364992H' }],
+      error: null,
+    }) // vat_number scan
+    enqueue({ data: null, error: null }) // inbox update on success
+
+    serviceDownloadMock.mockResolvedValue({
+      data: new Blob([new Uint8Array([1, 2, 3])], { type: 'application/pdf' }),
+      error: null,
+    })
+
+    vi.mocked(extractInvoiceFields).mockResolvedValueOnce({
+      ...EXTRACTION_SUCCESS,
+      data: {
+        ...EXTRACTION_SUCCESS.data,
+        supplier: {
+          ...EXTRACTION_SUCCESS.data.supplier,
+          name: 'Adobe Systems Software Ireland Ltd',
+          vatNumber: 'IE6364992H',
+        },
+      },
+    } as never)
+
+    const res = await retryRoute.handler(makeReq(), buildCtx(supabase))
+    const { status } = await parseJsonResponse(res)
+    expect(status).toBe(200)
+
+    const update = findCall('invoice_inbox_items', 'update')?.[0] as Record<string, unknown>
+    expect(update.matched_supplier_id).toBe('adobe-supplier')
+  })
+
+  it('leaves a hand-picked supplier alone when the retry finds no match', async () => {
+    const { supabase, enqueue, findCall } = createQueuedMockSupabase()
+    enqueue({
+      data: { id: 'item-1', document_id: 'doc-1', correlation_id: null, created_supplier_invoice_id: null },
+      error: null,
+    })
+    enqueue({ data: { is_sandbox: false }, error: null }) // sandbox check
+    enqueue({
+      data: { storage_path: 'path/to.pdf', mime_type: 'application/pdf', file_name: 'invoice.pdf' },
+      error: null,
+    })
+    enqueue({ data: null, error: null }) // supplier name lookup: no match
+    enqueue({ data: null, error: null }) // inbox update on success
+
+    serviceDownloadMock.mockResolvedValue({
+      data: new Blob([new Uint8Array([1, 2, 3])], { type: 'application/pdf' }),
+      error: null,
+    })
+    vi.mocked(extractInvoiceFields).mockResolvedValueOnce(EXTRACTION_SUCCESS as never)
+
+    const res = await retryRoute.handler(makeReq(), buildCtx(supabase))
+    const { status } = await parseJsonResponse(res)
+    expect(status).toBe(200)
+
+    const update = findCall('invoice_inbox_items', 'update')?.[0] as Record<string, unknown>
+    expect(update).not.toHaveProperty('matched_supplier_id')
+  })
+
  it('marks the item as error and returns 500 when extraction throws', async () => {
    const { supabase, enqueue } = createQueuedMockSupabase()
    enqueue({
@@ -1094,18 +1094,45 @@ export const invoiceInboxExtension: Extension = {
            fileName: doc.file_name,
          })

-          const { error: updateError } = await ctx.supabase
-            .from('invoice_inbox_items')
-            .update({
-              status: 'received',
-              error_message: null,
-              extracted_data: extracted as unknown as Record<string, unknown>,
-              // Retry is user-initiated and bypasses the page-count gate by
-              // design: the user explicitly opted into the slow path.
-              extraction_skipped: false,
-            })
-            .eq('id', id)
-            .eq('company_id', ctx.companyId)
+          // Re-running the extraction has to re-run the match too, or the one
+          // affordance the user reaches for when an item failed to auto-link
+          // ("Tolka om") can never produce a link: this path rewrote
+          // extracted_data and left matched_supplier_id untouched. Only a
+          // positive match is written, so a supplier the user picked by hand
+          // survives a retry that finds nothing.
+          const matchedSupplierId = await matchSupplierId(
+            ctx.supabase,
+            ctx.companyId,
+            extracted.supplier,
+          )
+
+          // Two literal payloads instead of one with a conditional spread:
+          // the phantom-column guard can only check columns written as inline
+          // object literals, and matched_supplier_id should be checkable.
+          const { error: updateError } = matchedSupplierId
+            ? await ctx.supabase
+                .from('invoice_inbox_items')
+                .update({
+                  status: 'received',
+                  error_message: null,
+                  extracted_data: extracted as unknown as Record<string, unknown>,
+                  // Retry is user-initiated and bypasses the page-count gate by
+                  // design: the user explicitly opted into the slow path.
+                  extraction_skipped: false,
+                  matched_supplier_id: matchedSupplierId,
+                })
+                .eq('id', id)
+                .eq('company_id', ctx.companyId)
+            : await ctx.supabase
+                .from('invoice_inbox_items')
+                .update({
+                  status: 'received',
+                  error_message: null,
+                  extracted_data: extracted as unknown as Record<string, unknown>,
+                  extraction_skipped: false,
+                })
+                .eq('id', id)
+                .eq('company_id', ctx.companyId)

          if (updateError) {
            return NextResponse.json({ error: updateError.message }, { status: 500 })