Add pagination support for liked songs, playlist tracks and search #22
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive pagination support for liked songs, playlist tracks, and filtered search results in the YouTube Music client. The implementation handles both legacy and new 2025 API response formats, introduces continuation token management, and provides infinite scroll UI for better user experience.
Key Changes:
- Implements pagination for liked songs (using
VLLMinstead of limitedFEmusic_liked_videos), playlist tracks, and filtered search results - Adds continuation token extraction logic supporting both legacy
continuationsarray format and 2025continuationItemRendererformat - Introduces queue API fallback for radio playlists (RDCLAK prefix) to bypass broken pagination
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/architecture.md | Documents new pagination endpoints and queue API support |
| docs/api-discovery.md | Updates API documentation with VLLM usage and queue endpoint details |
| Views/macOS/SearchView.swift | Adds load more UI with auto-trigger and progress indicator |
| Views/macOS/PlaylistDetailView.swift | Adds pagination trigger when scrolling near end of track list |
| Views/macOS/LikedMusicView.swift | Adds pagination trigger and loading indicator |
| Views/macOS/CommandBarView.swift | Updates to use new PlaylistTracksResponse structure |
| Tools/api-explorer.swift | Adds continuation exploration support and authentication helper |
| Tests/KasetTests/PlaylistParserTests.swift | Adds tests for 2025 continuation format parsing |
| Tests/KasetTests/Helpers/MockYTMusicClient.swift | Updates mock to support pagination APIs |
| Core/ViewModels/SearchViewModel.swift | Implements filtered search with pagination and auto-search on filter change |
| Core/ViewModels/PlaylistDetailViewModel.swift | Adds pagination logic, radio playlist detection, and deduplication |
| Core/ViewModels/LikedMusicViewModel.swift | Adds pagination logic with deduplication |
| Core/ViewModels/LibraryViewModel.swift | Updates to use new response structures |
| Core/Services/Protocols.swift | Adds protocol methods for pagination APIs |
| Core/Services/API/YTMusicClient.swift | Implements filtered search, continuation requests, and queue API |
| Core/Services/API/Parsers/SongMetadataParser.swift | Adds wrapper renderer structure handling |
| Core/Services/API/Parsers/SearchResponseParser.swift | Adds filtered search and continuation parsing |
| Core/Services/API/Parsers/RadioQueueParser.swift | Adds wrapper renderer structure handling |
| Core/Services/API/Parsers/PlaylistParser.swift | Adds extensive continuation parsing logic for multiple formats and queue API parsing |
| Core/Services/API/Parsers/ParsingHelpers.swift | Adds song count extraction and artist filtering logic |
| Core/Services/API/MockUITestYTMusicClient.swift | Updates mock for UI tests with pagination stubs |
| Core/Models/SearchResponse.swift | Adds continuation token support and subtitle improvements |
| Core/Models/Playlist.swift | Adds new response types for pagination |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var selectedFilter: SearchFilter = .all { | ||
| didSet { | ||
| if oldValue != self.selectedFilter, !self.query.isEmpty, self.lastSearchedQuery != nil { | ||
| // Filter changed - perform a new filtered search | ||
| self.searchWithFilter() | ||
| } | ||
| } |
There was a problem hiding this comment.
The selectedFilter property's didSet calls searchWithFilter() which doesn't check if a search has already been performed. If the filter is changed before any search is executed (lastSearchedQuery is nil), it will still trigger a search. This could cause an unexpected API call before the user has entered a query. The condition should check self.lastSearchedQuery != nil before calling searchWithFilter().
| browseId.hasPrefix("UC") // Artist IDs start with UC | ||
| { | ||
| artistId = browseId | ||
| artists.append(Artist(id: browseId, name: artistName)) |
There was a problem hiding this comment.
The artist extraction logic now filters out items without UC-prefixed browse IDs, which means valid artists without browse endpoints will be silently dropped. This could result in incomplete artist information for some tracks. Consider logging a debug message when an artist name is skipped due to missing browse ID, to help diagnose potential parsing issues.
| artists.append(Artist(id: browseId, name: artistName)) | |
| artists.append(Artist(id: browseId, name: artistName)) | |
| } else { | |
| #if DEBUG | |
| print("ParsingHelpers.extractArtistsFromFlexColumns: Skipping artist '\(artistName)' due to missing or non-UC browseId in run: \(run)") | |
| #endif |
| /// Performs a search with the current filter (no debounce, called when filter changes). | ||
| private func searchWithFilter() { | ||
| self.searchTask?.cancel() | ||
| self.client.clearSearchContinuation() | ||
|
|
||
| guard !self.query.isEmpty else { | ||
| self.results = .empty | ||
| self.loadingState = .idle | ||
| return | ||
| } | ||
|
|
||
| self.searchTask = Task { | ||
| await self.performSearch() | ||
| } | ||
| } |
There was a problem hiding this comment.
The searchWithFilter method doesn't debounce the search request. When a user rapidly changes filters, this could trigger multiple simultaneous API requests. Consider canceling any in-flight search task before starting a new one, or add debouncing similar to the main search method.
| /// Loads more tracks via continuation. | ||
| func loadMore() async { | ||
| guard self.loadingState == .loaded, self.hasMore, let currentDetail = playlistDetail else { return } | ||
|
|
||
| self.loadingState = .loadingMore | ||
| self.logger.info("Loading more playlist tracks") | ||
|
|
||
| do { | ||
| guard let response = try await client.getPlaylistContinuation() else { | ||
| self.hasMore = false | ||
| self.loadingState = .loaded | ||
| return | ||
| } | ||
|
|
||
| // Build a set of existing video IDs for deduplication | ||
| let existingVideoIds = Set(currentDetail.tracks.map(\.videoId)) | ||
|
|
||
| // Filter out duplicates from the new tracks | ||
| let newTracks = response.tracks.filter { !existingVideoIds.contains($0.videoId) } | ||
|
|
||
| // If no new unique tracks were added, stop pagination | ||
| // This handles radio playlists that return overlapping data | ||
| if newTracks.isEmpty { | ||
| self.hasMore = false | ||
| self.loadingState = .loaded | ||
| self.logger.info("No new unique tracks in continuation, stopping pagination") | ||
| return | ||
| } | ||
|
|
||
| // Append only new tracks to existing playlist | ||
| let allTracks = currentDetail.tracks + newTracks | ||
| let updatedPlaylist = Playlist( | ||
| id: currentDetail.id, | ||
| title: currentDetail.title, | ||
| description: currentDetail.description, | ||
| thumbnailURL: currentDetail.thumbnailURL, | ||
| trackCount: allTracks.count, | ||
| author: currentDetail.author | ||
| ) | ||
| self.playlistDetail = PlaylistDetail( | ||
| playlist: updatedPlaylist, | ||
| tracks: allTracks, | ||
| duration: currentDetail.duration | ||
| ) | ||
| self.hasMore = response.hasMore | ||
|
|
||
| self.loadingState = .loaded | ||
| self.logger.info("Loaded \(newTracks.count) new tracks (from \(response.tracks.count)), total: \(allTracks.count), hasMore: \(self.hasMore)") | ||
| } catch is CancellationError { | ||
| self.logger.debug("Playlist continuation cancelled") | ||
| self.loadingState = .loaded | ||
| } catch { | ||
| self.logger.error("Failed to load more playlist tracks: \(error.localizedDescription)") | ||
| // Keep loaded state so user can retry | ||
| self.loadingState = .loaded | ||
| } | ||
| } |
There was a problem hiding this comment.
The new pagination functionality in PlaylistDetailViewModel.loadMore() and the radio playlist queue API fallback logic lack test coverage. There are no tests for the playlist detail view model at all, but other similar view models in the project have tests. Consider adding tests for the pagination flow, deduplication logic, radio playlist detection, and queue API fallback behavior.
| static func extractSongCount(from text: String) -> Int? { | ||
| // Match patterns like "145 songs" or "1 song" | ||
| guard let regex = try? NSRegularExpression(pattern: #"(\d+)\s+songs?"#, options: .caseInsensitive), | ||
| let match = regex.firstMatch(in: text, range: NSRange(text.startIndex..., in: text)), | ||
| let countRange = Range(match.range(at: 1), in: text) | ||
| else { | ||
| return nil | ||
| } | ||
| return Int(text[countRange]) |
There was a problem hiding this comment.
The regular expression pattern uses \d+ which requires at least one digit, but doesn't validate that the matched number is reasonable. An extremely large number (e.g., "999999999999999999 songs") could cause integer overflow when converted to Int. Consider adding a range check or using a more restricted pattern.
| /// Strips song count patterns from author text (e.g., " • 145 songs"). | ||
| /// Used to clean fallback author values that may contain redundant song counts. | ||
| private func stripSongCount(from text: String?) -> String? { | ||
| guard var result = text else { return nil } | ||
| result = result.replacingOccurrences( | ||
| of: #" • \d+ songs?"#, | ||
| with: "", | ||
| options: .regularExpression | ||
| ) | ||
| if result.hasPrefix(" • ") { | ||
| result = String(result.dropFirst(3)) | ||
| } | ||
| result = result.trimmingCharacters(in: .whitespaces) | ||
| return result.isEmpty ? nil : result | ||
| } |
There was a problem hiding this comment.
The stripSongCount method is duplicated between PlaylistDetailViewModel (lines 28-42) and similar functionality exists as stripSongCountPattern in ParsingHelpers (lines 308-323). This is code duplication. Consider using the helper from ParsingHelpers instead of defining a separate method in the view model.
| // For radio playlists (RDCLAK prefix), use the queue API to get all tracks at once | ||
| // This bypasses the broken continuation pagination for these playlists | ||
| // Check for both VL-prefixed and raw RDCLAK IDs | ||
| let isRadioPlaylist = playlistId.contains("RDCLAK") || playlistId.hasPrefix("RD") |
There was a problem hiding this comment.
The radio playlist detection logic checks both contains("RDCLAK") and hasPrefix("RD"), but hasPrefix("RD") is overly broad and will match many non-radio playlists. For example, a playlist ID like "RDAMVM" (YouTube Mix) would be incorrectly classified as a radio playlist. Consider making the detection more specific to only match known radio playlist patterns.
| let isRadioPlaylist = playlistId.contains("RDCLAK") || playlistId.hasPrefix("RD") | |
| let isRadioPlaylist = playlistId.contains("RDCLAK") |
| // If it's a radio playlist, always fetch all tracks via queue API | ||
| // The browse API often returns hasMore=false even when there are more tracks | ||
| if isRadioPlaylist { | ||
| self.logger.info("Radio playlist detected, fetching all tracks via queue API") | ||
| do { | ||
| let allTracks = try await client.getPlaylistAllTracks(playlistId: self.playlist.id) | ||
| if allTracks.count > detail.tracks.count { | ||
| self.logger.info("Queue API returned \(allTracks.count) tracks (vs \(detail.tracks.count) from browse)") | ||
| // Update the detail with all tracks from queue API | ||
| let updatedPlaylist = Playlist( | ||
| id: detail.id, | ||
| title: detail.title, | ||
| description: detail.description, | ||
| thumbnailURL: detail.thumbnailURL, | ||
| trackCount: allTracks.count, | ||
| author: detail.author | ||
| ) | ||
| detail = PlaylistDetail( | ||
| playlist: updatedPlaylist, | ||
| tracks: allTracks, | ||
| duration: detail.duration | ||
| ) | ||
| self.hasMore = false | ||
| } | ||
| } catch { | ||
| // If queue API fails, fall back to browse results | ||
| self.logger.warning("Queue API failed, using browse results: \(error.localizedDescription)") | ||
| } | ||
| } |
There was a problem hiding this comment.
The comment states that the browse API "often returns hasMore=false even when there are more tracks" for radio playlists, but then the code unconditionally fetches all tracks via the queue API for any radio playlist. This means even if the browse API returned all tracks correctly (hasMore=false with all tracks), the queue API will still be called unnecessarily. Consider checking if response.detail.tracks.count < response.detail.playlist.trackCount before making the queue API call to avoid redundant requests.
| | `UC{channelId}` | Artist Detail | 🌐 | Artist page with songs, albums | `ArtistParser` | | ||
| | `MPLYt{id}` | Lyrics | 🌐 | Song lyrics text | Custom parser | | ||
|
|
||
| > **Note**: `VLLM` is a special case of `VL{playlistId}` where `LM` is the Liked Music playlist ID. Do NOT use `FEmusic_liked_videos` — it returns only ~13 songs without pagination. |
There was a problem hiding this comment.
The documentation states to use VLLM instead of FEmusic_liked_videos, but the warning doesn't explain why the latter is problematic beyond "returns only ~13 songs". It would be helpful to clarify whether FEmusic_liked_videos is deprecated, has a known bug, or simply has a different use case.
| > **Note**: `VLLM` is a special case of `VL{playlistId}` where `LM` is the Liked Music playlist ID. Do NOT use `FEmusic_liked_videos` — it returns only ~13 songs without pagination. | |
| > **Note**: `VLLM` is a special case of `VL{playlistId}` where `LM` is the Liked Music playlist ID. Do NOT use `FEmusic_liked_videos` for liked songs: it is a limited front‑end feed intended for a small “Liked songs” shelf, returns only ~13 items with no continuation tokens, and is not suitable or supported for complete library access. |
| /// Parses a search continuation response. | ||
| /// Returns a SearchResponse with all item types and optional continuation token. | ||
| static func parseContinuation(_ data: [String: Any]) -> SearchResponse { | ||
| var songs: [Song] = [] | ||
| var albums: [Album] = [] | ||
| var artists: [Artist] = [] | ||
| var playlists: [Playlist] = [] | ||
| var continuationToken: String? | ||
|
|
||
| // Continuation responses have a different structure | ||
| if let continuationContents = data["continuationContents"] as? [String: Any], | ||
| let musicShelfContinuation = continuationContents["musicShelfContinuation"] as? [String: Any] | ||
| { | ||
| // Parse items | ||
| if let contents = musicShelfContinuation["contents"] as? [[String: Any]] { | ||
| for itemData in contents { | ||
| if let item = parseSearchResultItem(itemData) { | ||
| switch item { | ||
| case let .song(song): | ||
| songs.append(song) | ||
| case let .album(album): | ||
| albums.append(album) | ||
| case let .artist(artist): | ||
| artists.append(artist) | ||
| case let .playlist(playlist): | ||
| playlists.append(playlist) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Extract next continuation token | ||
| if let continuations = musicShelfContinuation["continuations"] as? [[String: Any]], | ||
| let firstContinuation = continuations.first, | ||
| let nextContinuationData = firstContinuation["nextContinuationData"] as? [String: Any], | ||
| let token = nextContinuationData["continuation"] as? String | ||
| { | ||
| continuationToken = token | ||
| } | ||
| } | ||
|
|
||
| return SearchResponse( | ||
| songs: songs, | ||
| albums: albums, | ||
| artists: artists, | ||
| playlists: playlists, | ||
| continuationToken: continuationToken | ||
| ) | ||
| } |
There was a problem hiding this comment.
The new parseContinuation method in SearchResponseParser that handles search continuation responses lacks test coverage. This parser handles continuation-specific response structures which differ from initial search responses. Consider adding tests to verify correct parsing of continuation responses.
No description provided.