code-centralization

What this skill does

# Code Centralization - Mandatory Investigation Protocol

**Purpose**: Before writing ANY new code, you MUST investigate existing code to prevent duplication. This is not optional.

## CRITICAL: Investigation Before Implementation

**DEFAULT: REUSE EXISTING CODE** - You must prove similar code doesn't already exist before writing new code.

### Mandatory Pre-Implementation Checklist

Before writing ANY new function, class, or module:

1. **Search for existing implementations**
   ```bash
   # Search for similar function names
   grep -r "def similar_name" --include="*.py"

   # Search for similar logic patterns
   grep -r "pattern_keyword" --include="*.py"

   # Search imports to find utility modules
   grep -r "from.*utils import" --include="*.py"
   ```

2. **Document what you found**
   - List ALL similar functions/modules discovered
   - Explain why each cannot be reused (or CAN be reused)

3. **Justify new code only if reuse is impossible**
   - "Couldn't find similar code" is only valid with search evidence
   - "Existing code doesn't quite fit" requires explanation of gap

### Evidence Required

```markdown
### Code Investigation for: <what you're implementing>

**Search performed:**
- `grep -r "keyword1" --include="*.py"` → Found: <results>
- `grep -r "keyword2" --include="*.py"` → Found: <results>

**Similar code found:**
- `file1.py:function_name()` - Does X, but not Y (can't reuse because...)
- `file2.py:other_function()` - Does Y, can be extended to also do X

**Decision:**
- [ ] Reuse existing: <which function>
- [ ] Extend existing: <which function + what extension>
- [ ] New code justified: <why reuse/extension impossible>
```

## Core Principle

**Duplication is a maintenance liability.** When the same logic exists in multiple places, changes must be made in multiple locations, increasing the risk of inconsistencies and bugs.

## When to Centralize

### Red Flags: Code That Should Be Centralized

1. **Identical Logic in Multiple Files**
   - Same `hasattr()` checks repeated
   - Same fallback logic duplicated
   - Same validation patterns copied

2. **Similar Patterns with Minor Variations**
   - Type coercion logic repeated with slight differences
   - Property accessors with same fallback chains
   - API response building with duplicated field handling

3. **Maintenance Burden**
   - Bug fixes require changes in multiple places
   - Feature additions need updates across files
   - Tests must cover the same logic multiple times

### Example: action_resolution/outcome_resolution Duplication

**Before (Duplicated Logic)**:

```python
# ❌ BAD - llm_response.py (22 lines)
@property
def action_resolution(self) -> dict[str, Any]:
    if self.structured_response:
        if hasattr(self.structured_response, "action_resolution"):
            ar = self.structured_response.action_resolution
            if ar is not None:
                return ar
        if hasattr(self.structured_response, "outcome_resolution"):
            or_val = self.structured_response.outcome_resolution
            if or_val is not None:
                return or_val
    return {}

# ❌ BAD - world_logic.py (13 lines) - Same logic duplicated
if hasattr(structured_response, "action_resolution"):
    action_resolution = getattr(structured_response, "action_resolution", None)
    if action_resolution is not None:
        unified_response["action_resolution"] = (
            action_resolution if isinstance(action_resolution, dict) else {}
        )
if hasattr(structured_response, "outcome_resolution"):
    outcome_resolution = getattr(structured_response, "outcome_resolution", None)
    if outcome_resolution is not None:
        unified_response["outcome_resolution"] = (
            outcome_resolution if isinstance(outcome_resolution, dict) else {}
        )
```

**After (Centralized)**:

```python
# ✅ GOOD - action_resolution_utils.py (single source of truth)
def get_action_resolution(structured_response: Any) -> dict[str, Any]:
    """Get action_resolution with backward compat fallback."""
    if structured_response is None:
        return {}
    if hasattr(structured_response, "action_resolution"):
        ar = structured_response.action_resolution
        if ar is not None:
            return ar
    if hasattr(structured_response, "outcome_resolution"):
        or_val = structured_response.outcome_resolution
        if or_val is not None:
            return or_val
    return {}

def add_action_resolution_to_response(structured_response: Any, unified_response: dict[str, Any]) -> None:
    """Add action_resolution/outcome_resolution to API response."""
    if structured_response is None:
        return
    if hasattr(structured_response, "action_resolution"):
        ar = getattr(structured_response, "action_resolution", None)
        if ar is not None:
            unified_response["action_resolution"] = (
                ar if isinstance(ar, dict) else {}
            )
    if hasattr(structured_response, "outcome_resolution"):
        or_val = getattr(structured_response, "outcome_resolution", None)
        if or_val is not None:
            unified_response["outcome_resolution"] = (
                or_val if isinstance(or_val, dict) else {}
            )

# ✅ GOOD - llm_response.py (2 lines)
@property
def action_resolution(self) -> dict[str, Any]:
    return get_action_resolution(self.structured_response)

# ✅ GOOD - world_logic.py (1 line)
add_action_resolution_to_response(structured_response, unified_response)
```

**Results**:
- **Before**: 35 lines of duplicated logic across 2 files
- **After**: 20 lines in helper module + 3 lines total in consuming files
- **Net reduction**: 12 lines + single source of truth

## TDD Approach to Centralization

### Step 1: Write Tests First (RED)

Before extracting code, write comprehensive tests for the helper functions:

```python
# ✅ GOOD - Test all edge cases before extraction
def test_get_action_resolution_with_action_resolution(self):
    """Test returns action_resolution when present"""
    mock_response = MagicMock()
    mock_response.action_resolution = {"player_input": "I attack"}
    result = get_action_resolution(mock_response)
    self.assertEqual(result["player_input"], "I attack")

def test_get_action_resolution_falls_back_to_outcome_resolution(self):
    """Test falls back to outcome_resolution when action_resolution missing"""
    # ... test implementation

def test_get_action_resolution_handles_none(self):
    """Test handles None structured_response"""
    result = get_action_resolution(None)
    self.assertEqual(result, {})

# ... 15+ more edge case tests
```

### Step 2: Extract Helper Functions (GREEN)

Create helper module with functions that make tests pass:

```python
# ✅ GOOD - Helper module with clear responsibilities
# mvp_site/action_resolution_utils.py

def get_action_resolution(structured_response: Any) -> dict[str, Any]:
    """Single source of truth for fallback logic."""
    # Implementation that passes all tests

def add_action_resolution_to_response(structured_response: Any, unified_response: dict[str, Any]) -> None:
    """API response builder with type coercion."""
    # Implementation that passes all tests
```

### Step 3: Refactor Existing Code (REFACTOR)

Replace duplicated logic with helper calls:

```python
# ✅ GOOD - Replace 22 lines with 1 function call
@property
def action_resolution(self) -> dict[str, Any]:
    return get_action_resolution(self.structured_response)
```

### Step 4: Verify No Regressions

Run all existing tests to ensure behavior unchanged:

```bash
# ✅ GOOD - Verify backward compatibility
pytest mvp_site/tests/test_action_resolution.py
pytest mvp_site/tests/test_end2end/test_action_resolution_backward_compat_end2end.py
pytest mvp_site/tests/test_action_resolution_utils.py  # New helper tests
```

## Helper Module Design Principles

### 1. Single Responsibility

Each helper function should do one thing well:

```python
# ✅ GOOD - Clear, focused function
def get_action_