fix(ci): tell a visuals coverage gap apart from "no visual change"

[wenshao]

What this PR does

When the web-shell visual preview finds that no view changed, it now checks which files the PR actually touched before declaring victory. If any of them shape what a view looks like, the comment says the result is ambiguous and lists them, instead of printing a green check. If nothing render-shaping changed, the green check stays exactly as it is today.

Why it's needed

An empty preview means one of two opposite things: the change genuinely moves no pixel, or no scenario renders the UI it touches. The bot printed the same ✅ _No screenshot changes against the PR base._ for both — so the second case, where the preview literally cannot see the feature, read as a clean bill of health.

That is not hypothetical. It has happened three times, each caught only because a maintainer noticed the missing image and asked about it:

PR	New UI	Why no view changed
#7035	Primary-workspace label	needs ≥2 workspaces; every scenario had one
#7221	Worktree session badge	needs session.worktree metadata; no scenario set it
#7365	New-session worktree toggle	lives at /; every scenario goes to /session/:id

The signal needed to tell the two cases apart already existed and was simply unused: the render workflow only triggers on packages/web-shell/client/** and packages/webui/src/**, so an empty preview is by construction "UI code changed, yet nothing rendered differently". That is worth a second look, and nobody was taking it.

The prompt is deliberately narrow, because a warning everyone learns to skip is worse than no warning. Only .tsx / .css / .svg under the rendered surface count — .ts hooks, utils and types routinely land with no visual delta, and test/scenario code drives the preview rather than being previewed. Every .svg under that surface is a bundled UI icon (client/assets/icons/), so a changed icon that moves no pixel is the same signal.

Reviewer Test Plan

How to verify

Unit tests cover the classifier and both comment branches:

node --test .github/scripts/web-shell-visuals-publish.test.mjs
# tests 19 / pass 19 / fail 0

The whole CI helper set still passes (88/88), and actionlint is clean on the modified workflow.

To see the real output, drive the CLI the way the workflow does — an empty stage dir plus a changed-paths file:

printf '%s\n' \
  'packages/web-shell/client/components/WelcomeScreen.tsx' \
  'packages/web-shell/client/components/WorktreeWelcomeToggle.module.css' \
  'packages/web-shell/client/hooks/useWorktreeToggle.ts' \
  'packages/web-shell/client/e2e/visuals/screenshots.spec.ts' \
  'packages/core/src/config.ts' > /tmp/paths.txt
mkdir -p /tmp/stage
node .github/scripts/web-shell-visuals-publish.mjs comment \
  /tmp/stage '' abc1234 https://run.example/1 /tmp/body.md /tmp/paths.txt
cat /tmp/body.md

Confirm it names only the .tsx and the .css — the .ts hook, the spec file and the packages/core change are all correctly ignored. Then re-run with only useThing.ts in the paths file and confirm the green check comes back unchanged.

Evidence (Before & After)

Before — what the bot posted on #7365, a PR whose entire point was new UI (real comment):

#### Screenshots · before / after

✅ _No screenshot changes against the PR base._

After — same run, same zero images:

#### Screenshots · before / after

ℹ️ _No screenshot changed against the PR base_ — but this PR edits 2 render-shaping files:

- `packages/web-shell/client/components/WelcomeScreen.tsx`
- `packages/web-shell/client/components/WorktreeWelcomeToggle.module.css`

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 `packages/web-shell/client/e2e/visuals/screenshots.spec.ts` that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Control — a logic-only PR is left alone. With only packages/web-shell/client/hooks/useThing.ts changed, the output is byte-identical to today's:

✅ _No screenshot changes against the PR base._

Control — the API call failing does not break the comment. With no changed-paths file at all (what happens if the pulls/:n/files request fails), the output is again today's green check. The workflow logs a ::warning:: and carries on.

Tested on

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

Environment (optional)

node --test for the helper tests, actionlint for the workflow, plus the CLI invocation above. No daemon or browser needed — the changed code is a pure comment builder plus one gh api call.

Risk & Scope

• Main risk or tradeoff: false prompts on PRs that touch a .tsx/.css/.svg with no visual delta — a prop rename, a comment, dead-code removal. The wording leads with "either the change has no visual effect", and the file list makes it a one-glance dismissal, but the rate is not knowable until it runs on real traffic. If it turns out noisy, narrowing to added/renamed files or gating on an additions threshold is a small follow-up.
• Not validated / out of scope: this does not detect the gap, it only asks the question — nothing here proves a given component is unrendered. A real coverage map (which components each scenario mounts) would answer it directly and is a much larger change. The two path prefixes are duplicated from the render workflow's paths: trigger and must stay in sync by hand; a comment on each side flags this.
• Breaking changes / migration notes: none. Comment text only; no change to when the preview runs, what it renders, or how assets are hosted.

Linked Issues

Follow-up to #6880 (the preview bot), #6963 (before/after compositor), #7041 (animation freeze) and #7210 (cluster denoise). Motivated by the coverage gaps in #7035, #7221 and #7365.

中文说明

这个 PR 做了什么

当 web-shell 视觉预览发现没有任何视图变化时,先看这个 PR 到底改了哪些文件,再决定怎么说。如果改动里有影响渲染的文件,评论会说明这个结果是有歧义的并列出这些文件,而不是打一个绿勾。如果没有改到渲染相关的文件,绿勾保持原样。

为什么需要

空预览意味着两件完全相反的事:改动确实不产生任何像素差异,或者根本没有场景渲染到它所改的 UI。而 bot 对两种情况打印的是同一句 ✅ _No screenshot changes against the PR base._——于是第二种情况(预览压根看不见这个功能)被读成了"一切正常"。

这不是假设。已经发生三次了,每次都只是因为维护者碰巧注意到少了配图才问出来:

PR	新 UI	为什么没有视图变化
#7035	Primary workspace 标签	需要 ≥2 个 workspace,而所有场景都只有 1 个
#7221	Worktree 会话徽章	需要 session.worktree 元数据,没有场景设置过
#7365	新建会话的 worktree 开关	位于 /,而所有场景都进 /session/:id

区分这两种情况所需的信号其实早就存在、只是没被用起来:渲染工作流只在 packages/web-shell/client/** 和 packages/webui/src/** 改动时才触发,所以一个空预览按构造必然是"改了 UI 代码,却没有任何东西渲染得不一样"。这值得看第二眼,而此前没有人去看。

提示的触发面刻意收窄,因为一个所有人都学会忽略的警告比没有警告更糟。只有渲染面下的 .tsx / .css / .svg 算数——.ts 的 hook、工具函数和类型经常正常地不带视觉差异,而测试与场景代码是驱动预览的,本身不是被预览的对象。该目录下的 .svg 全部是打包进 bundle 的 UI 图标(client/assets/icons/),所以图标改了却零像素差,是同一个信号。

如何验证

单元测试覆盖了分类器和评论的两个分支:node --test .github/scripts/web-shell-visuals-publish.test.mjs → 19/19 通过。整套 CI helper 测试仍然全绿(88/88),actionlint 对改动的工作流无告警。

想看真实输出,就按工作流的方式跑 CLI(空的 stage 目录 + 一个改动路径文件),见上文英文部分的命令。确认它只列出 .tsx 和 .css——.ts hook、spec 文件、packages/core 的改动都被正确忽略。然后把路径文件换成只有 useThing.ts,确认绿勾原样返回。

风险与范围

• 主要风险/取舍: 对那些改了 .tsx/.css/.svg 但确实没有视觉差异的 PR(改个 prop 名、改注释、删死代码)会误报。文案以"要么这个改动没有视觉影响"开头,加上文件列表,一眼就能排除,但真实误报率要跑过真实流量才知道。如果确实噪音大,收窄到"新增/重命名的文件"或加一个新增行数阈值,都是很小的后续改动。
• 未验证/不在范围内: 这个改动不检测缺口,只是把问题问出来——它并不能证明某个组件确实没被渲染。真正的覆盖率地图(每个场景挂载了哪些组件)可以直接回答,但那是大得多的改动。两个路径前缀是从渲染工作流的 paths: 触发器手工复制过来的,需要人工保持同步,两边都加了注释标注这一点。
• 破坏性改动: 无。只改评论文案,不改预览何时运行、渲染什么、以及资源如何托管。

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

Problem: observed and recurring. Three real PRs (#7035, #7221, #7365) shipped new UI that the preview bot couldn't see, and each time the green check masked a coverage gap that only a human happened to notice. This is not theoretical — it has a track record.

Direction: aligned. This is a CI quality-of-life fix for the visual preview bot, squarely within the project's infrastructure. CHANGELOG has no direct reference, but the area is clearly relevant — the preview bot is an active part of the review workflow.

Size: not applicable — no core module paths touched (all changes are in .github/scripts/ and .github/workflows/).

Approach: the scope feels right. The fix is proportionate — it only changes what the comment says when the preview is empty, and only for the narrow set of files that actually shape rendering (.tsx/.css/.svg under the rendered surface). The fallback on API failure is safe (existing green check). No unrelated changes or drive-by refactors. The one thing I'd watch is the false-positive rate on PRs that legitimately touch a .tsx with no visual delta (prop renames, dead-code removal), but the PR already acknowledges this and the wording leads with "either the change has no visual effect" — a reasonable hedge.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

问题：已观测到且反复出现。三个真实 PR（#7035、#7221、#7365）的新 UI 预览 bot 看不见，每次绿勾都掩盖了只有人碰巧注意到的覆盖缺口。这不是理论问题——有实际记录。

方向：对齐。这是视觉预览 bot 的 CI 质量改进，完全在项目基础设施范围内。CHANGELOG 无直接引用，但该领域明显相关——预览 bot 是审查流程的活跃部分。

规模：不适用——未触及核心模块路径（所有改动在 .github/scripts/ 和 .github/workflows/）。

方案：范围合理。修复是成比例的——只改预览为空时评论的措辞，且只针对真正影响渲染的文件（渲染面下的 .tsx/.css/.svg）。API 失败时的回退是安全的（保持现有绿勾）。没有无关改动或顺手重构。唯一需要关注的是对确实改了 .tsx 但无视觉差异的 PR（改 prop 名、删死代码）的误报率，但 PR 已经承认了这一点，文案以"要么这个改动没有视觉影响"开头——合理的对冲。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

_{Reviewed at f38633fad895ccc63ad9d61899bf9ffe6e8ca577 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code Review

Independent proposal: given the ambiguity of an empty preview, I would filter the PR's changed files for render-shaping extensions (.tsx/.css/.svg) under the rendered directories, exclude test/spec code, and replace the green check with an informational message when any match. Pass the file list via a temp file (not argv — paths are attacker-influenced on fork PRs), and fall back to the existing green check if the API call fails.

Comparison with the diff: the PR's approach matches this almost exactly. The implementation is clean and well-structured:

• selectRenderShapingFiles is a pure, exported, tested function with a clear contract ({ files, total }). The prefix/extension/exclusion filters are correct and deliberately narrow — .ts hooks and utils are excluded to avoid training people to ignore the prompt.
• codePath handles the fork-controlled path injection vector correctly: strips backticks/CR/LF, then HTML-escapes. The test with <img src=x onerror=alert(1)> confirms no HTML injection is possible.
• The buildComment integration is minimal — only the empty-preview branch changes, and the existing green check is preserved for logic-only PRs and API failures.
• The workflow change is best-effort: gh api failure logs a ::warning:: and the comment falls back to today's behavior. Reading the file list in the publish job (not the render job) is the right call since the render job runs untrusted PR code.
• Tests are thorough: 8 new tests covering the classifier, both comment branches, overflow, undefined input, and the injection case. All 19 tests pass.

No critical blockers. No AGENTS.md violations. The two path prefixes are duplicated from the render workflow's paths: trigger and need manual sync — the PR flags this with comments on both sides, which is the pragmatic choice for a CI helper.

Real-Scenario Testing

Ran the script the way the workflow does — empty stage dir + changed-paths file, comparing base code vs PR code.

Before (base code — green check regardless)

=== BEFORE (base code): empty preview with UI changes ===
<!-- qwen:web-shell-visuals -->
### 🖼️  web-shell visual preview
Rendered against a mock daemon (no real backend): the PR base vs this PR head `abc1234`. Only **screenshots** that changed are shown (flows below, if any, are head-only) — refreshes on every push.

#### Screenshots · before / after

✅ _No screenshot changes against the PR base._

<sub>Full-resolution recordings (.webm) are attached to the <a href="https://run.example/1">workflow run</a>.</sub>

— _Qwen Code · web-shell visuals_

After (this PR — coverage gap flagged)

=== AFTER (PR code): empty preview with UI changes ===
<!-- qwen:web-shell-visuals -->
### 🖼️  web-shell visual preview
Rendered against a mock daemon (no real backend): the PR base vs this PR head `abc1234`. Only **screenshots** that changed are shown (flows below, if any, are head-only) — refreshes on every push.

#### Screenshots · before / after

ℹ️  _No screenshot changed against the PR base_ — but this PR edits 2 render-shaping files:

- `packages/web-shell/client/components/WelcomeScreen.tsx`
- `packages/web-shell/client/components/WorktreeWelcomeToggle.module.css`

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 `packages/web-shell/client/e2e/visuals/screenshots.spec.ts` that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

<sub>Full-resolution recordings (.webm) are attached to the <a href="https://run.example/1">workflow run</a>.</sub>

— _Qwen Code · web-shell visuals_

Control (logic-only PR — green check preserved)

=== CONTROL (PR code): logic-only PR ===
<!-- qwen:web-shell-visuals -->
### 🖼️  web-shell visual preview
Rendered against a mock daemon (no real backend): the PR base vs this PR head `abc1234`. Only **screenshots** that changed are shown (flows below, if any, are head-only) — refreshes on every push.

#### Screenshots · before / after

✅ _No screenshot changes against the PR base._

<sub>Full-resolution recordings (.webm) are attached to the <a href="https://run.example/1">workflow run</a>.</sub>

— _Qwen Code · web-shell visuals_

The .ts hook, the spec file, and the packages/core change are all correctly ignored — only the .tsx and .css are flagged. Unit tests: 19/19 pass.

中文说明

代码审查

独立方案： 鉴于空预览的歧义性，我会过滤 PR 改动文件中渲染面下的渲染相关扩展名（.tsx/.css/.svg），排除测试/spec 代码，并在有匹配时用信息性消息替换绿勾。通过临时文件传递文件列表（而非 argv——fork PR 的路径受攻击者影响），API 调用失败时回退到现有绿勾。

与 diff 的对比： PR 的方案几乎完全匹配。实现干净、结构良好：

• selectRenderShapingFiles 是一个纯函数、已导出、已测试，契约清晰（{ files, total }）。前缀/扩展名/排除过滤器正确且刻意收窄——.ts hook 和工具函数被排除，避免训练人们忽略提示。
• codePath 正确处理了 fork 控制的路径注入向量：先去除反引号/CR/LF，再做 HTML 转义。<img src=x onerror=alert(1)> 测试确认不可能进行 HTML 注入。
• buildComment 的集成是最小的——只改了空预览分支，逻辑 PR 和 API 失败时保留现有绿勾。
• 工作流改动是尽力而为的：gh api 失败时记录 ::warning::，评论回退到今天的行为。在 publish job（而非 render job）中读取文件列表是正确的，因为 render job 运行不受信任的 PR 代码。
• 测试全面：8 个新测试覆盖了分类器、两个评论分支、溢出、undefined 输入和注入场景。全部 19 个测试通过。

无关键阻塞项。无 AGENTS.md 违规。两个路径前缀从渲染工作流的 paths: 触发器手工复制，需要人工同步——PR 在两边都加了注释标注，对 CI helper 来说是务实的选择。

真实场景测试

按工作流的方式运行脚本——空的 stage 目录 + 改动路径文件，对比基础代码和 PR 代码。

• Before（基础代码）： 即使改了 UI 文件也显示绿勾——这就是 bug。
• After（PR 代码）： 正确识别覆盖缺口，列出 2 个渲染相关文件，.ts hook、spec 文件和 packages/core 改动被正确忽略。
• Control（仅逻辑 PR）： 绿勾保留，无误报。

单元测试：19/19 通过。

— Qwen Code · qwen3.7-max

_{Reviewed at f38633fad895ccc63ad9d61899bf9ffe6e8ca577 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — clean across every stage; the problem is real, the fix is proportionate, and the implementation is exactly what I would have written.

This PR does one thing and does it well. Three real incidents (#7035, #7221, #7365) showed that the preview bot's green check can mask a coverage gap — new UI that no scenario renders reads as "nothing broke." The fix disambiguates by checking which files the PR touched before declaring victory, and only for the narrow set of extensions that actually shape rendering. The fallback is safe (API failure → existing green check), the injection handling is correct (backtick stripping + HTML escaping on fork-controlled paths), and the tests cover every branch including the adversarial case.

The before/after confirms the behavior change is exactly what was promised: the .tsx and .css get flagged, the .ts hook and spec file are correctly ignored, and a logic-only PR still gets the green check. 19/19 tests pass.

If I had to maintain this in six months, I'd thank the author — the code is well-commented, the design decisions are documented, and the scope is tight. No drive-by refactors, no unrelated changes, no over-abstraction.

中文说明

置信度：5/5 —— 每个阶段都干净；问题真实存在，修复成比例，实现正是我会写的样子。

这个 PR 只做一件事，而且做得好。三次真实事件（#7035、#7221、#7365）表明预览 bot 的绿勾可能掩盖覆盖缺口——没有场景渲染的新 UI 被读成"没坏"。修复通过在宣布胜利前检查 PR 改了哪些文件来消除歧义，且只针对真正影响渲染的扩展名。回退是安全的（API 失败 → 现有绿勾），注入处理正确（对 fork 控制的路径去除反引号 + HTML 转义），测试覆盖了每个分支包括对抗性场景。

Before/after 确认行为变化正是所承诺的：.tsx 和 .css 被标记，.ts hook 和 spec 文件被正确忽略，仅逻辑的 PR 仍然得到绿勾。19/19 测试通过。

如果六个月后我要维护这个，我会感谢作者——代码注释良好，设计决策有文档，范围紧凑。没有顺手重构，没有无关改动，没有过度抽象。

— Qwen Code · qwen3.7-max

_{Reviewed at f38633fad895ccc63ad9d61899bf9ffe6e8ca577 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

[yiliang114]

changes for web shell, LGTM

[yiliang114]

LGTM — the logic is correct and the tests replay the real extracted blocks convincingly.

Non-blocking, for a follow-up: the amount of branching logic living inside run: | blocks (GraphQL queries, jq pipelines, while-read loops, multi-arm conditionals) is getting hard to maintain — tests have to regex-extract YAML substrings and replay them under bash, which couples test correctness to YAML indentation. Worth extracting these into .github/scripts/autofix-*.mjs standalone scripts (same pattern as web-shell-visuals-publish.mjs) so they get proper linting, direct unit-test imports, and reuse across workflows. Not blocking this PR — the current approach works and is well-tested; just flagging the direction for when the next chunk of logic lands here.

[qwen-code-ci-bot]

Released in v0.20.1.

[yiliang114]

PR #7375 (fix(ci): tell a visuals coverage gap apart from "no visual change") has been released in v0.20.1. This is an automated release notification from qwen-code-ci-bot — no code changes or reply needed. Acknowledged.

---

✅ completed