diff --git a/app/analytics/player_hitting.py b/app/analytics/player_hitting.py new file mode 100644 index 0000000..0ff58d1 --- /dev/null +++ b/app/analytics/player_hitting.py @@ -0,0 +1,133 @@ +"""Season-level hitting rates for one player. + +Answers one question: + + What did this player's overall offensive season look like? + +The input is one stored season aggregate, not a list of games, so this is a +single calculation rather than a trend: no rolling windows, no chart points, +and no comparison with MLB. For an in-progress season the aggregate reflects +the most recent import, and nothing here implies the season is complete. A +player who played for more than one club is described by the combined season +aggregate; the individual team stints are not modelled. + +Every rate is a ratio of season totals. A rate whose denominator is zero is +undefined and returned as ``None``, never ``0.0``: a player with no at-bats did +not bat ``.000``, and presenting it that way would fabricate a statistic. + +Like the rest of the package this layer is free of FastAPI, Jinja, SQLAlchemy, +Plotly, and the MLB API. Values keep full precision; rounding is presentation. +""" + +from app.schemas.analytics import PlayerHittingOverview, PlayerPlateAppearanceRates +from app.schemas.players import PlayerSeasonHitting + + +class PlayerHittingAnalysisError(ValueError): + """A stored hitting line contradicts itself and cannot be described.""" + + +def build_player_hitting_overview( + hitting: PlayerSeasonHitting, +) -> PlayerHittingOverview: + """Derive AVG, OBP, SLG, OPS, and the plate-appearance rate profile. + + Formulas, all over season totals: + + - ``TB = H + 2B + 2 * 3B + 3 * HR``. ``H`` already counts the first base + of every hit, so each extra-base hit adds only its extra bases. + - ``AVG = H / AB`` + - ``OBP = (H + BB + HBP) / (AB + BB + HBP + SF)``. Sacrifice bunts are not + in the denominator. + - ``SLG = TB / AB`` + - ``OPS = OBP + SLG``, undefined when either component is. + - ``K% = SO / PA``, ``BB% = BB / PA``, ``HR% = HR / PA``. + + Raises + ------ + PlayerHittingAnalysisError + The line records more of an outcome than the plate appearances or + at-bats that contain it, which no real season can. + """ + _require_subset_counts(hitting) + + total_bases = ( + hitting.hits + hitting.doubles + 2 * hitting.triples + 3 * hitting.home_runs + ) + batting_average = _ratio(hitting.hits, hitting.at_bats) + on_base_percentage = _ratio( + hitting.hits + hitting.base_on_balls + hitting.hit_by_pitch, + hitting.at_bats + + hitting.base_on_balls + + hitting.hit_by_pitch + + hitting.sac_flies, + ) + slugging_percentage = _ratio(total_bases, hitting.at_bats) + on_base_plus_slugging = ( + None + if on_base_percentage is None or slugging_percentage is None + else on_base_percentage + slugging_percentage + ) + + return PlayerHittingOverview( + hitting=hitting, + total_bases=total_bases, + batting_average=batting_average, + on_base_percentage=on_base_percentage, + slugging_percentage=slugging_percentage, + on_base_plus_slugging=on_base_plus_slugging, + plate_appearance_rates=_plate_appearance_rates(hitting), + ) + + +def _require_subset_counts(hitting: PlayerSeasonHitting) -> None: + """Reject a line whose numerators exceed the totals that contain them. + + ``PlayerSeasonHitting`` does not prove these relationships, and each one + bounds a rate derived here. Checking them explicitly turns a corrupted + stored row into a named data-integrity error rather than a schema failure + while constructing the analysis. A batting strikeout always ends an + at-bat, so it is bounded by at-bats as well as plate appearances. + + Plate-appearance bounds are checked first so the error names the rate + denominator that was actually exceeded. + """ + plate_appearances = hitting.plate_appearances + for count, count_name, total, total_name in ( + (hitting.strikeouts, "strikeouts", plate_appearances, "plate appearances"), + (hitting.base_on_balls, "walks", plate_appearances, "plate appearances"), + (hitting.home_runs, "home runs", plate_appearances, "plate appearances"), + (hitting.hits, "hits", hitting.at_bats, "at-bats"), + (hitting.strikeouts, "strikeouts", hitting.at_bats, "at-bats"), + ): + if count > total: + raise PlayerHittingAnalysisError( + f"Player {hitting.player_id}'s stored {hitting.season} hitting " + f"line records {count} {count_name} in {total} {total_name}; " + f"{count_name} cannot exceed {total_name}" + ) + + +def _plate_appearance_rates( + hitting: PlayerSeasonHitting, +) -> PlayerPlateAppearanceRates | None: + """Return K%, BB%, and HR% together, or None with no plate appearances.""" + plate_appearances = hitting.plate_appearances + if plate_appearances == 0: + return None + return PlayerPlateAppearanceRates( + plate_appearances=plate_appearances, + strikeouts=hitting.strikeouts, + base_on_balls=hitting.base_on_balls, + home_runs=hitting.home_runs, + strikeout_rate=hitting.strikeouts / plate_appearances, + walk_rate=hitting.base_on_balls / plate_appearances, + home_run_rate=hitting.home_runs / plate_appearances, + ) + + +def _ratio(numerator: int, denominator: int) -> float | None: + """Divide, or return None when the rate is undefined.""" + if denominator == 0: + return None + return numerator / denominator diff --git a/app/database/repositories.py b/app/database/repositories.py index 27cfcc3..50546ee 100644 --- a/app/database/repositories.py +++ b/app/database/repositories.py @@ -528,6 +528,46 @@ def list_player_catalog_seasons(session: Session) -> list[int]: ) from exc +def get_player_catalog_entry( + session: Session, + *, + player_id: int, + season: int, +) -> PlayerSeasonCatalogEntry | None: + """Return one player's catalog membership for one season, or None. + + Membership comes from ``player_seasons`` alone. A stored identity or a + stored hitting row does not by itself place a player in a season. + """ + stmt = ( + select(PlayerSeasonCatalogRecord, PlayerRecord) + .join( + PlayerRecord, + PlayerRecord.player_id == PlayerSeasonCatalogRecord.player_id, + ) + .where( + PlayerSeasonCatalogRecord.player_id == player_id, + PlayerSeasonCatalogRecord.season == season, + ) + ) + try: + row = session.execute(stmt).one_or_none() + except OperationalError as exc: + message = str(exc.orig if exc.orig is not None else exc).lower() + if "no such table" not in message or not ( + "player_seasons" in message or "players" in message + ): + raise + raise DatabaseSchemaMissingError( + "Player catalog tables are missing. " + f"Apply migrations with: {MIGRATION_HINT}" + ) from exc + if row is None: + return None + membership, player = row + return membership.to_domain(player.to_domain()) + + def list_player_catalog( session: Session, *, season: int ) -> list[PlayerSeasonCatalogEntry]: diff --git a/app/schemas/analytics.py b/app/schemas/analytics.py index c04c21f..d78c902 100644 --- a/app/schemas/analytics.py +++ b/app/schemas/analytics.py @@ -1,4 +1,4 @@ -"""Schemas for calculated team hitting analytics. +"""Schemas for calculated team and player analytics. These models are the contract between the analytics layer and everything that presents it. They carry finished numbers, not raw MLB payloads, and they keep @@ -8,6 +8,9 @@ than through a shared metric type. They are read the same way but mean different things, and honest duplication is cheaper to follow than an abstraction covering four cases. + +Player models describe one stored season aggregate rather than a game-by-game +trend, so they carry no chart points, rolling windows, or league context. """ from __future__ import annotations @@ -18,6 +21,7 @@ from pydantic import BaseModel, ConfigDict, Field, model_validator from app.schemas.games import HomeAway +from app.schemas.players import PlayerSeasonHitting def _validate_prior_window_pair( @@ -1676,3 +1680,172 @@ def _comparison_is_internally_consistent(self) -> TeamHitsAllowedLeagueCompariso f"team_hits_allowed_per_game - league.hits_per_game ({expected})" ) return self + + +def _validate_optional_ratio( + name: str, + value: float | None, + *, + numerator: int, + denominator: int, +) -> None: + """Require a rate to equal its components, and be None only at zero. + + An undefined rate and a real ``.000`` rate mean different things, so a zero + denominator must produce None and a non-zero one must produce the ratio. + """ + if denominator == 0: + if value is not None: + raise ValueError(f"{name} must be None when its denominator is zero") + return + expected = numerator / denominator + if value is None or not isclose(value, expected, rel_tol=1e-9, abs_tol=1e-9): + raise ValueError( + f"{name} ({value}) must equal {numerator} / {denominator} ({expected})" + ) + + +class PlayerPlateAppearanceRates(BaseModel): + """Share of a player's plate appearances ending in three outcomes. + + All three rates share ``plate_appearances`` as their denominator, which is + why they are grouped: they describe how the player's stored season + outcomes were composed, not how good those outcomes were. The model only + exists when the player had at least one plate appearance. + """ + + model_config = ConfigDict(frozen=True, extra="forbid") + + plate_appearances: int = Field(gt=0, description="The shared denominator.") + strikeouts: int = Field(ge=0, description="Batting strikeouts.") + base_on_balls: int = Field(ge=0, description="Walks, including intentional.") + home_runs: int = Field(ge=0, description="Home runs.") + strikeout_rate: float = Field( + ge=0, le=1, description="K%: strikeouts / plate_appearances." + ) + walk_rate: float = Field( + ge=0, le=1, description="BB%: base_on_balls / plate_appearances." + ) + home_run_rate: float = Field( + ge=0, le=1, description="HR%: home_runs / plate_appearances." + ) + + @model_validator(mode="after") + def _rates_match_their_components(self) -> PlayerPlateAppearanceRates: + for name, numerator in ( + ("strikeout_rate", self.strikeouts), + ("walk_rate", self.base_on_balls), + ("home_run_rate", self.home_runs), + ): + _validate_optional_ratio( + name, + getattr(self, name), + numerator=numerator, + denominator=self.plate_appearances, + ) + return self + + +class PlayerHittingOverview(BaseModel): + """One player's stored season hitting aggregate with its derived rates. + + The stored season aggregate is carried unchanged beside the rates derived + from it, and the validators below prove the two agree. Each rate is None + exactly when its denominator is zero, never ``0.0``. OPS is None whenever + either OBP or SLG is. + + This is the season aggregate as currently stored, which for an in-progress + season is the total at the most recent import rather than a completed + season. It is not a trend, does not compare the player with MLB, and says + nothing about which club or clubs the player played for. + """ + + model_config = ConfigDict(frozen=True, extra="forbid") + + hitting: PlayerSeasonHitting + total_bases: int = Field(ge=0, description="H + 2B + 2 * 3B + 3 * HR.") + batting_average: float | None = Field( + ge=0, le=1, description="AVG: hits / at_bats, or None with no at-bats." + ) + on_base_percentage: float | None = Field( + ge=0, + le=1, + description="OBP: (H + BB + HBP) / (AB + BB + HBP + SF), or None when " + "that denominator is zero.", + ) + slugging_percentage: float | None = Field( + ge=0, le=4, description="SLG: total_bases / at_bats, or None with no at-bats." + ) + on_base_plus_slugging: float | None = Field( + ge=0, le=5, description="OPS: OBP + SLG, or None when either is None." + ) + plate_appearance_rates: PlayerPlateAppearanceRates | None = Field( + description="K%, BB%, and HR%, or None with no plate appearances." + ) + + @model_validator(mode="after") + def _rates_match_the_stored_line(self) -> PlayerHittingOverview: + line = self.hitting + expected_total_bases = ( + line.hits + line.doubles + 2 * line.triples + 3 * line.home_runs + ) + if self.total_bases != expected_total_bases: + raise ValueError( + f"total_bases ({self.total_bases}) must equal H + 2B + 2 * 3B + " + f"3 * HR ({expected_total_bases})" + ) + _validate_optional_ratio( + "batting_average", + self.batting_average, + numerator=line.hits, + denominator=line.at_bats, + ) + _validate_optional_ratio( + "on_base_percentage", + self.on_base_percentage, + numerator=line.hits + line.base_on_balls + line.hit_by_pitch, + denominator=( + line.at_bats + line.base_on_balls + line.hit_by_pitch + line.sac_flies + ), + ) + _validate_optional_ratio( + "slugging_percentage", + self.slugging_percentage, + numerator=self.total_bases, + denominator=line.at_bats, + ) + if self.on_base_percentage is None or self.slugging_percentage is None: + if self.on_base_plus_slugging is not None: + raise ValueError( + "on_base_plus_slugging must be None when OBP or SLG is None" + ) + else: + expected_ops = self.on_base_percentage + self.slugging_percentage + if self.on_base_plus_slugging is None or not isclose( + self.on_base_plus_slugging, expected_ops, rel_tol=1e-9, abs_tol=1e-9 + ): + raise ValueError( + f"on_base_plus_slugging ({self.on_base_plus_slugging}) must " + f"equal OBP + SLG ({expected_ops})" + ) + rates = self.plate_appearance_rates + if (rates is None) != (line.plate_appearances == 0): + raise ValueError( + "plate_appearance_rates must be present exactly when " + "plate_appearances is non-zero" + ) + if rates is not None and ( + rates.plate_appearances, + rates.strikeouts, + rates.base_on_balls, + rates.home_runs, + ) != ( + line.plate_appearances, + line.strikeouts, + line.base_on_balls, + line.home_runs, + ): + raise ValueError( + "plate_appearance_rates components must match the stored line" + ) + return self diff --git a/app/web/charts.py b/app/web/charts.py index 98bbb9c..ed6ef76 100644 --- a/app/web/charts.py +++ b/app/web/charts.py @@ -1,4 +1,4 @@ -"""Plotly figure construction for team analytics visualizations. +"""Plotly figure construction for team and player analytics visualizations. Kept out of the route so the figure contract can be tested without HTTP and so the route stays about request handling. @@ -18,6 +18,7 @@ from app.analytics.team_pitching import build_pitch_count_points from app.schemas.analytics import ( + PlayerPlateAppearanceRates, TeamBaserunnersAnalysis, TeamBaserunnersLeagueComparison, TeamHitsAllowedAnalysis, @@ -33,7 +34,12 @@ TeamStrikeoutsAnalysis, TeamStrikeoutsLeagueComparison, ) -from app.web.formatting import format_long_date, format_matchup, format_short_date +from app.web.formatting import ( + format_long_date, + format_matchup, + format_plate_appearance_rate, + format_short_date, +) CHART_DIV_ID = "team-hits-chart" RAW_HITS_TRACE_NAME = "Game Hits" @@ -80,6 +86,9 @@ NORMALIZED_BASELINE_TRACE_NAME = "Baseline (100)" COMPARISON_Y_AXIS_TITLE = "Normalized Index (MLB Avg = 100)" +PLAYER_PA_RATES_CHART_DIV_ID = "player-plate-appearance-rates-chart" +PLAYER_PA_RATES_X_AXIS_TITLE = "Percentage of Plate Appearances" + _NAVY = "#12263f" _TEAL = "#0f8b8d" # Distinct hue *and* distinct dash from the navy team line, so the two @@ -1117,6 +1126,86 @@ def build_team_hitting_comparison_figure( return figure +def build_player_plate_appearance_rates_figure( + rates: PlayerPlateAppearanceRates, +) -> go.Figure: + """Build the K%, BB%, and HR% bars for one player-season. + + The three bars share one denominator, plate appearances, so their lengths + are directly comparable. One colour is used for all three: a strikeout is + not styled as bad or a walk as good, and there is no MLB reference line. + The chart describes how the stored season's outcomes were composed. + """ + bars = ( + ("K%", "Strikeouts", rates.strikeouts, rates.strikeout_rate), + ("BB%", "Walks", rates.base_on_balls, rates.walk_rate), + ("HR%", "Home runs", rates.home_runs, rates.home_run_rate), + ) + labels = [label for label, _, _, _ in bars] + values = [value for _, _, _, value in bars] + hover_data = [ + (name, f"{count:,}", f"{rates.plate_appearances:,}") + for _, name, count, _ in bars + ] + + figure = go.Figure() + figure.add_trace( + go.Bar( + x=values, + y=labels, + orientation="h", + marker={"color": _TEAL}, + text=[format_plate_appearance_rate(value) for value in values], + textposition="outside", + cliponaxis=False, + textfont={"size": 13, "color": _NAVY}, + customdata=hover_data, + hovertemplate=( + "%{y}: %{x:.1%}
" + "%{customdata[0]}: %{customdata[1]} of " + "%{customdata[2]} PA" + ), + showlegend=False, + ) + ) + # Headroom for the outside value labels without implying a scale maximum. + upper = max(max(values) * 1.2, 0.05) + figure.update_layout( + template="plotly_white", + margin={"l": 8, "r": 16, "t": 8, "b": 8}, + height=240, + hovermode="closest", + paper_bgcolor="rgba(0,0,0,0)", + plot_bgcolor="rgba(0,0,0,0)", + font={"family": "system-ui, -apple-system, 'Segoe UI', sans-serif", "size": 13}, + bargap=0.35, + xaxis={ + "title": { + "text": PLAYER_PA_RATES_X_AXIS_TITLE, + "standoff": 10, + "font": _AXIS_TITLE_FONT, + }, + "tickfont": _TICK_FONT, + "tickformat": ".0%", + "range": [0, upper], + "gridcolor": _GRID, + "griddash": "dot", + "zeroline": False, + "showline": True, + "linecolor": _AXIS_LINE, + "automargin": True, + }, + yaxis={ + # K% first, reading top to bottom in the order the page lists them. + "autorange": "reversed", + "tickfont": {"size": 13, "color": _NAVY}, + "showgrid": False, + "automargin": True, + }, + ) + return figure + + def render_figure_html(figure: go.Figure, *, div_id: str = CHART_DIV_ID) -> str: """Render a figure as an embeddable div. diff --git a/app/web/formatting.py b/app/web/formatting.py index b1f065d..c80e2c1 100644 --- a/app/web/formatting.py +++ b/app/web/formatting.py @@ -4,6 +4,7 @@ from datetime import date from app.schemas.analytics import ( + PlayerHittingOverview, TeamBaserunnersAnalysis, TeamBaserunnersLeagueComparison, TeamHitsAllowedAnalysis, @@ -39,6 +40,9 @@ ) NORMALIZED_INDEX_CAPTION = "MLB Avg = 100" NO_LEAGUE_COMPARISON_VALUE = "—" +# A rate with a zero denominator. Its card caption always says why, so the dash +# cannot be read as a real ``.000``. +UNDEFINED_RATE_VALUE = "—" NO_LEAGUE_COMPARISON_CAPTION = "Comparison unavailable" LEAGUE_COMPARISON_UNAVAILABLE_NOTE = ( "MLB comparison unavailable. A complete league-season import is " @@ -576,6 +580,18 @@ def format_win_pct(value: float) -> str: or winless season is written ``1.000`` and ``.000``, so the leading digit is kept only when it is not a zero. """ + return _format_three_decimal_rate(value) + + +def format_batting_rate(value: float) -> str: + """Render AVG, OBP, SLG, or OPS the way a box score does: ``.300``. + + OPS routinely passes one and keeps its leading digit, as in ``1.024``. + """ + return _format_three_decimal_rate(value) + + +def _format_three_decimal_rate(value: float) -> str: rendered = f"{value:.3f}" return rendered[1:] if rendered.startswith("0.") else rendered @@ -855,3 +871,101 @@ def format_hits_allowed_direction_sentence( f"{team_name}'s pitchers allowed {abs(difference):.2f} {direction} hits " f"per game than MLB overall across the stored season." ) + + +def format_plate_appearance_rate(value: float) -> str: + """Render a share of plate appearances, such as K%, to one decimal.""" + return f"{value:.1%}" + + +def build_player_hitting_rate_cards( + overview: PlayerHittingOverview, +) -> list[SummaryCard]: + """Build the AVG, OBP, SLG, and OPS cards for one player-season. + + An undefined rate is shown as ``—`` with a caption naming the missing + denominator, never as ``.000``. + """ + line = overview.hitting + at_bats_caption = ( + f"{line.hits:,} H in {line.at_bats:,} AB" + if overview.batting_average is not None + else "Undefined: no at-bats" + ) + return [ + SummaryCard( + label="AVG", + value=_format_optional_batting_rate(overview.batting_average), + caption=at_bats_caption, + ), + SummaryCard( + label="OBP", + value=_format_optional_batting_rate(overview.on_base_percentage), + caption=( + "On-base percentage" + if overview.on_base_percentage is not None + else "Undefined: no AB, BB, HBP, or SF" + ), + ), + SummaryCard( + label="SLG", + value=_format_optional_batting_rate(overview.slugging_percentage), + caption=( + f"{overview.total_bases:,} TB in {line.at_bats:,} AB" + if overview.slugging_percentage is not None + else "Undefined: no at-bats" + ), + ), + SummaryCard( + label="OPS", + value=_format_optional_batting_rate(overview.on_base_plus_slugging), + caption=( + "OBP + SLG" + if overview.on_base_plus_slugging is not None + else "Undefined: needs OBP and SLG" + ), + ), + ] + + +def build_player_hitting_total_cards( + overview: PlayerHittingOverview, +) -> list[SummaryCard]: + """Build the season counting-stat cards, exactly as stored.""" + line = overview.hitting + return [ + SummaryCard( + label="Games", + value=f"{line.games_played:,}", + caption="Games played", + ), + SummaryCard( + label="Plate Appearances", + value=f"{line.plate_appearances:,}", + caption=f"{line.at_bats:,} at-bats", + ), + SummaryCard( + label="Home Runs", + value=f"{line.home_runs:,}", + caption="Season total", + ), + SummaryCard( + label="Walks", + value=f"{line.base_on_balls:,}", + caption=f"Includes {line.intentional_walks:,} intentional", + ), + SummaryCard( + label="Strikeouts", + value=f"{line.strikeouts:,}", + caption="Batting strikeouts", + ), + SummaryCard( + label="Stolen Bases", + value=f"{line.stolen_bases:,}", + caption=f"{line.caught_stealing:,} caught stealing", + ), + ] + + +def _format_optional_batting_rate(value: float | None) -> str: + return UNDEFINED_RATE_VALUE if value is None else format_batting_rate(value) diff --git a/app/web/player_routes.py b/app/web/player_routes.py index 1f2aeeb..8fb6978 100644 --- a/app/web/player_routes.py +++ b/app/web/player_routes.py @@ -1,29 +1,64 @@ -"""DB-only Player directory, search, and season-scoped selection.""" +"""DB-only Player directory, selection, and season hitting overview.""" from typing import Annotated from urllib.parse import urlencode from fastapi import APIRouter, Depends, Query, Request -from fastapi.responses import HTMLResponse +from fastapi.responses import HTMLResponse, Response from fastapi.templating import Jinja2Templates from sqlalchemy.orm import Session +from app.analytics.player_hitting import ( + PlayerHittingAnalysisError, + build_player_hitting_overview, +) from app.config import Settings from app.database.repositories import ( MIGRATION_HINT, DatabaseSchemaMissingError, + get_player_catalog_entry, + get_player_season_hitting, list_player_catalog, list_player_catalog_seasons, ) from app.schemas.players import PlayerSeasonCatalogEntry, normalize_player_name +from app.web.charts import ( + PLAYER_PA_RATES_CHART_DIV_ID, + build_player_plate_appearance_rates_figure, + render_figure_html, +) from app.web.dependencies import get_db_session -from app.web.routes import MLB_LOGO_URL +from app.web.formatting import ( + build_player_hitting_rate_cards, + build_player_hitting_total_cards, +) +from app.web.routes import MLB_LOGO_URL, PLOTLY_BUNDLE_PATH # A fixed cap keeps broad searches readable without a pagination framework. PLAYER_SEARCH_LIMIT = 50 PLAYER_CATALOG_IMPORT_COMMAND = ( "poetry run python scripts/import_player_catalog.py --season {season}" ) +PLAYER_SEASON_IMPORT_COMMAND = ( + "poetry run python scripts/import_player_season.py " + "--player-id {player_id} --season {season}" +) +PLAYER_DIRECTORY_PATH = "/players" +PLAYER_HITTING_PATH = "/players/hitting" + + +def player_hitting_url(*, season: int, player_id: int) -> str: + """Return the shareable hitting overview URL for one Player-season.""" + return f"{PLAYER_HITTING_PATH}?" + urlencode( + {"season": season, "player_id": player_id} + ) + + +def player_selection_url(*, season: int, player_id: int) -> str: + """Return the directory URL with this Player-season already selected.""" + return f"{PLAYER_DIRECTORY_PATH}?" + urlencode( + {"season": season, "player_id": player_id} + ) def search_player_catalog( @@ -45,10 +80,10 @@ def search_player_catalog( def create_player_router(templates: Jinja2Templates, settings: Settings) -> APIRouter: - """Build the one real Player-domain destination.""" + """Build the Player directory and the Player season hitting overview.""" router = APIRouter() - @router.get("/players", response_class=HTMLResponse) + @router.get(PLAYER_DIRECTORY_PATH, response_class=HTMLResponse) def player_directory( request: Request, session: Annotated[Session, Depends(get_db_session)], @@ -86,6 +121,18 @@ def player_directory( ) status = 404 + # A local read only, to decide whether a real overview link exists. + # The directory itself never renders the stats. + hitting_overview_url = None + if selected_player is not None and season is not None: + hitting = get_player_season_hitting( + session, player_id=selected_player.player_id, season=season + ) + if hitting is not None: + hitting_overview_url = player_hitting_url( + season=season, player_id=selected_player.player_id + ) + query = q.strip() matches = search_player_catalog(catalog, query) results = matches[:PLAYER_SEARCH_LIMIT] @@ -109,6 +156,14 @@ def player_directory( "result_count": len(matches), "result_limit": PLAYER_SEARCH_LIMIT, "selected_player": selected_player, + "hitting_overview_url": hitting_overview_url, + "player_season_import_command": ( + PLAYER_SEASON_IMPORT_COMMAND.format( + player_id=selected_player.player_id, season=season + ) + if selected_player is not None + else None + ), "message": message, "schema_missing": schema_missing, "migration_command": MIGRATION_HINT, @@ -119,4 +174,80 @@ def player_directory( status_code=status, ) + @router.get(PLAYER_HITTING_PATH, response_class=HTMLResponse) + def player_hitting( + request: Request, + session: Annotated[Session, Depends(get_db_session)], + season: Annotated[int, Query(gt=0)], + player_id: Annotated[int, Query(gt=0)], + ) -> Response: + """Render one Player's stored season hitting line from the database only.""" + context: dict[str, object] = { + "app_name": settings.app_name, + "mlb_logo_url": MLB_LOGO_URL, + "season": season, + "player": None, + "directory_url": PLAYER_DIRECTORY_PATH, + } + + def render(state: str, status_code: int = 200) -> Response: + context["state"] = state + return templates.TemplateResponse( + request=request, + name="player_hitting.html", + context=context, + status_code=status_code, + ) + + try: + player = get_player_catalog_entry( + session, player_id=player_id, season=season + ) + except DatabaseSchemaMissingError as exc: + context["message"] = str(exc) + context["migration_command"] = MIGRATION_HINT + return render("schema_missing", 503) + + # Membership comes from the season's catalog, never from a stored + # identity or a stored hitting row alone. + if player is None: + context["message"] = ( + f"Player {player_id} is not in the locally stored {season} " + "Player catalog." + ) + return render("not_found", 404) + + context["player"] = player + context["directory_url"] = player_selection_url( + season=season, player_id=player_id + ) + context["import_command"] = PLAYER_SEASON_IMPORT_COMMAND.format( + player_id=player_id, season=season + ) + + hitting = get_player_season_hitting(session, player_id=player_id, season=season) + if hitting is None: + # The selection is valid; only the analytics are unavailable, like + # Team comparison without league data. Not zeroes, not an MLB call. + return render("missing_hitting") + + try: + overview = build_player_hitting_overview(hitting) + except PlayerHittingAnalysisError as exc: + context["message"] = str(exc) + return render("inconsistent_hitting", 409) + + context["overview"] = overview + context["rate_cards"] = build_player_hitting_rate_cards(overview) + context["total_cards"] = build_player_hitting_total_cards(overview) + if overview.plate_appearance_rates is not None: + context["plotly_bundle_path"] = PLOTLY_BUNDLE_PATH + context["chart_html"] = render_figure_html( + build_player_plate_appearance_rates_figure( + overview.plate_appearance_rates + ), + div_id=PLAYER_PA_RATES_CHART_DIV_ID, + ) + return render("ok") + return router diff --git a/app/web/static/css/app.css b/app/web/static/css/app.css index 4b62f62..50d3d91 100644 --- a/app/web/static/css/app.css +++ b/app/web/static/css/app.css @@ -415,6 +415,33 @@ body { font-size: 0.85rem; } +/* Player hitting overview */ + +.player-back { + margin: 0; +} + +.player-link { + color: var(--teal-dark); + font-weight: 600; +} + +.player-link:hover { + color: var(--teal); +} + +.player-link:focus-visible { + outline: 2px solid var(--teal); + outline-offset: 2px; + border-radius: 2px; +} + +.player-section-heading { + margin: 0.25rem 0 -0.5rem; + font-size: 1.1rem; + color: var(--navy); +} + /* Chart */ .chart-card { @@ -457,7 +484,8 @@ body { /* The normalized chart is intentionally composed for a phone-sized plot. Its three aggregate traces stay legible without forcing the internal horizontal scroller used by the denser game-level charts. */ -.chart-card__figure--comparison .plotly-graph-div { +.chart-card__figure--comparison .plotly-graph-div, +.chart-card__figure--player-rates .plotly-graph-div { min-width: 0; } @@ -469,6 +497,10 @@ body { gap: 1rem; } +.summary--player-totals { + grid-template-columns: repeat(3, minmax(0, 1fr)); +} + .summary-card { padding: 1.1rem 1.25rem 1.15rem; } @@ -687,6 +719,11 @@ body { grid-template-columns: minmax(0, 1fr); } + /* Short counts and three-digit rates fit two to a row on a phone. */ + .summary--player-totals { + grid-template-columns: repeat(2, minmax(0, 1fr)); + } + /* The comparison values are compact indexes. Keeping them in a two-by-two grid preserves the reference layout's visual hierarchy on a phone without making any card too narrow to read. */ diff --git a/app/web/templates/player_base.html b/app/web/templates/player_base.html new file mode 100644 index 0000000..4cd2dae --- /dev/null +++ b/app/web/templates/player_base.html @@ -0,0 +1,9 @@ +{% extends "base.html" %} + +{# Players is the current domain on every Player page. There is no secondary + Player navigation: each Player page needs a selected Player-season, so a + global link to it would have nowhere real to go. #} +{% block primary_navigation %} + Teams + Players +{% endblock %} diff --git a/app/web/templates/player_hitting.html b/app/web/templates/player_hitting.html new file mode 100644 index 0000000..2088099 --- /dev/null +++ b/app/web/templates/player_hitting.html @@ -0,0 +1,134 @@ +{% extends "player_base.html" %} + +{% block title %} + {%- if player -%} + {{ player.full_name }} — {{ season }} Hitting + {%- else -%} + Player Hitting Overview + {%- endif %} — {{ app_name }} +{% endblock %} + +{% block head %} + {# plotly.js must be parsed before the figure div's inline bootstrap script. #} + {% if chart_html %} + + {% endif %} +{% endblock %} + +{% block content %} +

