From 8a1c7f0714fcb3d887d6140da3256f30435432a4 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Thu, 7 May 2026 16:26:11 +0200 Subject: [PATCH] feat(invoice-inbox): loading state + review fixes + env-driven config (#416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(invoice-inbox): make Bedrock model + max_tokens env-overridable Read BEDROCK_MODEL_ID and BEDROCK_MAX_TOKENS from process.env so ops can swap models or raise token caps without a code deploy. Defaults align with production: eu.anthropic.claude-sonnet-4-6 in eu-north-1, 8192 tokens (up from 4096 — enough headroom for 20+ line-item invoices that were still occasionally truncating mid-JSON). Co-Authored-By: Claude Opus 4.7 (1M context) * fix(invoice-inbox): guard onFieldsUpdated, tighten env parsing, hoist afterAll Three fixes from Greptile review on #416: 1. Stale-closure bug in onFieldsUpdated. setSelected used an unconditional spread, so a PATCH that completed after the user had navigated to a different item would silently overwrite the new selection's extracted_data with the old item's payload. Guard with prev?.id === selected.id (capturing selected.id outside the updater so it's stable through both setSelected and setItems). 2. BEDROCK_MAX_TOKENS env parser used `Number(...) || 8192`, which silently treats a deliberate `0` the same as unset. Use Number.isFinite(n) && n > 0 so an invalid configuration is not masked by the fallback. 3. Hoist `afterAll` import in the test file to the top alongside other vitest imports. Was working via ES-module hoisting but reads weird to humans and linters. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../general/InvoiceInboxWorkspace.tsx | 12 ++++++++++-- .../__tests__/extract-invoice-fields.test.ts | 5 +---- .../lib/extract-invoice-fields.ts | 18 ++++++++++++------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/components/extensions/general/InvoiceInboxWorkspace.tsx b/components/extensions/general/InvoiceInboxWorkspace.tsx index 25b91bdd..aa0d1bec 100644 --- a/components/extensions/general/InvoiceInboxWorkspace.tsx +++ b/components/extensions/general/InvoiceInboxWorkspace.tsx @@ -452,10 +452,18 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) { onAttach={() => setAttachOpen(true)} isDeleting={isDeleting} onFieldsUpdated={(nextData) => { - setSelected((prev) => (prev ? { ...prev, extracted_data: nextData } : prev)) + // Guard against stale closure: if the user navigated to a + // different item between sending the PATCH and the response + // arriving, the captured `selected` is no longer the + // currently-selected one. Without the id check we'd write + // item A's payload onto item B's row. + const targetId = selected.id + setSelected((prev) => + prev?.id === targetId ? { ...prev, extracted_data: nextData } : prev + ) setItems((prev) => prev.map((it) => - it.id === selected.id ? { ...it, extracted_data: nextData } : it + it.id === targetId ? { ...it, extracted_data: nextData } : it ) ) }} diff --git a/extensions/general/invoice-inbox/__tests__/extract-invoice-fields.test.ts b/extensions/general/invoice-inbox/__tests__/extract-invoice-fields.test.ts index 3e3b043d..5e70a12f 100644 --- a/extensions/general/invoice-inbox/__tests__/extract-invoice-fields.test.ts +++ b/extensions/general/invoice-inbox/__tests__/extract-invoice-fields.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest' import { extractInvoiceFields } from '@/extensions/general/invoice-inbox/lib/extract-invoice-fields' // Mock the Bedrock SDK so tests drive the JSON parser without @@ -190,6 +190,3 @@ describe('extractInvoiceFields', () => { else delete process.env.AWS_SECRET_ACCESS_KEY }) }) - -// vitest doesn't auto-import afterAll -import { afterAll } from 'vitest' diff --git a/extensions/general/invoice-inbox/lib/extract-invoice-fields.ts b/extensions/general/invoice-inbox/lib/extract-invoice-fields.ts index d63ed50c..db51bfb1 100644 --- a/extensions/general/invoice-inbox/lib/extract-invoice-fields.ts +++ b/extensions/general/invoice-inbox/lib/extract-invoice-fields.ts @@ -17,12 +17,18 @@ import { createLogger } from '@/lib/logger' const log = createLogger('invoice-inbox-extract') -const MODEL = 'eu.anthropic.claude-sonnet-4-6' -// 4096 covers a 10-15 line invoice plus VAT breakdown comfortably. The JSON -// skeleton alone is ~200 tokens; 1500 left only ~1300 for content and -// silently truncated complex documents (response cut mid-JSON → -// JSON.parse throws → row lands with all-null fields). -const MAX_TOKENS = 4096 +// Both overridable via env vars so ops can swap models / raise token caps +// without a code deploy. Defaults match what's expected to be set in +// production (eu.anthropic.claude-sonnet-4-6 in eu-north-1, 8192 tokens — +// enough headroom for invoices with 20+ line items). +const MODEL = process.env.BEDROCK_MODEL_ID || 'eu.anthropic.claude-sonnet-4-6' +const MAX_TOKENS = (() => { + const parsed = Number(process.env.BEDROCK_MAX_TOKENS) + // Use the env value only if it's a positive number — `||` would also + // fall back on a deliberate `0`, masking what is really an invalid + // configuration rather than the intent to disable. + return Number.isFinite(parsed) && parsed > 0 ? parsed : 8192 +})() // Bedrock supports these document/image media types directly. HEIC/HEIF // are not on the list, so we skip AI for those — the inbox row still