From c2c916b16da2bbf0f533b1fd07dda75b443d88b7 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 20 Mar 2026 09:13:14 -0700 Subject: [PATCH] fix(cli): address bot review feedback on session export - Handle legacy assistant string content in extractAssistantText (Codex P1) - Escape triple backticks in tool result/call code blocks (Codex P2) - Use runtime.log instead of console.log (Greptile P2) - Use os.homedir() instead of process.env.HOME for tilde expansion (Greptile P2) - Include tool name in tool call summary tags (Greptile P2) - Improve truncation test assertion directness (Greptile P2) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/commands/session-export.test.ts | 57 ++++++++++++++++++++++++++--- src/commands/session-export.ts | 29 +++++++++++---- 2 files changed, 73 insertions(+), 13 deletions(-) diff --git a/src/commands/session-export.test.ts b/src/commands/session-export.test.ts index 95068a4c8c0..ee50a42880a 100644 --- a/src/commands/session-export.test.ts +++ b/src/commands/session-export.test.ts @@ -124,7 +124,7 @@ describe("sessionEntriesToMarkdown", () => { expect(md).toContain("**Messages:** 2"); }); - it("collapses tool calls into details blocks", () => { + it("collapses tool calls into details blocks with tool name", () => { const entries = [ makeAssistantEntry("Checking...", 1742470260000, [ { name: "weather.get", arguments: { city: "SF" } }, @@ -132,7 +132,7 @@ describe("sessionEntriesToMarkdown", () => { ]; const md = sessionEntriesToMarkdown(makeHeader(), entries); expect(md).toContain("
"); - expect(md).toContain("Tool call"); + expect(md).toContain("Tool call: weather.get"); expect(md).toContain("weather.get"); expect(md).toContain("
"); }); @@ -151,13 +151,12 @@ describe("sessionEntriesToMarkdown", () => { expect(md).toContain("(error)"); }); - it("truncates long tool results", () => { + it("truncates long tool results with ellipsis", () => { const longText = "x".repeat(1000); const entries = [makeToolResultEntry("read", longText)]; const md = sessionEntriesToMarkdown(makeHeader(), entries); expect(md).toContain("..."); - // Should not contain the full 1000 chars - expect(md.length).toBeLessThan(longText.length); + expect(md).not.toContain("x".repeat(501)); }); it("handles compaction entries", () => { @@ -187,6 +186,54 @@ describe("sessionEntriesToMarkdown", () => { expect(md).toContain("# Session: unknown"); }); + it("handles legacy assistant messages with string content", () => { + const entry: SessionMessageEntry = { + type: "message", + id: "msg-legacy", + parentId: null, + timestamp: new Date(1742470260000).toISOString(), + message: { + role: "assistant" as const, + content: "This is a legacy string response" as never, + api: "anthropic-messages" as const, + provider: "anthropic", + model: "claude-sonnet-4-5-20250514", + usage: { + input: 100, + output: 50, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 150, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop" as const, + timestamp: 1742470260000, + }, + }; + const md = sessionEntriesToMarkdown(makeHeader(), [entry]); + expect(md).toContain("**Assistant**"); + expect(md).toContain("This is a legacy string response"); + }); + + it("escapes triple backticks in tool result output", () => { + const entries = [makeToolResultEntry("bash", "output:\n```\nsome code\n```\nend")]; + const md = sessionEntriesToMarkdown(makeHeader(), entries); + // The inner backticks from the tool output should be escaped + expect(md).toContain("\\`\\`\\`"); + expect(md).not.toContain("output:\n```\n"); + }); + + it("truncates long tool results to 500 characters", () => { + const longText = "x".repeat(1000); + const entries = [makeToolResultEntry("read", longText)]; + const md = sessionEntriesToMarkdown(makeHeader(), entries); + expect(md).toContain("..."); + const codeBlockMatch = md.match(/```\n([\s\S]*?)\n```/); + expect(codeBlockMatch).toBeTruthy(); + // 500 chars + "..." = 503 + expect(codeBlockMatch![1]!.length).toBeLessThanOrEqual(503); + }); + it("handles user message with image content", () => { const entry: SessionMessageEntry = { type: "message", diff --git a/src/commands/session-export.ts b/src/commands/session-export.ts index 78e03dbac96..f42b2edab85 100644 --- a/src/commands/session-export.ts +++ b/src/commands/session-export.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import type { AssistantMessage, ToolResultMessage, UserMessage } from "@mariozechner/pi-ai"; import type { @@ -51,16 +52,24 @@ function extractUserText(msg: UserMessage): string { .join("\n"); } -function extractAssistantText(msg: AssistantMessage): { text: string; toolCalls: string[] } { +function extractAssistantText(msg: AssistantMessage): { + text: string; + toolCalls: Array<{ name: string; body: string }>; +} { + // Legacy messages may have string content instead of an array + if (typeof msg.content === "string") { + return { text: msg.content, toolCalls: [] }; + } + const textParts: string[] = []; - const toolCalls: string[] = []; + const toolCalls: Array<{ name: string; body: string }> = []; for (const block of msg.content) { if (block.type === "text") { textParts.push(block.text); } else if (block.type === "toolCall") { const argsStr = JSON.stringify(block.arguments, null, 2); - toolCalls.push(`${block.name}(${argsStr})`); + toolCalls.push({ name: block.name, body: `${block.name}(${argsStr})` }); } // Skip thinking blocks in export } @@ -83,6 +92,10 @@ function extractToolResultText(msg: ToolResultMessage): string { .join("\n"); } +function escapeCodeFence(text: string): string { + return text.replace(/```/g, "\\`\\`\\`"); +} + export function sessionEntriesToMarkdown( header: SessionHeader | null, entries: PiSessionEntry[], @@ -132,10 +145,10 @@ export function sessionEntriesToMarkdown( for (const tc of toolCalls) { lines.push(""); lines.push("
"); - lines.push(`Tool call`); + lines.push(`Tool call: ${tc.name}`); lines.push(""); lines.push("```"); - lines.push(tc); + lines.push(escapeCodeFence(tc.body)); lines.push("```"); lines.push(""); lines.push("
"); @@ -151,7 +164,7 @@ export function sessionEntriesToMarkdown( ); lines.push(""); lines.push("```"); - lines.push(truncated); + lines.push(escapeCodeFence(truncated)); lines.push("```"); lines.push(""); lines.push(""); @@ -207,14 +220,14 @@ export async function sessionExportCommand( if (output) { const outputPath = path.resolve( - output.startsWith("~") ? output.replace("~", process.env.HOME ?? "") : output, + output.startsWith("~") ? output.replace("~", os.homedir()) : output, ); const outputDir = path.dirname(outputPath); if (!fs.existsSync(outputDir)) { fs.mkdirSync(outputDir, { recursive: true }); } fs.writeFileSync(outputPath, result, "utf-8"); - console.log(`Exported session "${sessionKey}" to ${outputPath}`); + runtime.log(`Exported session "${sessionKey}" to ${outputPath}`); } else { process.stdout.write(result); }