code-review-axes-and-quality

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review Axes and Quality

代码评审维度与质量

Use this skill for a structured review of a commit range, feature, module, or working tree. It uses delegation mode as the execution layer and produces durable plans instead of a chat-only opinion.
本技能用于对提交范围、功能、模块或工作树进行结构化评审。它以delegation-mode作为执行层,生成可持久化的计划,而非仅提供聊天式意见。

Relevant companion skill

相关配套技能

Use
delegation-mode
as the execution layer for this skill. Before creating or running a review plan, read:
text
../delegation-mode/SKILL.md
Follow its worker contract, stable task IDs,
.agents/plans/
output convention, sequential/parallel selection rules, validated result-file marker, and cross-session
DONE_IDS
resume procedure. This skill defines review quality;
delegation-mode
defines how the review is dispatched and resumed. The executable review template also validates every axis artifact and rejects a final approval that ignores unresolved CRITICAL or HIGH findings.
The review template uses the shared
delegation-mode
validation core for plan identity, safe paths,
DONE_IDS
, frontmatter, completion markers, and durable file checks. Review-specific axis requirements and verdict/blocker checks remain local to this template.
使用
delegation-mode
作为本技能的执行层。创建或运行评审计划前,请阅读:
text
../delegation-mode/SKILL.md
遵循其中的工作契约、稳定任务ID、
.agents/plans/
输出约定、顺序/并行选择规则、验证结果文件标记,以及跨会话的
DONE_IDS
恢复流程。本技能定义评审质量标准;
delegation-mode
定义评审的调度与恢复方式。可执行评审模板还会验证每个维度的产物,若忽略未解决的CRITICAL或HIGH级问题,则会拒绝最终审批。
评审模板使用
delegation-mode
的共享验证核心来处理计划标识、安全路径、
DONE_IDS
、前置信息、完成标记和持久化文件检查。评审特有的维度要求、结论/阻塞项检查则保留在本模板内。

Review standard

评审标准

Review every change before it is merged, but keep the approval bar proportional: approve a change when it improves overall code health and has no unresolved blocking issue. Do not block on personal style preferences or perfect-code expectations. Do not rubber-stamp because tests pass or because the author is confident.
Every finding must be evidence-based, actionable, and ordered by leverage: correctness and security first, then structural regressions and missed simplifications, then lower-impact concerns. If a structural problem is reported, propose the move that fixes it rather than only describing that the code is complex.
所有变更在合并前都需经过评审,但审批门槛应与变更规模匹配:当变更能提升整体代码健康度且无未解决的阻塞问题时,方可批准。不要因个人风格偏好或完美代码预期而阻塞变更;也不要因为测试通过或作者自信就草率批准。
所有问题必须基于证据、可执行,并按影响优先级排序:正确性和安全性优先,其次是结构性退化和遗漏的简化机会,最后是低影响的问题。若发现结构性问题,应提出修复方案,而非仅描述代码复杂度。

Fixed review axes

固定评审维度

Run all ten axes, including axes with no findings:
#AxisFocus
1CorrectnessBugs, edge cases, incorrect assumptions, regressions
2PerformanceBottlenecks, unnecessary work, latency, memory, hot paths
3ScalabilityFailure or degradation under growth, concurrency, larger inputs
4InfrastructureExisting project wiring, boundaries, operations
5Library hygieneDependencies, supported APIs, versions, usage
6SecurityInput handling, auth, secrets, injection, SSRF, XSS, abuse
7Data integrity and concurrencyAtomicity, idempotency, races, ordering, retries
8ObservabilityLogs, metrics, traces, alerts, diagnostics, failure visibility
9MaintainabilityCoupling, cohesion, change risk, duplication, architecture
10ReadabilityNaming, control flow, local comprehension, cognitive load
Maintainability and readability are separate: maintainability evaluates whether future changes are safe and localized; readability evaluates whether current behavior is understandable without reconstructing hidden intent.
Every finding includes:
  • what is wrong;
  • file and line or symbol;
  • why it matters;
  • recommended fix;
  • severity:
    CRITICAL
    ,
    HIGH
    ,
    MEDIUM
    , or
    LOW
    .
Every axis result also includes the exact scope, assigned axis, and actual verification evidence. An empty axis explicitly states that no findings were found. A marker-only file is not a completed review result.
An empty axis is complete only when it states that the reviewed scope had no findings.
需执行全部十个维度,即使部分维度未发现问题:
#维度关注点
1Correctness(正确性)漏洞、边缘场景、错误假设、回归问题
2Performance(性能)瓶颈、不必要的操作、延迟、内存占用、热点路径
3Scalability(可扩展性)数据量、并发量、输入规模增长时的故障或性能退化
4Infrastructure(基础设施)现有项目架构、边界、运维相关内容
5Library hygiene(库卫生)依赖项、受支持的API、版本、使用方式
6Security(安全性)输入处理、认证、密钥、注入攻击、SSRF、XSS、滥用风险
7Data integrity and concurrency(数据完整性与并发)原子性、幂等性、竞态条件、顺序、重试机制
8Observability(可观测性)日志、指标、链路追踪、告警、诊断、故障可见性
9Maintainability(可维护性)耦合度、内聚性、变更风险、代码重复、架构设计
10Readability(可读性)命名、控制流、本地代码理解难度、认知负荷
可维护性与可读性是两个独立维度:可维护性评估未来变更是否安全且局部可控;可读性评估当前行为是否无需推测隐藏意图即可理解。
每个问题需包含:
  • 问题内容;
  • 文件路径、行号或符号;
  • 影响原因;
  • 推荐修复方案;
  • 严重程度:
    CRITICAL
    HIGH
    MEDIUM
    LOW
每个维度的结果还需包含明确的评审范围、分配的维度,以及实际的验证证据。无问题的维度需明确说明未发现问题。仅含标记的文件不算完整的评审结果。
仅当无问题的维度明确说明评审范围内未发现问题时,该维度才算完成。

Axis guidance

维度指导

Use the following lenses while staying within each assigned axis:
  • Correctness: Compare behavior with the specification and tests. Check null, empty, boundary, retry, timeout, and error paths, not only the happy path. Look for state inconsistencies, off-by-one errors, and regressions.
  • Performance: Check N+1 queries, unbounded work or fetching, avoidable synchronous operations, hot-path allocations, and UI re-render costs.
  • Scalability: Check behavior as data, traffic, tenants, concurrency, and payload size grow. Look for resource limits, queue/backpressure behavior, contention, and fan-out.
  • Infrastructure: Check project wiring, deployment/runtime assumptions, configuration, migrations, operational boundaries, rollback behavior, and failure recovery.
  • Library hygiene: Prefer the existing stack and standard library. Review new dependencies for maintenance, license, vulnerability, bundle/runtime impact, supported APIs, changelog/migration notes, and lockfile changes.
  • Security: Treat user input, external data, logs, and configuration as untrusted at boundaries. Check validation, authorization, secrets, injection, XSS, SSRF, abuse controls, and dependency supply-chain risk.
  • Data integrity and concurrency: Check atomicity, idempotency, ordering, retries, races, transaction boundaries, duplicate work, and partial failure.
  • Observability: Check useful structured logs, metrics, traces, alerts, diagnostics, and error visibility for important success and failure paths.
  • Maintainability: Look for coupling, duplication, feature logic in shared modules, circular dependencies, and abstractions that relocate rather than reduce complexity. Question unclear type boundaries and silent fallbacks.
  • Readability: Check names, control flow, local comprehension, dead code, comments, file size, and conditional branches bolted onto unrelated flows.
在每个分配的维度内,可参考以下视角:
  • 正确性:对比行为与需求规格和测试用例。检查空值、边界、重试、超时和错误路径,而非仅检查正常路径。留意状态不一致、差一错误和回归问题。
  • 性能:检查N+1查询、无界操作或数据获取、可避免的同步操作、热点路径内存分配,以及UI重渲染成本。
  • 可扩展性:检查数据量、流量、租户数、并发量和负载大小增长时的行为。留意资源限制、队列/背压机制、资源竞争和扇出问题。
  • 基础设施:检查项目架构、部署/运行时假设、配置、迁移、运维边界、回滚行为和故障恢复机制。
  • 库卫生:优先使用现有技术栈和标准库。评审新依赖的维护状态、许可证、漏洞、包大小/运行时影响、受支持的API、更新日志/迁移说明,以及锁文件变更。
  • 安全性:在边界处将用户输入、外部数据、日志和配置视为不可信。检查验证、授权、密钥管理、注入攻击、XSS、SSRF、滥用控制,以及依赖供应链风险。
  • 数据完整性与并发:检查原子性、幂等性、顺序、重试、竞态条件、事务边界、重复操作和部分故障场景。
  • 可观测性:检查重要成功与失败路径的结构化日志、指标、链路追踪、告警、诊断和错误可见性。
  • 可维护性:留意耦合、代码重复、共享模块中的业务逻辑、循环依赖,以及仅转移复杂度而非降低复杂度的抽象。质疑不清晰的类型边界和静默降级逻辑。
  • 可读性:检查命名、控制流、本地代码理解难度、死代码、注释、文件大小,以及附加在无关流程上的条件分支。

