From fc4d471b692fb2869ceb10fccbd35cfafd8698d4 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 01:46:19 +0800 Subject: [PATCH 01/15] feat(slash): /theme picker with live preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bare `/theme` opens a list picker whose Up / Down repaints the full TUI in the candidate theme; Esc snaps back to the original, Enter commits for the rest of the session. `/theme ` swaps directly. Session-only — restart returns to `[tui.theme] base` in the user config. Adds `ModalKey::Preview(ModalAction)` so live-preview modals can emit actions without popping; threads a `theme_name` field through `Config` / `ConfigSnapshot` so the picker marks the active row; and adds `set_theme` to `ChatView` / `StatusBar` / `InputArea` for mid-session repaint. --- CLAUDE.md | 3 +- crates/oxide-code/src/agent.rs | 4 +- crates/oxide-code/src/agent/event.rs | 9 + .../src/client/anthropic/testing.rs | 1 + crates/oxide-code/src/config.rs | 33 ++ crates/oxide-code/src/main.rs | 7 +- crates/oxide-code/src/slash.rs | 2 + crates/oxide-code/src/slash/registry.rs | 2 + crates/oxide-code/src/slash/theme.rs | 445 ++++++++++++++++++ crates/oxide-code/src/tui/app.rs | 121 ++++- crates/oxide-code/src/tui/components/chat.rs | 6 + crates/oxide-code/src/tui/components/input.rs | 9 + .../src/tui/components/input/popup.rs | 4 + ...r_empty_query_shows_each_command_once.snap | 1 + ...narrow_terminal_truncates_description.snap | 1 + .../oxide-code/src/tui/components/status.rs | 5 + crates/oxide-code/src/tui/modal.rs | 30 ++ crates/oxide-code/src/tui/theme.rs | 8 + docs/guide/slash-commands.md | 11 +- docs/roadmap.md | 7 +- 20 files changed, 699 insertions(+), 10 deletions(-) create mode 100644 crates/oxide-code/src/slash/theme.rs diff --git a/CLAUDE.md b/CLAUDE.md index 8fb015d3..29cd34c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,8 @@ ox # Start an interactive session │ ├── picker.rs # ModelEffortPicker — combined model + effort modal; emits a single SwapConfig │ ├── registry.rs # SlashCommand trait + SlashOutcome + BUILT_INS slice + alias-aware lookup │ ├── status.rs # /status — opens StatusModal -│ └── status_modal.rs # StatusModal — read-only kv-overview of the live session (Esc / Enter close) +│ ├── status_modal.rs # StatusModal — read-only kv-overview of the live session (Esc / Enter close) +│ └── theme.rs # /theme — bare opens the picker (live preview); `/theme ` validates against the curated roster and swaps ├── tool.rs # Tool trait, registry, definitions ├── tool/ │ ├── bash.rs # Shell command execution with timeout diff --git a/crates/oxide-code/src/agent.rs b/crates/oxide-code/src/agent.rs index 262ceda0..9401861c 100644 --- a/crates/oxide-code/src/agent.rs +++ b/crates/oxide-code/src/agent.rs @@ -264,7 +264,9 @@ where Some( action @ (UserAction::ConfirmExit | UserAction::Clear - | UserAction::SwapConfig { .. }), + | UserAction::SwapConfig { .. } + | UserAction::PreviewTheme { .. } + | UserAction::SwapTheme { .. }), ) => warn!("dropped mid-turn action: {action:?}"), }, output = &mut fut => return Ok(output), diff --git a/crates/oxide-code/src/agent/event.rs b/crates/oxide-code/src/agent/event.rs index b320d547..cbdb566f 100644 --- a/crates/oxide-code/src/agent/event.rs +++ b/crates/oxide-code/src/agent/event.rs @@ -77,6 +77,15 @@ pub(crate) enum UserAction { model: Option, effort: Option, }, + /// TUI-only: live-preview the picker's highlighted theme without committing it. Cursor moves + /// in the `/theme` modal emit this; cancelling the modal restores the snapshot taken on open. + PreviewTheme { + name: String, + }, + /// TUI-only: commit a theme swap for the rest of the session. Restart returns to user config. + SwapTheme { + name: String, + }, Cancel, /// TUI-only; agent loop ignores this. ConfirmExit, diff --git a/crates/oxide-code/src/client/anthropic/testing.rs b/crates/oxide-code/src/client/anthropic/testing.rs index 3f8ed692..1d7c085d 100644 --- a/crates/oxide-code/src/client/anthropic/testing.rs +++ b/crates/oxide-code/src/client/anthropic/testing.rs @@ -18,6 +18,7 @@ pub(crate) fn test_config(base_url: impl Into, auth: Auth, model: &str) thinking: None, show_thinking: false, theme: Theme::default(), + theme_name: "mocha".to_owned(), } } diff --git a/crates/oxide-code/src/config.rs b/crates/oxide-code/src/config.rs index 21a18c28..abd3c1fb 100644 --- a/crates/oxide-code/src/config.rs +++ b/crates/oxide-code/src/config.rs @@ -17,6 +17,8 @@ use crate::util::env; const DEFAULT_MODEL: &str = "claude-opus-4-7[1m]"; const DEFAULT_BASE_URL: &str = "https://api.anthropic.com"; +/// Mirrors the fallback `loader::resolve_theme` applies when no `[tui.theme] base` is set. +pub(crate) const DEFAULT_THEME: &str = "mocha"; // ── Auth ── @@ -49,6 +51,9 @@ pub(crate) struct ConfigSnapshot { pub(crate) max_tokens: u32, pub(crate) prompt_cache_ttl: PromptCacheTtl, pub(crate) show_thinking: bool, + /// Resolved theme base name — built-in catalogue key or filesystem path. `/theme` reads this + /// to mark the active row in the picker. + pub(crate) theme_name: String, } // ── ThinkingConfig ── @@ -183,6 +188,9 @@ pub(crate) struct Config { pub(crate) thinking: Option, pub(crate) show_thinking: bool, pub(crate) theme: Theme, + /// Built-in catalogue key (e.g. `"mocha"`) or filesystem path; mirrors `[tui.theme] base`, + /// falling back to [`DEFAULT_THEME`] when unset. + pub(crate) theme_name: String, } impl Config { @@ -245,6 +253,10 @@ impl Config { None => client.prompt_cache_ttl.unwrap_or(PromptCacheTtl::OneHour), }; + let theme_name = theme_config + .base + .clone() + .unwrap_or_else(|| DEFAULT_THEME.to_owned()); let theme = theme::resolve_theme( theme_config.base.as_deref(), &theme_config.overrides.unwrap_or_default(), @@ -260,6 +272,7 @@ impl Config { thinking, show_thinking, theme, + theme_name, }) } @@ -273,6 +286,7 @@ impl Config { max_tokens: self.max_tokens, prompt_cache_ttl: self.prompt_cache_ttl, show_thinking: self.show_thinking, + theme_name: self.theme_name.clone(), } } } @@ -461,6 +475,23 @@ mod tests { assert_eq!(config.prompt_cache_ttl, PromptCacheTtl::OneHour); assert!(!config.show_thinking); assert!(matches!(config.auth, Auth::ApiKey(k) if k == "sk-default")); + assert_eq!(config.theme_name, DEFAULT_THEME); + } + + #[tokio::test] + async fn load_theme_name_reflects_user_picked_base() { + let dir = tempfile::tempdir().unwrap(); + write_user_config( + dir.path(), + indoc::indoc! {r#" + [tui.theme] + base = "latte" + "#}, + ); + let config = temp_env::async_with_vars(env_vars(vec![xdg(&dir)]), Config::load()) + .await + .unwrap(); + assert_eq!(config.theme_name, "latte"); } #[tokio::test] @@ -825,6 +856,7 @@ mod tests { thinking: None, show_thinking: true, theme: Theme::default(), + theme_name: "macchiato".to_owned(), }; let snap = cfg.snapshot(); assert_eq!(snap.auth_label, "OAuth"); @@ -834,6 +866,7 @@ mod tests { assert_eq!(snap.max_tokens, 64_000); assert_eq!(snap.prompt_cache_ttl, PromptCacheTtl::FiveMin); assert!(snap.show_thinking); + assert_eq!(snap.theme_name, "macchiato"); } // ── display_effort ── diff --git a/crates/oxide-code/src/main.rs b/crates/oxide-code/src/main.rs index 874cb2a4..e631f0a5 100644 --- a/crates/oxide-code/src/main.rs +++ b/crates/oxide-code/src/main.rs @@ -373,7 +373,12 @@ async fn agent_loop_task( } } } - UserAction::Cancel | UserAction::ConfirmExit => {} + // Cancel / ConfirmExit are no-ops here; PreviewTheme / SwapTheme are TUI-only and + // applied client-side in `App::apply_action_locally`. + UserAction::Cancel + | UserAction::ConfirmExit + | UserAction::PreviewTheme { .. } + | UserAction::SwapTheme { .. } => {} UserAction::Clear => { let outcome = roll_session(&mut session, &store, &file_tracker, client.model()).await; diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 5e89a87a..055bef67 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -23,6 +23,7 @@ mod picker; mod registry; mod status; mod status_modal; +mod theme; pub(crate) use context::{LiveSessionInfo, SlashContext}; pub(crate) use matcher::MatchedCommand; @@ -111,6 +112,7 @@ pub(crate) fn test_session_info() -> LiveSessionInfo { max_tokens: 32_000, prompt_cache_ttl: PromptCacheTtl::OneHour, show_thinking: false, + theme_name: "mocha".to_owned(), }, } } diff --git a/crates/oxide-code/src/slash/registry.rs b/crates/oxide-code/src/slash/registry.rs index 3532ffa3..600c1af4 100644 --- a/crates/oxide-code/src/slash/registry.rs +++ b/crates/oxide-code/src/slash/registry.rs @@ -9,6 +9,7 @@ use super::help::HelpCmd; use super::init::InitCmd; use super::model::ModelCmd; use super::status::StatusCmd; +use super::theme::ThemeCmd; use crate::agent::event::UserAction; /// `Done` for client-side work via `ctx`; `Forward` for state-mutating commands. @@ -58,6 +59,7 @@ pub(crate) trait SlashCommand: Sync { /// Alphabetical for stable `/help` and empty-query popup ordering. pub(super) const BUILT_INS: &[&dyn SlashCommand] = &[ &ClearCmd, &ConfigCmd, &DiffCmd, &EffortCmd, &HelpCmd, &InitCmd, &ModelCmd, &StatusCmd, + &ThemeCmd, ]; /// Resolves `name` against canonical names first, then aliases. diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs new file mode 100644 index 00000000..67806440 --- /dev/null +++ b/crates/oxide-code/src/slash/theme.rs @@ -0,0 +1,445 @@ +//! `/theme` — open the picker, or swap with `/theme `. Bare form opens a list picker +//! whose cursor moves live-preview the highlighted theme by repainting the full TUI; Esc snaps +//! back to the original, Enter commits for the rest of the session. + +use crossterm::event::{KeyCode, KeyEvent}; +use ratatui::Frame; +use ratatui::layout::Rect; +use ratatui::text::{Line, Span}; +use ratatui::widgets::Paragraph; + +use super::context::SlashContext; +use super::registry::{SlashCommand, SlashKind, SlashOutcome}; +use crate::agent::event::UserAction; +use crate::tui::modal::list_picker::{ListPicker, PickerItem}; +use crate::tui::modal::{Modal, ModalAction, ModalKey}; +use crate::tui::theme::Theme; + +// ── Constants ── + +/// Curated roster shown in the picker. Order matches `tui/theme/builtin.rs` — Catppuccin variants +/// dark→light, then non-Catppuccin palettes. Static descriptions live alongside so the picker +/// can render a one-glance summary without re-resolving each TOML. +const LISTED_THEMES: &[(&str, &str)] = &[ + ("mocha", "Catppuccin dark (default)"), + ("macchiato", "Catppuccin dark (medium)"), + ("frappe", "Catppuccin dark (soft)"), + ("latte", "Catppuccin light"), + ("material", "Material dark"), +]; + +// ── PickerItem ── + +struct ThemeRow { + name: &'static str, + description: &'static str, + is_active: bool, + hint: Option, +} + +impl ThemeRow { + fn build(active_name: &str) -> Vec { + LISTED_THEMES + .iter() + .enumerate() + .map(|(idx, (name, description))| Self { + name, + description, + is_active: *name == active_name, + hint: numeric_hint(idx), + }) + .collect() + } +} + +/// `'1'`–`'9'` for the first nine rows; `None` after that. +fn numeric_hint(idx: usize) -> Option { + let digit = u32::try_from(idx).ok()?.checked_add(1)?; + if (1..=9).contains(&digit) { + char::from_digit(digit, 10) + } else { + None + } +} + +impl PickerItem for ThemeRow { + fn label(&self) -> &str { + self.name + } + fn description(&self) -> Option<&str> { + Some(self.description) + } + fn is_active(&self) -> bool { + self.is_active + } + fn key_hint(&self) -> Option { + self.hint + } +} + +// ── ThemePicker ── + +pub(super) struct ThemePicker { + list: ListPicker, + /// Active theme at open — Enter on the same row cancels rather than firing a no-op swap. + original_name: String, +} + +impl ThemePicker { + pub(super) fn new(active_name: &str) -> Self { + let rows = ThemeRow::build(active_name); + let mut list = ListPicker::new("Select theme", rows).with_description( + "Switch the active theme. Applies to this session only — restart returns to your config.", + ); + list.select_initial(|row| row.is_active); + Self { + list, + original_name: active_name.to_owned(), + } + } + + /// `Preview` for cursor-driven moves on the picker; `Cancelled` when there's no current row. + fn preview_current(&self) -> ModalKey { + match self.list.selected() { + Some(row) => ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { + name: row.name.to_owned(), + })), + None => ModalKey::Consumed, + } + } + + /// Enter: commit if the cursor moved off the original theme; otherwise cancel. + fn submit(&self) -> ModalKey { + let Some(row) = self.list.selected() else { + return ModalKey::Cancelled; + }; + if row.name == self.original_name { + return ModalKey::Cancelled; + } + ModalKey::Submitted(ModalAction::User(UserAction::SwapTheme { + name: row.name.to_owned(), + })) + } +} + +impl Modal for ThemePicker { + fn height(&self, width: u16) -> u16 { + // List body + spacer + footer. + self.list.height(width).saturating_add(2) + } + + fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme) { + let list_h = self.list.height(area.width); + let list_area = Rect { + height: list_h.min(area.height), + ..area + }; + self.list.render(frame, list_area, theme); + + let remaining = area.height.saturating_sub(list_h); + if remaining >= 2 { + let footer_area = Rect { + x: area.x, + y: area.y.saturating_add(list_h).saturating_add(1), + width: area.width, + height: 1, + }; + let footer = Line::from(Span::styled( + "Enter to confirm · Esc to cancel", + theme.dim(), + )); + frame.render_widget(Paragraph::new(footer).style(theme.surface()), footer_area); + } + } + + fn handle_key(&mut self, event: &KeyEvent) -> ModalKey { + match event.code { + KeyCode::Enter => self.submit(), + KeyCode::Up | KeyCode::Char('k') => { + self.list.select_prev(); + self.preview_current() + } + KeyCode::Down | KeyCode::Char('j') => { + self.list.select_next(); + self.preview_current() + } + KeyCode::Char(c @ '1'..='9') => { + if self.list.select_by_hint(c) { + self.preview_current() + } else { + ModalKey::Consumed + } + } + _ => ModalKey::Consumed, + } + } +} + +// ── ThemeCmd ── + +pub(super) struct ThemeCmd; + +impl SlashCommand for ThemeCmd { + fn name(&self) -> &'static str { + "theme" + } + + fn description(&self) -> &'static str { + "Open the theme picker or switch directly with `/theme `" + } + + fn classify(&self, args: &str) -> SlashKind { + if args.trim().is_empty() { + SlashKind::ReadOnly + } else { + SlashKind::Mutating + } + } + + fn usage(&self) -> Option<&'static str> { + Some("[]") + } + + fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { + let arg = args.trim(); + if arg.is_empty() { + ctx.open_modal(Box::new(ThemePicker::new(&ctx.info.config.theme_name))); + return Ok(SlashOutcome::Done); + } + let name = resolve_theme_arg(arg)?; + Ok(SlashOutcome::Forward(UserAction::SwapTheme { + name: name.to_owned(), + })) + } +} + +/// Validates `arg` against the curated picker roster. Custom file paths aren't accepted via +/// `/theme ` — users who maintain a custom TOML edit `~/.config/ox/config.toml` directly. +fn resolve_theme_arg(arg: &str) -> Result<&'static str, String> { + let lower = arg.to_ascii_lowercase(); + LISTED_THEMES + .iter() + .find(|(name, _)| *name == lower) + .map(|(name, _)| *name) + .ok_or_else(|| format!("Unknown theme: `{arg}`. Valid: {}.", valid_names())) +} + +fn valid_names() -> String { + LISTED_THEMES + .iter() + .map(|(name, _)| *name) + .collect::>() + .join(", ") +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::slash::test_session_info; + use crate::tui::components::chat::ChatView; + + fn key(code: KeyCode) -> KeyEvent { + KeyEvent::from(code) + } + + fn run_execute( + args: &str, + ) -> ( + ChatView, + Option>, + Result, + ) { + let mut chat = ChatView::new(&Theme::default(), false); + let info = test_session_info(); + let (outcome, modal) = { + let mut ctx = SlashContext::new(&mut chat, &info); + let outcome = ThemeCmd.execute(args, &mut ctx); + (outcome, ctx.take_modal()) + }; + (chat, modal, outcome) + } + + fn picker_with_active(name: &str) -> ThemePicker { + ThemePicker::new(name) + } + + // ── ThemeCmd metadata ── + + #[test] + fn metadata_matches_built_ins_contract() { + assert_eq!(ThemeCmd.name(), "theme"); + assert!(ThemeCmd.aliases().is_empty()); + assert!(!ThemeCmd.description().is_empty()); + assert_eq!(ThemeCmd.usage(), Some("[]")); + } + + #[test] + fn classify_splits_on_args() { + // Bare form opens the picker (read-only); typed arg races the client (mutating). + assert_eq!(ThemeCmd.classify(""), SlashKind::ReadOnly); + assert_eq!(ThemeCmd.classify(" "), SlashKind::ReadOnly); + assert_eq!(ThemeCmd.classify("latte"), SlashKind::Mutating); + } + + // ── ThemeCmd::execute ── + + #[test] + fn execute_no_args_opens_picker_and_pushes_no_chat_block() { + let (chat, modal, outcome) = run_execute(""); + assert_eq!(outcome, Ok(SlashOutcome::Done)); + assert!(modal.is_some(), "bare /theme must populate the modal slot"); + assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); + } + + #[test] + fn execute_with_known_name_forwards_swap_theme() { + for (name, _) in LISTED_THEMES { + let (_, _, outcome) = run_execute(name); + assert_eq!( + outcome, + Ok(SlashOutcome::Forward(UserAction::SwapTheme { + name: (*name).to_owned(), + })), + "`{name}` must forward SwapTheme", + ); + } + } + + #[test] + fn execute_is_case_insensitive() { + let (_, _, outcome) = run_execute("LATTE"); + assert_eq!( + outcome, + Ok(SlashOutcome::Forward(UserAction::SwapTheme { + name: "latte".to_owned(), + })), + ); + } + + #[test] + fn execute_unknown_name_errors_listing_valid_options() { + let (chat, _, outcome) = run_execute("solarized"); + let msg = outcome.expect_err("unknown name must error"); + assert!(msg.starts_with("Unknown theme: `solarized`."), "{msg}"); + for (name, _) in LISTED_THEMES { + assert!(msg.contains(name), "lists `{name}`: {msg}"); + } + assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); + } + + // ── ThemePicker::new ── + + #[test] + fn new_positions_cursor_on_active_theme() { + let p = picker_with_active("latte"); + let row = p.list.selected().expect("active row"); + assert_eq!(row.name, "latte"); + assert!(row.is_active); + } + + #[test] + fn new_with_unknown_active_keeps_cursor_on_first_row() { + // User-provided custom file path won't match any built-in row; cursor falls back to the + // first listed theme rather than panicking or silently picking nothing. + let p = picker_with_active("~/themes/custom.toml"); + let row = p.list.selected().expect("first row"); + assert_eq!(row.name, "mocha"); + } + + // ── ThemePicker::handle_key ── + + #[test] + fn down_arrow_emits_preview_for_next_row_without_popping() { + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(KeyCode::Down)); + match outcome { + ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { + assert_eq!(name, "macchiato", "Down from mocha lands on macchiato"); + } + other => panic!("expected Preview(PreviewTheme), got {other:?}"), + } + } + + #[test] + fn numeric_jump_emits_preview_for_target_row() { + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(KeyCode::Char('4'))); + match outcome { + ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { + assert_eq!(name, "latte", "row 4 is latte"); + } + other => panic!("expected Preview, got {other:?}"), + } + } + + #[test] + fn numeric_jump_unknown_digit_is_a_consumed_noop() { + // Only digits 1..=N (curated count) jump; out-of-range digits leave cursor and emit no + // preview, so the App doesn't repaint for nothing. + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(KeyCode::Char('9'))); + assert!(matches!(outcome, ModalKey::Consumed)); + } + + #[test] + fn enter_on_unchanged_cursor_returns_cancelled() { + // No-touch Enter mirrors Esc — nothing to commit. + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(KeyCode::Enter)); + assert!(matches!(outcome, ModalKey::Cancelled)); + } + + #[test] + fn enter_after_cursor_move_emits_swap_for_new_theme() { + let mut p = picker_with_active("mocha"); + p.handle_key(&key(KeyCode::Down)); // → macchiato + let outcome = p.handle_key(&key(KeyCode::Enter)); + match outcome { + ModalKey::Submitted(ModalAction::User(UserAction::SwapTheme { name })) => { + assert_eq!(name, "macchiato"); + } + other => panic!("expected Submitted(SwapTheme), got {other:?}"), + } + } + + #[test] + fn other_keys_are_consumed_and_modal_stays_open() { + let mut p = picker_with_active("mocha"); + for code in [KeyCode::Tab, KeyCode::Char('x'), KeyCode::Char('0')] { + let outcome = p.handle_key(&key(code)); + assert!( + matches!(outcome, ModalKey::Consumed), + "{code:?} must be consumed", + ); + } + } + + // ── ThemePicker::render ── + + #[test] + fn render_runs_at_typical_widths_without_panicking() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let p = picker_with_active("mocha"); + let theme = Theme::default(); + for width in [40_u16, 80, 120] { + let h = p.height(width).min(20); + let mut terminal = Terminal::new(TestBackend::new(width, h)).unwrap(); + terminal + .draw(|frame| p.render(frame, Rect::new(0, 0, width, h), &theme)) + .expect("render must not panic"); + } + } + + // ── numeric_hint ── + + #[test] + fn numeric_hint_covers_first_nine_rows_then_returns_none() { + for idx in 0..9 { + let expected = char::from_digit(u32::try_from(idx + 1).unwrap(), 10); + assert_eq!(numeric_hint(idx), expected, "idx={idx}"); + } + assert_eq!(numeric_hint(9), None, "10th row has no hint"); + } +} diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 5cf86e4f..5ecf816e 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -52,6 +52,9 @@ pub(crate) struct App { /// FIFO of prompts submitted mid-turn; drained at turn boundaries. pending_prompts: VecDeque, modals: ModalStack, + /// Saved theme captured when a `/theme` picker opens; restored if the modal cancels so + /// live-preview moves don't leak into the rest of the session. + preview_theme_snapshot: Option, should_quit: bool, dirty: bool, } @@ -92,6 +95,7 @@ impl App { pending_calls: PendingCalls::new(), pending_prompts: VecDeque::new(), modals: ModalStack::new(), + preview_theme_snapshot: None, should_quit: false, dirty: true, } @@ -202,11 +206,28 @@ impl App { fn apply_modal_action(&mut self, action: ModalAction) { match action { - ModalAction::None => {} + // Modal cancelled or silently closed: undo any in-flight theme preview so the user + // returns to exactly the state before opening the picker. + ModalAction::None => { + if let Some(theme) = self.preview_theme_snapshot.take() { + self.apply_theme(&theme); + } + } ModalAction::User(user_action) => self.dispatch_user_action(user_action), } } + /// Repaint every component with `theme`. Sole entry point for mid-session theme swaps so the + /// per-component `set_theme` calls stay grouped — adding a new theme-styled component should + /// only need a single edit here. + fn apply_theme(&mut self, theme: &Theme) { + self.theme = theme.clone(); + self.chat.set_theme(theme); + self.status_bar.set_theme(theme); + self.input.set_theme(theme); + self.dirty = true; + } + #[cfg(test)] pub(crate) fn push_modal(&mut self, modal: Box) { self.modals.push(modal); @@ -335,6 +356,28 @@ impl App { true } UserAction::Clear | UserAction::SwapConfig { .. } => true, + UserAction::PreviewTheme { name } => { + if let Some(preview) = super::theme::load_builtin(name) { + if self.preview_theme_snapshot.is_none() { + self.preview_theme_snapshot = Some(self.theme.clone()); + } + self.apply_theme(&preview); + } + false + } + UserAction::SwapTheme { name } => { + if let Some(theme) = super::theme::load_builtin(name) { + self.preview_theme_snapshot = None; + self.session_info.config.theme_name.clone_from(name); + self.apply_theme(&theme); + self.chat + .push_system_message(format!("Theme set to {name}.")); + } else { + self.chat + .push_error(&format!("unknown theme: `{name}` (this is a bug)")); + } + false + } } } @@ -683,6 +726,7 @@ mod tests { max_tokens: 32_000, prompt_cache_ttl: PromptCacheTtl::OneHour, show_thinking: false, + theme_name: "mocha".to_owned(), }, } } @@ -2274,6 +2318,81 @@ mod tests { ); } + // ── apply_action_locally / theme ── + + #[test] + fn preview_theme_repaints_components_and_caches_original() { + // First PreviewTheme captures the snapshot; cursor moves on the picker repaint without + // committing. Apply a known mocha → latte swap and confirm both axes mutate. + let (mut app, _rx, _agent_tx) = test_app(None); + let original = app.theme.clone(); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + assert!( + app.preview_theme_snapshot.is_some(), + "first PreviewTheme must cache the original", + ); + assert_ne!(app.theme.text, original.text, "live theme must change"); + // session_info.config.theme_name unchanged — preview is not a commit. + assert_eq!(app.session_info.config.theme_name, "mocha"); + } + + #[test] + fn preview_theme_then_modal_cancel_restores_original() { + // Esc / Ctrl+C surface as ModalAction::None; the snapshot must roll back. + let (mut app, _rx, _agent_tx) = test_app(None); + let original = app.theme.clone(); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + app.apply_modal_action(ModalAction::None); + assert!( + app.preview_theme_snapshot.is_none(), + "snapshot consumed on cancel", + ); + assert_eq!(app.theme.text, original.text, "theme rolled back to mocha"); + } + + #[test] + fn swap_theme_commits_and_clears_snapshot() { + // Enter on a moved cursor: snapshot drops, session theme name updates, chat shows the + // confirmation. A subsequent ModalAction::None then must NOT roll anything back. + let (mut app, _rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + let previewed = app.theme.clone(); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + assert!(app.preview_theme_snapshot.is_none()); + assert_eq!(app.session_info.config.theme_name, "latte"); + assert_eq!(app.theme.text, previewed.text); + + app.apply_modal_action(ModalAction::None); + assert_eq!( + app.session_info.config.theme_name, "latte", + "post-commit cancel must not restore the pre-preview theme", + ); + } + + #[tokio::test] + async fn theme_actions_are_not_forwarded_to_agent_loop() { + // PreviewTheme / SwapTheme are TUI-only; reaching `user_tx` would race the client. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "theme actions must stay client-side", + ); + } + // ── render_preview ── #[test] diff --git a/crates/oxide-code/src/tui/components/chat.rs b/crates/oxide-code/src/tui/components/chat.rs index c7f39e49..f3474fe1 100644 --- a/crates/oxide-code/src/tui/components/chat.rs +++ b/crates/oxide-code/src/tui/components/chat.rs @@ -56,6 +56,12 @@ impl ChatView { } } + /// Re-skin every subsequent render. Mid-session theme swap (`/theme`) calls this to repaint + /// the existing block stack — block content stays cached, only the styles change next frame. + pub(crate) fn set_theme(&mut self, theme: &Theme) { + self.theme = theme.clone(); + } + /// Populates from resumed transcript, projecting the same block shapes as live rendering. pub(crate) fn load_history( &mut self, diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index b8cf4fe2..b38ca567 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -72,6 +72,15 @@ impl InputArea { input } + /// Re-skin subsequent renders. The textarea keeps a cached `Style`, so the previous theme's + /// text / placeholder colors would persist if not reapplied here. + pub(crate) fn set_theme(&mut self, theme: &Theme) { + self.theme = theme.clone(); + self.textarea.set_style(theme.text()); + self.textarea.set_placeholder_style(theme.dim()); + self.popup.set_theme(theme); + } + pub(crate) fn set_has_queued(&mut self, has_queued: bool) { if self.has_queued == has_queued { return; diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 175d8d09..6d71daaf 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -33,6 +33,10 @@ impl SlashPopup { } } + pub(crate) fn set_theme(&mut self, theme: &Theme) { + self.theme = theme.clone(); + } + pub(crate) fn set_query(&mut self, query: Option<&str>) { let Some(q) = query else { self.matches.clear(); diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap index 6586d9ee..3c64b561 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap @@ -11,3 +11,4 @@ expression: "render_to_backend(&popup, 60)" "/init Generate or update the project's `AGENTS.md` / `..." "/model Open the model picker or switch directly with `/..." "/status Show session info: model, effort, version, worki..." +" ... (1 more) " diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap index ec19c644..5a74b8a1 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap @@ -11,3 +11,4 @@ expression: "render_to_backend(&popup, 30)" "/init Generate or update..." "/model Open the model pic..." "/status Show session info:..." +" ... (1 more) " diff --git a/crates/oxide-code/src/tui/components/status.rs b/crates/oxide-code/src/tui/components/status.rs index f6b15634..af98c725 100644 --- a/crates/oxide-code/src/tui/components/status.rs +++ b/crates/oxide-code/src/tui/components/status.rs @@ -60,6 +60,11 @@ impl StatusBar { self.model = model; } + /// Re-skin subsequent renders; the spinner / status state is unaffected. + pub(crate) fn set_theme(&mut self, theme: &Theme) { + self.theme = theme.clone(); + } + pub(crate) fn set_status(&mut self, status: Status) { if status != self.status { self.spinner_frame = 0; diff --git a/crates/oxide-code/src/tui/modal.rs b/crates/oxide-code/src/tui/modal.rs index 1ce4774a..fd88dcc0 100644 --- a/crates/oxide-code/src/tui/modal.rs +++ b/crates/oxide-code/src/tui/modal.rs @@ -41,6 +41,9 @@ pub(crate) enum ModalKey { Consumed, Cancelled, Submitted(ModalAction), + /// Emit `action` without popping — for live-preview modals where cursor moves should mutate + /// app state without committing. + Preview(ModalAction), } #[derive(Debug)] @@ -138,6 +141,7 @@ impl ModalStack { self.stack.pop(); Some(action) } + ModalKey::Preview(action) => Some(action), } } } @@ -160,7 +164,9 @@ pub(crate) mod testing { pub(crate) struct ScriptedModal { pub(crate) on_submit_key: char, pub(crate) on_cancel_key: char, + pub(crate) on_preview_key: char, pub(crate) submit_action: ModalAction, + pub(crate) preview_action: ModalAction, pub(crate) declared_height: u16, } @@ -169,7 +175,9 @@ pub(crate) mod testing { Self { on_submit_key: 's', on_cancel_key: 'c', + on_preview_key: 'p', submit_action, + preview_action: ModalAction::None, declared_height: 3, } } @@ -191,6 +199,11 @@ pub(crate) mod testing { ModalKey::Submitted(taken) } KeyCode::Char(c) if c == self.on_cancel_key => ModalKey::Cancelled, + KeyCode::Char(c) if c == self.on_preview_key => { + let mut taken = ModalAction::None; + std::mem::swap(&mut self.preview_action, &mut taken); + ModalKey::Preview(taken) + } _ => ModalKey::Consumed, } } @@ -334,6 +347,23 @@ mod tests { assert!(!stack.is_active()); } + #[test] + fn handle_key_preview_yields_action_without_popping_stack() { + // Live-preview modals (theme picker) emit a `User` action on cursor moves so the App can + // repaint, but the modal must stay on screen until Enter or Esc. + let mut stack = ModalStack::new(); + let mut modal = ScriptedModal::new(ModalAction::None); + modal.preview_action = ModalAction::User(UserAction::Cancel); + stack.push(Box::new(modal)); + + let outcome = stack.handle_key(&key('p')); + assert!( + matches!(outcome, Some(ModalAction::User(UserAction::Cancel))), + "preview must surface the modal's UserAction unchanged", + ); + assert!(stack.is_active(), "preview must NOT pop the stack"); + } + #[test] fn handle_key_on_empty_stack_returns_none_without_panicking() { // No active modal → no key delivery, no stack mutation. diff --git a/crates/oxide-code/src/tui/theme.rs b/crates/oxide-code/src/tui/theme.rs index 3c1175e3..0997105c 100644 --- a/crates/oxide-code/src/tui/theme.rs +++ b/crates/oxide-code/src/tui/theme.rs @@ -11,6 +11,14 @@ mod loader; pub(crate) use loader::{SlotPatch, resolve_theme}; +/// Resolve a built-in theme name (mocha / latte / ...) to a parsed [`Theme`]. `None` for unknown +/// names; never reads the filesystem. Used by `/theme` for live preview where each cursor move +/// must be cheap. +pub(crate) fn load_builtin(name: &str) -> Option { + let body = builtin::lookup(name)?; + loader::parse_theme(body).ok() +} + /// One theme slot — optional fg, bg, and modifiers composed into a [`Style`]. /// /// Each component is independently optional so an override can patch one axis (e.g., bg only) diff --git a/docs/guide/slash-commands.md b/docs/guide/slash-commands.md index a0352175..43a9fdd6 100644 --- a/docs/guide/slash-commands.md +++ b/docs/guide/slash-commands.md @@ -14,6 +14,7 @@ Slash commands are built-in shortcuts that run client-side, without involving th | `/init` | Generate or update the project's `AGENTS.md` / `CLAUDE.md`. | | `/model []` | Open the model + effort picker, or swap directly (alias / substring / exact id). | | `/status` | Show model, effort, working directory, version, auth, session id. | +| `/theme []` | Open the theme picker (live preview), or swap directly to a built-in theme. | ## Autocomplete Popup @@ -32,7 +33,7 @@ To send a message that _starts_ with a slash without invoking a command, double ## Mid-Turn Behavior -Read-only commands (`/config`, `/diff`, `/help`, `/status`, bare `/model` and bare `/effort` which open modals) are safe to run while the agent is streaming. State-mutating commands (`/clear`, `/init`, `/model `, `/effort `) refuse mid-turn — wait for the current response to complete, then retry. +Read-only commands (`/config`, `/diff`, `/help`, `/status`, and bare `/model` / `/effort` / `/theme` which open modals) are safe to run while the agent is streaming. State-mutating commands (`/clear`, `/init`, `/model `, `/effort `, `/theme `) refuse mid-turn — wait for the current response to complete, then retry. ## Model and Effort @@ -40,6 +41,12 @@ Bare `/model` opens the combined model + effort picker; bare `/effort` opens a S `/model ` resolves in four tiers: alias (`opus`, `sonnet`, `haiku`, with optional `[1m]` for the 1M-context variants) → exact / dated id → unique suffix → unique substring. Swapping clamps the current effort to the new model's caps; `/effort` on a model without effort (Haiku 4.5) errors with a recovery hint. See [Configuration](configuration.md) for the full tier reference and per-model defaults. +## Switching the Theme + +`/theme` (no argument) opens a list picker for the built-in themes — Up / Down repaints the full TUI in the candidate theme so you can compare without committing, number keys (`1`–`9`) jump to a row, Enter applies for the rest of the session, Esc snaps back to the original. Restart returns to the theme set in your `ox.toml`. + +`/theme ` swaps directly to a built-in (`mocha`, `macchiato`, `frappe`, `latte`, `material`). Custom file-path themes aren't accepted via the slash form — edit `~/.config/ox/config.toml` to point `[tui.theme] base` at a custom TOML. + ## Stance: No Silent Config Writes -Slash commands never write user config files. Runtime mutations (`/model`, `/effort` today, `/theme` later) stay session-local; restart returns to the user-declared configuration. Persisting a slash-command choice across restarts will require an explicit subcommand writing to an explicit user-opted-in path — never a silent merge. +Slash commands never write user config files. Runtime mutations (`/model`, `/effort`, `/theme`) stay session-local; restart returns to the user-declared configuration. Persisting a slash-command choice across restarts will require an explicit subcommand writing to an explicit user-opted-in path — never a silent merge. diff --git a/docs/roadmap.md b/docs/roadmap.md index 321ce3b9..3ba2a45b 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -61,10 +61,10 @@ The direction is simple: ### Slash Commands -- Built-in: `/clear` (aliases `/new`, `/reset`), `/config`, `/diff`, `/effort`, `/help`, `/init`, `/model`, `/status`. See the [user guide](guide/slash-commands.md). +- Built-in: `/clear` (aliases `/new`, `/reset`), `/config`, `/diff`, `/effort`, `/help`, `/init`, `/model`, `/status`, `/theme`. See the [user guide](guide/slash-commands.md). - Autocomplete popup on typing `/`, with ranked filter and Tab completion. -- Mid-session swap: `/model` and `/effort`. Session-only — no slash command writes user config files. -- Modal UI primitive (focus-grabbing overlays above the input). Bare `/model`, `/effort`, `/status` each open one. +- Mid-session swap: `/model`, `/effort`, and `/theme`. Session-only — no slash command writes user config files. +- Modal UI primitive (focus-grabbing overlays above the input). Bare `/model` opens a combined model + effort picker; `/effort` opens a Speed ↔ Intelligence slider; `/theme` opens a list picker with live preview (Up / Down repaints the TUI in the candidate theme; Esc snaps back, Enter commits); `/status` opens a read-only kv overview. Esc / Ctrl+C cancel any modal universally. ### Authentication & Configuration @@ -89,7 +89,6 @@ The direction is simple: Remaining surface beyond Working Today: - Session: `/resume`. -- Mid-session swap: `/theme`. - Inline argument placeholder — dim ghost-text hint (e.g. `[id]`) after a slash command's trailing space. - Deferred: `/compact`, `/cost`, `/login` / `/logout`, custom user commands, `/init` multi-phase flow, argument-aware popup completion. From f3e81a24d3505cc9ece598c7c4cf1c9b85d72cf9 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 18:29:27 +0800 Subject: [PATCH 02/15] fix(slash, tui): typed `/theme `, scrollable popup, surface-bg fill - Slash-synthesized non-`SubmitPrompt` actions now route through `dispatch_user_action` instead of forwarding direct to the agent. The agent silently dropped TUI-only actions like `SwapTheme`, so `/theme latte` did nothing. Synthesized prompts (e.g. `/init`) preserve the existing direct-forward path to avoid re-parsing the leading `/`. - Popup now scrolls with a centered cursor (Claude Code convention) instead of trimming with `... (N more)`. Once selection moves past the top half of the window the cursor anchors at the visual middle; near the end of the list it anchors at the bottom. Footer dropped. - `App::draw_frame` paints the full frame area with `theme.surface()` before sub-widgets render so light themes (e.g. `latte`) cover gaps between blocks, modal bodies, and the popup band. Belt-and-braces: the popup also paints surface bg explicitly. - New tests: `swap_theme_with_unknown_name_pushes_error_and_keeps_active_theme` and `slash_typed_swap_theme_routes_through_local_handler` pin the fixed dispatch path; `theme_row_picker_item_methods_return_curated_values` closes the codecov gap on `ThemeRow`'s trait impl; `scroll_offset_*` quartet pins the centered-scroll formula. --- crates/oxide-code/src/slash/theme.rs | 24 +++ crates/oxide-code/src/tui/app.rs | 54 +++++- .../src/tui/components/input/popup.rs | 157 ++++++++++++------ ...r_empty_query_shows_each_command_once.snap | 1 - ...narrow_terminal_truncates_description.snap | 1 - 5 files changed, 182 insertions(+), 55 deletions(-) diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index 67806440..b93f91be 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -432,6 +432,30 @@ mod tests { } } + // ── ThemeRow ── + + #[test] + fn theme_row_picker_item_methods_return_curated_values() { + // Pins the trait-impl surface for ThemeRow — label / description / is_active / key_hint + // each have a path the picker depends on and only the render smoke test exercises + // indirectly. + let rows = ThemeRow::build("latte"); + let names: Vec<&str> = rows.iter().map(PickerItem::label).collect(); + assert_eq!( + names, + LISTED_THEMES.iter().map(|(n, _)| *n).collect::>(), + ); + + let active_count = rows.iter().filter(|r| r.is_active()).count(); + assert_eq!(active_count, 1, "exactly one row marks the active theme"); + assert!(rows.iter().find(|r| r.is_active()).unwrap().label() == "latte"); + + for (idx, row) in rows.iter().enumerate() { + assert_eq!(row.key_hint(), numeric_hint(idx), "idx={idx}"); + assert_eq!(row.description(), Some(LISTED_THEMES[idx].1)); + } + } + // ── numeric_hint ── #[test] diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 5ecf816e..ebccfff5 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -293,10 +293,14 @@ impl App { } if let Some(action) = synthesized { if matches!(action, UserAction::SubmitPrompt(_)) { + // Synthesized prompts skip apply_action_locally — re-entry would + // re-parse the leading `/` and recurse. self.input.set_enabled(false); self.status_bar.set_status(Status::Streaming); + self.forward_to_agent(action); + } else { + self.dispatch_user_action(action); } - self.forward_to_agent(action); } return false; } @@ -544,6 +548,12 @@ impl App { let preview_height = self.preview_height(); let popup_height = self.input.popup_height(); let modal_height = self.modals.height(frame.area().width); + // Fill the backing buffer with surface bg so widgets that don't paint their entire area + // (gaps between blocks, modal bodies, popup rows) inherit the active theme. + frame.render_widget( + ratatui::widgets::Block::default().style(self.theme.surface()), + frame.area(), + ); let chunks = Layout::vertical([ Constraint::Length(2), Constraint::Min(1), @@ -2393,6 +2403,48 @@ mod tests { ); } + #[tokio::test] + async fn swap_theme_with_unknown_name_pushes_error_and_keeps_active_theme() { + // Pins the defensive `else` arm — load_builtin returning None must not silently swallow + // the request. Today the slash form pre-validates names, but the arm guards against any + // future caller passing through unchecked. + let (mut app, _rx, _agent_tx) = test_app(None); + let original = app.session_info.config.theme_name.clone(); + let entries_before = app.chat.entry_count(); + + app.dispatch_user_action(UserAction::SwapTheme { + name: "nonexistent".to_owned(), + }); + + assert_eq!( + app.session_info.config.theme_name, original, + "unknown name must not flip the active theme", + ); + assert_eq!( + app.chat.entry_count(), + entries_before + 1, + "error must surface as a chat block", + ); + } + + #[test] + fn slash_typed_swap_theme_routes_through_local_handler() { + // Regression: synthesized non-SubmitPrompt actions (e.g. `/theme latte`) used to be + // forwarded straight to the agent, which silently dropped them. They must now flow + // through dispatch_user_action so the local theme arm runs. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SubmitPrompt("/theme latte".to_owned())); + + assert_eq!( + app.session_info.config.theme_name, "latte", + "typed `/theme ` must mutate session theme", + ); + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "TUI-only theme swap must not leak to the agent channel", + ); + } + // ── render_preview ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 6d71daaf..9875a203 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -1,6 +1,9 @@ //! Slash-command autocomplete popup. Selected row paints in `text`, others dim — contrast //! stands in for a prefix glyph or fill. Aliases parenthesize only the typed alias //! (`/clear (new)`); never the full list. +//! +//! Lists longer than [`MAX_VISIBLE_ROWS`] scroll: the cursor stays at the visual middle once it +//! leaves the top half of the window, mirroring Claude Code's typeahead. use ratatui::Frame; use ratatui::layout::Rect; @@ -73,15 +76,11 @@ impl SlashPopup { }; } - /// Number of rows the popup needs in the surrounding layout. Zero - /// when hidden so the input keeps full height. + /// Number of rows the popup needs in the surrounding layout. Zero when hidden, capped at + /// [`MAX_VISIBLE_ROWS`] regardless of match count — overflow scrolls. pub(crate) fn height(&self) -> u16 { - if self.matches.is_empty() { - return 0; - } let visible = self.matches.len().min(MAX_VISIBLE_ROWS); - let footer = usize::from(self.matches.len() > MAX_VISIBLE_ROWS); - u16::try_from(visible + footer).unwrap_or(u16::MAX) + u16::try_from(visible).unwrap_or(u16::MAX) } pub(crate) fn render(&self, frame: &mut Frame, area: Rect) { @@ -89,22 +88,28 @@ impl SlashPopup { return; } let width = usize::from(area.width); - let label_width = self - .matches - .iter() - .take(MAX_VISIBLE_ROWS) - .map(|m| label(m).width()) - .max() - .unwrap_or(0); - let lines: Vec> = self - .matches + let offset = self.scroll_offset(); + let visible = self.matches.len().min(MAX_VISIBLE_ROWS); + let window = &self.matches[offset..offset + visible]; + let label_width = window.iter().map(|m| label(m).width()).max().unwrap_or(0); + let lines: Vec> = window .iter() - .take(MAX_VISIBLE_ROWS) .enumerate() - .map(|(i, m)| self.render_row(m, i == self.selected, label_width, width)) - .chain(self.render_footer(width)) + .map(|(i, m)| self.render_row(m, offset + i == self.selected, label_width, width)) .collect(); - frame.render_widget(Paragraph::new(lines), area); + frame.render_widget(Paragraph::new(lines).style(self.theme.surface()), area); + } + + /// First visible match index. Centered-cursor scroll: the selected row sits at the visual + /// middle once it leaves the top half, then anchors at the bottom near the end of the list. + fn scroll_offset(&self) -> usize { + let total = self.matches.len(); + if total <= MAX_VISIBLE_ROWS { + return 0; + } + let pad = MAX_VISIBLE_ROWS / 2; + let max_offset = total - MAX_VISIBLE_ROWS; + self.selected.saturating_sub(pad).min(max_offset) } fn render_row( @@ -129,19 +134,6 @@ impl SlashPopup { spans.push(Span::styled(desc, row_style)); Line::from(spans) } - - /// `... (N more)` overflow footer; iterator form keeps the caller branch-free. - fn render_footer(&self, width: usize) -> impl Iterator> { - let hidden = self.matches.len().saturating_sub(MAX_VISIBLE_ROWS); - let style = self.theme.dim(); - (hidden > 0) - .then(move || { - let raw = format!(" ... ({hidden} more)"); - let text = truncate_to_width(&raw, width); - Line::from(Span::styled(text, style)) - }) - .into_iter() - } } /// `/name` plus the typed alias when matched via alias (`/clear (new)`). Used for both @@ -269,11 +261,78 @@ mod tests { // ── height ── #[test] - fn height_matches_match_count_below_cap() { - // BUILT_INS sits below `MAX_VISIBLE_ROWS`, so empty query - // renders every command and the popup occupies that many rows. + fn height_caps_at_max_visible_rows() { let popup = popup_with_query(Some("")); - assert_eq!(usize::from(popup.height()), popup.matches.len()); + let expected = popup.matches.len().min(MAX_VISIBLE_ROWS); + assert_eq!(usize::from(popup.height()), expected); + } + + // ── scroll_offset ── + + fn long_popup(n: usize) -> SlashPopup { + // Hand-rolled match list keeps the test independent of registry growth. + let mut p = SlashPopup::new(&theme()); + p.matches = (0..n) + .map(|i| MatchedCommand { + name: Box::leak(format!("cmd{i}").into_boxed_str()), + description: "desc", + matched_alias: None, + }) + .collect(); + p + } + + #[test] + fn scroll_offset_anchors_at_top_while_cursor_in_first_half() { + let mut p = long_popup(MAX_VISIBLE_ROWS + 4); + for _ in 0..MAX_VISIBLE_ROWS / 2 { + assert_eq!(p.scroll_offset(), 0); + p.select_next(); + } + } + + #[test] + fn scroll_offset_centers_cursor_past_first_half() { + // pad = MAX_VISIBLE_ROWS / 2 = 4 for the default cap. At selected = pad + k the offset is + // k, keeping the cursor visually at row `pad`. + let mut p = long_popup(MAX_VISIBLE_ROWS + 4); + for _ in 0..=(MAX_VISIBLE_ROWS / 2) { + p.select_next(); + } + assert_eq!(p.scroll_offset(), 1, "cursor at pad + 1 → offset = 1"); + } + + #[test] + fn scroll_offset_anchors_at_bottom_near_end() { + // Once selected nears the end, the offset clamps to `len - MAX_VISIBLE_ROWS` so the last + // row stays visible while the cursor advances within the bottom-anchored window. + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + while p.selected < total - 1 { + p.select_next(); + } + assert_eq!(p.scroll_offset(), total - MAX_VISIBLE_ROWS); + } + + #[test] + fn scroll_offset_resets_on_wrap_to_first_row() { + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + for _ in 0..total { + p.select_next(); + } + assert_eq!(p.selected, 0, "wrap to row 0"); + assert_eq!(p.scroll_offset(), 0, "wrap snaps the window back to top"); + } + + #[test] + fn scroll_offset_is_zero_when_total_fits_window() { + // With `MAX_VISIBLE_ROWS = 8` the live registry of 9 commands does scroll, but a + // 5-element fake list must never offset. + let mut p = long_popup(5); + p.select_next(); + p.select_next(); + assert_eq!(p.scroll_offset(), 0); } // ── selected ── @@ -376,22 +435,16 @@ mod tests { } #[test] - fn render_overflow_emits_n_more_footer_when_matches_exceed_cap() { - // Cap is 8; a hand-rolled list of 10 commands triggers the - // "... (N more)" footer. Live registry is too small to drive this branch. - let mut popup = SlashPopup::new(&theme()); - popup.matches = (0..10) - .map(|i| MatchedCommand { - name: Box::leak(format!("cmd{i}").into_boxed_str()), - description: "fake", - matched_alias: None, - }) - .collect(); + fn render_after_scroll_shows_window_starting_at_offset() { + // 12 fake rows; advance to row 8 to push the window past the top so `cmd0` is hidden and + // `cmd8` is visible — confirms the slice and the offset agree. + let mut popup = long_popup(12); + for _ in 0..8 { + popup.select_next(); + } let backend = render_to_backend(&popup, 30); let rendered = format!("{backend}"); - assert!( - rendered.contains("(2 more)"), - "footer must surface the hidden count: {rendered}", - ); + assert!(!rendered.contains("cmd0"), "cmd0 scrolled off: {rendered}"); + assert!(rendered.contains("cmd8"), "cmd8 in window: {rendered}"); } } diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap index 3c64b561..6586d9ee 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap @@ -11,4 +11,3 @@ expression: "render_to_backend(&popup, 60)" "/init Generate or update the project's `AGENTS.md` / `..." "/model Open the model picker or switch directly with `/..." "/status Show session info: model, effort, version, worki..." -" ... (1 more) " diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap index 5a74b8a1..ec19c644 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap @@ -11,4 +11,3 @@ expression: "render_to_backend(&popup, 30)" "/init Generate or update..." "/model Open the model pic..." "/status Show session info:..." -" ... (1 more) " From b1c78e521b4a2867d5bbce590da1eb661841040e Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:30:54 +0800 Subject: [PATCH 03/15] fix(tui): expect vendored theme TOML to parse Prior `load_builtin` collapsed unknown-name and parse-error into a single `None`, so a regression in any vendored palette would surface to users as a misleading "unknown theme" error instead of a startup failure. Vendored TOML is build-time content; expect it to parse. --- crates/oxide-code/src/tui/theme.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/oxide-code/src/tui/theme.rs b/crates/oxide-code/src/tui/theme.rs index 0997105c..ece1e089 100644 --- a/crates/oxide-code/src/tui/theme.rs +++ b/crates/oxide-code/src/tui/theme.rs @@ -12,11 +12,10 @@ mod loader; pub(crate) use loader::{SlotPatch, resolve_theme}; /// Resolve a built-in theme name (mocha / latte / ...) to a parsed [`Theme`]. `None` for unknown -/// names; never reads the filesystem. Used by `/theme` for live preview where each cursor move -/// must be cheap. +/// names. Vendored TOML must parse: a parse failure is a build-time bug surfaced at startup. pub(crate) fn load_builtin(name: &str) -> Option { let body = builtin::lookup(name)?; - loader::parse_theme(body).ok() + Some(loader::parse_theme(body).expect("vendored builtin theme TOML must parse")) } /// One theme slot — optional fg, bg, and modifiers composed into a [`Style`]. From bcc5a2182bff770d3d71e5e9874c7f706446f5cd Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:31:07 +0800 Subject: [PATCH 04/15] fix(tui): invalidate streaming-prefix cache on mid-stream theme swap `StreamingAssistant` caches rendered lines up to the most recent `\n\n` boundary; styles bake in at cache time. Bare `/theme` is classified `ReadOnly` so the picker can open mid-stream, but the cached prefix used to keep painting under the previous theme until `commit_streaming` rebuilt it. `ChatView::set_theme` now drops the prefix cache so the next frame re-parses with the active palette. Adds `StreamingAssistant::invalidate_cache` and threads `invalidate_cache_for_width` through it to share the reset logic. --- crates/oxide-code/src/tui/components/chat.rs | 6 +++++- .../src/tui/components/chat/blocks/streaming.rs | 11 ++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/oxide-code/src/tui/components/chat.rs b/crates/oxide-code/src/tui/components/chat.rs index f3474fe1..2dab9c6e 100644 --- a/crates/oxide-code/src/tui/components/chat.rs +++ b/crates/oxide-code/src/tui/components/chat.rs @@ -57,9 +57,13 @@ impl ChatView { } /// Re-skin every subsequent render. Mid-session theme swap (`/theme`) calls this to repaint - /// the existing block stack — block content stays cached, only the styles change next frame. + /// the existing block stack; the streaming-prefix cache also drops so in-flight tokens + /// re-render under the new palette. pub(crate) fn set_theme(&mut self, theme: &Theme) { self.theme = theme.clone(); + if let Some(s) = &mut self.streaming { + s.invalidate_cache(); + } } /// Populates from resumed transcript, projecting the same block shapes as live rendering. diff --git a/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs b/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs index e6184f2e..72c9dd1e 100644 --- a/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs +++ b/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs @@ -43,12 +43,17 @@ impl StreamingAssistant { pub(crate) fn invalidate_cache_for_width(&mut self, width: u16) { if self.cached_width != 0 && self.cached_width != width { - self.rendered.clear(); - self.rendered_boundary = 0; - self.cached_width = 0; + self.invalidate_cache(); } } + /// Drop the prefix cache so the next render re-parses the buffer with current styles. + pub(crate) fn invalidate_cache(&mut self) { + self.rendered.clear(); + self.rendered_boundary = 0; + self.cached_width = 0; + } + /// Caches rendered lines up to the last `\n\n` boundary. pub(crate) fn advance_cache(&mut self, ctx: &RenderCtx<'_>, continues_turn: bool) { if ctx.width == 0 { From f34d48c8a1a17c248fd3dd1ff38ca630b0d0691d Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:33:17 +0800 Subject: [PATCH 05/15] refactor(tui): extract handle_submit_prompt from apply_action_locally The SubmitPrompt arm carried 60+ lines of slash-command dispatch logic inline. Splitting it into a dedicated method keeps `apply_action_locally` under the `clippy::too_many_lines` threshold and makes room for upcoming fixes that grow the variant arms (PreviewTheme drift warn, etc.). No behavior change. --- crates/oxide-code/src/tui/app.rs | 122 +++++++++++++++---------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index ebccfff5..9a2cbf20 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -279,67 +279,7 @@ impl App { /// Applies UI-state changes; returns whether to forward to the agent. fn apply_action_locally(&mut self, action: &UserAction) -> bool { match action { - UserAction::SubmitPrompt(text) => { - if self.input.is_enabled() { - if let Some(parsed) = slash::parse_slash(text) { - self.chat.push_user_message(text.clone()); - let (synthesized, modal) = { - let mut ctx = SlashContext::new(&mut self.chat, &self.session_info); - let action = slash::dispatch(&parsed, &mut ctx); - (action, ctx.take_modal()) - }; - if let Some(modal) = modal { - self.modals.push(modal); - } - if let Some(action) = synthesized { - if matches!(action, UserAction::SubmitPrompt(_)) { - // Synthesized prompts skip apply_action_locally — re-entry would - // re-parse the leading `/` and recurse. - self.input.set_enabled(false); - self.status_bar.set_status(Status::Streaming); - self.forward_to_agent(action); - } else { - self.dispatch_user_action(action); - } - } - return false; - } - self.chat.push_user_message(text.clone()); - self.input.set_enabled(false); - self.status_bar.set_status(Status::Streaming); - true - } else { - if let Some(parsed) = slash::parse_slash(text) { - self.chat.push_user_message(text.clone()); - match slash::classify(&parsed) { - SlashKind::ReadOnly | SlashKind::Unknown => { - let modal = { - let mut ctx = - SlashContext::new(&mut self.chat, &self.session_info); - _ = slash::dispatch(&parsed, &mut ctx); - ctx.take_modal() - }; - if let Some(modal) = modal { - self.modals.push(modal); - } - } - SlashKind::Mutating => { - self.chat.push_system_message(format!( - "/{} runs only when idle. Try again after the turn finishes.", - parsed.name, - )); - } - } - return false; - } - self.pending_prompts.push_back(text.clone()); - self.sync_input_queue_hint(); - // Forward the queued prompt to the agent so it can drain at turn end — - // unless we're already cancelling, in which case the agent will reset and - // the queue drains locally on the next idle transition. - !matches!(self.status_bar.status(), Status::Cancelling) - } - } + UserAction::SubmitPrompt(text) => self.handle_submit_prompt(text), UserAction::Cancel => { self.status_bar.set_status(Status::Cancelling); true @@ -385,6 +325,66 @@ impl App { } } + fn handle_submit_prompt(&mut self, text: &str) -> bool { + if self.input.is_enabled() { + if let Some(parsed) = slash::parse_slash(text) { + self.chat.push_user_message(text.to_owned()); + let (synthesized, modal) = { + let mut ctx = SlashContext::new(&mut self.chat, &self.session_info); + let action = slash::dispatch(&parsed, &mut ctx); + (action, ctx.take_modal()) + }; + if let Some(modal) = modal { + self.modals.push(modal); + } + if let Some(action) = synthesized { + if matches!(action, UserAction::SubmitPrompt(_)) { + // Synthesized prompts skip apply_action_locally — re-entry would + // re-parse the leading `/` and recurse. + self.input.set_enabled(false); + self.status_bar.set_status(Status::Streaming); + self.forward_to_agent(action); + } else { + self.dispatch_user_action(action); + } + } + return false; + } + self.chat.push_user_message(text.to_owned()); + self.input.set_enabled(false); + self.status_bar.set_status(Status::Streaming); + return true; + } + if let Some(parsed) = slash::parse_slash(text) { + self.chat.push_user_message(text.to_owned()); + match slash::classify(&parsed) { + SlashKind::ReadOnly | SlashKind::Unknown => { + let modal = { + let mut ctx = SlashContext::new(&mut self.chat, &self.session_info); + _ = slash::dispatch(&parsed, &mut ctx); + ctx.take_modal() + }; + if let Some(modal) = modal { + self.modals.push(modal); + } + } + SlashKind::Mutating => { + self.chat.push_system_message(format!( + "/{} runs only when idle. Try again after the turn finishes.", + parsed.name, + )); + } + } + return false; + } + self.pending_prompts.push_back(text.to_owned()); + self.sync_input_queue_hint(); + // Forward the queued prompt to the agent so it can drain at turn end — + // unless we're already cancelling, in which case the agent will reset and + // the queue drains locally on the next idle transition. + !matches!(self.status_bar.status(), Status::Cancelling) + } + fn handle_agent_event(&mut self, event: AgentEvent) { match event { AgentEvent::StreamToken(token) => { From d1d7523e45e0080181635b64dfe8740230fb617b Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:33:52 +0800 Subject: [PATCH 06/15] fix(tui): clear preview snapshot before SwapTheme load, log drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SwapTheme` only cleared `preview_theme_snapshot` inside the success arm of `load_builtin`. If the defensive `else` arm ever fired (today unreachable via the slash form, but a future caller could bypass validation), the snapshot would survive and roll back the next modal's cancel. Hoist the reset above the guard so the snapshot's lifetime ends when SwapTheme commits, regardless of result. `PreviewTheme` now logs `tracing::warn!` when `load_builtin` returns `None` — the picker roster (`LISTED_THEMES`) and the loader's lookup table must agree, so a `None` here means drift between the two. User-facing silence stays appropriate (preview repaints on every keystroke), but a log line surfaces the bug to developers. --- crates/oxide-code/src/tui/app.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 9a2cbf20..21a77ca0 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -306,12 +306,14 @@ impl App { self.preview_theme_snapshot = Some(self.theme.clone()); } self.apply_theme(&preview); + } else { + tracing::warn!(name, "PreviewTheme: unknown built-in; picker roster drift"); } false } UserAction::SwapTheme { name } => { + self.preview_theme_snapshot = None; if let Some(theme) = super::theme::load_builtin(name) { - self.preview_theme_snapshot = None; self.session_info.config.theme_name.clone_from(name); self.apply_theme(&theme); self.chat From 8f4906506090fd77c588739b2c873b446fa8f0bd Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:34:23 +0800 Subject: [PATCH 07/15] refactor(slash): simplify numeric_hint via char::from_digit `char::from_digit(_, 10)` already returns `None` for digits >= 10, making the explicit `(1..=9).contains` filter redundant. Collapse the `u32::try_from / checked_add / range-contains / branch` cascade to a single expression. --- crates/oxide-code/src/slash/theme.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index b93f91be..c705c9aa 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -54,12 +54,7 @@ impl ThemeRow { /// `'1'`–`'9'` for the first nine rows; `None` after that. fn numeric_hint(idx: usize) -> Option { - let digit = u32::try_from(idx).ok()?.checked_add(1)?; - if (1..=9).contains(&digit) { - char::from_digit(digit, 10) - } else { - None - } + char::from_digit(u32::try_from(idx).ok()? + 1, 10) } impl PickerItem for ThemeRow { From 6982c443c32309a624d99adfc702674628c90694 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:38:11 +0800 Subject: [PATCH 08/15] style: trim WHAT-only docstrings and drop X-not-Y antithesis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweep across the PR-touched files and adjacent code to align with the project comment guidelines: drop multi-line WHAT-only docstrings where the code conveys the same, and rephrase the `do X, not Y` antithesis tic into positive form. - `slash/theme.rs`: module doc, `LISTED_THEMES`, `resolve_theme_arg`, and the `ThemePicker::height` arithmetic comment. - `tui/app.rs`: module-level dirty-flag note, `apply_theme` doc, surface-bg fill comment, two test docstrings, and the dispatch-fix comment that was on the wrong arm — now lives next to the `else` branch it actually motivates. - `tui/components/input/popup.rs`: module doc and `height` doc. - `config.rs` and two test docstrings in `tui/app.rs`: rephrase `must X, not Y` constructions into positive form. --- crates/oxide-code/src/config.rs | 2 +- crates/oxide-code/src/slash/theme.rs | 16 +++++------- crates/oxide-code/src/tui/app.rs | 25 +++++++------------ .../src/tui/components/input/popup.rs | 8 +++--- 4 files changed, 19 insertions(+), 32 deletions(-) diff --git a/crates/oxide-code/src/config.rs b/crates/oxide-code/src/config.rs index abd3c1fb..33bb2fce 100644 --- a/crates/oxide-code/src/config.rs +++ b/crates/oxide-code/src/config.rs @@ -710,7 +710,7 @@ mod tests { #[tokio::test] async fn load_effort_clamps_xhigh_down_to_high_on_sonnet_4_6() { - // Sonnet 4.6 has effort but not `xhigh` / `max`; must clamp, not 400 the gateway. + // Sonnet 4.6 has effort but caps below `xhigh`; clamping keeps the request from 400ing. let dir = tempfile::tempdir().unwrap(); let vars = env_vars(vec![ xdg(&dir), diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index c705c9aa..0d0b9e9a 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -1,6 +1,5 @@ -//! `/theme` — open the picker, or swap with `/theme `. Bare form opens a list picker -//! whose cursor moves live-preview the highlighted theme by repainting the full TUI; Esc snaps -//! back to the original, Enter commits for the rest of the session. +//! `/theme` — open the picker, or swap directly with `/theme `. Picker live-previews on +//! cursor moves and reverts on cancel. use crossterm::event::{KeyCode, KeyEvent}; use ratatui::Frame; @@ -17,9 +16,7 @@ use crate::tui::theme::Theme; // ── Constants ── -/// Curated roster shown in the picker. Order matches `tui/theme/builtin.rs` — Catppuccin variants -/// dark→light, then non-Catppuccin palettes. Static descriptions live alongside so the picker -/// can render a one-glance summary without re-resolving each TOML. +/// Curated picker roster (name, one-line description). Order matches `tui/theme/builtin.rs`. const LISTED_THEMES: &[(&str, &str)] = &[ ("mocha", "Catppuccin dark (default)"), ("macchiato", "Catppuccin dark (medium)"), @@ -52,7 +49,6 @@ impl ThemeRow { } } -/// `'1'`–`'9'` for the first nine rows; `None` after that. fn numeric_hint(idx: usize) -> Option { char::from_digit(u32::try_from(idx).ok()? + 1, 10) } @@ -119,7 +115,7 @@ impl ThemePicker { impl Modal for ThemePicker { fn height(&self, width: u16) -> u16 { - // List body + spacer + footer. + // +2: blank spacer row, then the footer hint row. self.list.height(width).saturating_add(2) } @@ -208,8 +204,8 @@ impl SlashCommand for ThemeCmd { } } -/// Validates `arg` against the curated picker roster. Custom file paths aren't accepted via -/// `/theme ` — users who maintain a custom TOML edit `~/.config/ox/config.toml` directly. +/// Custom file paths are intentionally rejected here; users with a custom TOML edit +/// `~/.config/ox/config.toml` directly. fn resolve_theme_arg(arg: &str) -> Result<&'static str, String> { let lower = arg.to_ascii_lowercase(); LISTED_THEMES diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 21a77ca0..f1984b9a 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -2,7 +2,7 @@ //! //! [`App`] owns every component, holds the cross-task channels, and runs the `tokio::select!` //! loop multiplexing crossterm events, agent events, user actions, and a 60 FPS render tick. -//! A dirty flag coalesces redraws so render work tracks state change, not event throughput. +//! A dirty flag coalesces redraws so renders fire per state change rather than per event. use std::collections::{HashMap, VecDeque}; use std::sync::Arc; @@ -217,9 +217,7 @@ impl App { } } - /// Repaint every component with `theme`. Sole entry point for mid-session theme swaps so the - /// per-component `set_theme` calls stay grouped — adding a new theme-styled component should - /// only need a single edit here. + /// Sole entry point for mid-session theme swaps; repaints every theme-styled component. fn apply_theme(&mut self, theme: &Theme) { self.theme = theme.clone(); self.chat.set_theme(theme); @@ -341,12 +339,12 @@ impl App { } if let Some(action) = synthesized { if matches!(action, UserAction::SubmitPrompt(_)) { - // Synthesized prompts skip apply_action_locally — re-entry would - // re-parse the leading `/` and recurse. self.input.set_enabled(false); self.status_bar.set_status(Status::Streaming); self.forward_to_agent(action); } else { + // TUI-only synthesized actions (e.g. SwapTheme) need apply_action_locally — + // forwarding to the agent drops them. self.dispatch_user_action(action); } } @@ -550,8 +548,7 @@ impl App { let preview_height = self.preview_height(); let popup_height = self.input.popup_height(); let modal_height = self.modals.height(frame.area().width); - // Fill the backing buffer with surface bg so widgets that don't paint their entire area - // (gaps between blocks, modal bodies, popup rows) inherit the active theme. + // Pre-fill with surface bg so unpainted gaps inherit the theme. frame.render_widget( ratatui::widgets::Block::default().style(self.theme.surface()), frame.area(), @@ -921,7 +918,7 @@ mod tests { #[tokio::test] async fn handle_crossterm_key_ctrl_c_busy_forwards_cancel_without_quitting() { - // Mid-turn Ctrl+C must reach the agent loop as `Cancel`, not flip `should_quit`. + // Mid-turn Ctrl+C forwards `Cancel` to the agent loop and leaves `should_quit` clear. let (mut app, mut rx, _agent_tx) = test_app(None); app.handle_agent_event(AgentEvent::StreamToken("partial".into())); assert!(!app.input.is_enabled()); @@ -1359,8 +1356,8 @@ mod tests { #[tokio::test] async fn dispatch_read_only_slash_during_busy_runs_client_side_without_queueing() { - // Read-only slash commands during busy must run immediately, not queue — - // otherwise the LLM ends up answering `/help` literally on drain. + // Read-only slash commands during busy run client-side immediately; queueing them + // would make the LLM answer `/help` literally on drain. let (mut app, mut rx, _agent_tx) = test_app(None); app.dispatch_user_action(UserAction::SubmitPrompt("active".to_owned())); rx.recv().await.expect("active submit forwarded"); @@ -2334,8 +2331,6 @@ mod tests { #[test] fn preview_theme_repaints_components_and_caches_original() { - // First PreviewTheme captures the snapshot; cursor moves on the picker repaint without - // committing. Apply a known mocha → latte swap and confirm both axes mutate. let (mut app, _rx, _agent_tx) = test_app(None); let original = app.theme.clone(); app.dispatch_user_action(UserAction::PreviewTheme { @@ -2346,7 +2341,7 @@ mod tests { "first PreviewTheme must cache the original", ); assert_ne!(app.theme.text, original.text, "live theme must change"); - // session_info.config.theme_name unchanged — preview is not a commit. + // preview leaves session_info.config.theme_name as-is until commit. assert_eq!(app.session_info.config.theme_name, "mocha"); } @@ -2368,8 +2363,6 @@ mod tests { #[test] fn swap_theme_commits_and_clears_snapshot() { - // Enter on a moved cursor: snapshot drops, session theme name updates, chat shows the - // confirmation. A subsequent ModalAction::None then must NOT roll anything back. let (mut app, _rx, _agent_tx) = test_app(None); app.dispatch_user_action(UserAction::PreviewTheme { name: "latte".to_owned(), diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 9875a203..b7344577 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -1,9 +1,8 @@ //! Slash-command autocomplete popup. Selected row paints in `text`, others dim — contrast //! stands in for a prefix glyph or fill. Aliases parenthesize only the typed alias -//! (`/clear (new)`); never the full list. +//! (`/clear (new)`); the full list stays unparenthesized. //! -//! Lists longer than [`MAX_VISIBLE_ROWS`] scroll: the cursor stays at the visual middle once it -//! leaves the top half of the window, mirroring Claude Code's typeahead. +//! Lists past [`MAX_VISIBLE_ROWS`] scroll with a centered cursor (Claude Code typeahead style). use ratatui::Frame; use ratatui::layout::Rect; @@ -76,8 +75,7 @@ impl SlashPopup { }; } - /// Number of rows the popup needs in the surrounding layout. Zero when hidden, capped at - /// [`MAX_VISIBLE_ROWS`] regardless of match count — overflow scrolls. + /// Row count needed in layout; zero when hidden, capped at [`MAX_VISIBLE_ROWS`]. pub(crate) fn height(&self) -> u16 { let visible = self.matches.len().min(MAX_VISIBLE_ROWS); u16::try_from(visible).unwrap_or(u16::MAX) From 801de793a8a430101771482b9073bfcd70dd3345 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:38:46 +0800 Subject: [PATCH 09/15] test(tui): pin scroll-offset, surface-bg fill, and typed /model dispatch - `draw_frame_surface_fill_overwrites_unpainted_cells_with_surface_bg` pre-stains the buffer with a sentinel colour, renders the frame, and asserts no sentinel survives. Pins the buffer-wide invariant that any future widget leaving cells unpainted gets caught. - `slash_typed_model_routes_synthesized_swap_config_through_dispatch` mirrors the typed-`/theme` regression on the agent-bound side: `/model haiku` synthesizes a `SwapConfig` that must reach the agent via dispatch. Existing `dispatch_swap_config_*` covered direct emission; this exercises the synthesized-via-slash path. - Three new `scroll_offset_*` cases pin (a) the centering invariant directly via `visible_row == pad`, (b) the `total == MAX_VISIBLE_ROWS` boundary that the `<=` early-return guards, and (c) the symmetric `select_prev` wrap-from-top where the offset clamps to the bottom. --- crates/oxide-code/src/tui/app.rs | 70 ++++++++++++++++++- .../src/tui/components/input/popup.rs | 38 ++++++++++ 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index f1984b9a..110ae3ba 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -325,6 +325,9 @@ impl App { } } + /// Returns whether the submitted text should also forward to the agent. Slash commands always + /// return false — they emit synthesized actions through `dispatch_user_action` / + /// `forward_to_agent` directly. Plain prompts return true while idle, false while cancelling. fn handle_submit_prompt(&mut self, text: &str) -> bool { if self.input.is_enabled() { if let Some(parsed) = slash::parse_slash(text) { @@ -379,9 +382,9 @@ impl App { } self.pending_prompts.push_back(text.to_owned()); self.sync_input_queue_hint(); - // Forward the queued prompt to the agent so it can drain at turn end — - // unless we're already cancelling, in which case the agent will reset and - // the queue drains locally on the next idle transition. + // Forward the queued prompt to the agent so it can drain at turn end — unless we're + // already cancelling, in which case the agent will reset and the queue drains locally on + // the next idle transition. !matches!(self.status_bar.status(), Status::Cancelling) } @@ -2422,6 +2425,47 @@ mod tests { ); } + #[test] + fn draw_frame_surface_fill_overwrites_unpainted_cells_with_surface_bg() { + // Buffer-wide invariant: pre-stain every cell, render, and assert no sentinel survives. + // The frame-area surface fill is the only widget that guarantees this for cells no other + // widget covers (gaps that emerge with future layout or widget additions). + use ratatui::style::Color; + + let (mut app, _rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + let surface_bg = app + .theme + .surface() + .bg + .expect("latte sets explicit surface bg"); + let sentinel = Color::Rgb(254, 0, 254); + assert_ne!(surface_bg, sentinel, "sentinel must differ from theme bg"); + + let mut terminal = Terminal::new(TestBackend::new(60, 10)).unwrap(); + for cell in &mut terminal.current_buffer_mut().content { + cell.set_bg(sentinel); + } + terminal + .draw(|frame| { + _ = app.draw_frame(frame); + }) + .unwrap(); + + let buffer = terminal.backend().buffer(); + for y in 0..10 { + for x in 0..60 { + let cell = buffer.cell((x, y)).expect("cell in bounds"); + assert_ne!( + cell.bg, sentinel, + "cell ({x},{y}) kept the sentinel — surface fill regressed", + ); + } + } + } + #[test] fn slash_typed_swap_theme_routes_through_local_handler() { // Regression: synthesized non-SubmitPrompt actions (e.g. `/theme latte`) used to be @@ -2440,6 +2484,26 @@ mod tests { ); } + #[tokio::test] + async fn slash_typed_model_routes_synthesized_swap_config_through_dispatch() { + // Mirrors the typed-`/theme` regression on the agent-bound side: `/model ` synthesizes + // a SwapConfig that must reach the agent via dispatch_user_action → forward_to_agent. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SubmitPrompt("/model haiku".to_owned())); + + let forwarded = rx.recv().await.expect("SwapConfig forwarded to agent"); + match forwarded { + UserAction::SwapConfig { model, effort } => { + assert_eq!( + model.as_ref().map(crate::model::ResolvedModelId::as_str), + Some("claude-haiku-4-5") + ); + assert_eq!(effort, None); + } + other => panic!("expected SwapConfig, got {other:?}"), + } + } + // ── render_preview ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index b7344577..519780fd 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -333,6 +333,44 @@ mod tests { assert_eq!(p.scroll_offset(), 0); } + #[test] + fn scroll_offset_keeps_visible_row_at_pad_for_mid_list_selection() { + // Pin the centering invariant directly: the visible row of the cursor (selected - offset) + // equals `pad` whenever the selection is past the top half but not yet near the bottom. + // Mutating the divisor (e.g. `/3`) or the formula would shift this row index. + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + let pad = MAX_VISIBLE_ROWS / 2; + for _ in 0..=(pad + 1) { + p.select_next(); + } + let visible_row = p.selected - p.scroll_offset(); + assert_eq!(visible_row, pad, "cursor must sit at the visual middle row"); + } + + #[test] + fn scroll_offset_at_exactly_cap_returns_zero_for_last_row() { + // Boundary: total == MAX_VISIBLE_ROWS hits the `<=` early-return. Mutating to `<` would + // try to compute max_offset = 0 and still produce 0 here, but tightening the boundary + // pins the invariant for the only case where the edge matters. + let mut p = long_popup(MAX_VISIBLE_ROWS); + while p.selected < MAX_VISIBLE_ROWS - 1 { + p.select_next(); + } + assert_eq!(p.scroll_offset(), 0); + } + + #[test] + fn scroll_offset_select_prev_from_top_anchors_at_bottom_window() { + // Up-arrow from row 0 wraps to the last row; the bottom-anchored window must clamp to + // `len - MAX_VISIBLE_ROWS` (the symmetric case to the existing wrap-to-top test). + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + p.select_prev(); + assert_eq!(p.selected, total - 1, "wrap to last row"); + assert_eq!(p.scroll_offset(), total - MAX_VISIBLE_ROWS); + } + // ── selected ── #[test] From 13ca7a2745ee8df78a12fce84a3d9f543169edbe Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 19:52:45 +0800 Subject: [PATCH 10/15] test: cover codecov gaps from PR #67 patch Targeted gap-fillers identified from the codecov patch report: - `up_and_k_emit_preview_for_prev_row_without_popping` and `j_alias_for_down_emits_preview` exercise the `Up | k` and `Down | j` arms in `ThemePicker::handle_key`. Existing tests covered Down only. - `preview_theme_with_unknown_name_does_nothing_and_keeps_active_theme` hits the `tracing::warn!` arm in `App::apply_action_locally` when a preview-roster drift would surface. Pins user-visible no-op state. - `set_theme_invalidates_streaming_cache_so_in_flight_tokens_repaint` and `set_theme_without_active_streaming_is_a_noop_on_cache_state` cover both arms of the new `if let Some(streaming)` guard in `ChatView::set_theme`, ensuring future refactors don't invert it. --- crates/oxide-code/src/slash/theme.rs | 34 ++++++++++++++++++++ crates/oxide-code/src/tui/app.rs | 25 ++++++++++++++ crates/oxide-code/src/tui/components/chat.rs | 32 ++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index 0d0b9e9a..29e63dc5 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -351,6 +351,40 @@ mod tests { } } + #[test] + fn up_and_k_emit_preview_for_prev_row_without_popping() { + // Up arrow and `k` (vi binding) share an arm; both must cycle to the previous row and + // emit a preview. From the first row they wrap to the last via `select_prev`. + for code in [KeyCode::Up, KeyCode::Char('k')] { + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(code)); + match outcome { + ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { + assert_eq!( + name, + LISTED_THEMES.last().unwrap().0, + "{code:?} from first row wraps to last", + ); + } + other => panic!("expected Preview for {code:?}, got {other:?}"), + } + } + } + + #[test] + fn j_alias_for_down_emits_preview() { + // `j` shares the Down arm; pin the alias so a regression in the match doesn't silently + // drop it. + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(KeyCode::Char('j'))); + match outcome { + ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { + assert_eq!(name, "macchiato"); + } + other => panic!("expected Preview, got {other:?}"), + } + } + #[test] fn numeric_jump_emits_preview_for_target_row() { let mut p = picker_with_active("mocha"); diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 110ae3ba..c92a088b 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -2401,6 +2401,31 @@ mod tests { ); } + #[test] + fn preview_theme_with_unknown_name_does_nothing_and_keeps_active_theme() { + // The drift warn arm fires when a PreviewTheme name doesn't resolve. The picker roster + // and the loader's lookup table must agree, so this is a developer-facing log only; + // user-visible state stays put (no snapshot, no theme swap, no chat block). + let (mut app, _rx, _agent_tx) = test_app(None); + let original_text = app.theme.text; + let entries_before = app.chat.entry_count(); + + app.dispatch_user_action(UserAction::PreviewTheme { + name: "nonexistent".to_owned(), + }); + + assert!( + app.preview_theme_snapshot.is_none(), + "no snapshot when load_builtin returns None", + ); + assert_eq!(app.theme.text, original_text, "theme stays put on drift"); + assert_eq!( + app.chat.entry_count(), + entries_before, + "preview must never push a chat block", + ); + } + #[tokio::test] async fn swap_theme_with_unknown_name_pushes_error_and_keeps_active_theme() { // Pins the defensive `else` arm — load_builtin returning None must not silently swallow diff --git a/crates/oxide-code/src/tui/components/chat.rs b/crates/oxide-code/src/tui/components/chat.rs index 2dab9c6e..8f9463ec 100644 --- a/crates/oxide-code/src/tui/components/chat.rs +++ b/crates/oxide-code/src/tui/components/chat.rs @@ -2028,6 +2028,38 @@ mod tests { assert_eq!(chat.viewport_height, 20); } + #[test] + fn set_theme_invalidates_streaming_cache_so_in_flight_tokens_repaint() { + // Bare `/theme` opens its picker mid-stream; without dropping the cache, committed + // prefix tokens would keep painting under the previous palette until commit_streaming. + let mut chat = test_chat(); + _ = chat.update_layout(Rect::new(0, 0, 80, 24)); + chat.append_stream_token("a complete paragraph\n\n"); + let s = chat.streaming.as_ref().unwrap(); + assert_ne!(s.rendered_len(), 0, "cache populated"); + assert_eq!(s.cached_width(), 80); + + chat.set_theme(&Theme::default()); + + let s = chat.streaming.as_ref().unwrap(); + assert_eq!(s.rendered_len(), 0, "cache dropped on theme swap"); + assert_eq!(s.rendered_boundary(), 0); + assert_eq!(s.cached_width(), 0); + } + + #[test] + fn set_theme_without_active_streaming_is_a_noop_on_cache_state() { + // The `if let Some(streaming)` guard skips the cache reset when no stream is in flight; + // pin both branches so the guard isn't accidentally inverted in a future refactor. + let mut chat = test_chat(); + assert!(chat.streaming.is_none()); + chat.set_theme(&Theme::default()); + assert!( + chat.streaming.is_none(), + "set_theme must not spawn a stream" + ); + } + #[test] fn update_layout_invalidates_streaming_cache_on_width_change() { let mut chat = test_chat(); From 2589673d6d6f01c3e761af186d5ab1552028a41d Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 20:07:29 +0800 Subject: [PATCH 11/15] refactor(tui, slash): align test-section ordering with production MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Project convention requires test sections to mirror the production function order in the same file. Several recent additions landed in sections that no longer matched the prod layout — sweep them now. - `tui/components/chat.rs`: `set_theme` tests had no section header and lived inside the `update_layout` block. Move them into a fresh `// ── set_theme ──` section between `Fixtures` and `load_history`, matching the prod method's position at line 62. - `tui/app.rs`: `draw_frame_surface_fill_*` belongs in the `// ── draw_frame ──` section, not the theme section. Reorder the apply_action_locally / theme tests so each preview / swap variant groups with its happy-path neighbour (preview happy → preview cancel → preview unknown; swap happy → swap unknown). - `slash/theme.rs`: section order now mirrors prod — ThemeRow → numeric_hint → ThemePicker::new → ThemePicker::handle_key → ThemePicker::render → ThemeCmd metadata → ThemeCmd::execute. Combine the asymmetric `down_arrow_emits_preview` plus `j_alias_for_down_emits_preview` pair into a single `down_and_j_emit_preview_for_next_row_without_popping`, matching the existing `up_and_k_emit_preview_for_prev_row...`. - `tui/components/input/popup.rs`: move `selected` ahead of `select_next / select_prev` and `scroll_offset` after `render`, matching the prod method declarations at lines 56 / 60 / 84 / 103. Reorder the eight `scroll_offset_*` cases so each test groups with its thematic partner (no-scroll cases → top → mid pinning → bottom → wrap pair). - `tui/components/chat/blocks/streaming.rs`: have `take_buffer` delegate to `invalidate_cache` instead of duplicating its three resets — the helper now exists for `set_theme` to call. --- crates/oxide-code/src/slash/theme.rs | 182 +++++++------- crates/oxide-code/src/tui/app.rs | 146 ++++++------ crates/oxide-code/src/tui/components/chat.rs | 66 +++--- .../tui/components/chat/blocks/streaming.rs | 4 +- .../src/tui/components/input/popup.rs | 222 +++++++++--------- 5 files changed, 305 insertions(+), 315 deletions(-) diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index 29e63dc5..6b023d5a 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -254,68 +254,39 @@ mod tests { ThemePicker::new(name) } - // ── ThemeCmd metadata ── - - #[test] - fn metadata_matches_built_ins_contract() { - assert_eq!(ThemeCmd.name(), "theme"); - assert!(ThemeCmd.aliases().is_empty()); - assert!(!ThemeCmd.description().is_empty()); - assert_eq!(ThemeCmd.usage(), Some("[]")); - } + // ── ThemeRow ── #[test] - fn classify_splits_on_args() { - // Bare form opens the picker (read-only); typed arg races the client (mutating). - assert_eq!(ThemeCmd.classify(""), SlashKind::ReadOnly); - assert_eq!(ThemeCmd.classify(" "), SlashKind::ReadOnly); - assert_eq!(ThemeCmd.classify("latte"), SlashKind::Mutating); - } - - // ── ThemeCmd::execute ── + fn theme_row_picker_item_methods_return_curated_values() { + // Pins the trait-impl surface for ThemeRow — label / description / is_active / key_hint + // each have a path the picker depends on and only the render smoke test exercises + // indirectly. + let rows = ThemeRow::build("latte"); + let names: Vec<&str> = rows.iter().map(PickerItem::label).collect(); + assert_eq!( + names, + LISTED_THEMES.iter().map(|(n, _)| *n).collect::>(), + ); - #[test] - fn execute_no_args_opens_picker_and_pushes_no_chat_block() { - let (chat, modal, outcome) = run_execute(""); - assert_eq!(outcome, Ok(SlashOutcome::Done)); - assert!(modal.is_some(), "bare /theme must populate the modal slot"); - assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); - } + let active_count = rows.iter().filter(|r| r.is_active()).count(); + assert_eq!(active_count, 1, "exactly one row marks the active theme"); + assert!(rows.iter().find(|r| r.is_active()).unwrap().label() == "latte"); - #[test] - fn execute_with_known_name_forwards_swap_theme() { - for (name, _) in LISTED_THEMES { - let (_, _, outcome) = run_execute(name); - assert_eq!( - outcome, - Ok(SlashOutcome::Forward(UserAction::SwapTheme { - name: (*name).to_owned(), - })), - "`{name}` must forward SwapTheme", - ); + for (idx, row) in rows.iter().enumerate() { + assert_eq!(row.key_hint(), numeric_hint(idx), "idx={idx}"); + assert_eq!(row.description(), Some(LISTED_THEMES[idx].1)); } } - #[test] - fn execute_is_case_insensitive() { - let (_, _, outcome) = run_execute("LATTE"); - assert_eq!( - outcome, - Ok(SlashOutcome::Forward(UserAction::SwapTheme { - name: "latte".to_owned(), - })), - ); - } + // ── numeric_hint ── #[test] - fn execute_unknown_name_errors_listing_valid_options() { - let (chat, _, outcome) = run_execute("solarized"); - let msg = outcome.expect_err("unknown name must error"); - assert!(msg.starts_with("Unknown theme: `solarized`."), "{msg}"); - for (name, _) in LISTED_THEMES { - assert!(msg.contains(name), "lists `{name}`: {msg}"); + fn numeric_hint_covers_first_nine_rows_then_returns_none() { + for idx in 0..9 { + let expected = char::from_digit(u32::try_from(idx + 1).unwrap(), 10); + assert_eq!(numeric_hint(idx), expected, "idx={idx}"); } - assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); + assert_eq!(numeric_hint(9), None, "10th row has no hint"); } // ── ThemePicker::new ── @@ -340,14 +311,18 @@ mod tests { // ── ThemePicker::handle_key ── #[test] - fn down_arrow_emits_preview_for_next_row_without_popping() { - let mut p = picker_with_active("mocha"); - let outcome = p.handle_key(&key(KeyCode::Down)); - match outcome { - ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { - assert_eq!(name, "macchiato", "Down from mocha lands on macchiato"); + fn down_and_j_emit_preview_for_next_row_without_popping() { + // Down arrow and `j` (vi binding) share an arm; both must advance to the next row and + // emit a preview without popping the modal. + for code in [KeyCode::Down, KeyCode::Char('j')] { + let mut p = picker_with_active("mocha"); + let outcome = p.handle_key(&key(code)); + match outcome { + ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { + assert_eq!(name, "macchiato", "{code:?} from mocha lands on macchiato"); + } + other => panic!("expected Preview(PreviewTheme) for {code:?}, got {other:?}"), } - other => panic!("expected Preview(PreviewTheme), got {other:?}"), } } @@ -371,20 +346,6 @@ mod tests { } } - #[test] - fn j_alias_for_down_emits_preview() { - // `j` shares the Down arm; pin the alias so a regression in the match doesn't silently - // drop it. - let mut p = picker_with_active("mocha"); - let outcome = p.handle_key(&key(KeyCode::Char('j'))); - match outcome { - ModalKey::Preview(ModalAction::User(UserAction::PreviewTheme { name })) => { - assert_eq!(name, "macchiato"); - } - other => panic!("expected Preview, got {other:?}"), - } - } - #[test] fn numeric_jump_emits_preview_for_target_row() { let mut p = picker_with_active("mocha"); @@ -457,38 +418,67 @@ mod tests { } } - // ── ThemeRow ── + // ── ThemeCmd metadata ── #[test] - fn theme_row_picker_item_methods_return_curated_values() { - // Pins the trait-impl surface for ThemeRow — label / description / is_active / key_hint - // each have a path the picker depends on and only the render smoke test exercises - // indirectly. - let rows = ThemeRow::build("latte"); - let names: Vec<&str> = rows.iter().map(PickerItem::label).collect(); - assert_eq!( - names, - LISTED_THEMES.iter().map(|(n, _)| *n).collect::>(), - ); + fn metadata_matches_built_ins_contract() { + assert_eq!(ThemeCmd.name(), "theme"); + assert!(ThemeCmd.aliases().is_empty()); + assert!(!ThemeCmd.description().is_empty()); + assert_eq!(ThemeCmd.usage(), Some("[]")); + } - let active_count = rows.iter().filter(|r| r.is_active()).count(); - assert_eq!(active_count, 1, "exactly one row marks the active theme"); - assert!(rows.iter().find(|r| r.is_active()).unwrap().label() == "latte"); + #[test] + fn classify_splits_on_args() { + // Bare form opens the picker (read-only); typed arg races the client (mutating). + assert_eq!(ThemeCmd.classify(""), SlashKind::ReadOnly); + assert_eq!(ThemeCmd.classify(" "), SlashKind::ReadOnly); + assert_eq!(ThemeCmd.classify("latte"), SlashKind::Mutating); + } - for (idx, row) in rows.iter().enumerate() { - assert_eq!(row.key_hint(), numeric_hint(idx), "idx={idx}"); - assert_eq!(row.description(), Some(LISTED_THEMES[idx].1)); + // ── ThemeCmd::execute ── + + #[test] + fn execute_no_args_opens_picker_and_pushes_no_chat_block() { + let (chat, modal, outcome) = run_execute(""); + assert_eq!(outcome, Ok(SlashOutcome::Done)); + assert!(modal.is_some(), "bare /theme must populate the modal slot"); + assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); + } + + #[test] + fn execute_with_known_name_forwards_swap_theme() { + for (name, _) in LISTED_THEMES { + let (_, _, outcome) = run_execute(name); + assert_eq!( + outcome, + Ok(SlashOutcome::Forward(UserAction::SwapTheme { + name: (*name).to_owned(), + })), + "`{name}` must forward SwapTheme", + ); } } - // ── numeric_hint ── + #[test] + fn execute_is_case_insensitive() { + let (_, _, outcome) = run_execute("LATTE"); + assert_eq!( + outcome, + Ok(SlashOutcome::Forward(UserAction::SwapTheme { + name: "latte".to_owned(), + })), + ); + } #[test] - fn numeric_hint_covers_first_nine_rows_then_returns_none() { - for idx in 0..9 { - let expected = char::from_digit(u32::try_from(idx + 1).unwrap(), 10); - assert_eq!(numeric_hint(idx), expected, "idx={idx}"); + fn execute_unknown_name_errors_listing_valid_options() { + let (chat, _, outcome) = run_execute("solarized"); + let msg = outcome.expect_err("unknown name must error"); + assert!(msg.starts_with("Unknown theme: `solarized`."), "{msg}"); + for (name, _) in LISTED_THEMES { + assert!(msg.contains(name), "lists `{name}`: {msg}"); } - assert_eq!(numeric_hint(9), None, "10th row has no hint"); + assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); } } diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index c92a088b..984b6a7c 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -2310,6 +2310,47 @@ mod tests { insta::assert_snapshot!(render_app(&mut app, 60, 14)); } + #[test] + fn draw_frame_surface_fill_overwrites_unpainted_cells_with_surface_bg() { + // Buffer-wide invariant: pre-stain every cell, render, and assert no sentinel survives. + // The frame-area surface fill is the only widget that guarantees this for cells no other + // widget covers (gaps that emerge with future layout or widget additions). + use ratatui::style::Color; + + let (mut app, _rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + let surface_bg = app + .theme + .surface() + .bg + .expect("latte sets explicit surface bg"); + let sentinel = Color::Rgb(254, 0, 254); + assert_ne!(surface_bg, sentinel, "sentinel must differ from theme bg"); + + let mut terminal = Terminal::new(TestBackend::new(60, 10)).unwrap(); + for cell in &mut terminal.current_buffer_mut().content { + cell.set_bg(sentinel); + } + terminal + .draw(|frame| { + _ = app.draw_frame(frame); + }) + .unwrap(); + + let buffer = terminal.backend().buffer(); + for y in 0..10 { + for x in 0..60 { + let cell = buffer.cell((x, y)).expect("cell in bounds"); + assert_ne!( + cell.bg, sentinel, + "cell ({x},{y}) kept the sentinel — surface fill regressed", + ); + } + } + } + // ── preview_height ── #[test] @@ -2364,43 +2405,6 @@ mod tests { assert_eq!(app.theme.text, original.text, "theme rolled back to mocha"); } - #[test] - fn swap_theme_commits_and_clears_snapshot() { - let (mut app, _rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::PreviewTheme { - name: "latte".to_owned(), - }); - let previewed = app.theme.clone(); - app.dispatch_user_action(UserAction::SwapTheme { - name: "latte".to_owned(), - }); - assert!(app.preview_theme_snapshot.is_none()); - assert_eq!(app.session_info.config.theme_name, "latte"); - assert_eq!(app.theme.text, previewed.text); - - app.apply_modal_action(ModalAction::None); - assert_eq!( - app.session_info.config.theme_name, "latte", - "post-commit cancel must not restore the pre-preview theme", - ); - } - - #[tokio::test] - async fn theme_actions_are_not_forwarded_to_agent_loop() { - // PreviewTheme / SwapTheme are TUI-only; reaching `user_tx` would race the client. - let (mut app, mut rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::PreviewTheme { - name: "latte".to_owned(), - }); - app.dispatch_user_action(UserAction::SwapTheme { - name: "latte".to_owned(), - }); - assert!( - matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), - "theme actions must stay client-side", - ); - } - #[test] fn preview_theme_with_unknown_name_does_nothing_and_keeps_active_theme() { // The drift warn arm fires when a PreviewTheme name doesn't resolve. The picker roster @@ -2426,6 +2430,27 @@ mod tests { ); } + #[test] + fn swap_theme_commits_and_clears_snapshot() { + let (mut app, _rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + let previewed = app.theme.clone(); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + assert!(app.preview_theme_snapshot.is_none()); + assert_eq!(app.session_info.config.theme_name, "latte"); + assert_eq!(app.theme.text, previewed.text); + + app.apply_modal_action(ModalAction::None); + assert_eq!( + app.session_info.config.theme_name, "latte", + "post-commit cancel must not restore the pre-preview theme", + ); + } + #[tokio::test] async fn swap_theme_with_unknown_name_pushes_error_and_keeps_active_theme() { // Pins the defensive `else` arm — load_builtin returning None must not silently swallow @@ -2450,45 +2475,20 @@ mod tests { ); } - #[test] - fn draw_frame_surface_fill_overwrites_unpainted_cells_with_surface_bg() { - // Buffer-wide invariant: pre-stain every cell, render, and assert no sentinel survives. - // The frame-area surface fill is the only widget that guarantees this for cells no other - // widget covers (gaps that emerge with future layout or widget additions). - use ratatui::style::Color; - - let (mut app, _rx, _agent_tx) = test_app(None); + #[tokio::test] + async fn theme_actions_are_not_forwarded_to_agent_loop() { + // PreviewTheme / SwapTheme are TUI-only; reaching `user_tx` would race the client. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); app.dispatch_user_action(UserAction::SwapTheme { name: "latte".to_owned(), }); - let surface_bg = app - .theme - .surface() - .bg - .expect("latte sets explicit surface bg"); - let sentinel = Color::Rgb(254, 0, 254); - assert_ne!(surface_bg, sentinel, "sentinel must differ from theme bg"); - - let mut terminal = Terminal::new(TestBackend::new(60, 10)).unwrap(); - for cell in &mut terminal.current_buffer_mut().content { - cell.set_bg(sentinel); - } - terminal - .draw(|frame| { - _ = app.draw_frame(frame); - }) - .unwrap(); - - let buffer = terminal.backend().buffer(); - for y in 0..10 { - for x in 0..60 { - let cell = buffer.cell((x, y)).expect("cell in bounds"); - assert_ne!( - cell.bg, sentinel, - "cell ({x},{y}) kept the sentinel — surface fill regressed", - ); - } - } + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "theme actions must stay client-side", + ); } #[test] diff --git a/crates/oxide-code/src/tui/components/chat.rs b/crates/oxide-code/src/tui/components/chat.rs index 8f9463ec..eb1420a7 100644 --- a/crates/oxide-code/src/tui/components/chat.rs +++ b/crates/oxide-code/src/tui/components/chat.rs @@ -551,6 +551,40 @@ mod tests { terminal.backend().clone() } + // ── set_theme ── + + #[test] + fn set_theme_invalidates_streaming_cache_so_in_flight_tokens_repaint() { + // Bare `/theme` opens its picker mid-stream; without dropping the cache, committed + // prefix tokens would keep painting under the previous palette until commit_streaming. + let mut chat = test_chat(); + _ = chat.update_layout(Rect::new(0, 0, 80, 24)); + chat.append_stream_token("a complete paragraph\n\n"); + let s = chat.streaming.as_ref().unwrap(); + assert_ne!(s.rendered_len(), 0, "cache populated"); + assert_eq!(s.cached_width(), 80); + + chat.set_theme(&Theme::default()); + + let s = chat.streaming.as_ref().unwrap(); + assert_eq!(s.rendered_len(), 0, "cache dropped on theme swap"); + assert_eq!(s.rendered_boundary(), 0); + assert_eq!(s.cached_width(), 0); + } + + #[test] + fn set_theme_without_active_streaming_is_a_noop_on_cache_state() { + // The `if let Some(streaming)` guard skips the cache reset when no stream is in flight; + // pin both branches so the guard isn't accidentally inverted in a future refactor. + let mut chat = test_chat(); + assert!(chat.streaming.is_none()); + chat.set_theme(&Theme::default()); + assert!( + chat.streaming.is_none(), + "set_theme must not spawn a stream" + ); + } + // ── load_history ── #[test] @@ -2028,38 +2062,6 @@ mod tests { assert_eq!(chat.viewport_height, 20); } - #[test] - fn set_theme_invalidates_streaming_cache_so_in_flight_tokens_repaint() { - // Bare `/theme` opens its picker mid-stream; without dropping the cache, committed - // prefix tokens would keep painting under the previous palette until commit_streaming. - let mut chat = test_chat(); - _ = chat.update_layout(Rect::new(0, 0, 80, 24)); - chat.append_stream_token("a complete paragraph\n\n"); - let s = chat.streaming.as_ref().unwrap(); - assert_ne!(s.rendered_len(), 0, "cache populated"); - assert_eq!(s.cached_width(), 80); - - chat.set_theme(&Theme::default()); - - let s = chat.streaming.as_ref().unwrap(); - assert_eq!(s.rendered_len(), 0, "cache dropped on theme swap"); - assert_eq!(s.rendered_boundary(), 0); - assert_eq!(s.cached_width(), 0); - } - - #[test] - fn set_theme_without_active_streaming_is_a_noop_on_cache_state() { - // The `if let Some(streaming)` guard skips the cache reset when no stream is in flight; - // pin both branches so the guard isn't accidentally inverted in a future refactor. - let mut chat = test_chat(); - assert!(chat.streaming.is_none()); - chat.set_theme(&Theme::default()); - assert!( - chat.streaming.is_none(), - "set_theme must not spawn a stream" - ); - } - #[test] fn update_layout_invalidates_streaming_cache_on_width_change() { let mut chat = test_chat(); diff --git a/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs b/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs index 72c9dd1e..4983fb0d 100644 --- a/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs +++ b/crates/oxide-code/src/tui/components/chat/blocks/streaming.rs @@ -35,9 +35,7 @@ impl StreamingAssistant { } pub(crate) fn take_buffer(&mut self) -> String { - self.rendered.clear(); - self.rendered_boundary = 0; - self.cached_width = 0; + self.invalidate_cache(); std::mem::take(&mut self.buffer) } diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 519780fd..4a71528e 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -215,6 +215,22 @@ mod tests { assert_eq!(popup.selected, 0); } + // ── selected ── + + #[test] + fn selected_picks_match_at_index() { + let mut popup = popup_with_query(Some("")); + popup.select_next(); + let row = popup.selected().expect("popup visible"); + assert_eq!(row.name, popup.matches[1].name); + } + + #[test] + fn selected_is_none_when_hidden() { + let popup = popup_with_query(None); + assert!(popup.selected().is_none()); + } + // ── select_next / select_prev ── #[test] @@ -265,10 +281,11 @@ mod tests { assert_eq!(usize::from(popup.height()), expected); } - // ── scroll_offset ── + // ── render ── fn long_popup(n: usize) -> SlashPopup { - // Hand-rolled match list keeps the test independent of registry growth. + // Hand-rolled match list keeps the test independent of registry growth. Used by both the + // scroll-into-view render snapshot and the scroll_offset arithmetic suite below. let mut p = SlashPopup::new(&theme()); p.matches = (0..n) .map(|i| MatchedCommand { @@ -280,115 +297,6 @@ mod tests { p } - #[test] - fn scroll_offset_anchors_at_top_while_cursor_in_first_half() { - let mut p = long_popup(MAX_VISIBLE_ROWS + 4); - for _ in 0..MAX_VISIBLE_ROWS / 2 { - assert_eq!(p.scroll_offset(), 0); - p.select_next(); - } - } - - #[test] - fn scroll_offset_centers_cursor_past_first_half() { - // pad = MAX_VISIBLE_ROWS / 2 = 4 for the default cap. At selected = pad + k the offset is - // k, keeping the cursor visually at row `pad`. - let mut p = long_popup(MAX_VISIBLE_ROWS + 4); - for _ in 0..=(MAX_VISIBLE_ROWS / 2) { - p.select_next(); - } - assert_eq!(p.scroll_offset(), 1, "cursor at pad + 1 → offset = 1"); - } - - #[test] - fn scroll_offset_anchors_at_bottom_near_end() { - // Once selected nears the end, the offset clamps to `len - MAX_VISIBLE_ROWS` so the last - // row stays visible while the cursor advances within the bottom-anchored window. - let total = MAX_VISIBLE_ROWS + 4; - let mut p = long_popup(total); - while p.selected < total - 1 { - p.select_next(); - } - assert_eq!(p.scroll_offset(), total - MAX_VISIBLE_ROWS); - } - - #[test] - fn scroll_offset_resets_on_wrap_to_first_row() { - let total = MAX_VISIBLE_ROWS + 4; - let mut p = long_popup(total); - for _ in 0..total { - p.select_next(); - } - assert_eq!(p.selected, 0, "wrap to row 0"); - assert_eq!(p.scroll_offset(), 0, "wrap snaps the window back to top"); - } - - #[test] - fn scroll_offset_is_zero_when_total_fits_window() { - // With `MAX_VISIBLE_ROWS = 8` the live registry of 9 commands does scroll, but a - // 5-element fake list must never offset. - let mut p = long_popup(5); - p.select_next(); - p.select_next(); - assert_eq!(p.scroll_offset(), 0); - } - - #[test] - fn scroll_offset_keeps_visible_row_at_pad_for_mid_list_selection() { - // Pin the centering invariant directly: the visible row of the cursor (selected - offset) - // equals `pad` whenever the selection is past the top half but not yet near the bottom. - // Mutating the divisor (e.g. `/3`) or the formula would shift this row index. - let total = MAX_VISIBLE_ROWS + 4; - let mut p = long_popup(total); - let pad = MAX_VISIBLE_ROWS / 2; - for _ in 0..=(pad + 1) { - p.select_next(); - } - let visible_row = p.selected - p.scroll_offset(); - assert_eq!(visible_row, pad, "cursor must sit at the visual middle row"); - } - - #[test] - fn scroll_offset_at_exactly_cap_returns_zero_for_last_row() { - // Boundary: total == MAX_VISIBLE_ROWS hits the `<=` early-return. Mutating to `<` would - // try to compute max_offset = 0 and still produce 0 here, but tightening the boundary - // pins the invariant for the only case where the edge matters. - let mut p = long_popup(MAX_VISIBLE_ROWS); - while p.selected < MAX_VISIBLE_ROWS - 1 { - p.select_next(); - } - assert_eq!(p.scroll_offset(), 0); - } - - #[test] - fn scroll_offset_select_prev_from_top_anchors_at_bottom_window() { - // Up-arrow from row 0 wraps to the last row; the bottom-anchored window must clamp to - // `len - MAX_VISIBLE_ROWS` (the symmetric case to the existing wrap-to-top test). - let total = MAX_VISIBLE_ROWS + 4; - let mut p = long_popup(total); - p.select_prev(); - assert_eq!(p.selected, total - 1, "wrap to last row"); - assert_eq!(p.scroll_offset(), total - MAX_VISIBLE_ROWS); - } - - // ── selected ── - - #[test] - fn selected_picks_match_at_index() { - let mut popup = popup_with_query(Some("")); - popup.select_next(); - let row = popup.selected().expect("popup visible"); - assert_eq!(row.name, popup.matches[1].name); - } - - #[test] - fn selected_is_none_when_hidden() { - let popup = popup_with_query(None); - assert!(popup.selected().is_none()); - } - - // ── render ── - #[test] fn render_empty_query_shows_each_command_once() { let popup = popup_with_query(Some("")); @@ -483,4 +391,96 @@ mod tests { assert!(!rendered.contains("cmd0"), "cmd0 scrolled off: {rendered}"); assert!(rendered.contains("cmd8"), "cmd8 in window: {rendered}"); } + + // ── scroll_offset ── + + #[test] + fn scroll_offset_is_zero_when_total_fits_window() { + // With `MAX_VISIBLE_ROWS = 8` the live registry of 9 commands does scroll, but a + // 5-element fake list must never offset. + let mut p = long_popup(5); + p.select_next(); + p.select_next(); + assert_eq!(p.scroll_offset(), 0); + } + + #[test] + fn scroll_offset_at_exactly_cap_returns_zero_for_last_row() { + // Boundary: total == MAX_VISIBLE_ROWS hits the `<=` early-return. Tightening the boundary + // pins the invariant for the only case where the edge matters. + let mut p = long_popup(MAX_VISIBLE_ROWS); + while p.selected < MAX_VISIBLE_ROWS - 1 { + p.select_next(); + } + assert_eq!(p.scroll_offset(), 0); + } + + #[test] + fn scroll_offset_anchors_at_top_while_cursor_in_first_half() { + let mut p = long_popup(MAX_VISIBLE_ROWS + 4); + for _ in 0..MAX_VISIBLE_ROWS / 2 { + assert_eq!(p.scroll_offset(), 0); + p.select_next(); + } + } + + #[test] + fn scroll_offset_centers_cursor_past_first_half() { + // pad = MAX_VISIBLE_ROWS / 2 = 4 for the default cap. At selected = pad + k the offset is + // k, keeping the cursor visually at row `pad`. + let mut p = long_popup(MAX_VISIBLE_ROWS + 4); + for _ in 0..=(MAX_VISIBLE_ROWS / 2) { + p.select_next(); + } + assert_eq!(p.scroll_offset(), 1, "cursor at pad + 1 → offset = 1"); + } + + #[test] + fn scroll_offset_keeps_visible_row_at_pad_for_mid_list_selection() { + // Pin the centering invariant directly: the visible row of the cursor (selected - offset) + // equals `pad` whenever the selection is past the top half but not yet near the bottom. + // Mutating the divisor (e.g. `/3`) or the formula would shift this row index. + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + let pad = MAX_VISIBLE_ROWS / 2; + for _ in 0..=(pad + 1) { + p.select_next(); + } + let visible_row = p.selected - p.scroll_offset(); + assert_eq!(visible_row, pad, "cursor must sit at the visual middle row"); + } + + #[test] + fn scroll_offset_anchors_at_bottom_near_end() { + // Once selected nears the end, the offset clamps to `len - MAX_VISIBLE_ROWS` so the last + // row stays visible while the cursor advances within the bottom-anchored window. + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + while p.selected < total - 1 { + p.select_next(); + } + assert_eq!(p.scroll_offset(), total - MAX_VISIBLE_ROWS); + } + + #[test] + fn scroll_offset_resets_on_wrap_to_first_row() { + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + for _ in 0..total { + p.select_next(); + } + assert_eq!(p.selected, 0, "wrap to row 0"); + assert_eq!(p.scroll_offset(), 0, "wrap snaps the window back to top"); + } + + #[test] + fn scroll_offset_select_prev_from_top_anchors_at_bottom_window() { + // Up-arrow from row 0 wraps to the last row; the bottom-anchored window must clamp to + // `len - MAX_VISIBLE_ROWS` (the symmetric case to the wrap-to-top test above). + let total = MAX_VISIBLE_ROWS + 4; + let mut p = long_popup(total); + p.select_prev(); + assert_eq!(p.selected, total - 1, "wrap to last row"); + assert_eq!(p.scroll_offset(), total - MAX_VISIBLE_ROWS); + } } From 554cb7e7de81d8e171c040916b8a6bc7c6bbd207 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 21:11:43 +0800 Subject: [PATCH 12/15] feat(tui): latte / material default to terminal background MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mocha / macchiato / frappe already inherit the terminal bg via `surface = { bg = "reset" }`. Light and Material were the outliers forcing an explicit fill — apply the same convention so every built-in theme adapts to the user's terminal palette. The surface-fill regression test no longer needs to swap to latte; default theme is now sufficient since all built-ins use Reset. --- crates/oxide-code/src/tui/app.rs | 13 ++----------- crates/oxide-code/themes/latte.toml | 5 +---- crates/oxide-code/themes/material.toml | 4 +--- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 984b6a7c..2a63e311 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -2313,21 +2313,12 @@ mod tests { #[test] fn draw_frame_surface_fill_overwrites_unpainted_cells_with_surface_bg() { // Buffer-wide invariant: pre-stain every cell, render, and assert no sentinel survives. - // The frame-area surface fill is the only widget that guarantees this for cells no other - // widget covers (gaps that emerge with future layout or widget additions). + // The frame-area surface fill is the only widget that guarantees this for cells no + // other widget covers. use ratatui::style::Color; let (mut app, _rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::SwapTheme { - name: "latte".to_owned(), - }); - let surface_bg = app - .theme - .surface() - .bg - .expect("latte sets explicit surface bg"); let sentinel = Color::Rgb(254, 0, 254); - assert_ne!(surface_bg, sentinel, "sentinel must differ from theme bg"); let mut terminal = Terminal::new(TestBackend::new(60, 10)).unwrap(); for cell in &mut terminal.current_buffer_mut().content { diff --git a/crates/oxide-code/themes/latte.toml b/crates/oxide-code/themes/latte.toml index 328d1e76..f67ce7ce 100644 --- a/crates/oxide-code/themes/latte.toml +++ b/crates/oxide-code/themes/latte.toml @@ -7,10 +7,7 @@ muted = "#9ca0b0" dim = "#acb0be" # Surfaces -# Latte ships an opaque base bg (Catppuccin Latte `base`) so dark text -# is readable on terminals whose own bg is dark. Override to "reset" -# on transparent / light terminals to let the terminal bg show through. -surface = { bg = "#eff1f5" } +surface = { bg = "reset" } # Semantic accents accent = { fg = "#1e66f5", bold = true } diff --git a/crates/oxide-code/themes/material.toml b/crates/oxide-code/themes/material.toml index 6cf5e8e6..f45741cc 100644 --- a/crates/oxide-code/themes/material.toml +++ b/crates/oxide-code/themes/material.toml @@ -11,9 +11,7 @@ muted = "#999999" dim = "#616161" # Surfaces -# M2 dark theme baseline surface (elevation 0). Override to "reset" -# to inherit the terminal background instead. -surface = { bg = "#121212" } +surface = { bg = "reset" } # Semantic accents (M2 dark theme baseline primary, Material 200-tone) accent = { fg = "#bb86fc", bold = true } From 001d07ecea53fa5a9d30aee9b16bbf0d923f7194 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 21:12:02 +0800 Subject: [PATCH 13/15] feat(tui): hide input + popup while a modal is open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modals own focus and intercept every key, so the input row + slash popup are unreachable while one is on the stack. Collapse both layout slots so the chat reclaims that vertical space — parallels Claude Code's modal UX. Adds a layout-level test using a `FakeModal` so future modals inherit the behavior automatically without per-modal opt-in. --- crates/oxide-code/src/tui/app.rs | 79 +++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 2a63e311..a7bbc60f 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -547,10 +547,16 @@ impl App { /// Returns the chat area so the scroll cache can refresh its layout. fn draw_frame(&mut self, frame: &mut ratatui::Frame<'_>) -> ratatui::layout::Rect { - let input_height = self.input.height(); let preview_height = self.preview_height(); - let popup_height = self.input.popup_height(); let modal_height = self.modals.height(frame.area().width); + // Modal owns focus — input + popup are unreachable, so collapse them. + let modal_active = modal_height > 0; + let popup_height = if modal_active { + 0 + } else { + self.input.popup_height() + }; + let input_height = if modal_active { 0 } else { self.input.height() }; // Pre-fill with surface bg so unpainted gaps inherit the theme. frame.render_widget( ratatui::widgets::Block::default().style(self.theme.surface()), @@ -571,13 +577,14 @@ impl App { if preview_height > 0 { self.render_preview(frame, chunks[2]); } - if modal_height > 0 { + if modal_active { self.modals.render(frame, chunks[3], &self.theme); + } else { + if popup_height > 0 { + self.input.render_popup(frame, chunks[4]); + } + self.input.render(frame, chunks[5]); } - if popup_height > 0 { - self.input.render_popup(frame, chunks[4]); - } - self.input.render(frame, chunks[5]); chunks[1] } @@ -743,6 +750,40 @@ mod tests { } } + /// Minimal modal for layout tests: paints `title` on its only row, ignores keys. + struct FakeModal { + title: String, + } + + impl FakeModal { + fn new(title: &str) -> Self { + Self { + title: title.to_owned(), + } + } + } + + impl crate::tui::modal::Modal for FakeModal { + fn height(&self, _width: u16) -> u16 { + 1 + } + + fn render( + &self, + frame: &mut ratatui::Frame<'_>, + area: Rect, + theme: &crate::tui::theme::Theme, + ) { + use ratatui::widgets::Paragraph; + let line = Line::from(Span::styled(self.title.clone(), theme.text())); + frame.render_widget(Paragraph::new(line).style(theme.surface()), area); + } + + fn handle_key(&mut self, _event: &KeyEvent) -> crate::tui::modal::ModalKey { + crate::tui::modal::ModalKey::Consumed + } + } + #[derive(Clone)] struct SharedWriter(Arc>>); @@ -2310,6 +2351,30 @@ mod tests { insta::assert_snapshot!(render_app(&mut app, 60, 14)); } + #[test] + fn draw_frame_hides_input_and_popup_while_modal_active() { + // Modal owns focus, so the layout must collapse the input + popup bands. The idle + // placeholder is the cheapest substring proof that the input got rendered. + let (mut app, _rx, _agent_tx) = test_app(None); + let baseline = rendered_text(&mut app, 60, 14); + assert!( + baseline.contains("Ask anything..."), + "input placeholder must paint without a modal: {baseline}", + ); + + app.modals + .push(Box::new(FakeModal::new("FAKE-MODAL-TITLE"))); + let with_modal = rendered_text(&mut app, 60, 14); + assert!( + with_modal.contains("FAKE-MODAL-TITLE"), + "modal body must render: {with_modal}", + ); + assert!( + !with_modal.contains("Ask anything..."), + "input must collapse while modal is active: {with_modal}", + ); + } + #[test] fn draw_frame_surface_fill_overwrites_unpainted_cells_with_surface_bg() { // Buffer-wide invariant: pre-stain every cell, render, and assert no sentinel survives. From 5011e58354aba441fa45bfddc5a4576499dd514c Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 21:52:48 +0800 Subject: [PATCH 14/15] =?UTF-8?q?refactor:=20post-review=20sweep=20?= =?UTF-8?q?=E2=80=94=20drop=20defensive=20arms,=20trim=20comments,=20harde?= =?UTF-8?q?n=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the `SwapTheme` chat-error path (`tui/app.rs`): the slash form already validates names against `LISTED_THEMES`, so the only path where `load_builtin` returns None is roster drift between slash and builtin tables — a developer bug, not user error. Mirror `PreviewTheme`'s `tracing::warn!` and rename the test to `swap_theme_with_unknown_name_is_silent_noop`. Move the `apply_action_locally / theme` test section to sit between `dispatch_user_action` and `handle_agent_event` so the section header order mirrors the production-function order (CLAUDE.md testing convention). Trim multi-line comments flagged in review: - `apply_modal_action` rollback comment. - `apply_theme` doc. - `handle_submit_prompt` synthesized-action arm. - Queued-prompt forward gate. - `resolve_theme_arg` doc + a few test preambles. Harden tests: - `draw_frame_surface_fill` asserts `cell.bg == surface_bg` (mutation that paints any non-magenta color used to pass). - `numeric_jump_past_roster_is_a_consumed_noop` derives the digit from `LISTED_THEMES.len()` so it survives roster growth. - New `modal_cancel_without_snapshot_is_noop` pins the rollback arm's no-snapshot path. - `assert!(... == "latte")` → `assert_eq!` for better failure output. --- crates/oxide-code/src/slash/theme.rs | 27 +- crates/oxide-code/src/tui/app.rs | 342 +++++++++--------- .../src/tui/components/input/popup.rs | 6 +- 3 files changed, 188 insertions(+), 187 deletions(-) diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index 6b023d5a..f7ccd8b8 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -115,7 +115,7 @@ impl ThemePicker { impl Modal for ThemePicker { fn height(&self, width: u16) -> u16 { - // +2: blank spacer row, then the footer hint row. + // Reserve one blank spacer + one footer line below the list. self.list.height(width).saturating_add(2) } @@ -204,8 +204,8 @@ impl SlashCommand for ThemeCmd { } } -/// Custom file paths are intentionally rejected here; users with a custom TOML edit -/// `~/.config/ox/config.toml` directly. +/// Resolve a typed `/theme ` argument against the curated catalogue. Custom file paths +/// are rejected — users with a custom TOML edit `~/.config/ox/config.toml` directly. fn resolve_theme_arg(arg: &str) -> Result<&'static str, String> { let lower = arg.to_ascii_lowercase(); LISTED_THEMES @@ -258,9 +258,6 @@ mod tests { #[test] fn theme_row_picker_item_methods_return_curated_values() { - // Pins the trait-impl surface for ThemeRow — label / description / is_active / key_hint - // each have a path the picker depends on and only the render smoke test exercises - // indirectly. let rows = ThemeRow::build("latte"); let names: Vec<&str> = rows.iter().map(PickerItem::label).collect(); assert_eq!( @@ -270,7 +267,10 @@ mod tests { let active_count = rows.iter().filter(|r| r.is_active()).count(); assert_eq!(active_count, 1, "exactly one row marks the active theme"); - assert!(rows.iter().find(|r| r.is_active()).unwrap().label() == "latte"); + assert_eq!( + rows.iter().find(|r| r.is_active()).unwrap().label(), + "latte" + ); for (idx, row) in rows.iter().enumerate() { assert_eq!(row.key_hint(), numeric_hint(idx), "idx={idx}"); @@ -301,8 +301,7 @@ mod tests { #[test] fn new_with_unknown_active_keeps_cursor_on_first_row() { - // User-provided custom file path won't match any built-in row; cursor falls back to the - // first listed theme rather than panicking or silently picking nothing. + // Custom file path doesn't match any built-in row; cursor falls back to the first. let p = picker_with_active("~/themes/custom.toml"); let row = p.list.selected().expect("first row"); assert_eq!(row.name, "mocha"); @@ -359,11 +358,13 @@ mod tests { } #[test] - fn numeric_jump_unknown_digit_is_a_consumed_noop() { - // Only digits 1..=N (curated count) jump; out-of-range digits leave cursor and emit no - // preview, so the App doesn't repaint for nothing. + fn numeric_jump_past_roster_is_a_consumed_noop() { + // Out-of-range digits stay Consumed so the App doesn't repaint for nothing. Use the + // first digit past the curated count so the test survives roster growth. + let past_end = char::from_digit(u32::try_from(LISTED_THEMES.len()).unwrap() + 1, 10) + .expect("roster fits in a single digit"); let mut p = picker_with_active("mocha"); - let outcome = p.handle_key(&key(KeyCode::Char('9'))); + let outcome = p.handle_key(&key(KeyCode::Char(past_end))); assert!(matches!(outcome, ModalKey::Consumed)); } diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index a7bbc60f..c3ce0cfd 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -206,8 +206,7 @@ impl App { fn apply_modal_action(&mut self, action: ModalAction) { match action { - // Modal cancelled or silently closed: undo any in-flight theme preview so the user - // returns to exactly the state before opening the picker. + // Cancel: revert any in-flight theme preview to the snapshot taken on open. ModalAction::None => { if let Some(theme) = self.preview_theme_snapshot.take() { self.apply_theme(&theme); @@ -217,7 +216,7 @@ impl App { } } - /// Sole entry point for mid-session theme swaps; repaints every theme-styled component. + /// Repaints every theme-styled component for a mid-session theme swap. fn apply_theme(&mut self, theme: &Theme) { self.theme = theme.clone(); self.chat.set_theme(theme); @@ -317,8 +316,7 @@ impl App { self.chat .push_system_message(format!("Theme set to {name}.")); } else { - self.chat - .push_error(&format!("unknown theme: `{name}` (this is a bug)")); + tracing::warn!(name, "SwapTheme: unknown built-in; picker roster drift"); } false } @@ -346,8 +344,8 @@ impl App { self.status_bar.set_status(Status::Streaming); self.forward_to_agent(action); } else { - // TUI-only synthesized actions (e.g. SwapTheme) need apply_action_locally — - // forwarding to the agent drops them. + // TUI-only synthesized actions (e.g. SwapTheme) flow through dispatch + // so the local arm runs; forwarding to the agent would drop them. self.dispatch_user_action(action); } } @@ -382,9 +380,7 @@ impl App { } self.pending_prompts.push_back(text.to_owned()); self.sync_input_queue_hint(); - // Forward the queued prompt to the agent so it can drain at turn end — unless we're - // already cancelling, in which case the agent will reset and the queue drains locally on - // the next idle transition. + // Skip forwarding while cancelling — the agent resets and we drain locally on idle. !matches!(self.status_bar.status(), Status::Cancelling) } @@ -1546,6 +1542,169 @@ mod tests { assert!(app.chat.last_is_error()); } + // ── apply_action_locally / theme ── + + #[test] + fn preview_theme_repaints_components_and_caches_original() { + let (mut app, _rx, _agent_tx) = test_app(None); + let original = app.theme.clone(); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + assert!( + app.preview_theme_snapshot.is_some(), + "first PreviewTheme must cache the original", + ); + assert_ne!(app.theme.text, original.text, "live theme must change"); + // preview leaves session_info.config.theme_name as-is until commit. + assert_eq!(app.session_info.config.theme_name, "mocha"); + } + + #[test] + fn preview_theme_then_modal_cancel_restores_original() { + // Esc / Ctrl+C surface as ModalAction::None; the snapshot must roll back. + let (mut app, _rx, _agent_tx) = test_app(None); + let original = app.theme.clone(); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + app.apply_modal_action(ModalAction::None); + assert!( + app.preview_theme_snapshot.is_none(), + "snapshot consumed on cancel", + ); + assert_eq!(app.theme.text, original.text, "theme rolled back to mocha"); + } + + #[test] + fn modal_cancel_without_snapshot_is_noop() { + // Modals that never previewed (e.g. /model picker) cancel through the same + // ModalAction::None path; the rollback arm must skip cleanly with no snapshot. + let (mut app, _rx, _agent_tx) = test_app(None); + let original_text = app.theme.text; + app.apply_modal_action(ModalAction::None); + assert!(app.preview_theme_snapshot.is_none()); + assert_eq!(app.theme.text, original_text); + } + + #[test] + fn preview_theme_with_unknown_name_does_nothing_and_keeps_active_theme() { + // Roster drift between slash + builtin tables logs via tracing only — user state stays put. + let (mut app, _rx, _agent_tx) = test_app(None); + let original_text = app.theme.text; + let entries_before = app.chat.entry_count(); + + app.dispatch_user_action(UserAction::PreviewTheme { + name: "nonexistent".to_owned(), + }); + + assert!( + app.preview_theme_snapshot.is_none(), + "no snapshot when load_builtin returns None", + ); + assert_eq!(app.theme.text, original_text, "theme stays put on drift"); + assert_eq!( + app.chat.entry_count(), + entries_before, + "preview must never push a chat block", + ); + } + + #[test] + fn swap_theme_commits_and_clears_snapshot() { + let (mut app, _rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + let previewed = app.theme.clone(); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + assert!(app.preview_theme_snapshot.is_none()); + assert_eq!(app.session_info.config.theme_name, "latte"); + assert_eq!(app.theme.text, previewed.text); + + app.apply_modal_action(ModalAction::None); + assert_eq!( + app.session_info.config.theme_name, "latte", + "post-commit cancel must not restore the pre-preview theme", + ); + } + + #[tokio::test] + async fn swap_theme_with_unknown_name_is_silent_noop() { + // Roster drift between slash::theme::LISTED_THEMES and tui::theme::builtin::TABLE is a + // dev bug, not user error — log via tracing and leave UI state untouched. + let (mut app, _rx, _agent_tx) = test_app(None); + let original = app.session_info.config.theme_name.clone(); + let entries_before = app.chat.entry_count(); + + app.dispatch_user_action(UserAction::SwapTheme { + name: "nonexistent".to_owned(), + }); + + assert_eq!(app.session_info.config.theme_name, original); + assert_eq!( + app.chat.entry_count(), + entries_before, + "no chat block — drift is dev-only", + ); + } + + #[tokio::test] + async fn theme_actions_are_not_forwarded_to_agent_loop() { + // PreviewTheme / SwapTheme are TUI-only; reaching `user_tx` would race the client. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::PreviewTheme { + name: "latte".to_owned(), + }); + app.dispatch_user_action(UserAction::SwapTheme { + name: "latte".to_owned(), + }); + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "theme actions must stay client-side", + ); + } + + #[test] + fn slash_typed_swap_theme_routes_through_local_handler() { + // Regression: synthesized non-SubmitPrompt actions (e.g. `/theme latte`) used to be + // forwarded straight to the agent, which silently dropped them. They must now flow + // through dispatch_user_action so the local theme arm runs. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SubmitPrompt("/theme latte".to_owned())); + + assert_eq!( + app.session_info.config.theme_name, "latte", + "typed `/theme ` must mutate session theme", + ); + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "TUI-only theme swap must not leak to the agent channel", + ); + } + + #[tokio::test] + async fn slash_typed_model_routes_synthesized_swap_config_through_dispatch() { + // Mirrors the typed-`/theme` regression on the agent-bound side: `/model ` synthesizes + // a SwapConfig that must reach the agent via dispatch_user_action → forward_to_agent. + let (mut app, mut rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SubmitPrompt("/model haiku".to_owned())); + + let forwarded = rx.recv().await.expect("SwapConfig forwarded to agent"); + match forwarded { + UserAction::SwapConfig { model, effort } => { + assert_eq!( + model.as_ref().map(crate::model::ResolvedModelId::as_str), + Some("claude-haiku-4-5") + ); + assert_eq!(effort, None); + } + other => panic!("expected SwapConfig, got {other:?}"), + } + } + // ── handle_agent_event ── #[test] @@ -2384,6 +2543,7 @@ mod tests { let (mut app, _rx, _agent_tx) = test_app(None); let sentinel = Color::Rgb(254, 0, 254); + let surface_bg = app.theme.surface().bg.expect("surface slot defines bg"); let mut terminal = Terminal::new(TestBackend::new(60, 10)).unwrap(); for cell in &mut terminal.current_buffer_mut().content { @@ -2399,8 +2559,8 @@ mod tests { for y in 0..10 { for x in 0..60 { let cell = buffer.cell((x, y)).expect("cell in bounds"); - assert_ne!( - cell.bg, sentinel, + assert_eq!( + cell.bg, surface_bg, "cell ({x},{y}) kept the sentinel — surface fill regressed", ); } @@ -2427,164 +2587,6 @@ mod tests { ); } - // ── apply_action_locally / theme ── - - #[test] - fn preview_theme_repaints_components_and_caches_original() { - let (mut app, _rx, _agent_tx) = test_app(None); - let original = app.theme.clone(); - app.dispatch_user_action(UserAction::PreviewTheme { - name: "latte".to_owned(), - }); - assert!( - app.preview_theme_snapshot.is_some(), - "first PreviewTheme must cache the original", - ); - assert_ne!(app.theme.text, original.text, "live theme must change"); - // preview leaves session_info.config.theme_name as-is until commit. - assert_eq!(app.session_info.config.theme_name, "mocha"); - } - - #[test] - fn preview_theme_then_modal_cancel_restores_original() { - // Esc / Ctrl+C surface as ModalAction::None; the snapshot must roll back. - let (mut app, _rx, _agent_tx) = test_app(None); - let original = app.theme.clone(); - app.dispatch_user_action(UserAction::PreviewTheme { - name: "latte".to_owned(), - }); - app.apply_modal_action(ModalAction::None); - assert!( - app.preview_theme_snapshot.is_none(), - "snapshot consumed on cancel", - ); - assert_eq!(app.theme.text, original.text, "theme rolled back to mocha"); - } - - #[test] - fn preview_theme_with_unknown_name_does_nothing_and_keeps_active_theme() { - // The drift warn arm fires when a PreviewTheme name doesn't resolve. The picker roster - // and the loader's lookup table must agree, so this is a developer-facing log only; - // user-visible state stays put (no snapshot, no theme swap, no chat block). - let (mut app, _rx, _agent_tx) = test_app(None); - let original_text = app.theme.text; - let entries_before = app.chat.entry_count(); - - app.dispatch_user_action(UserAction::PreviewTheme { - name: "nonexistent".to_owned(), - }); - - assert!( - app.preview_theme_snapshot.is_none(), - "no snapshot when load_builtin returns None", - ); - assert_eq!(app.theme.text, original_text, "theme stays put on drift"); - assert_eq!( - app.chat.entry_count(), - entries_before, - "preview must never push a chat block", - ); - } - - #[test] - fn swap_theme_commits_and_clears_snapshot() { - let (mut app, _rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::PreviewTheme { - name: "latte".to_owned(), - }); - let previewed = app.theme.clone(); - app.dispatch_user_action(UserAction::SwapTheme { - name: "latte".to_owned(), - }); - assert!(app.preview_theme_snapshot.is_none()); - assert_eq!(app.session_info.config.theme_name, "latte"); - assert_eq!(app.theme.text, previewed.text); - - app.apply_modal_action(ModalAction::None); - assert_eq!( - app.session_info.config.theme_name, "latte", - "post-commit cancel must not restore the pre-preview theme", - ); - } - - #[tokio::test] - async fn swap_theme_with_unknown_name_pushes_error_and_keeps_active_theme() { - // Pins the defensive `else` arm — load_builtin returning None must not silently swallow - // the request. Today the slash form pre-validates names, but the arm guards against any - // future caller passing through unchecked. - let (mut app, _rx, _agent_tx) = test_app(None); - let original = app.session_info.config.theme_name.clone(); - let entries_before = app.chat.entry_count(); - - app.dispatch_user_action(UserAction::SwapTheme { - name: "nonexistent".to_owned(), - }); - - assert_eq!( - app.session_info.config.theme_name, original, - "unknown name must not flip the active theme", - ); - assert_eq!( - app.chat.entry_count(), - entries_before + 1, - "error must surface as a chat block", - ); - } - - #[tokio::test] - async fn theme_actions_are_not_forwarded_to_agent_loop() { - // PreviewTheme / SwapTheme are TUI-only; reaching `user_tx` would race the client. - let (mut app, mut rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::PreviewTheme { - name: "latte".to_owned(), - }); - app.dispatch_user_action(UserAction::SwapTheme { - name: "latte".to_owned(), - }); - assert!( - matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), - "theme actions must stay client-side", - ); - } - - #[test] - fn slash_typed_swap_theme_routes_through_local_handler() { - // Regression: synthesized non-SubmitPrompt actions (e.g. `/theme latte`) used to be - // forwarded straight to the agent, which silently dropped them. They must now flow - // through dispatch_user_action so the local theme arm runs. - let (mut app, mut rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::SubmitPrompt("/theme latte".to_owned())); - - assert_eq!( - app.session_info.config.theme_name, "latte", - "typed `/theme ` must mutate session theme", - ); - assert!( - matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), - "TUI-only theme swap must not leak to the agent channel", - ); - } - - #[tokio::test] - async fn slash_typed_model_routes_synthesized_swap_config_through_dispatch() { - // Mirrors the typed-`/theme` regression on the agent-bound side: `/model ` synthesizes - // a SwapConfig that must reach the agent via dispatch_user_action → forward_to_agent. - let (mut app, mut rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::SubmitPrompt("/model haiku".to_owned())); - - let forwarded = rx.recv().await.expect("SwapConfig forwarded to agent"); - match forwarded { - UserAction::SwapConfig { model, effort } => { - assert_eq!( - model.as_ref().map(crate::model::ResolvedModelId::as_str), - Some("claude-haiku-4-5") - ); - assert_eq!(effort, None); - } - other => panic!("expected SwapConfig, got {other:?}"), - } - } - // ── render_preview ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 4a71528e..5f7369b9 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -284,8 +284,7 @@ mod tests { // ── render ── fn long_popup(n: usize) -> SlashPopup { - // Hand-rolled match list keeps the test independent of registry growth. Used by both the - // scroll-into-view render snapshot and the scroll_offset arithmetic suite below. + // Hand-rolled list keeps the test independent of registry growth. let mut p = SlashPopup::new(&theme()); p.matches = (0..n) .map(|i| MatchedCommand { @@ -396,8 +395,7 @@ mod tests { #[test] fn scroll_offset_is_zero_when_total_fits_window() { - // With `MAX_VISIBLE_ROWS = 8` the live registry of 9 commands does scroll, but a - // 5-element fake list must never offset. + // total < cap → no scroll regardless of cursor. let mut p = long_popup(5); p.select_next(); p.select_next(); From b682091e76ecb21c58312c86bc58cd56eb42db9a Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 22:11:46 +0800 Subject: [PATCH 15/15] test(tui): close real-path codecov gaps + extract handle_submit_prompt section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `submit_slash_theme_pushes_picker_onto_modal_stack` covering the `handle_submit_prompt` modal-push branch (`tui/app.rs:339`), and extend `draw_frame_hides_input_and_popup_while_modal_active` with a key send so `FakeModal::handle_key` (the test fixture) is exercised. Move the two `slash_typed_*` regression tests out of `apply_action_locally / theme` into a new `// ── handle_submit_prompt ──` section. Production order is `apply_action_locally` (278) → `handle_submit_prompt` (329) → `handle_agent_event` (391); test sections now mirror that. Patch coverage rises to 99.21% — the remaining 6 missing lines in `slash/theme.rs` are line-attribution artifacts (defensive `None` arms shared with `slash/picker.rs`, plus `match { other => panic!(...) }` test-failure idioms). --- crates/oxide-code/src/tui/app.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index c3ce0cfd..7bc99fb4 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -1667,6 +1667,18 @@ mod tests { ); } + // ── handle_submit_prompt ── + + #[test] + fn submit_slash_theme_pushes_picker_onto_modal_stack() { + // Bare `/theme` is the ReadOnly modal-opening branch — `slash::dispatch` populates the + // SlashContext modal slot, then handle_submit_prompt drains it onto `App::modals`. + let (mut app, _rx, _agent_tx) = test_app(None); + assert!(!app.modals.is_active(), "stack starts empty"); + app.dispatch_user_action(UserAction::SubmitPrompt("/theme".to_owned())); + assert!(app.modals.is_active(), "bare `/theme` opens a picker modal"); + } + #[test] fn slash_typed_swap_theme_routes_through_local_handler() { // Regression: synthesized non-SubmitPrompt actions (e.g. `/theme latte`) used to be @@ -2532,6 +2544,14 @@ mod tests { !with_modal.contains("Ask anything..."), "input must collapse while modal is active: {with_modal}", ); + + // Non-cancel keys land at the modal's handle_key — proves the focus-grab is wired up + // and the layout collapse doesn't bypass it. + let action = app + .modals + .handle_key(&KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + assert!(action.is_none(), "FakeModal consumes without emitting"); + assert!(app.modals.is_active(), "Consumed must not pop the stack"); } #[test]