Skip to content

Commit 3f924f4

Browse files
TheGreatAxiosTheGreatAxios
committed
fix(tui): invalidate cached styles and repaint unconditionally on theme pin
Style registry keyed by themeVersion so pin flips invalidate cached styles. Rebuild and repaint unconditionally on pin change.
1 parent 66f979d commit 3f924f4

5 files changed

Lines changed: 221 additions & 67 deletions

File tree

‎src/tui/runner/settings-theme.test.ts‎

Lines changed: 118 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,48 +3,137 @@
33
* at once — it never records-and-waits-for-relaunch. Pins resolve exactly as
44
* startup does; explicit pins are deterministic regardless of terminal state,
55
* so these tests never touch `process.env` or spawn an OS probe.
6+
*
7+
* Every assertion below reads painted content or the live style registry —
8+
* never a repaint counter. The pin cycle runs with no cost movement at all,
9+
* so a meter-gated repaint (or a stale cached SyntaxStyle) leaves the old
10+
* palette on screen and fails.
611
*/
712

813
import { afterEach, describe, expect, test } from "bun:test";
14+
import { RGBA, type CapturedSpan } from "@opentui/core";
915

10-
import { setTheme, UI } from "../theme.js";
11-
import { applyThemePinLive } from "./settings.js";
16+
import { corbitsDark, corbitsLight, setTheme, UI } from "../theme.js";
17+
import { transcriptSyntaxStyle } from "../stream.js";
18+
import { withTestRenderer, type Harness } from "../harness.js";
19+
import { appendStreamRow } from "../shell/chrome.js";
20+
import { createAppShell } from "../shell/index.js";
21+
import { applyThemePinLive, repaintShellForTheme } from "./settings.js";
1222

1323
afterEach(() => {
1424
setTheme("corbits-dark");
1525
});
1626

