mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): resize images returned by tools (#7330)
Images produced by tools (extensions, MCP bridges, screenshot tools) went into session history at full resolution because processImage was only wired to the read tool and @file CLI attachments. Providers validate every image in the request, so one oversized image poisons the whole conversation: Anthropic drops its per-image cap from 8000px to 2000px once a request carries many images, and every later turn re-sends the same payload. Switching models does not help. Normalize tool result images in AgentSession.afterToolCall, after the extension tool_result hook so extension-injected images are covered too. Honors images.autoResize, and keeps the original block when processing fails so an unavailable image backend does not delete tool output. Refs #5369 Assisted-By: devx/86bf3b09-9aba-4d03-8154-50fca3ffc9b5
This commit is contained in:
parent
0f5286d8ab
commit
b0e05b4423
5 changed files with 331 additions and 18 deletions
|
|
@ -180,7 +180,7 @@ Keep `retry.provider.maxRetries` at `0` unless provider-level retries are explic
|
|||
| `terminal.showImages` | boolean | `true` | Show images in terminal (if supported) |
|
||||
| `terminal.imageWidthCells` | number | `60` | Preferred inline image width in terminal cells |
|
||||
| `terminal.clearOnShrink` | boolean | `false` | Clear empty rows when content shrinks (can cause flicker) |
|
||||
| `images.autoResize` | boolean | `true` | Resize images to 2000x2000 max |
|
||||
| `images.autoResize` | boolean | `true` | Resize images to 2000x2000 max. Applies to `@file` attachments, `read`, and images returned by tools |
|
||||
| `images.blockImages` | boolean | `false` | Block all images from being sent to LLM |
|
||||
|
||||
### Shell
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ import { getThemeByName, theme } from "../modes/interactive/theme/theme.ts";
|
|||
import { stripFrontmatter } from "../utils/frontmatter.ts";
|
||||
import { resolvePath } from "../utils/paths.ts";
|
||||
import { sleep } from "../utils/sleep.ts";
|
||||
import { normalizeToolResultImages } from "../utils/tool-result-images.ts";
|
||||
import { formatNoApiKeyFoundMessage, formatNoModelSelectedMessage } from "./auth-guidance.ts";
|
||||
import { type BashResult, executeBashWithOperations } from "./bash-executor.ts";
|
||||
import {
|
||||
|
|
@ -489,30 +490,34 @@ export class AgentSession {
|
|||
|
||||
this.agent.afterToolCall = async ({ toolCall, args, result, isError }) => {
|
||||
const runner = this._extensionRunner;
|
||||
if (!runner.hasHandlers("tool_result")) {
|
||||
return undefined;
|
||||
}
|
||||
const hookResult = runner.hasHandlers("tool_result")
|
||||
? await runner.emitToolResult({
|
||||
type: "tool_result",
|
||||
toolName: toolCall.name,
|
||||
toolCallId: toolCall.id,
|
||||
input: args as Record<string, unknown>,
|
||||
content: result.content,
|
||||
details: result.details,
|
||||
isError,
|
||||
usage: result.usage,
|
||||
})
|
||||
: undefined;
|
||||
|
||||
const hookResult = await runner.emitToolResult({
|
||||
type: "tool_result",
|
||||
toolName: toolCall.name,
|
||||
toolCallId: toolCall.id,
|
||||
input: args as Record<string, unknown>,
|
||||
content: result.content,
|
||||
details: result.details,
|
||||
isError,
|
||||
usage: result.usage,
|
||||
const content = hookResult?.content ?? result.content ?? [];
|
||||
// Runs after the extension hook so images injected or replaced by extensions are normalized too.
|
||||
const normalizedContent = await normalizeToolResultImages(content, {
|
||||
autoResizeImages: this.settingsManager.getImageAutoResize(),
|
||||
});
|
||||
|
||||
if (!hookResult) {
|
||||
if (!hookResult && normalizedContent === content) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return {
|
||||
content: hookResult.content,
|
||||
details: hookResult.details,
|
||||
isError: hookResult.isError ?? isError,
|
||||
usage: hookResult.usage,
|
||||
content: normalizedContent,
|
||||
details: hookResult?.details,
|
||||
isError: hookResult?.isError ?? isError,
|
||||
usage: hookResult?.usage,
|
||||
};
|
||||
};
|
||||
}
|
||||
|
|
|
|||
62
packages/coding-agent/src/utils/tool-result-images.ts
Normal file
62
packages/coding-agent/src/utils/tool-result-images.ts
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
import type { ImageContent, TextContent } from "@earendil-works/pi-ai";
|
||||
import { processImage } from "./image-process.ts";
|
||||
|
||||
export type ToolResultContent = TextContent | ImageContent;
|
||||
|
||||
export interface NormalizeToolResultImagesOptions {
|
||||
/** Whether oversized images are resized to inline provider limits. Default: true */
|
||||
autoResizeImages?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize image blocks returned by tool results.
|
||||
*
|
||||
* The `read` tool and `@file` CLI attachments run their images through `processImage`, but tools
|
||||
* that produce images themselves (extensions, MCP bridges, screenshot tools) hand back arbitrary
|
||||
* base64 payloads that go straight into session history and every subsequent provider request.
|
||||
* Oversized images make the provider reject the whole conversation, not just the offending turn,
|
||||
* so normalize them once as they enter history.
|
||||
*
|
||||
* Returns the original array when nothing changed so callers can skip rewriting the result.
|
||||
*/
|
||||
export async function normalizeToolResultImages(
|
||||
content: ToolResultContent[],
|
||||
options?: NormalizeToolResultImagesOptions,
|
||||
): Promise<ToolResultContent[]> {
|
||||
if (!content.some((block) => block.type === "image")) {
|
||||
return content;
|
||||
}
|
||||
|
||||
const autoResizeImages = options?.autoResizeImages ?? true;
|
||||
const normalized: ToolResultContent[] = [];
|
||||
let changed = false;
|
||||
|
||||
for (const block of content) {
|
||||
if (block.type !== "image") {
|
||||
normalized.push(block);
|
||||
continue;
|
||||
}
|
||||
|
||||
const processed = await processImage(Buffer.from(block.data, "base64"), block.mimeType, { autoResizeImages });
|
||||
if (!processed.ok) {
|
||||
// Unlike `read`, keep the original block. The tool already produced this image and the
|
||||
// failure may just be an unavailable image backend, so passing it through preserves the
|
||||
// behavior tools have today instead of silently deleting their output.
|
||||
normalized.push(block);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (processed.data === block.data && processed.mimeType === block.mimeType && processed.hints.length === 0) {
|
||||
normalized.push(block);
|
||||
continue;
|
||||
}
|
||||
|
||||
normalized.push({ type: "image", data: processed.data, mimeType: processed.mimeType });
|
||||
if (processed.hints.length > 0) {
|
||||
normalized.push({ type: "text", text: processed.hints.join("\n") });
|
||||
}
|
||||
changed = true;
|
||||
}
|
||||
|
||||
return changed ? normalized : content;
|
||||
}
|
||||
|
|
@ -0,0 +1,109 @@
|
|||
import { crc32, deflateSync } from "node:zlib";
|
||||
import type { AgentTool } from "@earendil-works/pi-agent-core";
|
||||
import type { ImageContent } from "@earendil-works/pi-ai";
|
||||
import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai";
|
||||
import { Type } from "typebox";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createHarness, type Harness } from "./harness.ts";
|
||||
|
||||
function pngChunk(type: string, body: Buffer): Buffer {
|
||||
const header = Buffer.alloc(8);
|
||||
header.writeUInt32BE(body.length, 0);
|
||||
header.write(type, 4, "ascii");
|
||||
const checksum = Buffer.alloc(4);
|
||||
checksum.writeUInt32BE(crc32(Buffer.concat([header.subarray(4), body])), 0);
|
||||
return Buffer.concat([header, body, checksum]);
|
||||
}
|
||||
|
||||
/** Build an 8-bit grayscale PNG of arbitrary dimensions without pulling in an encoder. */
|
||||
function createPng(width: number, height: number): Buffer {
|
||||
const ihdr = Buffer.alloc(13);
|
||||
ihdr.writeUInt32BE(width, 0);
|
||||
ihdr.writeUInt32BE(height, 4);
|
||||
ihdr[8] = 8; // bit depth
|
||||
ihdr[9] = 0; // color type: grayscale
|
||||
const raw = Buffer.alloc((width + 1) * height);
|
||||
for (let row = 0; row < height; row++) {
|
||||
raw.fill(row % 256, row * (width + 1) + 1, (row + 1) * (width + 1));
|
||||
}
|
||||
return Buffer.concat([
|
||||
Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
|
||||
pngChunk("IHDR", ihdr),
|
||||
pngChunk("IDAT", deflateSync(raw)),
|
||||
pngChunk("IEND", Buffer.alloc(0)),
|
||||
]);
|
||||
}
|
||||
|
||||
function readPngDimensions(base64Data: string): { width: number; height: number } {
|
||||
const buffer = Buffer.from(base64Data, "base64");
|
||||
return { width: buffer.readUInt32BE(16), height: buffer.readUInt32BE(20) };
|
||||
}
|
||||
|
||||
const OVERSIZED_PNG_BASE64 = createPng(2400, 4800).toString("base64");
|
||||
|
||||
/** Stands in for extension, MCP bridge, or screenshot tools that return images they produced. */
|
||||
const screenshotTool: AgentTool = {
|
||||
name: "screenshot",
|
||||
label: "Screenshot",
|
||||
description: "Return an oversized screenshot",
|
||||
parameters: Type.Object({}),
|
||||
execute: async () => ({
|
||||
content: [
|
||||
{ type: "text", text: "captured" },
|
||||
{ type: "image", data: OVERSIZED_PNG_BASE64, mimeType: "image/png" },
|
||||
],
|
||||
details: {},
|
||||
}),
|
||||
};
|
||||
|
||||
function getToolResultImages(harness: Harness): ImageContent[] {
|
||||
return harness.session.messages
|
||||
.filter((message) => message.role === "toolResult")
|
||||
.flatMap((message) => message.content)
|
||||
.filter((block): block is ImageContent => block.type === "image");
|
||||
}
|
||||
|
||||
describe("AgentSession tool result images", () => {
|
||||
const harnesses: Harness[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
while (harnesses.length > 0) {
|
||||
harnesses.pop()?.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it("resizes oversized tool result images before they enter history", async () => {
|
||||
const harness = await createHarness({ tools: [screenshotTool] });
|
||||
harnesses.push(harness);
|
||||
harness.setResponses([
|
||||
fauxAssistantMessage([fauxToolCall("screenshot", {})], { stopReason: "toolUse" }),
|
||||
fauxAssistantMessage("done"),
|
||||
]);
|
||||
|
||||
await harness.session.prompt("take a screenshot");
|
||||
|
||||
const images = getToolResultImages(harness);
|
||||
expect(images).toHaveLength(1);
|
||||
const { width, height } = readPngDimensions(images[0].data);
|
||||
expect(width).toBeLessThanOrEqual(2000);
|
||||
expect(height).toBeLessThanOrEqual(2000);
|
||||
});
|
||||
|
||||
it("honors images.autoResize being disabled", async () => {
|
||||
const harness = await createHarness({
|
||||
tools: [screenshotTool],
|
||||
settings: { images: { autoResize: false } },
|
||||
});
|
||||
harnesses.push(harness);
|
||||
harness.setResponses([
|
||||
fauxAssistantMessage([fauxToolCall("screenshot", {})], { stopReason: "toolUse" }),
|
||||
fauxAssistantMessage("done"),
|
||||
]);
|
||||
|
||||
await harness.session.prompt("take a screenshot");
|
||||
|
||||
const images = getToolResultImages(harness);
|
||||
expect(images).toHaveLength(1);
|
||||
expect(images[0].data).toBe(OVERSIZED_PNG_BASE64);
|
||||
});
|
||||
});
|
||||
137
packages/coding-agent/test/tool-result-images.test.ts
Normal file
137
packages/coding-agent/test/tool-result-images.test.ts
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
import { crc32, deflateSync } from "node:zlib";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { ToolResultContent } from "../src/utils/tool-result-images.ts";
|
||||
import { normalizeToolResultImages } from "../src/utils/tool-result-images.ts";
|
||||
|
||||
const TINY_PNG_BASE64 =
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg==";
|
||||
|
||||
function pngChunk(type: string, body: Buffer): Buffer {
|
||||
const header = Buffer.alloc(8);
|
||||
header.writeUInt32BE(body.length, 0);
|
||||
header.write(type, 4, "ascii");
|
||||
const checksum = Buffer.alloc(4);
|
||||
checksum.writeUInt32BE(crc32(Buffer.concat([header.subarray(4), body])), 0);
|
||||
return Buffer.concat([header, body, checksum]);
|
||||
}
|
||||
|
||||
/** Build an 8-bit grayscale PNG of arbitrary dimensions without pulling in an encoder. */
|
||||
function createPng(width: number, height: number): Buffer {
|
||||
const ihdr = Buffer.alloc(13);
|
||||
ihdr.writeUInt32BE(width, 0);
|
||||
ihdr.writeUInt32BE(height, 4);
|
||||
ihdr[8] = 8; // bit depth
|
||||
ihdr[9] = 0; // color type: grayscale
|
||||
const raw = Buffer.alloc((width + 1) * height);
|
||||
for (let row = 0; row < height; row++) {
|
||||
raw.fill(row % 256, row * (width + 1) + 1, (row + 1) * (width + 1));
|
||||
}
|
||||
return Buffer.concat([
|
||||
Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
|
||||
pngChunk("IHDR", ihdr),
|
||||
pngChunk("IDAT", deflateSync(raw)),
|
||||
pngChunk("IEND", Buffer.alloc(0)),
|
||||
]);
|
||||
}
|
||||
|
||||
function readPngDimensions(base64Data: string): { width: number; height: number } {
|
||||
const buffer = Buffer.from(base64Data, "base64");
|
||||
return { width: buffer.readUInt32BE(16), height: buffer.readUInt32BE(20) };
|
||||
}
|
||||
|
||||
function createTinyBmp1x1Red24bpp(): Buffer {
|
||||
const buffer = Buffer.alloc(58);
|
||||
buffer.write("BM", 0, "ascii");
|
||||
buffer.writeUInt32LE(buffer.length, 2);
|
||||
buffer.writeUInt32LE(54, 10);
|
||||
buffer.writeUInt32LE(40, 14);
|
||||
buffer.writeInt32LE(1, 18);
|
||||
buffer.writeInt32LE(1, 22);
|
||||
buffer.writeUInt16LE(1, 26);
|
||||
buffer.writeUInt16LE(24, 28);
|
||||
buffer.writeUInt32LE(0, 30);
|
||||
buffer.writeUInt32LE(4, 34);
|
||||
buffer[56] = 0xff;
|
||||
return buffer;
|
||||
}
|
||||
|
||||
function imageBlock(bytes: Buffer, mimeType: string): ToolResultContent {
|
||||
return { type: "image", data: bytes.toString("base64"), mimeType };
|
||||
}
|
||||
|
||||
describe("normalizeToolResultImages", () => {
|
||||
it("returns the original array when there are no image blocks", async () => {
|
||||
const content: ToolResultContent[] = [{ type: "text", text: "no images here" }];
|
||||
|
||||
expect(await normalizeToolResultImages(content)).toBe(content);
|
||||
});
|
||||
|
||||
it("returns the original array when images are already within limits", async () => {
|
||||
const content: ToolResultContent[] = [
|
||||
{ type: "text", text: "screenshot" },
|
||||
{ type: "image", data: TINY_PNG_BASE64, mimeType: "image/png" },
|
||||
];
|
||||
|
||||
expect(await normalizeToolResultImages(content)).toBe(content);
|
||||
});
|
||||
|
||||
it("resizes oversized images and reports the original dimensions", async () => {
|
||||
const content: ToolResultContent[] = [imageBlock(createPng(2400, 4800), "image/png")];
|
||||
|
||||
const normalized = await normalizeToolResultImages(content);
|
||||
|
||||
expect(normalized).not.toBe(content);
|
||||
expect(normalized).toHaveLength(2);
|
||||
const image = normalized[0];
|
||||
expect(image.type).toBe("image");
|
||||
if (image.type !== "image") return;
|
||||
const { width, height } = readPngDimensions(image.data);
|
||||
expect(width).toBeLessThanOrEqual(2000);
|
||||
expect(height).toBeLessThanOrEqual(2000);
|
||||
const note = normalized[1];
|
||||
expect(note.type).toBe("text");
|
||||
if (note.type !== "text") return;
|
||||
expect(note.text).toContain("original 2400x4800");
|
||||
});
|
||||
|
||||
it("leaves oversized images alone when auto-resize is disabled", async () => {
|
||||
const content: ToolResultContent[] = [imageBlock(createPng(2400, 4800), "image/png")];
|
||||
|
||||
const normalized = await normalizeToolResultImages(content, { autoResizeImages: false });
|
||||
|
||||
expect(normalized).toBe(content);
|
||||
});
|
||||
|
||||
it("converts unsupported image formats even when auto-resize is disabled", async () => {
|
||||
const content: ToolResultContent[] = [imageBlock(createTinyBmp1x1Red24bpp(), "image/bmp")];
|
||||
|
||||
const normalized = await normalizeToolResultImages(content, { autoResizeImages: false });
|
||||
|
||||
expect(normalized).not.toBe(content);
|
||||
expect(normalized[0]).toMatchObject({ type: "image", mimeType: "image/png" });
|
||||
expect(normalized[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: "[Image converted from image/bmp to image/png.]",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps undecodable images instead of dropping tool output", async () => {
|
||||
const content: ToolResultContent[] = [{ type: "image", data: "bm90LWFuLWltYWdl", mimeType: "image/png" }];
|
||||
|
||||
expect(await normalizeToolResultImages(content)).toBe(content);
|
||||
});
|
||||
|
||||
it("preserves surrounding text blocks and their order", async () => {
|
||||
const content: ToolResultContent[] = [
|
||||
{ type: "text", text: "before" },
|
||||
imageBlock(createPng(2400, 100), "image/png"),
|
||||
{ type: "text", text: "after" },
|
||||
];
|
||||
|
||||
const normalized = await normalizeToolResultImages(content);
|
||||
|
||||
expect(normalized.map((block) => block.type)).toEqual(["text", "image", "text", "text"]);
|
||||
expect(normalized[0]).toEqual({ type: "text", text: "before" });
|
||||
expect(normalized[3]).toEqual({ type: "text", text: "after" });
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue