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

CHANGELOG.md

@@ -23,6 +23,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Multiple drum parts now work when creating a new song too.** The several-
+  drum-parts feature previously covered only songs you re-open and Save; a
+  *create-mode* session (New Song from a Guitar Pro / MIDI import) was capped at
+  one drum chart. Now, once a melodic track exists, **＋ Track ▸ Drums** adds
+  another drum part in a create session just like an existing one, and Building
+  the song persists every part (the first as the song-level drum tab today's
+  game plays, the rest as `type: drums` arrangement entries per feedpak-spec
+  1.17.0). A drums-only draft still holds one part until you add a melodic
+  track (drums are never the primary chart). Packs stay backward-compatible.
+
 - **Track regions — drag a track's block to move its content.** In the Tracks
   view you can now grab a region block and slide it along the timeline; it snaps
   to bar lines (hold **Alt** for a free nudge), shows a dashed preview of where it

routes.py

@@ -339,6 +339,60 @@ def _sanitize_extra_drum_tab(tab):
     return out
 
 
+def _create_build_drum_entries(staging, drum_tab, drum_parts, drum_tab_id=None):
+    """The `type: drums` manifest arrangement entries for a CREATE-MODE build
+    with multiple drum parts (feedpak 1.17.0 "drums as arrangements"), and the
+    side effect of writing each EXTRA part's `drum_tab_<id>.json` into
+    `staging`. The PRIMARY entry aliases the song-level `drum_tab.json` (already
+    written by the caller) — it contributes id/name only; each extra part gets
+    its own side file. Ids are sanitized to a safe filename charset and
+    de-collided. Returns the entries to append AFTER the pitched arrangements.
+
+    Twin of /save_song's inline drum-parts block; shares
+    `_sanitize_extra_drum_tab` so both wire the same on-disk shape. Extracted
+    to module scope (out of `_write_sloppak_pak`'s closure) so pytest can drive
+    the write directly. Pure of the core `lib` (like `_sanitize_extra_drum_tab`):
+    each part's tab is schema-validated at the request boundary
+    (`build_song_endpoint`), so here junk hits are merely sanitized away.
+    """
+    from pathlib import Path as _Path
+
+    staging = _Path(staging)
+    # The primary's alias entry id FOLLOWS the incoming primary's id
+    # (drum_tab_id = parts[0].id) so a promoted primary round-trips its
+    # identity; legacy / old-client builds fall back to "drums". Twin of
+    # /save_song ~5404.
+    primary_id = _primary_drum_alias_id(drum_tab_id)
+    entries = [{
+        "id": primary_id,
+        "name": str(drum_tab.get("name") or "Drums")[:120],
+        "type": "drums",
+        "drum_tab": "drum_tab.json",
+    }]
+    used_ids = {primary_id}
+    for part in (drum_parts or []):
+        if not isinstance(part, dict) or not isinstance(part.get("drum_tab"), dict):
+            continue
+        pid_raw = str(part.get("id") or "").strip().lower()
+        pid = re.sub(r"[^a-z0-9_-]+", "-", pid_raw).strip("-_") or "drums-2"
+        if pid in used_ids:
+            n = 2
+            while f"{pid}-{n}" in used_ids:
+                n += 1
+            pid = f"{pid}-{n}"
+        used_ids.add(pid)
+        name = str(part.get("name") or part["drum_tab"].get("name") or "Drums")[:120]
+        tab = _sanitize_extra_drum_tab(part["drum_tab"])
+        tab["name"] = name
+        (staging / f"drum_tab_{pid}.json").write_text(
+            json.dumps(tab, separators=(",", ":")), encoding="utf-8")
+        entries.append({
+            "id": pid, "name": name, "type": "drums",
+            "drum_tab": f"drum_tab_{pid}.json",
+        })
+    return entries
+
+
 # Generic "field absent from request" sentinel used by the save endpoint
 # to distinguish "client didn't send this field" from "client explicitly
 # sent an empty list / null". The empty-list case is meaningful for
@@ -8633,6 +8687,32 @@ async def build_song_endpoint(data: dict):
                     {"error": "drum_tab must be a JSON object with a 'hits' array"},
                     400,
                 )
