Emrecdr Devt Code Review Guide

Code Review Guide

Overview

A code review is a structured inspection that produces a scored assessment with specific, actionable findings. Every finding must reference a concrete location, describe the problem, explain the impact, and suggest a fix.

Reviews are objective. They evaluate code against documented project standards, not personal preferences. If a standard does not exist for something, it is not a finding.

Precision mandate: Every finding must name a specific file, line number, and exact violation. If a finding could apply to any codebase, it is too vague to report.

When NOT to Use

Skip for self-review during implementation — this is for the formal review phase.

Time Budget

• Quick review (1-3 files): 2-3 minutes
• Full review (5+ files): 5-10 minutes

The Iron Law

EVERY VALID FINDING MUST BE REPORTED — NO FILTERING, NO MERCY

An unreported finding is a silently disabled quality gate. If the code violates a documented standard, report it — regardless of origin, perceived severity, or whether the developer "probably knows."

The tendency to filter findings by perceived importance, origin ("pre-existing"), or social comfort ("it's minor") directly undermines the review's value. Users rely on the review to catch what they missed. A finding omitted because it seemed small is a finding the developer never gets to evaluate. The reviewer's job is detection, not triage — the developer decides what to fix.

The Process

Step 1: Load Standards

Before reviewing any code, read the project's rules:

1. .devt/rules/coding-standards.md — code conventions
2. .devt/rules/architecture.md — structural boundaries
3. .devt/rules/quality-gates.md — pass/fail criteria
4. CLAUDE.md — project-specific rules

A review without loaded standards is a review against personal opinion. Do not start reviewing until all applicable standards are loaded.

Step 2: Understand Scope

Read the implementation summary or PR description:

• What files were changed and why
• What the acceptance criteria are
• What the intended behavior is

This sets context but does not limit findings. Issues found outside the stated scope are still valid.

Step 3: Review by Category

Evaluate each changed file against these categories:

Architecture (weight: high)

• Layer boundaries respected (domain has no infrastructure imports)
• Dependency direction correct (inner layers do not depend on outer)
• Separation of concerns (routes are thin, services hold logic)
• No circular dependencies
• Repository pattern followed (services use interfaces, not sessions)

Security (weight: critical)

• Input validation present (schema validation, type safety)
• Authentication/authorization checks in place
• No secrets in code or logs
• SQL injection prevention (parameterized queries)
• No data exposure in error responses

Performance (weight: medium)

• No N+1 query patterns
• No unnecessary database round-trips
• Appropriate use of async/sync
• No unbounded queries (missing pagination/limits)

Error Handling (weight: high)

• Custom errors inherit from base error classes (not plain Exception)
• No swallowed exceptions (bare except or except Exception: pass)
• Errors carry enough context for debugging
• Graceful degradation where appropriate

Test Coverage (weight: high)

• New code has corresponding tests
• Edge cases covered (empty input, null, boundary values)
• Tests are independent and deterministic
• Test names describe behavior, not implementation

UI/UX Quality (weight: medium, frontend projects only)

For Vue/frontend projects, also check:

• Focus states present and visible on interactive elements
• Touch targets meet minimum size (24x24px min)
• Text contrast ratio meets WCAG AA (4.5:1 for normal text)
• Semantic HTML used (<nav>, <main>, <button> vs generic <div>)
• prefers-reduced-motion respected for animations

Code Quality (weight: medium)

• Clear naming (variables, functions, classes)
• No deep nesting (3+ levels)
• Early returns used for guard clauses
• No duplication (DRY)
• No dead code, commented-out code, or TODOs
• Type hints present on all function signatures

Step 4: Score Each Finding

Every finding gets a severity:

Step 5: Calculate Score

Starting score: 100
Final score: 100 - sum(deductions)
Minimum: 0

Step 6: Determine Verdict

Step 7: Write Report

Structure the report as:

## Review Summary
Score: XX/100 — VERDICT

## Findings

### [Critical] File:Line — Short description
**Problem**: What is wrong
**Impact**: Why it matters
**Fix**: What to do

### [Important] File:Line — Short description
...

Scoring Examples

Critical finding (-15):

• PASS: "SQL injection in users.ext:47 -- user input concatenated into query string without parameterization"
• FAIL: "Possible security issue" (too vague -- name the file, line, exact problem)

Important finding (-7):

• PASS: "Missing error handling in payment_service.ext:89 -- API call to Stripe has no try/catch, will crash on network timeout"
• FAIL: "Error handling could be better" (unactionable)

Minor finding (-3):

• PASS: "Inconsistent naming: getUserData() at api.ext:23 but fetch_user_info() at api.ext:67 -- pick one convention"
• FAIL: "Naming is inconsistent" (where? what? be specific)

Gate: Honest Scoring

• No findings dismissed as "acceptable" or "pre-existing"
• Deductions applied consistently across findings
• Verdict matches the score (no manual override)

Anti-patterns

Integration

• Prerequisites: Standards files must exist in .devt/rules/
• Used by agents: code-reviewer (primary consumer)
• Related skills: codebase-scan (to verify no duplication introduced), architecture-health-scanner (for systemic issues)

Memory + Graphify integration

Reviews must include an "ADR Compliance" section. For each diff hunk:

1. node bin/devt-tools.cjs memory affects <changed-file> — enumerate governing docs
2. Verify diff respects active ADRs (treat violations as Critical findings)
3. node bin/devt-tools.cjs memory rejected-keywords — flag any diff text matching a REJ tombstone
4. When Graphify enabled, enumerate affected callers via graphify neighbors <symbol> --direction=in (uses skills/graphify-helpers/SKILL.md protocol — falls back to grep when disabled).

Source: emrecdr/devt — distributed by TomeVault.