Skip to content

fix(loudness): Phase 1 v2 — truePeak→dBTP, bpmConfidence→0–1, coherent PLR - #128

Merged
slittycode merged 4 commits into
mainfrom
claude/practical-hypatia-0l9jF
Jun 2, 2026
Merged

fix(loudness): Phase 1 v2 — truePeak→dBTP, bpmConfidence→0–1, coherent PLR#128
slittycode merged 4 commits into
mainfrom
claude/practical-hypatia-0l9jF

Conversation

@slittycode

Copy link
Copy Markdown
Owner

Summary

This is PR-A of a staged program (see session). It is the Phase 1 schema v2 migration for the loudness fields — WS1 of the "what's next" plan, which a consumer audit grew from a small fix into a deliberate units migration.

Why: mapping every consumer of truePeak showed the codebase already overwhelmingly assumed dBTP, while the field actually emitted a linear amplitude proxy:

  • analysisResultsViewModel.ts computed master ceilings as min(-0.3, truePeak - 0.1) → always -0.3 for a linear truePeak ≈ 1.0.
  • MeasurementDashboard.tsx rendered { value: truePeak, suffix: 'dBTP' } → showed 1.00 dBTP instead of 0.0.
  • analyze_core.analyze_plr computed truePeak_linear - lufsIntegrated — a linear amplitude minus a dB value (incoherent).
  • Only loudnessGuardrails.ts correctly treated it as linear.

Plus bpmConfidence was the only *Confidence field not in 0–1 (raw Essentia ~0–5.32), which exportUtils.ts rendered as up to ~500%.

Rather than convert at ~20 fragile consumer sites, we fix the source. This makes the majority of consumers correct by default and matches the convention the WASM loudness package also uses.

