diff --git a/src/app/golf/(dashboard)/dashboard/alerts/page.tsx b/src/app/golf/(dashboard)/dashboard/alerts/page.tsx index 200d27653..93f5aa8d5 100644 --- a/src/app/golf/(dashboard)/dashboard/alerts/page.tsx +++ b/src/app/golf/(dashboard)/dashboard/alerts/page.tsx @@ -16,6 +16,7 @@ import { createClient } from '@/lib/supabase/server'; import { getGolfSessionProfile } from '@/lib/auth/session'; import { getAlertCounts } from '@/app/golf/actions/alerts'; import { getInsightsForCoachWithMeta } from '@/app/golf/actions/insight-delivery'; +import { getTeamPlayers } from '@/app/golf/actions/roster'; import { fairwayScope } from '@/lib/redesign/flag'; import { FairwayCoachHelmSignals, EmptyState, Button } from '@/components/fairway'; import { resolveCoachTeamIdWithCookie } from '@/lib/golf/resolve-team-server'; @@ -98,17 +99,28 @@ export default async function AlertsPage() { // so the Signals workspace paints data on the first frame instead of // mounting at loading=true → an always-on client fetch + skeleton flash on // every /alerts visit. Mirrors how /patterns already SSR-seeds initialPatterns. - const [countsRes, insightsRes] = await Promise.all([ + // Roster names in the SAME round trip: the "By player" grouping resolves + // insight-sourced rows' `playerName` from this map (insights only carry + // `player_id`, never a joined name — unlike patterns, which resolve it + // inline via `getTeamPatterns`). Without it every insight/alert row falls + // back to the grouping's "Unknown player" bucket. + const [countsRes, insightsRes, rosterRes] = await Promise.all([ getAlertCounts(coach.id), getInsightsForCoachWithMeta(coach.id, { limit: 100, priorities: ['urgent', 'high'], }), + getTeamPlayers(), ]); const signalCount = countsRes.success ? (countsRes.counts?.critical ?? null) : null; // Honest fallback: a DB error leaves initialInsights empty so the surface // falls back to its own client fetch + error handling (never a fake feed). const initialInsights = insightsRes.ok ? insightsRes.data : []; + const playerNames: Record = {}; + for (const p of rosterRes.data ?? []) { + const name = [p.first_name, p.last_name].filter(Boolean).join(' ').trim(); + if (name) playerNames[p.id] = name; + } return (
= {}; + for (const p of rosterRes.data ?? []) { + const name = [p.first_name, p.last_name].filter(Boolean).join(' ').trim(); + if (name) playerNames[p.id] = name; + } return (
diff --git a/src/components/fairway/charts/StrokesGainedTornado.test.tsx b/src/components/fairway/charts/StrokesGainedTornado.test.tsx new file mode 100644 index 000000000..30b5726ea --- /dev/null +++ b/src/components/fairway/charts/StrokesGainedTornado.test.tsx @@ -0,0 +1,61 @@ +// @vitest-environment jsdom +/** + * ============================================================================ + * StrokesGainedTornado — duplicate-label row-collision regression guard + * ---------------------------------------------------------------------------- + * Bug: `TornadoInner`'s y-scale (`scaleBand`) used to key its domain by + * `d.label`. Two rows CAN legitimately share a label — e.g. the Effectiveness + * cockpit's "Most impactful patterns" tornado labels each bar with the + * player's NAME, and one player can have two separate top-N patterns — + * `scaleBand`'s domain is a distinct key set, so a duplicate label collapsed + * both rows onto the SAME band. Their bars AND their signed value-annotation + * text then rendered on top of each other — observed live as two adjacent + * labels like "+4.10" and "+4.67" reading as the single garbled string + * "+4.10+4.67". + * + * Fix: the y-scale is now keyed by ROW INDEX (always unique), never by the + * display label. This locks that duplicate labels get distinct rows. + * ========================================================================== */ +import { render } from '@testing-library/react'; +import { describe, it, expect } from 'vitest'; +import { TornadoInner, type SGCategory } from './StrokesGainedTornado'; + +describe('StrokesGainedTornado — duplicate-label collision guard', () => { + it('two rows sharing the SAME label render on DISTINCT y positions (never collide)', () => { + const data: SGCategory[] = [ + { label: 'Jordan Lee', value: 4.1 }, + { label: 'Jordan Lee', value: 4.67 }, // same player, two top-N patterns + { label: 'Sam Park', value: -2.3 }, + ]; + const { container } = render(); + const texts = Array.from(container.querySelectorAll('text')); + const find = (t: string) => texts.find((el) => el.textContent === t); + + const first = find('+4.10'); + const second = find('+4.67'); + const third = find('−2.30'); // formatSigned uses U+2212 minus, not a hyphen + + expect(first).toBeDefined(); + expect(second).toBeDefined(); + expect(third).toBeDefined(); + + const ys = [first, second, third].map((el) => el!.getAttribute('y')); + // All three rows must land on distinct y positions — a regression back to + // label-keying would collapse the two 'Jordan Lee' rows onto the same y. + expect(new Set(ys).size).toBe(3); + }); + + it('unique labels still render one row each at distinct positions (no regression)', () => { + const data: SGCategory[] = [ + { label: 'Off the tee', value: 1.1 }, + { label: 'Approach', value: -0.8 }, + ]; + const { container } = render(); + const texts = Array.from(container.querySelectorAll('text')); + const ys = texts + .filter((el) => el.textContent === '+1.10' || el.textContent === '−0.80') + .map((el) => el.getAttribute('y')); + expect(ys.length).toBe(2); + expect(new Set(ys).size).toBe(2); + }); +}); diff --git a/src/components/fairway/charts/StrokesGainedTornado.tsx b/src/components/fairway/charts/StrokesGainedTornado.tsx index 725706a55..052b0925d 100644 --- a/src/components/fairway/charts/StrokesGainedTornado.tsx +++ b/src/components/fairway/charts/StrokesGainedTornado.tsx @@ -105,7 +105,13 @@ export function StrokesGainedTornado({ ); } -function TornadoInner({ +/** + * Exported for unit testing only — the parent `StrokesGainedTornado` always + * mounts this through ``, which needs real DOM layout. Rendering + * `TornadoInner` directly with explicit `width`/`height` lets a test assert + * on the resulting SVG geometry without fighting ResizeObserver in jsdom. + */ +export function TornadoInner({ width, height, data, @@ -130,10 +136,18 @@ function TornadoInner({ [bound, innerW], ); + // Keyed by ROW INDEX, never by `d.label`. Two rows can legitimately share a + // label (e.g. `PatternImpactDeck`'s "Most impactful patterns" tornado uses + // the player's NAME as the label, and one player can have two separate + // high-impact patterns in the top-N) — `scaleBand`'s domain is a distinct + // key set, so a duplicate label previously collapsed both rows onto the + // SAME band, rendering their bars and value-annotation text on top of each + // other (e.g. two adjacent "+4.10" / "+4.67" labels reading as the single + // garbled string "+4.10+4.67"). Index-based keys are always unique. const yScale = React.useMemo( () => - scaleBand({ - domain: data.map((d) => d.label), + scaleBand({ + domain: data.map((_, i) => i), range: [0, innerH], padding: ROW_PAD, }), @@ -177,15 +191,15 @@ function TornadoInner({ strokeDasharray="5 4" /> - {data.map((d) => { - const y = yScale(d.label) ?? 0; + {data.map((d, i) => { + const y = yScale(i) ?? 0; const valX = xScale(d.value); const positive = d.value >= 0; const barX = positive ? zeroX : valX; const barW = Math.abs(valX - zeroX); const fill = positive ? VIZ_DIVERGING.positive : VIZ_DIVERGING.negative; return ( - + {/* category label in the left gutter */} display name` for the coach's team roster (SSR-resolved via + * `getTeamPlayers`). Insight-sourced rows (alerts/insights) don't carry a + * player name of their own — only patterns do (resolved inline by + * `getTeamPatterns`) — so this is how the "By player" grouping gets a real + * name instead of falling back to "Unknown player" for every row. Optional: + * omitting it just means insight rows group under "Unknown player", the + * prior (degraded) behavior — never a hard failure. + */ + playerNames?: Record; /* shell chrome */ /** Urgent+high open-signal count, computed once server-side (shell badge). */ @@ -287,6 +297,7 @@ export function FairwayCoachHelmSignals({ defaultFilter, initialInsights = [], initialPatterns = [], + playerNames, signalCount, showScanTeam = false, title, @@ -339,7 +350,7 @@ export function FairwayCoachHelmSignals({ const [isBulkPending, startActionTransition] = useTransition(); /* -- P058: the TRUE eligible insight total (post rank+dedupe, pre-100-slice) - from the meta read, so the "Loaded" tile can honestly disclose + from the meta read, so the "Showing" tile can honestly disclose "N of TOTAL" rather than silently truncating at the cap. Insights-only; patterns carry their own `patternCounts`. null until first load. */ const [eligibleTotal, setEligibleTotal] = useState(null); @@ -619,21 +630,46 @@ export function FairwayCoachHelmSignals({ /* ── project both sources into the ONE row vocabulary ──────────────────── */ const allRows: SignalRow[] = useMemo(() => { if (isPatterns) return patternsToSignalRows(patterns); - return insightsToSignalRows(insights); - }, [isPatterns, patterns, insights]); - - // Team "where are we bleeding strokes" rollup (LeakBoard) — groups the live - // insights by skill category by summed stroke-impact. Previously orphaned - // (vizlab demo only); fed here from the SAME live insight rows the list shows. - // Insights only (patterns have their own evidence shape); LeakBoard renders - // its own honest-empty state, so we just pass what we have. + return insightsToSignalRows(insights, playerNames); + }, [isPatterns, patterns, insights, playerNames]); + + // Team "where are we bleeding strokes" rollup (LeakBoard) — groups insights + // by skill category by summed stroke-impact. Fed from its OWN full + // team-wide read (below), NOT the active sub-tab's `insights` state: that + // state is scoped per route (e.g. /alerts fetches urgent+high ONLY, via + // `priorities: defaultFilter.fetchPriorities`; /insights fetches the full + // range), so reusing it made the SAME "Where the team is bleeding" banner + // silently re-total depending on which sub-tab was open. A stable, whole- + // team source means the board reads the same number on /alerts and + // /insights. Insights only (patterns have their own evidence shape); + // LeakBoard renders its own honest-empty state, so we just pass what we have. + const [leakSourceInsights, setLeakSourceInsights] = useState([]); + useEffect(() => { + if (isPatterns) return; + let cancelled = false; + void (async () => { + try { + // Deliberately NO `priorities` filter — the whole team, every + // severity, is the stable scope this rollup promises. + const res = await getInsightsForCoachWithMeta(coachId, { limit: 100 }); + if (!cancelled && res.ok) setLeakSourceInsights(res.data); + } catch { + // Best-effort / failure-silent — the board just stays at its own + // honest empty state, never a stale or wrong total. + } + })(); + return () => { + cancelled = true; + }; + }, [isPatterns, coachId]); + const leakInsights: LeakInsight[] = useMemo(() => { if (isPatterns) return []; const PRIORITY: Record = { critical: 'urgent', urgent: 'urgent', high: 'high', medium: 'medium', low: 'low', info: 'low', }; const out: LeakInsight[] = []; - for (const ins of insights) { + for (const ins of leakSourceInsights) { const impact = ins.evidence?.strokes_impact; if (typeof impact !== 'number' || impact === 0) continue; out.push({ @@ -648,7 +684,7 @@ export function FairwayCoachHelmSignals({ }); } return out; - }, [isPatterns, insights]); + }, [isPatterns, leakSourceInsights]); /* ── client-side filter (ported applyClientFilters) ────────────────────── */ const weight: Record = useMemo( @@ -1124,15 +1160,24 @@ export function FairwayCoachHelmSignals({ return Array.from(seen.entries()).map(([value, label]) => ({ value, label })); }, [allRows]); - /* ── header summary tiles (honest counts, never fabricated) ────────────── */ + /* ── header summary tiles (honest counts, never fabricated) ────────────── + "Urgent + high" is a SUBSET of "Open" — both scoped to open rows, priority + the narrower filter — so the tiles can never contradict each other (the + old version counted urgent+high across EVERY loaded row regardless of + status, which let a sub-tab that pre-filtered its server fetch to + urgent/high-only, like /alerts, show "Urgent + high" HIGHER than "Open" + whenever a few of those loaded rows were already acknowledged/resolved — + e.g. 13 urgent+high vs 10 open, reading as if there were MORE urgent + signals than open ones). This also now matches the shell badge's own + semantics (`getAlertCounts().counts.critical` — open urgent+high only). */ const summary = useMemo(() => { - const open = allRows.filter( + const openRows = allRows.filter( (r) => r.status === 'active' || r.status === 'Detected' || r.status === 'Confirmed', - ).length; - const urgent = allRows.filter( + ); + const urgent = openRows.filter( (r) => r.priority === 'critical' || r.priority === 'high', ).length; - return { total: allRows.length, open, urgent }; + return { total: allRows.length, open: openRows.length, urgent }; }, [allRows]); /* P058: honest "of N" footnote on the Loaded tile when the true eligible set @@ -1142,8 +1187,11 @@ export function FairwayCoachHelmSignals({ IS a known overflow — otherwise the value already IS complete. */ const loadedFootnote = useMemo(() => { if (isPatterns) { + // "capped at N" (not "showing first N") — the tile label itself is + // "Showing", so pairing it with a footnote that repeats the word read + // as a stutter ("SHOWING 5 / showing first 5"). if (patternCounts?.capped) { - return `showing first ${summary.total}`; + return `capped at ${summary.total}`; } return undefined; } @@ -1153,6 +1201,16 @@ export function FairwayCoachHelmSignals({ return undefined; }, [isPatterns, patternCounts, eligibleTotal, allRows.length, summary.total]); + /** The sub-tab noun the KPI tiles are scoped to — each tile already reflects + * ONLY the active segment's own data (its own fetch/state), so the fix is + * labeling that scope honestly instead of a generic "signals" that reads as + * if all three sub-tabs shared one team-wide count. */ + const SIGNAL_NOUN: Record = { + alerts: 'alerts', + insights: 'insights', + patterns: 'patterns', + }; + /* ── grouping — drives BOTH the dense default feed and the grouped view ── */ // While the smart-default shortlist is active, render it FLAT — a curated // cross-player triage list fragments badly grouped one-row-per-player. @@ -1587,20 +1645,31 @@ export function FairwayCoachHelmSignals({ {/* honest summary tiles — never fabricate a 0%; show counts only. Compact 3-up even on mobile so the triage feed isn't pushed below - the fold by three full-width stacked cards (premium-polish pass). */} + the fold by three full-width stacked cards (premium-polish pass). + All three tiles are scoped to the ACTIVE sub-tab's own data (its + own fetch/state) — the label now says so explicitly ("Open + alerts" on /alerts, "Open insights" on /insights, "Open patterns" + on /patterns) instead of a generic "Open signals" that read as if + it were a stable team-wide number, silently re-scoping as the + coach switched tabs. */}
- + {/* P035: no `goodDirection` here — it only colors a `delta` chip, and - there is no delta on this tile, so it was a dead no-op prop. */} + there is no delta on this tile, so it was a dead no-op prop. + Scoped to the SAME open rows as the tile before it (see the + `summary` useMemo above), so this can never read higher than + "Open" — it's a severity breakdown OF the open count, not an + independent count across every loaded row regardless of status. */} - {/* "Loaded" (not "Total"): this counts the rows currently in view — - the read caps at limit:100 and the smart default narrows further — - so labeling it "Total" over-claims completeness when more are - eligible than loaded. P058: when the eligible total exceeds the - loaded set, disclose "of N" honestly rather than silently - truncating; the footnote earns the tile its completeness claim. */} + {/* "Showing" (not "Loaded" — dev-speak a coach shouldn't have to + parse, and not "Total", which over-claims completeness when more + are eligible than loaded: the read caps at limit:100 and the + smart default narrows further). P058: when the eligible total + exceeds the loaded set, disclose "of N" honestly rather than + silently truncating; the footnote earns the tile its + completeness claim. */} diff --git a/src/components/fairway/pages/coachhelm/FairwayEffectiveness.test.ts b/src/components/fairway/pages/coachhelm/FairwayEffectiveness.test.ts new file mode 100644 index 000000000..977f3a972 --- /dev/null +++ b/src/components/fairway/pages/coachhelm/FairwayEffectiveness.test.ts @@ -0,0 +1,38 @@ +/** + * ============================================================================ + * FairwayEffectiveness — accuracy-headline gate regression guard + * ---------------------------------------------------------------------------- + * Bug: the PRIMARY accuracy headline ("ACCURACY 100% ▲+8%") gated on + * `GAUGE_MIN_RESOLVED` (2), a much looser threshold than the calibration side + * panel's own stated requirement ("Still calibrating — needs 5 resolved + * predictions…"). A near-empty sample (e.g. 2-for-2, or genuinely 0/1 + * resolved once any rounding/staleness in the upstream rollup is in play) + * could render an authoritative percentage a coach has no way to distrust. + * + * Fix: the headline now gates on the SAME threshold as the side panel + * (`BUCKET_MIN_RESOLVED` = 5). This locks that the two thresholds cannot + * drift apart again. + * ========================================================================== */ +import { describe, it, expect } from 'vitest'; +import { isAccuracyHeadlineLive } from './FairwayEffectiveness'; + +describe('isAccuracyHeadlineLive', () => { + it('is NOT live at zero resolved predictions', () => { + expect(isAccuracyHeadlineLive(0)).toBe(false); + }); + + it('is NOT live below the 5-resolved threshold (the old 2-resolved gate is gone)', () => { + expect(isAccuracyHeadlineLive(1)).toBe(false); + expect(isAccuracyHeadlineLive(2)).toBe(false); + expect(isAccuracyHeadlineLive(3)).toBe(false); + expect(isAccuracyHeadlineLive(4)).toBe(false); + }); + + it('is live at exactly 5 resolved predictions — matching the calibration panel\'s own stated "needs 5" copy', () => { + expect(isAccuracyHeadlineLive(5)).toBe(true); + }); + + it('stays live well above the threshold', () => { + expect(isAccuracyHeadlineLive(50)).toBe(true); + }); +}); diff --git a/src/components/fairway/pages/coachhelm/FairwayEffectiveness.tsx b/src/components/fairway/pages/coachhelm/FairwayEffectiveness.tsx index 934c059d1..50534ef2b 100644 --- a/src/components/fairway/pages/coachhelm/FairwayEffectiveness.tsx +++ b/src/components/fairway/pages/coachhelm/FairwayEffectiveness.tsx @@ -119,12 +119,27 @@ import { * Honesty thresholds (resolved decisions — deterministic product rule) * • per-bucket InsufficientData when a ConfidenceBucket has < 5 resolved * • a global low-confidence caption while total resolved < 50 - * • the gauge/ribbon need ≥ 2 validated points before they read a value + * • the ribbon needs ≥ 2 validated points before it draws a line * ─────────────────────────────────────────────────────────────────────────── */ const BUCKET_MIN_RESOLVED = 5; const GLOBAL_LOW_CONFIDENCE_RESOLVED = 50; const GAUGE_MIN_RESOLVED = 2; +/** + * Whether the PRIMARY accuracy headline (the huge "ACCURACY 100%" hero + its + * Climbing/Holding delta chip) is trustworthy enough to show a real number. + * Gated on `BUCKET_MIN_RESOLVED` — the SAME threshold the calibration side + * panel already states in its own copy ("Still calibrating — needs + * {BUCKET_MIN_RESOLVED} resolved predictions…"). This used to gate on the + * much looser `GAUGE_MIN_RESOLVED` (2), which let a near-empty sample (e.g. + * 2 for 2) render an authoritative "100% ▲+8%" headline — a coach reading + * the ONE cockpit tile has no way to know that came from two coin flips. + * Exported for unit testing. + */ +export function isAccuracyHeadlineLive(resolved: number): boolean { + return resolved >= BUCKET_MIN_RESOLVED; +} + /* ════════════════════════════════════════════════════════════════════════════ * P1-12 — INSIGHT TRUST LAYER (unified event-ledger rollup) * ---------------------------------------------------------------------------- @@ -277,9 +292,20 @@ function TrustTrendGlyph({ trend }: { trend: TrustSignal['recentTrend'] }) { ); } + // No trend yet (not enough measured outcomes to compute one). Previously a + // bare "—" character with no visible label — an orphan placeholder that + // reads as broken/meaningless at a glance rather than an honest "not enough + // data" state (the title/aria-label were the only context, invisible to a + // sighted user scanning the table without hovering). Now matches the SAME + // icon idiom as the other three states, dimmed to read as "no data" rather + // than a genuine flat trend. return ( - - — + + + No trend data yet ); } @@ -665,7 +691,7 @@ function PrimaryInstrument({ data, days }: { data?: PredictionPerformanceData; d const lowConfidence = resolved > 0 && resolved < GLOBAL_LOW_CONFIDENCE_RESOLVED; - const live = resolved >= GAUGE_MIN_RESOLVED; + const live = isAccuracyHeadlineLive(resolved); return ( @@ -680,8 +706,8 @@ function PrimaryInstrument({ data, days }: { data?: PredictionPerformanceData; d display={formatPercent(accuracy, 0)} size="hero" state={live ? 'live' : 'awaiting'} - samples={live ? undefined : { have: resolved, need: GAUGE_MIN_RESOLVED }} - awaitingLabel="Awaiting predictions" + samples={live ? undefined : { have: resolved, need: BUCKET_MIN_RESOLVED }} + awaitingLabel="Calibrating" /> {live && typeof climbDelta === 'number' ? ( display name` map (the SSR-resolved team roster) and resolve + * `playerName` from it. This locks that contract. + * ========================================================================== */ +import { describe, it, expect } from 'vitest'; +import { + insightToSignalRow, + insightsToSignalRows, +} from './patternToInsightVocabulary'; +import type { EvidenceInsight } from '@/app/golf/actions/insight-delivery'; +import type { InsightEvidence } from '@/lib/coachhelm/v2/insights/types'; + +function makeInsight(overrides: Partial = {}): EvidenceInsight { + return { + id: 'insight-1', + player_id: 'player-1', + category: 'putting', + insight_type: 'putts_per_round', + title: 'Three-putt rate is climbing', + content: 'Three-putts are up over the last 5 rounds.', + signature: 'v3:x', + evidence: { + strokes_impact: 1.2, + confidence: 0.8, + sample_n: 12, + } as unknown as InsightEvidence, + metadata: null, + lifecycle_state: 'detected', + status: 'active', + priority: 'high', + acknowledged_at: null, + resolved_at: null, + created_at: '2026-07-01T00:00:00.000Z', + updated_at: '2026-07-01T00:00:00.000Z', + ...overrides, + }; +} + +describe('insightToSignalRow — player-name resolution', () => { + it('resolves playerName from the roster map when the player_id is present', () => { + const row = insightToSignalRow( + makeInsight({ player_id: 'player-1' }), + { 'player-1': 'Nick Rini', 'player-2': 'Jordan Lee' }, + ); + expect(row.playerName).toBe('Nick Rini'); + // The raw id stays available on the row regardless (used for focus-area + // conversion / deep links) — resolving a name must not erase it. + expect(row.playerId).toBe('player-1'); + }); + + it('falls back to undefined (never a fabricated name) when the map has no entry for the player', () => { + const row = insightToSignalRow( + makeInsight({ player_id: 'player-not-on-roster' }), + { 'player-1': 'Nick Rini' }, + ); + expect(row.playerName).toBeUndefined(); + }); + + it('falls back to undefined when no roster map is supplied at all (back-compat)', () => { + const row = insightToSignalRow(makeInsight()); + expect(row.playerName).toBeUndefined(); + }); + + it('treats an empty resolved name (blank first/last name) the same as "not found"', () => { + const row = insightToSignalRow( + makeInsight({ player_id: 'player-1' }), + { 'player-1': '' }, + ); + expect(row.playerName).toBeUndefined(); + }); +}); + +describe('insightsToSignalRows — batch resolution forwards the SAME map to every row', () => { + it('resolves distinct names for distinct players in one batch', () => { + const rows = insightsToSignalRows( + [ + makeInsight({ id: 'a', player_id: 'player-1' }), + makeInsight({ id: 'b', player_id: 'player-2' }), + makeInsight({ id: 'c', player_id: 'player-3' }), + ], + { 'player-1': 'Nick Rini', 'player-2': 'Jordan Lee' }, + ); + expect(rows.map((r) => r.playerName)).toEqual([ + 'Nick Rini', + 'Jordan Lee', + undefined, // player-3 not on the roster map + ]); + // Distinct real names means the "By player" grouping produces distinct + // buckets instead of collapsing every row into "Unknown player". + const distinctNames = new Set(rows.map((r) => r.playerName).filter(Boolean)); + expect(distinctNames.size).toBe(2); + }); +}); diff --git a/src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts b/src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts index 5e0b86ca4..442d2dacd 100644 --- a/src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts +++ b/src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts @@ -274,8 +274,24 @@ function formatComparisonValue(value: number): string { return Number.isInteger(value) ? String(value) : value.toFixed(2); } -/** Project a single evidence insight into the shared row shape. */ -export function insightToSignalRow(insight: EvidenceInsight): SignalRow { +/** + * Project a single evidence insight into the shared row shape. + * + * `playerNames` is an OPTIONAL `player_id -> display name` lookup (the coach's + * team roster, SSR-resolved from `golf_players` by the route fork). Insights + * themselves never carry a resolved name — `EvidenceInsight` is joined off + * `golf_coach_insights` only — so without this map every insight-sourced row + * left `playerName` `undefined` and the "By player" grouping's fallback + * (`r.playerName?.trim() || 'Unknown player'`) collapsed EVERY insight/alert + * into one "Unknown player" bucket, unlike patterns (which resolve the name + * inline via `getTeamPatterns`' own `golf_players` join). A missing map entry + * (e.g. a player removed from the roster) still falls back to `undefined` — + * never a fabricated name. + */ +export function insightToSignalRow( + insight: EvidenceInsight, + playerNames?: Record, +): SignalRow { const categoryLabel = titleCaseToken(insight.category) || 'Signal'; const ev = insight.evidence; return { @@ -286,7 +302,7 @@ export function insightToSignalRow(insight: EvidenceInsight): SignalRow { body: insight.content ?? '', overline: `${categoryLabel} · Signal`, playerId: insight.player_id, - playerName: undefined, + playerName: playerNames?.[insight.player_id] || undefined, category: insight.category, status: insight.status, createdAt: insight.created_at, @@ -443,7 +459,11 @@ export function patternsToSignalRows(patterns: ExtendedPattern[]): SignalRow[] { return patterns.map(patternToSignalRow); } -/** Batch helper: project an array of insights. */ -export function insightsToSignalRows(insights: EvidenceInsight[]): SignalRow[] { - return insights.map(insightToSignalRow); +/** Batch helper: project an array of insights. `playerNames` is forwarded to + * every row — see `insightToSignalRow` for why it's needed. */ +export function insightsToSignalRows( + insights: EvidenceInsight[], + playerNames?: Record, +): SignalRow[] { + return insights.map((insight) => insightToSignalRow(insight, playerNames)); } diff --git a/src/components/golf/coachhelm/coach/LeakBoard.test.tsx b/src/components/golf/coachhelm/coach/LeakBoard.test.tsx new file mode 100644 index 000000000..d5898de45 --- /dev/null +++ b/src/components/golf/coachhelm/coach/LeakBoard.test.tsx @@ -0,0 +1,70 @@ +// @vitest-environment jsdom +/** + * ============================================================================ + * LeakBoard — "str/rd" honesty regression guard + * ---------------------------------------------------------------------------- + * The per-category total is a SUM of `strokes_impact` across every leak + * insight for every player in that category — NOT a per-round rate. It used + * to be labeled "str/rd" (a per-round unit), which read as though a −26.4 + * total meant the team lost 26 strokes EVERY round — ~8x the genuine team + * SG-putting figure (~−3.19/rd). This locks the honest "total" label so a + * regression back to "str/rd" (or an equivalent per-round claim) fails. + * ========================================================================== */ +import { render, screen } from '@testing-library/react'; +import { describe, it, expect } from 'vitest'; +import { LeakBoard, type LeakInsight } from './LeakBoard'; + +function makeLeak(overrides: Partial = {}): LeakInsight { + return { + id: 'leak-1', + category: 'putting', + title: 'Three-putt rate is climbing', + strokesImpact: 1.5, + priority: 'high', + playerName: 'player-1', + ...overrides, + }; +} + +describe('LeakBoard — honest units', () => { + it('labels the summed total "total", never "str/rd" (a per-round claim it cannot back)', () => { + render( + , + ); + expect(screen.getAllByText('total').length).toBeGreaterThan(0); + expect(screen.queryByText('str/rd')).toBeNull(); + }); + + it('sums the magnitude across every leak/player in a category (never averages or divides by round count)', () => { + render( + , + ); + // 2 + 3 = 5.0, not an average (2.5) and not a single player's figure. + expect(screen.getByText('−5.0')).toBeInTheDocument(); + }); + + it('never claims a per-round rate in the summary sentence', () => { + render( + , + ); + const summary = screen.getByText(/leak/i, { selector: 'p' }); + expect(summary.textContent).not.toMatch(/per round|str\/rd/i); + }); + + it('renders the honest empty state when there are no leaks', () => { + render(); + expect(screen.getByText(/No live leaks right now/i)).toBeInTheDocument(); + }); +}); diff --git a/src/components/golf/coachhelm/coach/LeakBoard.tsx b/src/components/golf/coachhelm/coach/LeakBoard.tsx index 3a8f940a5..740ca760a 100644 --- a/src/components/golf/coachhelm/coach/LeakBoard.tsx +++ b/src/components/golf/coachhelm/coach/LeakBoard.tsx @@ -9,12 +9,18 @@ * strokes." A coach with 12 players and 90 seconds needs the latter. * * The LeakBoard regroups live insights by skill category and makes the UNIT of - * every row the summed STROKE-IMPACT (−X.X str/rd), biggest bleed first — so the - * flood of insights collapses into a triage board read in one scan. Each row - * carries its leak count, the players affected, the worst severity, and a - * "high bleed" flag at ≥ 0.8 str/rd. + * every row the SUMMED stroke-impact across every leak in that category — + * biggest bleed first — so the flood of insights collapses into a triage + * board read in one scan. Each row carries its leak count, the players + * affected, the worst severity, and a "high bleed" flag at ≥ `flameThreshold`. * - * Honesty: stroke-impact is the engine's own counterfactual magnitude (already + * Honesty: the total is a SUM across every player and every leak insight in + * the category — NOT a per-round rate. It used to be labeled "str/rd" (a + * per-round unit), which read as if −26.4 meant a team losing 26 strokes + * EVERY round — 8x the genuine team SG-putting figure (~−3.19/rd) computed + * from real rounds. The label now says what the number actually is: a + * cross-player, cross-insight total for the window, never a rate. Stroke- + * impact itself is still the engine's own counterfactual magnitude (already * confidence- and sample-gated upstream); the board sums what the engine * surfaced, never invents a number. Decoupled shape — no server import. * ========================================================================== */ @@ -36,7 +42,8 @@ export interface LeakInsight { export interface LeakBoardProps { insights: LeakInsight[]; - /** str/rd at or above which a category is flagged a high bleed. */ + /** Summed total strokes (across every leak in the category) at or above + * which it's flagged a high bleed. NOT a per-round rate. */ flameThreshold?: number; className?: string; } @@ -109,7 +116,7 @@ export function LeakBoard({ insights, flameThreshold = 0.8, className }: LeakBoa

{insights.length} leak{insights.length !== 1 ? 's' : ''} {totalPlayers > 0 ? <> across {totalPlayers} player{totalPlayers !== 1 ? 's' : ''} : null} — - biggest stroke-bleed first. + total strokes lost this window, biggest bleed first.

@@ -119,9 +126,12 @@ export function LeakBoard({ insights, flameThreshold = 0.8, className }: LeakBoa
{r.label} + {/* "total" (never "str/rd") — this is a SUM across every leak + insight for every player in the category, not a per-round + rate. See the file header for why that distinction matters. */} −{r.total.toFixed(1)} - str/rd + total