fix: tool call UI cleanup (#515)
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
@@ -122,6 +122,7 @@ import {
|
||||
type Line,
|
||||
markIncompleteToolsAsCancelled,
|
||||
onChunk,
|
||||
setToolCallsRunning,
|
||||
toLines,
|
||||
} from "./helpers/accumulator";
|
||||
import { backfillBuffers } from "./helpers/backfill";
|
||||
@@ -2063,6 +2064,13 @@ export default function App({
|
||||
}
|
||||
}
|
||||
|
||||
// Set phase to "running" for auto-allowed tools
|
||||
setToolCallsRunning(
|
||||
buffersRef.current,
|
||||
autoAllowed.map((ac) => ac.approval.toolCallId),
|
||||
);
|
||||
refreshDerived();
|
||||
|
||||
// Execute auto-allowed tools (sequential for writes, parallel for reads)
|
||||
const autoAllowedResults = await executeAutoAllowedTools(
|
||||
autoAllowed,
|
||||
@@ -3120,6 +3128,13 @@ export default function App({
|
||||
|
||||
// Execute auto-allowed tools
|
||||
if (autoAllowed.length > 0) {
|
||||
// Set phase to "running" for auto-allowed tools
|
||||
setToolCallsRunning(
|
||||
buffersRef.current,
|
||||
autoAllowed.map((ac) => ac.approval.toolCallId),
|
||||
);
|
||||
refreshDerived();
|
||||
|
||||
const autoAllowedResults = await executeAutoAllowedTools(
|
||||
autoAllowed,
|
||||
(chunk) => onChunk(buffersRef.current, chunk),
|
||||
@@ -3172,7 +3187,7 @@ export default function App({
|
||||
// If check fails, proceed anyway (don't block user)
|
||||
return { blocked: false };
|
||||
}
|
||||
}, [agentId, processConversation]);
|
||||
}, [agentId, processConversation, refreshDerived]);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: refs read .current dynamically, complex callback with intentional deps
|
||||
const onSubmit = useCallback(
|
||||
@@ -4993,6 +5008,13 @@ DO NOT respond to these messages or otherwise consider them in your response unl
|
||||
}
|
||||
}
|
||||
|
||||
// Set phase to "running" for auto-allowed tools
|
||||
setToolCallsRunning(
|
||||
buffersRef.current,
|
||||
autoAllowed.map((ac) => ac.approval.toolCallId),
|
||||
);
|
||||
refreshDerived();
|
||||
|
||||
// Execute auto-allowed tools (sequential for writes, parallel for reads)
|
||||
const autoAllowedResults = await executeAutoAllowedTools(
|
||||
autoAllowed,
|
||||
@@ -5326,6 +5348,15 @@ DO NOT respond to these messages or otherwise consider them in your response unl
|
||||
...(additionalDecision ? [additionalDecision] : []),
|
||||
];
|
||||
|
||||
// Set phase to "running" for all approved tools
|
||||
setToolCallsRunning(
|
||||
buffersRef.current,
|
||||
allDecisions
|
||||
.filter((d) => d.type === "approve")
|
||||
.map((d) => d.approval.toolCallId),
|
||||
);
|
||||
refreshDerived();
|
||||
|
||||
// Execute approved tools and format results using shared function
|
||||
const { executeApprovalBatch } = await import(
|
||||
"../agent/approval-execution"
|
||||
@@ -5601,6 +5632,15 @@ DO NOT respond to these messages or otherwise consider them in your response unl
|
||||
setStreaming(true);
|
||||
buffersRef.current.interrupted = false;
|
||||
|
||||
// Set phase to "running" for all approved tools
|
||||
setToolCallsRunning(
|
||||
buffersRef.current,
|
||||
allDecisions
|
||||
.filter((d) => d.type === "approve")
|
||||
.map((d) => d.approval.toolCallId),
|
||||
);
|
||||
refreshDerived();
|
||||
|
||||
try {
|
||||
// Execute ALL decisions together
|
||||
const { executeApprovalBatch } = await import(
|
||||
@@ -6963,6 +7003,7 @@ Plan file path: ${planFilePath}`;
|
||||
line={ln}
|
||||
precomputedDiffs={precomputedDiffsRef.current}
|
||||
lastPlanFilePath={lastPlanFilePathRef.current}
|
||||
isStreaming={streaming}
|
||||
/>
|
||||
) : ln.kind === "error" ? (
|
||||
<ErrorMessage line={ln} />
|
||||
|
||||
@@ -401,7 +401,7 @@ export function AdvancedDiffRenderer(
|
||||
<Text dimColor>⎿</Text>
|
||||
</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1}>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - noChangesGutter)}>
|
||||
<Text wrap="wrap">{header}</Text>
|
||||
</Box>
|
||||
</Box>
|
||||
@@ -410,7 +410,7 @@ export function AdvancedDiffRenderer(
|
||||
<Box width={noChangesGutter} flexShrink={0}>
|
||||
<Text>{" "}</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1}>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - noChangesGutter)}>
|
||||
<Text dimColor>
|
||||
No changes to <Text bold>{relative}</Text> (file content
|
||||
identical)
|
||||
@@ -435,7 +435,7 @@ export function AdvancedDiffRenderer(
|
||||
<Text dimColor>⎿</Text>
|
||||
</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1}>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - toolResultGutter)}>
|
||||
<Text wrap="wrap">{header}</Text>
|
||||
</Box>
|
||||
</Box>
|
||||
@@ -443,7 +443,7 @@ export function AdvancedDiffRenderer(
|
||||
<Box width={toolResultGutter} flexShrink={0}>
|
||||
<Text>{" "}</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1}>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - toolResultGutter)}>
|
||||
<Text
|
||||
dimColor
|
||||
>{`Showing ~${ADV_DIFF_CONTEXT_LINES} context line${ADV_DIFF_CONTEXT_LINES === 1 ? "" : "s"}`}</Text>
|
||||
|
||||
@@ -69,10 +69,12 @@ export const ToolCallMessage = memo(
|
||||
line,
|
||||
precomputedDiffs,
|
||||
lastPlanFilePath,
|
||||
isStreaming,
|
||||
}: {
|
||||
line: ToolCallLine;
|
||||
precomputedDiffs?: Map<string, AdvancedDiffSuccess>;
|
||||
lastPlanFilePath?: string | null;
|
||||
isStreaming?: boolean;
|
||||
}) => {
|
||||
const columns = useTerminalWidth();
|
||||
|
||||
@@ -124,14 +126,36 @@ export const ToolCallMessage = memo(
|
||||
}
|
||||
}
|
||||
|
||||
// Format arguments for display using the old formatting logic
|
||||
// Pass rawName to enable special formatting for file tools
|
||||
const formatted = formatArgsDisplay(argsText, rawName);
|
||||
// Hide args for question tool (shown in result instead)
|
||||
const args = isQuestionTool(rawName) ? "" : `(${formatted.display})`;
|
||||
|
||||
const rightWidth = Math.max(0, columns - 2); // gutter is 2 cols
|
||||
|
||||
// Determine args display:
|
||||
// - Question tool: hide args (shown in result instead)
|
||||
// - Still streaming + phase "ready": args may be incomplete, show ellipsis
|
||||
// - Phase "running"/"finished" or stream done: args complete, show formatted
|
||||
let args = "";
|
||||
if (!isQuestionTool(rawName)) {
|
||||
// Args are complete once running, finished, or stream is done
|
||||
const argsComplete =
|
||||
line.phase === "running" || line.phase === "finished" || !isStreaming;
|
||||
|
||||
if (!argsComplete) {
|
||||
args = "(…)";
|
||||
} else {
|
||||
const formatted = formatArgsDisplay(argsText, rawName);
|
||||
// Normalize newlines to spaces to prevent forced line breaks
|
||||
const normalizedDisplay = formatted.display.replace(/\n/g, " ");
|
||||
// For max 2 lines: boxWidth * 2, minus parens (2) and margin (2)
|
||||
const argsBoxWidth = rightWidth - displayName.length;
|
||||
const maxArgsChars = Math.max(0, argsBoxWidth * 2 - 4);
|
||||
|
||||
const needsTruncation = normalizedDisplay.length > maxArgsChars;
|
||||
const truncatedDisplay = needsTruncation
|
||||
? `${normalizedDisplay.slice(0, maxArgsChars - 1)}…`
|
||||
: normalizedDisplay;
|
||||
args = `(${truncatedDisplay})`;
|
||||
}
|
||||
}
|
||||
|
||||
// If name exceeds available width, fall back to simple wrapped rendering
|
||||
const fallback = displayName.length >= rightWidth;
|
||||
|
||||
|
||||
@@ -508,3 +508,19 @@ export function toLines(b: Buffers): Line[] {
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set tool calls to "running" phase before execution.
|
||||
* This updates the UI to show the formatted args instead of ellipsis.
|
||||
*/
|
||||
export function setToolCallsRunning(b: Buffers, toolCallIds: string[]): void {
|
||||
for (const toolCallId of toolCallIds) {
|
||||
const lineId = b.toolCallIdToLineId.get(toolCallId);
|
||||
if (lineId) {
|
||||
const line = b.byId.get(lineId);
|
||||
if (line && line.kind === "tool_call") {
|
||||
b.byId.set(lineId, { ...line, phase: "running" });
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -258,9 +258,9 @@ export function formatArgsDisplay(
|
||||
if (k === "file_path") continue;
|
||||
if (v === undefined || v === null) continue;
|
||||
if (typeof v === "boolean" || typeof v === "number") {
|
||||
otherArgs.push(`${k}=${v}`);
|
||||
otherArgs.push(`${k}: ${v}`);
|
||||
} else if (typeof v === "string" && v.length <= 30) {
|
||||
otherArgs.push(`${k}="${v}"`);
|
||||
otherArgs.push(`${k}: "${v}"`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -316,16 +316,16 @@ export function formatArgsDisplay(
|
||||
} else {
|
||||
display = Object.entries(parsed)
|
||||
.map(([k, v]) => {
|
||||
if (v === undefined || v === null) return `${k}=${v}`;
|
||||
if (v === undefined || v === null) return `${k}: ${v}`;
|
||||
if (typeof v === "boolean" || typeof v === "number")
|
||||
return `${k}=${v}`;
|
||||
return `${k}: ${v}`;
|
||||
if (typeof v === "string")
|
||||
return v.length > 50 ? `${k}=…` : `${k}="${v}"`;
|
||||
if (Array.isArray(v)) return `${k}=[${v.length} items]`;
|
||||
return v.length > 50 ? `${k}: …` : `${k}: "${v}"`;
|
||||
if (Array.isArray(v)) return `${k}: [${v.length} items]`;
|
||||
if (typeof v === "object")
|
||||
return `${k}={${Object.keys(v as Record<string, unknown>).length} props}`;
|
||||
return `${k}: {${Object.keys(v as Record<string, unknown>).length} props}`;
|
||||
const str = JSON.stringify(v);
|
||||
return str.length > 50 ? `${k}=…` : `${k}=${str}`;
|
||||
return str.length > 50 ? `${k}: …` : `${k}: ${str}`;
|
||||
})
|
||||
.join(", ");
|
||||
}
|
||||
@@ -340,10 +340,10 @@ export function formatArgsDisplay(
|
||||
const neu = /"new_string"\s*:\s*"([\s\S]*?)"\s*(,|\})/.exec(s);
|
||||
const cont = /"content"\s*:\s*"([\s\S]*?)"\s*(,|\})/.exec(s);
|
||||
const parts: string[] = [];
|
||||
if (fp) parts.push(`file_path="${fp[1]}"`);
|
||||
if (old) parts.push(`old_string=…`);
|
||||
if (neu) parts.push(`new_string=…`);
|
||||
if (cont) parts.push(`content=…`);
|
||||
if (fp) parts.push(`file_path: "${fp[1]}"`);
|
||||
if (old) parts.push(`old_string: …`);
|
||||
if (neu) parts.push(`new_string: …`);
|
||||
if (cont) parts.push(`content: …`);
|
||||
if (parts.length) display = parts.join(", ");
|
||||
} catch {
|
||||
// If all else fails, use the ellipsis
|
||||
|
||||
@@ -16,8 +16,8 @@ export function getDisplayToolName(rawName: string): string {
|
||||
if (rawName === "edit" || rawName === "multi_edit") return "Update";
|
||||
if (rawName === "read") return "Read";
|
||||
if (rawName === "bash") return "Bash";
|
||||
if (rawName === "grep") return "Grep";
|
||||
if (rawName === "glob") return "Glob";
|
||||
if (rawName === "grep" || rawName === "Grep") return "Search";
|
||||
if (rawName === "glob" || rawName === "Glob") return "Glob";
|
||||
if (rawName === "ls") return "LS";
|
||||
if (rawName === "todo_write" || rawName === "TodoWrite") return "TODO";
|
||||
if (rawName === "EnterPlanMode" || rawName === "ExitPlanMode")
|
||||
@@ -29,7 +29,7 @@ export function getDisplayToolName(rawName: string): string {
|
||||
if (rawName === "shell_command" || rawName === "shell") return "Bash";
|
||||
if (rawName === "read_file") return "Read";
|
||||
if (rawName === "list_dir") return "LS";
|
||||
if (rawName === "grep_files") return "Grep";
|
||||
if (rawName === "grep_files") return "Search";
|
||||
if (rawName === "apply_patch") return "Patch";
|
||||
|
||||
// Codex toolset (PascalCase)
|
||||
@@ -37,7 +37,7 @@ export function getDisplayToolName(rawName: string): string {
|
||||
if (rawName === "ShellCommand" || rawName === "Shell") return "Bash";
|
||||
if (rawName === "ReadFile") return "Read";
|
||||
if (rawName === "ListDir") return "LS";
|
||||
if (rawName === "GrepFiles") return "Grep";
|
||||
if (rawName === "GrepFiles") return "Search";
|
||||
if (rawName === "ApplyPatch") return "Patch";
|
||||
|
||||
// Gemini toolset (snake_case)
|
||||
@@ -45,7 +45,7 @@ export function getDisplayToolName(rawName: string): string {
|
||||
if (rawName === "read_file_gemini") return "Read";
|
||||
if (rawName === "list_directory") return "LS";
|
||||
if (rawName === "glob_gemini") return "Glob";
|
||||
if (rawName === "search_file_content") return "Grep";
|
||||
if (rawName === "search_file_content") return "Search";
|
||||
if (rawName === "write_file_gemini") return "Write";
|
||||
if (rawName === "write_todos") return "TODO";
|
||||
if (rawName === "read_many_files") return "Read Multiple";
|
||||
@@ -55,7 +55,7 @@ export function getDisplayToolName(rawName: string): string {
|
||||
if (rawName === "ReadFileGemini") return "Read";
|
||||
if (rawName === "ListDirectory") return "LS";
|
||||
if (rawName === "GlobGemini") return "Glob";
|
||||
if (rawName === "SearchFileContent") return "Grep";
|
||||
if (rawName === "SearchFileContent") return "Search";
|
||||
if (rawName === "WriteFileGemini") return "Write";
|
||||
if (rawName === "WriteTodos") return "TODO";
|
||||
if (rawName === "ReadManyFiles") return "Read Multiple";
|
||||
|
||||
Reference in New Issue
Block a user