refactor(pending): one pending-op-owned preview for chat, /pending and flow views (#1537)

* refactor(pending): one pending-op-owned preview for chat, /pending and flow views A staged pending_operation was rendered three separate ways: the /pending page's OperationPreview switch (8 specialized renderers keyed on operation_type), ApprovalCard's own PreviewBlock (near-duplicate renderers keyed on 4 hardcoded MCP tool names), and AgentChat's toolNameFor() hack that mapped stored operation_types onto 'gnubok_'-prefixed tool names on hydration. This is the weakest seam ahead of flow-run views (plan seam 8.3): every new operation type had to be taught to render in two places and silently degraded in the third. Now there is one owner: - components/pending-operations/OperationPreview.tsx: the /pending renderers moved verbatim, dispatched on operation_type, consumed by /pending, ApprovalCard and future flow-run views. - components/pending-operations/vocabulary.ts: operation labels, single-action warnings and the one canonical rejection-category list (ApprovalCard's copy was byte-identical and is deleted). - lib/pending-operations/tool-name.ts: the single translation point between bare operation_types and 'gnubok_' tool names, with tests. toolNameFor gotcha fixed on the way: ApprovalCard's old dispatch only recognized 4 tool names, so a hydrated card for any other operation type (attach_document_to_transaction, match_transaction_invoice, ...) silently fell back to a raw generic preview. Hydration now passes the stored operation_type straight through attachStagedOperations to the card, and live streamed cards derive it from the event's tool name, so every operation type keeps its specialized preview on resume. Per-surface chrome (list row on /pending vs inline chat card) is deliberately kept: only the preview + vocabulary were the duplicated seam. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: drop a stray hunt_title copy rename that rode along 'Kvittojakten' -> 'Leta efter underlag' in messages/sv.json was uncommitted working-tree state from another session, swept into the extraction commit by git add breadth. It is a product-naming call with no en.json counterpart and does not belong in this refactor; preserved in this branch's first commit if it turns out to be wanted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(pending): carry params to chat previews; guard preview amounts CodeRabbit round on #1537, both real. (1) AttachDocumentPreview renders its DocumentViewButton from params.document_id, which neither chat path carried: the staged_operation stream event now includes the tool-use input (the same values the staging tool stored as pending_operations.params) and hydration selects the params column, so an attach-document card in chat shows its evidence button live and on resume. (2) InvoicePreview and CreateTransactionPreview cast amounts straight into formatCurrency; a payload without one rendered 'NaN kr'. They now share the same show-the-gap guard the legacy summary already had. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-13 16:17:15 +02:00
parent 22eaab82e5
commit ce6efdb3dc
12 changed files with 794 additions and 929 deletions
@@ -77,6 +77,10 @@ export type StreamEvent =
      kind: 'staged_operation'
      tool_use_id: string
      tool_name: string
+      // The tool-use input: the same values the staging tool stored as
+      // pending_operations.params. Carried so chat previews that need
+      // params (e.g. attach_document's DocumentViewButton) work live.
+      params: Record<string, unknown>
      staged: StagedOperationResult
    }
  | {
@@ -450,6 +454,7 @@ export async function runChatTurn(args: RunTurnArgs): Promise<void> {
            kind: 'staged_operation',
            tool_use_id: tu.id,
            tool_name: tu.name,
+            params: tu.input as Record<string, unknown>,
            staged: result,
          })
        }
@@ -0,0 +1,67 @@
+import { describe, it, expect } from 'vitest'
+import {
+  operationTypeFromToolName,
+  toolNameForOperationType,
+} from '../tool-name'
+
+/**
+ * pending_operations.operation_type is the bare action name; the MCP layer
+ * and the live chat stream carry 'gnubok_'-prefixed tool names. Getting the
+ * mapping wrong is invisible in mocks but silently drops every hydrated
+ * approval card to the generic raw preview (the toolNameFor gotcha this
+ * module replaces), so the round-trip is pinned here.
+ */
+
+describe('operationTypeFromToolName', () => {
+  it('strips the gnubok_ prefix from an MCP tool name', () => {
+    expect(operationTypeFromToolName('gnubok_categorize_transaction')).toBe(
+      'categorize_transaction',
+    )
+    expect(operationTypeFromToolName('gnubok_attach_document_to_transaction')).toBe(
+      'attach_document_to_transaction',
+    )
+  })
+
+  it('passes an already-bare operation_type through unchanged', () => {
+    expect(operationTypeFromToolName('categorize_transaction')).toBe(
+      'categorize_transaction',
+    )
+  })
+
+  it('passes a non-gnubok tool name through unchanged', () => {
+    expect(operationTypeFromToolName('some_vendor_tool')).toBe('some_vendor_tool')
+    expect(operationTypeFromToolName('remember_fact')).toBe('remember_fact')
+  })
+})
+
+describe('toolNameForOperationType', () => {
+  it('prefixes a bare operation_type', () => {
+    expect(toolNameForOperationType('create_voucher')).toBe('gnubok_create_voucher')
+  })
+
+  it('never double-prefixes an already-prefixed name', () => {
+    expect(toolNameForOperationType('gnubok_create_voucher')).toBe(
+      'gnubok_create_voucher',
+    )
+  })
+})
+
+describe('round-trip', () => {
+  // Every operation type with a specialized preview renderer must survive
+  // the trip in both directions: this is exactly the path a hydrated
+  // approval card's preview dispatch takes.
+  const opTypes = [
+    'categorize_transaction',
+    'create_customer',
+    'create_invoice',
+    'create_transaction',
+    'create_voucher',
+    'correct_entry',
+    'attach_document_to_transaction',
+    'match_transaction_invoice',
+  ]
+
+  it.each(opTypes)('%s -> tool name -> back', (opType) => {
+    expect(operationTypeFromToolName(toolNameForOperationType(opType))).toBe(opType)
+  })
+})
@@ -0,0 +1,29 @@
+// pending_operations.operation_type stores the bare action name
+// ('categorize_transaction'), while the MCP tool surface and the live chat
+// stream carry the prefixed tool name ('gnubok_categorize_transaction').
+// These two functions are the single translation point between the two
+// vocabularies: every surface that needs to cross over imports from here
+// instead of hand-rolling a prefix. The 'gnubok_' wire prefix is deliberate
+// and must stay (rebrand rule: wire-format identifiers keep the old name).
+
+const TOOL_NAME_PREFIX = 'gnubok_'
+
+/**
+ * MCP tool name -> bare operation_type. A name without the prefix is already
+ * a bare operation_type (or a non-gnubok tool) and passes through unchanged.
+ */
+export function operationTypeFromToolName(toolName: string): string {
+  return toolName.startsWith(TOOL_NAME_PREFIX)
+    ? toolName.slice(TOOL_NAME_PREFIX.length)
+    : toolName
+}
+
+/**
+ * Bare operation_type -> MCP tool name. Defensive on already-prefixed input
+ * so a value that was a tool name all along is never double-prefixed.
+ */
+export function toolNameForOperationType(operationType: string): string {
+  return operationType.startsWith(TOOL_NAME_PREFIX)
+    ? operationType
+    : `${TOOL_NAME_PREFIX}${operationType}`
+}