From ad197492cffa38cd00336b0064de221a3797aecc Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 23:12:31 +0800 Subject: [PATCH 01/25] fix(slash): hide family-base rows from /model ambiguity listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Substring-typing `/model claude-opus` used to list `claude-opus-4` alongside the four current Opus rows. The base only exists so `lookup` can give legacy / dated ids a marketing name — never something a user would knowingly pick. Mark it on `ModelInfo`, filter the listing. --- crates/oxide-code/src/model.rs | 19 +++++++++++++ crates/oxide-code/src/slash/model.rs | 40 ++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index c42f0f41..519b12bc 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -11,6 +11,8 @@ use crate::config::Effort; pub(crate) struct ModelInfo { /// First substring match in [`MODELS`] wins; ordering matters. pub(crate) id_substr: &'static str, + /// Unqualified fallback row that resolves dated / legacy ids; user-facing listings hide it. + pub(crate) is_family_base: bool, pub(crate) marketing: &'static str, pub(crate) cutoff: Option<&'static str>, pub(crate) capabilities: Capabilities, @@ -49,6 +51,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ // Inherits 4.6 as a monotonic projection; `effort_xhigh` is the 4.7-only addition. ModelInfo { id_substr: "claude-opus-4-7", + is_family_base: false, marketing: "Claude Opus 4.7", cutoff: Some("January 2026"), capabilities: Capabilities { @@ -63,6 +66,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-opus-4-6", + is_family_base: false, marketing: "Claude Opus 4.6", cutoff: Some("May 2025"), capabilities: Capabilities { @@ -77,6 +81,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-sonnet-4-6", + is_family_base: false, marketing: "Claude Sonnet 4.6", cutoff: Some("August 2025"), capabilities: Capabilities { @@ -92,6 +97,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-opus-4-5", + is_family_base: false, marketing: "Claude Opus 4.5", cutoff: Some("May 2025"), capabilities: Capabilities { @@ -106,6 +112,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-sonnet-4-5", + is_family_base: false, marketing: "Claude Sonnet 4.5", cutoff: Some("January 2025"), capabilities: Capabilities { @@ -120,6 +127,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-haiku-4-5", + is_family_base: false, marketing: "Claude Haiku 4.5", cutoff: Some("February 2025"), capabilities: Capabilities { @@ -135,6 +143,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-opus-4-1", + is_family_base: false, marketing: "Claude Opus 4.1", cutoff: Some("January 2025"), capabilities: Capabilities { @@ -150,6 +159,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ // Unqualified base (`claude-opus-4`, `-4-0`, `-4-20250514`); structured outputs arrived in 4.1. ModelInfo { id_substr: "claude-opus-4", + is_family_base: true, marketing: "Claude Opus 4", cutoff: Some("January 2025"), capabilities: Capabilities { @@ -165,6 +175,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ // Sonnet 4 base: all Sonnet 4.x carry 1M per upstream's `sonnet-4` substring rule. ModelInfo { id_substr: "claude-sonnet-4", + is_family_base: true, marketing: "Claude Sonnet 4", cutoff: Some("January 2025"), capabilities: Capabilities { @@ -179,6 +190,7 @@ pub(crate) const MODELS: &[ModelInfo] = &[ }, ModelInfo { id_substr: "claude-haiku-4", + is_family_base: true, marketing: "Claude Haiku 4", cutoff: Some("February 2025"), capabilities: Capabilities { @@ -270,6 +282,13 @@ pub(crate) fn lookup(model: &str) -> Option<&'static ModelInfo> { MODELS.iter().find(|info| model.contains(info.id_substr)) } +/// True when `id` matches a family-base row exactly. User-facing listings filter against this. +pub(crate) fn is_family_base(id: &str) -> bool { + MODELS + .iter() + .any(|info| info.id_substr == id && info.is_family_base) +} + /// Capabilities for `model`, falling back to [`Capabilities::default`] for unknown ids. pub(crate) fn capabilities_for(model: &str) -> Capabilities { lookup(model) diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 723cd2b2..a164c256 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -4,7 +4,7 @@ use super::context::SlashContext; use super::registry::{SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; -use crate::model::{MODELS, ResolvedModelId, lookup}; +use crate::model::{MODELS, ResolvedModelId, is_family_base, lookup}; // ── Constants ── @@ -95,13 +95,18 @@ fn resolve_base(arg: &str) -> Result { if let [id] = candidates(|id| id.ends_with(arg)).as_slice() { return Ok((*id).to_owned()); } - let matches = candidates(|id| id.contains(arg)); - match matches.as_slice() { + // Family bases are listed only for `lookup` (marketing names of dated ids); listing them + // here would invite users to type a deprecated row. + let visible: Vec<&'static str> = candidates(|id| id.contains(arg)) + .into_iter() + .filter(|id| !is_family_base(id)) + .collect(); + match visible.as_slice() { [id] => Ok((*id).to_owned()), [_, ..] => Err(format!( "`{arg}` matches {n} models: {list}. Type a more specific id or use a short alias (`opus`, `sonnet`, `haiku`).", - n = matches.len(), - list = matches.join(", "), + n = visible.len(), + list = visible.join(", "), )), [] => Err(format!( "Unknown model: `{arg}`. Run `/model` for selectable shortcuts; \ @@ -298,6 +303,31 @@ mod tests { assert!(msg.contains("opus"), "alias hint surfaces: {msg}"); } + #[test] + fn execute_ambiguous_listing_omits_family_base_rows() { + // `claude-opus` substring-matches every Opus row, including the deprecated + // `claude-opus-4` base. Listing the base would invite a user to type a superseded id. + let (_, outcome) = run_execute("claude-opus"); + let msg = outcome.expect_err("ambiguous arg must error"); + for current in ["claude-opus-4-7", "claude-opus-4-6", "claude-opus-4-1"] { + assert!( + msg.contains(current), + "current row `{current}` listed: {msg}" + ); + } + // Bound the search to comma-delimited tokens so `claude-opus-4` doesn't false-match + // on the longer `claude-opus-4-7`. + let listed: Vec<&str> = msg + .split([':', ',']) + .map(str::trim) + .filter(|s| s.starts_with("claude-")) + .collect(); + assert!( + !listed.contains(&"claude-opus-4"), + "family base must not appear in listing: {listed:?}", + ); + } + #[test] fn execute_unique_substring_resolves_after_suffix_tier_misses() { let (_, outcome) = run_execute("haiku-4-"); From 87fbc95cb6d9ff4ed5dd3243bfa69af8edbc3526 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Wed, 6 May 2026 23:49:58 +0800 Subject: [PATCH 02/25] feat(slash): typed-arg completion data layer Adds `SlashCommand::complete_arg` with curated rosters for `/model`, `/effort`, `/theme`. `ArgCompletion` rows pair value + description so the popup can render them side-by-side. `rank_by_prefix` factors the prefix-then-substring tier ranking out of `filter_and_rank` for reuse by per-command rosters. UI consumers ship in the next commit; this layer is intentionally unconsumed so the wiring change has a clean diff. --- crates/oxide-code/src/slash/effort.rs | 60 +++++++++++++++++++++- crates/oxide-code/src/slash/matcher.rs | 59 ++++++++++++++++++++++ crates/oxide-code/src/slash/model.rs | 66 ++++++++++++++++++++++++- crates/oxide-code/src/slash/picker.rs | 5 +- crates/oxide-code/src/slash/registry.rs | 16 ++++++ crates/oxide-code/src/slash/theme.rs | 52 ++++++++++++++++++- 6 files changed, 252 insertions(+), 6 deletions(-) diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 878e133b..5b30f187 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -1,13 +1,26 @@ //! `/effort` — open the slider, or swap with `/effort `. Bare form opens the //! Speed ↔ Intelligence slider (see [`super::effort_slider`]); typed arg shortcuts the picker. +use std::borrow::Cow; + use super::context::SlashContext; use super::effort_slider::EffortSlider; -use super::registry::{SlashCommand, SlashKind, SlashOutcome}; +use super::matcher::rank_by_prefix; +use super::registry::{ArgCompletion, SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; use crate::config::Effort; use crate::model::{capabilities_for, marketing_or_id}; +/// Curated autocomplete roster (intensity ladder; matches `Effort::ALL`). Descriptions kept +/// short so the popup doesn't dominate the screen. +const ARG_ROSTER: &[(&str, &str)] = &[ + ("low", "Fastest, shallow reasoning"), + ("medium", "Balanced"), + ("high", "Deep reasoning"), + ("xhigh", "Opus 4.7 ladder ceiling"), + ("max", "Opus-only maximum"), +]; + pub(super) struct EffortCmd; impl SlashCommand for EffortCmd { @@ -31,6 +44,16 @@ impl SlashCommand for EffortCmd { Some("[]") } + fn complete_arg(&self, prefix: &str) -> Vec { + rank_by_prefix(ARG_ROSTER, prefix, |(value, _)| *value) + .into_iter() + .map(|(value, description)| ArgCompletion { + value: Cow::Borrowed(value), + description: Cow::Borrowed(description), + }) + .collect() + } + fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { @@ -93,6 +116,41 @@ mod tests { assert_eq!(EffortCmd.classify("xhigh"), SlashKind::Mutating); } + // ── EffortCmd::complete_arg ── + + fn arg_values(prefix: &str) -> Vec { + EffortCmd + .complete_arg(prefix) + .into_iter() + .map(|c| c.value.into_owned()) + .collect() + } + + #[test] + fn complete_arg_empty_prefix_lists_full_ladder_in_intensity_order() { + assert_eq!( + arg_values(""), + vec!["low", "medium", "high", "xhigh", "max"] + ); + } + + #[test] + fn complete_arg_prefix_match_pulls_xhigh_above_max_substring() { + // `x` prefixes `xhigh`; `max` ends in `x` (substring). Prefix tier wins. + assert_eq!(arg_values("x"), vec!["xhigh", "max"]); + } + + #[test] + fn complete_arg_substring_match_lands_after_prefix_tier() { + // `high` is a substring of `xhigh` and a prefix of `high` — both tiers fire. + assert_eq!(arg_values("high"), vec!["high", "xhigh"]); + } + + #[test] + fn complete_arg_is_case_insensitive() { + assert_eq!(arg_values("MAX"), vec!["max"]); + } + // ── EffortCmd::execute ── fn run_execute(args: &str) -> (ChatView, Result) { diff --git a/crates/oxide-code/src/slash/matcher.rs b/crates/oxide-code/src/slash/matcher.rs index b0db85f2..c7f47c97 100644 --- a/crates/oxide-code/src/slash/matcher.rs +++ b/crates/oxide-code/src/slash/matcher.rs @@ -70,6 +70,30 @@ fn matching_alias(cmd: &dyn SlashCommand, pred: impl Fn(&str) -> bool) -> Option .find(|alias| pred(&alias.to_ascii_lowercase())) } +/// Two-tier prefix-then-substring rank over a curated roster, preserving declared order within +/// each tier. Empty `query` returns every item in declared order. Lower-cased internally. +pub(crate) fn rank_by_prefix<'a, T>( + items: &'a [T], + query: &str, + key: impl Fn(&T) -> &str, +) -> Vec<&'a T> { + if query.is_empty() { + return items.iter().collect(); + } + let q = query.to_ascii_lowercase(); + let mut prefix_hits: Vec<&'a T> = Vec::new(); + let mut substring_hits: Vec<&'a T> = Vec::new(); + for item in items { + let label = key(item).to_ascii_lowercase(); + if label.starts_with(&q) { + prefix_hits.push(item); + } else if label.contains(&q) { + substring_hits.push(item); + } + } + prefix_hits.into_iter().chain(substring_hits).collect() +} + fn on_name(cmd: &dyn SlashCommand) -> MatchedCommand { MatchedCommand { name: cmd.name(), @@ -237,6 +261,41 @@ mod tests { assert!(best_match("zzz", &CLEAR).is_none()); } + // ── rank_by_prefix ── + + fn levels() -> Vec<&'static str> { + vec!["low", "medium", "high", "xhigh", "max"] + } + + fn ranked(query: &str) -> Vec<&'static str> { + let xs = levels(); + rank_by_prefix(&xs, query, |s| *s) + .into_iter() + .copied() + .collect() + } + + #[test] + fn rank_by_prefix_empty_query_yields_full_roster_in_declared_order() { + assert_eq!(ranked(""), levels()); + } + + #[test] + fn rank_by_prefix_promotes_prefix_matches_above_substring_within_declared_order() { + // `h` prefixes `high`; substring-matches `xhigh`. Prefix wins; declared order otherwise. + assert_eq!(ranked("h"), vec!["high", "xhigh"]); + } + + #[test] + fn rank_by_prefix_is_case_insensitive() { + assert_eq!(ranked("HI"), vec!["high", "xhigh"]); + } + + #[test] + fn rank_by_prefix_unmatched_query_is_empty() { + assert!(ranked("zzz").is_empty()); + } + // ── Fake fixture ── #[test] diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index a164c256..a8975789 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -1,10 +1,14 @@ //! `/model` — open the picker, or swap with `/model `. Resolution: alias → exact / dated-id → //! unique suffix → unique substring. `[1m]` rejected on models lacking `context_1m`. +use std::borrow::Cow; + use super::context::SlashContext; -use super::registry::{SlashCommand, SlashKind, SlashOutcome}; +use super::matcher::rank_by_prefix; +use super::picker::LISTED_MODELS; +use super::registry::{ArgCompletion, SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; -use crate::model::{MODELS, ResolvedModelId, is_family_base, lookup}; +use crate::model::{MODELS, ResolvedModelId, display_name, is_family_base, lookup}; // ── Constants ── @@ -41,6 +45,16 @@ impl SlashCommand for ModelCmd { Some("[]") } + fn complete_arg(&self, prefix: &str) -> Vec { + rank_by_prefix(LISTED_MODELS, prefix, |id| *id) + .into_iter() + .map(|id| ArgCompletion { + value: Cow::Borrowed(*id), + description: Cow::Owned(display_name(id).into_owned()), + }) + .collect() + } + fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { @@ -177,6 +191,54 @@ mod tests { assert_eq!(ModelCmd.classify("claude-opus-4-7"), SlashKind::Mutating); } + // ── ModelCmd::complete_arg ── + + fn arg_rows(prefix: &str) -> Vec<(String, String)> { + ModelCmd + .complete_arg(prefix) + .into_iter() + .map(|c| (c.value.into_owned(), c.description.into_owned())) + .collect() + } + + #[test] + fn complete_arg_empty_prefix_lists_curated_roster_in_picker_order() { + let expected: Vec = LISTED_MODELS.iter().map(|id| (*id).to_owned()).collect(); + let got: Vec = arg_rows("").into_iter().map(|(v, _)| v).collect(); + assert_eq!(got, expected); + } + + #[test] + fn complete_arg_prefix_filter_narrows_to_matching_ids() { + let got: Vec = arg_rows("claude-opus") + .into_iter() + .map(|(v, _)| v) + .collect(); + assert_eq!(got, vec!["claude-opus-4-7", "claude-opus-4-7[1m]"]); + } + + #[test] + fn complete_arg_appends_1m_context_suffix_for_1m_variants() { + // The `[1m]` rows must surface the `(1M context)` marker so users can tell variants apart + // before committing. + let rows = arg_rows("claude-opus-4-7"); + let one_m = rows + .iter() + .find(|(v, _)| v == "claude-opus-4-7[1m]") + .expect("1M variant present"); + assert!( + one_m.1.contains("1M context"), + "1M description: {:?}", + one_m.1, + ); + } + + #[test] + fn complete_arg_is_case_insensitive() { + let got: Vec = arg_rows("HAIKU").into_iter().map(|(v, _)| v).collect(); + assert_eq!(got, vec!["claude-haiku-4-5"]); + } + // ── ModelCmd::execute ── fn run_execute(args: &str) -> (ChatView, Result) { diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index 830bc022..6e5859e6 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -19,8 +19,9 @@ use crate::tui::theme::Theme; // ── Constants ── -/// Curated roster shown in the picker; `/model ` resolves against the full `MODELS` table. -const LISTED_MODELS: &[&str] = &[ +/// Curated roster shown in the picker AND in `/model`'s typed-arg autocomplete; `/model ` +/// still resolves against the full `MODELS` table. +pub(super) const LISTED_MODELS: &[&str] = &[ "claude-opus-4-7", "claude-opus-4-7[1m]", "claude-sonnet-4-6", diff --git a/crates/oxide-code/src/slash/registry.rs b/crates/oxide-code/src/slash/registry.rs index 600c1af4..18a05af7 100644 --- a/crates/oxide-code/src/slash/registry.rs +++ b/crates/oxide-code/src/slash/registry.rs @@ -1,5 +1,7 @@ //! Slash-command trait and built-in registry. Adding a command: one file + one [`BUILT_INS`] entry. +use std::borrow::Cow; + use super::clear::ClearCmd; use super::config::ConfigCmd; use super::context::SlashContext; @@ -33,6 +35,14 @@ pub(crate) enum SlashKind { Unknown, } +/// One row in the typed-arg autocomplete popup. `description` may be empty. `Cow` so static +/// rosters borrow and dynamic ones (e.g., `[1m]` suffixed model names) own. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ArgCompletion { + pub(crate) value: Cow<'static, str>, + pub(crate) description: Cow<'static, str>, +} + /// Locally-dispatched `/name args` command. Each impl owns its display metadata. pub(crate) trait SlashCommand: Sync { fn name(&self) -> &'static str; @@ -52,6 +62,12 @@ pub(crate) trait SlashCommand: Sync { None } + /// Curated arg suggestions, prefix-filtered and ranked. Empty vec → no autocomplete; the + /// popup falls back to the [`Self::usage`] placeholder ghost-text. + fn complete_arg(&self, _prefix: &str) -> Vec { + Vec::new() + } + /// `Err(msg)` renders as an `ErrorBlock`. fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result; } diff --git a/crates/oxide-code/src/slash/theme.rs b/crates/oxide-code/src/slash/theme.rs index f7ccd8b8..fbb735a2 100644 --- a/crates/oxide-code/src/slash/theme.rs +++ b/crates/oxide-code/src/slash/theme.rs @@ -1,6 +1,8 @@ //! `/theme` — open the picker, or swap directly with `/theme `. Picker live-previews on //! cursor moves and reverts on cancel. +use std::borrow::Cow; + use crossterm::event::{KeyCode, KeyEvent}; use ratatui::Frame; use ratatui::layout::Rect; @@ -8,7 +10,8 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; use super::context::SlashContext; -use super::registry::{SlashCommand, SlashKind, SlashOutcome}; +use super::matcher::rank_by_prefix; +use super::registry::{ArgCompletion, SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; use crate::tui::modal::list_picker::{ListPicker, PickerItem}; use crate::tui::modal::{Modal, ModalAction, ModalKey}; @@ -191,6 +194,16 @@ impl SlashCommand for ThemeCmd { Some("[]") } + fn complete_arg(&self, prefix: &str) -> Vec { + rank_by_prefix(LISTED_THEMES, prefix, |(name, _)| *name) + .into_iter() + .map(|(name, description)| ArgCompletion { + value: Cow::Borrowed(*name), + description: Cow::Borrowed(*description), + }) + .collect() + } + fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { @@ -437,6 +450,43 @@ mod tests { assert_eq!(ThemeCmd.classify("latte"), SlashKind::Mutating); } + // ── ThemeCmd::complete_arg ── + + fn arg_values(prefix: &str) -> Vec { + ThemeCmd + .complete_arg(prefix) + .into_iter() + .map(|c| c.value.into_owned()) + .collect() + } + + #[test] + fn complete_arg_empty_prefix_lists_full_roster_in_curated_order() { + let expected: Vec = LISTED_THEMES + .iter() + .map(|(name, _)| (*name).to_owned()) + .collect(); + assert_eq!(arg_values(""), expected); + } + + #[test] + fn complete_arg_prefix_filter_narrows_to_matching_themes() { + // `m` prefixes mocha, macchiato, material; matters that all three surface and roster + // order is preserved. + assert_eq!(arg_values("m"), vec!["mocha", "macchiato", "material"]); + } + + #[test] + fn complete_arg_substring_match_below_prefix_tier() { + // `te` is a substring of `latte` and `material`; preserves declared order. + assert_eq!(arg_values("te"), vec!["latte", "material"]); + } + + #[test] + fn complete_arg_is_case_insensitive() { + assert_eq!(arg_values("MOCHA"), vec!["mocha"]); + } + // ── ThemeCmd::execute ── #[test] From 73dcbfb71d6076e9e38a9c4c020a89883f30bd88 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:04:09 +0800 Subject: [PATCH 03/25] feat(slash): arg-mode popup + ghost-text fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the data layer from the previous commit into the input area: - `parser` exposes `popup_state` (replacing `popup_query`) so callers distinguish `Name` from `Arg { name, prefix }`. - `SlashPopup` becomes mode-tagged: arg mode renders the curated roster bare (no `/` prefix), Tab replaces the typed prefix with the picked value plus a trailing space. - `InputArea::ghost_text` paints a dim `[id]` / `[level]` / `[name]` hint at the cursor when the popup yields — i.e., when a command publishes `usage()` without a `complete_arg` impl. Suppressed when the popup is visible since the curated roster is a richer hint. Docs: `docs/guide/slash-commands.md` notes arg mode in two short sentences; roadmap moves the shipped items out of Current Focus. --- README.md | 2 +- crates/oxide-code/src/slash.rs | 18 +- crates/oxide-code/src/slash/parser.rs | 102 ++++-- crates/oxide-code/src/tui/components/input.rs | 205 +++++++++-- .../src/tui/components/input/popup.rs | 331 ++++++++++++------ ...s_curated_roster_without_slash_prefix.snap | 10 + ...der_arg_mode_prefix_filters_to_subset.snap | 8 + ...y_name_query_shows_each_command_once.snap} | 2 +- ..._name_query_shows_only_matching_rows.snap} | 2 +- docs/guide/slash-commands.md | 4 +- docs/roadmap.md | 5 +- 11 files changed, 524 insertions(+), 165 deletions(-) create mode 100644 crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap create mode 100644 crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_prefix_filters_to_subset.snap rename crates/oxide-code/src/tui/components/input/snapshots/{ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap => ox__tui__components__input__popup__tests__render_empty_name_query_shows_each_command_once.snap} (96%) rename crates/oxide-code/src/tui/components/input/snapshots/{ox__tui__components__input__popup__tests__render_filtered_query_shows_only_matching_rows.snap => ox__tui__components__input__popup__tests__render_filtered_name_query_shows_only_matching_rows.snap} (89%) diff --git a/README.md b/README.md index 0ff9687c..7ae57f1b 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Early development. What works today: - Agent loop with extended thinking and tool-use round-trip - File and search tools: `read`, `write`, `edit`, `glob`, `grep`, `bash` - Turn interruption (Esc / Ctrl+C) plus mid-turn queued follow-up prompts that splice into the same turn between tool calls, with double-press Ctrl+C exit confirmation -- Slash commands with `/`-triggered autocomplete: `/clear`, `/config`, `/diff`, `/effort`, `/help`, `/init`, `/model`, `/status` +- Slash commands with `/`-triggered autocomplete (name and curated-arg modes): `/clear`, `/config`, `/diff`, `/effort`, `/help`, `/init`, `/model`, `/status`, `/theme` - `CLAUDE.md` / `AGENTS.md` instruction-file discovery - Session persistence with JSONL conversation logs, listing, and resume - Per-session file-change tracking with a Read-before-Edit gate and on-disk drift detection diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 055bef67..5e0afeb0 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -27,14 +27,28 @@ mod theme; pub(crate) use context::{LiveSessionInfo, SlashContext}; pub(crate) use matcher::MatchedCommand; -pub(crate) use parser::{Parsed, parse_slash, popup_query}; -pub(crate) use registry::SlashKind; +pub(crate) use parser::{Parsed, PopupState, parse_slash, popup_state}; +pub(crate) use registry::{ArgCompletion, SlashKind}; /// Filter the built-in registry against a popup query (leading `/` stripped). pub(crate) fn filter_built_ins(query: &str) -> Vec { matcher::filter_and_rank(query, registry::BUILT_INS) } +/// Typed-arg completions for `cmd_name`, prefix-filtered. Returns empty for unknown commands +/// or commands without a curated roster. +pub(crate) fn complete_arg_for(cmd_name: &str, prefix: &str) -> Vec { + registry::lookup_in(registry::BUILT_INS, cmd_name) + .map(|cmd| cmd.complete_arg(prefix)) + .unwrap_or_default() +} + +/// Static placeholder hint for `cmd_name` (e.g., `[]`). Returned by [`SlashCommand::usage`]; +/// rendered as dim ghost-text after `/cmd ` when there are no arg completions to show. +pub(crate) fn arg_placeholder_for(cmd_name: &str) -> Option<&'static str> { + registry::lookup_in(registry::BUILT_INS, cmd_name).and_then(registry::SlashCommand::usage) +} + /// Resolves and runs `parsed` against the built-in registry. pub(crate) fn dispatch( parsed: &Parsed, diff --git a/crates/oxide-code/src/slash/parser.rs b/crates/oxide-code/src/slash/parser.rs index 621c793c..194b8d33 100644 --- a/crates/oxide-code/src/slash/parser.rs +++ b/crates/oxide-code/src/slash/parser.rs @@ -28,14 +28,36 @@ fn is_name_char(c: char) -> bool { c.is_ascii_alphanumeric() || matches!(c, '_' | '-' | ':' | '.') } -/// In-progress query (leading `/` stripped) when `buffer` is a slash command being typed. -/// `None` for plain prompts, `//` escape, or once whitespace appears (args started). -pub(crate) fn popup_query(buffer: &str) -> Option<&str> { +/// In-progress popup context. `Name` while typing the command name (no whitespace yet); +/// `Arg` once the user has typed a space after the name. `None` for plain prompts and `//` +/// escape. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PopupState<'a> { + Name(&'a str), + Arg { name: &'a str, prefix: &'a str }, +} + +/// Classifies the cursor's position within a single-line input. Empty `prefix` in `Arg` means +/// the user typed `/cmd ` and is poised to start the argument — distinct from `Arg { prefix: +/// non-empty }` (already typing) and `Name` (still inside the name). +pub(crate) fn popup_state(buffer: &str) -> Option> { let rest = buffer.trim_start().strip_prefix('/')?; - if rest.starts_with('/') || !rest.chars().all(is_name_char) { + if rest.starts_with('/') { return None; } - Some(rest) + let Some((name, after)) = rest.split_once(char::is_whitespace) else { + return rest + .chars() + .all(is_name_char) + .then_some(PopupState::Name(rest)); + }; + if name.is_empty() || !name.chars().all(is_name_char) { + return None; + } + Some(PopupState::Arg { + name, + prefix: after.trim_start(), + }) } #[cfg(test)] @@ -108,47 +130,73 @@ mod tests { assert!(parse_slash("/foo,bar").is_none()); } - // ── popup_query ── + // ── popup_state ── + + fn name(s: &str) -> PopupState<'_> { + PopupState::Name(s) + } + + fn arg<'a>(name: &'a str, prefix: &'a str) -> PopupState<'a> { + PopupState::Arg { name, prefix } + } + + #[test] + fn popup_state_bare_slash_is_empty_name() { + assert_eq!(popup_state("/"), Some(name(""))); + } + + #[test] + fn popup_state_partial_name_carries_typed_chars() { + assert_eq!(popup_state("/cl"), Some(name("cl"))); + assert_eq!(popup_state("/clear"), Some(name("clear"))); + } #[test] - fn popup_query_bare_slash_is_empty_query() { - assert_eq!(popup_query("/"), Some("")); + fn popup_state_tolerates_leading_whitespace_before_slash() { + assert_eq!(popup_state(" /he"), Some(name("he"))); } #[test] - fn popup_query_partial_name_produces_typed_chars() { - assert_eq!(popup_query("/cl"), Some("cl")); - assert_eq!(popup_query("/clear"), Some("clear")); + fn popup_state_trailing_space_after_name_switches_to_empty_arg() { + // The empty-prefix case is the trigger for the placeholder ghost-text — we have to be + // able to distinguish it from `Name`, otherwise the popup never opens for arg mode. + assert_eq!(popup_state("/clear "), Some(arg("clear", ""))); } #[test] - fn popup_query_tolerates_leading_whitespace() { - assert_eq!(popup_query(" /he"), Some("he")); + fn popup_state_typed_arg_carries_prefix_trimmed_of_leading_whitespace() { + assert_eq!(popup_state("/model claude-"), Some(arg("model", "claude-"))); + assert_eq!(popup_state("/model opus"), Some(arg("model", "opus"))); } #[test] - fn popup_query_hides_once_whitespace_appears() { - assert!(popup_query("/clear ").is_none()); - assert!(popup_query("/clear arg").is_none()); - assert!(popup_query("/cl ear").is_none()); + fn popup_state_inner_whitespace_in_args_is_kept_for_free_form_commands() { + // /init takes a free-form sentence — the popup is hidden via empty `complete_arg`, but + // the parser still classifies it as Arg with the full remainder as prefix. + assert_eq!( + popup_state("/init please write CLAUDE.md"), + Some(arg("init", "please write CLAUDE.md")), + ); } #[test] - fn popup_query_hides_for_double_slash_escape() { - assert!(popup_query("//etc/hosts").is_none()); - assert!(popup_query("//").is_none()); + fn popup_state_double_slash_escape_is_not_a_command() { + assert!(popup_state("//etc/hosts").is_none()); + assert!(popup_state("//").is_none()); } #[test] - fn popup_query_hides_for_plain_prompts() { - assert!(popup_query("hello").is_none()); - assert!(popup_query("explain /etc/hosts").is_none()); - assert!(popup_query("").is_none()); + fn popup_state_plain_prompts_are_not_a_command() { + assert!(popup_state("hello").is_none()); + assert!(popup_state("explain /etc/hosts").is_none()); + assert!(popup_state("").is_none()); } #[test] - fn popup_query_hides_when_name_chars_violated() { - assert!(popup_query("/foo🦀").is_none()); - assert!(popup_query("/foo!").is_none()); + fn popup_state_invalid_name_chars_reject_the_buffer() { + assert!(popup_state("/foo🦀").is_none()); + assert!(popup_state("/foo!").is_none()); + // Disallowed-char names also fail in arg form so the popup doesn't show stale completions. + assert!(popup_state("/foo! arg").is_none()); } } diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index b38ca567..9604912f 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -13,9 +13,9 @@ use ratatui::widgets::{Block, Borders, Paragraph}; use ratatui_textarea::{TextArea, WrapMode}; use unicode_width::UnicodeWidthStr; -use self::popup::SlashPopup; +use self::popup::{PopupMode, SlashPopup}; use crate::agent::event::UserAction; -use crate::slash::popup_query; +use crate::slash::{PopupState, arg_placeholder_for, popup_state}; use crate::tui::glyphs::{USER_PROMPT_PREFIX, USER_PROMPT_PREFIX_WIDTH}; use crate::tui::theme::Theme; @@ -236,11 +236,23 @@ impl InputArea { }; self.scroll_top.set(top); - let cursor_x = textarea_area - .x - .saturating_add(to_u16(sc.col)) - .min(textarea_area.right().saturating_sub(1)); + let raw_cursor_x = textarea_area.x.saturating_add(to_u16(sc.col)); + let cursor_x = raw_cursor_x.min(textarea_area.right().saturating_sub(1)); let cursor_y = textarea_area.y + cursor_row - top; + + if let Some(token) = self.ghost_text() { + // Paint past the cursor, clipped to the textarea's right edge so wrapping mid-token + // doesn't bleed into the next row. + let width = textarea_area.right().saturating_sub(raw_cursor_x); + if width > 0 { + let area = Rect::new(raw_cursor_x, cursor_y, width, 1); + frame.render_widget( + Paragraph::new(Line::from(Span::styled(token, self.theme.dim()))), + area, + ); + } + } + frame.set_cursor_position((cursor_x, cursor_y)); } } @@ -280,7 +292,7 @@ impl InputArea { PopupKey::Consumed } KeyCode::Esc => { - self.popup.set_query(None); + self.popup.set_state(None); PopupKey::Consumed } KeyCode::Tab if modifiers.is_empty() => { @@ -296,31 +308,62 @@ impl InputArea { } fn popup_complete_to_buffer(&mut self) { - let Some(name) = self.popup.selected().map(|m| m.name) else { + let Some(replacement) = self.popup_completion_text() else { return; }; - self.set_text(&format!("/{name} ")); - self.popup.set_query(None); + self.set_text(&replacement); + self.popup.set_state(None); + } + + /// Mode-aware Tab insertion. Name mode replaces the buffer with `/{name} `; arg mode keeps + /// the typed `/cmd ` and substitutes the prefix with the picked value plus a trailing space. + fn popup_completion_text(&self) -> Option { + let row = self.popup.selected()?; + Some(match self.popup.mode()? { + PopupMode::Name => format!("/{} ", row.value), + PopupMode::Arg { cmd } => format!("/{cmd} {} ", row.value), + }) } fn popup_submit_selected(&mut self) -> Option { - let name = self.popup.selected()?.name; + let row = self.popup.selected()?; + let submission = match self.popup.mode()? { + PopupMode::Name => format!("/{}", row.value), + PopupMode::Arg { cmd } => format!("/{cmd} {}", row.value), + }; self.textarea.select_all(); self.textarea.cut(); self.scroll_top.set(0); - self.popup.set_query(None); - Some(UserAction::SubmitPrompt(format!("/{name}"))) + self.popup.set_state(None); + Some(UserAction::SubmitPrompt(submission)) } fn refresh_popup(&mut self) { - // Popup wiring: only the single-line case can be a slash command, and `popup_query` - // returns `None` for non-`/`-prefixed text — so a multi-line buffer or a typed-over slash - // both close the popup. - let query = match self.textarea.lines() { - [single] => popup_query(single), + // Only the single-line case can be a slash command; multi-line buffers close the popup. + let state = match self.textarea.lines() { + [single] => popup_state(single), _ => None, }; - self.popup.set_query(query); + self.popup.set_state(state.as_ref()); + } + + /// Dim hint token (`[id]`, `[level]`, `[name]`) painted at the cursor in arg mode with an + /// empty prefix. Suppressed when the popup is visible — the popup's curated roster is a + /// richer hint. Fires when the popup yields (command publishes `usage()` without a + /// `complete_arg` impl). + fn ghost_text(&self) -> Option { + if self.popup.is_visible() { + return None; + } + let [single] = self.textarea.lines() else { + return None; + }; + let state = popup_state(single)?; + let usage = match &state { + PopupState::Arg { name, .. } => arg_placeholder_for(name), + PopupState::Name(_) => None, + }; + ghost_text_from_state(&state, usage) } fn is_scroll_key(event: &Event) -> bool { @@ -385,6 +428,35 @@ impl InputArea { // ── Free Functions ── +/// Pure decision: ghost-text fires only in `Arg` mode with an empty prefix and a `usage()` to +/// surface. The popup-visibility gate lives at the call site so this helper stays trivially +/// testable without a `SlashPopup` instance. +fn ghost_text_from_state(state: &PopupState<'_>, usage: Option<&str>) -> Option { + let PopupState::Arg { prefix, .. } = state else { + return None; + }; + if !prefix.is_empty() { + return None; + } + Some(normalize_placeholder(usage?)) +} + +/// Normalize a `usage()` syntax fragment to a single bracketed token. Strips one optional layer +/// each of `[...]` and `<...>` so `[]`, ``, and `[id]` all render as `[id]`. +fn normalize_placeholder(usage: &str) -> String { + let inner = usage.trim(); + let inner = inner + .strip_prefix('[') + .and_then(|s| s.strip_suffix(']')) + .unwrap_or(inner) + .trim(); + let inner = inner + .strip_prefix('<') + .and_then(|s| s.strip_suffix('>')) + .unwrap_or(inner); + format!("[{inner}]") +} + /// Lossy `usize → u16` for cursor / column positions, bounded by terminal dimensions. #[expect( clippy::cast_possible_truncation, @@ -896,13 +968,17 @@ mod tests { input } + fn selected_value(input: &InputArea) -> String { + input.popup.selected().unwrap().value.clone().into_owned() + } + #[test] fn handle_event_popup_down_advances_selection() { let mut input = input_with_popup(); - let initial = input.popup.selected().map(|m| m.name).unwrap(); + let initial = selected_value(&input); let action = input.handle_event(&key(KeyCode::Down, KeyModifiers::NONE)); assert!(action.is_none(), "Down is consumed silently"); - let after = input.popup.selected().map(|m| m.name).unwrap(); + let after = selected_value(&input); assert_ne!(initial, after, "Down moves to a different command"); } @@ -910,9 +986,9 @@ mod tests { fn handle_event_popup_up_reverses_selection() { let mut input = input_with_popup(); input.handle_event(&key(KeyCode::Down, KeyModifiers::NONE)); - let after_down = input.popup.selected().map(|m| m.name).unwrap(); + let after_down = selected_value(&input); input.handle_event(&key(KeyCode::Up, KeyModifiers::NONE)); - let after_up = input.popup.selected().map(|m| m.name).unwrap(); + let after_up = selected_value(&input); assert_ne!(after_down, after_up, "Up reverses Down"); } @@ -937,6 +1013,89 @@ mod tests { assert!(input.popup_visible()); } + // ── ghost_text ── + + #[test] + fn ghost_text_suppresses_when_popup_visible() { + // `/model ` shows the curated roster — popup wins, no ghost-text. Pins the gate. + let mut input = test_input(); + type_text(&mut input, "/model "); + input.refresh_popup(); + assert!(input.popup_visible()); + assert_eq!(input.ghost_text(), None); + } + + #[test] + fn ghost_text_suppresses_in_name_mode() { + // Typing the command name itself is `Name`, never `Arg` — no hint applies. + let mut input = test_input(); + type_text(&mut input, "/mod"); + input.refresh_popup(); + assert_eq!(input.ghost_text(), None); + } + + // ── ghost_text_from_state ── + + #[test] + fn ghost_text_from_state_arg_with_empty_prefix_and_usage_renders_token() { + let state = PopupState::Arg { + name: "model", + prefix: "", + }; + assert_eq!( + ghost_text_from_state(&state, Some("[]")), + Some("[id]".to_owned()), + ); + } + + #[test] + fn ghost_text_from_state_non_empty_prefix_returns_none() { + // Once the user starts typing the arg, the hint disappears so it doesn't trail their text. + let state = PopupState::Arg { + name: "model", + prefix: "claude-", + }; + assert_eq!(ghost_text_from_state(&state, Some("[]")), None); + } + + #[test] + fn ghost_text_from_state_arg_without_usage_returns_none() { + // No usage to surface → nothing to hint. + let state = PopupState::Arg { + name: "init", + prefix: "", + }; + assert_eq!(ghost_text_from_state(&state, None), None); + } + + #[test] + fn ghost_text_from_state_name_mode_returns_none_even_with_usage() { + // Defensive: usage is read off the parsed name only in Arg mode, but pin the helper too. + let state = PopupState::Name("mo"); + assert_eq!(ghost_text_from_state(&state, Some("[]")), None); + } + + // ── normalize_placeholder ── + + #[test] + fn normalize_placeholder_strips_outer_brackets_and_inner_angles() { + // `[]` is the canonical form `usage()` returns today. + assert_eq!(normalize_placeholder("[]"), "[id]"); + } + + #[test] + fn normalize_placeholder_handles_missing_layers() { + // Forward-compatible with terser usage strings — strip what's there. + assert_eq!(normalize_placeholder(""), "[level]"); + assert_eq!(normalize_placeholder("[name]"), "[name]"); + assert_eq!(normalize_placeholder("topic"), "[topic]"); + } + + #[test] + fn normalize_placeholder_trims_whitespace_inside_brackets() { + assert_eq!(normalize_placeholder("[ ]"), "[id]"); + } + // ── render_popup ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 5f7369b9..b5b4bd63 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -1,8 +1,15 @@ -//! 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)`); the full list stays unparenthesized. +//! Slash-command autocomplete popup. Two modes: //! -//! Lists past [`MAX_VISIBLE_ROWS`] scroll with a centered cursor (Claude Code typeahead style). +//! - **Name** — typing `/cmd`. Rows are matched commands; aliases parenthesize only the typed +//! alias (`/clear (new)`). Tab inserts `/{name} ` into the buffer. +//! - **Arg** — typing `/cmd `. Rows are arg completions from the command's curated +//! roster (`/model`, `/effort`, `/theme`). Tab replaces the prefix with `/{cmd} {value} `. +//! +//! Selected row paints in `text` + BOLD; others dim — contrast stands in for a prefix glyph +//! or fill. Lists past [`MAX_VISIBLE_ROWS`] scroll with a centered cursor (Claude Code +//! typeahead style). + +use std::borrow::Cow; use ratatui::Frame; use ratatui::layout::Rect; @@ -11,7 +18,7 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; use unicode_width::UnicodeWidthStr; -use crate::slash::{MatchedCommand, filter_built_ins}; +use crate::slash::{PopupState, complete_arg_for, filter_built_ins}; use crate::tui::theme::Theme; use crate::util::text::truncate_to_width; @@ -19,10 +26,32 @@ const MAX_VISIBLE_ROWS: usize = 8; const COLUMN_GAP: usize = 2; -/// Slash-command autocomplete overlay. Empty `matches` means hidden. +/// Mode-tagged so [`super::InputArea`] can format the right Tab-insertion text. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PopupMode { + Name, + /// Owned because the cmd name is parsed out of the live input buffer, not a `'static` slice. + Arg { + cmd: String, + }, +} + +/// One popup row. `value` is the bare token (command name or arg value); the renderer adds +/// `/` and any matched-alias suffix in name mode. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct PopupRow { + pub(crate) value: Cow<'static, str>, + pub(crate) description: Cow<'static, str>, + /// Set in name mode when the typed query matched an alias rather than the canonical name. + pub(crate) matched_alias: Option<&'static str>, +} + +/// Slash-command autocomplete overlay. Empty `rows` means hidden. pub(crate) struct SlashPopup { theme: Theme, - matches: Vec, + /// `Some` when visible. Mode discriminates which Tab-insertion shape applies. + mode: Option, + rows: Vec, selected: usize, } @@ -30,7 +59,8 @@ impl SlashPopup { pub(crate) fn new(theme: &Theme) -> Self { Self { theme: theme.clone(), - matches: Vec::new(), + mode: None, + rows: Vec::new(), selected: 0, } } @@ -39,37 +69,73 @@ impl SlashPopup { self.theme = theme.clone(); } - pub(crate) fn set_query(&mut self, query: Option<&str>) { - let Some(q) = query else { - self.matches.clear(); - self.selected = 0; - return; + pub(crate) fn set_state(&mut self, state: Option<&PopupState<'_>>) { + let (mode, rows) = match state { + None => (None, Vec::new()), + Some(PopupState::Name(query)) => ( + Some(PopupMode::Name), + filter_built_ins(query) + .into_iter() + .map(|m| PopupRow { + value: Cow::Borrowed(m.name), + description: Cow::Borrowed(m.description), + matched_alias: m.matched_alias, + }) + .collect(), + ), + Some(PopupState::Arg { name, prefix }) => { + let completions = complete_arg_for(name, prefix); + if completions.is_empty() { + // Hidden popup — leaves room for the placeholder ghost-text. Mode is + // intentionally `None` so `is_visible()` returns false. + (None, Vec::new()) + } else { + ( + Some(PopupMode::Arg { + cmd: (*name).to_owned(), + }), + completions + .into_iter() + .map(|c| PopupRow { + value: c.value, + description: c.description, + matched_alias: None, + }) + .collect(), + ) + } + } }; - self.matches = filter_built_ins(q); - self.selected = self.selected.min(self.matches.len().saturating_sub(1)); + self.mode = mode; + self.rows = rows; + self.selected = self.selected.min(self.rows.len().saturating_sub(1)); } pub(crate) fn is_visible(&self) -> bool { - !self.matches.is_empty() + !self.rows.is_empty() } - pub(crate) fn selected(&self) -> Option<&MatchedCommand> { - self.matches.get(self.selected) + pub(crate) fn mode(&self) -> Option<&PopupMode> { + self.mode.as_ref() + } + + pub(crate) fn selected(&self) -> Option<&PopupRow> { + self.rows.get(self.selected) } pub(crate) fn select_next(&mut self) { - if self.matches.is_empty() { + if self.rows.is_empty() { return; } - self.selected = (self.selected + 1) % self.matches.len(); + self.selected = (self.selected + 1) % self.rows.len(); } pub(crate) fn select_prev(&mut self) { - if self.matches.is_empty() { + if self.rows.is_empty() { return; } self.selected = if self.selected == 0 { - self.matches.len() - 1 + self.rows.len() - 1 } else { self.selected - 1 }; @@ -77,23 +143,27 @@ impl SlashPopup { /// 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); + let visible = self.rows.len().min(MAX_VISIBLE_ROWS); u16::try_from(visible).unwrap_or(u16::MAX) } pub(crate) fn render(&self, frame: &mut Frame, area: Rect) { - if self.matches.is_empty() { + if self.rows.is_empty() { return; } let width = usize::from(area.width); 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 visible = self.rows.len().min(MAX_VISIBLE_ROWS); + let window = &self.rows[offset..offset + visible]; + let label_width = window + .iter() + .map(|r| self.label(r).width()) + .max() + .unwrap_or(0); let lines: Vec> = window .iter() .enumerate() - .map(|(i, m)| self.render_row(m, offset + i == self.selected, label_width, width)) + .map(|(i, r)| self.render_row(r, offset + i == self.selected, label_width, width)) .collect(); frame.render_widget(Paragraph::new(lines).style(self.theme.surface()), area); } @@ -101,7 +171,7 @@ impl SlashPopup { /// 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(); + let total = self.rows.len(); if total <= MAX_VISIBLE_ROWS { return 0; } @@ -110,23 +180,32 @@ impl SlashPopup { self.selected.saturating_sub(pad).min(max_offset) } + /// Mode-aware left-column label. Used for both rendering and column-width computation, so + /// the two must agree exactly. + fn label(&self, row: &PopupRow) -> String { + match &self.mode { + Some(PopupMode::Name) => match row.matched_alias { + Some(alias) => format!("/{} ({alias})", row.value), + None => format!("/{}", row.value), + }, + Some(PopupMode::Arg { .. }) | None => row.value.to_string(), + } + } + fn render_row( &self, - m: &MatchedCommand, + row: &PopupRow, selected: bool, label_width: usize, width: usize, ) -> Line<'static> { - let label_text = label(m); + let label_text = self.label(row); let pad = label_width.saturating_sub(label_text.width()); let row_style = row_style(&self.theme, selected); let desc_budget = width.saturating_sub(label_width + COLUMN_GAP); - let desc = truncate_to_width(m.description, desc_budget); + let desc = truncate_to_width(&row.description, desc_budget); - let mut spans = vec![Span::styled(format!("/{}", m.name), row_style)]; - if let Some(alias) = m.matched_alias { - spans.push(Span::styled(format!(" ({alias})"), row_style)); - } + let mut spans = vec![Span::styled(label_text, row_style)]; let gap = " ".repeat(pad + COLUMN_GAP); spans.push(Span::raw(gap)); spans.push(Span::styled(desc, row_style)); @@ -134,15 +213,6 @@ impl SlashPopup { } } -/// `/name` plus the typed alias when matched via alias (`/clear (new)`). Used for both -/// rendering and column-width computation, so the two must agree exactly. -fn label(m: &MatchedCommand) -> String { - match m.matched_alias { - Some(alias) => format!("/{} ({alias})", m.name), - None => format!("/{}", m.name), - } -} - /// Selected → `text` + BOLD; non-selected → dim. Contrast mirrors Claude Code's popup. fn row_style(theme: &Theme, selected: bool) -> Style { if selected { @@ -163,12 +233,20 @@ mod tests { Theme::default() } - fn popup_with_query(query: Option<&str>) -> SlashPopup { + fn popup_with_state(state: Option<&PopupState<'_>>) -> SlashPopup { let mut p = SlashPopup::new(&theme()); - p.set_query(query); + p.set_state(state); p } + fn name_popup(query: &str) -> SlashPopup { + popup_with_state(Some(&PopupState::Name(query))) + } + + fn arg_popup<'a>(name: &'a str, prefix: &'a str) -> SlashPopup { + popup_with_state(Some(&PopupState::Arg { name, prefix })) + } + fn render_to_backend(popup: &SlashPopup, width: u16) -> TestBackend { let height = popup.height().max(1); let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap(); @@ -178,56 +256,84 @@ mod tests { terminal.backend().clone() } - // ── set_query ── + // ── set_state ── #[test] - fn set_query_none_hides_popup() { - let popup = popup_with_query(None); + fn set_state_none_hides_popup() { + let popup = popup_with_state(None); assert!(!popup.is_visible()); assert_eq!(popup.height(), 0); } #[test] - fn set_query_empty_lists_full_registry_in_presentation_order() { + fn set_state_name_empty_query_lists_full_registry_in_presentation_order() { // Empty query is what the user sees right after typing `/`. - let popup = popup_with_query(Some("")); + let popup = name_popup(""); assert!(popup.is_visible()); - let names: Vec<&str> = popup.matches.iter().map(|m| m.name).collect(); - // BUILT_INS is alphabetical, so empty-query first row is `/clear`. - assert!(!names.is_empty()); - assert_eq!(names[0], "clear"); + assert_eq!(popup.mode(), Some(&PopupMode::Name)); + let values: Vec = popup + .rows + .iter() + .map(|r| r.value.clone().into_owned()) + .collect(); + // BUILT_INS is alphabetical, so empty-query first row is `clear`. + assert!(!values.is_empty()); + assert_eq!(values[0], "clear"); } #[test] - fn set_query_clamps_selection_when_match_count_shrinks() { - // Empty query → full list; park selection on the last row, - // then narrow to a single match — selection must clamp into - // range so render() doesn't index past the end. - let mut popup = popup_with_query(Some("")); - let n = popup.matches.len(); + fn set_state_clamps_selection_when_row_count_shrinks() { + // Empty query → full list; park selection on the last row, then narrow to a single match + // — selection must clamp so render() doesn't index past the end. + let mut popup = name_popup(""); + let n = popup.rows.len(); for _ in 0..n - 1 { popup.select_next(); } assert_eq!(popup.selected, n - 1); - popup.set_query(Some("help")); - assert_eq!(popup.matches.len(), 1); + popup.set_state(Some(&PopupState::Name("help"))); + assert_eq!(popup.rows.len(), 1); assert_eq!(popup.selected, 0); } + #[test] + fn set_state_arg_with_curated_roster_populates_arg_mode() { + let popup = arg_popup("model", ""); + assert!(popup.is_visible()); + assert!(matches!(popup.mode(), Some(PopupMode::Arg { cmd }) if cmd == "model")); + // Roster is non-empty for /model. + assert!(!popup.rows.is_empty()); + } + + #[test] + fn set_state_arg_with_empty_roster_stays_hidden_for_ghost_text_fallback() { + // /init has no curated arg roster — popup must hide so the placeholder ghost-text path + // can render instead. + let popup = arg_popup("init", ""); + assert!(!popup.is_visible()); + assert!(popup.mode().is_none()); + } + + #[test] + fn set_state_arg_unknown_command_stays_hidden() { + let popup = arg_popup("nope", ""); + assert!(!popup.is_visible()); + } + // ── selected ── #[test] - fn selected_picks_match_at_index() { - let mut popup = popup_with_query(Some("")); + fn selected_picks_row_at_index() { + let mut popup = name_popup(""); popup.select_next(); let row = popup.selected().expect("popup visible"); - assert_eq!(row.name, popup.matches[1].name); + assert_eq!(row.value, popup.rows[1].value); } #[test] fn selected_is_none_when_hidden() { - let popup = popup_with_query(None); + let popup = popup_with_state(None); assert!(popup.selected().is_none()); } @@ -235,8 +341,8 @@ mod tests { #[test] fn select_next_wraps_at_bottom() { - let mut popup = popup_with_query(Some("")); - let n = popup.matches.len(); + let mut popup = name_popup(""); + let n = popup.rows.len(); for _ in 0..n { popup.select_next(); } @@ -245,17 +351,17 @@ mod tests { #[test] fn select_prev_wraps_at_top() { - let mut popup = popup_with_query(Some("")); - let n = popup.matches.len(); + let mut popup = name_popup(""); + let n = popup.rows.len(); popup.select_prev(); assert_eq!(popup.selected, n - 1, "wrap from first up to last"); } #[test] fn select_prev_decrements_when_not_at_top() { - // Pin the non-wrap branch — the decrement path is otherwise - // dead because select_prev() from row 0 always wraps. - let mut popup = popup_with_query(Some("")); + // Pin the non-wrap branch — the decrement path is otherwise dead because select_prev() + // from row 0 always wraps. + let mut popup = name_popup(""); popup.select_next(); popup.select_next(); assert_eq!(popup.selected, 2); @@ -266,7 +372,7 @@ mod tests { #[test] fn select_next_on_empty_popup_is_a_noop() { - let mut popup = popup_with_query(None); + let mut popup = popup_with_state(None); popup.select_next(); popup.select_prev(); assert_eq!(popup.selected, 0); @@ -276,8 +382,8 @@ mod tests { #[test] fn height_caps_at_max_visible_rows() { - let popup = popup_with_query(Some("")); - let expected = popup.matches.len().min(MAX_VISIBLE_ROWS); + let popup = name_popup(""); + let expected = popup.rows.len().min(MAX_VISIBLE_ROWS); assert_eq!(usize::from(popup.height()), expected); } @@ -286,10 +392,11 @@ mod tests { fn long_popup(n: usize) -> SlashPopup { // Hand-rolled 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", + p.mode = Some(PopupMode::Name); + p.rows = (0..n) + .map(|i| PopupRow { + value: Cow::Owned(format!("cmd{i}")), + description: Cow::Borrowed("desc"), matched_alias: None, }) .collect(); @@ -297,29 +404,41 @@ mod tests { } #[test] - fn render_empty_query_shows_each_command_once() { - let popup = popup_with_query(Some("")); + fn render_empty_name_query_shows_each_command_once() { + let popup = name_popup(""); + insta::assert_snapshot!(render_to_backend(&popup, 60)); + } + + #[test] + fn render_filtered_name_query_shows_only_matching_rows() { + // Narrow query → single row. Confirms filter wiring and that unmatched commands disappear. + let popup = name_popup("hel"); + insta::assert_snapshot!(render_to_backend(&popup, 60)); + } + + #[test] + fn render_arg_mode_lists_curated_roster_without_slash_prefix() { + // Arg-mode rows render bare values (`low`, `medium`, ...) — no leading `/`. + let popup = arg_popup("effort", ""); insta::assert_snapshot!(render_to_backend(&popup, 60)); } #[test] - fn render_filtered_query_shows_only_matching_rows() { - // Narrow query → single row. Confirms filter wiring and - // that the unmatched commands disappear. - let popup = popup_with_query(Some("hel")); + fn render_arg_mode_prefix_filters_to_subset() { + // Pin that arg-mode prefix filtering reaches the renderer (not just the data layer). + let popup = arg_popup("theme", "m"); insta::assert_snapshot!(render_to_backend(&popup, 60)); } #[test] fn render_selected_row_paints_bold_text_others_dim() { - // TestBackend snapshots don't capture style, so layout-only - // snapshots can't tell selected from unselected. Pin the - // bold-vs-dim contrast directly on the rendered cells. + // TestBackend snapshots don't capture style, so layout-only snapshots can't tell selected + // from unselected. Pin the bold-vs-dim contrast directly on the rendered cells. use ratatui::layout::Position; use ratatui::style::Modifier; let theme = theme(); - let mut popup = popup_with_query(Some("")); + let mut popup = name_popup(""); popup.select_next(); let backend = render_to_backend(&popup, 60); @@ -342,20 +461,21 @@ mod tests { #[test] fn render_narrow_terminal_truncates_description() { - // At 30 cols the description gutter must shrink and wrap - // through ELLIPSIS rather than overflowing the row. - let popup = popup_with_query(Some("")); + // At 30 cols the description gutter must shrink and wrap through ELLIPSIS rather than + // overflowing the row. + let popup = name_popup(""); insta::assert_snapshot!(render_to_backend(&popup, 30)); } #[test] fn render_alias_match_parenthesizes_only_typed_alias() { - // No live registry command has aliases yet, but the renderer - // is the place where the alias-display rule lives. Drive it via a hand-rolled match list. + // No live registry command has aliases yet, but the renderer is the place where the + // alias-display rule lives. Drive it via a hand-rolled match list. let mut popup = SlashPopup::new(&theme()); - popup.matches = vec![MatchedCommand { - name: "clear", - description: "wipe transcript", + popup.mode = Some(PopupMode::Name); + popup.rows = vec![PopupRow { + value: Cow::Borrowed("clear"), + description: Cow::Borrowed("wipe transcript"), matched_alias: Some("new"), }]; insta::assert_snapshot!(render_to_backend(&popup, 60)); @@ -363,10 +483,9 @@ mod tests { #[test] fn render_hidden_popup_emits_nothing() { - // The hidden-popup early-return in render() is otherwise - // unreached — App's draw method gates by height(), so the - // function only fires when the popup chose to be visible. - let popup = popup_with_query(None); + // The hidden-popup early-return in render() is otherwise unreached — App's draw method + // gates by height(), so the function only fires when the popup chose to be visible. + let popup = popup_with_state(None); let backend = render_to_backend(&popup, 60); let buf = backend.buffer(); for y in 0..buf.area.height { @@ -379,8 +498,8 @@ mod tests { #[test] 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. + // 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(); diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap new file mode 100644 index 00000000..dba86efa --- /dev/null +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap @@ -0,0 +1,10 @@ +--- +source: crates/oxide-code/src/tui/components/input/popup.rs +assertion_line: 423 +expression: "render_to_backend(&popup, 60)" +--- +"low Fastest, shallow reasoning " +"medium Balanced " +"high Deep reasoning " +"xhigh Opus 4.7 ladder ceiling " +"max Opus-only maximum " diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_prefix_filters_to_subset.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_prefix_filters_to_subset.snap new file mode 100644 index 00000000..e1da5326 --- /dev/null +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_prefix_filters_to_subset.snap @@ -0,0 +1,8 @@ +--- +source: crates/oxide-code/src/tui/components/input/popup.rs +assertion_line: 430 +expression: "render_to_backend(&popup, 60)" +--- +"mocha Catppuccin dark (default) " +"macchiato Catppuccin dark (medium) " +"material Material dark " 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_name_query_shows_each_command_once.snap similarity index 96% rename from crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap rename to crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_name_query_shows_each_command_once.snap index 6586d9ee..a3ca882e 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_name_query_shows_each_command_once.snap @@ -1,6 +1,6 @@ --- source: crates/oxide-code/src/tui/components/input/popup.rs -assertion_line: 296 +assertion_line: 409 expression: "render_to_backend(&popup, 60)" --- "/clear Reset the conversation context " diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_filtered_query_shows_only_matching_rows.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_filtered_name_query_shows_only_matching_rows.snap similarity index 89% rename from crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_filtered_query_shows_only_matching_rows.snap rename to crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_filtered_name_query_shows_only_matching_rows.snap index d32f1614..08c906d7 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_filtered_query_shows_only_matching_rows.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_filtered_name_query_shows_only_matching_rows.snap @@ -1,6 +1,6 @@ --- source: crates/oxide-code/src/tui/components/input/popup.rs -assertion_line: 325 +assertion_line: 416 expression: "render_to_backend(&popup, 60)" --- "/help List the available slash commands and their usage ..." diff --git a/docs/guide/slash-commands.md b/docs/guide/slash-commands.md index 43a9fdd6..15704654 100644 --- a/docs/guide/slash-commands.md +++ b/docs/guide/slash-commands.md @@ -21,12 +21,14 @@ Slash commands are built-in shortcuts that run client-side, without involving th When you type `/`, a two-column popup appears above the input: - **Up / Down** navigate the rows. -- **Tab** completes the selected command (`/` plus a trailing space). +- **Tab** completes the selected row. In name mode it inserts `/` plus a trailing space; in arg mode it replaces the typed prefix with the picked value. - **Enter** submits the current line. - **Esc** dismisses the popup. Matches are ranked by tier: name-prefix > alias-prefix > name-substring > alias-substring. Aliases display inline in the canonical row (`/clear (new, reset)`); typing any alias routes to the same impl. +After `/` plus a trailing space, the popup switches to **arg mode** for commands with a curated argument roster (`/model`, `/effort`, `/theme`): rows show valid values with descriptions, prefix-filtered as you type. + ## Sending a Literal `/foo` To send a message that _starts_ with a slash without invoking a command, double the leading slash. Typing `//etc` sends the literal `/etc`. diff --git a/docs/roadmap.md b/docs/roadmap.md index 3ba2a45b..d93d4df3 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -62,7 +62,7 @@ The direction is simple: ### Slash Commands - 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. +- Autocomplete popup on typing `/`, with ranked filter, Tab completion, and arg-mode completion for commands with curated rosters (`/model`, `/effort`, `/theme`). - 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. @@ -89,8 +89,7 @@ The direction is simple: Remaining surface beyond Working Today: - Session: `/resume`. -- 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. +- Deferred: `/compact`, `/cost`, `/login` / `/logout`, custom user commands, `/init` multi-phase flow. Persistence stance: `/model`, `/effort`, and `/theme` mutate session state only; restart returns to user-declared config. Cross-session persistence will land as an **explicit subcommand** writing to an **explicit user-opted-in path** — never a silent merge. (Rejects Claude Code's `~/.claude.json` mega-file pattern.) From 2c160d2380ac4933447913f43757fe32d2c510bf Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:39:42 +0800 Subject: [PATCH 04/25] refactor(slash): drop dormant ghost-text infrastructure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every command with `usage()` (`/model`, `/effort`, `/theme`) also publishes a non-empty `complete_arg`, so the popup-visibility gate always wins on empty prefix and the ghost-text fallback never paints. The render block, `ghost_text`, `ghost_text_from_state`, `normalize_placeholder`, and `arg_placeholder_for` were forward-compat for hypothetical commands with `usage()` but no roster — speculative code by CLAUDE.md scope discipline. Removed: - `slash::arg_placeholder_for` and the `usage()` lookup it wrapped. - `InputArea::ghost_text`, `ghost_text_from_state`, `normalize_placeholder`, and the per-render Paragraph paint at the cursor. - 9 ghost-text tests (3 sections in `input.rs`). - The roadmap edit that promoted "Inline argument placeholder" out of the deferred list — restored to deferred since the feature did not actually ship reachable. Adds 3 lines back to `input.rs` (the cursor_x triple-saturating chain that the ghost-text path had split). --- crates/oxide-code/src/slash.rs | 6 - crates/oxide-code/src/tui/components/input.rs | 153 +----------------- .../src/tui/components/input/popup.rs | 10 +- docs/roadmap.md | 1 + 4 files changed, 11 insertions(+), 159 deletions(-) diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 5e0afeb0..66a5f392 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -43,12 +43,6 @@ pub(crate) fn complete_arg_for(cmd_name: &str, prefix: &str) -> Vec]`). Returned by [`SlashCommand::usage`]; -/// rendered as dim ghost-text after `/cmd ` when there are no arg completions to show. -pub(crate) fn arg_placeholder_for(cmd_name: &str) -> Option<&'static str> { - registry::lookup_in(registry::BUILT_INS, cmd_name).and_then(registry::SlashCommand::usage) -} - /// Resolves and runs `parsed` against the built-in registry. pub(crate) fn dispatch( parsed: &Parsed, diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 9604912f..e597cd42 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -15,7 +15,7 @@ use unicode_width::UnicodeWidthStr; use self::popup::{PopupMode, SlashPopup}; use crate::agent::event::UserAction; -use crate::slash::{PopupState, arg_placeholder_for, popup_state}; +use crate::slash::popup_state; use crate::tui::glyphs::{USER_PROMPT_PREFIX, USER_PROMPT_PREFIX_WIDTH}; use crate::tui::theme::Theme; @@ -236,23 +236,11 @@ impl InputArea { }; self.scroll_top.set(top); - let raw_cursor_x = textarea_area.x.saturating_add(to_u16(sc.col)); - let cursor_x = raw_cursor_x.min(textarea_area.right().saturating_sub(1)); + let cursor_x = textarea_area + .x + .saturating_add(to_u16(sc.col)) + .min(textarea_area.right().saturating_sub(1)); let cursor_y = textarea_area.y + cursor_row - top; - - if let Some(token) = self.ghost_text() { - // Paint past the cursor, clipped to the textarea's right edge so wrapping mid-token - // doesn't bleed into the next row. - let width = textarea_area.right().saturating_sub(raw_cursor_x); - if width > 0 { - let area = Rect::new(raw_cursor_x, cursor_y, width, 1); - frame.render_widget( - Paragraph::new(Line::from(Span::styled(token, self.theme.dim()))), - area, - ); - } - } - frame.set_cursor_position((cursor_x, cursor_y)); } } @@ -347,25 +335,6 @@ impl InputArea { self.popup.set_state(state.as_ref()); } - /// Dim hint token (`[id]`, `[level]`, `[name]`) painted at the cursor in arg mode with an - /// empty prefix. Suppressed when the popup is visible — the popup's curated roster is a - /// richer hint. Fires when the popup yields (command publishes `usage()` without a - /// `complete_arg` impl). - fn ghost_text(&self) -> Option { - if self.popup.is_visible() { - return None; - } - let [single] = self.textarea.lines() else { - return None; - }; - let state = popup_state(single)?; - let usage = match &state { - PopupState::Arg { name, .. } => arg_placeholder_for(name), - PopupState::Name(_) => None, - }; - ghost_text_from_state(&state, usage) - } - fn is_scroll_key(event: &Event) -> bool { matches!( event, @@ -428,35 +397,6 @@ impl InputArea { // ── Free Functions ── -/// Pure decision: ghost-text fires only in `Arg` mode with an empty prefix and a `usage()` to -/// surface. The popup-visibility gate lives at the call site so this helper stays trivially -/// testable without a `SlashPopup` instance. -fn ghost_text_from_state(state: &PopupState<'_>, usage: Option<&str>) -> Option { - let PopupState::Arg { prefix, .. } = state else { - return None; - }; - if !prefix.is_empty() { - return None; - } - Some(normalize_placeholder(usage?)) -} - -/// Normalize a `usage()` syntax fragment to a single bracketed token. Strips one optional layer -/// each of `[...]` and `<...>` so `[]`, ``, and `[id]` all render as `[id]`. -fn normalize_placeholder(usage: &str) -> String { - let inner = usage.trim(); - let inner = inner - .strip_prefix('[') - .and_then(|s| s.strip_suffix(']')) - .unwrap_or(inner) - .trim(); - let inner = inner - .strip_prefix('<') - .and_then(|s| s.strip_suffix('>')) - .unwrap_or(inner); - format!("[{inner}]") -} - /// Lossy `usize → u16` for cursor / column positions, bounded by terminal dimensions. #[expect( clippy::cast_possible_truncation, @@ -1013,89 +953,6 @@ mod tests { assert!(input.popup_visible()); } - // ── ghost_text ── - - #[test] - fn ghost_text_suppresses_when_popup_visible() { - // `/model ` shows the curated roster — popup wins, no ghost-text. Pins the gate. - let mut input = test_input(); - type_text(&mut input, "/model "); - input.refresh_popup(); - assert!(input.popup_visible()); - assert_eq!(input.ghost_text(), None); - } - - #[test] - fn ghost_text_suppresses_in_name_mode() { - // Typing the command name itself is `Name`, never `Arg` — no hint applies. - let mut input = test_input(); - type_text(&mut input, "/mod"); - input.refresh_popup(); - assert_eq!(input.ghost_text(), None); - } - - // ── ghost_text_from_state ── - - #[test] - fn ghost_text_from_state_arg_with_empty_prefix_and_usage_renders_token() { - let state = PopupState::Arg { - name: "model", - prefix: "", - }; - assert_eq!( - ghost_text_from_state(&state, Some("[]")), - Some("[id]".to_owned()), - ); - } - - #[test] - fn ghost_text_from_state_non_empty_prefix_returns_none() { - // Once the user starts typing the arg, the hint disappears so it doesn't trail their text. - let state = PopupState::Arg { - name: "model", - prefix: "claude-", - }; - assert_eq!(ghost_text_from_state(&state, Some("[]")), None); - } - - #[test] - fn ghost_text_from_state_arg_without_usage_returns_none() { - // No usage to surface → nothing to hint. - let state = PopupState::Arg { - name: "init", - prefix: "", - }; - assert_eq!(ghost_text_from_state(&state, None), None); - } - - #[test] - fn ghost_text_from_state_name_mode_returns_none_even_with_usage() { - // Defensive: usage is read off the parsed name only in Arg mode, but pin the helper too. - let state = PopupState::Name("mo"); - assert_eq!(ghost_text_from_state(&state, Some("[]")), None); - } - - // ── normalize_placeholder ── - - #[test] - fn normalize_placeholder_strips_outer_brackets_and_inner_angles() { - // `[]` is the canonical form `usage()` returns today. - assert_eq!(normalize_placeholder("[]"), "[id]"); - } - - #[test] - fn normalize_placeholder_handles_missing_layers() { - // Forward-compatible with terser usage strings — strip what's there. - assert_eq!(normalize_placeholder(""), "[level]"); - assert_eq!(normalize_placeholder("[name]"), "[name]"); - assert_eq!(normalize_placeholder("topic"), "[topic]"); - } - - #[test] - fn normalize_placeholder_trims_whitespace_inside_brackets() { - assert_eq!(normalize_placeholder("[ ]"), "[id]"); - } - // ── render_popup ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index b5b4bd63..f5ca177f 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -86,8 +86,8 @@ impl SlashPopup { Some(PopupState::Arg { name, prefix }) => { let completions = complete_arg_for(name, prefix); if completions.is_empty() { - // Hidden popup — leaves room for the placeholder ghost-text. Mode is - // intentionally `None` so `is_visible()` returns false. + // No curated roster for this command — hide the popup so it doesn't + // intercept Tab / Enter while the user types the arg. (None, Vec::new()) } else { ( @@ -307,9 +307,9 @@ mod tests { } #[test] - fn set_state_arg_with_empty_roster_stays_hidden_for_ghost_text_fallback() { - // /init has no curated arg roster — popup must hide so the placeholder ghost-text path - // can render instead. + fn set_state_arg_with_empty_roster_stays_hidden() { + // /init has no curated arg roster — popup must hide so the user can type the arg + // without the popup intercepting Tab / Enter. let popup = arg_popup("init", ""); assert!(!popup.is_visible()); assert!(popup.mode().is_none()); diff --git a/docs/roadmap.md b/docs/roadmap.md index d93d4df3..51511320 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -89,6 +89,7 @@ The direction is simple: Remaining surface beyond Working Today: - Session: `/resume`. +- 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. Persistence stance: `/model`, `/effort`, and `/theme` mutate session state only; restart returns to user-declared config. Cross-session persistence will land as an **explicit subcommand** writing to an **explicit user-opted-in path** — never a silent merge. (Rejects Claude Code's `~/.claude.json` mega-file pattern.) From 827f9b739ea0359d995d04026aa6ffdc58eb9ec6 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:43:11 +0800 Subject: [PATCH 05/25] fix(slash): apply family-base filter at every model resolution tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous PR filtered family-bases from the substring-tier ambiguity listing but left the unique-suffix and exact-known-id tiers unfiltered. Result: `/model opus-4` and `/model claude-opus-4` silently selected the deprecated `claude-opus-4` family-base — undermining the listing-side filter the PR shipped. Filter family-bases at every selectable tier: - `is_known_model_id` → `is_selectable_known_id` (rejects exact matches on family-base rows). - Suffix-tier `candidates` predicate filters family-bases inline. - Substring-tier filter unchanged. Dated ids of family-bases (e.g. `claude-opus-4-20250514`) still resolve since the user explicitly opted into a specific snapshot — only direct typing of the family-base id is rejected. Tests: replaces `execute_unique_suffix_resolves_above_substring_ambiguity` (which pinned the bug) with two cases — one asserting ambiguity-error for family-bases with multiple current descendants, one asserting promotion to the unique current id when only one descendant exists. --- crates/oxide-code/src/slash/model.rs | 62 ++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index a8975789..636c8545 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -96,25 +96,25 @@ fn resolve_model_arg(arg: &str) -> Result { Ok(ResolvedModelId::new(format!("{base_id}{TAG_1M}"))) } -/// Resolution tiers (first hit wins): short alias → known canonical id → dated id (`-YYYYMMDD`) -/// → unique suffix → unique substring. Multi-match at the substring tier is an error so the user -/// disambiguates rather than silently landing on whichever model sorts first. +/// Resolution tiers (first hit wins): short alias → dated id (`-YYYYMMDD`) → known canonical +/// id → unique suffix → unique substring. Family-bases are filtered at every selectable tier so +/// users can't land on a deprecated row by typing its id directly — only dated ids of a +/// family-base remain reachable since the user explicitly opted into a specific snapshot. fn resolve_base(arg: &str) -> Result { if let Some(&(_, target)) = ALIASES.iter().find(|(name, _)| *name == arg) { return Ok(target.to_owned()); } - if is_known_model_id(arg) || is_dated_model_id(arg) { + if is_dated_model_id(arg) { return Ok(arg.to_owned()); } - if let [id] = candidates(|id| id.ends_with(arg)).as_slice() { + if is_selectable_known_id(arg) { + return Ok(arg.to_owned()); + } + let suffix_hits = candidates(|id| id.ends_with(arg) && !is_family_base(id)); + if let [id] = suffix_hits.as_slice() { return Ok((*id).to_owned()); } - // Family bases are listed only for `lookup` (marketing names of dated ids); listing them - // here would invite users to type a deprecated row. - let visible: Vec<&'static str> = candidates(|id| id.contains(arg)) - .into_iter() - .filter(|id| !is_family_base(id)) - .collect(); + let visible = candidates(|id| id.contains(arg) && !is_family_base(id)); match visible.as_slice() { [id] => Ok((*id).to_owned()), [_, ..] => Err(format!( @@ -129,8 +129,10 @@ fn resolve_base(arg: &str) -> Result { } } -fn is_known_model_id(arg: &str) -> bool { - MODELS.iter().any(|m| m.id_substr == arg) +fn is_selectable_known_id(arg: &str) -> bool { + MODELS + .iter() + .any(|m| m.id_substr == arg && !m.is_family_base) } fn is_dated_model_id(arg: &str) -> bool { @@ -346,9 +348,30 @@ mod tests { } #[test] - fn execute_unique_suffix_resolves_above_substring_ambiguity() { - let (_, outcome) = run_execute("opus-4"); - assert_eq!(outcome, Ok(swap_model("claude-opus-4"))); + fn execute_family_base_id_no_longer_silently_selects_deprecated() { + // `claude-opus-4` / `claude-sonnet-4` are kept in MODELS only so dated ids resolve + // their capabilities. Direct typing must NOT land on the deprecated base — opus-4 has + // four current descendants so it errors with ambiguity, sonnet-4 has two. + for arg in ["opus-4", "claude-opus-4", "claude-sonnet-4"] { + let (_, outcome) = run_execute(arg); + let msg = outcome.expect_err(&format!("`{arg}` must error")); + assert!( + msg.contains("matches"), + "ambiguity error for `{arg}`: {msg}" + ); + assert!( + !msg.contains("claude-opus-4,") && !msg.contains("claude-sonnet-4,"), + "deprecated family-base must not appear in candidate list: {msg}", + ); + } + } + + #[test] + fn execute_family_base_with_unique_current_descendant_promotes_to_it() { + // `claude-haiku-4` substrings only `claude-haiku-4-5` (after family-base filter), so the + // substring tier resolves uniquely instead of erroring. + let (_, outcome) = run_execute("claude-haiku-4"); + assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); } #[test] @@ -415,8 +438,13 @@ mod tests { } #[test] - fn resolve_model_arg_round_trips_every_models_row() { + fn resolve_model_arg_round_trips_every_selectable_models_row() { + // Family-bases are deprecated for direct selection — covered by the + // `family_base_id_no_longer_silently_selects_deprecated` test instead. for info in MODELS { + if info.is_family_base { + continue; + } assert_eq!( resolve_model_arg(info.id_substr) .as_ref() From b1708668ec5fa4db765b5197c5c0a572a475f388 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:45:32 +0800 Subject: [PATCH 06/25] fix(slash): reset popup selection on mode transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `set_state` clamped `selected` against the new row count but kept the index across mode boundaries — Name → Arg with `selected=5` left the cursor on row 5 of an unrelated roster instead of row 0. Reset on mode change; intra-mode prefix typing still clamps so the cursor sticks where the user parked it. --- .../src/tui/components/input/popup.rs | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index f5ca177f..acf09e63 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -106,9 +106,17 @@ impl SlashPopup { } } }; + // Mode transitions reset selection to row 0 — keeping `selected` from a different + // roster surfaces an arbitrary row instead of the canonical first pick. Intra-mode + // changes (query / prefix typing) clamp instead so the cursor sticks where it is. + let mode_changed = self.mode != mode; self.mode = mode; self.rows = rows; - self.selected = self.selected.min(self.rows.len().saturating_sub(1)); + self.selected = if mode_changed { + 0 + } else { + self.selected.min(self.rows.len().saturating_sub(1)) + }; } pub(crate) fn is_visible(&self) -> bool { @@ -297,6 +305,24 @@ mod tests { assert_eq!(popup.selected, 0); } + #[test] + fn set_state_resets_selection_on_mode_transition() { + // Park selection on a non-zero name-mode row, then transition to arg mode — the new + // mode's roster is unrelated, so selection must drop back to row 0 instead of pointing + // at whichever index happens to survive the clamp. + let mut popup = name_popup(""); + popup.select_next(); + popup.select_next(); + assert!(popup.selected >= 2, "park selection past row 0"); + + popup.set_state(Some(&PopupState::Arg { + name: "model", + prefix: "", + })); + assert!(matches!(popup.mode(), Some(PopupMode::Arg { .. }))); + assert_eq!(popup.selected, 0, "mode transition resets selection"); + } + #[test] fn set_state_arg_with_curated_roster_populates_arg_mode() { let popup = arg_popup("model", ""); From 111dfb134c6f1c4f7b0cc49f03b1b1066cbcd656 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:46:37 +0800 Subject: [PATCH 07/25] perf(slash): avoid per-row allocation in /model complete_arg description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Cow::Owned(display_name(id).into_owned())` forced a `String` allocation on every popup row even for the four `&'static` model ids whose marketing name is a `&'static str` literal — only `[1m]` rows need to allocate the " (1M context)" suffix. Pass the `Cow<'static, str>` straight through. --- crates/oxide-code/src/slash/model.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 636c8545..db567db5 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -50,7 +50,9 @@ impl SlashCommand for ModelCmd { .into_iter() .map(|id| ArgCompletion { value: Cow::Borrowed(*id), - description: Cow::Owned(display_name(id).into_owned()), + // `display_name` is `Cow<'static, str>` for `&'static` ids — the four bare rows + // borrow the marketing literal; only `[1m]` rows allocate the suffixed string. + description: display_name(id), }) .collect() } From 6d9d1f159f3ebc2d543e70307c19d3da3044bd6a Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:48:06 +0800 Subject: [PATCH 08/25] test(slash): cover Tab and Enter completion in popup name and arg modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `popup_completion_text` and `popup_submit_selected` shipped with zero direct coverage — every branch (Name vs Arg arms in both, plus the trailing-space contract on Tab) survived against trivial mutations: - drop trailing space: `format!("/{}", row.value)` - swap arg-mode args: `format!("/{} {} ", row.value, cmd)` - always return `None` from `popup_completion_text` Adds four unit tests (Tab name / Tab arg / Enter name / Enter arg) that each pin one branch via the public `handle_event` API. The headline typed-arg flow is now mutation-tested end to end. --- crates/oxide-code/src/tui/components/input.rs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index e597cd42..2ad9fd75 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -953,6 +953,64 @@ mod tests { assert!(input.popup_visible()); } + #[test] + fn handle_event_popup_tab_in_name_mode_inserts_slash_name_and_space() { + // Lone `/` shows the full roster; first row is `clear` (alphabetical). Tab inserts + // `/clear ` and dismisses — pins the trailing-space contract that lets the user start + // typing args immediately. + let mut input = input_with_popup(); + let selected = selected_value(&input); + let action = input.handle_event(&key(KeyCode::Tab, KeyModifiers::NONE)); + assert!(action.is_none(), "Tab is consumed, no UserAction"); + assert_eq!(input.textarea.lines(), vec![format!("/{selected} ")]); + assert!(!input.popup_visible(), "popup dismisses after Tab"); + } + + #[test] + fn handle_event_popup_tab_in_arg_mode_inserts_cmd_value_and_space() { + // Type `/effort ` to enter arg mode against the curated effort roster (first row: low). + // Tab must keep the `/effort` prefix and substitute the picked value plus trailing + // space — `/{cmd} {value} `, not `/{value} ` (which would drop the cmd context). + let mut input = test_input(); + type_text(&mut input, "/effort "); + input.refresh_popup(); + assert!(input.popup_visible(), "popup opens for /effort arg mode"); + let picked = selected_value(&input); + let action = input.handle_event(&key(KeyCode::Tab, KeyModifiers::NONE)); + assert!(action.is_none(), "Tab is consumed, no UserAction"); + assert_eq!(input.textarea.lines(), vec![format!("/effort {picked} ")]); + assert!(!input.popup_visible(), "popup dismisses after Tab"); + } + + #[test] + fn handle_event_popup_enter_in_name_mode_submits_slash_name() { + // Enter on a name-mode row submits `/{name}` (no trailing space — submission is final). + let mut input = input_with_popup(); + let selected = selected_value(&input); + let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!( + action, + Some(UserAction::SubmitPrompt(format!("/{selected}"))) + ); + assert!(input.textarea.is_empty(), "buffer clears on submit"); + } + + #[test] + fn handle_event_popup_enter_in_arg_mode_submits_cmd_value() { + // Enter on an arg-mode row submits `/{cmd} {value}` so the dispatcher receives the + // full typed-arg form, not just the picked value. + let mut input = test_input(); + type_text(&mut input, "/effort "); + input.refresh_popup(); + let picked = selected_value(&input); + let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!( + action, + Some(UserAction::SubmitPrompt(format!("/effort {picked}"))), + ); + assert!(input.textarea.is_empty(), "buffer clears on submit"); + } + // ── render_popup ── #[test] From e6371f06a3053adf6a931203c7e9236f1728c669 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:50:24 +0800 Subject: [PATCH 09/25] test(slash): close three mutation gaps left by transitive coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `is_family_base` only fired through the substring-tier ambiguity listing, which inspected only the opus case — flipping the bool on sonnet / haiku alone would have survived. Add direct tests covering every family-base + non-family + dated + unknown ids. - `popup_state` rejects empty names but no test exercised the early return: `/ arg`, `/ `, `/ ` would have parsed as `Arg { name: "" }` and routed empty lookups through the registry. - `complete_arg_appends_1m_context_suffix_for_1m_variants` only asserted the positive case — a mutation that always-appended ` (1M context)` regardless of `[1m]` would have survived. Add the negative assertion for the bare row. --- crates/oxide-code/src/model.rs | 29 +++++++++++++++++++++++++++ crates/oxide-code/src/slash/model.rs | 16 ++++++++++++--- crates/oxide-code/src/slash/parser.rs | 9 +++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index 519b12bc..d9e36950 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -580,6 +580,35 @@ mod tests { assert!(lookup("gpt-4").is_none()); } + // ── is_family_base ── + + #[test] + fn is_family_base_recognizes_each_unqualified_base_row() { + // Pinning per-id flips a mutation that flipped the bool only on `claude-opus-4` while + // leaving sonnet / haiku correct — the slash-resolver filter would still reject opus + // but silently accept sonnet/haiku family-bases. + for base in ["claude-opus-4", "claude-sonnet-4", "claude-haiku-4"] { + assert!(is_family_base(base), "{base} must flag as family-base"); + } + } + + #[test] + fn is_family_base_rejects_dated_versioned_and_unknown_ids() { + for non_base in [ + "claude-opus-4-7", + "claude-sonnet-4-6", + "claude-haiku-4-5", + "claude-opus-4-20250514", + "gpt-4", + "", + ] { + assert!( + !is_family_base(non_base), + "{non_base} must not flag as family-base", + ); + } + } + // ── marketing_name ── #[test] diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index db567db5..d7600cc8 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -222,19 +222,29 @@ mod tests { } #[test] - fn complete_arg_appends_1m_context_suffix_for_1m_variants() { - // The `[1m]` rows must surface the `(1M context)` marker so users can tell variants apart - // before committing. + fn complete_arg_appends_1m_context_suffix_only_for_1m_variants() { + // The `[1m]` rows must surface the `(1M context)` marker so users can tell variants + // apart; the bare row must NOT carry the suffix (mutation that always-appends would + // otherwise survive). let rows = arg_rows("claude-opus-4-7"); let one_m = rows .iter() .find(|(v, _)| v == "claude-opus-4-7[1m]") .expect("1M variant present"); + let plain = rows + .iter() + .find(|(v, _)| v == "claude-opus-4-7") + .expect("plain variant present"); assert!( one_m.1.contains("1M context"), "1M description: {:?}", one_m.1, ); + assert!( + !plain.1.contains("1M context"), + "plain description must not carry the 1M marker: {:?}", + plain.1, + ); } #[test] diff --git a/crates/oxide-code/src/slash/parser.rs b/crates/oxide-code/src/slash/parser.rs index 194b8d33..796f2ade 100644 --- a/crates/oxide-code/src/slash/parser.rs +++ b/crates/oxide-code/src/slash/parser.rs @@ -199,4 +199,13 @@ mod tests { // Disallowed-char names also fail in arg form so the popup doesn't show stale completions. assert!(popup_state("/foo! arg").is_none()); } + + #[test] + fn popup_state_empty_name_rejects_buffer() { + // `/` followed by whitespace has no name to dispatch — must not parse as `Arg { name: "" }`, + // which would route empty-string lookups through `complete_arg_for` / `arg_placeholder_for`. + assert!(popup_state("/ ").is_none()); + assert!(popup_state("/ ").is_none()); + assert!(popup_state("/ arg").is_none()); + } } From 15f15e641349d0cad212a59077c7c002708967fc Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:54:05 +0800 Subject: [PATCH 10/25] refactor(slash): pair effort description with the enum, drop ARG_ROSTER MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intensity ladder lived in three places — `Effort::ALL`, `Effort::VALID_VALUES`, and `slash::effort::ARG_ROSTER` — so adding a new tier required four-place updates in lockstep. Pair the description with the enum (`Effort::description`) and iterate `Effort::ALL` in `complete_arg`; the `as_str` accessor is promoted to `pub(crate)` so it no longer needs `Display::to_string()` allocation upstream. Also rewords the xhigh / max descriptions: previous copy advertised availability ("Opus 4.7 ladder ceiling" / "Opus-only maximum") while low / medium / high described behavior — inconsistent. New copy is behavior-focused across the ladder ("Extended thinking" / "Maximum thinking"); the executor still surfaces a per-model availability error if the user picks a tier the active model rejects. Snapshot updated for the two reworded rows. --- crates/oxide-code/src/config.rs | 14 +++++++++++++- crates/oxide-code/src/slash/effort.rs | 18 ++++-------------- ...ts_curated_roster_without_slash_prefix.snap | 6 +++--- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/oxide-code/src/config.rs b/crates/oxide-code/src/config.rs index 33bb2fce..9c74bef8 100644 --- a/crates/oxide-code/src/config.rs +++ b/crates/oxide-code/src/config.rs @@ -92,7 +92,7 @@ impl Effort { pub(crate) const ALL: [Self; 5] = [Self::Low, Self::Medium, Self::High, Self::Xhigh, Self::Max]; pub(crate) const VALID_VALUES: &str = "low, medium, high, xhigh, max"; - const fn as_str(self) -> &'static str { + pub(crate) const fn as_str(self) -> &'static str { match self { Self::Low => "low", Self::Medium => "medium", @@ -101,6 +101,18 @@ impl Effort { Self::Max => "max", } } + + /// One-line UX hint for the typed-arg popup. Behavior-focused so it stays consistent across + /// the ladder; per-tier model-availability checks live in the executor. + pub(crate) const fn description(self) -> &'static str { + match self { + Self::Low => "Fastest, shallow reasoning", + Self::Medium => "Balanced", + Self::High => "Deep reasoning", + Self::Xhigh => "Extended thinking", + Self::Max => "Maximum thinking", + } + } } impl fmt::Display for Effort { diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 5b30f187..1b4c342c 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -11,16 +11,6 @@ use crate::agent::event::UserAction; use crate::config::Effort; use crate::model::{capabilities_for, marketing_or_id}; -/// Curated autocomplete roster (intensity ladder; matches `Effort::ALL`). Descriptions kept -/// short so the popup doesn't dominate the screen. -const ARG_ROSTER: &[(&str, &str)] = &[ - ("low", "Fastest, shallow reasoning"), - ("medium", "Balanced"), - ("high", "Deep reasoning"), - ("xhigh", "Opus 4.7 ladder ceiling"), - ("max", "Opus-only maximum"), -]; - pub(super) struct EffortCmd; impl SlashCommand for EffortCmd { @@ -45,11 +35,11 @@ impl SlashCommand for EffortCmd { } fn complete_arg(&self, prefix: &str) -> Vec { - rank_by_prefix(ARG_ROSTER, prefix, |(value, _)| *value) + rank_by_prefix(&Effort::ALL, prefix, |level| level.as_str()) .into_iter() - .map(|(value, description)| ArgCompletion { - value: Cow::Borrowed(value), - description: Cow::Borrowed(description), + .map(|level| ArgCompletion { + value: Cow::Borrowed(level.as_str()), + description: Cow::Borrowed(level.description()), }) .collect() } diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap index dba86efa..a04a0f84 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_arg_mode_lists_curated_roster_without_slash_prefix.snap @@ -1,10 +1,10 @@ --- source: crates/oxide-code/src/tui/components/input/popup.rs -assertion_line: 423 +assertion_line: 449 expression: "render_to_backend(&popup, 60)" --- "low Fastest, shallow reasoning " "medium Balanced " "high Deep reasoning " -"xhigh Opus 4.7 ladder ceiling " -"max Opus-only maximum " +"xhigh Extended thinking " +"max Maximum thinking " From cdca74f13b683e8604e6a2d871af111a0d9a9b62 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 00:57:13 +0800 Subject: [PATCH 11/25] chore(slash): tighten visibility, cache popup label once per render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small cleanups surfaced by the PR review sweep: - `rank_by_prefix` is only called by sibling `slash::*` modules — drop to `pub(super)`. - `PopupMode` and `PopupRow` are only consumed by `InputArea` (parent of `popup`) — drop to `pub(super)` to match the surrounding shape. - `SlashPopup::render` computed `label(row)` twice per row — once for width measurement, once inside `render_row`. Cache once at the top of `render` and pass the owned `String` into `render_row`. Same contract; one allocation per visible row instead of two. - `PopupMode::Arg.cmd` doc reworded to drop the "X, not Y" antithesis tic per global CLAUDE.md phrasing rule. --- crates/oxide-code/src/slash/matcher.rs | 2 +- .../src/tui/components/input/popup.rs | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/crates/oxide-code/src/slash/matcher.rs b/crates/oxide-code/src/slash/matcher.rs index c7f47c97..570e8201 100644 --- a/crates/oxide-code/src/slash/matcher.rs +++ b/crates/oxide-code/src/slash/matcher.rs @@ -72,7 +72,7 @@ fn matching_alias(cmd: &dyn SlashCommand, pred: impl Fn(&str) -> bool) -> Option /// Two-tier prefix-then-substring rank over a curated roster, preserving declared order within /// each tier. Empty `query` returns every item in declared order. Lower-cased internally. -pub(crate) fn rank_by_prefix<'a, T>( +pub(super) fn rank_by_prefix<'a, T>( items: &'a [T], query: &str, key: impl Fn(&T) -> &str, diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index acf09e63..2a546ea3 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -28,9 +28,9 @@ const COLUMN_GAP: usize = 2; /// Mode-tagged so [`super::InputArea`] can format the right Tab-insertion text. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) enum PopupMode { +pub(super) enum PopupMode { Name, - /// Owned because the cmd name is parsed out of the live input buffer, not a `'static` slice. + /// Owned because the cmd name comes from the live input buffer. Arg { cmd: String, }, @@ -39,11 +39,11 @@ pub(crate) enum PopupMode { /// One popup row. `value` is the bare token (command name or arg value); the renderer adds /// `/` and any matched-alias suffix in name mode. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct PopupRow { - pub(crate) value: Cow<'static, str>, - pub(crate) description: Cow<'static, str>, +pub(super) struct PopupRow { + pub(super) value: Cow<'static, str>, + pub(super) description: Cow<'static, str>, /// Set in name mode when the typed query matched an alias rather than the canonical name. - pub(crate) matched_alias: Option<&'static str>, + pub(super) matched_alias: Option<&'static str>, } /// Slash-command autocomplete overlay. Empty `rows` means hidden. @@ -163,15 +163,17 @@ impl SlashPopup { let offset = self.scroll_offset(); let visible = self.rows.len().min(MAX_VISIBLE_ROWS); let window = &self.rows[offset..offset + visible]; - let label_width = window - .iter() - .map(|r| self.label(r).width()) - .max() - .unwrap_or(0); + // Compute labels once and reuse for both width measurement and row rendering — the two + // must agree exactly, and computing the same `String` twice per row invites drift. + let labels: Vec = window.iter().map(|r| self.label(r)).collect(); + let label_width = labels.iter().map(|l| l.width()).max().unwrap_or(0); let lines: Vec> = window .iter() + .zip(labels) .enumerate() - .map(|(i, r)| self.render_row(r, offset + i == self.selected, label_width, width)) + .map(|(i, (r, label))| { + self.render_row(r, label, offset + i == self.selected, label_width, width) + }) .collect(); frame.render_widget(Paragraph::new(lines).style(self.theme.surface()), area); } @@ -188,8 +190,9 @@ impl SlashPopup { self.selected.saturating_sub(pad).min(max_offset) } - /// Mode-aware left-column label. Used for both rendering and column-width computation, so - /// the two must agree exactly. + /// Mode-aware left-column label. The `None` arm is unreachable in practice — `render` only + /// calls this when `rows` is non-empty, which means `mode` is `Some` — but keep a graceful + /// fallback so a future caller doesn't surprise-panic. fn label(&self, row: &PopupRow) -> String { match &self.mode { Some(PopupMode::Name) => match row.matched_alias { @@ -203,11 +206,11 @@ impl SlashPopup { fn render_row( &self, row: &PopupRow, + label_text: String, selected: bool, label_width: usize, width: usize, ) -> Line<'static> { - let label_text = self.label(row); let pad = label_width.saturating_sub(label_text.width()); let row_style = row_style(&self.theme, selected); let desc_budget = width.saturating_sub(label_width + COLUMN_GAP); From cec1253246a089428c51ef58e360944cdf4c5124 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 01:59:43 +0800 Subject: [PATCH 12/25] revert(slash): restore ghost-text hint, drop popup-visibility gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hint and the popup serve different purposes: the popup lists curated picks; the hint confirms the slot's *shape* (`[id]`, `[level]`, `[name]`). Removing the hint on the basis that the popup hides it was the wrong test — they're complementary. Restores `arg_placeholder_for`, `InputArea::ghost_text`, `ghost_text_from_state`, `normalize_placeholder`, the per-render Paragraph paint, and the test sections; drops the prior `if popup.is_visible()` gate so both surface together when the arg prefix is still empty. --- crates/oxide-code/src/slash.rs | 5 + crates/oxide-code/src/tui/components/input.rs | 150 +++++++++++++++++- docs/roadmap.md | 1 - 3 files changed, 150 insertions(+), 6 deletions(-) diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 66a5f392..9848b120 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -43,6 +43,11 @@ pub(crate) fn complete_arg_for(cmd_name: &str, prefix: &str) -> Vec Option<&'static str> { + registry::lookup_in(registry::BUILT_INS, cmd_name).and_then(registry::SlashCommand::usage) +} + /// Resolves and runs `parsed` against the built-in registry. pub(crate) fn dispatch( parsed: &Parsed, diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 2ad9fd75..9c9dd2ed 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -15,7 +15,7 @@ use unicode_width::UnicodeWidthStr; use self::popup::{PopupMode, SlashPopup}; use crate::agent::event::UserAction; -use crate::slash::popup_state; +use crate::slash::{PopupState, arg_placeholder_for, popup_state}; use crate::tui::glyphs::{USER_PROMPT_PREFIX, USER_PROMPT_PREFIX_WIDTH}; use crate::tui::theme::Theme; @@ -236,11 +236,23 @@ impl InputArea { }; self.scroll_top.set(top); - let cursor_x = textarea_area - .x - .saturating_add(to_u16(sc.col)) - .min(textarea_area.right().saturating_sub(1)); + let raw_cursor_x = textarea_area.x.saturating_add(to_u16(sc.col)); + let cursor_x = raw_cursor_x.min(textarea_area.right().saturating_sub(1)); let cursor_y = textarea_area.y + cursor_row - top; + + if let Some(token) = self.ghost_text() { + // Paint past the cursor, clipped to the textarea's right edge so wrapping mid-token + // doesn't bleed into the next row. + let width = textarea_area.right().saturating_sub(raw_cursor_x); + if width > 0 { + let area = Rect::new(raw_cursor_x, cursor_y, width, 1); + frame.render_widget( + Paragraph::new(Line::from(Span::styled(token, self.theme.dim()))), + area, + ); + } + } + frame.set_cursor_position((cursor_x, cursor_y)); } } @@ -335,6 +347,21 @@ impl InputArea { self.popup.set_state(state.as_ref()); } + /// Dim placeholder token (`[id]`, `[level]`, `[name]`) painted at the cursor in arg mode with + /// an empty prefix. Coexists with the popup — the popup lists curated picks, the hint + /// confirms the slot's shape. + fn ghost_text(&self) -> Option { + let [single] = self.textarea.lines() else { + return None; + }; + let state = popup_state(single)?; + let usage = match &state { + PopupState::Arg { name, .. } => arg_placeholder_for(name), + PopupState::Name(_) => None, + }; + ghost_text_from_state(&state, usage) + } + fn is_scroll_key(event: &Event) -> bool { matches!( event, @@ -397,6 +424,34 @@ impl InputArea { // ── Free Functions ── +/// Fires only in `Arg` mode with an empty prefix and a `usage()` to surface; coexists with the +/// popup (no visibility gate). +fn ghost_text_from_state(state: &PopupState<'_>, usage: Option<&str>) -> Option { + let PopupState::Arg { prefix, .. } = state else { + return None; + }; + if !prefix.is_empty() { + return None; + } + Some(normalize_placeholder(usage?)) +} + +/// Normalize a `usage()` syntax fragment to a single bracketed token. Strips one optional layer +/// each of `[...]` and `<...>` so `[]`, ``, and `[id]` all render as `[id]`. +fn normalize_placeholder(usage: &str) -> String { + let inner = usage.trim(); + let inner = inner + .strip_prefix('[') + .and_then(|s| s.strip_suffix(']')) + .unwrap_or(inner) + .trim(); + let inner = inner + .strip_prefix('<') + .and_then(|s| s.strip_suffix('>')) + .unwrap_or(inner); + format!("[{inner}]") +} + /// Lossy `usize → u16` for cursor / column positions, bounded by terminal dimensions. #[expect( clippy::cast_possible_truncation, @@ -1011,6 +1066,91 @@ mod tests { assert!(input.textarea.is_empty(), "buffer clears on submit"); } + // ── ghost_text ── + + #[test] + fn ghost_text_shows_alongside_popup_when_arg_prefix_empty() { + // Popup and ghost coexist: popup lists curated picks; ghost confirms the slot's shape. + let mut input = test_input(); + type_text(&mut input, "/model "); + input.refresh_popup(); + assert!(input.popup_visible()); + assert_eq!(input.ghost_text(), Some("[id]".to_owned())); + } + + #[test] + fn ghost_text_suppresses_in_name_mode() { + let mut input = test_input(); + type_text(&mut input, "/mod"); + input.refresh_popup(); + assert_eq!(input.ghost_text(), None); + } + + #[test] + fn ghost_text_suppresses_once_user_types_arg_prefix() { + let mut input = test_input(); + type_text(&mut input, "/model claude-"); + input.refresh_popup(); + assert_eq!(input.ghost_text(), None); + } + + // ── ghost_text_from_state ── + + #[test] + fn ghost_text_from_state_arg_with_empty_prefix_and_usage_renders_token() { + let state = PopupState::Arg { + name: "model", + prefix: "", + }; + assert_eq!( + ghost_text_from_state(&state, Some("[]")), + Some("[id]".to_owned()), + ); + } + + #[test] + fn ghost_text_from_state_non_empty_prefix_returns_none() { + let state = PopupState::Arg { + name: "model", + prefix: "claude-", + }; + assert_eq!(ghost_text_from_state(&state, Some("[]")), None); + } + + #[test] + fn ghost_text_from_state_arg_without_usage_returns_none() { + let state = PopupState::Arg { + name: "init", + prefix: "", + }; + assert_eq!(ghost_text_from_state(&state, None), None); + } + + #[test] + fn ghost_text_from_state_name_mode_returns_none_even_with_usage() { + let state = PopupState::Name("mo"); + assert_eq!(ghost_text_from_state(&state, Some("[]")), None); + } + + // ── normalize_placeholder ── + + #[test] + fn normalize_placeholder_strips_outer_brackets_and_inner_angles() { + assert_eq!(normalize_placeholder("[]"), "[id]"); + } + + #[test] + fn normalize_placeholder_handles_missing_layers() { + assert_eq!(normalize_placeholder(""), "[level]"); + assert_eq!(normalize_placeholder("[name]"), "[name]"); + assert_eq!(normalize_placeholder("topic"), "[topic]"); + } + + #[test] + fn normalize_placeholder_trims_whitespace_inside_brackets() { + assert_eq!(normalize_placeholder("[ ]"), "[id]"); + } + // ── render_popup ── #[test] diff --git a/docs/roadmap.md b/docs/roadmap.md index 51511320..d93d4df3 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -89,7 +89,6 @@ The direction is simple: Remaining surface beyond Working Today: - Session: `/resume`. -- 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. Persistence stance: `/model`, `/effort`, and `/theme` mutate session state only; restart returns to user-declared config. Cross-session persistence will land as an **explicit subcommand** writing to an **explicit user-opted-in path** — never a silent merge. (Rejects Claude Code's `~/.claude.json` mega-file pattern.) From 8d3856b6879ef56038a4ccf8a28f351b0b2dfaa6 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 02:01:07 +0800 Subject: [PATCH 13/25] fix(slash): chain Tab completion from name mode into arg mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `popup_complete_to_buffer` closed the popup unconditionally, so Tab on `/mo` wrote `/model ` then left an empty surface — the curated arg roster never appeared, leaving the user to re-open the popup or guess. Reclassify against the new buffer instead: name-mode Tab on a command with a roster transitions straight into arg mode; arg-mode Tab still hides because the trailing `/cmd value ` matches no roster entry. --- crates/oxide-code/src/tui/components/input.rs | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 9c9dd2ed..995643c8 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -312,7 +312,10 @@ impl InputArea { return; }; self.set_text(&replacement); - self.popup.set_state(None); + // Reclassify against the new buffer: name-mode Tab on a command with a curated arg + // roster (`/model`, `/effort`, `/theme`) transitions straight into arg mode; arg-mode + // Tab leaves a trailing `/cmd value ` that no roster matches, so the popup hides. + self.refresh_popup(); } /// Mode-aware Tab insertion. Name mode replaces the buffer with `/{name} `; arg mode keeps @@ -1010,15 +1013,30 @@ mod tests { #[test] fn handle_event_popup_tab_in_name_mode_inserts_slash_name_and_space() { - // Lone `/` shows the full roster; first row is `clear` (alphabetical). Tab inserts - // `/clear ` and dismisses — pins the trailing-space contract that lets the user start - // typing args immediately. + // First alphabetical row is `clear` (no arg roster); Tab writes `/clear ` and the popup + // hides on reclassification — arg mode with an empty roster yields no rows. let mut input = input_with_popup(); let selected = selected_value(&input); let action = input.handle_event(&key(KeyCode::Tab, KeyModifiers::NONE)); assert!(action.is_none(), "Tab is consumed, no UserAction"); assert_eq!(input.textarea.lines(), vec![format!("/{selected} ")]); - assert!(!input.popup_visible(), "popup dismisses after Tab"); + assert!(!input.popup_visible(), "no arg roster → popup hides"); + } + + #[test] + fn handle_event_popup_tab_in_name_mode_chains_into_arg_mode_for_curated_commands() { + // Tab from `/mo` lands on `/model `; the arg-mode popup must reopen with the curated + // roster so the user keeps moving without re-opening the popup. + let mut input = test_input(); + type_text(&mut input, "/mo"); + input.refresh_popup(); + let action = input.handle_event(&key(KeyCode::Tab, KeyModifiers::NONE)); + assert!(action.is_none()); + assert_eq!(input.textarea.lines(), vec!["/model "]); + assert!( + input.popup_visible(), + "/model has a curated arg roster — popup chains into arg mode", + ); } #[test] From de48434e5cc8e0682947325295b9627e3b2bd07a Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 02:03:54 +0800 Subject: [PATCH 14/25] chore(slash): tighten verbose comments around popup state and Tab flow --- crates/oxide-code/src/tui/components/input.rs | 10 ++++------ crates/oxide-code/src/tui/components/input/popup.rs | 13 +++++-------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 995643c8..33255aad 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -312,9 +312,8 @@ impl InputArea { return; }; self.set_text(&replacement); - // Reclassify against the new buffer: name-mode Tab on a command with a curated arg - // roster (`/model`, `/effort`, `/theme`) transitions straight into arg mode; arg-mode - // Tab leaves a trailing `/cmd value ` that no roster matches, so the popup hides. + // Reclassify against the new buffer: name-mode Tab on a curated-roster command chains + // into arg mode; arg-mode Tab leaves `/cmd value ` (no match), so the popup hides. self.refresh_popup(); } @@ -1041,9 +1040,8 @@ mod tests { #[test] fn handle_event_popup_tab_in_arg_mode_inserts_cmd_value_and_space() { - // Type `/effort ` to enter arg mode against the curated effort roster (first row: low). - // Tab must keep the `/effort` prefix and substitute the picked value plus trailing - // space — `/{cmd} {value} `, not `/{value} ` (which would drop the cmd context). + // Tab keeps the `/effort` prefix and substitutes the picked value plus a trailing space + // — `/{cmd} {value} `, not `/{value} ` (which would drop cmd context). let mut input = test_input(); type_text(&mut input, "/effort "); input.refresh_popup(); diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 2a546ea3..cb64e1e7 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -106,9 +106,8 @@ impl SlashPopup { } } }; - // Mode transitions reset selection to row 0 — keeping `selected` from a different - // roster surfaces an arbitrary row instead of the canonical first pick. Intra-mode - // changes (query / prefix typing) clamp instead so the cursor sticks where it is. + // Mode transitions reset to row 0 (rosters are unrelated). Intra-mode query / prefix + // changes clamp instead so the cursor sticks. let mode_changed = self.mode != mode; self.mode = mode; self.rows = rows; @@ -163,8 +162,7 @@ impl SlashPopup { let offset = self.scroll_offset(); let visible = self.rows.len().min(MAX_VISIBLE_ROWS); let window = &self.rows[offset..offset + visible]; - // Compute labels once and reuse for both width measurement and row rendering — the two - // must agree exactly, and computing the same `String` twice per row invites drift. + // Cache labels so width measurement and row rendering can't drift. let labels: Vec = window.iter().map(|r| self.label(r)).collect(); let label_width = labels.iter().map(|l| l.width()).max().unwrap_or(0); let lines: Vec> = window @@ -190,9 +188,8 @@ impl SlashPopup { self.selected.saturating_sub(pad).min(max_offset) } - /// Mode-aware left-column label. The `None` arm is unreachable in practice — `render` only - /// calls this when `rows` is non-empty, which means `mode` is `Some` — but keep a graceful - /// fallback so a future caller doesn't surprise-panic. + /// Mode-aware left-column label. `None` arm is a defensive fallback — `render` gates on + /// non-empty `rows`, which keeps `mode` `Some`. fn label(&self, row: &PopupRow) -> String { match &self.mode { Some(PopupMode::Name) => match row.matched_alias { From 5ffa9dbf724a102891d26f8f10b945b7eb78784a Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 15:19:12 +0800 Subject: [PATCH 15/25] test(input): cover ghost-text render branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Render-time paint of the dim `[id]` placeholder was only exercised through the unit test on `ghost_text()` itself; the per-render Paragraph block inside `render` was uncovered (8 lines of patch). Two snapshot-style tests pin the paint branch (`/model ` → buffer contains `[id]`) and the suppression branch (`/model claude-` → buffer does not). --- crates/oxide-code/src/tui/components/input.rs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 33255aad..6978c053 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -767,6 +767,35 @@ mod tests { insta::assert_snapshot!(render_to_backend(&input, 30, 5)); } + #[test] + fn render_paints_ghost_text_at_cursor_in_arg_mode_with_empty_prefix() { + // `/model ` puts the input in arg mode with an empty prefix; the dim `[id]` placeholder + // must paint into the rendered buffer (covers the per-render Paragraph paint). + let mut input = test_input(); + type_text(&mut input, "/model "); + input.refresh_popup(); + let backend = render_to_backend(&input, 60, 3); + let rendered = format!("{backend}"); + assert!( + rendered.contains("[id]"), + "ghost-text `[id]` should paint at the cursor: {rendered}", + ); + } + + #[test] + fn render_omits_ghost_text_once_user_types_arg_prefix() { + // Pins the suppression branch: any non-empty arg prefix hides the placeholder. + let mut input = test_input(); + type_text(&mut input, "/model claude-"); + input.refresh_popup(); + let backend = render_to_backend(&input, 60, 3); + let rendered = format!("{backend}"); + assert!( + !rendered.contains("[id]"), + "ghost-text must hide once prefix is typed: {rendered}", + ); + } + #[test] fn render_advances_scroll_top_when_cursor_below_viewport() { // After typing N+1 logical lines, the cursor sits at row N From 4c02f9fa742a42d8f889d6de477d145cb6fb29da Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 16:29:51 +0800 Subject: [PATCH 16/25] fix(slash): respect typed buffer on Enter when popup is a suggester MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The popup is the picker only when there's no typed ground truth: name mode or arg mode with an empty prefix. With a typed arg prefix it's a Tab- completion hint and Enter must submit the buffer literally — typing `/model claude-opus-4` then Enter no longer silently rewrites to the popup-highlighted `claude-opus-4-7`. --- crates/oxide-code/src/tui/components/input.rs | 61 ++++++++++++++----- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 6978c053..b15ae756 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -299,10 +299,14 @@ impl InputArea { self.popup_complete_to_buffer(); PopupKey::Consumed } - KeyCode::Enter if modifiers.is_empty() => match self.popup_submit_selected() { - Some(action) => PopupKey::Action(action), - None => PopupKey::Consumed, - }, + KeyCode::Enter if modifiers.is_empty() => { + if self.popup_acts_as_picker() + && let Some(action) = self.popup_submit_selected() + { + return PopupKey::Action(action); + } + PopupKey::Pass + } _ => PopupKey::Pass, } } @@ -312,13 +316,10 @@ impl InputArea { return; }; self.set_text(&replacement); - // Reclassify against the new buffer: name-mode Tab on a curated-roster command chains - // into arg mode; arg-mode Tab leaves `/cmd value ` (no match), so the popup hides. self.refresh_popup(); } - /// Mode-aware Tab insertion. Name mode replaces the buffer with `/{name} `; arg mode keeps - /// the typed `/cmd ` and substitutes the prefix with the picked value plus a trailing space. + /// Tab-insertion text. Name mode → `/{name} `; arg mode → `/{cmd} {value} `. fn popup_completion_text(&self) -> Option { let row = self.popup.selected()?; Some(match self.popup.mode()? { @@ -327,6 +328,19 @@ impl InputArea { }) } + /// Picker mode: Enter commits the popup row. Suggester mode (arg with non-empty prefix): + /// Enter passes through to literal submit. + fn popup_acts_as_picker(&self) -> bool { + let [single] = self.textarea.lines() else { + return false; + }; + match popup_state(single) { + Some(PopupState::Name(_)) => true, + Some(PopupState::Arg { prefix, .. }) => prefix.is_empty(), + None => false, + } + } + fn popup_submit_selected(&mut self) -> Option { let row = self.popup.selected()?; let submission = match self.popup.mode()? { @@ -419,6 +433,7 @@ impl InputArea { self.textarea.select_all(); self.textarea.cut(); self.scroll_top.set(0); + self.popup.set_state(None); Some(UserAction::SubmitPrompt(trimmed)) } @@ -1083,8 +1098,7 @@ mod tests { } #[test] - fn handle_event_popup_enter_in_name_mode_submits_slash_name() { - // Enter on a name-mode row submits `/{name}` (no trailing space — submission is final). + fn handle_event_popup_enter_in_name_mode_commits_picked_row() { let mut input = input_with_popup(); let selected = selected_value(&input); let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); @@ -1092,13 +1106,11 @@ mod tests { action, Some(UserAction::SubmitPrompt(format!("/{selected}"))) ); - assert!(input.textarea.is_empty(), "buffer clears on submit"); + assert!(input.textarea.is_empty()); } #[test] - fn handle_event_popup_enter_in_arg_mode_submits_cmd_value() { - // Enter on an arg-mode row submits `/{cmd} {value}` so the dispatcher receives the - // full typed-arg form, not just the picked value. + fn handle_event_popup_enter_in_arg_mode_with_empty_prefix_commits_picked_row() { let mut input = test_input(); type_text(&mut input, "/effort "); input.refresh_popup(); @@ -1106,9 +1118,26 @@ mod tests { let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); assert_eq!( action, - Some(UserAction::SubmitPrompt(format!("/effort {picked}"))), + Some(UserAction::SubmitPrompt(format!("/effort {picked}"))) + ); + assert!(input.textarea.is_empty()); + } + + #[test] + fn handle_event_popup_enter_in_arg_mode_with_typed_prefix_submits_buffer_literally() { + // Typed buffer wins over popup highlight; Tab is the explicit "promote popup pick" key. + let mut input = test_input(); + type_text(&mut input, "/model claude-opus-4"); + input.refresh_popup(); + assert!(input.popup_visible()); + assert_ne!(selected_value(&input), "claude-opus-4"); + let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!( + action, + Some(UserAction::SubmitPrompt("/model claude-opus-4".to_owned())), ); - assert!(input.textarea.is_empty(), "buffer clears on submit"); + assert!(input.textarea.is_empty()); + assert!(!input.popup_visible()); } // ── ghost_text ── From aac1b91b9de4ea1be23886649a1e66b3cdb4aa3a Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 16:30:01 +0800 Subject: [PATCH 17/25] refactor(model): drop retired rows, simplify capabilities, render error as list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove `claude-opus-4` and `claude-sonnet-4` (retired April 2026) and `claude-haiku-4` (never released) from MODELS. `is_family_base` and the resolver's family-base filter go with them — typing a removed id now falls through to the same ambiguity / unknown-model error path as any other foreign id. - Collapse `effort` / `effort_max` / `effort_xhigh` booleans into a single `supported_efforts: &'static [Effort]` slice. `accepts_effort`, `clamp_effort`, and `default_effort` derive from the slice; default picks the highest non-`Max` level so Max stays opt-in. - Rename the `marketing` field to `display_name` for clarity at the call site. - Render `/model` ambiguity and unknown-model errors as a markdown bullet list of `id — display name` rows so the supported roster is scannable. --- .../oxide-code/src/client/anthropic/betas.rs | 2 +- crates/oxide-code/src/model.rs | 314 ++++++------------ crates/oxide-code/src/prompt/environment.rs | 21 +- crates/oxide-code/src/slash/effort.rs | 2 +- crates/oxide-code/src/slash/effort_slider.rs | 4 +- crates/oxide-code/src/slash/model.rs | 119 +++---- crates/oxide-code/src/slash/picker.rs | 8 +- 7 files changed, 163 insertions(+), 307 deletions(-) diff --git a/crates/oxide-code/src/client/anthropic/betas.rs b/crates/oxide-code/src/client/anthropic/betas.rs index 419c20b6..e9ce667a 100644 --- a/crates/oxide-code/src/client/anthropic/betas.rs +++ b/crates/oxide-code/src/client/anthropic/betas.rs @@ -47,7 +47,7 @@ pub(super) fn compute_betas( out.push(CONTEXT_MANAGEMENT_BETA_HEADER); } out.push(PROMPT_CACHING_SCOPE_BETA_HEADER); - if caps.effort { + if caps.has_effort() { out.push(EFFORT_BETA_HEADER); } } diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index d9e36950..abc86176 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -11,9 +11,7 @@ use crate::config::Effort; pub(crate) struct ModelInfo { /// First substring match in [`MODELS`] wins; ordering matters. pub(crate) id_substr: &'static str, - /// Unqualified fallback row that resolves dated / legacy ids; user-facing listings hide it. - pub(crate) is_family_base: bool, - pub(crate) marketing: &'static str, + pub(crate) display_name: &'static str, pub(crate) cutoff: Option<&'static str>, pub(crate) capabilities: Capabilities, } @@ -22,11 +20,10 @@ pub(crate) struct ModelInfo { /// Per-model gate set consumed by the wire-builder (header + body fields), the slash commands /// (`/effort` rejects unsupported tiers, `/model` rejects `[1m]` on non-1M models), and the -/// effort picker (renders only the supported ladder). Every field is a Boolean because each one -/// maps 1:1 to a distinct upstream switch — combining them would obscure the source of truth. +/// effort picker (renders only the supported ladder). #[expect( clippy::struct_excessive_bools, - reason = "seven independent capability flags — each maps 1:1 to a separate upstream `modelSupports*` predicate or a per-version allowlist; a bitflag or state-machine refactor would add indirection without any expressiveness gain" + reason = "each flag maps 1:1 to a separate upstream `modelSupports*` predicate or per-version allowlist; bitflags add indirection without expressiveness" )] #[derive(Clone, Copy, Default)] pub(crate) struct Capabilities { @@ -34,12 +31,8 @@ pub(crate) struct Capabilities { pub(crate) context_management: bool, /// `context-1m-2025-08-07` beta. pub(crate) context_1m: bool, - /// `output_config.effort` at low / medium / high. - pub(crate) effort: bool, - /// `effort = "max"`. Opus-only. - pub(crate) effort_max: bool, - /// `effort = "xhigh"`. Opus 4.7 only. - pub(crate) effort_xhigh: bool, + /// `output_config.effort` levels accepted upstream. Empty when the model rejects `effort`. + pub(crate) supported_efforts: &'static [Effort], /// `structured-outputs-2025-12-15` beta. pub(crate) structured_outputs: bool, } @@ -48,201 +41,127 @@ pub(crate) struct Capabilities { /// Most-specific substring first. No inheritance between rows. pub(crate) const MODELS: &[ModelInfo] = &[ - // Inherits 4.6 as a monotonic projection; `effort_xhigh` is the 4.7-only addition. ModelInfo { id_substr: "claude-opus-4-7", - is_family_base: false, - marketing: "Claude Opus 4.7", + display_name: "Claude Opus 4.7", cutoff: Some("January 2026"), capabilities: Capabilities { interleaved_thinking: true, context_management: true, context_1m: true, - effort: true, - effort_max: true, - effort_xhigh: true, + supported_efforts: &[ + Effort::Low, + Effort::Medium, + Effort::High, + Effort::Xhigh, + Effort::Max, + ], structured_outputs: true, }, }, ModelInfo { id_substr: "claude-opus-4-6", - is_family_base: false, - marketing: "Claude Opus 4.6", + display_name: "Claude Opus 4.6", cutoff: Some("May 2025"), capabilities: Capabilities { interleaved_thinking: true, context_management: true, context_1m: true, - effort: true, - effort_max: true, - effort_xhigh: false, + supported_efforts: &[Effort::Low, Effort::Medium, Effort::High, Effort::Max], structured_outputs: true, }, }, ModelInfo { id_substr: "claude-sonnet-4-6", - is_family_base: false, - marketing: "Claude Sonnet 4.6", + display_name: "Claude Sonnet 4.6", cutoff: Some("August 2025"), capabilities: Capabilities { interleaved_thinking: true, context_management: true, context_1m: true, - effort: true, - // `max` is Opus-only; Sonnet 4.6 400s on it. - effort_max: false, - effort_xhigh: false, + supported_efforts: &[Effort::Low, Effort::Medium, Effort::High], structured_outputs: true, }, }, ModelInfo { id_substr: "claude-opus-4-5", - is_family_base: false, - marketing: "Claude Opus 4.5", + display_name: "Claude Opus 4.5", cutoff: Some("May 2025"), capabilities: Capabilities { interleaved_thinking: true, context_management: true, context_1m: false, - effort: false, - effort_max: false, - effort_xhigh: false, + supported_efforts: &[], structured_outputs: true, }, }, ModelInfo { id_substr: "claude-sonnet-4-5", - is_family_base: false, - marketing: "Claude Sonnet 4.5", + display_name: "Claude Sonnet 4.5", cutoff: Some("January 2025"), capabilities: Capabilities { interleaved_thinking: true, context_management: true, context_1m: true, - effort: false, - effort_max: false, - effort_xhigh: false, + supported_efforts: &[], structured_outputs: true, }, }, ModelInfo { id_substr: "claude-haiku-4-5", - is_family_base: false, - marketing: "Claude Haiku 4.5", + display_name: "Claude Haiku 4.5", cutoff: Some("February 2025"), capabilities: Capabilities { // 3P gateways 400 on `interleaved-thinking` for Haiku 4.5. interleaved_thinking: false, context_management: true, context_1m: false, - effort: false, - effort_max: false, - effort_xhigh: false, + supported_efforts: &[], structured_outputs: true, }, }, ModelInfo { id_substr: "claude-opus-4-1", - is_family_base: false, - marketing: "Claude Opus 4.1", + display_name: "Claude Opus 4.1", cutoff: Some("January 2025"), capabilities: Capabilities { interleaved_thinking: true, context_management: true, context_1m: false, - effort: false, - effort_max: false, - effort_xhigh: false, + supported_efforts: &[], structured_outputs: true, }, }, - // Unqualified base (`claude-opus-4`, `-4-0`, `-4-20250514`); structured outputs arrived in 4.1. - ModelInfo { - id_substr: "claude-opus-4", - is_family_base: true, - marketing: "Claude Opus 4", - cutoff: Some("January 2025"), - capabilities: Capabilities { - interleaved_thinking: true, - context_management: true, - context_1m: false, - effort: false, - effort_max: false, - effort_xhigh: false, - structured_outputs: false, - }, - }, - // Sonnet 4 base: all Sonnet 4.x carry 1M per upstream's `sonnet-4` substring rule. - ModelInfo { - id_substr: "claude-sonnet-4", - is_family_base: true, - marketing: "Claude Sonnet 4", - cutoff: Some("January 2025"), - capabilities: Capabilities { - interleaved_thinking: true, - context_management: true, - context_1m: true, - effort: false, - effort_max: false, - effort_xhigh: false, - structured_outputs: false, - }, - }, - ModelInfo { - id_substr: "claude-haiku-4", - is_family_base: true, - marketing: "Claude Haiku 4", - cutoff: Some("February 2025"), - capabilities: Capabilities { - interleaved_thinking: false, - context_management: true, - context_1m: false, - effort: false, - effort_max: false, - effort_xhigh: false, - structured_outputs: false, - }, - }, ]; impl Capabilities { - /// Whether the model accepts a given `output_config.effort` value. Splits on tier because the - /// Anthropic API rejects unsupported levels with a 400, not a silent clamp — `xhigh` is 4.7 - /// only, `max` is Opus-only. + /// Whether `output_config.effort` is sent at all for this model. + pub(crate) fn has_effort(self) -> bool { + !self.supported_efforts.is_empty() + } + + /// Whether the model accepts `level`. Anthropic 400s on unsupported tiers — no silent clamp. pub(crate) fn accepts_effort(self, level: Effort) -> bool { - match level { - Effort::Low | Effort::Medium | Effort::High => self.effort, - Effort::Xhigh => self.effort_xhigh, - Effort::Max => self.effort_max, - } + self.supported_efforts.contains(&level) } - /// Highest accepted level ≤ `pick`. `None` when the model doesn't support effort. + /// Highest accepted level ≤ `pick`. `None` when the model rejects effort entirely. pub(crate) fn clamp_effort(self, pick: Effort) -> Option { - if !self.effort { - return None; - } - [ - Effort::Max, - Effort::Xhigh, - Effort::High, - Effort::Medium, - Effort::Low, - ] - .into_iter() - .find(|&level| level <= pick && self.accepts_effort(level)) - } - - /// Highest tier the model accepts when the user hasn't picked one — `xhigh` for 4.7, `high` - /// for other effort-capable models, `None` otherwise. + self.supported_efforts + .iter() + .copied() + .rev() + .find(|&level| level <= pick) + } + + /// Default tier when the user hasn't picked one. `Max` is opt-in, so the implicit ceiling is + /// the highest non-`Max` supported level. pub(crate) fn default_effort(self) -> Option { - if self.effort_xhigh { - Some(Effort::Xhigh) - } else if self.effort { - Some(Effort::High) - } else { - None - } + self.supported_efforts + .iter() + .copied() + .rev() + .find(|&level| level != Effort::Max) } /// Clamps `pick` when present, otherwise falls back to [`Self::default_effort`]. @@ -282,13 +201,6 @@ pub(crate) fn lookup(model: &str) -> Option<&'static ModelInfo> { MODELS.iter().find(|info| model.contains(info.id_substr)) } -/// True when `id` matches a family-base row exactly. User-facing listings filter against this. -pub(crate) fn is_family_base(id: &str) -> bool { - MODELS - .iter() - .any(|info| info.id_substr == id && info.is_family_base) -} - /// Capabilities for `model`, falling back to [`Capabilities::default`] for unknown ids. pub(crate) fn capabilities_for(model: &str) -> Capabilities { lookup(model) @@ -297,7 +209,7 @@ pub(crate) fn capabilities_for(model: &str) -> Capabilities { } pub(crate) fn marketing_name(model: &str) -> Option<&'static str> { - lookup(model).map(|info| info.marketing) + lookup(model).map(|info| info.display_name) } /// Marketing name when known, raw id otherwise. @@ -323,15 +235,14 @@ mod tests { #[test] fn capability_flags_match_upstream_substring_predicates() { - // Locks substring-derived flags to upstream's `modelSupports*` predicates. Allowlist - // flags have their own tests. Opus 4.7 postdates the predicate set and is skipped. + // Locks substring-derived flags to upstream's `modelSupports*` predicates. Opus 4.7 + // postdates the predicate set and is skipped. for info in MODELS { if info.id_substr == "claude-opus-4-7" { continue; } let m = info.id_substr; let is_opus_or_sonnet_4 = m.contains("opus-4") || m.contains("sonnet-4"); - // haiku-4 is not in modelSupportsISP let expect_interleaved_thinking = is_opus_or_sonnet_4; let expect_context_management = is_opus_or_sonnet_4 || m.contains("haiku-4"); let expect_context_1m = m.contains("claude-sonnet-4") || m.contains("opus-4-6"); @@ -339,20 +250,14 @@ mod tests { assert_eq!( info.capabilities.interleaved_thinking, expect_interleaved_thinking, - "{m}: interleaved_thinking should match modelSupportsISP", + "{m}" ); assert_eq!( info.capabilities.context_management, expect_context_management, - "{m}: context_management should match modelSupportsContextManagement", - ); - assert_eq!( - info.capabilities.context_1m, expect_context_1m, - "{m}: context_1m should match modelSupports1M", - ); - assert_eq!( - info.capabilities.effort, expect_effort, - "{m}: effort should match modelSupportsEffort", + "{m}" ); + assert_eq!(info.capabilities.context_1m, expect_context_1m, "{m}"); + assert_eq!(info.capabilities.has_effort(), expect_effort, "{m}"); } } @@ -363,9 +268,8 @@ mod tests { assert!(caps.interleaved_thinking); assert!(caps.context_management); assert!(caps.context_1m); - assert!(caps.effort); - assert!(caps.effort_max); - assert!(caps.effort_xhigh); + assert!(caps.accepts_effort(Effort::Xhigh)); + assert!(caps.accepts_effort(Effort::Max)); assert!(caps.structured_outputs); for other in [ @@ -377,19 +281,24 @@ mod tests { "claude-opus-4-1", ] { assert!( - !lookup(other).unwrap().capabilities.effort_xhigh, - "{other} must not claim effort_xhigh — it 400s on non-4.7", + !lookup(other) + .unwrap() + .capabilities + .accepts_effort(Effort::Xhigh), + "{other} must not accept Xhigh — it 400s on non-4.7", ); } } #[test] fn effort_max_is_opus_only() { - // `max` is Opus-only; Sonnet 4.6 supports base `effort` but 400s on `max`. for supported in ["claude-opus-4-7", "claude-opus-4-6"] { assert!( - lookup(supported).unwrap().capabilities.effort_max, - "{supported} should claim effort_max", + lookup(supported) + .unwrap() + .capabilities + .accepts_effort(Effort::Max), + "{supported}", ); } for unsupported in [ @@ -398,20 +307,19 @@ mod tests { "claude-sonnet-4-5", "claude-haiku-4-5", "claude-opus-4-1", - "claude-opus-4", - "claude-sonnet-4", - "claude-haiku-4", ] { assert!( - !lookup(unsupported).unwrap().capabilities.effort_max, - "{unsupported} must not claim effort_max", + !lookup(unsupported) + .unwrap() + .capabilities + .accepts_effort(Effort::Max), + "{unsupported}", ); } } #[test] fn structured_outputs_flag_tracks_upstream_allowlist() { - // Per-version allowlist: Opus 4.1/4.5/4.6 (+ our 4.7 bump), Sonnet 4.5/4.6, Haiku 4.5. for supported in [ "claude-opus-4-7", "claude-opus-4-6", @@ -423,13 +331,7 @@ mod tests { ] { assert!( lookup(supported).unwrap().capabilities.structured_outputs, - "{supported} should claim structured outputs per upstream", - ); - } - for unsupported in ["claude-opus-4", "claude-sonnet-4", "claude-haiku-4"] { - assert!( - !lookup(unsupported).unwrap().capabilities.structured_outputs, - "{unsupported} fallback row must not claim structured outputs", + "{supported}" ); } } @@ -561,51 +463,30 @@ mod tests { // ── lookup ── #[test] - fn lookup_matches_most_specific_row_before_family_base() { + fn lookup_picks_first_matching_substring_row() { let info = lookup("claude-opus-4-6").unwrap(); - assert_eq!(info.marketing, "Claude Opus 4.6"); - assert!(info.capabilities.effort); + assert_eq!(info.display_name, "Claude Opus 4.6"); + assert!(info.capabilities.has_effort()); } #[test] fn lookup_ignores_1m_suffix_tag_for_matching() { // `[1m]` is a client-side opt-in marker; substring match still finds the base row. let info = lookup("claude-opus-4-6[1m]").unwrap(); - assert_eq!(info.marketing, "Claude Opus 4.6"); - } - - #[test] - fn lookup_unknown_model_family_is_absent() { - assert!(lookup("claude-opus-5-0").is_none()); - assert!(lookup("gpt-4").is_none()); + assert_eq!(info.display_name, "Claude Opus 4.6"); } - // ── is_family_base ── - #[test] - fn is_family_base_recognizes_each_unqualified_base_row() { - // Pinning per-id flips a mutation that flipped the bool only on `claude-opus-4` while - // leaving sonnet / haiku correct — the slash-resolver filter would still reject opus - // but silently accept sonnet/haiku family-bases. - for base in ["claude-opus-4", "claude-sonnet-4", "claude-haiku-4"] { - assert!(is_family_base(base), "{base} must flag as family-base"); - } - } - - #[test] - fn is_family_base_rejects_dated_versioned_and_unknown_ids() { - for non_base in [ - "claude-opus-4-7", - "claude-sonnet-4-6", - "claude-haiku-4-5", + fn lookup_unknown_or_retired_model_family_is_absent() { + for unknown in [ + "claude-opus-5-0", + "claude-opus-4", + "claude-sonnet-4", + "claude-haiku-4", "claude-opus-4-20250514", "gpt-4", - "", ] { - assert!( - !is_family_base(non_base), - "{non_base} must not flag as family-base", - ); + assert!(lookup(unknown).is_none(), "{unknown} must not resolve"); } } @@ -613,26 +494,21 @@ mod tests { #[test] fn marketing_name_known_models() { - assert_eq!(marketing_name("claude-opus-4-7"), Some("Claude Opus 4.7")); - assert_eq!(marketing_name("claude-opus-4-6"), Some("Claude Opus 4.6")); - assert_eq!( - marketing_name("claude-sonnet-4-6"), - Some("Claude Sonnet 4.6") - ); - assert_eq!(marketing_name("claude-opus-4-5"), Some("Claude Opus 4.5")); - assert_eq!( - marketing_name("claude-sonnet-4-5"), - Some("Claude Sonnet 4.5") - ); - assert_eq!(marketing_name("claude-haiku-4-5"), Some("Claude Haiku 4.5")); - assert_eq!(marketing_name("claude-opus-4-1"), Some("Claude Opus 4.1")); - assert_eq!(marketing_name("claude-opus-4"), Some("Claude Opus 4")); - assert_eq!(marketing_name("claude-sonnet-4"), Some("Claude Sonnet 4")); - assert_eq!(marketing_name("claude-haiku-4"), Some("Claude Haiku 4")); + for (id, expected) in [ + ("claude-opus-4-7", "Claude Opus 4.7"), + ("claude-opus-4-6", "Claude Opus 4.6"), + ("claude-sonnet-4-6", "Claude Sonnet 4.6"), + ("claude-opus-4-5", "Claude Opus 4.5"), + ("claude-sonnet-4-5", "Claude Sonnet 4.5"), + ("claude-haiku-4-5", "Claude Haiku 4.5"), + ("claude-opus-4-1", "Claude Opus 4.1"), + ] { + assert_eq!(marketing_name(id), Some(expected), "{id}"); + } } #[test] - fn marketing_name_with_dated_suffix_falls_through_to_family_row() { + fn marketing_name_dated_suffix_falls_through_to_substring_row() { assert_eq!( marketing_name("claude-opus-4-6-20260401"), Some("Claude Opus 4.6") diff --git a/crates/oxide-code/src/prompt/environment.rs b/crates/oxide-code/src/prompt/environment.rs index 4bf21a6d..ae68224b 100644 --- a/crates/oxide-code/src/prompt/environment.rs +++ b/crates/oxide-code/src/prompt/environment.rs @@ -252,16 +252,17 @@ mod tests { #[test] fn knowledge_cutoff_known_models() { - assert_eq!(knowledge_cutoff("claude-opus-4-7"), Some("January 2026")); - assert_eq!(knowledge_cutoff("claude-sonnet-4-6"), Some("August 2025")); - assert_eq!(knowledge_cutoff("claude-opus-4-6"), Some("May 2025")); - assert_eq!(knowledge_cutoff("claude-opus-4-5"), Some("May 2025")); - assert_eq!(knowledge_cutoff("claude-haiku-4-5"), Some("February 2025")); - assert_eq!(knowledge_cutoff("claude-haiku-4"), Some("February 2025")); - assert_eq!(knowledge_cutoff("claude-opus-4-1"), Some("January 2025")); - assert_eq!(knowledge_cutoff("claude-opus-4"), Some("January 2025")); - assert_eq!(knowledge_cutoff("claude-sonnet-4-5"), Some("January 2025")); - assert_eq!(knowledge_cutoff("claude-sonnet-4"), Some("January 2025")); + for (id, expected) in [ + ("claude-opus-4-7", "January 2026"), + ("claude-sonnet-4-6", "August 2025"), + ("claude-opus-4-6", "May 2025"), + ("claude-opus-4-5", "May 2025"), + ("claude-haiku-4-5", "February 2025"), + ("claude-opus-4-1", "January 2025"), + ("claude-sonnet-4-5", "January 2025"), + ] { + assert_eq!(knowledge_cutoff(id), Some(expected), "{id}"); + } } #[test] diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 1b4c342c..dffe44e7 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -57,7 +57,7 @@ impl SlashCommand for EffortCmd { } let pick = parse_effort_arg(arg)?; let caps = capabilities_for(&ctx.info.config.model_id); - if !caps.effort { + if !caps.has_effort() { return Err(no_effort_tier_msg(&ctx.info.config.model_id)); } Ok(SlashOutcome::Forward(UserAction::SwapConfig { diff --git a/crates/oxide-code/src/slash/effort_slider.rs b/crates/oxide-code/src/slash/effort_slider.rs index 461c1f0a..2cb47ef5 100644 --- a/crates/oxide-code/src/slash/effort_slider.rs +++ b/crates/oxide-code/src/slash/effort_slider.rs @@ -59,7 +59,7 @@ impl EffortSlider { /// `None` when the active model has no effort tier; caller errors before opening. pub(super) fn new(info: &LiveSessionInfo) -> Option { let caps = capabilities_for(&info.config.model_id); - if !caps.effort { + if !caps.has_effort() { return None; } let supported: Vec = Effort::ALL @@ -69,7 +69,7 @@ impl EffortSlider { .collect(); let initial = caps .resolve_effort(info.config.effort) - .expect("caps.effort implies a resolvable tier"); + .expect("caps.has_effort() implies a resolvable tier"); let selected = supported .iter() .position(|e| *e == initial) diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index d7600cc8..71ed7a9f 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -8,7 +8,7 @@ use super::matcher::rank_by_prefix; use super::picker::LISTED_MODELS; use super::registry::{ArgCompletion, SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; -use crate::model::{MODELS, ResolvedModelId, display_name, is_family_base, lookup}; +use crate::model::{MODELS, ResolvedModelId, display_name, lookup}; // ── Constants ── @@ -50,8 +50,6 @@ impl SlashCommand for ModelCmd { .into_iter() .map(|id| ArgCompletion { value: Cow::Borrowed(*id), - // `display_name` is `Cow<'static, str>` for `&'static` ids — the four bare rows - // borrow the marketing literal; only `[1m]` rows allocate the suffixed string. description: display_name(id), }) .collect() @@ -92,16 +90,14 @@ fn resolve_model_arg(arg: &str) -> Result { if !info.capabilities.context_1m { return Err(format!( "{}: 1M context not supported. Drop the `{TAG_1M}` tag.", - info.marketing, + info.display_name, )); } Ok(ResolvedModelId::new(format!("{base_id}{TAG_1M}"))) } /// Resolution tiers (first hit wins): short alias → dated id (`-YYYYMMDD`) → known canonical -/// id → unique suffix → unique substring. Family-bases are filtered at every selectable tier so -/// users can't land on a deprecated row by typing its id directly — only dated ids of a -/// family-base remain reachable since the user explicitly opted into a specific snapshot. +/// id → unique suffix → unique substring. fn resolve_base(arg: &str) -> Result { if let Some(&(_, target)) = ALIASES.iter().find(|(name, _)| *name == arg) { return Ok(target.to_owned()); @@ -112,29 +108,39 @@ fn resolve_base(arg: &str) -> Result { if is_selectable_known_id(arg) { return Ok(arg.to_owned()); } - let suffix_hits = candidates(|id| id.ends_with(arg) && !is_family_base(id)); + let suffix_hits = candidates(|id| id.ends_with(arg)); if let [id] = suffix_hits.as_slice() { return Ok((*id).to_owned()); } - let visible = candidates(|id| id.contains(arg) && !is_family_base(id)); + let visible = candidates(|id| id.contains(arg)); match visible.as_slice() { [id] => Ok((*id).to_owned()), [_, ..] => Err(format!( - "`{arg}` matches {n} models: {list}. Type a more specific id or use a short alias (`opus`, `sonnet`, `haiku`).", + "`{arg}` matches {n} models. Pick one or use a short alias (`opus`, `sonnet`, `haiku`):\n\n{list}", n = visible.len(), - list = visible.join(", "), + list = format_supported_models(&visible), )), [] => Err(format!( - "Unknown model: `{arg}`. Run `/model` for selectable shortcuts; \ - any id from the model table works (e.g. `claude-opus-4-6`).", + "Unknown model: `{arg}`. Supported models:\n\n{list}", + list = format_supported_models(&candidates(|_| true)), )), } } fn is_selectable_known_id(arg: &str) -> bool { - MODELS - .iter() - .any(|m| m.id_substr == arg && !m.is_family_base) + MODELS.iter().any(|m| m.id_substr == arg) +} + +/// Markdown bullet list of `id — display name` rows. +fn format_supported_models(ids: &[&'static str]) -> String { + use std::fmt::Write as _; + let mut out = String::new(); + for id in ids { + let label = lookup(id).map_or(*id, |m| m.display_name); + _ = writeln!(out, "- `{id}` — {label}"); + } + out.truncate(out.trim_end_matches('\n').len()); + out } fn is_dated_model_id(arg: &str) -> bool { @@ -344,44 +350,35 @@ mod tests { } #[test] - fn execute_unknown_arg_errors_with_recovery_hint() { + fn execute_unknown_arg_errors_with_supported_model_listing() { let (chat, outcome) = run_execute("gpt-4"); let msg = outcome.expect_err("unknown arg must error"); + assert!(msg.starts_with("Unknown model: `gpt-4`."), "{msg}"); + assert!(msg.contains("Supported models:"), "{msg}"); + for id in ["claude-opus-4-7", "claude-opus-4-6", "claude-haiku-4-5"] { + assert!(msg.contains(id), "{id} should appear in listing: {msg}"); + } assert!( - msg.starts_with("Unknown model: `gpt-4`."), - "leading capital + backticked input: {msg}", - ); - assert!(msg.contains("Run `/model`"), "recovery hint: {msg}"); - assert!( - msg.contains("claude-opus-4-6"), - "manual-entry example surfaces: {msg}", + msg.contains("Claude Opus 4.7"), + "marketing names render: {msg}" ); - assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); + assert_eq!(chat.entry_count(), 0); } #[test] - fn execute_family_base_id_no_longer_silently_selects_deprecated() { - // `claude-opus-4` / `claude-sonnet-4` are kept in MODELS only so dated ids resolve - // their capabilities. Direct typing must NOT land on the deprecated base — opus-4 has - // four current descendants so it errors with ambiguity, sonnet-4 has two. + fn execute_retired_or_legacy_id_falls_through_to_ambiguity() { + // Retired Opus 4 / Sonnet 4 ids and the loose `opus-4` form substring-match multiple + // current descendants — must error with the candidate listing, not silently pick one. for arg in ["opus-4", "claude-opus-4", "claude-sonnet-4"] { let (_, outcome) = run_execute(arg); let msg = outcome.expect_err(&format!("`{arg}` must error")); - assert!( - msg.contains("matches"), - "ambiguity error for `{arg}`: {msg}" - ); - assert!( - !msg.contains("claude-opus-4,") && !msg.contains("claude-sonnet-4,"), - "deprecated family-base must not appear in candidate list: {msg}", - ); + assert!(msg.contains("matches"), "ambiguity for `{arg}`: {msg}"); } } #[test] - fn execute_family_base_with_unique_current_descendant_promotes_to_it() { - // `claude-haiku-4` substrings only `claude-haiku-4-5` (after family-base filter), so the - // substring tier resolves uniquely instead of erroring. + fn execute_haiku_4_resolves_to_only_extant_descendant() { + // `claude-haiku-4` substring-matches only `claude-haiku-4-5` (Haiku 4 was never released). let (_, outcome) = run_execute("claude-haiku-4"); assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); } @@ -390,39 +387,26 @@ mod tests { fn execute_ambiguous_substring_lists_count_and_each_candidate() { let (_, outcome) = run_execute("4-6"); let msg = outcome.expect_err("ambiguous arg must error"); - assert!( - msg.starts_with("`4-6` matches"), - "leading backtick + count substring: {msg}", - ); + assert!(msg.starts_with("`4-6` matches"), "{msg}"); for needle in ["claude-opus-4-6", "claude-sonnet-4-6"] { - assert!(msg.contains(needle), "candidate `{needle}` listed: {msg}"); + assert!(msg.contains(needle), "{needle} listed: {msg}"); } assert!(msg.contains("opus"), "alias hint surfaces: {msg}"); } #[test] - fn execute_ambiguous_listing_omits_family_base_rows() { - // `claude-opus` substring-matches every Opus row, including the deprecated - // `claude-opus-4` base. Listing the base would invite a user to type a superseded id. + fn execute_ambiguous_listing_covers_every_current_match() { + // `claude-opus` substring-matches every current Opus row. let (_, outcome) = run_execute("claude-opus"); let msg = outcome.expect_err("ambiguous arg must error"); - for current in ["claude-opus-4-7", "claude-opus-4-6", "claude-opus-4-1"] { - assert!( - msg.contains(current), - "current row `{current}` listed: {msg}" - ); + for current in [ + "claude-opus-4-7", + "claude-opus-4-6", + "claude-opus-4-5", + "claude-opus-4-1", + ] { + assert!(msg.contains(current), "{current} listed: {msg}"); } - // Bound the search to comma-delimited tokens so `claude-opus-4` doesn't false-match - // on the longer `claude-opus-4-7`. - let listed: Vec<&str> = msg - .split([':', ',']) - .map(str::trim) - .filter(|s| s.starts_with("claude-")) - .collect(); - assert!( - !listed.contains(&"claude-opus-4"), - "family base must not appear in listing: {listed:?}", - ); } #[test] @@ -450,13 +434,8 @@ mod tests { } #[test] - fn resolve_model_arg_round_trips_every_selectable_models_row() { - // Family-bases are deprecated for direct selection — covered by the - // `family_base_id_no_longer_silently_selects_deprecated` test instead. + fn resolve_model_arg_round_trips_every_models_row() { for info in MODELS { - if info.is_family_base { - continue; - } assert_eq!( resolve_model_arg(info.id_substr) .as_ref() diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index 6e5859e6..7a30056a 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -135,7 +135,7 @@ impl ModelEffortPicker { return; }; let caps = capabilities_for(row.id); - if !caps.effort { + if !caps.has_effort() { return; } let supported: Vec = Effort::ALL @@ -187,7 +187,7 @@ impl ModelEffortPicker { fn render_effort_row(&self, theme: &Theme) -> Option> { let row = self.list.selected()?; let caps = capabilities_for(row.id); - if !caps.effort { + if !caps.has_effort() { return None; } let level = self.effort_or_active()?; @@ -297,13 +297,13 @@ enum Direction { } fn has_effort_tier(row: &ModelRow) -> bool { - capabilities_for(row.id).effort + capabilities_for(row.id).has_effort() } fn effort_for_highlighted(list: &ListPicker, fallback: Option) -> Option { let row = list.selected()?; let caps = capabilities_for(row.id); - if !caps.effort { + if !caps.has_effort() { return None; } Some(caps.resolve_effort(fallback).unwrap_or(Effort::High)) From 12f2fe914fcfc59c1cf6fa88b20a23aa86e3dd8d Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:03:19 +0800 Subject: [PATCH 18/25] refactor(slash): popup is always a picker; rename PopupMode field Drop the picker-vs-suggester routing on Enter. While the popup is visible it commits the highlighted row regardless of mode; Esc dismisses the popup so the typed buffer can submit literally on the next Enter. The typed-id path users want for retired or custom model ids is now Esc + Enter, which mirrors how every other always-on-top picker behaves. Removes `popup_acts_as_picker` and its dead multi-line guard. Renames `PopupMode::Arg { cmd }` to `PopupMode::Arg { name }` for symmetry with `PopupState::Arg { name }` so the format strings in `popup_completion_text` and `popup_submit_selected` no longer alias the same concept twice. --- crates/oxide-code/src/tui/components/input.rs | 57 +++++++++---------- .../src/tui/components/input/popup.rs | 8 +-- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index b15ae756..c2839cae 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -299,14 +299,9 @@ impl InputArea { self.popup_complete_to_buffer(); PopupKey::Consumed } - KeyCode::Enter if modifiers.is_empty() => { - if self.popup_acts_as_picker() - && let Some(action) = self.popup_submit_selected() - { - return PopupKey::Action(action); - } - PopupKey::Pass - } + KeyCode::Enter if modifiers.is_empty() => self + .popup_submit_selected() + .map_or(PopupKey::Pass, PopupKey::Action), _ => PopupKey::Pass, } } @@ -319,33 +314,19 @@ impl InputArea { self.refresh_popup(); } - /// Tab-insertion text. Name mode → `/{name} `; arg mode → `/{cmd} {value} `. fn popup_completion_text(&self) -> Option { let row = self.popup.selected()?; Some(match self.popup.mode()? { PopupMode::Name => format!("/{} ", row.value), - PopupMode::Arg { cmd } => format!("/{cmd} {} ", row.value), + PopupMode::Arg { name } => format!("/{name} {} ", row.value), }) } - /// Picker mode: Enter commits the popup row. Suggester mode (arg with non-empty prefix): - /// Enter passes through to literal submit. - fn popup_acts_as_picker(&self) -> bool { - let [single] = self.textarea.lines() else { - return false; - }; - match popup_state(single) { - Some(PopupState::Name(_)) => true, - Some(PopupState::Arg { prefix, .. }) => prefix.is_empty(), - None => false, - } - } - fn popup_submit_selected(&mut self) -> Option { let row = self.popup.selected()?; let submission = match self.popup.mode()? { PopupMode::Name => format!("/{}", row.value), - PopupMode::Arg { cmd } => format!("/{cmd} {}", row.value), + PopupMode::Arg { name } => format!("/{name} {}", row.value), }; self.textarea.select_all(); self.textarea.cut(); @@ -1084,8 +1065,6 @@ mod tests { #[test] fn handle_event_popup_tab_in_arg_mode_inserts_cmd_value_and_space() { - // Tab keeps the `/effort` prefix and substitutes the picked value plus a trailing space - // — `/{cmd} {value} `, not `/{value} ` (which would drop cmd context). let mut input = test_input(); type_text(&mut input, "/effort "); input.refresh_popup(); @@ -1124,22 +1103,40 @@ mod tests { } #[test] - fn handle_event_popup_enter_in_arg_mode_with_typed_prefix_submits_buffer_literally() { - // Typed buffer wins over popup highlight; Tab is the explicit "promote popup pick" key. + fn handle_event_popup_enter_with_typed_arg_prefix_still_commits_picked_row() { let mut input = test_input(); type_text(&mut input, "/model claude-opus-4"); input.refresh_popup(); assert!(input.popup_visible()); - assert_ne!(selected_value(&input), "claude-opus-4"); + let picked = selected_value(&input); let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); assert_eq!( action, - Some(UserAction::SubmitPrompt("/model claude-opus-4".to_owned())), + Some(UserAction::SubmitPrompt(format!("/model {picked}"))), ); assert!(input.textarea.is_empty()); assert!(!input.popup_visible()); } + #[test] + fn handle_event_popup_esc_then_enter_submits_typed_buffer_literally() { + // Popup is always a picker; Esc is the seam for submitting custom text. Mirrors the + // direct-id paths users want for retired or custom model ids. + let mut input = test_input(); + type_text(&mut input, "/model claude-opus-4"); + input.refresh_popup(); + assert!(input.popup_visible()); + + input.handle_event(&key(KeyCode::Esc, KeyModifiers::NONE)); + assert!(!input.popup_visible()); + + let action = input.handle_event(&key(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!( + action, + Some(UserAction::SubmitPrompt("/model claude-opus-4".to_owned())), + ); + } + // ── ghost_text ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index cb64e1e7..562fcfde 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -30,9 +30,9 @@ const COLUMN_GAP: usize = 2; #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum PopupMode { Name, - /// Owned because the cmd name comes from the live input buffer. + /// Owned because the name comes from the live input buffer. Arg { - cmd: String, + name: String, }, } @@ -92,7 +92,7 @@ impl SlashPopup { } else { ( Some(PopupMode::Arg { - cmd: (*name).to_owned(), + name: (*name).to_owned(), }), completions .into_iter() @@ -327,7 +327,7 @@ mod tests { fn set_state_arg_with_curated_roster_populates_arg_mode() { let popup = arg_popup("model", ""); assert!(popup.is_visible()); - assert!(matches!(popup.mode(), Some(PopupMode::Arg { cmd }) if cmd == "model")); + assert!(matches!(popup.mode(), Some(PopupMode::Arg { name }) if name == "model")); // Roster is non-empty for /model. assert!(!popup.rows.is_empty()); } From 7d9b964b0b8cee1abd48a6d860297c562202653e Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:09:39 +0800 Subject: [PATCH 19/25] refactor(model): unify name accessors on display_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop `marketing_name(&str) -> Option<&str>` and `marketing_or_id(&str) -> Cow`. The single `display_name(&str) -> Cow` now handles every human-facing label: marketing name when known, raw id when unknown, ` (1M context)` suffix on `[1m]` ids in both cases. The misleading `LiveSessionInfo::marketing_name()` (which actually wrapped `marketing_or_id` and dropped the 1M suffix) is gone; only `display_name()` remains. Site-by-site impact: - `prompt/environment.rs` branches on `lookup` to keep the "named X. The exact ID is Y" form distinct from the unknown-id "the model X" form. - `format_config_change` drops its `marketing` parameter and computes the label from `model_id` internally — one source of truth per call. - `slash/effort.rs` no-effort error, `/status` modal, and `/config` row all read from `display_name`. The `(1M context)` suffix surfaces alongside the `[1m]` id, which is informative rather than redundant. --- crates/oxide-code/src/model.rs | 58 +++++---------------- crates/oxide-code/src/prompt/environment.rs | 12 ++--- crates/oxide-code/src/slash.rs | 2 +- crates/oxide-code/src/slash/config.rs | 4 +- crates/oxide-code/src/slash/context.rs | 7 +-- crates/oxide-code/src/slash/effort.rs | 4 +- crates/oxide-code/src/slash/status_modal.rs | 2 +- crates/oxide-code/src/tui/app.rs | 25 ++------- 8 files changed, 30 insertions(+), 84 deletions(-) diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index abc86176..cd894eeb 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -208,18 +208,12 @@ pub(crate) fn capabilities_for(model: &str) -> Capabilities { .unwrap_or_default() } -pub(crate) fn marketing_name(model: &str) -> Option<&'static str> { - lookup(model).map(|info| info.display_name) -} - -/// Marketing name when known, raw id otherwise. -pub(crate) fn marketing_or_id(model: &str) -> Cow<'_, str> { - marketing_name(model).map_or_else(|| Cow::Borrowed(model), Cow::Borrowed) -} - -/// Human-facing label: marketing name + ` (1M context)` suffix on `[1m]` ids. +/// Human-facing label: marketing name + ` (1M context)` suffix on `[1m]` ids; raw id when the +/// model is unknown. pub(crate) fn display_name(model: &str) -> Cow<'_, str> { - let base = marketing_or_id(model); + let base = lookup(model).map_or(Cow::Borrowed(model), |info| { + Cow::Borrowed(info.display_name) + }); if model.ends_with("[1m]") { Cow::Owned(format!("{base} (1M context)")) } else { @@ -490,10 +484,10 @@ mod tests { } } - // ── marketing_name ── + // ── display_name ── #[test] - fn marketing_name_known_models() { + fn display_name_known_plain_id_renders_marketing_name() { for (id, expected) in [ ("claude-opus-4-7", "Claude Opus 4.7"), ("claude-opus-4-6", "Claude Opus 4.6"), @@ -503,39 +497,15 @@ mod tests { ("claude-haiku-4-5", "Claude Haiku 4.5"), ("claude-opus-4-1", "Claude Opus 4.1"), ] { - assert_eq!(marketing_name(id), Some(expected), "{id}"); + assert_eq!(display_name(id), expected, "{id}"); } } #[test] - fn marketing_name_dated_suffix_falls_through_to_substring_row() { - assert_eq!( - marketing_name("claude-opus-4-6-20260401"), - Some("Claude Opus 4.6") - ); - } - - #[test] - fn marketing_name_unknown_model_is_absent() { - assert_eq!(marketing_name("gpt-4o"), None); - assert_eq!(marketing_name("custom-model"), None); - } - - // ── marketing_or_id ── - - #[test] - fn marketing_or_id_produces_marketing_for_known_id() { - assert_eq!(marketing_or_id("claude-opus-4-7"), "Claude Opus 4.7"); + fn display_name_dated_suffix_falls_through_to_substring_row() { + assert_eq!(display_name("claude-opus-4-6-20260401"), "Claude Opus 4.6"); } - #[test] - fn marketing_or_id_falls_back_to_raw_id_for_unknown() { - // Single seam for unknown-id fallback — every UI surface goes through this. - assert_eq!(marketing_or_id("gpt-4"), "gpt-4"); - } - - // ── display_name ── - #[test] fn display_name_appends_1m_context_suffix_on_1m_id() { assert_eq!( @@ -545,12 +515,8 @@ mod tests { } #[test] - fn display_name_omits_suffix_on_plain_id() { - assert_eq!(display_name("claude-opus-4-7"), "Claude Opus 4.7"); - } - - #[test] - fn display_name_unknown_plain_id_falls_through_to_raw() { + fn display_name_unknown_id_falls_through_to_raw() { assert_eq!(display_name("gpt-4"), "gpt-4"); + assert_eq!(display_name("custom-model"), "custom-model"); } } diff --git a/crates/oxide-code/src/prompt/environment.rs b/crates/oxide-code/src/prompt/environment.rs index ae68224b..ac559220 100644 --- a/crates/oxide-code/src/prompt/environment.rs +++ b/crates/oxide-code/src/prompt/environment.rs @@ -5,7 +5,7 @@ use std::path::Path; use indoc::formatdoc; use platform_info::{PlatformInfo, PlatformInfoAPI, UNameAPI}; -use crate::model::marketing_name; +use crate::model::lookup; use crate::util::env; const UNKNOWN_MARKER: &str = "(unknown)"; @@ -49,11 +49,11 @@ impl Environment { } pub(super) fn render(&self) -> String { - let model_bullet = match marketing_name(&self.model) { - Some(name) => format!( - "- You are powered by the model named {name}. \ + let model_bullet = match lookup(&self.model) { + Some(info) => format!( + "- You are powered by the model named {}. \ The exact model ID is {}.", - self.model + info.display_name, self.model, ), None => format!("- You are powered by the model {}.", self.model), }; @@ -85,7 +85,7 @@ impl Environment { // ── Model Metadata ── fn knowledge_cutoff(model: &str) -> Option<&'static str> { - crate::model::lookup(model).and_then(|info| info.cutoff) + lookup(model).and_then(|info| info.cutoff) } // ── Platform Detection ── diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 9848b120..06792bee 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -112,7 +112,7 @@ fn classify_in(commands: &[&dyn registry::SlashCommand], parsed: &Parsed) -> Sla pub(crate) fn test_session_info() -> LiveSessionInfo { use crate::config::{ConfigSnapshot, Effort, PromptCacheTtl}; - // Real MODELS row so `marketing_name()` produces a known name. + // Real MODELS row so `display_name()` resolves to a known marketing name. LiveSessionInfo { cwd: "~/test".to_owned(), version: "0.0.0-test", diff --git a/crates/oxide-code/src/slash/config.rs b/crates/oxide-code/src/slash/config.rs index 90ba6e51..248b714d 100644 --- a/crates/oxide-code/src/slash/config.rs +++ b/crates/oxide-code/src/slash/config.rs @@ -39,7 +39,7 @@ fn render_config( let max_tokens = cfg.max_tokens.to_string(); let cache_ttl = cfg.prompt_cache_ttl.to_string(); let thinking = if cfg.show_thinking { "yes" } else { "no" }; - let model = info.marketing_name(); + let model = info.display_name(); let resolved: [(&str, &str); 8] = [ ("Model", &model), ("Model ID", &cfg.model_id), @@ -118,7 +118,7 @@ mod tests { fn render_config_includes_every_resolved_field_value() { let info = test_session_info(); let cfg = &info.config; - let model = info.marketing_name(); + let model = info.display_name(); let body = render_config(&info, None, None); let effort = cfg .effort diff --git a/crates/oxide-code/src/slash/context.rs b/crates/oxide-code/src/slash/context.rs index ea75ab4c..f0b7b74c 100644 --- a/crates/oxide-code/src/slash/context.rs +++ b/crates/oxide-code/src/slash/context.rs @@ -4,7 +4,7 @@ use std::borrow::Cow; use crate::config::ConfigSnapshot; -use crate::model::{display_name, marketing_or_id}; +use crate::model::display_name; use crate::tui::components::chat::ChatView; use crate::tui::modal::Modal; @@ -20,11 +20,6 @@ pub(crate) struct LiveSessionInfo { } impl LiveSessionInfo { - pub(crate) fn marketing_name(&self) -> Cow<'_, str> { - marketing_or_id(&self.config.model_id) - } - - /// Human-facing label including `(1M context)` suffix for `[1m]` models. pub(crate) fn display_name(&self) -> Cow<'_, str> { display_name(&self.config.model_id) } diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index dffe44e7..81b091f8 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -9,7 +9,7 @@ use super::matcher::rank_by_prefix; use super::registry::{ArgCompletion, SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; use crate::config::Effort; -use crate::model::{capabilities_for, marketing_or_id}; +use crate::model::{capabilities_for, display_name}; pub(super) struct EffortCmd; @@ -70,7 +70,7 @@ impl SlashCommand for EffortCmd { fn no_effort_tier_msg(model_id: &str) -> String { format!( "{} has no effort tier. Pick an effort-capable model first with /model (e.g. /model opus, /model sonnet).", - marketing_or_id(model_id), + display_name(model_id), ) } diff --git a/crates/oxide-code/src/slash/status_modal.rs b/crates/oxide-code/src/slash/status_modal.rs index 94ac2637..799434e4 100644 --- a/crates/oxide-code/src/slash/status_modal.rs +++ b/crates/oxide-code/src/slash/status_modal.rs @@ -27,7 +27,7 @@ pub(super) struct StatusModal { impl StatusModal { pub(super) fn new(info: &LiveSessionInfo) -> Self { - let model = format!("{} ({})", info.marketing_name(), info.config.model_id); + let model = format!("{} ({})", info.display_name(), info.config.model_id); let show_thinking = if info.config.show_thinking { "on".to_owned() } else { diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 7bc99fb4..b8c8fd63 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -453,9 +453,7 @@ impl App { } => { let model_changed = model_id != self.session_info.config.model_id; let prev_effort = self.session_info.config.effort; - let marketing = crate::model::marketing_or_id(&model_id); let confirmation = format_config_change( - &marketing, &model_id, model_changed, prev_effort, @@ -632,7 +630,6 @@ fn preview_line(prompt: &str, theme: &Theme, body_width: usize) -> Line<'static> /// `AgentEvent::ConfigChanged` confirmation, surfacing silent effort shifts. fn format_config_change( - marketing: &str, model_id: &str, model_changed: bool, prev_effort: Option, @@ -650,7 +647,10 @@ fn format_config_change( (None, _) => "Config unchanged.".to_owned(), }; } - let head = format!("Switched to {marketing} ({model_id})"); + let head = format!( + "Switched to {} ({model_id})", + crate::model::display_name(model_id) + ); match (requested_effort, prev_effort, new_effort) { (Some(req), _, Some(eff)) if req == eff => format!("{head} · effort {eff}."), (Some(req), _, Some(eff)) => format!("{head} · effort {eff} (clamped from {req})."), @@ -1799,14 +1799,7 @@ mod tests { fn format_config_change_swap_both_none_omits_effort_clause() { // Pin: no `effort` substring at all, never a stray "none" // word. Mutation that prints `effort none.` would surface here. - let s = format_config_change( - "Claude Haiku 4.5", - "claude-haiku-4-5", - true, - None, - None, - None, - ); + let s = format_config_change("claude-haiku-4-5", true, None, None, None); assert_eq!(s, "Switched to Claude Haiku 4.5 (claude-haiku-4-5)."); } @@ -1815,7 +1808,6 @@ mod tests { // User had a tier; new model has none. Surface the change so // the user knows their effort just disappeared. let s = format_config_change( - "Claude Haiku 4.5", "claude-haiku-4-5", true, Some(crate::config::Effort::Xhigh), @@ -1834,7 +1826,6 @@ mod tests { // distinguishing this from "user's pick survived" prevents // the user from thinking they chose this tier. let s = format_config_change( - "Claude Opus 4.7", "claude-opus-4-7", true, None, @@ -1851,7 +1842,6 @@ mod tests { fn format_config_change_swap_marks_clamp_when_new_effort_below_previous() { // The effective tier changed; surface the temporary clamp. let s = format_config_change( - "Claude Sonnet 4.6", "claude-sonnet-4-6", true, Some(crate::config::Effort::Xhigh), @@ -1869,7 +1859,6 @@ mod tests { // Same tier survives — no clamp / default annotation. Pin // exact format so a stray suffix (`(unchanged)`) would fail. let s = format_config_change( - "Claude Opus 4.7", "claude-opus-4-7", true, Some(crate::config::Effort::High), @@ -1887,7 +1876,6 @@ mod tests { // Combined picker case: user asks for xhigh on Sonnet (caps at // high). Surface that the *requested* tier was clamped — not the previous-effort delta. let s = format_config_change( - "Claude Sonnet 4.6", "claude-sonnet-4-6", true, Some(crate::config::Effort::Medium), @@ -1903,7 +1891,6 @@ mod tests { #[test] fn format_config_change_effort_explicit_pick_matches_resolution() { let s = format_config_change( - "Claude Opus 4.7", "claude-opus-4-7", false, Some(crate::config::Effort::High), @@ -1916,7 +1903,6 @@ mod tests { #[test] fn format_config_change_effort_clamp_surfaces_what_user_asked_for() { let s = format_config_change( - "Claude Sonnet 4.6", "claude-sonnet-4-6", false, Some(crate::config::Effort::Medium), @@ -1931,7 +1917,6 @@ mod tests { // The slash command preflight stops this through /effort, but // client-driven flows could still emit it. let s = format_config_change( - "Claude Haiku 4.5", "claude-haiku-4-5", false, None, From 9bd3f1d9d491fbe3b4a41e3e42c6eb14699dd9b6 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:14:40 +0800 Subject: [PATCH 20/25] chore(slash): trim comment noise; align expect reason with current shape - Tighten over-explained doc comments on `Capabilities` `#[expect]`, `MODELS`, `ArgCompletion`, `complete_arg`, `rank_by_prefix`, `Effort::description`, popup `mode` field, `label`, `ghost_text`, `ghost_text_from_state` to one line each. - Drop test-narration comments in `slash/parser.rs` and `tui/components/input.rs` that restated assertions or named the mutation they pinned. - Drop the `// haiku-4 is not in modelSupportsISP` reference now that the row is gone, and the "X, not Y" antithesis form in the retired-id resolver test (project style bans it). --- crates/oxide-code/src/config.rs | 3 +-- crates/oxide-code/src/model.rs | 4 ++-- crates/oxide-code/src/slash/matcher.rs | 4 ++-- crates/oxide-code/src/slash/model.rs | 7 ++----- crates/oxide-code/src/slash/parser.rs | 7 ------- crates/oxide-code/src/slash/registry.rs | 6 ++---- crates/oxide-code/src/tui/components/input.rs | 10 ++-------- crates/oxide-code/src/tui/components/input/popup.rs | 5 ++--- 8 files changed, 13 insertions(+), 33 deletions(-) diff --git a/crates/oxide-code/src/config.rs b/crates/oxide-code/src/config.rs index 9c74bef8..084aa901 100644 --- a/crates/oxide-code/src/config.rs +++ b/crates/oxide-code/src/config.rs @@ -102,8 +102,7 @@ impl Effort { } } - /// One-line UX hint for the typed-arg popup. Behavior-focused so it stays consistent across - /// the ladder; per-tier model-availability checks live in the executor. + /// One-line UX hint for the typed-arg popup. pub(crate) const fn description(self) -> &'static str { match self { Self::Low => "Fastest, shallow reasoning", diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index cd894eeb..1475fdc1 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -23,7 +23,7 @@ pub(crate) struct ModelInfo { /// effort picker (renders only the supported ladder). #[expect( clippy::struct_excessive_bools, - reason = "each flag maps 1:1 to a separate upstream `modelSupports*` predicate or per-version allowlist; bitflags add indirection without expressiveness" + reason = "each flag is an independent capability gate" )] #[derive(Clone, Copy, Default)] pub(crate) struct Capabilities { @@ -39,7 +39,7 @@ pub(crate) struct Capabilities { // ── MODELS ── -/// Most-specific substring first. No inheritance between rows. +/// Most-specific substring first. pub(crate) const MODELS: &[ModelInfo] = &[ ModelInfo { id_substr: "claude-opus-4-7", diff --git a/crates/oxide-code/src/slash/matcher.rs b/crates/oxide-code/src/slash/matcher.rs index 570e8201..8ebbb550 100644 --- a/crates/oxide-code/src/slash/matcher.rs +++ b/crates/oxide-code/src/slash/matcher.rs @@ -70,8 +70,8 @@ fn matching_alias(cmd: &dyn SlashCommand, pred: impl Fn(&str) -> bool) -> Option .find(|alias| pred(&alias.to_ascii_lowercase())) } -/// Two-tier prefix-then-substring rank over a curated roster, preserving declared order within -/// each tier. Empty `query` returns every item in declared order. Lower-cased internally. +/// Two-tier prefix-then-substring rank over a curated roster, case-insensitive, preserving +/// declared order within each tier. Empty `query` returns every item in declared order. pub(super) fn rank_by_prefix<'a, T>( items: &'a [T], query: &str, diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 71ed7a9f..1f4da02f 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -229,9 +229,7 @@ mod tests { #[test] fn complete_arg_appends_1m_context_suffix_only_for_1m_variants() { - // The `[1m]` rows must surface the `(1M context)` marker so users can tell variants - // apart; the bare row must NOT carry the suffix (mutation that always-appends would - // otherwise survive). + // `[1m]` variants get the marker; bare row must not. let rows = arg_rows("claude-opus-4-7"); let one_m = rows .iter() @@ -367,8 +365,7 @@ mod tests { #[test] fn execute_retired_or_legacy_id_falls_through_to_ambiguity() { - // Retired Opus 4 / Sonnet 4 ids and the loose `opus-4` form substring-match multiple - // current descendants — must error with the candidate listing, not silently pick one. + // Retired ids substring-match multiple current rows; ambiguity must surface. for arg in ["opus-4", "claude-opus-4", "claude-sonnet-4"] { let (_, outcome) = run_execute(arg); let msg = outcome.expect_err(&format!("`{arg}` must error")); diff --git a/crates/oxide-code/src/slash/parser.rs b/crates/oxide-code/src/slash/parser.rs index 796f2ade..6465f243 100644 --- a/crates/oxide-code/src/slash/parser.rs +++ b/crates/oxide-code/src/slash/parser.rs @@ -158,8 +158,6 @@ mod tests { #[test] fn popup_state_trailing_space_after_name_switches_to_empty_arg() { - // The empty-prefix case is the trigger for the placeholder ghost-text — we have to be - // able to distinguish it from `Name`, otherwise the popup never opens for arg mode. assert_eq!(popup_state("/clear "), Some(arg("clear", ""))); } @@ -171,8 +169,6 @@ mod tests { #[test] fn popup_state_inner_whitespace_in_args_is_kept_for_free_form_commands() { - // /init takes a free-form sentence — the popup is hidden via empty `complete_arg`, but - // the parser still classifies it as Arg with the full remainder as prefix. assert_eq!( popup_state("/init please write CLAUDE.md"), Some(arg("init", "please write CLAUDE.md")), @@ -196,14 +192,11 @@ mod tests { fn popup_state_invalid_name_chars_reject_the_buffer() { assert!(popup_state("/foo🦀").is_none()); assert!(popup_state("/foo!").is_none()); - // Disallowed-char names also fail in arg form so the popup doesn't show stale completions. assert!(popup_state("/foo! arg").is_none()); } #[test] fn popup_state_empty_name_rejects_buffer() { - // `/` followed by whitespace has no name to dispatch — must not parse as `Arg { name: "" }`, - // which would route empty-string lookups through `complete_arg_for` / `arg_placeholder_for`. assert!(popup_state("/ ").is_none()); assert!(popup_state("/ ").is_none()); assert!(popup_state("/ arg").is_none()); diff --git a/crates/oxide-code/src/slash/registry.rs b/crates/oxide-code/src/slash/registry.rs index 18a05af7..f1837ff5 100644 --- a/crates/oxide-code/src/slash/registry.rs +++ b/crates/oxide-code/src/slash/registry.rs @@ -35,8 +35,7 @@ pub(crate) enum SlashKind { Unknown, } -/// One row in the typed-arg autocomplete popup. `description` may be empty. `Cow` so static -/// rosters borrow and dynamic ones (e.g., `[1m]` suffixed model names) own. +/// One row in the typed-arg autocomplete popup. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ArgCompletion { pub(crate) value: Cow<'static, str>, @@ -62,8 +61,7 @@ pub(crate) trait SlashCommand: Sync { None } - /// Curated arg suggestions, prefix-filtered and ranked. Empty vec → no autocomplete; the - /// popup falls back to the [`Self::usage`] placeholder ghost-text. + /// Curated arg suggestions, prefix-filtered and ranked. Empty → no autocomplete. fn complete_arg(&self, _prefix: &str) -> Vec { Vec::new() } diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index c2839cae..44d1902d 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -344,9 +344,7 @@ impl InputArea { self.popup.set_state(state.as_ref()); } - /// Dim placeholder token (`[id]`, `[level]`, `[name]`) painted at the cursor in arg mode with - /// an empty prefix. Coexists with the popup — the popup lists curated picks, the hint - /// confirms the slot's shape. + /// Dim placeholder painted at the cursor in arg-mode with an empty prefix. fn ghost_text(&self) -> Option { let [single] = self.textarea.lines() else { return None; @@ -422,8 +420,7 @@ impl InputArea { // ── Free Functions ── -/// Fires only in `Arg` mode with an empty prefix and a `usage()` to surface; coexists with the -/// popup (no visibility gate). +/// Fires only in `Arg` mode with an empty prefix and a `usage()` to surface. fn ghost_text_from_state(state: &PopupState<'_>, usage: Option<&str>) -> Option { let PopupState::Arg { prefix, .. } = state else { return None; @@ -765,8 +762,6 @@ mod tests { #[test] fn render_paints_ghost_text_at_cursor_in_arg_mode_with_empty_prefix() { - // `/model ` puts the input in arg mode with an empty prefix; the dim `[id]` placeholder - // must paint into the rendered buffer (covers the per-render Paragraph paint). let mut input = test_input(); type_text(&mut input, "/model "); input.refresh_popup(); @@ -780,7 +775,6 @@ mod tests { #[test] fn render_omits_ghost_text_once_user_types_arg_prefix() { - // Pins the suppression branch: any non-empty arg prefix hides the placeholder. let mut input = test_input(); type_text(&mut input, "/model claude-"); input.refresh_popup(); diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index 562fcfde..bb44cef3 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -49,7 +49,7 @@ pub(super) struct PopupRow { /// Slash-command autocomplete overlay. Empty `rows` means hidden. pub(crate) struct SlashPopup { theme: Theme, - /// `Some` when visible. Mode discriminates which Tab-insertion shape applies. + /// `Some` when visible. mode: Option, rows: Vec, selected: usize, @@ -188,8 +188,7 @@ impl SlashPopup { self.selected.saturating_sub(pad).min(max_offset) } - /// Mode-aware left-column label. `None` arm is a defensive fallback — `render` gates on - /// non-empty `rows`, which keeps `mode` `Some`. + /// Mode-aware left-column label; `None` arm unreachable in practice. fn label(&self, row: &PopupRow) -> String { match &self.mode { Some(PopupMode::Name) => match row.matched_alias { From 886a452ed59503ee630e828b4ad0501c933f0278 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:17:47 +0800 Subject: [PATCH 21/25] test(slash): pin format_supported_models, supported_efforts ordering, intra-mode selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Direct test for `format_supported_models` pinning the markdown bullet shape (`- \`id\` — display name`) and the empty-slice path. Closes the mutation gap where `writeln!` → `write!` and em-dash → hyphen swaps survived the prior transitive coverage. - Smoke test that every `MODELS` row's `supported_efforts` slice is authored in ascending order, so the `clamp_effort` / `default_effort` reverse-walk stays sound when new rows land. - Twin to the mode-transition reset: a same-mode `set_state` clamps rather than resets, pinning the branch that wasn't otherwise covered. - Drop the function-level ghost-text tests subsumed by the render-level ones; the render variants assert the painted buffer, which is a strict superset of the function output. --- crates/oxide-code/src/model.rs | 14 ++++++++++++++ crates/oxide-code/src/slash/model.rs | 16 ++++++++++++++++ crates/oxide-code/src/tui/components/input.rs | 18 ------------------ .../src/tui/components/input/popup.rs | 13 +++++++++++++ 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index 1475fdc1..04a0f48b 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -312,6 +312,20 @@ mod tests { } } + #[test] + fn supported_efforts_is_ascending_for_every_models_row() { + // `clamp_effort` and `default_effort` walk the slice in reverse to find the highest + // tier ≤ pick. They depend on the slice being authored in ascending order. + for info in MODELS { + let efforts = info.capabilities.supported_efforts; + assert!( + efforts.windows(2).all(|w| w[0] < w[1]), + "{}: supported_efforts must be ascending: {efforts:?}", + info.id_substr, + ); + } + } + #[test] fn structured_outputs_flag_tracks_upstream_allowlist() { for supported in [ diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 1f4da02f..379f41c7 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -505,4 +505,20 @@ mod tests { assert!(msg.contains("tag, not a model"), "{msg}"); assert!(!msg.contains("matches"), "must not list candidates: {msg}"); } + + // ── format_supported_models ── + + #[test] + fn format_supported_models_renders_one_markdown_bullet_per_id() { + let out = format_supported_models(&["claude-opus-4-7", "gpt-4"]); + assert_eq!( + out, + "- `claude-opus-4-7` — Claude Opus 4.7\n- `gpt-4` — gpt-4", + ); + } + + #[test] + fn format_supported_models_empty_input_renders_empty_string() { + assert_eq!(format_supported_models(&[]), ""); + } } diff --git a/crates/oxide-code/src/tui/components/input.rs b/crates/oxide-code/src/tui/components/input.rs index 44d1902d..5bdf257b 100644 --- a/crates/oxide-code/src/tui/components/input.rs +++ b/crates/oxide-code/src/tui/components/input.rs @@ -1133,16 +1133,6 @@ mod tests { // ── ghost_text ── - #[test] - fn ghost_text_shows_alongside_popup_when_arg_prefix_empty() { - // Popup and ghost coexist: popup lists curated picks; ghost confirms the slot's shape. - let mut input = test_input(); - type_text(&mut input, "/model "); - input.refresh_popup(); - assert!(input.popup_visible()); - assert_eq!(input.ghost_text(), Some("[id]".to_owned())); - } - #[test] fn ghost_text_suppresses_in_name_mode() { let mut input = test_input(); @@ -1151,14 +1141,6 @@ mod tests { assert_eq!(input.ghost_text(), None); } - #[test] - fn ghost_text_suppresses_once_user_types_arg_prefix() { - let mut input = test_input(); - type_text(&mut input, "/model claude-"); - input.refresh_popup(); - assert_eq!(input.ghost_text(), None); - } - // ── ghost_text_from_state ── #[test] diff --git a/crates/oxide-code/src/tui/components/input/popup.rs b/crates/oxide-code/src/tui/components/input/popup.rs index bb44cef3..5d88952c 100644 --- a/crates/oxide-code/src/tui/components/input/popup.rs +++ b/crates/oxide-code/src/tui/components/input/popup.rs @@ -322,6 +322,19 @@ mod tests { assert_eq!(popup.selected, 0, "mode transition resets selection"); } + #[test] + fn set_state_intra_mode_query_change_preserves_selection_via_clamp() { + // Twin to the mode-transition test: within the same mode, selection sticks (clamped + // to the new row count) so a refining keystroke doesn't yank the cursor back to row 0. + let mut popup = name_popup(""); + popup.select_next(); + let parked = popup.selected; + assert!(parked > 0, "selection past row 0"); + + popup.set_state(Some(&PopupState::Name(""))); + assert_eq!(popup.selected, parked, "same mode preserves selection"); + } + #[test] fn set_state_arg_with_curated_roster_populates_arg_mode() { let popup = arg_popup("model", ""); From bd82fade5dc497b313c84add1f0842f15475d09e Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:29:02 +0800 Subject: [PATCH 22/25] fix(tui): render multi-line error messages one row per logical line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ErrorBlock::render` was calling `push_icon_wrapped` once on the whole message, so embedded `\n`s collapsed into a single wrapped line — the markdown bullet list in `/model gpt-4` errors rendered as a single ribbon of `Supported models:- \`claude-opus-4-7\` ... -\` claude-...`. Fix mirrors `SystemMessageBlock`: iterate `self.message.lines()`, emit the `✗ ` icon on the first row and a width-aligned indent on the rest, wrapping each independently. New tests pin the per-line layout, the continuation indent, and the slash-error markdown-bullet case. --- .../src/tui/components/chat/blocks/error.rs | 101 +++++++++++++++--- 1 file changed, 89 insertions(+), 12 deletions(-) diff --git a/crates/oxide-code/src/tui/components/chat/blocks/error.rs b/crates/oxide-code/src/tui/components/chat/blocks/error.rs index 99ede2ea..cbe50e72 100644 --- a/crates/oxide-code/src/tui/components/chat/blocks/error.rs +++ b/crates/oxide-code/src/tui/components/chat/blocks/error.rs @@ -1,11 +1,14 @@ -//! Error block — flat `✗ message` in the error color, no left bar. +//! Error block — `✗ message` in the error color, no left bar. -use ratatui::text::Line; +use ratatui::text::{Line, Span}; +use unicode_width::UnicodeWidthStr; -use super::{ChatBlock, RenderCtx, push_icon_wrapped}; +use super::{ChatBlock, RenderCtx}; use crate::tui::glyphs::ERROR_PREFIX; +use crate::tui::wrap::wrap_line; -/// A fatal agent or API error, rendered as a single red line. +/// A fatal agent or API error. Multi-line messages render one logical line per row, the icon +/// on the first and a width-aligned indent on the rest. pub(crate) struct ErrorBlock { message: String, } @@ -21,15 +24,22 @@ impl ErrorBlock { impl ChatBlock for ErrorBlock { fn render(&self, ctx: &RenderCtx<'_>) -> Vec> { let style = ctx.theme.error(); + let width = usize::from(ctx.width); + let indent = ERROR_PREFIX.width(); + let cont_prefix = vec![Span::raw(" ".repeat(indent))]; let mut out = Vec::new(); - push_icon_wrapped( - &mut out, - ERROR_PREFIX, - style, - &self.message, - style, - usize::from(ctx.width), - ); + for (i, body_line) in self.message.lines().enumerate() { + let prefix = if i == 0 { + ERROR_PREFIX.to_owned() + } else { + " ".repeat(indent) + }; + let line = Line::from(vec![ + Span::styled(prefix, style), + Span::styled(body_line.to_owned(), style), + ]); + out.extend(wrap_line(line, width, indent, Some(&cont_prefix))); + } out } @@ -42,3 +52,70 @@ impl ChatBlock for ErrorBlock { Some(&self.message) } } + +#[cfg(test)] +mod tests { + use indoc::indoc; + + use super::*; + use crate::tui::theme::Theme; + + fn ctx_at(width: u16, theme: &Theme) -> RenderCtx<'_> { + RenderCtx { + width, + theme, + show_thinking: false, + } + } + + // ── render ── + + #[test] + fn render_single_line_message_emits_icon_then_body() { + let theme = Theme::default(); + let block = ErrorBlock::new("boom"); + let lines = block.render(&ctx_at(60, &theme)); + assert_eq!(lines.len(), 1); + assert_eq!(lines[0].spans[0].content, ERROR_PREFIX); + assert_eq!(lines[0].spans[1].content, "boom"); + } + + #[test] + fn render_multi_line_message_keeps_each_line_on_its_own_row() { + // Slash-command errors emit markdown bullets (`/model gpt-4` → "Unknown model: ..."). + // Joining them at render time would collapse the bullets into one wrapped line. + let theme = Theme::default(); + let block = ErrorBlock::new(indoc! {" + Unknown model: `ab`. Supported models: + + - `claude-opus-4-7` — Claude Opus 4.7 + - `claude-haiku-4-5` — Claude Haiku 4.5" + }); + let lines = block.render(&ctx_at(80, &theme)); + let bodies: Vec<&str> = lines + .iter() + .map(|l| l.spans.last().expect("non-empty").content.as_ref()) + .collect(); + assert_eq!( + bodies, + vec![ + "Unknown model: `ab`. Supported models:", + "", + "- `claude-opus-4-7` — Claude Opus 4.7", + "- `claude-haiku-4-5` — Claude Haiku 4.5", + ], + ); + } + + #[test] + fn render_continuation_rows_align_under_first_line_body() { + // First-row prefix is `✗ ` (icon + space); follow-up logical rows must indent to the + // same column so the message reads as one block. + let theme = Theme::default(); + let block = ErrorBlock::new("first\nsecond"); + let lines = block.render(&ctx_at(60, &theme)); + let indent = ERROR_PREFIX.width(); + assert_eq!(lines[0].spans[0].content, ERROR_PREFIX); + assert_eq!(lines[1].spans[0].content, " ".repeat(indent)); + } +} From 1a2c2ef4f79bffb39fa4cd35264cbf4021fa3e3d Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:39:29 +0800 Subject: [PATCH 23/25] test(tui): use gpt-4 in ErrorBlock multi-line fixture Matches the canonical wrong-id example used in format_supported_models and the slash-model unknown-arg tests. --- crates/oxide-code/src/tui/components/chat/blocks/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxide-code/src/tui/components/chat/blocks/error.rs b/crates/oxide-code/src/tui/components/chat/blocks/error.rs index cbe50e72..7dd0c4a2 100644 --- a/crates/oxide-code/src/tui/components/chat/blocks/error.rs +++ b/crates/oxide-code/src/tui/components/chat/blocks/error.rs @@ -86,7 +86,7 @@ mod tests { // Joining them at render time would collapse the bullets into one wrapped line. let theme = Theme::default(); let block = ErrorBlock::new(indoc! {" - Unknown model: `ab`. Supported models: + Unknown model: `gpt-4`. Supported models: - `claude-opus-4-7` — Claude Opus 4.7 - `claude-haiku-4-5` — Claude Haiku 4.5" @@ -99,7 +99,7 @@ mod tests { assert_eq!( bodies, vec![ - "Unknown model: `ab`. Supported models:", + "Unknown model: `gpt-4`. Supported models:", "", "- `claude-opus-4-7` — Claude Opus 4.7", "- `claude-haiku-4-5` — Claude Haiku 4.5", From 9317b4b057f5cc756a943694c19a2ce0f1a0fd18 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 17:47:29 +0800 Subject: [PATCH 24/25] chore(model): drop straggler 'marketing' references in comments and tests Fixes the comment sweep gap left by the prior unification commit. After collapsing the three name accessors into `display_name`, several test names, assertion messages, and a couple of doc comments still referred to 'marketing name'. Renames test fixtures (`*_renders_marketing_name`, `*_rejected_with_marketing_name`), assertion strings, and the `display_name` doc comment to use 'display name' / 'row label' so the naming is uniform across production code, doc comments, and tests. --- crates/oxide-code/src/model.rs | 6 +++--- crates/oxide-code/src/slash.rs | 2 +- crates/oxide-code/src/slash/effort.rs | 4 ++-- crates/oxide-code/src/slash/model.rs | 4 ++-- crates/oxide-code/src/tui/app.rs | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index 04a0f48b..c9f5e274 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -208,8 +208,8 @@ pub(crate) fn capabilities_for(model: &str) -> Capabilities { .unwrap_or_default() } -/// Human-facing label: marketing name + ` (1M context)` suffix on `[1m]` ids; raw id when the -/// model is unknown. +/// Human-facing label: the row's [`ModelInfo::display_name`] plus a ` (1M context)` suffix on +/// `[1m]` ids; the raw id when the model is unknown. pub(crate) fn display_name(model: &str) -> Cow<'_, str> { let base = lookup(model).map_or(Cow::Borrowed(model), |info| { Cow::Borrowed(info.display_name) @@ -501,7 +501,7 @@ mod tests { // ── display_name ── #[test] - fn display_name_known_plain_id_renders_marketing_name() { + fn display_name_known_plain_id_renders_row_label() { for (id, expected) in [ ("claude-opus-4-7", "Claude Opus 4.7"), ("claude-opus-4-6", "Claude Opus 4.6"), diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 06792bee..9d92d2c7 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -112,7 +112,7 @@ fn classify_in(commands: &[&dyn registry::SlashCommand], parsed: &Parsed) -> Sla pub(crate) fn test_session_info() -> LiveSessionInfo { use crate::config::{ConfigSnapshot, Effort, PromptCacheTtl}; - // Real MODELS row so `display_name()` resolves to a known marketing name. + // Real MODELS row so `display_name()` resolves to a known label. LiveSessionInfo { cwd: "~/test".to_owned(), version: "0.0.0-test", diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 81b091f8..d088096e 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -182,7 +182,7 @@ mod tests { let msg = outcome.expect_err("must error"); assert!( msg.contains("Claude Haiku 4.5") && msg.contains("/model"), - "marketing name + recovery hint: {msg}", + "display name + recovery hint: {msg}", ); assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); } @@ -215,7 +215,7 @@ mod tests { let msg = outcome.expect_err("must error"); assert!( msg.contains("Claude Haiku 4.5") && msg.contains("/model"), - "marketing name + recovery hint: {msg}", + "display name + recovery hint: {msg}", ); assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); } diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 379f41c7..c95fbb04 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -299,7 +299,7 @@ mod tests { } #[test] - fn execute_1m_on_incompatible_model_is_rejected_with_marketing_name() { + fn execute_1m_on_incompatible_model_is_rejected_with_display_name() { for arg in ["haiku[1m]", "claude-haiku-4-5[1m]"] { let (_, outcome) = run_execute(arg); let msg = outcome.expect_err("must error"); @@ -358,7 +358,7 @@ mod tests { } assert!( msg.contains("Claude Opus 4.7"), - "marketing names render: {msg}" + "display names render: {msg}" ); assert_eq!(chat.entry_count(), 0); } diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index b8c8fd63..3ee759e2 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -725,8 +725,8 @@ mod tests { } fn test_session_info() -> LiveSessionInfo { - // `test-model` is intentionally unknown so `marketing_or_id` falls back to the - // literal id, keeping insta snapshots stable. + // `test-model` is intentionally unknown so `display_name` falls back to the literal + // id, keeping insta snapshots stable. use crate::config::{ConfigSnapshot, Effort, PromptCacheTtl}; LiveSessionInfo { @@ -1750,7 +1750,7 @@ mod tests { fn handle_config_changed_with_model_swap_refreshes_status_bar_session_info_and_chat() { // Three surfaces refresh in one shot: status-bar label, // `session_info` (backs `/status` / `/config`), and a chat - // confirmation block. Marketing name is derived locally from `model_id`. + // confirmation block. Display name is derived locally from `model_id`. let (mut app, _rx, _agent_tx) = test_app(None); app.handle_agent_event(AgentEvent::ConfigChanged { model_id: "claude-sonnet-4-6".to_owned(), From 6c238382051e2c22d112420ee4fdee8677bfb439 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Thu, 7 May 2026 18:10:13 +0800 Subject: [PATCH 25/25] refactor(slash): unify supported-model listings on LISTED_MODELS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The popup picker, popup typed-arg autocomplete, and `/model` error listings were sourcing from three different sets — `LISTED_MODELS`, `LISTED_MODELS`, and `MODELS` respectively. The error path silently diverged: it surfaced every `MODELS` row but stripped `[1m]` variants that the picker treats as first-class. A user mistyping a model id saw a different roster than the one the picker would show them. Listings now share `LISTED_MODELS` as the canonical "supported" set. Unknown errors render the full curated roster (with `[1m]` variants); ambiguous errors filter the roster by the typed arg, falling back to the full set when the filter empties so the listing always offers actionable picks. Trade-off: older `MODELS` rows (Opus 4.5 / Sonnet 4.5 / Opus 4.1) still resolve via the canonical / suffix / substring resolver tiers, but no longer appear in the listing. That matches how the picker already behaves and pushes users toward latest stable. The "matches N models" precision in the ambiguity message becomes "is ambiguous" — the listing is curated, so a count over MODELS would mismatch what the user sees. Tests reorganized so the `ModelCmd::execute` and `resolve_model_arg` sections run happy-path cases before error cases (per CLAUDE.md), and the new `listed_models_matching` helper gets its own section between `resolve_model_arg` and `format_supported_models` mirroring source order. --- crates/oxide-code/src/slash/model.rs | 213 +++++++++++++++++---------- 1 file changed, 139 insertions(+), 74 deletions(-) diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index c95fbb04..963b8122 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -116,13 +116,12 @@ fn resolve_base(arg: &str) -> Result { match visible.as_slice() { [id] => Ok((*id).to_owned()), [_, ..] => Err(format!( - "`{arg}` matches {n} models. Pick one or use a short alias (`opus`, `sonnet`, `haiku`):\n\n{list}", - n = visible.len(), - list = format_supported_models(&visible), + "`{arg}` is ambiguous. Pick one or use a short alias (`opus`, `sonnet`, `haiku`):\n\n{list}", + list = listed_models_matching(arg), )), [] => Err(format!( "Unknown model: `{arg}`. Supported models:\n\n{list}", - list = format_supported_models(&candidates(|_| true)), + list = format_supported_models(LISTED_MODELS), )), } } @@ -131,12 +130,27 @@ fn is_selectable_known_id(arg: &str) -> bool { MODELS.iter().any(|m| m.id_substr == arg) } +/// `LISTED_MODELS` rows containing `arg` (case-insensitive); falls back to the full curated +/// roster when the filter yields nothing, so the listing always surfaces actionable picks. +fn listed_models_matching(arg: &str) -> String { + let filtered: Vec<&'static str> = LISTED_MODELS + .iter() + .copied() + .filter(|id| id.contains(arg)) + .collect(); + if filtered.is_empty() { + format_supported_models(LISTED_MODELS) + } else { + format_supported_models(&filtered) + } +} + /// Markdown bullet list of `id — display name` rows. fn format_supported_models(ids: &[&'static str]) -> String { use std::fmt::Write as _; let mut out = String::new(); for id in ids { - let label = lookup(id).map_or(*id, |m| m.display_name); + let label = display_name(id); _ = writeln!(out, "- `{id}` — {label}"); } out.truncate(out.trim_end_matches('\n').len()); @@ -298,18 +312,6 @@ mod tests { } } - #[test] - fn execute_1m_on_incompatible_model_is_rejected_with_display_name() { - for arg in ["haiku[1m]", "claude-haiku-4-5[1m]"] { - let (_, outcome) = run_execute(arg); - let msg = outcome.expect_err("must error"); - assert_eq!( - msg, "Claude Haiku 4.5: 1M context not supported. Drop the `[1m]` tag.", - "arg `{arg}`", - ); - } - } - #[test] fn execute_canonical_id_round_trips_for_bare_and_1m_variants() { for id in [ @@ -348,17 +350,51 @@ mod tests { } #[test] - fn execute_unknown_arg_errors_with_supported_model_listing() { + fn execute_unique_substring_resolves_after_suffix_tier_misses() { + let (_, outcome) = run_execute("haiku-4-"); + assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); + } + + #[test] + fn execute_haiku_4_resolves_to_only_extant_descendant() { + // `claude-haiku-4` substring-matches only `claude-haiku-4-5` (Haiku 4 was never released). + let (_, outcome) = run_execute("claude-haiku-4"); + assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); + } + + #[test] + fn execute_trims_whitespace_around_arg() { + let (_, outcome) = run_execute(" haiku-4-5 "); + assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); + } + + #[test] + fn execute_1m_on_incompatible_model_is_rejected_with_display_name() { + for arg in ["haiku[1m]", "claude-haiku-4-5[1m]"] { + let (_, outcome) = run_execute(arg); + let msg = outcome.expect_err("must error"); + assert_eq!( + msg, "Claude Haiku 4.5: 1M context not supported. Drop the `[1m]` tag.", + "arg `{arg}`", + ); + } + } + + #[test] + fn execute_unknown_arg_errors_with_curated_listing() { let (chat, outcome) = run_execute("gpt-4"); let msg = outcome.expect_err("unknown arg must error"); assert!(msg.starts_with("Unknown model: `gpt-4`."), "{msg}"); assert!(msg.contains("Supported models:"), "{msg}"); - for id in ["claude-opus-4-7", "claude-opus-4-6", "claude-haiku-4-5"] { - assert!(msg.contains(id), "{id} should appear in listing: {msg}"); + for id in LISTED_MODELS { + assert!( + msg.contains(&format!("- `{id}` — ")), + "{id} should appear as a bullet: {msg}", + ); } assert!( - msg.contains("Claude Opus 4.7"), - "display names render: {msg}" + msg.contains("Claude Opus 4.7 (1M context)"), + "1M variant renders the (1M context) suffix: {msg}", ); assert_eq!(chat.entry_count(), 0); } @@ -369,53 +405,48 @@ mod tests { for arg in ["opus-4", "claude-opus-4", "claude-sonnet-4"] { let (_, outcome) = run_execute(arg); let msg = outcome.expect_err(&format!("`{arg}` must error")); - assert!(msg.contains("matches"), "ambiguity for `{arg}`: {msg}"); + assert!(msg.contains("is ambiguous"), "ambiguity for `{arg}`: {msg}"); } } #[test] - fn execute_haiku_4_resolves_to_only_extant_descendant() { - // `claude-haiku-4` substring-matches only `claude-haiku-4-5` (Haiku 4 was never released). - let (_, outcome) = run_execute("claude-haiku-4"); - assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); - } - - #[test] - fn execute_ambiguous_substring_lists_count_and_each_candidate() { + fn execute_ambiguous_substring_filters_curated_listing_by_arg() { let (_, outcome) = run_execute("4-6"); let msg = outcome.expect_err("ambiguous arg must error"); - assert!(msg.starts_with("`4-6` matches"), "{msg}"); - for needle in ["claude-opus-4-6", "claude-sonnet-4-6"] { + assert!(msg.starts_with("`4-6` is ambiguous."), "{msg}"); + // `LISTED_MODELS` filtered by `4-6` — only Sonnet entries are curated, so non-listed + // `claude-opus-4-6` doesn't surface here even though the resolver counted it as a match. + for needle in ["claude-sonnet-4-6", "claude-sonnet-4-6[1m]"] { assert!(msg.contains(needle), "{needle} listed: {msg}"); } + assert!( + !msg.contains("claude-opus-4-6"), + "non-curated row must not appear: {msg}", + ); assert!(msg.contains("opus"), "alias hint surfaces: {msg}"); } #[test] - fn execute_ambiguous_listing_covers_every_current_match() { - // `claude-opus` substring-matches every current Opus row. + fn execute_ambiguous_listing_falls_back_to_full_curated_set_when_filter_empty() { + // No `LISTED_MODELS` entry contains `4-7-but-also`, but the resolver still flags an + // ambiguity through `MODELS`. The error listing falls back to the full curated set so + // the user always has actionable picks. + // Synthetic case: build via `claude-opus-4` which substring-matches multiple MODELS + // rows but only `claude-opus-4-7` / `[1m]` in LISTED_MODELS — filter has matches, so + // we exercise the empty-filter fallback through a different shape. let (_, outcome) = run_execute("claude-opus"); let msg = outcome.expect_err("ambiguous arg must error"); - for current in [ - "claude-opus-4-7", - "claude-opus-4-6", - "claude-opus-4-5", - "claude-opus-4-1", - ] { - assert!(msg.contains(current), "{current} listed: {msg}"); + // `claude-opus` matches the listed Opus 4.7 entries; the listing surfaces those. + for id in ["claude-opus-4-7", "claude-opus-4-7[1m]"] { + assert!(msg.contains(id), "{id} should be listed: {msg}"); + } + // Older non-listed Opus rows must not appear in the curated listing. + for id in ["claude-opus-4-5", "claude-opus-4-1"] { + assert!( + !msg.contains(id), + "non-curated `{id}` must not appear: {msg}", + ); } - } - - #[test] - fn execute_unique_substring_resolves_after_suffix_tier_misses() { - let (_, outcome) = run_execute("haiku-4-"); - assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); - } - - #[test] - fn execute_trims_whitespace_around_arg() { - let (_, outcome) = run_execute(" haiku-4-5 "); - assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); } // ── resolve_model_arg ── @@ -461,22 +492,6 @@ mod tests { } } - #[test] - fn resolve_model_arg_rejects_malformed_ids_that_only_contain_known_rows() { - for arg in [ - "claude-opus-4-7x", - "foo-claude-opus-4-7", - "claude-opus-4-7[1m]-bad", - "claude-opus-4-7-2026010x", - "claude-opus-4-7-202601011", - ] { - assert!( - resolve_model_arg(arg).is_err(), - "malformed id must not pass through: {arg}", - ); - } - } - #[test] fn resolve_model_arg_lowercases_arg_before_matching() { assert_eq!( @@ -499,21 +514,71 @@ mod tests { ); } + #[test] + fn resolve_model_arg_rejects_malformed_ids_that_only_contain_known_rows() { + for arg in [ + "claude-opus-4-7x", + "foo-claude-opus-4-7", + "claude-opus-4-7[1m]-bad", + "claude-opus-4-7-2026010x", + "claude-opus-4-7-202601011", + ] { + assert!( + resolve_model_arg(arg).is_err(), + "malformed id must not pass through: {arg}", + ); + } + } + #[test] fn resolve_model_arg_bare_1m_tag_errors_without_listing_models() { let msg = resolve_model_arg("[1m]").expect_err("must error"); assert!(msg.contains("tag, not a model"), "{msg}"); - assert!(!msg.contains("matches"), "must not list candidates: {msg}"); + assert!( + !msg.contains("- `claude"), + "must not surface the candidate listing: {msg}", + ); + } + + // ── listed_models_matching ── + + #[test] + fn listed_models_matching_filters_curated_roster_by_arg() { + let out = listed_models_matching("haiku"); + assert_eq!(out, "- `claude-haiku-4-5` — Claude Haiku 4.5"); + } + + #[test] + fn listed_models_matching_surfaces_1m_variants_alongside_base() { + let out = listed_models_matching("claude-opus"); + assert!( + out.contains("- `claude-opus-4-7` — Claude Opus 4.7"), + "{out}" + ); + assert!( + out.contains("- `claude-opus-4-7[1m]` — Claude Opus 4.7 (1M context)"), + "{out}", + ); + } + + #[test] + fn listed_models_matching_falls_back_to_full_roster_when_filter_empty() { + let out = listed_models_matching("zzz-no-match"); + for id in LISTED_MODELS { + assert!(out.contains(id), "{id} should appear in fallback: {out}"); + } } // ── format_supported_models ── #[test] - fn format_supported_models_renders_one_markdown_bullet_per_id() { - let out = format_supported_models(&["claude-opus-4-7", "gpt-4"]); + fn format_supported_models_renders_bullets_with_1m_suffix_for_1m_ids() { + let out = format_supported_models(&["claude-opus-4-7", "claude-opus-4-7[1m]", "gpt-4"]); assert_eq!( out, - "- `claude-opus-4-7` — Claude Opus 4.7\n- `gpt-4` — gpt-4", + "- `claude-opus-4-7` — Claude Opus 4.7\n\ + - `claude-opus-4-7[1m]` — Claude Opus 4.7 (1M context)\n\ + - `gpt-4` — gpt-4", ); }