From 5de18e609019f0d6bccbfb40b67e155f245e5e22 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Tue, 10 Nov 2026 17:02:36 +0800 Subject: [PATCH] fix(ui-tool): localize and validate compact image output --- apps/web/tests/cua-run-code.e2e.ts | 2 +- .../src/client/tool/models/tool-call-model.ts | 25 ++++--- .../client/tool/toolviews/GenericToolCard.tsx | 9 ++- .../ui-tool/tests/tool-row.client.spec.tsx | 65 +++++++++++++++++-- snapshots/web/cua-run-code/ui.expected.md | 2 +- 5 files changed, 83 insertions(+), 20 deletions(-) diff --git a/apps/web/tests/cua-run-code.e2e.ts b/apps/web/tests/cua-run-code.e2e.ts index 0c8fa805d2..272f418272 100644 --- a/apps/web/tests/cua-run-code.e2e.ts +++ b/apps/web/tests/cua-run-code.e2e.ts @@ -82,7 +82,7 @@ it.skipIf(mode === 'record')('shows standard CUA programs and their recorded sou expect(events.filter(event => event.type === 'tool/ptc-dispatch' && event.data.content.some(block => block.type === 'image'))).toHaveLength(1) expect(events.filter(event => event.type === 'tool/result' && event.data.message.content.some(block => block.type === 'image'))).toHaveLength(1) - expect(await calls.first().textContent()).toContain('(1×1, 90 B)') + expect(await calls.first().textContent()).toContain('(1x1px, 90 bytes)') expect(await calls.first().textContent()).not.toContain('"type": "image"') await compareOrRefreshGolden(join(snapshotDirectory, 'ui.expected.md'), await captureStableAria(page, '[class*="centerCol"]', scaffold.workspaceCwd), mode) diff --git a/packages/client/ui-tool/src/client/tool/models/tool-call-model.ts b/packages/client/ui-tool/src/client/tool/models/tool-call-model.ts index c9c0ef4644..9f81138d3b 100644 --- a/packages/client/ui-tool/src/client/tool/models/tool-call-model.ts +++ b/packages/client/ui-tool/src/client/tool/models/tool-call-model.ts @@ -203,25 +203,26 @@ function deriveAutoReviewDenial(block: ToolCallBlock): AutoReviewDenial | null { /** * Flatten a settled result's content blocks to display text: text blocks - * verbatim, image references with dimensions and bytes, and other blocks as - * pretty JSON. Empty failure content falls back to the structured error's + * verbatim, images through the supplied formatter, and other blocks + * as pretty JSON. Empty failure content falls back to the structured error's * `name: code` line. * @param node - the settled result node. + * @param formatImage - Optional formatter for unvalidated image attachments; omission or undefined preserves image JSON. * @returns the flattened result text (may be empty). */ -export function resultText(node: ToolResultNode): string { +export function resultText(node: ToolResultNode, formatImage?: (image: unknown) => string | undefined): string { const parts: string[] = [] for (const block of node.content) { if (block.type === 'text') parts.push(block.text) - else if (block.type === 'image') { - const image = block.attachment - parts.push(`${image.name ?? image.attachmentId} (${image.width}×${image.height}, ${image.bytes} B)`) - } else parts.push(JSON.stringify(block, null, 2)) + else { + const image = block.type === 'image' ? formatImage?.(block.attachment) : undefined + parts.push(image ?? JSON.stringify(block, null, 2)) + } } if (parts.length === 0 && node.error !== undefined) { parts.push(`${node.error.name}: ${node.error.code}`) } - return parts.join('\n') + return parts.reduce((text, part, index) => text + (index === 0 || text.endsWith('\n') ? '' : '\n') + part, '') } function parseArgs(argsRaw: string): unknown { @@ -372,9 +373,13 @@ export function formatToolBody(variant: ToolRowVariant, argsRaw: string): string * @param block - preparing call, dispatched call, or result from the snapshot. * @param cwd - session workspace root; workspace-rooted path summaries display relative to it. * @param home - host account home; a leftover POSIX home path displays as `~`. + * @param formatImage - Optional image formatter; omission preserves image JSON. * @returns the row model. */ -export function toolRowModel(toolName: string, block: ToolCallBlock, cwd?: string, home?: string): ToolRowModel { +export function toolRowModel( + toolName: string, block: ToolCallBlock, cwd?: string, home?: string, + formatImage?: (image: unknown) => string | undefined, +): ToolRowModel { const variant = classifyTool(toolName) const titleKey = toolTitleKey(toolName) const done = 'kind' in block @@ -408,7 +413,7 @@ export function toolRowModel(toolName: string, block: ToolCallBlock, cwd?: strin // The empty string is "no text" for both derived result fields: a settled // call with blank content has nothing to expand, and a blank first line // would erase the collapsed error row's summary slot. - const output = done ? (resultText(block) || null) : null + const output = done ? (resultText(block, formatImage) || null) : null const errorSummary = state === 'error' && output !== null ? firstLine(output) : null const bodyRaw = argsRaw === '' ? null : argsRaw return { diff --git a/packages/client/ui-tool/src/client/tool/toolviews/GenericToolCard.tsx b/packages/client/ui-tool/src/client/tool/toolviews/GenericToolCard.tsx index 6ce48b990f..f5cf72d95d 100644 --- a/packages/client/ui-tool/src/client/tool/toolviews/GenericToolCard.tsx +++ b/packages/client/ui-tool/src/client/tool/toolviews/GenericToolCard.tsx @@ -36,7 +36,14 @@ export type GenericToolCardProps = ToolCallOwnerProps & { export function GenericToolCard({ toolName, block, cwd, home, openFile, getFileTriggerProps, inspect, useDisclosure, t, }: GenericToolCardProps) { - const model = toolRowModel(toolName, block, cwd, home) + const model = toolRowModel(toolName, block, cwd, home, (image) => { + if (typeof image !== 'object' || image === null || Array.isArray(image)) return undefined + const { attachmentId, name, width, height, bytes } = image as Record + if (typeof attachmentId !== 'string' || attachmentId === '' + || name !== undefined && typeof name !== 'string' + || ![width, height, bytes].every(value => typeof value === 'number' && Number.isInteger(value) && value > 0)) return undefined + return `${name ?? attachmentId} (${t('row.imageSize', { width, height, bytes })})` + }) const autoReview = model.autoReviewDenial === null ? null : localizeAutoReviewDenial(model.autoReviewDenial, t) diff --git a/packages/client/ui-tool/tests/tool-row.client.spec.tsx b/packages/client/ui-tool/tests/tool-row.client.spec.tsx index 54dbc685d3..a65f0f4179 100644 --- a/packages/client/ui-tool/tests/tool-row.client.spec.tsx +++ b/packages/client/ui-tool/tests/tool-row.client.spec.tsx @@ -15,7 +15,7 @@ import { } from '../src/client/tool/models/tool-call-model.ts' import { ToolRow } from '../src/client/tool/components/ToolRow.tsx' import { GenericToolCard, type GenericToolCardProps } from '../src/client/tool/toolviews/GenericToolCard.tsx' -import { zh } from '@deepseek-ai/dsh-client-ui-conversation/src/client/locales.ts' +import { en, zh } from '@deepseek-ai/dsh-client-ui-conversation/src/client/locales.ts' import { PartialArguments } from '@deepseek-ai/dsh-util-values' afterEach(() => { @@ -239,13 +239,10 @@ describe('tool-call-model', () => { .toBe('{\n "code": ""\n}') }) - it('resultText retains text, describes images, and uses the structured failure when content is empty', () => { + it('resultText retains text and image JSON and uses the structured failure when content is empty', () => { expect(resultText(result({ content: [{ type: 'text', text: 'a\nb' }] }))).toBe('a\nb') - const attachment = { attachmentId: AttachmentId('sha256:image'), mediaType: 'image/png' as const, width: 8, height: 4, bytes: 64 } - expect(resultText(result({ content: [{ type: 'text', text: 'a' }, { type: 'image', attachment }] }))) - .toBe('a\nsha256:image (8×4, 64 B)') - expect(resultText(result({ content: [{ type: 'image', attachment: { ...attachment, name: 'shot.png' } }] }))) - .toBe('shot.png (8×4, 64 B)') + const image = { type: 'image' as const, attachment: { attachmentId: AttachmentId('sha256:image'), mediaType: 'image/png' as const, width: 8, height: 4, bytes: 64 } } + expect(resultText(result({ content: [image] }))).toBe(JSON.stringify(image, null, 2)) const reasoning = { type: 'reasoning' as const, text: 'detail' } expect(resultText(result({ content: [reasoning] }))).toBe(JSON.stringify(reasoning, null, 2)) expect(resultText(result({ content: [], isError: true, error: { name: 'ToolError', code: 'denied' } }))) @@ -253,12 +250,39 @@ describe('tool-call-model', () => { expect(resultText(result({ content: [] }))).toBe('') }) + it('preserves image JSON when the formatter declines', () => { + for (const content of ['[{"type":"image"}]', '[{"type":"image","attachment":null}]', + '[{"type":"image","attachment":{"width":"8"}}]']) { + const blocks = JSON.parse(content) as ToolResultNode['content'] + expect(resultText(result({ content: blocks }), () => undefined)).toBe(JSON.stringify(blocks[0], null, 2)) + } + }) + + it('separates image references without duplicating an emitted newline', () => { + const attachment = { attachmentId: AttachmentId('sha256:image'), mediaType: 'image/png' as const, width: 8, height: 4, bytes: 64 } + for (const text of ['a', 'a\n']) { + expect(resultText(result({ content: [{ type: 'text', text }, { type: 'image', attachment }] }), () => 'sha256:image')) + .toBe('a\nsha256:image') + } + expect(resultText(result({ content: [{ type: 'text', text: 'a\n\n' }, { type: 'image', attachment }] }), () => 'sha256:image')) + .toBe('a\n\nsha256:image') + }) + it('derives output from the settled result and null while running or blank', () => { expect(toolRowModel('bash', result({ content: [{ type: 'text', text: 'out' }] })).output).toBe('out') expect(toolRowModel('bash', running()).output).toBeNull() expect(toolRowModel('bash', result({ content: [] })).output).toBeNull() }) + it.each([false, true])('shares formatted image output with its error summary, failure=%s', (isError) => { + const image = { type: 'image' as const, attachment: { attachmentId: AttachmentId('sha256:image'), mediaType: 'image/png' as const, width: 8, height: 4, bytes: 64 } } + const formatImage = vi.fn(() => 'selected image') + const model = toolRowModel('run_code', result({ content: [image], isError }), undefined, undefined, formatImage) + expect(model.output).toBe('selected image') + expect(model.errorSummary).toBe(isError ? 'selected image' : null) + expect(formatImage).toHaveBeenCalledOnce() + }) + it('derives errorSummary as the first output line on error rows only', () => { const failed = result({ content: [{ type: 'text', text: 'boom\ndetail' }], isError: true }) expect(toolRowModel('bash', failed).errorSummary).toBe('boom') @@ -597,6 +621,33 @@ describe('GenericToolCard', () => { useDisclosure, callId: 'c1', toolName, ...('kind' in block ? { phase: 'result' as const, block: block } : { phase: block.phase, block: block }), openFile: vi.fn(), t, }) + it.each([ + [t, 'sha256:image (8x4px，64 字节)'], + [makeTranslate(en), 'sha256:image (8x4px, 64 bytes)'], + ] as const)('localizes compact image references in the generic card', (translate, expected) => { + const attachment = { attachmentId: AttachmentId('sha256:image'), mediaType: 'image/png' as const, width: 8, height: 4, bytes: 64 } + const block = result({ content: [{ type: 'text', text: 'before\n' }, { type: 'image', attachment }] }) + const view = render() + fireEvent.click(view.getByRole('button')) + expect(view.container.textContent).toContain(expected) + expect(view.container.querySelector('[class*="output"]')?.textContent ?? view.container.textContent).not.toContain('before\n\nsha256:') + }) + + it.each([ + '{"type":"image"}', + '{"type":"image","attachment":null}', + '{"type":"image","attachment":[]}', + '{"type":"image","attachment":{"attachmentId":"image","width":"8","height":4,"bytes":64}}', + '{"type":"image","attachment":{"attachmentId":"image","width":8,"height":4,"bytes":0}}', + '{"type":"image","attachment":{"attachmentId":"image","name":12,"width":8,"height":4,"bytes":64}}', + '{"type":"image","attachment":{"width":8,"height":4,"bytes":64}}', + ])('renders malformed image references as JSON: %s', (json) => { + const content = JSON.parse(`[${json}]`) as ToolResultNode['content'] + const view = render() + fireEvent.click(view.getByRole('button')) + expect(view.container.textContent).toContain(JSON.stringify(content[0], null, 2)) + }) + it('renders the classified variant row from the frozen slice', () => { const view = render( ({})} />) diff --git a/snapshots/web/cua-run-code/ui.expected.md b/snapshots/web/cua-run-code/ui.expected.md index a3c1608087..cb5494a073 100644 --- a/snapshots/web/cua-run-code/ui.expected.md +++ b/snapshots/web/cua-run-code/ui.expected.md @@ -20,7 +20,7 @@ - button "Wrap lines" [pressed] - button "Copy" - code: "globalThis.visits = 1; var app = await cua.getApp(\"test.fixture\"); var win = await app.getWindow(7); await win.getAccessibilityTree({emit:false}); await win.click(\"0\"); var screenshot = await win.getScreenshot(); inspect(JSON.stringify({ visits, bindings: Object.keys(tools), app: app.appHandle, window: win.windowId })); inspect(screenshot)" -- text: "OUT {\"visits\":1,\"bindings\":[\"cua\"],\"app\":\"fixture-app\",\"window\":7} sha256:999f1d1527ee7e79266f16add5430fff76b1225d742464a5b1ff1f02971bb8ee (1×1, 90 B)" +- text: "OUT {\"visits\":1,\"bindings\":[\"cua\"],\"app\":\"fixture-app\",\"window\":7} sha256:999f1d1527ee7e79266f16add5430fff76b1225d742464a5b1ff1f02971bb8ee (1x1px, 90 bytes)" - button "Inspect" - button "Tool call cua · getCapabilities" - button "Tool call cua · getApp"