From 1edab990bb465db1d0010646f09b39d23c08b0c3 Mon Sep 17 00:00:00 2001 From: Tom Huang <1043269994@qq.com> Date: Sat, 2 May 2026 11:00:33 +0800 Subject: [PATCH] feat(craft): add brand-agnostic craft references + Refero-derived lint rules (#225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(craft): add brand-agnostic craft references and refero-derived lint rules Introduce `craft/` as a third top-level content axis alongside `skills/` and `design-systems/`, holding universal (brand-agnostic) craft rules that apply on top of any DESIGN.md. Skills opt in via a new `od.craft.requires` front-matter array; the daemon resolves the slug list and injects the matching files between DESIGN.md and the skill body in the system prompt. Initial vendor (MIT, adapted from referodesign/refero_skill): typography craft, color craft, anti-ai-slop. Pilot wired on saas-landing. Extend the existing lint-artifact pass with two refero-derived rules: - P0 ai-default-indigo — solid #6366f1 / #4f46e5 / #4338ca / #8b5cf6 as accent (not just gradients) is the most-reported AI tell. - P1 all-caps-no-tracking — `text-transform: uppercase` rules without ≥0.06em letter-spacing. The craft loader silently drops missing files so a skill can forward-reference future sections (e.g. `motion`) without breaking. * fix(daemon): skip :root token blocks in ai-default-indigo lint The ai-default-indigo P0 check scanned the whole HTML for the raw hex, so brands that intentionally encode indigo as `--accent: #6366f1` in :root and consume it via var(--accent) downstream were flagged as AI-default — a false positive that forced the agent to "fix" valid output. Strip :root token-definition blocks (including attribute-selector theme variants) before scanning, mirroring the existing pattern used by the raw-hex P1 check. Hex still flagged when it appears in component rules or inline styles. * docs(craft): address PR #225 P3 review feedback - craft/README.md: explain why missing craft sections are silently dropped (forward-compatibility) instead of surfacing a warning. - craft/typography.md: ground the 0.06em ALL CAPS tracking floor in Bringhurst-derived typographic practice rather than presenting the threshold as unattributed. - craft/color.md: cover the edge case where a brand's DESIGN.md intentionally encodes indigo as --accent — `var(--accent)` uses remain unflagged because the linter only inspects hardcoded hex. - docs/skills-protocol.md: link the "missing files dropped silently" note back to craft/README.md for the canonical slug list and the rationale behind the choice. * fix(craft): address PR #225 P0 review feedback - tools/pack: copy `craft/` into the packaged resource root alongside `skills`, `design-systems`, and `frames`, so the `od.craft.requires` integration isn't a silent no-op when the daemon resolves `${OD_RESOURCE_ROOT}/craft` in packaged builds. - packages/contracts: add `craftRequires?: string[]` to `SkillSummary` (and therefore `SkillDetail`) so the field that `listSkills()` already returns and `/api/skills(/:id)` already serializes via `...rest` is part of the documented web/daemon contract instead of leaking through as an untyped property. - apps/daemon/lint-artifact: expand the indigo token-strip pass to cover selector lists containing `:root` (e.g. `:root, [data-theme="light"]`) and any rule whose body is custom-property-only (e.g. a `[data-theme="dark"] { --accent: ... }` theme variant). Real component rules with a hardcoded indigo are still preserved so the P0 finding still fires; tests cover the new selector-list and theme-variant cases. * fix(craft): address PR #225 follow-up review feedback - lint-artifact: scope the indigo token-strip to + + `; + const findings = lintArtifact(html); + const hit = findings.find((f) => f.id === 'ai-default-indigo'); + expect(hit).toBeDefined(); + expect(hit.severity).toBe('P0'); + }); + + it('flags solid #4f46e5 (indigo-600) too', () => { + const html = `
Hi
`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + // Regression: the AI_DEFAULT_INDIGO list used to omit `#3730a3` and + // `#a855f7` even though `craft/anti-ai-slop.md` documents both as + // P0-blocked solid accents. An artifact could hard-code one of these + // as a button fill and slip past the lint. The list now matches the + // craft doc exactly; these regression tests pin the contract. + it.each([ + ['#3730a3', 'tailwind indigo-800'], + ['#a855f7', 'tailwind purple-500'], + ['#7c3aed', 'tailwind violet-600'], + ])('flags solid %s (%s) as a documented cardinal-sin accent', (hex) => { + const html = `
Hi
`; + const findings = lintArtifact(html); + const hit = findings.find((f) => f.id === 'ai-default-indigo'); + expect(hit).toBeDefined(); + expect(hit.severity).toBe('P0'); + }); + + it('does not double-fire when purple-gradient already caught the same color', () => { + const html = `
Hi
`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'purple-gradient')).toBeDefined(); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag artifacts that use var(--accent) only', () => { + const html = ` + + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag indigo declared as a token in :root and consumed via var(--accent)', () => { + // Brand whose accent is intentionally indigo: defines #6366f1 once + // in :root and uses var(--accent) downstream. This is the design + // system speaking, not the model defaulting — must not fire P0. + const html = ` + + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('still flags indigo when it appears outside :root even if also defined as a token', () => { + // If the artifact both defines the accent AND hard-codes the same + // hex in a component rule, the component rule is still raw indigo + // — fire as before. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('does not flag indigo in :root with attribute selector (theme variants)', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag indigo declared in a selector list containing :root', () => { + // Theme CSS often pairs `:root` with an attribute selector via a + // selector list so the same tokens apply to both default and + // light-themed roots. Whichever side comes first, the block is a + // token definition and must not fire P0. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag indigo declared in a selector list with :root second', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag indigo declared in a custom-property-only theme block without :root', () => { + // Theme-variant blocks that omit `:root` entirely (e.g. only + // `[data-theme="dark"]`) are still token definitions when their + // body is custom-property-only; treat them the same way. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag a :root token block that also declares non-custom properties like color-scheme', () => { + // Regression: the strip pass used to run its rule-shaped regex + // against the full HTML string, so the first selector capture + // included the leading ` + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('still flags indigo laundered through a component-local custom property', () => { + // Regression: the custom-property-only exemption used to apply + // to *any* selector, so an agent could hide #6366f1 in a local + // var (e.g. `.cta { --cta-bg: #6366f1 }`) and the linter would + // strip the rule and miss the P0. The exemption is now scoped + // to global theme selectors (:root, html, [data-theme=...], …). + const html = ` + + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags a non-token :root declaration containing #6366f1', () => { + // Regression: the `:root` exemption used to be unconditional, so + // a rule whose body wasn't actually a token definition (e.g. + // `:root { background: #6366f1 }`) was stripped before the indigo + // scan and the P0 silently disappeared. The exemption now requires + // a token-shaped body, so a non-token `:root` declaration keeps + // its hex in scope and the lint still fires. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo when :root sits in a list with a component selector', () => { + // Regression: `:root, .cta { --cta-bg: #6366f1 }` used to be + // exempted because the selector list contained `:root`, even + // though `.cta` is a component selector. The exemption now + // requires every selector in the list to be a global theme + // scope, so this mixed list is preserved and the P0 still fires. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on a bare component-attribute selector', () => { + // Regression: the bare-attribute branch of the global-theme-scope + // test used to accept ANY attribute selector (e.g. + // `[data-variant="primary"]`), so a custom-property-only rule on + // a component/state attribute was treated as a global token block + // and the indigo lint silently disappeared. The exemption now + // requires the attribute name to be one of the known global-theme + // switches (`data-theme`, `data-color-scheme`, `data-mode`). + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on a bare aria-state attribute selector', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on a :root prefixed with a component-attribute selector', () => { + // Regression: `:root[data-variant="primary"]` used to be exempted + // because the regex only checked the tag prefix and not the + // attribute name. A component/state attribute attached to `:root` + // is exactly the laundering pattern this lint must catch — the + // exemption now requires the attribute (when present) to name a + // known global-theme switch. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on an html prefixed with an aria-state attribute selector', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on a body prefixed with a component-attribute selector', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still exempts indigo on :root prefixed with the canonical data-theme switch', () => { + // Sanity check: the prefixed-attribute change must keep exempting + // legitimate theme-switch selectors (`:root[data-theme="dark"]`), + // even though the prefixed-form regex changed shape. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('still exempts indigo on html and body prefixed with data-theme', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('still exempts indigo on a bare data-color-scheme theme block', () => { + // The bare-attribute exemption still covers the canonical + // global-theme switches; a token block keyed off + // `[data-color-scheme="dark"]` is a theme variant, not a + // component-local rule, and must not fire. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag a :root token block whose body contains CSS comments', () => { + // Regression: `stripTokenBlocksFromCss` used to split the body on + // `;` and run `isTokenShapedDeclaration` from the start of each + // fragment. A common token block such as + // `:root { /* brand accent */ --accent: #6366f1; }` produced a + // declaration fragment beginning with the comment, failed the + // token-shape test, and the rule was left in scope of the + // indigo scan — a false P0 on a legitimate token definition. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag a :root token block with a trailing CSS comment', () => { + const html = ` + + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag a :root token block with a comment between declarations', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag indigo declared in a :root token block nested inside @media', () => { + // Regression: `stripTokenBlocksFromCss` only matched flat + // `selector { body }` rules, so a media-query-wrapped token block + // like `@media (prefers-color-scheme: dark) { :root { --accent: #6366f1 } }` + // had its outer `@media` rule treated as the selector/body pair and + // the inner `:root` token block was never stripped — producing a + // P0 false positive on legitimate responsive theme CSS. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('does not flag indigo declared in a :root token block nested inside @supports', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('still flags indigo declared on a non-accent global token feeding a CTA', () => { + // Regression: the strip pass used to remove every custom-property-only + // global theme block, even when the indigo hid behind a non-`--accent` + // token like `--primary` or `--button-bg`. The craft contract's escape + // hatch is `--accent` specifically — encoding indigo as any other + // token name still launders the LLM-default color, so the rule must + // stay in scope of the indigo scan. + const html = ` + + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo declared on a --button-bg global token alongside other tokens', () => { + // A laundered indigo token mixed with legitimate tokens in the same + // :root block must not be stripped — the non-`--accent` indigo + // declaration keeps the whole rule in scope so the literal hex is + // visible to the indigo scan. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on a non-accent token inside an @media-wrapped :root block', () => { + // The at-rule unwrapping must not bypass the non-accent check: + // a media-query-wrapped :root that declares indigo on `--primary` + // is still laundering the LLM default through an arbitrary name. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still flags indigo on a non-accent token declared via a theme-attribute selector', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + + it('still exempts a :root token block that mixes --accent indigo with non-indigo tokens', () => { + // The non-accent check should fire only on indigo-bearing tokens; + // legitimate sibling tokens whose values are unrelated colors must + // not be misread as laundering. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeUndefined(); + }); + + it('still flags indigo on a component rule nested inside @media', () => { + // The exemption only applies to global token blocks. A component + // rule that hard-codes the indigo hex inside an at-rule wrapper + // is still raw indigo and must fire. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'ai-default-indigo')).toBeDefined(); + }); + +}); + +describe('all-caps-no-tracking', () => { + it('flags uppercase rule with no letter-spacing at all', () => { + const html = ` + + New + `; + const findings = lintArtifact(html); + const hit = findings.find((f) => f.id === 'all-caps-no-tracking'); + expect(hit).toBeDefined(); + expect(hit.severity).toBe('P1'); + }); + + it('flags uppercase rule with too-small letter-spacing', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes uppercase rule with adequate letter-spacing in em', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes uppercase rule with adequate letter-spacing in px', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('does not flag a style block with no uppercase rule', () => { + const html = ``; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags an uppercase rule in a SECOND + + New + `; + const findings = lintArtifact(html); + const hit = findings.find((f) => f.id === 'all-caps-no-tracking'); + expect(hit).toBeDefined(); + expect(hit.severity).toBe('P1'); + }); + + it('does not flag an uppercase rule that is entirely inside a CSS comment', () => { + // Regression: the scan ran against the raw + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('still flags an active uppercase rule when surrounded by comments', () => { + // Comments are stripped only for structural matching; the live rule + // outside the comment must still fire. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags inline style with text-transform: uppercase and no letter-spacing', () => { + // Regression: the rule used to scan only + NEW + `; + const findings = lintArtifact(html); + const hits = findings.filter((f) => f.id === 'all-caps-no-tracking'); + expect(hits.length).toBe(1); + }); + + it('passes a 12px label with 1px tracking (resolves 0.06em via same-rule font-size)', () => { + // Regression: the previous absolute-fallback floor of >=1.5px was + // stricter than the craft rule. `font-size: 12px; letter-spacing: 1px` + // is `1 / 12 = 0.083em` — well above the 0.06em rule — and must pass. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes a 14px label with 1px tracking (resolves 0.06em via same-rule font-size)', () => { + // 14px * 0.06 = 0.84px floor, so 1px tracking satisfies the rule. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags a 14px label with 0.5px tracking (below same-rule 0.06em floor)', () => { + // 14px * 0.06 = 0.84px floor; 0.5px is below the rule and must flag. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes inline 12px label with 1px tracking', () => { + // Same regression as the +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes a 16px label with 0.06rem tracking (rem ≈ 1px ≈ 0.06em on 16px)', () => { + // 0.06rem * 16px/rem = 0.96px; on a 16px element that is 0.06em — + // exactly at the floor. The rem branch must accept it. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes a 48px heading with 0.18rem tracking (rem converted, meets element 0.06em)', () => { + // 0.18rem * 16px/rem = 2.88px; 48px * 0.06 = 2.88px floor — the + // converted rem matches the per-element em floor exactly. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags inline 48px heading with 0.06rem tracking', () => { + const html = `

Headline

`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes inline 16px label with 0.06rem tracking (rem ≈ 0.06em on 16px)', () => { + const html = `NEW`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes uppercase rule whose letter-spacing dereferences a compliant :root token', () => { + // Regression: the tracking helper used to recognise only literal + // numeric values, so a tokenized rule — exactly the pattern the + // craft prompt steers artifacts toward — was wrongly reported as + // `all-caps-no-tracking`. The helper now resolves `var(--name)` to + // its `:root` definition and judges the literal value against the + // 0.06em floor. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags uppercase rule whose letter-spacing dereferences a noncompliant :root token', () => { + // The token-resolution path must not blanket-pass `var()` refs: + // a token defined below the 0.06em floor still trips the lint. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags uppercase rule whose letter-spacing var() has no matching :root definition', () => { + // Unresolved references stay in place; the existing "no numeric + // letter-spacing" path then reports the rule as missing tracking. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes uppercase rule whose letter-spacing var() has a compliant fallback', () => { + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes inline uppercase whose letter-spacing dereferences a compliant :root token', () => { + const html = ` + + NEW + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags a 3rem heading with 1px tracking (rem font-size resolves to 48px, 0.06em floor = 2.88px)', () => { + // Regression: the px-vs-element-font-size resolution previously + // matched only `font-size: px`, so a `font-size: 3rem` heading + // fell through to the lenient `>= 1px` fallback and accepted 1px + // tracking — even though the rendered ~48px display has a 2.88px + // floor and 1px is well below the 0.06em rule. The helper now + // resolves `rem` font-size via the same root assumption used for + // tracking and applies the strict per-element floor. + const html = ` + +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes a 3rem heading with 0.06em tracking (em path is unaffected by font-size unit)', () => { + // Sanity check: the rem font-size fix must not regress the em + // letter-spacing branch. `0.06em` is the rule, regardless of how + // font-size is expressed. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes a 3rem heading with 3px tracking (3 ≥ 48 * 0.06 = 2.88)', () => { + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags a tokenized display size with 1px tracking (var() resolves to 3rem, then to 48px)', () => { + // Regression: same root cause via a CSS variable. The agent often + // hides the size behind a token (`--display-size: 3rem`); after + // `resolveCssVars` the body reads `font-size: 3rem;` and must take + // the same strict-floor branch. Without the fix, the rule slipped + // past via the lenient fallback. + const html = ` + +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags a tokenized px display size with 1px tracking', () => { + // The token-resolution path must also catch a px-valued token — + // `font-size: var(--display-size)` with `--display-size: 48px` + // resolves the same way and the 2.88px floor still applies. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags a heading with 1em font-size (unresolvable unit) and 1px tracking', () => { + // When the rule explicitly declares font-size in a unit we cannot + // resolve (`em`, `%`, `calc(...)`, unresolved var), the helper + // refuses the lenient body-text fallback — the element might be + // arbitrarily large. The rule must use `em` letter-spacing or an + // explicit px/rem font-size to be verifiable. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes a heading with 1em font-size and 0.06em tracking (em path is verifiable)', () => { + // The conservative refusal applies only when the caller leans on + // the px fallback. Em letter-spacing is per-element by definition, + // so an em font-size declaration is irrelevant to the check. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags inline 3rem heading with 1px tracking', () => { + // Same regression reproduced through the inline-style branch. + const html = `

Headline

`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags an uppercase rule whose only `letter-spacing` is a custom-property declaration', () => { + // Regression: the previous substring regex matched + // `--letter-spacing: 0.08em` because it scanned the whole rule body + // for `letter-spacing\s*:`. Token-name declarations have no rendered + // effect, so the rule renders ALL CAPS without tracking and must + // still trip the P1 lint. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags a 48px heading whose only `font-size` is a custom-property declaration and tracking is below the floor', () => { + // Regression: `--display-font-size: 48px` previously satisfied the + // bail-out branch that detected an unresolvable font-size, masking + // the fact that no real font-size is declared. With token names + // ignored, the rule falls back to the conservative >=1px floor and + // 0.5px tracking is correctly flagged. + const html = ` + +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags inline uppercase whose only `letter-spacing` is a custom-property declaration', () => { + const html = `NEW`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags an uppercase rule whose compliant letter-spacing is overridden by a later noncompliant one', () => { + // Regression: the helper used to pick the FIRST matching + // letter-spacing declaration in the rule, but CSS applies the LAST + // effective declaration in source order. So + // `.eyebrow { letter-spacing: 0.08em; letter-spacing: 0.02em }` + // renders the noncompliant 0.02em — the lint must judge against the + // last declaration, not the first. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags an inline uppercase whose compliant letter-spacing is overridden by a later noncompliant one', () => { + const html = `NEW`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags a 14px label whose 1px tracking would pass but a later font-size: 100px shifts the floor', () => { + // Regression: `resolveFontSizePx` used to pick the FIRST matching + // font-size declaration; the cascade resolves to the LAST. With + // `font-size: 14px; font-size: 100px`, the rendered floor is + // `100 * 0.06 = 6px`, so 1px tracking is well below the rule and + // must flag — even though the stale 14px would have accepted it + // (14 * 0.06 = 0.84px floor). + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes when the compliant letter-spacing is the LAST declaration (override of an earlier noncompliant one)', () => { + // Sanity check: the cascade fix must not regress the inverse case. + // An author intentionally restoring the floor with a later override + // — `letter-spacing: 0.02em; letter-spacing: 0.08em` — renders 0.08em + // and must not fire the lint. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags an uppercase rule when conflicting :root and [data-theme] tokens disagree on the floor', () => { + // Regression: `extractCssTokens` used to flatten all global theme- + // scope tokens to one map with last-write-wins, regardless of the + // selector that scoped each value. A scoped override that lifted + // the token above the floor could rescue a default-theme value + // that rendered below it, just because the second declaration + // happened to be parsed last. The helper now enumerates every + // applicable value and only passes if all resolutions satisfy the + // 0.06em floor — so the default-theme 0.02em still trips the lint. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags an uppercase rule even when the conflicting :root override comes second', () => { + // Same regression but with declaration order swapped — the previous + // last-write-wins behaviour was order-dependent, so both orderings + // must fail when ANY resolution is below the floor. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes when every conflicting scoped token value clears the floor', () => { + // The conservative cascade must not over-fire: when ALL theme + // variants of a token satisfy the 0.06em rule, the artifact is + // compliant under every applicable theme and the lint must not + // fire. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('passes when a single :root block redeclares the token with a compliant value last', () => { + // Regression: `extractCssTokens` used to record every distinct + // value seen for a custom property, even when the duplicates lived + // in the SAME cascade scope. CSS source-order cascade collapses + // `:root { --caps-tracking: 0.02em; --caps-tracking: 0.08em; }` + // to the second declaration — the first is dead weight, never + // reaches any element. Treating both as theme alternatives fed the + // stale 0.02em into `hasAdequateUppercaseTracking` and emitted a + // spurious P1 on what is normal CSS overriding. The fix collapses + // duplicate declarations within a single rule body to the last + // value before merging into the cross-scope token map. + const html = ` + + New + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags a 48px heading with 1px tracking nested inside @media (innermost-rule scan)', () => { + // Regression: `upperRe` used `[^}]*` for the rule body, so an + // outer `@media (...) { .display { font-size: 48px; text-transform: + // uppercase; letter-spacing: 1px; } }` matched as one rule whose + // selector was `@media (...)` and whose body began with + // `.display { font-size: 48px`. `parseDeclarations` then read the + // first property as `.display { font-size`, lost the same-rule + // font-size, and `hasAdequateUppercaseTracking` fell back to the + // lenient inherited-size path that accepts 1px on a 48px heading. + // Restricting the body alternation to `[^{}]*` makes the regex + // skip the `@media` wrapper and match the inner rule directly, + // restoring the strict per-element 0.06em floor (48 * 0.06 = + // 2.88px), so 1px tracking is correctly flagged. + const html = ` + +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('flags a 48px heading with 1px tracking nested inside @supports', () => { + // Same regression reproduced through @supports, the other + // common at-rule wrapper that previously hid noncompliant + // tracking from the lint. + const html = ` + +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); + + it('passes paired light/dark token values that are each compliant in their own scope', () => { + // Regression: `extractCssTokens` merged token values by name across + // scopes (`--caps-tracking = [1px, 3px]`, `--display-size = [16px, + // 48px]`), and the tracking helper then took an independent + // per-token cartesian product. The impossible cross-theme pairing + // `(--display-size: 48px, --caps-tracking: 1px)` failed the + // 0.06em floor (48 * 0.06 = 2.88px > 1px) and emitted a false + // `all-caps-no-tracking` even though the artifact is compliant + // under both real themes: + // default: 16px size + 1px tracking — 1 / 16 ≈ 0.0625em ≥ 0.06em + // dark: 48px size + 3px tracking — 3 / 48 ≈ 0.0625em ≥ 0.06em + // The fix preserves per-scope token maps and evaluates per-theme + // effective maps so paired declarations stay paired. + const html = ` + +

Headline

+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeUndefined(); + }); + + it('flags paired theme tokens when one scope is internally noncompliant', () => { + // The per-theme evaluation must not silently rescue a scope whose + // own paired values fall below the floor. Default theme here is + // 48px size + 1px tracking — 1 / 48 ≈ 0.021em, well below the + // 0.06em rule — and must flag, even though the dark scope is + // internally compliant. + const html = ` + + `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'all-caps-no-tracking')).toBeDefined(); + }); +}); + +describe('trust-gradient', () => { + it('flags a blue→cyan two-stop gradient with hex stops', () => { + // Regression: `craft/anti-ai-slop.md` documents blue→cyan as a + // P0 cardinal-sin trust gradient, but the existing purple-gradient + // rule only matches violet/indigo hex stops or the literal + // `purple`/`violet` keywords. A pure blue→cyan gradient slipped + // past unflagged. The new `trust-gradient` rule closes that gap. + const html = `
Hi
`; + const findings = lintArtifact(html); + const hit = findings.find((f) => f.id === 'trust-gradient'); + expect(hit).toBeDefined(); + expect(hit.severity).toBe('P0'); + }); + + it('flags a blue→cyan two-stop gradient with keyword stops', () => { + const html = `
Hi
`; + const findings = lintArtifact(html); + const hit = findings.find((f) => f.id === 'trust-gradient'); + expect(hit).toBeDefined(); + expect(hit.severity).toBe('P0'); + }); + + it('flags a sky→cyan gradient (sky shares the blue ramp under another name)', () => { + const html = `
Hi
`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'trust-gradient')).toBeDefined(); + }); + + it('does not double-fire when purple-gradient already caught a purple→blue/cyan stop list', () => { + // A gradient that mixes purple/indigo with blue/cyan triggers + // purple-gradient first. The trust-gradient rule must skip in that + // case so the agent gets a single corrective signal. + const html = `
Hi
`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'purple-gradient')).toBeDefined(); + expect(findings.find((f) => f.id === 'trust-gradient')).toBeUndefined(); + }); + + it('does not flag a blue-only gradient (no cyan stop)', () => { + // A single-color gradient (blue→darker-blue) is a different + // pattern; only the documented two-color blue→cyan trust ramp + // is the AI tell. + const html = `
Hi
`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'trust-gradient')).toBeUndefined(); + }); + + it('does not flag a gradient with only cyan stops', () => { + const html = `
Hi
`; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'trust-gradient')).toBeUndefined(); + }); + + it('flags a blue→cyan gradient declared inside a +
Welcome
+ `; + const findings = lintArtifact(html); + expect(findings.find((f) => f.id === 'trust-gradient')).toBeDefined(); + }); +}); diff --git a/craft/README.md b/craft/README.md new file mode 100644 index 000000000..e0ec276a6 --- /dev/null +++ b/craft/README.md @@ -0,0 +1,62 @@ +# Craft references + +Brand-agnostic craft knowledge. Each file is a small, dense rulebook on one +dimension of professional UI craft (typography, color, motion, …). Skills +opt into the references they need; the daemon injects only the requested +ones into the system prompt above the active skill body. + +## Why a third axis next to `skills/` and `design-systems/` + +| Axis | Scope | Example | +|---|---|---| +| `skills/` | Artifact shape | `saas-landing`, `dashboard`, `pricing-page` | +| `design-systems/` | Brand visual language (the 9-section `DESIGN.md`) | `linear-app`, `apple`, `notion` | +| `craft/` | **Universal** craft knowledge — true regardless of brand | letter-spacing rules, accent-overuse caps, anti-AI-slop | + +`DESIGN.md` tells the agent which colors and fonts a brand uses. `craft/` +tells the agent the universal rules a competent designer applies on top — +e.g. ALL CAPS always needs ≥0.06em tracking, regardless of the brand. + +## How a skill opts in + +Add an `od.craft.requires` array to the skill's front-matter. Only the +listed sections are injected, so a skill that needs only typography pays +no token cost for color/motion content. + +```yaml +od: + craft: + requires: [typography, color, anti-ai-slop] +``` + +Allowed values match the file names in this directory minus the `.md` +extension. Unknown values are silently ignored (forward-compatible). + +### Why silent fallback instead of fail-fast? + +A skeptical reader will ask: "If a skill requests `motion` and we don't +ship `motion.md` yet, shouldn't we warn the user?" We chose +forward-compatibility over fail-fast: a skill authored today can list +`motion` and start benefiting the moment we vendor `craft/motion.md` in +a follow-up PR, with no skill edit needed. The cost of a missed +reference is a missing paragraph in the system prompt, not a broken +skill — so the loud failure mode is not worth the friction. + +## Files + +| File | Section name | When to require | +|---|---|---| +| `typography.md` | `typography` | Any skill that emits typed content (~all skills) | +| `color.md` | `color` | Any skill that emits styled output (~all skills) | +| `anti-ai-slop.md` | `anti-ai-slop` | Marketing pages, landing pages, decks | + +More sections (`motion`, `icons`, `craft-details`) will be added in +follow-up PRs as we wire the linter side. + +## Attribution + +Craft content is adapted from the MIT-licensed +[refero_skill](https://github.com/referodesign/refero_skill) project +(© Refero Design), with edits to fit Open Design's house style and link +back to OD's design tokens (`var(--accent)` etc.) instead of generic +Tailwind hex values. diff --git a/craft/anti-ai-slop.md b/craft/anti-ai-slop.md new file mode 100644 index 000000000..fe358048a --- /dev/null +++ b/craft/anti-ai-slop.md @@ -0,0 +1,84 @@ +# Anti-AI-slop rules + +Concrete, checkable rules that distinguish "designed by a human who has +shipped product" from "default LLM output." Several rules below are +auto-enforced by the daemon's `lint-artifact` linter — failing an +enforced rule is not a style preference, it is a regression. The +rest are guidance for agents and reviewers and are flagged inline as +"(guidance, not auto-checked)" so the contract with the linter stays +honest. + +> Adapted from [refero_skill](https://github.com/referodesign/refero_skill) +> (MIT), tightened to match Open Design's lint surface. + +## The seven cardinal sins + +These are the patterns the linter blocks at P0 (must-fix): + +1. **Default Tailwind indigo as accent** — exactly `#6366f1`, `#4f46e5`, + `#4338ca`, `#3730a3`, `#8b5cf6`, `#7c3aed`, `#a855f7`. The active + `DESIGN.md` provides `--accent`; use it. Indigo is the textbook AI + tell. (The daemon's `lint-artifact` flags any of these as a solid + accent; keep this list in sync with `AI_DEFAULT_INDIGO` in + `apps/daemon/src/lint-artifact.ts`.) +2. **Two-stop "trust" gradient on the hero** — purple→blue, blue→cyan, + indigo→pink. A flat surface + intentional type beats this every + time. +3. **Emoji as feature icons** — `✨`, `🚀`, `🎯`, `⚡`, `🔥`, `💡` + inside ``, `