qe-code-review-quality

代码审查质量

<default_to_action> 审查代码或制定审查规范时：

1. 优先级排序： 🔴 阻断项（必须修复）→ 🟡 重大问题 → 🟢 次要问题 → 💡 建议
2. 关注： Bug、安全性、可测试性、可维护性（而非风格偏好）
3. 多提问，少命令：“你是否考虑过……？” > “把这个改成……”
4. 提供背景： 说明为什么这很重要，而不只是指出要改什么
5. 控制范围： 每次审查少于 400 行，效果更好

快速审查清单：

• 逻辑：能否正确运行？是否处理了边界情况？
• 安全性：是否验证输入？是否有身份验证检查？是否存在注入风险？
• 可测试性：能否测试？是否已经测试？
• 可维护性：命名是否清晰？是否遵循单一职责？是否遵循 DRY？
• 性能：是否存在 O(n²) 循环？N+1 查询？内存泄漏？

关键成功因素：

• 审查代码，而非针对个人
• 发现 Bug > 吹毛求疵地挑剔风格
• 快速反馈（< 24h）> 详尽反馈 </default_to_action>

快速参考卡

适用场景

• PR 代码审查
• 结对编程反馈
• 制定团队审查规范
• 指导开发者

反馈优先级

级别	图标	含义	操作
阻断项	🔴	Bug/安全问题/崩溃	合并前必须修复
重大问题	🟡	逻辑问题/测试缺口	合并前应修复
次要问题	🟢	风格/命名	最好修复
建议	💡	替代方案	可考虑用于后续工作

审查范围限制

修改行数	建议
< 200	一次审查完成
200-400	分块审查
> 400	请求拆分 PR

关注事项

✅ 审查	❌ 跳过
逻辑正确性	格式（使用 linter）
安全风险	命名偏好
测试覆盖率	架构争论
性能问题	风格意见
错误处理	无关紧要的更改

---

反馈模板

阻断项（必须修复）

🔴 **BLOCKER: SQL Injection Risk**

This query is vulnerable to SQL injection:
```javascript
db.query(`SELECT * FROM users WHERE id = ${userId}`)

修复： 使用参数化查询：

db.query('SELECT * FROM users WHERE id = ?', [userId])

原因： 直接将用户输入写入 SQL，会允许攻击者执行任意查询。

### Major (Should Fix)
```markdown
🟡 **MAJOR: Missing Error Handling**

What happens if `fetchUser()` throws? The error bubbles up unhandled.

**Suggestion:** Add try/catch with appropriate error response:
```javascript
try {
  const user = await fetchUser(id);
  return user;
} catch (error) {
  logger.error('Failed to fetch user', { id, error });
  throw new NotFoundError('User not found');
}

### Minor (Nice to Fix)
```markdown
🟢 **minor:** Variable name could be clearer

`d` doesn't convey meaning. Consider `daysSinceLastLogin`.

建议（可考虑）

💡 **suggestion:** Consider extracting this to a helper

This validation logic appears in 3 places. A `validateEmail()` helper would reduce duplication. Not blocking, but might be worth a follow-up PR.

---

可提出的审查问题

逻辑

• 当 X 为 null/空/负数时会发生什么？
• 这里是否存在竞态条件？
• 如果 API 调用失败会怎样？

安全性

• 是否验证/净化了用户输入？
• 是否进行了身份验证检查？
• 是否暴露了机密或个人身份信息？

可测试性

• 你会如何测试这个？
• 依赖项是否可注入？
• 是否测试了正常路径和边界情况？

可维护性

• 下一位开发者能看懂这段代码吗？
• 这段代码是否承担了太多职责？
• 是否存在可以减少的重复代码？

---

Agent 辅助审查

// Comprehensive code review
await Task("Code Review", {
  prNumber: 123,
  checks: ['security', 'performance', 'testability', 'maintainability'],
  feedbackLevels: ['blocker', 'major', 'minor'],
  autoApprove: { maxBlockers: 0, maxMajor: 2 }
}, "qe-quality-analyzer");

// Security-focused review
await Task("Security Review", {
  prFiles: changedFiles,
  scanTypes: ['injection', 'auth', 'secrets', 'dependencies']
}, "qe-security-scanner");

// Test coverage review
await Task("Coverage Review", {
  prNumber: 123,
  requireNewTests: true,
  minCoverageDelta: 0
}, "qe-coverage-analyzer");

---

Agent 协作提示

记忆命名空间

aqe/code-review/
├── review-history/*     - Past review decisions
├── patterns/*           - Common issues by team/repo
├── feedback-templates/* - Reusable feedback
└── metrics/*            - Review turnaround time

多 Agent 协作

const reviewFleet = await FleetManager.coordinate({
  strategy: 'code-review',
  agents: [
    'qe-quality-analyzer',    // Logic, maintainability
    'qe-security-scanner',    // Security risks
    'qe-performance-tester',  // Performance issues
    'qe-coverage-analyzer'    // Test coverage
  ],
  topology: 'parallel'
});

---

审查礼仪

✅ 应做	❌ 不应做
“你是否考虑过……？”	“这不对”
解释为什么这很重要	只说“修复这个”
肯定好的代码	只指出负面问题
提出建议，而非强求	居高临下
审查 < 400 行	一次审查 2000 行

---

相关技能

• agentic-quality-engineering - Agent 协作
• security-testing - 深入审查安全性
• refactoring-patterns - 可维护性模式

---

请记住

优先处理反馈： 🔴 阻断项 → 🟡 重大问题 → 🟢 次要问题 → 💡 建议。关注 Bug 和安全性，而非风格。多提问，少命令。每次审查少于 400 行。快速反馈（< 24h）胜过详尽反馈。

借助 Agent： Agent 可自动检查安全性、性能和覆盖率，让人工审查者专注于逻辑和设计。使用 Agent 进行一致且快速的初步审查。