PR Review Scout

Overview

Review code changes with a strong bias toward finding issues that would matter in production. Prioritize correctness, regressions, security, data integrity, concurrency, and missing test coverage over style nits.

Review Workflow

1. Build context before judging the change.
2. Read the diff carefully and identify behavior changes.
3. Trace each changed path through call sites, state transitions, and failure modes.
4. Report only meaningful findings with concrete evidence.
5. State residual risks and testing gaps if no clear bug is proven.

Build Context First

Before writing findings:

• Inspect the changed files and nearby code.
• Understand the intended behavior from names, comments, tests, and surrounding modules.
• Look for related validations, invariants, cleanup paths, retries, and error handling.
• Check whether the change modifies API contracts, persistence, caching, auth, permissions, async flow, or user-visible behavior.

If context is still incomplete, say what assumption you are making instead of filling gaps with speculation.

What to Look For

Prioritize findings in this order:

1. Correctness and Regressions

• Broken logic, wrong conditions, off-by-one errors, stale assumptions
• Missing null checks, bad defaults, incorrect fallback order
• Contract mismatches between caller and callee
• Partial refactors where one path changed but sibling paths did not

2. Data Safety and State Integrity

• Writes that can corrupt, duplicate, or silently drop data
• Missing transactions, rollback paths, idempotency checks, or cache invalidation
• Order-of-operations bugs during save, delete, retry, or migration flows

3. Security and Permissions

• Auth bypasses, missing authorization checks, privilege escalation
• Unsafe interpolation, injection risks, path traversal, secret leakage
• Logging or returning sensitive fields by accident

4. Reliability and Operations

• Races, deadlocks, retry storms, timeout gaps, resource leaks
• Async work that is no longer awaited or cancellation that is ignored
• Error handling that swallows failures or reports success too early

5. Performance with Real User Impact

• New N+1 queries, repeated heavy work in loops, expensive recomputation
• Unbounded memory or CPU growth on hot paths
• Large payloads or blocking I/O introduced into latency-sensitive code

6. Testing Gaps

• Missing tests for new branches, edge cases, failure modes, and regressions
• Tests updated only for happy paths while risky paths changed

For a deeper checklist, read references/review-checklist.md.

Finding Bar

Only report a finding when all of these are true:

• The issue is plausibly real, not a vague possibility.
• The impact matters to users, systems, or maintainers.
• The explanation ties directly to changed code.
• You can explain the failure mode in 2-5 sentences.

Avoid low-signal comments about naming, formatting, or hypothetical preferences unless the user explicitly asks for broader feedback.

Output Format

Present findings first, ordered by severity. Use this structure:

- [high] file:line - Short title
  Why it matters: ...
  Evidence: ...
  Suggested direction: ...

After findings, add:

• Open questions: only if an unanswered question changes the conclusion
• Residual risks: for important areas you could not fully verify
• Change summary: one short paragraph only after the findings

If no findings are discovered, say that explicitly and mention the main residual risk or testing gap.

Review Behavior

• Be skeptical but fair.
• Prefer precise claims over exhaustive coverage.
• Do not invent runtime behavior without code evidence.
• Do not require perfect proof when the failure mode is strong and concrete.
• Distinguish confirmed issues from assumptions.
• Keep the review concise and high signal.

Example Requests

• Review this PR for bugs and regressions.
• Check this diff like a senior reviewer and focus on correctness.
• Look at these changed files and tell me what is risky before merge.
• Review this patch and tell me what tests are still missing.