Review protocol

评审流程

Use this order for every review:
  1. Understand intent. Read the task/spec, relevant plans, current checkout, and recent commits. State the expected behavior change and exact scope.
  2. Read tests first. Identify tests for the change, check that they test behavior rather than implementation details, and note missing regression or edge-case coverage.
  3. Read complete implementation context. Do not review only diff hunks. Trace callers, canonical helpers, type boundaries, configuration, and dependencies needed to understand the behavior.
  4. Run all ten axes. Keep axes separate, including axes with no findings. Do not repeat the same finding across axes unless the impact is genuinely different; the final report will deduplicate overlaps.
  5. Verify the verification story. Record tests, lint, type checks, builds, manual checks, screenshots for UI changes, benchmarks, and dependency checks that were actually run. Do not claim a check that was not run.
  6. Give a verdict. Approve only when no unresolved
    CRITICAL
    or
    HIGH
    finding remains. If lower-severity work is deferred, record the rationale, owner/next action when known, and residual risk.
每次评审需遵循以下顺序:
  1. 理解意图:阅读任务/需求规格、相关计划、当前检出代码和近期提交。说明预期的行为变更和明确的评审范围。
  2. 先读测试用例:识别变更对应的测试用例,检查测试是否针对行为而非实现细节,并记录缺失的回归或边缘场景覆盖。
  3. 阅读完整实现上下文:不要仅评审差异片段。追踪调用方、标准工具函数、类型边界、配置和理解行为所需的依赖项。
  4. 执行全部十个维度:保持维度独立,即使部分维度未发现问题。除非影响确实不同,否则不要在多个维度重复记录同一问题;最终报告将自动去重。
  5. 验证验证过程:记录实际执行的测试、代码检查、类型检查、构建、手动检查、UI变更截图、基准测试和依赖检查。不要声称执行了未实际运行的检查。
  6. 给出结论:仅当无未解决的
    CRITICAL
    HIGH
    级问题时方可批准。若低严重程度的工作被推迟,需记录理由、已知的负责人/下一步行动,以及剩余风险。

Finding severity and status

问题严重程度与状态

Use these severity meanings consistently:
SeverityMeaningDefault decision
CRITICAL
Exploitable security issue, data loss, broken core behavior, or an immediate production blockerBlocks approval
HIGH
Likely bug, serious security/contract issue, or structural regression with material impactNormally blocks approval
MEDIUM
Material maintainability, performance, scalability, observability, or correctness gap with bounded impactFix before merge when in scope, otherwise document deferral
LOW
Limited-impact improvement, polish, or localized cleanupOptional unless project rules say otherwise
Use the existing
Status
column for the action state:
required
,
recommended
,
optional
,
accepted-risk
, or
resolved
. A finding must not be labeled
resolved
unless the reviewed scope contains evidence that it was fixed and verified.
需统一使用以下严重程度定义:
严重程度含义默认决策
CRITICAL
可被利用的安全问题、数据丢失、核心功能故障,或直接阻塞生产环境的问题阻止审批
HIGH
可能存在的漏洞、严重的安全/契约问题,或有实质性影响的结构性退化通常阻止审批
MEDIUM
有实质性影响但范围有限的可维护性、性能、可扩展性、可观测性或正确性缺陷若在变更范围内则需在合并前修复,否则需记录推迟理由
LOW
影响有限的改进、优化或局部清理可选,除非项目规则另有规定
使用现有的
Status
列标记行动状态:
required
(必填)、
recommended
(推荐)、
optional
(可选)、
accepted-risk
(接受风险)或
resolved
(已解决)。除非评审范围内有证据表明问题已修复并验证,否则不得标记为
resolved

Structural remedies

结构性修复方案

When the issue is structural, name a concrete remedy. Prefer the option that removes moving pieces:
  • replace a conditional chain with a typed model or explicit dispatcher;
  • collapse duplicate branches into one clear flow;
  • separate orchestration from business logic;
  • move feature-specific logic into the package that owns it;
  • reuse the canonical helper instead of adding a near-duplicate;
  • make a type boundary explicit so downstream branching disappears;
  • delete a pass-through wrapper that adds indirection without clarifying an API;
  • extract a helper or split a large file into focused modules.
