Skip to content

fix(tui): fall back to readable ANSI-16 palettes on basic-color terminals - #2674

Open
map-c wants to merge 2 commits into
MoonshotAI:mainfrom
map-c:fix/tui-ansi16-palette
Open

fix(tui): fall back to readable ANSI-16 palettes on basic-color terminals#2674
map-c wants to merge 2 commits into
MoonshotAI:mainfrom
map-c:fix/tui-ansi16-palette

Conversation

@map-c

@map-c map-c commented Aug 6, 2026

Copy link
Copy Markdown

Related Issue

Resolve #2671

Problem

See linked issue. On 16-color terminals (TERM=xterm, tput colors = 8/16), chalk only supports ANSI-16 (level 1), and chalk.hex() quantizes aggressively: mid grays below #808080 collapse to black (SGR 30 — invisible on dark backgrounds) and desaturated hues collapse to white. Several dark-theme tokens became unreadable (textMuted, border, diffGutter → black) or lost their hue (diffRemovedStrong, shellMode → white; diffAddedStrong → cyan).

What changed

  • Added basicDarkColors / basicLightColors fallback palettes in src/tui/theme/colors.ts. Every value was verified against chalk's level-1 rgb→ansi16 conversion: dark neutrals land on SGR 37/97 (never black), and state/role tokens keep their intended hue (primary → bright blue, diff strong → bright green/red, shellMode → bright magenta). Light-theme tokens that quantized to the wrong hue (success → black, warning/roleUser → red, shellMode → bright blue) are corrected to green/yellow/magenta.
  • Added isBasicColorTerminal() (chalk.level === 1) in detect.ts; level 0 needs no fallback (no color at all) and FORCE_COLOR=2/3 intentionally overrides TERM detection.
  • Built-in palette resolution (getColorPalette / getColorPaletteSync / auto-theme tracking) now goes through getBuiltInPaletteForTerminal, which swaps in the basic variants only on level-1 terminals. Custom themes always load as written.
  • Hue hierarchy is intentionally flatter in the basic palettes — 16 colors cannot express four neutral shades — readability takes priority.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue, or explained the problem above.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update.

@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c755faa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@moonshot-ai/kimi-code Patch

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48da1ce162

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


/** Built-in palette lookup with the ANSI-16 fallback applied. */
export function getBuiltInPaletteForTerminal(resolved: ResolvedTheme): ColorPalette {
return isBasicColorTerminal() ? getBasicPalette(resolved) : getBuiltInPalette(resolved);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve light-theme identity for ANSI-16 palettes

When chalk.level === 1, this returns basicLightColors rather than lightColors, but both applyThemeChoice in commands/config.ts and applyReloadedTuiConfig in commands/reload.ts still infer the current resolved theme using currentTheme.palette === lightColors. Consequently, selecting auto from an explicit light theme—or reloading while auto has resolved light—initially applies the dark palette; on terminals that do not answer the subsequent theme-report queries, it remains dark indefinitely. Recognize basicLightColors in that inference or track the resolved theme independently of palette object identity.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in c755faa: added inferBuiltInResolvedTheme(), which recognizes all four built-in palette identities (dark/light plus both ANSI-16 basic variants) and returns null for custom-theme palettes. Both call sites (applyThemeChoice in config.ts, applyReloadedTuiConfig in reload.ts) now use it with a ?? 'dark' fallback, preserving the previous semantics for custom themes. Covered by a new test including the basic variants and a custom-palette negative case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect text colors when TERM=xterm (8-color terminal)

1 participant