| name | code-review |
| description | Use when reviewing code changes for correctness, style, conventions, testing, and security. Load during Implementation phase as a quality gate. |
code-review
SDLC Phase: Implementation (Quality Gate)
Review code changes for correctness, style, conventions, testing, and security.
Principles
- Review intent, not syntax — Focus on whether the code solves the right problem correctly. Formatting and naming are caught by tools.
- Be specific — Point to exact lines and explain why something is a problem, not just that it is wrong.
- Know the conventions — AGENTS.md and docs/conventions.md define the rules. Refer to them directly.
- Think about the reader — Will another developer understand this code six months from now?
Review Dimensions
Correctness
- Does the code do what the requirement specifies? Trace the logic from input to output.
- Are edge cases handled? Consider: empty input, error states, boundary values, overflow, domain errors.
- Are error messages descriptive? A user should understand what went wrong and what to do about it.
- Does it avoid panics for expected failure modes? Only programming errors should panic.
- Are unsafe blocks justified and safe? Verify the safety invariants are documented and upheld.
Architecture & Design
- Does the code fit within the existing module hierarchy? Would another location be more natural?
- Does it introduce circular dependencies? Check that the dependency DAG is preserved.
- Is the public API minimal and intentional? Every public item should have a reason to be public.
- Are internal implementation details kept private? Prefer
pub(crate) over pub for crate-internal items.
- Does it follow the brain/body separation? rvnx defines ports, rvfx implements them. Never the reverse.
Conventions
- No vendor lock-in: no
RvlibsFoo prefixes. Disambiguate by module path.
- Naming follows docs/conventions.md:
snake_case for functions, PascalCase for types, UPPER_SNAKE_CASE for constants.
- Doc comments (
///) on all public items explaining what they do, with examples where helpful.
- Imports are organized. Unused imports are removed.
Testing
- New features include tests covering the happy path and edge cases.
- rvtest changes use the BDD API:
describe/it/run/assert_all_pass() — dogfooding is mandatory.
- Floating-point comparisons use epsilon, not
assert_eq!.
- Tests are independent — no shared mutable state between test cases.
Security
- No secrets, passwords, tokens, or API keys in code or comments.
- No hardcoded paths, URLs, or environment-specific values that would break in CI.
- No
unsafe without clear justification and documentation.
- No command injection risks in string-based command construction.
Output Requirements
A review summary listing:
- Approved changes — What looks correct and clean
- Issues found — Each issue with file path, line number, severity (critical/major/minor), and explanation
- Questions — Anything unclear that needs author clarification
- Recommendations — Optional improvements that are not blockers
