fix: normalize all-caps athlete names at render time#233
Conversation
Athlete names scraped from ironman.com arrive with inconsistent casing, so search results show "MULLER Nicolas" next to "Muller Alain". Add a pure formatAthleteName helper that title-cases only fully-uppercase tokens (handling hyphens, apostrophes, Mc prefixes, and accented letters) while leaving correctly mixed-case names like "McDonald" and "van der Berg" untouched, and apply it at every render site: global search results, command palette, athlete profile heading, result page heading, race page top-finisher tables, stats page, and the OG share image. No data files or index-build scripts are modified. Closes #224 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR introduces a new ChangesAthlete Name Formatting Utility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/app/stats/page.tsx (1)
1-18: ⚡ Quick winUse the shared time formatter here instead of the local copy.
app/src/lib/format.tsalready exportsformatTime, and this localformatSecondshas already drifted (seconds % 60here vsMath.round(seconds % 60)there). Reusing the shared helper keeps time formatting consistent across pages and avoids another copy to maintain.♻️ Proposed cleanup
import Link from "next/link"; import { getStatsPageData } from "`@/lib/data`"; import { getCountryFlagISO } from "`@/lib/flags`"; -import { formatAthleteName } from "`@/lib/format`"; +import { formatAthleteName, formatTime } from "`@/lib/format`"; @@ -function formatSeconds(seconds: number): string { - const h = Math.floor(seconds / 3600); - const m = Math.floor((seconds % 3600) / 60); - const s = seconds % 60; - if (h > 0) return `${h}:${String(m).padStart(2, "0")}:${String(s).padStart(2, "0")}`; - return `${m}:${String(s).padStart(2, "0")}`; -} - @@ - <BigNumber value={formatSeconds(agg.averageHalfFinishSeconds)} /> + <BigNumber value={formatTime(agg.averageHalfFinishSeconds)} /> @@ - <BigNumber value={formatSeconds(agg.averageFullFinishSeconds)} /> + <BigNumber value={formatTime(agg.averageFullFinishSeconds)} />🤖 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 `@app/src/app/stats/page.tsx` around lines 1 - 18, The local formatSeconds function in page.tsx should be removed and the shared formatter imported from app/src/lib/format.ts; replace all uses of formatSeconds with formatTime and add an import for formatTime from "`@/lib/format`", relying on formatTime's Math.round behavior to keep formatting consistent (update any references to function name formatSeconds to formatTime and delete the local formatSeconds implementation).
🤖 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.
Nitpick comments:
In `@app/src/app/stats/page.tsx`:
- Around line 1-18: The local formatSeconds function in page.tsx should be
removed and the shared formatter imported from app/src/lib/format.ts; replace
all uses of formatSeconds with formatTime and add an import for formatTime from
"`@/lib/format`", relying on formatTime's Math.round behavior to keep formatting
consistent (update any references to function name formatSeconds to formatTime
and delete the local formatSeconds implementation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: be87f6d8-923b-44ce-b841-d80f9015c485
📒 Files selected for processing (9)
app/src/app/athlete/[slug]/page.tsxapp/src/app/race/[slug]/page.tsxapp/src/app/race/[slug]/result/[id]/opengraph-image.tsxapp/src/app/race/[slug]/result/[id]/page.tsxapp/src/app/stats/page.tsxapp/src/components/CommandPalette.tsxapp/src/components/GlobalSearchBar.tsxapp/src/lib/__tests__/format.test.tsapp/src/lib/format.ts
Greptile SummaryIntroduces a pure
Confidence Score: 4/5Safe to merge; the change is purely cosmetic at render time and cannot corrupt data or break search matching. The helper is well-designed and the call sites are all straightforward substitutions. The one edge case is multi-letter period-separated initials (e.g. J.R.) which would be lowercased to J.r. — the test suite covers single-letter initials but not this combination. app/src/lib/format.ts and app/src/lib/tests/format.test.ts — the isAllCaps letter-count guard and test coverage for period-separated multi-initial tokens. Important Files Changed
Reviews (1): Last reviewed commit: "fix: normalize all-caps athlete names at..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function isAllCaps(token: string): boolean { | ||
| const letters = token.match(/\p{L}/gu); | ||
| if (!letters || letters.length < 2) return false; | ||
| return letters.every((c) => c !== c.toLowerCase() && c === c.toUpperCase()); | ||
| } |
There was a problem hiding this comment.
isAllCaps counts only Unicode letter code points, so a token like "J.R." has two uppercase letters and passes the >= 2 guard — titleCasePart then lowercases everything after the first character, turning it into "J.r.". The single-initial test works because "J." has only one letter (below the threshold). Any athlete stored as e.g. "SMITH J.R." would display as "Smith J.r.". A guard that treats tokens containing non-letter non-separator characters as non-transformable would cover this.
1 participant
Closes #224
Summary
Athlete names scraped from ironman.com have inconsistent casing, so search results showed "MULLER Nicolas" next to "Muller Alain", and the all-caps entries carried through to profile, result, and race pages.
This is a purely render-time fix (no data files, index scripts, or search/matching logic touched, to avoid conflicts with #218):
formatAthleteNameinapp/src/lib/format.ts:GlobalSearchBarsearch results (display only)CommandPalettesearch results/athlete/[slug])/race/[slug]/result/[id]) and its OG share image/race/[slug])Test notes
app/src/lib/__tests__/format.test.ts(13 cases: all-caps surname, already-correct names unchanged, hyphen/apostrophe/Mc/Mac, accented caps, initials, whitespace preservation) — written first and confirmed failing, then implementednpx vitest run: 10 files, 98 tests, all passingnpm run lint: clean🤖 Generated with Claude Code
Summary by CodeRabbit