name: maintainability description: Maintainability assessment criteria for code review. Apply when evaluating code for readability, change tolerance, hidden assumptions, and debuggability. user-invocable: false

Maintainability Assessment

Evaluate whether code will be easy to understand, modify, extend, and debug over time.

Quick Reference

Anti-Patterns

Readability

Check: Can someone understand this in 6 months?

# HIGH COGNITIVE LOAD: Too much at once
def process(data):
    return {k: sum(x['value'] for x in v if x.get('active', True))
            for k, v in groupby(sorted(filter(lambda x: x['status'] != 'deleted',
            data), key=lambda x: x['category']), key=lambda x: x['category'])}

# READABLE: Named intermediate steps
def process(data):
    active_items = [x for x in data if x['status'] != 'deleted']
    sorted_items = sorted(active_items, key=lambda x: x['category'])

    result = {}
    for category, items in groupby(sorted_items, key=lambda x: x['category']):
        result[category] = sum(x['value'] for x in items if x.get('active', True))
    return result

Heuristics

• Can you explain what a function does in one sentence?
• Would a new team member understand this without asking questions?
• Are variable names descriptive enough to skip comments?

Severity

• Critical: Understanding requires significant mental effort
• Improvement: Some parts require careful reading
• Nitpick: Minor clarity improvements possible

Change Tolerance

Check: How many places need modification for typical changes?

# BRITTLE: User display logic scattered
# views.py
def user_profile(user):
    return f"{user.first_name} {user.last_name}"

# emails.py
def format_recipient(user):
    return f"{user.first_name} {user.last_name} <{user.email}>"

# Adding middle name requires 2+ file changes

# RESILIENT: Centralized
class User:
    @property
    def display_name(self) -> str:
        return f"{self.first_name} {self.last_name}"

    @property
    def email_display(self) -> str:
        return f"{self.display_name} <{self.email}>"

Severity

• Critical: Typical changes require 5+ file modifications
• Improvement: 2-4 files need coordinated changes
• Nitpick: Changes generally localized but could be cleaner

Extensibility

Check: Can new behavior be added without modifying existing code?

# CLOSED: Adding new type requires modifying existing code
def process_payment(payment):
    if payment.type == "credit_card":
        process_credit_card(payment)
    elif payment.type == "paypal":
        process_paypal(payment)
    elif payment.type == "crypto":  # had to add this
        process_crypto(payment)

# OPEN: New types don't touch existing code
class PaymentProcessor(Protocol):
    def process(self, payment: Payment) -> Result: ...

PROCESSORS: dict[str, PaymentProcessor] = {
    "credit_card": CreditCardProcessor(),
    "paypal": PayPalProcessor(),
}

def process_payment(payment):
    return PROCESSORS[payment.type].process(payment)

Severity

• Improvement: Extending requires modifying existing functions/classes
• Nitpick: Extension points exist but could be cleaner

Hidden Assumptions

Check: Are constraints and expectations explicit?

# HIDDEN: items is never empty
def get_average(items):
    return sum(items) / len(items)  # ZeroDivisionError

# EXPLICIT: Documented and enforced
def get_average(items: list[float]) -> float:
    """Calculate average. Raises ValueError if items is empty."""
    if not items:
        raise ValueError("Cannot calculate average of empty list")
    return sum(items) / len(items)

Severity

• Critical: Hidden assumptions will cause production bugs
• Improvement: Assumptions reasonable but not documented
• Nitpick: Assumptions obvious from context

Debuggability

Check: When things fail, is it clear why?

# BAD: Silent failure
def update_user(user_id, data):
    user = get_user(user_id)
    if user:
        user.update(data)
    # if user doesn't exist... nothing happens

# BAD: Exception swallowing
try:
    result = complex_operation()
except Exception:
    pass

# GOOD: Specific error with context
def update_user(user_id, data):
    user = get_user(user_id)
    if user is None:
        raise UserNotFoundError(f"No user with id {user_id}")
    user.update(data)

# GOOD: Logged with structured context
def process_batch(items):
    for item in items:
        try:
            process(item)
        except Exception as e:
            logger.error(f"Failed to process item {item.id}: {e}",
                        extra={"item_id": item.id, "item_type": item.type})

Severity

• Critical: Failures are silent or swallowed
• Improvement: Errors lack context for debugging
• Nitpick: Errors could be slightly more informative

Intent Documentation

Check: Is the "why" captured, not just the "what"?

# USELESS: Restates the code
x = x + 1  # increment x

# USEFUL: Explains why
x = x + 1  # account for 0-indexing in the API response

# WITHOUT CONTEXT: What is this magic?
if (n & (n - 1)) == 0 and n != 0:
    ...

# WITH CONTEXT: Clear purpose
# Check if n is a power of 2 (only one bit set)
if (n & (n - 1)) == 0 and n != 0:
    ...

Severity

• Improvement: Complex logic lacks explanation
• Nitpick: Documentation could be clearer but code is understandable

Quick Smell Test

Answer these for any significant code change:

1. New developer test: Could someone unfamiliar fix a bug in this?
2. 2am test: Could you debug this at 2am with incomplete logs?
3. Feature request test: Could you add a related feature without rewriting?
4. Handoff test: Could you explain this to a colleague in 5 minutes?

If any answer is "no", the code likely has maintainability issues worth flagging.

Convention Reference

When flagging maintainability issues in reviews, reference this skill:

**Convention**: See `maintainability` skill: Debuggability