diff --git a/mobile/src/session/MobileNativeChatMessage.test.ts b/mobile/src/session/MobileNativeChatMessage.test.ts index bb687b77ba6..f38a437d55c 100644 --- a/mobile/src/session/MobileNativeChatMessage.test.ts +++ b/mobile/src/session/MobileNativeChatMessage.test.ts @@ -1,7 +1,8 @@ import { createElement } from 'react' -import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' +import { MAX_TOOL_RESULT_CHARS } from './mobile-native-chat-message-styles' vi.mock('react-native', async () => { const React = await import('react') @@ -30,7 +31,11 @@ function userMessage(blocks: NativeChatMessage['blocks']): NativeChatMessage { return { id: 'u1', role: 'user', blocks, timestamp: null, source: 'transcript' } } -describe('MobileNativeChatMessage image-ref rendering', () => { +function toolMessage(blocks: NativeChatMessage['blocks']): NativeChatMessage { + return { id: 'a1', role: 'assistant', blocks, timestamp: null, source: 'transcript' } +} + +describe('MobileNativeChatMessage', () => { let renderer: ReactTestRenderer | null = null beforeEach(() => { @@ -41,7 +46,10 @@ describe('MobileNativeChatMessage image-ref rendering', () => { renderer = null }) - function render(message: NativeChatMessage): ReactTestRenderer { + function render( + message: NativeChatMessage, + props: { toolsExpanded?: boolean } = {} + ): ReactTestRenderer { const original = console.error const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { @@ -51,7 +59,7 @@ describe('MobileNativeChatMessage image-ref rendering', () => { }) try { act(() => { - renderer = create(createElement(MobileNativeChatMessage, { message })) + renderer = create(createElement(MobileNativeChatMessage, { message, ...props })) }) } finally { spy.mockRestore() @@ -59,6 +67,9 @@ describe('MobileNativeChatMessage image-ref rendering', () => { return renderer! } + const textIn = (node: ReactTestInstance): string[] => + node.findAllByType('Text' as never).map((text) => String(text.children.join(''))) + it('renders a loadable preview URI as an image thumbnail', () => { const tree = render(userMessage([{ type: 'image-ref', url: 'file:///a.jpg', alt: 'a photo' }])) const image = tree.root.findByType('Image' as never) @@ -84,4 +95,50 @@ describe('MobileNativeChatMessage image-ref rendering', () => { .map((node) => String(node.children.join(''))) expect(texts.some((text) => text.includes('/tmp/host.png'))).toBe(true) }) + + it('labels a tool row with the target path instead of raw input JSON', () => { + const tree = render( + toolMessage([{ type: 'tool-call', name: 'Read', input: { file_path: 'src/index.ts' } }]), + { toolsExpanded: true } + ) + const texts = textIn(tree.root) + expect(texts).toContain('src/index.ts') + expect(texts.some((text) => text.includes('"file_path":"src/index.ts"'))).toBe(false) + }) + + it('bounds expanded diff-less tool input before native text layout', () => { + const tree = render( + toolMessage([ + { type: 'tool-call', name: 'CustomTool', input: { payload: 'x'.repeat(100_000) } } + ]), + { toolsExpanded: true } + ) + const detail = textIn(tree.root).find((text) => text.startsWith('{\n')) + expect(detail).toHaveLength(MAX_TOOL_RESULT_CHARS + 1) + expect(detail?.endsWith('…')).toBe(true) + }) + + it('expands formatted detail for a collapsed JSON-string tool input', () => { + const tree = render( + toolMessage([ + { + type: 'tool-call', + name: 'CustomTool', + input: '{"cmd":"git status","description":"Inspect changes"}' + } + ]) + ) + const pressableWith = (label: string): ReactTestInstance => + tree.root.findAllByType('Pressable' as never).find((node) => textIn(node).includes(label))! + + act(() => pressableWith('1×').props.onPress()) + // The row label is the command, and the detail stays closed until tapped. + expect(textIn(tree.root)).toContain('git status') + expect(textIn(tree.root).some((text) => text.startsWith('{\n'))).toBe(false) + + act(() => pressableWith('CustomTool').props.onPress()) + expect(textIn(tree.root)).toContain( + '{\n "cmd": "git status",\n "description": "Inspect changes"\n}' + ) + }) }) diff --git a/mobile/src/session/MobileNativeChatMessage.tsx b/mobile/src/session/MobileNativeChatMessage.tsx index d8458f1b889..6efd4069959 100644 --- a/mobile/src/session/MobileNativeChatMessage.tsx +++ b/mobile/src/session/MobileNativeChatMessage.tsx @@ -17,7 +17,9 @@ import { isRenderableImageUri } from './mobile-native-chat-image-preview' import { MAX_TOOL_RESULT_CHARS, styles, TEXT_SIZE } from './mobile-native-chat-message-styles' import { nativeChatMessageText } from './mobile-native-chat-message-text' import { - summarizeToolInput, + describeToolInput, + formatToolInput, + isStructuredToolInput, summarizeToolRun, toolFilePath } from './mobile-native-chat-tool-summary' @@ -46,6 +48,12 @@ function DiffView({ lines }: { lines: DiffLine[] }): React.JSX.Element { ) } +/** Cap a mono block at the same size as desktop's tool detail — native text + * layout has no scroll container to absorb a runaway payload. */ +function boundedText(text: string): string { + return text.length > MAX_TOOL_RESULT_CHARS ? `${text.slice(0, MAX_TOOL_RESULT_CHARS)}…` : text +} + /** A single inline tool line — `▸ ToolName preview` — that expands in place to * show the call's diff/input or the result's body. Mirrors the reference design * where tool calls read as flat lines in the conversation, not boxed blocks. */ @@ -63,11 +71,7 @@ function ResultBody({ } return ( - - {output.length > MAX_TOOL_RESULT_CHARS - ? `${output.slice(0, MAX_TOOL_RESULT_CHARS)}…` - : output} - + {boundedText(output)} ) } @@ -88,14 +92,23 @@ function ToolLine({ const [expanded, setExpanded] = useState(defaultExpanded) const { call, result } = pair const name = call ? call.name : 'Result' + // The row label is a human summary (path / primary arg), never raw JSON; the + // expanded detail below carries the full formatted input. const preview = call - ? summarizeToolInput(call.input) + ? describeToolInput(call.input) : (result?.output.split('\n')[0]?.slice(0, 80) ?? '') // Why: collapsed tool rows are the common path; defer bounded diff parsing - // until the user asks to reveal the detail. + // and detail formatting until the user asks to reveal the detail. const callDiff = expanded && call ? diffFromToolCall(call.name, call.input, diffLineLimit) : null const resultDiff = expanded && result ? diffFromText(result.output, diffLineLimit) : null - const hasDetail = callDiff !== null || result !== undefined || preview.length > 40 + // Bound before handing it to native text layout, like the result body. + const callDetail = + expanded && call && !callDiff ? boundedText(formatToolInput(call.input)) : undefined + const hasDetail = + callDiff !== null || + result !== undefined || + preview.length > 40 || + (call !== undefined && isStructuredToolInput(call.input)) // A tool that targets a file (Read/Edit/Write…) renders its preview as a // tappable link that opens the file, independent of the line's expand tap. const filePath = call ? toolFilePath(call.input) : null @@ -127,7 +140,7 @@ function ToolLine({ {expanded ? ( {callDiff ? : null} - {!callDiff && call && preview ? {preview} : null} + {callDetail ? {callDetail} : null} {result ? ( ) : null} diff --git a/mobile/src/session/mobile-native-chat-tool-summary.test.ts b/mobile/src/session/mobile-native-chat-tool-summary.test.ts index d08deec4b31..8e65ccf1958 100644 --- a/mobile/src/session/mobile-native-chat-tool-summary.test.ts +++ b/mobile/src/session/mobile-native-chat-tool-summary.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it, vi } from 'vitest' -import { briefToolArg, summarizeToolInput, toolFilePath } from './mobile-native-chat-tool-summary' +import { + briefToolArg, + describeToolInput, + summarizeToolInput, + toolFilePath +} from './mobile-native-chat-tool-summary' describe('summarizeToolInput', () => { it('passes short strings through, collapsing whitespace', () => { @@ -53,6 +58,13 @@ describe('toolFilePath', () => { }) }) +describe('describeToolInput', () => { + it('is re-exported and labels rows with the path or primary argument', () => { + expect(describeToolInput({ file_path: 'src/a.ts', offset: 3 })).toBe('src/a.ts') + expect(describeToolInput('{"cmd":"git status"}')).toBe('git status') + }) +}) + describe('briefToolArg', () => { it('takes the basename of a forward-slash path', () => { expect(briefToolArg({ path: 'src/session/app.ts' })).toBe('app.ts') diff --git a/mobile/src/session/mobile-native-chat-tool-summary.ts b/mobile/src/session/mobile-native-chat-tool-summary.ts index bc3f7a2f89a..375b6312bcb 100644 --- a/mobile/src/session/mobile-native-chat-tool-summary.ts +++ b/mobile/src/session/mobile-native-chat-tool-summary.ts @@ -1,6 +1,9 @@ export { briefToolArg, countToolCalls, + describeToolInput, + formatToolInput, + isStructuredToolInput, summarizeToolInput, summarizeToolRun, toolFilePath diff --git a/src/shared/native-chat-tool-summary.test.ts b/src/shared/native-chat-tool-summary.test.ts index ef6b385806d..cb710e6e71a 100644 --- a/src/shared/native-chat-tool-summary.test.ts +++ b/src/shared/native-chat-tool-summary.test.ts @@ -1,6 +1,77 @@ import { describe, expect, it } from 'vitest' import type { NativeChatBlock } from './native-chat-types' -import { briefToolArg, summarizeToolInput, summarizeToolRun } from './native-chat-tool-summary' +import { + briefToolArg, + describeToolInput, + formatToolInput, + isStructuredToolInput, + summarizeToolInput, + summarizeToolRun, + toolFilePath +} from './native-chat-tool-summary' + +describe('describeToolInput', () => { + it('labels a file-target call with its path, not raw JSON', () => { + expect(describeToolInput({ file_path: '/repo/src/app/Main.tsx', offset: 10 })).toBe( + '/repo/src/app/Main.tsx' + ) + expect(describeToolInput({ path: 'src/index.ts' })).toBe('src/index.ts') + expect(describeToolInput({ file_path: 'C:\\Users\\me\\project\\app.tsx' })).toBe( + 'C:\\Users\\me\\project\\app.tsx' + ) + }) + + it('labels a command-shaped call with its primary argument', () => { + expect(describeToolInput({ command: 'pnpm test', description: 'Run tests' })).toBe('pnpm test') + expect(describeToolInput({ pattern: 'foo.*bar', glob: '*.ts' })).toBe('foo.*bar') + expect(describeToolInput({ url: 'https://example.com', description: 'Fetch it' })).toBe( + 'https://example.com' + ) + expect(describeToolInput({ description: 'Only prose' })).toBe('Only prose') + }) + + it('falls back to the bounded JSON preview for other shapes', () => { + expect(describeToolInput({ todos: [{ id: 1 }] })).toBe( + summarizeToolInput({ todos: [{ id: 1 }] }) + ) + expect(describeToolInput({ command: ' ' })).toBe(summarizeToolInput({ command: ' ' })) + expect(describeToolInput('raw string input')).toBe('raw string input') + expect(describeToolInput(null)).toBe('') + }) + + it('truncates an overlong path like any other preview', () => { + const path = `/very/${'long/'.repeat(30)}file.ts` + expect(describeToolInput({ file_path: path }).length).toBeLessThanOrEqual(80) + expect(describeToolInput({ file_path: path })).toContain('…') + }) +}) + +describe('Codex JSON-string tool arguments', () => { + it('normalizes them for labels, details, file links and run summaries', () => { + expect(describeToolInput('{"cmd":"git status --short"}')).toBe('git status --short') + expect(formatToolInput('{"cmd":"git status --short"}')).toBe( + '{\n "cmd": "git status --short"\n}' + ) + expect(toolFilePath('{"file_path":"src/index.ts"}')).toBe('src/index.ts') + expect(briefToolArg('{"file_path":"src/app/index.ts"}')).toBe('index.ts') + expect(briefToolArg('{"cmd":"git status --short"}')).toBe('git status --short') + expect(isStructuredToolInput('{"cmd":"ls"}')).toBe(true) + }) + + it('joins an argv-array command into one label', () => { + expect(describeToolInput('{"command":["bash","-lc","make"]}')).toBe('bash -lc make') + expect(briefToolArg({ command: ['bash', '-lc', 'make'] })).toBe('bash -lc make') + }) + + it('leaves prose and malformed JSON as plain strings', () => { + expect(describeToolInput('{ not json')).toBe('{ not json') + expect(formatToolInput('just prose')).toBe('just prose') + expect(toolFilePath('{"file_path":')).toBeNull() + expect(isStructuredToolInput('just prose')).toBe(false) + // A JSON scalar is not an argument object — keep the literal text. + expect(formatToolInput('"quoted"')).toBe('"quoted"') + }) +}) describe('summarizeToolInput bounded preview', () => { it('collapses depth beyond the bound instead of serializing the whole tree', () => { diff --git a/src/shared/native-chat-tool-summary.ts b/src/shared/native-chat-tool-summary.ts index efe606f55a9..251963f5c07 100644 --- a/src/shared/native-chat-tool-summary.ts +++ b/src/shared/native-chat-tool-summary.ts @@ -13,49 +13,112 @@ export function summarizeToolInput(input: unknown): string { : `${collapsed.slice(0, MAX_PREVIEW_LENGTH - 1)}…` } -/** Full, pretty-printed tool-call input for the expanded detail view. Strings - * pass through as-is; objects/arrays print as indented JSON so a diff-less call - * (e.g. a question payload) reads cleanly instead of one long minified line. */ +/** Human label for a tool line: the target file path, else the primary string + * argument (command/query/…), else the bounded JSON preview. Keeps raw + * `{"file_path":…}` JSON out of the tappable row label. */ +export function describeToolInput(input: unknown): string { + const normalized = normalizeToolInput(input) + const path = toolFilePath(normalized) + if (path) { + return summarizeToolInput(path) + } + if (normalized && typeof normalized === 'object') { + const value = normalized as Record + // Concrete target/action first; prose `description` only as a last resort. + const primary = + value.command ?? value.cmd ?? value.query ?? value.pattern ?? value.url ?? value.description + const primarySummary = summarizePrimaryToolArg(primary) + if (primarySummary) { + return primarySummary + } + } + return summarizeToolInput(normalized) +} + +/** Full, pretty-printed tool-call input for the expanded detail view. Structured + * JSON strings and objects/arrays print as indented JSON so a diff-less call + * (e.g. a question payload) reads cleanly instead of one long minified line; + * other strings pass through as-is. */ export function formatToolInput(input: unknown): string { - if (input === null || input === undefined) { + const normalized = normalizeToolInput(input) + if (normalized === null || normalized === undefined) { return '' } - if (typeof input === 'string') { - return input + if (typeof normalized === 'string') { + return normalized } - if (typeof input === 'number' || typeof input === 'boolean') { - return String(input) + if (typeof normalized === 'number' || typeof normalized === 'boolean') { + return String(normalized) } try { - return JSON.stringify(input, null, 2) ?? '' + return JSON.stringify(normalized, null, 2) ?? '' } catch { return '' } } +/** Whether the expanded detail would show structured JSON rather than repeating + * the row label — i.e. whether expanding the row is worth offering. */ +export function isStructuredToolInput(input: unknown): boolean { + const normalized = normalizeToolInput(input) + return normalized !== null && typeof normalized === 'object' +} + export function toolFilePath(input: unknown): string | null { - if (!input || typeof input !== 'object') { + const normalized = normalizeToolInput(input) + if (!normalized || typeof normalized !== 'object') { return null } - const value = input as Record + const value = normalized as Record const path = value.file_path ?? value.filePath ?? value.path ?? value.notebook_path return typeof path === 'string' && path.length > 0 ? path : null } export function briefToolArg(input: unknown): string { - if (input && typeof input === 'object') { - const value = input as Record + const normalized = normalizeToolInput(input) + if (normalized && typeof normalized === 'object') { + const value = normalized as Record const path = value.file_path ?? value.filePath ?? value.path ?? value.notebook_path if (typeof path === 'string' && path.length > 0) { const parts = path.split(/[\\/]/).filter(Boolean) return parts.at(-1) ?? path } const command = value.command ?? value.cmd ?? value.query ?? value.pattern - if (typeof command === 'string') { - return summarizeToolInput(command).slice(0, 28) + const commandSummary = summarizePrimaryToolArg(command) + if (commandSummary) { + return commandSummary.slice(0, 28) } } - return summarizeToolInput(input).slice(0, 28) + return summarizeToolInput(normalized).slice(0, 28) +} + +/** Codex delivers tool arguments as a JSON string. Parse those into the object + * shape every helper below already understands; leave prose strings alone. */ +function normalizeToolInput(input: unknown): unknown { + if (typeof input !== 'string') { + return input + } + const first = input.trimStart()[0] + if (first !== '{' && first !== '[') { + return input + } + try { + const parsed: unknown = JSON.parse(input) + return parsed !== null && typeof parsed === 'object' ? parsed : input + } catch { + return input + } +} + +/** A label-worthy primary argument: a non-blank string, or an argv array. */ +function summarizePrimaryToolArg(input: unknown): string | null { + if (typeof input === 'string' && input.trim()) { + return summarizeToolInput(input) + } + if (Array.isArray(input) && input.length > 0 && input.every((part) => typeof part === 'string')) { + return summarizeToolInput(input.join(' ')) + } + return null } export function summarizeToolRun(blocks: readonly NativeChatBlock[]): string {