deckhearth/.convoys/tighten-visual-diff-path-filter.md

tighten-visual-diff-path-filter (P3 polish — single-file YAML tweak)

Status: OPEN 2026-05-26 (this PR) Priority: P3 polish (CI-cost / signal-noise; not a security or correctness issue — Screenshot diff already swallows its own "snapshot doesn't exist" failure via continue-on-error: true, so the false-trigger is wasted runtime + a slightly chatty checks tab, nothing more) Convoy owner: parent (no architect — single-file workflow YAML tweak following published GitHub Actions path-filter semantics) Opened: 2026-05-26 Branch: convoy/tighten-visual-diff-path-filter

Background

.github/workflows/visual-diff.yml is supposed to fire only on UI changes. Its current paths: filter:

paths:
  - 'pages/**'
  - 'components/**'
  - 'styles/**'
  - 'tailwind.config.js'
  - 'postcss.config.js'

But pages/** matches pages/api/** too, and the tcg-vault repo's API surface lives entirely under pages/api/ (Next.js Pages router). That means API-only PRs trigger the visual-diff workflow even though they can't possibly move a single rendered pixel. The empirical false-positives:

• PR #19 cors-tighten (squash da50d78, 2026-05-24) — touched 24 files all under pages/api/**. Triggered Screenshot diff, documented in .convoys/ship-readiness.md § P0 #5 As-shipped metrics: "Screenshot diff — workflow exited 0 because of continue-on-error: true, but the actual visual test failed with the documented 'snapshot doesn't exist' error… Triggered on PR #19 despite this being API-only because its paths: filter is pages/** which matches pages/api/** too — minor false-positive queued as tighten-visual-diff-path-filter."
• PR #20 add-rate-limiting (squash 708ef45, 2026-05-24) — touched 6 routes under pages/api/ (plus lib/rate-limit.js and pages/admin/card-import.js). Same false-trigger, same swallow. Documented in .convoys/ship-readiness.md § P0 #6 As-shipped metrics.

Cost per false-trigger: ~55s of CI runtime (the wait-for-vercel-preview step hits its 120s budget against the deployed preview, then Playwright npm ci + npx playwright install + the visual project runs — even though the test ultimately can't compare against a non-existent baseline). The Screenshot diff job exits 0 because of continue-on-error: true (Decision-4 end state of adopt-playwright-smoke, until seed-visual-baselines-on-linux lands), but it still posts a "Visual Diff — view run" comment and clutters the PR Checks tab with a green-but-meaningless run.

Design decision — negated-glob !pages/api/**

GitHub Actions evaluates paths: with minimatch and supports !-prefixed exclusion patterns per the official path-filter cheatsheet. Order matters: a !pattern only takes effect if it comes AFTER an include that already matched the path. So the canonical shape is:

paths:
  - 'pages/**'
  - '!pages/api/**'   # must follow 'pages/**' to subtract from it
  - 'components/**'
  ...

Considered alternatives:

1. Per-feature paths — replace pages/** with explicit subdirectory globs (pages/!(api)/** or pages/dashboard.js, pages/cards/**, pages/decks/**, …). Rejected: too verbose, needs to be touched every time a top-level page is added, defeats the "trigger on UI changes" intent.
2. Extglob pages/!(api)/** — would work in bash with extglob enabled, but minimatch's default options used by GitHub Actions do NOT enable extglob without a flag we can't set from YAML. The queue entry explicitly flagged this risk; the negated-glob shape is the safer documented path.
3. Move the gate into the gate: job — add a step that diffs pages/api/** and sets should_run=false if every changed file is API-only. Rejected: more code, more surface, doesn't actually fire faster (the gate job itself spins up a runner). The native paths: filter short-circuits BEFORE any runner spins up, which is the cheapest possible exclusion.

The simple negation is sufficient and matches GitHub's published guidance.

The fix

Single edit in .github/workflows/visual-diff.yml. Insert !pages/api/** immediately after pages/**, with an inline comment explaining the ordering rule and the empirical motivation:

paths:
  - 'pages/**'
  # Exclude API-only edits — they don't render UI, so they can't move
  # any visual-diff pixels. Order matters: GitHub Actions evaluates the
  # `paths:` list with minimatch and applies `!`-prefixed exclusions
  # only after they've already matched a prior include. Keep this entry
  # immediately AFTER `pages/**`.
  # Surfaced by `tighten-visual-diff-path-filter` after PR #19
  # (cors-tighten) and PR #20 (add-rate-limiting) both falsely
  # triggered Screenshot diff at ~55s/PR.
  - '!pages/api/**'
  - 'components/**'
  - 'styles/**'
  - 'tailwind.config.js'
  - 'postcss.config.js'

All five existing entries are preserved; only the one exclusion entry is added.

.github/workflows/preview-smoke.yml — left untouched

Verified the sibling workflow's shape:

on:
  pull_request:
    branches: [main]
    types: [labeled, opened, synchronize, reopened]

preview-smoke.yml has no paths: filter at all — it triggers on every PR targeting main (modulo the in-job gate: skip via pipeline: skip smoke in the PR body). This is intentional: a smoke test that hits the home redirect, the sign-in page, and /api/health SHOULD run on every PR including API-only ones, because changes to pages/api/** can break those routes too. There is no false-positive shape to fix here. Leaving preview-smoke.yml strictly out of scope.

Verification plan

1. YAML parse — python3 -c "import yaml; ..." confirms the paths: list deserializes to the expected 6-entry list with '!pages/api/**' at index 1 (immediately after 'pages/**'). Done at gate time, see § Acceptance criteria.

2. npm run lint — exits 1 with 128 problems (baseline preserved, no regression). YAML files don't go through ESLint; verification here is just that we didn't accidentally edit a .js source file.

3. npm run test:run — 21/21 pass. YAML changes don't touch any test surface; verification only.

4. Post-merge CI behavior verification — DEFERRED. The only true verification that the !pages/api/** exclusion actually fires the way we expect is observing the next API-only PR after this merges and confirming Screenshot diff does NOT appear in its Checks tab. We document this explicitly here so the doc-writer pass that closes the convoy can record the next API-only PR's number + a "Screenshot diff: not triggered" line as the as-shipped success criterion (mirroring the fix-reset-db-script convoy's Screenshot-diff-not-triggered line in its as-shipped block).

   We do NOT attempt to live-verify the path filter at convoy time (e.g. by pushing a throwaway API-only commit to a sacrificial branch and watching CI). That'd be theater — GitHub's path-filter semantics are documented and stable, and the YAML parse + the syntax match against the published cheatsheet is enough pre-merge confidence for a P3 polish convoy.

Acceptance criteria

• python3 -c "import yaml; d=yaml.safe_load(open('.github/workflows/visual-diff.yml')); print(d[True]['pull_request']['paths'])" → ['pages/**', '!pages/api/**', 'components/**', 'styles/**', 'tailwind.config.js', 'postcss.config.js'] (order-sensitive)
• npm run lint → exit 1 with 128 problems (baseline preserved)
• npm run test:run → 21/21 pass
• .github/workflows/preview-smoke.yml unchanged in this PR's diff
• No other workflow files touched

Risks

• R1 — minimatch syntax compatibility. GitHub Actions uses minimatch internally; the ! prefix at the start of a pattern is documented as the canonical exclusion syntax. If for any reason Actions rejects this shape on the next workflow load (unlikely — this exact shape is shown in the published cheatsheet), the workflow would either fail to register OR silently treat the ! pattern as a literal include. Fallback: restructure to per-feature path globs (pages/dashboard.js, pages/cards/**, pages/decks/**, pages/deck/**, pages/deck-builder.js, pages/scanner.js, pages/profile.js, pages/settings.js, pages/login.js, pages/register.js, pages/admin/**, pages/collection/**, pages/collections.js, pages/community/**, pages/invite/**, pages/my-cards.js, pages/card/**, pages/_app.js, pages/_document.js, pages/_error.js, pages/index.js). More verbose, but unambiguously valid. Track as a follow-up convoy ONLY if the post-merge verification step (next API-only PR) shows the exclusion didn't fire.
• R2 — future pages/<non-api>/<api-like> subdir. If someone later adds a directory like pages/server/** that contains both API-style endpoints AND visual UI pages, the simple !pages/api/** exclusion would not catch it, and visual-diff would fire on changes to that directory. Documented but accepted: the repo convention for the foreseeable future is "all backend lives under pages/api/**", and the only realistic alternative ("server components" or similar) would warrant its own paths-filter revisit at that point. R2 is a "watch this space" risk, not a blocker.

Scope

• In scope: .github/workflows/visual-diff.yml only (plus this convoy planning doc).
• Out of scope: any other workflow file. Verified preview-smoke.yml has no paths: filter and intentionally fires on every PR, so no mirror-fix is needed there.

Why no architect

This is a single-file YAML tweak following published vendor documentation. No new precedents; no new decisions; the queue entry in .convoys/ship-readiness.md § Queued convoys already ratified the negated-glob direction. Parent applies the fix, runs the bounded checks (YAML parse + lint baseline + vitest), opens the PR. If anything surprising surfaces (the YAML doesn't parse, minimatch rejects the syntax), the parent stops and dispatches an architect mid-execution.

Out of scope (queued follow-ups)

• seed-visual-baselines-on-linux (priority: P3 polish; was already queued by adopt-playwright-smoke) — once visual baselines are seeded under tests/visual/__screenshots__/ from a Linux runner (or the documented Playwright Docker container), the Screenshot diff job will start posting real visual-diff comparisons and continue-on-error: true can be removed. This convoy's path-filter tightening is orthogonal to baseline seeding — both are needed eventually, but neither blocks the other. Surfaced in AGENTS.md § 6 Testing.

As-shipped

(stub — populated by post-merge doc-writer pass)

• Squash commit: <TBD>
• PR: <TBD>
• Diff stat: <TBD> (expected: 2 files, +N / -0 — visual-diff.yml +N for the one entry + comment block; this convoy file +M for the full planning doc)
• Verification at merge:
  • YAML parse: paths list includes !pages/api/** immediately after pages/**
  • Lint: 128 problems (baseline preserved)
  • Vitest: 21/21
  • All pre-existing CI gates green at merge
• Post-merge success criterion (the deferred verification from § Verification plan): the next API-only PR after this merges does NOT show Screenshot diff in its Checks tab. Doc-writer to record that PR's number + the absence of Screenshot diff as the as-shipped success line, mirroring .convoys/fix-reset-db-script.md's "Screenshot diff: not triggered (script-only PR — paths: filter excludes scripts/**…)" line.

Owns

Parent (single-file proven-pattern fix; no architect or implementer subagent required).