第 24 课：Code Review Agent

设计能解析 diff、分类风险、指出测试缺口并按 severity 输出 findings 的代码审查 agent。

结论

Code Review Agent 的核心不是“评价代码好不好”，而是把 diff 转换成可行动的 findings。一个专业 review 输出至少要包含：位置、严重程度、风险类型、为什么会出问题、需要作者怎么改，以及是否缺少测试证据。

Pi 源码快照 61babc2 提供了工具系统、extension tool、tool_call 审批事件、active tools 管理和 session 记录。课程里的 Code Review Agent 可以先做成一个外层工具：输入 .diff，输出按 severity 排序的 findings；等稳定后，再把它接入 PR 工作流，成为 push 或 PR 创建前的 gate。

入口问题

一个普通 agent 可以读 diff 后说“这段看起来有风险”。但 reviewer 需要的是更严格的结构：

哪一行或哪个 hunk 有问题？
是正确性、安全、架构、可维护性、性能，还是测试缺口？
严重程度为什么是 blocking、important、minor？
这个 finding 是否基于 diff 事实，而不是凭空猜测？

本课从 /Users/ryanchen/codespace/pi-agent-course-lab/src/14-code-review-agent.ts 的轻量规则开始，逐步扩展成真正可用的 review pipeline。

心智模型

Code Review Agent 可以拆成五层：

diff parsing 层：把 unified diff 拆成 file、hunk、added line、removed line、line number。
risk detector 层：用规则和模型共同识别风险，例如 secret、权限扩大、竞态、错误处理缺失。
test gap 层：检查 diff 是否改了行为但没有相应测试或验证证据。
architecture 层：判断变更是否穿透边界、复制逻辑、破坏模块职责。
severity formatter 层：把所有 findings 按严重程度和可行动性排序。

不要让模型直接从原始 diff 输出长段评论。先把输入结构化，再让模型在结构化上下文上做判断。

教学步骤

1. 先定义 finding 格式

课程 starter 已经给出最小结构：

type Finding = {
  severity: "critical" | "important" | "minor";
  line: number;
  message: string;
};

真实 review agent 还应该加上 file、category、evidence 和 suggestion：

type ReviewFinding = {
  severity: "blocking" | "important" | "minor";
  category: "correctness" | "security" | "architecture" | "test-gap" | "maintainability";
  file: string;
  line: number;
  evidence: string;
  message: string;
  suggestion: string;
};

这一步的教学重点是：review 输出必须能落到 diff 位置，否则作者无法修。

2. 做最小 diff parsing

先不要写完整 Git parser。课堂可以从三个符号开始：

diff --git a/x b/x：切换当前文件。
@@ -a,b +c,d @@：记录新增侧起始行号。
+...：新增行，进入风险检测；但 +++ b/file 不是业务新增行。

解析后的中间结构类似：

type AddedLine = {
  file: string;
  newLine: number;
  text: string;
};

有了 AddedLine，后面所有 detector 都不需要重复理解 diff 格式。

3. 先用规则抓确定性风险

starter 中已有两个规则：

