Superpowers 教程 7：requesting/receiving code review（代码审查工作流）

教程概述

系列目录：Superpowers 教程系列索引

Superpowers 包含两个代码审查相关的技能：requesting-code-review 和 receiving-code-review。这两个技能配合使用，确保代码审查过程的技术严谨性和高效性。

你将学到

✅ requesting-code-review 的触发时机和流程
✅ receiving-code-review 的反馈处理策略
✅ 审查问题严重性分级
✅ 技术严谨性 vs 表演性同意
✅ 实际审查案例分析

为什么代码审查如此重要？

没有审查的问题

flowchart LR
    A[写完代码] --> B[直接提交]
    B --> C[问题未被发现]
    C --> D[生产环境 Bug]
    D --> E[紧急修复]
    E --> F[引入新问题]
    
    style B fill:#ffcccc
    style C fill:#ffcccc
    style D fill:#ffcccc

有审查的流程

flowchart TD
    A[完成实现] --> B[requesting-code-review]
    B --> C[审查问题列表]
    C --> D{严重性问题？}
    D -->|是 | E[阻止提交，修复]
    D -->|否 | F[记录技术债务]
    E --> G[receiving-code-review]
    G --> H[实施修复]
    H --> I[再次审查]
    I --> J[通过提交]
    
    style B fill:#e1f5ff
    style E fill:#ffebee
    style G fill:#e8f5e9
    style J fill:#e8f5e9

审查效果

指标	无审查	有审查
Bug 逃逸率	高	降低 70-80%
代码一致性	低	高
知识共享	无	有
技术债务	积累快	可控

requesting-code-review 技能

触发时机

requesting-code-review 在以下场景自动触发：

任务完成时 - 实现计划中的任务完成
功能完成时 - 主要功能实现完成
合并前 - 准备合并到主分支前

审查请求流程

sequenceDiagram
    participant D as 开发者 (AI)
    participant R as requesting-code-review
    participant C as 审查者 (AI/人)
    
    D->>R: 任务完成，请求审查
    R->>R: 准备审查清单
    R->>C: 提交审查请求 + 上下文
    
    Note over C: 审查内容：
    Note over C: - 是否符合设计规范
    Note over C: - 代码质量
    Note over C: - 测试覆盖
    Note over C: - 潜在问题
    
    C->>R: 返回审查意见
    R->>R: 按严重性分类
    R->>D: 返回审查结果
    
    alt 有关键问题
        D->>D: 修复问题
        D->>R: 重新请求审查
    else 无关键问题
        D->>D: 继续下一个任务
    end

审查清单

requesting-code-review 会自动准备以下审查清单：

## 代码审查请求

**任务**: Task 5 - 实现标签 API 端点

**设计文档**: `.project/designs/feature-tag-system.md`

**变更文件**:
- `app/Controllers/TagController.php` (新建，156 行)
- `routes/api.php` (修改，+8 行)
- `tests/Feature/TagApiTest.php` (新建，234 行)

**自测完成**:
- [x] 单元测试通过
- [x] 集成测试通过
- [x] 手动测试通过
- [x] Lint 检查通过

**审查重点**:
1. API 设计是否符合 REST 规范
2. 错误处理是否完整
3. 输入验证是否充分

**潜在问题**:
- 分页实现可能需要优化
- 错误响应格式需要统一

问题严重性分级

审查意见按严重性分为四级：

🔴 Critical（严重）

定义：必须修复，阻止提交

示例：

🔴 Critical:
- 安全漏洞（SQL 注入、XSS）
- 功能与设计规格不符
- 关键逻辑错误
- 数据损坏风险

修复要求：必须立即修复，阻止后续进度

🟠 Major（主要）

定义：应该修复，但可以讨论

示例：

🟠 Major:
- 代码重复
- 缺少错误处理
- 性能问题
- 测试覆盖不足

修复要求：建议修复，如不修复需记录技术债务

🟡 Minor（次要）

定义：可以修复，不影响功能

示例：

🟡 Minor:
- 命名不够清晰
- 注释不足
- 代码格式问题
- 小的优化建议

修复要求：时间允许时修复

📝 Note（备注）

定义：仅供参考，无需行动

示例：

📝 Note:
- 未来可能的优化
- 替代方案建议
- 文档改进建议

修复要求：无需行动，仅供参考

审查报告示例

## 代码审查报告

**审查任务**: Task 5 - 标签 API 端点
**审查者**: AI Reviewer
**审查时间**: 2026-02-28 15:30

---

### 🔴 Critical (1)

