fix(coding-agent): avoid full redraw on large edit results closes #2664
This commit is contained in:
@@ -6,6 +6,10 @@
|
||||
|
||||
- Added `ctx.signal` to `ExtensionContext` and wired it to the active agent turn so extension handlers can forward cancellation into nested model calls, `fetch()`, and other abort-aware work ([#2660](https://github.com/badlogic/pi-mono/issues/2660))
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed edit tool TUI rendering to defer large multi-edit diffs to the settled result, avoiding full-screen redraws when the tool completes ([#2664](https://github.com/badlogic/pi-mono/issues/2664))
|
||||
|
||||
## [0.63.1] - 2026-03-27
|
||||
|
||||
### Added
|
||||
|
||||
@@ -7,11 +7,8 @@ import { renderDiff } from "../../modes/interactive/components/diff.js";
|
||||
import type { ToolDefinition } from "../extensions/types.js";
|
||||
import {
|
||||
applyEditsToNormalizedContent,
|
||||
computeEditsDiff,
|
||||
detectLineEnding,
|
||||
type Edit,
|
||||
type EditDiffError,
|
||||
type EditDiffResult,
|
||||
generateDiffString,
|
||||
normalizeToLF,
|
||||
restoreLineEndings,
|
||||
@@ -22,10 +19,7 @@ import { resolveToCwd } from "./path-utils.js";
|
||||
import { invalidArgText, shortenPath, str } from "./render-utils.js";
|
||||
import { wrapToolDefinition } from "./tool-definition-wrapper.js";
|
||||
|
||||
type EditRenderState = {
|
||||
argsKey?: string;
|
||||
preview?: EditDiffResult | EditDiffError;
|
||||
};
|
||||
type EditRenderState = Record<string, never>;
|
||||
|
||||
const replaceEditSchema = Type.Object(
|
||||
{
|
||||
@@ -102,51 +96,19 @@ type RenderableEditArgs = {
|
||||
newText?: string;
|
||||
};
|
||||
|
||||
function getRenderablePreviewInput(args: RenderableEditArgs | undefined): { path: string; edits: Edit[] } | null {
|
||||
if (!args || typeof args.path !== "string") {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (
|
||||
Array.isArray(args.edits) &&
|
||||
args.edits.length > 0 &&
|
||||
args.edits.every((edit) => typeof edit?.oldText === "string" && typeof edit?.newText === "string")
|
||||
) {
|
||||
return { path: args.path, edits: args.edits };
|
||||
}
|
||||
|
||||
if (typeof args.oldText === "string" && typeof args.newText === "string") {
|
||||
return { path: args.path, edits: [{ oldText: args.oldText, newText: args.newText }] };
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
function formatEditCall(
|
||||
args: RenderableEditArgs | undefined,
|
||||
state: EditRenderState,
|
||||
theme: typeof import("../../modes/interactive/theme/theme.js").theme,
|
||||
): string {
|
||||
const invalidArg = invalidArgText(theme);
|
||||
const rawPath = str(args?.file_path ?? args?.path);
|
||||
const path = rawPath !== null ? shortenPath(rawPath) : null;
|
||||
const pathDisplay = path === null ? invalidArg : path ? theme.fg("accent", path) : theme.fg("toolOutput", "...");
|
||||
let text = `${theme.fg("toolTitle", theme.bold("edit"))} ${pathDisplay}`;
|
||||
|
||||
if (state.preview) {
|
||||
if ("error" in state.preview) {
|
||||
text += `\n\n${theme.fg("error", state.preview.error)}`;
|
||||
} else if (state.preview.diff) {
|
||||
text += `\n\n${renderDiff(state.preview.diff, { filePath: rawPath ?? undefined })}`;
|
||||
}
|
||||
}
|
||||
|
||||
return text;
|
||||
return `${theme.fg("toolTitle", theme.bold("edit"))} ${pathDisplay}`;
|
||||
}
|
||||
|
||||
function formatEditResult(
|
||||
args: RenderableEditArgs | undefined,
|
||||
state: EditRenderState,
|
||||
result: {
|
||||
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
|
||||
details?: EditToolDetails;
|
||||
@@ -160,19 +122,14 @@ function formatEditResult(
|
||||
.filter((c) => c.type === "text")
|
||||
.map((c) => c.text || "")
|
||||
.join("\n");
|
||||
let previewError: string | undefined;
|
||||
if (state.preview && "error" in state.preview) {
|
||||
previewError = state.preview.error;
|
||||
}
|
||||
if (!errorText || errorText === previewError) {
|
||||
if (!errorText) {
|
||||
return undefined;
|
||||
}
|
||||
return `\n${theme.fg("error", errorText)}`;
|
||||
}
|
||||
|
||||
const previewDiff = state.preview && !("error" in state.preview) ? state.preview.diff : undefined;
|
||||
const resultDiff = result.details?.diff;
|
||||
if (!resultDiff || resultDiff === previewDiff) {
|
||||
if (!resultDiff) {
|
||||
return undefined;
|
||||
}
|
||||
return `\n${renderDiff(resultDiff, { filePath: rawPath ?? undefined })}`;
|
||||
@@ -307,27 +264,12 @@ export function createEditToolDefinition(
|
||||
);
|
||||
},
|
||||
renderCall(args, theme, context) {
|
||||
if (context.argsComplete) {
|
||||
const previewInput = getRenderablePreviewInput(args as RenderableEditArgs);
|
||||
if (previewInput) {
|
||||
const argsKey = JSON.stringify({ path: previewInput.path, edits: previewInput.edits });
|
||||
if (context.state.argsKey !== argsKey) {
|
||||
context.state.argsKey = argsKey;
|
||||
computeEditsDiff(previewInput.path, previewInput.edits, context.cwd).then((preview) => {
|
||||
if (context.state.argsKey === argsKey) {
|
||||
context.state.preview = preview;
|
||||
context.invalidate();
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
const text = (context.lastComponent as Text | undefined) ?? new Text("", 0, 0);
|
||||
text.setText(formatEditCall(args, context.state, theme));
|
||||
text.setText(formatEditCall(args, theme));
|
||||
return text;
|
||||
},
|
||||
renderResult(result, _options, theme, context) {
|
||||
const output = formatEditResult(context.args, context.state, result as any, theme, context.isError);
|
||||
const output = formatEditResult(context.args, result as any, theme, context.isError);
|
||||
if (!output) {
|
||||
const component = (context.lastComponent as Container | undefined) ?? new Container();
|
||||
component.clear();
|
||||
|
||||
126
packages/coding-agent/test/edit-tool-no-full-redraw.test.ts
Normal file
126
packages/coding-agent/test/edit-tool-no-full-redraw.test.ts
Normal file
@@ -0,0 +1,126 @@
|
||||
import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { Container, type Terminal, Text, TUI } from "@mariozechner/pi-tui";
|
||||
import { afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { createEditToolDefinition } from "../src/core/tools/edit.js";
|
||||
import { computeEditsDiff, type Edit } from "../src/core/tools/edit-diff.js";
|
||||
import { ToolExecutionComponent } from "../src/modes/interactive/components/tool-execution.js";
|
||||
import { initTheme } from "../src/modes/interactive/theme/theme.js";
|
||||
|
||||
class FakeTerminal implements Terminal {
|
||||
columns = 80;
|
||||
rows = 24;
|
||||
kittyProtocolActive = true;
|
||||
writes: string[] = [];
|
||||
|
||||
start(): void {}
|
||||
stop(): void {}
|
||||
async drainInput(): Promise<void> {}
|
||||
write(data: string): void {
|
||||
this.writes.push(data);
|
||||
}
|
||||
moveBy(_lines: number): void {}
|
||||
hideCursor(): void {}
|
||||
showCursor(): void {}
|
||||
clearLine(): void {}
|
||||
clearFromCursor(): void {}
|
||||
clearScreen(): void {}
|
||||
setTitle(_title: string): void {}
|
||||
|
||||
get fullClearCount(): number {
|
||||
return this.writes.filter((write) => write.includes("\x1b[2J\x1b[H\x1b[3J")).length;
|
||||
}
|
||||
}
|
||||
|
||||
async function waitForRender(): Promise<void> {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
}
|
||||
|
||||
function createLargeEdits(lines: string[]): Edit[] {
|
||||
const targets = [50, 150, 250, 350, 450, 550, 650, 750, 850, 950];
|
||||
return targets.map((lineNumber) => ({
|
||||
oldText: `${lines[lineNumber - 1]}\n${lines[lineNumber]}\n${lines[lineNumber + 1]}`,
|
||||
newText: `${lines[lineNumber - 1]}\n${lines[lineNumber]} changed\n${lines[lineNumber + 1]}`,
|
||||
}));
|
||||
}
|
||||
|
||||
describe("edit tool TUI rendering", () => {
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
beforeAll(() => {
|
||||
initTheme("dark");
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
it("renders the large diff only in the settled result without triggering a full TUI redraw", async () => {
|
||||
const dir = await mkdtemp(join(tmpdir(), "pi-edit-redraw-"));
|
||||
tempDirs.push(dir);
|
||||
const filePath = join(dir, "large-edit.txt");
|
||||
await writeFile(
|
||||
filePath,
|
||||
`${Array.from({ length: 1000 }, (_, i) => `line ${i}`).join("\n")}
|
||||
`,
|
||||
"utf8",
|
||||
);
|
||||
const lines = (await readFile(filePath, "utf8")).trimEnd().split("\n");
|
||||
const edits = createLargeEdits(lines);
|
||||
const diff = await computeEditsDiff(filePath, edits, process.cwd());
|
||||
if ("error" in diff) {
|
||||
throw new Error(diff.error);
|
||||
}
|
||||
|
||||
const terminal = new FakeTerminal();
|
||||
const tui = new TUI(terminal);
|
||||
const root = new Container();
|
||||
for (let i = 0; i < 200; i++) {
|
||||
root.addChild(new Text(`history ${i}`, 0, 0));
|
||||
}
|
||||
|
||||
const component = new ToolExecutionComponent(
|
||||
"edit",
|
||||
"tool-call-1",
|
||||
{ path: filePath, edits },
|
||||
{},
|
||||
createEditToolDefinition(process.cwd()),
|
||||
tui,
|
||||
process.cwd(),
|
||||
);
|
||||
root.addChild(component);
|
||||
tui.addChild(root);
|
||||
tui.start();
|
||||
await waitForRender();
|
||||
|
||||
component.setArgsComplete();
|
||||
tui.requestRender();
|
||||
await waitForRender();
|
||||
|
||||
const callOnlyRender = component.render(80).join("\n");
|
||||
expect(callOnlyRender).toContain("edit");
|
||||
expect(callOnlyRender).not.toContain("line 50 changed");
|
||||
expect(callOnlyRender).not.toContain("+ 51");
|
||||
|
||||
const redrawsBeforeResult = tui.fullRedraws;
|
||||
const clearsBeforeResult = terminal.fullClearCount;
|
||||
component.updateResult(
|
||||
{
|
||||
content: [{ type: "text", text: `Successfully replaced ${edits.length} block(s) in ${filePath}.` }],
|
||||
details: diff,
|
||||
isError: false,
|
||||
},
|
||||
false,
|
||||
);
|
||||
tui.requestRender();
|
||||
await waitForRender();
|
||||
|
||||
expect(tui.fullRedraws).toBe(redrawsBeforeResult);
|
||||
expect(terminal.fullClearCount).toBe(clearsBeforeResult);
|
||||
|
||||
const settledRender = component.render(80).join("\n");
|
||||
expect(settledRender).toContain("line 50 changed");
|
||||
expect(settledRender).toContain("line 950 changed");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user