SuperAPI AI Directory
Partner中文AI newsFavorites
Skillscode-review

code-review

Testing & quality

View repository

Updated 10-16 04:57

About this skill

Code Review

Perform a dual-axis read-only review of a fixed code scope:

  • Standards: Does the code conform to the engineering standards recorded in the repository?
  • Spec: Does the code faithfully implement the requirement source?

The two dimensions are reviewed in parallel by mutually independent sub-Agents, then aggregated in separate columns, avoiding context contamination and conclusion overshadowing between them. This Skill only reports issues; it does not modify code, commits, or remote state.

Process

1. Load project context

Use the already loaded CONTEXT terminology and RULE. If not yet loaded, follow the project knowledge protocol; if knowledge is unavailable, continue and state so truthfully.

Record the CONTEXT and RULE sources loaded this time. Provide relevant CONTEXT to the Standards Agent and Spec Agent, provide the complete RULE to the Standards Agent, and have the main Agent perform a final item-by-item check before aggregation. When the review scope expands, reload project knowledge according to the new complete scope.

2. Fix the review scope

Take the commit, branch, tag, or range specified by the user as the fixed point, and first verify that the reference exists. For branch ranges use git diff <fixed-point>...HEAD, and record git log <fixed-point>..HEAD --oneline。

When the user does not explicitly give a fixed point, choose a reproducible scope based on the request:

  • PR number or URL: use the code hosting connector or CLI to resolve the real base, head, and patch;
  • Current branch: check the upstream branch and default branch, use merge-base when it can be determined, otherwise ask;
  • Uncommitted changes: fix unstaged, staged, and untracked files at the same time;
  • Current implementation of a file or directory: fully read SNAPSHOT.md fixed file list and workspace state.

When the user also specifies a path, restrict the entire scope with the same pathspec. Stop if the reference is invalid, the diff is empty, or the snapshot has no valid files, and do not start sub-Agents.

3. Locate the Spec source

Find the requirement basis in the following order:

  1. The PRD, API list, task card, Issue, PR, or current conversation goal explicitly specified by the user;
  2. The requirement source referenced by the commit message or the changed content;
  3. A local requirement document that clearly corresponds to the requirements directory, branch name, or change topic.

Read the complete content that actually exists; do not guess based on file names. When there is no available requirement source, skip the Spec Agent and indicate "no available requirement basis" in the final report.

4. Locate the Standards source

Read the AGENTS.md、CLAUDE.md or equivalent Agent instructions in effect for the current scope, as well as the coding standards, testing conventions, and module constraints in the repository. Standards explicitly recorded by the project always take precedence; issues that tools can check deterministically are left to the tools.

On top of project standards, Standards always uses the following code smell baseline. They are risk signals requiring evidence and judgment, not hard violations; when project standards explicitly endorse a certain way of writing, do not report the corresponding signal.

  • Mysterious Name: The name cannot explain the data or behavior it holds. Change it to an honest name; being unable to name it usually indicates that the design is still unclear.
  • Duplicated Code: The same logical form appears in multiple changed locations. Extract the shared form and reuse it.
  • Feature Envy：A method operates on another object's data more than its own data. Move the behavior to the side of the data it depends on.
  • Data Clumps：The same set of fields or parameters is repeatedly passed together. Combine them into an explicit type.
  • Primitive Obsession：A string or number carries a domain concept that should have its own independent constraints. Create a type for that concept.
  • Repeated Switches：Branches around the same type or state are repeated in multiple places. Consolidate them into a map or polymorphic behavior.
  • Shotgun Surgery：One logical change forces synchronized modifications in many places. Consolidate the common change into one module.
  • Divergent Change：The same module changes for multiple unrelated reasons. Split responsibilities by reason for change.
  • Speculative Generality：Add abstraction, parameters, or extension points for future scenarios that do not exist in the current requirements. Remove them and pull back to real requirements.
  • Message Chains：The caller depends on an overly long object navigation chain. Hide the navigation behind a method provided by the first object.
  • Middle Man：The module mainly passes calls through and hides no complexity. Remove the valueless intermediate layer.
  • Refused Bequest：A subclass or implementation ignores or rejects most of the inherited behavior. Remove the inappropriate inheritance and use composition instead.

