Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/src/main/res/values/metrolist_strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
<string name="sync_playlist">Sync playlist</string>
<string name="sync_disabled">Sync disabled</string>
<string name="allows_for_sync_witch_youtube">NOTE: This option enables syncing with YouTube Music and CANNOT be changed later</string>

<!-- Artist conjunction patterns for internationalized parsing -->
<string name="conjunction_and">and</string>
<string name="generating_image">Generating image…</string>
<string name="please_wait">Please wait</string>
<string name="cancel">Cancel</string>
Expand Down
24 changes: 24 additions & 0 deletions innertube/src/main/kotlin/com/metrolist/innertube/models/Runs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,30 @@ fun List<Run>.splitBySeparator(): List<List<Run>> {
return res
}

fun List<Run>.splitArtistsByConjunction(): List<Run> {
val result = mutableListOf<Run>()
val pattern = ArtistConjunctions.conjunctions.joinToString("|") { Regex.escape(it) }
val conjunctionPattern = Regex(" $pattern |, | & ")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

forEach { run ->
val text = run.text
if (text.contains(conjunctionPattern)) {
val parts = text.split(conjunctionPattern)
parts.forEachIndexed { index, part ->
if (part.isNotBlank()) {
result.add(Run(part.trim(), if (index == 0) run.navigationEndpoint else null))
}
}
} else {
result.add(run)
}
}
return result
}

object ArtistConjunctions {
var conjunctions: List<String> = listOf("and")
}