1. **SQL 注入风险**
   
   **位置**: `TagController.php:45`
   
   **问题**:
   ```php
   // ❌ 危险：直接拼接 SQL
   $query = "SELECT * FROM tags WHERE name = '$name'";

建议:

// ✅ 使用参数化查询
$query = "SELECT * FROM tags WHERE name = ?";
$stmt->execute([$name]);

阻止提交: 是

🟠 Major (2)

缺少输入验证

位置: TagController.php:32

问题: 没有验证标签名长度

建议: 添加长度验证

阻止提交: 否
测试覆盖不足

位置: tests/Feature/TagApiTest.php

问题: 错误场景测试缺失

建议: 添加 400/500 错误测试

阻止提交: 否

🟡 Minor (1)

变量命名不清晰

位置: TagController.php:78

问题: $t 命名含义不明

建议: 改为 $tag

阻止提交: 否

📝 Note (1)

未来优化: 考虑添加标签搜索功能

审查总结

Critical: 1 (必须修复)
Major: 2 (建议修复)
Minor: 1 (可选修复)
Note: 1 (参考)

审查结论: ❌ 不通过（有关键问题）

下一步: 修复 Critical 问题后重新提交审查


## receiving-code-review 技能

### 触发时机

receiving-code-review 在以下场景**自动触发**：

1. **收到审查反馈时** - 审查报告返回后
2. **实施建议前** - 准备修改代码前
3. **反馈不清晰时** - 需要理解审查意见时

### 核心原则

receiving-code-review 遵循以下原则：

```yaml
receiving_code_review:
  principles:
    - "技术严谨性优先于表演性同意"
    - "验证而非盲目实施"
    - "质疑不合理建议"
    - "寻求技术合理性解释"
  
  anti_patterns:
    - "盲目同意所有建议"
    - "不验证直接实施"
    - "表演性感谢但无行动"
    - "忽视技术上有问题的建议"

反馈处理流程

flowchart TD
    A[收到审查反馈] --> B[receiving-code-review]
    B --> C[分析每个问题]
    C --> D{问题合理吗？}
    D -->|是 | E[实施修复]
    D -->|否 | F[提出质疑]
    F --> G{审查者回应}
    G --> H[合理解释]
    H --> E
    G --> I[解释不合理]
    I --> J[记录分歧]
    E --> K[重新提交审查]
    
    style B fill:#e1f5ff
    style E fill:#e8f5e9
    style J fill:#fff3e0

反馈响应策略

响应合理问题

**审查意见**: SQL 注入风险

**响应**:
✅ 问题合理，立即修复

**修复计划**:
1. 将所有直接 SQL 拼接改为参数化查询
2. 添加 SQL 注入防护测试
3. 使用 ORM 或查询构建器

**修复后验证**:
- 运行安全测试
- 重新提交审查

响应不合理问题

**审查意见**: 将所有函数改为单行（过度优化）

**响应**:
⚠️ 建议商榷

**技术理由**:
1. 单行实现会降低可读性
2. 当前实现有清晰的逻辑分离
3. 没有性能问题需要这样优化

**建议**:
- 保持当前实现
- 记录为"已审查，决定不采纳"
- 如有性能问题，提供基准测试

**等待回应**: 是

响应不清晰问题

**审查意见**: "这个设计有问题"

**响应**:
❓ 需要澄清

**请求澄清**:
1. 具体是哪个设计方面有问题？
2. 问题会导致什么后果？
3. 有具体的改进建议吗？

**等待**: 澄清后再实施

实施修复的最佳实践

1. 逐个问题处理

# ❌ 坏：一次性修改所有内容
git commit -am "Fix all review comments"

# ✅ 好：每个问题单独提交
git commit -m "fix: address SQL injection vulnerability (review #1)"
git commit -m "feat: add input validation (review #2)"
git commit -m "refactor: improve variable naming (review #3)"

2. 保留审查上下文

提交信息格式：

fix: [问题类型] [审查引用]

[问题描述] [修复方案] [验证方法]

Review: #123


示例：

fix: security SQL injection vulnerability

Use parameterized queries instead of string concatenation
Add prepared statement for tag name lookup
Add SQL injection test case

Review: #123, Critical #1

3. 验证修复效果

# 修复后运行测试
pytest tests/ -v --tb=short

# 重新运行审查
# requesting-code-review 会自动触发

# 确认问题已解决
# 审查者确认

实际案例分析

案例 1：合理问题的处理

场景：API 端点缺少错误处理

审查意见：

🟠 Major: 缺少错误处理

位置：TagController.php:56

问题：数据库查询失败时没有错误处理

建议：
```php
try {
    $tags = $this->tagRepository->getAll();
    return response()->json($tags);
} catch (Exception $e) {
    return response()->json(['error' => 'Failed to fetch tags'], 500);
}


**响应**：
```markdown
✅ 接受建议

