review git:20260817.d4dfa41A

Immutable. This exact content is served forever at /api/v1/blob/598bbb9f7a2a0308.

---
name: review
description: 审查一批改动时用。缺陷优先：找静默的错答案、反了的失败方向、没测试守着的关键分支，按严重程度报告并说清审了多大范围。风格问题交给 lint，不在这里人肉报。
---

# 审查改动

## 先定范围

```bash
git diff                        # 未提交的
git log --oneline main..HEAD    # 这条分支有哪些提交
git diff --stat main...HEAD     # 相对基线的全部改动
```

基线分支不一定叫 main，先看 `git remote show origin` 里的 HEAD 指向。
用户说「审一下」时通常指整条分支，不只是未提交的那点。

## diff 只是入口，不是全貌

`[约束]` 逐个改动文件**读完整文件**，至少读到改动点所在的函数和它的
调用方。diff 里看起来对的行，放进调用方的语境里经常是错的——参数顺序、
单位、调用约定这类错误在 diff 视图里不可见。

范围太大读不过来时，说明白哪些只扫了 diff，不要假装全读了。

## 该找什么（按回报排序）

**1. 静默的错答案。** 不报错不崩、只是结果错的那类：错误被吞掉后返回
空集合、部分结果被当成全量、截断或超时提前收工没说出来、缓存该失效没
失效。判据：这条路上有没有可能「半份东西被当成全部」。

**2. 失败方向反了。** 解析失败、超时、认不出的输入，落在哪一侧？权限
判不准变成放行、校验失败当成通过、catch 里吞掉异常继续走——这类改动
单看每一行都合理，方向反了就是安全洞。

**3. 调用方没跟上。** 改了函数的签名、语义或返回值，所有调用点都重审
过吗？grep 一遍比信 diff 可靠：编译器只抓签名，抓不住语义（原来返回
空表示「没有」，现在表示「出错」——类型没变，含义全变）。

**4. 有实现没测试守着。** 对每个关键分支问：把这行删掉或改反，会有
测试红吗？答不上来就是发现。尤其是错误处理分支——它们几乎从不被手工
测到。

**5. 边界。** 空集合、0、负数、超长输入、重复调用、并发。别全列一遍，
只报这次改动**真的会碰到**的那几个。

**6. 不该入库的东西。** 密钥、调试输出、被注释掉的大段旧代码、意外
带上的生成物或临时文件。

## 怎么报

按**严重程度**排，不按文件顺序。每条给：

- 位置（`起始行:结束行:路径` 的引用格式）；
- **会怎么表现**——具体症状，不是「可能有风险」。说不出症状的发现
  多半不是发现；
- 建议的改法，或「需要一个测试守着这里」。

最后说清「没发现问题」覆盖了多大范围——审了 3 个文件和审了 30 个
文件，同一句「看起来没问题」含义完全不同。

## 不要做的事

- 不要顺手改。审查的产出是判断，改不改、怎么改由用户决定。
- 不要报 lint 管得了的事（命名、格式、能不能少写两行）。人肉报这些
  既浪费篇幅又稀释真发现。
- 不要为了显得认真而凑数。三条真问题比十条「建议考虑」有用。