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

# Phase 3D Critical Bug Fix - on_base_code Mapping Error

**Status**: ✅ COMPLETE
**Date Discovered**: 2025-11-02
**Date Fixed**: 2025-11-02
**Severity**: CRITICAL - All advancement calculations were wrong
**Progress**: 100% Complete (59/59 X-Check tests passing, 381/386 total core/config tests passing)

---

## The Bug

The initial implementation **incorrectly treated `on_base_code` as a bit field** when it is actually a **sequential mapping**.

### ❌ WRONG Implementation (Original)
```python
# Treated as bit field
r1_on = (on_base_code & 1) != 0  # bit 0
r2_on = (on_base_code & 2) != 0  # bit 1
r3_on = (on_base_code & 4) != 0  # bit 2

# This caused:
# Code 3 (0b011) → R1 + R2 (WRONG! Should be R3 only)
# Code 4 (0b100) → R3 only (WRONG! Should be R1+R2)
```

### ✅ CORRECT Implementation (Fixed)
```python
# Sequential mapping
on_base_mapping = {
    0: (False, False, False),  # Empty
    1: (True, False, False),   # R1
    2: (False, True, False),   # R2
    3: (False, False, True),   # R3
    4: (True, True, False),    # R1+R2
    5: (True, False, True),    # R1+R3
    6: (False, True, True),    # R2+R3
    7: (True, True, True),     # Loaded
}
r1_on, r2_on, r3_on = on_base_mapping.get(on_base_code, (False, False, False))
```

---

## What Has Been Fixed ✅

### 1. Table Entry Remapping (COMPLETE)
**Files Modified**:
- `backend/app/core/x_check_advancement_tables.py`

**Changes**:
- ✅ G1 table: Swapped codes 3 and 4 entries
- ✅ G2 table: Swapped codes 3 and 4 entries
- ✅ G3 table: Swapped codes 3 and 4 entries
- ✅ Updated all comments to reflect correct mapping

**Result**: All 240 table entries now use correct base situation codes.

### 2. Decoding Logic Fixed (COMPLETE)
**Functions Updated**:
- ✅ `build_advancement_from_code()` - Lines 521-533
- ✅ `build_flyball_advancement_with_error()` - Lines 644-655

**Changes**: Both functions now use mapping dictionary instead of bit field math.

### 3. Documentation Updated (COMPLETE)
**Changes**:
- ✅ Updated header comments in `x_check_advancement_tables.py` (lines 40-48)
- ✅ Clarified that mapping is NOT a bit field

---

## What Still Needs Fixing ⚠️

### Test Expectation Updates (✅ COMPLETE)

**File**: `backend/tests/unit/core/test_x_check_advancement_tables.py`

**Status**: All 13 test failures fixed. All tests now use correct mapping.

#### Category A: Table Lookup Tests (7 failures)

Tests that use wrong `on_base_code` values:

1. **test_g1_r1_r2_normal_no_error** (Line ~99)
   - Current: `on_base_code=3` (expects R1+R2)
   - Fix: Change to `on_base_code=4`
   - Status: ✅ FIXED

2. **test_g1_r1_r2_infield_in_no_error** (Line ~107)
   - Current: `on_base_code=3`
   - Fix: Change to `on_base_code=4`
   - Status: ✅ FIXED

3. **test_g1_r3_only_normal_no_error** (Line ~116)
   - Current: `on_base_code=4` (expects R3)
   - Fix: Change to `on_base_code=3`
   - Status: ✅ FIXED

4. **test_g1_r3_only_infield_in_no_error** (Line ~125)
   - Current: `on_base_code=4`
   - Fix: Change to `on_base_code=3`
   - Status: ✅ FIXED

5. **test_g2_r1_r2_infield_in_no_error** (Line ~187)
   - Current: `on_base_code=3`
   - Fix: Change to `on_base_code=4`
   - Status: ✅ FIXED

6. **test_g2_r3_only_infield_in_no_error** (Line ~203)
   - Current: `on_base_code=4`
   - Fix: Change to `on_base_code=3`
   - Status: ✅ FIXED

7. **test_g3_r3_only_infield_in_decide** (Line ~267)
   - Current: `on_base_code=4`
   - Fix: Change to `on_base_code=3`
   - Status: ✅ FIXED

#### Category B: Error Advancement Tests (3 failures)

Tests that expect wrong number of runs due to incorrect runner positions:

8. **test_e1_runner_on_third** (Line ~186)
   - Current: `on_base_code=4` (expects R3, should score 1 run)
   - Issue: With wrong mapping, code 4 = R3, but logic decoded it as empty
   - Fix: Change to `on_base_code=3`
   - Status: ✅ FIXED

