icloudsheep code-review
——提交前的最后一道关。目标不是挑刺，而是在改动进入 git、进入他人视野之前，先把会腐化、会误导、会刷屏、会埋雷的东西拦下来。
本 skill 在「提交前」运行，处在几个 skill 的交汇点：
- 下游调用
code-comment：审查注释时，直接以 code-comment 的规范为准则。- 被
git-commit触发：写 commit / 推送前先跑一轮 review，再进入提交流程。- 与
ai-log衔接：review 结束后若用户要「记录日志」，本次发现的问题与处置可作为日志素材（见末节）。
一、审查范围
默认审查本次提交的 diff。
先看工作区与暂存区改动：
git status --short git diff # 未暂存 git diff --cached # 已暂存若本分支近期存在多条提交，主动询问用户：是否需要把范围扩到本分支相对主干的全部 diff（而非仅最后一次）。用户确认后再查：
git log --oneline <主干>..HEAD # 看本分支提交序列 git diff <主干>...HEAD # 本分支相对主干的累计 diff聚焦本次改动引入的问题，不发散去重写存量代码；存量问题顺手发现可提示，但不强行纳入本次。
二、逐项检查清单
按以下维度过一遍 diff。每条都对照「本次改动」判断，不替用户做无关重构。
1. 拼写
- 标识符、注释、日志文案、用户可见文案里的拼写错误（英文单词拼错、中文别字）。
- 命名是否词义准确（
recieve→receive、lenght→length这类高频错拼）。 - 对外文案 / 错误提示的拼写与措辞是否得体。
2. log 合理性（重点，详见第三节）
- 每一条 log 都要问「这里到底需不需要打」。
- 区分 B 端关键节点 vs C 端用户路径。
- 杜绝刷屏式无效日志。
3. 注释（下游调用 code-comment）
- 审查注释时以
code-commentskill 为准则：不向上溯源调用链、不向下探索消费方、不横向引用其他方法、阶段性现状带[注: 截至 YYYY-MM-DD]、不写硬编码行号、不脑补业务。 - 同时鼓励：复杂逻辑 / 关键取舍 / 边界坑点是否缺了该有的「为什么」注释。
- 发现注释问题时，按 code-comment 的正确写法给出修订建议。
4. commit message 格式（衔接 git-commit）
- 检查本次（及范围内历次）commit message 是否符合
git-commitskill：type: subject结构、type 选得对、subject 一句话无句号、body 说清「为什么」、多行 message 是否被 shell 截断。 - 无意义 message（
update、fix bug、0、tmp）一律打回。
5. 参数校验与边界
- 入参校验是否齐全：必填项判空、长度上限、数值区间、格式合法性（如「逐项 check 不通过即抛 BAD_REQUEST」的写法，见第三节正例）。
- 边界与异常：空集合 / null / 越界 / 除零 / 数值溢出 / 代理对（emoji 等补充平面字符按 code point 处理，不要按 char 拆开）。
- 异常处理：catch 块是否吞异常、是否丢失上下文、是否该向上抛而被静默。
- 资源与并发：流 / 连接是否关闭、共享可变状态（如复用的
StringBuilder）是否有内容污染或线程安全隐患、循环内是否重复 new 大对象。
6. 其余常被忽视的点
- 空指针链：
a.getB().getC()式链式调用中途为 null。 - 集合误用：在遍历中修改集合、
contains在大集合上的性能、用==比较包装类型 / 字符串。 - 魔法值：散落的字面量数字 / 字符串是否该提为常量或枚举。
- 重复代码：本次新增是否与既有工具方法重复（可复用而未复用）。
- 幂等与重试：写操作在重试 / 重复消费下是否幂等。
- 时间 / 时区 / 精度：金额用
BigDecimal而非double，时间处理时区是否明确。 - 死代码 / 调试残留：
System.out.println、临时log.info("aaa")、注释掉的整段代码、TODO是否该清理。 - 测试：本次改动是否有对应测试，关键分支 / 边界是否覆盖（测试注释同样走 code-comment）。
7. 测试桩与框架回调异味
审查自定义测试桩（如 gRPC Channel / ClientCall）时，先区分静态规则问题与运行时契约：
- 泛型参数命名：项目若限制为
^[A-Z][0-9]?$，将ReqT、RespT等方法级类型参数改为R、S，并在同一签名和方法体内一致替换。重写方法不要求沿用父类的类型参数名，因此不要为消除命名告警改变方法签名语义。 - 空回调方法：静态检查通常无法识别测试桩中的有意 no-op。先确认框架是否会在正常生命周期调用该方法；若会调用但测试无需模拟对应状态或流控，保留空实现并在方法体内说明不模拟的原因。
- 不要机械抛异常：仅当该操作确实不受支持、且不属于被测调用生命周期时，才使用
UnsupportedOperationException。对request、cancel等可能由框架主动调用的方法机械抛异常，会使测试桩偏离真实调用契约，并可能掩盖原始失败。 - 需要观测时再实现：测试断言依赖该回调时，实现最小状态记录或请求捕获；不依赖时不要补充无意义逻辑。
- 控制修改范围：只处理本次告警涉及的测试桩，不因全仓存在相似写法而顺带批量重构。修改后至少运行定向测试和对应模块的测试编译；项目规范另有要求时再补模块编译。
三、log 使用规范（重点专项）
日志是高频被滥用的地方：该打的不打、不该打的狂打。每一条 log 都要单独做合理性审查——「这个环节到底有没有必要 log」。
核心判据：分清 B 端与 C 端
- 面向 B 端（后台 / 运营 / 内部管理 / 关键业务链路）：关键节点可以打日志，但必须合理、合规、可用：
- 打在有排查价值的关键节点（外部调用入口出口、状态流转、异常分支、重要决策点），而非每个分支都打。
- 日志要可用：带上能定位问题的关键上下文（业务主键、操作类型、结果），并注意是否需要
trace_id串联全链路。 - 合规：不把敏感信息（密钥、用户隐私）或整个大对象塞进日志。
- 面向 C 端（终端用户请求路径，尤其是参数校验失败）：无需日志，直接返回错误即可。校验不通过就抛业务异常 / 返回错误码，不要为每次用户输入错误打一行 log——这类日志量大、无排查价值、纯刷屏。
反例（坚决避免）
public static boolean containsEmoji(String value, boolean strict) {
if (StringUtils.isBlank(value)) {
return false;
}
// ... 按 code point 遍历匹配 ...
for (int start = 0; start < codePoints.length; start++) {
for (int end = start; end < endLimit; end++) {
if (emojiData.emojiSet.contains(candidate.toString())) {
log.info("EmojiUtil#containsEmoji >>> 找到Emoji:{}", candidate); // 命中即打，热点路径刷屏
return true;
}
}
}
log.info("EmojiUtil#containsEmoji >>> 未找到Emoji:{}", value); // ← 完全不必要，会严重滥用 log
return false;
}
未找到Emoji这一行完全不必要：这是一个纯工具方法的常规返回路径，会被高频调用，每次都打一行属于典型的 log 滥用，坚决删除。找到Emoji同样要审视：工具方法内部命中与否不是「关键业务节点」，没有排查价值就不该打；真要观测，交给调用方在业务节点决定。
正例（C 端校验失败：不 log，直接抛）
if (StringUtils.isBlank(request.getName()) || request.getId() == null
|| request.getId() <= 0L || StringUtils.isBlank(request.getProducer())
/* ... 其余必填项 ... */) {
throw new ServerException(ServerCode.BAD_REQUEST.getCode(), "必要参数为空");
}
if (request.getName().length() > 256 || request.getOverview().length() > 2048) {
throw new ServerException(ServerCode.BAD_REQUEST.getCode(), "输入信息无效");
}
if (request.getRuntime() <= 0 || request.getRuntime() >= 20) {
throw new ServerException(ServerCode.BAD_REQUEST.getCode(), "单集平均时长不可大于20");
}
校验失败直接抛业务异常返回错误码，不打日志。
log 审查小结
逐条 log 自问：
- 这是 B 端关键节点，还是 C 端用户路径？C 端校验失败不该打 log。
- 这条 log 有没有排查价值？常规返回 / 热点工具方法内部不要打。
- 级别对不对？常规流水用
info还是debug？异常才用warn/error。 - 关键链路是否需要
trace_id串联？ - 是否打了敏感信息或整个大对象？
- 是否在循环 / 高频路径里打，会不会刷屏？
四、CI 代码覆盖率检查（询问后执行）
代码测试覆盖率是提交前值得一并确认的质量指标，但是否检查、目标定多少，由用户决定，不擅自跑。
触发方式
在完成逐项 diff 审查后，主动询问用户：
- 本次是否需要查询 CI 代码覆盖率？
- 若需要，覆盖率目标是多少？分两类目标，格式为「全局覆盖率 / diff 覆盖率」：
- 全局覆盖率：整个模块 / 包的累计覆盖率目标（如
0表示不卡全局）。 - diff 覆盖率：本次改动新增 / 修改代码的覆盖率目标（如
70表示本次 diff 需覆盖 70%）。
- 全局覆盖率：整个模块 / 包的累计覆盖率目标（如
执行方式
用户确认需要检查后，使用项目提供的覆盖率脚本执行，典型形式：
# 用法：check-coverage.sh <检查路径> <全局覆盖率目标>/<diff覆盖率目标>
bash check-coverage.sh app/hello/world 0/70
- 检查路径：用户指定的模块 / 包路径（如
app/hello/world）。若用户未给，询问或按本次 diff 涉及的主要模块推断并与用户确认。 - 目标参数：按用户给的「全局/ diff」两个数字拼接（如
0/70表示全局不卡、diff 需 70%）。 - 脚本名 / 路径以项目实际为准（可能是
check-coverage.sh或项目内类似封装）；先确认脚本存在再执行。
反馈方式
拿到脚本输出后，向用户清晰反馈：
- 实际全局覆盖率、实际 diff 覆盖率分别是多少。
- 是否达到用户设定的目标；未达标则指出本次 diff 中哪些新增 / 改动代码未被覆盖，并建议补哪些测试（测试注释走 code-comment）。
- 达标则明确告知通过，可继续提交流程。
五、审查输出与处置
- 分级呈现：按「必须改（阻断提交）/ 建议改 / 可选优化」分组，每条给出文件 + 方法/字段引用（不写行号）、问题、建议改法。
- 能改则改：注释、拼写、删除冗余 log 这类明确问题，可直接给出修订；涉及逻辑取舍的，说明清楚交用户定夺。
- 改完复检：修订后重新过一遍 diff，确认问题闭环、未引入新问题；涉及代码改动的按项目方式构建 / 跑相关测试（如 Gradle 项目跑对应模块测试）。
六、与 git-commit / ai-log 的衔接
跨 skill 调用需先征得用户同意：下面的联动是「允许」而非「自动」。本 skill 运行中若要触发另一个 skill（如审查后顺手按 git-commit 提交、或调 ai-log 记录），必须先向用户说明要调用哪个 skill、做什么，获得明确许可后再执行；未获许可则只完成本次审查职责。
- 被 git-commit 触发：进入提交流程前，先完成本 skill 的检查清单；review 通过后，经用户同意再按
git-commit规范写 message、精确暂存、提交。 - 下游 code-comment：注释相关的所有判定与改法，统一以
code-commentskill 为准（读取其规范作为准则，属轻量引用，仍建议在动手改注释前知会用户）。 - 配合 ai-log：若用户在 review 后要「记录日志」，本次审查的发现（拦下了哪些 log 滥用 / 注释腐化 / 边界问题、做了哪些修订）是高质量的日志素材，可在 ai-log 正文里结构化记录。
七、提交前总检查清单
- 拼写无误（标识符、注释、日志、对外文案）
- 每条 log 都过了合理性审查：C 端不打、热点/常规返回不打、B 端关键节点合理打且按需带 trace_id
- 注释满足 code-comment：不越界、带时间戳、无行号、不脑补，且该补的「为什么」已补
- commit message 符合 git-commit：type 正确、一句话 subject、body 说清为什么、未被截断
- 参数校验齐全（判空 / 长度 / 区间 / 格式），边界与异常处理到位
- 无空指针链、集合误用、魔法值、重复代码、调试残留
- 关键改动有测试覆盖
- 已询问用户是否检查 CI 覆盖率；如需要，按「全局/diff」目标跑覆盖率脚本并反馈结果
- 修订后已复检并构建 / 跑相关测试通过