Skip to content

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

Closed
ChrisBeWithYou wants to merge 23 commits into
mt-drums-p3from
mt-drums-p4
Closed

feat(editor): create-mode songs can hold several drum parts too#340
ChrisBeWithYou wants to merge 23 commits into
mt-drums-p3from
mt-drums-p4

Conversation

@ChrisBeWithYou

Copy link
Copy Markdown
Contributor

PR4 of the drums-as-arrangement arc — stacked on #339 (base mt-drums-p3). Closes the last authoring gap for multiple drum parts.

Why

#339 gave N drums to edit-mode (re-open a song + Save → /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. This brings the create path to parity.

What

Multiple drum parts now persist from a create-mode build, per feedpak-spec 1.17.0: the primary as the song-level drum_tab alias, the 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 and returns the type:"drums" manifest entries (primary alias + extras; ids sanitized + de-collided). Twin of /save_song's inline block, sharing _sanitize_extra_drum_tab for one on-disk shape. Extracted out of the closure and kept pure of core lib so pytest drives the real file write directly.
  • _write_sloppak_pak gains a drum_parts param (opt-in: absent → byte-identical single-drum pack).
  • build_song_endpoint reads + boundary-validates drum_parts (schema-valid tab via lib.drums, fail-fast 400) like /save_song. Other _write_sloppak_pak callers pass nothing → unaffected.

Frontend

  • editorBuild ships the payload via new pure _drumBuildPayloadPure: the primary is the FIRST drums arrangement's tab, not the active S.drumTab (which tracks whichever part is open — the user may be editing a secondary at build time). Extras ship only when drums are materialized → legacy single-tab compose stays byte-identical.
  • _canAddAnotherDrums drops the !S.createMode guard — create sessions add a 2nd part beside a pitched track now. The only one-part case left is a drums-only session (drums are never index 0); the New-Track note + refusal message say so.

Verification

  • 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 + the drums-only guard.
  • pytest 369 green (+5): _create_build_drum_entries real file-write coverage — 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 previously-untested /build write is now directly unit-tested against real file I/O; the frontend wire is integration-tested against the real editorBuild; and the on-disk shape is identical to /save_song's drum-parts persistence that feat(editor): a song can hold several drum parts #339 runtime-verified end-to-end (create parts → save → reload → both parts return). A full create-flow Playwright pass would need a hand-crafted GP/MIDI + audio fixture for a create-mode session — disproportionate given the direct write coverage + format parity.

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

🤖 Generated with Claude Code

ChrisBeWithYou and others added 16 commits July 19, 2026 22:17
…ass)

First brick of the multitrack program (MULTITRACK-FEEDPAK-DESIGN.md Phase 1 /
Milestone A): make a track's instrument identity first-class DATA instead of
name-inferred. The backend already persists a manifest `type` facet
(feedpak-spec §5.2) and never clobbers an authored value; the frontend has been
ignoring it, inferring the instrument from the NAME in a dozen places with two
subtly disagreeing rules (the runtime prefix test vs the save-side word-boundary
test — the disagreement the rename guard band-aids).

- New leaf `src/instrument.js`: `_typeKind` canonicalizes the manifest `type`
  vocabulary ("piano"→keys, plus the plural set drums/vocals grow into) to the
  editor's runtime kind, or null when absent/blank/unrecognized; `_arrTypeKind`
  reads it off an arrangement. Imports nothing from src/ so keys.js / lanes.js
  consult it without closing a cycle.
- The load-bearing keys/bass DATA + view predicates now HONOR an authored `type`,
  falling back to their EXACT prior name test when untyped: `isKeysArr` +
  `viewFor` (keys.js) and the 4-vs-6 bass baseline in
  `_seedExtendedStringsFromTuning` (lanes.js). Untyped/legacy packs are
  byte-identical (a typed part named against its instrument — "Grand Piano" that
  is really a guitar — now resolves to its authored identity, which name
  inference could never do).

