fix(webui): make long tool output collapsible - #8251
Conversation
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewBefore reading the diff, my independent take was: the cleanest fix is to stop truncating and reuse the collapse UI that already exists locally in
Follow-up commit ("contain collapsible shell output"). The first revision had a layout regression: a long single-line result made the output box expand to ~5.7k px and widened the enclosing message. The follow-up fixes it the right way:
No correctness bugs, security concerns, or regressions found; conventions are followed (collocated tests, ESM, Testing evidenceThis is an unattended CI run, so I did not build or execute the PR's code — the signal below is the PR's own CI on the reviewed commit, read via the API. CI is fully green on this head, including the primary unit suite and the web-shell E2E smoke suite that runs the new layout spec.
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The new failure-first component tests ( The one surface still not exercised end-to-end is the literal 中文说明代码审查在读 diff 之前，我的独立判断是：最干净的修复是停止截断，并复用
后续 commit（"contain collapsible shell output"）。 首个修订有一个布局回归：长单行结果会让输出框扩展到约 5.7k 像素并撑宽外层消息。后续 commit 用正确的方式修复了它：
未发现正确性 bug、安全问题或回归；遵循项目约定（同目录测试、ESM、 测试证据这是无人值守的 CI 运行，因此我没有构建或执行 PR 的代码——以下信号来自通过 API 读取的、该被审查 commit 上 PR 自身的 CI。该 head 上 CI 全绿，包括主单元测试套件和运行新布局 spec 的 web-shell E2E smoke 套件。 新的失败优先组件测试（ 唯一仍未端到端验证的表面是字面意义上的 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; CI is green on the reviewed commit and I would merge without hesitation. Stepping back: this solves a real, observable problem — the shared WebUI was silently throwing away everything past 500 characters of shell output and reasoning traces, including in exported conversations — and it does so with the minimum viable change. Instead of bolting on a new widget, it extracts the collapse behavior that already lived inside The follow-up commit turned my one real reservation into a strength. The first revision had a layout regression (a long single line widening the message box); the follow-up contains it with the correct flexbox CI is fully green on this commit (primary unit suite plus the web-shell E2E smoke suite that runs the new layout cases), so I'm approving now rather than deferring. The only residual is non-blocking and unfixable here: the live 中文说明置信度：5/5 —— 每个阶段都干净；被审查 commit 上 CI 全绿，我会毫不犹豫地合并。 退一步看：这解决了一个真实、可观测的问题——共享 WebUI 会静默丢弃 shell 输出和推理轨迹 500 字符之后的全部内容，导出会话也不例外——而且用最小可行改动做到。它没有新加组件，而是提取 后续 commit 把我唯一真正的保留变成了优点。首个修订有一个布局回归（长单行撑宽消息框）；后续 commit 用正确的 flexbox 该 commit 上 CI 全绿（主单元测试套件，以及运行新布局用例的 web-shell E2E smoke 套件），因此我现在批准，而不再推迟。唯一的残留是非阻塞且此处无法修复的：线上 — Qwen Code · qwen3.8-max-preview Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 6 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks @destire-mio for the contribution! The shared-component extraction and the stopPropagation handling for the nested toggle are clean — appreciate the failure-first tests too. One request (non-blocking): could you add before/after screenshots showing the collapsed and expanded states? I understand the |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| {isCollapsible ? ( | ||
| <CollapsibleOutput | ||
| isCollapsible | ||
| collapsedHeight={60} | ||
| fadeStart={40} | ||
| > |
There was a problem hiding this comment.
[Suggestion] The output-subtle div + pre block is duplicated verbatim across both branches of the isCollapsible ternary — Concrete cost: a future change to the output rendering (class rename, data attribute, syntax highlighting) must be applied to both copies independently; missing one produces a visual inconsistency that only manifests for one length range (>500 vs ≤500 chars).
| {isCollapsible ? ( | |
| <CollapsibleOutput | |
| isCollapsible | |
| collapsedHeight={60} | |
| fadeStart={40} | |
| > | |
| {isCollapsible ? ( | |
| <CollapsibleOutput | |
| isCollapsible | |
| collapsedHeight={60} | |
| fadeStart={40} | |
| > | |
| {outputContent} | |
| </CollapsibleOutput> | |
| ) : ( | |
| outputContent | |
| )} |
(where outputContent is extracted as a local const above the return.)
— qwen3.8-max-preview via Qwen Code /review
| <CollapsibleOutput | ||
| isCollapsible={output.length > EXPAND_THRESHOLD} | ||
| className="text-[13px] opacity-90" | ||
| > |
There was a problem hiding this comment.
[Suggestion] GenericToolCall was refactored to use the shared CollapsibleOutput but has no test covering its specific wiring — Concrete cost: the 400-char threshold (EXPAND_THRESHOLD, distinct from the 500 used by Shell/Think) and the className="text-[13px] opacity-90" pass-through are unique to this consumer; if a future change breaks either, no test catches the regression. Consider adding a GenericToolCall.test.tsx mirroring ThinkToolCall.test.tsx with two cases: output > 400 chars renders a toggle and preserves full content; output ≤ 400 chars renders no toggle.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Local verification report — build + real-browser E2EI built this branch locally ( 1. What reproduces ✅
Real-browser behaviour on the export page (real
Side note on your "not validated" item: the stock export page does load WebUI from unpkg, but a local 2. Blocking: the collapsible OUT row blows out the layout 🔴
The 600px host column becomes a 6795px page. On the real export page the same input gives
Root cause. Fix I verified (restores every number in the last column, and your 7 focused tests still pass 7/7): .bash-toolcall-row-content.bash-toolcall-full {
min-width: 0;
}
.execute-toolcall-row-content.execute-toolcall-full {
min-width: 0;
}That the existing suite passes both with and without this fix is the real lesson: jsdom has no layout engine, so no test in this PR can see the bug. A guard that asserts 3. Smaller notes 🟡
VerdictThe idea and most of the implementation are right, and the collapse/expand, click-isolation and a11y work all hold up under a real browser. I would not merge as-is, though: the layout blow-out defeats the PR's own goal for long single-line output and visibly breaks the card in every consumer. Add the two 中文说明本地验证报告 —— 构建 + 真实浏览器 E2E我在本地构建了这个分支（ 1. 可复现的部分 ✅
导出页面上的真实浏览器行为（真实的
关于你标注为"未验证"的那一项：标准导出页确实从 unpkg 加载 WebUI，但今天已经可以做本地 2. 阻塞问题：可折叠的 OUT 行撑爆了布局 🔴在只加载
600px 的宿主列变成了 6795px 的页面。在真实导出页面上，同样的输入让 根因。 我验证过的修复（能恢复最后一列的全部数值，并且你的 7 个 focused 测试依旧 7/7 通过）： .bash-toolcall-row-content.bash-toolcall-full {
min-width: 0;
}
.execute-toolcall-row-content.execute-toolcall-full {
min-width: 0;
}现有测试在加不加这个修复的情况下都通过，这才是真正的教训：jsdom 没有布局引擎，所以本 PR 里的任何测试都看不见这个 bug。值得补一个在真实浏览器中断言 3. 次要问题 🟡
结论思路和大部分实现是对的，折叠/展开、点击隔离和无障碍改进在真实浏览器下都站得住。但我不建议按当前状态合并：布局撑爆在长单行输出上恰好抵消了本 PR 自己的目标，并且在所有消费方里都会把卡片撑破。补上两条 🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| ) : ( | ||
| <div className="italic opacity-90 leading-relaxed"> | ||
| {thoughts} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
[Suggestion] The non-collapsible branch duplicates the className="italic opacity-90 leading-relaxed" and {thoughts} child already passed to CollapsibleOutput, and diverges from GenericToolCall's always-render form in this same PR. — Concrete cost: restyling the thought text (or changing the wrapper) requires editing two places, and the shared component's isCollapsible={false} path is exercised by only one of three callers. CollapsibleOutput already renders cleanly with isCollapsible={false} (no maxHeight/mask, no toggle), so the whole ternary could collapse to:
<CollapsibleOutput
isCollapsible={isCollapsible}
className="italic opacity-90 leading-relaxed"
>
{thoughts}
</CollapsibleOutput>(The only structural difference for the 200–500-char band is an inert flex flex-col wrapper plus overflow-hidden with no maxHeight, which does not clip fitting text.)
— qwen3.8-max-preview via Qwen Code /review
| collapsedHeight={60} | ||
| fadeStart={40} |
There was a problem hiding this comment.
[Suggestion] The shell OUT row's collapsed appearance is encoded twice with nothing keeping the copies in sync: outputs ≤ 500 chars are clipped by .bash-toolcall-row-content:not(.bash-toolcall-full) in ShellToolCall.css (max-height: 60px, mask 40px → 60px), while outputs > 500 chars are clipped by these inline props (60/40) — the same row, two rendering paths selected by a length threshold. — Concrete cost: a maintainer tuning the short-output clip in CSS (say to 80px/50px, duplicated again for bash and execute) gets no error and no test failure, but 499-char and 501-char outputs suddenly collapse at different heights with different fades — a "random" layout bug with the two halves of the behavior living in different languages. Consider exporting shared constants used by both the CSS (via custom properties) and these props, or always wrapping the OUT content in CollapsibleOutput (toggle only when isCollapsible) and dropping the :not(.-toolcall-full) mask rule so one code path owns the collapsed appearance.
— qwen3.8-max-preview via Qwen Code /review
Re-verification on
|
| Check | Claimed | Measured here |
|---|---|---|
npx vitest run (packages/webui) |
31 files / 400 tests | 31 files / 400 passed |
| Focused component run (4 files) | 9 tests | 9 passed |
npm run test:e2e:smoke (web-shell) |
26 tests | 26 passed, including both new layout cases |
| webui typecheck / lint / build | pass | pass — both min-width: 0 rules present in the shipped dist/styles.css |
| web-shell lint / typecheck | pass | pass (after building sdk-typescript first, as your notes say) |
| Prettier on changed files | pass | pass — the 4 remaining web-shell warnings are pre-existing on main and untouched here |
Behaviour on the real export page is unchanged from my previous pass: bash / execute / think / generic all keep the complete text, clicking the toggle does not open the OUT-row temp-file modal (stopPropagation holds in a real browser), and clicking the OUT row still opens the modal with the full output.
2. The blocking regression is fixed 🟢
Same fixture as before — one 2 459-character line with no whitespace — measured on the real export page at a 1180 px viewport:
main af63c9d |
88b5f33 |
f8d1681 |
|
|---|---|---|---|
.chat-viewer-messages clientWidth / scrollWidth |
812 / 812 | 812 / 16405 🔴 | 812 / 812 ✅ |
.bash-toolcall-card clientWidth / scrollWidth |
740 / 740 | 740 / 16354 🔴 | 740 / 740 ✅ |
| OUT text length | 503, ends with ... |
2459 | 2459 |
| Tail reachable? | no — cut at 500 | no — 0 px scrollable | yes — scrollLeft 15622 of scrollWidth 16306 |
The two min-width: 0 rules restore the automatic minimum size that :not(...-full)'s overflow: hidden used to provide, which is exactly what I measured last time. The ShellToolCall.tsx change in the same commit is a pure extraction of outputContent — no behavioural difference, confirmed by the identical geometry on both branches of the ternary.
3. The new guard actually catches it — failure-first ✅
I removed only the two min-width: 0 rules from ShellToolCall.css and re-ran:
packages/web-shell→webui-tool-output-layout.spec.ts— 2 failed (bash and execute), atexpect(collapsed.outputScrollWidth).toBeGreaterThan(collapsed.outputClientWidth)withExpected: > 5544, Received: 5544.packages/webui→ the whole jsdom tool-call suite — 6 files / 28 tests, all passing.
That is precisely the gap I flagged: jsdom has no layout engine and cannot see this class of bug, and the new browser assertion sits exactly where it needs to. Restoring the rules turns both back green.
4. Feature evidence — collapsed / expanded screenshots
@yiliang114 — the captures you asked for, taken from the real export page rather than Storybook, so they reflect the shipped bundle.
Multi-line output, where the 60 px clamp actually bites:
Long single-line output — the case that motivated the PR — including the horizontal-scroll behaviour:
5. Non-blocking notes 🟡
-
For single-line output the toggle is a visual no-op. The threshold is character-based (
> 500) while the clamp is height-based (60px), so the 2 459-character single line renders at 18 px collapsed and 18 px expanded on the export page — "▼ Show more" appears, and pressing it changes nothing visible; the tail is reached by horizontal scrolling either way. Multi-line output behaves correctly (60 px → 318 px; the think card 200 px → 395 px). This one is new —mainshowed no toggle there at all because the content was truncated. GatingisCollapsibleon measured overflow (scrollHeight > collapsedHeight) rather than character count would fix it, and would also cover the inverse case (a 400-character, 20-line output still clamped to 60 px with no toggle). Pre-existing policy question, worth a follow-up rather than a change here. -
A caveat about the new harness, not a defect.
packages/web-shellserves@qwen-code/webuifrom source in dev, so webui's own Tailwind utility sheet is not loaded —min-w-0onToolCallRow/ToolCallCardcomputes toautoin the harness but to0pxwith the shippeddist/styles.css. The guard's assertions are all againstShellToolCall.css, which is a plain CSS import and always applies, so the test is sound; it just would not see a regression that only affects the Tailwind-generated utilities. Worth knowing before anyone extends this harness. (I chased an apparent think/generic overflow in that harness first — it does not reproduce against the shipped stylesheet.) -
The
<= 500path is not byte-identical tomain. A 400-character single-line output measures 470 / 470 onmainand 470 / 2699 here — it gains horizontal scrolling from thepre { overflow: visible }change. That is an improvement and clearly intentional, just not literally "unchanged"; error and empty-output paths are identical. -
My earlier notes 1 and 2 are both resolved — the CSS-source substring assertion and the
process.cwd()-relativereadFileSyncare gone, replaced by a real browser assertion. 👍
Verdict
Ready to merge from my side. The bug that blocked 88b5f33 is fixed, the fix is the minimal one, and it now has a regression guard that fails without it.
中文说明
针对 f8d1681 的复验 —— 阻塞性回归已修复
这是我在 88b5f33 上那份报告的后续。我在本地重新构建了这个分支（f8d1681，merge-base af63c9d），再次在真实 Chromium 中驱动了真实的导出会话页面 —— 真实的 packages/web-templates 模板、真实的 injectDataIntoHtmlTemplate、本地构建的 WebUI UMD bundle，并用真实的 git log / git rev-list 输出作为素材。布局撑爆的问题已经消失，新增的 Playwright 守卫也确实能抓住它本该抓的 bug。LGTM。
1. 更新后的 PR 描述里的结论都能复现 ✅
| 检查项 | PR 声称 | 我这边实测 |
|---|---|---|
npx vitest run（packages/webui） |
31 个文件 / 400 个测试 | 31 个文件 / 400 通过 |
| focused 组件测试（4 个文件） | 9 个测试 | 9 通过 |
npm run test:e2e:smoke（web-shell） |
26 个测试 | 26 通过，包含两个新的布局用例 |
| webui typecheck / lint / build | 通过 | 通过 —— 两条 min-width: 0 规则都出现在最终产物 dist/styles.css 中 |
| web-shell lint / typecheck | 通过 | 通过（按你的说明，先构建 sdk-typescript） |
| 改动文件的 Prettier | 通过 | 通过 —— web-shell 剩余的 4 个告警在 main 上就存在，本 PR 未触及 |
真实导出页面上的行为与我上一轮一致：bash / execute / think / generic 都保留完整文本；点击开关不会打开 OUT 行的临时文件弹窗（stopPropagation 在真实浏览器中同样有效）；点击 OUT 行仍然会打开包含完整输出的弹窗。
2. 阻塞性回归已修复 🟢
素材与上次相同 —— 一条 2 459 字符、不含空白的单行 —— 在真实导出页面、1180 px 视口下测得：
main af63c9d |
88b5f33 |
f8d1681 |
|
|---|---|---|---|
.chat-viewer-messages clientWidth / scrollWidth |
812 / 812 | 812 / 16405 🔴 | 812 / 812 ✅ |
.bash-toolcall-card clientWidth / scrollWidth |
740 / 740 | 740 / 16354 🔴 | 740 / 740 ✅ |
| OUT 文本长度 | 503，以 ... 结尾 |
2459 | 2459 |
| 尾部可达？ | 否 —— 在 500 处被砍 | 否 —— 0 px 可滚动 | 是 —— scrollLeft 15622 / scrollWidth 16306 |
那两条 min-width: 0 恢复了原本由 :not(...-full) 的 overflow: hidden 提供的自动最小尺寸，与我上次测出的结论完全一致。同一个 commit 里的 ShellToolCall.tsx 改动只是把 outputContent 提取出来，没有行为差异 —— 三元表达式两个分支的几何尺寸完全相同，可以确认这一点。
3. 新守卫确实能抓住问题 —— 失败优先验证 ✅
我只删掉了 ShellToolCall.css 里的两条 min-width: 0 规则，然后重新运行：
packages/web-shell→webui-tool-output-layout.spec.ts—— 2 个失败（bash 与 execute），失败点在expect(collapsed.outputScrollWidth).toBeGreaterThan(collapsed.outputClientWidth)，报Expected: > 5544, Received: 5544。packages/webui→ 整个 jsdom tool-call 测试集 —— 6 个文件 / 28 个测试全部通过。
这正是我之前指出的覆盖缺口：jsdom 没有布局引擎，看不见这类 bug，而新增的浏览器断言恰好补在该补的位置。把规则加回去，两边都恢复为绿色。
4. 功能证据 —— 折叠 / 展开截图
@yiliang114 —— 你要的截图在这里，是从真实导出页面（而非 Storybook）截取的，因此反映的是实际发布产物的效果。
多行输出，60 px 的 clamp 在这种情况下真正生效：
长单行输出 —— 也就是本 PR 想解决的场景 —— 包含横向滚动行为：
（截图见上方英文部分）
5. 非阻塞的观察 🟡
-
对单行输出来说，展开开关在视觉上是个空操作。 阈值按字符数算（
> 500），而 clamp 按高度算（60px），所以那条 2 459 字符的单行在导出页面上折叠时是 18 px，展开后仍然是 18 px —— "▼ Show more" 会出现，但点下去看不到任何变化；尾部无论如何都只能靠横向滚动到达。多行输出则表现正常（60 px → 318 px；think 卡片 200 px → 395 px）。这一点是本 PR 新引入的 ——main上因为内容被截断，那里根本不会出现开关。把isCollapsible改成基于实测溢出（scrollHeight > collapsedHeight）而不是字符数即可解决，同时也能覆盖反向的情况（400 字符、20 行的输出仍被 clamp 到 60 px 却没有开关）。这属于既有策略问题，建议后续单独处理，不必在本 PR 里改。 -
关于新 harness 的一个说明，不是缺陷。
packages/web-shell在 dev 模式下把@qwen-code/webui指向源码，因此不会加载 webui 自己的 Tailwind 工具类样式表 ——ToolCallRow/ToolCallCard上的min-w-0在 harness 里计算为auto，而在实际发布的dist/styles.css下是0px。新守卫的断言全部针对ShellToolCall.css，那是普通 CSS import、始终生效，所以测试本身是可靠的；只是它看不到仅影响 Tailwind 工具类的回归。后续有人扩展这个 harness 时值得注意。（我一开始在该 harness 里看到 think/generic 似乎也撑爆，但在实际发布样式表下并不复现。） -
<= 500这条路径与main并非逐字节一致。 400 字符的单行输出在main上是 470 / 470，在这里是 470 / 2699 —— 由于pre { overflow: visible }的改动，它获得了横向滚动能力。这是改进，显然也是有意为之，只是并不完全等于"保持不变"；错误路径和空输出路径确实完全一致。 -
我之前的第 1、2 条意见都已解决 —— 断言 CSS 源码子串的测试和依赖
process.cwd()的readFileSync都已移除，换成了真实浏览器断言。👍
结论
从我这边看可以合并。阻塞 88b5f33 的 bug 已修复，修复方式是最小改动，并且现在有了一个"去掉修复就会失败"的回归守卫。
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 453 passed · 0 failed · 453 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：453 通过 · 0 失败 · 453 总计 Verification reportPR #8251 Deep Verification —
|
| Cell | Test | Head | Base | Oracle |
|---|---|---|---|---|
| T1 | ThinkToolCall: tail marker in DOM | ✅ PASS | ❌ FAIL (truncated) | textContent.toContain('__THINK_TAIL_MARKER__') |
| T2 | ThinkToolCall: expand button present | ✅ PASS | ❌ FAIL (null) | querySelector('button[aria-label="Expand output"]') |
| T3 | ThinkToolCall: expand/collapse cycle | ✅ PASS | ❌ FAIL (null) | aria-expanded + maxHeight toggling |
| T4 | ThinkToolCall: 500 chars → no toggle | ✅ PASS | ✅ PASS | boundary control |
| S1-bash | ShellToolCall bash: tail in DOM | ✅ PASS | ❌ FAIL (truncated) | textContent.toContain('__SHELL_TAIL_MARKER__') |
| S1-exec | ShellToolCall execute: tail in DOM | ✅ PASS | ❌ FAIL (truncated) | same |
| S2-bash | ShellToolCall bash: expand button | ✅ PASS | ❌ FAIL (null) | button presence |
| S2-exec | ShellToolCall execute: expand button | ✅ PASS | ❌ FAIL (null) | button presence |
| S3 | Expand click ≠ openTempFile | ✅ PASS | ❌ FAIL (no button) | openTempFile not called |
| S4 | OUT row click → openTempFile | ✅ PASS | ✅ PASS | existing behavior control |
| S5 | Shell 500 chars → no toggle | ✅ PASS | ✅ PASS | boundary control |
Head: 11/11 pass. Base: 3/11 pass, 8 fail (all expected).
Evidence: 01-ab-head-all-pass.png, 02-ab-base-8-fail.png.
Correction to PR description: The description states "the OUT scroll container now owns horizontal overflow so a long single-line result remains scrollable […] instead of being clipped by the nested <pre>." This is accurate for the CSS change, but the description's framing may suggest the base had full content in the DOM with only visual clipping. In fact, the base ShellToolCall also hard-truncated at 500 chars via output.substring(0, 500) + '...' (base line 214–215), so the base had actual data loss in the DOM, not just CSS clipping. The PR correctly fixes both the data loss and the CSS overflow.
Vacuity check (mutation matrix)
Four single-point mutations were applied to the head source; each was killed by the PR's own tests. The unmutated control is green (evidence: 03-vacuity-head-green.png).
| # | Mutation | File | Suite that catches it | Failure message | Killed? |
|---|---|---|---|---|---|
| M1 | isCollapsible = false (was thoughts.length > 500) |
ThinkToolCall.tsx | ThinkToolCall.test.tsx | expected null not to be null (button absent) |
✅ |
| M2 | isCollapsible = false (was output.length > 500) |
ShellToolCall.tsx | ShellToolCall.test.tsx | expected null not to be null (button absent, both bash+execute) |
✅ |
| M3 | maxHeight: '' (was `${collapsedHeight}px`) |
CollapsibleOutput.tsx | CollapsibleOutput.test.tsx + ThinkToolCall.test.tsx | expected '' to be '200px' |
✅ |
| M4 | Removed event.stopPropagation() |
CollapsibleOutput.tsx | ShellToolCall.test.tsx | expected "spy" to not be called at all, but actually been called 1 times |
✅ |
All four mutations fail the intended behavioural assertion with the expected-versus-actual mismatch the test exists to catch. No survivors.
Targeted gates
| Gate | Result | Detail |
|---|---|---|
| Webui test suite | ✅ 416/416 pass, 32 files | npx vitest run in packages/webui |
| TypeScript typecheck | ✅ clean | tsc --noEmit exit 0 |
| ESLint (changed dir) | ✅ clean | npx eslint packages/webui/src/components/toolcalls/ exit 0 |
| ESLint liveness | ✅ live | Planted const unused_var = 42; → caught @typescript-eslint/no-unused-vars; removed |
Findings
None. No blocking or non-blocking issues found.
Not covered
- CSS horizontal scroll behaviour: The PR adds
*-toolcall-output-subtle .*-toolcall-pre { overflow: visible }to enable horizontal scrolling within the output container. jsdom does not perform layout, so this CSS-only claim cannot be behaviourally tested here. The PR author's Chrome layout probe (clientWidth/scrollWidth = 200/831) is cited but not independently reproduced. - Mask gradient visual effect: The
maskImage/WebkitMaskImagelinear-gradient fade is a visual-only concern; the inline style values are asserted in tests but the rendered appearance is not verified. - Export page (
/export) behaviour: The PR description notes the export page loads the published WebUI bundle from unpkg, not the worktree bundle, so the export scenario was not tested. - E2E test plan files:
.qwen/e2e-tests/2026-07-31-export-collapsible-tool-output.mdandpackages/webui/client/e2e/are documentation/harness files for manual or future automated verification; they were not executed. - Per-commit attribution: The checkout is shallow (depth 2); only the aggregate
HEAD^1..HEADdiff was verified. - Storybook stories: The one-line comment change in
ShellToolCall.stories.tsxwas not rendered.
Methodology
Verification ran in the CI container (node:22-bookworm) on the merge-ref checkout (HEAD = merge commit, HEAD^1 = base tip 2bbd82f, HEAD^2 = PR head f8d1681). The A/B harness was a vitest test file placed identically in both the head tree and a git worktree at HEAD^1; the base worktree reused the head's root node_modules via symlink (no dependency changes — verified). The code under test imports only relative paths within packages/webui, so no workspace symlink confound applies. Vacuity mutations were applied in-place with backup/restore. Raw logs: head-ab.log, base-ab.log, webui-tests.log, typecheck.log, lint.log.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.3. |
4 participants
What this PR does
This PR replaces the 500-character hard truncation for successful Bash/Execute output and long
thinkcontent with an expandable view that keeps the complete text while remaining collapsed by default. It extracts the existing generic-tool collapse UI into a renderer-agnostic shared component, preserves GenericToolCall's current threshold and rendering, keeps Shell's OUT-row full-content action, and prevents the nested expand/collapse button from triggering that row action.For Shell output, the OUT scroll container now owns horizontal overflow so a long single-line result remains scrollable in both Bash and Execute variants instead of being clipped by the nested
<pre>.Why it's needed
The shared WebUI, including exported conversations, currently replaces everything after 500 characters with an ellipsis for these paths. That permanently removes the tail of command output and reasoning traces, so users cannot inspect the complete result even after opening the exported page. Keeping the full content behind a compact default view preserves readability without losing diagnostic information.
Reviewer Test Plan
How to verify
Show more, expanding reveals the marker, and collapsing restores the compact view.thinktool call longer than 500 characters.Automated verification from
packages/webui:npm exec vitest run src/components/toolcalls/shared/CollapsibleOutput.test.tsx src/components/toolcalls/ShellToolCall.test.tsx src/components/toolcalls/ThinkToolCall.test.tsx; fullnpm test;npm run lint;npm run typecheck;npm run build.Evidence (Before & After)
Before: failure-first tests for the three long-output paths could not find their unique tail markers because the renderers replaced content after 500 characters with
.... A real Chrome layout probe for a long Shell line measured the scroll container atclientWidth/scrollWidth = 200/200while the nested<pre>overflowed, so the hidden tail was not horizontally reachable.After: the focused regression suite passes 7/7 and the full WebUI suite passes 398/398. The same Chrome probe measures
clientWidth/scrollWidth = 200/831for both Bash and Execute, with the complete tail preserved and reachable. A local/exportscreenshot is not presented as evidence because the normal export page loads the published WebUI bundle from unpkg rather than this worktree bundle; the included E2E plan records the release-bundle manual scenario.Tested on
Environment (optional)
macOS 26.3.1, Node.js 24.14.0, Vitest 3.2.4, system Google Chrome in headless mode.
Risk & Scope
@qwen-code/webuiconsumers; the existing GenericToolCall behavior is preserved and covered through the full WebUI suite.<=500/error-output collapse policy, and a release-bundle/exportsmoke test are outside this bounded fix.Linked Issues
Refs #8208
中文说明
本 PR 做了什么
本 PR 将成功的 Bash/Execute 输出和较长
think内容原有的 500 字符硬截断，替换为保留完整文本的可展开视图，同时默认保持折叠。它把 GenericToolCall 现有的折叠 UI 提取为与渲染器无关的共享组件，保留 GenericToolCall 当前的阈值和渲染行为，保留 Shell 的 OUT 行全文打开操作，并阻止内层展开/收起按钮触发外层 OUT 行操作。对于 Shell 输出，现在由 OUT 滚动容器负责横向溢出，因此无论 Bash 还是 Execute，长单行结果都能横向滚动，不再被内层
<pre>裁掉。为什么需要它
共享 WebUI（包括导出的会话）目前会在这些路径中把 500 字符之后的全部内容替换为省略号。这会永久丢失命令输出和思考轨迹的尾部，用户即使打开导出页面也无法检查完整结果。默认紧凑显示、按需展开完整内容，可以兼顾可读性和诊断信息的完整性。
Reviewer 测试计划
如何验证
Show more，展开后能看到尾部标记，收起后恢复紧凑视图。think工具调用重复上述验证。在
packages/webui中执行的自动验证：npm exec vitest run src/components/toolcalls/shared/CollapsibleOutput.test.tsx src/components/toolcalls/ShellToolCall.test.tsx src/components/toolcalls/ThinkToolCall.test.tsx；完整npm test；npm run lint；npm run typecheck；npm run build。证据（修改前与修改后）
修改前：三个长输出路径的失败优先测试都找不到各自唯一的尾部标记，因为渲染器把 500 字符后的内容替换为
...。真实 Chrome 对长 Shell 单行内容的布局探针显示，滚动容器为clientWidth/scrollWidth = 200/200，而内层<pre>已溢出，因此被隐藏的尾部无法通过横向滚动访问。修改后：focused 回归测试 7/7 通过，完整 WebUI 测试 398/398 通过。同一个 Chrome 探针在 Bash 和 Execute 中都测得
clientWidth/scrollWidth = 200/831，完整尾部已保留并可访问。这里没有把本地/export截图作为证据，因为常规导出页面会从 unpkg 加载已发布的 WebUI bundle，而不是当前 worktree 的 bundle；随 PR 提交的 E2E 计划记录了发布 bundle 可用后的手动验证场景。已测试平台
环境（可选）
macOS 26.3.1、Node.js 24.14.0、Vitest 3.2.4、无头模式下的系统 Google Chrome。
风险与范围
@qwen-code/webui消费方；本 PR 保留了 GenericToolCall 的现有行为，并通过完整 WebUI 测试覆盖。<=500/错误输出折叠策略的修改，以及发布 bundle 的/export冒烟测试不属于这个边界明确的修复。关联 Issue
Refs #8208