From 179519689b79e3010cf33aaea8d1a8ef3d838626 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 15 May 2026 07:57:00 -0400 Subject: [PATCH] =?UTF-8?q?feat(web):=20#415=20stage=203=20(web)=20?= =?UTF-8?q?=E2=80=94=20rotation-aware=20system=20playlist=20play?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Web half of Stage 3. System-playlist tile play now: - calls the new GET /api/playlists/system/{variant}/shuffle endpoint (rotation-aware order from the server) instead of getPlaylist; plays the returned order AS-IS — no client Fisher-Yates, since the server already ordered it. Supersedes #413's client shuffle for system playlists specifically; user playlists keep getPlaylist + stored order. - tags the queue with the system variant. The player store carries _queueSource; the events dispatcher includes `source` on play_started so the server advances that playlist's rotation. User playlists are unchanged (getPlaylist, plain playQueue, no source). Tests updated: For-You play hits systemShuffle (not getPlaylist/refresh) and passes source:for_you; user play uses getPlaylist + plain playQueue with no source. Flutter half is blocked — the Flutter client has no play-event reporting at all (no /api/events POST, no scrobble), so there's no play_started to attach `source` to. Surfacing that as a separate decision rather than silently scope-exploding #415. Co-Authored-By: Claude Opus 4.7 (1M context) --- web/src/lib/api/playlists.ts | 12 +++++ web/src/lib/components/PlaylistCard.svelte | 27 ++++++---- web/src/lib/components/PlaylistCard.test.ts | 55 ++++++++++++++++----- web/src/lib/player/events.svelte.ts | 6 ++- web/src/lib/player/store.svelte.ts | 14 +++++- 5 files changed, 91 insertions(+), 23 deletions(-) diff --git a/web/src/lib/api/playlists.ts b/web/src/lib/api/playlists.ts index 2b442068..e1dfa192 100644 --- a/web/src/lib/api/playlists.ts +++ b/web/src/lib/api/playlists.ts @@ -19,6 +19,18 @@ export async function getPlaylist(id: string): Promise { return api.get(`/api/playlists/${encodeURIComponent(id)}`); } +// #415 stage 2/3: rotation-aware play order for a system playlist. +// Same shape as getPlaylist but tracks are server-ordered (unplayed +// this rotation first, then heard; resets when exhausted). The model +// variant uses underscores; the route segment is hyphenated. +// Intentionally uncached — it varies per play, unlike getPlaylist. +export async function systemShuffle(variant: string): Promise { + const seg = variant === 'for_you' ? 'for-you' : variant; + return api.get( + `/api/playlists/system/${encodeURIComponent(seg)}/shuffle`, + ); +} + export type CreatePlaylistInput = { name: string; description?: string; diff --git a/web/src/lib/components/PlaylistCard.svelte b/web/src/lib/components/PlaylistCard.svelte index 78c64bb7..92f41875 100644 --- a/web/src/lib/components/PlaylistCard.svelte +++ b/web/src/lib/components/PlaylistCard.svelte @@ -3,7 +3,7 @@ import { useQueryClient } from '@tanstack/svelte-query'; import type { Playlist, PlaylistTrack, TrackRef } from '$lib/api/types'; import { user } from '$lib/auth/store.svelte'; - import { getPlaylist, refreshDiscover, refreshForYou } from '$lib/api/playlists'; + import { getPlaylist, systemShuffle, refreshDiscover, refreshForYou } from '$lib/api/playlists'; import { playlistTrackToRef } from '$lib/playlists/playlistTrackToRef'; import { errCode } from '$lib/api/errors'; import { qk } from '$lib/api/queries'; @@ -45,14 +45,23 @@ if (starting) return; starting = true; try { - const detail = await getPlaylist(playlist.id); - const refs = toTrackRefs(detail.tracks); - if (refs.length > 0) { - // System playlists default to shuffle so replaying within a - // day feels different. User playlists keep stored order. - // Explicit refresh moved to the kebab menu — playing no - // longer rebuilds the snapshot. - playQueue(refs, 0, { shuffle: playlist.system_variant != null }); + const variant = playlist.system_variant; + if (variant != null) { + // #415: server returns the rotation-aware order (unplayed + // this rotation first). Play it as-is — no client shuffle — + // and tag the queue so play_started carries `source` and + // the server advances the rotation. + const detail = await systemShuffle(variant); + const refs = toTrackRefs(detail.tracks); + if (refs.length > 0) { + playQueue(refs, 0, { source: variant }); + } + } else { + const detail = await getPlaylist(playlist.id); + const refs = toTrackRefs(detail.tracks); + if (refs.length > 0) { + playQueue(refs, 0); + } } } finally { starting = false; diff --git a/web/src/lib/components/PlaylistCard.test.ts b/web/src/lib/components/PlaylistCard.test.ts index 688f86d9..5eac8386 100644 --- a/web/src/lib/components/PlaylistCard.test.ts +++ b/web/src/lib/components/PlaylistCard.test.ts @@ -38,6 +38,36 @@ vi.mock('$lib/api/playlists', () => ({ } ] } as PlaylistDetail), + systemShuffle: vi.fn().mockResolvedValue({ + id: 'p-sys', + user_id: 'u-self', + owner_username: 'me', + name: 'For You', + description: '', + is_public: false, + kind: 'system', + system_variant: 'for_you', + seed_artist_id: null, + cover_url: '', + track_count: 1, + duration_sec: 60, + created_at: '2026-01-01T00:00:00Z', + updated_at: '2026-01-01T00:00:00Z', + tracks: [ + { + position: 0, + track_id: 't-9', + album_id: 'al-1', + artist_id: 'ar-1', + title: 'Rotation Track', + artist_name: 'Test Artist', + album_title: 'Test Album', + duration_sec: 60, + stream_url: '/s/t-9', + added_at: '2026-01-01T00:00:00Z' + } + ] + } as PlaylistDetail), refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5 }), refreshForYou: vi.fn().mockResolvedValue({ playlist_id: 'p-new', track_count: 25, track_ids: ['t-1'] }) })); @@ -228,8 +258,8 @@ describe('PlaylistCard', () => { await waitFor(() => expect(refreshForYou).toHaveBeenCalled()); }); - test('For-You play button does NOT call refreshForYou (uses existing snapshot)', async () => { - const { refreshForYou, getPlaylist } = await import('$lib/api/playlists'); + test('For-You play calls systemShuffle, not getPlaylist or refreshForYou', async () => { + const { refreshForYou, getPlaylist, systemShuffle } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); const forYouPlaylist: Playlist = { ...base, @@ -244,13 +274,15 @@ describe('PlaylistCard', () => { await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - // Play uses the existing playlist snapshot — no refresh on press. + // #415: system play hits the rotation-aware shuffle endpoint, + // not the cached detail GET, and never refreshes on press. + expect(systemShuffle).toHaveBeenCalledWith('for_you'); + expect(getPlaylist).not.toHaveBeenCalled(); expect(refreshForYou).not.toHaveBeenCalled(); - expect(getPlaylist).toHaveBeenCalledWith('p-1'); expect(playQueue).toHaveBeenCalled(); }); - test('For-You play passes shuffle:true to playQueue', async () => { + test('For-You play passes source:for_you to playQueue (no client shuffle)', async () => { const { playQueue } = await import('$lib/player/store.svelte'); const forYouPlaylist: Playlist = { ...base, @@ -266,22 +298,21 @@ describe('PlaylistCard', () => { expect(playQueue).toHaveBeenCalledWith( expect.any(Array), 0, - expect.objectContaining({ shuffle: true }) + expect.objectContaining({ source: 'for_you' }) ); }); - test('user-playlist play passes shuffle:false to playQueue', async () => { + test('user-playlist play uses getPlaylist + plain playQueue (no source)', async () => { + const { getPlaylist, systemShuffle } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); render(PlaylistCard, { props: { playlist: base } }); const playButton = screen.getByLabelText(/Play Saturday morning/i); await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(playQueue).toHaveBeenCalledWith( - expect.any(Array), - 0, - expect.objectContaining({ shuffle: false }) - ); + expect(systemShuffle).not.toHaveBeenCalled(); + expect(getPlaylist).toHaveBeenCalledWith('p-1'); + expect(playQueue).toHaveBeenCalledWith(expect.any(Array), 0); }); test('non-For-You play button does NOT call refreshForYou', async () => { diff --git a/web/src/lib/player/events.svelte.ts b/web/src/lib/player/events.svelte.ts index 60d82236..ab383c51 100644 --- a/web/src/lib/player/events.svelte.ts +++ b/web/src/lib/player/events.svelte.ts @@ -92,7 +92,11 @@ export function useEventsDispatcher(): void { const res = await api.post('/api/events', { type: 'play_started', track_id: trackId, - client_id: clientId + client_id: clientId, + // #415: tag the play with the system playlist it came from + // (null for library / user-playlist / radio) so the server + // can advance that playlist's rotation. + source: player.queueSource ?? undefined }); // The user may have moved on by the time the response arrives. Only // adopt the id if we're still on the same track and still playing. diff --git a/web/src/lib/player/store.svelte.ts b/web/src/lib/player/store.svelte.ts index 8339916f..156d2b49 100644 --- a/web/src/lib/player/store.svelte.ts +++ b/web/src/lib/player/store.svelte.ts @@ -41,6 +41,13 @@ let _pendingRestorePosition: number | null = null; let _radioSeedId = $state(null); let _radioRefreshInFlight = false; +// #415: which system playlist (if any) this queue was seeded from. +// 'for_you' | 'discover' | null. Read by the events dispatcher so +// play_started carries `source` and the play counts against that +// playlist's rotation. Re-set on every playQueue (a fresh non-system +// queue clears it). +let _queueSource = $state(null); + let _audioEl: HTMLAudioElement | null = null; export const player = { @@ -55,6 +62,7 @@ export const player = { get shuffle() { return _shuffle; }, get repeat() { return _repeat; }, get error() { return _error; }, + get queueSource() { return _queueSource; }, get queueDrawerOpen() { return _queueDrawerOpen; } }; @@ -65,9 +73,13 @@ export function registerAudioEl(el: HTMLAudioElement | null): void { export function playQueue( tracks: TrackRef[], startIndex = 0, - opts: { shuffle?: boolean } = {}, + opts: { shuffle?: boolean; source?: string | null } = {}, ): void { _radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state + // #415: a fresh queue resets the system-playlist source. Set only + // when seeded from a system playlist (server already returned the + // rotation-aware order, so no client shuffle in that path). + _queueSource = opts.source ?? null; if (opts.shuffle && tracks.length > 1) { // Fisher-Yates over the whole list. startIndex is ignored — the // caller is asking for "random play from this pool," so the first