Deliberately scoped: the ~30 other name-inference sites and the rename guard
still key off the name — relaxing them is only safe once every identity reader
consults `type`, so they follow behind this same seam. The load path that
populates `arr.type` from the manifest (making existing packs' inferred type
flow through) is the immediate next step.

Tests: tests/instrument_type.test.mjs (7) — the vocabulary map, type-over-name
in both directions, and the byte-identical untyped fallback for keys + the bass
baseline. JS 296/0 + 7 new, lint 0 errors / 3 baseline warnings, routes.py
untouched (no pytest).

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>
Makes the type-as-data seam live: the frontend now receives an authored
instrument `type` and round-trips it, so the keys/bass predicates (previous
commit) act on real data instead of a name guess.

- Load (`_load_sloppak`): carry each manifest arrangement entry's `type` facet
  (feedpak-spec §5.2) onto the arrangement the frontend receives — mirroring the
  existing `id` carry — but only when authored/non-empty, so an untyped entry
  stays untyped and the frontend falls back to name inference (byte-identical).
- Save (full-snapshot path): carry an editor-provided `type` into the rebuilt
  manifest entry, so a type SET in the editor persists and a new typed
  arrangement isn't re-inferred from its name by the infer-once pass. The
  existing `_merge_manifest_entry` still preserves an on-disk type when the
  editor sends none, and infer-once still stamps an untyped entry from its name.

For existing packs this is behavior-identical (their `type` was inferred from
the same name the predicates read); the win is that identity is now authoritative
DATA that survives a rename and can be set explicitly — the contract
drums-as-arrangements and a future "set instrument" action build on.

Tests: tests/test_manifest_type_preserve.py +2 — a newly-typed arrangement
persists its type, and an editor-set type overrides a stale on-disk value while
unrelated preserved keys survive. pytest 381/0; JS 296/0 + 7; lint 0 err / 3
baseline. CHANGELOG updated.

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>
Turns the type-as-data seam into a visible feature and consolidates the name
inference into one place.