27+
/** Live heading colour in the transcript registry. */
28+
function registryHeadingFg(): RGBA | undefined {
29+
return transcriptSyntaxStyle().getAllStyles().get("markup.heading.1")?.fg;
30+
}
31+
32+
function painted(spans: CapturedSpan[], text: string, fg: RGBA): boolean {
33+
return spans.some((s) => s.text.includes(text) && s.fg.equals(fg));
34+
}
35+
36+
async function settleSpans(
37+
h: Harness,
38+
ready: (spans: CapturedSpan[]) => boolean,
39+
): Promise<CapturedSpan[]> {
40+
let spans = h.captureSpans().lines.flatMap((line) => line.spans);
41+
for (let i = 0; i < 400 && !ready(spans); i++) {
42+
await h.renderOnce();
43+
await new Promise((resolve) => setTimeout(resolve, 25));
44+
spans = h.captureSpans().lines.flatMap((line) => line.spans);
45+
}
46+
return spans;
47+
}
48+
1749
describe("applyThemePinLive", () => {
18-
test("a dark pin swaps the shared UI binding and repaints", () => {
19-
let repaints = 0;
20-
const applied = applyThemePinLive("dark", () => {
21-
repaints += 1;
22-
});
23-
expect(applied).toBe("corbits-dark");
24-
expect(UI.name).toBe("corbits-dark");
25-
expect(repaints).toBe(1);
26-
});
50+
test("a light pin swaps the binding, rebuilds the registry, and repaints", async () => {
51+
await withTestRenderer(
52+
async (h) => {
53+
setTheme("corbits-dark");
54+
const shell = createAppShell(h.renderer, {
55+
terminal: { columns: 80, rows: 40 },
56+
wireKeys: false,
57+
});
58+
try {
59+
appendStreamRow(shell, { role: "assistant", text: "# pin cycle" });
60+
const darkHeading = RGBA.fromHex(corbitsDark.heading);
61+
const lightHeading = RGBA.fromHex(corbitsLight.heading);
62+
const darkGround = UI.ground;
63+
const before = await settleSpans(h, (spans) =>
64+
painted(spans, "pin cycle", darkHeading),
65+
);
66+
expect(painted(before, "pin cycle", darkHeading)).toBe(true);
67+
expect(registryHeadingFg()?.equals(darkHeading)).toBe(true);
2768

28-
test("a light pin swaps the shared UI binding and repaints", () => {
29-
setTheme("corbits-dark");
30-
const darkGround = UI.ground;
31-
let repaints = 0;
32-
const applied = applyThemePinLive("light", () => {
33-
repaints += 1;
34-
});
35-
expect(applied).toBe("corbits-light");
36-
expect(UI.name).toBe("corbits-light");
37-
expect(UI.ground).not.toBe(darkGround);
38-
expect(repaints).toBe(1);
39-
});
69+
// No cost movement anywhere in this cycle: the meter never moves,
70+
// so only an unconditional repaint repaints anything.
71+
const applied = applyThemePinLive("light", () =>
72+
repaintShellForTheme(shell),
73+
);
74+
expect(applied).toBe("corbits-light");
75+
expect(UI.name).toBe("corbits-light");
76+
expect(UI.ground).not.toBe(darkGround);
77+
expect(registryHeadingFg()?.equals(lightHeading)).toBe(true);
4078

41-
test("auto re-resolves through the startup path and repaints", () => {
42-
let repaints = 0;
43-
const applied = applyThemePinLive("auto", () => {
44-
repaints += 1;
45-
});
79+
const after = await settleSpans(h, (spans) =>
80+
painted(spans, "pin cycle", lightHeading),
81+
);
82+
expect(painted(after, "pin cycle", lightHeading)).toBe(true);
83+
expect(painted(after, "pin cycle", darkHeading)).toBe(false);
84+
} finally {
85+
shell.dispose();
86+
setTheme("corbits-dark");
87+
}
88+
},
89+
{ width: 80, height: 40 },
90+
);
91+
}, 30000);
92+
93+
test("pinning back to dark restores dark content", async () => {
94+
await withTestRenderer(
95+
async (h) => {
96+
setTheme("corbits-light");
97+
const shell = createAppShell(h.renderer, {
98+
terminal: { columns: 80, rows: 40 },
99+
wireKeys: false,
100+
});
101+
try {
102+
appendStreamRow(shell, { role: "assistant", text: "# pin cycle" });
103+
const darkHeading = RGBA.fromHex(corbitsDark.heading);
104+
const lightHeading = RGBA.fromHex(corbitsLight.heading);
105+
const before = await settleSpans(h, (spans) =>
106+
painted(spans, "pin cycle", lightHeading),
107+
);
108+
expect(painted(before, "pin cycle", lightHeading)).toBe(true);
109+
expect(registryHeadingFg()?.equals(lightHeading)).toBe(true);
110+
111+
const applied = applyThemePinLive("dark", () =>
112+
repaintShellForTheme(shell),
113+
);
114+
expect(applied).toBe("corbits-dark");
115+
expect(UI.name).toBe("corbits-dark");
116+
expect(registryHeadingFg()?.equals(darkHeading)).toBe(true);
117+
118+
const after = await settleSpans(h, (spans) =>
119+
painted(spans, "pin cycle", darkHeading),
120+
);
121+
expect(painted(after, "pin cycle", darkHeading)).toBe(true);
122+
expect(painted(after, "pin cycle", lightHeading)).toBe(false);
123+
} finally {
124+
shell.dispose();
125+
setTheme("corbits-dark");
126+
}
127+
},
128+
{ width: 80, height: 40 },
129+
);
130+
}, 30000);
131+
132+
test("auto re-resolves through the startup path and rebuilds the registry", () => {
133+
setTheme("corbits-dark");
134+
const applied = applyThemePinLive("auto", () => undefined);
46135
expect(["corbits-dark", "corbits-light"]).toContain(applied);
47136
expect(UI.name).toBe(applied);
48-
expect(repaints).toBe(1);
137+
expect(registryHeadingFg()?.equals(RGBA.fromHex(UI.heading))).toBe(true);
49138
});
50139
});

‎src/tui/runner/settings.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,13 @@ import { applyLiveModelSwitch } from "../../session/live-model-switch.js";
4646
import type { ThemeSetting } from "../theme-detect.js";
4747
import { applyStartupTheme } from "../theme-startup.js";
4848
import type { ThemeName } from "../theme.js";
49-
import { applyFocus } from "../shell/chrome.js";
49+
import {
50+
applyFocus,
51+
paintChrome,
52+
paintPromptBorder,
53+
repaintTranscriptWindow,
54+
} from "../shell/chrome.js";
55+
import type { AppShell } from "../shell/internals.js";
5056
import { setShellInputSuspended } from "../shell/prompt.js";
5157
import { warningsForPluginEntry } from "../../plugins/diagnostics.js";
5258
import { isPluginEnabledForSurface } from "../plugin-surface.js";
@@ -653,6 +659,20 @@ export function applyThemePinLive(
653659
return applied;
654660
}
655661

662+
/**
663+
* Unconditional post-pin repaint for the settings surface. The cost/context
664+
* meter path (`refreshCostContext` → `setPromptCostContext`) skips painting
665+
* when the meter did not move — the common pin-cycle case — so cycling pins
666+
* with no cost movement repainted nothing behind the overlay. Force the
667+
* chrome, repaint the border, and rebuild the transcript rows so markdown
668+
* bodies pick up the fresh SyntaxStyle registry. Synchronous, like the swap.
669+
*/
670+
export function repaintShellForTheme(shell: AppShell): void {
671+
paintChrome(shell, { force: true });
672+
paintPromptBorder(shell);
673+
repaintTranscriptWindow(shell);
674+
}
675+
656676
function createSettingsSurface(
657677
state: RunnerState,
658678
services: RunnerServices,
@@ -701,8 +721,9 @@ function createSettingsSurface(
701721
// The pin used to record-and-persist only, leaving the old palette on
702722
// screen until relaunch. Resolve it the same way startup does and paint
703723
// now: the swap is an instant rebinding of the shared UI object, never
704-
// an animated transition.
705-
applyThemePinLive(value, () => hostOf(state).refreshCostContext());
724+
// an animated transition. The repaint is unconditional — the meter path
725+
// would skip it whenever the cost context did not move.
726+
applyThemePinLive(value, () => repaintShellForTheme(hostOf(state).shell));
706727
void persistGlobalSettings("theme", (base) => ({
707728
...base,
708729
theme: value,

‎src/tui/shell/prompt.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import {
3535
enqueueSteer,
3636
interrupt,
3737
} from "../delivery-queue.js";
38-
import { UI } from "../theme.js";
38+
import { UI, onThemeChange } from "../theme.js";
3939

4040
import {
4141
type AppShell,
@@ -184,6 +184,16 @@ let cachedPromptSyntaxStyle: SyntaxStyle | null = null;
184184

185185
let cachedPromptRecognizedStyleId: number | null = null;
186186

187+
let promptStyleGeneration = 0;
188+
189+
const promptHighlightGenerations = new WeakMap<AppShell, number>();
190+
191+
onThemeChange(() => {
192+
cachedPromptSyntaxStyle = null;
193+
cachedPromptRecognizedStyleId = null;
194+
promptStyleGeneration += 1;
195+
});
196+
187197
/**
188198
* The style registry backing the prompt's highlights, plus the one style id
189199
* this feature uses. Lazy for the same reason as `transcriptSyntaxStyle`:
@@ -214,8 +224,13 @@ export function syncPromptHighlights(shell: AppShell): void {
214224
const source = shellRecognitionSource.get(shell);
215225
if (source === undefined) return;
216226
const value = shell.prompt.value;
217-
if (promptHighlightedValue.get(shell) === value) return;
227+
if (
228+
promptHighlightedValue.get(shell) === value &&
229+
promptHighlightGenerations.get(shell) === promptStyleGeneration
230+
)
231+
return;
218232
promptHighlightedValue.set(shell, value);
233+
promptHighlightGenerations.set(shell, promptStyleGeneration);
219234

220235
const styleId = promptRecognizedStyleId();
221236
shell.prompt.syntaxStyle = cachedPromptSyntaxStyle;

‎src/tui/stream.ts‎

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
thinkingSettledLine,
1414
type Thought,
1515
} from "./thinking.js";
16-
import { UI } from "./theme.js";
16+
import { UI, onThemeChange } from "./theme.js";
1717
import { pastTenseToolLabel } from "./tool-formatter.js";
1818

1919
/**
@@ -953,47 +953,57 @@ export function streamRowGutter(
953953
* markdown rows read as the same product skin as plain rows.
954954
* Native scope names: `markup.*` for markdown, the rest for fenced-code
955955
* syntax highlighting.
956+
*
957+
* A function of the live `UI` binding, never a module snapshot: the values
958+
* are read when a fresh registry is built, so a theme pin rebuilds them.
956959
*/
957-
const MARKDOWN_STYLES = {
958-
default: { fg: UI.text },
959-
conceal: { fg: UI.textFaint, dim: true },
960-
// Tree-sitter markdown tags headings by level, and SyntaxStyle matches whole
961-
// scope names, so the unnumbered scope alone would never be hit.
962-
"markup.heading": { fg: UI.heading, bold: true },
963-
"markup.heading.1": { fg: UI.heading, bold: true },
964-
"markup.heading.2": { fg: UI.heading, bold: true },
965-
"markup.heading.3": { fg: UI.heading, bold: true },
966-
"markup.heading.4": { fg: UI.heading, bold: true },
967-
"markup.heading.5": { fg: UI.heading, bold: true },
968-
"markup.heading.6": { fg: UI.heading, bold: true },
969-
"markup.strong": { fg: UI.text, bold: true },
970-
"markup.italic": { fg: UI.text, italic: true },
971-
"markup.strikethrough": { fg: UI.textFaint },
972-
"markup.raw": { fg: UI.inFlight },
973-
"markup.list": { fg: UI.inFlightBright },
974-
"markup.quote": { fg: UI.textDim, italic: true },
975-
"markup.link": { fg: UI.inFlightBright },
976-
"markup.link.label": { fg: UI.inFlightBright },
977-
"markup.link.url": { fg: UI.inFlightBright },
978-
keyword: { fg: UI.inFlightBright },
979-
string: { fg: UI.done },
980-
number: { fg: UI.done },
981-
comment: { fg: UI.textFaint, italic: true },
982-
function: { fg: UI.inFlight },
983-
type: { fg: UI.inFlightBright },
984-
variable: { fg: UI.text },
985-
punctuation: { fg: UI.textDim },
986-
} as const;
960+
function markdownStyles() {
961+
return {
962+
default: { fg: UI.text },
963+
conceal: { fg: UI.textFaint, dim: true },
964+
// Tree-sitter markdown tags headings by level, and SyntaxStyle matches whole
965+
// scope names, so the unnumbered scope alone would never be hit.
966+
"markup.heading": { fg: UI.heading, bold: true },
967+
"markup.heading.1": { fg: UI.heading, bold: true },
968+
"markup.heading.2": { fg: UI.heading, bold: true },
969+
"markup.heading.3": { fg: UI.heading, bold: true },
970+
"markup.heading.4": { fg: UI.heading, bold: true },
971+
"markup.heading.5": { fg: UI.heading, bold: true },
972+
"markup.heading.6": { fg: UI.heading, bold: true },
973+
"markup.strong": { fg: UI.text, bold: true },
974+
"markup.italic": { fg: UI.text, italic: true },
975+
"markup.strikethrough": { fg: UI.textFaint },
976+
"markup.raw": { fg: UI.inFlight },
977+
"markup.list": { fg: UI.inFlightBright },
978+
"markup.quote": { fg: UI.textDim, italic: true },
979+
"markup.link": { fg: UI.inFlightBright },
980+
"markup.link.label": { fg: UI.inFlightBright },
981+
"markup.link.url": { fg: UI.inFlightBright },
982+
keyword: { fg: UI.inFlightBright },
983+
string: { fg: UI.done },
984+
number: { fg: UI.done },
985+
comment: { fg: UI.textFaint, italic: true },
986+
function: { fg: UI.inFlight },
987+
type: { fg: UI.inFlightBright },
988+
variable: { fg: UI.text },
989+
punctuation: { fg: UI.textDim },
990+
};
991+
}
987992

988993
let cachedSyntaxStyle: SyntaxStyle | null = null;
989994

990995
/**
991996
* Shared transcript SyntaxStyle. Lazy because construction reaches into the
992-
* native render lib, which is unavailable until a renderer exists.
997+
* native render lib, which is unavailable until a renderer exists. Dropped
998+
* by `setTheme`, so the next read rebuilds from the new palette.
993999
*/
9941000
export function transcriptSyntaxStyle(): SyntaxStyle {
9951001
if (cachedSyntaxStyle === null) {
996-
cachedSyntaxStyle = SyntaxStyle.fromStyles({ ...MARKDOWN_STYLES });
1002+
cachedSyntaxStyle = SyntaxStyle.fromStyles({ ...markdownStyles() });
9971003
}
9981004
return cachedSyntaxStyle;
9991005
}
1006+
1007+
onThemeChange(() => {
1008+
cachedSyntaxStyle = null;
1009+
});

‎src/tui/theme.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,24 @@ export function resolveThemeName(name: string): Theme {
154154

155155
let activeTheme: Theme = corbitsDark;
156156

157+
/**
158+
* Reset hooks for paint state derived from the palette. Modules that snapshot
159+
* palette values into renderer-owned registries (a SyntaxStyle) register a
160+
* reset here; `setTheme` runs every hook right after swapping `UI`, so no
161+
* stale palette survives a pin change. The single choke point stays here —
162+
* call sites never reset caches themselves.
163+
*/
164+
type ThemeCacheReset = () => void;
165+
166+
const themeCacheResets = new Set<ThemeCacheReset>();
167+
168+
export function onThemeChange(reset: ThemeCacheReset): () => void {
169+
themeCacheResets.add(reset);
170+
return () => {
171+
themeCacheResets.delete(reset);
172+
};
173+
}
174+
157175
/**
158176
* Switch the live `UI` binding to the named theme, keeping the reference.
159177
* Everything outside this file paints through `UI`, so existing readers pick
@@ -163,6 +181,7 @@ let activeTheme: Theme = corbitsDark;
163181
export function setTheme(name: ThemeName | string): Theme {
164182
activeTheme = resolveThemeName(name);
165183
Object.assign(UI, activeTheme);
184+
for (const reset of themeCacheResets) reset();
166185
return activeTheme;
167186
}
168187

0 commit comments

Comments
 (0)