fix: address PR #104 review feedback
- Correct idempotency guard docstring in update_season_stats() to accurately describe the last_game FK check limitation: only detects replay of the most-recently-processed game; out-of-order re-delivery (game G after G+1) bypasses the guard. References issue #105 for the planned ProcessedGame ledger fix. - Fix migration card_type comment: 'batting' or 'pitching' → 'batter', 'sp', or 'rp' to match actual seeded values. - Remove local rarity fixture in test_season_stats_update.py that shadowed the conftest.py fixture; remove unused rarity parameter from player_batter and player_pitcher fixtures. - Update test_double_count_prevention docstring to note the known out-of-order re-delivery limitation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
f7bc248a9f
commit
b8c55b5723
@ -2,9 +2,15 @@
|
||||
season_stats.py — Incremental PlayerSeasonStats update logic.
|
||||
|
||||
Called once per completed StratGame to accumulate batting and pitching
|
||||
statistics into the player_season_stats table. The update is idempotent:
|
||||
if this game_id has already been processed (detected via last_game FK),
|
||||
the function returns early without double-counting.
|
||||
statistics into the player_season_stats table.
|
||||
|
||||
Idempotency limitation: re-delivery of a game is detected by checking
|
||||
whether any PlayerSeasonStats row still carries that game_id as last_game.
|
||||
This guard only works if no later game has been processed for the same
|
||||
players — if game G+1 is processed first, a re-delivery of game G will
|
||||
bypass the guard and double-count stats. A persistent processed-game
|
||||
ledger is needed for full idempotency across out-of-order re-delivery
|
||||
(see issue #105).
|
||||
|
||||
Peewee upsert strategy:
|
||||
- SQLite: on_conflict_replace() — simplest path, deletes + re-inserts
|
||||
@ -364,8 +370,15 @@ def update_season_stats(game_id: int) -> dict:
|
||||
Accumulate per-game batting and pitching stats into PlayerSeasonStats.
|
||||
|
||||
This function is safe to call exactly once per game. If called again
|
||||
for the same game_id (detected by checking last_game FK), it returns
|
||||
immediately without modifying any data.
|
||||
for the same game_id while it is still the most-recently-processed
|
||||
game for at least one affected player (detected by checking last_game
|
||||
FK), it returns early without modifying any data.
|
||||
|
||||
Limitation: the guard only detects re-delivery if no later game has
|
||||
been processed for the same players. Out-of-order re-delivery (e.g.
|
||||
game G re-delivered after game G+1 was already processed) will not be
|
||||
caught and will silently double-count stats. See issue #105 for the
|
||||
planned ProcessedGame ledger fix.
|
||||
|
||||
Algorithm:
|
||||
1. Fetch StratGame to get the season.
|
||||
@ -396,8 +409,10 @@ def update_season_stats(game_id: int) -> dict:
|
||||
season = game.season
|
||||
|
||||
with db.atomic():
|
||||
# Step 2 — Double-count prevention: check if any row already
|
||||
# carries this game_id as last_game
|
||||
# Step 2 — Double-count prevention: check if any row still
|
||||
# carries this game_id as last_game. Note: only detects replay
|
||||
# of the most-recently-processed game; out-of-order re-delivery
|
||||
# bypasses this guard (see issue #105).
|
||||
already_processed = (
|
||||
PlayerSeasonStats.select()
|
||||
.where(PlayerSeasonStats.last_game == game_id)
|
||||
|
||||
@ -90,7 +90,7 @@ CREATE INDEX IF NOT EXISTS player_season_stats_player_season_idx
|
||||
CREATE TABLE IF NOT EXISTS evolution_track (
|
||||
id SERIAL PRIMARY KEY,
|
||||
name VARCHAR(255) UNIQUE NOT NULL,
|
||||
card_type VARCHAR(50) NOT NULL, -- 'batting' or 'pitching'
|
||||
card_type VARCHAR(50) NOT NULL, -- 'batter', 'sp', or 'rp'
|
||||
formula VARCHAR(255) NOT NULL, -- e.g. 'hr', 'k_per_9', 'ops'
|
||||
t1_threshold INTEGER NOT NULL,
|
||||
t2_threshold INTEGER NOT NULL,
|
||||
|
||||
@ -156,11 +156,6 @@ def make_play(game, play_num, batter, batter_team, pitcher, pitcher_team, **stat
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def rarity():
|
||||
return Rarity.create(value=1, name="Common", color="#ffffff")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def team_a():
|
||||
return _make_team("TMA", gmid=1001)
|
||||
@ -172,13 +167,13 @@ def team_b():
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def player_batter(rarity):
|
||||
def player_batter():
|
||||
"""A batter-type player for team A."""
|
||||
return _make_player("Batter One", pos="CF")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def player_pitcher(rarity):
|
||||
def player_pitcher():
|
||||
"""A pitcher-type player for team B."""
|
||||
return _make_player("Pitcher One", pos="SP")
|
||||
|
||||
@ -399,15 +394,18 @@ def test_decision_integration(team_a, team_b, player_batter, player_pitcher, gam
|
||||
def test_double_count_prevention(team_a, team_b, player_batter, player_pitcher, game):
|
||||
"""Calling update_season_stats() twice for the same game must not double the stats.
|
||||
|
||||
What: Process a game once (pa=3), then call the function again. The
|
||||
second call should detect the already-processed state via the
|
||||
PlayerSeasonStats.last_game FK check and return early with 'skipped'=True.
|
||||
What: Process a game once (pa=3), then immediately call the function
|
||||
again with the same game_id. The second call detects via the
|
||||
PlayerSeasonStats.last_game FK check that this game is still the
|
||||
most-recently-processed game and returns early with 'skipped'=True.
|
||||
The resulting pa should still be 3, not 6.
|
||||
|
||||
Why: The bot infrastructure may deliver game-complete events more than
|
||||
once (network retries, message replays). Without idempotency, stats
|
||||
would accumulate incorrectly and could not be corrected without a full
|
||||
reset.
|
||||
once (network retries, message replays). The guard prevents
|
||||
double-counting when the replayed game is still the last game
|
||||
processed for those players. Note: this test only covers same-game
|
||||
immediate replay — out-of-order re-delivery (game G after G+1) is a
|
||||
known limitation tracked in issue #105.
|
||||
"""
|
||||
for i in range(3):
|
||||
make_play(
|
||||
|
||||
Loading…
Reference in New Issue
Block a user