From e0c7d1a944f2e09f478671a5e644eff6dbbbb22a Mon Sep 17 00:00:00 2001 From: Fable Integrator Date: Wed, 15 Jul 2026 03:10:27 -0400 Subject: [PATCH 1/2] fix(baseball): migrate player-today/passport/snapshot-cards off legacy stat layer (#379) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrates the three remaining Phase-1 read-model consumers of the deprecated flat/aggregate stat tables onto the canonical layers, per the #379 reconciliation design: - player-today.ts: "recent activity" now reads baseball_box_score_batting/ _pitching (joined to baseball_games) instead of baseball_player_stats. Box-score rows carry no CSV-import provenance, so trust/provenance are honestly null rather than a fabricated stamp. Manifest entry deleted. - player-passport.ts: the "recent activity" counts card (capturedSessions/ lastSessionDate/lastSessionSource) migrates the same way. The #434 thirds-aware IP summation (summarizePitchingSeason, merged as #815) is untouched — it already read box-score data and needed no change. Manifest entry deleted. - player-snapshot-cards.ts: exit-velocity fields now derive from baseball_batted_ball_events via elite-stat-events.ts's own buildHitterMetrics aggregator (closes the file's former "typed but un-migrated" comment) instead of baseball_player_stats.exit_velocity. Still reads baseball_player_aggregates for the Hitting/Pitching legacy-fallback tier and the game/scrimmage/practice "Performance" card — no canonical replacement exists yet for a standalone scrimmage split or a practice-session shape (see stats-migration-plan.md's open question). Manifest note updated to reflect this partial, deliberate scope; entry not deleted since the file still references the deprecated table. Also closes the #828 residual: adaptLegacyPlayerStats never read the legacy row's avg_pitch_velocity/max_pitch_velocity columns. Wired through as a per-field fallback under an explicit event-grain reading (pitch velocity has a legitimate legacy scalar; exit velocity has no legacy equivalent and stays event-only/null-safe). Adapter tests extended to cover the fallback, precedence, and no-data cases. Updated src/contracts/baseball/product-trust/player-today-honest-loop.test.ts and src/contracts/baseball/access/player-today-self-scope.test.ts fixtures to match the box-score source (not in the original chunk file list, but directly affected — these tests exercise getPlayerToday's DB path). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa --- .../access/player-today-self-scope.test.ts | 5 +- .../player-today-honest-loop.test.ts | 93 ++++++---- .../__tests__/legacy-stat-adapters.test.ts | 44 ++++- .../read-models/legacy-stat-adapters.ts | 35 +++- .../baseball/read-models/player-passport.ts | 158 +++++++++------- .../read-models/player-snapshot-cards.ts | 47 +++-- src/lib/baseball/read-models/player-today.ts | 169 +++++++++++------- src/lib/baseball/stat-layer-manifest.ts | 27 +-- 8 files changed, 375 insertions(+), 203 deletions(-) diff --git a/src/contracts/baseball/access/player-today-self-scope.test.ts b/src/contracts/baseball/access/player-today-self-scope.test.ts index 985423183..034cdf04a 100644 --- a/src/contracts/baseball/access/player-today-self-scope.test.ts +++ b/src/contracts/baseball/access/player-today-self-scope.test.ts @@ -47,7 +47,10 @@ function tablesWith(extra: Record = {}): Record { { id: TEAM_B, timezone: 'UTC' }, ], baseball_events: [], - baseball_player_stats: [], + // #379 — recentStats reads the canonical box-score tables now. + baseball_box_score_batting: [], + baseball_box_score_pitching: [], + baseball_games: [], baseball_actions: [], baseball_task_assignments: [], baseball_coach_notes: [], diff --git a/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts b/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts index 055beb37a..725f3b3eb 100644 --- a/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts +++ b/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts @@ -17,9 +17,11 @@ // ONLY by `error` — `available` is identical in both, which is the // actual fabrication risk: a caller that checks only `available` cannot // tell "genuinely nothing assigned" from "the query failed". -// 4. A hand-entered recent-stat row (no import_run_id, no -// source_trust_level) never gets a fabricated trust/provenance object; -// a stamped row does. +// 4. recentStats (#379 — box-score sourced, not the deprecated flat +// per-session stat table) never fabricates trust/provenance: a +// box-score row carries no CSV-import provenance columns at all, so +// every recentStats entry gets an honest trust:null/provenance:null +// rather than an implied import lineage that doesn't exist. // // Source of truth: `getPlayerToday` in // src/lib/baseball/read-models/player-today.ts. @@ -65,7 +67,11 @@ function baseTables(extra: Record = {}): Record { baseball_players: [{ id: PLAYER_ID, user_id: USER_ID }], baseball_team_members: [{ id: 'mem-1', team_id: TEAM_ID, player_id: PLAYER_ID }], baseball_events: [], - baseball_player_stats: [], + // #379 — recentStats now sources from the canonical box-score layer, not + // the deprecated flat/aggregate stat layer. + baseball_box_score_batting: [], + baseball_box_score_pitching: [], + baseball_games: [], helm_lifting_sessions: [], baseball_actions: [], helm_lifting_readiness_checkins: [], @@ -198,64 +204,81 @@ describe('getPlayerToday — sub-read failures are distinguishable from honest e }); }); -describe('getPlayerToday — recent stats never fabricate provenance for a hand-entered row (#377)', () => { - it('a hand-entered row (no import_run_id, no source_trust_level) gets trust:null + provenance:null', async () => { +describe('getPlayerToday — recent stats never fabricate provenance for a box-score row (#379)', () => { + // #379: recentStats migrated off the deprecated flat per-session stat + // table onto the canonical box-score layer (baseball_box_score_batting/ + // _pitching joined to baseball_games). Box-score rows are staff-entered via + // the box-score save flow, not imported, so they carry no CSV-import + // provenance columns at all — trust/provenance must always be an honest + // null, never a fabricated stamp implying an import lineage that doesn't + // exist. + it('a game with a captured box-score line surfaces trust:null + provenance:null (no import lineage exists to stamp)', async () => { fake = createFakeSupabase({ user: { id: USER_ID }, tables: baseTables({ - baseball_player_stats: [ + baseball_box_score_batting: [ + { id: 'bb-1', game_id: 'game-1', player_id: PLAYER_ID, team_id: TEAM_ID }, + ], + baseball_games: [ { - id: 'stat-hand', - player_id: PLAYER_ID, + id: 'game-1', team_id: TEAM_ID, - stat_type: 'batting', - session_date: DAY, - session_name: 'Hand-entered line', - source: 'manual', - source_trust_level: null, - source_match_tier: null, - source_match_confidence: null, - source_external_id: null, - import_run_id: null, + game_date: DAY, + game_type: 'official_game', + opponent_name: 'Rival High', }, ], }), }); const result = await getPlayerToday(TEAM_ID, { forDate: DAY }); - const row = result.recentStats.find((s) => s.id === 'stat-hand'); + const row = result.recentStats.find((s) => s.id === 'game-1'); expect(row).toBeTruthy(); + expect(row?.statType).toBe('official_game'); + expect(row?.sessionDate).toBe(DAY); + expect(row?.sessionName).toBe('vs Rival High'); expect(row?.trust).toBeNull(); expect(row?.provenance).toBeNull(); }); - it('a stamped row (source_trust_level set) DOES get a real trust + provenance object', async () => { + it('a game with only a pitching line (no batting row) still surfaces in recentStats', async () => { fake = createFakeSupabase({ user: { id: USER_ID }, tables: baseTables({ - baseball_player_stats: [ + baseball_box_score_pitching: [ + { id: 'bp-1', game_id: 'game-2', player_id: PLAYER_ID, team_id: TEAM_ID }, + ], + baseball_games: [ { - id: 'stat-stamped', - player_id: PLAYER_ID, + id: 'game-2', team_id: TEAM_ID, - stat_type: 'batting', - session_date: DAY, - session_name: 'Imported line', - source: 'csv_import', - source_trust_level: 'staff_entered', - source_match_tier: null, - source_match_confidence: null, - source_external_id: null, - import_run_id: null, + game_date: DAY, + game_type: 'scrimmage', + opponent_name: null, }, ], }), }); const result = await getPlayerToday(TEAM_ID, { forDate: DAY }); - const row = result.recentStats.find((s) => s.id === 'stat-stamped'); + const row = result.recentStats.find((s) => s.id === 'game-2'); expect(row).toBeTruthy(); - expect(row?.trust).not.toBeNull(); - expect(row?.provenance).not.toBeNull(); + expect(row?.statType).toBe('scrimmage'); + // No opponent on file -> honest null, never a fabricated label. + expect(row?.sessionName).toBeNull(); + expect(row?.trust).toBeNull(); + expect(row?.provenance).toBeNull(); + }); + + it('a FAILING box-score read returns recentStats:[] but sets `error` — distinguishable from a genuinely stat-less player', async () => { + failSelect(fake, 'baseball_box_score_batting', 'boom'); + + const result = await getPlayerToday(TEAM_ID, { forDate: DAY }); + // Same shape as the honest-empty case (recentStats: [])... + expect(result.recentStats).toEqual([]); + // ...but `error` is non-null — the ONLY signal this is a failure, not + // "this player has no captured box-score lines yet" (mirrors the item-3 + // sub-read contract above). + expect(result.error).toBe('Your recent stats could not be loaded.'); }); }); diff --git a/src/lib/baseball/read-models/__tests__/legacy-stat-adapters.test.ts b/src/lib/baseball/read-models/__tests__/legacy-stat-adapters.test.ts index eff2267b4..642b40b22 100644 --- a/src/lib/baseball/read-models/__tests__/legacy-stat-adapters.test.ts +++ b/src/lib/baseball/read-models/__tests__/legacy-stat-adapters.test.ts @@ -160,10 +160,10 @@ describe('adaptLegacyPlayerStats — practice carve-out', () => { }); describe('adaptLegacyPlayerStats — event-derived fields (null-safe)', () => { - it('defaults every event-derived field to null when no event input is given', () => { + it('defaults exit-velocity fields to null when no event input is given (no legacy equivalent exists)', () => { const result = adaptLegacyPlayerStats({ playerId: 'p1', - legacy: legacyRow({ avg_pitch_velocity: 88 }), + legacy: legacyRow(), // avg_pitch_velocity / max_pitch_velocity both null here boxScore: boxScoreRow(), }); @@ -188,6 +188,46 @@ describe('adaptLegacyPlayerStats — event-derived fields (null-safe)', () => { expect(result.event.avgPitchVelocity).toBeNull(); expect(result.event.maxPitchVelocity).toBeNull(); }); + + it('falls back to the legacy avg/max pitch-velocity scalars when no event input is given — a legitimate, previously-captured fallback (#379 residual)', () => { + const result = adaptLegacyPlayerStats({ + playerId: 'p1', + legacy: legacyRow({ avg_pitch_velocity: 88, max_pitch_velocity: 95 }), + boxScore: boxScoreRow(), + }); + + expect(result.event.avgPitchVelocity).toBe(88); + expect(result.event.maxPitchVelocity).toBe(95); + // Exit velocity has NO legacy column at all — stays null-safe even + // though this same legacy row carries real pitch-velocity data. + expect(result.event.avgExitVelocity).toBeNull(); + expect(result.event.maxExitVelocity).toBeNull(); + }); + + it('prefers an explicit event-grain pitch-velocity reading over the legacy fallback when both are present', () => { + const result = adaptLegacyPlayerStats({ + playerId: 'p1', + legacy: legacyRow({ avg_pitch_velocity: 88, max_pitch_velocity: 95 }), + boxScore: boxScoreRow(), + event: { avgPitchVelocity: 92.3 }, + }); + + expect(result.event.avgPitchVelocity).toBe(92.3); + // maxPitchVelocity wasn't supplied in the event input -> falls back to + // the legacy scalar independently, per-field (not all-or-nothing). + expect(result.event.maxPitchVelocity).toBe(95); + }); + + it('reports null pitch-velocity (never fabricated) when neither an event reading nor a legacy row exists', () => { + const result = adaptLegacyPlayerStats({ + playerId: 'p1', + legacy: null, + boxScore: boxScoreRow(), + }); + + expect(result.event.avgPitchVelocity).toBeNull(); + expect(result.event.maxPitchVelocity).toBeNull(); + }); }); describe('adaptLegacyStatsMap', () => { diff --git a/src/lib/baseball/read-models/legacy-stat-adapters.ts b/src/lib/baseball/read-models/legacy-stat-adapters.ts index d7d1af8ab..7016ba066 100644 --- a/src/lib/baseball/read-models/legacy-stat-adapters.ts +++ b/src/lib/baseball/read-models/legacy-stat-adapters.ts @@ -20,8 +20,15 @@ // §3.3 — game/practice contexts are never merged without a labeled filter). // // Event-derived fields (exit velocity, pitch velocity) are null-safe: they -// are populated ONLY from an explicit event-grain input (the elite-event -// read model), never fabricated from a legacy scalar. +// are populated from an explicit event-grain input (the elite-event read +// model) when one is supplied. Exit velocity has NO legacy column at all — +// it stays null unless an event input supplies it, never fabricated. Pitch +// velocity DOES have a legitimate legacy scalar (avg_pitch_velocity / +// max_pitch_velocity on the legacy aggregate row, predating the event-grain +// model) — an explicit event-grain value still wins outright when supplied, +// but the legacy scalar is a real, non-fabricated fallback when no +// event-grain reading exists, so a team that hasn't captured pitch-velocity +// events yet doesn't regress from "shows a real number" to "shows nothing". // // Every returned shape carries a `sourceLayer` tag so callers (and a future // UI source chip) can label a legacy-fallback number honestly instead of @@ -92,6 +99,12 @@ export interface BoxScoreGameContextRow { * Every field is null-safe: omit a field (or pass no `event` input at all) * when no matching event-grain row exists rather than fabricating a number * from an unrelated legacy scalar. + * + * Exception: `avgPitchVelocity`/`maxPitchVelocity` DO have a legitimate + * legacy fallback (see {@link adaptLegacyPlayerStats}) — the legacy + * aggregate row's own `avg_pitch_velocity`/`max_pitch_velocity` columns are a + * real, previously-captured measurement, not a fabrication. Exit velocity has + * no such legacy column and stays event-only. */ export interface EventDerivedFields { avgExitVelocity: number | null; @@ -189,8 +202,12 @@ export interface AdaptLegacyPlayerStatsInput { * Practice-context and legacy-only trend/development fields always pass * through the legacy row unchanged, regardless of box-score presence — the * canonical layers have no practice-session concept yet (permanent carve-out, - * see module doc). Event-derived fields are populated ONLY from the `event` - * input; they are never backfilled from a legacy scalar. + * see module doc). Event-derived fields are populated from the `event` input + * when supplied; `avgPitchVelocity`/`maxPitchVelocity` additionally fall back + * to the legacy row's own `avg_pitch_velocity`/`max_pitch_velocity` columns + * when no event-grain reading is supplied (a legitimate, previously-captured + * measurement — not a fabrication). Exit velocity has no legacy column at + * all and is never backfilled from anything but the `event` input. */ export function adaptLegacyPlayerStats( input: AdaptLegacyPlayerStatsInput, @@ -232,10 +249,16 @@ export function adaptLegacyPlayerStats( sessions: legacy?.practice_sessions ?? 0, }, event: { + // Exit velocity: event-only, null-safe — no legacy column exists at all. avgExitVelocity: event?.avgExitVelocity ?? EMPTY_EVENT_DERIVED_FIELDS.avgExitVelocity, maxExitVelocity: event?.maxExitVelocity ?? EMPTY_EVENT_DERIVED_FIELDS.maxExitVelocity, - avgPitchVelocity: event?.avgPitchVelocity ?? EMPTY_EVENT_DERIVED_FIELDS.avgPitchVelocity, - maxPitchVelocity: event?.maxPitchVelocity ?? EMPTY_EVENT_DERIVED_FIELDS.maxPitchVelocity, + // Pitch velocity: an explicit event-grain reading wins outright when + // supplied; otherwise falls back to the legacy aggregate row's own + // avg_pitch_velocity/max_pitch_velocity columns — a real, + // previously-captured measurement, not a fabrication (#379 residual — + // this adapter previously never read these two legacy columns at all). + avgPitchVelocity: event?.avgPitchVelocity ?? legacy?.avg_pitch_velocity ?? EMPTY_EVENT_DERIVED_FIELDS.avgPitchVelocity, + maxPitchVelocity: event?.maxPitchVelocity ?? legacy?.max_pitch_velocity ?? EMPTY_EVENT_DERIVED_FIELDS.maxPitchVelocity, }, legacyExtras: { pressureGap: legacy?.pressure_gap ?? null, diff --git a/src/lib/baseball/read-models/player-passport.ts b/src/lib/baseball/read-models/player-passport.ts index f41272c25..79a41d9e8 100644 --- a/src/lib/baseball/read-models/player-passport.ts +++ b/src/lib/baseball/read-models/player-passport.ts @@ -16,10 +16,12 @@ // and the model never fabricates a measurable it does not have. // // This read model READS existing tables only (baseball_players for identity + -// measurables, baseball_player_stats for recent activity counts, -// baseball_player_timeline_events via the development story). It stores no new -// per-field data of its own beyond the passport SETTINGS row -// (baseball_player_passport_settings) that controls exposure. +// measurables, baseball_box_score_batting/_pitching joined to baseball_games +// for recent activity counts (#379 — the canonical layer-2 source; see +// docs/baseball/stats-architecture.md), baseball_player_timeline_events via +// the development story). It stores no new per-field data of its own beyond +// the passport SETTINGS row (baseball_player_passport_settings) that +// controls exposure. // // VIEWER-AWARE: resolves the asking viewer (staff vs the subject player vs other) // server-side and filters fields by both the V2 data-visibility rules AND the @@ -40,11 +42,6 @@ import { type CaptureMode, } from '@/lib/baseball/source-record'; import { buildSourceTrust } from '@/components/baseball/source-trust/build-source-trust'; -import { - buildStampedSourceTrust, - buildImportProvenance, - type StampedStatProvenance, -} from '@/components/baseball/source-trust/stamped-trust'; import { getPlayerTimeline } from '@/lib/baseball/read-models/timeline'; import { getVideoLibrary } from '@/lib/baseball/read-models/video-classes'; import { sumInningsPitched, ipToInnings } from '@/lib/baseball/innings'; @@ -429,6 +426,71 @@ const MEASURABLE_DEFS: MeasurableDef[] = [ { key: 'arm_strength', column: 'arm_strength', label: 'Throwing Velocity', unit: 'mph' }, ]; +// ----------------------------------------------------------------------------- +// Recent activity (#379 — canonical box-score source; see +// docs/baseball/stats-architecture.md for the three-layer stat model this +// read model no longer reads the deprecated flat/aggregate layer of) +// ----------------------------------------------------------------------------- + +/** One game this player has a captured box-score line for. */ +interface RecentBoxScoreGame { + id: string; + game_date: string; + opponent_name: string | null; +} + +/** + * This player's most recent games with a captured box-score line (batting OR + * pitching), newest first, capped at `limit`. Reads + * baseball_box_score_batting/_pitching (game ids only) then baseball_games + * for the display fields — the canonical layer-2 tables, per #379's + * migration of this read model's "recent activity" card off the deprecated + * flat/aggregate stat layer. Degrades to an honest empty list + error string + * on a sub-read failure. + */ +async function fetchRecentBoxScoreActivity( + supabase: Awaited>, + playerId: string, + teamId: string, + limit: number, +): Promise<{ data: RecentBoxScoreGame[]; error: string | null }> { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const db = supabase as any; + const [battingRes, pitchingRes] = await Promise.all([ + db + .from('baseball_box_score_batting') + .select('game_id') + .eq('player_id', playerId) + .eq('team_id', teamId), + db + .from('baseball_box_score_pitching') + .select('game_id') + .eq('player_id', playerId) + .eq('team_id', teamId), + ]); + if (battingRes.error || pitchingRes.error) { + return { data: [], error: 'Recent activity could not be loaded.' }; + } + + const gameIds = [ + ...new Set([ + ...((battingRes.data ?? []) as Array<{ game_id: string }>).map((r) => r.game_id), + ...((pitchingRes.data ?? []) as Array<{ game_id: string }>).map((r) => r.game_id), + ]), + ]; + if (gameIds.length === 0) return { data: [], error: null }; + + const { data: games, error: gamesErr } = await db + .from('baseball_games') + .select('id, game_date, opponent_name') + .in('id', gameIds) + .order('game_date', { ascending: false }) + .limit(limit); + if (gamesErr) return { data: [], error: 'Recent activity could not be loaded.' }; + + return { data: (games ?? []) as RecentBoxScoreGame[], error: null }; +} + // ----------------------------------------------------------------------------- // getPassportSettingsForEditor // ----------------------------------------------------------------------------- @@ -593,19 +655,11 @@ export async function getPlayerPassport( .eq('player_id', targetPlayerId) .eq('team_id', teamId) .maybeSingle(), - // GAP 3 — pull the import-stamped provenance columns + run link so the most- - // recent session's "from" line carries the same SourceTrust chip + drawer. - // Stamped columns aren't in generated database.ts -> untyped client. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (supabase as any) - .from('baseball_player_stats') - .select( - 'session_date, session_name, source, source_trust_level, source_match_tier, source_match_confidence, source_external_id, import_run_id', - ) - .eq('player_id', targetPlayerId) - .eq('team_id', teamId) - .order('session_date', { ascending: false }) - .limit(50), + // #379 — recent activity is sourced from the canonical box-score layer + // (baseball_box_score_batting/_pitching joined to baseball_games), not the + // deprecated flat/aggregate stat layer this read model used before its + // #379 migration. + fetchRecentBoxScoreActivity(supabase, targetPlayerId, teamId, 50), ]); if (playerRes.error || !playerRes.data) { @@ -714,48 +768,23 @@ export async function getPlayerPassport( } // ---- Recent activity (counts only) ---- - const statRows = (statsRes.error ? [] : statsRes.data ?? []) as unknown as Array< - StampedStatProvenance & { session_date: string; session_name: string | null } - >; - const lastSession = statRows[0] ?? null; - - // GAP 3 — build the stamped trust + provenance for the most-recent session. - let lastSessionTrust: SourceTrust | null = null; - let lastSessionProvenance: SourceProvenance | null = null; - if (lastSession && (lastSession.import_run_id || lastSession.source_trust_level)) { - let reviewState: string | null = null; - if (lastSession.import_run_id) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const { data: run } = await (supabase as any) - .from('baseball_import_runs') - .select('review_state') - .eq('id', lastSession.import_run_id) - .maybeSingle(); - reviewState = (run as { review_state: string | null } | null)?.review_state ?? null; - } - const stamped: StampedStatProvenance = { - source: lastSession.source, - source_trust_level: lastSession.source_trust_level, - source_match_tier: lastSession.source_match_tier, - source_match_confidence: lastSession.source_match_confidence, - source_external_id: lastSession.source_external_id, - import_run_id: lastSession.import_run_id, - review_state: reviewState, - importedAt: lastSession.session_date, - }; - const label = lastSession.session_name?.trim() || 'Imported stats'; - lastSessionTrust = buildStampedSourceTrust(stamped, label); - lastSessionProvenance = buildImportProvenance(stamped, { label }); - } + // #379 — sourced from the canonical box-score/season layer (games this + // player has a captured batting or pitching line for), not the deprecated + // flat/aggregate stat layer. Box-score rows carry no CSV-import provenance + // columns (staff-entered via the box-score save flow, not imported), so + // lastSessionTrust/lastSessionProvenance are honestly null rather than a + // fabricated import stamp. + const recentGames = statsRes.error ? [] : statsRes.data; + const lastGame = recentGames[0] ?? null; const recentActivity = { - capturedSessions: statRows.length, - lastSessionDate: lastSession?.session_date ?? null, - lastSessionSource: lastSession - ? buildSourceRef({ source: lastSession.source }) + capturedSessions: recentGames.length, + lastSessionDate: lastGame?.game_date ?? null, + lastSessionSource: lastGame + ? buildSourceRef({ source: 'manual', sourceId: lastGame.id, label: 'Box score' }) : null, - lastSessionTrust, - lastSessionProvenance, + lastSessionTrust: null, + lastSessionProvenance: null, }; // --------------------------------------------------------------------------- @@ -826,14 +855,15 @@ export async function getPlayerPassport( section: 'stats', label: 'Captured stats', // In full mode we know the real game-log count; in compact we fall back to - // the captured-session count. Either way this is a real signal, not a guess. - complete: mode === 'full' ? performanceGameCount > 0 : statRows.length > 0, + // the captured-session count (#379 — box-score-sourced). Either way this + // is a real signal, not a guess. + complete: mode === 'full' ? performanceGameCount > 0 : recentGames.length > 0, note: mode === 'full' ? performanceGameCount > 0 ? `${performanceGameCount} game log${performanceGameCount === 1 ? '' : 's'} on file.` : 'No box-score game logs yet.' - : statRows.length > 0 + : recentGames.length > 0 ? 'Complete.' : 'No captured stat sessions yet.', }, diff --git a/src/lib/baseball/read-models/player-snapshot-cards.ts b/src/lib/baseball/read-models/player-snapshot-cards.ts index ec936e61a..2e565c702 100644 --- a/src/lib/baseball/read-models/player-snapshot-cards.ts +++ b/src/lib/baseball/read-models/player-snapshot-cards.ts @@ -57,6 +57,13 @@ import { } from '@/lib/lifting/resolve-baseball-context'; import { extractArmStatusFromNotes, sleepQualityToHours } from '@/lib/lifting/adapters/baseball-view-adapter'; import type { HelmLiftingReadinessCheckinRow } from '@/lib/types/helm-lifting-data'; +// #379 — exit velocity is derived via the same pure aggregator elite-stat- +// events.ts's canonical read model uses (avg_exit_velocity metric + the +// evLaPoints visual payload), fed with this player's own batted-ball events +// queried directly here (this file already resolves its own staff gate above +// and reads several other event-grain tables the same way). +import { buildHitterMetrics } from '@/lib/baseball/read-models/elite-stat-events'; +import type { BaseballBattedBallEvent } from '@/lib/types/baseball-stat-events'; // The V6 event/elite tables + V11 lifting/readiness tables ship via migrations // and are NOT in the generated database.ts (no db:types regen without a live @@ -664,19 +671,37 @@ export async function getPlayerSnapshotCards( .select('title, goals, status, coach_id, updated_at, coach:baseball_coaches(full_name)') .eq('player_id', playerId).eq('team_id', teamId) .order('updated_at', { ascending: false }).limit(5), - // Exit velocity lives on captured stat sessions (real column), NOT on - // baseball_player_aggregates (those EV fields are typed but un-migrated). - supabase.from('baseball_player_stats') - .select('exit_velocity') - .eq('player_id', playerId).eq('team_id', teamId) - .not('exit_velocity', 'is', null).limit(200), + // #379 — exit velocity is derived from the elite batted-ball event grain + // (the canonical layer-3 source), not the deprecated flat per-session stat + // table (previously "typed but un-migrated" per this file's own comment). + // Scoped to the official/scrimmage contexts — the same default set + // elite-stat-events.ts's canonical entry point uses — and to CURRENT rows + // only (a corrected import supersedes the prior row via GAP 5's + // superseded_by_run_id, mirrored from elite-stat-events.ts's own filter). + // Ordered newest-first BEFORE the cap so the 500-row sample is the most + // recent events, never an arbitrary PostgREST slice (#813) — mirrors the + // fielding/catching event queries above. + fromUntyped(db, 'baseball_batted_ball_events') + .select('*') + .eq('team_id', teamId).eq('batter_id', playerId) + .in('data_context', ['official_game', 'scrimmage']) + .is('superseded_by_run_id', null) + .order('created_at', { ascending: false }) + .limit(500), ]); - // Exit-velocity aggregation from captured sessions (honest: null when none). - const evRows = (evRes?.error ? [] : evRes?.data ?? []) as Array<{ exit_velocity: number | null }>; - const evValues = evRows.map((r) => r.exit_velocity).filter((v): v is number => v != null && Number.isFinite(v)); - const avgExitVelocity = evValues.length - ? Math.round((evValues.reduce((a, b) => a + b, 0) / evValues.length) * 10) / 10 : null; + // Exit-velocity aggregation from real batted-ball events (honest: null when + // none captured yet). Reuses elite-stat-events.ts's own pure aggregator so + // the numbers match the Stats Lab exactly rather than a second derivation. + const battedBalls = (evRes?.error ? [] : evRes?.data ?? []) as BaseballBattedBallEvent[]; + const hitterMetrics = buildHitterMetrics(playerId, [], battedBalls, 'official_game'); + const rawAvgExitVelocity = + hitterMetrics.metrics.find((m) => m.metricKey === 'avg_exit_velocity')?.value ?? null; + const avgExitVelocity = + rawAvgExitVelocity != null ? Math.round(rawAvgExitVelocity * 10) / 10 : null; + const evValues = hitterMetrics.visuals.evLaPoints + .map((p) => p.exitVelocity) + .filter((v): v is number => v != null && Number.isFinite(v)); const maxExitVelocity = evValues.length ? Math.round(Math.max(...evValues) * 10) / 10 : null; // --------------------------------------------------------------------------- diff --git a/src/lib/baseball/read-models/player-today.ts b/src/lib/baseball/read-models/player-today.ts index 9d95fe791..c11e97a3f 100644 --- a/src/lib/baseball/read-models/player-today.ts +++ b/src/lib/baseball/read-models/player-today.ts @@ -8,8 +8,17 @@ // // 1. schedule — today's baseball_events for their team, each annotated with // THIS player's acknowledgement status (acknowledged / pending). -// 2. recentStats — their last few captured stat sessions (active captures), -// source-labeled, so "Today" can show real recent activity. +// 2. recentStats — their last few games with a captured box score (active +// captures), source-labeled, so "Today" can show real +// recent activity. Reads baseball_box_score_batting / +// _pitching (joined to baseball_games) — the canonical +// layer-2 tables — not the deprecated flat per-session +// stat table this read model used before its #379 +// migration. Box-score rows carry no CSV-import +// provenance columns (they are staff-entered via the +// box-score save flow, not imported), so each entry's +// trust/provenance chip is honestly null rather than a +// fabricated import stamp. // 3. assignments — today's (and near-term upcoming) lift sessions for THIS // player, read from helm_lifting_sessions — the unified Lab // table the W2-G rewire moved publishLiftDay materialization, @@ -56,11 +65,6 @@ import { localDayBoundsUtc, } from '@/lib/baseball/daily-contract/contract-day'; import { buildSourceRef, type SourceRef } from '@/lib/baseball/source-record'; -import { - buildStampedSourceTrust, - buildImportProvenance, - type StampedStatProvenance, -} from '@/components/baseball/source-trust/stamped-trust'; import type { SourceTrust, SourceProvenance, @@ -166,11 +170,14 @@ export interface PlayerTodayStat { /** * GAP 3 — render-ready trust descriptor built from the import-stamped columns * (source_trust_level / match tier / confidence / import run / review state). - * Null for a row with no stamped provenance (e.g. a hand-entered line). Mounts - * the same SourceTrustBadge + SourceDrawer the event path uses, player side. + * Always null for box-score-sourced rows (#379): box-score entries are + * staff-entered via the box-score save flow, not imported, so there is no + * stamped import provenance to describe — an honest null, never a + * fabricated stamp. Kept as a real (not removed) field so the shape stays + * ready for a future source that does carry stamped provenance. */ trust: SourceTrust | null; - /** Rich provenance for the drawer (opens the Import Dossier run). */ + /** Rich provenance for the drawer (opens the Import Dossier run). Same honesty note as `trust`. */ provenance: SourceProvenance | null; } @@ -413,6 +420,70 @@ async function resolvePlayer( return { userId: user.id, playerId: player.id, isMember: !!member }; } +// ----------------------------------------------------------------------------- +// Recent activity (#379 — canonical box-score source; see +// docs/baseball/stats-architecture.md for the three-layer stat model this +// read model no longer reads the deprecated flat/aggregate layer of) +// ----------------------------------------------------------------------------- + +/** One game this player has a captured box-score line for. */ +interface RecentBoxScoreGame { + id: string; + game_date: string; + game_type: string; + opponent_name: string | null; +} + +/** + * This player's most recent games with a captured box-score line (batting OR + * pitching), newest first. Reads baseball_box_score_batting/_pitching (game + * ids only) then baseball_games for the display fields — the canonical + * layer-2 tables, per #379's migration of this read model off the deprecated + * flat/aggregate stat layer. Degrades to an honest empty list + error string + * on a sub-read failure, matching this read model's existing fault-tolerance + * convention. + */ +async function fetchRecentBoxScoreActivity( + supabase: Awaited>, + playerId: string, + teamId: string, + limit: number, +): Promise<{ data: RecentBoxScoreGame[]; error: string | null }> { + const [battingRes, pitchingRes] = await Promise.all([ + supabase + .from('baseball_box_score_batting') + .select('game_id') + .eq('player_id', playerId) + .eq('team_id', teamId), + supabase + .from('baseball_box_score_pitching') + .select('game_id') + .eq('player_id', playerId) + .eq('team_id', teamId), + ]); + if (battingRes.error || pitchingRes.error) { + return { data: [], error: 'Your recent stats could not be loaded.' }; + } + + const gameIds = [ + ...new Set([ + ...(battingRes.data ?? []).map((r) => r.game_id), + ...(pitchingRes.data ?? []).map((r) => r.game_id), + ]), + ]; + if (gameIds.length === 0) return { data: [], error: null }; + + const { data: games, error: gamesErr } = await supabase + .from('baseball_games') + .select('id, game_date, game_type, opponent_name') + .in('id', gameIds) + .order('game_date', { ascending: false }) + .limit(limit); + if (gamesErr) return { data: [], error: 'Your recent stats could not be loaded.' }; + + return { data: (games ?? []) as RecentBoxScoreGame[], error: null }; +} + // ----------------------------------------------------------------------------- // getPlayerToday // ----------------------------------------------------------------------------- @@ -575,17 +646,15 @@ export async function getPlayerToday( .gte('start_time', dayStart) .lte('start_time', dayEnd) .order('start_time', { ascending: true }), - // GAP 3 — also select the import-stamped provenance columns so each recent - // stat carries the same SourceTrust chip + drawer the event path has. - supabase - .from('baseball_player_stats') - .select( - 'id, stat_type, session_date, session_name, source, source_trust_level, source_match_tier, source_match_confidence, source_external_id, import_run_id', - ) - .eq('player_id', playerId) - .eq('team_id', teamId) - .order('session_date', { ascending: false }) - .limit(Math.min(Math.max(recentStatLimit, 1), 25)), + // #379 — recent activity is now sourced from the canonical box-score + // layer (baseball_box_score_batting/_pitching joined to baseball_games), + // not the deprecated flat/aggregate stat layer. + fetchRecentBoxScoreActivity( + supabase, + playerId, + teamId, + Math.min(Math.max(recentStatLimit, 1), 25), + ), // Assignments: this player's OPEN lift sessions — overdue (still not done) // through the near-term horizon. Reads helm_lifting_sessions — the unified // Lab table publishLiftDay materializes into and the Lift & Check-in card @@ -749,54 +818,24 @@ export async function getPlayerToday( }); // ---- Recent stats (active captures) ---- + // #379 — sourced from box-score/season-era games (canonical layer 2), not + // the deprecated flat/aggregate stat layer. Box-score rows carry no + // CSV-import provenance columns (staff-entered via the box-score save flow, + // not imported), so trust/provenance are honestly null rather than a + // fabricated import stamp. const recentStats: PlayerTodayStat[] = []; if (statsRes.error) { - error = error ?? 'Your recent stats could not be loaded.'; + error = error ?? statsRes.error; } else { - const statRows = (statsRes.data ?? []) as unknown as Array< - StampedStatProvenance & { - id: string; - stat_type: string; - session_date: string; - session_name: string | null; - } - >; - // GAP 3 — one batched lookup of run review_state for the imported rows, so the - // drawer can show reviewed vs unreviewed without an N+1. - const runIds = [...new Set(statRows.map((s) => s.import_run_id).filter(Boolean))] as string[]; - const reviewByRun = new Map(); - if (runIds.length > 0) { - const { data: runs } = await supabase - .from('baseball_import_runs') - .select('id, review_state') - .in('id', runIds); - for (const r of ((runs ?? []) as Array<{ id: string; review_state: string | null }>)) { - reviewByRun.set(r.id, r.review_state); - } - } - for (const s of statRows) { - const stamped: StampedStatProvenance = { - source: s.source, - source_trust_level: s.source_trust_level, - source_match_tier: s.source_match_tier, - source_match_confidence: s.source_match_confidence, - source_external_id: s.source_external_id, - import_run_id: s.import_run_id, - review_state: s.import_run_id ? reviewByRun.get(s.import_run_id) ?? null : null, - importedAt: s.session_date, - }; - // Only imported/device/official rows carry stamped provenance; a hand-entered - // line has no import_run_id and reads as a plain source label. - const hasStamp = !!s.import_run_id || !!s.source_trust_level; - const label = s.session_name?.trim() || 'Imported stats'; + for (const g of statsRes.data) { recentStats.push({ - id: s.id, - statType: s.stat_type, - sessionDate: s.session_date, - sessionName: s.session_name, - sourceRef: buildSourceRef({ source: s.source }), - trust: hasStamp ? buildStampedSourceTrust(stamped, label) : null, - provenance: hasStamp ? buildImportProvenance(stamped, { label }) : null, + id: g.id, + statType: g.game_type, + sessionDate: g.game_date, + sessionName: g.opponent_name ? `vs ${g.opponent_name}` : null, + sourceRef: buildSourceRef({ source: 'manual', sourceId: g.id, label: 'Box score' }), + trust: null, + provenance: null, }); } } diff --git a/src/lib/baseball/stat-layer-manifest.ts b/src/lib/baseball/stat-layer-manifest.ts index d150bd376..7d387c802 100644 --- a/src/lib/baseball/stat-layer-manifest.ts +++ b/src/lib/baseball/stat-layer-manifest.ts @@ -118,12 +118,6 @@ export const GRANDFATHERED_CONSUMERS: GrandfatheredStatLayerConsumer[] = [ note: 'Reads baseball_player_stats and upserts baseball_player_aggregates (career/practice/game averages, trend). The other half of the legacy write path alongside imports.ts.', }, - { - path: 'src/app/baseball/actions/insights.ts', - group: 'server-action', - status: 'pending migration', - note: 'Reads baseball_player_stats + baseball_player_aggregates as model input for legacy insight generation.', - }, { path: 'src/app/baseball/actions/operational-signals.ts', group: 'server-action', @@ -159,24 +153,12 @@ export const GRANDFATHERED_CONSUMERS: GrandfatheredStatLayerConsumer[] = [ note: 'The sole remaining direct reader of baseball_player_aggregates for the roster surfaces (#379). Fetches the raw legacy row map for a team so legacy-stat-adapters.ts (via roster-aggregates-merge.ts) can resolve its box-score > legacy-fallback > no-data precedence; roster.ts (server) and RosterClient.tsx (browser) both call it instead of querying the deprecated table inline.', }, - { - path: 'src/lib/baseball/read-models/player-today.ts', - group: 'read-model', - status: 'pending migration', - note: 'Reads baseball_player_stats for "today" snapshot context.', - }, { path: 'src/lib/baseball/read-models/player-snapshot-cards.ts', group: 'read-model', status: 'pending migration', note: - 'Reads both baseball_player_aggregates and baseball_player_stats; comment flags exit-velocity fields as "typed but un-migrated".', - }, - { - path: 'src/lib/baseball/read-models/player-passport.ts', - group: 'read-model', - status: 'pending migration', - note: 'Reads baseball_player_stats for recent-activity counts on the passport card.', + '#379 (partial): exit-velocity fields migrated off the deprecated flat stat table onto baseball_batted_ball_events via elite-stat-events.ts\'s own buildHitterMetrics aggregator — closes the former "typed but un-migrated" comment. Still reads baseball_player_aggregates for (a) the Hitting/Pitching season-average legacy-fallback tier and (b) the game/scrimmage/practice "Performance" card, which has no canonical replacement yet: stats-center.ts exposes official-vs-all splits, not a standalone scrimmage split, and neither canonical layer has a practice-session shape (see docs/baseball/stats-migration-plan.md\'s open practice-shape question). Full migration blocked on that decision, not on adapter availability.', }, { path: 'src/lib/baseball/read-models/command-center.ts', @@ -333,6 +315,13 @@ export const GRANDFATHERED_CONSUMERS: GrandfatheredStatLayerConsumer[] = [ note: 'Regression coverage for PR #664 (roster-scoped playerId verification + honest failed-upload status) on uploadStatsCSV in stats.ts, an already-grandfathered consumer above. Uses a table-aware Supabase recorder that inserts into baseball_player_stats and upserts baseball_player_aggregates to mirror that production write path — mirrors imports-registry.test.ts above; production reference is the server-action entry for stats.ts, not a new one.', }, + { + path: 'src/app/baseball/actions/__tests__/practice-effectiveness.test.ts', + group: 'test', + status: 'pending migration', + note: + 'Action-level coverage (#825) for practice-effectiveness.ts, an already-grandfathered consumer above. Its fake-supabase fixture seeds an (empty) baseball_player_stats table to mirror that action\'s practice-type read path; migrates in lockstep with the production file. Added to the manifest post-merge — the #825 PR landed without an entry, tripping the contract test\'s scan.', + }, { path: 'src/contracts/baseball/product-trust.contract.test.ts', group: 'test', From 7ac6198c3af05457931098372f6e000d98a72f59 Mon Sep 17 00:00:00 2001 From: Fable Integrator Date: Wed, 15 Jul 2026 05:47:37 -0400 Subject: [PATCH 2/2] fix(baseball): restore box-score > legacy-fallback > no-data precedence in #379 read paths (#845 review fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #379 migration of player-today.ts/player-passport.ts/player-snapshot- cards.ts onto the canonical box-score/event layer regressed legacy-only players — real history captured before the box-score/event-grain pipelines existed — from "shows real data" to a silent, honest-LOOKING empty state. None of the three read paths ever consulted the deprecated flat table as a fallback, so a genuine data-migration artifact was indistinguishable from a player who truly has no activity. - player-today.ts: fetchRecentBoxScoreActivity now falls back to baseball_player_stats ONLY when this player has zero box-score rows, restoring the pre-#379 stamped-provenance trust/provenance build. Every PlayerTodayStat entry now carries `sourceLayer` ('box-score' | 'legacy-fallback') so the UI can label an old number honestly. - player-passport.ts: fetchRecentActivity (renamed from fetchRecentBoxScoreActivity) applies the same fallback to recentActivity.capturedSessions/lastSessionDate/lastSessionSource/ lastSessionTrust/lastSessionProvenance and the compact-mode 'Captured stats' completeness signal. recentActivity now carries `sourceLayer` too. - player-snapshot-cards.ts: avgExitVelocity/maxExitVelocity fall back to the deprecated baseball_player_stats.exit_velocity column ONLY when this player has zero baseball_batted_ball_events rows. Extracted the precedence logic into a new pure, exported resolveExitVelocityFields() so it's directly unit-testable (the DB-bound getPlayerSnapshotCards itself stays integration-only, per this file's existing test strategy). All three mirror the box-score > legacy-fallback > no-data precedence legacy-stat-adapters.ts already enforces for aggregate rows; the two sources are never blended for the same player. Manifest: re-added grandfathered entries for player-today.ts and player-passport.ts (both now reference baseball_player_stats again, as an intentional fallback-only read), updated player-snapshot-cards.ts's note, and added entries for the two test files whose fixtures now reference the deprecated table. Tests: extended player-today-honest-loop.test.ts with a legacy-fallback describe block (shows real data, real stamped provenance, box-score precedence over legacy), added player-passport-recent-activity.test.ts (no prior DB-path coverage existed for getPlayerPassport), and added resolveExitVelocityFields unit tests to player-snapshot-cards.test.ts. 574 test files / 5206 tests green across src/lib/baseball + src/contracts/baseball + src/app/baseball. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa --- .../player-today-honest-loop.test.ts | 133 ++++++++++ .../player-passport-recent-activity.test.ts | 178 +++++++++++++ .../__tests__/player-snapshot-cards.test.ts | 34 ++- .../baseball/read-models/player-passport.ts | 240 ++++++++++++++---- .../read-models/player-snapshot-cards.ts | 68 ++++- src/lib/baseball/read-models/player-today.ts | 162 +++++++++--- src/lib/baseball/stat-layer-manifest.ts | 30 ++- 7 files changed, 763 insertions(+), 82 deletions(-) create mode 100644 src/lib/baseball/read-models/__tests__/player-passport-recent-activity.test.ts diff --git a/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts b/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts index 725f3b3eb..71b9b25ef 100644 --- a/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts +++ b/src/contracts/baseball/product-trust/player-today-honest-loop.test.ts @@ -22,6 +22,14 @@ // box-score row carries no CSV-import provenance columns at all, so // every recentStats entry gets an honest trust:null/provenance:null // rather than an implied import lineage that doesn't exist. +// 5. #845 review fix: a player with ZERO box-score rows but real history in +// the deprecated baseball_player_stats table must still see REAL +// recentStats entries (sourceLayer:'legacy-fallback'), never a silent +// honest-LOOKING empty list caused by the #379 migration itself — the +// same box-score > legacy-fallback > no-data precedence +// legacy-stat-adapters.ts enforces for aggregate rows. A legacy row's +// real stamped provenance (when it has any) surfaces too, never +// flattened to null just because it came through the fallback path. // // Source of truth: `getPlayerToday` in // src/lib/baseball/read-models/player-today.ts. @@ -282,3 +290,128 @@ describe('getPlayerToday — recent stats never fabricate provenance for a box-s expect(result.error).toBe('Your recent stats could not be loaded.'); }); }); + +describe('getPlayerToday — legacy-fallback recentStats for a legacy-only player (#845)', () => { + // #845 review fix: the #379 migration onto the box-score layer regressed a + // player with real PRE-box-score history (zero box-score rows, real + // baseball_player_stats rows) from "shows real recent stats" to a silent, + // honest-LOOKING empty list — the migration itself was the bug, not a + // genuine absence of activity. fetchRecentBoxScoreActivity now falls back + // to baseball_player_stats ONLY when box-score is empty for this player. + it('zero box-score rows, real baseball_player_stats rows -> recentStats shows REAL data, not an honest-looking empty', async () => { + fake = createFakeSupabase({ + user: { id: USER_ID }, + tables: baseTables({ + baseball_player_stats: [ + { + id: 'stat-1', + player_id: PLAYER_ID, + team_id: TEAM_ID, + stat_type: 'game', + session_date: '2026-03-10', + session_name: 'vs Rival High', + source: 'manual', + source_trust_level: null, + source_match_tier: null, + source_match_confidence: null, + source_external_id: null, + import_run_id: null, + }, + { + id: 'stat-2', + player_id: PLAYER_ID, + team_id: TEAM_ID, + stat_type: 'practice', + session_date: '2026-03-01', + session_name: null, + source: 'manual', + source_trust_level: null, + source_match_tier: null, + source_match_confidence: null, + source_external_id: null, + import_run_id: null, + }, + ], + }), + }); + + const result = await getPlayerToday(TEAM_ID, { forDate: DAY }); + expect(result.recentStats).toHaveLength(2); + expect(result.recentStats.every((s) => s.sourceLayer === 'legacy-fallback')).toBe(true); + // Newest-first, same ordering contract as the box-score path. + expect(result.recentStats[0]?.id).toBe('stat-1'); + expect(result.recentStats[0]?.sessionName).toBe('vs Rival High'); + expect(result.error).toBeNull(); + }); + + it('an imported legacy row surfaces its REAL stamped trust/provenance (never flattened to null just because it came through the fallback)', async () => { + fake = createFakeSupabase({ + user: { id: USER_ID }, + tables: baseTables({ + baseball_player_stats: [ + { + id: 'stat-3', + player_id: PLAYER_ID, + team_id: TEAM_ID, + stat_type: 'game', + session_date: '2026-02-01', + session_name: 'Imported batch', + source: 'csv_import', + source_trust_level: 'unreviewed', + source_match_tier: 'exact_roster', + source_match_confidence: 0.92, + source_external_id: 'ext-1', + import_run_id: 'run-1', + }, + ], + baseball_import_runs: [{ id: 'run-1', review_state: 'pending_review' }], + }), + }); + + const result = await getPlayerToday(TEAM_ID, { forDate: DAY }); + const row = result.recentStats.find((s) => s.id === 'stat-3'); + expect(row?.sourceLayer).toBe('legacy-fallback'); + expect(row?.trust).not.toBeNull(); + expect(row?.provenance).not.toBeNull(); + }); + + it('a box-score row present -> box-score wins outright, even with legacy rows also present (never blended)', async () => { + fake = createFakeSupabase({ + user: { id: USER_ID }, + tables: baseTables({ + baseball_box_score_batting: [ + { id: 'bb-1', game_id: 'game-1', player_id: PLAYER_ID, team_id: TEAM_ID }, + ], + baseball_games: [ + { + id: 'game-1', + team_id: TEAM_ID, + game_date: DAY, + game_type: 'official_game', + opponent_name: 'Rival High', + }, + ], + baseball_player_stats: [ + { + id: 'stat-old', + player_id: PLAYER_ID, + team_id: TEAM_ID, + stat_type: 'game', + session_date: '2020-01-01', + session_name: 'Ancient session', + source: 'manual', + source_trust_level: null, + source_match_tier: null, + source_match_confidence: null, + source_external_id: null, + import_run_id: null, + }, + ], + }), + }); + + const result = await getPlayerToday(TEAM_ID, { forDate: DAY }); + expect(result.recentStats.every((s) => s.sourceLayer === 'box-score')).toBe(true); + expect(result.recentStats.find((s) => s.id === 'stat-old')).toBeUndefined(); + }); +}); diff --git a/src/lib/baseball/read-models/__tests__/player-passport-recent-activity.test.ts b/src/lib/baseball/read-models/__tests__/player-passport-recent-activity.test.ts new file mode 100644 index 000000000..63604805c --- /dev/null +++ b/src/lib/baseball/read-models/__tests__/player-passport-recent-activity.test.ts @@ -0,0 +1,178 @@ +// ============================================================================= +// src/lib/baseball/read-models/__tests__/player-passport-recent-activity.test.ts +// +// PRODUCT TRUTH THIS FILE PINS (#845 review fix, post-#379): +// getPlayerPassport's recentActivity (capturedSessions/lastSessionDate/ +// lastSessionSource/lastSessionTrust/lastSessionProvenance/sourceLayer) and +// the compact-mode completeness engine's 'stats' signal must resolve via +// the SAME box-score > legacy-fallback > no-data precedence +// legacy-stat-adapters.ts enforces elsewhere: a player with real history +// captured before the box-score pipeline existed (zero +// baseball_box_score_batting/_pitching rows, real baseball_player_stats +// rows) must show REAL recent-activity data — never a silent honest-LOOKING +// empty caused by the #379 migration itself. +// +// Source of truth: fetchRecentActivity + getPlayerPassport in +// src/lib/baseball/read-models/player-passport.ts. +// +// Mocks ONLY '@/lib/supabase/server', mirroring the sibling +// player-today-honest-loop.test.ts contract-test pattern for player-today.ts. +// ============================================================================= + +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { createFakeSupabase, type FakeSupabase } from '@/test/fixtures/fake-supabase'; + +let fake: FakeSupabase; + +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn(async () => fake), +})); + +import { getPlayerPassport } from '@/lib/baseball/read-models/player-passport'; + +const TEAM_ID = 'team-1'; +const PLAYER_ID = 'player-1'; +const USER_ID = 'user-1'; + +type Row = Record; + +function baseTables(extra: Record = {}): Record { + return { + baseball_players: [ + { id: PLAYER_ID, user_id: USER_ID, first_name: 'Jordan', last_name: 'Rivera' }, + ], + baseball_coaches: [], + baseball_player_passport_settings: [], + // #379 — recentActivity now sources from the canonical box-score layer, + // falling back to the deprecated flat table (#845 review fix, below). + baseball_box_score_batting: [], + baseball_box_score_pitching: [], + baseball_games: [], + baseball_player_stats: [], + baseball_import_runs: [], + ...extra, + }; +} + +beforeEach(() => { + fake = createFakeSupabase({ user: { id: USER_ID }, tables: baseTables() }); +}); + +describe('getPlayerPassport — recentActivity legacy fallback for a legacy-only player (#845)', () => { + it('zero box-score rows, real baseball_player_stats rows -> recentActivity shows REAL data, not an honest-looking empty', async () => { + fake = createFakeSupabase({ + user: { id: USER_ID }, + tables: baseTables({ + baseball_player_stats: [ + { + id: 'stat-1', + player_id: PLAYER_ID, + team_id: TEAM_ID, + session_date: '2026-03-10', + session_name: 'vs Rival High', + source: 'manual', + source_trust_level: null, + source_match_tier: null, + source_match_confidence: null, + source_external_id: null, + import_run_id: null, + }, + { + id: 'stat-2', + player_id: PLAYER_ID, + team_id: TEAM_ID, + session_date: '2026-03-01', + session_name: null, + source: 'manual', + source_trust_level: null, + source_match_tier: null, + source_match_confidence: null, + source_external_id: null, + import_run_id: null, + }, + ], + }), + }); + + const result = await getPlayerPassport(TEAM_ID); + expect(result.authorized).toBe(true); + expect(result.recentActivity.sourceLayer).toBe('legacy-fallback'); + expect(result.recentActivity.capturedSessions).toBe(2); + expect(result.recentActivity.lastSessionDate).toBe('2026-03-10'); + expect(result.completeness.signals.find((s) => s.section === 'stats')?.complete).toBe(true); + expect(result.error).toBeNull(); + }); + + it('an imported legacy row surfaces its REAL stamped trust/provenance (never flattened to null just because it came through the fallback)', async () => { + fake = createFakeSupabase({ + user: { id: USER_ID }, + tables: baseTables({ + baseball_player_stats: [ + { + id: 'stat-3', + player_id: PLAYER_ID, + team_id: TEAM_ID, + session_date: '2026-02-01', + session_name: 'Imported batch', + source: 'csv_import', + source_trust_level: 'unreviewed', + source_match_tier: 'exact_roster', + source_match_confidence: 0.92, + source_external_id: 'ext-1', + import_run_id: 'run-1', + }, + ], + baseball_import_runs: [{ id: 'run-1', review_state: 'pending_review' }], + }), + }); + + const result = await getPlayerPassport(TEAM_ID); + expect(result.recentActivity.sourceLayer).toBe('legacy-fallback'); + expect(result.recentActivity.lastSessionTrust).not.toBeNull(); + expect(result.recentActivity.lastSessionProvenance).not.toBeNull(); + }); + + it('a box-score row present -> box-score wins outright, even with legacy rows also present (never blended)', async () => { + fake = createFakeSupabase({ + user: { id: USER_ID }, + tables: baseTables({ + baseball_box_score_batting: [ + { id: 'bb-1', game_id: 'game-1', player_id: PLAYER_ID, team_id: TEAM_ID }, + ], + baseball_games: [ + { id: 'game-1', team_id: TEAM_ID, game_date: '2026-04-01', opponent_name: 'Rival High' }, + ], + baseball_player_stats: [ + { + id: 'stat-old', + player_id: PLAYER_ID, + team_id: TEAM_ID, + session_date: '2020-01-01', + session_name: 'Ancient session', + source: 'manual', + source_trust_level: null, + source_match_tier: null, + source_match_confidence: null, + source_external_id: null, + import_run_id: null, + }, + ], + }), + }); + + const result = await getPlayerPassport(TEAM_ID); + expect(result.recentActivity.sourceLayer).toBe('box-score'); + expect(result.recentActivity.lastSessionDate).toBe('2026-04-01'); + // Box-score rows carry no CSV-import provenance — honest null, never the + // legacy row's stamped values leaking through. + expect(result.recentActivity.lastSessionTrust).toBeNull(); + }); + + it('zero box-score AND zero legacy rows -> honest no-data, never fabricated', async () => { + const result = await getPlayerPassport(TEAM_ID); + expect(result.recentActivity.sourceLayer).toBe('no-data'); + expect(result.recentActivity.capturedSessions).toBe(0); + expect(result.recentActivity.lastSessionDate).toBeNull(); + expect(result.completeness.signals.find((s) => s.section === 'stats')?.complete).toBe(false); + }); +}); diff --git a/src/lib/baseball/read-models/__tests__/player-snapshot-cards.test.ts b/src/lib/baseball/read-models/__tests__/player-snapshot-cards.test.ts index 1146748a7..6368d2b30 100644 --- a/src/lib/baseball/read-models/__tests__/player-snapshot-cards.test.ts +++ b/src/lib/baseball/read-models/__tests__/player-snapshot-cards.test.ts @@ -10,7 +10,7 @@ // ============================================================================= import { describe, it, expect } from 'vitest'; -import { inferRole, collapseBand, roleLabel } from '../player-snapshot-cards'; +import { inferRole, collapseBand, roleLabel, resolveExitVelocityFields } from '../player-snapshot-cards'; describe('inferRole — role drives which performance cards show', () => { it('a position player with only hitting data is a hitter (never an empty Pitching card)', () => { @@ -90,3 +90,35 @@ describe('roleLabel — baseball-only, no golf terms', () => { expect(roleLabel('utility')).toBe('Utility'); }); }); + +describe('resolveExitVelocityFields — box-score > legacy-fallback > no-data (#845 review fix)', () => { + it('a legacy-only player (zero batted-ball events, real legacy exit-velocity rows) gets REAL numbers, not an honest-looking null', () => { + const result = resolveExitVelocityFields(null, null, false, [88, 92, 90]); + expect(result.avgExitVelocity).toBeCloseTo(90, 5); + expect(result.maxExitVelocity).toBe(92); + }); + + it('any batted-ball events present -> event-grain numbers win outright, even with legacy values also on file (never blended)', () => { + const result = resolveExitVelocityFields(95.5, 101.2, true, [70, 71, 72]); + expect(result.avgExitVelocity).toBe(95.5); + expect(result.maxExitVelocity).toBe(101.2); + }); + + it('batted-ball events present but the event aggregator itself found no EV metric -> honest null, never a legacy substitution', () => { + const result = resolveExitVelocityFields(null, null, true, [80, 85]); + expect(result.avgExitVelocity).toBeNull(); + expect(result.maxExitVelocity).toBeNull(); + }); + + it('zero batted-ball events AND zero legacy exit-velocity rows -> honest null, never fabricated', () => { + const result = resolveExitVelocityFields(null, null, false, []); + expect(result.avgExitVelocity).toBeNull(); + expect(result.maxExitVelocity).toBeNull(); + }); + + it('filters out non-finite legacy values before averaging (defensive, matches the production query\'s honesty filter)', () => { + const result = resolveExitVelocityFields(null, null, false, [90, Number.NaN, 94]); + expect(result.avgExitVelocity).toBe(92); + expect(result.maxExitVelocity).toBe(94); + }); +}); diff --git a/src/lib/baseball/read-models/player-passport.ts b/src/lib/baseball/read-models/player-passport.ts index 79a41d9e8..845c7f9d0 100644 --- a/src/lib/baseball/read-models/player-passport.ts +++ b/src/lib/baseball/read-models/player-passport.ts @@ -42,9 +42,15 @@ import { type CaptureMode, } from '@/lib/baseball/source-record'; import { buildSourceTrust } from '@/components/baseball/source-trust/build-source-trust'; +import { + buildStampedSourceTrust, + buildImportProvenance, + type StampedStatProvenance, +} from '@/components/baseball/source-trust/stamped-trust'; import { getPlayerTimeline } from '@/lib/baseball/read-models/timeline'; import { getVideoLibrary } from '@/lib/baseball/read-models/video-classes'; import { sumInningsPitched, ipToInnings } from '@/lib/baseball/innings'; +import type { SourceLayer } from '@/lib/baseball/read-models/legacy-stat-adapters'; import type { SourceTrust, SourceProvenance, @@ -246,10 +252,20 @@ export interface PassportReadModel { * GAP 3 — render-ready trust + provenance for the most-recent session built * from its import-stamped columns, so the passport's "from" line can carry the * same SourceTrustBadge + drawer (coach side). Null when the latest session - * was hand-entered (no stamped provenance). + * was hand-entered (no stamped provenance), which is always true for + * box-score-sourced sessions and only sometimes true for legacy-fallback ones. */ lastSessionTrust: SourceTrust | null; lastSessionProvenance: SourceProvenance | null; + /** + * Which layer these counts/last-session fields came from — 'box-score' + * (canonical), 'legacy-fallback' (deprecated flat baseball_player_stats + * table, read ONLY when this player has zero box-score-era games; #379), + * or 'no-data' when neither has any rows for this player. Mirrors + * legacy-stat-adapters.ts's precedence so a legacy number is never shown + * as if it were equally fresh as a box-score one. + */ + sourceLayer: SourceLayer; }; /** * V5 Development Story (timeline). Populated only in mode:'full'. Viewer- @@ -439,21 +455,44 @@ interface RecentBoxScoreGame { opponent_name: string | null; } +/** The recentActivity-shaped result {@link fetchRecentActivity} resolves. */ +interface RecentActivityResult { + capturedSessions: number; + lastSessionDate: string | null; + lastSessionSource: SourceRef | null; + lastSessionTrust: SourceTrust | null; + lastSessionProvenance: SourceProvenance | null; + sourceLayer: SourceLayer; + error: string | null; +} + /** - * This player's most recent games with a captured box-score line (batting OR - * pitching), newest first, capped at `limit`. Reads - * baseball_box_score_batting/_pitching (game ids only) then baseball_games - * for the display fields — the canonical layer-2 tables, per #379's - * migration of this read model's "recent activity" card off the deprecated - * flat/aggregate stat layer. Degrades to an honest empty list + error string - * on a sub-read failure. + * This player's recent-activity summary, sourced with the same box-score > + * legacy-fallback > no-data precedence legacy-stat-adapters.ts enforces for + * aggregate rows: + * + * 1. baseball_box_score_batting/_pitching (game ids only) joined to + * baseball_games for the display fields — the canonical layer-2 tables, + * per #379's migration of this read model's "recent activity" card off + * the deprecated flat/aggregate stat layer. Box-score rows carry no + * CSV-import provenance, so lastSessionTrust/Provenance are honestly null. + * 2. ONLY when this player has ZERO box-score rows: the deprecated + * baseball_player_stats table (same table + columns this read model + * queried pre-#379) — so a player with real history captured before the + * box-score pipeline existed doesn't regress from "shows real recent + * activity" to an honest-LOOKING empty count that is actually a + * data-migration artifact. Carries its real stamped provenance when the + * most recent row has any. + * 3. Neither: an honest zero/null "no-data" result. + * + * Degrades to an honest empty result + error string on a sub-read failure. */ -async function fetchRecentBoxScoreActivity( +async function fetchRecentActivity( supabase: Awaited>, playerId: string, teamId: string, limit: number, -): Promise<{ data: RecentBoxScoreGame[]; error: string | null }> { +): Promise { // eslint-disable-next-line @typescript-eslint/no-explicit-any const db = supabase as any; const [battingRes, pitchingRes] = await Promise.all([ @@ -469,7 +508,15 @@ async function fetchRecentBoxScoreActivity( .eq('team_id', teamId), ]); if (battingRes.error || pitchingRes.error) { - return { data: [], error: 'Recent activity could not be loaded.' }; + return { + capturedSessions: 0, + lastSessionDate: null, + lastSessionSource: null, + lastSessionTrust: null, + lastSessionProvenance: null, + sourceLayer: 'no-data', + error: 'Recent activity could not be loaded.', + }; } const gameIds = [ @@ -478,17 +525,118 @@ async function fetchRecentBoxScoreActivity( ...((pitchingRes.data ?? []) as Array<{ game_id: string }>).map((r) => r.game_id), ]), ]; - if (gameIds.length === 0) return { data: [], error: null }; - const { data: games, error: gamesErr } = await db - .from('baseball_games') - .select('id, game_date, opponent_name') - .in('id', gameIds) - .order('game_date', { ascending: false }) + if (gameIds.length > 0) { + const { data: games, error: gamesErr } = await db + .from('baseball_games') + .select('id, game_date, opponent_name') + .in('id', gameIds) + .order('game_date', { ascending: false }) + .limit(limit); + if (gamesErr) { + return { + capturedSessions: 0, + lastSessionDate: null, + lastSessionSource: null, + lastSessionTrust: null, + lastSessionProvenance: null, + sourceLayer: 'no-data', + error: 'Recent activity could not be loaded.', + }; + } + + const recentGames = (games ?? []) as RecentBoxScoreGame[]; + const lastGame = recentGames[0] ?? null; + return { + capturedSessions: recentGames.length, + lastSessionDate: lastGame?.game_date ?? null, + lastSessionSource: lastGame + ? buildSourceRef({ source: 'manual', sourceId: lastGame.id, label: 'Box score' }) + : null, + lastSessionTrust: null, + lastSessionProvenance: null, + sourceLayer: 'box-score', + error: null, + }; + } + + // Legacy fallback — no box-score rows at all for this player. Stamped + // provenance columns aren't in generated database.ts -> untyped client, + // same as the pre-#379 read of this table. + const { data: legacyRows, error: legacyErr } = await db + .from('baseball_player_stats') + .select( + 'session_date, session_name, source, source_trust_level, source_match_tier, source_match_confidence, source_external_id, import_run_id', + ) + .eq('player_id', playerId) + .eq('team_id', teamId) + .order('session_date', { ascending: false }) .limit(limit); - if (gamesErr) return { data: [], error: 'Recent activity could not be loaded.' }; + if (legacyErr) { + return { + capturedSessions: 0, + lastSessionDate: null, + lastSessionSource: null, + lastSessionTrust: null, + lastSessionProvenance: null, + sourceLayer: 'no-data', + error: 'Recent activity could not be loaded.', + }; + } - return { data: (games ?? []) as RecentBoxScoreGame[], error: null }; + const statRows = (legacyRows ?? []) as unknown as Array< + StampedStatProvenance & { session_date: string; session_name: string | null } + >; + // Destructure-then-guard (rather than a `.length === 0` check) so TS can + // narrow `lastSession` to defined for the rest of this function. + const [lastSession] = statRows; + if (!lastSession) { + return { + capturedSessions: 0, + lastSessionDate: null, + lastSessionSource: null, + lastSessionTrust: null, + lastSessionProvenance: null, + sourceLayer: 'no-data', + error: null, + }; + } + let lastSessionTrust: SourceTrust | null = null; + let lastSessionProvenance: SourceProvenance | null = null; + if (lastSession.import_run_id || lastSession.source_trust_level) { + let reviewState: string | null = null; + if (lastSession.import_run_id) { + const { data: run } = await db + .from('baseball_import_runs') + .select('review_state') + .eq('id', lastSession.import_run_id) + .maybeSingle(); + reviewState = (run as { review_state: string | null } | null)?.review_state ?? null; + } + const stamped: StampedStatProvenance = { + source: lastSession.source, + source_trust_level: lastSession.source_trust_level, + source_match_tier: lastSession.source_match_tier, + source_match_confidence: lastSession.source_match_confidence, + source_external_id: lastSession.source_external_id, + import_run_id: lastSession.import_run_id, + review_state: reviewState, + importedAt: lastSession.session_date, + }; + const label = lastSession.session_name?.trim() || 'Imported stats'; + lastSessionTrust = buildStampedSourceTrust(stamped, label); + lastSessionProvenance = buildImportProvenance(stamped, { label }); + } + + return { + capturedSessions: statRows.length, + lastSessionDate: lastSession.session_date ?? null, + lastSessionSource: buildSourceRef({ source: lastSession.source }), + lastSessionTrust, + lastSessionProvenance, + sourceLayer: 'legacy-fallback', + error: null, + }; } // ----------------------------------------------------------------------------- @@ -613,6 +761,7 @@ export async function getPlayerPassport( lastSessionSource: null, lastSessionTrust: null, lastSessionProvenance: null, + sourceLayer: 'no-data', }, developmentStory: null, media: null, @@ -639,7 +788,7 @@ export async function getPlayerPassport( : 'other'; // ---- Load player identity + measurables, settings, recent activity ---- - const [playerRes, settingsRes, statsRes] = await Promise.all([ + const [playerRes, settingsRes, activityRes] = await Promise.all([ supabase .from('baseball_players') .select( @@ -656,10 +805,11 @@ export async function getPlayerPassport( .eq('team_id', teamId) .maybeSingle(), // #379 — recent activity is sourced from the canonical box-score layer - // (baseball_box_score_batting/_pitching joined to baseball_games), not the - // deprecated flat/aggregate stat layer this read model used before its - // #379 migration. - fetchRecentBoxScoreActivity(supabase, targetPlayerId, teamId, 50), + // (baseball_box_score_batting/_pitching joined to baseball_games) when any + // exists for this player, falling back to the deprecated flat/aggregate + // stat layer ONLY when this player has zero box-score rows (see + // fetchRecentActivity's doc comment). + fetchRecentActivity(supabase, targetPlayerId, teamId, 50), ]); if (playerRes.error || !playerRes.data) { @@ -769,22 +919,21 @@ export async function getPlayerPassport( // ---- Recent activity (counts only) ---- // #379 — sourced from the canonical box-score/season layer (games this - // player has a captured batting or pitching line for), not the deprecated - // flat/aggregate stat layer. Box-score rows carry no CSV-import provenance - // columns (staff-entered via the box-score save flow, not imported), so - // lastSessionTrust/lastSessionProvenance are honestly null rather than a - // fabricated import stamp. - const recentGames = statsRes.error ? [] : statsRes.data; - const lastGame = recentGames[0] ?? null; - + // player has a captured batting or pitching line for) when any exist, + // falling back to the deprecated flat/aggregate stat layer ONLY when this + // player has zero box-score rows (see fetchRecentActivity's doc comment) — + // the same box-score > legacy-fallback > no-data precedence + // legacy-stat-adapters.ts enforces. Box-score rows carry no CSV-import + // provenance columns (staff-entered via the box-score save flow, not + // imported), so lastSessionTrust/lastSessionProvenance are honestly null for + // those; legacy-fallback rows carry their real stamped provenance. const recentActivity = { - capturedSessions: recentGames.length, - lastSessionDate: lastGame?.game_date ?? null, - lastSessionSource: lastGame - ? buildSourceRef({ source: 'manual', sourceId: lastGame.id, label: 'Box score' }) - : null, - lastSessionTrust: null, - lastSessionProvenance: null, + capturedSessions: activityRes.capturedSessions, + lastSessionDate: activityRes.lastSessionDate, + lastSessionSource: activityRes.lastSessionSource, + lastSessionTrust: activityRes.lastSessionTrust, + lastSessionProvenance: activityRes.lastSessionProvenance, + sourceLayer: activityRes.sourceLayer, }; // --------------------------------------------------------------------------- @@ -855,15 +1004,16 @@ export async function getPlayerPassport( section: 'stats', label: 'Captured stats', // In full mode we know the real game-log count; in compact we fall back to - // the captured-session count (#379 — box-score-sourced). Either way this - // is a real signal, not a guess. - complete: mode === 'full' ? performanceGameCount > 0 : recentGames.length > 0, + // the captured-session count (#379 — box-score-sourced, or legacy-fallback + // per fetchRecentActivity when this player has no box-score rows). Either + // way this is a real signal, not a guess. + complete: mode === 'full' ? performanceGameCount > 0 : recentActivity.capturedSessions > 0, note: mode === 'full' ? performanceGameCount > 0 ? `${performanceGameCount} game log${performanceGameCount === 1 ? '' : 's'} on file.` : 'No box-score game logs yet.' - : recentGames.length > 0 + : recentActivity.capturedSessions > 0 ? 'Complete.' : 'No captured stat sessions yet.', }, @@ -927,9 +1077,7 @@ export async function getPlayerPassport( completeness: { percent: completePercent, signals }, withheldFieldCount: withheld, authorized: true, - error: statsRes.error - ? 'Recent activity could not be loaded.' - : sectionError, + error: activityRes.error ?? sectionError, }; } diff --git a/src/lib/baseball/read-models/player-snapshot-cards.ts b/src/lib/baseball/read-models/player-snapshot-cards.ts index 2e565c702..bc953cfc3 100644 --- a/src/lib/baseball/read-models/player-snapshot-cards.ts +++ b/src/lib/baseball/read-models/player-snapshot-cards.ts @@ -429,6 +429,42 @@ function bandLabel(band: SnapshotReadinessChip['band']): string | null { } } +// ----------------------------------------------------------------------------- +// Exit velocity: box-score > legacy-fallback > no-data (#845 review fix) +// ----------------------------------------------------------------------------- + +/** + * Resolve avgExitVelocity/maxExitVelocity per the same box-score > + * legacy-fallback > no-data precedence legacy-stat-adapters.ts enforces + * elsewhere in #379: the event-grain (batted-ball) numbers win outright + * whenever this player has ANY batted-ball events captured; the deprecated + * baseball_player_stats.exit_velocity column is consulted ONLY when there are + * none — so a legacy-only player (real EV numbers captured before the + * event-grain model existed) doesn't regress from "shows a real number" to an + * honest-LOOKING null. The two sources are never blended for the same player. + * Pure function — no I/O — so it's directly unit-testable (see + * player-snapshot-cards.test.ts) even though the DB-bound + * getPlayerSnapshotCards itself is exercised via RLS/integration, not here. + */ +export function resolveExitVelocityFields( + eventAvgExitVelocity: number | null, + eventMaxExitVelocity: number | null, + hasBattedBallEvents: boolean, + legacyExitVelocityValues: number[], +): { avgExitVelocity: number | null; maxExitVelocity: number | null } { + if (hasBattedBallEvents) { + return { avgExitVelocity: eventAvgExitVelocity, maxExitVelocity: eventMaxExitVelocity }; + } + const values = legacyExitVelocityValues.filter((v) => v != null && Number.isFinite(v)); + if (values.length === 0) { + return { avgExitVelocity: null, maxExitVelocity: null }; + } + return { + avgExitVelocity: Math.round((values.reduce((a, b) => a + b, 0) / values.length) * 10) / 10, + maxExitVelocity: Math.round(Math.max(...values) * 10) / 10, + }; +} + // ----------------------------------------------------------------------------- // Staff resolution (same link the timeline uses) // ----------------------------------------------------------------------------- @@ -600,6 +636,7 @@ export async function getPlayerSnapshotCards( tasksRes, devPlanRes, evRes, + evLegacyRes, ] = await Promise.all([ supabase.from('baseball_players') .select('primary_position, secondary_position') @@ -688,6 +725,17 @@ export async function getPlayerSnapshotCards( .is('superseded_by_run_id', null) .order('created_at', { ascending: false }) .limit(500), + // Legacy fallback (#845 review fix) — ONLY consulted below when this + // player has zero baseball_batted_ball_events rows. Exit velocity lives + // on captured stat sessions in the deprecated flat table (real column, + // predates the event-grain model), NOT on baseball_player_aggregates + // (those EV fields are typed but un-migrated). Mirrors the box-score > + // legacy-fallback > no-data precedence legacy-stat-adapters.ts enforces, + // and this file's own pre-#379 query. + supabase.from('baseball_player_stats') + .select('exit_velocity') + .eq('player_id', playerId).eq('team_id', teamId) + .not('exit_velocity', 'is', null).limit(200), ]); // Exit-velocity aggregation from real batted-ball events (honest: null when @@ -697,12 +745,28 @@ export async function getPlayerSnapshotCards( const hitterMetrics = buildHitterMetrics(playerId, [], battedBalls, 'official_game'); const rawAvgExitVelocity = hitterMetrics.metrics.find((m) => m.metricKey === 'avg_exit_velocity')?.value ?? null; - const avgExitVelocity = + const eventAvgExitVelocity = rawAvgExitVelocity != null ? Math.round(rawAvgExitVelocity * 10) / 10 : null; const evValues = hitterMetrics.visuals.evLaPoints .map((p) => p.exitVelocity) .filter((v): v is number => v != null && Number.isFinite(v)); - const maxExitVelocity = evValues.length ? Math.round(Math.max(...evValues) * 10) / 10 : null; + const eventMaxExitVelocity = evValues.length ? Math.round(Math.max(...evValues) * 10) / 10 : null; + + // Legacy fallback (#845 review fix) — see resolveExitVelocityFields's doc + // comment for the precedence rule. + const legacyEvRows = (evLegacyRes?.error ? [] : evLegacyRes?.data ?? []) as Array<{ + exit_velocity: number | null; + }>; + const legacyExitVelocityValues = legacyEvRows + .map((r) => r.exit_velocity) + .filter((v): v is number => v != null && Number.isFinite(v)); + + const { avgExitVelocity, maxExitVelocity } = resolveExitVelocityFields( + eventAvgExitVelocity, + eventMaxExitVelocity, + battedBalls.length > 0, + legacyExitVelocityValues, + ); // --------------------------------------------------------------------------- // Season stats -> Hitting + Pitching diff --git a/src/lib/baseball/read-models/player-today.ts b/src/lib/baseball/read-models/player-today.ts index c11e97a3f..9a3f1d80e 100644 --- a/src/lib/baseball/read-models/player-today.ts +++ b/src/lib/baseball/read-models/player-today.ts @@ -69,6 +69,12 @@ import type { SourceTrust, SourceProvenance, } from '@/components/baseball/source-trust/source-trust-types'; +import { + buildStampedSourceTrust, + buildImportProvenance, + type StampedStatProvenance, +} from '@/components/baseball/source-trust/stamped-trust'; +import type { SourceLayer } from '@/lib/baseball/read-models/legacy-stat-adapters'; import { computeReadiness, readinessBandLabel, @@ -173,12 +179,21 @@ export interface PlayerTodayStat { * Always null for box-score-sourced rows (#379): box-score entries are * staff-entered via the box-score save flow, not imported, so there is no * stamped import provenance to describe — an honest null, never a - * fabricated stamp. Kept as a real (not removed) field so the shape stays - * ready for a future source that does carry stamped provenance. + * fabricated stamp. Populated from the legacy row's own stamped columns when + * `sourceLayer` is 'legacy-fallback' (see {@link fetchRecentBoxScoreActivity}). */ trust: SourceTrust | null; /** Rich provenance for the drawer (opens the Import Dossier run). Same honesty note as `trust`. */ provenance: SourceProvenance | null; + /** + * Which layer this session came from — 'box-score' (canonical) or + * 'legacy-fallback' (the deprecated flat baseball_player_stats table, read + * ONLY when this player has zero box-score-era games; see #379 and + * legacy-stat-adapters.ts for the shared box-score > legacy-fallback > + * no-data precedence this mirrors). Lets the UI label an old number + * honestly instead of presenting it as equally fresh as a box-score row. + */ + sourceLayer: SourceLayer; } /** @@ -439,16 +454,27 @@ interface RecentBoxScoreGame { * pitching), newest first. Reads baseball_box_score_batting/_pitching (game * ids only) then baseball_games for the display fields — the canonical * layer-2 tables, per #379's migration of this read model off the deprecated - * flat/aggregate stat layer. Degrades to an honest empty list + error string - * on a sub-read failure, matching this read model's existing fault-tolerance - * convention. + * flat/aggregate stat layer. + * + * Legacy fallback (post-#845 review fix): when this player has ZERO box-score + * rows, this used to silently return an honest-LOOKING empty list even for a + * player with real history captured before the box-score pipeline existed — + * indistinguishable from a player who genuinely has no activity. We now fall + * back to the deprecated `baseball_player_stats` table in that case (same + * table + columns this read model queried pre-#379), mirroring the + * box-score > legacy-fallback > no-data precedence legacy-stat-adapters.ts + * enforces for aggregate rows. Every returned entry carries `sourceLayer` so + * the caller/UI can label a legacy number honestly instead of presenting it + * as equally fresh as a box-score row. Degrades to an honest empty list + + * error string on a sub-read failure, matching this read model's existing + * fault-tolerance convention. */ async function fetchRecentBoxScoreActivity( supabase: Awaited>, playerId: string, teamId: string, limit: number, -): Promise<{ data: RecentBoxScoreGame[]; error: string | null }> { +): Promise<{ data: PlayerTodayStat[]; error: string | null }> { const [battingRes, pitchingRes] = await Promise.all([ supabase .from('baseball_box_score_batting') @@ -471,17 +497,98 @@ async function fetchRecentBoxScoreActivity( ...(pitchingRes.data ?? []).map((r) => r.game_id), ]), ]; - if (gameIds.length === 0) return { data: [], error: null }; - const { data: games, error: gamesErr } = await supabase - .from('baseball_games') - .select('id, game_date, game_type, opponent_name') - .in('id', gameIds) - .order('game_date', { ascending: false }) + if (gameIds.length > 0) { + const { data: games, error: gamesErr } = await supabase + .from('baseball_games') + .select('id, game_date, game_type, opponent_name') + .in('id', gameIds) + .order('game_date', { ascending: false }) + .limit(limit); + if (gamesErr) return { data: [], error: 'Your recent stats could not be loaded.' }; + + const data: PlayerTodayStat[] = ((games ?? []) as RecentBoxScoreGame[]).map((g) => ({ + id: g.id, + statType: g.game_type, + sessionDate: g.game_date, + sessionName: g.opponent_name ? `vs ${g.opponent_name}` : null, + sourceRef: buildSourceRef({ source: 'manual', sourceId: g.id, label: 'Box score' }), + trust: null, + provenance: null, + sourceLayer: 'box-score', + })); + return { data, error: null }; + } + + // Legacy fallback — no box-score rows at all for this player. Stamped + // provenance columns aren't in generated database.ts -> untyped client, same + // as the pre-#379 read of this table. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const legacyDb = supabase as any; + const { data: legacyRows, error: legacyErr } = await legacyDb + .from('baseball_player_stats') + .select( + 'id, stat_type, session_date, session_name, source, source_trust_level, source_match_tier, source_match_confidence, source_external_id, import_run_id', + ) + .eq('player_id', playerId) + .eq('team_id', teamId) + .order('session_date', { ascending: false }) .limit(limit); - if (gamesErr) return { data: [], error: 'Your recent stats could not be loaded.' }; + if (legacyErr) return { data: [], error: 'Your recent stats could not be loaded.' }; + + const legacyStatRows = (legacyRows ?? []) as unknown as Array< + StampedStatProvenance & { + id: string; + stat_type: string; + session_date: string; + session_name: string | null; + } + >; + if (legacyStatRows.length === 0) return { data: [], error: null }; + + // Batched lookup of import-run review state for the imported rows, so the + // legacy entries' trust carries the same real "reviewed vs unreviewed" + // signal the pre-#379 read model surfaced. + const runIds = [...new Set(legacyStatRows.map((s) => s.import_run_id).filter(Boolean))] as string[]; + const reviewByRun = new Map(); + if (runIds.length > 0) { + const { data: runs } = await legacyDb + .from('baseball_import_runs') + .select('id, review_state') + .in('id', runIds); + for (const r of (runs ?? []) as Array<{ id: string; review_state: string | null }>) { + reviewByRun.set(r.id, r.review_state); + } + } + + const data: PlayerTodayStat[] = legacyStatRows.map((s) => { + const stamped: StampedStatProvenance = { + source: s.source, + source_trust_level: s.source_trust_level, + source_match_tier: s.source_match_tier, + source_match_confidence: s.source_match_confidence, + source_external_id: s.source_external_id, + import_run_id: s.import_run_id, + review_state: s.import_run_id ? reviewByRun.get(s.import_run_id) ?? null : null, + importedAt: s.session_date, + }; + // Only imported/device/official rows carry stamped provenance; a hand-entered + // line has no import_run_id and reads as a plain source label. + const hasStamp = !!s.import_run_id || !!s.source_trust_level; + const label = s.session_name?.trim() || 'Imported stats'; + return { + id: s.id, + statType: s.stat_type, + sessionDate: s.session_date, + sessionName: s.session_name, + sourceRef: buildSourceRef({ source: s.source }), + trust: hasStamp ? buildStampedSourceTrust(stamped, label) : null, + provenance: hasStamp ? buildImportProvenance(stamped, { label }) : null, + sourceLayer: 'legacy-fallback', + }; + }); - return { data: (games ?? []) as RecentBoxScoreGame[], error: null }; + return { data, error: null }; } // ----------------------------------------------------------------------------- @@ -818,26 +925,17 @@ export async function getPlayerToday( }); // ---- Recent stats (active captures) ---- - // #379 — sourced from box-score/season-era games (canonical layer 2), not - // the deprecated flat/aggregate stat layer. Box-score rows carry no - // CSV-import provenance columns (staff-entered via the box-score save flow, - // not imported), so trust/provenance are honestly null rather than a - // fabricated import stamp. - const recentStats: PlayerTodayStat[] = []; + // #379 — sourced from box-score/season-era games (canonical layer 2) when + // any exist for this player, falling back to the deprecated flat/aggregate + // stat layer ONLY when this player has zero box-score rows (see + // fetchRecentBoxScoreActivity's doc comment) — the same box-score > + // legacy-fallback > no-data precedence legacy-stat-adapters.ts enforces. + // Box-score rows carry no CSV-import provenance columns (staff-entered via + // the box-score save flow, not imported), so trust/provenance are honestly + // null for those; legacy-fallback rows carry their real stamped provenance. + const recentStats: PlayerTodayStat[] = statsRes.error ? [] : statsRes.data; if (statsRes.error) { error = error ?? statsRes.error; - } else { - for (const g of statsRes.data) { - recentStats.push({ - id: g.id, - statType: g.game_type, - sessionDate: g.game_date, - sessionName: g.opponent_name ? `vs ${g.opponent_name}` : null, - sourceRef: buildSourceRef({ source: 'manual', sourceId: g.id, label: 'Box score' }), - trust: null, - provenance: null, - }); - } } const eventsPendingAck = schedule.filter((e) => e.ackStatus === 'pending').length; diff --git a/src/lib/baseball/stat-layer-manifest.ts b/src/lib/baseball/stat-layer-manifest.ts index 7d387c802..356acfcfc 100644 --- a/src/lib/baseball/stat-layer-manifest.ts +++ b/src/lib/baseball/stat-layer-manifest.ts @@ -153,12 +153,26 @@ export const GRANDFATHERED_CONSUMERS: GrandfatheredStatLayerConsumer[] = [ note: 'The sole remaining direct reader of baseball_player_aggregates for the roster surfaces (#379). Fetches the raw legacy row map for a team so legacy-stat-adapters.ts (via roster-aggregates-merge.ts) can resolve its box-score > legacy-fallback > no-data precedence; roster.ts (server) and RosterClient.tsx (browser) both call it instead of querying the deprecated table inline.', }, + { + path: 'src/lib/baseball/read-models/player-today.ts', + group: 'read-model', + status: 'pending migration', + note: + "#845 review fix (post-#379): the initial #379 migration of this file's \"recent stats\" card onto baseball_box_score_batting/_pitching + baseball_games regressed a legacy-only player (real history captured before the box-score pipeline existed, zero box-score-era games since) from \"shows real recent stats\" to a silent, honest-LOOKING empty list. fetchRecentBoxScoreActivity now falls back to baseball_player_stats ONLY when this player has zero box-score rows (mirroring legacy-stat-adapters.ts's box-score > legacy-fallback > no-data precedence), tagging every entry's sourceLayer so the UI can label an old number honestly. See player-today-honest-loop.test.ts's legacy-fallback describe block.", + }, + { + path: 'src/lib/baseball/read-models/player-passport.ts', + group: 'read-model', + status: 'pending migration', + note: + "#845 review fix (post-#379): the initial #379 migration of the passport's recentActivity/completeness \"Captured stats\" signal onto baseball_box_score_batting/_pitching + baseball_games had the same legacy-only regression as player-today.ts (above). fetchRecentActivity now falls back to baseball_player_stats ONLY when this player has zero box-score rows, restoring the pre-#379 stamped-provenance last-session trust/provenance and tagging recentActivity.sourceLayer so the passport never shows an honest-LOOKING empty count for a legacy-only player.", + }, { path: 'src/lib/baseball/read-models/player-snapshot-cards.ts', group: 'read-model', status: 'pending migration', note: - '#379 (partial): exit-velocity fields migrated off the deprecated flat stat table onto baseball_batted_ball_events via elite-stat-events.ts\'s own buildHitterMetrics aggregator — closes the former "typed but un-migrated" comment. Still reads baseball_player_aggregates for (a) the Hitting/Pitching season-average legacy-fallback tier and (b) the game/scrimmage/practice "Performance" card, which has no canonical replacement yet: stats-center.ts exposes official-vs-all splits, not a standalone scrimmage split, and neither canonical layer has a practice-session shape (see docs/baseball/stats-migration-plan.md\'s open practice-shape question). Full migration blocked on that decision, not on adapter availability.', + '#379/#845 (partial): exit-velocity fields primarily derive from baseball_batted_ball_events via elite-stat-events.ts\'s own buildHitterMetrics aggregator — closes the former "typed but un-migrated" comment — but fall back to the deprecated baseball_player_stats.exit_velocity column ONLY when this player has zero batted-ball-event rows (#845 review fix: a #379 migration regressed a legacy-only player from "shows a real EV number" to an honest-LOOKING null), mirroring legacy-stat-adapters.ts\'s box-score > legacy-fallback > no-data precedence. Still reads baseball_player_aggregates for (a) the Hitting/Pitching season-average legacy-fallback tier and (b) the game/scrimmage/practice "Performance" card, which has no canonical replacement yet: stats-center.ts exposes official-vs-all splits, not a standalone scrimmage split, and neither canonical layer has a practice-session shape (see docs/baseball/stats-migration-plan.md\'s open practice-shape question). Full migration blocked on that decision, not on adapter availability.', }, { path: 'src/lib/baseball/read-models/command-center.ts', @@ -245,6 +259,20 @@ export const GRANDFATHERED_CONSUMERS: GrandfatheredStatLayerConsumer[] = [ }, // --- Tests / contract fixtures ---------------------------------------------- + { + path: 'src/contracts/baseball/product-trust/player-today-honest-loop.test.ts', + group: 'test', + status: 'pending migration', + note: + "#845 review fix. This file's #379 fixture migration onto box-score tables previously removed every reference to the deprecated table; a new describe block now seeds an (empty-by-default, populated per-test) baseball_player_stats + baseball_import_runs fixture to pin fetchRecentBoxScoreActivity's legacy-fallback path in player-today.ts (an already-grandfathered consumer above) — a legacy-only player must see real recentStats, never a silent honest-looking empty. Migrates in lockstep with the production file's own entry.", + }, + { + path: 'src/lib/baseball/read-models/__tests__/player-passport-recent-activity.test.ts', + group: 'test', + status: 'pending migration', + note: + "#845 review fix. NEW test file (no prior fixture coverage existed for getPlayerPassport's DB path — player-passport-innings.test.ts only covers the pure summarizePitchingSeason helper). Pins fetchRecentActivity's legacy-fallback path in player-passport.ts (an already-grandfathered consumer above) against a fake baseball_player_stats + baseball_import_runs fixture — a legacy-only player must see real recentActivity data, never a silent honest-looking empty. Migrates in lockstep with the production file's own entry.", + }, { path: 'src/lib/baseball/read-models/__tests__/command-center.test.ts', group: 'test',