fun List<List<Run>>.clean(): List<List<Run>> =
if (getOrNull(0)?.getOrNull(0)?.navigationEndpoint != null ||
(getOrNull(0)?.getOrNull(0)?.text?.contains(regex = Regex("[&,]"))) != false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.metrolist.innertube.models.PlaylistItem
import com.metrolist.innertube.models.SongItem
import com.metrolist.innertube.models.YTItem
import com.metrolist.innertube.models.oddElements
import com.metrolist.innertube.models.splitArtistsByConjunction
import com.metrolist.innertube.models.splitBySeparator
import com.metrolist.innertube.utils.parseTime

Expand All @@ -20,20 +21,21 @@ data class ArtistItemsPage(
companion object {
fun fromMusicResponsiveListItemRenderer(renderer: MusicResponsiveListItemRenderer): SongItem? {
// Split the secondary line by bullet separator to separate artists from other metadata (like views)
val secondaryLineRuns = renderer.flexColumns
val artistRuns = renderer.flexColumns
.getOrNull(1)
?.musicResponsiveListItemFlexColumnRenderer
?.text
?.runs
?.splitBySeparator()

// Extract artists from the first segment after splitting
val artists = secondaryLineRuns?.firstOrNull()?.oddElements()?.map {
Artist(
name = it.text,
id = it.navigationEndpoint?.browseEndpoint?.browseId
)
}
?.getOrNull(0)
?.splitArtistsByConjunction()
?.filter { it.text.isNotBlank() && it.text != "&" && it.text != "," }
?.map { run ->
Artist(
name = run.text.trim(),
id = run.navigationEndpoint?.browseEndpoint?.browseId
)
}

// Extract album from last flexColumn (like SimpMusic does)
val album = renderer.flexColumns.lastOrNull()
Expand All @@ -55,7 +57,7 @@ data class ArtistItemsPage(
title = renderer.flexColumns.firstOrNull()
?.musicResponsiveListItemFlexColumnRenderer?.text
?.runs?.firstOrNull()?.text ?: return null,
artists = artists ?: return null,
artists = artistRuns ?: return null,
album = album,
duration = renderer.fixedColumns?.firstOrNull()
?.musicResponsiveListItemFlexColumnRenderer?.text
Expand Down Expand Up @@ -89,21 +91,37 @@ data class ArtistItemsPage(
} != null
)
// Video
renderer.isSong -> SongItem(
id = renderer.navigationEndpoint.watchEndpoint?.videoId ?: return null,
title = renderer.title.runs?.firstOrNull()?.text ?: return null,
artists = renderer.subtitle?.runs?.splitBySeparator()?.firstOrNull()?.oddElements()?.map {
Artist(
name = it.text,
id = it.navigationEndpoint?.browseEndpoint?.browseId
)
} ?: return null,
album = null,
duration = null,
musicVideoType = renderer.musicVideoType,
thumbnail = renderer.thumbnailRenderer.musicThumbnailRenderer?.getThumbnailUrl() ?: return null,
endpoint = renderer.navigationEndpoint.watchEndpoint
)
renderer.isSong -> {
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()
},
Comment on lines +94 to +116
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

album = null,
duration = null,
musicVideoType = renderer.musicVideoType,
thumbnail = renderer.thumbnailRenderer.musicThumbnailRenderer?.getThumbnailUrl() ?: return null,
endpoint = renderer.navigationEndpoint.watchEndpoint
)
}
renderer.isPlaylist -> PlaylistItem(
id = renderer.navigationEndpoint.browseEndpoint?.browseId?.removePrefix("VL") ?: return null,
title = renderer.title.runs?.firstOrNull()?.text ?: return null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.metrolist.innertube.models.YTItem
import com.metrolist.innertube.models.filterExplicit
import com.metrolist.innertube.models.getItems
import com.metrolist.innertube.models.oddElements
import com.metrolist.innertube.models.splitArtistsByConjunction
import com.metrolist.innertube.models.splitBySeparator

data class ArtistSection(
Expand Down Expand Up @@ -71,23 +72,22 @@ data class ArtistPage(
}

private fun fromMusicResponsiveListItemRenderer(renderer: MusicResponsiveListItemRenderer): SongItem? {
// Split the secondary line by bullet separator to separate artists from other metadata (like views)
val secondaryLineRuns = renderer.flexColumns
val artistRuns = renderer.flexColumns
.getOrNull(1)
?.musicResponsiveListItemFlexColumnRenderer
?.text
?.runs
?.splitBySeparator()
?.getOrNull(0)
?.splitArtistsByConjunction()
?.filter { it.text.isNotBlank() && it.text != "&" && it.text != "," }
?.map { run ->
Artist(
name = run.text.trim(),
id = run.navigationEndpoint?.browseEndpoint?.browseId
)
}

// Extract artists from the first segment after splitting
val artists = secondaryLineRuns?.firstOrNull()?.oddElements()?.map {
Artist(
name = it.text,
id = it.navigationEndpoint?.browseEndpoint?.browseId
)
}

// Extract album from last flexColumn (like SimpMusic)
val album = renderer.flexColumns.lastOrNull()
?.musicResponsiveListItemFlexColumnRenderer?.text?.runs
?.firstOrNull()?.let {
Expand All @@ -99,15 +99,14 @@ data class ArtistPage(
} else null
}

// Extract library tokens using the new method that properly handles multiple toggle items
val libraryTokens = PageHelper.extractLibraryTokensFromMenuItems(renderer.menu?.menuRenderer?.items)

return SongItem(
id = renderer.playlistItemData?.videoId ?: return null,
title = renderer.flexColumns.firstOrNull()
?.musicResponsiveListItemFlexColumnRenderer?.text?.runs?.firstOrNull()
?.text ?: return null,
artists = artists ?: return null,
artists = artistRuns ?: return null,
album = album,
duration = null,
musicVideoType = renderer.musicVideoType,
Expand All @@ -125,21 +124,27 @@ data class ArtistPage(
private fun fromMusicTwoRowItemRenderer(renderer: MusicTwoRowItemRenderer): YTItem? {
return when {
renderer.isSong -> {
val subtitleRuns = renderer.subtitle?.runs?.oddElements() ?: return null
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 = subtitleRuns.filter {
artists = artistRuns.filter {
it.navigationEndpoint?.browseEndpoint?.browseId?.startsWith("UC") == true ||
it.navigationEndpoint?.browseEndpoint != null
}.map {
Artist(
name = it.text,
name = it.text.trim(),
id = it.navigationEndpoint?.browseEndpoint?.browseId
)
}.ifEmpty {
subtitleRuns.firstOrNull()?.let {
listOf(Artist(name = it.text, id = null))
artistRuns.firstOrNull()?.let {
listOf(Artist(name = it.text.trim(), id = null))
} ?: emptyList()
},
album = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.metrolist.innertube.models.SongItem
import com.metrolist.innertube.models.YTItem
import com.metrolist.innertube.models.clean
import com.metrolist.innertube.models.oddElements
import com.metrolist.innertube.models.splitArtistsByConjunction
import com.metrolist.innertube.models.splitBySeparator
import com.metrolist.innertube.utils.parseTime

Expand Down Expand Up @@ -159,6 +160,11 @@ object SearchSuggestionPage {
?.runs
?.splitBySeparator()
?.clean()
// Split artist runs by conjunction to handle "Artist1 & Artist2" cases
val artistRuns = secondaryLine
?.firstOrNull()
?.splitArtistsByConjunction()
?.filter { it.text.isNotBlank() && it.text != "&" && it.text != "," }
SongItem(
id = renderer.playlistItemData?.videoId ?: return null,
title =
Expand All @@ -170,15 +176,12 @@ object SearchSuggestionPage {
?.firstOrNull()
?.text ?: return null,
artists =
secondaryLine
?.firstOrNull()
?.oddElements()
?.map {
Artist(
name = it.text,
id = it.navigationEndpoint?.browseEndpoint?.browseId,
)
} ?: return null,
artistRuns?.map { run ->
Artist(
name = run.text.trim(),
id = run.navigationEndpoint?.browseEndpoint?.browseId,
)
} ?: return null,
album =
renderer.flexColumns
.getOrNull(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import com.metrolist.innertube.models.filterExplicit
import com.metrolist.innertube.models.filterVideoSongs
import com.metrolist.innertube.models.filterYoutubeShorts
import com.metrolist.innertube.models.oddElements
import com.metrolist.innertube.models.splitArtistsByConjunction
import com.metrolist.innertube.models.splitBySeparator
import com.metrolist.innertube.utils.parseTime

Expand Down Expand Up @@ -324,6 +325,10 @@ data class SearchSummaryPage(
?: emptyList()
val listRun = (secondaryLine + thirdLine).clean()

// Split artist runs by conjunction to handle "Artist1 & Artist2" cases
val artistRuns = listRun.getOrNull(0)?.splitArtistsByConjunction()
?.filter { it.text.isNotBlank() && it.text != "&" && it.text != "," }

SongItem(
id = renderer.playlistItemData?.videoId ?: return null,
title =
Expand All @@ -334,10 +339,10 @@ data class SearchSummaryPage(
?.runs
?.firstOrNull()
?.text ?: return null,
artists = listRun.getOrNull(0)?.oddElements()?.map {
artists = artistRuns?.map { run ->
Artist(
name = it.text,
id = it.navigationEndpoint?.browseEndpoint?.browseId
name = run.text.trim(),
id = run.navigationEndpoint?.browseEndpoint?.browseId
)
} ?: return null,
album = listRun.getOrNull(1)?.firstOrNull()?.takeIf { it.navigationEndpoint?.browseEndpoint != null }?.let {
Expand Down
Loading