Skip to content

feat(storybook): add visual regression testing for stories#3392

Open
pauldambra wants to merge 8 commits into
mainfrom
posthog-code/storybook-visual-regression
Open

feat(storybook): add visual regression testing for stories#3392
pauldambra wants to merge 8 commits into
mainfrom
posthog-code/storybook-visual-regression

Conversation

@pauldambra

Copy link
Copy Markdown
Member

What

Ports posthog/posthog's Storybook visual regression approach to this repo. Every story is snapshot-tested in Chromium in both dark and light themes using @storybook/test-runner + jest-image-snapshot (SSIM comparison, 1% failure threshold).

How it works

  • Harness (apps/code/.storybook/test-runner.ts): waits for fonts, quill spinners/skeletons, image loads, and DOM stability; disables animations; flips the theme global per snapshot so one render pass produces <story-id>--dark.png and --light.png. Component snapshots clip to the union of #storybook-root and any Radix-portaled dialogs/popovers so portaled content isn't missed. Per-story knobs via parameters.testOptions (viewport, waitForSelector, themes, snapshotTargetSelector) and tags: ["test-skip"].
  • Determinism: under the test runner the preview freezes the clock with mockdate and re-seeds Math.random before every story render (including theme flips and retries). This was needed because GeneratingIndicator renders a random thinking verb and a live elapsed timer. Two consecutive full verify passes ran clean locally (358/358 snapshots).
  • CI (.github/workflows/code-storybook.yml): builds workspace packages + static Storybook, serves it, regenerates all snapshots, and if anything changed auto-commits the PNGs back to the PR branch via GraphQL createCommitOnBranch (scripts/commit-storybook-snapshots.mjs) using the array-releaser app token — API commits are GitHub-signed, satisfying the signed-commit ruleset, and the app token retriggers checks on the new head. The retriggered run regenerates identical snapshots, finds no diff, and stops (no loop). Fork PRs can't receive auto-commits, so they fail with instructions instead. Failure/diff screenshots upload as artifacts; junit results go to Trunk.
  • Baselines are Linux-rendered and CI-owned — this PR ships an empty __snapshots__/ dir and the first CI run should populate all ~358 baselines via auto-commit (which doubles as an end-to-end test of the pipeline).

Bug fix surfaced by this work

8 of the 23 story suites could not load in Storybook at all: the @posthog/agent dist bundles are Node-targeted (tsup createRequire banner, fs/path imports), which threw at module load in the browser and killed every story sharing the chunk. The Storybook Vite config now shims node:module/node:fs and resolves node:path to pathe (Storybook-only; the app build is untouched).

Local usage

pnpm --filter code build-storybook
cd apps/code && pnpm exec http-server storybook-static --port 6006 --silent &
pnpm --filter code test:visual          # compare against baselines
pnpm --filter code test:visual:update   # regenerate (don't commit macOS renders)

Docs added to docs/testing.md.


Created with PostHog Code

Port posthog/posthog's Storybook test-runner approach: every story is
snapshot-tested in Chromium in dark and light themes via
@storybook/test-runner + jest-image-snapshot (ssim, 1% threshold).
Baselines are Linux-rendered, live in apps/code/.storybook/__snapshots__,
and are auto-committed to the PR branch by CI through the GitHub API so
they satisfy the signed-commit ruleset and show as reviewable PNG diffs.

The preview freezes time (mockdate) and seeds Math.random per story
render under the test runner so elapsed timers and randomized status
verbs render identically on every pass.

Also fixes 8 story suites that could not load in Storybook at all: the
@posthog/agent dist bundles are Node-targeted (createRequire banner,
fs/path imports), so the browser build now shims node:module and
node:fs and resolves node:path to pathe.

Generated-By: PostHog Code
Task-Id: c8fa7392-2afe-4919-b8be-e6e3b56e1419
@trunk-io

trunk-io Bot commented Jul 13, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3cf7b509cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/code-storybook.yml

@pauldambra pauldambra left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note

🤖 Automated comment by QA Swarm — not written by a human

