Risk-Based PR Review Policy

Risk-Based PR Review Policy

Overview

This policy guides developers in determining appropriate review requirements for pull requests based on risk assessment. The core question is: what kind of verification does this change need, and is there a judgment call that automated tools cannot make?

Risk Assessment Framework

🟢 Zero-Risk Changes

Definition: No behavior change. Cannot cause a production issue.

Typical Examples:

• Documentation updates (README, comments)
• CSS/styling changes with no logic
• Test additions (not modifications)
• Formatting/linting fixes
• Release/deployment PRs (e.g., “Release 99”)
• Configuration that doesn’t affect runtime behavior

Verification: None needed Code Review: AI review or none

🟡 Low-Risk Changes

Definition: Behavior change with a straightforward approach, fully verifiable by automated tests and QA. The approach is clear, coupling is minimal, and there are no hidden long-term concerns.

Typical Examples:

• Bug fixes with a behavioral test that fails before and passes after
• Standard CRUD operations following established patterns
• Permission/auth fixes with full test matrix coverage
• Feature-flagged UI work
• Mechanical refactors (renames, moves, adding $fillable, removing dead code)
• Changes across many files that are repetitive/mechanical

Verification: Automated tests + QA Code Review: AI only

Note: A PR is not higher risk just because it is large or touches many files. Mechanical changes across 50 files are lower risk than a clever 3-line change. Size ≠ risk.

🟠 Medium-Risk Changes

Definition: The change delivers the feature or fixes the problem correctly, but the approach is intricate, might create coupling that causes problems down the line, or has non-obvious interactions with other parts of the system. The concern isn’t that it breaks today — it’s that it might be a hidden bomb in 3 months. Also appropriate when an author wants a second set of eyes, or is onboarding and benefits from human feedback on their approach.

Typical Examples:

• Business logic with complex conditional paths
• New architectural patterns being introduced for the first time
• Performance optimizations that trade readability for speed
• Changes touching multiple bounded contexts
• Caching strategies (invalidation is where bugs hide later)
• Code by onboarding developers who want feedback

Verification: Automated tests + QA Code Review: AI review + 20% post-merge human sampling, or author can opt directly into the post-merge review queue

The purpose of post-merge review is NOT to re-verify correctness (tests did that). It’s to have a human assess the approach — catch things like “this works but will make the next feature in this area a nightmare” or “this couples two things that shouldn’t know about each other.” Authors who know their approach is intricate should put the PR straight into the post-merge queue rather than leaving it to the 20% random sample.

🔴 High-Risk Changes

Definition: Requires human judgment before merging. There is a decision that automated tests and AI review cannot make — a product tradeoff, an architecture precedent, domain-specific knowledge, or an irreversible change.

Typical Examples:

• Product decisions (“should we record original values when custom data is set?”)
• Architecture changes that set a precedent (new patterns other code will follow)
• Infrastructure changes requiring domain knowledge (deployment topology, database pooling)
• Irreversible changes (data migrations that can’t be undone, column drops)
• Changes to the permission model or multi-tenancy scoping logic

Verification: Automated tests + QA Code Review: Human reviewer required pre-merge, tagged with the specific question that needs their judgment

Important: Tag the specific person whose judgment is needed and state what you need from them. “Needs human review” without a question is not actionable.

When Human Review Is Actually Needed

Human review is needed when there is a judgment call that code cannot make:

Reason	Example	Who
Product decision	“Should we record original values when custom data is set?”	Product owner
Architecture precedent	Adding a $context parameter to toEntity() changes domain entity design	Tech lead
Domain knowledge	Fly.io deployment topology, PgBouncer transaction pooling behavior	Infrastructure owner
Tradeoff with no clear winner	Performance vs maintainability (raw HTML vs component library)	Team consensus
Author is onboarding	Developer learning the codebase wants feedback on approach	Any experienced team member

Human review is NOT needed just because:

• The PR is large — size ≠ risk. Mechanical changes across 50 files are lower risk than a clever 3-line change.
• It touches “security” — adding $fillable = [] is strictly safer. No judgment needed.
• It’s a write path — if behavioral tests cover the cases, tests ARE the review.
• It sounds scary — “database migration” and “auth changes” are categories, not risk levels. Adding an index is not the same as changing a column type.

Context Considerations

When assessing risk, consider:

• Who is affected: Internal users vs external customers
• Data sensitivity: Test data vs production data
• Reversibility: Can changes be easily rolled back?
• Safety mechanisms: Are feature flags or kill switches in place?
• Approach complexity: Is the solution straightforward or intricate?
• Future coupling: Does this create dependencies that will be hard to change later?

Special Risk Modifiers

Feature Flags [FF]

• Significantly reduces risk for UI/prototype work
• Does NOT reduce risk for core system changes
• Allows gradual rollout and quick rollback

Parallel Refactors [PARALLEL]

• Reduces risk when maintaining backward compatibility
• Old and new code paths coexist temporarily
• Does NOT reduce risk for database or auth changes

Hotfixes [HOTFIX]

• Expedited review process
• Still requires appropriate review level based on changes
• Should be followed by post-incident review

Post-Merge Sampling Process

Medium-risk PRs are sampled at 20% for post-merge human review:

1. After merge, PR is either randomly selected (1 in 5) or placed in the queue directly by the author
2. Reviewer focuses on approach, not correctness:
   • Will this create maintenance burden?
   • Does this introduce coupling that could cause issues later?
   • Is there a simpler way to achieve the same result?
   • Are there patterns here that other code will copy?
3. Findings are discussed with the author as learning, not blockers

Implementation Tools

• AI Reviewers: CodeRabbit, Devin, Kimi — provide automated code analysis on every PR
• PR Templates: Include risk level self-assessment
• QA Testing: Manual QA covers browser-level and UX verification
• Automated Tests: Behavioral tests verify correctness — a test that fails before and passes after is the strongest form of verification

Continuous Improvement

This policy evolves based on:

• Incident post-mortems
• Post-merge sampling findings
• Team feedback
• Changes in technology or architecture

Policy Review

• This policy will be reviewed annually and updated as necessary to reflect changes in our systems, processes, or regulatory requirements.