code-review

name	code-review
description	代码审查，从需求符合度/正确性/安全性/可维护性/性能五维度评审代码变更，输出风险分级的 Finding 列表
when_to_use	用于代码变更评审、PR review、实现与 spec/PRD 的一致性核对、五维度质量审查时调用。 典型触发："审查代码" / "PR review" / "看一下这段代码" / "代码合规吗" / "评估实现是否符合需求" / "代码和 PRD 对不对得上" / "有没有明显 bug 或缺陷" / "研发交付的代码帮我看下"。 不用于：bug 的根因定位与修复方案（用 systematic-debugging——但**发现** bug 属于本 skill）/ 编码本身（用 technical-design 完整路径段）/ 功能测试（用 test-case-design）。
user-invocable	true
allowed-tools	["Read","Glob","Grep","Bash"]

代码审查技能

产物去向

审查结论默认只在响应中呈现。用户要求存档时按 skill: document-norms §1 落 projects/modules/<basic>/<sub>/requirements/<req_slug>/<sub_req_slug>/reviews/； 发现的缺陷若需跟踪，走 projects/issues/ + board task 草稿（不直接写 board）。

适用场景

• @dev 交付代码变更后
• @qa 做回归验证的前置检查
• 安全/质量疑点时的深度审查

四步流程

第 1 步：变更范围识别

读取代码变更（diff 或指定文件），识别：

维度	内容
改动文件	新增/修改/删除的文件清单
改动范围	每个文件的具体改动行数和函数
影响模块	直接调用改动代码的其他模块
依赖变化	新增/移除的依赖关系

第 2 步：多维度审查

2.1 需求符合度（Requirement Conformance）

前提：先拿到规范资产（PRD 正文 / AC / spec / 词表 / 接口约定），逐条比对，不能只看 AC —— AC 是抽样，正文才是全量口径。

审查点	检查内容
规则落地	规范正文写明的每条规则，代码是否实现且口径一致
缺漏	规范写了、代码没做（含只做了一半、静默降级、开关/埋点等非功能要求）
超纲	代码做了、规范没写（自行发挥的逻辑，需回溯确认）
规范空白	规范本身没定，代码"忠实实现了不完整的规则" —— 不算实现错误，退回需求侧补口径
已符合项	明确列出实现正确的部分，供修其他问题时防误伤

「规范空白」与「缺漏」必须分开报：前者要 PM/需求方先拍板，研发改了也是白改；后者研发直接修。

2.2 正确性（Correctness）

审查点	检查内容
逻辑正确	代码实现是否符合需求/AC
边界处理	空值、零值、极值、负数等边界条件
错误处理	异常捕获、错误返回、失败恢复
并发安全	竞态条件、死锁、共享状态
类型安全	类型断言、null 检查

2.3 安全性（Security）

审查点	检查内容
注入风险	SQL 注入、命令注入、XSS
权限校验	未登录访问、越权访问
敏感信息	密码/密钥泄露、日志打印敏感数据
输入校验	未校验的用户输入、文件上传
依赖安全	已知漏洞的依赖库

2.4 可维护性（Maintainability）

审查点	检查内容
命名清晰	变量、函数、类名能自解释
结构清晰	单一职责、合理抽象、避免过深嵌套
注释充分	复杂逻辑有解释，公开接口有文档
代码复用	避免重复代码，合理提取公共函数
测试覆盖	关键逻辑有测试

2.5 性能（Performance）

审查点	检查内容
算法复杂度	O(n²) 或更差的循环
数据库查询	N+1 查询、缺失索引、全表扫描
网络请求	串行请求应并行、缺失缓存
内存使用	大对象加载、内存泄漏
IO 操作	同步阻塞 IO、文件句柄泄漏

第 3 步：Finding 分级与归因

每个 Finding 打两个正交标签：风险级别（多急）+ 依据来源（凭什么）。

风险级别：

级别	含义	处理要求
Critical	必须修复	阻塞合并/发布，P0 问题
Warning	建议修复	下次迭代或同一 PR 修复
Info	建议改进	记录待办，不阻塞

依据来源（决定谁来处理、能不能拒绝）：

来源	含义	处理方
违反规范	规范 / spec / PRD 明文写了，实现没做到	实现方直接修，无需再讨论口径
规范空白	规范本身没定，实现只是「忠实实现了不完整的规则」	需求方先补口径，实现方此时改了也可能白改
审查者判断	无规范依据，属审查者的经验或偏好（架构选型、分层、工程实践）	建议性质，实现方可拒绝；单独分区，不与前两类混排

审查方通常看不到对方的发版流程、依赖约束、历史包袱，架构决策权在实现方。把「我认为这样更好」和「你违反了确认过的规范」混在一起报，会稀释真缺陷的紧迫感。

第 4 步：审查结论

## 代码审查报告：{Task-ID / PR-ID}

### 审查范围
- 改动文件：{N} 个
- 改动行数：+{N} / -{N}
- 审查维度：需求符合度 / 正确性 / 安全性 / 可维护性 / 性能
- 比对的规范资产：{PRD / spec / 词表 / 接口约定，列出文件与版本}

### 已确认符合的部分（修其他问题时勿破坏）
- {规范条目} — {实现位置}

### Finding 列表（依据来源 = 违反规范）

#### 🔴 Critical ({N})
- **F-001**: {描述} — `文件:行号`
  - 依据：{PRD §X / spec 明文 / 团队规范}
  - 问题：{具体问题}
  - 建议：{修复建议}

#### 🟡 Warning ({N})
- **F-002**: {描述} — `文件:行号`
  - 依据 / 问题 / 建议：{...}

#### 🔵 Info ({N})
- **F-003**: {描述} — `文件:行号`

### 需求方待定（依据来源 = 规范空白，{N}）

- **N-001**: {描述} — 规范未定；当前实现 {现状}；备选 {a} / {b}，需 {需求方} 拍板后再排期

### 审查者建议（依据来源 = 审查者判断，可拒绝，{N}）

- **S-001**: {描述} — 无规范依据，属工程偏好，最终方案由实现方按自身约束判断

### 审查结论

- ✅ **通过** — 无 Critical，Warning ≤2
- ⚠️ **有条件通过** — 无 Critical，但有 Warning 需后续修复
- ❌ **不通过** — 有 Critical，必须修复后重新审查

与其他技能的协作

• test-case-design：代码审查与测试用例设计互补，审查静态、测试动态
• systematic-debugging：发现 Critical 问题时转为 Bug 追踪流程
• technical-design：如果审查发现方案设计缺陷，退回给 @dev 重新设计

审查优先级（时间有限时）

1. 必查：Critical 级别的安全问题和正确性问题
2. 重点查：修改密集区、历史 Bug 高发区
3. 快速扫：简单的文本修改、配置变更

反模式

• ❌ 只看代码不看上下文（没读 spec 就审查实现）
• ❌ 只对着 AC 审，不逐条比对规范正文（AC 是抽样，正文才是全量口径）
• ❌ 以审查者的架构偏好冒充缺陷（无规范依据的建议混进 Finding 列表，稀释真缺陷）
• ❌ 把「规范空白」当实现错误报给实现方（应退回需求方补口径）
• ❌ 纠结风格问题，忽视安全和正确性
• ❌ 只说"这里有问题"，不说"应该怎么改"
• ❌ 对 Info 级别吹毛求疵，浪费时间
• ❌ 用 @dev 的自测结果代替独立审查