From b152f40134c40e1f3f909f32e1cff2e82811f072 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 29 Jul 2026 00:52:05 +0000 Subject: [PATCH] fix(tui): stopped loader spinner re-wrapping text every tick The Loader baked the advancing braille glyph into the underlying Text via setText, so Text.render's wrap cache (keyed on text) missed every 80ms tick and re-ran wrapTextWithAnsi plus per-line width measurement over the whole message even though only a 1-cell glyph changed. On a single-subagent hub wait the direct-write render loop consumed ~48% of a saturated core. The wrapped text now carries a stable sentinel glyph (frames[0]); the spinner advance only bumps the frame index and requests a repaint, and render() swaps the visible glyph over the cached layout. wrapTextWithAnsi and stringWidth now run only when the message actually changes. Fixes #6940 (cherry picked from commit 6ec38053ebc1e33859358df3b66f71fea27532c7) --- packages/tui/CHANGELOG.md | 4 +++ packages/tui/src/components/loader.ts | 47 +++++++++++++++++---------- packages/tui/test/loader.test.ts | 26 +++++++++++++++ 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index a8d45c1af..9c2633d5d 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the `Loader` spinner pegging a CPU core during idle waits: advancing the braille glyph baked it into the underlying `Text` via `setText`, invalidating the wrap cache every 80 ms tick so `wrapTextWithAnsi` and per-line width measurement re-ran over the whole message. The wrapped text now carries a stable sentinel glyph and only the visible glyph is swapped at render time, so the wrap/width pipeline runs only when the message changes ([#6940](https://github.com/can1357/oh-my-pi/issues/6940)). + ## [17.1.8] - 2026-07-28 ### Fixed diff --git a/packages/tui/src/components/loader.ts b/packages/tui/src/components/loader.ts index 1f5aea96f..3ced728bc 100644 --- a/packages/tui/src/components/loader.ts +++ b/packages/tui/src/components/loader.ts @@ -58,12 +58,17 @@ export class Loader extends Text { } const frame = this.#frames[this.#currentFrame]; + // The wrapped text carries a fixed sentinel glyph (frames[0]); advancing + // the spinner swaps only the visible glyph here, so the wrap/width cache + // stays valid across ticks. Assumes every frame shares one display width + // (true for the default braille set and all in-repo callers). + const sentinel = this.#frames[0]; const lines = [""]; const layout = this.#layout ?? []; for (let i = 0; i < layout.length; i++) { const { leading, content, trailing } = layout[i]; - if (i === 0 && content.startsWith(frame)) { - const remainder = content.slice(frame.length); + if (i === 0 && content.startsWith(sentinel)) { + const remainder = content.slice(sentinel.length); const separator = remainder.startsWith(" ") ? " " : ""; const message = remainder.slice(separator.length); lines.push( @@ -78,7 +83,8 @@ export class Loader extends Text { start() { this.#lastSpinnerTick = performance.now(); - this.#updateDisplay(); + this.#syncText(); + this.#requestPaint(); const intervalMs = this.messageColorFn.animated === true ? RENDER_INTERVAL_MS : SPINNER_ADVANCE_MS; this.#intervalId = setInterval(() => { const now = performance.now(); @@ -90,7 +96,7 @@ export class Loader extends Text { this.#lastSpinnerTick += steps * SPINNER_ADVANCE_MS; } if (shouldAdvanceSpinner || this.#ui?.synchronizedOutput === true) { - this.#updateDisplay(); + this.#requestPaint(); } }, intervalMs); } @@ -112,22 +118,27 @@ export class Loader extends Text { return; } this.message = message; - this.#updateDisplay(); + this.#syncText(); + this.#requestPaint(); } - #updateDisplay() { - const frame = this.#frames[this.#currentFrame]; - const textChanged = this.setText(`${frame} ${this.message}`); - if ((textChanged || this.messageColorFn.animated === true) && this.#ui) { - // Direct write: a loader tick changes only this component, so the TUI - // can update the already-positioned rows without driving the full - // compose/prepare/diff pipeline. Lightweight test stubs may not carry - // the newer API; keep their legacy component-scoped path working. - if (typeof this.#ui.requestDirectWrite === "function") { - this.#ui.requestDirectWrite(this); - } else { - this.#ui.requestComponentRender(this); - } + /** Re-wrap the underlying Text with the stable sentinel glyph plus message. */ + #syncText(): boolean { + return this.setText(`${this.#frames[0]} ${this.message}`); + } + + #requestPaint() { + if (!this.#ui) { + return; + } + // Direct write: a loader tick changes only this component, so the TUI can + // update the already-positioned rows without driving the full + // compose/prepare/diff pipeline. Lightweight test stubs may not carry the + // newer API; keep their legacy component-scoped path working. + if (typeof this.#ui.requestDirectWrite === "function") { + this.#ui.requestDirectWrite(this); + } else { + this.#ui.requestComponentRender(this); } } } diff --git a/packages/tui/test/loader.test.ts b/packages/tui/test/loader.test.ts index 53cc02be6..78153c597 100644 --- a/packages/tui/test/loader.test.ts +++ b/packages/tui/test/loader.test.ts @@ -157,6 +157,32 @@ describe("Loader component", () => { loader.stop(); }); + it("reuses the wrapped layout across static spinner frames without re-measuring", () => { + vi.useFakeTimers(); + const ui = { synchronizedOutput: true, requestDirectWrite: vi.fn(), requestComponentRender: vi.fn() }; + const loader = new Loader( + ui as unknown as TUI, + s => s, + m => m, + "Checking", + ["⠋", "⠙", "⠹"], + ); + const stringWidth = spyOn(Bun, "stringWidth"); + + const initial = loader.render(40); + stringWidth.mockClear(); + vi.advanceTimersByTime(80); + const advanced = loader.render(40); + + // Advancing the spinner glyph must not re-run the wrap/width pipeline: + // only the leading 1-cell glyph changed, so the cached layout stands. + expect(stringWidth).not.toHaveBeenCalled(); + expect(advanced[1]).not.toBe(initial[1]); + expect(advanced[1]).toContain("⠙ Checking"); + expect(visibleWidth(initial[1])).toBe(visibleWidth(advanced[1])); + loader.stop(); + }); + it("holds animated message-only frames when synchronized output is unavailable", () => { vi.useFakeTimers(); setSystemTime(new Date(1_000));