Mobile overhaul M0+M1 + overnight fix waves: perf, More-sheet nav, condensing header, Bridge wiring, 69 verified fixes#797
Merged
Conversation
…ard retirement Perf (phone-class GPUs): - FairwayTopBar: opaque bg-surface at <md (no backdrop-blur on scrolling chrome); desktop glass + inset specular lines unchanged - FairwayBottomNav: backdrop-blur removed (matches its own design note) - AppShell drawer: 0.52s -> 0.3s, no remount-per-open; desktop rail gated behind useMediaQuery - Route transitions single-sourced: baseball template.tsx owns the fade, AppShell RouteTransition disabled (mirrors golf) - LazyMotion for real: async domAnimation loader (src/lib/motion/ load-features.ts), ~59 call sites off the synchronous import - React.memo actually effective: shell element props (brand, sidebarFooter, topBarActions/teamSwitcher) memoized in both product shells - use-unread-count realtime subscription per-user filtered - round-review First Load JS split (5.2MB -> dynamic segments) UI: - SectionMasthead overflow root-cause fixed (shrink-0 actions + unconstrained h1) — un-breaks ~65 baseball pages at 390px - Broken signup logo fixed; HelmMark extracted (src/components/brand/), 5 call sites - Accent-stripe cards retired both products -> dateline-rule idiom (28x2px tone rule); Card variant="stat" removed - Bridge sessions table wraps on mobile Gates: tsc 0, eslint 0, unit 455 files / 4616 passed, production build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
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. |
|
Too many files changed for review. ( Bypass the limit by tagging |
|
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. |
Qodo is busy workingCheck back in a few minutes. Qodo's code review agents are on it. |
WalkthroughThis PR centralizes Framer Motion feature loading, introduces shared branding and insight components, replaces left-border accents with dateline rules or dots, optimizes shell rendering and bundle loading, and updates polling, navigation, and animation timing behavior. ChangesUI platform and visual system
Shell and interaction performance
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 9 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/golf/(dashboard)/dashboard/rounds/new/new-round-client.tsx (1)
94-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood extraction — add unit tests for
decidePostHoleCompleteAction.Now that this is a pure, exported function with a small branching surface (last-hole-and-scored / re-edit / advance / fallback-finish), it's a great candidate for the
fast-checkproperty-testing pattern already used elsewhere (shot-level-sg.property.test.ts) — cheap to add, and it locks in round-completion navigation behavior against regressions.Want me to draft a
decidePostHoleCompleteAction.test.tscovering the four branches (last+scored, re-edit+scored, re-edit+unscored, normal-advance, last-unscored-fallback)?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/`(dashboard)/dashboard/rounds/new/new-round-client.tsx around lines 94 - 118, Add unit/property tests for the exported decidePostHoleCompleteAction function, following the fast-check pattern used by shot-level-sg.property.test.ts. Cover last-hole with all holes scored, re-edit with scored and unscored states, normal advancement, and last-hole unscored fallback, asserting the exact PostHoleCompleteAction returned for each case.src/components/baseball/NotificationBell.tsx (1)
141-173: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
isVisibleRefdoesn't reflect actual visibility at mount.
isVisibleRef(Line 141) initializes totrueunconditionally. If the component mounts while the tab/app is already backgrounded,handleVisibilityChangeonly fires on the nextvisibilitychangetransition — sorefreshCount()'s!isVisibleRef.currentgate (Line 148) stays falsely-open until the visibility state actually toggles once, meaning the initial fetch and any interval ticks before that first toggle bypass the gate entirely, defeating the intent of this change ("skips fetches while hidden instead of polling every 60s regardless").🐛 Proposed fix — seed the ref from actual document state
- const isVisibleRef = useRef(true); + const isVisibleRef = useRef(typeof document === 'undefined' || !document.hidden);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/baseball/NotificationBell.tsx` around lines 141 - 173, Initialize isVisibleRef from the browser’s current visibility state rather than unconditionally to true, using document.hidden when available, so refreshCount is gated correctly if the component mounts while hidden. Keep handleVisibilityChange updating the ref and retain the existing refresh behavior.src/hooks/use-unread-count.ts (1)
77-114: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing
INSERTlistener onbaseball_conversation_participants— newly-added conversations won't refresh in realtime.The old subscription listened to unfiltered
baseball_messagesINSERT, so any new message (including in a conversation the user was just added to) triggered a fullfetchUnreadCount(), which re-derivesconversationIdsReffrom a fresh query. The new subscription only listens forUPDATEonbaseball_conversations(filtered by the existingconversationIdsRef) andUPDATEon the user's ownbaseball_conversation_participantsrows.When a user is added to a new conversation, that's an
INSERTintobaseball_conversation_participants, not anUPDATE— so neither listener fires,conversationIdsRefnever gets repopulated, and the correspondingbaseball_conversationsUPDATEevents for that new conversation get silently dropped by theconversationIdsRef.current.has(...)check at Line 94. The unread badge for a freshly-joined conversation won't update until an unrelated refetch trigger fires (e.g. page reload).🐛 Proposed fix — add an INSERT listener scoped to the user's own participant rows
.on( 'postgres_changes', { event: 'UPDATE', schema: 'public', table: 'baseball_conversation_participants', filter: `user_id=eq.${user.id}`, }, () => { fetchUnreadCount(); } ) + // Catches being newly added to a conversation (a participant-row + // INSERT, not an UPDATE) so conversationIdsRef gets repopulated. + .on( + 'postgres_changes', + { + event: 'INSERT', + schema: 'public', + table: 'baseball_conversation_participants', + filter: `user_id=eq.${user.id}`, + }, + () => { + fetchUnreadCount(); + } + ) .subscribe();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-unread-count.ts` around lines 77 - 114, Add a realtime INSERT listener for the current user’s rows on baseball_conversation_participants alongside the existing UPDATE listener in the subscription chain. Use the same user_id filter and invoke fetchUnreadCount() so conversationIdsRef is refreshed when the user joins a conversation; retain the existing UPDATE listener for read-state changes.src/lib/calendar/event-styles.ts (1)
14-121: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep event dots on canonical palette tokens.
At
src/lib/calendar/event-styles.ts:14-121, the newdotColorconfiguration exposes rawamber,teal,violet,orange,sky,rose, andpurpleutilities, whichEventCardrenders directly. Map these to registered semantic status tokens or approvedprimary/warm/creamtokens instead of bypassing the design system.As per coding guidelines, use only canonical design-system color families and surfaces for rendered UI styling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/calendar/event-styles.ts` around lines 14 - 121, Update the event-style definitions in the event styles map so every dotColor uses registered semantic status tokens or approved primary, warm, or cream palette utilities; replace the raw amber, teal, violet, orange, sky, rose, and purple classes while preserving each event type’s intended visual distinction and ensuring EventCard receives only canonical design-system tokens.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/admin/jobs/page.tsx`:
- Around line 29-34: Update the KeyPanelRule component’s span to replace the
non-canonical bg-accent-500 class with the approved canonical primary color
token, while preserving the existing sizing, spacing, rounding, and
accessibility attributes.
In `@src/app/admin/users/`[id]/EngagementPanel.tsx:
- Around line 58-60: Replace the noncanonical bg-accent-500 class on the
dateline span in EngagementPanel with an approved primary color token,
preferably bg-primary-600, while preserving the existing sizing and layout
classes.
In `@src/app/baseball/`(dashboard)/_components/hub-sub-nav.tsx:
- Around line 34-35: Replace the loadFeatures import with loadMaxFeatures from
the motion loader, then update the LazyMotion features prop in the hub
sub-navigation component to use loadMaxFeatures so the layoutId active underline
animation is supported.
In `@src/app/baseball/`(onboarding)/player/page.tsx:
- Around line 312-322: Fix the responsive sizing typo in the HelmMark component
by changing the duplicated sm:h-12 class to sm:w-12, matching the intended
square dimensions at the sm breakpoint.
In `@src/app/golf/`(auth)/signup/page.tsx:
- Around line 7-8: The LazyMotion feature loader in loadFeatures should
dynamically import a sibling features module rather than importing framer-motion
directly. Add features.ts to re-export domAnimation/domMax, then have
loadFeatures use import('./features').then((mod) => mod.default) so the signup
page’s animation payload remains isolated.
In `@src/app/golf/`(dashboard)/dashboard/rounds/[id]/review/page.tsx:
- Around line 72-98: Update the loading placeholders for RoundReviewLlmCard,
HoleByHoleShotPaths, and RoundIntelligence to reserve panel-sized space matching
their mounted review panels, including appropriate surface styling, padding,
rounded corners, headers, and grid/content skeletons; replace the null or
undersized placeholders to prevent layout shifts.
In
`@src/app/golf/`(dashboard)/dashboard/rounds/continue/[id]/continue-round-client.tsx:
- Around line 32-45: Add a visible loading fallback to the dynamic declarations
for FairwaySaveRoundModal, FairwayRoundSubmitOverlay, and
FairwayRoundSummarySheet. Define or reuse an appropriate Skeleton/loading
component and pass it through each dynamic() call’s loading option so the
completion UI does not render null while its chunk loads.
In `@src/app/golf/admin/components/tracer/TracerRoundInspector.tsx`:
- Line 420: Replace the non-canonical bg-red-50/20 class in the stuck-row
styling with the repository’s token-backed destructive/notice-error surface
class, keeping the conditional stuck behavior unchanged and ensuring the result
uses only approved color families.
In `@src/app/golf/admin/crm/components/resend/KPIGrid.tsx`:
- Around line 32-36: The KPI status color configuration uses noncanonical blue,
amber, and red tokens. Update the `info`, `warning`, and `error` entries in the
KPI color mapping to use registered semantic `info`, `warning`, and
`destructive` tokens, or approved `primary`/`warm` tokens, while preserving the
existing icon/text structure.
In `@src/components/baseball/announcements/AnnouncementsPlayerView.tsx`:
- Around line 61-64: The dateline rule geometry is duplicated as hard-coded
h-[2px] w-7 classes in the skeleton and announcement card. Define or reuse a
shared design-system token, class, or primitive for this rule and apply it in
both locations within AnnouncementsPlayerView, removing the ad-hoc sizing.
In `@src/components/baseball/import-center/ImportWizardClient.tsx`:
- Around line 1121-1125: The dateline marker is duplicated across
ImportWizardClient, DevPlanClient, RosterTable, and both admin KeyPanelRule
helpers. Create a shared rule primitive with a tone prop preserving the
aria-hidden, sizing, rounding, and color contract, then replace each local
marker/helper—including the blocking row in ImportWizardClient—with this
primitive and remove redundant implementations.
In `@src/components/baseball/performance/lift-canvas/ExerciseBin.tsx`:
- Around line 73-81: Replace the raw amber/orange/red stress colors in
STRESS_CLS with the existing semantic warning/destructive design-system tokens,
preserving the StressLevel mapping. Replace the repeated h-[2px] w-7 styling at
the dateline rule usages with the shared dateline-rule class or primitive,
including the locations around the exercise title and related render paths.
In `@src/components/baseball/performance/PlayerLiftToday.tsx`:
- Around line 354-362: Replace the non-canonical amber classes in the coach-note
callout within PlayerLiftToday with the warning status-token classes used by the
sibling implementation: use bg-warning/10 for the container, bg-warning/40 for
the accent bar, and text-warning for the label.
In `@src/components/golf/calendar/PlayerRSVPCard.tsx`:
- Around line 18-19: Move the Framer Motion feature configuration into a
dedicated local features module, and update loadFeatures in load-features.ts to
import that module rather than importing the feature payload directly from
framer-motion. Keep PlayerRSVPCard.tsx limited to the runtime m,
AnimatePresence, LazyMotion, and useReducedMotion imports, ensuring the features
module is loaded only through the lazy boundary.
In `@src/components/golf/coachhelm/insights/InsightCallout.tsx`:
- Around line 27-28: Replace the non-canonical bg-accent-50 and text-accent-700
classes in InsightCallout with the corresponding approved primary-* color
tokens, preserving the existing styling and className composition.
- Around line 27-29: Add vertical spacing between the caption and body in the
InsightCallout component by applying a consistent gap or top margin to the
children wrapper/container while preserving the existing label styling and
caller behavior.
In `@src/components/ui/card.tsx`:
- Around line 263-272: The dateline and trend styling in the card component uses
non-canonical raw red classes. Update the `resolvedTrend` negative-state classes
in the dateline span and the trend icon background/text styling below to use the
sanctioned `destructive` or appropriate canonical design-system tokens, removing
all `bg-red-600`/related raw-red classes while preserving positive and neutral
states.
In `@src/lib/calendar/premium-utils.ts`:
- Around line 60-73: Handle the `match` and `social` event types consistently
with `getEventTypeClass()` and `getEventDotColorVar()`: add corresponding
`--event-match` and `--event-social` tokens in `calendar-tokens.css` and include
both keys in the `known` set, or remove their distinct classification so they
intentionally use the neutral `other` color.
In `@src/lib/motion/load-features.ts`:
- Around line 14-16: Update loadFeatures and loadMaxFeatures in load-features.ts
to dynamically import dedicated local wrapper modules rather than the
framer-motion package root. Add tiny modules that re-export domAnimation and
domMax respectively, then resolve those exports from the loaders so LazyMotion
keeps the feature payload in separate chunks.
---
Outside diff comments:
In `@src/app/golf/`(dashboard)/dashboard/rounds/new/new-round-client.tsx:
- Around line 94-118: Add unit/property tests for the exported
decidePostHoleCompleteAction function, following the fast-check pattern used by
shot-level-sg.property.test.ts. Cover last-hole with all holes scored, re-edit
with scored and unscored states, normal advancement, and last-hole unscored
fallback, asserting the exact PostHoleCompleteAction returned for each case.
In `@src/components/baseball/NotificationBell.tsx`:
- Around line 141-173: Initialize isVisibleRef from the browser’s current
visibility state rather than unconditionally to true, using document.hidden when
available, so refreshCount is gated correctly if the component mounts while
hidden. Keep handleVisibilityChange updating the ref and retain the existing
refresh behavior.
In `@src/hooks/use-unread-count.ts`:
- Around line 77-114: Add a realtime INSERT listener for the current user’s rows
on baseball_conversation_participants alongside the existing UPDATE listener in
the subscription chain. Use the same user_id filter and invoke
fetchUnreadCount() so conversationIdsRef is refreshed when the user joins a
conversation; retain the existing UPDATE listener for read-state changes.
In `@src/lib/calendar/event-styles.ts`:
- Around line 14-121: Update the event-style definitions in the event styles map
so every dotColor uses registered semantic status tokens or approved primary,
warm, or cream palette utilities; replace the raw amber, teal, violet, orange,
sky, rose, and purple classes while preserving each event type’s intended visual
distinction and ensuring EventCard receives only canonical design-system tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d36f4a9-fb23-4e55-8f98-1919c9377058
📒 Files selected for processing (112)
src/app/about/page.tsxsrc/app/admin/_components/AdminShell.tsxsrc/app/admin/_components/SessionsPanel.tsxsrc/app/admin/_components/TeamHealthTable.tsxsrc/app/admin/_motion-provider.tsxsrc/app/admin/activity/ActivityFeed.tsxsrc/app/admin/baseball/page.tsxsrc/app/admin/golf/page.tsxsrc/app/admin/golf/tracer/page.tsxsrc/app/admin/jobs/page.tsxsrc/app/admin/teams/[id]/RosterTable.tsxsrc/app/admin/teams/[id]/page.tsxsrc/app/admin/users/[id]/EngagementPanel.tsxsrc/app/baseball/(dashboard)/BaseballFairwayShell.tsxsrc/app/baseball/(dashboard)/_components/hub-sub-nav.tsxsrc/app/baseball/(dashboard)/dashboard/dev-plan/DevPlanClient.tsxsrc/app/baseball/(dashboard)/dashboard/template.tsxsrc/app/baseball/(onboarding)/coach-onboarding/page.tsxsrc/app/baseball/(onboarding)/player/page.tsxsrc/app/baseball/(player-dashboard)/player/template.tsxsrc/app/baseball/(public)/team/[id]/page.tsxsrc/app/golf/(auth)/demo/page.tsxsrc/app/golf/(auth)/forgot-password/page.tsxsrc/app/golf/(auth)/login/page.tsxsrc/app/golf/(auth)/reset-password/page.tsxsrc/app/golf/(auth)/signup/page.tsxsrc/app/golf/(auth)/welcome/page.tsxsrc/app/golf/(dashboard)/FairwayDashboardShell.tsxsrc/app/golf/(dashboard)/dashboard/rounds/[id]/review/page.tsxsrc/app/golf/(dashboard)/dashboard/rounds/continue/[id]/continue-round-client.tsxsrc/app/golf/(dashboard)/dashboard/rounds/new/new-round-client.tsxsrc/app/golf/(onboarding)/coach/page.tsxsrc/app/golf/(onboarding)/player/page.tsxsrc/app/golf/admin/_motion-provider.tsxsrc/app/golf/admin/components/AdminStatCard.tsxsrc/app/golf/admin/components/SessionHeatmap.tsxsrc/app/golf/admin/components/StatCardV2.tsxsrc/app/golf/admin/components/TeamUserDirectory.tsxsrc/app/golf/admin/components/tracer/TracerRoundInspector.tsxsrc/app/golf/admin/crm/components/CRMDashboard.tsxsrc/app/golf/admin/crm/components/CalendarView.tsxsrc/app/golf/admin/crm/components/CoachTable.tsxsrc/app/golf/admin/crm/components/resend/KPIGrid.tsxsrc/app/golf/admin/crm/components/resend/ResendActivityView.tsxsrc/app/golf/join/[code]/golf-join-team-client.tsxsrc/app/golf/join/page.tsxsrc/app/products/page.tsxsrc/components/auth/baseball-auth-shell.tsxsrc/components/baseball/NotificationBell.tsxsrc/components/baseball/announcements/AnnouncementsCoachView.tsxsrc/components/baseball/announcements/AnnouncementsPlayerView.tsxsrc/components/baseball/announcements/PlayerSelector.tsxsrc/components/baseball/coach-daily-contract/CoachPlayerDailyContractPanel.tsxsrc/components/baseball/command-center/DailyBriefPanel.tsxsrc/components/baseball/command-center/RiskFeedStrip.tsxsrc/components/baseball/daily-contract/DailyContract.tsxsrc/components/baseball/import-center/ImportWizardClient.tsxsrc/components/baseball/living-annual/SectionMasthead.tsxsrc/components/baseball/passport/PlayerPassportCard.tsxsrc/components/baseball/passport/ScoutPacketManager.tsxsrc/components/baseball/passport/ScoutPacketView.tsxsrc/components/baseball/performance/PlayerLiftToday.tsxsrc/components/baseball/performance/lift-canvas/ExerciseBin.tsxsrc/components/baseball/performance/lift-canvas/SessionBlock.tsxsrc/components/baseball/player-profile/PlayerInsightsPanel.tsxsrc/components/baseball/player-profile/PlayerNotesSection.tsxsrc/components/baseball/player-profile/ProfileTimeline.tsxsrc/components/baseball/position-planner/PositionPlayerPill.tsxsrc/components/baseball/practice-effectiveness/PracticeEffectivenessClient.tsxsrc/components/baseball/practice-player/PlayerPracticeClient.tsxsrc/components/baseball/settings/PhilosophySettingsClient.tsxsrc/components/baseball/settings/ProgramSettingsClient.tsxsrc/components/baseball/settings/SeasonSettingsClient.tsxsrc/components/baseball/source-trust/SourceDrawer.tsxsrc/components/baseball/staff/StaffSettingsClient.tsxsrc/components/baseball/stat-visuals/StatVisualFrame.tsxsrc/components/baseball/stats-center/StatsCenterClient.tsxsrc/components/baseball/video/VideoCard.tsxsrc/components/baseball/video/VideoLibraryClient.tsxsrc/components/brand/HelmMark.tsxsrc/components/fairway/app-shell/AppShell.tsxsrc/components/fairway/app-shell/FairwayBottomNav.tsxsrc/components/fairway/app-shell/FairwaySidebar.tsxsrc/components/fairway/app-shell/FairwayTopBar.tsxsrc/components/fairway/pages/messages/MessageConversationRail.tsxsrc/components/fairway/pages/player-game/FairwayMyGameProfile.tsxsrc/components/fairway/pages/rounds-new/FairwayRoundSubmitOverlay.tsxsrc/components/fairway/pages/rounds-new/FairwayRoundSummarySheet.tsxsrc/components/golf/calendar/ConflictWarning.tsxsrc/components/golf/calendar/EventCard.tsxsrc/components/golf/calendar/PlayerRSVPCard.tsxsrc/components/golf/calendar/PremiumEventBlock.tsxsrc/components/golf/calendar/WeekView.tsxsrc/components/golf/coachhelm/insights/DiagnosisPanel.tsxsrc/components/golf/coachhelm/insights/DiagnosisSheet.tsxsrc/components/golf/coachhelm/insights/InsightCallout.tsxsrc/components/golf/coachhelm/round-review/RoundIntelligence.tsxsrc/components/golf/coachhelm/v3/HoleShotPath/index.tsxsrc/components/golf/coachhelm/v3/PuttHeatmap/index.tsxsrc/components/golf/layout/AnimatedPage.tsxsrc/components/golf/rounds/RoundLibraryClient.tsxsrc/components/landing/Hero.tsxsrc/components/landing/MobileNav.tsxsrc/components/lifting/athletes/AthleteRosterClient.tsxsrc/components/lifting/players/PlayerLiftHomeClient.tsxsrc/components/ui/card.tsxsrc/hooks/use-unread-count.tssrc/lib/calendar/event-styles.tssrc/lib/calendar/premium-utils.tssrc/lib/motion/load-features.tssrc/lib/types/calendar.tssrc/styles/calendar-tokens.css
💤 Files with no reviewable changes (2)
- src/app/golf/admin/components/StatCardV2.tsx
- src/app/golf/admin/crm/components/CalendarView.tsx
Comment on lines
+312
to
+322
| <HelmMark | ||
| sport="baseball" | ||
| size={48} | ||
| className="h-10 w-10 sm:h-12 sm:w-12" | ||
| glow | ||
| glowClassName="blur-xl scale-150" | ||
| glowOpacity={0.2} | ||
| alt="BaseballHelm Logo" | ||
| priority | ||
| unoptimized | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Same sm:h-12 sm:h-12 typo as coach-onboarding/page.tsx:448.
Duplicate sm:h-12, missing sm:w-12 — squishes the mark at sm: breakpoint. See the combined fix above (this line is folded into the diff for the structural issue at 305-325); flagging separately since it's an independent bug (copy-pasted into two files).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(onboarding)/player/page.tsx around lines 312 - 322, Fix
the responsive sizing typo in the HelmMark component by changing the duplicated
sm:h-12 class to sm:w-12, matching the intended square dimensions at the sm
breakpoint.
Comment on lines
+7
to
+8
| import { LazyMotion, m, useReducedMotion } from 'framer-motion'; | ||
| import { loadFeatures } from '@/lib/motion/load-features'; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does webpack/Turbopack dedupe a dynamic import() of a module that is also statically imported elsewhere in the same file into the same chunk?
💡 Result:
Yes, both Webpack and Turbopack generally aim to deduplicate modules, ensuring that a module imported both statically and dynamically is not bundled multiple times [1][2]. However, the specific behavior regarding chunk generation depends on how you structure your code and your configuration. In Webpack, a dynamic import acts as a code-splitting point [3][4]. By default, Webpack moves the dynamically imported module and its dependencies into a separate chunk to enable lazy loading [4]. If that same module is also imported statically elsewhere in your application, Webpack recognizes that both imports refer to the same module [1]. Its optimization algorithms, particularly the SplitChunksPlugin, are designed to manage this shared dependency by placing it into a single, shared chunk if it meets certain criteria (such as minimum size), or by including it in the main bundle while ensuring it is only executed once [5][6]. If the module is small, Webpack might choose to inline it to avoid the overhead of an extra network request, but it will still deduplicate the code [7]. Turbopack similarly supports both static and dynamic imports and explicitly handles deduplication for modules imported in both ways [1][8]. It has undergone specific improvements to fix issues where references to the same asset across static and dynamic boundaries were not being correctly unified, ensuring that duplicate code is not shipped [1]. Turbopack also features runtime optimizations that allow it to avoid redundant chunk loading if the module is already available in the browser's memory [2]. If you find that a dynamic import is not producing the expected separate chunk or is causing duplication, it is often due to how the module graph is structured or configuration thresholds (like minSize in Webpack) [5][9][7]. To ensure a module is split, you may need to: 1. Verify that the module is not being statically imported in a way that forces it into the main bundle (e.g., as a dependency of an entry point that Webpack decides must be static) [10][9]. 2. Adjust SplitChunksPlugin settings if you want to force specific modules into shared chunks [7]. 3. Ensure that you are consistently using ES modules for both static and dynamic imports, as mixing CommonJS and ES module interop can occasionally lead to issues with instance deduplication [11].
Citations:
- 1: Fix deduplication when importing module statically and dynamically vercel/turborepo#3297
- 2: add included metadata to chunks to allow deduplicate chunk loads at runtime vercel/turborepo#4488
- 3: https://docs.webpack.js.org/api/module-methods/import
- 4: https://webpack.js.org/api/module-methods/
- 5: https://webpack.js.org/plugins/split-chunks-plugin/
- 6: https://webpack.js.org/guides/code-splitting/
- 7: dynamic import of modules containing same import statements sometimes result in duplicate code generation webpack/webpack#13768
- 8: https://nextjs.org/docs/app/api-reference/turbopack
- 9: https://stackoverflow.com/questions/67110919/webpack-code-splitting-when-importing-dynamic-and-static-mixed
- 10: https://stackoverflow.com/questions/63072279/webpack-both-static-and-dynamic-import-leads-to-no-chunk
- 11: https://github.com/orgs/webpack/discussions/13854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant files ==\n'
git ls-files 'src/app/golf/(auth)/signup/page.tsx' 'src/lib/motion/load-features.ts' 'src/components/**/RiskFeedStrip.tsx' 'src/components/**/Hero.tsx' 'src/components/**/HoleShotPath/index.tsx' 'src/app/golf/**/forgot-password/page.tsx' | sed 's#^`#-` #'
printf '\n== signup/page.tsx ==\n'
cat -n src/app/golf/(auth)/signup/page.tsx | sed -n '1,220p'
printf '\n== load-features.ts ==\n'
cat -n src/lib/motion/load-features.ts | sed -n '1,220p'
printf '\n== motion imports usage ==\n'
rg -n "loadFeatures|domAnimation|LazyMotion|useReducedMotion|from 'framer-motion'|from \"framer-motion\"" \
src/app/golf/(auth)/signup/page.tsx src/lib/motion/load-features.ts \
src/components src/app/golf | sed -n '1,220p'Repository: njrini99-code/helmv3
Length of output: 582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant files ==\n'
git ls-files 'src/app/golf/(auth)/signup/page.tsx' 'src/lib/motion/load-features.ts' 'src/components/**/RiskFeedStrip.tsx' 'src/components/**/Hero.tsx' 'src/components/**/HoleShotPath/index.tsx' 'src/app/golf/**/forgot-password/page.tsx' | sed 's#^`#-` #'
printf '\n== signup/page.tsx ==\n'
cat -n 'src/app/golf/(auth)/signup/page.tsx' | sed -n '1,220p'
printf '\n== load-features.ts ==\n'
cat -n 'src/lib/motion/load-features.ts' | sed -n '1,220p'
printf '\n== motion imports usage ==\n'
rg -n "loadFeatures|domAnimation|LazyMotion|useReducedMotion|from 'framer-motion'|from \"framer-motion\"" \
'src/app/golf/(auth)/signup/page.tsx' 'src/lib/motion/load-features.ts' \
src/components src/app/golf | sed -n '1,220p'Repository: njrini99-code/helmv3
Length of output: 36509
🌐 Web query:
framer-motion LazyMotion loader function separate file domAnimation code splitting docs
💡 Result:
To implement code splitting with Framer Motion using LazyMotion and a separate file for feature bundles, follow these steps: 1. Create a separate file (e.g., features.js) to export the required feature bundle [1][2]. This allows you to isolate the import of heavy motion features from your main application logic [3]. // features.js import { domAnimation } from "framer-motion"; export default domAnimation; 2. In your application component, import the LazyMotion component and the m component [1][2]. You must use the m component instead of the standard motion component to benefit from code splitting [4][1]. 3. Provide a loader function to the features prop of LazyMotion that dynamically imports your separate features file [1][2]. // App.jsx import { LazyMotion, m } from "framer-motion"; const loadFeatures = => import("./features.js").then(res => res.default); function App { return ( <m.div animate={{ x: 100 }} /> ); } Key concepts: - LazyMotion enables on-demand loading of animation and gesture features [4][1]. - The m component is a lightweight version of the motion component that relies on features provided by the nearest LazyMotion parent [1][2]. - Using the strict prop on LazyMotion will throw an error if you accidentally use a standard motion component, ensuring the bundle size benefits are maintained across your application [1][2][3]. - Feature bundles like domAnimation (for basic animations/gestures) or domMax (for full features including drag/pan) can be chosen based on your needs [3].
Citations:
- 1: https://www.mintlify.com/motiondivision/motion/api/lazy-motion
- 2: https://github.com/motiondivision/motion/blob/cd531784/packages/framer-motion/src/components/LazyMotion/types.ts
- 3: https://heckerspace.com/article/boosting-framer-motion-performance-and-accessibility
- 4: https://motion.dev/docs/react-lazy-motion
Split the LazyMotion feature bundle into a local module src/lib/motion/load-features.ts:14 should dynamic-import a sibling features.ts that re-exports domAnimation/domMax, then load that file (import('./features').then((mod) => mod.default)) instead of importing framer-motion directly; that keeps the animation payload isolated from the consumer bundle used at src/app/golf/(auth)/signup/page.tsx:7-8.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/golf/`(auth)/signup/page.tsx around lines 7 - 8, The LazyMotion
feature loader in loadFeatures should dynamically import a sibling features
module rather than importing framer-motion directly. Add features.ts to
re-export domAnimation/domMax, then have loadFeatures use
import('./features').then((mod) => mod.default) so the signup page’s animation
payload remains isolated.
…ader, Bridge ops wiring, 69 verified fixes M1 — shell & chrome (6 packets, per-packet adversarial verify): - More-sheet bottom nav replaces hamburger/drawer repo-wide: MoreNavSheet (vaul) with identity row, grouped overflow, per-product footer; AppShell drawer + hand-rolled focus trap deleted; FairwayBottomNav gains a 5th More button (aggregate badge, overflow-active state) - Registry-driven daily-loop bottom nav: golf 5->4; baseball 3->4+More via navKey stamping + getBaseballBottomNavKeys (per-program-type differentiator slots; showcase org/team scopes) - iOS-style condensing large-title header: LargeTitleContext provider + IntersectionObserver sentinel, FairwayLargeTitle primitive, crumbs + cmdK pill now desktop-only, single chrome band (flush sub-nav); three-tier anchor resolution (data-fw-title-anchor marker on all masthead primitives -> title-sized first child -> sentinel) - Instant tab swaps: useRouteRevealMotion + pure lateral-destination classifiers (golf/baseball/bridge registries) — zero cross-fade between tab roots, 180ms opacity reveal on detail pushes only; Bridge gets its first route transition (admin/template.tsx) - StatStrip primitive: KPI strips render 2x2 grids (<=4) or snap-rails (>=5) at 390px; desktop grids byte-identical; baseball KPIContentsStrip + admin overview/golf callers converted - Bridge mobile chrome: bottom nav (Overview/Errors/Health/Users + More), Errors badge from cheap incident count, thumb-reachable refresh + "updated Xs ago" freshness, banner-before-hero, CommandHeader condensed on phone Fix waves (15 packets from 69 adversarially-verified findings; all four P0s fixed): - P0: College Interest infinite skeleton for multi-team coaches (server team resolution + 8 sibling pages off .single()); baseball notification bell now deep-links per type; Recruiting Preferences card no longer bounces players to login; coach Dev-Plan insight chip dead-end - Bridge integrity: incident feed uncapped (fetchAllRowsResult), Sentry pagination + honest truncation flag, Overview/Activity "today" unified (status=completed), status banner copy no longer claims uncached data, security-events KPI renamed honestly, breakdown/feature chips are real filter links, cron lastError + integrity sample rows surfaced, /admin/work off the blocking await, deploy rows link commit + deployment - Golf rollups: activation funnel / adoption / retention / DAU-WAU-MAU now rendered on /admin/golf; baseball rollup parity - Sessions/audit: impersonation + revocation visible on user timelines; get_active_sessions(p_user_id) filters in SQL (migration applied to prod) - Passport/recruiting consent: visibility public states now require real recruiting activation; staff cannot silently flip exposure (honest rejection); self path auto-activates via non-bypassable gate + service-role write; self-preview invisibility banner + copy-link row - Polish: designed error/empty states, watchlist toggle failure surfaced, keyboard-accessible reorder, micro-motion pass (domAnimation only) DB (applied to prod + mirrored): 19 service policies re-scoped TO service_role; ~160 FK covering indexes (0 unindexed FKs remain); get_active_sessions per-user filter. docs/MOBILE_DOCTRINE.md codified (11 rules + craft bar + perf floor); inventory docs regenerated. Gates: tsc 0, eslint 0 (max-warnings 0), unit 465 files / 4818 passed, production build clean (unpiped exit codes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
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. |
njrini99-code
changed the title
coderabbitai
Bot
added
database
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/golf/approach-miss-selector.tsx (1)
58-83: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd accessible labels and pressed state to miss-direction buttons.
src/components/golf/approach-miss-selector.tsx:58-83renders interactive buttons whose visible labels are ambiguous (Lmeans both Left and Long), and selection is not exposed to assistive technology. Addaria-label={config.label}andaria-pressed={isSelected}.Proposed fix
<motion.button key={cell} type="button" onClick={() => onDirectionChange(cell)} disabled={disabled} + aria-label={`${config.label} miss`} + aria-pressed={isSelected} whileTap={{ scale: 0.95 }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/golf/approach-miss-selector.tsx` around lines 58 - 83, Add aria-label={config.label} and aria-pressed={isSelected} to each motion.button rendered in the approach miss-direction selector, using the existing config and isSelected values.Source: Path instructions
src/components/fairway/pages/team-hub/FairwayTeamHub.tsx (1)
386-420: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame floating-promise nit as
PlayerActionCenter.tsx.
badges.refetch();here is likewise an unhandled/un-voided promise call, for consistency withvoid refreshCount()used elsewhere in this PR'sNotificationBell.tsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/fairway/pages/team-hub/FairwayTeamHub.tsx` around lines 386 - 420, The badges refetch promise is invoked without being explicitly handled in handleCompleteTask. Update the success branch of handleCompleteTask to call badges.refetch() with the project’s established void pattern, matching NotificationBell.tsx.src/components/fairway/pages/dashboard/PlayerActionCenter.tsx (1)
100-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
badges.refetch()called withoutvoid/await, inconsistent with the siblingNotificationBell.tsxchange in this same PR.
NotificationBell.tsx(same cohort) usesvoid refreshCount()for fire-and-forget async calls; herebadges.refetch();is a bare floating-promise call. Not currently harmful sincefetchCountsin the provider appears to try/catch internally, but worth the explicitvoidfor consistency and to guard against a future unguarded rejection.- badges.refetch(); + void badges.refetch();(same fix applies to
handleRSVPat line 141)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/fairway/pages/dashboard/PlayerActionCenter.tsx` around lines 100 - 146, Mark both fire-and-forget badge refreshes with void to avoid floating promises and match NotificationBell.tsx: update the badges.refetch() calls in handleCompleteTask and handleRSVP to use void badges.refetch().src/components/coach/discover/PlayerCardGrid.tsx (1)
173-227: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDrop
initial/animatefrom the child cards
src/components/coach/discover/PlayerCardGrid.tsx:180andsrc/components/coach/discover/PlayerCardGrid.tsx:210:initial="hidden" animate="visible"on each childmotion.divbreaks the parent variant tree, sostaggerContainer’sstaggerChildrenwon’t sequence the cards and thereduceMotiongate at:176/:206won’t suppress the entrance animation. Keep onlyvariants={fadeUp}on the children and let the parent driveanimate="visible".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/coach/discover/PlayerCardGrid.tsx` around lines 173 - 227, Remove the explicit initial="hidden" and animate="visible" props from each child motion.div in PlayerCardGrid’s compact and grid render paths. Keep variants={fadeUp} so the parent motion.div controls sequencing through staggerContainer and respects the reduceMotion gate.src/components/baseball/passport/PassportVisibilityControls.tsx (1)
253-253: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
exposureEnabledgates a live public-link UI with unsaved draft state — user can copy/open a link before it's actually persisted.
exposureEnabled(line 253) is derived fromstate, the local draft the user is still editing — notinitialState, the server-confirmed value. Clicking a tile flipsexposureEnabledtotrueimmediately, beforeonSave(line 291) runs, so the "Copy link" / "Open" block at Line 413-441 appears and lets the user copy/share a URL that/baseball/player/[id]will still gate as private until Save actually persists. That's a real footgun: copy the link, send it to a recruiter, recruiter hits a 403 because the coach never clicked Save.Split the concerns — keep
exposureEnabledfor the draft-only field-visibility form controls, and gate the public-link block on the persisted state:🐛 Proposed fix — gate the live-link block on persisted state
const exposureEnabled = state === 'public_profile' || state === 'scout_packet'; + // Gates the *live* public-link block below. Deliberately keyed off + // `initialState` (server-confirmed), NOT `state` (unsaved draft) — the + // field-visibility form still uses `exposureEnabled` for its draft UX. + const publicLinkActive = initialState === 'public_profile' || initialState === 'scout_packet';- {exposureEnabled && ( + {publicLinkActive && ( <div className="mt-3 flex items-center justify-between gap-3 rounded-xl border border-warm-200 bg-cream-50 px-3.5 py-2.5"> <div className="flex min-w-0 items-center gap-2"> <IconEye size={14} className="shrink-0 text-warm-400" /> <p className="truncate text-sm text-warm-600">{publicProfilePath}</p> </div> + {dirty && ( + <p className="shrink-0 text-eyebrow text-warm-400">Unsaved changes</p> + )} <div className="flex shrink-0 items-center gap-1">Also applies to: 318-331, 412-441
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/baseball/passport/PassportVisibilityControls.tsx` at line 253, Separate draft visibility controls from live-link availability in PassportVisibilityControls: retain exposureEnabled derived from state for form fields, but derive the Copy/Open public-link block near the live-link UI from initialState (the server-confirmed value) instead. Ensure clicking a tile before onSave cannot reveal or enable the live link, while saved public_profile or scout_packet states still do.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/admin/_components/__tests__/relative-time.test.tsx`:
- Around line 9-61: Add a test in the RelativeTime describe block that renders
the component with an interval, captures the unmount function, unmounts it, and
advances fake timers inside act to verify no error occurs after teardown. This
should cover cleanup of the interval and visibilitychange listener.
In `@src/app/admin/errors/page.tsx`:
- Around line 226-239: Add the canonical focus-visible classes used by KpiTile
and ErrorsFilterChips to the Link className in the source-breakdown chip
rendering, including focus-visible:outline, focus-visible:outline-2,
focus-visible:outline-offset-2, and focus-visible:outline-accent-500.
In `@src/app/admin/golf/tracer/StuckRoundsPanel.tsx`:
- Around line 1-46: Replace the render-time new Date().toISOString() passed to
PanelAllClear in StuckRoundsPanel with a hydration-stable value: initialize
checkedAt after mount using useEffect and render a stable placeholder until
available, or accept a server-provided timestamp. Ensure PanelAllClear receives
the same value during server render and initial client hydration to avoid
mismatches.
In `@src/app/admin/golf/tracer/TracerIncidentRow.tsx`:
- Around line 1-49: Replace the direct `new
Date(incident.lastSeen).toLocaleDateString()` rendering in `TracerIncidentRow`
with the shared `LocalTime` formatter, passing `incident.lastSeen` and the
appropriate date-only format; preserve the existing styling so server and client
output remain consistent.
In `@src/app/admin/golf/tracer/TracerPlayerList.tsx`:
- Around line 44-45: Remove the independent .slice(0, 100) cap from the player
rendering in TracerPlayerList, mapping over the complete players collection so
all server-returned rows are displayed. Keep the existing truncated-based footer
behavior as the sole indication of server-side limits.
- Around line 55-63: Replace the non-canonical accent color classes in the
TracerPlayerList markup with the appropriate canonical design-system color
family, using primary, destructive, warm, or cream tokens while preserving the
existing visual intent for the selected player background and indicator.
In `@src/app/admin/golf/tracer/TracerRoundDiagnostic.tsx`:
- Around line 143-186: Prevent concurrent round fixes by gating every action
while a fix is pending. Update the buttons rendered from RECALC_ACTIONS, the
“Resolve as stuck…” button, and confirmation controls in the
TracerRoundDiagnostic component to disable interactions whenever pendingFix is
not null, while preserving each action’s busy state.
In `@src/app/admin/page.tsx`:
- Around line 353-357: Fix the Shared badge/drill-down mismatch in the
sportCounts definition and the corresponding errors filtering logic: either
count only explicit “shared” items in the badge, or update the “shared” filter
used by the errors view to include null sports as well. Keep the count and the
?sport=shared drill-down behavior consistent across sportCounts and the relevant
filtering functions in errors data handling.
- Around line 99-107: The StatStrip count is manually duplicated and can drift
from the rendered KPI tiles. In the admin page, identify the KPI collection used
to render the six KpiTile children and pass its length to the count prop instead
of the literal 6, keeping the existing column configuration unchanged.
In `@src/app/admin/users/`[id]/ViewAsButton.tsx:
- Around line 20-52: Preserve accessibility when ViewAsButton switches to
confirmation mode: use a ref and effect to move focus to the confirmation
control after setting confirming, and add an aria-live region announcing the
confirmation state. Update ViewAsButton so keyboard and screen-reader users are
informed and retain focus when the original button is replaced.
In `@src/app/golf/`(dashboard)/FairwayDashboardShell.tsx:
- Around line 539-549: Memoize the FairwayBottomNav element before passing it to
AppShell, matching the stabilization pattern used by brand, sidebarFooter,
moreSheetFooter, subNav, and teamSwitcher. Add a useMemo for bottomNav using
bottomNavItems, pathname, openMoreSheet, more.active, more.badge, and mobileOpen
as dependencies, then pass the memoized value into AppShell.
In `@src/components/baseball/living-annual/molecules/KPIContentsStrip.tsx`:
- Around line 58-80: Replace the post-reorder index-based key in the
KPIContentsStrip render with a stable key derived from each item's original
index. Preserve the original index while building the pin-first partition, then
use that index for each RuledStatLine key so toggling leader or emphasis does
not remount existing items.
In `@src/components/baseball/peek-panel/QuickStatsDisplay.tsx`:
- Around line 20-39: Preserve stat-specific decimal precision instead of
deriving it from integer status in StatFigure. Update the stats construction in
PlayerPeekContent for pitchVelo, exitVelo, sixtyTime, popTime, and gpa to
provide explicit decimal metadata or preformatted values, then update StatFigure
to honor that precision so values such as 3.75 and 2.34 are not rounded.
In `@src/components/baseball/performance/PlayerReadinessClient.tsx`:
- Around line 192-207: Update the completion branch in PlayerReadinessClient’s
saved-state rendering so the “Check-in saved” confirmation is exposed as a
status/live region to assistive technologies. Add appropriate status semantics
to the PaperCard or its confirmation container while preserving the existing
visual layout and secondary notice behavior.
In `@src/components/baseball/recruiting-philosophy/PositionPriorityRanker.tsx`:
- Around line 202-218: Replace the one-off sizing on both reorder Buttons in the
keyboard-equivalent reorder controls with the shared 44px hit-target button
pattern, removing min-h-0 and p-0.5 while preserving their handlers, disabled
states, labels, and visual styling.
In `@src/components/fairway/app-shell/LargeTitleContext.tsx`:
- Around line 136-150: Cache the result of measureSafeAreaInsetTop at module
scope so the DOM probe and synchronous layout calculation run only once instead
of on every effect rerun. Update the effect using measureSafeAreaInsetTop to
reuse the cached value, while preserving the existing fallback behavior and
allowing the cache to be refreshed only when safe-area conditions can change,
such as rotation or fullscreen transitions.
In `@src/components/fairway/app-shell/more-nav.ts`:
- Around line 24-32: Remove the duplicate local matchActive implementations and
make more-nav.ts the shared source of truth by exporting matchActive alongside
selectOverflow. Update MoreNavSheet.tsx and FairwayBottomNav.tsx to import and
use matchActive from ./more-nav, then delete their local copies and related
comments while preserving existing behavior.
In `@src/components/fairway/app-shell/MoreNavSheet.tsx`:
- Around line 196-207: Replace the custom avatar block with the shared Avatar
component, passing user.name, user.avatarUrl ?? undefined, size="md", and
square; otherwise remove role="img" and mark the decorative block
aria-hidden="true" since the adjacent user.name provides the label.
In `@src/components/fairway/pages/coachhelm/FairwayPlayerInsight.tsx`:
- Around line 602-619: Update the deep-link useEffect in FairwayPlayerInsight to
consume the `#insight-` hash after successfully finding and scrolling to its
element. Remove the hash via the appropriate history/router replacement without
triggering a reload, while preserving other URL components, so subsequent
refreshes do not repeat the scroll.
In `@src/components/fairway/pages/travel/FairwayExpenseList.tsx`:
- Around line 64-90: Extract the shared expense-category label and color
mappings from FairwayExpenseList’s CATEGORY_LABEL/CATEGORY_SWATCH and
FairwayExpenseSummary’s CATEGORY_CONFIG into a new expense-category module.
Update both components to import and use the shared table, while keeping
CATEGORY_ICON local to FairwayExpenseList, and remove the duplication/comment
referencing identical mappings.
- Around line 366-374: Add a restrictive sandbox attribute to the receipt iframe
in the FairwayExpenseList component, leaving it without permissions such as
scripts, forms, or top-level navigation. Keep the existing PDF source,
load/error handlers, and display styling unchanged.
In `@src/components/fairway/pages/travel/FairwayTravel.tsx`:
- Around line 155-176: Respect reduced-motion preferences in the deep-link
scroll effect. In FairwayTravel’s component, import and call framer-motion’s
useReducedMotion hook, then use its value in the requestAnimationFrame
scrollIntoView call to choose 'auto' for reduced-motion users and 'smooth'
otherwise, while preserving block: 'start'.
In `@src/components/golf/putt-miss-tag-selector.tsx`:
- Around line 59-64: In the selected-state class within the putt miss tag
selector styling, replace the undefined text-text-on-accent token with the
configured text-on-accent token so selected labels use the intended foreground
color.
In `@src/lib/admin/data/baseball.ts`:
- Around line 71-134: Fix the rolling date boundaries used by `gamesThisWeek`
and `gamesLastWeek` in the baseball metrics calculation. Derive `ago7dIso` and
`ago14dIso` from the UTC start of `todayIso`, subtracting whole calendar days
before converting back to date strings, then use those corrected strings in the
existing game-count loop.
- Around line 66-160: Update fetchBaseballTab to inspect gamesRes.error,
liftCountRes.error, and liftAthletesRes.error after the parallel queries,
throwing or returning a typed error envelope when any query fails instead of
defaulting failed data to empty arrays or zero counts. Update BaseballBody and
its surrounding PanelBoundary integration to detect that failure and render
PanelStale, while preserving valid zero values for successful queries.
In `@src/lib/admin/data/users.ts`:
- Around line 51-72: Team-filtered counts are based on the capped users array
and can underreport rosters. Update the users-loading/query flow around the team
resolution, platform-wide limit, and usersInTeam filtering so team IDs are
applied before the 500-user cap, or compute the count from an uncapped team
membership set; then update computeTotalUsersCount to use that complete
team-scoped count while preserving SQL counts for unfiltered queries.
In `@src/lib/admin/sentry-api.ts`:
- Around line 23-28: Bound the total duration of fetchSentryIssues rather than
allowing up to MAX_PAGES sequential requests to stall the force-dynamic
dashboard indefinitely. Add a deadline or timeout mechanism around each Sentry
request/overall pagination loop, ensure the loop exits cleanly when the deadline
is reached, and preserve the truncated indication whenever pagination stops
before the cursor is exhausted.
In
`@supabase/migrations/20260710004100_fk_covering_indexes_batch1_admin_baseball_a_m.sql`:
- Around line 4-42: The index definitions are acceptable as-is; no code change
is required for this migration. For future large-table FK indexes such as those
on baseball_batted_ball_events, use CREATE INDEX CONCURRENTLY in a separate
non-transactional migration rather than this transactional batch.
---
Outside diff comments:
In `@src/components/baseball/passport/PassportVisibilityControls.tsx`:
- Line 253: Separate draft visibility controls from live-link availability in
PassportVisibilityControls: retain exposureEnabled derived from state for form
fields, but derive the Copy/Open public-link block near the live-link UI from
initialState (the server-confirmed value) instead. Ensure clicking a tile before
onSave cannot reveal or enable the live link, while saved public_profile or
scout_packet states still do.
In `@src/components/coach/discover/PlayerCardGrid.tsx`:
- Around line 173-227: Remove the explicit initial="hidden" and
animate="visible" props from each child motion.div in PlayerCardGrid’s compact
and grid render paths. Keep variants={fadeUp} so the parent motion.div controls
sequencing through staggerContainer and respects the reduceMotion gate.
In `@src/components/fairway/pages/dashboard/PlayerActionCenter.tsx`:
- Around line 100-146: Mark both fire-and-forget badge refreshes with void to
avoid floating promises and match NotificationBell.tsx: update the
badges.refetch() calls in handleCompleteTask and handleRSVP to use void
badges.refetch().
In `@src/components/fairway/pages/team-hub/FairwayTeamHub.tsx`:
- Around line 386-420: The badges refetch promise is invoked without being
explicitly handled in handleCompleteTask. Update the success branch of
handleCompleteTask to call badges.refetch() with the project’s established void
pattern, matching NotificationBell.tsx.
In `@src/components/golf/approach-miss-selector.tsx`:
- Around line 58-83: Add aria-label={config.label} and aria-pressed={isSelected}
to each motion.button rendered in the approach miss-direction selector, using
the existing config and isSelected values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 209a0ba9-5b84-4b96-bc87-c709c39a77d2
⛔ Files ignored due to path filters (1)
src/lib/types/database.tsis excluded by!src/lib/types/database.ts
📒 Files selected for processing (179)
docs/MOBILE_DOCTRINE.mdmemory/projects/golfhelm.mdsrc/app/admin/_components/AdminShell.tsxsrc/app/admin/_components/AdminStatusBanner.tsxsrc/app/admin/_components/FeatureHealthRollup.tsxsrc/app/admin/_components/RelativeTime.tsxsrc/app/admin/_components/__tests__/admin-nav.test.tssrc/app/admin/_components/__tests__/admin-status-banner.test.tsxsrc/app/admin/_components/__tests__/relative-time.test.tsxsrc/app/admin/_components/admin-nav.tssrc/app/admin/actions/golf-tracer.tssrc/app/admin/actions/sessions.tssrc/app/admin/actions/view-as.tssrc/app/admin/auth/page.tsxsrc/app/admin/baseball/__tests__/page.test.tsxsrc/app/admin/baseball/page.tsxsrc/app/admin/deploys/page.tsxsrc/app/admin/errors/page.tsxsrc/app/admin/golf/page.tsxsrc/app/admin/golf/tracer/StuckRoundsPanel.tsxsrc/app/admin/golf/tracer/TracerIncidentRow.tsxsrc/app/admin/golf/tracer/TracerPlayerList.tsxsrc/app/admin/golf/tracer/TracerRoundDiagnostic.tsxsrc/app/admin/golf/tracer/page.tsxsrc/app/admin/golf/tracer/tracer-shared.tssrc/app/admin/jobs/page.tsxsrc/app/admin/layout.tsxsrc/app/admin/page.tsxsrc/app/admin/template.tsxsrc/app/admin/users/[id]/ViewAsButton.tsxsrc/app/admin/users/[id]/page.tsxsrc/app/admin/users/page.tsxsrc/app/admin/work/page.tsxsrc/app/baseball/(dashboard)/BaseballFairwayShell.tsxsrc/app/baseball/(dashboard)/dashboard/analytics/loading.tsxsrc/app/baseball/(dashboard)/dashboard/college-interest/CollegeInterestClient.tsxsrc/app/baseball/(dashboard)/dashboard/college-interest/page.tsxsrc/app/baseball/(dashboard)/dashboard/dev-plan/loading.tsxsrc/app/baseball/(dashboard)/dashboard/discover/DiscoverClient.tsxsrc/app/baseball/(dashboard)/dashboard/loading.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/page.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/passport/page.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/stats/page.tsxsrc/app/baseball/(dashboard)/dashboard/postgame/page.tsxsrc/app/baseball/(dashboard)/dashboard/practice/loading.tsxsrc/app/baseball/(dashboard)/dashboard/profile/page.tsxsrc/app/baseball/(dashboard)/dashboard/settings/page.tsxsrc/app/baseball/(dashboard)/dashboard/settings/program/loading.tsxsrc/app/baseball/(dashboard)/dashboard/signals/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/[gameId]/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/create/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/upload/page.tsxsrc/app/baseball/(dashboard)/dashboard/template.tsxsrc/app/baseball/(player-dashboard)/player/passport/page.tsxsrc/app/baseball/(player-dashboard)/player/template.tsxsrc/app/baseball/(player-dashboard)/player/today/page.tsxsrc/app/baseball/(public)/player/[id]/PlayerProfileClient.tsxsrc/app/baseball/(public)/player/[id]/page.tsxsrc/app/baseball/actions/passport-settings.tssrc/app/golf/(dashboard)/FairwayDashboardShell.tsxsrc/app/golf/(dashboard)/dashboard/calendar/page.tsxsrc/app/golf/(dashboard)/dashboard/messages/loading.tsxsrc/app/golf/(dashboard)/dashboard/tasks/page.tsxsrc/app/golf/(dashboard)/dashboard/template.tsxsrc/app/golf/(dashboard)/dashboard/travel/loading.tsxsrc/app/golf/(dashboard)/dashboard/travel/page.tsxsrc/app/golf/admin/components/BusinessIntelligenceTab.tsxsrc/app/golf/admin/components/GrowthCard.tsxsrc/app/lifting/(dashboard)/dashboard/check-ins/check-ins-client.tsxsrc/components/baseball/NotificationBell.tsxsrc/components/baseball/box-score/BoxScoreView.tsxsrc/components/baseball/living-annual/SectionMasthead.tsxsrc/components/baseball/living-annual/molecules/KPIContentsStrip.test.tsxsrc/components/baseball/living-annual/molecules/KPIContentsStrip.tsxsrc/components/baseball/passport/PassportVisibilityControls.tsxsrc/components/baseball/peek-panel/PlayerQuickActions.tsxsrc/components/baseball/peek-panel/QuickStatsDisplay.tsxsrc/components/baseball/performance/ExerciseWizard.tsxsrc/components/baseball/performance/LiftBuilderClient.tsxsrc/components/baseball/performance/PlayerLiftToday.tsxsrc/components/baseball/performance/PlayerReadinessClient.tsxsrc/components/baseball/performance/lift-canvas/ExerciseBin.tsxsrc/components/baseball/performance/lift-canvas/LiftCanvas.tsxsrc/components/baseball/performance/lift-canvas/SessionBlock.tsxsrc/components/baseball/player-today/PlayerTodayClient.tsxsrc/components/baseball/recruiting-philosophy/PositionPriorityRanker.tsxsrc/components/baseball/stat-visuals/chart-primitives.tsxsrc/components/baseball/tasks/TaskCard.tsxsrc/components/coach/discover/PlayerCardGrid.tsxsrc/components/fairway/app-shell/AppShell.tsxsrc/components/fairway/app-shell/FairwayBottomNav.tsxsrc/components/fairway/app-shell/FairwayLargeTitle.test.tsxsrc/components/fairway/app-shell/FairwayLargeTitle.tsxsrc/components/fairway/app-shell/FairwayTopBar.test.tsxsrc/components/fairway/app-shell/FairwayTopBar.tsxsrc/components/fairway/app-shell/LargeTitleContext.test.tsxsrc/components/fairway/app-shell/LargeTitleContext.tsxsrc/components/fairway/app-shell/MoreNavSheet.tsxsrc/components/fairway/app-shell/MoreSheetFooter.tsxsrc/components/fairway/app-shell/index.tssrc/components/fairway/app-shell/more-nav.test.tssrc/components/fairway/app-shell/more-nav.tssrc/components/fairway/app-shell/types.tssrc/components/fairway/charts/StatStrip.test.tsxsrc/components/fairway/charts/StatStrip.tsxsrc/components/fairway/charts/index.tssrc/components/fairway/controls/badge.tsxsrc/components/fairway/controls/status-pill.tsxsrc/components/fairway/index.tssrc/components/fairway/pages/calendar/FairwayCalendar.tsxsrc/components/fairway/pages/calendar/FairwayEventDetailDrawer.tsxsrc/components/fairway/pages/calendar/FairwayEventEditor.tsxsrc/components/fairway/pages/calendar/FairwayMonthGrid.tsxsrc/components/fairway/pages/calendar/__tests__/FairwayEventDetailDrawer.test.tsxsrc/components/fairway/pages/coachhelm/FairwayCoachHelmSignals.tsxsrc/components/fairway/pages/coachhelm/FairwayEffectiveness.tsxsrc/components/fairway/pages/coachhelm/FairwayPlayerCoachHelm.tsxsrc/components/fairway/pages/coachhelm/FairwayPlayerInsight.tsxsrc/components/fairway/pages/coachhelm/FocusAreaCard.tsxsrc/components/fairway/pages/coachhelm/InsightTrustChips.tsxsrc/components/fairway/pages/dashboard/FairwayPlayerDashboard.tsxsrc/components/fairway/pages/dashboard/PlayerActionCenter.tsxsrc/components/fairway/pages/dashboard/player-dashboard-parts.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifierLeaderboard.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifyingWorkspace.tsxsrc/components/fairway/pages/rounds/FairwayUnfinishedBanner.tsxsrc/components/fairway/pages/tasks/FairwayTasks.tsxsrc/components/fairway/pages/team-hub/FairwayTeamHub.tsxsrc/components/fairway/pages/travel/FairwayExpenseList.tsxsrc/components/fairway/pages/travel/FairwayExpenseSummary.tsxsrc/components/fairway/pages/travel/FairwayTravel.tsxsrc/components/fairway/pages/travel/FairwayTripDetail.tsxsrc/components/fairway/view-header/view-header.tsxsrc/components/golf/approach-miss-selector.tsxsrc/components/golf/calendar/AttendancePanel.tsxsrc/components/golf/coachhelm/insights/FocusAreaCard.tsxsrc/components/golf/coachhelm/insights/PlayerFocusAreas.tsxsrc/components/golf/coachhelm/player/FocusAreasGrid.tsxsrc/components/golf/layout/LargeTitleHeader.tsxsrc/components/golf/putt-miss-tag-selector.tsxsrc/components/lifting/shell/LabNav.tsxsrc/components/lifting/shell/SportTabBar.tsxsrc/hooks/golf/use-task-realtime.tssrc/lib/admin-logger.tssrc/lib/admin/__tests__/sentry-api.test.tssrc/lib/admin/data/__tests__/baseball.test.tssrc/lib/admin/data/__tests__/users.test.tssrc/lib/admin/data/auth.tssrc/lib/admin/data/baseball.tssrc/lib/admin/data/golf.tssrc/lib/admin/data/incident-feed.tssrc/lib/admin/data/jobs.tssrc/lib/admin/data/overview.tssrc/lib/admin/data/user-engagement.tssrc/lib/admin/data/users.tssrc/lib/admin/fetch-result.tssrc/lib/admin/sentry-api.tssrc/lib/baseball/__tests__/bottom-nav.test.tssrc/lib/baseball/__tests__/nav-manifest.test.tssrc/lib/baseball/bottom-nav.tssrc/lib/baseball/nav-registry.tssrc/lib/baseball/program-type-variants.tssrc/lib/baseball/server-route-guards.tssrc/lib/bridge/nav-registry.test.tssrc/lib/bridge/nav-registry.tssrc/lib/golf/nav-registry.test.tssrc/lib/golf/nav-registry.tssrc/lib/motion/route-motion.tssrc/styles/design-tokens.csssrc/test/golf/calendar-page.test.tsxsupabase/migrations/20260710004000_rescope_service_policies_to_service_role.sqlsupabase/migrations/20260710004100_fk_covering_indexes_batch1_admin_baseball_a_m.sqlsupabase/migrations/20260710004200_fk_covering_indexes_batch2_baseball_p_z.sqlsupabase/migrations/20260710004300_fk_covering_indexes_batch3_crm_email_golf.sqlsupabase/migrations/20260710004400_fk_covering_indexes_batch4_helm_lifting.sqlsupabase/migrations/20260710010000_get_active_sessions_per_user_filter.sqltailwind.config.ts
💤 Files with no reviewable changes (1)
- src/lib/admin/data/user-engagement.ts
Comment on lines
+1
to
+49
| import { ChevronDown } from 'lucide-react'; | ||
| import { StatusPill } from '@/components/fairway'; | ||
| import type { TracerIncident } from '@/app/golf/actions/admin-tracer-data'; | ||
| import { TRACER_SEVERITY_TONE } from './tracer-shared'; | ||
|
|
||
| /** | ||
| * One tracer incident, collapsed to severity/title/occurrences/lastSeen — | ||
| * expand-on-click (native `<details>`, no client JS needed) reveals the | ||
| * remediation narrative (`summary`/`whyItHappened`/`howToFix`/ | ||
| * `operatorImpact`) the backend already computes but the list previously | ||
| * dropped on the floor. Presentational only — safe to render from a Server | ||
| * Component (the main incidents list) or from inside a Client Component | ||
| * (the round-scoped diagnostic view); no hooks, no 'use client' needed. | ||
| */ | ||
| export function TracerIncidentRow({ incident }: { incident: TracerIncident }) { | ||
| return ( | ||
| <li> | ||
| <details className="group py-2"> | ||
| <summary | ||
| className="flex cursor-pointer list-none flex-wrap items-center gap-x-3 gap-y-1 text-sm [&::-webkit-details-marker]:hidden" | ||
| > | ||
| <StatusPill tone={TRACER_SEVERITY_TONE[incident.severity]} dot size="sm"> | ||
| {incident.severity} | ||
| </StatusPill> | ||
| <span className="min-w-0 flex-1 basis-full truncate text-warm-900 sm:basis-auto"> | ||
| {incident.title} | ||
| </span> | ||
| <span className="font-fw-mono text-xs tabular-nums text-warm-600"> | ||
| {incident.occurrences} occurrence{incident.occurrences === 1 ? '' : 's'} | ||
| </span> | ||
| <span className="font-fw-mono text-xs tabular-nums text-warm-500"> | ||
| {new Date(incident.lastSeen).toLocaleDateString()} | ||
| </span> | ||
| <ChevronDown | ||
| size={14} | ||
| className="shrink-0 text-warm-400 transition-transform duration-150 group-open:rotate-180" | ||
| aria-hidden | ||
| /> | ||
| </summary> | ||
| <div className="mt-2 space-y-1.5 rounded-lg bg-surface-sunken p-3 text-xs leading-5 text-warm-700"> | ||
| <p><span className="font-semibold text-warm-900">What happened — </span>{incident.summary}</p> | ||
| <p><span className="font-semibold text-warm-900">Why — </span>{incident.whyItHappened}</p> | ||
| <p><span className="font-semibold text-warm-900">How to fix — </span>{incident.howToFix}</p> | ||
| <p><span className="font-semibold text-warm-900">Operator impact — </span>{incident.operatorImpact}</p> | ||
| </div> | ||
| </details> | ||
| </li> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether TracerRoundDiagnostic.tsx is a Client Component and renders TracerIncidentRow.
fd TracerRoundDiagnostic.tsx --exec cat -n {}Repository: njrini99-code/helmv3
Length of output: 9175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== TracerIncidentRow location ==\n'
cat -n src/app/admin/golf/tracer/TracerIncidentRow.tsx
printf '\n== LocalTime component search ==\n'
rg -n "function LocalTime|const LocalTime|export .*LocalTime|<LocalTime" src -g '!**/node_modules/**'Repository: njrini99-code/helmv3
Length of output: 4601
Use the shared LocalTime formatter for lastSeen src/app/admin/golf/tracer/TracerIncidentRow.tsx:32 is rendered from the client diagnostic view (src/app/admin/golf/tracer/TracerRoundDiagnostic.tsx:1-11), so new Date(...).toLocaleDateString() can hydrate differently between server and browser. Swap this to src/app/admin/_components/LocalTime.tsx or pin locale/timezone explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/golf/tracer/TracerIncidentRow.tsx` around lines 1 - 49, Replace
the direct `new Date(incident.lastSeen).toLocaleDateString()` rendering in
`TracerIncidentRow` with the shared `LocalTime` formatter, passing
`incident.lastSeen` and the appropriate date-only format; preserve the existing
styling so server and client output remain consistent.
Comment on lines
+44
to
+45
| <ul className="mt-2 divide-y divide-warm-200/60"> | ||
| {players.slice(0, 100).map((p, i) => ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Client-side 100-row cap silently hides data beyond what truncated reports.
truncated reflects the server's bounded-query cap. The separate .slice(0, 100) here is an independent cap that isn't reflected in the footer message — if a team has >100 players but the server query wasn't capped, rows past #100 disappear with no indication to the admin.
🐛 Proposed fix
- <ul className="mt-2 divide-y divide-warm-200/60">
- {players.slice(0, 100).map((p, i) => (
+ {(() => {
+ const visiblePlayers = players.slice(0, 100);
+ const clientCapped = players.length > visiblePlayers.length;
+ return (
+ <ul className="mt-2 divide-y divide-warm-200/60">
+ {visiblePlayers.map((p, i) => (
<li key={p.player_id}>
...
</li>
))}
</ul>
- {truncated ? <p className="mt-2 text-xs text-warm-500">Showing latest data — the bounded query hit its row cap.</p> : null}
+ {(truncated || clientCapped) ? <p className="mt-2 text-xs text-warm-500">Showing latest data — the bounded query hit its row cap.</p> : null}
+ );
+ })()}Also applies to: 79-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/golf/tracer/TracerPlayerList.tsx` around lines 44 - 45, Remove
the independent .slice(0, 100) cap from the player rendering in
TracerPlayerList, mapping over the complete players collection so all
server-returned rows are displayed. Keep the existing truncated-based footer
behavior as the sole indication of server-side limits.
…ations CodeRabbit remediation, 6 clusters: - Token policy: banned #DC2626 literal in CRM KPIGrid -> semantic tokens; accent-* off non-Fairway admin surfaces -> primary-*; DatelineRule extracted as a shared ui/card primitive (5 hand-copies consolidated) - Motion: layoutId/domAnimation mismatch resolved; stagger initial/animate placement fixed; LazyMotion feature payload isolated - Bridge/tracer integrity: silent zero-data error states surfaced, hydration-safe timestamps, fix-action race guarded, KPI counts pinned to their own source data, capped user query paginated - Golf shells: dynamic() loading skeletons sized to content (no layout jump / empty-modal flash), unmemoized AppShell prop memoized, calendar dot color mapping completed - Baseball: realtime INSERT listener + visibility ref fixed, unsaved-draft no longer exposes a live public link, decimal stat precision, a11y live-regions + hit targets - App-shell/travel: receipt iframe SANDBOXED, safe-area probe cached (one reflow per page load, not per nav), matchActive single-sourced, deep-link hash cleared after scroll + reduced-motion honored, Avatar primitive reused in MoreNavSheet - Several findings rebutted with evidence (already fixed in b34faee, or deliberate severity-ramp idiom matching ui/badge) Migrations rewritten replay-safe: all five 2026-07-10 files are now existence-guarded DO blocks (indexes/policies generated from live prod catalogs; CI shadow-DB replays lack drifted columns like baseball_ai_audit.outcome_by). Prod unchanged - already applied+verified. Stale AdminStatusBanner test updated to assert the new honest copy. Gates: tsc 0, eslint 0, unit 465 files / 4804 passed, build 0 (unpiped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
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. |
There was a problem hiding this comment.
Actionable comments posted: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/golf/putt-miss-tag-selector.tsx (1)
52-65: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLine 52-65: Expose the selected tag state to assistive technology.
These buttons behave as toggles, but only the visual class changes when
isSelectedchanges. Addaria-pressed={isSelected}so screen readers can determine which miss tags are active.Proposed fix
<motion.button key={tag} type="button" onClick={() => toggleTag(tag)} disabled={disabled} + aria-pressed={isSelected}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/golf/putt-miss-tag-selector.tsx` around lines 52 - 65, Add aria-pressed={isSelected} to each toggle button rendered in the putt miss tag selector, alongside the existing disabled and click behavior, so assistive technology can identify active tags.Source: Path instructions
src/components/baseball/performance/lift-canvas/SessionBlock.tsx (1)
139-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
DatelineRulefor the conflict marker.Line 139 duplicates the shared
h-[2px] w-7rule geometry. RenderDatelineRulewithtone={conflictRule}andclassName="mb-0.5"so this surface stays aligned with the shared primitive.As per coding guidelines and path instructions, reuse shared design-system primitives instead of introducing one-off geometry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/baseball/performance/lift-canvas/SessionBlock.tsx` at line 139, Replace the inline conflict marker in SessionBlock with the shared DatelineRule component, passing conflictRule as tone and "mb-0.5" as className while preserving conditional rendering and accessibility behavior.Sources: Coding guidelines, Path instructions
src/app/admin/users/page.tsx (1)
168-205: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply
activeTeamIdas an actual team filter.
?team=…only expands the matching panel; KPI counts, watchlist, and other team panels remain platform-wide. Filterteamsbefore derivingplayers, and usefilteredTeams.lengthfor “Teams mapped.”Proposed fix
const filteredTeams = teams + .filter((team) => !activeTeamId || team.teamId === activeTeamId) .filter((team) => !sport || team.sport === sport) @@ -<StatTile label="Teams mapped" value={teams.length} tone="neutral" mono /> +<StatTile label="Teams mapped" value={filteredTeams.length} tone="neutral" mono />Also applies to: 246-248, 367-367
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/admin/users/page.tsx` around lines 168 - 205, Apply activeTeamId as a team filter before deriving players, watchlist, and KPI values: restrict teams to the matching team when activeTeamId is set, otherwise retain all teams. Update the “Teams mapped” statistic to use filteredTeams.length, and ensure all team panels and derived counts use the filtered collection rather than the platform-wide teams array.src/components/fairway/app-shell/AppShell.tsx (1)
292-303: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear the controlled sheet state when switching to desktop.
In
src/components/fairway/app-shell/AppShell.tsx, Line 428 unmounts the sheet without resetting a controlledmobileOpen. Rotating back to mobile therefore reopens it unexpectedly.Proposed fix
const isDesktop = useMediaQuery('(min-width: 768px)'); + +useEffect(() => { + if (isDesktop && mobileOpen) { + setMobileOpen(false); + } +}, [isDesktop, mobileOpen, setMobileOpen]);Also applies to: 423-440
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/fairway/app-shell/AppShell.tsx` around lines 292 - 303, Reset the controlled mobile sheet state when the viewport switches to desktop. In AppShell, update the logic around the isDesktop-dependent rendering and the mobileOpen state/setter so that transitioning to desktop explicitly calls the setter with false before unmounting the sheet, preventing it from reopening when returning to mobile.src/components/fairway/app-shell/FairwayBottomNav.tsx (1)
168-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo component test for the new interactive "More" column.
The 5th column touches
aria-haspopup,aria-expanded, badge capping (9+), andonMoreOpen— all correctness-sensitive for a11y.more-nav.test.tsonly covers the pure helpers, not this render path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/fairway/app-shell/FairwayBottomNav.tsx` around lines 168 - 225, Add component tests for the interactive More column rendered by FairwayBottomNav. Cover aria-haspopup="dialog", aria-expanded updates, invoking onMoreOpen, conditional badge rendering including the 9+ cap, and the absence of the column when onMoreOpen is unavailable; keep existing more-nav.test.ts helper coverage unchanged.
♻️ Duplicate comments (2)
src/app/admin/golf/tracer/TracerPlayerList.tsx (1)
61-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNon-canonical
accent-*color tokens still unaddressed.Same violation flagged previously —
bg-accent-50/60(line 63) andbg-accent-500(line 68) aren't part of the canonical family (primary-*,destructive,warm-*,cream-*).🎨 Proposed fix
className={cn( 'block h-auto w-full rounded-lg border-0 px-2 py-2 text-left text-sm font-normal leading-5', - i === 0 ? 'bg-accent-50/60' : undefined, + i === 0 ? 'bg-primary-50/60' : undefined, )} > <span className="flex flex-wrap items-center gap-x-3 gap-y-1"> <span className="min-w-0 flex-1 basis-full sm:basis-auto"> - {i === 0 && <span aria-hidden className="mb-1 block h-[2px] w-7 rounded-full bg-accent-500" />} + {i === 0 && <span aria-hidden className="mb-1 block h-[2px] w-7 rounded-full bg-primary-600" />}As per coding guidelines, "Use only canonical design-system color families and surfaces:
primary-*,destructive,warm-*, andcream-*".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/admin/golf/tracer/TracerPlayerList.tsx` around lines 61 - 68, Replace the non-canonical bg-accent-50/60 and bg-accent-500 classes in the TracerPlayerList row styling with appropriate canonical design-system color tokens from the primary, destructive, warm, or cream families, preserving the existing visual hierarchy.Source: Coding guidelines
src/app/admin/golf/tracer/TracerIncidentRow.tsx (1)
28-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winInconsistent with the rest of this feature's date-rendering pattern — swap to
LocalTime.Every other timestamp in this tracer feature (
StuckRoundsPanel'supdated_at,TracerPlayerList'slast_activity/round_date) routes through the sharedLocalTimecomponent specifically to avoid server/client locale divergence. This row still callsnew Date(incident.lastSeen).toLocaleDateString()directly. Current call sites don't trip a live hydration mismatch today (theTracerRoundDiagnosticmount is gated behind post-hydration state, and thepage.tsxusage is a pure Server Component render), but the file's own doc comment claims unconditional safety in "Server Component or Client Component" contexts — that's only true today because of how callers happen to gate the render, not because the component itself is locale-safe. A future unconditional client-side render (e.g. a live-updating incident ticker) would reintroduce exactly the bug the sibling components were fixed to avoid.🩹 Proposed fix — use LocalTime for consistency
- <span className="font-fw-mono text-xs tabular-nums text-warm-500"> - {new Date(incident.lastSeen).toLocaleDateString()} - </span> + <span className="font-fw-mono text-xs tabular-nums text-warm-500"> + <LocalTime iso={incident.lastSeen} variant="date" /> + </span>Verify
LocalTimeactually exposes a date-onlyvariantbefore applying — if not, add one rather than reaching fortoLocaleDateString()again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/admin/golf/tracer/TracerIncidentRow.tsx` around lines 28 - 33, Replace the direct new Date(incident.lastSeen).toLocaleDateString() call in TracerIncidentRow with the shared LocalTime component, using its date-only variant if supported. Verify LocalTime’s available variants first; if date-only is unavailable, add that variant to LocalTime and then use it here, preserving the existing styling and locale-safe rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/admin/actions/view-as.ts`:
- Around line 77-82: Do not pass the unverified cookie-derived targetUserId as
the optional userId when logging the “Admin exited view-as” event. In the
view-as exit action around logSecurityEvent, validate the token’s HMAC and
expiry first; pass the verified user ID only when validation succeeds, and
otherwise retain the raw value solely in event metadata.
In `@src/app/admin/golf/tracer/page.tsx`:
- Around line 66-67: Replace the non-canonical `border-accent-600/25` class on
the “Daily activity (30d)” and “Stuck rounds” headers in the tracer page with
the appropriate canonical design-system border token from the `primary-*`,
`destructive`, `warm-*`, or `cream-*` families, matching the correction used in
`TracerPlayerList.tsx`.
In `@src/app/admin/golf/tracer/StuckRoundsPanel.tsx`:
- Around line 34-91: Replace the scalar pendingId state in StuckRoundsPanel with
a Set of pending round IDs, and update resolve to add the round ID before
starting the request and remove it in finally without affecting other entries.
Update each row’s isPending/disabled logic to check whether its round ID exists
in the pending set, preserving independent busy states for concurrent
resolutions.
In `@src/app/admin/golf/tracer/tracer-shared.ts`:
- Around line 16-20: Update ROUND_STATUS_TONE to use
Record<TracerRoundDetail['status'], FwStatusTone> instead of Record<string,
FwStatusTone>, importing or referencing TracerRoundDetail from
admin-tracer-data.ts so the map is exhaustively checked against the actual round
status union.
In `@src/app/baseball/`(dashboard)/BaseballFairwayShell.tsx:
- Around line 489-490: Replace the duplicated pathname equality/prefix checks in
ShellFooter and BaseballMoreSheetFooter with the existing matchesRoutePrefix
helper, passing pathname and the relevant settings href; remove the redundant
settingsActive route-matching logic while preserving the current active-state
behavior.
In
`@src/app/baseball/`(dashboard)/dashboard/college-interest/CollegeInterestClient.tsx:
- Around line 201-219: Prevent stale fetches in CollegeInterestClient’s
useEffect from updating interests or loading state after teamId or filter
changes. Use an AbortController or request-generation guard in fetchInterests,
cancel or invalidate the prior request during effect cleanup, and only apply
results when the request is still current; ensure cleanup handles unmounts and
aborted requests without showing errors.
In `@src/app/baseball/`(dashboard)/dashboard/discover/DiscoverClient.tsx:
- Around line 322-327: The watchlist fetch catch block currently reports only to
the console. In DiscoverClient’s watchlist error handler, replace or supplement
console.error with the existing client logError helper or Sentry, passing the
caught error and relevant context, while preserving the existing addToast user
notification.
- Around line 319-321: The discover fetch effect must scope watchlist state to
the active coach. Update the effect managing watchlistIds to clear it when
coach?.id changes, capture the coach ID for each request, and ignore successful
or failed responses whose captured ID no longer matches the current coach;
preserve existing state only for transient failures belonging to the same coach.
In `@src/app/baseball/`(dashboard)/dashboard/stats/games/create/page.tsx:
- Line 4: Repeated coach/team resolution logic should be centralized in
server-route-guards.ts. Add a requireCollegeOrJucoTeamOrRedirect function that
performs authentication, coach type and organization checks,
resolveCoachTeamIdWithCookie, team lookup, and redirects; then replace the
duplicated sequences in the affected dashboard pages with this helper and use
its returned team.
In `@src/app/baseball/`(dashboard)/dashboard/template.tsx:
- Around line 42-45: Extract the duplicated LazyMotion, MotionConfig, and
RouteReveal logic into a shared BaseballRouteReveal component accepting children
and an isLateralDestination classifier, then update both dashboard templates to
use it and remove their local boilerplate and motion imports. Preserve the
existing animation behavior and styling comments while passing each template’s
route classifier to the shared wrapper.
In `@src/app/baseball/`(player-dashboard)/player/passport/page.tsx:
- Around line 71-91: Extract the duplicated recruiting eligibility query and
college-player exclusion logic into a shared resolveRecruitingActivation helper
in the baseball recruiting activation library, accepting the typed Supabase
client and player ID and returning the activation result or null on ineligible
players/errors. Update both the passport page and today page to create their
client and call this helper, removing their inline query and try/catch logic.
In `@src/app/baseball/`(public)/player/[id]/PlayerProfileClient.tsx:
- Around line 462-485: Gate the self-viewing recruiting disclosure and “Activate
Recruiting” CTA in the isSelfViewing block by also requiring player.player_type
!== 'college', so college players do not see an unusable activation action.
In `@src/app/baseball/actions/passport-settings.ts`:
- Around line 231-244: Handle the query error returned by the baseball_players
lookup in the wantsPublicExposure branch before evaluating subject. If the read
fails, return a failed result instead of treating subject as null and continuing
to self-activation; preserve the existing college-player and
recruiting_activated checks for successful reads. Update the
fromUntyped/supabase query handling near subjectRow accordingly.
In `@src/components/baseball/NotificationBell.tsx`:
- Around line 281-289: Update handleSelect and handleMarkRead so the read-state
UI is only finalized when the mark-read action returns success; handle the
returned result or failure by reverting the optimistic state and preserving the
unread badge when unsuccessful, while retaining navigation behavior in
handleSelect.
In `@src/components/baseball/passport/PassportVisibilityControls.tsx`:
- Around line 437-445: Add an explicit aria-label describing the external-link
action to the icon-only anchor in PassportVisibilityControls, and add
appropriate focus-visible ring/outline utility classes matching the component’s
existing focus styling conventions. Keep the existing title and link behavior
intact.
In `@src/components/baseball/performance/lift-canvas/LiftCanvas.tsx`:
- Line 201: Move the `space-y-4` class from the `PaperCard` usage in the detail
pane into `PaperCard`’s inner wrapper `<div>`, preserving any existing classes
so spacing applies between its child sections.
In `@src/components/coach/discover/PlayerCardGrid.tsx`:
- Line 8: Move the shared fadeUp and staggerContainer variants out of the
GolfHelm-specific v3 motion module into a neutral shared module such as
lib/motion/variants.ts, then update PlayerCardGrid’s import to use that module
and remove the cross-sport dependency.
In `@src/components/fairway/app-shell/LargeTitleContext.tsx`:
- Around line 285-322: Coalesce MutationObserver-triggered rescans in
FairwayContentAnchor instead of calling sync immediately for every mutation
batch. Add a requestAnimationFrame guard that schedules at most one sync per
frame, cancel any pending frame during effect cleanup, and retain the initial
synchronous sync and observer behavior.
In `@src/components/fairway/charts/StatStrip.tsx`:
- Around line 103-105: Add visible keyboard focus styling to the tabbable scroll
rail configured by RAIL_BASE, using a focus-visible outline or equivalent
indicator that remains clear across themes and forced-colors mode; ensure the
class is applied to the rail element referenced around the tabIndex logic.
- Around line 120-124: Update the three-item grid span logic in GRID_LAST_SPAN_3
and its equivalent usages for the responsive configurations: release the
trailing cell span at the actual breakpoint where the grid becomes three
columns, using md for mdColumns={3} and xl for xlColumns, instead of always
using lg. Keep the two-column span active below that breakpoint.
In `@src/components/fairway/pages/calendar/FairwayCalendar.tsx`:
- Around line 636-652: The deep-link effect in FairwayCalendar currently tracks
only whether any event was opened, preventing later initialEventId values from
opening their events. Replace autoOpenedRef with a useRef<string | null>(null),
compare the stored ID with initialEventId, and update it to the matched event’s
ID before calling openDrawerForEvent so each distinct deep link opens once.
In `@src/components/fairway/pages/travel/FairwayExpenseSummary.tsx`:
- Around line 117-139: Hoist CustomTooltip out of the parent component to module
scope so it is not recreated on every render. Pass summary.total as an explicit
prop, update its prop type accordingly, and preserve the existing tooltip
rendering and percentage calculation.
- Around line 90-93: Update handleSaveBudget to show fairwayToast.error with a
clear invalid-budget message before returning when the parsed amount is NaN or
negative, matching the error handling used by confirmDelete and the failure
branch below.
In `@src/components/golf/coachhelm/insights/InsightCallout.tsx`:
- Around line 29-31: Replace the Fairway-specific `rounded-fw-md` class in the
`InsightCallout` component’s root div with an approved canonical radius token
such as `rounded-lg` or `rounded-xl`, preserving the remaining classes.
In `@src/components/golf/coachhelm/insights/PlayerFocusAreas.tsx`:
- Around line 72-81: Preserve the selected focus-area context instead of routing
every card generically: update the FocusAreaCard onClick in PlayerFocusAreas to
include area.id in the My Development URL, then update the My Development page
to read that identifier and select or scroll to the matching row; if deep
linking is not supported, remove the click handler.
In `@src/lib/admin/data/baseball.ts`:
- Around line 82-86: Align the date windows and chart range with their labels:
in the completedGames30d filtering logic, use a strict `>` comparison against
`ago30dIso` so the window covers 30 calendar days, and in the 84-day chart query
or bucket generation use the current week minus 11 weeks rather than `ago84dIso`
to avoid 13 Monday buckets. Update the related logic around `completedGames30d`
and the chart data construction consistently.
In `@src/lib/admin/data/users.ts`:
- Around line 318-331: Update the team-user query logic in the users listing
flow to skip the Supabase query when usersInTeam is empty instead of calling
.in('id', ['']). After awaiting teamUsersQuery, explicitly check
teamUsersRes.error and throw it rather than defaulting failed results to an
empty roster; preserve the existing data fallback only for successful responses.
In `@src/lib/baseball/program-type-variants.ts`:
- Around line 87-102: Replace the unbounded string type on playerBottomNavRows
in the relevant program-type definition with the exported literal union or
registry-id type derived from PLAYER_HUB_ROW_IDS. Update imports and any
dependent assignments so each configured row is compile-time validated against
the available player hub IDs, including player-stats-hub and
player-recruiting-hub.
In `@src/lib/baseball/server-route-guards.ts`:
- Around line 100-108: The recruiting guard currently uses a permissive
coach-type-or-program-type check and allows Showcase access. Update
requireRecruitingCoachRoute and the underlying requireBaseballCoachRoute
configuration so recruiting authorization is restricted to eligible college
coaches and correctly configured JUCO coaches, excluding Showcase and High
School coaches regardless of program type; preserve the existing redirect,
session, and recruiting program configuration.
In `@src/lib/motion/route-motion.ts`:
- Around line 66-108: Add a test-only reset mechanism for the module-level
didFirstPaintThisSession state used by useRouteRevealMotion, allowing each unit
test to restore the initial first-paint behavior without resetting modules.
Export a clearly test-scoped helper such as __resetRouteRevealSessionForTests
and guard its availability so production behavior and bundles are unaffected.
In `@supabase/migrations/20260710010000_get_active_sessions_per_user_filter.sql`:
- Around line 17-20: Update the SECURITY DEFINER function
public.get_active_sessions to use an explicit empty search path (SET search_path
= '') instead of including public and pg_temp, preserving its schema-qualified
references. Add an inline comment on the function declaration justifying the
SECURITY DEFINER elevation, as required by the path instructions.
---
Outside diff comments:
In `@src/app/admin/users/page.tsx`:
- Around line 168-205: Apply activeTeamId as a team filter before deriving
players, watchlist, and KPI values: restrict teams to the matching team when
activeTeamId is set, otherwise retain all teams. Update the “Teams mapped”
statistic to use filteredTeams.length, and ensure all team panels and derived
counts use the filtered collection rather than the platform-wide teams array.
In `@src/components/baseball/performance/lift-canvas/SessionBlock.tsx`:
- Line 139: Replace the inline conflict marker in SessionBlock with the shared
DatelineRule component, passing conflictRule as tone and "mb-0.5" as className
while preserving conditional rendering and accessibility behavior.
In `@src/components/fairway/app-shell/AppShell.tsx`:
- Around line 292-303: Reset the controlled mobile sheet state when the viewport
switches to desktop. In AppShell, update the logic around the
isDesktop-dependent rendering and the mobileOpen state/setter so that
transitioning to desktop explicitly calls the setter with false before
unmounting the sheet, preventing it from reopening when returning to mobile.
In `@src/components/fairway/app-shell/FairwayBottomNav.tsx`:
- Around line 168-225: Add component tests for the interactive More column
rendered by FairwayBottomNav. Cover aria-haspopup="dialog", aria-expanded
updates, invoking onMoreOpen, conditional badge rendering including the 9+ cap,
and the absence of the column when onMoreOpen is unavailable; keep existing
more-nav.test.ts helper coverage unchanged.
In `@src/components/golf/putt-miss-tag-selector.tsx`:
- Around line 52-65: Add aria-pressed={isSelected} to each toggle button
rendered in the putt miss tag selector, alongside the existing disabled and
click behavior, so assistive technology can identify active tags.
---
Duplicate comments:
In `@src/app/admin/golf/tracer/TracerIncidentRow.tsx`:
- Around line 28-33: Replace the direct new
Date(incident.lastSeen).toLocaleDateString() call in TracerIncidentRow with the
shared LocalTime component, using its date-only variant if supported. Verify
LocalTime’s available variants first; if date-only is unavailable, add that
variant to LocalTime and then use it here, preserving the existing styling and
locale-safe rendering.
In `@src/app/admin/golf/tracer/TracerPlayerList.tsx`:
- Around line 61-68: Replace the non-canonical bg-accent-50/60 and bg-accent-500
classes in the TracerPlayerList row styling with appropriate canonical
design-system color tokens from the primary, destructive, warm, or cream
families, preserving the existing visual hierarchy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e8816102-6a3c-44eb-a265-d0f78cfb853e
⛔ Files ignored due to path filters (1)
src/lib/types/database.tsis excluded by!src/lib/types/database.ts
📒 Files selected for processing (195)
docs/MOBILE_DOCTRINE.mdmemory/projects/golfhelm.mdsrc/app/admin/_components/AdminShell.tsxsrc/app/admin/_components/AdminStatusBanner.tsxsrc/app/admin/_components/FeatureHealthRollup.tsxsrc/app/admin/_components/RelativeTime.tsxsrc/app/admin/_components/__tests__/admin-nav.test.tssrc/app/admin/_components/__tests__/admin-status-banner.test.tsxsrc/app/admin/_components/__tests__/relative-time.test.tsxsrc/app/admin/_components/admin-nav.tssrc/app/admin/actions/golf-tracer.tssrc/app/admin/actions/sessions.tssrc/app/admin/actions/view-as.tssrc/app/admin/auth/page.tsxsrc/app/admin/baseball/__tests__/page.test.tsxsrc/app/admin/baseball/page.tsxsrc/app/admin/deploys/page.tsxsrc/app/admin/errors/page.tsxsrc/app/admin/golf/page.tsxsrc/app/admin/golf/tracer/StuckRoundsPanel.tsxsrc/app/admin/golf/tracer/TracerIncidentRow.tsxsrc/app/admin/golf/tracer/TracerPlayerList.tsxsrc/app/admin/golf/tracer/TracerRoundDiagnostic.tsxsrc/app/admin/golf/tracer/page.tsxsrc/app/admin/golf/tracer/tracer-shared.tssrc/app/admin/jobs/page.tsxsrc/app/admin/layout.tsxsrc/app/admin/page.tsxsrc/app/admin/template.tsxsrc/app/admin/users/[id]/EngagementPanel.tsxsrc/app/admin/users/[id]/ViewAsButton.tsxsrc/app/admin/users/[id]/page.tsxsrc/app/admin/users/page.tsxsrc/app/admin/work/page.tsxsrc/app/baseball/(dashboard)/BaseballFairwayShell.tsxsrc/app/baseball/(dashboard)/_components/hub-sub-nav.tsxsrc/app/baseball/(dashboard)/dashboard/analytics/loading.tsxsrc/app/baseball/(dashboard)/dashboard/college-interest/CollegeInterestClient.tsxsrc/app/baseball/(dashboard)/dashboard/college-interest/page.tsxsrc/app/baseball/(dashboard)/dashboard/dev-plan/loading.tsxsrc/app/baseball/(dashboard)/dashboard/discover/DiscoverClient.tsxsrc/app/baseball/(dashboard)/dashboard/loading.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/page.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/passport/page.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/stats/page.tsxsrc/app/baseball/(dashboard)/dashboard/postgame/page.tsxsrc/app/baseball/(dashboard)/dashboard/practice/loading.tsxsrc/app/baseball/(dashboard)/dashboard/profile/page.tsxsrc/app/baseball/(dashboard)/dashboard/settings/page.tsxsrc/app/baseball/(dashboard)/dashboard/settings/program/loading.tsxsrc/app/baseball/(dashboard)/dashboard/signals/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/[gameId]/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/create/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/upload/page.tsxsrc/app/baseball/(dashboard)/dashboard/template.tsxsrc/app/baseball/(player-dashboard)/player/passport/page.tsxsrc/app/baseball/(player-dashboard)/player/template.tsxsrc/app/baseball/(player-dashboard)/player/today/page.tsxsrc/app/baseball/(public)/player/[id]/PlayerProfileClient.tsxsrc/app/baseball/(public)/player/[id]/page.tsxsrc/app/baseball/actions/passport-settings.tssrc/app/golf/(dashboard)/FairwayDashboardShell.tsxsrc/app/golf/(dashboard)/dashboard/calendar/page.tsxsrc/app/golf/(dashboard)/dashboard/messages/loading.tsxsrc/app/golf/(dashboard)/dashboard/rounds/[id]/review/page.tsxsrc/app/golf/(dashboard)/dashboard/rounds/continue/[id]/continue-round-client.tsxsrc/app/golf/(dashboard)/dashboard/tasks/page.tsxsrc/app/golf/(dashboard)/dashboard/template.tsxsrc/app/golf/(dashboard)/dashboard/travel/loading.tsxsrc/app/golf/(dashboard)/dashboard/travel/page.tsxsrc/app/golf/admin/components/BusinessIntelligenceTab.tsxsrc/app/golf/admin/components/GrowthCard.tsxsrc/app/golf/admin/crm/components/resend/KPIGrid.tsxsrc/app/lifting/(dashboard)/dashboard/check-ins/check-ins-client.tsxsrc/components/baseball/NotificationBell.tsxsrc/components/baseball/announcements/AnnouncementsPlayerView.tsxsrc/components/baseball/box-score/BoxScoreView.tsxsrc/components/baseball/import-center/ImportWizardClient.tsxsrc/components/baseball/living-annual/SectionMasthead.tsxsrc/components/baseball/living-annual/molecules/KPIContentsStrip.test.tsxsrc/components/baseball/living-annual/molecules/KPIContentsStrip.tsxsrc/components/baseball/passport/PassportVisibilityControls.tsxsrc/components/baseball/peek-panel/PlayerPeekContent.tsxsrc/components/baseball/peek-panel/PlayerQuickActions.tsxsrc/components/baseball/peek-panel/QuickStatsDisplay.tsxsrc/components/baseball/performance/ExerciseWizard.tsxsrc/components/baseball/performance/LiftBuilderClient.tsxsrc/components/baseball/performance/PlayerLiftToday.tsxsrc/components/baseball/performance/PlayerReadinessClient.tsxsrc/components/baseball/performance/lift-canvas/ExerciseBin.tsxsrc/components/baseball/performance/lift-canvas/LiftCanvas.tsxsrc/components/baseball/performance/lift-canvas/SessionBlock.tsxsrc/components/baseball/player-today/PlayerTodayClient.tsxsrc/components/baseball/recruiting-philosophy/PositionPriorityRanker.tsxsrc/components/baseball/stat-visuals/chart-primitives.tsxsrc/components/baseball/tasks/TaskCard.tsxsrc/components/coach/discover/PlayerCardGrid.tsxsrc/components/fairway/app-shell/AppShell.tsxsrc/components/fairway/app-shell/FairwayBottomNav.tsxsrc/components/fairway/app-shell/FairwayLargeTitle.test.tsxsrc/components/fairway/app-shell/FairwayLargeTitle.tsxsrc/components/fairway/app-shell/FairwayTopBar.test.tsxsrc/components/fairway/app-shell/FairwayTopBar.tsxsrc/components/fairway/app-shell/LargeTitleContext.test.tsxsrc/components/fairway/app-shell/LargeTitleContext.tsxsrc/components/fairway/app-shell/MoreNavSheet.tsxsrc/components/fairway/app-shell/MoreSheetFooter.tsxsrc/components/fairway/app-shell/index.tssrc/components/fairway/app-shell/more-nav.test.tssrc/components/fairway/app-shell/more-nav.tssrc/components/fairway/app-shell/types.tssrc/components/fairway/charts/StatStrip.test.tsxsrc/components/fairway/charts/StatStrip.tsxsrc/components/fairway/charts/index.tssrc/components/fairway/controls/badge.tsxsrc/components/fairway/controls/status-pill.tsxsrc/components/fairway/index.tssrc/components/fairway/pages/calendar/FairwayCalendar.tsxsrc/components/fairway/pages/calendar/FairwayEventDetailDrawer.tsxsrc/components/fairway/pages/calendar/FairwayEventEditor.tsxsrc/components/fairway/pages/calendar/FairwayMonthGrid.tsxsrc/components/fairway/pages/calendar/__tests__/FairwayEventDetailDrawer.test.tsxsrc/components/fairway/pages/coachhelm/FairwayCoachHelmSignals.tsxsrc/components/fairway/pages/coachhelm/FairwayEffectiveness.tsxsrc/components/fairway/pages/coachhelm/FairwayPlayerCoachHelm.tsxsrc/components/fairway/pages/coachhelm/FairwayPlayerInsight.tsxsrc/components/fairway/pages/coachhelm/FocusAreaCard.tsxsrc/components/fairway/pages/coachhelm/InsightTrustChips.tsxsrc/components/fairway/pages/dashboard/FairwayPlayerDashboard.tsxsrc/components/fairway/pages/dashboard/PlayerActionCenter.tsxsrc/components/fairway/pages/dashboard/player-dashboard-parts.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifierLeaderboard.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifyingWorkspace.tsxsrc/components/fairway/pages/rounds/FairwayUnfinishedBanner.tsxsrc/components/fairway/pages/tasks/FairwayTasks.tsxsrc/components/fairway/pages/team-hub/FairwayTeamHub.tsxsrc/components/fairway/pages/travel/FairwayExpenseList.tsxsrc/components/fairway/pages/travel/FairwayExpenseSummary.tsxsrc/components/fairway/pages/travel/FairwayTravel.tsxsrc/components/fairway/pages/travel/FairwayTripDetail.tsxsrc/components/fairway/pages/travel/expense-category.tssrc/components/fairway/view-header/view-header.tsxsrc/components/golf/approach-miss-selector.tsxsrc/components/golf/calendar/AttendancePanel.tsxsrc/components/golf/coachhelm/insights/FocusAreaCard.tsxsrc/components/golf/coachhelm/insights/InsightCallout.tsxsrc/components/golf/coachhelm/insights/PlayerFocusAreas.tsxsrc/components/golf/coachhelm/player/FocusAreasGrid.tsxsrc/components/golf/layout/LargeTitleHeader.tsxsrc/components/golf/putt-miss-tag-selector.tsxsrc/components/lifting/shell/LabNav.tsxsrc/components/lifting/shell/SportTabBar.tsxsrc/components/ui/card.tsxsrc/hooks/golf/use-task-realtime.tssrc/hooks/use-unread-count.tssrc/lib/admin-logger.tssrc/lib/admin/__tests__/sentry-api.test.tssrc/lib/admin/data/__tests__/baseball.test.tssrc/lib/admin/data/__tests__/users.test.tssrc/lib/admin/data/auth.tssrc/lib/admin/data/baseball.tssrc/lib/admin/data/golf.tssrc/lib/admin/data/incident-feed.tssrc/lib/admin/data/jobs.tssrc/lib/admin/data/overview.tssrc/lib/admin/data/user-engagement.tssrc/lib/admin/data/users.tssrc/lib/admin/fetch-result.tssrc/lib/admin/sentry-api.tssrc/lib/baseball/__tests__/bottom-nav.test.tssrc/lib/baseball/__tests__/nav-manifest.test.tssrc/lib/baseball/bottom-nav.tssrc/lib/baseball/nav-registry.tssrc/lib/baseball/program-type-variants.tssrc/lib/baseball/server-route-guards.tssrc/lib/bridge/nav-registry.test.tssrc/lib/bridge/nav-registry.tssrc/lib/calendar/premium-utils.tssrc/lib/golf/nav-registry.test.tssrc/lib/golf/nav-registry.tssrc/lib/motion/features-max.tssrc/lib/motion/features-min.tssrc/lib/motion/load-features.tssrc/lib/motion/route-motion.tssrc/styles/design-tokens.csssrc/test/golf/calendar-page.test.tsxsupabase/migrations/20260710004000_rescope_service_policies_to_service_role.sqlsupabase/migrations/20260710004100_fk_covering_indexes_batch1_admin_baseball_a_m.sqlsupabase/migrations/20260710004200_fk_covering_indexes_batch2_baseball_p_z.sqlsupabase/migrations/20260710004300_fk_covering_indexes_batch3_crm_email_golf.sqlsupabase/migrations/20260710004400_fk_covering_indexes_batch4_helm_lifting.sqlsupabase/migrations/20260710010000_get_active_sessions_per_user_filter.sqltailwind.config.ts
💤 Files with no reviewable changes (1)
- src/lib/admin/data/user-engagement.ts
Comment on lines
+77
to
+82
| logSecurityEvent( | ||
| 'Admin exited view-as', | ||
| 'info', | ||
| { targetUserId, adminUserId: admin.userId }, | ||
| targetUserId !== 'unknown' ? targetUserId : undefined, | ||
| ).catch(() => {}); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not attribute exit events from an unverified cookie value.
src/app/admin/actions/view-as.ts:77-82 passes targetUserId, extracted from the raw cookie at Line 74 without HMAC/expiry validation, into admin_events.user_id. A tampered cookie can therefore attach a false security event to any user's timeline. Verify the token before supplying the optional userId; otherwise log the value only as metadata.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/actions/view-as.ts` around lines 77 - 82, Do not pass the
unverified cookie-derived targetUserId as the optional userId when logging the
“Admin exited view-as” event. In the view-as exit action around
logSecurityEvent, validate the token’s HMAC and expiry first; pass the verified
user ID only when validation succeeds, and otherwise retain the raw value solely
in event metadata.
Comment on lines
+66
to
+67
| <Surface padding="sm"> | ||
| <h2 className="border-b border-accent-600/25 pb-2 text-xs font-semibold uppercase tracking-widest text-warm-500">Daily activity (30d)</h2> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
New section headers copy the non-canonical border-accent-600/25 token.
Same design-system violation as flagged in TracerPlayerList.tsx — accent-* isn't in the canonical family (primary-*, destructive, warm-*, cream-*). These two new headers ("Daily activity (30d)", "Stuck rounds") replicate the pattern from the pre-existing headers below rather than fixing it.
- <h2 className="border-b border-accent-600/25 pb-2 text-xs font-semibold uppercase tracking-widest text-warm-500">Daily activity (30d)</h2>
+ <h2 className="border-b border-primary-600/25 pb-2 text-xs font-semibold uppercase tracking-widest text-warm-500">Daily activity (30d)</h2>- <h2 className="border-b border-accent-600/25 pb-2 text-xs font-semibold uppercase tracking-widest text-warm-500">
+ <h2 className="border-b border-primary-600/25 pb-2 text-xs font-semibold uppercase tracking-widest text-warm-500">
Stuck rounds — in-progress, no activity in 1h+
</h2>As per coding guidelines, "Use only canonical design-system color families and surfaces: primary-*, destructive, warm-*, and cream-*".
Also applies to: 90-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/golf/tracer/page.tsx` around lines 66 - 67, Replace the
non-canonical `border-accent-600/25` class on the “Daily activity (30d)” and
“Stuck rounds” headers in the tracer page with the appropriate canonical
design-system border token from the `primary-*`, `destructive`, `warm-*`, or
`cream-*` families, matching the correction used in `TracerPlayerList.tsx`.
Source: Coding guidelines
Comment on lines
+34
to
+91
| export function StuckRoundsPanel({ rounds }: { rounds: readonly StuckRound[] }) { | ||
| const router = useRouter(); | ||
| const [removedIds, setRemovedIds] = useState<ReadonlySet<string>>(new Set()); | ||
| const [confirmingId, setConfirmingId] = useState<string | null>(null); | ||
| const [pendingId, setPendingId] = useState<string | null>(null); | ||
| const [feedback, setFeedback] = useState<ReadonlyMap<string, { ok: boolean; message: string }>>(new Map()); | ||
| const [, startTransition] = useTransition(); | ||
| const [checkedAt, setCheckedAt] = useState(PLACEHOLDER_CHECKED_AT); | ||
|
|
||
| useEffect(() => { | ||
| setCheckedAt(new Date().toISOString()); | ||
| }, []); | ||
|
|
||
| const visible = rounds.filter((r) => !removedIds.has(r.round_id)); | ||
|
|
||
| if (visible.length === 0) { | ||
| return ( | ||
| <PanelAllClear | ||
| label={ | ||
| rounds.length > 0 | ||
| ? 'All stuck rounds resolved' | ||
| : 'No stuck rounds — every in-progress round has moved in the last hour' | ||
| } | ||
| checkedAt={checkedAt} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| function resolve(roundId: string) { | ||
| setConfirmingId(null); | ||
| setPendingId(roundId); | ||
| setFeedback((prev) => { | ||
| if (!prev.has(roundId)) return prev; | ||
| const next = new Map(prev); | ||
| next.delete(roundId); | ||
| return next; | ||
| }); | ||
| startTransition(() => { | ||
| void bridgeFixRoundData(roundId, 'resolve_stuck_round') | ||
| .then((result) => { | ||
| if (result.success) { | ||
| setRemovedIds((prev) => new Set([...prev, roundId])); | ||
| router.refresh(); | ||
| } else { | ||
| setFeedback((prev) => new Map(prev).set(roundId, { ok: false, message: result.message })); | ||
| } | ||
| }) | ||
| .catch((err: unknown) => { | ||
| setFeedback((prev) => | ||
| new Map(prev).set(roundId, { | ||
| ok: false, | ||
| message: err instanceof Error ? err.message : 'Could not resolve this round — try again.', | ||
| }), | ||
| ); | ||
| }) | ||
| .finally(() => setPendingId((current) => (current === roundId ? null : current))); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Single pendingId scalar can't track concurrent per-row mutations — busy state gets silently stolen between rows.
pendingId (line 38) is one string for the whole list. Trigger round A's resolve, then before it settles trigger round B's resolve — setPendingId(B) (line 64) overwrites A's tracking. Round A's "Resolve" button then reads isPending=false (line 97 → disabled={isPending} at line 135) and becomes clickable again while A's original bridgeFixRoundData call is still in flight, letting a second resolve_stuck_round fire against the same round. The .finally cleanup (line 89) only matches on roundId string equality, so it can't tell the two in-flight A-calls apart either.
Since rounds are independent, this should track a set of pending ids, not a single scalar.
🔒 Proposed fix — track pending ids as a Set
- const [pendingId, setPendingId] = useState<string | null>(null);
+ const [pendingIds, setPendingIds] = useState<ReadonlySet<string>>(new Set());
const [checkedAt, setCheckedAt] = useState(PLACEHOLDER_CHECKED_AT);
@@
function resolve(roundId: string) {
setConfirmingId(null);
- setPendingId(roundId);
+ setPendingIds((prev) => new Set([...prev, roundId]));
setFeedback((prev) => {
if (!prev.has(roundId)) return prev;
const next = new Map(prev);
next.delete(roundId);
return next;
});
startTransition(() => {
void bridgeFixRoundData(roundId, 'resolve_stuck_round')
.then((result) => {
if (result.success) {
setRemovedIds((prev) => new Set([...prev, roundId]));
router.refresh();
} else {
setFeedback((prev) => new Map(prev).set(roundId, { ok: false, message: result.message }));
}
})
.catch((err: unknown) => {
setFeedback((prev) =>
new Map(prev).set(roundId, {
ok: false,
message: err instanceof Error ? err.message : 'Could not resolve this round — try again.',
}),
);
})
- .finally(() => setPendingId((current) => (current === roundId ? null : current)));
+ .finally(() =>
+ setPendingIds((prev) => {
+ const next = new Set(prev);
+ next.delete(roundId);
+ return next;
+ }),
+ );
});
}
@@
const fb = feedback.get(r.round_id);
- const isPending = pendingId === r.round_id;
+ const isPending = pendingIds.has(r.round_id);Also applies to: 96-97, 128-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/golf/tracer/StuckRoundsPanel.tsx` around lines 34 - 91, Replace
the scalar pendingId state in StuckRoundsPanel with a Set of pending round IDs,
and update resolve to add the round ID before starting the request and remove it
in finally without affecting other entries. Update each row’s isPending/disabled
logic to check whether its round ID exists in the pending set, preserving
independent busy states for concurrent resolutions.
Comment on lines
+16
to
+20
| export const ROUND_STATUS_TONE: Record<string, FwStatusTone> = { | ||
| completed: 'success', | ||
| in_progress: 'warning', | ||
| draft: 'neutral', | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ROUND_STATUS_TONE is looser than its sibling map — consider keying it to the actual status union.
TRACER_SEVERITY_TONE above is keyed to TracerIncident['severity'], giving exhaustiveness checking. ROUND_STATUS_TONE uses Record<string, FwStatusTone>, so a new/renamed round status silently falls back to ?? 'neutral' at the call site with no compiler signal. TracerRoundDetail['status'] (from admin-tracer-data.ts) is the type to key this to instead, matching the pattern set 8 lines above.
-export const ROUND_STATUS_TONE: Record<string, FwStatusTone> = {
+export const ROUND_STATUS_TONE: Record<TracerRoundDetail['status'], FwStatusTone> = {
completed: 'success',
in_progress: 'warning',
draft: 'neutral',
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const ROUND_STATUS_TONE: Record<string, FwStatusTone> = { | |
| completed: 'success', | |
| in_progress: 'warning', | |
| draft: 'neutral', | |
| }; | |
| export const ROUND_STATUS_TONE: Record<TracerRoundDetail['status'], FwStatusTone> = { | |
| completed: 'success', | |
| in_progress: 'warning', | |
| draft: 'neutral', | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/golf/tracer/tracer-shared.ts` around lines 16 - 20, Update
ROUND_STATUS_TONE to use Record<TracerRoundDetail['status'], FwStatusTone>
instead of Record<string, FwStatusTone>, importing or referencing
TracerRoundDetail from admin-tracer-data.ts so the map is exhaustively checked
against the actual round status union.
Comment on lines
+489
to
+490
| const settingsHref = BASEBALL_SETTINGS_HREF; | ||
| const settingsActive = pathname === settingsHref || pathname.startsWith(`${settingsHref}/`); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicated settingsActive route-match logic — matchesRoutePrefix already exists in this file.
ShellFooter (L490) and the new BaseballMoreSheetFooter (L547) both hand-roll pathname === X || pathname.startsWith(\${X}/`), which is exactly what matchesRoutePrefix(L145-147, already used bytoNavItem/playerHubToNavItem`) does.
♻️ Reuse the existing helper
- const settingsActive = pathname === settingsHref || pathname.startsWith(`${settingsHref}/`);
+ const settingsActive = matchesRoutePrefix(pathname, settingsHref);- const settingsActive = pathname === BASEBALL_SETTINGS_HREF || pathname.startsWith(`${BASEBALL_SETTINGS_HREF}/`);
+ const settingsActive = matchesRoutePrefix(pathname, BASEBALL_SETTINGS_HREF);Also applies to: 544-547
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/BaseballFairwayShell.tsx around lines 489 -
490, Replace the duplicated pathname equality/prefix checks in ShellFooter and
BaseballMoreSheetFooter with the existing matchesRoutePrefix helper, passing
pathname and the relevant settings href; remove the redundant settingsActive
route-matching logic while preserving the current active-state behavior.
Comment on lines
+318
to
+331
| let teamUsersQuery = filters.team | ||
| ? admin | ||
| .from('users') | ||
| .select('id, email, role, created_at, last_seen') | ||
| .in('id', usersInTeam.size > 0 ? Array.from(usersInTeam) : ['']) | ||
| : null; | ||
| if (teamUsersQuery && filters.q) teamUsersQuery = teamUsersQuery.ilike('email', `%${filters.q}%`); | ||
| if (teamUsersQuery && filters.role && (USER_ROLES as readonly string[]).includes(filters.role)) { | ||
| teamUsersQuery = teamUsersQuery.eq('role', filters.role as UserRole); | ||
| } | ||
| const teamUsersRes = teamUsersQuery ? await teamUsersQuery : null; | ||
| const userRows = filters.team ? (teamUsersRes?.data ?? []) : (usersRes.data ?? []); | ||
|
|
||
| const users: DirectoryUser[] = userRows |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not convert failed team-user queries into an empty roster.
When usersInTeam is empty, .in('id', ['']) sends an invalid UUID. That error—and any other query failure—is ignored by teamUsersRes?.data ?? [].
Skip the query for an empty set and throw when teamUsersRes.error is present.
Proposed fix
-const teamUsersRes = teamUsersQuery ? await teamUsersQuery : null;
-const userRows = filters.team ? (teamUsersRes?.data ?? []) : (usersRes.data ?? []);
+const teamUsersRes =
+ teamUsersQuery && usersInTeam.size > 0 ? await teamUsersQuery : null;
+
+if (teamUsersRes?.error) {
+ throw new Error(`fetchUsersTab: team users query failed: ${teamUsersRes.error.message}`);
+}
+
+const userRows = filters.team
+ ? (teamUsersRes?.data ?? [])
+ : (usersRes.data ?? []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let teamUsersQuery = filters.team | |
| ? admin | |
| .from('users') | |
| .select('id, email, role, created_at, last_seen') | |
| .in('id', usersInTeam.size > 0 ? Array.from(usersInTeam) : ['']) | |
| : null; | |
| if (teamUsersQuery && filters.q) teamUsersQuery = teamUsersQuery.ilike('email', `%${filters.q}%`); | |
| if (teamUsersQuery && filters.role && (USER_ROLES as readonly string[]).includes(filters.role)) { | |
| teamUsersQuery = teamUsersQuery.eq('role', filters.role as UserRole); | |
| } | |
| const teamUsersRes = teamUsersQuery ? await teamUsersQuery : null; | |
| const userRows = filters.team ? (teamUsersRes?.data ?? []) : (usersRes.data ?? []); | |
| const users: DirectoryUser[] = userRows | |
| let teamUsersQuery = filters.team | |
| ? admin | |
| .from('users') | |
| .select('id, email, role, created_at, last_seen') | |
| .in('id', usersInTeam.size > 0 ? Array.from(usersInTeam) : ['']) | |
| : null; | |
| if (teamUsersQuery && filters.q) teamUsersQuery = teamUsersQuery.ilike('email', `%${filters.q}%`); | |
| if (teamUsersQuery && filters.role && (USER_ROLES as readonly string[]).includes(filters.role)) { | |
| teamUsersQuery = teamUsersQuery.eq('role', filters.role as UserRole); | |
| } | |
| const teamUsersRes = | |
| teamUsersQuery && usersInTeam.size > 0 ? await teamUsersQuery : null; | |
| if (teamUsersRes?.error) { | |
| throw new Error(`fetchUsersTab: team users query failed: ${teamUsersRes.error.message}`); | |
| } | |
| const userRows = filters.team | |
| ? (teamUsersRes?.data ?? []) | |
| : (usersRes.data ?? []); | |
| const users: DirectoryUser[] = |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/admin/data/users.ts` around lines 318 - 331, Update the team-user
query logic in the users listing flow to skip the Supabase query when
usersInTeam is empty instead of calling .in('id', ['']). After awaiting
teamUsersQuery, explicitly check teamUsersRes.error and throw it rather than
defaulting failed results to an empty roster; preserve the existing data
fallback only for successful responses.
Comment on lines
+87
to
+102
| /** | ||
| * M1 (baseball-nav-4) — the bottom-nav slot-3 DIFFERENTIATOR hub for this | ||
| * mode's COACH bar (src/lib/baseball/bottom-nav.ts). A single-entry array: | ||
| * universal slots 1/2/4 (Dashboard/Team/Messages) are added by that module, | ||
| * never forked here, so the per-mode diff stays a single token. Read as | ||
| * `coachBottomNavHubs[0]`. | ||
| */ | ||
| coachBottomNavHubs: readonly BaseballNavHub[]; | ||
| /** | ||
| * The bottom-nav slot-3 DIFFERENTIATOR row id for this mode's PLAYER bar. A | ||
| * single-entry array of a PLAYER_HUB_ROW_IDS value or registry id (never a | ||
| * BaseballNavHub — players are never grouped into a coach hub). Universal | ||
| * slots 1/2/4 (Today/Schedule/Messages) are added by bottom-nav.ts. | ||
| */ | ||
| playerBottomNavRows: readonly string[]; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
playerBottomNavRows: readonly string[] has no compile-time guard against dangling ids — consider a typed union.
Unlike coachBottomNavHubs: readonly BaseballNavHub[] (which TypeScript checks against a closed union), playerBottomNavRows is readonly string[]. A typo or a value that doesn't exist in whatever PLAYER_HUB_ROW_IDS/registry-id set the shell joins against (see the player-stats-hub / player-recruiting-hub verification request on bottom-nav.ts) would compile cleanly and only fail at runtime as a missing nav tab. If PLAYER_HUB_ROW_IDS is exported as a literal array/union somewhere, typing this field against it would catch the same class of bug this PR is trying to prevent for the coach side.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/baseball/program-type-variants.ts` around lines 87 - 102, Replace the
unbounded string type on playerBottomNavRows in the relevant program-type
definition with the exported literal union or registry-id type derived from
PLAYER_HUB_ROW_IDS. Update imports and any dependent assignments so each
configured row is compile-time validated against the available player hub IDs,
including player-stats-hub and player-recruiting-hub.
Comment on lines
+100
to
+108
| export async function requireRecruitingCoachRoute( | ||
| redirectTo = '/baseball/dashboard/command-center', | ||
| session?: SessionProfile, | ||
| ) { | ||
| return requireBaseballCoachRoute({ | ||
| allowedCoachTypes: ['college', 'juco', 'showcase'], | ||
| allowedProgramTypes: [...RECRUITING_PROGRAM_TYPES], | ||
| redirectTo, | ||
| session, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not authorize recruiting through the permissive OR gate.
At server-route-guards.ts:104-107, showcase is explicitly allowed, and requireBaseballCoachRoute accepts either an allowed coach type or program type. This can admit Showcase coaches—or High School coaches attached to academy/club programs—to recruiting surfaces.
Gate recruiting to eligible coach types (college, plus correctly configured juco) rather than the current OR combination.
As per path instructions, coach types have asymmetric feature access, and Showcase/High School coaches do not receive recruiting access.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/baseball/server-route-guards.ts` around lines 100 - 108, The
recruiting guard currently uses a permissive coach-type-or-program-type check
and allows Showcase access. Update requireRecruitingCoachRoute and the
underlying requireBaseballCoachRoute configuration so recruiting authorization
is restricted to eligible college coaches and correctly configured JUCO coaches,
excluding Showcase and High School coaches regardless of program type; preserve
the existing redirect, session, and recruiting program configuration.
Source: Path instructions
Comment on lines
+66
to
+108
| // Module-level singleton — this is a SINGLE flag for the lifetime of the | ||
| // browser tab's JS module (a fresh page load re-executes the module, so this | ||
| // starts `false` again), and it is only ever flipped by a CLIENT `useEffect`. | ||
| // SSR-safe: the server render never runs an effect, so it always computes | ||
| // `false` here; the first CLIENT render (before hydration's effects flush) | ||
| // reads the exact same fresh-module `false` — server and first-client-paint | ||
| // output are therefore identical, with no hydration mismatch. It survives a | ||
| // template's per-navigation REMOUNT (the reason a local ref/state cannot do | ||
| // this job) because the module itself — not the component instance — is | ||
| // where the flag lives. | ||
| let didFirstPaintThisSession = false; | ||
|
|
||
| /** | ||
| * Doctrine Rule 9 — classify the CURRENT route and return the motion props | ||
| * for the content column's keyed `m.div`. `isLateralDestination` must be a | ||
| * stable, module-level pure function (one of `isGolfLateralDestination` / | ||
| * `isBaseballLateralDestination` / `isBridgeLateralDestination`). | ||
| * | ||
| * Decision table (first match wins): | ||
| * 1. reduced-motion → INSTANT (no motion anywhere, ever) | ||
| * 2. first paint of the session → INSTANT (never fight hydration/skeleton) | ||
| * 3. current path is a registered lateral destination → INSTANT | ||
| * 4. otherwise (a detail push) → opacity 0→1, 0.18s, --fw-ease-glide | ||
| */ | ||
| export function useRouteRevealMotion( | ||
| isLateralDestination: (pathname: string) => boolean, | ||
| ): RouteRevealMotion { | ||
| const pathname = usePathname() ?? ''; | ||
| const prefersReducedMotion = useReducedMotion(); | ||
|
|
||
| // Snapshot "is this the first paint of the session" ONCE per mounted | ||
| // instance (a lazy ref initializer, evaluated only on this instance's | ||
| // FIRST render). The next navigation remounts a brand-new instance, which | ||
| // re-reads the module flag fresh — that is how the classification stays | ||
| // correct across remounts without ever comparing "previous vs next" route. | ||
| const isFirstPaintRef = useRef<boolean | undefined>(undefined); | ||
| if (isFirstPaintRef.current === undefined) { | ||
| isFirstPaintRef.current = !didFirstPaintThisSession; | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| didFirstPaintThisSession = true; | ||
| }, []); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Module-level didFirstPaintThisSession singleton will make direct unit tests of this hook order-dependent.
Once any test in a given test file mounts a component using useRouteRevealMotion and flushes its effect (Line 106-108 sets didFirstPaintThisSession = true permanently for the module instance), every subsequent test in that same file/module graph loses the "first paint" branch (Line 111) for free — tests would need vi.resetModules() + a fresh dynamic import() per test to get a clean slate, which is easy to forget and would produce silently-wrong assertions (a test expecting the "push" opacity-fade branch could instead get INSTANT from stale first-paint state, or vice versa depending on run order). Consider exporting a test-only reset (e.g. __resetRouteRevealSessionForTests()) guarded so it can't ship to prod bundles, to make this testable without fighting the singleton.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/motion/route-motion.ts` around lines 66 - 108, Add a test-only reset
mechanism for the module-level didFirstPaintThisSession state used by
useRouteRevealMotion, allowing each unit test to restore the initial first-paint
behavior without resetting modules. Export a clearly test-scoped helper such as
__resetRouteRevealSessionForTests and guard its availability so production
behavior and bundles are unaffected.
Comment on lines
+17
to
+20
| CREATE OR REPLACE FUNCTION public.get_active_sessions(p_user_id uuid DEFAULT NULL) RETURNS jsonb | ||
| LANGUAGE plpgsql STABLE SECURITY DEFINER | ||
| SET search_path TO 'public', 'pg_temp' | ||
| AS $$ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
search_path isn't hardened to the empty string per path instructions.
SET search_path TO 'public', 'pg_temp' (line 19) doesn't match the rule: "Forbid SECURITY DEFINER functions without an explicit SET search_path = '' and an inline comment justifying the elevation." Not actively exploitable here — pg_catalog is implicitly searched ahead of the explicit list, and the one user-namespaced call (public.is_super_admin()) is already schema-qualified — but it's a defense-in-depth gap, and it also introduces a third search_path convention alongside the guideline's = '' and the v3 RLS-helper docs' TO 'public' (no pg_temp).
🔒 Proposed tightening
CREATE OR REPLACE FUNCTION public.get_active_sessions(p_user_id uuid DEFAULT NULL) RETURNS jsonb
LANGUAGE plpgsql STABLE SECURITY DEFINER
- SET search_path TO 'public', 'pg_temp'
+ SET search_path = ''
AS $$Since every table/function reference in the body (auth.sessions, auth.users, public.is_super_admin()) is already schema-qualified, and jsonb_agg/jsonb_build_object/COALESCE resolve via pg_catalog regardless of search_path, this should be a behavior-neutral tightening.
As per path instructions, "Forbid SECURITY DEFINER functions without an explicit SET search_path = '' and an inline comment justifying the elevation."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CREATE OR REPLACE FUNCTION public.get_active_sessions(p_user_id uuid DEFAULT NULL) RETURNS jsonb | |
| LANGUAGE plpgsql STABLE SECURITY DEFINER | |
| SET search_path TO 'public', 'pg_temp' | |
| AS $$ | |
| CREATE OR REPLACE FUNCTION public.get_active_sessions(p_user_id uuid DEFAULT NULL) RETURNS jsonb | |
| LANGUAGE plpgsql STABLE SECURITY DEFINER | |
| SET search_path = '' | |
| AS $$ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260710010000_get_active_sessions_per_user_filter.sql`
around lines 17 - 20, Update the SECURITY DEFINER function
public.get_active_sessions to use an explicit empty search path (SET search_path
= '') instead of including public and pg_temp, preserving its schema-qualified
references. Add an inline comment on the function declaration justifying the
SECURITY DEFINER elevation, as required by the path instructions.
Source: Path instructions
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.
Two commits, both fully gated (tsc 0 / eslint 0 / unit green / production build clean, unpiped exit codes).
Commit 1 — M0 mobile pass (perf + overflow + accent retirement)
Backdrop-blur off scrolling chrome at <md; one route fade; LazyMotion actually lazy (~59 call sites); React.memo effective on shell props; per-user realtime filters; SectionMasthead overflow root-cause (~65 pages at 390px); signup logo fix + HelmMark; accent-stripe cards → dateline-rule idiom.
Commit 2 — M1 shell & chrome + overnight fix waves
M1 (six packets, each adversarially verified):
Fix waves — 69 adversarially-verified findings across 15 packets, incl. all four P0s:
College Interest infinite skeleton for multi-team coaches; app-wide dead notification bell now deep-links; Recruiting Preferences login-bounce; coach Dev-Plan dead-end. Bridge integrity (uncapped canonical incident feed, honest Sentry truncation, unified "today" definitions, truthful status banner, chips are real filter links, cron
lastError+ integrity samples surfaced,/admin/workoff its blocking await, deploy links). Golf activation funnel/adoption/DAU-WAU-MAU now rendered. Sessions/impersonation audit visibility. Passport visibility now requires real recruiting consent (staff can no longer silently flip exposure). Designed error/empty states + micro-motion polish.DB (applied to prod, mirrored here): 19 service policies re-scoped
TO service_role; ~160 FK covering indexes (0 unindexed FKs remain inpublic);get_active_sessions(p_user_id)SQL-side filter.docs/MOBILE_DOCTRINE.mdcodified (11 rules + craft bar + perf floor).Owner-decision items intentionally NOT in this PR: super-admin dual-authority unification (security surface), unused-index drops, extension relocation, avatars-bucket listing policy, SECURITY DEFINER views/functions audit — detailed in the morning report.
🤖 Generated with Claude Code
https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX