Improve edit tool diff display with context-aware rendering

- Add generateDiffString() function in edit tool to create unified diffs with line numbers and 4 lines of context
- Store only the formatted diff string in tool result details instead of full file contents
- Update tool-execution renderer to parse and colorize the diff string
- Filter out message_update events from session saving to prevent verbose session files
- Add markdown nested list and table rendering tests
This commit is contained in:
Mario Zechner 2025-11-12 20:09:11 +01:00
parent 2f0f0a913e
commit c75f53f6f2
7 changed files with 584 additions and 64 deletions

View file

@ -337,8 +337,10 @@ export async function main(args: string[]) {
sessionManager.saveMessage(event.message);
}
// Log all events
sessionManager.saveEvent(event);
// Log all events except message_update (too verbose)
if (event.type !== "message_update") {
sessionManager.saveEvent(event);
}
});
// Determine mode: interactive if no messages provided

View file

@ -1,6 +1,7 @@
import * as os from "node:os";
import type { AgentTool } from "@mariozechner/pi-ai";
import { Type } from "@sinclair/typebox";
import * as Diff from "diff";
import { constants } from "fs";
import { access, readFile, writeFile } from "fs/promises";
import { resolve as resolvePath } from "path";
@ -18,6 +19,99 @@ function expandPath(filePath: string): string {
return filePath;
}
/**
* Generate a unified diff string with line numbers and context
*/
function generateDiffString(oldContent: string, newContent: string, contextLines = 4): string {
const parts = Diff.diffLines(oldContent, newContent);
const output: string[] = [];
const oldLines = oldContent.split("\n");
const newLines = newContent.split("\n");
const maxLineNum = Math.max(oldLines.length, newLines.length);
const lineNumWidth = String(maxLineNum).length;
let oldLineNum = 1;
let newLineNum = 1;
let lastWasChange = false;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
const raw = part.value.split("\n");
if (raw[raw.length - 1] === "") {
raw.pop();
}
if (part.added || part.removed) {
// Show the change
for (const line of raw) {
if (part.added) {
const lineNum = String(newLineNum).padStart(lineNumWidth, " ");
output.push(`+${lineNum} ${line}`);
newLineNum++;
} else {
// removed
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(`-${lineNum} ${line}`);
oldLineNum++;
}
}
lastWasChange = true;
} else {
// Context lines - only show a few before/after changes
const nextPartIsChange = i < parts.length - 1 && (parts[i + 1].added || parts[i + 1].removed);
if (lastWasChange || nextPartIsChange) {
// Show context
let linesToShow = raw;
let skipStart = 0;
let skipEnd = 0;
if (!lastWasChange) {
// Show only last N lines as leading context
skipStart = Math.max(0, raw.length - contextLines);
linesToShow = raw.slice(skipStart);
}
if (!nextPartIsChange && linesToShow.length > contextLines) {
// Show only first N lines as trailing context
skipEnd = linesToShow.length - contextLines;
linesToShow = linesToShow.slice(0, contextLines);
}
// Add ellipsis if we skipped lines at start
if (skipStart > 0) {
output.push(` ${"".padStart(lineNumWidth, " ")} ...`);
}
for (const line of linesToShow) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(` ${lineNum} ${line}`);
oldLineNum++;
newLineNum++;
}
// Add ellipsis if we skipped lines at end
if (skipEnd > 0) {
output.push(` ${"".padStart(lineNumWidth, " ")} ...`);
}
// Update line numbers for skipped lines
oldLineNum += skipStart + skipEnd;
newLineNum += skipStart + skipEnd;
} else {
// Skip these context lines entirely
oldLineNum += raw.length;
newLineNum += raw.length;
}
lastWasChange = false;
}
}
return output.join("\n");
}
const editSchema = Type.Object({
path: Type.String({ description: "Path to the file to edit (relative or absolute)" }),
oldText: Type.String({ description: "Exact text to find and replace (must match exactly)" }),
@ -37,7 +131,10 @@ export const editTool: AgentTool<typeof editSchema> = {
) => {
const absolutePath = resolvePath(expandPath(path));
return new Promise<{ content: Array<{ type: "text"; text: string }>; details: undefined }>((resolve, reject) => {
return new Promise<{
content: Array<{ type: "text"; text: string }>;
details: { diff: string } | undefined;
}>((resolve, reject) => {
// Check if already aborted
if (signal?.aborted) {
reject(new Error("Operation aborted"));
@ -148,7 +245,7 @@ export const editTool: AgentTool<typeof editSchema> = {
text: `Successfully replaced text in ${path}. Changed ${oldText.length} characters to ${newText.length} characters.`,
},
],
details: undefined,
details: { diff: generateDiffString(content, newContent) },
});
} catch (error: any) {
// Clean up abort handler

View file

@ -1,6 +1,7 @@
import * as os from "node:os";
import { Container, Spacer, Text } from "@mariozechner/pi-tui";
import chalk from "chalk";
import * as Diff from "diff";
/**
* Convert absolute path to tilde notation if it's in home directory
@ -21,36 +22,101 @@ function replaceTabs(text: string): string {
}
/**
* Generate a unified diff between old and new strings with line numbers
* Generate a unified diff with line numbers and context
*/
function generateDiff(oldStr: string, newStr: string): string {
// Split into lines
const parts = Diff.diffLines(oldStr, newStr);
const output: string[] = [];
// Calculate max line number for padding
const oldLines = oldStr.split("\n");
const newLines = newStr.split("\n");
const diff: string[] = [];
// Calculate line number padding (for alignment)
const maxLineNum = Math.max(oldLines.length, newLines.length);
const lineNumWidth = String(maxLineNum).length;
// Show old lines with line numbers
diff.push(chalk.red("- old:"));
for (let i = 0; i < oldLines.length; i++) {
const lineNum = String(i + 1).padStart(lineNumWidth, " ");
diff.push(chalk.red(`- ${chalk.dim(lineNum)} ${oldLines[i]}`));
const CONTEXT_LINES = 2; // Show 2 lines of context around changes
let oldLineNum = 1;
let newLineNum = 1;
let lastWasChange = false;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
const raw = part.value.split("\n");
if (raw[raw.length - 1] === "") {
raw.pop();
}
if (part.added || part.removed) {
// Show the change
for (const line of raw) {
if (part.added) {
const lineNum = String(newLineNum).padStart(lineNumWidth, " ");
output.push(chalk.green(`${lineNum} ${line}`));
newLineNum++;
} else {
// removed
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(chalk.red(`${lineNum} ${line}`));
oldLineNum++;
}
}
lastWasChange = true;
} else {
// Context lines - only show a few before/after changes
const isFirstPart = i === 0;
const isLastPart = i === parts.length - 1;
const nextPartIsChange = i < parts.length - 1 && (parts[i + 1].added || parts[i + 1].removed);
if (lastWasChange || nextPartIsChange || isFirstPart || isLastPart) {
// Show context
let linesToShow = raw;
let skipStart = 0;
let skipEnd = 0;
if (!isFirstPart && !lastWasChange) {
// Show only last N lines as leading context
skipStart = Math.max(0, raw.length - CONTEXT_LINES);
linesToShow = raw.slice(skipStart);
}
if (!isLastPart && !nextPartIsChange && linesToShow.length > CONTEXT_LINES) {
// Show only first N lines as trailing context
skipEnd = linesToShow.length - CONTEXT_LINES;
linesToShow = linesToShow.slice(0, CONTEXT_LINES);
}
// Add ellipsis if we skipped lines at start
if (skipStart > 0) {
output.push(chalk.dim(`${"".padStart(lineNumWidth, " ")} ...`));
}
for (const line of linesToShow) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(chalk.dim(`${lineNum} ${line}`));
oldLineNum++;
newLineNum++;
}
// Add ellipsis if we skipped lines at end
if (skipEnd > 0) {
output.push(chalk.dim(`${"".padStart(lineNumWidth, " ")} ...`));
}
// Update line numbers for skipped lines
oldLineNum += skipStart + skipEnd;
newLineNum += skipStart + skipEnd;
} else {
// Skip these context lines entirely
oldLineNum += raw.length;
newLineNum += raw.length;
}
lastWasChange = false;
}
}
diff.push("");
// Show new lines with line numbers
diff.push(chalk.green("+ new:"));
for (let i = 0; i < newLines.length; i++) {
const lineNum = String(i + 1).padStart(lineNumWidth, " ");
diff.push(chalk.green(`+ ${chalk.dim(lineNum)} ${newLines[i]}`));
}
return diff.join("\n");
return output.join("\n");
}
/**
@ -63,6 +129,7 @@ export class ToolExecutionComponent extends Container {
private result?: {
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
isError: boolean;
details?: any;
};
constructor(toolName: string, args: any) {
@ -83,6 +150,7 @@ export class ToolExecutionComponent extends Container {
updateResult(result: {
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
details?: any;
isError: boolean;
}): void {
this.result = result;
@ -183,9 +251,20 @@ export class ToolExecutionComponent extends Container {
const path = shortenPath(this.args?.file_path || this.args?.path || "");
text = chalk.bold("edit") + " " + (path ? chalk.cyan(path) : chalk.dim("..."));
// Show diff if we have old_string and new_string
if (this.args?.old_string && this.args?.new_string) {
text += "\n\n" + generateDiff(this.args.old_string, this.args.new_string);
// Show diff if available
if (this.result?.details?.diff) {
// Parse the diff string and apply colors
const diffLines = this.result.details.diff.split("\n");
const coloredLines = diffLines.map((line: string) => {
if (line.startsWith("+")) {
return chalk.green(line);
} else if (line.startsWith("-")) {
return chalk.red(line);
} else {
return chalk.dim(line);
}
});
text += "\n\n" + coloredLines.join("\n");
}
} else {
// Generic tool

View file

@ -288,15 +288,7 @@ export class TuiRenderer {
// Update the existing tool component with the result
const component = this.pendingTools.get(event.toolCallId);
if (component) {
// Update the component with the result
const content =
typeof event.result === "string"
? [{ type: "text" as const, text: event.result }]
: event.result.content;
component.updateResult({
content,
isError: event.isError,
});
component.updateResult(event.result);
this.pendingTools.delete(event.toolCallId);
this.ui.requestRender();
}
@ -388,16 +380,16 @@ export class TuiRenderer {
}
}
} else if (message.role === "toolResult") {
// Update existing tool execution component with results
const toolResultMsg = message as any;
const component = this.pendingTools.get(toolResultMsg.toolCallId);
// Update existing tool execution component with results ;
const component = this.pendingTools.get(message.toolCallId);
if (component) {
component.updateResult({
content: toolResultMsg.content,
isError: toolResultMsg.isError,
content: message.content,
details: message.details,
isError: message.isError,
});
// Remove from pending map since it's complete
this.pendingTools.delete(toolResultMsg.toolCallId);
this.pendingTools.delete(message.toolCallId);
}
}
}