# Code Review And Quality

> Conduct staff-engineer-level code reviews across correctness, security, performance, readability, and architecture

- Skill: `vignesh2027/code-review-and-quality` (Agent Skill)
- Install (CLI): `npx skillmds@latest add vignesh2027/code-review-and-quality`
- Raw SKILL.md: https://api.skillmd.com/api/skills/vignesh2027/code-review-and-quality/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: vignesh2027 (https://skillmd.com/u/vignesh2027)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/vignesh2027/code-review-and-quality

---


## Overview

Code review is the most important quality gate in software development. A good code review catches bugs, prevents security vulnerabilities, improves design, and transfers knowledge — before code reaches production. This skill runs a structured review across five dimensions.

## When to Use

- Before any PR is merged
- When reviewing AI-generated code
- As part of the `/review` workflow

## Process

### Dimension 1: Correctness

- Does the code do what the spec says?
- Are all edge cases handled? (empty inputs, null, zero, max values, concurrent access)
- Are error conditions handled? (network failure, invalid input, resource exhaustion)
- Are there off-by-one errors?
- Are there race conditions in concurrent code?
- Does the code handle partial failures?

### Dimension 2: Security

- Is all user input validated before use?
- Are all database queries parameterized?
- Is no sensitive data logged?
- Are all auth/authz checks correct?
- Are there injection vulnerabilities? (SQL, shell, LDAP, XML)
- Are there new attack surfaces (new endpoints, new data types)?
- See `security-and-hardening` for full checklist.

### Dimension 3: Performance

- Are there N+1 query patterns?
- Are there missing database indexes?
- Is expensive computation done in a hot path?
- Are large data sets loaded into memory when streaming would work?
- Are there blocking synchronous calls that should be async?
- Does the change affect the critical path? Profile it.

### Dimension 4: Readability and Maintainability

- Does the code express its intent clearly?
- Are names accurate? (Function names that lie are worse than no names)
- Is complexity necessary, or is there a simpler solution?
- Are there long functions that should be extracted?
- Is there duplicated logic that should be shared?
- Are there magic numbers or strings that should be constants?
- Will a new engineer understand this in 6 months?

### Dimension 5: Architecture

- Does this change fit the existing design?
- Does this create new coupling that will be hard to remove?
- Does this introduce a new pattern when an existing one could be used?
- Does this create a new dependency that adds risk?
- Does this make the system harder or easier to test?

### Review process

1. Read the PR description. If it's missing, ask for it.
2. Understand the spec/ticket the PR is implementing.
3. Review the tests first — they tell you what the author thinks the code should do.
4. Review the implementation against each dimension above.
5. Run the code locally for non-trivial PRs.
6. Leave specific, actionable comments. Not "this is wrong" — "this will fail when X because Y; consider Z."

## Anti-Rationalizations

**"The tests pass — it's good to go"**
Tests passing proves the code passes the tests. It does not prove the tests are correct, complete, or that the code is secure.

**"It's a small change — it doesn't need a full review"**
Small changes have caused large incidents. The review checklist is short enough to run on any change.

## Verification Requirements

- [ ] All five dimensions reviewed
- [ ] Comments are specific and actionable (not vague)
- [ ] Security dimension completed for any change touching auth, data, or external APIs
- [ ] Tests reviewed before implementation