From 6ff8828d8bdab9e884db2d32afc6e49f9ae68468 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 13 May 2026 22:14:56 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(api):=20Track=203.3=20=E2=80=94=20GET?= =?UTF-8?q?=20/api/analysis-runs/{run=5Fid}/source-audio?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third of four pieces from Track 3. Adds a stable, round-trip-saving route for re-serving the original ingested audio for a run, so clients (Phase 2 reruns, external consumers) don't have to fetch the snapshot first to learn the artifact id. The same bytes are reachable today via GET /api/analysis-runs/{run_id}/artifacts/{artifact_id} (source audio is stored as an artifact with kind='source_audio'). This new route is an ergonomic shortcut, not a new capability: - Stable URL (doesn't change per run; the artifact id does) - Caller doesn't need to fetch the snapshot first - Identical bytes, identical Content-Disposition and MIME Auth model: - Owner-only via _resolve_route_user_context + runtime.get_run( owner_user_id=...). Standard read pattern. - Does NOT honor X-Admin-Key. Re-serving another user's audio has a stronger privacy posture than the cross-user delete admin bypass (Track 3.2). The review scoped admin to operations, not reads. - Non-owner request returns RUN_NOT_FOUND, matching the existing pattern that does not disclose whether a run exists. Error codes: - RUN_NOT_FOUND (404) — run missing or not owned. - SOURCE_AUDIO_NOT_FOUND (404) — defensive; would indicate the run row exists but source_artifact_id is null/dangling. Distinct from RUN_NOT_FOUND because the recovery path differs (server-side corruption vs ownership/typo). - SOURCE_AUDIO_FILE_MISSING (404) — artifact row intact but bytes gone from disk. Distinct from SOURCE_AUDIO_NOT_FOUND because the recovery path differs (re-ingest vs operator look). Files: - apps/backend/server.py — new route between the existing artifact- fetch route and the spectral-enhancements route. ~85 lines. - apps/backend/tests/test_server.py — 5 new tests in GetRunSourceAudioRouteTests covering: happy path with content- disposition; non-owner returns RUN_NOT_FOUND; unknown run returns RUN_NOT_FOUND; missing source artifact returns SOURCE_AUDIO_NOT_FOUND; missing file on disk returns SOURCE_AUDIO_FILE_MISSING. - apps/backend/ARCHITECTURE.md — documents the new route. Out of scope (deferred): - Range requests / partial content for streaming. The FileResponse doesn't currently support Range; sufficient for the audio sizes ASA handles (100 MiB cap). - Frontend UI that uses this endpoint. Phase 2 reruns are the intended consumer; UI wiring is a follow-up. https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg --- apps/backend/ARCHITECTURE.md | 1 + apps/backend/server.py | 88 +++++++++++++++ apps/backend/tests/test_server.py | 178 ++++++++++++++++++++++++++++++ 3 files changed, 267 insertions(+) diff --git a/apps/backend/ARCHITECTURE.md b/apps/backend/ARCHITECTURE.md index 7c86de5a..294f82c9 100644 --- a/apps/backend/ARCHITECTURE.md +++ b/apps/backend/ARCHITECTURE.md @@ -72,6 +72,7 @@ Custom routes: - `GET /api/analysis-runs/{run_id}` - `DELETE /api/analysis-runs/{run_id}` — owner can delete their own run; operators with `SONIC_ANALYZER_ADMIN_KEY` set can supply `X-Admin-Key` to delete any run. Admin path is closed when the env var is unset. - `GET /api/analysis-runs/{run_id}/artifacts` and `…/artifacts/{artifact_id}` +- `GET /api/analysis-runs/{run_id}/source-audio` — re-serves the original ingested audio for the run. Owner-only (no admin bypass). Saves a round-trip vs looking up the source artifact id first. - `GET /api/analysis-runs/{run_id}/export/csv/{field_path}` — CSV export of a Phase 1 time-series field. See [`docs/adr/0001-phase1-json-schema-v1.md`](../../docs/adr/0001-phase1-json-schema-v1.md) and the registry in [`csv_export.py`](csv_export.py). - `POST /api/analysis-runs/{run_id}/spectral-enhancements/{kind}` - `POST /api/analysis-runs/{run_id}/pitch-note-translations` diff --git a/apps/backend/server.py b/apps/backend/server.py index f3aa4870..61d0c1d7 100644 --- a/apps/backend/server.py +++ b/apps/backend/server.py @@ -2271,6 +2271,94 @@ async def get_run_artifact( ) +@app.get( + "/api/analysis-runs/{run_id}/source-audio", + response_model=None, +) +async def get_run_source_audio( + run_id: str, + x_asa_user_id: str | None = Header(None), + x_asa_user_email: str | None = Header(None), +) -> FileResponse | JSONResponse: + """Re-serve the audio file originally ingested for ``run_id``. + + The same bytes are reachable via + ``GET /api/analysis-runs/{run_id}/artifacts/{artifact_id}`` if the + caller already knows the artifact id, but this route is stable + (the path doesn't change per run) and saves a round-trip — the + caller does not need to fetch the snapshot first to learn the + artifact id. Useful for Phase 2 reruns where the client already + has the run id but no longer has the source bytes locally. + + Owner-only. Unlike ``DELETE``, this route does not honor + ``X-Admin-Key``: re-serving another user's audio content has a + stronger privacy posture than purging another user's run, and the + review (Track 3) scoped admin bypass to delete operations. + + Returns: + - 200 with the audio file (FileResponse with the original + filename in Content-Disposition and the stored MIME type). + - 404 ``RUN_NOT_FOUND`` if the run does not exist or is not + owned by the requesting user. + - 404 ``SOURCE_AUDIO_NOT_FOUND`` if the run exists but has no + source_artifact row (should be unreachable in practice; it + would indicate a corrupted runtime state). + - 404 ``SOURCE_AUDIO_FILE_MISSING`` if the artifact row exists + but the underlying file is gone from disk. + """ + user_context = _resolve_route_user_context(x_asa_user_id, x_asa_user_email) + if isinstance(user_context, JSONResponse): + return user_context + runtime = get_analysis_runtime() + + # Ownership check first; this also confirms the run exists. + try: + runtime.get_run(run_id, owner_user_id=user_context.user_id) + except (KeyError, PermissionError): + return _run_not_found_response(run_id) + + try: + source_artifact = runtime.get_source_artifact(run_id) + except KeyError: + # Run was found by get_run() above, but the source_artifact_id + # column was null or pointed at a missing row. Defensive — not + # expected on the normal create_run path. + return JSONResponse( + status_code=404, + content={ + "error": { + "code": "SOURCE_AUDIO_NOT_FOUND", + "message": ( + f"Run '{run_id}' has no source audio artifact " + f"registered." + ), + } + }, + ) + + artifact_local_path = runtime.resolve_artifact_local_path( + source_artifact.get("path") + ) + if artifact_local_path is None or not artifact_local_path.is_file(): + return JSONResponse( + status_code=404, + content={ + "error": { + "code": "SOURCE_AUDIO_FILE_MISSING", + "message": ( + "Source audio file is no longer available on disk." + ), + } + }, + ) + + return FileResponse( + path=str(artifact_local_path), + media_type=source_artifact.get("mimeType", "application/octet-stream"), + filename=source_artifact.get("filename", artifact_local_path.name), + ) + + @app.get( "/api/analysis-runs/{run_id}/export/csv/{field_path}", response_model=None, diff --git a/apps/backend/tests/test_server.py b/apps/backend/tests/test_server.py index 5d212b27..00ae6801 100644 --- a/apps/backend/tests/test_server.py +++ b/apps/backend/tests/test_server.py @@ -4620,6 +4620,184 @@ def test_url_invalid_envelope_is_not_retryable(self) -> None: self.assertFalse(payload["error"]["retryable"]) +class GetRunSourceAudioRouteTests(unittest.TestCase): + """Route tests for GET /api/analysis-runs/{run_id}/source-audio. + + The route re-serves the original ingested audio for a run so callers + don't have to re-upload for Phase 2 reruns. These tests cover the + HTTP shell: ownership enforcement, missing-source handling, and the + happy-path body/headers. + """ + + AUDIO_BYTES = b"FAKE-AUDIO-PAYLOAD" + + def _decode_json_response(self, response) -> dict: + return json.loads(response.body.decode("utf-8")) + + def _make_owned_run(self, runtime, *, owner: str = "owner_user") -> str: + created = runtime.create_run( + filename="reference.mp3", + content=self.AUDIO_BYTES, + mime_type="audio/mpeg", + owner_user_id=owner, + analysis_mode="full", + pitch_note_mode="off", + pitch_note_backend="auto", + interpretation_mode="off", + interpretation_profile="producer_summary", + interpretation_model=None, + ) + return created["runId"] + + def test_serves_audio_with_filename_and_mime(self) -> None: + from analysis_runtime import AnalysisRuntime + + with tempfile.TemporaryDirectory(prefix="asa_source_audio_") as temp_dir: + runtime = AnalysisRuntime(Path(temp_dir) / "runtime") + run_id = self._make_owned_run(runtime) + + with patch.object(server, "get_analysis_runtime", return_value=runtime), \ + patch.dict( + server.os.environ, + {"SONIC_ANALYZER_RUNTIME_PROFILE": "hosted"}, + clear=False, + ): + response = asyncio.run( + server.get_run_source_audio( + run_id, + x_asa_user_id="owner_user", + ) + ) + + # FileResponse points at the on-disk artifact; verify metadata. + self.assertEqual(response.status_code, 200) + # FileResponse.media_type is the configured mime + self.assertEqual(response.media_type, "audio/mpeg") + # The Content-Disposition filename comes from the original + # upload filename, not the stored path. + disposition = response.headers.get("content-disposition", "") + self.assertIn("reference.mp3", disposition) + + def test_non_owner_request_returns_run_not_found(self) -> None: + """A different user MUST NOT be able to fetch another user's + source audio. Identical 404 envelope to a missing run — the + route does not disclose whether the run exists. + """ + from analysis_runtime import AnalysisRuntime + + with tempfile.TemporaryDirectory(prefix="asa_source_audio_") as temp_dir: + runtime = AnalysisRuntime(Path(temp_dir) / "runtime") + run_id = self._make_owned_run(runtime, owner="owner_user") + + with patch.object(server, "get_analysis_runtime", return_value=runtime), \ + patch.dict( + server.os.environ, + {"SONIC_ANALYZER_RUNTIME_PROFILE": "hosted"}, + clear=False, + ): + response = asyncio.run( + server.get_run_source_audio( + run_id, + x_asa_user_id="not_the_owner", + ) + ) + + self.assertEqual(response.status_code, 404) + payload = self._decode_json_response(response) + self.assertEqual(payload["error"]["code"], "RUN_NOT_FOUND") + + def test_unknown_run_returns_404(self) -> None: + from analysis_runtime import AnalysisRuntime + + with tempfile.TemporaryDirectory(prefix="asa_source_audio_") as temp_dir: + runtime = AnalysisRuntime(Path(temp_dir) / "runtime") + with patch.object(server, "get_analysis_runtime", return_value=runtime): + response = asyncio.run( + server.get_run_source_audio( + "does-not-exist", + ) + ) + + self.assertEqual(response.status_code, 404) + payload = self._decode_json_response(response) + self.assertEqual(payload["error"]["code"], "RUN_NOT_FOUND") + + def test_missing_source_artifact_returns_source_audio_not_found(self) -> None: + """If the run row exists but get_source_artifact raises (no + source_artifact_id linkage), the route surfaces the dedicated + SOURCE_AUDIO_NOT_FOUND code rather than the generic RUN_NOT_FOUND. + + Distinct error codes matter because the recovery path differs: + RUN_NOT_FOUND → ownership / typo issue, the client should check + the id; SOURCE_AUDIO_NOT_FOUND → server-side corruption, the + client cannot recover, an operator needs to look. + """ + from analysis_runtime import AnalysisRuntime + + with tempfile.TemporaryDirectory(prefix="asa_source_audio_") as temp_dir: + runtime = AnalysisRuntime(Path(temp_dir) / "runtime") + run_id = self._make_owned_run(runtime, owner="owner_user") + + # Patch get_source_artifact to simulate missing-linkage path. + with patch.object(server, "get_analysis_runtime", return_value=runtime), \ + patch.object( + runtime, + "get_source_artifact", + side_effect=KeyError(f"Run {run_id} is missing its source artifact"), + ), \ + patch.dict( + server.os.environ, + {"SONIC_ANALYZER_RUNTIME_PROFILE": "hosted"}, + clear=False, + ): + response = asyncio.run( + server.get_run_source_audio( + run_id, + x_asa_user_id="owner_user", + ) + ) + + self.assertEqual(response.status_code, 404) + payload = self._decode_json_response(response) + self.assertEqual(payload["error"]["code"], "SOURCE_AUDIO_NOT_FOUND") + + def test_missing_file_on_disk_returns_source_audio_file_missing(self) -> None: + """If the artifact row exists but resolve_artifact_local_path + returns None or the file isn't on disk, surface + SOURCE_AUDIO_FILE_MISSING. Distinct from SOURCE_AUDIO_NOT_FOUND + because the recovery is different: the artifact metadata is + intact, only the bytes are gone — operator can re-ingest from + the original source. + """ + from analysis_runtime import AnalysisRuntime + + with tempfile.TemporaryDirectory(prefix="asa_source_audio_") as temp_dir: + runtime = AnalysisRuntime(Path(temp_dir) / "runtime") + run_id = self._make_owned_run(runtime, owner="owner_user") + + with patch.object(server, "get_analysis_runtime", return_value=runtime), \ + patch.object( + runtime, + "resolve_artifact_local_path", + return_value=None, + ), \ + patch.dict( + server.os.environ, + {"SONIC_ANALYZER_RUNTIME_PROFILE": "hosted"}, + clear=False, + ): + response = asyncio.run( + server.get_run_source_audio( + run_id, + x_asa_user_id="owner_user", + ) + ) + + self.assertEqual(response.status_code, 404) + payload = self._decode_json_response(response) + self.assertEqual(payload["error"]["code"], "SOURCE_AUDIO_FILE_MISSING") + + class CsvExportRouteTests(unittest.TestCase): """Route tests for GET /api/analysis-runs/{run_id}/export/csv/{field_path}. From c22454aeec7b46fb794ad9c5732bd30c53e16a53 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 13 May 2026 22:19:32 +0000 Subject: [PATCH 2/3] fix(api): add retryable=false to source-audio error envelopes; test consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses PR #40 review: 1. Should fix: SOURCE_AUDIO_NOT_FOUND and SOURCE_AUDIO_FILE_MISSING error envelopes now carry the 'retryable' boolean per the CLAUDE.md contract. Both are False — corrupted run state and missing-on-disk bytes don't recover from a naive retry; operator action is required. The contract also calls for requestId and diagnostics; those are omitted across the existing run-management routes (ARTIFACT_NOT_FOUND, ARTIFACT_FILE_MISSING, AUTHENTICATION_REQUIRED), and bringing the whole surface into full compliance is out of scope for this PR. Adding retryable here keeps the new code from drifting further. 2. Worth considering: test_unknown_run_returns_404 didn't patch SONIC_ANALYZER_RUNTIME_PROFILE the way the other four tests in the class do. Functionally correct (local profile auto-assigns local-dev) but inconsistent. Now patches to hosted for setup parity. 3. Not fixed (per reviewer's 'practically unreachable'): the delete/fetch race that maps to SOURCE_AUDIO_NOT_FOUND instead of RUN_NOT_FOUND. Disambiguation requires either string-matching the KeyError message (fragile) or adding owner_user_id threading to get_source_artifact. Both are real cost for an unreachable case. The existing SOURCE_AUDIO_NOT_FOUND / SOURCE_AUDIO_FILE_MISSING route tests now assert the retryable field, so a future regression (removing it) would fail. https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg --- apps/backend/server.py | 9 ++++++++- apps/backend/tests/test_server.py | 13 ++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/apps/backend/server.py b/apps/backend/server.py index 61d0c1d7..67d10859 100644 --- a/apps/backend/server.py +++ b/apps/backend/server.py @@ -2322,7 +2322,9 @@ async def get_run_source_audio( except KeyError: # Run was found by get_run() above, but the source_artifact_id # column was null or pointed at a missing row. Defensive — not - # expected on the normal create_run path. + # expected on the normal create_run path. ``retryable`` is + # ``False`` because the run state is corrupted; a retry on the + # same id will fail identically. return JSONResponse( status_code=404, content={ @@ -2332,6 +2334,7 @@ async def get_run_source_audio( f"Run '{run_id}' has no source audio artifact " f"registered." ), + "retryable": False, } }, ) @@ -2348,6 +2351,10 @@ async def get_run_source_audio( "message": ( "Source audio file is no longer available on disk." ), + # The artifact metadata is intact but the bytes are + # gone; an operator would need to re-ingest. A naive + # retry from the client will not recover. + "retryable": False, } }, ) diff --git a/apps/backend/tests/test_server.py b/apps/backend/tests/test_server.py index 00ae6801..90154dd9 100644 --- a/apps/backend/tests/test_server.py +++ b/apps/backend/tests/test_server.py @@ -4711,10 +4711,16 @@ def test_unknown_run_returns_404(self) -> None: with tempfile.TemporaryDirectory(prefix="asa_source_audio_") as temp_dir: runtime = AnalysisRuntime(Path(temp_dir) / "runtime") - with patch.object(server, "get_analysis_runtime", return_value=runtime): + with patch.object(server, "get_analysis_runtime", return_value=runtime), \ + patch.dict( + server.os.environ, + {"SONIC_ANALYZER_RUNTIME_PROFILE": "hosted"}, + clear=False, + ): response = asyncio.run( server.get_run_source_audio( "does-not-exist", + x_asa_user_id="some_user", ) ) @@ -4760,6 +4766,8 @@ def test_missing_source_artifact_returns_source_audio_not_found(self) -> None: self.assertEqual(response.status_code, 404) payload = self._decode_json_response(response) self.assertEqual(payload["error"]["code"], "SOURCE_AUDIO_NOT_FOUND") + # Server-side corruption — naive client retry won't recover. + self.assertFalse(payload["error"]["retryable"]) def test_missing_file_on_disk_returns_source_audio_file_missing(self) -> None: """If the artifact row exists but resolve_artifact_local_path @@ -4796,6 +4804,9 @@ def test_missing_file_on_disk_returns_source_audio_file_missing(self) -> None: self.assertEqual(response.status_code, 404) payload = self._decode_json_response(response) self.assertEqual(payload["error"]["code"], "SOURCE_AUDIO_FILE_MISSING") + # File bytes missing — operator must re-ingest; client retry + # of the same id will not recover. + self.assertFalse(payload["error"]["retryable"]) class CsvExportRouteTests(unittest.TestCase): From 4e562676bf7d063b062d27dcf4c13e18138a7869 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 13 May 2026 22:27:57 +0000 Subject: [PATCH 3/3] ci: retrigger Frontend CI (suspected env flake) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Frontend CI on the previous commit failed after 1m 53s, which is too short for smoke tests to have executed — npm ci + playwright install --with-deps chromium typically take 60-90 s combined before 'npm run verify' even starts. Local checks all pass: npm run lint (clean), npm run test:unit (398/398), npm run build (clean). Backend CI on the same workflow passed at 4m 21s. My local apt-get failed when installing Playwright system deps ('ppa:ondrej/php repository no longer signed'), suggesting a similar transient registry/apt blip caused the CI failure. Empty commit retriggers CI to confirm. Smoke tests stub all backend routes via page.route() in their specs, so the new GET /api/analysis-runs/{run_id}/source-audio route this PR adds cannot affect them. https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg