vibe-review

SKILL.md

代码审查Skill

审查目标

$ARGUMENTS

参数解析:

  • 文件路径(如src/foo.cpp)→ 单文件审查
  • git range(如HEAD~3..HEAD)→ git diff <range>获取变更
  • PR/MR URL → 通过平台工具获取diff
  • 无参数 → 询问用户要审查什么

当前环境

  • 仓库: !git remote -v 2>/dev/null | head -1 || echo "NOT_A_GIT_REPO"
  • 工作目录: !pwd

参考标准

根据上方仓库信息判断仓库类型,按下表加载标准文件。

始终加载

文件 内容
references/standards-company.md 华为编码规范
references/standards-dept.md 算子编码红线及TOPN
references/standards-personal.md 个人审查习惯

按仓库加载

仓库 额外加载
hccl, hcomm standards-productline-cann-cpp.md + standards-project-hccl.md
ops-transformer standards-productline-cann-cpp.md + standards-project-ops-transformer.md
ops-nn standards-productline-cann-cpp.md + standards-project-ops-nn.md
CANN其他仓 standards-productline-cann-cpp.md
非CANN (无额外标准)

领域知识

审查hccl/hcomm仓库时,若~/repo/me/docs/hccl/目录存在,可按需查阅其中的文档辅助理解业务语义:

文件 内容
user-guide.md HCCL使用指南,理解集合通信整体流程
api-c.md / api-cpp.md / api-python.md HCCL对外API规格,校验接口用法是否符合契约
ascendc-hccl.md AscendC集成HCCL的方式,理解算子与通信的交互
env-vars.md 环境变量定义,校验环境变量使用是否正确
faq.md 常见问题,识别已知陷阱
migration.md 版本迁移指南,理解API演进和废弃路径

使用方式:当审查中遇到不确定的API语义、通信流程、环境变量用途时,用Grep搜索相关关键词。不要主动全量加载这些文件。

按需查阅

文件 何时查阅
references/google-cpp-style-guide.md C++审查需查阅规则细节时(文件6000+行,优先用Grep搜索具体规则号)
references/false-positives.md 报出发现前查阅,排除已知误报模式

1. 理解变更上下文

此步骤是AI审查的核心优势——充分利用对整个代码库的访问能力。

PR/MR审查:

  1. 通读所有diff,理解MR整体意图(新特性?bug修复?重构?)
  2. 对每个变更的函数/类,主动读取:调用者(grep函数名)、所属类头文件、基类/派生类、同模块文件
  3. 识别遗漏:基于MR意图,检查是否有应改而未改的地方

单文件审查:读取完整文件+关键依赖文件+grep可疑函数的其他用法。

2. 工具验证(质量门槛)

硬性规则:未经工具验证的发现只能标注"待确认",且仅严重级别时才报出。

以下场景必须使用工具确认,不要仅凭diff猜测:

  • 指针赋值/传递:读函数签名,确认值传递vs引用(值传递形参地址返回后失效)
  • 算术运算:读变量声明,确认类型和值域(uint32_t*1000可能溢出)
  • 结构体成员增删:grep成员名,检查所有引用点是否同步修改
  • sizeof():确认操作数类型(容器对象≠数据大小)
  • 函数返回指针:读实现,确认返回null的路径
  • 跨文件遗漏:grep被修改/删除的标识符,检查其他文件中的引用

通过工具验证可将"待确认"升级为"较确定"。不要因节省工具调用而跳过验证。

3. 逐条过筛必检规则

C++必检规则

对diff中每个格式化调用、每个指针操作、每个数组访问、每个算术运算逐条检查。命中即为【严重】,不可降级:

  1. 格式字符串参数匹配:%说明符的个数和类型是否与实参一一对应?(规则3.1.3)
  2. 空指针解引用:指针解引用前是否有非空检查?(红线1.5)
  • 排除:已在调用链上游保证非空的参数透传(需工具验证调用链)
  1. 数组越界:索引是否有边界保护?(红线1.2)
  2. 除零保护:除数是否可能为零?(红线1.1)
  3. 整数溢出/翻转:加减乘结果是否可能溢出?(红线1.3)
  • 排除:值域已证明不可能溢出的场景(需工具验证值域约束)
  1. 变量未初始化:所有分支上是否先初始化再使用?(红线1.4)
  2. 资源泄漏:所有路径(含异常)上是否正确释放?(红线1.6)
  3. 禁用函数:是否用了memcpy/sprintf/strcpy等而非_s安全替代?(规则2.18.1)
  4. 并发安全:共享数据是否有data race?(红线1.7)

Python必检规则

对diff中每个函数、每个资源操作、每个外部数据处理逐条检查。命中即为【严重】,不可降级:

  1. 类型混淆:None返回值是否被当作有效对象使用?Optional类型是否先检查再访问?
  2. 异常吞没:except Exception/except:是否隐藏了关键错误?异常处理是否过于宽泛?
  3. 资源泄漏:文件/连接/锁是否使用with语句或try/finally确保释放?
  4. 可变默认参数:函数默认参数是否使用了list/dict/set等可变对象?
  5. 注入风险:是否用eval()/exec()/os.system()处理外部输入?SQL是否用参数化查询?
  6. 路径遍历:外部输入的文件路径是否做了规范化和白名单校验?
  7. 并发安全:多线程共享数据是否有竞争条件?GIL不保护跨语句的原子性
  8. 除零保护:除数是否可能为零?
  9. 索引越界:列表/元组索引是否有边界保护?字典key是否可能不存在?

4. 分层检查其余规则

C++

【严重】 — 必检规则命中 + 内存分配后未判空(2.16.1)+ 安全函数返回值未检(2.18.6)+ 外部数据入exec/dlopen(2.18.7-8)+ 路径未规范化(2.17.1)+ 内存安全:UAF/悬垂指针/泄漏(3.1.2)+ 敏感信息未清零(2.15.12)+ format受外部控制(3.5.5)

【一般】 — 命名违规(1.1.x)+ typedef→using(2.3.2)+ C头文件(2.2.1)+ C风格转换(2.7.1)+ 只读形参缺const(2.10.6)+ explicit缺失(2.15.5)+ 裸new/delete(2.10.4)+ 持有c_str()指针(2.10.1)+ 缺大括号(1.2.4-5)+ catch(...)(2.1.4.1)+ switch缺default(2.8.1)+ 虚函数缺override(2.13.3)+ lambda按引用捕获局部变量(2.14.2)+ 拷贝/移动未成对(2.15.6)+ delete/move/swap缺noexcept(3.2.1)

【建议】 — 注释风格(1.3.x)+ TODO/FIXME(1.3.3)+ 魔鬼数字(2.4.2)+ 冗余代码(2.1.3)

Python

【严重】 — 必检规则命中 + 敏感信息硬编码(密码/密钥/token写在代码中)+ pickle/yaml.load反序列化不可信数据 + subprocess.shell=True拼接外部输入

【一般】 — 裸except:未re-raise + 未使用logging而用print做日志 + 全局可变状态 + 函数超过50行 + 嵌套超过4层 + 未使用类型注解(公共API)+ import * + 循环导入

【建议】 — PEP 8风格违规 + 魔鬼数字/字符串 + 冗余代码 + TODO/FIXME + 缺少docstring(公共API)

4.5约束反证

对每个非"确定"置信度的发现,输出前执行反证检验。

核心:不要只确认"这像是个bug",要反过来问——在什么条件下这个问题不成立?然后用工具验证这些条件是否成立。

步骤:

  1. 列出反证条件。按bug类别,典型的"若成立则为误报"条件:
  • 空指针解引用:调用链上游已判空(grep调用者);构造函数/工厂保证非空;值域约束排除null
  • 整数溢出:操作数值域受业务约束(枚举值、小常量、循环计数器);类型提升后安全
  • 资源泄漏:RAII/智能指针/自定义Guard管理;析构函数自动释放;调用者契约负责释放
  • 数组越界:索引值域受循环条件/前置校验约束;容器有边界保护
  • 并发问题:变量实际仅单线程访问(grep所有引用点);已有锁/原子操作/屏障保护
  • 逻辑缺陷:隐含前置条件使问题路径不可达;防御性编程已覆盖该场景
  • 参考references/false-positives.md中的已确认误报模式
  1. 工具验证。对每个反证条件,用Read/Grep/git show尝试验证,不要仅凭推测。

  2. 判定:

  • 反证条件经工具验证成立 → 抑制该发现
  • 部分成立或无法判断 → 保持"待确认",在报告中标注未能否证的原因
  • 反证条件不成立 → 升级为"较确定",附验证依据(如"已确认CreateSocket()失败时返回nullptr,见socket.cpp:78,调用链无判空")

注意:严重级别的发现即使反证不彻底也应报出(标"待确认"),宁多勿漏。此步骤目的是减少误报,不是放过bug。

5. 输出格式

禁止使用表格展示发现。逐条块状输出,按严重级别降序,---分隔。"待确认"集中放末尾。无发现直接输出"未发现问题",不凑数。

## 变更概述

本MR为[模块名]实现了[功能/修复],主要变更:
- file_a.cpp: [变更要点]
涉及N个文件,M处新增/修改。

## 审查发现

共发现N个问题(严重x / 一般y / 建议z)

---

### #1 [严重] 使用了禁用函数memcpy
- 位置:`src/platform/transport/tcp_transport.cpp:142`
- 规则:2.18.1
- 置信度:确定

问题代码:
```cpp
memcpy(dst, src, len);
```

修复建议:
```cpp
errno_t ret = memcpy_s(dst, dstMax, src, len);
CHK_SAFETY_FUNC_RET(ret);
```

---

### #2 [严重] Destroy持锁期间执行阻塞同步操作
- 位置:`src/channel/hcomm_channel.cpp:245-250`
- 规则:CON-04
- 置信度:较确定
- 反证:已检查是否有异步替代路径或锁外执行的可能。确认`hcclStreamSynchronize`必须在`channelMutex_`持锁期间调用(因为stream句柄在锁保护范围内),且无超时参数,阻塞时间取决于设备负载(实测可达60秒)。反证条件不成立。

问题代码:
```cpp
std::lock_guard<std::mutex> lock(channelMutex_);
// ... 持锁期间
hcclStreamSynchronize(stream);  // 可阻塞60s+
```

修复建议:
将stream同步移到锁外,或使用带超时的同步API。

---

## 总结

[1-2句总体评价]
建议优先处理x个严重问题,其中N个确定,M个待确认。

输出规则:

  • 每个发现必须含:位置(file:linefile:start-end)、规则编号、置信度、问题代码(只含问题行)、修复建议。"较确定"/"待确认"的发现还必须含- 反证:字段(见4.5节和输出格式示例)。连续多行问题用范围格式file:199-201,不连续用逗号file:199, 210
  • 代码片段用围栏代码块(C++用```cpp,Python用```python,支持语法高亮)
  • 报告即终止:报告以"## 总结"段落结尾。之后不要追加任何客套话、后续提示或对话性文字(如"如果需要更深入分析请告诉我")。报告本身就是最终输出。

5.5输出自检清单

输出报告前逐项检查,不合格则修正后再输出:

  1. 发现标题是否为 ### #N [等级] 描述 格式?(禁止 ### 发现 N: 等变体)
  2. 等级标签是否为 [严重]/[一般]/[建议] 之一?
  3. 置信度是否为"确定"/"较确定"/"待确认"之一?(禁止"高"/"中"/"低")
  4. 规则字段是否引用了具体规范编号(如2.18.1、红线1.5)而非自由描述(如"类型安全")?
  5. 属性表格是否包含"发现"汇总行(| 发现 | 严重x / 一般y / 建议z |)?
  6. "## 审查发现"下方是否有 共发现N个问题(严重x / 一般y / 建议z) 汇总行?
  7. 行号是否经过验证?(见下方行号自检)
  8. 每个"较确定"/"待确认"发现是否经过约束反证(4.5节)?若未执行反证,回退执行后再输出。

5.6行号自检

输出报告前,对每个发现的行号执行自检:

  1. 对照diff的@@ +行号 @@ hunk头验证行号正确性
  2. hunk起始行号是第一个上下文行,不是第一个+
  3. 禁止猜测行号——如无法确认,用git showRead工具重新定位

置信度

每个发现必须标注置信度。只允许以下三个词,禁止使用"高/中/低"等替代表述:

  • 确定:机械匹配可判定(禁用函数、命名违规、缺大括号、未检查返回值)
  • 较确定:已通过工具验证且经过约束反证(4.5节)。在输出中用- 反证:字段标注尝试否证的过程和结论,如:"已检查调用链上游无判空保护(grep CreateSocket调用点共3处,均未检查返回值),反证条件不成立"
  • 待确认:已尝试约束反证但无法判定。用- 反证:字段标注未能否证的原因和需人工确认的具体问题

过滤规则:不报告置信度低于"待确认"的发现。优先用工具提升置信度。宁多勿漏:宁可多报"待确认"的严重/一般问题让人工判断,也不漏掉真实缺陷。建议级别低置信度可省略。


C++命名规范速查

类型 风格 示例
类/结构体/枚举/命名空间 大驼峰 UrlTable, FileUtils
函数 大驼峰(动宾) AddElement, GetValue
局部变量/参数 小驼峰 tableName, bufferSize
类成员变量 小驼峰+_ fileName_, isReady_
全局变量 g_+小驼峰 g_activeConnectCount
宏/枚举值/全局const 全大写下划线 MAX_SIZE
文件名 小写下划线 url_table.cpp

C++禁用函数速查

memcpy/bcopy→memcpy_s, memmove→memmove_s, memset→memset_s, strcpy/strncpy→strcpy_s/strncpy_s, strcat/strncat→strcat_s/strncat_s, sprintf/snprintf→sprintf_s/snprintf_s, vsprintf/vsnprintf→vsprintf_s/vsnprintf_s, scanf/sscanf/fscanf→scanf_s/sscanf_s/fscanf_s, gets→gets_s

Python PEP 8速查

类型 风格 示例
模块/包 小写下划线 my_module, utils
大驼峰 MyClass, HTTPServer
函数/方法 小写下划线 get_value, calculate_sum
变量 小写下划线 total_count, file_path
常量 全大写下划线 MAX_RETRY, DEFAULT_TIMEOUT
私有属性 前置下划线 _internal_state
名称修饰 双前置下划线 __private_method

Python常见反模式速查:

  • except:/except Exception: → 指定具体异常类型
  • def f(x=[]):def f(x=None): x = x or []
  • eval(user_input) → 使用ast.literal_eval或白名单解析
  • os.system(cmd)subprocess.run([...], shell=False)
  • open(f)无close → with open(f) as fh:
  • == Noneis None
  • type(x) == intisinstance(x, int)
Weekly Installs
7
First Seen
5 days ago
Installed on
opencode7
gemini-cli7
github-copilot7
codex7
kimi-cli7
cursor7