feat(reborn): REPL UX — thinking spinner + markdown rendering - #6289
loopstring wants to merge 1 commit into
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
📝 WalkthroughWalkthroughThe CLI adds ChangesTerminal-aware CLI output
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runtime
participant Spinner
participant ReplyOutput
CLI->>Runtime: Send user message
Runtime->>Spinner: Await request with conditional spinner
Spinner-->>Runtime: Return assistant reply
Runtime->>ReplyOutput: Print reply with terminal render flag
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ 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 |
github-actions
Bot
added
size: M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an animated spinner on stderr while waiting for assistant replies and adds markdown rendering support using the termimad crate when stdout is a terminal. The feedback suggests ensuring the spinner is only displayed when stderr is a terminal to prevent ANSI escape sequences from corrupting redirected logs, and setting the tokio interval's missed tick behavior to Skip to avoid rapid spinner flickering if the executor thread is delayed.
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.
| let result = run_with_spinner( | ||
| stdin_is_tty, | ||
| runtime.send_user_message_with_cancellation( | ||
| conversation, | ||
| &text, | ||
| cancellation.clone(), | ||
| ) | ||
| .await | ||
| { | ||
| Ok(reply) if reply.is_successful_final_reply() => print_reply(&reply), | ||
| Ok(reply) if stdin_is_tty => print_reply(&reply), | ||
| ), | ||
| ) | ||
| .await; |
There was a problem hiding this comment.
If stdin is a TTY but stderr is redirected to a file (e.g., ironclaw-reborn repl 2> stderr.log), passing stdin_is_tty directly to run_with_spinner will cause it to write ANSI escape sequences into the redirected stderr stream. To prevent log corruption, only show the spinner if both stdin and stderr are terminals.
| let result = run_with_spinner( | |
| stdin_is_tty, | |
| runtime.send_user_message_with_cancellation( | |
| conversation, | |
| &text, | |
| cancellation.clone(), | |
| ) | |
| .await | |
| { | |
| Ok(reply) if reply.is_successful_final_reply() => print_reply(&reply), | |
| Ok(reply) if stdin_is_tty => print_reply(&reply), | |
| ), | |
| ) | |
| .await; | |
| let result = run_with_spinner( | |
| stdin_is_tty && std::io::stderr().is_terminal(), | |
| runtime.send_user_message_with_cancellation( | |
| conversation, | |
| &text, | |
| cancellation.clone(), | |
| ), | |
| ) | |
| .await; |
| } | ||
| const FRAMES: [&str; 10] = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; | ||
| tokio::pin!(fut); | ||
| let mut ticker = tokio::time::interval(Duration::from_millis(90)); |
There was a problem hiding this comment.
By default, tokio::time::interval uses MissedTickBehavior::Burst. If the executor thread is blocked or delayed, the ticker will accumulate missed ticks and burst them rapidly once it yields, causing the spinner to flicker or spin extremely fast. Setting the missed tick behavior to Skip ensures a smooth and consistent animation.
| let mut ticker = tokio::time::interval(Duration::from_millis(90)); | |
| let mut ticker = tokio::time::interval(Duration::from_millis(90)); | |
| ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 84ff5cf35777 |
Head: 84ff5cf3577757d4c5c3e8a51dee2dc72b908933
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The terminal UX branches lack caller-level regression coverage required for this production-wired CLI behavior change.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Exercise terminal rendering through the CLI
Location: crates/ironclaw_reborn_cli/src/runtime/mod.rs:329
The added test only proves that run_with_spinner returns its future's value; it never exercises either TTY branch or captures output. There is consequently no regression coverage that run --message/the REPL emits and clears the spinner only in the intended modes, renders markdown on a terminal, and preserves raw output when piped. Add a PTY/subprocess test or an injectable terminal/output seam that drives the command paths and asserts both terminal and piped behavior.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
| let reply = runtime | ||
| .send_user_message_with_cancellation(conversation, text, cancellation) | ||
| .await?; | ||
| let reply = run_with_spinner( |
There was a problem hiding this comment.
The only new test checks generic future passthrough, not this production CLI path. Please add caller-level terminal/piped output coverage (via PTY/subprocess or an injectable terminal/output seam) for the spinner and markdown branches.
The CLI REPL blocked silently while a turn ran and printed the raw markdown of the reply. Two self-contained UX fixes: - Spinner: show an animated `⠋ thinking…` on stderr while the (blocking) turn runs, cleared when the reply lands. No-op for non-interactive/piped runs so stdout stays byte-clean. - Markdown: when stdout is a terminal, render the assistant reply's markdown to ANSI (bold, code, lists, headers) via termimad; otherwise emit raw text so piping is unchanged. Applies to both the one-shot `run --message` and interactive `repl` paths. Not included (follow-up): real token streaming — the REPL uses the blocking `send_user_message_with_cancellation`; streaming needs a CLI-facing subscribe surface on the runtime (the WebUI already streams via SSE).
84ff5cf to
25a4c2f
Compare
![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
🤖 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.
Inline comments:
In `@crates/ironclaw_reborn_cli/src/runtime/mod.rs`:
- Around line 372-386: Update the REPL loop’s run_with_spinner invocation to
enable the spinner only when both stdin_is_tty and stderr is a terminal,
matching the send_once behavior. Preserve the existing cancellation, reply
handling, and render_markdown logic.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: cfe6b30f-0299-4dfd-ab8b-a3c7f658153e
📒 Files selected for processing (2)
crates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/runtime/mod.rs
| let render_markdown = std::io::stdout().is_terminal(); | ||
| let result = run_with_spinner( | ||
| stdin_is_tty, | ||
| runtime.send_user_message_with_cancellation( | ||
| conversation, | ||
| &text, | ||
| cancellation.clone(), | ||
| ) | ||
| .await | ||
| { | ||
| Ok(reply) if reply.is_successful_final_reply() => print_reply(&reply), | ||
| Ok(reply) if stdin_is_tty => print_reply(&reply), | ||
| ), | ||
| ) | ||
| .await; | ||
| match result { | ||
| Ok(reply) if reply.is_successful_final_reply() => { | ||
| print_reply(&reply, render_markdown) | ||
| } | ||
| Ok(reply) if stdin_is_tty => print_reply(&reply, render_markdown), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Disable spinner when stderr is redirected.
In the REPL loop, the spinner is enabled based solely on stdin_is_tty. If the user redirects stderr (e.g., ironclaw repl 2> log.txt), the spinner's ANSI clear codes (\r\x1b[K) and frames will corrupt the redirected log file. Ensure the spinner is only shown when stderr is also a terminal, matching the safe behavior implemented in send_once.
🐛 Proposed fix to protect redirected stderr
let render_markdown = std::io::stdout().is_terminal();
let result = run_with_spinner(
- stdin_is_tty,
+ stdin_is_tty && std::io::stderr().is_terminal(),
runtime.send_user_message_with_cancellation(
conversation,
&text,
cancellation.clone(),
),
)
.await;📝 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 render_markdown = std::io::stdout().is_terminal(); | |
| let result = run_with_spinner( | |
| stdin_is_tty, | |
| runtime.send_user_message_with_cancellation( | |
| conversation, | |
| &text, | |
| cancellation.clone(), | |
| ) | |
| .await | |
| { | |
| Ok(reply) if reply.is_successful_final_reply() => print_reply(&reply), | |
| Ok(reply) if stdin_is_tty => print_reply(&reply), | |
| ), | |
| ) | |
| .await; | |
| match result { | |
| Ok(reply) if reply.is_successful_final_reply() => { | |
| print_reply(&reply, render_markdown) | |
| } | |
| Ok(reply) if stdin_is_tty => print_reply(&reply, render_markdown), | |
| let render_markdown = std::io::stdout().is_terminal(); | |
| let result = run_with_spinner( | |
| stdin_is_tty && std::io::stderr().is_terminal(), | |
| runtime.send_user_message_with_cancellation( | |
| conversation, | |
| &text, | |
| cancellation.clone(), | |
| ), | |
| ) | |
| .await; | |
| match result { | |
| Ok(reply) if reply.is_successful_final_reply() => { | |
| print_reply(&reply, render_markdown) | |
| } | |
| Ok(reply) if stdin_is_tty => print_reply(&reply, render_markdown), |
🤖 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 `@crates/ironclaw_reborn_cli/src/runtime/mod.rs` around lines 372 - 386, Update
the REPL loop’s run_with_spinner invocation to enable the spinner only when both
stdin_is_tty and stderr is a terminal, matching the send_once behavior. Preserve
the existing cancellation, reply handling, and render_markdown logic.
This branch has not been deployed
2 participants
Standalone on
main.Summary
The CLI REPL blocked silently during a turn and printed raw markdown. Two self-contained UX fixes:
⠋ thinking…on stderr while the (blocking) turn runs, cleared when the reply lands. No-op for non-interactive/piped runs (stdout stays byte-clean).termimad; raw text otherwise so piping is unchanged.Applies to both
run --messageand interactiverepl.Not included (follow-up)
Real token streaming — the REPL uses the blocking
send_user_message_with_cancellation; streaming needs a CLI-facing subscribe surface on the runtime (the WebUI already streams via SSE).Verification
fmt + clippy clean;
run_with_spinnerunit test (value passes through in shown/hidden modes). Spinner/markdown render only on a real TTY (not capturable in CI), but the send→render path executes end-to-end.Files
crates/ironclaw_reborn_cli/Cargo.toml(addstermimad)crates/ironclaw_reborn_cli/src/runtime/mod.rs