fix(review): re-sign instrumental preview URLs on expiry (refresh-on-error)

.github/workflows/ci.yml

@@ -687,8 +687,20 @@ jobs:
             curl -fsSL https://packages.cloud.google.com/apt/doc/apt-key.gpg | \
               sudo gpg --dearmor --yes -o /usr/share/keyrings/cloud.google.gpg
           fi
-          sudo apt-get update -y
-          sudo apt-get install -y google-cloud-cli-firestore-emulator
+          # Google's cloud-sdk apt mirror intermittently serves a Packages index
+          # whose checksum doesn't match its Release file ("Hash Sum mismatch"),
+          # and a stale runner apt-lists cache triggers the same error. Clearing
+          # the cache forces a fresh fetch; retry to ride out mirror desyncs.
+          for attempt in 1 2 3 4 5; do
+            sudo rm -rf /var/lib/apt/lists/*
+            if sudo apt-get update -y && \
+               sudo apt-get install -y google-cloud-cli-firestore-emulator; then
+              break
+            fi
+            echo "apt install attempt ${attempt} failed (likely a transient Hash Sum mismatch); retrying in 15s..."
+            sleep 15
+          done
+          dpkg -s google-cloud-cli-firestore-emulator >/dev/null 2>&1
 
       - name: Install dependencies
         run: poetry install

backend/api/routes/review.py

@@ -409,6 +409,57 @@ def _dev_audio_url(job_id: str, gcs_path: Optional[str], base_url: str = "") ->
     return f"{base_url.rstrip('/')}/api/review/{job_id}/dev-audio?path={quote(gcs_path, safe='')}"
 
 
+async def _build_instrumental_options(job, storage, request) -> List[Dict[str, Any]]:
+    """Build the ``instrumental_options`` list (transcoded, freshly-signed OGG URLs).
+
+    Signed GCS URLs expire (see ``AudioTranscodingService`` — 120 min), so the
+    review payload's baked-in URLs go stale during a long review session. This
+    helper is shared by the combined-review corrections endpoint (initial load)
+    and ``GET /{job_id}/instrumental-urls`` (frontend refresh-on-expiry), so both
+    return the identical shape with URLs signed at call time.
+    """
+    from backend.services.audio_transcoding_service import AudioTranscodingService
+
+    transcoding = AudioTranscodingService(storage_service=storage)
+    stems = job.file_urls.get("stems", {})
+    clean_url = stems.get("instrumental_clean")
+    backing_url = stems.get("instrumental_with_backing")
+
+    signed_urls: Dict[str, Optional[str]] = {}
+    if _dev_audio_proxy_enabled():
+        _base = str(request.base_url)
+        if clean_url:
+            signed_urls["clean"] = _dev_audio_url(job.job_id, clean_url, _base)
+        if backing_url:
+            signed_urls["with_backing"] = _dev_audio_url(job.job_id, backing_url, _base)
+    else:
+        url_tasks = {}
+        if clean_url:
+            url_tasks["clean"] = transcoding.get_review_audio_url_async(clean_url, expiration_minutes=120)
+        if backing_url:
+            url_tasks["with_backing"] = transcoding.get_review_audio_url_async(backing_url, expiration_minutes=120)
+        if url_tasks:
+            results = await asyncio.gather(*url_tasks.values())
+            signed_urls = dict(zip(url_tasks.keys(), results))
+
+    instrumental_options: List[Dict[str, Any]] = []
+    if clean_url:
+        instrumental_options.append({
+            "id": "clean",
+            "label": "Clean Instrumental",
+            "description": "No backing vocals - just the music",
+            "audio_url": signed_urls.get("clean"),
+        })
+    if backing_url:
+        instrumental_options.append({
+            "id": "with_backing",
+            "label": "Instrumental with Backing Vocals",
+            "description": "Includes harmonies and background vocals",
+            "audio_url": signed_urls.get("with_backing"),
+        })
+    return instrumental_options
+
+
 def _reconstruct_post_ai_segments(
     raw_segments: List[Dict[str, Any]],
     edit_log: Optional[Dict[str, Any]],
@@ -632,50 +683,11 @@ async def get_correction_data(
         }
 
         # === Add instrumental data for combined review ===
-        from backend.services.audio_transcoding_service import AudioTranscodingService
-        transcoding = AudioTranscodingService(storage_service=storage)
-
-        # Get instrumental stem URLs
-        stems = job.file_urls.get('stems', {})
-        clean_url = stems.get('instrumental_clean')
-        backing_url = stems.get('instrumental_with_backing')
-
-        # Build instrumental options with transcoded signed URLs (OGG Opus).
-        # In local dev without signing creds, use same-origin byte-proxy URLs.
-        instrumental_options = []
-        signed_urls = {}
-        if _dev_audio_proxy_enabled():
-            _base = str(request.base_url)
-            if clean_url:
-                signed_urls['clean'] = _dev_audio_url(job_id, clean_url, _base)
-            if backing_url:
-                signed_urls['with_backing'] = _dev_audio_url(job_id, backing_url, _base)
-        else:
-            url_tasks = {}
-            if clean_url:
-                url_tasks['clean'] = transcoding.get_review_audio_url_async(clean_url, expiration_minutes=120)
-            if backing_url:
-                url_tasks['with_backing'] = transcoding.get_review_audio_url_async(backing_url, expiration_minutes=120)
-            if url_tasks:
-                results = await asyncio.gather(*url_tasks.values())
-                signed_urls = dict(zip(url_tasks.keys(), results))
-
-        if clean_url:
-            instrumental_options.append({
-                "id": "clean",
-                "label": "Clean Instrumental",
-                "description": "No backing vocals - just the music",
-                "audio_url": signed_urls['clean'],
-            })
-        if backing_url:
-            instrumental_options.append({
-                "id": "with_backing",
-                "label": "Instrumental with Backing Vocals",
-                "description": "Includes harmonies and background vocals",
-                "audio_url": signed_urls['with_backing'],
-            })
-
-        corrections_data['instrumental_options'] = instrumental_options
+        # Transcoded signed OGG URLs (freshly signed here; the frontend re-fetches
+        # them via GET /{job_id}/instrumental-urls when they expire mid-review).
+        corrections_data['instrumental_options'] = await _build_instrumental_options(
+            job, storage, request
+        )
 
         # Get backing vocals analysis from state_data (populated by screens_worker)
         backing_vocals_analysis = job.state_data.get('backing_vocals_analysis', {})
@@ -1854,6 +1866,31 @@ async def get_annotation_stats(
         return {"total_annotations": 0, "by_type": {}}
 
 
+@router.get("/{job_id}/instrumental-urls")
+async def get_instrumental_urls(
+    job_id: str,
+    request: Request,
+    auth_info: Tuple[str, str] = Depends(require_review_auth)
+):
+    """Return freshly-signed instrumental stem URLs for the combined review.
+
+    The signed URLs baked into the initial review payload expire after 120 min,
+    and a long review session (or a page reload that rehydrates cached correction
+    data from localStorage) can outlive them — the preview modal's instrumental
+    audio then fails to load (``NS_ERROR_DOM_NETWORK_ERR``). The frontend calls
+    this on an audio load error to swap in a fresh URL and resume playback.
+    """
+    job_manager = JobManager()
+    storage = StorageService()
+
+    job = job_manager.get_job(job_id)
+    if not job:
+        raise HTTPException(status_code=404, detail=t("en", "review.jobNotFound"))
+
+    instrumental_options = await _build_instrumental_options(job, storage, request)
+    return {"instrumental_options": instrumental_options}
+
+
 @router.get("/{job_id}/instrumental-analysis")
 async def get_instrumental_analysis(
     job_id: str,

backend/tests/test_routes_review.py

@@ -313,6 +313,84 @@ def test_force_flag_is_passed_through(self, test_client):
         assert mock_ops.add_lyrics_source.call_args.kwargs["force"] is True
 
 
+class TestInstrumentalUrlsRefresh:
+    """GET /{job_id}/instrumental-urls re-signs the stem URLs on demand.
+
+    The signed URLs baked into the initial review payload expire after 120 min;
+    a long review session (or a reload that rehydrates cached correction data)
+    outlives them and the preview modal's overlaid audio fails to load. This
+    endpoint hands the frontend a freshly-signed batch to swap in.
+    """
+
+    @pytest.fixture(autouse=True)
+    def auth_overrides(self):
+        from backend.main import app
+        from backend.api.dependencies import require_review_auth
+
+        async def mock_require_review_auth(job_id: str = "test123"):
+            return (job_id, "full")
+
+        app.dependency_overrides[require_review_auth] = mock_require_review_auth
+        yield
+        app.dependency_overrides.pop(require_review_auth, None)
+
+    def test_returns_freshly_signed_options_for_both_stems(self, test_client):
+        mock_job = MagicMock()
+        mock_job.job_id = "job1"
+        mock_job.file_urls = {
+            "stems": {
+                "instrumental_clean": "jobs/job1/stems/instrumental_clean.flac",
+                "instrumental_with_backing": "jobs/job1/stems/instrumental_with_backing.flac",
+            }
+        }
+
+        async def fake_sign(src, expiration_minutes=120):
+            return f"https://signed/{src}?fresh=1"
+
+        with patch("backend.api.routes.review.JobManager") as mock_jm, \
+                patch("backend.api.routes.review.StorageService"), \
+                patch("backend.api.routes.review._dev_audio_proxy_enabled", return_value=False), \
+                patch("backend.services.audio_transcoding_service.AudioTranscodingService.get_review_audio_url_async",
+                      side_effect=fake_sign):
+            mock_jm.return_value.get_job.return_value = mock_job
+            response = test_client.get("/api/review/job1/instrumental-urls")
+
+        assert response.status_code == 200
+        options = response.json()["instrumental_options"]
+        ids = {o["id"]: o["audio_url"] for o in options}
+        assert ids["clean"].endswith("instrumental_clean.flac?fresh=1")
+        assert ids["with_backing"].endswith("instrumental_with_backing.flac?fresh=1")
+
+    def test_omits_stems_that_do_not_exist(self, test_client):
+        mock_job = MagicMock()
+        mock_job.job_id = "job1"
+        mock_job.file_urls = {
+            "stems": {"instrumental_clean": "jobs/job1/stems/instrumental_clean.flac"}
+        }
+
+        async def fake_sign(src, expiration_minutes=120):
+            return f"https://signed/{src}"
+
+        with patch("backend.api.routes.review.JobManager") as mock_jm, \
+                patch("backend.api.routes.review.StorageService"), \
+                patch("backend.api.routes.review._dev_audio_proxy_enabled", return_value=False), \
+                patch("backend.services.audio_transcoding_service.AudioTranscodingService.get_review_audio_url_async",
+                      side_effect=fake_sign):
+            mock_jm.return_value.get_job.return_value = mock_job
+            response = test_client.get("/api/review/job1/instrumental-urls")
+
+        assert response.status_code == 200
+        options = response.json()["instrumental_options"]
+        assert [o["id"] for o in options] == ["clean"]
+
+    def test_missing_job_returns_404(self, test_client):
+        with patch("backend.api.routes.review.JobManager") as mock_jm, \
+                patch("backend.api.routes.review.StorageService"):
+            mock_jm.return_value.get_job.return_value = None
+            response = test_client.get("/api/review/job1/instrumental-urls")
+        assert response.status_code == 404
+
+
 class TestReviewResubmissionClearsWorkerProgress:
     """Tests for ensuring review submission clears worker progress keys.
 

frontend/components/lyrics-review/PreviewVideoSection.tsx

@@ -35,8 +35,15 @@ interface ApiClient {
     message?: string
   }>
   getPreviewVideoUrl: (hash: string) => string
+  /** Re-fetch freshly-signed instrumental stem URLs (used when the baked-in
+   *  signed URL expires mid-review and the overlaid audio fails to load). */
+  refreshInstrumentalUrls?: () => Promise<InstrumentalOption[]>
 }
 
