npx skills add https://github.com/adobe/helix-website --skill 'Code Review'遵循既定的编码标准、性能要求和最佳实践,审查 AEM Edge Delivery Services (EDS) 项目的代码。
此技能支持两种操作模式:
当您完成代码编写,并希望在提交或开启 PR 前进行审查时,使用此模式。这是推荐的工作流集成点。
何时调用:
git add 和 git commit 之前如何调用:
/code-review(审查工作目录中未提交的更改)功能:
使用此模式来审查现有的拉取请求(您自己或他人的)。
何时调用:
广告位招租
在这里展示您的产品或服务
触达数万 AI 开发者,精准高效
如何调用:
/code-review <PR-编号> 或 /code-review <PR-URL>pull_request 事件的 GitHub 工作流功能:
对于自审(未提供 PR 编号):
# 查看哪些文件已被修改
git status
# 查看实际更改
git diff
# 对于已暂存的更改
git diff --staged
理解范围:
对于 PR 审查(提供了 PR 编号):
# 获取 PR 详情
gh pr view <PR-编号> --json title,body,author,baseRefName,headRefName,files,additions,deletions
# 获取更改的文件
gh pr diff <PR-编号>
# 获取 PR 评论和审查
gh api repos/{owner}/{repo}/pulls/<PR-编号>/comments
gh api repos/{owner}/{repo}/pulls/<PR-编号>/reviews
理解范围:
对于自审模式,跳过此步骤。
PR 的必需元素(必须拥有):
| 元素 | 要求 | 检查 |
|---|---|---|
| 预览 URL | 显示更改的前/后 URL | 必需 |
| 描述 | 清晰说明更改内容和原因 | 必需 |
| 范围一致性 | 更改与 PR 标题和描述匹配 | 必需 |
| Issue 引用 | 链接到 GitHub issue(如适用) | 推荐 |
预览 URL 格式:
https://main--{repo}--{owner}.aem.page/{path}https://{branch}--{repo}--{owner}.aem.page/{path}标记缺失项:
代码检查与风格:
eslint-disable 注释eslint-disable 指令.js 扩展名架构:
loadScript() 加载,而非 head.htmlIntersectionObserveraem.js(改进请提交上游 PR)代码模式:
需要标记的常见问题:
// 错误:在 JavaScript 中使用 CSS
element.style.backgroundColor = 'blue';
// 正确:使用 CSS 类
element.classList.add('highlighted');
// 错误:硬编码配置
const temperature = 0.7;
// 正确:使用配置或常量
const { temperature } = CONFIG;
// 错误:全局 eslint-disable
/* eslint-disable */
// 正确:特定、有理由的禁用
/* eslint-disable-next-line no-console -- 故意的调试输出 */
代码检查与风格:
!important作用域与选择器:
.{block-name} .selector 或 main .{block-name}[aria-expanded="true"])响应式设计:
600px、900px、1200px(全部为 min-width)min-width 和 max-width 查询框架与预处理器:
需要标记的常见问题:
/* 错误:未限定作用域的选择器 */
.title { color: red; }
/* 正确:限定在模块内 */
main .my-block .title { color: red; }
/* 错误:滥用 !important */
.button { color: white !important; }
/* 正确:改为增加特异性 */
main .my-block .button { color: white; }
/* 错误:混合断点方向 */
@media (max-width: 600px) { }
@media (min-width: 900px) { }
/* 正确:一致的移动优先 */
@media (min-width: 600px) { }
@media (min-width: 900px) { }
/* 错误:CSS in JS 模式 */
element.innerHTML = '<style>.foo { color: red; }</style>';
/* 正确:使用外部 CSS 文件 */
head.html 中没有内联样式或脚本<head> 中(影响性能)关键要求:
head.html)性能检查清单:
IntersectionObserver 或延迟加载预览 URL 验证: 如果提供了预览 URL,请检查:
目的: 捕获预览 URL 的截图以验证视觉外观。对于自审,这可以在提交前确认您的更改看起来正确。对于 PR 审查,这在审查评论中提供视觉证据。
何时捕获截图:
如何捕获截图:
选项 1:Playwright(推荐用于自动化)
// capture-screenshots.js
import { chromium } from 'playwright';
async function captureScreenshots(afterUrl, outputDir = './screenshots') {
const browser = await chromium.launch();
const page = await browser.newPage();
// 桌面端截图
await page.setViewportSize({ width: 1200, height: 800 });
await page.goto(afterUrl, { waitUntil: 'networkidle' });
await page.waitForTimeout(1000); // 等待动画
await page.screenshot({
path: `${outputDir}/desktop.png`,
fullPage: true
});
// 平板端截图
await page.setViewportSize({ width: 768, height: 1024 });
await page.screenshot({
path: `${outputDir}/tablet.png`,
fullPage: true
});
// 移动端截图
await page.setViewportSize({ width: 375, height: 667 });
await page.screenshot({
path: `${outputDir}/mobile.png`,
fullPage: true
});
// 可选:捕获特定模块/元素
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`
};
}
// 用法
captureScreenshots('https://branch--repo--owner.aem.page/path');
选项 2:使用 MCP 浏览器工具
如果您有可用的 MCP 浏览器或 Playwright 工具:
选项 3:带有指导的手动捕获
指导审查者或 PR 作者:
将截图上传到 GitHub:
# 将截图作为 PR 评论上传并附带图片
# 首先,上传到托管服务或使用 GitHub 的图片上传功能
# 选项 A:嵌入到 PR 评论中(在 GitHub UI 中拖放)
gh pr comment <PR-编号> --body "## 视觉预览
### 桌面端 (1200px)

### 移动端 (375px)

"
# 选项 B:使用 GitHub 的附件 API(用于自动化)
# 截图可以作为评论正文的一部分上传
截图检查清单:
需要留意的视觉问题:
内容模型(如适用):
静态资源:
rel="noopener noreferrer"输出取决于审查模式:
直接报告发现结果以继续开发工作流:
## 代码审查摘要
### 已审查的文件
- `blocks/my-block/my-block.js` (新建)
- `blocks/my-block/my-block.css` (新建)
### 视觉验证

✅ 布局在所有视口下正确渲染
✅ 没有控制台错误
✅ 响应式行为已验证
### 发现的问题
#### 提交前必须修复
- [ ] `blocks/my-block/my-block.js:45` - 移除 console.log 调试语句
- [ ] `blocks/my-block/my-block.css:12` - 选择器 `.title` 需要模块作用域限定
#### 建议
- [ ] 考虑使用 `loadScript()` 加载外部库
### 可以提交了吗?
- [ ] 所有“必须修复”的问题已解决
- [ ] 代码检查通过:`npm run lint`
- [ ] 视觉验证完成
自审后: 修复发现的任何问题,然后继续提交并开启 PR。
为 GitHub 构建审查评论:
## PR 审查摘要
### 概述
[PR 及其目的的简要摘要]
### 已验证的预览 URL
- [ ] 之前:[URL]
- [ ] 之后:[URL]
### 视觉预览
#### 桌面端 (1200px)

#### 移动端 (375px)

<details>
<summary>附加截图</summary>
#### 平板端 (768px)

#### 模块详情

</details>
### 视觉评估
- [ ] 布局在所有视口下正确渲染
- [ ] 与主分支相比没有视觉回归
- [ ] 颜色和排版一致
- [ ] 图像和图标显示正常
### 检查清单结果
#### 必须修复(阻塞性)
- [ ] [关键问题,附带文件:行号引用]
#### 应该修复(高优先级)
- [ ] [重要问题,附带文件:行号引用]
#### 考虑(建议)
- [ ] [锦上添花的改进]
### 详细发现
#### [类别:例如 JavaScript、CSS、性能]
**文件:** `path/to/file.js:123`
**问题:** [问题描述]
**建议:** [如何修复]
对于自审模式跳过此步骤 - 在自审中,您直接在工作目录中修复问题。
在 PR 审查中识别问题后,提供可操作的修复方案,使 PR 作者更容易解决它们。目标是尽可能提供一键式修复。
主要方法:GitHub 建议(用于约 70-80% 的可修复问题)
次要方法:指导性评论(约 20-30% 的问题)
罕见方法:修复提交(除非必要,否则避免使用)
| 方法 | 何时使用 | 示例 |
|---|---|---|
| GitHub 建议(主要) | 任何可以表示为代码替换的更改 | 移除 console.log、修复拼写错误、添加注释、重构选择器、更新函数、添加错误处理 |
| 修复提交(次要) | 需要测试、跨多个文件或对于建议来说太大的更改 | 复杂的多文件重构、需要验证的安全修复、超过 20 行的更改 |
| 仅提供指导(后备) | 架构更改、主观改进或存在多种方法时 | “考虑使用 IntersectionObserver”、设计模式建议、性能优化 |
重要提示: 尽可能优先使用 GitHub 建议 - 它们提供了一键接受和正确的 git 归属,提供了最佳用户体验。
对于大多数修复,使用 GitHub 的原生建议功能。这提供了一键接受和正确的 git 归属,提供了最佳用户体验。
何时使用:
何时不使用:
好处:
如何创建建议:
GitHub 建议是使用 Pull Request Reviews API 并在评论正文中使用特殊的 markdown 语法创建的:
```suggestion
// 修正后的代码放在这里
```
包含示例的完整工作流:
# 步骤 1:获取 PR 信息
PR_NUMBER=196
OWNER="adobe"
REPO="helix-tools-website"
# 获取当前的 HEAD 提交 SHA(审查 API 必需)
COMMIT_SHA=$(gh api repos/$OWNER/$REPO/pulls/$PR_NUMBER --jq '.head.sha')
# 步骤 2:分析差异以找到行位置
# 重要提示:使用差异中的 'position',而不是原始文件中的 'line'
# position 是统一差异输出中的行号,从第一个差异块开始计数
# 获取差异以理解位置
gh pr diff $PR_NUMBER --repo $OWNER/$REPO > /tmp/pr-$PR_NUMBER.diff
# 步骤 3:创建包含建议的审查 JSON
# 每个评论需要:
# - path:相对于仓库根目录的文件路径
# - position:差异中的行号(不是文件中的行号!)
# - body:描述 + ```suggestion 块
cat > /tmp/review-suggestions.json <<JSON
{
"commit_id": "$COMMIT_SHA",
"event": "COMMENT",
"comments": [
{
"path": "tools/page-status/diff.js",
"position": 58,
"body": "**修复:添加 XSS 安全文档** (阻塞性)\\n\\n添加注释以说明此 HTML 注入是安全的:\\n\\n\`\`\`suggestion\\n const previewBodyHtml = previewDom.querySelector('body').innerHTML;\\n\\n // XSS 安全:previewBodyHtml 由受信任的管理员 API 通过 mdToDocDom 清理\\n const newPageHtml = \\\`\\n\`\`\`\\n\\n通过明确说明已考虑 XSS 问题来解决安全担忧。"
},
{
"path": "tools/page-status/diff.js",
"position": 6,
"body": "**修复:改进错误处理模式**\\n\\n添加 \\\`ok\\\` 标志以实现更一致的错误处理:\\n\\n\`\`\`suggestion\\n * @returns {Promise<{content: string|null, status: number, ok: boolean}>} 内容、状态和成功标志\\n\`\`\`"
},
{
"path": "tools/page-status/diff.js",
"position": 12,
"body": "**修复:返回一致的结果对象**\\n\\n\`\`\`suggestion\\n return { content: null, status: res.status, ok: false };\\n\`\`\`"
},
{
"path": "tools/page-status/diff.js",
"position": 16,
"body": "**修复:在成功情况下包含 ok 标志**\\n\\n\`\`\`suggestion\\n return { content, status: res.status, ok: true };\\n\`\`\`"
},
{
"path": "tools/page-status/diff.css",
"position": 41,
"body": "**修复:通过重构选择器移除 stylelint-disable**\\n\\n使用 \\\`.diff-new-page\\\` 作为中间选择器以避免特异性冲突:\\n\\n\`\`\`suggestion\\n.page-diff .diff-new-page .doc-diff-side-header {\\n padding: var(--spacing-s) var(--spacing-m);\\n\`\`\`"
}
]
}
JSON
# 步骤 4:一次性提交包含所有建议的审查
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
repos/$OWNER/$REPO/pulls/$PR_NUMBER/reviews \
--input /tmp/review-suggestions.json
# 步骤 5:添加友好的摘要评论
gh pr comment $PR_NUMBER --repo $OWNER/$REPO --body "$(cat <<'EOF'
## ✨ 已添加一键修复建议!
我已添加 **GitHub 建议**,您可以一键应用!
### 如何应用
1. 转到 **Files changed** 标签页
2. 找到带有建议的内联评论
3. 点击 **"Commit suggestion"** 单独应用
4. 或者在多个建议上点击 **"Add suggestion to batch"**,然后点击 **"Commit suggestions"** 进行批量处理
### 包含的内容
- ✅ [阻塞性] XSS 安全文档
- ✅ 错误处理改进
- ✅ CSS 选择器重构(移除代码检查禁用)
应用后,运行 \`npm run lint\` 以验证所有检查通过!
EOF
)"
echo "✅ 包含建议的审查已成功发布!"
echo "查看地址:https://github.com/$OWNER/$REPO/pull/$PR_NUMBER"
关键点:
position(差异位置)而不是 line(文件行号) - 这很关键!position 是统一差异输出中的行号,从第一个 @@ 块标记开始计数如何确定正确的 position 值:
position 是统一差异中的行号,不是文件中的行号。以下是查找方法:
获取差异:
gh pr diff <PR-编号> > pr.diff
打开差异并从上到下计数行,包括:
--- a/file 和 +++ b/file)@@ -old,lines +new,lines @@)position 是您想要评论的添加行的行号
来自实际差异的示例:
--- 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 }; ← 位置 12(从顶部开始计数)
+ }
建议的最佳实践:
position,而不是文件中的 line(关键!)对于更复杂的修复,直接在 PR 分支上创建提交。这在以下情况下特别有用:
先决条件:
工作流:
# 1. 获取 PR 分支信息
PR_INFO=$(gh pr view <PR-编号> --json headRefName,headRepository,headRepositoryOwner)
BRANCH=$(echo $PR_INFO | jq -r '.headRefName')
REPO_OWNER=$(echo $PR_INFO | jq -r '.headRepositoryOwner.login')
# 2. 获取 PR 分支
git fetch origin pull/<PR-编号>/head:pr-<PR-编号>
# 3. 检出 PR 分支
git checkout pr-<PR-编号>
# 4. 根据审查发现进行修复
# 示例:通过重构选择器修复 CSS 代码检查问题
# 5. 测试您的修复
npm run lint
# 运行任何相关的测试
# 6. 提交并附上详细理由
git commit -m "$(cat <<'EOF'
fix: 重构 CSS 选择器以消除代码检查禁用
重构了 diff.css 中的 CSS 选择器,以在不使用 stylelint-disable 注释的情况下解决特异性冲突。更改包括:
- 使用 .diff-new-page 父选择器增加特异性
- 重新排序规则以防止降序特异性问题
- 保持相同的视觉外观和功能
处理代码审查反馈:https://github.com/{owner}/{repo}/pull/<PR-编号>#issuecomment-XXXXX
EOF
)"
# 7. 推送到 PR 分支
git push origin pr-<PR-编号>:$BRANCH
# 8. 向 PR 添加评论解释您修复了什么
gh pr comment <PR-编号> --body "$(cat <<'EOF'
## 已应用的修复
我已推送提交以解决部分审查发现:
### 提交 1:重构 CSS 选择器
- ✅ 消除了所有 `stylelint-disable` 注释
- ✅ 正确解决了特异性冲突
- ✅ 保持了视觉外观
### 提交 2:标准化错误处理
- ✅ 更新 fetchContent 以返回一致的结果对象
- ✅ 更新了所有调用者以使用新模式
- ✅ 添加了 JSDoc 以提高清晰度
### 仍需您处理:
- [ ] **[阻塞性]** 添加 XSS 安全注释(请参阅审查中的建议)
- [ ] 考虑提取 renderDiffPanel 辅助函数(可选)
现在所有代码检查都通过了。请审查提交并处理剩余项目。
EOF
)"
修复的提交消息格式:
fix: <简短描述>
<修复内容和原因的详细说明>
更改:
- <具体更改 1>
- <具体更改 2>
处理审查反馈:<指向审查评论的链接>
对于大多数 PR,使用此混合策略:
典型工作流:
# 1. 发布包含摘要的全面审查评论(来自步骤 8)
gh pr comment <PR-编号> --repo {owner}/{repo} --body "<详细审查摘要>"
# 2. 为所有可修复的问题提交包含 GitHub 建议的审查
# (如方法 1 所示创建包含所有建议的 JSON)
gh api POST repos/{owner}/{repo}/pulls/<PR-编号>/reviews \
--input /tmp/review-suggestions.json
# 3. 添加关于建议的友好摘要
gh pr comment <PR-编号> --repo {owner}/{repo} --body "$(cat <<'EOF'
## ✨ 已添加一键建议!
我已为可修复的问题添加了 GitHub 建议:
- ✅ [阻塞性] 安全文档
- ✅ 错误处理改进
- ✅ CSS 选择器重构
转到 **Files changed** 标签页并点击 **"Commit suggestion"** 来应用!
EOF
)"
# 4. 对于主观/架构性问题,单独添加指导性评论
# (仅在需要时 - 大多数问题应该有建议)
实际示例分布:
对于一个有 10 个问题的典型 PR:
此方法提供:
PR #196:在差异工具中支持未发布的页面
审查中发现的问题:
采取的行动:
# 创建了两次审查,共包含 9 个 GitHub 建议
# - diff.js 的 4 个建议(XSS 注释、错误处理)
# - diff.css 的 5 个建议(选择器重构)
# 发布了友好的摘要评论解释一键应用
# 结果:PR 作者可以在约 30 秒内修复所有问题,而不是 5-10 分钟的手动工作
查看实际实现:
节省的时间:
使用 GitHub 建议当:(默认 - 约 70-80% 的时间使用此方法)
使用修复提交当:(罕见 - 仅在建议不起作用时使用)
仅使用指导当:(约 20-30% 的问题)
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 correctposition 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:
Weekly Installs
0
Repository
GitHub Stars
40
First Seen
Jan 1, 1970
Security Audits
agent-browser 浏览器自动化工具 - Vercel Labs 命令行网页操作与测试
136,300 周安装