Epic 4: define and fill the Energy scale (ADR-0015) - #44
Merged
Merged
Conversation
`cache.audio_features.energy` fed six read surfaces — the browser column, the smartlist `energy` field, Mixable Tracks, both Field Mapping profiles, the Track Timeline and Playlist Tools' energy sort — and no production path had ever written a non-NULL value into it. The only non-NULL energies in the repository were test fixtures. ADR-0015 defines the scale, per `docs/lexicon/04-analysis.md §Energy` and the request in `GAPS.md` open question 2 to write it down before implementing: an absolute 1-10 from loudness (0.35), percussive drive (0.25), brightness (0.25) and tempo (0.15), each anchored to a fixed physical quantity so a track's number never moves because the library moved around it. - Loudness takes the louder half of the RMS envelope, so an ambient intro does not drag a loud track down. Drive counts rises only, since signed differences sum to ~0 over a track. Both drive and brightness are ratios, so mastering level moves the loudness term and nothing else. - Brightness inverts `rms(diff(x))/rms(x) = 2*sin(pi*f/fs)` rather than running an FFT: one pass, no dependency, tested to 5% against synthesised sines and pinned invariant under sample rate and volume. - Stored 0.1-1.0 so the `(e*10).round()` mapping `sync_mappings.rs` and `write_tags.rs` already use lands in 1-10; Lexicon's scale has no zero. - `ANALYZER_VERSION` -> `stratum-dsp-v2`, so v1 rows (BPM + key, NULL energy) stop satisfying the cache lookup and existing libraries actually gain energies. - `libebur128` (ADR-0012) deliberately not pulled in for one term of four; the swap to gated LUFS is contained to `energy::loudness_dbfs` plus a version bump, and is recorded in the ADR as a known approximation. No new UI: `analyze_file_cached` is the single fill path, so the context-menu Analyse, watch-folder arrivals and the agent tools gain it at once. The Energy bar now announces "Energy 7 of 10" rather than a raw 0.42, and the analysis toast reports it beside BPM and key. Parity: 61 done / 19 partial / 14 missing / 2 blocked / 16 deferred. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gn43w2xFL3JRBRkMv3vRo
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughAdded absolute audio energy analysis with weighted feature extraction, cache persistence, and analyzer versioning. The frontend now displays stored energy as 1–10 while retaining proportional 0–1 visual fills. Documentation and test fixtures were updated. ChangesEnergy analysis
Write Tags documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AudioFile
participant analyze_file_cached
participant energy_analyze
participant AnalysisCache
participant EnergyBar
AudioFile->>analyze_file_cached: decoded samples and BPM
analyze_file_cached->>energy_analyze: analyze samples
energy_analyze-->>analyze_file_cached: energy 0.1–1.0
analyze_file_cached->>AnalysisCache: store energy with analysis result
AnalysisCache-->>EnergyBar: cached analysis result
EnergyBar-->>EnergyBar: display energy as 1–10
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Both `PARITY.md` and `06-files.md §Write Tags` said "no field mappings, no auto-write". Both shipped in Epic 4 — `write_tags::apply_mappings` runs the same profile machinery as Sync, and `AUTO_WRITE_TAGS` is wired in `watch.rs`. The same stale claim had already been found once, on the Automatic Actions row. The row stays `partial`, because auto-write is genuinely narrower than the manual's "whenever a change is detected": it fires on watch-folder arrivals only and writes BPM and key only. That is now the stated gap, in place of two things that are not gaps at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Gn43w2xFL3JRBRkMv3vRo
cole-hackman
marked this pull request as ready for review
August 8, 2026 08:25
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.
Closes the
Energyrow perdocs/lexicon/04-analysis.md §Energy, and closesGAPS.mdopen question 2.The problem
cache.audio_features.energyfed six read surfaces — the browser Energy column, the smartlistenergyfield, Mixable Tracks' energy rule, both Field Mapping profiles, the Track Timeline, and Playlist Tools' energy sort — and no production path had ever written a non-NULL value into it.analyze_file_cachedpassedNoneon every analysis. The only non-NULL energies anywhere in the repository were two test fixtures.Nothing lied, so this is not an ADR-0008 violation by claim; it is one by omission. The column was always empty, and the browser tooltip reported a raw
0.62on no published scale at all.The scale (ADR-0015)
GAPS.mdasked for the definition to be written down before implementing, because Lexicon itself ships two mutually incompatible energy scales and a third invented by accident would be worse than none. Absolute 1–10:"Absolute" is the spec's own word, and it is what rules out ranking or percentile normalisation — every anchor is a fixed physical quantity, so a track's number never moves because the library moved around it.
Shape decisions, each from asking what would be wrong rather than what would be easy:
rms(diff(x))/rms(x) = 2·sin(π·f/fs)exactly, so inverting recovers the frequency. One pass, no dependency.Two things stated rather than smoothed over
A silent file at 128 BPM scores 2, not 1. Tempo is still a real measurement of that file and it is 15% of the score. No single term can carry the number, so none can sink it either. Asserted by test rather than special-cased.
libebur128deliberately not pulled in, though ADR-0012 adopted it for ITU-R BS.1770 loudness. A gated LUFS reading would be a better loudness term than frame RMS. It is one term of four and the crate is not otherwise in the tree; the swap is contained toenergy::loudness_dbfsplus a version bump. Recorded in the ADR as a known approximation, not left implicit.The weights themselves are a judgement call, tuned against synthesised signals —
fixtures/audio/holds only a.gitkeepby design, so there is nothing in the repository to validate against. They are stated in the ADR so they can be argued with, and they live in one place.ANALYZER_VERSION→stratum-dsp-v2A v1 row has BPM and key but a NULL energy. Without the bump it satisfies the cache lookup forever, no existing library ever gains energies, and the feature looks shipped while doing nothing.
Reachability
No new UI.
analyze_file_cachedis the single fill path, so the context-menu Analyse, watch-folder arrivals and the agent tools all gain it at once. What changed for the user: the Energy bar's tooltip and ARIA range now sayEnergy 7 of 10instead of0.42, and the analysis toast reports energy beside BPM and key. The converter lives inapps/desktop/src/lib/energy.tsrather than beside the bar, because three consumers with a copy each is how the two halves of a scale drift apart — its rounding is tested against the Rust half's boundaries.Verification
Full definition of done, all green:
New tests: 21 in
crates/audio-analysis/src/energy.rs(anchors, clamping, per-term monotonicity, NaN containment, sine-frequency recovery to 5%, sample-rate and volume invariance for both ratios, dBFS against known amplitudes, determinism, full 1–10 coverage with no gaps) and 5 inapps/desktop/src/lib/energy.test.tspinning the cross-language rounding.Parity: 61 done / 19 partial / 14 missing / 2 blocked / 16 deferred.
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation