Repository navigation
feat(visual-targets): canonical 31-PNG manifest with regions + viewports + themes (W15 A8) - #206
Conversation
…rts + themes (W15 A8) Adds the canonical visual target registry covering all 31 Images-GUI PNGs: - 03_implementation/ui/tests/visual/visual-targets.canonical.json — 37 parent targets (20 single + 11 collage parents + 6 theme variants) plus 103 region children, 140 total tracked targets. - 03_implementation/ui/tests/visual/visual-targets.canonical.schema.json — JSON Schema 2020-12 spec for the registry (single / collage / region / theme_variant / user_direction classes, with conditional region/parent requirements). - 03_implementation/docs/handoffs/W15_A2_IMAGES_GUI_INVENTORY_2026-05-10.md — inventory reconstructed from filesystem scan (A2 handoff was missing from develop at A8 dispatch time). Per-target schema covers target_id, image_path, dimensions, route, viewport (per-PNG override for the 5 1672x941 and 1 1586x992 outliers), theme, ui_state, crop_region, backend_truth_source, playwright_proof_screenshot, pass_fail_status, owning_pr, final_verdict, class, live_or_future. Collage parents expand into clip-coordinate region children; the 09-themes palette parent additionally expands into 6 theme-variant siblings (default / cyberpunk / matrix / tron / industrial_forge / aurora_operator). Lock collision: visual-targets.json + visual-targets.schema.json are held by the active W15-A9 oracle harness lane. To avoid mutating their files mid-flight this PR introduces a new canonical superset path (visual-targets.canonical.*), which downstream lanes will adopt as the registry source of truth. Self-audit (all PASS): - 31/31 Images-GUI PNGs present in registry - all 15 required schema fields populated on every parent + region - 37 parents >= 31 baseline - no secrets Sources cited: - JSON Schema 2020-12 (https://json-schema.org/draft/2020-12/schema) - Storybook stories.json + Chromatic baselines.json conventions Hermes locks: claude-w15-a8-registry on the 3 new files (released on PR open).
📝 WalkthroughWalkthroughThis PR establishes comprehensive visual regression test infrastructure for the Hermes3D GUI. It introduces a JSON Schema defining the canonical visual-targets registry contract, populates a large registry mapping 31 PNG GUI assets to Playwright proof screenshots with backend truth sources, and documents the source asset inventory with metadata on dimensions and collage regions. ChangesVisual Test Target Registry
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@03_implementation/docs/handoffs/W15_A2_IMAGES_GUI_INVENTORY_2026-05-10.md`:
- Line 48: Update the collage row entry for
`09-themes/theme-variants-reference.png` to use the canonical theme IDs instead
of the short forms; replace `forge` with `industrial_forge` and `aurora` with
`aurora_operator` so the row reads the same canonical IDs used elsewhere (e.g.,
use `industrial_forge/aurora_operator` in place of `forge/aurora`) to prevent
downstream mapping mismatches.
In `@03_implementation/ui/tests/visual/visual-targets.canonical.json`:
- Line 9: The top-level "themes" array is missing "dark" which many targets
reference via "theme":"dark"; update the top-level themes list to include "dark"
so metadata matches target entries and downstream consumers see a consistent
contract (locate the top-level "themes" key in the canonical visual-targets JSON
and add "dark" to the array).
In `@03_implementation/ui/tests/visual/visual-targets.canonical.schema.json`:
- Around line 191-200: The schema's conditional rules under the allOf array
enforce parent_target_id for class "region" but not for class "theme_variant",
allowing orphaned theme variants; add a new conditional object mirroring the
"region" rule: an if that checks { "properties": { "class": { "const":
"theme_variant" } }, "required": ["class"] } and a then that adds { "required":
["parent_target_id"] } so that any target with "class": "theme_variant" must
include parent_target_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5729475-749e-4c27-b401-26be0fd60aa1
📒 Files selected for processing (3)
03_implementation/docs/handoffs/W15_A2_IMAGES_GUI_INVENTORY_2026-05-10.md03_implementation/ui/tests/visual/visual-targets.canonical.json03_implementation/ui/tests/visual/visual-targets.canonical.schema.json
| | `07-plugins-skills-mcp/plugins-skills-mcp-app-connectors.png` | 4 | | ||
| | `08-app-utility-pages/workflow-printqueue-files-logs.png` | 4 | | ||
| | `08-app-utility-pages/proof-health-notifications-safety.png` | 4 | | ||
| | `09-themes/theme-variants-reference.png` | 6 (default/cyberpunk/matrix/tron/forge/aurora) | |
There was a problem hiding this comment.
Use canonical theme IDs in the collage row for consistency.
This line uses forge/aurora, while the canonical IDs used elsewhere are industrial_forge/aurora_operator. Keeping these identical avoids downstream mapping mistakes.
✏️ Suggested doc fix
-| `09-themes/theme-variants-reference.png` | 6 (default/cyberpunk/matrix/tron/forge/aurora) |
+| `09-themes/theme-variants-reference.png` | 6 (default/cyberpunk/matrix/tron/industrial_forge/aurora_operator) |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `09-themes/theme-variants-reference.png` | 6 (default/cyberpunk/matrix/tron/forge/aurora) | | |
| | `09-themes/theme-variants-reference.png` | 6 (default/cyberpunk/matrix/tron/industrial_forge/aurora_operator) | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@03_implementation/docs/handoffs/W15_A2_IMAGES_GUI_INVENTORY_2026-05-10.md` at
line 48, Update the collage row entry for
`09-themes/theme-variants-reference.png` to use the canonical theme IDs instead
of the short forms; replace `forge` with `industrial_forge` and `aurora` with
`aurora_operator` so the row reads the same canonical IDs used elsewhere (e.g.,
use `industrial_forge/aurora_operator` in place of `forge/aurora`) to prevent
downstream mapping mismatches.
| "tolerance_default": 0.1, | ||
| "reference_viewport": "1536x1024", | ||
| "viewport": { "width": 1536, "height": 1024 }, | ||
| "themes": ["default", "cyberpunk", "matrix", "tron", "industrial_forge", "aurora_operator"], |
There was a problem hiding this comment.
Top-level themes is inconsistent with actual target data.
themes omits dark, but many targets use "theme": "dark" (starting at Line 18). This creates a contract mismatch for downstream consumers that trust top-level metadata.
🧩 Minimal consistency fix
- "themes": ["default", "cyberpunk", "matrix", "tron", "industrial_forge", "aurora_operator"],
+ "themes": ["dark", "default", "cyberpunk", "matrix", "tron", "industrial_forge", "aurora_operator"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "themes": ["default", "cyberpunk", "matrix", "tron", "industrial_forge", "aurora_operator"], | |
| "themes": ["dark", "default", "cyberpunk", "matrix", "tron", "industrial_forge", "aurora_operator"], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@03_implementation/ui/tests/visual/visual-targets.canonical.json` at line 9,
The top-level "themes" array is missing "dark" which many targets reference via
"theme":"dark"; update the top-level themes list to include "dark" so metadata
matches target entries and downstream consumers see a consistent contract
(locate the top-level "themes" key in the canonical visual-targets JSON and add
"dark" to the array).
| "allOf": [ | ||
| { | ||
| "if": { "properties": { "class": { "const": "collage" } }, "required": ["class"] }, | ||
| "then": { "required": ["regions"], "properties": { "regions": { "minItems": 1 } } } | ||
| }, | ||
| { | ||
| "if": { "properties": { "class": { "const": "region" } }, "required": ["class"] }, | ||
| "then": { "required": ["parent_target_id"] } | ||
| } | ||
| ] |
There was a problem hiding this comment.
Require parent_target_id for theme_variant targets as well.
theme_variant currently has no conditional requirement for parent_target_id, so orphaned variants can pass validation even though linkage is part of the model.
🛠️ Suggested schema update
"allOf": [
{
"if": { "properties": { "class": { "const": "collage" } }, "required": ["class"] },
"then": { "required": ["regions"], "properties": { "regions": { "minItems": 1 } } }
},
{
"if": { "properties": { "class": { "const": "region" } }, "required": ["class"] },
"then": { "required": ["parent_target_id"] }
+ },
+ {
+ "if": { "properties": { "class": { "const": "theme_variant" } }, "required": ["class"] },
+ "then": { "required": ["parent_target_id"] }
}
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "allOf": [ | |
| { | |
| "if": { "properties": { "class": { "const": "collage" } }, "required": ["class"] }, | |
| "then": { "required": ["regions"], "properties": { "regions": { "minItems": 1 } } } | |
| }, | |
| { | |
| "if": { "properties": { "class": { "const": "region" } }, "required": ["class"] }, | |
| "then": { "required": ["parent_target_id"] } | |
| } | |
| ] | |
| "allOf": [ | |
| { | |
| "if": { "properties": { "class": { "const": "collage" } }, "required": ["class"] }, | |
| "then": { "required": ["regions"], "properties": { "regions": { "minItems": 1 } } } | |
| }, | |
| { | |
| "if": { "properties": { "class": { "const": "region" } }, "required": ["class"] }, | |
| "then": { "required": ["parent_target_id"] } | |
| }, | |
| { | |
| "if": { "properties": { "class": { "const": "theme_variant" } }, "required": ["class"] }, | |
| "then": { "required": ["parent_target_id"] } | |
| } | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@03_implementation/ui/tests/visual/visual-targets.canonical.schema.json`
around lines 191 - 200, The schema's conditional rules under the allOf array
enforce parent_target_id for class "region" but not for class "theme_variant",
allowing orphaned theme variants; add a new conditional object mirroring the
"region" rule: an if that checks { "properties": { "class": { "const":
"theme_variant" } }, "required": ["class"] } and a then that adds { "required":
["parent_target_id"] } so that any target with "class": "theme_variant" must
include parent_target_id.
There was a problem hiding this comment.
Code Review
This pull request establishes a canonical visual target registry for the Hermes3D project, introducing a detailed image inventory, a JSON registry mapping images to UI routes and states, and a supporting JSON schema. The review feedback focuses on improving the robustness of the visual testing harness by suggesting the addition of wait_test_id to region definitions to ensure deterministic loading before capturing screenshots. Additionally, the reviewer recommended updating the schema to require parent_target_id for theme_variant classes to maintain structural consistency across the registry.
| "owning_pr": { "type": ["string", "null"] }, | ||
| "final_verdict": { "$ref": "#/$defs/finalVerdict" }, | ||
| "live_or_future": { "$ref": "#/$defs/liveOrFuture" }, | ||
| "notes": { "type": "string" } |
There was a problem hiding this comment.
The region definition is missing the wait_test_id property. Since many regions in the registry point to specific sub-routes (e.g., /#design or /#settings:providers), they require deterministic wait conditions to ensure the UI is stable before a screenshot is captured for comparison. Adding this field allows the visual testing harness to handle these sub-routes reliably.
"wait_test_id": { "type": "string" },
"notes": { "type": "string" }| { | ||
| "if": { "properties": { "class": { "const": "region" } }, "required": ["class"] }, | ||
| "then": { "required": ["parent_target_id"] } | ||
| } |
There was a problem hiding this comment.
Targets with the theme_variant class should also be required to provide a parent_target_id, as they are logically derived from a parent collage (like 09_theme_variants_reference). This ensures structural consistency across the registry.
| { | |
| "if": { "properties": { "class": { "const": "region" } }, "required": ["class"] }, | |
| "then": { "required": ["parent_target_id"] } | |
| } | |
| { | |
| "if": { "properties": { "class": { "enum": ["region", "theme_variant"] } }, "required": ["class"] }, | |
| "then": { "required": ["parent_target_id"] } | |
| } |
| "live_or_future": "live" | ||
| }, |
There was a problem hiding this comment.
Regions that point to distinct logical routes (like /#autopilot, /#design, etc.) should include a wait_test_id. This allows the visual proof harness to wait for the specific root element of that sub-page when navigating directly to it for verification. Please apply this pattern to all regions in the collage targets (02_primary_*, 03_settings_*, etc.) where the sub-route requires a specific loading check.
"live_or_future": "live",
"wait_test_id": "autopilot-root"
},
Summary
W15 Agent 8 — Visual Target Registry Builder. Creates the canonical visual-targets registry covering all 31 Images-GUI PNGs with full per-target schema. No UI/source code edits — manifest + schema + W15-A2 inventory handoff only.
03_implementation/ui/tests/visual/visual-targets.canonical.json— 37 parent targets (20 single + 11 collage parents + 6 theme variants) plus 103 region children, 140 total tracked targets03_implementation/ui/tests/visual/visual-targets.canonical.schema.json— JSON Schema 2020-12 (single / collage / region / theme_variant / user_direction classes; conditional region+parent requirements)03_implementation/docs/handoffs/W15_A2_IMAGES_GUI_INVENTORY_2026-05-10.md— inventory handoff reconstructed from filesystem scan (the A2-authored variant was not present onorigin/developat A8 dispatch time)Per-target schema (15 required fields)
target_id,image_path,dimensions,route,viewport,theme,ui_state,crop_region,backend_truth_source,playwright_proof_screenshot,pass_fail_status,owning_pr,final_verdict,class,live_or_future.Collage parents additionally carry a
regions[]array of clip-coordinate children. Theme-variant siblings carryparent_target_id.Viewport outliers handled
Theme palettes (6)
default,cyberpunk,matrix,tron,industrial_forge,aurora_operator— collage parent at09_theme_variants_referenceexpands into 6 region children and 6 theme-variant sibling targets at/#dashboard?theme=<name>.Lock collision note
visual-targets.json(owned byclaude-w14-pr1-harnessuntil 22:13:06Z) andvisual-targets.schema.json(owned byclaude-w15-a9-oracleuntil 22:38:30Z) were both actively locked at A8 dispatch time. To avoid clobbering the W15-A9 oracle harness mid-flight, this PR introduces a new canonical superset path (visual-targets.canonical.*) which downstream lanes will adopt. The W6-6visual-targets.jsonis left untouched.Self-audit (4/4 PASS)
Additionally validated by
jsonschema.Draft202012Validatoragainst the embedded schema: SCHEMA VALIDATION: PASS.Sources
stories.json+ Chromaticbaselines.jsonconventions (parent / region / variant separation with deterministic clip coordinates)Hermes MCP locks
Owner
claude-w15-a8-registryon:03_implementation/ui/tests/visual/visual-targets.canonical.json03_implementation/ui/tests/visual/visual-targets.canonical.schema.json03_implementation/docs/handoffs/W15_A2_IMAGES_GUI_INVENTORY_2026-05-10.mdReleased on PR open.
Test plan
image_pathin the registrydumps())jsonschema🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores