Skip to content

fix(visual-proof): outputPath escape + networkidle timeout (W8-14) - #194

Merged
Ghenghis merged 1 commit into
feat/hermes3d-7-complete-gui-repo-wiringfrom
claude/w8-14-visual-proof-harness-fix
May 10, 2026
Merged

Ghenghis merged 1 commit into
feat/hermes3d-7-complete-gui-repo-wiringfrom
claude/w8-14-visual-proof-harness-fix

Conversation

@Ghenghis

Copy link
Copy Markdown
Owner

Summary

Surgical fixes to W6-6's Playwright visual-proof harness so it produces real match/diff data against Images-GUI/. Stacks on PR #188 (W8-12 Vite Fast-Refresh). Builds on PR #189 (W6-6 first-run JSON summary).

  • outputPath is not allowed outside of the parent directory — snapshotPathTemplate: \"{arg}{ext}\" plus spec segments that escaped the test root tripped Playwright's safety check. Fix: new tests/visual/global-setup.ts mirrors Images-GUI/ into tests/visual/__refs__/ (idempotent, size+mtime check, one-way only). Spec now emits forward-only segments into __refs__/. The mirror is gitignored; Images-GUI/ remains the single source of truth.
  • 30s networkidle timeouts — the Hermes3D SPA's long-poll + SSE channels never go idle. Per Playwright's docs, networkidle is DISCOURAGED for this shape. Fix: drop networkidle from waitForStable(); keep domcontentloaded + 500ms quiet + the per-target wait_test_id toBeVisible(15s) assertion that already proves the route mounted.

Re-run Delta

03_implementation/docs/evidence/visual_proof_2026-05-09/summary.json:

Before (PR #189) After
match 0 0
diff 0 11
error 11 0
skipped-future 20 20

All 11 live targets now produce real pixel-diff data. The harness end-to-end works.

Persistence — 11 unfixed diffs (single shared cause)

All 11 live targets exceed tolerance with ratio ~0.28-0.31. Single shared root cause: reference PNGs were captured at 1536x1024, harness viewport is 1920x1080. Not 11 independent UI defects. Owned by W6-3 to fix (either re-render references at 1920x1080 or drop viewport to 1536x1024). See handoff doc table for per-target evidence_path / diff_path.

Sources

  1. https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-path-template (parent-dir safety check that triggered Bug 1)
  2. https://playwright.dev/docs/api/class-page#page-wait-for-load-state (networkidle marked DISCOURAGED)

Test plan

  • Single-target run: npx playwright test --config playwright.visual.config.ts -g \"01_dashboard_advanced_a\" — completes in 2.3s with real diff data (diff_pixels=594010 ratio=0.29).
  • Full harness run — 31 targets in ~38s; reporter writes summary.json with 11 diff / 20 skipped / 0 error.
  • globalSetup idempotent — second run hits (copied=0 skipped=31 pruned=0).
  • No commits include mirrored PNGs (verified via git check-ignore against tests/visual/.gitignore).
  • W6-6's design preserved — manifest, reporter contract, updateSnapshots: \"none\", fail-on-missing-baseline all unchanged.

Hermes evidence chain: PASS

  • Task ID: W8-14-VISUAL-PROOF-HARNESS-FIX-2026-05-09
  • Lock owner: claude-w8-14-harness-fix
  • hermes_run_gate: not applicable (no policy gate for visual-proof)

🤖 Generated with Claude Code

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented May 10, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0b839def-81e0-4f15-935b-0a0d0475f71e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/w8-14-visual-proof-harness-fix

Comment @coderabbitai help to get the list of available commands and usage tips.

Two surgical fixes to the W6-6 visual-proof harness so it produces real
match/diff data against Images-GUI/.

1. outputPath bug. snapshotPathTemplate "{arg}{ext}" + spec segments that
   walked UP from the test root into Images-GUI/ tripped Playwright's
   "outputPath is not allowed outside of the parent directory" safety
   check. Fix: new tests/visual/global-setup.ts mirrors Images-GUI/ into
   tests/visual/__refs__/ on every run (idempotent, size+mtime check).
   The spec now emits forward-only segments into __refs__/, fully inside
   the test root. Images-GUI/ remains the single source of truth; the
   mirror is one-way and gitignored.

2. networkidle timeout bug. The Hermes3D SPA's long-poll + SSE channels
   (recovery_controller, agent updates, A2A) keep the network busy past
   Playwright's 30s default. Per Playwright's docs, networkidle is
   DISCOURAGED for this app shape. Fix: drop networkidle from
   waitForStable(); rely on domcontentloaded + 500ms quiet + the
   per-target wait_test_id toBeVisible(15s) assertion.

Re-run delta vs PR #189:
  before: 0 match / 0 diff / 11 error / 20 skipped
  after:  0 match / 11 diff / 0 error / 20 skipped (all 11 live targets
          now produce real diff data; uniform ~0.29 ratio diagnosed as
          1920x1080 viewport vs 1536x1024 reference, owned by W6-3 to
          fix).

Stacks on PR #188 (W8-12 Vite Fast-Refresh fix). Builds on PR #189
(W6-6 first-run JSON summary).

Sources cited in code + handoff doc:
  - https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-path-template
  - https://playwright.dev/docs/api/class-page#page-wait-for-load-state

Files
  03_implementation/ui/playwright.visual.config.ts
  03_implementation/ui/tests/visual/visual-proof.spec.ts
  03_implementation/ui/tests/visual/global-setup.ts (NEW)
  03_implementation/ui/tests/visual/__refs__/.gitkeep (NEW)
  03_implementation/ui/tests/visual/.gitignore (NEW)
  03_implementation/docs/evidence/visual_proof_2026-05-09/summary.json
  03_implementation/docs/handoffs/W8-14_VISUAL_PROOF_HARNESS_FIX_2026-05-09.md (NEW)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Ghenghis
Ghenghis force-pushed the claude/w8-14-visual-proof-harness-fix branch from fb83e49 to 66aee96 Compare May 10, 2026 05:16
@Ghenghis
Ghenghis merged commit b2f7c54 into feat/hermes3d-7-complete-gui-repo-wiring May 10, 2026
3 checks passed
@Ghenghis
Ghenghis deleted the claude/w8-14-visual-proof-harness-fix branch May 10, 2026 05:20
Ghenghis added a commit that referenced this pull request May 10, 2026
W8-14 left 11 live targets reporting a uniform ~0.28-0.31 diff ratio
because the harness viewport (1920x1080) did not match the Images-GUI/
reference PNG dimensions (1536x1024 for 9 of 11). toHaveScreenshot is
not resolution-tolerant, so the diff was geometric, not content.

Surgical fix: drop the harness viewport to 1536x1024 to match the
reference pack (single source of truth, PR #128 user-approved).

  03_implementation/ui/playwright.visual.config.ts (use.viewport +
    projects[0].use.viewport)
  03_implementation/ui/tests/visual/visual-targets.json (viewport +
    new reference_viewport field for clarity)

Re-run delta vs PR #194:
  before: 0 match / 11 diff / 0 error / 20 skipped
  after:  9 match / 1 diff / 1 error / 20 skipped

Two remaining failures are NOT viewport-related — both are reference
PNGs captured at non-canonical sizes:

  04_source_os_60_app_coverage_matrix : ref 1672x941 (diff)
  04_source_os_remaining_categories   : ref 1586x992 (error)

Both need re-capture in PR #128 v2 (Images-GUI maintainer scope, NOT
W8-15). Documented in
03_implementation/docs/handoffs/W8-15_VISUAL_VIEWPORT_ALIGN_2026-05-09.md
with diff_pixels, ratio, evidence_path, and next_fix_attempt for each.

Stacks on PR #194 (W8-14 harness fix). Builds on PR #188 (W8-12 Vite
Fast-Refresh) for the SPA mount.

No --update-snapshots, no reference PNGs touched, no paid services.
visual-proof.spec.ts unchanged (no hardcoded viewport literals).

Sources cited:
  - https://playwright.dev/docs/api/class-testoptions#test-options-viewport
  - Images-GUI/ folder pack from PR #128 (verified PNG IHDR dims)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ghenghis added a commit that referenced this pull request May 10, 2026
W8-14 left 11 live targets reporting a uniform ~0.28-0.31 diff ratio
because the harness viewport (1920x1080) did not match the Images-GUI/
reference PNG dimensions (1536x1024 for 9 of 11). toHaveScreenshot is
not resolution-tolerant, so the diff was geometric, not content.

Surgical fix: drop the harness viewport to 1536x1024 to match the
reference pack (single source of truth, PR #128 user-approved).

  03_implementation/ui/playwright.visual.config.ts (use.viewport +
    projects[0].use.viewport)
  03_implementation/ui/tests/visual/visual-targets.json (viewport +
    new reference_viewport field for clarity)

Re-run delta vs PR #194:
  before: 0 match / 11 diff / 0 error / 20 skipped
  after:  9 match / 1 diff / 1 error / 20 skipped

Two remaining failures are NOT viewport-related — both are reference
PNGs captured at non-canonical sizes:

  04_source_os_60_app_coverage_matrix : ref 1672x941 (diff)
  04_source_os_remaining_categories   : ref 1586x992 (error)

Both need re-capture in PR #128 v2 (Images-GUI maintainer scope, NOT
W8-15). Documented in
03_implementation/docs/handoffs/W8-15_VISUAL_VIEWPORT_ALIGN_2026-05-09.md
with diff_pixels, ratio, evidence_path, and next_fix_attempt for each.

Stacks on PR #194 (W8-14 harness fix). Builds on PR #188 (W8-12 Vite
Fast-Refresh) for the SPA mount.

No --update-snapshots, no reference PNGs touched, no paid services.
visual-proof.spec.ts unchanged (no hardcoded viewport literals).

Sources cited:
  - https://playwright.dev/docs/api/class-testoptions#test-options-viewport
  - Images-GUI/ folder pack from PR #128 (verified PNG IHDR dims)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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