// Review code for AEM Edge Delivery Services projects. Use at the end of development (before PR) for self-review, or to review pull requests. Validates code quality, performance, accessibility, and adherence to EDS best practices.
| name | Code Review |
| description | Review code for AEM Edge Delivery Services projects. Use at the end of development (before PR) for self-review, or to review pull requests. Validates code quality, performance, accessibility, and adherence to EDS best practices. |
Review code for AEM Edge Delivery Services (EDS) projects following established coding standards, performance requirements, and best practices.
This skill supports two modes of operation:
Use this mode when you've finished writing code and want to review it before committing or opening a PR. This is the recommended workflow integration point.
When to invoke:
git add and git commitHow to invoke:
/code-review (reviews uncommitted changes in working directory)What it does:
Use this mode to review an existing pull request (your own or someone else's).
When to invoke:
How to invoke:
/code-review <PR-number> or /code-review <PR-URL>pull_request eventWhat it does:
For Self-Review (no PR number provided):
# See what files have been modified
git status
# See the actual changes
git diff
# For staged changes
git diff --staged
Understand the scope:
For PR Review (PR number provided):
# Get PR details
gh pr view <PR-number> --json title,body,author,baseRefName,headRefName,files,additions,deletions
# Get changed files
gh pr diff <PR-number>
# Get PR comments and reviews
gh api repos/{owner}/{repo}/pulls/<PR-number>/comments
gh api repos/{owner}/{repo}/pulls/<PR-number>/reviews
Understand the scope:
Skip this step for Self-Review mode.
Required elements for PRs (MUST HAVE):
| Element | Requirement | Check |
|---|---|---|
| Preview URLs | Before/After URLs showing the change | Required |
| Description | Clear explanation of what changed and why | Required |
| Scope alignment | Changes match PR title and description | Required |
| Issue reference | Link to GitHub issue (if applicable) | Recommended |
Preview URL format:
https://main--{repo}--{owner}.aem.page/{path}https://{branch}--{repo}--{owner}.aem.page/{path}Flag if missing:
Linting & Style:
eslint-disable comments without justificationeslint-disable directives.js extensions included in importsArchitecture:
loadScript() in blocks, not head.htmlIntersectionObserver for heavy librariesaem.js is NOT modified (submit upstream PRs for improvements)Code Patterns:
Common Issues to Flag:
// BAD: CSS in JavaScript
element.style.backgroundColor = 'blue';
// GOOD: Use CSS classes
element.classList.add('highlighted');
// BAD: Hardcoded configuration
const temperature = 0.7;
// GOOD: Use config or constants
const { temperature } = CONFIG;
// BAD: Global eslint-disable
/* eslint-disable */
// GOOD: Specific, justified disables
/* eslint-disable-next-line no-console -- intentional debug output */
Linting & Style:
!important unless absolutely necessary (with justification)Scoping & Selectors:
.{block-name} .selector or main .{block-name}[aria-expanded="true"])Responsive Design:
600px, 900px, 1200px (all min-width)min-width and max-width queriesFrameworks & Preprocessors:
Common Issues to Flag:
/* BAD: Unscoped selector */
.title { color: red; }
/* GOOD: Scoped to block */
main .my-block .title { color: red; }
/* BAD: !important abuse */
.button { color: white !important; }
/* GOOD: Increase specificity instead */
main .my-block .button { color: white; }
/* BAD: Mixed breakpoint directions */
@media (max-width: 600px) { }
@media (min-width: 900px) { }
/* GOOD: Consistent mobile-first */
@media (min-width: 600px) { }
@media (min-width: 900px) { }
/* BAD: CSS in JS patterns */
element.innerHTML = '<style>.foo { color: red; }</style>';
/* GOOD: Use external CSS files */
head.html<head> (performance impact)Critical Requirements:
head.html)Performance Checklist:
IntersectionObserver or delayed loadingPreview URL Verification: If preview URLs provided, check:
Purpose: Capture screenshots of the preview URL to validate visual appearance. For self-review, this confirms your changes look correct before committing. For PR review, this provides visual evidence in the review comment.
When to capture screenshots:
How to capture screenshots:
Option 1: Playwright (Recommended for automation)
// capture-screenshots.js
import { chromium } from 'playwright';
async function captureScreenshots(afterUrl, outputDir = './screenshots') {
const browser = await chromium.launch();
const page = await browser.newPage();
// Desktop screenshot
await page.setViewportSize({ width: 1200, height: 800 });
await page.goto(afterUrl, { waitUntil: 'networkidle' });
await page.waitForTimeout(1000); // Wait for animations
await page.screenshot({
path: `${outputDir}/desktop.png`,
fullPage: true
});
// Tablet screenshot
await page.setViewportSize({ width: 768, height: 1024 });
await page.screenshot({
path: `${outputDir}/tablet.png`,
fullPage: true
});
// Mobile screenshot
await page.setViewportSize({ width: 375, height: 667 });
await page.screenshot({
path: `${outputDir}/mobile.png`,
fullPage: true
});
// Optional: Capture specific block/element
const block = page.locator('.my-block');
if (await block.count() > 0) {
await block.screenshot({ path: `${outputDir}/block.png` });
}
await browser.close();
return {
desktop: `${outputDir}/desktop.png`,
tablet: `${outputDir}/tablet.png`,
mobile: `${outputDir}/mobile.png`
};
}
// Usage
captureScreenshots('https://branch--repo--owner.aem.page/path');
Option 2: Using MCP Browser Tools
If you have MCP browser or Playwright tools available:
Option 3: Manual capture with guidance
Instruct the reviewer or PR author to:
Uploading screenshots to GitHub:
# Upload screenshot as PR comment with image
# First, upload to a hosting service or use GitHub's image upload
# Option A: Embed in PR comment (drag & drop in GitHub UI)
gh pr comment <PR-number> --body "## Visual Preview
### Desktop (1200px)
### Mobile (375px)
"
# Option B: Use GitHub's attachment API (for automation)
# Screenshots can be uploaded as part of the comment body
Screenshot checklist:
Visual issues to look for:
Content Model (if applicable):
Static Resources:
rel="noopener noreferrer"Output depends on the review mode:
Report findings directly to continue the development workflow:
## Code Review Summary
### Files Reviewed
- `blocks/my-block/my-block.js` (new)
- `blocks/my-block/my-block.css` (new)
### Visual Validation
✅ Layout renders correctly across viewports
✅ No console errors
✅ Responsive behavior verified
### Issues Found
#### Must Fix Before Committing
- [ ] `blocks/my-block/my-block.js:45` - Remove console.log debug statement
- [ ] `blocks/my-block/my-block.css:12` - Selector `.title` needs block scoping
#### Recommendations
- [ ] Consider using `loadScript()` for the external library
### Ready to Commit?
- [ ] All "Must Fix" issues resolved
- [ ] Linting passes: `npm run lint`
- [ ] Visual validation complete
After self-review: Fix any issues found, then proceed with committing and opening a PR.
Structure the review comment for GitHub:
## PR Review Summary
### Overview
[Brief summary of the PR and its purpose]
### Preview URLs Validated
- [ ] Before: [URL]
- [ ] After: [URL]
### Visual Preview
#### Desktop (1200px)
#### Mobile (375px)
<details>
<summary>Additional Screenshots</summary>
#### Tablet (768px)
#### Block Detail
</details>
### Visual Assessment
- [ ] Layout renders correctly across viewports
- [ ] No visual regressions from main branch
- [ ] Colors and typography consistent
- [ ] Images and icons display properly
### Checklist Results
#### Must Fix (Blocking)
- [ ] [Critical issue with file:line reference]
#### Should Fix (High Priority)
- [ ] [Important issue with file:line reference]
#### Consider (Suggestions)
- [ ] [Nice-to-have improvement]
### Detailed Findings
#### [Category: e.g., JavaScript, CSS, Performance]
**File:** `path/to/file.js:123`
**Issue:** [Description of the issue]
**Suggestion:** [How to fix it]
Skip this step for Self-Review mode - in self-review, you fix issues directly in your working directory.
After identifying issues in a PR review, provide actionable fixes to make it easier for the PR author to address them. The goal is to provide one-click fixes whenever possible.
PRIMARY METHOD: GitHub Suggestions (use for ~70-80% of fixable issues)
SECONDARY: Guidance Comments (~20-30% of issues)
RARE: Fix Commits (avoid unless necessary)
| Approach | When to Use | Examples |
|---|---|---|
| GitHub Suggestions (PRIMARY) | Any change that can be expressed as a code replacement | Remove console.log, fix typos, add comments, refactor selectors, update functions, add error handling |
| Fix Commits (SECONDARY) | Changes that need testing, span many files, or are too large for suggestions | Complex multi-file refactors, security fixes requiring validation, changes >20 lines |
| Guidance Only (FALLBACK) | Architectural changes, subjective improvements, or when multiple approaches exist | "Consider using IntersectionObserver", design pattern suggestions, performance optimizations |
IMPORTANT: Always prefer GitHub Suggestions when possible - they provide the best user experience with one-click acceptance and proper git attribution.
Use GitHub's native suggestion feature for most fixes. This provides the best user experience with one-click acceptance and proper git attribution.
When to use:
When NOT to use:
Benefits:
How to create suggestions:
GitHub suggestions are created using the Pull Request Reviews API with a special markdown syntax in the comment body:
```suggestion
// The corrected code here
```
Complete workflow with examples:
# Step 1: Get PR information
PR_NUMBER=196
OWNER="adobe"
REPO="helix-tools-website"
# Get the current HEAD commit SHA (required for review API)
COMMIT_SHA=$(gh api repos/$OWNER/$REPO/pulls/$PR_NUMBER --jq '.head.sha')
# Step 2: Analyze the diff to find line positions
# IMPORTANT: Use 'position' in the diff, NOT 'line' in the original file
# Position is the line number in the unified diff output starting from the first diff hunk
# Get the diff to understand positions
gh pr diff $PR_NUMBER --repo $OWNER/$REPO > /tmp/pr-$PR_NUMBER.diff
# Step 3: Create review JSON with suggestions
# Each comment needs:
# - path: file path relative to repo root
# - position: line number IN THE DIFF (not in the file!)
# - body: description + ```suggestion block
cat > /tmp/review-suggestions.json <<JSON
{
"commit_id": "$COMMIT_SHA",
"event": "COMMENT",
"comments": [
{
"path": "tools/page-status/diff.js",
"position": 58,
"body": "**Fix: Add XSS Safety Documentation** (BLOCKING)\\n\\nAdd a comment to document that this HTML injection is safe:\\n\\n\`\`\`suggestion\\n const previewBodyHtml = previewDom.querySelector('body').innerHTML;\\n\\n // XSS Safe: previewBodyHtml is sanitized by mdToDocDom from trusted admin API\\n const newPageHtml = \\\`\\n\`\`\`\\n\\nThis addresses the security concern by making it clear that XSS has been considered."
},
{
"path": "tools/page-status/diff.js",
"position": 6,
"body": "**Fix: Improve Error Handling Pattern**\\n\\nAdd an \\\`ok\\\` flag for more consistent error handling:\\n\\n\`\`\`suggestion\\n * @returns {Promise<{content: string|null, status: number, ok: boolean}>} Content, status, and success flag\\n\`\`\`"
},
{
"path": "tools/page-status/diff.js",
"position": 12,
"body": "**Fix: Return consistent result object**\\n\\n\`\`\`suggestion\\n return { content: null, status: res.status, ok: false };\\n\`\`\`"
},
{
"path": "tools/page-status/diff.js",
"position": 16,
"body": "**Fix: Include ok flag in success case**\\n\\n\`\`\`suggestion\\n return { content, status: res.status, ok: true };\\n\`\`\`"
},
{
"path": "tools/page-status/diff.css",
"position": 41,
"body": "**Fix: Remove stylelint-disable by refactoring selector**\\n\\nUse \\\`.diff-new-page\\\` as intermediate selector to avoid specificity conflict:\\n\\n\`\`\`suggestion\\n.page-diff .diff-new-page .doc-diff-side-header {\\n padding: var(--spacing-s) var(--spacing-m);\\n\`\`\`"
}
]
}
JSON
# Step 4: Submit the review with all suggestions at once
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
repos/$OWNER/$REPO/pulls/$PR_NUMBER/reviews \
--input /tmp/review-suggestions.json
# Step 5: Add a friendly summary comment
gh pr comment $PR_NUMBER --repo $OWNER/$REPO --body "$(cat <<'EOF'
## ✨ One-Click Fix Suggestions Added!
I've added **GitHub Suggestions** that you can apply with a single click!
### How to Apply
1. Go to the **Files changed** tab
2. Find the inline comments with suggestions
3. Click **"Commit suggestion"** to apply individually
4. Or click **"Add suggestion to batch"** on multiple, then **"Commit suggestions"** to batch
### What's Included
- ✅ [BLOCKING] XSS safety documentation
- ✅ Error handling improvements
- ✅ CSS selector refactoring (removes linter disables)
After applying, run \`npm run lint\` to verify all checks pass!
EOF
)"
echo "✅ Review with suggestions posted successfully!"
echo "View at: https://github.com/$OWNER/$REPO/pull/$PR_NUMBER"
Key points:
position (diff position) NOT line (file line number) - This is critical!position is the line number in the unified diff output, counting from the first @@ hunk markerHow to determine the correct position value:
The position is the line number in the unified diff, NOT the line number in the file. Here's how to find it:
Get the diff:
gh pr diff <PR-number> > pr.diff
Open the diff and count lines from the top, including:
--- a/file and +++ b/file)@@ -old,lines +new,lines @@)The position is the line number of the ADDED line you want to comment on
Example from actual diff:
--- a/tools/page-status/diff.js
+++ b/tools/page-status/diff.js
async function fetchContent(url) {
const res = await fetch(url);
- if (!res.ok) throw new Error(`Failed to fetch ${url}: ${res.status}`);
- return res.text();
+ if (!res.ok) {
+ return { content: null, status: res.status }; ← Position 12 (counting from top)
+ }
Best practices for suggestions:
position in the diff, not line in the file (critical!)For more complex fixes, create commits directly on the PR branch. This is especially useful when:
Prerequisites:
Workflow:
# 1. Get PR branch information
PR_INFO=$(gh pr view <PR-number> --json headRefName,headRepository,headRepositoryOwner)
BRANCH=$(echo $PR_INFO | jq -r '.headRefName')
REPO_OWNER=$(echo $PR_INFO | jq -r '.headRepositoryOwner.login')
# 2. Fetch the PR branch
git fetch origin pull/<PR-number>/head:pr-<PR-number>
# 3. Check out the PR branch
git checkout pr-<PR-number>
# 4. Make fixes based on review findings
# Example: Fix CSS linter issues by refactoring selectors
# 5. Test your fixes
npm run lint
# Run any relevant tests
# 6. Commit with detailed reasoning
git commit -m "$(cat <<'EOF'
fix: refactor CSS selectors to eliminate linter disables
Refactored CSS selectors in diff.css to resolve specificity conflicts
without using stylelint-disable comments. Changes:
- Increased specificity using .diff-new-page parent selector
- Reordered rules to prevent descending specificity issues
- Maintains same visual appearance and functionality
Addresses code review feedback: https://github.com/{owner}/{repo}/pull/<PR-number>#issuecomment-XXXXX
EOF
)"
# 7. Push to the PR branch
git push origin pr-<PR-number>:$BRANCH
# 8. Add comment to PR explaining what you fixed
gh pr comment <PR-number> --body "$(cat <<'EOF'
## Fixes Applied
I've pushed commits to address some of the review findings:
### Commit 1: Refactor CSS selectors
- ✅ Eliminated all `stylelint-disable` comments
- ✅ Resolved specificity conflicts properly
- ✅ Maintained visual appearance
### Commit 2: Standardize error handling
- ✅ Updated fetchContent to return consistent result objects
- ✅ Updated all callers to use new pattern
- ✅ Added JSDoc for clarity
### Still Need Action From You:
- [ ] **[BLOCKING]** Add XSS safety comment (see suggestion in review)
- [ ] Consider extracting renderDiffPanel helper (optional)
All linting passes now. Please review the commits and address the remaining item.
EOF
)"
Commit message format for fixes:
fix: <short description>
<Detailed explanation of what was fixed and why>
Changes:
- <specific change 1>
- <specific change 2>
Addresses review feedback: <link to review comment>
For the majority of PRs, use this hybrid strategy:
Typical workflow:
# 1. Post comprehensive review comment with summary (from Step 8)
gh pr comment <PR-number> --repo {owner}/{repo} --body "<detailed review summary>"
# 2. Submit a review with GitHub suggestions for ALL fixable issues
# (Create JSON as shown in Approach 1 with all suggestions)
gh api POST repos/{owner}/{repo}/pulls/<PR-number>/reviews \
--input /tmp/review-suggestions.json
# 3. Add a friendly summary about the suggestions
gh pr comment <PR-number> --repo {owner}/{repo} --body "$(cat <<'EOF'
## ✨ One-Click Suggestions Added!
I've added GitHub Suggestions for the fixable issues:
- ✅ [BLOCKING] Security documentation
- ✅ Error handling improvements
- ✅ CSS selector refactoring
Go to **Files changed** tab and click **"Commit suggestion"** to apply!
EOF
)"
# 4. For subjective/architectural issues, add guidance comments separately
# (Only if needed - most issues should have suggestions)
Real-world example distribution:
For a typical PR with 10 issues:
This approach provides:
PR #196: Support unpublished pages in diff tool
Issues identified in review:
Action taken:
# Created two reviews with 9 total GitHub Suggestions
# - 4 suggestions for diff.js (XSS comment, error handling)
# - 5 suggestions for diff.css (selector refactoring)
# Posted friendly summary comment explaining one-click application
# Result: PR author can fix all issues in ~30 seconds vs 5-10 minutes of manual work
View the actual implementation:
Time saved:
Use GitHub Suggestions when: (DEFAULT - use this ~70-80% of the time)
Use Fix Commits when: (RARE - use only when suggestions won't work)
Use Guidance Only when: (~20-30% of issues)
Decision flowchart:
Issue identified
↓
Do I know the exact fix? ──NO──→ Use Guidance
↓ YES
Is it < 20 lines? ──NO──→ Use Fix Commit or Guidance
↓ YES
Use GitHub Suggestion ✅ (BEST OPTION)
Before posting suggestions or commits:
Don't:
Common issues and solutions:
| Problem | Cause | Solution |
|---|---|---|
| "Validation Failed: line could not be resolved" | Used line instead of position | Use position (diff line number) not line (file line number) |
| Suggestion doesn't appear inline | Wrong position value | Count lines in the diff carefully, including headers |
| Can't apply suggestion | Merge conflict or branch updated | Author needs to update branch first |
| Suggestion formatting broken | Unescaped JSON characters | Escape quotes, backticks, newlines in JSON |
| "Invalid commit_id" | Used old commit SHA | Get current HEAD SHA before creating review |
How to verify your review before posting:
# 1. Validate JSON syntax
cat /tmp/review-suggestions.json | jq . > /dev/null && echo "✅ Valid JSON"
# 2. Check position values against diff
gh pr diff <PR-number> > pr.diff
# Manually verify each position value in your JSON matches an added line
# 3. Test with one suggestion first
# Before posting 10 suggestions, test with 1 to ensure positions are correct
A good fix provision should:
position values (diff lines, not file lines)Copy this template to quickly create a review with GitHub Suggestions:
#!/bin/bash
# Quick script to create GitHub Suggestions for PR review
PR_NUMBER=YOUR_PR_NUMBER
OWNER="adobe"
REPO="helix-tools-website"
# Get commit SHA
COMMIT_SHA=$(gh api repos/$OWNER/$REPO/pulls/$PR_NUMBER --jq '.head.sha')
echo "✅ PR Head SHA: $COMMIT_SHA"
# Create review JSON
cat > /tmp/review-$PR_NUMBER.json <<JSON
{
"commit_id": "$COMMIT_SHA",
"event": "COMMENT",
"comments": [
{
"path": "path/to/file.js",
"position": DIFF_LINE_NUMBER,
"body": "**Fix: Issue Title**\\n\\nExplanation of the issue.\\n\\n\`\`\`suggestion\\nYour fixed code here\\n\`\`\`\\n\\nReasoning for the fix."
}
]
}
JSON
# Submit review
gh api POST repos/$OWNER/$REPO/pulls/$PR_NUMBER/reviews \
--input /tmp/review-$PR_NUMBER.json
# Add summary comment
gh pr comment $PR_NUMBER --repo $OWNER/$REPO --body "✨ GitHub Suggestions added! Go to **Files changed** tab and click **Commit suggestion** to apply."
echo "✅ Review posted! View at: https://github.com/$OWNER/$REPO/pull/$PR_NUMBER"
To use:
PR_NUMBER, OWNER, REPOIssues that MUST be resolved before merge:
aem.jsIssues that SHOULD be resolved:
!important usage without justificationImprovements to consider:
Based on actual PR reviews, watch for these patterns:
!important?" - Strong preference against !important!important## Approved
Preview URLs verified and changes look good.
### Visual Preview
<details>
<summary>Mobile View</summary>
</details>
**Verified:**
- [x] Code quality and linting
- [x] Performance (Lighthouse scores)
- [x] Visual appearance (screenshots captured)
- [x] Responsive behavior
- [x] Accessibility basics
[Any additional notes]
## Changes Requested
### Blocking Issues
[List with file:line references]
### Suggestions
[List of recommendations]
Please address the blocking issues before merge.
## Review Notes
[Non-blocking observations and questions]
When triggered via GitHub Actions, the skill should:
GitHub Actions integration points:
pull_request event triggersgh pr review for posting reviewsgh pr comment for detailed feedbackA complete self-review should:
A complete PR review should:
This skill integrates with the content-driven-development workflow:
CDD Workflow:
Step 1: Start dev server
Step 2: Analyze & plan
Step 3: Design content model
Step 4: Identify/create test content
Step 5: Implement (building-blocks skill)
└── testing-blocks skill (browser testing)
└── **code-review skill (self-review)** ← Invoke here
Step 6: Lint & test
Step 7: Final validation
Step 8: Ship it (commit & PR)
Recommended invocation point: After implementation and testing-blocks skill complete, before final linting and committing.
What this catches before PR:
Benefits of self-review:
Analyze content sequences and determine authoring approach (default content vs blocks). Validates block selection and section styling for import/migration to AEM Edge Delivery Services.
Survey available blocks from local AEM Edge Delivery Services project and Block Collection to understand the block palette available for authoring. Returns block inventory with purposes to inform content modeling decisions.
Searches the aem.live documentation for information on AEM Edge Delivery Services features. Use this skill when you need more information about a feature, want guidance on how to implement a feature, and using existing tools you have to search the web isn't turning up relevant results.
Generate structured HTML from authoring analysis for AEM Edge Delivery Services. Creates section structure, applies block tables, handles metadata, and manages images folder.
Identify section boundaries and content sequences within a scraped webpage for AEM Edge Delivery Services import. Performs two-level analysis (sections, then sequences per section) and surveys available blocks.
Analyze content sequences within a section and provide neutral descriptions for AEM Edge Delivery Services. Invoked per section during page import to identify breaking points between default content and blocks.