feat(slash): typed-arg autocomplete + always-picker popup + model cleanup - #68
Merged
Conversation
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.
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.
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.Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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).
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.
`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.
`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.
`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.- `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.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.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.
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.
`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.
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).
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`.
…or as list - 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.
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.Drop `marketing_name(&str) -> Option<&str>` and `marketing_or_id(&str) -> Cow<str>`. The single `display_name(&str) -> Cow<str>` 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.
- 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).
… intra-mode selection - 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.
`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.
Matches the canonical wrong-id example used in format_supported_models and the slash-model unknown-arg tests.
hakula139force-pushed
the
feat/slash-arg-autocomplete
branch
from
May 7, 2026 09:42
0d92244 to
1a2c2efCompare…ests 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.
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.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slash-command popup gains an arg mode for commands with curated value rosters (
/model,/effort,/theme): after/<name>plus a trailing space, the popup re-purposes itself to show valid argument values with descriptions, prefix-filtered as you type. The popup is always a picker — Tab and Enter both commit the highlighted row, and Esc dismisses the popup so the typed buffer can submit literally on the next Enter. A dim[id]/[level]/[name]placeholder paints alongside the popup at the cursor as a shape hint while the prefix is still empty.MODELSdrops the retiredclaude-opus-4/claude-sonnet-4rows and the never-releasedclaude-haiku-4row. The per-model effort capability flags (effort/effort_max/effort_xhigh) collapse into a singlesupported_efforts: &'static [Effort]slice. The three name accessors (marketing_name,marketing_or_id,display_name) collapse into a singledisplay_name(&str) -> Cow<str>. Picker, popup typed-arg autocomplete, and/modelerror listings all read from the sameLISTED_MODELScurated roster, so[1m]variants surface consistently and the listings stay in lockstep.Design decisions
PopupMode::NameandPopupMode::Arg { name }discriminate Tab-insertion shapes (/{name}vs/{name} {value}) inside oneSlashPopup. Mode transitions reset selection to row 0; intra-mode query / prefix changes clamp to keep the cursor sticky. Tab from name mode reclassifies against the new buffer — Tab on/mowrites/modeland the curated arg roster surfaces immediately, no second keypress needed.PopupStateexposed by parser, owned by input area.popup_state(buffer)publishesName(query)/Arg { name, prefix }from a single entry point so the input area decides what to show without re-parsing.[id]/[level]/[name]token at the cursor confirms the slot's shape and reminds users the resolver still accepts any matching id (aliases, dated ids, suffix matches).ArgCompletionusesCow<'static, str>for value + description. Static rosters (/effort,/theme) borrow; dynamic descriptions (/modelwith[1m]suffixes) own. Keeps the trait return type uniform without forcing all rosters to allocate.claude-opus-4/claude-sonnet-4/claude-haiku-4fromMODELSlets direct typing fall through to the same ambiguity / unknown-model error path as any foreign id; theis_family_basecolumn and its filter go away with them.effort/effort_max/effort_xhighbooleans collapse intosupported_efforts: &'static [Effort].accepts_effort,clamp_effort, anddefault_effortderive from the slice; default picks the highest non-Maxlevel so Max stays opt-in. The slice ordering invariant (ascending) is pinned by a smoke test over everyMODELSrow.display_name.marketing_name(Option) andmarketing_or_id(no 1M suffix) gone.display_name(&str) -> Cow<str>is the single seam: marketing name when known, raw id when unknown,(1M context)suffix on[1m]ids in both cases.format_config_changeno longer takes a separatemarketingparameter — it derives the label frommodel_idinternally. The misleadingLiveSessionInfo::marketing_name()(which actually wrappedmarketing_or_id) is gone; onlydisplay_name()remains.LISTED_MODELS. Picker, popup typed-arg autocomplete, and/modelambiguity / unknown errors all read from the same curated roster. The unknown error renders the full roster (with[1m]variants); the ambiguous error filters by the typed arg, falling back to the full roster when the filter empties so the listing always offers actionable picks. OlderMODELSrows still resolve via the canonical / suffix / substring resolver tiers — they just don't appear in advertised listings, matching how the picker treats them./modelerrors. Ambiguity and unknown-model errors emit a bullet list ofid — display namerows. TheErrorBlockrenderer iterates each logical line so the bullets stay on their own rows; the prior single-pass wrapping flattened embedded newlines and the bullets ran together as one ribbon.rank_by_prefixlifted out offilter_and_rank. Both the slash popup and per-command rosters need prefix-then-substring tier ranking that preserves declared order. The generic helper takesitems: &[T],query: &str,key: impl Fn(&T) -> &str, so future rosters reuse it without reimplementing.Effort::descriptionpaired with the enum. The intensity ladder lived in three places (Effort::ALL,Effort::VALID_VALUES,slash::effort::ARG_ROSTER). Description is now a method on the enum andcomplete_argiteratesEffort::ALLdirectly; adding a tier is a single-file change.Changes
slash/registry.rscomplete_arg(prefix) -> Vec<ArgCompletion>trait method (default empty) and theArgCompletion { value, description }struct.slash/matcher.rsrank_by_prefixgeneric helper (prefix-then-substring tier, declared-order preserving).slash/effort.rscomplete_argiteratesEffort::ALLand usesEffort::description; switches tocaps.has_effort()anddisplay_namefor the no-tier error.slash/model.rscomplete_argimpl reusingLISTED_MODELS; resolver no longer filters family-bases (rows removed);format_supported_models+listed_models_matchingsource error listings fromLISTED_MODELS.slash/theme.rscomplete_argimpl reusingLISTED_THEMES;Cowfor description text.slash/picker.rsLISTED_MODELSbumped topub(super)for reuse frommodel::complete_argand the error path;caps.has_effort()accessor.slash/effort_slider.rscaps.has_effort()accessor.slash/parser.rspopup_querywithPopupStateenum +popup_state(buffer).slash/context.rsLiveSessionInfo::marketing_name; onlydisplay_nameremains.slash/status_modal.rs,slash/config.rsinfo.display_name().slash.rsPopupState,popup_state,ArgCompletion. New helpers:complete_arg_for(name, prefix),arg_placeholder_for(name).model.rsis_family_baseand its filter; collapse effort flags intosupported_efforts: &'static [Effort]plushas_effort()accessor; collapsemarketing_name/marketing_or_id/display_nameinto a singledisplay_name(&str) -> Cow<str>.config.rsEffort::description(UX hint) +pub(crate) const fn as_str.client/anthropic/betas.rscaps.has_effort()accessor.prompt/environment.rslookupand readsinfo.display_name; drops retired-model rows from theknowledge_cutofftable-driven test.tui/components/input/popup.rsPopupMode { Name, Arg { name } }+PopupRow;set_state(Option<&PopupState>)rebuilds rows for either mode and resets selection on mode transitions; arg-mode renders bare values without/prefix; row labels cached once per render.tui/components/input.rspopup_querytopopup_state; mode-aware Tab insertion; Enter always commits the picked row when the popup is visible;submit()clears popup state;ghost_textpaints a dim placeholder at the cursor for arg-mode empty prefix.tui/components/chat/blocks/error.rsself.message.lines()so multi-line errors (markdown bullets) stay on their own rows instead of collapsing throughwrap_line.tui/app.rsformat_config_changedrops themarketingparameter and derives the label frommodel_idviadisplay_name.docs/guide/slash-commands.mddocs/roadmap.mdREADME.md/theme); mention name + curated-arg autocomplete.Test plan
cargo fmt --all --checkcargo clippy --all-targets -- -D warnings— zero warningscargo test --bin ox— 1722 passedpnpm lint,pnpm spellcheck— clean/{name}; Tab in arg mode inserts/{name} {value}; pinned byhandle_event_popup_tab_*tests against the publichandle_eventAPI./mo→/model) keeps the popup visible with the arg-mode roster —handle_event_popup_tab_in_name_mode_chains_into_arg_mode_for_curated_commands.handle_event_popup_enter_in_*tests.handle_event_popup_esc_then_enter_submits_typed_buffer_literally.ghost_textfires only inArgmode with an empty prefix, coexists with the popup, and suppresses once the user types any prefix; render-time paint pinned byrender_paints_ghost_text_at_cursor_in_arg_mode_with_empty_prefixand the suppression branch byrender_omits_ghost_text_once_user_types_arg_prefix.claude-opus-4/claude-sonnet-4/opus-4fall through to the markdown ambiguity listing;claude-haiku-4substring-resolves uniquely toclaude-haiku-4-5.LISTED_MODELS— unknown errors render the full curated roster including[1m]variants; ambiguous errors filter by the typed arg with full-roster fallback when the filter empties. Pinned byexecute_unknown_arg_errors_with_curated_listing,execute_ambiguous_substring_filters_curated_listing_by_arg, andexecute_ambiguous_listing_falls_back_to_full_curated_set_when_filter_empty.format_supported_modelsrenders one bullet per id with the(1M context)suffix on[1m]ids and raw-id fallback for unknown ids; pinned byformat_supported_models_renders_bullets_with_1m_suffix_for_1m_ids.listed_models_matchingfilters the curated roster by arg with a full-roster fallback when the filter empties, and surfaces[1m]variants alongside the base id.Capabilities::supported_effortsis ascending for everyMODELSrow, so theclamp_effort/default_effortreverse-walk stays sound —supported_efforts_is_ascending_for_every_models_row.set_stateclamps rather than resets selection (twin to the mode-transition reset test) —set_state_intra_mode_query_change_preserves_selection_via_clamp.ErrorBlockkeeps each logical line on its own row (markdown bullets stay separated) —render_multi_line_message_keeps_each_line_on_its_own_row./modelopens popup with curated roster AND the dim[id]hint; typeclaude-filters and clears the hint; Tab inserts the picked id with trailing space./effortand/themeshow their rosters plus[level]/[name]hints./model claude-opus-4+ Enter commits the popup pick (always-picker); Esc + Enter submits the typed buffer literally and surfaces the markdown ambiguity error with the curatedLISTED_MODELSlisting./model gptshows the unknown-model error with the curated roster including[1m]variants and(1M context)suffix labels.