design-review

Design Review

Anti-pattern detection for implementation plans. Catches structural problems before they become code.

Iron Law

CATCH IT IN THE PLAN, NOT IN THE PR

A plan that implies N+1 queries will produce N+1 queries. A plan that omits error handling will produce code with no error handling. Review the plan as rigorously as you review the code — the plan is the blueprint that every implementation decision follows.

---

Anti-Pattern Detection Rules

1. N+1 Queries

What to look for in a plan:

• Steps that fetch a list, then loop to fetch related data per item
• "For each X, get Y" language without mentioning batch operations
• JOIN or batch-fetch never mentioned for related data
• "Display X with Y" without specifying how Y is fetched

How to flag:

[N+1 RISK] Step {N}: "{quoted plan text}" — fetches {entity} per iteration.
Resolution: Replace loop with batch query: {proposed batch operation}.

What to suggest instead:

• Identify all IDs in the list first, then batch-fetch related records in one query
• Use ORM eager loading (.include, .with, JOIN) at the list query level
• Cache frequently accessed related data at the service layer

2. God Functions

What to look for in a plan:

• Single step handling more than 3 distinct responsibilities
• "Validate AND process AND notify AND log AND update" in one step
• Steps described as "the main handler" that touch multiple subsystems
• No clear single-sentence description of what the function does

How to flag:

[GOD FUNCTION RISK] Step {N}: "{quoted plan text}" — handles {n} responsibilities.
Resolution: Split into: {responsibility 1} → {responsibility 2} → {responsibility 3}.

What to suggest instead:

• Each function should have a single, clear sentence description of its purpose
• Extract notification, logging, and side effects to separate functions
• Orchestrate composition at a higher level (service layer calls multiple focused functions)

3. Missing Parallelism