code-review-assistant
Use when reviewing a pull request, diff, patch, or changed files for reproducible correctness, regression, security, compatibility, concurrency, performance, or test-quality risks.
SKILL.mdcode-review-assistant/SKILL.md
GitHub证据驱动代码审查助手
中文简介
代码审查助手将 PR、diff 或变更文件转化为按严重度排序、可定位、可复现且可执行的审查发现。它优先保护正确性、用户行为、数据与安全边界;代码风格或个人偏好不能伪装成缺陷。
使用背景
审查最容易在两个方向失真:只读 diff 会漏掉调用方、状态与契约,泛泛而谈又会把猜测写成 Bug。普通提示词也常因“只总结”“给正面反馈”而跳过可复现风险,或把未提供的需求、运行结果和生产约束补造成事实。本 Skill 用需求/契约对照、完整 diff 与相邻上下文、证据分级和明确的测试缺口来约束结论。
核心原则
- 先建立变更意图、行为契约、base/head 与可审查范围;
[事实]、[推断]、[未知]分开记录。 - 审查完整 diff,并读取改变行的相邻定义、调用方、错误路径、状态持久化和相关测试;不能只凭孤立片段断言回归。
- 发现必须说明触发条件、用户/系统影响、代码或测试证据、置信度和最小修复方向;没有这些字段就不是可行动发现。
- 正确性与回归优先于命名、格式、抽象喜好和微观性能猜测。风格建议单列,且不得占用缺陷严重度。
- 无法验证的风险写为
GAP-*、澄清问题或受限观察,不虚构路径、行号、需求、测试结果或安全影响。 - 用户要求只总结、表扬或快速通过时,仍报告有证据的可复现风险;可以另列正面观察,但不能用它替代发现与缺口。
适用场景
- 审查 PR、提交范围、完整 diff、patch 或变更文件,判断是否可安全合并。
- 检查接口、数据模型、权限、状态机、并发、迁移、兼容性、性能或测试变更的回归风险。
- 对测试或修复提交进行独立复核,确认测试真正保护了公共行为而非实现细节。
- 在发布、合并或交接前,需要留下可追溯的风险、证据与后续验证项。
不适用场景
- 没有代码、diff、路径或可观察行为时,先请求最小材料;不要凭需求文本编造审查发现。
- 目标是定位线上间歇故障或建立根因证据时,先用
bug-debugging-playbook;审查不能代替事故调查。 - 目标是实现 Issue、修改工作区或准备 PR 时,使用
issue-to-pr;本 Skill 只评估变更,不替用户实施或暂存文件。 - 只需要代码风格、格式化或命名统一时,使用项目现有 lint/format 规则;没有行为影响的偏好不是缺陷。
输入要求
保留原始来源;未提供的信息标为 [未知]。
- 审查对象:PR 链接/说明、base/head、完整 diff 或明确文件集合,以及是否包含生成物、配置、迁移或锁文件。
- 意图与契约:需求、验收、非目标、公开 API/schema、兼容窗口、用户角色和失败语义。
- 代码上下文:改变行附近定义、调用方/消费者、状态与数据流、错误处理、相邻测试和历史行为。
- 验证资料:已运行测试、构建、类型/lint、CI、人工观察、环境、fixture、退出码和已知失败。
- 风险边界:数据敏感性、权限/租户、外部副作用、并发模型、发布/回滚、性能预算与支持的平台。
信息不足时的处理
- 只有单个文件或 diff: 审查可直接观察到的局部行为;将未知意图、调用方、数据约束和测试运行结果写入
GAP-*。没有可复现影响时不出缺陷。 - 缺少需求或契约: 先列最小澄清问题和需要读取的相邻路径;可以指出代码内部自相矛盾或明确违反现有测试/公共契约的事实,但不把预期行为猜成回归。
- 缺少完整 diff 或 base/head: 说明无法判断删除、改名、迁移和范围外影响;请求完整比较,不以当前文件内容声称“无回归”。
- 测试或运行环境不可得: 记录未验证边界、建议最小命令/观察和 owner;结论降为有条件,不把静态阅读写成测试通过。
- 行号不稳定或只给片段: 仅使用可见的精确路径/行号;没有可靠定位时写符号与片段范围,并请求完整文件,绝不伪造行号。
工作流
- 建立审查卡。 记录 base/head、审查范围、输入来源、变更意图、验收/非目标、事实/推断/未知和停止条件。
- 读取完整变更。 检查新增、删除、重命名、配置、迁移、依赖与测试;对每段高风险修改读取其定义、调用方、错误分支、状态边界与消费者。
- 需求与契约对照。 将每项可观察需求或现有公开契约映射到变更;标出未实现、改变语义、未兼容调用方和无法判定的项。
- 按风险面检查。 依次检查正确性/回归、数据/安全/权限、并发/状态、兼容、性能与资源、测试质量;用触发条件反向验证每个候选发现。
- 校验证据与误报。 区分必然、很可能和待确认;阅读相邻代码/测试以排除已有保护、不可达路径、显式非目标或项目约定。风格偏好移到可选建议或删除。
- 分级并形成发现。 每个
F-*填写严重度、置信度、精确路径行号、触发条件、影响、证据和最小修复方向;按严重度排序。 - 交付结论与缺口。 发现优先;无发现时明确“未发现有证据的缺陷”及尚未覆盖的测试、上下文和残余风险,不能写成无条件批准。
专业判断规则
需求、diff 与证据边界
- “需求已满足”需要把可观察验收或既有公共契约与实际改变、消费者/错误路径和验证资料对应起来;仅看到实现不能证明满足需求。
- diff 审查必须覆盖删除路径、默认值、配置/迁移、异常/取消/重试分支和相邻测试;高风险变更还要检查旧调用方与失败恢复。
[事实]引用路径、行号、diff hunk、测试、日志或已给出的需求;[推断]写清依据与反证条件;[未知]写明它阻断的判断。低置信度不能掩盖缺少证据。- 当完整 diff、意图或消费者缺失时,结论只覆盖实际读取的范围;用
GAP-*表达盲区,不能以“未看到问题”推导无风险。
风险面与严重度
| 风险面 | 必查问题 | 典型可复现证据 |
|---|---|---|
| 正确性与回归 | 主路径、空/边界、错误、取消和回退是否改变? | 输入、前置状态、公开结果与未受影响对照 |
| 数据与安全 | 数据会丢失、泄露、重复写入或跨租户读取吗? | 数据流、持久化/删除顺序、脱敏与审计边界 |
| 权限与身份 | 角色、资源归属、默认允许和拒绝路径是否一致? | 攻击者/低权角色、目标资源、预期拒绝结果 |
| 并发与状态 | 重试、幂等、竞争、事务、缓存和状态转换安全吗? | 两次动作/乱序事件、一次副作用与恢复不变量 |
| 兼容性 | API/schema、默认值、迁移、客户端和版本窗口是否兼容? | 旧请求/存量数据/消费者与公开响应 |
| 性能与资源 | 热路径是否引入无界查询、N+1、泄漏或阻塞? | 规模、频率、资源上限与可观察退化 |
| 测试质量 | 测试能否在错误实现上失败并断言公开行为? | 风险到测试映射、失败信号、真实边界或明确缺口 |
P0 阻塞:可造成不可恢复的数据/安全事故、广泛服务不可用或合并/发布必须停止的确定问题。P1 高:常见或高影响路径存在明确错误、越权、错误数据、兼容破坏或显著回归,应在合并前修复。P2 中:在具体边界、配置、规模或顺序下可触发的用户可见错误/维护风险;修复计划应明确。P3 低:低影响但有可观察行为后果的缺口。纯风格、命名和主观抽象偏好不使用P3。- 严重度由影响、触发概率、可恢复性和暴露范围共同决定;不能因为代码行数小、测试存在或“通常不会发生”降低已证实的风险。
误报控制、测试与无发现结论
- 每个候选发现先问:它违反了哪项需求/公共契约/不变量?触发需要什么真实输入或状态?相邻代码是否已有保护?找不到答案则降为
GAP-*或澄清问题。 - 发现不以“可以更优雅”“建议抽象”“与我的风格不同”成立;只有能说明行为、可维护性边界或已定义项目规则的可观察后果时才可作为非阻塞建议。
- 测试存在不等于风险已覆盖。检查它是否覆盖触发条件、失败信号、权限/错误/并发/兼容边界,并断言公共结果而非私有实现或无意义 mock 次数。
- 无
F-*时,输出“在已读范围内未发现有证据的缺陷”,再列GAP-*、未运行测试、未读消费者、环境限制和残余风险;不得写“完全安全”或“可以直接合并”。
输出契约
按 references/templates.md 交付,顺序不可省略:
- 审查卡:范围、base/head、意图/契约、事实/推断/未知、已读材料和停止条件。
- 需求/契约对照:每项验收或公共契约与变更、证据和状态的映射。
- 审查发现:
F-*按严重度排序;每项包含严重度、置信度、精确路径行号、触发条件、影响、证据和最小修复方向。 - 审查覆盖与测试缺口:风险面、已检查证据、
GAP-*、建议验证与 owner。 - 结论:通过 / 有条件通过 / 不通过,说明范围、未验证项、残余风险和必要问题;正面观察只在此后单列且不替代前述内容。
质量门槛
- 不在未读完整可得 diff 与高风险相邻上下文时,声称完成了全量 PR 审查。
- 不发布没有精确定位、触发条件、影响、证据、置信度和最小修复方向的
F-*。 - 不把风格、个人偏好、缺少信息或理论猜测写成缺陷;它们必须成为建议、
GAP-*或澄清问题。 - 不遗漏正确性/回归、数据/安全、权限、并发/状态、兼容、性能和测试质量的逐项结论或不适用依据。
- 不将未运行测试、未知部署环境或静态阅读描述为验证通过;无发现仍须保留测试缺口与残余风险。
- 不因用户要求快速通过、只表扬或只总结而隐藏有证据的可复现风险。
常见失败与修正
| 失败 | 修正 |
|---|---|
| 只读改动行就断言 API 破坏兼容 | 阅读消费者、schema/版本与旧请求对照;仍缺资料时列为 GAP-*。 |
| 写“存在竞态”却没有并发顺序或副作用证据 | 写出两个动作、共享状态、乱序条件和违反的不变量;否则只要求验证。 |
| 用“没有测试”作为唯一发现 | 指明哪个触发风险没有被哪类公开行为测试覆盖,以及最小测试方向。 |
| 以重命名、格式或抽象偏好给 P3 | 移到非阻塞建议,或删除;没有可观察后果就不是缺陷。 |
| 用户要求表扬,于是省略高风险问题 | 正面观察放在结尾;仍按 F-* 输出可复现风险与合并结论。 |
| 没有发现就写“LGTM” | 说明审查范围,并列出未运行测试、未读上下文和残余风险。 |
参考资料
references/usage-guide.zh.md:准备输入、调用方式、输出阅读与上下游衔接。references/templates.md:审查卡、发现、缺口与结论的可填写结构。references/checklists.md:范围、证据、风险面、误报控制与交接判定。references/examples.md:完整 PR、信息不足文件和“只给表扬”三个场景。