mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(mobile): name search rows by their term and keep the filename in path labels (STA-3333)
Review follow-ups to the tool-row summary, all in the shared helper: - A Grep/Glob row labelled itself with the directory it scanned and dropped the pattern entirely, because `toolFilePath` treats `path` as a file target. That path is a scan root, so it also rendered a tap-to-open link that asked the app to open a folder. `toolFilePath` now ignores the generic `path` key for search-shaped input, which lets the pattern win the label and drops the bogus link; an explicit `file_path` still wins. - An overlong path was truncated from the head, cutting off the basename — the one part that tells two rows apart. Trim from the front instead, so the label reads `…/session/MobileNativeChatMessage.tsx`. - The primary-argument chain used `??`, so a present-but-blank key selected itself and swallowed the keys ranked after it, dropping the label all the way back to raw JSON. Take the first key that actually yields a label. Refs STA-3333.
This commit is contained in:
@@ -44,6 +44,48 @@ describe('describeToolInput', () => {
|
||||
expect(describeToolInput({ file_path: path }).length).toBeLessThanOrEqual(80)
|
||||
expect(describeToolInput({ file_path: path })).toContain('…')
|
||||
})
|
||||
|
||||
it('keeps the filename when trimming an overlong path', () => {
|
||||
// Head-truncating an absolute path drops the basename — the one part that
|
||||
// tells two rows apart — so the trim has to come off the front.
|
||||
const path = `/Users/me/orca/workspaces/orca/sta-3333/src/shared/${'nested/'.repeat(4)}app.tsx`
|
||||
const label = describeToolInput({ file_path: path, offset: 10 })
|
||||
expect(label.length).toBeLessThanOrEqual(80)
|
||||
expect(label.startsWith('…')).toBe(true)
|
||||
expect(label.endsWith('/app.tsx')).toBe(true)
|
||||
})
|
||||
|
||||
it('distinguishes two long paths that share a deep prefix', () => {
|
||||
const base = '/Users/me/orca/workspaces/orca/sta-3333-tool-summary/src/session/native'
|
||||
const a = describeToolInput({ file_path: `${base}/MobileNativeChatMessage.tsx` })
|
||||
const b = describeToolInput({ file_path: `${base}/MobileNativeChatComposer.tsx` })
|
||||
expect(a).not.toBe(b)
|
||||
})
|
||||
|
||||
it('names a search call by its pattern, not the directory it scanned', () => {
|
||||
// `path` on a Grep/Glob is the scan root; labelling with it loses the term.
|
||||
expect(describeToolInput({ pattern: 'summarizeToolInput', path: 'src/shared' })).toBe(
|
||||
'summarizeToolInput'
|
||||
)
|
||||
expect(describeToolInput({ pattern: '**/*.tsx', path: 'mobile/src' })).toBe('**/*.tsx')
|
||||
expect(describeToolInput({ query: 'auth flow', path: 'src' })).toBe('auth flow')
|
||||
// ...and offers no open-file link, since that path is a folder.
|
||||
expect(toolFilePath({ pattern: 'x', path: 'src/shared' })).toBeNull()
|
||||
expect(briefToolArg({ pattern: 'x', path: 'src/shared' })).toBe('x')
|
||||
})
|
||||
|
||||
it('still treats an explicit file target as one alongside a search term', () => {
|
||||
expect(toolFilePath({ pattern: 'x', file_path: 'src/a.ts' })).toBe('src/a.ts')
|
||||
expect(toolFilePath({ path: 'src/c.ts' })).toBe('src/c.ts')
|
||||
})
|
||||
|
||||
it('skips a present-but-blank key instead of falling through to raw JSON', () => {
|
||||
expect(describeToolInput({ command: '', query: 'needle' })).toBe('needle')
|
||||
expect(describeToolInput({ command: ' ', url: 'https://example.com' })).toBe(
|
||||
'https://example.com'
|
||||
)
|
||||
expect(briefToolArg({ cmd: '', query: 'needle' })).toBe('needle')
|
||||
})
|
||||
})
|
||||
|
||||
describe('Codex JSON-string tool arguments', () => {
|
||||
|
||||
@@ -5,6 +5,8 @@ const MAX_PREVIEW_STRING_INPUT = 160
|
||||
const MAX_PREVIEW_COLLECTION_ITEMS = 8
|
||||
const MAX_PREVIEW_DEPTH = 2
|
||||
const MAX_TOOL_RUN_SUMMARY_PARTS = 3
|
||||
const PRIMARY_ARG_KEYS = ['command', 'cmd', 'query', 'pattern', 'url', 'description'] as const
|
||||
const BRIEF_ARG_KEYS = ['command', 'cmd', 'query', 'pattern'] as const
|
||||
|
||||
export function summarizeToolInput(input: unknown): string {
|
||||
const collapsed = toRawPreview(input).replace(/\s+/g, ' ').trim()
|
||||
@@ -20,16 +22,13 @@ export function describeToolInput(input: unknown): string {
|
||||
const normalized = normalizeToolInput(input)
|
||||
const path = toolFilePath(normalized)
|
||||
if (path) {
|
||||
return summarizeToolInput(path)
|
||||
return summarizeToolPath(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
|
||||
const primary = firstPrimaryToolArg(normalized as Record<string, unknown>, PRIMARY_ARG_KEYS)
|
||||
if (primary) {
|
||||
return primary
|
||||
}
|
||||
}
|
||||
return summarizeToolInput(normalized)
|
||||
@@ -70,23 +69,24 @@ export function toolFilePath(input: unknown): string | null {
|
||||
return null
|
||||
}
|
||||
const value = normalized as Record<string, unknown>
|
||||
const path = value.file_path ?? value.filePath ?? value.path ?? value.notebook_path
|
||||
// A search call's `path` is the directory it scanned, not a file it opened —
|
||||
// taking it would drop the pattern from the label and offer a link to a folder.
|
||||
const directory = isSearchToolInput(value) ? undefined : value.path
|
||||
const path = value.file_path ?? value.filePath ?? directory ?? value.notebook_path
|
||||
return typeof path === 'string' && path.length > 0 ? path : null
|
||||
}
|
||||
|
||||
export function briefToolArg(input: unknown): string {
|
||||
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 path = toolFilePath(normalized)
|
||||
if (path) {
|
||||
const parts = path.split(/[\\/]/).filter(Boolean)
|
||||
return parts.at(-1) ?? path
|
||||
}
|
||||
const command = value.command ?? value.cmd ?? value.query ?? value.pattern
|
||||
const commandSummary = summarizePrimaryToolArg(command)
|
||||
if (commandSummary) {
|
||||
return commandSummary.slice(0, 28)
|
||||
const command = firstPrimaryToolArg(normalized as Record<string, unknown>, BRIEF_ARG_KEYS)
|
||||
if (command) {
|
||||
return command.slice(0, 28)
|
||||
}
|
||||
}
|
||||
return summarizeToolInput(normalized).slice(0, 28)
|
||||
@@ -110,6 +110,42 @@ function normalizeToolInput(input: unknown): unknown {
|
||||
}
|
||||
}
|
||||
|
||||
/** A search call is named by what it looked for, so its `path` is a scan root
|
||||
* rather than a file target. */
|
||||
function isSearchToolInput(value: Record<string, unknown>): boolean {
|
||||
return (
|
||||
summarizePrimaryToolArg(value.query) !== null || summarizePrimaryToolArg(value.pattern) !== null
|
||||
)
|
||||
}
|
||||
|
||||
/** The first key that yields a label — a present-but-blank key must not swallow
|
||||
* the keys ranked after it. */
|
||||
function firstPrimaryToolArg(
|
||||
value: Record<string, unknown>,
|
||||
keys: readonly string[]
|
||||
): string | null {
|
||||
for (const key of keys) {
|
||||
const summary = summarizePrimaryToolArg(value[key])
|
||||
if (summary) {
|
||||
return summary
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/** A path is identified by its tail, so trim from the front: head-truncating an
|
||||
* absolute path drops the filename, the one part that tells two rows apart. */
|
||||
function summarizeToolPath(path: string): string {
|
||||
const collapsed = path.replace(/\s+/g, ' ').trim()
|
||||
if (collapsed.length <= MAX_PREVIEW_LENGTH) {
|
||||
return collapsed
|
||||
}
|
||||
const tail = collapsed.slice(collapsed.length - (MAX_PREVIEW_LENGTH - 1))
|
||||
// Start at a segment boundary so the label doesn't open mid-name.
|
||||
const boundary = tail.search(/[\\/]/)
|
||||
return `…${boundary > 0 ? tail.slice(boundary) : tail}`
|
||||
}
|
||||
|
||||
/** 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()) {
|
||||
|
||||
Reference in New Issue
Block a user