Skip to content

fix(cli): show tool descriptions in multi-tool compact summaries - #7589

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
ovochouovo:fix/tool-summary-6014
Jul 26, 2026
Merged

fix(cli): show tool descriptions in multi-tool compact summaries#7589
wenshao merged 3 commits into
QwenLM:mainfrom
ovochouovo:fix/tool-summary-6014

Conversation

@ovochouovo

Copy link
Copy Markdown
Contributor

What this PR does

When multiple tools of the same category (read/search/list) are grouped in the compact tool summary, the summary previously only showed counts like "Read 2 files" or "Searched 2 patterns" without revealing the actual file paths or search patterns. This PR modifies buildToolSummary() to show individual descriptions inline when ≤3 tools are present (e.g. "Read a.ts, b.ts, c.ts"), and the first 2 descriptions plus "...and N more" when >3 tools are present. Additionally, getActiveToolHint() now skips the redundant hint line when descriptions are already visible inline in the summary.

Why it's needed

Users rely on the compact tool summary to quickly understand what the agent is doing. Showing only counts ("Searched 2 patterns") forces users to expand the group to see what was actually searched, defeating the purpose of the compact view. This is especially frustrating during code exploration workflows where the same tool is called multiple times with different inputs. The fix preserves the compact grouping design while surfacing the actionable information users need.

Reviewer Test Plan

How to verify

  1. npm run build && npm run bundle
  2. node dist/cli.js (or npm run dev)
  3. Try these prompts and observe the compact tool summary line:
    • "read package.json and tsconfig.json" → should show Read package.json, tsconfig.json
    • "search for buildToolSummary and safeDescription" → should show Searched 'buildToolSummary', 'safeDescription'
    • "read all .ts files directly under packages/cli/src/" → should show Read xxx.ts, yyy.ts, ...and N more
  4. Run tests:
    • npm test -- CompactToolGroupDisplay.test.tsx (48 tests)
    • npm test -- ToolGroupMessage.test.tsx (53 tests)

Evidence (Before & After)

Screenshots attached in a follow-up comment. Text summary:

Before (count-only):

✓ Read 2 files
✓ Searched 2 patterns

After (descriptions inline):

✓ Read package.json (first 10 lines), tsconfig.json
✓ Searched 'buildToolSummary', 'safeDescription'
✓ Read packages/cli/src/cli.ts, packages/cli/src/cli.test.ts, ...and 6 more

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

node dist/cli.js (built from source) and npm run dev.

Risk & Scope

  • Main risk or tradeoff: Long descriptions may cause the summary line to wrap to multiple lines at narrow terminal widths. The existing wrap="truncate-end" behavior handles this gracefully.
  • Not validated / out of scope: Daemon/ACP path (DaemonTuiAdapter) — not wired into production TUI yet.
  • Breaking changes / migration notes: None. This is a pure display enhancement.

Linked Issues

Closes #6014

中文说明

本次 PR 做了什么

当多个同类工具(read/search/list)被分组到 compact summary 中时,之前只显示数量(如 "Read 2 files" 或 "Searched 2 patterns"),不会显示具体的文件路径或搜索 pattern。本次 PR 修改了 buildToolSummary(),在 ≤3 个工具时内联显示各自的 description(如 "Read a.ts, b.ts, c.ts"),>3 个工具时显示前 2 个 description 加 "...and N more"。此外,getActiveToolHint() 在 description 已经内联显示时会跳过多余的 hint 行。

为什么需要这个改动

用户依赖 compact tool summary 快速了解 agent 正在做什么。只显示数量("Searched 2 patterns")迫使用户展开分组才能看到实际搜索了什么,这违背了 compact 视图的初衷。在代码探索工作流中,同一工具被多次调用时尤其令人沮丧。本次修复保留了紧凑分组设计,同时展示了用户需要的可操作信息。

审阅者测试计划

