Phase 3: Audition sample generation + UI chain-of-custody improvements - #52
Closed
slittycode wants to merge 9 commits into
Closed
Phase 3: Audition sample generation + UI chain-of-custody improvements#52slittycode wants to merge 9 commits into
slittycode wants to merge 9 commits into
Conversation
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
Co-authored-by: Claude <noreply@anthropic.com>
… applied tracker, grammar post-process (#41)
…edBandpass (#44) * docs(history): add torchfx library review Library evaluation of matteospanio/torchfx against ASA's per-band filtering call sites. Conclusion: do not adopt — GPL v3 vs ASA's MIT license is a one-way door, the shipped filter inventory is too sparse to displace scipy, and Phase 1 bandpass cost is not on the critical path. The torchfx |/+ operator pattern and the (tensor, fs, device) carrier object are worth borrowing only if a future profile shows per-band filtering is a top-three Phase 1 cost on long tracks. https://claude.ai/code/session_01WJ4q6kzeEMscgnSa9gRqQk * refactor(backend): centralize per-band Butterworth filtering in BatchedBandpass The torchfx audit (docs/history/library-review-torchfx-2026-05-13.md, PR #39) ruled out adopting matteospanio/torchfx as a dependency (GPL v3 vs ASA's MIT) but flagged the underlying pattern — a small filter primitive that batches per-band work and gives a clean hook for future GPU acceleration — as worth borrowing internally. This commit implements the minimum slice of that pattern that earns its complexity today: PR 1 of the audit's plan. Three scipy-Butterworth bandpass call sites in analyze_detection.py (_bandpass_signal, the reverb per-band RT60 loop) were each re-implementing the same butter(4, [lo, hi], output='sos') + sosfiltfilt sequence with slightly different glue. They now share one BatchedBandpass instance per sample rate (cached via functools.lru_cache(maxsize=4)) that memoises SOS coefficients per (lo_hz, hi_hz). The 7-band transient-density loop in analyze_per_band_transient_density and the Essentia BandPass call sites are intentionally out of scope for this PR. Output is bit-identical to the previous inline implementation. The load-bearing test (test_filter_one_matches_inline_scipy_bit_for_bit) compares the new class against a literal copy of the pre-refactor _bandpass_signal arithmetic on a fixed-seed signal and asserts np.allclose(atol=1e-12, rtol=1e-12). The full pre/post snapshot of analyze.py output on a synthetic click-train fixture is zero-diff (including reverbDetail.perBandRt60), so Phase 1 numbers do not move. No JSON field changes, no schema changes, no new dependencies. The BatchedBandpass(backend=...) argument is a forward-looking seam — only "scipy" is implemented today; a future torch path would land here if profiling shows per-band scipy filtering is hot. Tests: - tests.test_dsp_utils.BatchedBandpassTests: 8/8 OK (incl. bit-identical parity for both filter_one and filter_many) - tests.test_analyze.ReverbDetailTests: 7/7 OK (unchanged) - discover -s tests: 551/551 OK https://claude.ai/code/session_01WJ4q6kzeEMscgnSa9gRqQk * refactor(backend): accept dtype kwarg on BatchedBandpass.filter_one Addresses review feedback on PR #44: the original filter_one hardcoded float32 while filter_many defaulted to float64, which would be a footgun for the planned analyze_per_band_transient_density migration (PR 2 of the audit). filter_one now accepts a keyword-only ``dtype`` argument, making the public API symmetric with filter_many. The default remains ``np.float32`` — bit-identicality with the pre-refactor _bandpass_signal is preserved, and the existing reverb call site is unchanged. The asymmetric defaults (float32 for filter_one, float64 for filter_many) are intentional and now documented in a class-level note: each default matches its primary use case verbatim, and callers can override either at the call site. Two new tests lock the contract: - test_filter_one_default_dtype_is_float32 — asserts default is float32 (bit-identicality guarantee with _bandpass_signal) - test_filter_one_respects_dtype_override — asserts dtype=np.float64 produces a float64 array (the forward-looking override path) BatchedBandpassTests: 10/10 OK. ReverbDetailTests: 7/7 OK unchanged. https://claude.ai/code/session_01WJ4q6kzeEMscgnSa9gRqQk --------- Co-authored-by: Claude <noreply@anthropic.com>
…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>
* docs: sync architecture docs with current code Phase 3 audition samples, URL ingestion, CSV export, publicStatus, and the shared DSP primitives shipped after the last doc refresh (#38) but weren't reflected in the canonical docs. Update CLAUDE.md, the backend ARCHITECTURE.md / AGENTS.md / README.md, the frontend AGENTS.md / README.md, BACKLOG.md, and the docs/ index so the file maps, route lists, and module inventories match the tree. https://claude.ai/code/session_01MajhpbrcKaAoZiuCqwFDH8 * docs: correct types barrel re-export list in apps/ui/AGENTS.md src/types.ts re-exports only measurement, interpretation, and backend. samples.ts is imported directly, not through the barrel. https://claude.ai/code/session_01MajhpbrcKaAoZiuCqwFDH8 --------- Co-authored-by: Claude <noreply@anthropic.com>
slittycode
commented
May 16, 2026
slittycode
left a comment
slittycode
left a comment
Owner
Author
There was a problem hiding this comment.
Verdict: COMMENT
Summary
Large feature PR shipping Phase 3 audition-sample generation (6 new backend modules + HTTP layer), the chain-of-custody UI overhaul (CitationBlock, phase1Picker, applied-recommendations tracker), and a Phase 2 grammar post-processor. Frontend test suite runs at 567 passed / 3 failed / 1 skipped — all 3 failures are the pre-existing decision_gate.*.live.test.ts fixture-dependency bugs identified in PR #47 and slated for fix in PR #51; none are introduced here. Backend venv unavailable in this environment so backend tests could not be executed directly, but test coverage for every new module is present and meaningful (bit-for-bit parity test for BatchedBandpass, chain-of-custody assertion in test_sample_generation, precondition/force/fetch round-trip in test_server_samples).
Findings
Should fix
1. Dead or True branch — server_phase2.py:1027
return record if updated or True else record # always return; updated flag unusedor True makes the else record branch unreachable; updated is a dead variable. The in-place mutation still happens so this isn't a correctness bug, but it's misleading. PR #51 (claude/plan-pr-fixes-v0zZR → main) already has the one-line fix (return record, drop updated). This PR ships before that fix merges, so the dead code lands here.
2. SpectralArtifactRef.kind union missing 'spectrogram_reassigned' — apps/ui/src/types/backend.ts:86-92
server.py now registers "reassigned": ("generate_reassigned_spectrogram", ["spectrogram_reassigned"], True). The TypeScript union for SpectralArtifactRef.kind was not extended to include 'spectrogram_reassigned'. The frontend can't safely branch on this kind without a type error, and any future display code for the new enhancement won't typecheck. One-line fix:
| 'spectrogram_reassigned';Worth considering
3. Reassigned spectrogram uses cmap="magma" + linear Hz Y axis — spectral_viz.py:2265, 2276
Project rules flag colormap and Y-axis changes as visual contract changes. The technique is fundamentally incompatible with mel spacing (reassigned coordinates are continuous-valued, not grid-indexed), so linear Hz is the only sensible choice. magma vs inferno is a judgment call — magma's lighter end reads better on scatter points than inferno's near-black low end. Both points are defensible, but they should be acknowledged as intentional visual contract extensions for this artifact type rather than falling under the mel/inferno rule that governs the primary spectrogram.
Test results
Frontend: 567 passed / 3 failed / 1 skipped
All 3 failures: decision_gate.{multi,real,stems}.live.test.ts — expect(ok.length).toBeGreaterThan(0) on a machine with no /tmp/decision_gate_*.json snapshots. Pre-existing, tracked in PR #47, fix pending in PR #51.
Backend: not runnable in this environment (no venv).
Phase boundary check
Clean.
sample_generation.py/server_samples.py: read Phase 1 fields via_safe_get/ dict lookup; never assign back. Phase 2 result is consumed read-only in_phase2_tonal_citations/_phase2_kick_citations.server_phase2.pygrammar fixes: operate on the already-normalized Phase 2 dict inside_normalize_and_salvage_phase2_result, after it has been separated from the stored measurement. No Phase 1 data touched.phase1Picker.ts/CitationBlock.tsx: pure read functions;pickPhase1Valuetraverses the object but never assigns.phase2Validator.ts: unchanged guardrail logic;CONFIDENCE_PAIRSexport is additive.publicStatusis an additive envelope field on the HTTP snapshot; thePhase1Resultmeasurement object is untouched.
Generated by Claude Code
…union server.py registers the reassigned spectrogram artifact at #L2532, but the TypeScript union for SpectralArtifactRef.kind wasn't extended. Without this the frontend can't branch on the new kind, and any display code for the reassigned-spectrogram artifact won't typecheck.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR ships Phase 3 (audition sample generation) and implements a comprehensive audit-driven overhaul of the UI's chain-of-custody transparency. The backend now generates heuristic audio/MIDI clips derived from Phase 1 measurements and Phase 2 recommendations, while the frontend surfaces the measurement citations that justify every recommendation card.
Key Changes
Phase 3: Audition Sample Generation (Backend)
sample_generation.py: Orchestrator that reads Phase 1 + Phase 2 results and produces audition WAV/MIDI artifacts with a manifest documenting citation metadata (which Phase 1 field justifies each sample).sample_theory.py: Music-theory adapter with graceful fallback. Converts Phase 1 key/BPM/melody hints into MIDI plans. Includes pure-Python Western music theory implementation when PyTheory is unavailable.sample_synthesis.py: MIDI → WAV rendering with two backends: FluidSynth (when available) and sine-additive fallback (always available). Both produce identical float32 44.1 kHz output.sample_drums.py: NumPy-based drum synthesis. Renders kick one-shots from measured fundamental + decay; snare/hi-hat are heuristic defaults.dsp_bandbank.py: Shared 4th-order Butterworth bandpass bank primitive (extracted from inline patterns in detection module).server_samples.py: HTTP helper layer for/api/analysis-runs/{run_id}/samplesendpoints (POST to generate, GET to fetch existing manifest, stream artifacts).stage_status.py: Public-facing collapse of 8 internal stage statuses → 5 user-facing values (completed,failed,running,blocked,not_requested).docs/SAMPLE_GENERATION.md: Full specification of the manifest contract and audition philosophy.test_sample_generation.py,test_sample_theory.py,test_sample_synthesis.py,test_sample_drums.py,test_sample_audio_content.py,test_server_samples.py.Phase 2 Grammar Post-Process (Backend)
server_phase2.py: Added_apply_phase2_grammar_fixes()to rewrite Gemini's consistent "by <3rd-person-singular>" errors to "by " (e.g., "by recreates" → "by recreating").test_phase2_grammar_fix.py: Locks in the conversion rules and guards against regressions.Chain-of-Custody UI Layer (Frontend)
CitationBlock.tsx: New primitive that renders a "GROUNDED IN" structured-evidence block above every Mix Chain / Patches / Sonic Element card. Shows human-readable Phase 1 field labels, values, and worst-confidence pills (Audit Finding Add phase1 visual story v2 deck assets and build scripts #2 + Preserve signal monitor playback during analysis and harden run flow #3).phase1Picker.ts: Pure-function helpers to resolve dotted Phase 1 field paths (e.g.,kickDetail.fundamentalHz) to measured values and paired confidence siblings. Used by CitationBlock and confidence-band computation.userLabels.ts: Single translation layer from internal camelCase field paths to producer-readable labels. Fallback word-splitting rule for unknown fields.SamplePlayback.tsx: Audition panel that fetches/generates samples and renders playable clips with category labels and confidence indicators.sampleGenerationClient.ts: Typed HTTP client for Phase 3 endpoints (generate, fetch manifest, stream artifacts).appliedRecommendations.ts: Per-file applied-recommendations tracker (localStorage-backed, keyed by audio content SHA-256). Producers can check off Mix Chain / Patches cards as they wire them into Live; progress survives page reload and re-analysis (Audit Finding #3q #14 + Fix backend run, cleanup, and upload contract failures #15).UI Idle State Replacement (Audit Finding #5)
IdleValuePropPanel.tsx:https://claude.ai/code/session_01Cbh3iSuEhBFVGzU7dg412U