/code-review

name: code-review
description: Review a diff against its acceptance criteria in four segments (convention adherence, bug scan, historical-context regressions, test-coverage gaps) as a NON-implementing agent. Findings are sorted by severity, each with file:line + a failure scenario. A planted defect must be caught; a clean diff must not be over-reported.
license: MIT
scope: coding
compatibility:
  - claude-code
allowed-tools:
  - Bash
metadata:
  category: review

code-review

对一个 **diff + 验收标准**做**四段评审**，产出按严重度排序的 findings（每条带 `file:line` + 失败场景），把 `/review` 的纪律移植到任意 coding agent（`apc/05` §2 S5 · `apc/12` code-review 行）。

**什么时候用**：一个 coding task 改完、进 review 门时，需要由**非实施者**独立审这份 diff。

评审独立性（第一铁律，P2-17）

**评审必须由非实施 agent 执行**——实施者不自评自的 diff，与 no-self-approval 同精神（`apc/05` §2 评审独立性）。服务端也有牙：`code-review` 工件若 `reviewerId === task.assigneeId` 直接 403（`task-review-artifact.service.ts:126` `forbidden_implementer_self_review`）。

开审**第一步**先自证独立：

git log --format='%an <%ae> %H' <base>..<head>    # 被审提交的作者
# 断言：作者 ∉ {你自己}。若你就是作者 → 停手，报告"不能自评"，让另一个 agent 审。

四段评审（缺一段不算完）

移植自 `/review` 的分段纪律——每段独立扫一个维度，产出 findings 再合并按严重度排序：

| 段 | 扫什么 | 怎么做 | 典型 finding | | --- | --- | --- | --- | | **1. 规约符合** | diff 是否违反 CLAUDE.md / 既有约定 | 读根 + 相关目录的 CLAUDE.md，逐条对 diff | 用了被禁的 pattern、破坏 layer rules | | **2. bug 浅扫** | 只看 diff 本身的明显 bug | 读 `git diff`，聚焦大 bug（空/边界/错误分支/资源泄漏/并发），**不纠 lint/类型/格式**（CI 会管） | off-by-one、未处理的 null、错误吞异常 | | **3. 历史上下文** | 结合被改代码的历史看回归 | `git blame` / `git log -p` 被改的行，看这段代码为何这么写、这次改会不会破坏原意图 | 改动撤销了一个之前修 bug 的提交、重新引入已知回归 | | **4. 测试覆盖缺口** | 改动引入的行为有没有测试护 | `rg` 找对应测试文件、看新增/改动路径是否被断言 | 新分支零测试、负控缺失、断言放松 |

**只标真问题，不纠鸡毛**（`/review` 纪律）：预存 bug、非本次改动行、CI 会抓的（lint/类型/格式/import）、senior 不会提的吹毛求疵——都不进 findings。这是**避免虚报**的纪律，直接对应负控"干净 diff 不虚报"。

Procedure

1. 独立性自证（见上）— 不过则停手

2. 取 diff + 验收标准

git diff <base>..<head> > /tmp/review.diff        # 或对 working tree: git diff
cloud task acceptance "$PRISMER_TASK_ID"           # 读该 task 的验收标准（可选，若挂 task）

3. 四段逐段扫，每段产 findings

每条 finding 的形状（缺一项不算数）：

{ severity: blocker|major|minor,
  segment: convention|bug|history|coverage,
  file: "src/foo.ts", line: 42,
  finding: "<一句话说清是什么问题>",
  failureScenario: "<在什么输入/时序下它会真的坏，可复现的因果>" }

**`failureScenario` 是承重字段**：说不清"怎么坏"的 = 还没验证的猜测，降级成 openQuestion，不当 finding 报。

4. 合并 + 按严重度排序

blocker → major → minor。产出：

• `findings[]`（排序后）
• `changeSummary`（这份 diff 干了什么，一段话）
• `openQuestions[]`（拿不准的，不是 finding）
• `residualTestGaps[]`（第 4 段发现的、本次不补但要记的覆盖缺口）

5. 上报 criterion（挂 task 时）

四段评审产物可作为结构化 review artifact 上报（服务端 schema `code-review`：要求 `findings[] / openQuestions[] / changeSummary / residualTestGaps[]`）：

# 写 artifact JSON（形状见上）到文件
cat > /tmp/review-artifact.json <<'JSON'
{ "findings": [...], "openQuestions": [...], "changeSummary": "...", "residualTestGaps": [...] }
JSON

# 有 blocker → failed；无 blocker → passed
cloud task verify-criterion "$PRISMER_TASK_ID" "<criterion-id>" \
  --outcome failed --review-artifact /tmp/review-artifact.json \
  --note "1 blocker: src/foo.ts:42 unhandled null → NPE on empty input"

`--outcome` 合法值恰好四个：`passed | failed | n/a | waived`。**注意**：`verify-criterion` 的 review-artifact 走服务端 `reviewerId` = 调用凭据身份，若你的凭据解析成 assignee，服务端会 403（自评拦截）——这正是独立性的第二道门。

输出契约（机器判据按这个复验，别自由发挥格式）

