feat: audit vrt tools (#9592)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: arii <342438+arii@users.noreply.github.com>

Author: arii

.github/copilot-instructions.md

@@ -294,6 +294,11 @@ This project prioritizes a lean and maintainable codebase. AI assistants should
 - Simplifying complex functions into smaller, more manageable units.
 - Removing boilerplate or over-specified types that can be inferred.
 
+### 8. Visual Regression Testing (VRT) Stabilization Standards
+
+Strictly adhere to the standards defined in:
+[.github/instructions/vrt-stability.instructions.md](.github/instructions/vrt-stability.instructions.md)
+
 ## Quick Reference: Anti-Patterns to Avoid
 
 | ❌ AI Slop                                              | ✅ Correct Approach                                                                    |

.github/instructions/vrt-stability.instructions.md

@@ -0,0 +1,46 @@
+---
+applyTo: 'tests/playwright/**/*.spec.ts,tests/playwright/lib/visual.ts'
+description: 'Use when editing Playwright visual regression tests; enforce deterministic VRT setup, strict thresholds, and no sleep-based stabilization.'
+---
+
+# VRT Stability Guardrails
+
+When changing Playwright visual regression tests, follow these standards.
+
+## Threshold policy
+
+- Default to `maxDiffPixelRatio: 0.1`.
+- Allow up to `0.15` only for clearly documented dynamic captures.
+- Do not introduce `maxDiffPixelRatio > 0.15`.
+- Keep `threshold: 0.2` and `scale: 'css'` in shared screenshot behavior.
+
+## Synchronization policy
+
+- Do not use `waitForTimeout(...)` for stabilization.
+- Use deterministic checks instead:
+  - `await page.evaluateHandle(() => document.fonts.ready)`
+  - `await page.waitForLoadState('networkidle')`
+  - explicit assertions on target UI state (`toBeVisible`, `toHaveCSS`, `toHaveText`)
+  - layout read/reflow (`await page.evaluate(() => document.body.offsetHeight)`)
+
+## Responsive screenshot policy
+
+- Always set viewport explicitly before responsive snapshots.
+- Wait for viewport convergence before capture:
+  - `await page.waitForFunction((w) => document.body.clientWidth === w, width)`
+
+## Masking policy
+
+- Mask only truly dynamic elements (timers, live HR values, websocket-driven changing text).
+- Use shared mask helpers (`getDynamicContentMasks`, `getHrMasks`) to keep masks consistent.
+- Do not mask large stable layout containers.
+
+## Portal and locator policy
+
+- For menu/popover/modal locator screenshots, avoid full-page scroll side effects that can detach portals.
+- `skipA11y: true` is allowed for portal snapshots only when accompanied by a brief rationale comment.
+
+## Stateful suite policy
+
+- Use `resetServerState(request)` in `beforeEach` when server state can leak between tests.
+- Add deterministic teardown in `afterEach` for long-running state (for example `stopTimer(...)`).

.github/prompts/AGENTS.md

@@ -37,6 +37,23 @@ Security and Performance: Ensure the refactored code is performant, especially f
   - Use data factories or mock data generators for all tests to ensure consistency and readability.
   - Use ARIA labels and `data-testid` attributes for easier and more reliable testing of UI components.
 
+- **Playwright VRT Review Standards (Critical):**
+  - Treat `maxDiffPixelRatio: 0.1` as the baseline for standard snapshots.
+  - Allow up to `maxDiffPixelRatio: 0.15` only for clearly justified dynamic/complex captures (for example dense chart + HR overlays).
+  - Do not approve or suggest `maxDiffPixelRatio > 0.15` as a stabilization tactic.
+  - Keep `threshold: 0.2` and `scale: 'css'` in shared screenshot defaults.
+  - Reject arbitrary sleep-based stabilization (`waitForTimeout(...)`); require deterministic checks instead:
+    - `await page.evaluateHandle(() => document.fonts.ready)`
+    - `await page.waitForLoadState('networkidle')`
+    - explicit state assertions (`toBeVisible`, `toHaveCSS`, `toHaveText`)
+    - layout read/reflow (`await page.evaluate(() => document.body.offsetHeight)`)
+  - For responsive snapshots, require explicit viewport convergence checks:
+    - `await page.setViewportSize({ width, height })`
+    - `await page.waitForFunction((w) => document.body.clientWidth === w, width)`
+  - Prefer dynamic masking helpers (`getDynamicContentMasks`, `getHrMasks`) over raising thresholds.
+  - For portal-based targets (MUI menus/popovers/modals), avoid full-page scroll side effects on locator screenshots and allow `skipA11y: true` only with a brief rationale.
+  - In stateful VRT suites, require deterministic lifecycle handling (`resetServerState(request)` in setup and teardown hooks such as `stopTimer(...)` in `afterEach`).
+
 Output Format:
 
 Explanation: Start with a section titled Improvements: containing a concise, technical explanation of what was improved and why, with specific references to the modern features used (e.g., "Improved data transformation using array spread for guaranteed immutability and used optional chaining for safe access to the nested session.data object.").

prompts/fix-mode.md

@@ -24,6 +24,24 @@ The CI pipeline has failed. Your ONLY goal is to diagnose the failure and provid
 2.  **Diagnose**: Why did it fail? (e.g., type error, test timeout, missing mock).
 3.  **Fix**: Provide a concrete code fix.
 
+### VRT Failure Guardrails (Apply when failure is visual regression related)
+
+1. Do not raise `maxDiffPixelRatio` as the first fix.
+2. Keep baseline `maxDiffPixelRatio: 0.1`; allow up to `0.15` only with explicit dynamic-capture justification.
+3. Treat `maxDiffPixelRatio > 0.15` as a policy violation unless exceptional rationale is provided.
+4. Keep screenshot defaults aligned with `threshold: 0.2` and `scale: 'css'`.
+5. Reject sleep-based stabilization (`waitForTimeout(...)`); prefer deterministic readiness checks:
+
+- `await page.evaluateHandle(() => document.fonts.ready)`
+- `await page.waitForLoadState('networkidle')`
+- explicit UI assertions (`toBeVisible`, `toHaveCSS`, `toHaveText`)
+- layout read (`await page.evaluate(() => document.body.offsetHeight)`)
+
+6. For responsive captures, require explicit viewport lock and width convergence checks.
+7. Prefer dynamic masking (`getDynamicContentMasks`, `getHrMasks`) over tolerance inflation.
+8. For portal-based locator captures, avoid full-page scroll side effects and use `skipA11y: true` only with rationale.
+9. Ensure deterministic lifecycle handling in stateful suites (`resetServerState(request)` and teardown hooks such as `stopTimer(...)`).
+
 ### Output Format
 
 Return a JSON object:

prompts/standard-review.md

@@ -115,6 +115,22 @@ You are a senior software engineer. Your goal is to provide a high-signal, low-n
 3.  **Performance**: Look for N+1 queries, unnecessary re-renders, or memory leaks.
 4.  **Maintainability**: Is the code readable? D.R.Y.? suitably typed?
 
+### 2.1 VRT-Specific Review Policy (When Playwright visual tests are touched)
+
+- Enforce baseline `maxDiffPixelRatio: 0.1`.
+- Allow at most `maxDiffPixelRatio: 0.15` only for documented dynamic/complex captures.
+- Treat `maxDiffPixelRatio > 0.15` as a review issue unless there is exceptional, explicit rationale.
+- Ensure shared screenshot behavior keeps `threshold: 0.2` and `scale: 'css'`.
+- Reject sleep-based stabilization (`waitForTimeout(...)`) and require deterministic checks:
+  - `document.fonts.ready`
+  - `page.waitForLoadState('networkidle')`
+  - explicit `toBeVisible` / `toHaveCSS` / `toHaveText` assertions
+  - layout read (`document.body.offsetHeight`)
+- For responsive snapshots, require explicit viewport setup and width convergence checks (`document.body.clientWidth`).
+- Prefer dynamic masks (`getDynamicContentMasks`, `getHrMasks`) over threshold increases.
+- For portal captures (MUI menu/popover/modal), avoid full-page scroll side effects on locator screenshots and allow `skipA11y: true` only with rationale.
+- In stateful suites, require deterministic setup/teardown (for example `resetServerState(request)` and timer teardown hooks).
+
 ### 3. Feedback Style
 
 - **Actionable**: Suggest specific code changes with examples.

scripts/audit-vrt-changes.sh

@@ -0,0 +1,58 @@
+#!/bin/bash
+
+for cmd in gh jq; do
+  if ! command -v "$cmd" &> /dev/null; then
+    echo "Error: $cmd is not installed." >&2
+    exit 1
+  fi
+done
+
+VRT_PATTERN="tests/playwright/.*(spec\.ts|visual\.ts|test-helpers\.ts)"
+OUTPUT_DIR="vrt-file-audits"
+
+rm -rf "$OUTPUT_DIR"
+mkdir -p "$OUTPUT_DIR"
+
+prs=$(gh pr list --state open --json number,headRefName,title)
+
+echo "$prs" | jq -c '.[]' | while read -r pr; do
+    pr_num=$(echo "$pr" | jq -r '.number')
+    branch=$(echo "$pr" | jq -r '.headRefName')
+    title=$(echo "$pr" | jq -r '.title')
+
+    changed_files=$(gh pr diff "$pr_num" --name-only | grep -E "$VRT_PATTERN")
+
+    if [ -n "$changed_files" ]; then
+        audit_file="$OUTPUT_DIR/pr-${pr_num}.md"
+
+        {
+            echo "# PR #$pr_num: $title"
+            echo "**Branch:** \`$branch\`"
+            echo ""
+            echo "#### [[TODO]] Global Feedback"
+            echo "- [ ] Reconcile conflicting \`maxDiffPixelRatio\` changes"
+            echo "- [ ] Ensure consistent masking strategy across all PRs"
+            echo ""
+            echo "---"
+        } > "$audit_file"
+
+        pr_diff=$(gh pr diff "$pr_num" --patch)
+        for file_path in $changed_files; do
+            {
+                echo "### \`$file_path\`"
+                echo "\`\`\`diff"
+                echo "$pr_diff" | awk -v path="$file_path" '
+                    $0 ~ "diff --git a/"path" " {hunk=1; print; next}
+                    $0 ~ "diff --git a/" {hunk=0}
+                    hunk {print}
+                '
+                echo "\`\`\`"
+                echo ""
+            } >> "$audit_file"
+        done
+        
+        echo "   ✅ Logged PR #$pr_num changes to pr-${pr_num}.md"
+    fi
+done
+
+echo "🎉 File-based audit complete. View reports in /$OUTPUT_DIR"