Skip to content

ci(visual): R2 Phase B - screenshot tests on the pinned rendering stack + report-only PR gate - #413

Merged
pftg merged 3 commits into
masterfrom
claude/devx-r2-phase-b
Jul 31, 2026
Merged

ci(visual): R2 Phase B - screenshot tests on the pinned rendering stack + report-only PR gate#413
pftg merged 3 commits into
masterfrom
claude/devx-r2-phase-b

Conversation

@pftg

@pftg pftg commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

Second PR of DevX Round 2: brings visual regression back to CI — the thing PR #386 had to remove — now that the rendering stack is pinned and glibc everywhere.

Why it's viable now

The old blocker was Alpine/musl-recorded baselines vs Ubuntu/glibc runners (3–28% text-rendering divergence). Since Phase 4, the canonical stack is Debian/glibc with pinned Chrome for Testing (.dev/cft-version), a deterministic fontconfig (.dev/fonts.conf), and pinned fonts — and bin/setup-test-env installs that exact stack on any glibc host, including GitHub's runners. A CI job provisioned that way renders the same pixels as the linux/ baselines.

What changed in .github/workflows/test.yml

  • Both modes now run on the pinned stack: bin/setup-test-env + eval --print-env exports CHROME_BIN/CHROMEDRIVER_PATH; the CfT download is cached keyed on .dev/cft-version. Previously the workflow used whatever Chrome ubuntu-latest shipped that week — compare runs diffed unpinned pixels against pinned baselines, and record mode would have silently overwritten the canonical linux/ baselines with unpinned renders.
  • New pull_request trigger, paths-filtered to visual surfaces (themes/**, config/**, postcss.config.js, test/system/**, fixtures, the stack pins) — runs rake test:critical as a report-only gate (continue-on-error: true): failures upload the snap_diff report and comment on the PR without blocking the merge. After a clean soak week, flipping that one flag makes it blocking.
  • Record mode is now workflow_dispatch-only and sets ALLOW_DIRTY_SCREENSHOTS=1 explicitly (the one place a dirty fixtures tree is intended, given the Phase 3 guard).
  • Floating snap-diff/...@master composite refs pinned to the SHA Gemfile.lock already locks.

Follow-through after merge

Dispatch Screenshot Tests with update-baselines: true once — that re-records the linux/ baselines on the pinned stack in CI, which also closes Round 1's pending re-baseline without needing a Docker-capable machine.

.okf/build/ci-gates.md updated to record the decision reversal + soak plan.

Verification

  • YAML validated; job conditions traced per event: PR → report-only compare; dispatch compare → full rake test; dispatch record → record + commit, never on PRs.
  • bin/setup-test-env handles non-root fontconfig (~/.config/fontconfig) and GitHub runners have passwordless sudo for the font packages; the CfT bucket (storage.googleapis.com) is reachable from runners.
  • This PR itself touches no visual surface, so the new PR trigger won't fire here; the first real exercise is the post-merge baseline dispatch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH


Generated by Claude Code

claude added 3 commits July 31, 2026 12:04
…eport-only gate

test.yml previously used whatever Chrome ubuntu-latest ships - compare
runs diffed unpinned pixels against pinned baselines, and record mode
would silently overwrite the canonical linux/ baselines with unpinned
renders. Now both modes install the canonical stack via
bin/setup-test-env (Chrome for Testing from .dev/cft-version, fonts,
.dev/fonts.conf) and export CHROME_BIN/CHROMEDRIVER_PATH; the CfT
download is cached keyed on .dev/cft-version.

New pull_request trigger (paths-filtered to visual surfaces) runs
test:critical as a REPORT-ONLY gate (continue-on-error) - failures
upload the snap_diff report and comment on the PR without blocking.
Flip continue-on-error off after the soak period to make it blocking.

Record mode sets ALLOW_DIRTY_SCREENSHOTS=1 explicitly (the one place a
dirty tree is intended) and is now guarded to workflow_dispatch only.
Floating snap-diff @master composite refs pinned to the same SHA the
Gemfile.lock already locks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@pftg, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 32 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 47f51f5b-3291-4abe-91db-8405cffec5fa

📥 Commits

Reviewing files that changed from the base of the PR and between fd57bef and d5df5da.

📒 Files selected for processing (3)
  • .github/workflows/test.yml
  • .okf/build/ci-gates.md
  • .okf/log.md

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.

@pftg
pftg merged commit ede4d23 into master Jul 31, 2026
3 checks passed
@pftg
pftg deleted the claude/devx-r2-phase-b branch July 31, 2026 12:15
pftg pushed a commit that referenced this pull request Jul 31, 2026
bin/README.md listed four scripts that do not exist (debug,
benchmark-dev, bunx, generate-cover-image) and omitted eleven that do
(qtest, dtest-all, setup-test-env, build-if-stale, hugo-build, ...).
Rewritten to match the actual directory contents.

.okf test-gates: drop the stale 'CI does NOT run screenshot diffs'
claim (superseded by #413/#417), record the shared build-if-stale
warm-tree flow, qtest untracked-file detection, and the .gitignore
dot-path gotcha that had kept the pre-push hook out of the repo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH
pftg added a commit that referenced this pull request Jul 31, 2026
* fix(devx): R3-1 - one truthful build path for every test runner

All four runners (test/qtest/dtest/dtest-all) now build through a shared
bin/build-if-stale helper that routes via bin/hugo-build (PurgeCSS
cold-start warm-up guard) and skips the rebuild on a warm tree:

- bin/test: DEST honors a caller-preset HUGO_DEFAULT_PATH and the build
  is skipped entirely under PRECOMPILED_ASSETS - un-breaks bin/dtest,
  whose container has no hugo and points at _dest/public-dtest
- bin/qtest: shares bin/test's _dest/public-test tree (warm reruns skip
  the ~26s rebuild it previously paid on every micro-commit), and
  --changed now counts untracked files (a brand-new pages/foo.css no
  longer exits 0 green with 'nothing to run')
- staleness list gains assets/ static/ package.json bun.lockb
- .githooks/pre-push: run each unit-test file in its own ruby
  invocation (ruby file1 file2 executes only file1) - and commit the
  hook itself: the .* gitignore rule had kept it out of the repo, so
  fresh clones got a hooksPath pointing at nothing (added !.githooks)
- bin_scripts_test: 3 new guards - no bare 'hugo --environment
  production' outside hugo-build/build-if-stale, pre-push multi-file
  ruby anti-pattern, bin/test HUGO_DEFAULT_PATH contract

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH

* docs(devx): R3-1 - truthful bin/README, test-gates concept + log entry

bin/README.md listed four scripts that do not exist (debug,
benchmark-dev, bunx, generate-cover-image) and omitted eleven that do
(qtest, dtest-all, setup-test-env, build-if-stale, hugo-build, ...).
Rewritten to match the actual directory contents.

.okf test-gates: drop the stale 'CI does NOT run screenshot diffs'
claim (superseded by #413/#417), record the shared build-if-stale
warm-tree flow, qtest untracked-file detection, and the .gitignore
dot-path gotcha that had kept the pre-push hook out of the repo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH

* fix(devx): address review - rake guards entrypoint + deletion staleness

- New `rake test:guards` task (toolchain pins + bin scripts, ~2s) is the
  pre-push hook's single test entrypoint - repo-provided command instead
  of direct ruby file invocations, and structurally immune to the
  multi-file-ruby trap
- bin/build-if-stale staleness probe now includes directory mtimes:
  deleting a source file leaves nothing to be -newer, but bumps its
  parent dir - verified warm tree goes stale on a removed content file
- pre-push guard test asserts the rake entrypoint is present (no longer
  passes vacuously if the hook drops its test invocation)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH

---------

Co-authored-by: Claude <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.

2 participants