feat(skill): pf design token check skill#108
Conversation
|
Too much diff to scan? Review this PR in Change Stack to start with the highest-impact changes. 📝 WalkthroughWalkthroughThis PR adds comprehensive documentation for the Changespf-design-token-check Skill Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
plugins/react/skills/pf-design-token-check/SKILL.md (3)
3-3: ⚡ Quick winNitpick: Align skill description with required “Use when …” formula.
The description starts correctly with an action verb, but it should include an explicit trigger-context sentence beginning with “Use when …” to match repo skill-discovery rules.
As per coding guidelines, “description includes trigger contexts in a ‘Use when’ clause.”🤖 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 `@plugins/react/skills/pf-design-token-check/SKILL.md` at line 3, Update the skill description in SKILL.md to follow the repo's "Use when …" formula by appending a trigger-context sentence that begins with "Use when …" and describes when the checker should run (e.g., when reviewing CSS/SCSS/CSS-in-JS or inline styles for hardcoded color, spacing, typography, or shadow values that have PF token equivalents); keep the existing action-verb lead and ensure the new sentence clearly states the trigger context for the skill.
139-139: ⚡ Quick winNitpick: Fix malformed token filename example.
Line 139 lists
tokens-.scss, which looks like a typo and can mislead token-file search behavior in usage docs.
As per coding guidelines, skill instructions should be clear and outcome-oriented; malformed references reduce reliability.🤖 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 `@plugins/react/skills/pf-design-token-check/SKILL.md` at line 139, Replace the malformed token filename example `tokens-.scss` with the correct dark-theme filename (e.g., `tokens-dark.scss`) in SKILL.md; search for other occurrences of `tokens-.scss` in the file and update them to `tokens-dark.scss` (or the actual dark-theme token filename used by the project) so documentation and token-file search examples are accurate and consistent.
210-210: ⚡ Quick winNitpick: Add language tags to fenced code blocks (MD040).
These fences are missing language identifiers; adding
text,css, ormd(as appropriate) resolves the current markdownlint warnings.Also applies to: 224-224, 229-229, 250-250, 266-266
🤖 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 `@plugins/react/skills/pf-design-token-check/SKILL.md` at line 210, In SKILL.md update each fenced code block that currently has plain triple backticks to include an appropriate language tag (e.g., text, css, md) to satisfy MD040; locate the example/code fences around the token examples and replace ``` with language-tagged fences like ```text or ```css as appropriate so markdownlint stops flagging them (apply this change to all similar fences noted in the comment).plugins/react/skills/pf-design-token-check/reference/comparison.md (1)
42-42: ⚡ Quick winNitpick: Add language identifiers to code fences (MD040).
These blocks should include a language label to satisfy markdownlint and improve rendering clarity.
Also applies to: 52-52, 73-73
🤖 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 `@plugins/react/skills/pf-design-token-check/reference/comparison.md` at line 42, The markdown contains bare code fences ("```") in comparison.md that trigger MD040; update each triple-backtick block (the fences shown as "```" at the example code blocks) to include the appropriate language identifier (for example ```json, ```css, ```js as applicable) so each code fence is labeled and markdownlint passes.plugins/react/skills/pf-design-token-check/README.md (1)
83-83: ⚡ Quick winNitpick: Specify languages for fenced code blocks (MD040).
Please add fence languages (
text,bash, etc.) to clear markdownlint warnings and keep docs consistent.Also applies to: 112-112, 120-120
🤖 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 `@plugins/react/skills/pf-design-token-check/README.md` at line 83, Update the fenced code blocks in the README.md that currently use bare ``` by adding explicit languages (for example change ``` to ```bash for shell/CLI examples and to ```text for plain output/snippets) so markdownlint MD040 is satisfied; locate the anonymous fences around the CLI examples and output/examples and add the appropriate language token after the opening backticks.
🤖 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.
Nitpick comments:
In `@plugins/react/skills/pf-design-token-check/README.md`:
- Line 83: Update the fenced code blocks in the README.md that currently use
bare ``` by adding explicit languages (for example change ``` to ```bash for
shell/CLI examples and to ```text for plain output/snippets) so markdownlint
MD040 is satisfied; locate the anonymous fences around the CLI examples and
output/examples and add the appropriate language token after the opening
backticks.
In `@plugins/react/skills/pf-design-token-check/reference/comparison.md`:
- Line 42: The markdown contains bare code fences ("```") in comparison.md that
trigger MD040; update each triple-backtick block (the fences shown as "```" at
the example code blocks) to include the appropriate language identifier (for
example ```json, ```css, ```js as applicable) so each code fence is labeled and
markdownlint passes.
In `@plugins/react/skills/pf-design-token-check/SKILL.md`:
- Line 3: Update the skill description in SKILL.md to follow the repo's "Use
when …" formula by appending a trigger-context sentence that begins with "Use
when …" and describes when the checker should run (e.g., when reviewing
CSS/SCSS/CSS-in-JS or inline styles for hardcoded color, spacing, typography, or
shadow values that have PF token equivalents); keep the existing action-verb
lead and ensure the new sentence clearly states the trigger context for the
skill.
- Line 139: Replace the malformed token filename example `tokens-.scss` with the
correct dark-theme filename (e.g., `tokens-dark.scss`) in SKILL.md; search for
other occurrences of `tokens-.scss` in the file and update them to
`tokens-dark.scss` (or the actual dark-theme token filename used by the project)
so documentation and token-file search examples are accurate and consistent.
- Line 210: In SKILL.md update each fenced code block that currently has plain
triple backticks to include an appropriate language tag (e.g., text, css, md) to
satisfy MD040; locate the example/code fences around the token examples and
replace ``` with language-tagged fences like ```text or ```css as appropriate so
markdownlint stops flagging them (apply this change to all similar fences noted
in the comment).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: acdde748-1ec4-48be-aa10-a3c1099a4b03
📒 Files selected for processing (4)
plugins/react/skills/pf-design-token-check/README.mdplugins/react/skills/pf-design-token-check/SKILL.mdplugins/react/skills/pf-design-token-check/reference/comparison.mdplugins/react/skills/pf-design-token-check/reference/example-violations.md
jpuzz0
left a comment
jpuzz0
left a comment
There was a problem hiding this comment.
Plugin placement: This skill belongs in plugins/design-audit/ rather than plugins/react/. The decision test for design-audit is "does this check whether existing code or designs follow PF standards?" — that's exactly what this does. Can you move it to plugins/design-audit/skills/pf-design-token-check/?
Skill length: At 431 lines this is close to our 500-line hard limit. A lot of the length comes from the inline examples and the detailed regex patterns in Scanning Logic (the LLM doesn't need regex spelled out). Could both be trimmed since reference/example-violations.md already covers test cases?
| @@ -0,0 +1,431 @@ | |||
| --- | |||
| name: pf-design-token-check | |||
| description: Detect hardcoded color, spacing, typography, and shadow values that have PF token equivalents and suggest the correct design token replacements. Works on CSS, SCSS, CSS-in-JS, and inline styles. | |||
There was a problem hiding this comment.
Can you add a "Use when..." clause to the description? Our other skills front-load trigger context so the AI knows when to suggest it.
| --- | ||
|
|
||
| ### Role | ||
| You are a Senior Design Systems Engineer specializing in CSS refactoring and Design Token implementation for PatternFly. |
There was a problem hiding this comment.
Is the "You are a Senior Design Systems Engineer" role section needed? None of our other skills use role-play preambles — removing it would also help with length.
There was a problem hiding this comment.
That's been a habit of mine. I can remove it.
| ### Exception Handling | ||
| **Do NOT flag:** | ||
| 1. Values already using design tokens (e.g., `var(--pf-v6-...)`) | ||
| 2. Values already using design tokens with fallbacks (e.g., `var(--pf-v6-..., 0)`) (but flag the raw value within the fallback) |
There was a problem hiding this comment.
These say "Do NOT flag" but then add "(but flag the raw value within)" — could you clarify the intent? The AI might get confused about whether to flag or skip.
| - `tokens-glass-dark.scss` - Glass Dark theme tokens | ||
| - `tokens-highcontrast-dark.scss` - High Contrast Dark theme tokens | ||
| - `tokens-highcontrast.scss` - High Contrast theme tokens | ||
| - `tokens-.scss` - Dark theme tokens |
There was a problem hiding this comment.
nit: tokens-.scss looks like it's missing a name segment — what file was this meant to be?
There was a problem hiding this comment.
Removed as it was unneeded. This was a placeholder when I was working on it, forgot to remove.
|
|
||
| ## Migration Path | ||
|
|
||
| `pf-raw-colors-scan` should be considered deprecated in favor of `pf-design-token-check`. |
There was a problem hiding this comment.
The comparison is useful, but the deprecation language ("should be considered deprecated") is a maintainer decision we should make separately. Same claim in README.md line 137 ("legacy, use this skill instead") — could both be reframed as capability comparison without deprecation language?
| marginTop: '24px', | ||
| borderRadius: '8px' | ||
| }}> | ||
| ``` No newline at end of file |
There was a problem hiding this comment.
nit: missing trailing newline.
| @@ -0,0 +1,144 @@ | |||
| # pf-design-token-check | |||
There was a problem hiding this comment.
Best practice is to keep all skill documentation in SKILL.md and references/ — could the useful parts here move to a reference file to avoid two sources of truth drifting apart?
There was a problem hiding this comment.
I'll clean this up to be more user-focused so there isn't overlap.
| @@ -0,0 +1,97 @@ | |||
| # Comparison: pf-design-token-check vs pf-raw-colors-scan | |||
There was a problem hiding this comment.
nit: CONTRIBUTING-SKILLS.md specifies references/ (plural) as the convention — would you mind renaming reference/ to references/?
|
@jpuzz0, for the movement to |
Hey @jcmill! I think you'll have to pull down the latest changes on main to see the |
2 participants
resolves: #101
This skill detects hardcoded css values that should use a PatternFly design token. Works across CSS, SCSS, CSS in JS, React and inline styles.
Summary by CodeRabbit