修复：
- 添加 try-catch 块
- 记录错误日志
- 返回友好的错误消息

验证：
- 添加数据库错误测试
- 手动测试错误场景

案例 2：不合理问题的处理

场景：审查者要求删除所有注释

审查意见：

🟡 Minor: 注释过多

问题：代码应该自解释，不需要这么多注释

建议：删除所有注释

响应：

⚠️ 建议商榷

技术理由：
1. 复杂业务逻辑需要注释解释"为什么"
2. 公共 API 需要文档注释
3. 团队规范要求 JSDoc

当前注释分类：
- 文档注释（必要）：保留
- 业务逻辑解释（必要）：保留  
- 冗余注释（如 `i++ // 增加 i`）：删除

决定：保留必要注释，删除冗余注释

案例 3：模糊问题的澄清

审查意见：

🟠 Major: 这个实现不够好

建议：重构

响应：

❓ 请求澄清

具体问题：
1. "不够好"的具体标准是什么？
2. 有什么问题（性能/可读性/可维护性）？
3. 期望的重构方向是什么？

当前实现的优点：
- 测试覆盖完整
- 符合设计规范
- 性能基准达标

等待更具体的审查意见后再行动

技术严谨性 vs 表演性同意

表演性同意的反模式

# ❌ 坏：表演性同意

"感谢您的宝贵意见！您说得非常对！
我会立即按照您的建议修改！
您真是帮了大忙了！"

然后：
- 不验证建议的合理性
- 盲目实施可能有问题的建议
- 没有技术讨论

技术严谨性的做法

# ✅ 好：技术严谨

"感谢审查。关于问题 #1：

我的分析：
- 这个问题确实存在 SQL 注入风险
- 我测试了恶意输入 ' OR '1'='1
- 确认可以绕过验证

修复方案：
- 使用参数化查询（见上面的代码）
- 添加 SQL 注入测试用例
- 考虑使用 ORM 完全避免此类问题

关于问题 #2：
- 我认为这个建议值得商榷
- 理由是...
- 建议保持当前实现

请确认我的理解是否正确。"

对比效果

	表演性同意	技术严谨
代码质量	不稳定	稳定提升
问题发现	晚	早
团队学习	无	有
审查价值	低	高

审查自动化

自动审查规则

code_review:
  auto_check:
    # 代码风格
    - lint_passed
    - format_correct
    
    # 测试
    - tests_passed
    - coverage_minimum: 85%
    
    # 安全
    - no_sql_injection
    - no_xss_vulnerability
    - no_hardcoded_secrets
    
    # 质量
    - no_duplicate_code
    - function_length_max: 50
    - cyclomatic_complexity_max: 10

审查机器人集成

code_review:
  integrations:
    - github_pr_review
    - gitlab_mr_review
    - phabricator
    - gerrit
  
  notifications:
    - slack
    - email
    - teams

小结

代码审查技能确保技术严谨性和高质量代码：

requesting-code-review - 主动寻求反馈
receiving-code-review - 严谨处理反馈
问题分级 - Critical/Major/Minor/Note
验证优先 - 不盲目同意或实施

关键要点

✅ 主动请求审查，不要逃避
✅ 技术严谨，不要表演性同意
✅ 问题分级处理，Critical 必须修复
✅ 验证审查意见的合理性

审查金句

“代码审查不是找茬，是集体提升代码质量。”

“表演性同意是技术债务的温床。”

系列导航：

← 上一篇：教程 6：using-git-worktrees - 隔离开发工作流
→ 下一篇：教程 8：subagent-driven-development 与并行任务
返回：教程系列索引

教程概述

你将学到

为什么代码审查如此重要？

没有审查的问题

有审查的流程

审查效果

requesting-code-review 技能

触发时机

审查请求流程

审查清单

问题严重性分级

🔴 Critical（严重）

🟠 Major（主要）

🟡 Minor（次要）

📝 Note（备注）

审查报告示例

🟠 Major (2)

🟡 Minor (1)

📝 Note (1)

审查总结

反馈处理流程

反馈响应策略

响应合理问题

响应不合理问题

响应不清晰问题

实施修复的最佳实践

1. 逐个问题处理

2. 保留审查上下文

3. 验证修复效果

实际案例分析

案例 1：合理问题的处理

案例 2：不合理问题的处理

案例 3：模糊问题的澄清

技术严谨性 vs 表演性同意

表演性同意的反模式

技术严谨性的做法

对比效果

审查自动化

自动审查规则

审查机器人集成

小结

关键要点

审查金句

参考资源

🍪 Cookie 使用通知

Cookie 偏好设置

必要 Cookie

分析 Cookie

广告 Cookie