Song search: forgiving, multilingual guess matching - #12
Conversation
Typing 'one greater than one' now surfaces '>one - greater than one', and accented/other-language titles match too. Adds lib/search.js: - normalizeText: lowercase, fold Latin accents, drop punctuation/symbols, collapse whitespace — Unicode-aware. Decomposes → strips only the Latin accent range → recomposes, and preserves combining marks (category M), so Japanese dakuten (パ) and Devanagari matras (होने) survive instead of being mangled/gutted. - matchesQuery: normalized substring OR all-tokens-present surfacing. GamePage's guess dropdown builds its index and filters through these. Song correctness is unchanged (still by track id — the dropdown only surfaces candidates), so this is purely better discovery, not looser scoring. splitArtists/matchesArtist are included for the upcoming artist-mode work but not yet wired into correctness here. 14 unit tests (npm test). Frontend-only; no API contract change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/lib/search.js`:
- Around line 37-61: Update ARTIST_SEP so alphabetic collaboration tokens only
match as standalone words, using word-boundary or equivalent whitespace-aware
constraints while preserving punctuation separators and case-insensitive
matching. Keep splitArtists and matchesArtist behavior unchanged otherwise, and
add regression coverage for names such as “Daft Punk,” “Bill Withers,” and
“Xscape” so they remain single artists.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 449f392b-990e-47d7-8355-533477007ea4
📒 Files selected for processing (3)
frontend/src/lib/search.jsfrontend/src/lib/search.test.jsfrontend/src/pages/GamePage.js
| // Separators Spotify (and people) use between collaborating artists. | ||
| const ARTIST_SEP = /\s*(?:,|&|\/|\+|feat\.?|ft\.?|featuring|with|x|×)\s*/gi; | ||
|
|
||
| // Split a raw artist field into individual normalized artist names. | ||
| // "Drake, 21 Savage" → ["drake", "21 savage"]; "A feat. B" → ["a", "b"]. | ||
| export function splitArtists(rawArtist) { | ||
| if (!rawArtist) return []; | ||
| const seen = new Set(); | ||
| const out = []; | ||
| for (const part of rawArtist.split(ARTIST_SEP)) { | ||
| const n = normalizeText(part); | ||
| if (n && !seen.has(n)) { seen.add(n); out.push(n); } | ||
| } | ||
| return out; | ||
| } | ||
|
|
||
| // Artist correctness: the guess names ANY credited artist on the track. | ||
| export function matchesArtist(guess, trackArtist) { | ||
| const g = normalizeText(guess); | ||
| if (!g) return false; | ||
| const artists = splitArtists(trackArtist); | ||
| // exact per-artist match, plus the whole normalized field (covers a track | ||
| // credited as a single "A & B" act the player types in full). | ||
| return artists.includes(g) || normalizeText(trackArtist) === g; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ARTIST_SEP splits inside ordinary words — will corrupt real artist names once wired up.
None of the alphabetic separator alternatives (feat, ft, featuring, with, x) are word-boundary anchored, and \s* permits a zero-width match on either side. So these tokens are also matched as bare substrings inside unrelated words:
"Daft Punk"contains"ft"→ splits into["da", "punk"]"Bill Withers"contains"with"as a prefix → splits into["", "ers"]"Xscape"starts with"x"→ splits into["", "scape"]
This breaks splitArtists/matchesArtist for these (and similar) artists. The PR notes these helpers aren't wired into correctness yet, but the bug already violates the tested contract and will surface the moment this is connected. Also consider adding regression tests for these substring cases alongside the fix.
🐛 Proposed fix using word boundaries
-const ARTIST_SEP = /\s*(?:,|&|\/|\+|feat\.?|ft\.?|featuring|with|x|×)\s*/gi;
+// \b is placed before the optional trailing "." (rather than after) because
+// \b cannot match between two non-word characters (e.g. "." followed by a space).
+const ARTIST_SEP = /\s*(?:,|&|\/|\+|×|\bfeat\b\.?|\bft\b\.?|\bfeaturing\b|\bwith\b|\bx\b)\s*/gi;📝 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.
| // Separators Spotify (and people) use between collaborating artists. | |
| const ARTIST_SEP = /\s*(?:,|&|\/|\+|feat\.?|ft\.?|featuring|with|x|×)\s*/gi; | |
| // Split a raw artist field into individual normalized artist names. | |
| // "Drake, 21 Savage" → ["drake", "21 savage"]; "A feat. B" → ["a", "b"]. | |
| export function splitArtists(rawArtist) { | |
| if (!rawArtist) return []; | |
| const seen = new Set(); | |
| const out = []; | |
| for (const part of rawArtist.split(ARTIST_SEP)) { | |
| const n = normalizeText(part); | |
| if (n && !seen.has(n)) { seen.add(n); out.push(n); } | |
| } | |
| return out; | |
| } | |
| // Artist correctness: the guess names ANY credited artist on the track. | |
| export function matchesArtist(guess, trackArtist) { | |
| const g = normalizeText(guess); | |
| if (!g) return false; | |
| const artists = splitArtists(trackArtist); | |
| // exact per-artist match, plus the whole normalized field (covers a track | |
| // credited as a single "A & B" act the player types in full). | |
| return artists.includes(g) || normalizeText(trackArtist) === g; | |
| } | |
| // Separators Spotify (and people) use between collaborating artists. | |
| // \b is placed before the optional trailing "." (rather than after) because | |
| // \b cannot match between two non-word characters (e.g. "." followed by a space). | |
| const ARTIST_SEP = /\s*(?:,|&|\/|\+|×|\bfeat\b\.?|\bft\b\.?|\bfeaturing\b|\bwith\b|\bx\b)\s*/gi; | |
| // Split a raw artist field into individual normalized artist names. | |
| // "Drake, 21 Savage" → ["drake", "21 savage"]; "A feat. B" → ["a", "b"]. | |
| export function splitArtists(rawArtist) { | |
| if (!rawArtist) return []; | |
| const seen = new Set(); | |
| const out = []; | |
| for (const part of rawArtist.split(ARTIST_SEP)) { | |
| const n = normalizeText(part); | |
| if (n && !seen.has(n)) { seen.add(n); out.push(n); } | |
| } | |
| return out; | |
| } | |
| // Artist correctness: the guess names ANY credited artist on the track. | |
| export function matchesArtist(guess, trackArtist) { | |
| const g = normalizeText(guess); | |
| if (!g) return false; | |
| const artists = splitArtists(trackArtist); | |
| // exact per-artist match, plus the whole normalized field (covers a track | |
| // credited as a single "A & B" act the player types in full). | |
| return artists.includes(g) || normalizeText(trackArtist) === g; | |
| } |
🤖 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 `@frontend/src/lib/search.js` around lines 37 - 61, Update ARTIST_SEP so
alphabetic collaboration tokens only match as standalone words, using
word-boundary or equivalent whitespace-aware constraints while preserving
punctuation separators and case-insensitive matching. Keep splitArtists and
matchesArtist behavior unchanged otherwise, and add regression coverage for
names such as “Daft Punk,” “Bill Withers,” and “Xscape” so they remain single
artists.
feat/ft/with/x were matching as substrings inside artist names (Daft Punk, Bill Withers, Xscape), splitting them incorrectly.
271208b
into
codex/rollback-d5f02f1-working-state
Song search: forgiving, multilingual guess matching
Fixes your
>one - greater than oneexample and multilingual titles (Hindi, Spanish, Japanese, Korean).Problem
The guess dropdown matched raw lowercased strings, so special characters and accents broke discovery — typing
one greater than onenever surfaced>one - greater than one, and accented/non-latin titles were hard to find.Fix (frontend-only)
New
lib/search.js:>one→one), collapse whitespace. Unicode-aware: it decomposes, strips only the Latin accent range, then recomposes and keeps combining marks, so non-latin scripts survive intact rather than getting mangled (I caught NFKD turning Japanese パ→ハ and gutting Devanagari matras during testing — both fixed).The dropdown index and filter now run through these.
Not made too lenient
Song correctness is unchanged — still an exact track-id match (you pick the actual song). This only improves surfacing candidates in the dropdown, never scoring.
splitArtists/matchesArtistare included and unit-tested for the next PR (artist mode) but not wired into correctness here.Verified
14 Jest unit tests (
npm test) covering the>onecase, accent folding, Japanese/Korean/Hindi preservation, token-order surfacing, and artist split/match. Production build compiles clean. One file changed + one new lib + its test; zero backend/contract changes.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes