ashermahonin PR Review
Purpose
Find defects and material maintenance risks before a change lands. Review behavior first; style matters only when it obscures correctness or raises change cost.
Change context
- Identify changed files and intent of the change.
- Find affected flows, contracts, data, security boundaries, and tests.
- Read the diff with the old behavior in mind.
- Look for missing validation and docs drift.
Change method
- Reconstruct the intended behavior from the request, diff, tests, and nearby contracts.
- Trace correctness, failure handling, concurrency, compatibility, migration, and rollback effects.
- Check authorization, secrets, privacy, untrusted input, and destructive operations where relevant.
- Use
engineering-qualitycriteria for responsibilities, dependencies, side effects, errors, and tests; do not turn preferences into findings. - Review performance and operations when shared or hot paths changed.
- Report findings by severity with a tight file/line reference, concrete impact, and fix direction. If none are found, state residual risk and checks not run.
Engineering constraints
- Keep review read-only unless the user also asks for fixes.
- Verify uncertain external behavior with current documentation before raising a finding.
- Do not approve behavior you could not reconstruct from code and evidence.
Evidence
- Prioritized findings with evidence
- Open questions and assumptions
- Missing or unrun validation
- Residual risk and merge recommendation
Ready when
- Findings lead the response and are ordered by impact.
- Every finding describes a plausible failure, not a hypothetical preference.
- Line references are as narrow as the evidence permits.
- A clean review still reports test gaps and residual risk.
Handoff
Hand off actionable findings to implementation or release decision with severity and evidence.
References
references/review-checklist.md: Use this checklist for reviews.