Fix 2016-2022 weekly SB data: BRef re-fetch, coverage validator, fetch retries#8
Fix 2016-2022 weekly SB data: BRef re-fetch, coverage validator, fetch retries#8lambertchu wants to merge 2 commits into
Conversation
validate_weekly_counting_coverage() sums weekly R/RBI/SB/CS per player against local FanGraphs season totals and warns below 80% coverage; it runs on every snapshot build and inside the Statcast fallback. The old Statcast-derived 2016 file scores 9% SB / 38% CS on it; the defect can no longer reach training data silently. fetch_batter_weekly_stats gains per-week retries with linear backoff (--retries, default 3): BRef soft-throttles sustained scraping with table-less responses, and one transient week previously aborted a whole season pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2016-2022 weekly batting is now BRef-sourced (SB coverage ~100%, was ~6-9% from the Statcast fallback). Rewrote the §3.2 defect note to the fixed state, documented the coverage validator and fetch retries, and flagged that the recorded Phase 2/3 benchmark numbers predate the fix (clean-data re-run pending). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a validation layer to ensure weekly counting stats match season totals, preventing lossy data sources from silently corrupting training data. It also introduces retry logic to the Baseball Reference weekly scraper to handle soft-throttling, and updates the documentation and tests accordingly. The reviewer feedback suggests making the internal helper _load_season_batting_totals public, defensively handling potential NaN values when summing weekly stats, and ensuring the retry loop executes at least once if a negative retry count is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import pandas as pd | ||
|
|
||
| from src.data.fetch_game_logs import ( | ||
| _load_season_batting_totals, |
There was a problem hiding this comment.
The function _load_season_batting_totals is defined with a leading underscore in src/data/fetch_game_logs.py, indicating it is a private/internal helper. Importing and using a private function across modules violates encapsulation.\n\nConsider renaming _load_season_batting_totals to load_season_batting_totals (removing the leading underscore) in both src/data/fetch_game_logs.py and here to make it explicitly public.
| _load_season_batting_totals, | |
| load_season_batting_totals, |
| # poisoned Phase 2/3 SB targets without a single warning. | ||
| validate_weekly_counting_coverage( | ||
| batting_wk, | ||
| _load_season_batting_totals(year, raw_dir), |
There was a problem hiding this comment.
Update the usage of _load_season_batting_totals to the public load_season_batting_totals name.
| _load_season_batting_totals(year, raw_dir), | |
| load_season_batting_totals(year, raw_dir), |
| ) | ||
| if denom <= 0: | ||
| continue | ||
| ratio = float(weekly_sums.loc[shared, stat].sum()) / denom |
There was a problem hiding this comment.
To ensure robust and defensive programming, we should coerce the weekly sums to numeric and handle potential NaN values, similar to how totals is handled. If weekly_sums contains non-numeric or NaN values, calling .sum() directly might raise an error or return NaN.
weekly_sum = float(\n pd.to_numeric(weekly_sums.loc[shared, stat], errors="coerce").fillna(0).sum()\n )\n ratio = weekly_sum / denom| logger.exception(" Failed week %d-W%02d", iso_year, iso_week) | ||
| raw = None | ||
| week_failed = True | ||
| for attempt in range(1, retries + 2): |
There was a problem hiding this comment.
If retries is configured to be a negative integer, range(1, retries + 2) could be empty, causing the loop to be skipped entirely and resulting in an unexpected failure.\n\nUsing max(0, retries) ensures that the loop runs at least once (the initial attempt) even if a negative value is provided.
| for attempt in range(1, retries + 2): | |
| for attempt in range(1, max(0, retries) + 2): |
1 participant
Summary
Fixes the SB/CS defect documented in CLAUDE.md §3.2: the 2016-2022 weekly batting logs were Statcast-derived and carried only ~6-9% of stolen bases, poisoning SB features and
ros_sb*training targets in 7 of 9 Phase 2 training seasons.validate_weekly_counting_coverage— runs on every snapshot build and in the Statcast fallback; warns when weekly R/RBI/SB/CS sum to <80% of season totals. A lossy weekly source can no longer reach training silently (the old 2016 file scores 9% SB on it).--retries, default 3) — BRef soft-throttles sustained scraping, and one transient week previously aborted a whole 12-minute season pass. Fail-hard semantics preserved when retries are exhausted.Draft because the clean-data re-benchmark hasn't run yet (
benchmark_ros.py --years 2023 2024 2025 --retrain --pit-plot, ~30 min). It answers whether clean SB data moves Phase 2 from +7% behind shrinkage to parity; §7.3/README tables get refreshed with its results before this leaves draft.Test plan
uv run pytest tests/ -q— 500 passed (7 new: validator behavior incl. the 6%-coverage defect shape, retry recovery, fail-hard preserved)uv run ruff checkon touched files — clean🤖 Generated with Claude Code