fix(artists): properly handle artist conjunctions in YouTube Music API responses#3695
Open
kairosci wants to merge 5 commits intoMetrolistGroup:mainfrom
Open
fix(artists): properly handle artist conjunctions in YouTube Music API responses#3695kairosci wants to merge 5 commits intoMetrolistGroup:mainfrom
kairosci wants to merge 5 commits intoMetrolistGroup:mainfrom
Conversation
…I responses - Add splitArtistsByConjunction() function to Runs.kt to handle & and , separators in artist names - Fix ArtistPage.kt to properly parse artists from MusicResponsiveListItemRenderer and MusicTwoRowItemRenderer - Fix ArtistItemsPage.kt to properly parse artists from both renderer types - Fix SearchSummaryPage.kt and SearchSuggestionPage.kt to handle artist conjunctions - This resolves issues where songs appeared under wrong artists, artists had conjunction symbols in their names, and single-song artist pages were created incorrectly The changes ensure that when YouTube Music returns "Artist1 & Artist2" or "Artist1, Artist2", each artist is properly extracted and associated with the song.
Add localized string for 'and' conjunction to support internationalized artist name parsing.
Revert to using only 'and' (via \band\b regex) for artist name parsing. The localized conjunction strings in metrolist_strings.xml will be used later for full internationalization.
…nctions - ArtistConjunctions object holds the list of conjunction words - splitArtistsByConjunction() now uses ArtistConjunctions.conjunctions - App module can now set ArtistConjunctions.conjunctions with localized strings from metrolist_strings.xml at startup
Contributor
📝 WalkthroughWalkthroughThis PR introduces artist name conjunction splitting across music metadata parsers. A new string resource ChangesArtist Conjunction Splitting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
innertube/src/main/kotlin/com/metrolist/innertube/pages/SearchSummaryPage.kt (1)
246-253:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
clean()can discard localized artist groups before the new splitter runs.
clean()ininnertube/src/main/kotlin/com/metrolist/innertube/models/Runs.ktonly keeps the first group when it has a browse endpoint or contains[&,]. For inputs likeArtist1 y Artist2/Artist1 e Artist2with no linked artist runs, the artist block is dropped on Lines 246-253/326 beforesplitArtistsByConjunction()ever sees it, so this path still won't parse the localized cases the PR is targeting.SearchSuggestionPage.fromMusicResponsiveListItemRendererhas the same sequence.Also applies to: 326-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@innertube/src/main/kotlin/com/metrolist/innertube/pages/SearchSummaryPage.kt` around lines 246 - 253, clean() in Runs.kt is currently dropping groups that lack a browse endpoint or an explicit "[&,]" token, which causes localized artist conjunctions (e.g., "Artist1 y Artist2" / "Artist1 e Artist2") to be removed before splitBySeparator()/splitArtistsByConjunction() run; update clean() to preserve groups that contain localized conjunction patterns or non-linked artist runs (or move the split logic earlier) so splitBySeparator() and splitArtistsByConjunction() can operate on those raw runs—specifically, modify the Runs.clean() logic to detect and retain runs containing conjunction tokens (or defer trimming) and ensure SearchSummaryPage (and SearchSuggestionPage.fromMusicResponsiveListItemRenderer) use the revised clean/split sequence so secondaryLine parsing keeps localized artist groups.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@innertube/src/main/kotlin/com/metrolist/innertube/models/Runs.kt`:
- Line 34: conjunctionPattern in Runs.kt currently treats any comma as an
unconditional separator; remove the plain ", " alternative from the Regex("
$pattern |, | & ") and instead handle commas with a narrower, explicit rule:
keep conjunctionPattern to match " $pattern " and " & ", and add a targeted
comma-splitting step where you only split on ", " when the token after the comma
does not start with an article/common prefix (e.g., "The", "A", "An") and/or
when there are multiple comma-separated items (indicating a list); update the
code paths that call conjunctionPattern (the artist-splitting logic) to use this
new conditional comma-split so names like "Tyler, The Creator" are not split.
In `@innertube/src/main/kotlin/com/metrolist/innertube/pages/ArtistItemsPage.kt`:
- Around line 95-117: The current artist extraction filters out split segments
that have navigationEndpoint == null, then the fallback collapses to a single
artist; change the filter so plain-text segments produced by
splitArtistsByConjunction() are preserved: in the artist selection logic (the
artistRuns variable and the subsequent filter/map block) allow runs that either
have a browseEndpoint (browseId startsWith "UC" or browseEndpoint != null) OR
have no navigationEndpoint (navigationEndpoint == null) so they are mapped to
Artist(name = it.text.trim(), id = null); apply the same change in
ArtistPage.fromMusicTwoRowItemRenderer to keep unlinked split artists instead of
collapsing them to the first name.
---
Outside diff comments:
In
`@innertube/src/main/kotlin/com/metrolist/innertube/pages/SearchSummaryPage.kt`:
- Around line 246-253: clean() in Runs.kt is currently dropping groups that lack
a browse endpoint or an explicit "[&,]" token, which causes localized artist
conjunctions (e.g., "Artist1 y Artist2" / "Artist1 e Artist2") to be removed
before splitBySeparator()/splitArtistsByConjunction() run; update clean() to
preserve groups that contain localized conjunction patterns or non-linked artist
runs (or move the split logic earlier) so splitBySeparator() and
splitArtistsByConjunction() can operate on those raw runs—specifically, modify
the Runs.clean() logic to detect and retain runs containing conjunction tokens
(or defer trimming) and ensure SearchSummaryPage (and
SearchSuggestionPage.fromMusicResponsiveListItemRenderer) use the revised
clean/split sequence so secondaryLine parsing keeps localized artist groups.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6abfe804-86d7-460d-8984-4ba3baa2ba4b
📒 Files selected for processing (6)
app/src/main/res/values/metrolist_strings.xmlinnertube/src/main/kotlin/com/metrolist/innertube/models/Runs.ktinnertube/src/main/kotlin/com/metrolist/innertube/pages/ArtistItemsPage.ktinnertube/src/main/kotlin/com/metrolist/innertube/pages/ArtistPage.ktinnertube/src/main/kotlin/com/metrolist/innertube/pages/SearchSuggestionPage.ktinnertube/src/main/kotlin/com/metrolist/innertube/pages/SearchSummaryPage.kt
| fun List<Run>.splitArtistsByConjunction(): List<Run> { | ||
| val result = mutableListOf<Run>() | ||
| val pattern = ArtistConjunctions.conjunctions.joinToString("|") { Regex.escape(it) } | ||
| val conjunctionPattern = Regex(" $pattern |, | & ") |
Contributor
There was a problem hiding this comment.
Don't treat commas as an unconditional artist separator.
Line 34 will split legitimate single-artist names like Tyler, The Creator into multiple artists, which is a worse misclassification than the one this PR is fixing. The shared helper needs a narrower heuristic for commas than , in the global regex.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@innertube/src/main/kotlin/com/metrolist/innertube/models/Runs.kt` at line 34,
conjunctionPattern in Runs.kt currently treats any comma as an unconditional
separator; remove the plain ", " alternative from the Regex(" $pattern |, | & ")
and instead handle commas with a narrower, explicit rule: keep
conjunctionPattern to match " $pattern " and " & ", and add a targeted
comma-splitting step where you only split on ", " when the token after the comma
does not start with an article/common prefix (e.g., "The", "A", "An") and/or
when there are multiple comma-separated items (indicating a list); update the
code paths that call conjunctionPattern (the artist-splitting logic) to use this
new conditional comma-split so names like "Tyler, The Creator" are not split.
Comment on lines
+95
to
+117
| val subtitleRuns = renderer.subtitle?.runs ?: return null | ||
| // Split any runs that contain conjunctions (&, ,) to properly extract individual artists | ||
| val expandedRuns = subtitleRuns.splitArtistsByConjunction() | ||
| // Filter out separator runs (like "&", ",") to get only artist runs | ||
| val artistRuns = expandedRuns.filter { | ||
| it.text.isNotBlank() && it.text != "&" && it.text != "," | ||
| } | ||
| SongItem( | ||
| id = renderer.navigationEndpoint.watchEndpoint?.videoId ?: return null, | ||
| title = renderer.title.runs?.firstOrNull()?.text ?: return null, | ||
| artists = artistRuns.filter { | ||
| it.navigationEndpoint?.browseEndpoint?.browseId?.startsWith("UC") == true || | ||
| it.navigationEndpoint?.browseEndpoint != null | ||
| }.map { | ||
| Artist( | ||
| name = it.text.trim(), | ||
| id = it.navigationEndpoint?.browseEndpoint?.browseId | ||
| ) | ||
| }.ifEmpty { | ||
| artistRuns.firstOrNull()?.let { | ||
| listOf(Artist(name = it.text.trim(), id = null)) | ||
| } ?: emptyList() | ||
| }, |
Contributor
There was a problem hiding this comment.
Two-row songs still collapse unlinked split artists back to the first name.
After splitArtistsByConjunction(), any segment created from a plain text run has navigationEndpoint == null. The filter on Lines 105-108 removes those artists, and the fallback on Lines 113-116 keeps only the first one, so Artist1 & Artist2 still ends up as a single artist in this path. ArtistPage.fromMusicTwoRowItemRenderer has the same copied logic.
Suggested direction
- val subtitleRuns = renderer.subtitle?.runs ?: return null
- // Split any runs that contain conjunctions (&, ,) to properly extract individual artists
- val expandedRuns = subtitleRuns.splitArtistsByConjunction()
- // Filter out separator runs (like "&", ",") to get only artist runs
- val artistRuns = expandedRuns.filter {
- it.text.isNotBlank() && it.text != "&" && it.text != ","
- }
+ val artistRuns = renderer.subtitle?.runs
+ ?.splitBySeparator()
+ ?.firstOrNull()
+ .orEmpty()
+ .splitArtistsByConjunction()
+ .filter { it.text.isNotBlank() }
SongItem(
id = renderer.navigationEndpoint.watchEndpoint?.videoId ?: return null,
title = renderer.title.runs?.firstOrNull()?.text ?: return null,
- artists = artistRuns.filter {
- it.navigationEndpoint?.browseEndpoint?.browseId?.startsWith("UC") == true ||
- it.navigationEndpoint?.browseEndpoint != null
- }.map {
+ artists = artistRuns.map {
Artist(
name = it.text.trim(),
id = it.navigationEndpoint?.browseEndpoint?.browseId
)
- }.ifEmpty {
- artistRuns.firstOrNull()?.let {
- listOf(Artist(name = it.text.trim(), id = null))
- } ?: emptyList()
- },
+ }.ifEmpty { return null },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@innertube/src/main/kotlin/com/metrolist/innertube/pages/ArtistItemsPage.kt`
around lines 95 - 117, The current artist extraction filters out split segments
that have navigationEndpoint == null, then the fallback collapses to a single
artist; change the filter so plain-text segments produced by
splitArtistsByConjunction() are preserved: in the artist selection logic (the
artistRuns variable and the subsequent filter/map block) allow runs that either
have a browseEndpoint (browseId startsWith "UC" or browseEndpoint != null) OR
have no navigationEndpoint (navigationEndpoint == null) so they are mapped to
Artist(name = it.text.trim(), id = null); apply the same change in
ArtistPage.fromMusicTwoRowItemRenderer to keep unlinked split artists instead of
collapsing them to the first name.
Contributor
Author
|
[WIP]: I HAVEN'T TEST YET. |
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
Songs appear under wrong artist pages because the YouTube Music API returns multiple artists joined by conjunctions (e.g., "Artist1 & Artist2", "Artist1, Artist2", "Artist1 y Artist2") as a single string. The app was treating these as a single artist.
Cause
The artist parsing logic in
ArtistPage.kt,ArtistItemsPage.kt,SearchSummaryPage.kt, andSearchSuggestionPage.ktdid not split artist names by conjunction patterns. When artists like "Duki & LIT killah" were returned as a single run, the app created one artist with the combined name.Solution
splitArtistsByConjunction()function inRuns.ktto split artist runs by conjunction patternsandstring to allmetrolist_strings.xmlfiles (60+ languages)ArtistConjunctions.conjunctionreads fromR.string.andat app startup viaApp.ktsplitArtistsByConjunction()to all relevant artist parsing locations:ArtistPage.kt-fromMusicResponsiveListItemRendererandfromMusicTwoRowItemRendererArtistItemsPage.kt- both renderer functionsSearchSummaryPage.kt-fromMusicResponsiveListItemRendererSearchSuggestionPage.kt-fromMusicResponsiveListItemRendererTesting
./gradlew :innertube:compileDebugKotlinRelated Issues
Summary by CodeRabbit
New Features
Bug Fixes