bm-review-checklist

name	bm.review-checklist
description	代码审查清单：结构化 code review 流程、质量门禁、最佳实践检查。Use when user requests code review, PR review, quality check, 审查, review, 代码审查, PR检查. 涵盖安全、性能、可维护性、测试覆盖等维度。
source	opencrew
version	20260521.01

Skill: 代码审查清单

多维度代码审查的检查项和输出格式。

文件落点

• 最终产物：./reviews/...（cwd 下，可见目录，跟随用户惯例）
• 中间产物：./working/review/
• 目录不存在：主动创建；用户已有惯例则跟随
• 永远在 cwd 内：不写 /tmp/、~/Desktop/、~/Downloads/ 等 cwd 之外位置（用户明确指定除外）

---

审查流程

Step 1：理解上下文

委派方会告诉你：

• 改了什么（新功能/bug fix/重构）
• 涉及哪些文件
• 需要关注的重点

如果没给上下文，自己读 commit message 和 diff 摘要推断。

Step 2：读代码

1. 读被改动的文件，理解改动意图
2. 读相关上下文文件（被调用的函数、依赖的模块）
3. 检查是否跟项目现有模式一致

Step 3：逐维度检查

维度一：正确性

• 核心逻辑是否符合预期（对照需求/spec）
• 边界条件：空值 null/undefined/None、零值 0/""、负数、超大值
• 类型是否正确（类型推断 vs 显式标注）
• 并发/竞态：共享状态、异步操作顺序、锁的范围
• 资源泄漏：文件句柄、数据库连接、内存（特别是循环内创建的）
• 幂等性：重复调用是否安全

维度二：安全性

• 输入校验：用户输入、外部 API 返回值是否被信任
• SQL 注入：是否用了参数化查询，不是字符串拼接
• XSS：用户生成内容是否被转义
• CSRF：是否有 token 验证
• 权限检查：当前用户有权做这个操作吗
• 敏感数据：密码/token 不落日志、不暴露在 API 响应中
• 加密：传输层 HTTPS、存储层 hash/encrypt

维度三：性能

• N+1 查询：循环内有没有数据库调用
• 全量加载：是否需要分页/懒加载/流式处理
• 缓存策略：热点数据是否缓存、缓存失效策略是否合理
• 内存使用：大数组/列表的处理（是否可以 streaming）
• 异步 vs 同步：I/O 操作是否异步

维度四：可维护性

• 命名：函数名是否表达意图（不需要注释解释）
• 函数长度：超过 30 行考虑拆分
• 耦合：改 A 是否会影响 B
• 重复：3 处以上相同逻辑 → 抽取公共函数
• 魔法数字/字符串：硬编码常量 → 提取为命名常量
• 过度抽象：只有 1 个实现时不需要 interface/策略模式

维度五：一致性

• 代码风格是否跟项目一致（命名、格式、结构）
• 错误处理方式是否一致（throw vs return error vs Result）
• 日志格式是否一致
• API 设计风格是否一致（RESTful、命名约定）

维度六：错误处理

• 异常被正确捕获（不是空 catch/except pass）
• 错误信息有意义（不只是 "error" 或 "something went wrong"）
• 失败时有合理降级或回滚
• 错误可追踪（有足够上下文：请求 ID、用户 ID、关键参数）

严重程度分级

级别	含义	示例
🔴 必须改	会导致 bug/安全问题/数据丢失	SQL 注入、空指针、资源泄漏、权限绕过
🟡 建议改	不改不会炸，但应该改	N+1 查询、命名不清晰、缺少错误处理、缺少测试
🟢 可选	锦上添花	代码风格微调、微小可维护性提升

输出格式

## 审查结论
[PASS / PASS_WITH_NOTES / REQUEST_CHANGES]

## 🔴 必须修改
- [问题描述] `文件路径:行号` — [原因] → 建议改为 [具体改法]

## 🟡 建议修改
- [问题描述] `文件路径:行号` — [原因]

## 🟢 可选改进
- [建议]

## 总结
[一两句话总评]

审查原则

• 代码没问题就不要硬找问题。PASS 就是 PASS
• 每个 🔴/🟡 必须带文件路径和行号
• 每个问题给具体改法，不只是指出问题
• 不挑格式问题（那是 linter 的活）
• 不熟悉的领域标注"此处不确定，建议找熟悉的人看"