Conversation
routes.tsx exported only types and data arrays (TabDef, TABS, PRIMARY_TABS) with zero React components and zero JSX. The .tsx extension caused @vitejs/plugin-react to inject react-refresh boundary instrumentation, which fails with 'ReferenceError: $RefreshReg$ is not defined' when no React components are exported from the module. Renaming to .ts removes the file from the babel-plugin-react-refresh transform pipeline. All 5 importers use bare paths (no extension) so no consumer changes are required. Root cause class 2: mixed React + non-React exports in the same file (in this case, ALL non-React exports under a .tsx name). Refs: https://github.com/facebook/react/tree/main/packages/react-refresh Refs: https://github.com/vitejs/vite-plugin-react Task ID: W8-12-VITE-REFRESH-FIX-2026-05-09
Add a dedicated visual-proof Playwright suite that pixel-diffs every PNG
in Images-GUI/ against the live Hermes3D OS dashboard. 31 reference
images are catalogued in visual-targets.json: 11 live targets exercise
toHaveScreenshot today; 20 future targets are skipped with reasons that
point to the owning lane (W6-3 dashboard mode switcher, W6-4 Action
Window, theme switcher).
Implementation notes:
- updateSnapshots: "none" guarantees this harness never auto-updates a
reference. Refresh requires --update-snapshots and a PR review.
- snapshotPathTemplate: "{arg}{ext}" lets the spec point directly at
Images-GUI/ paths so the reference PNGs remain the single source of
truth (no __snapshots__/ duplicates).
- No new npm dep: pixel diff goes through Playwright's bundled
pixelmatch via toHaveScreenshot.
- A custom reporter writes 03_implementation/docs/evidence/visual_proof_
2026-05-09/summary.json with one row per target (match/diff/skipped/
error + diff_pixels + diff_path + evidence_path).
Sources:
- https://playwright.dev/docs/test-snapshots
- https://playwright.dev/docs/api/class-pageassertions#page-assertions-to-have-screenshot
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first run of the visual harness on
feat/hermes3d-7-complete-gui-repo-wiring shows 11 errors / 20
skipped-future / 0 match. Root cause of the 11 errors is a pre-existing
Vite Fast-Refresh issue in src/app/routes.tsx (`$RefreshReg$ is not
defined`) blocking the SPA from mounting; the existing E2E spec
reproduces the same failure mode in the same worktree. The harness is
working correctly — it reports concrete per-target error data instead
of fabricating matches.
Reporter fix: test.skip(reason, fn) does NOT execute the wrapped
function, so the inline `test.info().annotations.push(...)` never
fired for the 20 future-targets, and they were missing from the JSON
summary. Switched the reporter to look up the manifest entry by the
test's describe title ("visual: <target_name>") so skipped rows are
populated from visual-targets.json directly.
First-run summary persisted at:
03_implementation/docs/evidence/visual_proof_2026-05-09/summary.json
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vite@8.0.10 + @vitejs/plugin-react@6.0.1 do not reliably inject the Fast-Refresh preamble via transformIndexHtml on the plain SPA path. Every .tsx user module then threw at top of file: ReferenceError: $RefreshReg$ is not defined ReferenceError: $RefreshSig$ is not defined This blocked the entire SPA from mounting (white screen) and was the upstream cause of all 11 ERROR rows in W6-6's first visual-proof run. Combined with 1cca312 (rename routes.tsx -> routes.ts), the SPA now mounts cleanly. Fix verified by: - npx playwright (boot-check spec) -> PASS, no console errors - npm run lint (tsc --noEmit) -> 0 errors - npx vite build -> succeeds, 0 RefreshReg/RefreshSig refs in dist js - W6-6 visual-proof rerun: 11 errors are now meaningful Playwright timeouts + harness snapshot-path bugs (NOT $RefreshReg$ errors) Root cause class 4: react-refresh runtime not loaded before component module evaluates. Production builds set skipFastRefresh=true so the inline stubs are inert dead code (~150 bytes in dist/index.html). Refs: https://github.com/vitejs/vite-plugin-react/blob/plugin-react@6.0.1/packages/plugin-react/README.md#initialize-hmr-runtime-in-client-entrypoint Refs: https://github.com/facebook/react/tree/main/packages/react-refresh Task ID: W8-12-VITE-REFRESH-FIX-2026-05-09 hermes_run_gate: PASS Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced May 10, 2026
Ghenghis
merged commit May 10, 2026
b6617c2
into
feat/hermes3d-7-complete-gui-repo-wiring
3 checks passed
Ghenghis
added a commit
that referenced
this pull request
May 10, 2026
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
added a commit
that referenced
this pull request
May 10, 2026
) 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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
03_implementation/ui/src/app/routes.tsx->routes.ts(data-only file, no JSX, was triggering bogus refresh-wrapper instrumentation)<script>preamble stubs toindex.htmlthat pre-definewindow.$RefreshReg$andwindow.$RefreshSig$BEFORE any ESM module evaluates, fixingvite@8.0.10+@vitejs/plugin-react@6.0.1HTML preamble regressionHermes evidence chain: PASS
W8-12-VITE-REFRESH-FIX-2026-05-09npm run lintPASS (0 TS errors),npx vite buildPASS (0 RefreshReg refs in dist js), W6-6 visual-proof rerun shows fix is upstream-resolvedfeat/hermes3d-7-complete-gui-repo-wiringclaude-w8-12-vite-fixRoot cause
Two cascading bugs identified by W6-6's harness:
Bug A (Class 2 — mixed React + non-React exports):
routes.tsxexported only types and data arrays (TabDef,TABS,PRIMARY_TABS) with zero React components / zero JSX. The.tsxextension matched plugin-react's default include filter (/\.(mdx|js|jsx|ts|tsx)$/), causing OXC to insert refresh-wrapper calls referencing undefined globals.Bug B (Class 4 — react-refresh runtime not loaded): In
vite@8.0.10+@vitejs/plugin-react@6.0.1,transformIndexHtml's preamble injection does not fire reliably for the plain SPA path. Inspection of the served HTML showed no inline<script type="module">defining the$RefreshReg$/$RefreshSig$globals. Importing@vitejs/plugin-react/preamblefrommain.tsxalso did not work — the virtual module returned empty content because itsisEnabled()closure readskipFastRefresh === true(timing/closure mismatch withviteBabel.configResolvedin Vite 8's environment system).Fix: pre-define the global stubs inline in
index.htmlwith a non-type=module<script>block. These match exactly the stubs that the plugin'spreambleCodewould install. In production,plugin-reactsetsskipFastRefresh=trueand the OXC refresh-wrapper is never emitted, so the stubs are unreferenced dead code (~150 bytes indist/index.html, zero refs indist/assets/*.jsverified via grep).Files added/changed
03_implementation/ui/src/app/routes.tsx->routes.ts(rename, no consumer changes — all 5 importers use bare paths)03_implementation/ui/index.html(4 lines of stubs + 21 lines of comment explaining why)03_implementation/docs/handoffs/W8-12_VITE_REFRESH_FIX_2026-05-09.md(new)03_implementation/docs/evidence/visual_proof_2026-05-09/summary.json(regenerated by harness re-run; cherry-picked from W6-6)03_implementation/ui/playwright.visual.config.ts,tests/visual/visual-proof.spec.ts,tests/visual/visual-proof-reporter.ts,tests/visual/visual-targets.json,docs/handoffs/HERMES_GUI_VISUAL_PROOF_2026-05-09.md(cherry-picked from W6-6)Verification
$RefreshReg$ ReferenceErroron every .tsx module#rootpopulated, no console errorsnpm run lint(tsc --noEmit)npx vite buildRefreshReg/RefreshSigrefs in prod js bundlewait_test_id dashboard-root mounts(SPA never mounted)outputPath outside parentconfig bug (unrelated to this PR)The fact that the error counts are identical but the error CLASSES are completely different is the proof: before, the SPA could not mount at all; after, the SPA mounts and Playwright is interacting with a live page. The remaining 11 errors are downstream issues outside W8-12 scope.
Sources cited
@vitejs/plugin-react@6.0.1README — "Initialize HMR runtime in client entrypoint": https://github.com/vitejs/vite-plugin-react/blob/plugin-react@6.0.1/packages/plugin-react/README.md#initialize-hmr-runtime-in-client-entrypointTest plan
cd 03_implementation/ui && npx playwright test --config=playwright.boot.config.ts-> 1 passed (boot-check, since removed)cd 03_implementation/ui && npm run lint-> 0 errorscd 03_implementation/ui && npx vite build-> succeeds, dist/index.html ~1.96 kBcd 03_implementation/ui && PYTHON=python npx playwright test --config=playwright.visual.config.ts-> errors changed class (proof of fix); summary.json regenerateddist/assets/*.jshas zeroRefreshReg/RefreshSigreferences aftervite build