Skip to content

feat(api): Track 3.4 — additive publicStatus field on every stage - #42

Merged
slittycode merged 1 commit into
mainfrom
claude/track3-public-status-5XA5r
May 14, 2026
Merged

feat(api): Track 3.4 — additive publicStatus field on every stage#42
slittycode merged 1 commit into
mainfrom
claude/track3-public-status-5XA5r

Conversation

@slittycode

Copy link
Copy Markdown
Owner

Fourth and final piece of Track 3 from the merged external-repo review. Closes Track 3.

What this adds

Every stage object in the run snapshot (stages.measurement, stages.pitchNoteTranslation, stages.interpretation) now carries a publicStatus field alongside the existing status field. Additive — nothing removed, nothing renamed.

"stages": {
  "measurement": {
    "status": "blocked",      // 8-state internal vocabulary, unchanged
    "publicStatus": "queued"  // 5-state collapse, new
    // ...
  },
  "interpretation": {
    "status": "not_requested",
    "publicStatus": null      // not in pipeline
    // ...
  }
}

Mapping

Internal status (8) Public publicStatus (5+null) Why
queued "queued" 1:1
running "running" 1:1
blocked "queued" Transient internal scheduling — client sees it as queued
ready "queued" Scheduled but not yet running — client sees it as queued
completed "completed" 1:1
failed "failed" 1:1
interrupted "interrupted" 1:1
not_requested null Stage not in pipeline; distinguishes "you didn't ask" from "queued"

null is explicit in JSON (not key omission) so clients in strongly-typed languages can read one shape without branching on key presence.

Why additive instead of contract-changing

This was the explicit user decision when scoping the piece. The contract-changing alternative (replace 8 internal states with 5 in the response) would have:

  • Required UI code updates (the UI already handles 8 states)
  • Risked breaking any other consumer that depended on the broader vocabulary
  • Required a schema v2 bump per docs/adr/0001-phase1-json-schema-v1.md's compatibility policy

The additive choice keeps internal scheduling vocabulary stable (useful for tooling, debugging, internal UI) while exposing a tidier external surface for new consumers. Trade-off acknowledged in the commit message: it doubles the status fields forever; collapsing to one would be a v2-era task.

Files

File Role
apps/backend/stage_status.py (new) Pure module with the canonical 8→5 mapping table, to_public_status() helper, and PUBLIC_STATUS_VALUES exported set.
apps/backend/server_phase1.py _normalize_run_snapshot now annotates each stage with publicStatus via a new _annotate_public_status helper. Original status preserved.
apps/backend/tests/test_stage_status.py (new, 16 tests) Pins every cell of the mapping plus a completeness check that fires if the set of internal states drifts without explicit mapping update.
apps/backend/tests/test_server.py (+1 test) Integration test asserting publicStatus appears on all three stages of a fresh run, with "queued" for measurement and null for the two not_requested stages.
apps/ui/src/types/backend.ts New PublicStageStatus type (the 5-value union). publicStatus: PublicStageStatus | null added to all three top-level stage interfaces.
apps/ui/src/services/analysisRunsClient.ts New parsePublicStageStatus helper (lenient — accepts null/missing for legacy compat). Threaded into the three stage parsers.
docs/adr/0001-phase1-json-schema-v1.md Documents the additive field, mapping table, and rationale.

Verification

  • 16 backend test_stage_status tests pass locally:
    $ python -m unittest tests.test_stage_status
    ................
    Ran 16 tests in 0.001s
    OK
    
  • UI npm run lint: clean. npm run test:unit: 398/398 (unchanged from main).
  • 1 new integration test in test_server.py exercises _normalize_run_snapshot end-to-end via get_analysis_run.

What this PR does NOT do

  • Does NOT change the existing status field on any stage. Same 8-state vocabulary, same values.
  • Does NOT add publicStatus to attempt summaries (attemptsSummary[] inside pitch-note and interpretation stages). Those are debug/audit views, not the canonical stage status.
  • Does NOT make existing UI code use publicStatus. Adding the field is the API change; consuming it is a separate decision per surface. The UI continues to use status everywhere.
  • Does NOT remove not_requested from anywhere. It's still in the internal vocabulary; publicStatus just exposes null for that case.

Track 3 closeout

This is the last piece. Full Track 3 sequencing as shipped:

After this lands, all four Track 3 pieces from the external-repo review are on main.

https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg


Generated by Claude Code

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