if (/process\.env|API_KEY|TOKEN/.test(line)) critical;
if (/setTimeout\(.+5000/.test(line)) important;

课堂要解释：规则适合抓“低召回但高确定性”的风险，例如 secret、硬编码长 timeout、危险 shell、权限扩大。不要把所有审查都写成正则；规则负责先把明显风险标出来，模型负责解释上下文和边界。

4. 加测试缺口检查

测试缺口不是“没有测试文件就报错”。更合理的判断是：

diff 修改了 src/、packages/、api/ 等行为文件。
同一个 diff 没有修改 test/、__tests__/、.spec.ts、.test.ts。
PR evidence 里也没有相关测试命令。

因此 test gap detector 需要同时看 diff 和第 23 课产生的测试证据。

5. 加架构和安全分类

架构问题通常不是单行 bug，而是边界被打破。例如 UI 层直接访问数据库、工具层绕过权限 gate、extension 直接读写不该碰的路径。

安全问题要优先级更高：secret 泄漏、命令注入、路径穿越、权限绕过、把外部工具写操作暴露成无需确认的 tool。Pi 的 permission-gate.ts 示例就是一个安全审查锚点：危险 bash 不应该只靠模型自觉。

6. 输出按 severity 排序

排序规则建议固定：

blocking：可能导致数据泄漏、破坏用户数据、构建不可用、明显 correctness bug。
important：可能引入 flaky、维护成本明显上升、测试覆盖不足。
minor：命名、重复、局部可读性、非阻塞建议。

最终输出要少而硬。review agent 不是越啰嗦越好，每条 finding 都应该能让作者改代码。

实践例子

本课实践入口：

/Users/ryanchen/codespace/pi-agent-course-lab/src/14-code-review-agent.ts

先运行 starter，观察它如何把 diff 字符串映射成 findings：

const diff = [
  "+ const token = process.env.API_KEY;",
  "+ setTimeout(resolve, 5000);",
].join("\n");

const findings = reviewDiff(diff);

变量流是：

diff 是原始事实。
split("\n") 得到候选行。
每个规则只看新增行。
命中的规则 append 一个 finding。
最终 findings 按 severity 输出给人读。

下一步把 line 从“diff 文件中的行号”升级成“目标文件的新行号”，这样 reviewer 可以直接定位。

课堂代码推演

课堂上先写一个极小 diff parser：

function parseAddedLines(diff: string): AddedLine[] {
  let file = "unknown";
  let newLine = 0;
  const added: AddedLine[] = [];

  for (const line of diff.split("\n")) {
    if (line.startsWith("diff --git")) file = line.split(" b/")[1] ?? file;
    if (line.startsWith("@@")) newLine = Number(line.match(/\+(\d+)/)?.[1] ?? 0);
    else if (line.startsWith("+") && !line.startsWith("+++")) {
      added.push({ file, newLine, text: line.slice(1) });
      newLine += 1;
    } else if (!line.startsWith("-")) {
      newLine += 1;
    }
  }

  return added;
}

然后把 detector 写成纯函数：

function detectSecurity(line: AddedLine): ReviewFinding[] {
  if (/API_KEY|TOKEN|process\.env/.test(line.text)) {
    return [{
      severity: "blocking",
      category: "security",
      file: line.file,
      line: line.newLine,
      evidence: line.text,
      message: "diff 中新增了 secret 或凭据相关逻辑，需要确认不会泄漏到日志、客户端或 PR 描述。",
      suggestion: "把凭据读取限制在服务端边界，并补充不会打印 secret 的测试或审查说明。",
    }];
  }
  return [];
}

这段推演要让学生看到两个控制点：

第一，parser 只负责把 diff 变成事实，不负责判断好坏。

第二，detector 返回结构化 finding，不直接输出自然语言长文。这样后续可以排序、过滤、转成 GitHub review comment，也可以进入第 23 课的 PR checklist。

源码锚点

/Users/ryanchen/codespace/pi-agent-course-lab/src/14-code-review-agent.ts：本课 lab starter，包含最小 reviewDiff() 和 severity 示例。
/Users/ryanchen/codespace/external-sources/pi/packages/coding-agent/src/core/extensions/types.ts at 61babc2：定义 ToolDefinition、registerTool()、tool_call、tool_result 等 review agent 可挂接的 extension API。
/Users/ryanchen/codespace/external-sources/pi/packages/coding-agent/src/core/extensions/wrapper.ts at 61babc2：把 extension 注册的 tool 包装成 agent-core 可执行工具。
/Users/ryanchen/codespace/external-sources/pi/packages/coding-agent/src/core/agent-session.ts at 61babc2：getAllTools()、setActiveToolsByName() 和 _refreshToolRegistry() 展示 review mode 如何只暴露只读工具或 review 工具。
/Users/ryanchen/codespace/external-sources/pi/packages/coding-agent/examples/extensions/permission-gate.ts at 61babc2：用 tool_call 事件阻断危险 bash，是安全类 finding 的源码参考。
/Users/ryanchen/codespace/external-sources/pi/packages/coding-agent/examples/extensions/dynamic-tools.ts at 61babc2：展示 runtime 动态注册工具，可用于把 review_diff 做成只在 review mode 启用的工具。
/Users/ryanchen/codespace/external-sources/pi/packages/coding-agent/src/core/session-manager.ts at 61babc2：appendCustomEntry() 可用于把 review findings 归档进 session 历史。

掌握标准

学完本课后，你应该能做到：

把 unified diff 拆成 file、hunk、新增行和新行号。
设计包含 severity、category、file、line、message、suggestion 的 finding 格式。
区分 correctness、安全、架构、测试缺口和可维护性问题。
说明为什么 secret、权限绕过、危险命令默认属于 blocking。
能基于 /Users/ryanchen/codespace/pi-agent-course-lab/src/14-code-review-agent.ts 扩展出至少三类 review 规则。

课后习题

实践题：扩展 /Users/ryanchen/codespace/pi-agent-course-lab/src/14-code-review-agent.ts，解析 diff --git 和 @@ hunk，把 finding 的 line 改成目标文件的新行号，并新增一个 test-gap finding。
思考题：哪些 review finding 应该阻止 PR 创建，哪些只应该作为建议？请用 security、correctness、architecture、test-gap 四类各举一个例子。