If .claude/skills/review-conventions/SKILL.md.disabled exists, stop before reading a fallback.
Project configuration: If the current project contains .claude/skills/review-conventions/SKILL.md, read and apply it instead of this bundled default. The project copy is the capability source of truth.
Repository context: Read ../setup/references/task-context.md when resolving a task, project commands, named agent roles, or legacy .claude/ resource paths. Use repository evidence and applicable project instructions; a missing local workflow copy does not require setup. Resolve bundled resources from the installed skill, never from the target cwd.
Review Conventions
Shared knowledge preloaded into review agents. Apply these conventions when reviewing code.
Project context
This reference stays generic so it can run in any repository. Read an
applicable CLAUDOPS.md, manifests, CI configuration, source layout, and
existing code before applying project-specific review rules. Use the project's
own language, framework, layers, test commands, and documentation locations;
leave unknowns explicit. {{...}} markers are historical template notation,
never runtime defaults or executable paths.
Architecture Rules
Review boundaries and dependencies as the target project defines them. Use its
architecture vocabulary where it already has one; do not rename service,
API, module, or similar identifiers merely to fit this reference.
Code Standards
- Prefer project-established type/interface conventions
- Preserve local naming and type conventions; apply language-specific rules such
as underscore prefixes or
any only when the repository's tooling establishes
them.
- Secrets never logged; environment vars flow only through config providers
- Database queries use parameter binding — no dynamic SQL or string interpolation
Review Quality Rules
Reviews run in two stages so we get recall AND precision:
Find stage (this skill) — report every issue you notice, including low-severity
and uncertain ones. For each finding, include:
severity: CRITICAL | MAJOR | MINOR | INFO
confidence: high | medium | low
location: file:line
suggestion: concrete fix or next step
Do not self-censor based on severity or confidence — a later verification pass
will filter before anything reaches the user.
Presentation — consolidate repeats ("5 functions missing error handling" with
a list, not 5 separate findings). Explain why each issue matters. Highlight
positive practices alongside problems.
Sizing & Comment Hygiene
Norms from Google's engineering practices guide:
Change sizing — a reviewable change is small (~100 lines is a healthy target). If the diff is large and not mechanically generated, flag it and suggest splitting; a 1000-line PR gets a worse review, not a better one.
The standard is "improves code health" — approve a change that improves the codebase even if it isn't perfect. Don't block on personal preference; block on real problems.
Mark non-blocking comments — prefix advisory notes (Nit:, Optional:, FYI:) so the author can tell what must change from what's a suggestion. Severity inflation trains authors to ignore you.
Primary scope: only review files in changed_files
Use full_diff to focus on changed lines
You may read unchanged files for context (interfaces, contracts), but don't raise
findings against unchanged code — the author isn't touching it in this PR.
Skip pre-existing issues unless the current changes make them worse (e.g., a bug
that used to be in dead code is now reachable).
Ownership Boundaries
Each agent owns specific concerns — do not duplicate other agents' work:
| Concern |
Owner |
| Spec requirements match |
spec-compliance-reviewer |
| Architecture fit, layers, module boundaries |
senior-architecture-reviewer |
| Security, auth, injection, OWASP |
security-code-reviewer |
| Code quality, naming, DRY, complexity |
code-quality-reviewer |
| Test coverage and quality |
test-coverage-reviewer |
| Documentation accuracy |
documentation-accuracy-reviewer |
| Performance, N+1, memory |
performance-reviewer |
If you spot something outside your scope, note it as a one-line INFO finding with
the likely owner (e.g. "possible security concern — flag for security-code-reviewer")
and move on.
Project File Locations
- Architecture docs: resolve from the repository's documented location or
changed-file references; do not assume a
docs/ subdirectory.
- Product docs (PRDs, JTBDs):
product-docs/
- Task documents: resolve through
../setup/references/task-context.md; preserve
repository conventions such as tasks/<task-dir>/tech-decomposition*.md
- Test structure: infer from the repository's existing test layout and CI.
1---2name: review-conventions-43description: Internal reference skill — shared conventions for all code review agents. Not user-invocable.4---56<!-- claudops-build: project-config-pointer -->7> If `.claude/skills/review-conventions/SKILL.md.disabled` exists, stop before reading a fallback.8> **Project configuration:** If the current project contains `.claude/skills/review-conventions/SKILL.md`, read and apply it instead of this bundled default. The project copy is the capability source of truth.9> **Repository context:** Read `../setup/references/task-context.md` when resolving a task, project commands, named agent roles, or legacy `.claude/` resource paths. Use repository evidence and applicable project instructions; a missing local workflow copy does not require setup. Resolve bundled resources from the installed skill, never from the target cwd.1011# Review Conventions1213Shared knowledge preloaded into review agents. Apply these conventions when reviewing code.1415## Project context1617This reference stays generic so it can run in any repository. Read an18applicable `CLAUDOPS.md`, manifests, CI configuration, source layout, and19existing code before applying project-specific review rules. Use the project's20own language, framework, layers, test commands, and documentation locations;21leave unknowns explicit. `{{...}}` markers are historical template notation,22never runtime defaults or executable paths.2324## Architecture Rules2526Review boundaries and dependencies as the target project defines them. Use its27architecture vocabulary where it already has one; do not rename `service`,28`API`, `module`, or similar identifiers merely to fit this reference.2930## Code Standards3132- Prefer project-established type/interface conventions33- Preserve local naming and type conventions; apply language-specific rules such34 as underscore prefixes or `any` only when the repository's tooling establishes35 them.36- Secrets never logged; environment vars flow only through config providers37- Database queries use parameter binding — no dynamic SQL or string interpolation3839## Review Quality Rules4041Reviews run in two stages so we get recall AND precision:42431. **Find stage (this skill)** — report every issue you notice, including low-severity44 and uncertain ones. For each finding, include:45 - `severity`: CRITICAL | MAJOR | MINOR | INFO46 - `confidence`: high | medium | low47 - `location`: `file:line`48 - `suggestion`: concrete fix or next step49 Do not self-censor based on severity or confidence — a later verification pass50 will filter before anything reaches the user.51522. **Presentation** — consolidate repeats ("5 functions missing error handling" with53 a list, not 5 separate findings). Explain why each issue matters. Highlight54 positive practices alongside problems.5556## Sizing & Comment Hygiene5758Norms from Google's [engineering practices guide](https://google.github.io/eng-practices/):5960- **Change sizing** — a reviewable change is small (~100 lines is a healthy target). If the diff is large and not mechanically generated, flag it and suggest splitting; a 1000-line PR gets a worse review, not a better one.61- **The standard is "improves code health"** — approve a change that improves the codebase even if it isn't perfect. Don't block on personal preference; block on real problems.62- **Mark non-blocking comments** — prefix advisory notes (`Nit:`, `Optional:`, `FYI:`) so the author can tell what must change from what's a suggestion. Severity inflation trains authors to ignore you.6364- Primary scope: only review files in `changed_files`65- Use `full_diff` to focus on changed lines66- You may read unchanged files for context (interfaces, contracts), but don't raise67 findings against unchanged code — the author isn't touching it in this PR.68- Skip pre-existing issues unless the current changes make them worse (e.g., a bug69 that used to be in dead code is now reachable).7071## Ownership Boundaries7273Each agent owns specific concerns — do not duplicate other agents' work:7475| Concern | Owner |76|---------|-------|77| Spec requirements match | `spec-compliance-reviewer` |78| Architecture fit, layers, module boundaries | `senior-architecture-reviewer` |79| Security, auth, injection, OWASP | `security-code-reviewer` |80| Code quality, naming, DRY, complexity | `code-quality-reviewer` |81| Test coverage and quality | `test-coverage-reviewer` |82| Documentation accuracy | `documentation-accuracy-reviewer` |83| Performance, N+1, memory | `performance-reviewer` |8485If you spot something outside your scope, note it as a one-line INFO finding with86the likely owner (e.g. "possible security concern — flag for `security-code-reviewer`")87and move on.8889## Project File Locations9091- Architecture docs: resolve from the repository's documented location or92 changed-file references; do not assume a `docs/` subdirectory.93- Product docs (PRDs, JTBDs): `product-docs/`94- Task documents: resolve through `../setup/references/task-context.md`; preserve95 repository conventions such as `tasks/<task-dir>/tech-decomposition*.md`96- Test structure: infer from the repository's existing test layout and CI.