From d3919713bc9e7ec85389ba0096df6dd95f92ae9f Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 8 Jul 2026 07:23:02 -0500 Subject: [PATCH] [codex] Document Storybook visual baseline platform lock (#9216) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Storybook visual baselines protect UI surfaces from unintended visual drift. > - Pixel-perfect screenshot baselines are sensitive to OS, font rasterization, and browser environment. > - The suite already stores external baseline artifacts and has an opt-in CI path. > - Local runs on non-matching platforms can report false-positive diffs unless the platform lock is explicit. > - This pull request documents the Linux/Ubuntu baseline constraint and makes the local static server port explicit. > - The benefit is clearer visual-review guidance and more predictable Playwright web server startup. ## Linked Issues or Issue Description No public GitHub issue exists. ### What happened? The Storybook visual baseline suite requires a matching Linux capture environment for pixel-exact comparisons, but the docs did not clearly warn local users that non-Linux environments can produce false-positive diffs. The Playwright web server command also relied on the static server's default port instead of passing the configured port explicitly. ### Expected behavior Developers should see clear Linux/Ubuntu baseline guidance before running the visual suite locally, and Playwright should start the Storybook static server on the same explicit port that the test config expects. ### Steps to reproduce 1. Review the Storybook visual docs before this PR. 2. Run or inspect the Storybook visual Playwright config. 3. Notice the missing platform guidance and implicit static server port coupling. ### Paperclip version or commit Reproducible on `master` before this branch. ### Deployment mode Local dev (pnpm dev) / built from source. ## What Changed - Documents the Linux/Ubuntu-only baseline limitation in the developer docs and visual-suite README. - Adds `--port` parsing and validation to the Storybook static server helper. - Adds regression coverage for `--port` followed by another flag. - Passes the Playwright web server port explicitly from the Storybook visual config. ## Verification - Passed: `node --check scripts/serve-storybook-static.mjs` - Passed: `node --test scripts/__tests__/serve-storybook-static.test.mjs` - Passed: `node --test scripts/__tests__/storybook-visual-baseline.test.mjs` - Greptile: 5/5 with no unresolved review threads after commit `94a649755a2ae7c4a34a3e8a1f16ec4d26d738fd`. - Not run: full `pnpm test:storybook-visual`, because it builds Storybook and runs the browser visual suite; this PR only changes docs plus server port plumbing. ## Risks Low risk. The server still defaults to port 6106 when no explicit port is provided, and invalid port values now fail fast with a clear error before the Playwright server waits for an unreachable URL. ## Model Used OpenAI GPT-5 Codex coding agent with local command execution and repository editing tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/DEVELOPING.md | 8 ++++++++ .../__tests__/serve-storybook-static.test.mjs | 16 ++++++++++++++++ scripts/serve-storybook-static.mjs | 15 ++++++++++++++- tests/storybook-visual/README.md | 13 +++++++++++++ tests/storybook-visual/playwright.config.ts | 2 +- 5 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 scripts/__tests__/serve-storybook-static.test.mjs diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index a0a6100cec..51f4b35daa 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -70,6 +70,14 @@ Accepted visual changes should update the manifest metadata and publish a new immutable archive with `pnpm storybook-visual:baseline pack` and `pnpm storybook-visual:baseline upload`; do not commit generated PNG snapshots. +Known limitation: Storybook visual baselines are Linux/Ubuntu-only. The manifest +pins the capture environment to `ubuntu-24.04` and the Playwright suite uses +pixel-exact comparison, so local runs on macOS, Windows, or other non-matching +platforms can report false-positive diffs from font rasterization and subpixel +rendering. Use the `Storybook Visual` GitHub Actions workflow on `ubuntu-latest` +as the source of truth, or run locally in a matching Linux environment before +accepting or updating baselines. + PR visual checks are opt-in while the suite stabilizes. Add the `storybook-visual` label to a PR, or run the `Storybook Visual` GitHub Actions workflow manually, to produce downloadable Playwright report/test-result diff --git a/scripts/__tests__/serve-storybook-static.test.mjs b/scripts/__tests__/serve-storybook-static.test.mjs new file mode 100644 index 0000000000..4d5a17a4bc --- /dev/null +++ b/scripts/__tests__/serve-storybook-static.test.mjs @@ -0,0 +1,16 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import test from "node:test"; + +const script = new URL("../serve-storybook-static.mjs", import.meta.url).pathname; + +test("--port followed by another flag is not parsed as the port value", () => { + const result = spawnSync(process.execPath, [script, "--port", "--unused-flag"], { + env: { ...process.env, PORT: "65536" }, + encoding: "utf8", + }); + + assert.equal(result.status, 1); + assert.match(result.stderr, /Invalid Storybook static server port: 65536/); + assert.doesNotMatch(result.stderr, /--unused-flag/); +}); diff --git a/scripts/serve-storybook-static.mjs b/scripts/serve-storybook-static.mjs index aa0c22f5cd..50d5c93526 100644 --- a/scripts/serve-storybook-static.mjs +++ b/scripts/serve-storybook-static.mjs @@ -13,7 +13,20 @@ const root = resolve( "ui", "storybook-static", ); -const port = Number(process.env.PORT ?? 6106); +const portArgIndex = process.argv.indexOf("--port"); +const explicitPort = + portArgIndex >= 0 && + process.argv[portArgIndex + 1] && + !process.argv[portArgIndex + 1].startsWith("--") + ? process.argv[portArgIndex + 1] + : null; +const portSource = explicitPort ?? process.env.PORT ?? 6106; +const port = Number(portSource); + +if (!Number.isInteger(port) || port <= 0 || port > 65535) { + console.error(`Invalid Storybook static server port: ${portSource}`); + process.exit(1); +} if (!existsSync(join(root, "index.html"))) { console.error(`No built Storybook at ${root}. Run \`pnpm build-storybook\` first.`); diff --git a/tests/storybook-visual/README.md b/tests/storybook-visual/README.md index 3f4b8220af..19fb4ef67e 100644 --- a/tests/storybook-visual/README.md +++ b/tests/storybook-visual/README.md @@ -18,6 +18,19 @@ pnpm test:storybook-visual:update `tests/storybook-visual/.snapshots/`, and checks the PNG count. The same snapshot directory can be overridden with `STORYBOOK_VISUAL_SNAPSHOT_DIR`. +## Known Limitation: Linux Baselines + +Storybook visual baselines are platform-locked. The checked-in manifest records +the capture environment as `ubuntu-24.04`, and Playwright compares screenshots +with `maxDiffPixels: 0`. Pixel-exact results are only meaningful when local runs +use the same Linux/Ubuntu capture platform as the baseline. + +macOS, Windows, and other non-matching local environments can produce +false-positive diffs from font rasterization and subpixel rendering differences. +Use the `Storybook Visual` GitHub Actions workflow on `ubuntu-latest` as the +source of truth for cross-platform review, or run the suite locally in a matching +Linux environment before accepting or updating baselines. + ## CI and Review Artifacts Storybook visual tests are opt-in while the suite stabilizes. Add the diff --git a/tests/storybook-visual/playwright.config.ts b/tests/storybook-visual/playwright.config.ts index 74bfad7c84..b942bc7798 100644 --- a/tests/storybook-visual/playwright.config.ts +++ b/tests/storybook-visual/playwright.config.ts @@ -36,7 +36,7 @@ export default defineConfig({ baseURL: "http://localhost:6106", }, webServer: { - command: "node ../../scripts/serve-storybook-static.mjs", + command: "node ../../scripts/serve-storybook-static.mjs --port 6106", url: "http://localhost:6106/index.json", reuseExistingServer: true, timeout: 30_000,