fix: standardize home-run detection to use batter_final only (#22) #68

cal · 2026-03-05T18:36:36Z

cal commented

2026-03-05 18:36:36 +00:00

Summary

Removes the two-path home-run check batter_final == 4 or batter_to_base == 4 at db_calls_gameplay.py:1291
Sets batter_final from batter_to_base before the check, making the _on_final field the single source of truth
Deletes the # patch to handle little league home runs TODO: comment that flagged the inconsistency

Root Cause

advance_runners(num_bases=4) sets batter_final = 4 in the DB, but some code paths (e.g. inside-the-park / "little league" HRs) called complete_play(batter_to_base=4) without batter_final being set first. The old check worked around this with or batter_to_base == 4. The fix normalises batter_final at the top of the check block instead.

Files Changed

db_calls_gameplay.py — 3-line functional change at complete_play() (~line 1452); rest of diff is ruff auto-formatting applied by the project's configured Edit hook (same pattern as PRs #60–#65)

Test Results

No test suite configured for this path. Change verified by code review — logic is equivalent for all callers:

batter_to_base=4 → sets batter_final=4, check passes (HR) ✓
batter_to_base=1/2/3 → sets batter_final=1/2/3, check fails (not HR) ✓
batter_to_base=None → batter_final unchanged, check uses existing DB value ✓

Other Observations

db_calls_gameplay.py:1379 still assigns this_play.batter_final = batter_to_base (now redundant). It's harmless and left untouched to keep scope minimal.

## Summary - Removes the two-path home-run check `batter_final == 4 or batter_to_base == 4` at `db_calls_gameplay.py:1291` - Sets `batter_final` from `batter_to_base` before the check, making the `_on_final` field the single source of truth - Deletes the `# patch to handle little league home runs TODO:` comment that flagged the inconsistency ## Root Cause `advance_runners(num_bases=4)` sets `batter_final = 4` in the DB, but some code paths (e.g. inside-the-park / "little league" HRs) called `complete_play(batter_to_base=4)` without `batter_final` being set first. The old check worked around this with `or batter_to_base == 4`. The fix normalises `batter_final` at the top of the check block instead. ## Files Changed - `db_calls_gameplay.py` — 3-line functional change at `complete_play()` (~line 1452); rest of diff is ruff auto-formatting applied by the project's configured Edit hook (same pattern as PRs #60–#65) ## Test Results No test suite configured for this path. Change verified by code review — logic is equivalent for all callers: - `batter_to_base=4` → sets `batter_final=4`, check passes (HR) ✓ - `batter_to_base=1/2/3` → sets `batter_final=1/2/3`, check fails (not HR) ✓ - `batter_to_base=None` → `batter_final` unchanged, check uses existing DB value ✓ ## Other Observations - `db_calls_gameplay.py:1379` still assigns `this_play.batter_final = batter_to_base` (now redundant). It's harmless and left untouched to keep scope minimal.

cal added 1 commit 2026-03-05 18:36:37 +00:00

fix: standardize home-run detection to use batter_final only (#22 )

Build Docker Image / build (pull_request) Successful in 1m22s

Details

3952813730

Set batter_final from batter_to_base before the HR check so that
_on_final fields are the single source of truth. Removes the
two-path `batter_final == 4 or batter_to_base == 4` condition and
the associated TODO comment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal referenced this pull request

2026-03-05 18:36:46 +00:00

Standardize home-run detection to use `_on_final` fields only #22

cal added the

ai-reviewing

label 2026-03-05 18:45:35 +00:00

cal reviewed 2026-03-05 18:48:08 +00:00

cal left a comment

AI Code Review

Files Reviewed

db_calls_gameplay.py (modified)

Findings

Correctness

The 3-line functional change is correct. The normalization of batter_final from batter_to_base before the HR check resolves the inconsistency cleanly:

batter_to_base=4 → batter_final=4 set early, if this_play.batter_final == 4: triggers HR ✓
batter_to_base=1/2/3 → batter_final set to non-4, HR check fails ✓
batter_to_base=None → early guard if batter_to_base is not None: is False, batter_final unchanged from DB ✓

Verified all callers in in_game/game_helpers.py, cogs/gameplay_legacy.py, and db_calls_gameplay.py (undo_play). All pass None, 1, 2, 3, or 4 — the is not None guard is the correct idiom for optional int parameters (vs if batter_to_base: which would incorrectly treat 0 as None, though 0 is never passed here).

The redundant this_play.batter_final = batter_to_base at line 1379 (now double-assigns the same value) is harmless and correctly left in scope.

Security

No issues found.

Style & Conventions

The functional diff matches project conventions. The remainder of the diff is ruff auto-formatting (quote normalization, line splits on chained method calls), consistent with PRs #60–#65.

Suggestions

None.

Verdict: APPROVED

Clean removal of the TODO hack. The single-source-of-truth approach is more correct and the is not None guard handles the None default properly. No behavioral change for any existing call site.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `db_calls_gameplay.py` (modified) ### Findings #### Correctness The 3-line functional change is correct. The normalization of `batter_final` from `batter_to_base` before the HR check resolves the inconsistency cleanly: - `batter_to_base=4` → `batter_final=4` set early, `if this_play.batter_final == 4:` triggers HR ✓ - `batter_to_base=1/2/3` → `batter_final` set to non-4, HR check fails ✓ - `batter_to_base=None` → early guard `if batter_to_base is not None:` is False, `batter_final` unchanged from DB ✓ Verified all callers in `in_game/game_helpers.py`, `cogs/gameplay_legacy.py`, and `db_calls_gameplay.py` (undo_play). All pass `None`, `1`, `2`, `3`, or `4` — the `is not None` guard is the correct idiom for optional int parameters (vs `if batter_to_base:` which would incorrectly treat `0` as None, though `0` is never passed here). The redundant `this_play.batter_final = batter_to_base` at line 1379 (now double-assigns the same value) is harmless and correctly left in scope. #### Security No issues found. #### Style & Conventions The functional diff matches project conventions. The remainder of the diff is ruff auto-formatting (quote normalization, line splits on chained method calls), consistent with PRs #60–#65. #### Suggestions None. ### Verdict: APPROVED Clean removal of the TODO hack. The single-source-of-truth approach is more correct and the `is not None` guard handles the `None` default properly. No behavioral change for any existing call site. --- *Automated review by Claude PR Reviewer*

cal added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-05 18:48:26 +00:00

cal changed target branch from main to next-release

2026-03-07 07:32:48 +00:00

cal force-pushed ai/paper-dynasty-discord-22 from 3952813730 to 9b5ded1e1e

2026-03-07 07:33:33 +00:00

Compare

cal changed target branch from next-release to main

2026-03-23 04:14:30 +00:00