feat(tui): semantic colour layer, folded pickers, and reveal-before-run (#556)

[fanhongy]

What

Two related changes to the cao tui front door.

1. The semantic colour layer (#556). A new tui/src/theme.rs defines six
semantic roles over an ANSI-16 palette, honours NO_COLOR, and the frame's
regions move from Vec<String> to Vec<Line<'static>> so styling lives at one
seam. tui/tests/no_colour_literal_outside_theme.rs is the NFR-3 guard: no
production module outside the theme may name a colour, and no colour may be
non-ANSI or inverting.

FR-1.5 / FR-1.7 are preserved — an unloadable profile and an uninstalled
provider stay listed with their textual marker, never filtered.

2. Bounding the left column (no issue — see below). The left column is one
non-scrolling Paragraph, so anything past the last row was silently clipped.
At 40 rows the 42-command list alone consumed the whole column and the pickers,
form and banner never painted at all.

• The command list now windows around the cursor.
• Both pickers are collapsed-by-default folds: ▸ agents (25, 3 unloadable).
  Enter/Space toggles, Up/Down scroll the expanded window, Esc
  collapses. Every row stays reachable, and the residue count names what is
  off-window in each direction so a partial view never reads as complete.
• The collapsed header carries the diagnosis count forward, so folding hides
  the rows without hiding the fact.
• First Enter reveals folded options; the second runs. Previously Enter
  on the launch form ran the command immediately, before the operator had seen
  the optional args. The condition is the visible fold state, not a press
  counter — an operator who expanded with Space does not then owe a wasted
  Enter.

Verification

• Full suite: 225 passed, 0 failed (cargo test --locked --no-fail-fast),
  186 of them unit tests on the binary.
• cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings and
  scripts/assert_no_ffi.py were all clean on this exact tree; no source
  changed between those runs and this commit, but CI is the authority here — I
  did not re-run them immediately before committing.
• Mutation-tested, 8 mutations, all caught. One initially survived
  (deleting the fold's diagnosis clause, because an existing assert matched
  only a prefix of the header); a_collapsed_fold_still_states_how_many_rows_are_unavailable
  closes it.
• Verified in a real pty at 40x120 with live data — collapsed, expanded and
  scrolled (… 12 above, 3 below), and one Enter expanding rather than
  running. This mattered: the fold's own unit tests were green while nothing
  rendered, because frame.pickers is populated whether or not it survives
  layout. at_a_realistic_size_every_region_survives_the_layout now asserts on
  the drawn buffer instead.

Notes for review

• No issue covers change 2. I searched the open issues and found none; I did
  not want to cite a number that resolves to unrelated work, so the new code
  carries no issue citation. Happy to file one and amend if you'd prefer.
• The #556 spec artifacts under docs/issues/556-tui-semantic-colour-layer/
  are deliberately git-excluded and are not in this branch.
• tasks.md proposed splitting [Feat] cao tui: semantic colour layer — ANSI-16 theme tokens, NO_COLOR support, and an NFR-3 strip-styling guard #556 into two commits (migration, then
  styling); this ships as one, with the layout work folded in as a third
  logical change. Say the word if you want it split for review.
• Unverified: dark/light legibility is a human judgement call, and neither the
  ssh path nor the Linux CI leg has been exercised locally.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR enhances the cao tui entrypoint by introducing a semantic color/theme layer (ANSI-16, NO_COLOR aware) and by making the left column reliably usable on shorter terminals via windowing and foldable pickers, including “reveal-before-run” behavior for Enter.

Changes:

• Add tui/src/theme.rs with six semantic roles over an ANSI-16 palette and NO_COLOR support; thread the theme through the renderer and results pane.
• Add/extend tripwire tests to enforce “no Color:: literals outside theme”, forbid RGB/Indexed and Black/White (with scoped allowances), and keep source scans coverage-complete.
• Rework the left-column UX: window the command list, fold pickers by default with scrolling, and make first Enter reveal folded options before running.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tui/tests/no_colour_literal_outside_theme.rs	New guard ensuring only src/theme.rs can name Color, plus palette-shape constraints and anti-vacuity checks.
tui/tests/no_backend_attach_call.rs	Extend embedded source list to include src/theme.rs so the subprocess tripwire still fully covers src/.
tui/tests/hermeticity_tripwire.rs	Extend embedded source list (including the new guard test) and add a mod-declaration cross-check to prevent unscanned files.
tui/src/theme.rs	Introduce the semantic theme (ANSI-16 only) with NO_COLOR selection logic and exhaustive self-tests.
tui/src/results_pane.rs	Store and apply Theme to pane rendering (footer/body/collapsed strip), with tests asserting style behavior and strip-styling invariants.
tui/src/renderer.rs	Migrate frame regions to Vec<Line>, apply semantic styles at one seam, add foldable/scrolled pickers + command list windowing + reveal-then-run behavior, and update tests accordingly.
tui/src/main.rs	Read NO_COLOR once at startup, set the renderer theme, and document piped output expectations with a test-backed invariant.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[haofeif]

Summary: the semantic theme work looks sound, but the layout changes have two P2 usability defects. First, expanding all sections can overflow the non-scrolling left pane, so the options claimed as revealed remain clipped at supported terminal sizes; the first Enter still prevents execution, which is why this is P2 rather than P1. Second, loading, empty, and failed picker states remain in the focus ring without rendering any focus marker, making Tab appear to do nothing.

[haofeif]

[P2] Keep revealed controls inside the rendered viewport. I reproduced this with the existing fake at 120x40 and 25 agents: after the first Enter, this expands the optional section and both pickers into the one non-scrolling left Paragraph; the drawn buffer ends midway through --memory and contains neither the agent nor provider fold, while has_folded_options() is already false and the footer says the next Enter will run. The second Enter can therefore execute without the operator ever seeing the rows that were supposedly revealed. The sizing also fails before expansion at the supported 80x24 boundary: the collapsed provider header is clipped because command_list_window() bounds logical entries, not their wrapped screen rows. A scrollable left viewport that follows focus, or one shared rendered-row budget, is needed; independent rows/3 and rows/4 caps cannot guarantee that the composed column fits.

[haofeif]

[P2] Preserve visible focus when a picker has no rows. The focus ring now always visits AgentPicker and ProviderPicker, but only the Loaded(non-empty) arm calls fold_lines(..., focused) and emits a > marker. Loading, empty, and Failed render an unmarked line. Reproduced with the server-down fake: tabbing to AgentPicker leaves the two failure lines byte-for-byte unchanged with no structural focus cue, so Tab appears to do nothing for two stops in the ring. Either skip non-foldable picker states in the focus order or render their state line with the same focus marker/style.

[call-me-ram]

The colour layer is the strongest half of this and I'd take it as-is. I planted a Color::Yellow in renderer.rs's production region and the NFR-3 guard failed with the exact file:line; I moved the same literal into a comment and it went green — so the guard is genuinely load-bearing in both directions, not just an absence check. NO_COLOR holds end to end: with the monochrome theme installed and an agent fold expanded, 0 of 4800 drawn cells at 120x40 carried a non-Reset foreground. FR-1.5/FR-1.7 are asserted at both layers — every_dim_marker_dims_its_marker_and_not_its_row reads the drawn buffer and checks the row is not dimmed as a whole, and a_collapsed_fold_still_states_how_many_rows_are_unavailable pins the diagnosis into the collapsed header (agents (3, 2 unloadable), providers (2, 1 not installed)) with the , 0 unloadable negative case alongside it. cargo fmt --check and cargo clippy --locked --all-targets -- -D warnings are both clean, and the unstable-rendered-line-info claim that Cargo.lock is unchanged checks out — --locked passes with no lockfile in the diff.

The layout half has two defects I can reproduce, and one of them re-creates the exact accident this PR was written to prevent. Requesting changes on those two; both fixes are small.

Must-fix

1. [enter] on the expanded optional section launches the command, while the header it is sitting on advertises [enter] expand/collapse — renderer.rs:1625 (on_key_form's Enter arm) against the header string at renderer.rs:1051.

With focus on Focus::OptionalSection and the section already expanded, on_key_form(key, false) routes Enter to reveal_options_or_run(). Nothing is folded at that point, so it falls straight through to run_selected(). Driven through the real key handler:

header advertises: "> ▾ optional (9) — [enter] expand/collapse"
optional_expanded before = true
Enter consumed = true
optional_expanded after  = true          <- did not collapse
pending_action after     = Some(Launch)  <- it launched
running = true
banner = Some(Banner { severity: "info", what: "creating the session", ... })

The operator reads [enter] expand/collapse on the line their focus marker is on, presses [enter] to fold the section back up, and creates a session instead. That is the failure reveal_options_or_run's own docstring describes at renderer.rs:1723 — "the operator asking for this described running the CLI by accident while trying to open the options" — arriving one keystroke later than before.

The string is pre-existing (it is on main at renderer.rs:801), so this is not a regression you introduced. I'm calling it a must-fix here because this PR is what makes it inconsistent and what makes it reachable as a deliberate keystroke: your new picker folds advertise [enter] collapse when expanded and honour it (on_key_fold:1669), and your new footer says then [enter] to run. So the same advertised affordance now means "collapse" on two of three folds and "launch a session" on the third, and the operator has just been trained by the footer that [enter] is the reveal key.

Minimal fix — make the header advertise the key that is actually wired, since Esc already collapses at renderer.rs:1633:

// renderer.rs:1049-1053
let glyph = if self.optional_expanded { "▾" } else { "▸" };
let action = if self.optional_expanded { "[esc] collapse" } else { "[enter] expand" };
let mut lines = vec![format!("{marker} {glyph} optional ({count}) — {action}", ...)];

The alternative — giving the section the picker folds' Enter-toggles semantics — is more consistent but costs you the "second [enter] runs" contract whenever focus happens to rest there, so I'd take the honest label.

2. fold_lines clamps the scroll offset to the last row, not the last window, contradicting its own docstring — renderer.rs:1231.

let start = scroll.min(rows.len().saturating_sub(1));

on_key_fold's doc at renderer.rs:1649-1651 states the opposite and is what a reader will believe: "Scrolling is clamped to the last window, not the last row: allowing scroll to reach len - 1 would let the operator scroll into a view holding a single row with empty space below it." on_key_fold's Down arm does clamp correctly (rows.saturating_sub(window)), so the two disagree, and any path that changes rows or window without a keypress lands in the gap. Copilot flagged the line; here are the two concrete reproductions.

Terminal resized taller. 25 agents, expanded at 100x24 (picker_window() = 6), scrolled to the bottom (agent_scroll = 19), then resize(100, 200) so the window becomes 50 and the whole list fits:

--- pickers after resize to 200 rows ---   (window = 50, rows = 25)
> ▾ agents (25) — [enter] collapse
  agent-19 … agent-24
    … 19 above, [↑↓] scroll

Six of 25 rows in a pane with room for all 25. Then pressing Down — the key the residue line advertises — sets last = 25.saturating_sub(50) = 0 and snaps scroll from 19 to 0, so Down scrolls up to the top of the list.

[ctrl+r] returns a shorter list. retry() does not reset agent_scroll, so with scroll still 19 and a refetch answering 2 profiles:

scroll after retry = 19, rows = 2
> ▾ agents (2) — [enter] collapse
  only-two
    … 1 above, [↑↓] scroll

The header says 2, one row is drawn, and only-one is off-window. Up decrements scroll 19→18→17… while start = scroll.min(1) stays pinned at 1, so the key advertised to reveal what is above does nothing visible for 18 presses. That is a row the operator cannot reach by the advertised means, which is the defect the fold was built to fix.

One-line fix, which I applied and re-ran:

let start = scroll.min(rows.len().saturating_sub(window));

After it, the resize case shows all 25 from agent-00 with no residue, and the refetch case shows both only-one and only-two. The full binary suite stays green either way (200 passed with the fix in) — so nothing currently covers this and it wants a regression test. A resize-after-scroll case is the cheaper of the two to write, and resetting the two *_scroll fields in retry() would close the refetch path at the source as well as at the render.

Non-blocking

1. tui/Cargo.toml:108 justifies taking the unstable ratatui feature by naming its pin: "wrapped_heights_agrees_with_what_is_actually_drawn pins the behaviour against a real rendered buffer". There is no test by that name — grep -rn wrapped_heights_agrees matches only the comment. The real test is the_wrap_measurement_agrees_with_what_is_actually_drawn at renderer.rs:7737, and it does exactly what the comment claims. Worth fixing the citation precisely because this is the one place in the diff where a reviewer is asked to accept an unstable upstream dependency on the strength of a named guard.

2. every_revealed_region_is_reachable_at_the_minimum_supported_size (renderer.rs:7676) asserts only drawn.lines().any(|line| line.starts_with('>')). That happens to be sound today because exactly one line carries the marker at a time (I checked — with focus on OptionalSection the count is 1), but it would pass if the wrong region's marker were on screen. Asserting the marked line also contains the region's own label would make it prove what its name says.

3. render_form's bare " … form scrolled" branch (renderer.rs:2545) looks unreachable. The branch needs parts.is_empty(), i.e. scroll == 0 && below == 0; but it is only entered when total > area.height, and viewport = area.height - 1, so with scroll == 0 you get below = total - viewport >= 2. It never fired in any of the sizes I rendered. Either drop it or note why it is kept.

4. " … {above} below of {total} commands" at renderer.rs:955 reads as "34 below of 42 commands" in the drawn output. The number is right (8 shown + 34 = 42); the word order fights it.

Asks

The footer is clipped at the 80-column floor and loses [q] quit. Not yours — the hint string and the Constraint::Length(frame.footer.len()) sizing are both unchanged from main — but it is the same bug class this PR exists to fix, one region down, and you have just built the tool for it. The hint is 89 characters, the region is 80 wide and gets Length(2) for two logical lines, so the hint wraps to two rows into one row of space:

[tab] focus · [←] commands · [enter] run · [k] stop following · [ctrl+r] retry ·

drawn.contains("[q] quit") is false at 80x24. Sizing header and footer with wrapped_heights(&frame.footer, area.width).iter().sum() instead of .len() would fix it with the function you added in this PR. Separate commit or separate PR is fine — I'd just like it not to sit there, since NFR-6's rule is "wrap, never truncate" and this is a truncation.

Relatedly and also pre-existing: that same hint advertises [k] stop following in every non-text-entry focus, but [k] only does anything in Focus::Results. Given #547's precedent on advertising unwired keys, it belongs on the same list.

On the bundle. I'd take it, and I want the trade-off named rather than waved through. Change 2 has no tracking issue, and it is the half with both must-fixes — the themed refactor is mechanical and test-guarded, the layout work is where the judgement calls are. What makes the bundle reviewable anyway is that Vec<String> → Vec<Line<'static>> is a genuine prerequisite: render_form's viewport measures wrapped height per line, which needs the Line values, so splitting would mean landing the migration and then immediately reworking the same call sites. The cost is that a +2413/-98 diff on one file makes the layout logic hard to isolate, and it did — the two defects above are both in the un-issued half. Please file the issue for change 2 and cite it, so the layout work has a requirements trail like the colour layer does. Not a merge blocker.

On the red CI. Security Scan is the only failing check and it is not this PR. I confirmed rather than assumed: the same check is failure on main at e592b21e, which is not a commit from this branch, and #569 ("bump js-yaml to 4.3.1 and make the Trivy gate legible") is the fix in flight. Worth noting that the failing job's log contains no finding at all — the Trivy step emits SARIF and exits 1, so ##[error]Process completed with exit code 1 is the entire signal, which is exactly what #569's second half is for.

On haofeif's review. Both of their P2s were real and both are addressed by 75e85a7, which I verified at head rather than taking on trust:

• Revealed controls stayed outside the viewport at 80x24 — confirmed and now fixed. At 80x24 with 25 agents, after the reveal press, tabbing to AgentPicker and then ProviderPicker brings each fold into the drawn buffer with its > marker, and the residue line names what is off-screen. Their diagnosis that independent rows/3 and rows/4 caps cannot bound the composed column was the right call, and the shared scrolling viewport is the right answer to it.
• Loading/empty/Failed pickers sat in the Tab ring with no focus marker — confirmed and now fixed by routing all four arms through focus_marked. One note: a focused failed picker takes theme.error rather than theme.focus, because style_pickers tests PICKER_FAILURE_MARKER before the > prefix (renderer.rs:2757 vs 2770). I think that's the right precedence and NFR-3 is satisfied by the textual >, but it is worth a line in style_pickers' docs so it reads as a decision.

I'd dispute nothing in their review. What neither round caught is the two items above — the [enter]-launches-from-the-optional-header path, and the render-side scroll clamp behind the resize and refetch paths.

What I verified

No system Rust toolchain on this box, so I bootstrapped one: rustup stable-x86_64-unknown-linux-gnu into a scratch CARGO_HOME, and — with no cc and no root to install one — zig cc (zig 0.13.0) as the linker driver. Everything below therefore ran for real; nothing here is inferred from reading.

Worktree at 75e85a7f (PR head), git worktree add off refs/remotes/pr/564, removed afterwards.

• cargo test --locked --no-fail-fast → 226 passed, 3 failed. All three failures are tests/endpoint_contract.rs refusing to reach cao-server at 127.0.0.1:9889 (Connection refused) — environmental, and that file fails rather than skips by design (BR-7). Every other target green, including all 191 binary unit tests.
• cargo fmt --check → clean. cargo clippy --locked --all-targets -- -D warnings → clean, 0 warnings.
• NFR-3 guard, both directions. Planted let _planted = ratatui::style::Color::Yellow; inside plain() → no_production_module_outside_the_theme_names_a_colour FAILED, reporting src/renderer.rs:2580 — let _planted = ratatui::style::Color::Yellow;. Moved the identical literal into a // comment → all 7 guard tests pass. Also confirmed the guard's region split is sound: renderer.rs's first #[cfg(test)] is at line 3173 and the only top-level item after it is mod tests, so no production code sits outside the scanned region.
• NO_COLOR. With Theme::monochrome() installed and the agent fold expanded, filtered all 4800 drawn cells at 120x40 for a non-Reset foreground → 0. Also verified Renderer::set_theme propagates to ResultsPane (renderer.rs:654) and that ResultsPane::attach sets its fields individually and does not clobber theme — so the palette survives a run.
• Must-fix 1 reproduced through on_key with the launch form, required field filled, optional section revealed and focused: pending_action goes to Some(Launch) and running to true, with the header still reading [enter] expand/collapse.
• Must-fix 2 reproduced twice — resize 100x24 → 100x200 after scrolling to the bottom, and a [ctrl+r] refetch shrinking 25 profiles to 2. Applied the one-line clamp change, re-ran: both views correct, and the binary suite still 200 passed / 0 failed.
• Extreme sizes, no panic. Drew at 1x1, 1x2, 2x1, 80x1, 1x24, 80x2, 80x3, 0x0, 3x3, 79x23 and 200x4, each after a reveal press, a Tab walk to ProviderPicker and five Down presses. No subtract-with-overflow anywhere — the saturating_sub discipline in the layout arithmetic holds up.
• Footer clipping measured directly: hint line is 89 chars, drawn.contains("[q] quit") is false at 80x24 while "stop following" is true.
• Focus markers: exactly one line carries > with focus on OptionalSection, so focused_line_index's first-match anchor cannot pick the wrong region today.
• CI: Security Scan = failure on main at e592b21e as well as on this PR; #569 is the in-flight fix.

[fanhongy]

Both must-fixes are in, plus the four non-blocking items and the footer ask. Four commits, 75e85a7..cf69b5b.

Thanks for the two reproductions — both were real, and the second one was reachable by two paths I had not considered.

Must-fix 1 — [enter] on the expanded optional header launched the command (37b55c6)

Reproduced before changing anything, and worth noting the exact path, because the obvious one does not trigger it: pressing [enter] on the collapsed header is absorbed as the reveal, since the pickers are still folded. The launch only fires when the reveal happened elsewhere and left nothing folded — reveal press on the required field, then [tab] to the now-expanded header:

header advertises: "> ▾ optional (9) — [enter] expand/collapse"
has_folded_options() = false
Enter consumed = true, optional_expanded after = true
pending_action = Some(Launch), running = true
create_session_calls after the tick = 1

Took your recommendation — the honest label — for the reason you gave: Enter-toggles semantics would cost the "second [enter] runs" contract whenever focus rests there. The expanded header now reads [esc] collapse, the collapsed one [enter] expand. Verified on the drawn screen that it round-trips [enter] expand → [esc] collapse → [enter] expand with create_session_calls staying 0.

the_expanded_optional_header_advertises_the_key_that_collapses_it asserts on create_session_calls, not on the label, so a build that relabels the header and leaves the launch wired still fails.

Must-fix 2 — the render clamp (37b55c6)

Applied your one-line fix and closed the refetch path at its source as you suggested: retry()'s Retryable::Pickers arm now resets both *_scroll fields. The fold flags are deliberately left alone — the operator opened them, and re-collapsing on a retry they asked for would hide the answer they were waiting for.

Both of your reproductions are now one test, a_stale_scroll_offset_never_hides_rows_that_now_fit, and I checked each half of the fix is independently load-bearing: reverting the clamp reddens the resize assertion, reverting the reset reddens the refetch assertion (left: 19, right: 0). You were right that nothing covered this — the suite was green either way before.

Non-blocking 1–4 (86f3e08, fbdfc0d)

1. Citation fixed to the_wrap_measurement_agrees_with_what_is_actually_drawn. Confirmed the cited name matched only the comment.
2. every_revealed_region_is_reachable_at_the_minimum_supported_size now asserts the marked line carries the region's own label.
3. Dropped the " … form scrolled" branch. Your reasoning holds: entry needs total > area.height, one row goes to the notice, so scroll == 0 forces below >= 1 and parts is never empty.
4. Reworded the command residue.

The footer ask (cf69b5b)

Fixed with wrapped_heights, both header and footer.

One correction to the scope: this affects Focus::Results only, not every non-text-entry focus. The form focus renders the shorter [enter] select/run · [ctrl+c] quit hint, which fits in 80 columns — so a drawn.contains("[q] quit") == false reading taken on the form is a false positive, since that hint never contains the string. Focus::Results is the one focus whose hint is the 89-character one, and there the defect is exactly as you measured. Post-fix at 80x24 on Focus::Results, [q] quit and [k] stop following are both drawn, with the form regions still on screen — the test asserts that second half too, so reclaiming rows by pushing the form off screen would fail it.

Also made [k] honest: advertised only in Focus::Results, which is the only arm routing it anywhere.

Still open

• The tracking issue for change 2. Not filed yet — I'd rather not open it without a number to cite here. Filing next and will edit this comment with the reference.
• [k]'s pre-existing half is covered above; nothing else from your review is outstanding.

Verification

cargo test --locked --all-targets → 195 binary unit tests plus every other target green. cargo fmt --check and cargo clippy --locked --all-targets -- -D warnings both clean. Did not run tests/endpoint_contract.rs against a live server — it fails by design without one (BR-7), which matches the 3 environmental failures you saw.

Extreme-size sweep after a reveal, a [tab] walk and five Down presses, with a stale offset resized both taller and shorter: 1x1, 1x2, 2x1, 80x1, 1x24, 80x2, 80x3, 0x0, 3x3, 79x23, 200x4, 120x40 — no panic.

Agreed on Security Scan being #569's, not this branch's.