feat(repl): add persistent input prompt during LLM processing#168
feat(repl): add persistent input prompt during LLM processing#168dobesv wants to merge 2 commits into
Conversation
Allow users to type their next message while the LLM is processing. Keystrokes are captured during spinner and stream rendering, shown as a mini input line below the spinner. Pressing Enter enqueues the message (shown with ⏳ indicator), and it is sent at the next opportunity. Editing a queued message un-queues it until Enter is pressed again. At most one message can be queued at a time. New file: src/repl/input_queue.rs - InputQueue shared state type Modified: abort_signal, spinner, stream renderer, render_stream, call_chat_completions, call_chat_completions_streaming, Repl struct, ask/ask_inner chain, run_repl_command, config/mod.rs macro execution.
📝 WalkthroughWalkthroughA persistent input prompt is added: an Changes
Sequence DiagramsequenceDiagram
actor User
participant REPL
participant ChatCompletion
participant Spinner
participant InputQueue
participant AbortPoll
User->>REPL: Enter command
REPL->>ChatCompletion: call_chat_completions(..., input_queue)
ChatCompletion->>Spinner: abortable_run_with_spinner_with_input(..., input_queue)
par LLM processing
ChatCompletion->>ChatCompletion: Await LLM response
and User typing
User->>AbortPoll: Key events
AbortPoll->>InputQueue: forward non-abort keys via poll_abort_signal_with_input
InputQueue->>InputQueue: update buffer / enqueue on Enter
Spinner->>InputQueue: get_display_line()
InputQueue-->>Spinner: display ">" or "⏳ ..." line
Spinner->>Spinner: render spinner + input line
end
ChatCompletion->>Spinner: LLM completes -> stop spinner
Spinner->>REPL: spinner stopped
REPL->>InputQueue: dequeue()
InputQueue-->>REPL: return queued message (if any)
REPL->>REPL: execute queued command
REPL->>User: show results
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 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)
⚔️ Resolve merge conflicts
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/render/stream.rs (1)
46-77:⚠️ Potential issue | 🟠 Major
raw_streambypasses queued input entirely whenhighlight = false.
_input_queueis unused here, and this loop blocks onrx.recv().awaitwithout ever pollingpoll_abort_signal_with_input(...)or drawing the mini prompt. Terminal users who disable highlighting lose the new queued-input flow completely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/render/stream.rs` around lines 46 - 77, raw_stream currently ignores _input_queue and blocks on rx.recv().await, so when highlighting is disabled queued input and the mini-prompt are never polled; change the receive loop in raw_stream to concurrently poll both the SSE channel and the abort/input helper by using poll_abort_signal_with_input(abort_signal, _input_queue, /*...*/) (or an equivalent select! between rx.recv() and poll_abort_signal_with_input) so you still handle input/abort events while waiting for SseEvent; keep the existing spinner logic (spawn_spinner/stop) and still process SseEvent::Text and SseEvent::Done as before, but ensure poll_abort_signal_with_input is invoked each iteration so queued input and the mini prompt are drawn when highlight=false.src/config/mod.rs (1)
2821-2833:⚠️ Potential issue | 🟠 MajorThis macro-scoped queue is never drained or handed back to the REPL.
InputQueuestate persists untildequeue()/clear(), but this function reuses one local queue across all steps and then drops it. Unlike the main REPL loop, there is no post-command drain here, so a message queued during step N can bleed into step N+1, and any leftover queued input is lost when the macro returns.src/repl/mod.rs (1)
266-268:⚠️ Potential issue | 🟠 MajorDrain queued commands after every
ask, and propagate exit requests.
process_pending_async_resume()can capture queued input too, but Line 267continues before this drain runs, so those follow-ups sit until some later manual submit. Inside the drain loop,Ok(true)only breaks the innerwhile, and Line 296 resets a pending Ctrl+D before the outer loop sees it. Please move this into a shared drain helper that returnsshould_exitand call it after everyaskpath.Also applies to: 294-319, 359-369
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repl/mod.rs` around lines 266 - 268, Extract the queued-command draining logic from process_pending_async_resume() into a shared helper (e.g., drain_queued_commands_or_exit()) that iterates the queued inputs, returns a bool should_exit (true if an exit/Ctrl+D was requested) and does not clear/reset the pending-Ctrl+D flag until the outer loop has a chance to act; replace the in-place drain loops and the await call to process_pending_async_resume() in the ask paths with calls to this helper so that after every ask branch you call drain_queued_commands_or_exit() and immediately propagate its should_exit result to break the outer loop; update the call sites that currently duplicate the drain (the blocks covering the existing ranges around process_pending_async_resume() and the other drains) to use this single helper so exit requests are consistently observed and queued follow-ups are handled immediately.
🧹 Nitpick comments (1)
src/client/common.rs (1)
3-8: Keep REPL state out of the client layer.These helpers now depend on
crate::repl::input_queue::InputQueue, which makessrc/clientknow about a REPL-only type and leaks that concern into generic chat-completion entry points. Non-REPL callers likesrc/main.rsalready have to thread an extra parameter that can only ever beNone. Moving the queue abstraction intosrc/renderorsrc/utilswould keep the client boundary transport-focused.Also applies to: 567-573, 625-629
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/common.rs` around lines 3 - 8, The client layer imports and references the REPL-specific type InputQueue, leaking REPL concern into src/client; refactor by removing crate::repl::input_queue::InputQueue usage from client helpers and instead depend on a transport-agnostic queue abstraction placed under src/render or src/utils (e.g., define a trait like RenderQueue or use an existing utils queue type). Update all function signatures in the client that currently take or reference InputQueue to accept the new abstraction (or an Option thereof), replace the import of InputQueue with the new module, and adjust call sites (including the other occurrences around the 567-573 and 625-629 usages) so non-REPL callers can pass None or a simple noop implementation without pulling in REPL types. Ensure symbols to change include the imported InputQueue and any client helper functions that consume it so the client layer no longer directly depends on crate::repl.
🤖 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/render/stream.rs`:
- Around line 86-87: The streaming path consumes the input queue via
poll_abort_signal_with_input(...) but never renders the draft, so users type
blind; update the markdown streaming flow to include
input_queue.get_display_line() in the render output (either by having
poll_abort_signal_with_input return or call the display string, or by calling
input_queue.get_display_line() inside the streaming render loop) so the queued
draft (e.g., the "⏳" state) is written to the terminal/UI; look for symbols
poll_abort_signal_with_input, input_queue, and get_display_line and ensure the
streaming writer/loop that handles the markdown stream also appends or writes
input_queue.get_display_line() (same fix needed in the other occurrence around
the second block mentioned).
In `@src/repl/mod.rs`:
- Around line 294-319: The loop that drains queued lines uses
self.input_queue.dequeue() but never restores the live draft buffer kept by the
input queue, so any typing/editing that wasn't Entered is lost; before executing
each queued message (i.e., before calling run_repl_command and before the next
read_line), fetch/consume the current draft buffer from self.input_queue and
load it back into the Reedline editor (or the component that supplies read_line)
so the prompt remains persistent—use the input queue API that exposes the draft
(e.g., a method like take_buffer() or get_buffer()) and call the Reedline/Editor
method to set the current buffer (or enqueue it back into the editor) so edits
survive queued-message execution; ensure this is done both when draining via
self.input_queue.dequeue() and when a queued message is edited back into draft
state.
In `@src/utils/abort_signal.rs`:
- Line 2: Import KeyEventKind alongside Event, KeyCode, KeyEvent, KeyModifiers
in the module and update poll_abort_signal_inner to ignore KeyEventKind::Release
events: after reading an event (event::read or the variable returned by
event::read()) check if it is an Event::Key(k) and that k.kind is either
KeyEventKind::Press or KeyEventKind::Repeat before dispatching to abort or input
handling. Ensure the updated import includes KeyEventKind and the filter
references KeyEventKind, KeyEvent, and the poll_abort_signal_inner function to
locate the change.
In `@src/utils/spinner.rs`:
- Around line 188-199: The spinner path currently polls keyboard events without
enabling raw mode; update abortable_run_with_spinner_with_input (and the called
poll_abort_signal_with_input/poll_abort_signal code paths) to enable crossterm
raw mode before starting the spinner/keyboard-poll loop and disable raw mode
afterwards (ensuring disable_raw_mode runs on all exits/errors, e.g., via a drop
guard or finally-like cleanup). Specifically, call terminal::enable_raw_mode()
before creating/entering the spinner loop that uses event::poll()/event::read(),
and call terminal::disable_raw_mode() when the spinner finishes or on early
return/error to restore terminal state; propagate or log any errors from
enable/disable and ensure input_queue handling remains unchanged.
---
Outside diff comments:
In `@src/render/stream.rs`:
- Around line 46-77: raw_stream currently ignores _input_queue and blocks on
rx.recv().await, so when highlighting is disabled queued input and the
mini-prompt are never polled; change the receive loop in raw_stream to
concurrently poll both the SSE channel and the abort/input helper by using
poll_abort_signal_with_input(abort_signal, _input_queue, /*...*/) (or an
equivalent select! between rx.recv() and poll_abort_signal_with_input) so you
still handle input/abort events while waiting for SseEvent; keep the existing
spinner logic (spawn_spinner/stop) and still process SseEvent::Text and
SseEvent::Done as before, but ensure poll_abort_signal_with_input is invoked
each iteration so queued input and the mini prompt are drawn when
highlight=false.
In `@src/repl/mod.rs`:
- Around line 266-268: Extract the queued-command draining logic from
process_pending_async_resume() into a shared helper (e.g.,
drain_queued_commands_or_exit()) that iterates the queued inputs, returns a bool
should_exit (true if an exit/Ctrl+D was requested) and does not clear/reset the
pending-Ctrl+D flag until the outer loop has a chance to act; replace the
in-place drain loops and the await call to process_pending_async_resume() in the
ask paths with calls to this helper so that after every ask branch you call
drain_queued_commands_or_exit() and immediately propagate its should_exit result
to break the outer loop; update the call sites that currently duplicate the
drain (the blocks covering the existing ranges around
process_pending_async_resume() and the other drains) to use this single helper
so exit requests are consistently observed and queued follow-ups are handled
immediately.
---
Nitpick comments:
In `@src/client/common.rs`:
- Around line 3-8: The client layer imports and references the REPL-specific
type InputQueue, leaking REPL concern into src/client; refactor by removing
crate::repl::input_queue::InputQueue usage from client helpers and instead
depend on a transport-agnostic queue abstraction placed under src/render or
src/utils (e.g., define a trait like RenderQueue or use an existing utils queue
type). Update all function signatures in the client that currently take or
reference InputQueue to accept the new abstraction (or an Option thereof),
replace the import of InputQueue with the new module, and adjust call sites
(including the other occurrences around the 567-573 and 625-629 usages) so
non-REPL callers can pass None or a simple noop implementation without pulling
in REPL types. Ensure symbols to change include the imported InputQueue and any
client helper functions that consume it so the client layer no longer directly
depends on crate::repl.
🪄 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: 3cbdac6b-d299-4ac1-8a08-21dc8e808b1b
📒 Files selected for processing (11)
.changesets/persistent-input-prompt.mdsrc/client/common.rssrc/config/mod.rssrc/main.rssrc/rag/mod.rssrc/render/mod.rssrc/render/stream.rssrc/repl/input_queue.rssrc/repl/mod.rssrc/utils/abort_signal.rssrc/utils/spinner.rs
| // Drain any queued message from the input queue | ||
| while let Some(queued_line) = self.input_queue.dequeue() { | ||
| self.abort_signal.reset(); | ||
| println!("{}", dimmed_text(&format!(">> {queued_line}"))); | ||
| match run_repl_command( | ||
| &self.config, | ||
| self.abort_signal.clone(), | ||
| &queued_line, | ||
| &mut self.async_manager, | ||
| &self.persistent_manager, | ||
| &mut self.pending_async_context, | ||
| &self.input_queue, | ||
| ) | ||
| .await | ||
| { | ||
| Ok(exit) => { | ||
| if exit { | ||
| break; | ||
| } | ||
| } | ||
| Err(err) => { | ||
| render_error(err); | ||
| println!() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Draft input disappears unless Enter was pressed.
This path only consumes queued_message via dequeue(). In src/repl/input_queue.rs, the queue also keeps the live draft buffer, so typing during generation without pressing Enter—or editing a queued message back into draft state—never makes it back into Reedline before the next read_line(). The prompt is not actually persistent until that draft buffer is handed off to the editor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repl/mod.rs` around lines 294 - 319, The loop that drains queued lines
uses self.input_queue.dequeue() but never restores the live draft buffer kept by
the input queue, so any typing/editing that wasn't Entered is lost; before
executing each queued message (i.e., before calling run_repl_command and before
the next read_line), fetch/consume the current draft buffer from
self.input_queue and load it back into the Reedline editor (or the component
that supplies read_line) so the prompt remains persistent—use the input queue
API that exposes the draft (e.g., a method like take_buffer() or get_buffer())
and call the Reedline/Editor method to set the current buffer (or enqueue it
back into the editor) so edits survive queued-message execution; ensure this is
done both when draining via self.input_queue.dequeue() and when a queued message
is edited back into draft state.
| @@ -1,5 +1,5 @@ | |||
| use anyhow::Result; | |||
| use crossterm::event::{self, Event, KeyCode, KeyModifiers}; | |||
| use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyModifiers}; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For crossterm 0.28.x, does event::read()emitKeyEventKind::Release/Repeaton Windows, and should CLI key handlers usually ignoreRelease events?
💡 Result:
Yes, for crossterm 0.28.x on Windows, event::read emits KeyEventKind::Release events (and Repeat events for held keys). CLI key handlers should usually ignore Release events (and often Repeat unless handling key repeat is desired), matching only on KeyEventKind::Press to avoid duplicate processing.
Citations:
- 1: https://docs.rs/crossterm/latest/crossterm/event/struct.KeyEvent.html
- 2: Receiving release
Entercommand upon application start. crossterm-rs/crossterm#752 - 3: Key press event fires twice crossterm-rs/crossterm#797
- 4: Double key input on Windows crossterm-rs/crossterm#772
- 5: https://github.com/crossterm-rs/crossterm/blob/master/src/event.rs
🏁 Script executed:
cat -n src/utils/abort_signal.rs | head -150Repository: dobesv/harnx
Length of output: 3549
Import KeyEventKind and filter out release events before dispatching terminal input.
event::read() is forwarded directly into abort and input handling without checking the event kind. On platforms where crossterm emits release events (including Windows), a single key press becomes two events—one for press and one for release—causing duplicate abort triggers or duplicate input characters.
Update the import on line 2 to include KeyEventKind, then add a kind filter in poll_abort_signal_inner (around line 89) to process only Press and Repeat events:
Suggested change
-use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyModifiers};
+use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers};In poll_abort_signal_inner (line 89):
if let Event::Key(key) = event::read()? {
+ if !matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) {
+ return Ok(false);
+ }
if handle_abort_key(abort_signal, &key) {📝 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.
| use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyModifiers}; | |
| use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/abort_signal.rs` at line 2, Import KeyEventKind alongside Event,
KeyCode, KeyEvent, KeyModifiers in the module and update poll_abort_signal_inner
to ignore KeyEventKind::Release events: after reading an event (event::read or
the variable returned by event::read()) check if it is an Event::Key(k) and that
k.kind is either KeyEventKind::Press or KeyEventKind::Repeat before dispatching
to abort or input handling. Ensure the updated import includes KeyEventKind and
the filter references KeyEventKind, KeyEvent, and the poll_abort_signal_inner
function to locate the change.
| pub async fn abortable_run_with_spinner_with_input<F, T>( | ||
| task: F, | ||
| message: &str, | ||
| abort_signal: AbortSignal, | ||
| input_queue: &InputQueue, | ||
| ) -> Result<T> | ||
| where | ||
| F: Future<Output = Result<T>>, | ||
| { | ||
| let (_, spinner_rx) = Spinner::create(message); | ||
| abortable_run_with_spinner_rx(task, spinner_rx, abort_signal, Some(input_queue.clone())).await | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What do the crossterm 0.28.x docs say about enabling raw mode before using event::poll/event::read for immediate keyboard input?
💡 Result:
The crossterm 0.28.x documentation, specifically in the crossterm::event module, states that you must enable raw mode before using event::poll or event::read for proper keyboard event handling, including immediate/non-blocking keyboard input. This is explicitly documented with the statement: "Make sure to enable raw mode in order for keyboard events to work properly". This requirement enables character-by-character input without line buffering, necessary for immediate key detection via poll/read with zero or short timeouts. Use crossterm::terminal::enable_raw_mode before event polling/reading, and disable_raw_mode afterward. Mouse/resize events may work without it, but keyboard requires raw mode, particularly on Unix-like systems for unbuffered input.
Citations:
- 1: https://laezerus.github.io/Polars/crossterm/event/index.html
- 2: https://github.com/crossterm-rs/crossterm/blob/master/src/event.rs
- 3: https://docs.rs/crossterm/latest/crossterm/event/index.html
- 4: https://docs.rs/crossterm/latest/src/crossterm/event.rs.html
🏁 Script executed:
cd src/utils && wc -l spinner.rsRepository: dobesv/harnx
Length of output: 70
🏁 Script executed:
cd src/utils && sed -n '180,210p' spinner.rsRepository: dobesv/harnx
Length of output: 876
🏁 Script executed:
cd src/utils && sed -n '270,290p' spinner.rsRepository: dobesv/harnx
Length of output: 516
🏁 Script executed:
cd src && grep -n "enable_raw_mode\|disable_raw_mode" **/*.rsRepository: dobesv/harnx
Length of output: 220
🏁 Script executed:
cd src/utils && sed -n '1,50p' spinner.rs | head -20Repository: dobesv/harnx
Length of output: 490
🏁 Script executed:
cd src && grep -n "fn poll_abort_signal_with_input\|fn poll_abort_signal" **/*.rsRepository: dobesv/harnx
Length of output: 266
🏁 Script executed:
cd src/utils && sed -n '77,110p' abort_signal.rsRepository: dobesv/harnx
Length of output: 1073
🏁 Script executed:
cd src/utils && sed -n '84,130p' abort_signal.rsRepository: dobesv/harnx
Length of output: 918
🏁 Script executed:
cd src && grep -rn "abortable_run_with_spinner_with_input\|poll_abort_signal_with_input" --include="*.rs" | head -20Repository: dobesv/harnx
Length of output: 677
🏁 Script executed:
cd src && grep -B 5 -A 5 "abortable_run_with_spinner_with_input" **/*.rsRepository: dobesv/harnx
Length of output: 1160
🏁 Script executed:
cd src/client && sed -n '570,590p' common.rsRepository: dobesv/harnx
Length of output: 769
🏁 Script executed:
cd src/client && sed -n '550,600p' common.rs | grep -B 20 -A 20 "abortable_run_with_spinner_with_input"Repository: dobesv/harnx
Length of output: 1082
abortable_run_with_spinner_with_input polls keyboard events without enabling raw mode.
Per crossterm 0.28.x documentation, event::poll() and event::read() require raw mode to be enabled for immediate keyboard input. This function and its called path in poll_abort_signal_with_input() use these functions without entering raw mode first, causing keyboard input to be line-buffered and echoed instead of responding to individual keystrokes.
Raw mode must be enabled before the spinner loop starts polling for keyboard events.
Also applies to: 205-206, 276-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/spinner.rs` around lines 188 - 199, The spinner path currently
polls keyboard events without enabling raw mode; update
abortable_run_with_spinner_with_input (and the called
poll_abort_signal_with_input/poll_abort_signal code paths) to enable crossterm
raw mode before starting the spinner/keyboard-poll loop and disable raw mode
afterwards (ensuring disable_raw_mode runs on all exits/errors, e.g., via a drop
guard or finally-like cleanup). Specifically, call terminal::enable_raw_mode()
before creating/entering the spinner loop that uses event::poll()/event::read(),
and call terminal::disable_raw_mode() when the spinner finishes or on early
return/error to restore terminal state; propagate or log any errors from
enable/disable and ensure input_queue handling remains unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/render/stream.rs (2)
91-105: Recompute terminal geometry during the render loop to handle resize safely.
terminal::size()and derivedcontent_height/footer_rowsare computed once. After a terminal resize, subsequent cursor positioning and clears use stale dimensions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/render/stream.rs` around lines 91 - 105, Recompute terminal geometry each iteration of the render loop instead of once: call terminal::size() inside the loop and recalculate needs_spinner (based on spinner_message), has_input (based on input_queue), footer_rows, and content_height there so cursor positioning and clears use up-to-date dimensions; update any code using the previously computed content_height/footer_rows to reference the recomputed values within the loop (e.g., in functions or blocks that use content_height, footer_rows, needs_spinner, input_queue).
116-121:spinner_stoppedis currently redundant state.The flag is toggled once but has no functional impact after that; removing it would simplify this path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/render/stream.rs` around lines 116 - 121, The local flag spinner_stopped is redundant: remove the spinner_stopped variable declaration and the entire conditional that just sets spinner_stopped = true (the if !spinner_stopped { ... } block) in the render/stream logic so no-op state toggling remains; also remove any other references/initialization of spinner_stopped in the same scope so the code compiles cleanly (there is no replacement behavior needed since the block had no side effects beyond flipping the flag).
🤖 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/render/stream.rs`:
- Around line 252-253: The footer/input print is writing the full input line
without bounding by terminal width, causing long input to wrap and corrupt
spinner/content; clip the rendered input to the available columns before
printing by using the terminal width field (e.g. _columns or
term_height/term_width) to compute a max display width, truncate the input
string to that width (accounting for spinner/prefix width) and then pass the
clipped string to the existing print/write call that renders the footer (the
same call referenced around lines ~300–305); apply the same truncation logic to
all places that render the input/footer to prevent wrapping into neighboring
rows.
- Around line 248-258: The function render_footer_absolute currently uses
#[allow(clippy::too_many_arguments)]; remove that attribute and introduce a
small FooterContext struct (e.g., FooterContext { columns: u16, term_height:
u16, needs_spinner: bool, spinner_message: String or &str, spinner_index: usize,
input_queue: Option<InputQueueRef> }) to group the footer-related parameters
(columns, term_height, needs_spinner, spinner_message, spinner_index,
input_queue), update the render_footer_absolute signature to accept &mut Stdout,
&mut MarkdownRender, and a FooterContext (or a reference to it), adapt the
function body to use context fields, update all call sites to build and pass
FooterContext, and remove the clippy allow attribute.
---
Nitpick comments:
In `@src/render/stream.rs`:
- Around line 91-105: Recompute terminal geometry each iteration of the render
loop instead of once: call terminal::size() inside the loop and recalculate
needs_spinner (based on spinner_message), has_input (based on input_queue),
footer_rows, and content_height there so cursor positioning and clears use
up-to-date dimensions; update any code using the previously computed
content_height/footer_rows to reference the recomputed values within the loop
(e.g., in functions or blocks that use content_height, footer_rows,
needs_spinner, input_queue).
- Around line 116-121: The local flag spinner_stopped is redundant: remove the
spinner_stopped variable declaration and the entire conditional that just sets
spinner_stopped = true (the if !spinner_stopped { ... } block) in the
render/stream logic so no-op state toggling remains; also remove any other
references/initialization of spinner_stopped in the same scope so the code
compiles cleanly (there is no replacement behavior needed since the block had no
side effects beyond flipping the flag).
🪄 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: f084bf92-ec58-40f7-bb39-12e2b0c611fa
📒 Files selected for processing (2)
src/render/stream.rssrc/repl/input_queue.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/repl/input_queue.rs
| #[allow(clippy::too_many_arguments)] | ||
| fn render_footer_absolute( | ||
| writer: &mut Stdout, | ||
| render: &mut MarkdownRender, | ||
| _columns: u16, | ||
| term_height: u16, | ||
| needs_spinner: bool, | ||
| spinner_message: &str, | ||
| spinner_index: &mut usize, | ||
| input_queue: Option<&InputQueue>, | ||
| ) -> Result<()> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove #[allow(clippy::too_many_arguments)] by grouping footer parameters.
This attribute suppresses a clippy warning instead of fixing it. Please refactor the function args into a small context struct.
♻️ Suggested refactor
-#[allow(clippy::too_many_arguments)]
-fn render_footer_absolute(
- writer: &mut Stdout,
- render: &mut MarkdownRender,
- _columns: u16,
- term_height: u16,
- needs_spinner: bool,
- spinner_message: &str,
- spinner_index: &mut usize,
- input_queue: Option<&InputQueue>,
-) -> Result<()> {
+struct FooterRenderCtx<'a> {
+ render: &'a mut MarkdownRender,
+ columns: u16,
+ term_height: u16,
+ needs_spinner: bool,
+ spinner_message: &'a str,
+ spinner_index: &'a mut usize,
+ input_queue: Option<&'a InputQueue>,
+}
+
+fn render_footer_absolute(writer: &mut Stdout, ctx: FooterRenderCtx<'_>) -> Result<()> {As per coding guidelines, "Do not ignore clippy warnings; treat all clippy warnings as errors (CI sets RUSTFLAGS=--deny warnings)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/render/stream.rs` around lines 248 - 258, The function
render_footer_absolute currently uses #[allow(clippy::too_many_arguments)];
remove that attribute and introduce a small FooterContext struct (e.g.,
FooterContext { columns: u16, term_height: u16, needs_spinner: bool,
spinner_message: String or &str, spinner_index: usize, input_queue:
Option<InputQueueRef> }) to group the footer-related parameters (columns,
term_height, needs_spinner, spinner_message, spinner_index, input_queue), update
the render_footer_absolute signature to accept &mut Stdout, &mut MarkdownRender,
and a FooterContext (or a reference to it), adapt the function body to use
context fields, update all call sites to build and pass FooterContext, and
remove the clippy allow attribute.
| _columns: u16, | ||
| term_height: u16, |
There was a problem hiding this comment.
Long queued input can wrap and corrupt the pinned footer/content layout.
Line 303/305 prints the full input line without width bounds. When input exceeds terminal width, it wraps into neighboring rows and can overwrite spinner/content.
🐛 Suggested fix (clip footer line to terminal width)
-fn render_footer_absolute(
+fn render_footer_absolute(
writer: &mut Stdout,
render: &mut MarkdownRender,
- _columns: u16,
+ columns: u16,
@@
if let Some(iq) = input_queue {
queue!(writer, cursor::MoveTo(0, current_row))?;
let display = iq.get_display_line();
- let input_output = render.render_line(&display);
+ let input_output = render.render_line(&display);
+ let input_output = truncate_to_width(&input_output, columns);
queue!(writer, style::Print(&input_output))?;
}
@@
}
+
+fn truncate_to_width(s: &str, columns: u16) -> String {
+ let max = columns as usize;
+ if display_width(s) <= max {
+ return s.to_string();
+ }
+ let mut out = String::new();
+ let mut width = 0usize;
+ for ch in s.chars() {
+ let mut buf = [0u8; 4];
+ let chs = ch.encode_utf8(&mut buf);
+ let cw = display_width(chs);
+ if width + cw > max {
+ break;
+ }
+ out.push(ch);
+ width += cw;
+ }
+ out
+}Also applies to: 300-305
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/render/stream.rs` around lines 252 - 253, The footer/input print is
writing the full input line without bounding by terminal width, causing long
input to wrap and corrupt spinner/content; clip the rendered input to the
available columns before printing by using the terminal width field (e.g.
_columns or term_height/term_width) to compute a max display width, truncate the
input string to that width (accounting for spinner/prefix width) and then pass
the clipped string to the existing print/write call that renders the footer (the
same call referenced around lines ~300–305); apply the same truncation logic to
all places that render the input/footer to prevent wrapping into neighboring
rows.
1 participant
Allow users to type their next message while the LLM is processing. Keystrokes are captured during spinner and stream rendering, shown as a mini input line below the spinner. Pressing Enter enqueues the message (shown with ⏳ indicator), and it is sent at the next opportunity. Editing a queued message un-queues it until Enter is pressed again. At most one message can be queued at a time.
New file: src/repl/input_queue.rs - InputQueue shared state type Modified: abort_signal, spinner, stream renderer, render_stream, call_chat_completions, call_chat_completions_streaming, Repl struct, ask/ask_inner chain, run_repl_command, config/mod.rs macro execution.
Summary by CodeRabbit