+// Cap how many times we silently re-fetch fresh signed URLs per modal open, so a
+// genuinely-broken stem (404, not just expiry) can't spin in a refresh→error loop.
+const MAX_URL_REFRESHES = 3
+
 export type InstrumentalChoice = 'clean' | 'with_backing'
 
 export interface PreviewVideoHandle {
@@ -94,7 +101,14 @@ function PreviewVideoSection(
   const isInstrumentalRef = useRef(false)
   isInstrumentalRef.current = isInstrumental
 
-  const options = instrumentalOptions?.filter((o) => o.audio_url) ?? []
+  // Signed stem URLs expire (120 min) and can outlive a long review session — the
+  // overlaid <audio> then fails to load. On that error we re-fetch fresh URLs and
+  // hold them here, overriding the (now-stale) prop options for playback.
+  const [refreshedOptions, setRefreshedOptions] = useState<InstrumentalOption[] | null>(null)
+  const refreshingRef = useRef(false)
+  const refreshCountRef = useRef(0)
+
+  const options = (refreshedOptions ?? instrumentalOptions)?.filter((o) => o.audio_url) ?? []
   const cleanOption = options.find((o) => o.id === 'clean')
 
   // Which instrumental stem the "Instrumental" preview pill plays. Defaults to the
@@ -289,6 +303,31 @@ function PreviewVideoSection(
     }
   }, [isInstrumental, previewState.status, instrumentalUrl, onTimeUpdate])
 
+  // Allow a fresh batch of refreshes each time the modal (re)opens.
+  useEffect(() => {
+    if (isModalOpen) refreshCountRef.current = 0
+  }, [isModalOpen])
+
+  // The overlaid stem failed to load — most often the baked-in signed URL expired
+  // during a long review session. Re-fetch freshly-signed URLs and swap them in;
+  // the sync effect (keyed on instrumentalUrl) then resumes playback in-place.
+  const handleInstrumentalAudioError = useCallback(() => {
+    if (refreshingRef.current) return
+    if (refreshCountRef.current >= MAX_URL_REFRESHES) return
+    const refresh = apiClient?.refreshInstrumentalUrls
+    if (!refresh) return
+    refreshingRef.current = true
+    refreshCountRef.current += 1
+    refresh()
+      .then((fresh) => {
+        if (fresh && fresh.length > 0) setRefreshedOptions(fresh)
+      })
+      .catch(() => {})
+      .finally(() => {
+        refreshingRef.current = false
+      })
+  }, [apiClient])
+
   useImperativeHandle(
     ref,
     () => ({
@@ -356,7 +395,12 @@ function PreviewVideoSection(
           </video>
           {instrumentalUrl && (
             // Hidden stem player kept in sync with the video for the audio toggle.
-            <audio ref={instrumentalAudioRef} src={instrumentalUrl} preload="auto" />
+            <audio
+              ref={instrumentalAudioRef}
+              src={instrumentalUrl}
+              preload="auto"
+              onError={handleInstrumentalAudioError}
+            />
           )}
         </div>
       )}