feat(editor): stem mixer — isolate any instrument while editing - #275
Closed
Sabissimo wants to merge 4 commits into
Closed
feat(editor): stem mixer — isolate any instrument while editing#275Sabissimo wants to merge 4 commits into
Sabissimo wants to merge 4 commits into
Conversation
…nstrument while editing The /load payload has advertised per-instrument stem URLs (result["stems"]) since the backend landed them "so the editor's stem mixer can load and balance them live" — the frontend just never consumed them. Now it does: - Engine (src/audio.js): a @pure:stem-mixer block (DAW rule as per-stem gains, unity detection, source-path predicate) plus a lazy stem path — stems fetch+decode in parallel on the first strip gesture (generation- guarded like cancelAudioLoad), then play as N sample-synced BufferSources through per-stem GainNodes summing into _refGain, so the Recording fader, A/B guide-pass mute and first-play fade govern the stem mix unchanged. While every strip sits at unity the shipped mixdown keeps playing, bit-identical to before; gain moves ramp live, only unity-boundary crossings restart via _restartPlaybackAt. Audition < 100% bypasses stems (single-media-element slow path). All source stop/create paths route through _stopAllAudioSources. - Panel (src/mixer-panel.js + screen.html): a Stems section above Tracks, one strip per stem (volume / M / S) in the parts-strip idiom on its own data-stem-* attributes and its own session map S.stemMix (reset on song load like S.partMix); loading / unavailable / slow-bypass notes from the engine. Panel ↔ engine cross via host.stemMixChanged / stemUiState / stemUiChanged — audio.js still never imports the panel. - Stem-less packs (< 2 stems) hide the section and behave byte-identically. Tests: tests/stem_mixer.test.js (gain math, unity boundary, path predicate), mixer_panel.test.mjs stem strips/wiring/memo, audition_clock stubs extended for the new engine names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STL2MctqAyEw5J5Myz3rPr
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe editor adds per-instrument stem mixer controls with lazy decoding, mute/solo/volume processing, synchronized playback, slow-audition bypass, lifecycle resets, host wiring, tests, and documentation. ChangesStem mixer
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MixerPanel
participant HostHooks
participant AudioEngine
participant StemSources
MixerPanel->>HostHooks: update stem volume, mute, or solo
HostHooks->>AudioEngine: stemMixChanged()
AudioEngine->>StemSources: lazily decode and start synchronized stems
StemSources->>AudioEngine: route stem gains into reference output
AudioEngine->>MixerPanel: stemUiChanged()
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The M/S pressed-state colors (mute amber, solo sky) were keyed on data-mix-act only, so the Stems section's buttons — which use the disjoint data-stem-act attribute set — never lit up: stem mute/solo worked audibly but showed no visual state. Extend both aria-pressed rules to cover data-stem-act. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/stem_mixer.test.js (1)
15-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse real ESM imports instead of extracting and evaluating source.
This new suite bypasses the module-loading contract and depends on
@purecomments and source layout. Rename it to.test.mjsand import the exported helpers directly, consistent with the repository’s documented convention for new tests.🤖 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/stem_mixer.test.js` around lines 15 - 36, The test suite currently extracts and evaluates helpers from source text instead of exercising the module-loading contract. Rename the test file to use the .test.mjs convention, replace the fs/path extraction and Function-based setup around extract with real ESM imports of the exported stem helper functions, and update references to use those imported symbols directly.
🤖 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 `@src/mixer-panel.js`:
- Around line 141-143: Update _stemMsBtn and the related stem-row rendering
paths to HTML-escape every interpolated attribute value, including stem IDs,
actions, pressed state, and titles, before building markup. Replace any
percentage-element querySelector that constructs a selector from p.key with a
lookup scoped to the rendered row, so IDs containing selector metacharacters are
handled safely. Apply the same changes to the additional stem-button and
percentage-rendering paths noted in the review.
- Around line 98-106: Update _mixerStemsPure so it filters invalid stems before
applying the minimum-length check. Return an empty list when fewer than two
valid entries remain, then map the filtered stems into the existing key and name
objects.
In `@tests/mixer_panel.test.mjs`:
- Around line 246-258: Update the _mixerStemsPure tests to cover inputs
containing two raw entries but only one valid stem after malformed entries are
filtered, including [{ id: 'guitar' }, null]. Assert that the result is [] to
preserve the “fewer than two valid stems is not a mixer” invariant.
---
Nitpick comments:
In `@tests/stem_mixer.test.js`:
- Around line 15-36: The test suite currently extracts and evaluates helpers
from source text instead of exercising the module-loading contract. Rename the
test file to use the .test.mjs convention, replace the fs/path extraction and
Function-based setup around extract with real ESM imports of the exported stem
helper functions, and update references to use those imported symbols directly.
🪄 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: 621d7c14-d084-4e82-bca6-ce0fcd4d3d08
📒 Files selected for processing (15)
CHANGELOG.mdREADME.mdassets/v3-theme.cssscreen.htmlsrc/audio.jssrc/create.jssrc/file-ops.jssrc/host.jssrc/main.jssrc/mixer-panel.jssrc/replace-audio.jssrc/state.jstests/audition_clock.test.mjstests/mixer_panel.test.mjstests/stem_mixer.test.js
This was referenced Jul 15, 2026
feat(editor): multi-track MIDI playback — the chart plays as a band, mixed by the Tracks strips
#280
Merged
…ring CodeRabbit review on got-feedBack#275: - _mixerStemsPure now filters invalid entries BEFORE the <2 gate, so a two-entry payload with one junk stem can't render a lone unusable strip (+ regression case in tests/mixer_panel.test.mjs). - Stem ids are /load payload data — escape them in every attribute they reach (data-stem-row, data-stem-part), and find the volume readout by row-scoped [data-stem-val] lookup instead of building a selector from the id (metacharacters would throw; markup can't inject). - tests/stem_mixer.test.js → .test.mjs: the five stem pures are exported from src/audio.js and imported for real, retiring the @pure:stem-mixer slice-and-eval harness per the repo's test convention. - JSDoc on the new stem functions in src/audio.js and src/mixer-panel.js (docstring-coverage pre-merge check). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/audio.js (1)
1304-1332: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse null-prototype stem-state maps.
S.stemMixis still initialized with{}in the song-reset paths, and_stemGainsPurealso allocates{}. A stem id like__proto__can hit the special accessor instead of a normal key, so stem lookups and writes should useObject.create(null).🤖 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 `@src/audio.js` around lines 1304 - 1332, Use null-prototype objects for all stem-state maps: change the song-reset initializations of S.stemMix and the gains accumulator created in _stemGainsPure from {} to Object.create(null). Preserve the existing stem lookup and gain behavior while ensuring ids such as __proto__ are treated as ordinary keys.
🤖 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.
Outside diff comments:
In `@src/audio.js`:
- Around line 1304-1332: Use null-prototype objects for all stem-state maps:
change the song-reset initializations of S.stemMix and the gains accumulator
created in _stemGainsPure from {} to Object.create(null). Preserve the existing
stem lookup and gain behavior while ensuring ids such as __proto__ are treated
as ordinary keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aecd892b-de03-42f4-96f0-e4477e4a736f
📒 Files selected for processing (4)
src/audio.jssrc/mixer-panel.jstests/mixer_panel.test.mjstests/stem_mixer.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/mixer_panel.test.mjs
- src/mixer-panel.js
…oto__ are ordinary keys Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
byrongamatos
pushed a commit
that referenced
this pull request
Jul 15, 2026
…air studio tracks with chart tracks Christian's ask: more than demucs's five — take N tracks from an actual studio session, ingest, rename, rearrange, and transcribe/chart against each as the user wishes. Backend (routes.py): - POST /import-stems (multipart, any count): filename-derived ids (_stem_safe_id: sanitized, deduped, 60-char cap), files into the sloppak session's stems/ + manifest stems list (zip-form persists on Save — the replace-audio rule) or, for create-mode GP/MIDI imports, into the session's stem_files which Build packs into the new pak (staging/stems/ + manifest entries in _write_sloppak_pak). - POST /stem-op rename|reorder|delete: pure list ops (_stems_rename_pure refuses collisions; _stems_reorder_pure demands a full permutation — partial orders get no authority to drop entries; _stems_manifest_list preserves unknown entry fields so another tool's stem metadata survives our edits); renames MOVE the file and follow the links; deletes drop their links. - editor_stem_links manifest extension key (chart-track key → stem id, the audio_shift absent/empty/garbage contract) at all three save paths + /load; /load's stems gate relaxed >=2 → >=1 (a lone imported track must reach the manager; the mixer strips keep their own gate). Frontend (src/stem-tracks.js + modal): - File ▸ Audio tracks…: import (multi-file), rename (click the name), reorder (▴▾ — the manifest is order-authoritative), delete, and a per-track pairing select over the chart tracks. Chart keys use the _partViewKeyPure id-or-name rule — never a bare index, so links survive part reordering. - 'Solo my source track' (registry + Transport menu): solos the current track's paired stem via S.stemMix — the transcription move; audible once the stem-mixer strips (#275) land, honest status either way. tests/stem_tracks.test.mjs (4, fails on main): row model + key rule, one-stem-per-track pairing, solo verb exactness, persistence wire. tests/test_stem_tracks.py (5): id sanitize/dedupe, rename/reorder pure hard-edges, unknown-field preservation, the links contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
byrongamatos
added a commit
that referenced
this pull request
Jul 15, 2026
…air studio tracks with chart tracks (#283) * feat(editor): multitrack stem ingest — import, rename, reorder, and pair studio tracks with chart tracks Christian's ask: more than demucs's five — take N tracks from an actual studio session, ingest, rename, rearrange, and transcribe/chart against each as the user wishes. Backend (routes.py): - POST /import-stems (multipart, any count): filename-derived ids (_stem_safe_id: sanitized, deduped, 60-char cap), files into the sloppak session's stems/ + manifest stems list (zip-form persists on Save — the replace-audio rule) or, for create-mode GP/MIDI imports, into the session's stem_files which Build packs into the new pak (staging/stems/ + manifest entries in _write_sloppak_pak). - POST /stem-op rename|reorder|delete: pure list ops (_stems_rename_pure refuses collisions; _stems_reorder_pure demands a full permutation — partial orders get no authority to drop entries; _stems_manifest_list preserves unknown entry fields so another tool's stem metadata survives our edits); renames MOVE the file and follow the links; deletes drop their links. - editor_stem_links manifest extension key (chart-track key → stem id, the audio_shift absent/empty/garbage contract) at all three save paths + /load; /load's stems gate relaxed >=2 → >=1 (a lone imported track must reach the manager; the mixer strips keep their own gate). Frontend (src/stem-tracks.js + modal): - File ▸ Audio tracks…: import (multi-file), rename (click the name), reorder (▴▾ — the manifest is order-authoritative), delete, and a per-track pairing select over the chart tracks. Chart keys use the _partViewKeyPure id-or-name rule — never a bare index, so links survive part reordering. - 'Solo my source track' (registry + Transport menu): solos the current track's paired stem via S.stemMix — the transcription move; audible once the stem-mixer strips (#275) land, honest status either way. tests/stem_tracks.test.mjs (4, fails on main): row model + key rule, one-stem-per-track pairing, solo verb exactness, persistence wire. tests/test_stem_tracks.py (5): id sanitize/dedupe, rename/reorder pure hard-edges, unknown-field preservation, the links contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * fix(editor): atomic stem-link submission, session dirty marks, solo-source gating + isolation (review #283 items 15-18) Item 15: every stem op (pairing sync via new op 'links', rename, reorder, delete, import) ships the CURRENT S.stemLinks, the backend session seeds from the manifest on load and honors the submitted snapshot, and pairings ride the same manifest write as the op — so the authoritative response can never resurrect stale links over an unsaved pairing. Failed POSTs half-apply nothing. Item 16: stem responses carry a persisted verdict (dir-form sloppak = durable now; zip/create = Save/Build) and _adopt marks the session dirty on non-durable changes so the lifecycle guard can't silently discard them. Create-mode Build now ships stem_links too (the third save path). Item 17: editorSoloMyStem is gated on a real host.stemMixChanged consumer (stemMixerAvailable); with none wired it reports unavailable instead of flipping state nothing reads, and the menu greys it via needs:'stemMixer'. Item 18: solo-my-source is an exclusive isolate — enabling clears every other stem's solo, toggle-off restores the no-solo state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * fix(editor): stem reorder rejected 400 when manifest carries a `full` mix The stem manager hides the `full` combined-mix entry (both /load and _stem_state_payload skip id=="full"), so S.stems — and the `order` the UI sends on a reorder — never contains it. But the stem-op reorder validated that order against the WHOLE manifest stems list via _stems_reorder_pure, which still includes `full`, so `sorted(order) != sorted(ids)` and every reorder on a real (stem-separated / replace-audio'd) sloppak returned 400 "order must be a full permutation". Root cause: the permutation was checked against the full manifest list instead of the surfaced subset. Fix: _reorder_with_hidden reorders only the entries the manager surfaced (the ids `order` names), keeps hidden entries (`full`, plus any payload-dropped missing-file/path-escape stems) at the front, and still fails closed on a foreign or duplicate id. Wired into both the sloppak and create-mode reorder branches. Regression: test_reorder_with_hidden_keeps_full_and_reorders_managed. 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 16, 2026
Collaborator
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.
What
Adds a Stems section to the editor's docked Mixer panel (Shift+C): one strip per audio stem (guitar / bass / drums / vocals / …) with volume / mute / solo, so you can isolate any instrument's audio while editing — e.g. solo the guitar to hear exactly the note you're tuning.
The backend has been serving per-instrument stem URLs from
/load(result["stems"], gated on ≥2 stems) with a comment saying it exists "so the editor's stem mixer can load and balance them live" — this PR is the frontend half that finally consumes it. No backend changes.How
src/audio.js): new@pure:stem-mixerblock (the DAW rule — mute wins, any solo isolates — folded into per-stem gains; unity detection; the source-path predicate) plus a lazy stem playback path:cancelAudioLoad), so stem-less sessions and untouched mixers pay nothing.BufferSources (identical(when, offset)on the same context clock) → per-stemGainNode→ one stem bus →_refGain, so the Recording fader, A/B guide-pass mute, first-play hearing-safety fade, count-in and loop wraps all govern the stem mix exactly as they governed the single recording.setTargetAtTime, ~20 ms); only unity-boundary crossings restart via the existing_restartPlaybackAtidiom. Every source stop/create path now routes through_stopAllAudioSources.src/mixer-panel.js,screen.html): Stems section above Tracks in the parts-strip idiom, on its owndata-stem-*attributes (a stem id likedrumsis also a valid part key) and its own session mapS.stemMix, reset on song load likeS.partMix. Loading / unavailable / slow-bypass notes come from the engine.src/host.js,src/main.js):host.stemMixChanged/stemUiState/stemUiChanged—audio.jsstill never imports the panel module.teardownAudiodrops the graph.Tests
tests/stem_mixer.test.js— gain math, mute-beats-solo, stale-solo inertness, the unity boundary, the path predicate (10 cases).tests/mixer_panel.test.mjs— stem strip rendering/visibility, click/input wiring + engine pokes, memo invalidation on mix and engine-state changes (7 new cases).tests/audition_clock.test.mjs— stub list extended for the new engine names.npm test: 158/159 pass — the one failure istests/song_fit.test.mjs, which also fails on a clean checkout of main (pre-existing).pytest: 254 passed.npm run lint: 0 errors (3 pre-existing warnings).Notes for reviewers
S.audioSourceis intentionallynull; the MIDI-recordonendedend-of-song fallback (guarded byif (S.audioSource)) is skipped on that path and the rAF EOF clamp still finalizes — only a backgrounded-tab punch-in while stem-mixing loses the fallback.S.partMix): a persisted solo silently isolating one instrument next session would read as a playback bug.🤖 Generated with Claude Code
https://claude.ai/code/session_01STL2MctqAyEw5J5Myz3rPr
Summary by CodeRabbit