feat(editor): docked Mixer panel — per-part mute/solo/volume (workspace-shell B6) - #213
Merged
Conversation
…ce-shell B6)
Consolidate the floating audio-mixer popover and the orphaned stem-mixer
stub (dead markup wired to functions that never existed) into one
first-class docked Mixer panel beside the canvas, the inspector idiom:
one channel strip per part (volume / M / S) over the recording / guide /
click bus faders and the edit blip. All four entry points — View ▸
Panels ▸ Mixer, the toolbar Mix button, the transport util Mix button,
Shift+C — route through one toggle.
The strips own the canonical per-part mix state, S.partMix ('arr:<idx>' /
'drums' → { vol, mute, solo }) — the state the Parts-gutter M/S/A (§2.5)
and per-part instrument voices will read when they arrive. Today the only
per-part sound is the guide voice, so mute/solo/volume gate and scale the
claps for the part being edited, under the DAW rule (mute wins; any solo
isolates). Part solo never touches the recording: the reference is a bus
on its own transparent gain path (D5), and audio.js consults the state
only on the clap path, through an inert-default host.partClapState hook
(host.js stays the cycle-breaker; audio.js never imports the panel).
Bus levels keep their editorMix* prefs (write path stays in audio.js;
the panel seeds via host.mixUiState); panel open state is the
editorMixerPanel pref; part mute/solo is session state, reset where
create.js/file-ops.js install arrangements. Strip re-rendering is
memoized on (editGen, partMix, part list) off the updateStatus tail —
no per-frame work — and the delegated listeners are double-init-guarded.
tests/mixer_panel.test.mjs (15): strip-state model, audibility rules,
solo-keeps-reference, clap-state wiring, pref round-trips, memo, and
listener no-stack. Suite 108/108, lint 0 errors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
|
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 (8)
📝 WalkthroughWalkthroughReplaces floating mixer surfaces with a docked Mixer panel. Adds per-part volume/mute/solo state, host hooks for audio integration, guide-clap gating and scaling, updated controls and entry points, session resets, styling, changelog documentation, and automated tests. ChangesDocked Mixer Panel
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
![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
🧹 Nitpick comments (2)
src/mixer-panel.js (1)
118-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPer-part volume readout lacks
aria-live, unlike the bus faders.The bus fader value spans in screen.html (e.g.
editor-mix-ref-val) usearia-live="polite"so assistive tech announces level changes; the per-partdata-mix-valspan here doesn't, so screen-reader users won't hear volume updates while dragging a part's slider.- + `<span data-mix-val="${p.key}" class="w-9 text-right font-mono text-gray-400">${st.vol}%</span>` + + `<span data-mix-val="${p.key}" aria-live="polite" class="w-9 text-right font-mono text-gray-400">${st.vol}%</span>`🤖 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/mixer-panel.js` around lines 118 - 122, Update the per-part volume value span identified by data-mix-val in the mixer-panel template to include aria-live="polite", matching the bus fader readout behavior while preserving its existing value display and styling.tests/mixer_panel.test.mjs (1)
125-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the host-to-audio wiring.
These assertions cover the pure helper and the inert default
host.partClapState(), but not themain.jsassignment oraudio.jsconsumption. A broken hook, or accidental gating of the reference bus by part solo, would still pass. Add an integration assertion for that path, or point this test to existing coverage elsewhere.🤖 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 125 - 141, Extend the mixer panel tests to exercise the live host-to-audio wiring, not only _mixerClapStatePure and the default host.partClapState behavior. Verify that main.js assigns the hook consumed by audio.js on the CLAP path, and confirm part solo/mute does not gate the reference bus; reuse existing integration coverage if available, otherwise add a focused assertion for this path.
🤖 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 155-204: The volume input handler in _wire mutates S.partMix
without synchronizing _lastKey, causing _mixerPanelRefresh to re-render during
dragging. After applying the volume update and label patch, update _lastKey to
the current memo key or otherwise exclude live volume changes from the memo key,
while preserving refreshes for structural part changes.
---
Nitpick comments:
In `@src/mixer-panel.js`:
- Around line 118-122: Update the per-part volume value span identified by
data-mix-val in the mixer-panel template to include aria-live="polite", matching
the bus fader readout behavior while preserving its existing value display and
styling.
In `@tests/mixer_panel.test.mjs`:
- Around line 125-141: Extend the mixer panel tests to exercise the live
host-to-audio wiring, not only _mixerClapStatePure and the default
host.partClapState behavior. Verify that main.js assigns the hook consumed by
audio.js on the CLAP path, and confirm part solo/mute does not gate the
reference bus; reuse existing integration coverage if available, otherwise add a
focused assertion for this path.
🪄 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: a0865293-5dcc-4663-b97e-a6011289e97b
📒 Files selected for processing (14)
CHANGELOG.mdassets/v3-theme.cssscreen.htmlsrc/audio.jssrc/create.jssrc/file-ops.jssrc/host.jssrc/input.jssrc/main.jssrc/mixer-panel.jssrc/shortcuts.jssrc/state.jssrc/transport-bar.jstests/mixer_panel.test.mjs
Comment on lines
+155
to
+204
| function _setPart(key, patch) { | ||
| if (!S.partMix || typeof S.partMix !== 'object') S.partMix = {}; | ||
| S.partMix[key] = { ..._mixerPartStatePure(S.partMix, key), ...patch }; | ||
| } | ||
|
|
||
| // One delegated listener pair on the (static) panel element — guarded so a | ||
| // defensive double-init can never stack handlers. The panel element itself is | ||
| // replaced when the host re-injects the screen, so nothing leaks across | ||
| // re-injection either. | ||
| function _wire(panel) { | ||
| if (panel.__mixerWired) return; | ||
| panel.__mixerWired = true; | ||
| panel.addEventListener('click', (e) => { | ||
| const btn = e.target && e.target.closest ? e.target.closest('[data-mix-act="mute"],[data-mix-act="solo"]') : null; | ||
| if (!btn) return; | ||
| const key = btn.getAttribute('data-mix-part'); | ||
| const act = btn.getAttribute('data-mix-act'); | ||
| const st = _mixerPartStatePure(S.partMix, key); | ||
| _setPart(key, act === 'mute' ? { mute: !st.mute } : { solo: !st.solo }); | ||
| _lastKey = ''; | ||
| _mixerPanelRefresh(); | ||
| const now = _mixerPartStatePure(S.partMix, key); | ||
| setStatus(act === 'mute' | ||
| ? (now.mute ? 'Part muted — its guide voice is silent' : 'Part unmuted') | ||
| : (now.solo ? 'Part soloed — other parts’ guide voices are silent; the recording stays audible' : 'Solo off')); | ||
| }); | ||
| panel.addEventListener('input', (e) => { | ||
| const el = e.target; | ||
| if (!el || el.getAttribute('data-mix-act') !== 'vol') return; | ||
| const key = el.getAttribute('data-mix-part'); | ||
| _setPart(key, { vol: Number(el.value) }); | ||
| const val = panel.querySelector(`[data-mix-val="${key}"]`); | ||
| if (val) val.textContent = _mixerPartStatePure(S.partMix, key).vol + '%'; | ||
| }); | ||
| } | ||
|
|
||
| // Memoized refresh, called from updateStatus() beside the other companion | ||
| // strips: re-renders the part strips only when an edit (rename/add/delete) | ||
| // or the part list itself changed. No-op while the panel is hidden. | ||
| export function _mixerPanelRefresh() { | ||
| const panel = _panel(); | ||
| if (!panel || panel.classList.contains('hidden')) { _lastKey = ''; return; } | ||
| const container = document.getElementById('editor-mixer-parts'); | ||
| if (!container) return; | ||
| const parts = _mixerPartsPure(S.arrangements, S.drumTab); | ||
| const key = editGen + '|' + JSON.stringify(S.partMix) + '|' + parts.map(p => p.key + ':' + p.name).join(','); | ||
| if (key === _lastKey) return; | ||
| _lastKey = key; | ||
| _renderParts(container); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== mixer-panel outline ==\n'
ast-grep outline src/mixer-panel.js --view expanded || true
printf '\n== mixer-panel relevant lines ==\n'
nl -ba src/mixer-panel.js | sed -n '1,260p'
printf '\n== main.js relevant lines ==\n'
nl -ba src/main.js | sed -n '1,220p'Repository: got-feedBack/feedBack-plugin-editor
Length of output: 1166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== mixer-panel lines 100-240 ==\n'
sed -n '100,240p' src/mixer-panel.js | cat -n
printf '\n== main.js search for updateStatus and mixer refresh ==\n'
rg -n "updateStatus|_mixerPanelRefresh|mixerPanelRefresh|editorToggleMixerPanel" src/main.js src -g '!**/node_modules/**' || true
printf '\n== main.js lines around updateStatus ==\n'
awk '
/function updateStatus|export function updateStatus/ {start=NR-20}
start && NR<=start+80 {print NR ":" $0}
' src/main.jsRepository: got-feedBack/feedBack-plugin-editor
Length of output: 14498
Keep the mixer refresh memo in sync with volume writes. _mixerPanelRefresh() keys off JSON.stringify(S.partMix), but the volume input handler only mutates S.partMix and patches the label. The next updateStatus() call will see a new key and re-render the panel, replacing the range input while it’s being dragged. Update _lastKey after local volume changes, or exclude live volume edits from the memo key.
🤖 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/mixer-panel.js` around lines 155 - 204, The volume input handler in _wire
mutates S.partMix without synchronizing _lastKey, causing _mixerPanelRefresh to
re-render during dragging. After applying the volume update and label patch,
update _lastKey to the current memo key or otherwise exclude live volume changes
from the memo key, while preserving refreshes for structural part changes.
This was referenced Jul 12, 2026
The mixer's audible gate lived inside _guideSourceTimes(), whose second consumer is _composeSongDuration(). Muting the edited part (or soloing another) made _guideSourceTimes() return [], collapsing contentEnd to 0 — so S.duration truncated to the grid end, or fell to 0 on a sparse grid and startPlayback bailed. Mix state must never move transport bounds. Move the gate to the clap scheduler (_guideTick): the audibility check now ANDs into the `if (claps ...)` guard, leaving _guideSourceTimes as the raw event set the duration reads. Per-voice volume scaling in _guideClapVoiceAt is unchanged. Adds two source-level regression guards to compose_transport.test.mjs (the gate must not sit in _guideSourceTimes; it must sit in _guideTick). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # src/main.js
This was referenced Jul 12, 2026
feat(editor): multi-track MIDI playback — the chart plays as a band, mixed by the Tracks strips
#280
Merged
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
Phase B6 of the workspace-shell charrette (§2.6): the mixer becomes a first-class docked panel beside the canvas (the inspector idiom), consolidating the floating audio-mixer popover and the orphaned stem-mixer stub into one surface:
View ▸ Panels ▸ Mixer, the toolbarMixbutton, the transport's left utilMixbutton, andShift+C(the existingtoggleMixercommand id, so shortcut profiles are untouched).S.partMixis the canonical per-part mix state ('arr:<idx>'/'drums'→{vol, mute, solo}) — the state the Parts-gutter M/S/A (§2.5) and per-part instrument voices (GM slice) will read when they arrive. Today the only per-part sound is the guide voice, so mute/solo/volume gate and scale the claps for the part being edited, under the DAW rule: mute wins, any solo isolates.host.partClapStatehook (host.js stays the cycle-breaker — audio.js never imports the panel, the panel never imports audio.js).editorMix*prefs (write path stays insrc/audio.js; the panel seeds via ahost.mixUiStatehook); panel open state is a neweditorMixerPaneleditor pref. Part mute/solo is session state and resets wherecreate.js/file-ops.jsinstall arrangements — never the pack.updateStatustail — no per-frame work; the delegated listeners are double-init-guarded (re-inject safe).Premise corrections vs the charrette text
Recon found the charrette's two "existing pieces" half-true on current main:
#editor-stem-mixerwas dead markup wired toeditorToggleStemMixer()/_renderStemMixer()which exist nowhere in the repo (there is also no multi-stem audio graph — playback is one buffer), and the Parts-gutter M/S/A is unbuilt. So this slice removes the dead stub outright and makes the panel the source of the per-part state rather than a mirror of a gutter that doesn't exist yet — the gutter slice mirrors it later.Testing
tests/mixer_panel.test.mjs(new, 15 tests): pure strip-state model (defaults/clamps), mute-wins/solo-isolates audibility, clap-state wiring (drums vs current arrangement, volume scaling), the D5 pin (part gate only; host default leaves audio untouched), open-state pref round-trip, bus-fader seeding, memoized render, and listener no-stack across double init.routes.pychanges.🤖 Generated with Claude Code
https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Summary by CodeRabbit