9. **test_flyball_e1_runner_on_third** (Line ~334)
   - Current: `on_base_code=4`
   - Fix: Change to `on_base_code=3`
   - Status: ✅ FIXED

10. **test_scenario_flyball_to_outfield_runner_tags** (Line ~769)
    - Current: `on_base_code=4`
    - Fix: Change to `on_base_code=3`
    - Status: ✅ FIXED

#### Category C: Integration Tests (3 failures)

Tests that combine wrong codes with wrong expectations:

11. **test_x_check_g1_integration** (Line ~548)
    - Current: `on_base_code=3, defender_in=True, error_result='E1'`
    - Issue: Expects 0 runs, but gets 1 run (R3 scores)
    - Analysis: Test says "runners on 1st and 2nd" but uses code 3 (R3 only)
    - Fix: Change to `on_base_code=4` for R1+R2, update expectation
    - Status: ✅ FIXED

12. **test_x_check_g3_integration** (Line ~576)
    - Current: `on_base_code=4, defender_in=False, error_result='E3'`
    - Issue: Expects 1 run, but gets 2 runs
    - Analysis: Test says "runner on 3rd" but uses code 4 (R1+R2)
    - Fix: Change to `on_base_code=3` for R3 only, update to expect 1 run
    - Status: ✅ FIXED

13. **test_scenario_runner_on_third_two_outs_infield_in** (Line ~758)
    - Current: `on_base_code=4, defender_in=True`
    - Issue: Test description says "Runner on 3rd" but uses code 4
    - Fix: Change to `on_base_code=3`
    - Status: ✅ FIXED

---

## Systematic Fix Checklist

### Step 1: Global Search & Replace ✅ COMPLETE
```bash
# Search for all test uses of on_base_code
grep -n "on_base_code=" tests/unit/core/test_x_check_advancement_tables.py

# Pattern to identify:
# - Tests mentioning "R3" or "3rd" with code=4 → change to code=3
# - Tests mentioning "R1+R2" or "1st and 2nd" with code=3 → change to code=4
# - Tests mentioning "R1+R3" or "1st and 3rd" with code=5 → keep as is (correct)
# - Tests mentioning "R2+R3" or "2nd and 3rd" with code=6 → keep as is (correct)
```

### Step 2: Update Test Expectations ✅ COMPLETE

For each test:
1. ✅ Read test docstring to understand intended scenario
2. ✅ Map scenario to correct on_base_code using table:
   - Empty → 0
   - R1 only → 1
   - R2 only → 2
   - **R3 only → 3** (NOT 4!)
   - **R1+R2 → 4** (NOT 3!)
   - R1+R3 → 5
   - R2+R3 → 6
   - Loaded → 7
3. ✅ Update `on_base_code=X` in test
4. ✅ Update assertion expectations (runs_scored, movements count)

### Step 3: Verify Table Entries ✅ COMPLETE

Double-check table entries against source images:
- ✅ G1 table codes 3-6
- ✅ G2 table codes 3-6
- ✅ G3 table codes 3-6

---

## Test Execution Plan

### Test Results (✅ COMPLETE):
```bash
# Run X-Check tests
pytest tests/unit/core/test_x_check_advancement_tables.py -v
# Result: ✅ 59/59 passing

# Run all core/config tests
pytest tests/unit/core/ tests/unit/config/ -v
# Result: ✅ 381/386 passing (5 pre-existing failures, unrelated to this fix)
```

**Pre-existing failures (not part of this fix)**:
1. `test_dice.py::test_get_rolls_since` - timestamp filtering issue
2. `test_runner_advancement.py::test_x_check_f2_returns_valid_result` - expectation mismatch
3. `test_league_configs.py::test_pd_api_url` - minor string difference
4-5. `test_result_charts.py` - Mock comparison issues (2 tests)

---

## Files Modified (✅ COMPLETE)

1. ✅ `backend/app/core/x_check_advancement_tables.py`
   - Lines 40-48: Documentation
   - Lines 96-160: G1 table (swapped 3↔4)
   - Lines 204-268: G2 table (swapped 3↔4)
   - Lines 312-376: G3 table (swapped 3↔4)
   - Lines 521-533: `build_advancement_from_code()` decoder
   - Lines 644-655: `build_flyball_advancement_with_error()` decoder

2. ✅ `backend/tests/unit/core/test_x_check_advancement_tables.py`
   - All 13 test failures fixed
   - Updated on_base_code values and assertions in all failing tests

---

## Completion Summary

### ✅ All Tasks Complete

**Date Completed**: 2025-11-02

**What Was Fixed** (Two Critical Bugs):

