fix(coachhelm): coach-facing surfaces speak coach voice, not player voice - #931
Closed
njrini99-code wants to merge 3 commits into
Closed
fix(coachhelm): coach-facing surfaces speak coach voice, not player voice#931njrini99-code wants to merge 3 commits into
njrini99-code wants to merge 3 commits into
Conversation
… math, accuracy gate
Four independently-verified bugs on the Signals/Effectiveness surfaces:
1. By-player grouping ("Unknown player"): EvidenceInsight never carried a
resolved player name (only golf_coach_insights.player_id), so every
insight/alert row hard-coded playerName: undefined and collapsed into one
"Unknown player" bucket. Patterns were unaffected (getTeamPatterns already
joins golf_players inline). insightToSignalRow/insightsToSignalRows now
accept an optional player_id -> name map; /alerts and /insights SSR-fetch
the team roster via getTeamPlayers() and pass it through.
2. KPI tile semantics: "Urgent + high" counted every loaded row regardless of
status while "Open" filtered by status, so /alerts could show
"Urgent + high 13 > Open 10" — a severity breakdown reading as MORE than
its own superset. Urgent+high is now a true subset of Open (matches the
shell badge's own getAlertCounts().counts.critical semantics). Tile labels
now say the sub-tab noun ("Open alerts"/"Open insights"/"Open patterns")
instead of a generic "Open signals", and "Loaded" (dev-speak) is renamed
"Showing".
3. LeakBoard math/label: the "Where the team is bleeding" total is a SUM
across every player and every leak insight in a category, but was labeled
"str/rd" (a per-round rate) — reading as if −26.4 meant 26 strokes lost
EVERY round, ~8x the real team SG-putting figure (~−3.19/rd). Relabeled to
an honest "total", and the rollup now reads from its own full team-wide
fetch (no priority filter) instead of the active sub-tab's own scoped
`insights` state, so the banner no longer silently re-totals per sub-tab.
4. Effectiveness accuracy headline: gated on GAUGE_MIN_RESOLVED (2) while the
calibration side panel's own copy says "needs 5 resolved predictions" —
the two thresholds could disagree, letting a near-empty sample render an
authoritative "100% ▲+8%". Now gated on the SAME 5-resolved threshold
(isAccuracyHeadlineLive), with "Calibrating" replacing "Awaiting
predictions". Also fixed: StrokesGainedTornado's y-scale keyed rows by
display label, so two rows sharing a label (e.g. one player with two
top-N patterns) collapsed onto the same band — bars and value text
rendered on top of each other ("+4.10+4.67"). Now keyed by row index.
And the "no trend yet" cell rendered a bare "—" with no visible label
(only a tooltip); now matches the icon idiom the other three trend states
already use, dimmed to read as "no data" rather than a genuine flat trend.
Gates: tsc --noEmit clean, eslint clean on changed files, full vitest suite
green (272 files / 2636 passed).
Part of #914 + Fixes #907
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…oice (#915) Player-POV components rendered on coach-facing surfaces, and several color/icon valences were inverted: 1. SG cards (StandingStrip, shared by Team Stats and the player's own stats view) always said "YOU -3.34 ... Below team average / Bottom of your team" regardless of who was reading. Threaded a viewer_context/player_name pair through StandingBar's shared StandingBarProps -> StandingStrip -> FairwayTeamStats/ FairwayStatsCockpit/FairwayPlayerStats: a coach reader now sees the player's name/initials and possessive-free cohort text ("Bottom of team"); the player's own view is unchanged ("You" / "your team"). 2. Mined-pattern copy on the coach-only Patterns tab was player-first-person and grammatically malformed: "When After 5+ days off and In tournament, you tend to score 4.7 strokes worse than average. ... discuss with your coach." Fixed the template concatenation in PatternMiner.generateDescription (new joinConditionLabels helper) so condition labels compose into one natural clause instead of double-conjuncting "When X and Y", and added a toCoachVoice rewrite in the coach-only patternToInsightVocabulary.ts adapter that swaps "you tend to" for the player's name and "discuss with your coach" for "Worth a conversation with <name>." 3. Pattern card icon/accent was derived from `priority`, a severity tier bucketed by |stroke_impact| magnitude alone (blind to sign) - a high-magnitude plays-BETTER pattern could land in the 'high' tier (warning-orange flame) while a low-magnitude plays-WORSE pattern landed in 'medium' (green sparkle). Added a signed `valence` field to SignalRow and an `iconTone` override to InsightCard/InsightPanel that recolors the icon by direction, independent of the severity tint bar. 4. The Score-by-round trend Ribbon (Team Stats / player stats cockpit) rendered an improving (falling) score as an amber "-7.0" decline because Ribbon never told Readout which raw direction was good for the metric. Added a `goodDirection` prop to Ribbon (reusing the shared classifyTrend classifier) and set it to "down" for the score series. Also fixed an inverted `direction` ternary on FairwayRoundDetail's score-to-par delta found during the same audit (an under-par round rendered the same false amber decline). Gates: tsc --noEmit clean; eslint clean on all changed files; 636 vitest tests passing (0 failing) across the new/updated suites plus adjacent regression suites (FairwayTeamStats, surface-registry, CoachInsightCard). Fixes #915 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 (11)
WalkthroughThe PR fixes Bug ChangesCoach Helm semantics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PatternMiner
participant SignalVocabulary
participant CoachHelmSignals
participant InsightCard
participant InsightPanel
PatternMiner->>PatternMiner: joinConditionLabels(conditions)
PatternMiner->>SignalVocabulary: generated pattern text
SignalVocabulary->>SignalVocabulary: derivePatternValence(strokeImpact)
SignalVocabulary->>CoachHelmSignals: coach-voiced SignalRow
CoachHelmSignals->>InsightCard: row.valence as iconTone
CoachHelmSignals->>InsightPanel: openRow.valence as iconTone
sequenceDiagram
participant FairwayPlayerStats
participant FairwayStatsCockpit
participant StandingStrip
participant StandingBarUtils
FairwayPlayerStats->>FairwayStatsCockpit: playerName and isOwnStats
FairwayStatsCockpit->>StandingStrip: viewer_context and player_name
StandingStrip->>StandingBarUtils: derive subject and neutralize cohort text
StandingBarUtils-->>StandingStrip: coach-aware labels and aria text
Possibly related issues
Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (11 passed)
✨ 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 |
![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/cards-insight/InsightCard.test.tsx`:
- Around line 18-20: Add a local framer-motion mock in InsightCard.test.tsx, or
extend the shared setup mock, including useReducedMotion with a deterministic
return value so InsightCard assertions do not depend on animation behavior.
Preserve the existing mocks and test structure.
In `@src/components/fairway/charts/Ribbon.test.tsx`:
- Around line 92-96: Remove the getDeltaDirection helper and its
document.querySelector/data-direction assertion from the Ribbon tests. Import or
reuse screen from `@testing-library/react`, then update each direction test to
assert the user-visible glyph rendered for that state: ▲, ▼, or ►, using
screen.getByText and preserving each test’s existing expectations.
In
`@src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts`:
- Around line 94-104: Use one metric-aware strokeImpact sign contract: update
the valence documentation and derivation logic in patternToInsightVocabulary.ts
so lower-is-better score_to_par inverts the raw impact sign, pass the outcome
metric from patternToSignalRow into valence derivation, and leave insight
priority-only behavior unchanged. In patternToInsightVocabulary.test.ts, assert
positive score deltas produce negative valence and negative deltas produce
positive valence; rename the relevant pattern-miner test in
pattern-miner.test.ts to state that negative score impact means “better.”
In `@src/test/coachhelm/v2/mining/pattern-miner.test.ts`:
- Around line 248-251: Correct the test name in the `it` block to state that a
negative `stroke_impact` reads “better,” not “worse,” matching the `-1.8`
argument passed to `generateDescription`. Leave the test implementation and
expected description unchanged.
🪄 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: 060a891b-37ba-4d54-b733-38aad79aa008
📒 Files selected for processing (19)
src/components/fairway/cards-insight/InsightCard.test.tsxsrc/components/fairway/cards-insight/InsightCard.tsxsrc/components/fairway/cards-insight/InsightPanel.tsxsrc/components/fairway/charts/Ribbon.test.tsxsrc/components/fairway/charts/Ribbon.tsxsrc/components/fairway/charts/StandingStrip.tsxsrc/components/fairway/pages/coachhelm/FairwayCoachHelmSignals.tsxsrc/components/fairway/pages/coachhelm/FairwayPlayerStats.tsxsrc/components/fairway/pages/coachhelm/FairwayStatsCockpit.tsxsrc/components/fairway/pages/coachhelm/FairwayTeamStats.tsxsrc/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.test.tssrc/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.tssrc/components/fairway/pages/rounds/FairwayRoundDetail.tsxsrc/components/golf/coachhelm/v3/StandingBar/index.tsxsrc/components/golf/coachhelm/v3/StandingBar/types.tssrc/components/golf/coachhelm/v3/StandingBar/utils.tssrc/lib/coachhelm/v2/mining/pattern-miner.tssrc/test/coachhelm/v2/mining/pattern-miner.test.tssrc/test/golf/components/StandingBar.test.tsx
Comment on lines
+18
to
+20
| import { describe, it, expect } from 'vitest'; | ||
| import { render } from '@testing-library/react'; | ||
| import { InsightCard } from './InsightCard'; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i '.*(framer|motion|setup).*' src/test . |
xargs -r rg -n -C3 "useReducedMotion|vi\\.mock\\(['\"]framer-motion"Repository: njrini99-code/helmv3
Length of output: 15653
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== InsightCard.test.tsx ==\n'
sed -n '1,220p' src/components/fairway/cards-insight/InsightCard.test.tsx
printf '\n== Vitest setup candidates ==\n'
git ls-files | rg '(^|/)(vitest|vitest\.config|setup|test-setup|tests?/setup).*'
printf '\n== Motion/mock references in test setup files ==\n'
for f in $(git ls-files | rg '(^|/)(vitest|vitest\.config|setup|test-setup|tests?/setup).*'); do
echo "--- $f"
rg -n "framer-motion|useReducedMotion|vi\.mock|mock\('framer-motion'" "$f" || true
doneRepository: njrini99-code/helmv3
Length of output: 6408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== vitest.config.ts ==\n'
sed -n '1,220p' vitest.config.ts
printf '\n== src/test/setup.tsx ==\n'
sed -n '1,220p' src/test/setup.tsx
printf '\n== InsightCard component references ==\n'
rg -n "useReducedMotion|framer-motion|motion" src/components/fairway/cards-insight/InsightCard.tsx src/components/fairway/cards-insight -g '!*.test.tsx'Repository: njrini99-code/helmv3
Length of output: 14368
Add the missing framer-motion test mock
src/test/setup.tsx:12-67 mocks next/navigation, next/font/google, next/image, and @number-flow/react, but not framer-motion. InsightCard.test.tsx:18-20 needs a local vi.mock('framer-motion', ...) or a shared mock with useReducedMotion so these assertions stay deterministic.
🤖 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/cards-insight/InsightCard.test.tsx` around lines 18 -
20, Add a local framer-motion mock in InsightCard.test.tsx, or extend the shared
setup mock, including useReducedMotion with a deterministic return value so
InsightCard assertions do not depend on animation behavior. Preserve the
existing mocks and test structure.
Source: Path instructions
Comment on lines
+92
to
+96
| function getDeltaDirection(): string | null { | ||
| const delta = document.querySelector('[data-slot="readout-delta"]'); | ||
| expect(delta).not.toBeNull(); | ||
| return delta!.getAttribute('data-direction'); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Query user-visible state instead of DOM implementation details.
document.querySelector couples the test to an internal data attribute. As per path instructions, prefer @testing-library/react queries and assert on user-visible state.
Drop this helper and assert the presence of the semantic visual glyphs (▲, ▼, or ►) in each test instead.
♻️ Proposed pattern
import { screen } from '`@testing-library/react`';
// ... inside your test block
it('goodDirection="down": the SAME falling series now reads "up" (green)', () => {
render(<Ribbon title="Score by round" data={DECLINING} seriesName="Score" goodDirection="down" />);
// The 'up' direction visually renders a '▲' glyph
expect(screen.getByText(/▲/)).toBeInTheDocument();
});🤖 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/charts/Ribbon.test.tsx` around lines 92 - 96, Remove
the getDeltaDirection helper and its document.querySelector/data-direction
assertion from the Ribbon tests. Import or reuse screen from
`@testing-library/react`, then update each direction test to assert the
user-visible glyph rendered for that state: ▲, ▼, or ►, using screen.getByText
and preserving each test’s existing expectations.
Source: Path instructions
Comment on lines
+94
to
+104
| /** | ||
| * Bug #915 — the pattern icon/accent must derive from the SIGNED | ||
| * stroke_impact (a plays-better pattern is 'positive', plays-worse is | ||
| * 'negative'), not from the severity/priority tier alone. `priority` | ||
| * buckets by |impact| magnitude regardless of sign, which let a | ||
| * high-magnitude POSITIVE pattern land in the 'high' tier (warning-orange | ||
| * flame) and a low-magnitude NEGATIVE one land in 'medium' (green | ||
| * sparkle) — backwards. Set ONLY by `patternToSignalRow`; insights leave | ||
| * it undefined (their priority-only tone is unaffected by this bug). | ||
| */ | ||
| valence?: 'positive' | 'negative' | 'neutral'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use one metric-aware strokeImpact sign contract.
The miner stores score_to_par impact as condition average minus baseline, so positive means worse and negative means better. The adapter and its tests currently interpret that sign backwards.
src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts#L94-L104: document that valence depends on metric direction, not the raw sign alone.src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts#L181-L195: invert classification for lower-is-betterscore_to_par.src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts#L502-L502: pass the outcome metric into the valence derivation.src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.test.ts#L121-L155: assert positive score deltas are negative valence and negative deltas are positive valence.src/test/coachhelm/v2/mining/pattern-miner.test.ts#L248-L251: rename the test to state that negative score impact reads “better.”
📍 Affects 3 files
src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts#L94-L104(this comment)src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts#L181-L195src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.ts#L502-L502src/components/fairway/pages/coachhelm/signals/patternToInsightVocabulary.test.ts#L121-L155src/test/coachhelm/v2/mining/pattern-miner.test.ts#L248-L251
🤖 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/signals/patternToInsightVocabulary.ts`
around lines 94 - 104, Use one metric-aware strokeImpact sign contract: update
the valence documentation and derivation logic in patternToInsightVocabulary.ts
so lower-is-better score_to_par inverts the raw impact sign, pass the outcome
metric from patternToSignalRow into valence derivation, and leave insight
priority-only behavior unchanged. In patternToInsightVocabulary.test.ts, assert
positive score deltas produce negative valence and negative deltas produce
positive valence; rename the relevant pattern-miner test in
pattern-miner.test.ts to state that negative score impact means “better.”
Comment on lines
+248
to
+251
| it('a positive stroke_impact reads "better", never "worse"', () => { | ||
| const miner = new PatternMiner('player-1') as unknown as Miner; | ||
| const description = miner.generateDescription([AFTER_5_DAYS], {}, -1.8); | ||
| expect(description).toBe('After 5+ days off, you tend to score 1.8 strokes better than average.'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the sign stated in the test name.
Line 250 passes -1.8, so this verifies that a negative score delta reads “better,” not a positive one.
- it('a positive stroke_impact reads "better", never "worse"', () => {
+ it('a negative score_to_par stroke_impact reads "better", never "worse"', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('a positive stroke_impact reads "better", never "worse"', () => { | |
| const miner = new PatternMiner('player-1') as unknown as Miner; | |
| const description = miner.generateDescription([AFTER_5_DAYS], {}, -1.8); | |
| expect(description).toBe('After 5+ days off, you tend to score 1.8 strokes better than average.'); | |
| it('a negative score_to_par stroke_impact reads "better", never "worse"', () => { | |
| const miner = new PatternMiner('player-1') as unknown as Miner; | |
| const description = miner.generateDescription([AFTER_5_DAYS], {}, -1.8); | |
| expect(description).toBe('After 5+ days off, you tend to score 1.8 strokes better than average.'); |
🤖 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/test/coachhelm/v2/mining/pattern-miner.test.ts` around lines 248 - 251,
Correct the test name in the `it` block to state that a negative `stroke_impact`
reads “better,” not “worse,” matching the `-1.8` argument passed to
`generateDescription`. Leave the test implementation and expected description
unchanged.
Owner
Author
|
🤖 Mission Control — PR summary What it changes: Coach-facing surfaces rendered player-POV copy and inverted color/icon valences (#915):
Risk / areas: golf CoachHelm shared stats components (Team Stats / Patterns / player stats). Watch: CI: ⏳ just opened (22:05Z) — checks still initializing (28 passing, several pending, 0 failing); mergeable state BLOCKED on required review. Awaiting review + CI completion. |
…s in patternToInsightVocabulary.test.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
Owner
Author
This was referenced Jul 18, 2026
This was referenced Jul 18, 2026
This was referenced Aug 2, 2026
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
1 participant
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.
Problem
Player-POV components render on coach-facing surfaces, and several color/icon valences are inverted (bug #915). Verified live:
StandingStrip, shared with the player's own stats view viaFairwayStatsCockpit) always render "YOU −3.34 … Below team average / Bottom of your team" regardless of who is reading — a coach viewing a teammate's card is never "you" to that player.priority, bucketed by|stroke_impact|magnitude) instead of direction — a high-magnitude plays-better pattern could land in the'high'tier (warning-orange flame) while a low-magnitude plays-worse pattern landed in'medium'(green sparkle).Readoutwhich raw direction is good for the plotted metric.Fix
viewer_context: 'self' | 'coach'+player_nameto the sharedStandingBarProps.neutralizeForCoach/initialsFromName/standingSubjectLabel(new pure helpers inutils.ts) swap "You" → the player's initials and strip the "your team" possessive for a coach reader. Threaded throughFairwayTeamStats(always coach),FairwayStatsCockpit(derives it from the existingisOwnStatsflag — no new flag for callers to keep in sync), andFairwayPlayerStats(passes the already-resolved teammate name down). The player's own view is byte-for-byte unchanged.joinConditionLabelshelper fixes the "When X and Y" double-conjunction (condition labels already carry their own leading connector) so"After 5+ days off in tournament rounds, …"reads naturally instead of"When After 5+ days off and In tournament, …"./dashboard/patternsguardsif (!coach)) — newtoCoachVoicerewrites the mined pattern's "you tend to…" into third person with the player's name, and swaps the generic "discuss with your coach" recommendation for "Worth a conversation with<name>." Team-authored patterns (already third-person) pass through as a no-op.iconTone: 'positive' | 'negative' | 'neutral'prop overrides the icon glyph + wrap color independently ofpriority(which keeps driving the tint bar/severity ordering — a legitimate, separate signal). NewSignalRow.valence, derived from the signedstroke_impact, is wired into both the scanned card and the expand-in-place panel so a pattern reads identically either way.goodDirectionprop (reuses the sharedclassifyTrendclassifier already used bySparkline/StatTile/TrendChip) computes the trend delta's verdict instead of leavingReadoutto infer it from raw sign. WiredgoodDirection="down"on the "Score by round" Ribbon. Also fixed an inverteddirectionternary onFairwayRoundDetail's score-to-par delta found during the same audit (an under-par round was rendering the same false amber decline).Gates
npx tsc --noEmit -p tsconfig.json— cleannpx eslint <19 changed files>— cleannpx vitest runon all new/updated suites + adjacent regression suites (FairwayTeamStats.test.ts,surface-registry.test.ts,CoachInsightCard.test.tsx) — 636 passing, 0 failing (20 pre-existing skips, unrelated)Tests added
StandingBar.test.tsx—neutralizeForCoach,initialsFromName,standingSubjectLabel, coach-contextderiveAriaLabelpattern-miner.test.ts—joinConditionLabels(including the exact reported compound-pattern case) + end-to-endPatternMiner.generateDescriptionpatternToInsightVocabulary.test.ts(new) —toCoachVoice+patternToSignalRowtitle/body/valence, including a same-priority-tier/opposite-valence regression lockInsightCard.test.tsx(new) —iconToneoverriding the icon wrap color independent ofpriorityRibbon.test.tsx—goodDirection="down"flipping the trend verdict on the same falling seriesCaveats
StandingBar/StandingStrip,InsightCard/InsightPanel,Ribbon) fanning out to their real call sites plus co-located tests; I kept each touch as small as possible rather than batching fixes into fewer, larger files.team-pattern-generator.ts(already correctly third-person) or the broadercoachhelm/v2/mining/*generators that also use first-person "you"/"your" phrasing outside the two reported templates — out of scope for this issue; flagging in case a follow-up wants a wider audience-voice sweep.FairwayRoundDetail.tsx's inverted-delta fix is a bonus finding from auditing "other metrics rendered by the same component" (Readout), not one of the four originally reported items — no dedicated test added for it (no existing test scaffold for that component); covered by the samedirection-semantics reasoning locked down inRibbon.test.tsx.Fixes #915
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg