feat(transcript): collapse consecutive tool runs into expandable summary cells (#2692)#2740
feat(transcript): collapse consecutive tool runs into expandable summary cells (#2692)#2740idling11 wants to merge 2 commits into
Conversation
…ary cells (#2692)
Add a transcript feature that groups contiguous successful tool calls
(exceeding a configurable threshold) into a single compact summary row.
Users can click the summary to expand it back to individual cells.
Core changes:
- `history.rs`:
- Add `is_success`, `is_failed`, `is_running`, `is_collapsible_guard`
helper methods to `ToolCell`.
- Add `ToolRun`, `detect_tool_runs()`, `is_collapsible_tool()`,
`tool_display_name()`, `tool_run_summary()` for run grouping.
- 6 new unit tests.
- `app.rs`:
- Add `tool_collapse_threshold: usize` (default 3, 0 disables).
- Add `expanded_tool_runs: HashSet<usize>` for click-to-toggle state.
- Clear expanded runs on `mark_history_updated()`.
- `widgets/mod.rs`:
- Pre-process `app.history` before rendering: detect tool runs,
replace collapsed runs with a single `"▼ N tools collapsed"`
summary cell, skip collapsed members from the output.
- `mouse_ui.rs`:
- Add `toggle_tool_run_expand()` — click on a collapsed summary
toggles expansion; click again to re-collapse.
- `settings.rs`:
- Add `tool_collapse_threshold` setting (serde default 3).
- Read setting at `App` initialization.
Non-goals for this PR (can be follow-ups):
- Calm-mode-only collapse (requires wiring calm mode into the widget).
- Right-aligned timestamps and token/context meters.
- Keyboard shortcut to expand/collapse.
Closes #2692
|
Thanks @idling11 for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
![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 a feature to collapse contiguous successful tool-call runs in the TUI transcript history to reduce clutter, allowing users to expand them via mouse clicks. The review feedback identifies a critical bug where clearing expanded runs in mark_history_updated makes the expansion non-functional, missing enum variants in the status helper that prevent certain tools from collapsing, a generic display name for MCP tools, and a potential performance bottleneck in the rendering loop that should be optimized with a pre-built HashMap.
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.
| pub fn mark_history_updated(&mut self) { | ||
| self.expanded_tool_runs.clear(); | ||
| self.history_version = self.history_version.wrapping_add(1); |
There was a problem hiding this comment.
Clearing expanded_tool_runs inside mark_history_updated() will immediately collapse any expanded tool runs whenever mark_history_updated() is called. Since toggle_tool_run_expand() calls mark_history_updated() to trigger a redraw, this means expanding a tool run immediately collapses it again on the same interaction, making the feature non-functional. It also collapses expanded runs during streaming.
Recommendation: Remove self.expanded_tool_runs.clear(); from mark_history_updated(), and instead clear it in clear_history() to ensure the expanded state persists across redraws and streaming updates.
| pub fn mark_history_updated(&mut self) { | |
| self.expanded_tool_runs.clear(); | |
| self.history_version = self.history_version.wrapping_add(1); | |
| pub fn mark_history_updated(&mut self) { | |
| self.history_version = self.history_version.wrapping_add(1); |
| fn status(&self) -> Option<ToolStatus> { | ||
| match self { | ||
| ToolCell::Exec(c) => Some(c.status), | ||
| ToolCell::Review(c) => Some(c.status), | ||
| ToolCell::WebSearch(c) => Some(c.status), | ||
| ToolCell::Generic(c) => Some(c.status), | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
The status() helper does not handle PlanUpdate, PatchSummary, or Mcp variants, even though they all contain a status: ToolStatus field. Consequently, is_success() will return false for successful MCP tools or plan updates, preventing them from ever being collapsed.
Recommendation: Add matches for PlanUpdate, PatchSummary, and Mcp to the status() helper.
fn status(&self) -> Option<ToolStatus> {
match self {
ToolCell::Exec(c) => Some(c.status),
ToolCell::Review(c) => Some(c.status),
ToolCell::WebSearch(c) => Some(c.status),
ToolCell::Generic(c) => Some(c.status),
ToolCell::PlanUpdate(c) => Some(c.status),
ToolCell::PatchSummary(c) => Some(c.status),
ToolCell::Mcp(c) => Some(c.status),
_ => None,
}
}| fn tool_display_name(tc: &ToolCell) -> String { | ||
| match tc { | ||
| ToolCell::Exec(c) => c | ||
| .command | ||
| .split_whitespace() | ||
| .next() | ||
| .unwrap_or("exec") | ||
| .to_string(), | ||
| ToolCell::Generic(c) => c.name.clone(), | ||
| ToolCell::Exploring(_) => "explore".to_string(), | ||
| ToolCell::PlanUpdate(_) => "update_plan".to_string(), | ||
| ToolCell::PatchSummary(_) => "apply_patch".to_string(), | ||
| ToolCell::Review(_) => "review".to_string(), | ||
| ToolCell::DiffPreview(_) => "diff".to_string(), | ||
| ToolCell::Mcp(_) => "mcp".to_string(), | ||
| ToolCell::ViewImage(_) => "view_image".to_string(), | ||
| ToolCell::WebSearch(_) => "web_search".to_string(), | ||
| } | ||
| } |
There was a problem hiding this comment.
tool_display_name returns a generic "mcp" string for any MCP tool cell. Since MCP tools have distinct names (e.g., ggr, fetch, read_file) stored in c.tool, returning "mcp" groups all different MCP tools under a single generic name in the collapsed summary.
Recommendation: Return c.tool.clone() for ToolCell::Mcp(c) to make the collapsed summary much more descriptive.
| fn tool_display_name(tc: &ToolCell) -> String { | |
| match tc { | |
| ToolCell::Exec(c) => c | |
| .command | |
| .split_whitespace() | |
| .next() | |
| .unwrap_or("exec") | |
| .to_string(), | |
| ToolCell::Generic(c) => c.name.clone(), | |
| ToolCell::Exploring(_) => "explore".to_string(), | |
| ToolCell::PlanUpdate(_) => "update_plan".to_string(), | |
| ToolCell::PatchSummary(_) => "apply_patch".to_string(), | |
| ToolCell::Review(_) => "review".to_string(), | |
| ToolCell::DiffPreview(_) => "diff".to_string(), | |
| ToolCell::Mcp(_) => "mcp".to_string(), | |
| ToolCell::ViewImage(_) => "view_image".to_string(), | |
| ToolCell::WebSearch(_) => "web_search".to_string(), | |
| } | |
| } | |
| fn tool_display_name(tc: &ToolCell) -> String { | |
| match tc { | |
| ToolCell::Exec(c) => c | |
| .command | |
| .split_whitespace() | |
| .next() | |
| .unwrap_or("exec") | |
| .to_string(), | |
| ToolCell::Generic(c) => c.name.clone(), | |
| ToolCell::Exploring(_) => "explore".to_string(), | |
| ToolCell::PlanUpdate(_) => "update_plan".to_string(), | |
| ToolCell::PatchSummary(_) => "apply_patch".to_string(), | |
| ToolCell::Review(_) => "review".to_string(), | |
| ToolCell::DiffPreview(_) => "diff".to_string(), | |
| ToolCell::Mcp(c) => c.tool.clone(), | |
| ToolCell::ViewImage(_) => "view_image".to_string(), | |
| ToolCell::WebSearch(_) => "web_search".to_string(), | |
| } | |
| } |
| for (idx, cell) in app.history.iter().enumerate() { | ||
| if app.collapsed_cells.contains(&idx) { | ||
| continue; | ||
| } | ||
| if collapsed_skip.contains(&idx) { | ||
| continue; | ||
| } | ||
| // Replace the first cell of a collapsed tool run with | ||
| // a compact summary cell. | ||
| if let Some(run) = tool_runs | ||
| .iter() | ||
| .find(|r| r.start == idx && !app.expanded_tool_runs.contains(&r.start)) | ||
| { |
There was a problem hiding this comment.
Inside the loop over app.history, calling tool_runs.iter().find(...) results in
Recommendation: Pre-build a HashMap<usize, &ToolRun> of collapsed tool runs before the loop to achieve
let collapsed_runs: std::collections::HashMap<usize, &crate::tui::history::ToolRun> = tool_runs
.iter()
.filter(|r| !app.expanded_tool_runs.contains(&r.start))
.map(|r| (r.start, r))
.collect();
for (idx, cell) in app.history.iter().enumerate() {
if app.collapsed_cells.contains(&idx) {
continue;
}
if collapsed_skip.contains(&idx) {
continue;
}
// Replace the first cell of a collapsed tool run with
// a compact summary cell.
if let Some(run) = collapsed_runs.get(&idx)![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| app.expanded_tool_runs.insert(original_idx); | ||
| app.mark_history_updated(); | ||
| return true; |
There was a problem hiding this comment.
mark_history_updated()
app.expanded_tool_runs.insert(original_idx) is called on line 731, but app.mark_history_updated() on line 732 unconditionally calls self.expanded_tool_runs.clear() (see app.rs line 2620). The insert is wiped out before the widget ever re-renders, so clicking a collapsed run will never expand it — the summary row always stays collapsed. The collapse direction (lines 721-723) is fine because the remove happens before the clear.
| let mut ok_count = 0usize; | ||
| let mut failed_count = 0usize; | ||
| while i < history.len() && is_collapsible_tool(&history[i]) { | ||
| if let HistoryCell::Tool(tc) = &history[i] { | ||
| let name = tool_display_name(tc); | ||
| if !tool_names.contains(&name) { | ||
| tool_names.push(name); | ||
| } | ||
| if tc.is_success() { | ||
| ok_count += 1; | ||
| } else if tc.is_failed() { | ||
| failed_count += 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
failed_count is always 0; ToolRun carries misleading fields
is_collapsible_tool only returns true for cells where is_success() && !is_collapsible_guard(), so the inner loop body can never reach the else if tc.is_failed() branch. failed_count will always be 0 and the "N ok, M failed" branch in tool_run_summary is dead code. Similarly, has_running is hardcoded false. These fields should either be removed or the doc-comment updated to clarify that a "run" is by construction all-successful.
| let tool_runs = if app.tool_collapse_threshold > 0 { | ||
| crate::tui::history::detect_tool_runs(&app.history, app.tool_collapse_threshold) | ||
| } else { | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
detect_tool_runs called redundantly on every click
detect_tool_runs is called here (on click) and again during every render in ChatWidget::new. For a long history the two O(n) passes on each click are redundant. Consider caching the run-list alongside history_version so click handling and rendering share the same computed result.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…dated (Hmbown#2740) - Remove `expanded_tool_runs.clear()` from `mark_history_updated()` which was called by `toggle_tool_run_expand`, causing immediate re-collapse. - Clear expanded runs in the correct lifecycle hooks: new session, load session, and history trimming. - Guard `collapsed_skip` allocation behind `!tool_runs.is_empty()` to avoid an unnecessary HashSet allocation when no runs are detected. - Apply `retain` to `expanded_tool_runs` when history is truncated so stale indices are dropped. Closes Hmbown#2740 review comments.
| fn is_running(&self) -> bool { | ||
| self.status() == Some(ToolStatus::Running) | ||
| } | ||
|
|
||
| fn status(&self) -> Option<ToolStatus> { | ||
| match self { | ||
| ToolCell::Exec(c) => Some(c.status), | ||
| ToolCell::Review(c) => Some(c.status), | ||
| ToolCell::WebSearch(c) => Some(c.status), | ||
| ToolCell::Generic(c) => Some(c.status), | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
McpToolCell omitted from status(), breaking run detection for MCP tools
McpToolCell has pub status: ToolStatus just like Exec, WebSearch, and Generic, but the status() match arm covers only those three plus Review. As a result is_success() always returns false for ToolCell::Mcp, so successful MCP calls are never eligible for collapse and silently break what would otherwise be a continuous run.
|
Thanks @idling11 — this follow-up is superseded by the local #2738 harvest that is now on the public v0.9.0 integration branch ( The landed #2738 slice covers the common dense tool-run collapse direction for #2692, including normal rendering preservation, Enter/Space/mouse expansion, compact/expanded/calm modes, MCP grouping/name handling, and full-detail index mapping for copy/view paths. The reviewed #2740 head still had common-case collapse and MCP grouping/name issues, so it was not merged directly. The integration branch credits you for the #2692 direction in the execution map and changelog. Closing this PR as superseded by the credited integration-branch harvest, not as rejected work. |
2 participants
Add a transcript feature that groups contiguous successful tool calls
(exceeding a configurable threshold) into a single compact summary row.
Users can click the summary to expand it back to individual cells.
Core changes:
history.rs:is_success,is_failed,is_running,is_collapsible_guardhelper methods to
ToolCell.ToolRun,detect_tool_runs(),is_collapsible_tool(),tool_display_name(),tool_run_summary()for run grouping.app.rs:tool_collapse_threshold: usize(default 3, 0 disables).expanded_tool_runs: HashSet<usize>for click-to-toggle state.mark_history_updated().widgets/mod.rs:app.historybefore rendering: detect tool runs,replace collapsed runs with a single
"▼ N tools collapsed"summary cell, skip collapsed members from the output.
mouse_ui.rs:toggle_tool_run_expand()— click on a collapsed summarytoggles expansion; click again to re-collapse.
settings.rs:tool_collapse_thresholdsetting (serde default 3).Appinitialization.Non-goals for this PR (can be follow-ups):
Closes #2692
Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist
Greptile Summary
This PR adds a transcript feature that groups consecutive successful tool calls (≥
tool_collapse_threshold, default 3) into a single expandable summary row, with click-to-toggle expansion state stored inexpanded_tool_runs.history.rsgainsToolRun,detect_tool_runs, and helpers;app.rsaddstool_collapse_threshold+expanded_tool_runs;widgets/mod.rssynthesises summary cells;mouse_ui.rswires the click handler.ChatWidget::new, which is only entered whenapp.collapsed_cellsis non-empty — meaning in a typical session with no folded thinking cells the feature silently never fires.McpToolCellhas astatusfield but is omitted fromToolCell::status(), so successful MCP calls returnis_success() = false, they can never be grouped, and they silently split otherwise-continuous runs.Confidence Score: 3/5
The core collapse feature does not work in the common case and MCP tool calls are excluded from grouping; both defects need fixes before this can be merged.
The slow path that contains all tool-run collapse logic is only entered when app.collapsed_cells is non-empty — a set that tracks folded thinking cells, not tool runs. In a normal session with no folded thinking cells, detect_tool_runs is never called, no summary cells are ever produced, and the feature silently does nothing. Separately, McpToolCell has a status field identical to Exec/Generic/WebSearch but is omitted from the status() dispatch, so successful MCP calls always return is_success() = false, are excluded from every run, and silently break continuity in mixed-tool sequences.
crates/tui/src/tui/widgets/mod.rs (fast-path bypass) and crates/tui/src/tui/history.rs (Mcp omission from status())
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant mouse_ui as mouse_ui.rs participant App as App (state) participant Widget as ChatWidget (render) User->>mouse_ui: click transcript row mouse_ui->>App: transcript_cell_index_from_mouse() App-->>mouse_ui: filtered_idx mouse_ui->>App: collapsed_cell_map[filtered_idx] App-->>mouse_ui: "original_idx (= run.start)" alt Already expanded mouse_ui->>App: expanded_tool_runs.remove(original_idx) else Collapsed → expand mouse_ui->>App: detect_tool_runs(history, threshold) App-->>mouse_ui: tool_runs mouse_ui->>App: expanded_tool_runs.insert(original_idx) end mouse_ui->>App: "mark_history_updated() → needs_redraw = true" User->>Widget: render frame alt collapsed_cells is EMPTY (fast path — common case) Widget->>Widget: tool-run collapse logic skipped entirely Widget->>App: "collapsed_cell_map = identity mapping" else collapsed_cells non-empty (slow path) Widget->>App: detect_tool_runs(history, threshold) App-->>Widget: tool_runs Widget->>Widget: build collapsed_skip set Widget->>Widget: emit summary cell OR individual cells Widget->>App: "collapsed_cell_map = filtered_to_original" endReviews (2): Last reviewed commit: "fix(transcript): move expanded_tool_runs..." | Re-trigger Greptile