---
name: vibe-review
description: "在涉及代码审查的任何场景下使用此skill。包括：审查单个文件、审查PR/MR的变更、检查编码规范合规性、查找安全漏洞或内存问题。当用户提到'code review'、'CR'、'review'、'代码审查'、'代码检视'、'编码规范检查'、'规范'，或要求检查代码质量时触发。支持C++和Python代码审查。"
argument-hint: "[file|PR-URL|git-range]"
allowed-tools: Read, Grep, Glob, Bash(git diff*), Bash(git log*), Bash(git show*), Bash(git remote*), Bash(wc *)
---

# 代码审查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/cann/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）
   - 排除：已在调用链上游保证非空的参数透传（需工具验证调用链）
3. 数组越界：索引是否有边界保护？（红线1.2）
4. 除零保护：除数是否可能为零？（红线1.1）
5. 整数溢出/翻转：加减乘结果是否可能溢出？（红线1.3）
   - 排除：值域已证明不可能溢出的场景（需工具验证值域约束）
6. 变量未初始化：所有分支上是否先初始化再使用？（红线1.4）
7. 资源泄漏：所有路径（含异常）上是否正确释放？（红线1.6）
8. 禁用函数：是否用了memcpy/sprintf/strcpy等而非`_s`安全替代？（规则2.18.1）
9. 并发安全：共享数据是否有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 中的已确认误报模式

2. 工具验证。对每个反证条件，用Read/Grep/git show尝试验证，不要仅凭推测。

3. 判定：
   - 反证条件经工具验证成立 → 抑制该发现
   - 部分成立或无法判断 → 保持"待确认"，在报告中标注未能否证的原因
   - 反证条件不成立 → 升级为"较确定"，附验证依据（如"已确认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:line`或`file: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 show`或`Read`工具重新定位

---

## 置信度

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

- 确定：机械匹配可判定（禁用函数、命名违规、缺大括号、未检查返回值）
- 较确定：已通过工具验证且经过约束反证（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:`
- `== None` → `is None`
- `type(x) == int` → `isinstance(x, int)`