fix(viz): sakura + heartbeat AGC; remove firework#64
Merged
Conversation
Live testing after v1.2.0 surfaced three viz bugs in the same class as CLI-89: amplitude-scaling math tuned for ≈1.0 signals fails at typical listening volumes (peak_sample ≈ 0.05–0.15). sakura — black screen until energy ≥ 0.33. `spawn_count = (energy * 3.0) as usize` truncated to zero at quiet volume. Routed spawn intensity through `SpectrumScaler::normalise` so intensity is in per-frame-peak-normalised units, then added a stochastic carry on the fractional remainder so a normalised 0.05 still yields ≥1 petal occasionally (continuous precipitation at quiet volume, dense at loud). EnergyTracker dropped — it had no other users in this viz. heartbeat — flat line. Raw bipolar samples at ~0.05 peak multiplied by `sample * |sample|` compressed to ~0.0025, collapsing the Y excursion to one pixel above the baseline. Added a sibling `SampleScaler` (envelope-follower AGC over `|sample|`, mirrors the SpectrumScaler attack/release philosophy) and normalise each sample before the `n * |n|` spike shape. Trace now lifts off the baseline at any reasonable listening volume and clamps cleanly on loud transients. firework — black screen. Transient detector (`delta > TRANSIENT_THRESHOLD = 0.12`) never tripped at normal volume, so no fireworks ever launched. Full deletion: module, re-export, enum variant, parse/as_str arms, catalogue entry (23 → 22), pane_mode registration, fuzzy/command_bar test catalogues, README family list, and the cli.md reference row. Historical CHANGELOG entries left intact — they describe prior releases that shipped firework. Closes clitunes-wwv / CLI-97
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
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
SpectrumScaler::normalise+ stochastic fractional-carry so quiet volumes still precipitate.SampleScaler(sample-domain AGC, mirrors the SpectrumScaler attack/release philosophy) lifts the ECG trace off the baseline at typical listening volumes.Why
Live-testing v1.2.0 surfaced three viz bugs in the same class as CLI-89: amplitude-scaling math tuned for ≈1.0 signals silently fails at typical listening volumes (peak_sample ≈ 0.05–0.15).
sakuraspawn_count = (energy * 3.0) as usizetruncates to 0 until energy ≥ 0.33heartbeatfireworkdelta > TRANSIENT_THRESHOLDnever trips at normal volumeThe sakura and heartbeat fixes extend the CLI-89 pattern (PR #57): put the input on an envelope-follower AGC so typical listening volume reads as full-scale visual amplitude, with fast attack + slow release so transients pop and the gain ceiling doesn't strobe on brief dips.
SampleScaleris a bipolar-sample sibling toSpectrumScaler—SpectrumScalerdoes log/dB compression on magnitudes (non-negative), which doesn't cleanly map to[-1, 1]PCM samples; the sample-domain tracker keeps sign and tracks|sample|peak directly.Firework's transient detector was the wrong reactive primitive for ambient audio — it required a sharp spike in already-smoothed energy, which essentially never occurs outside contrived test fixtures. Rather than re-tune (and introduce another ghost viz in the carousel), removed entirely.
Test plan
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace --all-featuresscaling::sample_scaler_quiet_signal_fills_majority_of_rangescaling::sample_scaler_preserves_signscaling::sample_scaler_loud_transient_clamps_without_overshootscaling::sample_scaler_silence_does_not_auto_gainscaling::sample_scaler_attack_faster_than_releasesakura::quiet_listening_volume_still_spawns_petals(≥10 petals over 30 frames at ~0.05 peak)heartbeat::quiet_listening_volume_lifts_trace_off_baseline(≥20% of pane height)heartbeat::loud_volume_saturates_without_overshoot(≥80% peak, clamped ≤1.0)pto cycle tosakura, verify petals rain continuously at normal volumeheartbeat, verify the ECG trace sweeps across the pane instead of sitting flat on the baseline:viz fireworkis rejected (name no longer in the catalogue)