如何验证

  1. npm run build && npm run bundle
  2. node dist/cli.js(或 npm run dev
  3. 尝试以下 prompt 并观察 compact tool summary 行:
    • "read package.json and tsconfig.json" → 应显示 Read package.json, tsconfig.json
    • "search for buildToolSummary and safeDescription" → 应显示 Searched 'buildToolSummary', 'safeDescription'
    • "read all .ts files directly under packages/cli/src/" → 应显示 Read xxx.ts, yyy.ts, ...and N more
  4. 运行测试:
    • npm test -- CompactToolGroupDisplay.test.tsx(48 个测试)
    • npm test -- ToolGroupMessage.test.tsx(53 个测试)

证据(修复前后)

截图在后续 comment 中附上。文字摘要:

修复前(仅数量):

✓ Read 2 files
✓ Searched 2 patterns

修复后(内联 description):

✓ Read package.json (first 10 lines), tsconfig.json
✓ Searched 'buildToolSummary', 'safeDescription'
✓ Read packages/cli/src/cli.ts, packages/cli/src/cli.test.ts, ...and 6 more

已测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境

node dist/cli.js(从源码构建)和 npm run dev

风险与范围

  • 主要风险或权衡:长 description 在窄终端宽度下可能导致 summary 行换行。现有的 wrap="truncate-end" 行为能优雅处理。
  • 未验证/超出范围:Daemon/ACP 路径(DaemonTuiAdapter)— 尚未接入生产 TUI。
  • 破坏性变更/迁移说明:无。这是纯显示增强。

关联 Issue

Closes #6014

…nLM#6014)

buildToolSummary() previously discarded descriptions when 2+ tools of
the same category were grouped, showing only counts like "Read 3 files"
or "Searched 2 patterns". Now shows actual descriptions inline when ≤3
tools (e.g. "Read a.ts, b.ts, c.ts"), and first 2 + "...and N more"
when >3 tools. getActiveToolHint() skips the redundant ⎿ hint line
when descriptions are already visible inline.
@ovochouovo

Copy link
Copy Markdown
Contributor Author

Before (count-only, no descriptions)

before ### After (descriptions inline) after

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug with evidence — issue #6014 includes a screenshot showing "read 1 file" without the filename, and the user explicitly calls it a downgrade. This is a real user-facing regression, not theoretical hardening.

Direction: aligned. The CHANGELOG shows a clear trajectory of investment in this area — #6448 added file paths for single collapsible tools, #7043 added active path display, #6847 fixed wrapping. This PR is the natural next step: extending description display to multi-tool groups.

Size: not applicable — no core paths touched. All changes are in packages/cli/src/ui/components/messages/.

Approach: the scope feels right. Three files, one component + its tests. The ≤3 inline / >3 preview threshold is a reasonable design choice, and the fallback to count format when descriptions are missing is the correct defensive behavior. No unrelated changes or scope creep.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,有证据——issue #6014 包含截图,显示 "read 1 file" 但没有文件名,用户明确指出这是一个功能退化。这是真实的用户可见回归,不是理论性加固。

方向:对齐。CHANGELOG 显示了对这一区域的持续投入——#6448 为单个可折叠工具添加了文件路径,#7043 添加了活跃路径显示,#6847 修复了换行。这个 PR 是自然的下一步:将描述显示扩展到多工具分组。

规模:不适用——未触及核心路径。所有改动都在 packages/cli/src/ui/components/messages/ 中。

方案:范围合理。三个文件,一个组件加测试。≤3 内联 / >3 预览的阈值是合理的设计选择,描述缺失时回退到计数格式是正确的防御行为。没有无关改动或范围蔓延。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at f47991f502e2b509bf98f7c748d2ee015194a8af · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Code review

Independent proposal: I'd modify buildToolSummary() to show individual descriptions inline for small groups (≤3), preview + "...and N more" for larger groups, fall back to count format when descriptions are missing, and suppress the redundant hint when descriptions are already visible.

The PR's approach matches this almost exactly. Clean implementation — two well-named constants (DESCRIPTION_INLINE_LIMIT = 3, DESCRIPTION_PREVIEW_COUNT = 2), a focused categoryShowsDescriptionsInline() helper, and the getActiveToolHint() change that naturally eliminates the redundant hint row. The verb hoist out of the single-tool branch avoids duplication. estimateCompactToolGroupHeight benefits automatically since it already reads from getActiveToolHint. No critical blockers, no convention violations.

Testing

This is a fork PR — the sandboxed @qwen-code /tmux and @qwen-code /verify lanes require the author to have write access, which external contributors don't. A maintainer can verify the TUI behavior by checking out the PR in a disposable container and running the prompts from the Reviewer Test Plan.

CI evidence for the reviewed commit:

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
precheck-pr / precheck ✅ success
Classify PR ✅ success
route ✅ success
remove-suspicious-attachments ✅ success
Test (macos-latest, Node 22.x) ⏭️ skipped
Test (windows-latest, Node 22.x) ⏭️ skipped
Integration Tests (CLI, No Sandbox) ⏭️ skipped

macOS/Windows tests and integration tests were skipped (fork PR, expected). Ubuntu unit tests and E2E smoke passed. No failures.

not verified: real-scenario TUI output (fork PR — cannot execute PR-derived code; sandboxed lanes unavailable for external contributors).

中文说明

代码审查

独立方案:修改 buildToolSummary(),在小分组(≤3)中内联显示各描述,较大分组显示预览 + "...and N more",描述缺失时回退到计数格式,并在描述已内联显示时抑制多余的 提示行。

PR 的方案与此几乎完全一致。实现干净——两个命名清晰的常量(DESCRIPTION_INLINE_LIMIT = 3DESCRIPTION_PREVIEW_COUNT = 2),一个聚焦的 categoryShowsDescriptionsInline() 辅助函数,以及 getActiveToolHint() 的改动自然地消除了多余的提示行。verb 提升到单工具分支外避免了重复。estimateCompactToolGroupHeight 自动受益,因为它已经从 getActiveToolHint 读取。无关键阻塞项,无规范违反。

测试

这是一个 fork PR——沙箱化的 @qwen-code /tmux@qwen-code /verify 通道需要作者有写入权限,外部贡献者没有。维护者可以在一次性容器中检出 PR 并运行审阅者测试计划中的 prompt 来验证 TUI 行为。

macOS/Windows 测试和集成测试被跳过(fork PR,预期行为)。Ubuntu 单元测试和 E2E 冒烟测试通过。无失败。

未验证:真实场景 TUI 输出(fork PR——无法执行 PR 衍生代码;沙箱通道对外部贡献者不可用)。

Qwen Code · qwen3.8-max-preview

Reviewed at f47991f502e2b509bf98f7c748d2ee015194a8af · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean, well-scoped fix for a real user complaint; would merge without hesitation.

This is exactly the kind of PR the gate should wave through quickly. The problem is real (issue #6014, screenshot evidence, user calling it a downgrade), the fix is minimal (one component, two constants, one helper), and the implementation matches what I'd have written independently. The fallback to count format when descriptions are missing is the right defensive choice, and the hint suppression follows naturally from the inline display. Tests cover the key paths including edge cases (missing descriptions, >3 tools, height estimation). CI is green on Ubuntu. The only gap is real-scenario TUI verification, which is inherent to fork PRs — the unit tests and the author's before/after screenshots cover the behavioral claim well enough for a display-only change.

中文说明

置信度:5/5 ——干净、范围合理的修复,针对真实用户投诉;毫不犹豫可以合并。

这正是门控应该快速放行的 PR。问题是真实的(issue #6014,截图证据,用户称之为功能退化),修复是最小的(一个组件,两个常量,一个辅助函数),实现与我独立会写的一致。描述缺失时回退到计数格式是正确的防御选择,提示抑制自然地跟随内联显示。测试覆盖了关键路径,包括边缘情况(描述缺失、>3 工具、高度估算)。CI 在 Ubuntu 上通过。唯一的缺口是真实场景 TUI 验证,这是 fork PR 的固有限制——单元测试和作者的修复前后截图足以覆盖这个纯显示变更的行为声明。

Qwen Code · qwen3.8-max-preview

Reviewed at f47991f502e2b509bf98f7c748d2ee015194a8af · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

.filter((d): d is string => d !== undefined);
if (previewDescs.length === DESCRIPTION_PREVIEW_COUNT) {
const remaining = tools.length - DESCRIPTION_PREVIEW_COUNT;
part = `${verb} ${previewDescs.join(', ')}, ...and ${remaining} more`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The "...and N more" phrase is hardcoded English, bypassing the t() localization function — even though a translated i18n key '... and {{count}} more' already exists in all 9 shipped locales.

Failure scenario: Before this PR, the multi-tool path used t(forms.many, { count }) which produced fully translated output (e.g., French: "Lu 4 fichiers", Chinese: "读取了 4 个文件"). The new > 3 path replaces this with a hardcoded English template literal. Non-English users now see "Read a.ts, b.ts, ...and 2 more" — English verb, English conjunction, English quantifier — where the old code was fully translated. Three other call sites in the codebase (CommandFormatMigrationNudge.tsx:59, StickyTodoList.tsx:123, SourcesTab.tsx:574) correctly use t('... and {{count}} more', { count: ... }) for this exact phrase.

Suggested change
part = `${verb} ${previewDescs.join(', ')}, ...and ${remaining} more`;
const morePhrase = t('... and {{count}} more', { count: String(remaining) });
part = `${verb} ${previewDescs.join(', ')}, ${morePhrase}`;

Note: the existing i18n key uses "... and" (space before "and") while the current code uses "...and" (no space). Update test expectations to match.

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 19af80d.

Changed from hardcoded ...and ${remaining} more to the existing i18n key:

const morePhrase = t('... and {{count}} more', { count: String(remaining) });
part = `${verb} ${previewDescs.join(', ')}, ${morePhrase}`;

Also updated test expectations to match the i18n format (... and with space before "and", consistent with the translation key used across the codebase).

gwinthis
gwinthis previously approved these changes Jul 23, 2026

@gwinthis gwinthis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: APPROVE (C=0)

UX improvement (+142/-32) that shows actual tool descriptions in compact summaries instead of generic counts. "Read a.ts, b.ts, c.ts" is far more informative than "Read 3 files".

Design:

  • ≤ 3 tools with descriptions → inline: Read a.ts, b.ts, c.ts
  • > 3 tools → first 2 + ...and N more: Read a.ts, b.ts, ...and 2 more
  • Missing descriptions → fallback to count: Read 3 files
  • Active batch with ≤ 3 tools → no redundant hint line (descriptions already inline)

Test coverage: Thorough — 10+ updated assertions covering inline display, truncation, fallback, mixed categories, height estimation. The buildToolSummary unit tests pin each format variant.

Pattern: Progressive disclosure in compact UI: show specifics when the count is small enough to be readable, fall back to aggregates when it isn't. The threshold (3) balances information density against line width.

中文说明

评审:APPROVE (C=0)

UX 改进(+142/-32):紧凑摘要显示实际工具描述而非泛化计数。"Read a.ts, b.ts, c.ts" 比 "Read 3 files" 信息量更大。

模式: 紧凑 UI 的渐进式披露:数量小时显示具体项,数量大时回退到聚合。阈值(3)平衡信息密度与行宽。

— qwen3.7-max via Qwen Code /review

Use existing '... and {{count}} more' translation key instead of
hardcoded English. This ensures the phrase is localized correctly
for all 9 shipped locales (fr, zh, de, zh-TW, ca, ja, ru, pt, en).

Also updates test expectations to match the i18n format (space before
'and' instead of no space).

Addresses CR suggestion on PR QwenLM#7589.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

— qwen3.7-max via Qwen Code /review

Comment on lines +366 to +369
it('more than 3 tools fall back to count when preview descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: '' }),
make({ callId: 'c2', description: '' }),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The > 3 tools fallback branch is only tested when both of the first two preview descriptions are missing (previewDescs.length === 0). The sub-case where exactly one of the first two has a usable description (previewDescs.length === 1) is never exercised. — Failure scenario: With 4+ same-category tools where descriptions are ['a.ts', '', 'c.ts', 'd.ts'], a future change loosening the condition to previewDescs.length > 0 would ship 'Read a.ts, ... and 3 more' with only one description shown, and no test would catch it.

Suggested change
it('more than 3 tools fall back to count when preview descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: '' }),
make({ callId: 'c2', description: '' }),
it('more than 3 tools fall back to count when preview descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: '' }),
make({ callId: 'c2', description: '' }),
make({ callId: 'c3', description: 'c.ts' }),
make({ callId: 'c4', description: 'd.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 4 files');
});
it('more than 3 tools fall back to count when only one preview description is available', () => {
const tools = [
make({ callId: 'c1', description: 'a.ts' }),
make({ callId: 'c2', description: '' }),
make({ callId: 'c3', description: 'c.ts' }),
make({ callId: 'c4', description: 'd.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 4 files');
});

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Local verification report — real TUI build & run

Verified at PR head f47991f5 against main @ 8fa80850. Everything below was run locally on macOS (darwin 24.6.0, Node 22.23.1); no CI results were reused.

Verdict: merge-ready. The feature does what it claims in a real terminal session, the test suite is genuinely load-bearing (10 of 12 mutants killed), and the branch still merges cleanly and passes on today's main — including alongside #7633, which edits the same file. Three non-blocking findings are listed at the end; none of them justify holding the merge.


1. How this was verified

The bot review noted it could not do interactive TUI testing. That gap is what I closed. I built two full bundles and drove each through a real pseudo-terminal:

  • dist/cli.js built from PR head f47991f5 and from the PR's merge-base 868d195b9, each via node esbuild.config.js in an isolated worktree.
  • Differential check on the artifactshasCategoryPeers / categoryShowsDescriptionsInline appear in exactly one chunk of the PR bundle and in zero chunks of the base bundle; the base bundle still carries usesCountSummary. So the two binaries are a true A/B pair and not a stale-dist illusion.
  • Driver: @lydell/node-pty@xterm/headless, plus the repo's own integration-tests/fake-openai-server.ts as the model. The fake model returns N parallel read_file tool calls, so every line below comes out of the production tool scheduler and the production Ink render — not a component harness.
  • Screenshots are the captured PTY byte stream replayed through a real xterm.js in Chromium, so they are the actual terminal cells.

2. Real TUI, before / after (100 columns)

real TUI A/B at 100 columns

scenario main PR 7589
2 × read_file Read 2 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts
3 × read_file Read 3 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts, packages/cli/src/config/config.ts
4 × read_file Read 4 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts, ... and 2 more
10 × read_file Read 10 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts, ... and 8 more

All four scenarios reached finishReason: tool_calls on the fake server and completed the turn. This is a direct fix for the complaint in #6014.

3. Narrow terminal — the one real trade-off

real TUI A/B at 60 columns

Rendered row count for the summary line, using real qwen repo paths, measured against the real ToolStatusIndicator:

terminal width 2 reads 3 reads 4 reads 6 reads 10 reads
60 +1 +2 +1 +1 +1
80 0 +1 +1 +1 +1
100 0 +1 0 0 0
120 0 0 0 0 0
160 0 0 0 0 0

The cost is bounded and self-limiting: because > 3 collapses to first 2 + "... and N more", a 30-read batch is no taller than a 4-read batch. Worst case is +2 rows at 60 columns. At ≥120 columns there is no cost at all. I consider this an acceptable price for the information gained, and it matches what the PR description already discloses under "Risk & Scope".

4. Merge safety against today's main

This matters more than usual here: #7633 (fix(cli): align all TUI icon columns to a uniform 2-col width) landed on main after this PR's merge-base and edits the same file — it drops COMPACT_GROUP_HORIZONTAL_PADDING and changes STATUS_INDICATOR_WIDTH from 3 to 2. Textually clean merges between two PRs touching the same function are exactly where silent breakage hides, so I merged for real rather than trusting mergeable.

  • git merge origin/main into the PR branch → clean, no conflicts.
  • Merged CompactToolGroupDisplay.tsx correctly keeps fix(cli): align all TUI icon columns to a uniform 2-col width #7633's padding removal and this PR's buildToolSummary / getActiveToolHint rewrite. No duplicated constants, no orphaned code.
  • 101/101 tests pass on the merged tree (CompactToolGroupDisplay.test.tsx 48, ToolGroupMessage.test.tsx 53) — the merged test files did not lose either side's assertions.
  • prettier --check and eslint --max-warnings 0 clean on all three changed files.
  • Two failures elsewhere in src/ui/components/messages/ (DiffRenderer, ToolMessage, both in selection-text.ts boundaryJoiner) reproduce on plain origin/main with this PR absent. Not caused by this PR.

5. Are the tests real? Mutation matrix

I flipped 12 individual decisions in the new logic and re-ran both test files each time. A surviving mutant means nothing pins that behaviour.

# mutation result
M1 DESCRIPTION_INLINE_LIMIT 3 → 2 killed
M2 DESCRIPTION_INLINE_LIMIT 3 → 4 killed
M3 DESCRIPTION_PREVIEW_COUNT 2 → 1 killed
M4 > 3 preview guard === COUNT> 0 survived
M5 ≤ 3 guard === tools.length> 0 killed
M6 categoryShowsDescriptionsInline: .every.some survived
M7 drop the count-limit gate in the hint suppressor killed
M8 never suppress the hint (revert half the PR) killed
M9 always suppress the hint killed
M10 drop the ... and N more suffix killed
M11 remaining off-by-one killed
M12 revert the ≤ 3 inline branch to count format killed

10 / 12 killed. That is a strong suite for a display change — the two survivors are narrow and are written up below.

6. Layout-estimate oracle

estimateCompactToolGroupHeight() feeds staticHeight in ToolGroupMessage.tsx:436, which budgets how much room the non-collapsible tools' output gets. If it under-counts, a mixed group can render past its allotted height. I swept 1680 combinations of description length × tool count × terminal width × active/idle, comparing the estimate to the rows the real Ink render actually produces, with the real ToolStatusIndicator:

outcome PR 7589 main
exact 1241
over-estimate (safe: reserves a spare row) 423
under-estimate (unsafe) 16 0

The 16 under-estimating cells are all the same shape and are described in F1.


Findings

All three are Suggestion-level. None blocks the merge.

F1 — estimateCompactToolGroupHeight under-counts by one row for active groups of ≥ 4 tools (new; 0 such cells on main)

Trigger: group is executing, ≥ 4 tools of one category, and a description token longer than roughly contentWidth - 14.

Root cause: the estimator subtracts EXECUTING_ELAPSED_TIME_RESERVED_LABEL (12 columns for 99h 59m 59s) from its wrap width. At that narrower width wrapAnsi(..., {hard: true}) force-breaks a long path mid-token and packs it into fewer rows, while Ink word-wraps at the true width and needs one row more. Confirmed independent of whether the elapsed timer is actually on screen — I re-ran the same group with executionStartTime 30 s in the past and the mismatch persisted.

Repro (4 reads, one executing, contentWidth = 80, description length 70):

estimate = 4
actual   = 5
  [0] ⊷ Reading
  [1]   p/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
  [2]   p/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, ...
  [3]   and 2 more…
  [4]   ⎿ p/dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd

On main the same input is est=2, actual=2 — the count summary always fit one row, so this was unreachable.

Impact: transient and cosmetic. It only bites in the mixed-group path, only while the batch is executing, and only by one row; it self-corrects the moment the batch completes. read_file descriptions are capped by shortenPath(..., maxLen = 80), so an 80-column terminal is the realistic window. Worth a follow-up rather than a revision here — the cheapest fix is to estimate with the width Ink will actually use, or to take the max of the reserved-width and full-width wraps.

F2 — the > 3 preview guard is not pinned by any test (mutation M4; also raised by the bot on 2026-07-24 and still open)

Loosening previewDescs.length === DESCRIPTION_PREVIEW_COUNT to > 0 leaves all 101 tests green. With descriptions ['a.ts', '', 'c.ts', 'd.ts'] that regression ships Read a.ts, ... and 2 more — one description shown plus "2 more" to account for four tools, an off-by-one the user would see. The existing test only covers the case where both previews are missing. The bot's suggested extra it() closes this exactly; adding it would be a real improvement.

F3 — .every in categoryShowsDescriptionsInline is load-bearing but unpinned (mutation M6; not previously reported)

Changing .every to .some leaves all 101 tests green. The consequence is a direct regression of the complaint this PR is fixing: with 2–3 reads where one description is missing, the summary correctly falls back to Reading 3 files, but the ⎿ current-file hint line would also be suppressed — so the user sees a bare count and no filename at all. Verified against the real render that the current .every behaves correctly:

3 reads, one missing description, one Executing
 •Reading 3 files…
    ⎿ c.ts                     ← correct today; nothing prevents a future change from removing it

A test asserting the hint survives when a peer description is missing would pin it.

Nits (no action needed)

  • The PR description's Evidence block writes ...and N more; the shipped string is ... and N more (from the pre-existing i18n key, which is the right call — that key exists in all shipped locales and matches StickyTodoList / SourcesTab / CommandFormatMigrationNudge).
  • Mixed-category groups read slightly ambiguously, because the separator inside the list is the same as the one between categories: Read a.ts, b.ts, ... and 2 more, listed packages/cli.
  • Repeated reads of one file now repeat verbatim: Read src/config.ts, src/config.ts, src/config.ts where main said Read 3 files.
  • The test files stub ToolStatusIndicator with a bare <Text>, which is one column narrower than production (<Box minWidth={2}>). Combined with this PR relaxing expect(frame.split('\n')).toHaveLength(2) to toBeGreaterThanOrEqual(2), the suite can no longer catch row-count regressions. Stubbing only GeminiRespondingSpinner keeps the real indicator width — that is what I used for the oracle in §6.
Reproduction commands
# bundles (isolated worktrees at PR head and merge-base)
node esbuild.config.js && node scripts/copy_bundle_assets.js

# unit tests, on the PR branch and on the branch merged with current main
npx vitest run src/ui/components/messages/CompactToolGroupDisplay.test.tsx \
                src/ui/components/messages/ToolGroupMessage.test.tsx

# merge safety
git checkout -B pr7589-onmain f47991f50 && git merge --no-edit origin/main

# gates
npx prettier --check packages/cli/src/ui/components/messages/{CompactToolGroupDisplay.tsx,CompactToolGroupDisplay.test.tsx,ToolGroupMessage.test.tsx}
npx eslint <same three files> --max-warnings 0

The end-to-end driver (node-pty + @xterm/headless + integration-tests/fake-openai-server.ts), the 12-mutant matrix and the 1680-cell layout oracle were ad-hoc scripts for this review and are not proposed as repo additions.

中文说明

本地验证报告 —— 真实 TUI 构建与运行

在 PR head f47991f5 上、对照 main @ 8fa80850 完成验证。以下全部在本地 macOS(darwin 24.6.0,Node 22.23.1)实跑,未复用任何 CI 结果。

结论:可以合并。 该功能在真实终端会话中确实做到了它声称的事;测试套件是真正起作用的(12 个变异体杀死 10 个);分支在今天的 main 上仍能干净合并并通过测试——包括与改动同一文件的 #7633 共存。文末列出 3 项非阻塞发现,均不足以拖延合并。

1. 验证方式

bot 评审提到它无法做交互式 TUI 测试,我补上的正是这一块。我构建了两个完整 bundle,各自通过真实伪终端驱动:

  • 分别从 PR head f47991f5 与 PR 的 merge-base 868d195b9 构建 dist/cli.js,在隔离 worktree 中用 node esbuild.config.js 生成。
  • 产物差分校验hasCategoryPeers / categoryShowsDescriptionsInline 仅出现在 PR bundle 的一个 chunk 中,base bundle 中为零;base bundle 仍保留 usesCountSummary。因此两个二进制确实是 A/B 对照,而非 dist 陈旧造成的假象。
  • 驱动链路:@lydell/node-pty@xterm/headless,模型侧使用仓库自带的 integration-tests/fake-openai-server.ts。假模型返回 N 个并行 read_file tool call,因此下面每一行都出自生产环境的 tool scheduler 与 Ink 渲染,而非组件级 harness。
  • 截图是捕获的 PTY 字节流在 Chromium 中经真实 xterm.js 重放的结果,即真实终端字符单元。

2. 真实 TUI 前后对比(100 列)

场景 main PR 7589
2 × read_file Read 2 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts
3 × read_file Read 3 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts, packages/cli/src/config/config.ts
4 × read_file Read 4 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts, ... and 2 more
10 × read_file Read 10 files Read packages/cli/src/ui/App.tsx, packages/core/src/core/client.ts, ... and 8 more

四个场景在假服务端均命中 finishReason: tool_calls 并完成整轮。这直接解决了 #6014 中的投诉。

3. 窄终端 —— 唯一真实的权衡

用真实 qwen 仓库路径、对照真实 ToolStatusIndicator 测得的 summary 行渲染行数:

终端宽度 2 reads 3 reads 4 reads 6 reads 10 reads
60 +1 +2 +1 +1 +1
80 0 +1 +1 +1 +1
100 0 +1 0 0 0
120 0 0 0 0 0
160 0 0 0 0 0

代价有界且自限:由于 > 3 会折叠为 前 2 个 + "... and N more",30 个 read 的批次不会比 4 个更高。最坏情况是 60 列下 +2 行;≥120 列时完全无代价。相对于获得的信息量,我认为这个代价可以接受,且 PR 描述的 "Risk & Scope" 已如实披露。

4. 与当前 main 的合并安全性

这一点在本 PR 上格外重要:#7633fix(cli): align all TUI icon columns to a uniform 2-col width)在本 PR 的 merge-base 之后合入 main,且改动同一文件——它删除了 COMPACT_GROUP_HORIZONTAL_PADDING,并把 STATUS_INDICATOR_WIDTH 从 3 改为 2。两个 PR 改同一函数却"文本上干净合并",正是隐性破坏最容易藏身之处,所以我没有依赖 mergeable 字段,而是真的做了一次合并。

  • git merge origin/main 进 PR 分支 → 干净,无冲突。
  • 合并后的 CompactToolGroupDisplay.tsx 同时正确保留了 fix(cli): align all TUI icon columns to a uniform 2-col width #7633 的 padding 删除本 PR 的 buildToolSummary / getActiveToolHint 重写。无重复常量,无孤儿代码。
  • 合并后的树上 101/101 测试通过CompactToolGroupDisplay.test.tsx 48 个,ToolGroupMessage.test.tsx 53 个)——合并后的测试文件没有丢失任一侧的断言。
  • 三个改动文件的 prettier --checkeslint --max-warnings 0 均干净。
  • src/ui/components/messages/ 下另有 2 个失败(DiffRendererToolMessage,均源于 selection-text.tsboundaryJoiner),在不含本 PR 的纯 origin/main 上同样复现,与本 PR 无关。

5. 测试是否真的起作用?变异矩阵

我逐一翻转新逻辑中的 12 个判断,每次重跑两个测试文件。变异体存活即表示该行为无任何测试锁定。

# 变异 结果
M1 DESCRIPTION_INLINE_LIMIT 3 → 2 杀死
M2 DESCRIPTION_INLINE_LIMIT 3 → 4 杀死
M3 DESCRIPTION_PREVIEW_COUNT 2 → 1 杀死
M4 > 3 预览守卫 === COUNT> 0 存活
M5 ≤ 3 守卫 === tools.length> 0 杀死
M6 categoryShowsDescriptionsInline.every.some 存活
M7 去掉 hint 抑制器中的数量上限门 杀死
M8 永不抑制 hint(回退 PR 的一半) 杀死
M9 永远抑制 hint 杀死
M10 去掉 ... and N more 后缀 杀死
M11 remaining 差一错误 杀死
M12 ≤ 3 内联分支回退为计数格式 杀死

12 杀 10。 对一个显示类改动来说这是很强的测试;两个存活项范围很窄,见下方 F2 / F3。

6. 布局估算 oracle

estimateCompactToolGroupHeight()ToolGroupMessage.tsx:436staticHeight 供数,决定非折叠工具输出能分到多少行。若低估,混合分组就可能渲染超出其配额高度。我扫描了 1680 组(description 长度 × 工具数 × 终端宽度 × 活跃/完成),对照真实 Ink 渲染的实际行数,并使用真实的 ToolStatusIndicator

结果 PR 7589 main
精确 1241
高估(安全,多预留一行) 423
低估(不安全) 16 0

这 16 个低估单元形态一致,见 F1。

发现

三项均为 Suggestion 级,都不阻塞合并。

F1 —— 活跃状态下 ≥ 4 个工具的分组,estimateCompactToolGroupHeight 低估一行(新增;main 上此类单元为 0)

触发条件:分组处于 executing、同类工具 ≥ 4 个、且存在长度大于约 contentWidth - 14 的 description token。

根因:估算函数从换行宽度中扣掉了 EXECUTING_ELAPSED_TIME_RESERVED_LABEL99h 59m 59s,12 列)。在这个更窄的宽度下,wrapAnsi(..., {hard: true}) 会在 token 中间强制断行并压得更紧、行数更少;而 Ink 在真实宽度下按单词换行,需要多一行。已确认与计时器是否真的在屏幕上无关——我把 executionStartTime 设为 30 秒前重跑同一分组,偏差依然存在。

影响:短暂且仅影响观感。它只在混合分组路径中出现,只在批次执行期间出现,且只差一行;批次完成即自愈。read_file 的 description 受 shortenPath(..., maxLen = 80) 限制,因此 80 列终端是现实窗口。建议作为后续跟进而非本次返工——最省事的修法是按 Ink 实际使用的宽度估算,或取"预留宽度换行"与"完整宽度换行"两者的较大值。

F2 —— > 3 预览守卫没有任何测试锁定(变异 M4;bot 已于 2026-07-24 提出,至今未处理)

previewDescs.length === DESCRIPTION_PREVIEW_COUNT 放宽为 > 0,101 个测试全绿。当 description 为 ['a.ts', '', 'c.ts', 'd.ts'] 时,该回归会输出 Read a.ts, ... and 2 more——只显示 1 个却用 "2 more" 去凑 4 个工具,是用户可见的差一错误。现有测试只覆盖了两个预览项缺失的情况。bot 建议补的那个 it() 正好堵住这个口子,值得加。

F3 —— categoryShowsDescriptionsInline 中的 .every 是关键逻辑但无测试锁定(变异 M6;此前未被报告)

.every 改成 .some,101 个测试仍全绿。后果恰恰是本 PR 要修的那个问题的回归:2–3 个 read 且其中一个 description 缺失时,summary 会正确回退为 Reading 3 files,但 ⎿ 当前文件 那行 hint 也会被抑制——用户将只看到一个光秃秃的计数,完全没有文件名。已对照真实渲染确认当前 .every 行为正确:

3 个 read,其中一个 description 缺失,一个处于 Executing
 •Reading 3 files…
    ⎿ c.ts                     ← 今天是对的;但没有任何东西阻止未来的改动把它弄丢

补一个"当同类工具的 description 缺失时 hint 仍然保留"的断言即可锁定。

小问题(无需处理)

  • PR 描述的 Evidence 段写的是 ...and N more,实际输出是 ... and N more(来自既有 i18n key——这是正确选择:该 key 在所有已发布语言包中都存在,且与 StickyTodoList / SourcesTab / CommandFormatMigrationNudge 一致)。
  • 混合类别分组读起来略有歧义,因为列表内分隔符与类别间分隔符相同:Read a.ts, b.ts, ... and 2 more, listed packages/cli
  • 重复读同一文件会原样重复:Read src/config.ts, src/config.ts, src/config.ts,而 main 显示的是 Read 3 files
  • 测试文件用裸 <Text> 打桩 ToolStatusIndicator,比生产环境(<Box minWidth={2}>)窄一列。叠加本 PR 把 expect(frame.split('\n')).toHaveLength(2) 放宽为 toBeGreaterThanOrEqual(2),该套件已无法捕捉行数回归。只打桩 GeminiRespondingSpinner 就能保住真实指示器宽度——§6 的 oracle 用的就是这种方式。

Verified locally at PR head f47991f5 vs main 8fa80850. Evidence images: pr-assets/7589-verify.

@wenshao

wenshao commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 26, 2026
Merged via the queue into QwenLM:main with commit 60812d4 Jul 26, 2026
68 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new version no longer show what file the agent read???

4 participants