fix(editor): a multi-drum Guitar Pro import keeps each drummer separate

CHANGELOG.md

@@ -94,6 +94,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   accepts the create-flow upload path too, and the drum-tab stash step is
   shared with the Add-Drums modal (`_stashImportedDrumTab`). Covered by
   `tests/midi_drum_autosplit.test.mjs` and runtime-verified end-to-end.
+- **Importing a Guitar Pro song with several drum tracks no longer squashes
+  them into one part.** The Create-New GP import folded *every* drum track into
+  a single `drum_tab`, so a second drummer or an aux-percussion layer lost its
+  separation. It now keeps each drum track as its own `type:"drums"` part — the
+  first as the primary and the rest as `drum_parts` (the same shape a reload
+  adopts and `/build` persists), with each GP track's name preserved. A song
+  with a single drum track, or a drums-only import (which can't hold plural
+  parts — a drums arrangement never sits at index 0), still folds into one, so
+  those imports are byte-identical. The split lives in a new module-level
+  `_gp_drum_arrs_to_parts` (covered by `tests/test_gp_multidrum_split.py`); the
+  frontend adopts the extras via the existing `adoptDrumParts` load path. This
+  brings the GP create-import to parity with the MIDI drum auto-split.
 
 - **A drum edit no longer strips another tool's authored keys from drum-part
   manifest entries.** Saving with drum changes rebuilds the `type: drums`

routes.py

@@ -283,6 +283,49 @@ def _take(entry):
     }
 
 
