-
Notifications
You must be signed in to change notification settings - Fork 0
fix(baseball): compute OBP/SLG/OPS in legacy aggregate recalc (#436) #646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import { | |
| BaseballActionError, | ||
| } from '@/lib/baseball/with-baseball-action'; | ||
| import { BaseballCapabilityError, requireBaseballCapability } from '@/lib/baseball/capabilities'; | ||
| import { computeCareerSlashLine } from '@/lib/baseball/aggregates/career-slash-line'; | ||
|
|
||
| /** | ||
| * Shared error-message mapping for every action in this file. All exported | ||
|
|
@@ -551,6 +552,10 @@ const recalculatePlayerAggregatesAction = withBaseballAction( | |
| const totalAtBats = stats.reduce((sum, s) => sum + (s.at_bats || 0), 0); | ||
| const totalHits = stats.reduce((sum, s) => sum + (s.hits || 0), 0); | ||
|
|
||
| // Slash line — OBP/SLG/OPS from summed counting stats (null on zero | ||
| // denominators). Pure helper is unit-tested; see #436. | ||
| const { obp: careerObp, slg: careerSlg, ops: careerOps } = computeCareerSlashLine(stats); | ||
|
|
||
| // Last 5 and 10 sessions | ||
| const last5 = stats.slice(0, 5); | ||
| const last10 = stats.slice(0, 10); | ||
|
|
@@ -584,6 +589,9 @@ const recalculatePlayerAggregatesAction = withBaseballAction( | |
| team_id: teamId, | ||
| total_sessions: stats.length, | ||
| career_avg: careerAvg, | ||
| career_obp: careerObp, | ||
| career_slg: careerSlg, | ||
| career_ops: careerOps, | ||
|
Comment on lines
+592
to
+594
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3. Schema rollout breaks recalc recalculatePlayerAggregates now always upserts career_obp/career_slg/career_ops, so on any DB where those columns don’t exist yet the upsert will error and no aggregates (including career_avg) will be persisted for that run. uploadStatsCSV awaits recalculatePlayerAggregates but does not check its returned success flag, so uploads can still report success while aggregates stop updating until the migration is applied. Agent Prompt
|
||
| practice_avg: practiceAvg, | ||
| game_avg: gameAvg, | ||
| pressure_gap: pressureGap, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // ============================================================================= | ||
| // career-slash-line.test.ts | ||
| // | ||
| // Guards the career OBP/SLG/OPS derivation used by | ||
| // `recalculatePlayerAggregates` (BaseballHelm #436). Pure function; no DB. | ||
| // The headline case is a multi-session aggregate that exercises walks, sac | ||
| // flies, and hit-by-pitch — the fields that pull OBP away from AVG. | ||
| // ============================================================================= | ||
|
|
||
| import { describe, it, expect } from 'vitest'; | ||
| import { | ||
| computeCareerSlashLine, | ||
| type SlashLineStatRow, | ||
| } from '@/lib/baseball/aggregates/career-slash-line'; | ||
|
|
||
| const row = (partial: Partial<SlashLineStatRow>): SlashLineStatRow => ({ | ||
| at_bats: 0, | ||
| hits: 0, | ||
| doubles: 0, | ||
| triples: 0, | ||
| home_runs: 0, | ||
| walks: 0, | ||
| hit_by_pitch: 0, | ||
| sacrifice_flies: 0, | ||
| ...partial, | ||
| }); | ||
|
|
||
| describe('computeCareerSlashLine', () => { | ||
| it('aggregates OBP/SLG/OPS across multiple sessions with walks, SF, and HBP', () => { | ||
| // Session 1: 4 AB, 2 H (1 double), 1 BB, 1 HBP | ||
| // Session 2: 6 AB, 2 H (1 HR), 1 BB, 1 SF | ||
| // Totals: AB 10, H 4, 2B 1, HR 1, BB 2, HBP 1, SF 1 | ||
| const stats = [ | ||
| row({ at_bats: 4, hits: 2, doubles: 1, walks: 1, hit_by_pitch: 1 }), | ||
| row({ at_bats: 6, hits: 2, home_runs: 1, walks: 1, sacrifice_flies: 1 }), | ||
| ]; | ||
|
|
||
| const { obp, slg, ops } = computeCareerSlashLine(stats); | ||
|
|
||
| // OBP = (H + BB + HBP) / (AB + BB + HBP + SF) = (4 + 2 + 1) / (10 + 2 + 1 + 1) = 7/14 = .500 | ||
| expect(obp).toBeCloseTo(0.5, 6); | ||
| // TB = H + 2B + 2·3B + 3·HR = 4 + 1 + 0 + 3 = 8; SLG = 8/10 = .800 | ||
| expect(slg).toBeCloseTo(0.8, 6); | ||
| // OPS = .500 + .800 = 1.300 | ||
| expect(ops).toBeCloseTo(1.3, 6); | ||
| }); | ||
|
|
||
| it('returns null rates when every denominator is zero (no fabricated .000)', () => { | ||
| expect(computeCareerSlashLine([])).toEqual({ obp: null, slg: null, ops: null }); | ||
| expect(computeCareerSlashLine([row({})])).toEqual({ obp: null, slg: null, ops: null }); | ||
| }); | ||
|
|
||
| it('yields a non-null OBP but null SLG/OPS when a player only walks (zero AB)', () => { | ||
| // A plate appearance that is all walks: OBP denominator is non-zero, AB is zero. | ||
| const { obp, slg, ops } = computeCareerSlashLine([row({ walks: 2, hit_by_pitch: 1 })]); | ||
| // OBP = (0 + 2 + 1) / (0 + 2 + 1 + 0) = 3/3 = 1.000 | ||
| expect(obp).toBeCloseTo(1, 6); | ||
| expect(slg).toBeNull(); // zero AB → null, not 0 | ||
| expect(ops).toBeNull(); // null when either component is null | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // ============================================================================= | ||
| // career-slash-line.ts | ||
| // | ||
| // Pure derivation of a player's career OBP / SLG / OPS from summed counting | ||
| // stats. Extracted from `recalculatePlayerAggregates` (src/app/baseball/ | ||
| // actions/stats.ts) so it can be unit-tested without a database. | ||
| // | ||
| // Honesty contract: every rate is null on a zero denominator — never a | ||
| // fabricated .000. See BaseballHelm #436. | ||
| // ============================================================================= | ||
|
|
||
| import type { BaseballPlayerStats } from '@/lib/types'; | ||
|
|
||
| /** The counting-stat fields the slash line is derived from. */ | ||
| export type SlashLineStatRow = Pick< | ||
| BaseballPlayerStats, | ||
| 'at_bats' | 'hits' | 'doubles' | 'triples' | 'home_runs' | 'walks' | 'hit_by_pitch' | 'sacrifice_flies' | ||
| >; | ||
|
|
||
| export interface CareerSlashLine { | ||
| /** On-base percentage: (H + BB + HBP) / (AB + BB + HBP + SF); null on zero PA. */ | ||
| obp: number | null; | ||
| /** Slugging: total bases / AB; null on zero AB. */ | ||
| slg: number | null; | ||
| /** OPS = OBP + SLG; null when either component is null. */ | ||
| ops: number | null; | ||
| } | ||
|
|
||
| /** | ||
| * Derive career OBP/SLG/OPS from a set of per-session counting stats. | ||
| * Sums across every row, then applies the standard formulas. Missing | ||
| * counting values are treated as 0; denominators of 0 yield null. | ||
| */ | ||
| export function computeCareerSlashLine( | ||
| stats: ReadonlyArray<SlashLineStatRow>, | ||
| ): CareerSlashLine { | ||
| const sum = (pick: (s: SlashLineStatRow) => number | null | undefined): number => | ||
| stats.reduce((acc, s) => acc + (pick(s) || 0), 0); | ||
|
|
||
| const atBats = sum((s) => s.at_bats); | ||
| const hits = sum((s) => s.hits); | ||
| const doubles = sum((s) => s.doubles); | ||
| const triples = sum((s) => s.triples); | ||
| const homeRuns = sum((s) => s.home_runs); | ||
| const walks = sum((s) => s.walks); | ||
| const hitByPitch = sum((s) => s.hit_by_pitch); | ||
| const sacFlies = sum((s) => s.sacrifice_flies); | ||
|
|
||
| // OBP = (H + BB + HBP) / (AB + BB + HBP + SF) | ||
| const obpDenominator = atBats + walks + hitByPitch + sacFlies; | ||
| const obp = obpDenominator === 0 ? null : (hits + walks + hitByPitch) / obpDenominator; | ||
|
|
||
| // SLG = total bases / AB, where total bases = singles + 2·2B + 3·3B + 4·HR | ||
| // = H + 2B + 2·3B + 3·HR | ||
| const totalBases = hits + doubles + 2 * triples + 3 * homeRuns; | ||
| const slg = atBats === 0 ? null : totalBases / atBats; | ||
|
|
||
| const ops = obp != null && slg != null ? obp + slg : null; | ||
|
|
||
| return { obp, slg, ops }; | ||
| } |
20 changes: 20 additions & 0 deletions
20
supabase/migrations/20260701020000_baseball_player_aggregates_slash_line.sql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| -- BaseballHelm #436 — persist the slash line on player aggregates. | ||
| -- | ||
| -- `recalculatePlayerAggregates` (src/app/baseball/actions/stats.ts) previously | ||
| -- only wrote `career_avg`, but My Stats `StatsOverviewCards` reads | ||
| -- `career_obp`, `career_slg`, and `career_ops`. Those columns never existed on | ||
| -- the live `baseball_player_aggregates` table, so OBP/SLG/OPS permanently | ||
| -- showed "---". Add them here (numeric, matching `career_avg`) so the recalc | ||
| -- can compute and upsert the full slash line. | ||
| -- | ||
| -- Idempotent: ADD COLUMN IF NOT EXISTS. No data backfill — the columns fill in | ||
| -- on the next aggregate recalculation for each player. | ||
|
|
||
| ALTER TABLE "public"."baseball_player_aggregates" | ||
| ADD COLUMN IF NOT EXISTS "career_obp" numeric; | ||
|
|
||
| ALTER TABLE "public"."baseball_player_aggregates" | ||
| ADD COLUMN IF NOT EXISTS "career_slg" numeric; | ||
|
|
||
| ALTER TABLE "public"."baseball_player_aggregates" | ||
| ADD COLUMN IF NOT EXISTS "career_ops" numeric; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Slash line change undocumented
📘 Rule violation⚙ MaintainabilityAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools