systematic-code-review

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Systematic Code Review Skill

系统化代码评审技能

Operator Context

操作场景

This skill operates as an operator for systematic code review, configuring Claude's behavior for thorough, verifiable review processes. It implements the Iterative Refinement architectural pattern — understand context, verify claims, assess risks, document findings — with Domain Intelligence embedded in severity classification and Go-specific review patterns.

本技能作为系统化代码评审的操作规范，用于配置Claude的行为以实现全面、可验证的评审流程。它采用迭代优化架构模式——理解上下文、验证声明、评估风险、记录结果，并在严重程度分类和Go语言特定评审模式中嵌入了领域智能。

Hardcoded Behaviors (Always Apply)

硬编码行为（始终适用）

CLAUDE.md Compliance: Read and follow repository CLAUDE.md files before executing review
Over-Engineering Prevention: Don't suggest features outside PR scope, but DO flag all issues IN the changed code even if fixing them requires touching other files. No speculative improvements.
Strict Severity Classification: Use the Severity Classification Rules below. When in doubt, classify UP not down.
NEVER approve without running tests: Review must include test execution
NEVER trust comments without verification: All claims must be verified against code
NEVER skip security assessment: Security implications must be explicitly evaluated
Complete phase gates: Each phase must complete before proceeding

CLAUDE.md合规性：执行评审前需阅读并遵循仓库中的CLAUDE.md文件
避免过度设计：不建议超出PR范围的功能，但需标记变更代码中的所有问题，即使修复这些问题需要修改其他文件。不做推测性改进。
严格的严重程度分类：遵循下方的《严重程度分类规则》。若存疑，优先向上归类（即更严格的级别）。
未运行测试绝不批准：评审必须包含测试执行环节
绝不轻信未验证的注释：所有声明必须与代码进行验证
绝不跳过安全评估：必须明确评估安全影响
完成阶段关卡：每个阶段必须完成后才能进入下一阶段

Default Behaviors (ON unless disabled)

默认行为（默认开启，可关闭）

Communication Style: Report facts without self-congratulation. Show command output rather than describing it. Be concise but informative
Temporary File Cleanup: Remove any temporary analysis files, notes, or debug outputs created during review at task completion. Keep only the final review document
Read all changed files: Don't review summaries, read actual code
Check affected dependencies: Identify ripple effects
Verify test coverage: Confirm tests exist for changes
Document all findings: Create structured review output

沟通风格：只报告事实，不自我夸耀。直接展示命令输出而非描述内容。简洁且信息完整
临时文件清理：评审任务完成后，删除所有评审过程中创建的临时分析文件、笔记或调试输出。仅保留最终评审文档
阅读所有变更文件：不依赖摘要评审，需阅读实际代码
检查受影响的依赖项：识别连锁影响
验证测试覆盖率：确认变更内容有对应测试覆盖
记录所有发现：生成结构化的评审输出

Optional Behaviors (OFF unless enabled)

可选行为（默认关闭，可开启）

Performance profiling: Benchmark affected code paths
Historical analysis: Check for similar past issues
Extended security audit: Deep security analysis beyond standard checks

性能分析：对受影响的代码路径进行基准测试
历史分析：检查是否存在类似的过往问题
扩展安全审计：在标准检查之外进行深度安全分析

What This Skill CAN Do

本技能可实现的功能

Systematic phase-gated reviews with explicit gates between phases
Severity classification (BLOCKING / SHOULD FIX / SUGGESTION) with decision tree
Security, performance, and architecture risk assessment
Go-specific pattern detection (concurrency, resource management, type exports)
Go ecosystem pattern validation
Receiving review feedback patterns (when acting as code author)

带有明确阶段关卡的系统化评审流程
基于决策树的严重程度分类（BLOCKING / SHOULD FIX / SUGGESTION）
安全、性能和架构风险评估
Go语言特定模式检测（并发、资源管理、类型导出）
Go生态系统模式验证
接收评审反馈的模式（当作为代码作者时）

What This Skill CANNOT Do

本技能不可实现的功能

Write implementation code or implement features
Skip phases or bypass phase gates
Approve without running tests
Review without reading all changed files
Make speculative improvements outside PR scope

编写实现代码或开发功能
跳过阶段或绕过阶段关卡
未运行测试就批准
未阅读所有变更文件就评审
提出超出PR范围的推测性改进

Instructions

操作指南

Phase 1: UNDERSTAND

阶段1：理解（UNDERSTAND）

Goal: Map all changes and their relationships before forming any opinions.

Step 1: Read every changed file

Use Read tool on EVERY changed file completely
Map what each file does and how changes affect it

Step 2: Identify dependencies

Use Grep to find all callers/consumers of changed code
Note any comments that make claims about behavior

Step 2a: Caller Tracing (mandatory when diff modifies function signatures, parameter semantics, or introduces sentinel/special values)

When the change modifies how a function/method is called or what parameters mean:

Find ALL callers — Grep for the function name with receiver syntax (e.g.,
```
.GetEvents(
```
not just
```
GetEvents
```
) across the entire repo. For Go repos, prefer gopls
```
go_symbol_references
```
via ToolSearch("gopls") — it's type-aware and catches interface implementations.
Trace the VALUE SPACE — For each parameter source, classify what values can flow through:
- Query parameters (
```
r.FormValue
```
  ,
```
r.URL.Query
```
  ): user-controlled — ANY string including sentinel values like
