Tool error backticks (#53)

This looks more reasonable now, because the code is now rendered in a
monospace font:

<img width="475" height="450" alt="Screenshot 2025-09-15 at 10 58 20 AM"
src="https://github.com/user-attachments/assets/9870436b-ee8f-44cd-b814-b22ad9261ebe"
/>

That said, this definitely would benefit from a horizontal scroll bar on
the code blocks. You can't really read what it's saying without copying
the text out of the block.

Of course, in this particular case the relevant info is all in the
beginning of the error message (namely, that it failed to find the
`old_string`), and this does look overall better than before (where it
was just this big jumble of non-monospaced code), so I think it's worth
merging as-is and then revisiting horizontal scrolling later.
This commit is contained in:
Richard Feldman
2025-09-15 11:02:47 -04:00
committed by GitHub
parent ee17e45030
commit 491efe32e8
2 changed files with 18 additions and 6 deletions

View File

@@ -778,7 +778,10 @@ describe("tool conversions", () => {
// Should return empty object when parsing fails
expect(update).toEqual({
content: [
{ content: { type: "text", text: "Failed to find `old_string`" }, type: "content" },
{
content: { type: "text", text: "```\nFailed to find `old_string`\n```" },
type: "content",
},
],
});
});

View File

@@ -487,7 +487,7 @@ export function toolUpdateFromToolResult(
case "Write": {
if (toolResult.is_error && toolResult.content?.length > 0) {
// Only return errors
return toAcpContentUpdate(toolResult.content);
return toAcpContentUpdate(toolResult.content, true);
}
return {};
}
@@ -511,17 +511,26 @@ export function toolUpdateFromToolResult(
case "WebSearch":
case "Other":
default: {
return toAcpContentUpdate(toolResult.content);
return toAcpContentUpdate(toolResult.content, toolResult.is_error);
}
}
}
function toAcpContentUpdate(content: any): { content?: ToolCallContent[] } {
function toAcpContentUpdate(
content: any,
isError: boolean = false,
): { content?: ToolCallContent[] } {
if (Array.isArray(content) && content.length > 0) {
return {
content: content.map((content: any) => ({
type: "content",
content,
content:
isError && content.type === "text"
? {
...content,
text: `\`\`\`\n${content.text}\n\`\`\``,
}
: content,
})),
};
} else if (typeof content === "string" && content.length > 0) {
@@ -531,7 +540,7 @@ function toAcpContentUpdate(content: any): { content?: ToolCallContent[] } {
type: "content",
content: {
type: "text",
text: content,
text: isError ? `\`\`\`\n${content}\n\`\`\`` : content,
},
},
],