When the current scope involves module responsibilities, interface surface area, seams, adapters, test surface, or depth, read codebase-design and hand its judgment criteria to the Standards Agent.

5. Execute two dimensions in parallel

Create the Standards Agent and the Spec Agent in parallel. Both receive the same fixed scope and commit list, but do not read each other's intermediate conclusions and do not modify files.

Standards Agent also receives:

  • project specification sources;
  • the complete code smell baseline above;
  • the loaded RULEs;
  • the judgment criteria for codebase-design decided to be loaded this time.

It first compares all loaded RULEs one by one against the complete review scope, marks each RULE as "compliant", "not applicable", or "violated", preserves file and line number evidence, and returns a one-by-one checklist to the main Agent; if any RULE is not checked, the Standards review is considered incomplete. It then reports project specification violations and risk signals with actual impact file by file or hunk by hunk. Each finding cites a specification or naming risk signal and provides a compact code location and specific impact; it clearly distinguishes hard violations from judgment calls and skips items already checked by tools.

Spec Agent receives the complete requirement sources and relevant CONTEXT, and reports:

  • missing or only partially completed requirements;
  • scope creep not required by the requirements;
  • requirements that appear implemented but behave incorrectly.

Each finding must cite the corresponding requirement basis. If there is no Spec source, do not create a Spec Agent.

6. Main Agent review and final RULE check

Before aggregation, the main Agent independently reviews each candidate finding from the two sub-Agents: re-read the cited code, context, specification, or requirement basis, trace relevant call paths when necessary, and keep only issues that can be directly proven by the fixed review scope and have specific impact. Delete candidates with insufficient evidence, reliance on unverified assumptions, out-of-scope issues, or issues that do not actually hold; based on the review results, correct the issue description, code location, and recommendation, and independently determine severity, which may be raised or lowered from the P0 to P3given by the sub-Agent. Review is not forwarding as-is; if the one-by-one review is not completed, do not output the final report.

The main Agent re-confirms that the Standards Agent's item-by-item check covered all loaded RULEs, and personally compares all RULEs against the complete review scope item by item, marking them as compliant, not applicable, or violated, retaining file and line number evidence. When the Standards Agent misses a check, redo it, and add missed violations to Standards; if the final check cannot be completed, state this clearly and do not claim that Standards passed.

The final report lists the item-by-item coverage status of Standards and the RULE files covered this time, and marks the main Agent's final check as "passed", "the above violations exist", or "not completed".

7. Aggregation

Under ## Standards and ## Spec respectively present the issues retained after the main Agent's review; do not output deleted candidates, and do not merge findings across dimensions.

Each retained finding is assigned a number that is unique, consecutive, and easy to reference in this report: Standards uses STD-001、STD-002, and Spec uses SPEC-001、SPEC-002. Numbers are generated after filtering and sorting are completed. Severity uses the main Agent's final determination, from P0 to P3：P0 blocks delivery or causes a major incident,P1 is highly likely to cause erroneous behavior,P2 is a local defect or obvious maintenance risk,P3 is a low-risk improvement. Use the following format:

评审范围：<固定点、diff 命令与路径；或 snapshot 的 HEAD、路径和文件数>

## Standards

- [STD-001] [P1] <问题标题> — `<文件:行>`
  - 证据：<项目规范或风险信号>
  - 影响：<实际风险>
  - 建议：<最小修复方向>

RULE 核对：Standards 已逐条覆盖 <RULE 文件列表 / 未完成>；主 Agent 终检 <通过 / 存在上述违反 / 未完成>

## Spec

- [SPEC-001] [P1] <问题标题> — `<文件:行>`
  - 需求依据：<对应需求条目>
  - 偏差：<遗漏、错误、scope creep 或验收缺口>
  - 建议：<最小修复方向>

When nothing is found, explicitly write "no issues found", and do not manufacture opinions to fill the report. Finally, use one line to separately summarize the number of findings retained after review in the two dimensions, and reference each one's most severe issue by number, without selecting a single cross-dimension winner.

Why split into two dimensions

A change can pass on one side but fail on the other:

  • fully complies with coding standards, but implements the wrong requirement:Standards passes, Spec fails。
  • fully implements the requirement, but violates project conventions:Spec passes, Standards fails。

Reporting separately can prevent one dimension from masking the other.