Code Review Standards

Code Review Standards

Review Checklist

Correctness

• Code implements the requirements correctly
• Edge cases are handled
• Error handling is appropriate (not swallowing exceptions)
• Null checks where needed (nullable reference types preferred)
• No off-by-one errors in loops/collections
• Async/await used correctly (no async void, no .Result blocking)

Architecture & Design

• Follows existing patterns in the codebase
• Single Responsibility Principle maintained
• Dependencies injected (not new-ed internally for services)
• Interfaces used for testability where appropriate
• No unnecessary abstractions or over-engineering

Performance

• No unnecessary allocations in hot paths
• Collections sized appropriately (List capacity, Dictionary)
• LINQ queries not causing N+1 or excessive enumeration
• Async I/O for all I/O operations
• No blocking calls on async code paths
• String building uses StringBuilder for loops
• Latency prioritized over memory usage

Security

• No SQL injection vulnerabilities (parameterized queries)
• Input validation at system boundaries
• No sensitive data in logs
• Authentication/authorization checks in place
• No hardcoded secrets or credentials

Code Quality

• Naming follows .NET conventions (PascalCase for public, camelCase for private)
• No dead code or commented-out blocks
• Methods under 30 lines (guideline, not absolute)
• Cyclomatic complexity reasonable
• Magic numbers/strings extracted to constants
• XML doc comments on public API

Testing

• New code has corresponding tests
• Tests follow MSTest conventions
• Test names describe behavior
• No test coupling or order dependency

Severity Levels

• Critical: Must fix — security vulnerability, data loss risk, crash
• Major: Should fix — bug, performance issue, design violation
• Minor: Consider fixing — style, naming, minor optimization
• Nit: Optional — preference, cosmetic

Review Document

Stored at .memory_bank/features/<feature-name>/code-reviews/phase-N-review-M.md.

Template

# Code Review: Phase N - <Phase Name> (Review #M)

## Date
<date>

## Result: APPROVED / CHANGES NEEDED

## Summary
Brief overall assessment.

## Findings

### Critical
- **File:** `path/to/file.cs` (Line N)
  - **Issue:** Description
  - **Fix:** Recommended fix

### Major
- ...

### Minor
- ...

### Nits
- ...

## Positive Notes
- Good patterns observed
- Well-handled edge cases

## Changes Required (if CHANGES NEEDED)
1. Fix description with file references
2. ...