Skip to content

fix: bright-yellow SGR crashes TUI log render#81

Merged
pbean merged 2 commits into
bmad-code-org:mainfrom
thesleepingsage:fix/tui-bright-yellow-color
Jul 9, 2026
Merged

fix: bright-yellow SGR crashes TUI log render#81
pbean merged 2 commits into
bmad-code-org:mainfrom
thesleepingsage:fix/tui-bright-yellow-color

Conversation

@thesleepingsage

@thesleepingsage thesleepingsage commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What

Fixes the TUI log pane rendering blank due to a ColorParseError on bright‑yellow output. Closes #80.

Why

pyte names SGR 93 brightbrown. _PYTE_COLOR_FIX remapped it to bright_yellow — a value that already carries the bright_ prefix. _rich_color then applies its "bright""bright_" transform unconditionally, doubling the underscore into bright__yellow, which rich rejects:

data._rich_color("brightbrown")   # before: 'bright__yellow'  -> ColorParseError

The _poll worker raises on the first bright‑yellow line (common in agent output), so bmad-loop tui's dashboard log pane never draws.

The fix

Keep the remap target in pyte's underscore‑free namespace so the downstream transform produces a valid color:

-_PYTE_COLOR_FIX = {"brown": "yellow", "brightbrown": "bright_yellow"}
+_PYTE_COLOR_FIX = {"brown": "yellow", "brightbrown": "brightyellow"}

Now brightbrownbrightyellowbright_yellow. I also expanded the comment above the table to record the invariant (remap values must stay underscore‑free) so it can't be reintroduced.

Test

Added test_rich_color_maps_pyte_names_to_valid_rich_colors to tests/test_tui_data.py: asserts the pyte→rich mapping for the brown/bright‑brown/bright/hex/default cases and that every non‑None result parses via rich.color.Color.parse — so a future invalid‑color regression fails the test instead of the TUI.

$ uv run --all-extras pytest tests/test_tui_data.py -q
59 passed
$ uv run --all-extras ruff check src/bmad_loop/tui/data.py tests/test_tui_data.py
All checks passed!

Scope

One‑line behavior change + comment + one test. No control‑loop logic touched.

Summary by CodeRabbit

  • Bug Fixes
    • Improved terminal UI color handling by remapping pyte “bright” color names to valid Rich display colors.
    • Hardened style rendering so unparseable color values no longer raise errors, falling back gracefully while preserving other text attributes (e.g., bold/underline).
  • Tests
    • Added tests to confirm pyte-to-Rich color translations always produce parseable values.
    • Added coverage to ensure unparseable colors degrade safely and caching behavior remains consistent.

pyte names SGR 93 'brightbrown'; _PYTE_COLOR_FIX remapped it to the
already-prefixed 'bright_yellow', which _rich_color then re-prefixed into
the invalid 'bright__yellow' — raising ColorParseError and blanking the
dashboard log pane on the first bright-yellow line (common in agent output).

Keep the remap in pyte's underscore-free namespace ('brightyellow') so the
'bright' -> 'bright_' transform produces a valid 'bright_yellow'. Add a
regression test asserting every mapped color parses via rich.
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b1f86561-38c2-4cc7-b03b-1946dc998ccf

📥 Commits

Reviewing files that changed from the base of the PR and between da01c27 and e592059.

📒 Files selected for processing (2)
  • src/bmad_loop/tui/data.py
  • tests/test_tui_data.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/bmad_loop/tui/data.py

Walkthrough

The TUI color helpers now avoid invalid Rich color names during pyte color conversion and fall back safely when Rich cannot parse a derived color. Tests cover both the remap behavior and the unparseable-color fallback.

Changes

TUI color parsing hardening

Layer / File(s) Summary
Fix pyte color remap and validate Rich parsing
src/bmad_loop/tui/data.py, tests/test_tui_data.py
brightbrown now maps to brightyellow before the later bright-prefix rewrite, and tests verify representative _rich_color outputs plus exhaustive parseability for installed pyte color names.
Fallback for unparseable styles
src/bmad_loop/tui/data.py, tests/test_tui_data.py
_char_style now catches ColorParseError, preserves non-color style attributes, and returns an attribute-only style when color parsing fails; tests confirm the fallback and cache reuse.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a rabbit with a cheerful hop,
I fixed the colors so they won’t stop.
No double underscores in the night,
Just brightyellow shining clean and right.
The log pane glows; I nibble joyfully. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The _char_style fallback that suppresses any unparseable color is broader than the linked bright-yellow remap fix. Split the generic color-parse fallback into a separate change or justify it in the issue so the PR stays focused on the reported crash.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the bright-yellow SGR crash fix in the TUI log renderer.
Linked Issues check ✅ Passed The remap fix matches #80 by making brightbrown produce a valid Rich color and adding regression coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Completes the bright__yellow fix for the whole failure class:

- pyte 0.8.2's BG_AIXTERM[105] is misspelled "bfightmagenta"; it passed
  through _rich_color untouched and raised the same ColorParseError on any
  background bright-magenta (SGR 105). Remap it alongside brown/brightbrown.
- Sweep every name in pyte's FG/BG/FG_AIXTERM/BG_AIXTERM tables through
  _rich_color in the regression test so a pyte bump that adds or renames
  entries fails in CI, not in the dashboard's poll worker.
- Guard _char_style: an unparseable color now degrades to an uncolored run
  instead of killing the poll worker (textual workers default to
  exit_on_error=True, so that crash could take the whole TUI down).
@pbean

pbean commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks @thesleepingsage — great catch and a clean diagnosis; the regression test was exactly the right shape. Confirmed the repro on main (_rich_color("brightbrown")bright__yellow) and validated the fix.

While reviewing I swept every name in pyte 0.8.2's color tables through _rich_color and found one more crash of the same class just out of this PR's reach: pyte's own BG_AIXTERM[105] entry is misspelled bfightmagenta (an upstream typo in pyte), which passes through unmapped and raises the same ColorParseError on any background bright-magenta (SGR 105). Since maintainer edits were enabled I pushed a completion commit (e592059) onto your branch rather than round-tripping a review:

  • adds "bfightmagenta": "brightmagenta" to _PYTE_COLOR_FIX
  • extends your test with an exhaustive sweep of pyte's FG/BG/FG_AIXTERM/BG_AIXTERM tables, so a future pyte bump that adds or renames entries fails in CI instead of in the dashboard
  • guards _char_style so an unparseable color degrades to an uncolored run instead of killing the poll worker — textual's @work defaults to exit_on_error=True, so that exception can take down the whole TUI, slightly worse than the blank pane described in TUI log pane blank: bright-yellow SGR -> 'bright__yellow' ColorParseError #80

Merging once CI is green.

@pbean pbean left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issue confirmed and reproduced on main; fix validated locally (full suite + an end-to-end LogView render of raw SGR 93/105 bytes). Completed the failure class in e592059 (pyte's bfightmagenta typo + exhaustive table sweep + graceful degrade in _char_style).

@pbean pbean merged commit 37a01f1 into bmad-code-org:main Jul 9, 2026
9 checks passed
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.

TUI log pane blank: bright-yellow SGR -> 'bright__yellow' ColorParseError

2 participants