feat(tui): partition tool display by type — collapse read/search, show mutation tools individually#5661
Merged
Merged
Conversation
chiga0
commented
Jun 22, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
Author
There was a problem hiding this comment.
Overview
Final Verdict: LGTM with one Minor — Well-designed unification of tool output rendering. The buildToolSummary() category-based approach with tense support is clean, the border removal simplifies the visual hierarchy, and test coverage is comprehensive (10 dedicated buildToolSummary unit tests + updated component tests + snapshot updates).
Findings
- Minor: 1 item
- Nit: 0 items
Minor
i18n gap in buildToolSummary (CompactToolGroupDisplay.tsx)
The function uses hardcoded English verbs ("Read", "Edited", "Ran", "Searched", etc.) and noun forms ("file/files", "command/commands"). The old code used localizeToolDisplayName() from i18n/index.js, and ToolMessage.tsx still uses it for individual tool names. When tools are displayed in the new unified collapsed view, the summary line won't be localized.
For the initial launch this may be acceptable if English-only TUI is the target, but worth tracking as a follow-up if i18n is a priority. The fix would be to extract verb/noun strings into the i18n system or use localizeToolDisplayName for the tool name portion.
Key Observations
The design is solid — replacing the dual-mode toggle with always-collapsed completed tools eliminates unnecessary cognitive overhead. The semantic summary ("Read 3 files, edited 2 files, ran 1 command") is significantly more informative than the old "ReadFile × 3" format. Category-based grouping with CATEGORY_ORDER ensures commands appear first, which matches user priorities.
The border removal is a clean visual simplification. The Ink rendering bug comment that motivated the width constraint is still relevant (non-border Box still needs width to prevent layout issues), and the code correctly preserves it.
The showCompact condition change (compactMode || allComplete) is the right approach — it ensures the compact summary shows for completed groups regardless of mode, while still respecting the compact mode toggle for in-progress groups.
Note: PR is currently in CONFLICTING state — merge conflicts need resolution before merge.
This review was generated by QoderWork AI
Collaborator
Author
Code review: unified tool output / semantic summaries我按原始需求（“不要再区分精简/详细模式，完成后的工具调用尽量展示类似 阻塞/高优先级问题
中低优先级问题
验证结果我在 PR worktree 上做了以下验证：
总体结论这版 PR 的方向符合需求，但还没有严格 follow 设计，也没有完全保护现有特殊语义。建议先处理 merge conflict，然后修复 summary 重复显示、memory-only 被吞、canceled 行为不一致这三类核心问题；最后补上上述边界测试，再重新跑目标组件测试和可用的类型/组件验证。 |
chiga0
commented
Jun 22, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
Author
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Remove round borders from ToolGroupMessage, CompactToolGroupDisplay, and InlineParallelAgentsDisplay. Completed tools now default to a single collapsed header line with dimColor styling. Executing/error/confirming tools continue to show their full result block. Part of #4588 (Track 3: Simplify tool-call rendering). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Only collapse completed tool results in compact mode, preserving
full visibility in non-compact mode
- Subtract 2 from innerWidth to account for ToolMessage paddingX={1}
- Update snapshots to reflect removed borders
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Gate isDim on compact mode so non-compact tools stay fully styled
- Add paddingX={1} to CompactToolGroupDisplay for left-edge alignment
- Delete Border Color Logic test block (borders removed)
- Add compact-mode test coverage for Error/Executing/Pending/forceShowResult
- Clean up stale border references in comments
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Replace the dual compact/normal mode tool output with a single unified
mode. Completed tools always show a semantic overview line
("Read 3 files, edited 2 files") instead of dumping full results.
- Add buildToolSummary() for category-based semantic summaries
- Remove compactMode gate from shouldCollapse and isDim in ToolMessage
- Make all-completed tool groups use CompactToolGroupDisplay
- Remove unused useCompactMode hook calls from ToolMessage
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Add 10 dedicated unit tests for buildToolSummary covering edge cases - Fix stale comment referencing old compactMode gate logic Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Add Canceled status to allComplete check in ToolGroupMessage - Move memory-only group rendering before showCompact to prevent them being swallowed by CompactToolGroupDisplay - Fix LLM summary duplication: absorbedCallIds now tracks completed groups in non-compact mode; HistoryItemDisplay no longer bypasses summaryAbsorbed when !compactMode - Update StandaloneSessionPicker test for new compact rendering - Fix design doc category order example and add missing rendering rules Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
force-pushed
the
worktree-feat+tui-tool-collapse
branch
from
1915d3d to
4146279
Compare
- Add SHELL_COMMAND_NAME and @ file-reference pseudo-tools to TOOL_NAME_TO_CATEGORY mapping for correct category classification - Fix height calculation test to use Executing status so expanded path is actually exercised - Update stale comment about empty toolCalls behavior Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
|
All findings have been addressed: Blocking/High priority — All fixed
Medium/Low priority
Inline comments
|
Collaborator
Build failure root cause — one line blocks every checkAll six red checks (Lint, Test ×3 on ubuntu/macos/windows, Integration Tests (No-AK Smoke), Post Coverage Comment) come from a single TypeScript compile error, not six independent problems. Root cause — const { compactMode } = useCompactMode();
Why it takes down everything — every job runs
Fix the one line and all six should go green. Suggested fix — remove the dead read: - const { compactMode } = useCompactMode();
-import { useCompactMode } from '../contexts/CompactModeContext.js';The If instead 中文版构建失败根因 —— 一行代码阻断了所有检查六个红叉（Lint、ubuntu/macos/windows 三个 Test、Integration Tests (No-AK Smoke)、Post Coverage Comment）全部源于同一个 TypeScript 编译错误，而不是六个独立问题。 根因 —— const { compactMode } = useCompactMode();
为什么会拖垮全部检查 —— 每个 job 在做正事之前都会先跑
修掉这一行，六个检查应该都会转绿。 建议修复 —— 删掉这处无用的读取： - const { compactMode } = useCompactMode();
-import { useCompactMode } from '../contexts/CompactModeContext.js';本文件里 如果 |
Fixes CI build failure caused by TS6133 (noUnusedLocals) — the compactMode destructure became dead code after the summary gating was moved to summaryAbsorbed. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
|
Fixed in 500200c — removed the dead |
Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Update — the
|
| commit | role | compactMode in HistoryItemDisplay.tsx |
|---|---|---|
3275994e8 |
old head the stale merge ref built | 1 ← the error |
500200c5 |
your fix | 0 |
089ffaf63 |
current head | 0 |
Your re-trigger was the right call. The empty commit 089ffaf63 (ci: trigger re-run with updated merge ref) forced GitHub to recompute the merge ref. It is now bc3aea739 = Merge 089ffaf63 into d350dd8df (current main), which contains the fix.
Verified green build. I checked out that exact current merge ref bc3aea739 and ran the full npm run build — it completes with exit code 0 (core → cli → webui → web-shell → everything; 0 TS errors). So the build is genuinely fixed; the fresh run for 089ffaf63 should pass.
One unrelated note: the Test (ubuntu) job in the superseded run failed at its Checkout step (git ... not our ref, tests never ran) — that's the same transient merge-ref fetch race seen on other PRs, not a test failure. The re-run should clear it too.
Net: no further code change needed for the build failure — it's resolved.
中文版
更新 —— compactMode 修复是对的；后面仍然红是 refs/pull/5661/merge 陈旧,不是 bug 复发
接我之前那条根因评论。500200c5（fix(tui): remove unused compactMode import）这个修复是正确的 —— 我在本地构建过,能通过。但之后 CI 又报了同一个 HistoryItemDisplay.tsx(217,9): error TS6133: 'compactMode' 错误,看上去就像"修复没生效"。其实生效了 —— 真实情况如下。
CI 构建的是修复前的陈旧代码。 在 pull_request 事件里,actions/checkout 检出的是 refs/pull/5661/merge（合并提交),不是你的 head。在那个名义上对应 500200c5 的 run 里,checkout 实际解析到了:
HEAD is now at 8605662c2 Merge 3275994e8... into 2eb3b803...
3275994e8 是修复前的旧 head（fix(tui): address inline review findings)—— 它里面还有 compactMode。GitHub 当时还没把 merge ref 重新生成到包含 500200c5,于是 runner 编译了旧提交,又报出一模一样的 TS6133。已核验:
| 提交 | 角色 | HistoryItemDisplay.tsx 里 compactMode 数量 |
|---|---|---|
3275994e8 |
陈旧 merge ref 实际构建的旧 head | 1 ← 报错根源 |
500200c5 |
你的修复 | 0 |
089ffaf63 |
当前 head | 0 |
你那个重新触发的操作是对的。 空提交 089ffaf63（ci: trigger re-run with updated merge ref)强制 GitHub 重算了 merge ref。现在它是 bc3aea739 = Merge 089ffaf63 into d350dd8df（当前 main),里面包含了修复。
已验证构建通过。 我检出了这个当前 merge ref bc3aea739,跑了完整的 npm run build —— 退出码 0（core → cli → webui → web-shell → 全部;0 个 TS 错误)。所以构建确实修好了;089ffaf63 的新 run 应该会通过。
一个无关的补充: 被取代的那个 run 里的 Test (ubuntu) job 是挂在 Checkout 步骤（git ... not our ref,测试根本没跑)—— 那是其他 PR 上也见过的那种短暂 merge-ref 拉取竞态,不是测试失败。重跑应该也会一并消除。
结论:构建失败无需再改任何代码 —— 已解决。
chiga0
changed the title
Collaborator
Author
|
Thanks for the thorough investigation. Confirmed — the stale merge ref race was the root cause. The new run (triggered by |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
requested changes
Jun 23, 2026
Align tool display with read/search collapse pattern: only information-gathering tools (read, search, list) are collapsed into a summary line; mutation tools (edit, write, command, agent) always render individually with their results. - Remove compactLabel prop and cross-group merge logic from MainContent - Add isCollapsibleTool predicate and partition in ToolGroupMessage - Remove shouldCollapse gate from ToolMessage (results always shown) - Update CATEGORY_ORDER to search → read → list → command → ... - Update tests and snapshots to match partition-based rendering Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Only string and ANSI results are hidden for completed (Success/Canceled) tools — diff, plan, todo, and task results always render since they carry non-repeatable information the user needs to review inline. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
changed the title
- Fix TS2740: add required AnsiToken properties to ANSI collapse test - Rename mergedLengthRef → visibleHistoryLengthRef to match removed mergedHistory concept - Fix design doc Rule 5: forceExpandAll forces results only for triggering tools, not all siblings - Update compactToggleHasVisualEffect JSDoc: tool_group no longer checked, only gemini_thought items affected by compact mode - Fix diff bypass test: use collapsible tool name (ReadFile) so shouldCollapseResult is actually exercised - Add all-collapsible memory badge test covering the early-return path Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] The compactMode setting description in packages/cli/src/config/settingsSchema.ts (line ~958) still reads "Hide tool output and thinking for a cleaner view (toggle with Ctrl+O)". After this PR, compact mode no longer hides tool output — that's now handled by the type-based isCollapsibleTool() partition, independent of compactMode. Compact mode only affects gemini_thought / gemini_thought_content blocks. The description should be updated to reflect this, e.g., "Hide thinking blocks for a cleaner view (toggle with Ctrl+O)".
— qwen3.7-max via Qwen Code /review
wenshao
previously approved these changes
Jun 25, 2026
Collaborator
|
@qwen-codee /triage |
wenshao
reviewed
Jun 25, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
R5: No new issues found. All R4 suggestions (Canceled test gap, legacy tool name tests, memory badge mixed-group test) have been addressed. Deterministic analysis clean (tsc + eslint 0 findings). All 135 tests pass. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Build passes (0 errors), all 229 relevant tests pass, typecheck and lint clean. 2 test coverage suggestions below — no blockers.
— qwen3.7-max via Qwen Code /review
- Add test: user-initiated group renders all collapsible tools individually (bypasses partition into summary line) - Add test: memory-only group with errored tool falls through to expanded path instead of showing compact badge Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
approved these changes
Jun 25, 2026
Collaborator
Local runtime verification — PR #5661Verdict: functionally verified. The type‑based partition behaves exactly as designed across every scenario I drove through the real built bundle (not just vitest). Unit tests pass and are load‑bearing, the merged tree is type‑clean, and a BEFORE/AFTER bundle A/B in tmux shows the intended collapse/expand split. One design trade‑off is flagged for the merge decision (non‑blocking, see Notes). Method & environment
1. Runtime A/B (the headline)Same prompts, same mock, same terminal — only the bundle differs. Scenario COLLAPSE — one assistant message calling → 4 individual rows collapse to one category‑ordered summary line ( Scenario MIXED — → Mixed group = summary line for the read + the Shell rendered individually with its output. ✅ Scenario MUTATE — → Non‑collapsible tools are unchanged — always individual, results stay visible. ✅ Scenario ERRGROUP — a → An errored collapsible tool triggers
2. Unit tests — all PR‑touched suites pass (merged tree)
(The first AppContainer run failed only to load because the 3. Typecheck — clean
4. Revert‑proof — the new tests are load‑bearingKeeping the PR's tests but swapping a source file back to
So the tests genuinely exercise the new behavior rather than passing vacuously. Notes (non‑blocking, for the merge decision)
Verified locally on the real esbuild bundle via tmux + a mock provider. Happy to share the mock/driver or re‑run any scenario. 🇨🇳 中文说明（点击展开）本地真实运行验证 — PR #5661结论：功能验证通过。 在真实构建产物（esbuild bundle，而非仅 vitest）上把每个场景都跑了一遍，按类型分区的行为完全符合设计；单测全部通过且“可证伪”（reverts 会让新测试失败），合并后的代码 方法与环境
1. 运行期 A/B（核心证据）
2. 单测：PR 改动涉及的 6 个测试文件全绿（19 + 41 + 45 + 94 + 51）。AppContainer 首次失败只是因为 3. 类型检查：构建好无关的 4. 可证伪（revert‑proof）：保留 PR 测试、把单个源文件换回
注意事项（非阻塞，供合并参考）
（以上均在真实 esbuild 产物上经 tmux + mock provider 验证；如需 mock/驱动脚本或复跑任意场景，我可以提供。） |
|
|
||
| // Priority: Confirming > Executing > Error > Canceled > Pending > Success | ||
| function getOverallStatus( | ||
| export function getOverallStatus( |
Collaborator
There was a problem hiding this comment.
[Suggestion] getOverallStatus was changed from private to export function, but no other file imports it — grep confirms only 2 matches (declaration + internal call site at line 240). The export appears unnecessary unless a specific external consumer is planned.
Suggested change
| export function getOverallStatus( | |
| function getOverallStatus( |
— qwen3.7-max via Qwen Code /review
| )} | ||
| <Text wrap="truncate-end" bold> | ||
| {buildToolSummary(toolCalls, isActive)} | ||
| {isActive && <Text key="ellipsis">…</Text>} |
Collaborator
There was a problem hiding this comment.
[Suggestion] No component-level test renders CompactToolGroupDisplay with an executing/pending/confirming tool and asserts the … character appears in the output. All existing component tests use ToolCallStatus.Success tools. If the ellipsis rendering is ever accidentally removed (e.g., during a future refactor of the active-state logic), no test would catch it.
Consider adding a test in CompactToolGroupDisplay.test.tsx:
it('renders ellipsis when tools are still active', () => {
const tools = [makeTool({ status: ToolCallStatus.Executing })];
const { lastFrame } = render(<CompactToolGroupDisplay toolCalls={tools} contentWidth={80} />);
expect(lastFrame()).toContain('…');
});— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code-ci-bot /triage |
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 25, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| }); | ||
|
|
||
| it('shows result when forceShowResult overrides compact collapse', () => { | ||
| it('shows result when forceShowResult overrides collapse', () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] This test uses baseProps which has name: 'test-tool'. Since isCollapsibleTool('test-tool') returns false, shouldCollapseResult is false regardless of forceShowResult. The assertion (toContain('MockMarkdown:Test result')) passes trivially — the result is never collapsed for non-collapsible tools.
The actual override behavior — where forceShowResult=true prevents collapse of a collapsible tool that WOULD otherwise be collapsed — is never tested. A regression that removes the !forceShowResult gate in shouldCollapseResult would go undetected.
it('shows result when forceShowResult overrides collapse', () => {
const { lastFrame } = renderWithContext(
<ToolMessage {...baseProps} name="ReadFile" forceShowResult />,
StreamingState.Idle,
);
expect(lastFrame()).toContain('MockMarkdown:Test result');
});— qwen3.7-max via Qwen Code /review
| // to reduce scrollback noise. Non-collapsible tools (command/edit/agent/MCP/etc.) | ||
| // always show results — their output IS the answer. Canceled tools keep partial | ||
| // output visible. Diff, plan, todo, task results always render regardless. | ||
| const shouldCollapseResult = |
Collaborator
There was a problem hiding this comment.
[Suggestion] No test verifies that a Canceled collapsible tool keeps its string/ANSI result visible. The existing Canceled test at line ~297 uses name: 'test-tool' (non-collapsible), so shouldCollapseResult is false regardless of status.
The status === ToolCallStatus.Success gate here was deliberately narrowed from isCompleted (which initially included Canceled). If someone accidentally broadens it back, no test catches the regression — partial output from interrupted read/search tools would silently disappear.
Suggested test:
it('shows result for Canceled collapsible tool (partial output preserved)', () => {
const { lastFrame } = renderWithContext(
<ToolMessage {...baseProps} name="ReadFile" description="big.ts"
status={ToolCallStatus.Canceled} />,
StreamingState.Idle,
);
expect(lastFrame()).toContain('MockMarkdown:Test result');
});— qwen3.7-max via Qwen Code /review
| // must see full details: confirmation prompts, errors, user-initiated | ||
| // batches, focused shells, terminal subagents. | ||
| const hasTerminalSubagent = inlineToolCalls.some(isTerminalSubagentTool); | ||
| const forceExpandAll = |
Collaborator
There was a problem hiding this comment.
[Suggestion] isEmbeddedShellFocused and hasSubagentPendingConfirmation are two of six forceExpandAll triggers, but neither has a test in ToolGroupMessage.test.tsx verifying they bypass the collapsible/non-collapsible partition. The other four triggers (hasErrorTool, isUserInitiated, hasConfirmingTool, hasTerminalSubagent) all have explicit partition-bypass tests.
If a refactor drops one of these conditions, collapsible tools in an embedded-shell-focused or subagent-pending-confirmation group would silently collapse into a summary line instead of rendering individually.
Suggested: add two tests similar to the existing isUserInitiated test — asserting that all collapsible tools render as MockTool[...] entries instead of a summary line.
— qwen3.7-max via Qwen Code /review
| /* marginBottom */ 1 + collapsibleSummaryHeight + memoryBadgeHeight; | ||
| const innerWidth = contentWidth - 2; | ||
|
|
||
| let countToolCallsWithResults = 0; |
Collaborator
There was a problem hiding this comment.
[Suggestion] When forceExpandAll=true, collapsible tools are routed to nonCollapsibleTools. This loop counts them in countToolCallsWithResults if they have a resultDisplay. However, shouldCollapseResult in ToolMessage still hides their output for non-triggering siblings (forceShowResult is per-tool, narrower than forceExpandAll).
The denominator countToolCallsWithResults is inflated — it includes tools whose results won't render — stealing height budget from non-collapsible tools (Shell, Edit) whose output IS the diagnostic content. In groups with many collapsible tools and few non-collapsible ones (e.g., a subagent that read 5 files then ran 1 shell command that failed), the Shell output gets truncated more than necessary.
Fix: reclassify collapsible tools whose results will be collapsed as one-line tools in the height budget calculation.
— qwen3.7-max via Qwen Code /review
| | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `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 | |
Collaborator
There was a problem hiding this comment.
[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.
Suggested change
| | `ToolGroupMessage.tsx` | Type-based partition replaces `showCompact`; `forceShowResult` simplified to `forceExpandAll`; height budget accounts for collapsible summary row | | |
| | `ToolGroupMessage.tsx` | Type-based partition replaces `showCompact`; `forceShowResult` uses per-tool computation (Confirming, Error, agent-pending, terminal-subagent); height budget accounts for collapsible summary row | |
— qwen3.7-max via Qwen Code /review
| SearchFiles: 'search', | ||
| FindFiles: 'search', | ||
| ReadFolder: 'list', | ||
| Task: 'agent', |
Collaborator
There was a problem hiding this comment.
[Suggestion] TodoWrite uses a bare literal key while every other current tool name uses [ToolDisplayNames.X] computed keys. ToolDisplayNames.TODO_WRITE resolves to 'TodoList', which is currently unmapped — functionally equivalent (both 'other') but inconsistent. If 'other' ever gets different properties, 'TodoList' (current name) and 'TodoWrite' (old session name) would silently diverge.
Suggested change
| Task: 'agent', | |
| [ToolDisplayNames.TODO_WRITE]: 'other', | |
| // Legacy display names (from ToolDisplayNamesMigration) | |
| SearchFiles: 'search', | |
| FindFiles: 'search', | |
| ReadFolder: 'list', | |
| Task: 'agent', | |
| TodoWrite: 'other', |
— qwen3.7-max via Qwen Code /review
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jun 26, 2026
Resolve conflicts by rebasing onto QwenLM#5661's merged type-based tool partition (the branch was built on an earlier state-based snapshot of QwenLM#5661 that diverged during its own review). Per the PR's two audit comments: - Tool partition: keep main's type-based model (isCollapsibleTool / forceExpandAll — collapse read/search/list, render mutation tools individually). Drop the branch's state-based showCompact/allComplete rewrite and the compactLabel summary-absorption machinery (absorbedCallIds / getCompactLabel / isSummaryAbsorbed); tool_use_summary renders as a standalone line, matching main. - fullDetail (Ctrl+O transcript): layered onto the type-based base — fullDetail forces forceExpandAll=true (skip partition) + per-tool forceShowResult=true + uncapped height. No new shouldForceFullDetail predicate; reuse forceExpandAll. - Remove global compact mode: CompactModeContext + mergeCompactToolGroups deleted; test scaffolding uses a local no-op CompactModeProvider. Touched suites green (ToolGroupMessage/ToolMessage/CompactToolGroupDisplay, MainContent, HistoryItemDisplay, TranscriptView, StandaloneSessionPicker, AppContainer, SettingsDialog, keyMatchers — 355 tests). Design doc (docs/design/ctrl-o-detail-expand) still describes the state-based baseline and needs a follow-up rewrite to type-based. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jun 26, 2026
…tion The design doc was written against an early state-based snapshot of QwenLM#5661 (showCompact = (compactMode || allComplete), whole-group collapse) and even asserted that forceExpandAll / isCollapsibleTool "don't exist". The merged QwenLM#5661 is type-based partition and those symbols are its core. Rewrite the affected sections to match the shipped baseline: - §1/§2: baseline described as type-based partition (collapse read/search/list via isCollapsibleTool, render mutation tools individually); compactMode no longer affects tool rendering. Added a revision note. - §3.1: table + bullets rewritten to forceExpandAll + collapsible/ non-collapsible split; shouldCollapseResult's isCollapsibleTool guard (Shell/Edit results always visible); mixed groups = summary line + per-tool. - §4.1: smaller delete scope (no showCompact / compactMode|| term to remove); delete mergeCompactToolGroups.ts; keep web-shell ui.compactMode passthrough. - §4.5: fullDetail = forceExpandAll=true (not showCompact=false) + per-tool forceShowResult=true + availableTerminalHeight=undefined. - §4.8/§5/§7/§8/§9/appendix: symbols/forensics corrected to the real merged implementation; tool_use_summary renders as a standalone line (no absorption). Matches the resolution already applied to the code in the preceding merge. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jun 26, 2026
Three independent audits confirmed the doc is now faithful to the merged QwenLM#5661 type-based partition; they surfaced three concrete fixes: - CATEGORY_ORDER: corrected to the real array order search/read/list/command/edit/write/agent/other (was listed as command/read/edit/write/search/list/agent/other). - CompactToolGroupDisplay exports: only getOverallStatus / isCollapsibleTool / buildToolSummary / CompactToolGroupDisplay are exported; ToolCategory / TOOL_NAME_TO_CATEGORY / CATEGORY_ORDER / getToolCategory are internal — relabeled accordingly. - §5.B file table: fixed a broken 4-column separator and escaped the literal `||` pipes in the AppContainer row so it renders as a clean 2-column table. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jun 26, 2026
…w-up) Latest audit confirms the technical design is implementable and side-effect coverage is sufficient; it flagged status/scope inconsistencies for the doc to serve as an acceptance baseline. Fixes: 1. Status: "design review (docs-only)" → "implementation in progress; this doc is the acceptance baseline for the current PR". Added an implemented-vs-pending status table. 2. Mouse click-to-expand: added a banner marking it NOT yet implemented and stating the open scope decision (merge blocker vs VP-only follow-up). 3. QwenLM#5751 (and QwenLM#5661) dependency: corrected from "OPEN, must merge first" to "already merged into main; branch rebased on top". 4. alt-screen degradation: removed the undefined "overlay" fallback in the DefaultAppLayout row; non-TTY degrades via the AlternateScreen isTTY guard to in-buffer rendering (§4.2), no separate overlay path. 5. Fixed a broken bold marker (`\*\*`) in the AppContainer row. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jun 26, 2026
Assessed the mouse click-to-expand effort against the real code: it's ~250–400 lines across 4–5 files (ToolExpandedContext + AppContainer wiring + a ClickableToolMessage component — can't call useMouseEvents inside the .map() — + ToolGroupMessage wiring + mouse hit-test tests). More importantly, under QwenLM#5661's type-based partition the collapsed read/search tools are aggregated into a single summary line, so there is no per-tool click target — the click granularity must be redesigned to "click the summary row → expand the whole group". Plus the known SGR-mouse vs native text-selection risk. Per the "small code → include, otherwise follow-up" rule: this is not small, so scope it OUT of the current PR. The current PR delivers Ctrl+O transcript only. Marked §1 goal QwenLM#4, §4.8 (banner + draft), §9 commit 4, and the status table accordingly; the §4.8 design is kept as a draft for the follow-up PR. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What this PR does
Replaces the binary compact/full rendering model with a type-based partition:
Key behaviors:
buildToolSummary()generates human-readable descriptions with past/present tense verbsisCollapsibleTool()predicate determines tool category — also gatesshouldCollapseResultin ToolMessage so adding a category suppresses both grouping AND result outputWhy it's needed
The previous approach collapsed ALL completed tools into a summary (compact mode) or showed ALL results inline (full mode). Neither matched the ideal: read/search output is rarely useful in scrollback, but edit diffs and shell errors need to stay visible. The type-based partition gives the best of both.
Related PRs
Reviewer Test Plan
How to verify
npm run devand execute a multi-step task (e.g., "read package.json, grep for imports, edit a file, run tests").cd packages/cli && npx vitest run src/ui/components/messages/CompactToolGroupDisplay.test.tsx src/ui/components/messages/ToolGroupMessage.test.tsx src/ui/components/messages/ToolMessage.test.tsxTested on
Risk & Scope
'other'category → "Used N tools" until the name→category map is updated.mergeCompactToolGroupsutility is now dead code (onlycompactToggleHasVisualEffectis still imported by AppContainer). Cleanup deferred to feat(tui): Ctrl+O frozen transcript view and unified tool output rendering #5666.中文说明
这个 PR 做了什么
将工具展示从二元模式（compact/full）改为按类型分区：
关联 PR