diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 79de1c868..bcd287c18 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -17,7 +17,7 @@ ### Fixed - Fixed tool-call argument parsing dropping fields when a string contains an invalid `\u` escape such as `\usepackage` ([#10868](https://github.com/earendil-works/pi/issues/10868)) -- Fixed Anthropic requests failing with a 400 when an error tool result contains images; the images are now replaced with a text note ([#10794](https://github.com/earendil-works/pi/issues/10794)) +- Fixed Anthropic and Bedrock Claude requests failing with a 400 when an error tool result contains images; the images are now sent after the tool results of the turn, so the model still sees them ([#10794](https://github.com/earendil-works/pi/issues/10794)) - Fixed OpenCode Go Gemini models failing with a 404 by moving OpenCode Zen and OpenCode Go to the `opencode.ai/inference` endpoints, and added Google Generative AI support to the OpenCode Go provider - Fixed OpenCode Go MiniMax M2.7 failing with "Model does not support this protocol" by using the Anthropic Messages API - Fixed automatic retry not triggering for gateway `Unable to forward upstream stream` errors ([#10920](https://github.com/earendil-works/pi/issues/10920)) diff --git a/packages/ai/src/api/anthropic-messages.ts b/packages/ai/src/api/anthropic-messages.ts index a10dd52d8..af4327e8a 100644 --- a/packages/ai/src/api/anthropic-messages.ts +++ b/packages/ai/src/api/anthropic-messages.ts @@ -60,7 +60,7 @@ import { } from "./constrained-sampling.ts"; import { buildCopilotDynamicHeaders, hasCopilotVisionInput } from "./github-copilot-headers.ts"; import { adjustMaxTokensForThinking, buildBaseOptions, clampMaxTokensToContext } from "./simple-options.ts"; -import { transformMessages } from "./transform-messages.ts"; +import { splitErrorToolResultImages, transformMessages } from "./transform-messages.ts"; /** * Resolve cache retention preference. @@ -131,6 +131,17 @@ const fromClaudeCodeName = (name: string, tools?: Tool[]) => { return name; }; +function convertImageBlock(image: ImageContent) { + return { + type: "image" as const, + source: { + type: "base64" as const, + media_type: image.mimeType as "image/jpeg" | "image/png" | "image/gif" | "image/webp", + data: image.data, + }, + }; +} + /** * Convert content blocks to Anthropic API format */ @@ -161,14 +172,7 @@ function convertContentBlocks(content: (TextContent | ImageContent)[]): text: sanitizeSurrogates(block.text), }; } - return { - type: "image" as const, - source: { - type: "base64" as const, - media_type: block.mimeType as "image/jpeg" | "image/png" | "image/gif" | "image/webp", - data: block.data, - }, - }; + return convertImageBlock(block); }); // If only images (no text), add placeholder text block @@ -1332,16 +1336,7 @@ function normalizeToolCallId(id: string): string { return id.replace(/[^a-zA-Z0-9_-]/g, "_").slice(0, 64); } -function convertToolResult(msg: ToolResultMessage): ContentBlockParam { - // Anthropic rejects error tool results that contain anything but text, so images are replaced - // with a note (#10794). - const content = msg.isError - ? msg.content.map((block) => - block.type === "image" - ? { type: "text" as const, text: "[image omitted: error tool results cannot contain images]" } - : block, - ) - : msg.content; +function convertToolResult(msg: ToolResultMessage, content = msg.content): ContentBlockParam { return { type: "tool_result", tool_use_id: msg.toolCallId, @@ -1418,14 +1413,7 @@ function convertMessages( text: sanitizeSurrogates(item.text), }; } else { - return { - type: "image", - source: { - type: "base64", - media_type: item.mimeType as "image/jpeg" | "image/png" | "image/gif" | "image/webp", - data: item.data, - }, - }; + return convertImageBlock(item); } }); const filteredBlocks = blocks.filter((b) => { @@ -1512,9 +1500,17 @@ function convertMessages( } else if (msg.role === "toolResult") { // Collect all consecutive toolResult messages, needed for z.ai Anthropic endpoint. const toolResults: ContentBlockParam[] = []; + // Anthropic rejects error tool results that contain anything but text, so their images follow + // all tool results of the turn, which must come first in the user message (#10794). + const errorImages: ContentBlockParam[] = []; let j = i; while (j < transformedMessages.length && transformedMessages[j].role === "toolResult") { - toolResults.push(convertToolResult(transformedMessages[j] as ToolResultMessage)); + const toolMsg = transformedMessages[j] as ToolResultMessage; + const split = splitErrorToolResultImages(toolMsg); + toolResults.push(convertToolResult(toolMsg, split?.content)); + if (split) { + errorImages.push({ type: "text", text: split.caption }, ...split.images.map(convertImageBlock)); + } j++; } @@ -1523,7 +1519,7 @@ function convertMessages( params.push({ role: "user", - content: toolResults, + content: [...toolResults, ...errorImages], }); } } diff --git a/packages/ai/src/api/bedrock-converse-stream.ts b/packages/ai/src/api/bedrock-converse-stream.ts index 04cd40cf2..7434d81d2 100644 --- a/packages/ai/src/api/bedrock-converse-stream.ts +++ b/packages/ai/src/api/bedrock-converse-stream.ts @@ -72,7 +72,7 @@ import { clampMaxTokensToContext, clampReasoning, } from "./simple-options.ts"; -import { transformMessages } from "./transform-messages.ts"; +import { splitErrorToolResultImages, transformMessages } from "./transform-messages.ts"; export type BedrockThinkingDisplay = "summarized" | "omitted"; @@ -1093,35 +1093,34 @@ function convertMessages( case "toolResult": { // Collect all consecutive toolResult messages into a single user message // Bedrock requires all tool results to be in one message - const toolResults: ContentBlock.ToolResultMember[] = []; - - // Add current tool result with all content blocks combined - toolResults.push({ - toolResult: { - toolUseId: m.toolCallId, - content: convertToolResultContent(m.content), - status: m.isError ? ToolResultStatus.ERROR : ToolResultStatus.SUCCESS, - }, - }); - - // Look ahead for consecutive toolResult messages - let j = i + 1; + const toolResults: ContentBlock[] = []; + // Claude rejects error tool results that contain anything but text, so for Claude their + // images follow all tool results of the turn (#10794). Other models keep them in place. + const errorImages: ContentBlock[] = []; + let j = i; while (j < transformedMessages.length && transformedMessages[j].role === "toolResult") { - const nextMsg = transformedMessages[j] as ToolResultMessage; + const toolMsg = transformedMessages[j] as ToolResultMessage; + const split = isAnthropicClaudeModel(model) ? splitErrorToolResultImages(toolMsg) : undefined; toolResults.push({ toolResult: { - toolUseId: nextMsg.toolCallId, - content: convertToolResultContent(nextMsg.content), - status: nextMsg.isError ? ToolResultStatus.ERROR : ToolResultStatus.SUCCESS, + toolUseId: toolMsg.toolCallId, + content: convertToolResultContent(split?.content ?? toolMsg.content), + status: toolMsg.isError ? ToolResultStatus.ERROR : ToolResultStatus.SUCCESS, }, }); + if (split) { + errorImages.push( + { text: split.caption }, + ...split.images.map((image) => ({ image: createImageBlock(image.mimeType, image.data) })), + ); + } j++; } // Skip the messages we've already processed i = j - 1; - result.push({ role: ConversationRole.USER, content: toolResults }); + result.push({ role: ConversationRole.USER, content: [...toolResults, ...errorImages] }); break; } default: diff --git a/packages/ai/src/api/transform-messages.ts b/packages/ai/src/api/transform-messages.ts index 94bf539cc..7830c0ed2 100644 --- a/packages/ai/src/api/transform-messages.ts +++ b/packages/ai/src/api/transform-messages.ts @@ -11,6 +11,33 @@ import type { const NON_VISION_USER_IMAGE_PLACEHOLDER = "(image omitted: model does not support images)"; const NON_VISION_TOOL_IMAGE_PLACEHOLDER = "(tool image omitted: model does not support images)"; +const ERROR_TOOL_IMAGE_PLACEHOLDER = "(image attached after the tool results)"; + +/** An error tool result split into a text-only result and the images to send after the tool results. */ +export interface SplitErrorToolResult { + /** The result content with each run of images replaced by a note. */ + content: TextContent[]; + /** Text naming the tool call, to send before `images`. */ + caption: string; + images: ImageContent[]; +} + +/** + * Split the images out of an error tool result, for APIs that reject images in error results + * (Anthropic, and Claude on Bedrock). Send the returned content as the tool result and the caption and + * images after all tool results of the turn, in the same user message. Returns undefined for results + * that need no split (#10794). + */ +export function splitErrorToolResultImages(msg: ToolResultMessage): SplitErrorToolResult | undefined { + if (!msg.isError) return undefined; + const images = msg.content.filter((block): block is ImageContent => block.type === "image"); + if (images.length === 0) return undefined; + return { + content: replaceImagesWithPlaceholder(msg.content, ERROR_TOOL_IMAGE_PLACEHOLDER), + caption: `Images from the error result of tool call ${msg.toolCallId} (${msg.toolName}):`, + images, + }; +} function replaceImagesWithPlaceholder(content: (TextContent | ImageContent)[], placeholder: string): TextContent[] { const result: TextContent[] = []; diff --git a/packages/ai/test/anthropic-error-tool-result-images.test.ts b/packages/ai/test/anthropic-error-tool-result-images.test.ts new file mode 100644 index 000000000..08c9cfdd6 --- /dev/null +++ b/packages/ai/test/anthropic-error-tool-result-images.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, it } from "vitest"; +import { streamSimple } from "../src/compat.ts"; +import type { AssistantMessage, Context, Model } from "../src/types.ts"; + +interface AnthropicPayload { + messages: Array<{ role: string; content: string | Array> }>; +} + +class PayloadCaptured extends Error { + constructor() { + super("payload captured"); + this.name = "PayloadCaptured"; + } +} + +const model: Model<"anthropic-messages"> = { + id: "claude-test", + name: "Claude Test", + api: "anthropic-messages", + provider: "anthropic", + baseUrl: "http://127.0.0.1:9", + reasoning: false, + input: ["text", "image"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 200000, + maxTokens: 1024, +}; + +const assistant: AssistantMessage = { + role: "assistant", + content: [ + { type: "toolCall", id: "call-1", name: "codemode", arguments: {} }, + { type: "toolCall", id: "call-2", name: "read", arguments: {} }, + ], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-test", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp: Date.now(), +}; + +async function capturePayload(context: Context): Promise { + let capturedPayload: AnthropicPayload | undefined; + const stream = streamSimple(model, context, { + apiKey: "fake-key", + cacheRetention: "none", + onPayload: (payload) => { + capturedPayload = payload as AnthropicPayload; + throw new PayloadCaptured(); + }, + }); + await stream.result(); + if (!capturedPayload) throw new Error("Expected payload capture before request"); + return capturedPayload; +} + +const image = (data: string) => ({ type: "image", source: { type: "base64", media_type: "image/png", data } }); + +describe("Anthropic error tool results with images", () => { + // Regression for #10794: Anthropic rejects error tool results that contain images. + it("moves the images after the tool results of the turn", async () => { + const payload = await capturePayload({ + messages: [ + { role: "user", content: "Show the screenshots", timestamp: Date.now() }, + assistant, + { + role: "toolResult", + toolCallId: "call-1", + toolName: "codemode", + content: [ + { type: "text", text: "Script failed" }, + { type: "image", data: "AAAA", mimeType: "image/png" }, + { type: "image", data: "CCCC", mimeType: "image/png" }, + { type: "text", text: "Script error" }, + ], + isError: true, + timestamp: Date.now(), + }, + { + role: "toolResult", + toolCallId: "call-2", + toolName: "read", + content: [{ type: "image", data: "BBBB", mimeType: "image/png" }], + isError: false, + timestamp: Date.now(), + }, + ], + }); + + expect(payload.messages[2]).toEqual({ + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "call-1", + content: "Script failed\n(image attached after the tool results)\nScript error", + is_error: true, + }, + { + type: "tool_result", + tool_use_id: "call-2", + content: [{ type: "text", text: "(see attached image)" }, image("BBBB")], + is_error: false, + }, + { type: "text", text: "Images from the error result of tool call call-1 (codemode):" }, + image("AAAA"), + image("CCCC"), + ], + }); + }); + + it("keeps error tool results without images unchanged", async () => { + const payload = await capturePayload({ + messages: [ + { role: "user", content: "Run it", timestamp: Date.now() }, + { ...assistant, content: [assistant.content[0]] }, + { + role: "toolResult", + toolCallId: "call-1", + toolName: "codemode", + content: [{ type: "text", text: "Script failed" }], + isError: true, + timestamp: Date.now(), + }, + ], + }); + + expect(payload.messages[2]).toEqual({ + role: "user", + content: [{ type: "tool_result", tool_use_id: "call-1", content: "Script failed", is_error: true }], + }); + }); +}); diff --git a/packages/ai/test/bedrock-convert-messages.test.ts b/packages/ai/test/bedrock-convert-messages.test.ts index e35169069..88535c5dc 100644 --- a/packages/ai/test/bedrock-convert-messages.test.ts +++ b/packages/ai/test/bedrock-convert-messages.test.ts @@ -325,6 +325,98 @@ describe("bedrock convertMessages skips unknown content types", () => { expect(p.messages[0].content[0].toolResult.content).toEqual([{ text: "" }]); }); + // Regression for #10794: Claude rejects error tool results that contain images. + it("moves images of error tool results after the tool results", async () => { + const messages: Message[] = [ + { + role: "assistant", + content: [ + { type: "toolCall", id: "call-1", name: "codemode", arguments: {} }, + { type: "toolCall", id: "call-2", name: "read", arguments: {} }, + ], + api: "bedrock-converse-stream", + provider: "amazon-bedrock", + model: baseModel.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp: Date.now(), + }, + { + role: "toolResult", + toolCallId: "call-1", + toolName: "codemode", + content: [ + { type: "text", text: "Script failed" }, + { type: "image", data: "AAAA", mimeType: "image/png" }, + { type: "text", text: "Script error" }, + ], + isError: true, + timestamp: Date.now(), + }, + { + role: "toolResult", + toolCallId: "call-2", + toolName: "read", + content: [{ type: "image", data: "BBBB", mimeType: "image/png" }], + isError: false, + timestamp: Date.now(), + }, + ]; + const payload = (await capturePayload({ messages })) as { + messages: Array<{ role: string; content: Array> }>; + }; + const content = payload.messages[1].content; + expect(content).toHaveLength(4); + expect(content[0].toolResult).toEqual({ + toolUseId: "call-1", + content: [ + { text: "Script failed" }, + { text: "(image attached after the tool results)" }, + { text: "Script error" }, + ], + status: "error", + }); + expect(content[1].toolResult.status).toBe("success"); + expect(content[1].toolResult.content).toEqual([{ image: expect.objectContaining({ format: "png" }) }]); + expect(content[2]).toEqual({ text: "Images from the error result of tool call call-1 (codemode):" }); + expect(content[3]).toEqual({ image: expect.objectContaining({ format: "png" }) }); + }); + + it("keeps images in error tool results for non-Claude models", async () => { + const messages: Message[] = [ + { + role: "toolResult", + toolCallId: "call-1", + toolName: "codemode", + content: [ + { type: "text", text: "Script failed" }, + { type: "image", data: "AAAA", mimeType: "image/png" }, + ], + isError: true, + timestamp: Date.now(), + }, + ]; + const payload = (await capturePayload({ messages }, novaModel)) as { + messages: Array<{ role: string; content: Array> }>; + }; + expect(payload.messages[0].content).toEqual([ + { + toolResult: { + toolUseId: "call-1", + content: [{ text: "Script failed" }, { image: expect.objectContaining({ format: "png" }) }], + status: "error", + }, + }, + ]); + }); + it("skips assistant messages with only unknown content blocks", async () => { const messages: Message[] = [ {