Added Shade conventions skills for AI agents#28960
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
✅ Files skipped from review due to trivial changes (24)
WalkthroughTen new agent skill documents are added under Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 9m 48s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run-many -t test:unit -p @tryghost/shade,@tr... |
✅ Succeeded | 7m 2s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 4m 12s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 39s | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 6s | View ↗ |
nx run-many -t lint -p ghost-monorepo,@tryghost... |
✅ Succeeded | 2s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 1s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-30 11:07:23 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
.agents/skills/shade-no-dark-variants/SKILL.md (2)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider expanding trigger to
.cssfiles.The
autoTriggeronly targets.tsxfiles, but.cssfiles in Shade-consuming apps could also containdark:variants (e.g., in@applydirectives or custom variants). The companionshade-tokens-not-hexskill triggers on both.tsxand.css. Consider aligning scopes unless CSS dark: variants are handled differently or impossible in this codebase.🤖 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 @.agents/skills/shade-no-dark-variants/SKILL.md at line 5, The autoTrigger scope in the shade-no-dark-variants skill is too narrow because it only matches .tsx files, so CSS files with dark: variants can be missed. Update the fileEdit pattern in SKILL.md to include .css files for the Shade-consuming apps, aligning it with the shade-tokens-not-hex skill unless there is a deliberate reason to keep CSS excluded.
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify why
dark:bg-surface-elevatedis harmful.The example shows
bg-surface-elevated dark:bg-surface-elevatedas "BAD — duplicating a semantic token with a dark: override". While redundant, explain why this is bad beyond verbosity: it increases bundle size, defeats Tailwind's purge optimization, and may cause specificity issues if the token value changes. Or remove if it's truly harmless.🤖 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 @.agents/skills/shade-no-dark-variants/SKILL.md at line 27, Clarify the guidance in the shade-no-dark-variants skill example where the same semantic token is used in both the base class and the dark: override. Update the explanatory text around bg-surface-elevated dark:bg-surface-elevated to state that this pattern adds redundant CSS, can increase bundle size despite Tailwind’s optimization, and may create override/specificity problems if the token value changes; if that behavior is acceptable, remove the “BAD” example instead..agents/skills/shade-dropdown-surface-contract/SKILL.md (1)
12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
The contract code block lacks a language tag, triggering markdownlint MD040. Add
tsxortextto clarify this is a class string snippet, not executable code.+bg-surface-elevated-2
border border-border/60 dark:border-border/30
shadow-md🤖 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 @.agents/skills/shade-dropdown-surface-contract/SKILL.md around lines 12 - 16, The fenced contract snippet in SKILL.md is missing a language identifier, which triggers markdownlint MD040. Update the fenced block to include an appropriate specifier such as text or tsx so the class string in the shade-dropdown-surface-contract section is clearly marked as non-executable code.Source: Linters/SAST tools
.agents/skills/shade-tokens-not-hex/SKILL.md (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCross-reference mismatch:
theme-variables.cssvstailwind.theme.css.Line 30 directs readers to
apps/shade/theme-variables.cssfor the full inventory, but line 92 directs toapps/shade/tailwind.theme.cssfor raw tokens. Clarify which file contains what: typicallytheme-variables.cssholds the CSS custom property definitions (the actual values), whiletailwind.theme.cssholds the@themeblock that maps Tailwind v4--color-*keys to those variables. Agents need to know the difference to avoid editing the wrong file.🤖 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 @.agents/skills/shade-tokens-not-hex/SKILL.md at line 30, Clarify the cross-reference between theme-variables.css and tailwind.theme.css in the shade-tokens-not-hex skill doc: update the guidance so theme-variables.css is identified as the source of the actual CSS custom property values, while tailwind.theme.css is identified as the Tailwind v4 `@theme` mapping layer for --color-* tokens. Adjust the references around the inventory and raw token sections so readers know which file to edit for values versus mappings, and keep the wording consistent wherever these two files are mentioned.
🤖 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 @.agents/skills/shade-component-decision/SKILL.md:
- Line 21: Update the Pattern examples in SKILL.md by removing ListPage from the
Pattern layer list, since it now belongs under the page-templates barrel and
should not be classified as a pattern. If you intentionally want to keep it
referenced here, add a clear note in the same section that ListPage was promoted
to page-templates/ so the taxonomy stays accurate. Use the Pattern examples list
in SKILL.md to locate the entry.
In @.agents/skills/shade-input-surface-recipe/SKILL.md:
- Line 84: Remove the incorrect dark-mode token from the Background row in the
shade input surface recipe: the `inputSurfaceClasses.base` symbol in
`apps/shade/src/components/ui/input-surface.ts` only maps to
`bg-surface-elevated`, while `dark:bg-transparent` belongs to focus state
styling, not the base background. Update the documentation entry so the
Background section matches the actual `inputSurfaceClasses` definitions and keep
the focus-related transparent background out of the base token list.
In @.agents/skills/shade-tokens-not-hex/SKILL.md:
- Line 21: Clarify the “Standard border” entry so it no longer treats
`border-border` and `border-border-default` as interchangeable; update the
`SKILL.md` mapping to separate the generic/legacy `border-border` token from the
current standard `border-border-default` token, and add a note that
`border-border-default` maps to `--border-default` for surface/form-control
borders while `border-border` maps to `--border` for broader legacy use.
---
Nitpick comments:
In @.agents/skills/shade-dropdown-surface-contract/SKILL.md:
- Around line 12-16: The fenced contract snippet in SKILL.md is missing a
language identifier, which triggers markdownlint MD040. Update the fenced block
to include an appropriate specifier such as text or tsx so the class string in
the shade-dropdown-surface-contract section is clearly marked as non-executable
code.
In @.agents/skills/shade-no-dark-variants/SKILL.md:
- Line 5: The autoTrigger scope in the shade-no-dark-variants skill is too
narrow because it only matches .tsx files, so CSS files with dark: variants can
be missed. Update the fileEdit pattern in SKILL.md to include .css files for the
Shade-consuming apps, aligning it with the shade-tokens-not-hex skill unless
there is a deliberate reason to keep CSS excluded.
- Line 27: Clarify the guidance in the shade-no-dark-variants skill example
where the same semantic token is used in both the base class and the dark:
override. Update the explanatory text around bg-surface-elevated
dark:bg-surface-elevated to state that this pattern adds redundant CSS, can
increase bundle size despite Tailwind’s optimization, and may create
override/specificity problems if the token value changes; if that behavior is
acceptable, remove the “BAD” example instead.
In @.agents/skills/shade-tokens-not-hex/SKILL.md:
- Line 30: Clarify the cross-reference between theme-variables.css and
tailwind.theme.css in the shade-tokens-not-hex skill doc: update the guidance so
theme-variables.css is identified as the source of the actual CSS custom
property values, while tailwind.theme.css is identified as the Tailwind v4
`@theme` mapping layer for --color-* tokens. Adjust the references around the
inventory and raw token sections so readers know which file to edit for values
versus mappings, and keep the wording consistent wherever these two files are
mentioned.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 979ce2bd-e1a5-4ca1-99fb-cfd627f54fc7
📒 Files selected for processing (21)
.agents/skills/shade-component-decision/SKILL.md.agents/skills/shade-dropdown-surface-contract/SKILL.md.agents/skills/shade-imports/SKILL.md.agents/skills/shade-input-surface-recipe/SKILL.md.agents/skills/shade-new-component/SKILL.md.agents/skills/shade-no-dark-variants/SKILL.md.agents/skills/shade-page-templates/SKILL.md.agents/skills/shade-shadcn-install/SKILL.md.agents/skills/shade-tokens-not-hex/SKILL.md.agents/skills/shade-use-primitives/SKILL.md.claude/skills/shade-component-decision.claude/skills/shade-dropdown-surface-contract.claude/skills/shade-imports.claude/skills/shade-input-surface-recipe.claude/skills/shade-new-component.claude/skills/shade-no-dark-variants.claude/skills/shade-page-templates.claude/skills/shade-shadcn-install.claude/skills/shade-tokens-not-hex.claude/skills/shade-use-primitivesapps/shade/AGENTS.md
| | Recipe owns | You add | | ||
| |---|---| | ||
| | Border (`border-control-border`) | Height, padding | | ||
| | Background (`bg-surface-elevated`, `dark:bg-transparent`) | Typography (`text-control`, `text-sm`) | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual inputSurfaceClasses.base content
cat -n apps/shade/src/components/ui/input-surface.ts | head -20Repository: TryGhost/Ghost
Length of output: 1199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SKILL.md around line 84 ---'
sed -n '76,90p' .agents/skills/shade-input-surface-recipe/SKILL.md | cat -n
echo
echo '--- input-surface.ts around class definitions ---'
sed -n '20,120p' apps/shade/src/components/ui/input-surface.ts | cat -nRepository: TryGhost/Ghost
Length of output: 3307
Remove dark:bg-transparent from the Background row .agents/skills/shade-input-surface-recipe/SKILL.md:84 lists dark:bg-transparent, but apps/shade/src/components/ui/input-surface.ts only defines bg-surface-elevated on inputSurfaceClasses.base; the transparent background is used in focus states, not as a dark-mode background token.
🤖 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 @.agents/skills/shade-input-surface-recipe/SKILL.md at line 84, Remove the
incorrect dark-mode token from the Background row in the shade input surface
recipe: the `inputSurfaceClasses.base` symbol in
`apps/shade/src/components/ui/input-surface.ts` only maps to
`bg-surface-elevated`, while `dark:bg-transparent` belongs to focus state
styling, not the base background. Update the documentation entry so the
Background section matches the actual `inputSurfaceClasses` definitions and keep
the focus-related transparent background out of the base token list.
| | Floating menu on top of elevated | `bg-surface-elevated-2` | `--surface-elevated-2` | | ||
| | Primary text | `text-foreground` / `text-primary` | `--text-primary` | | ||
| | Secondary / muted text | `text-muted-foreground` | `--text-secondary` | | ||
| | Standard border | `border-border` / `border-border-default` | `--border-default` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify border-border vs border-border-default distinction.
The table lists both as alternatives for "Standard border", but the upstream tailwind.theme.css defines --color-border: var(--border) and --color-border-default: var(--border-default) as separate tokens with different semantic purposes. Conflating them risks agents picking the wrong token for form-control contexts where --border-default is explicitly required vs. generic borders.
Consider splitting the row or adding a note that border-border (from --border) is legacy/generic while border-border-default (from --border-default) is the current standard surface border.
🤖 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 @.agents/skills/shade-tokens-not-hex/SKILL.md at line 21, Clarify the
“Standard border” entry so it no longer treats `border-border` and
`border-border-default` as interchangeable; update the `SKILL.md` mapping to
separate the generic/legacy `border-border` token from the current standard
`border-border-default` token, and add a note that `border-border-default` maps
to `--border-default` for surface/form-control borders while `border-border`
maps to `--border` for broader legacy use.
ref https://linear.app/ghost/issue/DES-1394 - 10 auto-triggering skills under .agents/skills/shade-* encode Shade design system rules so AI agents follow them at the moment of edit, without needing to discover apps/shade/AGENTS.md. - Tier 1 (broad triggers): shade-imports, shade-tokens-not-hex, shade-no-dark-variants, shade-use-primitives, shade-component-decision. - Tier 2 (narrow, high-payoff triggers): shade-new-component, shade-shadcn-install, shade-page-templates, shade-input-surface-recipe, shade-dropdown-surface-contract. - Triggers scoped to actual Shade-consuming apps (shade, admin, admin-x-settings, admin-x-framework, posts, stats, activitypub). Public apps and the legacy admin-x-design-system are excluded to avoid false positives. - AGENTS.md: fixed stale ListPage import (now from /page-templates), removed ListPage from Patterns examples, documented page-templates and posts-stats as additional barrels. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
58c2236 to
6ce561d
Compare
ref https://linear.app/ghost/issue/DES-1394 - patterns-guide.mdx had a broken example importing ListPage from @tryghost/shade/patterns (it lives in /page-templates). Fixed the import and trimmed the example to a PageHeader-only snippet, linking to page-types for the full ListPage wrapper. - Removed ListPage from the Patterns examples in introduction.mdx, layers.mdx, contributing.mdx, and patterns-guide.mdx; added a "Page templates" row/bullet pointing to @tryghost/shade/page-templates. - page-types.mdx: changed the ListPage label from **Pattern** to **Page template** to match the actual layer. This makes the human docs match apps/shade/AGENTS.md and the new Shade skills, so engineers and AI agents see one consistent story for where ListPage lives. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ref https://linear.app/ghost/issue/DES-1394 - shade-component-decision: removed ListPage from Pattern examples (it belongs in page-templates/), matching the same fix already in AGENTS.md and the Storybook MDX docs. - shade-tokens-not-hex: split the border row into three distinct entries — border-border-default (opaque, default), border-border (translucent, for floating surfaces with /60-/30 opacity modifiers), and border-control-border (form controls). Also clarified the role of theme-variables.css (token values + dark overrides) vs tailwind.theme.css (Tailwind v4 @theme utility mapping + raw scale). - shade-no-dark-variants: explained why bg-foo dark:bg-foo is bad — not just verbose, but reads as if dark mode is special-cased when it isn't. - shade-dropdown-surface-contract: added `text` language tag to the fenced contract block (markdownlint MD040). Skipped two CodeRabbit notes after verification: - "dark:bg-transparent not in inputSurface base" — CodeRabbit reviewed pre-rebase; after rebasing onto main (with PR #28895 merged), base does include dark:bg-transparent. Skill is correct. - "Add .css to no-dark-variants trigger" — dark: isn't valid CSS-file syntax and Shade doesn't use @apply dark: patterns. Low value. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
closes https://linear.app/ghost/issue/DES-1394/add-shade-skills
Adds 10 auto-triggering Claude Code skills under
.agents/skills/shade-*that encode Shade design system rules so AI agents follow them at the moment of edit, without needing to discoverapps/shade/AGENTS.md.Tier 1 — broad triggers, fires often:
shade-imports— layer-specific subpaths, never root barrelshade-tokens-not-hex— semantic tokens only, with full inventoryshade-no-dark-variants— don't writedark:for colourshade-use-primitives—Stack/Inline/Box/Gridover bare flex divsshade-component-decision— layer decision flow + promotion checklistTier 2 — narrow, high-payoff triggers:
shade-new-component— acceptance checklist for new Shade filesshade-shadcn-install—pnpm dlx shadcnguardrails + post-install cleanupshade-page-templates—ListPage/PageHeaderskeleton + page-type taxonomyshade-input-surface-recipe—inputSurface()compositionshade-dropdown-surface-contract—DropdownMenu/Select/Popoverchange togetherTriggers are scoped to actual Shade-consuming apps (
shade,admin,admin-x-settings,admin-x-framework,posts,stats,activitypub). Public apps (portal,comments-ui,signup-form,sodo-search,announcement-bar) and the legacyadmin-x-design-systemare excluded to avoid false positives.apps/shade/AGENTS.mdis also tightened:ListPageimport (now@tryghost/shade/page-templates, not/patterns)ListPagefrom the Patterns examples rowpage-templates/andposts-stats/as additional barrelsDependency on PR #28895
Several skills (
shade-tokens-not-hex,shade-dropdown-surface-contract,shade-input-surface-recipe) reference tokens introduced in #28895 —--surface-elevated-2,--control-border,--interactive-hover,--button-hover,--tab-hover,--tab-active,--table-row-hover. The skills don't break anything if merged first, but their examples reference tokens that don't yet exist onmain. Ideally land #28895 first; otherwise the example references just become accurate as soon as #28895 merges.Test plan
shade-*skills appear in the available-skills listapps/admin-x-settings/src/containing@tryghost/shadeand confirm relevant skills loadapps/portal/src/and confirm Shade skills don't loadapps/shade/AGENTS.mdrenders correctly with the updated barrel list🤖 Generated with Claude Code