From 543031e48cebb82c468b85db39bf14a5612e1d47 Mon Sep 17 00:00:00 2001 From: kami Date: Thu, 30 Jul 2026 23:50:08 +0400 Subject: [PATCH] fix: track a playback currentIndex so prev and repeat-all work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit next() did `queue.slice(idx + 1)`, so the current track was always queue[0]. prev()'s `idx > 0` guard could therefore never pass after an auto-advance — Previous did nothing, ever — and repeat: 'all' jumped to queue[0], which is the track that just finished, looping the last track of an album instead of restarting it. Replaced with a currentIndex cursor; the queue is no longer trimmed behind the playhead. The old slice did serve a purpose — bounding Vibe-prefetch growth — so that is preserved as a MAX_HISTORY = 50 cap that drops the oldest entries and re-bases the index, rather than dropped outright. setQueue/playTrack/setCurrentTrack recompute the cursor, next()/prev() fall back to findIndex if it drifts, and shuffle now picks by index so the cursor stays valid. Consumer audit: NowPlayingPanel and Vibe.tsx already derived position via findIndex and needed no change. TrackRow.handlePlay did `setQueue(queue.slice(index))`, which re-broke prev at the point of click even with the store fixed; it now passes the intact queue. This commit also includes a pre-existing uncommitted fix from the working tree (not authored by Claude): the end-of-queue auto-resume loop, which stops playback at the end of the queue instead of restarting. It is correct and independent of the cursor bug, and is preserved verbatim here. REVIEW-2026-07-30.md finding 7. Co-Authored-By: Claude Opus 5 --- frontend/src/components/TrackRow.tsx | 6 +- frontend/src/store/usePlaybackStore.ts | 142 ++++++++++++++++++++----- 2 files changed, 117 insertions(+), 31 deletions(-) diff --git a/frontend/src/components/TrackRow.tsx b/frontend/src/components/TrackRow.tsx index bf7a292..33a3862 100644 --- a/frontend/src/components/TrackRow.tsx +++ b/frontend/src/components/TrackRow.tsx @@ -26,7 +26,7 @@ interface TrackRowProps { showVibe?: boolean; } -export function TrackRow({ track, queue, index, showActions = true, variant = 'default', showVibe = false }: TrackRowProps) { +export function TrackRow({ track, queue, showActions = true, variant = 'default', showVibe = false }: TrackRowProps) { const { setQueue, playTrack, play, pause, currentTrack, isPlaying } = usePlaybackStore(); const dislikeTrack = useDislikeTrack(); const router = useRouter(); @@ -35,7 +35,9 @@ export function TrackRow({ track, queue, index, showActions = true, variant = 'd const handlePlay = () => { if (isCurrent) { isPlaying ? pause() : play(); return; } - setQueue(queue.slice(index)); + // Queue the whole list and start at this track, so Previous can walk back + // into the tracks before it. + setQueue(queue); playTrack(track); }; diff --git a/frontend/src/store/usePlaybackStore.ts b/frontend/src/store/usePlaybackStore.ts index 3fa2738..01615e8 100644 --- a/frontend/src/store/usePlaybackStore.ts +++ b/frontend/src/store/usePlaybackStore.ts @@ -3,9 +3,17 @@ import type { Track } from '../types'; export type RepeatMode = 'none' | 'all' | 'one'; +/** + * How many already-played tracks to keep behind the cursor. Bounds queue growth + * from the Vibe prefetch loop while still leaving real history for prev(). + */ +const MAX_HISTORY = 50; + interface PlaybackState { currentTrack: Track | null; queue: Track[]; + /** Cursor into `queue` for the current track, or -1 when the current track is not queued. */ + currentIndex: number; isPlaying: boolean; position: number; duration: number; @@ -29,9 +37,33 @@ interface PlaybackState { cycleRepeat: () => void; } +/** + * Move to `index` in `queue`, trimming stale history so the queue stays bounded. + * Returns the state patch (queue may be re-sliced, so the index is adjusted). + */ +function advanceTo(queue: Track[], index: number) { + let nextQueue = queue; + let nextIndex = index; + if (index > MAX_HISTORY) { + const drop = index - MAX_HISTORY; + nextQueue = queue.slice(drop); + nextIndex = index - drop; + } + const track = nextQueue[nextIndex]; + return { + queue: nextQueue, + currentIndex: nextIndex, + currentTrack: track, + position: 0, + duration: track?.duration ?? 0, + isPlaying: true, + }; +} + export const usePlaybackStore = create((set, get) => ({ currentTrack: null, queue: [], + currentIndex: -1, isPlaying: false, position: 0, duration: 0, @@ -40,25 +72,42 @@ export const usePlaybackStore = create((set, get) => ({ repeat: 'none', shufflePlayed: new Set(), - setQueue: (queue) => set({ queue, shufflePlayed: new Set() }), + setQueue: (queue) => + set((state) => ({ + queue, + // Keep the cursor pointing at whatever is playing, if it is still queued. + currentIndex: state.currentTrack ? queue.findIndex((t) => t.id === state.currentTrack!.id) : -1, + shufflePlayed: new Set(), + })), playTrack: (track) => - set({ + set((state) => ({ currentTrack: track, + currentIndex: state.queue.findIndex((t) => t.id === track.id), isPlaying: true, position: 0, duration: track.duration ?? 0, shufflePlayed: new Set(), - }), + })), play: () => set({ isPlaying: true }), pause: () => set({ isPlaying: false }), next: () => { const { queue, currentTrack, shuffle, repeat, shufflePlayed } = get(); - if (queue.length === 0) return; + if (queue.length === 0) { + set({ isPlaying: false, position: 0 }); + return; + } - const idx = currentTrack ? queue.findIndex((t) => t.id === currentTrack.id) : -1; + // Prefer the tracked cursor; fall back to a lookup if it has drifted. + const tracked = get().currentIndex; + const idx = + tracked >= 0 && queue[tracked]?.id === currentTrack?.id + ? tracked + : currentTrack + ? queue.findIndex((t) => t.id === currentTrack.id) + : -1; // Repeat one: replay current track if (repeat === 'one' && currentTrack) { @@ -71,49 +120,84 @@ export const usePlaybackStore = create((set, get) => ({ // so repeat-all doesn't bounce between the same few tracks. const played = new Set(shufflePlayed); if (currentTrack) played.add(currentTrack.id); - let remaining = queue.filter((t) => !played.has(t.id)); + const indices = queue.map((_, i) => i); + let remaining = indices.filter((i) => !played.has(queue[i].id)); if (remaining.length === 0) { - if (repeat !== 'all') return; + if (repeat !== 'all') { + set({ isPlaying: false, position: 0 }); + return; + } // Lap complete — start a fresh one. played.clear(); if (currentTrack) played.add(currentTrack.id); - remaining = queue.filter((t) => t.id !== currentTrack?.id); - if (remaining.length === 0) return; + remaining = indices.filter((i) => queue[i].id !== currentTrack?.id); + if (remaining.length === 0) { + set({ isPlaying: false, position: 0 }); + return; + } } - const pick = remaining[Math.floor(Math.random() * remaining.length)]; - played.add(pick.id); - set({ currentTrack: pick, shufflePlayed: played, position: 0, duration: pick.duration ?? 0, isPlaying: true }); + const pickIdx = remaining[Math.floor(Math.random() * remaining.length)]; + played.add(queue[pickIdx].id); + const pick = queue[pickIdx]; + set({ + currentTrack: pick, + currentIndex: pickIdx, + shufflePlayed: played, + position: 0, + duration: pick.duration ?? 0, + isPlaying: true, + }); return; } - // Sequential - const nextTrack = idx >= 0 ? queue[idx + 1] : null; - if (nextTrack) { - // Trim played tracks from queue to prevent unbounded growth (Vibe prefetch leak) - const trimmed = queue.slice(idx + 1); - set({ queue: trimmed, currentTrack: nextTrack, position: 0, duration: nextTrack.duration ?? 0, isPlaying: true }); - } else if (repeat === 'all') { - const first = queue[0]; - if (first) { - set({ currentTrack: first, position: 0, duration: first.duration ?? 0, isPlaying: true }); - } + // Sequential — keep the queue intact and move the cursor, so prev() has + // history and repeat-all restarts from the true first track. + if (idx < 0) { + // Nothing playing (or the current track left the queue) — start at the top. + set(advanceTo(queue, 0)); + } else if (idx + 1 < queue.length) { + set(advanceTo(queue, idx + 1)); + } else if (repeat === 'all' && queue.length > 0) { + set(advanceTo(queue, 0)); + } else { + // End of queue with no looping — stop playback so the store, DOM audio + // element, and MediaSession all agree nothing is playing. Without this + // the browser's now-playing widget can auto-resume the finished track + // from its near-end position, causing a tight "play last second → end → + // auto-resume → play last second → …" loop that also degrades performance. + set({ isPlaying: false, position: 0 }); } }, prev: () => { - const { queue, currentTrack } = get(); + const { queue, currentTrack, currentIndex } = get(); if (queue.length === 0) return; - const idx = currentTrack ? queue.findIndex((t) => t.id === currentTrack.id) : -1; - const prevTrack = idx > 0 ? queue[idx - 1] : null; - if (prevTrack) { - set({ currentTrack: prevTrack, position: 0, duration: prevTrack.duration ?? 0, isPlaying: true }); + const idx = + currentIndex >= 0 && queue[currentIndex]?.id === currentTrack?.id + ? currentIndex + : currentTrack + ? queue.findIndex((t) => t.id === currentTrack.id) + : -1; + if (idx > 0) { + const prevTrack = queue[idx - 1]; + set({ + currentTrack: prevTrack, + currentIndex: idx - 1, + position: 0, + duration: prevTrack.duration ?? 0, + isPlaying: true, + }); } }, setPosition: (position) => set({ position }), setDuration: (duration) => set({ duration }), setVolume: (volume) => set({ volume }), - setCurrentTrack: (currentTrack) => set({ currentTrack }), + setCurrentTrack: (currentTrack) => + set((state) => ({ + currentTrack, + currentIndex: currentTrack ? state.queue.findIndex((t) => t.id === currentTrack.id) : -1, + })), toggleShuffle: () => set((state) => ({ shuffle: !state.shuffle })), cycleRepeat: () =>