Legacy Code Review and Refactoring Plan

Core Philosophy

Legacy code review is not just about finding bugs—it's about understanding the original intent, reducing technical debt, and incrementally modernizing while maintaining stability. Apply the principle: "Make it work, make it right, make it fast" in that order.

Instructions

1. Discovery & Analysis

Use Read, Grep, and Glob tools to:

• Identify files referenced by the user
• Search for deprecated APIs, patterns, or libraries (e.g., collections.MutableMapping → collections.abc.MutableMapping)
• Detect code smells:
  • Unused imports/functions (candidates for deletion)
  • Duplicated code blocks (refactor to DRY)
  • High cyclomatic complexity (McCabe > 10)
  • Missing type hints (Python) or JSDoc (JavaScript)

2. Modern Tooling Recommendations

Python Legacy Code (2024-2025 Best Practices):

JavaScript/TypeScript Legacy Code:

AI-Powered Code Review (GitHub Integration):

3. Refactoring Process (Step-by-Step)

[!IMPORTANT] Always write tests BEFORE refactoring. If no tests exist, create characterization tests first.

1. Understand the Intent

   • Ask: "What problem was this solving?"
   • Review git history: git log --follow <file> and git blame <file>
   • Document assumptions in code comments

2. Establish Safety Nets

   • Run existing tests: pytest tests/ (Python) or npm test (JS)
   • Measure current coverage: pytest --cov → target 80%+ for critical paths
   • Add characterization tests if coverage < 60%

3. Incremental Refactoring (Layer by Layer)

   • Phase 1: Fix linting/formatting (Ruff, Prettier) — low risk
   • Phase 2: Add type hints (mypy, TypeScript) — medium risk
   • Phase 3: Extract functions, reduce complexity — medium risk
   • Phase 4: Replace deprecated APIs with modern equivalents — high risk
   • Phase 5: Extract to new modules (strangler fig pattern) — high risk

4. Verification Gates

   • All existing tests pass (pytest, npm test)
   • No new linting errors (ruff check, eslint)
   • Code coverage unchanged or improved (pytest --cov)
   • Security scan passes (bandit, npm audit)

5. Documentation

   • Update docstrings with modern examples
   • Add inline comments explaining why, not what
   • Create migration guide if API changes affect users

4. Common Legacy Code Patterns (Python)

5. GitHub Pull Request Best Practices (2024-2025)

[!TIP] Keep PRs small (<400 lines) for faster, more accurate reviews.

• Small PRs: Break large refactors into atomic changes
• Self-Review First: Review your own PR before requesting others
• Automated Checks: Enforce linting, tests, security scans in CI/CD
• Clear Descriptions: Explain what, why, impact, how tested
• Use Draft PRs: For work-in-progress, get early feedback
• Feature Flags: For risky changes, use toggles to A/B test old vs new logic

Output Format

Provide the following in your review:

1. Summary of Findings

   • List deprecated patterns found (with line numbers)
   • Complexity metrics (if applicable)
   • Security vulnerabilities (if any)

2. Recommended Tooling

   • Specific commands to run (e.g., ruff check --fix src/)
   • Expected output/improvements

3. Refactoring Plan (Phased)

   • Phase 1: Low-risk fixes (formatting, linting)
   • Phase 2: Medium-risk (type hints, small extractions)
   • Phase 3: High-risk (API migrations, architectural changes)

4. Example Code Block

   • Show before and after code
   • Include comments explaining the change
   • Demonstrate modern conventions

Example Usage

User Request:

"Can you review @src/legacy/old_database_connector.py and suggest how to update it for better performance and maintainability?"

Agent Response:

Summary of Findings

• Line 23: Using deprecated collections.MutableMapping (should be collections.abc.MutableMapping)
• Line 45-67: Database connection logic duplicated 3 times (DRY violation)
• Line 89: No type hints, making debugging difficult
• Complexity: McCabe score of 15 (threshold is 10)

Recommended Tooling

# Run modern linters
ruff check --fix src/legacy/old_database_connector.py
mypy src/legacy/old_database_connector.py
bandit -r src/legacy/

# Check complexity
radon cc -s src/legacy/old_database_connector.py

Refactoring Plan

Phase 1 (Low Risk):

• Fix import: collections.MutableMapping → collections.abc.MutableMapping
• Run Ruff to auto-fix formatting/linting

Phase 2 (Medium Risk):

• Add type hints to all functions
• Extract duplicated connection logic to _get_db_connection() helper

Phase 3 (High Risk):

• Replace raw SQL with SQLAlchemy ORM (if applicable)
• Implement connection pooling for performance

Example Refactoring

Before (Legacy):

from collections import MutableMapping  # DEPRECATED

def connect_db(host, port):
    conn = psycopg2.connect(host=host, port=port)  # Duplicated 3 times
    return conn

After (Modern):

from collections.abc import MutableMapping
from typing import Optional
import psycopg2
from psycopg2 import pool

# Connection pool (shared resource)
_connection_pool: Optional[pool.SimpleConnectionPool] = None

def get_db_connection(host: str, port: int) -> psycopg2.extensions.connection:
    """
    Get a database connection from the pool.
    
    Args:
        host: Database host (e.g., 'localhost')
        port: Database port (e.g., 5432)
    
    Returns:
        Active database connection
    
    Raises:
        psycopg2.OperationalError: If connection fails
    """
    global _connection_pool
    if _connection_pool is None:
        _connection_pool = pool.SimpleConnectionPool(
            minconn=1, maxconn=10, host=host, port=port
        )
    return _connection_pool.getconn()

Key Improvements:

• ✅ Fixed deprecated import
• ✅ Added type hints (str, int, psycopg2.extensions.connection)
• ✅ Extracted to single function (DRY)
• ✅ Implemented connection pooling (performance)
• ✅ Added comprehensive docstring

Verification Checklist

Before marking refactoring complete:

• All existing tests pass
• Linting passes (ruff check, eslint)
• Type checking passes (mypy, tsc)
• Security scan clean (bandit, npm audit)
• Code coverage maintained or improved
• Documentation updated
• PR reviewed by at least one peer
• Manual testing performed (if applicable)

Additional Resources

• GitHub Copilot for Legacy Code
• Refactoring Guru (Design Patterns)
• Python Type Hints Cheat Sheet
• Google's Code Review Best Practices