Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/tui/src/tui/active_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ mod tests {
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
live_output: None,
}))
}

Expand Down
69 changes: 69 additions & 0 deletions crates/tui/src/tui/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2514,6 +2514,75 @@ impl App {
self.needs_redraw = true;
}

/// Poll shell manager for live output of running exec cells and update
/// their `live_output` field so the TUI shows incremental progress.
/// Called from the idle frame handler in `run_event_loop`.
pub fn poll_shell_progress(&mut self) {
use crate::tui::history::{ExecCell, ToolCell};

let Some(ref shell_mgr) = self.runtime_services.shell_manager else {
return;
};
let Some(ref active) = self.active_cell else {
return;
};

// Collect (index, command) of running exec cells
let mut running_execs: Vec<(usize, String)> = Vec::new();
for (i, entry) in active.entries().iter().enumerate() {
if let HistoryCell::Tool(ToolCell::Exec(ExecCell {
command,
status: crate::tui::history::ToolStatus::Running,
..
})) = entry
{
running_execs.push((i, command.clone()));
}
}
Comment on lines +2531 to +2541
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Cloning the command string for every running execution cell on every frame is inefficient. You can store just the indices and retrieve the command from the active_cell when needed to avoid unnecessary allocations.

        let mut running_exec_indices: Vec<usize> = Vec::new();
        for (i, entry) in active.entries().iter().enumerate() {
            if let HistoryCell::Tool(ToolCell::Exec(ExecCell {
                status: crate::tui::history::ToolStatus::Running,
                ..
            })) = entry
            {
                running_exec_indices.push(i);
            }
        }

if running_execs.is_empty() {
return;
}

// Get live output from shell manager
let jobs = match shell_mgr.lock() {
Ok(mut mgr) => mgr.list_jobs(),
Err(_) => return,
};

let active_ref = self.active_cell.as_mut().expect("active_cell just checked");
let mut updated = false;
for (idx, cmd) in &running_execs {
let cmd_prefix: String = cmd.chars().take(100).collect();
for job in &jobs {
if job.status == crate::tools::shell::ShellStatus::Running {
let job_prefix: String = job.command.chars().take(100).collect();
if cmd_prefix == job_prefix {
let tail = if !job.stderr_tail.trim().is_empty() {
job.stderr_tail.trim()
} else {
job.stdout_tail.trim()
};
if !tail.is_empty() {
if let Some(HistoryCell::Tool(ToolCell::Exec(ref mut ec))) =
active_ref.entry_mut(*idx)
{
let new_output = tail.to_string();
if ec.live_output.as_deref() != Some(&new_output) {
ec.live_output = Some(new_output);
updated = true;
}
}
}
}
}
}
}
Comment on lines +2554 to +2579
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The matching and output selection logic can be significantly optimized and improved:

  1. Efficiency: The current code performs multiple string allocations (collect::<String>()) inside a nested loop on every frame. Using direct string comparison (cmd == &job.command) avoids this overhead. Additionally, checking if the output changed before calling to_string() avoids redundant allocations.
  2. Correctness: Matching by a 100-character prefix is fragile and can lead to incorrect matches if multiple similar commands are running simultaneously. Direct comparison is safer.
  3. Output Integrity: Using trim() removes potentially significant whitespace, such as indentation or progress-indicating newlines. It is better to use the raw tail strings.
  4. Priority: The current logic completely ignores stdout if stderr has any content. While often acceptable, it can hide progress if the process emits warnings to stderr while continuing work on stdout.
        for idx in &running_exec_indices {
            let cmd = if let HistoryCell::Tool(ToolCell::Exec(ec)) = &active_ref.entries()[*idx] { &ec.command } else { continue };
            for job in &jobs {
                if job.status == crate::tools::shell::ShellStatus::Running && &job.command == cmd {
                    let tail = if !job.stderr_tail.is_empty() {
                        &job.stderr_tail
                    } else {
                        &job.stdout_tail
                    };
                    if !tail.is_empty() {
                        if let Some(HistoryCell::Tool(ToolCell::Exec(ref mut ec))) = active_ref.entry_mut(*idx) {
                            if ec.live_output.as_deref() != Some(tail) {
                                ec.live_output = Some(tail.to_string());
                                updated = true;
                            }
                        }
                    }
                }
            }
        }

if updated {
self.bump_active_cell_revision();
}
}


/// Total number of cells in the *virtual* transcript: `history.len()`
/// plus active cell entries (if any).
#[must_use]
Expand Down
11 changes: 11 additions & 0 deletions crates/tui/src/tui/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,10 @@ pub struct ExecCell {
pub interaction: Option<String>,
/// Cached output summary — avoids re-parsing JSON every frame.
pub output_summary: Option<String>,
/// Live incremental output shown while the command is still running.
/// Polled from the shell manager during idle frames and displayed
/// before the final output is available.
pub live_output: Option<String>,
}

impl ExecCell {
Expand Down Expand Up @@ -714,6 +718,13 @@ impl ExecCell {
TOOL_OUTPUT_LINE_LIMIT,
mode,
));
} else if let Some(live) = self.live_output.as_ref() {
lines.extend(render_exec_output_mode(
live,
width,
TOOL_OUTPUT_LINE_LIMIT,
mode,
));
} else if self.status == ToolStatus::Running && self.source == ExecSource::Assistant {
lines.extend(wrap_plain_line(
" Ctrl+B opens shell controls.",
Expand Down
5 changes: 5 additions & 0 deletions crates/tui/src/tui/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,7 @@ mod tests {
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
live_output: None,
})),
);
}
Expand Down Expand Up @@ -2239,6 +2240,7 @@ mod tests {
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
live_output: None,
})),
);
app.active_cell = Some(active);
Expand Down Expand Up @@ -2373,6 +2375,7 @@ mod tests {
source: ExecSource::Assistant,
interaction: None,
output_summary: Some("2 checks pending".to_string()),
live_output: None,
})));
}

Expand Down Expand Up @@ -2413,6 +2416,7 @@ mod tests {
source: ExecSource::Assistant,
interaction: None,
output_summary: Some("test failed".to_string()),
live_output: None,
})));

let text = lines_to_text(&task_panel_lines(&app, 80, 8));
Expand Down Expand Up @@ -2442,6 +2446,7 @@ mod tests {
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
live_output: None,
})));

let text = lines_to_text(&task_panel_lines(&app, 80, 8));
Expand Down
2 changes: 2 additions & 0 deletions crates/tui/src/tui/tool_routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ pub(super) fn handle_tool_call_started(
source,
interaction: Some(summary.clone()),
output_summary: None,
live_output: None,
})),
);
return;
Expand Down Expand Up @@ -144,6 +145,7 @@ pub(super) fn handle_tool_call_started(
source,
interaction: None,
output_summary: None,
live_output: None,
})),
);
return;
Expand Down
1 change: 1 addition & 0 deletions crates/tui/src/tui/transcript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ mod tests {
output: None,
started_at: None,
duration_ms: None,
live_output: None,
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
Expand Down
2 changes: 2 additions & 0 deletions crates/tui/src/tui/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,8 @@ async fn run_event_loop(
// Expire the "Press Ctrl+C again to quit" prompt silently after its
// window. Triggers a redraw if the prompt was visible.
app.tick_quit_armed();
// Poll shell manager for live output of running exec cells
app.poll_shell_progress();
app.tick_receipt();
// While the user is drag-selecting past the transcript edge, advance
// the viewport on a fixed cadence and extend the selection head so a
Expand Down
3 changes: 3 additions & 0 deletions crates/tui/src/tui/ui/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,7 @@ fn active_tool_status_label_summarizes_live_tool_group() {
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
live_output: None,
})),
);
active.push_tool(
Expand Down Expand Up @@ -1567,6 +1568,7 @@ fn active_tool_status_label_strips_shell_wrappers_from_ci_polling() {
source: ExecSource::Assistant,
interaction: None,
output_summary: None,
live_output: None,
})),
);
app.active_cell = Some(active);
Expand Down Expand Up @@ -3595,6 +3597,7 @@ fn terminal_pause_has_live_owner_only_for_running_exec_cells() {
source: ExecSource::Assistant,
interaction: Some("interactive".to_string()),
output_summary: None,
live_output: None,
})),
);
app.active_cell = Some(active);
Expand Down