- `src/instrument.js`: `KEYS_PATTERN` moves here (its canonical home — keys.js
  re-exports it so its ~10 importers are unchanged), plus `_arrKindFromName` (the
  ONE runtime name-inference implementation) and `arrKind(arr)` — the canonical
  instrument kind, authored `type` first, name inference fallback. (arrangement.js
  `_arrKindPure` stays self-contained because its `@pure:rename-arr` block is
  sliced+eval'd by rename_part.test.mjs; it mirrors `_arrKindFromName`.)
- `src/track-session.js`: the Tracks list badges a transcription row with its
  INSTRUMENT (GTR / BAS / KEY / DRM / VOX) via `arrKind`, not a generic "MIDI";
  audio rows still show their layer (MIX / AUD). New pure `_trackKindBadgePure`
  so it unit-tests without the DOM; a mis-named part badges by what it IS.

Tests: tests/instrument_type.test.mjs +4 — the re-exported KEYS_PATTERN,
`_arrKindFromName` (keys-before-bass, /^drums/ prefix), `arrKind` type-over-name,
and the badge for audio/drums/keys/bass(typed)/vocals rows. JS 296/0, lint 0
err / 3 baseline. CHANGELOG updated.

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>
Extends the type-authoritative identity from the keys/bass DATA predicates to
the keys/drums VIEW and audio routing — every one honors an authored `type`
first, falling back to its exact prior name test, so all are byte-identical for
untyped/legacy packs (verified: full suite green) but correct for a typed part
named against its instrument.

Converted (13 sites, 6 files): key-view.js (the view switcher's fretted /
isDrums / "keys are piano-locked" / drums-have-no-tab decisions), audio.js
(drum parts voice their rhythm as claps, not GM pitch), tab-view-live.js (the
engraved lens refuses keys/drums), main.js (the Strings button hides for keys/
drums), parts-view.js (the keys silhouette mini-roll), keys.js `_rollPitchCtxFor`
(no fretted pitch context for keys). KEYS_PATTERN's now-unused imports dropped
where every use was converted (its home is the instrument leaf; keys.js
re-exports it for the sites still on the name).

Still on the name (a follow-up completes them, and the rename guard STAYS as the
safety net until every runtime layout reader is converted): the bass/string-count
sites (strings.js, lanes add/remove) and the import/export helpers.

Tests: tests/instrument_type.test.mjs +1 — `_rollPitchCtxFor` returns null for a
typed-keys part (and a real ctx for a typed guitar named "Piano"). JS 296/0,
lint 0 err / 3 baseline.

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>
…type

Finishes the RENAME-SAFETY-CRITICAL readers: every string-count / open-tuning
decision — where a bass↔guitar flip would strand notes onto invisible strings —
now honors an authored `type` first, via `_isBassArr` (new in instrument.js).
Crucially `_isBassArr` keeps the INDEPENDENT `/bass/` name fallback, NOT
`arrKind === 'bass'`: bass and keys are independent facets in the legacy
inference, so an untyped "Synth Bass" must stay a 4-string bass (arrKind, being
single-kind with keys-wins, would wrongly make it 6). Byte-identical for untyped.

Converted: lanes.js `_stringCountFor` / `_openMidiForArr` / isBassArr /
`_seedExtendedStringsFromTuning` (unified on `_isBassArr`); strings.js (the modal
gate + all 7 add/remove-string bass sites + `_stringButtonsVisiblePure`, now
kind-based); file-ops.js archive-limit; and the import/create readers
(import.js keys/bass filters, create.js keys-detection). KEYS_PATTERN's
now-unused imports dropped where every use was converted.

Still name-based ON PURPOSE (the capstone follow-up converts them + relaxes the
rename guard): the @pure/self-contained NON-stranding readers — gm-guide voice
`_gmKindPure`, the tab-preview / gp5-export guards, the parts-view silhouette
`_partsArrKindPure` — and the rename guard's own name inference (arrangement.js),
which stays until every reader honors type.

Tests: instrument_type.test.mjs +2 — `_isBassArr` independent-fallback and
`_stringCountFor` type-driven baseline; strings_modal.test.mjs injects the real
`_isBassArr` into its sliced-handler env; canvas_string_buttons passes kinds.
JS 296/0, lint 0 err / 3 baseline.

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>
…honor type

The capstone of the instrument-type-as-data arc: convert the remaining
NON-stranding name readers to the resolved kind, then RELAX the rename guard so
a typed track's name becomes a pure display label. Every reader now consults an
authored `type` (via arrKind), so identity no longer rides the name anywhere.

Converted (each takes the resolved kind; caller passes arrKind(arr)):
- gm-guide `_gmKindPure` — the guide-instrument VOICE (keys/bass/guitar; drums
  & vocals fall to the guitar voice). Callers: audio.js x2, menu-bar.js.
- tab-preview `_tabPreviewGuardPure` + gp5-export `_gp5ExportGuardPure` — the
  fretted-only guards; keys/drums are refused by identity, not a name regex.
- parts-view `_partsArrKindPure` — the per-lane silhouette tag.
KEYS_PATTERN's now-unused import dropped from gm-guide.js.

Rename guard: `_renameGuardPure` gains a `typed` param — when the arrangement
carries an authored `type` the kind-change refusal is SKIPPED (a rename can't
re-lane a part whose identity is data), while empty/too-long/duplicate/no-op
still hold. Untyped/legacy packs keep the full name-inference guard, so they're
still protected from silently re-laning notes onto invisible strings. Threaded
through arrangement.js (`!!_arrTypeKind(arr)`) and track-session.js's
`_trackTranscriptionRenameGuardPure`.

Payoff: a guitar authored `type:"guitar"` but named "Piano" now sounds with a
guitar voice, previews/exports its tab, silhouettes as guitar, and renames
freely — on main all of these keyed off the misleading name.

Byte-identical for UNTYPED inputs, so the 296-suite can't catch a bad
conversion — added a TYPED test per reader: gm_guide / gp5_export (incl. the
"guitar-typed Piano-named exports" payoff through the real orchestrator) /
rename_part (typed renames across kinds; structural checks still hold) /
parts_view (kind→tag), plus a consolidated "capstone readers honor type via
arrKind" block in instrument_type.test.mjs (exported the tab-preview & parts
pures to compose them there). The sliced-source tests now pass kinds;
tab_preview_race injects arrKind. JS 296/0, lint 0 err / 3 baseline.

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>
The foundation for multiple drum charts (the drums-as-arrangements arc). The
single drum tab has always lived ENTIRELY OUTSIDE S.arrangements[] as a lone
off-array singleton edited through a global mode — the one instrument that
wasn't an ordinary arrangement. This gives it a home IN the list as a derived
`type:"drums"` arrangement (via the #335 instrument-type-as-data seam), the
substrate multiple drum charts grow from. BYTE-IDENTICAL: no pack change, no UI
change, drum-editor undo untouched.

New leaf `src/drum-arrangement.js`:
- `syncDrumArrangement(S)` materializes / updates / removes the drums
  arrangement so its `.drumTab` payload IS S.drumTab — the SAME object
  reference, so every existing S.drumTab reader/mutator and every drum undo
  command (which hold references into S.drumTab.hits) keep working unchanged.
  APPENDS (never inserts), so existing arr indices / arr:<idx> keys are stable.
  Called at every S.drumTab (re)assignment: load migration (file-ops), GP/MIDI
  import + empty-add (arrangement.js), delete + its undo-restore (DeleteDrumTabCmd).
- `isDrumArrangement` keys on the authored `type` ALONE (normalized), NOT
  arrKind — arrKind would name-infer and wrongly catch a pitched part a user
  literally named "Drums", then hide/drop it. The materialized arrangement
  always carries `type:"drums"`, so a type-only test is exact and safe.
- `pitchedArrangementCount` / `clampAwayFromDrums` / `pitchedIndexOf` keep the
  index math correct now that S.arrangements[] can hold a drums entry: the
  remove-last-arrangement guard counts pitched parts, S.currentArr never lands
  on the drums arrangement, and the /remove-arrangement backend index is mapped
  to its pitched-only position (the backend manifest has no drums arrangement).

Byte-identical surfaces (the drums arrangement is bridged to the legacy paths;
promoting them to arr:<idx> + drum-edit-via-selection is the follow-up):
- Save (file-ops `_buildSaveBody`, create `editorBuild`): the drums arrangement
  is EXCLUDED from body.arrangements — drums still persist as the song-level
  `drum_tab`, so the built pack is byte-identical (and no drums entry reaches
  arrangements[], where an old core would fretted-grade it as garbage).
- Tracks targets (`_trackSessionTargetsPure`), Parts view (`_partsListPure`),
  band roster (`_bandPartsPure`), pitched switcher (updateArrangementSelector):
  each skips the drums arrangement so it isn't listed twice — drums stay the
  legacy `'drums'` target/key. routes.py untouched.

Tests (tests/drum_arrangement.test.mjs, +15): the sync state machine
(materialize/update/remove/idempotent/append/same-ref/degrade), the
remove→restore undo round-trip, byte-identical load→save, the "Drums"-NAMED-but-
untyped safety case (survives save, keeps its arr target), no duplicate
tracks/band rows, and the index helpers incl. the interspersed-drums backend
index. JS 297/0, lint 0 err / 3 baseline, routes.py untouched (no pytest).

Stacked on #335 (needs arrKind / _arrTypeKind / the `type` round-trip).

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>
Builds on the drums-as-arrangement foundation (#336): the drums arrangement now
appears as a "🥁 Drums" option in the part <select>, so a charter opens the drum
editor by picking it — exactly like switching to Lead/Rhythm/Bass — instead of
reaching for a mode button off to the side. The drum chart becomes a first-class
part in the one place parts are chosen, and the set-up for choosing between
MULTIPLE drum charts later.

SAFE invariant kept: S.currentArr NEVER moves onto the drums arrangement. The
drum grid stays a MODE (S.drumEditMode) OVER the current pitched part — a brand-
new "currentArr === drums" state would have exposed the drums arrangement (empty
notes, no tuning) to the pervasive "current arrangement" readers, none of which
expect it. So a new `editorSwitcherSelect` wrapper (the <select> onchange) routes
a drums option into drum-edit mode WITHOUT touching currentArr (mirroring the
Tracks 'drums' row / openTrackSessionTarget), and a pitched option leaves drum
mode and selects it normally. editorSelectArrangement stays drums-unaware, so its
other callers (undo replay, the Tracks row) are unchanged. A drums index can
never reach editorSelectArrangement (the isDrumArrangement branch returns first).

- screen.html: the switcher onchange → editorSwitcherSelect.
- main.js: editorSwitcherSelect (new); updateArrangementSelector lists the drums
  option (🥁-marked) and, while drum-edit mode is on, DISPLAYS it as selected via
  the new `switcherShownIndex` pure (currentArr stays pitched); openTrackSessionTarget
  refreshes the selector so a Tracks-row pick reflects in the dropdown.
- drum.js: the 🥁 Edit Drums toggle refreshes the selector too — all three drum
  entry points keep the dropdown in sync.
- drum-arrangement.js: `drumArrangementIndex` + `switcherShownIndex` (pure).

RUNTIME-VERIFIED (Playwright, the one library pack with drums — keys arr + drums):
dropdown shows [Keys, 🥁 Drums]; selecting Drums opens the drum grid + shows
🥁 Drums selected; selecting Keys returns the piano roll; the 🥁 Edit Drums toggle
also flips the dropdown; no console errors; the pitched view is intact after
toggling back (currentArr never landed on drums). Tests: drum_arrangement.test.mjs
+2 (drumArrangementIndex, switcherShownIndex). JS 297/0, lint 0 err / 3 baseline,
routes.py untouched. Stacked on #336.

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>
…hannel

PR2b of the drums-as-arrangement arc (after #336 foundation, #337 switcher):
retire the legacy `'drums'` MIX-KEY singleton so the drums arrangement mixes on
its own `arr:<idx>` channel — the SAME address every pitched part uses — and the
mixer strip, Tracks mix, band roster and S.partMix key all agree. This is the
substrate the N-drums work (PR3/PR4) needs: two drum charts can't share one
'drums' slot; each drum arrangement now gets its own strip by index.

DELIBERATELY NARROW (holds the #337 invariant): S.currentArr stays PITCHED and
the drum grid stays a MODE (S.drumEditMode). Only the mix ADDRESS moves. The
drums arrangement's DURABLE target id stays 'drums' (DRUM_TARGET_ID), so every
tracks-row / stemLinks / delete / rename / selection hook that keys on it — and
main.js's selectTrackSessionTarget/openTrackSessionTarget 'drums' branches — are
untouched. Only the audio/mix channel key changes.

Also fixes a latent double-strip: #336 materialized the drums arrangement into
S.arrangements but left _mixerPartsPure ALSO synthesizing a 'drums' strip from
the drum tab, so the mixer showed the drums twice (arr:<idx> + 'drums'). The
drums strip now comes solely from the arrangement pass.

- mixer-panel.js: _mixerPartsPure drops the synthesized 'drums' strip (drums ride
  the arrangement pass as arr:<idx>); new `_mixerActivePartKeyPure` maps drum-edit
  mode to arr:<drumIdx> (currentArr stays pitched); _mixerClapStatePure takes the
  drum index; _selectedStripKeyPure and the meter active-part resolve drums via the
  general id->index path (its id is 'drums').
- track-session.js: the drum target's mixKey -> 'arr:<drumIdx>' (id stays 'drums');
  DeleteDrumTabCmd saves/restores partMix['arr:<drumIdx>'] (re-derives the index on
  undo, robust to a shifted pitched count).
- audio.js: _bandPartsPure keys the drum band entry 'arr:<drumIdx>' (idx -> the
  drums arrangement) so the strip and engine agree; the band scheduler routes the
  type:"drums" arrangement to real GM percussion (_drumKitVoicesInWindow) BEFORE
  the clap-notes path (its own notes are empty), keeping the legacy "Drums"-NAMED
  pitched-part clap path intact.
- host.js / state.js: partMix key-convention docs.

RUNTIME-VERIFIED (Playwright, asdf_asdf.feedpak — the one library pack with drums):
the mixer shows a single Drums strip keyed arr:1 (no 'drums' key, no double-strip);
mute lights it (aria-pressed, real S.partMix) + status "its guide voice is silent";
solo lights it; the fader scales it (+0.0 -> -10.5 dB); drums<->pitched round-trips.
Tests: JS 297/0 (5 suites re-pointed to arr:<idx> + a live-S clap-gate + a
_mixerActivePartKeyPure test), lint 0 err / 3 baseline, routes.py untouched.
Stacked on #337.

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>
PR3 of the drums-as-arrangement arc (after #336 foundation, #337 switcher,
#338 mixer channel): N DRUMS. Each `type:"drums"` arrangement now OWNS its
`.drumTab` payload and `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); the 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 per-entry file/notation gate), so built packs
stay fully compatible: an old reader sees one drum, the editor reads all back.

Verbs and surfaces:
- New Track / +Drums with drums present ADDS another part (unique durable id
  'drums-2'…, de-duplicated name); a GP/MIDI drum import ADDs instead of
  replacing (create mode keeps one-part replace: its build persists a single
  drum_tab). Guarded so a drums-only session never materializes drums at
  index 0 (currentArr would land on it — the #337 invariant holds throughout;
  the same guard now also materializes drums in create-mode imports beside a
  pitched part, restoring the create-mode mixer strip).
- Switcher: one 🥁 option per part; selecting one re-points the grid to ITS
  tab; the dropdown displays the ACTIVE part while the grid is open.
- Tracks: every drum part is its own row/target (id = arrangement id; the
  primary keeps 'drums', so stemLinks/delete/rename hooks are unchanged);
  rename writes the part's OWN tab name (not the active grid tab's — a rename
  of a non-open part previously would have hit the wrong tab); delete is
  per-part via the generalized DeleteDrumTabCmd (full partMix snapshot,
  active-part handover / promotion, exact undo round-trip).
- Audio: band mode voices each part's OWN hits through its own arr:<idx>
  channel, part-scoped dedupe (two parts hitting the same piece on the same
  millisecond BOTH sound). Parts-view lanes render per-part.
- Wire: body.drum_tab = the PRIMARY part's tab (NOT S.drumTab — the user may
  be editing a secondary when saving; pinned by test), body.drum_parts = the
  extras (empty list = removal wire), both gated on drumTabDirty; drum_parts
  hits ride the same seconds-only beat-strip. routes.py: validates drum_parts
  at the boundary, splits drum pointer entries 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),
  writes drum_tab_<id>.json per extra + reconciles the type:drums entries
  (primary alias included per the spec example), preserves other writers'
  entries when the field is absent, drops orphaned side files, and reads
  extras back on load (path-guarded, validated, alias-deduped).

Tests: JS 297 files green — new coverage for the active-part model, add/adopt
ids, multi-part sync safety, per-part delete round-trip incl. partMix map +
mode/promotion, the primary-not-active save-wire pin, 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 (Playwright, asdf_asdf.feedpak): 14/14 — the New
Track dialog adds "Drums 2" (says "add another", Create enabled), switcher
[Keys, 🥁 Drums, 🥁 Drums 2], grids flip per-part, mixer strips arr:1+arr:2
(no 'drums' key), SAVE writes drum_tab_drums-2.json + both type:drums manifest
entries + keeps the song-level key, RELOAD brings both parts back with their
strips, no console errors.

Docs: CHANGELOG + USER-GUIDE §7 (multiple drum parts). Stacked on #338.

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>
Materializing drums as a type:"drums" entry in S.arrangements left several
index/iteration consumers unaudited. Fixes, all via the existing
isDrumArrangement / pitchedArrangementCount helpers:
- _mixerPartsPure: skip the drums arrangement so it no longer gets a duplicate
  arr:<idx> strip on top of its dedicated drums strip
- reorder bounds + down-button gated on pitchedArrangementCount so a pitched
  part can't swap below drums (which would shift arr:<idx> mix keys)
- Remove/reorder button gates use pitchedArrangementCount (1-pitched+drums song)
- stem->chart pairing dropdown filters out the drums arrangement
- create/import path calls syncDrumArrangement so drums materializes immediately

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
editorSwitcherSelect cleared partsViewMode/tempoMapMode but not tabViewMode, so
switching between drums and a pitched part kept rendering the previous part's
tab lens (draw() checks tabViewMode first). Clear the tab lens on both branches
and in openTrackSessionTarget (shared root cause), and resync the <select>
before the sloppak guard's early-return. The pitched branch also failed to
clear partsViewMode/tempoMapMode/tempoSel, so switching to a pitched part from
Tempo Map or Parts view left the old lens painted over it; clear those too,
mirroring the drums branch and openTrackSessionTarget.

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

DeleteDrumTabCmd deleted the drums arrangement without renumbering the higher
arr:<idx> partMix keys, so deleting drums from a non-last position stranded
every pitched strip's mute/solo/volume and could silence the band via an
orphaned solo. Route the drop through the existing _partMixDropArrangementPure
so keys renumber like the pitched-delete path.

Undo must be a true inverse: syncDrumArrangement re-appends drums LAST, which
shifts later arrangements and strands index-based undo commands + currentArr.
Restore drums to its ORIGINAL slot on rollback; add _partMixInsertArrangementPure
(exact inverse of the drop) so live mix edits made while drums was gone ride
along instead of being clobbered; snapshot/restore currentArr; guard drumIndex>=0
so an unmaterialized drums tab can't shift arr:0 to arr:-1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Instrument is now first-class data, so an already-saved pack whose fretted chart
was stamped type=piano (keys-word name) opened piano-locked with no way back to
string view. Add a Guitar/Bass/Keys selector by the arrangement dropdown that
authors arr.type, undoable via S.history (SetArrangementTypeCmd), rebuilding the
chart in place with no data loss.

Codex preflight fixes:
- metadataScope lock opt-out so the type set works in the read-only piano roll
  (the escape hatch was non-functional exactly where it is needed)
- canonicalize keys -> "piano" on write (spec canonical spelling; other
  consumers key on piano)
- carry authored type through the create-mode build whitelist (was dropped)
- guard the no-op check on the authored kind so stamping an inferred kind works
  (unblocks the rename-proofing workflow)

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

The primary drum part's id was regenerated positionally as "drums" instead of
round-tripping. After deleting the original primary, the promoted survivor
(e.g. drums-2) was re-saved with alias id "drums" while its stem link and tree
placement stayed keyed by drums-2, so _trackSessionNormalizePure dropped them on
reload (chart data survived; the stem pairing + folder placement were lost).

Make the id follow parts[0].id end to end: ship body.drum_tab_id on save; add
_primary_drum_alias_id() (filename-safe, defaults "drums") so routes.py writes
the alias entry id from it and surfaces it on load; syncDrumArrangement honors a
non-colliding persisted id. Legacy single-drum packs round-trip byte-identically
(no drum_tab_id -> primary stays "drums"). Adds a JS load-seam round-trip test
and a backend alias-id test (closes the untested save/reload seam).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a8f81cfb-01dd-475c-aee1-4c14c8b04731

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mt-drums-p4

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.

@byrongamatos
byrongamatos deleted the branch mt-drums-p3 July 21, 2026 11:40
byrongamatos added a commit that referenced this pull request Jul 21, 2026
…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
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants