| name | review-design |
| description | Review frontend/design changes for klass-hero by spawning the design-system-guardian (static code: tokens, duplication, lint_typography + credo) and ui-critic (rendered: hierarchy, spacing, contrast, accessibility, mobile) agents in parallel, then consolidating into a unified report. Use when: reviewing a PR that touches LiveViews/components/templates/CSS, checking UI or design quality before merge, validating frontend changes, or after editing anything under lib/klass_hero_web/ or assets/css/. Invoke with: /review-design |
Review Design
Run a comprehensive frontend/design review by dispatching two specialized agents in
parallel — one static (code), one rendered (browser) — then consolidating their reports.
Type: Rigid workflow. Follow steps exactly. Do not improvise a single manual review
in place of the agents — spawning both is the entire point (see Rules).
This skill reviews the visual quality and design-system consistency of a change. Flow /
journey friction is out of scope here — that's product-flow-designer, run standalone.
Step 1: Determine Changed Frontend Files
Detect the review scope relative to the base branch, including unstaged and untracked work.
BASE=$(git merge-base HEAD main 2>/dev/null || echo "main")
git diff --name-only "$BASE"...HEAD -- 'lib/klass_hero_web/**/*.ex' 'lib/klass_hero_web/**/*.heex' 'assets/css/app.css'
git diff --name-only -- 'lib/klass_hero_web/**/*.ex' 'lib/klass_hero_web/**/*.heex' 'assets/css/app.css'
git ls-files --others --exclude-standard -- 'lib/klass_hero_web/**/*.ex' 'lib/klass_hero_web/**/*.heex' 'assets/css/app.css'
Union the three lists. Keep only front-end files: anything under lib/klass_hero_web/
(LiveViews, components, templates) and assets/css/app.css.
If the union is empty, STOP:
No frontend files changed. Nothing to review.
(/review-architecture covers backend .ex; this skill is front-end only.)
Step 2: Map Changes to Affected Routes
The guardian works on the file list directly. The critic is route-scoped — it needs URLs
to visit, not files. Derive them:
- Changed LiveView module → its route(s). Grep
lib/klass_hero_web/router.ex for the module.
- Changed component → grep for the component's usages → the LiveView(s) that render it →
their route(s).
- Changed
app.css → pick a few representative routes that exercise the affected styles.
Also note the role each route needs (parent / provider / admin) so the critic logs in with
the right seed user. Display:
Affected routes: /provider/dashboard (provider), /dashboard (parent)
If a changed component has no on-screen consumer yet (dead/new code), say so — the critic can't
render what isn't wired up; the guardian still reviews it statically.
Step 3: Server Preflight (for the rendered lens)
The critic must drive the running app. Probe http://localhost:4000 (and one real route
like /programs) — the server must be healthy, not merely listening. A stale process
returning 5xx counts as down.
- Healthy (2xx/3xx): proceed with both agents.
- Down or erroring (no response, or 5xx): ask the user to (re)start it (
mix phx.server).
If they decline, proceed with the guardian only and mark the report
rendered lens skipped — server unavailable. Never let the critic fabricate a review from
code; it STOPs without a healthy server by design.
Step 4: Spawn Both Agents in Parallel
Launch both in the same message so they run concurrently.
Agent 1: Design System Guardian (static lens)
Use the design-system-guardian subagent. Provide:
Review these changed frontend files for design-system consistency.
Scope: changed-files
Changed files:
<file list>
Run all 8 checks scoped to these files. Run mix lint_typography and mix credo for ground
truth. Report findings in your standard output format.
Agent 2: UI Critic (rendered lens)
Use the ui-critic subagent. Provide:
Review the rendered UI for these screens.
Routes: <affected routes, with the role to log in as for each>
Server: http://localhost:4000 (confirmed up)
Walk each route at 375/768/1280 and the relevant interaction states. Run all 10 checks.
Report findings in your standard output format.
(If the server was unavailable and the user declined to start it, skip Agent 2 and note it.)
Step 5: Consolidate Reports
Merge into one report. The two agents are designed with disjoint lanes, so overlap is small —
but reconcile these:
Deduplication
- Typography — guardian flags raw
font-display / wrong atom from code (with file:line
- linter); critic flags type-scale / hierarchy from the render. If it's the same element,
keep the guardian's framing (it has the file + linter evidence) and fold in the critic's
visual note.
- Spacing / consistency — guardian = off-scale token in code; critic = visual rhythm on
screen. Same element → merge into one finding.
- Everything else is single-lens: contrast / focus / tap-targets / viewport → critic only;
duplication / token gaps / variant logic / credo → guardian only.
Unified Severity
The agents use different ladders. Map both onto one:
| Unified | ui-critic | design-system-guardian |
|---|
| Blocker | Blocker | Must-fix (breaks precommit/CI) |
| Major | Major | Should-fix |
| Minor | Minor | (lesser Should-fix) |
| Nit / Opportunity | Nit | Opportunity |
Unified Report Format
# Design Review Report
**Branch:** [branch name]
**Affected routes:** [list] **Files reviewed:** [count]
**Rendered lens:** ran @375/768/1280 | skipped — server unavailable
## Summary
| Source | Checks | Blocker | Major | Minor | Nit/Opp |
|--------|--------|---------|-------|-------|---------|
| Static (design-system-guardian) | 8 | N | N | N | N |
| Rendered (ui-critic) | 10 | N | N | N | N |
| **Total** | **18** | **N** | **N** | **N** | **N** |
Linters: lint_typography PASS|FAIL (n) · credo n design-relevant findings
## Findings
(grouped Blocker → Major → Minor → Nit/Opp; omit empty)
### [Blocker] <title>
- **Where:** <route / file:line / element>
- **Issue:** what is wrong (note lens: static | rendered)
- **Fix:** specific — name the Theme atom / token / component, or the design change
## Passed / clean
- [checks that returned nothing]
Order by severity, then by file/route.
Step 6: Present and Advise
- Zero findings: confirm the change is visually and design-system sound.
- Only Minor/Nit: note them, confirm the change is acceptable.
- Blocker/Major: list the specific files/screens and changes needed; offer to fix.
- If the rendered lens was skipped, say so explicitly — the review is partial.
Rules
- Always spawn both agents in parallel in one message. Do NOT substitute an improvised
manual review for the agents — the failure mode this skill prevents is a single hand-rolled
pass that skips the browser and the linters.
- Both lenses or say which is missing. Static-only (server down + user declined) is allowed
but must be labelled partial. Never silently drop the rendered lens.
- Scope to changes. Guardian gets the changed-file list; critic gets the routes those
changes render on. Don't run full-codebase sweeps from this skill (run an agent standalone for
that).
- Run the linters for typography/credo (via the guardian) — don't eyeball them.
- Deduplicate across lenses. Same element flagged twice → one finding, guardian's framing for
code-level, critic's for rendered.
- Never auto-fix without confirmation. Present findings first, then offer to fix.
- Flow/journey friction is out of scope. That's
product-flow-designer, run separately.
- Include the full check count (18) even when everything passes, so the user knows what was verified.
