feat(tui): add hotbar action registry foundation#2866
Conversation
Introduce the hotbar action trait and registry, and register the built-in app actions needed by the first hotbar slice.
|
Thanks @reidliu41 for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
There was a problem hiding this comment.
Code Review
This pull request introduces a hotbar action registry foundation for the TUI, defining the HotbarAction trait, a registry for built-in actions (such as mode toggles, sidebar toggles, and command palette triggers), and integrating it into the main App state. The review feedback highlights several areas for improvement: ensuring that app.needs_redraw = true is set when updating status messages, pushing views, or toggling trust mode to guarantee immediate UI updates; avoiding hardcoded user-facing strings to comply with internationalization standards; and handling runtime conditions gracefully instead of using bail!, which could disrupt the TUI event loop.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| app.status_message = | ||
| Some("Voice input is not available in this terminal session yet.".to_string()); |
| app.status_message = | ||
| Some("Voice input is not available in this terminal session yet.".to_string()); | ||
| Ok(HotbarDispatch::Handled) |
There was a problem hiding this comment.
When handling a hotbar action that updates the application state (such as setting app.status_message), you should set app.needs_redraw = true to ensure the UI immediately reflects the changes.
app.status_message =
Some("Voice input is not available in this terminal session yet.".to_string());
app.needs_redraw = true;
Ok(HotbarDispatch::Handled)| if app.auto_model { | ||
| bail!("Reasoning effort is controlled by auto model routing."); | ||
| } |
There was a problem hiding this comment.
Returning a hard Result::Err (via bail!) for a common runtime condition (such as trying to cycle reasoning effort when auto model routing is active) can propagate up to the main event loop and potentially disrupt the TUI. Instead, handle this gracefully by setting a status message and returning Ok(HotbarDispatch::Handled).
| if app.auto_model { | |
| bail!("Reasoning effort is controlled by auto model routing."); | |
| } | |
| if app.auto_model { | |
| app.status_message = Some("Reasoning effort is controlled by auto model routing.".to_string()); | |
| app.needs_redraw = true; | |
| return Ok(HotbarDispatch::Handled); | |
| } |
| app.view_stack | ||
| .push(CommandPaletteView::new(build_command_palette_entries( | ||
| app.ui_locale, | ||
| &app.skills_dir, | ||
| &app.workspace, | ||
| &app.mcp_config_path, | ||
| app.mcp_snapshot.as_ref(), | ||
| ))); | ||
| Ok(HotbarDispatch::Handled) |
There was a problem hiding this comment.
Pushing a new view onto the view_stack changes the active modal/view. Set app.needs_redraw = true so the command palette is rendered immediately.
app.view_stack
.push(CommandPaletteView::new(build_command_palette_entries(
app.ui_locale,
&app.skills_dir,
&app.workspace,
&app.mcp_config_path,
app.mcp_snapshot.as_ref(),
)));
app.needs_redraw = true;
Ok(HotbarDispatch::Handled)| app.trust_mode = !app.trust_mode; | ||
| app.status_message = Some(if app.trust_mode { | ||
| "Workspace trust mode enabled.".to_string() | ||
| } else { | ||
| "Workspace trust mode disabled.".to_string() | ||
| }); | ||
| Ok(HotbarDispatch::Handled) |
There was a problem hiding this comment.
Toggling the workspace trust mode and updating the status message requires a redraw to update the UI elements (such as the footer/status bar). Set app.needs_redraw = true before returning.
app.trust_mode = !app.trust_mode;
app.status_message = Some(if app.trust_mode {
"Workspace trust mode enabled.".to_string()
} else {
"Workspace trust mode disabled.".to_string()
});
app.needs_redraw = true;
Ok(HotbarDispatch::Handled)
Summary:
Scope:
Not in this slice:
/hotbarsetup wizard.Issue:
Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist
Greptile Summary
This PR introduces the hotbar action registry foundation for the TUI: a
HotbarActiontrait, aHotbarDispatchresult type, and aHotbarActionRegistrybacked by aBTreeMap<String, Arc<dyn HotbarAction>>. Ten built-in app actions are registered atApp::newviawith_builtins(), and each is covered by unit tests for lookup, active-state reporting, and dispatch behavior.HotbarActionRegistrystores trait objects keyed by stable string IDs;register_builtinsis correctly scoped topub(crate)and the publicregistermethod is kept open for future slash/MCP/skill/plugin adapters.voice.toggle,session.compact,mode.*,reasoning.cycle,sidebar.toggle,filetree.toggle,palette.open,trust.toggle) are implemented with guard logic (e.g., compaction re-entrancy check,auto_modelbail for reasoning cycle).hotbar_actionsis added as a#[allow(dead_code)]field toAppand initialised inApp::new, with the field marked public for the future rendering and key-dispatch layers.Confidence Score: 5/5
Safe to merge — this is a pure foundation layer with no active dispatch wiring yet; all built-in actions are unit-tested and the only integration change is an initialisation call in App::new.
The change adds a new registry struct and 10 built-in action impls that are stored but not yet called from any live code path (the field is
#[allow(dead_code)]). Guard logic for compaction re-entrancy and auto-model reasoning is in place and tested. The only nit is one test unnecessarily declared async.No files require special attention;
actions.rscontains the bulk of the logic and is well covered by unit tests.Important Files Changed
HotbarActiontrait,HotbarDispatchresult,HotbarActionRegistry, and 10 built-in app actions with full unit tests; one test is unnecessarily markedasync/#[tokio::test]when all operations are synchronousHotbarActionRegistryfrom theactionssubmodule with a brief doc comment scoping future hotbar sliceshotbar_actions: HotbarActionRegistryfield toAppstruct (dead_code allowed for now) and initialises it viawith_builtins()inApp::newpub mod hotbardeclaration to expose the new hotbar moduleSequence Diagram
sequenceDiagram participant App participant HotbarActionRegistry participant HotbarAction participant EventLoop Note over App: App::new() App->>HotbarActionRegistry: with_builtins() HotbarActionRegistry->>HotbarActionRegistry: register_builtins() [10 actions] HotbarActionRegistry-->>App: registry stored as hotbar_actions Note over EventLoop: Future: key dispatch (not in this slice) EventLoop->>HotbarActionRegistry: get(slot_id) HotbarActionRegistry-->>EventLoop: "Arc<dyn HotbarAction>" EventLoop->>HotbarAction: "is_active(&app)" HotbarAction-->>EventLoop: bool EventLoop->>HotbarAction: "dispatch(&mut app)" alt Handled HotbarAction-->>EventLoop: Ok(HotbarDispatch::Handled) else AppAction HotbarAction-->>EventLoop: Ok(HotbarDispatch::AppAction(action)) EventLoop->>EventLoop: route AppAction to handler else Error HotbarAction-->>EventLoop: Err(anyhow) endReviews (2): Last reviewed commit: "fix(tui): harden hotbar action dispatch" | Re-trigger Greptile