ci(web-shell): denoise cross-job font-AA so visual previews stop false-flagging - #7210
Merged
Conversation
…e-flagging The before/after preview flagged text-heavy views (e.g. workspace-sidebar) as ~0.1-0.3% "changed" on PRs that do not touch them — #7204 is a live example (the two panels are pixel-for-pixel indistinguishable). Root cause: base and head render in SEPARATE CI jobs, so Linux font anti-aliasing is not bit-identical between them, and the naive per-pixel diff counts the scatter of isolated / 1px-wide glyph-edge pixels that leaves. At the tight 0.02% threshold that scatter crosses the line. Measure the changed fraction AFTER a cluster denoise: a differing pixel counts only when at least 4 of its 8 neighbours also differ. AA scatter (isolated = 0 neighbours, a 1px line = 2) erodes to ~zero, while a real change — a badge, chip, icon, panel — is a solid block whose interior keeps 5-8 and easily clears the threshold, so the threshold stays tight without raising it (which would miss small real changes like a workspace badge). The browser now returns a compact bit-mask; the denoise + count run in node against the unit-tested countDenoisedChanges, so there is one tested implementation of the metric.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Observed bug with clear evidence. The web-shell visual-preview engine keeps false-flagging text-heavy views (e.g. Direction: Aligned — this is a targeted fix for a known CI-tooling false-positive class. The cluster-denoise approach exploits the real distinguishing feature (AA noise is spatially scattered; real changes are solid blocks), which is exactly the right discriminator when a flat threshold can't separate them. Size: Not applicable — CI scripts only ( Approach: Scope feels right and minimal. Two focused additions — a pure Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有清晰证据。web-shell 视觉预览引擎持续将文字密集的视图误标为"变更"（如 #7204 上的 方向：对齐——这是对已知 CI 工具假阳性类别的定向修复。聚类去噪利用了真正的区分特征（AA 噪声在空间上分散，真实改动是实心块），这在扁平阈值无法区分二者时正是正确的判别器。 规模：不适用——仅 CI 脚本（ 方案：范围合理且精简。两个聚焦的新增——一个纯函数 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal (before reading diff): The false-positive comes from counting per-pixel differences that are spatially isolated (AA noise) vs spatially clustered (real change). I would have added a morphological operation — erosion or connected-component filtering — to discard isolated differing pixels before computing the changed fraction. The key design choice is how to separate "isolated" from "clustered": neighbor counting on a binary mask is the simplest correct approach. Comparison with diff: The PR's approach matches this exactly. Correctness: No bugs found. The erosion correctly handles edge pixels (fewer possible neighbours → more aggressive erosion, which is correct for glyph-edge AA noise). The Tests: All 13 pass. The test suite pins exactly the shapes that matter:
No critical blockers, no AGENTS.md violations. Code is focused, well-commented, and follows project conventions. 中文说明代码审查独立方案（读 diff 前）：假阳性来自把空间上孤立的差异像素（AA 噪声）和空间上聚集的差异像素（真实改动）一视同仁地计数。我会加一个形态学操作——腐蚀或连通分量过滤——在算变更比例前丢弃孤立的差异像素。关键设计选择是如何区分"孤立"和"聚集"：在二值掩码上做邻域计数是最简正确方案。 与 diff 对比：PR 的方案完全吻合。 正确性：未发现 bug。腐蚀正确处理边缘像素（可用邻域更少→腐蚀更激进，对字形边缘 AA 噪声而言是正确的）。 测试：全部 13 个通过。测试套件精确钉住了关键形状：孤立散点→0；1px 线→0； 无关键阻断，无 AGENTS.md 违规。代码聚焦，注释到位，遵循项目约定。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 This is a clean, well-scoped CI-tooling fix. The problem is real and documented (false-positive composites from cross-job font AA), the solution is minimal (one pure function + bit-mask transfer), and the tests pin exactly the shapes that matter — including the decisive 1280×800 separation that proves AA scatter erodes below the 0.02% threshold while a real badge-sized change stays above it. No product code is touched, no public contract changes, and the risk/tradeoff (sub-2px features erode away) is acceptable for a visual preview tool. Approving. ✅ 中文说明信心度：5/5 这是一个干净、范围精准的 CI 工具修复。问题是真实且有记录的（跨 job 字体 AA 导致的假阳性合成图），方案精简（一个纯函数 + 位掩码传输），测试精确钉住关键形状——包括 1280×800 下的决定性分离，证明 AA 散点去噪后低于 0.02% 阈值而真实徽章大小的改动仍在阈值之上。未触及产品代码，公开契约不变，风险/取舍（小于约 2px 的特征被侵蚀）对视觉预览工具而言可接受。 批准 ✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 19, 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
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after✅ No screenshot changes against the PR base. Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
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
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. |
yiliang114
approved these changes
Jul 19, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. CI-tooling only, and the public metric contract is unchanged — diffPct still returns 0–100, isChanged/manifest untouched, and the size-mismatch / undecodable → 100% short-circuits are preserved.
I ran the two new pure functions standalone to confirm the metric independently of the bundled tests: isolated scatter and a 1px line erode to 0, a solid block survives, and at 1280×800 the ~0.25% AA scatter denoises below the 0.02% threshold while the 55×18 badge stays above it (0.096% after denoise) — the separation this change is after. The LSB-first bit-mask round-trips, and the String.fromCharCode/btoa path stays within Latin-1 for every byte value.
No blockers.
— reviewed via Qwen Code /review
github-actions
Bot
added
the
skip-changelog-auto
Collaborator
|
Released in v0.20.1. |
Collaborator
|
Agent run timed out after 1800000ms ❌ failed |
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 before/after visual-preview diff cluster-denoise the differing-pixel mask before measuring the changed fraction: a differing pixel counts only when at least
MIN_CLUSTER_NEIGHBORS(4) of its 8 neighbours also differ. This erodes cross-render font anti-aliasing scatter while leaving real changes untouched. The browser now returns a compact bit-mask and the denoise + count run in node against a unit-tested pure function, so the metric has one tested implementation.Why it's needed
The preview keeps flagging text-heavy views — most recently
workspace-sidebaron #7204 — as changed on PRs that don't touch them. The two panels are pixel-for-pixel indistinguishable, yet the header reads "0.2% changed":Root cause: base and head render in separate CI jobs, so Linux font anti-aliasing (freetype/fontconfig) is not bit-identical between the two runs. A text-heavy view leaves a scatter of isolated / 1px-wide differing pixels along glyph edges — ~0.1–0.3% of the frame — which the naive per-pixel diff counts. At the deliberately tight 0.02% threshold, that scatter crosses the line. (The earlier
sidebar-attentionspinner flake was a different, animation class, fixed in #7041 by freezing looping animations — that fix is intact; this is the residual sub-pixel text class it was never meant to cover.)Raising the flat threshold isn't viable: the change this scenario exists to catch — the primary-workspace "Primary" badge — is itself only ~0.08%, the same order as the AA scatter. The distinguishing feature is spatial clustering: AA noise is scattered single pixels / thin lines; a real change is a solid block. The denoise keys off exactly that.
Reviewer Test Plan
How to verify
The new unit tests pin the metric on the exact shapes involved:
minNeighborserosion strength on a solid 3×3 (8/4/3 → 1/5/9 survivors)unpackBitMaskround-trips a packed maskEnd-to-end through the real browser→node pipeline (identical vs a genuine change):
An identical pair reports
0% diffand emits no composite; a genuinely different pair (e.g. a light-theme image in the dark slot) reports99.89% diffand emits the composite. Verified locally.Evidence (Before & After)
Before: the #7204 composite above — "0.2% changed" with no visible difference. After: that scatter denoises to ~0, so the view is no longer flagged; a real block-shaped change still is (unit + end-to-end proof above). The CI-only AA noise itself can't be reproduced on macOS (local same-commit renders are already 0px), so the fix is validated by the unit tests on the exact noise/real-change shapes plus the end-to-end pipeline run, not a live CI screenshot.
Tested on
Node
--test+ the compose CLI, macOS. The behaviour it fixes is Linux-CI-only.Environment (optional)
node --test(no build); the CLI lazy-imports@playwright/testchromium.Risk & Scope
.github/scripts/), no product code. The public metric contract is unchanged —diffPctstill returns a 0–100 number,isChangedand the manifest are untouched, and the size-mismatch / undecodable → 100% short-circuits are preserved.MIN_CLUSTER_NEIGHBORSagainst real CI captures (4 is derived from the 1px-line = 2-neighbour geometry at the 1280×800 DPR-1 capture).Linked Issues
None. Hardens the visual-preview engine (#6963); complements the animation-freeze fix (#7041). Motivated by the false positive on #7204.
中文说明
这个 PR 做了什么
让 web-shell before/after 视觉预览在统计"变更像素比例"之前先做聚类去噪:一个差异像素只有在其 8 邻域中至少有
MIN_CLUSTER_NEIGHBORS(4)个也差异时才计数。这会侵蚀掉跨渲染的字体抗锯齿散点,同时不动真实改动。浏览器现在返回一个紧凑位掩码,去噪+计数在 node 里用一个有单测的纯函数完成,度量只有一份被测实现。为什么需要它
预览一直把文字密集的视图——最近是 #7204 的
workspace-sidebar——在没碰它们的 PR 上标为"变更"。两半逐像素无法区分,标题却写着"0.2% changed"(见上图)。根因:base 和 head 在两个独立的 CI job 里渲染,Linux 字体抗锯齿(freetype/fontconfig)在两次运行间并非逐位一致。文字密集的视图会在字形边缘留下一片孤立/1px 宽的差异像素散点(约占画面 0.1–0.3%),朴素逐像素 diff 会把它们全算上。在刻意设很紧的 0.02% 阈值下,这片散点就越线了。(此前
sidebar-attention的转圈是另一类——动画,已在 #7041 用冻结循环动画修复,那个修复仍在生效;本次是它未覆盖的亚像素文字残留类。)抬高扁平阈值行不通:这个场景要抓的改动——主工作区 "Primary" 徽章——本身也才约 0.08%,和 AA 散点同一量级。区分特征是空间聚集性:AA 噪声是散点/细线,真实改动是实心块。去噪正是抓这一点。
复现测试计划
如何验证
新单测在涉及的确切形状上钉住度量:孤立散点(跨 job AA)→ 侵蚀为 0;1px 宽线(字形边缘,2 邻域)→ 侵蚀为 0;
minNeighbors在实心 3×3 上的侵蚀强度(8/4/3 → 1/5/9);徽章形实心块大体存活;1280×800 下:约 0.25% 的 AA 散点去噪后低于 0.02% 阈值,而 55×18 徽章块仍在阈值之上(决定性分离);unpackBitMask位掩码往返。端到端走真实"浏览器→node"管线:相同图对报
0% diff、不产出合成图;真正不同的图对(如把浅色图放进深色槽)报99.89% diff、产出合成图。已本地验证。证据(Before & After)
Before:上方 #7204 合成图——"0.2% changed" 却无可见差异。After:该散点去噪为约 0,不再被标记;而实心块状真实改动照旧被标记(上方单测+端到端证明)。CI-only 的 AA 噪声在 macOS 上复现不出(本地同 commit 渲染本就 0px),因此本修复靠"在确切噪声/真实改动形状上的单测 + 端到端管线运行"验证,而非一张实时 CI 截图。
测试平台
Node
--test+ compose CLI,macOS。它修复的行为是 Linux-CI-only。风险与范围
.github/scripts/),无产品代码。公开度量契约不变——diffPct仍返回 0–100 数值,isChanged/manifest 不动,尺寸不符/无法解码→100% 的短路保留。MIN_CLUSTER_NEIGHBORS(4 是从 1280×800 DPR-1 下"1px 线=2 邻域"的几何推出的)。关联 Issue
无。加固视觉预览引擎(#6963);与动画冻结修复(#7041)互补。由 #7204 的假阳性触发。