+ ← Back to Player Directory +

+ +
+ {% if player %} +

{{ player.full_name }} — {{ season }} Hitting

+

+ Season overview: the stored {{ season }} season aggregate, not a + game-by-game trend. +

+ {% else %} +

Player Hitting Overview

+

A season-level hitting line for one stored Player.

+ {% endif %} +
+ + {% if state == "schema_missing" %} +
+

The database schema is not ready

+

{{ message }}

+
{{ migration_command }}
+
+ {% elif state == "not_found" %} +
+

That Player selection is not stored locally

+

{{ message }}

+

Find a Player in a stored season from the Player Directory.

+
+ {% elif state == "missing_hitting" %} +
+

No {{ season }} hitting line is stored for {{ player.full_name }}

+

+ {{ player.full_name }} is in the stored {{ season }} Player catalog, but + their season hitting statistics have not been imported. Nothing is + shown rather than treating the missing line as zero production. +

+

Import it from the command line, then reload this page:

+
{{ import_command }}
+
+ {% elif state == "inconsistent_hitting" %} +
+

The stored {{ season }} hitting line cannot be summarized

+

{{ message }}

+

Re-import the season line to replace it:

+
{{ import_command }}
+
+ {% else %} +

Rate Stats

+ {% with summary_cards=rate_cards, summary_label="Season rate statistics" %} + {% include "_summary_cards.html" %} + {% endwith %} + +

