.agents/skills/deep-review/SKILL.md
Multi-dimensional code review built on independent subagents. Review breadth comes from parallel per-dimension reviewers; precision comes from an adversarial verification pass that falsifies findings before they reach the report.
Every design choice below serves one of these. When unsure how to execute a step, come back here.
confirmed / false_positive / need_more_context). Three-way verdicts beat confidence percentages: calibrated-sounding scores are unreliable as hard filters.| Mode | Trigger | What runs |
|---|---|---|
| Light (default) | Any ordinary review ask: "review this PR", an informal "look at this change for problems", a diff pasted for review — but not explain-only questions about a change | Main agent reviews inline against the Quick checklist section of each applicable dimension file. No subagents. |
| Deep | Explicit only: /deep-review, "run deep review", "full multi-agent review" | Full orchestration: per-dimension review subagents → pipelined verify subagents → structured report → interactive fix flow. |
Do not auto-escalate light to deep. Do not run deep mode for a casual "看看这个改动" — that is light mode.
Rules live in one place: references/dimensions/, one file per dimension. Both modes read the same files (light mode reads only the Quick checklist section; deep-mode subagents read the full file plus its listed rule sources).
| Dimension | id prefix | Covers | Verified? |
|---|---|---|---|
| code-style | style | naming, readability, dead code, comments, i18n hardcoding, UI-library and styling conventions | yes |
| logic | logic | logic correctness: edge cases, null, races, error handling, state machines, requirement deviation, test coverage | yes |
| business-logic | design | design judgment: framework misuse, best-practice violations, solution-weight mismatch, self-inflicted complexity | yes |
| reuse-architecture | reuse | duplicate implementations, unused existing patterns, extensibility, architectural boundaries | yes |
| performance | perf | N+1, blocking calls, resource leaks, render-path waste, DB migration locking and idempotency | yes |
| release-risk | risk | ship/no-ship gate: irreversible persisted state (incl. schemaless shape drift), dev-cycle migration residue, in-flight work & config at deploy time, irreversible outbound effects, prompt/tool-description behavior shifts, shared-surface and high-frequency-UI blast radius, PR purity | yes + checks |
| security | sec | injection, auth bypass, secret/PII leakage, business-slot confidentiality | yes |
| compatibility | compat | light/dark theme, desktop app / web (desktop, mobile) / RN, released-client API compatibility, client vs server agent runtime (gateway on/off), Vercel vs Docker deploys, paired router configs | yes |
| ux | ux | empty/loading/error states, async feedback, confirmation flows, design-value adherence | yes |
| observability | obs | debug: unexplained fixes, uncommented hacks, silent catches, missing logs; product: analytics on qualifying new capabilities; perf: monitoring on high-frequency / high-traffic / polling paths | yes |
| workflow | flow | issue tracking state, PR description freshness, undocumented key decisions, CI / preview build status | no (objective state) |
| skill-freshness | skill | agent skills invalidated by this diff, knowledge worth distilling into a new skill | no (advisory) |
Verified? no means findings from that dimension are objective state checks or advisories — they skip the verify pass and go straight to the report. yes + checks means the dimension's findings are verified normally, but it also emits a second, unverified output — release_checks, pre-deploy confirmation items about production state the repo cannot answer (see its dimension file).
Before spawning, the main agent prunes dimensions that cannot apply to the diff. List pruned dimensions and the one-line reason in the report header. When in doubt, run the dimension.
| Dimension | Skip when |
|---|---|
| code-style, logic, business-logic, reuse-architecture | never (skip only for docs/lockfile-only diffs) |
| performance | no server/db/loop/render-path code touched (e.g. docs, copy, pure type changes) |
| release-risk | no DB schema/migration, no schemaless persisted payload, no queue/cron/config/outbound dependency, no prompt or tool-description text, no shared component or package public API, no high-frequency user surface, and the diff has one obvious purpose |
| security | lockfile/generated-only diff — docs and copy still run it (text is a leak vector: secrets, internal URLs, commercial details) |
| compatibility | diff touches no UI theming/routing, no API contract, no deployment config, no runtime-branching code |
| ux | no user-facing surface changed (components, styles, copy, interaction flows) |
| observability | no error handling, async flow, server code, new user-facing capability, or perf-sensitive path touched |
| workflow | never in deep mode (cheap external-state checks) |
| skill-freshness | never in deep mode (cheap) |
"Docs-only" means human-facing prose only. Files that are executable instructions for agents — .agents/skills/**, AGENTS.md / CLAUDE.md, prompt templates, orchestration manuals — count as code for pruning purposes: their "prose" carries control flow, contracts, and rules whose contradictions are exactly what logic / business-logic / reuse-architecture exist to catch. A diff touching them is never docs-only.
Light mode applies the same table to decide which Quick checklists to read.
A wrapping repository (e.g. a private deployment that vendors this repo as a submodule) can extend the rule set without forking this skill: any sibling skill directory in the active skills root matching deep-review-* (for example .agents/skills/deep-review-cloud/) is an extension pack.
dimensions/*.md files in the same format; a file named after a built-in dimension extends it (load both), a new name adds a dimension.extends: <name> where no such built-in dimension exists (typically because the wrapping repo pins an older submodule commit) falls back to standing on its own. Such a file must therefore carry its own id_prefix / verify / skip_when so it stays usable either way; when the built-in is present, the built-in's frontmatter wins.Quick checklist section of each surviving dimension file, plus extension-pack counterparts.git blame / git log -L on the location) and offer to file a Linear issue for that owner. The only exception is an old bug this diff makes reachable or harmful for the first time, at P0.low-likelihood, non-blocking finding is a follow-up, not a fix-now item; on a repeat review of the same change, list it in one line and move on instead of re-arguing it.release-risk produced pre-deploy confirmation items, list them as a short checklist separate from the findings — they are questions about production state, not defects. Mention that deep mode exists if findings suggest the diff deserves a full pass.Pick the manual for the current environment and follow it end to end:
references/claude-code/main.mdreferences/codex/main.mdIf the environment is not listed, tell the user deep mode does not support it yet and offer light mode instead. Do not improvise another environment's mechanics, and do not degrade to a single-agent pass (see principle 2).
The skill-freshness dimension and the workflow_feedback channel in the subagent return schema exist to feed observations back into these files. When a review surfaces a rule gap, an outdated rule, or a recurring team preference, update the relevant dimension file in the same PR or a follow-up — that is how calibration stays current.