Repository navigation
Move Player hitting subset-count invariants into the domain contract #64
Description
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_appearancesdoubles + triples + home_runs <= hitsintentional_walks <= base_on_balls
But app.analytics.player_hitting._require_subset_counts() still has to reject additional impossible states such as:
hits > at_batsstrikeouts > at_batsstrikeouts > plate_appearancesbase_on_balls > plate_appearanceshome_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:
PlayerSeasonHittingrejectshits > 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 intoPlayerDataErrorrather 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
- Bulk ingestion implementation (Add bulk player-season hitting ingestion #60).
- Player refresh/completeness metadata.
- New Player statistics.
- Generic validation frameworks.
- Unrelated batting/pitching schema changes.
Completion criteria
- Normalization cannot produce a
PlayerSeasonHittingobject with impossible subset counts such ashits > 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