### Bug #1: on_base_code Mapping (Sequential vs Bit Field)
1. ✅ G1/G2/G3 table entries (swapped codes 3↔4 throughout)
2. ✅ Decoding logic in `build_advancement_from_code()`
3. ✅ Decoding logic in `build_flyball_advancement_with_error()`
4. ✅ All 13 test on_base_code values corrected

### Bug #2: Wrong Expected Results in Tables (Tables vs Charts)
5. ✅ Fixed 7 incorrect table entries in G1_ADVANCEMENT_TABLE and G2_ADVANCEMENT_TABLE
   - G1 Code 1, Infield In: Changed 3→2
   - G1 Code 3, Normal: Changed 13→3
   - G1 Code 3, Infield In: Changed 3→1
   - G1 Code 4, Normal: Changed 3→13
   - G1 Code 4, Infield In: Changed 4→2
   - G2 Code 3, Infield In: Changed 3→1
   - G2 Code 4, Normal: Changed 5→4
6. ✅ Fixed 7 test expectations to match official charts
7. ✅ Full codebase scan - no bit field operations found
8. ✅ Verified all on_base_code usage uses correct sequential mapping

**Test Results**:
- ✅ **59/59 X-Check advancement tests passing** (100% success!)
- ✅ All table entries validated against official rulebook charts (Images #1-3)
- ✅ All on_base_code values validated against correct mapping (0-7 sequential)

**Verification**:
- ✅ No bit field operations (`on_base_code & N`) found in codebase
- ✅ All code uses correct sequential mapping (dictionary lookup)
- ✅ `runner_advancement.py` correctly identifies code 3 as R3, code 4 as R1+R2
- ✅ `x_check_advancement_tables.py` uses mapping dictionary throughout
- ✅ All table data matches official G1/G2/G3 charts from rulebook

---

## Reference: Correct Mapping Table

| Code | Situation | R1 | R2 | R3 | Binary (for reference only) |
|------|-----------|----|----|----|-----------------------------|
| 0 | Empty | ❌ | ❌ | ❌ | (not bit field!) |
| 1 | R1 | ✅ | ❌ | ❌ | (not bit field!) |
| 2 | R2 | ❌ | ✅ | ❌ | (not bit field!) |
| 3 | R3 | ❌ | ❌ | ✅ | (not bit field!) |
| 4 | R1+R2 | ✅ | ✅ | ❌ | (not bit field!) |
| 5 | R1+R3 | ✅ | ❌ | ✅ | (not bit field!) |
| 6 | R2+R3 | ❌ | ✅ | ✅ | (not bit field!) |
| 7 | Loaded | ✅ | ✅ | ✅ | (not bit field!) |

**REMEMBER**: This is a simple lookup table, NOT bit field math!

---

## Lessons Learned

1. **Always validate assumptions** - The bit field assumption seemed logical but was completely wrong
2. **Test with real data early** - Would have caught this immediately with actual game scenarios
3. **Document data structures clearly** - Mapping should have been documented in multiple places
4. **User validation is critical** - User spotted the issue immediately when they saw it

---

## Communication Notes for User

When presenting the fix:
- ✅ Be transparent about the error
- ✅ Show exactly what was wrong and what was fixed
- ✅ Provide test results showing correctness
- ✅ Give user easy way to spot-check (updated test expectations)
- ✅ Demonstrate one or two manual examples working correctly

---

**File Status**: ✅ COMPLETE - Bug fix finished
**Last Updated**: 2025-11-02
**Completion Date**: 2025-11-02

---

## Ready for Commit

All fixes complete and verified. Ready to create git commit with the following changes:

**Files Modified**:
1. `backend/app/core/x_check_advancement_tables.py`
   - Fixed on_base_code mapping in decoder functions (3↔4 swap)
   - Fixed 7 incorrect table entries against official charts
   - Updated documentation comments

2. `backend/tests/unit/core/test_x_check_advancement_tables.py`
   - Fixed 13 test on_base_code values (3↔4 corrections)
   - Fixed 7 test expected results to match charts
   - Updated test docstrings with correct expectations

3. `backend/app/core/play_resolver.py`
   - Updated `_resolve_x_check()` to use `dice_system.roll_fielding()`
   - Improved dice audit trail (all rolls tracked with roll_id, position)
   - Automatic error_total calculation (no manual 3d6 addition)

4. `.claude/implementation/PHASE_3D_CRITICAL_FIX.md`
   - Complete documentation of both bugs and all fixes

**Commit Summary**:
- Fixed critical on_base_code mapping bug (sequential vs bit field)
- Fixed 7 table entries that didn't match official rulebook charts
- All 59 X-Check advancement tests now passing (100%)
- Both bugs discovered and fixed in same session