fix(golf): classify remaining CoachHelm no-rounds empty states as expected#807
Merged
Conversation
…ected Sentry alerted at 23:11 UTC on /golf/dashboard/coachhelm: a rounds-less player loading the page fires FOUR sibling soft failures, and #804 only gave getPlayerShotAnalytics a stable code. getPlayerProfile, getPlayerTrendAnalysis, getPlayerShotContext, and the what-if scenario path in coachhelm-data.ts now carry codes on their empty-state returns (no_completed_rounds / insufficient_rounds / no_rounds_in_period), all registered under EXPECTED_EMPTY_STATE_CODES — 'info', skipSentry, same convention as #804. A brand-new player browsing CoachHelm is not a production error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WJ1Gzjj9MT3UsxhMoMzSV
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbit
WalkthroughServer action errors now include stable codes for routine missing-data conditions. Golf actions distinguish absent rounds and insufficient usable data from completed rounds lacking required score fields, while the observer classifies recognized codes as informational empty states. ChangesEmpty-State Error Classification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GolfAction
participant RoundData
participant ActionObserver
participant ServerLogger
GolfAction->>RoundData: query completed-round and scoring data
RoundData-->>GolfAction: empty, insufficient, or unusable results
GolfAction->>RoundData: count completed rounds when needed
RoundData-->>GolfAction: completed-round count
GolfAction-->>ActionObserver: return coded soft failure
ActionObserver->>ServerLogger: log expected empty state at info severity
🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (11 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml Comment |
Greptile SummaryThis PR reclassifies expected CoachHelm empty states so new-player pages do not create production-error noise. The main changes are:
Confidence Score: 4/5This is close, but the scoring-data case should be fixed before merging.
src/app/golf/actions/coachhelm-data.ts Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/app/golf/actions/coachhelm-data.ts:610-611
**Missing Score Guard**
`insufficient_rounds` is now treated as an expected empty state for every observed action. This branch counts only completed rounds where `score_to_par` is already populated, so a player with 3+ completed rounds whose score-to-par values failed to populate can still return `insufficient_rounds`. That logs as info with Sentry skipped instead of surfacing the scoring-data failure. The same check is needed before the what-if branch returns the same code.
Reviews (2): Last reviewed commit: "fix(golf): distinguish unscored complete..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…_STATE_CODES Greptile on #807: classification is by code globally, so a future real failure reusing one of these codes would silently skip Sentry. Spell that contract out where the set is defined. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WJ1Gzjj9MT3UsxhMoMzSV
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/golf/actions/coachhelm-data.ts`:
- Around line 299-300: Update the rounds handling around the completed-round
query and the roundsData empty check to distinguish players with no completed
rounds from players whose completed rounds lack usable scores. Only assign the
globally expected `no_completed_rounds` code when the completed-round query
itself is empty; return a separate data-quality failure without that code when
completed rounds exist but roundsData is empty after score filtering.
In `@src/lib/admin/observe-action-result.ts`:
- Around line 57-67: Extend the tests around the existing expected-empty-state
assertions in observe-action-result.test.ts to cover no_rounds_in_period,
no_completed_rounds, and insufficient_rounds. Verify each code takes the
expected empty-state classification and exercises the info/skipSentry path,
preserving the existing coverage for previously registered codes.
🪄 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: ae24a807-b43e-44f7-bd99-0dda0ad01866
📒 Files selected for processing (2)
src/app/golf/actions/coachhelm-data.tssrc/lib/admin/observe-action-result.ts
… new empty-state codes getPlayerProfile's round query filters total_score IS NOT NULL, so an empty result was ambiguous: the globally-silenced no_completed_rounds code could also fire when completed rounds exist but all lack scores — a data-quality failure that must stay visible per the EXPECTED_EMPTY_STATE_CODES contract. The empty branch now runs a head-count of completed rounds and only attaches the code when the player truly has none. Also adds regression coverage for the three newly registered codes (no_rounds_in_period, no_completed_rounds, insufficient_rounds) asserting the info/skipSentry classification path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WJ1Gzjj9MT3UsxhMoMzSV
PR Summary by QodoClassify CoachHelm no-rounds empty states via stable codes (skip Sentry)
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Comment on lines
610
to
611
| } | ||
|
|
There was a problem hiding this comment.
insufficient_rounds is now treated as an expected empty state for every observed action. This branch counts only completed rounds where score_to_par is already populated, so a player with 3+ completed rounds whose score-to-par values failed to populate can still return insufficient_rounds. That logs as info with Sentry skipped instead of surfacing the scoring-data failure. The same check is needed before the what-if branch returns the same code.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/golf/actions/coachhelm-data.ts
Line: 610-611
Comment:
**Missing Score Guard**
`insufficient_rounds` is now treated as an expected empty state for every observed action. This branch counts only completed rounds where `score_to_par` is already populated, so a player with 3+ completed rounds whose score-to-par values failed to populate can still return `insufficient_rounds`. That logs as info with Sentry skipped instead of surfacing the scoring-data failure. The same check is needed before the what-if branch returns the same code.
How can I resolve this? If you propose a fix, please make it concise.
Owner
Author
There was a problem hiding this comment.
Fixed in a35da6f — both insufficient_rounds sites (trend analysis and the what-if scenario path) now run the same unfiltered completed-round head-count as the profile fix: the globally-silenced code only attaches when the player truly has fewer than 3 completed rounds; 3+ completed rounds lacking score data returns an uncoded data-quality error that stays visible to Sentry. Follow-up 8864372 also guards all three count queries' own error, so a failed count can't fall through to the silenced path either.
Generated by Claude Code
Code Review by Qodo
Context used✅ Compliance rules (platform):
98 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Same contract violation as the getPlayerProfile fix in aeda380, in the two insufficient_rounds sites: getPlayerTrendAnalysis and the what-if scenario path both filter score_to_par IS NOT NULL, so a player with 3+ completed rounds whose scores never populated would be silenced as an expected empty state. Both branches now head-count completed rounds and only attach the globally-silenced code when the player truly has fewer than 3; otherwise they return an uncoded data-quality error that stays visible to Sentry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WJ1Gzjj9MT3UsxhMoMzSV
… states Qodo caught that the three fallback head-count queries ignored the Supabase error: a failed count leaves count null, which fell through to the globally-silenced empty-state code — hiding a genuine backend failure from Sentry. All three guards now return an uncoded failure when the count query errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WJ1Gzjj9MT3UsxhMoMzSV
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
2 participants
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.
Summary
Sentry alerted at 23:11 UTC tonight on
GET /golf/dashboard/coachhelm. Root cause: a player with no completed rounds loading the CoachHelm page fires four sibling expected-empty-state soft failures, and #804 only gavegetPlayerShotAnalyticsa stable code —getPlayerProfile,getPlayerTrendAnalysis,getPlayerShotContext, and the what-if scenario path (all incoachhelm-data.ts) still log at error severity and reach Sentry.This extends #804's convention: each empty-state return now carries a stable
code(no_completed_rounds/insufficient_rounds/no_rounds_in_period), all registered underEXPECTED_EMPTY_STATE_CODESin the soft-failure observer (info, skipSentry). TheActionErrorenvelope gains an optionalcodefield.Partner-readable summary
A brand-new player opening the CoachHelm page was being counted as a production error four different ways. It's now correctly treated as a normal empty state, keeping error dashboards meaningful.
Type of change
Area
golf · coachhelm
Risk level
successonly)Git Activity Timeline note
Stops new players' empty CoachHelm pages from being logged as production errors.
Checklist
npm run typecheckpasses; eslint --max-warnings 0 clean on touched files🤖 Generated with Claude Code
https://claude.ai/code/session_019WJ1Gzjj9MT3UsxhMoMzSV
Generated by Claude Code