fix(golf): rounds data hygiene — date off-by-one, qualifier tagging, putts unification, nav polish - #936
fix(golf): rounds data hygiene — date off-by-one, qualifier tagging, putts unification, nav polish#936njrini99-code wants to merge 1 commit into
Conversation
…putts unification, nav polish 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 <title> 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
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 Mission Control — PR summary What it changes: Rounds data hygiene (#916 + #917 non-copy items). (1) Date off-by-one — Area: GolfHelm. Risk / reviewers watch:
CI: 4 required gates green. CodeRabbit advisory + |
1 participant
Problem
Parts of #916 + #917 (the remaining non-copy items) — four independent data-hygiene bugs on the Rounds/Stats/Players surfaces:
<title>read "Development Plans"; stray♦glyphs rendered on sortable ROUNDS/GOALS (and every other) column header.Root causes + fixes
1. Date off-by-one —
round_dateis a bare'YYYY-MM-DD'date-only Postgres column.FairwayRoundRow'sdateParts()parsed it vianew Date(iso)with no timezone pin, sotoLocaleDateString()read the UTC-midnight-parsed value back in the host's local timezone — west of UTC that's the previous calendar day.FairwayRoundDetail's header already pinned its formatter totimeZone: 'UTC'and showed the correct day, exposing the disagreement.Fix: added
src/lib/golf/date-only.ts—parseDateOnly(pulls Y/M/D digits directly, never through a naivenew Date→ local-TZ round trip) plus UTC-pinned formatters (formatDateOnlyWeekdayShort/Long,formatDateOnlyShort/Full). BothFairwayRoundRowandFairwayRoundDetailnow consume it, so they can't disagree again. Tests lock in the exact boundary case (a Tuesday date rendering as "Mon"/"Jun 1" under the old naive-Date bug), plus year-boundary and fallback cases.2. Qualifier rounds untagged — traced the
golf_qualifier_entries → golf_roundsrelationship: the Qualifiers tab's results (updateQualifierEntryStatsingolf.ts) aggregate a qualifier's completed rounds purely byqualifier_id, ignoringround_typeentirely. The Rounds list's "Qualifier" format filter (FairwayRoundsLibrary.tsx) filters purely byround_type. A round withqualifier_idset butround_typestuck at something else counts as a qualifier result but is invisible to the filter.Found a concrete write-path gap: the legacy/offline draft-save action
saveRoundDraftImpl(round-drafts.ts, used by the offline sync queue viasaveRoundDraft) builds itsgolf_roundsUPSERT payload fromRoundDraftDatabut never wrotequalifier_id/qualifier_round_numberat all, even though the type carriesselectedQualifierId/selectedRoundNumber. Fixed — conditionally included only when the caller explicitly supplies it, so a draft-save that doesn't carry qualifier context can never clobber a value an earlier save already set.Shipped
scripts/backfill-qualifier-round-tags.ts— idempotent, dry-run-by-default UPDATE for existing rows wherequalifier_id IS NOT NULL AND round_type NOT IN ('qualifier','qualifying'). Script only — not executed by me, per the task's constraints; a human runs it (dry-run first) after review.3. Putts mismatch — Team Stats (
stats/team/page.tsx) sumsgolf_holes.puttsand divides by the count of holes that actually have a recorded putts value (null-honest). The player stats cockpit (golf-stats-calculator-shots.ts, behindstats-data.ts'sgetDetailedStats) summed the same putts but divided by every hole played (stats.holesPlayed), including unlogged holes — diluting the average downward for any round with even one missing hole. That's exactly the reported direction (Team Stats 33.3 > profile 32.6).Unified into
src/lib/golf/putts-per-round.ts(calculatePuttsPerRound,aggregatePuttsFromHoles, with tests covering the exact dilution regression) and switched both call sites to consume it.4. #917 leftovers:
src/lib/golf/surface-registry.tsintentionally has two name levels for the same/dashboard/developmentroute:players-tab("Players", the masthead tab identity) anddevelopment("Development Plans", the page-content identity). The page's<title>metadata was readingsurfaceName('development')instead ofsurfaceName('players-tab')— fixed to match what the user actually clicked.DataTable'sSortGlyphdraws two stacked triangle SVGs (▲ over ▼) with no gap between them (-mb-pxoverlap). In the default unsorted state both render the same muted color — the up-triangle's wide base and the down-triangle's wide base touch seamlessly and read as one solid diamond (♦) instead of two chevrons. Fixed by replacing the overlap with a smallgap-0.5, restoring the intended two-chevron sort affordance on every sortable column (ROUNDS/GOALS included).Gates
npx tsc --noEmit -p tsconfig.json— cleannpx eslinton all 12 changed/new files — cleannpx vitest run— 1324 tests passed across the new + touched suites (date-only.test.ts,putts-per-round.test.ts,surface-registry.test.ts,golf-stats-calculator-shots*.test.ts,FairwayTeamStats.test.ts,golf-save-partial-round.test.ts,round-review-system.test.ts,stats-data.test.ts) — the putts formula change is a no-op for the common fully-logged-round case, confirmed by the existing suites staying green.Caveats
submitGolfRoundComprehensive→submit_round_atomicRPC) already threadsround_type/qualifier_idconsistently from the same client payload today, so the 2 already-mistagged rows are most likely historical drift predating that path's current form — the backfill script is the safety net for those.buildFallbackDetailedStats(stats-data.ts) has its own, separately-necessary round-level putts approximation for the degraded/error-recovery path (no hole-level data available there) — left untouched, out of scope for the two surfaces named in the task.FairwayRoundsLibrary.tsx's month/week grouping (honestRange,getWeekKey, etc.) parsesround_datevia the same un-pinnednew Date()pattern and could theoretically mis-bucket a first-of-month round at certain UTC offsets — not touched here since the task named exactly two renderings (rounds-list row + detail header) to normalize; flagging for a future pass.save_partial_round_atomic/submit_round_atomicPostgres RPCs (SQL migrations) — the TypeScript write-path gap fully explains the reported symptom class without touching live DB functions I can't test against here.🤖 Generated with Claude Code
https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg