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)

# 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)

# 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

# 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):

# 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