diff --git a/README.md b/README.md index 623bfce..43206b5 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,11 @@ League averages are game-weighted over persisted team-game records, not an unweighted mean of team averages. League statistics are shown only when the stored league-season ingestion state indicates complete coverage. +Pitching context additionally requires the nonempty set of stored pitching +`(team_id, game_pk)` identities to match the stored batting identities for that +season. A legacy `COMPLETE` batting import followed by one team's pitching +backfill cannot establish an MLB-wide pitching baseline. + A `COMPLETE` refresh does **not** mean the baseball season has ended. It means every discovered club was successfully refreshed for that import run. diff --git a/app/analytics/league_pitching.py b/app/analytics/league_pitching.py index cd660d5..aafa8ff 100644 --- a/app/analytics/league_pitching.py +++ b/app/analytics/league_pitching.py @@ -52,10 +52,9 @@ def supports_league_wide_pitching_average( drift and let one page call a season MLB-wide while another does not. Complete coverage is necessary but **not** sufficient here, and the caller - must check the second condition itself: a league season imported before - pitching was collected has complete batting coverage and no pitching rows - at all. ``build_league_pitching_context`` refuses an empty set of records, - which is what that state produces. + must also verify that pitching identities match the nonempty stored league + batting dataset. A legacy COMPLETE import followed by one team's pitching + backfill otherwise produces a nonempty but incomplete pitching dataset. """ return supports_league_wide_average(coverage) diff --git a/app/database/repositories.py b/app/database/repositories.py index 50546ee..0914d57 100644 --- a/app/database/repositories.py +++ b/app/database/repositories.py @@ -271,6 +271,33 @@ def list_team_season_pitching( return [record.to_domain() for record in records] +def has_matching_league_pitching_identities(session: Session, *, season: int) -> bool: + """Check pitching identities against the season's stored batting dataset. + + This does not establish league ingestion completeness; callers must also + require COMPLETE ingestion state. Both tables must contain the same nonempty + set of (team_id, game_pk) identities. Extra pitching rows are refused too, + because the league pitching loader would include them in the baseline. + """ + batting_identities = { + (team_id, game_pk) + for team_id, game_pk in session.execute( + select( + TeamGameBattingLineRecord.team_id, TeamGameBattingLineRecord.game_pk + ).where(TeamGameBattingLineRecord.season == season) + ) + } + pitching_identities = { + (team_id, game_pk) + for team_id, game_pk in session.execute( + select( + TeamGamePitchingLineRecord.team_id, TeamGamePitchingLineRecord.game_pk + ).where(TeamGamePitchingLineRecord.season == season) + ) + } + return bool(batting_identities) and batting_identities == pitching_identities + + def list_league_season_pitching( session: Session, *, @@ -280,8 +307,9 @@ def list_league_season_pitching( The pitching counterpart of ``list_league_season``, and it answers the same limited question: what is stored, not whether that is actually MLB-wide. - The recorded league-season coverage state is what says whether the stored - rows may be described as covering the league. + Callers need both COMPLETE league ingestion state and matching batting and + pitching identities (``has_matching_league_pitching_identities``) before + describing these rows as covering the league. """ stmt = ( select(TeamGamePitchingLineRecord) diff --git a/app/web/routes.py b/app/web/routes.py index cc84ebd..ae6691a 100644 --- a/app/web/routes.py +++ b/app/web/routes.py @@ -69,6 +69,7 @@ MIGRATION_HINT, DatabaseSchemaMissingError, get_league_season_ingestion, + has_matching_league_pitching_identities, list_available_team_seasons, list_league_season, list_league_season_pitching, @@ -970,25 +971,19 @@ def _load_league_pitching_comparison( ) -> TeamPitchingLeagueComparison | None: """Read MLB pitching context, or None when it is not earned. - Two conditions must hold. The season's latest league-wide refresh must have - reached ``COMPLETE`` coverage, which is the shared Milestone 5 rule. And - the season must actually have stored pitching lines: a league season - imported before pitching was collected has complete batting coverage and no - pitching rows at all, so coverage alone would wrongly promise an MLB ERA. - - Unlike the baserunner backfill there is no partially-known state to report. - Every pitching column is NOT NULL, so the rows either exist or they do not, - and an absent set yields the plain missing-comparison note rather than - backfill guidance. + COMPLETE ingestion state may predate pitching persistence. Require matching + stored batting and pitching team-game identities as well, so a single-team + backfill cannot become an MLB baseline. Missing or mismatched coverage uses + the existing unavailable comparison state. """ coverage = get_league_season_ingestion(session, season=analysis.season) if not supports_league_wide_pitching_average(coverage): return None - league_games = list_league_season_pitching(session, season=analysis.season) - if not league_games: + if not has_matching_league_pitching_identities(session, season=analysis.season): return None + league_games = list_league_season_pitching(session, season=analysis.season) league = build_league_pitching_context(league_games) return compare_team_pitching_to_league(analysis, league) diff --git a/docs/team-pitching-visualization.md b/docs/team-pitching-visualization.md index 1af1810..493f848 100644 --- a/docs/team-pitching-visualization.md +++ b/docs/team-pitching-visualization.md @@ -166,6 +166,28 @@ simply has no line on this chart yet. Its sign convention is worth noting when it does surface: a **negative** ERA difference is the better direction, the opposite of every other comparison in the application. +### Pitching coverage guard (issue #61) + +The route requires both `COMPLETE` league ingestion state and matching nonempty +sets of persisted `(team_id, game_pk)` identities in the batting and pitching +tables for the selected season. The batting identities are the same stored +dataset used by the league batting loader. A focused repository helper checks +the identities; rate calculations remain in analytics. + +This extra guard is necessary because a `COMPLETE` league import may predate +pitching persistence. Backfilling one team produces valid, non-null pitching +rows without supplying every other team's rows. Missing identities refuse the +baseline; extra pitching identities also refuse it because the pitching loader +would otherwise include records outside the expected dataset. Empty datasets +cannot establish coverage. Neither team counts nor scheduled season lengths +are used to infer completeness. + +Unavailable coverage returns the existing `None` comparison and neutral note +in the route context. Team pitching analysis and the existing template stay +unchanged. This checks the currently stored dataset only: it does not establish +refresh snapshot consistency, change ingestion state semantics, or import data +during browser requests. + ## 7. Missing-data state A team-season with batting rows but no pitching rows returns **409** and names diff --git a/tests/test_web_pitching.py b/tests/test_web_pitching.py index 30a120d..8995fcf 100644 --- a/tests/test_web_pitching.py +++ b/tests/test_web_pitching.py @@ -7,16 +7,21 @@ """ from collections.abc import Callable, Generator, Iterator +from datetime import datetime from pathlib import Path import pytest +import requests from fastapi.testclient import TestClient from sqlalchemy.orm import Session from app.analytics.team_pitching import build_team_pitching_analysis from app.database.engine import build_engine, build_session_factory from app.database.repositories import ( + has_matching_league_pitching_identities, list_team_season_pitching, + record_league_season_ingestion_finish, + record_league_season_ingestion_start, upsert_team_season, upsert_team_season_pitching, ) @@ -32,10 +37,17 @@ rolling_average_trace_name, ) from app.web.dependencies import get_db_session -from app.web.formatting import build_pitching_summary_cards, format_innings +from app.web.formatting import ( + LEAGUE_PITCHING_UNAVAILABLE_NOTE, + build_pitching_summary_cards, + format_innings, +) +from app.web.routes import _load_league_pitching_comparison from tests.factories import ( MARINERS_ID, MARINERS_NAME, + TWINS_ID, + TWINS_NAME, make_pitching_season, make_season, ) @@ -87,6 +99,168 @@ def seed( session.close() +def test_complete_legacy_league_with_one_team_backfilled_refuses_mlb_baseline( + client: TestClient, session_factory: Callable[[], Session] +) -> None: + # The original league import predates pitching persistence. + with session_factory() as session: + upsert_team_season(session, lines=make_season([8, 8])) + upsert_team_season( + session, + lines=make_season([6, 6], team_id=TWINS_ID, team_name=TWINS_NAME), + ) + record_league_season_ingestion_finish( + session, + season=2025, + expected_team_count=2, + successful_team_count=2, + failed_team_count=0, + started_at=datetime(2025, 10, 1, 12), + completed_at=datetime(2025, 10, 1, 13), + ) + session.commit() + + # A later single-team re-import does not change the COMPLETE league state. + with session_factory() as session: + upsert_team_season_pitching(session, lines=make_pitching_season([2, 4])) + session.commit() + + response = client.get(PATH, params={"team_id": MARINERS_ID, "season": 2025}) + + assert response.status_code == 200 + assert response.context["league_comparison"] is None + + +@pytest.mark.parametrize( + ("state", "pitching_case", "available"), + [ + ("complete", "matching", True), + ("complete", "missing_game", False), + ("complete", "wrong_game", False), + ("complete", "wrong_team", False), + ("complete", "wrong_season", False), + ("complete", "extra_game", False), + ("incomplete", "matching", False), + ("running", "matching", False), + ("absent", "matching", False), + ], +) +def test_league_pitching_requires_matching_season_identities( + client: TestClient, + session_factory: Callable[[], Session], + monkeypatch: pytest.MonkeyPatch, + state: str, + pitching_case: str, + available: bool, +) -> None: + def fail_network(*args, **kwargs): + pytest.fail("Browser rendering must not call MLB") + + monkeypatch.setattr(requests.Session, "request", fail_network) + monkeypatch.setattr("mlbstatsapi.Mlb.__init__", fail_network) + monkeypatch.setattr("mlbstatsapi.AsyncMlb.__init__", fail_network) + + # Two clubs and unequal game counts deliberately avoid MLB count assumptions. + seed(session_factory, earned_runs=[2, 4]) + twins = make_pitching_season([6], team_id=TWINS_ID, team_name=TWINS_NAME) + if pitching_case == "wrong_game": + twins = [twins[0].model_copy(update={"game_pk": 999999})] + elif pitching_case == "wrong_team": + twins = [twins[0].model_copy(update={"team_id": 108})] + elif pitching_case == "wrong_season": + twins = [twins[0].model_copy(update={"season": 2024})] + elif pitching_case == "extra_game": + twins.append(twins[0].model_copy(update={"game_pk": 999999})) + + with session_factory() as session: + upsert_team_season( + session, + lines=make_season( + [6, 6] if pitching_case == "missing_game" else [6], + team_id=TWINS_ID, + team_name=TWINS_NAME, + ), + ) + upsert_team_season_pitching(session, lines=twins) + # Unmatched data from another season must not block matching 2025 data. + upsert_team_season(session, lines=make_season([8], season=2023)) + upsert_team_season_pitching( + session, lines=make_pitching_season([1], season=2022) + ) + if state == "running": + record_league_season_ingestion_start( + session, + season=2025, + expected_team_count=2, + started_at=datetime(2025, 10, 1, 12), + ) + elif state != "absent": + failed = int(state == "incomplete") + record_league_season_ingestion_finish( + session, + season=2025, + expected_team_count=2, + successful_team_count=2 - failed, + failed_team_count=failed, + started_at=datetime(2025, 10, 1, 12), + completed_at=datetime(2025, 10, 1, 13), + ) + session.commit() + + if not available: + + def fail_league_calculation(*args, **kwargs): + pytest.fail("Unavailable coverage must not reach league analytics") + + monkeypatch.setattr( + "app.web.routes.build_league_pitching_context", fail_league_calculation + ) + + response = client.get(PATH, params={"team_id": MARINERS_ID, "season": 2025}) + + assert response.status_code == 200 + # Team calculations and chart survive every league availability state. + assert response.context["analysis"] == build_team_pitching_analysis( + make_pitching_season([2, 4]) + ) + assert PITCHING_CHART_DIV_ID in response.text + assert "Season ERA" in response.text + comparison = response.context["league_comparison"] + if available: + assert comparison is not None + assert comparison.league.team_game_records == 3 + assert comparison.league.teams_represented == 2 + assert comparison.league.era == pytest.approx(4.0) + else: + assert comparison is None + assert response.context["league_comparison_note"] == ( + LEAGUE_PITCHING_UNAVAILABLE_NOTE + ) + assert response.context["comparison_sentence"] == "" + + +@pytest.mark.parametrize("with_batting", [True, False]) +def test_complete_state_without_pitching_has_no_baseline( + migrated_session: Session, with_batting: bool +) -> None: + if with_batting: + upsert_team_season(migrated_session, lines=make_season([8])) + record_league_season_ingestion_finish( + migrated_session, + season=2025, + expected_team_count=1, + successful_team_count=1, + failed_team_count=0, + started_at=datetime(2025, 10, 1, 12), + completed_at=datetime(2025, 10, 1, 13), + ) + migrated_session.commit() + analysis = build_team_pitching_analysis(make_pitching_season([2])) + + assert not has_matching_league_pitching_identities(migrated_session, season=2025) + assert _load_league_pitching_comparison(migrated_session, analysis) is None + + class TestPageStates: def test_the_page_renders_when_pitching_is_stored( self, client: TestClient, session_factory