fix: normalize all-caps athlete names at render time#233
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR introduces a new ChangesAthlete Name Formatting Utility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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 SummaryThis PR adds a
Confidence Score: 5/5Pure render-time change with no data mutations; safe to merge. All changes are isolated to display formatting: a single pure helper function applied at render sites, no data files or indexing touched, and 13 passing unit tests covering the documented edge cases. The known limitation around multi-letter period-separated initials was already surfaced in a prior review thread. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["formatAthleteName(name)"] --> B["Split by /(\s+)/ capturing whitespace"]
B --> C{For each token}
C --> D["isAllCaps(token)?"]
D -->|No| E["Leave token unchanged"]
D -->|Yes| F["titleCaseToken(token)"]
F --> G["Split by /([-''])/ capturing separators"]
G --> H["titleCasePart(part) for each part"]
H --> I{Starts with Mc + lowercase?}
I -->|Yes| J["Restore: Mc + uppercase 3rd char + rest"]
I -->|No| K["lowercase all, uppercase first char"]
J --> L["Join parts back"]
K --> L
L --> M["Join tokens back"]
E --> M
M --> N["Return formatted name"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["formatAthleteName(name)"] --> B["Split by /(\s+)/ capturing whitespace"]
B --> C{For each token}
C --> D["isAllCaps(token)?"]
D -->|No| E["Leave token unchanged"]
D -->|Yes| F["titleCaseToken(token)"]
F --> G["Split by /([-''])/ capturing separators"]
G --> H["titleCasePart(part) for each part"]
H --> I{Starts with Mc + lowercase?}
I -->|Yes| J["Restore: Mc + uppercase 3rd char + rest"]
I -->|No| K["lowercase all, uppercase first char"]
J --> L["Join parts back"]
K --> L
L --> M["Join tokens back"]
E --> M
M --> N["Return formatted name"]
Reviews (2): 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}
Comment on lines
+21
to
+25
| 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.
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>
rootulp
force-pushed
the
fix/224-normalize-name-casing
branch
from
e39e76d to
1145ceb
Compare
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.
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