Skip to content

Commit 3c6fd86

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
feat(design): add conformance check and local Studio
1 parent 52cc796 commit 3c6fd86

93 files changed

Lines changed: 28494 additions & 229 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.agents/skills/babysit/SKILL.md‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ All three must hold:
5757
reported yet, and treating "not failing" as "passing" reports the PR clean before CI has
5858
had its say. Wait for it — the step-10 stop condition covers a check that never settles.
5959

60+
A passing design-conformance CI step can still contain warnings. Read its latest report and
61+
triage findings using `/ship`'s [committed design check](../ship/SKILL.md#committed-design-check).
62+
Intentional system changes and justified exceptions may remain once explained in the PR;
63+
they do not prevent a clean review or require another fix loop. Honor decisions already made
64+
in this session. Operational failures must be resolved before reporting the PR clean.
65+
6066
Do not stop early on "no new comments this round" alone — a thread can be open from an earlier
6167
round, and cubic often lands its first threads a round after Greptile's. Always check all three
6268
conditions freshly after every push.
@@ -85,11 +91,14 @@ conditions freshly after every push.
8591
If `mergeable` is `CONFLICTING`, fix that first (step 2). If a check is failing, fix that too
8692
— treat it exactly like a review finding. If a check is still `pending`, do not evaluate
8793
"clean" at all: go to step 9 and wait for it. Otherwise, if Greptile is 5/5, every thread
88-
across all pages has `isResolved: true`, and every check has finished and passed, stop —
94+
across all pages has `isResolved: true`, every check has finished and passed, and any design
95+
warnings have been triaged as above, stop —
8996
report the outcome (see "Reporting" below) and skip the rest of this list.
9097

9198
2. **If the PR has a merge conflict**, merge `origin/staging`, resolve the conflicts, run the
92-
usual pre-push checks, push, and go to step 8 to re-trigger review.
99+
usual pre-commit checks and commit the resolution. Run `/ship`'s
100+
[committed design check](../ship/SKILL.md#committed-design-check) against the resulting HEAD
101+
before pushing, then go to step 8 to re-trigger review.
93102

94103
3. **If no review has run yet** (fresh PR, no bot comments): both run automatically on PR open —
95104
confirm via `gh pr checks <n>` (look for `Greptile Review` and `cubic · AI code reviewer`) and
@@ -129,7 +138,11 @@ conditions freshly after every push.
129138
migration safety, and the regenerate + audit phases. A review-fix round is still a code change
130139
and can trip any of them just as easily as the original commit did.
131140

132-
7. **Commit and push** the round's fixes as one commit — `--force-with-lease` whenever step 6's
141+
7. **Commit, check and push** the round's fixes as one commit. After committing and before
142+
every push, follow `/ship`'s [committed design check](../ship/SKILL.md#committed-design-check),
143+
including warning triage and committing/rechecking any resulting fixes. Push only the
144+
checked HEAD; rerun after a rebase or any other change to the comparison.
145+
Use `--force-with-lease` whenever step 6's
133146
sync check rewrote history, which includes a plain `git rebase origin/staging` that completed
134147
with no conflicts, not only the cherry-pick rebuild path; both rewrite commits already
135148
published to the remote, so a plain `git push` can be rejected either way — then run `/ship`
Lines changed: 9 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,19 @@
11
---
22
name: emcn-design-review
3-
description: Review UI code for alignment with the emcn design system — components, tokens, patterns, and conventions
3+
description: Review product UI changes for design drift using the local conformance check, EMCN components, and global styles.
44
argument-hint: "[scope] [fix=true|false]"
55
---
66

7-
# EMCN Design Review
8-
9-
Arguments:
10-
- scope: what to review (default: your current changes). Examples: "diff to main", "PR #123", "src/components/", "whole codebase"
11-
- fix: whether to apply fixes (default: true). Set to false to only propose changes.
7+
# EMCN design review
128

139
User arguments: $ARGUMENTS
1410

15-
## Context
16-
17-
This codebase uses **emcn**, a custom component library built on Radix UI primitives with CVA variants and CSS variable design tokens. All UI must use emcn components and tokens.
18-
19-
## Steps
20-
21-
1. Read the emcn public barrel at `packages/emcn/src/index.ts` (re-exports components, Calendar, Table*, and icons) to know what's available; for the full icon set read `packages/emcn/src/icons/index.ts`
22-
2. Read `apps/sim/app/_styles/globals.css` for CSS variable tokens
23-
3. Analyze the specified scope against every rule below
24-
4. If fix=true, apply the fixes. If fix=false, propose the fixes without applying.
25-
26-
---
27-
28-
## Imports
29-
30-
- Components, `cn`, and tokens from the `@sim/emcn` barrel, never component subpaths
31-
- Icons from `@sim/emcn/icons`
32-
33-
## Design Tokens
34-
35-
Use CSS variable pattern (`text-[var(--text-primary)]`), never Tailwind semantics (`text-muted-foreground`) or hardcoded colors (`text-gray-500`, `#333`).
36-
37-
**Text**: `--text-primary`, `--text-secondary`, `--text-tertiary`, `--text-muted`, `--text-body` (canonical value text), `--text-icon`, `--text-placeholder`, `--text-subtle`, `--text-inverse`, `--text-error`
38-
**Surfaces**: `--bg`, `--surface-1` through `--surface-7`, `--surface-hover`, `--surface-active`
39-
**Borders**: `--border` (`--border-1`/`--border-muted` are legacy aliases resolving to it — flag new uses)
40-
**Brand/accent**: `--brand-secondary`, `--brand-accent`
41-
**Z-Index**: `--z-dropdown` (100), `--z-toast` (150), `--z-modal` (200), `--z-popover` (300), `--z-tooltip` (400), `--z-takeover` (500), `--z-shell-gate` (600)
42-
**Shadows**: `shadow-subtle`, `shadow-medium`, `shadow-overlay`, `shadow-card`
43-
**Badges**: `--badge-*` semantic families (success/error/gray/blue/purple/orange/amber/teal/cyan/pink, each with `-bg`/`-text`)
44-
45-
## Buttons
46-
47-
Intent-to-variant mapping (read the actual `buttonVariants` in `packages/emcn/src/components/button/button.tsx` for the full variant set — it exposes more than listed here):
48-
49-
| Action | Variant |
50-
|--------|---------|
51-
| Toolbar, icon-only | `ghost` |
52-
| Create, save, submit | `primary` |
53-
| Cancel, close | `default` |
54-
| Delete, remove | `destructive` |
55-
| Selected state | `active` |
56-
| Toggle | `outline` |
57-
58-
## Delete/Remove Confirmations
59-
60-
`ChipModal` `size='sm'`, title "Delete/Remove {ItemType}", destructive confirm button, plain Cancel (follow the chip footer layout in `.claude/rules/emcn-components.md`). Use `text-[var(--text-error)]` for irreversible warnings.
61-
62-
## Toast
63-
64-
`toast.success()`, `toast.error()`, `toast()` from `@sim/emcn`. Never custom notification UI.
65-
66-
## Badges
67-
68-
`red`=error/failed, `gray-secondary`=metadata/roles, `type`=type annotations, `green`=success/active, `gray`=neutral, `amber`=processing, `orange`=paused, `blue`=info. Use `dot` prop for status indicators.
69-
70-
## Icons
71-
72-
Default: `size-[14px]`. Color: `text-[var(--text-icon)]`. Scale: 14px > 16px > 12px > 20px. Use the `size-*` shorthand — flag `h-[Npx] w-[Npx]` and `h-N w-N` pairs as refactor targets.
11+
Interpret the arguments as the product UI scope (default: current changes) and an optional `fix=true|false` mode (default: `false`). When `fix=false`, explain proposed changes without applying them.
7312

74-
## Anti-patterns to flag
13+
1. When EMCN, global styles, recipes or design ownership metadata change, run the diff check in step 2. It derives facts from each source revision, so the originating design-system change remains visible without a committed metadata file.
14+
2. During UI work, run `bun run check:design --base origin/staging --working-tree` from the repo root, substituting the actual PR target for `origin/staging`. After committing, use `--head HEAD` for the immutable PR comparison. Exit 1 means findings to review; exit 2 means the check failed and must be repaired or reported. CI is warning-only for findings and fails on incomplete analysis.
15+
3. For each new finding, inspect the cited source, the applicable public EMCN export in `packages/emcn/src/index.ts`, and tokens and recipes in `apps/sim/app/_styles/globals.css`. Reuse a suitable component, prop, variant, or global token when it expresses the design intent. Avoid near-duplicate local colours or overriding EMCN chrome merely for convenience.
16+
4. A genuinely new product treatment may remain an Extra. Explain its visual intent and why existing EMCN or global styling does not fit in the PR. The check does not decide design approval and must not be silenced by adding an arbitrary token, broad exclusion, or fake component wrapper. Ask the designer or engineer when changing a shared recipe would have broad or ambiguous effects.
17+
5. Keep unresolved `unchecked` inputs and inspection failures separate from findings. A quiet diff means no *new detected* debt, not proof of complete visual conformance. Existing debt stays quiet; a new copy can warn. Landing and docs are out of product scope; Monaco presentation, provider branding, and block identity palettes have deliberate exclusions. See `scripts/design-conformance/README.md` for exact rule boundaries.
7518

76-
- Raw `<button>`/`<input>`, or legacy `Input`/`Textarea`/`Modal`, instead of the canonical chip components (`ChipInput`/`ChipTextarea`/`ChipModal`)
77-
- Hand-rolled field rows inside a `ChipModalBody` instead of `ChipModalField`
78-
- Hardcoded colors (`text-gray-*`, `#hex`, `rgb()`)
79-
- Tailwind semantics (`text-muted-foreground`) instead of CSS variables
80-
- Template literal className instead of `cn()`
81-
- Inline styles for colors/static values (dynamic values OK)
82-
- Importing from emcn subpaths instead of barrel
83-
- Arbitrary z-index instead of tokens
84-
- Wrong button variant for action type
19+
Do not turn this review into an unrelated whole-codebase cleanup. Preserve intended appearance when migrating product UI and use before/after screenshots when a treatment changes.

‎.agents/skills/ship/SKILL.md‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
name: ship
3-
description: Commit, push, and open a PR to staging in one shot — runs the cleanup pass and, when migrations changed, the db-migrate safety review first
3+
description: Commit, check design conformance, push, and open a PR to staging — runs cleanup and the applicable migration safety review first
44
argument-hint: "[optional context or scope notes]"
55
---
66

@@ -82,7 +82,7 @@ When the user runs `/ship`:
8282
bun run docs-manifest:check || { echo "❌ docs manifest out of sync — do not ship"; exit 1; }
8383
```
8484
If Phase A regenerated a file, its matching `:check` in Phase B now passes trivially — that parity is the point. Do not ship with any generator or audit failing; fix the cause (never silence it) and re-run. `check:migrations` and `type-check` are covered by steps 5 and CI respectively and are not repeated here.
85-
7. **Stage and commit** the changes with the generated message — including any files Phase A regenerated in step 6
85+
7. **Stage and commit** the changes with the generated message — including any files Phase A regenerated in step 6. Then run the [committed design check](#committed-design-check) below and resolve or explain its findings before step 8.
8686
8. **Push to origin** using the current branch name — `--force-with-lease` if step 2's sync
8787
check did any history rewrite (a clean rebase or a cherry-pick rebuild) on a branch that had
8888
already been pushed once; a plain push would be rejected in exactly the polluted-remote case
@@ -98,7 +98,33 @@ When the user runs `/ship`:
9898
positional/line-by-line comparison against the PR's oldest-first list can spuriously fail on
9999
any multi-commit branch. These two lists must describe the same commits in the same order
100100
(same subjects, the last one being the commit from step 7). If they don't match, the branch
101-
still has a problem — redo step 2's fix and `git push --force-with-lease`.
101+
still has a problem — redo step 2's fix, repeat the committed design check for the resulting HEAD, and `git push --force-with-lease`.
102+
103+
## Committed design check
104+
105+
When central EMCN sources, global styles, recipes or `packages/emcn/src/design-ownership.json` change, run the design diff check and review its central-system findings. The checker derives metadata independently from the base and proposed source; no generated artifact needs to be committed.
106+
107+
During product UI work, run `bun run check:design --base origin/staging --working-tree` so staged, unstaged and nonignored new files are included. Review findings against EMCN and `globals.css`; explain intentional new Extras rather than weakening the checker. After committing and before **every push**, run this from the repository root with the repository-pinned Bun version:
108+
109+
```bash
110+
bun run check:design --base origin/staging --head HEAD
111+
```
112+
113+
Run it for every `/ship`; let the checker apply its own scope. A `.tsx`-only condition would miss CSS, Tailwind configuration, artwork and contract-registry changes. `check:audits` deliberately excludes this base-dependent command. It reads committed merge-base → HEAD blobs, so a run before committing cannot validate the pending changes.
114+
115+
Interpret both the exit status and the report:
116+
117+
- **0 with a completed report:** no findings; continue to push.
118+
- **1 with a completed report:** read the usage violations and central-system notifications. Triage them before pushing; findings are warnings, not an automatic shipping failure.
119+
- **2, unexpected termination, or no completed report:** the check did not complete. Fix the operational problem and rerun before pushing. A startup failure with exit 1 is not a findings report. Do not hide failures with `|| true` or treat missing output as a pass.
120+
121+
Fix straightforward usage violations through the cited central component, prop, recipe or token. For a small local gray correction, choose the approved token appropriate to its role. Do not invent a new token or loosen a contract just to remove the warning. Keep fixes within the work being shipped; unchanged debt elsewhere can wait for its own cleanup.
122+
123+
When a change has broad shared impact or ambiguous intent, explain the finding and ask the engineer how to proceed. Intentional central-system changes and justified exceptions can proceed with an explanation in the PR; involve the designer for new standards or ambiguous broad changes. Honor decisions already given in this session. Retain the warning rather than weakening the linter or requiring every intended system change to produce a clean report.
124+
125+
If this review produces edits, rerun the affected generation/lint/audit checks from step 6, commit the fixes, then repeat the design check against the new HEAD. Also rerun after a rebase, conflict resolution or other change to the comparison. Push only the checked commit; uncommitted fixes are not covered by an earlier result.
126+
127+
In the PR's **Testing** section, record the design-check outcome and explain any retained warnings. Leave **No new warnings introduced** unchecked when warnings remain. This review is about central design-system conformance; approved component variants, colours and fonts remain available for the engineer's product decisions. See `scripts/design-conformance/README.md` for scope and known unchecked inputs.
102128
103129
## Commit Message Format
104130
@@ -154,7 +180,7 @@ Describe the checks, tests, and E2E artifacts run
154180
- [x] Code follows project style guidelines
155181
- [x] Self-reviewed my changes
156182
- [ ] Tests added/updated and passing (new tests pass the `test-audit` authoring gate)
157-
- [x] No new warnings introduced
183+
- [ ] No new warnings introduced
158184
- [x] I confirm that I have read and agree to the terms outlined in the [Contributor License Agreement (CLA)](./CONTRIBUTING.md#contributor-license-agreement-cla)
159185
```
160186

0 commit comments

Comments
 (0)