test(web-shell): make visual-preview captures deterministic + add workspace-sidebar scenario - #7041
Merged
wenshao merged 3 commits intoJul 17, 2026
Conversation
…ace-sidebar scenario The split-view "restored" screenshot was byte-nondeterministic between identical renders (the reappearing pane re-renders its content just after the restore click), so it periodically diffed above the before/after threshold and surfaced a false-positive "changed view" unrelated to the PR under review. It is also visually identical to the tiled `split view` shot. Drop the capture but keep the restore click + "both panes back" assertion, so the restore path still has behavioral coverage. Add a `workspace sidebar` scenario with two workspaces so the sidebar groups sessions per workspace and tags the primary one. This is the only scenario that renders the primary-workspace label/badge (it is gated on more than one displayed workspace), so changes to those labels — which no single-workspace scenario can surface — now show up in the visual preview.
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
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. |
Collaborator
|
Thanks for the PR! (Re-run at Template looks good ✓ Problem: Observed, with concrete evidence. The pixel-diff comparison of two identical renders shows Direction: Aligned. The visual-preview suite needs deterministic captures to be useful. Freezing infinite animations at the source, dropping a redundant capture, and adding coverage for the primary-workspace badge (a surface no other scenario exercises) are all straightforward test-infrastructure improvements. No product-direction concerns. Size: Not applicable — test-only change under Approach: Scope is tight — three focused commits. The freeze mechanism ( Moving on to code review. 🔍 中文说明感谢贡献！（在 模板完整 ✓ 问题： 已观测到的问题，有具体证据。两次相同渲染的像素级 diff 显示 方向： 对齐。视觉预览套件需要确定性截图才能发挥作用。从源头冻结无限动画、删除冗余截图、并增加对主工作区徽章的覆盖（其他场景都无法触及的界面）都是对测试基础设施的直接改进。无产品方向方面的顾虑。 规模： 不适用——仅测试改动，位于 方案： 范围紧凑——三个聚焦的提交。冻结机制（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Code review (at Three files, 157 additions / 2 deletions — all under The freeze mechanism is correct: The contract test ( The The removed No correctness bugs, no security concerns, no AGENTS.md convention violations. Code style, comments, and license headers match the existing suite. CI validation (all green on this branch):
The visual suite on CI includes the new contract test and workspace sidebar scenario — both pass alongside the existing 21 tests. 中文说明代码审查（ 三个文件，157 行增加 / 2 行删除——全部位于 冻结机制正确： 契约测试（
删除的 无正确性 bug，无安全隐患，无 AGENTS.md 规范违反。代码风格、注释和许可证头与现有套件匹配。 CI 验证（此分支全部通过）：
CI 上的视觉套件包含新的契约测试和 workspace sidebar 场景——与现有 21 个测试一起全部通过。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 Clean test-only PR, improved since the last pass. The freeze mechanism is sound and now pinned by a contract test. The workspace sidebar scenario fills a real coverage gap (the only view exercising the primary-workspace badge) with explicit dependency pinning. CI validates the full suite (visuals, E2E smoke, unit tests) — all green. Two non-blocking notes for the author:
LGTM. ✅ 中文说明置信度：5/5 干净的纯测试 PR，自上次审查以来有改进。冻结机制正确且现由契约测试固定。workspace sidebar 场景填补了真实的覆盖缺口（唯一使用主工作区徽章的视图），并显式固定了依赖。CI 验证完整套件（视觉、E2E 冒烟、单元测试）——全部通过。 两个非阻塞建议给作者：
LGTM。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 16, 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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
The sidebar's activity spinner is a GPU-composited transform loop that Playwright's `animations: 'disabled'` captures mid-rotation at a random angle, so `sidebar attention` differed in ~0.12% of pixels between two identical renders — above the 0.02% before/after threshold, i.e. a false-positive "changed view" on any PR that renders it. Before each capture, pause every infinite Web Animation and rewind it to time 0 (a two-frame wait lets the compositor commit the frozen frame); finite animations are still left to `animations: 'disabled'`. Verified with a pixel diff: the whole suite now renders pixel-identical across two runs (worst 0.0001% of pixels, vs the 0.02% threshold).
wenshao
changed the title
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Jul 16, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: Agent 0: Issue fidelity & root-cause ownership — no prompt was built for it (agent-prompt --role 0 never ran). Not reviewed: Agent 1a: Line-by-line correctness — no prompt was built for it (agent-prompt --role 1a never ran). Not reviewed: Agent 2: Security — no prompt was built for it (agent-prompt --role 2 never ran). Not reviewed: Agent 3: Code quality — no prompt was built for it (agent-prompt --role 3 never ran). Not reviewed: Agent 4: Performance & efficiency — no prompt was built for it (agent-prompt --role 4 never ran). Not reviewed: Agent 5: Test coverage — no prompt was built for it (agent-prompt --role 5 never ran). Not reviewed: Agent 6a: Undirected audit — attacker mindset — no prompt was built for it (agent-prompt --role 6a never ran). Not reviewed: Agent 6b: Undirected audit — 3 AM oncall mindset — no prompt was built for it (agent-prompt --role 6b never ran). Not reviewed: Agent 6c: Undirected audit — six-months-later maintainer — no prompt was built for it (agent-prompt --role 6c never ran). Not reviewed: Agent 1b: Removed-behavior audit — no prompt was built for it (agent-prompt --role 1b never ran). Not reviewed: Agent 1c: Cross-file tracer — no prompt was built for it (agent-prompt --role 1c never ran). Not reviewed: Agent 7: Build & test verification — no prompt was built for it (agent-prompt --role 7 never ran). Not reviewed: reverse audit — no auditor ran (Step 5 builds its prompt with agent-prompt --role reverse-audit; none was recorded, so the pass that looks for what Step 3 missed was skipped). Not reviewed: verification — the review posts findings, but no verifier ran (Step 4 builds its prompt with agent-prompt --role verify; none was recorded, so the findings were not verified).
— qwen3.7-max via Qwen Code /review
…reeze Address review on the visual-capture determinism changes: - Note freezeLoopingAnimations' coverage scope in its docstring (WAAPI + CSS @Keyframes via document.getAnimations(), not a hand-rolled requestAnimationFrame loop), so a future spinner rewrite that reintroduces the flake leads a debugger back to this function. - Pin the workspace-sidebar scenario's primary workspace cwd and loaded session name explicitly rather than leaning on createWebShellDaemonScenario defaults, so renaming those defaults in mockDaemon.ts can't turn the settle-wait into a cryptic "not visible" failure. - Add harness.spec.ts pinning the freeze contract: an infinite animation is paused and rewound to time 0, while a finite one is left running.
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Jul 16, 2026
doudouOUC
reviewed
Jul 16, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
Code review @
|
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 16, 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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
doudouOUC
approved these changes
Jul 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. I reviewed the full diff and the changes are coherent and well-scoped:
- Removing the local verify:pr path and aligning docs/scripts with npm run preflight + CI simplifies the contribution workflow without leaving dangling references.
- The MCP HTTP OAuth capture logic is correctly guarded by server config and has solid test coverage for the 401 / WWW-Authenticate handshake path.
- The web-shell visual-regression hardening (freezing infinite WAAPI animations, removing the redundant restored split-view screenshot, and adding the extensions manager + workspace sidebar scenarios) directly addresses the stated determinism goal.
- The channel daemon-worker sourceType change is narrowly applied to newly created sessions and is backed by a targeted test.
No critical issues, warnings, or blocking concerns were found. Approving.
This was referenced Jul 19, 2026
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
Makes the web-shell visual-preview captures deterministic and adds a scenario for a class of change the suite couldn't see. Three parts:
Freeze looping animations before each capture. The sidebar's activity spinner is a GPU-composited transform loop that Playwright's
animations: 'disabled'captures mid-rotation at a random angle, so thesidebar attentionview differed in ~0.12% of pixels between two identical renders — above the 0.02% before/after threshold, i.e. a false-positive "changed view" on any PR that renders it.captureScreenshotnow pauses every infinite Web Animation and rewinds it to time 0 (a two-frame wait lets the compositor commit the frozen frame) before shooting.Drop the split-view "restored" screenshot. It is visually identical to the tiled
split viewshot above it (restore returns to the same two-pane layout), so it adds no coverage that shot doesn't — and it was the specific view refactor(web-shell): drop redundant primary-workspace label #7035's preview flagged. The restore click and the "both panes back" assertion stay, so the restore path keeps its behavioral coverage; only the redundant capture is removed.Add a
workspace sidebarscenario with two workspaces, so the sidebar groups sessions per workspace and tags the primary one. It is the only scenario that renders the primary-workspace label/badge (gated on more than one displayed workspace), so changes to those labels — which no single-workspace scenario can surface — now appear in the preview.Why it's needed
The before/after preview on #7035 flagged
split-view-restored-darkas the only "changed" view, even though that PR (dropping a redundant primary-workspace label) touches neither split-view rendering nor anything else the suite captures. Root causes: animated content (spinners) is captured at a random frame and reads as changed between the base and head render passes, and the PR's real change was invisible because no scenario exercises multiple workspaces. This freezes the animations at the source, drops a redundant capture, and adds the missing scenario.Reviewer Test Plan
How to verify
|ΔR|+|ΔG|+|ΔB| > 30, matching the bot):Before this PR,
sidebar-attention-*differs by ~0.12% of pixels (the spinner). With the freeze, every one of the 22 views is pixel-identical (worst 0.0001%, threshold 0.02%).npx playwright test --config playwright.visuals.config.ts -g "workspace sidebar" --project=chromium→2 passed; the primary workspace (qwen-web-shell-e2e) carries aPrimarybadge, the other (qwen-api-service) does not.npm run test:e2e:visuals→23 passed(was 21; +2 for the new scenario × 2 themes). Nosplit-view-restored-*.pngis emitted;workspace-sidebar-*.pngis.Evidence (Before & After)
Freeze — pixel-diff of two identical renders (the number the 0.02% threshold actually measures;
cmpbyte-compare is misleading here because PNG encoding is non-deterministic even for pixel-identical frames):New
workspace sidebarscenario — the surface the preview will now watch for primary-workspace-label changes (thePrimarybadge is exactly the node a "drop the primary label" change removes):Tested on
Verified locally on macOS; Windows/Linux left to CI (the visuals job runs on Linux).
Environment (optional)
npm run devweb-shell dev server + Playwright (chromium), driven byplaywright.visuals.config.ts.Risk & Scope
client/e2e/visuals/).captureScreenshotnow runs a smallpage.evaluatebefore every shot. It only touches infinite Web Animations (pause + rewind to 0); finite animations are left to Playwright, so settled dialogs/transitions are unaffected. On this PR's own before/after preview, spinner-bearing views (e.g.sidebar attention) will read as "changed" once — base renders the spinner at a random angle, head freezes it — which is the fix demonstrating itself; it stabilizes afterwards.SplitViewsession-picker andSessionOverviewPanelvariants of the primary label (the sidebar badge is its highest-signal surface).Linked Issues
None. Follows the visual-preview suite (#6880), the before/after engine (#6963), and the scenario set (#6964, #6997). Motivated by the flaky preview observed on #7035.
中文说明
这个 PR 做了什么
让 web-shell 视觉预览的截图具备确定性,并新增一个套件此前无法覆盖的改动类别的场景。三部分:
每次截图前冻结循环动画。 侧边栏的活动转圈图标是一个 GPU 合成的 transform 循环,Playwright 的
animations: 'disabled'会在随机角度把它截下来,导致sidebar attention视图在两次相同渲染之间有约 0.12% 的像素不同——超过了 before/after 的 0.02% 阈值,也就是任何渲染到它的 PR 上都会出现的假"变更视图"。captureScreenshot现在会在截图前暂停每一个无限 Web Animation 并把它回退到 time 0(等两帧让合成器提交冻结帧)。删掉 split-view "restored"(恢复)截图。 它与上面那张平铺的
split view截图视觉一致(恢复回到同样的双 pane 布局),不提供额外覆盖——而且它正是 refactor(web-shell): drop redundant primary-workspace label #7035 预览标记的那张。恢复的点击与"两个 pane 都回来了"的断言保留(恢复路径的行为覆盖仍在),只删掉这张冗余截图。新增
workspace sidebar(工作区侧边栏)场景,含两个工作区,使侧边栏按工作区分组会话并给主工作区打标。它是唯一会渲染主工作区标签/徽章的场景(仅在展示的工作区多于一个时才出现),因此对这些标签的改动——单工作区场景无法呈现的——现在会出现在预览里。为什么需要它
#7035 的 before/after 预览把
split-view-restored-dark标为唯一的变更视图,尽管那个 PR(删除一个冗余的主工作区标签)既没动 split-view 渲染,也没动套件截取的任何东西。根因:动画内容(转圈)被截在随机帧,于是在 base 与 head 两次渲染之间读作"变更";而该 PR 的真实改动不可见,因为没有任何场景用到多工作区。本 PR 从源头冻结动画、删掉一张冗余截图、并补上缺失的场景。复现测试计划
如何验证
|ΔR|+|ΔG|+|ΔB| > 30时算一个像素不同,与 bot 一致):本 PR 之前,
sidebar-attention-*有约 0.12% 的像素不同(转圈);加上冻结后,22 个视图每一个都逐像素一致(最差 0.0001%,阈值 0.02%)。npx playwright test --config playwright.visuals.config.ts -g "workspace sidebar" --project=chromium→2 passed;主工作区(qwen-web-shell-e2e)带Primary徽章,另一个(qwen-api-service)没有。npm run test:e2e:visuals→23 passed(原为 21;新场景 × 2 主题 = +2)。不再产出split-view-restored-*.png;产出workspace-sidebar-*.png。证据(Before & After)
冻结——两次相同渲染的像素级 diff(这才是 0.02% 阈值实际度量的数值;
cmp逐字节比较在这里会误导,因为 PNG 编码即便对逐像素一致的帧也非确定):新的
workspace sidebar场景——预览今后据此监视主工作区标签的改动(Primary徽章正是"删主标签"类改动会移除的节点):见上方深色/浅色两图。测试平台
仅在本地 macOS 验证;Windows/Linux 交由 CI(视觉任务在 Linux 上运行)。
风险与范围
client/e2e/visuals/下)。captureScreenshot现在每次截图前会跑一小段page.evaluate。它只碰无限的 Web Animation(暂停 + 回退到 0);有限动画交给 Playwright,因此已稳定的弹窗/过渡不受影响。在本 PR 自己的 before/after 预览上,带转圈的视图(如sidebar attention)会一次性读作"变更"——base 把转圈截在随机角度,head 冻结它——这正是修复本身的体现,之后即稳定。SplitView会话选择器和SessionOverviewPanel里的变体(侧边栏徽章是它信号最强的一面)。关联 Issue
无。延续视觉预览套件(#6880)、before/after 引擎(#6963)与场景集合(#6964、#6997)。由 #7035 上观察到的 flaky 预览触发。