DevFlow 代码评审 Rubric 全解:六维检查项、blocking 判定与 agent-core-java 落地实践
DevFlow 代码评审 Rubric 全解:六维检查项、blocking 判定与 agent-core-java 落地实践
本篇文章面向在 agent-core-java 仓库中参与评审与被评审的开发者、AI 编码助手与工作流编排者,完整解读 代码评审 Rubric 的六大检查维度、严重级判定规则与 Verdict 指引,并结合仓库内的 DevFlow 配套技能、Java 编码规范与评审流程给出可直接执行的落地方法。读完本文,你将掌握"以怀疑者视角读 diff"的具体检查清单、哪些问题必须按 blocking/critical 处理,以及一轮评审从派发、落盘到 findings 闭环的完整操作路径。
评审定位:这份 Rubric 在 DevFlow 中扮演什么角色
代码评审 Rubric 是 DevFlow 工作流中 R3 代码评审门禁 的检查清单,定义在 .claude/skills/devflow-review/references/code-review-rubric.md。它明确界定了评审的输入与输出:
- 评审对象:实现 diff(即 TDD 阶段产出的代码变更)。
- 上游输入:
design.md、spec.md、适用的 coding-standards 与领域技能。 - 作者侧标准:
devflow-clean-code(见 devflow-clean-code 技能)。
在完整工作流中,评审是必经节点而非可选预审。根据 using-devflow 技能 的定义,DevFlow 把"产出高质量代码"拆成三层质量——第一层 SDD(做对的事)、第二层 TDD(证明做对)、第三层 Clean Code(做好)——而代码评审正是第三层质量的外在把关。specify、design、tdd 每个阶段产物完成后都要经过独立评审(R1/R2/R3),评审通过前不进入下一阶段。
评审的协作姿态是 human-on-the-loop:AI 生产,独立评审暴露问题,人做最终把关。因此 Rubric 的开篇给出了整份清单的灵魂句:
核心怀疑:这段代码哪里在对读者撒谎?错误路径和资源路径先读。
这句话决定了评审者阅读 diff 的方式:不是顺着主路径寻找"对不对",而是先假设代码可能在欺骗读者(命名撒谎、错误被吞、状态契约被破坏),并且把阅读重心放在错误路径与资源路径上——那是问题密度最高的地方。
评审者的三条不变量
围绕 R3 评审,devflow-review 技能 明确要求三条不变量,评审者在使用 Rubric 时必须同时遵守:
- 作者不自审:写代码的会话/agent 不能给自己出 verdict;评审由独立 subagent 或新会话执行。
- 评审者不动手修:评审产出 findings 和 verdict,修改由作者根据 findings 执行——裁判不下场。
- 没有记录的评审等于没有评审:每轮评审必须在
reviews/落盘记录,findings 的修复过程必须回写同一份记录(Resolution 闭环)。
检查项之一:正确性与一致性(不过 = critical)
Rubric 的第一组检查项关注"代码是否诚实":
- [ ] 实现行为与 design.md 一致;偏离处有显式说明且工件已更新(代码与设计默默不一致 = critical)
- [ ]
modify/remove的基线语义正确落地,无未批准的既有行为破坏 - [ ] 边界条件无 off-by-one / 截断 / 溢出遗漏;无逻辑上不可达却被依赖的分支
- [ ] traceability.md 每条受影响需求行的代码文件/函数、测试代码、验证证据列非空,或有明确
N/A理由;抽查 2-3 行与 diff 实际内容一致(漂移 = critical)
这里的关键判据是"默默不一致"与"漂移"。评审者抽查 traceability.md 的 2-3 行与 diff 实际内容对照,一旦发现代码与 design 不符、测试与 spec 不符,一律按 critical 处理:要么改产物,要么改工件,不允许默默不一致。这条规则在 devflow-review 的 evals.json 的 drift-is-critical 场景中被显式固化——评审者看到实现偏离设计但功能正常时,不能放行,必须要求"改代码"或"改设计并重新确认"二选一,且评审者本人不得动手改工件。
边界条件的检查技法
off-by-one、截断、溢出这类问题单靠"通读"容易漏掉,评审时应结合 测试评审 Rubric 的要求做 mutation 自检:把实现关键行改错(边界条件反转、删错误返回、删事件投递),测试必须变红。如果某个"边界条件翻转"后测试仍然全绿,说明断言存在漏洞——这正是边界问题漏网的高发地带。
检查项之二:错误路径与资源(优先细读,不过 = critical)
这是 Rubric 明确要求"优先细读"的一组检查项,也是整份清单的重心:
- [ ] 每个可失败调用被检查,或显式注明忽略理由
- [ ] 每条错误路径:已获取的资源被回收(成对获取/释放;C 集中清理出口 / C++ RAII)
- [ ] 失败后状态符合契约(回滚/保留/明确的中间态);无吞错误(捕获后既不处理也不上报)
- [ ] 错误信息带定位上下文
在 devflow-clean-code 的 quality-dimensions.md 的"可靠"维度中,这些信号被进一步展开:
| 坏信号 | 为什么重要 | 动作 |
|---|---|---|
| 可失败调用未检查 | 失败变成隐式状态污染 | 检查返回值/异常;忽略必须有理由 |
| catch 后只打日志继续 | 调用方以为成功,系统状态可能已错 | 恢复、降级、上报或返回错误 |
| 多资源获取有多个提前返回 | 新增资源时容易漏释放 | C 用集中清理出口;C++ 用 RAII |
| 错误信息无上下文 | 线上问题不可定位 | 带对象、索引、实际值、期望值 |
在 Java 项目中,这些检查项的落点非常具体:
- 资源回收对应 Java 的 try-with-resources(对应编码规范
G.PRM.07),文件、流、连接、锁都必须能从代码结构上看出释放路径; - 错误信息带定位上下文要求报错时携带对象标识、索引、实际值与期望值,而不是笼统的 "verify failed"(对应规范
G.ERR.05:抛出的异常类型要匹配方法的抽象层级,并包含足够定位失败操作的上下文); - 不吞错误意味着空的 catch、捕获后只打日志然后当没发生,都要按 critical 处理;
G.ERR.01明确禁止空 catch 块,G.ERR.02/G.ERR.06禁止直接捕获Throwable/Exception/RuntimeException并要求包装异常时保留原始 cause。
失败后状态契约的判定
"失败后状态符合契约(回滚/保留/明确的中间态)"要求评审者对照设计层的错误模型来判断:设计定了什么错误分类与传播策略、失败路径的状态保证是什么(详见 设计评审 Rubric 中的"错误模型三件事")。代码评审只负责核验编码层是否落地了这些契约,例如:事务性操作失败后是否回滚、部分成功的中间态是否有明确语义且被测试覆盖。
检查项之三:并发与生命周期(适用时)
Rubric 第三组检查项按"适用时"触发,主要面向多线程、中断上下文与指针所有权场景:
- [ ] 共享数据访问受保护;锁顺序一致;临界区精简
- [ ] 中断上下文限制遵守(无阻塞调用/动态分配;具体规则见适用的领域开发技能)
- [ ] 跨边界指针的存活期符合设计的所有权约定;无悬垂风险
在 agent-core-java 这一 Java 项目中,"中断上下文"通常对应中断处理/取消场景,具体规则参考 Java Coding Standards 的并发分组:G.CON.10 要求协作式地处理线程中断、业务代码不直接调用 Thread.interrupt(),G.CON.12 要求使用线程池而非不受控地新建线程,X.CON.06 要求显式创建 ThreadPoolExecutor 而不使用 Executors 便捷方法。锁顺序问题在存在多把锁的模块中需要对照调用关系逐一核验,避免死锁隐患。
"跨边界指针的存活期"在 Java 中的对应物是跨线程/跨异步边界的对象引用生命周期——评审时关注被传递到其他线程或异步回调中的对象是否有清晰的所有权约定,是否存在被释放/清理后仍被引用的风险。
检查项之四:整洁标准(按 devflow-clean-code)
Rubric 将整洁标准整体委托给 devflow-clean-code 技能,并列出九条明细检查项:
- [ ] 命名不撒谎:函数名说出全部行为;布尔肯定语义;量有单位
- [ ] 函数单一职责、单一抽象层;无未登记的 >50 行 / >4 参数 / >3 层嵌套
- [ ] 主路径低缩进、卫语句处理异常;无裸魔法数
- [ ] 注释解释 why 不复述 what;无注释掉的代码、无死代码、无 test-only 后门
- [ ] 重复代码:第三次出现的相同知识已提取;未提取"长得像但知识不同"的代码
- [ ] SOLID 编码落点:无上帝对象/大而全接口;无破坏契约的伪多态;无高层流程直接依赖底层细节(除非设计说明允许)
- [ ] 新增抽象有真实边界或真实变化轴;无只为 mock 或"以后可能"创建的单实现接口
- [ ] 测试代码同样可读、独立、可重复;无过度 helper 抽象、共享可变状态或 mock 内部纯逻辑
- [ ] 热路径无明显 N+1、无界读取、重复重计算或资源生命周期不清;无证据微优化不阻塞但可登记
这些规则在 devflow-clean-code SKILL.md 中有系统的展开,其中"经验阈值"被明确为审视触发线而非机械红线:函数 ≤ 50 行、参数 ≤ 4 个、嵌套 ≤ 3 层,超出即审视。命名规则强调"名字是最廉价的文档":函数动词开头并说出做什么(反例 process() → 正例 discard_expired_sessions())、布尔用 is/has/can/should 开头且保持肯定语义、带单位的量把单位写进名字(timeout → timeout_ms)。
控制流方面,Rubric 要求"主路径低缩进、卫语句处理异常"——让主路径(happy path)保持在最低缩进层级,异常分支尽早返回。以下示例来自 devflow-clean-code SKILL.md,是卫语句重构的标准演示:
/* ❌ 主逻辑埋在三层嵌套里 */
int session_send(session_t *s, const msg_t *m) {
if (s != NULL) {
if (s->state == SESSION_OPEN) {
if (msg_is_valid(m)) {
/* 真正的发送逻辑,缩进三层 */
} else { return ERR_INVALID_ARG; }
} else { return ERR_BAD_STATE; }
} else { return ERR_INVALID_ARG; }
}
/* ✅ 卫语句:前置检查依次出场,主逻辑零缩进 */
int session_send(session_t *s, const msg_t *m) {
if (s == NULL) return ERR_INVALID_ARG;
if (s->state != SESSION_OPEN) return ERR_BAD_STATE;
if (!msg_is_valid(m)) return ERR_INVALID_ARG;
/* 真正的发送逻辑 */
}
测试代码同样适用这些标准,但有一条差异:测试宁可略有重复也要可读——读一个测试不应需要跳转三层辅助函数才能知道它在验证什么(参见 test-quality.md)。测试命名格式为 <被测对象>_<场景>_<预期结果>:SetModeRejectsInvalidModeWithoutStateChange 是好名字,Test1/WorksCorrectly 是坏名字——失败时无法告诉读者哪个行为坏了。
五维质量判据速查
quality-dimensions.md 把整洁标准收敛为五维,clean_code_check 与 R3 评审至少覆盖这五维:
| 维度 | 代码应呈现 | 阻塞信号 |
|---|---|---|
| 简洁 | 当前规格需要的最少结构;抽象有真实理由 | 为"以后可能"添加插件点、单实现接口、配置矩阵 |
| 可靠 | 错误路径、资源释放、状态契约与主路径一样清楚 | 吞错误、漏检查可失败调用、失败后资源/状态不符合契约 |
| 可维护 | 名字揭示意图;职责按变化理由聚合;类型表达契约 | 命名撒谎、上帝对象、未登记的大函数/深嵌套 |
| 可测试 | 逻辑可通过公共行为验证;测试快速、独立、可重复 | 为测试加生产后门、mock 内部纯逻辑、共享可变状态 |
| 高性能 | 算法、资源与热路径成本匹配;性能取舍可解释 | 热路径 N+1、无界循环/读取、重复重计算、资源泄漏 |
blocking 清单:为什么这些"风格问题"必须按 critical 处理
Rubric 特别用一整段强调:以下整洁问题不是风格偏好,按 blocking/critical 处理:
- 命名撒谎导致调用方误解行为;
- 吞错误或错误路径状态不符合契约;
- 未登记的大函数/深嵌套掩盖多职责或错误路径;
- spec/design 未批准的新抽象、单实现接口、插件点;
- test-only 后门进入生产代码;
- 热路径中有证据的 N+1/无界资源风险。
这些问题的共同特征是:让代码"能跑但不可维护"——它们不会导致测试失败,却会让维护者(包括半年后的自己和下一轮迭代的 AI)付出长期成本。Rubric 的结论是"必须修复或回 devflow-design",评审者不允许把它们降级成 minor 风格建议。这一要求在 evals.json 的 functionally-correct-but-not-clean 场景中被验证:测试全绿但 diff 里有一个 120 行的 process() 函数、直接检查 mode == 3、吞掉错误只打日志、为测试引入单实现接口——正确行为不能成为放行理由,必须按 blocking 报告并给 需修改 或 重新设计。
对应地,Java 项目的 编码规范 提供了大量可对照的具体规则:G.MET.01(方法 ≤ 50 行、≤ 5 参数、≤ 4 层嵌套)、G.NAM.07(无否定布尔命名)、G.NAM.08(布尔以肯定谓词开头)、G.ERR.01(无空 catch)、G.ERR.07(不忽略返回值)、G.OTH.03/05(删除未使用代码与死代码)、X.TST.03(不做静态 mock,优先可注入依赖)等。
检查项之五:范围与 diff 卫生
Rubric 第五组检查项约束变更本身的形式:
- [ ] diff 只含本任务范围;行为变更与重构/格式化未混在一个提交
- [ ] 无顺手修改的无关文件;路过问题已登记而非默默修掉
- [ ] 构建无新增警告;静态分析新增项已修复或带理由抑制
这与 devflow-clean-code 的"范围与提交纪律"完全一致:一个 diff 一个目的,行为变更、重构、格式化分开提交——否则评审者无法在 500 行混合 diff 里分辨哪个变化是有意的。童子军规则的边界是:触碰范围内的小清理值得随手做;超出触碰范围、或清理本身值得独立评审的问题,登记(issue 或 plan.md 债务节)而非顺手修。
在 agent-core-java 中,这条检查项与 AGENTS.md 的工程纪律直接联动:提交前要运行 git diff --check、确认意图范围、扫描完整变更文件(而非仅新增行)检查 tab、尾随空格、通配符 import、超过 120 字符的行、TODO/FIXME、未使用 import 与死代码,并运行 mvn compile 与最窄范围的受影响测试。
检查项之六:语言与领域规则
Rubric 最后一组检查项要求评审者按 diff 涉及的语言加载对应的语言规范与领域技能:
- [ ] 适用语言的
<language>-coding-standards规则已过;按 diff 涉及的语言加载对应技能 - [ ] 命中 description 的领域开发技能已加载;适用项有实现或显式 N/A,未命中的领域有不适用理由
这里的核心纪律是"语言规范是叠加约束而非替代":加载了 <language>-coding-standards 不能替代第三层通用标准 devflow-clean-code。在 agent-core-java 中,Java 编码规范由 coding-standard.md(Huawei CodeArts Check 规则集速查)与 coding-standard-full(144 条完整规则)承载。AGENTS.md 还定义了 Java 评审时的 Rule-Loading Gate:在第一个 Java 补丁前必须完整读取基线类别文件(G.FMT → G.NAM → G.DCL → G.MET → G.CTL → G.EXP → G.ERR → G.CMT → G.OTH),再按场景分组加载(对象/集合/并发/日志/IO/序列化/安全等),评审完成的 diff 时按"结构声明 → 控制流与表达式 → 空值与异常契约 → 命名格式 → 场景分组"的固定顺序复查。
此外 AGENTS.md 提供了 Recurring CodeArts Regression Checklist——历史上反复出现云上检查项的强制复查清单,包括:G.MET.01 的方法规模限制(提取 helper 后要重新计算)、G.DCL.02 局部变量声明到首次使用距离不超过 10 行、G.MET.05/06 空容器与 Optional 的使用、G.ERR.02/06 捕获与 cause 保留、G.CON.10 协作式取消、G.NAM.08 布尔命名、G.FMT.04 成员声明顺序、G.CTL.02 else-if 链收尾、G.EXP.03 三目运算符类型一致、G.ERR.05 异常抽象层级、G.FMT 系列格式规则。评审者可以把它作为 Java diff 语言规则检查的本地速查表。
Verdict 指引:需修改、重新设计还是放行
Rubric 收尾给出了 verdict 判定的三条主干规则:
- 正确性/错误路径/一致性 critical →
需修改 - blocking/critical 整洁问题 →
需修改;若反映模块边界或抽象方向错误,verdict重新设计 - 结构性问题超出任务边界(模块划分错误、接口契约缺陷)→
重新设计指向devflow-design - 纯格式、局部命名偏好等 minor 不阻塞,列出由人决定取舍;不要把上面列出的 blocking 问题降级成 minor
在 devflow-review SKILL.md 中,verdict 三选一被完整定义为:
| verdict | 含义 | 处理 |
|---|---|---|
通过 |
无 critical/important,或仅剩已被人接受的 minor | 进入下一阶段 |
需修改 |
findings 可定向修复 | 回作者阶段修复,回填 Resolution 后复审 |
重新设计 |
问题出在上游(规格漏洞、设计方向错误) | 打回 devflow-specify / devflow-design |
findings 的严重级与分类
每条 finding 必须包含:位置 + 问题 + 为什么是问题 + 严重级 + 分类 + 建议返工阶段。严重级三档:
critical:不修不能继续——会导致做错事、留 bug 或不可审(例如错误路径资源泄漏);important:完成前应修(例如边界用例缺失、命名误导);minor:建议改进(措辞、风格微调)。
分类三档决定"要不要问人":
LLM-FIXABLE:信息已足够,作者可按 finding 定向修复,不问人;USER-INPUT:缺业务事实、优先级、验收阈值、外部来源确认——只问 finding 指向的最小问题;TEAM-EXPERT:需要模块架构师、资深工程师或团队规则裁决——把问题封装成 1-2 个具体决策点上抛。
建议返工阶段按问题本质填写:规格问题回 devflow-specify,设计契约/错误模型/组件边界问题回 devflow-design,R3 中的测试断言、RED 证据、实现 bug、代码整洁问题回 devflow-tdd。R3 的 需修改 默认回 devflow-tdd——测试弱就先补强或重写会失败的测试,代码问题用 RED/GREEN/REFACTOR 或纯 REFACTOR 修复;只有 finding 明确证明规格或设计工件本身错误,才回更上游阶段(此规则在 evals.json 的 r3-rework-target-is-tdd 场景中显式验证)。
自动返工循环的上限
同一 R 节点最多自动返工复审 3 轮。第 3 轮仍有未闭环 critical/important,或持续出现新的同级问题,停止自动循环,把剩余问题、已修证据和需要人裁决的具体问题呈给人(three-round-review-loop-cap 场景)。超过 3 轮仍打回,说明问题不再是局部修复能解决的,需要升级到方向裁决。
从 Rubric 到完整流程:一轮 R3 代码评审怎么落地
Rubric 本身是检查清单,把它放进 devflow-review SKILL.md 的工作流中,一轮完整的代码评审包含六个动作:
- 确定目标与 rubric:代码评审使用
references/code-review-rubric.md+devflow-clean-code,关注核心是"正确性、与设计一致、整洁标准、语言/领域规则"。 - 以独立上下文执行:派发
devflow-reviewersubagent(agent name:devflow-reviewer),输入只给被评审产物、上游工件(design + diff)、对应 rubric、devflow-clean-code、适用的 coding-standards / 领域技能。不给作者的推理过程和聊天历史。 - 产出 findings 与 verdict:按 Rubric 逐项过,不凭整体印象打分;rubric 之外发现的问题照样列出。
- 落盘评审记录(必做):记录写入组件根下
features/<id>/reviews/<目标>-review-<日期>.md,同一目标的复审追加轮次后缀(-r2、-r3)。每份记录包含评审对象(含版本/commit)、findings 表(含 Resolution 列、分类、建议返工阶段)、verdict、抽查记录(如做了 mutation 自检,写明改了哪行、哪个测试红了)。 - Findings 闭环(作者侧职责):verdict 为
需修改/重新设计时,作者按 findings 返工并逐条回写原评审记录的 Resolution 列——修复了(怎么改的 + commit 锚点)、人接受不修(理由 + 谁接受的)、升级为债务(登记去向)。全部 critical/important 有 resolution 后才能复审。 - 人工确认(按运行模式):
attended(默认)下把评审记录与 verdict 呈给人,人同意后才进入下一阶段;unattended下不停顿,但独立评审、落盘记录、critical 阻塞返工与复审照常执行。
评审者纪律速查
- 按 rubric 逐项过,不凭整体印象打分;
- 每条 critical/important finding 给出具体位置和可执行的修复方向,不写"质量有待提高";
- 抽查重于通读:测试评审必做 2-3 个关键用例的 mutation 自检;代码评审优先读错误路径与资源路径;
- 不确定的判断标注"待人裁决",不假装确定;
- 发现产物间漂移 → 一律 critical。
风险信号:评审走过场的识别
- 作者会话自己宣布"评审通过";
- 声称评审完成但
reviews/没有对应记录文件(= 未评审); - findings 修复后没有回写 Resolution,复审记录里问题"凭空消失";
- findings 全是 minor 措辞建议,对错误路径、断言强度、契约完整性只字不提(评审走过场);
- verdict 为"需修改"但 findings 没有一条具体到位置;
- 评审者直接动手改了代码。
与上下游工件的衔接:Rubric 不是孤立的清单
代码评审 Rubric 是 DevFlow 四份评审 Rubric 之一,与其他环节构成闭环:
| 文件 | 用途 |
|---|---|
| spec-review-rubric.md | 规格评审检查项(可测试性、变更风险显式、边界纪律) |
| design-review-rubric.md | 设计评审检查项(契约完整、结构与复杂度、方案取舍、测试设计) |
| test-review-rubric.md | 测试评审检查项(覆盖映射、断言强度、TDD 证据、mock 纪律) |
| code-review-rubric.md(本文主体) | 代码评审检查项(正确性、错误路径、并发、整洁、范围、语言规则) |
评审记录最终被 Definition of Done 消费:工作项关闭前,R1/R2/R3 全部评审记录必须存在于 reviews/,最终 verdict 为通过,findings 全部有 Resolution;attended 模式下各门禁均有人工确认记录。Resolution 列有空着的 critical/important,门禁不算通过——这是从 Rubric 检查项通向最终交付的强制闭环。
总结:用怀疑者的视角读每一行 diff
代码评审 Rubric 的全部检查项可以浓缩为三句话:
- 先读错误路径与资源路径——那是代码最可能"骗你"的地方;
- 按六组检查项逐项过——正确性与一致性、错误路径与资源、并发与生命周期、整洁标准、范围与 diff 卫生、语言与领域规则;
- blocking 问题绝不降级——命名撒谎、吞错误、投机抽象、test-only 后门、混合 diff,这些让代码"能跑但不可审"的问题必须修复、登记债务或升级设计裁决,verdict 落在
需修改/重新设计,并用落盘记录 + Resolution 闭环保证每一轮评审都可追溯。
在 agent-core-java 这样的 Java 工程中,把 Rubric 与 AGENTS.md、Java Coding Standards、coding-standard-full 的规则加载门禁与回归清单配合使用,就能把这份通用评审标准落成一份真正可执行的 Java 代码评审检查表。