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
June 23, 2026 02:34
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
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>
pull Bot
pushed a commit
to edisplay/qwen-code
that referenced
this pull request
Jul 9, 2026
…ering (QwenLM#5666) * feat(tui): remove tool group borders and collapse completed tool results 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 QwenLM#4588 (Track 3: Simplify tool-call rendering). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): gate collapse on compact mode and fix innerWidth calculation - 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> * fix(tui): address review feedback on collapse and visual alignment - 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> * feat(tui): unify tool output with semantic summaries 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> * test(tui): add buildToolSummary unit tests and fix stale comment - 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> * fix(tui): address audit findings for unified tool output - 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> * fix(tui): address inline review findings - 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> * fix(tui): remove unused compactMode import in HistoryItemDisplay 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> * ci: trigger re-run with updated merge ref Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): design — remove global compact mode, add Ctrl+O transcript + mouse click-to-expand Design-only. Stacks on QwenLM#5661 (type-based tool partition baseline) and QwenLM#5751 (VP mouse foundation). Scope: remove residual global compactMode, add Ctrl+O transcript (alt-screen frozen snapshot) and mouse click to expand a tool's title/output in place. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(tui): remove global compact mode toggle (on top of QwenLM#5661 partition baseline) Builds on QwenLM#5661's type-based tool partition. Removes only the residual global compactMode switch, keeping the partition baseline intact: - ToolGroupMessage: showCompact = (compactMode || allComplete) → allComplete - delete CompactModeContext, mergeCompactToolGroups (isForceExpandGroup / compactToggleHasVisualEffect no longer used once the cross-group merge and the Ctrl+O toggle are gone) - MainContent: drop the compactMode-gated merge path; mergedHistory = visibleHistory - remove TOGGLE_COMPACT_MODE binding/matcher, ui.compactMode/compactInline settings, the compact-mode tip and shortcut entry, AppContainer state + provider + toggle keypress branch - KEEP CompactToolGroupDisplay + partition, ToolMessage forceShowResult / shouldCollapse, ToolConfirmationMessage's local compactMode prop, and ui.compactMode in WEB_SHELL_SETTINGS (web shell is a separate surface) typecheck + affected suites green (224 tests). Ctrl+O is a temporary no-op until the TranscriptView lands. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(tui): Ctrl+O opens a frozen alt-screen transcript full-detail view Adds the keyboard half of the Ctrl+O redesign on top of the QwenLM#5661 partition baseline: - fullDetail render path (HistoryItemDisplay → ToolGroupMessage): fullDetail composes into thinking `expanded`, and on tool groups forces showCompact=false + forceShowResult=true + uncapped height — so every block renders in full. - new TranscriptView: an AlternateScreen overlay (disabled in VP mode where Ink already owns the alt screen) rendering a frozen snapshot (history length + a pending copy) through ScrollableList with fullDetail, reusing QwenLM#5751's keyboard/wheel/scrollbar scrolling. Adaptive estimatedItemHeight for the taller full-detail rows. - AppContainer wiring mirrors ThinkingViewer: transcript guard is the FIRST handleGlobalKeypress branch (Esc/q/Ctrl+C/Ctrl+O close, everything else swallowed) so close keys beat QUIT and the vim INSERT guard; Ctrl+O opens when closed; auto-close on any blocking dialog / WaitingForConfirmation; message-queue drain and refreshStatic are suppressed while open. - Command.TOGGLE_TRANSCRIPT bound to Ctrl+O. typecheck + 8 suites (268 tests) green. Mouse click-to-expand (per-tool) follows in a later commit. Alt-screen enter/exit behavior still needs real-terminal verification across tmux/iTerm/VSCode. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): repaint normal buffer when transcript closes (no duplicate scrollback) E2E (VHS) caught the design's flagged highest-risk issue: in the legacy <Static> path, closing the alt-screen transcript leaked its full-detail rows into the main scrollback (a duplicate "完整记录 / Transcript" block appeared below the live history). Fix: when isTranscriptOpen goes true→false in non-VP mode, force one clearTerminal + Static remount, deferred a tick so the AlternateScreen's exit escape (\x1b[?1049l) flushes first and the during-transcript refreshStatic guard has already cleared. VP mode keeps its own scrollback via the React tree and is unaffected. Verified via VHS: open shows the transcript overlay; Esc restores the main view cleanly with no duplicated content. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): rebase ctrl-o design doc to QwenLM#5661's type-based partition 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> * docs(tui): fix factual nits from cross-audit of the ctrl-o design doc 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> * fix(tui): don't let fullDetail be bypassed by compact early returns Audit (PR QwenLM#5666) point 2: ToolGroupMessage computed `forceExpandAll = fullDetail || ...` only AFTER two early returns — the pure-parallel-agent group (→ InlineParallelAgentsDisplay dense panel) and the completed memory-only group (→ "Recalled/Wrote N memories" badge). In transcript full-detail mode those groups were therefore NOT fully expanded. Guard both early returns with `!fullDetail` so transcript falls through to the per-tool ToolMessage path (forceExpandAll + per-tool forceShowResult + uncapped height). Add a regression test asserting a completed memory-only group renders each op individually (not the badge) under fullDetail. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): resolve open design decisions from source evidence Settle the two outstanding decision points from the PR audit using the codebase + reference implementations (not preference): - Non-TTY (audit point 3): AlternateScreen has NO isTTY guard today (doc claimed it did — corrected). The TUI is already gated by stdin.isTTY (config.ts:1532), so non-TTY rarely mounts; the only edge is `-i`. Decision: add a process.stdout.isTTY guard to AlternateScreen, matching the repo convention (startInteractiveUI/notificationService guard isTTY before terminal escapes). Doc now marks it "to implement" + test. - Transcript / per-tool expansion state location: per claude-code (REPL-local transcript state), gemini-cli (dedicated ToolActionsContext), and this repo's own ThinkingViewer (AppContainer-local useState + minimal action via a dedicated context) — transcript open/freeze stays AppContainer-local and is NOT surfaced via UIStateContext (the implemented code already does this; only the doc was wrong). Per-tool expansion uses a dedicated ToolExpandedContext (real cross-layer producer/consumer), not the broad UIStateContext. Also document the fullDetail early-return guard (the just-landed fix): the pure-parallel-agent and memory-only early returns are skipped under fullDetail so transcript shows every tool in full. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): align design doc status/scope with current PR (audit follow-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> * docs(tui): scope mouse click-to-expand out as a follow-up 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 #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> * feat(tui): isTTY guard for AlternateScreen + transcript shortcut/i18n cleanup Completes the remaining in-scope items for the Ctrl+O transcript PR: - AlternateScreen: guard the alt-screen escape writes on `process.stdout.isTTY` (skip when non-TTY: piped/redirected/CI), matching the repo convention (startInteractiveUI / notificationService). Non-TTY now degrades to in-buffer rendering. Adds AlternateScreen.test.tsx (enter/exit on TTY, skip when disabled, skip when non-TTY). - KeyboardShortcuts: add the `ctrl+o → view transcript` entry that was removed with the old compact-mode line but never replaced. - i18n (all 9 locales): drop the dead `to toggle compact mode` and the `Press Ctrl+O to toggle compact mode — …` tip strings (no longer referenced after compact-mode removal); add `to view transcript`. Touched suites green (AlternateScreen, i18n index/mustTranslateKeys, TranscriptView, Help). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): mark isTTY guard + i18n cleanup as implemented in status table Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(i18n): add TranscriptView strings to all locales TranscriptView.tsx renders t('Transcript'), t('to close') and t('to scroll'), but these keys existed only in en/zh. The strict key-parity check (zh, zh-TW) failed CI on the missing zh-TW entries. Add all three keys to zh-TW (the failing strict-parity locale) and to ca/de/fr/ja/pt/ru for completeness so check-i18n is fully clean. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): add before/after transcript capture evidence Add VHS-captured screenshots (main-view collapsed vs Ctrl+O transcript expanded) under docs/design/ctrl-o-detail-expand/assets/ and reference them from §3.4 of the design doc. Captured on the local branch build via the mac-autotest skill; shows read/search/list tools folding to a single summary row in the main view and each expanding in the transcript, with zh i18n strings rendering correctly. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): design §4.9 — full tool detail passthrough in transcript Document the data-layer gap behind the "second-level fold" seen in the Ctrl+O transcript: read/ls/grep returnDisplay only stores a summary, and IndividualToolCallDisplay carries no full-content field, so fullDetail (which correctly clears partition/result folding and height limits) has no detail to render. Spec the chosen fix (path C): derive a contentForDisplay string from the raw llmContent at the single core success-assembly point (partToString + existing 32k retention cap), thread it through to a new IndividualToolCallDisplay.detailedDisplay, and render it in ToolMessage when fullDetail + isCollapsibleTool. Scope limited to read/search/list in the transcript; main-view summaries and shell/edit/write are unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): adopt plan Y for §4.9 and address transcript-detail audit Address the audit on §4.9 (full tool detail in the Ctrl+O transcript): - Rewrite §4.9 to plan Y — reuse the complete content already persisted in functionResponse.response.output (responseParts) via a single core helper, instead of adding a contentForDisplay field threaded through serialize/ replay. Saved/replayed transcripts get full detail for free (audit #6). - Split fullDetail (data-source switch) from forceShowResult (un-fold) so main-view force cases (user-initiated/error) don't leak full detail into the main view (audit #2). - Use the exported compactStringForHistory, not the internal compactString (audit #4). - Scope by isCollapsibleTool incl. glob, not a hardcoded read/ls/grep list (audit #5). - §3.4: stop claiming the screenshot already shows full output; add a pre-§4.9 caveat and a merge-blocker row in the status table (audit #1). - Sync §5 file list, §8 tests, §9 commit 4 (merge blocker); move mouse click-expand out of the commit sequence to follow-up (audit #3). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): tighten §4.9 per second audit (no 2nd truncation, nested media, plan-Y guard) - P1: detailedDisplay no longer runs compactStringForHistory — the 32k cap would make Ctrl+O a "32k bounded preview", contradicting the "full detail" promise (read_file has maxOutputChars=Infinity and can legitimately exceed 32k). Detail is now the full getToolResponseDisplayText output, bounded only by core's existing truncateToolOutput/pagination. - P2: spell out getToolResponseDisplayText's priority rule — media lives in nested functionResponse.parts (not top-level); read response.output, then walk nested parts for inlineData/fileData/text placeholders; undefined when neither output nor media so the UI falls back to the summary. - P3: add an explicit §8 plan-Y protection test (output >32k survives recording/loadSession/resume/replay; detailedDisplay derives from message.parts, not resultDisplay or API compressedHistory) and document the fall-back-to-X trigger. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): address PR review findings on transcript view - AppContainer: freeze a committed-history copy (not just a length) so in-place compaction can't corrupt the open transcript; memoize the stitched items list so streaming re-renders don't rebuild it - AppContainer: clear thinkingViewerData on openTranscript and guard openThinkingViewer so no stale "ghost" thinking popup resurfaces - AppContainer: read prevTranscriptOpen during render (StrictMode-safe) - AppContainer: close the transcript on Ctrl+D instead of swallowing it - TranscriptView: wrap content in a new ErrorBoundary and React.memo the component (stable items + onClose make the shallow compare effective) - CompactToolGroupDisplay: localize buildToolSummary via t() and add the per-category count phrases to all 9 locales - workspace-settings: drop the stale ui.compactMode web-shell allowlist entry - tests: TranscriptView default alt-screen + negative-id keyExtractor; HistoryItemDisplay fullDetail expansion + forwarding; ToolGroupMessage fullDetail parallel-agent bypass; MainContent.test import-first order Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): second review round — web-shell compactMode + anti-deadlock deps - settingsSchema: re-add ui.compactMode as a hidden (showInDialog:false) schema entry so the web shell's independent compact toggle keeps persisting via the daemon settings routes (mirrors voiceModel). The TUI compact mode stays retired — it just isn't shown in the TUI dialog. - workspace-settings: restore ui.compactMode in WEB_SHELL_SETTINGS now that the schema definition resolves again (fixes the web shell 400 / revert). - AppContainer: add isTranscriptOpen to the anti-deadlock auto-close effect deps so opening the transcript while a blocking prompt is already visible re-fires the effect and closes it (previously it could open over an invisible prompt and deadlock). - ToolGroupMessage.test: cover the fullDetail height-truncation lift (availableTerminalHeight undefined under fullDetail, numeric otherwise). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): regenerate vscode settings schema for re-added ui.compactMode The previous commit re-added ui.compactMode (showInDialog:false) to settingsSchema.ts but did not regenerate the generated vscode schema, which the CI "settings schema is up-to-date" gate checks. Regenerated. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * chore(ctrl-o): reset MCP/acp-bridge files to main (drop stale merge diff) These 6 files are unrelated to the Ctrl+O work. Reset to origin/main so the PR diff carries only transcript changes. Committed with --no-verify because the classic-CLI pre-commit prettier reflows union types differently than the repo's experimental-CLI formatter (CI's prettier step does not gate on this). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): update compact-mode docs for transcript model; drop orphaned i18n key - settings.md: ui.compactMode is retired in the TUI (web-shell only); Ctrl+O now opens the full-detail transcript - tool-use-summaries.md: reframe "compact vs full mode" toggle as "main view (completed group) vs Ctrl+O full-detail transcript / force-expanded" - remove the now-orphaned 'Hide tool output and thinking…' locale key (was the old compactMode description) from all 9 locales Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(ctrl-o)!: §4.9 full tool-detail passthrough in transcript Implement plan Y: read/search/list tools now show their COMPLETE output in the Ctrl+O transcript instead of the summary count line, while the main view is unchanged. - core: add `getToolResponseDisplayText(parts)` — extracts the full `functionResponse.response.output` (skipping the non-informative "Tool execution succeeded." placeholder), emits `<media: mime>` placeholders for nested media parts, keeps nested text, returns undefined when nothing is extractable. No second truncation: the only bound is whatever core already applied (truncateToolOutput / paging). - cli: add derived (non-persisted) `IndividualToolCallDisplay.detailedDisplay`. Populated from the already-persisted response parts on both the live path (useReactToolScheduler success branch) and the resume path (resumeHistoryUtils tool_result, falling back to message.parts for older records). - cli: rendering split — ToolGroupMessage forwards `fullDetail` to ToolMessage; ToolMessage swaps the summary `resultDisplay` for `detailedDisplay` ONLY when `fullDetail && isCollapsibleTool(name) && detailedDisplay`. Kept separate from `forceShowResult` so main-view force scenarios (user-initiated / error / confirming) still render the summary, never the full output. - ACP path needs no change: ToolCallEmitter.transformPartsToToolCallContent already writes the same full output into the ACP `content[]` for its SSE clients; the TUI transcript does not flow through it, so no new protocol field is added. Tests: core helper unit tests (placeholder skip, nested media, plain-text part, empty fallback); ToolMessage data-source switch (collapsible+fullDetail uses detail, force-but-not-fullDetail keeps summary, non-collapsible keeps summary, missing-detail falls back); ToolGroupMessage prop-forwarding. BREAKING CHANGE: Ctrl+O is now a frozen full-detail transcript view, not a global compact-mode toggle. The `TOGGLE_COMPACT_MODE` command and the TUI effect of `ui.compactMode` / `ui.compactInline` are removed; the keys remain read-tolerant (ignored by the CLI) and `ui.compactMode` is still forwarded to the web shell. See docs/design/ctrl-o-detail-expand/design.md §6 for migration. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): address review — repaint race, suppressOnRestore parity, transcript error logging - AppContainer: fix close-repaint setTimeout being cancelled by streaming re-renders. `wasOpenPrevRender`/`isTranscriptOpen` were in the effect deps, so the next streaming render flipped them, ran cleanup, and clearTimeout'd the pending repaint — leaving stale pre-transcript content in the legacy <Static> normal buffer. Drive the effect off a close-transition counter instead, so post-close re-renders don't change deps and the scheduled repaint fires exactly once per close. - AppContainer: transcript snapshot now mirrors MainContent's `!display.suppressOnRestore` filter, so items collapsed on session resume (ui.history.collapseOnResume) are not re-exposed in the Ctrl+O view. - TranscriptView: pass `onError` to the ErrorBoundary so caught render errors in the fullDetail paths are logged to the debug channel, not just shown. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ctrl-o): cover detailedDisplay resume derivation + message.parts fallback Add dedicated resumeHistoryUtils tests for §4.9: detailedDisplay derived from toolCallResult.responseParts, the `responseParts ?? message.parts` fallback for older records lacking responseParts, and the undefined fallback when neither source carries output. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): address review — plain-text detail, shared placeholder const, resume status guard, scroll hint Four review fixes on the §4.9 transcript work: - ToolMessage: when fullDetail swaps the data source to detailedDisplay (raw file content / grep hits / dir listings), force renderOutputAsMarkdown to false. The existing `if (availableHeight)` guard never fires in the transcript (height cap is lifted, availableTerminalHeight is undefined), so raw `#`/`*`/`-`/`>` characters were being Markdown-formatted. - core: export TOOL_SUCCEEDED_OUTPUT as the single source of truth for the "Tool execution succeeded." placeholder. coreToolScheduler (the producer, two sites) and getToolResponseDisplayText (the consumer) now share one constant so the filter can't silently drift if the wording changes. - resumeHistoryUtils: only derive detailedDisplay for SUCCESS tools, matching the live path (useReactToolScheduler sets it only in its 'success' branch). Previously it was populated unconditionally, so a resumed errored/cancelled collapsible tool would surface raw output in the transcript while the same tool live would not. - TranscriptView: footer hint now reads "Shift+↑↓ to scroll" — plain Up/Down do not scroll (ScrollableList listens for SCROLL_UP/DOWN bound to Shift+↑↓); the old "↑↓" hint was misleading. Tests: ToolMessage plain-text-detail assertion + new raw-markdown case; resume errored-tool no-detailedDisplay case. typecheck/lint/tests green (core scheduler 222, cli suites pass). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): guard transcript non-TTY output + clear detailedDisplay on compaction Addresses three review findings on the Ctrl+O transcript work: - Non-TTY byte leak: `useMouseEvents` enabled SGR mouse mode (?1002h ?1006h) whenever stdin supported raw mode, ignoring stdout. With stdout piped (`qwen | tee log`) the transcript's focused ScrollableList (bypassVpGate) leaked raw control bytes into the captured output. Gate the enable on `stdout.isTTY`, and likewise guard the transcript close-repaint `clearTerminal` write in AppContainer — both now mirror AlternateScreen's existing isTTY guard, so the non-TTY fallback stays byte-clean. - Compaction privacy regression: `compactOldItems` replaced old tool `resultDisplay` with the cleared placeholder but left `detailedDisplay` (the raw functionResponse text added for the full-detail transcript) intact, so reopening Ctrl+O after compaction re-surfaced the supposedly cleared read/search/list output. Clear `detailedDisplay` wherever `resultDisplay` is cleared, with a regression test. - Docs: keyboard-shortcuts.md still described Ctrl+O as "toggle compact mode"; updated to the open/close full-detail transcript behavior. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): report a TTY stdout in ScrollableList mouse-scroll tests The new `stdout.isTTY` gate in `useMouseEvents` (which stops SGR mouse escapes leaking into piped output) left ink-testing-library's fake stdout — which has no `isTTY` — with the mouse pipeline disabled, so the scrollbar-drag and wheel-scroll assertions never received events. Mock ink's `useStdout` to report `isTTY: true` so the pipeline arms exactly as it does in a real terminal; all other ink exports are preserved. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): address Ctrl+O transcript review — q-guard, callback churn, tests, cleanup Resolves the qwen3.7-max /review findings: - Modifier guard on the transcript close key: bare `q` closed the transcript, but Ink reports Ctrl/Alt/Shift+Q as `{ name: 'q', … }` too (Alt arrives as `meta`), so those silently closed it. Guard `!key.ctrl && !key.meta && !key.shift` (Shift+Q is a literal `Q`). - Stable `openTranscript`: it captured `historyManager.history` and `pendingHistoryItems` as deps, both of which change identity every streaming tick, rebuilding the callback — and the whole `handleGlobalKeypress` closure that lists it — on every render during streaming. Read both via refs so the callback is referentially stable. - AppContainer transcript integration tests (the removed TOGGLE_COMPACT tests had no replacement): Ctrl+O installs TranscriptView; Esc / q / Ctrl+C / Ctrl+D close it; Ctrl+Q / Alt+Q / Shift+Q do NOT (modifier guard); arbitrary keys are swallowed and keep it open; a blocking confirmation (WaitingForConfirmation) auto-closes it (anti-deadlock). - Dead i18n string: removed the orphaned 'Press Ctrl+O to show full tool output' key from all 9 locale files (no `t()` reference remained after the compact-mode sweep). - Design doc: replaced the leaked absolute worktree path with a placeholder, and corrected the §6 keybinding-migration note — the codebase has no user-configurable keybinding override surface (`keyMatchers` always uses hardcoded defaults), so there is no persisted `toggleCompactMode` binding to migrate; the startup-detection step is not applicable until such a feature exists. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): escape ANSI in transcript detailedDisplay + gate its extraction Two findings from the qwen3.7-max /review on §4.9: - [Critical] ANSI escape injection: `detailedDisplay` carries raw, un-sanitized tool output (file contents, grep hits, directory listings). The Ctrl+O transcript rendered it straight to <Text> without escaping, so a malicious repo file with embedded terminal control sequences (e.g. `\x1b[?1049l` to drop the alt-screen, OSC 52 for clipboard poisoning) would execute when the transcript opened — and fullDetail lifts the height cap, exposing the whole file. Run it through `escapeAnsiCtrlCodes` (already used for agent names in this file) before rendering. Added a regression test asserting the raw ESC bytes don't survive. - [perf] `detailedDisplay` was extracted on every successful tool call (~25K chars from core's truncation) but is consumed only by the transcript's fullDetail render for collapsible (read/search/list) tools. Gate the extraction on `isCollapsibleTool(displayName)` so edit/write/command/agent calls no longer store a large string the renderer never reads — mirrors ToolMessage's `usingDetailedDisplay` gate (which also keys off the display name). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): gate resume-path detailedDisplay on isCollapsibleTool (match live path) The resume path (resumeHistoryUtils.ts) extracted `detailedDisplay` for every successful tool call, unlike the live path in useReactToolScheduler which gates on `isCollapsibleTool(displayName)`. Since the transcript's `usingDetailedDisplay` only consumes it for collapsible (read/search/list) tools, resuming a session with many edit/write/command/agent calls stored large (~25K char) strings the renderer never reads. Apply the same gate so live and resume stay consistent, using `toolCall.name` (the display name, set from `tool.displayName`) to match the renderer's key. Updated the existing derivation tests to use a collapsible read tool (an edit tool now correctly yields undefined) and added a regression asserting a non-collapsible tool leaves detailedDisplay undefined on resume. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): strip bare C0 control bytes from transcript detailedDisplay + memoize Follow-up to the ANSI-escape fix. `escapeAnsiCtrlCodes` delegates to ansi-regex, which only matches ESC-prefixed sequences, so bare C0 control bytes without an ESC prefix (BEL \x07, BS \x08, FF \x0c, SO \x0e, SI \x0f, CR, …) passed through to <Text> and could still corrupt the display or ring the bell from a malicious file's contents. Add a second pass that strips those bytes (keeping only TAB and LF, which structure multi-line output). Memoize the two-pass sanitization with useMemo keyed on detailedDisplay so the ~25K-char regex work doesn't re-run every render. Extended the ToolMessage regression test to assert bare C0 bytes are stripped alongside the ESC sequences. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): memoize HistoryItemDisplay, add ErrorBoundary tests + TAB/LF invariant Addresses three review suggestions: - Wrap `HistoryItemDisplay` in `React.memo` so the Ctrl+O transcript (which re-renders on every scroll tick) skips re-rendering frozen-snapshot items whose props are shallowly unchanged. The transcript passes stable `item` references, so the default shallow compare is effective; harmless for the main view (items live in `<Static>` and render once). - Add ErrorBoundary.test.tsx covering the four behaviors: renders children when healthy, catches a render error into the default fallback with the message, renders a custom fallback, calls `onError` with the error + component stack, and `reset` clears the error state so the subtree recovers. - Lock the C0-strip invariant: assert TAB and LF survive in detailedDisplay (the regex intentionally skips \x09/\x0a) so a future regex change can't silently collapse multi-line/columnar output. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(tui): review cleanups — gate sanitize memo, drop dead code, add tests Addresses the latest /review suggestions: - ToolMessage: gate the `sanitizedDetailedDisplay` useMemo on `usingDetailedDisplay` so the ~25K-char escape+strip no longer runs for every collapsible tool in the main view (where the result is discarded). - TranscriptView: remove the dead `listRef` (created + passed as `ref` but never used imperatively) and the dead `onClose` prop (declared, then `void`-ed; close keys are owned entirely by AppContainer's global keypress guard). Dropped the now-unused `useRef` / `ScrollableListRef` imports and the `onClose` call-site + props. - Tests: add TranscriptView error-fallback coverage (a throwing item renders the recovery fallback, not a crash); add live-path `mapToDisplay` detailedDisplay extraction coverage (collapsible → extracted, non-collapsible → undefined); add Ctrl+O to the transcript close-keys it.each (the toggle key was the only close key untested). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): remove orphaned no-op CompactModeProvider stubs This PR deleted the CompactModeContext, leaving identical no-op `CompactModeProvider` passthrough stubs (with an ignored `value` prop) in ToolGroupMessage.test.tsx, ToolMessage.test.tsx and MainContent.test.tsx, each still wrapping every render. Remove the stubs and unwrap the renders; drop the now-meaningless `compactMode` params/args from the local render helpers. Behavior-preserving (the stubs rendered children verbatim) — all three suites still pass. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): strip bidi overrides, sanitize error fallbacks, share filters Latest /review round: - [Critical] Strip Unicode bidirectional override / isolate chars (Trojan Source, CVE-2021-42572) from transcript `detailedDisplay` — a third sanitize pass after ANSI + C0 stripping, mirroring the repo's existing BIDI_CONTROL_RE. Regression test added. - Sanitize `error.message` with `escapeAnsiCtrlCodes` in both the ErrorBoundary default fallback and the TranscriptView custom fallback (defense-in-depth against control codes in a crafted error message). - Ctrl+O while the ThinkingViewer is open now swaps to the transcript (falls through to openTranscript, which clears the viewer) instead of being silently swallowed. - Extract the shared `isHistoryItemVisibleAfterRestore` predicate into types.ts and use it from both MainContent (main view) and AppContainer (transcript freeze), so the two surfaces can't diverge on which collapse-on-resume items are hidden. - Tests: use the exported `TOOL_SUCCEEDED_OUTPUT` constant instead of the hardcoded literal in generateContentResponseUtilities.test.ts. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): harden compaction guard to always clear detailedDisplay The compaction cleanup only cleared `detailedDisplay` inside the `resultDisplay != null` branch (both the group-level trigger, the group-count pass, and the per-tool clear). A tool carrying only `detailedDisplay` (no resultDisplay) would skip compaction and leave the raw transcript detail intact — a latent privacy leak if the two fields ever decouple. Widen all three checks to also match `detailedDisplay != null` so the memory/privacy safeguard is robust. Added a defensive regression test. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): sanitize mime/uri in getToolResponseDisplayText media placeholders The `<media: …>` placeholder interpolated `inlineData.mimeType` / `fileData.mimeType` / `fileData.fileUri` from tool responses verbatim. A crafted response could embed control characters or angle brackets to inject terminal codes or forge/mangle the placeholder markup. Add a `sanitizeMediaLabel` helper that strips C0/C1 control bytes and `<`/`>` before interpolation, falling back to the default label when emptied. Regression test added. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): report a TTY stdout in BaseSelectionList mouse integration test The `stdout.isTTY` gate added to `useMouseEvents` (stops SGR mouse escapes leaking into piped output) left QwenLM#6011's BaseSelectionList mouse test — which renders via ink-testing-library where the hook-provided stdout reads as non-TTY — with the mouse layer disabled, so the any-event enable escape was never written. Mock ink's `useStdout` to report `isTTY: true` with a capturing write spy (matching useMouseEvents.test.tsx / ScrollableList.test .tsx), and assert the `?1003h` enable via that spy while items still render through ink's own stdout. Both cases pass. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(core): fix JSDoc placement + note ErrorBoundary fallback is un-translated Two small review nits: - getToolResponseDisplayText's JSDoc had ended up above sanitizeMediaLabel (added last commit), making it read as that helper's docs. Reorder so sanitizeMediaLabel + its own JSDoc come first and each doc sits directly above its function. - Document why the ErrorBoundary default fallback's title is intentionally a plain English string (last-resort message for callers with no `fallback`; renders mid-crash, so it avoids pulling in the i18n layer — the transcript passes its own localized fallback anyway). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): share terminal-sanitize pipeline; guard AlternateScreen writes - Extract the three-pass sanitizer (ANSI escape + bare-C0 strip + bidi strip) into `sanitizeTerminalText` in textUtils.ts as the single source of truth, and use it at all raw-text render sites: ToolMessage's `detailedDisplay`, and the TranscriptView + ErrorBoundary error-message fallbacks (previously those only escaped ANSI, missing C0/bidi — the boundary catches errors from the fullDetail path that processes raw tool output, so a crafted item shape could carry unsanitized bytes into error.message). Removes the duplicated regex consts from ToolMessage. - AlternateScreen: wrap the alt-screen escape writes (and the exit/cleanup writes) in try/catch so a synchronous stdout error (EPIPE on terminal close, EAGAIN under backpressure) can't propagate uncaught from the effect and crash the app or corrupt the terminal. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.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