review-changes
Compare original and translation side by side
🇺🇸
Original
English🇨🇳
Translation
ChineseReview Changes
审查代码变更
Review code changes in a feature branch and identify issues before merging.
在合并前审查功能分支中的代码变更并识别问题。
Workflow
工作流程
1. Determine Review Target
1. 确定审查目标
-
Remote PR: If user provides PR number or URL (e.g., "review PR #123", "review https://github.com/org/repo/pull/123"):
- Checkout the PR:
gh pr checkout <PR_NUMBER> - Read PR context:
gh pr view <PR_NUMBER> --json title,body,comments - Use the PR description and comments as additional context for the review
- Checkout the PR:
-
Local Changes: If no PR specified, review current branch against default branch (continue to step 2)
-
远程PR:如果用户提供PR编号或URL(例如:"review PR #123"、"review https://github.com/org/repo/pull/123"):
- 检出PR:
gh pr checkout <PR_NUMBER> - 读取PR上下文:
gh pr view <PR_NUMBER> --json title,body,comments - 将PR描述和评论作为审查的额外上下文
- 检出PR:
-
本地变更:如果未指定PR,则将当前分支与默认分支进行审查(继续步骤2)
1.5 Load Repository Guidelines
1.5 加载仓库指南
Search for repository-specific coding guidelines. These take precedence over built-in guidelines.
Discovery order (highest to lowest priority):
- ,
CLAUDE.md.claude/CLAUDE.md - ,
CODE_GUIDELINES.md,.github/CODE_GUIDELINES.mddocs/CODE_GUIDELINES.md - ,
STYLE_GUIDE.md,.github/STYLE_GUIDE.mddocs/STYLE_GUIDE.md - ,
CONTRIBUTING.md,.github/CONTRIBUTING.mddocs/CONTRIBUTING.md
Extract review-relevant content (coding standards, error handling, testing, security, naming conventions). Skip non-review content (issue templates, code of conduct). User guidelines take precedence over built-in guidelines and are additive. When reviewing a remote PR, load guidelines from the remote repository.
搜索仓库特定的编码指南。这些指南优先级高于内置指南。
发现顺序(从高到低优先级):
- ,
CLAUDE.md.claude/CLAUDE.md - ,
CODE_GUIDELINES.md,.github/CODE_GUIDELINES.mddocs/CODE_GUIDELINES.md - ,
STYLE_GUIDE.md,.github/STYLE_GUIDE.mddocs/STYLE_GUIDE.md - ,
CONTRIBUTING.md,.github/CONTRIBUTING.mddocs/CONTRIBUTING.md
提取与审查相关的内容(编码标准、错误处理、测试、安全、命名约定)。跳过与审查无关的内容(问题模板、行为准则)。用户指南优先级高于内置指南,且可补充内置指南。审查远程PR时,从远程仓库加载指南。
2. Detect Branches
2. 检测分支
bash
undefinedbash
undefinedGet current branch
获取当前分支
git branch --show-current
git branch --show-current
Detect default branch (try remote HEAD, fall back to main)
检测默认分支(尝试远程HEAD,回退到main)
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main"
undefinedgit symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main"
undefined3. Get Changed Files
3. 获取变更文件
bash
undefinedbash
undefinedAssess scope first
首先评估范围
git diff --stat <default-branch>...HEAD
git diff --stat <default-branch>...HEAD
Get full diff
获取完整差异
git diff <default-branch>...HEAD
**Performance guardrails:**
1. Skip lock files, `.min.js`, `.min.css`, generated files, compiled output
2. If >30 changed files, prioritize source over tests/config; summarize skipped files
3. If a single file has >500 lines changed, summarize rather than line-by-line review
4. If total diff >3000 lines, select the ~20 most important files and summarize the restgit diff <default-branch>...HEAD
**性能防护规则:**
1. 跳过锁文件、`.min.js`、`.min.css`、生成文件、编译输出
2. 如果变更文件超过30个,优先审查源码而非测试/配置文件;汇总跳过的文件
3. 如果单个文件变更超过500行,进行汇总而非逐行审查
4. 如果总差异超过3000行,选择约20个最重要的文件进行审查,其余部分汇总4. Analyze Changes
4. 分析变更
Use the diff as the primary input. Apply guidelines holistically across the diff rather than file-by-file. Only read full files when surrounding context is needed to understand a change.
Review process:
- Identify applicable guidelines: repository-specific (from step 1.5), general (below), and language-specific (from reference files)
- Check for critical issues first — report before scanning for minor ones
- For each issue found, record: guideline violated, file and line number, problem description, fix suggestion
- Skip inapplicable guidelines (no database operations → skip Database & Persistence)
将差异作为主要输入。整体应用指南进行审查,而非逐文件审查。仅当需要上下文理解变更时才读取完整文件。
审查流程:
- 确定适用的指南:仓库特定指南(来自步骤1.5)、通用指南(如下)和语言特定指南(来自参考文件)
- 首先检查严重问题——在检查次要问题前先报告
- 对于每个发现的问题,记录:违反的指南、文件和行号、问题描述、修复建议
- 跳过不适用的指南(无数据库操作→跳过数据库与持久化)
5. Check Test Coverage
5. 检查测试覆盖率
For each new or modified file containing business logic:
- Check if corresponding test file exists
- If tests exist, verify new code paths have coverage
- Flag missing tests for critical paths
对于每个包含业务逻辑的新增或修改文件:
- 检查是否存在对应的测试文件
- 如果存在测试,验证新代码路径是否有覆盖
- 标记关键路径缺失的测试
6. Format Output
6. 格式化输出
Present findings grouped by severity, ordered Critical → Major → Minor.
undefined按严重程度分组展示结果,顺序为Critical → Major → Minor。
undefinedBranch Review: feature/xyz
→ main
feature/xyzmain分支审查:feature/xyz
→ main
feature/xyzmainRepository guidelines loaded:
- - coding standards, testing requirements
.github/CONTRIBUTING.md
(These take precedence over built-in rules where they conflict)
已加载仓库指南:
- - 编码标准、测试要求
.github/CONTRIBUTING.md
(当与内置规则冲突时,这些指南优先级更高)
🔴 Critical (X issues)
🔴 Critical(X个问题)
1. [Brief title]
- File:
path/to/file.ts:42 - Problem: Clear description of what's wrong
- Fix: Specific solution
1. [简短标题]
- 文件:
path/to/file.ts:42 - 问题:清晰描述问题所在
- 修复:具体解决方案
🟠 Major (X issues)
🟠 Major(X个问题)
1. [Brief title]
[repo]- File:
path/to/file.ts:87 - Problem: Description
- Fix: Solution
1. [简短标题]
[repo]- 文件:
path/to/file.ts:87 - 问题:问题描述
- 修复:解决方案
🟡 Minor (X issues)
🟡 Minor(X个问题)
1. [Brief title]
- File:
path/to/file.ts:123 - Problem: Description
- Fix: Solution
Summary: X critical, Y major, Z minor issues found.
[Ready to merge / Needs fixes before merge]
If no issues found in a category, omit that section. End with clear merge recommendation. Issues marked `[repo]` were flagged based on repository-specific guidelines. If no repository guidelines were loaded, omit the "Repository guidelines loaded" section.
**Feedback tone:** Be constructive — explain *why* a change is needed. Provide actionable suggestions. Assume positive intent. For approvals, acknowledge the specific value of the contribution.
---1. [简短标题]
- 文件:
path/to/file.ts:123 - 问题:问题描述
- 修复:解决方案
总结:发现X个严重问题、Y个主要问题、Z个次要问题。
[可合并 / 修复后可合并]
如果某类问题未发现,省略该部分。结尾给出明确的合并建议。标记为`[repo]`的问题是基于仓库特定指南标记的。如果未加载仓库指南,省略“已加载仓库指南”部分。
**反馈语气**:保持建设性——解释*为什么*需要修改。提供可执行的建议。假设用户的积极意图。对于通过的审查,认可贡献的具体价值。
---General Guidelines
通用指南
These apply to every review regardless of language.
这些指南适用于所有语言的审查。
API & Breaking Changes
API与破坏性变更
- Removed or renamed public functions/methods without deprecation period
- Changed function signatures (new required parameters, changed return types)
- Modified response shapes in API endpoints (removed fields, changed types)
- Changed default values that alter existing behavior
- Database schema changes without migration scripts
- 在没有 deprecation 周期的情况下移除或重命名公共函数/方法
- 更改函数签名(新增必填参数、更改返回类型)
- 修改API端点的响应结构(移除字段、更改类型)
- 更改会改变现有行为的默认值
- 数据库 schema 变更但未提供迁移脚本
Authentication & Authorization (Critical)
身份验证与授权(严重)
- Missing auth checks on new endpoints or routes
- Downgraded permissions (admin-only → public)
- Hardcoded credentials or API keys
- JWT/session issues: missing expiry, weak secrets, improper validation
- CORS misconfigurations: overly permissive origins (in production)
*
- 新增端点或路由缺失权限检查
- 权限降级(仅管理员→公开)
- 硬编码凭证或API密钥
- JWT/会话问题:缺失过期时间、弱密钥、验证不当
- CORS配置错误:生产环境中使用过于宽松的来源()
*
Database & Persistence
数据库与持久化
- Missing transactions for multi-step atomic operations
- N+1 query patterns: fetching related data in loops instead of joins/eager loading
- Missing indexes on frequently queried columns
- Unbounded queries: without
SELECT *on large tablesLIMIT - SQL injection: string concatenation in queries instead of parameterized queries
- 多步骤原子操作缺失事务
- N+1查询模式:在循环中获取关联数据而非使用连接/预加载
- 频繁查询的列缺失索引
- 无界查询:在大表上使用且未加
SELECT *LIMIT - SQL注入:使用字符串拼接构建查询而非参数化查询
Concurrency (Critical for data corruption)
并发(数据损坏风险:严重)
- Shared mutable state accessed without synchronization
- Missing locks/mutexes when modifying shared resources
- Check-then-act patterns without atomicity (TOCTOU)
- Deadlock potential: acquiring multiple locks in inconsistent order
- 访问共享可变状态时未同步
- 修改共享资源时缺失锁/互斥量
- 无原子性的检查-然后-操作模式(TOCTOU)
- 死锁风险:以不一致的顺序获取多个锁
Async Code
异步代码
- After any , verify assumptions are still valid — state may have changed
await - Flag when code returns success without verifying the expected outcome of an async operation
- 在任何之后,验证假设是否仍然有效——状态可能已更改
await - 当代码在未验证异步操作预期结果的情况下返回成功时,标记该问题
External API Handling
外部API处理
- Missing timeouts on HTTP requests
- No retry logic for transient failures
- Missing circuit breakers for repeatedly failing services
- Not distinguishing between 4xx and 5xx errors
- Missing rate limiting awareness (no backoff on 429)
- HTTP请求缺失超时设置
- 瞬时故障无重试逻辑
- 重复失败的服务未配置断路器
- 未区分4xx和5xx错误
- 未考虑速率限制(429错误时无退避策略)
Edge Cases & Boundaries
边缘情况与边界
- Code assumes arrays/lists are non-empty
- Missing null/undefined checks on optional values
- Off-by-one errors in loops, incorrect range checks
- Type coercion issues leading to unexpected behavior
- Unicode/encoding issues: assuming ASCII, incorrect string length
- 代码假设数组/列表非空
- 可选值缺失null/undefined检查
- 循环中的差一错误、不正确的范围检查
- 类型强制转换导致意外行为
- Unicode/编码问题:假设ASCII、字符串长度计算错误
Defensive Coding
防御性编码
- Using external input (APIs/users) without validation
- Not handling failure cases for operations that can fail
- Array access without verifying index is valid
- Code relying on undocumented behavior or ordering
- 使用外部输入(API/用户)时未验证
- 未处理可能失败的操作的失败情况
- 数组访问时未验证索引有效性
- 代码依赖未记录的行为或顺序
Input Sanitization (Critical)
输入 sanitization(严重)
- Command injection: unsanitized input in shell commands — use argument arrays, not string interpolation
- Path traversal: user input in file paths escaping intended directories — resolve and validate paths
- XSS: user input rendered as HTML — escape or use safe APIs, validate URL protocols
- Log injection: unsanitized input forging log entries — use structured logging
- SQL injection: string concatenation in queries — use parameterized queries
- 命令注入:shell命令中使用未 sanitized 的输入——使用参数数组而非字符串插值
- 路径遍历:文件路径中的用户输入超出预期目录——解析并验证路径
- XSS:用户输入直接渲染为HTML——转义或使用安全API、验证URL协议
- 日志注入:未 sanitized 的输入伪造日志条目——使用结构化日志
- SQL注入:使用字符串拼接构建查询而非参数化查询
Memory & Performance
内存与性能
- Unbounded collections (arrays/maps growing without limits)
- Memory leaks: event listeners not removed, closures holding references
- Large allocations in loops that could be reused
- Blocking synchronous I/O or CPU-heavy work on main thread
- Missing pagination: loading entire datasets
- 无界集合(数组/映射无限增长)
- 内存泄漏:事件监听器未移除、闭包持有引用
- 循环中可复用的大内存分配
- 主线程上的阻塞同步I/O或CPU密集型工作
- 缺失分页:加载整个数据集
File System Operations
文件系统操作
- Reading files without verifying they exist
- Writing files without ensuring parent directory exists
- Missing error handling on file operations
- 读取文件时未验证文件是否存在
- 写入文件时未确保父目录存在
- 文件操作缺失错误处理
Logging & Observability
日志与可观测性
- Insufficient logging for important operations
- Excessive logging creating noise or performance issues
- Missing correlation IDs for cross-service tracing
- Critical: Logging sensitive data (PII, passwords, tokens)
- New features without observability hooks
- 重要操作的日志不足
- 日志过多产生噪音或性能问题
- 跨服务追踪缺失关联ID
- 严重:记录敏感数据(PII、密码、令牌)
- 新功能无观测钩子
Testing Quality
测试质量
- Tests that don't assert anything meaningful
- Missing edge case coverage (only happy path)
- Flaky tests: race conditions, time dependencies, external dependencies
- Test pollution: shared state, missing cleanup
- Mocking too much: tests don't exercise real code paths
- 测试未断言任何有意义的内容
- 缺失边缘情况覆盖(仅覆盖正常路径)
- 不稳定测试:竞态条件、时间依赖、外部依赖
- 测试污染:共享状态、缺失清理
- 过度Mock:测试未执行真实代码路径
Accessibility
可访问性
- Missing alt text on images
- Non-semantic HTML (divs for buttons, missing form labels)
- Interactive elements not reachable via keyboard
- Missing ARIA labels on icon-only buttons
- Color-only indicators
- 图片缺失替代文本
- 非语义化HTML(用div代替按钮、缺失表单标签)
- 交互元素无法通过键盘访问
- 仅图标按钮缺失ARIA标签
- 仅依赖颜色的指示器
Code Style
代码风格
- Prefer early returns over nested if statements — flat code is easier to read
- 优先使用提前返回而非嵌套if语句——扁平化代码更易读
Error Messages
错误消息
- Error messages must be actionable, contextual, and specific
- Include identifiers (order IDs, user IDs) for debugging
- Don't expose stack traces or internal details to end users
- Include error codes for programmatic handling
- 错误消息必须可执行、上下文相关且具体
- 包含用于调试的标识符(订单ID、用户ID)
- 不向终端用户暴露堆栈跟踪或内部细节
- 包含用于程序化处理的错误码
Language-Specific Guidelines
语言特定指南
Based on file extensions in the diff, load the relevant reference:
- ,
.ts,.js,.mjs→ Read references/typescript.md.cjs - ,
.tsx→ Read references/react.md (also load typescript.md).jsx - → Read references/python.md
.py - → Read references/go.md
.go - → Read references/rust.md
.rs - ,
.java→ Read references/java-kotlin.md.kt
Only load references for languages present in the changed files.
根据差异中的文件扩展名,加载相关参考:
- ,
.ts,.js,.mjs→ 读取 references/typescript.md.cjs - ,
.tsx→ 读取 references/react.md(同时加载typescript.md).jsx - → 读取 references/python.md
.py - → 读取 references/go.md
.go - → 读取 references/rust.md
.rs - ,
.java→ 读取 references/java-kotlin.md.kt
仅加载变更文件中存在的语言对应的参考。
Severity Definitions
严重程度定义
- Critical — Block the merge. Broken code, security vulnerabilities, data leaks, runtime failures that will crash production.
- Major — Should fix. Unhandled async, missing error handling, resource leaks, race conditions, missing validation.
- Minor — Nice to fix. Code clarity, consistency, performance, dead code, duplication, unresolved TODOs.
- Critical(严重) — 阻止合并。代码损坏、安全漏洞、数据泄露、会导致生产环境崩溃的运行时故障。
- Major(主要) — 应该修复。未处理的异步操作、缺失的错误处理、资源泄漏、竞态条件、缺失的验证。
- Minor(次要) — 建议修复。代码清晰度、一致性、性能、死代码、重复代码、未解决的TODO。