Changes

  • truePeak → dBTP (0.0 == full scale, > 0 == inter-sample over; null for silence).
  • bpmConfidence → normalized 0–1 (raw / 5.0, clamped).
  • plr is now a coherent dB-domain subtraction (truePeak_dBTP − lufsIntegrated) in analyze_core, the server_phase1 HTTP fallback, and the phase1_evaluation consistency gate. Unit (LU) unchanged — a correctness fix.
  • New top-level phase1Version = "phase1.v2" so consumers can detect the generation (ADR 0001's prescribed v2 mechanism).
  • loudnessGuardrails over-check moves to dBTP (> 0); phase2Validator drops its special-case bpmConfidence threshold (now shares the standard 0.4 hedge threshold).
  • loudness_rec_evaluation (research-only) converts the now-dBTP peak back to linear for its limiter-ceiling reachability math.
  • Re-baselined the Phase 1 golden (truePeak: 0.0, plr: 5.6); added dedicated analyze_plr (dB subtraction) and dBTP analyze_true_peak unit tests; updated fixture expectations, Phase1Result typing, and JSON_SCHEMA.md.
  • ADR 0002 records the v2 bump, superseding ADR 0001 for these two fields only.

Test plan

  • apps/backend: ./venv/bin/python -m unittest discover -s tests — green.
  • Golden re-baselined deliberately (UPDATE_PHASE1_GOLDEN=1); diff is exactly phase1Version added + plr 6.6→5.6 + truePeak 1.0→0.0.
  • apps/ui: npm run lint (tsc) clean, npm run test:unit 716/716, npm run build clean.
  • Smoke (npm run test:smoke) runs in CI (needs the live stack).

Notes

This is a breaking Phase 1 contract change under ADR 0001's policy (units + range), taken deliberately and recorded in ADR 0002 — acceptable here because ASA is effectively its own only consumer, and the change makes the payload more conventional. All other fields are unchanged.

https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q


Generated by Claude Code

claude added 2 commits June 2, 2026 00:01
…oherent 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
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

@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: COMMENT

Summary

A deliberate, well-scoped Phase 1 schema migration: truePeak moves from a linear amplitude proxy to dBTP, bpmConfidence normalizes to 0–1, and plr becomes a coherent dB-domain subtraction. The math is correct throughout, the ADR documents the breaking change properly, and the new TestAnalyzePlrIsDbtpMinusLufs / TestAnalyzeTruePeakEmitsDbtp test classes are genuinely meaningful (not coverage padding). CI was still in progress at review time; smoke tests are gated there.

Findings

Should fix

analysisResultsViewModel.ts — three display sites still label truePeak with "dB" instead of "dBTP" (lines 534, 590, 818: `${phase1.truePeak.toFixed(1)} dB` and `${phase1.truePeak.toFixed(1)} dB peak`). The values are now correct (the v2 migration fixed the semantics), but the unit suffix is wrong and will confuse anyone comparing the display to a true-peak meter. Lines 849/900/1135 (the derived Math.min(-0.3, truePeak - 0.1) ceiling) are fine — the formula is now semantically meaningful for the first time and the "dB" suffix on a ceiling number is close enough. MeasurementDashboard.tsx:1670 already has suffix: 'dBTP' correct.

Worth considering

Phase1Result.truePeak is typed number (non-nullable) in measurement.ts, but the backend can now emit null for silent signals and already could emit null on TruePeakDetector failure. analysisResultsViewModel.ts calls phase1.truePeak.toFixed(1) without a null guard in multiple places; TypeScript won't catch this because the type says it's always a number. This is pre-existing, but the v2 change makes the null path more reachable (any silent signal, not just a crashed detector). Worth typing it as number | null and guarding the display callsites.

On the bpmConfidence threshold shift

The validator's effective low-confidence threshold moves from raw 1.0 → raw 2.0 (i.e., normalized 0.2 → 0.4). This is correct: the JSON_SCHEMA.md v1 docs always said "above 2.0 is reliable" while the validator was inconsistently testing against 1.0. The v2 migration aligns code to its own documentation. Not a concern — just noting it's more than a unit change.

Test results

CI still running at review time (Backend, Frontend, Loudness WASM in-progress; Chromatic passed). Can't report final counts. The smoke gate (npm run test:smoke) is deferred to CI per the PR checklist.

New unit tests verified by inspection:

  • analyze_plr(-8.9, 0.0)["plr"] == 8.9
  • analyze_plr(-9.0, -1.2)["plr"] == 7.8
  • 20*log10(1.032) ≈ 0.27 → rounds to 0.3 dBTP ✓
  • 20*log10(0.8913) ≈ -1.0 dBTP ✓
  • Golden delta (truePeak 1.0 → 0.0, plr 6.6 → 5.6) consistent with lufsIntegrated ≈ -5.6
  • Audio fixture delta (plr 9.9 → 8.9) consistent with lufsIntegrated ≈ -8.9

Phase boundary check

Clean. No Phase 2/3 code overrides Phase 1 measurements. PLR derivation in analyze_core / server_phase1 reads measured fields and produces a derived value — not an override, same as before. loudness_rec_evaluation.py's linear reconversion is research-only and correctly isolated.


Generated by Claude Code

claude added 2 commits June 2, 2026 00:13
…bels

- 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
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
@slittycode
slittycode merged commit d576583 into main Jun 2, 2026
4 checks passed
@slittycode
slittycode deleted the claude/practical-hypatia-0l9jF branch June 2, 2026 03:44
slittycode pushed a commit that referenced this pull request Jun 2, 2026
Stacked on the Phase 1 v2 branch (#128), the prompt's loudness guidance now
matches what the backend actually emits under v2:

- truePeak framing corrected from the v1 "linear amplitude proxy; >1.0 is an
  over" to dBTP ">0.0 dBTP is an inter-sample over; null for silence" — in
  both the citation hint table and the load-bearing MUST loudness-defect rule.
  Under v1 these would now tell Gemini the wrong over-threshold.
- New PLR-driven mastering decision bands (plr = truePeak_dBTP − lufsIntegrated):
  PLR <=8 already-limited (don't pile on), 8-12 transparent limiter, >=14 dynamic
  (glue into a limiter). Plus a worked Glue Compressor card citing plr +
  lufsIntegrated. plr added to the Saturator/Limiter citation row.
- bpmConfidence hedge at the v2 normalized 0.4 threshold (matches the frontend
  validator's NORMALIZED_LOW_CONFIDENCE_THRESHOLD): hedge tempo-locked advice
  and name the half/double-time alternative below 0.4.

All cited paths (plr, truePeak, bpmConfidence, lufsIntegrated, crestFactor) are
real top-level Phase1Result fields, so the citation-existence validator accepts
them. test_phase2_prompt_catalog gains a regression case pinning the v2 units +
new rules; the PLR worked card validates against the Live 12 catalog.

Descriptor-hooks expansion (snare/hihat/reverb in _build_descriptor_hooks) is
deferred — those fields are already in the citable catalog and the hook work
carries the documented invented-citation hazard; a focused follow-up is safer.

https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q
slittycode added a commit that referenced this pull request Jun 2, 2026
…ge (#131)

* WS2: Phase 2 prompt — v2 loudness units, PLR mastering rules, bpm hedge

Stacked on the Phase 1 v2 branch (#128), the prompt's loudness guidance now
matches what the backend actually emits under v2:

- truePeak framing corrected from the v1 "linear amplitude proxy; >1.0 is an
  over" to dBTP ">0.0 dBTP is an inter-sample over; null for silence" — in
  both the citation hint table and the load-bearing MUST loudness-defect rule.
  Under v1 these would now tell Gemini the wrong over-threshold.
- New PLR-driven mastering decision bands (plr = truePeak_dBTP − lufsIntegrated):
  PLR <=8 already-limited (don't pile on), 8-12 transparent limiter, >=14 dynamic
  (glue into a limiter). Plus a worked Glue Compressor card citing plr +
  lufsIntegrated. plr added to the Saturator/Limiter citation row.
- bpmConfidence hedge at the v2 normalized 0.4 threshold (matches the frontend
  validator's NORMALIZED_LOW_CONFIDENCE_THRESHOLD): hedge tempo-locked advice
  and name the half/double-time alternative below 0.4.

All cited paths (plr, truePeak, bpmConfidence, lufsIntegrated, crestFactor) are
real top-level Phase1Result fields, so the citation-existence validator accepts
them. test_phase2_prompt_catalog gains a regression case pinning the v2 units +
new rules; the PLR worked card validates against the Live 12 catalog.

Descriptor-hooks expansion (snare/hihat/reverb in _build_descriptor_hooks) is
deferred — those fields are already in the citable catalog and the hook work
carries the documented invented-citation hazard; a focused follow-up is safer.

https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q

* WS2 review: close the PLR band gap (extend middle band to 8–14 LU)

The PLR mastering bands were ≤8 / 8–12 / ≥14, leaving 12–14 LU with no
instruction (a PLR-13 master had nothing to anchor to). Extend the middle
band to 8–14 LU so the bands are contiguous. Regression test pins it.

https://claude.ai/code/session_01Wq1vE1D7o3W9xZKSHXz43Q

---------

Co-authored-by: Claude <noreply@anthropic.com>
slittycode added a commit that referenced this pull request Jun 2, 2026
…ard (#135)

Two defects surfaced by an independent review of the loudness-v2 program
(#128#133):

patchSmith read spectralBalance bands as "dB above/below the mix average", but
the analyzer emits absolute band energy (10·log10(mean_energy), ~ -100..0). On
real tracks the sub oscillator never engaged (guard subBass<=1.0 against a real
~ -41 dB reading) and the acid cutoff pinned to the 200 Hz floor, while the
cited rationale misstated the measurement (invariant #2). Fix locally: add
meanBandDb() and read each band as a prominence relative to the 7-band mix
average — the analyzer contract is unchanged (mixDoctor/dashboard rely on the
absolute scale). Rationales are now true; tests use realistic absolute dB with
a regression case that fails on the old logic.

phase1Version was emitted but never consumed, so persisted pre-#128 runs
(stored verbatim in analysis_runtime, never migrated) replayed v1-unit values
into v2 consumers: a linear truePeak of 0.98 read as a +0.98 dBTP over (false
TRUE_PEAK_OVER / MISSING_LOUDNESS_ACTION) and a raw bpmConfidence of 3.2
rendered as 320%. Add _migrate_v1_loudness_units() at the top of _build_phase1
(the single chokepoint every persisted snapshot passes through): a payload
lacking "phase1.v2" gets truePeak linear→dBTP, bpmConfidence raw→0-1, plr
dropped and recomputed, then stamped v2. No-op for v2 payloads.

Verified: tsc clean; 750 frontend unit tests; vite build; 1102 backend
unittests (skipped=3). Playwright smoke could not run (chromium download
blocked by the sandbox network allowlist); both changes are pure logic covered
by the suites that ran.

https://claude.ai/code/session_01C2VkJk4JAUGtz3b9BWqVGE

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