Skip to content

ci(gallery): diff each PR's touched gallery states against its merge-base - #18205

Merged
teamleaderleo merged 4 commits into
feat-cmux-nextfrom
ci/gallery-pr-diff
Oct 7, 2026
Merged

teamleaderleo merged 4 commits into
feat-cmux-nextfrom
ci/gallery-pr-diff

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Each PR that touches the webviews now gets a gallery diff: which gallery states it changes, shown against its merge-base. This is the first part of the per-PR gallery (cx-4oa.1): the base diff and the diff page.

  • Which entries. webviews/scripts/gallery/touched.ts picks the entries a change reaches:

    • an entry's own *.gallery.ts;
    • anything its covers import, followed through relative imports;
    • every entry of a host whose stylesheet changed, because CSS is global to its page;
    • every entry, when the gallery itself, its build config or the lockfile changes.
  • Renders. .github/workflows/gallery-pr.yml renders those entries x variants x both default themes x Chromium and WebKit with scripts/gallery-matrix/runner.ts, on Linux. It renders three times: once at the merge-base, using the base's own gallery code, and twice at the head.

  • Comparison (compare.ts). Each state ends up as one of:

    • changed, with the changed regions boxed;
    • new or removed;
    • broken: the head stage did not mount;
    • nondeterministic: the head differs from a second render of itself. That points to a clock or fixture leak, so it's listed apart and never counted as a PR change.
  • Report (report.ts, pr.ts):

    • A diff page lists changed states first. Each has a highlight overlay, a before/after slider and an onion skin. Unchanged states are folded.
    • One sticky comment says, for example, "7 states changed: agent-pane.composer/streaming, ...". It shows before/after thumbnails of the changed regions.
    • summary.json holds the same summary for the team feed's PR card.
  • Security. The render job runs the PR's code with a read-only token and no secrets. The publish job runs only the base branch's scripts:

    • it rebuilds the comment from outcomes.json;
    • only plain file names become thumbnail URLs;
    • only PNG files are uploaded to pr-media.

    Same-repository PRs only.

