sanguo_moziplus_v2/skills/review-quality.md

name, description

name	description
review-quality	代码评审的最佳实践：闭环标准、枚举一致性、三层对照、自我纠正。 在执行代码评审、或准备提交评审时触发。

Review Quality（评审质量）

来源：从司马懿 122 次纠正和庞统评审经验中蒸馏（卡片 1、2、3、4、5）

适用场景

• 执行代码评审时（作为评审者）
• 准备提交代码给评审者时（作为被评审者）
• 设计文档与代码实现的对齐验证
• 多方案并行开发时的闭环跟踪

模式清单

模式 1：一键三连闭环 — 文档→代码→最终代码确认

评审不是止于方案确认。完整闭环是三步：改文档（设计/方案）→ 改代码（实现）→ 评审者确认最终代码片段。"已采纳建议"不等于"已验证"——采纳后的代码可能引入新问题。评审者的确认必须是具体的（引用代码行号，确认与实际改动一致），不是笼统的"通过"。

Evidence: 卡片 1 — 庞统让司马懿 review 了方案，但实现后没发最终代码确认。用户指出"一键三连最终肯定是以司马懿 review 完最终成果物收尾的"。卡片 5 — 状态跟踪表应包含三列：方案评审 / 最终代码确认 / 状态。

模式 2：枚举值一致性审查 — 零容忍硬编码字符串

状态机/枚举值不一致是高频 Bug 根源。审查时重点检查：新代码中的状态值是否从枚举定义取值（硬编码字符串零容忍）、状态转换是否只有一个入口函数（如 _transition_status）、所有 spawn/查询/恢复路径是否使用相同的枚举引用。发现不一致时标记为 high severity。

Evidence: 卡片 2 — get_all_executing_tasks SQL 用 'executing' 但实际枚举是 'working'，导致重启恢复遗漏 working 节点（P0-2）。max_per_agent=2 vs 设计文档的 1 也是同类问题。

模式 3：三层对照审查 — PRD→设计→代码逐条覆盖

每次代码评审前，先对设计文档逐条检查实现覆盖度。设计文档用明确编号（§B1/B2/B3）便于逐条追踪。评审者的核心职责就是做这种三层对照：PRD 要求了什么 → 设计文档怎么规划的 → 代码实现了多少。遗漏一个章节就等于功能缺失。

Evidence: 卡片 4 — spawner-monitor 设计文档 §B2（compact 进行中继续等）在代码中完全遗漏，直到庞统重新读设计文档才暴露。庞统专门发 #311 邮件请求"PRD→设计→代码三层一致性审查"。

模式 4：自我纠正机制 — 承认误判比坚持错误更有价值

评审中发现自己的判断有误时，主动发出修正并明确标注。交叉验证是发现误判的有效手段：翻回设计文档对照，看代码实际行为 vs 自己的判断。庞统和司马懿在同一次评审中都做了自我纠正——这说明交叉验证机制在工作。修正保留审计轨迹，不能静默修改。

Evidence: 卡片 3 — 庞统修正"建议删除 recent_compact"为"设计文档明确要求"；司马懿回复"你说得对，我判断错了"。

模式 5：评审前验证实测数据，不接受"应该是"

涉及外部命令输出解析、API 返回结构、配置文件内容时，评审者应要求看到实测输出样例，而非接受代码中的假设路径。"从第一天就存在的根因 bug"往往源于解析路径是猜的而非验证的。评审时把"需实测验证"作为一条显式检查项。

Evidence: 卡片 1 — 司马懿多次在代码评审中发现方案评审没暴露的 Bug（如 _do_retry 续杯失效），靠的是看最终代码而非方案描述。卡片 2 — auto-sync 可能静默修改配置文件，需要与设计文档对照验证。

检查清单

• 方案评审通过后，是否安排了最终代码确认环节？
• 状态/枚举值是否全部从集中定义取值，无硬编码字符串？
• 设计文档是否逐条（按编号）检查了实现覆盖度？
• 发现自己判断有误时，是否主动发出了修正？
• 外部输出解析路径是否提供了实测样例验证？
• 评审结论是否具体到代码行号，而非笼统"通过"？
• 所有 spawn/查询/恢复路径是否覆盖了新功能的参数传递？