code-review

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:

1. Approved changes — What looks correct and clean
2. Issues found — Each issue with file path, line number, severity (critical/major/minor), and explanation
3. Questions — Anything unclear that needs author clarification
4. Recommendations — Optional improvements that are not blockers