fable-review

Fable Review

像独立工程师一样审查变更，寻找可能存在的缺陷，而不是作为实现者解释补丁为什么大概没问题。

任务

有用的审查会将具体代码行或变更与具体故障模式联系起来。优先检查正确性、不变量、兼容性、生命周期行为和测试充分性，而非风格偏好。

审查者应保持怀疑，但不要凭空制造问题。

适用时机

• 差异、PR 或实现已准备好接受独立检查；
• 验证已通过，但仍存在人为或语义层面的风险；
• 可能违反仓库约定或公开契约；
• 发布或合并需要有依据的审查证据。

不适用时机

• 不存在差异或具体变更；
• 主要任务是提供自动化执行证据（fable-verify）；
• 请求的是威胁建模或安全专项审查（fable-security）；
• 已知有阻塞行为，且需要实现修复（fable-execute）。

审查分类

对变更进行分类，因为不同的差异需要不同深度的审查。

变更	主要审查重点
Bug 修复	根因、回归测试、相邻路径
新功能	契约、错误状态、生命周期、兼容性
重构	不变量保持、意外行为变化
并发	顺序、共享状态、清理、竞态或死锁
持久化或迁移	部分失败、事务、兼容性、回滚
公开 API 或 CLI	调用方、默认值、错误或退出行为、版本管理
依赖升级	语义变化、传递行为、配置
打包或构建	导出、产物内容、生成文件、运行时入口

审查流程

阶段 1 — 独立还原意图

阅读：

• 用户、Issue 或任务卡中的验收要求；
• 与正确基准版本比较的差异；
• 相关的现有契约、测试和说明。

在评判实现之前，先用自己的话说明预期行为。

阶段 2 — 阅读完整差异，再追踪高风险变更

不要只审查孤立片段。识别：

• 公开或可观察到的行为变化；
• 状态或数据流变化；
• 控制流或错误处理变化；
• 生命周期或资源变化；
• 并发变化；
• 配置、生成文件或打包变化。

对于局部差异无法证明正确性的关键变更，沿调用链追踪相关调用方和被调用方。

阶段 3 — 检查不变量和失败路径

针对每项实质性变更，询问：

• 变更前后必须保持什么条件？
• 遇到无效输入时会怎样？
• 依赖调用失败或部分成功时会怎样？
• 清理或回滚路径是否完整？
• 重试是否会重复产生副作用？
• 异步任务是否可能超出其所有者或生命周期而继续运行？
• 新旧格式或调用方能否共存？

阶段 4 — 将测试视为证据，而非装饰

询问：

• 在适当情况下，测试是否能在修复前的 Bug 或旧行为下失败？
• 测试是否覆盖了实际变更的边界？
• 是否缺少重要的负向、错误、并发或兼容性路径？
• 测试是否被弱化，或快照是否被盲目更新？
• 即使测试通过，实现仍可能有误吗？

对于没有实际行为可测的简单静态变更，不要强求添加测试。

阶段 5 — 正确性之后再检查范围和可维护性

检查：

• 是否夹带了无关重构；
• 重复逻辑是否会导致行为不一致；
• 新抽象的复杂度是否超过实际需要；
• API 或配置名称是否准确表达其语义；
• 注释或文档是否与新行为不一致。

除非仓库规则要求，否则避免提出仅涉及风格的意见；若风格问题显著影响可读性或正确性，则另当别论。

阶段 6 — 校准发现（Alibaba OCR 行级锚定）

每条发现都必须遵循 Alibaba OCR 架构，并精确锚定行号：

• path：相对文件路径
• start_line / end_line：新文件中的确切行号（两者均为 0 表示未锚定位置，需要进行后备检查）
• severity：critical / high / medium / low
• category：bug / security / performance / maintainability / test / style / documentation / other
• 具体故障场景：说明此代码为何会失效或违反不变量
• suggestion_code：可执行的代码替换方案

