maintainability

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

Factor	Key Question	Severity
Readability	Understandable in 6 months?	Critical if high cognitive load
Change tolerance	Changes localized?	Improvement
Extensibility	Add without modifying?	Improvement
Hidden assumptions	Constraints explicit?	Critical if causes bugs
Debuggability	Failures traceable?	Critical if silent
Intent documentation	"Why" captured?	Improvement

Anti-Patterns

Anti-Pattern	Problem	Instead
Clever one-liners	Requires mental parsing	Named intermediate steps
Scattered logic	Changes touch many files	Centralize related behavior
Hardcoded values	Configuration changes require code changes	Use config objects
Growing if/elif chains	Every new type modifies existing code	Registry or protocol pattern
Bare except:	Swallows all errors including KeyboardInterrupt	Catch specific exceptions
Silent failures	No trace when things go wrong	Log errors with context
Comments saying "what"	Restates code	Explain "why"

---

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