From d08eebf0eb1f5827043e915a688818e0ccb7c2dc Mon Sep 17 00:00:00 2001
From: Mistral Vibe <vibe@mistral.ai>
Date: Wed, 8 Apr 2026 21:10:21 +0200
Subject: [PATCH] fix(audio): survive navigation, clear stale state, silence
 noisy logs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Bug 1 — playback stops on navigation:
WaveSurfer v7 creates its <audio> element inside the container div. When
SongPage unmounts, the container is removed from the DOM, taking the audio
element with it and stopping playback. Fix: AudioService owns a persistent
hidden <audio> element on document.body and passes it to WaveSurfer via the
`media` option. WaveSurfer uses it for playback but does not destroy it on
WaveSurfer.destroy(), so audio survives any number of navigations.

Bug 2 — stale playhead/duration when opening a new song:
initialize() called destroyWaveSurfer() but never reset the store, so the
previous song's currentTime, duration, and isPlaying leaked into the new song's
load sequence. Fix: reset those three fields in the store immediately after
tearing down the old WaveSurfer instance. cleanup() also now resets duration.

Bug 3 — excessive console noise on mobile:
Remove console.warn from play() (silent return when not ready) and from
useWaveform's play() wrapper. Only console.error on actual errors remains.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 web/src/hooks/useWaveform.ts     |  6 +-----
 web/src/services/audioService.ts | 37 +++++++++++++++++++++++++-------
 web/tests/audioService.test.ts   | 37 ++++++++++++++++++--------------
 3 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/web/src/hooks/useWaveform.ts b/web/src/hooks/useWaveform.ts