Do not recommend a broad abstraction simply because code is repeated once. Generalize when a real shared invariant or third use case justifies it.
当问题属于结构性问题时,需给出具体的修复方案。优先选择减少复杂度的方案:
  • 用类型化模型或显式调度器替换条件链;
  • 将重复分支合并为清晰的单一流程;
  • 将编排逻辑与业务逻辑分离;
  • 将特定功能的逻辑移至所属包内;
  • 复用标准工具函数,而非添加近似重复的代码;
  • 明确类型边界,消除下游分支逻辑;
  • 删除仅增加间接性而未明确API的透传包装器;
  • 提取工具函数或将大文件拆分为聚焦的模块。
不要仅因代码重复一次就推荐宽泛的抽象。仅当存在真正的共享不变量或第三个用例时才进行泛化。

Change sizing and decomposition

变更规模与拆分

Review the resulting file size and change size, not only whether the diff passes tests:
text
~100 changed lines   → easy to review
~300 changed lines   → acceptable for one logical change
~1000 changed lines  → split the change
~1000 total lines in one file → inspection signal; consider decomposition
These are inspection signals, not automatic rejection rules. Recommend stack, by-file-group, horizontal, or vertical decomposition when a change is too large. Separate refactoring from new behavior unless a small cleanup is necessary to make the feature safe.
评审最终的文件大小和变更规模,而非仅关注差异是否通过测试:
text
~100行变更 → 易于评审
~300行变更 → 单一逻辑变更可接受
~1000行变更 → 拆分变更
~1000行的单个文件 → 需关注,考虑拆分
这些是参考信号,而非自动拒绝规则。当变更过大时,建议按栈、文件组、横向或纵向拆分。除非小范围清理是确保功能安全的必要操作,否则应将重构与新功能分离。

Dead code and dependency hygiene

死代码与依赖卫生

After refactors, identify unreachable or unused code explicitly. Axis workers must not silently delete uncertain code; list the symbol and why it appears unused, then recommend a separate removal or a verified cleanup decision.
For dependency changes, check whether the existing stack already solves the problem, package maintenance and license, known vulnerabilities, bundle or runtime impact, supported APIs, changelog/migration notes, and the lockfile diff. Prefer one dependency upgrade per logical change and never hand-edit a lockfile.
重构后,需明确标识不可达或未使用的代码。维度处理程序不得静默删除存疑代码;需列出符号及看似未使用的原因,然后建议单独移除或进行验证后的清理决策。
对于依赖变更,检查现有技术栈是否已解决该问题、包的维护状态和许可证、已知漏洞、包大小或运行时影响、受支持的API、更新日志/迁移说明,以及锁文件差异。优先每次逻辑变更仅升级一个依赖,且绝不手动编辑锁文件。

Establish scope before dispatch

调度前确定范围

Before creating the plan:
  1. Read
    .agents/chats/current.md
    , when present.
  2. Read relevant active or reference plans.
  3. Inspect the current checkout and recent commits.
  4. Determine the exact review range or working-tree scope.
  5. Read project rules and relevant memory topics.
Verify saved handoffs against the current checkout. Never trust a stale branch, file, commit, or validation claim.
创建计划前:
  1. 若存在
    .agents/chats/current.md
    ,请阅读该文件。
  2. 阅读相关的活跃或参考计划。
  3. 检查当前检出代码和近期提交。
  4. 确定明确的评审范围或工作树范围。
  5. 阅读项目规则和相关记忆主题。
验证保存的交接信息与当前检出代码是否一致。切勿信任过时的分支、文件、提交或验证声明。

Run the reusable plan

运行可复用计划

Copy
scripts/review-plan.template.js
to:
text
.agents/plans/YYYY-MM-DD-<slug>.js
Fill in
DATE
,
SLUG
,
PROJECT_CONTEXT
, and the review scope. Choose:
js
const MODE = "sequential"; // or "parallel"
Each axis writes one flat plan file:
text
YYYY-MM-DD-<slug>-01-correctness.md
...
YYYY-MM-DD-<slug>-10-readability.md
YYYY-MM-DD-<slug>-FINAL.md
Axis workers must be
config: { $kind: "general" }
because they write files. They must read complete relevant files, avoid modifying application source, and include the exact marker in the file and end only after writing their exact file:
text
FILE_WRITTEN: <exact-path>
The runner treats a missing marker as unresolved and does not aggregate until all ten axes confirm.
Each axis result must include:
  • exact reviewed scope and relevant evidence;
  • the assigned axis and explicit no-findings statement when empty;
  • findings with severity, location, impact, recommended fix, and status;
  • verification performed or unavailable;
  • structural remedy, dead-code note, or dependency note when relevant;
    • FILE_WRITTEN: <exact-path>
      as the final non-empty line in the file and response. The dispatcher confirms the file itself, not the response alone.
