# PR Review Checklist

Walk this list before you click "Approve". If anything is unchecked, leave a comment instead.

## Scope

- [ ] PR does one thing — title describes that thing in one line
- [ ] No drive-by refactors mixed with the change
- [ ] Diff fits in your head (< 400 lines, or split)

## Correctness

- [ ] Acceptance criteria from the linked ticket are met
- [ ] Edge cases handled: empty input, max input, concurrent calls, network failure
- [ ] Off-by-one and timezone bugs ruled out
- [ ] Error paths return useful errors, not silent failures

## Tests

- [ ] New code has tests — at least one per acceptance criterion
- [ ] Tests assert behaviour, not implementation
- [ ] Tests would actually fail if the code regressed
- [ ] No `skip`, `xit`, `it.only`, or commented-out assertions

## Readability

- [ ] Names describe intent, not type or position
- [ ] Comments explain WHY, not WHAT
- [ ] No magic numbers, no copy-pasted blocks
- [ ] Public APIs documented

## Security

- [ ] User input validated at boundary
- [ ] No secrets / tokens / keys in code or tests
- [ ] Authorization checked, not just authentication
- [ ] No new SQL injection / XSS / SSRF surface

## Performance

- [ ] N+1 queries ruled out (count queries in tests)
- [ ] No unbounded loops or unbounded result sets
- [ ] Indexes added for new query patterns
- [ ] Heavy work backgrounded, not in request cycle

## Operations

- [ ] Migrations are reversible (or explicitly forward-only)
- [ ] Feature flag added for risky changes
- [ ] Logs / metrics added for new flows
- [ ] Runbook / docs updated if behaviour changed

## Approve / request changes

- **Approve** if every box is checked
- **Request changes** if any blocker is unchecked
- **Comment** if you have suggestions but no blockers

---

Definition of Done glossary: https://sprintflint.com/glossary/definition-of-done