[3607] Surface live mutating tool progress in the interactive TUI#3608
[3607] Surface live mutating tool progress in the interactive TUI#3608
Conversation
There was a problem hiding this comment.
njfio has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7f88210c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| pub(crate) fn build_tool_summary_lines(app: &App) -> Vec<Line<'static>> { | ||
| let mut lines = build_build_status_lines(app); | ||
| lines.extend(build_mutating_progress_lines(app)); |
There was a problem hiding this comment.
Gate mutating-progress summary to active build turns
build_mutating_progress_lines is appended unconditionally, so any turn with a write/edit event can show "Mutating now" or "Latest … target" even when the prompt is not a build/create request or when the agent is idle. This regresses the intended behavior (idle and non-build turns should omit mutating-progress wording) and creates misleading UI state after normal non-build edits or after a build turn has completed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a dedicated interactive-TUI surface for mutating tool activity (write/edit) so users can see live “we’re writing/editing files” progress during build/create turns, and splits rendering logic into a new module with integration-style render tests.
Changes:
- Introduces
ui_chat_mutating_progressto render “Mutating now” / “Latest write/edit target” lines based on current-turn tool entries. - Wires the new mutating-progress lines into the chat summary strip (
build_tool_summary_lines). - Adds render-path tests covering running/successful mutating tools and a read→edit transition within a build turn.
- Adds a spec document for issue #3607.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
specs/3607-live-mutating-tool-progress.md |
Adds spec text for #3607 (but currently not in the repo’s expected spec path/format). |
crates/tau-tui/src/interactive/ui_mutating_tool_progress_tests.rs |
Adds integration-style render tests for the new mutating-progress UI surface. |
crates/tau-tui/src/interactive/ui_chat_tool_lines.rs |
Wires mutating-progress lines into the tool summary strip. |
crates/tau-tui/src/interactive/ui_chat_mutating_progress.rs |
Implements mutating-progress line construction for running/successful write/edit tools. |
crates/tau-tui/src/interactive/mod.rs |
Registers the new module and its tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 3607 Live Mutating Tool Progress | ||
|
|
||
| ## Objective | ||
| Make the interactive TUI show unmistakable live evidence when Tau is making mutating progress during build/create turns, especially for `write` and `edit` tool activity. | ||
|
|
||
| ## Inputs/Outputs | ||
| - Inputs: | ||
| - current agent state from `StatusBar` | ||
| - latest user prompt in the active turn | ||
| - current-turn `ToolEntry` records | ||
| - Outputs: | ||
| - live summary lines in the interactive chat/status surfaces that distinguish: | ||
| - no mutating evidence yet | ||
| - read-only progress | ||
| - mutating progress in flight | ||
| - latest successful mutating target path when available | ||
|
|
||
| ## Boundaries/Non-goals | ||
| - No runtime/provider/tool execution changes. | ||
| - No new tool payload fields. | ||
| - No full TUI layout redesign. | ||
| - No webchat changes. | ||
|
|
||
| ## Failure Modes | ||
| - Build/create turns incorrectly show mutating progress for read-only tools. | ||
| - Non-build turns show mutating progress lines. | ||
| - Prior-turn write/edit success leaks into the next turn. | ||
| - Missing tool detail produces malformed or misleading path output. | ||
|
|
||
| ## Acceptance Criteria | ||
| - [ ] Active build/create turn with a running `write` or `edit` tool shows a distinct mutating-progress line. | ||
| - [ ] Active build/create turn with a successful `write` or `edit` tool shows the latest mutating target path when detail is present. | ||
| - [ ] Active build/create turn that only has read-only tool activity does not show mutating-in-flight wording. | ||
| - [ ] Idle turns and non-build turns omit the mutating-progress lines. | ||
| - [ ] Integration coverage exercises a render path that transitions from read-only to mutating activity within one build turn. | ||
|
|
||
| ## Files To Touch | ||
| - `crates/tau-tui/src/interactive/ui_chat_tool_lines.rs` | ||
| - `crates/tau-tui/src/interactive/tools.rs` | ||
| - `crates/tau-tui/src/interactive/ui_tool_visibility_tests.rs` | ||
| - `crates/tau-tui/src/interactive/ui_build_status_tests.rs` | ||
| - `specs/3607-live-mutating-tool-progress.md` | ||
|
|
||
| ## Error Semantics | ||
| - No silent fallback to mutating wording without matching `write`/`edit` evidence. | ||
| - If tool detail is empty, render mutating state without inventing a path. | ||
| - Rendering helpers remain pure; no logging or swallowing state mismatches. | ||
|
|
||
| ## Test Plan | ||
| - Add red render tests for: | ||
| - running `write` mutating-progress line | ||
| - successful `write` target path line | ||
| - read-only build turn omits mutating wording | ||
| - new turn reset behavior remains correct | ||
| - Run targeted `tau-tui` tests first, then full `cargo test -p tau-tui`. | ||
|
|
||
| ## Status | ||
| - Implemented on branch `3607-live-mutating-tool-progress`. | ||
| - No deviations from the original scope. | ||
|
|
||
| ## Validation Evidence | ||
| - `cargo test -p tau-tui 3607 -- --nocapture` | ||
| - `cargo test -p tau-tui` | ||
| - `cargo clippy -p tau-tui --all-targets --all-features -- -D warnings` | ||
| - live smoke: `target/debug/tau-tui interactive --profile ops-interactive` |
There was a problem hiding this comment.
This spec won’t be discovered by the repo’s spec tooling/contract as written: specs are expected at specs/<issue-id>/spec.md (and the indexer scans only specs/*/spec.md). Consider moving this content to specs/3607/spec.md to keep it discoverable and consistent with the repository contract.
| # 3607 Live Mutating Tool Progress | |
| ## Objective | |
| Make the interactive TUI show unmistakable live evidence when Tau is making mutating progress during build/create turns, especially for `write` and `edit` tool activity. | |
| ## Inputs/Outputs | |
| - Inputs: | |
| - current agent state from `StatusBar` | |
| - latest user prompt in the active turn | |
| - current-turn `ToolEntry` records | |
| - Outputs: | |
| - live summary lines in the interactive chat/status surfaces that distinguish: | |
| - no mutating evidence yet | |
| - read-only progress | |
| - mutating progress in flight | |
| - latest successful mutating target path when available | |
| ## Boundaries/Non-goals | |
| - No runtime/provider/tool execution changes. | |
| - No new tool payload fields. | |
| - No full TUI layout redesign. | |
| - No webchat changes. | |
| ## Failure Modes | |
| - Build/create turns incorrectly show mutating progress for read-only tools. | |
| - Non-build turns show mutating progress lines. | |
| - Prior-turn write/edit success leaks into the next turn. | |
| - Missing tool detail produces malformed or misleading path output. | |
| ## Acceptance Criteria | |
| - [ ] Active build/create turn with a running `write` or `edit` tool shows a distinct mutating-progress line. | |
| - [ ] Active build/create turn with a successful `write` or `edit` tool shows the latest mutating target path when detail is present. | |
| - [ ] Active build/create turn that only has read-only tool activity does not show mutating-in-flight wording. | |
| - [ ] Idle turns and non-build turns omit the mutating-progress lines. | |
| - [ ] Integration coverage exercises a render path that transitions from read-only to mutating activity within one build turn. | |
| ## Files To Touch | |
| - `crates/tau-tui/src/interactive/ui_chat_tool_lines.rs` | |
| - `crates/tau-tui/src/interactive/tools.rs` | |
| - `crates/tau-tui/src/interactive/ui_tool_visibility_tests.rs` | |
| - `crates/tau-tui/src/interactive/ui_build_status_tests.rs` | |
| - `specs/3607-live-mutating-tool-progress.md` | |
| ## Error Semantics | |
| - No silent fallback to mutating wording without matching `write`/`edit` evidence. | |
| - If tool detail is empty, render mutating state without inventing a path. | |
| - Rendering helpers remain pure; no logging or swallowing state mismatches. | |
| ## Test Plan | |
| - Add red render tests for: | |
| - running `write` mutating-progress line | |
| - successful `write` target path line | |
| - read-only build turn omits mutating wording | |
| - new turn reset behavior remains correct | |
| - Run targeted `tau-tui` tests first, then full `cargo test -p tau-tui`. | |
| ## Status | |
| - Implemented on branch `3607-live-mutating-tool-progress`. | |
| - No deviations from the original scope. | |
| ## Validation Evidence | |
| - `cargo test -p tau-tui 3607 -- --nocapture` | |
| - `cargo test -p tau-tui` | |
| - `cargo clippy -p tau-tui --all-targets --all-features -- -D warnings` | |
| - live smoke: `target/debug/tau-tui interactive --profile ops-interactive` | |
| # 3607 Live Mutating Tool Progress | |
| This spec has been moved to `specs/3607/spec.md` to comply with the | |
| repository's spec discovery contract (`specs/<issue-id>/spec.md`). | |
| Please refer to `specs/3607/spec.md` for the full and up-to-date | |
| specification for live mutating tool progress. |
| - Run targeted `tau-tui` tests first, then full `cargo test -p tau-tui`. | ||
|
|
||
| ## Status | ||
| - Implemented on branch `3607-live-mutating-tool-progress`. | ||
| - No deviations from the original scope. | ||
|
|
There was a problem hiding this comment.
The spec archive tooling expects a literal Status: line in the spec (it parses lines that start with status:). This document currently has a ## Status section but no Status: line, so its status would be reported as Unknown even if moved under specs/<id>/spec.md. Add a Status: Implemented (or the appropriate value) line near the top to keep indexing/reporting accurate.
|
|
||
| pub(crate) fn build_tool_summary_lines(app: &App) -> Vec<Line<'static>> { | ||
| let mut lines = build_build_status_lines(app); | ||
| lines.extend(build_mutating_progress_lines(app)); |
There was a problem hiding this comment.
build_mutating_progress_lines is currently rendered unconditionally from build_tool_summary_lines, so mutating wording can appear on idle turns or non-build prompts (it doesn’t check agent_state/build prompt like current_build_status does). Gate the mutating-progress surface behind the same “active build/create turn” criteria so it can’t show up for non-build turns.
| lines.extend(build_mutating_progress_lines(app)); | |
| // Only show mutating progress when there is an active build/create turn, | |
| // matching the gating used by `current_build_status` in `build_build_status_lines`. | |
| if !lines.is_empty() { | |
| lines.extend(build_mutating_progress_lines(app)); | |
| } |
| app::App, | ||
| tools::{ToolEntry, ToolStatus}, | ||
| }; | ||
|
|
||
| pub(crate) fn build_mutating_progress_lines(app: &App) -> Vec<Line<'static>> { |
There was a problem hiding this comment.
build_mutating_progress_lines doesn’t currently verify that the current turn is a build/create turn (or that the agent isn’t idle). Because the chat summary is always rendered, a write/edit tool event during a non-build prompt would still show “Mutating now/Latest … target” despite the acceptance criteria. Consider early-returning unless current_build_status(app.status.agent_state, app.latest_user_prompt(), app.current_turn_tools()) is Some(_) (or equivalent shared predicate).
| app::App, | |
| tools::{ToolEntry, ToolStatus}, | |
| }; | |
| pub(crate) fn build_mutating_progress_lines(app: &App) -> Vec<Line<'static>> { | |
| app::{current_build_status, App}, | |
| tools::{ToolEntry, ToolStatus}, | |
| }; | |
| pub(crate) fn build_mutating_progress_lines(app: &App) -> Vec<Line<'static>> { | |
| // Only show mutating progress when the current turn is a build/create turn | |
| // and the agent is not idle, as determined by the shared build status predicate. | |
| if current_build_status(app.status.agent_state, app.latest_user_prompt(), app.current_turn_tools()).is_none() { | |
| return Vec::new(); | |
| } |
| vec![ | ||
| Line::from(vec![ | ||
| Span::styled("Mutating now: ", Style::default().fg(Color::Yellow)), | ||
| Span::styled( | ||
| entry.name.clone(), | ||
| Style::default() | ||
| .fg(Color::Yellow) | ||
| .add_modifier(Modifier::BOLD), | ||
| ), | ||
| ]), | ||
| Line::from(Span::styled( | ||
| format!(" {}", entry.detail), | ||
| Style::default().fg(Color::DarkGray), | ||
| )), | ||
| Line::from(""), | ||
| ] |
There was a problem hiding this comment.
When a mutating tool is running with an empty detail, running_mutating_lines still renders the indented “path” line (it becomes just whitespace). To avoid malformed output and to align with the existing pattern in ui_chat_tool_lines (push_tool_detail_line), only render the detail line when entry.detail is non-empty.
| vec![ | |
| Line::from(vec![ | |
| Span::styled("Mutating now: ", Style::default().fg(Color::Yellow)), | |
| Span::styled( | |
| entry.name.clone(), | |
| Style::default() | |
| .fg(Color::Yellow) | |
| .add_modifier(Modifier::BOLD), | |
| ), | |
| ]), | |
| Line::from(Span::styled( | |
| format!(" {}", entry.detail), | |
| Style::default().fg(Color::DarkGray), | |
| )), | |
| Line::from(""), | |
| ] | |
| let mut lines = Vec::new(); | |
| lines.push(Line::from(vec![ | |
| Span::styled("Mutating now: ", Style::default().fg(Color::Yellow)), | |
| Span::styled( | |
| entry.name.clone(), | |
| Style::default() | |
| .fg(Color::Yellow) | |
| .add_modifier(Modifier::BOLD), | |
| ), | |
| ])); | |
| if !entry.detail.is_empty() { | |
| lines.push(Line::from(Span::styled( | |
| format!(" {}", entry.detail), | |
| Style::default().fg(Color::DarkGray), | |
| ))); | |
| } | |
| lines.push(Line::from("")); | |
| lines |
|
|
||
| assert!(mutating.contains("Mutating now:"), "rendered:\n{mutating}"); | ||
| assert!(mutating.contains("src/game.rs"), "rendered:\n{mutating}"); | ||
| } |
There was a problem hiding this comment.
The new mutating-progress renderer has tests for build turns, but there’s no coverage ensuring the mutating-progress lines are omitted for (a) idle turns and (b) non-build prompts—both are explicit acceptance criteria and are easy to regress when wiring the renderer into the summary strip. Add integration-style render tests for those cases (and optionally for empty detail on running write/edit).
| } | |
| } | |
| #[test] | |
| fn integration_spec_3607_idle_turn_omits_mutating_progress_wording() { | |
| // Even if the previous prompt looked like a build request, an idle turn | |
| // should not show any mutating-progress summary lines. | |
| let mut app = build_app( | |
| "create a playable breakout clone", | |
| AgentStateDisplay::Idle, | |
| ); | |
| let text = render_text(&mut app); | |
| assert!( | |
| !text.contains("Mutating now:"), | |
| "rendered:\n{text}" | |
| ); | |
| assert!( | |
| !text.contains("Latest write target:"), | |
| "rendered:\n{text}" | |
| ); | |
| } | |
| #[test] | |
| fn integration_spec_3607_non_build_prompt_omits_mutating_progress_wording() { | |
| // A non-build style prompt (simple Q&A) should not show mutating-progress | |
| // lines in the summary strip when there has been no mutating tool usage. | |
| let mut app = build_app( | |
| "what is the capital of France?", | |
| AgentStateDisplay::ToolExec, | |
| ); | |
| let text = render_text(&mut app); | |
| assert!( | |
| !text.contains("Mutating now:"), | |
| "rendered:\n{text}" | |
| ); | |
| assert!( | |
| !text.contains("Latest write target:"), | |
| "rendered:\n{text}" | |
| ); | |
| } |
Closes #3607
Spec:
specs/3607-live-mutating-tool-progress.mdWhat/why:
writeandedittools during build/create turnsTest evidence:
cargo test -p tau-tui 3607 -- --nocapturecargo test -p tau-tuicargo clippy -p tau-tui --all-targets --all-features -- -D warningstarget/debug/tau-tui interactive --profile ops-interactive