Audit Wave 3 — AI content contract (5 fixes + data cleanup) - #963
Merged
Conversation
…s-player identical Signals cards (audit W3)
Two content-dedup defects at the assembly/render layer, no generator or
prose changes:
(a) On the Team Brief, the SAME category insight sentence could render
TWICE on one page: once in the WORK-ON-THIS-FIRST hero (as the
headline support text for a non-yardage-band category, or the foot
strip's honest fallback when no player is individually flagged), and
again in that same category's own Category Detail row below, which
rendered `cat.insights[0]` unconditionally. `heroAlreadyShowsLeadInsight`
+ `selectCategoryRowInsight` (FairwayBrief.tsx) detect the on-page
collision and fall through to the category's next REAL insight
(already computed by generateCategoryInsights/assembleBriefEngineInsights)
instead of repeating it verbatim — or omit, never fabricate, when
there is no second insight.
(b) On the Signals surfaces, the flat/ungrouped feed (the smart-default
cross-player triage view) renders insight/pattern cards with no
player-name header at all. Two players who genuinely cross the same
generated-content threshold (the generator narrates the condition,
not the individual — the deferred engine-side half of #944, not
touched here) got visually indistinguishable cards. `overline` in
patternToInsightVocabulary.ts now leads with the resolved roster
name when available, falling back to the original category-only
label — never a fabricated name — so identical content still reads
honestly as two separate players' cards instead of one collapsed
duplicate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…in same sentence (audit W3)
toCoachVoice's INSIGHT_VOICE_REWRITES rewrote each known player-voiced
clause independently, so a multi-clause coach card mixed person mid-
breath ("Jackson Hale escapes the bunker fine" immediately followed by
"88% of YOUR sand shots reached the green") and repeated the player's
name every time a rule fired ("Jackson Hale is only making 47%" ...
"costing Jackson Hale a stroke").
Extend the rewrite table (scrambling.ts's two bunker branches +
lag-distance-3putt.ts's composite) to close the residual "your"/"you"
clauses that had no rule at all, and thread a `mentioned` flag through
every rule so only the FIRST player reference in a call uses the real
name — every later reference in the same text reads as "they"/"them"/
"their" instead of repeating the name or staying second-person.
No prose is invented and no data changes: same literal known-engine
phrases, same fallback to "the player" when no name is available.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…tat strip (audit W3)
The SG tab's "Biggest gain"/"Biggest leak" callout (FairwayStatsCockpit) picked
its two RuledLeaderStat slots by RANK alone (max/min across the 4 SG
categories) and hard-coded "gain"/green-leader onto the max slot and
"leak"/plain onto the min slot regardless of the value's actual sign. A
player below the Tour baseline in every category still saw "BIGGEST GAIN
−0.10 · Putting" rendered in green — a stroke cost dressed up as a strength.
Symmetrically, a player ahead in every category saw their smallest genuine
gain mislabeled "Biggest leak".
Root-caused to the inline best/worst reducer never checking sign. Extracted
it into `computeGainLeak`, which still picks by rank but now also classifies
each slot against the real zero point (bestIsGain / worstIsLeak), so the
label text, the green `leader` treatment, and the SgVerdict prose ("Strongest
in" / "leaking most in") all track the same sign the numeral already shows
via `formatSg`. Same bug class as #944 (insightEvidenceLines), applied here
to the stat-strip rendering path instead of the insight-evidence path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
… stamp contract (audit W3)
Audit W3 reported the Brief's "updated" freshness stamp as dishonest: a
fabricated "8:00 PM" clock time from a date-only golf_rounds.round_date
parsed as UTC midnight, and stale-looking dates ("Jun 15"/"Jun 8") viewed
weeks later. Root-caused: getTeamCategoryInsights already derives
`lastAnalyzed` honestly (the most recent COMPLETED round actually feeding
the shown trends, '' when none exists in-window), and FairwayBrief's
`formatAnalyzed` already renders it as a UTC-anchored date-only label with
no fabricated time-of-day — both fixed upstream (#920/#925). A "stale"-
looking date is correctly TRUTHFUL when that's genuinely the team's last
completed round; the bug was only ever the fabricated-time formatting, which
is not reproducible on this branch.
That said, this exact composition was silently dropped once already during
cross-PR merge-conflict resolution (#925/#929, restored in 09f13c0) and
regressed to the fabricated-timestamp label in the interim — with no
render-layer test to catch it, only formatAnalyzed's isolated unit tests.
Extracted the PulseStrip meta-caption assembly (player count + optional
"need attention" + optional "updated <date>") into an exported pure
function, `pulseMetaLine`, so the "omit the freshness clause entirely when
it can't be honestly determined" contract is pinned by a direct test at the
composition layer that actually broke — zero behavior change, same rendered
string.
Checked src/components/fairway/pages/coachhelm/FairwayCoachHelmSignals.tsx
and its signals/ helpers (Toolbar, ScanTeamControl, patternToInsightVocabulary)
for an analogous "updated"/"as of" stamp: none exists — Signals renders no
freshness claim today, so there's nothing dishonest to omit or fix there.
Gates: eslint clean on both changed files; vitest — 16 files / 92 tests
passing (FairwayBrief.pulseMetaLine.test.ts [new], .formatAnalyzed.test.ts,
.heroSupportDetail.test.ts, .footStripLabel.test.ts, + the rest of the
coachhelm fairway-pages suite).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…ntheticals at display (audit W3) Data cleaned at source (golf_rounds + golf_courses, 0 remain); this is the render-layer backstop via cleanCourseName(), applied at the shared round row + detail masthead so a future stray suffix can't re-leak into the UI or prose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…up use Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
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. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
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 (12)
Summary by CodeRabbit
WalkthroughThe PR adds Fairway Brief content deduplication and PulseStrip formatting helpers, corrects strokes-gained gain/leak classification, refines coach-voice rewriting and signal identity, and normalizes course names in round views with regression tests. ChangesFairway Brief presentation
Strokes-gained classification
Coach voice and signal identity
Course-name normalization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
✨ 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 🔧 ESLint
src/components/fairway/pages/coachhelm/FairwayBrief.heroSupportDetail.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/components/fairway/pages/coachhelm/FairwayBrief.pulseMetaLine.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/components/fairway/pages/coachhelm/FairwayBrief.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
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.
Third fix wave. The engine stops shipping raw.
cleanCourseName()render-layer backstop applied at the shared round row + detail masthead so a future stray suffix can't re-leak into UI or generated prose.pulseMetaLine()and pinned the omit-rather-than-mislead contract with a regression test (it had been silently lost in a merge once).Gates
tsc --noEmitclean · eslint clean · full vitest: 2,509 files / 23,780 tests green. Note: one lane (the suffix strip) tripped a spurious content-filter false positive in the fleet, so I authored that fix directly.🤖 Generated with Claude Code