我用AI辅助代码审查的经验：优先级框架比检查清单更重要

我尝试 AI 辅助代码审查已经有一段时间了。

最初的几次尝试并没有特别有用。

它们不能说完全错误。审查确实找到了一些东西：这里一个值得商榷的命名，那里一个可能需要提取的函数，也许还有一个可以更精确的 any。

活动倒是不少。

但真正的审查却很少。

让我困扰的是比例失调。一个命名建议和一个可能的竞态条件可以并列出现，仿佛它们值得大致相同的关注。

这个观察促使我做了一个小实验。

我没有让 AI 去查找代码中的问题，而是尝试描述我希望审查如何进行。

我最终写了一个 code-review skill。

这是一个关于其中投入了什么、过程中发生了什么变化，以及这个练习如何教会我关于代码审查本身的故事。

第一个版本是一份检查清单

这看起来很合理。

前端代码有很多值得检查的东西：

所以我把它们写了下来。

结果很全面。

但也不是很好。

只要有足够多的检查项，AI 几乎总能找到可说的东西。困难在于，软件工程很少是关于找到尽可能多的技术性站得住脚的观察。

而是关于决定什么重要。

考虑一个小改动：

useEffect(() => {
  fetchUser(userId).then(setUser)
}, [userId])

关于这四行代码，人们可能会说很多事情。

但对我来说有趣的问题是，当 userId 在第一次请求完成前发生变化时会发生什么。

请求 A 为用户 10 发起。

组件发生变化。

请求 B 为用户 11 发起。

代码仍然很短。类型可能仍然是正确的。linter 可能完全满意。

但 UI 现在可能显示的是错误的用户。

这是 skill 中的第一个有用的转变：

审查代码周围的行为，而不仅仅是 diff 中的代码。

审查需要有轻重缓急的概念

一旦我注意到了这一点，检查清单开始看起来像错误的抽象。

这个 skill 需要优先级。

我在开头最终写了这句话：

将代码审查作为生产风险审计，而不是风格检查。

这句话变得出乎意料地有用。

它给了审查一个方向。

资源泄漏比命名偏好更重要。

破坏的权限边界比重复代码更重要。

丢失用户的更改比 helper 是否应该被提取更重要。

乱序响应比不必要的分配更重要。

这并不是说可维护性、性能或风格无关紧要。

只是给了它们一个站立的位置。

审查从那些可能破坏用户信任或导致运行系统偏离其预期行为的事情开始：生命周期泄漏、异步竞态、状态完整性、权限、持久化和其他高影响路径。

这个排序让审查明显安静了下来。

我认为这是进步。

先契约，后实现

下一个问题更加微妙。

如果审查者不首先决定代码应该做什么，他如何判断代码是否正确？

写下来时这听起来很明显。

在实践中，我一直在跳过它。

所以第一个审查关卡变成了：契约。

契约不一定是写在一个地方的。它可能分布在 pull request 描述、类型、测试、公共导出、文档、相邻代码和项目的词汇中。

甚至 "refactor" 这个词也包含契约。

这只是一个重构。

我在做一个相当强的声明：

可观察行为没有改变。

这提高了性能。

包含另一个声明：

有一个可衡量的改进，且相关行为已被保留。

一旦我开始把这些声明当作声明而不是描述来处理，审查就变了。

"这个实现好吗？"

变成了更有用的问题：

"这个更改承诺了什么，什么能说服我相信这个承诺成立？"

我喜欢这个框架，因为它从对话中去除相当多的个人偏好。

diff 比变更更小

另一件我不断遇到的事情是 pull request 的边界。

一个 prop 在一个组件中变化，但行为变化属于每个消费者。

一个查询 key 在一个 hook 中变化，但后果属于缓存。

一个 API 字段在一个类型中变化，但实际表面可能贯穿验证、映射、持久化和好几处 UI。

一个新选项尤其有趣。

弄清楚每个现有消费者在该选项出现时做什么，往往是真正审查开始的地方。

所以我加了另一个关卡：受影响的表面。

指令本质上是跟踪变化的值和行为在整个系统中的流动：props、state、API payloads、事件、权限、加载状态、错误、缓存 key、导出和副作用。

与评论更改的行相比，这很昂贵。

但它也更接近于我在手动审查重要变更时实际做的事情。

diff 告诉我编辑发生在哪里。

它不一定告诉我变更发生在哪里。