本 skill 的验收判据不是「报告里出现了 `foo.ts:42` 这种形状」——旧判据 `[\w./-]+\.(ts|tsx|js):\d+` 正是这么写的，**随手编一个路径行号就能拿满分**。现在判据把报告里的每条 `file:line` **读回磁盘复核**（`structured-criteria.ts` 的 `cited-evidence`）。所以报告必须带下面两样**可机器解析**的东西——形状对但内容假的报告会被当场判红。

**① 评审锚点声明**（一行；符号会被复核确实出现在该文件里）：

CHANGE-POINT: <被审的符号/表名/端点> @ <path>

**② findings 表**（markdown 表，**首列是 `file:line`**，按严重度排序）：判据逐行把 `file:line` 读回磁盘——**文件不存在 / 行号越界 / 该行是空行**，任意一条就判红；并且至少 2 条引用**所在的那一行必须真的含锚点符号**（"真实存在的行" ≠ "真实的搜索命中"）。

| file:line | severity | segment | finding | failureScenario |
| --- | --- | --- | --- | --- |
| src/lib/orphan-vacuum.ts:131 | blocker | bug | 空父集下 NOT IN 判 TRUE | im_tasks 清空后 vacuum 删掉合法审计行 |

**行号照抄 `rg -n` 的真实输出，不许凭印象写。** 说不清 `failureScenario` 的降级成 `openQuestion`，别塞进 findings 表凑行数。

**③ 四段 keyed 行**（四段各占**独立一行**，key 逐字如下）：旧判据是「全文里出现过 convention/bug/history/coverage 这四组词」的 lookahead——**一篇编造的报告照样满分**，而且**整段跳过**也照绿（只要正文里恰好提到那个词）。现在判据（`dimension-coverage`）只认**这条 keyed 行**：某段没写这一行 = 该段**缺失判红**，正文里出现那个词**不算数**。每段的答案必须是**这一段真的看过的** `path:line`（读回磁盘复核），或者显式 `N/A — <理由≥20 非空白字符>`：

**每段必须以一个 verdict token 开头**（`covered` = 该段审过且没问题 / `gap` = 该段发现了问题 / `N/A — <理由≥20 字符>` = 该段不适用），然后才是 `path:line`：

- SEGMENT-convention: covered | src/im/sql/528_task_log_nullable_taskid_for_ops_audit.sql:1 | 编号无缺口、双 schema 同步
- SEGMENT-bug: gap | src/lib/orphan-vacuum.ts:131 | 空父集下 NOT IN 判 TRUE，删掉合法审计行
- SEGMENT-history: covered | src/im/sql/528_task_log_nullable_taskid_for_ops_audit.sql:13 | 描述符自 177773e9 起未随列语义更新
- SEGMENT-coverage: N/A — `rg -l` 在本改动面上零测试文件命中，本段没有可审的测试面（缺口本身记进 residualTestGaps）

> **⚠️ `SEGMENT-bug` 在本 sample task 上被钉死为 `gap`**（判据 arg `requiredVerdicts`）。原因是实测出来的：一份**明确写着「未发现 blocker 级问题，建议合入」、还主动复述那条错误安全性断言**的评审，在加这条之前拿 **6/6 满分**——因为旧判据「the real defect surfaced」是一串宽泛关键词（`vacuum` / `删除` / `delete` / `审计行`），而**这个 diff 的主题本身就是 orphan-vacuum 和审计行**，任何谈论它的报告都必然命中。判据的 label 与它实际验的东西之间没有强制关系（apc/12 §0.13）。 > > **诚实边界**：`requiredVerdicts` 把「答案」编进了判据 —— 它只对**这个** sample task 有意义（换一个待审 diff 就必须换 args，甚至该判 `covered`）。这是 design-review 用同一个 arg 时已登记的取舍，不是新问题。

（上面四行是**真引用**、可原样复核——照抄这个形状不会把如实的报告判红；换成你自己那轮的真行号即可。）

四段中任意一段**只能**二选一：给可复核引用，或显式 `N/A —` + 真理由（`N/A` 后面空着/敷衍 <20 字符同样判红）。**不许**把段名写进散文当作"覆盖过了"。

**引用写法三条（判据按这个复核，写错会把如实的报告判红）**：

1. **必须 repo-root-relative**：写 `src/lib/orphan-vacuum.ts:131`，不要写简写 `lib/orphan-vacuum.ts:131`，也不要写绝对路径。
2. **计数不是行号**：`rg -c` 输出的 `path:12` 是"12 个命中"。要引用计数就写成 `count=12`；若整行只贴 `rg -c` 的裸 `path:数字`，判据会认出它是计数并跳过，但**别把它混在句子里**，那样会被当行号复核。
3. **散文里出现的 `path:line` 一样会被复核**——报告里任何一处 `path:line` 都是一次声明。拿不准就只写路径不写行号。

## 产出（副作用 oracle，报告里必须给）

1. **独立性证据**：`git log` 作者 ≠ 评审者。
2. **四段 findings**：按严重度排序，每条 `{severity, segment, file: