Skip to content

Fix MLB-wide pitching coverage validation - #62

Merged
Mattsface merged 2 commits into
mainfrom
feature/issue-61-pitching-coverage
Sep 29, 2026
Merged

Mattsface merged 2 commits into
mainfrom
feature/issue-61-pitching-coverage

Conversation

@Mattsface

Copy link
Copy Markdown
Member

Summary

Fixes #61.

Prevent partial pitching data from being presented as an MLB-wide pitching baseline.

The pitching comparison now requires:

  • the existing league ingestion state to be COMPLETE; and
  • the nonempty persisted (team_id, game_pk) identity set in team_game_pitching_lines to exactly match the corresponding stored batting identity set for the season.

This closes the mixed-state hole where a historical complete batting import followed by a one-team pitching backfill could otherwise be treated as league-wide pitching coverage.

What changed

  • Add a focused repository-level pitching coverage check.
  • Gate MLB-wide pitching comparison on matching batting/pitching identities.
  • Refuse missing or extra pitching identities.
  • Preserve existing team-only pitching analytics and unavailable-state behavior.
  • Add regression coverage for the historical-import + one-team-backfill scenario.
  • Add coverage for incomplete/running/absent league state and identity mismatch cases.
  • Document the pitching-specific coverage requirement.

Scope

This intentionally does not redesign league completeness or add snapshot/version tracking. It only verifies that the currently stored pitching dataset covers the same stored team-game identities as the complete batting dataset before an MLB-wide pitching baseline is calculated.

Verification

Expected PR checks:

poetry run pytest
poetry run ruff check .
poetry run ruff format --check .

@Mattsface

Copy link
Copy Markdown
Member Author

@claude please review

@claude

claude Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Claude finished @Mattsface's task in 20s —— View job


Review of PR #62: MLB-wide pitching coverage validation

The change does what the PR says. I found no blocking issues.

Scope of this review: I read the diff for README.md, app/, and docs/. I did not read tests/test_web_pitching.py or run pytest, ruff, or the format check, so please rely on CI for those.

What looks good

  • Layering follows AGENTS.md. The identity query is in the repository, the route only orchestrates, and analytics is unchanged apart from a docstring.
  • The fix closes the stated hole. The old guard was if not league_games. That guard passed when a one-team pitching backfill followed a complete batting import. Requiring the nonempty batting and pitching (team_id, game_pk) sets to be equal handles that case.
  • The extra-row refusal is correct. list_league_season_pitching loads every pitching row for the season, so extra rows would end up in the baseline.
  • No inferred completeness. The check doesn't use team counts or schedule length, and empty datasets are refused.
  • The unavailable state is unchanged. It still returns None, so the template and team-only analytics are untouched.
  • Docs explain why. The new README and design-doc text says why the guard exists and what it doesn't establish (snapshot consistency).

Suggestions (non-blocking)

  1. Naming overstates the guarantee. has_complete_league_pitching_coverage (app/database/repositories.py:274) suggests completeness. It only checks that the pitching identities equal the batting identities. Its own docstring says it doesn't establish completeness, and AGENTS.md asks that labels not imply completeness. Something like pitching_identities_match_batting would be more accurate. If you rename it, update the route import and the references in the list_league_season_pitching docstring.

  2. Comparison can be done in the database. The helper loads both identity sets into Python. At about 2,430 games × 2 teams per season that is small, so this is fine today. A pair of EXCEPT queries or an anti-join would do the same thing without materializing the sets. It's optional, and the set comparison is arguably the more readable choice.

  3. Three reads per request. The route now runs the coverage check (two queries) and then list_league_season_pitching (a third). A concurrent ingestion between them could in theory make the checked data differ from the loaded data. The docs already say snapshot consistency is out of scope, so this is only worth a follow-up issue if ingestion ever runs alongside web traffic.

  4. Docs check. Please confirm no other section of docs/team-pitching-visualization.md (for example, earlier text describing the "no pitching rows" check as the guard) still describes the old rule. I only saw the added hunk, not the surrounding sections.

Verdict

Looks good to merge once CI is green. Suggestion 1 is the one I'd consider before merging.
· branch feature/issue-61-pitching-coverage

@Mattsface
Mattsface merged commit fa0fcff into main Sep 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Require complete pitching coverage before showing MLB-wide pitching baselines

2 participants