fix(tui): decode ANSI alpha-channel encoding in syntax themes (#13382)

## Problem The `ansi`, `base16`, and `base16-256` syntax themes are designed to emit ANSI palette colors so that highlighted code respects the user's terminal color scheme. Syntect encodes this intent in the alpha channel of its `Color` struct — a convention shared with `bat` — but `convert_style` was ignoring it entirely, treating every foreground color as raw RGB. This caused ANSI-family themes to produce hard-coded RGB values (e.g. `Rgb(0x02, 0, 0)` instead of `Green`), defeating their purpose and rendering them as near-invisible dark colors on most terminals. Reported in #12890. ## Mental model Syntect themes use a compact encoding in their `Color` struct: | `alpha` | Meaning of `r` | Mapped to | |---------|----------------|-----------| | `0x00` | ANSI palette index (0–255) | `RtColor::Black`…`Gray` for 0–7, `Indexed(n)` for 8–255 | | `0x01` | Unused (sentinel) | `None` — inherit terminal default fg/bg | | `0xFF` | True RGB red channel | `RtColor::Rgb(r, g, b)` | | other | Unexpected | `RtColor::Rgb(r, g, b)` (silent fallback) | This encoding is a bat convention that three bundled themes rely on. The new `convert_syntect_color` function decodes it; `ansi_palette_color` maps indices 0–7 to ratatui's named ANSI variants. | macOS - Dark | macOS - Light | Windows - ansi | Windows - base16 | |---|---|---|---| | <img width="1064" height="1205" alt="macos-dark" src="https://github.com/user-attachments/assets/f03d92fb-b44b-4939-b2b9-503fde133811" /> | <img width="1073" height="1227" alt="macos-light" src="https://github.com/user-attachments/assets/2ecb2089-73b5-4676-bed8-e4e6794250b4" /> | ![windows-ansi](https://github.com/user-attachments/assets/d41029e6-ffd3-454e-ab72-6751607e5d5c) | ![windows-base16](https://github.com/user-attachments/assets/b48aafcc-0196-4977-8ee1-8f8eaddd1698) | ## Non-goals - Background color decoding — we intentionally skip backgrounds to preserve the terminal's own background. The decoder supports it, but `convert_style` does not apply it. - Italic/underline changes — those remain suppressed as before. - Custom `.tmTheme` support for ANSI encoding — only the bundled themes use this convention. ## Tradeoffs - The alpha-channel encoding is an undocumented bat/syntect convention, not a formal spec. We match bat's behavior exactly, trading formality for ecosystem compatibility. - Indices 0–7 are mapped to ratatui's named variants (`Black`, `Red`, …, `Gray`) rather than `Indexed(0)`…`Indexed(7)`. This lets terminals apply bold/bright semantics to named colors, which is the expected behavior for ANSI themes, but means the two representations are not perfectly round-trippable. ## Architecture All changes are in `codex-rs/tui/src/render/highlight.rs`, within the style-conversion layer between syntect and ratatui: ``` syntect::highlighting::Color └─ convert_syntect_color(color) [NEW — alpha-dispatch] ├─ a=0x00 → ansi_palette_color() [NEW — index→named/indexed] ├─ a=0x01 → None (terminal default) ├─ a=0xFF → Rgb(r,g,b) (standard opaque path) └─ other → Rgb(r,g,b) (silent fallback) ``` `convert_style` delegates foreground mapping to `convert_syntect_color` instead of inlining the `Rgb(r,g,b)` conversion. The core highlighter is refactored into `highlight_to_line_spans_with_theme` (accepts an explicit theme reference) so tests can highlight against specific themes without mutating process-global state. ### ANSI-family theme contract The ANSI-family themes (`ansi`, `base16`, `base16-256`) rely on upstream alpha-channel encoding from two_face/syntect. We intentionally do **not** validate this contract at runtime — if the upstream format changes, the `ansi_themes_use_only_ansi_palette_colors` test catches it at build time, long before it reaches users. A runtime warning would be unactionable noise. ### Warning copy cleanup User-facing warning messages were rewritten for clarity: - Removed internal jargon ("alpha-encoded ANSI color markers", "RGB fallback semantics", "persisted override config") - Dropped "syntax" prefix from "syntax theme" — users just think "theme" - Downgraded developer-only diagnostics (duplicate override, resolve fallback) from `warn` to `debug` ## Observability - The `ansi_themes_use_only_ansi_palette_colors` test enforces the ANSI-family contract at build time. - The snapshot test provides a regression tripwire for palette color output. - User-facing warnings are limited to actionable issues: unknown theme names and invalid custom `.tmTheme` files. ## Tests - **Unit tests for each alpha branch:** `alpha=0x00` with low index (named color), `alpha=0x00` with high index (`Indexed`), `alpha=0x01` (terminal default), unexpected alpha (falls back to RGB), ANSI white → Gray mapping. - **Integration test:** `ansi_family_themes_use_terminal_palette_colors_not_rgb` — highlights a Rust snippet with each ANSI-family theme and asserts zero `Rgb` foreground colors appear. - **Snapshot test:** `ansi_family_foreground_palette_snapshot` — records the exact set of unique foreground colors each ANSI-family theme produces, guarding against regressions. - **Warning validation tests:** verify user-facing warnings for missing custom themes, invalid `.tmTheme` files, and bundled theme resolution. ## Test plan - [ ] `cargo test -p codex-tui` passes all new and existing tests - [ ] Select `ansi`, `base16`, or `base16-256` theme and verify code blocks render with terminal palette colors (not near-black RGB) - [ ] Select a standard RGB theme (e.g. `dracula`) and verify no regression in color output
2026-03-05 21:45:28 +03:00 · 2026-03-04 17:03:34 -03:00
parent b200a5f45b
commit 98923e53cc
2 changed files with 284 additions and 37 deletions
--- a/codex-rs/tui/src/render/highlight.rs
+++ b/codex-rs/tui/src/render/highlight.rs
@@ -31,6 +31,7 @@ use std::path::PathBuf;
 use std::sync::OnceLock;
 use std::sync::RwLock;
 use syntect::easy::HighlightLines;
+use syntect::highlighting::Color as SyntectColor;
 use syntect::highlighting::FontStyle;
 use syntect::highlighting::Highlighter;
 use syntect::highlighting::Style as SyntectStyle;
@@ -49,10 +50,23 @@ static THEME: OnceLock<RwLock<Theme>> = OnceLock::new();
 static THEME_OVERRIDE: OnceLock<Option<String>> = OnceLock::new();
 static CODEX_HOME: OnceLock<Option<PathBuf>> = OnceLock::new();

+// Syntect/bat encode ANSI palette semantics in alpha:
+// `a=0` => indexed ANSI palette via RGB payload, `a=1` => terminal default.
+const ANSI_ALPHA_INDEX: u8 = 0x00;
+const ANSI_ALPHA_DEFAULT: u8 = 0x01;
+const OPAQUE_ALPHA: u8 = 0xFF;
+
 fn syntax_set() -> &'static SyntaxSet {
    SYNTAX_SET.get_or_init(two_face::syntax::extra_newlines)
 }

+// NOTE: We intentionally do NOT emit a runtime diagnostic when an ANSI-family
+// theme (ansi, base16, base16-256) lacks the expected alpha-channel marker
+// encoding.  If the upstream two_face/syntect theme format changes, the
+// `ansi_themes_use_only_ansi_palette_colors` test will catch it at build
+// time — long before it reaches users.  A runtime warning would be
+// unactionable noise since users can't fix upstream themes.
+
 /// Set the user-configured syntax theme override and codex home path.
 ///
 /// Call this with the **final resolved config** (after onboarding, resume, and
@@ -62,15 +76,13 @@ fn syntax_set() -> &'static SyntaxSet {
 /// Subsequent calls cannot change the persisted `OnceLock` values, but they
 /// still update the runtime theme immediately for live preview flows.
 ///
-/// Returns a warning message when the configured theme name cannot be
-/// resolved to a bundled theme or a custom `.tmTheme` file on disk.
-/// The caller should surface this via `Config::startup_warnings` so it
-/// appears as a `⚠` banner in the TUI.
+/// Returns user-facing warnings for actionable configuration issues, such as
+/// unknown/invalid theme names or duplicate override persistence.
 pub(crate) fn set_theme_override(
    name: Option<String>,
    codex_home: Option<PathBuf>,
 ) -> Option<String> {
-    let mut warning = validate_theme_name(name.as_deref(), codex_home.as_deref());
+    let warning = validate_theme_name(name.as_deref(), codex_home.as_deref());
    let override_set_ok = THEME_OVERRIDE.set(name.clone()).is_ok();
    let codex_home_set_ok = CODEX_HOME.set(codex_home.clone()).is_ok();
    if THEME.get().is_some() {
@@ -80,11 +92,10 @@ pub(crate) fn set_theme_override(
        ));
    }
    if !override_set_ok || !codex_home_set_ok {
-        let duplicate_msg = "Ignoring duplicate or late syntax theme override persistence; runtime theme was updated from the latest override, but persisted override config can only be initialized once.";
-        tracing::warn!("{duplicate_msg}");
-        if warning.is_none() {
-            warning = Some(duplicate_msg.to_string());
-        }
+        // This should never happen in practice — set_theme_override is only
+        // called once at startup.  Keep as a debug breadcrumb in case a second
+        // call site is added in the future.
+        tracing::debug!("set_theme_override called more than once; OnceLock values unchanged");
    }
    warning
 }
@@ -109,15 +120,15 @@ pub(crate) fn validate_theme_name(name: Option<&str>, codex_home: Option<&Path>)
                return None;
            }
            return Some(format!(
-                "Syntax theme \"{name}\" was found at {custom_theme_path_display} \
-                 but could not be parsed. Falling back to auto-detection."
+                "Custom theme \"{name}\" at {custom_theme_path_display} could not \
+                 be loaded (invalid .tmTheme format). Falling back to the default theme."
            ));
        }
    }
    Some(format!(
-        "Unknown syntax theme \"{name}\", falling back to auto-detection. \
-         Use a bundled name or place a .tmTheme file at \
-         {custom_theme_path_display}"
+        "Theme \"{name}\" not found. Using the default theme. \
+         To use a custom theme, place a .tmTheme file at \
+         {custom_theme_path_display}."
    ))
 }

@@ -189,7 +200,7 @@ pub(crate) fn adaptive_default_theme_name() -> &'static str {
    adaptive_default_theme_selection().1
 }

-/// Build the theme from current override/auto-detection settings.
+/// Build the theme from current override/default-theme settings.
 /// Extracted from the old `theme()` init closure so it can be reused.
 fn resolve_theme_with_override(name: Option<&str>, codex_home: Option<&Path>) -> Theme {
    let ts = two_face::theme::extra();
@@ -206,13 +217,13 @@ fn resolve_theme_with_override(name: Option<&str>, codex_home: Option<&Path>) ->
        {
            return theme;
        }
-        tracing::warn!("unknown syntax theme \"{name}\", falling back to auto-detection");
+        tracing::debug!("Theme \"{name}\" not recognized; using default theme");
    }

    ts.get(adaptive_default_embedded_theme_name()).clone()
 }

-/// Build the theme from current override/auto-detection settings.
+/// Build the theme from current override/default-theme settings.
 /// Extracted from the old `theme()` init closure so it can be reused.
 fn build_default_theme() -> Theme {
    let name = THEME_OVERRIDE.get().and_then(|name| name.as_deref());
@@ -412,20 +423,71 @@ const BUILTIN_THEME_NAMES: &[&str] = &[

 // -- Style conversion (syntect -> ratatui) ------------------------------------

+/// Map a low ANSI palette index (0–7) to ratatui's named color variants,
+/// falling back to `Indexed(n)` for indices 8–255.
+///
+/// Named variants are preferred over `Indexed(0)`…`Indexed(7)` because many
+/// terminals apply bold/bright treatment differently for named vs indexed
+/// colors, and ANSI themes expect the named behavior.
+///
+/// `clippy::disallowed_methods` is explicitly allowed here because this helper
+/// intentionally constructs `ratatui::style::Color::Indexed`.
+#[allow(clippy::disallowed_methods)]
+fn ansi_palette_color(index: u8) -> RtColor {
+    match index {
+        0x00 => RtColor::Black,
+        0x01 => RtColor::Red,
+        0x02 => RtColor::Green,
+        0x03 => RtColor::Yellow,
+        0x04 => RtColor::Blue,
+        0x05 => RtColor::Magenta,
+        0x06 => RtColor::Cyan,
+        // ANSI code 37 is "white", represented as `Gray` in ratatui.
+        0x07 => RtColor::Gray,
+        n => RtColor::Indexed(n),
+    }
+}
+
+/// Decode a syntect foreground `Color` into a ratatui color, respecting the
+/// alpha-channel encoding that bat's `ansi`, `base16`, and `base16-256` themes
+/// use to signal ANSI palette semantics instead of true RGB.
+///
+/// Returns `None` when the color signals "use the terminal's default
+/// foreground", allowing the caller to omit the foreground attribute entirely.
+///
+/// Passing a color from a standard RGB theme (alpha 0xFF) returns
+/// `Some(Rgb(..))`, so this function is backward-compatible with non-ANSI
+/// themes. Unexpected intermediate alpha values are treated as RGB.
+///
+/// `clippy::disallowed_methods` is explicitly allowed here because this helper
+/// intentionally constructs `ratatui::style::Color::Rgb`.
+#[allow(clippy::disallowed_methods)]
+fn convert_syntect_color(color: SyntectColor) -> Option<RtColor> {
+    match color.a {
+        // Bat-compatible encoding used by `ansi`, `base16`, and `base16-256`:
+        // alpha 0x00 means `r` stores an ANSI palette index, not RGB red.
+        ANSI_ALPHA_INDEX => Some(ansi_palette_color(color.r)),
+        // alpha 0x01 means "use terminal default foreground/background".
+        ANSI_ALPHA_DEFAULT => None,
+        OPAQUE_ALPHA => Some(RtColor::Rgb(color.r, color.g, color.b)),
+        // Non-ANSI alpha values appear in some bundled themes; treat as plain RGB.
+        _ => Some(RtColor::Rgb(color.r, color.g, color.b)),
+    }
+}
+
 /// Convert a syntect `Style` to a ratatui `Style`.
 ///
-/// Syntax highlighting themes inherently produce RGB colors, so we allow
-/// `Color::Rgb` here despite the project-wide preference for ANSI colors.
-#[allow(clippy::disallowed_methods)]
+/// Most themes produce RGB colors. The built-in `ansi`/`base16`/`base16-256`
+/// themes encode ANSI palette semantics in the alpha channel, matching bat.
 fn convert_style(syn_style: SyntectStyle) -> Style {
    let mut rt_style = Style::default();

-    // Map foreground color when visible.
-    let fg = syn_style.foreground;
-    if fg.a > 0 {
-        rt_style = rt_style.fg(RtColor::Rgb(fg.r, fg.g, fg.b));
+    if let Some(fg) = convert_syntect_color(syn_style.foreground) {
+        rt_style = rt_style.fg(fg);
    }
    // Intentionally skip background to avoid overwriting terminal bg.
+    // If background support is added later, decode with `convert_syntect_color`
+    // to reuse the same alpha-marker semantics as foreground.

    if syn_style.font_style.contains(FontStyle::BOLD) {
        rt_style.add_modifier |= Modifier::BOLD;
@@ -500,10 +562,16 @@ pub(crate) fn exceeds_highlight_limits(total_bytes: usize, total_lines: usize) -

 // -- Core highlighting --------------------------------------------------------

-/// Parse `code` using syntect for `lang` and return per-line styled spans.
-/// Each inner Vec represents one source line.  Returns None when the language
-/// is not recognized or the input exceeds safety limits.
-fn highlight_to_line_spans(code: &str, lang: &str) -> Option<Vec<Vec<Span<'static>>>> {
+/// Core highlighter that accepts an explicit theme reference.
+///
+/// This keeps production behavior and test behavior on the same code path:
+/// production callers pass the global theme lock, while tests can pass a
+/// concrete theme without mutating process-global state.
+fn highlight_to_line_spans_with_theme(
+    code: &str,
+    lang: &str,
+    theme: &Theme,
+) -> Option<Vec<Vec<Span<'static>>>> {
    // Empty input has nothing to highlight; fall back to the plain text path
    // which correctly produces a single empty Line.
    if code.is_empty() {
@@ -518,11 +586,7 @@ fn highlight_to_line_spans(code: &str, lang: &str) -> Option<Vec<Vec<Span<'stati
    }

    let syntax = find_syntax(lang)?;
-    let theme_guard = match theme_lock().read() {
-        Ok(theme_guard) => theme_guard,
-        Err(poisoned) => poisoned.into_inner(),
-    };
-    let mut h = HighlightLines::new(syntax, &theme_guard);
+    let mut h = HighlightLines::new(syntax, theme);
    let mut lines: Vec<Vec<Span<'static>>> = Vec::new();

    for line in LinesWithEndings::from(code) {
@@ -546,6 +610,17 @@ fn highlight_to_line_spans(code: &str, lang: &str) -> Option<Vec<Vec<Span<'stati
    Some(lines)
 }

+/// Parse `code` using syntect for `lang` and return per-line styled spans.
+/// Each inner Vec represents one source line.  Returns None when the language
+/// is not recognized or the input exceeds safety limits.
+fn highlight_to_line_spans(code: &str, lang: &str) -> Option<Vec<Vec<Span<'static>>>> {
+    let theme_guard = match theme_lock().read() {
+        Ok(theme_guard) => theme_guard,
+        Err(poisoned) => poisoned.into_inner(),
+    };
+    highlight_to_line_spans_with_theme(code, lang, &theme_guard)
+}
+
 // -- Public API ---------------------------------------------------------------

 /// Highlight code in any supported language, returning styled ratatui `Line`s.
@@ -596,6 +671,7 @@ pub(crate) fn highlight_code_to_styled_spans(
 #[cfg(test)]
 mod tests {
    use super::*;
+    use insta::assert_snapshot;
    use pretty_assertions::assert_eq;
    use std::str::FromStr;
    use syntect::highlighting::Color as SyntectColor;
@@ -673,6 +749,25 @@ mod tests {
            .join("\n")
    }

+    fn unique_foreground_colors_for_theme(theme_name: &str) -> Vec<String> {
+        let theme = resolve_theme_by_name(theme_name, None)
+            .unwrap_or_else(|| panic!("expected built-in theme {theme_name} to resolve"));
+        let lines = highlight_to_line_spans_with_theme(
+            "fn main() { let answer = 42; println!(\"hello\"); }\n",
+            "rust",
+            &theme,
+        )
+        .expect("expected highlighted spans");
+        let mut colors: Vec<String> = lines
+            .iter()
+            .flat_map(|line| line.iter().filter_map(|span| span.style.fg))
+            .map(|fg| format!("{fg:?}"))
+            .collect();
+        colors.sort();
+        colors.dedup();
+        colors
+    }
+
    fn theme_item(scope: &str, background: Option<(u8, u8, u8)>) -> ThemeItem {
        ThemeItem {
            scope: ScopeSelectors::from_str(scope).expect("scope selector should parse"),
@@ -791,7 +886,6 @@ mod tests {
    }

    #[test]
-    #[allow(clippy::disallowed_methods)]
    fn convert_style_suppresses_underline() {
        // Dracula (and other themes) set FontStyle::UNDERLINE on type scopes,
        // producing distracting underlines on type names in terminal output.
@@ -807,7 +901,7 @@ mod tests {
                r: 0,
                g: 0,
                b: 0,
-                a: 0,
+                a: 0xFF,
            },
            font_style: FontStyle::UNDERLINE,
        };
@@ -820,6 +914,138 @@ mod tests {
        );
    }

+    #[test]
+    fn style_conversion_uses_ansi_named_color_when_alpha_is_zero_low_index() {
+        let syn = SyntectStyle {
+            foreground: syntect::highlighting::Color {
+                r: 0x02,
+                g: 0,
+                b: 0,
+                a: 0,
+            },
+            background: syntect::highlighting::Color {
+                r: 0,
+                g: 0,
+                b: 0,
+                a: 0xFF,
+            },
+            font_style: FontStyle::empty(),
+        };
+        let rt = convert_style(syn);
+        assert_eq!(rt.fg, Some(RtColor::Green));
+    }
+
+    #[test]
+    fn style_conversion_uses_indexed_color_when_alpha_is_zero_high_index() {
+        let syn = SyntectStyle {
+            foreground: syntect::highlighting::Color {
+                r: 0x9a,
+                g: 0,
+                b: 0,
+                a: 0,
+            },
+            background: syntect::highlighting::Color {
+                r: 0,
+                g: 0,
+                b: 0,
+                a: 0xFF,
+            },
+            font_style: FontStyle::empty(),
+        };
+        let rt = convert_style(syn);
+        assert!(matches!(rt.fg, Some(RtColor::Indexed(0x9a))));
+    }
+
+    #[test]
+    fn style_conversion_uses_terminal_default_when_alpha_is_one() {
+        let syn = SyntectStyle {
+            foreground: syntect::highlighting::Color {
+                r: 0,
+                g: 0,
+                b: 0,
+                a: 1,
+            },
+            background: syntect::highlighting::Color {
+                r: 0,
+                g: 0,
+                b: 0,
+                a: 0xFF,
+            },
+            font_style: FontStyle::empty(),
+        };
+        let rt = convert_style(syn);
+        assert_eq!(rt.fg, None);
+    }
+
+    #[test]
+    fn style_conversion_unexpected_alpha_falls_back_to_rgb() {
+        let syn = SyntectStyle {
+            foreground: syntect::highlighting::Color {
+                r: 10,
+                g: 20,
+                b: 30,
+                a: 0x80,
+            },
+            background: syntect::highlighting::Color {
+                r: 0,
+                g: 0,
+                b: 0,
+                a: 0xFF,
+            },
+            font_style: FontStyle::empty(),
+        };
+        let rt = convert_style(syn);
+        assert!(matches!(rt.fg, Some(RtColor::Rgb(10, 20, 30))));
+    }
+
+    #[test]
+    fn ansi_palette_color_maps_ansi_white_to_gray() {
+        assert_eq!(ansi_palette_color(0x07), RtColor::Gray);
+    }
+
+    #[test]
+    fn ansi_family_themes_use_terminal_palette_colors_not_rgb() {
+        for theme_name in ["ansi", "base16", "base16-256"] {
+            let theme = resolve_theme_by_name(theme_name, None)
+                .unwrap_or_else(|| panic!("expected built-in theme {theme_name} to resolve"));
+            let lines = highlight_to_line_spans_with_theme(
+                "fn main() { let answer = 42; println!(\"hello\"); }\n",
+                "rust",
+                &theme,
+            )
+            .expect("expected highlighted spans");
+            let mut has_non_default_fg = false;
+            for line in &lines {
+                for span in line {
+                    match span.style.fg {
+                        Some(RtColor::Rgb(..)) => {
+                            panic!("theme {theme_name} produced RGB foreground: {span:?}")
+                        }
+                        Some(_) => has_non_default_fg = true,
+                        None => {}
+                    }
+                }
+            }
+            assert!(
+                has_non_default_fg,
+                "theme {theme_name} should produce at least one non-default foreground color"
+            );
+        }
+    }
+
+    #[test]
+    fn ansi_family_foreground_palette_snapshot() {
+        let mut out = String::new();
+        for theme_name in ["ansi", "base16", "base16-256"] {
+            let colors = unique_foreground_colors_for_theme(theme_name);
+            out.push_str(&format!("{theme_name}:\n"));
+            for color in colors {
+                out.push_str(&format!("  {color}\n"));
+            }
+        }
+        assert_snapshot!("ansi_family_foreground_palette", out);
+    }
+
    #[test]
    fn highlight_multiline_python() {
        let code = "def hello():\n    print(\"hi\")\n    return 42";
@@ -1152,7 +1378,7 @@ mod tests {
        assert!(
            warning
                .as_deref()
-                .is_some_and(|msg| msg.contains("could not be parsed")),
+                .is_some_and(|msg| msg.contains("could not be loaded")),
            "warning should explain that the theme file is invalid"
        );
    }
--- a/codex-rs/tui/src/render/snapshots/codex_tuirenderhighlighttestsansi_family_foreground_palette.snap
+++ b/codex-rs/tui/src/render/snapshots/codex_tuirenderhighlighttestsansi_family_foreground_palette.snap
@@ -0,0 +1,21 @@
+---
+source: tui/src/render/highlight.rs
+expression: out
+---
+ansi:
+  Blue
+  Green
+  Magenta
+  Yellow
+base16:
+  Blue
+  Gray
+  Green
+  Indexed(9)
+  Magenta
+base16-256:
+  Blue
+  Gray
+  Green
+  Indexed(16)
+  Magenta