strat-gameplay-webapp/BUGFIX_DOUBLE_PLAY_RECOVERY.md
Cal Corum dd1b087af1 CLAUDE: Fix double play bug after state recovery
Critical bug fix for issue where Groundball A with runner on first would
fail to execute a double play after game recovery from database.

Root cause: current_on_base_code field was not recalculated during state
recovery, defaulting to 0 (empty bases) even when runners were on base.
This caused runner advancement logic to select Result 1 (batter out,
runners hold) instead of Result 2 (double play).

Changes:
- Added calculate_on_base_code() helper method to GameState model
- Updated _prepare_next_play() to use helper (eliminates duplication)
- Fixed state recovery to calculate current_on_base_code from runners
- Fixed X-Check G1 mapping (was GROUNDBALL_B, should be GROUNDBALL_A)
- Added 5 regression tests to prevent recurrence

Testing:
- All 359 unit tests passing
- New regression tests verify fix and demonstrate bug scenario
- Tested in network dev environment - double plays now work correctly

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-07 13:55:10 -06:00

153 lines
4.7 KiB
Markdown

# Bug Fix: Double Play Not Working After Game Recovery
**Issue**: [Gitea #3](https://git.manticorum.com/cal/strat-gameplay-webapp/issues/3)
**Reported By**: User during test game
**Date**: 2025-02-07
## Problem Description
When a user rejoined a game (after the game state was evicted from memory and recovered from the database), rolling a Groundball A with 0 outs and a runner on first base incorrectly resulted in:
- ❌ Runner on first out at second (correct)
- ❌ Batter **safe** at first (WRONG - should be out)
-**1 out recorded** (WRONG - should be 2 outs)
**Expected Behavior**: Double play - runner out at second, batter out at first, 2 outs recorded.
## Root Cause
The `current_on_base_code` field in `GameState` is a **snapshot field** that stores the base situation as a bit field (1=first, 2=second, 4=third, 7=loaded). This field is used by the runner advancement logic to determine which result to apply.
During normal gameplay, this field is set by `_prepare_next_play()` in the game engine. However, during **state recovery** from the database, it was not being recalculated, leaving it at the default value of `0` (empty bases).
This caused the runner advancement logic to select:
- **Result 1** (batter out, runners hold) instead of
- **Result 2** (double play at second and first)
## Fix Summary
### 1. Created Helper Method (PR #1)
Added `calculate_on_base_code()` method to `GameState` model to centralize the bit field calculation:
**File**: `backend/app/models/game_models.py`
```python
def calculate_on_base_code(self) -> int:
"""
Calculate on-base code from current runner positions.
Returns bit field where:
- Bit 0 (value 1): runner on first
- Bit 1 (value 2): runner on second
- Bit 2 (value 4): runner on third
- Value 7: bases loaded (1 + 2 + 4)
"""
code = 0
if self.on_first:
code |= 1
if self.on_second:
code |= 2
if self.on_third:
code |= 4
return code
```
### 2. Updated Game Engine (PR #1)
Refactored `_prepare_next_play()` to use the helper method:
**File**: `backend/app/core/game_engine.py` (line ~1014)
```python
# Before:
state.current_on_base_code = 0
if state.on_first:
state.current_on_base_code |= 1
if state.on_second:
state.current_on_base_code |= 2
if state.on_third:
state.current_on_base_code |= 4
# After:
state.current_on_base_code = state.calculate_on_base_code()
```
### 3. Fixed State Recovery (PR #1)
Added calculation of `current_on_base_code` at the end of state recovery:
**File**: `backend/app/core/state_manager.py` (line ~608)
```python
# CRITICAL FIX: Calculate current_on_base_code from actual runner positions
# This field is used by runner advancement logic to determine double plays
# Without this, recovered games default to on_base_code=0 (empty bases)
# even when runners are on base, breaking double play logic
state.current_on_base_code = state.calculate_on_base_code()
logger.debug(f"Recovery: Set current_on_base_code = {state.current_on_base_code}")
```
### 4. Fixed X-Check G1 Mapping (PR #2)
Also discovered and fixed a separate bug where X-Check G1 results were incorrectly mapped to GROUNDBALL_B instead of GROUNDBALL_A:
**File**: `backend/app/core/play_resolver.py` (line ~1450)
```python
# Before:
"G1": PlayOutcome.GROUNDBALL_B, # WRONG
# After:
"G1": PlayOutcome.GROUNDBALL_A, # Fixed: G1 is fast grounder (double play capable)
```
## Testing
### Regression Test Added
**File**: `backend/tests/unit/core/test_recovery_double_play_fix.py`
- ✅ Test verify `calculate_on_base_code()` for all base combinations
- ✅ Test recovered state with runner on first executes double play
- ✅ Test demonstrates bug behavior without fix
### Test Results
```
359 unit tests passing
5 new regression tests passing
```
## Impact
**Before Fix**:
- Games recovered from database would have incorrect on_base_code (always 0)
- Double plays would not work correctly after rejoining a game
- Force outs and other runner advancement scenarios also affected
**After Fix**:
- State recovery correctly calculates on_base_code from runner positions
- Double plays work correctly regardless of whether state was recovered
- All runner advancement logic functions properly after rejoin
## Related Issues
- This also fixes any other runner advancement issues that depend on `current_on_base_code` being accurate
- Prevents similar issues with Result 4 (force out), Result 10 (home-to-first DP), etc.
## Deployment Notes
No database migration required. The fix is purely in the application logic.
Users who experienced this bug should:
1. Refresh their browser after deployment
2. Previous games will work correctly after next state recovery
---
**Fixed**: 2025-02-07
**Tested**: ✅ All unit tests passing
**Status**: Ready for deployment