From 8e401e63d83f36ceda3964c7a359a3552b4b8f79 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 00:01:20 +0000 Subject: [PATCH 1/4] fix(loudness): Phase 1 v2 - truePeak to dBTP, bpmConfidence to 0-1, coherent PLR Phase 1 schema v2 migration for the loudness fields, motivated by a consumer audit: ~12 UI/recommendation consumers already assumed `truePeak` was dBTP (several were latent bugs), while only the loudness guardrail treated it as the linear amplitude proxy v1 actually emitted. - truePeak now emitted in dBTP (0.0 == full scale, >0 == inter-sample over); null for silence. bpmConfidence normalized to 0-1 (raw Essentia ~0-5.32 / 5). - PLR is now a coherent dB-domain subtraction (truePeak_dBTP - lufsIntegrated), fixing the prior linear-minus-dB unit mismatch, across analyze_core, the HTTP fallback, and the phase1 eval consistency gate. - Add top-level `phase1Version` ("phase1.v2") so consumers can detect the generation; loudness guardrail over-check moves to dBTP (>0); the validator drops its special-case bpmConfidence threshold (now shares the 0.4 hedge). - Re-baseline the golden (truePeak 0.0, plr 5.6); add dedicated analyze_plr and dBTP true-peak unit tests; update fixture expectations, types, JSON_SCHEMA. - ADR 0002 records the v2 bump, superseding ADR 0001 for these two fields. Verified: backend unittest suite green; frontend tsc + 716 unit tests + build green. https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q --- apps/backend/JSON_SCHEMA.md | 7 +- apps/backend/analyze.py | 2 + apps/backend/analyze_core.py | 30 ++++++-- apps/backend/analyze_fast.py | 17 ++++- apps/backend/loudness_rec_evaluation.py | 8 +- apps/backend/phase1_evaluation.py | 1 + apps/backend/server_phase1.py | 3 + .../tests/fixtures/golden/phase1_default.json | 6 +- apps/backend/tests/test_analyze.py | 2 + apps/backend/tests/test_audio_fixture.py | 10 ++- apps/backend/tests/test_loudness_r128.py | 76 ++++++++++++++++++- apps/ui/src/services/loudnessGuardrails.ts | 15 ++-- apps/ui/src/services/phase1Picker.ts | 16 ++-- apps/ui/src/services/phase2Validator.ts | 10 +-- apps/ui/src/types/measurement.ts | 4 + apps/ui/src/utils/exportUtils.ts | 2 +- .../tests/services/loudnessGuardrails.test.ts | 24 +++--- apps/ui/tests/services/phase1Picker.test.ts | 2 +- .../ui/tests/services/phase2Validator.test.ts | 20 +++-- docs/adr/0002-phase1-loudness-units-v2.md | 75 ++++++++++++++++++ 20 files changed, 267 insertions(+), 63 deletions(-) create mode 100644 docs/adr/0002-phase1-loudness-units-v2.md diff --git a/apps/backend/JSON_SCHEMA.md b/apps/backend/JSON_SCHEMA.md index d3cfe44f..d010fcb6 100644 --- a/apps/backend/JSON_SCHEMA.md +++ b/apps/backend/JSON_SCHEMA.md @@ -15,7 +15,7 @@ Conventions: Top-level keys: -`bpm`, `bpmConfidence`, `bpmPercival`, `bpmAgreement`, `bpmDoubletime`, `bpmSource`, `bpmRawOriginal`, `key`, `keyConfidence`, `keyProfile`, `tuningFrequency`, `tuningCents`, `timeSignature`, `timeSignatureSource`, `timeSignatureConfidence`, `durationSeconds`, `sampleRate`, `lufsIntegrated`, `lufsRange`, `lufsMomentaryMax`, `lufsShortTermMax`, `lufsCurve`, `truePeak`, `crestFactor`, `dynamicSpread`, `monoCompatible`, `plr`, `dynamicCharacter`, `textureCharacter`, `stereoDetail`, `spectralBalance`, `spectralBalanceTimeSeries`, `spectralDetail`, `stemAnalysis`, `transientDensityDetail`, `saturationDetail`, `snareDetail`, `hihatDetail`, `rhythmDetail`, `melodyDetail`, `transcriptionDetail`, `pitchDetail`, `grooveDetail`, `beatsLoudness`, `rhythmTimeline`, `sidechainDetail`, `reverbDetail`, `vocalDetail`, `acidDetail`, `supersawDetail`, `bassDetail`, `kickDetail`, `genreDetail`, `effectsDetail`, `synthesisCharacter`, `danceability`, `structure`, `arrangementDetail`, `segmentLoudness`, `segmentSpectral`, `segmentStereo`, `segmentKey`, `chordDetail`, `perceptual`, `essentiaFeatures`. +`phase1Version`, `bpm`, `bpmConfidence`, `bpmPercival`, `bpmAgreement`, `bpmDoubletime`, `bpmSource`, `bpmRawOriginal`, `key`, `keyConfidence`, `keyProfile`, `tuningFrequency`, `tuningCents`, `timeSignature`, `timeSignatureSource`, `timeSignatureConfidence`, `durationSeconds`, `sampleRate`, `lufsIntegrated`, `lufsRange`, `lufsMomentaryMax`, `lufsShortTermMax`, `lufsCurve`, `truePeak`, `crestFactor`, `dynamicSpread`, `monoCompatible`, `plr`, `dynamicCharacter`, `textureCharacter`, `stereoDetail`, `spectralBalance`, `spectralBalanceTimeSeries`, `spectralDetail`, `stemAnalysis`, `transientDensityDetail`, `saturationDetail`, `snareDetail`, `hihatDetail`, `rhythmDetail`, `melodyDetail`, `transcriptionDetail`, `pitchDetail`, `grooveDetail`, `beatsLoudness`, `rhythmTimeline`, `sidechainDetail`, `reverbDetail`, `vocalDetail`, `acidDetail`, `supersawDetail`, `bassDetail`, `kickDetail`, `genreDetail`, `effectsDetail`, `synthesisCharacter`, `danceability`, `structure`, `arrangementDetail`, `segmentLoudness`, `segmentSpectral`, `segmentStereo`, `segmentKey`, `chordDetail`, `perceptual`, `essentiaFeatures`. **Shared (fast + full) vs full-only.** Fast-mode output is asserted byte-for-byte against the `EXPECTED_TOP_LEVEL_KEYS` set in [`tests/test_analyze.py`](tests/test_analyze.py). Full mode emits those keys *plus* a handful of detail-only fields that are deliberately absent from the shared snapshot: `keyProfile`, `tuningFrequency`, `tuningCents`, `lufsMomentaryMax`, `lufsShortTermMax`, and `pitchDetail`. When changing the schema, update both this list and `EXPECTED_TOP_LEVEL_KEYS`; full-only fields stay out of that set on purpose. See CLAUDE.md tripwire #4. @@ -149,8 +149,9 @@ Current server behavior that affects schema expectations: | Field | Type | Description | Units / Scale | LLM interpretation note | |---|---|---|---|---| +| `phase1Version` | `string` | Phase 1 JSON schema version, e.g. `"phase1.v2"`. | identifier | Lets consumers detect the schema generation. v2 changed `truePeak` (now dBTP) and `bpmConfidence` (now 0-1) units vs v1. | | `bpm` | `float \| null` | Primary tempo estimate from `RhythmExtractor2013`. | beats per minute | Main tempo anchor for Ableton project tempo and clip warp assumptions. | -| `bpmConfidence` | `float \| null` | Confidence output from `RhythmExtractor2013` for primary BPM. | unbounded float (RhythmExtractor2013-specific; observed values typically 1.0-4.0 on real material) | Not normalised to 0-1. Higher values indicate stronger rhythmic periodicity. Values above 2.0 generally indicate reliable tempo detection. Low values (below 1.0) suggest ambiguous pulse or half/double-time content. | +| `bpmConfidence` | `float \| null` | Tempo confidence from `RhythmExtractor2013`, normalized to 0-1. | 0-1 (Phase 1 v2: raw Essentia confidence ~0-5.32 divided by 5.0 and clamped) | Higher = stronger rhythmic periodicity. Below 0.4 (raw < 2.0) suggests an ambiguous pulse or half/double-time content; hedge tempo claims there. | | `key` | `string \| null` | Global key label from `KeyExtractor` (`edma` profile), e.g. `"A Minor"`. | categorical | Starting point for harmonic reconstruction; validate by ear against bass/chord roots. | | `keyConfidence` | `float \| null` | Confidence/strength of global key estimate. | 0-1 (approx) | Low values indicate ambiguous tonality or modal/atonal content. | | `timeSignature` | `string \| null` | Time signature estimate (currently defaults to `"4/4"` when rhythm exists). | string | Treat as prior; verify manually on odd-metre material. | @@ -182,7 +183,7 @@ Current server behavior that affects schema expectations: |---|---|---|---|---| | `lufsIntegrated` | `float \| null` | Integrated loudness via `LoudnessEBUR128`. | LUFS | Global loudness target reference for gain staging and master chain matching. | | `lufsRange` | `float \| null` | Loudness range via `LoudnessEBUR128`. | LU | Indicates macro-dynamic movement across sections. | -| `truePeak` | `float \| null` | Max true peak across stereo channels. | linear amplitude proxy (rounded) | Helps detect clipping risk and required headroom when rebuilding. | +| `truePeak` | `float \| null` | Max true peak across stereo channels. | dBTP (Phase 1 v2: was a linear amplitude proxy in v1) | 0.0 dBTP == full scale; > 0.0 == inter-sample over. Helps detect clipping risk and required headroom when rebuilding. | | `crestFactor` | `float \| null` | Peak-to-RMS ratio over mono signal. | dB | Higher crest means stronger transients/less compression; lower crest suggests denser limiting/compression. | | `lufsMomentaryMax` | `float \| null` | Maximum momentary loudness (400 ms window) via `LoudnessEBUR128`. | LUFS | Peak short-burst loudness; useful for detecting loud transient moments. | | `lufsShortTermMax` | `float \| null` | Maximum short-term loudness (3 s window) via `LoudnessEBUR128`. | LUFS | Peak sustained loudness; gap between this and integrated LUFS indicates dynamic range use. | diff --git a/apps/backend/analyze.py b/apps/backend/analyze.py index 6463cd1f..375e8096 100644 --- a/apps/backend/analyze.py +++ b/apps/backend/analyze.py @@ -1469,6 +1469,7 @@ def main(): ) fast_plr = analyze_plr(result.get("lufsIntegrated"), result.get("truePeak")).get("plr") output = { + "phase1Version": "phase1.v2", "bpm": result.get("bpm"), "bpmConfidence": result.get("bpmConfidence"), "bpmPercival": result.get("bpmPercival"), @@ -1872,6 +1873,7 @@ def main(): # Build final output in the exact requested key order output = { + "phase1Version": "phase1.v2", "bpm": result.get("bpm"), "bpmConfidence": result.get("bpmConfidence"), "bpmPercival": result.get("bpmPercival"), diff --git a/apps/backend/analyze_core.py b/apps/backend/analyze_core.py index 1439fdd7..c279b905 100644 --- a/apps/backend/analyze_core.py +++ b/apps/backend/analyze_core.py @@ -93,7 +93,11 @@ def analyze_bpm( if rhythm_data is not None: bpm = round(float(rhythm_data["bpm"]), 1) - bpm_confidence = round(float(rhythm_data["confidence"]), 2) + # Phase 1 schema v2: normalize Essentia RhythmExtractor2013 confidence + # (raw ~0-5.32) to 0-1 by dividing by 5.0 and clamping. Consistent with + # every other *Confidence field; raw < 2.0 ("ambiguous" per the detector) + # maps to < 0.4, the standard low-confidence hedge threshold. + bpm_confidence = round(min(max(float(rhythm_data["confidence"]), 0.0) / 5.0, 1.0), 3) percival_cls = getattr(es, "PercivalBpmEstimator", None) if percival_cls is not None: @@ -248,8 +252,13 @@ def analyze_true_peak(stereo: np.ndarray) -> dict: peaks.append(float(np.max(peak_value)) if len(peak_value) > 0 else 0.0) else: peaks.append(float(peak_value)) - true_peak = max(peaks) if peaks else 0.0 - return {"truePeak": round(true_peak, 1)} + true_peak_linear = max(peaks) if peaks else 0.0 + # Phase 1 schema v2: emit dBTP (v1 emitted a linear amplitude proxy). + # 0.0 dBTP == full scale; > 0.0 dBTP == inter-sample over. A silent + # signal (linear 0) has no defined dBTP, so emit None. + if true_peak_linear <= 0.0: + return {"truePeak": None} + return {"truePeak": round(20.0 * float(np.log10(true_peak_linear)), 1)} except Exception as e: print(f"[warn] True peak detection failed: {e}", file=sys.stderr) return {"truePeak": None} @@ -621,13 +630,20 @@ def analyze_spectral_balance( return {"spectralBalance": None, "spectralBalanceTimeSeries": None} -def analyze_plr(lufs_integrated: float | None, true_peak: float | None) -> dict: - """Peak-to-loudness ratio (PLR): truePeak - LUFS integrated.""" +def analyze_plr(lufs_integrated: float | None, true_peak_dbtp: float | None) -> dict: + """Peak-to-loudness ratio (PLR) in LU: truePeak (dBTP) - integrated loudness (LUFS). + + As of Phase 1 schema v2, ``truePeak`` is expressed in dBTP (see + ``analyze_true_peak``), so PLR is a direct dB-domain subtraction. (In v1 + ``truePeak`` was a linear amplitude proxy and this subtraction was unit- + incoherent — the v2 migration fixes that at the source.) Returns None for + missing or non-finite inputs. + """ try: - if lufs_integrated is None or true_peak is None: + if lufs_integrated is None or true_peak_dbtp is None: return {"plr": None} lufs_value = float(lufs_integrated) - true_peak_value = float(true_peak) + true_peak_value = float(true_peak_dbtp) if not np.isfinite(lufs_value) or not np.isfinite(true_peak_value): return {"plr": None} return {"plr": round(true_peak_value - lufs_value, 2)} diff --git a/apps/backend/analyze_fast.py b/apps/backend/analyze_fast.py index fea78e76..1deebcd4 100644 --- a/apps/backend/analyze_fast.py +++ b/apps/backend/analyze_fast.py @@ -42,7 +42,12 @@ def analyze_fast(mono: np.ndarray, sample_rate: int = 44100) -> dict: rhythm_extractor = es.RhythmExtractor2013(method="multifeature") bpm, beats, bpm_confidence, _, _ = rhythm_extractor(mono) result["bpm"] = round(float(bpm), 2) if bpm is not None else None - result["bpmConfidence"] = round(float(bpm_confidence), 3) if bpm_confidence is not None else None + # Phase 1 v2: normalize RhythmExtractor2013 confidence (~0-5.32) to 0-1. + result["bpmConfidence"] = ( + round(min(max(float(bpm_confidence), 0.0) / 5.0, 1.0), 3) + if bpm_confidence is not None + else None + ) except Exception as e: print(f"[warn] Fast mode BPM analysis failed: {e}", file=sys.stderr) result["bpm"] = None @@ -106,11 +111,15 @@ def analyze_fast(mono: np.ndarray, sample_rate: int = 44100) -> dict: result["lufsIntegrated"] = None result["lufsRange"] = None - # True peak (from stereo) + # True peak (from stereo), emitted in dBTP (Phase 1 v2). NOTE: fast mode uses + # a plain sample peak (np.max(np.abs)), NOT the oversampled inter-sample true + # peak the standard path computes via Essentia's TruePeakDetector — so this is + # a sample-peak approximation and may read lower than the standard path on + # material with real inter-sample overs. try: if stereo is not None: - max_peak = np.max(np.abs(stereo)) - result["truePeak"] = round(float(max_peak), 6) if max_peak > 0 else None + max_peak = float(np.max(np.abs(stereo))) + result["truePeak"] = round(20.0 * np.log10(max_peak), 1) if max_peak > 0 else None else: result["truePeak"] = None except Exception as e: diff --git a/apps/backend/loudness_rec_evaluation.py b/apps/backend/loudness_rec_evaluation.py index f9cf9dcc..0ea7431b 100644 --- a/apps/backend/loudness_rec_evaluation.py +++ b/apps/backend/loudness_rec_evaluation.py @@ -189,4 +189,10 @@ def measure_loudness_true_peak( loudness = analyze_core.analyze_loudness(stereo, sample_rate=sample_rate) true_peak = analyze_core.analyze_true_peak(stereo) - return loudness.get("lufsIntegrated"), true_peak.get("truePeak") + # Phase 1 v2 emits truePeak in dBTP; this eval works in linear amplitude + # (limiter-ceiling reachability), so convert back to a linear peak. + true_peak_dbtp = true_peak.get("truePeak") + true_peak_linear = ( + 10.0 ** (float(true_peak_dbtp) / 20.0) if true_peak_dbtp is not None else None + ) + return loudness.get("lufsIntegrated"), true_peak_linear diff --git a/apps/backend/phase1_evaluation.py b/apps/backend/phase1_evaluation.py index 9b0893a6..90a5e002 100644 --- a/apps/backend/phase1_evaluation.py +++ b/apps/backend/phase1_evaluation.py @@ -223,6 +223,7 @@ def _evaluate_plr_consistency(payload: dict[str, Any]) -> FixtureCheck: passed=False, message=f"non-numeric values plr={plr} truePeak={true_peak} lufsIntegrated={lufs}", ) + # truePeak is dBTP (Phase 1 v2), so PLR is a direct dB-domain subtraction. expected = float(true_peak) - float(lufs) delta = abs(float(plr) - expected) passed = delta <= 0.11 diff --git a/apps/backend/server_phase1.py b/apps/backend/server_phase1.py index 5d5276a2..71c178d1 100644 --- a/apps/backend/server_phase1.py +++ b/apps/backend/server_phase1.py @@ -137,6 +137,8 @@ def _build_phase1(payload: dict[str, Any]) -> dict[str, Any]: if plr is None: lufs_integrated = _coerce_nullable_number(payload.get("lufsIntegrated")) true_peak = _coerce_nullable_number(payload.get("truePeak")) + # truePeak is dBTP (Phase 1 v2); PLR is a direct dB-domain subtraction + # (mirrors analyze_core.analyze_plr). if lufs_integrated is not None and true_peak is not None: plr = round(true_peak - lufs_integrated, 2) @@ -146,6 +148,7 @@ def _build_phase1(payload: dict[str, Any]) -> dict[str, Any]: mono_compatible = sub_bass_mono if isinstance(sub_bass_mono, bool) else None return { + "phase1Version": _coerce_nullable_string(payload.get("phase1Version")), "bpm": _coerce_number(payload.get("bpm")), "bpmConfidence": _coerce_number(payload.get("bpmConfidence")), "bpmPercival": _coerce_nullable_number(payload.get("bpmPercival")), diff --git a/apps/backend/tests/fixtures/golden/phase1_default.json b/apps/backend/tests/fixtures/golden/phase1_default.json index 1f520262..a6b973b5 100644 --- a/apps/backend/tests/fixtures/golden/phase1_default.json +++ b/apps/backend/tests/fixtures/golden/phase1_default.json @@ -10,7 +10,7 @@ "lufsRange": 0.8, "lufsShortTermMax": -5.2, "monoCompatible": true, - "plr": 6.6, + "plr": 5.6, "sampleRate": 44100, "spectralBalance.brilliance": -64.7, "spectralBalance.highs": -62.2, @@ -25,7 +25,7 @@ "stereoDetail.subBassMono": true, "timeSignature": "4/4", "timeSignatureSource": "assumed_four_four", - "truePeak": 1.0 + "truePeak": 0.0 }, "topLevelKeys": [ "acidDetail", @@ -62,6 +62,7 @@ "melodyDetail", "monoCompatible", "perceptual", + "phase1Version", "pitchDetail", "plr", "reverbDetail", @@ -129,6 +130,7 @@ "melodyDetail": "dict", "monoCompatible": "bool", "perceptual": "dict", + "phase1Version": "str", "pitchDetail": "null", "plr": "number", "reverbDetail": "dict", diff --git a/apps/backend/tests/test_analyze.py b/apps/backend/tests/test_analyze.py index f45522ea..3fdc8648 100644 --- a/apps/backend/tests/test_analyze.py +++ b/apps/backend/tests/test_analyze.py @@ -37,6 +37,7 @@ # here would force the field to always appear in CLI output and break the # "absent when off" contract. See JSON_SCHEMA.md "Optional MT3 Namespace". EXPECTED_TOP_LEVEL_KEYS = { + "phase1Version", "bpm", "bpmConfidence", "bpmPercival", "bpmAgreement", "bpmDoubletime", "bpmSource", "bpmRawOriginal", "key", "keyConfidence", "timeSignature", "timeSignatureSource", @@ -57,6 +58,7 @@ # Fields fast mode populates with real values. FAST_MODE_POPULATED_FIELDS = { + "phase1Version", "bpm", "bpmConfidence", "bpmPercival", "bpmAgreement", "bpmDoubletime", "bpmSource", "bpmRawOriginal", "key", "keyConfidence", "timeSignature", "timeSignatureSource", diff --git a/apps/backend/tests/test_audio_fixture.py b/apps/backend/tests/test_audio_fixture.py index 0ab22dc0..d8b5f71a 100644 --- a/apps/backend/tests/test_audio_fixture.py +++ b/apps/backend/tests/test_audio_fixture.py @@ -10,6 +10,7 @@ EXPECTED_TOP_LEVEL_KEYS = { + "phase1Version", "bpm", "bpmConfidence", "bpmPercival", "bpmAgreement", "bpmDoubletime", "bpmSource", "bpmRawOriginal", "key", "keyConfidence", "keyProfile", "tuningFrequency", "tuningCents", @@ -106,9 +107,12 @@ class AudioFixtureSmokeTest(unittest.TestCase): EXPECTED_LUFS_TOLERANCE = 0.5 EXPECTED_LUFS_RANGE = 0.8 EXPECTED_LUFS_RANGE_TOLERANCE = 0.5 - EXPECTED_TRUE_PEAK = 1.0 - EXPECTED_TRUE_PEAK_TOLERANCE = 0.1 - EXPECTED_PLR = 9.9 + # Phase 1 v2: truePeak is dBTP. The clipped fixture peaks at ~full scale, + # so truePeak ~ 0.0 dBTP (was 1.0 as a linear amplitude proxy in v1). + EXPECTED_TRUE_PEAK = 0.0 + EXPECTED_TRUE_PEAK_TOLERANCE = 0.2 + # PLR (LU) = truePeak(dBTP) - lufsIntegrated = ~0.0 - (-8.9) = ~8.9. + EXPECTED_PLR = 8.9 EXPECTED_PLR_TOLERANCE = 0.7 EXPECTED_CREST_FACTOR = 11.0 EXPECTED_CREST_FACTOR_TOLERANCE = 0.5 diff --git a/apps/backend/tests/test_loudness_r128.py b/apps/backend/tests/test_loudness_r128.py index d4f9b4cb..cee8542f 100644 --- a/apps/backend/tests/test_loudness_r128.py +++ b/apps/backend/tests/test_loudness_r128.py @@ -56,7 +56,7 @@ try: import essentia.standard as es # noqa: F401 import analyze_core - from analyze_core import analyze_loudness + from analyze_core import analyze_loudness, analyze_plr, analyze_true_peak ESSENTIA_AVAILABLE = True except Exception: # pragma: no cover - guarded by skip ESSENTIA_AVAILABLE = False @@ -322,5 +322,79 @@ def test_unusual_sample_rate_is_passed_verbatim(self) -> None: fake_class.assert_called_once_with(sampleRate=96_000) +@unittest.skipUnless(ESSENTIA_AVAILABLE, "Essentia not available in test env") +class TestAnalyzePlrIsDbtpMinusLufs(unittest.TestCase): + """``analyze_plr`` is a direct dB-domain subtraction: truePeak(dBTP) - LUFS. + + Phase 1 schema v2 emits ``truePeak`` in dBTP, so PLR needs no log conversion. + Regression gate for the prior unit-mismatch bug where PLR mixed a linear + amplitude with a dB value. The golden fixture only covers one signal, so + these pure-function cases lock the contract (including negative dBTP, which + real masters always have). + """ + + def test_full_scale_peak_plr_equals_negative_lufs(self) -> None: + # 0.0 dBTP (full scale) → PLR == -lufsIntegrated. + self.assertAlmostEqual(analyze_plr(-8.9, 0.0)["plr"], 8.9, places=2) + + def test_negative_dbtp_peak(self) -> None: + # A mastered track at -1.2 dBTP, -9.0 LUFS → PLR 7.8 LU. (A positivity + # guard would have wrongly nulled this — most masters are negative dBTP.) + self.assertAlmostEqual(analyze_plr(-9.0, -1.2)["plr"], 7.8, places=2) + + def test_inter_sample_over_positive_dbtp(self) -> None: + # +0.6 dBTP over, -7.0 LUFS → PLR 7.6 LU. + self.assertAlmostEqual(analyze_plr(-7.0, 0.6)["plr"], 7.6, places=2) + + def test_missing_or_non_finite_inputs_return_none(self) -> None: + self.assertIsNone(analyze_plr(None, 0.0)["plr"]) + self.assertIsNone(analyze_plr(-8.0, None)["plr"]) + self.assertIsNone(analyze_plr(float("nan"), 0.0)["plr"]) + self.assertIsNone(analyze_plr(-8.0, float("inf"))["plr"]) + + +@unittest.skipUnless(ESSENTIA_AVAILABLE, "Essentia not available in test env") +class TestAnalyzeTruePeakEmitsDbtp(unittest.TestCase): + """``analyze_true_peak`` emits dBTP (Phase 1 schema v2). A linear peak above + 1.0 must surface as a *positive* dBTP (an inter-sample over), full scale as + 0.0 dBTP, and silence as None. + + White-box: patches ``TruePeakDetector`` to return a known linear peak so the + dBTP conversion is asserted independent of Essentia's oversampler. + """ + + def _true_peak_dbtp_for_linear(self, linear: float) -> float | None: + stereo = _make_stereo_sine( + peak_dbfs=0.0, duration_s=0.1, sample_rate=44_100 + ) + fake_class = mock.MagicMock(name="TruePeakDetector_class") + fake_instance = mock.MagicMock(name="TruePeakDetector_instance") + fake_instance.return_value = ( + np.zeros(8, dtype=np.float32), + np.array([linear], dtype=np.float32), + ) + fake_class.return_value = fake_instance + with mock.patch.object( + analyze_core.es, "TruePeakDetector", new=fake_class + ): + return analyze_true_peak(stereo)["truePeak"] + + def test_inter_sample_over_is_positive_dbtp(self) -> None: + # linear 1.032 → 20*log10(1.032) == +0.27 dBTP (an over). + result = self._true_peak_dbtp_for_linear(1.032) + self.assertAlmostEqual(result, 0.3, places=1) + self.assertGreater(result, 0.0, "an inter-sample over must read > 0 dBTP") + + def test_full_scale_is_zero_dbtp(self) -> None: + self.assertAlmostEqual(self._true_peak_dbtp_for_linear(1.0), 0.0, places=1) + + def test_minus_one_dbtp(self) -> None: + # linear 0.8913 → -1.0 dBTP (the conventional master ceiling). + self.assertAlmostEqual(self._true_peak_dbtp_for_linear(0.8913), -1.0, places=1) + + def test_silence_returns_none(self) -> None: + self.assertIsNone(self._true_peak_dbtp_for_linear(0.0)) + + if __name__ == "__main__": unittest.main() diff --git a/apps/ui/src/services/loudnessGuardrails.ts b/apps/ui/src/services/loudnessGuardrails.ts index ba487b52..8b605f2d 100644 --- a/apps/ui/src/services/loudnessGuardrails.ts +++ b/apps/ui/src/services/loudnessGuardrails.ts @@ -3,11 +3,10 @@ import type { Phase1Result } from '../types'; /** * Objective, genre-independent loudness defects. * - * Unit note: Phase 1 `truePeak` is a LINEAR amplitude proxy (see - * apps/backend/JSON_SCHEMA.md — "linear amplitude proxy (rounded)"; the - * full-scale audio fixture in apps/backend/tests/test_audio_fixture.py asserts - * truePeak ≈ 1.0), NOT dBTP. 1.0 == 0 dBFS full scale; > 1.0 == inter-sample - * over. Always compare in linear amplitude, never in dB. + * Unit note: as of Phase 1 schema v2, `truePeak` is dBTP (see + * apps/backend/JSON_SCHEMA.md). 0.0 dBTP == full scale; > 0.0 dBTP == + * inter-sample over. Compare in dBTP. (In v1 this field was a linear amplitude + * proxy with a 1.0 boundary; the v2 migration moved it to dBTP.) * * The robust trigger is `saturationDetail.clippedSampleCount` (stereo samples * with |x| >= 0.9999) — unit-independent and unambiguous. truePeak is a @@ -18,8 +17,8 @@ import type { Phase1Result } from '../types'; * safety net only asserts correctness — that a measured defect is addressed. */ -/** Linear amplitude above which `truePeak` is an inter-sample over (0 dBFS). */ -export const TRUE_PEAK_OVER_LINEAR = 1.0; +/** dBTP above which `truePeak` is an inter-sample over (0.0 dBTP == full scale). */ +export const TRUE_PEAK_OVER_DBTP = 0.0; export type LoudnessDefectKind = 'CLIPPING' | 'TRUE_PEAK_OVER'; @@ -60,7 +59,7 @@ export function loudnessDefectsDemandingAction(phase1: Phase1Result): LoudnessDe if ( typeof truePeak === 'number' && Number.isFinite(truePeak) && - truePeak > TRUE_PEAK_OVER_LINEAR + truePeak > TRUE_PEAK_OVER_DBTP ) { defects.push({ kind: 'TRUE_PEAK_OVER', diff --git a/apps/ui/src/services/phase1Picker.ts b/apps/ui/src/services/phase1Picker.ts index 95c72448..bfa8aba3 100644 --- a/apps/ui/src/services/phase1Picker.ts +++ b/apps/ui/src/services/phase1Picker.ts @@ -46,7 +46,7 @@ export function pickPhase1Value( * formatCitedValue("reverbDetail.rt60", 2.04) → "2.04s" * formatCitedValue("bpmConfidence", 0.86) → "86%" * formatCitedValue("lufsIntegrated", -9.3) → "-9.3 LUFS" - * formatCitedValue("truePeak", -0.2) → "-0.2 dB" + * formatCitedValue("truePeak", -0.2) → "-0.2 dBTP" * formatCitedValue("key", "F minor") → "F minor" * formatCitedValue("acidDetail.isAcid", true) → "yes" * formatCitedValue("anything", null | undefined) → "" @@ -100,13 +100,17 @@ export function formatCitedValue(path: string, value: unknown): string { return `${value.toFixed(2)}s`; } - // True peak / dynamic / peak family — dB without sign-prefix rule. - // Match by suffix so nested paths like `kickDetail.crestFactor` also pick - // up the dB formatter rather than falling through to the bare-number rule. + // True peak is dBTP (Phase 1 v2). + if (path === 'truePeak' && typeof value === 'number') { + return `${value.toFixed(1)} dBTP`; + } + + // Dynamic / peak family — dB without sign-prefix rule. Match by suffix so + // nested paths like `kickDetail.crestFactor` also pick up the dB formatter + // rather than falling through to the bare-number rule. if ( typeof value === 'number' && - (path === 'truePeak' || - /(^|\.)(crestFactor|dynamicSpread|plr)$/.test(path)) + /(^|\.)(crestFactor|dynamicSpread|plr)$/.test(path) ) { return `${value.toFixed(1)} dB`; } diff --git a/apps/ui/src/services/phase2Validator.ts b/apps/ui/src/services/phase2Validator.ts index 5b98f471..82a9bd47 100644 --- a/apps/ui/src/services/phase2Validator.ts +++ b/apps/ui/src/services/phase2Validator.ts @@ -58,11 +58,10 @@ const TRIVIAL_CITATION_DOMINANCE_THRESHOLD = 0.6; /** * Confidence below which paired recommendation text must use hedged language. - * Most detector confidences are normalized 0-1. `bpmConfidence` is Essentia's - * unbounded RhythmExtractor2013 confidence, so it uses its own threshold. + * All detector confidences — including `bpmConfidence`, normalized to 0-1 as of + * Phase 1 schema v2 — share this threshold. */ const NORMALIZED_LOW_CONFIDENCE_THRESHOLD = 0.4; -const BPM_LOW_CONFIDENCE_THRESHOLD = 1.0; /** * Phase 1 fields whose presence-but-zero-citations should warn. The list now @@ -1231,10 +1230,7 @@ function lowConfidenceTriggerForCitation( const value = readNumberAtPath(phase1, confidencePath); if (value === null) return null; - const threshold = - confidencePath === 'bpmConfidence' - ? BPM_LOW_CONFIDENCE_THRESHOLD - : NORMALIZED_LOW_CONFIDENCE_THRESHOLD; + const threshold = NORMALIZED_LOW_CONFIDENCE_THRESHOLD; if (value >= threshold) return null; return { field: cited, diff --git a/apps/ui/src/types/measurement.ts b/apps/ui/src/types/measurement.ts index 426f309e..0cf2bb63 100644 --- a/apps/ui/src/types/measurement.ts +++ b/apps/ui/src/types/measurement.ts @@ -628,7 +628,10 @@ export interface GenreDetail { } export interface Phase1Result { + /** Phase 1 schema version, e.g. "phase1.v2". Absent on pre-v2 snapshots. */ + phase1Version?: string | null; bpm: number; + /** Tempo confidence, normalized 0-1 (Phase 1 v2; raw Essentia ~0-5.32 in v1). */ bpmConfidence: number; bpmPercival?: number | null; bpmAgreement?: boolean | null; @@ -649,6 +652,7 @@ export interface Phase1Result { lufsRange?: number | null; lufsMomentaryMax?: number | null; lufsShortTermMax?: number | null; + /** Max true peak in dBTP (Phase 1 v2; was a linear amplitude proxy in v1). 0.0 == full scale, > 0 == inter-sample over. */ truePeak: number; plr?: number | null; crestFactor?: number | null; diff --git a/apps/ui/src/utils/exportUtils.ts b/apps/ui/src/utils/exportUtils.ts index 1d0f4fdf..23a368c7 100644 --- a/apps/ui/src/utils/exportUtils.ts +++ b/apps/ui/src/utils/exportUtils.ts @@ -54,7 +54,7 @@ export function generateMarkdown( md += `- **Time Signature**: ${phase1.timeSignature}\n`; md += `- **Duration (s)**: ${phase1.durationSeconds}\n`; md += `- **Integrated LUFS**: ${phase1.lufsIntegrated}\n`; - md += `- **True Peak**: ${phase1.truePeak}\n`; + md += `- **True Peak**: ${phase1.truePeak} dBTP\n`; md += `- **Stereo Width**: ${phase1.stereoWidth}\n`; md += `- **Stereo Correlation**: ${phase1.stereoCorrelation}\n\n`; diff --git a/apps/ui/tests/services/loudnessGuardrails.test.ts b/apps/ui/tests/services/loudnessGuardrails.test.ts index 49eb0616..15f72d87 100644 --- a/apps/ui/tests/services/loudnessGuardrails.test.ts +++ b/apps/ui/tests/services/loudnessGuardrails.test.ts @@ -1,27 +1,28 @@ import { describe, it, expect } from 'vitest'; import { - TRUE_PEAK_OVER_LINEAR, + TRUE_PEAK_OVER_DBTP, citationAddressesLoudnessDefect, loudnessDefectsDemandingAction, } from '../../src/services/loudnessGuardrails'; import type { Phase1Result } from '../../src/types'; // Minimal Phase1Result — only the fields the predicate reads matter; the rest -// are irrelevant to loudness defect detection. +// are irrelevant to loudness defect detection. Default truePeak is a normal +// mastered peak below full scale (negative dBTP). const phase1 = (overrides: Partial): Phase1Result => - ({ truePeak: 0.5, ...overrides }) as Phase1Result; + ({ truePeak: -1.0, ...overrides }) as Phase1Result; describe('loudnessDefectsDemandingAction', () => { it('returns no defects for a clean master (no clipping, peak below full scale)', () => { const result = loudnessDefectsDemandingAction( - phase1({ truePeak: 0.5, saturationDetail: { clippedSampleCount: 0 } as any }), + phase1({ truePeak: -1.0, saturationDetail: { clippedSampleCount: 0 } as any }), ); expect(result).toEqual([]); }); it('flags CLIPPING when clippedSampleCount > 0', () => { const result = loudnessDefectsDemandingAction( - phase1({ truePeak: 0.5, saturationDetail: { clippedSampleCount: 1280 } as any }), + phase1({ truePeak: -1.0, saturationDetail: { clippedSampleCount: 1280 } as any }), ); expect(result).toHaveLength(1); expect(result[0]).toMatchObject({ @@ -31,14 +32,14 @@ describe('loudnessDefectsDemandingAction', () => { }); }); - it('flags TRUE_PEAK_OVER when truePeak exceeds full scale (linear > 1.0)', () => { + it('flags TRUE_PEAK_OVER when truePeak exceeds full scale (dBTP > 0)', () => { const result = loudnessDefectsDemandingAction(phase1({ truePeak: 1.1 })); expect(result).toHaveLength(1); expect(result[0]).toMatchObject({ kind: 'TRUE_PEAK_OVER', field: 'truePeak', value: 1.1 }); }); - it('does not flag a true peak exactly at full scale (1.0 is the boundary, not an over)', () => { - const result = loudnessDefectsDemandingAction(phase1({ truePeak: TRUE_PEAK_OVER_LINEAR })); + it('does not flag a true peak exactly at full scale (0.0 dBTP is the boundary, not an over)', () => { + const result = loudnessDefectsDemandingAction(phase1({ truePeak: TRUE_PEAK_OVER_DBTP })); expect(result).toEqual([]); }); @@ -49,8 +50,9 @@ describe('loudnessDefectsDemandingAction', () => { expect(result.map(d => d.kind).sort()).toEqual(['CLIPPING', 'TRUE_PEAK_OVER']); }); - it('treats truePeak as LINEAR, so a dB-style negative value never trips the over check', () => { - // Guards the unit contract: truePeak is a linear amplitude proxy, not dBTP. + it('treats truePeak as dBTP, so a normal master below full scale (negative dBTP) is not an over', () => { + // Guards the unit contract: truePeak is dBTP (Phase 1 v2). -0.2 dBTP is below + // full scale, so it is not an inter-sample over. const result = loudnessDefectsDemandingAction(phase1({ truePeak: -0.2 })); expect(result).toEqual([]); }); @@ -59,7 +61,7 @@ describe('loudnessDefectsDemandingAction', () => { expect(loudnessDefectsDemandingAction(phase1({ truePeak: null as any }))).toEqual([]); expect( loudnessDefectsDemandingAction( - phase1({ truePeak: 0.5, saturationDetail: null as any }), + phase1({ truePeak: -1.0, saturationDetail: null as any }), ), ).toEqual([]); expect(loudnessDefectsDemandingAction(phase1({ truePeak: NaN }))).toEqual([]); diff --git a/apps/ui/tests/services/phase1Picker.test.ts b/apps/ui/tests/services/phase1Picker.test.ts index b485cd22..36818fb7 100644 --- a/apps/ui/tests/services/phase1Picker.test.ts +++ b/apps/ui/tests/services/phase1Picker.test.ts @@ -115,7 +115,7 @@ describe('formatCitedValue', () => { }); it('formats dB-family paths', () => { - expect(formatCitedValue('truePeak', -0.2)).toBe('-0.2 dB'); + expect(formatCitedValue('truePeak', -0.2)).toBe('-0.2 dBTP'); expect(formatCitedValue('crestFactor', 11.6)).toBe('11.6 dB'); }); diff --git a/apps/ui/tests/services/phase2Validator.test.ts b/apps/ui/tests/services/phase2Validator.test.ts index 0e5a01e7..f2692bbb 100644 --- a/apps/ui/tests/services/phase2Validator.test.ts +++ b/apps/ui/tests/services/phase2Validator.test.ts @@ -1089,8 +1089,10 @@ describe('validatePhase2Consistency', () => { expect(violations).toHaveLength(0); }); - it('uses bpmConfidence < 1.0 as the low-confidence tempo threshold', () => { - const phase1 = createBasePhase1({ bpm: 126, bpmConfidence: 0.82 }); + it('uses the normalized 0.4 threshold for low-confidence bpmConfidence (v2)', () => { + // Phase 1 v2: bpmConfidence is normalized 0-1, so it shares the standard + // 0.4 hedge threshold (0.2 < 0.4 → unhedged tempo claim is a violation). + const phase1 = createBasePhase1({ bpm: 126, bpmConfidence: 0.2 }); const phase2 = createBasePhase2({ abletonRecommendations: [ { @@ -1109,11 +1111,11 @@ describe('validatePhase2Consistency', () => { v => v.type === 'LOW_CONFIDENCE_NOT_HEDGED', ); expect(violations).toHaveLength(1); - expect(violations[0].message).toContain('bpmConfidence < 1'); + expect(violations[0].message).toContain('bpmConfidence < 0.4'); }); - it('does not apply the normalized 0.4 threshold to reliable bpmConfidence values', () => { - const phase1 = createBasePhase1({ bpm: 126, bpmConfidence: 1.2 }); + it('does not flag a reliable (>= 0.4) bpmConfidence value', () => { + const phase1 = createBasePhase1({ bpm: 126, bpmConfidence: 0.9 }); const phase2 = createBasePhase2({ abletonRecommendations: [ { @@ -1375,7 +1377,8 @@ describe('Loudness action presence (objective safety net)', () => { const clippingPhase1 = () => createBasePhase1({ - truePeak: 0.9, + // -0.5 dBTP: below full scale, so CLIPPING is the only defect under test. + truePeak: -0.5, saturationDetail: { clippedSampleCount: 1280, clippedSamplePercent: 0.4 } as any, }); @@ -1415,7 +1418,7 @@ describe('Loudness action presence (objective safety net)', () => { expect(loudnessViolations(validatePhase2Consistency(phase1, phase2))).toHaveLength(0); }); - it('warns on an unaddressed true-peak over (linear > 1.0)', () => { + it('warns on an unaddressed true-peak over (dBTP > 0)', () => { const result = validatePhase2Consistency(createBasePhase1({ truePeak: 1.1 }), newShapePhase2()); const loudness = loudnessViolations(result); expect(loudness).toHaveLength(1); @@ -1424,7 +1427,8 @@ describe('Loudness action presence (objective safety net)', () => { it('does not warn for a clean master (no clipping, peak below full scale)', () => { const phase1 = createBasePhase1({ - truePeak: 0.8, + // -0.8 dBTP: a clean master sitting below full scale. + truePeak: -0.8, saturationDetail: { clippedSampleCount: 0, clippedSamplePercent: 0 } as any, }); expect(loudnessViolations(validatePhase2Consistency(phase1, newShapePhase2()))).toHaveLength(0); diff --git a/docs/adr/0002-phase1-loudness-units-v2.md b/docs/adr/0002-phase1-loudness-units-v2.md new file mode 100644 index 00000000..0dfb5ef7 --- /dev/null +++ b/docs/adr/0002-phase1-loudness-units-v2.md @@ -0,0 +1,75 @@ +# ADR 0002 — Phase 1 loudness-field units (schema v2) + +**Status:** Accepted +**Date:** 2026-06-01 +**Supersedes:** Partially supersedes [ADR 0001](0001-phase1-json-schema-v1.md) for two fields. +**Anchor:** [PURPOSE.md](../../PURPOSE.md) quality invariants #1 (measurement authority) and #4 (honest uncertainty). + +## Context + +ADR 0001 declared the Phase 1 payload "v1" and classified unit changes and range +changes as breaking (forbidden under v1). Two v1 fields carried representations that +fought the grain of nearly every consumer: + +1. **`truePeak`** was emitted as a **linear amplitude proxy** (1.0 == 0 dBFS). The + conventional meaning of "true peak" is **dBTP**, and a code audit found ~12 consumers + already *assumed* dBTP — several were latent bugs because the field was actually linear: + - `analysisResultsViewModel.ts` computed master ceilings as `min(-0.3, truePeak - 0.1)`, + which always returned `-0.3` for a linear `truePeak ≈ 1.0`. + - `MeasurementDashboard.tsx` rendered `{ value: truePeak, suffix: 'dBTP' }` (showing + `1.00 dBTP` instead of `0.0`) and scaled the meter on a dB range. + - `analyze_core.analyze_plr` computed `truePeak_linear - lufsIntegrated` — a linear + amplitude minus a dB value, an incoherent number. + Only `loudnessGuardrails.ts` correctly treated the field as linear. + +2. **`bpmConfidence`** was the raw Essentia `RhythmExtractor2013` confidence (~0–5.32), + the *only* `*Confidence` field not normalized to 0–1. Consumers that assumed 0–1 + mis-rendered it (`exportUtils.ts` multiplied by 100 → up to ~500%; the confidence-band + ladder mis-banded it), and the validator carried a special-case threshold. + +The WASM loudness package (`packages/loudness-spectro-wasm`) also emits `true_peak_dbtp`. +Keeping v1's linear/raw representations required converting at ~20 fragile consumer sites; +fixing the *source* makes the majority of consumers correct by default. + +## Decision + +Bump the Phase 1 measurement schema to **`phase1.v2`** and change two field units: + +| Field | v1 | v2 | +|---|---|---| +| `truePeak` | linear amplitude proxy (1.0 == full scale) | **dBTP** (0.0 == full scale, > 0 == inter-sample over); `null` for silence | +| `bpmConfidence` | raw Essentia confidence (~0–5.32) | **normalized 0–1** (raw / 5.0, clamped) | + +A new top-level string field **`phase1Version`** (e.g. `"phase1.v2"`) is added so consumers +can detect the generation — this is the canonical mechanism ADR 0001 anticipated for a v2 bump. + +Derived/consumer effects: +- `plr` is now a direct dB-domain subtraction (`truePeak_dBTP - lufsIntegrated`); its *unit* + (LU) is unchanged, so this is a correctness fix, not a contract change. +- `loudnessGuardrails.ts` over-detection moves from `truePeak > 1.0` (linear) to + `truePeak > 0.0` (dBTP). +- `bpmConfidence` now shares the standard `0.4` low-confidence hedge threshold; the + validator's BPM-specific threshold is removed. + +## Compatibility + +This is a **breaking change** under ADR 0001's policy (units + range). It is acceptable +because ASA is, in practice, its own only consumer (personal use), and the change makes the +payload *more* conventional, not less. External consumers can branch on `phase1Version`. +ADR 0001's compatibility table still governs all other fields. + +## Enforcement / where v2 is pinned + +- `EXPECTED_TOP_LEVEL_KEYS` and `FAST_MODE_POPULATED_FIELDS` in `tests/test_analyze.py` + (now include `phase1Version`). +- The Phase 1 golden snapshot (`tests/fixtures/golden/phase1_default.json`), re-baselined + for `truePeak: 0.0` (dBTP) and `plr: 5.6`. +- Dedicated unit tests in `tests/test_loudness_r128.py` (dBTP `analyze_true_peak`, dB-domain + `analyze_plr`) and the fixture expectations in `tests/test_audio_fixture.py`. +- `Phase1Result` typing in `apps/ui/src/types/measurement.ts` and `JSON_SCHEMA.md`. + +## Not in scope + +- No other field units change. No machine-readable JSON Schema artifact (still deferred). +- The WASM package is not yet wired in; this ADR only aligns the Essentia path's *units* + with dBTP convention (which happens to match WASM), easing a future parity comparison. From 0b66839ce9cdbee3b644caf49120409785999c27 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 00:03:36 +0000 Subject: [PATCH 2/4] chore: ignore Python bytecode repo-wide The .gitignore only covered apps/backend/__pycache__/, so bytecode generated outside the backend (e.g. scripts/__pycache__/ when build_live12_catalogue.py is imported) showed up as untracked. Generalize the __pycache__/ and *.py[cod] rules to the whole repo. https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q --- .gitignore | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 22ec7817..1ba81381 100644 --- a/.gitignore +++ b/.gitignore @@ -15,9 +15,10 @@ apps/ui/.env apps/ui/.env* !apps/ui/.env.example -apps/backend/__pycache__/ -apps/backend/*.py[cod] -apps/backend/*.pyo +# Python bytecode (repo-wide — covers scripts/, apps/backend/, etc.) +__pycache__/ +*.py[cod] +*.pyo apps/backend/*.egg-info/ apps/backend/build/ apps/backend/dist/ From 2e69dd1840e2ad8f7376610e23dff3d4a562034f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 00:13:50 +0000 Subject: [PATCH 3/4] fix(loudness): v2 follow-ups - phase1 eval manifest targets + dBTP labels - Backend CI fix: phase1_eval_manifest.json truePeak/plr thresholds still encoded v1 linear values (click_120 truePeak 0.9 / plr 24.8; sine_220 0.5 / 7.4). Migrated to the v2 dBTP measurements (click -0.9 / 23.2; sine -6.0 / 1.0). The plr:consistency gate already passes; only the manifest's expected targets were stale. - Review (#128): three analysisResultsViewModel.ts display sites labeled the now-correct truePeak value as "dB"; corrected to "dBTP". Verified: tests.test_phase1_evaluation green; frontend tsc + 716 unit green. https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q --- apps/backend/tests/fixtures/phase1_eval_manifest.json | 8 ++++---- apps/ui/src/components/analysisResultsViewModel.ts | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/backend/tests/fixtures/phase1_eval_manifest.json b/apps/backend/tests/fixtures/phase1_eval_manifest.json index adcb7b97..7556e6d5 100644 --- a/apps/backend/tests/fixtures/phase1_eval_manifest.json +++ b/apps/backend/tests/fixtures/phase1_eval_manifest.json @@ -14,8 +14,8 @@ "thresholds": { "bpm": { "target": 120.1, "tolerance": 0.2 }, "lufsIntegrated": { "target": -23.9, "tolerance": 0.5 }, - "truePeak": { "target": 0.9, "tolerance": 0.2 }, - "plr": { "target": 24.8, "tolerance": 0.7 }, + "truePeak": { "target": -0.9, "tolerance": 0.2 }, + "plr": { "target": 23.2, "tolerance": 0.7 }, "monoCompatible": { "equals": true } } }, @@ -31,8 +31,8 @@ "thresholds": { "bpm": { "target": 112.2, "tolerance": 0.2 }, "lufsIntegrated": { "target": -6.9, "tolerance": 0.5 }, - "truePeak": { "target": 0.5, "tolerance": 0.2 }, - "plr": { "target": 7.4, "tolerance": 0.7 }, + "truePeak": { "target": -6.0, "tolerance": 0.2 }, + "plr": { "target": 1.0, "tolerance": 0.7 }, "monoCompatible": { "equals": true } } } diff --git a/apps/ui/src/components/analysisResultsViewModel.ts b/apps/ui/src/components/analysisResultsViewModel.ts index 8e4a04c2..915188b7 100644 --- a/apps/ui/src/components/analysisResultsViewModel.ts +++ b/apps/ui/src/components/analysisResultsViewModel.ts @@ -531,7 +531,7 @@ function getSonicMeasurements( const sets: Record = { kick: [ { icon: "🎚", label: "Low Bass", value: formatSignedDb(phase1.spectralBalance.lowBass) }, - { icon: "📏", label: "Peak", value: `${phase1.truePeak.toFixed(1)} dB` }, + { icon: "📏", label: "Peak", value: `${phase1.truePeak.toFixed(1)} dBTP` }, { icon: "⏱", label: "Tempo", value: `${Math.round(phase1.bpm)} BPM` }, ], bass: [ @@ -587,7 +587,7 @@ function getSonicMeasurements( widthAndStereo: [ { icon: "↔", label: "Width", value: phase1.stereoWidth.toFixed(2) }, { icon: "📡", label: "Correlation", value: phase1.stereoCorrelation.toFixed(2) }, - { icon: "🧲", label: "Peak", value: `${phase1.truePeak.toFixed(1)} dB` }, + { icon: "🧲", label: "Peak", value: `${phase1.truePeak.toFixed(1)} dBTP` }, ], harmonicContent: [ { icon: "🎼", label: "Key", value: phase1.key ?? "Unknown" }, @@ -815,7 +815,7 @@ function buildDerivedChainParameters( case "DRUM PROCESSING": return [ { label: "Tempo Sync", value: `${Math.round(phase1.bpm)} BPM` }, - { label: "Punch Target", value: `${phase1.truePeak.toFixed(1)} dB peak` }, + { label: "Punch Target", value: `${phase1.truePeak.toFixed(1)} dBTP peak` }, ]; case "BASS PROCESSING": return [ From 7bfdfed068627c96dcde009d3c5a8ada51fe3f4f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 01:53:29 +0000 Subject: [PATCH 4/4] Harden nullable truePeak across the v2 contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 v2 emits truePeak: null for digital silence (no defined dBTP), but the frontend type declared it `number` and several display sites called .toFixed unguarded — a latent crash on silent input. Worse, the HTTP envelope coerced null -> 0.0, falsely reporting a full-scale peak for a silent track (violating measurement-honesty invariants #1/#4). Backend: - server_phase1._build_phase1 now preserves null via _coerce_nullable_number instead of defaulting to 0.0; PLR fallback already guards None. - Tests: truePeak passthrough/null + PLR-null-when-peak-null. Frontend: - Phase1Result.truePeak typed `number | null`. - analysisResultsViewModel: formatTruePeak (renders "—" for null, matching the house formatNumber convention) and masterCeilingDb (falls back to the -0.3 dB cap when there is no peak to duck below); applied at all 6 sites. - MeasurementDashboard: omit the true-peak marker, PLR gap fill, and PLR annotation for silence (LUFS marker still renders). - backendPhase1Client: accept an explicit null truePeak (legacy path) while still rejecting a malformed/missing field. - exportUtils markdown renders "—" for a null peak. - Tests: null-silence card renders "—" without crashing; helper unit cases. Docs: JSON_SCHEMA truePeak note now states null == digital silence. https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q --- apps/backend/JSON_SCHEMA.md | 2 +- apps/backend/server_phase1.py | 5 ++- apps/backend/tests/test_server.py | 17 ++++++++ .../src/components/MeasurementDashboard.tsx | 40 ++++++++++--------- .../components/analysisResultsViewModel.ts | 31 +++++++++++--- apps/ui/src/services/backendPhase1Client.ts | 7 +++- apps/ui/src/types/measurement.ts | 4 +- apps/ui/src/utils/exportUtils.ts | 2 +- .../services/analysisResultsViewModel.test.ts | 36 +++++++++++++++++ 9 files changed, 113 insertions(+), 31 deletions(-) diff --git a/apps/backend/JSON_SCHEMA.md b/apps/backend/JSON_SCHEMA.md index d010fcb6..d9097464 100644 --- a/apps/backend/JSON_SCHEMA.md +++ b/apps/backend/JSON_SCHEMA.md @@ -183,7 +183,7 @@ Current server behavior that affects schema expectations: |---|---|---|---|---| | `lufsIntegrated` | `float \| null` | Integrated loudness via `LoudnessEBUR128`. | LUFS | Global loudness target reference for gain staging and master chain matching. | | `lufsRange` | `float \| null` | Loudness range via `LoudnessEBUR128`. | LU | Indicates macro-dynamic movement across sections. | -| `truePeak` | `float \| null` | Max true peak across stereo channels. | dBTP (Phase 1 v2: was a linear amplitude proxy in v1) | 0.0 dBTP == full scale; > 0.0 == inter-sample over. Helps detect clipping risk and required headroom when rebuilding. | +| `truePeak` | `float \| null` | Max true peak across stereo channels. | dBTP (Phase 1 v2: was a linear amplitude proxy in v1) | 0.0 dBTP == full scale; > 0.0 == inter-sample over; `null` for digital silence (no defined dBTP). Helps detect clipping risk and required headroom when rebuilding. | | `crestFactor` | `float \| null` | Peak-to-RMS ratio over mono signal. | dB | Higher crest means stronger transients/less compression; lower crest suggests denser limiting/compression. | | `lufsMomentaryMax` | `float \| null` | Maximum momentary loudness (400 ms window) via `LoudnessEBUR128`. | LUFS | Peak short-burst loudness; useful for detecting loud transient moments. | | `lufsShortTermMax` | `float \| null` | Maximum short-term loudness (3 s window) via `LoudnessEBUR128`. | LUFS | Peak sustained loudness; gap between this and integrated LUFS indicates dynamic range use. | diff --git a/apps/backend/server_phase1.py b/apps/backend/server_phase1.py index 71c178d1..7ae13aee 100644 --- a/apps/backend/server_phase1.py +++ b/apps/backend/server_phase1.py @@ -171,7 +171,10 @@ def _build_phase1(payload: dict[str, Any]) -> dict[str, Any]: "lufsMomentaryMax": _coerce_nullable_number(payload.get("lufsMomentaryMax")), "lufsShortTermMax": _coerce_nullable_number(payload.get("lufsShortTermMax")), "lufsCurve": payload.get("lufsCurve"), - "truePeak": _coerce_number(payload.get("truePeak")), + # truePeak is dBTP (Phase 1 v2) and is null for digital silence (no + # defined dBTP). Preserve that null rather than coercing it to 0.0, + # which would falsely report a full-scale peak for a silent track. + "truePeak": _coerce_nullable_number(payload.get("truePeak")), "plr": plr, "crestFactor": _coerce_nullable_number(payload.get("crestFactor")), "dynamicSpread": _coerce_nullable_number(payload.get("dynamicSpread")), diff --git a/apps/backend/tests/test_server.py b/apps/backend/tests/test_server.py index 8495baf4..e4807ea9 100644 --- a/apps/backend/tests/test_server.py +++ b/apps/backend/tests/test_server.py @@ -2798,6 +2798,23 @@ def test_plr_falls_back_to_true_peak_minus_lufs(self) -> None: phase1 = server._build_phase1(self._minimal_payload(plr=None, truePeak=-0.1, lufsIntegrated=-8.2)) self.assertEqual(phase1["plr"], 8.1) + def test_true_peak_valid_float_passes_through(self) -> None: + phase1 = server._build_phase1(self._minimal_payload(truePeak=-0.1)) + self.assertEqual(phase1["truePeak"], -0.1) + + def test_true_peak_none_stays_none(self) -> None: + # Phase 1 v2: silence has no defined dBTP, so the analyzer emits + # truePeak=null. The envelope must preserve null rather than coercing + # it to 0.0 (which would falsely report a full-scale peak). + phase1 = server._build_phase1(self._minimal_payload(truePeak=None)) + self.assertIsNone(phase1["truePeak"]) + + def test_plr_is_none_when_true_peak_is_none(self) -> None: + # With no true peak there is nothing to subtract LUFS from, so the + # PLR fallback must also stay null rather than fabricating a value. + phase1 = server._build_phase1(self._minimal_payload(plr=None, truePeak=None)) + self.assertIsNone(phase1["plr"]) + def test_mono_compatible_passes_through(self) -> None: phase1 = server._build_phase1(self._minimal_payload(monoCompatible=False)) self.assertFalse(phase1["monoCompatible"]) diff --git a/apps/ui/src/components/MeasurementDashboard.tsx b/apps/ui/src/components/MeasurementDashboard.tsx index d2eb1fb2..ccbed19b 100644 --- a/apps/ui/src/components/MeasurementDashboard.tsx +++ b/apps/ui/src/components/MeasurementDashboard.tsx @@ -1615,15 +1615,17 @@ export function MeasurementDashboard({ ))} - {/* True Peak marker */} -
- - TP {formatNumber(phase1.truePeak, 1)} - -
+ {/* True Peak marker (omitted for silence — no defined dBTP) */} + {phase1.truePeak !== null && ( +
+ + TP {formatNumber(phase1.truePeak, 1)} + +
+ )} {/* Integrated LUFS marker */}
- {/* PLR gap fill */} -
+ {/* PLR gap fill (spans true-peak → integrated; omitted for silence) */} + {phase1.truePeak !== null && ( +
+ )} {/* PLR annotation */} - {phase1.plr !== undefined && phase1.plr !== null && ( + {phase1.plr !== undefined && phase1.plr !== null && phase1.truePeak !== null && (
= 0 ? "+" : ""}${fixed} dB`; } +/** + * Phase 1 v2 emits `truePeak: null` for digital silence (no defined dBTP). + * Report the measured peak, or an em-dash when it's absent — matching the + * house convention (`formatNumber` renders missing values as "—"). + */ +export function formatTruePeak(truePeak: number | null, suffix = " dBTP"): string { + return truePeak === null ? "—" : `${truePeak.toFixed(1)}${suffix}`; +} + +/** + * Master-limiter ceiling target derived from the true peak. When the peak is + * null (silence) there is nothing to duck below, so fall back to the -0.3 dB + * safety cap — the same value the formula yields for any peak ≥ -0.2 dBTP — + * keeping the recommendation card actionable rather than showing a blank. + */ +export function masterCeilingDb(truePeak: number | null): number { + return truePeak === null ? -0.3 : Math.min(-0.3, truePeak - 0.1); +} + function midiToNoteName(midi: number): string { const noteNames = ["C", "C#", "D", "D#", "E", "F", "F#", "G", "G#", "A", "A#", "B"] as const; const clamped = Math.max(0, Math.min(127, Math.round(midi))); @@ -531,7 +550,7 @@ function getSonicMeasurements( const sets: Record = { kick: [ { icon: "🎚", label: "Low Bass", value: formatSignedDb(phase1.spectralBalance.lowBass) }, - { icon: "📏", label: "Peak", value: `${phase1.truePeak.toFixed(1)} dBTP` }, + { icon: "📏", label: "Peak", value: formatTruePeak(phase1.truePeak) }, { icon: "⏱", label: "Tempo", value: `${Math.round(phase1.bpm)} BPM` }, ], bass: [ @@ -587,7 +606,7 @@ function getSonicMeasurements( widthAndStereo: [ { icon: "↔", label: "Width", value: phase1.stereoWidth.toFixed(2) }, { icon: "📡", label: "Correlation", value: phase1.stereoCorrelation.toFixed(2) }, - { icon: "🧲", label: "Peak", value: `${phase1.truePeak.toFixed(1)} dBTP` }, + { icon: "🧲", label: "Peak", value: formatTruePeak(phase1.truePeak) }, ], harmonicContent: [ { icon: "🎼", label: "Key", value: phase1.key ?? "Unknown" }, @@ -815,7 +834,7 @@ function buildDerivedChainParameters( case "DRUM PROCESSING": return [ { label: "Tempo Sync", value: `${Math.round(phase1.bpm)} BPM` }, - { label: "Punch Target", value: `${phase1.truePeak.toFixed(1)} dBTP peak` }, + { label: "Punch Target", value: formatTruePeak(phase1.truePeak, " dBTP peak") }, ]; case "BASS PROCESSING": return [ @@ -846,7 +865,7 @@ function buildDerivedChainParameters( ); case "MASTER BUS": return [ - { label: "Ceiling", value: `${Math.min(-0.3, phase1.truePeak - 0.1).toFixed(1)} dB` }, + { label: "Ceiling", value: `${masterCeilingDb(phase1.truePeak).toFixed(1)} dB` }, { label: "Integrated Loudness", value: `${phase1.lufsIntegrated.toFixed(1)} LUFS` }, ]; default: @@ -897,7 +916,7 @@ function makeLimiterFallbackCard(phase1: Phase1Result, nextOrder: number) { trackContext: "Master", workflowStage: prettifyWorkflowStage("MASTER"), parameters: [ - { label: "Ceiling", value: `${Math.min(-0.3, phase1.truePeak - 0.1).toFixed(1)} dB` }, + { label: "Ceiling", value: `${masterCeilingDb(phase1.truePeak).toFixed(1)} dB` }, { label: "Integrated Loudness", value: `${phase1.lufsIntegrated.toFixed(1)} LUFS` }, { label: "Stereo Width", value: phase1.stereoWidth.toFixed(2) }, ], @@ -1132,7 +1151,7 @@ function buildStereoWidthPatchCard( { label: "Stereo Width", value: phase1.stereoWidth.toFixed(2) }, { label: "Correlation Floor", value: phase1.stereoCorrelation.toFixed(2) }, { label: "Bass mono below", value: "100 Hz" }, - { label: "Ceiling", value: `${Math.min(-0.3, phase1.truePeak - 0.1).toFixed(1)} dB` }, + { label: "Ceiling", value: `${masterCeilingDb(phase1.truePeak).toFixed(1)} dB` }, ], proTip: "Make width moves in the highs first, then mono-check kick and bass before committing the setting.", diff --git a/apps/ui/src/services/backendPhase1Client.ts b/apps/ui/src/services/backendPhase1Client.ts index 05f7082d..3ba9c12d 100644 --- a/apps/ui/src/services/backendPhase1Client.ts +++ b/apps/ui/src/services/backendPhase1Client.ts @@ -556,9 +556,12 @@ export function parsePhase1Result(value: unknown): Phase1Result { } const lufsIntegrated = expectNumber(phase1, "lufsIntegrated"); - const truePeak = expectNumber(phase1, "truePeak"); + // Phase 1 v2: truePeak is `null` for digital silence (no defined dBTP). Accept + // an explicit null, but still reject a malformed/missing field via expectNumber. + const truePeak = phase1.truePeak === null ? null : expectNumber(phase1, "truePeak"); const explicitPlr = toNumber(phase1.plr); - const normalizedPlr = explicitPlr ?? roundToTwoDecimals(truePeak - lufsIntegrated); + const normalizedPlr = + explicitPlr ?? (truePeak !== null ? roundToTwoDecimals(truePeak - lufsIntegrated) : null); const monoCompatible = phase1.monoCompatible === true ? true : phase1.monoCompatible === false diff --git a/apps/ui/src/types/measurement.ts b/apps/ui/src/types/measurement.ts index 0cf2bb63..87cf5ecd 100644 --- a/apps/ui/src/types/measurement.ts +++ b/apps/ui/src/types/measurement.ts @@ -652,8 +652,8 @@ export interface Phase1Result { lufsRange?: number | null; lufsMomentaryMax?: number | null; lufsShortTermMax?: number | null; - /** Max true peak in dBTP (Phase 1 v2; was a linear amplitude proxy in v1). 0.0 == full scale, > 0 == inter-sample over. */ - truePeak: number; + /** Max true peak in dBTP (Phase 1 v2; was a linear amplitude proxy in v1). 0.0 == full scale, > 0 == inter-sample over. `null` for digital silence (no defined dBTP). */ + truePeak: number | null; plr?: number | null; crestFactor?: number | null; dynamicSpread?: number | null; diff --git a/apps/ui/src/utils/exportUtils.ts b/apps/ui/src/utils/exportUtils.ts index 23a368c7..2b42de7e 100644 --- a/apps/ui/src/utils/exportUtils.ts +++ b/apps/ui/src/utils/exportUtils.ts @@ -54,7 +54,7 @@ export function generateMarkdown( md += `- **Time Signature**: ${phase1.timeSignature}\n`; md += `- **Duration (s)**: ${phase1.durationSeconds}\n`; md += `- **Integrated LUFS**: ${phase1.lufsIntegrated}\n`; - md += `- **True Peak**: ${phase1.truePeak} dBTP\n`; + md += `- **True Peak**: ${phase1.truePeak === null ? '—' : `${phase1.truePeak} dBTP`}\n`; md += `- **Stereo Width**: ${phase1.stereoWidth}\n`; md += `- **Stereo Correlation**: ${phase1.stereoCorrelation}\n\n`; diff --git a/apps/ui/tests/services/analysisResultsViewModel.test.ts b/apps/ui/tests/services/analysisResultsViewModel.test.ts index 53da2cc2..aa1b9b0c 100644 --- a/apps/ui/tests/services/analysisResultsViewModel.test.ts +++ b/apps/ui/tests/services/analysisResultsViewModel.test.ts @@ -5,6 +5,8 @@ import { buildPatchCards, buildPatchGroups, buildSonicElementCards, + formatTruePeak, + masterCeilingDb, toConfidenceBadges, truncateAtSentenceBoundary, truncateBySentenceCount, @@ -787,4 +789,38 @@ describe('analysisResultsViewModel helpers', () => { const confidenceRow = guide?.parameters.find((p) => p.label === 'Melody Confidence'); expect(confidenceRow?.value).toBe('72% (Workable draft)'); }); + + it('renders true peak as an em-dash when the v2 measurement is null (silence)', () => { + // Phase 1 v2 emits truePeak: null for digital silence (no defined dBTP). + // Building the card must not crash on an unguarded .toFixed and the Peak + // readout should fall back to the house "—" convention. + const silent = { ...measurement, truePeak: null }; + const sonicCards = buildSonicElementCards(silent, { + kick: 'Kick sentence.', + bass: 'Bass sentence.', + melodicArp: 'Arp sentence.', + grooveAndTiming: 'Groove sentence.', + effectsAndTexture: 'Fx sentence.', + widthAndStereo: 'Width sentence.', + harmonicContent: 'Harmony sentence.', + }); + const kickCard = sonicCards.find((card) => card.id === 'kick'); + const peakRow = kickCard?.measurements.find((m) => m.label === 'Peak'); + expect(peakRow?.value).toBe('—'); + }); + + it('formatTruePeak renders dBTP for a number and an em-dash for null', () => { + expect(formatTruePeak(-0.2)).toBe('-0.2 dBTP'); + expect(formatTruePeak(0)).toBe('0.0 dBTP'); + expect(formatTruePeak(-1.3, ' dBTP peak')).toBe('-1.3 dBTP peak'); + expect(formatTruePeak(null)).toBe('—'); + }); + + it('masterCeilingDb ducks below the peak but falls back to the -0.3 dB cap for null', () => { + // A hot peak yields peak - 0.1; any peak ≥ -0.2 is capped at -0.3; silence + // (null) has no peak to duck below, so the same -0.3 safety cap applies. + expect(masterCeilingDb(-3.0)).toBeCloseTo(-3.1, 5); + expect(masterCeilingDb(0)).toBeCloseTo(-0.3, 5); + expect(masterCeilingDb(null)).toBeCloseTo(-0.3, 5); + }); });