mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(mobile): label tool rows with a clean summary, expand full input (STA-3333)
Mobile tool rows showed the raw input JSON (`{"file_path":…}`) as the row
label, and the expanded detail just repeated that same truncated string.
- `describeToolInput` labels a row with the target file path, else the
primary argument (command/cmd/query/pattern/url/description), else the
bounded JSON preview.
- Codex delivers tool arguments as a JSON string; normalize those into the
object shape the helpers already understand, so labels, file links,
run summaries and the expanded detail all work for Codex calls too.
- The expanded detail now renders the fully formatted input, capped at
MAX_TOOL_RESULT_CHARS like desktop's tool detail (and like the result
body), and a structured input makes the row expandable.
This commit is contained in:
@@ -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}'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 (
|
||||
<View style={[styles.toolResult, isError && styles.toolResultError]}>
|
||||
<Text style={styles.mono}>
|
||||
{output.length > MAX_TOOL_RESULT_CHARS
|
||||
? `${output.slice(0, MAX_TOOL_RESULT_CHARS)}…`
|
||||
: output}
|
||||
</Text>
|
||||
<Text style={styles.mono}>{boundedText(output)}</Text>
|
||||
</View>
|
||||
)
|
||||
}
|
||||
@@ -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 ? (
|
||||
<View style={styles.toolDetail}>
|
||||
{callDiff ? <DiffView lines={callDiff} /> : null}
|
||||
{!callDiff && call && preview ? <Text style={styles.mono}>{preview}</Text> : null}
|
||||
{callDetail ? <Text style={styles.mono}>{callDetail}</Text> : null}
|
||||
{result ? (
|
||||
<ResultBody output={result.output} isError={result.isError} diff={resultDiff} />
|
||||
) : null}
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
export {
|
||||
briefToolArg,
|
||||
countToolCalls,
|
||||
describeToolInput,
|
||||
formatToolInput,
|
||||
isStructuredToolInput,
|
||||
summarizeToolInput,
|
||||
summarizeToolRun,
|
||||
toolFilePath
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<string, unknown>
|
||||
// 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<string, unknown>
|
||||
const value = normalized as Record<string, unknown>
|
||||
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<string, unknown>
|
||||
const normalized = normalizeToolInput(input)
|
||||
if (normalized && typeof normalized === 'object') {
|
||||
const value = normalized as Record<string, unknown>
|
||||
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 {
|
||||
|
||||
Reference in New Issue
Block a user