diff --git a/packages/coding-agent/src/tools/eval.ts b/packages/coding-agent/src/tools/eval.ts index ce3b9b7ea..c93f31170 100644 --- a/packages/coding-agent/src/tools/eval.ts +++ b/packages/coding-agent/src/tools/eval.ts @@ -8,7 +8,7 @@ import { jsBackend, parseEvalInput, pythonBackend, sniffEvalLanguage } from "../ import type { ExecutorBackend } from "../eval/backend"; import evalGrammar from "../eval/eval.lark" with { type: "text" }; import { ABORT_WARNING, type ParsedEvalCell } from "../eval/parse"; -import type { EvalCellResult, EvalLanguage, EvalStatusEvent, EvalToolDetails } from "../eval/types"; +import type { EvalCellResult, EvalDisplayOutput, EvalLanguage, EvalStatusEvent, EvalToolDetails } from "../eval/types"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import { truncateToVisualLines } from "../modes/components/visual-truncate"; import { getMarkdownTheme, type Theme } from "../modes/theme/theme"; @@ -47,6 +47,38 @@ function formatJsonScalar(value: unknown): string { return "[object]"; } +/** Cap per `display()` value sent back to the model. */ +const MAX_DISPLAY_TEXT_BYTES = 8000; + +function formatDisplayJsonForText(value: unknown): string { + let text: string; + try { + text = JSON.stringify(value, null, 2) ?? String(value); + } catch { + text = String(value); + } + if (text.length > MAX_DISPLAY_TEXT_BYTES) { + text = `${text.slice(0, MAX_DISPLAY_TEXT_BYTES)}\n\u2026 (${text.length - MAX_DISPLAY_TEXT_BYTES} chars truncated)`; + } + return text; +} + +/** + * Format display() JSON values into text the model can see. Images are surfaced + * separately as ImageContent so the model can actually inspect them; this helper + * intentionally does not touch images. + */ +function formatDisplayOutputsForText(outputs: EvalDisplayOutput[]): string { + const chunks: string[] = []; + let displayIndex = 0; + for (const output of outputs) { + if (output.type !== "json") continue; + displayIndex++; + chunks.push(`display[${displayIndex}]:\n${formatDisplayJsonForText(output.data)}`); + } + return chunks.join("\n\n"); +} + function renderJsonTree(value: unknown, theme: Theme, expanded: boolean, maxDepth = expanded ? 6 : 2): string[] { const maxItems = expanded ? 20 : 5; @@ -370,13 +402,16 @@ export class EvalTool implements AgentTool { const durationMs = Date.now() - startTime; const cellStatusEvents: EvalStatusEvent[] = []; + const cellDisplayOutputs: EvalDisplayOutput[] = []; let cellHasMarkdown = false; for (const output of result.displayOutputs) { if (output.type === "json") { jsonOutputs.push(output.data); + cellDisplayOutputs.push(output); } if (output.type === "image") { images.push({ type: "image", data: output.data, mimeType: output.mimeType }); + cellDisplayOutputs.push(output); } if (output.type === "status") { statusEvents.push(output.event); @@ -387,7 +422,10 @@ export class EvalTool implements AgentTool { } } - const cellOutput = result.output.trim(); + const stdoutTrimmed = result.output.trim(); + const displayText = formatDisplayOutputsForText(cellDisplayOutputs); + const cellOutput = + stdoutTrimmed && displayText ? `${stdoutTrimmed}\n\n${displayText}` : stdoutTrimmed || displayText; cellResult.output = cellOutput; cellResult.exitCode = result.exitCode; cellResult.durationMs = durationMs; @@ -431,14 +469,13 @@ export class EvalTool implements AgentTool { languages, cells: cellResults, jsonOutputs: jsonOutputs.length > 0 ? jsonOutputs : undefined, - images: images.length > 0 ? images : undefined, statusEvents: statusEvents.length > 0 ? statusEvents : undefined, isError: true, }; if (notice) details.notice = notice; return toolResult(details) - .text(outputText) + .content([{ type: "text", text: outputText }, ...images]) .truncationFromSummary(summaryForMeta, { direction: "tail" }) .done(); } @@ -461,14 +498,13 @@ export class EvalTool implements AgentTool { languages, cells: cellResults, jsonOutputs: jsonOutputs.length > 0 ? jsonOutputs : undefined, - images: images.length > 0 ? images : undefined, statusEvents: statusEvents.length > 0 ? statusEvents : undefined, isError: true, }; if (notice) details.notice = notice; return toolResult(details) - .text(outputText) + .content([{ type: "text", text: outputText }, ...images]) .truncationFromSummary(summaryForMeta, { direction: "tail" }) .done(); } @@ -479,9 +515,12 @@ export class EvalTool implements AgentTool { const combinedOutput = cellOutputs.join("\n\n"); const abortSuffix = parsedInput.aborted ? `\n\n${ABORT_WARNING}` : ""; + const hasImages = images.length > 0; const outputText = - (combinedOutput || (jsonOutputs.length > 0 || images.length > 0 ? "(no text output)" : "(no output)")) + - abortSuffix; + (combinedOutput || + (hasImages + ? `(displayed ${images.length} image${images.length === 1 ? "" : "s"}; no text output)` + : "(no output)")) + abortSuffix; const summaryForMeta = await summarizeFinal(combinedOutput, finalizeOutput); const details: EvalToolDetails = { @@ -489,13 +528,12 @@ export class EvalTool implements AgentTool { languages, cells: cellResults, jsonOutputs: jsonOutputs.length > 0 ? jsonOutputs : undefined, - images: images.length > 0 ? images : undefined, statusEvents: statusEvents.length > 0 ? statusEvents : undefined, }; if (notice) details.notice = notice; return toolResult(details) - .text(outputText) + .content([{ type: "text", text: outputText }, ...images]) .truncationFromSummary(summaryForMeta, { direction: "tail" }) .done(); } finally { diff --git a/packages/coding-agent/test/tools/eval-display-text.test.ts b/packages/coding-agent/test/tools/eval-display-text.test.ts new file mode 100644 index 000000000..37acfdd7e --- /dev/null +++ b/packages/coding-agent/test/tools/eval-display-text.test.ts @@ -0,0 +1,137 @@ +import { afterEach, describe, expect, it, vi } from "bun:test"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import * as evalIndex from "@oh-my-pi/pi-coding-agent/eval"; +import * as pyKernel from "@oh-my-pi/pi-coding-agent/eval/py/kernel"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import { EvalTool } from "@oh-my-pi/pi-coding-agent/tools/eval"; + +function makeSession(): ToolSession { + return { + cwd: "/tmp/eval-test", + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => null, + settings: Settings.isolated(), + }; +} + +function baseResult(overrides: Record = {}) { + return { + output: "", + exitCode: 0, + cancelled: false, + truncated: false, + artifactId: undefined, + totalLines: 0, + totalBytes: 0, + outputLines: 0, + outputBytes: 0, + displayOutputs: [] as unknown[], + ...overrides, + }; +} + +describe("EvalTool display() text surfacing", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("includes display() JSON values in the text content the model sees", async () => { + vi.spyOn(pyKernel, "checkPythonKernelAvailability").mockResolvedValue({ ok: true }); + vi.spyOn(evalIndex.jsBackend, "execute").mockResolvedValue( + baseResult({ + displayOutputs: [{ type: "json", data: { stdout: "hi", exit_code: 0 } }], + }) as never, + ); + + const tool = new EvalTool(makeSession()); + const result = await tool.execute("call-display-json", { + input: "```js\ndisplay({ stdout: 'hi', exit_code: 0 });\n```\n", + }); + + const text = result.content.map(c => (c.type === "text" ? c.text : "")).join("\n"); + expect(text).toContain("display[1]"); + expect(text).toContain('"stdout": "hi"'); + expect(text).toContain('"exit_code": 0'); + expect(text).not.toBe("(no text output)"); + }); + + it("interleaves stdout text and display() JSON values", async () => { + vi.spyOn(pyKernel, "checkPythonKernelAvailability").mockResolvedValue({ ok: true }); + vi.spyOn(evalIndex.jsBackend, "execute").mockResolvedValue( + baseResult({ + output: "before\n", + displayOutputs: [{ type: "json", data: [1, 2, 3] }], + }) as never, + ); + + const tool = new EvalTool(makeSession()); + const result = await tool.execute("call-mixed", { + input: "```js\nprint('before'); display([1,2,3]);\n```\n", + }); + + const text = result.content.map(c => (c.type === "text" ? c.text : "")).join("\n"); + expect(text).toContain("before"); + expect(text.indexOf("before")).toBeLessThan(text.indexOf("display[1]")); + expect(text).toContain("[\n 1,\n 2,\n 3\n]"); + }); + + it("surfaces displayed images to the model as ImageContent blocks, not inlined base64", async () => { + vi.spyOn(pyKernel, "checkPythonKernelAvailability").mockResolvedValue({ ok: true }); + const base64 = Buffer.from([0, 1, 2, 3]).toString("base64"); + vi.spyOn(evalIndex.jsBackend, "execute").mockResolvedValue( + baseResult({ + displayOutputs: [{ type: "image", data: base64, mimeType: "image/png" }], + }) as never, + ); + + const tool = new EvalTool(makeSession()); + const result = await tool.execute("call-image", { + input: "```js\ndisplay({ type: 'image', data: '...', mimeType: 'image/png' });\n```\n", + }); + + const imageBlocks = result.content.filter(c => c.type === "image"); + expect(imageBlocks).toHaveLength(1); + expect(imageBlocks[0]).toMatchObject({ type: "image", data: base64, mimeType: "image/png" }); + + const textBlocks = result.content.filter(c => c.type === "text"); + const text = textBlocks.map(c => (c.type === "text" ? c.text : "")).join("\n"); + expect(text).not.toContain(base64); // base64 must not leak into text channel + expect(text).toMatch(/displayed 1 image/); + + // Image is in content, so details.images must be empty to avoid double-rendering. + expect(result.details?.images).toBeUndefined(); + }); + + it("still reports (no text output) when nothing was printed or displayed", async () => { + vi.spyOn(pyKernel, "checkPythonKernelAvailability").mockResolvedValue({ ok: true }); + vi.spyOn(evalIndex.jsBackend, "execute").mockResolvedValue(baseResult() as never); + + const tool = new EvalTool(makeSession()); + const result = await tool.execute("call-empty", { + input: "```js\nconst x = 1;\n```\n", + }); + + const text = result.content.map(c => (c.type === "text" ? c.text : "")).join("\n"); + expect(text).toContain("(no output)"); + }); + + it("truncates oversized display values rather than blasting the context", async () => { + vi.spyOn(pyKernel, "checkPythonKernelAvailability").mockResolvedValue({ ok: true }); + const huge = "x".repeat(20000); + vi.spyOn(evalIndex.jsBackend, "execute").mockResolvedValue( + baseResult({ + displayOutputs: [{ type: "json", data: { payload: huge } }], + }) as never, + ); + + const tool = new EvalTool(makeSession()); + const result = await tool.execute("call-huge", { + input: "```js\ndisplay({ payload: 'x'.repeat(20000) });\n```\n", + }); + + const text = result.content.map(c => (c.type === "text" ? c.text : "")).join("\n"); + expect(text).toContain("chars truncated"); + expect(text.length).toBeLessThan(20000); + }); +});