strat-gameplay-webapp/.claude/implementation/GAMESTATE_REFACTOR_PLAN.md

# GameState Refactor Plan - Self-Contained State with Position Ratings

**Created**: 2025-11-03
**Completed**: 2025-11-04
**Status**: ✅ COMPLETE - All phases implemented and tested
**Priority**: High - Prerequisite for Phase 3E (SATISFIED)

## Problem Statement

### Current Architectural Inconsistency

GameState has an inconsistency in how it references players:

```python
# FULL OBJECTS for runners
on_first: Optional[LineupPlayerState] = None
on_second: Optional[LineupPlayerState] = None
on_third: Optional[LineupPlayerState] = None

# ONLY IDs for active players
current_batter_lineup_id: int
current_pitcher_lineup_id: Optional[int] = None
current_catcher_lineup_id: Optional[int] = None
```

This creates several problems:
1. **Inconsistent API**: Runners are objects, but batter/pitcher/catcher are IDs
2. **Extra lookups needed**: PlayResolver must look up lineup to get player details
3. **Coupling**: PlayResolver depends on StateManager for player lookups
4. **Missing data**: No direct access to position ratings for X-Check resolution

## Proposed Solution

### Phase 1: Make All Player References Consistent

Change `current_batter/pitcher/catcher_lineup_id` to full `LineupPlayerState` objects:

```python
class GameState(BaseModel):
    # ... existing fields

    # Runners (already full objects) ✅
    on_first: Optional[LineupPlayerState] = None
    on_second: Optional[LineupPlayerState] = None
    on_third: Optional[LineupPlayerState] = None

    # CHANGE: Make these full objects too
    current_batter: LineupPlayerState
    current_pitcher: Optional[LineupPlayerState] = None
    current_catcher: Optional[LineupPlayerState] = None

    # KEEP for database persistence (Play table needs IDs)
    current_on_base_code: int = Field(default=0, ge=0)
```

### Phase 2: Add Position Ratings to LineupPlayerState

Enhance `LineupPlayerState` to hold position ratings loaded at game start:

```python
# In app/models/player_models.py
@dataclass
class PositionRating:
    """Defensive ratings for a player at a specific position."""
    position: str       # Position code (SS, 3B, LF, etc.)
    range: int          # Range rating (1-5)
    error: int          # Error rating (0-88 depending on position)
    arm: int            # Arm strength (1-5)
    speed: Optional[int] = None  # Speed rating (0-20) for SPD tests

# In app/models/game_models.py
class LineupPlayerState(BaseModel):
    lineup_id: int
    card_id: int
    position: str
    batting_order: Optional[int] = None
    is_active: bool = True

    # NEW: Position rating (loaded at game start for PD league)
    position_rating: Optional[PositionRating] = None
```

### Phase 3: Load Position Ratings at Game Start

Create PD API client and load ratings when lineups are created:

```python
# In app/services/pd_api_client.py (NEW)
async def fetch_position_rating(player_id: int, position: str) -> PositionRating:
    """Fetch position rating from PD API."""
    url = f"{PD_API_BASE}/cardpositions/player/{player_id}"
    async with httpx.AsyncClient() as client:
        response = await client.get(url)
        data = response.json()

        # Find rating for this position
        for pos_data in data.get('positions', []):
            if pos_data['position'] == position:
                return PositionRating(
                    position=position,
                    range=pos_data['range'],
                    error=pos_data['error'],
                    arm=pos_data['arm'],
                    speed=pos_data.get('speed')
                )
    return None

# In game_engine.py - at lineup creation
async def _load_lineup_with_ratings(self, lineup_entries, league_id):
    """Load lineup and attach position ratings for PD league."""
    lineup_players = []

    for entry in lineup_entries:
        player = LineupPlayerState(
            lineup_id=entry.id,
            card_id=entry.card_id,
            position=entry.position,
            batting_order=entry.batting_order
        )

        # Load position rating for PD league
        if league_id == 'pd':
            rating = await pd_api.fetch_position_rating(
                entry.card_id,
                entry.position
            )
            player.position_rating = rating

        lineup_players.append(player)

    return lineup_players
```

## Benefits

### 1. Architectural Consistency ✅
- All player references in GameState are `LineupPlayerState` objects
- Uniform API for accessing player data
- More intuitive to work with

### 2. Self-Contained State ✅
- GameState has everything needed for play resolution
- No external lookups required during resolution
- Truly represents "complete game state"

### 3. Simplified PlayResolver ✅
```python
# BEFORE (needs StateManager)
def _resolve_x_check(self, state, state_manager, ...):
    lineup = state_manager.get_lineup(state.game_id, team_id)
    defender = next(p for p in lineup.players if p.position == position)
    # Still need to fetch rating somehow...

# AFTER (fully self-contained)
def _resolve_x_check(self, state, ...):
    lineup = self.state_manager.get_lineup(state.game_id, team_id)
    defender = next(p for p in lineup.players if p.position == position)

    # Direct access to everything needed!
    defender_range = defender.position_rating.range
    defender_error = defender.position_rating.error
    batter_speed = state.current_batter.position_rating.speed
```