+def _gp_drum_arrs_to_parts(drum_arrs, has_pitched):
+    """Split GP drum arrangements into (primary_tab, extra_parts, unmapped).
+
+    With SEVERAL drum tracks AND a melodic part, keep each drummer as its own
+    part: one drum_tab per arrangement (the GP track name preserved), the first
+    non-empty as the primary (song-level `drum_tab` back-compat alias), the rest
+    as `drum_parts` [{id, name, drum_tab}] — the shape a reload adopts and
+    /build persists. One drum track, or a drums-only import (no melodic part to
+    sit beside — a drums arrangement never sits at index 0), folds into a single
+    tab, byte-identical to the prior behavior. `primary_tab` is None only when
+    there is nothing to keep (no hits and nothing to remap). Module-level so
+    pytest can reach it, mirroring `_drum_arrs_to_drum_tab`.
+    """
+    unmapped: dict[object, dict] = {}
+    if len(drum_arrs) > 1 and has_pitched:
+        kept = []
+        for darr in drum_arrs:
+            part_unmapped: dict[int, dict] = {}
+            tab = _drum_arrs_to_drum_tab([darr], out_unmapped=part_unmapped)
+            name = (darr.get("name") or "").strip()
+            if name:
+                tab["name"] = name[:120]
+            # An unmapped-only track still needs its own writable tab. The
+            # manual mapper uses this retained part index to restore notes to
+            # their source drummer instead of aggregating them on the primary.
+            if tab["hits"] or part_unmapped:
+                part_index = len(kept)
+                kept.append(tab)
+                for midi, rec in part_unmapped.items():
+                    unmapped[(part_index, midi)] = rec
+        if kept:
+            extras = [
+                {"id": "", "name": t.get("name") or "Drums", "drum_tab": t}
+                for t in kept[1:]
+            ]
+            return kept[0], extras, unmapped
+        return None, [], unmapped
+    tab = _drum_arrs_to_drum_tab(drum_arrs, out_unmapped=unmapped)
+    if tab["hits"] or unmapped:
+        return tab, [], unmapped
+    return None, [], unmapped
+
+
 def _is_drum_pointer_entry(entry):
     """A manifest `arrangements[]` entry that is a DRUM-PART POINTER —
     feedpak-spec 1.17.0 "drums as arrangements": `type: drums` with a
@@ -7848,22 +7891,29 @@ def _convert():
             # add route's bare `unmapped`) because this response also carries
             # arrangements, tempo and track data — an unqualified key there
             # would read as "unmapped WHAT?".
-            _unmapped: dict[int, dict] = {}
-            _tab = _drum_arrs_to_drum_tab(
-                [_arrs[i] for i in sorted(_drum_idx)], out_unmapped=_unmapped,
+            _drum_arr_list = [_arrs[i] for i in sorted(_drum_idx)]
+            _pitched = [a for i, a in enumerate(_arrs) if i not in _drum_idx]
+            _primary, _extras, _unmapped = _gp_drum_arrs_to_parts(
+                _drum_arr_list, bool(_pitched),
             )
-            # Keep an empty tab when every note needs manual mapping. The
-            # mapper writes its recovered hits into this object; omitting it
-            # would make the all-unmapped case unrecoverable.
-            if _tab["hits"] or _unmapped:
-                result["drum_tab"] = _tab
+            if _primary is not None:
+                result["drum_tab"] = _primary
+            if _extras:
+                result["drum_parts"] = _extras
             if _unmapped:
-                result["drum_unmapped"] = [
-                    _safe_unmapped_entry(m, rec) for m, rec in sorted(_unmapped.items())
-                ]
-            result["arrangements"] = [
-                a for i, a in enumerate(_arrs) if i not in _drum_idx
-            ]
+                _rows = []
+                for _key, _rec in sorted(_unmapped.items()):
+                    if isinstance(_key, tuple):
+                        _part_index, _midi = _key
+                    else:
+                        _part_index, _midi = 0, _key
+                    _row = _safe_unmapped_entry(_midi, _rec)
+                    # The create mapper uses this to restore each note to the
+                    # GP drum track that owned it. Legacy folded imports are 0.
+                    _row["drum_part_index"] = _part_index
+                    _rows.append(_row)
+                result["drum_unmapped"] = _rows
+            result["arrangements"] = _pitched
             _xf = sessions[session_id]["xml_files"]
             sessions[session_id]["xml_files"] = [
                 xf for i, xf in enumerate(_xf) if i not in _drum_idx

src/create.js

@@ -27,6 +27,7 @@
 import { isDrumArrangement, pitchedArrangementCount, syncDrumArrangement } from './drum-arrangement.js';
 import { importMidiDrumTracksIntoSession } from './arrangement.js';
 import { EditHistory } from './history.js';
+import { adoptDrumParts } from './drum-arrangement.js';
 import { host } from './host.js';
 import { isKeysMode, updatePianoRange } from './keys.js';
 import { arrKind } from './instrument.js';
@@ -1711,7 +1712,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();
@@ -1912,7 +1913,7 @@
             createState.lastSync = { ...createState.lastSync, ...data };
         }
         return data;
     } catch (e) {
         return null;
     }
 }
@@ -2470,7 +2471,17 @@
     // sessions as well — but ONLY beside a pitched part: a drums-only import
     // must not put a drums arrangement at index 0, where the default
     // currentArr would land on it (it stays a legacy off-array tab instead).
-    if (pitchedArrangementCount(S.arrangements) > 0) syncDrumArrangement(S);
+    if (pitchedArrangementCount(S.arrangements) > 0) {
+        syncDrumArrangement(S);
+        // A GP file with SEVERAL drum tracks (a second drummer, an aux-perc
+        // layer) returns the extras as `drum_parts` — adopt each as its own
+        // type:"drums" part beside the primary instead of folding them into
+        // one. The backend only emits drum_parts when a melodic part exists,
+        // so the primary is always materialized here first.
+        if (Array.isArray(data.drum_parts) && data.drum_parts.length) {
+            adoptDrumParts(S, data.drum_parts);
+        }
+    }
     // The DAW track-session feature is stacked independently. Its host hook is
     // inert on this branch, but when present it receives every create-time
     // source immediately so stems do not appear only after a save/reopen.

src/import.js

@@ -14,6 +14,7 @@ import { DRUM_PIECE_META, DRUM_PIECE_ORDER, _drumImportHitPure } from './drum.js
 import { _uniqueKeysName, updatePianoRange } from './keys.js';
 import { arrKind, _isBassArr } from './instrument.js';
 import { flattenChords } from './chords.js';
+import { drumArrangements } from './drum-arrangement.js';
 import { host } from './host.js';
 
 // ════════════════════════════════════════════════════════════════════
@@ -1100,6 +1101,17 @@ export function _maybeOfferMidiTempoMap(tempoMap, onDone) {
     document.body.appendChild(modal);
 }
 
+export function _drumImportTargetTabPure(state, rawPartIndex) {
+    const index = Number(rawPartIndex);
+    if (Number.isInteger(index) && index >= 0) {
+        const holder = drumArrangements(state && state.arrangements)[index];
+        if (holder && holder.drumTab && Array.isArray(holder.drumTab.hits)) {
+            return holder.drumTab;
+        }
+    }
+    return state && state.drumTab;
+}
+
 export function _showDrumImportUnmappedModal(unmapped) {
     document.getElementById('editor-drum-unmapped-modal')?.remove();
 
@@ -1147,6 +1159,7 @@ export function _showDrumImportUnmappedModal(unmapped) {
         tr.className = 'border-t border-gray-800';
         tr.dataset.midi = String(u.midi);
         rowTimes.set(tr, {
+            targetTab: _drumImportTargetTabPure(S, u.drum_part_index),
             times: Array.isArray(u.times) ? u.times : [],
             // Optional, index-aligned with times when the server captured
             // them — the source notes' REAL velocities.
@@ -1205,18 +1218,22 @@ export function _showDrumImportUnmappedModal(unmapped) {
             modal.remove();
             return;
         }
-        // Build a key-set of existing hits so we don't duplicate against
-        // the imported drum_tab if two unmapped notes resolve to the
-        // same (rounded-time, piece) — keeps the editor's in-memory
-        // hits consistent with what the server would dedupe on save.
-        const seen = new Set(S.drumTab.hits.map(
-            h => `${Math.round((h.t || 0) * 1000)}|${h.p}`));
+        const seenByTab = new Map();
+        const touchedTabs = new Set();
         let added = 0, skipped = 0;
         for (const tr of tbody.querySelectorAll('tr')) {
             const sel = tr.querySelector('select');
             if (!sel || !sel.value) continue;
             const pid = sel.value;
-            const row = rowTimes.get(tr) || { times: [], vels: null };
+            const row = rowTimes.get(tr) || { targetTab: S.drumTab, times: [], vels: null };
+            const targetTab = row.targetTab || S.drumTab;
+            if (!targetTab || !Array.isArray(targetTab.hits)) continue;
+            let seen = seenByTab.get(targetTab);
+            if (!seen) {
+                seen = new Set(targetTab.hits.map(
+                    h => `${Math.round((h.t || 0) * 1000)}|${h.p}`));
+                seenByTab.set(targetTab, seen);
+            }
             for (let ti = 0; ti < row.times.length; ti++) {
                 const t = row.times[ti];
                 // Guard against malformed payload: skip NaN / Infinity /
@@ -1236,12 +1253,13 @@ export function _showDrumImportUnmappedModal(unmapped) {
                 // and round-trips as a ghost identically to the same note
                 // imported through the normal path.
                 const rawV = row.vels ? row.vels[ti] : undefined;
-                S.drumTab.hits.push(_drumImportHitPure(tRounded, pid, rawV));
+                targetTab.hits.push(_drumImportHitPure(tRounded, pid, rawV));
+                touchedTabs.add(targetTab);
                 added++;
             }
         }
         if (added > 0) {
-            S.drumTab.hits.sort((a, b) => (a.t || 0) - (b.t || 0));
+            for (const tab of touchedTabs) tab.hits.sort((a, b) => (a.t || 0) - (b.t || 0));
             S.drumTabDirty = true;
             host.updateArrangementSelector();
             host.draw();

tests/create_drum_materialize.test.js

@@ -54,5 +54,23 @@ t('editorApplyCreateResult calls syncDrumArrangement AFTER assigning S.drumTab',
         'sync must run AFTER the drumTab assignment, so it materializes the live payload');
 });
 
+// GP multi-drum split: several drum tracks arrive as `drum_parts`; the create
+// path must adopt them as extra type:"drums" arrangements beside the primary.
+t('create.js imports adoptDrumParts from the drum-arrangement leaf', () => {
+    assert.ok(/import\s*\{[^}]*\badoptDrumParts\b[^}]*\}\s*from\s*['"]\.\/drum-arrangement\.js['"]/.test(src),
+        'adoptDrumParts must be imported to materialize the EXTRA GP drum parts');
+});
+
+t('editorApplyCreateResult adopts drum_parts AFTER syncDrumArrangement (extras beside the primary)', () => {
+    const body = extractFn('editorApplyCreateResult');
+    const syncAt = body.indexOf('syncDrumArrangement(S)');
+    const adoptAt = body.indexOf('adoptDrumParts(S, data.drum_parts)');
+    assert.ok(adoptAt >= 0, "the path adopts the import's extra drum_parts");
+    assert.ok(adoptAt > syncAt,
+        'adopt must run AFTER the primary is materialized, so the extras append beside it');
+    assert.ok(/Array\.isArray\(data\.drum_parts\)\s*&&\s*data\.drum_parts\.length/.test(body),
+        'the adopt is guarded on data.drum_parts being a non-empty list (a single-drum import never calls it)');
+});
+
 console.log(`\n${pass} passed, ${fail} failed`);
 if (fail) process.exit(1);

tests/drum_arrangement.test.mjs

@@ -28,6 +28,7 @@ import {
     activeDrumArrangementIndex, addDrumArrangement, adoptDrumParts, drumArrangements,
 } from '../src/drum-arrangement.js';
 import { _trackSessionTargetsPure } from '../src/track-session.js';
+import { _drumImportTargetTabPure } from '../src/import.js';
 
 let pass = 0, fail = 0;
 function t(name, fn) {
@@ -273,6 +274,19 @@ t('adoptDrumParts: load-time extras keep their persisted ids and sort their hits
         'the primary stays the active grid target');
 });
 
+t('unmapped GP percussion resolves to the drum part that owned it', () => {
+    const S = { arrangements: [gtr('Lead')], drumTab: tab() };
+    syncDrumArrangement(S);
+    adoptDrumParts(S, [
+        { id: 'aux', name: 'Aux', drum_tab: tab({ name: 'Aux', hits: [] }) },
+    ]);
+    const parts = drumArrangements(S.arrangements);
+    assert.strictEqual(_drumImportTargetTabPure(S, 0), parts[0].drumTab);
+    assert.strictEqual(_drumImportTargetTabPure(S, 1), parts[1].drumTab);
+    assert.strictEqual(_drumImportTargetTabPure(S, 99), S.drumTab,
+        'malformed/legacy part metadata safely falls back to the primary');
+});
+
 t('syncDrumArrangement with SEVERAL parts: a null active tab never mass-deletes', () => {
     const S = { arrangements: [gtr('Lead')], drumTab: tab() };
     syncDrumArrangement(S);

tests/test_gp_multidrum_split.py

@@ -0,0 +1,114 @@
+"""Whole-song GP import keeps SEVERAL drum tracks as separate parts.
+
+Before this, `convert_gp` folded every drum arrangement into ONE `drum_tab`
+(`_drum_arrs_to_drum_tab([all of them])`), so a two-drummer / kit+aux-perc GP
+came in as a single squashed drum part. `_gp_drum_arrs_to_parts` now splits
+them: one tab per drum track, the first as the primary and the rest as
+`drum_parts` (the shape a reload adopts and /build persists) — but only when a
+melodic part exists to sit beside (a drums arrangement never sits at index 0),
+so a drums-only import still folds.
+"""
+import sys
+import types
+
+import pytest
+
+from routes import _gp_drum_arrs_to_parts
+
+KICK, SNARE, HH_CLOSED = 36, 38, 42
+CONGA_HI, TIMBALE = 62, 65  # outside the 18-piece vocab → unmapped
+
+_FAKE_PIECES = {KICK: "kick", SNARE: "snare", HH_CLOSED: "hh_closed"}
+
+
+@pytest.fixture(autouse=True)
+def fake_core_drums(monkeypatch):
+    mod = types.ModuleType("lib.drums")
+    mod.midi_to_piece = lambda midi: _FAKE_PIECES.get(midi)
+    mod.SCHEMA_VERSION = 1
+    pkg = sys.modules.get("lib") or types.ModuleType("lib")
+    monkeypatch.setitem(sys.modules, "lib", pkg)
+    monkeypatch.setitem(sys.modules, "lib.drums", mod)
+
+
+def note(midi, time=0.0):
+    return {"string": midi // 24, "fret": midi % 24, "time": time}
+
+
+def arr(name="Drums", notes=()):
+    return {"name": name, "notes": list(notes), "chords": []}
+
+
+def pieces(tab):
+    return [h["p"] for h in tab["hits"]]
+
+
+def test_several_drum_tracks_with_a_melodic_part_split_into_parts():
+    a1 = arr("Drums", [note(KICK, 0.0), note(SNARE, 0.5)])
+    a2 = arr("Percussion", [note(HH_CLOSED, 0.25)])
+    primary, extras, unmapped = _gp_drum_arrs_to_parts([a1, a2], has_pitched=True)
+    assert primary is not None
+    assert primary["name"] == "Drums" and pieces(primary) == ["kick", "snare"]
+    assert len(extras) == 1
+    assert extras[0]["name"] == "Percussion"
+    assert extras[0]["id"] == "", "id blank → the frontend assigns a fresh one"
+    assert extras[0]["drum_tab"]["name"] == "Percussion"
+    assert pieces(extras[0]["drum_tab"]) == ["hh_closed"]
+    assert not unmapped
+
+
+def test_a_single_drum_track_folds_with_no_extras():
+    primary, extras, _ = _gp_drum_arrs_to_parts([arr("Drums", [note(KICK)])], has_pitched=True)
+    assert pieces(primary) == ["kick"]
+    assert extras == []
+
+
+def test_drums_only_import_folds_even_with_several_tracks():
+    # No melodic part → plural drum parts can't be represented (a drums
+    # arrangement never sits at index 0), so fold — no loss, no split.
+    a1 = arr("Drums", [note(KICK, 0.0)])
+    a2 = arr("Percussion", [note(SNARE, 1.0)])
+    primary, extras, _ = _gp_drum_arrs_to_parts([a1, a2], has_pitched=False)
+    assert extras == []
+    assert pieces(primary) == ["kick", "snare"], "both tracks folded, time-sorted"
+
+
+def test_an_empty_leading_track_is_dropped_primary_is_first_with_hits():
+    a1 = arr("Click", [])                 # no mappable hits
+    a2 = arr("Drums", [note(KICK)])
+    a3 = arr("Perc", [note(SNARE)])
+    primary, extras, _ = _gp_drum_arrs_to_parts([a1, a2, a3], has_pitched=True)
+    assert primary["name"] == "Drums"
+    assert len(extras) == 1 and extras[0]["name"] == "Perc"
+
+
+def test_all_unmapped_keeps_one_empty_tab_for_recovery():
+    a1 = arr("Drums", [note(CONGA_HI, 0.0)])
+    a2 = arr("Perc", [note(TIMBALE, 1.0)])
+    primary, extras, unmapped = _gp_drum_arrs_to_parts([a1, a2], has_pitched=True)
+    assert primary is not None and primary["hits"] == []
+    assert len(extras) == 1 and extras[0]["drum_tab"]["hits"] == []
+    assert set(unmapped) == {(0, CONGA_HI), (1, TIMBALE)}
+
+
+def test_mixed_mapped_and_unmapped_tracks_keep_mapping_ownership():
+    a1 = arr("Drums", [note(KICK, 0.0), note(CONGA_HI, 0.5)])
+    a2 = arr("Aux", [note(TIMBALE, 1.0)])
+    primary, extras, unmapped = _gp_drum_arrs_to_parts([a1, a2], has_pitched=True)
+    assert pieces(primary) == ["kick"]
+    assert len(extras) == 1
+    assert extras[0]["name"] == "Aux"
+    assert extras[0]["drum_tab"]["hits"] == []
+    assert set(unmapped) == {(0, CONGA_HI), (1, TIMBALE)}
+
+
+def test_nothing_mappable_and_nothing_to_remap_returns_none():
+    primary, extras, unmapped = _gp_drum_arrs_to_parts([arr("Drums", [])], has_pitched=True)
+    assert primary is None and extras == [] and not unmapped
+
+
+def test_gp_track_name_is_preserved_and_length_capped():
+    primary, extras, _ = _gp_drum_arrs_to_parts(
+        [arr("D" * 200, [note(KICK)]), arr("Perc", [note(SNARE)])], has_pitched=True)
+    assert primary["name"] == "D" * 120 and len(primary["name"]) == 120
+    assert extras[0]["name"] == "Perc"