feat(slash): modal infrastructure — picker + /status + SwapConfig#64
Merged
feat(slash): modal infrastructure — picker + /status + SwapConfig#64
Conversation
Flat layout was creaking past ~10 files per directory. Group by topic so related docs sit together and new entries (e.g. slash/modals.md) have a natural home. Subdirs: api/, session/, slash/, tools/, tui/. Filenames lose the redundant topic prefix (slash-commands.md → slash/commands.md, etc.). docs/design/ mirrors docs/research/ for paired surfaces. Also fixes three stale source-comment paths that pointed at docs/research/design/* — a layout that hasn't existed since PR #51 — and adds a one-paragraph docs index pointer to root CLAUDE.md so agents land at docs/README.md without crawling.
…y with classify
Three coordinated renames so the trait surface speaks the same vocabulary
the new modal infrastructure will need:
- SlashOutcome::Local → Done. "Local" was opaque; the variant means
"command finished, no further dispatch needed".
- SlashOutcome::Action(_) → Forward(_). "Action" was redundant once
every outcome is an action; the new name names what the dispatcher
does with it.
- is_read_only(args) -> bool → classify(args) -> SlashKind. The bool
was already overloaded ("safe mid-turn" vs. "informational"); a
typed kind makes both axes explicit and gives modals a third
variant to slot in next commit.
SlashKind moves from slash.rs to registry.rs (alongside the trait that
owns it) and is re-exported. classify_in collapses the two-arm match
into a single direct delegation: lookup → cmd.classify(args).
Resolves slash-model-effort-followups.md §7. No behavior change.
Symmetric two-axis payload via `UserAction::SwapConfig { model, effort }`
and `AgentEvent::ConfigChanged { model_id, effort, requested_effort }`.
Resolves slash-model-effort-followups.md §8 and unblocks the upcoming
combined `/model + /effort` picker, which can mutate both axes in one
event without ordering bugs or double confirmations.
`requested_effort` separates the user's explicit pick from the resolved
value so `format_config_change` can surface clamping honestly. Helper
`apply_swap_config` lives in main.rs; `Client::effort()` getter mirrors
`Client::model()`.
Modal is a focus-grabbing UI overlay — render, height, handle_key — that produces a typed result on submission. ModalStack owns the active modal(s) and lives on App. Key gate runs first in handle_crossterm_event so a modal owns keyboard focus end-to-end while active. Layout band sits between the prompt preview and the slash popup; when the stack is empty the band is zero rows and the existing layout is unchanged. Synthetic ScriptedModal under #[cfg(test)] exercises the manager and the App-side gate end-to-end. Concrete modals + slash-dispatch wiring land in upcoming commits, which retire the cfg(not(test))-gated dead_code expectations on ModalKey, ModalAction::User, and ModalStack::push.
Reusable selection UI for any "pick one of N items" modal: cursor state, navigation (next/prev/jump-by-hint), and render. Items implement the small `PickerItem` trait — label, optional description, active marker, optional 1-9 key hint. Not a Modal itself: concrete pickers (model + effort, future theme, future approval) embed a ListPicker and own their submit semantics. This keeps the primitive free of `Box<dyn Fn(&T) -> ModalAction>` callbacks while still being broadly reusable. The `cfg(not(test))` dead-code expectation clears in the next commit when the combined `/model + /effort` picker becomes the first production consumer.
Bare /model and bare /effort both open a single ModelEffortPicker — the only difference is initial focus (model axis vs effort axis pre-armed). Both axes commit through one UserAction::SwapConfig event, so the agent loop sees an atomic config swap. SlashContext gains a `modal` slot that commands populate via `open_modal`; the dispatcher (App::apply_action_locally) takes it after execute and pushes onto the App's ModalStack. SlashOutcome stays a Done/Forward enum — no Box<dyn Modal> variant — which keeps the 26 in-tree assert_eq! sites untouched. Picker UX borrowed from Claude Code: numbered shortcuts (1-9), `>` cursor + `✓` active marker, effort axis under the model list with ← →. Drops the opinionated "Default (recommended)" label and the `/fast` upsell. Effort row hides on no-tier models (Haiku 4.5). The text-output `render_model_list` and `render_effort_list` helpers are gone; bare `/model` / `/effort` now open the picker. Manual `/model <id>` / `/effort <level>` typed-arg paths keep their direct SwapConfig forwarding for scripting and power users. Snapshot tests for the popup re-baselined for the updated descriptions on /model and /effort.
Single-panel read-only modal listing the live session descriptors (model, effort, working dir, session id, auth, version, context cache, show-thinking). Esc closes; Enter also closes since there's nothing to confirm; everything else is consumed locally so the input area never sees a key while the modal is up. Replaces the kv-table chat row /status used to print. The underlying SessionInfo + ConfigSnapshot data flow is unchanged — only the UI seam moved from chat into the modal band.
Capture the shipped modal infrastructure in docs/design/slash/modals.md — trait shape, ownership model, the 9 design decisions, and per-modal notes for the picker and /status. Update commands.md to reference the modal flow + the new SwapConfig payload, and the docs/design/README.md index. Mark the research note as implemented. CLAUDE.md crate tree gets entries for tui/modal.rs, tui/modal/list_picker.rs, slash/picker.rs, slash/status_modal.rs, plus refreshed descriptions on slash/effort.rs, slash/model.rs, slash/status.rs, and slash/context.rs.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…on row Three coverage gaps that codecov flagged were real user behaviors paired with a sibling test for the opposite case. Add the matching tests so a regression that swaps Forward/Backward, always renders the effort row, or always renders "off" fails immediately. - picker.rs: left_arrow_walks_effort_backward_with_wrap pins the Backward arithmetic in cycle_effort independently of Forward. - picker.rs: height_drops_when_highlighted_model_lacks_effort_tier and the no-tier branch in the render smoke test exercise the is_some_and(has_effort_tier) false arm. - status_modal.rs: new_renders_thinking_on_when_snapshot_says_true pins the on branch the default fixture cannot reach.
…n is set header_height reserved an extra row beyond what render emits, causing the picker to claim 1 more terminal row than it actually draws.
…n headers - picker.rs: replace stale reference to nonexistent `slash::model::LISTED_MODELS` - picker.rs, status_modal.rs: merge `super::` into `crate::` import block - picker.rs: rename test sections to function-name convention (new, handle_key, height, render); fold submit tests under handle_key - status_modal.rs: rename Construction → new, Render smoke → render
`apply_action_locally` was returning `false` for `SwapConfig` and `Clear`, so `dispatch_user_action` returned early without forwarding. Modal-emitted SwapConfig (e.g. from the `/model + /effort` picker submit) never reached the agent loop, so `client.set_model` never ran, no `ConfigChanged` event fired, and the title bar / chat confirmation stayed silent. Typed-arg `/model <id>` worked because the slash path uses `forward_to_agent` directly, bypassing the gate. Update the test fixture: `dispatch_local_only_actions_return_false_...` had encoded the bug as the contract. Rename + invert to assert SwapConfig and Clear actually forward through user_tx.
Adds a one-row horizontal separator above the modal body so the picker / status overlay visually delineates from the chat above (mirrors Claude Code's modal chrome). `ModalStack::height` reserves an extra row; `ModalStack::render` paints the dim `─` band on the first row, then renders the modal body in the remainder. Existing height-assertion tests pin the increment via `TOP_BORDER_HEIGHT` so the budget stays in sync.
The `[1m]` opt-in tag enables the 1M-context beta header on capable models. Opus 4.7's `context_1m` cap is `true`, so the default now lands users on the wider window without requiring an explicit `model = "claude-opus-4-7[1m]"` line in their config. The tag is client-side only — `betas::api_model_id` strips it before the wire — so no change to the model id sent in the request body.
…t coverage Splits `// ── ModalStack ──` into per-method sections (`is_active`, `push`, `render`, `handle_key`) so section order mirrors the production function order, per CLAUDE.md test conventions. Renames `nested_push_routes_keys_to_top_only` to `handle_key_with_nested_stack_routes_to_top_modal_only` to live under the right section. Also adds a `ListPicker` test that drives every `PickerItem` trait default (`description` / `is_active` / `key_hint` all unset) plus the no-description render branches — coverage gap surfaced by codecov.
Bare `/effort` opened the same combined picker as `/model`, only with the effort axis pre-armed — useful in theory but indistinguishable from `/model` in practice once both axes commit through one `SwapConfig`. The duplication confused the popup help (two commands for one surface) and made `/effort` look like a thin alias. Bare `/effort` now errors with a usage hint pointing at the picker; the typed `/effort <level>` form keeps the direct one-shot shortcut. Removes `InitialFocus` from `ModelEffortPicker` — only `/model` opens the picker now, so there's no second focus to choose. `effort_dirty` always starts `false`; the previous Effort-focus-arms-the-axis trick is gone. A future PR may add a Claude Code-style horizontal slider for `/effort` (speed ←→ intelligence axis), tracked in `.claude/plans/effort-slider-and-comment-sweep-merge.md`.
4 tasks
hakula139
added a commit
that referenced
this pull request
May 5, 2026
PR #64 (modal infrastructure) shipped Option C: bare /model opens the combined picker, bare /effort errors with a usage hint pointing at /model. The user guide, design notes, and roadmap still described the older "both bare forms open the picker with different initial focus" shape. Updated: - docs/guide/slash-commands.md — table description, mid-turn classification paragraph, and the "Switching the Effort" / "Switching the Model" sections. - docs/design/slash/commands.md — design decision #5, /effort and /model per-command notes, source list (`agent_loop_task` → `agent_turn`). - docs/design/slash/modals.md — design decisions #4 (`SessionInfo` → `LiveSessionInfo`) and #7 (typed-arg-only contract). - docs/roadmap.md — moved the combined picker out of "Current Focus" (shipped in PR #64) into Working Today; replaced with the deferred /effort slider. - CLAUDE.md — `slash/effort.rs` description updated to match the typed-arg contract.
hakula139
added a commit
that referenced
this pull request
May 5, 2026
…ps (#65) ## Summary Crate-wide cleanup applying the `CLAUDE.md` comment conventions: trims `//` blocks that restate WHAT the code does, expands one-line `///` docs into proper contract descriptions where signatures alone left preconditions / invariants / error modes implicit, and reflows surviving multi-line comments from ~80-col to the 100-col `rustfmt` `max_width`. Net: 1827 insertions, 3632 deletions across 110 files; coverage holds at 98.47% line / 97.97% function. ## Notable structural changes - **Renamed `slash::SessionInfo` → `LiveSessionInfo`** to disambiguate from `session::entry::SessionInfo` (persisted JSONL record). Two same-named types in different modules caused reader confusion and ambiguous imports. - **Test sections re-grouped to mirror production order** in `session/handle.rs` (SharedState first, file-snapshot tests folded into `finish` / `resume`), `config.rs` (Config::load sub-sections grouped, snapshot before display_effort), `tool.rs` (misplaced display_cwd_path_from test moved to its own section), and added missing `Capabilities::accepts_effort` / `default_effort` sections in `model.rs`. - **Sidecar types in `file_tracker.rs`** (`GatePurpose`, `FileSnapshot`, `RecordRead`, `StatCheck`, `GateError`) gained the contract docs their documented siblings already had. - **Slash docs refreshed** for the post-PR-#64 `/effort` typed-arg-only contract — guide, design notes, and roadmap (combined picker moved from Current Focus to Working Today). ## Conventions enforced - **WHY, not WHAT** — every retained comment explains intent, trade-offs, invariants, or constraints the code can't convey on its own. - **Contract over mechanics on `///`** — collapse to one line for simple contracts; expand to paragraphs + sublists when multiple facts need to land. - **100-col exactly** — multi-byte glyphs (`—`, `→`) count as one column, not three. Sublist continuations indent 2 spaces under `- ` to satisfy `clippy::doc_overindented_list_items`. ## Test plan - [x] `cargo fmt --all --check`, `cargo build`, `cargo clippy --all-targets -- -D warnings` — clean - [x] `cargo test` — 1612 tests pass - [x] `cargo llvm-cov --ignore-filename-regex 'main\.rs'` — 98.47% line / 97.97% function (held) - [x] `pnpm lint`, `pnpm spellcheck` — clean
5 tasks
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 free
to 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
Introduce a focus-grabbing modal overlay primitive for slash commands and ship two concrete consumers: a combined
/model + /effortpicker and a/statusoverview. Modals open above the input (with a one-row top separator), intercept keyboard focus while active, and emit results through the sameUserActionchannel as keys.Modaltrait +ModalStack(Vec-based for nesting) + genericListPicker<T: PickerItem>primitive (tui/modal.rs,tui/modal/list_picker.rs). Top-of-modal─band visually delineates the overlay from the chat above./model + /effortpicker — bare/modelopens the picker; the picker mutates both axes through one atomicSwapConfig. Typed/model <id>and/effort <level>keep their direct-switch behaviour. Bare/efforterrors with a usage hint pointing at/model(avoids the redundant lookalike-of-/modelsurface)./statusoverview — read-only kv modal; replaces the chat-row text output.SwapConfig/ConfigChanged— collapsesSwitchModel/SwitchEffortinto one payload so the picker can mutate both axes atomically;requested_effortletsformat_config_changesurface clamping (xhigh (clamped from max)) honestly.SlashOutcomerename —Local→Done,Action(_)→Forward(_);is_read_only(args) -> bool→classify(args) -> SlashKind. Mechanical, lands ahead of the modal-opening flow.DEFAULT_MODELtoclaude-opus-4-7[1m]so first-run sessions get the 1M context window.docs/research/anddocs/design/split by topic (api/,session/,slash/,tools/,tui/).Design decisions
Vecexists so a future "confirm leave?" overlay canpushwithout rework. Rejected boxed callbacks (lifetime /Sendcomplexity, hides dispatch graph) for typedModalKey::Submitted(ModalAction)— manager dispatches.SlashContext::open_modal, not a thirdSlashOutcomevariant. KeepsSlashOutcomederive-clean (Debug + PartialEq + Eq), so existingassert_eq!sites don't migrate tomatches!.ListPickeris not aModal. State + render only — concrete pickers embed it and own their submit semantics. Splits "list selection" from "what does Enter dispatch", broadly reusable without callbacks./modelopens the combined picker;/effortis typed-arg-only. Sharing the same modal between two bare commands made/effortlook like a/modelalias and split the discoverability surface for no real benefit. A horizontal slider for/effort(Speed ←→ Intelligence) is a follow-up.apply_action_locallyreturns "still forward". The dispatch path was returningfalseforSwapConfig/Clear, so modal-emitted swaps never reached the agent loop. Fixed to forward after the local apply, matching keyboard-typed actions.Test plan
cargo buildcleancargo clippy --all-targets -- -D warnings— zero warningscargo test— 1610 tests passcargo llvm-cov --ignore-filename-regex 'main\.rs'— picker / modal / status / list_picker all > 94% line coveragepnpm lint,pnpm spellcheck— clean/model,/effort <level>,/statusopen, navigate, submit / cancel cleanly across narrow / wide terminal widths