code-review-and-quality

Follow a five-step process: understand the change's intent, review the tests first, walk the implementation across five axes (correctness, readability, architecture, security, performance), categorize findings by severity, and verify the author's testing story before approving.

A review checklist should cover correctness against the spec, edge case and error handling, naming and simplicity, architectural fit, input validation and secret hygiene, N+1 queries and unbounded operations, and confirmation that tests and builds pass.

Around 100 changed lines is ideal and reviewable in one sitting; 300 lines is acceptable for a single logical change; 1000 lines is too large and should be split using stacking, file-group, horizontal, or vertical splitting strategies.

Use no prefix for required changes, 'Critical:' for merge blockers like security vulnerabilities, 'Nit:' for optional style points, 'Optional:' or 'Consider:' for suggestions, and 'FYI' for informational context requiring no action.

Upgrade one dependency per change so a broken build has an obvious cause and a clean revert. Read the changelog rather than trusting semver, review the lockfile diff for transitive changes, and require a green test suite before and after.

Passing tests are necessary but not sufficient. Tests do not catch architecture problems, security issues, readability concerns, or complexity that was relocated rather than reduced, so review must cover all five axes.