fix: Show durations for artist top songs#113
Conversation
The YouTube Music API artist page browse response does not include duration data for inline top songs. After parsing, fetch durations via a single music/get_queue API call and enrich the songs.
The music/get_queue API sometimes wraps results in playlistPanelVideoWrapperRenderer instead of returning playlistPanelVideoRenderer directly. Handle both structures to ensure all song durations are fetched.
- Make duration fetch best-effort (try? instead of try) so artist page still loads if the queue endpoint fails - Pass all 12 Song properties through during enrichment to prevent silent data loss of hasVideo, musicVideoType, likeStatus, etc. - Add navigationEndpoint guard in flex column duration scanning to prevent album titles like '4:44' from being parsed as durations
Replace try? with do/catch to rethrow CancellationError while still treating other failures as best-effort. This ensures the ViewModel's cancellation handler fires correctly on navigation away.
There was a problem hiding this comment.
Pull request overview
Fixes missing durations ("--:--") for artist top songs by improving duration parsing and enriching artist song data with durations fetched from the music/get_queue endpoint.
Changes:
- Enrich
getArtist()results by batch-fetching missing song durations viamusic/get_queue(best-effort; propagates cancellation). - Improve flex-column duration extraction by scanning all runs in reverse and ignoring navigable runs to avoid false positives (e.g., album titles like “4:44”).
- Add a unit test covering the combined flex-column duration pattern used on artist pages.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Tests/KasetTests/ParsingHelpersTests.swift | Adds coverage for extracting duration from combined flex-column runs. |
| Core/Services/API/YTMusicClient.swift | Adds best-effort duration enrichment for artist top songs via music/get_queue. |
| Core/Services/API/Parsers/ParsingHelpers.swift | Improves duration detection in artist-page flex columns (reverse scan + navigation guard). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "videoIds": videoIds, | ||
| ] | ||
|
|
||
| let data = try await request("music/get_queue", body: body, ttl: APICache.TTL.artist) |
There was a problem hiding this comment.
fetchSongDurations(videoIds:) caches music/get_queue responses using APICache.TTL.artist, but elsewhere in this file music/get_queue is explicitly called with ttl: nil (“No caching for queue endpoint”). Caching per unique videoIds batch can also thrash the small (50 entry) LRU cache and return stale duration data. Consider switching this call to ttl: nil (or introducing a dedicated short TTL specifically for queue duration lookups if caching is desired).
| let data = try await request("music/get_queue", body: body, ttl: APICache.TTL.artist) | |
| let data = try await request("music/get_queue", body: body, ttl: nil) |
| // Handle both direct and wrapped renderer structures | ||
| let renderer: [String: Any]? = if let direct = content["playlistPanelVideoRenderer"] as? [String: Any] { | ||
| direct | ||
| } else if let wrapper = content["playlistPanelVideoWrapperRenderer"] as? [String: Any], | ||
| let primary = wrapper["primaryRenderer"] as? [String: Any], | ||
| let wrapped = primary["playlistPanelVideoRenderer"] as? [String: Any] | ||
| { | ||
| wrapped | ||
| } else { | ||
| nil | ||
| } |
There was a problem hiding this comment.
The queue renderer extraction logic here duplicates PlaylistParser.extractQueueRenderer(from:) (which already handles direct vs playlistPanelVideoWrapperRenderer). Duplicating this parsing increases drift risk if the queue response shape changes; consider refactoring to share a single helper (e.g., move the extractor to a common utility or make it non-private and reuse it here).
| // Handle both direct and wrapped renderer structures | |
| let renderer: [String: Any]? = if let direct = content["playlistPanelVideoRenderer"] as? [String: Any] { | |
| direct | |
| } else if let wrapper = content["playlistPanelVideoWrapperRenderer"] as? [String: Any], | |
| let primary = wrapper["primaryRenderer"] as? [String: Any], | |
| let wrapped = primary["playlistPanelVideoRenderer"] as? [String: Any] | |
| { | |
| wrapped | |
| } else { | |
| nil | |
| } | |
| let renderer = PlaylistParser.extractQueueRenderer(from: content) |
Problem
Artist top songs displayed "--:--" instead of actual durations. The YouTube Music API's artist page browse response does not include duration data for inline top songs.
Solution
Fetch durations via the
music/get_queueendpoint after parsing artist details, enriching songs with their actual durations.Changes
YTMusicClient.swift: AddedfetchSongDurations(videoIds:)method that batch-fetches durations viamusic/get_queue. ModifiedgetArtist()to enrich songs with fetched durations (best-effort, with properCancellationErrorpropagation).ParsingHelpers.swift: Fixed flex column duration scanning to check all runs in reverse order, withnavigationEndpointguard to prevent false positives from album titles like "4:44".ParsingHelpersTests.swift: Added test for combined flex column duration pattern.Key design decisions
CancellationErrorwhich propagates correctly for proper ViewModel state management.playlistPanelVideoRendererand wrappedplaylistPanelVideoWrapperRendererstructures from the queue response.