Skip to content

feat(editor): a song can hold several drum parts - #339

Merged
byrongamatos merged 1 commit into
mainfrom
mt-drums-p3
Jul 21, 2026
Merged

feat(editor): a song can hold several drum parts#339
byrongamatos merged 1 commit into
mainfrom
mt-drums-p3

Conversation

@ChrisBeWithYou

@ChrisBeWithYou ChrisBeWithYou commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

PR3 of the drums-as-arrangement arc — stacked on #338 (base mt-drums-p2b) → #337 → #336 → #335. This is the multiple-drum-charts payoff (a second drummer, an aux-percussion layer, a programmed layer under an acoustic kit).

Model

Each type:"drums" arrangement now owns its .drumTab; S.drumTab becomes a pointer to the ACTIVE part's tab (the one the drum grid edits) — so all ~163 S.drumTab readers and every drum undo command keep meaning "the tab being edited", unchanged. Primary = the first drums arrangement: its tab persists as the song-level drum_tab (the back-compat alias current cores play). Extras persist as type: drums manifest arrangement entries with per-arrangement drum_tab pointers (feedpak-spec 1.17.0, FEP #63) — file-less entries the current core loader verifiably skips (lib/sloppak's per-entry file/notation gate), so built packs stay fully compatible: an old reader sees one drum; the editor reads them all back. The #337 invariant holds throughout: S.currentArr never lands on a drums arrangement.

What you can do now

  • ＋ Track ▸ Drums with drums present adds another part; a GP/MIDI drum import is added instead of replacing (create mode keeps one-part replace — its build persists a single drum_tab).
  • One 🥁 option per part in the switcher; picking one opens its grid (the dropdown shows the active part).
  • Each part gets its own Tracks row, mixer strip, and band-mode channel (part-scoped dedupe: two parts hitting the same piece on the same millisecond both sound). Parts-view lanes render per-part.
  • Delete is per-part (generalized DeleteDrumTabCmd: full partMix snapshot, active-part handover, primary promotion, exact undo round-trip); rename writes the part's own tab name — fixing a latent wrong-tab rename when the renamed row wasn't the part open in the grid.
  • Save/reload round-trips every part.

Backend (routes.py)

drum_parts wire field (validated; empty list = removal; always beside drum_tab); drum pointer entries are split out of the pitched manifest pairing (a file-less entry must never pair with a wire arrangement) and the load-side id list mirrors the core loader's skip so ids can't misalign on a multi-drum pack; writes drum_tab_<id>.json per extra + reconciles the type: drums entries (primary alias emitted per the spec example); preserves other writers' entries when the field is absent; drops orphaned side files; reads extras back on load (path-guarded, schema-validated, alias-deduped).

Verification

  • JS 297 files green — new coverage: active-part model, add/adopt ids, multi-part sync safety, per-part delete round-trip (partMix map, mode, promotion), the primary-not-active save-wire pin (saving while editing a secondary ships the PRIMARY as drum_tab), per-part clap gating, multi-part band roster. pytest 364 green (+4: pointer-entry split, extra-tab sanitation). Lint 0 err / 3 baseline.
  • Runtime-verified end-to-end, 14/14 (Playwright, asdf_asdf.feedpak): dialog adds "Drums 2" ("this will add another Drums track", Create enabled) → switcher [Keys, 🥁 Drums, 🥁 Drums 2] → grids flip per-part → mixer strips arr:1+arr:2 → save writes drum_tab_drums-2.json + both type: drums manifest entries + keeps the song-level key → reload restores both parts with their strips; no console errors.

