Skip to content

Move Player hitting subset-count invariants into the domain contract #64

Description

@Mattsface

Summary

Strengthen PlayerSeasonHitting so impossible counting-stat relationships are rejected during normalization/domain construction rather than only later when analytics tries to interpret them.

The current domain model already enforces several definitional relationships, including:

  • at_bats <= plate_appearances
  • doubles + triples + home_runs <= hits
  • intentional_walks <= base_on_balls

But app.analytics.player_hitting._require_subset_counts() still has to reject additional impossible states such as:

  • hits > at_bats
  • strikeouts > at_bats
  • strikeouts > plate_appearances
  • base_on_balls > plate_appearances
  • home_runs > plate_appearances

That means ingestion/normalization can currently accept a Player season line that the application later refuses to summarize.

Goal

Make PlayerSeasonHitting itself express the definitional counting-stat relationships required for a valid persisted Player hitting aggregate.

Invalid upstream data should fail before persistence.

Scope

Audit the existing subset-count checks in:

app/schemas/players.py
app/services/players.py
app/analytics/player_hitting.py
app/database/models.py
tests/

Move the appropriate definitional invariants into the PlayerSeasonHitting domain validator.

At minimum, address the audit-reproduced case:

hits > at_bats

and reconcile the other checks currently owned only by _require_subset_counts().

Design rule

Only encode relationships that are baseball definitions, not empirical expectations.

Examples:

  • a hit is an at-bat outcome, so hits <= at_bats;
  • a batting strikeout consumes an at-bat, so strikeouts <= at_bats;
  • walks and strikeouts cannot exceed plate appearances;
  • a home run is already bounded by doubles + triples + home_runs <= hits, but any retained redundant bound should have a clear reason.

Do not add speculative constraints merely because they are usually true.

Analytics behavior

Analytics may retain defensive checks if they still provide value for corrupted/legacy stored rows, but those checks should no longer be the first place normal imported data becomes invalid.

Avoid creating two divergent definitions of the same invariant.

If defensive analytics validation remains, centralize or clearly align the rules so future changes cannot drift.

Persistence boundary

Review whether the database should mirror any newly added definitional relationships with CHECK constraints.

If database constraints are added:

  • use Alembic;
  • preserve existing valid data;
  • test the migration on a fresh database;
  • do not silently rewrite invalid legacy rows.

If the smallest correct fix is domain-only for some relationships, document why.

Tests

Cover at minimum:

  • PlayerSeasonHitting rejects hits > at_bats;
  • rejects strikeouts > at_bats;
  • rejects any other definitional subset relationships moved from analytics;
  • valid zero-PA / zero-AB lines continue to work;
  • valid single-team, traded-player aggregate, and two-way-player fixtures remain valid;
  • get_player_season_hitting() converts domain validation failures into PlayerDataError rather than persisting bad data;
  • analytics still handles corrupted legacy/persistence data intentionally if its defensive checks remain;
  • full ingestion/repository/UI tests continue to pass.

Preserve

  • Existing Player aggregate semantics: one row per player-season, no team stint rows.
  • Missing data remains unknown rather than zero.
  • Single-player importer behavior.
  • Bulk Player hitting design for Add bulk player-season hitting ingestion #60.
  • DB-only browser rendering.
  • Existing rate formulas and presentation behavior.

Out of scope

Completion criteria

  • Normalization cannot produce a PlayerSeasonHitting object with impossible subset counts such as hits > at_bats.
  • Ingestion rejects such upstream data before persistence.
  • Analytics and domain rules are consistent rather than contradictory.
  • Relevant persistence constraints are aligned where appropriate.
  • Full tests and Ruff checks pass.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions