0. 我给自己做了一次代码审查，然后否决了它的大部分方案
有一阵子我觉得代码里积了不少问题——文件越来越大、有几处逻辑我自己都记不清了。于是我让一个模型对 develop 做了一轮系统性审查，产出一份问题清单。
清单挺长的，分了 P0/P1/P2，每条都有问题描述和修复建议。
我的第一反应是"那就按清单修"。但真的开始动手之后，我发现这份清单有一个根本问题：
它是在没有读全部代码的情况下写的。
举一个具体的例子。清单里有一条说 domain/ 这个目录"名不副实"——应该叫别的名字，因为里面的东西不像领域模型。
这条看起来有点道理，但它不知道一件事：我在 AGENTS.md 里明确写了 domain/ 是"跨服务共享领域库"，并且定义了它的准入标准。 按我的定义，domain/ 里的东西恰恰是对的。
所以这一条不是"要修的问题"，而是"审查者和我对某个概念的理解不同"。
于是我做了第二轮：逐条复核，而不逐条执行。
这篇讲这个过程——哪些我认了、哪些我否了、哪些我决定推后、以及哪些我决定什么都不做。
1. 复核的结构：四个字段，而不是一个状态
我给每条问题都有四个字段：
| 字段 | 含义 |
|---|---|
| 原条目 | 报告里怎么说的 |
| 复核结论 | 我核实之后是否成立（可能推翻、可能降级） |
| 优化后的处理 | 我要怎么做（可能不等于原建议） |
| 状态 | ✅ 已修复 / 🟡 按既有路线图进行 / ⏸ 独立治理 |
关键在第二个字段。 它允许三种结论：
- 成立 —— 直接采纳；
- 部分成立 —— 风险真的存在，但描述不准，处理方式要改；
- 不成立 —— 我核实后推翻。
而"优化后的处理"这个字段允许我采纳问题但不采纳方案。这是最重要的一点。
2. 三种"不照做"的情况
情况一：问题成立，但原方案会制造新问题
这是最危险的一类——按建议改会引入 bug。
原报告里有一条 P1-1：上游已经成功返回之后，本地的 commit 失败会触发重复调用上游。
问题确实成立，而且很严重（重复调用上游等于付两次钱）。
但原报告给的方案是：
继续返回成功响应，再由 cleanup / reconciliation 补齐。
我的复核结论是：
P1-1 正确识别了"上游成功后不能再次调用上游"，但"继续返回成功响应，再由 cleanup / reconciliation 补齐"缺少持久化事实。现有 cleanup 会释放过期 reservation，reconciliation 不能从 reservation 还原实际 usage；照原方案实施可能产生免费调用。
拆开说：
- cleanup 会释放过期的 reservation —— 它的语义是"这次预扣没结算，把钱退回去"。如果 commit 失败但上游已经成功了，那这笔钱不该退（用户确实消费了）；
- reconciliation 不能从 reservation 还原实际 usage —— 对账能看到"这个 reservation 没结算"，但它不知道这次调用实际用了多少 token。它能发现差异，但不能修复它。
所以"交给 cleanup / reconciliation 补齐"这个方案的实际效果是：免费调用被系统地合法化了。
我采用的方案是：
上游响应完成后的本地错误标记为
PostForwardError：不可重试、不可污染渠道健康；仍向客户端返回结算失败。
也就是这次请求失败（返回结算失败），而不是假装成功。宁可让一次调用失败，也不制造一次免费调用——而且这个失败会进指标，会被看见。
这个决定和第 15 篇里 PostForwardError 的设计是同一件事。审查发现的问题是"会重复付费"，原方案把它变成了"会免费"，两个都不对。
情况二：问题成立，但原方案不完整
另一条 P2-1 说：上游返回 405 时会被当成"协议能力不匹配"而去重试别的渠道，但 405 是"这个端点的方法不允许"，重试没意义。
问题成立。原方案是"从通用状态表里移除 405"。
我的复核结论：
只从通用状态表移除 405 不会改变行为，因为原
IsProtocolCapabilityMismatch把所有 405 都视为能力不匹配。必须先引入 endpoint-aware 类型，再移除通用 405。
因为决定"要不要重试"的不是状态码表，而是那个 IsProtocolCapabilityMismatch 函数。 那里有一句 status != 400 的提前返回，还有对 405 的隐式处理。
所以修法是两步，而且顺序不能反：
- 先给 Responses 那条路径引入类型化的能力错误（第 3 篇讲过的
ProtocolCapabilityError）； - 再从通用配置里移除 405。
如果反着做（先移除、后引入），中间会有一个窗口：405 既不在重试表里、也不被类型化——它会变成一个"立刻失败且不可重试"的错误，把本来该重试的场景也堵死了。
"原方案不完整"和"原方案是错的"不一样。 前者是缺了一步，补上就行；后者要换方案。区分它们需要看代码，而不是看清单。
情况三：问题不成立
就是我开头说的 domain/ 那条。复核结论直接写"不成立"，理由是指出仓库规范已经定义了它。
还有一条更细的，关于"构造函数冗余"：
| 原条目 | 复核结论 |
|---|---|
| P2-5 构造函数冗余 | 原报告数量错误：当前是 3 个公开构造函数，不是 5 个 |
报告先说"有 5 个构造函数"，我数出来是 3 个。 数字错了说明它没有真的数——而一个数字不准的条目，它的其他结论我该不该信？
不过这里有个更值得说的后续：我写这篇的时候又数了一遍，现在是 5 个。
orchestrator.go:158 NewRelayOrchestrator
orchestrator.go:164 NewRelayOrchestratorWithProviderFactory
orchestrator.go:170 NewRelayOrchestratorWithDependencies
orchestrator.go:178 NewRelayExecutorWithDependencies
orchestrator.go:186 NewRelayExecutorWithForwarder
也就是说，当初"是 3 个不是 5 个"这个更正本身，现在也过期了。而这条技术债的状态是"随 executor 退场收敛"——executor 还没退场，构造函数就先从 3 个涨到了 5 个。
这比"报告数错了"更值得记：一份审查报告里的数字，从写下的那一刻就在过期。 这也是我坚持给报告文件名加日期的原因——它提醒读的人"这些数字属于那一天"。
我现在对审查报告的态度是：它给的是线索，不是结论。 每一条都要回到代码里核实，而核实本身经常改变处理方式。
3. 严重程度经常被高估
有一条 P1-2 叫"字符串错误分类"。报告的语气是"用字符串匹配来判断错误类型，很脆弱"。
我核实之后：
P1-2 字符串错误分类：严重程度被高估；provider / adaptor 已返回类型化
UpstreamHTTPError，但IsRetryable未复用既有类型化提取，模型权限仍是普通字符串。
问题真实存在，但成因不是"到处都在用字符串"，而是"已经有的类型化错误没有被复用"。
这是两种完全不同的修法：
- 如果满仓库都在匹配字符串，那是一次大重构；
- 如果是"有类型化对象但某个函数没用它"，那改一个函数就行。
我最后做的是：
IsRetryable统一走UpstreamStatus；模型权限在 biz 源头改为ReasonModelForbidden。
两处小改动，而不是一次大扫除。
这让我意识到审查报告的一个系统性问题：它会根据"看到的现象"推断"问题的规模"，而规模需要数。 看到一处字符串匹配，很容易写成"错误处理依赖字符串匹配"——但可能全仓库只有这一处。
4. 明确写出"不做什么"
这一节是整份报告里我最满意的部分，标题就叫"明确不纳入本轮的工作"：
- 不在生产 7 天观察完成前删除 legacy handler、WebSocket 路径或双
RelayRequest；- 不为 3 个迁移期构造函数引入 functional options；
- 不以"大文件行数"为唯一依据拆 channel Repository；拆分必须先标明聚合、事务和测试边界；
- 不在缺少 CSRF / refresh / logout 契约时把一部分 token 迁入 cookie；
- 不新增 relay 进程内 Commit 重试队列，它不能提供崩溃恢复，还会与 billing 异步队列形成双写语义。
五条，每条都有理由。我挑三条说。
4.1 不在观察期内动执行模型
第一条和第二条都属于这一类。当时 executor 那条新路径正在做 7 天生产观察（第 2 篇提过），而 RelayRequest 有两个版本（legacy 和 executor 各一个），构造函数有三个。
"清理这些重复"确实该做，但现在不能做。 因为：
观察期的目的是看新路径在生产下的表现。如果我在观察期内改了执行模型，那观察的结论就失效了。 第 2 篇里我写过 executor 那次观察最后判 FAIL 并且窗口作废——如果我在那期间还改过别的东西，我甚至无法归因那 69.7% 的回归是谁造成的。
所以复核结论是"随 executor 退场"——这些技术债和 executor 的退场绑定在一起，不单独处理。
4.2 不以行数为依据拆文件
第三条针对的是一条"channel data 层有 3152 / 1883 行，太大"的问题。
行数是事实（我核实了），但报告的"纯机械、风险低"这个判断：
A2 channel data 上帝文件 | 3152 / 1883 行事实成立；"纯机械、风险低"判断过于乐观 | 在对应聚合发生业务改动时分 PR 拆；每次保持 Repository 与事务边界不变
"拆大文件"看起来是纯机械操作，但它会动事务边界。 而第 1 篇里我讲过那个 ...InTx 的方法群——事务的所有权在 biz，data 只提供"在事务里做这一步"的原子操作。如果拆文件的时候顺手把一个方法挪到另一个 repository，事务的语义可能就变了。
所以我给拆分加了个前置条件：先标明聚合、事务和测试边界。而且触发方式是"在对应聚合发生业务改动时顺手拆"，而不是单独开一轮。
"顺手拆"这个策略的价值在于：我不是为了拆而拆，而是在真正需要理解那段代码的时候拆。 那时候我对边界的理解是最准的。
4.3 不做一个"看起来能解决问题"的重试队列
最后一条我觉得最值得讲：
不新增 relay 进程内 Commit 重试队列，它不能提供崩溃恢复，还会与 billing 异步队列形成双写语义。
这条针对的是"commit 失败怎么办"这个问题。一个直觉的方案是：在 relay 里加一个内存重试队列，commit 失败了就排队重试。
我否掉的理由是两句话：
第一，"进程内"等于不能提供崩溃恢复。 如果 relay 进程挂了，队列里所有没提交的结算全部丢失——而它们对应的调用是已经成功返回给用户的。 这是最糟的组合：用户拿到了结果，账上没有记录。
第二，"还会与 billing 异步队列形成双写语义"。 billing 服务自己有一个异步结算队列（第 2 篇提到的 AsyncBillingQueue）。如果 relay 也排队，那么同一次 commit 可能从两条路径到达 billing——一个是 relay 的重试、一个是 billing 自己队列的，而它们之间没有共同的去重键保证。
一个方案如果解决不了根本问题（崩溃恢复），还引入了新的问题（双写语义），那它不该做，即使它能"让指标看起来变好"。
5. 完成条件也写成了清单
报告的最后一节是"完成条件"：
本轮只有在下列条件全部满足后才能标记完成：
internal/biz、internal/server、platform/middleware和 billing biz 相关测试通过；./scripts/check-architecture.sh通过；- Markdown 本地链接和
git diff --check通过；- executor 观察手册的用户现有记录保持不变；
- 本文状态和验证表更新为最终事实，不把尚未结束的生产观察误报为完成。
第 5 条是我特意加的。因为那轮修复的时候，executor 的 7 天观察还在跑——它不属于这轮"完成"的范围。
如果我在报告头部写"全部完成"，读者会以为整个系统状态是干净的，而实际上有一条观察还在进行。所以头部写的是：
状态：✅ 本轮方案复核、修复与验证已完成。executor 生产 7 天观察仍在进行，不属于本轮完成状态。
"完成"必须有明确的边界，否则它会膨胀成"我觉得差不多好了"。
第 4 条也很具体："executor 观察手册的用户现有记录保持不变"。意思是这轮修复不许去改那份观察记录里的历史数据——因为那是我用来做判断的原始事实，不能被后续的修复覆盖掉。
6. 验证记录：交叉的，不只是跑测试
这一节列了八项验证：
| 验证 | 结果 |
|---|---|
| 新增用例的定向回归 | ✅ |
go test 相关包 |
✅ |
go test -race 相关包 |
✅ |
go test ./cmd/relay-gateway ./internal/conf（验证真实配置能加载） |
✅ |
./scripts/check-architecture.sh |
✅ |
./scripts/check-deployment-docs.sh |
✅ |
python3 scripts/check-markdown-links.py（142 份文档） |
✅ |
git diff --check |
✅ |
有意思的是后面几项——它们和"我改的那几行代码"没有直接关系：
- 架构检查（第 1 篇那十条规则）；
- 部署文档检查（Compose / K8s 资源和文档链接有效）；
- 142 份 Markdown 文档的本地链接检查；
git diff --check（空白字符错误）。
为什么改几行 Go 代码要跑这些？ 因为我的改动会顺带改文档（每轮修复都要更新报告），而文档里的链接会断。而 check-deployment-docs.sh 是因为有一处修复动了环境变量（CORS 的默认值），配置模板里引用了它。
"验证"不是"跑一遍相关测试"，而是"把所有可能被我这次改动破坏的东西都检查一遍"。 而"可能被破坏的东西"包括文档和部署模板——只要我动过它们。
7. 我现在对"审查清单"的态度
做完这两轮，我的看法变了。几点：
第一，清单给的是线索，不是任务。 每一条都要回到代码里核实，而核实会改变处理方式——有时候是降级，有时候是换方案，有时候是推翻。
第二，允许"采纳问题但不采纳方案"。 这是最有用的一条。审查者看到的现象通常是真的，但他根据现象给的修法不一定对——因为他没有全部上下文。
第三，"明确不做什么"要写下来。
这一条我认为是整份报告最重要的部分。原因很实际：如果不写，同一个问题会在下一轮审查里被重新提出来。 而写下来之后，它就变成了一个有理由的决定，下一个人（包括未来的我）能看到"这个我考虑过，当时决定不做，因为……"。
第四，修复的报告要带日期。 文件名是 systematic-code-review-remediation-2026-08-25.md。因为这类报告的结论是有时效的——不仅"已修复"的部分可能被改回去，连里面用来论证的数字本身都会过期（本文第 2 节那个构造函数数量就是现成的例子：报告写 5 个、复核时是 3 个、现在又是 5 个）。
第五，数字要自己数。 "3 个构造函数不是 5 个"这种错误说明报告在估。而估出来的数字一旦进了清单，就会被当成事实用来做决策。
8. 现在的状态和欠账
已经能用的：
- 一份带日期的复核报告，逐条记录"原条目 / 复核结论 / 优化后的处理 / 状态"
- 修了可复现的正确性问题（
PostForwardError、流取消、类型化 405、CORS 默认值、共享契约常量） - 明确列出五条"不纳入本轮"，每条带理由
- 完成条件包含"不把未结束的生产观察误报为完成"
- 验证覆盖代码、架构、部署模板、142 份文档链接、diff 空白
还没解决的：
第一，延期项没有独立的跟踪位置。 ⏸ 独立治理 / ⏸ 独立设计 这些条目现在只存在于这份报告里。没有 issue 或者 TODO 列表把它们聚合起来，所以"以后要做"实际上等于"可能会忘"。
第二，只有两轮，没有常态化。 我做的是"攒一阵子然后集中审查一次"，而不是持续的小步复核。这导致每轮的清单都很长，而且有些条目在等待期间会继续恶化。
第三，审查是模型做的，我复核。 这个分工里我承担的是"判断"，而模型承担的是"发现"。发现率取决于它的上下文完整性——它没有读全部代码，所以会有"数量错误"和"概念误解"这类问题。如果需要更准的发现，得让它读更多，而那是另一个成本。
第四，A2 channel data 那条只是定了策略（按聚合分 PR 拆），没有实际执行。 3152 行的文件还在。策略定了不等于债还了，这是这份报告里最容易产生错觉的一条。
第五，P2-3 localStorage token 我还是保持现状。 复核结论是"迁 cookie 需要 CSRF 与身份接口联合设计，不在本轮半迁移"。这是对的判断，但"半迁移"的替代方案是"不迁移"——而风险还在。
9. 下一篇
下一篇讲性能决策：为什么包一层 jsonx 而不是直接调 sonic、一个 24 小时 P95 回归 69.7% 的方案我是怎么决定回滚的、以及"准入线不降"这条规则怎么执行。