Not in this PR:

  • Freestyle VMs. CI has no Freestyle key yet, so this renders on the Linux runner itself.
  • Publishing the diff page and gallery to the gallery host. Until then they're in the run's gallery-pr artifact, and the comment links to it.
  • Feed card integration, playbook filmstrips (needs hq-5c's play-step API), and the layout-shift and long-frame numbers.

Testing

  • scripts/gallery-matrix/compare.test.ts covers:
    • box merging;
    • size changes;
    • the thumbnail layout;
    • every status from synthetic runs (changed, new, removed, broken, nondeterministic, unchanged);
    • the summary line, the comment, and safe embedding in the page.
  • webviews/test/gallery-touched.test.ts covers:
    • import following through stylesheets, helpers and dynamic imports;
    • the host-stylesheet rule;
    • the everything rule;
    • the real composer entry being touched by the pane stylesheet.
  • None of this has run yet: bun is not allowed on the lane's host. This PR's own gallery PR diff run executes the matrix tests, and that run's results will go in a comment. webviews/test runs in cmux-next web bundles (signal only).
  • The publish job skips on this PR, because the base branch has no publisher yet. It first runs on the next PR after this lands.

Changelog

none

Proof

CI only. This PR's own run renders every entry, because it changes webviews/scripts/gallery/. Its step summary will show the comment.

Checklist

  • Behavior changes have added or updated tests, or Testing says why not

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds CI that renders and diffs gallery states on every PR to feat-cmux-next that touches webviews. Each PR now gets a diff page and a sticky comment comparing the gallery states it changes against the merge-base.

  • touched.ts picks the entries a change reaches: an entry's own *.gallery.ts, anything its covers imports, every entry of a host whose stylesheet changed, and every entry when the gallery, build config, or lockfile changes.
  • The matrix renders touched entries × variants × both default themes × Chromium and WebKit on Linux at the merge-base and twice at the head. Each state ends up changed (regions boxed), new, removed, broken, or nondeterministic; a state whose second head render differs from the first is nondeterministic, never a PR change.
  • The diff page lists changed states first with highlight, slider, and onion-skin views; the sticky comment summarizes e.g. "3 states changed: ..." with before/after thumbnails; summary.json feeds the PR card.
  • The render job runs the PR's code with a read-only token and no secrets, and executes the matrix and touched-entry tests. The publish job runs only the base branch's scripts, rebuilding the comment from outcomes.json; only plain-file PNG thumbnails reach pr-media, and only same-repository PRs are supported.

Not in this PR: Freestyle VMs (CI has no Freestyle key), publishing the gallery and diff page to the gallery host (until then they live in the gallery-pr artifact, which the comment links to), feed card integration, and the play-step, filmstrip, and layout-shift numbers.

Written for commit 09787bb. Summary will update on new commits.

Review in cubic Turn on auto-fix


Note

Medium Risk
New CI runs untrusted PR code in render (mitigated by read-only token and isolated publish), writes to pr-media and PR comments with contents-write, and pixel diffs can be noisy or miss logic-only changes.

Overview
Adds per-PR gallery visual regression on feat-cmux-next when webviews/** (and related paths) change: CI picks affected entries, renders the matrix at merge-base vs head (plus a second head render for flake detection), pixel-compares screenshots, and posts a sticky PR comment with before/after thumbnails.

Entry selection (webviews/scripts/gallery/touched.ts): maps changed files to gallery entries via import closure from *.gallery.ts / covers, host-global stylesheets, or a full-matrix rerun when gallery/build/lockfile changes.

Diff pipeline (scripts/gallery-matrix/compare.ts, pr.ts, report.ts): classifies each state as changed (boxed regions + side-by-side thumbs), new, removed, broken mount, nondeterministic (head ≠ repeat head), or unchanged; emits index.html, comment.md, summary.json, and artifacts.

Workflow split (.github/workflows/gallery-pr.yml): render runs PR code read-only (Playwright on Linux); publish uses only base-branch scripts to rebuild the comment from outcomes.json, upload PNG thumbs to pr-media, and update the bot comment (same-repo PRs only).

Reviewed by Cursor Bugbot for commit 09787bb. Bugbot is set up for automated code reviews on this repo. Configure here.

teamleaderleo and others added 2 commits October 6, 2026 20:24
An entry is touched by its own file, by anything its covers import, and by a
changed stylesheet of its host's page. Gallery and build-config changes touch
every entry. The per-PR gallery renders only these.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…base

Renders entry x variant x theme at the merge-base and at the head (twice, so a
state that differs from itself is reported as nondeterministic and never as a
change), boxes the changed regions, and writes a diff page with highlight,
slider and onion-skin views, a sticky comment with before/after thumbnails and
a summary for the feed. The render job runs the PR's code without secrets; the
publish job runs only the base branch's scripts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 42d5d1fd-b0e1-49d5-b504-c04031265f64

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Passes: CI passes on 09787bbf1c.

CI passes on 09787bbf1c (run 37567935078 attempt 1).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 4 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d7795dc. Configure here.

# thumbs.txt names plain file names (report.ts SAFE_KEY); take only PNGs from the render.
src="$RUNNER_TEMP/render/diff/thumbs/$key"
if [ -f "$src" ] && [ "$(head -c 8 "$src" | od -An -tx1 | tr -d ' \n')" = "89504e470d0a1a0a" ]; then
cp "$src" "$upload/$prefix$key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thumbnail URLs miss sanitized filenames

High Severity

Comment thumbnail URLs use the raw outcome key, but pr-media.py always runs sanitize on the uploaded name and collapses -- to -. Manifest case ids join parts with --, so every uploaded PNG lands under a different name than the comment's img src and the sticky before/after images 404.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d7795dc. Configure here.

set -euo pipefail
sha=$(git merge-base "origin/$BASE_REF" HEAD)
echo "sha=$sha" >> "$GITHUB_OUTPUT"
git diff --name-only "$sha" HEAD > "$RUNNER_TEMP/changed.txt"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkout fetches every repository branch

Medium Severity

The render job checks out the head SHA with fetch-depth: 0 so git merge-base origin/$BASE_REF HEAD can run. On this repo that fetch pulls every branch (thousands) and has already been measured at several minutes, which competes with two gallery builds and three Playwright passes inside a 30-minute timeout.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d7795dc. Configure here.

function highlight(o) {
const frame = el("div", {class: "frame"}, img(o.head, "head"));
for (const b of o.boxes) frame.append(el("div", {class: "box", style: "left:" + pct(b.x, o.width) + ";top:" + pct(b.y, o.height) + ";width:" + pct(b.w, o.width) + ";height:" + pct(b.h, o.height)}));
return frame;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlight overlay ignores screenshot size

Low Severity

Changed-region boxes live in max(base, head) coordinates (diff.width / diff.height), but the highlight view positions them as percentages of that max size over the head PNG. When the head shot is smaller than the base, the boxes do not line up with the pixels they describe.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d7795dc. Configure here.

/^webviews\/package\.json$/,
/^bun\.lock$/,
/^scripts\/gallery-matrix\//,
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webviews lockfile skips full re-render

Low Severity

EVERYTHING treats root bun.lock and webviews/package.json as gallery-wide, but the render job installs from webviews/ and uses webviews/bun.lock. A lockfile-only dependency bump there marks no entries, so the workflow runs and then skips the diff.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d7795dc. Configure here.

teamleaderleo and others added 2 commits October 6, 2026 20:37
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo merged commit f227446 into feat-cmux-next Oct 7, 2026
53 of 54 checks passed
@teamleaderleo
teamleaderleo deleted the ci/gallery-pr-diff branch October 7, 2026 03:53
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/37569422881 at 552e7cb failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at 86019f6. This pull request is one of 4 merged since: #18105, #18198, #18165, #18205.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/37569270312 at f60b3d7 failed: cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next generated files.
Those jobs last passed at 86019f6. This pull request is one of 4 merged since: #18105, #18198, #18165, #18205.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/37569941713 at 46215de failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at 86019f6. This pull request is one of 4 merged since: #18105, #18198, #18165, #18205.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/37569551101 at 5976099 failed: cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next generated files.
Those jobs last passed at 86019f6. This pull request is one of 4 merged since: #18105, #18198, #18165, #18205.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/37570246034 at e8a5696 failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at 86019f6. This pull request is one of 5 merged since: #18105, #18203, #18198, #18165, #18205.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

teamleaderleo added a commit that referenced this pull request Oct 7, 2026
…18336)

* ci(gallery): publish on the trusted runner selector, not a bare GitHub-hosted label

#18205 gave gallery-pr.yml's publish job `runs-on: ubuntu-24.04` with a
`github-hosted-required` comment, which tests/test_ci_self_hosted_guard.sh no
longer accepts as an exemption. The job runs trusted base-branch scripts with
a write token, so it takes the CI_TRUSTED_RUNNER selector like the other
trusted write jobs: an ephemeral Blacksmith VM by default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci(cmux-next): run the companion workflows' guard tests in the checks job

tests/test_ci_workflow_guards_are_wired.py fails on feat-cmux-next because no
workflow runs three tests that read a workflow:
- test_cmux_next_generated_catch_up.py (#17658)
- test_cmux_next_regenerate_bundles_workflow.py (#18154)
- test_next_batch.py (#17535)
The checks job now runs them as one step, and its path filters include the
tests and the workflows they guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant