Fast mode: /fast command, model marker, and transient toast#490
Fast mode: /fast command, model marker, and transient toast#490gnanam1990 wants to merge 3 commits into
Conversation
/fast [on|off] switches the session model between its base and a faster same-family variant (e.g. Sonnet<->Haiku, GPT-4.1<->GPT-4.1-mini). State is DERIVED from the model identity, never stored: the session is "in fast mode" iff the current model has a base variant. - modelregistry: a config-driven base<->fast mapping table (fastVariantPairs) plus Registry.FastVariant / BaseVariant, mirroring the existing UpgradeTarget pattern. Adding a pair is a one-line data change — no logic edit. Populated for the models with a meaningful fast variant (Sonnet/Haiku, GPT-4.1/mini, GPT-4o/mini, Gemini-2.5 Pro/Flash), guarded by tests that every id resolves and no id is both a base and a fast. - tui: /fast registered in commandDefinitions (model group) and dispatched via a pure planFastCommand decision + handleFastCommand, which delegates the real switch to handleModelCommand (provider rebuild, compaction guard, persistence) rather than mutating state directly. Notices use the existing appendSystem mechanism. - Tests: table-driven over every case — invalid arg, no model set, no-op both ways, no fast variant, switch to fast (⚡) / back to base, switch-error surfaced, and the variant helpers. Gate: make build / go vet ./... / go test ./... -race / make lint all green.
…istry Add a FastVariantID field to ModelEntry (mirroring UpgradeTargetID), populated in the catalog for the four same-family base/fast pairs and validated at registry build. Refactor the /fast helpers to derive both directions from this field instead of a hardcoded table, and add a HasFastVariant predicate for UI callers. Memoize DefaultRegistry with sync.OnceValues: the default catalog is static, every accessor returns clones, and a built registry is never mutated, so per-frame TUI callers no longer re-clone entries and recompile fuzzy-match regexes each render.
Surface which models have a fast variant: a dim "f" badge on the active-model chip, the /model picker, and the /provider wizard, plus a focused-row "fast: <name>" hint (the pickers are keyboard-driven, so the selected row is the reveal). All read Registry.FastVariant, so coverage tracks the catalog automatically. Give /fast a distinctive voice and stop it cluttering the transcript: reword its messages and show the result as a transient toast pill in the row above the composer, auto-dismissed after 3s (seq-gated, same pattern as the copy-status flash).
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
WalkthroughAdds a fast/base model variant system: ChangesFast variant feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: Straight talk for the reviewer: check that 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/modelregistry/models.go (1)
378-389: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider hardening the FastVariantID validation at build time.
The self-reference and uniqueness invariants (
TestCatalogFastVariantsResolve,TestFastVariantPairsUnambiguous) are currently only enforced by unit tests, not byNewRegistryitself — unlike the "resolves to a known model" check added here. A future catalog edit could introduce a self-referencing or ambiguousFastVariantIDwithout the registry failing loudly, only surfacing via CI test runs.♻️ Proposed hardening
for _, entry := range registry.models { fastID := strings.TrimSpace(entry.FastVariantID) if fastID == "" { continue } - if _, ok := registry.Get(fastID); !ok { + target, ok := registry.Get(fastID) + if !ok { return Registry{}, fmt.Errorf("model %q fast variant %q does not resolve to a known model", entry.ID, fastID) } + if target.ID == entry.ID { + return Registry{}, fmt.Errorf("model %q names itself as its fast variant", entry.ID) + } } + seenFastTargets := make(map[string]string) + for _, entry := range registry.models { + fastID := strings.TrimSpace(entry.FastVariantID) + if fastID == "" { + continue + } + target, _ := registry.Get(fastID) + if prevBase, dup := seenFastTargets[target.ID]; dup { + return Registry{}, fmt.Errorf("models %q and %q both declare %q as their fast variant", prevBase, entry.ID, target.ID) + } + seenFastTargets[target.ID] = entry.ID + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/modelregistry/models.go` around lines 378 - 389, Strengthen FastVariantID validation inside NewRegistry alongside the existing registry.Get(fastID) check: verify each non-empty FastVariantID is not self-referencing, and ensure fast-variant mappings are unambiguous across registry.models. Use the existing FastVariantID handling in the registry build path to reject duplicate or ambiguous fast targets and return an error immediately, rather than relying only on TestCatalogFastVariantsResolve and TestFastVariantPairsUnambiguous.internal/tui/fast_command.go (1)
22-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant initial assignment flagged by ineffassign.
enable := trueon line 25 is always overwritten before use — every switch branch reassigns it (or returns early indefault). golangci-lint correctly flags this; declare it unset and let the switch set it.🧹 Proposed fix
- enable := true + var enable bool switch strings.ToLower(strings.TrimSpace(arg)) { case "", "on": enable = true case "off": enable = false default: return fastPlan{message: fmt.Sprintf("⚡ /fast takes on or off, not %q", strings.TrimSpace(arg))} }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/fast_command.go` around lines 22 - 33, The initial assignment in planFastCommand is redundant and gets overwritten before use, so golangci-lint flags it. Update planFastCommand in fast_command.go by removing the default enable initialization and let the switch on strings.ToLower(strings.TrimSpace(arg)) set enable explicitly in the ""/"on" and "off" branches, keeping the existing default usage-error return unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/modelregistry/models.go`:
- Around line 378-389: Strengthen FastVariantID validation inside NewRegistry
alongside the existing registry.Get(fastID) check: verify each non-empty
FastVariantID is not self-referencing, and ensure fast-variant mappings are
unambiguous across registry.models. Use the existing FastVariantID handling in
the registry build path to reject duplicate or ambiguous fast targets and return
an error immediately, rather than relying only on TestCatalogFastVariantsResolve
and TestFastVariantPairsUnambiguous.
In `@internal/tui/fast_command.go`:
- Around line 22-33: The initial assignment in planFastCommand is redundant and
gets overwritten before use, so golangci-lint flags it. Update planFastCommand
in fast_command.go by removing the default enable initialization and let the
switch on strings.ToLower(strings.TrimSpace(arg)) set enable explicitly in the
""/"on" and "off" branches, keeping the existing default usage-error return
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 783afd58-50af-4c08-8df0-253f28a3097b
📒 Files selected for processing (13)
internal/modelregistry/catalog.gointernal/modelregistry/models.gointernal/modelregistry/variants.gointernal/modelregistry/variants_test.gointernal/tui/commands.gointernal/tui/fast_command.gointernal/tui/fast_command_test.gointernal/tui/fast_marker.gointernal/tui/fast_marker_test.gointernal/tui/model.gointernal/tui/picker.gointernal/tui/provider_wizard.gointernal/tui/view.go
Summary
Adds a
/fastcommand and a first-class "fast mode" affordance to the TUI: switch the active model to its faster same-family variant, see at a glance which models offer one, and get transient feedback that stays out of the transcript.Three focused commits:
/fast [on|off]— toggle the session model between its base and fast variant (state is derived from model identity, not stored).FastVariantIDfield as the single source of truth, plus a memoizedDefaultRegistry().fmarker + focused-row hint across the model/provider pickers and the active-model chip, and a transient toast for/fastfeedback.Registry:
FastVariantID(data, not logic)FastVariantIDfield onModelEntry, mirroring the existingUpgradeTargetIDpattern: populated in the catalog viadecorateModelDepth, validated at registry build (must resolve to a known model).Registry.FastVariant/BaseVariant/HasFastVariantderive both directions from this one field — the previous hardcoded pair table is gone. Adding a pair is now a one-line catalog change.claude-sonnet-4.5 → claude-haiku-4.5gpt-4.1 → gpt-4.1-minigpt-4o → gpt-4o-minigemini-2.5-pro → gemini-2.5-flashDefaultRegistry()is now memoized (sync.OnceValues): the default catalog is static, every accessor returns clones, and a built registry is never mutated, so the per-frame TUI callers (title chip, context gauge, pickers) stop re-cloning entries and recompiling fuzzy-match regexes on every render. This also removes a pre-existing per-frame cost in the context gauge.UI: marker, hint, and toast
fmarker on models that have a fast variant — on the/modelpicker rows, the/providerwizard rows, and the active-model chip in the title bar.⚡ fast: <name> · /fast on): the pickers are keyboard-driven and the TUI has no floating-tooltip primitive, so the reveal is the selected-row detail line (works for keyboard-only users)./fastfeedback no longer appends a permanent transcript line — it shows a brand pill in the fixed-height row above the composer and auto-dismisses after 3s (seq-gated, reusing the existing copy-status self-clear pattern). Messages have a distinct voice:⚡ Fast lane on · <model>,Fast lane off · <model>,⚡ No fast lane for <model>,⚡ Pick a model first · /model.Coverage
Fast mode covers the four base models above (the curated registry's three providers: openai / anthropic / google). Runtime-configured providers and off-catalog models simply show no marker until they are catalogued with a
FastVariantID— the field-driven design means they light up automatically once added, with no code change.Testing
go build ./...,go vet ./...,gofmtclean.go test ./... -race→ 73 packages, 0 failures, 0 races.FastVariant/BaseVariant/HasFastVariant+ catalog-pair invariants), markers/hint across all three surfaces, and the toast (raise / newline-flatten / dismiss).Notes
Rebased on latest
main(includes/newfrom #478); no conflicts remain. Team contribution.Summary by CodeRabbit
New Features
/fastcommand to switch between standard and faster model variants.Bug Fixes