feat(editor): create-mode songs can hold several drum parts too - #340
Closed
ChrisBeWithYou wants to merge 23 commits into
Closed
feat(editor): create-mode songs can hold several drum parts too#340ChrisBeWithYou wants to merge 23 commits into
ChrisBeWithYou wants to merge 23 commits into
Conversation
…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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
added 7 commits
July 20, 2026 22:10
…s-p1 # Conflicts: # src/arrangement.js # src/main.js
# Conflicts: # src/main.js
# Conflicts: # src/arrangement.js # src/create.js # src/main.js # src/track-session.js
# Conflicts: # tests/test_drum_parts.py
byrongamatos
force-pushed
the
mt-drums-p3
branch
from
July 21, 2026 11:40
96bdbb4 to
f61eb10
Compare
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>
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
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.
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-leveldrum_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_tabalias, the extras astype:"drums"arrangement entries with per-arrangementdrum_tabpointers (old readers see one drum, new readers all).Backend (
routes.py)_create_build_drum_entries(staging, drum_tab, drum_parts)— writes each extra part'sdrum_tab_<id>.jsonand returns thetype:"drums"manifest entries (primary alias + extras; ids sanitized + de-collided). Twin of/save_song's inline block, sharing_sanitize_extra_drum_tabfor one on-disk shape. Extracted out of the closure and kept pure of corelibso pytest drives the real file write directly._write_sloppak_pakgains adrum_partsparam (opt-in: absent → byte-identical single-drum pack).build_song_endpointreads + boundary-validatesdrum_parts(schema-valid tab vialib.drums, fail-fast 400) like/save_song. Other_write_sloppak_pakcallers pass nothing → unaffected.Frontend
editorBuildships the payload via new pure_drumBuildPayloadPure: the primary is the FIRST drums arrangement's tab, not the activeS.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._canAddAnotherDrumsdrops the!S.createModeguard — 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
_drumBuildPayloadPure(primary-not-active, empty-extras-still-ships, byte-identical fallback) + a realeditorBuild→/buildbody integration test throughsaveCDLC(asserts the primary + extras on the wire); new_track create-mode add-a-2nd + the drums-only guard._create_build_drum_entriesreal file-write coverage — side files + alias, empty extras, hit sanitation + name propagation, id de-collision + filename sanitization, non-list-hits sanitized-not-crashing./buildwrite is now directly unit-tested against real file I/O; the frontend wire is integration-tested against the realeditorBuild; 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.🤖 Generated with Claude Code