display node names and status in statusbar#537
Conversation
github-actions
Bot
added
rust
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR shortens Display output strings for several hive step types (SwitchToConfiguration, Build, Evaluate, PushKeyAgent, Ping, PushEvaluatedOutput, PushBuildOutput) and reworks status.rs: NodeStatus gains ordering derives with reordered variants, Status.statuses is keyed by Name, and status rendering/clearing logic is rebuilt to sort, truncate, cap, and precisely clear multi-line terminal output. ChangesStatus and step display changes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Worker as status_tick_worker
participant Status as Status
participant Terminal as Terminal Writer
Worker->>Status: UiMessage::SetStatus (Name, NodeStatus)
Status->>Status: insert into statuses map (keyed by Name)
Worker->>Status: write_status(writer)
Status->>Status: clear(writer) using previous_number_lines
Status->>Status: get_msg() - filter, sort, cap, truncate
Status->>Terminal: write rendered multi-line message
Status->>Status: update previous_number_lines
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/status.rs`:
- Around line 183-184: The trailing `padding` computation in `status.rs` is dead
because `strip_str(&line)` already makes the subtraction clamp to zero, so the
`write!` call in the status rendering path adds no real alignment. Remove the
unused padding logic from the code that builds `msg` (and drop the `strip_str`
import if nothing else uses it), or adjust the width calculation so it reflects
the intended remaining space after the name/status text. Use the existing status
formatting flow around `line`, `max_name_len`, and the final `write!` call to
keep the output unchanged except for eliminating the redundant work.
- Around line 218-228: `write_status` is tracking logical newline count instead
of the number of rendered terminal rows, which breaks `clear()` on wrapped
status text. Update `Status::write_status` so `previous_number_lines` reflects
physical rows based on terminal width (or ensure the status message always fits
on one visual row), and keep the clearing logic in sync with that row count.
🪄 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: 6019689a-1a3f-40b5-8d5c-e335dd8ff824
📒 Files selected for processing (7)
crates/core/src/hive/steps/activate.rscrates/core/src/hive/steps/build.rscrates/core/src/hive/steps/evaluate.rscrates/core/src/hive/steps/keys.rscrates/core/src/hive/steps/ping.rscrates/core/src/hive/steps/push.rscrates/core/src/status.rs
| let padding = " ".repeat(max_name_len.saturating_sub(strip_str(&line).len())); | ||
| let _ = write!(&mut msg, "{line}{padding}"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
padding here is always empty — dead computation.
strip_str(&line) returns the whole visible line (name + spacing + status label, including the leading \n), which is always longer than max_name_len, so max_name_len.saturating_sub(...) is 0 and padding is always "". Column alignment is already handled by the space run at Line 171, so this trailing padding does nothing. Drop it (and the strip_str import if unused elsewhere) or fix it to compute the intended remaining width.
♻️ Proposed cleanup
- let padding = " ".repeat(max_name_len.saturating_sub(strip_str(&line).len()));
- let _ = write!(&mut msg, "{line}{padding}");
+ let _ = write!(&mut msg, "{line}");📝 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 padding = " ".repeat(max_name_len.saturating_sub(strip_str(&line).len())); | |
| let _ = write!(&mut msg, "{line}{padding}"); | |
| let _ = write!(&mut msg, "{line}"); |
🤖 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/core/src/status.rs` around lines 183 - 184, The trailing `padding`
computation in `status.rs` is dead because `strip_str(&line)` already makes the
subtraction clamp to zero, so the `write!` call in the status rendering path
adds no real alignment. Remove the unused padding logic from the code that
builds `msg` (and drop the `strip_str` import if nothing else uses it), or
adjust the width calculation so it reflects the intended remaining space after
the name/status text. Use the existing status formatting flow around `line`,
`max_name_len`, and the final `write!` call to keep the output unchanged except
for eliminating the redundant work.
| pub fn write_status<T: std::io::Write>(&mut self, writer: &mut T) { | ||
| if self.show_progress { | ||
| let _ = write!(writer, "{}", self.get_msg()); | ||
| if !self.show_progress { | ||
| return; | ||
| } | ||
|
|
||
| let msg = self.get_msg(); | ||
|
|
||
| let _ = write!(writer, "{msg}"); | ||
| let _ = writer.flush(); | ||
|
|
||
| self.previous_number_lines = msg.lines().count().saturating_sub(1); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first
ast-grep outline crates/core/src/status.rs --view expanded
# Show the relevant section with line numbers
sed -n '1,320p' crates/core/src/status.rs | cat -n
# Search for cursor movement / clearing logic and terminal width usage
rg -n "cursor::Up|previous_number_lines|terminal width|width|wrap|truncate|clear" crates/core/src/status.rs crates/core/src -S
# Lightweight behavioral probe: inspect whether rendered status text is width-bounded
python3 - <<'PY'
from pathlib import Path
p = Path("crates/core/src/status.rs")
text = p.read_text()
# Print the specific functions around write_status and clear if present
for name in ["write_status", "clear", "get_msg"]:
idx = text.find(f"fn {name}")
if idx != -1:
start = max(0, text.rfind("\n", 0, idx-1)-1)
end = min(len(text), text.find("\n pub fn ", idx+1) if text.find("\n pub fn ", idx+1) != -1 else len(text))
print(f"\n--- {name} ---")
print(text[idx:end])
PYRepository: forallsys/wire
Length of output: 28968
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any width-aware cursor clearing or wrapping logic elsewhere
rg -n "terminal_size\(|cursor::Up\(|clear::CurrentLine|strip_str\(|wrap|truncate|width" crates/core/src -S
# Estimate the maximum visible length of the summary line and a typical status line from the code.
python3 - <<'PY'
from pathlib import Path
import re
text = Path("crates/core/src/status.rs").read_text()
# Extract the get_msg function body roughly
m = re.search(r'pub fn get_msg\(&self\) -> String \{([\s\S]*?)^\s*\}\s*$', text, re.M)
body = m.group(1) if m else text
# Visible summary skeleton examples based on the code
examples = [
"[0 / 0] 0s",
"[123 / 456] (12 Failed, 34 Deploying) 999s",
"[123 / 456] (12 Failed) 999s",
"[123 / 456] (34 Deploying) 999s",
]
for s in examples:
print(len(s), s)
PYRepository: forallsys/wire
Length of output: 6310
Track rendered rows, not msg.lines()
previous_number_lines only counts logical \ns, but clear() moves up one physical row per count. Any wrapped status line will leave stale rows behind on narrow terminals. Count rendered rows with the terminal width, or keep each status line to a single visual row.
🤖 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/core/src/status.rs` around lines 218 - 228, `write_status` is tracking
logical newline count instead of the number of rendered terminal rows, which
breaks `clear()` on wrapped status text. Update `Status::write_status` so
`previous_number_lines` reflects physical rows based on terminal width (or
ensure the status message always fits on one visual row), and keep the clearing
logic in sync with that row count.
mrshmllow
changed the title
3c397a1 to
405faef
Compare
1 participant
A cut down version of #416 that is far smaller and easier to maintain. Full nix job tracking can come at a later date
Summary by CodeRabbit