scripts/review-plan.template.js
复制到:
text
.agents/plans/YYYY-MM-DD-<slug>.js
填写
DATE
SLUG
PROJECT_CONTEXT
和评审范围。选择:
js
const MODE = "sequential"; // 或 "parallel"
每个维度会生成一个扁平化的计划文件:
text
YYYY-MM-DD-<slug>-01-correctness.md
...
YYYY-MM-DD-<slug>-10-readability.md
YYYY-MM-DD-<slug>-FINAL.md
维度处理程序必须设置为
config: { $kind: "general" }
,因为它们需要写入文件。处理程序必须读取完整的相关文件,避免修改应用源代码,并在文件中包含明确标记,且仅在写入完确切文件后结束:
text
FILE_WRITTEN: <exact-path>
运行器会将缺失标记视为未完成,直到所有十个维度确认完成后才会汇总结果。
每个维度的结果必须包含:
  • 明确的评审范围和相关证据;
  • 分配的维度,若为空则需明确说明未发现问题;
  • 包含严重程度、位置、影响、推荐修复方案和状态的问题;
  • 已执行或不可用的验证操作;
  • 相关的结构性修复方案、死代码说明或依赖说明;
    • FILE_WRITTEN: <exact-path>
      作为文件和响应的最后一行非空内容。调度器会验证文件本身,而非仅依赖响应。

Resume across sessions

跨会话恢复

After an interrupted run:
bash
ls .agents/plans/YYYY-MM-DD-<slug>-*.md 2>/dev/null
Inspect the output files and update
DONE_IDS
with only confirmed axis IDs:
js
const DONE_IDS = [
  "01-correctness",
  "02-performance",
];
Rerun the complete plan. It skips those IDs and dispatches only missing axes. Set
FINAL_DONE = true
only after verifying the final report exists and is complete. Do not infer completion from chat silence or a partial file.
运行中断后:
bash
ls .agents/plans/YYYY-MM-DD-<slug>-*.md 2>/dev/null
检查输出文件,并仅将已确认完成的维度ID更新到
DONE_IDS
js
const DONE_IDS = [
  "01-correctness",
  "02-performance",
];
重新运行完整计划。计划会跳过已完成的ID,仅调度未完成的维度。仅在验证最终报告存在且完整后,设置
FINAL_DONE = true
。不要通过聊天沉默或部分文件推断完成状态。

Final report

最终报告

The final plan deduplicates findings, sorts the master table by severity, groups concrete priority work items, and gives the next agent enough context to act. It must also include:
  • a concise context and intent summary;
  • a verdict:
    APPROVE
    ,
    APPROVE_WITH_FOLLOW_UP
    , or
    REQUEST_CHANGES
    ;
  • unresolved blockers and deferred lower-severity risks;
  • verification evidence and explicit gaps;
  • change-size/decomposition, dependency, and dead-code conclusions when applicable.
It does not change application code.
After the review:
  1. Verify all ten axis files and the final report exist.
  2. Check frontmatter and relative links.
  3. Run applicable tests, lint, type checks, compilation, and build.
  4. Run
    git diff --check
    .
  5. Record verification and unresolved risks in the final plan.
  6. Confirm no
    CRITICAL
    or
    HIGH
    finding is marked
    resolved
    without evidence.
  7. Do not claim approval while unresolved
    CRITICAL
    or
    HIGH
    findings remain. The template enforces this postcondition; the final report must contain a machine-readable
    **Verdict:**
    line.
最终计划会对问题去重,按严重程度排序主表,分组具体的优先级工作项,并为后续处理人员提供足够的上下文。最终报告还必须包含:
  • 简洁的上下文和意图摘要;
  • 结论:
    APPROVE
    (批准)、
    APPROVE_WITH_FOLLOW_UP
    (批准并跟进)或
    REQUEST_CHANGES
    (要求变更);
  • 未解决的阻塞项和推迟的低严重程度风险;
  • 验证证据和明确的缺口;
  • 适用时的变更规模/拆分、依赖和死代码结论。
最终报告不得修改应用代码。
评审完成后:
  1. 验证所有十个维度文件和最终报告是否存在。
  2. 检查前置信息和相对链接。
  3. 运行适用的测试、代码检查、类型检查、编译和构建。
  4. 运行
    git diff --check
  5. 在最终计划中记录验证操作和未解决的风险。
  6. 确认无
    CRITICAL
    HIGH
    级问题在无证据的情况下被标记为
    resolved
  7. 若存在未解决的
    CRITICAL
    HIGH
    级问题,不得声称批准。模板会强制执行此后置条件;最终报告必须包含机器可读的
    **Verdict:**
    行。