feat: Add WordPress block theme development skills and utilities#281
feat: Add WordPress block theme development skills and utilities#281
Conversation
…ities and usage details
…ck Pattern Generator
…ck Pattern Generator
… validation and auto-fix capabilities
… Mapper including usage, capabilities, and mapping process
…th detailed blockTypes requirements and validation processes
…ation checks for fontFamily attributes, font size classes, and non-WordPress comments
…with new validation rules for navigation blocks and font size handling
…d image attribute handling and validation rules
…d image attribute handling and validation rules
- New script to format PHP files in the inc/ folder - Adds namespace, removes dp_ prefix from function names - Updates add_action/add_filter to use __NAMESPACE__ . '\function_name' feat: introduce Spacing Mapper for Die Papier to Ollie migration - New script to scan theme files for spacing preset references - Maps spacing from Die Papier slugs to Ollie slugs - Supports both var:preset|spacing|40 and var(--wp--preset--spacing--40) formats
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Changelog & Release Documentation .github/changelog.md, CHANGELOG.md |
Initial changelog entry for v0.1.0 release documenting new WordPress skills, documentation artifacts, and CLI tools. |
Phase Completion & Alignment Reports .github/skills/PHASE-1-COMPLETION-REPORT.md, .github/skills/PHASE-2-COMPLETION-REPORT.md, .github/skills/PHASE-3-COMPLETION-REPORT.md, .github/skills/SKILL-ALIGNMENT-RECOMMENDATIONS.md, .github/skills/PROPOSED-DESCRIPTION-REFINEMENTS.md |
Structural and content optimization reports detailing skills directory reorganization, YAML frontmatter standardization, context-window reduction via progressive disclosure, and description refinements with intent keywords. |
Skills System Documentation .github/skills/README.md, .github/skills/theme-json-to-preset-folders/SKILL.md, .github/skills/wordpress-block-pattern-generator/SKILL.md, .github/skills/wordpress-block-pattern-validator/SKILL.md, .github/skills/wordpress-theme-json-mapper/SKILL.md, .github/skills/spacing-mapper/SKILL.md, .github/skills/theme-inc-folder-formatter/SKILL.md |
High-level skill documentation introducing CLI tools, workflows, capabilities, validation requirements, and usage examples for WordPress theme transformations. |
Reference & Guide Documentation .github/skills/spacing-mapper/references/*, .github/skills/wordpress-block-pattern-generator/references/*, .github/skills/wordpress-block-pattern-validator/references/*, .github/skills/wordpress-theme-json-mapper/references/*, .github/skills/theme-inc-folder-formatter/references/* |
Detailed reference guides covering migration workflows, validation rules, conversion patterns, setup procedures, and bug fix reports for each skill. |
Evaluation Queries .github/skills/spacing-mapper/eval-queries.json, .github/skills/wordpress-block-pattern-generator/eval-queries.json, .github/skills/wordpress-block-pattern-validator/eval-queries.json, .github/skills/wordpress-theme-json-mapper/eval-queries.json, .github/skills/theme-inc-folder-formatter/eval-queries.json, .github/skills/theme-json-to-preset-folders/eval-queries.json |
JSON datasets defining natural-language trigger patterns and scope boundaries for skill activation (e.g., spacing migration, pattern validation, theme.json modularization). |
CLI Tools & Scripts .github/skills/spacing-mapper/scripts/spacing-mapper.cjs, .github/skills/wordpress-block-pattern-validator/scripts/validate-patterns.cjs, .github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs |
Executable Node.js CLI tools for spacing preset mapping, WordPress block pattern validation, and PHP inc-folder namespace/function-prefix modernization with scanning, transformation, and reporting capabilities. |
Refactor Report bin/footer-sections-refactor-report.md |
Documentation of footer contact/accreditations sections refactor to block-style usage with theme.json tokens, including validation results and testing checklist. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~30 minutes
Suggested labels
status:needs-review, documentation, lang:javascript, js, lang:php, area:theme, area:block-editor, test
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | The PR description is comprehensive and well-structured with clear sections covering overview, skills added, utilities, documentation, and changes. However, it lacks the formal required sections from the template (Linked issues details, Risk Assessment, How to Test, and Checklist). | Add formal PR template sections: complete 'Linked issues' section, provide 'Risk Assessment' with level/impact/mitigation, add 'How to Test' with prerequisites/steps/expected results, and include checklist for DoD items. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and concisely summarizes the main change: adding WordPress block theme development skills and utilities. It directly reflects the primary objective of the PR. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/basic-agent-skills
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.
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment @coderabbitai help to get the list of available commands and usage tips.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f29e1c9f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| const oldPattern = this.buildPattern(patternType, match.oldSlug); | ||
| const newPattern = this.buildPattern(patternType, match.newSlug); | ||
| content = content.replace(new RegExp(oldPattern, 'g'), newPattern); |
There was a problem hiding this comment.
buildPattern() returns literal strings like var:preset|spacing|40, but this code wraps them in new RegExp(...) without escaping regex metacharacters. For pipe-format matches, the | operators are treated as alternation, so --update replaces unrelated substrings (var:preset, spacing, 40) and corrupts file contents instead of doing a targeted slug substitution.
Useful? React with 👍 / 👎.
| if (line === 'endif;' || line.startsWith('endif;')) { | ||
| // This is an endif that needs to be removed | ||
| // (since we removed all the if ( ! function_exists...) statements) | ||
| linesToRemove.push(i); |
There was a problem hiding this comment.
After detecting any function_exists wrapper, this loop removes every line beginning with endif; in the file, not just the closing token for the wrapper being removed. In files that also use legitimate alternative-syntax if (...) : ... endif; blocks, formatting deletes required endif; statements and leaves invalid PHP.
Useful? React with 👍 / 👎.
| const fixedHtml = this.fixHtmlTag( | ||
| error.htmlLine, | ||
| error.expectedClasses, | ||
| error.expectedStyles |
There was a problem hiding this comment.
validateFile() records non-wordpress-comment issues without htmlLine/htmlLineNumber, but fixFile() assumes those fields exist for every error and unconditionally calls fixHtmlTag(error.htmlLine, ...). Running with --fix on a file containing a normal HTML comment therefore throws a TypeError and aborts validation.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new skills toolkit for WordPress theme development, including tools for spacing migration, PHP formatting, theme.json modularization, and block pattern generation/validation. The review identified critical issues in the inc-formatter.cjs script where the endif; removal logic was overly aggressive and ignored identified orphaned lines, as well as a regex escaping bug in spacing-mapper.cjs that could lead to incorrect string replacements.
| // Now remove orphaned endif; statements | ||
| // Split content into lines and look for standalone endif; | ||
| const lines = content.split('\n'); | ||
| const linesToRemove = []; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i].trim(); | ||
|
|
||
| // Check if this is a standalone endif; (possibly with comment) | ||
| if (line === 'endif;' || line.startsWith('endif;')) { | ||
| // This is an endif that needs to be removed | ||
| // (since we removed all the if ( ! function_exists...) statements) | ||
| linesToRemove.push(i); | ||
| } | ||
| } | ||
|
|
||
| // Remove endifs in reverse order to preserve line indices | ||
| for (let i = linesToRemove.length - 1; i >= 0; i--) { | ||
| lines.splice(linesToRemove[i], 1); | ||
| changeCount++; | ||
| } | ||
|
|
||
| content = lines.join('\n'); | ||
| } |
There was a problem hiding this comment.
The logic to remove endif; statements here is too aggressive. After removing the if ( ! function_exists(...) ) : wrappers, this code proceeds to remove all endif; statements from the file. This is problematic because it can break other legitimate if (...): ... endif; control structures that are not related to the function wrappers.
A safer approach would be to specifically identify and remove only the endif; statements that correspond to the wrappers being removed. This would likely require a more sophisticated parsing mechanism to correctly pair if and endif statements.
| if (analysis.orphanedEndifs && analysis.orphanedEndifs.length > 0) { | ||
| const lines = content.split('\n'); | ||
| const linesToRemove = []; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i].trim(); | ||
| if (line === 'endif;' || line.startsWith('endif;')) { | ||
| linesToRemove.push(i); | ||
| } | ||
| } | ||
|
|
||
| // Remove in reverse order | ||
| for (let i = linesToRemove.length - 1; i >= 0; i--) { | ||
| lines.splice(linesToRemove[i], 1); | ||
| changeCount++; | ||
| } | ||
|
|
||
| content = lines.join('\n'); | ||
| } |
There was a problem hiding this comment.
This logic for cleaning up orphaned endif; statements is too aggressive. It removes all endif; statements in the file if any orphans are detected, rather than only removing the specific orphaned statements identified during the analysis phase. The analysis.orphanedEndifs array, which contains the line numbers of the orphaned statements, is not being used here. This can break legitimate if(...): ... endif; control structures in the code.
To fix this, you should use the analysis.orphanedEndifs array to target only the orphaned lines for removal.
if (analysis.orphanedEndifs && analysis.orphanedEndifs.length > 0) {
const lines = content.split('\n');
// analysis.orphanedEndifs contains 1-based line numbers. Convert to 0-based indices and sort descending.
const indicesToRemove = analysis.orphanedEndifs.map(line => line - 1).sort((a, b) => b - a);
// Remove in reverse order to preserve indices
indicesToRemove.forEach(index => {
lines.splice(index, 1);
changeCount++;
});
content = lines.join('\n');
}
| Object.entries(matchesByType).forEach(([patternType, typeMatches]) => { | ||
| typeMatches.forEach(match => { | ||
| if (!match.hasDirectMapping) { | ||
| if (this.options.includeSuggestions && match.suggestion) { | ||
| // Only update if suggestions are enabled | ||
| const oldPattern = this.buildPattern(patternType, match.oldSlug); | ||
| const newPattern = this.buildPattern(patternType, match.suggestion); | ||
| content = content.replace(new RegExp(oldPattern, 'g'), newPattern); | ||
| replacements++; | ||
| } | ||
| // Skip if no direct mapping and suggestions disabled | ||
| } else { | ||
| const oldPattern = this.buildPattern(patternType, match.oldSlug); | ||
| const newPattern = this.buildPattern(patternType, match.newSlug); | ||
| content = content.replace(new RegExp(oldPattern, 'g'), newPattern); | ||
| replacements++; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The updateFile function has a bug where it creates a regular expression from a string that may contain special regex characters (e.g., |, (, )), without escaping them. This will cause the replacement to fail or have unintended consequences.
For example, new RegExp('var:preset|spacing|40', 'g') will match var:preset OR spacing OR 40, which is not the intended behavior. You should escape the pattern string before creating the RegExp.
I'd recommend adding a helper function to escape regex characters and using it before creating the RegExp.
const escapeRegExp = (str) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
Object.entries(matchesByType).forEach(([patternType, typeMatches]) => {
typeMatches.forEach(match => {
if (!match.hasDirectMapping) {
if (this.options.includeSuggestions && match.suggestion) {
// Only update if suggestions are enabled
const oldPattern = this.buildPattern(patternType, match.oldSlug);
const newPattern = this.buildPattern(patternType, match.suggestion);
content = content.replace(new RegExp(escapeRegExp(oldPattern), 'g'), newPattern);
replacements++;
}
// Skip if no direct mapping and suggestions disabled
} else {
const oldPattern = this.buildPattern(patternType, match.oldSlug);
const newPattern = this.buildPattern(patternType, match.newSlug);
content = content.replace(new RegExp(escapeRegExp(oldPattern), 'g'), newPattern);
replacements++;
}
});
});
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (16)
.github/skills/spacing-mapper.cjs-43-47 (1)
43-47:⚠️ Potential issue | 🟠 MajorKeep
70as a manual-review case.
.github/skills/SPACING-MIGRATION.md:58-62documents70as a choice betweenx-largeandxx-large, but this map hard-codesx-large. With--include-suggestions, the tool will auto-apply one side of a documented judgement call and can shift spacing unexpectedly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/spacing-mapper.cjs around lines 43 - 47, The SPACING_SUGGESTIONS object incorrectly hard-codes a suggestion for the '70' key; remove or neutralize that entry in SPACING_SUGGESTIONS so '70' remains a manual-review case (e.g., delete the '70' property or set its value to a sentinel like null/'manual-review') to prevent --include-suggestions from auto-applying one side of the documented judgement; update only the SPACING_SUGGESTIONS constant (referenced by the '70' key) so other mappings remain unchanged..github/skills/inc-formatter.cjs-317-348 (1)
317-348:⚠️ Potential issue | 🟠 MajorRename ordinary
dp_call sites as well.Only declarations,
function_exists()checks, and string hook callbacks are rewritten. Direct calls likedp_helper()remain untouched, so renamed helpers can still be invoked under the old name at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/inc-formatter.cjs around lines 317 - 348, The code misses rewriting direct call sites (e.g., dp_helper()) so renamed helpers still work under the old name; add a new step (after the "Remove prefix from function declarations" loop that iterates analysis.prefixedFunctions) to replace plain function-call occurrences by matching word-boundary function names followed by a parenthesis: for each func in analysis.prefixedFunctions create a RegExp like new RegExp(`\\b${func.oldName}\\s*\\(`, 'g') and replace with `${func.newName}(` (and increment changeCount); this ensures bare calls are updated in addition to the existing declaration, function_exists, and hook-string rewrites referenced by analysis.prefixedFunctions, changeCount and analysis.hookCalls..github/skills/inc-formatter.cjs-1-2 (1)
1-2:⚠️ Potential issue | 🟠 MajorDeclare the Node environment for this CLI.
ESLint is currently treating
consoleandprocessas undefined throughout the file, so this script will fail lint unless the file or config declares Node globals.Suggested fix
+/* eslint-env node */ #!/usr/bin/env node🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/inc-formatter.cjs around lines 1 - 2, Add an ESLint environment declaration for Node at the top of the CLI file so Node globals (console, process) are recognized; insert a comment like /* eslint-env node */ immediately after the existing shebang (#!/usr/bin/env node) in .github/skills/inc-formatter.cjs to enable Node globals for the whole file..github/skills/spacing-mapper.cjs-1-2 (1)
1-2:⚠️ Potential issue | 🟠 MajorDeclare the Node environment for this CLI.
ESLint is flagging
consoleandprocessas undefined here too, so the new script will fail lint without Node globals.Suggested fix
+/* eslint-env node */ #!/usr/bin/env node🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/spacing-mapper.cjs around lines 1 - 2, The script starts with the shebang "#!/usr/bin/env node" but lacks an ESLint Node environment declaration, so ESLint flags Node globals like console and process as undefined; fix by adding an ESLint Node environment directive immediately after the shebang (or alternatively declare console and process as globals) so Node globals are recognized when linting this CLI file..github/skills/wordpress-block-pattern-generator/SKILL.md-495-505 (1)
495-505:⚠️ Potential issue | 🟠 MajorRemove Tailwind utility class recommendations from generic WordPress Block Pattern guidance.
The skill explicitly recommends Tailwind utilities in lines 496-497 (
top-0,z-50) and in the "Additional Sticky Positioning Considerations" section (lines 518-521), which recommendsz-50,shadow-sm, and "responsive Tailwind classes." Since this repo doesn't ship Tailwind and the guidance is framed as generic WordPress Block Pattern documentation, these recommendations create a false contract that generated patterns require Tailwind. Replace Tailwind class examples with WordPress-native alternatives (e.g., recommend inlinez-indexstyles or WordPress theme class conventions instead ofz-50).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-generator/SKILL.md around lines 495 - 505, The document currently recommends Tailwind utilities (e.g., top-0, z-50, shadow-sm and "responsive Tailwind classes") in the example block and the "Additional Sticky Positioning Considerations" section; remove those Tailwind-specific class recommendations and replace them with WordPress-native guidance: keep the sticky positioning in both block attributes and inline style (`"style":{"position":{"type":"sticky","top":"0px"}}` and `style="position:sticky;top:0px"`), but suggest using inline z-index (e.g., `style="z-index:100;"`) or a theme class naming convention (e.g., `.site-header--sticky`) and note using theme-provided shadow utilities or inline box-shadow styles instead of Tailwind classes; update all mentions of top-0, z-50, shadow-sm, and "responsive Tailwind classes" accordingly..github/skills/inc-formatter.cjs-225-249 (1)
225-249:⚠️ Potential issue | 🟠 MajorHandle
declare()statements when inserting the namespace.The code inserts
namespaceimmediately after<?phpand any docblock, but per PHP specification,declare()statements must come before the namespace declaration. Valid PHP syntax order is:<?php→declare()→namespace. If this formatter processes files withdeclare(strict_types=1);or similar directives, it will produce invalid syntax by placing the namespace before the declare statement.While no files with declare statements currently exist in this repository, the formatter should handle this case correctly to avoid breakage on future PHP files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/inc-formatter.cjs around lines 225 - 249, The namespace insertion currently places `namespace ${NAMESPACE};` immediately after `<?php` and any docblock (using phpTag, phpTagIndex, afterPhpTag, docBlockMatch, insertPosition), which will break files that include a `declare(...)` statement; modify the logic to detect and skip a leading `declare(...)` block (and any trailing semicolon/newlines) when computing insertPosition so the code inserts the namespace after the declare statement rather than before it; if an existing namespace exists (namespaceMatch) still replace it as before, otherwise locate a declare regex (e.g. /^\s*declare\s*\(.*?\)\s*;/m) in afterPhpTag and set insertPosition to after that block (or after docblock if no declare) before injecting `namespace ${NAMESPACE};`..github/skills/SPACING-MIGRATION.md-74-119 (2)
74-119:⚠️ Potential issue | 🟠 MajorFix broken CLI example paths (
spacing-mapper.js→spacing-mapper.cjs).Several command examples in this doc reference
node scripts/spacing-mapper.js ..., but the PR context/implementation appears to bespacing-mapper.cjs. As written, users who follow the guide without creating a.jssymlink/rename will likely hit “file not found”.🛠️ Proposed doc fix
- node scripts/spacing-mapper.js --scan ./ + node scripts/spacing-mapper.cjs --scan ./ ... - node scripts/spacing-mapper.js --update ./ --dry-run + node scripts/spacing-mapper.cjs --update ./ --dry-run ... - node scripts/spacing-mapper.js --update ./ + node scripts/spacing-mapper.cjs --update ./ ... - node scripts/spacing-mapper.js --update ./ --include-suggestions + node scripts/spacing-mapper.cjs --update ./ --include-suggestions ... - node scripts/spacing-mapper.js --help + node scripts/spacing-mapper.cjs --help ... - node scripts/spacing-mapper.js --map + node scripts/spacing-mapper.cjs --map ... - node scripts/spacing-mapper.js --scan ./ --verbose + node scripts/spacing-mapper.cjs --scan ./ --verbose ... - node scripts/spacing-mapper.js --update ./patterns/ + node scripts/spacing-mapper.cjs --update ./patterns/If you intentionally expect a
spacing-mapper.jssymlink, add a short note next to the commands explaining that renaming is required.Also applies to: 148-167, 176-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/SPACING-MIGRATION.md around lines 74 - 119, The CLI examples in the doc reference spacing-mapper.js which doesn't exist in the PR; update every example invocation to use spacing-mapper.cjs (e.g., change "node scripts/spacing-mapper.js --scan ./", "--update ./ --dry-run", and similar lines to "node scripts/spacing-mapper.cjs ...") or, if you intend to keep the .js name, add a one-line note explaining a required symlink/rename before running the commands; ensure all occurrences (including the other blocks called out) are fixed consistently and keep the example flags unchanged.
74-119:⚠️ Potential issue | 🟠 MajorFix broken CLI example paths (
spacing-mapper.js→spacing-mapper.cjs).Several command examples in this doc reference
node scripts/spacing-mapper.js ..., but the PR context/implementation appears to bespacing-mapper.cjs. As written, users who follow the guide without creating a.jssymlink/rename will likely hit “file not found”.🛠️ Proposed doc fix
- node scripts/spacing-mapper.js --scan ./ + node scripts/spacing-mapper.cjs --scan ./ ... - node scripts/spacing-mapper.js --update ./ --dry-run + node scripts/spacing-mapper.cjs --update ./ --dry-run ... - node scripts/spacing-mapper.js --update ./ + node scripts/spacing-mapper.cjs --update ./ ... - node scripts/spacing-mapper.js --update ./ --include-suggestions + node scripts/spacing-mapper.cjs --update ./ --include-suggestions ... - node scripts/spacing-mapper.js --help + node scripts/spacing-mapper.cjs --help ... - node scripts/spacing-mapper.js --map + node scripts/spacing-mapper.cjs --map ... - node scripts/spacing-mapper.js --scan ./ --verbose + node scripts/spacing-mapper.cjs --scan ./ --verbose ... - node scripts/spacing-mapper.js --update ./patterns/ + node scripts/spacing-mapper.cjs --update ./patterns/If you intentionally expect a
spacing-mapper.jssymlink, add a short note next to the commands explaining that renaming is required.Also applies to: 148-167, 176-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/SPACING-MIGRATION.md around lines 74 - 119, Replace the incorrect CLI examples that call "node scripts/spacing-mapper.js" with the actual script name "node scripts/spacing-mapper.cjs" (or add a short note that a .js symlink must be created) wherever the document references spacing-mapper.js (e.g., the command examples in this section and the other ranges noted); update every occurrence of spacing-mapper.js to spacing-mapper.cjs in the examples and add one brief parenthetical note after the first command explaining that the .cjs filename is the shipped artifact or that a .js symlink is required if you intentionally expect the .js name..github/skills/SPACING-MAPPER-USAGE.md-13-39 (2)
13-39:⚠️ Potential issue | 🟠 MajorFix broken CLI example paths (
spacing-mapper.js→spacing-mapper.cjs).The quick commands in this doc reference
node scripts/spacing-mapper.js ...multiple times. Given the implementation/tooling isspacing-mapper.cjs, these examples are likely to break for users who don’t rename/symlink the file.🛠️ Proposed doc fix
- node scripts/spacing-mapper.js --map + node scripts/spacing-mapper.cjs --map ... - node scripts/spacing-mapper.js --scan ./ + node scripts/spacing-mapper.cjs --scan ./ ... - node scripts/spacing-mapper.js --scan ./styles/presets + node scripts/spacing-mapper.cjs --scan ./styles/presets ... - node scripts/spacing-mapper.js --update ./ --dry-run + node scripts/spacing-mapper.cjs --update ./ --dry-run ... - node scripts/spacing-mapper.js --update ./ + node scripts/spacing-mapper.cjs --update ./ ... - node scripts/spacing-mapper.js --update ./ --include-suggestions + node scripts/spacing-mapper.cjs --update ./ --include-suggestions ... - node scripts/spacing-mapper.js --scan ./ --verbose + node scripts/spacing-mapper.cjs --scan ./ --verbose ... - node scripts/spacing-mapper.js --help + node scripts/spacing-mapper.cjs --help🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/SPACING-MAPPER-USAGE.md around lines 13 - 39, Update the CLI examples that reference spacing-mapper.js to use spacing-mapper.cjs instead: replace all occurrences of "node scripts/spacing-mapper.js" with "node scripts/spacing-mapper.cjs" in the doc so the commands match the actual tool filename (scan, --map, --update, --dry-run, --include-suggestions, --verbose, --help); search for any remaining references to spacing-mapper.js and update them to spacing-mapper.cjs to ensure examples work as-is.
13-39:⚠️ Potential issue | 🟠 MajorFix broken CLI example paths (
spacing-mapper.js→spacing-mapper.cjs).The quick commands in this doc reference
node scripts/spacing-mapper.js ...multiple times. Given the implementation/tooling isspacing-mapper.cjs, these examples are likely to break for users who don’t rename/symlink the file.🛠️ Proposed doc fix
- node scripts/spacing-mapper.js --map + node scripts/spacing-mapper.cjs --map ... - node scripts/spacing-mapper.js --scan ./ + node scripts/spacing-mapper.cjs --scan ./ ... - node scripts/spacing-mapper.js --scan ./styles/presets + node scripts/spacing-mapper.cjs --scan ./styles/presets ... - node scripts/spacing-mapper.js --update ./ --dry-run + node scripts/spacing-mapper.cjs --update ./ --dry-run ... - node scripts/spacing-mapper.js --update ./ + node scripts/spacing-mapper.cjs --update ./ ... - node scripts/spacing-mapper.js --update ./ --include-suggestions + node scripts/spacing-mapper.cjs --update ./ --include-suggestions ... - node scripts/spacing-mapper.js --scan ./ --verbose + node scripts/spacing-mapper.cjs --scan ./ --verbose ... - node scripts/spacing-mapper.js --help + node scripts/spacing-mapper.cjs --help🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/SPACING-MAPPER-USAGE.md around lines 13 - 39, Update the CLI examples to point to the actual script filename: replace all occurrences of spacing-mapper.js with spacing-mapper.cjs in the examples so commands like node scripts/spacing-mapper.cjs --map, --scan, --update, --dry-run, --include-suggestions, --verbose and --help work without requiring renames or symlinks; ensure every example line in the file that currently references spacing-mapper.js (the examples block) is updated to spacing-mapper.cjs..github/skills/INC-FORMATTER.md-12-28 (2)
12-28:⚠️ Potential issue | 🟠 MajorFix broken CLI example paths (
inc-formatter.js→inc-formatter.cjs).This doc’s Quick Start uses
node scripts/inc-formatter.js ..., but the skill implementation referenced elsewhere isinc-formatter.cjs. Users following these commands directly from a theme directory will likely fail unless they renamed/aliased the file.🛠️ Proposed doc fix
- node scripts/inc-formatter.js --scan inc/ + node scripts/inc-formatter.cjs --scan inc/ - node scripts/inc-formatter.js --format inc/ --dry-run + node scripts/inc-formatter.cjs --format inc/ --dry-run - node scripts/inc-formatter.js --format inc/ + node scripts/inc-formatter.cjs --format inc/ - node scripts/inc-formatter.js --format inc/block-bindings.php + node scripts/inc-formatter.cjs --format inc/block-bindings.phpIf you want to keep
.js, add an explicit note that users must create ainc-formatter.jssymlink/rename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/INC-FORMATTER.md around lines 12 - 28, Update the Quick Start CLI examples to reference the actual script name used by the project: replace all occurrences of "node scripts/inc-formatter.js ..." with "node scripts/inc-formatter.cjs ...", or alternatively add a short note explaining that users must create a symlink/rename from inc-formatter.js to inc-formatter.cjs if they prefer the .js name; ensure the examples shown in the "Quick Start" block (the four node commands) and any adjacent examples use the corrected "inc-formatter.cjs" symbol so commands work out-of-the-box.
12-28:⚠️ Potential issue | 🟠 MajorFix broken CLI example paths (
inc-formatter.js→inc-formatter.cjs).This doc’s Quick Start uses
node scripts/inc-formatter.js ..., but the skill implementation referenced elsewhere isinc-formatter.cjs. Users following these commands directly from a theme directory will likely fail unless they renamed/aliased the file.🛠️ Proposed doc fix
- node scripts/inc-formatter.js --scan inc/ + node scripts/inc-formatter.cjs --scan inc/ - node scripts/inc-formatter.js --format inc/ --dry-run + node scripts/inc-formatter.cjs --format inc/ --dry-run - node scripts/inc-formatter.js --format inc/ + node scripts/inc-formatter.cjs --format inc/ - node scripts/inc-formatter.js --format inc/block-bindings.php + node scripts/inc-formatter.cjs --format inc/block-bindings.phpIf you want to keep
.js, add an explicit note that users must create ainc-formatter.jssymlink/rename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/INC-FORMATTER.md around lines 12 - 28, Update the Quick Start examples in .github/skills/INC-FORMATTER.md to reference the actual script filename by replacing all occurrences of "inc-formatter.js" with "inc-formatter.cjs" in the shown CLI commands (e.g., the node scripts/inc-formatter.* calls), or alternatively add a one-line note after the examples explaining that users must create a symlink/rename "inc-formatter.js" to "inc-formatter.cjs" if they want to keep the .js name; ensure the text mentions both "inc-formatter.cjs" and "inc-formatter.js" so readers understand the mapping..github/skills/wordpress-theme-json-mapper/SKILL.md-72-99 (1)
72-99:⚠️ Potential issue | 🟠 MajorRemove or correct the non-existent schema reference
.github/schemas/theme.6.9.json.The schema file referenced in the theme.json examples does not exist in the repository. The
.github/schemas/directory contains only: changelog.schema.json, coderabbit-overrides.v2.json, frontmatter.schema.json, project-fields.schema.json, and version.schema.json. Users following these examples will be unable to validate their theme.json files using the specified$schemapath, and any generated theme.json that uses this reference will fail validation.This issue appears in two locations:
- Lines 72-99: Theme.json Schema example
- Lines 351-366: Step 6 validation requirements
Either create the missing schema file or remove the
$schemafield from the documentation examples.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-theme-json-mapper/SKILL.md around lines 72 - 99, The theme.json example currently references a non-existent $schema path ".github/schemas/theme.6.9.json"; update the documentation by removing or correcting that $schema field in the Theme.json Schema example and the Step 6 validation requirements (the example blocks around "Theme.json Schema" and the Step 6 validation text), either by deleting the "$schema" line from the JSON snippets or replacing it with an existing schema file reference (e.g., one of the repo schemas like version.schema.json) so the examples match available schemas and validate correctly..github/skills/wordpress-block-pattern-validator/SKILL.md-194-349 (1)
194-349:⚠️ Potential issue | 🟠 MajorThe skill spec promises checks the validator does not implement yet.
These sections describe background-image conflict detection,
style.shadow,style.position, and Cover child-order validation as supported/critical checks, but the implementation in.github/skills/wordpress-block-pattern-validator/validate-patterns.cjsonly derives spacing/border/color/background-image/typography expectations and never inspects shadow, position, conflicting background attributes, or Cover child ordering. A clean validation report would therefore overstate what the tool actually guarantees. Either narrow the documented scope here or add the missing validations.Also applies to: 494-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-validator/SKILL.md around lines 194 - 349, The spec documents validations (background-image vs backgroundColor/gradient conflicts, style.shadow inline-presence rules, style.position inline-presence and offsets, and Cover child-order) that are not implemented in validate-patterns.cjs; either shrink the doc or implement them: in the module (look for deriveExpectations and the main validatePattern/validateBlockPattern function) add checks that 1) detect style.background.backgroundImage and error if backgroundColor or gradient is also present, 2) if style.shadow exists require a matching inline box-shadow unless className contains "pattern-" or "section-" (warn for those), 3) if style.position exists require matching inline position and matching offset properties (top/bottom/left/right), and 4) enforce Cover block child-order (ensure the expected child element order for cover blocks); update validation reports to only claim checks that are actually implemented..github/skills/wordpress-block-pattern-validator/validate-patterns.cjs-37-49 (1)
37-49:⚠️ Potential issue | 🟠 MajorValid WordPress block comments are skipped by the parser.
This regex only matches
<!-- wp:block {…} -->, so common valid forms like<!-- wp:group -->and<!-- wp:navigation {...} /-->returnnulland bypass validation entirely. That creates silent false negatives across normal block markup.Possible fix
- const match = line.match(/<!--\s+wp:([a-z/-]+)\s+({.*?})\s+-->/); + const match = line.match(/<!--\s+wp:([a-z0-9-]+(?:\/[a-z0-9-]+)?)\s*(\{.*?\})?\s*(\/)?-->/); if (!match) return null; try { return { blockType: match[1], - attributes: JSON.parse(match[2]), + attributes: match[2] ? JSON.parse(match[2]) : {}, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs around lines 37 - 49, parseBlockComment currently only matches comments with an attributes object, causing valid forms like "<!-- wp:group -->" and self-closing blocks to be skipped; update the parsing in parseBlockComment to use a regex that allows the attributes JSON to be optional and also tolerates an optional self-closing slash before the closing "-->", then only attempt JSON.parse on the attributes capture if it exists (and still catch parse errors), so blockType is always returned for bare blocks while attributes remain null/undefined when not present..github/skills/wordpress-block-pattern-validator/validate-patterns.cjs-1-2 (1)
1-2:⚠️ Potential issue | 🟠 MajorAdd Node.js environment globals to the ESLint configuration for
.cjsfiles.The
.cjsconfiguration ineslint.config.cjs(lines 109-124) is missing Node.js globals likeconsoleandprocess, while both.jsand.mjsconfigurations include them. The file usesconsoleextensively, so it will fail linting without this configuration. Either add/* eslint-env node */as a file-level pragma, or update the.cjsconfiguration to include Node.js globals matching the.jsconfiguration (lines 183-212).Possible fix (file-level pragma)
#!/usr/bin/env node +/* eslint-env node */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs around lines 1 - 2, The file validate-patterns.cjs (starts with the shebang "#!/usr/bin/env node") is missing Node.js globals for ESLint; add a file-level pragma "/* eslint-env node */" immediately after the shebang so console and process are recognized, or alternatively update the .cjs section in eslint.config.cjs to include Node environment globals (matching the .js/.mjs config) so console/process are allowed during linting.
🟡 Minor comments (2)
bin/footer-sections-refactor-report.md-97-122 (1)
97-122:⚠️ Potential issue | 🟡 MinorThe placeholder format is inconsistent.
This section says SVG placeholders were created, but every listed asset is a
.png. That makes the follow-up instructions ambiguous and can send replacement work to the wrong file type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/footer-sections-refactor-report.md` around lines 97 - 122, The asset documentation incorrectly states "SVG placeholders" while listing files with .png extensions (press-council-sa.png, sanef.png, abc.png, ifj.png); update the text to make the format consistent by either (A) renaming the listed assets to .svg if they are SVG placeholders, or (B) changing the descriptive text and the "Recommended Format" section to indicate PNG placeholders—ensure the filenames (press-council-sa.png, sanef.png, abc.png, ifj.png) match the described placeholder type and clarify the replacement requirements (file format, transparency, min width, aspect ratio)..github/skills/wordpress-theme-json-mapper/SKILL.md-72-99 (1)
72-99:⚠️ Potential issue | 🟡 MinorReplace the non-existent schema path with the official WordPress schema URL.
The documentation references
.github/schemas/theme.6.9.jsonat two locations (lines ~75 and ~357), but this file does not exist in the repository. The.github/schemas/directory contains only:changelog.schema.json,coderabbit-overrides.v2.json,frontmatter.schema.json,project-fields.schema.json, andversion.schema.json.Change both occurrences to use the official WordPress schema:
https://schemas.wp.org/wp/6.9/theme.json(as correctly shown elsewhere in the document at line 106).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-theme-json-mapper/SKILL.md around lines 72 - 99, Replace the invalid local schema reference ".github/schemas/theme.6.9.json" with the official WordPress schema URL "https://schemas.wp.org/wp/6.9/theme.json" in both occurrences in the document (the two places shown in the diff), ensuring the JSON block header and any other schema references use the official URL exactly as elsewhere in the file.
🧹 Nitpick comments (7)
.github/skills/wordpress-block-pattern-generator.md (1)
322-329: Make the related-doc references actual links.These are plain code spans right now, so readers cannot navigate them and link validation cannot catch drift. Convert them to relative Markdown links. As per coding guidelines, "Prefer relative links within the repo" and "Validate all documentation links after editing to ensure they remain valid."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-generator.md around lines 322 - 329, The related-doc entries are plain code spans and should be converted to relative Markdown links so they are navigable and validateable; update the list items referencing `/pattern-specification.json`, `/.github/instructions/block-theme-development.instructions.md`, `/.github/instructions/a11y.instructions.md`, `/.github/instructions/naming-conventions.instructions.md`, and `/guidelines/design-tokens/spacing.md` into standard Markdown link syntax (e.g., [Pattern Specification](/pattern-specification.json)) keeping the same display text, ensuring paths remain relative and re-run link validation after editing.CHANGELOG.md (1)
17-31: Keep the Unreleased section on the standard changelog headings.
DocumentationandUtilitiesmake the Unreleased block drift away from the required Keep a Changelog structure. Fold these bullets intoAddedorChangedso the section stays consistent. As per coding guidelines, "CHANGELOG.md must follow Keep a Changelog format with sections for Added, Changed, Fixed, and Removed."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 17 - 31, The Unreleased section currently uses non-standard headings "Documentation" and "Utilities"; update the CHANGELOG.md so the Unreleased block uses the standard Keep a Changelog headings (e.g., Added, Changed, Fixed, Removed) by folding the listed bullets under the appropriate section (move the items under "Documentation" and "Utilities" into "Added" or "Changed" as appropriate), remove the custom "Documentation" and "Utilities" headings, and ensure the Unreleased header remains present and the entries keep their current wording..github/skills/INC-FORMATTER-BUGFIX-REPORT.md (1)
1-9: Add frontmatter and a TOC to this new report.This report starts straight at the H1, and it is already long enough to benefit from a table of contents. Adding the repo's required YAML metadata now will keep it consistent with the other documentation automation around these skills. As per coding guidelines, "Use YAML frontmatter in documentation files following the schema defined in .github/schemas/frontmatter.schema.json" and "Include a Table of Contents for Markdown files longer than approximately 100 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/INC-FORMATTER-BUGFIX-REPORT.md around lines 1 - 9, Add YAML frontmatter at the top of INC-FORMATTER-BUGFIX-REPORT.md that conforms to the repository schema (.github/schemas/frontmatter.schema.json) including required fields (e.g., title, date, status, authors, tags) and place it before the existing H1; then insert a Table of Contents immediately below the frontmatter (and before the H1) that lists the major headings in the document so readers can jump to sections—ensure the frontmatter keys match the schema and the TOC reflects the file's headings exactly..github/skills/theme-json-to-preset-folders/SKILL.md (1)
1-12: Add frontmatter to the new skill document.This file is missing the required YAML metadata block, which will make schema validation and documentation discovery harder. It is also long enough that a small TOC would pay off. As per coding guidelines, "Use YAML frontmatter in documentation files following the schema defined in .github/schemas/frontmatter.schema.json" and "Include a Table of Contents for Markdown files longer than approximately 100 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/theme-json-to-preset-folders/SKILL.md around lines 1 - 12, Add a YAML frontmatter block to the top of the "Theme JSON To Preset Folders Skill" document (the file starting with the "Theme JSON To Preset Folders Skill" header) that conforms to the schema defined in .github/schemas/frontmatter.schema.json (include title, description, tags, author, and any required fields), and insert a short Table of Contents after the frontmatter for the document (since it exceeds ~100 lines) linking to main sections like Description, Goal, Use Cases, and Steps so schema validation and navigation work correctly..github/skills/wordpress-block-pattern-generator/SKILL.md (1)
1-6: Add the required Markdown metadata block.This new skill doc starts at the H1 and skips the YAML frontmatter the repo expects. Given the file length, a TOC would also help navigation. As per coding guidelines, "Always include complete YAML frontmatter with title, description, last_updated, and owners fields" and "Include a Table of Contents for Markdown files longer than approximately 100 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-generator/SKILL.md around lines 1 - 6, The document is missing the required YAML frontmatter and a Table of Contents: add a YAML metadata block at the very top containing title: "WordPress Block Pattern Generator Skill", description (short summary), last_updated (ISO date), and owners (list of maintainers), then move the existing H1 below that block; if the file grows beyond ~100 lines (or already is), insert a Table of Contents after the intro paragraph linking to major sections to improve navigation. Ensure the frontmatter is valid YAML and that the H1 text "WordPress Block Pattern Generator Skill" remains unchanged..github/skills/SPACING-MAPPER-USAGE.md (2)
41-57: Clarify whether the “Current Theme Status” numbers are example vs real scan output.Lines 43-57 present exact counts (e.g., “126 spacing references across 32 files”, “96 references can be auto-updated safely”). If these aren’t produced dynamically from the user’s scan, mark them explicitly as an example output to avoid stale/confusing guidance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/SPACING-MAPPER-USAGE.md around lines 41 - 57, The "Current Theme Status" section currently shows fixed numeric counts (e.g., "126 spacing references across 32 files") that may be mistaken for real scan output; update the .github/skills/SPACING-MAPPER-USAGE.md content under the "Current Theme Status" heading to either: 1) clearly label these lines as example/sample output (add a one-line prefix like "Example output:"), or 2) replace the hardcoded numbers with a brief note directing readers to run the scan tool to get live counts. Reference the "Current Theme Status" heading and the listed numeric bullets when making this change so the documentation no longer presents static numbers as real scan results.
41-57: Clarify whether the “Current Theme Status” numbers are example vs real scan output.Lines 43-57 present exact counts (e.g., “126 spacing references across 32 files”, “96 references can be auto-updated safely”). If these aren’t produced dynamically from the user’s scan, mark them explicitly as an example output to avoid stale/confusing guidance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/SPACING-MAPPER-USAGE.md around lines 41 - 57, The "Current Theme Status" section shows exact counts but doesn't indicate whether these are live scan results or static examples; update the "Current Theme Status" block (the header and its bullet list) to clearly state if the numbers are example/sample output or dynamically populated, e.g., add a parenthetical like "(example output — actual scan results will vary)" or replace the hardcoded numbers with template placeholders and a clarifying sentence such as "Values below are example results from a sample scan" so readers won't be misled by stale counts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/skills/spacing-mapper.cjs:
- Around line 186-205: The literals returned by buildPattern(...) (used when
iterating matchesByType and inside the includeSuggestions/hasDirectMapping
branches) are passed directly to new RegExp(oldPattern, 'g') which treats
characters like |, (, ) as regex operators; escape the literal before
constructing the RegExp (e.g., use a small helper to replace
/[.*+?^${}()|[\]\\]/g with '\\$&' or similar) and use the escaped string when
calling RegExp for both the suggestion branch (where match.suggestion/oldSlug
are used) and the direct-mapping branch (where match.newSlug/oldSlug are used)
so content.replace(new RegExp(..., 'g'), ...) matches the exact token.
In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs:
- Around line 130-159: The color class logic is applying text/background color
classes to the wrapper for buttons; update the handling so attributes.textColor
and attributes.backgroundColor only push color classes when isButtonLink is true
(i.e., treat them as button-link-only) and skip adding those classes in the
generic path for the wrapper; move or duplicate the class.push calls for
`has-${attributes.textColor}-color`, `has-text-color`,
`has-${attributes.backgroundColor}-background-color`, and `has-background` into
the button-link branch that renders the inner <a> (use the existing isButtonLink
check and classes array) and remove them from the unconditional generic color
block to avoid reporting missing wrapper classes.
- Around line 254-309: The button-check currently inspects only the single line
passed into validateBlock(), so when the <a> is on the next line the inner link
is never seen and fixes are generated only for the wrapper; update
validateFile() and/or validateBlock() so button validation has multi-line
context and return link expectations: when block.blockType === 'button' in
validateBlock(), if html.tagName === 'div' locate the following line(s) (from
validateFile's input) to parse the inner <a> (use parseHtmlTag or extend it to
accept a string with multiple lines), validate classes using
generateExpectedClasses(block.blockType, block.attributes, true, false) for the
link, add any missingClasses and misplaced-style warnings for the link, and
ensure the returned expectedClasses/expectedStyles include both wrapper and
inner-link expectations so --fix can modify the anchor as well; adjust
validateFile() to pass adjacent lines or provide an API (e.g., a second
parameter) to validateBlock() so it can access the next HTML line when needed.
- Around line 549-562: The fixFile function is calling fixHtmlTag with
error.htmlLine without guarding against diagnostics that lack
htmlLine/htmlLineNumber (produced by validateFile like the
non-wordpress-comment), causing parseHtmlTag to receive undefined and throw;
update fixFile to skip errors that are not fixable by checking that
error.htmlLine and error.htmlLineNumber (or a boolean like error.fixable) exist
before calling fixHtmlTag, or branch on error.code to invoke dedicated fix
handlers for non-html diagnostics (referencing fixFile, fixHtmlTag,
parseHtmlTag, validateFile).
---
Major comments:
In @.github/skills/inc-formatter.cjs:
- Around line 317-348: The code misses rewriting direct call sites (e.g.,
dp_helper()) so renamed helpers still work under the old name; add a new step
(after the "Remove prefix from function declarations" loop that iterates
analysis.prefixedFunctions) to replace plain function-call occurrences by
matching word-boundary function names followed by a parenthesis: for each func
in analysis.prefixedFunctions create a RegExp like new
RegExp(`\\b${func.oldName}\\s*\\(`, 'g') and replace with `${func.newName}(`
(and increment changeCount); this ensures bare calls are updated in addition to
the existing declaration, function_exists, and hook-string rewrites referenced
by analysis.prefixedFunctions, changeCount and analysis.hookCalls.
- Around line 1-2: Add an ESLint environment declaration for Node at the top of
the CLI file so Node globals (console, process) are recognized; insert a comment
like /* eslint-env node */ immediately after the existing shebang
(#!/usr/bin/env node) in .github/skills/inc-formatter.cjs to enable Node globals
for the whole file.
- Around line 225-249: The namespace insertion currently places `namespace
${NAMESPACE};` immediately after `<?php` and any docblock (using phpTag,
phpTagIndex, afterPhpTag, docBlockMatch, insertPosition), which will break files
that include a `declare(...)` statement; modify the logic to detect and skip a
leading `declare(...)` block (and any trailing semicolon/newlines) when
computing insertPosition so the code inserts the namespace after the declare
statement rather than before it; if an existing namespace exists
(namespaceMatch) still replace it as before, otherwise locate a declare regex
(e.g. /^\s*declare\s*\(.*?\)\s*;/m) in afterPhpTag and set insertPosition to
after that block (or after docblock if no declare) before injecting `namespace
${NAMESPACE};`.
In @.github/skills/INC-FORMATTER.md:
- Around line 12-28: Update the Quick Start CLI examples to reference the actual
script name used by the project: replace all occurrences of "node
scripts/inc-formatter.js ..." with "node scripts/inc-formatter.cjs ...", or
alternatively add a short note explaining that users must create a
symlink/rename from inc-formatter.js to inc-formatter.cjs if they prefer the .js
name; ensure the examples shown in the "Quick Start" block (the four node
commands) and any adjacent examples use the corrected "inc-formatter.cjs" symbol
so commands work out-of-the-box.
- Around line 12-28: Update the Quick Start examples in
.github/skills/INC-FORMATTER.md to reference the actual script filename by
replacing all occurrences of "inc-formatter.js" with "inc-formatter.cjs" in the
shown CLI commands (e.g., the node scripts/inc-formatter.* calls), or
alternatively add a one-line note after the examples explaining that users must
create a symlink/rename "inc-formatter.js" to "inc-formatter.cjs" if they want
to keep the .js name; ensure the text mentions both "inc-formatter.cjs" and
"inc-formatter.js" so readers understand the mapping.
In @.github/skills/SPACING-MAPPER-USAGE.md:
- Around line 13-39: Update the CLI examples that reference spacing-mapper.js to
use spacing-mapper.cjs instead: replace all occurrences of "node
scripts/spacing-mapper.js" with "node scripts/spacing-mapper.cjs" in the doc so
the commands match the actual tool filename (scan, --map, --update, --dry-run,
--include-suggestions, --verbose, --help); search for any remaining references
to spacing-mapper.js and update them to spacing-mapper.cjs to ensure examples
work as-is.
- Around line 13-39: Update the CLI examples to point to the actual script
filename: replace all occurrences of spacing-mapper.js with spacing-mapper.cjs
in the examples so commands like node scripts/spacing-mapper.cjs --map, --scan,
--update, --dry-run, --include-suggestions, --verbose and --help work without
requiring renames or symlinks; ensure every example line in the file that
currently references spacing-mapper.js (the examples block) is updated to
spacing-mapper.cjs.
In @.github/skills/spacing-mapper.cjs:
- Around line 43-47: The SPACING_SUGGESTIONS object incorrectly hard-codes a
suggestion for the '70' key; remove or neutralize that entry in
SPACING_SUGGESTIONS so '70' remains a manual-review case (e.g., delete the '70'
property or set its value to a sentinel like null/'manual-review') to prevent
--include-suggestions from auto-applying one side of the documented judgement;
update only the SPACING_SUGGESTIONS constant (referenced by the '70' key) so
other mappings remain unchanged.
- Around line 1-2: The script starts with the shebang "#!/usr/bin/env node" but
lacks an ESLint Node environment declaration, so ESLint flags Node globals like
console and process as undefined; fix by adding an ESLint Node environment
directive immediately after the shebang (or alternatively declare console and
process as globals) so Node globals are recognized when linting this CLI file.
In @.github/skills/SPACING-MIGRATION.md:
- Around line 74-119: The CLI examples in the doc reference spacing-mapper.js
which doesn't exist in the PR; update every example invocation to use
spacing-mapper.cjs (e.g., change "node scripts/spacing-mapper.js --scan ./",
"--update ./ --dry-run", and similar lines to "node scripts/spacing-mapper.cjs
...") or, if you intend to keep the .js name, add a one-line note explaining a
required symlink/rename before running the commands; ensure all occurrences
(including the other blocks called out) are fixed consistently and keep the
example flags unchanged.
- Around line 74-119: Replace the incorrect CLI examples that call "node
scripts/spacing-mapper.js" with the actual script name "node
scripts/spacing-mapper.cjs" (or add a short note that a .js symlink must be
created) wherever the document references spacing-mapper.js (e.g., the command
examples in this section and the other ranges noted); update every occurrence of
spacing-mapper.js to spacing-mapper.cjs in the examples and add one brief
parenthetical note after the first command explaining that the .cjs filename is
the shipped artifact or that a .js symlink is required if you intentionally
expect the .js name.
In @.github/skills/wordpress-block-pattern-generator/SKILL.md:
- Around line 495-505: The document currently recommends Tailwind utilities
(e.g., top-0, z-50, shadow-sm and "responsive Tailwind classes") in the example
block and the "Additional Sticky Positioning Considerations" section; remove
those Tailwind-specific class recommendations and replace them with
WordPress-native guidance: keep the sticky positioning in both block attributes
and inline style (`"style":{"position":{"type":"sticky","top":"0px"}}` and
`style="position:sticky;top:0px"`), but suggest using inline z-index (e.g.,
`style="z-index:100;"`) or a theme class naming convention (e.g.,
`.site-header--sticky`) and note using theme-provided shadow utilities or inline
box-shadow styles instead of Tailwind classes; update all mentions of top-0,
z-50, shadow-sm, and "responsive Tailwind classes" accordingly.
In @.github/skills/wordpress-block-pattern-validator/SKILL.md:
- Around line 194-349: The spec documents validations (background-image vs
backgroundColor/gradient conflicts, style.shadow inline-presence rules,
style.position inline-presence and offsets, and Cover child-order) that are not
implemented in validate-patterns.cjs; either shrink the doc or implement them:
in the module (look for deriveExpectations and the main
validatePattern/validateBlockPattern function) add checks that 1) detect
style.background.backgroundImage and error if backgroundColor or gradient is
also present, 2) if style.shadow exists require a matching inline box-shadow
unless className contains "pattern-" or "section-" (warn for those), 3) if
style.position exists require matching inline position and matching offset
properties (top/bottom/left/right), and 4) enforce Cover block child-order
(ensure the expected child element order for cover blocks); update validation
reports to only claim checks that are actually implemented.
In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs:
- Around line 37-49: parseBlockComment currently only matches comments with an
attributes object, causing valid forms like "<!-- wp:group -->" and self-closing
blocks to be skipped; update the parsing in parseBlockComment to use a regex
that allows the attributes JSON to be optional and also tolerates an optional
self-closing slash before the closing "-->", then only attempt JSON.parse on the
attributes capture if it exists (and still catch parse errors), so blockType is
always returned for bare blocks while attributes remain null/undefined when not
present.
- Around line 1-2: The file validate-patterns.cjs (starts with the shebang
"#!/usr/bin/env node") is missing Node.js globals for ESLint; add a file-level
pragma "/* eslint-env node */" immediately after the shebang so console and
process are recognized, or alternatively update the .cjs section in
eslint.config.cjs to include Node environment globals (matching the .js/.mjs
config) so console/process are allowed during linting.
In @.github/skills/wordpress-theme-json-mapper/SKILL.md:
- Around line 72-99: The theme.json example currently references a non-existent
$schema path ".github/schemas/theme.6.9.json"; update the documentation by
removing or correcting that $schema field in the Theme.json Schema example and
the Step 6 validation requirements (the example blocks around "Theme.json
Schema" and the Step 6 validation text), either by deleting the "$schema" line
from the JSON snippets or replacing it with an existing schema file reference
(e.g., one of the repo schemas like version.schema.json) so the examples match
available schemas and validate correctly.
---
Minor comments:
In @.github/skills/wordpress-theme-json-mapper/SKILL.md:
- Around line 72-99: Replace the invalid local schema reference
".github/schemas/theme.6.9.json" with the official WordPress schema URL
"https://schemas.wp.org/wp/6.9/theme.json" in both occurrences in the document
(the two places shown in the diff), ensuring the JSON block header and any other
schema references use the official URL exactly as elsewhere in the file.
In `@bin/footer-sections-refactor-report.md`:
- Around line 97-122: The asset documentation incorrectly states "SVG
placeholders" while listing files with .png extensions (press-council-sa.png,
sanef.png, abc.png, ifj.png); update the text to make the format consistent by
either (A) renaming the listed assets to .svg if they are SVG placeholders, or
(B) changing the descriptive text and the "Recommended Format" section to
indicate PNG placeholders—ensure the filenames (press-council-sa.png, sanef.png,
abc.png, ifj.png) match the described placeholder type and clarify the
replacement requirements (file format, transparency, min width, aspect ratio).
---
Nitpick comments:
In @.github/skills/INC-FORMATTER-BUGFIX-REPORT.md:
- Around line 1-9: Add YAML frontmatter at the top of
INC-FORMATTER-BUGFIX-REPORT.md that conforms to the repository schema
(.github/schemas/frontmatter.schema.json) including required fields (e.g.,
title, date, status, authors, tags) and place it before the existing H1; then
insert a Table of Contents immediately below the frontmatter (and before the H1)
that lists the major headings in the document so readers can jump to
sections—ensure the frontmatter keys match the schema and the TOC reflects the
file's headings exactly.
In @.github/skills/SPACING-MAPPER-USAGE.md:
- Around line 41-57: The "Current Theme Status" section currently shows fixed
numeric counts (e.g., "126 spacing references across 32 files") that may be
mistaken for real scan output; update the .github/skills/SPACING-MAPPER-USAGE.md
content under the "Current Theme Status" heading to either: 1) clearly label
these lines as example/sample output (add a one-line prefix like "Example
output:"), or 2) replace the hardcoded numbers with a brief note directing
readers to run the scan tool to get live counts. Reference the "Current Theme
Status" heading and the listed numeric bullets when making this change so the
documentation no longer presents static numbers as real scan results.
- Around line 41-57: The "Current Theme Status" section shows exact counts but
doesn't indicate whether these are live scan results or static examples; update
the "Current Theme Status" block (the header and its bullet list) to clearly
state if the numbers are example/sample output or dynamically populated, e.g.,
add a parenthetical like "(example output — actual scan results will vary)" or
replace the hardcoded numbers with template placeholders and a clarifying
sentence such as "Values below are example results from a sample scan" so
readers won't be misled by stale counts.
In @.github/skills/theme-json-to-preset-folders/SKILL.md:
- Around line 1-12: Add a YAML frontmatter block to the top of the "Theme JSON
To Preset Folders Skill" document (the file starting with the "Theme JSON To
Preset Folders Skill" header) that conforms to the schema defined in
.github/schemas/frontmatter.schema.json (include title, description, tags,
author, and any required fields), and insert a short Table of Contents after the
frontmatter for the document (since it exceeds ~100 lines) linking to main
sections like Description, Goal, Use Cases, and Steps so schema validation and
navigation work correctly.
In @.github/skills/wordpress-block-pattern-generator.md:
- Around line 322-329: The related-doc entries are plain code spans and should
be converted to relative Markdown links so they are navigable and validateable;
update the list items referencing `/pattern-specification.json`,
`/.github/instructions/block-theme-development.instructions.md`,
`/.github/instructions/a11y.instructions.md`,
`/.github/instructions/naming-conventions.instructions.md`, and
`/guidelines/design-tokens/spacing.md` into standard Markdown link syntax (e.g.,
[Pattern Specification](/pattern-specification.json)) keeping the same display
text, ensuring paths remain relative and re-run link validation after editing.
In @.github/skills/wordpress-block-pattern-generator/SKILL.md:
- Around line 1-6: The document is missing the required YAML frontmatter and a
Table of Contents: add a YAML metadata block at the very top containing title:
"WordPress Block Pattern Generator Skill", description (short summary),
last_updated (ISO date), and owners (list of maintainers), then move the
existing H1 below that block; if the file grows beyond ~100 lines (or already
is), insert a Table of Contents after the intro paragraph linking to major
sections to improve navigation. Ensure the frontmatter is valid YAML and that
the H1 text "WordPress Block Pattern Generator Skill" remains unchanged.
In `@CHANGELOG.md`:
- Around line 17-31: The Unreleased section currently uses non-standard headings
"Documentation" and "Utilities"; update the CHANGELOG.md so the Unreleased block
uses the standard Keep a Changelog headings (e.g., Added, Changed, Fixed,
Removed) by folding the listed bullets under the appropriate section (move the
items under "Documentation" and "Utilities" into "Added" or "Changed" as
appropriate), remove the custom "Documentation" and "Utilities" headings, and
ensure the Unreleased header remains present and the entries keep their current
wording.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: 709b5a5d-63df-4030-a6da-ffc6b2e42f85
📒 Files selected for processing (18)
.github/changelog.md.github/skills/INC-FORMATTER-BUGFIX-REPORT.md.github/skills/INC-FORMATTER.md.github/skills/README.md.github/skills/SPACING-MAPPER-USAGE.md.github/skills/SPACING-MIGRATION.md.github/skills/inc-formatter.cjs.github/skills/spacing-mapper.cjs.github/skills/theme-json-to-preset-folders/SKILL.md.github/skills/wordpress-block-pattern-generator.md.github/skills/wordpress-block-pattern-generator/SKILL.md.github/skills/wordpress-block-pattern-validator/README.md.github/skills/wordpress-block-pattern-validator/SKILL.md.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs.github/skills/wordpress-theme-json-mapper/README.md.github/skills/wordpress-theme-json-mapper/SKILL.mdCHANGELOG.mdbin/footer-sections-refactor-report.md
| // Process each pattern type | ||
| Object.entries(matchesByType).forEach(([patternType, typeMatches]) => { | ||
| typeMatches.forEach(match => { | ||
| if (!match.hasDirectMapping) { | ||
| if (this.options.includeSuggestions && match.suggestion) { | ||
| // Only update if suggestions are enabled | ||
| const oldPattern = this.buildPattern(patternType, match.oldSlug); | ||
| const newPattern = this.buildPattern(patternType, match.suggestion); | ||
| content = content.replace(new RegExp(oldPattern, 'g'), newPattern); | ||
| replacements++; | ||
| } | ||
| // Skip if no direct mapping and suggestions disabled | ||
| } else { | ||
| const oldPattern = this.buildPattern(patternType, match.oldSlug); | ||
| const newPattern = this.buildPattern(patternType, match.newSlug); | ||
| content = content.replace(new RegExp(oldPattern, 'g'), newPattern); | ||
| replacements++; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Demonstrate why the current literal-to-RegExp conversion is unsafe,
# then show the affected lines in the script.
python - <<'PY'
import re
sample = 'var:preset|spacing|40'
print("pattern:", sample)
print("result :", re.sub(sample, 'X', sample))
PY
rg -n "new RegExp\\(oldPattern, 'g'\\)|buildPattern\\(" .github/skills/spacing-mapper.cjsRepository: lightspeedwp/.github
Length of output: 615
🏁 Script executed:
# Find and examine the buildPattern method
rg -A 15 "buildPattern\(patternType, slug\)" .github/skills/spacing-mapper.cjsRepository: lightspeedwp/.github
Length of output: 405
🏁 Script executed:
# Check lines 223-233 for similar patterns
sed -n '223,233p' .github/skills/spacing-mapper.cjs | cat -nRepository: lightspeedwp/.github
Length of output: 426
🏁 Script executed:
# Search entire file for any other RegExp constructions or pattern usage
rg -n "new RegExp|\.replace\(" .github/skills/spacing-mapper.cjs | head -20Repository: lightspeedwp/.github
Length of output: 221
Escape literal patterns before building RegExp.
buildPattern() returns literal strings like var:preset|spacing|40 and var(--wp--preset--spacing--${slug}), but new RegExp(oldPattern, 'g') on lines 194 and 201 interprets |, (, and ) as regex operators. The pipe-format branch will match and replace var:preset, spacing, and 40 separately instead of the full token; the CSS variable branch will treat parentheses as capture groups instead of literals.
The fix requires escaping the literal string before passing it to RegExp(). Use a helper like pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') or JavaScript's RegExp.escape() if available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/spacing-mapper.cjs around lines 186 - 205, The literals
returned by buildPattern(...) (used when iterating matchesByType and inside the
includeSuggestions/hasDirectMapping branches) are passed directly to new
RegExp(oldPattern, 'g') which treats characters like |, (, ) as regex operators;
escape the literal before constructing the RegExp (e.g., use a small helper to
replace /[.*+?^${}()|[\]\\]/g with '\\$&' or similar) and use the escaped string
when calling RegExp for both the suggestion branch (where
match.suggestion/oldSlug are used) and the direct-mapping branch (where
match.newSlug/oldSlug are used) so content.replace(new RegExp(..., 'g'), ...)
matches the exact token.
| // Text color | ||
| if (attributes.textColor) { | ||
| classes.push(`has-${attributes.textColor}-color`); | ||
| classes.push('has-text-color'); | ||
| } | ||
|
|
||
| // Background color | ||
| if (attributes.backgroundColor) { | ||
| classes.push(`has-${attributes.backgroundColor}-background-color`); | ||
| classes.push('has-background'); | ||
| } | ||
|
|
||
| // Text alignment | ||
| if (attributes.textAlign) { | ||
| classes.push(`has-text-align-${attributes.textAlign}`); | ||
| } | ||
|
|
||
| // Font size | ||
| // SPECIAL CASE: Button links always get font size classes, even for default (base) | ||
| // Wrapper divs only get them for non-default sizes | ||
| if (attributes.fontSize) { | ||
| if (isButtonLink || attributes.fontSize !== 'base') { | ||
| classes.push(`has-${attributes.fontSize}-font-size`); | ||
| } | ||
| } | ||
|
|
||
| // Custom font size indicator (appears when fontSize is set) | ||
| if (attributes.fontSize && isButtonLink) { | ||
| classes.push('has-custom-font-size'); | ||
| } |
There was a problem hiding this comment.
Button color classes are being validated on the wrong element.
For wp:button, WordPress applies textColor / backgroundColor classes to the inner <a>, not the wrapper <div>. Because the generic color logic runs for both button branches, valid buttons will be reported as missing wrapper classes. Keep these classes button-link-only instead of applying the shared path unconditionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs
around lines 130 - 159, The color class logic is applying text/background color
classes to the wrapper for buttons; update the handling so attributes.textColor
and attributes.backgroundColor only push color classes when isButtonLink is true
(i.e., treat them as button-link-only) and skip adding those classes in the
generic path for the wrapper; move or duplicate the class.push calls for
`has-${attributes.textColor}-color`, `has-text-color`,
`has-${attributes.backgroundColor}-background-color`, and `has-background` into
the button-link branch that renders the inner <a> (use the existing isButtonLink
check and classes array) and remove them from the unconditional generic color
block to avoid reporting missing wrapper classes.
| // SPECIAL CASE: Button blocks have two parts to validate: | ||
| // 1. Wrapper div: <div class="wp-block-button is-style-*"> | ||
| // 2. Inner link: <a class="wp-block-button__link wp-element-button"> | ||
| // Style classes (is-style-*) belong on the wrapper div, NOT the inner link | ||
| if (block.blockType === 'button' && html.tagName === 'div') { | ||
| isButtonWrapper = true; | ||
|
|
||
| // Validate the wrapper div | ||
| const wrapperExpectedClasses = this.generateExpectedClasses(block.blockType, block.attributes, false, true); | ||
| wrapperExpectedClasses.forEach(expectedClass => { | ||
| if (!html.classes.includes(expectedClass)) { | ||
| missingClasses.push(expectedClass); | ||
| } | ||
| }); | ||
|
|
||
| // Try to extract and validate the inner <a> tag | ||
| const innerLinkMatch = htmlTag.match(/<a\s+([^>]*)>/); | ||
| if (innerLinkMatch) { | ||
| const linkHtml = this.parseHtmlTag(innerLinkMatch[0]); | ||
| if (linkHtml) { | ||
| isButtonLink = true; | ||
| const linkExpectedClasses = this.generateExpectedClasses(block.blockType, block.attributes, true, false); | ||
|
|
||
| // Check for missing classes on the link | ||
| linkExpectedClasses.forEach(expectedClass => { | ||
| if (!linkHtml.classes.includes(expectedClass)) { | ||
| missingClasses.push(`${expectedClass} (on inner link)`); | ||
| } | ||
| }); | ||
|
|
||
| // Check if style classes are incorrectly on the inner link | ||
| const styleClassesOnLink = linkHtml.classes.filter(cls => cls.startsWith('is-style-')); | ||
| if (styleClassesOnLink.length > 0) { | ||
| warnings.push({ | ||
| type: 'misplaced-style-class', | ||
| message: `Style classes should be on wrapper div, not inner link: ${styleClassesOnLink.join(', ')}`, | ||
| fix: `Move ${styleClassesOnLink.join(', ')} from <a> to <div class="wp-block-button">`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If there are errors or warnings, return them | ||
| if (missingClasses.length > 0 || warnings.length > 0) { | ||
| const allExpectedClasses = this.generateExpectedClasses(block.blockType, block.attributes, false, true); | ||
| return { | ||
| lineNumber, | ||
| blockType: block.blockType, | ||
| blockComment, | ||
| htmlTag, | ||
| missingClasses, | ||
| missingStyles: [], | ||
| warnings, | ||
| expectedClasses: allExpectedClasses, | ||
| expectedStyles: '', | ||
| currentClasses: html.classes, |
There was a problem hiding this comment.
Button validation never has enough context to validate or fix the inner link.
Line 270 searches the wrapper line for <a>, but validateFile() only passes a single HTML line into validateBlock(). In the normal multiline form (<div> on one line, <a> on the next), the anchor is never seen, so the wrapper-vs-link checks silently do nothing. Even if an inner-link issue were found, the returned expectedClasses only describe the wrapper, so --fix still cannot repair the <a>.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs
around lines 254 - 309, The button-check currently inspects only the single line
passed into validateBlock(), so when the <a> is on the next line the inner link
is never seen and fixes are generated only for the wrapper; update
validateFile() and/or validateBlock() so button validation has multi-line
context and return link expectations: when block.blockType === 'button' in
validateBlock(), if html.tagName === 'div' locate the following line(s) (from
validateFile's input) to parse the inner <a> (use parseHtmlTag or extend it to
accept a string with multiple lines), validate classes using
generateExpectedClasses(block.blockType, block.attributes, true, false) for the
link, add any missingClasses and misplaced-style warnings for the link, and
ensure the returned expectedClasses/expectedStyles include both wrapper and
inner-link expectations so --fix can modify the anchor as well; adjust
validateFile() to pass adjacent lines or provide an API (e.g., a second
parameter) to validateBlock() so it can access the next HTML line when needed.
| fixFile(filePath, validationResult) { | ||
| if (validationResult.errors.length === 0) return false; | ||
|
|
||
| const lines = [...validationResult.lines]; | ||
|
|
||
| // Fix errors in reverse order to maintain line numbers | ||
| validationResult.errors.reverse().forEach(error => { | ||
| const fixedHtml = this.fixHtmlTag( | ||
| error.htmlLine, | ||
| error.expectedClasses, | ||
| error.expectedStyles | ||
| ); | ||
| lines[error.htmlLineNumber - 1] = fixedHtml; | ||
| }); |
There was a problem hiding this comment.
--fix will throw on comment-only findings.
validateFile() creates non-wordpress-comment errors without htmlLine / htmlLineNumber, but fixFile() unconditionally passes error.htmlLine into fixHtmlTag(). Running --fix on a file with one descriptive HTML comment will hit undefined.match(...) in parseHtmlTag() and abort the entire fix pass. Skip non-fixable diagnostics here or add dedicated fix handlers.
Possible guard
// Fix errors in reverse order to maintain line numbers
validationResult.errors.reverse().forEach(error => {
+ if (!error.htmlLine || !error.htmlLineNumber) {
+ return;
+ }
const fixedHtml = this.fixHtmlTag(
error.htmlLine,
error.expectedClasses,
error.expectedStyles
);📝 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.
| fixFile(filePath, validationResult) { | |
| if (validationResult.errors.length === 0) return false; | |
| const lines = [...validationResult.lines]; | |
| // Fix errors in reverse order to maintain line numbers | |
| validationResult.errors.reverse().forEach(error => { | |
| const fixedHtml = this.fixHtmlTag( | |
| error.htmlLine, | |
| error.expectedClasses, | |
| error.expectedStyles | |
| ); | |
| lines[error.htmlLineNumber - 1] = fixedHtml; | |
| }); | |
| fixFile(filePath, validationResult) { | |
| if (validationResult.errors.length === 0) return false; | |
| const lines = [...validationResult.lines]; | |
| // Fix errors in reverse order to maintain line numbers | |
| validationResult.errors.reverse().forEach(error => { | |
| if (!error.htmlLine || !error.htmlLineNumber) { | |
| return; | |
| } | |
| const fixedHtml = this.fixHtmlTag( | |
| error.htmlLine, | |
| error.expectedClasses, | |
| error.expectedStyles | |
| ); | |
| lines[error.htmlLineNumber - 1] = fixedHtml; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/wordpress-block-pattern-validator/validate-patterns.cjs
around lines 549 - 562, The fixFile function is calling fixHtmlTag with
error.htmlLine without guarding against diagnostics that lack
htmlLine/htmlLineNumber (produced by validateFile like the
non-wordpress-comment), causing parseHtmlTag to receive undefined and throw;
update fixFile to skip errors that are not fixable by checking that
error.htmlLine and error.htmlLineNumber (or a boolean like error.fixable) exist
before calling fixHtmlTag, or branch on error.code to invoke dedicated fix
handlers for non-html diagnostics (referencing fixFile, fixHtmlTag,
parseHtmlTag, validateFile).
- Implemented a new WordPress Block Pattern Validator to validate and auto-fix block pattern files, ensuring compliance with WordPress rendering rules. - Added detailed validation rules and usage instructions in VALIDATION-RULES.md. - Created a validation script (validate-patterns.cjs) for checking and fixing block patterns. - Introduced a new WordPress Theme.json Mapper to automate the translation of design tokens into theme.json configuration. - Documented the mapping process, including common mappings for colors, typography, spacing, and layout in SCHEMA-REFERENCE.md.
- Created eval-queries.json for all 6 skills (96 realistic test queries) - Refined 5 skill descriptions with improved triggering keywords - Added 'even if/when' clauses for broader semantic matching - Optimized descriptions per agentskills.io best practices - All descriptions remain under 1024 char limit (max 552 chars) - Added keywords: modernizing, CSS variables, Figma, Tailwind, pattern errors, team collaboration - Created comprehensive Phase 3 completion report Alignment: agentskills.io specification v1.0
- Remove hardcoded DiePapierTema namespace and dp_ prefix - Add auto-detection for namespace from existing PHP files - Add auto-detection for function prefix from existing files - Add CLI options --namespace and --prefix for explicit control - Add interactive prompts when auto-detection unavailable - Update help text to reflect generic WordPress theme usage - Update SKILL.md examples to use generic theme names - Update description to mention auto-detection capability The script now works for any WordPress theme instead of being tied to Die Papier Tema.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
.github/skills/spacing-mapper/scripts/spacing-mapper.cjs (1)
192-201:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winEscape literal spacing tokens before constructing
RegExp.
buildPattern()returns literal tokens, butnew RegExp(oldPattern, 'g')treats|and parentheses as regex operators. This can replace unintended text and corrupt file updates.Proposed fix
+ function escapeRegExp(literal) { + return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + } + // Process each pattern type Object.entries(matchesByType).forEach(([patternType, typeMatches]) => { typeMatches.forEach(match => { if (!match.hasDirectMapping) { if (this.options.includeSuggestions && match.suggestion) { // Only update if suggestions are enabled const oldPattern = this.buildPattern(patternType, match.oldSlug); const newPattern = this.buildPattern(patternType, match.suggestion); - content = content.replace(new RegExp(oldPattern, 'g'), newPattern); + content = content.replace(new RegExp(escapeRegExp(oldPattern), 'g'), newPattern); replacements++; } // Skip if no direct mapping and suggestions disabled } else { const oldPattern = this.buildPattern(patternType, match.oldSlug); const newPattern = this.buildPattern(patternType, match.newSlug); - content = content.replace(new RegExp(oldPattern, 'g'), newPattern); + content = content.replace(new RegExp(escapeRegExp(oldPattern), 'g'), newPattern); replacements++; } }); });#!/bin/bash set -euo pipefail python - <<'PY' import re literal = "var:preset|spacing|40" text = "var:preset|spacing|40" print("unescaped :", re.sub(literal, "X", text)) print("escaped :", re.sub(re.escape(literal), "X", text)) PY rg -n "new RegExp\\(oldPattern, 'g'\\)|buildPattern\\(" .github/skills/spacing-mapper/scripts/spacing-mapper.cjs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/spacing-mapper/scripts/spacing-mapper.cjs around lines 192 - 201, The code passes literal tokens from buildPattern(...) directly into new RegExp(oldPattern, 'g'), which treats characters like | and parentheses as regex meta-characters; fix by escaping the pattern before constructing the RegExp (e.g., add an escapeRegExp helper and call escapeRegExp(oldPattern) when creating new RegExp) and ensure the replacement string is safe by escaping any "$" in newPattern (e.g., newPattern.replace(/\$/g,'$$')) so content.replace(new RegExp(...), newPattern) performs a literal replace; update the occurrences around buildPattern(patternType, match.oldSlug) / buildPattern(patternType, match.suggestion) / buildPattern(patternType, match.newSlug) accordingly.
🧹 Nitpick comments (1)
.github/skills/wordpress-block-pattern-generator/references/CONVERSION-GUIDES.md (1)
287-299: 💤 Low valueDuplicate checklist items in Pattern 2 section.
The Pattern 2 checklist has redundant items:
- "No backgroundColor or gradient attributes present with background image" appears at lines 286 and 294
- "Empty HTML div added..." appears at lines 291 and 296
Consider consolidating these to avoid confusion.
📝 Suggested consolidated checklist
**For Pattern 2 (Advanced effects with SCSS):** - [ ] Background image copied to theme assets directory - [ ] Background properties added to block comment attributes ONLY - [ ] NO background properties in HTML div inline styles - [ ] Empty HTML div added for SCSS to target - [ ] SCSS module created with background + advanced effects - [ ] Image URL uses theme-relative path - [ ] No backgroundColor or gradient attributes present with background image -- [ ] Advanced effects (blend modes, opacity) handled in SCSS -- [ ] Empty HTML div added if overlay effects needed +- [ ] Advanced effects (blend modes, opacity, filters) handled in SCSS - [ ] SCSS module targets overlay div correctly - [ ] Theme rebuilt after SCSS changes🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/skills/wordpress-block-pattern-generator/references/CONVERSION-GUIDES.md around lines 287 - 299, Consolidate duplicate checklist entries under the "Pattern 2 (Advanced effects with SCSS)" heading by removing the repeated lines: eliminate the second occurrence of "No backgroundColor or gradient attributes present with background image" and the duplicate "Empty HTML div added..." entry so each checklist item appears only once; ensure the remaining checklist preserves the intended order and meaning (background image copied..., background properties in block comment attributes ONLY, NO inline styles, Empty HTML div added for SCSS to target, SCSS module created..., Image URL uses theme-relative path, No backgroundColor or gradient attributes present with background image, Advanced effects handled in SCSS, SCSS module targets overlay div correctly, Theme rebuilt after SCSS changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/skills/inc-formatter/references/BUGFIX-REPORT.md:
- Around line 79-103: The documentation and implementation disagree: the new
Step 2.5 in formatFile currently iterates over content and unconditionally
removes every line that equals or starts with 'endif;' when
analysis.orphanedEndifs is truthy, which can delete valid PHP alternative-syntax
endifs; update the code to only remove lines whose indices match actual orphan
locations (use analysis.orphanedEndifs to map to specific line numbers or unique
markers) instead of removing all 'endif;' occurrences, or if you choose to keep
the broad removal, change the docs to state that formatFile will remove all
standalone 'endif;' lines and note the risk; refer to the formatFile function,
analysis.orphanedEndifs, the content variable and changeCount when making the
fix.
In @.github/skills/inc-formatter/scripts/inc-formatter.cjs:
- Around line 134-141: The current regex in the backward scan
(prevLine.trim().match(/^if\s*\([^)]+\)\s*:\s*$/)) fails on nested or multiple
parentheses; update the check in the loop that searches for matching ifs (the
for loop referencing prevLine) to either use a more permissive pattern like
matching any content between parentheses (e.g., /^if\s*\(.*\)\s*:\s*$/) or,
preferably, replace the regex with a simple paren-balancing check: confirm the
line starts with "if", ends with ":" and that the parentheses in the condition
are balanced by iterating characters and tracking open/close counts before
deciding hasMatchingIf = true. Ensure this change is applied to the same loop
that sets hasMatchingIf so nested conditions like if (isset($a) && isset($b)) :
are detected.
In @.github/skills/spacing-mapper/scripts/spacing-mapper.cjs:
- Around line 14-16: Update the usage/help examples in this file to reference
the correct script filename/extension by replacing occurrences of
"scripts/spacing-mapper.js" with "scripts/spacing-mapper.cjs" (e.g., the
top-of-file usage block and any other help/usage strings around lines noted in
the comment); ensure all help output and example commands emitted by functions
or constants that print usage text use "spacing-mapper.cjs" so examples match
the actual script file name.
In @.github/skills/wordpress-block-pattern-generator/SKILL.md:
- Around line 104-106: Update the three anchor links in SKILL.md to match actual
headings: replace `#background-images` with `#background-image-conversion` to
match "Background Image Conversion", replace `#drop-shadows` with
`#drop-shadow-conversion` to match "Drop Shadow Conversion", and either remove
or point `#advanced-effects` to an existing heading (or add an "Advanced
Effects" section in CONVERSION-GUIDES.md) so the
`references/CONVERSION-GUIDES.md#advanced-effects` link is not broken.
In
@.github/skills/wordpress-block-pattern-validator/scripts/validate-patterns.cjs:
- Around line 664-672: The run(targetPath) method currently calls
fs.statSync(targetPath) without handling exceptions; wrap the fs.statSync call
in a try/catch (or pre-check with fs.existsSync) to handle non-existent or
inaccessible paths, and on error either log a clear message and return an empty
files array or rethrow a controlled Error; update references in this function
(run) and ensure behavior remains consistent with scanDirectory by returning []
or producing a user-friendly error instead of letting the raw exception bubble
up.
---
Duplicate comments:
In @.github/skills/spacing-mapper/scripts/spacing-mapper.cjs:
- Around line 192-201: The code passes literal tokens from buildPattern(...)
directly into new RegExp(oldPattern, 'g'), which treats characters like | and
parentheses as regex meta-characters; fix by escaping the pattern before
constructing the RegExp (e.g., add an escapeRegExp helper and call
escapeRegExp(oldPattern) when creating new RegExp) and ensure the replacement
string is safe by escaping any "$" in newPattern (e.g.,
newPattern.replace(/\$/g,'$$')) so content.replace(new RegExp(...), newPattern)
performs a literal replace; update the occurrences around
buildPattern(patternType, match.oldSlug) / buildPattern(patternType,
match.suggestion) / buildPattern(patternType, match.newSlug) accordingly.
---
Nitpick comments:
In
@.github/skills/wordpress-block-pattern-generator/references/CONVERSION-GUIDES.md:
- Around line 287-299: Consolidate duplicate checklist entries under the
"Pattern 2 (Advanced effects with SCSS)" heading by removing the repeated lines:
eliminate the second occurrence of "No backgroundColor or gradient attributes
present with background image" and the duplicate "Empty HTML div added..." entry
so each checklist item appears only once; ensure the remaining checklist
preserves the intended order and meaning (background image copied..., background
properties in block comment attributes ONLY, NO inline styles, Empty HTML div
added for SCSS to target, SCSS module created..., Image URL uses theme-relative
path, No backgroundColor or gradient attributes present with background image,
Advanced effects handled in SCSS, SCSS module targets overlay div correctly,
Theme rebuilt after SCSS changes).
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: c4dee4e0-3a74-4913-8a29-5fe1028fc64b
📒 Files selected for processing (28)
.github/skills/PHASE-1-COMPLETION-REPORT.md.github/skills/PHASE-2-COMPLETION-REPORT.md.github/skills/PHASE-3-COMPLETION-REPORT.md.github/skills/PROPOSED-DESCRIPTION-REFINEMENTS.md.github/skills/SKILL-ALIGNMENT-RECOMMENDATIONS.md.github/skills/inc-formatter/SKILL.md.github/skills/inc-formatter/eval-queries.json.github/skills/inc-formatter/references/BUGFIX-REPORT.md.github/skills/inc-formatter/scripts/inc-formatter.cjs.github/skills/spacing-mapper/SKILL.md.github/skills/spacing-mapper/eval-queries.json.github/skills/spacing-mapper/references/MIGRATION-GUIDE.md.github/skills/spacing-mapper/references/USAGE.md.github/skills/spacing-mapper/scripts/spacing-mapper.cjs.github/skills/theme-json-to-preset-folders/SKILL.md.github/skills/theme-json-to-preset-folders/eval-queries.json.github/skills/wordpress-block-pattern-generator/SKILL.md.github/skills/wordpress-block-pattern-generator/eval-queries.json.github/skills/wordpress-block-pattern-generator/references/CONVERSION-GUIDES.md.github/skills/wordpress-block-pattern-generator/references/SETUP-GUIDE.md.github/skills/wordpress-block-pattern-generator/references/VALIDATION.md.github/skills/wordpress-block-pattern-validator/SKILL.md.github/skills/wordpress-block-pattern-validator/eval-queries.json.github/skills/wordpress-block-pattern-validator/references/VALIDATION-RULES.md.github/skills/wordpress-block-pattern-validator/scripts/validate-patterns.cjs.github/skills/wordpress-theme-json-mapper/SKILL.md.github/skills/wordpress-theme-json-mapper/eval-queries.json.github/skills/wordpress-theme-json-mapper/references/SCHEMA-REFERENCE.md
✅ Files skipped from review due to trivial changes (18)
- .github/skills/inc-formatter/eval-queries.json
- .github/skills/PROPOSED-DESCRIPTION-REFINEMENTS.md
- .github/skills/wordpress-block-pattern-generator/references/SETUP-GUIDE.md
- .github/skills/wordpress-theme-json-mapper/references/SCHEMA-REFERENCE.md
- .github/skills/theme-json-to-preset-folders/eval-queries.json
- .github/skills/spacing-mapper/eval-queries.json
- .github/skills/PHASE-3-COMPLETION-REPORT.md
- .github/skills/spacing-mapper/SKILL.md
- .github/skills/wordpress-theme-json-mapper/SKILL.md
- .github/skills/wordpress-theme-json-mapper/eval-queries.json
- .github/skills/theme-json-to-preset-folders/SKILL.md
- .github/skills/spacing-mapper/references/USAGE.md
- .github/skills/inc-formatter/SKILL.md
- .github/skills/wordpress-block-pattern-generator/eval-queries.json
- .github/skills/wordpress-block-pattern-generator/references/VALIDATION.md
- .github/skills/PHASE-1-COMPLETION-REPORT.md
- .github/skills/wordpress-block-pattern-validator/references/VALIDATION-RULES.md
- .github/skills/wordpress-block-pattern-validator/eval-queries.json
| ### Phase 2: Cleanup (formatFile) | ||
| Added Step 2.5 that removes ALL standalone `endif;` statements when orphaned endifs are detected: | ||
|
|
||
| ```javascript | ||
| // Step 2.5: Clean up orphaned endif; statements | ||
| if (analysis.orphanedEndifs && analysis.orphanedEndifs.length > 0) { | ||
| const lines = content.split('\n'); | ||
| const linesToRemove = []; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i].trim(); | ||
| if (line === 'endif;' || line.startsWith('endif;')) { | ||
| linesToRemove.push(i); | ||
| } | ||
| } | ||
|
|
||
| // Remove in reverse order to preserve indices | ||
| for (let i = linesToRemove.length - 1; i >= 0; i--) { | ||
| lines.splice(linesToRemove[i], 1); | ||
| changeCount++; | ||
| } | ||
|
|
||
| content = lines.join('\n'); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Documentation claims safety but describes unsafe behavior.
Line 80 states the solution "removes ALL standalone endif; statements when orphaned endifs are detected," and line 165 confirms "Just remove all standalone endif; when wrappers are detected/removed." However, line 151 claims "Safe: ✅ Only removes truly orphaned endif statements."
These statements contradict each other. The implemented approach removes all endif; lines, not just the orphaned ones, which can break legitimate PHP alternative-syntax conditionals. Either update the documentation to reflect the actual risk, or update the code to only remove truly orphaned endif; statements.
Also applies to: 149-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/inc-formatter/references/BUGFIX-REPORT.md around lines 79 -
103, The documentation and implementation disagree: the new Step 2.5 in
formatFile currently iterates over content and unconditionally removes every
line that equals or starts with 'endif;' when analysis.orphanedEndifs is truthy,
which can delete valid PHP alternative-syntax endifs; update the code to only
remove lines whose indices match actual orphan locations (use
analysis.orphanedEndifs to map to specific line numbers or unique markers)
instead of removing all 'endif;' occurrences, or if you choose to keep the broad
removal, change the docs to state that formatFile will remove all standalone
'endif;' lines and note the risk; refer to the formatFile function,
analysis.orphanedEndifs, the content variable and changeCount when making the
fix.
| for (let j = i - 1; j >= 0; j--) { | ||
| const prevLine = lines[j]; | ||
| // Look for if statements using colon syntax (alternative control structure) | ||
| if (prevLine.trim().match(/^if\s*\([^)]+\)\s*:\s*$/)) { | ||
| hasMatchingIf = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Regex fails to match if conditions with nested parentheses.
The pattern [^)]+ in the regex on line 137 matches any character except ), so it stops at the first closing paren. Conditions like if (function_exists('dp_foo')) : or if (isset($a) && isset($b)) : won't be matched, causing their corresponding endif; to be incorrectly flagged as orphaned.
Consider using a more permissive pattern or balanced-paren matching:
Proposed fix
- if (prevLine.trim().match(/^if\s*\([^)]+\)\s*:\s*$/)) {
+ if (prevLine.trim().match(/^if\s*\(.+\)\s*:\s*$/)) {Note: This greedy match is still imperfect for edge cases with ) in strings, but handles most real-world conditions. For robust matching, consider a simple paren-balancing loop instead of regex.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/inc-formatter/scripts/inc-formatter.cjs around lines 134 -
141, The current regex in the backward scan
(prevLine.trim().match(/^if\s*\([^)]+\)\s*:\s*$/)) fails on nested or multiple
parentheses; update the check in the loop that searches for matching ifs (the
for loop referencing prevLine) to either use a more permissive pattern like
matching any content between parentheses (e.g., /^if\s*\(.*\)\s*:\s*$/) or,
preferably, replace the regex with a simple paren-balancing check: confirm the
line starts with "if", ends with ":" and that the parentheses in the condition
are balanced by iterating characters and tracking open/close counts before
deciding hasMatchingIf = true. Ensure this change is applied to the same loop
that sets hasMatchingIf so nested conditions like if (isset($a) && isset($b)) :
are detected.
| * node scripts/spacing-mapper.js --scan [folder] | ||
| * node scripts/spacing-mapper.js --update [folder] [--dry-run] | ||
| */ |
There was a problem hiding this comment.
Update help examples to the actual script path/extension (.cjs).
The usage/help text currently points to scripts/spacing-mapper.js, which doesn’t match this file and can mislead users running commands verbatim.
Proposed fix
- * node scripts/spacing-mapper.js --scan [folder]
- * node scripts/spacing-mapper.js --update [folder] [--dry-run]
+ * node .github/skills/spacing-mapper/scripts/spacing-mapper.cjs --scan [folder]
+ * node .github/skills/spacing-mapper/scripts/spacing-mapper.cjs --update [folder] [--dry-run]
...
- node scripts/spacing-mapper.js <command> [path] [options]
+ node .github/skills/spacing-mapper/scripts/spacing-mapper.cjs <command> [path] [options]Also applies to: 430-460
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/spacing-mapper/scripts/spacing-mapper.cjs around lines 14 -
16, Update the usage/help examples in this file to reference the correct script
filename/extension by replacing occurrences of "scripts/spacing-mapper.js" with
"scripts/spacing-mapper.cjs" (e.g., the top-of-file usage block and any other
help/usage strings around lines noted in the comment); ensure all help output
and example commands emitted by functions or constants that print usage text use
"spacing-mapper.cjs" so examples match the actual script file name.
| - **Background Images**: See [Conversion Guides](references/CONVERSION-GUIDES.md#background-images) | ||
| - **Drop Shadows**: See [Conversion Guides](references/CONVERSION-GUIDES.md#drop-shadows) | ||
| - **Advanced Effects**: See [Conversion Guides](references/CONVERSION-GUIDES.md#advanced-effects) |
There was a problem hiding this comment.
Verify anchor link targets exist in CONVERSION-GUIDES.md.
The anchor links reference sections that may not match the actual headings in the conversion guide:
#background-images- actual heading is "Background Image Conversion"#drop-shadows- actual heading is "Drop Shadow Conversion"#advanced-effects- no such heading exists
Markdown anchor slugs are auto-generated from headings, so these links may be broken.
📝 Suggested fix
## Code Conversion Guides
When converting from React/TSX to WordPress blocks:
-- **Background Images**: See [Conversion Guides](references/CONVERSION-GUIDES.md#background-images)
-- **Drop Shadows**: See [Conversion Guides](references/CONVERSION-GUIDES.md#drop-shadows)
-- **Advanced Effects**: See [Conversion Guides](references/CONVERSION-GUIDES.md#advanced-effects)
+- **Background Images**: See [Conversion Guides](references/CONVERSION-GUIDES.md#background-image-conversion-tsxreact-to-wordpress-blocks)
+- **Drop Shadows**: See [Conversion Guides](references/CONVERSION-GUIDES.md#drop-shadow-conversion-tsxreact-to-wordpress-blocks)
+- **Sticky Position**: See [Conversion Guides](references/CONVERSION-GUIDES.md#sticky-position-tsxreact-to-wordpress-blocks)📝 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.
| - **Background Images**: See [Conversion Guides](references/CONVERSION-GUIDES.md#background-images) | |
| - **Drop Shadows**: See [Conversion Guides](references/CONVERSION-GUIDES.md#drop-shadows) | |
| - **Advanced Effects**: See [Conversion Guides](references/CONVERSION-GUIDES.md#advanced-effects) | |
| - **Background Images**: See [Conversion Guides](references/CONVERSION-GUIDES.md#background-image-conversion-tsxreact-to-wordpress-blocks) | |
| - **Drop Shadows**: See [Conversion Guides](references/CONVERSION-GUIDES.md#drop-shadow-conversion-tsxreact-to-wordpress-blocks) | |
| - **Sticky Position**: See [Conversion Guides](references/CONVERSION-GUIDES.md#sticky-position-tsxreact-to-wordpress-blocks) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/wordpress-block-pattern-generator/SKILL.md around lines 104 -
106, Update the three anchor links in SKILL.md to match actual headings: replace
`#background-images` with `#background-image-conversion` to match "Background
Image Conversion", replace `#drop-shadows` with `#drop-shadow-conversion` to
match "Drop Shadow Conversion", and either remove or point `#advanced-effects`
to an existing heading (or add an "Advanced Effects" section in
CONVERSION-GUIDES.md) so the `references/CONVERSION-GUIDES.md#advanced-effects`
link is not broken.
| run(targetPath) { | ||
| let files = []; | ||
|
|
||
| const stat = fs.statSync(targetPath); | ||
| if (stat.isDirectory()) { | ||
| files = this.scanDirectory(targetPath); | ||
| } else if (stat.isFile()) { | ||
| files = [targetPath]; | ||
| } |
There was a problem hiding this comment.
Missing error handling for file system operations.
fs.statSync(targetPath) will throw an uncaught exception if the target path doesn't exist, providing a poor user experience for CLI usage.
🛡️ Suggested fix
run(targetPath) {
let files = [];
+ if (!fs.existsSync(targetPath)) {
+ console.error(`Error: Path does not exist: ${targetPath}`);
+ return {
+ success: false,
+ filesScanned: 0,
+ filesWithErrors: 0,
+ totalErrors: 0,
+ };
+ }
+
const stat = fs.statSync(targetPath);
if (stat.isDirectory()) {
files = this.scanDirectory(targetPath);📝 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.
| run(targetPath) { | |
| let files = []; | |
| const stat = fs.statSync(targetPath); | |
| if (stat.isDirectory()) { | |
| files = this.scanDirectory(targetPath); | |
| } else if (stat.isFile()) { | |
| files = [targetPath]; | |
| } | |
| run(targetPath) { | |
| let files = []; | |
| if (!fs.existsSync(targetPath)) { | |
| console.error(`Error: Path does not exist: ${targetPath}`); | |
| return { | |
| success: false, | |
| filesScanned: 0, | |
| filesWithErrors: 0, | |
| totalErrors: 0, | |
| }; | |
| } | |
| const stat = fs.statSync(targetPath); | |
| if (stat.isDirectory()) { | |
| files = this.scanDirectory(targetPath); | |
| } else if (stat.isFile()) { | |
| files = [targetPath]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.github/skills/wordpress-block-pattern-validator/scripts/validate-patterns.cjs
around lines 664 - 672, The run(targetPath) method currently calls
fs.statSync(targetPath) without handling exceptions; wrap the fs.statSync call
in a try/catch (or pre-check with fs.existsSync) to handle non-existent or
inaccessible paths, and on error either log a clear message and return an empty
files array or rethrow a controlled Error; update references in this function
(run) and ensure behavior remains consistent with scanDirectory by returning []
or producing a user-friendly error instead of letting the raw exception bubble
up.
- Rename skill directory to be more descriptive - Update name field in YAML frontmatter to match directory - Maintains alignment with agentskills.io spec (name must match directory) The new name better describes the skill's purpose: formatting theme inc/ folders.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/skills/theme-inc-folder-formatter/references/BUGFIX-REPORT.md:
- Around line 141-143: Update the stale "Files Modified" path entry that
currently references `.github/skills/inc-formatter.cjs` in the BUGFIX-REPORT.md
to point to the script's actual location under
theme-inc-folder-formatter/scripts (e.g., change the entry to
`theme-inc-folder-formatter/scripts/inc-formatter.cjs`) so the notes referring
to analyzeFile() and formatFile() accurately locate the modified file.
In @.github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs:
- Around line 12-13: Update the help/usage examples that incorrectly reference
"inc-formatter.js" to the actual shipped filename "inc-formatter.cjs" throughout
the file; search for the literal string "inc-formatter.js" in
.github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs (including
the top usage block and the other occurrences around lines 676-710) and replace
each occurrence with "inc-formatter.cjs" so the printed examples and docs match
the real script name.
- Around line 374-377: The code uses the undefined identifier NAMESPACE inside
the content.replace call which will throw a ReferenceError; update that
replacement to reference the class property this.namespace (same pattern used at
line 386) so the regex substitution uses the initialized namespace value—i.e.,
change the content.replace call that references NAMESPACE to use this.namespace,
ensuring the replacement string becomes `namespace ${this.namespace};` and keep
the surrounding content variable and regex unchanged.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: a71f502a-ac9c-43ae-8cd0-7a1b346e2c37
📒 Files selected for processing (4)
.github/skills/theme-inc-folder-formatter/SKILL.md.github/skills/theme-inc-folder-formatter/eval-queries.json.github/skills/theme-inc-folder-formatter/references/BUGFIX-REPORT.md.github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs
✅ Files skipped from review due to trivial changes (2)
- .github/skills/theme-inc-folder-formatter/eval-queries.json
- .github/skills/theme-inc-folder-formatter/SKILL.md
| 1. **`.github/skills/inc-formatter.cjs`** | ||
| - Lines ~107-138: Added orphaned endif detection in `analyzeFile()` | ||
| - Lines ~294-308: Added Step 2.5 for orphaned endif cleanup in `formatFile()` |
There was a problem hiding this comment.
Files Modified path is stale and points to the wrong location.
Please update the path to the current script location under theme-inc-folder-formatter/scripts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/theme-inc-folder-formatter/references/BUGFIX-REPORT.md around
lines 141 - 143, Update the stale "Files Modified" path entry that currently
references `.github/skills/inc-formatter.cjs` in the BUGFIX-REPORT.md to point
to the script's actual location under theme-inc-folder-formatter/scripts (e.g.,
change the entry to `theme-inc-folder-formatter/scripts/inc-formatter.cjs`) so
the notes referring to analyzeFile() and formatFile() accurately locate the
modified file.
| * node scripts/inc-formatter.js --scan [file/folder] [--namespace=...] [--prefix=...] | ||
| * node scripts/inc-formatter.js --format [file/folder] [--dry-run] [--namespace=...] [--prefix=...] |
There was a problem hiding this comment.
Help examples reference the wrong script filename.
The docs use inc-formatter.js while this tool is shipped as inc-formatter.cjs, so command examples are misleading.
💡 Suggested update
- * node scripts/inc-formatter.js --scan [file/folder] [--namespace=...] [--prefix=...]
- * node scripts/inc-formatter.js --format [file/folder] [--dry-run] [--namespace=...] [--prefix=...]
+ * node scripts/inc-formatter.cjs --scan [file/folder] [--namespace=...] [--prefix=...]
+ * node scripts/inc-formatter.cjs --format [file/folder] [--dry-run] [--namespace=...] [--prefix=...]
...
- node scripts/inc-formatter.js <command> [path] [options]
+ node scripts/inc-formatter.cjs <command> [path] [options]
...
- node scripts/inc-formatter.js --scan ./inc
+ node scripts/inc-formatter.cjs --scan ./inc
...
- node scripts/inc-formatter.js --scan ./inc --namespace="MyTheme\\\\includes" --prefix="mt_"
+ node scripts/inc-formatter.cjs --scan ./inc --namespace="MyTheme\\\\includes" --prefix="mt_"
...
- node scripts/inc-formatter.js --format ./inc --dry-run
+ node scripts/inc-formatter.cjs --format ./inc --dry-run
...
- node scripts/inc-formatter.js --format ./inc
+ node scripts/inc-formatter.cjs --format ./incAlso applies to: 676-710
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs around
lines 12 - 13, Update the help/usage examples that incorrectly reference
"inc-formatter.js" to the actual shipped filename "inc-formatter.cjs" throughout
the file; search for the literal string "inc-formatter.js" in
.github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs (including
the top usage block and the other occurrences around lines 676-710) and replace
each occurrence with "inc-formatter.cjs" so the printed examples and docs match
the real script name.
| content = content.replace( | ||
| /namespace\s+[^;]+;/, | ||
| `namespace ${NAMESPACE};` | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs"
echo "NAMESPACE token usage:"
rg -n '\bNAMESPACE\b' "$FILE"
echo "NAMESPACE declarations (expected: none):"
rg -n '^\s*(const|let|var)\s+NAMESPACE\b' "$FILE" || trueRepository: lightspeedwp/.github
Length of output: 165
🏁 Script executed:
# Check the broader context around line 376
head -n 390 .github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs | tail -n 40Repository: lightspeedwp/.github
Length of output: 1323
🏁 Script executed:
# Check if NAMESPACE could be a global, parameter, or from an outer scope
rg -n -B 30 'namespace.*NAMESPACE' .github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs | head -100Repository: lightspeedwp/.github
Length of output: 1023
🏁 Script executed:
# Look for namespace-related properties or parameter names in the file
rg -n 'this\.namespace|namespace\s*[=:]|namespace\s+\w+' .github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjsRepository: lightspeedwp/.github
Length of output: 1864
Undefined identifier breaks namespace replacement path.
Line 376 uses NAMESPACE, which is never defined in this scope; it will throw a ReferenceError at runtime during namespace updates. The adjacent code path (line 386) correctly uses this.namespace, the class property initialized at line 34.
💡 Proposed fix
- `namespace ${NAMESPACE};`
+ `namespace ${this.namespace};`📝 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.
| content = content.replace( | |
| /namespace\s+[^;]+;/, | |
| `namespace ${NAMESPACE};` | |
| ); | |
| content = content.replace( | |
| /namespace\s+[^;]+;/, | |
| `namespace ${this.namespace};` | |
| ); |
🧰 Tools
🪛 ESLint
[error] 376-376: 'NAMESPACE' is not defined.
(no-undef)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/skills/theme-inc-folder-formatter/scripts/inc-formatter.cjs around
lines 374 - 377, The code uses the undefined identifier NAMESPACE inside the
content.replace call which will throw a ReferenceError; update that replacement
to reference the class property this.namespace (same pattern used at line 386)
so the regex substitution uses the initialized namespace value—i.e., change the
content.replace call that references NAMESPACE to use this.namespace, ensuring
the replacement string becomes `namespace ${this.namespace};` and keep the
surrounding content variable and regex unchanged.
1 participant
Overview
This PR introduces comprehensive WordPress block theme development skills, validation tools, and utilities for the LightSpeed organisation.
Skills Added
🎨 WordPress Block Pattern Generator
✅ WordPress Block Pattern Validator
🎯 WordPress Theme.json Mapper
📦 Theme JSON to Preset Folders
Utilities & Tools
Documentation
Changes
.github/skills/Testing
Related Issues: N/A
Breaking Changes: None
Summary by CodeRabbit
Release Notes
New Features
Documentation