feat(editor): active-source switching — chart against an isolated stem - #296
Merged
Conversation
|
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 (18)
📝 WalkthroughWalkthroughThe editor now models master and stem audio as live sources with stable metadata, offsets, and cache names. Tracks can activate a source for waveform and onset analysis without changing playback. The mixer renders and orders a Master Mix strip, while waveform rendering, tempo-guide behavior, and removal return values are updated. ChangesAudio source contracts and session loading
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ChrisBeWithYou
force-pushed
the
feat/editor-mixer-meters
branch
from
July 16, 2026 03:54
dc09c19 to
2d71828
Compare
ChrisBeWithYou
force-pushed
the
feat/editor-active-source
branch
from
July 16, 2026 03:54
fe73592 to
3e195de
Compare
ChrisBeWithYou
force-pushed
the
feat/editor-mixer-meters
branch
from
July 16, 2026 03:57
2d71828 to
85395ef
Compare
ChrisBeWithYou
force-pushed
the
feat/editor-active-source
branch
from
July 16, 2026 03:57
3e195de to
378ccb5
Compare
ChrisBeWithYou
force-pushed
the
feat/editor-mixer-meters
branch
from
July 16, 2026 04:11
85395ef to
ff7807b
Compare
ChrisBeWithYou
force-pushed
the
feat/editor-active-source
branch
from
July 16, 2026 04:11
378ccb5 to
e1f4e49
Compare
byrongamatos
added a commit
that referenced
this pull request
Jul 16, 2026
…lo/mute Byron's product call: audio:master is the final OUTPUT destination (Logic/Ableton master bus), not a peer channel. Another stem/part's mute or solo removes only that channel's own contribution and must never silence the master output. Master's OWN mute (the output fader) still mutes it. Root cause was the shared audibility rule _mixerPartAudiblePure, where #296 made the master a participant in the whole-map solo rule — so soloing a stem or muting a part zeroed the active master reference gain (host.partStripState → _ensureStemGain/applyStemMix all route through this one rule). Exclude audio:master from the solo rule while keeping its own mute check ahead of the guard. Regression test fails pre-fix: with a stem soloed AND a different track muted, audio:master stays audible; muting the master strip itself still mutes it (over-correction guard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
@coderabbitai review |
Collaborator
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
tests/mixer_panel.test.mjs (1)
115-130: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the master strip in the ordering test.
The fixture omits
audio:master, even thoughsrc/mixer-panel.jsnow prepends it and sends unlisted keys to the tail. Assert the intended result whenaudio:masteris both present and absent fromorderedKeys; otherwise a host/order contract mismatch can silently move the master strip away from its required position.🤖 Prompt for 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. In `@tests/mixer_panel.test.mjs` around lines 115 - 130, Update the ordering test around _mixerOrderedPartsPure to include an audio:master fixture and assert it remains in the required prepended position when orderedKeys includes it. Also add coverage for when audio:master is absent from orderedKeys, verifying the implementation’s unlisted-key tail behavior and preserving the expected relative order of the remaining strips.
🤖 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 `@CHANGELOG.md`:
- Line 10: Update the changelog headings so only one `### Added` section
remains: merge the later Added content into the existing section or rename the
later heading appropriately, preserving all entries and avoiding duplicate
markdown headings.
- Around line 17-18: Clarify the CHANGELOG terminology by distinguishing the
mixer’s master strip from the selectable Master source row in the Tracks pane,
then revise the entries around the Tracks pane and Master row so they
consistently describe their respective placement and controls.
In `@routes.py`:
- Around line 4202-4207: Update the synthetic master source construction and
related source-ID handling near _audio_sources so its ID uses a reserved runtime
namespace that cannot collide with manifest stem IDs such as "master". Keep stem
IDs unchanged, and ensure all frontend-facing references to the synthetic master
use the new namespaced ID consistently.
In `@src/audio.js`:
- Around line 1775-1791: Update the active-source repair logic around
activateTrackAudioSource so a failed fallback activation is not treated as
repaired. If the preferred fallback returns false, try another decoded live
source; if none activates successfully, clear the active source and audio state
and restart playback using the existing no-source reset path.
In `@src/file-ops.js`:
- Around line 193-199: Move the master metadata assignments for
S.masterAudioUrl, S.masterAudioDuration, and S.masterAudioName before the
installTrackSession() call so its immediate render uses the current song’s
values and cannot retain the previous song’s label.
In `@src/parts-view.js`:
- Around line 256-259: Update the header-row handling around
host.selectTrackSessionSource so the generic “Audio track” status is set before
activation, preserving any specific synchronous error status produced when the
source is unavailable. Keep the existing source selection behavior unchanged.
In `@src/track-session.js`:
- Around line 708-710: Update the delegated row handler around
host.selectTrackSessionSource so source activation occurs only for direct row or
name clicks, not when interacting with M/S, fader, or guide controls. Preserve
the existing control-handling behavior while preventing those controls from
switching the active waveform.
- Around line 417-428: Update the Parts-view source projection call to pass the
stable master URL expression S.masterAudioUrl || S.audioUrl instead of only
S.audioUrl, and provide the master display name using the same fallback order as
_liveSources: S.masterAudioName || S.title || 'Master Mix'. Preserve the
existing stems argument and projection behavior.
- Around line 912-927: Update the guide source selection logic around the
`guide-set` branch and `tempoGuideMode` assignment so reselecting the existing
`tempoGuideSourceId` preserves its metronome mode. Reset `next.tempoGuideMode`
to `'audio'` only when the selected source ID differs from the previous guide
source ID, while retaining the current source-selection behavior.
- Around line 536-543: Update trackSessionOrderedMixKeys() to derive mixer
ordering from the complete track hierarchy rather than _rowsLive().rows, which
excludes descendants of collapsed folders. Traverse all tracks regardless of
folder collapse state, retain only rows with mixKey, and preserve their
track-column order.
- Around line 464-468: When restoring the master in the initialization block
around S.activeAudioSourceId, also reset S.activeAudioSourceOffset to the
master’s default offset (zero) so no focused stem offset carries into waveform
and onset analysis.
In `@tests/stem_engine.test.mjs`:
- Around line 123-149: Update the test teardown for the repair and waveform
tests around syncStemAudio to restore every mutated audio-state field, including
duration, masterAudioDuration, audio URLs, active source identity and offset,
and related playback state. Save these fields before each test, call
resetStemAudioCache() during teardown, then restore the saved state so tests
remain order-independent.
---
Nitpick comments:
In `@tests/mixer_panel.test.mjs`:
- Around line 115-130: Update the ordering test around _mixerOrderedPartsPure to
include an audio:master fixture and assert it remains in the required prepended
position when orderedKeys includes it. Also add coverage for when audio:master
is absent from orderedKeys, verifying the implementation’s unlisted-key tail
behavior and preserving the expected relative order of the remaining strips.
🪄 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: 18fbcc0e-c127-4257-b104-d7969affafad
📒 Files selected for processing (17)
CHANGELOG.mdroutes.pysrc/arrangement.jssrc/audio.jssrc/file-ops.jssrc/host.jssrc/main.jssrc/mixer-panel.jssrc/parts-view.jssrc/shortcuts.jssrc/state.jssrc/track-session.jssrc/waveform.jstests/audition_clock.test.mjstests/mixer_panel.test.mjstests/stem_engine.test.mjstests/test_editor_stem_cache.py
byrongamatos
added a commit
that referenced
this pull request
Jul 16, 2026
…296) - audio.js: on active-source removal, try every live source as a fallback; reset to the no-source state only when none decode, so a failed fallback can't strand the removed buffer/id as active. - track-session.js: reset activeAudioSourceOffset on the create/import master restore; derive mixer order from the FULL tree (collapsed folders included) so collapse can't reshuffle strips; gate row-click source activation to direct row/name clicks (not M/S/fader/guide controls); preserve a guide's metronome mode when the same source is reselected; export _liveSources. - parts-view.js: project unified rows from the stable master URL/name via _liveSources (not S.audioUrl, which points at the focused stem); set the generic status before activation so a specific source error survives. - file-ops.js: assign master metadata (url/duration/name) before installTrackSession so its first render can't flash the previous song's label. - CHANGELOG.md: merge the duplicate ### Added heading; clarify master row vs master strip placement. - tests: regression tests for the failed-fallback reset, the create master offset reset, and collapse-preserving mix order (all fail pre-fix); restore every mutated audio-state field and the cache in the stem-engine teardowns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/track_session.test.mjs`:
- Around line 202-209: Update the test around installCreatedTrackSession to
isolate its mutations to S: snapshot the affected session, URL, stems,
arrangements, and active-source fields before setup, then restore them in a
finally block (or use the existing suite reset fixture) after the assertions.
- Around line 211-223: Update the test “collapsing a folder does not drop its
stems from the mixer order” to capture the result of
trackSessionOrderedMixKeys() before setting folder:1.collapsed, then assert the
keys after collapsing are deeply identical to that captured order. Replace the
membership-only includes assertion while preserving the existing setup and
cleanup.
🪄 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: 5de8af0c-93ba-49a4-b1c5-16e032dee9a6
📒 Files selected for processing (7)
CHANGELOG.mdsrc/audio.jssrc/file-ops.jssrc/parts-view.jssrc/track-session.jstests/stem_engine.test.mjstests/track_session.test.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/file-ops.js
- CHANGELOG.md
- src/track-session.js
- tests/stem_engine.test.mjs
- src/audio.js
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
byrongamatos
force-pushed
the
feat/editor-active-source
branch
from
July 16, 2026 18:43
21921eb to
f60be62
Compare
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.
The last parity gap from #285 — completes the re-cut.
What it does
Clicking an audio track in the Tracks column (the master mix or any stem) makes it the active source: its decoded buffer becomes the one the main waveform shows and the onset tools (Suggest, snapping) analyze — so you can line the grid up against an isolated stem. Playback is unaffected: the newly-active source plays via the reference path and every other live source keeps playing through the scheduler, so what you hear is unchanged; only what you see and analyze follows the click.
How (faithful behavior without #285's full unified scheduler)
My stem engine is additive (the master keeps its own
S.audioSourcepath), so I couldn't repurposeS.audioBufferas a display buffer the way #285 does. Instead I generalized it minimally:_liveAudioSources()now returns master + stems (master's URL held in the newS.masterAudioUrlso it survives while a stem is active andS.audioUrlpoints at the stem)._startStemSourcesschedules every live source except the active one (exposed as the pure_scheduledSourceIdsPure); the active source rides the existingS.audioSource → _refGainpath.activateTrackAudioSource(id)decodes on demand (reusing the source cache), installs the buffer as the reference, recomputes the waveform, lets onsets self-invalidate on the buffer swap, and restarts the play split if playing.syncStemAudioadopts the already-decoded active buffer for free instead of re-fetching.Net: master active (default) = the scheduler plays the stems, exactly as before. Focus a stem = it becomes the reference (waveform + onsets), the master joins the scheduler — everything still plays.
Wiring
host.selectTrackSessionSource → activateTrackAudioSource; the Tracks-column audio-row click focuses the source.resetStemAudioCache+ the load/create paths resetactiveAudioSourceIdtomasterand seedS.masterAudioUrl.Tests
stem_engine.test.mjspins the active-vs-scheduled split (_scheduledSourceIdsPure: master-active → stems only; stem-active → master + other stems; unknown active excludes nothing). 184 JS suites, 291 pytest, lint 0 errors.The re-cut is complete
With #294 (lock guard), #295 (mixer console + meters + +6 dB), and this, every #285 feature now exists on current main across #289–#296. #285 is fully superseded.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Summary by CodeRabbit
New Features
Bug Fixes
Tests