Pragmatic Review

Pragmatic Code Review: YAGNI & KISS Focus

You will perform an interactive code review with laser focus on YAGNI (You Aren't Gonna Need It) and KISS (Keep It Simple, Stupid) principles.

Review Modes

Default mode: Fast YAGNI/KISS-focused review

• Scans for over-engineering, unused abstractions, unnecessary complexity
• Quick security and performance checks (OWASP basics, obvious N+1 queries)
• Self-reflection to validate findings with evidence

Deep mode (--deep flag): Multi-pass comprehensive review

• Pass 1: Security (OWASP Top 10, input validation, auth issues)
• Pass 2: Architecture (SOLID principles, separation of concerns)
• Pass 3: Logic (edge cases, error handling, correctness)
• Pass 4: Performance (algorithm complexity, resource leaks)
• Pass 5: YAGNI/KISS (over-engineering, unnecessary abstractions)
• Pass 6: Maintainability (readability, tests, documentation)
• Self-reflection after all passes

Use --deep when:

• Security-critical changes (auth, payment, data handling)
• Core architecture modifications
• Complex logic changes with many edge cases
• Performance-sensitive code paths

Use default mode when:

• Feature additions
• Bug fixes
• Refactoring
• Documentation changes

CI mode (--ci flag): Non-interactive mode for GitHub Actions

• Skips ALL interactive prompts
• Auto-selects: all branch changes vs base branch
• Uses $GITHUB_BASE_REF environment variable if available
• Outputs all findings at once as markdown (summary view)

Step 1: Determine Review Scope

Check Current Git State

First, verify we're in a git repository by running:

• test -d .git to check if .git directory exists

If not in a git repository, ask the user to specify files to review manually.

If in a git repository, gather information:

Current branch:

Run: git rev-parse --abbrev-ref HEAD

Default branch detection:

1. Try: git rev-parse --verify main
2. If that fails, try: git rev-parse --verify master
3. If that fails, try: git rev-parse --verify develop

If user specified --base [branch] in arguments, use that instead.

Working directory status:

Run: git status --short | head -20

Present Options to User

If --ci flag is present: Skip all interactive prompts and auto-select option 2: Review all changes on current branch vs base.

Unless --auto or --ci flag is present, ask the user:

📋 CODE REVIEW SCOPE SELECTION
════════════════════════════════

What would you like to review?

1️⃣  Current uncommitted changes
2️⃣  All changes on current branch (compared to [detected default branch])
3️⃣  Specific files or directory
4️⃣  Last N commits
5️⃣  Staged changes only

Please enter your choice (1-5):

Step 2: YAGNI/KISS Analysis Framework

For each file identified, analyze for these patterns:

YAGNI Detection Patterns

1. Unused abstractions

   • Interfaces/protocols with single implementations
   • Abstract base classes with one concrete subclass
   • Generic types that are always the same

2. Premature flexibility

   • Configuration for things that never change
   • Plugin systems with no plugins
   • Feature flags that are always on/off

3. Over-engineering indicators

   • Factory classes for simple objects
   • Builder patterns for objects with 2-3 fields
   • Event systems with single listeners

4. Speculative code

   • "TODO: might need this" comments
   • Commented-out code "just in case"
   • Unreachable code paths
   • Methods that are never called

5. The GenericButton Anti-Pattern

   • Components with 8+ optional parameters serving different use cases
   • So many props that using it is as complex as writing from scratch

6. Premature Abstraction - Rule of Three

   • Abstraction created at 1st or 2nd duplication (wait for 3rd!)
   • Reference: Martin Fowler - "Tolerate duplication twice, refactor on the third"

KISS Violation Patterns

1. Verbose implementations

   • Can be reduced by >50% lines
   • Reimplements standard library functions
   • Complex regex when simple string operations work

2. Abstraction addiction

   • More than 3 levels of inheritance/wrapping
   • Interfaces between every layer

3. Clever code

   • Needs extensive comments to explain
   • Uses obscure language features unnecessarily
   • One-liners that should be 5 clear lines

4. Catch-Log-Exit Anti-Pattern

   • Catching exceptions just to log and exit
   • Replaces actual error with a guess about what went wrong

   // TERRIBLE: replaces actual error with a guess
   try {
     await createNewBranch({ branchName, cwd })
   } catch (error) {
     console.error('Error: Not in a git repository') // Maybe wrong!
     process.exit(1)
   }
   
   // CORRECT: let it throw naturally
   await createNewBranch({ branchName, cwd })

Security Patterns to Check

Even in a YAGNI/KISS review, flag critical security issues:

1. SQL Injection

   • String concatenation in SQL queries
   • Missing parameterized queries

2. Authentication/Authorization

   • Hardcoded secrets
   • Weak defaults: SECRET = os.getenv('KEY', 'default')
   • JWT without expiration

3. Unvalidated External Inputs

   • URL parameters used directly without validation
   • API response data trusted without schema validation