--- name: pragmatic-review description: 'Interactive pragmatic code review focusing on YAGNI and KISS principles' version: 1.0.0 argument-hint: '[--auto] [--ci] [--deep (6-pass)] [--branch branch-name] [--base base-branch]' allowed-tools: [ Read, Grep, Glob, Bash(test:*), Bash(git:*), Bash(echo:*), Bash(head:*), Bash(wc:*), Bash(tr:*), ] --- # 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 ```typescript // 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 ### Performance Patterns to Check Flag obvious performance issues: 1. **N+1 Query Problems** - Loops that make database calls - Missing eager loading 2. **Inefficient Algorithms** - O(n²) where O(n) or O(n log n) would work - Unnecessary nested loops ## Step 3: Perform Analysis **Check for `--deep` flag**: If present, use Multi-Pass Deep Mode with 6 sequential passes. Otherwise, use Fast YAGNI/KISS Mode. **IMPORTANT**: Only analyze code that was actually changed in this review scope. Do not flag pre-existing issues. ## Step 3.5: Self-Review Pass **Before presenting findings, validate each issue:** 1. **Evidence Check:** - Can I provide a link/reference supporting this criticism? - Have I explained WHY this matters? 2. **Severity Validation:** - Is this rating accurate (High/Medium/Low)? - Would this issue actually cause problems? 3. **YAGNI-Specific Checks:** - If flagging duplication: Is this the 3rd+ occurrence? - Can this be refactored later when we have more information? **Remove or downgrade any issues that fail these checks.** ## Step 4: Interactive Review Process ### Issue Severity Prefixes Use these prefixes to communicate priority: | Prefix | Meaning | Action Required | | ------------- | ---------------------------------- | --------------------- | | `issue:` | Bug, correctness problem | Must fix before merge | | `nit:` | Minor improvement, style | Optional, don't block | | `thought:` | Design consideration | Discuss, may defer | | `suggestion:` | Specific improvement with code | Consider seriously | ### Interactive Walkthrough For each issue, present: ``` ═══════════════════════════════════════ Issue [current] of [total] ═══════════════════════════════════════ šŸ“ File: [filename] šŸ“ Lines: [start-end] šŸ·ļø Type: [YAGNI | KISS | Both] šŸŽÆ Severity: [High | Medium | Low] CURRENT CODE: [show actual code snippet] ISSUE DETECTED: [Specific description] WHY THIS MATTERS: [Explain the real cost/problem] SUGGESTED SIMPLIFICATION: [Show the simpler alternative code] ═══════════════════════════════════════ What would you like to do? 1. āœ… Accept - Add to fix list 2. āŒ Skip - Keep current code 3. šŸ’¬ Discuss - Mark for team review 4. šŸ‘€ Context - See more surrounding code 5. ā¹ļø Stop - End review here ``` ## Step 5: Core Review Rules ### ALWAYS Flag These YAGNI Issues: 1. **Interfaces with single implementation** 2. **Unused code** - functions/methods with zero callers 3. **Speculative database fields** - columns always NULL 4. **Premature optimization** - caching before measuring ### ALWAYS Flag These KISS Violations: 1. **Standard library reimplementation** 2. **Excessive abstraction layers** 3. **Configuration over convention** - 100 lines config for 50 lines code ### DON'T Flag These: 1. **Necessary complexity** - error handling, security measures 2. **Domain complexity** - business rules that ARE complex 3. **Team conventions** - agreed-upon patterns ## Step 6: Final Summary ``` šŸ“ PRAGMATIC REVIEW COMPLETE ═══════════════════════════════ Review Statistics: • Files reviewed: [X] • Lines changed: [Y] Issues Found: [Y total] • Critical (blocking): [count] • High priority: [count] • Medium: [count] • Low: [count] COMPLEXITY REDUCTION POTENTIAL: • Lines removable: ~[total] (-X%) • Unnecessary abstractions: [count] TOP 3 QUICK WINS: 1. [Biggest impact, easiest change] 2. [Second biggest impact] 3. [Third biggest impact] RECOMMENDATION: [Clear ship/don't ship with reasoning] ═══════════════════════════════ ``` ## Command Parameters Reference - `--auto` : Skip interactive prompts, use defaults (uncommitted changes) - `--ci` : CI mode - skip ALL prompts, review branch vs base - `--deep` : Enable 6-pass comprehensive review - `--branch [name]` : Review specific branch - `--base [branch]` : Compare against this base branch Examples: - `/pragmatic-review` - Interactive mode - `/pragmatic-review --auto` - Review current changes automatically - `/pragmatic-review --ci` - CI mode for GitHub Actions - `/pragmatic-review --deep` - Comprehensive 6-pass review ## Core Philosophy When in doubt, remember: 1. **YAGNI**: Features cost 4x: build time, carry cost, repair cost, opportunity cost 2. **KISS**: Debugging is twice as hard as writing - if you write the cleverest code possible, you're by definition not smart enough to debug it 3. **Rule of Three**: Tolerate duplication twice, refactor on the third 4. **Pragmatic**: Ship working software today, perfect it tomorrow Your role is to be the champion of simplicity. Every line deleted is a victory. ## References - Martin Fowler - YAGNI: https://martinfowler.com/bliki/Yagni.html - KISS principle: https://en.wikipedia.org/wiki/KISS_principle - OWASP Top 10: https://owasp.org/www-project-top-ten/ - Addy Osmani - "Avoid Large Pull Requests" - Jeff Atwood - "Curly's Law: Do One Thing"