feat(tui): add Ratatui terminal UI with streaming output#175
feat(tui): add Ratatui terminal UI with streaming output#175
Conversation
Add an opt-in TUI module (--features tui) built on ratatui 0.29, crossterm 0.28, and tui-textarea 0.7. Provides a scrollable output pane, animated spinner, multiline input with slash commands, and async mpsc-based integration via spawn_tui(tx, rx). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds an optional, feature-gated TUI: new Cargo dependencies and a Changes
Sequence DiagramsequenceDiagram
participant User as User
participant TUI as TUI Event Loop
participant Agent as Agent (mpsc)
participant Term as Terminal/Ratatui
User->>TUI: Key event (text / Enter / Esc / scroll)
alt Enter (submit)
TUI->>Agent: tx.try_send(input)
TUI->>TUI: start spinner, clear input
else Esc (cancel)
TUI->>TUI: if spinner active stop spinner
TUI->>Agent: tx.try_send(CANCEL_SENTINEL)
TUI->>TUI: append "[cancelled]"
else Scroll / Edit
TUI->>TUI: adjust scroll_offset or textarea
end
Agent->>TUI: rx has output
TUI->>TUI: drain rx, append lines
TUI->>Term: draw frame (output pane, spinner line, input area)
Term->>User: rendered terminal
TUI->>TUI: sleep/poll (tick)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (4)
src/tui/mod.rs (3)
136-141: Unused method:visible_output_height.This method is defined but never called within the module. If it's intended for future use, consider adding a
#[allow(dead_code)]annotation or removing it until needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/mod.rs` around lines 136 - 141, The method visible_output_height is defined but never used; either remove it or mark it as intentionally unused by adding an #[allow(dead_code)] attribute above the function. Locate the function visible_output_height(&self, area: Rect) in src/tui/mod.rs (it references self.spinner) and either delete the entire function if not needed, or add #[allow(dead_code)] directly above its signature to suppress the unused warning.
192-195: Terminal restoration on panic is not guaranteed.If
run_tuipanics betweenenable_raw_mode()and the cleanup code, the terminal will be left in raw mode. Consider using a guard pattern (e.g., a struct withDropimpl) to ensure terminal state is always restored.💡 Example guard pattern
struct TerminalGuard; impl Drop for TerminalGuard { fn drop(&mut self) { let _ = terminal::disable_raw_mode(); let _ = io::stdout().execute(LeaveAlternateScreen); } } fn run_tui(...) -> io::Result<()> { terminal::enable_raw_mode()?; io::stdout().execute(EnterAlternateScreen)?; let _guard = TerminalGuard; // Will restore on drop, even on panic // ... rest of the function Ok(()) // Guard drops here normally }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/mod.rs` around lines 192 - 195, run_tui currently calls terminal::enable_raw_mode() and executes Enter/LeaveAlternateScreen directly, so if a panic occurs the cleanup code (terminal::disable_raw_mode() and executing LeaveAlternateScreen) may never run; introduce a guard struct (e.g., TerminalGuard) with a Drop impl that calls terminal::disable_raw_mode() and io::stdout().execute(LeaveAlternateScreen) and instantiate it immediately after enable_raw_mode()/EnterAlternateScreen so the terminal is always restored even on panic; ensure the guard is created in run_tui and dropped at function end (or when you intentionally restore earlier).
179-183: Consider usingblocking_recvwith timeout instead oftry_recvin the blocking context.
rx.try_recv()on a tokiompsc::Receiverinsidespawn_blockingwill always returnTryRecvError::Emptyimmediately if no message is available. This works for non-blocking draining, but since you're already blocking onevent::poll(100ms), you may want to userx.blocking_recv()or a timeout-based approach to avoid busy-polling between ticks.The current implementation should function (messages will be picked up on subsequent ticks), but it's less efficient than it could be.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/mod.rs` around lines 179 - 183, The current non-blocking drain using rx.try_recv() inside the spawn_blocking loop causes busy-polling between event::poll(100ms) ticks; change the drain to use a blocking receive with a timeout (e.g., rx.blocking_recv() or a recv-with-timeout variant) so the thread blocks until a message or timeout instead of spinning. Locate the loop that uses rx.try_recv(), replace it with a blocking receive pattern that respects a short timeout aligned with event::poll(100ms), and then call app.spinner = None; app.push_output(line) for each received line (same behavior as before) so the spinner/output handling (app.push_output) remains unchanged.src/tui/widgets.rs (1)
14-28: Consider makingSpinnercrate-private andnew()aconst fn.
Spinnerappears to be an internal implementation detail. Considerpub(crate) struct Spinnerto limit the public API surface. Additionally,new()could beconst fnsince it only assigns static data.♻️ Optional refinement
-pub struct Spinner { +pub(crate) struct Spinner { pub frames: &'static [&'static str], } impl Spinner { - pub fn new() -> Self { + pub const fn new() -> Self { Self { frames: SPINNER_FRAMES, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/widgets.rs` around lines 14 - 28, Make Spinner crate-private and its constructor a const fn: change `pub struct Spinner` to `pub(crate) struct Spinner`, remove the `pub` on the `frames` field so it becomes private (e.g. `frames: &'static [&'static str],`), and change `pub fn new() -> Self` to `pub(crate) const fn new() -> Self`; keep `frame(&self, tick: usize)` accessible at the same (crate or public) visibility you need (e.g. `pub(crate) fn frame(...)`) so callers inside the crate still work, and keep using `SPINNER_FRAMES` as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tui/theme.rs`:
- Around line 6-19: The functions style(), bold(), and dim() are marked const
but call non-const methods (Style::bg and add_modifier) which breaks
compilation; change their signatures to non-const (pub fn style(), pub fn
bold(), pub fn dim()) so they can call Style::bg() and .add_modifier(), or
alternatively remove the bg() and .add_modifier() calls and keep them const by
limiting to const-safe APIs (Style::new() and .fg()); keep border() as-is since
it only uses const-safe methods.
---
Nitpick comments:
In `@src/tui/mod.rs`:
- Around line 136-141: The method visible_output_height is defined but never
used; either remove it or mark it as intentionally unused by adding an
#[allow(dead_code)] attribute above the function. Locate the function
visible_output_height(&self, area: Rect) in src/tui/mod.rs (it references
self.spinner) and either delete the entire function if not needed, or add
#[allow(dead_code)] directly above its signature to suppress the unused warning.
- Around line 192-195: run_tui currently calls terminal::enable_raw_mode() and
executes Enter/LeaveAlternateScreen directly, so if a panic occurs the cleanup
code (terminal::disable_raw_mode() and executing LeaveAlternateScreen) may never
run; introduce a guard struct (e.g., TerminalGuard) with a Drop impl that calls
terminal::disable_raw_mode() and io::stdout().execute(LeaveAlternateScreen) and
instantiate it immediately after enable_raw_mode()/EnterAlternateScreen so the
terminal is always restored even on panic; ensure the guard is created in
run_tui and dropped at function end (or when you intentionally restore earlier).
- Around line 179-183: The current non-blocking drain using rx.try_recv() inside
the spawn_blocking loop causes busy-polling between event::poll(100ms) ticks;
change the drain to use a blocking receive with a timeout (e.g.,
rx.blocking_recv() or a recv-with-timeout variant) so the thread blocks until a
message or timeout instead of spinning. Locate the loop that uses rx.try_recv(),
replace it with a blocking receive pattern that respects a short timeout aligned
with event::poll(100ms), and then call app.spinner = None; app.push_output(line)
for each received line (same behavior as before) so the spinner/output handling
(app.push_output) remains unchanged.
In `@src/tui/widgets.rs`:
- Around line 14-28: Make Spinner crate-private and its constructor a const fn:
change `pub struct Spinner` to `pub(crate) struct Spinner`, remove the `pub` on
the `frames` field so it becomes private (e.g. `frames: &'static [&'static
str],`), and change `pub fn new() -> Self` to `pub(crate) const fn new() ->
Self`; keep `frame(&self, tick: usize)` accessible at the same (crate or public)
visibility you need (e.g. `pub(crate) fn frame(...)`) so callers inside the
crate still work, and keep using `SPINNER_FRAMES` as before.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04e1cbe9-2f54-4e23-8edc-142d254c3588
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (5)
Cargo.tomlsrc/lib.rssrc/tui/mod.rssrc/tui/theme.rssrc/tui/widgets.rs
| pub const fn style() -> Style { | ||
| Style::new().fg(PRIMARY).bg(BG) | ||
| } | ||
|
|
||
| pub const fn bold() -> Style { | ||
| style().add_modifier(Modifier::BOLD) | ||
| } | ||
|
|
||
| pub const fn border() -> Style { | ||
| Style::new().fg(PRIMARY) | ||
| } | ||
|
|
||
| pub const fn dim() -> Style { | ||
| Style::new().fg(PRIMARY).add_modifier(Modifier::DIM) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ratatui 0.29 const fn Style methods
💡 Result:
In ratatui 0.29.0, the Style struct in ratatui::style provides the following const fn methods: pub const fn new -> Style Returns a Style with default properties. pub const fn reset -> Style Returns a Style resetting all properties. pub const fn fg(self, color: Color) -> Style Changes the foreground color. Other methods like bg(self, color: Color) -> Style and add_modifier(self, modifier: Modifier) -> Style exist but are not explicitly marked as const fn in the 0.29.0 documentation. The const fn designation for new and reset enables their use in const contexts. Note: Stylize shorthand methods (e.g., .red, .bold) were implemented directly as const fn on Style in v0.30.0 via PR #1572, enabling const MY_STYLE = Style::new.blue.on_black;. In 0.29.0, these shorthands require the Stylize trait and are not direct const methods on Style.
Citations:
- 1: https://docs.rs/ratatui/latest/ratatui/style/struct.Style.html
- 2: https://docs.rs/ratatui/latest/ratatui/style/struct.Modifier.html
- 3: feat!: implement stylize methods directly on Style ratatui/ratatui#1572
- 4: https://docs.rs/crate/ratatui/^0.29
Remove const fn or refactor to use non-const methods.
In ratatui 0.29.0, Style::bg() and add_modifier() are not const fn. The following functions will fail to compile:
style(): uses.bg()(not const)bold(): uses.add_modifier()(not const)dim(): uses.add_modifier()(not const)
Either remove the const fn qualifier from these functions or restructure them to use only const-compatible Style methods (Style::new(), .fg()).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tui/theme.rs` around lines 6 - 19, The functions style(), bold(), and
dim() are marked const but call non-const methods (Style::bg and add_modifier)
which breaks compilation; change their signatures to non-const (pub fn style(),
pub fn bold(), pub fn dim()) so they can call Style::bg() and .add_modifier(),
or alternatively remove the bg() and .add_modifier() calls and keep them const
by limiting to const-safe APIs (Style::new() and .fg()); keep border() as-is
since it only uses const-safe methods.
- Add TerminalGuard with Drop impl for panic-safe terminal restoration - Remove unused visible_output_height method - Tighten Spinner/SpinnerState/render fns to pub(crate) visibility - Make Spinner::new() const fn Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tui/mod.rs`:
- Around line 117-120: The code currently ignores the Result from tx.try_send in
both the input path (where self.spinner = Some(SpinnerState::new("pondering")))
and the cancel path, which can leave UI state (self.spinner / cancel status)
inconsistent if the channel is Full or Closed; change both sites to match on
tx.try_send(...) and handle Err variants: on Ok(_) proceed as now; on
Err(TrySendError::Full) revert the UI change (set self.spinner = None or clear
the cancel flag) and update/log an appropriate message so the spinner/cancel
state reflects that the message was not delivered; on Err(TrySendError::Closed)
also clear UI state and log/propagate an error so callers can handle it. Ensure
you reference SpinnerState::new, self.spinner, and tx.try_send in both
locations.
- Around line 162-164: Create the TerminalGuard immediately after successfully
calling terminal::enable_raw_mode() so raw mode is always guarded even if
subsequent calls (like io::stdout().execute(EnterAlternateScreen)) fail; move
the creation of TerminalGuard to directly follow enable_raw_mode() (i.e., call
enable_raw_mode(), then let _guard = TerminalGuard, then execute
EnterAlternateScreen) so the guard's Drop will reliably restore terminal state.
- Around line 131-134: The bottom-anchoring logic in scroll_to_bottom() is wrong
because it uses total.saturating_sub(1) instead of accounting for visible
height; change scroll_to_bottom(&mut self) to accept visible_height: u16 and
compute scroll_offset = total.saturating_sub(visible_height).max(0) (i.e., use
total.saturating_sub(visible_height) bounded to zero) so
Paragraph::scroll((y,x)) pins the last visible lines; update all callers (e.g.,
render_output_pane and the auto-scroll re-enable logic) to pass area.height
(converted to u16) and adjust any signature/usage accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a7ac849-fffd-42d4-be24-816bb96f3ed1
📒 Files selected for processing (2)
src/tui/mod.rssrc/tui/widgets.rs
| self.spinner = Some(SpinnerState::new("pondering")); | ||
| // Non-blocking send; if the channel is full we drop the message | ||
| let _ = tx.try_send(text); | ||
| } |
There was a problem hiding this comment.
Do not ignore try_send failures for input/cancel paths.
Line 119 and Line 209 drop errors silently. If the channel is full/closed, UI state diverges (spinner/cancel status shown even when message wasn’t delivered).
Suggested fix
_ => {
self.push_output(format!("> {text}"));
- self.spinner = Some(SpinnerState::new("pondering"));
- // Non-blocking send; if the channel is full we drop the message
- let _ = tx.try_send(text);
+ match tx.try_send(text) {
+ Ok(()) => {
+ self.spinner = Some(SpinnerState::new("pondering"));
+ }
+ Err(mpsc::error::TrySendError::Full(_)) => {
+ self.push_output("[send failed: input channel full]".into());
+ }
+ Err(mpsc::error::TrySendError::Closed(_)) => {
+ self.push_output("[send failed: agent disconnected]".into());
+ }
+ }
} KeyCode::Esc => {
if app.spinner.is_some() {
app.spinner = None;
- let _ = tx.try_send(CANCEL_SENTINEL.to_string());
- app.push_output("[cancelled]".into());
+ match tx.try_send(CANCEL_SENTINEL.to_string()) {
+ Ok(()) => app.push_output("[cancelled]".into()),
+ Err(_) => app.push_output("[cancel failed: agent disconnected]".into()),
+ }
}
false
}Also applies to: 207-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tui/mod.rs` around lines 117 - 120, The code currently ignores the Result
from tx.try_send in both the input path (where self.spinner =
Some(SpinnerState::new("pondering"))) and the cancel path, which can leave UI
state (self.spinner / cancel status) inconsistent if the channel is Full or
Closed; change both sites to match on tx.try_send(...) and handle Err variants:
on Ok(_) proceed as now; on Err(TrySendError::Full) revert the UI change (set
self.spinner = None or clear the cancel flag) and update/log an appropriate
message so the spinner/cancel state reflects that the message was not delivered;
on Err(TrySendError::Closed) also clear UI state and log/propagate an error so
callers can handle it. Ensure you reference SpinnerState::new, self.spinner, and
tx.try_send in both locations.
| fn scroll_to_bottom(&mut self) { | ||
| let total = u16::try_from(self.output.len()).unwrap_or(u16::MAX); | ||
| self.scroll_offset = total.saturating_sub(1); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In ratatui 0.29, for Paragraph::scroll((y, x)), is y the number of lines skipped from the top? What is the correct formula to keep the view pinned to the bottom given total lines and visible content height?
💡 Result:
Yes, in ratatui 0.29, for Paragraph::scroll((y, x)), y is the number of lines skipped from the top of the wrapped and aligned text. To keep the view pinned to the bottom given total lines T (computed via paragraph.line_count(area_width)) and visible content height H (area.height after block margins), use scroll((max(0, T as u16.saturating_sub(H)), 0)). This skips all but the last H lines, showing the bottom. Note: line_count requires the "unstable-rendered-line-info" feature.
Citations:
- 1: https://docs.rs/ratatui/latest/ratatui/widgets/struct.Paragraph.html
- 2: https://github.com/ratatui/ratatui/blob/2c8d28da/ratatui-widgets/src/paragraph.rs
- 3: https://ratatui.rs/examples/widgets/scrollbar/
- 4: https://www.ratatui-ruby.dev/docs/v0.8/examples/widget_scroll_text/app_rb.html
🏁 Script executed:
# First, locate the file and inspect the scroll_to_bottom function and surrounding context
fd -t f "mod.rs" -path "*tui*" | head -20Repository: 5queezer/hrafn
Length of output: 226
🏁 Script executed:
# Read the specific lines mentioned in the review comment
if [ -f "src/tui/mod.rs" ]; then
echo "=== Lines 131-140 (scroll_to_bottom function) ===" && \
sed -n '131,140p' src/tui/mod.rs && \
echo "" && \
echo "=== Lines 220-230 (also applies to) ===" && \
sed -n '220,230p' src/tui/mod.rs
else
find . -name "mod.rs" -path "*tui*" -type f
fiRepository: 5queezer/hrafn
Length of output: 995
🏁 Script executed:
# Get broader context around scroll_to_bottom to understand where visible_height might come from
if [ -f "src/tui/mod.rs" ]; then
sed -n '100,150p' src/tui/mod.rs
fiRepository: 5queezer/hrafn
Length of output: 1849
🏁 Script executed:
# Check for struct definition and parameters to understand available context
if [ -f "src/tui/mod.rs" ]; then
rg -n "struct|fn scroll_to_bottom|fn render|auto_scroll" src/tui/mod.rs | head -30
fiRepository: 5queezer/hrafn
Length of output: 323
🏁 Script executed:
# Find where scroll_offset is used in the render/drawing code
if [ -f "src/tui/mod.rs" ]; then
rg -n "scroll_offset|Paragraph|render" src/tui/mod.rs | head -40
fiRepository: 5queezer/hrafn
Length of output: 738
🏁 Script executed:
# Look for the main render/draw function to understand the widget setup
if [ -f "src/tui/mod.rs" ]; then
sed -n '250,350p' src/tui/mod.rs
fiRepository: 5queezer/hrafn
Length of output: 40
🏁 Script executed:
# Search for imports to confirm ratatui usage and version
if [ -f "src/tui/mod.rs" ]; then
head -30 src/tui/mod.rs
fiRepository: 5queezer/hrafn
Length of output: 791
🏁 Script executed:
# Find the widgets module
fd -t f "widgets.rs" -o "widgets/mod.rs"Repository: 5queezer/hrafn
Length of output: 205
🏁 Script executed:
# Search for render_output_pane function
rg -n "fn render_output_pane" --type rustRepository: 5queezer/hrafn
Length of output: 184
🏁 Script executed:
# Get the widgets module content
if [ -f "src/tui/widgets.rs" ]; then
wc -l src/tui/widgets.rs
fiRepository: 5queezer/hrafn
Length of output: 79
🏁 Script executed:
# If it's a module directory, find the files
fd . "src/tui/widgets" -type f 2>/dev/null || echo "Not a directory"Repository: 5queezer/hrafn
Length of output: 73
🏁 Script executed:
# Read the render_output_pane function
sed -n '48,82p' src/tui/widgets.rsRepository: 5queezer/hrafn
Length of output: 1133
🏁 Script executed:
# Get more context around it
sed -n '40,82p' src/tui/widgets.rsRepository: 5queezer/hrafn
Length of output: 1249
Scroll offset calculation for bottom-anchoring is incorrect and needs visible height.
The current implementation using total.saturating_sub(1) (lines 133 and 223–224) does not account for the visible pane height. For ratatui's Paragraph::scroll((y, x)) where y is lines skipped from the top, the correct formula to keep the view pinned to the bottom is scroll((max(0, total.saturating_sub(visible_height)), 0)). Using total - 1 causes the paragraph to skip past the visible area instead of anchoring to the last visible lines.
This requires passing the output pane's visible height (available as area.height in render_output_pane) to scroll_to_bottom() and the auto-scroll re-enable logic. Without this fix, wrapped text at the bottom will be hidden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tui/mod.rs` around lines 131 - 134, The bottom-anchoring logic in
scroll_to_bottom() is wrong because it uses total.saturating_sub(1) instead of
accounting for visible height; change scroll_to_bottom(&mut self) to accept
visible_height: u16 and compute scroll_offset =
total.saturating_sub(visible_height).max(0) (i.e., use
total.saturating_sub(visible_height) bounded to zero) so
Paragraph::scroll((y,x)) pins the last visible lines; update all callers (e.g.,
render_output_pane and the auto-scroll re-enable logic) to pass area.height
(converted to u16) and adjust any signature/usage accordingly.
| terminal::enable_raw_mode()?; | ||
| io::stdout().execute(EnterAlternateScreen)?; | ||
| let _guard = TerminalGuard; |
There was a problem hiding this comment.
Create TerminalGuard immediately after enabling raw mode.
Line 162 enables raw mode, but the guard is only created at Line 164. If Line 163 fails, raw mode may remain enabled.
Suggested fix
fn run_tui(tx: mpsc::Sender<String>, rx: &mut mpsc::Receiver<String>) -> io::Result<()> {
terminal::enable_raw_mode()?;
+ let _guard = TerminalGuard;
io::stdout().execute(EnterAlternateScreen)?;
- let _guard = TerminalGuard;📝 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.
| terminal::enable_raw_mode()?; | |
| io::stdout().execute(EnterAlternateScreen)?; | |
| let _guard = TerminalGuard; | |
| terminal::enable_raw_mode()?; | |
| let _guard = TerminalGuard; | |
| io::stdout().execute(EnterAlternateScreen)?; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tui/mod.rs` around lines 162 - 164, Create the TerminalGuard immediately
after successfully calling terminal::enable_raw_mode() so raw mode is always
guarded even if subsequent calls (like
io::stdout().execute(EnterAlternateScreen)) fail; move the creation of
TerminalGuard to directly follow enable_raw_mode() (i.e., call
enable_raw_mode(), then let _guard = TerminalGuard, then execute
EnterAlternateScreen) so the guard's Drop will reliably restore terminal state.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/tui/widgets.rs (2)
80-85: Avoid reconstructingSpinneron every frame draw.
Spinner::new()is cheap, but this runs in the hot render path. A shared const/static instance keeps intent clearer and avoids repeated construction.Diff suggestion
const SPINNER_FRAMES: &[&str] = &["\u{25DC}", "\u{25DD}", "\u{25DE}", "\u{25DF}"]; +const SPINNER: Spinner = Spinner::new(); @@ - let spinner = Spinner::new(); let elapsed = state.elapsed_secs(); let text = format!( "{} {}... ({:.1}s)", - spinner.frame(tick), + SPINNER.frame(tick), state.label, elapsed );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/widgets.rs` around lines 80 - 85, Replace the per-render construction of Spinner (currently calling Spinner::new() before every draw and then using spinner.frame(tick)) with a shared static instance so the hot render path doesn't reconstruct it; create a single static/spawn-once Spinner (e.g., with once_cell or lazy_static) and call its frame(tick) where you currently call spinner.frame(tick), keeping usage of state.elapsed_secs() and state.label unchanged.
60-69: Remove redundant style application in output rendering.Line styles are set on each
Line(Line 62) and again on theParagraph(Line 69). Keeping one source of style avoids ambiguity and a bit of per-frame overhead.Diff suggestion
- let lines: Vec<Line> = output + let lines: Vec<Line> = output .iter() - .map(|s| Line::from(s.as_str()).style(theme::style())) + .map(|s| Line::from(s.as_str())) .collect(); let paragraph = Paragraph::new(lines) .block(block) .wrap(Wrap { trim: false }) .scroll((scroll_offset, 0)) .style(theme::style());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tui/widgets.rs` around lines 60 - 69, The paragraph-level style is redundant because each Line is already styled via Line::from(...).style(theme::style()) when building lines; remove the paragraph .style(theme::style()) call on the Paragraph built from Paragraph::new(lines) (i.e., drop the final .style(theme::style()) on the paragraph) so styling is applied only on the Line instances and avoid duplicate per-frame styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/tui/widgets.rs`:
- Around line 80-85: Replace the per-render construction of Spinner (currently
calling Spinner::new() before every draw and then using spinner.frame(tick))
with a shared static instance so the hot render path doesn't reconstruct it;
create a single static/spawn-once Spinner (e.g., with once_cell or lazy_static)
and call its frame(tick) where you currently call spinner.frame(tick), keeping
usage of state.elapsed_secs() and state.label unchanged.
- Around line 60-69: The paragraph-level style is redundant because each Line is
already styled via Line::from(...).style(theme::style()) when building lines;
remove the paragraph .style(theme::style()) call on the Paragraph built from
Paragraph::new(lines) (i.e., drop the final .style(theme::style()) on the
paragraph) so styling is applied only on the Line instances and avoid duplicate
per-frame styling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b0d8e37-76bb-416f-b2fa-606a203346b8
📒 Files selected for processing (1)
src/tui/widgets.rs
1 participant
Summary
tuifeature flag with ratatui 0.29, crossterm 0.28, and tui-textarea 0.7◜◝◞◟+ elapsed time), and multiline inputpub fn spawn_tui(tx: Sender<String>, rx: Receiver<String>) -> JoinHandle<()>for async integration/quit,/clear,/helpintercepted locally; ESC sends cancel sentinelFile layout
src/tui/mod.rsspawn_tuientry pointsrc/tui/widgets.rssrc/tui/theme.rsColor::Rgb(0, 255, 70)constants + style helpersTest plan
cargo check --features tuipassescargo clippy --features tui -- -D warningscleancargo run --features tuidisplays layout, spinner animates, input submits/quitexits🤖 Generated with Claude Code
Summary by CodeRabbit
tuifeature flag.