diff --git a/README.md b/README.md index 7aa0224..eb77df8 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,13 @@ discovered club, and a persisted record of whether a run actually covered them all. It adds no visualization. See [docs/league-season-ingestion.md](docs/league-season-ingestion.md). +Milestone 5 adds MLB-wide context to the team hits page: an MLB hits-per-game +reference line, a `vs MLB` summary card, and a difference between the two. The +MLB average is a game-weighted mean over stored team-game records, and it is +shown only when Milestone 4 recorded `COMPLETE` league coverage for the +selected season. See +[docs/team-vs-mlb-comparison.md](docs/team-vs-mlb-comparison.md). + ## Planned MVP A local web application that: @@ -443,12 +450,14 @@ poetry run ruff format --check . │ ├── league-season-ingestion.md │ ├── team-game-data-spike.md │ ├── team-hits-visualization.md -│ └── team-season-ingestion.md +│ ├── team-season-ingestion.md +│ └── team-vs-mlb-comparison.md ├── tests/ │ ├── conftest.py │ ├── factories.py │ ├── fixtures/ │ │ └── team_game_logs/ +│ ├── test_analytics_league_hitting.py │ ├── test_analytics_schemas.py │ ├── test_analytics_team_hitting.py │ ├── test_charts.py @@ -463,10 +472,12 @@ poetry run ruff format --check . │ ├── test_repositories.py │ ├── test_repositories_catalog.py │ ├── test_repositories_league.py +│ ├── test_repositories_league_season.py │ ├── test_selection.py │ ├── test_team_game_logs.py │ ├── test_team_season_ingestion.py -│ └── test_web.py +│ ├── test_web.py +│ └── test_web_league_comparison.py ├── .github/ │ └── workflows/ │ └── test.yml @@ -479,11 +490,14 @@ poetry run ruff format --check . ## Later milestones -League-wide ingestion landed in Milestone 4, so the league comparison it was a -prerequisite for is now unblocked. MLB average lines, league ranks, percentiles, -and team-vs-league comparisons belong to Milestone 5, and should check a -season's stored ingestion coverage before presenting any league statistic. See -[docs/league-season-ingestion.md](docs/league-season-ingestion.md). +Milestone 5 delivered the MLB hits-per-game comparison the Milestone 4 coverage +state unblocked, gated on that state exactly as planned. See +[docs/team-vs-mlb-comparison.md](docs/team-vs-mlb-comparison.md). + +Still unimplemented, and each needing its own definition before it is drawn: +league rank, percentiles, normalized indexes, and a league comparison for +batting strikeouts. Any of them must check a season's stored ingestion coverage +before presenting a league statistic, the way the hits comparison does. ## Disclaimer diff --git a/app/analytics/league_hitting.py b/app/analytics/league_hitting.py new file mode 100644 index 0000000..9793c35 --- /dev/null +++ b/app/analytics/league_hitting.py @@ -0,0 +1,126 @@ +"""MLB-wide hitting calculations over normalized game batting lines. + +Separate from ``app/analytics/team_hitting.py`` because it answers a different +question — how MLB overall hit, not how one club hit — and separate from +ingestion because it only reads what ingestion already persisted. Like every +other module under ``app/analytics``, it knows nothing about FastAPI, Jinja, +SQLAlchemy, Plotly, or the MLB API. +""" + +from collections.abc import Sequence + +from app.schemas.analytics import ( + LeagueHitsContext, + TeamHitsAnalysis, + TeamHitsLeagueComparison, +) +from app.schemas.games import TeamGameBattingLine +from app.schemas.ingestion import ( + LeagueSeasonIngestionState, + LeagueSeasonIngestionStatus, +) + + +class LeagueHitsAnalysisError(ValueError): + """League hitting analysis was requested with input it cannot describe.""" + + +def supports_league_wide_average( + coverage: LeagueSeasonIngestionState | None, +) -> bool: + """Say whether a season's stored games may be described as MLB-wide. + + The only acceptable evidence is the coverage state Milestone 4 records: + ``COMPLETE`` means one league-wide run discovered every MLB team for that + season and successfully ingested all of them. Anything else — a run still + ``RUNNING``, a run that lost a club, or a season no league-wide run has + ever touched — leaves an unknown number of teams missing, and an average of + whichever teams happen to be stored is not an MLB average. + + Completeness is never inferred from how many rows exist. A row count cannot + tell a full season from a season missing a club, and it certainly cannot + tell either from a season still being played. + + ``COMPLETE`` describes the refresh, not the season: an in-progress season + can hold complete coverage while every club still has games left to play. + """ + if coverage is None: + return False + return coverage.status is LeagueSeasonIngestionStatus.COMPLETE + + +def build_league_hits_context( + games: Sequence[TeamGameBattingLine], +) -> LeagueHitsContext: + """Calculate MLB hits per game across every stored team-game record. + + The average is **game-weighted**:: + + hits per game = total hits / total team-game records + + Every stored team-game record counts once, so a club that has played more + games contributes proportionally more. Averaging each club's own average + instead would silently give a team with 40 games the same weight as a team + with 162, which answers a different question and is wrong for this one. + + Raises + ------ + LeagueHitsAnalysisError + ``games`` is empty, or the records span more than one season. + """ + if not games: + raise LeagueHitsAnalysisError( + "Cannot describe MLB hitting from no team-game records" + ) + + seasons = {game.season for game in games} + if len(seasons) > 1: + raise LeagueHitsAnalysisError( + f"All team-game records must belong to one season, got {sorted(seasons)}" + ) + + team_game_records = len(games) + total_hits = sum(game.hits for game in games) + return LeagueHitsContext( + season=games[0].season, + teams_represented=len({game.team_id for game in games}), + team_game_records=team_game_records, + total_hits=total_hits, + hits_per_game=total_hits / team_game_records, + ) + + +def compare_team_hits_to_league( + analysis: TeamHitsAnalysis, + league: LeagueHitsContext, +) -> TeamHitsLeagueComparison: + """Place a team-season's hits per game beside MLB overall. + + The team side is ``TeamHitsAnalysis.summary.season_average``, which is the + same number the chart's team reference line and the Season Avg card read, + so the page cannot show two different team averages. + + The difference is descriptive subtraction and nothing more. It is not + normalized, not tested for significance, and carries no claim about why the + two numbers differ. + + Raises + ------ + LeagueHitsAnalysisError + The team analysis and the league context describe different seasons. + """ + if analysis.season != league.season: + raise LeagueHitsAnalysisError( + f"Cannot compare a {analysis.season} team-season against " + f"{league.season} MLB context" + ) + + team_hits_per_game = analysis.summary.season_average + return TeamHitsLeagueComparison( + team_id=analysis.team_id, + team_name=analysis.team_name, + season=analysis.season, + team_hits_per_game=team_hits_per_game, + league=league, + difference_vs_mlb=team_hits_per_game - league.hits_per_game, + ) diff --git a/app/database/repositories.py b/app/database/repositories.py index 2daebf7..f5d09ba 100644 --- a/app/database/repositories.py +++ b/app/database/repositories.py @@ -120,6 +120,36 @@ def list_team_season( return [record.to_domain() for record in records] +def list_league_season( + session: Session, + *, + season: int, +) -> list[TeamGameBattingLine]: + """Return every persisted batting line for a season, across all teams. + + Whether that is actually MLB-wide is not a question this function answers. + It reports what is stored; the recorded league-season coverage state is + what says whether the stored rows may be described as covering the league. + + Ordered by team, then in each team's chart order, so a run over the season + is reproducible. A full MLB season is roughly 4,860 team-game records, so + the rows are returned as domain objects and the statistics are calculated + in the analytics layer rather than pushed into SQL. + """ + stmt = ( + select(TeamGameBattingLineRecord) + .where(TeamGameBattingLineRecord.season == season) + .order_by( + TeamGameBattingLineRecord.team_id, + TeamGameBattingLineRecord.game_date, + TeamGameBattingLineRecord.game_number, + TeamGameBattingLineRecord.game_pk, + ) + ) + records = session.scalars(stmt).all() + return [record.to_domain() for record in records] + + def upsert_team_season( session: Session, *, diff --git a/app/schemas/analytics.py b/app/schemas/analytics.py index b70aaa9..1193559 100644 --- a/app/schemas/analytics.py +++ b/app/schemas/analytics.py @@ -13,6 +13,7 @@ from __future__ import annotations from datetime import date +from math import isclose from pydantic import BaseModel, ConfigDict, Field, model_validator @@ -215,3 +216,93 @@ def _summary_matches_points(self) -> TeamStrikeoutsAnalysis: def last_game_date(self) -> date: """Date of the most recent completed game in the analysis.""" return self.points[-1].game_date + + +class LeagueHitsContext(BaseModel): + """MLB-wide hitting context for one season. + + Built from every persisted team-game batting line for the season, so + ``hits_per_game`` is a game-weighted mean across team-game records rather + than the unweighted mean of each club's own average. Teams do not all play + the same number of games, so those two numbers are not the same statistic. + + Every field describes the games **currently stored** for the season. For a + season still being played that is the completed games held by the most + recent complete league-wide refresh, not a whole season. + """ + + model_config = ConfigDict(frozen=True, extra="forbid") + + season: int = Field(gt=0, description="Season the context describes.") + teams_represented: int = Field( + ge=1, + description="Distinct teams with at least one stored game in the season.", + ) + team_game_records: int = Field( + ge=1, + description="Team-game batting lines counted. One MLB game contributes " + "two records once both clubs are stored, so this is not a game count.", + ) + total_hits: int = Field( + ge=0, description="Hits summed across every counted team-game record." + ) + hits_per_game: float = Field( + ge=0, + description="total_hits / team_game_records.", + ) + + @model_validator(mode="after") + def _hits_per_game_matches_the_totals(self) -> LeagueHitsContext: + expected = self.total_hits / self.team_game_records + if not isclose(self.hits_per_game, expected, rel_tol=1e-9, abs_tol=1e-9): + raise ValueError( + f"hits_per_game ({self.hits_per_game}) must equal total_hits / " + f"team_game_records ({expected})" + ) + if self.teams_represented > self.team_game_records: + raise ValueError( + f"teams_represented ({self.teams_represented}) cannot exceed " + f"team_game_records ({self.team_game_records})" + ) + return self + + +class TeamHitsLeagueComparison(BaseModel): + """One team's hits per game placed beside MLB overall for the same season. + + Purely descriptive. A difference here says the selected team averaged more + or fewer hits per game than MLB across the stored season; it carries no + claim of significance, of skill, or of anything predictive. + """ + + model_config = ConfigDict(frozen=True, extra="forbid") + + team_id: int = Field(gt=0, description="MLB team id of the selected team.") + team_name: str = Field(min_length=1, description="Name for the season.") + season: int = Field(gt=0, description="Season compared.") + team_hits_per_game: float = Field( + ge=0, + description="The selected team's average across its stored games, taken " + "from TeamHitsSummary.season_average so the page cannot disagree with " + "itself.", + ) + league: LeagueHitsContext = Field(description="MLB-wide context compared against.") + difference_vs_mlb: float = Field( + description="team_hits_per_game - league.hits_per_game. Positive means " + "the team averaged more hits per game than MLB overall.", + ) + + @model_validator(mode="after") + def _comparison_is_internally_consistent(self) -> TeamHitsLeagueComparison: + if self.season != self.league.season: + raise ValueError( + f"season ({self.season}) must match the league context season " + f"({self.league.season})" + ) + expected = self.team_hits_per_game - self.league.hits_per_game + if not isclose(self.difference_vs_mlb, expected, rel_tol=1e-9, abs_tol=1e-9): + raise ValueError( + f"difference_vs_mlb ({self.difference_vs_mlb}) must equal " + f"team_hits_per_game - league.hits_per_game ({expected})" + ) + return self diff --git a/app/web/charts.py b/app/web/charts.py index ffbb7bd..d9053e3 100644 --- a/app/web/charts.py +++ b/app/web/charts.py @@ -16,12 +16,21 @@ from plotly.io import to_html from plotly.offline import get_plotlyjs -from app.schemas.analytics import TeamHitsAnalysis, TeamStrikeoutsAnalysis +from app.schemas.analytics import ( + TeamHitsAnalysis, + TeamHitsLeagueComparison, + TeamStrikeoutsAnalysis, +) from app.web.formatting import format_long_date, format_matchup CHART_DIV_ID = "team-hits-chart" RAW_HITS_TRACE_NAME = "Game Hits" SEASON_AVERAGE_TRACE_NAME = "Season Average" +# The hits chart can carry two horizontal reference lines at once, so its +# team line says whose average it is. The strikeout chart has one, and keeps +# the shorter label. +TEAM_SEASON_AVERAGE_TRACE_NAME = "Team Season Average" +MLB_AVERAGE_TRACE_NAME = "MLB Average" X_AXIS_TITLE = "Season Game Number" Y_AXIS_TITLE = "Hits per Game" @@ -34,6 +43,9 @@ _NAVY = "#12263f" _TEAL = "#0f8b8d" +# Distinct hue *and* distinct dash from the navy team line, so the two +# reference lines stay apart in greyscale and for a colour-blind reader. +_AMBER = "#b26a00" _RAW_LINE = "#b7c7d8" _RAW_MARKER = "#7c93ab" _GRID = "#e6ebf1" @@ -51,8 +63,16 @@ def rolling_average_trace_name(rolling_window: int) -> str: return f"{rolling_window}-Game Average" -def build_team_hits_figure(analysis: TeamHitsAnalysis) -> go.Figure: - """Build the hits-per-game figure for one team-season analysis.""" +def build_team_hits_figure( + analysis: TeamHitsAnalysis, + league_comparison: TeamHitsLeagueComparison | None = None, +) -> go.Figure: + """Build the hits-per-game figure for one team-season analysis. + + ``league_comparison`` adds a fourth trace, a horizontal MLB reference line. + It is optional: a season without complete league coverage has no MLB + average to draw, and the team's own chart must still render. + """ game_numbers = [point.season_game_number for point in analysis.points] hits = [point.hits for point in analysis.points] rolling = [point.rolling_average for point in analysis.points] @@ -107,12 +127,24 @@ def build_team_hits_figure(analysis: TeamHitsAnalysis) -> go.Figure: go.Scatter( x=[game_numbers[0], game_numbers[-1]], y=[season_average, season_average], - name=SEASON_AVERAGE_TRACE_NAME, + name=TEAM_SEASON_AVERAGE_TRACE_NAME, mode="lines", line={"color": _NAVY, "width": 2, "dash": "dash"}, hoverinfo="skip", ) ) + if league_comparison is not None: + mlb_average = league_comparison.league.hits_per_game + figure.add_trace( + go.Scatter( + x=[game_numbers[0], game_numbers[-1]], + y=[mlb_average, mlb_average], + name=MLB_AVERAGE_TRACE_NAME, + mode="lines", + line={"color": _AMBER, "width": 2, "dash": "dot"}, + hoverinfo="skip", + ) + ) figure.update_layout( template="plotly_white", diff --git a/app/web/formatting.py b/app/web/formatting.py index 5422b95..4712c66 100644 --- a/app/web/formatting.py +++ b/app/web/formatting.py @@ -3,13 +3,23 @@ from dataclasses import dataclass from datetime import date -from app.schemas.analytics import TeamHitsAnalysis, TeamStrikeoutsAnalysis +from app.schemas.analytics import ( + TeamHitsAnalysis, + TeamHitsLeagueComparison, + TeamStrikeoutsAnalysis, +) from app.schemas.games import HomeAway HITS_PER_GAME_CAPTION = "Hits per Game" STRIKEOUTS_PER_GAME_CAPTION = "Batting Strikeouts per Game" NO_PRIOR_WINDOW_VALUE = "—" NO_PRIOR_WINDOW_CAPTION = "Not enough games" +NO_LEAGUE_COMPARISON_VALUE = "—" +NO_LEAGUE_COMPARISON_CAPTION = "Comparison unavailable" +LEAGUE_COMPARISON_UNAVAILABLE_NOTE = ( + "MLB comparison unavailable. A complete league-season import is " + "required before an MLB-wide average can be shown." +) _MONTHS = ( "January", @@ -51,21 +61,33 @@ class SummaryCard: caption: str -def build_summary_cards(analysis: TeamHitsAnalysis) -> list[SummaryCard]: - """Round the analysis for display only; the calculations keep full precision.""" +def build_summary_cards( + analysis: TeamHitsAnalysis, + league_comparison: TeamHitsLeagueComparison | None = None, +) -> list[SummaryCard]: + """Round the analysis for display only; the calculations keep full precision. + + The third card is the team's difference against MLB. Without complete + league coverage for the season there is no MLB average to compare with, so + the card reads ``—`` rather than showing a number the data cannot support. + + ``TeamHitsSummary`` still calculates the prior-window comparison, and the + chart's rolling average still shows the same trend the old ``vs Prior N`` + card described. Only the card was replaced, to keep four cards on the row. + """ window = analysis.rolling_window summary = analysis.summary - if summary.change_vs_prior_window is None: - change_card = SummaryCard( - label=f"vs Prior {window}", - value=NO_PRIOR_WINDOW_VALUE, - caption=NO_PRIOR_WINDOW_CAPTION, + if league_comparison is None: + league_card = SummaryCard( + label="vs MLB", + value=NO_LEAGUE_COMPARISON_VALUE, + caption=NO_LEAGUE_COMPARISON_CAPTION, ) else: - change_card = SummaryCard( - label=f"vs Prior {window}", - value=f"{summary.change_vs_prior_window:+.2f}", + league_card = SummaryCard( + label="vs MLB", + value=f"{league_comparison.difference_vs_mlb:+.2f}", caption=HITS_PER_GAME_CAPTION, ) @@ -80,7 +102,7 @@ def build_summary_cards(analysis: TeamHitsAnalysis) -> list[SummaryCard]: value=f"{summary.season_average:.2f}", caption=HITS_PER_GAME_CAPTION, ), - change_card, + league_card, SummaryCard( label="Games Played", value=f"{summary.games_played}", @@ -89,6 +111,30 @@ def build_summary_cards(analysis: TeamHitsAnalysis) -> list[SummaryCard]: ] +def format_league_comparison_note( + comparison: TeamHitsLeagueComparison | None, +) -> str: + """Explain the MLB context on the page, or why there is none. + + The available wording deliberately says "currently stored" and names the + number of team-game records behind the average. Complete league coverage + means every team was refreshed, not that the season has finished being + played, and the sentence must not let a reader conclude otherwise. + """ + if comparison is None: + return LEAGUE_COMPARISON_UNAVAILABLE_NOTE + + league = comparison.league + return ( + f"MLB averaged {league.hits_per_game:.2f} hits per game across the " + f"{league.team_game_records:,} team-game records currently stored for " + f"{league.season}, covering {league.teams_represented} teams — total " + f"hits divided by total team-game records, so a club that has played " + f"more games counts for more. Complete league coverage means every " + f"team was refreshed, not that the season has finished being played." + ) + + def build_strikeout_summary_cards( analysis: TeamStrikeoutsAnalysis, ) -> list[SummaryCard]: diff --git a/app/web/routes.py b/app/web/routes.py index e4f8c59..06c6f27 100644 --- a/app/web/routes.py +++ b/app/web/routes.py @@ -12,6 +12,11 @@ from pydantic import BeforeValidator from sqlalchemy.orm import Session +from app.analytics.league_hitting import ( + build_league_hits_context, + compare_team_hits_to_league, + supports_league_wide_average, +) from app.analytics.team_hitting import DEFAULT_ROLLING_WINDOW, build_team_hits_analysis from app.analytics.team_strikeouts import ( MissingStrikeoutDataError, @@ -21,9 +26,12 @@ from app.database.repositories import ( MIGRATION_HINT, DatabaseSchemaMissingError, + get_league_season_ingestion, list_available_team_seasons, + list_league_season, list_team_season, ) +from app.schemas.analytics import TeamHitsAnalysis, TeamHitsLeagueComparison from app.web.charts import ( STRIKEOUTS_CHART_DIV_ID, build_team_hits_figure, @@ -36,6 +44,7 @@ from app.web.formatting import ( build_strikeout_summary_cards, build_summary_cards, + format_league_comparison_note, format_long_date, ) from app.web.navigation import HITS_PATH, STRIKEOUTS_PATH, build_nav_links @@ -168,7 +177,8 @@ def index( session, team_id=selected_team.team_id, season=selected_season ) analysis = build_team_hits_analysis(games, rolling_window=window) - figure = build_team_hits_figure(analysis) + league_comparison = _load_league_comparison(session, analysis) + figure = build_team_hits_figure(analysis, league_comparison) context.update( { @@ -176,7 +186,11 @@ def index( "analysis": analysis, "chart_html": render_figure_html(figure), "rolling_average_label": rolling_average_trace_name(window), - "summary_cards": build_summary_cards(analysis), + "summary_cards": build_summary_cards(analysis, league_comparison), + "league_comparison": league_comparison, + "league_comparison_note": format_league_comparison_note( + league_comparison + ), "data_through": format_long_date(analysis.last_game_date), } ) @@ -335,6 +349,29 @@ async def health() -> dict[str, str]: return router +def _load_league_comparison( + session: Session, + analysis: TeamHitsAnalysis, +) -> TeamHitsLeagueComparison | None: + """Read MLB context for the analysed season, or None when it is not earned. + + The completeness rule lives in ``app.analytics.league_hitting`` and the + formula lives there too; this only wires the persisted coverage state and + the persisted season to them. A season without complete coverage simply + yields None, and the team's own page renders exactly as it did before. + + The season query cannot come back empty here: the analysis was built from + games stored for this season, so those rows are part of what it returns. + """ + coverage = get_league_season_ingestion(session, season=analysis.season) + if not supports_league_wide_average(coverage): + return None + + league_games = list_league_season(session, season=analysis.season) + league = build_league_hits_context(league_games) + return compare_team_hits_to_league(analysis, league) + + def _render_schema_error( templates: Jinja2Templates, request: Request, diff --git a/app/web/templates/index.html b/app/web/templates/index.html index 3d54409..8346f4f 100644 --- a/app/web/templates/index.html +++ b/app/web/templates/index.html @@ -72,6 +72,18 @@

