Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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 +95 to +117
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