```
"*"
```
- Auth token fields: server-controlled (UUIDs, structured IDs)
- Constants/enums: fixed set
- Do NOT conclude a sentinel is "unreachable" because no Go code constructs that string. If the source is user input, the user constructs it.
Verify each caller — For each call site, check that parameters are validated before being passed. Pay special attention to sentinel values (e.g.,
```
"*"
```
meaning "all/unfiltered") that bypass security filtering.
Report unvalidated paths — Any caller that passes user input to a security-sensitive parameter without validation is a BLOCKING finding.
Do NOT trust PR descriptions about who calls the function — verify independently. The PR author may have forgotten about callers, or new callers may have been added in other branches.

This step catches:

Unchecked paths where user input reaches a security-sensitive parameter
Callers the PR author forgot about or didn't mention
Interface implementations that don't enforce the same preconditions

Step 3: Document scope

PHASE 1: UNDERSTAND

Changed Files:
  - [file1.ext]: [+N/-M lines] [brief description of change]
  - [file2.ext]: [+N/-M lines] [brief description of change]

Change Type: [feature | bugfix | refactor | config | docs]

Scope Assessment:
  - Primary: [what's directly changed]
  - Secondary: [what's affected by the change]
  - Dependencies: [external systems/files impacted]

Caller Tracing (if signature/parameter semantics changed):
  - [function/method]: [N] callers found
    - [caller1:line] — parameter validated: [yes/no]
    - [caller2:line] — parameter validated: [yes/no]
  - Unvalidated paths: [list or "none"]

Questions for Author:
  - [Any unclear aspects that need clarification]

Gate: All changed files read, scope fully mapped, callers traced (if applicable). Proceed only when gate passes.

目标：在形成任何观点之前，梳理所有变更及其关联关系。

步骤1：阅读所有变更文件

使用阅读工具完整读取每一个变更文件
梳理每个文件的功能以及变更对其的影响

步骤2：识别依赖关系

使用Grep工具查找变更代码的所有调用方/消费者
记录任何涉及行为描述的注释声明

步骤2a：调用方追踪（当diff修改函数签名、参数语义或引入哨兵/特殊值时，为必填步骤）

当变更修改了函数/方法的调用方式或参数含义时：

查找所有调用方——在整个仓库中使用接收者语法（例如
```
.GetEvents(
```
而非仅
```
GetEvents
```
）搜索函数名。对于Go仓库，优先通过ToolSearch("gopls")使用gopls的
```
go_symbol_references
```
功能——它支持类型感知，能捕获接口实现。
追踪值空间——对每个参数来源，分类可传入的取值类型：
- 查询参数（
```
r.FormValue
```
  、
```
r.URL.Query
```
  ）：用户可控——包括
```
"*"
```
  等哨兵值在内的任意字符串
- 认证令牌字段：服务端可控（UUID、结构化ID）
- 常量/枚举：固定取值集合
- 请勿仅因没有Go代码构造该字符串就判定哨兵值「不可达」。若来源是用户输入，则用户可构造该值。
验证每个调用方——对每个调用点，检查参数在传入前是否经过验证。需特别注意绕过安全过滤的哨兵值（如
```
"*"
```
表示「全部/未过滤」）。
报告未验证路径——任何将用户输入直接传入安全敏感参数且未做验证的调用方，均属于BLOCKING级问题。
不要轻信PR描述中关于调用方的内容——需独立验证。PR作者可能遗漏了调用方，或者其他分支中新增了调用方。

此步骤可发现：

用户输入直接到达安全敏感参数的未验证路径
PR作者遗漏或未提及的调用方
未强制执行相同前置条件的接口实现

步骤3：记录范围信息

PHASE 1: UNDERSTAND

变更文件:
  - [file1.ext]: [+N/-M行] [变更内容简要描述]
  - [file2.ext]: [+N/-M行] [变更内容简要描述]

变更类型: [feature | bugfix | refactor | config | docs]

范围评估:
  - 核心变更: [直接修改的内容]
  - 次要影响: [受变更影响的内容]
  - 依赖项: [受影响的外部系统/文件]

调用方追踪（若修改了签名/参数语义）:
  - [function/method]: 共找到[N]个调用方
    - [caller1:行号] — 参数已验证: [是/否]
    - [caller2:行号] — 参数已验证: [是/否]
  - 未验证路径: [列表或"无"]

需向作者确认的问题:
  - [任何需要澄清的模糊点]

关卡要求：已读取所有变更文件，完整梳理范围，已完成调用方追踪（若适用）。仅当满足要求时才可进入下一阶段。

Phase 2: VERIFY

阶段2：验证（VERIFY）

Goal: Validate all assertions in code, comments, and PR description against actual behavior.

Step 1: Run tests

Execute existing tests for changed files
Capture complete test output

Step 2: Verify claims

Check every comment claim against code behavior
Verify edge cases mentioned are actually handled
Trace through critical code paths manually

Step 3: Document verification

PHASE 2: VERIFY

Claims Verification:
  Claim: "[Quote from comment or PR description]"
  Verification: [How verified - test run, code trace, etc.]
  Result: VALID | INVALID | NEEDS-DISCUSSION

Test Execution:
  $ [test command]
  Result: [PASS/FAIL with summary]

Behavior Verification:
  - Expected: [what change claims to do]
  - Actual: [what code actually does]
  - Match: YES | NO | PARTIAL

Gate: All assertions in code/comments verified against actual behavior. Proceed only when gate passes.

目标：验证代码、注释和PR描述中的所有断言是否与实际行为一致。

步骤1：运行测试

执行为变更文件编写的现有测试
捕获完整的测试输出

步骤2：验证声明

检查每条注释声明是否与代码行为一致
验证提及的边缘情况是否已处理
手动追踪关键代码路径

步骤3：记录验证结果

PHASE 2: VERIFY

声明验证:
  声明: "[引用自注释或PR描述的内容]"
  验证方式: [如何验证——测试运行、代码追踪等]
  结果: VALID | INVALID | NEEDS-DISCUSSION

测试执行:
  $ [测试命令]
  结果: [PASS/FAIL及摘要]

行为验证:
  - 预期行为: [变更声称要实现的功能]
  - 实际行为: [代码实际实现的功能]
  - 匹配度: 是 | 否 | 部分匹配

关卡要求：已验证代码/注释中的所有断言与实际行为一致。仅当满足要求时才可进入下一阶段。

Phase 3: ASSESS

阶段3：评估（ASSESS）

Goal: Evaluate security, performance, and architectural risks specific to these changes.

Step 1: Security assessment

Evaluate OWASP top 10 against changes
Explain HOW each vulnerability was ruled out (not just checkboxes)

Step 2: Performance assessment

Identify performance-critical paths and evaluate impact
Check for N+1 queries, unbounded loops, unnecessary allocations

Step 3: Architectural assessment

Compare patterns to existing codebase conventions
Assess breaking change potential

Step 4: Extraction severity escalation

If the diff extracts inline code into named helper functions, re-evaluate all defensive guards
A missing check rated LOW as inline code (1 caller, "upstream validates") becomes MEDIUM as a reusable function (N potential callers)

See

skills/shared-patterns/severity-classification.md

for the full rule

Step 4: Document assessment

PHASE 3: ASSESS

Security Assessment:
  SQL Injection: [N/A | CHECKED - how verified | ISSUE - details]
  XSS: [N/A | CHECKED - how verified | ISSUE - details]
  Input Validation: [N/A | CHECKED - how verified | ISSUE - details]
  Auth: [N/A | CHECKED - how verified | ISSUE - details]
  Secrets: [N/A | CHECKED - how verified | ISSUE - details]
  Findings: [specific issues or "No security issues found"]

Performance Assessment:
  Findings: [specific issues or "No performance issues found"]

Architectural Assessment:
  Findings: [specific issues or "Architecturally sound"]

Risk Level: LOW | MEDIUM | HIGH | CRITICAL

Gate: Security, performance, and architectural risks explicitly evaluated. Proceed only when gate passes.

目标：针对本次变更评估安全、性能和架构风险。

步骤1：安全评估

对照OWASP Top 10评估变更
说明如何排除每个漏洞（不只是勾选复选框）

步骤2：性能评估

识别性能关键路径并评估影响
检查是否存在N+1查询、无界循环、不必要的内存分配

步骤3：架构评估

将变更模式与现有代码库规范进行对比
评估潜在的破坏性变更风险

步骤4：提取操作的严重程度升级

若diff将内联代码提取为命名辅助函数，需重新评估所有防御性检查
作为内联代码时仅1个调用方且「上游已验证」的缺失检查，作为可复用函数时（潜在N个调用方）严重程度升级为MEDIUM

完整规则请参考

skills/shared-patterns/severity-classification.md

步骤4：记录评估结果

PHASE 3: ASSESS

安全评估:
  SQL注入: [不适用 | 已检查——验证方式 | 存在问题——详情]
  XSS跨站脚本: [不适用 | 已检查——验证方式 | 存在问题——详情]
  输入验证: [不适用 | 已检查——验证方式 | 存在问题——详情]
  认证授权: [不适用 | 已检查——验证方式 | 存在问题——详情]
  敏感信息: [不适用 | 已检查——验证方式 | 存在问题——详情]
  发现: [具体问题或"未发现安全问题"]

性能评估:
  发现: [具体问题或"未发现性能问题"]

架构评估:
  发现: [具体问题或"架构设计合理"]

风险等级: LOW | MEDIUM | HIGH | CRITICAL

关卡要求：已明确评估安全、性能和架构风险。仅当满足要求时才可进入下一阶段。

Phase 4: DOCUMENT

阶段4：记录（DOCUMENT）

Goal: Produce structured review output with clear verdict and rationale.

PHASE 4: DOCUMENT

Review Summary:
  Files Reviewed: [N]
  Lines Changed: [+X/-Y]
  Test Status: [PASS/FAIL/SKIPPED]
  Risk Level: [LOW/MEDIUM/HIGH/CRITICAL]

Findings (use Severity Classification Rules - when in doubt, classify UP):

BLOCKING (cannot merge - security/correctness/reliability):
  1. [Issue with file:line reference and category from rules]

SHOULD FIX (fix unless urgent - patterns/tests/debugging):
  1. [Issue with file:line reference and category from rules]

SUGGESTIONS (author's choice - purely stylistic):
  1. [Suggestion with benefit - only if genuinely optional]

POSITIVE NOTES:
  1. [Good practice observed]

Verdict: APPROVE | REQUEST-CHANGES | NEEDS-DISCUSSION

Rationale: [1-2 sentences explaining verdict]

Gate: Structured review output with clear verdict. Review is complete.

目标：生成带有明确结论和理由的结构化评审输出。

PHASE 4: DOCUMENT

评审摘要:
  已评审文件数: [N]
  变更行数: [+X/-Y]
  测试状态: [PASS/FAIL/SKIPPED]
  风险等级: [LOW/MEDIUM/HIGH/CRITICAL]

发现问题（遵循严重程度分类规则——若存疑，优先向上归类）:

BLOCKING（无法合并——安全/正确性/可靠性问题）:
  1. [问题详情，含文件:行号引用及规则中的分类]

SHOULD FIX（除非紧急否则需修复——模式/测试/调试问题）:
  1. [问题详情，含文件:行号引用及规则中的分类]

SUGGESTIONS（作者可自行决定——纯风格类问题）:
  1. [建议内容及收益——仅当确实可选时才记录]

正面评价:
  1. [观察到的良好实践]

评审结论: APPROVE | REQUEST-CHANGES | NEEDS-DISCUSSION

结论理由: [1-2句话解释结论]

关卡要求：已生成带有明确结论的结构化评审输出。至此评审完成。

Trust Hierarchy

信任优先级

When conflicting information exists, trust in this order:

Running code (highest) - What tests show
HTTP/API requests - Verified external behavior
Grep results - What exists in codebase
Reading source - Direct file inspection
Comments/docs - Claims that need verification
PR description (lowest) - May be outdated or incomplete

当存在冲突信息时，按以下优先级采信:

运行中的代码（最高优先级）——测试展示的结果
HTTP/API请求——已验证的外部行为
Grep结果——代码库中实际存在的内容
源代码阅读——直接文件检查
注释/文档——需要验证的声明
PR描述（最低优先级）——可能已过时或不完整

Severity Classification Rules

严重程度分类规则

Guiding principle: When in doubt, classify UP. It's better to require a fix and have the author push back than to let a real issue slip through as "optional."

指导原则：若存疑，优先向上归类。要求修复比将实际问题标记为「可选」而遗漏更稳妥。

BLOCKING (cannot merge without fixing)

BLOCKING（不修复则无法合并）

These issues MUST be fixed. Never mark these as "needs discussion" or "optional":

Category	Examples
Security vulnerabilities	Authentication bypass, injection (SQL/XSS/command), data exposure, secrets in code, missing authorization checks
Test failures	Any failing test, including pre-existing failures touched by the change
Breaking changes	API breaking without migration, backward incompatible changes without versioning
Missing error handling	Unhandled errors on network/filesystem/database operations, panics in production paths
Race conditions	Concurrent access without synchronization, data races
Resource leaks	Unclosed file handles, database connections, memory leaks in hot paths
Logic errors	Off-by-one errors, incorrect conditionals, wrong return values

此类问题必须修复。绝不能标记为「需要讨论」或「可选」:

分类	示例
安全漏洞	认证绕过、注入（SQL/XSS/命令）、数据泄露、代码中包含敏感信息、缺失权限检查
测试失败	任何测试失败，包括变更触及的原有失败测试
破坏性变更	无迁移方案的API破坏性变更、无版本控制的向后不兼容变更
缺失错误处理	网络/文件系统/数据库操作的未处理错误、生产路径中的panic
竞态条件	无同步的并发访问、数据竞争
资源泄漏	未关闭的文件句柄、数据库连接、热点路径中的内存泄漏
逻辑错误	边界错误、条件判断错误、返回值错误

SHOULD FIX (merge only if urgent, otherwise fix)

SHOULD FIX（除非紧急否则需修复）

These issues should be fixed unless there's time pressure. Never mark as "suggestion":

Category	Examples
Missing tests	New code paths without test coverage, untested error conditions
Unhelpful error messages	Errors that don't include context for debugging (missing IDs, states, inputs)
Pattern violations	Inconsistent with established codebase patterns (but still functional)
Performance in hot paths	N+1 queries, unnecessary allocations in loops, missing indexes for frequent queries
Deprecated API usage	Using APIs marked for removal, outdated patterns with better alternatives
Poor encapsulation	Exposing internal state unnecessarily, breaking abstraction boundaries

此类问题应修复，除非时间紧迫。绝不能标记为「建议」:

分类	示例
缺失测试	新代码路径无测试覆盖、未测试的错误条件
无用的错误信息	不包含调试上下文的错误（缺失ID、状态、输入）
模式违规	与已有代码库模式不一致但仍可运行
热点路径性能问题	N+1查询、循环中的不必要分配、频繁查询缺失索引
已弃用API使用	使用标记为移除的API、有更好替代方案的过时模式
封装性差	不必要地暴露内部状态、破坏抽象边界

SUGGESTIONS (author's choice)

SUGGESTIONS（作者可自行决定）

These are genuinely optional - author can reasonably decline:

Category	Examples
Naming preferences	Variable/function names that are adequate but could be clearer
Comment additions	Places where a comment would help but code is understandable
Alternative approaches	Different implementation that isn't clearly better
Style not in CLAUDE.md	Formatting preferences not codified in project standards
Micro-optimizations	Performance improvements in cold paths with no measurable impact

此类问题确实可选——作者可合理拒绝:

分类	示例
命名偏好	变量/函数名可用但可更清晰
添加注释	添加注释会更易懂但代码本身可理解
替代实现方案	不同的实现但无明显优势
CLAUDE.md未规定的风格	项目标准未明确的格式偏好
微优化	冷路径中的性能改进，无可衡量的影响

Classification Decision Tree

分类决策树

Is there a security, correctness, or reliability risk?
|- YES -> BLOCKING
|- NO -> Does it violate established patterns or create maintenance burden?
          |- YES -> SHOULD FIX
          |- NO -> Is this purely stylistic or preferential?
                   |- YES -> SUGGESTION (or don't mention)
                   |- NO -> Re-evaluate: probably SHOULD FIX

是否存在安全、正确性或可靠性风险?
|- 是 -> BLOCKING
|- 否 -> 是否违反既定模式或增加维护负担?
          |- 是 -> SHOULD FIX
          |- 否 -> 是否纯风格或偏好类问题?
                   |- 是 -> SUGGESTION（或不提及）
                   |- 否 -> 重新评估：可能属于SHOULD FIX

Common Misclassifications to Avoid

需避免的常见分类错误

Issue	Wrong	Correct	Why
Missing error check on `os.Open()`	SUGGESTION	BLOCKING	Resource leak + potential panic
No test for new endpoint	SUGGESTION	SHOULD FIX	Untested code is liability
Race condition in cache	NEEDS DISCUSSION	BLOCKING	Data corruption risk
Inconsistent naming	BLOCKING	SUGGESTION	No functional impact
Missing context in error	SUGGESTION	SHOULD FIX	Debugging nightmare
Unused import	BLOCKING	SHOULD FIX	Linter will catch, low impact

问题	错误分类	正确分类	原因
`os.Open()` 缺失错误检查	SUGGESTION	BLOCKING	资源泄漏+潜在panic
新端点无测试	SUGGESTION	SHOULD FIX	未测试代码是潜在风险
缓存中的竞态条件	NEEDS DISCUSSION	BLOCKING	存在数据损坏风险
命名不一致	BLOCKING	SUGGESTION	无功能影响
错误信息缺失上下文	SUGGESTION	SHOULD FIX	调试困难
未使用的导入	BLOCKING	SHOULD FIX	会被Linter捕获，影响较小

Examples

示例

Example 1: Pull Request Review

示例1：拉取请求评审

User says: "Review this PR" Actions:

Read all changed files, map scope and dependencies (UNDERSTAND)
Run tests, verify claims in comments and PR description (VERIFY)
Evaluate security/performance/architecture risks (ASSESS)
Produce structured findings with severity and verdict (DOCUMENT) Result: Structured review with clear verdict and rationale

用户指令："Review this PR" 操作步骤:

阅读所有变更文件，梳理范围和依赖关系（UNDERSTAND）
运行测试，验证注释和PR描述中的声明（VERIFY）
评估安全/性能/架构风险（ASSESS）
生成带有严重程度分类和结论的结构化发现（DOCUMENT）结果：带有明确结论和理由的结构化评审报告

Example 2: Pre-Merge Verification

示例2：合并前验证

User says: "Check this before we merge" Actions:

Read all changes, identify breaking change potential (UNDERSTAND)
Run full test suite, verify backward compatibility claims (VERIFY)
Assess risk level for production deployment (ASSESS)
Document findings with APPROVE/REQUEST-CHANGES verdict (DOCUMENT) Result: Go/no-go decision with evidence

用户指令："Check this before we merge" 操作步骤:

阅读所有变更，识别潜在的破坏性变更（UNDERSTAND）
运行完整测试套件，验证向后兼容性声明（VERIFY）
评估生产部署的风险等级（ASSESS）
记录发现并给出APPROVE/REQUEST-CHANGES结论（DOCUMENT）结果：带有证据的是否合并决策

Error Handling

错误处理

Error: "Incomplete Information"

错误："信息不完整"

Cause: Missing context about the change or its purpose Solution:

Ask for clarification in Phase 1
Do not proceed past UNDERSTAND with unanswered questions
Document gaps in scope assessment

原因：缺少关于变更或其目的的上下文解决方案:

在阶段1中请求澄清
若有未解答的问题，请勿进入UNDERSTAGE之后的阶段
在范围评估中记录信息缺口

Error: "Test Failures"

错误："测试失败"

Cause: Tests fail during Phase 2 verification Solution:

Document in Phase 2
Automatic BLOCKING finding in Phase 4
Cannot APPROVE with failing tests

原因：阶段2验证过程中测试失败解决方案:

在阶段2中记录
在阶段4中自动标记为BLOCKING级问题
测试失败时无法APPROVE

Error: "Unclear Risk"

错误："风险不明确"

Cause: Cannot determine severity of an issue Solution:

Default to higher risk level (classify UP)
Document uncertainty in assessment
REQUEST-CHANGES if critical uncertainty exists

原因：无法确定问题的严重程度解决方案:

默认归类为更高风险等级（向上归类）
在评估中记录不确定性
若存在严重不确定性，给出REQUEST-CHANGES结论

Anti-Patterns

反模式

Anti-Pattern 1: Reviewing Without Running Tests

反模式1：未运行测试就评审

What it looks like: "I reviewed the code and it looks good. The logic seems correct." Why wrong: Comments and code may not match actual behavior. Tests reveal edge cases not visible in code reading. Cannot verify claims without execution. Do instead: Run tests in Phase 2. Show complete test output. Verify behavior matches claims.

表现："我已评审代码，看起来没问题。逻辑似乎正确。" 错误原因：注释和代码可能与实际行为不符。测试可发现代码阅读中看不到的边缘情况。不执行测试就无法验证声明。 正确做法：在阶段2中运行测试。展示完整的测试输出。验证行为是否与声明一致。

Anti-Pattern 2: Accepting Comments as Truth

反模式2：将注释视为事实

What it looks like: Marking claims as verified just because a comment says so. Why wrong: Comments frequently become outdated. Developers may not understand what "thread-safe" actually means. Claims need verification against actual code. Do instead: Inspect code for every claim. Verify "thread-safe" means mutexes exist. Mark INVALID when comments lie.

表现：仅因注释提及就标记声明为已验证。 错误原因：注释经常会过时。开发者可能并不真正理解「线程安全」的含义。声明需要与实际代码验证。 正确做法：对每条声明检查代码。验证「线程安全」是否确实存在互斥锁。当注释与事实不符时标记为INVALID。

Anti-Pattern 3: Skipping Phase Gates

反模式3：跳过阶段关卡

What it looks like: Reading 2 of 5 changed files and saying "I get the gist, moving to VERIFY..." Why wrong: Missing context leads to incorrect conclusions. Dependencies between files may be missed. Cannot assess full impact without complete understanding. Do instead: Read ALL changed files. Complete every gate before proceeding.

表现：阅读5个变更文件中的2个后说「我了解大意了，进入VERIFY阶段...」 错误原因：缺失上下文会导致错误结论。可能会遗漏文件之间的依赖关系。不完整理解就无法评估全部影响。 正确做法：阅读所有变更文件。完成所有关卡要求后再进入下一阶段。

Anti-Pattern 4: Generic Security Checklist Without Context

反模式4：无上下文的通用安全检查清单

What it looks like: Checking all security boxes without explaining how vulnerabilities were ruled out. Why wrong: Checkbox approach misses context-specific vulnerabilities. Gives false confidence without actual analysis. Do instead: Explain HOW each vulnerability was ruled out. Mark N/A for irrelevant checks. Show evidence for findings.

表现：勾选所有安全复选框但不说明如何排除漏洞。 错误原因：复选框方法会遗漏特定上下文的漏洞。未进行实际分析就给出虚假的信心。 正确做法：说明如何排除每个漏洞。对不相关的检查标记为N/A。为发现的问题提供证据。

Go-Specific Review Patterns

Go语言特定评审模式

When reviewing Go code, watch for these patterns that linters miss:

评审Go代码时，需注意以下Linter可能遗漏的模式:

Type Export Design

类型导出设计

Are implementation types unnecessarily exported?
Should types be unexported with only constructors exported?
Red flag:
```
type FooStore struct{}
```
exported but only implements an interface

是否不必要地导出了实现类型？
是否应该隐藏类型仅导出构造函数？
危险信号：
```
type FooStore struct{}
```
被导出但仅实现了一个接口

Concurrency Patterns

并发模式

Does batch+callback pattern protect against concurrent writes?
Does
```
commit()
```
only remove specific items, not clear all?
Are loop variables using outdated patterns? (Go 1.22+ doesn't need cloning)
- No
```
i := i
```
  reassignment inside loops
- No closure arguments for loop variables:
```
go func(id int) { }(i)
```
Red flag:
```
s.events = nil
```
in commit callback
Red flag:
```
go func(x int) { ... }(loopVar)
```
- closure argument unnecessary since Go 1.22

批量+回调模式是否防止了并发写入？
```
commit()
```
是否仅删除特定项而非清空全部？
循环变量是否使用了过时模式？（Go 1.22+无需克隆）
- 循环内无
```
i := i
```
  重赋值
- 循环变量无闭包参数：
```
go func(id int) { }(i)
```
危险信号：commit回调中存在
```
s.events = nil
```
危险信号：
```
go func(x int) { ... }(loopVar)
```
——Go 1.22+无需闭包参数

Resource Management

资源管理

Is
```
defer f.Close()
```
placed AFTER error check?
Are database connection pools shared, not duplicated?
Is file traversal done once, not repeated for size calculation?
Red flag:
```
defer f.Close()
```
immediately after
```
os.OpenFile()
```

```
defer f.Close()
```
是否放在错误检查之后？
数据库连接池是否共享而非重复创建？
文件遍历是否仅执行一次而非重复计算大小？
危险信号：
```
os.OpenFile()
```
后立即执行
```
defer f.Close()
```

Metrics & Observability

指标与可观测性

Are Prometheus counter metrics pre-initialized with
```
.Add(0)
```
?
Are all known label combinations initialized at startup?
Red flag: CounterVec registered but not initialized

Prometheus计数器指标是否通过
```
.Add(0)
```
预初始化？
是否在启动时初始化了所有已知的标签组合？
危险信号：已注册CounterVec但未初始化

Testing Patterns

测试模式

Are interface implementation tests deduplicated?
Do tests use
```
assert.Equal
```
(no reflection) for comparable types?
Does test setup use
```
prometheus.NewPedanticRegistry()
```
?
Red flag: Copy-pasted tests for FileStore, MemoryStore, SQLStore

接口实现测试是否已去重？
测试是否对可比较类型使用
```
assert.Equal
```
（无反射）？
测试设置是否使用
```
prometheus.NewPedanticRegistry()
```
？
危险信号：FileStore、MemoryStore、SQLStore的测试存在复制粘贴

Code Organization

代码组织

Is function extraction justified (reuse or complexity hiding)?
Are unnecessary helper functions wrapping stdlib calls?
Red flag: Helper that just calls through to another function

函数提取是否合理（复用或隐藏复杂度）？
是否存在不必要的包装标准库调用的辅助函数？
危险信号：仅直接调用其他函数的辅助函数

Organization Library Ecosystem Patterns

组织级库生态系统模式

When reviewing projects that use shared organization libraries, apply these additional checks:

评审使用组织级共享库的项目时，需额外执行以下检查:

Library Usage

库使用

Are optional fields using the organization's preferred option type?
Is SQL iteration using helper functions instead of manual
```
rows.Next()
```
loops?
Are tests using the organization's assertion helpers?
Red flag: Manual SQL row iteration with defer/Next/Scan/Err pattern when helpers exist

可选字段是否使用组织偏好的option类型？
SQL迭代是否使用辅助函数而非手动
```
rows.Next()
```
循环？
测试是否使用组织的断言辅助函数？
危险信号：当存在辅助函数时仍手动实现SQL行迭代的defer/Next/Scan/Err模式

Test Assertions

测试断言

Is the correct assertion function used for the type being compared?
Is deep comparison only used for non-comparable types (slices, maps, structs)?
Red flag: Deep comparison used for simple types like int, string, bool

是否为要比较的类型使用了正确的断言函数？
是否仅对不可比较类型（切片、映射、结构体）使用深度比较？
危险信号：对int、string、bool等简单类型使用深度比较

Test Infrastructure

测试基础设施

Are DB tests using the organization's test database helpers?
Are Prometheus tests using
```
NewPedanticRegistry()
```
?
Red flag: Raw
```
sql.Open()
```
in test setup instead of test helpers

数据库测试是否使用组织的测试数据库辅助函数？
Prometheus测试是否使用
```
NewPedanticRegistry()
```
？
危险信号：测试设置中使用原始
```
sql.Open()
```
而非测试辅助函数

Dead Code

死代码

Are there leftover
```
*_migration.sql
```
files without usage?
Are there helper functions that just wrap single stdlib calls?
Are there redundant checks (e.g., empty string check before regex)?
Red flag: Wrapper functions that add no value over the underlying call

是否存在未使用的
```
*_migration.sql
```
文件？
是否存在仅包装单个标准库调用的辅助函数？
是否存在冗余检查（例如正则表达式前的空字符串检查）？
危险信号：对底层调用无任何增值的包装函数

Database Naming

数据库命名

Do functions using database-specific syntax indicate this in names?
Red flag: Generic
```
SQLStoreFactory
```
that uses database-specific syntax

使用数据库特定语法的函数是否在名称中体现？
危险信号：通用的
```
SQLStoreFactory
```
却使用数据库特定语法

Receiving Review Feedback

接收评审反馈

When YOU are the one receiving code review feedback (not giving it), apply these patterns:

当你作为代码作者接收评审反馈时，需遵循以下模式:

The Reception Pattern

接收模式

WHEN receiving code review feedback:

1. READ: Complete feedback without reacting
2. UNDERSTAND: Restate requirement in own words (or ask)
3. VERIFY: Check against codebase reality
4. EVALUATE: Technically sound for THIS codebase?
5. RESPOND: Technical acknowledgment or reasoned pushback
6. IMPLEMENT: One item at a time, test each

当接收代码评审反馈时:

1. 阅读：完整阅读反馈后再做出反应
2. 理解：用自己的话重述要求（或提问）
3. 验证：对照代码库实际情况检查
4. 评估：对**本代码库**是否技术合理？
5. 回应：技术层面的确认或有依据的反驳
6. 实现：逐项实现，每项都要测试

No Performative Agreement

避免形式化同意

NEVER:

"You're absolutely right!"
"Great point!" / "Excellent feedback!"
"Thanks for catching that!"

INSTEAD:

Restate the technical requirement
Ask clarifying questions
Push back with technical reasoning if wrong
Just start working (actions > words)

When feedback IS correct:

"Fixed. [Brief description of what changed]"
"Good catch - [specific issue]. Fixed in [location]."
[Just fix it and show in the code]

绝不要说:

"你完全正确！"
"好观点！" / "很棒的反馈！"
"感谢你发现这个问题！"

应该说:

重述技术要求
提出澄清问题
若反馈错误，用技术理由反驳
直接开始行动（行动胜于言语）

当反馈正确时:

"已修复。[变更内容简要描述]"
"发现问题——[具体问题]。已在[位置]修复。"
[直接修复并在代码中展示]

YAGNI Check for "Professional" Features

对「专业」功能的YAGNI检查

IF reviewer suggests "implementing properly":
  grep codebase for actual usage

  IF unused: "This endpoint isn't called. Remove it (YAGNI)?"
  IF used: Then implement properly

若评审者建议「正确实现」:
  在代码库中搜索实际使用情况

  若未使用: "此端点未被调用。是否移除（YAGNI原则）？"
  若已使用: 则正确实现

Handling Unclear Feedback

处理模糊的反馈

IF any item is unclear:
  STOP - do not implement anything yet
  ASK for clarification on unclear items

WHY: Items may be related. Partial understanding = wrong implementation.

Example:

Reviewer: "Fix items 1-6"
You understand 1,2,3,6. Unclear on 4,5.

WRONG: Implement 1,2,3,6 now, ask about 4,5 later
RIGHT: "I understand items 1,2,3,6. Need clarification on 4 and 5 before proceeding."

若任何内容模糊:
  停止——暂不实现任何内容
  对模糊内容提问澄清

原因: 内容可能相关。部分理解会导致错误实现。

示例:

评审者: "修复项1-6"
你理解项1、2、3、6，但对项4、5有疑问。

错误做法: 先实现项1、2、3、6，之后再询问项4、5
正确做法: "我理解项1、2、3、6。在开始前需要澄清项4和5的内容。"

When to Push Back

何时反驳

Push back when:

Suggestion breaks existing functionality
Reviewer lacks full context
Violates YAGNI (unused feature)
Technically incorrect for this stack
Legacy/compatibility reasons exist

How to push back:

Use technical reasoning, not defensiveness
Ask specific questions
Reference working tests/code

Example:

Reviewer: "Remove legacy code"
WRONG: "You're absolutely right! Let me remove that..."
RIGHT: "Checking... build target is 10.15+, this API needs 13+. Need legacy for backward compat. Fix bundle ID or drop pre-13 support?"

当出现以下情况时可反驳:

建议会破坏现有功能
评审者缺乏完整上下文
违反YAGNI原则（未使用的功能）
对本技术栈在技术上不正确
存在遗留/兼容性原因

反驳方式:

使用技术理由而非防御性言辞
提出具体问题
引用可运行的测试/代码

示例:

评审者: "移除遗留代码"
错误做法: "你完全正确！我这就移除..."
正确做法: "正在检查...构建目标为10.15+，但此API需要13+。为了向后兼容需要保留遗留代码。是否修改Bundle ID或放弃13以下版本的支持？"

Implementation Order

实现顺序

FOR multi-item feedback:
  1. Clarify anything unclear FIRST
  2. Then implement in this order:
     - Blocking issues (breaks, security)
     - Simple fixes (typos, imports)
     - Complex fixes (refactoring, logic)
  3. Test each fix individually
  4. Verify no regressions

对于多项目反馈:
  1. 首先澄清所有模糊内容
  2. 然后按以下顺序实现:
     - 阻塞性问题（破坏功能、安全问题）
     - 简单修复（拼写错误、导入）
     - 复杂修复（重构、逻辑）
  3. 每项修复都要单独测试
  4. 验证是否存在回归

External vs Internal Reviewers

外部评审者 vs 内部评审者

From external reviewers:

BEFORE implementing:
  1. Check: Technically correct for THIS codebase?
  2. Check: Breaks existing functionality?
  3. Check: Reason for current implementation?
  4. Check: Does reviewer understand full context?

IF suggestion seems wrong:
  Push back with technical reasoning

IF can't easily verify:
  Say so: "I can't verify this without [X]. Should I investigate/proceed?"

来自外部评审者的反馈:

在实现前:
  1. 检查: 对**本代码库**是否技术正确？
  2. 检查: 是否会破坏现有功能？
  3. 检查: 当前实现的原因？
  4. 检查: 评审者是否理解完整上下文？

若建议似乎错误:
  用技术理由反驳

若无法轻松验证:
  说明情况: "没有[X]我无法验证。是否需要我调查/继续？"

References

参考资料

This skill uses these shared patterns:

Anti-Rationalization - Prevents shortcut rationalizations
Anti-Rationalization (Review) - Review-specific rationalizations
Verification Checklist - Pre-completion checks
Severity Classification - Issue severity definitions

本技能使用以下共享模式:

反合理化（核心） - 防止捷径式合理化
反合理化（评审） - 评审特定的合理化
验证检查清单 - 完成前检查
严重程度分类 - 问题严重程度定义

Domain-Specific Anti-Rationalization

领域特定反合理化

Rationalization	Why It's Wrong	Required Action
"Code looks correct to me"	Visual inspection misses runtime issues	Run tests in Phase 2
"Comment says it's thread-safe"	Comments can be wrong or outdated	Verify claims against code
"Only a small change"	Small changes cause large regressions	Complete all 4 phases
"Tests pass, ship it"	Tests may not cover the changed paths	Verify coverage of changes

合理化理由	错误原因	要求操作
"代码看起来是正确的"	视觉检查会遗漏运行时问题	在阶段2中运行测试
"注释说它是线程安全的"	注释可能错误或过时	对照代码验证声明
"只是小变更"	小变更会导致大的回归	完成全部4个阶段
"测试通过，发布吧"	测试可能未覆盖变更路径	验证变更的测试覆盖率