# Code Review

> 审查自指定固定点（commit、branch、tag、PR 引用或 merge-base）以来的改动，分两轴——规范（代码是否遵守本仓库已记录的编码规范？）与规格（代码是否实现了原始 issue / 规格要的东西？）。两轴依次审查，各自验证存活的发现才入报告，结果并排呈现。当用户要 review 分支、PR、WIP 改动，或说'review since X'时使用。

- Skill: `cislunarspace/code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add cislunarspace/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/cislunarspace/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: cislunarspace (https://skillmd.com/u/cislunarspace)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/cislunarspace/code-review

---


对 `HEAD` 与固定点之间的 diff 做两轴审查：

- **规范（Standards）**——代码是否遵守本仓库已记录的编码规范？
- **规格（Spec）**——代码是否忠实地实现了原始 issue / 规格？

两轴**依次**审查，各自产出一份独立报告，最终由本 skill 汇总两边发现。

issue tracker 应当已经提供给你——若 `docs/agents/issue-tracker.md` 不存在，跑 `/setup-ouyangjiahong-skills`。

## 流程

### 1. 钉住固定点

从参数按这个顺序认固定点：

1. PR/issue 引用（`#473`、GitLab `!67`）——按 `docs/agents/issue-tracker.md` 的流程拉取，PR 描述留给步骤 2 当规格，固定点取 PR 的 base 分支（`origin/<base>`）。
2. 明确的 git 引用——commit SHA、分支名、tag、`main`、`HEAD~5`。
3. 什么都没给——用 `@{upstream}`（上游跟踪分支）。
4. 认不出，或没有上游跟踪——问用户。

把 diff 命令一次性记下来：`git diff <fixed-point>...HEAD`（三点，比较的是 merge-base）。同时记下 commit 列表：`git log <fixed-point>..HEAD --oneline`。

继续之前，先确认固定点能解析（`git rev-parse <fixed-point>`）、diff 非空。坏引用、空 diff、PR 场景的分支不匹配，按边界情况表处理——在这里就挂，不要拖到审查中途才挂。

### 2. 找规格来源

按这个顺序找原始规格：

1. 步骤 1 拉到的 PR 描述——参数里给了 PR 引用时，它就是首选规格。
2. commit message 里的 issue 引用（`#123`、`Closes #45`、GitLab `!67` 等）——按 `docs/agents/issue-tracker.md` 的流程拉取。
3. 用户作为参数传入的路径。
4. 本会话中用户的原始请求——这批提交出自当前会话时，用户当初的指令就是规格。
5. `docs/`、`specs/`、`.scratch/` 下与分支名或功能匹配的规格文件。
6. 都没找到，问用户规格在哪。用户说没有规格，则**规格**轴跳过，报告"无规格可用"。

### 3. 找规范来源

仓库里所有声明"代码该怎么写"的东西，比如 `CODING_STANDARDS.md` 或 `CONTRIBUTING.md`。

在仓库声明之上，规范轴永远带**异味基线**——一组固定的 Fowler 代码异味（_Refactoring_ 第 3 章），即使仓库什么都没声明，它也适用。两条规则绑定它：

- **仓库优先。** 已记录的仓库规范永远赢；仓库在某处明确认可了基线会标红的东西，压住这条异味。
- **永远是判断题。** 每条异味是一个带标签的启发式判断（"疑似 Feature Envy"），不是硬性违反——和这里任何一条规范一样，工具已经在管的，跳过。

每条异味按 *是什么* → *怎么修* 读；对照 diff 比对：

