ci: cut ds-shots PR wall-clock — build cache + 2-width captures (TASK-22081)

.github/workflows/tests.yml

@@ -302,8 +302,9 @@ jobs:
                   echo 'Files that can move a pixel:'
                   printf '%s\n' "$HITS"
 
-    # Visual diff over src/dev/fixtures/registry.ts — 30 fixtures at four phone
-    # widths, 120 PNGs. No API, no database, no provider key and no secret: a
+    # Visual diff over src/dev/fixtures/registry.ts — 30 fixtures, four phone
+    # widths on baseline pushes and two on PRs (see the capture step).
+    # No API, no database, no provider key and no secret: a
     # fixture answers every request the app makes.
     #
     # ADVISORY. It is deliberately NOT in `ci-success.needs`, so it can never
@@ -410,6 +411,23 @@ jobs:
                   echo "::error::playwright install-deps failed after 3 attempts"
                   exit 1
 
+            # Next's incremental build cache — roughly halves the ~4 min build
+            # when warm. Key follows the Next.js docs recipe: exact key per
+            # source hash, restore-keys falls back to the newest cache for the
+            # same lockfile, which is the incremental rebuild Next optimizes.
+            # Restore-only on purpose: a save on every PR push would upload a
+            # fresh 100-400 MB entry that is branch-scoped (no other PR can
+            # read it), churning the shared 10 GB cache quota and spending
+            # 1-2 min on the upload. Push runs save below instead, so every
+            # PR restores a near-fresh dev-scoped entry via restore-keys.
+            - name: Restore the Next build cache
+              uses: actions/cache/restore@v4
+              with:
+                  path: .next/cache
+                  key: ds-shots-next-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-${{ hashFiles('src/**/*.ts', 'src/**/*.tsx', 'src/**/*.js', 'src/**/*.jsx') }}
+                  restore-keys: |
+                      ds-shots-next-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-
+
             - name: Build
               env:
                   # Without this DEV_TOOLS_ENABLED is false, ?__fixture= is
@@ -424,12 +442,32 @@ jobs:
                   NEXT_PUBLIC_BASE_URL: http://127.0.0.1:3080
               run: pnpm build
 
+            # Push runs only (dev/main/DS train land several times a day), so
+            # PRs always find a near-fresh entry without per-PR churn. Right
+            # after Build rather than a post step: a capture failure must not
+            # lose the cache. A re-run finds the key taken — losing that race
+            # costs nothing, the entry is already there.
+            - name: Save the Next build cache
+              if: github.event_name == 'push'
+              continue-on-error: true
+              uses: actions/cache/save@v4
+              with:
+                  path: .next/cache
+                  key: ds-shots-next-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-${{ hashFiles('src/**/*.ts', 'src/**/*.tsx', 'src/**/*.js', 'src/**/*.jsx') }}
+
             # The capture fails a shot that still has a spinner on camera, on
             # purpose. No continue-on-error: a red step is how anyone finds out.
+            #
+            # PR runs shoot only the two extreme widths — 320 finds the most
+            # overflow bugs, 430 is the widest device — which halves capture
+            # time. Baseline pushes keep all four widths so a later full
+            # capture always has something to diff against; visual-diff.mjs
+            # ignores baseline widths a partial capture never shot. The
+            # project names are the widths (playwright.shots.config.ts).
             - name: Capture screenshots
               env:
                   SHOTS_OUT: e2e/__shots__/${{ github.event_name == 'push' && 'base' || 'head' }}
-              run: pnpm test:visual:capture
+              run: pnpm test:visual:capture ${{ github.event_name == 'pull_request' && '--project=320 --project=430' || '' }}
 
             - name: Save this commit as the baseline
               if: github.event_name == 'push'

scripts/__tests__/visual-diff-widths.test.js