index 974ff9f..0edc39d 100755
--- a/web/src/hooks/useWaveform.ts
+++ b/web/src/hooks/useWaveform.ts
@@ -80,12 +80,8 @@ export function useWaveform(
   }, [options.url, options.songId, options.bandId]);
 
   const play = () => {
-    if (!audioService.isWaveformReady()) {
-      console.warn('[useWaveform] play() called but not ready', { url: options.url });
-      return;
-    }
     audioService.play(options.songId ?? null, options.bandId ?? null)
-      .catch(err => console.error('useWaveform.play failed:', err));
+      .catch(err => console.error('[useWaveform] play failed:', err));
   };
 
   const pause = () => {
diff --git a/web/src/services/audioService.ts b/web/src/services/audioService.ts
index 2ecbc8f..2b4fd66 100755
--- a/web/src/services/audioService.ts
+++ b/web/src/services/audioService.ts
@@ -7,6 +7,11 @@ class AudioService {
   private currentUrl: string | null = null;
   private isReady = false;
   private lastTimeUpdate = 0;
+  // Persistent audio element attached to document.body so playback survives
+  // SongPage unmounts. WaveSurfer v7 supports passing an existing media element
+  // via the `media` option — it uses it for playback but does NOT destroy it
+  // when WaveSurfer.destroy() is called.
+  private mediaElement: HTMLAudioElement | null = null;
 
   private constructor() {}
 
@@ -19,9 +24,21 @@ class AudioService {
 
   // For use in tests only
   public static resetInstance(): void {
+    if (this.instance?.mediaElement) {
+      this.instance.mediaElement.remove();
+    }
     this.instance = undefined as any;
   }
 
+  private getOrCreateMediaElement(): HTMLAudioElement {
+    if (!this.mediaElement) {
+      this.mediaElement = document.createElement('audio');
+      this.mediaElement.style.display = 'none';
+      document.body.appendChild(this.mediaElement);
+    }
+    return this.mediaElement;
+  }
+
   public async initialize(container: HTMLElement, url: string): Promise<void> {
     if (!container) throw new Error('Container element is required');
     if (!url) throw new Error('Valid audio URL is required');
@@ -29,11 +46,17 @@ class AudioService {
     // Reuse the existing instance when the URL hasn't changed
     if (this.currentUrl === url && this.wavesurfer) return;
 
-    // Tear down the previous instance before creating a new one
-    if (this.wavesurfer) this.destroyWaveSurfer();
+    // Tear down the previous instance and clear stale store state
+    if (this.wavesurfer) {
+      this.destroyWaveSurfer();
+      usePlayerStore.getState().batchUpdate({ isPlaying: false, currentTime: 0, duration: 0 });
+    }
 
     const ws = WaveSurfer.create({
       container,
+      // Provide a persistent audio element so playback continues even when
+      // the SongPage container div is removed from the DOM on navigation.
+      media: this.getOrCreateMediaElement(),
       waveColor: "rgba(255,255,255,0.09)",
       progressColor: "#c8861a",
       cursorColor: "#e8a22a",
@@ -80,10 +103,7 @@ class AudioService {
   }
 
   public async play(songId: string | null = null, bandId: string | null = null): Promise<void> {
-    if (!this.wavesurfer || !this.isReady) {
-      console.warn('[AudioService] play() called before ready');
-      return;
-    }
+    if (!this.wavesurfer || !this.isReady) return;
     await this.wavesurfer.play();
     if (songId && bandId) {
       usePlayerStore.getState().setCurrentSong(songId, bandId);
@@ -120,13 +140,14 @@ class AudioService {
     this.destroyWaveSurfer();
     const store = usePlayerStore.getState();
     store.setCurrentSong(null, null);
-    store.batchUpdate({ isPlaying: false, currentTime: 0 });
+    store.batchUpdate({ isPlaying: false, currentTime: 0, duration: 0 });
   }
 
   private destroyWaveSurfer(): void {
     if (!this.wavesurfer) return;
     try {
-      if (this.wavesurfer.isPlaying()) this.wavesurfer.pause();
+      // Don't pause — if audio is playing and we're switching songs, we want
+      // the pause to happen naturally when the media element src changes.
       this.wavesurfer.unAll();
       this.wavesurfer.destroy();
     } catch (err) {
diff --git a/web/tests/audioService.test.ts b/web/tests/audioService.test.ts
index 07fd3b4..510ebb9 100644
--- a/web/tests/audioService.test.ts
+++ b/web/tests/audioService.test.ts
@@ -129,6 +129,22 @@ describe('AudioService', () => {
       expect(vi.mocked(WaveSurfer.create).mock.calls.length).toBe(createCallCount); // no new instance
     });
 
+    it('resets store state when URL changes', async () => {
+      await initService(service, { url: 'http://example.com/a.mp3', duration: 180 });
+      usePlayerStore.getState().batchUpdate({ isPlaying: true, currentTime: 45 });
+
+      const WaveSurfer = (await import('wavesurfer.js')).default;
+      const mockWs2 = createMockWaveSurfer();
+      vi.mocked(WaveSurfer.create).mockReturnValue(mockWs2 as any);
+      const p2 = service.initialize(makeContainer(), 'http://example.com/b.mp3');
+      mockWs2._emit('ready');
+      await p2;
+
+      // State should be reset, not carry over from the previous song
+      expect(usePlayerStore.getState().currentTime).toBe(0);
+      expect(usePlayerStore.getState().isPlaying).toBe(false);
+    });
+
     it('destroys old instance when URL changes', async () => {
       const WaveSurfer = (await import('wavesurfer.js')).default;
 
@@ -152,10 +168,8 @@ describe('AudioService', () => {
 
   describe('play()', () => {
     it('does nothing if not ready', async () => {
-      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
-      await service.play('song-1', 'band-1');
-      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('before ready'));
-      warnSpy.mockRestore();
+      // play() silently returns when not ready — no throw, no warn
+      await expect(service.play('song-1', 'band-1')).resolves.toBeUndefined();
     });
 
     it('calls wavesurfer.play()', async () => {
@@ -225,23 +239,14 @@ describe('AudioService', () => {
       expect(service.isWaveformReady()).toBe(false);
     });
 
-    it('resets isPlaying and currentTime in the store', async () => {
-      await initService(service);
+    it('resets isPlaying, currentTime, and duration in the store', async () => {
+      await initService(service, { duration: 180 });
       usePlayerStore.getState().batchUpdate({ isPlaying: true, currentTime: 30 });
       service.cleanup();
       const state = usePlayerStore.getState();
       expect(state.isPlaying).toBe(false);
       expect(state.currentTime).toBe(0);
-    });
-
-    it('pauses before destroying if playing', async () => {
-      const callOrder: string[] = [];
-      const mockWs = await initService(service);
-      mockWs.isPlaying.mockReturnValue(true);
-      mockWs.pause.mockImplementation(() => callOrder.push('pause'));
-      mockWs.destroy.mockImplementation(() => callOrder.push('destroy'));
-      service.cleanup();
-      expect(callOrder).toEqual(['pause', 'destroy']);
+      expect(state.duration).toBe(0);
     });
 
     it('is safe to call when not initialized', () => {