diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d1722f2ec..b370b70c4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the TUI busy-looping at ~40–50% CPU during long-running tool sessions. `EventController.handleEvent` no longer rebuilds the status-line top border synchronously on every session event; the rebuild now runs inside a lazy provider that fires at most once per painted frame, so streaming/tool bursts collapse into the render throttle instead of stacking `getContextUsage` serialization work between paints. ([#4145](https://github.com/can1357/oh-my-pi/issues/4145)) + ## [16.2.12] - 2026-07-01 ### Breaking Changes diff --git a/packages/coding-agent/src/collab/guest.ts b/packages/coding-agent/src/collab/guest.ts index 2a2ef0cec..a045885d2 100644 --- a/packages/coding-agent/src/collab/guest.ts +++ b/packages/coding-agent/src/collab/guest.ts @@ -625,7 +625,7 @@ export class CollabGuestLink { setSessionTerminalTitle(this.#ctx.sessionManager.getSessionName(), this.#ctx.sessionManager.getCwd()); this.#ctx.statusLine.invalidate(); this.#ctx.statusLine.resetActiveTime(); - this.#ctx.updateEditorTopBorder(); + this.#ctx.ui.requestRender(); this.#ctx.updateEditorBorderColor(); this.#ctx.renderInitialMessages({ clearTerminalHistory: true }); await this.#ctx.reloadTodos(); diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 98e446646..d083aaaee 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -867,7 +867,7 @@ export class CommandController { this.ctx.statusLine.invalidate(); this.ctx.statusLine.resetActiveTime(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.updateEditorBorderColor(); this.ctx.chatContainer.clear(); this.ctx.pendingMessagesContainer.clear(); @@ -893,7 +893,7 @@ export class CommandController { } const stateLabel = result.closedProviderSessions === 1 ? "provider state" : "provider states"; this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.showStatus(`Fresh provider session started (${result.closedProviderSessions} ${stateLabel} pruned).`); } @@ -923,7 +923,7 @@ export class CommandController { } this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); const sessionFile = this.ctx.session.sessionFile; const shortPath = sessionFile ? sessionFile.split("/").pop() : "new session"; @@ -1156,7 +1156,7 @@ export class CommandController { } this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.showStatus(formatShakeSummary(result)); } @@ -1219,7 +1219,7 @@ export class CommandController { this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); } catch (error) { if (error instanceof CompactionCancelledError) { outcome = "cancelled"; @@ -1285,7 +1285,7 @@ export class CommandController { this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.updateEditorBorderColor(); await this.ctx.reloadTodos(); diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index aeb80e5a7..22c8772db 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -287,7 +287,7 @@ export class EventController { } this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); const run = this.#handlers[event.type] as (e: AgentSessionEvent) => Promise; await run(event); @@ -800,7 +800,7 @@ export class EventController { this.ctx.showPinnedError(event.message.errorMessage); } this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); } this.ctx.ui.requestRender(); } @@ -1158,21 +1158,21 @@ export class EventController { if (!event.skipped) { this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); } this.ctx.showWarning(event.errorMessage); } else if (!event.skipped) { this.ctx.lastAssistantUsage = undefined; this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.showStatus("Auto-shake completed"); } } else if (event.result) { this.ctx.lastAssistantUsage = undefined; this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); } else if (event.errorMessage) { this.ctx.showWarning(event.errorMessage); } else if (isHandoffAction) { @@ -1180,7 +1180,7 @@ export class EventController { this.ctx.lastAssistantUsage = undefined; this.ctx.rebuildChatFromMessages(); this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); await this.ctx.reloadTodos(); this.ctx.showStatus("Auto-handoff completed"); } else if (event.skipped) { diff --git a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts index a380b1d3e..433d9e772 100644 --- a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts +++ b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts @@ -171,7 +171,6 @@ export class ExtensionUiController { // Reset and update status line this.ctx.statusLine.invalidate(); this.ctx.statusLine.resetActiveTime(); - this.ctx.updateEditorTopBorder(); this.ctx.ui.requestRender(); // Clear UI state diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index d12dd8dcc..a09d8a1c6 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -157,7 +157,7 @@ export class SelectorController { const result = await previewTheme(themeName); if (result.success) { this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.ui.invalidate(); this.ctx.ui.requestRender(); } @@ -175,7 +175,6 @@ export class SelectorController { compactThinkingLevel: settings.get("statusLine.compactThinkingLevel"), ...previewSettings, }); - this.ctx.updateEditorTopBorder(); this.ctx.ui.requestRender(); }, getStatusLinePreview: () => { @@ -203,7 +202,6 @@ export class SelectorController { transparent: settings.get("statusLine.transparent"), compactThinkingLevel: settings.get("statusLine.compactThinkingLevel"), }); - this.ctx.updateEditorTopBorder(); this.ctx.ui.requestRender(); }, }, @@ -456,7 +454,6 @@ export class SelectorController { case "tui.tight": setTuiTight(value as boolean); this.ctx.ui.invalidate(); - this.ctx.updateEditorTopBorder(); this.ctx.ui.requestRender(); break; @@ -472,7 +469,7 @@ export class SelectorController { case "theme": { setTheme(value as string, true).then(result => { this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.ui.invalidate(); if (!result.success) { this.ctx.showError(`Failed to load theme "${value}": ${result.error}\nFell back to dark theme.`); @@ -483,7 +480,7 @@ export class SelectorController { case "symbolPreset": { setSymbolPreset(value as "unicode" | "nerd" | "ascii").then(() => { this.ctx.statusLine.invalidate(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.ui.invalidate(); }); break; @@ -557,7 +554,6 @@ export class SelectorController { compactThinkingLevel: settings.get("statusLine.compactThinkingLevel"), }; this.ctx.statusLine.updateSettings(statusLineSettings); - this.ctx.updateEditorTopBorder(); this.ctx.ui.requestRender(); break; } @@ -1046,7 +1042,7 @@ export class SelectorController { this.ctx.clearTransientSessionUi(); this.ctx.statusLine.invalidate(); this.ctx.statusLine.resetActiveTime(); - this.ctx.updateEditorTopBorder(); + this.ctx.ui.requestRender(); this.ctx.updateEditorBorderColor(); this.ctx.renderInitialMessages({ clearTerminalHistory: true }); await this.ctx.reloadTodos(); diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 0d9a5e152..1cb23e635 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -645,7 +645,7 @@ export class InteractiveMode implements InteractiveModeContext { this.#syncEditorMaxHeight(); this.#resizeHandler = () => { this.#syncEditorMaxHeight(); - this.updateEditorTopBorder(); + this.ui.requestRender(); }; process.stdout.on("resize", this.#resizeHandler); try { @@ -662,6 +662,12 @@ export class InteractiveMode implements InteractiveModeContext { this.editorContainer.addChild(this.editor); this.statusLine = new StatusLineComponent(session); this.statusLine.setAutoCompactEnabled(session.autoCompactionEnabled); + // Lazy provider — the top border rebuild coalesces to at most one + // invocation per painted frame instead of firing on every session event + // (#4145). The TUI throttles renders at ~30fps, so a long-running eval + // spraying events no longer runs `getTopBorder` synchronously in the + // hot path where the render never gets to paint the result. + this.editor.setTopBorderProvider(availableWidth => this.statusLine.getTopBorder(availableWidth)); this.hideThinkingBlock = settings.get("hideThinkingBlock"); this.proseOnlyThinking = settings.get("proseOnlyThinking"); @@ -975,14 +981,12 @@ export class InteractiveMode implements InteractiveModeContext { onTerminalAppearanceChange(mode); }); - // Set up git branch watcher + // A branch change (checkout, worktree switch, `git switch`) invalidates + // the status-line git segments; the lazy top-border provider picks up + // the fresh branch on the next painted frame. this.statusLine.watchBranch(() => { - this.updateEditorTopBorder(); this.ui.requestRender(); }); - - // Initial top border update - this.updateEditorTopBorder(); } /** Reload the title-generation system prompt override for the provided working directory. */ @@ -1059,7 +1063,7 @@ export class InteractiveMode implements InteractiveModeContext { await this.session.refreshSshTool({ activateIfAvailable: true }); setSessionTerminalTitle(this.sessionManager.getSessionName(), this.sessionManager.getCwd()); this.statusLine.invalidate(); - this.updateEditorTopBorder(); + this.ui.requestRender(); } async getUserInput(): Promise { @@ -1200,7 +1204,6 @@ export class InteractiveMode implements InteractiveModeContext { this.loopLimit = undefined; this.#cancelLoopAutoSubmit(); this.statusLine.setLoopModeStatus(undefined); - this.updateEditorTopBorder(); this.ui.requestRender(); if (wasEnabled) { this.showStatus(message); @@ -1231,7 +1234,6 @@ export class InteractiveMode implements InteractiveModeContext { this.loopPrompt = undefined; this.loopLimit = createLoopLimitRuntime(parsed.limit); this.statusLine.setLoopModeStatus({ enabled: true }); - this.updateEditorTopBorder(); this.ui.requestRender(); const limitSuffix = parsed.limit ? ` Limited to ${describeLoopLimit(parsed.limit)}.` : ""; const remainingSuffix = this.loopLimit ? ` ${describeLoopLimitRuntime(this.loopLimit)}.` : ""; @@ -1452,7 +1454,6 @@ export class InteractiveMode implements InteractiveModeContext { const base = this.editor.borderColor; this.editor.borderColor = (str: string) => `\x1b[2m${base(str)}\x1b[22m`; } - this.updateEditorTopBorder(); this.ui.requestRender(); } @@ -1469,13 +1470,7 @@ export class InteractiveMode implements InteractiveModeContext { } const count = countRunningSubagentBadgeAgents(registry); this.statusLine.setSubagentCount(count); - this.updateEditorTopBorder(); - } - - updateEditorTopBorder(): void { - const availableWidth = this.editor.getTopBorderAvailableWidth(this.ui.terminal.columns); - const topBorder = this.statusLine.getTopBorder(availableWidth); - this.editor.setTopBorder(topBorder); + this.ui.requestRender(); } rebuildChatFromMessages(): void { @@ -1806,7 +1801,6 @@ export class InteractiveMode implements InteractiveModeContext { } : undefined; this.statusLine.setPlanModeStatus(status); - this.updateEditorTopBorder(); this.ui.requestRender(); } @@ -1816,7 +1810,6 @@ export class InteractiveMode implements InteractiveModeContext { ? { enabled: this.goalModeEnabled, paused: this.goalModePaused } : undefined; this.statusLine.setGoalModeStatus(status); - this.updateEditorTopBorder(); this.ui.requestRender(); } @@ -3348,6 +3341,7 @@ export class InteractiveMode implements InteractiveModeContext { this.ui.requestRender(); }; nextEditor.setShimmerRepaintHandler(() => this.ui.requestComponentRender(this.editor)); + nextEditor.setTopBorderProvider(availableWidth => this.statusLine.getTopBorder(availableWidth)); nextEditor.setMaxHeight(this.#computeEditorMaxHeight()); if (this.historyStorage) { nextEditor.setHistoryStorage(this.historyStorage); @@ -3367,7 +3361,6 @@ export class InteractiveMode implements InteractiveModeContext { }); this.updateEditorBorderColor(); - this.updateEditorTopBorder(); this.ui.requestRender(); } @@ -3767,7 +3760,6 @@ export class InteractiveMode implements InteractiveModeContext { } else { this.#cleanupMicAnimation(); } - this.updateEditorTopBorder(); this.ui.requestRender(); }, }); diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index 8412f16bc..5ce2b7d3e 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -297,7 +297,6 @@ export interface InteractiveModeContext { getUserMessageText(message: Message): string; findLastAssistantMessage(): AssistantMessage | undefined; extractAssistantText(message: AssistantMessage): string; - updateEditorTopBorder(): void; /** Refresh the running-subagents status badge from the active local or collab registry. */ syncRunningSubagentBadge(): void; updateEditorBorderColor(): void; diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index 189967edc..98365d213 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -69,7 +69,7 @@ export interface TuiBuiltinSlashCommand extends BuiltinSlashCommand { function refreshStatusLine(ctx: InteractiveModeContext): void { ctx.statusLine.invalidate(); - ctx.updateEditorTopBorder(); + ctx.ui.requestRender(); ctx.ui.requestRender(); } diff --git a/packages/coding-agent/test/modes/controllers/event-controller-message-start.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-message-start.test.ts index 25fb631f9..c50acc6ee 100644 --- a/packages/coding-agent/test/modes/controllers/event-controller-message-start.test.ts +++ b/packages/coding-agent/test/modes/controllers/event-controller-message-start.test.ts @@ -222,14 +222,17 @@ describe("EventController IRC expiry", () => { await controller.handleEvent({ type: "irc_message", message }); expect(chatContainer.children).toHaveLength(2); - expect(requestRender).toHaveBeenCalledTimes(1); + // handleEvent now requests a repaint up front so the lazy top-border + // provider can flush the latest status-line state (#4145); the IRC + // handler adds a second request when it mounts the card. + expect(requestRender).toHaveBeenCalledTimes(2); vi.advanceTimersByTime(9_999); expect(chatContainer.children).toHaveLength(2); vi.advanceTimersByTime(1); expect(chatContainer.children).toHaveLength(1); - expect(requestRender).toHaveBeenCalledTimes(2); + expect(requestRender).toHaveBeenCalledTimes(3); }); it("keeps a card whose rows may already be committed (no live block above)", async () => { @@ -290,6 +293,6 @@ describe("EventController IRC expiry", () => { vi.advanceTimersByTime(10_000); expect(chatContainer.children).toHaveLength(1); - expect(requestRender).toHaveBeenCalledTimes(1); + expect(requestRender).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/coding-agent/test/selector-settings-side-effects.test.ts b/packages/coding-agent/test/selector-settings-side-effects.test.ts index ba92fefc0..3e54ea139 100644 --- a/packages/coding-agent/test/selector-settings-side-effects.test.ts +++ b/packages/coding-agent/test/selector-settings-side-effects.test.ts @@ -18,11 +18,9 @@ afterEach(() => { describe("selector setting side effects", () => { it("refreshes the status line when git integration changes at runtime", () => { const updateSettings = vi.fn(); - const updateEditorTopBorder = vi.fn(); const requestRender = vi.fn(); const controller = new SelectorController({ statusLine: { updateSettings }, - updateEditorTopBorder, ui: { requestRender }, } as unknown as ConstructorParameters[0]); @@ -36,23 +34,21 @@ describe("selector setting side effects", () => { rightSegments: Settings.instance.get("statusLine.rightSegments"), }), ); - expect(updateEditorTopBorder).toHaveBeenCalledTimes(1); + // The setting-change side effect is a single render request — the lazy + // top-border provider rebuilds during paint (#4145). expect(requestRender).toHaveBeenCalledTimes(1); }); - it("invalidates UI and updates editor top border when tui.tight changes", () => { + it("invalidates the UI and requests a repaint when tui.tight changes", () => { const invalidate = vi.fn(); - const updateEditorTopBorder = vi.fn(); const requestRender = vi.fn(); const controller = new SelectorController({ ui: { invalidate, requestRender }, - updateEditorTopBorder, } as unknown as ConstructorParameters[0]); controller.handleSettingChange("tui.tight", true); expect(invalidate).toHaveBeenCalledTimes(1); - expect(updateEditorTopBorder).toHaveBeenCalledTimes(1); expect(requestRender).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 1a0bd9c8d..f19ae0ac0 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Added + +- Added `Editor.setTopBorderProvider()` so hosts can install a lazy top-border builder that runs once per painted frame instead of eagerly rebuilding after every state change. Falls back to the existing `setTopBorder()` slot when no provider is registered. + +### Fixed + +- Added adaptive render backpressure: a frame that overruns the 30 fps cadence now inflates the following frame's delay to at most twice its own cost (capped at 200 ms), preventing the render loop from busy-looping when a slow paint would otherwise fire the next frame at `setTimeout(0)`. ([#4145](https://github.com/can1357/oh-my-pi/issues/4145)) + ## [16.2.12] - 2026-07-01 ### Fixed diff --git a/packages/tui/src/components/editor.ts b/packages/tui/src/components/editor.ts index 5fa01fd7b..ea5fbe37c 100644 --- a/packages/tui/src/components/editor.ts +++ b/packages/tui/src/components/editor.ts @@ -467,8 +467,12 @@ export class Editor implements Component, Focusable { onAutocompleteCancel?: () => void; disableSubmit: boolean = false; - // Custom top border (for status line integration) + // Custom top border (for status line integration). Either an eager `content` + // (set once, reused every frame) or a `provider` that recomputes lazily just + // before the editor paints — the second form lets the host coalesce + // per-event rebuilds down to one per rendered frame (see #4145). #topBorderContent?: EditorTopBorder; + #topBorderProvider?: (availableWidth: number) => EditorTopBorder | undefined; #borderVisible = true; constructor(theme: EditorTheme) { @@ -483,11 +487,30 @@ export class Editor implements Component, Focusable { /** * Set custom content for the top border (e.g., status line). * Pass undefined to use the default plain border. + * + * Eager: the passed value is cached and reused every frame. Callers that + * mutate status upstream must recompute and call this again. Prefer + * {@link setTopBorderProvider} for high-frequency updates — it collapses + * per-event rebuilds to one per painted frame. */ setTopBorder(content: EditorTopBorder | undefined): void { this.#topBorderContent = content; } + /** + * Install a lazy provider invoked once per editor render with the current + * `availableWidth`. Overrides any eager content set via {@link setTopBorder} + * — pass `undefined` to detach and fall back to the eager slot. + * + * Use this when the top border derives from state that mutates far faster + * than the render cadence (session events, streaming, subagent updates). + * The TUI already throttles renders, so a provider is invoked at most once + * per frame and never does wasted work between paints. + */ + setTopBorderProvider(provider: ((availableWidth: number) => EditorTopBorder | undefined) | undefined): void { + this.#topBorderProvider = provider; + } + /** * Show or hide the editor border chrome. */ @@ -806,8 +829,12 @@ export class Editor implements Component, Focusable { if (borderVisible) { // Render top border: ╭─ [status content] ────────────────╮ const topFillWidth = Math.max(0, width - borderWidth * 2); - if (this.#topBorderContent) { - const { content, width: statusWidth } = this.#topBorderContent; + // Provider (lazy) wins over eager content — a host that installs both + // wants the coalesced path; falling back to eager keeps existing + // setTopBorder callers working unchanged. + const topBorder = this.#topBorderProvider ? this.#topBorderProvider(topFillWidth) : this.#topBorderContent; + if (topBorder) { + const { content, width: statusWidth } = topBorder; if (statusWidth <= topFillWidth) { // Status fits - add fill after it const fillWidth = topFillWidth - statusWidth; diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 1cc57a670..3acf611ca 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -923,8 +923,23 @@ export class TUI extends Container { #renderTimer: RenderTimer | undefined; #renderScheduler: RenderScheduler; #lastRenderAt = 0; + /** + * Wall-clock cost of the most recent `#doRender()` call. Used by + * `#scheduleRender` to inflate the next render delay proportionally so a + * spike of slow frames (large transcript diffs, huge assistant text wrap, + * component-tree walks) does not busy-loop the CPU: the throttle would + * otherwise collapse to zero once `elapsed >= MIN_RENDER_INTERVAL_MS` and + * fire the next frame immediately (see #4145). + */ + #lastFrameCostMs = 0; static readonly #MIN_RENDER_INTERVAL_MS = 1000 / 30; static readonly #INPUT_RENDER_GRACE_MS = TUI.#MIN_RENDER_INTERVAL_MS; + /** + * Cap on the adaptive floor derived from `#lastFrameCostMs`. Bounds the UI + * responsiveness at ~5 fps under sustained heavy renders — anything slower + * feels dead to the user and no longer justifies further CPU savings. + */ + static readonly #MAX_ADAPTIVE_RENDER_MS = 200; #inputRenderGraceUntilMs = 0; // Pane-reflow settle window for tmux/screen/zellij. The host process gets // SIGWINCH (and `process.stdout` already reports the new geometry) before @@ -1831,8 +1846,7 @@ export class TUI extends Container { this.#prepareForcedRender(!isMultiplexerSession()); this.#resizeEventPending = true; this.#renderRequested = false; - this.#lastRenderAt = this.#renderScheduler.now(); - this.#doRender(); + this.#executeRender(); } requestRender(force = false, options?: RenderRequestOptions): void { @@ -1863,8 +1877,7 @@ export class TUI extends Container { return; } this.#renderRequested = false; - this.#lastRenderAt = this.#renderScheduler.now(); - this.#doRender(); + this.#executeRender(); }); return; } @@ -2085,8 +2098,7 @@ export class TUI extends Container { this.#ghosttyInitialImageDelayTimer = undefined; this.#ghosttyInitialImageDelayDone = true; if (this.#stopped) return; - this.#lastRenderAt = this.#renderScheduler.now(); - this.#doRender(); + this.#executeRender(); if (this.#renderRequested) this.#scheduleRender(); }, delayMs); return true; @@ -2114,22 +2126,41 @@ export class TUI extends Container { const now = this.#renderScheduler.now(); const elapsed = now - this.#lastRenderAt; const cadenceDelay = Math.max(0, TUI.#MIN_RENDER_INTERVAL_MS - elapsed); + // Adaptive backpressure — target ~50% render duty cycle: the next frame + // starts no sooner than `last_frame_end + last_frame_cost`, i.e. + // `last_frame_start + 2 × last_frame_cost`. So `elapsed` (which counts + // from the last frame's start) must already exceed twice the cost + // before we allow the follow-up render to fire. Capped so a + // pathological one-off spike doesn't lock the UI (#4145). + const adaptiveFloor = Math.min(TUI.#MAX_ADAPTIVE_RENDER_MS, this.#lastFrameCostMs * 2); + const adaptiveDelay = Math.max(0, adaptiveFloor - elapsed); const inputGraceDelay = Math.max(0, this.#inputRenderGraceUntilMs - now); - const delay = Math.max(cadenceDelay, inputGraceDelay); + const delay = Math.max(cadenceDelay, adaptiveDelay, inputGraceDelay); this.#renderTimer = this.#renderScheduler.scheduleRender(() => { this.#renderTimer = undefined; if (this.#stopped || !this.#renderRequested) { return; } this.#renderRequested = false; - this.#lastRenderAt = this.#renderScheduler.now(); - this.#doRender(); + this.#executeRender(); if (this.#renderRequested) { this.#scheduleRender(); } }, delay); } + /** + * Wrap `#doRender()` so every path records the wall-clock frame cost that + * feeds adaptive backpressure. Set `#lastRenderAt` first (some render code + * reads it re-entrantly) and compute the cost once the paint returns. + */ + #executeRender(): void { + const start = this.#renderScheduler.now(); + this.#lastRenderAt = start; + this.#doRender(); + this.#lastFrameCostMs = this.#renderScheduler.now() - start; + } + #handleInput(data: string): void { // Raw-mode Ctrl+C/Esc arrive as stdin data, not process signals. If the // first key in a double-key gesture schedules an immediate slow repaint, @@ -3277,8 +3308,7 @@ export class TUI extends Container { #requestResizeViewportPaint(): void { if (this.#stopped) return; this.#renderRequested = false; - this.#lastRenderAt = this.#renderScheduler.now(); - this.#doRender(); + this.#executeRender(); if (this.#renderRequested) this.#scheduleRender(); } diff --git a/packages/tui/test/adaptive-render-backpressure.test.ts b/packages/tui/test/adaptive-render-backpressure.test.ts new file mode 100644 index 000000000..72b7f1ab9 --- /dev/null +++ b/packages/tui/test/adaptive-render-backpressure.test.ts @@ -0,0 +1,175 @@ +/** + * Regression for oh-my-pi#4145 (TUI busy loop during long-running eval). + * + * When a rendered frame exceeded the 33ms cadence budget, the previous + * scheduler collapsed the cadence delay to zero and scheduled the next frame + * immediately (`setTimeout(0)`). During a heavy eval that turns the render + * loop into a busy loop consuming 40–50% CPU with visible frames dropped. + * + * The fix adds adaptive backpressure: the next render's delay is inflated to + * (at minimum) the previous frame's cost, capped so responsiveness never + * degrades below ~5 fps. A fast frame keeps the ~30 fps cadence untouched; + * a slow frame idles proportionally. + * + * Contract this test defends: + * 1. Fast frames leave the cadence delay at the plain min-interval floor. + * 2. A slow frame inflates the following delay to at least its measured cost. + * 3. The inflated delay is capped so a pathological frame doesn't stall the + * UI indefinitely. + */ +import { describe, expect, it } from "bun:test"; +import { type Component, type RenderTimer, TUI } from "@oh-my-pi/pi-tui"; +import { VirtualTerminal } from "./virtual-terminal"; + +const MIN_RENDER_INTERVAL_MS = 1000 / 30; +const MAX_ADAPTIVE_RENDER_MS = 200; + +class ScriptedFrameCost implements Component { + #nextCostMs: number | null = null; + scheduler!: { nowMs: number }; + + /** Program the next render() to virtually consume `costMs` on the scheduler clock. */ + scheduleCost(costMs: number): void { + this.#nextCostMs = costMs; + } + + invalidate(): void {} + + render(_width: number): readonly string[] { + if (this.#nextCostMs !== null) { + this.scheduler.nowMs += this.#nextCostMs; + this.#nextCostMs = null; + } + return ["probe"]; + } +} + +class DeferredRenderScheduler { + nowMs = 0; + readonly immediates: Array<() => void> = []; + readonly timers: Array<{ callback: () => void; canceled: boolean; delayMs: number }> = []; + + now(): number { + return this.nowMs; + } + + scheduleImmediate(callback: () => void): void { + this.immediates.push(callback); + } + + scheduleRender(callback: () => void, delayMs: number): RenderTimer { + const timer = { callback, canceled: false, delayMs }; + this.timers.push(timer); + return { + cancel: () => { + timer.canceled = true; + }, + }; + } +} + +/** Drain immediates + fire the next scheduled render timer. Returns its `delayMs`. */ +function stepRender(scheduler: DeferredRenderScheduler): number | null { + while (scheduler.immediates.length > 0) scheduler.immediates.shift()!(); + const timer = scheduler.timers.shift(); + if (!timer || timer.canceled) return null; + scheduler.nowMs += timer.delayMs; + timer.callback(); + return timer.delayMs; +} + +describe("TUI adaptive render backpressure (#4145)", () => { + it("keeps the plain min-interval cadence when frames are cheap", () => { + const term = new VirtualTerminal(20, 4); + const scheduler = new DeferredRenderScheduler(); + const probe = new ScriptedFrameCost(); + probe.scheduler = scheduler; + const tui = new TUI(term, undefined, { renderScheduler: scheduler }); + tui.addChild(probe); + + try { + tui.start(); + // Drain the initial start-time render. + stepRender(scheduler); + scheduler.timers.length = 0; + + // Three cheap (1ms) renders back-to-back: each next delay hugs the + // 33ms floor (not zero — the previous frame ended right before), so + // they arrive at the throttled cadence. + for (let i = 0; i < 3; i++) { + probe.scheduleCost(1); + tui.requestRender(); + const delay = stepRender(scheduler); + expect(delay).not.toBeNull(); + // The cadence floor is min-interval; adaptive floor is + // max(1ms) which is well below it, so delay ≈ min-interval. + expect(delay!).toBeGreaterThanOrEqual(0); + expect(delay!).toBeLessThanOrEqual(MIN_RENDER_INTERVAL_MS + 1); + } + } finally { + tui.stop(); + } + }); + + it("inflates the next delay to the previous frame's cost when a slow frame busts the cadence", () => { + const term = new VirtualTerminal(20, 4); + const scheduler = new DeferredRenderScheduler(); + const probe = new ScriptedFrameCost(); + probe.scheduler = scheduler; + const tui = new TUI(term, undefined, { renderScheduler: scheduler }); + tui.addChild(probe); + + try { + tui.start(); + stepRender(scheduler); + scheduler.timers.length = 0; + + // One slow frame — 100ms, well over the 33ms cadence. + const slowFrameCostMs = 100; + probe.scheduleCost(slowFrameCostMs); + tui.requestRender(); + stepRender(scheduler); + + // The next requested render should idle proportional to the last + // frame's cost. Pre-fix this delay collapsed to zero and pinned CPU. + probe.scheduleCost(1); + tui.requestRender(); + const delay = stepRender(scheduler); + expect(delay).not.toBeNull(); + // `elapsed` at scheduling time is 0 (last render just ended), so + // the adaptive floor equals the recorded 100ms cost directly. + expect(delay!).toBeGreaterThanOrEqual(slowFrameCostMs); + } finally { + tui.stop(); + } + }); + + it("caps the adaptive delay so a pathological frame doesn't stall the UI", () => { + const term = new VirtualTerminal(20, 4); + const scheduler = new DeferredRenderScheduler(); + const probe = new ScriptedFrameCost(); + probe.scheduler = scheduler; + const tui = new TUI(term, undefined, { renderScheduler: scheduler }); + tui.addChild(probe); + + try { + tui.start(); + stepRender(scheduler); + scheduler.timers.length = 0; + + // A pathological 5-second frame — the adaptive floor must cap so + // the follow-up delay doesn't become 5s. + probe.scheduleCost(5_000); + tui.requestRender(); + stepRender(scheduler); + + probe.scheduleCost(1); + tui.requestRender(); + const delay = stepRender(scheduler); + expect(delay).not.toBeNull(); + expect(delay!).toBeLessThanOrEqual(MAX_ADAPTIVE_RENDER_MS); + } finally { + tui.stop(); + } + }); +}); diff --git a/packages/tui/test/editor-top-border-provider.test.ts b/packages/tui/test/editor-top-border-provider.test.ts new file mode 100644 index 000000000..0c0598339 --- /dev/null +++ b/packages/tui/test/editor-top-border-provider.test.ts @@ -0,0 +1,93 @@ +/** + * Regression for oh-my-pi#4145 (TUI busy loop during long-running eval). + * + * The pre-fix hot path rebuilt the editor's top border synchronously on every + * session event, even though renders are throttled to ~30 fps. On a busy + * streaming turn that meant dozens of `getTopBorder` calls per painted frame. + * + * The fix installs a lazy provider on the editor: the host mutates status-line + * state as much as it wants, and the provider is invoked exactly once per + * editor render — bounded by the TUI's render throttle, not by event rate. + * + * Contract this test defends: + * 1. Provider takes precedence over any eager `setTopBorder` content. + * 2. Provider runs once per render (2 renders = 2 calls, no more). + * 3. Provider observes the CURRENT status-line state at render time, so + * state mutations landing between renders coalesce into one rebuild. + * 4. Clearing the provider falls back to the eager slot. + */ +import { describe, expect, it } from "bun:test"; +import { Editor, type EditorTopBorder } from "@oh-my-pi/pi-tui/components/editor"; +import { defaultEditorTheme } from "./test-themes"; + +function stubTopBorder(label: string): EditorTopBorder { + return { content: label, width: label.length }; +} + +describe("Editor lazy top-border provider (#4145)", () => { + it("invokes the provider once per render regardless of intervening state changes", () => { + const editor = new Editor(defaultEditorTheme); + let observedCounter = 0; + let counter = 0; + const calls: number[] = []; + + editor.setTopBorderProvider(availableWidth => { + calls.push(availableWidth); + observedCounter = counter; + return stubTopBorder(`counter=${counter}`); + }); + + // Simulate a burst of "events" mutating upstream state between two + // painted frames. Under the old eager rebuild path this would have + // been 25 rebuilds; under the lazy provider it should be zero here… + for (let i = 0; i < 25; i++) counter += 1; + expect(calls).toHaveLength(0); + + // …and exactly one per painted frame. + editor.render(80); + expect(calls).toHaveLength(1); + expect(observedCounter).toBe(25); + + for (let i = 0; i < 25; i++) counter += 1; + editor.render(80); + expect(calls).toHaveLength(2); + expect(observedCounter).toBe(50); + }); + + it("prefers the provider over any eager setTopBorder content", () => { + const editor = new Editor(defaultEditorTheme); + editor.setTopBorder(stubTopBorder("eager")); + editor.setTopBorderProvider(() => stubTopBorder("lazy")); + + const frame = editor.render(80).join("\n"); + expect(frame).toContain("lazy"); + expect(frame).not.toContain("eager"); + }); + + it("falls back to eager content when the provider is cleared", () => { + const editor = new Editor(defaultEditorTheme); + editor.setTopBorder(stubTopBorder("eager")); + editor.setTopBorderProvider(() => stubTopBorder("lazy")); + editor.setTopBorderProvider(undefined); + + const frame = editor.render(80).join("\n"); + expect(frame).toContain("eager"); + expect(frame).not.toContain("lazy"); + }); + + it("passes the visually-available width (terminal width minus border chrome) to the provider", () => { + const editor = new Editor(defaultEditorTheme); + const widths: number[] = []; + editor.setTopBorderProvider(availableWidth => { + widths.push(availableWidth); + return undefined; + }); + + editor.render(80); + editor.render(120); + + expect(widths).toHaveLength(2); + expect(widths[0]).toBe(editor.getTopBorderAvailableWidth(80)); + expect(widths[1]).toBe(editor.getTopBorderAvailableWidth(120)); + }); +});