-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(tui): partition tool display by type — collapse read/search, show mutation tools individually #5661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
feat(tui): partition tool display by type — collapse read/search, show mutation tools individually #5661
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
6d5fa82
feat(tui): remove tool group borders and collapse completed tool results
qinqi-ali a0b018c
fix(tui): gate collapse on compact mode and fix innerWidth calculation
qinqi-ali 9948cbe
fix(tui): address review feedback on collapse and visual alignment
qinqi-ali ea80b74
feat(tui): unify tool output with semantic summaries
qinqi-ali 89faa9c
test(tui): add buildToolSummary unit tests and fix stale comment
qinqi-ali 4146279
fix(tui): address audit findings for unified tool output
qinqi-ali 3275994
fix(tui): address inline review findings
qinqi-ali 500200c
fix(tui): remove unused compactMode import in HistoryItemDisplay
qinqi-ali 089ffaf
ci: trigger re-run with updated merge ref
qinqi-ali f496048
refactor(tui): partition tools by type instead of completion status
qinqi-ali 39f5076
fix(tui): collapse text/ANSI output for completed tools
qinqi-ali 480f7db
fix(tui): unify summary format and show results in error-expanded groups
qinqi-ali 7cbf39f
test(tui): improve coverage for partition logic and result collapse
qinqi-ali 2e33ae7
fix(tui): align CompactToolGroupDisplay style with ToolInfo
qinqi-ali 626f89f
fix(tui): address review findings on result collapse and memory safety
qinqi-ali ccda337
docs(tui): sync design doc with actual implementation
qinqi-ali 50ea752
test(tui): update AppContainer test for compactToggleHasVisualEffect …
qinqi-ali a0f1ed3
Merge branch 'main' into worktree-feat+tui-tool-collapse
wenshao 5ccbcd4
fix(tui): restore per-tool forceShowResult and harden edge cases
qinqi-ali 6a50310
test(tui): add coverage for canceled partition, memory badge, and leg…
qinqi-ali 3ab8365
fix(tui): document dual effect of isCollapsibleTool and add ANSI coll…
qinqi-ali b58839c
Merge branch 'main' into worktree-feat+tui-tool-collapse
wenshao b20b744
fix(tui): address review round 6 — type fix, stale refs, and test gaps
qinqi-ali 71e3422
test(tui): add isUserInitiated and memory-only error test coverage
qinqi-ali File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Unified Tool Output Rendering | ||
|
|
||
| ## Background | ||
|
|
||
| The TUI previously had two rendering modes for tool results: | ||
|
|
||
| - **Compact mode** (Ctrl+O): collapsed completed tool results into a one-line summary | ||
| - **Normal mode**: showed full tool results inline, causing excessive vertical noise | ||
|
|
||
| Users had to manually toggle between modes. Most of the time, completed tool results (file contents, search results, etc.) added no value to the conversation flow. | ||
|
|
||
| ## Design | ||
|
|
||
| ### Core Principle | ||
|
|
||
| **One unified mode**: tool rendering is determined by tool category, not by a user-toggled mode. Information-gathering tools (read/search/list) are collapsed into a summary; mutation tools (edit/write/command/agent) always render individually with full results. | ||
|
|
||
| ### Semantic Summary (`buildToolSummary`) | ||
|
|
||
| Instead of showing raw tool names and counts (`ReadFile x 3`), generate human-readable summaries using a count-based format: | ||
|
|
||
| | Scenario | Output | | ||
| | ------------------ | --------------------------------------------- | | ||
| | Single tool | `Read 1 file` / `Ran 1 command` | | ||
| | Multiple same-type | `Read 3 files` | | ||
| | Mixed types | `Ran 1 command, read 3 files, edited 2 files` | | ||
| | Active (executing) | `Reading 1 file` (present progressive) | | ||
| | Completed | `Read 1 file` (past tense) | | ||
|
|
||
| ### Tool Categories | ||
|
|
||
| | Category | Display Names | Past Verb | Active Verb | Collapsible | | ||
| | -------- | ---------------------------- | --------- | ----------- | ----------- | | ||
| | read | ReadFile, Read File(s) | Read | Reading | Yes | | ||
| | edit | Edit, NotebookEdit | Edited | Editing | No | | ||
| | write | WriteFile | Wrote | Writing | No | | ||
| | search | Grep, Glob | Searched | Searching | Yes | | ||
| | list | ListFiles, Read Directory | Listed | Listing | Yes | | ||
| | command | Shell | Ran | Running | No | | ||
| | agent | Agent, Workflow, SendMessage | Ran | Running | No | | ||
| | other | (everything else) | Used | Using | No | | ||
|
|
||
| ### Rendering Rules | ||
|
|
||
| 1. **Type-based partition**: tools are split by `isCollapsibleTool()` — collapsible tools (read/search/list) render as a `CompactToolGroupDisplay` summary line; non-collapsible tools (edit/write/command/agent/other) render individually via `ToolMessage` | ||
| 2. **Memory-only groups** have a dedicated rendering path (read/write counts badge) that takes priority, but only when all ops succeed (`!hasErrorTool && every status === Success`) | ||
| 3. **Result collapse**: only collapsible tools with `Success` status have their text/ANSI output collapsed. Non-collapsible tools (including MCP tools, WebFetch, etc.) always show results. Canceled tools keep partial output visible | ||
| 4. **Tool names** render bold regardless of status, providing consistent styling across both `CompactToolGroupDisplay` and individual `ToolMessage` paths | ||
| 5. **Force-expand conditions**: when any tool in a group is confirming, errored, user-initiated, in a focused shell, or a terminal subagent, ALL tools render individually (no partition) with results forced visible only for the triggering tools (errored, confirming, terminal subagent) — successful siblings keep normal collapse behavior | ||
| 6. **`tool_use_summary`** items (LLM-generated semantic summaries) render unconditionally alongside `CompactToolGroupDisplay`'s mechanical count — they serve different purposes (semantic context vs tool count) | ||
| 7. **Memory badge**: rendered in both the all-collapsible path and the mixed path when memory ops are present in a non-memory-only group | ||
|
|
||
| ### Key Changes | ||
|
|
||
| | File | Change | | ||
| | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `CompactToolGroupDisplay.tsx` | Added `buildToolSummary()` with count format, `isCollapsibleTool()`, removed border styles | | ||
| | `ToolMessage.tsx` | `shouldCollapseResult` gated on `isCollapsibleTool()` and `Success` only; `isDim` removed | | ||
| | `ToolGroupMessage.tsx` | Type-based partition replaces `showCompact`; `forceShowResult` simplified to `forceExpandAll`; height budget accounts for collapsible summary row | | ||
| | `MainContent.tsx` | Removed `mergedHistory` alias, `absorbedCallIds`, `summaryByCallId`, cross-group merging | | ||
| | `HistoryItemDisplay.tsx` | `tool_use_summary` renders unconditionally (removed `summaryAbsorbed` gate) | | ||
| | `mergeCompactToolGroups.ts` | `compactToggleHasVisualEffect` no longer triggers on `tool_group` (compact mode has no effect on tool rendering) | | ||
|
|
||
| ## Alternatives Considered | ||
|
|
||
| 1. **Keep two modes with improved summaries**: Rejected — unnecessary cognitive overhead for users | ||
| 2. **Per-tool summary (Gemini CLI style)**: Each tool gets its own summary arrow. Rejected — still too verbose for large tool batches | ||
| 3. **Phased rollout**: Rejected — user preference for single implementation pass | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The Key Changes table states
forceShowResult simplified to forceExpandAll, but the code was reverted to per-tool computation (commit 5ccbcd4). Rule 5 in the same doc correctly says "results forced visible only for the triggering tools" — the table contradicts it. A future maintainer reading this table will make incorrect assumptions about the implementation.— qwen3.7-max via Qwen Code /review