时间值得拥有自己的审查

前端软件还有另一个棘手的属性：很多行为存在于时间之中。

组件渲染、挂载、更新和卸载。

请求发起、被取消、重试、解决或变为过时。

表单初始化、编辑、验证、提交、被拒绝、重置，也许中途被放弃。

在这些流程中看一个时刻会隐藏数量惊人的 bug。

所以 skill 开始显式描述生命周期：

Component
render → hydrate → mount → update → suspend → unmount

Async work
start → cancel → retry → resolve/reject → stale result → cleanup

Form
initialise → edit → dirty → validate → submit → error/success → reset

Data
fetch → normalise → validate → cache → derive → render → mutate → invalidate

这些不是作为通用的状态机。

它们是注意力的提示。

当一个 effect 变化时，我希望审查跟踪它直到卸载。

当一个表单变化时，我想知道服务器拒绝提交后会发生什么。

当缓存变化时，我想知道陈旧数据最终如何再次变新鲜。

这成为更有用的添加之一。

证据改变了审查的语气

仍然有问题。

AI 非常擅长产生看似合理的担忧。

这可能导致不必要的重新渲染。

这里可能有竞态条件。

考虑对这个值进行记忆化。

每句话听起来都合理。

但没有一个是一定正确的。

我不想通过让模型更有信心来解决问题。

我希望不确定性成为输出的一部分。

这导致了证据关卡。

一个正确性更改应该有回归测试，或者一个为什么一个不切实际的解释。

一个行为保留的重构应该有相关旧行为仍然存在的证据。

一个性能改进应该有测量。

当可用上下文无法确定某事时，审查应该说出来。

我无法确定这个请求在组件卸载时是否被取消。调用者生命周期或一个集中的取消测试可以弥合这个差距。

我觉得这比以下内容更有用：

可能的内存泄漏。

前者告诉我什么是已知的，什么是未知的，以及如何减少不确定性。

后者主要告诉我内存泄漏是可以想象的。

也许不足为奇，这也让审查变得不那么有争议了。

证据比信心更容易讨论。

重构需要不同的规则

在某个时候，我尝试用同一个 skill 来做重构。

这暴露了另一个区别。

当我请求审查时，我希望系统挑战行为。

当我请求重构时，我通常想要相反的：在行为上极度保守的同时改进实现。

所以重构获得了一个相当严格的约束：

除非明确批准，否则零行为更改。

公共 API、执行顺序、副作用、错误语义和可观察身份在这里都很重要。

即使实现更清晰，如果它悄悄地改变了其中之一，也不是改进。

这也改变了我对抽象的思考方式。

两段代码看起来相似并不是将它们合并的充分理由。

我现在更倾向于问它们是否共享契约和相同的边缘情况。

如果是，也许存在一个抽象。

如果它们只是今天恰好包含相似的行，重复可能更便宜。

skill 现在长什么样

经过几次迭代，结构变得比最初的检查清单更小。

在概念上，它大约是：

Contract
   ↓
Impacted surface
   ↓
Failure & divergence
   ↓
Evidence
   ↓
Lower-priority quality

在它下面有针对 effects、forms、caches、permissions、dates、APIs、生成文件、依赖关系和其他领域的专门扩展。

但这些是次要的。

顺序才是重要的。

它告诉审查者在哪里投入注意力，然后才告诉它可能注意到什么细节。

这个区别花了我比预期更长的时间才看到。

我以为我在为 AI 写指令

这个实验中稍微出乎意料的部分是，我不再确定这个 skill 主要是关于 AI 的。

写它迫使我将自己的审查习惯明确化。

为什么我有时在评论一行更改的代码之前会打开五个文件？

因为我在追踪受影响的表面。

为什么一个看起来无害的 useEffect 会让我不舒服？

通常是因为我还没有考虑到它的生命周期。

为什么我不愿意批准一个重构，尽管新代码明显更清晰？

因为更清晰在 diff 中是可见的。

行为保留需要证据。

为什么有些有十五条评论的审查比只有一条评论的审查感觉更没用？

因为观察的数量不等于消除的不确定性的量。

我从一个相当机械的问题开始：

AI 应该如何审查前端代码？

我最终得到了另一个问题：

当我相信自己正在好好审查代码时，我实际上在做什么？

这个 skill 是我目前的答案。

我预计它会改变。

这可能是它最有用的属性。