@slittycode slittycode left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE (posted as COMMENT — can't self-approve)

Summary

Adds an additive publicStatus field (5-value collapse) alongside the existing internal status field (8-value) on every stage in the run snapshot. Nothing removed, nothing renamed, no measurement values touched. 16/16 unit tests pass locally; the new integration test in test_server.py is sound but requires venv/FastAPI to run (CI handles it). This is a clean infra change.

Findings

No blocking or should-fix findings.

Worth considering (not blocking):

_normalize_run_snapshot relies on _annotate_public_status returning shallow-copied stage dicts so that measurement["result"] = _build_phase1(raw_result) mutates the copy already installed into snapshot["stages"]. The comment in the code calls this out ("Reuse the dict we already copied"). Both functions are in the same module and the comment is honest about the dependency — fine as-is.

The removal of the old early-return pattern (if measurement or raw_result not a dict, return snapshot unchanged) in favor of conditionals is an improvement: publicStatus is now annotated on all stages even when measurement hasn't populated a result yet, which is correct for queued state. This is not a bug — noting the semantics changed intentionally.

Test results

test_stage_status: 16/16 pass. The completeness guard (test_internal_states_unchanged, test_all_non_null_mappings_are_public_values) is a particularly good call — it will catch silent mapping drift if a new internal state gets added without a deliberate public-side decision.

test_server integration test: structurally correct, can't execute locally (no venv), normal.

Frontend lint: pre-existing type errors unrelated to this PR. The TranscriptionDetail errors at line 621–625 are not in this diff; the infrastructure errors (React, Vite types) are missing node_modules.

Phase boundary check

Clean. analyze.py and all analyze_*.py DSP modules untouched. _build_phase1 called identically to before. Phase1Result in types/measurement.ts not touched. The change is entirely in the HTTP normalization layer (_normalize_run_snapshot) and the run snapshot types — the right place for it.


Generated by Claude Code

@slittycode
slittycode merged commit 4674976 into main May 14, 2026
2 checks passed
@slittycode
slittycode deleted the claude/track3-public-status-5XA5r branch May 14, 2026 03:02
slittycode added a commit that referenced this pull request May 14, 2026
…nt (#43)

* feat(spectral): librosa.reassigned_spectrogram as an opt-in enhancement

Last item from the merged external-repo review's adopt list. Adds a
new on-demand spectral-enhancement kind, 'reassigned', that produces
a sharper spectrogram via per-bin (time, frequency) reassignment.

Why reassignment: vanilla STFT spectrograms smear transients across
the analyzing window (~46 ms for n_fft=2048 at 44.1 kHz). Reassignment
relocates each STFT bin to the local centroid of energy in the
(time, freq) plane — transients land at their true onset and stable
partials collapse to sharp horizontal lines. Useful when a producer
is trying to read attack timing or harmonic detail from the existing
mel/STFT views and finding them too blurry.

Implementation:

- apps/backend/spectral_viz.py — new generate_reassigned_spectrogram
  function. Calls librosa.reassigned_spectrogram with both frequency
  and time reassignment enabled, fill_nan=True (handles silent-region
  numerical instability without dropping points), and renders via
  matplotlib scatter (not librosa.display.specshow — specshow would
  re-rasterize to a uniform grid and undo the sharpening). Magnitude
  floor of -60 dBFS below peak filters the noise wash without losing
  attack detail.

- apps/backend/server.py — registered as 'reassigned' in the existing
  _ENHANCEMENT_GENERATORS dict. Inherits all existing wiring: auth,
  measurement-completed precondition, idempotency, artifact storage,
  MIME types, JSON response envelope.

- apps/backend/tests/test_spectral_viz.py — 3 new tests:
  - produces_single_reassigned_png: shape + non-trivial size
  - output_is_valid_png: magic bytes
  - handles_silent_input_without_raising: regression gate against
    NaN propagation on near-silent fixtures

No new dependencies — librosa is already in requirements.txt
(librosa==0.11.0 exposes reassigned_spectrogram).

Usage:

  POST /api/analysis-runs/{run_id}/spectral-enhancements/reassigned

Returns the new 'spectrogram_reassigned' artifact ID alongside the
existing PNG-style spectrograms. Idempotent: re-posting returns the
existing artifact without regenerating.

Out of scope (deferred):

- Frontend UI toggle to display the reassigned PNG. The backend now
  produces it on demand; the UI wiring is a separate follow-up.
- Raw (time, freq, mag) JSON artifact for client-side custom
  visualization. Could be added as a sibling 'reassigned_data'
  artifact if a future consumer needs it.

Closes the last review-adopt item. Full session summary in the
merge commits for #33 through #42.

https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg

* perf(spectral): cap reassigned-scatter at 300K points

PR #43 review caught a real perf issue: the original
generate_reassigned_spectrogram fed every STFT cell into
matplotlib.scatter unchecked. At default params (n_fft=2048,
hop=1024, sr=44100), that's:

  ~8M raw points on a 3-minute track
  ~15M raw points on a 10-minute track

Both well above the floor-mask survival count and well above the
threshold where matplotlib's render step starts taking tens of
seconds. The librosa compute cost isn't the bottleneck — the
scatter render is.

Fix:

- New REASSIGNED_MAX_SCATTER_POINTS = 300_000 constant. At
  FIG_DPI=100 / ~1200x400 px figure, 300K is the visual sweet
  spot — sharper-looking than vanilla STFT, renders in under a
  second.

- After the floor mask, the post-mask arrays (times_visible,
  freqs_visible, mags_visible) get subsampled via a seeded RNG
  (np.random.default_rng(0)). Seeded so re-running on the same
  audio produces a byte-identical PNG — matters for idempotency
  tests and content-hash artifact caching.

- New test test_scatter_point_cap_is_enforced_for_long_inputs:
  generates a 60 s multi-component fixture (~1M+ post-mask points,
  comfortably above the 300K cap), patches matplotlib.scatter to
  capture call args without rendering, and asserts the actual
  point count passed to scatter is <= REASSIGNED_MAX_SCATTER_POINTS.

The 'inferno colormap' worth-noting from the review was checked —
no 'inferno' reference exists in CLAUDE.md or anywhere in the
codebase. The reviewer's worth-noting was itself stale; nothing
to fix there.

https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants