fix(coachhelm): coach-facing surfaces speak coach voice, not player voice

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,24 +99,36 @@ 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<string, string> = {};
+  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 (
     <div className={fairwayScope('min-h-full bg-canvas bg-canvas-gradient font-fw-sans text-text-primary')}>
       <FairwayCoachHelmSignals
         coachId={coach.id}
         teamId={teamId}
         signalSource="insights"
         initialInsights={initialInsights}
+        playerNames={playerNames}
         defaultFilter={{
           // The client filter compares against MAPPED row tones
           // (insight `urgent` → row `critical`; see patternToInsightVocabulary

src/app/golf/(dashboard)/dashboard/insights/page.tsx

@@ -1,6 +1,7 @@
 import { redirect } from 'next/navigation';
 import { getGolfSessionProfile } from '@/lib/auth/session';
 import { createClient } from '@/lib/supabase/server';
+import { getTeamPlayers } from '@/app/golf/actions/roster';
 import { fairwayScope } from '@/lib/redesign/flag';
 import { FairwayCoachHelmSignals, FeatureUnavailable } from '@/components/fairway';
 import { resolveCoachTeamIdWithCookie } from '@/lib/golf/resolve-team-server';
@@ -104,6 +105,14 @@ export default async function InsightsPage({ searchParams }: InsightsPageProps)
   // getAlertCounts read is kept off this path entirely — it was only ever
   // feeding the now-suppressed badge.
   const signalCount = null;
+  // Roster names for the "By player" grouping — insight rows don't carry a
+  // joined name of their own (see alerts/page.tsx for the full rationale).
+  const rosterRes = await getTeamPlayers();
+  const playerNames: Record<string, string> = {};
+  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 (
     <div className={fairwayScope('min-h-full bg-canvas bg-canvas-gradient font-fw-sans text-text-primary')}>
       <FairwayCoachHelmSignals
@@ -115,6 +124,7 @@ export default async function InsightsPage({ searchParams }: InsightsPageProps)
           smartDefault: 'new_and_critical_this_week',
           view: 'table',
         }}
+        playerNames={playerNames}
         signalCount={signalCount}
         initialSearchParams={params}
       />

src/components/fairway/cards-insight/InsightCard.test.tsx

@@ -0,0 +1,95 @@
+// @vitest-environment jsdom
+/**
+ * ============================================================================
+ * InsightCard.tsx — `iconTone` valence override (bug #915)
+ * ----------------------------------------------------------------------------
+ * A mined pattern's icon/accent was previously always derived from
+ * `priority` — a severity tier bucketed by |stroke_impact| MAGNITUDE, blind
+ * to sign. That let a high-magnitude plays-BETTER pattern land in the 'high'
+ * tier (a warning-orange flame, as if it were bad news) while a
+ * low-magnitude plays-WORSE pattern landed in 'medium' (a green sparkle, as
+ * if it were good news).
+ *
+ * These tests lock the fix: `iconTone` overrides BOTH the glyph and its
+ * wrap color independent of `priority`, so a caller with a genuinely signed
+ * metric can make icon+accent agree with "good or bad for the player"
+ * instead of "how large is this number".
+ * ========================================================================== */
+import { describe, it, expect } from 'vitest';
+import { render } from '@testing-library/react';
+import { InsightCard } from './InsightCard';
+
+/** The rendered lead icon's wrap <span> — carries the color classes. */
+function iconWrapClassName(container: HTMLElement): string {
+  const svg = container.querySelector('svg');
+  expect(svg).not.toBeNull();
+  const wrap = svg!.parentElement;
+  expect(wrap).not.toBeNull();
+  return wrap!.className;
+}
+
+describe('InsightCard — iconTone valence override', () => {
+  it('with no iconTone, the icon wrap follows priority (unchanged default behavior)', () => {
+    const { container } = render(
+      <InsightCard priority="critical" title="Critical signal" />,
+    );
+    const cls = iconWrapClassName(container);
+    expect(cls).toContain('text-fw-danger');
+    expect(cls).toContain('bg-fw-danger-bg');
+  });
+
+  it('iconTone="positive" renders the success wrap even on a "critical" priority card', () => {
+    // Regression lock: a HIGH-magnitude plays-better pattern (severity
+    // 'critical') must still read as good news, not an alarm.
+    const { container } = render(
+      <InsightCard priority="critical" iconTone="positive" title="Plays better under fatigue" />,
+    );
+    const cls = iconWrapClassName(container);
+    expect(cls).toContain('text-fw-success');
+    expect(cls).toContain('bg-fw-success-bg');
+    expect(cls).not.toContain('bg-fw-danger-bg');
+  });
+
+  it('iconTone="negative" renders the warning wrap even on a "medium"/"low" priority card', () => {
+    // Regression lock: a LOW-magnitude plays-worse pattern (severity
+    // 'medium'/'low') must still read as a leak, not a strength.
+    const { container: medium } = render(
+      <InsightCard priority="medium" iconTone="negative" title="Small leak" />,
+    );
+    expect(iconWrapClassName(medium)).toContain('bg-fw-warning-bg');
+    expect(iconWrapClassName(medium)).not.toContain('bg-fw-success-bg');
+
+    const { container: low } = render(
+      <InsightCard priority="low" iconTone="negative" title="Small leak" />,
+    );
+    expect(iconWrapClassName(low)).toContain('bg-fw-warning-bg');
+  });
+
+  it('iconTone="neutral" renders the neutral wrap regardless of priority', () => {
+    const { container } = render(
+      <InsightCard priority="high" iconTone="neutral" title="No clear direction" />,
+    );
+    const cls = iconWrapClassName(container);
+    expect(cls).toContain('bg-surface-sunken');
+    expect(cls).not.toContain('bg-fw-warning-bg');
+  });
+
+  it('two cards with the SAME priority tier but OPPOSITE iconTone render visibly different wraps', () => {
+    // The exact reported shape: same magnitude tier, opposite sign.
+    const { container: better } = render(
+      <InsightCard priority="medium" iconTone="positive" title="Better" />,
+    );
+    const { container: worse } = render(
+      <InsightCard priority="medium" iconTone="negative" title="Worse" />,
+    );
+    expect(iconWrapClassName(better)).not.toBe(iconWrapClassName(worse));
+  });
+
+  it('undefined iconTone is a true no-op — identical to omitting the prop entirely', () => {
+    const { container: omitted } = render(<InsightCard priority="low" title="A" />);
+    const { container: explicitUndefined } = render(
+      <InsightCard priority="low" iconTone={undefined} title="A" />,
+    );
+    expect(iconWrapClassName(omitted)).toBe(iconWrapClassName(explicitUndefined));
+  });
+});

src/components/fairway/cards-insight/InsightCard.tsx

@@ -44,6 +44,8 @@ import {
   Sparkles,
   Info,
   Lightbulb,
+  TrendingUp,
+  TrendingDown,
   type LucideIcon,
 } from 'lucide-react';
 import { cn } from '@/lib/utils';
@@ -93,6 +95,13 @@ export interface InsightCardProps
   children?: ReactNode;
   /** Lead icon override. Defaults to a priority-appropriate icon. */
   icon?: ReactNode;
+  /**
+   * Recolor the icon wrap (and, unless `icon` overrides the glyph too, the
+   * default icon) by DIRECTION rather than `priority`'s severity tier (bug
+   * #915 — see `ICON_TONE`'s docstring). The wrap color always follows
+   * `iconTone` when set, even alongside a custom `icon`.
+   */
+  iconTone?: InsightIconTone;
   /** Hide the lead icon entirely. */
   hideIcon?: boolean;
   /** Evidence / supporting detail rendered in a tinted Inset (not on compact). */
@@ -189,6 +198,34 @@ export const PRIORITY: Record<InsightPriority, PriorityTone> = {
   },
 };
 
+/* -- icon tone (bug #915 — icon/accent by DIRECTION, not category) --------- */
+
+/**
+ * `priority` is a severity/urgency tier — appropriate for most signals
+ * (a critical alert IS alarming regardless of "direction"). But a mined
+ * PATTERN's priority is bucketed by |stroke_impact| magnitude alone, blind to
+ * sign — which let a high-magnitude pattern where the player plays BETTER
+ * (a strength) land in the 'high' tier (warning-orange flame) while a
+ * low-magnitude one where they play WORSE (a leak) landed in 'medium' (green
+ * sparkle): the icon read backwards from what the pattern actually means.
+ *
+ * `iconTone`, when supplied, overrides BOTH the default icon AND its
+ * background/text color independent of `priority` — so a caller with a
+ * genuinely signed metric (a pattern's stroke_impact) can make the glyph +
+ * accent agree with "is this good or bad for the player" instead of "how
+ * large is this number". `priority` keeps driving the tint bar (severity/
+ * triage ordering is still a legitimate, separate signal) and the a11y word.
+ * Omitted (the default) → fully unchanged priority-driven rendering for
+ * every other InsightCard caller.
+ */
+export type InsightIconTone = 'positive' | 'negative' | 'neutral';
+
+export const ICON_TONE: Record<InsightIconTone, { iconWrap: string; icon: LucideIcon }> = {
+  positive: { iconWrap: 'text-fw-success bg-fw-success-bg', icon: TrendingUp },
+  negative: { iconWrap: 'text-fw-warning bg-fw-warning-bg', icon: TrendingDown },
+  neutral: { iconWrap: 'text-text-secondary bg-surface-sunken', icon: Info },
+};
+
 /* -- component -------------------------------------------------------------- */
 
 const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
@@ -200,6 +237,7 @@ const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
       title,
       children,
       icon,
+      iconTone,
       hideIcon = false,
       evidence,
       actions,
@@ -220,13 +258,18 @@ const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
   ) {
     const prefersReduced = useReducedMotion();
     const tone = PRIORITY[priority];
+    // Bug #915: an explicit iconTone overrides the icon glyph AND its
+    // background/text color independent of priority — see ICON_TONE's
+    // docstring. The tint bar below stays priority-driven either way.
+    const toneOverride = iconTone ? ICON_TONE[iconTone] : null;
+    const iconWrapClass = toneOverride?.iconWrap ?? tone.iconWrap;
     const titleId = useId();
     const isHero = variant === 'hero';
     const isCompact = variant === 'compact';
 
     useHeroGlassFallback(isHero);
 
-    const LeadIcon = tone.icon;
+    const LeadIcon = toneOverride?.icon ?? tone.icon;
     const leadIcon =
       icon ?? (hideIcon ? null : <LeadIcon aria-hidden className="h-full w-full" strokeWidth={1.5} />);
 
@@ -373,7 +416,7 @@ const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
           <span
             className={cn(
               'pointer-events-none relative z-10 flex shrink-0 items-center justify-center rounded-fw-md',
-              tone.iconWrap,
+              iconWrapClass,
               isCompact ? 'h-8 w-8 p-1.5' : 'h-10 w-10 p-2',
             )}
           >

src/components/fairway/cards-insight/InsightPanel.tsx

@@ -54,7 +54,10 @@ import { Button, IconButton } from '../controls/button';
 import { InsufficientData } from '../feedback/InsufficientData';
 // SHARED vocabulary — the single source of truth for priority tone, imported
 // (never re-declared) so a signal looks identical scanned (card) vs read (panel).
-import { PRIORITY, type InsightPriority } from './InsightCard';
+// ICON_TONE/InsightIconTone is the bug #915 direction-by-sign override (see
+// InsightCard's docstring) — shared here for the same "must look identical
+// scanned vs read" reason.
+import { PRIORITY, ICON_TONE, type InsightPriority, type InsightIconTone } from './InsightCard';
 
 export type InsightPanelMode = 'auto' | 'sheet' | 'docked';
 
@@ -92,6 +95,13 @@ export interface InsightPanelProps {
   children?: ReactNode;
   /** Lead icon override. Defaults to the priority-appropriate icon (shared). */
   icon?: ReactNode;
+  /**
+   * Override the icon glyph + color by DIRECTION rather than `priority`'s
+   * severity tier — the SAME bug #915 override InsightCard exposes, kept in
+   * sync so a pattern's icon reads identically scanned vs read. Ignored when
+   * `icon` is also passed.
+   */
+  iconTone?: InsightIconTone;
   /** Hide the lead icon entirely. */
   hideIcon?: boolean;
   /** Right-aligned meta in the header (timestamp, source chip, confidence). */
@@ -179,6 +189,7 @@ interface PanelBodyProps
     | 'title'
     | 'children'
     | 'icon'
+    | 'iconTone'
     | 'hideIcon'
     | 'meta'
     | 'evidence'
@@ -201,6 +212,7 @@ function PanelBody({
   title,
   children,
   icon,
+  iconTone,
   hideIcon = false,
   meta,
   evidence,
@@ -215,7 +227,11 @@ function PanelBody({
   titleId,
 }: PanelBodyProps) {
   const tone = PRIORITY[priority];
-  const LeadIcon = tone.icon;
+  // Bug #915: the SAME direction-by-sign override InsightCard exposes — see
+  // its docstring. `priority` still drives the tint bar below.
+  const toneOverride = iconTone ? ICON_TONE[iconTone] : null;
+  const iconWrapClass = toneOverride?.iconWrap ?? tone.iconWrap;
+  const LeadIcon = toneOverride?.icon ?? tone.icon;
   const leadIcon =
     icon ?? (hideIcon ? null : <LeadIcon aria-hidden className="h-full w-full" strokeWidth={2} />);
 
@@ -233,7 +249,7 @@ function PanelBody({
             <span
               className={cn(
                 'flex h-11 w-11 shrink-0 items-center justify-center rounded-fw-md p-2.5',
-                tone.iconWrap,
+                iconWrapClass,
               )}
             >
               {leadIcon}
@@ -334,6 +350,7 @@ export const InsightPanel = forwardRef<HTMLDivElement, InsightPanelProps>(
       title,
       children,
       icon,
+      iconTone,
       hideIcon,
       meta,
       evidence,
@@ -368,6 +385,7 @@ export const InsightPanel = forwardRef<HTMLDivElement, InsightPanelProps>(
         overline={overline}
         title={title}
         icon={icon}
+        iconTone={iconTone}
         hideIcon={hideIcon}
         meta={meta}
         evidence={evidence}
@@ -420,6 +438,7 @@ export const InsightPanel = forwardRef<HTMLDivElement, InsightPanelProps>(
                 overline={overline}
                 title={title}
                 icon={icon}
+                iconTone={iconTone}
                 hideIcon={hideIcon}
                 meta={meta}
                 evidence={evidence}