feat(stats): premium glass driving spray chart in the Driving tab - #303
feat(stats): premium glass driving spray chart in the Driving tab#303njrini99-code wants to merge 1 commit into
Conversation
Adds FairwayDrivingSpray — a premium SVG "landing view" of off-the-tee
directions — to the Driving tab on the stats cockpit, replacing the flat canvas
ShotDispersion that previously only lived (for driving) in the Analysis tab.
The chart:
- Frosted glass plot panel (gradient + lit top edge), a dashed green intended-
target line, and an honest 1σ covariance ellipse (anisotropic-safe projected
path) showing the dispersion shape.
- Shot landings colored by outcome (fairway / trouble / penalty) + the player's
mean-bias marker.
- A left·center·right distribution bar + chips (dominant miss accented) and an
outcome legend, with a plain-English tendency takeaway ("Misses lean right")
and avg carry.
- Honest: plots ONLY real SprayChartShotGroup.points (no fabricated coords),
ellipse needs ≥3 shots, and an empty state when there's no tee-shot spray yet.
role="img" with a full text summary for screen readers.
Wiring:
- FairwayStatsCockpit Driving tab now renders <FairwayDrivingSpray
group={sprayData?.driving} /> beneath DrivingSection.
- Removed the old driving ShotDispersion from the Analysis "Shot patterns" board
(kept the approach miss map + putting heatmap) so there's no trashy duplicate.
- Deep-imports Surface/InsufficientData (not the big @/components/fairway barrel)
to avoid a circular-init in the cockpit chunk.
Verified: rendered in an isolated harness (real-shaped mock data) — 24 points +
mean marker, accurate aria summary, correct empty state. typecheck clean · lint
0 errors · 2510 unit tests pass. (Live cockpit screenshot was blocked by an
unrelated Turbopack dev server-action "module factory" wedge affecting all
dashboard routes — dev-only; the route builds fine in prod.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
WalkthroughAdds a new ChangesDriving Spray Chart and Cockpit Wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.43.0)src/components/fairway/pages/coachhelm/FairwayDrivingSpray.tsxError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml ✖ Caused by src/components/fairway/pages/coachhelm/FairwayStatsCockpit.tsxError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml ✖ Caused by 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a69c1769b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /** | ||
| * ============================================================================ | ||
| * FairwayDrivingSpray — the premium off-the-tee spray chart (driving directions) |
There was a problem hiding this comment.
The repository AGENTS.md requires mapped feature code to be routed through memory/registry.yml, but running npm run knowledge:map -- --files src/components/fairway/pages/coachhelm/FairwayDrivingSpray.tsx src/components/fairway/pages/coachhelm/FairwayStatsCockpit.tsx returns impactedFeatures: []. This new stats/CoachHelm component will therefore be reviewed without the required current-state documentation and checks; add the Fairway cockpit path to the appropriate stats feature mapping.
Useful? React with 👍 / 👎.
| // ── Honest empty state — no fabricated chart when there are no plotted shots. | ||
| if (!group || n === 0 || !model) { |
There was a problem hiding this comment.
When the spray query fails, getSprayChartData catches the error and returns a fulfilled response containing an empty driving group, so this branch displays “No tee-shot spray yet” and tells an existing player to log rounds. Previously the failed/empty driving enrichment was hidden with the shot-pattern section; preserve an error/availability signal or omit the chart on fetch failure rather than presenting the zero-point response as genuine absence of data.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const legend = [ | ||
| { label: 'Fairway', count: group.playableCount, color: ACCENT }, | ||
| { label: 'Trouble', count: group.troubleCount, color: WARNING }, | ||
| { label: 'Penalty', count: group.penaltyCount, color: DANGER }, | ||
| ].filter((l) => l.count > 0); |
There was a problem hiding this comment.
group.playableCount, group.troubleCount, and group.penaltyCount are server-supplied fields on SprayChartShotGroup. Looking at the type, they sit alongside totalShots and plottedShots as separate fields, which suggests they may count all shots tracked (even those without valid coordinates), not just the plottedShots that appear in points[]. If that's the case, a user could see e.g. "Penalty: 2" in the legend while only one red dot is visible on the chart — confusing at best, silently wrong at worst. The direction bar already uses only points (plotted shots) for its counts; the legend should be consistent with that. Worth confirming whether these counts are scoped to plottedShots on the server side, and if not, deriving them directly from points instead. Do playableCount, troubleCount, and penaltyCount in SprayChartShotGroup count only shots that have valid coordinates (i.e. appear in points[]), or do they count all shots in totalShots (including those that couldn't be plotted)?
| const total = left + center + right; | ||
| const pct = (c: number) => (total > 0 ? Math.round((c / total) * 100) : 0); |
There was a problem hiding this comment.
pct rounds each bucket independently, so three values like 13, 42, 46 sum to 101 (as demonstrated in the PR's own aria preview). At 101%, the overflow-hidden container clips the rightmost segment, potentially trimming its last:rounded-r-full corner. At 99%, a 1-pixel sliver of bg-surface-sunken is visible at the right edge. A common fix is to compute the last segment as 100 - pct(left) - pct(center) (or equivalently use a single Math.round for the remainder), so the three values always sum to exactly 100.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/components/fairway/pages/coachhelm/FairwayDrivingSpray.tsx`:
- Around line 58-60: The labels in SECTOR_TAKEAWAY constant and the outcome
bucket labels around lines 234-243 are overstating what the data represents by
conflating sector direction and outcome bucket values with specific terrain
features. The data contract only provides sector direction (center, left, right)
and outcomeBucket values (playable, trouble, penalty), but the current labels
suggest specific terrain types like "short grass" and "Fairway." Update the
SECTOR_TAKEAWAY labels to accurately describe the center, left, and right
sectors without assuming terrain type, and update the outcome bucket labels in
the affected code region to describe playable, trouble, and penalty outcomes
honestly without conflating them with specific fairway features.
- Around line 275-285: Replace the hardcoded hex colors in the SVG gradient and
filter definitions with Fairway design tokens to maintain theme consistency. In
the linearGradient element, replace the stopColor="`#ffffff`" values with an
appropriate design token variable. In the feDropShadow filter element, replace
the floodColor="`#1c1917`" hardcoded hex value with the corresponding Fairway
design token. Follow the existing pattern where ACCENT is already being used for
the radialGradient stops, ensuring all color references throughout the component
use design-system tokens instead of inline hex values.
- Around line 29-33: The import of SprayChartShotGroup, SprayChartPoint, and
SprayChartSector types in FairwayDrivingSpray.tsx is currently sourced from the
action module '`@/app/golf/actions/stats-data-types`', but component TSX files
should import types from the shared type boundary '`@/lib/types`'. First, verify
that these three types are exported from '`@/lib/types`'; if they are not yet
exported there, re-export them from that shared types module. Then update the
import statement in FairwayDrivingSpray.tsx to import these types from
'`@/lib/types`' instead of from the action module, ensuring the component adheres
to the coding guideline of consuming types from the centralized type
definitions.
In `@src/components/fairway/pages/coachhelm/FairwayStatsCockpit.tsx`:
- Line 574: The issue is that when getSprayChartData rejects, sprayData is set
to null, which causes FairwayDrivingSpray to display "No tee-shot spray yet"
incorrectly since this message should only appear for successful responses with
empty data, not for fetch failures. To fix this, distinguish between fetch
failures and actual no-data by tracking the error state separately from
sprayData, or by using a different null/undefined sentinel value for failures.
Modify the code that handles the getSprayChartData rejection to set a separate
error state instead of setting sprayData to null, then only pass null to
FairwayDrivingSpray when sprayData is populated but contains no driving data,
not when the fetch fails.
🪄 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
Run ID: 5d7dbb26-6403-4b15-8837-d9dcc87bf6fe
📒 Files selected for processing (2)
src/components/fairway/pages/coachhelm/FairwayDrivingSpray.tsxsrc/components/fairway/pages/coachhelm/FairwayStatsCockpit.tsx
| import type { | ||
| SprayChartShotGroup, | ||
| SprayChartPoint, | ||
| SprayChartSector, | ||
| } from '@/app/golf/actions/stats-data-types'; |
There was a problem hiding this comment.
Import spray chart types through @/lib/types.
Line 29 imports type definitions from an action module, but component TSX should consume type definitions from the shared type surface.
Proposed change
import { InsufficientData } from '`@/components/fairway/feedback/InsufficientData`';
import type {
SprayChartShotGroup,
SprayChartPoint,
SprayChartSector,
-} from '`@/app/golf/actions/stats-data-types`';
+} from '`@/lib/types`';If these spray-chart types are not exported there yet, re-export them from @/lib/types and keep this component on the shared type boundary.
As per coding guidelines, “Import types from '@/lib/types' only; never use '@/types/database' or '@/types/supabase'” and “Import types from @/lib/types for type definitions.”
🤖 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/coachhelm/FairwayDrivingSpray.tsx` around lines
29 - 33, The import of SprayChartShotGroup, SprayChartPoint, and
SprayChartSector types in FairwayDrivingSpray.tsx is currently sourced from the
action module '`@/app/golf/actions/stats-data-types`', but component TSX files
should import types from the shared type boundary '`@/lib/types`'. First, verify
that these three types are exported from '`@/lib/types`'; if they are not yet
exported there, re-export them from that shared types module. Then update the
import statement in FairwayDrivingSpray.tsx to import these types from
'`@/lib/types`' instead of from the action module, ensuring the component adheres
to the coding guideline of consuming types from the centralized type
definitions.
Source: Coding guidelines
| const SECTOR_TAKEAWAY: Record<string, string> = { | ||
| center: 'Tee shots tend to find the short grass', | ||
| left: 'Misses lean left', |
There was a problem hiding this comment.
Keep sector and outcome labels data-honest.
Line 59 treats center as “short grass,” line 236 labels the center sector as “Fairway line,” and line 241 labels the playable bucket as “Fairway.” The provided contract exposes sector direction separately from outcomeBucket: 'playable' | 'trouble' | 'penalty', so these labels can overstate the data.
Proposed change
const SECTOR_TAKEAWAY: Record<string, string> = {
- center: 'Tee shots tend to find the short grass',
+ center: 'Tee shots tend to finish near the intended line',
left: 'Misses lean left',
right: 'Misses lean right',
@@
const dist = [
{ key: 'left', label: 'Left', count: left, color: dominantMiss === 'left' ? WARNING : 'var(--fw-color-text-tertiary)' },
- { key: 'center', label: 'Fairway line', count: center, color: ACCENT },
+ { key: 'center', label: 'Target line', count: center, color: ACCENT },
{ key: 'right', label: 'Right', count: right, color: dominantMiss === 'right' ? WARNING : 'var(--fw-color-text-tertiary)' },
] as const;
const legend = [
- { label: 'Fairway', count: group.playableCount, color: ACCENT },
+ { label: 'Playable', count: group.playableCount, color: ACCENT },
{ label: 'Trouble', count: group.troubleCount, color: WARNING },
{ label: 'Penalty', count: group.penaltyCount, color: DANGER },
].filter((l) => l.count > 0);Also applies to: 234-243
🤖 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/coachhelm/FairwayDrivingSpray.tsx` around lines
58 - 60, The labels in SECTOR_TAKEAWAY constant and the outcome bucket labels
around lines 234-243 are overstating what the data represents by conflating
sector direction and outcome bucket values with specific terrain features. The
data contract only provides sector direction (center, left, right) and
outcomeBucket values (playable, trouble, penalty), but the current labels
suggest specific terrain types like "short grass" and "Fairway." Update the
SECTOR_TAKEAWAY labels to accurately describe the center, left, and right
sectors without assuming terrain type, and update the outcome bucket labels in
the affected code region to describe playable, trouble, and penalty outcomes
honestly without conflating them with specific fairway features.
| <stop offset="0%" stopColor="#ffffff" stopOpacity="0.55" /> | ||
| <stop offset="42%" stopColor="#ffffff" stopOpacity="0.14" /> | ||
| <stop offset="100%" stopColor={ACCENT} stopOpacity="0.06" /> | ||
| </linearGradient> | ||
| {/* 1σ ellipse fill — soft helm-green from the center out. */} | ||
| <radialGradient id={ellId} cx="50%" cy="50%" r="60%"> | ||
| <stop offset="0%" stopColor={ACCENT} stopOpacity="0.22" /> | ||
| <stop offset="100%" stopColor={ACCENT} stopOpacity="0.06" /> | ||
| </radialGradient> | ||
| <filter id={glowId} x="-40%" y="-40%" width="180%" height="180%"> | ||
| <feDropShadow dx="0" dy="0.5" stdDeviation="1.1" floodColor="#1c1917" floodOpacity="0.18" /> |
There was a problem hiding this comment.
Replace inline SVG hex colors with Fairway tokens.
Lines 275, 276, 285, and 306 hardcode hex colors in a component under src/components. Use the existing design tokens/CSS variables so the glass treatment remains theme-consistent.
Proposed change
- <stop offset="0%" stopColor="`#ffffff`" stopOpacity="0.55" />
- <stop offset="42%" stopColor="`#ffffff`" stopOpacity="0.14" />
+ <stop offset="0%" stopColor={SURFACE} stopOpacity="0.55" />
+ <stop offset="42%" stopColor={SURFACE} stopOpacity="0.14" />
@@
- <feDropShadow dx="0" dy="0.5" stdDeviation="1.1" floodColor="`#1c1917`" floodOpacity="0.18" />
+ <feDropShadow dx="0" dy="0.5" stdDeviation="1.1" floodColor="var(--fw-color-text-primary)" floodOpacity="0.18" />
@@
- stroke="`#ffffff`"
+ stroke={SURFACE}As per coding guidelines, “React + Tailwind. Use design-system tokens only … Reject inline hex values, ad-hoc spacing, or radius outside rounded-2xl/rounded-xl/ rounded-lg.”
Also applies to: 306-306
🤖 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/coachhelm/FairwayDrivingSpray.tsx` around lines
275 - 285, Replace the hardcoded hex colors in the SVG gradient and filter
definitions with Fairway design tokens to maintain theme consistency. In the
linearGradient element, replace the stopColor="`#ffffff`" values with an
appropriate design token variable. In the feDropShadow filter element, replace
the floodColor="`#1c1917`" hardcoded hex value with the corresponding Fairway
design token. Follow the existing pattern where ACCENT is already being used for
the radialGradient stops, ensuring all color references throughout the component
use design-system tokens instead of inline hex values.
Source: Coding guidelines
| <DrivingSection detailedStats={detailedStats} /> | ||
| <div className="flex flex-col gap-6"> | ||
| <DrivingSection detailedStats={detailedStats} /> | ||
| <FairwayDrivingSpray group={sprayData?.driving ?? null} /> |
There was a problem hiding this comment.
Don’t show “No tee-shot spray yet” for a failed spray fetch.
Line 574 passes null into FairwayDrivingSpray, but sprayData is also set to null when getSprayChartData rejects. That turns an enrichment failure into a false no-data message; fulfilled no-data responses already carry an empty driving group.
Proposed change
- <FairwayDrivingSpray group={sprayData?.driving ?? null} />
+ {sprayData ? <FairwayDrivingSpray group={sprayData.driving} /> : null}🤖 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/coachhelm/FairwayStatsCockpit.tsx` at line 574,
The issue is that when getSprayChartData rejects, sprayData is set to null,
which causes FairwayDrivingSpray to display "No tee-shot spray yet" incorrectly
since this message should only appear for successful responses with empty data,
not for fetch failures. To fix this, distinguish between fetch failures and
actual no-data by tracking the error state separately from sprayData, or by
using a different null/undefined sentinel value for failures. Modify the code
that handles the getSprayChartData rejection to set a separate error state
instead of setting sprayData to null, then only pass null to FairwayDrivingSpray
when sprayData is populated but contains no driving data, not when the fetch
fails.
|
Superseded by #304, which rolls this onto a single branch (feat/coachhelm-stats-roundup) with all four PRs merged clean + a merge-interaction fix. Branch |
1 participant
What
Adds
FairwayDrivingSpray— a premium SVG "landing view" of off-the-tee directions — to the Driving tab on the stats cockpit, and removes the old flat canvas driving scatter.The chart
SprayChartShotGroup.points(no fabricated coords), the ellipse needs ≥3 shots, and there's a calm empty state when there's no tee-shot spray yet.role="img"carries a full text summary for screen readers.Wiring
FairwayStatsCockpit) now renders<FairwayDrivingSpray group={sprayData?.driving} />beneathDrivingSection.ShotDispersionfrom the Analysis "Shot patterns" board (kept the approach miss map + putting heatmap) — no trashy duplicate.Surface/InsufficientData(not the giant@/components/fairwaybarrel) to avoid a circular-init in the cockpit chunk.Preview
Rendered in an isolated harness with real-shaped mock data (24 right-biased tee shots):
(Screenshot attached in the chat that opened this PR.)
Verification
tsc --noEmitclean ·npm run lint0 errors · 2510 unit tests pass.Reviewer note
Independent of #299/#300/#302. Touches
FairwayStatsCockpitin different regions than #302 (lie filter), so they merge cleanly. Not merged.🤖 Generated with Claude Code
Greptile Summary
This PR replaces the old flat-canvas driving
ShotDispersionwithFairwayDrivingSpray, a new premium SVG spray chart rendered in the Driving tab ofFairwayStatsCockpit. The old driving chart is simultaneously removed from the Analysis "Shot patterns" board, leaving only the approach miss map and putting heatmap there.FairwayDrivingSpray(421 lines, new): pure SVG frosted-glass plot with a 1σ covariance ellipse, shot landings coloured by outcome, a mean-bias marker, a left/center/right distribution bar, and an outcome legend — all backed only by realSprayChartShotGroup.pointswith a calm empty state when data is absent.FairwayStatsCockpit: Driving tab now wrapsDrivingSection+FairwayDrivingSprayin aflex flex-col gap-6;ShotPatternsstrips the drivingShotDispersionand its associated computed variables, and thehasAnyShotDataguard now only checks approach/putting data.Confidence Score: 4/5
Safe to merge — the new component is a self-contained leaf with a calm empty state and no data fabrication; the cockpit wiring is minimal and touches only the Driving tab and the Analysis shot-patterns board in the expected regions.
The math (covariance ellipse, projection, scale clamping) is correct and well-guarded. The main open question is whether the legend's server-supplied outcome counts (
playableCount/troubleCount/penaltyCount) are scoped to plotted shots or to all tracked shots — if they include unplotted shots the legend will show numbers larger than the visible dot count. The independent-rounding issue in the distribution bar is cosmetic. Neither finding blocks shipping but both are worth a quick check before the feature is promoted.src/components/fairway/pages/coachhelm/FairwayDrivingSpray.tsx — specifically the legend section and the pct() rounding in the distribution bar.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[FairwayStatsCockpit] -->|Driving tab| B[DrivingSection] A -->|Driving tab| C[FairwayDrivingSpray\ngroup=sprayData?.driving ?? null] A -->|Analysis tab| D[ShotPatterns] C --> E{group & points.length > 0?} E -- No --> F[InsufficientData empty state] E -- Yes --> G[Compute model\nlatMax, distMax, stats] G --> H[SVG glass panel\n+ target line] G --> I{n >= 3?} I -- Yes --> J[1σ covariance ellipse\nellipsePath] I -- No --> K[skip ellipse] H --> L[Shot landing dots\ncoloured by outcomeBucket] H --> M[Mean-bias marker] H --> N[Axis hints LEFT / TARGET / RIGHT] G --> O[Left·Center·Right\ndistribution bar] G --> P[Outcome legend\nplayable / trouble / penalty] D --> Q[ShotDispersion\nApproach only] D --> R[PuttingHeatmap]Reviews (1): Last reviewed commit: "feat(stats): premium glass driving spray..." | Re-trigger Greptile