From 8e46a77f4a5ff9eec014888e35fe1073f5813239 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 00:46:54 +0800 Subject: [PATCH 01/19] docs: reorganize research/ and design/ into topic subdirectories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .cspell/words.txt | 3 +- CLAUDE.md | 7 +- crates/oxide-code/src/session/actor.rs | 2 +- crates/oxide-code/src/slash.rs | 2 +- crates/oxide-code/src/slash/init.rs | 2 +- docs/design/README.md | 35 ++++++-- docs/design/{ => session}/file-tracking.md | 0 .../persistence.md} | 0 .../{slash-commands.md => slash/commands.md} | 0 .../truncation.md} | 0 .../cancellation.md} | 0 docs/design/{tui.md => tui/overview.md} | 0 docs/research/README.md | 50 +++++++---- .../{anthropic-api.md => api/anthropic.md} | 0 docs/research/{ => api}/extended-thinking.md | 0 docs/research/{ => api}/system-prompt.md | 6 +- docs/research/{ => session}/file-tracking.md | 0 .../persistence.md} | 0 .../{slash-commands.md => slash/commands.md} | 10 ++- docs/research/slash/modals.md | 87 +++++++++++++++++++ .../truncation.md} | 0 .../cancellation.md} | 0 docs/research/{tui.md => tui/overview.md} | 0 23 files changed, 168 insertions(+), 36 deletions(-) rename docs/design/{ => session}/file-tracking.md (100%) rename docs/design/{session-persistence.md => session/persistence.md} (100%) rename docs/design/{slash-commands.md => slash/commands.md} (100%) rename docs/design/{tool-truncation.md => tools/truncation.md} (100%) rename docs/design/{cancellation-and-queued-input.md => tui/cancellation.md} (100%) rename docs/design/{tui.md => tui/overview.md} (100%) rename docs/research/{anthropic-api.md => api/anthropic.md} (100%) rename docs/research/{ => api}/extended-thinking.md (100%) rename docs/research/{ => api}/system-prompt.md (97%) rename docs/research/{ => session}/file-tracking.md (100%) rename docs/research/{session-persistence.md => session/persistence.md} (100%) rename docs/research/{slash-commands.md => slash/commands.md} (78%) create mode 100644 docs/research/slash/modals.md rename docs/research/{tool-truncation.md => tools/truncation.md} (100%) rename docs/research/{cancellation-and-queued-input.md => tui/cancellation.md} (100%) rename docs/research/{tui.md => tui/overview.md} (100%) diff --git a/.cspell/words.txt b/.cspell/words.txt index e5db79c5..f5d72701 100644 --- a/.cspell/words.txt +++ b/.cspell/words.txt @@ -22,7 +22,6 @@ deserialize desync disambiguable dtolnay -frappe EACCES EISDIR ENOENT @@ -30,6 +29,7 @@ ENOSPC ENOTDIR ESRCH feff +frappe getpwuid gitui hakula @@ -38,6 +38,7 @@ indoc insta isatty killpg +Kobalte latte macchiato misparse diff --git a/CLAUDE.md b/CLAUDE.md index 68c3b33f..b346f22d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -144,6 +144,11 @@ ox # Start an interactive session └── text.rs # Display-width-aware text helpers (`truncate_to_width`, `ELLIPSIS`) ``` +## Documentation + +- [`docs/README.md`](docs/README.md) — top-level index of design specs, research notes, user guides, and the roadmap. +- Subdirectories under [`docs/research/`](docs/research/) and [`docs/design/`](docs/design/) carry their own README index with per-doc summaries grouped by topic (api, session, slash, tools, tui). + ## Coding Conventions ### Trait Design @@ -241,7 +246,7 @@ Follows global CLAUDE.md commit / branch / PR conventions, plus: - Keep `README.md` user-facing. It should describe value, supported features, and usage, not internal progress tracking. - Keep `docs/roadmap.md` as the canonical in-repo roadmap / status summary. Update it when shipped capability areas or planned priorities change. - Crate structure diagrams must match the actual filesystem. When adding, removing, or renaming modules, update the tree in this file. Entries are sorted alphabetically; directories sort alongside their parent `.rs` file. -- After substantive changes, sweep docs for stale claims: `README.md` status bullets, `docs/roadmap.md` Working Today / Current Focus sections, this file's crate tree and conventions, `docs/guide/*` user instructions, and `docs/research/*` deferred / follow-up notes that the change now resolves. +- After substantive changes, sweep docs for stale claims: `README.md` status bullets, `docs/roadmap.md` Working Today / Current Focus sections, this file's crate tree and conventions, `docs/guide/*` user instructions, and `docs/research/**/*` deferred / follow-up notes that the change now resolves. ## Verification diff --git a/crates/oxide-code/src/session/actor.rs b/crates/oxide-code/src/session/actor.rs index 0029c0fd..b2b4e7e7 100644 --- a/crates/oxide-code/src/session/actor.rs +++ b/crates/oxide-code/src/session/actor.rs @@ -8,7 +8,7 @@ //! before this drain runs; isolated writes (a text-only turn, the //! AI title append, the final summary) flush immediately because //! the drain returns `Empty` after the first cmd. No interval timer -//! — see `docs/research/design/session-persistence.md`. +//! — see `docs/design/session/persistence.md`. use std::sync::Arc; diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 4c19a636..9b8cacff 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -12,7 +12,7 @@ //! //! Persistence: commands never write user config files. Mutations are //! session-local; restart returns to the user-declared config (see -//! `docs/research/design/slash-commands.md` § Design Decisions 6). +//! `docs/design/slash/commands.md` § Design Decisions 6). mod clear; mod config; diff --git a/crates/oxide-code/src/slash/init.rs b/crates/oxide-code/src/slash/init.rs index 7f1a8d90..8cc554e2 100644 --- a/crates/oxide-code/src/slash/init.rs +++ b/crates/oxide-code/src/slash/init.rs @@ -1,6 +1,6 @@ //! `/init` — synthesizes a prompt asking the model to author or update //! the project's `AGENTS.md` / `CLAUDE.md`. See -//! `docs/research/design/slash-commands/init.md`. +//! `docs/design/slash/commands.md` § /init. use indoc::indoc; diff --git a/docs/design/README.md b/docs/design/README.md index 94e79087..a8341411 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -2,11 +2,30 @@ Architecture decisions and implementation specs for oxide-code. -| Document | Description | -| ----------------------------------------------------------------- | ------------------------------------------------------- | -| [Session Persistence](session-persistence.md) | JSONL format, actor-owned writes, resume semantics | -| [Terminal UI](tui.md) | Core stack, rendering strategy, streaming architecture | -| [Tool Output Truncation](tool-truncation.md) | Per-tool view-shape caps + centralized byte-budget | -| [File Change Tracking](file-tracking.md) | Read-before-Edit gate, staleness detection, persistence | -| [Cancellation and Queued Input](cancellation-and-queued-input.md) | Cancel, exit, and mid-turn queued prompts | -| [Slash Commands](slash-commands.md) | Registry, dispatch, popup, per-command notes | +Organized by topic. Each subdirectory mirrors the corresponding directory in [`docs/research/`](../research/), where the underlying research lives. + +## Session + +| Document | Description | +| ----------------------------------------------------- | ------------------------------------------------------- | +| [Persistence](session/persistence.md) | JSONL format, actor-owned writes, resume semantics | +| [File Change Tracking](session/file-tracking.md) | Read-before-Edit gate, staleness detection, persistence | + +## Slash Commands + +| Document | Description | +| ----------------------------------------------------- | ---------------------------------------------------- | +| [Commands](slash/commands.md) | Registry, dispatch, popup, per-command notes | + +## Tools + +| Document | Description | +| ----------------------------------------------------- | ---------------------------------------------------- | +| [Output Truncation](tools/truncation.md) | Per-tool view-shape caps + centralized byte-budget | + +## Terminal UI + +| Document | Description | +| ----------------------------------------------------- | ------------------------------------------------------ | +| [Overview](tui/overview.md) | Core stack, rendering strategy, streaming architecture | +| [Cancellation and Queued Input](tui/cancellation.md) | Cancel, exit, and mid-turn queued prompts | diff --git a/docs/design/file-tracking.md b/docs/design/session/file-tracking.md similarity index 100% rename from docs/design/file-tracking.md rename to docs/design/session/file-tracking.md diff --git a/docs/design/session-persistence.md b/docs/design/session/persistence.md similarity index 100% rename from docs/design/session-persistence.md rename to docs/design/session/persistence.md diff --git a/docs/design/slash-commands.md b/docs/design/slash/commands.md similarity index 100% rename from docs/design/slash-commands.md rename to docs/design/slash/commands.md diff --git a/docs/design/tool-truncation.md b/docs/design/tools/truncation.md similarity index 100% rename from docs/design/tool-truncation.md rename to docs/design/tools/truncation.md diff --git a/docs/design/cancellation-and-queued-input.md b/docs/design/tui/cancellation.md similarity index 100% rename from docs/design/cancellation-and-queued-input.md rename to docs/design/tui/cancellation.md diff --git a/docs/design/tui.md b/docs/design/tui/overview.md similarity index 100% rename from docs/design/tui.md rename to docs/design/tui/overview.md diff --git a/docs/research/README.md b/docs/research/README.md index 78697ecb..e4328776 100644 --- a/docs/research/README.md +++ b/docs/research/README.md @@ -2,21 +2,39 @@ External research and API reference for oxide-code development. Covers Claude Code, OpenAI Codex, opencode, and the Anthropic API. +Organized by topic. Each subdirectory mirrors the corresponding directory in [`docs/design/`](../design/), where shipped decisions live. + ## API References -| Document | Description | -| ----------------------------------------- | -------------------------------------------------- | -| [Anthropic API](anthropic-api.md) | OAuth flow, required headers, system prompt prefix | -| [Extended Thinking](extended-thinking.md) | Content block types, signatures, round-tripping | -| [System Prompt](system-prompt.md) | Section assembly, CLAUDE.md, caching, block layout | - -## Design Surveys - -| Document | Description | -| ----------------------------------------------------------------- | -------------------------------------------------------- | -| [Session Persistence](session-persistence.md) | JSONL format, storage layout, write strategy | -| [Terminal UI](tui.md) | Reference TUI patterns, flickering prevention, ecosystem | -| [Tool Output Truncation](tool-truncation.md) | Per-tool vs central caps, spillover strategies | -| [File Change Tracking](file-tracking.md) | Read-before-Edit gates, staleness detection | -| [Cancellation and Queued Input](cancellation-and-queued-input.md) | Cancel, exit, and input queueing patterns | -| [Slash Commands](slash-commands.md) | Registry shape, popup UX, execution models | +| Document | Description | +| ----------------------------------------------------- | -------------------------------------------------- | +| [Anthropic API](api/anthropic.md) | OAuth flow, required headers, system prompt prefix | +| [Extended Thinking](api/extended-thinking.md) | Content block types, signatures, round-tripping | +| [System Prompt](api/system-prompt.md) | Section assembly, CLAUDE.md, caching, block layout | + +## Session + +| Document | Description | +| ----------------------------------------------------- | ---------------------------------------------------- | +| [Persistence](session/persistence.md) | JSONL format, storage layout, write strategy | +| [File Change Tracking](session/file-tracking.md) | Read-before-Edit gates, staleness detection | + +## Slash Commands + +| Document | Description | +| ----------------------------------------------------- | ---------------------------------------------------- | +| [Commands](slash/commands.md) | Registry shape, popup UX, execution models | +| [Modals](slash/modals.md) | Picker / dialog primitives across the three CLIs | + +## Tools + +| Document | Description | +| ----------------------------------------------------- | ---------------------------------------------------- | +| [Output Truncation](tools/truncation.md) | Per-tool vs central caps, spillover strategies | + +## Terminal UI + +| Document | Description | +| ----------------------------------------------------- | -------------------------------------------------------- | +| [Overview](tui/overview.md) | Reference TUI patterns, flickering prevention, ecosystem | +| [Cancellation and Queued Input](tui/cancellation.md) | Cancel, exit, and input queueing patterns | diff --git a/docs/research/anthropic-api.md b/docs/research/api/anthropic.md similarity index 100% rename from docs/research/anthropic-api.md rename to docs/research/api/anthropic.md diff --git a/docs/research/extended-thinking.md b/docs/research/api/extended-thinking.md similarity index 100% rename from docs/research/extended-thinking.md rename to docs/research/api/extended-thinking.md diff --git a/docs/research/system-prompt.md b/docs/research/api/system-prompt.md similarity index 97% rename from docs/research/system-prompt.md rename to docs/research/api/system-prompt.md index 33813c64..872330d1 100644 --- a/docs/research/system-prompt.md +++ b/docs/research/api/system-prompt.md @@ -140,7 +140,7 @@ Tool schemas are sent via the API `tools` parameter, **not** in the system promp The API supports prompt caching via `cache_control` on `TextBlockParam` blocks. Cache scopes: -- `global` — static instructions identical across all sessions. **First-party only**; 3P gateways reject a `scope: "global"` block downstream of tool definitions (they render before `system` and taint the cache prefix). See [Anthropic API § Prompt Caching Scope](anthropic-api.md#prompt-caching-scope) for the full invariance rule. +- `global` — static instructions identical across all sessions. **First-party only**; 3P gateways reject a `scope: "global"` block downstream of tool definitions (they render before `system` and taint the cache prefix). See [Anthropic API § Prompt Caching Scope](anthropic.md#prompt-caching-scope) for the full invariance rule. - _(absent)_ — default (org-scoped) ephemeral cache. Universally accepted. - `null` (no `cache_control`) — dynamic content, not cached. @@ -211,11 +211,11 @@ The URL includes a `?beta=true` query parameter. Third-party gateways validate the system-block layout in addition to wire-shape signals. Empirically, prompt-content checks are content-similarity-based and treat the static prefix as load-bearing: -- The identity prefix block (`"You are Claude Code..."` and friends, see [Anthropic API § System prompt prefix](anthropic-api.md#2-system-prompt-prefix-as-a-separate-block)) must occupy its own block. Concatenating it into the prompt body fails the same way 1P fails non-Haiku OAuth. +- The identity prefix block (`"You are Claude Code..."` and friends, see [Anthropic API § System prompt prefix](anthropic.md#2-system-prompt-prefix-as-a-separate-block)) must occupy its own block. Concatenating it into the prompt body fails the same way 1P fails non-Haiku OAuth. - Static section text is accepted when it closely matches the known Claude Code prompt content shipped with this version. Heavily customized static prompts can trip the verifier even when every header is correct. - Dynamic sections (after the `SYSTEM_PROMPT_DYNAMIC_BOUNDARY` marker) are accepted alongside valid static content. The boundary itself is consumed by `splitSysPromptPrefix()` and never reaches the wire. -For the wire-shape side of the check (Stainless headers, billing attestation, beta header set, `metadata.user_id` shape, `User-Agent`), see [Anthropic API § Third-Party Gateway Validation](anthropic-api.md#third-party-gateway-validation). +For the wire-shape side of the check (Stainless headers, billing attestation, beta header set, `metadata.user_id` shape, `User-Agent`), see [Anthropic API § Third-Party Gateway Validation](anthropic.md#third-party-gateway-validation). ## Sources diff --git a/docs/research/file-tracking.md b/docs/research/session/file-tracking.md similarity index 100% rename from docs/research/file-tracking.md rename to docs/research/session/file-tracking.md diff --git a/docs/research/session-persistence.md b/docs/research/session/persistence.md similarity index 100% rename from docs/research/session-persistence.md rename to docs/research/session/persistence.md diff --git a/docs/research/slash-commands.md b/docs/research/slash/commands.md similarity index 78% rename from docs/research/slash-commands.md rename to docs/research/slash/commands.md index f22f1635..5ea284ef 100644 --- a/docs/research/slash-commands.md +++ b/docs/research/slash/commands.md @@ -1,12 +1,14 @@ # Slash Commands (Reference) -Research on client-side command surfaces. Based on [Claude Code](https://github.com/hakula139/claude-code) (v2.1.87), [OpenAI Codex](https://github.com/openai/codex), and [opencode](https://github.com/anomalyco/opencode). +Research on client-side command surfaces. Based on [Claude Code](https://github.com/hakula139/claude-code), [OpenAI Codex](https://github.com/openai/codex), and [opencode](https://github.com/anomalyco/opencode). Cross-checked against locally-mirrored sources (2026-05-05). + +For modal-specific architecture (how `local-jsx` / `BottomPaneView` / `dialog.show()` actually work) see [modals.md](modals.md). ## Claude Code (TypeScript) Declarative registry with three execution modes; lazy-loaded implementations. -- **Registry**: `COMMANDS()` returns ~24 `Command` records. Metadata: `name`, `aliases`, `description`, `type: 'local' | 'local-jsx' | 'prompt'`, `isEnabled`, `isHidden`, `immediate`, `isSensitive`, `availability`. +- **Registry**: ~100 `Command` records under `src/commands//index.ts`, ~50 of which are `local-jsx` modals. Metadata: `name`, `aliases`, `description`, `type: 'local' | 'local-jsx' | 'prompt'`, `isEnabled`, `isHidden`, `immediate`, `isSensitive`, `availability`. Each command directory ships its own modal component (`.tsx`) loaded via `load: () => import('./.js')`. - **Parser**: `slashCommandParsing.ts` splits on whitespace. Unknown names use Fuse.js (threshold 0.3). - **Dispatch**: Three modes -- `local` returns `{ resultText, displayMode }`, `local-jsx` returns React JSX (modal pickers), `prompt` expands to text and submits. - **Output**: Display modes: `'skip'` (no transcript entry), `'system'` (synthetic local-stdout message), `'user'` (default). Meta flag (`isMeta: true`) keeps a message model-visible while hiding from UI. @@ -39,7 +41,7 @@ Slim `CommandOption[]` from a React hook (~12 built-ins). | Repo | Registry shape | Variants | Parser site | Dispatch | Output target | Custom commands | | ----------- | --------------------------- | -------- | -------------- | --------------------- | ----------------------------------- | ---------------------------- | -| Claude Code | declarative `Command[]` | ~24 | submit handler | three modes | synthetic messages w/ display modes | yes (markdown + YAML) | -| Codex | strum enum + impl methods | ~50 | input layer | one big `match` | synthetic `history_cell` | no | +| Claude Code | declarative `Command[]` | ~100 | submit handler | three modes | synthetic messages w/ display modes | yes (markdown + YAML) | +| Codex | strum enum + impl methods | ~55 | input layer | one big `match` | synthetic `history_cell` | no | | opencode | `CommandOption[]` from hook | ~12 | input layer | closures + server | toast / dialog / synthetic message | yes (server-published) | | oxide-code | trait + `&[&dyn]` slice | 8 | submit handler | `SlashOutcome` return | `SystemMessageBlock` / `ErrorBlock` | not yet (namespace reserved) | diff --git a/docs/research/slash/modals.md b/docs/research/slash/modals.md new file mode 100644 index 00000000..4fe20a02 --- /dev/null +++ b/docs/research/slash/modals.md @@ -0,0 +1,87 @@ +# Slash-Command Modals (Reference) + +Research on the modal / picker / dialog primitives that turn slash commands into interactive surfaces (live model picker, theme switcher, permission prompt). Companion to [commands.md](commands.md), which covers the command-surface shape; this file focuses on the interactive UI. + +Verified against locally-mirrored sources (2026-05-05): [Claude Code](https://github.com/hakula139/claude-code), [OpenAI Codex](https://github.com/openai/codex) `codex-rs/tui`, [opencode](https://github.com/anomalyco/opencode) `packages/`. + +## Claude Code (TypeScript + Ink) + +Modals are React components rendered by Ink. ~50 of ~100 commands are `type: 'local-jsx'`. + +- **Discriminator**: `type: 'local-jsx'` on the command record. Lazy module via `load: () => import('./.js')` — the modal component never loads until the command runs. +- **Registry shape**: each command directory ships `index.ts` (declarative metadata) + `.tsx` (the modal component itself). Adding a modal command is one directory. +- **Lifecycle**: trigger → lazy-load module → mount component via `showSetupDialog(root, render)` → component captures keys (arrows / Enter / Esc) → `onSelect(value)` callback → unmount → result delivered to dispatcher. +- **State ownership**: per-modal local state (selected index, filter query). No shared modal-state store; each component is self-contained. +- **Layering**: modal renders **above** the input area, **not** inside chat scroll. Chat is visible behind. Input is implicitly suppressed because Ink routes keys to the active component. +- **Result delivery**: callback-driven. Modal calls `onDone(value, { displayMode, shouldQuery })`. `displayMode: 'system'` posts a synthetic `SystemMessageBlock`; `shouldQuery: true` forwards to the agent loop. +- **Reusable primitives**: lightweight — `Box`, `Text`, `SelectInput`-style hand-rolled lists. No shared `Modal` / `Dialog` wrapper. Each command builds its UI directly with Ink primitives. +- **Nesting**: supported via React tree (modal can render another modal as a child). +- **Live data**: dynamic `description` getter on the command record pulls live state (e.g., `Set the AI model (currently ${renderModelName(getMainLoopModel())})`). + +## OpenAI Codex (Rust + Ratatui) + +Trait-based view stack at the bottom of the frame. The closest stack-wise to oxide-code. + +- **Core trait**: `BottomPaneView` (`tui/src/bottom_pane/bottom_pane_view.rs`). Methods: `handle_key_event`, `is_complete`, `completion()`, `on_ctrl_c`, `terminal_title_requires_action`, plus paste / approval / input-request consumption hooks. +- **View stack**: `BottomPane` owns `view_stack: Vec>` plus a permanent `ChatComposer` that hides while a view is active. +- **Focus model**: stack presence **is** focus state. Non-empty stack = top view receives keys; empty stack = composer receives keys. No global flag. +- **Auto-cascade**: when `view.is_complete()` returns true, `BottomPane` pops the view, re-renders the next one (or composer), and emits the view's `completion()` payload as an `AppEvent`. +- **Triggers**: two paths. + 1. Slash command match arm pushes a view (`/model`, `/effort`, etc.). + 2. Agent emits `AskForApproval` event → `BottomPane::try_consume_approval_request()` pushes an `ApprovalOverlay`. +- **Reusable primitives**: + - `ListSelectionView` — generic ranked + filtered picker (`SelectionItem` items, Up / Down / numeric shortcuts, side-by-side preview). + - `MultiSelectPicker` — checkbox variant. + - `SelectionTabs` — tab bar across multiple list views. + - All implement the `Renderable` ratatui-widget trait. +- **Result delivery**: async via `AppEvent::SubmitApprovalDecision { decision, ... }`. The agent doesn't block; views drop a typed event on the app event channel. +- **Keymap composition**: `RuntimeKeymap` + per-view overrides (`ApprovalKeymap` locks Esc to Cancel; `ListKeymap` adds j/k navigation). + +## opencode (TypeScript + Solid.js + Kobalte) + +Imperative single-modal API. + +- **Primitive**: `dialog.show(component, onClose?)` from `useDialog()` context. Single active modal — `.show()` disposes any prior modal before mounting. +- **Component shape**: dialog components live in `packages/app/src/components/dialog-*.tsx`. Each is a Solid component wrapping a Kobalte `` (overlay, focus trap, accessibility built in). +- **Trigger pattern**: command's `onSelect` handler does a lazy import then `dialog.show()`: + + ```typescript + const chooseModel = () => { + void import("@/components/dialog-select-model").then((x) => { + dialog.show(() => ) + }) + } + ``` + +- **Layering**: full-screen modal overlay via Kobalte portal. `z: modal` for dialogs, `z: toast` (higher) for toasts. Input field is rendered outside the portal but is layered under the overlay. +- **Result delivery**: side effects + explicit close. Component mutates app state directly (`local.model.set(selected)`) and calls `dialog.close()`. No promise-based result return. +- **Toast vs dialog**: separate. `showToast({ variant, title, description })` is a different API for non-blocking notifications, never embedded in a dialog. +- **Nesting**: not supported (rationale: simpler focus management, but blocks confirm-inside-picker UX). + +## Comparison + +| Aspect | Claude Code | Codex (Rust) | opencode | +| ------------------ | ------------------------------------ | ----------------------------------- | ------------------------------ | +| Trigger | `local-jsx` discriminator on command | match arm in slash dispatch | command `onSelect` callback | +| Trait / API | React component | `BottomPaneView` trait + view stack | imperative `dialog.show(node)` | +| Focus gating | implicit (Ink scope) | stack presence | full-screen overlay + portal | +| Nesting | yes (React tree) | yes (Vec stack) | no (single-at-a-time) | +| Result delivery | callback (`onDone(value, opts)`) | async event (`AppEvent::*`) | side effects + `close()` | +| Reusable primitive | none — each modal hand-rolled | `ListSelectionView` (generic) | Kobalte Dialog primitives | +| Lazy load | `load: () => import(...)` | n/a (compiled binary) | `void import(...).then()` | +| Agent-triggered | no first-class path | `try_consume_approval_request()` | no first-class path | + +## Patterns Worth Borrowing for oxide-code + +1. **Codex's `BottomPaneView` trait + view stack.** Idiomatic Rust+Ratatui shape — small trait, single owner, stack semantics give nesting for free without adding focus-flag bookkeeping. Direct port target. +2. **Codex's generic `ListSelectionView`.** A single picker primitive parameterized by an item trait is what `/model`, `/effort`, `/theme`, future `/agents`, and any approval prompt all want. Beats hand-rolling per command (Claude Code's path). +3. **Two open paths from day one.** Slash-triggered + agent-triggered. The trait shape is identical for both — only the call site differs. Codex already does this with `try_consume_approval_request`. Designing the trait for both up front means the future Permission feature drops in without re-shaping. +4. **Async event for completion, not callback.** Modal pushes a `UserAction` (or richer `ModalEvent`) onto the existing `user_tx` channel. Symmetric with how the rest of oxide-code's UI talks to the agent loop. Avoids the `Box` / lifetime gymnastics of a callback-based design. +5. **Lazy live-data getters on slash command metadata.** Claude Code's dynamic `description()` lets the popup show `(currently sonnet)` without a refresh hook. Cheap, useful, doesn't require the modal infrastructure. + +## Patterns to Reject + +1. **opencode's single-modal-only.** Saves ~30 lines of stack code; costs every confirm-inside-picker flow forever. +2. **Claude Code's per-command UI hand-rolling.** With ~50 modal commands they need either a shared kit or a lot of duplication. A generic picker primitive scales better. Build it before the second modal lands. +3. **opencode's imperative `dialog.show()` with side-effecting components.** State-mutation-as-result is hard to test in Rust. Prefer typed `ModalAction` returns. +4. **Codex's permanent `ChatComposer` behind every view.** Useful for "preserve typed text across modal dismiss" but adds layout complexity. Only worth it if oxide-code's typing-during-modal turns out to be common — defer. diff --git a/docs/research/tool-truncation.md b/docs/research/tools/truncation.md similarity index 100% rename from docs/research/tool-truncation.md rename to docs/research/tools/truncation.md diff --git a/docs/research/cancellation-and-queued-input.md b/docs/research/tui/cancellation.md similarity index 100% rename from docs/research/cancellation-and-queued-input.md rename to docs/research/tui/cancellation.md diff --git a/docs/research/tui.md b/docs/research/tui/overview.md similarity index 100% rename from docs/research/tui.md rename to docs/research/tui/overview.md From 271ca5024c4a592fbadc01a3d099d2d4bf16ce73 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 00:47:39 +0800 Subject: [PATCH 02/19] docs: expand CLAUDE.md docs index to include guide/ and roadmap.md --- CLAUDE.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index b346f22d..77213ad7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -147,7 +147,9 @@ ox # Start an interactive session ## Documentation - [`docs/README.md`](docs/README.md) — top-level index of design specs, research notes, user guides, and the roadmap. -- Subdirectories under [`docs/research/`](docs/research/) and [`docs/design/`](docs/design/) carry their own README index with per-doc summaries grouped by topic (api, session, slash, tools, tui). +- [`docs/guide/`](docs/guide/) — user-facing docs (quickstart, configuration, slash commands, instructions, sessions, theming). +- [`docs/design/`](docs/design/) and [`docs/research/`](docs/research/) — internal architecture decisions and external research, both organized by topic (api, session, slash, tools, tui). Each subdirectory has its own README with per-doc summaries. +- [`docs/roadmap.md`](docs/roadmap.md) — working features, current focus, and explicit non-goals. ## Coding Conventions From f5ef5091747f144fc3f79ca0448c4944952dfc1c Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 00:58:53 +0800 Subject: [PATCH 03/19] refactor(slash): rename SlashOutcome variants and replace is_read_only with classify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/oxide-code/src/slash.rs | 22 ++------- crates/oxide-code/src/slash/clear.rs | 18 +++---- crates/oxide-code/src/slash/config.rs | 2 +- crates/oxide-code/src/slash/context.rs | 2 +- crates/oxide-code/src/slash/diff.rs | 4 +- crates/oxide-code/src/slash/effort.rs | 26 ++++++----- crates/oxide-code/src/slash/help.rs | 6 +-- crates/oxide-code/src/slash/init.rs | 16 +++---- crates/oxide-code/src/slash/matcher.rs | 2 +- crates/oxide-code/src/slash/model.rs | 44 +++++++++--------- crates/oxide-code/src/slash/registry.rs | 62 +++++++++++++++---------- crates/oxide-code/src/slash/status.rs | 2 +- crates/oxide-code/src/tui/app.rs | 6 +-- docs/design/slash/commands.md | 8 ++-- 14 files changed, 114 insertions(+), 106 deletions(-) diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 9b8cacff..9d555418 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -31,6 +31,7 @@ mod status; pub(crate) use context::{SessionInfo, SlashContext}; pub(crate) use matcher::MatchedCommand; pub(crate) use parser::{Parsed, parse_slash, popup_query}; +pub(crate) use registry::SlashKind; /// Filter the built-in registry against a popup query (the buffer /// with the leading `/` stripped). Convenience wrapper around @@ -70,8 +71,8 @@ fn dispatch_with( return None; }; match cmd.execute(&parsed.args, ctx) { - Ok(registry::SlashOutcome::Local) => None, - Ok(registry::SlashOutcome::Action(action)) => Some(action), + Ok(registry::SlashOutcome::Done) => None, + Ok(registry::SlashOutcome::Forward(action)) => Some(action), Err(msg) => { ctx.chat.push_error(&format!("/{}: {msg}", parsed.name)); None @@ -100,23 +101,10 @@ pub(crate) fn classify(parsed: &Parsed) -> SlashKind { fn classify_in(commands: &[&dyn registry::SlashCommand], parsed: &Parsed) -> SlashKind { match registry::lookup_in(commands, &parsed.name) { None => SlashKind::Unknown, - Some(cmd) if cmd.is_read_only(&parsed.args) => SlashKind::ReadOnly, - Some(_) => SlashKind::Mutating, + Some(cmd) => cmd.classify(&parsed.args), } } -/// Whether a slash command can run while the agent is busy. -#[derive(Debug, PartialEq, Eq)] -pub(crate) enum SlashKind { - /// Safe mid-turn — dispatch immediately. - ReadOnly, - /// State-mutating — refuse mid-turn, let the user retry when idle. - Mutating, - /// Not in the registry — dispatch anyway so the user sees the - /// canonical "unknown command" error block with recovery hints. - Unknown, -} - /// Shared test fixture — a fully-populated `SessionInfo` for the /// per-command test modules. #[cfg(test)] @@ -165,7 +153,7 @@ mod tests { args: String::new(), }; let outcome = dispatch(&parsed, &mut SlashContext::new(&mut chat, &info)); - assert!(outcome.is_none(), "/help is Local, not PromptSubmit"); + assert!(outcome.is_none(), "/help is Done, not Forward"); assert!(!chat.last_is_error()); assert_eq!(chat.entry_count(), 1); // Pin: SystemMessageBlock inherits `error_text` default `None` diff --git a/crates/oxide-code/src/slash/clear.rs b/crates/oxide-code/src/slash/clear.rs index 8b6174a7..b5a80462 100644 --- a/crates/oxide-code/src/slash/clear.rs +++ b/crates/oxide-code/src/slash/clear.rs @@ -2,7 +2,7 @@ //! it to the agent loop, which rolls the session. use super::context::SlashContext; -use super::registry::{SlashCommand, SlashOutcome}; +use super::registry::{SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; pub(super) struct ClearCmd; @@ -20,12 +20,12 @@ impl SlashCommand for ClearCmd { "Reset the conversation context" } - fn is_read_only(&self, _args: &str) -> bool { - false + fn classify(&self, _args: &str) -> SlashKind { + SlashKind::Mutating } fn execute(&self, _args: &str, _ctx: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Action(UserAction::Clear)) + Ok(SlashOutcome::Forward(UserAction::Clear)) } } @@ -46,10 +46,10 @@ mod tests { } #[test] - fn is_read_only_is_false() { - // Override to `false` — refuses mid-turn rather than racing - // the live `messages` / session writer. - assert!(!ClearCmd.is_read_only("")); + fn classify_is_mutating() { + // Refuses mid-turn rather than racing the live `messages` / + // session writer. + assert_eq!(ClearCmd.classify(""), SlashKind::Mutating); } // ── ClearCmd::execute ── @@ -64,7 +64,7 @@ mod tests { .execute("", &mut SlashContext::new(&mut chat, &info)) .expect("/clear must succeed"); - assert_eq!(outcome, SlashOutcome::Action(UserAction::Clear)); + assert_eq!(outcome, SlashOutcome::Forward(UserAction::Clear)); assert_eq!(chat.entry_count(), 1, "execute must not touch the chat"); } } diff --git a/crates/oxide-code/src/slash/config.rs b/crates/oxide-code/src/slash/config.rs index 2d47b8f1..69984c6f 100644 --- a/crates/oxide-code/src/slash/config.rs +++ b/crates/oxide-code/src/slash/config.rs @@ -31,7 +31,7 @@ impl SlashCommand for ConfigCmd { let project = file::find_project_config(); ctx.chat .push_system_message(render_config(ctx.info, user.as_deref(), project.as_deref())); - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } diff --git a/crates/oxide-code/src/slash/context.rs b/crates/oxide-code/src/slash/context.rs index dc4dab43..2f538af3 100644 --- a/crates/oxide-code/src/slash/context.rs +++ b/crates/oxide-code/src/slash/context.rs @@ -40,7 +40,7 @@ impl SessionInfo { /// Borrowed view of App-owned state for one /// [`super::registry::SlashCommand::execute`] call. Never stored. -/// State-mutating commands return [`super::registry::SlashOutcome::Action`]; +/// State-mutating commands return [`super::registry::SlashOutcome::Forward`]; /// the dispatcher owns forwarding to the agent loop. pub(crate) struct SlashContext<'a> { pub(crate) chat: &'a mut ChatView, diff --git a/crates/oxide-code/src/slash/diff.rs b/crates/oxide-code/src/slash/diff.rs index ae56b0e8..b61d6363 100644 --- a/crates/oxide-code/src/slash/diff.rs +++ b/crates/oxide-code/src/slash/diff.rs @@ -50,7 +50,7 @@ fn execute_in(cwd: &Path, ctx: &mut SlashContext<'_>) -> Result bool { - args.trim().is_empty() + fn classify(&self, args: &str) -> SlashKind { + if args.trim().is_empty() { + SlashKind::ReadOnly + } else { + SlashKind::Mutating + } } fn usage(&self) -> Option<&'static str> { @@ -36,7 +40,7 @@ impl SlashCommand for EffortCmd { let arg = args.trim(); if arg.is_empty() { ctx.chat.push_system_message(render_effort_list(ctx.info)); - return Ok(SlashOutcome::Local); + return Ok(SlashOutcome::Done); } let pick = parse_effort_arg(arg)?; // Preflight: setting an explicit level on a no-effort model is @@ -49,7 +53,7 @@ impl SlashCommand for EffortCmd { marketing_or_id(&ctx.info.config.model_id), )); } - Ok(SlashOutcome::Action(UserAction::SwitchEffort(pick))) + Ok(SlashOutcome::Forward(UserAction::SwitchEffort(pick))) } } @@ -107,10 +111,10 @@ mod tests { } #[test] - fn is_read_only_splits_on_args() { - assert!(EffortCmd.is_read_only("")); - assert!(EffortCmd.is_read_only(" ")); - assert!(!EffortCmd.is_read_only("xhigh")); + fn classify_splits_on_args() { + assert_eq!(EffortCmd.classify(""), SlashKind::ReadOnly); + assert_eq!(EffortCmd.classify(" "), SlashKind::ReadOnly); + assert_eq!(EffortCmd.classify("xhigh"), SlashKind::Mutating); } // ── EffortCmd::execute ── @@ -136,7 +140,7 @@ mod tests { #[test] fn execute_no_args_pushes_list_with_marker_and_swap_hint() { let (chat, outcome) = run_execute(""); - assert_eq!(outcome, Ok(SlashOutcome::Local)); + assert_eq!(outcome, Ok(SlashOutcome::Done)); let body = chat.last_system_text().expect("system block present"); assert!( body.starts_with("Effort levels for"), @@ -204,7 +208,7 @@ mod tests { let (_, outcome) = run_execute(arg); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchEffort(level))), + Ok(SlashOutcome::Forward(UserAction::SwitchEffort(level))), "`{arg}` should dispatch SwitchEffort({level:?})", ); } diff --git a/crates/oxide-code/src/slash/help.rs b/crates/oxide-code/src/slash/help.rs index db65509b..cdd0dfe0 100644 --- a/crates/oxide-code/src/slash/help.rs +++ b/crates/oxide-code/src/slash/help.rs @@ -23,7 +23,7 @@ impl SlashCommand for HelpCmd { fn execute(&self, _args: &str, ctx: &mut SlashContext<'_>) -> Result { ctx.chat.push_system_message(render_help()); - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } @@ -153,7 +153,7 @@ mod tests { let mut chat = ChatView::new(&Theme::default(), false); let info = crate::slash::test_session_info(); let mut ctx = SlashContext::new(&mut chat, &info); - assert_eq!(Fake::CLEAR.execute("", &mut ctx), Ok(SlashOutcome::Local)); + assert_eq!(Fake::CLEAR.execute("", &mut ctx), Ok(SlashOutcome::Done)); } #[test] @@ -208,7 +208,7 @@ mod tests { self.usage } fn execute(&self, _: &str, _: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } } diff --git a/crates/oxide-code/src/slash/init.rs b/crates/oxide-code/src/slash/init.rs index 8cc554e2..f8fc6c5e 100644 --- a/crates/oxide-code/src/slash/init.rs +++ b/crates/oxide-code/src/slash/init.rs @@ -5,7 +5,7 @@ use indoc::indoc; use super::context::SlashContext; -use super::registry::{SlashCommand, SlashOutcome}; +use super::registry::{SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; pub(super) struct InitCmd; @@ -19,12 +19,12 @@ impl SlashCommand for InitCmd { "Generate or update the project's `AGENTS.md` / `CLAUDE.md`" } - fn is_read_only(&self, _args: &str) -> bool { - false + fn classify(&self, _args: &str) -> SlashKind { + SlashKind::Mutating } fn execute(&self, _args: &str, _ctx: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Action(UserAction::SubmitPrompt( + Ok(SlashOutcome::Forward(UserAction::SubmitPrompt( PROMPT.to_owned(), ))) } @@ -112,10 +112,10 @@ mod tests { } #[test] - fn is_read_only_is_false() { + fn classify_is_mutating() { // Override is load-bearing: a parallel turn would race the // in-flight one over `messages` / the session writer. - assert!(!InitCmd.is_read_only("")); + assert_eq!(InitCmd.classify(""), SlashKind::Mutating); } // ── InitCmd::execute ── @@ -135,7 +135,7 @@ mod tests { assert!( matches!( &outcome, - Ok(SlashOutcome::Action(UserAction::SubmitPrompt(p))) + Ok(SlashOutcome::Forward(UserAction::SubmitPrompt(p))) if p.contains("AGENTS.md") && p.contains("CLAUDE.md") ), "prompt must target both AGENTS.md and CLAUDE.md: {outcome:?}", @@ -150,7 +150,7 @@ mod tests { assert!( matches!( &outcome, - Ok(SlashOutcome::Action(UserAction::SubmitPrompt(p))) + Ok(SlashOutcome::Forward(UserAction::SubmitPrompt(p))) if p.contains("already exists") && p.contains("not overwrite") ), "prompt must instruct the model not to overwrite an existing file: {outcome:?}", diff --git a/crates/oxide-code/src/slash/matcher.rs b/crates/oxide-code/src/slash/matcher.rs index 3a03f06e..c31b50a6 100644 --- a/crates/oxide-code/src/slash/matcher.rs +++ b/crates/oxide-code/src/slash/matcher.rs @@ -133,7 +133,7 @@ mod tests { self.description } fn execute(&self, _: &str, _: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index ee0ab87e..27840271 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -7,7 +7,7 @@ use std::fmt::Write as _; use super::context::{SessionInfo, SlashContext}; use super::format::write_kv_table; -use super::registry::{SlashCommand, SlashOutcome}; +use super::registry::{SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; use crate::model::{MODELS, ResolvedModelId, lookup, marketing_or_id}; @@ -48,10 +48,13 @@ impl SlashCommand for ModelCmd { "List models or switch the active one" } - fn is_read_only(&self, args: &str) -> bool { - // Bare `/model` (list view) is safe mid-turn; the swap form - // races the in-flight `Client` and must wait for idle. - args.trim().is_empty() + fn classify(&self, args: &str) -> SlashKind { + // Bare lists; the swap form races the in-flight `Client`. + if args.trim().is_empty() { + SlashKind::ReadOnly + } else { + SlashKind::Mutating + } } fn usage(&self) -> Option<&'static str> { @@ -62,10 +65,10 @@ impl SlashCommand for ModelCmd { let arg = args.trim(); if arg.is_empty() { ctx.chat.push_system_message(render_model_list(ctx.info)); - return Ok(SlashOutcome::Local); + return Ok(SlashOutcome::Done); } let id = resolve_model_arg(arg)?; - Ok(SlashOutcome::Action(UserAction::SwitchModel(id))) + Ok(SlashOutcome::Forward(UserAction::SwitchModel(id))) } } @@ -216,13 +219,12 @@ mod tests { } #[test] - fn is_read_only_splits_on_args() { - // Bare list form stays read-only; arg-bearing form refuses - // mid-turn. Whitespace-only args route the same as bare. - assert!(ModelCmd.is_read_only("")); - assert!(ModelCmd.is_read_only(" ")); - assert!(!ModelCmd.is_read_only("opus")); - assert!(!ModelCmd.is_read_only("claude-opus-4-7")); + fn classify_splits_on_args() { + // Whitespace-only args route the same as bare. + assert_eq!(ModelCmd.classify(""), SlashKind::ReadOnly); + assert_eq!(ModelCmd.classify(" "), SlashKind::ReadOnly); + assert_eq!(ModelCmd.classify("opus"), SlashKind::Mutating); + assert_eq!(ModelCmd.classify("claude-opus-4-7"), SlashKind::Mutating); } // ── ModelCmd::execute ── @@ -237,7 +239,7 @@ mod tests { #[test] fn execute_no_args_pushes_list_with_legend_and_switch_hint() { let (chat, outcome) = run_execute(""); - assert_eq!(outcome, Ok(SlashOutcome::Local)); + assert_eq!(outcome, Ok(SlashOutcome::Done)); assert_eq!(chat.entry_count(), 1); assert!(!chat.last_is_error()); let body = chat.last_system_text().expect("system block present"); @@ -319,7 +321,7 @@ mod tests { let (_, outcome) = run_execute(alias); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchModel(resolved( + Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( expected )))), "alias `{alias}` should route to `{expected}`", @@ -357,7 +359,7 @@ mod tests { let (_, outcome) = run_execute(id); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchModel(resolved(id)))), + Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved(id)))), "canonical `{id}` must round-trip", ); } @@ -378,7 +380,7 @@ mod tests { let (_, outcome) = run_execute(arg); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchModel(resolved( + Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( expected )))), "`{arg}` should resolve to `{expected}`", @@ -410,7 +412,7 @@ mod tests { let (_, outcome) = run_execute("opus-4"); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchModel(resolved( + Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( "claude-opus-4" )))), ); @@ -438,7 +440,7 @@ mod tests { let (_, outcome) = run_execute("haiku-4-"); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchModel(resolved( + Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( "claude-haiku-4-5" )))), ); @@ -450,7 +452,7 @@ mod tests { let (_, outcome) = run_execute(" haiku-4-5 "); assert_eq!( outcome, - Ok(SlashOutcome::Action(UserAction::SwitchModel(resolved( + Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( "claude-haiku-4-5" )))), ); diff --git a/crates/oxide-code/src/slash/registry.rs b/crates/oxide-code/src/slash/registry.rs index b001b832..11a173e1 100644 --- a/crates/oxide-code/src/slash/registry.rs +++ b/crates/oxide-code/src/slash/registry.rs @@ -16,15 +16,31 @@ use super::model::ModelCmd; use super::status::StatusCmd; use crate::agent::event::UserAction; -/// What [`SlashCommand::execute`] returns. `Local` for client-side -/// work that finishes via `ctx`; `Action` for state-mutating +/// What [`SlashCommand::execute`] returns. `Done` for client-side +/// work that finishes via `ctx`; `Forward` for state-mutating /// commands that hand a [`UserAction`] back for the dispatcher to /// forward to the agent loop. The trait stays the only seam — slash /// impls never reach into `user_tx` themselves. #[derive(Debug, PartialEq, Eq)] pub(crate) enum SlashOutcome { - Local, - Action(UserAction), + Done, + Forward(UserAction), +} + +/// Whether a slash command can run while the agent is busy. Returned +/// by [`SlashCommand::classify`]; the free [`super::classify`] wraps +/// it with `Unknown` when lookup fails. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum SlashKind { + /// Safe mid-turn — dispatch immediately. + ReadOnly, + /// State-mutating — refuse mid-turn, let the user retry when idle. + Mutating, + /// Not in the registry — dispatch anyway so the user sees the + /// canonical "unknown command" error block with recovery hints. + /// Trait implementations never return this; only the free + /// dispatcher does. + Unknown, } /// A locally-dispatched command typed as `/name args`. Each command @@ -45,12 +61,13 @@ pub(crate) trait SlashCommand: Sync { /// One-line description for help and the popup gutter. fn description(&self) -> &'static str; - /// Whether this invocation is safe to run mid-turn. Mutating - /// commands return `false` to refuse instead of racing the live - /// turn. `args` enables per-form classification — `/model` lists - /// when bare and mutates when given an id. - fn is_read_only(&self, _args: &str) -> bool { - true + /// Whether this invocation is safe to run mid-turn. `args` enables + /// per-form classification — `/model` lists when bare and mutates + /// when given an id. Trait implementations return `ReadOnly` or + /// `Mutating`; never `Unknown` (that's the dispatcher's lookup + /// signal). + fn classify(&self, _args: &str) -> SlashKind { + SlashKind::ReadOnly } /// Optional usage hint used by the error message when the command @@ -62,8 +79,8 @@ pub(crate) trait SlashCommand: Sync { /// Runs the command. Mutations land through `ctx`. `Err(msg)` is /// rendered by the dispatcher as a single `ErrorBlock` — commands - /// must not push errors themselves. `Ok(Local)` commands push - /// their own informational block; `Ok(Action(_))` commands hand + /// must not push errors themselves. `Ok(Done)` commands push + /// their own informational block; `Ok(Forward(_))` commands hand /// a `UserAction` back for the dispatcher to forward. fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result; } @@ -169,7 +186,7 @@ mod tests { "collide" } fn execute(&self, _: &str, _: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } let registry: &[&dyn SlashCommand] = &[&HelpCmd, &ColliderCmd]; @@ -177,7 +194,7 @@ mod tests { // Exercise the trait stubs the helper doesn't reach. assert_eq!(ColliderCmd.description(), "collide"); - assert_eq!(run_execute(&ColliderCmd, ""), Ok(SlashOutcome::Local)); + assert_eq!(run_execute(&ColliderCmd, ""), Ok(SlashOutcome::Done)); } #[test] @@ -202,13 +219,13 @@ mod tests { "" } fn execute(&self, _: &str, _: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } assert_eq!(empty_metadata_offenders(&[&EmptyDescCmd]), vec!["no-desc"]); // Exercise the execute stub the offender helper doesn't reach. - assert_eq!(run_execute(&EmptyDescCmd, ""), Ok(SlashOutcome::Local)); + assert_eq!(run_execute(&EmptyDescCmd, ""), Ok(SlashOutcome::Done)); } // ── lookup_in ── @@ -227,22 +244,19 @@ mod tests { "fake" } fn execute(&self, _: &str, _: &mut SlashContext<'_>) -> Result { - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } #[test] fn aliased_cmd_fixture_satisfies_trait_contract() { - // Pin so a fixture drift fails here rather than silently - // misleading the lookup_in tests. The `is_read_only` calls - // also exercise the trait's default body — `AliasedCmd` - // doesn't override it. + // The `classify` calls exercise the trait default body. assert_eq!(AliasedCmd.name(), "primary"); assert_eq!(AliasedCmd.aliases(), &["alt", "shortcut"]); assert_eq!(AliasedCmd.description(), "fake"); - assert!(AliasedCmd.is_read_only("")); - assert!(AliasedCmd.is_read_only("anything")); - assert_eq!(run_execute(&AliasedCmd, ""), Ok(SlashOutcome::Local)); + assert_eq!(AliasedCmd.classify(""), SlashKind::ReadOnly); + assert_eq!(AliasedCmd.classify("anything"), SlashKind::ReadOnly); + assert_eq!(run_execute(&AliasedCmd, ""), Ok(SlashOutcome::Done)); } #[test] diff --git a/crates/oxide-code/src/slash/status.rs b/crates/oxide-code/src/slash/status.rs index 7169ffcb..87daed90 100644 --- a/crates/oxide-code/src/slash/status.rs +++ b/crates/oxide-code/src/slash/status.rs @@ -22,7 +22,7 @@ impl SlashCommand for StatusCmd { fn execute(&self, _args: &str, ctx: &mut SlashContext<'_>) -> Result { ctx.chat.push_system_message(render_status(ctx.info)); - Ok(SlashOutcome::Local) + Ok(SlashOutcome::Done) } } diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 4de689d1..975c6b6a 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -1359,9 +1359,9 @@ mod tests { #[tokio::test] async fn dispatch_bare_slash_during_busy_runs_list_view() { - // The bare form is read-only via `is_read_only(args)` returning - // true for empty args, so it dispatches immediately even - // mid-turn. Regressing the args-aware classification fails here. + // Bare form classifies as ReadOnly so it dispatches mid-turn; + // arg-bearing form refuses. Regressing the args-aware + // classification fails here. for (cmd, header_prefix) in [ ("/model", "Available models"), ("/effort", "Effort levels for"), diff --git a/docs/design/slash/commands.md b/docs/design/slash/commands.md index b9ca83e1..71e71a25 100644 --- a/docs/design/slash/commands.md +++ b/docs/design/slash/commands.md @@ -18,14 +18,14 @@ Eight built-ins: `/clear`, `/config`, `/diff`, `/effort`, `/help`, `/init`, `/mo 2. **Parse at submit, not in `InputArea`.** `App::dispatch_user_action` runs `parse_slash` first, then dispatches locally or forwards. 3. **One synthetic block kind: `SystemMessageBlock`.** Left-bar in `accent`. Errors reuse `ErrorBlock`. 4. **Two-column popup, plain rows.** Name left, description right. Filter ranks name-prefix > alias-prefix > name-substring > alias-substring, alphabetical within each tier. Names accept `:` and `.` for future `/plugin:cmd` namespace. -5. **Mid-session model + effort swap via `&mut Client`.** `/model` returns `Action(UserAction::SwitchModel(id))`, `/effort` returns `Action(UserAction::SwitchEffort(pick))`. Per-request paths re-read config every call so betas / `output_config` pick up the swap. `is_read_only(&self, args: &str)` lets bare list-view forms dispatch mid-turn while arg-bearing forms refuse. +5. **Mid-session model + effort swap via `&mut Client`.** `/model` returns `Forward(UserAction::SwitchModel(id))`, `/effort` returns `Forward(UserAction::SwitchEffort(pick))`. Per-request paths re-read config every call so betas / `output_config` pick up the swap. `classify(&self, args: &str) -> SlashKind` lets bare list-view forms dispatch mid-turn while arg-bearing forms refuse. 6. **Slash commands never write user config files.** Session-only state. Restart returns to config.toml values. Deliberate rejection of Claude Code's silent mega-file writes. 7. **Aliases resolve to canonical but display by surface.** `/clear` is canonical; `/new` and `/reset` are aliases. The popup shows only the alias the user typed. 8. **No `/quit` or `/exit`.** Ctrl+C x2 / Ctrl+D already exit. 9. **`/config` is read-only in v1.** Prints resolved effective config + layered file paths. 10. **Built-in only in v1.** The trait registry leaves room for `~/.config/ox/commands/*.md` discovery later. -11. **Read-only commands fast-path the busy turn.** `is_read_only` defaults `true`; dispatcher runs them client-side even when input is disabled. State-mutating commands override to `false` and refuse mid-turn. -12. **Two command kinds, one trait return: `SlashOutcome { Local, Action(UserAction) }`.** `Local` covers read-only commands. `Action(_)` is state-mutating: handed back to the App, which forwards to the agent loop. +11. **Read-only commands fast-path the busy turn.** `classify` defaults to `SlashKind::ReadOnly`; dispatcher runs them client-side even when input is disabled. State-mutating commands override to `SlashKind::Mutating` and refuse mid-turn. +12. **Two command kinds, one trait return: `SlashOutcome { Done, Forward(UserAction) }`.** `Done` covers read-only commands. `Forward(_)` is state-mutating: handed back to the App, which forwards to the agent loop. ## Per-Command Notes @@ -37,7 +37,7 @@ Key design: send-first ordering in `execute` -- forward `UserAction::Clear` to ` ### /init -Returns `SlashOutcome::Action(UserAction::SubmitPrompt(PROMPT))` with a static body asking the model to author/update AGENTS.md. The App pushes the typed `/init` line as a `UserMessage` block, flips turn-start UI state, then forwards. The expanded body is invisible in the live session; on resume, JSONL records the full body. +Returns `SlashOutcome::Forward(UserAction::SubmitPrompt(PROMPT))` with a static body asking the model to author/update AGENTS.md. The App pushes the typed `/init` line as a `UserMessage` block, flips turn-start UI state, then forwards. The expanded body is invisible in the live session; on resume, JSONL records the full body. ### /model From 659d5355b89720e2ac9e401110981f74ae86f273 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 01:16:37 +0800 Subject: [PATCH 04/19] refactor(agent): collapse SwitchModel + SwitchEffort into SwapConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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()`. --- crates/oxide-code/src/agent.rs | 13 +- crates/oxide-code/src/agent/event.rs | 31 +-- crates/oxide-code/src/client/anthropic.rs | 5 + crates/oxide-code/src/main.rs | 46 +++-- crates/oxide-code/src/model.rs | 12 +- crates/oxide-code/src/slash/context.rs | 10 +- crates/oxide-code/src/slash/effort.rs | 14 +- crates/oxide-code/src/slash/model.rs | 43 ++--- crates/oxide-code/src/tui/app.rs | 223 ++++++++++++++-------- 9 files changed, 247 insertions(+), 150 deletions(-) diff --git a/crates/oxide-code/src/agent.rs b/crates/oxide-code/src/agent.rs index fcb56c9e..29fe8c74 100644 --- a/crates/oxide-code/src/agent.rs +++ b/crates/oxide-code/src/agent.rs @@ -257,8 +257,7 @@ where Some( action @ (UserAction::ConfirmExit | UserAction::Clear - | UserAction::SwitchModel(_) - | UserAction::SwitchEffort(_)), + | UserAction::SwapConfig { .. }), ) => warn!("dropped mid-turn action: {action:?}"), }, output = &mut fut => return Ok(output), @@ -1158,8 +1157,14 @@ mod tests { for action in [ UserAction::ConfirmExit, UserAction::Clear, - UserAction::SwitchModel(ResolvedModelId::new("claude-opus-4-7".to_owned())), - UserAction::SwitchEffort(Effort::High), + UserAction::SwapConfig { + model: Some(ResolvedModelId::new("claude-opus-4-7".to_owned())), + effort: None, + }, + UserAction::SwapConfig { + model: None, + effort: Some(Effort::High), + }, ] { let dir = tempfile::tempdir().unwrap(); let session = test_session(dir.path()); diff --git a/crates/oxide-code/src/agent/event.rs b/crates/oxide-code/src/agent/event.rs index 8ebcbf17..22a4284f 100644 --- a/crates/oxide-code/src/agent/event.rs +++ b/crates/oxide-code/src/agent/event.rs @@ -41,13 +41,14 @@ pub(crate) enum AgentEvent { SessionRolled { id: String, }, - ModelSwitched { + /// Live config after a [`UserAction::SwapConfig`] applied. `effort` + /// is the resolved value (post-clamp); `requested_effort` is the + /// user's pick if they explicitly chose one — used to surface + /// `(clamped from X)` in the confirmation message. + ConfigChanged { model_id: String, effort: Option, - }, - EffortSwitched { - pick: Effort, - effort: Option, + requested_effort: Option, }, Error(String), } @@ -58,8 +59,14 @@ pub(crate) enum AgentEvent { pub(crate) enum UserAction { SubmitPrompt(String), Clear, - SwitchModel(ResolvedModelId), - SwitchEffort(Effort), + /// Symmetric model + effort swap. At least one field must be `Some`; + /// `None` means "leave that axis as-is". Modal pickers and the + /// typed-arg `/model ` / `/effort ` paths both flow + /// through here. + SwapConfig { + model: Option, + effort: Option, + }, Cancel, /// TUI-only; agent loop ignores this. ConfirmExit, @@ -142,8 +149,7 @@ impl StdioSink { AgentEvent::PromptDrained(_) | AgentEvent::SessionTitleUpdated { .. } | AgentEvent::SessionRolled { .. } - | AgentEvent::ModelSwitched { .. } - | AgentEvent::EffortSwitched { .. } => {} + | AgentEvent::ConfigChanged { .. } => {} AgentEvent::TurnComplete => { writeln!(stdout)?; } @@ -304,13 +310,10 @@ mod tests { AgentEvent::SessionRolled { id: "rolled".to_owned(), }, - AgentEvent::ModelSwitched { + AgentEvent::ConfigChanged { model_id: "claude-opus-4-7".to_owned(), effort: Some(Effort::Xhigh), - }, - AgentEvent::EffortSwitched { - pick: Effort::High, - effort: Some(Effort::High), + requested_effort: Some(Effort::Xhigh), }, ] { let (stdout, stderr) = render_one(&test_sink(false), event); diff --git a/crates/oxide-code/src/client/anthropic.rs b/crates/oxide-code/src/client/anthropic.rs index a2464fe7..92e5857c 100644 --- a/crates/oxide-code/src/client/anthropic.rs +++ b/crates/oxide-code/src/client/anthropic.rs @@ -155,6 +155,11 @@ impl Client { &self.config.model } + /// Returns the effort tier the next request will be issued at. + pub(crate) fn effort(&self) -> Option { + self.config.effort + } + /// Client-side session id carried in `x-claude-code-session-id` and /// billing metadata. Caller-supplied or auto-generated UUID v4. #[cfg(test)] diff --git a/crates/oxide-code/src/main.rs b/crates/oxide-code/src/main.rs index 9ad1f396..09b8df86 100644 --- a/crates/oxide-code/src/main.rs +++ b/crates/oxide-code/src/main.rs @@ -25,9 +25,10 @@ use tracing::{debug, warn}; use agent::event::{AgentEvent, AgentSink, StdioSink, UserAction, inert_user_action_channel}; use agent::{TurnAbort, agent_turn}; use client::anthropic::Client; -use config::Config; +use config::{Config, Effort}; use file_tracker::FileTracker; use message::Message; +use model::ResolvedModelId; use session::handle::{ResumedSession, SessionHandle, roll as roll_session}; use session::list_view::render_list; use session::resolver::resolve_session; @@ -388,20 +389,8 @@ async fn agent_loop_task( warn!("session-rolled event dropped: {e}"); } } - UserAction::SwitchModel(id) => { - let effort = client.set_model(id.as_str().to_owned()); - if let Err(e) = sink.send(AgentEvent::ModelSwitched { - model_id: id.into_inner(), - effort, - }) { - warn!("model-switched event dropped: {e}"); - } - } - UserAction::SwitchEffort(pick) => { - let effort = client.set_effort(pick); - if let Err(e) = sink.send(AgentEvent::EffortSwitched { pick, effort }) { - warn!("effort-switched event dropped: {e}"); - } + UserAction::SwapConfig { model, effort } => { + apply_swap_config(&mut client, &sink, model, effort); } UserAction::Quit => break, } @@ -410,6 +399,33 @@ async fn agent_loop_task( Ok(()) } +/// Apply a [`UserAction::SwapConfig`] to the live `Client` and emit +/// [`AgentEvent::ConfigChanged`]. Order is load-bearing: the model +/// swap re-clamps existing effort first, then the explicit effort +/// pick (if any) clamps against the new model's caps. `requested_effort` +/// echoes the user's pick so the UI can surface clamping. +fn apply_swap_config( + client: &mut Client, + sink: &dyn AgentSink, + model: Option, + effort: Option, +) { + if let Some(id) = model { + client.set_model(id.into_inner()); + } + let resolved = match effort { + Some(pick) => client.set_effort(pick), + None => client.effort(), + }; + if let Err(e) = sink.send(AgentEvent::ConfigChanged { + model_id: client.model().to_owned(), + effort: resolved, + requested_effort: effort, + }) { + warn!("config-changed event dropped: {e}"); + } +} + // ── Bare REPL Mode ── async fn bare_repl( diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index 6b9d25b3..ec9e3dca 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -289,7 +289,8 @@ impl Capabilities { /// A model id that has passed through the `/model` resolver. The private /// inner field ensures arbitrary strings cannot flow into -/// [`UserAction::SwitchModel`] without validation. +/// [`UserAction::SwapConfig`](crate::agent::event::UserAction::SwapConfig) +/// without validation. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ResolvedModelId(String); @@ -300,13 +301,14 @@ impl ResolvedModelId { Self(id) } - pub(crate) fn as_str(&self) -> &str { - &self.0 - } - pub(crate) fn into_inner(self) -> String { self.0 } + + #[cfg(test)] + pub(crate) fn as_str(&self) -> &str { + &self.0 + } } // ── Lookup ── diff --git a/crates/oxide-code/src/slash/context.rs b/crates/oxide-code/src/slash/context.rs index 2f538af3..9ddfc7e2 100644 --- a/crates/oxide-code/src/slash/context.rs +++ b/crates/oxide-code/src/slash/context.rs @@ -12,12 +12,10 @@ use crate::tui::components::chat::ChatView; /// Session-level descriptors surfaced by read-only slash commands. /// Built at TUI startup, then rebound mid-session by /// [`AgentEvent::SessionRolled`](crate::agent::event::AgentEvent::SessionRolled) -/// (`/clear`), -/// [`AgentEvent::ModelSwitched`](crate::agent::event::AgentEvent::ModelSwitched) -/// (`/model`), and -/// [`AgentEvent::EffortSwitched`](crate::agent::event::AgentEvent::EffortSwitched) -/// (`/effort`). Embeds [`ConfigSnapshot`] so `/config` reads from a -/// single source. +/// (`/clear`) and +/// [`AgentEvent::ConfigChanged`](crate::agent::event::AgentEvent::ConfigChanged) +/// (`/model`, `/effort`). Embeds [`ConfigSnapshot`] so `/config` reads +/// from a single source. pub(crate) struct SessionInfo { /// Tildified working directory (`$HOME` rewritten as `~`). pub(crate) cwd: String, diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 99ca3642..477445bd 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -53,7 +53,10 @@ impl SlashCommand for EffortCmd { marketing_or_id(&ctx.info.config.model_id), )); } - Ok(SlashOutcome::Forward(UserAction::SwitchEffort(pick))) + Ok(SlashOutcome::Forward(UserAction::SwapConfig { + model: None, + effort: Some(pick), + })) } } @@ -197,7 +200,7 @@ mod tests { } #[test] - fn execute_with_level_dispatches_switch_effort() { + fn execute_with_level_forwards_swap_config_with_effort_only() { for (arg, level) in [ ("low", Effort::Low), ("medium", Effort::Medium), @@ -208,8 +211,11 @@ mod tests { let (_, outcome) = run_execute(arg); assert_eq!( outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchEffort(level))), - "`{arg}` should dispatch SwitchEffort({level:?})", + Ok(SlashOutcome::Forward(UserAction::SwapConfig { + model: None, + effort: Some(level), + })), + "`{arg}` should forward SwapConfig {{ effort: Some({level:?}) }}", ); } } diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 27840271..62bc8ff1 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -68,7 +68,10 @@ impl SlashCommand for ModelCmd { return Ok(SlashOutcome::Done); } let id = resolve_model_arg(arg)?; - Ok(SlashOutcome::Forward(UserAction::SwitchModel(id))) + Ok(SlashOutcome::Forward(UserAction::SwapConfig { + model: Some(id), + effort: None, + })) } } @@ -208,6 +211,13 @@ mod tests { ResolvedModelId::new(id.to_owned()) } + fn swap_model(id: &str) -> SlashOutcome { + SlashOutcome::Forward(UserAction::SwapConfig { + model: Some(resolved(id)), + effort: None, + }) + } + // ── ModelCmd metadata ── #[test] @@ -321,9 +331,7 @@ mod tests { let (_, outcome) = run_execute(alias); assert_eq!( outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( - expected - )))), + Ok(swap_model(expected)), "alias `{alias}` should route to `{expected}`", ); } @@ -359,7 +367,7 @@ mod tests { let (_, outcome) = run_execute(id); assert_eq!( outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved(id)))), + Ok(swap_model(id)), "canonical `{id}` must round-trip", ); } @@ -380,9 +388,7 @@ mod tests { let (_, outcome) = run_execute(arg); assert_eq!( outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( - expected - )))), + Ok(swap_model(expected)), "`{arg}` should resolve to `{expected}`", ); } @@ -410,12 +416,7 @@ mod tests { // the suffix tier must short-circuit before the substring // ambiguity check runs. let (_, outcome) = run_execute("opus-4"); - assert_eq!( - outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( - "claude-opus-4" - )))), - ); + assert_eq!(outcome, Ok(swap_model("claude-opus-4"))); } #[test] @@ -438,24 +439,14 @@ mod tests { #[test] fn execute_unique_substring_resolves_after_suffix_tier_misses() { let (_, outcome) = run_execute("haiku-4-"); - assert_eq!( - outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( - "claude-haiku-4-5" - )))), - ); + assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); } #[test] fn execute_trims_whitespace_around_arg() { // Padded input resolves the same as bare input. let (_, outcome) = run_execute(" haiku-4-5 "); - assert_eq!( - outcome, - Ok(SlashOutcome::Forward(UserAction::SwitchModel(resolved( - "claude-haiku-4-5" - )))), - ); + assert_eq!(outcome, Ok(swap_model("claude-haiku-4-5"))); } // ── resolve_model_arg ── diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 975c6b6a..c2aa0f33 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -292,7 +292,7 @@ impl App { self.should_quit = true; true } - UserAction::Clear | UserAction::SwitchModel(_) | UserAction::SwitchEffort(_) => false, + UserAction::Clear | UserAction::SwapConfig { .. } => false, } } @@ -358,21 +358,29 @@ impl App { self.chat .push_system_message("Conversation cleared. Next message starts fresh."); } - AgentEvent::ModelSwitched { model_id, effort } => { + AgentEvent::ConfigChanged { + model_id, + effort, + requested_effort, + } => { + 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_swap_confirmation(&marketing, &model_id, prev_effort, effort); - self.status_bar.set_model(marketing.into_owned()); + let confirmation = format_config_change( + &marketing, + &model_id, + model_changed, + prev_effort, + effort, + requested_effort, + ); + if model_changed { + self.status_bar.set_model(marketing.into_owned()); + } self.session_info.config.model_id = model_id; self.session_info.config.effort = effort; self.chat.push_system_message(confirmation); } - AgentEvent::EffortSwitched { pick, effort } => { - self.session_info.config.effort = effort; - self.chat - .push_system_message(format_effort_confirmation(pick, effort)); - } AgentEvent::Error(msg) => { self.chat.push_error(&msg); self.finish_turn(); @@ -523,37 +531,44 @@ fn preview_line(prompt: &str, theme: &Theme, body_width: usize) -> Line<'static> ]) } -/// `AgentEvent::ModelSwitched` confirmation. Surfaces silent effort -/// changes (cleared / clamped / model-default) the user wouldn't -/// otherwise see. -fn format_swap_confirmation( +/// `AgentEvent::ConfigChanged` confirmation. Surfaces silent effort +/// shifts — model-driven (cleared / clamped / model-default) and +/// user-driven (clamped pick / lost on no-tier model) — so the user +/// sees the resulting state, not just a generic "OK". +fn format_config_change( marketing: &str, model_id: &str, + model_changed: bool, prev_effort: Option, new_effort: Option, + requested_effort: Option, ) -> String { + if !model_changed { + return match (requested_effort, new_effort) { + (Some(req), Some(eff)) if req == eff => format!("Effort set to {eff}."), + (Some(req), Some(eff)) => format!("Effort set to {eff} (clamped from {req})."), + (Some(req), None) => { + format!("Effort unchanged — model has no effort tier (asked for {req}).") + } + // No-op SwapConfig — slash dispatch keeps this unreachable + // in practice, but a clear fallback beats a panic. + (None, _) => "Config unchanged.".to_owned(), + }; + } let head = format!("Switched to {marketing} ({model_id})"); - match (prev_effort, new_effort) { - (None, None) => format!("{head}."), - (Some(_), None) => format!("{head}. Effort cleared (model has no effort tier)."), - (None, Some(new)) => format!("{head} · effort {new} (model default)."), - (Some(prev), Some(new)) if new < prev => { + 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})."), + (Some(req), _, None) => { + format!("{head}. Effort unchanged — model has no effort tier (asked for {req}).") + } + (None, None, None) => format!("{head}."), + (None, Some(_), None) => format!("{head}. Effort cleared (model has no effort tier)."), + (None, None, Some(eff)) => format!("{head} · effort {eff} (model default)."), + (None, Some(prev), Some(new)) if new < prev => { format!("{head} · effort {new} (clamped from {prev}).") } - (Some(_), Some(new)) => format!("{head} · effort {new}."), - } -} - -/// `AgentEvent::EffortSwitched` confirmation. `pick` is the user's -/// input; `effort` is the resolved value. -fn format_effort_confirmation( - pick: crate::config::Effort, - effort: Option, -) -> String { - match (pick, effort) { - (p, Some(level)) if p == level => format!("Effort set to {level}."), - (p, Some(level)) => format!("Effort set to {level} (clamped from {p})."), - (p, None) => format!("Effort unchanged — model has no effort tier (asked for {p})."), + (None, Some(_), Some(eff)) => format!("{head} · effort {eff}."), } } @@ -1192,10 +1207,16 @@ mod tests { fn dispatch_local_only_actions_return_false_to_prevent_double_send() { for action in [ UserAction::Clear, - UserAction::SwitchModel(crate::model::ResolvedModelId::new( - "claude-opus-4-7".to_owned(), - )), - UserAction::SwitchEffort(crate::config::Effort::High), + UserAction::SwapConfig { + model: Some(crate::model::ResolvedModelId::new( + "claude-opus-4-7".to_owned(), + )), + effort: None, + }, + UserAction::SwapConfig { + model: None, + effort: Some(crate::config::Effort::High), + }, ] { let (mut app, mut rx, _agent_tx) = test_app(None); app.dispatch_user_action(action.clone()); @@ -1427,15 +1448,16 @@ mod tests { } #[test] - fn handle_model_switched_refreshes_status_bar_session_info_and_chat() { + 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`. let (mut app, _rx, _agent_tx) = test_app(None); - app.handle_agent_event(AgentEvent::ModelSwitched { + app.handle_agent_event(AgentEvent::ConfigChanged { model_id: "claude-sonnet-4-6".to_owned(), effort: Some(crate::config::Effort::High), + requested_effort: None, }); assert_eq!(app.session_info.config.model_id, "claude-sonnet-4-6"); @@ -1452,25 +1474,55 @@ mod tests { assert!(app.dirty); } - // ── format_swap_confirmation ── + #[test] + fn handle_config_changed_effort_only_keeps_status_bar_model_label() { + // Effort-only swap leaves the cached model label alone; only + // the snapshot effort and chat confirmation update. + let (mut app, _rx, _agent_tx) = test_app(None); + let original_model = app.status_bar.model().to_owned(); + app.handle_agent_event(AgentEvent::ConfigChanged { + model_id: app.session_info.config.model_id.clone(), + effort: Some(crate::config::Effort::Xhigh), + requested_effort: Some(crate::config::Effort::Xhigh), + }); + assert_eq!( + app.session_info.config.effort, + Some(crate::config::Effort::Xhigh), + ); + assert_eq!(app.status_bar.model(), original_model); + let body = app.chat.last_system_text().expect("confirmation block"); + assert_eq!(body, "Effort set to xhigh."); + assert!(app.dirty); + } + + // ── format_config_change ── #[test] - fn format_swap_confirmation_both_none_omits_effort_clause() { + 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_swap_confirmation("Claude Haiku 4.5", "claude-haiku-4-5", None, None); + let s = format_config_change( + "Claude Haiku 4.5", + "claude-haiku-4-5", + true, + None, + None, + None, + ); assert_eq!(s, "Switched to Claude Haiku 4.5 (claude-haiku-4-5)."); } #[test] - fn format_swap_confirmation_clears_effort_when_new_model_drops_it() { + fn format_config_change_swap_clears_effort_when_new_model_drops_it() { // User had a tier; new model has none. Surface the change so // the user knows their effort just disappeared. - let s = format_swap_confirmation( + let s = format_config_change( "Claude Haiku 4.5", "claude-haiku-4-5", + true, Some(crate::config::Effort::Xhigh), None, + None, ); assert_eq!( s, @@ -1479,15 +1531,17 @@ mod tests { } #[test] - fn format_swap_confirmation_marks_default_when_previous_was_none() { + fn format_config_change_swap_marks_default_when_previous_was_none() { // None → Some means the new model's default kicked in; // distinguishing this from "user's pick survived" prevents // the user from thinking they chose this tier. - let s = format_swap_confirmation( + let s = format_config_change( "Claude Opus 4.7", "claude-opus-4-7", + true, None, Some(crate::config::Effort::Xhigh), + None, ); assert_eq!( s, @@ -1496,13 +1550,15 @@ mod tests { } #[test] - fn format_swap_confirmation_marks_clamp_when_new_effort_below_previous() { + fn format_config_change_swap_marks_clamp_when_new_effort_below_previous() { // The effective tier changed; surface the temporary clamp. - let s = format_swap_confirmation( + let s = format_config_change( "Claude Sonnet 4.6", "claude-sonnet-4-6", + true, Some(crate::config::Effort::Xhigh), Some(crate::config::Effort::High), + None, ); assert_eq!( s, @@ -1511,14 +1567,16 @@ mod tests { } #[test] - fn format_swap_confirmation_quiet_when_effort_unchanged() { + fn format_config_change_swap_quiet_when_effort_unchanged() { // Same tier survives — no clamp / default annotation. Pin // exact format so a stray suffix (`(unchanged)`) would fail. - let s = format_swap_confirmation( + let s = format_config_change( "Claude Opus 4.7", "claude-opus-4-7", + true, Some(crate::config::Effort::High), Some(crate::config::Effort::High), + None, ); assert_eq!( s, @@ -1527,49 +1585,62 @@ mod tests { } #[test] - fn handle_effort_switched_refreshes_session_info_and_pushes_confirmation() { - // /effort updates the snapshot effort and pushes a confirmation - // block. Status bar caches model, not effort, so it doesn't - // change here. - let (mut app, _rx, _agent_tx) = test_app(None); - app.handle_agent_event(AgentEvent::EffortSwitched { - pick: crate::config::Effort::Xhigh, - effort: Some(crate::config::Effort::Xhigh), - }); - assert_eq!( - app.session_info.config.effort, + fn format_config_change_swap_with_explicit_effort_clamped_against_new_caps() { + // 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), + Some(crate::config::Effort::High), Some(crate::config::Effort::Xhigh), ); - let body = app.chat.last_system_text().expect("confirmation block"); - assert_eq!(body, "Effort set to xhigh."); - assert!(app.dirty); + assert_eq!( + s, + "Switched to Claude Sonnet 4.6 (claude-sonnet-4-6) · effort high (clamped from xhigh)." + ); } - // ── format_effort_confirmation ── - #[test] - fn format_effort_confirmation_explicit_pick_matches_resolution() { - let s = format_effort_confirmation( - crate::config::Effort::Xhigh, + 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), + Some(crate::config::Effort::Xhigh), Some(crate::config::Effort::Xhigh), ); assert_eq!(s, "Effort set to xhigh."); } #[test] - fn format_effort_confirmation_clamp_surfaces_what_user_asked_for() { - let s = format_effort_confirmation( - crate::config::Effort::Xhigh, + 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), Some(crate::config::Effort::High), + Some(crate::config::Effort::Xhigh), ); assert_eq!(s, "Effort set to high (clamped from xhigh)."); } #[test] - fn format_effort_confirmation_pick_on_no_tier_model_surfaces_loss() { - // The slash command preflight stops this from happening through - // /effort, but client-driven flows could still emit it. - let s = format_effort_confirmation(crate::config::Effort::High, None); + fn format_config_change_effort_pick_on_no_tier_model_surfaces_loss() { + // 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, + None, + Some(crate::config::Effort::High), + ); assert_eq!( s, "Effort unchanged — model has no effort tier (asked for high)." From 3587360e60a9b6d4ce64f61c7cd01b5eb519c4b4 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 01:26:05 +0800 Subject: [PATCH 05/19] feat(tui): introduce Modal trait, ModalStack, and key routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/oxide-code/src/tui.rs | 1 + crates/oxide-code/src/tui/app.rs | 92 ++++++++- crates/oxide-code/src/tui/modal.rs | 297 +++++++++++++++++++++++++++++ 3 files changed, 388 insertions(+), 2 deletions(-) create mode 100644 crates/oxide-code/src/tui/modal.rs diff --git a/crates/oxide-code/src/tui.rs b/crates/oxide-code/src/tui.rs index 40517e08..9b21c010 100644 --- a/crates/oxide-code/src/tui.rs +++ b/crates/oxide-code/src/tui.rs @@ -10,6 +10,7 @@ pub(crate) mod components; pub(crate) mod event; pub(crate) mod glyphs; pub(crate) mod markdown; +pub(crate) mod modal; pub(crate) mod pending_calls; pub(crate) mod terminal; pub(crate) mod theme; diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index c2aa0f33..542bbc66 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -22,6 +22,7 @@ use super::components::chat::ChatView; use super::components::input::InputArea; use super::components::status::{Status, StatusBar}; use super::glyphs::{NEWLINE_GLYPH, USER_PROMPT_PREFIX, USER_PROMPT_PREFIX_WIDTH}; +use super::modal::{ModalAction, ModalStack}; use super::pending_calls::{PendingCall, PendingCalls, result_header}; use super::terminal::{Tui, draw_sync}; use super::theme::Theme; @@ -58,6 +59,8 @@ pub(crate) struct App { pending_calls: PendingCalls, /// FIFO of prompts submitted mid-turn; drained at turn boundaries. pending_prompts: VecDeque, + /// Active modal overlay(s). Empty when no modal is on screen. + modals: ModalStack, should_quit: bool, /// Whether state has changed since the last render. dirty: bool, @@ -98,6 +101,7 @@ impl App { tools, pending_calls: PendingCalls::new(), pending_prompts: VecDeque::new(), + modals: ModalStack::new(), should_quit: false, dirty: true, } @@ -160,6 +164,17 @@ impl App { // ── Event Handling ── fn handle_crossterm_event(&mut self, event: &Event) { + // First-priority: an active modal owns keyboard focus end-to-end. + // Other components don't see the key until the modal closes. + if let Event::Key(key) = event + && self.modals.is_active() + { + if let Some(action) = self.modals.handle_key(key) { + self.apply_modal_action(action); + } + self.dirty = true; + return; + } match event { Event::Key(KeyEvent { code: KeyCode::Esc, .. @@ -187,6 +202,20 @@ impl App { self.dirty = true; } + /// Dispatcher for actions emitted by a closed modal. + fn apply_modal_action(&mut self, action: ModalAction) { + match action { + ModalAction::None => {} + ModalAction::User(user_action) => self.dispatch_user_action(user_action), + } + } + + #[cfg(test)] + pub(crate) fn push_modal(&mut self, modal: Box) { + self.modals.push(modal); + self.dirty = true; + } + /// Routes Esc: cancel if busy, pop queue if idle+empty, else no-op. fn handle_esc(&mut self) { if !self.input.is_enabled() { @@ -461,10 +490,12 @@ impl App { let input_height = self.input.height(); let preview_height = self.preview_height(); let popup_height = self.input.popup_height(); + let modal_height = self.modals.height(frame.area().width); let chunks = Layout::vertical([ Constraint::Length(2), Constraint::Min(1), Constraint::Length(preview_height), + Constraint::Length(modal_height), Constraint::Length(popup_height), Constraint::Length(input_height), ]) @@ -475,10 +506,13 @@ impl App { if preview_height > 0 { self.render_preview(frame, chunks[2]); } + if modal_height > 0 { + self.modals.render(frame, chunks[3], &self.theme); + } if popup_height > 0 { - self.input.render_popup(frame, chunks[3]); + self.input.render_popup(frame, chunks[4]); } - self.input.render(frame, chunks[4]); + self.input.render(frame, chunks[5]); chunks[1] } @@ -952,6 +986,60 @@ mod tests { ); } + // ── modal gate ── + + #[tokio::test] + async fn modal_gate_intercepts_keys_before_input_sees_them() { + // While a modal is on screen, any key event lands on the modal + // first — the input area must NOT receive them. Pin so a + // regression that fans keys to both surfaces (double-handling + // a single keystroke) fails here. + use crate::tui::modal::ModalAction; + use crate::tui::modal::testing::ScriptedModal; + + let (mut app, mut rx, _agent_tx) = test_app(None); + app.push_modal(Box::new(ScriptedModal::new(ModalAction::User( + UserAction::Cancel, + )))); + + // Type a printable that the input area would otherwise capture. + app.handle_crossterm_event(&key_event(KeyCode::Char('x'), KeyModifiers::NONE)); + assert!( + app.input.lines().iter().all(String::is_empty), + "input must stay empty while modal is active", + ); + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "no UserAction must reach user_tx for a key the modal consumed", + ); + + // Submit the modal — its action flows through the normal + // dispatch path, just like a keyboard-typed UserAction. + app.handle_crossterm_event(&key_event(KeyCode::Char('s'), KeyModifiers::NONE)); + let forwarded = rx.recv().await.expect("modal-submitted action forwarded"); + assert!(matches!(forwarded, UserAction::Cancel)); + } + + #[test] + fn modal_gate_cancel_closes_modal_without_dispatching() { + // `ModalKey::Cancelled` pops the modal but does not dispatch a + // UserAction. The next key must reach the input area. + use crate::tui::modal::ModalAction; + use crate::tui::modal::testing::ScriptedModal; + + let (mut app, mut rx, _agent_tx) = test_app(None); + app.push_modal(Box::new(ScriptedModal::new(ModalAction::None))); + app.handle_crossterm_event(&key_event(KeyCode::Char('c'), KeyModifiers::NONE)); + + assert!( + matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), + "Cancelled must not dispatch any UserAction", + ); + // Modal closed; subsequent keys reach the input. + app.handle_crossterm_event(&key_event(KeyCode::Char('y'), KeyModifiers::NONE)); + assert_eq!(app.input.lines(), vec!["y".to_owned()]); + } + // ── handle_esc ── #[tokio::test] diff --git a/crates/oxide-code/src/tui/modal.rs b/crates/oxide-code/src/tui/modal.rs new file mode 100644 index 00000000..cbe446e8 --- /dev/null +++ b/crates/oxide-code/src/tui/modal.rs @@ -0,0 +1,297 @@ +//! Modal overlay primitive. +//! +//! A modal is a focus-grabbing UI surface that intercepts keyboard +//! events while active and produces a typed result on submission. +//! Lives in the band between the chat scroll and the input area — +//! the same row range the slash autocomplete popup uses, but wider. +//! +//! [`Modal`] is the trait every concrete modal implements. +//! [`ModalStack`] owns the active modal(s) and is held by +//! [`crate::tui::app::App`]. The stack is `Vec`-backed so a future +//! "confirm leave?" overlay can `push` over an existing picker +//! without redesigning ownership. +//! +//! Companion design: `docs/design/slash/modals.md` (added with the +//! first concrete modal). +//! +//! Related research: `docs/research/slash/modals.md`. + +use crossterm::event::KeyEvent; +use ratatui::Frame; +use ratatui::layout::Rect; + +use crate::agent::event::UserAction; +use crate::tui::theme::Theme; + +// ── Modal Trait ── + +/// A focus-grabbing UI overlay. While active, the modal owns keyboard +/// focus end-to-end — App routes keys to it before any other component +/// sees them. A modal renders into a band the manager allocates above +/// the input area. +/// +/// `Send` because App lives on the tokio runtime; never `Sync` — +/// modals own mutable state and are not shared across threads. +pub(crate) trait Modal: Send { + /// Visible height in rows for the given width. The manager + /// allocates exactly this much vertical space; the modal must + /// render fully within it. May vary with `width` (wrap-aware + /// modals) or stay constant (fixed-height pickers). + fn height(&self, width: u16) -> u16; + + /// Render into `area`. Width and height match what `height` last + /// returned for `area.width`. + fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme); + + /// Process a key. The return value drives manager behavior: + /// stay open, dismiss, or submit a typed action. + fn handle_key(&mut self, event: &KeyEvent) -> ModalKey; +} + +// ── Outcomes ── + +/// Outcome of a single key event delivered to a modal. +#[cfg_attr( + not(test), + expect( + dead_code, + reason = "trait-return variants are constructed by Modal impls; production impls land in upcoming concrete-modal commits — only fixture impls exist in test builds today" + ) +)] +pub(crate) enum ModalKey { + /// Stay open. Key was consumed. + Consumed, + /// Close without dispatching anything. + Cancelled, + /// Close and apply this action. + Submitted(ModalAction), +} + +/// What a submitted modal asks the manager to do. +pub(crate) enum ModalAction { + /// Modal already applied its effect locally — no dispatch needed. + /// Reserved for future live-preview modals (e.g. `/theme`) where + /// arrowing through the list already mutated UI state. + None, + /// Forward a [`UserAction`] to the agent loop. Same channel as a + /// keyboard-typed action, so `/model` swaps and friends share one + /// path. + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "constructed by modal impls forwarding UserActions; production impls land in upcoming concrete-modal commits" + ) + )] + User(UserAction), +} + +// ── ModalStack ── + +/// Owns the active modal(s). Single-modal-at-a-time today; the `Vec` +/// is there so a "confirm leave?" overlay inside a picker can `push` +/// without ownership rework. +#[derive(Default)] +pub(crate) struct ModalStack { + stack: Vec>, +} + +impl ModalStack { + pub(crate) fn new() -> Self { + Self::default() + } + + pub(crate) fn is_active(&self) -> bool { + !self.stack.is_empty() + } + + /// Push a modal onto the stack. The new modal receives keys until + /// it submits or cancels; the previous top resumes. + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "production push site is the slash-dispatch OpenModal arm; lands with the first concrete modal" + ) + )] + pub(crate) fn push(&mut self, modal: Box) { + self.stack.push(modal); + } + + /// Total height the stack needs above the input. Today only the + /// top modal renders; the height reflects that. If we ever stack + /// visually, this sums. + pub(crate) fn height(&self, width: u16) -> u16 { + self.stack.last().map_or(0, |m| m.height(width)) + } + + /// Render the visible modal into `area`. No-op if empty. + pub(crate) fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme) { + if let Some(top) = self.stack.last() { + top.render(frame, area, theme); + } + } + + /// Deliver `event` to the top modal. Returns the action to dispatch + /// (or `ModalAction::None` to indicate a silent close), or `None` + /// if the modal stayed open (key consumed) or the stack is empty. + pub(crate) fn handle_key(&mut self, event: &KeyEvent) -> Option { + let outcome = self.stack.last_mut()?.handle_key(event); + match outcome { + ModalKey::Consumed => None, + ModalKey::Cancelled => { + self.stack.pop(); + Some(ModalAction::None) + } + ModalKey::Submitted(action) => { + self.stack.pop(); + Some(action) + } + } + } +} + +// ── Test Fixtures ── + +#[cfg(test)] +pub(crate) mod testing { + //! Synthetic modal for exercising the manager without coupling + //! tests to a concrete picker. + + use super::*; + + /// Modal with scripted key handling — emits a fixed action on a + /// sentinel key. Used to drive `ModalStack` tests and the App-side + /// gate before any concrete modal exists. + pub(crate) struct ScriptedModal { + pub(crate) on_submit_key: char, + pub(crate) on_cancel_key: char, + pub(crate) submit_action: ModalAction, + pub(crate) declared_height: u16, + } + + impl ScriptedModal { + pub(crate) fn new(submit_action: ModalAction) -> Self { + Self { + on_submit_key: 's', + on_cancel_key: 'c', + submit_action, + declared_height: 3, + } + } + } + + impl Modal for ScriptedModal { + fn height(&self, _width: u16) -> u16 { + self.declared_height + } + + fn render(&self, _frame: &mut Frame<'_>, _area: Rect, _theme: &Theme) {} + + fn handle_key(&mut self, event: &KeyEvent) -> ModalKey { + use crossterm::event::KeyCode; + match event.code { + KeyCode::Char(c) if c == self.on_submit_key => { + let mut taken = ModalAction::None; + std::mem::swap(&mut self.submit_action, &mut taken); + ModalKey::Submitted(taken) + } + KeyCode::Char(c) if c == self.on_cancel_key => ModalKey::Cancelled, + _ => ModalKey::Consumed, + } + } + } +} + +#[cfg(test)] +mod tests { + use crossterm::event::KeyCode; + + use super::testing::ScriptedModal; + use super::*; + + fn key(c: char) -> KeyEvent { + KeyEvent::from(KeyCode::Char(c)) + } + + // ── ModalStack ── + + #[test] + fn empty_stack_reports_inactive_and_zero_height() { + let stack = ModalStack::new(); + assert!(!stack.is_active()); + assert_eq!(stack.height(80), 0); + } + + #[test] + fn push_activates_stack_and_height_reflects_top_modal() { + let mut stack = ModalStack::new(); + stack.push(Box::new(ScriptedModal::new(ModalAction::None))); + assert!(stack.is_active()); + assert_eq!(stack.height(80), 3); + } + + #[test] + fn handle_key_consumed_keeps_modal_active() { + let mut stack = ModalStack::new(); + stack.push(Box::new(ScriptedModal::new(ModalAction::None))); + // Any key that's neither submit-sentinel nor cancel-sentinel + // is consumed — stack stays active and returns None. + assert!(stack.handle_key(&key('x')).is_none()); + assert!(stack.is_active()); + } + + #[test] + fn handle_key_cancel_pops_and_yields_modal_action_none() { + let mut stack = ModalStack::new(); + stack.push(Box::new(ScriptedModal::new(ModalAction::None))); + // Cancel must surface a `Some(ModalAction::None)` so App can + // distinguish "modal closed silently" from "key consumed". + let outcome = stack.handle_key(&key('c')); + assert!(matches!(outcome, Some(ModalAction::None))); + assert!(!stack.is_active()); + } + + #[test] + fn handle_key_submit_pops_and_yields_modal_action_user() { + let mut stack = ModalStack::new(); + let action = UserAction::Cancel; + stack.push(Box::new(ScriptedModal::new(ModalAction::User( + action.clone(), + )))); + let outcome = stack.handle_key(&key('s')); + assert!( + matches!(outcome, Some(ModalAction::User(a)) if a == action), + "submit must surface the modal's UserAction unchanged", + ); + assert!(!stack.is_active()); + } + + #[test] + fn handle_key_on_empty_stack_returns_none_without_panicking() { + // No active modal → no key delivery, no stack mutation. + let mut stack = ModalStack::new(); + assert!(stack.handle_key(&key('s')).is_none()); + assert!(!stack.is_active()); + } + + #[test] + fn nested_push_routes_keys_to_top_only() { + // Two-deep stack: keys go to the top until it pops, then the + // inner one resumes. Pin so a regression that fans keys to all + // layers fails here. + let mut stack = ModalStack::new(); + stack.push(Box::new(ScriptedModal::new(ModalAction::User( + UserAction::Clear, + )))); + let mut top = ScriptedModal::new(ModalAction::None); + top.declared_height = 5; + stack.push(Box::new(top)); + + assert_eq!(stack.height(80), 5, "top modal's height wins"); + let outcome = stack.handle_key(&key('s')); + assert!(matches!(outcome, Some(ModalAction::None))); + assert!(stack.is_active(), "inner modal still active"); + assert_eq!(stack.height(80), 3, "inner modal's height resumes"); + } +} From cd75bd6376cbebefd8751126aea0d1dc5150fd4b Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 01:29:56 +0800 Subject: [PATCH 06/19] feat(tui): generic ListPicker primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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. --- crates/oxide-code/src/tui/modal.rs | 2 + .../oxide-code/src/tui/modal/list_picker.rs | 430 ++++++++++++++++++ 2 files changed, 432 insertions(+) create mode 100644 crates/oxide-code/src/tui/modal/list_picker.rs diff --git a/crates/oxide-code/src/tui/modal.rs b/crates/oxide-code/src/tui/modal.rs index cbe446e8..290be24d 100644 --- a/crates/oxide-code/src/tui/modal.rs +++ b/crates/oxide-code/src/tui/modal.rs @@ -16,6 +16,8 @@ //! //! Related research: `docs/research/slash/modals.md`. +pub(crate) mod list_picker; + use crossterm::event::KeyEvent; use ratatui::Frame; use ratatui::layout::Rect; diff --git a/crates/oxide-code/src/tui/modal/list_picker.rs b/crates/oxide-code/src/tui/modal/list_picker.rs new file mode 100644 index 00000000..cea4eb82 --- /dev/null +++ b/crates/oxide-code/src/tui/modal/list_picker.rs @@ -0,0 +1,430 @@ +//! Generic list-picker primitive for [`Modal`](super::Modal) impls. +//! +//! [`ListPicker`] is the state + render surface for "select one of +//! N items" pickers. It is **not** a [`Modal`](super::Modal) itself — +//! concrete pickers own their submit semantics (Enter dispatches what? +//! Esc cancels) so the picker stays free of `Box +//! ModalAction>` callbacks. +//! +//! Each item implements [`PickerItem`]: label, optional description, +//! `is_active` for the active marker, and an optional `key_hint` +//! character (typically `'1'`–`'9'`) for muscle-memory jumps. + +#![cfg_attr( + not(test), + expect( + dead_code, + reason = "primitive is exercised by its own unit tests; concrete picker consumers land in upcoming commits" + ) +)] + +use ratatui::Frame; +use ratatui::layout::Rect; +use ratatui::style::Modifier; +use ratatui::text::{Line, Span}; +use ratatui::widgets::Paragraph; + +use crate::tui::theme::Theme; +use crate::util::text::truncate_to_width; + +// ── PickerItem ── + +/// One row in a [`ListPicker`]. Concrete picker types wrap their data +/// in a small adapter type that implements this trait — see +/// `slash::picker::ModelRow` for the canonical example. +pub(crate) trait PickerItem { + /// Primary text, left-aligned in the row's first column. + fn label(&self) -> &str; + + /// Optional secondary text, right-aligned. Renders dim. + fn description(&self) -> Option<&str> { + None + } + + /// Marks the row currently in effect (e.g. the active model). Drawn + /// with a `✓` marker. Independent of cursor position. + fn is_active(&self) -> bool { + false + } + + /// Single-character mnemonic for jump-to-row (typically `'1'`–`'9'`). + /// `None` ⇒ row is not directly addressable; cursor reaches it via + /// arrows only. + fn key_hint(&self) -> Option { + None + } +} + +// ── ListPicker ── + +/// Marker rendered to the left of the cursor row. +const CURSOR_MARKER: &str = "> "; +/// Width of `CURSOR_MARKER`. ASCII so byte length matches column width. +const CURSOR_MARKER_WIDTH: usize = 2; +/// Marker rendered to the right of the row currently in effect. +const ACTIVE_MARKER: &str = "✓"; + +/// Padding columns between the label and the description. +const COLUMN_GAP: usize = 2; + +/// Body row count not counting items: title + blank + (description? + +/// blank?). Updated by [`ListPicker::header_height`]. +const TITLE_ROW_HEIGHT: u16 = 1; +const TITLE_BLANK_ROW: u16 = 1; + +/// Selectable list with cursor + active marker. Concrete modals embed +/// this and forward navigation keys (`↑`/`↓`/`j`/`k`/`1`–`9`) to it. +pub(crate) struct ListPicker { + title: String, + description: Option, + items: Vec, + selected: usize, +} + +impl ListPicker { + /// New picker with cursor at index 0. Empty `items` is allowed — + /// height shrinks accordingly and `selected()` returns `None`. + pub(crate) fn new(title: impl Into, items: Vec) -> Self { + Self { + title: title.into(), + description: None, + items, + selected: 0, + } + } + + /// Builder: optional description line under the title (rendered dim). + pub(crate) fn with_description(mut self, description: impl Into) -> Self { + self.description = Some(description.into()); + self + } + + /// Position the cursor on the first item matching `predicate`. No-op + /// if no item matches — cursor stays where it was. + pub(crate) fn select_initial(&mut self, predicate: impl Fn(&T) -> bool) { + if let Some(idx) = self.items.iter().position(predicate) { + self.selected = idx; + } + } + + /// Currently-highlighted item. `None` when the list is empty. + pub(crate) fn selected(&self) -> Option<&T> { + self.items.get(self.selected) + } + + /// Cursor row index. Useful when the wrapping modal needs to render + /// secondary state for the highlighted row (e.g. the effort axis + /// for the model picker). + pub(crate) fn selected_index(&self) -> usize { + self.selected + } + + /// Move cursor down one row; wraps from last to first. + pub(crate) fn select_next(&mut self) { + if self.items.is_empty() { + return; + } + self.selected = (self.selected + 1) % self.items.len(); + } + + /// Move cursor up one row; wraps from first to last. + pub(crate) fn select_prev(&mut self) { + if self.items.is_empty() { + return; + } + self.selected = if self.selected == 0 { + self.items.len() - 1 + } else { + self.selected - 1 + }; + } + + /// Jump cursor to the row whose `key_hint` matches `c`. Returns + /// whether a jump happened — `false` for keys that don't address a + /// row, so the wrapping modal can fall through to other handling. + pub(crate) fn select_by_hint(&mut self, c: char) -> bool { + if let Some(idx) = self.items.iter().position(|i| i.key_hint() == Some(c)) { + self.selected = idx; + return true; + } + false + } + + /// Total rows the picker needs at `width`. Title + optional + /// description + items, with one-row gutters between sections. + pub(crate) fn height(&self, _width: u16) -> u16 { + let header = self.header_height(); + let body = u16::try_from(self.items.len()).unwrap_or(u16::MAX); + header.saturating_add(body) + } + + /// Number of rows the title + (description?) header occupies, plus + /// the trailing blank that visually separates header from list. + fn header_height(&self) -> u16 { + let mut h = TITLE_ROW_HEIGHT + TITLE_BLANK_ROW; + if self.description.is_some() { + h += 2; // description row + blank + } + h + } + + pub(crate) fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme) { + let mut lines: Vec> = + Vec::with_capacity(usize::from(self.height(area.width))); + + lines.push(Line::from(Span::styled( + self.title.clone(), + theme.accent().add_modifier(Modifier::BOLD), + ))); + if let Some(desc) = &self.description { + lines.push(Line::from(Span::styled(desc.clone(), theme.dim()))); + } + lines.push(Line::default()); + + let label_width = self + .items + .iter() + .map(|i| i.label().chars().count()) + .max() + .unwrap_or(0); + + for (idx, item) in self.items.iter().enumerate() { + lines.push(self.render_row(item, idx, label_width, area.width, theme)); + } + + frame.render_widget(Paragraph::new(lines).style(theme.surface()), area); + } + + fn render_row( + &self, + item: &T, + idx: usize, + label_width: usize, + area_width: u16, + theme: &Theme, + ) -> Line<'static> { + let is_cursor = idx == self.selected; + let row_style = if is_cursor { + theme.text().add_modifier(Modifier::BOLD) + } else { + theme.dim() + }; + + let mut spans: Vec> = Vec::with_capacity(6); + + // Cursor gutter — `> ` on cursor row, two spaces otherwise. + spans.push(Span::styled( + if is_cursor { + CURSOR_MARKER.to_owned() + } else { + " ".repeat(CURSOR_MARKER_WIDTH) + }, + theme.accent(), + )); + + // Numeric mnemonic (or two-space gutter when none). + if let Some(c) = item.key_hint() { + spans.push(Span::styled(format!("{c}. "), row_style)); + } else { + spans.push(Span::styled(" ".to_owned(), row_style)); + } + + let label = format!("{:width$}", item.label(), width = label_width); + spans.push(Span::styled(label, row_style)); + + // Active marker: `✓` after the label, before the description. + spans.push(Span::styled( + if item.is_active() { + format!(" {ACTIVE_MARKER} ") + } else { + " ".to_owned() + }, + theme.accent(), + )); + + if let Some(desc) = item.description() { + let used: usize = spans.iter().map(|s| s.content.chars().count()).sum(); + let budget = usize::from(area_width).saturating_sub(used + COLUMN_GAP); + let truncated = truncate_to_width(desc, budget); + spans.push(Span::styled(truncated, theme.dim())); + } + + Line::from(spans) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── Test fixture ── + + /// Minimal `PickerItem` impl that exposes every trait method so + /// the tests can pin `description` / `is_active` / `key_hint` + /// behavior without coupling to any concrete picker. + struct FakeItem { + label: &'static str, + description: Option<&'static str>, + active: bool, + hint: Option, + } + + impl FakeItem { + fn new(label: &'static str) -> Self { + Self { + label, + description: None, + active: false, + hint: None, + } + } + } + + impl PickerItem for FakeItem { + fn label(&self) -> &str { + self.label + } + fn description(&self) -> Option<&str> { + self.description + } + fn is_active(&self) -> bool { + self.active + } + fn key_hint(&self) -> Option { + self.hint + } + } + + fn picker(items: Vec) -> ListPicker { + ListPicker::new("Pick one", items) + } + + // ── select_next / select_prev ── + + #[test] + fn select_next_advances_and_wraps_at_end() { + let mut p = picker(vec![FakeItem::new("a"), FakeItem::new("b")]); + p.select_next(); + assert_eq!(p.selected_index(), 1); + p.select_next(); + assert_eq!(p.selected_index(), 0, "wraps past the last row"); + } + + #[test] + fn select_prev_retreats_and_wraps_at_zero() { + let mut p = picker(vec![FakeItem::new("a"), FakeItem::new("b")]); + p.select_prev(); + assert_eq!(p.selected_index(), 1, "wraps past the first row"); + p.select_prev(); + assert_eq!(p.selected_index(), 0); + } + + #[test] + fn select_next_and_prev_on_empty_list_are_noops() { + let mut p = picker(Vec::new()); + p.select_next(); + p.select_prev(); + assert_eq!(p.selected_index(), 0); + assert!(p.selected().is_none()); + } + + // ── select_by_hint ── + + #[test] + fn select_by_hint_jumps_to_matching_item_and_returns_true() { + let mut p = picker(vec![ + FakeItem { + hint: Some('1'), + ..FakeItem::new("a") + }, + FakeItem { + hint: Some('2'), + ..FakeItem::new("b") + }, + FakeItem { + hint: Some('3'), + ..FakeItem::new("c") + }, + ]); + assert!(p.select_by_hint('2')); + assert_eq!(p.selected_index(), 1); + } + + #[test] + fn select_by_hint_unknown_key_leaves_cursor_and_returns_false() { + let mut p = picker(vec![FakeItem { + hint: Some('1'), + ..FakeItem::new("a") + }]); + assert!(!p.select_by_hint('9')); + assert_eq!(p.selected_index(), 0, "cursor stays put on miss"); + } + + // ── select_initial ── + + #[test] + fn select_initial_seeks_first_matching_item() { + let mut p = picker(vec![ + FakeItem::new("a"), + FakeItem::new("b"), + FakeItem::new("c"), + ]); + p.select_initial(|i| i.label() == "b"); + assert_eq!(p.selected_index(), 1); + } + + #[test] + fn select_initial_no_match_leaves_cursor_at_zero() { + let mut p = picker(vec![FakeItem::new("a"), FakeItem::new("b")]); + p.select_initial(|i| i.label() == "missing"); + assert_eq!(p.selected_index(), 0); + } + + // ── height ── + + #[test] + fn height_with_no_description_is_two_header_rows_plus_items() { + // Title (1) + blank (1) + items. + let p = picker(vec![FakeItem::new("a"), FakeItem::new("b")]); + assert_eq!(p.height(80), 4); + } + + #[test] + fn height_with_description_adds_two_more_rows() { + // Title (1) + description (1) + blank (1) + 1 spacer between + // header sections totals 4 header rows + items. + let p = picker(vec![FakeItem::new("a")]).with_description("a small picker"); + assert_eq!(p.height(80), 5); + } + + #[test] + fn height_with_empty_items_is_just_header_rows() { + let p: ListPicker = picker(Vec::new()); + assert_eq!(p.height(80), 2, "empty list still draws title + blank"); + } + + // ── render ── + + #[test] + fn render_runs_without_panicking_at_minimum_width() { + // Smoke test: extreme narrow widths must not panic on the + // truncation arithmetic. Real visual snapshots happen in the + // concrete picker tests where output is meaningful. + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let p = picker(vec![FakeItem { + description: Some("a long description"), + ..FakeItem::new("very-long-label") + }]) + .with_description("title-line"); + let theme = Theme::default(); + let mut terminal = Terminal::new(TestBackend::new(20, 8)).unwrap(); + terminal + .draw(|frame| { + let area = Rect::new(0, 0, 20, p.height(20).min(8)); + p.render(frame, area, &theme); + }) + .expect("render must not panic"); + } +} From 5bfb107775bdd4ca12e4a25e33701b8efa7c4ec2 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 01:45:07 +0800 Subject: [PATCH 07/19] feat(slash): combined /model + /effort picker modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 ` / `/effort ` 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. --- crates/oxide-code/src/slash.rs | 1 + crates/oxide-code/src/slash/context.rs | 29 +- crates/oxide-code/src/slash/effort.rs | 116 +--- crates/oxide-code/src/slash/model.rs | 211 +------- crates/oxide-code/src/slash/picker.rs | 506 ++++++++++++++++++ crates/oxide-code/src/tui/app.rs | 38 +- ...r_empty_query_shows_each_command_once.snap | 5 +- ...narrow_terminal_truncates_description.snap | 5 +- crates/oxide-code/src/tui/modal.rs | 23 +- .../oxide-code/src/tui/modal/list_picker.rs | 15 +- 10 files changed, 617 insertions(+), 332 deletions(-) create mode 100644 crates/oxide-code/src/slash/picker.rs diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index 9d555418..ee77c862 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -25,6 +25,7 @@ mod init; mod matcher; mod model; mod parser; +mod picker; mod registry; mod status; diff --git a/crates/oxide-code/src/slash/context.rs b/crates/oxide-code/src/slash/context.rs index 9ddfc7e2..f42a823a 100644 --- a/crates/oxide-code/src/slash/context.rs +++ b/crates/oxide-code/src/slash/context.rs @@ -8,6 +8,7 @@ use std::borrow::Cow; use crate::config::ConfigSnapshot; use crate::model::marketing_or_id; use crate::tui::components::chat::ChatView; +use crate::tui::modal::Modal; /// Session-level descriptors surfaced by read-only slash commands. /// Built at TUI startup, then rebound mid-session by @@ -39,14 +40,38 @@ impl SessionInfo { /// Borrowed view of App-owned state for one /// [`super::registry::SlashCommand::execute`] call. Never stored. /// State-mutating commands return [`super::registry::SlashOutcome::Forward`]; -/// the dispatcher owns forwarding to the agent loop. +/// the dispatcher owns forwarding to the agent loop. Commands open +/// modals via [`SlashContext::open_modal`] — the dispatcher harvests +/// the slot after `execute` returns and pushes onto the App's modal +/// stack. pub(crate) struct SlashContext<'a> { pub(crate) chat: &'a mut ChatView, pub(crate) info: &'a SessionInfo, + /// Out-parameter for modals opened by `execute`. `None` until set. + /// One slot per dispatch — only one modal can open per command. + modal: Option>, } impl<'a> SlashContext<'a> { pub(crate) fn new(chat: &'a mut ChatView, info: &'a SessionInfo) -> Self { - Self { chat, info } + Self { + chat, + info, + modal: None, + } + } + + /// Open `modal` after this command finishes. Overwriting an + /// existing slot is a programmer error — only one modal per + /// dispatch is meaningful. + pub(crate) fn open_modal(&mut self, modal: Box) { + debug_assert!(self.modal.is_none(), "modal slot set twice in one dispatch"); + self.modal = Some(modal); + } + + /// Take the modal slot, if any. The dispatcher calls this once + /// after `execute` returns. + pub(crate) fn take_modal(&mut self) -> Option> { + self.modal.take() } } diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 477445bd..2fce961c 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -1,13 +1,11 @@ -//! `/effort` — list / swap the active effort tier mid-session. -//! -//! Bare lists the levels supported by the active model with the current -//! marked. `/effort ` swaps to that tier. The agent loop calls +//! `/effort` — open the model+effort picker focused on the effort +//! axis, or `/effort ` to swap directly. The agent loop calls //! [`Client::set_effort`](crate::client::anthropic::Client::set_effort) -//! which clamps against the active model's caps. +//! on the typed-arg path; the picker emits a single +//! [`UserAction::SwapConfig`] which routes through the same client +//! resolver. -use std::fmt::Write as _; - -use super::context::{SessionInfo, SlashContext}; +use super::context::SlashContext; use super::registry::{SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; use crate::config::Effort; @@ -21,10 +19,12 @@ impl SlashCommand for EffortCmd { } fn description(&self) -> &'static str { - "List effort levels or set the active one" + "Open the picker focused on effort, or set a level with `/effort `" } fn classify(&self, args: &str) -> SlashKind { + // Bare opens the picker (UI-local; safe mid-turn). The + // typed-arg form races the in-flight `Client` and must wait. if args.trim().is_empty() { SlashKind::ReadOnly } else { @@ -39,7 +39,10 @@ impl SlashCommand for EffortCmd { fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { - ctx.chat.push_system_message(render_effort_list(ctx.info)); + ctx.open_modal(Box::new(super::picker::ModelEffortPicker::new( + ctx.info, + super::picker::InitialFocus::Effort, + ))); return Ok(SlashOutcome::Done); } let pick = parse_effort_arg(arg)?; @@ -67,35 +70,6 @@ fn parse_effort_arg(arg: &str) -> Result { .map_err(|_| format!("Unknown effort: `{arg}`. Valid: {}.", Effort::VALID_VALUES)) } -/// `* level` list with the active marker, plus a header naming the -/// active model so the user knows which caps the levels reflect. -fn render_effort_list(info: &SessionInfo) -> String { - let marketing = marketing_or_id(&info.config.model_id); - let caps = capabilities_for(&info.config.model_id); - let active = info.config.effort; - - let mut out = format!("Effort levels for {marketing} (* = active)\n\n"); - - if !caps.effort { - _ = writeln!(out, " (no effort tier — {marketing} ignores effort)"); - out.push_str("\nSwitch models first with /model."); - return out; - } - - for level in Effort::ALL - .iter() - .copied() - .filter(|level| caps.accepts_effort(*level)) - { - let marker = if Some(level) == active { '*' } else { ' ' }; - _ = writeln!(out, " {marker} {level}"); - } - - out.push_str("\nSwitch with: /effort \n"); - out.push_str("Only levels supported by the active model are shown."); - out -} - #[cfg(test)] mod tests { use super::*; @@ -141,62 +115,20 @@ mod tests { } #[test] - fn execute_no_args_pushes_list_with_marker_and_swap_hint() { - let (chat, outcome) = run_execute(""); + fn execute_no_args_opens_picker_focused_on_effort() { + // Bare `/effort` opens the same picker as `/model`, but pre-armed + // on the effort axis so a single Enter submits the active level. + // Picker behavior is covered in `slash::picker` tests. + let mut chat = ChatView::new(&Theme::default(), false); + let info = test_session_info(); + let mut ctx = SlashContext::new(&mut chat, &info); + let outcome = EffortCmd.execute("", &mut ctx); assert_eq!(outcome, Ok(SlashOutcome::Done)); - let body = chat.last_system_text().expect("system block present"); - assert!( - body.starts_with("Effort levels for"), - "header leads the output: {body}", - ); - assert!(body.contains("Switch with: /effort "), "{body}"); - assert!( - body.contains("Only levels supported"), - "supported-level hint: {body}", - ); - for level in [Effort::Low, Effort::Medium, Effort::High] { - assert!( - body.contains(&level.to_string()), - "level `{level}` listed: {body}", - ); - } - } - - #[test] - fn execute_no_args_marks_only_the_active_level() { - // `test_session_info` ships effort=High; assert both the row - // count AND that "high" is the marked one so a misrouted - // marker (e.g. always-mark-low regression) fails here. - let (chat, _) = run_execute(""); - let body = chat.last_system_text().unwrap(); - let marked: Vec<&str> = body.lines().filter(|l| l.contains(" * ")).collect(); - assert_eq!(marked.len(), 1, "exactly one marker row: {marked:?}"); - assert!( - marked[0].contains("high"), - "active row marks `high`: {marked:?}", - ); - } - - #[test] - fn execute_no_args_hides_unsupported_levels() { - let (chat, _) = run_execute_with_model("claude-sonnet-4-6", ""); - let body = chat.last_system_text().unwrap(); - for unsupported in ["xhigh", "max"] { - assert!( - !body.contains(unsupported), - "unsupported level `{unsupported}` should not be listed: {body}", - ); - } - } - - #[test] - fn execute_no_args_warns_when_active_model_has_no_effort_tier() { - let (chat, _) = run_execute_with_model("claude-haiku-4-5", ""); - let body = chat.last_system_text().unwrap(); assert!( - body.contains("no effort tier") && body.contains("/model"), - "no-tier warning + recovery hint: {body}", + ctx.take_modal().is_some(), + "bare /effort must populate the modal slot", ); + assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); } #[test] diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index 62bc8ff1..d3e88dcb 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -1,15 +1,14 @@ -//! `/model` — list selectable models or swap the active one. +//! `/model` — open the picker, or swap directly with `/model `. //! -//! Resolution tiers: alias → exact / dated-id → unique suffix → unique substring. -//! `[1m]` is a first-class variant; rejected on models without `context_1m`. +//! Resolution tiers for the typed-arg form: alias → exact / dated-id → +//! unique suffix → unique substring. `[1m]` is a first-class variant; +//! rejected on models without `context_1m`. The bare form opens +//! [`super::picker::ModelEffortPicker`]; the curated roster lives there. -use std::fmt::Write as _; - -use super::context::{SessionInfo, SlashContext}; -use super::format::write_kv_table; +use super::context::SlashContext; use super::registry::{SlashCommand, SlashKind, SlashOutcome}; use crate::agent::event::UserAction; -use crate::model::{MODELS, ResolvedModelId, lookup, marketing_or_id}; +use crate::model::{MODELS, ResolvedModelId, lookup}; // ── Constants ── @@ -17,17 +16,6 @@ use crate::model::{MODELS, ResolvedModelId, lookup, marketing_or_id}; /// context window on models whose capability row has `context_1m`. const TAG_1M: &str = "[1m]"; -/// Curated roster shown by bare `/model`. Manual swap resolves against -/// the full [`MODELS`] table — this constant only governs what the list -/// view displays. -const LISTED_MODELS: &[&str] = &[ - "claude-opus-4-7", - "claude-opus-4-7[1m]", - "claude-sonnet-4-6", - "claude-sonnet-4-6[1m]", - "claude-haiku-4-5", -]; - /// Short aliases resolved before suffix / substring matching. const ALIASES: &[(&str, &str)] = &[ ("opus", "claude-opus-4-7"), @@ -45,11 +33,12 @@ impl SlashCommand for ModelCmd { } fn description(&self) -> &'static str { - "List models or switch the active one" + "Open the model picker or switch directly with `/model `" } fn classify(&self, args: &str) -> SlashKind { - // Bare lists; the swap form races the in-flight `Client`. + // Bare opens the picker (UI-local; safe mid-turn). The + // swap form races the in-flight `Client` and must wait. if args.trim().is_empty() { SlashKind::ReadOnly } else { @@ -64,7 +53,10 @@ impl SlashCommand for ModelCmd { fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { - ctx.chat.push_system_message(render_model_list(ctx.info)); + ctx.open_modal(Box::new(super::picker::ModelEffortPicker::new( + ctx.info, + super::picker::InitialFocus::Model, + ))); return Ok(SlashOutcome::Done); } let id = resolve_model_arg(arg)?; @@ -156,50 +148,6 @@ fn candidates(pred: impl Fn(&str) -> bool) -> Vec<&'static str> { .collect() } -// ── List View ── - -/// Renders the selectable model table with active marker. -fn render_model_list(info: &SessionInfo) -> String { - let active = info.config.model_id.as_str(); - let labels: Vec = LISTED_MODELS - .iter() - .map(|id| label_for(id, *id == active)) - .collect(); - let descriptions: Vec = LISTED_MODELS.iter().map(|id| description_for(id)).collect(); - let rows = labels - .iter() - .zip(&descriptions) - .map(|(label, desc)| (label.as_str(), desc.as_str())); - - let mut out = String::from("Available models (* = active)\n\n"); - write_kv_table(&mut out, rows); - - out.push_str("\nSwitch: /model (aliases: opus, sonnet, haiku)"); - - if !LISTED_MODELS.contains(&active) { - _ = write!( - out, - "\n\nCurrent model: {active} (not in the selectable list).", - ); - } - out -} - -fn label_for(id: &'static str, active: bool) -> String { - let marker = if active { '*' } else { ' ' }; - format!("{marker} {id}") -} - -/// Marketing name, appending `(1M context)` for `[1m]` variants. -fn description_for(id: &'static str) -> String { - let name = marketing_or_id(id); - if id.ends_with("[1m]") { - format!("{name} (1M context)") - } else { - name.into_owned() - } -} - #[cfg(test)] mod tests { use super::*; @@ -247,76 +195,20 @@ mod tests { } #[test] - fn execute_no_args_pushes_list_with_legend_and_switch_hint() { - let (chat, outcome) = run_execute(""); - assert_eq!(outcome, Ok(SlashOutcome::Done)); - assert_eq!(chat.entry_count(), 1); - assert!(!chat.last_is_error()); - let body = chat.last_system_text().expect("system block present"); - assert!( - body.starts_with("Available models (* = active)"), - "header + legend must lead the output: {body}", - ); - assert!(body.contains("Switch: /model "), "switch hint: {body}"); - assert!(body.contains("aliases: opus, sonnet, haiku"), "{body}"); - } - - #[test] - fn execute_no_args_lists_every_selectable_in_declared_order() { - // Pin the row order — a mutation reversing or sorting the - // LISTED_MODELS iteration would survive a per-row contains check. - let (chat, _) = run_execute(""); - let body = chat.last_system_text().unwrap(); - let mut last_idx = 0usize; - for id in LISTED_MODELS { - let idx = body - .find(id) - .unwrap_or_else(|| panic!("missing {id}: {body}")); - assert!( - idx >= last_idx, - "row order broken: {id} at {idx} before previous row at {last_idx}", - ); - last_idx = idx; - } - } - - #[test] - fn execute_no_args_marks_only_the_active_row() { - // Active row is the exact-match against LISTED_MODELS. - // `claude-opus-4-7` (bare) marks only itself, never - // `claude-opus-4-7[1m]` — `[1m]` distinctness matters. - let mut chat = ChatView::new(&Theme::default(), false); - let mut info = test_session_info(); - info.config.model_id = "claude-opus-4-7".to_owned(); - ModelCmd - .execute("", &mut SlashContext::new(&mut chat, &info)) - .unwrap(); - let body = chat.last_system_text().unwrap(); - let marked: Vec<&str> = body.lines().filter(|l| l.contains(" * ")).collect(); - assert_eq!(marked.len(), 1, "exactly one marker row: {marked:?}"); - assert!(marked[0].contains("claude-opus-4-7"), "{marked:?}"); - assert!( - !marked[0].contains("[1m]"), - "bare id must not match the [1m] row: {marked:?}", - ); - } - - #[test] - fn execute_no_args_warns_when_current_model_is_not_selectable() { - // A user with `model = claude-opus-4-1` set via config gets - // an unmarked list plus a footer naming their current model - // so they understand why nothing is starred. + fn execute_no_args_opens_picker_via_ctx_and_pushes_no_chat_block() { + // Bare `/model` opens the combined picker. Nothing should land + // in the chat — the modal is the UI. The picker's own tests in + // `slash::picker` cover its initial state and key handling. let mut chat = ChatView::new(&Theme::default(), false); - let mut info = test_session_info(); - info.config.model_id = "claude-opus-4-1".to_owned(); - ModelCmd - .execute("", &mut SlashContext::new(&mut chat, &info)) - .unwrap(); - let body = chat.last_system_text().unwrap(); + let info = test_session_info(); + let mut ctx = SlashContext::new(&mut chat, &info); + let outcome = ModelCmd.execute("", &mut ctx); + assert_eq!(outcome, Ok(SlashOutcome::Done)); assert!( - body.contains("Current model: claude-opus-4-1 (not in the selectable list)"), - "warning footer expected: {body}", + ctx.take_modal().is_some(), + "bare /model must populate the modal slot", ); + assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); } #[test] @@ -549,57 +441,4 @@ mod tests { assert!(msg.contains("tag, not a model"), "{msg}"); assert!(!msg.contains("matches"), "must not list candidates: {msg}"); } - - // ── render_model_list ── - - fn render(model_id: &str) -> String { - let mut info = test_session_info(); - info.config.model_id = model_id.to_owned(); - render_model_list(&info) - } - - #[test] - fn render_model_list_marker_column_aligns_within_table() { - // Pin the column alignment — `write_kv_table` pads to the - // longest id, and the marker prepended by the active-row - // logic must not break that gutter. Filter to table rows - // (have BOTH the canonical id AND the marketing name). - let body = render("claude-opus-4-7"); - let value_cols: Vec = body - .lines() - .filter(|l| l.contains("claude-") && l.contains("Claude")) - .map(|l| l.find("Claude").expect("description present")) - .collect(); - assert_eq!(value_cols.len(), LISTED_MODELS.len(), "row count: {body}"); - assert!( - value_cols.windows(2).all(|w| w[0] == w[1]), - "columns not aligned: {value_cols:?} — body: {body}", - ); - } - - #[test] - fn render_model_list_appends_1m_context_suffix_to_1m_rows() { - let body = render("claude-opus-4-7"); - // Every [1m] entry must carry the `(1M context)` suffix - // so users can tell variants apart in the list. - for id in LISTED_MODELS.iter().filter(|id| id.ends_with("[1m]")) { - let row = body - .lines() - .find(|l| l.contains(id)) - .unwrap_or_else(|| panic!("row for {id} missing: {body}")); - assert!( - row.contains("(1M context)"), - "1M suffix missing on {id}: {row}", - ); - } - // Non-1M rows must NOT carry the suffix. - let bare_row = body - .lines() - .find(|l| l.contains("claude-opus-4-7 ")) - .expect("bare opus-4-7 row"); - assert!( - !bare_row.contains("(1M context)"), - "leaked suffix: {bare_row}" - ); - } } diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs new file mode 100644 index 00000000..59b0dc5f --- /dev/null +++ b/crates/oxide-code/src/slash/picker.rs @@ -0,0 +1,506 @@ +//! Combined `/model + /effort` picker modal. +//! +//! Bare `/model` and bare `/effort` both open this surface — the only +//! difference is initial focus (model axis vs effort axis). The two +//! axes commit through a single [`UserAction::SwapConfig`] event, so +//! the agent loop sees one atomic config swap. +//! +//! Companion design: `docs/design/slash/modals.md` (added with the +//! design-notes commit at the end of this PR). + +use crossterm::event::{KeyCode, KeyEvent}; +use ratatui::Frame; +use ratatui::layout::Rect; +use ratatui::style::Modifier; +use ratatui::text::{Line, Span}; +use ratatui::widgets::Paragraph; + +use crate::agent::event::UserAction; +use crate::config::Effort; +use crate::model::{ResolvedModelId, capabilities_for, marketing_or_id}; +use crate::tui::modal::list_picker::{ListPicker, PickerItem}; +use crate::tui::modal::{Modal, ModalAction, ModalKey}; +use crate::tui::theme::Theme; + +use super::context::SessionInfo; + +// ── Constants ── + +/// Curated roster shown in the picker. Manual `/model ` resolves +/// against the full `MODELS` table; this slice only governs what the +/// modal lists. Mirrors `slash::model::LISTED_MODELS` — the typed-arg +/// path and the picker share semantics deliberately. +const LISTED_MODELS: &[&str] = &[ + "claude-opus-4-7", + "claude-opus-4-7[1m]", + "claude-sonnet-4-6", + "claude-sonnet-4-6[1m]", + "claude-haiku-4-5", +]; + +// ── PickerItem ── + +/// One model row in the picker. Holds the canonical id plus the active +/// flag; `key_hint` derives from the row's position in the list. +struct ModelRow { + id: &'static str, + is_active: bool, + description: String, + hint: Option, +} + +impl ModelRow { + fn build(active_id: &str) -> Vec { + LISTED_MODELS + .iter() + .enumerate() + .map(|(idx, id)| Self { + id, + is_active: *id == active_id, + description: describe(id), + hint: numeric_hint(idx), + }) + .collect() + } +} + +/// Marketing name + "(1M context)" suffix for `[1m]` rows. +fn describe(id: &str) -> String { + let name = marketing_or_id(id); + if id.ends_with("[1m]") { + format!("{name} (1M context)") + } else { + name.into_owned() + } +} + +/// `'1'`–`'9'` for the first nine rows; `None` after that. Numeric +/// shortcuts are muscle-memory aids, not addressability for every row. +fn numeric_hint(idx: usize) -> Option { + let digit = u32::try_from(idx).ok()?.checked_add(1)?; + if (1..=9).contains(&digit) { + char::from_digit(digit, 10) + } else { + None + } +} + +impl PickerItem for ModelRow { + fn label(&self) -> &str { + self.id + } + fn description(&self) -> Option<&str> { + Some(&self.description) + } + fn is_active(&self) -> bool { + self.is_active + } + fn key_hint(&self) -> Option { + self.hint + } +} + +// ── ModelEffortPicker ── + +/// Initial focus. `/model` opens with the model list active; +/// `/effort` opens with the effort axis pre-armed so Enter submits +/// just-the-effort change. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum InitialFocus { + Model, + Effort, +} + +pub(super) struct ModelEffortPicker { + list: ListPicker, + /// Active model captured at open — used to detect whether the user + /// changed the model axis on submit. + active_model: String, + /// Active effort captured at open. Same purpose for the effort axis. + active_effort: Option, + /// User's current effort pick. Tracks Left/Right navigation. + /// `None` when the highlighted model has no effort tier. + effort: Option, + /// Whether the user touched the effort axis after open. When false + /// and the model didn't change either, Enter is a no-op. + effort_dirty: bool, +} + +impl ModelEffortPicker { + pub(super) fn new(info: &SessionInfo, focus: InitialFocus) -> Self { + let active_model = info.config.model_id.clone(); + let active_effort = info.config.effort; + let rows = ModelRow::build(&active_model); + + let mut list = ListPicker::new( + "Select model", + rows, + ) + .with_description( + "Switch the active model. Applies to this session only — restart returns to your config.", + ); + list.select_initial(|row| row.is_active); + + // Bare `/effort` arms the effort axis so Left/Right is the + // first navigation; bare `/model` leaves both axes pristine. + let effort_dirty = matches!(focus, InitialFocus::Effort); + let effort = effort_for_highlighted(&list, active_effort); + + Self { + list, + active_model, + active_effort, + effort, + effort_dirty, + } + } + + /// Re-resolve the effort axis after the cursor moves. The axis + /// reflects the highlighted model's caps — `None` when that model + /// has no effort tier. + fn refresh_effort_for_cursor(&mut self) { + self.effort = effort_for_highlighted(&self.list, self.effort_or_active()); + } + + fn effort_or_active(&self) -> Option { + self.effort.or(self.active_effort) + } + + fn cycle_effort(&mut self, direction: Direction) { + let Some(row) = self.list.selected() else { + return; + }; + let caps = capabilities_for(row.id); + if !caps.effort { + return; + } + let supported: Vec = Effort::ALL + .iter() + .copied() + .filter(|level| caps.accepts_effort(*level)) + .collect(); + if supported.is_empty() { + return; + } + let current_idx = self + .effort + .and_then(|e| supported.iter().position(|s| *s == e)) + .unwrap_or(0); + let next_idx = match direction { + Direction::Forward => (current_idx + 1) % supported.len(), + Direction::Backward => { + if current_idx == 0 { + supported.len() - 1 + } else { + current_idx - 1 + } + } + }; + self.effort = Some(supported[next_idx]); + self.effort_dirty = true; + } + + fn submit(&self) -> ModalKey { + let model = self + .list + .selected() + .map_or_else(|| self.active_model.clone(), |row| row.id.to_owned()); + let model_changed = model != self.active_model; + let effort_changed = self.effort_dirty && self.effort != self.active_effort; + + if !model_changed && !effort_changed { + return ModalKey::Cancelled; + } + ModalKey::Submitted(ModalAction::User(UserAction::SwapConfig { + model: model_changed.then(|| ResolvedModelId::new(model)), + effort: effort_changed.then_some(self.effort).flatten(), + })) + } + + fn render_effort_row(&self, theme: &Theme) -> Option> { + let row = self.list.selected()?; + let caps = capabilities_for(row.id); + if !caps.effort { + return None; + } + let level = self.effort_or_active()?; + let was_default = self.active_effort.is_none() && !self.effort_dirty; + let suffix = if was_default { " (default)" } else { "" }; + Some(Line::from(vec![ + Span::styled("● ", theme.accent()), + Span::styled( + format!("{level} effort{suffix}"), + theme.text().add_modifier(Modifier::BOLD), + ), + Span::styled(" ← → to adjust", theme.dim()), + ])) + } +} + +impl Modal for ModelEffortPicker { + fn height(&self, width: u16) -> u16 { + // List height + (effort row + spacer)? + footer + spacer + let list_height = self.list.height(width); + let mut h = list_height + 1; // spacer before footer + if self.list.selected().is_some_and(has_effort_tier) { + h += 2; // spacer + effort row + } + h + 1 // footer line + } + + fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme) { + let list_h = self.list.height(area.width); + let list_area = Rect { + height: list_h.min(area.height), + ..area + }; + self.list.render(frame, list_area, theme); + + let mut cursor_y = area.y.saturating_add(list_h); + let mut remaining = area.height.saturating_sub(list_h); + + if let Some(line) = self.render_effort_row(theme) { + cursor_y = cursor_y.saturating_add(1); + remaining = remaining.saturating_sub(1); + let row_area = Rect { + x: area.x, + y: cursor_y, + width: area.width, + height: 1.min(remaining), + }; + frame.render_widget(Paragraph::new(line).style(theme.surface()), row_area); + cursor_y = cursor_y.saturating_add(1); + remaining = remaining.saturating_sub(1); + } + + if remaining >= 2 { + let footer_area = Rect { + x: area.x, + y: cursor_y.saturating_add(1), + width: area.width, + height: 1, + }; + let footer = Line::from(Span::styled( + "Enter to confirm · Esc to cancel", + theme.dim(), + )); + frame.render_widget(Paragraph::new(footer).style(theme.surface()), footer_area); + } + } + + fn handle_key(&mut self, event: &KeyEvent) -> ModalKey { + match event.code { + KeyCode::Esc => ModalKey::Cancelled, + KeyCode::Enter => self.submit(), + KeyCode::Up | KeyCode::Char('k') => { + self.list.select_prev(); + self.refresh_effort_for_cursor(); + ModalKey::Consumed + } + KeyCode::Down | KeyCode::Char('j') => { + self.list.select_next(); + self.refresh_effort_for_cursor(); + ModalKey::Consumed + } + KeyCode::Right | KeyCode::Char('l') => { + self.cycle_effort(Direction::Forward); + ModalKey::Consumed + } + KeyCode::Left | KeyCode::Char('h') => { + self.cycle_effort(Direction::Backward); + ModalKey::Consumed + } + KeyCode::Char(c @ '1'..='9') => { + if self.list.select_by_hint(c) { + self.refresh_effort_for_cursor(); + } + ModalKey::Consumed + } + _ => ModalKey::Consumed, + } + } +} + +// ── Helpers ── + +#[derive(Debug, Clone, Copy)] +enum Direction { + Forward, + Backward, +} + +fn has_effort_tier(row: &ModelRow) -> bool { + capabilities_for(row.id).effort +} + +fn effort_for_highlighted(list: &ListPicker, fallback: Option) -> Option { + let row = list.selected()?; + let caps = capabilities_for(row.id); + if !caps.effort { + return None; + } + Some(caps.resolve_effort(fallback).unwrap_or(Effort::High)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::slash::test_session_info; + + fn picker(focus: InitialFocus) -> ModelEffortPicker { + ModelEffortPicker::new(&test_session_info(), focus) + } + + fn key(code: KeyCode) -> KeyEvent { + KeyEvent::from(code) + } + + // ── Initial state ── + + #[test] + fn new_positions_cursor_on_active_model() { + // `test_session_info` ships claude-opus-4-7 active. + let p = picker(InitialFocus::Model); + let row = p.list.selected().expect("active row"); + assert_eq!(row.id, "claude-opus-4-7"); + assert!(row.is_active); + } + + #[test] + fn new_with_effort_focus_marks_effort_dirty() { + // Bare /effort opens the picker with the effort axis already + // armed, so a single Enter submits the current pick. + let p = picker(InitialFocus::Effort); + assert!( + p.effort_dirty, + "InitialFocus::Effort must arm the effort axis", + ); + } + + #[test] + fn new_with_model_focus_keeps_effort_clean() { + let p = picker(InitialFocus::Model); + assert!(!p.effort_dirty); + } + + // ── handle_key navigation ── + + #[test] + fn down_arrow_advances_cursor_and_refreshes_effort() { + let mut p = picker(InitialFocus::Model); + let before = p.list.selected_index(); + p.handle_key(&key(KeyCode::Down)); + assert_eq!(p.list.selected_index(), before + 1); + } + + #[test] + fn numeric_jump_routes_cursor_to_matching_row() { + // `5` jumps to the fifth listed model — Haiku 4.5. + let mut p = picker(InitialFocus::Model); + p.handle_key(&key(KeyCode::Char('5'))); + let row = p.list.selected().expect("selected row"); + assert_eq!(row.id, "claude-haiku-4-5"); + } + + #[test] + fn right_arrow_cycles_effort_within_supported_levels() { + // Opus 4.7 supports the full ladder. Pressing Right walks + // through it; Left walks back. + let mut p = picker(InitialFocus::Model); + let initial = p.effort; + p.handle_key(&key(KeyCode::Right)); + assert_ne!(p.effort, initial, "Right must change effort"); + assert!(p.effort_dirty, "navigation marks effort dirty"); + } + + #[test] + fn right_arrow_on_no_tier_model_is_a_noop() { + // Haiku 4.5 has no effort tier — Left/Right must not mutate + // the (None) effort state. + let mut p = picker(InitialFocus::Model); + p.handle_key(&key(KeyCode::Char('5'))); // jump to Haiku + assert!(p.effort.is_none()); + assert!(!p.effort_dirty); + p.handle_key(&key(KeyCode::Right)); + assert!(p.effort.is_none(), "no-tier model must stay None"); + assert!( + !p.effort_dirty, + "navigation that no-ops must not mark effort dirty", + ); + } + + // ── submit ── + + #[test] + fn enter_with_no_changes_returns_cancelled() { + // Open + Enter without touching anything is the same shape as + // Esc — nothing to dispatch. + let mut p = picker(InitialFocus::Model); + let outcome = p.handle_key(&key(KeyCode::Enter)); + assert!(matches!(outcome, ModalKey::Cancelled)); + } + + #[test] + fn enter_after_model_change_emits_swap_with_model_only() { + let mut p = picker(InitialFocus::Model); + p.handle_key(&key(KeyCode::Down)); + let outcome = p.handle_key(&key(KeyCode::Enter)); + match outcome { + ModalKey::Submitted(ModalAction::User(UserAction::SwapConfig { model, effort })) => { + assert!(model.is_some(), "model must be set"); + assert!( + effort.is_none(), + "effort must NOT be set when only the model axis moved", + ); + } + other => panic!("expected Submitted(SwapConfig {{ model: Some, .. }}), got {other:?}"), + } + } + + #[test] + fn enter_after_effort_change_emits_swap_with_effort_only() { + // Opus 4.7 active + xhigh; cycle effort once, model unchanged. + let mut p = picker(InitialFocus::Effort); + p.handle_key(&key(KeyCode::Right)); + let outcome = p.handle_key(&key(KeyCode::Enter)); + match outcome { + ModalKey::Submitted(ModalAction::User(UserAction::SwapConfig { model, effort })) => { + assert!( + model.is_none(), + "model must NOT be set when only the effort axis moved", + ); + assert!(effort.is_some(), "effort must be set"); + } + other => panic!("expected Submitted with effort-only SwapConfig, got {other:?}"), + } + } + + #[test] + fn esc_returns_cancelled_regardless_of_axis_state() { + let mut p = picker(InitialFocus::Model); + p.handle_key(&key(KeyCode::Down)); + p.handle_key(&key(KeyCode::Right)); + let outcome = p.handle_key(&key(KeyCode::Esc)); + assert!(matches!(outcome, ModalKey::Cancelled)); + } + + // ── Render smoke ── + + #[test] + fn render_runs_at_typical_widths_without_panicking() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let p = picker(InitialFocus::Model); + let theme = Theme::default(); + for width in [40_u16, 80, 120] { + let h = p.height(width).min(20); + let mut terminal = Terminal::new(TestBackend::new(width, h)).unwrap(); + terminal + .draw(|frame| { + p.render(frame, Rect::new(0, 0, width, h), &theme); + }) + .expect("render must not panic"); + } + } +} diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index 542bbc66..b5fdeae5 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -263,10 +263,14 @@ impl App { if self.input.is_enabled() { if let Some(parsed) = slash::parse_slash(text) { self.chat.push_user_message(text.clone()); - let synthesized = { + let (synthesized, modal) = { let mut ctx = SlashContext::new(&mut self.chat, &self.session_info); - slash::dispatch(&parsed, &mut ctx) + let action = slash::dispatch(&parsed, &mut ctx); + (action, ctx.take_modal()) }; + if let Some(modal) = modal { + self.modals.push(modal); + } if let Some(action) = synthesized { if matches!(action, UserAction::SubmitPrompt(_)) { self.input.set_enabled(false); @@ -285,8 +289,15 @@ impl App { self.chat.push_user_message(text.clone()); match slash::classify(&parsed) { SlashKind::ReadOnly | SlashKind::Unknown => { - let mut ctx = SlashContext::new(&mut self.chat, &self.session_info); - _ = slash::dispatch(&parsed, &mut ctx); + let modal = { + let mut ctx = + SlashContext::new(&mut self.chat, &self.session_info); + _ = slash::dispatch(&parsed, &mut ctx); + ctx.take_modal() + }; + if let Some(modal) = modal { + self.modals.push(modal); + } } SlashKind::Mutating => { self.chat.push_system_message(format!( @@ -1467,21 +1478,20 @@ mod tests { } #[tokio::test] - async fn dispatch_bare_slash_during_busy_runs_list_view() { - // Bare form classifies as ReadOnly so it dispatches mid-turn; - // arg-bearing form refuses. Regressing the args-aware - // classification fails here. - for (cmd, header_prefix) in [ - ("/model", "Available models"), - ("/effort", "Effort levels for"), - ] { + async fn dispatch_bare_slash_during_busy_opens_modal_picker() { + // Bare form classifies as ReadOnly so it dispatches mid-turn — + // and now opens the picker modal instead of printing a list. + // The arg-bearing form continues to refuse mid-turn. + for cmd in ["/model", "/effort"] { let (mut app, _rx, _agent_tx) = test_app(None); app.dispatch_user_action(UserAction::SubmitPrompt("active".to_owned())); app.dispatch_user_action(UserAction::SubmitPrompt(cmd.to_owned())); - let body = app.chat.last_system_text().expect("system block from list"); - assert!(body.starts_with(header_prefix), "{cmd}: {body}"); + assert!( + app.modals.is_active(), + "{cmd}: bare form must push a modal mid-turn", + ); } } diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap index 54160293..946a553a 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap @@ -1,13 +1,12 @@ --- source: crates/oxide-code/src/tui/components/input/popup.rs -assertion_line: 327 expression: "render_to_backend(&popup, 60)" --- "/clear Reset the conversation context " "/config Show the resolved configuration and the layered ..." "/diff Show uncommitted working-tree changes (`git diff..." -"/effort List effort levels or set the active one " +"/effort Open the picker focused on effort, or set a leve..." "/help List the available slash commands and their usag..." "/init Generate or update the project's `AGENTS.md` / `..." -"/model List models or switch the active one " +"/model Open the model picker or switch directly with `/..." "/status Show session info: model, effort, version, worki..." diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap index 6a62542f..2f8590e2 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap @@ -1,13 +1,12 @@ --- source: crates/oxide-code/src/tui/components/input/popup.rs -assertion_line: 373 expression: "render_to_backend(&popup, 30)" --- "/clear Reset the conversa..." "/config Show the resolved ..." "/diff Show uncommitted w..." -"/effort List effort levels..." +"/effort Open the picker fo..." "/help List the available..." "/init Generate or update..." -"/model List models or swi..." +"/model Open the model pic..." "/status Show session info:..." diff --git a/crates/oxide-code/src/tui/modal.rs b/crates/oxide-code/src/tui/modal.rs index 290be24d..e9644e10 100644 --- a/crates/oxide-code/src/tui/modal.rs +++ b/crates/oxide-code/src/tui/modal.rs @@ -53,13 +53,7 @@ pub(crate) trait Modal: Send { // ── Outcomes ── /// Outcome of a single key event delivered to a modal. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "trait-return variants are constructed by Modal impls; production impls land in upcoming concrete-modal commits — only fixture impls exist in test builds today" - ) -)] +#[derive(Debug)] pub(crate) enum ModalKey { /// Stay open. Key was consumed. Consumed, @@ -70,6 +64,7 @@ pub(crate) enum ModalKey { } /// What a submitted modal asks the manager to do. +#[derive(Debug)] pub(crate) enum ModalAction { /// Modal already applied its effect locally — no dispatch needed. /// Reserved for future live-preview modals (e.g. `/theme`) where @@ -78,13 +73,6 @@ pub(crate) enum ModalAction { /// Forward a [`UserAction`] to the agent loop. Same channel as a /// keyboard-typed action, so `/model` swaps and friends share one /// path. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "constructed by modal impls forwarding UserActions; production impls land in upcoming concrete-modal commits" - ) - )] User(UserAction), } @@ -109,13 +97,6 @@ impl ModalStack { /// Push a modal onto the stack. The new modal receives keys until /// it submits or cancels; the previous top resumes. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "production push site is the slash-dispatch OpenModal arm; lands with the first concrete modal" - ) - )] pub(crate) fn push(&mut self, modal: Box) { self.stack.push(modal); } diff --git a/crates/oxide-code/src/tui/modal/list_picker.rs b/crates/oxide-code/src/tui/modal/list_picker.rs index cea4eb82..77beb611 100644 --- a/crates/oxide-code/src/tui/modal/list_picker.rs +++ b/crates/oxide-code/src/tui/modal/list_picker.rs @@ -10,14 +10,6 @@ //! `is_active` for the active marker, and an optional `key_hint` //! character (typically `'1'`–`'9'`) for muscle-memory jumps. -#![cfg_attr( - not(test), - expect( - dead_code, - reason = "primitive is exercised by its own unit tests; concrete picker consumers land in upcoming commits" - ) -)] - use ratatui::Frame; use ratatui::layout::Rect; use ratatui::style::Modifier; @@ -112,9 +104,10 @@ impl ListPicker { self.items.get(self.selected) } - /// Cursor row index. Useful when the wrapping modal needs to render - /// secondary state for the highlighted row (e.g. the effort axis - /// for the model picker). + /// Cursor row index. Test-only — production picker logic reaches + /// the highlighted row through [`Self::selected`] which already + /// returns the value most callers need. + #[cfg(test)] pub(crate) fn selected_index(&self) -> usize { self.selected } From 0a4929cc2bb0bc11b9d0cd4e5e5c6c90f2a57e83 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 01:48:53 +0800 Subject: [PATCH 08/19] feat(slash): /status overview modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/oxide-code/src/slash.rs | 1 + crates/oxide-code/src/slash/status.rs | 123 ++--------- crates/oxide-code/src/slash/status_modal.rs | 221 ++++++++++++++++++++ 3 files changed, 238 insertions(+), 107 deletions(-) create mode 100644 crates/oxide-code/src/slash/status_modal.rs diff --git a/crates/oxide-code/src/slash.rs b/crates/oxide-code/src/slash.rs index ee77c862..bde38a4c 100644 --- a/crates/oxide-code/src/slash.rs +++ b/crates/oxide-code/src/slash.rs @@ -28,6 +28,7 @@ mod parser; mod picker; mod registry; mod status; +mod status_modal; pub(crate) use context::{SessionInfo, SlashContext}; pub(crate) use matcher::MatchedCommand; diff --git a/crates/oxide-code/src/slash/status.rs b/crates/oxide-code/src/slash/status.rs index 87daed90..4205a351 100644 --- a/crates/oxide-code/src/slash/status.rs +++ b/crates/oxide-code/src/slash/status.rs @@ -1,13 +1,10 @@ -//! `/status` — print live session descriptors. +//! `/status` — open the read-only [`StatusModal`](super::status_modal::StatusModal) +//! overview of the live session. //! -//! Reads from the [`SessionInfo`] snapshot the dispatcher hands in; -//! never mutates state. Output mirrors `/help`'s shape: a heading, a -//! blank line, then key-value rows aligned to a shared gutter. +//! No args, no chat output — the modal is the surface. Esc closes. -use super::context::{SessionInfo, SlashContext}; -use super::format::write_kv_section; +use super::context::SlashContext; use super::registry::{SlashCommand, SlashOutcome}; -use crate::config::display_effort; pub(super) struct StatusCmd; @@ -21,36 +18,17 @@ impl SlashCommand for StatusCmd { } fn execute(&self, _args: &str, ctx: &mut SlashContext<'_>) -> Result { - ctx.chat.push_system_message(render_status(ctx.info)); + ctx.open_modal(Box::new(super::status_modal::StatusModal::new(ctx.info))); Ok(SlashOutcome::Done) } } -/// `key value` table. Keys live here (not derived from struct field -/// names) so the rendered labels stay stable when the struct grows. -/// Model identity (Model, Model ID, Effort) leads so a routing-debug -/// glance shows the trio that drives every per-request decision. -fn render_status(info: &SessionInfo) -> String { - let model = info.marketing_name(); - let effort = display_effort(info.config.effort); - let rows: [(&str, &str); 7] = [ - ("Model", &model), - ("Model ID", &info.config.model_id), - ("Effort", &effort), - ("Working Directory", &info.cwd), - ("Version", info.version), - ("Auth", info.config.auth_label), - ("Session ID", &info.session_id), - ]; - let mut out = String::new(); - write_kv_section(&mut out, "Session Status", rows); - out -} - #[cfg(test)] mod tests { use super::*; use crate::slash::test_session_info; + use crate::tui::components::chat::ChatView; + use crate::tui::theme::Theme; // ── StatusCmd metadata ── @@ -64,88 +42,19 @@ mod tests { // ── StatusCmd::execute ── #[test] - fn status_execute_pushes_a_non_error_block() { - // Trait-method end-to-end success path. - use crate::tui::components::chat::ChatView; - use crate::tui::theme::Theme; - + fn execute_opens_the_status_modal_via_ctx_and_pushes_no_chat_block() { + // The modal is the UI; chat must stay clean. Behavioural tests + // for the modal's content + key handling live in + // `slash::status_modal`. let mut chat = ChatView::new(&Theme::default(), false); let info = test_session_info(); let mut ctx = SlashContext::new(&mut chat, &info); - StatusCmd.execute("", &mut ctx).unwrap(); - assert_eq!(chat.entry_count(), 1); - assert!(!chat.last_is_error()); - } - - // ── render_status ── - - #[test] - fn render_status_starts_with_heading_and_blank_line() { - let body = render_status(&test_session_info()); - let mut lines = body.lines(); - assert_eq!(lines.next(), Some("Session Status")); - assert_eq!(lines.next(), Some(""), "heading separated by blank line"); - } - - #[test] - fn render_status_emits_one_row_per_session_field() { - // Pin every field reaches the user, plus the row count — a - // dropped row mustn't slip past the per-value checks. - let info = test_session_info(); - let model = info.marketing_name(); - let effort = info.config.effort.expect("fixture sets effort").to_string(); - let body = render_status(&info); - for needle in [ - model.as_ref(), - info.config.model_id.as_str(), - effort.as_str(), - info.cwd.as_str(), - info.version, - info.config.auth_label, - info.session_id.as_str(), - ] { - assert!(body.contains(needle), "missing `{needle}`: {body}"); - } - let row_count = body.lines().skip(2).filter(|l| !l.is_empty()).count(); - assert_eq!(row_count, 7, "expected 7 rendered rows: {body}"); - } - - #[test] - fn render_status_aligns_values_to_a_shared_gutter() { - // Pin the absolute column, not just "all rows agree" — a - // uniformly broken renderer would pass the latter. - let info = test_session_info(); - let model = info.marketing_name(); - let effort = info.config.effort.expect("fixture sets effort").to_string(); - let values = [ - model.as_ref(), - info.config.model_id.as_str(), - effort.as_str(), - info.cwd.as_str(), - info.version, - info.config.auth_label, - info.session_id.as_str(), - ]; - let body = render_status(&info); - let cols: Vec = body - .lines() - .skip(2) - .filter(|l| !l.is_empty()) - .zip(values) - .map(|(line, value)| line.find(value).expect("value missing from row")) - .collect(); - // Longest label is "Working Directory" (17) ⇒ prefix(2) + 17 + gap(2) = 21. + let outcome = StatusCmd.execute("", &mut ctx); + assert_eq!(outcome, Ok(SlashOutcome::Done)); assert!( - cols.iter().all(|c| *c == 21), - "value columns not aligned at col 21: {cols:?}", + ctx.take_modal().is_some(), + "/status must populate the modal slot", ); - } - - #[test] - fn render_status_renders_no_effort_tier_when_none() { - let mut info = test_session_info(); - info.config.effort = None; - let body = render_status(&info); - assert!(body.contains("(no effort tier)"), "{body}"); + assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); } } diff --git a/crates/oxide-code/src/slash/status_modal.rs b/crates/oxide-code/src/slash/status_modal.rs new file mode 100644 index 00000000..c4322996 --- /dev/null +++ b/crates/oxide-code/src/slash/status_modal.rs @@ -0,0 +1,221 @@ +//! `/status` overview modal — read-only single panel of session +//! descriptors. Esc closes; no other keys do anything. +//! +//! Replaces the text-output `/status` block; the underlying data is +//! the same [`SessionInfo`] / [`ConfigSnapshot`] snapshot that backed +//! the old chat row. + +use crossterm::event::{KeyCode, KeyEvent}; +use ratatui::Frame; +use ratatui::layout::Rect; +use ratatui::style::Modifier; +use ratatui::text::{Line, Span}; +use ratatui::widgets::Paragraph; + +use crate::config::display_effort; +use crate::tui::modal::{Modal, ModalKey}; +use crate::tui::theme::Theme; + +use super::context::SessionInfo; + +// ── Constants ── + +/// Title rendered in bold above the rows. +const TITLE: &str = "Status"; + +/// Footer line rendered dim under the rows. +const FOOTER: &str = "Esc to close"; + +/// Padding columns between the label and value columns. Mirrors +/// [`ListPicker`](crate::tui::modal::list_picker::ListPicker). +const COLUMN_GAP: usize = 2; + +// ── StatusModal ── + +pub(super) struct StatusModal { + rows: Vec<(&'static str, String)>, +} + +impl StatusModal { + pub(super) fn new(info: &SessionInfo) -> Self { + let model = format!("{} ({})", info.marketing_name(), info.config.model_id); + let show_thinking = if info.config.show_thinking { + "on".to_owned() + } else { + "off".to_owned() + }; + // Pin order: identity (Model / Effort) first, then session + // descriptors, then runtime knobs. Mirrors the routing-debug + // glance from the old text /status. + let rows = vec![ + ("Model", model), + ("Effort", display_effort(info.config.effort)), + ("Working Directory", info.cwd.clone()), + ("Session", info.session_id.clone()), + ("Auth", info.config.auth_label.to_owned()), + ("Version", info.version.to_owned()), + ("Context Cache", info.config.prompt_cache_ttl.to_string()), + ("Show Thinking", show_thinking), + ]; + Self { rows } + } +} + +impl Modal for StatusModal { + fn height(&self, _width: u16) -> u16 { + // Title + blank + rows + blank + footer. + let body = u16::try_from(self.rows.len()).unwrap_or(u16::MAX); + body.saturating_add(4) + } + + fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme) { + let label_width = self + .rows + .iter() + .map(|(k, _)| k.chars().count()) + .max() + .unwrap_or(0); + + let mut lines: Vec> = + Vec::with_capacity(usize::from(self.height(area.width))); + lines.push(Line::from(Span::styled( + TITLE.to_owned(), + theme.accent().add_modifier(Modifier::BOLD), + ))); + lines.push(Line::default()); + + for (label, value) in &self.rows { + lines.push(Line::from(vec![ + Span::styled(format!("{label: ModalKey { + match event.code { + // Esc and Enter both dismiss — there's nothing to confirm. + KeyCode::Esc | KeyCode::Enter => ModalKey::Cancelled, + _ => ModalKey::Consumed, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::slash::test_session_info; + + fn modal() -> StatusModal { + StatusModal::new(&test_session_info()) + } + + fn key(code: KeyCode) -> KeyEvent { + KeyEvent::from(code) + } + + // ── Construction ── + + #[test] + fn new_produces_one_row_per_session_descriptor() { + // Pin row count so a dropped descriptor (e.g. forgotten Auth) + // surfaces here. Independent of label / value contents. + let m = modal(); + assert_eq!(m.rows.len(), 8); + } + + #[test] + fn new_collects_every_session_field_value() { + // Pin the actual data flow — every value the user expects to + // see in the modal must come straight from the snapshot. + let info = test_session_info(); + let m = StatusModal::new(&info); + let body: String = m + .rows + .iter() + .map(|(_, v)| v.as_str()) + .collect::>() + .join("|"); + for needle in [ + info.config.model_id.as_str(), + info.cwd.as_str(), + info.config.auth_label, + info.version, + info.session_id.as_str(), + ] { + assert!(body.contains(needle), "missing `{needle}`: {body}"); + } + } + + #[test] + fn new_renders_thinking_off_when_snapshot_says_false() { + let info = test_session_info(); + let m = StatusModal::new(&info); + let thinking_row = m + .rows + .iter() + .find(|(k, _)| *k == "Show Thinking") + .expect("show-thinking row"); + // Default fixture has `show_thinking: false`. + assert_eq!(thinking_row.1, "off"); + } + + // ── handle_key ── + + #[test] + fn esc_closes_modal_silently() { + // Status is read-only; closing it must NOT dispatch an action. + let mut m = modal(); + let outcome = m.handle_key(&key(KeyCode::Esc)); + assert!(matches!(outcome, ModalKey::Cancelled)); + } + + #[test] + fn enter_also_closes_modal_silently() { + // Enter on a no-op view feels natural; pin both code paths so + // a regression that special-cases Esc only fails here. + let mut m = modal(); + let outcome = m.handle_key(&key(KeyCode::Enter)); + assert!(matches!(outcome, ModalKey::Cancelled)); + } + + #[test] + fn other_keys_are_consumed_and_modal_stays_open() { + // Arrow / printable / Tab — all must stay locally consumed + // so the input area never sees them while status is on screen. + let mut m = modal(); + for code in [KeyCode::Up, KeyCode::Down, KeyCode::Char('x'), KeyCode::Tab] { + let outcome = m.handle_key(&key(code)); + assert!( + matches!(outcome, ModalKey::Consumed), + "{code:?} must be consumed", + ); + } + } + + // ── Render smoke ── + + #[test] + fn render_runs_at_typical_widths_without_panicking() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let m = modal(); + let theme = Theme::default(); + for width in [40_u16, 80, 120] { + let h = m.height(width).max(1); + let mut terminal = Terminal::new(TestBackend::new(width, h)).unwrap(); + terminal + .draw(|frame| { + m.render(frame, Rect::new(0, 0, width, h), &theme); + }) + .expect("render must not panic"); + } + } +} From f79df8c777353d99e8ea3d73b59394751d4a71c5 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 01:53:59 +0800 Subject: [PATCH 09/19] docs(design): modal-ui design notes and crate-tree update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CLAUDE.md | 13 +++-- docs/design/README.md | 7 +-- docs/design/slash/commands.md | 22 ++++++--- docs/design/slash/modals.md | 92 +++++++++++++++++++++++++++++++++++ docs/research/slash/modals.md | 2 + 5 files changed, 123 insertions(+), 13 deletions(-) create mode 100644 docs/design/slash/modals.md diff --git a/CLAUDE.md b/CLAUDE.md index 77213ad7..078043f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,17 +71,19 @@ ox # Start an interactive session ├── slash/ │ ├── clear.rs # /clear (new, reset) — forwards UserAction::Clear, resets ChatView, drops the AI title │ ├── config.rs # /config — read-only resolved config + layered file paths -│ ├── context.rs # SlashContext (borrowed ChatView + SessionInfo) handed to each command's execute +│ ├── context.rs # SlashContext (borrowed ChatView + SessionInfo + modal slot) handed to each command's execute │ ├── diff.rs # /diff — `git diff HEAD` + untracked, 64 KB cap on UTF-8 boundary -│ ├── effort.rs # /effort — list / swap explicit effort tier +│ ├── effort.rs # /effort — bare opens picker focused on effort; `/effort ` swaps directly │ ├── format.rs # Shared kv-section / kv-table renderer │ ├── help.rs # /help — registry-driven command listing │ ├── init.rs # /init — synthesize an AGENTS.md / CLAUDE.md author-or-update prompt │ ├── matcher.rs # filter_and_rank: tier-ranked popup matches -│ ├── model.rs # /model — list / swap; resolver alias → lookup → unique suffix → unique substring; `[1m]` first-class +│ ├── model.rs # /model — bare opens picker; `/model ` resolves alias → lookup → unique suffix → unique substring; `[1m]` first-class │ ├── parser.rs # parse_slash + popup_query — detect `/cmd args`; allows `:` and `.` +│ ├── picker.rs # ModelEffortPicker — combined model + effort modal; emits a single SwapConfig │ ├── registry.rs # SlashCommand trait + SlashOutcome + BUILT_INS slice + alias-aware lookup -│ └── status.rs # /status — model, effort, cwd, version, auth, session id +│ ├── status.rs # /status — opens StatusModal +│ └── status_modal.rs # StatusModal — read-only kv-overview of the live session (Esc / Enter close) ├── tool.rs # Tool trait, registry, definitions ├── tool/ │ ├── bash.rs # Shell command execution with timeout @@ -131,6 +133,9 @@ ox # Start an interactive session │ ├── markdown/ │ │ ├── highlight.rs # Syntax highlighting (syntect lazy-loaded SyntaxSet / ThemeSet) │ │ └── render.rs # pulldown-cmark event walker, inline / block / list / table rendering +│ ├── modal.rs # Modal trait, ModalKey, ModalAction, ModalStack — focus-grabbing UI overlays +│ ├── modal/ +│ │ └── list_picker.rs # Generic ListPicker — cursor + render primitive used by concrete pickers │ ├── pending_calls.rs # Tool-call correlation state for streaming and transcript resume │ ├── terminal.rs # Terminal init / restore, synchronized output, panic hook │ └── wrap.rs # Word-wrap with continuation indent for styled lines diff --git a/docs/design/README.md b/docs/design/README.md index a8341411..846d6584 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -13,9 +13,10 @@ Organized by topic. Each subdirectory mirrors the corresponding directory in [`d ## Slash Commands -| Document | Description | -| ----------------------------------------------------- | ---------------------------------------------------- | -| [Commands](slash/commands.md) | Registry, dispatch, popup, per-command notes | +| Document | Description | +| ----------------------------------------------------- | -------------------------------------------------------------------- | +| [Commands](slash/commands.md) | Registry, dispatch, popup, per-command notes | +| [Modal UI](slash/modals.md) | `Modal` trait, `ModalStack`, `ListPicker`, model + effort + status | ## Tools diff --git a/docs/design/slash/commands.md b/docs/design/slash/commands.md index 71e71a25..18ca9a1f 100644 --- a/docs/design/slash/commands.md +++ b/docs/design/slash/commands.md @@ -18,7 +18,7 @@ Eight built-ins: `/clear`, `/config`, `/diff`, `/effort`, `/help`, `/init`, `/mo 2. **Parse at submit, not in `InputArea`.** `App::dispatch_user_action` runs `parse_slash` first, then dispatches locally or forwards. 3. **One synthetic block kind: `SystemMessageBlock`.** Left-bar in `accent`. Errors reuse `ErrorBlock`. 4. **Two-column popup, plain rows.** Name left, description right. Filter ranks name-prefix > alias-prefix > name-substring > alias-substring, alphabetical within each tier. Names accept `:` and `.` for future `/plugin:cmd` namespace. -5. **Mid-session model + effort swap via `&mut Client`.** `/model` returns `Forward(UserAction::SwitchModel(id))`, `/effort` returns `Forward(UserAction::SwitchEffort(pick))`. Per-request paths re-read config every call so betas / `output_config` pick up the swap. `classify(&self, args: &str) -> SlashKind` lets bare list-view forms dispatch mid-turn while arg-bearing forms refuse. +5. **Mid-session model + effort swap via `&mut Client`.** Both `/model ` and `/effort ` return `Forward(UserAction::SwapConfig { model, effort })` — the same payload the picker modal emits, so the typed-arg path and the modal share one resolver. Per-request paths re-read config every call so betas / `output_config` pick up the swap. `classify(&self, args: &str) -> SlashKind` lets bare forms (which open the picker, [`docs/design/slash/modals.md`](modals.md)) dispatch mid-turn while arg-bearing forms refuse. 6. **Slash commands never write user config files.** Session-only state. Restart returns to config.toml values. Deliberate rejection of Claude Code's silent mega-file writes. 7. **Aliases resolve to canonical but display by surface.** `/clear` is canonical; `/new` and `/reset` are aliases. The popup shows only the alias the user typed. 8. **No `/quit` or `/exit`.** Ctrl+C x2 / Ctrl+D already exit. @@ -26,6 +26,7 @@ Eight built-ins: `/clear`, `/config`, `/diff`, `/effort`, `/help`, `/init`, `/mo 10. **Built-in only in v1.** The trait registry leaves room for `~/.config/ox/commands/*.md` discovery later. 11. **Read-only commands fast-path the busy turn.** `classify` defaults to `SlashKind::ReadOnly`; dispatcher runs them client-side even when input is disabled. State-mutating commands override to `SlashKind::Mutating` and refuse mid-turn. 12. **Two command kinds, one trait return: `SlashOutcome { Done, Forward(UserAction) }`.** `Done` covers read-only commands. `Forward(_)` is state-mutating: handed back to the App, which forwards to the agent loop. +13. **Modals open via a `SlashContext` side-channel, not a third `SlashOutcome` variant.** Commands set `ctx.open_modal(Box::new(...))` and return `Done`; the dispatcher harvests the slot after `execute` and pushes onto the App's modal stack. Keeps `SlashOutcome` derive-clean. See [modals.md](modals.md) for the full modal design. ## Per-Command Notes @@ -41,19 +42,28 @@ Returns `SlashOutcome::Forward(UserAction::SubmitPrompt(PROMPT))` with a static ### /model -Bare `/model` lists the curated `LISTED_MODELS` set marking the active row. `/model ` resolves via: alias -> exact/dated id -> unique suffix -> unique substring. `[1m]` is an opt-in tag (strip -> resolve -> reattach). Effort coupling stays explicit and lossy -- re-clamps current effort against the new model. +Bare `/model` opens the combined picker modal ([modals.md](modals.md)) focused on the model axis. `/model ` resolves via: alias -> exact/dated id -> unique suffix -> unique substring. `[1m]` is an opt-in tag (strip -> resolve -> reattach). Effort coupling stays explicit and lossy -- re-clamps current effort against the new model. Both forms emit the same `UserAction::SwapConfig`. ### /effort -Mirrors `/model` shape. Accepts concrete tiers (`low`, `medium`, `high`, `xhigh`, `max`). No `auto` state. +Bare `/effort` opens the combined picker modal pre-armed on the effort axis. `/effort ` accepts concrete tiers (`low`, `medium`, `high`, `xhigh`, `max`). No `auto` state. + +### /status + +Bare `/status` opens a read-only overview modal ([modals.md](modals.md)). No args, no chat output -- the modal is the surface. Esc / Enter close. ## Sources - `crates/oxide-code/src/slash.rs` -- dispatch, `SlashOutcome`. - `crates/oxide-code/src/slash/registry.rs` -- `SlashCommand` trait, `BUILT_INS`, `SlashOutcome`. +- `crates/oxide-code/src/slash/context.rs` -- `SlashContext`, `open_modal` / `take_modal`. - `crates/oxide-code/src/slash/clear.rs` -- `ClearCmd`, send-first ordering. - `crates/oxide-code/src/slash/init.rs` -- `InitCmd`, `PROMPT`. -- `crates/oxide-code/src/slash/model.rs` -- `ModelCmd`, `LISTED_MODELS`, resolver. +- `crates/oxide-code/src/slash/model.rs` -- `ModelCmd`, resolver. - `crates/oxide-code/src/slash/effort.rs` -- `EffortCmd`, level parser. -- `crates/oxide-code/src/tui/app.rs` -- `dispatch_user_action`, `apply_action_locally`. -- `crates/oxide-code/src/agent.rs` -- `agent_loop_task` Clear arm, model/effort switch handling. +- `crates/oxide-code/src/slash/picker.rs` -- combined model + effort picker modal. +- `crates/oxide-code/src/slash/status_modal.rs` -- `/status` overview modal. +- `crates/oxide-code/src/tui/app.rs` -- `dispatch_user_action`, `apply_action_locally`, modal gate. +- `crates/oxide-code/src/tui/modal.rs` -- `Modal` trait, `ModalStack`, key routing. +- `crates/oxide-code/src/tui/modal/list_picker.rs` -- generic `ListPicker` primitive. +- `crates/oxide-code/src/agent.rs` -- `agent_loop_task` Clear and SwapConfig arms. diff --git a/docs/design/slash/modals.md b/docs/design/slash/modals.md new file mode 100644 index 00000000..8144afad --- /dev/null +++ b/docs/design/slash/modals.md @@ -0,0 +1,92 @@ +# Modal UI + +Focus-grabbing UI overlays that any slash command can open. Lives in the band between the chat scroll and the input area, the same row range the slash autocomplete popup uses, but wider. + +Companion: [commands.md](commands.md) — slash-command surface that opens modals. Research: [`docs/research/slash/modals.md`](../../research/slash/modals.md). + +## Goals + +A modal is a self-contained UI that takes keyboard focus, owns its render, emits a typed result, and dismisses. Chat blocks are persistent transcript artifacts; modals are ephemeral overlays. They are not the same primitive. + +Three things drove the abstraction: + +1. **Live preview.** A future `/theme` command needs to swap palettes as the user arrows through choices and snap back on Esc. +2. **Multi-step interaction.** The combined `/model + /effort` picker. Plan approval. MCP server pick-then-configure. +3. **Agent-driven prompts.** When a tool wants permission, the agent must surface a prompt and route the user's decision back. Today there is no UI seam for this; the modal trait is shaped to support it later. + +## Trait Shape + +```rust +pub(crate) trait Modal: Send { + fn height(&self, width: u16) -> u16; + fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme); + fn handle_key(&mut self, event: &KeyEvent) -> ModalKey; +} + +pub(crate) enum ModalKey { + Consumed, // stay open; key handled + Cancelled, // close; no dispatch + Submitted(ModalAction), // close; apply action +} + +pub(crate) enum ModalAction { + None, // modal already applied effects locally + User(UserAction), // forward through the agent channel +} +``` + +`Send` because App lives on tokio; never `Sync` — modals own mutable state and are not shared across threads. + +## Implementation + +[`crates/oxide-code/src/tui/modal.rs`](../../../crates/oxide-code/src/tui/modal.rs) defines the trait, key outcome, and `ModalStack` manager. [`crates/oxide-code/src/tui/modal/list_picker.rs`](../../../crates/oxide-code/src/tui/modal/list_picker.rs) is the generic primitive that concrete pickers embed. + +Two concrete modals ship today: + +- [`crates/oxide-code/src/slash/picker.rs`](../../../crates/oxide-code/src/slash/picker.rs) — combined `/model + /effort` picker. +- [`crates/oxide-code/src/slash/status_modal.rs`](../../../crates/oxide-code/src/slash/status_modal.rs) — `/status` overview. + +App owns `ModalStack` and runs the key gate first in `handle_crossterm_event`: an active modal sees every key before any other component, then `apply_modal_action` dispatches the result through the same path as a keyboard `UserAction`. + +## Design Decisions + +1. **Modal trait, not enum.** Each concrete modal is its own type implementing `Modal`. Adding one is a new file plus a constructor — no central match arm. +2. **Stack-based ownership (`Vec>`).** Single-element today; the `Vec` is there so a future "confirm leave?" overlay inside a picker can `push` without a redesign. +3. **Typed result delivery, no callbacks.** Modal emits `ModalKey::Submitted(ModalAction)`; manager dispatches. Boxed `FnOnce` callbacks were rejected for lifetime / `Send` complexity and because they hide the dispatch graph. +4. **Modals receive a `&SessionInfo` snapshot at open.** Reactive subscriptions are deferred — when a value changes mid-modal (rare), the modal closes and reopens with fresh state. +5. **Layout band sized by `ModalStack::height(width)`.** Zero rows when empty (existing layout unchanged); displaces the chat upward when active, just like the slash popup. +6. **Modals open via `SlashContext::open_modal`, not a new `SlashOutcome` variant.** Keeps `SlashOutcome` derive-clean (`Debug + PartialEq + Eq`). The dispatcher harvests the slot after `execute` and pushes onto the App's stack — same shape as `chat: &mut ChatView` for write-effects. +7. **Bare `/model` and bare `/effort` open the same modal**, with different initial focus (Codex pattern). Typed-arg `/model ` and `/effort ` keep their direct-switch behaviour for scripting and power users. +8. **Generic [`ListPicker`] is _not_ a `Modal`.** It is a state + render primitive that concrete pickers embed and forward keys to. This separates "list selection state" from "what does Enter dispatch", which avoids the boxed-callback pattern while staying broadly reusable (`/model + /effort` today; future `/theme`, future approval prompts). +9. **`/status` on Esc and Enter both dismiss.** Read-only overview — there's nothing to "confirm". The dual binding makes the close gesture muscle-memory-friendly across users coming from different conventions. + +## Per-Modal Notes + +### Combined `/model + /effort` picker + +[`slash::picker::ModelEffortPicker`](../../../crates/oxide-code/src/slash/picker.rs) wraps `ListPicker` and tracks the effort axis separately. `ModelRow` carries `is_active` (drawn with `✓`) and a `key_hint` (`'1'`–`'9'`) for jump-to-row. The picker drops Claude Code's "Default (recommended)" upsell label, drops the `/fast` line, and shows the canonical model id alongside the marketing name (so `/model ` direct-switch and the picker share a mental model). + +Effort row hides automatically on no-tier models (Haiku 4.5). Left/Right cycles only through tiers the highlighted model supports; the resolved effort is recomputed on every cursor move so the displayed value never claims a tier the next request would silently clamp. + +Submit emits a single `UserAction::SwapConfig { model, effort }`. Both axes are `Option`s — only the axes the user actually changed are populated, so the agent loop can re-clamp atomically without redundant work. When neither axis changed, Enter is treated as `Cancelled`. + +### `/status` overview + +[`slash::status_modal::StatusModal`](../../../crates/oxide-code/src/slash/status_modal.rs) renders a kv-table of session descriptors (model, effort, working directory, session id, auth, version, context-cache TTL, show-thinking). Single panel — no tabs today. When `/usage` and `/stats` exist, the modal grows a tab bar (modal-internal change, no infrastructure work). + +## Out of Scope / Deferred + +- **Persistent modals in chat scroll.** Modals are ephemeral. Persistent "what models exist" output goes through `/help` or future `/model --list` text. +- **Mouse interaction.** Defer to a polish PR if the workflow asks for it. +- **Concurrent modals on different layers** (e.g. toast over modal). The chat error-block path already covers what toasts would. +- **Custom user-defined modal commands.** The trait is open, but `~/.config/ox/commands/*.md` discovery / loader is tracked separately under "Workflow Skills" in the roadmap. +- **Agent-triggered modal path** (`AgentEvent::PromptRequest` round-trip). The trait shape supports it; lands with the Permission & Approval roadmap item. + +## Sources + +- [`crates/oxide-code/src/tui/modal.rs`](../../../crates/oxide-code/src/tui/modal.rs) — `Modal`, `ModalKey`, `ModalAction`, `ModalStack`. +- [`crates/oxide-code/src/tui/modal/list_picker.rs`](../../../crates/oxide-code/src/tui/modal/list_picker.rs) — generic `ListPicker`. +- [`crates/oxide-code/src/slash/picker.rs`](../../../crates/oxide-code/src/slash/picker.rs) — model + effort picker. +- [`crates/oxide-code/src/slash/status_modal.rs`](../../../crates/oxide-code/src/slash/status_modal.rs) — status overview. +- [`crates/oxide-code/src/slash/context.rs`](../../../crates/oxide-code/src/slash/context.rs) — `SlashContext::open_modal` / `take_modal`. +- [`crates/oxide-code/src/tui/app.rs`](../../../crates/oxide-code/src/tui/app.rs) — `App::handle_crossterm_event` modal gate, `apply_modal_action`, layout band. diff --git a/docs/research/slash/modals.md b/docs/research/slash/modals.md index 4fe20a02..2832e607 100644 --- a/docs/research/slash/modals.md +++ b/docs/research/slash/modals.md @@ -4,6 +4,8 @@ Research on the modal / picker / dialog primitives that turn slash commands into Verified against locally-mirrored sources (2026-05-05): [Claude Code](https://github.com/hakula139/claude-code), [OpenAI Codex](https://github.com/openai/codex) `codex-rs/tui`, [opencode](https://github.com/anomalyco/opencode) `packages/`. +> **Status:** the design synthesis from this research has been implemented. See [`docs/design/slash/modals.md`](../../design/slash/modals.md) for the shipped shape. + ## Claude Code (TypeScript + Ink) Modals are React components rendered by Ink. ~50 of ~100 commands are `type: 'local-jsx'`. From 88f0ca706d32b3cd0acec449a52e9bf715b78610 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 19:10:12 +0800 Subject: [PATCH 10/19] test(slash): cover left-arrow effort cycle, no-tier render, thinking-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. --- crates/oxide-code/src/slash/picker.rs | 68 ++++++++++++++++++--- crates/oxide-code/src/slash/status_modal.rs | 15 +++++ 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index 59b0dc5f..c0bc862b 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -429,6 +429,26 @@ mod tests { ); } + #[test] + fn left_arrow_walks_effort_backward_with_wrap() { + // Backward branch in `cycle_effort` has different arithmetic + // from the forward branch — pin it independently. Cycle Left + // until the effort returns to the initial pick, asserting it + // wraps past the first tier (ladder length is finite). + let mut p = picker(InitialFocus::Effort); + let initial = p.effort.expect("Opus 4.7 has an effort axis"); + for _ in 0..16 { + p.handle_key(&key(KeyCode::Left)); + if p.effort == Some(initial) { + return; // wrapped back to the starting tier + } + } + panic!( + "Left-arrow cycle never returned to the starting tier; got {:?}", + p.effort + ); + } + // ── submit ── #[test] @@ -484,6 +504,24 @@ mod tests { assert!(matches!(outcome, ModalKey::Cancelled)); } + // ── height ── + + #[test] + fn height_drops_when_highlighted_model_lacks_effort_tier() { + // The effort row + spacer (2 rows) only render when the + // highlighted model has an effort tier. Pin the no-tier path + // so a regression that always reserves the row fails here. + let mut p = picker(InitialFocus::Model); + let with_tier = p.height(80); + p.handle_key(&key(KeyCode::Char('5'))); // jump to Haiku 4.5 + let no_tier = p.height(80); + assert_eq!( + with_tier.saturating_sub(no_tier), + 2, + "no-tier model drops exactly the effort row + spacer", + ); + } + // ── Render smoke ── #[test] @@ -491,16 +529,28 @@ mod tests { use ratatui::Terminal; use ratatui::backend::TestBackend; - let p = picker(InitialFocus::Model); let theme = Theme::default(); - for width in [40_u16, 80, 120] { - let h = p.height(width).min(20); - let mut terminal = Terminal::new(TestBackend::new(width, h)).unwrap(); - terminal - .draw(|frame| { - p.render(frame, Rect::new(0, 0, width, h), &theme); - }) - .expect("render must not panic"); + // Two cursor positions: an effort-tier model (Opus 4.7) so the + // effort row renders, and a no-tier model (Haiku 4.5) so the + // hide branch executes. Without the second case the hide path + // is reachable only via mutation tests. + for setup in [ + None, // Opus 4.7 — has effort tier + Some(KeyCode::Char('5')), // Haiku 4.5 — no effort tier + ] { + let mut p = picker(InitialFocus::Model); + if let Some(jump) = setup { + p.handle_key(&key(jump)); + } + for width in [40_u16, 80, 120] { + let h = p.height(width).min(20); + let mut terminal = Terminal::new(TestBackend::new(width, h)).unwrap(); + terminal + .draw(|frame| { + p.render(frame, Rect::new(0, 0, width, h), &theme); + }) + .expect("render must not panic"); + } } } } diff --git a/crates/oxide-code/src/slash/status_modal.rs b/crates/oxide-code/src/slash/status_modal.rs index c4322996..0f9bedbe 100644 --- a/crates/oxide-code/src/slash/status_modal.rs +++ b/crates/oxide-code/src/slash/status_modal.rs @@ -166,6 +166,21 @@ mod tests { assert_eq!(thinking_row.1, "off"); } + #[test] + fn new_renders_thinking_on_when_snapshot_says_true() { + // Pin the on-branch so a regression that always renders "off" + // fails here. The off-branch test alone can't catch that. + let mut info = test_session_info(); + info.config.show_thinking = true; + let m = StatusModal::new(&info); + let thinking_row = m + .rows + .iter() + .find(|(k, _)| *k == "Show Thinking") + .expect("show-thinking row"); + assert_eq!(thinking_row.1, "on"); + } + // ── handle_key ── #[test] From 8c70d9727f1b11134a1441c837d97f23c1dc6b67 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 19:56:56 +0800 Subject: [PATCH 11/19] test(slash): tighten picker submit assertions and add tier-restore scenario --- crates/oxide-code/src/slash/picker.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index c0bc862b..d408d0f6 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -449,6 +449,18 @@ mod tests { ); } + #[test] + fn navigating_from_no_tier_back_to_tier_model_restores_effort() { + let mut p = picker(InitialFocus::Model); + p.handle_key(&key(KeyCode::Char('5'))); // jump to Haiku + assert!(p.effort.is_none(), "Haiku has no effort tier"); + p.handle_key(&key(KeyCode::Up)); // back to Sonnet 4.6 [1m] (index 3) + assert!( + p.effort.is_some(), + "tier model must restore effort via effort_or_active fallback", + ); + } + // ── submit ── #[test] @@ -467,7 +479,10 @@ mod tests { let outcome = p.handle_key(&key(KeyCode::Enter)); match outcome { ModalKey::Submitted(ModalAction::User(UserAction::SwapConfig { model, effort })) => { - assert!(model.is_some(), "model must be set"); + assert_eq!( + model.map(ResolvedModelId::into_inner).as_deref(), + Some("claude-opus-4-7[1m]"), + ); assert!( effort.is_none(), "effort must NOT be set when only the model axis moved", @@ -479,7 +494,7 @@ mod tests { #[test] fn enter_after_effort_change_emits_swap_with_effort_only() { - // Opus 4.7 active + xhigh; cycle effort once, model unchanged. + // Opus 4.7 active + High; cycle effort Forward once → Xhigh. let mut p = picker(InitialFocus::Effort); p.handle_key(&key(KeyCode::Right)); let outcome = p.handle_key(&key(KeyCode::Enter)); @@ -489,7 +504,7 @@ mod tests { model.is_none(), "model must NOT be set when only the effort axis moved", ); - assert!(effort.is_some(), "effort must be set"); + assert_eq!(effort, Some(Effort::Xhigh)); } other => panic!("expected Submitted with effort-only SwapConfig, got {other:?}"), } From ade6a933bee9220afc503dc9bb81ef65ff2fb0a7 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 19:53:30 +0800 Subject: [PATCH 12/19] fix(tui): correct ListPicker header_height off-by-one when description 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. --- crates/oxide-code/src/tui/modal/list_picker.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/oxide-code/src/tui/modal/list_picker.rs b/crates/oxide-code/src/tui/modal/list_picker.rs index 77beb611..a18cd3c9 100644 --- a/crates/oxide-code/src/tui/modal/list_picker.rs +++ b/crates/oxide-code/src/tui/modal/list_picker.rs @@ -156,7 +156,7 @@ impl ListPicker { fn header_height(&self) -> u16 { let mut h = TITLE_ROW_HEIGHT + TITLE_BLANK_ROW; if self.description.is_some() { - h += 2; // description row + blank + h += 1; // description row (blank is already TITLE_BLANK_ROW) } h } @@ -383,11 +383,10 @@ mod tests { } #[test] - fn height_with_description_adds_two_more_rows() { - // Title (1) + description (1) + blank (1) + 1 spacer between - // header sections totals 4 header rows + items. + fn height_with_description_adds_one_more_row() { + // Title (1) + description (1) + blank (1) = 3 header rows + items. let p = picker(vec![FakeItem::new("a")]).with_description("a small picker"); - assert_eq!(p.height(80), 5); + assert_eq!(p.height(80), 4); } #[test] From e0ea986ab0174a0fb3a28585b0764cc190170104 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 19:54:43 +0800 Subject: [PATCH 13/19] fix(slash): prevent spurious SwapConfig on bare /effort with implicit default --- crates/oxide-code/src/slash/picker.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index d408d0f6..54484ef2 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -116,8 +116,14 @@ pub(super) struct ModelEffortPicker { /// Active model captured at open — used to detect whether the user /// changed the model axis on submit. active_model: String, - /// Active effort captured at open. Same purpose for the effort axis. + /// Active effort captured at open. Used for rendering the "(default)" + /// suffix in the effort row. active_effort: Option, + /// Resolved initial effort at open. Compared on submit to detect + /// whether the user actually changed the effort axis — prevents + /// spurious `SwapConfig` when the raw `active_effort` is `None` but + /// the model's default resolves to a concrete tier. + initial_effort: Option, /// User's current effort pick. Tracks Left/Right navigation. /// `None` when the highlighted model has no effort tier. effort: Option, @@ -150,6 +156,7 @@ impl ModelEffortPicker { list, active_model, active_effort, + initial_effort: effort, effort, effort_dirty, } @@ -206,7 +213,7 @@ impl ModelEffortPicker { .selected() .map_or_else(|| self.active_model.clone(), |row| row.id.to_owned()); let model_changed = model != self.active_model; - let effort_changed = self.effort_dirty && self.effort != self.active_effort; + let effort_changed = self.effort_dirty && self.effort != self.initial_effort; if !model_changed && !effort_changed { return ModalKey::Cancelled; @@ -472,6 +479,18 @@ mod tests { assert!(matches!(outcome, ModalKey::Cancelled)); } + #[test] + fn enter_immediately_after_effort_focus_with_no_explicit_effort_returns_cancelled() { + // When `active_effort` is `None` (user config has no explicit + // effort), the picker resolves the model's default. Pressing + // Enter immediately must not emit a spurious `SwapConfig`. + let mut info = test_session_info(); + info.config.effort = None; + let mut p = ModelEffortPicker::new(&info, InitialFocus::Effort); + let outcome = p.handle_key(&key(KeyCode::Enter)); + assert!(matches!(outcome, ModalKey::Cancelled)); + } + #[test] fn enter_after_model_change_emits_swap_with_model_only() { let mut p = picker(InitialFocus::Model); From eab69f19fab576fea7cbe9464c65e1bbcbf878c9 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 20:05:21 +0800 Subject: [PATCH 14/19] style(slash): fix stale doc comment, import grouping, and test section headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- crates/oxide-code/src/slash/picker.rs | 17 +++++--------- crates/oxide-code/src/slash/status_modal.rs | 25 ++++----------------- 2 files changed, 10 insertions(+), 32 deletions(-) diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index 54484ef2..23b28698 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -15,6 +15,7 @@ use ratatui::style::Modifier; use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; +use super::context::SessionInfo; use crate::agent::event::UserAction; use crate::config::Effort; use crate::model::{ResolvedModelId, capabilities_for, marketing_or_id}; @@ -22,14 +23,10 @@ use crate::tui::modal::list_picker::{ListPicker, PickerItem}; use crate::tui::modal::{Modal, ModalAction, ModalKey}; use crate::tui::theme::Theme; -use super::context::SessionInfo; - // ── Constants ── -/// Curated roster shown in the picker. Manual `/model ` resolves -/// against the full `MODELS` table; this slice only governs what the -/// modal lists. Mirrors `slash::model::LISTED_MODELS` — the typed-arg -/// path and the picker share semantics deliberately. +/// Curated roster shown in the picker. The typed-arg `/model ` resolves against the full +/// `MODELS` table; this slice governs what the modal lists. const LISTED_MODELS: &[&str] = &[ "claude-opus-4-7", "claude-opus-4-7[1m]", @@ -362,7 +359,7 @@ mod tests { KeyEvent::from(code) } - // ── Initial state ── + // ── new ── #[test] fn new_positions_cursor_on_active_model() { @@ -390,7 +387,7 @@ mod tests { assert!(!p.effort_dirty); } - // ── handle_key navigation ── + // ── handle_key ── #[test] fn down_arrow_advances_cursor_and_refreshes_effort() { @@ -468,8 +465,6 @@ mod tests { ); } - // ── submit ── - #[test] fn enter_with_no_changes_returns_cancelled() { // Open + Enter without touching anything is the same shape as @@ -556,7 +551,7 @@ mod tests { ); } - // ── Render smoke ── + // ── render ── #[test] fn render_runs_at_typical_widths_without_panicking() { diff --git a/crates/oxide-code/src/slash/status_modal.rs b/crates/oxide-code/src/slash/status_modal.rs index 0f9bedbe..b66665a0 100644 --- a/crates/oxide-code/src/slash/status_modal.rs +++ b/crates/oxide-code/src/slash/status_modal.rs @@ -1,9 +1,4 @@ -//! `/status` overview modal — read-only single panel of session -//! descriptors. Esc closes; no other keys do anything. -//! -//! Replaces the text-output `/status` block; the underlying data is -//! the same [`SessionInfo`] / [`ConfigSnapshot`] snapshot that backed -//! the old chat row. +//! `/status` overview modal — read-only single panel of session descriptors. Esc / Enter close. use crossterm::event::{KeyCode, KeyEvent}; use ratatui::Frame; @@ -12,22 +7,15 @@ use ratatui::style::Modifier; use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; +use super::context::SessionInfo; use crate::config::display_effort; use crate::tui::modal::{Modal, ModalKey}; use crate::tui::theme::Theme; -use super::context::SessionInfo; - // ── Constants ── -/// Title rendered in bold above the rows. const TITLE: &str = "Status"; - -/// Footer line rendered dim under the rows. const FOOTER: &str = "Esc to close"; - -/// Padding columns between the label and value columns. Mirrors -/// [`ListPicker`](crate::tui::modal::list_picker::ListPicker). const COLUMN_GAP: usize = 2; // ── StatusModal ── @@ -44,9 +32,6 @@ impl StatusModal { } else { "off".to_owned() }; - // Pin order: identity (Model / Effort) first, then session - // descriptors, then runtime knobs. Mirrors the routing-debug - // glance from the old text /status. let rows = vec![ ("Model", model), ("Effort", display_effort(info.config.effort)), @@ -63,7 +48,6 @@ impl StatusModal { impl Modal for StatusModal { fn height(&self, _width: u16) -> u16 { - // Title + blank + rows + blank + footer. let body = u16::try_from(self.rows.len()).unwrap_or(u16::MAX); body.saturating_add(4) } @@ -100,7 +84,6 @@ impl Modal for StatusModal { fn handle_key(&mut self, event: &KeyEvent) -> ModalKey { match event.code { - // Esc and Enter both dismiss — there's nothing to confirm. KeyCode::Esc | KeyCode::Enter => ModalKey::Cancelled, _ => ModalKey::Consumed, } @@ -120,7 +103,7 @@ mod tests { KeyEvent::from(code) } - // ── Construction ── + // ── new ── #[test] fn new_produces_one_row_per_session_descriptor() { @@ -214,7 +197,7 @@ mod tests { } } - // ── Render smoke ── + // ── render ── #[test] fn render_runs_at_typical_widths_without_panicking() { From b5a3332359bb8791b8600a833b8ba23c3ba52cce Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 21:57:06 +0800 Subject: [PATCH 15/19] fix(tui): forward modal-emitted SwapConfig and Clear through user_tx `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 ` 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. --- crates/oxide-code/src/tui/app.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index b5fdeae5..f62c3069 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -332,7 +332,7 @@ impl App { self.should_quit = true; true } - UserAction::Clear | UserAction::SwapConfig { .. } => false, + UserAction::Clear | UserAction::SwapConfig { .. } => true, } } @@ -1302,10 +1302,13 @@ mod tests { assert!(matches!(forwarded, UserAction::SubmitPrompt(s) if s == "queued")); } - #[test] - fn dispatch_local_only_actions_return_false_to_prevent_double_send() { + #[tokio::test] + async fn dispatch_swap_config_forwards_to_agent_through_user_tx() { + // Modal-emitted SwapConfig must reach the agent loop so it can call + // `apply_swap_config` and emit `ConfigChanged`. The earlier `=> false` arm in + // `apply_action_locally` swallowed it silently — caused empty title bar updates after + // picker submit. Pin both axes. for action in [ - UserAction::Clear, UserAction::SwapConfig { model: Some(crate::model::ResolvedModelId::new( "claude-opus-4-7".to_owned(), @@ -1316,14 +1319,13 @@ mod tests { model: None, effort: Some(crate::config::Effort::High), }, + UserAction::Clear, ] { let (mut app, mut rx, _agent_tx) = test_app(None); app.dispatch_user_action(action.clone()); - assert!( - matches!(rx.try_recv(), Err(mpsc::error::TryRecvError::Empty)), - "{action:?} must not reach user_tx via dispatch_user_action", - ); + let forwarded = rx.recv().await.expect("action forwarded to agent"); + assert_eq!(forwarded, action); assert!(!app.should_quit); assert_eq!(app.chat.entry_count(), 0); } From a31e54bb02dc5756d23d7519c501733611cb137c Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 21:57:15 +0800 Subject: [PATCH 16/19] feat(tui): paint top border above modal overlay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/oxide-code/src/tui/modal.rs | 93 ++++++++++++++++++++++++++---- 1 file changed, 83 insertions(+), 10 deletions(-) diff --git a/crates/oxide-code/src/tui/modal.rs b/crates/oxide-code/src/tui/modal.rs index e9644e10..c38b976c 100644 --- a/crates/oxide-code/src/tui/modal.rs +++ b/crates/oxide-code/src/tui/modal.rs @@ -21,10 +21,16 @@ pub(crate) mod list_picker; use crossterm::event::KeyEvent; use ratatui::Frame; use ratatui::layout::Rect; +use ratatui::text::{Line, Span}; +use ratatui::widgets::Paragraph; use crate::agent::event::UserAction; use crate::tui::theme::Theme; +/// One-row top separator above the modal body — visually delineates the modal from the chat. +const TOP_BORDER_HEIGHT: u16 = 1; +const TOP_BORDER_GLYPH: char = '─'; + // ── Modal Trait ── /// A focus-grabbing UI overlay. While active, the modal owns keyboard @@ -101,18 +107,43 @@ impl ModalStack { self.stack.push(modal); } - /// Total height the stack needs above the input. Today only the - /// top modal renders; the height reflects that. If we ever stack - /// visually, this sums. + /// Total height the stack needs above the input — top modal's body plus a one-row separator. pub(crate) fn height(&self, width: u16) -> u16 { - self.stack.last().map_or(0, |m| m.height(width)) + self.stack + .last() + .map_or(0, |m| m.height(width).saturating_add(TOP_BORDER_HEIGHT)) } - /// Render the visible modal into `area`. No-op if empty. + /// Render the visible modal into `area`. Paints a one-row top separator first, then delegates + /// the remainder to the modal. No-op if empty. pub(crate) fn render(&self, frame: &mut Frame<'_>, area: Rect, theme: &Theme) { - if let Some(top) = self.stack.last() { - top.render(frame, area, theme); + let Some(top) = self.stack.last() else { + return; + }; + if area.height == 0 { + return; + } + let border_area = Rect { + height: TOP_BORDER_HEIGHT.min(area.height), + ..area + }; + let border = Line::from(Span::styled( + TOP_BORDER_GLYPH.to_string().repeat(usize::from(area.width)), + theme.dim(), + )); + frame.render_widget(Paragraph::new(border).style(theme.surface()), border_area); + + let body_height = area.height.saturating_sub(TOP_BORDER_HEIGHT); + if body_height == 0 { + return; } + let body_area = Rect { + x: area.x, + y: area.y.saturating_add(TOP_BORDER_HEIGHT), + width: area.width, + height: body_height, + }; + top.render(frame, body_area, theme); } /// Deliver `event` to the top modal. Returns the action to dispatch @@ -211,7 +242,8 @@ mod tests { let mut stack = ModalStack::new(); stack.push(Box::new(ScriptedModal::new(ModalAction::None))); assert!(stack.is_active()); - assert_eq!(stack.height(80), 3); + // Modal body (3) + one-row top separator. + assert_eq!(stack.height(80), 3 + TOP_BORDER_HEIGHT); } #[test] @@ -250,6 +282,39 @@ mod tests { assert!(!stack.is_active()); } + #[test] + fn render_paints_top_border_then_delegates_body_below_it() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let mut stack = ModalStack::new(); + let modal = ScriptedModal::new(ModalAction::None); + let body_height = modal.declared_height; + stack.push(Box::new(modal)); + + let theme = Theme::default(); + let width: u16 = 12; + let total_height = stack.height(width); + assert_eq!(total_height, body_height + TOP_BORDER_HEIGHT); + + let mut terminal = Terminal::new(TestBackend::new(width, total_height)).unwrap(); + terminal + .draw(|frame| { + stack.render(frame, Rect::new(0, 0, width, total_height), &theme); + }) + .expect("render must not panic"); + + let buf = terminal.backend().buffer(); + for x in 0..width { + let symbol = buf[(x, 0)].symbol(); + assert_eq!( + symbol, + TOP_BORDER_GLYPH.to_string(), + "top row col {x} must be border glyph; got {symbol:?}", + ); + } + } + #[test] fn handle_key_on_empty_stack_returns_none_without_panicking() { // No active modal → no key delivery, no stack mutation. @@ -271,10 +336,18 @@ mod tests { top.declared_height = 5; stack.push(Box::new(top)); - assert_eq!(stack.height(80), 5, "top modal's height wins"); + assert_eq!( + stack.height(80), + 5 + TOP_BORDER_HEIGHT, + "top modal's height wins (plus border)" + ); let outcome = stack.handle_key(&key('s')); assert!(matches!(outcome, Some(ModalAction::None))); assert!(stack.is_active(), "inner modal still active"); - assert_eq!(stack.height(80), 3, "inner modal's height resumes"); + assert_eq!( + stack.height(80), + 3 + TOP_BORDER_HEIGHT, + "inner modal's height resumes (plus border)" + ); } } From 264993b17dd8e8505c2f55a9a08bf9996a366cf6 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 21:59:25 +0800 Subject: [PATCH 17/19] feat(config): default to claude-opus-4-7[1m] for 1M context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/oxide-code/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxide-code/src/config.rs b/crates/oxide-code/src/config.rs index e4528cc5..294e7f58 100644 --- a/crates/oxide-code/src/config.rs +++ b/crates/oxide-code/src/config.rs @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; use crate::tui::theme::{self, Theme}; use crate::util::env; -const DEFAULT_MODEL: &str = "claude-opus-4-7"; +const DEFAULT_MODEL: &str = "claude-opus-4-7[1m]"; const DEFAULT_BASE_URL: &str = "https://api.anthropic.com"; // ── Auth ── From 3b96b5f83468834dc5055b311be8411b82360bf6 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 22:18:26 +0800 Subject: [PATCH 18/19] test(tui): regroup modal tests by function and add list_picker default coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/oxide-code/src/tui/modal.rs | 115 ++++++++++++------ .../oxide-code/src/tui/modal/list_picker.rs | 29 +++++ 2 files changed, 104 insertions(+), 40 deletions(-) diff --git a/crates/oxide-code/src/tui/modal.rs b/crates/oxide-code/src/tui/modal.rs index c38b976c..92a77d87 100644 --- a/crates/oxide-code/src/tui/modal.rs +++ b/crates/oxide-code/src/tui/modal.rs @@ -228,7 +228,7 @@ mod tests { KeyEvent::from(KeyCode::Char(c)) } - // ── ModalStack ── + // ── is_active ── #[test] fn empty_stack_reports_inactive_and_zero_height() { @@ -237,6 +237,8 @@ mod tests { assert_eq!(stack.height(80), 0); } + // ── push ── + #[test] fn push_activates_stack_and_height_reflects_top_modal() { let mut stack = ModalStack::new(); @@ -246,6 +248,73 @@ mod tests { assert_eq!(stack.height(80), 3 + TOP_BORDER_HEIGHT); } + // ── render ── + + #[test] + fn render_paints_top_border_then_delegates_body_below_it() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let mut stack = ModalStack::new(); + let modal = ScriptedModal::new(ModalAction::None); + let body_height = modal.declared_height; + stack.push(Box::new(modal)); + + let theme = Theme::default(); + let width: u16 = 12; + let total_height = stack.height(width); + assert_eq!(total_height, body_height + TOP_BORDER_HEIGHT); + + let mut terminal = Terminal::new(TestBackend::new(width, total_height)).unwrap(); + terminal + .draw(|frame| { + stack.render(frame, Rect::new(0, 0, width, total_height), &theme); + }) + .expect("render must not panic"); + + let buf = terminal.backend().buffer(); + for x in 0..width { + let symbol = buf[(x, 0)].symbol(); + assert_eq!( + symbol, + TOP_BORDER_GLYPH.to_string(), + "top row col {x} must be border glyph; got {symbol:?}", + ); + } + } + + #[test] + fn render_no_ops_when_stack_empty_or_area_smaller_than_body() { + // Three short-circuit branches in `render`: empty stack, area.height == 0, and + // body_height == 0 (area only big enough for the border row). + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let theme = Theme::default(); + + let empty = ModalStack::new(); + let mut t1 = Terminal::new(TestBackend::new(8, 2)).unwrap(); + t1.draw(|frame| empty.render(frame, Rect::new(0, 0, 8, 2), &theme)) + .expect("empty render"); + + let mut stack = ModalStack::new(); + stack.push(Box::new(ScriptedModal::new(ModalAction::None))); + let mut t2 = Terminal::new(TestBackend::new(8, 1)).unwrap(); + t2.draw(|frame| stack.render(frame, Rect::new(0, 0, 8, 0), &theme)) + .expect("zero-height render"); + + // area.height == TOP_BORDER_HEIGHT — only the border fits; body skipped. + let mut t3 = Terminal::new(TestBackend::new(8, TOP_BORDER_HEIGHT)).unwrap(); + t3.draw(|frame| { + stack.render(frame, Rect::new(0, 0, 8, TOP_BORDER_HEIGHT), &theme); + }) + .expect("border-only render"); + let buf = t3.backend().buffer(); + assert_eq!(buf[(0, 0)].symbol(), TOP_BORDER_GLYPH.to_string()); + } + + // ── handle_key ── + #[test] fn handle_key_consumed_keeps_modal_active() { let mut stack = ModalStack::new(); @@ -282,39 +351,6 @@ mod tests { assert!(!stack.is_active()); } - #[test] - fn render_paints_top_border_then_delegates_body_below_it() { - use ratatui::Terminal; - use ratatui::backend::TestBackend; - - let mut stack = ModalStack::new(); - let modal = ScriptedModal::new(ModalAction::None); - let body_height = modal.declared_height; - stack.push(Box::new(modal)); - - let theme = Theme::default(); - let width: u16 = 12; - let total_height = stack.height(width); - assert_eq!(total_height, body_height + TOP_BORDER_HEIGHT); - - let mut terminal = Terminal::new(TestBackend::new(width, total_height)).unwrap(); - terminal - .draw(|frame| { - stack.render(frame, Rect::new(0, 0, width, total_height), &theme); - }) - .expect("render must not panic"); - - let buf = terminal.backend().buffer(); - for x in 0..width { - let symbol = buf[(x, 0)].symbol(); - assert_eq!( - symbol, - TOP_BORDER_GLYPH.to_string(), - "top row col {x} must be border glyph; got {symbol:?}", - ); - } - } - #[test] fn handle_key_on_empty_stack_returns_none_without_panicking() { // No active modal → no key delivery, no stack mutation. @@ -324,10 +360,9 @@ mod tests { } #[test] - fn nested_push_routes_keys_to_top_only() { - // Two-deep stack: keys go to the top until it pops, then the - // inner one resumes. Pin so a regression that fans keys to all - // layers fails here. + fn handle_key_with_nested_stack_routes_to_top_modal_only() { + // Two-deep stack: keys go to the top until it pops, then the inner one resumes. + // Pin so a regression that fans keys to all layers fails here. let mut stack = ModalStack::new(); stack.push(Box::new(ScriptedModal::new(ModalAction::User( UserAction::Clear, @@ -339,7 +374,7 @@ mod tests { assert_eq!( stack.height(80), 5 + TOP_BORDER_HEIGHT, - "top modal's height wins (plus border)" + "top modal's height wins (plus border)", ); let outcome = stack.handle_key(&key('s')); assert!(matches!(outcome, Some(ModalAction::None))); @@ -347,7 +382,7 @@ mod tests { assert_eq!( stack.height(80), 3 + TOP_BORDER_HEIGHT, - "inner modal's height resumes (plus border)" + "inner modal's height resumes (plus border)", ); } } diff --git a/crates/oxide-code/src/tui/modal/list_picker.rs b/crates/oxide-code/src/tui/modal/list_picker.rs index a18cd3c9..b41a8ae7 100644 --- a/crates/oxide-code/src/tui/modal/list_picker.rs +++ b/crates/oxide-code/src/tui/modal/list_picker.rs @@ -419,4 +419,33 @@ mod tests { }) .expect("render must not panic"); } + + #[test] + fn render_handles_picker_and_items_with_no_descriptions() { + // Drives the `PickerItem` default impls (`description` / `is_active` / `key_hint` all + // unset) and the render branches that skip the optional description rows. Without this + // the trait-default arms and the no-description render arms stay uncovered. + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + struct MinimalItem(&'static str); + impl PickerItem for MinimalItem { + fn label(&self) -> &str { + self.0 + } + } + + let item = MinimalItem("solo"); + assert!(item.description().is_none()); + assert!(!item.is_active()); + assert!(item.key_hint().is_none()); + + let p = ListPicker::new("Pick one", vec![MinimalItem("a"), MinimalItem("b")]); + let theme = Theme::default(); + let h = p.height(40); + let mut terminal = Terminal::new(TestBackend::new(40, h)).unwrap(); + terminal + .draw(|frame| p.render(frame, Rect::new(0, 0, 40, h), &theme)) + .expect("render must not panic without descriptions"); + } } From d841ff17d5e1f409ba3d29b98386b411b5933b7b Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Tue, 5 May 2026 22:18:45 +0800 Subject: [PATCH 19/19] refactor(slash): drop bare /effort picker form, keep typed-arg shortcut MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ` 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`. --- crates/oxide-code/src/slash/effort.rs | 66 ++++----- crates/oxide-code/src/slash/model.rs | 5 +- crates/oxide-code/src/slash/picker.rs | 126 ++++++------------ crates/oxide-code/src/tui/app.rs | 23 ++-- ...r_empty_query_shows_each_command_once.snap | 2 +- ...narrow_terminal_truncates_description.snap | 2 +- 6 files changed, 82 insertions(+), 142 deletions(-) diff --git a/crates/oxide-code/src/slash/effort.rs b/crates/oxide-code/src/slash/effort.rs index 2fce961c..c60c2205 100644 --- a/crates/oxide-code/src/slash/effort.rs +++ b/crates/oxide-code/src/slash/effort.rs @@ -1,9 +1,5 @@ -//! `/effort` — open the model+effort picker focused on the effort -//! axis, or `/effort ` to swap directly. The agent loop calls -//! [`Client::set_effort`](crate::client::anthropic::Client::set_effort) -//! on the typed-arg path; the picker emits a single -//! [`UserAction::SwapConfig`] which routes through the same client -//! resolver. +//! `/effort ` — direct effort swap. The bare form errors with a usage hint; users adjust +//! effort interactively via the `/model` picker (Left / Right on the effort row). use super::context::SlashContext; use super::registry::{SlashCommand, SlashKind, SlashOutcome}; @@ -19,31 +15,26 @@ impl SlashCommand for EffortCmd { } fn description(&self) -> &'static str { - "Open the picker focused on effort, or set a level with `/effort `" + "Set the effort tier with `/effort `" } - fn classify(&self, args: &str) -> SlashKind { - // Bare opens the picker (UI-local; safe mid-turn). The - // typed-arg form races the in-flight `Client` and must wait. - if args.trim().is_empty() { - SlashKind::ReadOnly - } else { - SlashKind::Mutating - } + fn classify(&self, _args: &str) -> SlashKind { + // Both bare (error response) and typed (real swap) paths reach `execute`; the typed + // path races the in-flight client, so gate as Mutating. + SlashKind::Mutating } fn usage(&self) -> Option<&'static str> { - Some("[]") + Some("") } fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { - ctx.open_modal(Box::new(super::picker::ModelEffortPicker::new( - ctx.info, - super::picker::InitialFocus::Effort, - ))); - return Ok(SlashOutcome::Done); + return Err(format!( + "Usage: /effort . Valid: {}. Or use /model to pick interactively.", + Effort::VALID_VALUES, + )); } let pick = parse_effort_arg(arg)?; // Preflight: setting an explicit level on a no-effort model is @@ -84,13 +75,13 @@ mod tests { assert_eq!(EffortCmd.name(), "effort"); assert!(EffortCmd.aliases().is_empty()); assert!(!EffortCmd.description().is_empty()); - assert_eq!(EffortCmd.usage(), Some("[]")); + assert_eq!(EffortCmd.usage(), Some("")); } #[test] - fn classify_splits_on_args() { - assert_eq!(EffortCmd.classify(""), SlashKind::ReadOnly); - assert_eq!(EffortCmd.classify(" "), SlashKind::ReadOnly); + fn classify_is_mutating_regardless_of_args() { + assert_eq!(EffortCmd.classify(""), SlashKind::Mutating); + assert_eq!(EffortCmd.classify(" "), SlashKind::Mutating); assert_eq!(EffortCmd.classify("xhigh"), SlashKind::Mutating); } @@ -115,20 +106,17 @@ mod tests { } #[test] - fn execute_no_args_opens_picker_focused_on_effort() { - // Bare `/effort` opens the same picker as `/model`, but pre-armed - // on the effort axis so a single Enter submits the active level. - // Picker behavior is covered in `slash::picker` tests. - let mut chat = ChatView::new(&Theme::default(), false); - let info = test_session_info(); - let mut ctx = SlashContext::new(&mut chat, &info); - let outcome = EffortCmd.execute("", &mut ctx); - assert_eq!(outcome, Ok(SlashOutcome::Done)); - assert!( - ctx.take_modal().is_some(), - "bare /effort must populate the modal slot", - ); - assert_eq!(chat.entry_count(), 0, "chat must stay clean on open"); + fn execute_no_args_errors_with_usage_hint_and_model_pointer() { + // Bare `/effort` is invalid usage — the typed-arg form is the only direct shortcut; + // interactive adjustment lives in `/model`. + let (chat, outcome) = run_execute(""); + let msg = outcome.expect_err("bare /effort must error"); + assert!(msg.contains("Usage: /effort "), "{msg}"); + assert!(msg.contains("/model"), "must point at the picker: {msg}"); + for valid in Effort::VALID_VALUES.split(", ") { + assert!(msg.contains(valid), "lists `{valid}`: {msg}"); + } + assert_eq!(chat.entry_count(), 0, "execute must not push on Err"); } #[test] diff --git a/crates/oxide-code/src/slash/model.rs b/crates/oxide-code/src/slash/model.rs index d3e88dcb..a4443fa2 100644 --- a/crates/oxide-code/src/slash/model.rs +++ b/crates/oxide-code/src/slash/model.rs @@ -53,10 +53,7 @@ impl SlashCommand for ModelCmd { fn execute(&self, args: &str, ctx: &mut SlashContext<'_>) -> Result { let arg = args.trim(); if arg.is_empty() { - ctx.open_modal(Box::new(super::picker::ModelEffortPicker::new( - ctx.info, - super::picker::InitialFocus::Model, - ))); + ctx.open_modal(Box::new(super::picker::ModelEffortPicker::new(ctx.info))); return Ok(SlashOutcome::Done); } let id = resolve_model_arg(arg)?; diff --git a/crates/oxide-code/src/slash/picker.rs b/crates/oxide-code/src/slash/picker.rs index 23b28698..c63594ca 100644 --- a/crates/oxide-code/src/slash/picker.rs +++ b/crates/oxide-code/src/slash/picker.rs @@ -99,54 +99,34 @@ impl PickerItem for ModelRow { // ── ModelEffortPicker ── -/// Initial focus. `/model` opens with the model list active; -/// `/effort` opens with the effort axis pre-armed so Enter submits -/// just-the-effort change. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum InitialFocus { - Model, - Effort, -} - pub(super) struct ModelEffortPicker { list: ListPicker, - /// Active model captured at open — used to detect whether the user - /// changed the model axis on submit. + /// Active model captured at open — used to detect whether the user changed the model axis + /// on submit. active_model: String, - /// Active effort captured at open. Used for rendering the "(default)" - /// suffix in the effort row. + /// Active effort captured at open. Used for rendering the "(default)" suffix in the effort row. active_effort: Option, - /// Resolved initial effort at open. Compared on submit to detect - /// whether the user actually changed the effort axis — prevents - /// spurious `SwapConfig` when the raw `active_effort` is `None` but - /// the model's default resolves to a concrete tier. + /// Resolved initial effort. Compared on submit to detect a real effort change — prevents + /// spurious `SwapConfig` when `active_effort` is `None` but the model resolves a default. initial_effort: Option, - /// User's current effort pick. Tracks Left/Right navigation. - /// `None` when the highlighted model has no effort tier. + /// Current effort pick. Tracks Left / Right navigation. `None` when the highlighted model + /// has no effort tier. effort: Option, - /// Whether the user touched the effort axis after open. When false - /// and the model didn't change either, Enter is a no-op. + /// Whether the user touched the effort axis. False on open; only Left / Right cycling sets it. effort_dirty: bool, } impl ModelEffortPicker { - pub(super) fn new(info: &SessionInfo, focus: InitialFocus) -> Self { + pub(super) fn new(info: &SessionInfo) -> Self { let active_model = info.config.model_id.clone(); let active_effort = info.config.effort; let rows = ModelRow::build(&active_model); - let mut list = ListPicker::new( - "Select model", - rows, - ) - .with_description( + let mut list = ListPicker::new("Select model", rows).with_description( "Switch the active model. Applies to this session only — restart returns to your config.", ); list.select_initial(|row| row.is_active); - // Bare `/effort` arms the effort axis so Left/Right is the - // first navigation; bare `/model` leaves both axes pristine. - let effort_dirty = matches!(focus, InitialFocus::Effort); let effort = effort_for_highlighted(&list, active_effort); Self { @@ -155,7 +135,7 @@ impl ModelEffortPicker { active_effort, initial_effort: effort, effort, - effort_dirty, + effort_dirty: false, } } @@ -351,8 +331,8 @@ mod tests { use super::*; use crate::slash::test_session_info; - fn picker(focus: InitialFocus) -> ModelEffortPicker { - ModelEffortPicker::new(&test_session_info(), focus) + fn picker() -> ModelEffortPicker { + ModelEffortPicker::new(&test_session_info()) } fn key(code: KeyCode) -> KeyEvent { @@ -364,26 +344,18 @@ mod tests { #[test] fn new_positions_cursor_on_active_model() { // `test_session_info` ships claude-opus-4-7 active. - let p = picker(InitialFocus::Model); + let p = picker(); let row = p.list.selected().expect("active row"); assert_eq!(row.id, "claude-opus-4-7"); assert!(row.is_active); } #[test] - fn new_with_effort_focus_marks_effort_dirty() { - // Bare /effort opens the picker with the effort axis already - // armed, so a single Enter submits the current pick. - let p = picker(InitialFocus::Effort); - assert!( - p.effort_dirty, - "InitialFocus::Effort must arm the effort axis", - ); - } - - #[test] - fn new_with_model_focus_keeps_effort_clean() { - let p = picker(InitialFocus::Model); + fn new_opens_with_clean_effort_axis() { + // Effort dirty must start false so a no-touch Enter cancels rather than firing a + // spurious SwapConfig (matters when the model resolves a default effort but the user + // hasn't expressed a pick). + let p = picker(); assert!(!p.effort_dirty); } @@ -391,7 +363,7 @@ mod tests { #[test] fn down_arrow_advances_cursor_and_refreshes_effort() { - let mut p = picker(InitialFocus::Model); + let mut p = picker(); let before = p.list.selected_index(); p.handle_key(&key(KeyCode::Down)); assert_eq!(p.list.selected_index(), before + 1); @@ -400,7 +372,7 @@ mod tests { #[test] fn numeric_jump_routes_cursor_to_matching_row() { // `5` jumps to the fifth listed model — Haiku 4.5. - let mut p = picker(InitialFocus::Model); + let mut p = picker(); p.handle_key(&key(KeyCode::Char('5'))); let row = p.list.selected().expect("selected row"); assert_eq!(row.id, "claude-haiku-4-5"); @@ -410,7 +382,7 @@ mod tests { fn right_arrow_cycles_effort_within_supported_levels() { // Opus 4.7 supports the full ladder. Pressing Right walks // through it; Left walks back. - let mut p = picker(InitialFocus::Model); + let mut p = picker(); let initial = p.effort; p.handle_key(&key(KeyCode::Right)); assert_ne!(p.effort, initial, "Right must change effort"); @@ -419,9 +391,8 @@ mod tests { #[test] fn right_arrow_on_no_tier_model_is_a_noop() { - // Haiku 4.5 has no effort tier — Left/Right must not mutate - // the (None) effort state. - let mut p = picker(InitialFocus::Model); + // Haiku 4.5 has no effort tier — Left/Right must not mutate the (None) effort state. + let mut p = picker(); p.handle_key(&key(KeyCode::Char('5'))); // jump to Haiku assert!(p.effort.is_none()); assert!(!p.effort_dirty); @@ -435,16 +406,15 @@ mod tests { #[test] fn left_arrow_walks_effort_backward_with_wrap() { - // Backward branch in `cycle_effort` has different arithmetic - // from the forward branch — pin it independently. Cycle Left - // until the effort returns to the initial pick, asserting it - // wraps past the first tier (ladder length is finite). - let mut p = picker(InitialFocus::Effort); + // Backward branch in `cycle_effort` has different arithmetic from the forward branch + // — pin it independently. Cycle Left until the effort returns to the initial pick. + let mut p = picker(); + p.handle_key(&key(KeyCode::Right)); // arm the axis with a known starting tier let initial = p.effort.expect("Opus 4.7 has an effort axis"); for _ in 0..16 { p.handle_key(&key(KeyCode::Left)); if p.effort == Some(initial) { - return; // wrapped back to the starting tier + return; } } panic!( @@ -455,7 +425,7 @@ mod tests { #[test] fn navigating_from_no_tier_back_to_tier_model_restores_effort() { - let mut p = picker(InitialFocus::Model); + let mut p = picker(); p.handle_key(&key(KeyCode::Char('5'))); // jump to Haiku assert!(p.effort.is_none(), "Haiku has no effort tier"); p.handle_key(&key(KeyCode::Up)); // back to Sonnet 4.6 [1m] (index 3) @@ -469,26 +439,14 @@ mod tests { fn enter_with_no_changes_returns_cancelled() { // Open + Enter without touching anything is the same shape as // Esc — nothing to dispatch. - let mut p = picker(InitialFocus::Model); - let outcome = p.handle_key(&key(KeyCode::Enter)); - assert!(matches!(outcome, ModalKey::Cancelled)); - } - - #[test] - fn enter_immediately_after_effort_focus_with_no_explicit_effort_returns_cancelled() { - // When `active_effort` is `None` (user config has no explicit - // effort), the picker resolves the model's default. Pressing - // Enter immediately must not emit a spurious `SwapConfig`. - let mut info = test_session_info(); - info.config.effort = None; - let mut p = ModelEffortPicker::new(&info, InitialFocus::Effort); + let mut p = picker(); let outcome = p.handle_key(&key(KeyCode::Enter)); assert!(matches!(outcome, ModalKey::Cancelled)); } #[test] fn enter_after_model_change_emits_swap_with_model_only() { - let mut p = picker(InitialFocus::Model); + let mut p = picker(); p.handle_key(&key(KeyCode::Down)); let outcome = p.handle_key(&key(KeyCode::Enter)); match outcome { @@ -509,7 +467,7 @@ mod tests { #[test] fn enter_after_effort_change_emits_swap_with_effort_only() { // Opus 4.7 active + High; cycle effort Forward once → Xhigh. - let mut p = picker(InitialFocus::Effort); + let mut p = picker(); p.handle_key(&key(KeyCode::Right)); let outcome = p.handle_key(&key(KeyCode::Enter)); match outcome { @@ -526,7 +484,7 @@ mod tests { #[test] fn esc_returns_cancelled_regardless_of_axis_state() { - let mut p = picker(InitialFocus::Model); + let mut p = picker(); p.handle_key(&key(KeyCode::Down)); p.handle_key(&key(KeyCode::Right)); let outcome = p.handle_key(&key(KeyCode::Esc)); @@ -537,10 +495,10 @@ mod tests { #[test] fn height_drops_when_highlighted_model_lacks_effort_tier() { - // The effort row + spacer (2 rows) only render when the - // highlighted model has an effort tier. Pin the no-tier path - // so a regression that always reserves the row fails here. - let mut p = picker(InitialFocus::Model); + // The effort row + spacer (2 rows) only render when the highlighted model has an + // effort tier. Pin the no-tier path so a regression that always reserves the row + // fails here. + let mut p = picker(); let with_tier = p.height(80); p.handle_key(&key(KeyCode::Char('5'))); // jump to Haiku 4.5 let no_tier = p.height(80); @@ -559,15 +517,13 @@ mod tests { use ratatui::backend::TestBackend; let theme = Theme::default(); - // Two cursor positions: an effort-tier model (Opus 4.7) so the - // effort row renders, and a no-tier model (Haiku 4.5) so the - // hide branch executes. Without the second case the hide path - // is reachable only via mutation tests. + // Two cursor positions: an effort-tier model (Opus 4.7) so the effort row renders, and + // a no-tier model (Haiku 4.5) so the hide branch executes. for setup in [ None, // Opus 4.7 — has effort tier Some(KeyCode::Char('5')), // Haiku 4.5 — no effort tier ] { - let mut p = picker(InitialFocus::Model); + let mut p = picker(); if let Some(jump) = setup { p.handle_key(&key(jump)); } diff --git a/crates/oxide-code/src/tui/app.rs b/crates/oxide-code/src/tui/app.rs index f62c3069..6b85eac3 100644 --- a/crates/oxide-code/src/tui/app.rs +++ b/crates/oxide-code/src/tui/app.rs @@ -1481,20 +1481,19 @@ mod tests { #[tokio::test] async fn dispatch_bare_slash_during_busy_opens_modal_picker() { - // Bare form classifies as ReadOnly so it dispatches mid-turn — - // and now opens the picker modal instead of printing a list. - // The arg-bearing form continues to refuse mid-turn. - for cmd in ["/model", "/effort"] { - let (mut app, _rx, _agent_tx) = test_app(None); - app.dispatch_user_action(UserAction::SubmitPrompt("active".to_owned())); + // Bare `/model` classifies as ReadOnly so it dispatches mid-turn and opens the picker + // modal instead of printing a list. (`/effort` is Mutating regardless of args after the + // typed-arg-only refactor — its bare-form busy path is covered by + // `dispatch_arg_bearing_slash_during_busy_refuses_with_system_message_no_forward`.) + let (mut app, _rx, _agent_tx) = test_app(None); + app.dispatch_user_action(UserAction::SubmitPrompt("active".to_owned())); - app.dispatch_user_action(UserAction::SubmitPrompt(cmd.to_owned())); + app.dispatch_user_action(UserAction::SubmitPrompt("/model".to_owned())); - assert!( - app.modals.is_active(), - "{cmd}: bare form must push a modal mid-turn", - ); - } + assert!( + app.modals.is_active(), + "bare /model must push a modal mid-turn", + ); } #[tokio::test] diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap index 946a553a..6453efb1 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_empty_query_shows_each_command_once.snap @@ -5,7 +5,7 @@ expression: "render_to_backend(&popup, 60)" "/clear Reset the conversation context " "/config Show the resolved configuration and the layered ..." "/diff Show uncommitted working-tree changes (`git diff..." -"/effort Open the picker focused on effort, or set a leve..." +"/effort Set the effort tier with `/effort ` " "/help List the available slash commands and their usag..." "/init Generate or update the project's `AGENTS.md` / `..." "/model Open the model picker or switch directly with `/..." diff --git a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap index 2f8590e2..12eed609 100644 --- a/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap +++ b/crates/oxide-code/src/tui/components/input/snapshots/ox__tui__components__input__popup__tests__render_narrow_terminal_truncates_description.snap @@ -5,7 +5,7 @@ expression: "render_to_backend(&popup, 30)" "/clear Reset the conversa..." "/config Show the resolved ..." "/diff Show uncommitted w..." -"/effort Open the picker fo..." +"/effort Set the effort tie..." "/help List the available..." "/init Generate or update..." "/model Open the model pic..."