feat(editor): audio mixer — recording/guide/click faders, first-play fade, edit-preview blip - #109
Merged
Merged
Conversation
…st-play fade, edit blip Roadmap 1.4 (Phase 1, "Hear your chart"). A Mix toolbar button (Shift+C in both shortcut profiles) opens a 3-fader popover over the shipped guide bus: - Recording fader = a new TRANSPARENT gain node straight to destination — the reference still never sums through the guide limiter; unity default. - Guide / click faders drive the existing bus gains; defaults preserve the shipped 0.35 / 0.25 balance exactly. All levels persist as editor prefs (never pack data) and apply with a ~20 ms setTargetAtTime ramp. - Hearing safety: the first play each session fades the recording in from ~30% over 0.35 s. - Edit-preview blip (toggleable, default on): a soft 1320 Hz tick through the limited guide bus confirms note adds and pitch changes only (fret set/adjust, string moves, pitch-changing drags) — never marquee or time-only moves; group edits rate-limit to one cue; never fires when the audio context is not running. Tests: tests/audio_mixer.test.js drives the real _ensureMasterBus / _ensureRefGain / _mixSetBusGain / _editBlipAt against stub AudioContext + localStorage — the bus-seeding and blip assertions fail on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Mix audio-mixer popover with persisted faders, smoother gain control, a first-play reference fade, and an optional edit-blip cue wired into several edit flows. It also adds mixer UI wiring and a dedicated test suite. ChangesAudio Mixer and Edit Blip
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant editorToggleMixer
participant editorSetMixLevel
participant screenjs as screen.js
participant AudioContext
User->>editorToggleMixer: open or close mixer popover
User->>editorSetMixLevel: change ref / guide / click level
editorSetMixLevel->>screenjs: _mixSetBusGain(bus, val)
screenjs->>AudioContext: setTargetAtTime on gain node
screenjs->>AudioContext: route ref audio through _ensureRefGain()
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@screen.js`:
- Around line 5758-5777: Reset the first-play fade state when a new
song/recording is loaded so the hearing-safety ramp runs again for each track.
Update the load flow in loadCDLC() and the create/import path to clear
_mixFirstPlayDone (and reinitialize any related ref-gain state if needed) before
playback starts, so _mixApplyFirstPlayFade() can apply the fade on every new
song instead of only once per session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f8638609-84dc-4658-a13a-c49f22131111
📒 Files selected for processing (4)
CHANGELOG.mdscreen.htmlscreen.jstests/audio_mixer.test.js
Review follow-ups on the audio mixer: - Edit-preview blip now sums straight into the shared limiter instead of through the guide fader gain, so muting guide claps no longer also silences the edit cue (still limited). Test + CHANGELOG updated. - aria-label on each fader + aria-live on the percent readouts so screen readers announce level changes during a drag. - Correct the _mixDragChangedPitchPure comment: it fires on any string or fret delta (both change pitch), not only keys-mode fret deltas. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ding CodeRabbit flagged that _mixFirstPlayDone was a true once-per-screen-boot latch: loadCDLC() and the create/import/replace-audio load paths never cleared it, so only the first song of a session got the hearing-safety fade-in — every later song jumped straight to full target gain. Add _mixResetFirstPlay() (audio-bus pure block) and call it from loadAudio() right after a new buffer decodes successfully — the single chokepoint all three load paths (loadCDLC, create/import, replace-audio) funnel through — so the ramp re-arms per loaded recording, not per session. Test + CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
Addressed in cd39292: added _mixResetFirstPlay() (audio-bus pure block) and wired it into loadAudio(), the single chokepoint loadCDLC/create-import/replace-audio all funnel through. The fade now re-arms per newly-loaded recording rather than once per screen boot. New test covers it; CHANGELOG updated. |
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
byrongamatos
added a commit
that referenced
this pull request
Jul 7, 2026
…op pass (#122) * feat(editor): loop A/B compare — alternate recording and guide per loop pass Roadmap 1.6, the last Phase-1 slice that was waiting on #109 (it mutes the reference through the mixer transparent _refGain). The ear-training loop: while looping, each pass alternates RECORDING (reference audible, claps off) and GUIDE (reference muted, claps on — overriding the claps pref in both directions), so a charter hears their chart against the artist one pass apart. - Phase flips ride the loop wrap (before _restartPlaybackAt so the ramp lands with the wrap); every (re)start/seek begins on the recording pass; stop mid-guide-pass restores the fader level; clearing the loop region disarms A/B; song load resets it (session-only by design). - Arming A/B with a region set but looping off arms the loop too. - _guideTimerSync now schedules whenever A/B is active (guide passes need claps even with the pref off). - Toolbar A/B button next to Loop + registry toggleLoopAB (Alt+B). Tests: tests/loop_ab.test.js (5) — pref-override truth table, strict two-cycle, mute-only-during-active-playing-guide (incl. the stop-restores case), and a composed 4-pass alternation. Full suite green except the pre-existing CRLF section_coverage failure (#116). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * feat(editor): review fixes for #122 (loop A/B) - ref fader no longer un-mutes an active A/B guide pass (route ref bus moves through the A/B-aware target; guarded for the @pure sandbox) - mid-play A/B arming delegates to _setLoopRegionEnabled so the cursor seeks into the loop region instead of riding pre-loop audio - disabling the loop mid-guide-pass restores the recording to its fader level instead of leaving it silently muted - regression tests drive the real stateful runtime; all 3 fail pre-fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(editor): address CodeRabbit review on #122 (loop A/B) - reset A/B phase + ref gain BEFORE the first playbackTick/_guideTimerSync so a fresh start/seek can't schedule a guide pass off a stale phase (and the first-play fade stays the last ref-gain automation) - on song load, also reapply ref gain + sync scheduler + refresh controls so a load mid-guide-pass never leaves a stale mute or button styling Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
This was referenced Jul 8, 2026
This was referenced Jul 15, 2026
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.
Roadmap 1.4 (Phase 1 — "Hear your chart"): the mixer + safety layer over the guide-clap bus shipped in #90/#91. With this, Phase 1 is complete except the wafonts-gated pitched synth (1.2/1.5).
What
Shift+C, both shortcut profiles; registry commandtoggleMixer) opens a popover with three faders:setTargetAtTimeramp — a fader move is never a stepped jump mid-audio.Tests
tests/audio_mixer.test.js(15 cases): pure math (clamping, linear gain map, first-play floor, rate limit, drag pitch detection) plus stateful drives of the real_ensureMasterBus/_ensureRefGain/_mixSetBusGain/_editBlipAtagainst a stub AudioContext + localStorage. The bus-seeding, ramp, fade-once, and blip assertions fail on main. Full JS suite: 42 files green;node --checkclean.Fresh region vs the open queue (#101–#108) — audio engine + new popover only.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu
Summary by CodeRabbit