@@ -0,0 +1,112 @@
+const { spawnSync } = require('child_process')
+const fs = require('fs')
+const os = require('os')
+const path = require('path')
+const { PNG } = require('pngjs')
+
+const SCRIPT_PATH = path.join(__dirname, '..', 'visual-diff.mjs')
+
+// PR captures shoot 2 of the baseline's 4 widths (tests.yml), so the diff
+// must not report the widths a partial capture never shot as "removed" —
+// while a screen that is really gone must still be reported. The script is a
+// CI entrypoint, so drive it the way the workflow does: two directories in,
+// JSON out.
+function png(color) {
+    const img = new PNG({ width: 2, height: 2 })
+    for (let i = 0; i < img.data.length; i += 4) {
+        img.data[i] = color
+        img.data[i + 3] = 255
+    }
+    return PNG.sync.write(img)
+}
+
+function capture(dir, shots) {
+    fs.mkdirSync(dir, { recursive: true })
+    for (const [file, color] of Object.entries(shots)) fs.writeFileSync(path.join(dir, file), png(color))
+}
+
+function diff(before, after) {
+    const out = path.join(path.dirname(before), 'diff')
+    const result = spawnSync(process.execPath, [SCRIPT_PATH, before, after, `--out=${out}`, '--json'], {
+        encoding: 'utf-8',
+    })
+    expect(result.status).toBe(0)
+    return JSON.parse(result.stdout)
+}
+
+describe('visual-diff partial-width capture', () => {
+    let root
+    beforeEach(() => {
+        root = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-diff-'))
+    })
+
+    it('does not report baseline-only widths as removed', () => {
+        capture(path.join(root, 'base'), {
+            'home@320.png': 0,
+            'home@375.png': 0,
+            'home@430.png': 0,
+        })
+        capture(path.join(root, 'head'), {
+            'home@320.png': 0,
+            'home@430.png': 0,
+        })
+
+        const report = diff(path.join(root, 'base'), path.join(root, 'head'))
+
+        expect(report.removed).toEqual([])
+        expect(report.added).toEqual([])
+        expect(report.changed).toEqual([])
+        expect(report.unchanged).toBe(2)
+    })
+
+    it('still reports a screen that is really gone', () => {
+        capture(path.join(root, 'base'), {
+            'home@320.png': 0,
+            'home@375.png': 0,
+            'gone@320.png': 0,
+            'gone@375.png': 0,
+        })
+        capture(path.join(root, 'head'), {
+            'home@320.png': 0,
+        })
+
+        const report = diff(path.join(root, 'base'), path.join(root, 'head'))
+
+        // 375 was never shot, so only the captured width proves the removal
+        expect(report.removed).toEqual(['gone@320.png'])
+    })
+
+    it('fails loud when the capture produced nothing', () => {
+        capture(path.join(root, 'base'), { 'home@320.png': 0 })
+        capture(path.join(root, 'head'), {})
+
+        const out = path.join(root, 'diff')
+        const result = spawnSync(
+            process.execPath,
+            [SCRIPT_PATH, path.join(root, 'base'), path.join(root, 'head'), `--out=${out}`, '--json'],
+            { encoding: 'utf-8' }
+        )
+
+        // an empty after dir must never diff all-green — that is tool breakage
+        expect(result.status).toBe(2)
+        expect(result.stderr).toContain('capture produced nothing')
+    })
+
+    it('still detects pixel changes and additions at captured widths', () => {
+        capture(path.join(root, 'base'), {
+            'home@320.png': 0,
+            'home@375.png': 0,
+        })
+        capture(path.join(root, 'head'), {
+            'home@320.png': 255,
+            'new-screen@320.png': 0,
+        })
+
+        const report = diff(path.join(root, 'base'), path.join(root, 'head'))
+
+        expect(report.changed.map((c) => c.file)).toEqual(['home@320.png'])
+        expect(report.changed[0].percent).toBe(100)
+        expect(report.added).toEqual(['new-screen@320.png'])
+        expect(report.removed).toEqual([])
+    })
+})

scripts/visual-diff.mjs

@@ -63,6 +63,14 @@ const shots = (dir) => new Set(readdirSync(dir).filter((f) => f.endsWith('.png')
 const before = shots(beforeDir)
 const after = shots(afterDir)
 
+// A capture that produced nothing must stay loud: with zero after-files the
+// width-aware removal filter below would turn "every screenshot vanished"
+// into an all-green report. That is tool breakage, not a diff result.
+if (before.size > 0 && after.size === 0) {
+    console.error(`no PNGs in ${afterDir} — the capture produced nothing`)
+    process.exit(2)
+}
+
 mkdirSync(outDir, { recursive: true })
 
 const changed = []
@@ -95,7 +103,15 @@ for (const file of [...after].filter((f) => before.has(f)).sort()) {
 }
 
 const added = [...after].filter((f) => !before.has(f)).sort()
-const removed = [...before].filter((f) => !after.has(f)).sort()
+
+// A partial capture must not report the widths it never shot as "removed":
+// PR runs shoot 2 of the baseline's 4 widths (see tests.yml), so only a width
+// the after capture actually produced can prove a removal. A screen that is
+// really gone is still reported — its files are missing at the captured
+// widths too.
+const widthOf = (f) => f.match(/@(\d+)\.png$/)?.[1] ?? ''
+const afterWidths = new Set([...after].map(widthOf))
+const removed = [...before].filter((f) => !after.has(f) && afterWidths.has(widthOf(f))).sort()
 
 changed.sort((x, y) => y.percent - x.percent)