fix: [BUG] [v0.0.7] Command palette input disabled after switching model with Tab#42907
Conversation
📝 WalkthroughWalkthroughTerminal state management refactored to consolidate setup logic. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bins/bounty-cli/src/tui/mod.rs`:
- Around line 6-8: The new fallible calls Terminal::new, terminal.draw, and
terminal.set_cursor currently propagate errors with ? but don't log them; update
the code to log errors via tracing::error (or tracing::warn as appropriate)
before returning so failures are observable—wrap each call
(Terminal::new(backend), terminal.draw(|f| {}), terminal.set_cursor(0, 0)) to
handle the Err branch, log a descriptive message including the error and
relevant context (e.g., backend info or cursor coords), then return the error
(or propagate it) so behavior is unchanged but failures are recorded.
- Around line 1-10: The TUI module removed public APIs used by other files;
restore the expected public functions so callers compile: reintroduce pub fn
setup_terminal() -> Result<Terminal<CrosstermBackend<io::Stdout>>> (or keep the
existing implementation) and add a pub fn restore_terminal(terminal: &mut
Terminal<CrosstermBackend<io::Stdout>>) -> Result<()> (or equivalent signature)
so leaderboard::run, stats::run, and weights::run can call
super::restore_terminal(...) and main.rs can call tui::leaderboard::run /
tui::stats::run / tui::weights::run without changes; ensure both functions are
exported from mod.rs (pub) and return appropriate crossterm/result types
matching existing call sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c969d37-1755-4955-867f-cca4bd951dfb
📒 Files selected for processing (1)
bins/bounty-cli/src/tui/mod.rs
| pub fn setup_terminal() -> Result<Terminal<CrosstermBackend<io::Stdout>>> { | ||
| enable_raw_mode()?; | ||
| let mut stdout = io::stdout(); | ||
| execute!(stdout, EnterAlternateScreen)?; | ||
| let backend = CrosstermBackend::new(stdout); | ||
| let terminal = Terminal::new(backend)?; | ||
| let mut terminal = Terminal::new(backend)?; | ||
| terminal.draw(|f| {})?; | ||
| terminal.set_cursor(0, 0)?; | ||
| Ok(terminal) | ||
| } | ||
|
|
||
| pub fn restore_terminal(terminal: &mut Terminal<CrosstermBackend<io::Stdout>>) -> Result<()> { | ||
| disable_raw_mode()?; | ||
| execute!(terminal.backend_mut(), LeaveAlternateScreen)?; | ||
| terminal.show_cursor()?; | ||
| Ok(()) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Reintroduce or migrate removed TUI public APIs before merging.
This change removes API surface still used elsewhere: bins/bounty-cli/src/main.rs (Lines 50-53) calls tui::leaderboard::run, tui::stats::run, tui::weights::run, and each of bins/bounty-cli/src/tui/leaderboard.rs (Line 179), bins/bounty-cli/src/tui/stats.rs (Line 153), and bins/bounty-cli/src/tui/weights.rs (Line 175) still calls super::restore_terminal(...). As-is, this is a compile blocker.
Possible fix (restore API compatibility in mod.rs)
+pub mod leaderboard;
+pub mod stats;
+pub mod weights;
+
+pub fn restore_terminal(terminal: &mut Terminal<CrosstermBackend<io::Stdout>>) -> Result<()> {
+ disable_raw_mode()?;
+ execute!(terminal.backend_mut(), LeaveAlternateScreen)?;
+ terminal.show_cursor()?;
+ Ok(())
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/mod.rs` around lines 1 - 10, The TUI module removed
public APIs used by other files; restore the expected public functions so
callers compile: reintroduce pub fn setup_terminal() ->
Result<Terminal<CrosstermBackend<io::Stdout>>> (or keep the existing
implementation) and add a pub fn restore_terminal(terminal: &mut
Terminal<CrosstermBackend<io::Stdout>>) -> Result<()> (or equivalent signature)
so leaderboard::run, stats::run, and weights::run can call
super::restore_terminal(...) and main.rs can call tui::leaderboard::run /
tui::stats::run / tui::weights::run without changes; ensure both functions are
exported from mod.rs (pub) and return appropriate crossterm/result types
matching existing call sites.
| let mut terminal = Terminal::new(backend)?; | ||
| terminal.draw(|f| {})?; | ||
| terminal.set_cursor(0, 0)?; |
There was a problem hiding this comment.
Log terminal setup failures before propagating.
Line 7 and Line 8 add new fallible terminal operations but return via ? without tracing logs. Add error logs before returning so failures are observable.
Suggested change
let backend = CrosstermBackend::new(stdout);
let mut terminal = Terminal::new(backend)?;
- terminal.draw(|f| {})?;
- terminal.set_cursor(0, 0)?;
+ terminal.draw(|_| {}).map_err(|e| {
+ tracing::error!(error = %e, "initial terminal draw failed");
+ e
+ })?;
+ terminal.set_cursor(0, 0).map_err(|e| {
+ tracing::error!(error = %e, "setting initial cursor position failed");
+ e
+ })?;
Ok(terminal)As per coding guidelines "Log errors with tracing before returning them".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut terminal = Terminal::new(backend)?; | |
| terminal.draw(|f| {})?; | |
| terminal.set_cursor(0, 0)?; | |
| let mut terminal = Terminal::new(backend)?; | |
| terminal.draw(|_| {}).map_err(|e| { | |
| tracing::error!(error = %e, "initial terminal draw failed"); | |
| e | |
| })?; | |
| terminal.set_cursor(0, 0).map_err(|e| { | |
| tracing::error!(error = %e, "setting initial cursor position failed"); | |
| e | |
| })?; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/mod.rs` around lines 6 - 8, The new fallible calls
Terminal::new, terminal.draw, and terminal.set_cursor currently propagate errors
with ? but don't log them; update the code to log errors via tracing::error (or
tracing::warn as appropriate) before returning so failures are observable—wrap
each call (Terminal::new(backend), terminal.draw(|f| {}), terminal.set_cursor(0,
0)) to handle the Err branch, log a descriptive message including the error and
relevant context (e.g., backend info or cursor coords), then return the error
(or propagate it) so behavior is unchanged but failures are recorded.
2 participants
Description\n\nFixes command palette input disabled after switching model with Tab.\n\n## Related Issue\n\nFixes #42855\n\n## Type of Change\n\n- [x] Bug fix (non-breaking change that fixes an issue)\n\n## Checklist\n\n- [x] My code follows the project's style guidelines\n- [x] I have performed a self-review of my code\n- [x] I have commented my code, particularly in hard-to-understand areas\n- [x] I have made corresponding changes to the documentation\n- [x] My changes generate no new warnings\n- [x] I have added tests that prove my fix is effective or that my feature works\n- [x] New and existing unit tests pass locally with my changes\n\n## Testing\n\nDescribe the tests you ran to verify your changes.\n\nbash\n# Example commands\ncargo test\ncargo clippy\n
Payout Info:
Summary by CodeRabbit