From 09429fe4ea40595ffaee7e6b129f953cf44d2da8 Mon Sep 17 00:00:00 2001
From: roboomp <omp@can.ac>
Date: Tue, 14 Jul 2026 19:47:33 +0000
Subject: [PATCH] fix(tui): cached loader layout across shimmer frames

Separated stable plain-text layout from animated ANSI coloring so 30fps shimmer updates no longer repeat wrapping and width measurement.

Fixes #5230
---
 packages/tui/CHANGELOG.md             |  4 ++
 packages/tui/src/components/loader.ts | 54 ++++++++++++++++++++-------
 packages/tui/test/loader.test.ts      | 23 ++++++++++++
 3 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md
index d7700b4be..cc0ae821c 100644
--- a/packages/tui/CHANGELOG.md
+++ b/packages/tui/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed animated Loader ANSI updates invalidating stable text layout, avoiding repeated wrapping and width measurement on shimmer-only frames ([#5230](https://github.com/can1357/oh-my-pi/issues/5230)).
+
 ## [16.4.5] - 2026-07-11
 
 ### Added
diff --git a/packages/tui/src/components/loader.ts b/packages/tui/src/components/loader.ts
index 2a4f14e24..1f5aea96f 100644
--- a/packages/tui/src/components/loader.ts
+++ b/packages/tui/src/components/loader.ts
@@ -1,28 +1,29 @@
 import type { TUI } from "../tui";
-import { sliceByColumn, visibleWidth } from "../utils";
+import { getPaddingX, sliceByColumn, visibleWidth } from "../utils";
 import { Text } from "./text";
 
-/**
- * Loader component. Spinner frames advance at `SPINNER_ADVANCE_MS`.
- *
- * Message colorizers that are time-dependent can opt into 30fps redraws by
- * setting `animated` to `true` on the function object.
- */
 const RENDER_INTERVAL_MS = 1000 / 30;
 const SPINNER_ADVANCE_MS = 80;
 
 type ColorFn = (str: string) => string;
 
+/**
+ * Styles Loader message fragments without changing their visible text or width.
+ * Set `animated` for colorizers whose ANSI output changes over time.
+ */
 export type LoaderMessageColorFn = ColorFn & {
 	readonly animated?: true;
 };
 
+/** Animates a spinner and colorized message while asynchronous work is pending. */
 export class Loader extends Text {
 	#frames = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"];
 	#currentFrame = 0;
 	#intervalId?: NodeJS.Timeout;
 	#ui: TUI | null = null;
 	#lastSpinnerTick = 0;
+	#layoutSource?: readonly string[];
+	#layout?: readonly { leading: string; content: string; trailing: string }[];
 
 	constructor(
 		ui: TUI,
@@ -40,11 +41,36 @@ export class Loader extends Text {
 	}
 
 	render(width: number): readonly string[] {
-		const lines = ["", ...super.render(width)];
-		for (let i = 0; i < lines.length; i++) {
-			const line = lines[i];
-			if (visibleWidth(line) > width) {
-				lines[i] = sliceByColumn(line, 0, width, true);
+		const source = super.render(width);
+		if (source !== this.#layoutSource) {
+			const paddingX = getPaddingX(1);
+			this.#layoutSource = source;
+			this.#layout = source.map(line => {
+				const clamped = visibleWidth(line) > width ? sliceByColumn(line, 0, width, true) : line;
+				const body = clamped.slice(paddingX);
+				const content = body.trimEnd();
+				return {
+					leading: clamped.slice(0, paddingX),
+					content,
+					trailing: body.slice(content.length),
+				};
+			});
+		}
+
+		const frame = this.#frames[this.#currentFrame];
+		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);
+				const separator = remainder.startsWith(" ") ? " " : "";
+				const message = remainder.slice(separator.length);
+				lines.push(
+					`${leading}${this.spinnerColorFn(frame)}${separator}${message ? this.messageColorFn(message) : ""}${trailing}`,
+				);
+			} else {
+				lines.push(`${leading}${content ? this.messageColorFn(content) : ""}${trailing}`);
 			}
 		}
 		return lines;
@@ -91,8 +117,8 @@ export class Loader extends Text {
 
 	#updateDisplay() {
 		const frame = this.#frames[this.#currentFrame];
-		const text = `${this.spinnerColorFn(frame)} ${this.messageColorFn(this.message)}`;
-		if (this.setText(text) && this.#ui) {
+		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
diff --git a/packages/tui/test/loader.test.ts b/packages/tui/test/loader.test.ts
index fce366090..53cc02be6 100644
--- a/packages/tui/test/loader.test.ts
+++ b/packages/tui/test/loader.test.ts
@@ -134,6 +134,29 @@ describe("Loader component", () => {
 		loader.stop();
 	});
 
+	it("reuses text layout when only animated ANSI styling changes", () => {
+		vi.useFakeTimers();
+		let colorFrame = 0;
+		const ui = { synchronizedOutput: true, requestDirectWrite: vi.fn(), requestComponentRender: vi.fn() };
+		const colorMessage = ((text: string) => `\x1b[3${colorFrame++ % 3}m${text}\x1b[0m`) as LoaderMessageColorFn & {
+			animated: true;
+		};
+		colorMessage.animated = true;
+		const loader = new Loader(ui as unknown as TUI, text => text, colorMessage, "Checking", ["⠸"]);
+		const stringWidth = spyOn(Bun, "stringWidth");
+
+		const initial = loader.render(40);
+		stringWidth.mockClear();
+		vi.advanceTimersByTime(34);
+		const animated = loader.render(40);
+
+		expect(ui.requestDirectWrite).toHaveBeenCalledTimes(2);
+		expect(stringWidth).not.toHaveBeenCalled();
+		expect(initial[1]).not.toBe(animated[1]);
+		expect(visibleWidth(initial[1])).toBe(visibleWidth(animated[1]));
+		loader.stop();
+	});
+
 	it("holds animated message-only frames when synchronized output is unavailable", () => {
 		vi.useFakeTimers();
 		setSystemTime(new Date(1_000));