Polish web-shell execution display#5190
Merged
Merged
Conversation
added 5 commits
June 16, 2026 13:16
ytahdn
requested review from
qqqys,
qwen-code-ci-bot,
wenshao and
yiliang114
and removed request for
wenshao
June 16, 2026 08:42
- Remove unused trace field and withTrace() helper from DisplayItem - Simplify getRowClassName to a single ternary, drop unused index param - Restore !content guard on thinking preview tail-scroll - Add CSS comment documenting margin-left cross-file coupling - Rename isNonAssistantStep to isExecutionWorkStep for clarity - Replace APL glyphs (⌄/⌃) with Geometric Shapes (▼/▲) - Add keyboard accessibility tests for tool row expand (Enter/Space)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Qwen Code Review — PR #5190
2 new findings (1 Critical, 1 Suggestion). Event downgraded from APPROVE to COMMENT because CI is still running.
| # | Severity | File | Summary |
|---|---|---|---|
| 1 | Critical | AssistantMessage.module.css |
Touch-device users cannot expand/collapse thinking blocks |
| 2 | Suggestion | MessageList.tsx |
summarizeSteps prop silently overrides collapseCompletedTurns |
Model: qwen3.7-max
chiga0
approved these changes
Jun 16, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #5190 Polish web-shell execution display
Type: Refactor + Minor Feature + Bug Fix
Change size: +568/-174 across 22 files
Findings Summary
- Critical/Major: 0 items
- Minor: 0 items
- Nit: 2 items
Key Observations
Well-executed UI polish PR with strong attention to accessibility (keyboard support, aria attributes, touch device fallback) and thorough test coverage. The per-commit iteration addressing round-1 review feedback is exemplary — all 7 suggestions from qwen-code-ci-bot's first review were cleanly addressed in commit 1ccd95c. The round-2 findings (touch device accessibility, aria-label override) are also already resolved at HEAD.
Cross-Validation
| Finding | Reviewer | My Assessment |
|---|---|---|
Dead trace field / withTrace() |
qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — removed from DisplayItem |
Unused index in getRowClassName |
qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — simplified to ternary |
Missing !content guard in thinking tail-scroll |
qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — guard restored, content in deps |
| CSS cross-file coupling undocumented | qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — comment added |
isNonAssistantStep misleading name |
qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — renamed to isExecutionWorkStep |
| Missing keyboard accessibility tests | qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — Enter/Space/other-key tests added |
| APL glyphs (⌄/⌃) cross-platform risk | qwen-code-ci-bot R1 | ✅ Fixed in 1ccd95c — reverted to ▼/▲ |
aria-label overrides accessible name on tool rows |
qwen-code-ci-bot R2 | ✅ Already fixed at HEAD — aria-label removed, only aria-expanded + title remain |
| Touch device expand/collapse inaccessible (Critical) | qwen-code-ci-bot R2 | ✅ Already fixed at HEAD — @media (hover: none) present in CSS |
summarizeSteps overrides collapseCompletedTurns |
qwen-code-ci-bot R2 | summarizeSteps does not exist in current code |
Additional Audit Coverage
Areas I independently checked beyond existing findings:
- [Architecture]: Verified
commandsprop threading through App → MessageList → MessageItem → UserMessage, includingareMessageItemPropsEqualmemo comparison update. Consistent and complete. - [Robustness]: Checked
isKnownSlashCommandPromptedge cases — empty commands, non-slash content, unknown slash prefixes. All handled correctly. - [Data Flow]: Verified
toolCallCountcomputation inapplyTurnCollapse— counts tool_group tools + parallel agent counts. Test covers nested subTools (4 total). Correct. - [Accessibility]: Tool row keyboard handler properly calls
event.preventDefault()on Space (prevents page scroll), only handles Enter/Space.role="button"+tabIndex={0}pattern is correct. - [Sub-agent rendering]: Verified
source="assistant"addition to SubAgentResult Markdown component — enables proper markdown rendering (bold, lists, code) in sub-agent results. Test covers<strong>rendering. - [Token semantics]: Confirmed the intentional change to include sub-agent tokens in parent turn totals (was previously excluded). Updated JSDoc in both
sdk-typescript/types.tsandadapters/types.ts. - [CSS changes]: Verified
.lineToggleremoval,.lineExpandablehover/cursor styles, thinking preview overflow with fade gradient. All consistent.
Nits
margin-left: calc(-0.6em - 8px)in UserMessage.module.css encodes a cross-file dependency on PromptChevron width and .message gap. The CSS comment documenting this coupling is present and helpful.- The PR description references "Linked Issues: N/A" — CONTRIBUTING.md recommends linking to an existing issue. Minor process nit for a polish PR.
Final Verdict
LGTM — Clean UI polish with strong test coverage and accessibility. All prior review findings are addressed at HEAD. No blocking issues found.
This review was generated by QoderWork AI
yiliang114
approved these changes
Jun 16, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the UI polish changes, prior review threads, and CI status. Scope is coherent, accessibility follow-ups are addressed, and I did not find any blocking issues.
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
4 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
This PR polishes the web-shell execution transcript so completed turns are easier to scan in a narrow shell layout. It refines turn metrics, step summaries, thinking previews, trace row alignment, slash-command display, tool row interaction, and sub-agent result rendering.
It removes the leading expand/collapse arrow from tool rows while keeping expandable tool rows clickable and keyboard accessible. Expandable rows now use the row itself as the interaction target, with hover and accessibility hints for expand/collapse.
It also improves thinking previews so collapsed thinking still renders markdown, follows the newest streamed content while generation is active, and returns to the beginning of the preview after streaming completes.
Why it's needed
The previous tool row layout used both a status icon and a separate disclosure icon at the start of each row. In practice this consumed valuable horizontal space in the shell, especially for long file paths, commands, and tool summaries. It also made the row visually noisy because the status mark and disclosure mark appeared as two competing leading icons with different meanings.
The extra disclosure icon also duplicated interaction that already exists at the row level and at the turn summary level. Removing it makes the tool row start from the status signal and tool name, which is more compact and avoids the visual confusion of multiple adjacent icons that all look like state controls.
The trace indentation and left rail could make process output appear to jump in and out while streaming, depending on whether a message was currently treated as process text or final output. Aligning the execution content with the prompt and relying on the compact summary keeps the transcript steadier and easier to read.
Slash commands are command invocations rather than normal conversational prompts, so showing the same collapse summary row under known slash commands added noise without helping the reviewer understand the turn.
Reviewer Test Plan
How to verify
Run web-shell and send a prompt that invokes tools. Confirm the prompt row shows compact execution metrics, expanded execution rows align with the normal transcript content, tool rows no longer show a leading disclosure arrow, and hovering an expandable tool row shows an expand/collapse hint while clicking the row or pressing Enter/Space still toggles it.
Run or enter a known slash command such as
/review. Confirm the command line does not show the collapse metadata row underneath it. Enter unknown slash-prefixed text such as/Users/projectand confirm it still behaves like a normal prompt.Run a sub-agent that returns markdown in its result. Confirm the result renders through assistant markdown, including basic formatting such as bold text, lists, or code.
Trigger thinking output longer than five lines. Confirm the collapsed preview scrolls with the newest content while streaming, then returns to the first lines after thinking finishes.
Evidence (Before & After)
Before: Tool rows started with both a status icon and a disclosure arrow, which duplicated visual controls and reduced space for the actual tool summary. Trace rows also used a left rail and indentation that could visually shift while streaming.
After: Tool rows start with the status icon and tool name only, expandable rows remain interactive through row click, keyboard, hover title, and aria label, and execution rows stay aligned with the transcript content.
Tested on
Environment (optional)
npx eslint packages/web-shell --ext .ts,.tsx --max-warnings 0npm run build --workspace=@qwen-code/webuinpm run build --workspace=@qwen-code/sdknpm run build --workspace=@qwen-code/web-shellcd packages/web-shell && npx vitest run client/components/messages/UserMessage.test.tsx client/components/MessageList.test.tscd packages/web-shell && npx vitest run client/components/messages/ToolGroup.test.tsxcd packages/web-shell && npx vitest run client/components/messages/tools/SubAgentPanel.test.tsxcd packages/web-shell && npx vitest run client/components/messages/AssistantMessage.test.tsxRisk & Scope
Linked Issues
N/A
改动说明：
中文说明
What this PR does
这个 PR 优化了 web-shell 的执行过程展示，让窄屏 shell 布局里的 completed turn 更容易阅读。改动包括 turn 指标、步骤摘要、thinking 预览、trace 行对齐、斜杠命令展示、工具行交互以及 sub-agent 结果的 markdown 渲染。
这个 PR 移除了工具行最前面的展开/收起箭头，但保留了工具行本身的点击和键盘可访问能力。可展开的工具行现在用整行作为交互目标，并通过 hover 与无障碍标签提示展开/收起状态。
同时优化了 thinking 预览：收起态仍然使用 markdown 渲染，生成过程中跟随最新流式内容，生成完成后回到预览开头。
Why it's needed
之前的工具行布局会在行首同时展示状态图标和单独的展开箭头。在 shell 这种横向空间紧张的界面里，这会挤占文件路径、命令和工具摘要的展示空间。两个相邻的行首图标也会带来视觉噪音，因为状态标记和展开标记语义不同，但看起来都像状态控制。
这个额外的展开箭头也和工具行本身的交互、turn 汇总层的收起展开能力重复。移除后，工具行从状态信号和工具名称开始，更紧凑，也避免多个相邻 icon 同时出现造成的视觉错乱。
trace 的缩进和左侧竖线会让流式输出过程中出现内容“进去又出来”的感觉，因为一段内容可能暂时被当作过程文本，随后又被判断为最终输出。让执行内容与 prompt 对齐，并通过紧凑摘要表达结构，可以让 transcript 更稳定、更易读。
斜杠命令是命令调用，不是普通对话 prompt。在已知斜杠命令下面继续展示同样的 collapse metadata row 会增加噪音，对理解当前 turn 没有太大帮助。
Reviewer Test Plan
How to verify
运行 web-shell 并发送一个会调用工具的 prompt。确认 prompt 行展示紧凑执行指标，展开后的执行过程与普通 transcript 内容对齐，工具行不再展示行首展开箭头，并且 hover 可展开工具行时会展示展开/收起提示，点击整行或按 Enter/Space 仍可切换展开状态。
运行或输入一个已知斜杠命令，例如
/review。确认命令行下面不再展示 collapse metadata row。再输入未知的斜杠前缀文本，例如/Users/project，确认它仍按普通 prompt 处理。运行一个返回 markdown 结果的 sub-agent。确认结果通过 assistant markdown 渲染，基础格式例如粗体、列表或代码都能正常展示。
触发超过五行的 thinking 输出。确认收起态预览在流式输出时跟随最新内容，thinking 完成后回到最开始几行。
Evidence (Before & After)
Before：工具行开头同时有状态图标和展开箭头，造成视觉控制重复，并减少真实工具摘要的展示空间。trace 行也使用左侧竖线和缩进，流式过程中容易出现视觉位移。
After：工具行只以状态图标和工具名称开始；可展开行仍支持整行点击、键盘操作、hover title 和 aria label；执行过程行与 transcript 内容保持对齐。
Tested on
Environment (optional)
npx eslint packages/web-shell --ext .ts,.tsx --max-warnings 0npm run build --workspace=@qwen-code/webuinpm run build --workspace=@qwen-code/sdknpm run build --workspace=@qwen-code/web-shellcd packages/web-shell && npx vitest run client/components/messages/UserMessage.test.tsx client/components/MessageList.test.tscd packages/web-shell && npx vitest run client/components/messages/ToolGroup.test.tsxcd packages/web-shell && npx vitest run client/components/messages/tools/SubAgentPanel.test.tsxcd packages/web-shell && npx vitest run client/components/messages/AssistantMessage.test.tsxRisk & Scope
Linked Issues
N/A