fix(tui): apply textStrong color to markdown bold spans#1879
fix(tui): apply textStrong color to markdown bold spans#1879C0d3N1nja97342 wants to merge 2 commits into
Conversation
The pi-tui MarkdownTheme adapter's `bold` handler was `chalk.bold(text)` —
it emitted only the SGR bold code and no foreground color. Chalk then
inherited whatever fg the surrounding style left set (often none), and
most terminals rendered bold as a dim/faint gray on dark backgrounds,
nearly unreadable and unresponsive to theme changes. The palette already
defines a `textStrong` token documented as "emphasised / bold text", so
route bold through it: `chalk.bold.hex(currentTheme.color('textStrong'))`,
mirroring how `heading` is composed. Bold now tracks `textStrong` on both
the built-in dark theme and any custom theme, and `/reload-tui` picks up
palette swaps because the lookup happens per call.
Only touch the `bold` handler — italic/strikethrough/underline are not
covered by the issue and their inherited-fg behavior is intentional in
some flows (e.g. italic inside a colored codespan).
Fixes MoonshotAI#1872
🦋 Changeset detectedLatest commit: 65958d4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49910738da
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // — nearly unreadable, and unresponsive to theme changes. `textStrong` | ||
| // is defined as "emphasised / bold text" in the palette, so this makes | ||
| // the token actually govern markdown bold spans. See #1872. | ||
| bold: (text) => chalk.bold.hex(currentTheme.color('textStrong'))(text), |
There was a problem hiding this comment.
When rendering any Markdown heading, packages/pi-tui/src/components/markdown.ts:345-347 calls theme.heading(theme.bold(...)); this callback now embeds textStrong inside the heading's text wrapper, so the inner foreground code wins and every heading is rendered with textStrong rather than the documented text token. Custom themes that intentionally give headings a distinct color therefore stop working. Preserve contextual foreground colors for structural bold (or adjust the heading composition), while applying textStrong only to actual strong spans.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — this is a real regression and I've addressed it in 65958d4.
You're right: packages/pi-tui/src/components/markdown.ts composes headings as theme.heading(theme.bold(text)) (lines 356–358), and pinning textStrong on theme.bold let the inner SGR override the outer heading wrapper's fg. Same pattern applies to table headers (line 833) and inline style contexts (lines 275/307) — theme.bold is expected to be structural (SGR bold code only, no fg) so an outer wrapper's colour can win.
The fix splits the two roles:
- Added an optional
stronghandler to theMarkdownThemeinterface, falling back toboldfor themes that don't define it. - Updated pi-tui's
case "strong"renderer to preferstrong. - Reverted
boldincreateMarkdownThemetochalk.bold(text)(no fg), matching its structural role. - Wired the new
stronghandler tochalk.bold.hex(currentTheme.color('textStrong'))— the actual fix reporter TUI: Markdown bold in assistant messages renders dark-on-dark, ignoring theme textStrong #1872 asked for, now scoped to real**strong**spans.
Added a third regression test asserting theme.bold('hello') contains no truecolor fg SGR (\[38;2;), so a future refactor that pins fg on bold can no longer land silently. The existing 67 pi-tui markdown tests still pass under node:test.
… colours Address codex review on MoonshotAI#1879: pinning textStrong on `theme.bold` broke heading composition. pi-tui composes headings as `theme.heading(theme.bold(text))` (markdown.ts:356-358), so any fg color set inside `bold` bleeds through and overrides the outer `heading` wrapper's fg — a custom theme that sets a distinct heading colour via `text` no longer works. Table headers (markdown.ts:833) and inline style contexts (markdown.ts:275, 307) show the same pattern: `theme.bold` is expected to be *structural* (SGR bold code only, no fg), so an outer wrapper's colour can win. Split them: - Add an optional `strong` handler to `MarkdownTheme`, falling back to `bold` for themes that don't define it. - Update the `case "strong"` renderer in pi-tui to prefer `strong`. - Revert `bold` in `createMarkdownTheme` to `chalk.bold(text)` — no fg, as its role demands. - Wire the new `strong` handler to `chalk.bold.hex(currentTheme.color('textStrong'))`, which is the actual fix reporter MoonshotAI#1872 asked for. Regression tests: `strong` emits the textStrong hex, `bold` emits no truecolor fg SGR, both track `currentTheme.setPalette()` at call time. Fixes MoonshotAI#1872 (still). Refs codex review.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Related Issue
Resolves #1872
Problem
See linked issue. Summary: TUI markdown bold in assistant messages renders as dark gray on dark terminal backgrounds — nearly unreadable — even though the active theme's
textStrongtoken (documented as "emphasised / bold text") is near-white. The reporter verified this against both the built-indarkpalette (textStrong#F5F5F5) and a customnordtheme, ruled out iTerm2 profile settings (bold color already white, "use bright bold" on, minimum contrast off), and confirmed it reproduces on 0.27.0.Root cause is in the pi-tui
MarkdownThemeadapter atapps/kimi-code/src/tui/theme/pi-tui-theme.ts:43:That emits only the SGR bold code (
ESC[1m) with no foreground colour. Chalk then inherits whatever fg the surrounding style left set, which for a fresh markdown paragraph is nothing — and most terminals render pure "bold, no colour" as a dim/faint gray on a dark background. Because no theme token is ever queried on this path, neither the built-in dark palette nor any custom theme can influence bold rendering, matching the reporter's ruled-out list.What changed
Route bold through
textStrong, mirroring howheadingis composed at line 30:The lookup happens per call, so
/reload-tuiand palette swaps take effect immediately — same pattern as every other adapter in this file.Scope kept tight:
bold.italic,strikethrough, andunderlineon lines 44–46 have the same "SGR-only, no colour" shape but the issue doesn't cover them, and their inherited-fg behaviour is intentional in some flows (e.g. italic inside a coloured codespan, where forcingtextStrongwould override the codespan'sprimaryfg). A separate follow-up can revisit those once someone actually observes the regression.textStrongis already defined inColorPalette(line 33 ofcolors.ts) and set on bothdarkColorsandlightColors— the fix just wires the markdown adapter to it.createMarkdownTheme— pi-tui's markdown renderer — keeps the sameMarkdownThemeinterface.Reviewer Test Plan
How to verify
Unit coverage in
apps/kimi-code/test/tui/theme/pi-tui-theme.test.ts(new file):emits the textStrong hex for bold spans so themes actually govern bold (#1872)— sets the dark palette, callstheme.bold('hello'), asserts the output contains both the truecolor foreground SGR for#F5F5F5and the SGR bold code. Without the fix, the first assertion fails.follows the active palette when the theme changes (#1872)— creates the theme once, flips palettes between dark and light, asserts the two bold outputs differ. Guards against a future refactor that would readtextStrongonce at factory time instead of per call.Both tests set
chalk.level = 3for their duration to keep vitest's non-TTY environment from stripping the SGRs we're asserting on, matching the pattern inbanner.test.ts/footer.test.ts/server.test.ts.Ran locally:
apps/kimi-code/test/tui/theme/— 7/7 pass (2 new + 5 pre-existing).oxlinton the two touched files — 0 warnings, 0 errors.For a manual smoke: enable a dark palette, send any prompt whose response contains
**bold**markdown, and observe the bold spans now render attextStrongbrightness (near-white on the built-in dark) instead of dark-gray-on-black.Evidence (Before & After)
The reporter offered screenshots on request; I don't have a live model to reproduce the dialog myself, but the deterministic mechanism above (SGR without fg → terminal default dim rendering) is what the two unit tests pin.
Tested on
Environment
vitest run test/tui/theme/pi-tui-theme.test.tsandoxlintlocally.Checklist
gen-changesetsskill, or this PR needs no changeset. — added.changeset/fix-tui-markdown-bold-uses-textstrong.md(patch on@moonshot-ai/kimi-code, since this changes user-visible CLI rendering).gen-docsskill, or this PR needs no doc update. — the touched behaviour is described exactly as the docs already promise (textStrong= "emphasised / bold text"). No docs page mentions the old "no-colour" behaviour, so no doc changes needed.