awslabs / awslabs/cli-agent-orchestrator
[Feat] cao tui: semantic colour layer — ANSI-16 theme tokens, NO_COLOR support, and an NFR-3 strip-styling guard
- Dominant language
- Python
- Stars
- 1.3k
- Forks
- 267
- Avg merge
- 1d 23h
- Merged PRs (30d)
- 70
Description
## Summary
The Rust TUI (`cao tui`, #321 / PR #547) currently renders **entirely unstyled** — verified: zero `Color::` or `Modifier::` references in any of the 10 production modules under `tui/src/`. This proposes a small, semantic colour layer.
The reason this is lightweight rather than a design project: **NFR-3 already did the hard part.** The affirmed rule is *"no state conveyed by colour alone"*, and every state marker is already textual — `✓`, `!`, `⚠`, `[unavailable]`, `● running`, and the literal exit code. So colour is *decorative reinforcement only*. We are not deciding what colour **means**; we are deciding what it **emphasizes**. Nothing breaks if it is absent.
That also means the existing suite stays valid: all renderer tests read the buffer via `.symbol()` (text, not style), so adding `.style()` cannot redden any of the current **152** tests.
## Proposed approach
**1. One `tui/src/theme.rs` holding semantic tokens, not colours at call sites.**
```rust
pub struct Theme {
pub focus: Style, // active region border, selected row
pub required: Style, // the `!` unmet-required marker
pub ok: Style, // `✓`, exit 0
pub error: Style, // failures, non-zero exit, FR-5.3 refusals
pub warn: Style, // `⚠`
pub dim: Style, // `[unavailable]` rows, key hints
}
```
Call sites say `theme.required`, never `Color::Yellow`. One file to audit, one file to swap — the same "put the decision in one reviewable place" discipline as `catalog.rs`.
**2. ANSI-16 only (`Color::Cyan`), never RGB or 256-colour.** The 16 base colours resolve against the *user's own* terminal palette, so they respect the theme the operator already chose and behave over SSH, in `tmux`, and in `screen`. Hardcoded RGB fights their setup.
**Avoid `Black` and `White` entirely** — they invert catastrophically between light and dark terminals. Use `Color::Reset` for default foreground.
**3. A deliberately tiny palette — 6 roles:**
| Role | Colour | Applied to |
|---|---|---|
| focus | `Cyan` | focused region border, selected row |
| required-unmet | `Yellow` | `!` plus "required" |
| ok | `Green` | `✓`, `exit 0` |
| error | `Red` | failures, non-zero exit, refusals |
| warn | `Yellow` | `⚠` |
| dim | `DarkGray` | `[unavailable]`, hints, collapsed strip |
**4. Honour `NO_COLOR`.** A `Theme::from_env()` returning a monochrome theme when `NO_COLOR` is set (per ). Cheap, and it is what operators with unusual terminals will actually need. Also the natural place to respect a future `CAO_TUI_NO_COLOR`.
**5. The one non-negotiable test: render each state, strip all styling, assert the state is still identifiable from text alone.**
This is the guard that cannot be left as a convention. `accessibility-checklist.md` A-2 already names exactly this verification method — *"inspecting rendered text with styling stripped"* — it simply was never automated. Without it, someone later removes a textual marker because "the colour already shows it", and **the regression is invisible on a colour terminal**. That is the whole failure mode NFR-3 exists to prevent.
## Non-goals
- No RGB / 24-bit / 256-colour support.
- No user-configurable colour scheme (a `Theme` struct leaves the door open; wiring config is out of scope).
- **No colour carrying meaning on its own** — that would violate NFR-3.
## Size estimate
~120 lines for `theme.rs`, ~80 lines of tests, plus mechanical call-site edits in `renderer.rs` and `results_pane.rs`. No new dependencies — `ratatui::style` is already in the tree.
## Acceptance
- [ ] `theme.rs` exists; no `Color::` literal appears outside it (a source-text guard, mirroring the existing `no_backend_attach_call.rs` tripwire pattern)
- [ ] `NO_COLOR` yields a monochrome render
- [ ] A strip-styling test proves every state is legible without colour (NFR-3 / A-2)
- [ ] No `Color::Black` / `Color::White`
- [ ] `cargo fmt --check`, `cargo clippy --locked --all-targets -- -D warnings`, `cargo test --locked` all green
## Context
- Depends on #321 / PR #547 landing.
- Blocked on nothing else; purely additive.
- A separate idea discussed alongside this — a **contributor scaffold** for adding a new command (enum variant → catalog row → route arm → `DISPLAY_ORDER` → `COMMAND_COUNT`) — is deliberately *not* included here and deserves its own issue.
Contributor guide
Assessment
This issue has not been assessed yet.