fix(coachhelm): unify player/team trend across Brief, Players tab, Team Stats - #929
fix(coachhelm): unify player/team trend across Brief, Players tab, Team Stats#929njrini99-code wants to merge 3 commits into
Conversation
Four fixes in the post-round-to-insight pipeline that were letting
CoachHelm insights go stale silently:
1. RACE — round submit registered two independent after() callbacks
(stats-cache refresh + postRoundTrigger). after() callbacks run
concurrently, not in registration order, so the engine could read
golf_player_stats_cache before the refresh finished writing it.
Chained into a single after() that awaits the cache refresh THEN
runs postRoundTrigger.
2. ROSTER-SWEEP SKIP — the nightly sweep skipped a player whenever ANY
of their rounds was analyzed within a 12h window, even if their
genuinely most-recent round was still unanalyzed (e.g. an older
round got re-analyzed by the safety-net cron inside that window).
Now skips a player only when their MOST RECENT completed round has
coachhelm_analyzed_at set — sweeps whenever the latest round is
unanalyzed, regardless of when some earlier round was touched.
3. SILENT POSTURE VISIBILITY — alert_posture='silent' maps to an
infinite confidence threshold, silently blocking every insight for
that player with no signal anywhere. Kept the behavior, surfaced it:
(a) logServerEvent (info, skipSentry) once per run when the gate
blocks; (b) the CoachHelm Players-tab roster row now shows an
"Insights muted" indicator for players with silent posture (reads
the same golf_coach_player_intent the Roster page already loads via
loadCoachIntents — no new query pattern).
4. FAKE TIMESTAMP — the Brief's "updated" label rendered
golf_rounds.round_date (a DATE column, no time component) through
`new Date(dateOnly).toISOString()` then a viewer-timezone
toLocaleString with hour/minute — fabricating a specific clock time
("Jun 8, 8:00 PM") the round never had, and for viewers west of UTC,
silently shifting the calendar date itself back a day. Now formats
as a DATE ONLY ("Jun 8"), anchoring both parse and format in UTC so
every viewer sees the same calendar day.
Gates: tsc clean, eslint clean on all changed files, and the full
unit-test slate under the touched directories (55 files / 492 tests)
passes. Added regression tests for 1, 2 and 4 — each one verified to
FAIL against the pre-fix code and PASS against the fix.
Fixes #920
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…am Stats Part of #914. FOUR surfaces computed "improving/steady/declining" independently and disagreed for the same player/roster: - Brief category cards (team-category-insights.ts): per-round category metric, 5-vs-5 window, 0.5 threshold. - Players tab roster table (development/page.tsx): read golf_player_stats_cache.trend_direction, a DB-trigger value with its OWN 5-vs-5 window and 1.0-stroke threshold. - Team Stats trajectory tile + player cards (FairwayTeamStats.tsx / stats/team/page.tsx): live-computed 5-vs-5 window, 0.3-stroke threshold. Same underlying rounds, three different thresholds/sources, so a player could read "declining" on the roster table and "improving" on Team Stats. The Brief also summed attentionCount across all 5 categories for its "N need attention" figure, double-counting any player flagged in more than one category (observed live: "8 need attention" on a 7-player team). Fix: - New src/lib/coachhelm/trend.ts: canonical classifyTrendDelta + computeSeriesTrend (one 5-vs-5 window, one threshold-comparison definition, a hasSignal flag so "not enough history" never reads as a fabricated zero-delta "Steady"). Unit tested. - New src/lib/golf/scoring-trend.ts: computeScoringTrendFromRounds, the shared "player's overall score trend" wrapper (18-hole normalization + the canonical classifier) now consumed by BOTH stats/team/page.tsx and development/page.tsx so the same rounds always read the same verdict. - team-category-insights.ts now calls computeSeriesTrend instead of a private reimplementation (behavior preserved exactly — same window, same 0.5 threshold, same 1-previous-round floor). - development/page.tsx no longer reads golf_player_stats_cache.trend_direction for the roster table; it fetches each player's recent rounds (same per-player sampler team-category-insights.ts uses) and classifies via the canonical function, so it agrees with Team Stats for the same rounds. - FairwayTeamStats.tsx's classifyScoringTrend now delegates the improving/declining/steady decision to classifyTrendDelta. - FairwayBrief.tsx: "N need attention" is now the union of flagged player ids across categories (bounded by roster size), not a sum. The hero vs. category-card contradiction ("Putting needs the most work (20/100)" next to a green "Trending up" chip) is resolved by blending rating + trend in the category status label — a below-mid-line category that's improving now reads "Weak, improving" / "<label> is weak but improving" instead of a bare "Trending up" that reads as disagreeing with the hero. The 20/100 rating itself is untouched — rating and trend stay separate numbers, just labeled so they don't read as contradictory. - PlayersGridView.tsx: the "N players to look at" header count and its capped top-5 list now agree — a "+N more players need a look — showing the top 5 by priority" caption appears whenever the list is truncated. Gates: npx tsc --noEmit -p tsconfig.json (clean); npx eslint on all changed files (clean); npx vitest run on the new/touched test files (264 passed) plus a broader sweep of src/components/fairway, src/app/golf/actions/__tests__, src/lib/golf (288 files / 3492 tests passed, 4 pre-existing skips). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 Mission Control — PR summary What it changes: Part of #914. Four surfaces computed "improving/steady/declining" independently and disagreed for the same player/roster:
Same rounds, three thresholds/sources, so a player could read "declining" on the roster table and "improving" on Team Stats. The Brief also summed Risk / areas: golf CoachHelm trend logic across Brief, Players tab, Team Stats. Watch: every surface now adopts the unified threshold — displayed trends will shift for some players (expected); the de-duped attention count; parity of the new module vs the retired DB-trigger value. CI: ✅ green so far — 34 checks passing, 4 pending, 0 failing; mergeable state BLOCKED on required review (no CI failure). Awaiting review. |
…opment page import conflict Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
1 participant
Problem
Part of #914. FOUR CoachHelm surfaces each computed "improving / steady / declining" independently and disagreed for the same team/player (verified live):
team-category-insights.ts) — "Team putting trending up — 4 of 7 players improving" (per-round category metric, 5-vs-5 window, 0.5 threshold).development/page.tsx) — 6 Declining + 1 Steady, 0 improving (readgolf_player_stats_cache.trend_direction, a DB-trigger value with its OWN 5-vs-5 window and 1.0-stroke threshold).FairwayTeamStats.tsx) — 2 improving / 0 steady / 5 declining.Same underlying rounds, three independently-reimplemented thresholds/windows/sources — a player could read "declining" on the roster table and "improving" on Team Stats for the identical rounds.
Two more symptoms of the same root cause:
totalAttentionsummed each category'sattentionCount, double-counting any player flagged in more than one category.Fix
src/lib/coachhelm/trend.ts— canonicalclassifyTrendDelta+computeSeriesTrend: one 5-vs-5 recent/previous window, one threshold-comparison definition, ahasSignalflag so "not enough history" never gets fabricated into a zero-delta "Steady". Unit tested.src/lib/golf/scoring-trend.ts—computeScoringTrendFromRounds, the shared "player's overall score trend" wrapper (18-hole normalization + the canonical classifier), now consumed by bothstats/team/page.tsxanddevelopment/page.tsxso the same rounds always read the same verdict.team-category-insights.tsnow callscomputeSeriesTrendinstead of a private reimplementation (behavior preserved exactly — same window, same 0.5 threshold, same 1-previous-round floor).development/page.tsxno longer readstrend_directionfor the roster table's Trend column; it fetches each player's recent rounds (same per-player samplerteam-category-insights.tsalready uses, avoiding the "most-active player starves everyone else" bug) and classifies via the canonical function — so it agrees with Team Stats for the same rounds.FairwayTeamStats.tsx'sclassifyScoringTrendnow delegates the improving/declining/steady decision toclassifyTrendDelta(same 0.3 threshold, now sourced canonically instead of a second hardcoded copy).FairwayBrief.tsx:PlayersGridView.tsx— the "N players to look at" header count and its capped top-5 list now agree: a "+N more players need a look — showing the top 5 by priority" caption appears whenever the list is truncated.Gates
npx tsc --noEmit -p tsconfig.json— cleannpx eslinton all changed files — cleannpx vitest runon the new/touched test files — 264 passedsrc/components/fairway,src/app/golf/actions/__tests__,src/lib/golf) — 288 files / 3492 tests passed, 4 pre-existing skipsCaveats
development/page.tsx's new rounds fetch adds one paginated query (golf_rounds,player_id/total_score/holes_played/round_date, batched viafetchAllRowsResult, capped to each player's last 10 rounds in a 365-day window) — same pattern already used inteam-category-insights.ts, but it is additional DB load on that route.golf_player_stats_cache.trend_direction— that column is simply no longer read by the Players tab display. It may still be read elsewhere (patterns/predictions); a follow-up could migrate those too if this PR's approach is confirmed correct.Fixes #914
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg