From 6e952b452f04325917b5dd99d746b4fea92f2fae Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 14 May 2026 02:47:33 +0000 Subject: [PATCH] =?UTF-8?q?feat(api):=20Track=203.4=20=E2=80=94=20additive?= =?UTF-8?q?=20publicStatus=20field=20on=20every=20stage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth and final piece of Track 3 from the merged external-repo review. Additive over the existing 8-state stage status — does not change or remove anything. What's new: - apps/backend/stage_status.py (new) — pure module with the canonical 8 -> 5 mapping table, the to_public_status() helper, and PUBLIC_STATUS_VALUES exported for type generation / docs. Mapping (internal -> public): queued -> queued running -> running blocked -> queued (transient scheduling state) ready -> queued (scheduled but not yet running) completed -> completed failed -> failed interrupted -> interrupted not_requested -> null (stage not in pipeline) - apps/backend/server_phase1.py — _normalize_run_snapshot now also annotates each stage with publicStatus via the new _annotate_public_status helper. The original status field is preserved untouched. - apps/backend/tests/test_stage_status.py (new, 16 tests) — pins every cell of the mapping, the public-value set, defensive cases (None, unknown, empty, case-sensitive), and a completeness check that fails if the set of internal states drifts without an explicit mapping update. - apps/backend/tests/test_server.py (+1 test) — integration test that GET /api/analysis-runs/{run_id} returns publicStatus on all three stages with the correct values for a fresh run. - apps/ui/src/types/backend.ts — new PublicStageStatus type (the 5-value union). publicStatus: PublicStageStatus | null added to MeasurementStageSnapshot, PitchNoteTranslationStageSnapshot, InterpretationStageSnapshot. attempt summaries unchanged (publicStatus is only on top-level stages). - apps/ui/src/services/analysisRunsClient.ts — new parsePublicStageStatus helper + PUBLIC_STAGE_STATUSES set; threaded into the three stage parsers. The helper accepts null and missing values without throwing so legacy snapshots that pre-date the field continue to deserialize. - docs/adr/0001-phase1-json-schema-v1.md — documents the additive field, the mapping table, and the rationale (internal scheduling states are scheduling concerns the client doesn't need to act on). Out of scope (deferred): - Removing publicStatus duplication from the internal status field later. The additive approach was the explicit choice; collapsing to one field would be a breaking change requiring a v2 schema bump. - Frontend UI that reads publicStatus. Existing UI continues to use the 8-state status field; publicStatus is the additive surface for future external consumers and any new UI views that prefer the smaller vocabulary. This closes Track 3 (all four pieces shipped): - 3.1 URL ingestion (#36, merged) - 3.2 X-Admin-Key on DELETE (#37, merged) - 3.3 GET /source-audio re-serve (#40, merged) - 3.4 publicStatus additive (this PR) https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg --- apps/backend/server_phase1.py | 48 +++++-- apps/backend/stage_status.py | 88 +++++++++++++ apps/backend/tests/test_server.py | 42 +++++++ apps/backend/tests/test_stage_status.py | 140 +++++++++++++++++++++ apps/ui/src/services/analysisRunsClient.ts | 30 +++++ apps/ui/src/types/backend.ts | 30 +++++ docs/adr/0001-phase1-json-schema-v1.md | 11 ++ 7 files changed, 380 insertions(+), 9 deletions(-) create mode 100644 apps/backend/stage_status.py create mode 100644 apps/backend/tests/test_stage_status.py diff --git a/apps/backend/server_phase1.py b/apps/backend/server_phase1.py index 401aa3f9..dee30da3 100644 --- a/apps/backend/server_phase1.py +++ b/apps/backend/server_phase1.py @@ -9,6 +9,7 @@ from analysis_runtime import AnalysisRuntime from server_upload import ERROR_PHASE_LOCAL_DSP +from stage_status import to_public_status # ── Constants ──────────────────────────────────────────────────────────────── @@ -226,6 +227,30 @@ def _build_phase1(payload: dict[str, Any]) -> dict[str, Any]: } +def _annotate_public_status(stages: dict[str, Any]) -> dict[str, Any]: + """Attach ``publicStatus`` to every stage whose ``status`` is set. + + Returns a shallow-copy of ``stages`` with each stage dict updated. + The original ``status`` field is preserved untouched; ``publicStatus`` + is the additive 5-state collapse documented in + :mod:`stage_status`. + + Defensive against non-dict stage entries (e.g. nulls in legacy + snapshots) — those pass through unchanged. + """ + annotated: dict[str, Any] = {} + for stage_name, stage_value in stages.items(): + if not isinstance(stage_value, dict): + annotated[stage_name] = stage_value + continue + stage_copy = dict(stage_value) + # `status` may legitimately be absent on legacy/partial snapshots; + # to_public_status(None) yields None which we expose as null. + stage_copy["publicStatus"] = to_public_status(stage_copy.get("status")) + annotated[stage_name] = stage_copy + return annotated + + def _normalize_run_snapshot( snapshot: dict[str, Any], runtime: AnalysisRuntime | None = None ) -> dict[str, Any]: @@ -236,22 +261,27 @@ def _normalize_run_snapshot( _build_phase1 produces for the legacy /api/analyze endpoint — notably, top-level stereoWidth/stereoCorrelation extracted from stereoDetail. + Each stage in ``stages`` is also annotated with a ``publicStatus`` field + that collapses the 8 internal stage statuses to 5 public ones (see + :mod:`stage_status`). The original ``status`` field is preserved; this + is purely additive. + When *runtime* is provided, spectral visualization artifacts (spectrogram PNGs and time-series JSON) are attached under ``artifacts.spectral``. """ stages = snapshot.get("stages") if not isinstance(stages, dict): return snapshot - measurement = stages.get("measurement") - if not isinstance(measurement, dict): - return snapshot - raw_result = measurement.get("result") - if not isinstance(raw_result, dict): - return snapshot snapshot = dict(snapshot) - snapshot["stages"] = dict(stages) - snapshot["stages"]["measurement"] = dict(measurement) - snapshot["stages"]["measurement"]["result"] = _build_phase1(raw_result) + snapshot["stages"] = _annotate_public_status(stages) + + measurement = snapshot["stages"].get("measurement") + if isinstance(measurement, dict): + raw_result = measurement.get("result") + if isinstance(raw_result, dict): + # Reuse the dict we already copied in _annotate_public_status; + # no need to clone twice. + measurement["result"] = _build_phase1(raw_result) if runtime is not None: run_id = snapshot.get("runId") diff --git a/apps/backend/stage_status.py b/apps/backend/stage_status.py new file mode 100644 index 00000000..eea88e0c --- /dev/null +++ b/apps/backend/stage_status.py @@ -0,0 +1,88 @@ +"""Public-facing collapse of the internal stage-status state machine. + +ASA's run-state machine has eight internal statuses +(``queued``, ``running``, ``blocked``, ``ready``, ``completed``, +``failed``, ``interrupted``, ``not_requested``). Most of these are +runtime-scheduling concerns the client does not need to act on: + +- ``blocked`` and ``ready`` are transient states between "waiting on + a dependency" and "scheduled to run." Both look identical to a + client polling the snapshot — there's nothing to display or do + differently. +- ``not_requested`` means the caller did not ask for this stage to + run (e.g. ``pitch_note_mode="off"``). Conceptually distinct from + "queued"; the stage is simply absent from the requested pipeline. + +This module exposes the five-state vocabulary intended for external +consumers and the mapping from the internal eight to that public +five. The internal ``status`` field on each stage stays untouched; +this is an *additive* collapse — the route layer attaches a parallel +``publicStatus`` field next to ``status`` on each stage. + +Mapping: + +============== =============================== +Internal Public (publicStatus) +============== =============================== +queued queued +running running +blocked queued (transient internal) +ready queued (scheduled, not yet running) +completed completed +failed failed +interrupted interrupted +not_requested None (not in the pipeline) +============== =============================== + +A ``None`` mapping is exposed in JSON as ``"publicStatus": null``. +This is deliberate — explicit ``null`` is easier for clients than +checking key presence, especially in strongly-typed languages where +optional vs missing are different shapes. + +See ``docs/adr/0001-phase1-json-schema-v1.md`` for the schema-version +treatment of this field. +""" + +from __future__ import annotations + +from typing import Final + + +PUBLIC_STATUS_VALUES: Final[frozenset[str]] = frozenset( + {"queued", "running", "completed", "failed", "interrupted"} +) + + +_INTERNAL_TO_PUBLIC: Final[dict[str, str | None]] = { + "queued": "queued", + "running": "running", + "blocked": "queued", + "ready": "queued", + "completed": "completed", + "failed": "failed", + "interrupted": "interrupted", + "not_requested": None, +} + + +def to_public_status(internal_status: str | None) -> str | None: + """Map an internal stage status to its public-facing equivalent. + + Returns ``None`` when: + - ``internal_status`` is ``None`` (stage exists but no status set) + - ``internal_status`` is ``"not_requested"`` (stage is not in the pipeline) + - ``internal_status`` is an unrecognized value (defensive — should not + happen with the current state machine, but a forwards-compat + guard against accidental internal-only additions leaking out) + """ + if internal_status is None: + return None + return _INTERNAL_TO_PUBLIC.get(internal_status) + + +def public_status_values() -> frozenset[str]: + """The complete set of non-null public status values. + + Useful for type generation, validation, and documentation. + """ + return PUBLIC_STATUS_VALUES diff --git a/apps/backend/tests/test_server.py b/apps/backend/tests/test_server.py index 90154dd9..3b46e449 100644 --- a/apps/backend/tests/test_server.py +++ b/apps/backend/tests/test_server.py @@ -1289,6 +1289,48 @@ def test_get_analysis_run_returns_persisted_stage_snapshot(self) -> None: self.assertEqual(payload["runId"], created["runId"]) self.assertIn("artifacts", payload) + def test_get_analysis_run_attaches_public_status_to_every_stage(self) -> None: + """Track 3.4: every stage in the response carries publicStatus + alongside the internal status. Pitch-note and interpretation + stages here are not_requested (publicStatus must be null); + measurement stage is queued (publicStatus must be queued). + """ + from analysis_runtime import AnalysisRuntime + + with tempfile.TemporaryDirectory(prefix="asa_server_runtime_") as temp_dir: + runtime = AnalysisRuntime(Path(temp_dir) / "runtime") + created = runtime.create_run( + filename="track.mp3", + content=b"fake-audio", + mime_type="audio/mpeg", + pitch_note_mode="off", + pitch_note_backend="auto", + interpretation_mode="off", + interpretation_profile="producer_summary", + interpretation_model=None, + ) + with patch.object(server, "get_analysis_runtime", return_value=runtime): + response = asyncio.run(server.get_analysis_run(created["runId"])) + + payload = self._decode_json_response(response) + stages = payload["stages"] + + # measurement starts queued; publicStatus should match + self.assertEqual(stages["measurement"]["status"], "queued") + self.assertEqual(stages["measurement"]["publicStatus"], "queued") + + # not_requested stages must surface publicStatus: null so a + # client can distinguish "you didn't ask for this" from + # "queued and waiting" without checking a specific string. + self.assertEqual( + stages["pitchNoteTranslation"]["status"], "not_requested" + ) + self.assertIsNone(stages["pitchNoteTranslation"]["publicStatus"]) + self.assertEqual( + stages["interpretation"]["status"], "not_requested" + ) + self.assertIsNone(stages["interpretation"]["publicStatus"]) + def test_interrupt_analysis_run_marks_stages_interrupted_and_reports_terminated_children(self) -> None: from analysis_runtime import AnalysisRuntime diff --git a/apps/backend/tests/test_stage_status.py b/apps/backend/tests/test_stage_status.py new file mode 100644 index 00000000..5d182f4c --- /dev/null +++ b/apps/backend/tests/test_stage_status.py @@ -0,0 +1,140 @@ +"""Unit tests for stage_status — the 8 → 5 public-status collapse. + +The collapse is the documented contract for ``publicStatus`` on every +stage object in the run snapshot. Tests here pin the mapping cell-by-cell +so a quiet edit to the lookup table is caught. +""" + +import sys +import unittest +from pathlib import Path + + +_BACKEND_ROOT = Path(__file__).resolve().parents[1] +if str(_BACKEND_ROOT) not in sys.path: + sys.path.insert(0, str(_BACKEND_ROOT)) + + +import stage_status # noqa: E402 — load after sys.path is set + + +class PublicStatusValuesTests(unittest.TestCase): + """The five public-facing values, exhaustively.""" + + def test_exact_five_values(self): + self.assertEqual( + stage_status.PUBLIC_STATUS_VALUES, + frozenset({"queued", "running", "completed", "failed", "interrupted"}), + ) + + def test_helper_returns_same_set(self): + self.assertEqual( + stage_status.public_status_values(), + stage_status.PUBLIC_STATUS_VALUES, + ) + + +class ToPublicStatusTests(unittest.TestCase): + """Cell-by-cell mapping pins for the eight internal states.""" + + def test_queued_maps_to_queued(self): + self.assertEqual(stage_status.to_public_status("queued"), "queued") + + def test_running_maps_to_running(self): + self.assertEqual(stage_status.to_public_status("running"), "running") + + def test_blocked_maps_to_queued(self): + """``blocked`` is transient internal scheduling; clients see it + the same as ``queued``.""" + self.assertEqual(stage_status.to_public_status("blocked"), "queued") + + def test_ready_maps_to_queued(self): + """``ready`` means scheduled but not yet started; same as queued + from the client's perspective.""" + self.assertEqual(stage_status.to_public_status("ready"), "queued") + + def test_completed_maps_to_completed(self): + self.assertEqual(stage_status.to_public_status("completed"), "completed") + + def test_failed_maps_to_failed(self): + self.assertEqual(stage_status.to_public_status("failed"), "failed") + + def test_interrupted_maps_to_interrupted(self): + self.assertEqual( + stage_status.to_public_status("interrupted"), "interrupted" + ) + + def test_not_requested_maps_to_none(self): + """``not_requested`` means the caller didn't ask for this stage; + conceptually distinct from queued. Exposed as ``null`` in JSON.""" + self.assertIsNone(stage_status.to_public_status("not_requested")) + + +class ToPublicStatusDefensiveTests(unittest.TestCase): + """Defensive cases: nones, unknowns, and the empty string.""" + + def test_none_returns_none(self): + self.assertIsNone(stage_status.to_public_status(None)) + + def test_unknown_internal_status_returns_none(self): + """A new internal-only state added without a public mapping + should fall through as None rather than leak through. This is + the forwards-compat guard.""" + self.assertIsNone(stage_status.to_public_status("does_not_exist")) + + def test_empty_string_returns_none(self): + self.assertIsNone(stage_status.to_public_status("")) + + def test_case_sensitive(self): + """The internal vocabulary is lowercase. Uppercase shouldn't + match — the mapping table is case-sensitive by design.""" + self.assertIsNone(stage_status.to_public_status("QUEUED")) + self.assertIsNone(stage_status.to_public_status("Running")) + + +class MappingCompletenessTests(unittest.TestCase): + """Pins the set of internal states the mapping covers. + + If a new internal status is added without a corresponding entry + here, this test fails. If an old one is removed, this test fails. + Either way, the developer must make a deliberate choice about the + public mapping rather than silently drift. + """ + + EXPECTED_INTERNAL_STATES = frozenset( + { + "queued", + "running", + "blocked", + "ready", + "completed", + "failed", + "interrupted", + "not_requested", + } + ) + + def test_internal_states_unchanged(self): + actual = frozenset(stage_status._INTERNAL_TO_PUBLIC.keys()) + self.assertEqual(actual, self.EXPECTED_INTERNAL_STATES) + + def test_all_non_null_mappings_are_public_values(self): + """Every non-None target of the mapping must be in + PUBLIC_STATUS_VALUES — no smuggling new public states in via + the mapping table without updating the canonical set.""" + for internal, public in stage_status._INTERNAL_TO_PUBLIC.items(): + if public is None: + continue + self.assertIn( + public, + stage_status.PUBLIC_STATUS_VALUES, + msg=( + f"{internal!r} maps to {public!r}, which is not in " + f"PUBLIC_STATUS_VALUES. Either add it to the public " + f"set or change the mapping." + ), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/apps/ui/src/services/analysisRunsClient.ts b/apps/ui/src/services/analysisRunsClient.ts index 24f86fae..2ad68371 100644 --- a/apps/ui/src/services/analysisRunsClient.ts +++ b/apps/ui/src/services/analysisRunsClient.ts @@ -18,6 +18,7 @@ import { StemSummaryResult, PitchNoteTranslationAttemptSummary, PitchNoteTranslationStageSnapshot, + PublicStageStatus, } from '../types'; import { parsePhase1Result } from './backendPhase1Client'; import { requestBackendEstimate } from './backendPhase1Client'; @@ -60,6 +61,17 @@ const ANALYSIS_RUN_STATUSES = new Set([ 'not_requested', ]); +// Mirror of stage_status.PUBLIC_STATUS_VALUES on the backend. Five-value +// collapse exposed as the additive `publicStatus` field; the Python +// source of truth is `apps/backend/stage_status.py`. +const PUBLIC_STAGE_STATUSES = new Set([ + 'queued', + 'running', + 'completed', + 'failed', + 'interrupted', +]); + export async function estimateAnalysisRun( file: File, options: EstimateAnalysisRunOptions, @@ -288,6 +300,7 @@ function parseAnalysisRunSnapshot(value: unknown): AnalysisRunSnapshot { stages: { measurement: { status: expectStageStatus(measurement.status), + publicStatus: parsePublicStageStatus(measurement.publicStatus), authoritative: true, result: measurement.result == null ? null : parseCanonicalMeasurementResult(measurement.result), provenance: parseNullableRecord(measurement.provenance), @@ -354,6 +367,7 @@ function parseSpectralArtifacts(value: unknown): SpectralArtifacts { function parsePitchNoteStage(value: Record): PitchNoteTranslationStageSnapshot { return { status: expectStageStatus(value.status), + publicStatus: parsePublicStageStatus(value.publicStatus), authoritative: false, preferredAttemptId: asString(value.preferredAttemptId), attemptsSummary: Array.isArray(value.attemptsSummary) @@ -385,6 +399,7 @@ function parseInterpretationStage(value: Record): Interpretatio : undefined; return { status: expectStageStatus(value.status), + publicStatus: parsePublicStageStatus(value.publicStatus), authoritative: false, preferredAttemptId: asString(value.preferredAttemptId), attemptsSummary, @@ -684,6 +699,21 @@ function expectStageStatus(value: unknown): AnalysisStageStatus { return value as AnalysisStageStatus; } +/** + * Parse the additive `publicStatus` field on a stage. Returns `null` for + * either a JSON null (the documented "not in pipeline" signal) or for + * missing values on legacy snapshots that pre-date the field — callers + * that need the public collapse can use this without crashing on + * older runs. + */ +function parsePublicStageStatus(value: unknown): PublicStageStatus | null { + if (value === null || value === undefined) return null; + if (typeof value !== 'string') return null; + return PUBLIC_STAGE_STATUSES.has(value as PublicStageStatus) + ? (value as PublicStageStatus) + : null; +} + function asString(value: unknown): string | null { return typeof value === 'string' && value.trim() !== '' ? value : null; } diff --git a/apps/ui/src/types/backend.ts b/apps/ui/src/types/backend.ts index ef172b30..f6ef3759 100644 --- a/apps/ui/src/types/backend.ts +++ b/apps/ui/src/types/backend.ts @@ -40,6 +40,30 @@ export type AnalysisStageStatus = | 'interrupted' | 'not_requested'; +/** + * The 5-value collapse of {@link AnalysisStageStatus} that the response + * boundary attaches as `publicStatus` on every stage. + * + * Mapping (from `apps/backend/stage_status.py`): + * - queued | blocked | ready → `'queued'` + * - running → `'running'` + * - completed → `'completed'` + * - failed → `'failed'` + * - interrupted → `'interrupted'` + * - not_requested → `null` (stage exists but not in pipeline) + * + * Additive over `status` — the original 8-state field is preserved on + * every stage. New code that doesn't need to distinguish blocked-vs- + * queued or ready-vs-queued can read `publicStatus` and ignore the + * internal vocabulary. + */ +export type PublicStageStatus = + | 'queued' + | 'running' + | 'completed' + | 'failed' + | 'interrupted'; + export interface AnalysisStageError { code: string; message: string; @@ -124,6 +148,8 @@ export interface MeasurementAvailabilityContext { export interface MeasurementStageSnapshot { status: AnalysisStageStatus; + /** Additive 5-value collapse of `status`. `null` when status is `not_requested`. */ + publicStatus: PublicStageStatus | null; authoritative: true; result: MeasurementResult | null; provenance: Record | null; @@ -147,6 +173,8 @@ export interface InterpretationAttemptSummary { export interface PitchNoteTranslationStageSnapshot { status: AnalysisStageStatus; + /** Additive 5-value collapse of `status`. `null` when status is `not_requested`. */ + publicStatus: PublicStageStatus | null; authoritative: false; preferredAttemptId: string | null; attemptsSummary: PitchNoteTranslationAttemptSummary[]; @@ -158,6 +186,8 @@ export interface PitchNoteTranslationStageSnapshot { export interface InterpretationStageSnapshot { status: AnalysisStageStatus; + /** Additive 5-value collapse of `status`. `null` when status is `not_requested`. */ + publicStatus: PublicStageStatus | null; authoritative: false; preferredAttemptId: string | null; attemptsSummary: InterpretationAttemptSummary[]; diff --git a/docs/adr/0001-phase1-json-schema-v1.md b/docs/adr/0001-phase1-json-schema-v1.md index 23240af8..e702c2fe 100644 --- a/docs/adr/0001-phase1-json-schema-v1.md +++ b/docs/adr/0001-phase1-json-schema-v1.md @@ -59,6 +59,17 @@ The following nested shapes are the load-bearing time-series fields exported via Adding a new exportable time-series field is an additive change: register a serializer in [`apps/backend/csv_export.py`](../../apps/backend/csv_export.py), update the test in `apps/backend/tests/test_csv_export.py`, and document the CSV columns in the table above. +## Stage status field — `status` (internal) + `publicStatus` (additive) + +Every stage object inside the run snapshot (`stages.measurement`, `stages.pitchNoteTranslation`, `stages.interpretation`) carries two status fields: + +- **`status`** (8-state, internal vocabulary): `queued | running | blocked | ready | completed | failed | interrupted | not_requested`. This is the runtime's scheduling vocabulary and reflects internal state-machine transitions. Stable. +- **`publicStatus`** (5-state collapse + null): `queued | running | completed | failed | interrupted | null`. Added in Track 3.4 of the external-repo incorporation work. Maps `blocked`/`ready` → `queued`, `not_requested` → `null`, everything else 1:1. Stable. + +The collapse exists so external consumers who don't care about the distinction between internal scheduling states (e.g. blocked-waiting-for-dependency vs queued-and-ready) can read one field with a smaller vocabulary. The internal `status` is preserved for tools that need to debug or inspect runtime behavior. + +Source of truth for the mapping: [`apps/backend/stage_status.py`](../../apps/backend/stage_status.py). TypeScript mirror: `PublicStageStatus` in [`apps/ui/src/types/backend.ts`](../../apps/ui/src/types/backend.ts). + ## Where v1 is enforced | Layer | Enforcement | If you break v1 here |