Season Totals

+ {% with summary_cards=total_cards, summary_class="summary summary--player-totals", summary_label="Season counting statistics" %} + {% include "_summary_cards.html" %} + {% endwith %} + + {% if chart_html %} +
+
+

Plate Appearance Rate Profile

+

+ K%, BB%, and HR% as percentages of {{ "{:,}".format(overview.hitting.plate_appearances) }} plate appearances +

+
+
{{ chart_html | safe }}
+
+ {% else %} +
+

Plate Appearance Rate Profile

+

K%, BB%, and HR% are undefined: no plate appearances are stored for this season.

+
+ {% endif %} + +
+ +
+

About this overview

+

+ These figures describe {{ player.full_name }}'s {{ season }} season + aggregate currently stored locally. For an in-progress season, they + reflect the data available at the most recent import and do not imply + the season is complete. +

+

+ If the player appeared for more than one club, the stored line is the + combined season aggregate; this page does not model the individual + team stints. +

+

+ AVG is hits per at-bat. OBP is (H + BB + HBP) / (AB + BB + HBP + SF). + SLG is total bases per at-bat, and OPS is OBP + SLG. A rate shown as + — is undefined because its denominator is zero, which is not the + same as .000. +

+

+ K%, BB%, and HR% each divide by plate appearances, so the bars + describe how the player's plate appearances ended. They are not a + ranking or a quality score, and nothing here is compared with MLB. +

+

Primary position

+

+ {{ player.primary_position }}. This is the stored identity value, not + a historical position for {{ season }}. +

+
+
+ {% endif %} +{% endblock %} diff --git a/app/web/templates/players.html b/app/web/templates/players.html index cfcbbc8..5e75823 100644 --- a/app/web/templates/players.html +++ b/app/web/templates/players.html @@ -1,12 +1,7 @@ -{% extends "base.html" %} +{% extends "player_base.html" %} {% block title %}Player Directory — {{ app_name }}{% endblock %} -{% block primary_navigation %} - Teams - Players -{% endblock %} - {% block content %}

Player Directory

@@ -67,6 +62,12 @@

Selected Player

You selected {{ selected_player.full_name }} for the {{ season }} catalog.

Primary position: {{ selected_player.primary_position }}

Position is the stored identity value, not a historical position for this season.

+ {% if hitting_overview_url %} +

View Hitting Overview

+ {% else %} +

No {{ season }} hitting line is stored locally for this Player. Import it with:

+
{{ player_season_import_command }}
+ {% endif %}
{% endif %} diff --git a/docs/ui-information-architecture.md b/docs/ui-information-architecture.md index b4609ed..cfa861b 100644 --- a/docs/ui-information-architecture.md +++ b/docs/ui-information-architecture.md @@ -17,8 +17,9 @@ All existing URLs remain canonical, with no aliases or redirects. `base.html` owns the entity-neutral shell, primary navigation, skip link, main landmark, and footer. `team_base.html` supplies the active Team domain and grouped navigation through `_team_navigation.html`. Individual metric templates retain -selectors, charts, interpretation, and recovery states. `players.html` extends -the same shell directly, with its own season/search form and no secondary nav. +selectors, charts, interpretation, and recovery states. Player pages extend +`player_base.html`, which only marks Players as the current domain; there is no +secondary Player navigation (see below). Team metric pages include `_team_selector_form.html` for their Team, season, and rolling-window GET controls. It relies on the Team-season catalog and stays @@ -50,6 +51,39 @@ Submitting the season/search form clears `player_id`. Selection confirms only the stored name, primary position, and season membership; identity fields are not historical season attributes. Browser requests read the database only. -Player metrics remain intentionally absent. Player charts and additional routes -will be added only when a real metric is chosen. Team URLs, metric navigation, -and Team query semantics remain unchanged. +The directory remains selection infrastructure and renders no Player stats. When +the selected Player has a stored `player_season_hitting` row for that season, it +links to the hitting overview; otherwise it shows the real import command. That +existence check is a local repository read, not an MLB call. + +## Player hitting overview + +Issue #57 adds the first Player analytics page: + +```text +/players/hitting?season=&player_id= +``` + +It answers *what did this player's overall offensive season look like?* from +one stored season aggregate in `player_season_hitting`. Because an in-progress +season can be re-imported as its totals grow, the page describes the aggregate +as of the most recent import and never calls the season complete. It is not a +game-by-game trend, a projection, or a comparison with MLB; none of those exist +for Players yet. AVG, OBP, SLG, OPS, and the K%/BB%/HR% plate-appearance rate +profile are derived in `app/analytics/player_hitting.py` and never persisted. + +Both parameters are required positive integers. Membership comes from +`player_seasons` only: a global identity or a hitting row without catalog +membership returns 404. A member with no stored hitting line returns a 200 +data-unavailable state with the `import_player_season.py` command, because the +selection is valid and only the analytics are absent; nothing is shown as zero. +A rate with a zero denominator renders as `—` with its reason, never `.000`. + +A multi-club season is one combined aggregate. No historical team-stint model +exists, so the page names no club. Primary position is the stored identity value, not a +historical season attribute. Browser requests remain DB-only. + +The page links back to `/players?season=…&player_id=…` with that Player still +selected. Because each Player page needs a selected Player-season, a global +"Hitting" nav item would have no real destination, so none exists. Team URLs, +metric navigation, and Team query semantics remain unchanged. diff --git a/tests/test_analytics_player_hitting.py b/tests/test_analytics_player_hitting.py new file mode 100644 index 0000000..6421fe9 --- /dev/null +++ b/tests/test_analytics_player_hitting.py @@ -0,0 +1,247 @@ +"""Tests for season-level Player hitting rates. + +Expected values are worked by hand from the stored components so each formula +is checked independently of the implementation. The zero-denominator cases +hold the rule this module exists for: an undefined rate is None, never 0.0. +""" + +import pytest +from pydantic import ValidationError + +from app.analytics.player_hitting import ( + PlayerHittingAnalysisError, + build_player_hitting_overview, +) +from app.schemas.analytics import PlayerHittingOverview, PlayerPlateAppearanceRates +from tests.test_repositories_players import make_hitting + +NO_PLATE_APPEARANCES = { + "games_played": 1, + "plate_appearances": 0, + "at_bats": 0, + "runs": 0, + "hits": 0, + "doubles": 0, + "triples": 0, + "home_runs": 0, + "rbi": 0, + "base_on_balls": 0, + "intentional_walks": 0, + "hit_by_pitch": 0, + "strikeouts": 0, + "stolen_bases": 0, + "caught_stealing": 0, + "sac_flies": 0, + "sac_bunts": 0, +} + + +class TestFormulas: + """make_hitting: 600 PA, 500 AB, 150 H (30 2B, 3 3B, 20 HR), 60 BB, + 5 HBP, 4 SF, 2 SH, 100 SO.""" + + def test_total_bases_counts_every_base(self) -> None: + # 97 singles + 2 * 30 + 3 * 3 + 4 * 20 = 97 + 60 + 9 + 80 = 246. + overview = build_player_hitting_overview(make_hitting()) + assert overview.total_bases == 246 + + def test_batting_average_is_hits_per_at_bat(self) -> None: + overview = build_player_hitting_overview(make_hitting()) + assert overview.batting_average == pytest.approx(150 / 500) + + def test_on_base_percentage_uses_the_standard_denominator(self) -> None: + # (150 + 60 + 5) / (500 + 60 + 5 + 4) = 215 / 569. + overview = build_player_hitting_overview(make_hitting()) + assert overview.on_base_percentage == pytest.approx(215 / 569) + + def test_sacrifice_bunts_are_not_in_the_obp_denominator(self) -> None: + few = build_player_hitting_overview(make_hitting(sac_bunts=0)) + many = build_player_hitting_overview(make_hitting(sac_bunts=20)) + assert few.on_base_percentage == many.on_base_percentage + + def test_slugging_is_total_bases_per_at_bat(self) -> None: + overview = build_player_hitting_overview(make_hitting()) + assert overview.slugging_percentage == pytest.approx(246 / 500) + + def test_ops_is_obp_plus_slg(self) -> None: + overview = build_player_hitting_overview(make_hitting()) + assert overview.on_base_plus_slugging == pytest.approx(215 / 569 + 246 / 500) + + def test_rates_keep_full_precision(self) -> None: + overview = build_player_hitting_overview(make_hitting()) + assert overview.on_base_percentage == 215 / 569 + assert round(overview.on_base_percentage, 3) != overview.on_base_percentage + + def test_plate_appearance_rates_share_one_denominator(self) -> None: + rates = build_player_hitting_overview(make_hitting()).plate_appearance_rates + assert rates is not None + assert rates.plate_appearances == 600 + assert rates.strikeout_rate == pytest.approx(100 / 600) + assert rates.walk_rate == pytest.approx(60 / 600) + assert rates.home_run_rate == pytest.approx(20 / 600) + + def test_stored_line_is_carried_unchanged(self) -> None: + hitting = make_hitting() + assert build_player_hitting_overview(hitting).hitting == hitting + + +class TestUndefinedRates: + def test_no_plate_appearances_leaves_every_rate_undefined(self) -> None: + overview = build_player_hitting_overview(make_hitting(**NO_PLATE_APPEARANCES)) + assert overview.total_bases == 0 + assert overview.batting_average is None + assert overview.on_base_percentage is None + assert overview.slugging_percentage is None + assert overview.on_base_plus_slugging is None + assert overview.plate_appearance_rates is None + + def test_walks_without_at_bats_define_obp_but_not_avg_slg_or_ops(self) -> None: + overview = build_player_hitting_overview( + make_hitting( + **{ + **NO_PLATE_APPEARANCES, + "plate_appearances": 3, + "base_on_balls": 2, + "hit_by_pitch": 1, + } + ) + ) + assert overview.batting_average is None + assert overview.slugging_percentage is None + assert overview.on_base_percentage == pytest.approx(1.0) + assert overview.on_base_plus_slugging is None + assert overview.plate_appearance_rates is not None + assert overview.plate_appearance_rates.walk_rate == pytest.approx(2 / 3) + + def test_a_lone_sacrifice_fly_is_a_real_zero_obp(self) -> None: + overview = build_player_hitting_overview( + make_hitting( + **{**NO_PLATE_APPEARANCES, "plate_appearances": 1, "sac_flies": 1} + ) + ) + assert overview.on_base_percentage == 0.0 + assert overview.on_base_percentage is not None + assert overview.batting_average is None + + def test_hitless_at_bats_are_a_real_zero_not_undefined(self) -> None: + overview = build_player_hitting_overview( + make_hitting( + **{ + **NO_PLATE_APPEARANCES, + "plate_appearances": 4, + "at_bats": 4, + "strikeouts": 4, + } + ) + ) + assert overview.batting_average == 0.0 + assert overview.slugging_percentage == 0.0 + assert overview.on_base_percentage == 0.0 + assert overview.on_base_plus_slugging == 0.0 + assert overview.plate_appearance_rates is not None + assert overview.plate_appearance_rates.strikeout_rate == 1.0 + assert overview.plate_appearance_rates.home_run_rate == 0.0 + + +def test_more_hits_than_at_bats_is_a_data_integrity_error() -> None: + hitting = make_hitting( + **{**NO_PLATE_APPEARANCES, "plate_appearances": 10, "at_bats": 3, "hits": 5} + ) + with pytest.raises(PlayerHittingAnalysisError, match="5 hits in 3 at-bats"): + build_player_hitting_overview(hitting) + + +@pytest.mark.parametrize( + ("overrides", "message"), + [ + ( + {"strikeouts": 601}, + "Player 677594's stored 2025 hitting line records 601 strikeouts in " + "600 plate appearances; strikeouts cannot exceed plate appearances", + ), + ( + {"base_on_balls": 601}, + "records 601 walks in 600 plate appearances; walks cannot exceed " + "plate appearances", + ), + ( + # The schema requires home runs <= hits, so the row is also over + # at-bats; the plate-appearance bound is still the one reported. + { + **NO_PLATE_APPEARANCES, + "plate_appearances": 2, + "at_bats": 2, + "hits": 3, + "home_runs": 3, + }, + "records 3 home runs in 2 plate appearances; home runs cannot exceed " + "plate appearances", + ), + ( + {"strikeouts": 501}, + "records 501 strikeouts in 500 at-bats; strikeouts cannot exceed at-bats", + ), + ], + ids=["so-over-pa", "bb-over-pa", "hr-over-pa", "so-over-ab"], +) +def test_counts_over_their_denominator_are_data_integrity_errors( + overrides: dict[str, int], message: str +) -> None: + """A corrupted row must raise the analytics error the route handles, + never a schema ValidationError from building the rate models.""" + hitting = make_hitting(**overrides) + with pytest.raises(PlayerHittingAnalysisError) as raised: + build_player_hitting_overview(hitting) + assert message in str(raised.value) + + +def test_rate_schema_remains_a_second_integrity_boundary() -> None: + with pytest.raises(ValidationError, match="strikeout_rate"): + PlayerPlateAppearanceRates( + plate_appearances=600, + strikeouts=601, + base_on_balls=0, + home_runs=0, + strikeout_rate=601 / 600, + walk_rate=0.0, + home_run_rate=0.0, + ) + + +class TestOverviewSchema: + def _dump(self, **overrides: object) -> dict[str, object]: + dumped = build_player_hitting_overview(make_hitting()).model_dump() + dumped.update(overrides) + return dumped + + def test_rejects_a_drifted_rate(self) -> None: + with pytest.raises(ValidationError, match="batting_average"): + PlayerHittingOverview.model_validate(self._dump(batting_average=0.301)) + + def test_rejects_wrong_total_bases(self) -> None: + with pytest.raises(ValidationError, match="total_bases"): + PlayerHittingOverview.model_validate(self._dump(total_bases=245)) + + def test_rejects_zero_in_place_of_an_undefined_rate(self) -> None: + dumped = build_player_hitting_overview( + make_hitting(**NO_PLATE_APPEARANCES) + ).model_dump() + dumped["batting_average"] = 0.0 + with pytest.raises(ValidationError, match="must be None"): + PlayerHittingOverview.model_validate(dumped) + + def test_rejects_ops_without_both_components(self) -> None: + dumped = build_player_hitting_overview( + make_hitting( + **{**NO_PLATE_APPEARANCES, "plate_appearances": 1, "base_on_balls": 1} + ) + ).model_dump() + dumped["on_base_plus_slugging"] = 1.0 + with pytest.raises(ValidationError, match="on_base_plus_slugging"): + PlayerHittingOverview.model_validate(dumped) + + def test_rejects_rates_detached_from_the_line(self) -> None: + dumped = self._dump() + dumped["plate_appearance_rates"] = None + with pytest.raises(ValidationError, match="plate_appearance_rates"): + PlayerHittingOverview.model_validate(dumped) diff --git a/tests/test_charts_player_hitting.py b/tests/test_charts_player_hitting.py new file mode 100644 index 0000000..5ff479b --- /dev/null +++ b/tests/test_charts_player_hitting.py @@ -0,0 +1,38 @@ +"""The Player plate-appearance rate profile figure contract.""" + +import plotly.graph_objects as go +import pytest + +from app.analytics.player_hitting import build_player_hitting_overview +from app.web.charts import ( + PLAYER_PA_RATES_X_AXIS_TITLE, + build_player_plate_appearance_rates_figure, +) +from tests.test_repositories_players import make_hitting + + +def _figure() -> go.Figure: + rates = build_player_hitting_overview(make_hitting()).plate_appearance_rates + assert rates is not None + return build_player_plate_appearance_rates_figure(rates) + + +def test_three_bars_share_the_plate_appearance_denominator() -> None: + figure = _figure() + assert len(figure.data) == 1 + bars = figure.data[0] + assert bars.type == "bar" + assert list(bars.y) == ["K%", "BB%", "HR%"] + assert list(bars.x) == pytest.approx([100 / 600, 60 / 600, 20 / 600]) + assert list(bars.text) == ["16.7%", "10.0%", "3.3%"] + assert figure.layout.xaxis.title.text == PLAYER_PA_RATES_X_AXIS_TITLE + assert figure.layout.xaxis.tickformat == ".0%" + + +def test_no_quality_colouring_or_reference_lines() -> None: + figure = _figure() + # One colour for every bar: a strikeout is not styled as worse than a walk. + assert isinstance(figure.data[0].marker.color, str) + assert not figure.layout.shapes + assert not figure.layout.annotations + assert figure.layout.xaxis.range[0] == 0 diff --git a/tests/test_formatting_player_hitting.py b/tests/test_formatting_player_hitting.py new file mode 100644 index 0000000..f3d3e58 --- /dev/null +++ b/tests/test_formatting_player_hitting.py @@ -0,0 +1,72 @@ +"""Presentation of Player season hitting rates and totals.""" + +import pytest + +from app.analytics.player_hitting import build_player_hitting_overview +from app.web.formatting import ( + UNDEFINED_RATE_VALUE, + build_player_hitting_rate_cards, + build_player_hitting_total_cards, + format_batting_rate, + format_plate_appearance_rate, +) +from tests.test_analytics_player_hitting import NO_PLATE_APPEARANCES +from tests.test_repositories_players import make_hitting + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + (0.3, ".300"), + (215 / 569, ".378"), + (0.0, ".000"), + (1.0243, "1.024"), + (0.9996, "1.000"), + ], +) +def test_batting_rates_use_box_score_notation(value: float, expected: str) -> None: + assert format_batting_rate(value) == expected + + +@pytest.mark.parametrize( + ("value", "expected"), [(100 / 600, "16.7%"), (0.1, "10.0%"), (0.0, "0.0%")] +) +def test_plate_appearance_rates_are_percentages(value: float, expected: str) -> None: + assert format_plate_appearance_rate(value) == expected + + +def test_rate_cards_round_only_for_display() -> None: + overview = build_player_hitting_overview(make_hitting()) + cards = build_player_hitting_rate_cards(overview) + assert [(card.label, card.value) for card in cards] == [ + ("AVG", ".300"), + ("OBP", ".378"), + ("SLG", ".492"), + ("OPS", ".870"), + ] + assert cards[0].caption == "150 H in 500 AB" + assert cards[2].caption == "246 TB in 500 AB" + + +def test_undefined_rates_render_as_explained_dashes_not_zero() -> None: + overview = build_player_hitting_overview(make_hitting(**NO_PLATE_APPEARANCES)) + cards = build_player_hitting_rate_cards(overview) + assert {card.value for card in cards} == {UNDEFINED_RATE_VALUE} + assert all(card.caption.startswith("Undefined:") for card in cards) + + +def test_total_cards_show_stored_counts() -> None: + overview = build_player_hitting_overview( + make_hitting(plate_appearances=1_200, at_bats=1_050) + ) + cards = build_player_hitting_total_cards(overview) + assert [(card.label, card.value) for card in cards] == [ + ("Games", "150"), + ("Plate Appearances", "1,200"), + ("Home Runs", "20"), + ("Walks", "60"), + ("Strikeouts", "100"), + ("Stolen Bases", "10"), + ] + assert cards[3].caption == "Includes 5 intentional" + assert cards[5].caption == "3 caught stealing" diff --git a/tests/test_repositories_player_catalog.py b/tests/test_repositories_player_catalog.py index 9047aac..9d2c44e 100644 --- a/tests/test_repositories_player_catalog.py +++ b/tests/test_repositories_player_catalog.py @@ -1,8 +1,14 @@ """Tests for season-level Player catalog repository behavior.""" +from pathlib import Path + +import pytest from sqlalchemy.orm import Session +from app.database.engine import build_engine, build_session_factory from app.database.repositories import ( + DatabaseSchemaMissingError, + get_player_catalog_entry, list_player_catalog, list_player_catalog_seasons, upsert_player, @@ -126,3 +132,41 @@ def test_hitting_rows_and_global_identities_do_not_supply_catalog_seasons( upsert_player_catalog_entry(migrated_session, entry=entry(season=2001)) migrated_session.commit() assert list_player_catalog_seasons(migrated_session) == [2001] + + +def test_catalog_entry_is_one_season_membership(migrated_session: Session) -> None: + upsert_player_catalog_entry(migrated_session, entry=entry(season=2003)) + upsert_player_catalog_entry(migrated_session, entry=entry(2, "Other", season=1999)) + migrated_session.commit() + assert get_player_catalog_entry( + migrated_session, player_id=677594, season=2003 + ) == entry(season=2003) + assert ( + get_player_catalog_entry(migrated_session, player_id=677594, season=1999) + is None + ) + assert get_player_catalog_entry(migrated_session, player_id=5, season=2003) is None + + +def test_identity_and_hitting_row_do_not_make_a_catalog_entry( + migrated_session: Session, +) -> None: + upsert_player(migrated_session, identity=make_identity()) + upsert_player_season_hitting(migrated_session, hitting=make_hitting()) + migrated_session.commit() + assert ( + get_player_catalog_entry(migrated_session, player_id=677594, season=2025) + is None + ) + + +def test_catalog_entry_reports_missing_schema(tmp_path: Path) -> None: + engine = build_engine(f"sqlite:///{tmp_path / 'unmigrated.db'}") + try: + with ( + build_session_factory(engine)() as session, + pytest.raises(DatabaseSchemaMissingError, match="alembic upgrade"), + ): + get_player_catalog_entry(session, player_id=1, season=2025) + finally: + engine.dispose() diff --git a/tests/test_web_players.py b/tests/test_web_players.py index b8a2634..6076412 100644 --- a/tests/test_web_players.py +++ b/tests/test_web_players.py @@ -1,5 +1,10 @@ -"""Player discovery stays local, season-scoped, accessible, and stats-free.""" +"""Player pages stay local, season-scoped, and accessible. +The directory selects Players and never renders stats; the hitting overview +renders one stored season line only after catalog membership is confirmed. +""" + +import re from collections.abc import Iterator from html import unescape from pathlib import Path @@ -25,7 +30,12 @@ @pytest.fixture(autouse=True) def forbid_mlb(monkeypatch: pytest.MonkeyPatch) -> None: - """Fail every Player browser state immediately on network/ingestion calls.""" + """Fail every Player browser state immediately on network/ingestion calls. + + ``app.services.players.get_player_season_hitting`` normalizes an MLB + response and is forbidden. The repository function of the same name is a + local read that both Player pages are allowed to make, so it is not here. + """ def fail(*args: object, **kwargs: object) -> None: raise AssertionError("Player browser requests must read the database only") @@ -42,7 +52,6 @@ def fail(*args: object, **kwargs: object) -> None: "app.services.team_game_logs.get_team_game_batting_lines", "app.services.league_teams.discover_mlb_teams", "app.services.league_season_ingestion.ingest_league_season", - "app.database.repositories.get_player_season_hitting", ): monkeypatch.setattr(target, fail) @@ -201,10 +210,16 @@ def test_result_link_is_shareable_and_selection_is_identity_only( 'name="team_id"', 'name="window"', 'name="player_id"', - "/players/hitting", "/players/compare", + "import_player_season.py", ): assert absent not in selected.text + # The one deliberate addition: a real link, because hitting is stored. + assert selected.text.count("/players/hitting") == 1 + assert ( + 'View Hitting Overview' + ) in selected.text # Search form submits only season and q, intentionally clearing selection. assert 'method="get" action="/players"' in selected.text assert '' in selected.text @@ -303,3 +318,333 @@ def override_session() -> Iterator[Session]: assert_player_navigation(response.text) finally: engine.dispose() + + +# --------------------------------------------------------------------------- +# Directory → hitting overview +# --------------------------------------------------------------------------- + + +@pytest.mark.usefixtures("catalog") +def test_selection_without_hitting_has_no_overview_link(client: TestClient) -> None: + response = client.get("/players?season=2003&player_id=677594") + assert response.status_code == 200 + assert "You selected Julio Rodríguez" in response.text + assert "/players/hitting" not in response.text + assert "View Hitting Overview" not in response.text + assert "No 2003 hitting line is stored locally for this Player." in response.text + assert ( + "poetry run python scripts/import_player_season.py " + "--player-id 677594 --season 2003" + ) in response.text + + +@pytest.mark.usefixtures("catalog") +def test_overview_link_needs_hitting_for_the_selected_season( + client: TestClient, migrated_session: Session +) -> None: + # Hitting stored for another season must not link from this one. + upsert_player_catalog_entry( + migrated_session, entry=entry(677594, "Julio Rodríguez", "CF", 1999) + ) + upsert_player_season_hitting(migrated_session, hitting=make_hitting(season=1999)) + migrated_session.commit() + assert ( + "/players/hitting" + not in client.get("/players?season=2003&player_id=677594").text + ) + assert ( + "/players/hitting?season=1999&player_id=677594" + in client.get("/players?season=1999&player_id=677594").text + ) + + +# --------------------------------------------------------------------------- +# /players/hitting +# --------------------------------------------------------------------------- + +HITTING_URL = "/players/hitting?season=2003&player_id=677594" + + +@pytest.fixture +def stored_hitting(catalog: None, migrated_session: Session) -> None: + upsert_player_season_hitting(migrated_session, hitting=make_hitting(season=2003)) + migrated_session.commit() + + +def prose(body: str) -> str: + """Collapse template line wrapping so sentences can be asserted whole.""" + return " ".join(body.split()) + + +def card_values(body: str) -> dict[str, str]: + """Map each summary card label to its rendered value.""" + labels = re.findall(r'summary-card__label">([^<]+)<', body) + values = re.findall(r'summary-card__value">([^<]+)<', body) + assert len(labels) == len(values) + return dict(zip(labels, values, strict=True)) + + +@pytest.mark.usefixtures("stored_hitting") +def test_overview_renders_stored_season_line(client: TestClient) -> None: + response = client.get(HITTING_URL) + assert response.status_code == 200 + body = response.text + assert "