### 4. Better Performance ✅
- Ratings loaded **once** at game start
- No API calls during play resolution
- No Redis lookups during plays
- Faster resolution (<500ms target easily met)

### 5. Redis Still Useful ✅
- Cache ratings for future games with same players
- Reduce PD API load
- Fast game recovery (load ratings from cache)

## Trade-offs

### Memory Impact
- `LineupPlayerState` currently: ~100 bytes
- Adding `PositionRating`: +50 bytes
- Per game (18 players): 18 × 150 = **~2.7KB total**
- **Verdict**: Negligible ✅

### Complexity
- **Loading**: Slightly more complex at game start (API calls)
- **Resolution**: Much simpler (direct access)
- **Overall**: Net reduction in complexity ✅

### Database
- Play table still stores `batter_id`, `pitcher_id`, etc. (IDs)
- No schema changes needed ✅
- IDs used for referential integrity, objects used in memory

## Implementation Plan

### Phase 3E-Prep: Refactor GameState (2-3 hours)

**Goal**: Make all player references consistent (full objects)

**Files to Modify**:
1. `backend/app/models/game_models.py`
   - Change fields: `current_batter`, `current_pitcher`, `current_catcher`
   - Remove: `current_batter_lineup_id`, etc. (IDs)
   - Update validators if needed

2. `backend/app/core/game_engine.py`
   - Update `_prepare_next_play()` to set full objects
   - Change from: `state.current_batter_lineup_id = batter.id`
   - Change to: `state.current_batter = LineupPlayerState(...)`

3. `backend/app/core/play_resolver.py`
   - Update any references to `state.current_batter_lineup_id`
   - Change to: `state.current_batter.lineup_id`

4. `backend/app/database/operations.py`
   - When saving Play, extract IDs from objects
   - Change from: `batter_id=state.current_batter_lineup_id`
   - Change to: `batter_id=state.current_batter.lineup_id`

5. Update all tests
   - `tests/unit/models/test_game_models.py`
   - `tests/unit/core/test_game_engine.py`
   - `tests/integration/test_state_persistence.py`

**Acceptance Criteria**:
- [x] All player references in GameState are `LineupPlayerState` objects ✅
- [x] All tests passing (609/609 unit tests) ✅
- [x] No regressions in existing functionality ✅
- [x] Type checking passes ✅

**Completion Notes** (2025-11-04):
- Commits: cf7cc23, 76e0142, bb78de2, e6bd66e, c7b376d
- 7 files modified (game_models, game_engine, play_resolver, runner_advancement, state_manager, display, repl)
- 34 runner advancement tests passing
- DO3 bug fixed, game recovery working
- Documentation updated in CLAUDE.md files

**Test Command**:
```bash
pytest tests/unit/models/test_game_models.py -v
pytest tests/unit/core/test_game_engine.py -v
pytest tests/integration/ -v
```

---

### Phase 3E-Main: Add Position Ratings (3-4 hours)

**Goal**: Load and attach position ratings at game start

**Files to Create**:
1. `backend/app/services/pd_api_client.py` (NEW)
   - `fetch_position_rating(player_id, position)` - Get rating from PD API
   - `fetch_all_positions(player_id)` - Get all positions for a player
   - Async HTTP client with proper error handling

2. `backend/app/services/position_rating_cache.py` (NEW)
   - Redis caching wrapper
   - Cache key: `position_rating:{player_id}:{position}`
   - TTL: 24 hours
   - Graceful degradation if Redis unavailable

**Files to Modify**:
1. `backend/app/models/player_models.py`
   - Add `PositionRating` dataclass
   - Fields: position, range, error, arm, speed

2. `backend/app/models/game_models.py`
   - Add `position_rating: Optional[PositionRating]` to `LineupPlayerState`

3. `backend/app/core/game_engine.py`
   - Add `_load_lineup_with_ratings()` method
   - Call during lineup creation
   - Handle PD vs SBA (only load for PD)

4. `backend/app/config/settings.py`
   - Add `PD_API_URL` setting
   - Add `PD_API_TOKEN` setting (if needed)

**Tests to Create**:
1. `tests/unit/services/test_pd_api_client.py`
   - Mock HTTP responses
   - Test successful fetch
   - Test error handling
   - Test API response parsing

2. `tests/unit/services/test_position_rating_cache.py`
   - Test cache hit/miss
   - Test TTL expiration
   - Test graceful degradation

3. `tests/integration/test_lineup_rating_loading.py`
   - End-to-end test of lineup creation with ratings
   - Verify ratings attached to LineupPlayerState

**Acceptance Criteria**:
- [x] PD API client created and tested ✅
- [x] Redis caching implemented (760x speedup achieved) ✅
- [x] Ratings loaded at game start for PD league ✅
- [x] SBA league unaffected (no ratings loaded) ✅
- [x] All tests passing ✅
- [x] Graceful handling of API failures ✅