- **神秘命名（Mysterious Name）**——函数、变量或类型的名字没说清它做什么、装什么。→ 改名；想不出诚实的名字，是设计浑浊的信号。
- **重复代码（Duplicated Code）**——同样的逻辑形状在改动的多个 hunk 或文件里出现。→ 抽出共形，两边都调它。
- **依恋情结（Feature Envy）**——一个方法更多地伸手到别的对象的数据里，而不是自己的。→ 把方法搬到它羡慕的那份数据上。
- **数据泥团（Data Clumps）**——同样几个字段或参数老黏在一起出行（一个呼之欲出的类型）。→ 拢成一个类型，传它。
- **基本类型偏执（Primitive Obsession）**——用基本类型或字符串代替本该有自己类型的领域概念。→ 给概念一个自己的小类型。
- **重复 switch（Repeated Switches）**——对同一类型的 `switch` / `if` 阶梯在改动里反复出现。→ 用多态替换，或两边共享一张 map。
- **霰弹手术（Shotgun Surgery）**——一个逻辑改动逼着在 diff 的许多文件里散点改。→ 把一起变的东西拢到一个模块。
- **发散式变化（Divergent Change）**——一个文件或模块因几种互不相关的原因被改。→ 拆开，让每个模块只为一种原因变。
- **投机式泛化（Speculative Generality）**——为规格没提的需要加了抽象、参数或钩子。→ 删掉，inline 回去，直到真实需要出现。
- **消息链（Message Chains）**——一长串 `a.b().c().d()` 导航，调用方本不该依赖。→ 把这段走步藏在第一个对象上的一个方法里。
- **中间人（Middle Man）**——一个类或函数主要只是转发。→ 删掉，直接调真正的目标。
- **被拒的遗赠（Refused Bequest）**——子类或实现者把继承来的大部分东西忽略或 override。→ 放弃继承，改用组合。

### 4. 依次跑两轴

先跑**规范轴**，再跑**规格轴**。每轴先审后验，两步走完才算这轴的报告；两轴的发现互不合并、互不重排。

**审**——按任务说明过一遍 diff，记下草稿发现：

- **规范轴的任务**——对照规范源文件（步骤 3）和上文的异味基线：按文件 / hunk 记（在相关处）：(a) diff 违反了哪条已记录的规范——引用规范（文件 + 那条规则）；(b) 你看到的基线异味——点名并引用对应 hunk。区分硬性违反和判断题——已记录规范的违反可以是硬性的，但基线异味永远是判断题，且已记录的仓库规范压住基线。工具已经在管的，跳过。
- **规格轴的任务**——对照规格（路径或拉取到的内容）：记 (a) 规格要、但缺失或半成品的需求；(b) diff 里有、但规格没要的行为（范围蔓延）；(c) 看似已实现、但实现看起来不对的需求。每条配规格的对应行。

**验**——对着 diff 逐条复核草稿发现，存活才进报告：

- 引用的事实对回去：hunk 里真有那行、规范文件真有那条规则、规格真有那句要求。
- 站在被审代码的立场反驳一次——行为有解释（别处已经处理、规格确有此意、仓库规范认可这种写法）的，丢弃。
- 反驳不动、引用坐实的，保留原判，不降级成"可能"。

丢弃的发现不进报告，也不提"曾发现后丢弃"。验证后的报告每轴 400 字以内。

规格缺失时跳过规格轴，并在最终报告里写明。

### 5. 汇总

把两份报告分别放在 `## 规范` 和 `## 规格` 标题下，逐字或略加清理。**不要**合并或重排发现项——两轴故意分开（见"为什么分两轴"）。

末尾一行总结：每轴各几条发现、每轴最坏的问题（如有）是什么。不要跨轴选出一个"最坏"——那正是分开要避免的重排。

## 边界情况

| 情况 | 处理方式 |
|---|---|
| 固定点认不出，或没给参数也没有 `@{upstream}` | 问用户要固定点，拿到后从步骤 1 继续 |
| diff 为空 | 停下，告诉用户固定点与 HEAD 之间无改动，不跑两轴 |
| 参数是 PR 引用，但当前分支不是该 PR 的 head | 提示先检出 PR 分支（如 `gh pr checkout <N>`）再重跑；用户坚持就地审，改按该 PR 的 merge commit 对比并说明局限 |
| PR/issue 拉取失败 | 报出失败原因，问用户：贴规格原文，还是跳过拉取、从步骤 2 第 2 条继续找 |
| `docs/agents/issue-tracker.md` 不存在 | 跑 `/setup-ouyangjiahong-skills`；用户不想配，跳过所有 tracker 拉取，从步骤 2 第 3 条继续找 |
| 规格缺失 | 规格轴跳过，最终报告写明"无规格可用" |
| 仓库没有任何规范声明文件 | 规范轴照跑，只用异味基线 |

## 为什么分两轴

一个改动可以过一轴、挂另一轴：

- 代码每条规范都遵守，但实现的是错的东西 → **规范过、规格挂。**
- 代码恰好做了 issue 要的事，但破了项目约定 → **规格过、规范挂。**

分开报告，防止一轴遮住另一轴。