Docs: CHANGELOG + USER-GUIDE §7. Next in the arc: the core loader learns type:drums (core repo, spec #63), then region R3b ("import a drums-only GP into an existing project").

⚠️ Stacked PR: base is mt-drums-p2b (#338). CI is main-only until the stack retargets.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Warning

Review limit reached

@byrongamatos, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dce9ee23-767b-4406-a2a6-5319a6bceeaa

📥 Commits

Reviewing files that changed from the base of the PR and between 1e5c56e and f61eb10.

📒 Files selected for processing (22)
  • CHANGELOG.md
  • docs/USER-GUIDE.md
  • routes.py
  • src/arrangement.js
  • src/audio.js
  • src/create.js
  • src/drum-arrangement.js
  • src/file-ops.js
  • src/main.js
  • src/mixer-panel.js
  • src/new-track.js
  • src/parts-view.js
  • src/tempo.js
  • src/track-session.js
  • tests/drum_arrangement.test.mjs
  • tests/drum_delete_undo.test.mjs
  • tests/drum_primary_id_roundtrip.test.mjs
  • tests/midi_playback.test.mjs
  • tests/mixer_panel.test.mjs
  • tests/new_track.test.mjs
  • tests/test_drum_parts.py
  • tests/view_switcher.test.mjs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mt-drums-p3

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This was referenced Jul 20, 2026
Merged
Closed
@byrongamatos
byrongamatos changed the base branch from mt-drums-p2b to main July 21, 2026 11:35
@byrongamatos @claude
feat(editor): a song can hold several drum parts (#339)
f61eb10 
Includes review fix: persist the primary drum part's id so a promoted primary
survives reload (drum_tab_id round-trip).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@byrongamatos
byrongamatos merged commit 0286707 into main Jul 21, 2026
3 of 4 checks passed
@byrongamatos
byrongamatos deleted the mt-drums-p3 branch July 21, 2026 11:40
@ChrisBeWithYou ChrisBeWithYou mentioned this pull request Jul 21, 2026
Merged
ChrisBeWithYou pushed a commit that referenced this pull request Jul 21, 2026
ChrisBeWithYou @claude
fix(editor): region commands act on the drum part the track names, no…
7f42183 
…t the active tab

A song can hold several drum parts now (#339): each type:"drums"
arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid
target. The region commands and the parts-view drag arming predate that
(#334) — kind:'drums' always operated on S.drumTab, and a materialized
extra part ('drums-2'…) armed as 'notation' over its empty shell
notes[], so dragging its block moved nothing (or, for the primary while
another part was active, moved the WRONG part's hits).

Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part
whose track carries the region → its own .drumTab; arrIdx < 0 stays the
legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared
_contentList, and parts-view arms kind:'drums' for any drum-arrangement
row. Tests drive a NON-active part and pin that the active tab never
moves, plus the legacy no-arrIdx fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
byrongamatos pushed a commit that referenced this pull request Jul 21, 2026
ChrisBeWithYou @claude @byrongamatos
feat(editor): create-mode songs can hold several drum parts too
65c2344 
PR4 of the drums-as-arrangement arc — closes the last authoring gap. #339
gave N drums to EDIT-mode (re-open + Save via /save_song); a CREATE-mode
session (New Song from a GP/MIDI import) still capped at one drum chart,
because its build persists through /build → _write_sloppak_pak, which only
knew a single song-level drum_tab. Now the create-mode build path reaches
parity: multiple drum parts persist per feedpak-spec 1.17.0 (primary as the
song-level drum_tab alias, extras as type:"drums" arrangement entries with
per-arrangement drum_tab pointers — old readers see one drum, new readers all).

Backend (routes.py):
- New module-level `_create_build_drum_entries(staging, drum_tab, drum_parts)`:
  writes each extra part's drum_tab_<id>.json into the staging dir and returns
  the type:"drums" manifest entries (primary alias + extras; ids sanitized to a
  safe filename charset and de-collided). Twin of /save_song's inline drum-parts
  block, sharing `_sanitize_extra_drum_tab` for one on-disk shape. Extracted to
  module scope (out of _write_sloppak_pak's closure) and kept PURE of the core
  `lib` — so pytest drives the real file write directly.
- `_write_sloppak_pak` gains a `drum_parts` param; appends the entries after the
  pitched arrangements when the client opted in (a list, possibly empty). Absent
  → an old frontend → the pack stays byte-identical with just the song-level key.
- `build_song_endpoint` reads + boundary-validates `drum_parts` (each part a
  dict with a schema-valid drum_tab, via lib.drums like /save_song — fail-fast
  400) and threads it into the build. The other _write_sloppak_pak callers
  (save-as-sloppak) pass nothing → unaffected.

Frontend:
- editorBuild ships the drum payload via new pure `_drumBuildPayloadPure`
  (create.js): the PRIMARY drum_tab is the FIRST drums arrangement's payload,
  NOT the active S.drumTab (which tracks whichever part is open in the grid —
  the user may be editing a secondary at build time); `drum_parts` (the extras)
  ships only when drums are materialized as arrangements, so a legacy single-tab
  compose build stays byte-identical.
- `_canAddAnotherDrums` drops the `!S.createMode` guard — create sessions add a
  2nd part beside a pitched track now that the build persists it. The only
  remaining one-part case is a drums-only session (no melodic track to sit
  beside — drums are never index 0); the New-Track note + refusal message say so.

Tests: JS 297 files green — `_drumBuildPayloadPure` (primary-not-active,
empty-extras-still-ships, byte-identical fallback) + a REAL editorBuild→/build
body integration test through saveCDLC (asserts the primary + extras on the
wire); new_track create-mode add-a-2nd-part + the drums-only one-part guard.
pytest 369 green (+5): `_create_build_drum_entries` real file write — side
files + alias, empty extras, hit sanitation + name propagation, id de-collision
+ filename sanitization, non-list-hits sanitized-not-crashing. Lint 0 err / 3
baseline.

Runtime posture: the create-mode /build write is now directly unit-tested
against real file I/O (the extracted helper), the frontend build wire is
integration-tested against the real editorBuild, and the on-disk shape is
identical to /save_song's drum-parts persistence that #339 runtime-verified
end-to-end (create parts → save → reload → both parts return). Stacked on #339.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>
byrongamatos added a commit that referenced this pull request Jul 21, 2026
@byrongamatos @claude
feat(editor): create-mode songs can hold several drum parts too (re-l…
4e8fbfe 
…and of #340)

Includes review fix: round-trip the primary drum part's id through the create/
build path (drum_tab_id → _primary_drum_alias_id), mirroring the #339 save-path
fix, so a promoted primary keeps its stem links + tree placement on reload.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos added a commit that referenced this pull request Jul 21, 2026
@ChrisBeWithYou ChrisBeWithYou
@claude @byrongamatos
feat(editor): create-mode songs can hold several drum parts too (re-l…
f976f91 
…and of #340) (#343)

* feat(editor): create-mode songs can hold several drum parts too

PR4 of the drums-as-arrangement arc — closes the last authoring gap. #339
gave N drums to EDIT-mode (re-open + Save via /save_song); a CREATE-mode
session (New Song from a GP/MIDI import) still capped at one drum chart,
because its build persists through /build → _write_sloppak_pak, which only
knew a single song-level drum_tab. Now the create-mode build path reaches
parity: multiple drum parts persist per feedpak-spec 1.17.0 (primary as the
song-level drum_tab alias, extras as type:"drums" arrangement entries with
per-arrangement drum_tab pointers — old readers see one drum, new readers all).

Backend (routes.py):
- New module-level `_create_build_drum_entries(staging, drum_tab, drum_parts)`:
  writes each extra part's drum_tab_<id>.json into the staging dir and returns
  the type:"drums" manifest entries (primary alias + extras; ids sanitized to a
  safe filename charset and de-collided). Twin of /save_song's inline drum-parts
  block, sharing `_sanitize_extra_drum_tab` for one on-disk shape. Extracted to
  module scope (out of _write_sloppak_pak's closure) and kept PURE of the core
  `lib` — so pytest drives the real file write directly.
- `_write_sloppak_pak` gains a `drum_parts` param; appends the entries after the
  pitched arrangements when the client opted in (a list, possibly empty). Absent
  → an old frontend → the pack stays byte-identical with just the song-level key.
- `build_song_endpoint` reads + boundary-validates `drum_parts` (each part a
  dict with a schema-valid drum_tab, via lib.drums like /save_song — fail-fast
  400) and threads it into the build. The other _write_sloppak_pak callers
  (save-as-sloppak) pass nothing → unaffected.

Frontend:
- editorBuild ships the drum payload via new pure `_drumBuildPayloadPure`
  (create.js): the PRIMARY drum_tab is the FIRST drums arrangement's payload,
  NOT the active S.drumTab (which tracks whichever part is open in the grid —
  the user may be editing a secondary at build time); `drum_parts` (the extras)
  ships only when drums are materialized as arrangements, so a legacy single-tab
  compose build stays byte-identical.
- `_canAddAnotherDrums` drops the `!S.createMode` guard — create sessions add a
  2nd part beside a pitched track now that the build persists it. The only
  remaining one-part case is a drums-only session (no melodic track to sit
  beside — drums are never index 0); the New-Track note + refusal message say so.

Tests: JS 297 files green — `_drumBuildPayloadPure` (primary-not-active,
empty-extras-still-ships, byte-identical fallback) + a REAL editorBuild→/build
body integration test through saveCDLC (asserts the primary + extras on the
wire); new_track create-mode add-a-2nd-part + the drums-only one-part guard.
pytest 369 green (+5): `_create_build_drum_entries` real file write — side
files + alias, empty extras, hit sanitation + name propagation, id de-collision
+ filename sanitization, non-list-hits sanitized-not-crashing. Lint 0 err / 3
baseline.

Runtime posture: the create-mode /build write is now directly unit-tested
against real file I/O (the extracted helper), the frontend build wire is
integration-tested against the real editorBuild, and the on-disk shape is
identical to /save_song's drum-parts persistence that #339 runtime-verified
end-to-end (create parts → save → reload → both parts return). Stacked on #339.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>

* feat(editor): create-mode songs can hold several drum parts too (re-land of #340)

Includes review fix: round-trip the primary drum part's id through the create/
build path (drum_tab_id → _primary_drum_alias_id), mirroring the #339 save-path
fix, so a promoted primary keeps its stem links + tree placement on reload.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
@ChrisBeWithYou ChrisBeWithYou mentioned this pull request Jul 21, 2026
Merged
ChrisBeWithYou added a commit that referenced this pull request Jul 22, 2026
@ChrisBeWithYou ChrisBeWithYou @claude
feat(editor): track regions — import a part into an existing project …
a3c219f 
…as a placed, movable region (#344)

* feat(editor): track regions — place + delete commands (the import-region foundation)

PlaceRegionCmd drops freshly-added track content onto the timeline as a
BOUNDED, selected, draggable block: it slides the content so its first
onset lands on the snapped startBeat (a musical move — beats preserved on
a varying grid, mirroring MoveRegionCmd's remap with the constant-tempo
fast path) and writes an explicit-length region covering it. Even a
bar-1 placement stays bounded — never the implicit default — so the
block is a distinct addressable object from birth. DeleteRegionCmd
removes exactly the notes/hits a region's end-exclusive window owns plus
the region entry, leaving neighbours untouched.

Both round-trip: rollback restores a verbatim content snapshot (the
beatOf∘timeOf round trip is not bit-reversible), the track's raw
`regions` key (including deleting one that was never there), and the
prior selection. Node-runnable with no DOM, like the move command.

_nextRegionIdPure allocates the next free region:N, counting the
implicit default region:1 even when unpersisted so a placed region can
never collide with it.

This is the command layer for "Add Track from File → placed region"
(TRACK-REGIONS-DESIGN.md PR 3); the import front door wires in next.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* fix(editor): region commands act on the drum part the track names, not the active tab

A song can hold several drum parts now (#339): each type:"drums"
arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid
target. The region commands and the parts-view drag arming predate that
(#334) — kind:'drums' always operated on S.drumTab, and a materialized
extra part ('drums-2'…) armed as 'notation' over its empty shell
notes[], so dragging its block moved nothing (or, for the primary while
another part was active, moved the WRONG part's hits).

Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part
whose track carries the region → its own .drumTab; arrIdx < 0 stays the
legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared
_contentList, and parts-view arms kind:'drums' for any drum-arrangement
row. Tests drive a NON-active part and pin that the active tab never
moves, plus the legacy no-arrIdx fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* feat(editor): the drum import lands as a placed, selected region (Add Track from File)

The R3b payoff: importing a drum part into an existing project now
surfaces it in the Tracks view as a SELECTED region, ready to drag. The
Add-Drums dialog gains an inline "Place at" choice — Keep source timing
(default: the file's own timing, no content motion, no persisted
window), Bar 1, or Playhead — the latter two sliding the whole part
there as one undoable PlaceRegionCmd (bar-snapped, beat-preserving).

placeImportedPartAsRegion (track-session.js) is the orchestrator: it
re-normalizes the session so the fresh part's row exists (the import
just created the part — its row is synthesized here), resolves the
track from the part's arr:<idx> target, then places or just selects.
The import flow (arrangement.js) reaches it through the host table —
track-session imports arrangement, so the call inverts through the
usual seam. _placeAtStartBeatPure (region.js) resolves the dialog
choice to a startBeat, converter-free like the other region pures.

Create mode keeps replace semantics: the Place-at row hides, the choice
is forced to 'keep', and with no materialized arrangement the hook
no-ops gracefully.

Tests drive the real orchestrator: row synthesis + placement + selection
+ undo restore, keep-writes-no-window, playhead snapping, and the pure's
resolution table.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* feat(editor): Delete removes the selected region block (content + window, undoable)

Del/Backspace in the Tracks view deletes the selected region as one
DeleteRegionCmd: the notes/hits its end-exclusive window owns plus the
window entry, neighbours untouched. parts-view owns the resolution
(same kind/arrIdx logic as the drag arming, so a drum part's region
always deletes ITS OWN hits, never the active grid's) and input.js's
Delete ladder asks it first through the host table — transcription rows
only; audio regions stay select-only until the audio-region PR. Falls
through cleanly outside the Tracks view / with nothing selected.

Tests drive the real handler: delete + undo round-trip, the gate table,
and the non-active-drum-part case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* fix(editor): route region-delete inside the Tracks-overview key gate

Runtime verification caught the Del key doing nothing in the Tracks
view: onKeyDown's partsViewMode gate returns for every key except the
parts toggle, so the region-delete branch further down the Delete
ladder was unreachable on the only surface it serves. The gate itself
now offers Delete/Backspace to host.partsViewRegionDelete() before
ignoring the key — a region op on this surface, not a note-edit on the
hidden chart — and the dead ladder branch is folded into it.

Verified end-to-end via Playwright against the real host (import a
drums MIDI → Place at Bar 1 → region:2 lands selected → drag commits
"Moved region:2 later" → Delete commits "Deleted region region:2 and
its notes" → undo restores).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* Harden imported region placement commands

* Support legacy drum region placement

---------

Co-authored-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@ChrisBeWithYou ChrisBeWithYou mentioned this pull request Jul 22, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChrisBeWithYou @byrongamatos