**Completion Notes** (2025-11-03):
- Commits: 02e816a, 7d15018, adf7c76
- Files created: pd_api_client.py, position_rating_service.py, redis_client.py
- Performance: 0.274s API → 0.000361s Redis (760x speedup)
- Live Redis integration test validated
- WebSocket events enhanced with X-Check details

**Test Command**:
```bash
pytest tests/unit/services/test_pd_api_client.py -v
pytest tests/unit/services/test_position_rating_cache.py -v
pytest tests/integration/test_lineup_rating_loading.py -v
```

---

### Phase 3E-Final: Update X-Check Resolution (1 hour)

**Goal**: Use attached position ratings in X-Check resolution

**Files to Modify**:
1. `backend/app/core/play_resolver.py`
   - Update `_resolve_x_check()` to use defender ratings
   - Remove placeholders:
     ```python
     # OLD (placeholder)
     defender_range = 3
     defender_error_rating = 10

     # NEW (from GameState)
     defender_range = defender.position_rating.range
     defender_error_rating = defender.position_rating.error
     ```
   - Update SPD test to use batter speed
   - Add fallback for missing ratings (use league defaults)

2. Update tests
   - Add position ratings to test fixtures
   - Test X-Check with actual ratings
   - Test fallback behavior

**Acceptance Criteria**:
- [x] X-Check uses actual position ratings ✅
- [x] SPD test uses actual batter speed ✅
- [x] Graceful fallback for missing ratings ✅
- [x] All PlayResolver tests passing ✅
- [x] Integration test with full flow (terminal client testing) ✅

**Completion Notes** (2025-11-04):
- Commits: bb78de2, 8fb740f
- Terminal client integration with `resolve_with x-check <position>` command
- Complete X-Check resolution with defense tables and error charts
- All resolution steps shown with audit trail
- Works with actual player ratings from PD API

**Test Command**:
```bash
pytest tests/unit/core/test_play_resolver.py -v
pytest tests/integration/test_x_check_full_flow.py -v
```

---

## Verification Steps

After all phases complete:

1. **Unit Tests**:
   ```bash
   pytest tests/unit/ -v
   ```
   Expected: All tests passing

2. **Integration Tests**:
   ```bash
   pytest tests/integration/ -v
   ```
   Expected: All tests passing

3. **Type Checking**:
   ```bash
   mypy app/
   ```
   Expected: No new type errors

4. **Manual Testing** (Terminal Client):
   ```bash
   python -m terminal_client
   > new_game
   > start_game
   > defensive
   > offensive
   > resolve  # Should use actual ratings for X-Check
   > status
   ```

5. **Memory Usage**:
   - Check GameState size in memory
   - Expected: < 5KB per game (negligible increase)

6. **Performance**:
   - Time play resolution
   - Expected: < 500ms (should be faster with direct access)

## Success Criteria ✅ ALL COMPLETE

Phase 3E is **100% complete** as of 2025-11-04:

- [x] All player references in GameState are consistent (full objects) ✅
- [x] Position ratings loaded at game start (PD league) ✅
- [x] X-Check resolution uses actual ratings (no placeholders) ✅
- [x] Redis caching implemented and working (760x speedup) ✅
- [x] All unit tests passing (609/609 tests) ✅
- [x] All integration tests passing (with documented asyncpg issues) ✅
- [x] Type checking passes with no new errors ✅
- [x] Documentation updated (CLAUDE.md, NEXT_SESSION.md) ✅
- [x] Performance targets met (< 500ms resolution) ✅
- [x] Memory usage acceptable (< 5KB increase per game) ✅

**Additional Achievements**:
- Terminal client X-Check testing (`resolve_with x-check <position>`)
- 100% test requirement policy with git hooks
- DO3 bug fixed, game recovery working
- 679 total tests in test suite

## Rollback Plan

If issues arise during implementation:

1. **Phase 3E-Prep issues**:
   - Revert game_models.py changes
   - Keep using IDs instead of objects
   - Phase 3E-Main can still proceed with lookups

2. **Phase 3E-Main issues**:
   - Skip position rating loading
   - Use default/placeholder ratings
   - Complete Phase 3E-Prep first (still valuable)

3. **Phase 3E-Final issues**:
   - Keep placeholders in PlayResolver
   - Complete position rating loading
   - Update resolution in separate PR

## Related Documentation

- **Phase 3 Overview**: `./.claude/implementation/PHASE_3_OVERVIEW.md`
- **Next Session Plan**: `./.claude/implementation/NEXT_SESSION.md`
- **Game Models**: `../backend/app/models/CLAUDE.md`
- **Play Resolver**: `../backend/app/core/play_resolver.py` (lines 589-758)

---

**Estimated Total Time**: 6-8 hours (spread across 3 phases)

**Priority**: High - This refactor makes Phase 3E cleaner and more maintainable

**Blocking Work**: None - can start immediately

**Next Milestone**: Phase 3F (Testing & Integration)