审视大型人工智能重构:为何通过测试却忽略了被删除的意图
了解为何人工智能辅助的代码重构会悄悄移除关键代码,为何绿色测试无法发现这一问题,以及五种能让大规模代码重构安全发布的审查方法。
来源: I Let Claude Refactor 4,000 Lines. Here’s the Part I Had to Undo.
如今的AI编程助手足以在下午时间内整理好一个长期被忽视的模块,其优化后的代码质量通常优于原有代码。真正的危险并非粗制滥造的代码,而是那些看似正确却隐含了未被记录的功能行为的代码。本指南将通过一个真实的案例分析此类问题,解释为何测试套件和审查人员都会漏掉这些问题,并为大规模的AI辅助代码重构提供具体的审查流程。
一次提升了代码质量却导致系统故障的重构
想象这样一个已经积累了两年修改的模块:共有约4,000行代码分布在四个文件中,是工程师们尽可能避免触碰的代码库中的“死角”。该模块存在不一致的错误处理机制、三种不同的日期解析方式,还有几个功能是否仍在被调用也无人能确定。
开发人员分两次将代码交给AI助手处理,最终得到了明显更好的结果:格式统一、类型提示正确、冗余代码被删除,行数减少了约三分之一。测试也通过了。开发人员查看了差异对比,或认为自己已查看完毕,随后便将代码发布上线。
六天后,某个合作伙伴的集成系统开始丢失记录。并没有出现明显的错误提示,大约4%的记录不再被传输。问题的根源在于代码重构,尤其是开发人员已经阅读并批准的某项更改。
助手真正表现出色的地方
有必要明确指出哪些方面做得好,因为这并不意味着“AI不擅长代码重构”。对于那些仅依赖于当前代码的转换任务,该助手表现得非常出色:
- 将三个日期解析辅助函数合并为一个函数
对于那些通过阅读就能验证的简单局部修改,这种工具比人工操作更快且更一致,也不会在数百行代码之后就失去重点——而这正是手动重构往往表现不佳的地方。当任务是“让这段代码更清晰地表达相同的行为”时,这些工具的表现非常出色。问题则出现在“相同行为”的定义范围上。
为何承重代码会被删除
那些没有明显存在理由的代码看起来简直就是冗余代码。以下是获批的修改内容:
# before
def normalise_timestamp(raw):
ts = raw.strip()
if ts.endswith("Z") and "." in ts:
ts = ts.split(".")[0] + "Z" # partner sends 7-digit microseconds
return datetime.fromisoformat(ts.replace("Z", "+00:00"))
# after
def normalise_timestamp(raw):
return datetime.fromisoformat(raw.strip().replace("Z", "+00:00"))
被移除的代码分支看起来正是典型的重构目标:一个特例处理、一种过于具体的后缀检查以及字符串分割操作。它似乎是多余的,因为fromisoformat已经能够解析ISO时间戳了。
问题出在输入数据上。其中一方会发送带有七位小数位的时间戳。在 Python 3.11 之前,fromisoformat 只能解析 isoformat() 自身生成的格式,这意味着小数位最多只能有六位,因此七位的小数值会引发 ValueError 错误。Python 3.11 及更高版本能够支持更广泛的 ISO 8601 子集,在这方面更为宽容,所以这一变更是否会导致问题取决于运行环境,这又是另一个需要验证而非直接假设的理由。该分支的存在源于十八个月前的一份支持工单,而其简短的内联注释并不足以让任何人——无论是人类还是模型——充分权衡其必要性与其看似可移除的特性。另外还需注意,旧分支的实际功能比看上去的更复杂:它完全舍弃了小数部分,因此即便在“后续”版本中,对于那些确实可以解析的输入数据,其输出结果也会有所不同。
核心问题很简单:模型能够看到代码,却看不到生成该代码的故障场景。工单、事后分析报告、聊天记录以及调试所花费的时间都不在文件范围内。运行git blame会显示出类似“修复合作伙伴数据源的时间戳解析问题”这样的提交信息,这便是删除代码的全部理由,但没人要求助手去查看相关内容。审核人也出于与助手相同的理由批准了删除操作:两人都只看了代码差异,而没有查看历史记录。
为何测试套件仍显示通过
这些测试检查的是函数对人们所设想的输入值返回的结果,而存在问题的输入值按定义本来就是没人想到的那种。
这些测试用例中的时间戳带有三位小数,因为人们手动编写测试数据时通常会这样输入。而七位数的时间戳仅出现在某个合作伙伴的实时数据流中。
无论是否使用人工智能,大规模代码重构都存在风险:测试通过只能证明那些被测试用例覆盖的功能依然正常,却无法说明未被测试的功能状况。当手动重构30行代码时,你会仔细考虑每一行;但一旦有4000行代码同时发生变化,测试用例几乎要承担所有的验证工作,而它们本来就不是为这种负荷设计的。
代码覆盖率指标也毫无帮助。该函数的行覆盖率为100%:测试过程中每一行代码都得到了执行。只是对于某些关键输入,某个分支根本就没有被触发。行覆盖率衡量的是哪些代码被执行,而非使用了哪些输入情况。
按风险对AI修改进行分类
实用的应对方法是不再统一审查AI生成的差异,而是按类别进行分类处理。这些类别的风险程度差异很大:
- 低风险:类型注解、重命名、格式调整、提前返回以及类似的机械性改写。这类修改影响范围有限,通过阅读即可轻松验证。
- 中风险:合并重复的辅助函数。通常没有问题,但需确认合并后的版本能处理所有调用者的输入。
- 高风险:删除看似未使用的代码,移除看似多余的特例,统一看似不一致的行为。
高风险类别有一个共同特点:在每种情况下,模型都能正确识别出问题所在,而正是存在这些问题,才使得模型需要被设计出来。
最后一个问题也以更为隐蔽的方式显现。设想有一个模块对错误的格式处理方式与其他模块不同,后来进行了重构以使其一致。这种差异是刻意为之的,因为下游系统会解析那种特定的错误格式。追求一致性本是合理的直觉,但在此处应用却是个错误。
下一次大规模重构的五项实践
这些实践是按照其各自在多大程度上能够避免上述问题而大致排序的。
将每次修改的范围控制在便于实际审查的程度
四千行代码根本无法进行仔细审查。一旦超过几百行,大多数人就会只粗略查看明显的问题,而不会像自己说的那样逐段分析。每次审查仅针对一个模块,每份拉取请求也只包含一个模块的修改内容。瓶颈不在于模型的处理速度,而在于审查者的注意力。
要求为每次删除提供理由
在接受输出结果之前,先让助手列出它删除的所有内容,并说明针对每项内容为何认为删除是安全的。将推理过程以文字形式呈现是你能采取的最有价值的措施之一。任何类似“此内容看似冗余”之类的理由都应被视为需要人工核查的信号。
向模型提供历史记录
告诉助手针对它建议删除的每一行使用git blame命令查看,并报告对应的提交信息。异常分支的成因往往记录在那些提交信息中,模型在有这些上下文时能很好地利用它们;否则它不会主动去查找。
在重构前写下“原因”注释
应先添加解释性注释,而非事后补充。如果能有明确的注释,注明相关方、七位数的分数以及六位数的解析器限制,很可能就能避免此次删除。而仅关于相关方时间戳的模糊说明则起不到作用。如果后续阅读代码的人——无论是人类还是工具——无法理解某行代码的存在原因,那么该行很快就会被删除。
结合实际流量对比旧版本与新版本
将一天内的真实生产数据分别输入到旧版本和新版本中,比较两者的输出结果。对于解析和规范化这类纯转换操作而言,这样做成本很低,而且能够准确发现测试用例遗漏的问题类型,因为真实生产数据中包含那些没人想到要记录下来的输入内容。如果无法重新模拟流量,那么从真实数据中抽取样本作为回归测试用例也是次优选择。
关键要点
这次重构仍然是值得的:模块功能得到了提升,工作量仅需一个下午即可完成,而且在四千行修改代码中仅出现一次回归问题,这与许多大规模的手动重构相比已经相当不错。将结果称为“AI的错误”其实偏离了重点——AI只是根据它能看到的信息得出了合理的结论,而那只是一个没有明确原因的特例;人类审查员也是从同样的不完整信息中得出了相同结论。
- 代码仓库只是系统的一部分;那些奇怪代码背后的原因存在于工单、提交信息、事后分析以及人们的记忆中。
- 只关注代码的工具会轻易删除那些从未有过书面解释的内容。
- 绿色测试和完整的代码覆盖率并不能证明对于那些未被建模的输入,程序的行为依然保持不变。