From 46ba89ccf2519a487c5fe5b7070623f078f93413 Mon Sep 17 00:00:00 2001 From: Fable Integrator Date: Fri, 17 Jul 2026 18:39:38 -0400 Subject: [PATCH] =?UTF-8?q?fix(golf):=20rounds=20data=20hygiene=20?= =?UTF-8?q?=E2=80=94=20date=20off-by-one,=20qualifier=20tagging,=20putts?= =?UTF-8?q?=20unification,=20nav=20polish?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parts of #916 + #917 (the remaining non-copy items). 1. DATE OFF-BY-ONE: FairwayRoundRow's date formatter parsed round_date (a bare 'YYYY-MM-DD' date-only column) via `new Date(iso)` with no timezone pin, so `toLocaleDateString()` read it back in the local (west-of-UTC) timezone and printed the previous day ("Mon Jun 1") while FairwayRoundDetail's header, which DID pin to UTC, correctly showed "June 2". Added src/lib/golf/date-only.ts (parseDateOnly + UTC-pinned formatters, with tests locking in the exact boundary case) and switched both surfaces to it so they can never disagree again. 2. QUALIFIER ROUNDS UNTAGGED: the Qualifiers tab aggregates a qualifier's results purely by golf_rounds.qualifier_id (see updateQualifierEntryStats in golf.ts), but the Rounds list's "Qualifier" format filter filters purely by round_type — so a round with qualifier_id set but round_type stuck at something else shows 0 there while still counting as a completed qualifier result. Found a concrete write-path gap: the legacy/offline draft-save action (round-drafts.ts's saveRoundDraftImpl) never wrote qualifier_id/qualifier_round_number to golf_rounds at all, despite RoundDraftData carrying selectedQualifierId/selectedRoundNumber — fixed (guarded so a caller that doesn't know about qualifiers can never clobber a value an earlier save set). Shipped scripts/backfill-qualifier-round-tags.ts to reconcile rows that already drifted before the fix (dry-run by default; NOT executed). 3. PUTTS MISMATCH (#917): Team Stats summed golf_holes.putts and divided by the count of holes that actually carry a recorded putts value; the player stats cockpit (golf-stats-calculator-shots.ts, behind stats-data.ts's getDetailedStats) summed the same putts but divided by EVERY hole played, diluting the average downward for any round with an unlogged hole — exactly the reported 33.3 vs 32.6 drift. Unified both into src/lib/golf/putts-per-round.ts (calculatePuttsPerRound, with tests) and switched both call sites to consume it. 4. #917 leftovers: the Players tab's browser read surfaceName('development') ("Development Plans", the page-content identity) instead of surfaceName('players-tab') ("Players", the masthead tab identity the user actually clicked) — both are intentional per surface-registry.ts's two-name-level design, only the <title> was reading the wrong one. Fixed the stray '♦' glyphs on sortable table headers (ROUNDS/GOALS and every other sortable column): DataTable's SortGlyph draws two stacked triangle SVGs (▲ over ▼) with no gap between them — in the default unsorted state both render the same muted color, so the two triangles' touching wide bases read as one solid diamond instead of two chevrons. Added a small gap so they read as sort chevrons again. Gates: npx tsc --noEmit (clean), npx eslint on all changed files (clean), npx vitest run across the new + touched test files (1324 tests passed, including the existing golf-stats-calculator-shots and FairwayTeamStats suites, unaffected by the putts formula's denominator fix for the common fully-logged-round case). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg --- scripts/backfill-qualifier-round-tags.ts | 151 ++++++++++++++++++ .../dashboard/development/page.tsx | 7 +- .../(dashboard)/dashboard/stats/team/page.tsx | 9 +- src/app/golf/actions/round-drafts.ts | 14 ++ .../fairway/data-table/data-table.tsx | 14 +- .../pages/rounds/FairwayRoundDetail.tsx | 19 +-- .../fairway/pages/rounds/FairwayRoundRow.tsx | 10 +- src/lib/golf/date-only.test.ts | 85 ++++++++++ src/lib/golf/date-only.ts | 90 +++++++++++ src/lib/golf/putts-per-round.test.ts | 71 ++++++++ src/lib/golf/putts-per-round.ts | 70 ++++++++ src/lib/utils/golf-stats-calculator-shots.ts | 19 ++- 12 files changed, 534 insertions(+), 25 deletions(-) create mode 100644 scripts/backfill-qualifier-round-tags.ts create mode 100644 src/lib/golf/date-only.test.ts create mode 100644 src/lib/golf/date-only.ts create mode 100644 src/lib/golf/putts-per-round.test.ts create mode 100644 src/lib/golf/putts-per-round.ts diff --git a/scripts/backfill-qualifier-round-tags.ts b/scripts/backfill-qualifier-round-tags.ts new file mode 100644 index 000000000..9e6af19fb --- /dev/null +++ b/scripts/backfill-qualifier-round-tags.ts @@ -0,0 +1,151 @@ +/** + * backfill-qualifier-round-tags.ts — one-time backfill for + * `golf_rounds.round_type` on rounds that ARE linked to a qualifier + * (`qualifier_id IS NOT NULL`) but were never tagged `round_type = 'qualifier'`. + * + * Symptom (#916): the Qualifiers tab correctly shows completed qualifiers with + * results — `updateQualifierEntryStats()` (src/app/golf/actions/golf.ts) + * aggregates a qualifier's results purely by `qualifier_id` + `status = + * 'completed'`, ignoring `round_type` entirely — but the Rounds list's + * "Qualifier" format filter (src/components/fairway/pages/rounds/ + * FairwayRoundsLibrary.tsx) filters purely by `round_type`, so a round that + * has `qualifier_id` set but `round_type` stuck at something else (`practice`, + * `tournament`, or the legacy `qualifying` spelling — the filter already + * accepts `qualifying` too) never shows under "Qualifier" there, even though + * it counts toward the qualifier's results. + * + * Root cause (write path, fixed alongside this script): the legacy/offline + * draft-save action `saveRoundDraft` (src/app/golf/actions/round-drafts.ts) + * never wrote `qualifier_id`/`qualifier_round_number` to `golf_rounds` at all + * — any round that passed through that write path before the fix could end up + * with the qualifier link set by an earlier/later save but a stale + * `round_type`. This script reconciles rows that already drifted before the + * fix landed; it does NOT need to run again for rounds created after it. + * + * Idempotent: only touches rows where `qualifier_id IS NOT NULL AND + * round_type NOT IN ('qualifier', 'qualifying')`. Re-running after a first + * successful pass finds zero matching rows and is a no-op. UPDATE only (one + * column, `round_type`) — no inserts, no deletes, no other columns touched. + * + * Connection modeled on scripts/backfill-baseball-slash-lines.ts: env from + * .env.local, `createClient(url, key, { auth: { persistSession: false, + * autoRefreshToken: false } })`. Dry-run by default (prints the plan, writes + * nothing) — pass --confirm to write. + * + * SCRIPT ONLY — per repo policy this is reviewed in the PR and is NOT run by + * the author. A human runs it (dry-run first) after review. + * + * Run: + * DOTENV_CONFIG_PATH=.env.local npx tsx -r dotenv/config scripts/backfill-qualifier-round-tags.ts # dry run + * DOTENV_CONFIG_PATH=.env.local npx tsx -r dotenv/config scripts/backfill-qualifier-round-tags.ts --confirm # write + * + * Requires env: NEXT_PUBLIC_SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY. + */ +import { config as loadEnv } from 'dotenv'; +import { createClient, type SupabaseClient } from '@supabase/supabase-js'; + +loadEnv({ path: '.env.local' }); + +const DRY = !process.argv.includes('--confirm'); + +interface MistaggedRoundRow { + id: string; + player_id: string; + qualifier_id: string; + round_type: string | null; + status: string | null; + round_date: string; +} + +/** Page through a table past PostgREST's 1000-row default cap, with a stable order. */ +async function fetchAllRows<T>( + build: (from: number, to: number) => PromiseLike<{ data: T[] | null; error: { message: string } | null }>, +): Promise<T[]> { + const PAGE = 1000; + const out: T[] = []; + for (let from = 0; ; from += PAGE) { + const { data, error } = await build(from, from + PAGE - 1); + if (error) throw error; + const rows = data ?? []; + out.push(...rows); + if (rows.length < PAGE) break; + } + return out; +} + +async function main() { + const url = (process.env.NEXT_PUBLIC_SUPABASE_URL ?? '').trim(); + const key = (process.env.SUPABASE_SERVICE_ROLE_KEY ?? '').trim(); + if (!url || !key) throw new Error('Missing NEXT_PUBLIC_SUPABASE_URL / SUPABASE_SERVICE_ROLE_KEY'); + const supabase: SupabaseClient = createClient(url, key, { + auth: { persistSession: false, autoRefreshToken: false }, + }); + + console.log( + `${DRY ? '[DRY RUN] printing plan, writing NOTHING. Re-run with --confirm.\n' : ''}` + + `Backfilling golf_rounds.round_type for qualifier-linked rounds...\n`, + ); + + // Fetch every round with a qualifier_id set, then filter client-side for a + // round_type that isn't already 'qualifier'/'qualifying' — keeps the query + // a single simple .not.is filter (no OR-of-NOT-IN needed against PostgREST). + const rows = await fetchAllRows<MistaggedRoundRow>((from, to) => + supabase + .from('golf_rounds') + .select('id, player_id, qualifier_id, round_type, status, round_date') + .not('qualifier_id', 'is', null) + .order('id', { ascending: true }) + .range(from, to) + .returns<MistaggedRoundRow[]>(), + ); + + const mistagged = rows.filter( + (r) => r.round_type !== 'qualifier' && r.round_type !== 'qualifying', + ); + + if (mistagged.length === 0) { + console.log(`Scanned ${rows.length} qualifier-linked round(s). All already tagged 'qualifier' — nothing to backfill.`); + return; + } + + console.log(`Found ${mistagged.length} of ${rows.length} qualifier-linked round(s) with the wrong round_type.\n`); + + let changed = 0; + let errors = 0; + + for (const row of mistagged) { + const label = `round ${row.id.slice(0, 8)} (player ${row.player_id.slice(0, 8)}, qualifier ${row.qualifier_id.slice(0, 8)}, ${row.round_date}, status=${row.status ?? 'unknown'})`; + console.log(` ${DRY ? '[DRY] would update' : '✓ updating'} ${label}: round_type '${row.round_type ?? 'null'}' → 'qualifier'`); + + if (!DRY) { + const { error: updateError } = await supabase + .from('golf_rounds') + .update({ round_type: 'qualifier' }) + .eq('id', row.id) + // Idempotency guard against a concurrent write between the SELECT + // above and this UPDATE: only touch the row if it's still mistagged. + .not('round_type', 'in', '(qualifier,qualifying)'); + + if (updateError) { + console.warn(` ⚠ ${label}: update failed — ${updateError.message}`); + errors++; + continue; + } + } + + changed++; + } + + console.log(`\n${DRY ? '[DRY RUN] ' : ''}Done. mistagged=${mistagged.length} ${DRY ? 'would-update' : 'updated'}=${changed} errors=${errors}`); + + if (errors > 0) process.exitCode = 1; + + if (DRY) { + console.log('\nRe-run with --confirm to write.'); + } +} + +main().catch((err) => { + console.error('Backfill failed:', err); + process.exit(1); +}); diff --git a/src/app/golf/(dashboard)/dashboard/development/page.tsx b/src/app/golf/(dashboard)/dashboard/development/page.tsx index 23a4cf8d2..c787000cd 100644 --- a/src/app/golf/(dashboard)/dashboard/development/page.tsx +++ b/src/app/golf/(dashboard)/dashboard/development/page.tsx @@ -14,7 +14,12 @@ import type { FairwayGoalCardData } from '@/components/fairway/pages/coachhelm/F import { surfaceName } from '@/lib/golf/surface-registry'; export const metadata: Metadata = { - title: `${surfaceName('development')} | Helm Golf`, + // The browser tab should match the masthead tab identity the user actually + // clicked ('Players', the coachhelm-tab surface-registry entry) — not the + // page-content identity ('Development Plans', the 'page' group entry for + // the SAME href). Both entries are intentional per surface-registry.ts's + // two-name-level design; only the <title> was still reading the wrong one (#917). + title: `${surfaceName('players-tab')} | Helm Golf`, description: 'Manage player development plans and focus areas for your team.', }; diff --git a/src/app/golf/(dashboard)/dashboard/stats/team/page.tsx b/src/app/golf/(dashboard)/dashboard/stats/team/page.tsx index 61bfd1040..463cfcf86 100644 --- a/src/app/golf/(dashboard)/dashboard/stats/team/page.tsx +++ b/src/app/golf/(dashboard)/dashboard/stats/team/page.tsx @@ -11,6 +11,7 @@ import { ViewHeader, EmptyState, Button } from '@/components/fairway'; import { fetchAllRows, fetchAllRowsResult } from '@/lib/supabase/fetch-all-rows'; import { getTeamLeakMaps } from '@/app/golf/actions/stats-leak-maps'; import { loadPlayersStandingMap } from '@/lib/coachhelm/v3/standing/loader'; +import { calculatePuttsPerRound } from '@/lib/golf/putts-per-round'; import type { Metadata } from 'next'; export const metadata: Metadata = { @@ -328,10 +329,10 @@ export default async function TeamStatsPage() { // carry a putts value (totalHolesWithPutts), NOT every scored hole — the // numerator only summed holes with a non-null putts, so dividing by all // scored holes (Σ holes_played) understated putts/round whenever some holes - // lacked a recorded putt count. - const puttsPerRound = totalHolesWithPutts > 0 && totalPutts > 0 - ? (totalPutts / totalHolesWithPutts) * 18 - : null; + // lacked a recorded putt count. Shared with the player stats cockpit + // (src/lib/utils/golf-stats-calculator-shots.ts) via calculatePuttsPerRound + // so the two surfaces can never disagree on the same player again (#917). + const puttsPerRound = calculatePuttsPerRound(totalPutts, totalHolesWithPutts); // Birdies per round: normalize to 18-hole equivalent // golf_holes.score is stored per-hole — null values indicate pre-score-tracking rounds diff --git a/src/app/golf/actions/round-drafts.ts b/src/app/golf/actions/round-drafts.ts index 3706f671f..d7d0d8b6e 100644 --- a/src/app/golf/actions/round-drafts.ts +++ b/src/app/golf/actions/round-drafts.ts @@ -158,6 +158,20 @@ async function saveRoundDraftImpl( total_score: null as null, score_to_par: null as null, total_putts: null as null, + // Qualifier linkage (#916): `data.selectedQualifierId`/`selectedRoundNumber` + // carry the qualifier a draft round belongs to (set by the round-setup + // UI's qualifier picker), but this record never wrote them to + // golf_rounds — so any round that passed through this (legacy/offline- + // sync) draft-save path silently lost its qualifier_id even when + // round_type correctly said 'qualifier', and never showed up in the + // qualifier's results. Only include the field when the caller + // explicitly supplied it, so a draft-save that doesn't carry qualifier + // context (e.g. an older offline queue entry) can never clobber a + // qualifier_id an earlier save already set. + ...(data.selectedQualifierId !== undefined ? { qualifier_id: data.selectedQualifierId } : {}), + ...(data.selectedRoundNumber !== undefined + ? { qualifier_round_number: data.selectedRoundNumber } + : {}), }; const hasTrackedRoundData = async (roundId: string): Promise<boolean> => { diff --git a/src/components/fairway/data-table/data-table.tsx b/src/components/fairway/data-table/data-table.tsx index e44c1e850..abfdedc9e 100644 --- a/src/components/fairway/data-table/data-table.tsx +++ b/src/components/fairway/data-table/data-table.tsx @@ -87,16 +87,24 @@ const TableCheckbox = React.forwardRef< ); }); -/** Sort affordance — three states (none / asc / desc) drawn with chevrons. */ +/** + * Sort affordance — three states (none / asc / desc) drawn with two stacked + * chevrons (▲ over ▼). The two SVGs must keep a visible gap between them: with + * NO gap (the previous `-mb-px` overlap) and both triangles the same muted + * color in the default unsorted state, the up-triangle's wide base and the + * down-triangle's wide base touch seamlessly and read as one solid diamond + * glyph rather than two chevrons — the stray "♦" reported on every sortable + * column header (#917), most visible on the right-aligned numeric columns. + */ function SortGlyph({ dir }: { dir: false | 'asc' | 'desc' }) { return ( - <span aria-hidden="true" className="ml-1.5 inline-flex flex-col leading-none"> + <span aria-hidden="true" className="ml-1.5 inline-flex flex-col gap-0.5 leading-none"> <svg width="8" height="5" viewBox="0 0 8 5" className={cn( - '-mb-px transition-colors [transition-duration:180ms]', + 'transition-colors [transition-duration:180ms]', dir === 'asc' ? 'text-accent-600' : 'text-text-tertiary/50', )} > diff --git a/src/components/fairway/pages/rounds/FairwayRoundDetail.tsx b/src/components/fairway/pages/rounds/FairwayRoundDetail.tsx index a27fede5d..5bcef775b 100644 --- a/src/components/fairway/pages/rounds/FairwayRoundDetail.tsx +++ b/src/components/fairway/pages/rounds/FairwayRoundDetail.tsx @@ -58,6 +58,7 @@ import { Button, } from '@/components/fairway'; import { cn } from '@/lib/utils'; +import { formatDateOnlyWeekdayLong, formatDateOnlyFull } from '@/lib/golf/date-only'; /* ─────────────────────────────────────────────────────────────────────────── * Props — fully-resolved, serializable data from the server page. @@ -181,17 +182,13 @@ export function FairwayRoundDetail({ const reviewHref = `/golf/dashboard/rounds/${round.id}/review`; // ── Masthead copy ────────────────────────────────────────────────────────── - // round_date is a DATE column ('YYYY-MM-DD') → new Date() = midnight UTC. - // Pin the formatters to UTC so SSR (server TZ) and hydration (client TZ) agree — - // without this, west-of-UTC clients render the previous day (React #418 + off-by-one). - const roundDate = new Date(round.round_date); - const dayOfWeek = roundDate.toLocaleDateString('en-US', { weekday: 'long', timeZone: 'UTC' }); - const dateLabel = roundDate.toLocaleDateString('en-US', { - month: 'long', - day: 'numeric', - year: 'numeric', - timeZone: 'UTC', - }); + // round_date is a DATE column ('YYYY-MM-DD'). Parsed + formatted through the + // shared date-only helper (pinned to UTC) so SSR (server TZ) and hydration + // (client TZ) agree, AND this header can never disagree with the rounds-list + // row on the calendar day (#916: a sibling surface's un-pinned formatter + // read the previous day west of UTC). + const dayOfWeek = formatDateOnlyWeekdayLong(round.round_date); + const dateLabel = formatDateOnlyFull(round.round_date); const heroTitle = `${dayOfWeek} at ${shortCourse(round.course_name)}`; const holesPlayed = round.holes_played ?? 18; const contextLine = `${roundTypeLabel(round.round_type)} · ${holesPlayed} holes · ${playerName}`; diff --git a/src/components/fairway/pages/rounds/FairwayRoundRow.tsx b/src/components/fairway/pages/rounds/FairwayRoundRow.tsx index f891f8626..f72ff7457 100644 --- a/src/components/fairway/pages/rounds/FairwayRoundRow.tsx +++ b/src/components/fairway/pages/rounds/FairwayRoundRow.tsx @@ -28,6 +28,7 @@ import { Badge, Chip } from '@/components/fairway/controls/badge'; import { Avatar } from '@/components/fairway/controls/avatar'; import type { RoundLibraryRound } from './FairwayRoundsLibrary'; import { scoreToParTone, formatToPar, getRoundTypeLabel } from './FairwayRoundCard'; +import { formatDateOnlyWeekdayShort, formatDateOnlyShort } from '@/lib/golf/date-only'; export interface FairwayRoundRowProps { round: RoundLibraryRound; @@ -36,11 +37,14 @@ export interface FairwayRoundRowProps { userRole: 'coach' | 'player'; } +// round_date is a DATE column ('YYYY-MM-DD') — parsed + formatted through the +// shared date-only helper so this row can never disagree with the round detail +// header on the calendar day (#916: `new Date(iso).toLocaleDateString()` with +// no timeZone pin read the previous day west of UTC). function dateParts(iso: string): { weekday: string; md: string } { - const d = new Date(iso); return { - weekday: d.toLocaleDateString('en-US', { weekday: 'short' }), - md: d.toLocaleDateString('en-US', { month: 'short', day: 'numeric' }), + weekday: formatDateOnlyWeekdayShort(iso), + md: formatDateOnlyShort(iso), }; } diff --git a/src/lib/golf/date-only.test.ts b/src/lib/golf/date-only.test.ts new file mode 100644 index 000000000..4dbae69e7 --- /dev/null +++ b/src/lib/golf/date-only.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect } from 'vitest'; +import { + parseDateOnly, + dateOnlyToUtcDate, + formatDateOnly, + formatDateOnlyWeekdayShort, + formatDateOnlyWeekdayLong, + formatDateOnlyShort, + formatDateOnlyFull, +} from './date-only'; + +describe('parseDateOnly', () => { + it('parses a bare YYYY-MM-DD string', () => { + expect(parseDateOnly('2026-06-02')).toEqual({ year: 2026, month: 6, day: 2 }); + }); + + it('parses the leading date out of a full timestamp', () => { + expect(parseDateOnly('2026-06-02T00:00:00.000Z')).toEqual({ year: 2026, month: 6, day: 2 }); + }); + + it('returns null for null/undefined/empty input', () => { + expect(parseDateOnly(null)).toBeNull(); + expect(parseDateOnly(undefined)).toBeNull(); + expect(parseDateOnly('')).toBeNull(); + }); + + it('returns null for garbage input', () => { + expect(parseDateOnly('not-a-date')).toBeNull(); + expect(parseDateOnly('2026/06/02')).toBeNull(); + }); + + it('returns null for an out-of-range month or day', () => { + expect(parseDateOnly('2026-13-01')).toBeNull(); + expect(parseDateOnly('2026-01-32')).toBeNull(); + }); +}); + +describe('dateOnlyToUtcDate', () => { + it('anchors at UTC midnight for the given calendar day', () => { + const d = dateOnlyToUtcDate({ year: 2026, month: 6, day: 2 }); + expect(d.getUTCFullYear()).toBe(2026); + expect(d.getUTCMonth()).toBe(5); // 0-indexed + expect(d.getUTCDate()).toBe(2); + expect(d.getUTCHours()).toBe(0); + }); +}); + +describe('formatDateOnly / boundary case — the #916 off-by-one', () => { + // The exact bug: a naive `new Date('2026-06-02').toLocaleDateString()` (no + // timeZone pin) parses as UTC midnight, then reads back in the *local* + // timezone — which prints June 1 anywhere west of UTC (every US zone). + // Every formatter here must print June 2 regardless of host TZ, because + // Node/vitest runs these tests in whatever TZ the CI/dev machine has. + it('formats a first-of-a-run date-only string as the correct calendar day', () => { + expect(formatDateOnlyShort('2026-06-02')).toBe('Jun 2'); + expect(formatDateOnlyFull('2026-06-02')).toBe('June 2, 2026'); + }); + + it('formats the correct weekday for the boundary date (Tuesday, not Monday)', () => { + // 2026-06-02 is a Tuesday. The reported bug showed "Mon Jun 1" in one + // surface and "June 2" in the other for this exact date. + expect(formatDateOnlyWeekdayShort('2026-06-02')).toBe('Tue'); + expect(formatDateOnlyWeekdayLong('2026-06-02')).toBe('Tuesday'); + }); + + it('is stable across a full-timestamp variant of the same date', () => { + expect(formatDateOnlyShort('2026-06-02T00:00:00.000Z')).toBe( + formatDateOnlyShort('2026-06-02'), + ); + }); + + it('handles a year boundary correctly (Dec 31 never becomes Jan 1 or vice versa)', () => { + expect(formatDateOnlyFull('2025-12-31')).toBe('December 31, 2025'); + expect(formatDateOnlyFull('2026-01-01')).toBe('January 1, 2026'); + }); + + it('falls back to the em dash for unparseable input', () => { + expect(formatDateOnly(null, { month: 'short', day: 'numeric' })).toBe('—'); + expect(formatDateOnly('garbage', { month: 'short', day: 'numeric' })).toBe('—'); + }); + + it('honors a custom fallback string', () => { + expect(formatDateOnly(null, { month: 'short', day: 'numeric' }, 'N/A')).toBe('N/A'); + }); +}); diff --git a/src/lib/golf/date-only.ts b/src/lib/golf/date-only.ts new file mode 100644 index 000000000..fa35b718c --- /dev/null +++ b/src/lib/golf/date-only.ts @@ -0,0 +1,90 @@ +/** + * Timezone-safe formatting for DATE-only columns (e.g. `golf_rounds.round_date`, + * `golf_qualifiers.start_date`). + * + * Postgres `date` columns serialize over PostgREST as a bare `YYYY-MM-DD` + * string with no time-of-day or offset. Handed to `new Date(str)`, the JS spec + * parses that as UTC midnight — so a formatter that then reads it back with + * `toLocaleDateString()` (which defaults to the *local* timezone) prints the + * PREVIOUS calendar day in every US timezone (west of UTC). Two Fairway rounds + * surfaces disagreed on the very same `round_date` for exactly this reason: + * one rendering pinned its formatter to UTC, the other didn't (#916). + * + * This module is the one parse path date-only round/qualifier fields should + * go through: pull the Y/M/D digits directly out of the string (never through + * a naive `new Date(str)` → local-timezone round trip) and format with + * `timeZone: 'UTC'` explicitly, so the calendar day is identical in every + * timezone and identical between SSR (server TZ) and hydration (client TZ) — + * no hydration mismatch, no off-by-one. + */ + +const DATE_ONLY_RE = /^(\d{4})-(\d{2})-(\d{2})/; + +export interface DateOnlyParts { + year: number; + /** 1-12 (calendar month, NOT the 0-indexed JS Date month). */ + month: number; + day: number; +} + +/** + * Parse a bare `YYYY-MM-DD` value (or a full timestamp that starts with one, + * e.g. `YYYY-MM-DDTHH:mm:ss.sssZ`) into its calendar parts, ignoring any + * time-of-day/offset entirely. Returns `null` for unparseable input. + */ +export function parseDateOnly(value: string | null | undefined): DateOnlyParts | null { + if (!value) return null; + const match = DATE_ONLY_RE.exec(value); + if (!match) return null; + const year = Number(match[1]); + const month = Number(match[2]); + const day = Number(match[3]); + if (!Number.isFinite(year) || !Number.isFinite(month) || !Number.isFinite(day)) return null; + if (month < 1 || month > 12 || day < 1 || day > 31) return null; + return { year, month, day }; +} + +/** + * A `Date` anchored at UTC midnight for the given calendar parts — a pure + * formatting engine, never meant to be read back via local-timezone getters + * (`getDate()`/`getMonth()`/`toLocaleDateString()` without `timeZone: 'UTC'`). + * Always pair with `{ timeZone: 'UTC' }` when formatting. + */ +export function dateOnlyToUtcDate(parts: DateOnlyParts): Date { + return new Date(Date.UTC(parts.year, parts.month - 1, parts.day)); +} + +/** + * Format a date-only value with explicit `Intl.DateTimeFormat` options, + * always pinned to UTC so the result is identical in every timezone/host. + * Returns `fallback` (default `'—'`) for unparseable input. + */ +export function formatDateOnly( + value: string | null | undefined, + options: Intl.DateTimeFormatOptions, + fallback = '—', +): string { + const parts = parseDateOnly(value); + if (!parts) return fallback; + return dateOnlyToUtcDate(parts).toLocaleDateString('en-US', { ...options, timeZone: 'UTC' }); +} + +/** Short weekday, e.g. "Mon". */ +export function formatDateOnlyWeekdayShort(value: string | null | undefined, fallback = '—'): string { + return formatDateOnly(value, { weekday: 'short' }, fallback); +} + +/** Long weekday, e.g. "Monday". */ +export function formatDateOnlyWeekdayLong(value: string | null | undefined, fallback = '—'): string { + return formatDateOnly(value, { weekday: 'long' }, fallback); +} + +/** Short month + day, e.g. "Jun 2". */ +export function formatDateOnlyShort(value: string | null | undefined, fallback = '—'): string { + return formatDateOnly(value, { month: 'short', day: 'numeric' }, fallback); +} + +/** Long month + day + year, e.g. "June 2, 2026". */ +export function formatDateOnlyFull(value: string | null | undefined, fallback = '—'): string { + return formatDateOnly(value, { month: 'long', day: 'numeric', year: 'numeric' }, fallback); +} diff --git a/src/lib/golf/putts-per-round.test.ts b/src/lib/golf/putts-per-round.test.ts new file mode 100644 index 000000000..fa1627e9b --- /dev/null +++ b/src/lib/golf/putts-per-round.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect } from 'vitest'; +import { + aggregatePuttsFromHoles, + calculatePuttsPerRound, + calculatePuttsPerRoundFromHoles, + type PuttsPerRoundHole, +} from './putts-per-round'; + +describe('calculatePuttsPerRound', () => { + it('normalizes an 18-hole round to itself', () => { + expect(calculatePuttsPerRound(30, 18)).toBe(30); + }); + + it('normalizes a 9-hole round up to an 18-hole equivalent', () => { + expect(calculatePuttsPerRound(16, 9)).toBe(32); + }); + + it('returns null when there are no holes with putts', () => { + expect(calculatePuttsPerRound(0, 0)).toBeNull(); + }); + + it('returns null when total putts is zero even if holes are present', () => { + expect(calculatePuttsPerRound(0, 18)).toBeNull(); + }); + + // The #917 regression: a round with some holes missing a recorded putts + // value must divide by the holes that DO have one, not by every hole + // played — dividing by "every hole played" dilutes the average downward + // for any player with even one unlogged hole, which is exactly why Team + // Stats (33.3) and the player profile (32.6) disagreed for the same player. + it('divides by holes-with-putts, not total holes played', () => { + // 16 holes logged (32 putts), 2 holes in the round have no recorded putts. + // Correct: 32 / 16 * 18 = 36. The old player-cockpit formula divided by + // 18 (every hole played) instead of 16, giving 32/18*18 = 32 — the wrong, + // diluted number. + expect(calculatePuttsPerRound(32, 16)).toBe(36); + }); +}); + +describe('aggregatePuttsFromHoles', () => { + it('sums putts and counts only holes with a recorded, positive value', () => { + const holes: PuttsPerRoundHole[] = [ + { putts: 2 }, + { putts: null }, // unlogged — excluded from both numerator and denominator + { putts: 1 }, + { putts: 0 }, // treated as unlogged, not a real 0-putt hole + ]; + expect(aggregatePuttsFromHoles(holes)).toEqual({ totalPutts: 3, holesWithPutts: 2 }); + }); + + it('returns zeros for an empty list', () => { + expect(aggregatePuttsFromHoles([])).toEqual({ totalPutts: 0, holesWithPutts: 0 }); + }); +}); + +describe('calculatePuttsPerRoundFromHoles', () => { + it('matches the Team Stats hole-level aggregation for a mixed-completeness round', () => { + // One 18-hole round where 2 holes never got a putts value recorded. + const holes: PuttsPerRoundHole[] = Array.from({ length: 16 }, (): PuttsPerRoundHole => ({ putts: 2 })).concat([ + { putts: null }, + { putts: null }, + ]); + // 16 holes * 2 putts = 32 total, over 16 holes-with-putts, normalized to 18. + expect(calculatePuttsPerRoundFromHoles(holes)).toBe(36); + }); + + it('returns null for an all-unlogged round', () => { + const holes: PuttsPerRoundHole[] = [{ putts: null }, { putts: null }]; + expect(calculatePuttsPerRoundFromHoles(holes)).toBeNull(); + }); +}); diff --git a/src/lib/golf/putts-per-round.ts b/src/lib/golf/putts-per-round.ts new file mode 100644 index 000000000..ea9ffdd81 --- /dev/null +++ b/src/lib/golf/putts-per-round.ts @@ -0,0 +1,70 @@ +/** + * Shared putts-per-round formula (#917). + * + * Team Stats (src/app/golf/(dashboard)/dashboard/stats/team/page.tsx) and the + * player-facing Stats cockpit (src/lib/utils/golf-stats-calculator-shots.ts, + * behind src/app/golf/actions/stats-data.ts) computed putts/round two + * different ways for the same player: + * + * - Team Stats summed `golf_holes.putts` and divided by the count of holes + * that actually carry a recorded putts value (null-honest). + * - The player stats cockpit summed the same putts but divided by EVERY + * hole played (`holes_played`/`stats.holesPlayed`), including holes with + * no putts logged at all. + * + * A hole with no recorded putts contributes 0 to the numerator either way, + * but counting it in the player-cockpit's denominator dilutes the average + * downward — which is exactly the reported drift (Team Stats 33.3 vs. player + * profile 32.6 for the same player). Team Stats' formula is the honest one + * (matches the codebase's "null-skip, don't fabricate" convention elsewhere + * in these files), so BOTH surfaces now compute through this one function. + * + * Normalizes to the 18-hole equivalent so 9-hole and 18-hole rounds combine + * correctly (9 putts over 9 holes and 18 over 18 both read as "18 putts / + * round", not double-counted or under-counted). + */ + +export interface PuttsPerRoundHole { + /** A hole with no recorded putts is `null` — never fabricated as 0. */ + putts: number | null; +} + +export interface PuttsPerRoundAggregate { + totalPutts: number; + /** Count of holes that actually carry a putts value — the denominator. */ + holesWithPutts: number; +} + +/** + * Reduce a flat list of holes (any shape carrying `putts`) into the two raw + * numbers `calculatePuttsPerRound` needs. Null and non-positive putts values + * are skipped entirely (not counted in either the numerator or denominator), + * matching the existing per-hole putts conventions elsewhere in the codebase. + */ +export function aggregatePuttsFromHoles(holes: readonly PuttsPerRoundHole[]): PuttsPerRoundAggregate { + let totalPutts = 0; + let holesWithPutts = 0; + for (const hole of holes) { + if (hole.putts !== null && hole.putts > 0) { + totalPutts += hole.putts; + holesWithPutts++; + } + } + return { totalPutts, holesWithPutts }; +} + +/** + * The one putts-per-round formula: total putts across every hole that + * actually has a recorded value, normalized to an 18-hole equivalent. + * Returns `null` (never a fabricated 0) when nothing is loggable yet. + */ +export function calculatePuttsPerRound(totalPutts: number, holesWithPutts: number): number | null { + if (holesWithPutts <= 0 || totalPutts <= 0) return null; + return (totalPutts / holesWithPutts) * 18; +} + +/** Convenience: aggregate + compute in one call for a flat hole list. */ +export function calculatePuttsPerRoundFromHoles(holes: readonly PuttsPerRoundHole[]): number | null { + const { totalPutts, holesWithPutts } = aggregatePuttsFromHoles(holes); + return calculatePuttsPerRound(totalPutts, holesWithPutts); +} diff --git a/src/lib/utils/golf-stats-calculator-shots.ts b/src/lib/utils/golf-stats-calculator-shots.ts index bad3c3fe9..be86ff4d2 100644 --- a/src/lib/utils/golf-stats-calculator-shots.ts +++ b/src/lib/utils/golf-stats-calculator-shots.ts @@ -8,6 +8,8 @@ * with a pure shot-based approach that derives everything from individual shots. */ +import { calculatePuttsPerRound } from '@/lib/golf/putts-per-round'; + // ============================================================================ // TYPES - Raw Data from Database // ============================================================================ @@ -1624,6 +1626,12 @@ function aggregateRoundStats(rounds: Array<{ let currentBirdieStreak = 0; let currentParStreak = 0; let current3PuttStreak = 0; + // Denominator for puttsPerRound (#917) — holes that actually carry a + // recorded putts value, NOT every hole played (stats.holesPlayed). Dividing + // by every hole played dilutes the average for any round with an unlogged + // hole; see src/lib/golf/putts-per-round.ts for the shared formula this + // and Team Stats both consume. + let totalHolesWithPutts = 0; // Process each round for (const round of rounds) { @@ -1917,6 +1925,7 @@ function aggregateRoundStats(rounds: Array<{ // (the old known-hole path fabricated putts=2 for these holes). if (hole.putts !== null) { stats.totalPutts += hole.putts; + if (hole.putts > 0) totalHolesWithPutts++; if (hole.threePutts) stats.threePuttsTotal++; if (hole.putts === 1) stats.onePuttsTotal++; } @@ -2357,9 +2366,13 @@ function aggregateRoundStats(rounds: Array<{ par5: finalizeParScore(scorePar5), }; - stats.puttsPerRound = stats.holesPlayed > 0 - ? Math.round(((stats.totalPutts / stats.holesPlayed) * 18) * 100) / 100 - : null; + // Shared formula with Team Stats (src/app/golf/(dashboard)/dashboard/stats/ + // team/page.tsx) via calculatePuttsPerRound — divides by holes that + // actually carry a recorded putts value (totalHolesWithPutts), not every + // hole played (stats.holesPlayed), so the two surfaces can never disagree + // on the same player again (#917). + const rawPuttsPerRound = calculatePuttsPerRound(stats.totalPutts, totalHolesWithPutts); + stats.puttsPerRound = rawPuttsPerRound != null ? Math.round(rawPuttsPerRound * 100) / 100 : null; stats.puttsPerHole = safeAverage(stats.totalPutts, stats.holesPlayed); // Denominator = GIR holes with KNOWN putts (null-skip both sides of the // ratio; a GIR hole with unrecorded putts must not drag the average down).