QA Swarm review complete. See inline comments.

Comment thread .github/workflows/code-storybook.yml
Comment thread .github/workflows/code-storybook.yml Outdated
Comment thread .github/workflows/code-storybook.yml Outdated
Comment thread .github/workflows/code-storybook.yml Outdated
Comment thread .github/workflows/code-storybook.yml
Comment thread docs/testing.md Outdated
Comment thread apps/code/.storybook/test-runner.ts Outdated
Comment thread apps/code/.storybook/test-runner.ts Outdated
Comment thread apps/code/.storybook/test-runner.ts Outdated
Comment thread apps/code/.storybook/preview.tsx Outdated
@pauldambra

Copy link
Copy Markdown
Member Author

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: qa-team (specialists + generalists), paul-reviewer, xp-reviewer, security-audit

Verdict: ⚠️ REQUEST CHANGES (round 1 @ 3cf7b50)

The harness and determinism work are solid, but the workflow ships with a verified fail-open bug: the paths-filter never matches, so the visual-regression job silently skips on every PR — the feature is inert as merged.

Key findings

  • 🟠 HIGHpredicate-quantifier: every + four disjoint positive globs means the change filter can never match; visual-regression always skips (green-but-skipped required check). All four reviewers flagged this independently; verified against dorny/paths-filter source.
  • 🟡 MEDIUM — bare wait after backgrounded package builds swallows build failures.
  • 🟡 MEDIUM — auto-commit loop has no circuit breaker; the in-file comment's "snapshots-only diff skips the run" rationale is wrong (paths-filter diffs the whole PR, not the push). Real guard is render determinism only.
  • 🟡 MEDIUM — baselines are append-only: orphaned PNGs from renamed/removed stories are never pruned; the commit script's deletions path is dead code.
  • 🟢 LOW — app token minted unscoped and handed to PR-controlled code (prefer actions/create-github-app-token with repo/permission narrowing); porcelain parsing fragile for exotic paths; stale-head race dumps raw GraphQL error; no timeout-minutes; docs' cleanup command misses untracked PNGs; 500ms magic sleep per theme.

Convergence

  • paths-filter every bug: all four reviewers (highest confidence).
  • porcelain/rename parsing fragility: qa-team + paul + xp.
  • auto-commit loop risk: qa-team + security-audit.
  • token scoping: qa-team + security-audit.
  • 500ms sleep and navigator?. nit: paul + xp.

Reviewer summaries

Reviewer Assessment
🔍 qa-team Careful port, but a verified showstopper in change detection plus three mediums (wait-swallow, loop guard, no pruning).
👤 paul Really nice work with why-comments throughout; wants the filter question nailed down before merge — a check that goes green by skipping is the worst kind.
📐 xp Clean infra, comments earn their place; the every filter makes the guard protect nothing while showing green (Rule 1: does it actually work?).
🛡 security-audit Fundamentals sound (fork gating, no injection, pathspec-constrained writes, SHA pins); fail-open filter is the main defect, plus token least-privilege hardening.

Automated by QA Swarm — not a human review

Fixes the paths-filter predicate-quantifier bug that made the workflow
never trigger, makes background build failures fail loudly instead of
being swallowed by a bare `wait`, adds a real commit-loop circuit
breaker, swaps to actions/create-github-app-token with narrowed
permissions, prunes obsolete snapshot baselines via jest-image-snapshot's
outdated-snapshot reporter, hardens the snapshot-commit script's git
status parsing and stale-branch handling, and tightens several small
issues in the test runner and preview decorator.

Generated-By: PostHog Code
Task-Id: c8fa7392-2afe-4919-b8be-e6e3b56e1419
Extract wait_all helper in the workflow build step, compute the
auto-commit/fork conditions once, and tie the post-theme-flip wait to an
actual paint (plus a short settle) instead of a flat sleep alone.

Generated-By: PostHog Code
Task-Id: c8fa7392-2afe-4919-b8be-e6e3b56e1419
…k-visual-regression

# Conflicts:
#	pnpm-lock.yaml
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 001fc69.

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