fix(coding-agent): corrected plan review previews to re-append at chat tail on refresh

- Fixed plan review previews to re-append at the chat tail on refresh, keeping them adjacent to the active selector instead of updating off-screen. - Added comprehensive test coverage for plan review rendering behavior across multiple refresh cycles.
2026-04-07 22:38:49 +02:00
parent ff4ed130d3
commit e6d36e2d8a
3 changed files with 114 additions and 11 deletions
@@ -1,7 +1,6 @@
 # Changelog

 ## [Unreleased]
-
 ### Added

 - Chunk read formatting: `anchorStyle` (full / kind / bare), `read.anchorstyle` setting, and `chunked` flag on file display mode
@@ -35,6 +34,7 @@

 ### Fixed

+- Plan review previews now re-append at the chat tail on refresh, keeping them adjacent to the active selector instead of updating off-screen
 - `log_experiment` validates and reverts run-scoped file changes without clobbering unrelated dirty worktree state
 - Chunk edit targets that embed CRC in the selector (e.g. `fn_foo#ABCD`) parse correctly
 - Shell paths check errors before consuming chunk output (bash executor, config resolution)
@@ -779,17 +779,21 @@ export class InteractiveMode implements InteractiveModeContext {
 	}

 	#renderPlanPreview(planContent: string): void {
-		if (!this.#planReviewContainer) {
-			this.#planReviewContainer = new Container();
-			this.chatContainer.addChild(this.#planReviewContainer);
+		const planReviewContainer = this.#planReviewContainer ?? new Container();
+		if (this.#planReviewContainer) {
+			// Re-append the preview so repeated plan-review refreshes stay adjacent to the
+			// active selector instead of updating an older off-screen preview in place.
+			this.chatContainer.removeChild(this.#planReviewContainer);
 		}
-		this.#planReviewContainer.clear();
-		this.#planReviewContainer.addChild(new Spacer(1));
-		this.#planReviewContainer.addChild(new DynamicBorder());
-		this.#planReviewContainer.addChild(new Text(theme.bold(theme.fg("accent", "Plan Review")), 1, 1));
-		this.#planReviewContainer.addChild(new Spacer(1));
-		this.#planReviewContainer.addChild(new Markdown(planContent, 1, 1, getMarkdownTheme()));
-		this.#planReviewContainer.addChild(new DynamicBorder());
+		planReviewContainer.clear();
+		planReviewContainer.addChild(new Spacer(1));
+		planReviewContainer.addChild(new DynamicBorder());
+		planReviewContainer.addChild(new Text(theme.bold(theme.fg("accent", "Plan Review")), 1, 1));
+		planReviewContainer.addChild(new Spacer(1));
+		planReviewContainer.addChild(new Markdown(planContent, 1, 1, getMarkdownTheme()));
+		planReviewContainer.addChild(new DynamicBorder());
+		this.chatContainer.addChild(planReviewContainer);
+		this.#planReviewContainer = planReviewContainer;
 		this.ui.requestRender();
 	}

@@ -0,0 +1,99 @@
+import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
+import * as path from "node:path";
+import { Agent } from "@oh-my-pi/pi-agent-core";
+import { _resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
+import { resolveLocalUrlToPath } from "@oh-my-pi/pi-coding-agent/internal-urls";
+import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
+import { Text } from "@oh-my-pi/pi-tui";
+import { TempDir } from "@oh-my-pi/pi-utils";
+import { ModelRegistry } from "../src/config/model-registry";
+import { InteractiveMode } from "../src/modes/interactive-mode";
+import { AgentSession } from "../src/session/agent-session";
+import { AuthStorage } from "../src/session/auth-storage";
+import { SessionManager } from "../src/session/session-manager";
+
+describe("InteractiveMode plan review rendering", () => {
+	let tempDir: TempDir;
+	let authStorage: AuthStorage;
+	let session: AgentSession;
+	let mode: InteractiveMode;
+
+	beforeAll(() => {
+		initTheme();
+	});
+
+	beforeEach(async () => {
+		_resetSettingsForTest();
+		tempDir = TempDir.createSync("@pi-plan-review-");
+		await Settings.init({ inMemory: true, cwd: tempDir.path() });
+		authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
+		const modelRegistry = new ModelRegistry(authStorage);
+		const model = modelRegistry.find("anthropic", "claude-sonnet-4-5");
+		if (!model) {
+			throw new Error("Expected claude-sonnet-4-5 to exist in registry");
+		}
+
+		session = new AgentSession({
+			agent: new Agent({
+				initialState: {
+					model,
+					systemPrompt: "Test",
+					tools: [],
+					messages: [],
+				},
+			}),
+			sessionManager: SessionManager.create(tempDir.path(), tempDir.path()),
+			settings: Settings.isolated(),
+			modelRegistry,
+		});
+		mode = new InteractiveMode(session, "test");
+	});
+
+	afterEach(async () => {
+		vi.restoreAllMocks();
+		mode?.stop();
+		await session?.dispose();
+		authStorage?.close();
+		tempDir?.removeSync();
+		_resetSettingsForTest();
+	});
+
+	it("re-appends refreshed plan review previews at the chat tail", async () => {
+		const planFilePath = "local://PLAN.md";
+		const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, {
+			getArtifactsDir: () => session.sessionManager.getArtifactsDir(),
+			getSessionId: () => session.sessionManager.getSessionId(),
+		});
+		await Bun.write(resolvedPlanPath, "# First plan\n\nalpha");
+
+		mode.planModeEnabled = true;
+		mode.planModePlanFilePath = planFilePath;
+		vi.spyOn(mode, "showHookSelector").mockResolvedValue("Stay in plan mode");
+
+		await mode.handleExitPlanModeTool({
+			planFilePath,
+			planExists: true,
+			title: "PLAN",
+			finalPlanFilePath: "local://PLAN.md",
+		});
+
+		const firstPreview = mode.chatContainer.children.at(-1);
+		expect(firstPreview).toBeDefined();
+		expect(firstPreview!.render(120).join("\n")).toContain("First plan");
+
+		const marker = new Text("MARKER", 0, 0);
+		mode.chatContainer.addChild(marker);
+		await Bun.write(resolvedPlanPath, "# Second plan\n\nbeta");
+
+		await mode.handleExitPlanModeTool({
+			planFilePath,
+			planExists: true,
+			title: "PLAN",
+			finalPlanFilePath: "local://PLAN.md",
+		});
+
+		expect(mode.chatContainer.children.at(-1)).toBe(firstPreview);
+		expect(mode.chatContainer.children.at(-2)).toBe(marker);
+		expect(firstPreview!.render(120).join("\n")).toContain("Second plan");
+	});
+});