Julio Rodríguez — 2003 Hitting

" in body + assert "Julio Rodríguez — 2003 Hitting" in body + # 150/500, 215/569, 246/500, and their sum, to three places. + assert card_values(body) == { + "AVG": ".300", + "OBP": ".378", + "SLG": ".492", + "OPS": ".870", + "Games": "150", + "Plate Appearances": "600", + "Home Runs": "20", + "Walks": "60", + "Strikeouts": "100", + "Stolen Bases": "10", + } + assert "246 TB in 500 AB" in body + assert "the stored 2003 season aggregate, not a game-by-game trend" in prose(body) + assert "do not imply the season is complete" in prose(body) + for completeness_claim in ("full 2003", "full-season", "complete season", "final"): + assert completeness_claim not in prose(body).lower() + assert "not a historical position for 2003" in prose(body) + assert_player_navigation(body) + assert "team-nav" not in body + + +@pytest.mark.usefixtures("stored_hitting") +def test_overview_chart_is_a_local_plate_appearance_profile( + client: TestClient, +) -> None: + body = client.get(HITTING_URL).text + assert '<script src="/vendor/plotly.min.js"></script>' in body + assert "cdn.plot.ly" not in body + assert 'id="player-plate-appearance-rates-chart"' in body + assert "Plate Appearance Rate Profile" in body + assert "percentages of 600 plate appearances" in body + for label in ('"K%"', '"BB%"', '"HR%"', '"16.7%"', '"10.0%"', '"3.3%"'): + assert label in body + for absent in ("MLB Average", "League Average", "Percentile", "Rank", "Elite"): + assert absent not in body + + +@pytest.mark.usefixtures("stored_hitting") +def test_overview_makes_no_team_or_comparison_claims(client: TestClient) -> None: + body = client.get(HITTING_URL).text + assert ( + "the stored line is the combined season aggregate; this page does not " + "model the individual team stints" + ) in prose(body) + for absent in ('name="team_id"', "team_id=", "vs MLB", "Mariners"): + assert absent not in body + + +@pytest.mark.usefixtures("stored_hitting") +def test_overview_links_back_to_the_selected_player(client: TestClient) -> None: + body = client.get(HITTING_URL).text + back = "/players?season=2003&player_id=677594" + assert f'<a class="player-link" href="{back}">← Back to Player Directory</a>' in ( + body + ) + directory = client.get(unescape(back)) + assert directory.status_code == 200 + assert "You selected <strong>Julio Rodríguez</strong>" in directory.text + assert "View Hitting Overview" in directory.text + + +@pytest.mark.usefixtures("stored_hitting") +def test_overview_query_is_shareable_and_order_independent( + client: TestClient, +) -> None: + first = client.get("/players/hitting?player_id=677594&season=2003") + assert first.status_code == 200 + assert first.text == client.get(HITTING_URL).text + + +@pytest.mark.usefixtures("catalog") +def test_missing_hitting_row_is_unavailable_not_zero(client: TestClient) -> None: + """200: the Player-season selection is valid; only its analytics are absent. + + This follows Team comparison without league data rather than an unstored + Team-season (404) or rows that conflict with the page (409). + """ + response = client.get(HITTING_URL) + assert response.status_code == 200 + body = response.text + assert "No 2003 hitting line is stored for Julio Rodríguez" in body + assert ( + "poetry run python scripts/import_player_season.py " + "--player-id 677594 --season 2003" + ) in body + assert "summary-card" not in body + assert ".000" not in body + assert "zero production" in prose(body) + assert "plotly" not in body + assert "/players?season=2003&player_id=677594" in body + assert_player_navigation(body) + + +@pytest.mark.parametrize( + ("query", "reason"), + [ + ("season=2003&player_id=99", "global identity without membership"), + ("season=2003&player_id=3", "member of another season only"), + ("season=2003&player_id=99999", "unknown Player"), + ("season=1980&player_id=677594", "season with no stored catalog"), + ], +) +@pytest.mark.usefixtures("catalog") +def test_overview_requires_catalog_membership( + client: TestClient, migrated_session: Session, query: str, reason: str +) -> None: + # A hitting row for every requested pair proves stats cannot bypass + # the catalog. Player 99999 has no identity row, so no hitting row either. + for player_id, season in ((99, 2003), (3, 2003), (677594, 1980)): + upsert_player_season_hitting( + migrated_session, + hitting=make_hitting(player_id=player_id, season=season), + ) + migrated_session.commit() + response = client.get(f"/players/hitting?{query}") + assert response.status_code == 404, reason + assert "That Player selection is not stored locally" in response.text + assert "is not in the locally stored" in response.text + assert "summary-card" not in response.text + assert "plotly" not in response.text + assert '<a class="player-link" href="/players">' in response.text + assert_player_navigation(response.text) + + +@pytest.mark.usefixtures("catalog") +def test_no_plate_appearances_leaves_rates_undefined( + client: TestClient, migrated_session: Session +) -> None: + zeros = dict.fromkeys( + ( + "plate_appearances", + "at_bats", + "runs", + "hits", + "doubles", + "triples", + "home_runs", + "rbi", + "base_on_balls", + "intentional_walks", + "hit_by_pitch", + "strikeouts", + "stolen_bases", + "caught_stealing", + "sac_flies", + "sac_bunts", + ), + 0, + ) + upsert_player_season_hitting( + migrated_session, + hitting=make_hitting(season=2003, games_played=2, **zeros), + ) + migrated_session.commit() + response = client.get(HITTING_URL) + assert response.status_code == 200 + values = card_values(response.text) + assert {values[label] for label in ("AVG", "OBP", "SLG", "OPS")} == {"—"} + assert values["Plate Appearances"] == "0" + assert "Undefined: no at-bats" in response.text + assert ".000<" not in response.text + assert "0.0%" not in response.text + assert "plotly" not in response.text + assert "K%, BB%, and HR% are undefined" in response.text + + +@pytest.mark.usefixtures("catalog") +def test_self_contradicting_hitting_row_is_a_conflict( + client: TestClient, migrated_session: Session +) -> None: + upsert_player_season_hitting( + migrated_session, + hitting=make_hitting(season=2003, at_bats=100, hits=150), + ) + migrated_session.commit() + response = client.get(HITTING_URL) + assert response.status_code == 409 + assert "150 hits in 100 at-bats" in response.text + assert "summary-card" not in response.text + assert "Traceback" not in response.text + + +@pytest.mark.usefixtures("catalog") +def test_strikeouts_over_plate_appearances_is_a_conflict_not_a_500( + client: TestClient, migrated_session: Session +) -> None: + """Regression: this row once reached the K% schema bound and escaped as a + Pydantic ValidationError instead of the route's 409 state.""" + upsert_player_season_hitting( + migrated_session, hitting=make_hitting(season=2003, strikeouts=601) + ) + migrated_session.commit() + response = client.get(HITTING_URL) + assert response.status_code == 409 + assert "601 strikeouts in 600 plate appearances" in response.text + assert "The stored 2003 hitting line cannot be summarized" in response.text + assert "summary-card" not in response.text + assert "plotly" not in response.text + assert "Traceback" not in response.text + assert "ValidationError" not in response.text + + +@pytest.mark.parametrize( + "query", + [ + "season=0&player_id=677594", + "season=no&player_id=677594", + "season=2003&player_id=-1", + "season=2003&player_id=no", + "season=2003", + "player_id=677594", + "", + ], +) +def test_overview_invalid_parameters_follow_browser_validation( + client: TestClient, query: str +) -> None: + response = client.get(f"/players/hitting?{query}", headers={"accept": "text/html"}) + assert response.status_code == 422 + assert "That link has a value this page cannot use" in response.text + assert "Traceback" not in response.text + + +def test_overview_missing_schema_has_migration_guidance(tmp_path: Path) -> None: + engine = build_engine(f"sqlite:///{tmp_path / 'unmigrated.db'}") + factory = build_session_factory(engine) + app = create_app() + + def override_session() -> Iterator[Session]: + with factory() as session: + yield session + + app.dependency_overrides[get_db_session] = override_session + try: + response = TestClient(app).get(HITTING_URL) + assert response.status_code == 503 + assert "The database schema is not ready" in response.text + assert "poetry run alembic upgrade head" in response.text + assert "Traceback" not in response.text + assert_player_navigation(response.text) + finally: + engine.dispose() + + +@pytest.mark.usefixtures("stored_hitting") +def test_team_navigation_and_health_are_unchanged(client: TestClient) -> None: + team = NavigationParser(client.get("/").text) + assert team.landmarks["Primary"] == [ + {"label": "Teams", "href": "/?window=15", "current": "location"}, + {"label": "Players", "href": "/players", "current": ""}, + ] + health = client.get("/health") + assert health.status_code == 200 + assert health.json()["status"] == "ok"