ci(web-shell): before/after visual previews, showing only changed views - #6963
Merged
Conversation
The visual-preview bot posted the same fixed set of canned screenshots on every web-shell PR, so it could not show what a PR actually changed (a mermaid/split feature was invisible) and added noise on PRs that touch the UI only trivially. Render each scenario against BOTH the PR base (`main`) and the PR head, pixel-diff them, and post a stitched "main | this PR" composite for only the views that CHANGED. A PR with no visual impact composites nothing → "no visual change". This makes the preview feature-aware with no per-PR understanding: the diff finds exactly the surface the PR moved (and it subsumes the backend-PR noise #6959 pre-filtered, at the content level). - web-shell-visuals-compose.mjs: pixel-diff (canvas) + stitch a labelled composite; pure helpers (parseShot/isChanged/planWork) unit-tested. - web-shell-visuals.yml: also render the base — trusted `main` via pull_request.base.sha, so no secret exposure in the untrusted-PR job — then compose; composites replace the raw after-shots (same `<view>-<theme>.png` name the publisher already expects). - publish buildComment: list composites; "no visual change" when none. Verified locally by overlaying #6881's real changes onto main + a new mermaid scenario: the compositor flagged the mermaid view 6.5% changed (its new zoom controls) and correctly skipped the unchanged transcript.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed and well-documented — the visual preview bot (#6880) posted the same canned screenshots on every PR, so feature PRs like #6881 (mermaid zoom) and #6951 (split-pane maximize) never appeared in the preview. #6959 already stopped it firing on pure-backend PRs; this makes the remaining previews feature-aware at the content level. Direction: aligned. CI tooling that reduces noise and makes the preview bot actually useful is clearly within scope. No CHANGELOG reference needed for internal CI infrastructure. Size: 437 production lines, 110 test lines, 0 generated. No core paths touched — all changes are in Approach: the scope feels right. The compositor (pixel-diff + stitch), the workflow wiring (merge-base checkout + before render), and the publish updates (no-change comment handling) are all needed together and none is extraneous. The merge-base choice over base-branch tip is correct — prevents showing already-landed changes as "reversed" in the before. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的问题——视觉预览机器人(#6880)在每个 PR 上发同一套罐头截图，导致 #6881(mermaid 缩放)、#6951(分屏最大化)等功能 PR 的预览完全看不到变化。#6959 已让它不再在纯后端 PR 上触发；本 PR 在内容层面让其余预览具备功能感知能力。 方向：对齐。减少噪音、让预览机器人真正有用的 CI 工具改进显然在范围内。内部 CI 基础设施不需要 CHANGELOG 引用。 规模：437 行生产代码，110 行测试，0 行生成代码。未触及核心路径——所有改动在 方案：范围合理。拼接器（像素 diff + 拼接）、workflow 接线（merge-base checkout + before 渲染）、发布更新（无变化评论处理）缺一不可，没有多余的部分。选择 merge-base 而非 base 分支头是正确的——避免把已合入的改动在 before 中显示为"反转"。base 步骤上的 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Code review — no blocking issues found. The compositor ( The pixel diff logic in The workflow wiring in The publish workflow changes handle the "no changed screenshots" case cleanly: The Tests: CI shows all green — web-shell visuals capture (10m17s), test suite (10m26s), web-shell E2E smoke (4m51s). This is CI infrastructure — no user-facing CLI behavior to drive in tmux. The "product" is the workflow itself, and CI already exercised it end-to-end on this PR (the capture job ran successfully with the new before/after wiring). 中文说明代码审查——未发现阻塞性问题。 拼接器（
发布 workflow 的改动干净地处理了"无可视变化"的情况：
测试：CI 全绿——web-shell visuals capture (10m17s)、测试套件 (10m26s)、web-shell E2E smoke (4m51s)。 这是 CI 基础设施——没有用户可见的 CLI 行为需要在 tmux 中驱动。"产品"就是 workflow 本身，CI 已在此 PR 上端到端地验证了它（capture job 使用新的 before/after 接线成功运行）。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage, ready to merge. This PR does exactly what it says: turns the visual preview bot from a noise source (same canned screenshots on every PR) into a signal source (only changed views, with a clear before/after composite). The merge-base approach is the right call — it avoids the reversed-diff trap that a naive base-branch-tip comparison would hit on branches behind The degradation design is particularly good: every base step is The publish workflow now treats zero composites as a meaningful state (updates the marker comment with "no screenshot changes") rather than silently exiting, so stale images from a prior run never linger. The The code has been through multiple rounds of human review (findings 1-6 + nits, then 5 Criticals), all addressed. CI is green. Author is a maintainer who clearly understands the CI infrastructure. No reservations. Ship it. ✅ 中文说明置信度：5/5 — 各阶段均通过，可以合并。 这个 PR 完全兑现了承诺：把视觉预览机器人从噪音源（每个 PR 都发同一套罐头截图）变成信号源（只显示变化的视图，带清晰的 before/after 拼接图）。merge-base 方案是正确的选择——避免了在落后于 降级设计特别好：每个 base 步骤都是 发布 workflow 现在将零拼接图视为有意义的状态（更新标记评论显示"无截图变化"），而不是静默退出，避免之前运行的过期图片残留。 代码经过多轮人工审查（findings 1-6 + nits，然后 5 个 Critical），全部解决。CI 全绿。作者是熟悉 CI 基础设施的 maintainer。没有顾虑。发吧。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 15, 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. ✅
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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /triage |
Collaborator
Author
ReviewOverviewThis turns the visuals bot's canned screenshots into a real before/after diff: render each scenario against the PR base and head, pixel-diff per Verified locally: Findings (ordered by priority)
Nits
VerdictSolid direction and careful execution on the security/naming contracts; the pure-helper extraction with tests follows the pattern that's worked for the publish script. I'd fix 1 (CI wiring + lazy import) before merge, and strongly consider 2–5 either here or as an immediate follow-up, since they all shape whether the first real runs succeed. 6 is a tuning knob to watch on early runs. 🤖 Generated with Claude Code — Claude Fable 5 |
qwen-code-ci-bot
previously approved these changes
Jul 15, 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. ✅
… robustness Addresses the /review findings on the before/after preview: - Lazy the @playwright/test import in the compositor so the pure exports (parseShot/isChanged/planWork) load dependency-free, and wire web-shell-visuals-compose.test.mjs into the github_ci_only test step — it was never actually running in CI. (finding 1) - Diff against the MERGE-BASE, not the base-branch tip, so a PR branch behind main doesn't render others' already-landed changes reversed as this PR's diff. (finding 2) - continue-on-error on the base checkout + install so a flaky base degrades to after-only instead of sinking the job; compose likewise degrades to the raw after-shots on failure. (finding 3) - timeout 20->30 (the job ~doubled) and scope the base render to screenshots.spec.ts, skipping the discarded flow videos. (finding 4) - diffPct: add img.onerror so a corrupt/truncated baseline PNG can't hang page.evaluate to the job timeout. (finding 5) - Lower CHANGED_PCT_THRESHOLD 0.1 -> 0.02 (~205px at 1280x800) so an icon swap or one-word label change isn't classified "no change". (finding 6) - Nits: correct the stdout comment, esc() the burned-in labels, and scope the comment wording to "screenshots" (flows are always head-only).
Collaborator
Author
|
Thanks — all six findings + nits addressed in the latest commit (and the two transferable ones ported to the daemon A/B sibling #6975).
Nits: fixed the misleading Tests: 17/17 across both |
doudouOUC
reviewed
Jul 15, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed at b97515b7 — the branch moved since my first pass and most of my earlier notes are already addressed. Verified on the current head: node --test for the compose + publish helpers passes, ESLint/Prettier are clean, and because this PR edits the workflow + compositor it triggers its own capture job, which exercises the merge-base → base-render → compose path end-to-end.
Addressed nicely since the earlier revision
- The base arm is now fully best-effort —
continue-on-erroron the merge-base checkout,npm ci, and the base render — so a flaky base degrades to after-only instead of sinking the job. - Keying "before" off the merge-base rather than the base-branch tip is the right call: a branch behind
mainwould otherwise show already-landed web-shell changes, reversed, as this PR's diff. - Compose no longer takes the whole preview down on failure:
img.onerrorsettles the load (no hang to the job timeout), an undecodable image counts as changed, and the step falls back to publishing the raw after-shots. - Lazy
@playwright/testimport + wiring the compose helper tests into the dependency-free CI test step.
One residual thing — Suggestion / question, not a blocker
The "no screenshot changes" comment likely never actually posts. The publish workflow gates on count == 0, where count = accepted screenshots + GIFs (web-shell-visuals-publish.yml L164-168, unmodified by this PR). For a PR that touches web-shell paths but changes nothing visual, screenshots compose to 0; and the capture logs on this PR show ::warning::ffmpeg not found on the runner → GIFs = 0. So count == 0 → publish exits with "nothing to publish", and the new empty branch in buildComment (✅ No screenshot changes against main``) never renders. Net: the header/description still promise "no visual impact → 'no visual change'", but in practice no comment is posted at all in that case (confirmed: this PR has had no visuals comment across its runs).
If surfacing the "no screenshot changes" note is the intent, the count == 0 gate would need to let the empty-but-ran case through (that lives in the untouched publish workflow, so likely a follow-up). If "no comment when nothing changed" is acceptable, consider softening the description so it doesn't promise one. Either way is fine — just flagging the description-vs-behavior mismatch.
Everything else (fork PRs falling back to the base tip when the compare API can't resolve a merge-base; the 0.02% sensitivity threshold) reads as deliberate, safe trade-offs. Nice iteration overall.
Collaborator
Author
|
@doudouOUC thanks for the re-review. On the residual — you're right: with 0 changed composites, the "no screenshot changes" body only reaches the PR when GIFs push the publish |
yiliang114
previously requested changes
Jul 15, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Not reviewed: coverage — could not read the agents' transcripts (the CLI did not export QWEN_CODE_PROJECT_DIR / QWEN_CODE_SESSION_ID, so this run cannot find the harness's record of what its agents did), so this run cannot show that any of the diff was read.
— GPT-5 via Qwen Code /review
- diffPct: a dimension change IS a visual change — comparing only the overlapping rectangle hid it (a taller viewport with unchanged top pixels read 0%). Short-circuit any size mismatch to changed. (Critical) - Composite/comment label: "PR base (before)" not "main" — the workflow also runs for release/**, whose base is not main. (Critical) - Merge-base resolve: retry the compare API, then emit an EMPTY sha and SKIP the base render (after-only) rather than falling back to the base-branch tip, which reintroduces the reversed-diff bug. (Critical) - Base steps get ids; the before render runs only when the base checkout AND install both succeeded — else base/ (nested under head) resolves node_modules up to head's and produces a hybrid before. (Critical) - Publisher: a zero-change run now UPDATES the marker comment (image-less "no screenshot changes") instead of exiting, so a prior preview's stale images + SHA do not linger. (Critical) Finding 6 (helper tests skip full CI) is a pre-existing repo-wide gap for every .github/scripts test; left for a focused follow-up.
doudouOUC
previously approved these changes
Jul 15, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the full diff and ran the checks locally — LGTM, approving. This is a well-scoped, well-reasoned CI-only change.
Verified locally
- ✅ Unit tests 17/17 (
compose.test.mjs+publish.test.mjs). - ✅ Lazy
@playwright/testimport is genuinely dependency-free — the pure exports load with nonode_modulespresent. This is load-bearing: thegithub_ci_onlystep inci.ymlis explicitly dependency-free and runs before anynpm ci, and this PR wirescompose.test.mjsinto it. - ✅ ESLint + Prettier clean; both workflow YAMLs parse.
- ✅ Wiring cross-checked against the real suite: output layout (
$OUTPUT/screenshots), the-- screenshots.spec.tspositional filter, and the fixed 1280×800 viewport (screenshots aren'tfullPage, so before/after dimensions always match — themin(w,h)indiffPctis purely defensive).
What's good
- Security model is sound. Untrusted PR code runs with
contents: readand no secrets; the privileged publish stays in the separateworkflow_run. The merge-base checkout adds no new exposure (trusted history; the job already builds head code). Head SHA is deliberately not shipped, and artifact size/count are capped before upload. - Genuinely fail-safe. Every base step is
continue-on-error; an emptybefore/degrades to after-only (all NEW); a compose failure leaves the raw after-shots; a corrupt PNG is treated as changed (shown, not silently dropped);img.onerrorguards against apage.evaluatehang to the job timeout. - Correctly diffs against the merge-base, not the base tip — so a branch behind
maindoesn't attribute others' already-landed changes, reversed, to this PR.
Minor / non-blocking
- Degraded-path wording: when compose fails (the
elsebranch), raw head-only shots are published but the comment still reads "Only screenshots that changed are shown" and alt-texts them "before/after". Cosmetic, and it's a rare double-failure path. - Removed scenarios never appear —
planWorkonly walksafternames, so a view the PR deletes produces no composite. Documented as a design choice; a deletion is arguably a visual change worth surfacing. - Base-arm version drift: the base arm installs its own
@playwright/testfrom the merge-base lockfile, but only the head's Chromium is installed. A mismatch fails the base render → silently degrades to after-only (only a CI warning signals it). Safe, but the before/after can quietly disappear on older bases. - The browser diff+stitch path (
diffPct, stitch) has no automated test — acknowledged in the description; its first real exercise is the next web-shell PR. The pure classification (isChanged) is covered.
None of these block. Nice work.
qwen-code-ci-bot
requested changes
Jul 15, 2026
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
previously approved these changes
Jul 15, 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. ✅
Collaborator
Author
|
Thanks @yiliang114 — all five Criticals fixed in
On finding 6 (helper tests only in |
A mid-loop rejection in diffPct (evaluate timeout / CDP disconnect on a corrupt or oversized PNG) exited composeCli via the exception and skipped browser.close(), leaking a ~200 MB Chromium child for the rest of the CI job. Wrap the page + loop in try/finally so the browser always closes. Also drop the stale "main (before)" labels from the docstring (the composite/comment say "PR base" now, since the workflow also runs for release/**).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
… truth Round-3 F6 fix duplicated the .github/scripts node:test list across the github_ci_only and full-profile steps; a missed edit would silently drop coverage in one path. Hoist it to a workflow-level env var both reference.
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
ytahdn
reviewed
Jul 16, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
Nice work on the before/after visual diff feature — the merge-base approach, continue-on-error degradation chain, and lazy Playwright import are all well-designed. A few minor suggestions inline.
— qwen3.7-max via Qwen Code /review
| } | ||
| } finally { | ||
| await browser.close(); | ||
| } |
Collaborator
There was a problem hiding this comment.
[Suggestion] browser.close() in finally can swallow the original error — if Chromium crashes mid-diff and close() also throws (e.g. CDP disconnect), the diagnostic from the real failure is replaced by the cleanup error.
Suggested change
| } | |
| } finally { | |
| try { await browser.close(); } catch { /* ignore cleanup errors */ } | |
| } |
— qwen3.7-max via Qwen Code /review
| // job's own Chromium (which already runs PR code — no boundary crossed), so | ||
| // this is purely hygiene, keeping that safety argument obvious. | ||
| const esc = (s) => | ||
| String(s) |
Collaborator
There was a problem hiding this comment.
[Suggestion] esc() is duplicated between this file and web-shell-visuals-publish.mjs (lines 119–124). Both escape & < > " but neither escapes '. Not exploitable today (all interpolation sites use double-quoted attributes), but a future single-quoted attribute context would silently introduce an injection vector, and the fix would need to land in two places.
Consider extracting into a tiny shared module (e.g. .github/scripts/lib/html.mjs) and adding .replace(/'/g, ''') while at it.
— qwen3.7-max via Qwen Code /review
|
|
||
| writeFileSync( | ||
| join(outDir, 'manifest.json'), | ||
| JSON.stringify(manifest, null, 2), |
Collaborator
There was a problem hiding this comment.
[Suggestion] manifest.json is written here but immediately deleted by the workflow (rm -f "${OUT_DIR}/composite/manifest.json") and no downstream code reads it. The write-then-delete pattern suggests unclear intent.
Either consume it in buildComment (e.g. to show diff percentages in the PR comment — which would add real value), or skip the file write and log the metadata to stderr only (the summary loop below already does this).
— qwen3.7-max via Qwen Code /review
doudouOUC
previously approved these changes
Jul 16, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed at head (2efcbc85) after the latest push dismissed my earlier approval.
Verified against the current head, not just the diff:
- The "no visual change" path now genuinely posts — the round-2
HAS_IMAGESgate updates the marker comment with an image-less "no screenshot changes" note instead of exiting early, so my earlier concern about that note never rendering is resolved. - Correctness holds: diff is against the merge-base (not the base tip), a dimension mismatch short-circuits to changed,
img.onerrorsettles the load sopage.evaluatecan't hang to the job timeout, and the browser closes in afinally. - Fail-safe throughout: every base step is
continue-on-error; an unresolvable merge-base skips the before render (after-only) rather than falling back to the base tip; a compose failure leaves the raw after-shots. - The dependency-free helper tests are wired into full CI via the shared
HELPER_TESTSenv and pass (theRun .github/scripts helper testsstep is green).
Non-blocking, accepted trade-offs: the real GitHub Actions capture-job wiring (second checkout + base render) can only be exercised by a live run — as flagged in the description; it degrades safely. Deleted views never surface, and the compose-failure path keeps the "before/after" wording on head-only shots — both cosmetic.
The one failing check, Test (ubuntu-latest, Node 22.x), is an unrelated flaky test (github.test.ts › "should cancel while extracting a tar archive", 1 failed / 15722 passed); this PR touches only .github/, and its new helper-tests step is green. Recommend a re-run of that job before merge.
LGTM — approving. ✅
Collaborator
Author
|
@qwen-code /triage |
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. ✅
Collaborator
Author
|
@qwen-code /resolve |
Merge origin/main into the PR branch. The only conflict was in .github/workflows/ci.yml: the PR extracted the helper test list into a shared HELPER_TESTS env variable while main added serve-ab-diff.test.mjs to the hardcoded list. Resolution: keep the env-variable approach and append serve-ab-diff.test.mjs to HELPER_TESTS so both CI profiles run it.
Collaborator
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution SummaryPR: #6963 — ci(web-shell): before/after visual previews, showing only changed views Conflicted file
What each side did
Resolution
Files modified
No other files were touched. |
qwen-code-dev-bot
dismissed stale reviews from qwen-code-ci-bot and doudouOUC
via
July 16, 2026 01:13
b1c3d2d
Collaborator
🖼️ web-shell visual previewAuto-rendered from this PR head Screenshots · light / dark
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Collaborator
Author
|
@qwen-code /resolve |
Collaborator
|
Qwen Code did not run conflict resolution for this request. PR #6963 does not currently have merge conflicts with main. |
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. ✅
This was referenced Jul 16, 2026
Merged
This was referenced Jul 21, 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
6 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
Turns the web-shell visual-preview bot from a fixed set of canned screenshots into a before/after diff that shows only the views that changed. Each scenario is rendered against both the PR base (
main) and the PR head, the two are pixel-diffed, and a stitchedmain| this PR composite is posted for only the views that actually moved. A PR with no visual impact composites nothing and the comment says "no visual change".Why it's needed
The bot (#6880) posted the same canned screenshots on every PR, so it could never show what a PR actually changed — a feature like #6881 (mermaid zoom) or #6951 (split-pane maximize) never appeared — and it added noise on PRs whose UI impact was trivial. #6959 already stopped it firing on pure-backend PRs; this makes the remaining previews feature-aware with no per-PR understanding, because the diff finds exactly the surface the PR touched (and subsumes the backend-noise case at the content level).
Reviewer Test Plan
How to verify
node --test .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs→ 17/17 pass (pure helpersparseShot/isChanged/planWork, and the composite/"no visual change" comment builder).node .github/scripts/web-shell-visuals-compose.mjs <beforeDir> <afterDir> <outDir>. WithbeforeDir= mermaid scenario onmainandafterDir= the same scenario with feat(web-shell): add zoom, pan and drag controls to Mermaid diagrams #6881's changes overlaid, it reports the mermaid viewCHANGED (6.5%),skips the unchanged transcript, and writes one composite per changed view (stdout= composited count).npx eslint .github/scripts/web-shell-visuals-*.mjsandnpx prettier --checkare clean; the workflow YAML parses.Evidence (Before & After)
The composite the compositor produced from the run above (left =
main, right = the PR; the mermaid header gains− ↻ +):dark
Tested on
Environment (optional)
Node
node --test; local Playwright/Chromium render of the visuals suite. The GitHub Actions capture-job wiring (second checkout + base render) runs Linux-only in CI.Risk & Scope
continue-on-error, and compose degrades to after-only (every view tagged NEW) rather than sinking the preview.<view>-<theme>.pngnaming, so the publish workflow is unchanged.Linked Issues
Builds on #6880 and #6959. Gives #6881 / #6951 a real before/after surface. No closing keyword.
中文说明
这个 PR 做了什么
把 web-shell 视觉预览机器人从"固定罐头截图"改成只显示发生变化视图的 before/after diff。每个场景分别对着 PR base(
main)和 PR head 各渲染一遍,做像素 diff,只对真正变化的视图发一张拼接的main| this PR 对比图。无可视变化的 PR 什么都不拼,评论显示 "no visual change"。为什么需要
机器人(#6880)在每个 PR 上都发同一套罐头图,所以照不出 PR 到底改了什么 —— 像 #6881(mermaid 缩放)、#6951(分屏最大化)这样的功能一帧都看不到 —— 还在 UI 影响很小的 PR 上添噪音。#6959 已让它不再在纯后端 PR 上触发;本 PR 让其余预览无需理解 PR 就能感知功能,因为 diff 能精确找到 PR 动过的界面(并在内容层面吸收了后端噪音那种情况)。
Reviewer Test Plan
如何验证
node --test .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs→ 17/17 通过(纯函数parseShot/isChanged/planWork,以及拼接图 / "no visual change" 评论构建)。node .github/scripts/web-shell-visuals-compose.mjs <beforeDir> <afterDir> <outDir>。beforeDir=main上的 mermaid 场景,afterDir= 叠加了 feat(web-shell): add zoom, pan and drag controls to Mermaid diagrams #6881 改动的同一场景时,它报告 mermaid 视图CHANGED (6.5%)、skip未变的 transcript,并对每个变化视图写一张拼接图(stdout= 拼接数量)。npx eslint与npx prettier --check干净;workflow YAML 解析通过。证据(Before & After)
上面那次运行 compositor 产出的拼接图(左 =
main,右 = 本 PR;mermaid 表头多出− ↻ +):见上方英文正文中的图。测试平台
运行环境(可选)
Node
node --test;本地 Playwright/Chromium 渲染视觉套件。GitHub Actions 抓图 job(第二次 checkout + base 渲染)在 CI 里仅 Linux 运行。风险与范围
continue-on-error,compose 退化为 after-only(每个视图标 NEW),不拖垮预览。<view>-<theme>.png命名,发布 workflow 不变。关联 Issue
基于 #6880 与 #6959。给 #6881 / #6951 提供真正的 before/after 展示面。无关闭关键字。