+        # EXTRA drum parts (feedpak 1.17.0 "drums as arrangements"): the
+        # create-mode /build twin of /save_song's `drum_parts`. Absent → an
+        # old frontend, pack keeps just the song-level drum_tab (byte-
+        # identical). A list (possibly empty — the new editorBuild always
+        # ships one) → the authoritative extras, written beside the primary.
+        drum_parts = data.get("drum_parts")
+        if drum_parts is not None:
+            if not isinstance(drum_parts, list):
+                return JSONResponse({"error": "drum_parts must be a list"}, 400)
+            if drum_tab is None:
+                return JSONResponse(
+                    {"error": "drum_parts requires drum_tab in the same build"}, 400)
+            from lib.drums import validate_drum_tab as _validate_part_tab
+            for _pi, _part in enumerate(drum_parts):
+                if not isinstance(_part, dict) or not isinstance(_part.get("drum_tab"), dict):
+                    return JSONResponse(
+                        {"error": f"drum_parts[{_pi}] must be an object with a drum_tab object"},
+                        400,
+                    )
+                # Schema-validate at the boundary (like /save_song), so a bad
+                # tab fails fast with 400 rather than being sanitized to empty
+                # deeper in _create_build_drum_entries.
+                _p_ok, _p_reason = _validate_part_tab(_part["drum_tab"])
+                if not _p_ok:
+                    return JSONResponse(
+                        {"error": f"invalid drum_parts[{_pi}].drum_tab: {_p_reason}"}, 400)
 
         def _build_sloppak():
             """Build a `.sloppak` for the create-mode session.
@@ -8670,6 +8750,8 @@ def _build_sloppak():
                 meta=meta,
                 output_path=output,
                 drum_tab=drum_tab if isinstance(drum_tab, dict) else None,
+                drum_parts=drum_parts,
+                drum_tab_id=data.get("drum_tab_id"),
                 audio_tracks=extra_audio_tracks,
                 audio_guide_name=(build_audio_tracks[0]["name"]
                                   if build_audio_tracks else ""),
@@ -8780,6 +8862,8 @@ def _write_sloppak_pak(*, audio_file: str, art_path: str,
                           arrangements_data: list, beats: list, sections: list,
                           meta: dict, output_path: Path,
                           drum_tab: dict | None = None,
+                          drum_parts: list | None = None,
+                          drum_tab_id: str | None = None,
                           lyrics: list | None = None,
                           preview_path: str = "",
                           fail_if_exists: bool = False,
@@ -9070,6 +9154,20 @@ def _write_sloppak_pak(*, audio_file: str, art_path: str,
                 )
                 manifest["drum_tab"] = "drum_tab.json"
 
+                # DRUM-PART entries (feedpak 1.17.0 "drums as arrangements"):
+                # a create-mode session can hold several drum charts. Written
+                # only when the client OPTED IN by shipping `drum_parts` (a
+                # list, possibly empty) — an old frontend omits it and the pack
+                # stays byte-identical with just the song-level key. The write
+                # lives in the module-level _create_build_drum_entries so pytest
+                # exercises it directly (this closure isn't reachable).
+                if drum_parts is not None:
+                    # Append after the pitched arrangements (drums are never
+                    # index 0 — that slot is the played chart).
+                    manifest["arrangements"] = (
+                        list(manifest.get("arrangements") or [])
+                        + _create_build_drum_entries(staging, drum_tab, drum_parts, drum_tab_id))
+
             # Vocals seed: an empty (or authored) lyrics track. feedpak §7.1
             # lyrics.json is a flat array of syllables — an empty array is a
             # valid, empty track the author fills in later.

src/arrangement.js

@@ -316,13 +316,14 @@ export function editorShowAddDrumsModal() {
 }
 
 // Can a SECOND (third, …) drum part be added — i.e. does a drum import ADD
-// another part (vs replace the existing tab)? True only when the primary part
-// is materialized as a type:"drums" arrangement in a non-create sloppak
-// session — the create flow's build (create_sloppak) persists a single
-// drum_tab, so create mode keeps the legacy one-part replace semantics.
-// (Exported for new-track.js's plan gate + the tests.)
+// another part (vs replace the existing tab)? True once the primary part is
+// materialized as a type:"drums" arrangement in a sloppak session — both the
+// /save_song and create-mode /build paths persist the extras now. A part
+// only materializes beside a pitched track (drums are never index 0), so a
+// drums-only compose session — its lone off-array tab — can't add a second
+// until a melodic track exists. (Exported for new-track.js's plan gate + tests.)
 export function _canAddAnotherDrums() {
-    return !!(S.drumTab && !S.createMode && S.format === 'sloppak'
+    return !!(S.drumTab && S.format === 'sloppak'
         && findDrumArrangement(S.arrangements));
 }
 
@@ -334,13 +335,14 @@ export function editorHideAddDrumsModal() {
 // tab is pure client state until save (S.drumTab + S.drumTabDirty — the
 // same stash the GP/MIDI import path uses), and the create flow's
 // init_drums seeds the identical empty shape, so no backend call is
-// needed. When drums already exist, ADDS another drum part (a song can
-// hold several) — except in create mode, whose build persists one part.
+// needed. When drums already exist, ADDS another drum part (a song can hold
+// several — create and edit sessions alike, once a melodic track exists).
 export function editorAddEmptyDrums() {
     if (!S.sessionId || S.format !== 'sloppak') return false;
     if (S.drumTab && !_canAddAnotherDrums()) {
-        // Create-mode (or an unmaterialized legacy tab): still one part max.
-        setStatus('This song already has a Drums track — open it with 🥁 Edit Drums. Save the song to add more drum parts.');
+        // The only remaining one-part case: a drums-only session whose tab
+        // isn't materialized as an arrangement (no melodic track to sit beside).
+        setStatus('This song already has a Drums track — open it with 🥁 Edit Drums. Add a melodic track first to hold more than one drum part.');
         return false;
     }
     const tab = { version: 1, name: 'Drums', kit: [], hits: [] };

src/create.js

@@ -801,8 +801,34 @@
         })),
     };
 }
+// The drum payload the create-mode /build ships (feedpak 1.17.0 N drums).
+// The PRIMARY drum_tab is the FIRST drums arrangement's payload — NOT the
+// active `drumTab`, which tracks whichever part is open in the grid (the user
+// may be editing a secondary at build time). `drum_parts` (the extras) is only
+// present when drums are materialized as arrangements, so a legacy single-tab
+// compose build (no pitched part → no drums arrangement) stays byte-identical.
+function _drumBuildPayloadPure(arrangements, drumTab) {
+    const drumArrs = (Array.isArray(arrangements) ? arrangements : []).filter(isDrumArrangement);
+    const primaryTab = drumArrs.length ? drumArrs[0].drumTab : drumTab;
+    const out = { drum_tab: (primaryTab && Array.isArray(primaryTab.hits)) ? primaryTab : null };
+    // The promoted primary's durable id (drumArrs[0].id) — the backend
+    // persists the alias entry under it so a survivor promoted to primary
+    // (e.g. "drums-2" after the original was deleted) round-trips its identity
+    // and its stem links / tree rows survive reload. Blank when there is no
+    // materialized drums arrangement → the backend defaults to "drums". Twin
+    // of file-ops.js `_buildSaveBody`'s `body.drum_tab_id`.
+    out.drum_tab_id = drumArrs.length ? String(drumArrs[0].id || '') : '';
+    if (drumArrs.length) {
+        out.drum_parts = drumArrs.slice(1).map(a => ({
+            id: String(a.id || ''),
+            name: String(a.name || 'Drums').slice(0, 120),
+            drum_tab: a.drumTab,
+        }));
+    }
+    return out;
+}
 /* @pure:create-track-table:end */
-export { _createAudioPayloadPure, _createGuideIdPure, _createTrackRowsPure };
+export { _createAudioPayloadPure, _createGuideIdPure, _createTrackRowsPure, _drumBuildPayloadPure };
 
 // Community arrangement XML and MusicXML share the .xml extension. Sniff the
 // document root before choosing an importer; declarations, DOCTYPEs, comments,
@@ -1675,7 +1701,7 @@
 // Left in place rather than deleted, because deleting them is a separate change
 // from the bug fix that made them redundant. They arrived with the same
 // half-wired Create-New redesign (977ec65, #45).
 function _populateCreateArrButtons() {
     const wrap = document.getElementById('editor-create-arr-buttons');
     if (!wrap) return;
     wrap.replaceChildren();
@@ -1876,7 +1902,7 @@
             createState.lastSync = { ...createState.lastSync, ...data };
         }
         return data;
     } catch (e) {
         return null;
     }
 }
@@ -2674,9 +2700,12 @@
                 // sloppak. editorDoCreate sets S.format='sloppak' when the GP
                 // import brought either; forward that as the build target so
                 // the server writes a sloppak (not a archive that silently drops
-                // them), and ship the imported drum_tab so it's persisted.
+                // them), and ship the drum parts so they're persisted.
                 target_format: S.format === 'sloppak' ? 'sloppak' : '',
-                drum_tab: (S.drumTab && Array.isArray(S.drumTab.hits)) ? S.drumTab : null,
+                // The drum payload (primary tab + extras) — see
+                // _drumBuildPayloadPure: the primary is the FIRST drums
+                // arrangement's tab, NOT the active S.drumTab.
+                ..._drumBuildPayloadPure(S.arrangements, S.drumTab),
                 // Audio placement shift ("Shift Audio…") — persisted into the
                 // built pack's manifest (read back on load via data.audio_shift)
                 // so the alignment survives the first build, not just re-saves.

src/new-track.js

@@ -78,16 +78,17 @@ function _renderNewTrackModal() {
     for (const r of modal.querySelectorAll('input[name="new-track-source"]')) {
         r.checked = r.value === _sel.source;
     }
-    // Drums with an existing drum tab: in a saved sloppak session the empty
-    // start ADDS another drum part — say so; in create mode (one part max)
-    // it stays blocked, phrased inline instead of failing at Create.
+    // Drums with an existing drum tab: once a drum part is materialized the
+    // empty start ADDS another (create + edit sessions alike) — say so. The
+    // only blocked case left is a drums-only session (no melodic track to sit
+    // beside), phrased inline instead of failing at Create.
     const note = _byId('editor-new-track-note');
     if (note) {
         const drumsEmpty = isTrans && _sel.instrument === 'Drums' && _sel.source === 'empty' && !!S.drumTab;
         note.textContent = !drumsEmpty ? ''
             : _canAddAnotherDrums()
                 ? 'This song already has drums — this will add another Drums track.'
-                : 'This song already has a Drums track — save the song first to add more drum parts, or choose "Import from a file" to replace it.';
+                : 'This song already has a Drums track — add a melodic track first to hold more than one drum part, or choose "Import from a file" to replace it.';
     }
     const create = _byId('editor-new-track-create');
     if (create) {

tests/create_save_routing.test.mjs

@@ -26,6 +26,7 @@ globalThis.document = globalThis.document || { getElementById: () => null };
 
 const { saveCDLC, _removeEmptyPickedFile, _suggestedSaveNamePure } =
     await import('../src/file-ops.js');
+const { _drumBuildPayloadPure } = await import('../src/create.js');
 const { S } = await import('../src/state.js');
 
 let pass = 0, fail = 0;
@@ -158,5 +159,81 @@ await t('_suggestedSaveNamePure: create mode derives the name the build will wri
     assert.strictEqual(_suggestedSaveNamePure(null, null, null), 'song.feedpak');
 });
 
+// ── create-mode N drums: the /build drum payload (feedpak 1.17.0) ─────────
+const dtab = (name, hits = [{ t: 1, p: 'kick' }]) => ({ version: 1, name, kit: [], hits });
+
+await t('_drumBuildPayloadPure: primary = first drums arr (NOT the active tab); extras follow', () => {
+    const kit = dtab('Drums');
+    const live = dtab('Drums (Live)', [{ t: 2, p: 'snare' }]);
+    const arrs = [
+        { id: 'lead', name: 'Lead', notes: [] },
+        { id: 'drums', name: 'Drums', type: 'drums', drumTab: kit },
+        { id: 'drums-2', name: 'Drums (Live)', type: 'drums', drumTab: live },
+    ];
+    // The user is editing the SECOND part (active tab = live) — the primary
+    // shipped must still be the FIRST part's tab, not the active one.
+    const payload = _drumBuildPayloadPure(arrs, live);
+    assert.strictEqual(payload.drum_tab, kit, 'primary is the first drums arrangement, not the active tab');
+    assert.strictEqual(payload.drum_tab_id, 'drums', 'ships the primary drums arr id, not the active one');
+    assert.deepStrictEqual(payload.drum_parts, [
+        { id: 'drums-2', name: 'Drums (Live)', drum_tab: live },
+    ]);
+});
+
+await t('_drumBuildPayloadPure: a PROMOTED primary ships drum_tab_id = drumArrs[0].id (orphan-on-reload fix)', () => {
+    // Original "drums" deleted → "drums-2" promoted to parts[0]. drum_tab_id
+    // must follow it so the backend aliases the entry under "drums-2".
+    const live = dtab('Drums (Live)', [{ t: 2, p: 'snare' }]);
+    const payload = _drumBuildPayloadPure(
+        [{ id: 'lead', name: 'Lead' }, { id: 'drums-2', name: 'Drums (Live)', type: 'drums', drumTab: live }], live);
+    assert.strictEqual(payload.drum_tab, live);
+    assert.strictEqual(payload.drum_tab_id, 'drums-2');
+});
+
+await t('_drumBuildPayloadPure: one drum part ships an EMPTY extras list (writes the alias entry)', () => {
+    const kit = dtab('Drums');
+    const payload = _drumBuildPayloadPure(
+        [{ id: 'lead', name: 'Lead' }, { id: 'drums', name: 'Drums', type: 'drums', drumTab: kit }], kit);
+    assert.strictEqual(payload.drum_tab, kit);
+    assert.strictEqual(payload.drum_tab_id, 'drums');
+    assert.deepStrictEqual(payload.drum_parts, [], 'materialized single drum → empty extras (not absent)');
+});
+
+await t('_drumBuildPayloadPure: a legacy unmaterialized tab stays byte-identical (no drum_parts key)', () => {
+    const kit = dtab('Drums');
+    // Drums-only compose (no pitched part → no drums arrangement): the tab
+    // rides S.drumTab and there is NO drum_parts key at all.
+    const payload = _drumBuildPayloadPure([], kit);
+    assert.strictEqual(payload.drum_tab, kit);
+    assert.strictEqual(payload.drum_tab_id, '', 'no materialized drums arr → blank id (backend defaults to "drums")');
+    assert.strictEqual('drum_parts' in payload, false);
+    // No drums at all → drum_tab null, blank id, still no drum_parts.
+    assert.deepStrictEqual(_drumBuildPayloadPure([{ id: 'lead' }], null), { drum_tab: null, drum_tab_id: '' });
+});
+
+await t('create-mode build ships the drum parts on the /build wire (real editorBuild through saveCDLC)', async () => {
+    seedCreateSession();
+    const kit = dtab('Drums');
+    const live = dtab('Drums (Live)', [{ t: 2, p: 'snare' }]);
+    // Two materialized drum parts; the user is on the secondary in the grid.
+    S.arrangements.push({ id: 'drums', name: 'Drums', type: 'drums', drumTab: kit });
+    S.arrangements.push({ id: 'drums-2', name: 'Drums (Live)', type: 'drums', drumTab: live });
+    S.drumTab = live;
+    let body = null;
+    globalThis.fetch = async (url, opts) => {
+        body = JSON.parse(opts.body);
+        return { json: async () => ({ success: true, filename: 'x.feedpak' }) };
+    };
+    const ok = await saveCDLC();
+    assert.strictEqual(ok, true);
+    assert.deepStrictEqual(body.drum_tab, kit, 'the PRIMARY tab ships as drum_tab, not the active secondary');
+    assert.strictEqual(body.drum_tab_id, 'drums', 'the /build wire carries the primary id for round-trip');
+    assert.strictEqual(body.drum_parts.length, 1);
+    assert.strictEqual(body.drum_parts[0].id, 'drums-2');
+    assert.strictEqual(body.drum_parts[0].name, 'Drums (Live)');
+    assert.deepStrictEqual(body.drum_parts[0].drum_tab, live);
+    S.arrangements = []; S.drumTab = null;
+});
+
 console.log(`\n${pass} passed, ${fail} failed`);
 process.exit(fail ? 1 : 0);