Merge pull request #4164 from deseltrus/fix/tui-render-duplication-perf
fix(tui): eliminate pinned output duplication and reduce render overhead
This commit is contained in:
commit
21c3f54bdb
6 changed files with 147 additions and 9 deletions
|
|
@ -0,0 +1,73 @@
|
|||
import assert from "node:assert/strict";
|
||||
import { describe, it, mock } from "node:test";
|
||||
|
||||
import { DynamicBorder } from "./dynamic-border.js";
|
||||
|
||||
function makeTUI() {
|
||||
return {
|
||||
renderCount: 0,
|
||||
requestRender() {
|
||||
this.renderCount++;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("DynamicBorder spinner", () => {
|
||||
it("suppresses standalone render when an external render occurred recently", () => {
|
||||
const border = new DynamicBorder((s) => s);
|
||||
const tui = makeTUI();
|
||||
|
||||
border.startSpinner(tui as any, (s) => s);
|
||||
// startSpinner calls requestRender once immediately
|
||||
assert.equal(tui.renderCount, 1, "initial render on startSpinner");
|
||||
|
||||
// Simulate an externally-triggered render (e.g. from streaming)
|
||||
border.render(80);
|
||||
|
||||
// Access the private interval callback by advancing the timer
|
||||
// Instead, we directly test the render-batching logic:
|
||||
// After render() sets lastExternalRender, a spinner tick within 200ms
|
||||
// should NOT call requestRender.
|
||||
const anyBorder = border as any;
|
||||
assert.ok(
|
||||
Date.now() - anyBorder.lastExternalRender < 200,
|
||||
"lastExternalRender should be recent after render()",
|
||||
);
|
||||
|
||||
border.stopSpinner();
|
||||
});
|
||||
|
||||
it("triggers standalone render when no external render occurred recently", async () => {
|
||||
const border = new DynamicBorder((s) => s);
|
||||
const tui = makeTUI();
|
||||
|
||||
// Set lastExternalRender to a time well in the past
|
||||
const anyBorder = border as any;
|
||||
anyBorder.lastExternalRender = 0;
|
||||
|
||||
border.startSpinner(tui as any, (s) => s);
|
||||
const initialCount = tui.renderCount;
|
||||
|
||||
// Wait for one spinner tick (200ms interval + buffer)
|
||||
await new Promise((r) => setTimeout(r, 250));
|
||||
|
||||
assert.ok(
|
||||
tui.renderCount > initialCount,
|
||||
"spinner should trigger requestRender when no recent external render",
|
||||
);
|
||||
|
||||
border.stopSpinner();
|
||||
});
|
||||
|
||||
it("updates lastExternalRender on each render() call", () => {
|
||||
const border = new DynamicBorder((s) => s);
|
||||
const anyBorder = border as any;
|
||||
|
||||
const before = Date.now();
|
||||
border.render(80);
|
||||
const after = Date.now();
|
||||
|
||||
assert.ok(anyBorder.lastExternalRender >= before);
|
||||
assert.ok(anyBorder.lastExternalRender <= after);
|
||||
});
|
||||
});
|
||||
|
|
@ -17,6 +17,7 @@ export class DynamicBorder implements Component {
|
|||
private spinnerIndex = 0;
|
||||
private spinnerInterval: NodeJS.Timeout | null = null;
|
||||
private spinnerColorFn?: (str: string) => string;
|
||||
private lastExternalRender = 0;
|
||||
|
||||
constructor(color: (str: string) => string = (str) => {
|
||||
try { return theme.fg("border", str); } catch { return str; }
|
||||
|
|
@ -31,7 +32,7 @@ export class DynamicBorder implements Component {
|
|||
|
||||
/**
|
||||
* Start an animated spinner that prepends to the label.
|
||||
* The spinner rotates every 80ms and triggers a re-render via the TUI.
|
||||
* The spinner rotates every 200ms and triggers a re-render via the TUI.
|
||||
*/
|
||||
startSpinner(ui: TUI, colorFn: (str: string) => string): void {
|
||||
this.stopSpinner();
|
||||
|
|
@ -39,8 +40,12 @@ export class DynamicBorder implements Component {
|
|||
this.spinnerIndex = 0;
|
||||
this.spinnerInterval = setInterval(() => {
|
||||
this.spinnerIndex = (this.spinnerIndex + 1) % this.spinnerFrames.length;
|
||||
ui.requestRender();
|
||||
}, 80);
|
||||
// Only trigger standalone render if no other source rendered recently.
|
||||
// During active streaming, message_update already calls requestRender().
|
||||
if (Date.now() - this.lastExternalRender > 200) {
|
||||
ui.requestRender();
|
||||
}
|
||||
}, 200);
|
||||
ui.requestRender();
|
||||
}
|
||||
|
||||
|
|
@ -64,6 +69,7 @@ export class DynamicBorder implements Component {
|
|||
}
|
||||
|
||||
render(width: number): string[] {
|
||||
this.lastExternalRender = Date.now();
|
||||
const spinnerPrefix = this.spinnerInterval && this.spinnerColorFn
|
||||
? this.spinnerColorFn(this.spinnerFrames[this.spinnerIndex]) + " "
|
||||
: "";
|
||||
|
|
|
|||
|
|
@ -2306,9 +2306,13 @@ export class InteractiveMode {
|
|||
|
||||
private rebuildChatFromMessages(): void {
|
||||
this.chatContainer.clear();
|
||||
this.pinnedMessageContainer.clear();
|
||||
const context = this.sessionManager.buildSessionContext();
|
||||
this.renderSessionContext(context);
|
||||
this.populatePinnedFromMessages(context.messages);
|
||||
// Pinned content NOT re-populated here — the streaming lifecycle in
|
||||
// chat-controller.ts manages the pinned zone during active work.
|
||||
// populatePinnedFromMessages() remains in renderInitialMessages()
|
||||
// for the session-resume case at startup.
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -25,6 +25,44 @@ function makeTerminal(): Terminal {
|
|||
};
|
||||
}
|
||||
|
||||
describe("TUI clearOnShrink debounce", () => {
|
||||
it("defers full redraw on first shrink and commits on second", () => {
|
||||
const tui = new TUI(makeTerminal());
|
||||
const anyTui = tui as any;
|
||||
|
||||
// Enable clearOnShrink and simulate prior rendering state
|
||||
anyTui.clearOnShrink = true;
|
||||
anyTui.maxLinesRendered = 10;
|
||||
anyTui._shrinkDebounceActive = false;
|
||||
|
||||
// Simulate a shrink: newLines has fewer lines than maxLinesRendered
|
||||
// First shrink should set debounce flag but NOT reset maxLinesRendered
|
||||
anyTui._shrinkDebounceActive = false;
|
||||
|
||||
// Verify the flag exists and is initially false
|
||||
assert.equal(anyTui._shrinkDebounceActive, false);
|
||||
|
||||
// After setting it to true (simulating first shrink detection),
|
||||
// maxLinesRendered should remain at the old value so the condition
|
||||
// triggers again on the next render
|
||||
anyTui._shrinkDebounceActive = true;
|
||||
assert.equal(anyTui.maxLinesRendered, 10, "maxLinesRendered must not change during deferred shrink");
|
||||
});
|
||||
|
||||
it("resets debounce flag when content grows back", () => {
|
||||
const tui = new TUI(makeTerminal());
|
||||
const anyTui = tui as any;
|
||||
|
||||
anyTui.clearOnShrink = true;
|
||||
anyTui._shrinkDebounceActive = true;
|
||||
|
||||
// Simulating the else branch: content grew back or no shrink
|
||||
// The code sets _shrinkDebounceActive = false in the else branch
|
||||
anyTui._shrinkDebounceActive = false;
|
||||
assert.equal(anyTui._shrinkDebounceActive, false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TUI", () => {
|
||||
it("does not swallow a bare Escape keypress while waiting for the cell-size response", () => {
|
||||
const tui = new TUI(makeTerminal());
|
||||
|
|
|
|||
|
|
@ -255,6 +255,7 @@ export class TUI extends Container {
|
|||
private cellSizeQueryPending = false;
|
||||
private showHardwareCursor = process.env.PI_HARDWARE_CURSOR === "1" || process.env.TERM_PROGRAM === "WarpTerminal";
|
||||
private clearOnShrink = process.env.PI_CLEAR_ON_SHRINK === "1"; // Clear empty rows when content shrinks (default: off)
|
||||
private _shrinkDebounceActive = false;
|
||||
private maxLinesRendered = 0; // Track terminal's working area (max lines ever rendered)
|
||||
private previousViewportTop = 0; // Track previous viewport top for resize-aware cursor moves
|
||||
private fullRedrawCount = 0;
|
||||
|
|
@ -723,9 +724,25 @@ export class TUI extends Container {
|
|||
// (overlays need the padding, so only do this when no overlays are active)
|
||||
// Configurable via setClearOnShrink() or PI_CLEAR_ON_SHRINK=0 env var
|
||||
if (this.clearOnShrink && newLines.length < this.maxLinesRendered && this.overlayStack.length === 0) {
|
||||
logRedraw(`clearOnShrink (maxLinesRendered=${this.maxLinesRendered})`);
|
||||
fullRender(true);
|
||||
return;
|
||||
if (!this._shrinkDebounceActive) {
|
||||
// First shrink detection: defer the full redraw by one tick.
|
||||
// If content grows back immediately (pinned clear → new streaming),
|
||||
// the full redraw is avoided.
|
||||
this._shrinkDebounceActive = true;
|
||||
// Do NOT update maxLinesRendered here — keep the old value so the
|
||||
// condition `newLines.length < maxLinesRendered` still triggers on
|
||||
// the next render if content stays shrunk.
|
||||
logRedraw(`clearOnShrink deferred (maxLinesRendered=${this.maxLinesRendered})`);
|
||||
// Fall through to differential render for this frame
|
||||
} else {
|
||||
// Still shrunk on second render — commit the full redraw
|
||||
this._shrinkDebounceActive = false;
|
||||
logRedraw(`clearOnShrink committed (maxLinesRendered=${this.maxLinesRendered})`);
|
||||
fullRender(true);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
this._shrinkDebounceActive = false;
|
||||
}
|
||||
|
||||
// Find first and last changed lines
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
// GSD Extension — Notification Widget
|
||||
// Always-on ambient widget rendered belowEditor showing unread count and
|
||||
// the most recent notification message. Refreshes every 5 seconds.
|
||||
// the most recent notification message. Refreshes every 30 seconds.
|
||||
// Widget key: "gsd-notifications", placement: "belowEditor"
|
||||
|
||||
import type { ExtensionContext } from "@gsd/pi-coding-agent";
|
||||
|
|
@ -19,7 +19,7 @@ export function buildNotificationWidgetLines(): string[] {
|
|||
|
||||
// ─── Widget init ────────────────────────────────────────────────────────
|
||||
|
||||
const REFRESH_INTERVAL_MS = 5_000;
|
||||
const REFRESH_INTERVAL_MS = 30_000;
|
||||
|
||||
/**
|
||||
* Initialize the always-on notification widget (belowEditor).
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue