[3604] Surface mutating tool evidence status in the interactive TUI#3606
[3604] Surface mutating tool evidence status in the interactive TUI#3606
Conversation
# Conflicts: # crates/tau-tui/src/interactive/mod.rs
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a11f5fbe7
ℹ️ 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".
| let Some(entry) = app.tools.latest_entry() else { | ||
| return Vec::new(); | ||
| return lines; | ||
| }; | ||
|
|
||
| terminal_summary_lines(entry) | ||
| lines.extend(terminal_summary_lines(entry)); |
There was a problem hiding this comment.
This still appends terminal_summary_lines from app.tools.latest_entry() (global history), so a new build/create turn with no current-tool activity can show both Build status: no mutating evidence yet and Last tool: write from a prior turn. In the context of this change (resetting evidence per turn), that leaks prior mutating evidence back into the active-turn surface and gives contradictory operator guidance. Use current-turn entries for the terminal summary (or suppress it until the current turn has tool events) when build status is shown.
Useful? React with 👍 / 👎.
| pub fn current_turn_tools(&self) -> &[ToolEntry] { | ||
| let start = self.current_turn_tool_start.min(self.tools.entries().len()); | ||
| &self.tools.entries()[start..] |
There was a problem hiding this comment.
current_turn_tools() slices by a stored vector index, but ToolPanel evicts from the front once history exceeds 200 entries. During long turns, that start index is not adjusted as evictions happen, so early tool events from the same turn are dropped from the slice; a turn with an early successful write/edit can later be misclassified as read-only or missing. The turn boundary needs to be tracked in a truncation-safe way (e.g., monotonic IDs or eviction-aware offset updates).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a “Build status” banner to the interactive TUI chat surface during active build/create turns, indicating whether the current turn has no successful tool evidence yet, is still read-only, or has confirmed mutating evidence—while resetting evidence per user turn.
Changes:
- Introduce
build_statusclassification and render it in the chat summary strip (only for non-idle build/create turns). - Track “current turn” tool entries in
Appso prior-turn successful mutations don’t affect the next turn’s status. - Add integration-style ratatui render-path tests covering all banner states and the per-turn reset behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/todo.md | Roadmap status date bump. |
| tasks/tau-vs-ironclaw-gap-list.md | Status snapshot date bump. |
| specs/3604/spec.md | New spec capturing ACs, failure modes, and test evidence. |
| crates/tau-tui/src/interactive/build_status.rs | New build/create prompt + tool-evidence classifier and unit tests. |
| crates/tau-tui/src/interactive/ui_chat_tool_lines.rs | Prepends build-status banner lines to the chat summary tool lines. |
| crates/tau-tui/src/interactive/app.rs | Tracks per-turn tool slice start; exposes current_turn_tools() and latest_user_prompt(). |
| crates/tau-tui/src/interactive/ui_build_status_tests.rs | New render-path tests validating banner states and turn reset. |
| crates/tau-tui/src/interactive/app_commands.rs | Routes user submissions through App::push_message() to trigger per-turn reset. |
| crates/tau-tui/src/interactive/{chat,input,tools}.rs | Adds Default impls for panel structs. |
| crates/tau-tui/src/interactive/mod.rs | Wires new module + tests. |
| crates/tau-tools/src/tools.rs | Small refactors using is_some_and and simplified recursion guard. |
| crates/tau-coding-agent/src/tests/auth_provider/runtime_and_startup.rs | Adds restore_current_dir helper to make tests more robust. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let start = self.current_turn_tool_start.min(self.tools.entries().len()); | ||
| &self.tools.entries()[start..] |
There was a problem hiding this comment.
current_turn_tools() slices tools.entries() using current_turn_tool_start captured from tools.total_count(). Since ToolPanel::add_entry() can evict the oldest entries when max_entries is exceeded, indices can shift during a turn, causing current_turn_tool_start to point too far into the truncated buffer and incorrectly drop early tool entries from the current turn (breaking per-turn evidence classification after long sessions). Consider tracking a monotonic tool sequence/turn id (e.g., store an ever-increasing counter on each ToolEntry or in ToolPanel and filter by seq >= turn_start_seq), or adjust the start marker when eviction occurs so it remains aligned with the in-memory entries buffer.
| let start = self.current_turn_tool_start.min(self.tools.entries().len()); | |
| &self.tools.entries()[start..] | |
| // Interpret `current_turn_tool_start` as a global sequence number from | |
| // `ToolPanel::total_count()`, not as a direct index into the current | |
| // in-memory entries buffer. Since `ToolPanel` may evict old entries, | |
| // we translate that sequence number into an index relative to the | |
| // current sliding window. | |
| let entries = self.tools.entries(); | |
| if entries.is_empty() { | |
| return entries; | |
| } | |
| let total = self.tools.total_count(); | |
| // Global sequence number of the first entry currently in memory. | |
| let first_seq = total.saturating_sub(entries.len()); | |
| let start_seq = self.current_turn_tool_start; | |
| let start_index = if start_seq <= first_seq { | |
| // Either all in-memory entries are from the current turn, or | |
| // earlier entries from this turn have been evicted. In both | |
| // cases, we start from the beginning of the buffer. | |
| 0 | |
| } else { | |
| start_seq - first_seq | |
| }; | |
| &entries[start_index..] |
| matches!( | ||
| word.to_ascii_lowercase().as_str(), | ||
| "build" | "create" | "implement" | "make" | ||
| ) |
There was a problem hiding this comment.
is_build_verb() calls to_ascii_lowercase() for every token, allocating a new String each time current_build_status() runs. Since this is invoked during rendering (potentially every tick), this can create avoidable per-frame allocations for long prompts. Consider using eq_ignore_ascii_case() (e.g., word.eq_ignore_ascii_case("build")) or pre-normalizing without allocation to keep render-path overhead predictable.
| matches!( | |
| word.to_ascii_lowercase().as_str(), | |
| "build" | "create" | "implement" | "make" | |
| ) | |
| word.eq_ignore_ascii_case("build") | |
| || word.eq_ignore_ascii_case("create") | |
| || word.eq_ignore_ascii_case("implement") | |
| || word.eq_ignore_ascii_case("make") |
Closes #3604
Spec:
specs/3604/spec.mdWhat/why:
Test evidence:
cargo test -p tau-tui 3604 -- --nocapturecargo test -p tau-tuicargo clippy -p tau-tui --all-targets --all-features -- -D warningscargo clippy -p tau-tools --all-targets --all-features -- -D warningscargo test -p tau-coding-agent regression_spec_3555_c01_run_local_runtime_uses_cli_request_timeout_for_agent -- --nocapture./scripts/dev/fast-validate.sh --base origin/master