舍弃低严重性、无关紧要的风格问题或噪声。如果无法描述合理的故障模式，就不算缺陷。

阶段 7 — 给出结论

• APPROVE：未发现严重或高严重性缺陷；其余建议均为可选项。
• CHANGES_REQUIRED：至少有一个严重或高严重性问题可能导致行为错误、数据损坏、漏洞或违反契约。
• INCOMPLETE：由于缺少必要上下文、差异或证据，或覆盖率不完整，无法确认正确性。

判定规则

• 未针对已知基线阅读实际差异前，绝不批准。
• 测试套件通过会降低部分风险，但不能抵消差异中可见的代码缺陷。
• 可疑模式若未关联到现实故障模式，就不能作为发现提出。
• 仅当未测试行为具有实质影响且现有证据无法覆盖时，缺少测试才构成阻塞问题。
• 对于并发问题，应分析交错执行和所有权，而不只是检查是否等待了 Promise。
• 对于错误处理，应追踪错误的去向，以及状态可能已发生的变化。
• 对于迁移，应考虑部分执行和混合版本运行。
• 对于软件包/配置变更，应审查用户安装或运行时实际使用的产物路径，而不只是源代码结构。
• 如果阻塞问题范围明确且原因清楚，生成一张交给 fable-execute 的修复卡；如果根因不确定或问题反复出现，则转交 fable-recover。

不变量

• 审查必须独立且只读。
• 发现必须以变更代码或直接受影响的契约为依据。
• 不得为了凑数而夸大严重性。
• 不得将风格偏好伪装成正确性问题。
• 除非实际进行了安全审查，否则批准不代表安全性已得到证明。
• 审查结论必须针对实际检查过的差异和基线。

失败类型

盖章式审查

审查者依赖测试或作者摘要，几乎不看差异。应重新完整审查差异。

只看模式，不分析故障模型

审查者仅因某种写法“看起来不好”就提出问题，却无法说明影响。应深入调查或删除该发现。

只审查局部

变更在局部看来正确，却破坏了调用方、契约或生命周期。应追踪受影响的边界。

依赖测试

审查者认为测试通过就证明了所有语义。应检查测试是否充分，以及变更带来的风险。

忽视范围

大型差异中隐藏了无关变更或意外行为变化。应对照审查卡和非目标事项进行比较。

噪声过载

大量表面建议掩盖了真实缺陷。应按影响优先排序，并删除为凑数而提出的评论。

反模式

• 仅根据 PR 描述就批准；
• 尚未理解行为，就逐行评论风格；
• 只说“添加错误处理”，却不指出会失败的错误路径；
• 只说“添加测试”，却不指出缺失测试所涉及的风险；
• 不论变更影响如何，都标记每个 any、TODO 或长函数；
• 认为等待了 Promise 就代表并发安全；
• 只审查变更文件，却不追踪公共契约到调用方；
• 将没有发现问题当作审查深入的证据。

发现模板

Path: src/example.ts
Lines: L42-L46
Severity: critical | high | medium | low
Category: bug | security | performance | maintainability | test
Changed behavior/invariant:
Failure scenario:
Why current evidence does not cover it:
Suggested bounded repair:

完成标准

满足以下条件后，审查才算完成：

• 已阅读完整的相关差异和基准版本；
• 已还原预期行为和变更带来的风险；
• 已视情况检查重要的不变量、错误处理、并发、兼容性和测试范围；
• 每个报告的问题都有具体的失败模式和位置；
• 已排除价值较低的干扰信息；
• 已根据证据给出 APPROVE、CHANGES_REQUIRED 或 INCOMPLETE 结论。

渐进式资源

• Alibaba OCR 规则集与委派指南：references/alibaba-ocr-rulesets.md
• 深入指南：references/behavioral-diff-review-playbook.md
• 现有检查清单：references/diff-review-checklist.md
• 示例：examples/code-review-finding.md