feat(pf-design-tokens): add analyze-modifiers skill#45
feat(pf-design-tokens): add analyze-modifiers skill#45srambach wants to merge 6 commits intopatternfly:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughNew skill definition adds "pf-analyze-modifiers": an assistant workflow to scan PatternFly SCSS for Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Assistant as Assistant
participant Repo as "Repo / Filesystem"
participant Parser as "Extractor/Parser"
User->>Assistant: Request analysis + choose scope
Assistant->>Repo: Search SCSS for pf-m-* patterns (grep)
Repo-->>Assistant: Matching files & lines
Assistant->>Parser: Extract selectors, targets, properties, variants
Parser-->>Assistant: Structured modifier records
Assistant->>Assistant: Group/organize results (by modifier/component/stats)
Assistant-->>User: Return Markdown outputs and filenames
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Guidelines used: CONTRIBUTING.md, CONTRIBUTING-SKILLS.md, skill-creator plugin checklist. Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/pf-design-tokens/skills/analyze-modifiers/SKILL.md (1)
97-102: Add conditional output guidance with inline fallback for portability.Lines 97–102 specify file writing without addressing environments where filesystem output is unavailable. Per CONTRIBUTING-SKILLS.md, skills should be tool-agnostic—they need to work across different contexts. Anthropic's skill-creator guidance requires inline-output fallback: present results directly in the conversation when the file/viewer flow isn't available; only write files when the user needs them for download or inspection.
Revise Step 5 to state: "Present results as markdown in the conversation. When file output is needed (e.g., user requests download), write to appropriately named files:
modifiers-listing.md,{component-name}-modifiers.md, or{modifier-name}-usage.md."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/pf-design-tokens/skills/analyze-modifiers/SKILL.md` around lines 97 - 102, Update Step 5 in SKILL.md (the "Generate Output" section) to be tool-agnostic by instructing the skill to present results as markdown directly in the conversation by default and only write files when the user explicitly requests file output (for download/inspection); keep the existing filenames (`modifiers-listing.md`, `{component-name}-modifiers.md`, `{modifier-name}-usage.md`) as the names to use when writing files, and add a brief note referencing CONTRIBUTING-SKILLS.md / skill-creator guidance to prefer inline fallback when filesystem or viewer flows are unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/pf-design-tokens/skills/analyze-modifiers/SKILL.md`:
- Around line 46-67: Update the examples in SKILL.md to use the plugin-qualified
invocation form `/<plugin-name>:<skill-name>` as specified in
CONTRIBUTING-SKILLS.md; specifically replace all occurrences of
`/analyze-modifiers` with `/pf-design-tokens:analyze-modifiers` in the Example
blocks (e.g., the Example 1, Example 2, Example 3, and Example 4 snippets) so
the skill name `analyze-modifiers` is always prefixed with the plugin id
`pf-design-tokens`.
- Around line 47-67: The fenced code blocks showing the example commands (e.g.,
"User: /analyze-modifiers", "User: /analyze-modifiers for the Button component",
"User: /analyze-modifiers find where pf-m-disabled is used", "User:
/analyze-modifiers compare pf-m-expanded and pf-m-collapsed") need language
identifiers to satisfy markdownlint MD040; update each triple-backtick fence in
SKILL.md to include a language token (for example `text`) so the blocks read
```text before the command and ``` after.
---
Nitpick comments:
In `@plugins/pf-design-tokens/skills/analyze-modifiers/SKILL.md`:
- Around line 97-102: Update Step 5 in SKILL.md (the "Generate Output" section)
to be tool-agnostic by instructing the skill to present results as markdown
directly in the conversation by default and only write files when the user
explicitly requests file output (for download/inspection); keep the existing
filenames (`modifiers-listing.md`, `{component-name}-modifiers.md`,
`{modifier-name}-usage.md`) as the names to use when writing files, and add a
brief note referencing CONTRIBUTING-SKILLS.md / skill-creator guidance to prefer
inline fallback when filesystem or viewer flows are unavailable.
🪄 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: 0b6b25e4-498b-4533-811c-381da88707c9
📒 Files selected for processing (1)
plugins/pf-design-tokens/skills/analyze-modifiers/SKILL.md
| ``` | ||
| User: /analyze-modifiers | ||
| ``` | ||
| Expected: Create a complete listing of all modifiers across all components, organized alphabetically by modifier name. | ||
|
|
||
| ### Example 2: Analyze specific component | ||
| ``` | ||
| User: /analyze-modifiers for the Button component | ||
| ``` | ||
| Expected: List all modifiers used in the Button component with brief descriptions of what they do. | ||
|
|
||
| ### Example 3: Find modifier usage | ||
| ``` | ||
| User: /analyze-modifiers find where pf-m-disabled is used | ||
| ``` | ||
| Expected: List all components that use the pf-m-disabled modifier and show the CSS properties it applies in each context. | ||
|
|
||
| ### Example 4: Compare modifiers | ||
| ``` | ||
| User: /analyze-modifiers compare pf-m-expanded and pf-m-collapsed | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
The four example fences should declare a language (e.g., text) to satisfy markdownlint.
Proposed diff
-```
+```text
User: /analyze-modifiers@@
- +text
User: /analyze-modifiers for the Button component
@@
-```
+```text
User: /analyze-modifiers find where pf-m-disabled is used
@@
- +text
User: /analyze-modifiers compare pf-m-expanded and pf-m-collapsed
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| User: /analyze-modifiers | |
| ``` | |
| Expected: Create a complete listing of all modifiers across all components, organized alphabetically by modifier name. | |
| ### Example 2: Analyze specific component | |
| ``` | |
| User: /analyze-modifiers for the Button component | |
| ``` | |
| Expected: List all modifiers used in the Button component with brief descriptions of what they do. | |
| ### Example 3: Find modifier usage | |
| ``` | |
| User: /analyze-modifiers find where pf-m-disabled is used | |
| ``` | |
| Expected: List all components that use the pf-m-disabled modifier and show the CSS properties it applies in each context. | |
| ### Example 4: Compare modifiers | |
| ``` | |
| User: /analyze-modifiers compare pf-m-expanded and pf-m-collapsed | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/pf-design-tokens/skills/analyze-modifiers/SKILL.md` around lines 47 -
67, The fenced code blocks showing the example commands (e.g., "User:
/analyze-modifiers", "User: /analyze-modifiers for the Button component", "User:
/analyze-modifiers find where pf-m-disabled is used", "User: /analyze-modifiers
compare pf-m-expanded and pf-m-collapsed") need language identifiers to satisfy
markdownlint MD040; update each triple-backtick fence in SKILL.md to include a
language token (for example `text`) so the blocks read ```text before the
command and ``` after.
srambach
force-pushed
the
analyze-modifiers-skill
branch
from
45da3bc to
8cfe5a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/pf-styling/skills/analyze-modifiers/SKILL.md (1)
114-114: Consider capitalizing "Markdown" as a proper noun.The formatting language is typically written as "Markdown" (capitalized) rather than "markdown" when referring to the language itself.
📝 Optional capitalization fix
-- Use proper markdown formatting for readability +- Use proper Markdown formatting for readability🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/pf-styling/skills/analyze-modifiers/SKILL.md` at line 114, Change the lowercase term "markdown" to the proper noun "Markdown" in the SKILL.md text snippet (the line that reads "Use proper markdown formatting for readability") so the language name is capitalized consistently; update any other occurrences of "markdown" in this document to "Markdown" for consistency and readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/pf-styling/skills/analyze-modifiers/SKILL.md`:
- Around line 1-191: Update the analyze-modifiers skill description (name:
analyze-modifiers) to add a "Use when..." trigger context exactly as: "Use when
analyzing component styling patterns, documenting modifier usage, or auditing
CSS consistency." Replace tool-specific Step 2 language ("Use Glob... Use
Grep...") and any other tool-prescriptive phrasing in Steps 2–5 with
outcome-focused instructions such as "Locate all SCSS files containing component
modifiers and extract modifier patterns from selectors and comments" and
similarly rephrase Step 3–5 to describe the expected outcomes (extract CSS
properties, organize results by modifier or component, and generate markdown
outputs) without mentioning specific tools or commands.
- Around line 1-191: The skill violates the PatternFly naming convention: update
the frontmatter name field from "analyze-modifiers" to "pf-analyze-modifiers",
rename the skill directory from plugins/pf-styling/skills/analyze-modifiers/ to
plugins/pf-styling/skills/pf-analyze-modifiers/, and update all user-facing
invocation examples that reference "/pf-styling:analyze-modifiers" to
"/pf-styling:pf-analyze-modifiers" (update the four example occurrences in the
markdown); ensure the frontmatter name, directory name, and invocation strings
all match "pf-analyze-modifiers".
---
Nitpick comments:
In `@plugins/pf-styling/skills/analyze-modifiers/SKILL.md`:
- Line 114: Change the lowercase term "markdown" to the proper noun "Markdown"
in the SKILL.md text snippet (the line that reads "Use proper markdown
formatting for readability") so the language name is capitalized consistently;
update any other occurrences of "markdown" in this document to "Markdown" for
consistency and readability.
🪄 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: 94026349-79dc-4355-997d-3754a9fe65a7
📒 Files selected for processing (1)
plugins/pf-styling/skills/analyze-modifiers/SKILL.md
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md (2)
3-3:⚠️ Potential issue | 🟠 MajorAdd explicit “Use when …” trigger context to the description.
The description still states capability but not trigger conditions; this weakens skill auto-selection behavior.
As per coding guidelines,
CONTRIBUTING-SKILLS.mdrequires descriptions to include trigger context (“Use when …”) so the assistant can decide when to load the skill from description alone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md` at line 3, The skill's metadata description (the description field in SKILL.md) lacks an explicit trigger phrase; update the description string "Find, list, and summarize PatternFly component modifiers (pf-m- classes) across the codebase" to append a "Use when ..." sentence that states the trigger/context (for example: "Use when scanning code for PatternFly utility modifier classes or when asked to inventory pf-m- classes across the repo"), so automated skill selection can detect when to load this skill.
80-82:⚠️ Potential issue | 🟠 MajorRephrase Step 2 to outcome-focused, tool-agnostic instructions.
“Use Glob” / “Use Grep” is implementation-prescriptive. Keep the step focused on the expected result (locate SCSS files and extract modifier patterns), not specific tools.
As per coding guidelines,
CONTRIBUTING-SKILLS.mdsays to “Describe outcomes, not implementation” and that skills must be tool-agnostic across environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md` around lines 80 - 82, Update Step 2 in SKILL.md to be outcome-focused and tool-agnostic: replace the implementation-prescriptive phrasing “Use Grep to extract modifier patterns: `&\.pf-m-[\w-]+|\.pf-m-[\w-]+`” with a description of the expected outcome (e.g., find and list all modifier occurrences matching the pattern &.pf-m-... or .pf-m-... within each SCSS file) so the step describes the result—extracting modifier patterns like &.pf-m-[name] and .pf-m-[name] and associating them with their component—without mentioning specific tools or commands; keep the existing regex/pattern as the matching criteria but present it as the pattern to locate rather than a required tool to use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md`:
- Line 3: The skill's metadata description (the description field in SKILL.md)
lacks an explicit trigger phrase; update the description string "Find, list, and
summarize PatternFly component modifiers (pf-m- classes) across the codebase" to
append a "Use when ..." sentence that states the trigger/context (for example:
"Use when scanning code for PatternFly utility modifier classes or when asked to
inventory pf-m- classes across the repo"), so automated skill selection can
detect when to load this skill.
- Around line 80-82: Update Step 2 in SKILL.md to be outcome-focused and
tool-agnostic: replace the implementation-prescriptive phrasing “Use Grep to
extract modifier patterns: `&\.pf-m-[\w-]+|\.pf-m-[\w-]+`” with a description of
the expected outcome (e.g., find and list all modifier occurrences matching the
pattern &.pf-m-... or .pf-m-... within each SCSS file) so the step describes the
result—extracting modifier patterns like &.pf-m-[name] and .pf-m-[name] and
associating them with their component—without mentioning specific tools or
commands; keep the existing regex/pattern as the matching criteria but present
it as the pattern to locate rather than a required tool to use.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d779bae4-df68-467e-a2b8-0d562a137f25
📒 Files selected for processing (1)
plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md`:
- Line 112: In the SKILL.md file update the phrase in the output-quality
guidance that currently uses the lowercase word "markdown" to the proper noun
"Markdown" for consistency; search for the string "output-quality guidance" or
the exact lowercase "markdown" instance in SKILL.md and replace it with
"Markdown" so the documentation uses the correct capitalization.
🪄 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: 842e0f9f-4453-4ec9-acd5-d9d5253df5b4
📒 Files selected for processing (1)
plugins/pf-styling/skills/pf-analyze-modifiers/SKILL.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
|
||
| ## Instructions for the Assistant | ||
|
|
||
| ### Step 1: Determine Scope |
There was a problem hiding this comment.
Steps 1-5 here plus the "Output Quality Guidelines", "Edge Cases", "Error Handling", and "Success Criteria" sections are all telling the AI how to do its job. Per CONTRIBUTING-SKILLS.md: "Describe outcomes, not implementation." The pf-unit-test-generator covers a comparable task in 69 lines by trusting the AI. Could this be trimmed to the output format, modifier categories reference, and usage examples?
jpuzz0
left a comment
jpuzz0
left a comment
There was a problem hiding this comment.
The 1 comment left I consider non-blocking - if you want to address it, feel free, if not, we can merge as-is.
2 participants
*Note: I believe this belongs in a new plugin for core/CSS, not in the design-tokens plugin, but since one doesn't exist I'll put it here for now.
Summary
Use Cases
Test plan
/analyze-modifierson the PatternFly codebase/analyze-modifiers for Button/analyze-modifiers find pf-m-disabled🤖 Generated with Claude Code
Summary by CodeRabbit