About this chart

the team's average across the completed games currently stored for this season.

+

+ {% if league_comparison %} + The dotted line is MLB overall. + {% endif %} + {{ league_comparison_note }} + {% if league_comparison %} + A positive vs MLB value means + {{ analysis.team_name }} averaged more hits per game than MLB across + the stored season, and a negative value fewer. It is descriptive + context, not a measure of significance. + {% endif %} +

{% endif %} {% endif %} diff --git a/docs/league-season-ingestion.md b/docs/league-season-ingestion.md index 6291934..0e3f5e9 100644 --- a/docs/league-season-ingestion.md +++ b/docs/league-season-ingestion.md @@ -260,6 +260,11 @@ itself assert, plus the coverage the application does assert. No separate season-state infrastructure was invented for this distinction. It is carried by naming, wording, and this document. +Milestone 5 is the first reader of this state. `COMPLETE` is the only value +that lets the team hits page describe a number as an MLB-wide average, and its +wording repeats the distinction above rather than relaxing it. See +`docs/team-vs-mlb-comparison.md`. + ## 6. Persistence ### Schema diff --git a/docs/team-hits-visualization.md b/docs/team-hits-visualization.md index f285903..16489ce 100644 --- a/docs/team-hits-visualization.md +++ b/docs/team-hits-visualization.md @@ -89,9 +89,15 @@ change : recent_average - prior_window_average The comparison is only made when two **complete** windows exist, that is when `games_played >= 2 * rolling_window`. Otherwise `prior_window_average` and -`change_vs_prior_window` are both `None` and the card reads `—` / -"Not enough games". Comparing a full window against a partial one would report -a difference caused by sample size rather than by hitting. +`change_vs_prior_window` are both `None`. Comparing a full window against a +partial one would report a difference caused by sample size rather than by +hitting. + +> **Milestone 5 update.** The `vs Prior 15` card was replaced on this page by a +> `vs MLB` card. Both summary values above are still calculated, validated, and +> tested on `TeamHitsSummary`, and the strikeouts page still shows its own +> `vs Prior N` card with the `—` / "Not enough games" behaviour described here. +> See `docs/team-vs-mlb-comparison.md`. Summary formulas: @@ -128,6 +134,11 @@ Three traces, in order: | 2 | `{window}-Game Average` | thick teal line | the trend, visually dominant | | 3 | `Season Average` | dashed navy horizontal line | reference level | +Milestone 5 renamed this third trace to `Team Season Average` and added an +optional fourth trace, `MLB Average`, when the season has complete league +coverage. With two horizontal reference lines on one chart, "Season Average" no +longer said whose. See `docs/team-vs-mlb-comparison.md`. + The rolling average joins its points with straight segments (`line.shape: "linear"`). Spline smoothing is deliberately not used: it bows between games and would draw averages at positions where no average was @@ -254,7 +265,14 @@ No team data has been imported yet and the import command. The page does not fetch anything from MLB to fill itself in. -## 10. Why the MLB average is deferred +## 10. Why the MLB average was deferred + +> **Milestone 5 update.** The MLB Average line now exists, under exactly the +> condition this section asked for: it is drawn only when the selected season +> has `COMPLETE` league-season coverage recorded by Milestone 4, and it is +> omitted otherwise. The reasoning below is why it did not exist in Milestone 3 +> and is left as written. See `docs/team-vs-mlb-comparison.md`. + The original mockup included an MLB Average line. It is deliberately not implemented. @@ -270,6 +288,12 @@ league-wide ingestion is defined and completeness can be checked. ## 11. Recommendation for Milestone 4 +> **Update.** Milestone 4 delivered the league-wide ingestion and coverage +> state recommended here (`docs/league-season-ingestion.md`), and Milestone 5 +> delivered the MLB-average trace it unblocked +> (`docs/team-vs-mlb-comparison.md`). League rank remains unimplemented. + + Define **league-wide ingestion** next, because it unblocks the most requested missing feature on this page and nothing else can honestly deliver it. diff --git a/docs/team-vs-mlb-comparison.md b/docs/team-vs-mlb-comparison.md new file mode 100644 index 0000000..1e2a5d6 --- /dev/null +++ b/docs/team-vs-mlb-comparison.md @@ -0,0 +1,306 @@ +# Team vs MLB comparison + +This document describes how Milestone 5 adds MLB-wide context to the team hits +page, what the MLB average means mathematically, and the coverage rule that +decides whether it may be shown at all. + +It answers one question: + +> How many hits per game does the selected team average compared with MLB +> overall? + +See `docs/team-hits-visualization.md` for the team page this extends and +`docs/league-season-ingestion.md` for the coverage state it depends on. + +## 1. MLB Hits/Game formula + +```text +MLB Hits/Game = total hits across all persisted team-game batting lines + for the selected season + ───────────────────────────────────────────────────────── + total persisted team-game batting lines for that season +``` + +Implemented in `app/analytics/league_hitting.py::build_league_hits_context`, +returning a `LeagueHitsContext`: + +| Field | Meaning | +| --- | --- | +| `season` | The season every counted record belongs to | +| `teams_represented` | Distinct `team_id`s with at least one stored game | +| `team_game_records` | Team-game batting lines counted | +| `total_hits` | Hits summed across those records | +| `hits_per_game` | `total_hits / team_game_records` | + +`LeagueHitsContext` re-derives `hits_per_game` from the two totals in a +validator, so a context holding an average nothing in it produced cannot be +constructed — by this code or by any future caller. + +### Why the denominator is team-game records + +One real MLB game produces **two** team batting lines, one per club. The +denominator counts those lines, not games. A full 30-team, 162-game season is +about 4,860 team-game records. Both the schema field name and this document say +"team-game records" everywhere for that reason. + +### Why it is game-weighted, and not the mean of team averages + +Every stored team-game record counts once. A club that has played more games +therefore contributes proportionally more, which is what "MLB overall" means. + +The alternative — calculating each club's own hits per game and averaging the +results — answers a different question and gives a different number: + +```text +Team A: 10 hits, 10 hits (2 games) +Team B: 4 hits (1 game) + +game-weighted (implemented) : 24 / 3 = 8.0 +mean of team averages : (10 + 4) / 2 = 7.0 +``` + +The unweighted mean silently gives a club with 40 games the same weight as a +club with 162. That is wrong for this question and is the specific mistake +`test_unequal_team_game_counts_are_weighted_by_games_played` exists to catch. + +The application does **not** assume equal games per club, thirty clubs, 162 +games, or 4,860 records anywhere. The denominator is always counted from the +records actually stored. + +## 2. When an MLB-wide average may be shown + +`app/analytics/league_hitting.py::supports_league_wide_average` holds the whole +rule: + +```python +coverage is not None +and coverage.status is LeagueSeasonIngestionStatus.COMPLETE +``` + +That is the coverage state Milestone 4 records for a league-wide ingestion run, +read back with `get_league_season_ingestion`. Nothing else counts as evidence. + +Completeness is **never** inferred from: + +- how many rows the season holds +- thirty team ids being present +- 4,860 team-game records existing +- 162 games per club + +A row count cannot tell a full season from a season missing a club, and it +certainly cannot tell either from a season still being played. Averaging +whichever teams happen to be stored and labelling the result "MLB" would look +authoritative while being wrong, which is worse than showing nothing. + +### Behavior in each state + +| Coverage | MLB average | Chart | Cards | Page | +| --- | --- | --- | --- | --- | +| `COMPLETE` | calculated and shown | MLB reference line added | `vs MLB` shows a signed number | works | +| `INCOMPLETE` | not calculated | no MLB line | `vs MLB` reads `—` | works | +| `RUNNING` | not calculated | no MLB line | `vs MLB` reads `—` | works | +| no record | not calculated | no MLB line | `vs MLB` reads `—` | works | + +`RUNNING` is treated exactly like `INCOMPLETE`: a run that never finished left +its coverage unknown, so it cannot support an MLB-wide claim. + +In every unavailable case the page reads: + +```text +MLB comparison unavailable. A complete league-season import is required +before an MLB-wide average can be shown. +``` + +A missing or incomplete league comparison never breaks an otherwise valid team +hits page. Everything Milestone 3 rendered still renders. + +## 3. In-progress seasons + +`COMPLETE` describes the **refresh**, not the season. It means every team +discovered for that season was successfully ingested by one league-wide run, +and — through the per-game check inside the team-season path — that each of +those clubs had every completed scheduled game represented. + +It does **not** mean the regular season has ended. Nothing in the UI or in this +document calls `COMPLETE` "season complete". + +Both of these are valid and supported: + +| Season | Coverage | Situation | Comparison | +| --- | --- | --- | --- | +| 2025 | `COMPLETE` | finished historical season | available | +| 2026 | `COMPLETE` | still being played, well under a full season of games | available | + +The 2026 case is available precisely because coverage is not a row-count rule. +Forty stored team-game records with complete coverage qualify exactly as 4,860 +would. What the MLB average then describes is MLB-wide performance across the +completed games currently stored by the latest complete league refresh, which +the page says in those words. + +## 4. Team-vs-MLB difference + +```text +difference_vs_mlb = selected_team_hits_per_game - mlb_hits_per_game +``` + +`compare_team_hits_to_league` produces a `TeamHitsLeagueComparison`, which +carries the team identity, the season, the team's average, the full +`LeagueHitsContext`, and the difference. Like the context, it re-derives the +difference in a validator and refuses a league context from a different season. + +The team side is `TeamHitsAnalysis.summary.season_average` — the same value the +chart's team reference line and the `Season Avg` card read. There is one team +average on the page, so the card, the line, and the comparison cannot disagree. + +```text +Team H/G: 8.70 MLB H/G: 8.20 vs MLB: +0.50 H/G +Team H/G: 7.95 MLB H/G: 8.20 vs MLB: -0.25 H/G +``` + +### What it does and does not mean + +A **positive** difference means the selected team averaged more hits per game +than MLB overall across the stored season. A **negative** difference means +fewer. + +That is all. The difference is plain subtraction of two descriptive averages. +It is not normalized, not ranked, not tested for significance, and says nothing +about why the two numbers differ, whether the gap will persist, or what will +happen next. The page states the direction and leaves interpretation to the +reader. + +League rank, percentiles, and normalized indexes were deliberately left out. +They need a definition and a completeness story of their own, and the basic +comparison had to be trustworthy first. + +## 5. Where each responsibility lives + +```text +GET /?team_id=136&season=2025&window=15 + │ + ├─ list_team_season(session, ...) ──► team games + ├─ build_team_hits_analysis(games, ...) ──► TeamHitsAnalysis + │ + ├─ get_league_season_ingestion(session, season=...) ──► coverage + ├─ supports_league_wide_average(coverage) ──► the rule + ├─ list_league_season(session, season=...) ──► every stored record + ├─ build_league_hits_context(records) ──► LeagueHitsContext + ├─ compare_team_hits_to_league(analysis, …) ──► TeamHitsLeagueComparison + │ + ├─ build_team_hits_figure(analysis, comparison) + └─ index.html +``` + +| Layer | Holds | +| --- | --- | +| `app/database/repositories.py` | `list_league_season` — persistence and querying only | +| `app/analytics/league_hitting.py` | the formula, the difference, the coverage rule | +| `app/schemas/analytics.py` | `LeagueHitsContext`, `TeamHitsLeagueComparison` and their invariants | +| `app/web/routes.py` | wiring, in `_load_league_comparison` | +| `app/web/charts.py` | the MLB reference trace | +| `app/web/formatting.py` | rounding and the wording | + +The analytics layer takes typed domain objects — `TeamGameBattingLine`, +`TeamHitsAnalysis`, `LeagueSeasonIngestionState` — never ORM records and never +`dict[str, Any]`. It imports no SQLAlchemy, no FastAPI, no Jinja, no Plotly, and +no MLB client, so the formula is testable with a list of batting lines. + +The route contains no formula and no completeness rule. It reads the coverage +state, asks the analytics layer whether that state earns an MLB-wide average, +and if so hands it the stored season. + +### Why the statistic was not pushed into SQL + +A full MLB season is roughly 4,860 team-game records. Summing that in Python +over domain objects is immediate, keeps the formula in the layer that owns +baseball calculations, and keeps it testable without a database. `SUM()` in the +repository would move a baseball calculation into persistence and buy nothing +measurable. If a season ever grows large enough for this to matter, the change +belongs in `build_league_hits_context`'s caller, with the measurement that +motivated it. + +`list_league_season` orders by `(team_id, game_date, game_number, game_pk)` so +a run over a season is reproducible. The statistic itself does not depend on +order. + +## 6. Chart + +`build_team_hits_figure(analysis, league_comparison=None)` gains an optional +fourth trace: + +| # | Name | Style | Purpose | +| --- | --- | --- | --- | +| 1 | `Game Hits` | thin grey line, small markers | game-to-game variation | +| 2 | `{window}-Game Average` | thick teal line | the trend | +| 3 | `Team Season Average` | dashed navy horizontal line | the team's own level | +| 4 | `MLB Average` | dotted amber horizontal line | MLB overall, when earned | + +The team reference line was renamed from `Season Average` to +`Team Season Average`. With two horizontal lines on one chart, "Season Average" +no longer says whose. The strikeout chart still has one reference line and keeps +the shorter label; Milestone 5 does not touch that chart. + +The two reference lines differ in both hue and dash pattern, so they stay +distinguishable in greyscale and for a colour-blind reader. Both are straight +lines with `hoverinfo: skip`. No spline is used anywhere on this chart, and no +annotations or controls were added. + +When `league_comparison` is `None` the figure is exactly the three-trace chart +Milestone 3 built. + +## 7. Summary cards + +```text +Recent 15-Game Avg Season Avg vs MLB Games Played + 8.87 8.90 +0.75 40 +``` + +The `vs MLB` card replaced the `vs Prior 15` card rather than being added +beside it, so the row keeps four cards on the existing grid. + +`TeamHitsSummary.prior_window_average` and `change_vs_prior_window` are still +calculated, still validated, and still tested. Only the card was removed; the +chart's rolling average shows the same trend that card described, and the +strikeouts page still displays its own `vs Prior N` card unchanged. + +Without a comparison the card reads `—` / "Comparison unavailable". It never +shows `0.00`, which would be a real number the data cannot support. + +Rounding to two decimals happens only in `app/web/formatting.py`. Every +calculation keeps full floating-point precision. + +## 8. No MLB access from the browser path + +Normal browser requests remain database-only. `app/web/routes.py` imports no +service module, and the league comparison reads two repository queries and +nothing else. Tests assert this directly by making `requests.Session.request`, +`mlbstatsapi.Mlb.__init__`, `get_team_game_batting_lines`, +`discover_mlb_teams`, and `ingest_league_season` raise, then loading the page +with complete coverage and asserting the MLB line is still drawn. + +Getting league data into the database is still an explicit CLI step: + +```bash +poetry run python scripts/import_league_season.py --season 2025 +``` + +## 9. Limitations + +- The comparison covers hits only. Batting strikeouts have no league context in + this milestone. +- `teams_represented` counts clubs with stored games for the season. Under + complete coverage that is the league; it is reported for transparency, not + used as a completeness rule. +- Coverage is per season. Complete 2025 coverage says nothing about 2026, and + the page treats them independently. +- No live MLB validation was performed for this milestone; see section 10. + +## 10. Validation performed + +Every automated test is offline. Coverage states are written through the +Milestone 4 repository functions and game rows are seeded from normalized test +records, so no test depends on MLB availability. + +The page was rendered manually against a seeded local database with complete +2025 coverage for two clubs, confirming the four summary cards, the MLB +reference line, and the explanation wording. No live MLB request was made. diff --git a/tests/factories.py b/tests/factories.py index 73f2583..3c2fe70 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -4,6 +4,7 @@ from datetime import date, timedelta from typing import Any +from app.schemas.analytics import LeagueHitsContext from app.schemas.games import TeamGameBattingLine MARINERS_ID = 136 @@ -74,3 +75,25 @@ def make_season( ) for index, value in enumerate(hits) ] + + +def make_league_hits_context( + *, + season: int = 2025, + total_hits: int = 80, + team_game_records: int = 10, + teams_represented: int = 2, +) -> LeagueHitsContext: + """Build MLB-wide context directly, for tests about presentation. + + Tests of the formula itself build the context from batting lines through + ``build_league_hits_context``. Tests about cards, traces, and wording only + need a context holding a chosen average, so they build one here. + """ + return LeagueHitsContext( + season=season, + teams_represented=teams_represented, + team_game_records=team_game_records, + total_hits=total_hits, + hits_per_game=total_hits / team_game_records, + ) diff --git a/tests/test_analytics_league_hitting.py b/tests/test_analytics_league_hitting.py new file mode 100644 index 0000000..187cc63 --- /dev/null +++ b/tests/test_analytics_league_hitting.py @@ -0,0 +1,219 @@ +"""Tests for MLB-wide hitting analytics and the coverage rule that gates them.""" + +from datetime import datetime + +import pytest + +from app.analytics.league_hitting import ( + LeagueHitsAnalysisError, + build_league_hits_context, + compare_team_hits_to_league, + supports_league_wide_average, +) +from app.analytics.team_hitting import build_team_hits_analysis +from app.schemas.ingestion import ( + LeagueSeasonIngestionState, + LeagueSeasonIngestionStatus, +) +from tests.factories import ( + MARINERS_ID, + MARINERS_NAME, + TWINS_ID, + TWINS_NAME, + make_league_hits_context, + make_season, +) + +ANGELS_ID = 108 +ANGELS_NAME = "Los Angeles Angels" + + +def coverage( + status: LeagueSeasonIngestionStatus, + *, + season: int = 2025, +) -> LeagueSeasonIngestionState: + """Build a persisted coverage state in one of its three real shapes.""" + started = datetime(2026, 3, 1, 12, 0, 0) + if status is LeagueSeasonIngestionStatus.RUNNING: + return LeagueSeasonIngestionState( + season=season, + status=status, + expected_team_count=30, + successful_team_count=0, + failed_team_count=0, + started_at=started, + ) + failed = 0 if status is LeagueSeasonIngestionStatus.COMPLETE else 1 + return LeagueSeasonIngestionState( + season=season, + status=status, + expected_team_count=30, + successful_team_count=30 - failed, + failed_team_count=failed, + started_at=started, + completed_at=datetime(2026, 3, 1, 12, 30, 0), + ) + + +# ---------------------------------------------------------------- the formula + + +def test_mlb_hits_per_game_is_total_hits_over_total_team_game_records() -> None: + games = make_season([8, 10, 6], team_id=MARINERS_ID, team_name=MARINERS_NAME) + context = build_league_hits_context(games) + assert context.total_hits == 24 + assert context.team_game_records == 3 + assert context.hits_per_game == pytest.approx(24 / 3) + + +def test_unequal_team_game_counts_are_weighted_by_games_played() -> None: + """The average is game-weighted, not the mean of each club's own average. + + Team A: 10 and 10 hits. Team B: 4 hits in its only game. + + game-weighted : 24 / 3 == 8.0 <- what this must be + mean of averages : (10 + 4) / 2 == 7.0 + """ + games = [ + *make_season([10, 10], team_id=MARINERS_ID, team_name=MARINERS_NAME), + *make_season([4], team_id=TWINS_ID, team_name=TWINS_NAME), + ] + context = build_league_hits_context(games) + assert context.hits_per_game == pytest.approx(8.0) + assert context.hits_per_game != pytest.approx(7.0) + + +def test_a_club_with_more_games_pulls_the_average_further() -> None: + """The heavier club's own average must dominate, which weighting delivers.""" + games = [ + *make_season([12] * 100, team_id=MARINERS_ID, team_name=MARINERS_NAME), + *make_season([2], team_id=TWINS_ID, team_name=TWINS_NAME), + ] + context = build_league_hits_context(games) + assert context.hits_per_game == pytest.approx(1202 / 101) + assert context.hits_per_game > 11.5 + + +def test_several_teams_in_one_season_are_accepted() -> None: + games = [ + *make_season([8, 8], team_id=MARINERS_ID, team_name=MARINERS_NAME), + *make_season([6, 10], team_id=TWINS_ID, team_name=TWINS_NAME), + *make_season([7, 5], team_id=ANGELS_ID, team_name=ANGELS_NAME), + ] + context = build_league_hits_context(games) + assert context.teams_represented == 3 + assert context.team_game_records == 6 + assert context.total_hits == 44 + assert context.hits_per_game == pytest.approx(44 / 6) + assert context.season == 2025 + + +def test_mixed_seasons_are_rejected() -> None: + games = [ + *make_season([8], season=2025), + *make_season([9], season=2026), + ] + with pytest.raises(LeagueHitsAnalysisError, match="one season"): + build_league_hits_context(games) + + +def test_empty_input_is_rejected() -> None: + """No records means no MLB average, not an average of nothing.""" + with pytest.raises(LeagueHitsAnalysisError, match="no team-game records"): + build_league_hits_context([]) + + +def test_a_partial_season_is_still_averaged_over_the_games_it_holds() -> None: + """An in-progress season divides by its own record count, not 162 or 4,860.""" + games = [ + *make_season([9] * 40, season=2026, team_id=MARINERS_ID), + *make_season([7] * 38, season=2026, team_id=TWINS_ID), + ] + context = build_league_hits_context(games) + assert context.team_game_records == 78 + assert context.hits_per_game == pytest.approx((9 * 40 + 7 * 38) / 78) + + +def test_a_zero_hit_game_counts_as_a_game() -> None: + games = make_season([0, 8, 4]) + context = build_league_hits_context(games) + assert (context.total_hits, context.team_game_records) == (12, 3) + assert context.hits_per_game == pytest.approx(4.0) + + +# ------------------------------------------------------------ the comparison + + +def test_a_team_above_mlb_gets_a_positive_difference() -> None: + analysis = build_team_hits_analysis(make_season([8, 9, 9]), rolling_window=2) + league = make_league_hits_context(total_hits=820, team_game_records=100) + result = compare_team_hits_to_league(analysis, league) + assert result.team_hits_per_game == pytest.approx(26 / 3) + assert result.difference_vs_mlb == pytest.approx(26 / 3 - 8.20) + assert result.difference_vs_mlb > 0 + + +def test_a_team_below_mlb_gets_a_negative_difference() -> None: + analysis = build_team_hits_analysis(make_season([8, 8, 8, 7]), rolling_window=2) + league = make_league_hits_context(total_hits=820, team_game_records=100) + result = compare_team_hits_to_league(analysis, league) + assert result.team_hits_per_game == pytest.approx(7.75) + assert result.difference_vs_mlb == pytest.approx(-0.45) + + +def test_the_comparison_reuses_the_team_season_average_it_was_given() -> None: + """One team average on the page, so the card and the chart cannot disagree.""" + analysis = build_team_hits_analysis(make_season([3, 4, 5, 12]), rolling_window=2) + result = compare_team_hits_to_league(analysis, make_league_hits_context()) + assert result.team_hits_per_game == analysis.summary.season_average + + +def test_the_comparison_carries_the_team_identity_and_league_context() -> None: + analysis = build_team_hits_analysis(make_season([8] * 4), rolling_window=2) + league = make_league_hits_context( + teams_represented=30, team_game_records=100, total_hits=820 + ) + result = compare_team_hits_to_league(analysis, league) + assert (result.team_id, result.team_name) == (MARINERS_ID, MARINERS_NAME) + assert result.season == 2025 + assert result.league == league + + +def test_comparing_across_seasons_is_rejected() -> None: + analysis = build_team_hits_analysis(make_season([8] * 4, season=2026)) + league = make_league_hits_context(season=2025) + with pytest.raises(LeagueHitsAnalysisError, match="2026"): + compare_team_hits_to_league(analysis, league) + + +# -------------------------------------------------------------- the coverage rule + + +def test_complete_coverage_supports_an_mlb_wide_average() -> None: + assert supports_league_wide_average(coverage(LeagueSeasonIngestionStatus.COMPLETE)) + + +@pytest.mark.parametrize( + "status", + [LeagueSeasonIngestionStatus.INCOMPLETE, LeagueSeasonIngestionStatus.RUNNING], +) +def test_other_coverage_states_do_not(status: LeagueSeasonIngestionStatus) -> None: + assert not supports_league_wide_average(coverage(status)) + + +def test_a_season_with_no_coverage_record_does_not() -> None: + assert not supports_league_wide_average(None) + + +def test_complete_coverage_of_an_in_progress_season_still_supports_it() -> None: + """COMPLETE describes the refresh, not the season being over.""" + assert supports_league_wide_average( + coverage(LeagueSeasonIngestionStatus.COMPLETE, season=2026) + ) + + +def test_more_teams_than_records_is_refused_by_the_context() -> None: + """A team cannot be represented without at least one stored record.""" + with pytest.raises(ValueError, match="teams_represented"): + make_league_hits_context(teams_represented=30, team_game_records=10) diff --git a/tests/test_analytics_schemas.py b/tests/test_analytics_schemas.py index 1839000..f594f90 100644 --- a/tests/test_analytics_schemas.py +++ b/tests/test_analytics_schemas.py @@ -6,7 +6,9 @@ from pydantic import ValidationError from app.schemas.analytics import ( + LeagueHitsContext, TeamHitsAnalysis, + TeamHitsLeagueComparison, TeamHitsPoint, TeamHitsSummary, TeamStrikeoutsAnalysis, @@ -249,3 +251,87 @@ def test_strikeout_analysis_last_game_date_is_the_final_point() -> None: def test_strikeout_analysis_rejects_unknown_fields() -> None: with pytest.raises(ValidationError): make_strikeout_analysis(strikeout_rate=0.22) + + +# ---------------------------------------------- league context and comparison + + +def league_context(**overrides: object) -> LeagueHitsContext: + base: dict[str, object] = { + "season": 2025, + "teams_represented": 30, + "team_game_records": 4860, + "total_hits": 39852, + "hits_per_game": 39852 / 4860, + } + base.update(overrides) + return LeagueHitsContext(**base) + + +def test_league_context_accepts_consistent_totals() -> None: + context = league_context() + assert context.hits_per_game == pytest.approx(39852 / 4860) + + +def test_league_context_rejects_an_average_that_disagrees_with_its_totals() -> None: + """The context cannot be built holding a number nothing in it produced.""" + with pytest.raises(ValidationError, match="hits_per_game"): + league_context(hits_per_game=8.0) + + +def test_league_context_rejects_more_teams_than_records() -> None: + with pytest.raises(ValidationError, match="teams_represented"): + league_context( + teams_represented=30, team_game_records=10, total_hits=80, hits_per_game=8.0 + ) + + +def test_league_context_requires_at_least_one_record() -> None: + with pytest.raises(ValidationError): + league_context( + team_game_records=0, teams_represented=0, total_hits=0, hits_per_game=0.0 + ) + + +def test_league_context_is_immutable_and_closed() -> None: + context = league_context() + with pytest.raises(ValidationError): + context.hits_per_game = 9.0 + with pytest.raises(ValidationError): + league_context(games_played=10) + + +def test_comparison_rejects_a_difference_that_does_not_subtract() -> None: + with pytest.raises(ValidationError, match="difference_vs_mlb"): + TeamHitsLeagueComparison( + team_id=136, + team_name="Seattle Mariners", + season=2025, + team_hits_per_game=8.7, + league=league_context(), + difference_vs_mlb=99.0, + ) + + +def test_comparison_rejects_a_league_context_from_another_season() -> None: + with pytest.raises(ValidationError, match="season"): + TeamHitsLeagueComparison( + team_id=136, + team_name="Seattle Mariners", + season=2026, + team_hits_per_game=8.7, + league=league_context(season=2025), + difference_vs_mlb=8.7 - 39852 / 4860, + ) + + +def test_comparison_allows_a_negative_difference() -> None: + comparison = TeamHitsLeagueComparison( + team_id=136, + team_name="Seattle Mariners", + season=2025, + team_hits_per_game=7.0, + league=league_context(), + difference_vs_mlb=7.0 - 39852 / 4860, + ) + assert comparison.difference_vs_mlb < 0 diff --git a/tests/test_charts.py b/tests/test_charts.py index dbc44a0..7bf07d5 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -2,12 +2,14 @@ import pytest +from app.analytics.league_hitting import compare_team_hits_to_league from app.analytics.team_hitting import build_team_hits_analysis from app.web.charts import ( CHART_CONFIG, CHART_DIV_ID, + MLB_AVERAGE_TRACE_NAME, RAW_HITS_TRACE_NAME, - SEASON_AVERAGE_TRACE_NAME, + TEAM_SEASON_AVERAGE_TRACE_NAME, X_AXIS_TITLE, Y_AXIS_TITLE, build_team_hits_figure, @@ -15,7 +17,7 @@ render_figure_html, rolling_average_trace_name, ) -from tests.factories import make_season +from tests.factories import make_league_hits_context, make_season @pytest.fixture @@ -24,7 +26,18 @@ def figure(): return build_team_hits_figure(analysis) -def test_figure_has_three_traces(figure) -> None: +@pytest.fixture +def league_figure(): + """A figure built with MLB context, as a season with complete coverage gets.""" + analysis = build_team_hits_analysis(make_season([8, 4, 12, 6, 9]), rolling_window=5) + league = make_league_hits_context(total_hits=61, team_game_records=10) + return build_team_hits_figure( + analysis, compare_team_hits_to_league(analysis, league) + ) + + +def test_figure_has_three_traces_without_mlb_context(figure) -> None: + """No complete league coverage means no MLB line, and a working chart.""" assert len(figure.data) == 3 @@ -32,10 +45,44 @@ def test_trace_names_describe_the_three_series(figure) -> None: assert [trace.name for trace in figure.data] == [ RAW_HITS_TRACE_NAME, "5-Game Average", - SEASON_AVERAGE_TRACE_NAME, + TEAM_SEASON_AVERAGE_TRACE_NAME, + ] + + +def test_mlb_context_adds_a_fourth_named_trace(league_figure) -> None: + assert [trace.name for trace in league_figure.data] == [ + RAW_HITS_TRACE_NAME, + "5-Game Average", + TEAM_SEASON_AVERAGE_TRACE_NAME, + MLB_AVERAGE_TRACE_NAME, ] +def test_mlb_trace_is_a_flat_straight_reference_line(league_figure) -> None: + trace = league_figure.data[3] + assert list(trace.y) == pytest.approx([6.1, 6.1]) + assert list(trace.x) == [1, 5] + assert trace.line.shape in (None, "linear") + assert trace.hoverinfo == "skip" + + +def test_mlb_trace_reads_the_league_context_average(league_figure) -> None: + """The chart line and the vs MLB card must not be able to disagree.""" + analysis = build_team_hits_analysis(make_season([3, 4, 5, 12]), rolling_window=2) + league = make_league_hits_context(total_hits=45, team_game_records=6) + comparison = compare_team_hits_to_league(analysis, league) + figure = build_team_hits_figure(analysis, comparison) + assert list(figure.data[3].y) == pytest.approx( + [comparison.league.hits_per_game] * 2 + ) + + +def test_the_two_reference_lines_are_visually_distinguishable(league_figure) -> None: + team, mlb = league_figure.data[2], league_figure.data[3] + assert team.line.dash != mlb.line.dash + assert team.line.color != mlb.line.color + + @pytest.mark.parametrize("window", [5, 10, 15, 30]) def test_rolling_trace_label_reflects_the_selected_window(window: int) -> None: analysis = build_team_hits_analysis(make_season([7] * 40), rolling_window=window) diff --git a/tests/test_formatting.py b/tests/test_formatting.py index 6e746ed..1fe2dfb 100644 --- a/tests/test_formatting.py +++ b/tests/test_formatting.py @@ -4,15 +4,18 @@ import pytest +from app.analytics.league_hitting import compare_team_hits_to_league from app.analytics.team_hitting import build_team_hits_analysis from app.analytics.team_strikeouts import build_team_strikeouts_analysis from app.web.formatting import ( + LEAGUE_COMPARISON_UNAVAILABLE_NOTE, build_strikeout_summary_cards, build_summary_cards, + format_league_comparison_note, format_long_date, format_matchup, ) -from tests.factories import make_season +from tests.factories import make_league_hits_context, make_season def test_long_date_has_no_padded_day() -> None: @@ -28,15 +31,31 @@ def test_matchup_uses_vs_at_home_and_at_on_the_road() -> None: assert format_matchup("Minnesota Twins", "away") == "at Minnesota Twins" +def comparison(hits: list[int], *, window: int, mlb_hits_per_game: float): + """Build a team analysis and an MLB comparison against a chosen average.""" + analysis = build_team_hits_analysis(make_season(hits), rolling_window=window) + league = make_league_hits_context( + total_hits=round(mlb_hits_per_game * 100), team_game_records=100 + ) + return analysis, compare_team_hits_to_league(analysis, league) + + def test_summary_cards_are_labelled_with_the_selected_window() -> None: + """Milestone 5 replaced the prior-window card with the MLB comparison. + + ``TeamHitsSummary`` still calculates the prior-window change, and the + strikeouts page still shows it; only the hits card row changed, so that the + row keeps four cards instead of growing a fifth. + """ analysis = build_team_hits_analysis(make_season([8] * 40), rolling_window=10) labels = [card.label for card in build_summary_cards(analysis)] assert labels == [ "Recent 10-Game Avg", "Season Avg", - "vs Prior 10", + "vs MLB", "Games Played", ] + assert analysis.summary.change_vs_prior_window is not None def test_summary_card_values_are_rounded_for_display() -> None: @@ -49,23 +68,36 @@ def test_summary_card_values_are_rounded_for_display() -> None: assert cards[3].value == "10" -def test_change_card_is_signed() -> None: - analysis = build_team_hits_analysis( - make_season([4] * 5 + [7] * 5), rolling_window=5 - ) - assert build_summary_cards(analysis)[2].value == "+3.00" +def test_league_card_is_signed() -> None: + above, above_comparison = comparison([9] * 10, window=5, mlb_hits_per_game=8.50) + card = build_summary_cards(above, above_comparison)[2] + assert (card.value, card.caption) == ("+0.50", "Hits per Game") - declining = build_team_hits_analysis( - make_season([9] * 5 + [8] * 5), rolling_window=5 - ) - assert build_summary_cards(declining)[2].value == "-1.00" + below, below_comparison = comparison([8] * 10, window=5, mlb_hits_per_game=8.25) + assert build_summary_cards(below, below_comparison)[2].value == "-0.25" -def test_change_card_explains_a_missing_prior_window() -> None: +def test_league_card_says_when_no_mlb_average_is_available() -> None: + """Without complete league coverage the card must not invent a number.""" analysis = build_team_hits_analysis(make_season([6] * 9), rolling_window=5) card = build_summary_cards(analysis)[2] assert card.value == "—" - assert card.caption == "Not enough games" + assert card.caption == "Comparison unavailable" + + +def test_league_note_explains_why_a_comparison_is_missing() -> None: + note = format_league_comparison_note(None) + assert note == LEAGUE_COMPARISON_UNAVAILABLE_NOTE + assert "complete league-season import" in note + + +def test_league_note_reports_the_average_and_what_it_covers() -> None: + _, available = comparison([9] * 10, window=5, mlb_hits_per_game=8.20) + note = format_league_comparison_note(available) + assert "8.20 hits per game" in note + assert "100 team-game records" in note + assert "currently stored" in note + assert "finished being played" in note def test_hits_per_game_is_the_caption_for_rate_cards() -> None: diff --git a/tests/test_repositories_league_season.py b/tests/test_repositories_league_season.py new file mode 100644 index 0000000..0d95f7a --- /dev/null +++ b/tests/test_repositories_league_season.py @@ -0,0 +1,146 @@ +"""Tests for the season-wide team-game query league analytics reads.""" + +from datetime import date + +from sqlalchemy.orm import Session + +from app.database.repositories import list_league_season, upsert_team_season +from app.schemas.games import TeamGameBattingLine +from tests.factories import ( + MARINERS_ID, + MARINERS_NAME, + TWINS_ID, + TWINS_NAME, + make_season, +) + +ANGELS_ID = 108 +ANGELS_NAME = "Los Angeles Angels" + + +def store(session: Session, lines: list[TeamGameBattingLine]) -> None: + upsert_team_season(session, lines=lines) + session.commit() + + +def store_team_season( + session: Session, + *, + hits: list[int], + team_id: int, + team_name: str, + season: int = 2025, +) -> list[TeamGameBattingLine]: + """Persist one team-season, keeping game ids unique across teams. + + ``make_season`` derives game ids from the season alone, so two clubs built + for the same season would collide on ``game_pk``. Offsetting by team id + keeps each stored row's identity distinct, as real MLB data is. + """ + lines = [ + line.model_copy(update={"game_pk": line.game_pk + team_id * 100_000}) + for line in make_season( + hits, team_id=team_id, team_name=team_name, season=season + ) + ] + store(session, lines) + return lines + + +def test_every_team_game_record_for_the_season_is_returned( + migrated_session: Session, +) -> None: + store_team_season( + migrated_session, hits=[8, 9], team_id=MARINERS_ID, team_name=MARINERS_NAME + ) + store_team_season( + migrated_session, hits=[4, 5, 6], team_id=TWINS_ID, team_name=TWINS_NAME + ) + store_team_season( + migrated_session, hits=[7], team_id=ANGELS_ID, team_name=ANGELS_NAME + ) + + stored = list_league_season(migrated_session, season=2025) + assert len(stored) == 6 + assert {line.team_id for line in stored} == {MARINERS_ID, TWINS_ID, ANGELS_ID} + assert sum(line.hits for line in stored) == 39 + + +def test_other_seasons_are_excluded(migrated_session: Session) -> None: + store_team_season( + migrated_session, + hits=[8, 8, 8], + team_id=MARINERS_ID, + team_name=MARINERS_NAME, + season=2025, + ) + store_team_season( + migrated_session, + hits=[3, 3], + team_id=MARINERS_ID, + team_name=MARINERS_NAME, + season=2026, + ) + + stored = list_league_season(migrated_session, season=2025) + assert [line.season for line in stored] == [2025] * 3 + assert sum(line.hits for line in stored) == 24 + assert len(list_league_season(migrated_session, season=2026)) == 2 + + +def test_a_season_with_nothing_stored_returns_no_records( + migrated_session: Session, +) -> None: + store_team_season( + migrated_session, hits=[8], team_id=MARINERS_ID, team_name=MARINERS_NAME + ) + assert list_league_season(migrated_session, season=1998) == [] + + +def test_domain_objects_are_returned_not_orm_records( + migrated_session: Session, +) -> None: + lines = store_team_season( + migrated_session, hits=[8, 9], team_id=MARINERS_ID, team_name=MARINERS_NAME + ) + stored = list_league_season(migrated_session, season=2025) + assert all(isinstance(line, TeamGameBattingLine) for line in stored) + assert stored == lines + + +def test_records_come_back_grouped_by_team_in_game_order( + migrated_session: Session, +) -> None: + """Deterministic order, so a run over a season is reproducible.""" + store_team_season( + migrated_session, hits=[1, 2, 3], team_id=TWINS_ID, team_name=TWINS_NAME + ) + store_team_season( + migrated_session, hits=[4, 5, 6], team_id=ANGELS_ID, team_name=ANGELS_NAME + ) + + stored = list_league_season(migrated_session, season=2025) + assert [line.team_id for line in stored] == [ANGELS_ID] * 3 + [TWINS_ID] * 3 + for team_lines in (stored[:3], stored[3:]): + dates = [line.game_date for line in team_lines] + assert dates == sorted(dates) + assert list_league_season(migrated_session, season=2025) == stored + + +def test_both_halves_of_a_doubleheader_keep_their_sequence( + migrated_session: Session, +) -> None: + day = date(2025, 7, 4) + lines = [ + line.model_copy(update={"game_date": day, "game_number": number}) + for line, number in zip( + make_season([5, 9], team_id=MARINERS_ID, team_name=MARINERS_NAME), + (2, 1), + strict=True, + ) + ] + store(migrated_session, lines) + + stored = list_league_season(migrated_session, season=2025) + assert [line.game_number for line in stored] == [1, 2] + assert [line.hits for line in stored] == [9, 5] diff --git a/tests/test_web.py b/tests/test_web.py index 9ca5c64..e081db2 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -231,11 +231,12 @@ def test_page_contains_all_three_chart_series(client: TestClient, seed: SeedFn) def test_page_contains_the_summary_cards(client: TestClient, seed: SeedFn) -> None: + """Milestone 5 replaced the prior-window card with the MLB comparison.""" seed(hits=[6, 8, 10, 12] * 10) body = client.get("/").text assert "Recent 15-Game Avg" in body assert "Season Avg" in body - assert "vs Prior 15" in body + assert "vs MLB" in body assert "Games Played" in body assert "40" in body diff --git a/tests/test_web_league_comparison.py b/tests/test_web_league_comparison.py new file mode 100644 index 0000000..8978af3 --- /dev/null +++ b/tests/test_web_league_comparison.py @@ -0,0 +1,354 @@ +"""Tests for how the hits page gates MLB context on league-season coverage. + +Every case here is offline. The database is seeded directly and the recorded +coverage state is written directly, because coverage is what the page reads — +never a live MLB call, and never a row count. +""" + +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.database.engine import build_engine, build_session_factory +from app.database.repositories import ( + record_league_season_ingestion_finish, + record_league_season_ingestion_start, + upsert_team_season, +) +from app.main import create_app +from app.web.dependencies import get_db_session +from app.web.formatting import LEAGUE_COMPARISON_UNAVAILABLE_NOTE +from tests.factories import ( + MARINERS_ID, + MARINERS_NAME, + TWINS_ID, + TWINS_NAME, + make_season, +) + +STARTED = datetime(2026, 3, 1, 12, 0, 0) +FINISHED = datetime(2026, 3, 1, 12, 30, 0) + +SeedFn = Callable[..., None] +CoverageFn = Callable[..., None] + + +@pytest.fixture +def session_factory(migrated_db_path: Path) -> Generator[Callable[[], Session]]: + engine = build_engine(f"sqlite:///{migrated_db_path}") + factory = build_session_factory(engine) + try: + yield factory + finally: + engine.dispose() + + +@pytest.fixture +def seed(session_factory: Callable[[], Session]) -> SeedFn: + """Persist one team-season, with game ids kept unique across teams.""" + + def _seed( + hits: list[int], + *, + team_id: int = MARINERS_ID, + team_name: str = MARINERS_NAME, + season: int = 2025, + strikeouts: list[int] | None = None, + ) -> None: + lines = [ + line.model_copy(update={"game_pk": line.game_pk + team_id * 100_000}) + for line in make_season( + hits, + team_id=team_id, + team_name=team_name, + season=season, + strikeouts=strikeouts, + ) + ] + session = session_factory() + try: + upsert_team_season(session, lines=lines) + session.commit() + finally: + session.close() + + return _seed + + +@pytest.fixture +def record_coverage(session_factory: Callable[[], Session]) -> CoverageFn: + """Write a league-season coverage row the way an ingestion run would.""" + + def _record( + *, + season: int = 2025, + teams: int = 30, + failed: int = 0, + finished: bool = True, + ) -> None: + session = session_factory() + try: + with session.begin(): + record_league_season_ingestion_start( + session, + season=season, + expected_team_count=teams, + started_at=STARTED, + ) + if not finished: + return + with session.begin(): + record_league_season_ingestion_finish( + session, + season=season, + expected_team_count=teams, + successful_team_count=teams - failed, + failed_team_count=failed, + started_at=STARTED, + completed_at=FINISHED, + ) + finally: + session.close() + + return _record + + +@pytest.fixture +def client(session_factory: Callable[[], Session]) -> TestClient: + app = create_app() + + def override_session() -> Iterator[Session]: + session = session_factory() + try: + yield session + finally: + session.close() + + app.dependency_overrides[get_db_session] = override_session + return TestClient(app) + + +def seed_two_teams(seed: SeedFn, *, season: int = 2025) -> None: + """Mariners average 9 hits, Twins 7; MLB across both is 8.00.""" + seed([9] * 20, team_id=MARINERS_ID, team_name=MARINERS_NAME, season=season) + seed([7] * 20, team_id=TWINS_ID, team_name=TWINS_NAME, season=season) + + +def hits_page(client: TestClient, *, season: int = 2025) -> str: + response = client.get(f"/?team_id={MARINERS_ID}&season={season}") + assert response.status_code == 200 + return response.text + + +# ------------------------------------------------------------------ COMPLETE + + +def test_complete_coverage_shows_the_mlb_comparison( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed) + record_coverage(teams=2) + body = hits_page(client) + + assert "vs MLB" in body + assert "+1.00" in body + assert "8.00 hits per game" in body + assert LEAGUE_COMPARISON_UNAVAILABLE_NOTE not in body + + +def test_complete_coverage_draws_the_mlb_reference_line( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed) + record_coverage(teams=2) + body = hits_page(client) + + assert "MLB Average" in body + assert "Team Season Average" in body + + +def test_a_team_below_mlb_reads_as_a_negative_difference( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed) + record_coverage(teams=2) + body = client.get(f"/?team_id={TWINS_ID}&season=2025").text + assert "-1.00" in body + + +def test_the_page_does_not_call_complete_coverage_a_finished_season( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed) + record_coverage(teams=2) + body = hits_page(client) + assert "season complete" not in body.lower() + assert "currently stored" in body + + +def test_the_page_reads_the_difference_as_descriptive_context( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + """A positive difference means more hits per game, and nothing more.""" + seed_two_teams(seed) + record_coverage(teams=2) + body = hits_page(client) + assert "averaged more hits per game than MLB" in body + assert "not a measure of significance" in body + + +# ------------------------------------------- INCOMPLETE, RUNNING, and no record + + +def test_incomplete_coverage_withholds_the_mlb_average( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed) + record_coverage(teams=2, failed=1) + body = hits_page(client) + + assert LEAGUE_COMPARISON_UNAVAILABLE_NOTE in body + assert "MLB Average" not in body + assert "hits per game across" not in body + + +def test_incomplete_coverage_still_renders_the_team_chart( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed) + record_coverage(teams=2, failed=1) + body = hits_page(client) + + assert "Game Hits" in body + assert "15-Game Average" in body + assert "Team Season Average" in body + assert "Season Avg" in body + + +def test_running_coverage_behaves_like_incomplete( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + """A run that never finished leaves coverage unknown, so it is not trusted.""" + seed_two_teams(seed) + record_coverage(teams=2, finished=False) + body = hits_page(client) + + assert LEAGUE_COMPARISON_UNAVAILABLE_NOTE in body + assert "MLB Average" not in body + assert "Game Hits" in body + + +def test_no_coverage_record_behaves_like_incomplete( + client: TestClient, seed: SeedFn +) -> None: + seed_two_teams(seed) + body = hits_page(client) + + assert LEAGUE_COMPARISON_UNAVAILABLE_NOTE in body + assert "MLB Average" not in body + assert "Game Hits" in body + + +def test_coverage_for_another_season_does_not_unlock_this_one( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + seed_two_teams(seed, season=2025) + seed([9] * 5, season=2026) + record_coverage(season=2025, teams=2) + + assert LEAGUE_COMPARISON_UNAVAILABLE_NOTE in hits_page(client, season=2026) + assert LEAGUE_COMPARISON_UNAVAILABLE_NOTE not in hits_page(client, season=2025) + + +def test_the_unavailable_card_shows_a_dash_not_a_number( + client: TestClient, seed: SeedFn +) -> None: + seed_two_teams(seed) + body = hits_page(client) + assert "Comparison unavailable" in body + + +# ----------------------------------------------------- in-progress 2026 season + + +def test_complete_coverage_of_a_partial_season_still_compares( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + """Far fewer rows than a full season, and the comparison is still allowed. + + Coverage says every discovered team was refreshed. Row count is never the + completeness rule, so 40 team-game records qualify exactly as 4,860 would. + """ + seed([10] * 20, team_id=MARINERS_ID, team_name=MARINERS_NAME, season=2026) + seed([6] * 20, team_id=TWINS_ID, team_name=TWINS_NAME, season=2026) + record_coverage(season=2026, teams=2) + + body = hits_page(client, season=2026) + assert "MLB Average" in body + assert "8.00 hits per game" in body + assert "+2.00" in body + assert "40 team-game records" in body + + +def test_unequal_game_counts_are_weighted_on_the_page( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + """Mariners 10 hits over 20 games, Twins 4 hits in 1: MLB is 204/21, not 7. + + The unweighted mean of the two club averages would be 7.00, which would + show a +3.00 difference. The game-weighted answer is about 9.71. + """ + seed([10] * 20, team_id=MARINERS_ID, team_name=MARINERS_NAME, season=2026) + seed([4], team_id=TWINS_ID, team_name=TWINS_NAME, season=2026) + record_coverage(season=2026, teams=2) + + body = hits_page(client, season=2026) + assert "9.71 hits per game" in body + assert "+0.29" in body + assert "+3.00" not in body + + +# ------------------------------------------------------------------ no MLB calls + + +def test_the_comparison_never_reaches_the_mlb_api( + client: TestClient, + seed: SeedFn, + record_coverage: CoverageFn, + monkeypatch: pytest.MonkeyPatch, +) -> None: + def fail(*args: object, **kwargs: object) -> None: + raise AssertionError("The web layer must not reach the MLB Stats API") + + seed_two_teams(seed) + record_coverage(teams=2) + monkeypatch.setattr(requests.Session, "request", fail) + monkeypatch.setattr("mlbstatsapi.Mlb.__init__", fail) + monkeypatch.setattr("app.services.team_game_logs.get_team_game_batting_lines", fail) + monkeypatch.setattr("app.services.league_teams.discover_mlb_teams", fail) + monkeypatch.setattr( + "app.services.league_season_ingestion.ingest_league_season", fail + ) + + assert "MLB Average" in hits_page(client) + + +def test_the_strikeouts_page_is_unaffected_by_league_coverage( + client: TestClient, seed: SeedFn, record_coverage: CoverageFn +) -> None: + """Milestone 5 adds no league context to batting strikeouts.""" + seed([9] * 20, strikeouts=[8] * 20) + record_coverage(teams=1) + response = client.get(f"/strikeouts?team_id={MARINERS_ID}&season=2025") + assert response.status_code == 200 + + body = response.text + assert "Game Strikeouts" in body + assert "vs Prior 15" in body + assert "MLB Average" not in body + assert "vs MLB" not in body