Bruno AI 代码评审实战：正确性与根因评审员（Correctness & Root-Cause Reviewer）的完整方法论

本篇围绕 Bruno 仓库内置的 AI 代码评审技能中的「正确性与根因评审员」展开。Bruno 是一个跨平台 Electron 桌面 API 测试工具（轻量级 Postman/Insomnia 替代方案），其 .claude/skills/code-review/ 目录定义了一套与 CI 侧 CodeRabbit 评审对齐的本地多视角评审流程。读完本文，你将掌握：如何构建一个只做「正确性 + 根因」单一视角的评审员、其 blocker 级反模式清单（症状补丁、错误层级修复、单点复现修复、x || default 隐式造值）、Bruno 特有的「双路径（twin path）」核对方法，以及如何把这套检查清单移植到你自己的仓库评审流程中。

一、定位：多视角并行评审架构中的一员

correctness.md 不是一个独立的规则文件，而是 code-review 技能 下 8 个并行评审员（lens）之一。整个技能的编排逻辑是：取一次 diff → 按文件范围分派 → 各评审员并行独立审查 → 按文件合并去重并保留更高严重级别。

SKILL.md 中的评审员清单（L67-L80）如下：

评审员文件	视角	文件范围
reviewers/correctness.md	正确性与根因	全部源码（不含 tests/**）
reviewers/architecture.md	架构与依赖边界	packages/**
reviewers/conventions.md	编码规范与可读性	全部文件
reviewers/react.md	React 应用代码	packages/bruno-app/**
reviewers/cross-platform.md	跨平台（macOS/Windows/Linux）	全部文件
reviewers/security.md	安全与数据安全	全部源码（不含 tests/**）
reviewers/dsl-changes.md	磁盘 DSL 与序列化（向后兼容）	bruno-app、bruno-electron、bruno-cli、bruno-lang、bruno-filestore、bruno-schema(-types)、bruno-converters
reviewers/e2e-tests.md	Playwright E2E 测试	tests/**

correctness 评审员被明确标注为「scope 覆盖所有文件的视角」之一——按 SKILL.md 第 44 行的编排规则，这类"全文件范围"的视角在任何一次 diff 中都不允许被跳过。其审查范围在文件开头一行声明：all changed source（**/*，排除 tests/**），即只审改动过的源码，绝不评审未触碰的代码。

diff 的两种获取模式

评审的前提是"只审改动"。SKILL.md（L20-L40）定义了两种模式：

• 已提交范围（默认）：git diff main...HEAD，基于 base 分支（main 或 release/*）。要求先 git fetch 确认 base 是最新的——过期的 base 会把已合并的无关变更灌进 diff，白白扩大评审范围。
• 工作区/未提交变更：由于评审期间工作区可能变动，逐个评审员重跑 git diff 会导致各自看到不同快照。正确做法是一次性冻结 diff 到临时文件，再让所有评审员读同一份快照：

git add -N . && git diff HEAD > "$SCRATCH/review.diff"

git add -N .（intent-to-add，可用 git reset 回退）让未跟踪的新文件也进入 diff。各评审员随后只在自己的 glob 范围内读这份冻结的 diff，工作区文件本身已包含未提交状态，作为周边上下文读取。

二、共享人设与输出契约

每个评审员都不是自成一体的——correctness.md 第 5-6 行要求评审员先"采用评审员人设，并按 _contract.md 定义的输出契约返回结果"。这份 共享契约文件 是所有 8 个评审员共读的"小文件"，集中定义了人设与输出格式，避免在每个评审员文件里重复维护。

人设（Persona） 要点（_contract.md L5-L11）：

• 扮演企业级团队中精通 TypeScript、JavaScript、Node.js 与 Electron 的专家评审员；
• 简洁：每条发现只说一句话，被追问才展开；
• 按项目标准评审，不因作者资历或假设意图而降低严重级别；
• 每一条发现都必须落在真实代码上：文档、指南、注释与实际代码冲突时，以仓库代码为准；不得引用未验证的行号或编造示例值。

输出契约（Output contract） 是一个扁平列表，每条发现一行：

<blocker|suggestion|nit> | <file>:<line> | <one-sentence finding>

范围干净时只返回 no findings，且严禁为了凑数而编造 nit（"Never invent nits to fill the list"）。三档严重级别分别对应：blocker（必须解决）、suggestion（建议改进）、nit（吹毛求疵级）。这个契约与 CI 侧 .coderabbit.yaml 中配置的评审 tone（"expert code reviewer in TypeScript, JavaScript, NodeJS, and ElectronJS… concise and clear"）刻意保持一致，本地技能与 CI 自动评审因此输出同构的发现列表，可以互相对照。

三、核心哲学：修根因，不修症状

correctness 评审员的第一原则是：验证变更的正确性，并且对每一个 bug fix，验证它解决的是根本问题而非可见症状。"表面补丁——掩盖缺陷却不解决缺陷"在本评审体系中被定级为 blocker，这是该文件给出的唯一最高级判定标准。具体展开为五条检查规则（correctness.md L8-L23）：

1. 先弄清"为什么会出现"，再接受修复

在认可一个修复之前，必须先理解问题存在的原因——把 bug 追溯到其起源，而不是停在它显现的地方。症状出现的位置和根因所在的位置经常相距很远，停在症状处的修复只消灭了这一次报错。

2. 揪出"压制症状"的补丁

文档列出了一个高度可操作的反模式清单，凡是属于"压制症状、保留真因"的写法都应被标记：

• 多余的 null 守卫（extra null guards）——值不该为 null 却在下游兜底；
• try/catch 吞错（swallowing）——错误被静默吸收，真实失败永远浮不上来；
• 防御性重复检查（defensive re-checks）——重复验证本应由上游保证的不变量；
• 重试、超时、数值钳制（retries, timeouts, clamping values）——用时间或范围掩盖数据问题。

配套的追问句式也被明确写入："如果这个补丁在防御一个坏值，那么坏值是从哪来的？是否应该在那里修？" 这正是根因分析与症状缓解的分界线。

3. 错误层级的修复 = 症状补丁

文档给出两个典型例子：

• UI 守卫去兜数据层的 bug——界面层做了校验/隐藏，但数据层的错误生成逻辑原封不动；
• 调用方绕过被调用方的契约违规（caller working around a callee's contract violation）——被调用方违反了它自己的约定，调用方却写 workaround 去适配。

评审员的职责是指出正确的修复层级。

4. 警惕"只为单个复现用例"的修复

当一个修复的作用域恰好覆盖某一个报告的复现场景时，要警惕同一根因在其他调用点同样可能触发——正确的修复通常覆盖所有调用点，而不只是被报告的那一个。这与 x || default 问题（见第五节）常常是同一枚硬币的两面。

5. 看起来像 workaround 的修复必须点名

当修复看起来像一个绕行方案时，评审员要直说，并点名那个真正能解决问题的更深层改动——即使更大的修复超出本 PR 范围，PR 中也应当承认（acknowledge）它的存在，而不是无声地留下绕行。

四、常规正确性检查清单

除了 bug fix 的根因判定，文档还要求对一切变更做常规正确性核查（correctness.md L24-L26），清单包括：

• off-by-one 与边界错误（off-by-one and boundary errors）；
• 未处理的 Promise rejection / 缺失的 await（unhandled promise rejections / missing await）——在大量异步 I/O 的 Electron/Node 代码库中这是高频缺陷；
• 被吞掉的错误（swallowed errors）；
• 不正确的 null/undefined 处理；
• 变更自身引入的新边界情况（edge cases the change introduces）——不只是旧 bug，新代码制造的新坑同样在评审范围内。

这份清单与项目测试规范形成互补：CODING_STANDARDS.md 的 Tests 一节要求"覆盖 happy path 和现实中的问题路径，包括错误处理与边界情况"，并在 E2E 部分要求测试验证用户可见行为而非实现细节——评审员发现的"被吞掉的错误""缺失 await"这类问题，最终要落到能确定性复现失败的行为级测试上才算闭环。

五、Bruno 特色规则一：核对"双路径"（Twin Path）

这是 correctness 评审员中最具 Bruno 项目特色的一条规则（correctness.md L27-L31）：

Check the twin path. Bruno keeps parallel implementations of the same behavior — .bru vs .yml serializers, the default app-data workspace vs custom-filesystem workspaces. A change that touches one path must be verified against its twin; behavior that diverges between them is a bug, not two independent features.

Bruno 把用户的集合、请求、环境、文件夹和配置全部以纯文本文件持久化到磁盘和 Git 仓库中，且存在两组平行的实现：

第一组：.bru 与 .yml 两套磁盘序列化格式。 按 DSL 变更规则（L17-L25）：.bru 走 Bru DSL 的 v2（ohm-js）语法，读写入口在 packages/bruno-lang/v2/src，经 packages/bruno-filestore/src/formats/bru 的 bruToJsonV2 / jsonToBruV2；.yml 是 OpenCollection YAML，由 packages/bruno-filestore/src/formats/yml 处理，且是当前 DEFAULT_COLLECTION_FORMAT（常量定义于 packages/bruno-filestore/src/constants.ts，渲染进程侧在 packages/bruno-app/src/utils/common/constants.js 有一份副本）。新集合默认就是 .yml，除非用户另选。

DSL 规则文件 进一步补充了双路径核对的关键细节：Bruno 没有专门的 bru↔yml 转换器——一个请求在两种格式间迁移时，是先在源格式解析成共享内存对象、再用目标格式重新序列化（SaveTransientRequest 会把 sourceFormat + targetFormat 传给 renderer:save-transient-request）。因此一个只在单侧格式处理的字段，在跨格式复制/保存的瞬间会被静默丢弃。规则原文（L103-L104）对"未设置态"（unset case）的分歧做了精确描述：

a field that defaults or persists one way in .bru and another in .yml (e.g. one path returns '', the other '1') is a divergence bug.

第二组：默认 app-data 工作区与自定义文件系统工作区。 同一个值在一种工作区中持久化、在另一种中被静默丢弃，属于与"格式分歧"同类的 bug（见 DSL 规则 L105-L106 与 correctness.md L28 的呼应）。

这条规则给出的判定标准非常干脆：两条路径行为分歧就是 bug，而不是两个独立功能。它要求评审任何触碰单侧路径的变更时，必须主动去验证另一侧。

六、Bruno 特色规则二：警惕 x || default 在有意义的"未设置"字段上

文档最后一条规则（correctness.md L32-L36）针对一个极易被忽视的语义缺陷：

x || default on a field whose absence is meaningful. When "not set" / "never configured" is a distinct state, falsy-coalescing (version || '1') fabricates a value for the unset case, erases the distinction, and often diverges from a sibling path that handles it correctly. Flag it; use ?? or an explicit undefined check when the unset state must survive.

要点拆解：

• 当一个字段的"未设置/从未配置"本身是一个有区别的状态时，falsy 合并运算符 || 会为未设置的用例凭空造出一个值，抹掉了"未设置"与"设置为 falsy 值"之间的区分；
• 这类写法常常还与正确处理了该状态的**兄弟路径（sibling path）**产生行为分歧——正好落在上一条"twin path"规则的火线上；
• 修复方向：需要保留未设置状态时，改用 ??（nullish coalescing）或显式的 undefined 检查。

这不是纸面假设。Bruno 仓库中当前就存在这一模式的实例：ImportCollectionLocation/index.js 第 154 行 在导入集合转换时写有：

version: convertedCollection.version || '1',

按 correctness 评审员的规则，这一行正是需要被标记（flag）的对象：如果 version 的"缺失"在 DSL 契约中有独立含义，|| '1' 就在导入路径上伪造了一个版本号，且可能与另一条解析路径的缺省行为产生分歧。仓库在 DSL 规则 中甚至把"一条路径返回 ''、另一条返回 '1'"列为了分歧 bug 的示例形态——文档与实现检查清单在此互相咬合。

七、与 CI 评审的镜像关系与文件级检查清单

cost-review 技能 的定位是"mirror the automated CodeRabbit review (.coderabbit.yaml)，让同样的评审可以在本地运行"。两边的权威来源顺序一致：编码标准 → CODING_STANDARDS.md；架构/行为 → .claude/rules/*；.coderabbit.yaml 只是把这些规则镜像到 CI 以保持一致，若两者冲突以 .claude/rules/ 为准。例如 .coderabbit.yaml 的 path instructions 覆盖了三块与本评审直接相关的内容：全部路径的跨平台 OS-agnostic 检查（对应 cross-platform.md 视角）、packages/** 下按包粒度判断的 TypeScript 渐进迁移检查、以及 tests/** 的 Playwright 规范检查（对应 e2e-tests.md 视角）——correctness 视角则对应 CI 的通用 tone instructions。

最后，把 correctness 评审员的完整检查清单收敛为一份可执行的 checklist，供移植到其他仓库时参考：

评审范围与格式

• [ ] 只审 diff 内的改动文件，范围是全部源码、排除 tests/**
• [ ] 输出为 <blocker|suggestion|nit> | <file>:<line> | <一句话发现> 的扁平列表；干净时只回 no findings

根因判定（blocker 级）

• [ ] 每个 bug fix：修复前追溯了问题的起源，而非停在症状显现处
• [ ] 没有用 null 守卫、try/catch 吞错、防御性重复检查、重试、超时、数值钳制压制症状
• [ ] 修复发生在正确的层级（数据层的 bug 不在 UI 层兜底，调用方不绕过被调用方的契约违规）
• [ ] 修复覆盖同一根因的所有调用点，而不只是报告的单个复现场景
• [ ] workaround 类修复已点名并说明真正解决问题的更深层改动（即便超出本 PR 范围）

常规正确性

• [ ] 无 off-by-one / 边界错误
• [ ] 无未处理的 Promise rejection、无缺失的 await
• [ ] 无被吞掉的错误
• [ ] null/undefined 处理正确
• [ ] 变更引入的新边界情况已被考虑

Bruno 项目特化（在含平行实现的项目中同样适用）

• [ ] 触碰了 .bru 路径？对照 .yml 侧的解析与序列化行为
• [ ] 触碰了默认 app-data 工作区？对照自定义文件系统工作区
• [ ] 两条路径在"未设置态"上的默认/持久化行为一致
• [ ] 在"缺失有意义"的字段上未使用 || 隐式造值，改用 ?? 或显式 undefined 检查
• [ ] 相关字段若新增，已同步 bruno-schema（Yup，.noUnknown(true).strict() 会在保存路径拒绝未声明字段）与 bruno-schema-types（TS 类型），并在两侧格式的 round-trip spec 中有测试

八、小结

correctness.md 的价值在于把"正确性评审"从一个模糊概念落成了一份可并行执行、可验证、与 CI 对齐的操作规程：三档严重级别与单行输出契约保证发现可合并可追踪；"症状补丁即 blocker"的根因判定给出了明确的否决标准；twin path 与 x || default 两条规则则是对 Bruno 这类"同行为多实现、磁盘格式即公共契约"的复杂代码库量身定制的深挖探针。对于拥有平行实现路径（多格式序列化、多环境、多端）的项目，这套"修根因 + 核双路径 + 护未设置态"的方法论可以直接复用。