review-changes

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Review 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"):
    1. Checkout the PR:
      gh pr checkout <PR_NUMBER>
    2. Read PR context:
      gh pr view <PR_NUMBER> --json title,body,comments
    3. Use the PR description and comments as additional context for the review
  • 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"):
    1. 检出PR:
      gh pr checkout <PR_NUMBER>
    2. 读取PR上下文:
      gh pr view <PR_NUMBER> --json title,body,comments
    3. 将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):
  1. CLAUDE.md
    ,
    .claude/CLAUDE.md
  2. CODE_GUIDELINES.md
    ,
    .github/CODE_GUIDELINES.md
    ,
    docs/CODE_GUIDELINES.md
  3. STYLE_GUIDE.md
    ,
    .github/STYLE_GUIDE.md
    ,
    docs/STYLE_GUIDE.md
  4. CONTRIBUTING.md
    ,
    .github/CONTRIBUTING.md
    ,
    docs/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.
搜索仓库特定的编码指南。这些指南优先级高于内置指南。
发现顺序(从高到低优先级):
  1. CLAUDE.md
    ,
    .claude/CLAUDE.md
  2. CODE_GUIDELINES.md
    ,
    .github/CODE_GUIDELINES.md
    ,
    docs/CODE_GUIDELINES.md
  3. STYLE_GUIDE.md
    ,
    .github/STYLE_GUIDE.md
    ,
    docs/STYLE_GUIDE.md
  4. CONTRIBUTING.md
    ,
    .github/CONTRIBUTING.md
    ,
    docs/CONTRIBUTING.md
提取与审查相关的内容(编码标准、错误处理、测试、安全、命名约定)。跳过与审查无关的内容(问题模板、行为准则)。用户指南优先级高于内置指南,且可补充内置指南。审查远程PR时,从远程仓库加载指南。

2. Detect Branches

2. 检测分支

bash
undefined
bash
undefined

Get 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"
undefined
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main"
undefined

3. Get Changed Files

3. 获取变更文件

bash
undefined
bash
undefined

Assess 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 rest
git 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:
  1. Identify applicable guidelines: repository-specific (from step 1.5), general (below), and language-specific (from reference files)
  2. Check for critical issues first — report before scanning for minor ones
  3. For each issue found, record: guideline violated, file and line number, problem description, fix suggestion
  4. Skip inapplicable guidelines (no database operations → skip Database & Persistence)
将差异作为主要输入。整体应用指南进行审查,而非逐文件审查。仅当需要上下文理解变更时才读取完整文件。
审查流程:
  1. 确定适用的指南:仓库特定指南(来自步骤1.5)、通用指南(如下)和语言特定指南(来自参考文件)
  2. 首先检查严重问题——在检查次要问题前先报告
  3. 对于每个发现的问题,记录:违反的指南、文件和行号、问题描述、修复建议
  4. 跳过不适用的指南(无数据库操作→跳过数据库与持久化)

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。
undefined

Branch Review:
feature/xyz
main

分支审查:
feature/xyz
main

Repository guidelines loaded:
  • .github/CONTRIBUTING.md
    - coding standards, testing requirements
(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:
    SELECT *
    without
    LIMIT
    on large tables
  • 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
    await
    , verify assumptions are still valid — state may have changed
  • 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
    ,
    .cjs
    → Read references/typescript.md
  • .tsx
    ,
    .jsx
    → Read references/react.md (also load typescript.md)
  • .py
    → Read references/python.md
  • .go
    → Read references/go.md
  • .rs
    → Read references/rust.md
  • .java
    ,
    .kt
    → Read references/java-kotlin.md
Only load references for languages present in the changed files.

根据差异中的文件扩展名,加载相关参考:
  • .ts
    ,
    .js
    ,
    .mjs
    ,
    .cjs
    → 读取 references/typescript.md
  • .tsx
    ,
    .jsx
    → 读取 references/react.md(同时加载typescript.md)
  • .py
    → 读取 references/python.md
  • .go
    → 读取 references/go.md
  • .rs
    → 读取 references/rust.md
  • .java
    ,
    .kt
    → 读取 references/java-kotlin.md
仅加载变更文件中存在的语言对应的参考。

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。