Code Review Pyramid
The Code Review Pyramid provides guidance on aspects to focus on during code reviews, prioritized by the cost of fixing issues later. Layers at the bottom of the pyramid deserve more attention because they are harder and more expensive to change after. Layers at the top are easier to address (often automatable) and warrant proportionally less manual review effort.
Adapted from the Code Review Pyramid by Gunnar Morling, licensed under CC BY-SA 4.0.
The Pyramid
Five layers, numbered from the base up. Manual review attention should be highest at the base and taper toward the apex, where checks should be automated as much as possible:
- API Semantics — base: focus here most
- Implementation Semantics — focus here
- Documentation
- Tests — automate where possible
- Code Style — apex: automate
Layer 1 — API Semantics
Questions to ask:
- API as small as possible, as large as needed?
- Is there one way of doing one thing, not multiple?
- Is it consistent? Does it follow the principle of least surprise?
- Clean split of API/internals, without internals leaking in the API?
- Are there no breaking changes to user-facing parts (API classes, configuration, metrics, log formats, etc.)?
- Is a new API generally useful and not overly specific?
Layer 2 — Implementation Semantics
Questions to ask:
- Does it satisfy the original requirements?
- Is it logically correct?
- Is there no unnecessary complexity?
- Is it robust? (no concurrency issues, proper error handling)
- Is it performant?
- Is it secure? (e.g. no SQL injection, no sensitive data exposure, etc.)
- Is it observable? (e.g. metrics, logging, tracing, etc.)
- Do newly added dependencies pull their weight? Are their licenses acceptable?
Layer 3 — Documentation
Questions to ask:
- Are new features reasonably documented?
- Are the relevant kinds of docs covered: README, API docs, user guide, reference docs, etc.?
- Are docs understandable? Are there no significant typos or grammar mistakes?
Layer 4 — Tests
Questions to ask:
- Are all tests passing?
- Are new features reasonably tested?
- Are corner cases tested?
- Is it using unit tests where possible, integration tests where necessary?
- Are there tests for NFRs, e.g. performance?
Layer 5 — Code Style
Questions to ask:
- Is the project's formatting style applied?
- Does it adhere to agreed naming conventions?
- Is it DRY?
- Is the code sufficiently readable? (method lengths, etc.)
1---2name: code-review-pyramid3description: Knowledge base for the Code Review Pyramid — a framework for structuring code reviews across five layers prioritized by cost-of-change. Use when the user asks how to approach a code review, wants to understand what to focus on in a review, asks about review priorities, or references "the pyramid". Also use when another skill needs a structured review framework (e.g. review-changes loads it to apply consistent layer priorities and questions).4---56# Code Review Pyramid78The Code Review Pyramid provides guidance on aspects to focus on during code reviews, prioritized by the cost of fixing issues later. Layers at the bottom of the pyramid deserve more attention because they are harder and more expensive to change after. Layers at the top are easier to address (often automatable) and warrant proportionally less manual review effort.910*Adapted from the [Code Review Pyramid](https://www.morling.dev/blog/the-code-review-pyramid/) by [Gunnar Morling](https://www.morling.dev/), licensed under [CC BY-SA 4.0](https://creativecommons.org/licenses/by-sa/4.0/).*1112## The Pyramid1314Five layers, numbered from the base up. Manual review attention should be highest at the base and taper toward the apex, where checks should be automated as much as possible:15161. **API Semantics** — base: focus here most172. **Implementation Semantics** — focus here183. **Documentation**194. **Tests** — automate where possible205. **Code Style** — apex: automate2122## Layer 1 — API Semantics2324Questions to ask:25- API as small as possible, as large as needed?26- Is there one way of doing one thing, not multiple?27- Is it consistent? Does it follow the principle of least surprise?28- Clean split of API/internals, without internals leaking in the API?29- Are there no breaking changes to user-facing parts (API classes, configuration, metrics, log formats, etc.)?30- Is a new API generally useful and not overly specific?3132## Layer 2 — Implementation Semantics3334Questions to ask:35- Does it satisfy the original requirements?36- Is it logically correct?37- Is there no unnecessary complexity?38- Is it robust? (no concurrency issues, proper error handling)39- Is it performant?40- Is it secure? (e.g. no SQL injection, no sensitive data exposure, etc.)41- Is it observable? (e.g. metrics, logging, tracing, etc.)42- Do newly added dependencies pull their weight? Are their licenses acceptable?4344## Layer 3 — Documentation4546Questions to ask:47- Are new features reasonably documented?48- Are the relevant kinds of docs covered: README, API docs, user guide, reference docs, etc.?49- Are docs understandable? Are there no significant typos or grammar mistakes?5051## Layer 4 — Tests5253Questions to ask:54- Are all tests passing?55- Are new features reasonably tested?56- Are corner cases tested?57- Is it using unit tests where possible, integration tests where necessary?58- Are there tests for NFRs, e.g. performance?5960## Layer 5 — Code Style6162Questions to ask:63- Is the project's formatting style applied?64- Does it adhere to agreed naming conventions?65- Is it DRY?66- Is the code sufficiently readable? (method lengths, etc.)
azborgonovo