feat(skill): pf design token check skill#108
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds documentation for the Changespf-design-token-check Skill Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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.
|
@jpuzz0, for the movement to |
Hey @jcmill! I think you'll have to pull down the latest changes on main to see the |
jcmill
force-pushed
the
feat/101-pf-token-check-skill
branch
from
d7904fc to
49233e0
Compare
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