Audit Wave 1 — 9 criticals + top trust bugs (10 root-caused fixes) - #960
Merged
Conversation
… pure-CSS entrance (audit W1) AnimatedPage/AnimatedItem (the only wrapper around the round-entry pages — New Round and Continue Round, i.e. the hole-1 shot-tracking screen) rendered a framer-motion m.div with initial="hidden" (opacity: 0), computed inline on the SERVER too, and only reached opacity: 1 once LazyMotion's async feature chunk resolved client-side. On a slow/cold connection — exactly the condition a player on a course is likely to hit — that chunk fetch could take seconds or stall outright, leaving the screen completely blank with no guaranteed terminal state. Replaced the reveal with the existing `.animate-fade-in-up` CSS keyframe (globals.css, already used by MessageConversationRail) plus `motion-reduce:animate-none`. A CSS animation paints on the first frame with zero JS/network dependency and guarantees opacity: 1 via animation-fill-mode: forwards — nothing left to get stuck on. Also removed the unused AnimatedList export, which carried the identical bug class and had zero importers anywhere in the app. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…me-local (audit W1) Root cause: FairwayEventCard (agenda/week/day rows), FairwayEventDetailDrawer, and FairwayMonthGrid formatted start/end times with date-fns `format(new Date(iso), 'h:mm a')`, which reads the EXECUTING environment's own implicit local timezone. SSR on Vercel runs in UTC while the browser (and this team) is America/New_York, so the identical start_time rendered ~4-5h apart depending on whether the text came from server-rendered HTML or a client re-render. Those spans were marked `suppressHydrationWarning`, so React never patched the mismatched server text to the client value — it could stick indefinitely until an unrelated re-render touched that subtree, producing the reported non-determinism across reloads. The Event Detail drawer only ever mounts client-side post-interaction, so it always computed the browser-local value — while an Agenda row from the initial SSR paint could stay frozen on the wrong (UTC) value, explaining the two surfaces disagreeing on the SAME event. Fix: add explicit-timezone formatters (formatEventTime, formatEventTimeCompact, formatEventDateLabel) to src/lib/calendar/timezone.ts using Intl.DateTimeFormat with a `timeZone` option — NOT affected by the runtime's own local zone — anchored to golf_team_settings.timezone (falling back to DEFAULT_TIMEZONE = America/New_York, matching the "Times shown in EDT" label already on FairwayEventEditor). Threaded `teamTimezone` from FairwayCalendar down through FairwayAgendaView → FairwayEventCard, FairwayEventDetailDrawer, and FairwayMonthGrid, replacing every runtime-local `format(...)` call for event times with the deterministic team-tz formatter. Removed the now-unneeded suppressHydrationWarning on those spans since server and client compute the identical string for the identical instant. Added src/test/lib/calendar/timezone.test.ts pinning a known UTC start_time to its expected EDT/EST wall-clock time (DST-correct) and asserting the output is IDENTICAL across simulated ambient timezones (UTC/ET/PT/JST) — the exact property the bug violated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…eate-Task dialog (audit W1) The dialog rendered its whole <Form> (fields + Cancel/Create task) as one flat block directly under ModalShell, with no ModalShell.Body scroll region and no ModalShell.Footer. At short viewports the panel's own max-h-[calc(100dvh-4rem)] overflow-hidden clipped whatever didn't fit, and since the actions were the last thing in that unbounded block, Cancel/Create task fell off-screen with no way to reach them (390px and similar). Restructures to the Body-wraps-Form / Footer-outside composition the other Fairway dialogs already use (e.g. FairwayRecruitFormSheet): ModalShell.Body (flex-1 overflow-y-auto) now owns the scrolling field list, ModalShell.Footer is a pinned sibling outside it. handleSubmit's event param is now optional so the Footer's "Create task" button — now outside the <form> element — can invoke it directly via onClick (Enter-to-submit inside the form still works via Form's onSubmit). Added a colocated regression test asserting Cancel/Create task render inside the Footer slot (never swept into the scrolling Body), plus a submit-still- works check for the new onClick wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…selection badge (audit W1) Two separate status-derivation bugs let a qualifier card contradict itself: 1. getPlayerQualifiersImpl (golf.ts) computed `completedRoundNumbers` (the "Thru" field) independently from `roundsCompleted`/`totalScore`/ `totalToPar`: it dropped any completed round whose nullable `qualifier_round_number` was null, and stayed `[]` on the entries-only fallback path (zero matching `golf_rounds` rows), while the other three fields still fell back to the `golf_qualifier_entries` aggregate. Result: a card could show "Completed" + a real posted TOTAL 70 / TO PAR -2 + THRU reading "Not started", all for the same entry. Extracted the shared derivation into `derivePlayerQualifierProgress` so `completedRoundNumbers.length` always equals `roundsCompleted`. 2. FairwayQualifierLeaderboard's committed-selections fetch ignored the `golf_qualifier_selections` query's `error` and did `sels ?? []`, silently turning a FAILED read into an empty-but-truthy Set — which rendered every entrant, including a genuine top-4, as "Not selected". Extracted `deriveCommittedSelections`, which now returns null (falls back to the honest merit-tier projection) on a fetch error instead of a false "confirmed nobody selected". Added regression tests for both pure derivation functions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…udit W1) KeyboardShortcutHint (mounted globally by FairwayDashboardShell, the sole active shell for every golf dashboard route) rendered its "Press ⌘K for quick actions" toast unconditionally on all viewports and pointer types. ⌘K has no meaning on touch, and the fixed-position pill floated over real content on mobile (new-message button, announcement text, qualifier rules, insight sentences, settings). Gate display with a single combined arbitrary media variant so the pill stays `hidden` (no box, no overlap, no focus stop) unless the pointer is fine AND the viewport is sm+ (640px): `hidden [@media(pointer:fine)_and_(min-width:640px)]:flex`. A coarse pointer at sm+ width (touchscreen laptop, tablet) still stays hidden. Desktop mouse/trackpad behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
… (audit W1)
The Course Library's per-tee Holes editor (TeeFormDrawer) rendered all 18
hole cards in one long list with no way to jump to holes 10-18 — it relied
entirely on scrolling a fixed-height container with no visible affordance,
so the back nine was effectively unreachable on real viewports. It now
shows one nine at a time behind a Front 9 / Back 9 Segmented toggle
(mirroring the existing FairwayHoleConfig pattern), so both nines are
always directly reachable regardless of viewport height.
Also fixes the tee-set row (CourseDetailDrawer's TeeRow): the name and
action buttons (Set default / Edit / Delete) shared one unwrapped flex
row, so a long tee name ("Championship") could sit under the action
buttons on narrow viewports. The row now stacks (name, then a wrapping
action row) below sm and returns to a single inline row from sm+.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…th the vs-team arrow (audit W1)
StandingStrip (the Fairway-native matte SG stat card used on the player
detail Strokes Gained tab) derived its "vs team" caption from the
percentile-based teamCohortText(team_pct, team_n), while its own arrow +
tone badge derives from the mean-relative deltaVsTeam(player_value,
team_avg, direction). On a skewed roster the two can disagree, e.g.
player 0.81 > team-mean 0.65 (up-arrow, "better than team") while
team_pct still lands in the 25-49 percentile bucket ("Below team
average") — reproducing exactly the SG caption/arrow mismatch bug class
that was already fixed on the legacy StandingBar Card/Inline/Hero
variants via teamRelativeText, but never ported to this newer sibling.
Swap StandingStrip's caption to teamRelativeText (same helper, same
mean-relative comparison as the arrow), export it from the StandingBar
public surface, and add a regression test that pins value + arrow +
caption agreement.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…ion default (audit W1) The coach dashboard's "Performance Trend" chart drew axes/gridlines but never the line/area itself. TrendChart hardcoded `isAnimationActive` (== `true`) on the Area/Line, which overrides Recharts v3's own `isAnimationActive: "auto"` default — a default that exists specifically to skip the entrance reveal (a clip-path keyed to the on-screen point positions) during SSR and under prefers-reduced-motion. With it forced on, the reveal always engaged, including for the server-rendered first paint (at a ResponsiveContainer fallback width, since SSR has nothing real to measure); the client's immediate remeasure-and-rerender at the real width then reset that same reveal before it ever finished, leaving the series clipped to 0% — axes and gridlines aren't part of that clip, so they drew fine regardless. Three changes, all closing a distinct way that reveal could get stuck: - TrendChart.tsx: `isAnimationActive="auto"` on both Area and Line, so Recharts itself skips the reveal whenever it's server-rendering or the user prefers reduced motion, instead of us silently disabling that guard. - FairwayCoachDashboard.tsx: memoize `trendPoints` (it was rebuilt as a new array/object graph on every render — this component re-renders at least once post-mount from the greeting effect), since a new `data` identity handed to an animated series can independently restart its reveal. - FairwayCoachDashboard.tsx: lazy-load TrendChart with `ssr:false`, mirroring FairwayPlayerDashboard's identical Scoring Trend chart (which already does this for the same reason) — avoids the SSR-fallback-width render and its guaranteed first-load resize entirely. Added TrendChart.test.tsx: renders under `Global.isSsr = true` (the exact condition the chart is actually mounted under on first paint) and asserts the series draws with no lingering reveal-clip ancestor; reverting the fix makes this fail (recharts throws) before it ever reaches that assertion. Also locks the chart's data against the "View as table" fallback per the audit's own verification method. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…sis (audit W1) Root cause: DataTable's <th> applied an inline width from TanStack's header.getSize(), which defaults to 150px for any column that doesn't set an explicit `size` — and no Fairway column def ever does. So every column, name or not, requested an equal-weight 150px hint. Under table-layout: auto, the browser's width-distribution algorithm starves whichever column has the smallest min-content when the combined hints overrun the viewport — and the name/identity column (min-w-0 + truncate, by design) is the one with near-zero min-content, while short numeric/badge siblings (Score, To Par, Trend, Focus areas) hold their natural width. On a 390px phone this collapsed player/course names to 2-4 visible characters. Fix: - data-table.tsx: stop applying the TanStack default width; add a new opt-in `meta.minWidth` (FairwayColumnMeta) applied as a real CSS min-width on both the header and body cell — a genuine floor that min-w-0 can't undo. - Give the Player column minWidth:160 in Recent Rounds (FairwayCoachDashboard) and the Players-tab roster table (PlayersGridView); Course gets minWidth:120. - Recent Rounds' player_name/course_name cells had no overflow handling at all (not even truncate) — wired proper min-w-0/flex-1/truncate so long names ellipsize gracefully instead of visually overflowing into the next column. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…b-announcements (audit W1) Three surfaces stated a count that disagreed with what they actually rendered/held: - FairwayCoachDashboard's Action Items header badge (and the coach-signal.ts hero "N items are waiting on you", sourced from the same enhancedData.actionItems array) showed the FULL item count, but the panel rendered items.slice(0, 6) — any team with >6 open items saw a header that disagreed with its own list. Render the full list instead of an arbitrary slice so the badge always describes what's on screen. - use-task-realtime's stats computation bucketed overdue_tasks into an if/else-if chain with status (completed -> in_progress -> overdue -> pending), making overdue mutually exclusive with in_progress. An in_progress-and-overdue task counted only toward in_progress_tasks, so the "N overdue tasks need attention" banner undercounted relative to the per-row overdue flag FairwayTaskCard already renders independently. Extracted the computation into computeTaskStats() and count overdue as a cross-cutting flag, not a fourth status bucket. - Team Hub's Announcements tab folded a FAILED getPlayerHubAnnouncements fetch into a bare [], then gated on announcements.length > 0 alone -- so a load error rendered the exact same "No announcements" EmptyState as a genuinely empty team, while the dedicated /dashboard/announcements page (a separate query path) could still show real rows. Threads an announcementsLoadError flag through team-hub/page.tsx -> FairwayTeamHub, mirroring the announcementsLoadError pattern player-hub-data.ts already uses for PlayerActionCenter's identical AnnouncementsList call, so a load failure always reaches AnnouncementsList's own honest "Couldn't load" + retry state instead of the plain empty state. Tests: ActionItemsPanel.test.tsx (render coherence), use-task-realtime .logic.test.ts (computeTaskStats, including the audit's exact 3-vs-5 repro), FairwayTeamHub.logic.test.ts (showAnnouncementsList). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…of use-server, test type-casts - derivePlayerQualifierProgress moved golf.ts -> qualifier-progress.ts (a 'use server' module may only export async actions; the sync helper tripped the server-action observability coverage gate and Next's use-server rule) - StandingStrip caption resolved as union: coach-voice neutralizeForCoach() now wraps the W1 mean-relative teamRelativeText() (keeps #915 + arrow-agreement) - data-table + create-task layout test type-casts for full tsc Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (37)
Summary by CodeRabbit
WalkthroughThis PR adds deterministic calendar timezone rendering, centralized qualifier and task-state derivation, responsive dashboard and golf UI behavior, improved load-state handling, and regression tests covering the updated flows. ChangesCalendar timezone rendering
Qualifier progress derivation
Qualifier selection state
Task statistics
Chart and table UI behavior
Team dashboard state handling
Task modal layout
Golf interaction and presentation updates
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant FairwayCalendar
participant FairwayEventCard
participant timezoneFormatter
FairwayCalendar->>FairwayEventCard: pass teamTimezone
FairwayEventCard->>timezoneFormatter: formatEventTime(event instant, timezone)
timezoneFormatter-->>FairwayEventCard: deterministic event time label
FairwayEventCard-->>FairwayCalendar: render time and aria label
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml 🔧 ESLint
src/app/golf/(dashboard)/dashboard/team-hub/page.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/app/golf/actions/__tests__/golf-qualifier-progress.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/app/golf/actions/golf.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First fix wave from the full product audit. Ten independently root-caused fixes, each with a regression test, integrated and gated together.
Criticals
AnimatedPage.tsxTrendChart.tsx+ dashboard feed; verified against the "View as table" fallback.src/lib/calendar/timezone.ts, deterministic across ambient TZ (test pins a UTC instant to 6:00 PM EDT identically under UTC/LA/NY/Tokyo). Agenda, drawer, month cells all agree.data-table+ roster + dashboard rounds).ModalShell.Body(scroll)/ModalShell.Footer(pinned) composition.Trust / high
derivePlayerQualifierProgressmakes THRU/roundsCompleted/totals derive from one source; fail-safe selection badges.neutralizeForCoach).pointer:fine+sm+, no longer floats over content on phones.Gates
tsc --noEmitclean · eslint clean on all 37 changed files · full vitest: 2,465 files / 23,484 tests green. Integration note: extractedderivePlayerQualifierProgressout of the'use server'golf.tsintoqualifier-progress.ts(server modules may only export async actions).🤖 Generated with Claude Code