mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 23:01:30 +00:00
Show edit diff before tool execution (fixes #393)
- Extract diff computation from edit.ts into shared edit-diff.ts - ToolExecutionComponent computes and caches diff when args are complete - Diff is visible while permission hooks block, before tool executes
This commit is contained in:
parent
173b81bc04
commit
1ed009e2cf
4 changed files with 277 additions and 134 deletions
203
packages/coding-agent/src/core/tools/edit-diff.ts
Normal file
203
packages/coding-agent/src/core/tools/edit-diff.ts
Normal file
|
|
@ -0,0 +1,203 @@
|
|||
/**
|
||||
* Shared diff computation utilities for the edit tool.
|
||||
* Used by both edit.ts (for execution) and tool-execution.ts (for preview rendering).
|
||||
*/
|
||||
|
||||
import * as Diff from "diff";
|
||||
import { constants } from "fs";
|
||||
import { access, readFile } from "fs/promises";
|
||||
import { resolveToCwd } from "./path-utils.js";
|
||||
|
||||
export function detectLineEnding(content: string): "\r\n" | "\n" {
|
||||
const crlfIdx = content.indexOf("\r\n");
|
||||
const lfIdx = content.indexOf("\n");
|
||||
if (lfIdx === -1) return "\n";
|
||||
if (crlfIdx === -1) return "\n";
|
||||
return crlfIdx < lfIdx ? "\r\n" : "\n";
|
||||
}
|
||||
|
||||
export function normalizeToLF(text: string): string {
|
||||
return text.replace(/\r\n/g, "\n").replace(/\r/g, "\n");
|
||||
}
|
||||
|
||||
export function restoreLineEndings(text: string, ending: "\r\n" | "\n"): string {
|
||||
return ending === "\r\n" ? text.replace(/\n/g, "\r\n") : text;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a unified diff string with line numbers and context.
|
||||
* Returns both the diff string and the first changed line number (in the new file).
|
||||
*/
|
||||
export function generateDiffString(
|
||||
oldContent: string,
|
||||
newContent: string,
|
||||
contextLines = 4,
|
||||
): { diff: string; firstChangedLine: number | undefined } {
|
||||
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;
|
||||
let firstChangedLine: number | undefined;
|
||||
|
||||
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) {
|
||||
// Capture the first changed line (in the new file)
|
||||
if (firstChangedLine === undefined) {
|
||||
firstChangedLine = newLineNum;
|
||||
}
|
||||
|
||||
// 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, " ")} ...`);
|
||||
// Update line numbers for the skipped leading context
|
||||
oldLineNum += skipStart;
|
||||
newLineNum += skipStart;
|
||||
}
|
||||
|
||||
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 the skipped trailing context
|
||||
oldLineNum += skipEnd;
|
||||
newLineNum += skipEnd;
|
||||
}
|
||||
} else {
|
||||
// Skip these context lines entirely
|
||||
oldLineNum += raw.length;
|
||||
newLineNum += raw.length;
|
||||
}
|
||||
|
||||
lastWasChange = false;
|
||||
}
|
||||
}
|
||||
|
||||
return { diff: output.join("\n"), firstChangedLine };
|
||||
}
|
||||
|
||||
export interface EditDiffResult {
|
||||
diff: string;
|
||||
firstChangedLine: number | undefined;
|
||||
}
|
||||
|
||||
export interface EditDiffError {
|
||||
error: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute the diff for an edit operation without applying it.
|
||||
* Used for preview rendering in the TUI before the tool executes.
|
||||
*/
|
||||
export async function computeEditDiff(
|
||||
path: string,
|
||||
oldText: string,
|
||||
newText: string,
|
||||
cwd: string,
|
||||
): Promise<EditDiffResult | EditDiffError> {
|
||||
const absolutePath = resolveToCwd(path, cwd);
|
||||
|
||||
try {
|
||||
// Check if file exists and is readable
|
||||
try {
|
||||
await access(absolutePath, constants.R_OK);
|
||||
} catch {
|
||||
return { error: `File not found: ${path}` };
|
||||
}
|
||||
|
||||
// Read the file
|
||||
const content = await readFile(absolutePath, "utf-8");
|
||||
|
||||
const normalizedContent = normalizeToLF(content);
|
||||
const normalizedOldText = normalizeToLF(oldText);
|
||||
const normalizedNewText = normalizeToLF(newText);
|
||||
|
||||
// Check if old text exists
|
||||
if (!normalizedContent.includes(normalizedOldText)) {
|
||||
return {
|
||||
error: `Could not find the exact text in ${path}. The old text must match exactly including all whitespace and newlines.`,
|
||||
};
|
||||
}
|
||||
|
||||
// Count occurrences
|
||||
const occurrences = normalizedContent.split(normalizedOldText).length - 1;
|
||||
if (occurrences > 1) {
|
||||
return {
|
||||
error: `Found ${occurrences} occurrences of the text in ${path}. The text must be unique. Please provide more context to make it unique.`,
|
||||
};
|
||||
}
|
||||
|
||||
// Compute the new content
|
||||
const index = normalizedContent.indexOf(normalizedOldText);
|
||||
const normalizedNewContent =
|
||||
normalizedContent.substring(0, index) +
|
||||
normalizedNewText +
|
||||
normalizedContent.substring(index + normalizedOldText.length);
|
||||
|
||||
// Check if it would actually change anything
|
||||
if (normalizedContent === normalizedNewContent) {
|
||||
return {
|
||||
error: `No changes would be made to ${path}. The replacement produces identical content.`,
|
||||
};
|
||||
}
|
||||
|
||||
// Generate the diff
|
||||
return generateDiffString(normalizedContent, normalizedNewContent);
|
||||
} catch (err) {
|
||||
return { error: err instanceof Error ? err.message : String(err) };
|
||||
}
|
||||
}
|
||||
|
|
@ -1,132 +1,10 @@
|
|||
import type { AgentTool } from "@mariozechner/pi-agent-core";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import * as Diff from "diff";
|
||||
import { constants } from "fs";
|
||||
import { access, readFile, writeFile } from "fs/promises";
|
||||
import { detectLineEnding, generateDiffString, normalizeToLF, restoreLineEndings } from "./edit-diff.js";
|
||||
import { resolveToCwd } from "./path-utils.js";
|
||||
|
||||
function detectLineEnding(content: string): "\r\n" | "\n" {
|
||||
const crlfIdx = content.indexOf("\r\n");
|
||||
const lfIdx = content.indexOf("\n");
|
||||
if (lfIdx === -1) return "\n";
|
||||
if (crlfIdx === -1) return "\n";
|
||||
return crlfIdx < lfIdx ? "\r\n" : "\n";
|
||||
}
|
||||
|
||||
function normalizeToLF(text: string): string {
|
||||
return text.replace(/\r\n/g, "\n").replace(/\r/g, "\n");
|
||||
}
|
||||
|
||||
function restoreLineEndings(text: string, ending: "\r\n" | "\n"): string {
|
||||
return ending === "\r\n" ? text.replace(/\n/g, "\r\n") : text;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a unified diff string with line numbers and context
|
||||
* Returns both the diff string and the first changed line number (in the new file)
|
||||
*/
|
||||
function generateDiffString(
|
||||
oldContent: string,
|
||||
newContent: string,
|
||||
contextLines = 4,
|
||||
): { diff: string; firstChangedLine: number | undefined } {
|
||||
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;
|
||||
let firstChangedLine: number | undefined;
|
||||
|
||||
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) {
|
||||
// Capture the first changed line (in the new file)
|
||||
if (firstChangedLine === undefined) {
|
||||
firstChangedLine = newLineNum;
|
||||
}
|
||||
|
||||
// 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, " ")} ...`);
|
||||
// Update line numbers for the skipped leading context
|
||||
oldLineNum += skipStart;
|
||||
newLineNum += skipStart;
|
||||
}
|
||||
|
||||
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 the skipped trailing context
|
||||
oldLineNum += skipEnd;
|
||||
newLineNum += skipEnd;
|
||||
}
|
||||
} else {
|
||||
// Skip these context lines entirely
|
||||
oldLineNum += raw.length;
|
||||
newLineNum += raw.length;
|
||||
}
|
||||
|
||||
lastWasChange = false;
|
||||
}
|
||||
}
|
||||
|
||||
return { diff: output.join("\n"), firstChangedLine };
|
||||
}
|
||||
|
||||
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)" }),
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {
|
|||
} from "@mariozechner/pi-tui";
|
||||
import stripAnsi from "strip-ansi";
|
||||
import type { CustomTool } from "../../../core/custom-tools/types.js";
|
||||
import { computeEditDiff, type EditDiffError, type EditDiffResult } from "../../../core/tools/edit-diff.js";
|
||||
import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize } from "../../../core/tools/truncate.js";
|
||||
import { sanitizeBinaryOutput } from "../../../utils/shell.js";
|
||||
import { getLanguageFromPath, highlightCode, theme } from "../theme/theme.js";
|
||||
|
|
@ -58,11 +59,15 @@ export class ToolExecutionComponent extends Container {
|
|||
private isPartial = true;
|
||||
private customTool?: CustomTool;
|
||||
private ui: TUI;
|
||||
private cwd: string;
|
||||
private result?: {
|
||||
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
|
||||
isError: boolean;
|
||||
details?: any;
|
||||
};
|
||||
// Cached edit diff preview (computed when args arrive, before tool executes)
|
||||
private editDiffPreview?: EditDiffResult | EditDiffError;
|
||||
private editDiffArgsKey?: string; // Track which args the preview is for
|
||||
|
||||
constructor(
|
||||
toolName: string,
|
||||
|
|
@ -70,6 +75,7 @@ export class ToolExecutionComponent extends Container {
|
|||
options: ToolExecutionOptions = {},
|
||||
customTool: CustomTool | undefined,
|
||||
ui: TUI,
|
||||
cwd: string = process.cwd(),
|
||||
) {
|
||||
super();
|
||||
this.toolName = toolName;
|
||||
|
|
@ -77,6 +83,7 @@ export class ToolExecutionComponent extends Container {
|
|||
this.showImages = options.showImages ?? true;
|
||||
this.customTool = customTool;
|
||||
this.ui = ui;
|
||||
this.cwd = cwd;
|
||||
|
||||
this.addChild(new Spacer(1));
|
||||
|
||||
|
|
@ -98,6 +105,47 @@ export class ToolExecutionComponent extends Container {
|
|||
this.updateDisplay();
|
||||
}
|
||||
|
||||
/**
|
||||
* Signal that args are complete (tool is about to execute).
|
||||
* This triggers diff computation for edit tool.
|
||||
*/
|
||||
setArgsComplete(): void {
|
||||
this.maybeComputeEditDiff();
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute edit diff preview when we have complete args.
|
||||
* This runs async and updates display when done.
|
||||
*/
|
||||
private maybeComputeEditDiff(): void {
|
||||
if (this.toolName !== "edit") return;
|
||||
|
||||
const path = this.args?.path;
|
||||
const oldText = this.args?.oldText;
|
||||
const newText = this.args?.newText;
|
||||
|
||||
// Need all three params to compute diff
|
||||
if (!path || oldText === undefined || newText === undefined) return;
|
||||
|
||||
// Create a key to track which args this computation is for
|
||||
const argsKey = JSON.stringify({ path, oldText, newText });
|
||||
|
||||
// Skip if we already computed for these exact args
|
||||
if (this.editDiffArgsKey === argsKey) return;
|
||||
|
||||
this.editDiffArgsKey = argsKey;
|
||||
|
||||
// Compute diff async
|
||||
computeEditDiff(path, oldText, newText, this.cwd).then((result) => {
|
||||
// Only update if args haven't changed since we started
|
||||
if (this.editDiffArgsKey === argsKey) {
|
||||
this.editDiffPreview = result;
|
||||
this.updateDisplay();
|
||||
this.ui.requestRender();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
updateResult(
|
||||
result: {
|
||||
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
|
||||
|
|
@ -415,22 +463,31 @@ export class ToolExecutionComponent extends Container {
|
|||
const rawPath = this.args?.file_path || this.args?.path || "";
|
||||
const path = shortenPath(rawPath);
|
||||
|
||||
// Build path display, appending :line if we have a successful result with line info
|
||||
// Build path display, appending :line if we have diff info
|
||||
let pathDisplay = path ? theme.fg("accent", path) : theme.fg("toolOutput", "...");
|
||||
if (this.result && !this.result.isError && this.result.details?.firstChangedLine) {
|
||||
pathDisplay += theme.fg("warning", `:${this.result.details.firstChangedLine}`);
|
||||
const firstChangedLine =
|
||||
(this.editDiffPreview && "firstChangedLine" in this.editDiffPreview
|
||||
? this.editDiffPreview.firstChangedLine
|
||||
: undefined) ||
|
||||
(this.result && !this.result.isError ? this.result.details?.firstChangedLine : undefined);
|
||||
if (firstChangedLine) {
|
||||
pathDisplay += theme.fg("warning", `:${firstChangedLine}`);
|
||||
}
|
||||
|
||||
text = `${theme.fg("toolTitle", theme.bold("edit"))} ${pathDisplay}`;
|
||||
|
||||
if (this.result) {
|
||||
if (this.result.isError) {
|
||||
const errorText = this.getTextOutput();
|
||||
if (errorText) {
|
||||
text += `\n\n${theme.fg("error", errorText)}`;
|
||||
}
|
||||
} else if (this.result.details?.diff) {
|
||||
text += `\n\n${renderDiff(this.result.details.diff, { filePath: rawPath })}`;
|
||||
if (this.result?.isError) {
|
||||
// Show error from result
|
||||
const errorText = this.getTextOutput();
|
||||
if (errorText) {
|
||||
text += `\n\n${theme.fg("error", errorText)}`;
|
||||
}
|
||||
} else if (this.editDiffPreview) {
|
||||
// Use cached diff preview (works both before and after execution)
|
||||
if ("error" in this.editDiffPreview) {
|
||||
text += `\n\n${theme.fg("error", this.editDiffPreview.error)}`;
|
||||
} else if (this.editDiffPreview.diff) {
|
||||
text += `\n\n${renderDiff(this.editDiffPreview.diff, { filePath: rawPath })}`;
|
||||
}
|
||||
}
|
||||
} else if (this.toolName === "ls") {
|
||||
|
|
|
|||
|
|
@ -902,6 +902,11 @@ export class InteractiveMode {
|
|||
});
|
||||
}
|
||||
this.pendingTools.clear();
|
||||
} else {
|
||||
// Args are now complete - trigger diff computation for edit tools
|
||||
for (const [, component] of this.pendingTools.entries()) {
|
||||
component.setArgsComplete();
|
||||
}
|
||||
}
|
||||
this.streamingComponent = undefined;
|
||||
this.streamingMessage = undefined;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue