# Code Review Excellence

> 为 TypeScript 与 C#/.NET 提供高信号代码审查指导。用于 PR review、review diff、代码走查、变更后复核、建立审查标准，以及检查 bug、行为回归、边界条件、类型安全、异步问题、资源管理、性能陷阱、EF Core、ASP.NET Core、LINQ 和缺失测试。适用于 TypeScript/JavaScript、Node/Web、C#/.NET 8、ASP.NET Core、EF Core 场景。按改动特征读取 `references/typescript.md`、`references/csharp.md`、`references/testing.md` 与 `references/review-checklist.md`。对纯格式化、机械改名、纯文档、生成代码和第三方目录改动不默认触发。

- Skill: `zpqq132555/code-review-excellence` (Agent Skill, multi-file: 7 files)
- Install (CLI): `npx skillmds@latest add zpqq132555/code-review-excellence`
- Raw SKILL.md: https://api.skillmd.com/api/skills/zpqq132555/code-review-excellence/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: zpqq132555 (https://skillmd.com/u/zpqq132555)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/zpqq132555/code-review-excellence

---


# 代码审查实践

代码审查的首要目标是发现真实风险并帮助改进，而不是进行风格执法。
优先关注正确性、行为回归、稳定性、安全性、性能、类型安全和缺失测试。

## 审查边界

- 先理解改动意图、影响面、调用链和外部边界。
- 对纯格式化、机械改名、纯文档、生成代码、第三方目录和不涉及行为变化的小改动，简化审查。
- 若仓库基线不干净，至少确认本次改动没有新增同类问题。
- 对遗留代码、紧急修复和受控例外，披露剩余风险，不把例外描述为“没有风险”。
- 团队偏好和项目私有约束只有在仓库明确采用时才生效，不冒充通用规则。

## 执行动作

1. 读取仓库规则、任务描述和 diff，区分本次改动与既有基线。
2. 使用搜索追踪调用方、公共 API、配置入口和数据边界。
3. 根据改动特征加载专项参考；命中框架或引擎时，若存在对应专项技能则组合使用，否则披露覆盖不足。
4. 能运行时执行相关 `build`、`test`、`lint` 和类型检查，并披露未运行项。
5. 上下文不足时明确假设和未验证项，不把推测写成确定缺陷。

## 审查深度

- **机械改动**：确认没有行为变化，不展开完整审查。
- **局部 bugfix**：检查原始触发路径、回归风险和可重复验证方式。
- **跨模块或高风险改动**：追踪调用链、外部边界和失败路径，按逻辑单元分批审查。

机械改动只执行必要确认；局部 bugfix 聚焦触发路径和验证；跨模块或高风险改动执行完整流程。发现行为变化、公共 API 影响、跨模块调用，或涉及持久化、权限、并发和资源生命周期时，升级到完整审查。

## 审查流程

### 1. 建立上下文

1. 阅读任务、PR 描述、仓库规则和关联说明。
2. 查看改动规模、调用方、公共 API 和数据边界。
3. 标记高风险区域：持久化、异步链路、权限、缓存、序列化、共享组件、资源生命周期。

### 2. 先看方案

- 解决方案是否符合仓库既有模式。
- 是否存在更简单直接的实现。
- 新职责是否落在合理位置。
- 测试是否覆盖关键行为、失败路径和边界条件。

### 3. 再看实现

- **正确性**：状态流转、空值、边界输入、off-by-one、竞态、异常路径。
- **安全性**：输入验证、授权、注入风险、敏感数据处理。
- **资源与异步**：取消传播、任务生命周期、释放、阻塞异步、并发访问。
- **性能**：N+1、重复计算、过度分配、无界结果集、热点路径。
- **可维护性**：职责膨胀、过深嵌套、晦涩抽象、无收益重复、公共面扩大。

大型改动按逻辑单元分批审查；发现构建失败、数据损坏、安全漏洞或明显并发错误时，先停止扩散风险并处理阻断项。

### 4. 输出结论

1. findings 永远放在最前面，按严重性排序，并给出文件与行号。
2. 先说明“为什么这是问题”，再给出可落地的修复方向。
3. 补充 open questions、假设和未验证项。
4. 最后给出简短总结与结论：`Approve`、`Comment` 或 `Request Changes`。

若未发现问题，明确写出“未发现需要提出的审查问题”，并说明剩余测试缺口或未验证项。

## 严重级别

- `[blocking]`：高概率 bug、回归、数据损坏、安全漏洞、资源泄漏或并发错误，合并前应修复。
- `[important]`：常见路径上的稳定性、性能、类型安全或维护风险，强烈建议修复。
- `[nit]`：低风险改进，不阻断合并。
- `[suggestion]`：收益明确的可选方案。
- `[question]`：依赖上下文才能确认的问题。
- `[learning]`：机制或背景说明，不要求动作。
- `[praise]`：值得保留的实现，简短指出即可。

## 反馈方式

- 用代码事实支撑判断，不空泛下结论。
- 区分确定缺陷、风险推断和风格建议。
- 在上下文不足时提出问题，不武断定性。
- 解释风险，聚焦代码，不评价作者。
- 不把个人偏好包装成阻断项。

## 专项参考

根据本次改动按需读取。命中专项特征时应读取对应参考，不要只依赖主流程：

| 命中特征 | 参考文件 | 重点 |
|---|---|---|
| `.ts`、`.tsx`、Promise、类型守卫、事件回调 | [typescript Guide](references/typescript.md) | 运行时边界、类型收窄、泛型、异步、typed linting、回调上下文 |
| `.cs`、`DbContext`、LINQ、ASP.NET Core、DI | [csharp Guide](references/csharp.md) | 异步、资源释放、EF Core、ASP.NET Core、DI、LINQ、集合 |
| bug 修复、新增测试、失败路径变化 | [testing Guide](references/testing.md) | 回归证明、边界、测试替身、稳定性 |
| 跨语言 diff 或需要快速稳定扫查 | [review-checklist](references/review-checklist.md) | 通用高频风险点和决策矩阵 |

React、Vue、游戏引擎或安全审计等领域规则适合由专项技能补充，不在本技能中重复维护。缺少对应专项技能时，披露覆盖限制。

## 输出模板

需要结构化 PR 评论时读取 [模板](assets/pr-review-template.md)。
模板是输出辅助，不应机械展开空章节，也不要填写无法从上下文确认的数据。

