feat: design knowledge split with rendered design review#68
Open
svihra wants to merge 4 commits into
Open
Conversation
Split design guidance by discipline. DesignFrontend now owns distinctive and premium aesthetics (landing, marketing, brand); DesignDashboard owns data-dense product UI (dashboards, admin, analytics) with density, state completeness, chart-to-data-shape matching, and progressive disclosure. DesignPrinciples is a non-invocable reference holding the cross-cutting canon both skills cite: durable foundations, WCAG-anchored hard checks, and advisory thresholds tiered so only spec-defined values fail.
Plan for verifying built UI against the DesignPrinciples canon. Owns the review contract (screenshot + axe-core JSON per state/viewport, axe rule IDs mapped to principle checks) rather than a browser: capture is delegated to an existing tool such as a Playwright MCP. Records the dangers (false confidence via axe 'incomplete' or missing states, vision measurement leakage, non-determinism) and the not-verified reporting semantics that close them.
Implements ARCH-0015. Capture-agnostic reviewer: consumes a screenshot plus an axe-core JSON per state and viewport, sources hard checks only from axe (color-contrast, target-size mapped to DesignPrinciples), and reports axe 'incomplete' and absent states as not-verified rather than pass. Screenshots drive vision judgment (structure, anti-slop, state coverage) only. No artifacts means a no-op that reports the gap.
Playwright + axe-core captures of a seeded-defect dashboard exercised the full contract: axe-only hard verdicts, incomplete-as-not-verified, withheld states reported not-verified, no-artifact no-op, and emoji as smell-not-ban all held across three independent review runs. Two contract fixes from verification: target-size maps to WCAG 2.5.8 including its spacing exception (isolated small targets pass legitimately), and screenshot judgments get a dedicated judgment-fail verdict so verified-* stays reserved for axe-backed findings. tests/fixtures/design-review/ holds the validated artifact set with expected findings and the capture script to regenerate it.
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.
Problem
Design guidance in forge-core stops at aesthetics:
DesignFrontendcovers distinctive marketing/landing design, but nothing covers data-dense product UI (dashboards, admin, analytics), nothing holds the cross-cutting design canon in one authoritative place, and nothing can verify a built interface against that canon. Source-level review cannot do the last part: it is blind on CSS-in-JS and React Native, and no static scan computes rendered contrast.Approach
Split design knowledge by discipline and add rendered verification:
DesignPrinciples— non-invocable canon: durable framework-agnostic foundations, WCAG-anchored hard checks, advisory thresholds tiered so only spec-defined values fail. Cited by the other design skills.DesignDashboard— data-UI discipline: density, chart-to-data-shape matching, state completeness, progressive disclosure.DesignFrontend— rescoped to marketing/landing/brand-aesthetic surfaces; dashboards route toDesignDashboard.DesignReview— capture-agnostic reviewer per ARCH-0015: consumes screenshot + axe-core JSON per (state, viewport); hard verdicts come only from axe; axeincompleteand absent states report not-verified, never pass; screenshot judgments carry a separatejudgment-failverdict. Capture is delegated (e.g. Playwright MCP), never bundled — forge-core owns the contract, not a browser.Verified end-to-end with real Playwright + axe-core captures of a seeded-defect dashboard; the validated artifact set and expected findings live in
tests/fixtures/design-review/. Verification produced two contract fixes now included: WCAG 2.5.8's spacing exception (isolated small targets pass legitimately) and thejudgment-failverdict split.Out of scope
Bundled capture tooling, multi-theme review, auto-discovery of states, and interaction/video review (recorded in ARCH-0015).
Test plan
forge validategreen;forge assemblebuilds all five Design skillstarget-size, isolated small target passes via spacing exceptionincomplete→ not-verified, withheld states → not-verified, empty artifact dir → loud no-op, emoji flagged as smell-not-bantests/fixtures/design-review/expected-findings.md)DesignFrontend/DesignDashboardUSE WHEN split routes correctly🤖 Generated with Claude Code