From 61f818102323416de4aad8b2b860dc8e91839b60 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Wed, 22 Jul 2026 09:29:11 -0500 Subject: [PATCH 1/2] fix(editor): a MIDI create-import no longer silently loses the drums MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core's `list_midi_tracks` excludes channel-9 (a keys-import of a drum channel yields an empty arrangement), so when you created a project from a multi-track MIDI the drum track never reached the picker and vanished — a full-band MIDI came in with everything except its drums. /import-midi now also returns the channel-9 `drum_tracks` (via the existing `list_drum_tracks`), and the create flow auto-imports each as its own type:"drums" arrangement, mirroring the auto-split the Guitar Pro create-import already does. Drums ride a SEPARATE list from the pitched picker, so the existing pitched selection/import path is untouched (no row-id or index collision); a drums-only MIDI still imports as a drums session. /import-drums-midi now accepts the create-flow upload path (slopsmith_midi_) as well as the Add-Drums list path, and the drum-tab stash step (sort hits, add-as-extra-or-primary, materialize beside a pitched part, land as a region) is factored into a shared _stashImportedDrumTab that the Add-Drums modal now reuses too. Tests: tests/midi_drum_autosplit.test.mjs pins the stateful wiring (per-track routing, request params, empty/failed-track resilience, guards, and the shared stash). Full pytest green (381); lint clean; the only JS reds are the two known-on-main failures (mixer_meter_teardown, song_fit). Runtime-verified end-to-end on the real host: a synthesized MIDI with a ch9 drums track surfaces in drum_tracks and converts to a 6-hit drum_tab through the create-flow path. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix Signed-off-by: ChrisBeWithYou --- CHANGELOG.md | 14 +++ routes.py | 24 +++- src/arrangement.js | 99 ++++++++++++---- src/create.js | 28 ++++- tests/midi_drum_autosplit.test.mjs | 183 +++++++++++++++++++++++++++++ 5 files changed, 317 insertions(+), 31 deletions(-) create mode 100644 tests/midi_drum_autosplit.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 70462d6..8c2cff3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Creating a project from a multi-track MIDI no longer silently loses the + drums.** Core's `list_midi_tracks` excludes channel-9 (a keys-import of a + drum channel is empty), so a full-band MIDI's drum track never reached the + create picker and vanished. `/import-midi` now also returns the channel-9 + `drum_tracks` (via `list_drum_tracks`), and the create flow auto-imports each + as its own `type:"drums"` arrangement — the same auto-split the Guitar Pro + create-import already does — so drums arrive alongside the pitched parts + instead of disappearing. The pitched picker and its import are unchanged + (drums ride a separate list, so no picker-row/selection collision); a + drums-only MIDI still imports as a drums session. `/import-drums-midi` now + 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. + - **A drum edit no longer strips another tool's authored keys from drum-part manifest entries.** Saving with drum changes rebuilds the `type: drums` arrangement entries; that rebuild now merges onto the prior same-id entry — diff --git a/routes.py b/routes.py index a3f0f65..1cd4b08 100644 --- a/routes.py +++ b/routes.py @@ -7169,7 +7169,7 @@ async def parse_goplayalong_sync(file: UploadFile = File(...)): @app.post("/api/plugins/editor/import-midi") async def import_midi(file: UploadFile = File(...)): """Upload a MIDI file and return track listing.""" - from lib.midi_import import list_midi_tracks + from lib.midi_import import list_midi_tracks, list_drum_tracks # Validate extension — the browser accept filter is advisory only. orig_suffix = Path(file.filename or "").suffix.lower() @@ -7201,13 +7201,25 @@ async def import_midi(file: UploadFile = File(...)): def _list(): return list_midi_tracks(midi_path) + # Core's `list_midi_tracks` EXCLUDES channel-9 (GM percussion) — a + # keys-import of a drum channel would yield an empty arrangement, so + # drums are dropped from the pitched listing. List them separately so + # the create picker can auto-split them into `type:"drums"` parts + # instead of silently losing a full-band MIDI's drums. + def _list_drums(): + try: + return list_drum_tracks(midi_path) + except Exception: + return [] + try: tracks = await asyncio.get_event_loop().run_in_executor(None, _list) + drum_tracks = await asyncio.get_event_loop().run_in_executor(None, _list_drums) except Exception as e: shutil.rmtree(tmp, ignore_errors=True) return JSONResponse({"error": f"Failed to parse MIDI file: {e}"}, 500) - return {"midi_path": midi_path, "tracks": tracks} + return {"midi_path": midi_path, "tracks": tracks, "drum_tracks": drum_tracks} # ── MIDI import: convert a track to a Keys arrangement ──────────── @@ -8459,7 +8471,13 @@ async def import_drums_midi(data: dict): except (TypeError, ValueError): return JSONResponse({"error": "audio_offset must be a number"}, 400) - validated = _validate_editor_upload_path(midi_path_raw, "slopsmith_drums_midi_") + # Accept both the Add-Drums list upload (slopsmith_drums_midi_) and the + # generic create-flow MIDI upload (slopsmith_midi_): the create picker's + # auto drum-split reuses the file it already staged via /import-midi. + validated = ( + _validate_editor_upload_path(midi_path_raw, "slopsmith_drums_midi_") + or _validate_editor_upload_path(midi_path_raw, "slopsmith_midi_") + ) if not validated: return JSONResponse({"error": "MIDI file not found"}, 400) midi_path = str(validated) diff --git a/src/arrangement.js b/src/arrangement.js index 27ab15e..910b524 100644 --- a/src/arrangement.js +++ b/src/arrangement.js @@ -456,6 +456,74 @@ export async function editorDrumsFileSelected(input) { // scaffolds might still wire it up. Forwards to the new dispatcher. export function editorDrumsGPSelected(input) { return editorDrumsFileSelected(input); } +// Shared "stash a freshly-imported drum tab onto the session" step, used by +// both the Add-Drums modal and the create-flow MIDI auto-split. Sorts hits, +// adds a NEW type:"drums" arrangement when a drums part already exists (else +// this tab becomes the primary), re-points S.drumTab at it, materializes it +// beside a pitched part (drums never sit at index 0), and lands it as a +// selected region at `placeAt` ('keep' = source timing). Returns whether it +// was ADDED as an extra part (vs. becoming the first/primary tab). +export function _stashImportedDrumTab(tab, placeAt = 'keep') { + if (tab && Array.isArray(tab.hits)) { + tab.hits.sort((a, b) => (a.t || 0) - (b.t || 0)); + } + const added = _canAddAnotherDrums(); + if (added) addDrumArrangement(S, tab); // its own type:"drums" arrangement + S.drumTab = tab; // the imported part is now the grid target + S.drumTabDirty = true; // user-imported — persist on next save + S.drumSel = new Set(); + // Reflect the imported tab in S.arrangements[] — beside a pitched part + // only (a drums-only session must not put drums at index 0). + if (pitchedArrangementCount(S.arrangements) > 0) syncDrumArrangement(S); + host.placeImportedPartAsRegion({ + kind: 'drums', + arrIdx: activeDrumArrangementIndex(S.arrangements, tab), + placeAt, + items: Array.isArray(tab.hits) ? tab.hits.slice() : [], + }); + return added; +} + +// Create-window MIDI auto-split. Core's `list_midi_tracks` drops channel-9, so +// the pitched picker never sees a MIDI's drums; /import-midi now also returns +// the channel-9 `drum_tracks`, and this imports each as its own type:"drums" +// part so a full-band MIDI keeps its drums instead of silently losing them +// (mirrors the GP create-import auto-extract). Must run AFTER the pitched +// import so each drum part has a pitched sibling to materialize beside. A bad +// track is skipped, not fatal. Returns how many parts were imported. +export async function importMidiDrumTracksIntoSession(midiPath, drumTracks, statusEl = null) { + if (!midiPath || !S.sessionId || !Array.isArray(drumTracks) || !drumTracks.length) return 0; + let imported = 0; + for (const t of drumTracks) { + if (!t || Number(t.notes) <= 0) continue; + try { + const resp = await fetch('/api/plugins/editor/import-drums-midi', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + midi_path: midiPath, + track_index: Number(t.index) || 0, + audio_offset: host.effectiveAudioOffset(), + arrangement_name: t.name || 'Drums', + }), + }); + const data = await resp.json(); + if (data.error || !data.drum_tab) continue; + _stashImportedDrumTab(data.drum_tab, 'keep'); + imported++; + } catch (_) { /* skip this track, keep importing the rest */ } + } + if (imported > 0) { + host.updateArrangementSelector(); + host.draw(); + if (statusEl) { + const base = statusEl.textContent ? statusEl.textContent.trim() + ' ' : ''; + statusEl.textContent = `${base}+ ${imported} drum part${imported === 1 ? '' : 's'} imported.`; + } + } + return imported; +} + export async function editorDoAddDrums() { if (!_addDrumsFile || !S.sessionId) return; @@ -495,32 +563,15 @@ export async function editorDoAddDrums() { // any stale selection so indices from the old tab don't point into // the new hits array. When drums already exist (saved sloppak), the // import ADDS another drum part; create mode still replaces. + // Stash the imported tab: sort hits, add it as an extra part (or make + // it the primary), materialize beside a pitched part, land as a region. + // The dialog's Place-at row picks where it lands; create mode forces + // 'keep' (its Place-at row is hidden — placement is deferred until the + // create build path proves it round-trips regions[]). const tab = data.drum_tab; - if (tab && Array.isArray(tab.hits)) { - tab.hits.sort((a, b) => (a.t || 0) - (b.t || 0)); - } - const added = _canAddAnotherDrums(); - if (added) addDrumArrangement(S, tab); // its own type:"drums" arrangement - S.drumTab = tab; // the imported part is now the grid target - S.drumTabDirty = true; // user-imported — persist on next save - S.drumSel = new Set(); - // Reflect the imported tab in S.arrangements[] — beside a pitched - // part only (a drums-only session must not put drums at index 0). - if (pitchedArrangementCount(S.arrangements) > 0) syncDrumArrangement(S); - - // Land the fresh part in the Tracks view as a selected region — placed - // at bar 1 / the playhead when the dialog said so, else left at the - // source file's own timing (R3b import-into-existing). Create mode - // forces 'keep' (its Place-at row is hidden — placement is deferred - // until the create build path proves it round-trips regions[]), so - // there the import lands selected at source timing. const placeSel = document.getElementById('editor-add-drums-place'); - host.placeImportedPartAsRegion({ - kind: 'drums', - arrIdx: activeDrumArrangementIndex(S.arrangements, tab), - placeAt: (placeSel && !S.createMode) ? placeSel.value : 'keep', - items: Array.isArray(tab.hits) ? tab.hits.slice() : [], - }); + const added = _stashImportedDrumTab( + tab, (placeSel && !S.createMode) ? placeSel.value : 'keep'); editorHideAddDrumsModal(); const hitCount = Array.isArray(data.drum_tab.hits) diff --git a/src/create.js b/src/create.js index 7cb3265..8aa1e31 100644 --- a/src/create.js +++ b/src/create.js @@ -25,6 +25,7 @@ import { } from './annotation-lanes.js'; import { _handshapesAreDirty, flattenChords, reconstructChords } from './chords.js'; import { isDrumArrangement, pitchedArrangementCount, syncDrumArrangement } from './drum-arrangement.js'; +import { importMidiDrumTracksIntoSession } from './arrangement.js'; import { EditHistory } from './history.js'; import { host } from './host.js'; import { isKeysMode, updatePianoRange } from './keys.js'; @@ -149,7 +150,7 @@ export function editorShowCreateModal() { musicXmlFile: null, musicXmlData: null, musicXmlName: null, audioUrl: null, audioName: null, audioDuration: null, audioFile: null, audioTracks: [], guideTrackId: '', youtubeSelected: true, - midiInfo: null, midiFiles: null, midiPath: null, midiTracks: [], + midiInfo: null, midiFiles: null, midiPath: null, midiTracks: [], midiDrumTracks: [], artPath: null, previewPath: null, gp8AudioMode: 'none', autoSyncAudioUrl: null, lastSync: null, autoSyncCoupled: false, // GoPlayAlong sync sidecar (goplayalong.com): a .xml that carries @@ -1083,6 +1084,7 @@ async function _stageMidi(files) { createState.midiFiles = [file]; createState.midiPath = null; createState.midiTracks = []; + createState.midiDrumTracks = []; try { const form = new FormData(); form.append('file', file); @@ -1094,6 +1096,13 @@ async function _stageMidi(files) { ...track, selected: Number(track.notes) > 0, })); + // Channel-9 drums arrive on a separate list — list_midi_tracks omits + // them (a keys-import of channel 9 is empty), so the backend lists them + // via list_drum_tracks; auto-imported as type:"drums" parts on Create. + createState.midiDrumTracks = (data.drum_tracks || []).map(track => ({ + ...track, + selected: Number(track.notes) > 0, + })); } catch (error) { const iStatus = document.getElementById('editor-create-import-status'); if (iStatus) iStatus.textContent = 'MIDI read failed: ' + error.message; @@ -1185,6 +1194,7 @@ export function editorStagedRemove(role) { createState.midiFiles = null; createState.midiPath = null; createState.midiTracks = []; + createState.midiDrumTracks = []; } const iStatus = document.getElementById('editor-create-import-status'); if (iStatus) iStatus.textContent = ''; @@ -1997,9 +2007,19 @@ async function _editorDoMidiCreate() { // tag a later import on a DIFFERENT song would delete that song's arrangement 0. if (seeded) { S._midiSeedArrIdx = 0; S._midiSeedSession = S.sessionId; } const picked = (createState.midiTracks || []).filter(track => track.selected !== false && Number(track.notes) > 0); - if (createState.midiPath && picked.length) { - await importMidiTracksIntoSession(createState.midiPath, picked, - document.getElementById('editor-create-status')); + // Core's list_midi_tracks drops channel-9, so drums arrive on a separate + // list — import them alongside the pitched picks so a full-band MIDI keeps + // its drums instead of silently losing them. + const drumPicked = (createState.midiDrumTracks || []).filter(track => track.selected !== false && Number(track.notes) > 0); + const midiStatusEl = document.getElementById('editor-create-status'); + if (createState.midiPath && (picked.length || drumPicked.length)) { + // Pitched first so each drum part has a pitched sibling to sit beside. + if (picked.length) { + await importMidiTracksIntoSession(createState.midiPath, picked, midiStatusEl); + } + if (drumPicked.length) { + await importMidiDrumTracksIntoSession(createState.midiPath, drumPicked, midiStatusEl); + } } else if (typeof window.editorShowAddKeysModal === 'function' && typeof window._editorKeysHandleFile === 'function') { // Compatibility fallback for a core that could not list the MIDI at diff --git a/tests/midi_drum_autosplit.test.mjs b/tests/midi_drum_autosplit.test.mjs new file mode 100644 index 0000000..71c4190 --- /dev/null +++ b/tests/midi_drum_autosplit.test.mjs @@ -0,0 +1,183 @@ +/* + * MIDI create-import drum auto-split — the stateful wiring, not a pure helper. + * + * Core's `list_midi_tracks` EXCLUDES channel-9, so a full-band MIDI's drums + * never reach the pitched picker and were silently lost on create. /import-midi + * now also returns the channel-9 `drum_tracks`, and `importMidiDrumTracksIntoSession` + * imports each as its own `type:"drums"` arrangement (via the shared + * `_stashImportedDrumTab` step editorDoAddDrums also uses). + * + * Pinned here (all fail on main, where neither function exists): + * - _stashImportedDrumTab sorts hits, marks the tab dirty, clears the drum + * selection, materializes a type:"drums" arrangement beside a pitched part, + * and reports add-as-extra vs. become-primary; + * - importMidiDrumTracksIntoSession POSTs each drum track to import-drums-midi + * with the right track_index / arrangement_name, materializes one drums + * part each, skips empty/failed tracks without aborting the rest, and guards + * no-session / no-path / empty-list. + * + * Run: node tests/midi_drum_autosplit.test.mjs + */ +import assert from 'node:assert'; + +// Minimal DOM (collaborator surface only — no subject under test is stubbed). +const _els = new Map(); +function _mkEl() { + return { + textContent: '', innerHTML: '', value: '', disabled: false, style: {}, + classList: { add() {}, remove() {}, toggle() {}, contains: () => false }, + appendChild() {}, addEventListener() {}, removeEventListener() {}, + querySelector: () => null, querySelectorAll: () => [], setAttribute() {}, getAttribute: () => null, + }; +} +globalThis.document = { + getElementById(id) { if (!_els.has(id)) _els.set(id, _mkEl()); return _els.get(id); }, + createElement: () => _mkEl(), querySelectorAll: () => [], addEventListener() {}, removeEventListener() {}, +}; +globalThis.window = globalThis.window || globalThis; + +const { S } = await import('../src/state.js'); +const { setHostHooks } = await import('../src/host.js'); +const { isDrumArrangement, drumArrangements } = await import('../src/drum-arrangement.js'); +const { _stashImportedDrumTab, importMidiDrumTracksIntoSession } = await import('../src/arrangement.js'); + +// Inert host hooks the drum stash / import reach for (defaults are type-honest, +// but pin them so a placement/redraw can never throw in the headless run). +setHostHooks({ + effectiveAudioOffset: () => 0, + placeImportedPartAsRegion: () => true, + updateArrangementSelector: () => {}, + draw: () => {}, +}); + +let pass = 0, fail = 0; +async function ta(name, fn) { + try { await fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} +function t(name, fn) { + try { fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} + +const gtr = (name = 'Lead') => ({ id: name.toLowerCase(), name, notes: [], chords: [] }); +const drumTab = (name, hits) => ({ version: 1, name, kit: [], hits }); + +// Fresh drums session with a single pitched part so drums can materialize +// (a drums arrangement is never index 0). +function seed(extra = {}) { + Object.assign(S, { + filename: 'autosplit.sloppak', format: 'sloppak', sessionId: 'sess-drums', + arrangements: [gtr('Lead')], currentArr: 0, + drumTab: null, drumTabDirty: false, drumSel: new Set(['stale']), + ...extra, + }); +} + +// Route import-drums-midi to a FRESH tab per call (the real endpoint does), so +// the multi-part path never collapses on a shared object reference. +function installDrumFetch() { + const log = []; + let n = 0; + globalThis.fetch = async (url, opts = {}) => { + log.push({ url: String(url), body: opts.body ? JSON.parse(opts.body) : null }); + if (String(url).includes('import-drums-midi')) { + n += 1; + return { ok: true, status: 200, json: async () => ({ + drum_tab: drumTab(`Drums ${n}`, [{ t: 0, p: 'kick' }]), + }) }; + } + throw new Error('unstubbed fetch: ' + url); + }; + return log; +} + +// ── _stashImportedDrumTab — the shared materialization step ────────────────── + +t('_stashImportedDrumTab: sorts hits, marks dirty, clears sel, materializes a drums part, first is primary', () => { + seed(); + const added = _stashImportedDrumTab(drumTab('Kit', [{ t: 2, p: 'kick' }, { t: 0, p: 'snare' }, { t: 1, p: 'hat' }])); + assert.strictEqual(added, false, 'the first drum part is the PRIMARY, not an add'); + assert.strictEqual(S.drumTab.name, 'Kit', 'S.drumTab points at the imported tab'); + assert.deepStrictEqual(S.drumTab.hits.map(h => h.t), [0, 1, 2], 'hits are time-sorted'); + assert.strictEqual(S.drumTabDirty, true, 'user-imported → dirty for the next save'); + assert.strictEqual(S.drumSel.size, 0, 'stale drum selection cleared (indices point into the old tab)'); + assert.strictEqual(drumArrangements(S.arrangements).length, 1, 'materialized one type:"drums" arrangement'); + assert.ok(isDrumArrangement(S.arrangements.at(-1)), 'appended last, typed drums'); + assert.strictEqual(S.arrangements[0].name, 'Lead', 'the pitched part is untouched'); +}); + +t('_stashImportedDrumTab: a second call ADDS a second drum part (a song can hold several)', () => { + seed(); + _stashImportedDrumTab(drumTab('Kit A', [{ t: 0, p: 'kick' }])); + const added2 = _stashImportedDrumTab(drumTab('Kit B', [{ t: 0, p: 'snare' }])); + assert.strictEqual(added2, true, 'the second drum part is ADDED, not a replace'); + assert.strictEqual(drumArrangements(S.arrangements).length, 2, 'two type:"drums" arrangements now'); + assert.strictEqual(S.drumTab.name, 'Kit B', 'the freshest part is the active grid target'); +}); + +// ── importMidiDrumTracksIntoSession — the create-flow routing ──────────────── + +await ta('imports each channel-9 drum track as its own drums part, with the right request', async () => { + seed(); + const log = installDrumFetch(); + const n = await importMidiDrumTracksIntoSession('/tmp/slopsmith_midi_x/upload.mid', [ + { index: 9, name: 'Drums', notes: 40 }, + { index: 12, name: 'Aux Perc', notes: 8 }, + ]); + assert.strictEqual(n, 2, 'both drum tracks imported'); + assert.strictEqual(drumArrangements(S.arrangements).length, 2, 'two type:"drums" arrangements materialized'); + const reqs = log.filter(r => r.url.includes('import-drums-midi')); + assert.strictEqual(reqs.length, 2, 'one import-drums-midi call per drum track'); + assert.strictEqual(reqs[0].body.track_index, 9, 'track_index from the drum track'); + assert.strictEqual(reqs[0].body.arrangement_name, 'Drums', 'the track name rides as the arrangement name'); + assert.strictEqual(reqs[1].body.track_index, 12); + assert.strictEqual(reqs[1].body.arrangement_name, 'Aux Perc'); + assert.strictEqual(reqs[0].body.midi_path, '/tmp/slopsmith_midi_x/upload.mid', 'the staged create-flow path is reused'); +}); + +await ta('skips a note-less drum track without calling the backend', async () => { + seed(); + const log = installDrumFetch(); + const n = await importMidiDrumTracksIntoSession('/tmp/slopsmith_midi_x/upload.mid', [ + { index: 9, name: 'Empty', notes: 0 }, + { index: 10, name: 'Real', notes: 5 }, + ]); + assert.strictEqual(n, 1, 'only the note-bearing track imported'); + const reqs = log.filter(r => r.url.includes('import-drums-midi')); + assert.strictEqual(reqs.length, 1, 'the empty track was never fetched'); + assert.strictEqual(reqs[0].body.track_index, 10); +}); + +await ta('one failing track does not abort the rest', async () => { + seed(); + let call = 0; + globalThis.fetch = async (url) => { + call += 1; + if (String(url).includes('import-drums-midi')) { + if (call === 1) return { ok: true, status: 200, json: async () => ({ error: 'bad track' }) }; + return { ok: true, status: 200, json: async () => ({ drum_tab: drumTab('Kit', [{ t: 0, p: 'kick' }]) }) }; + } + throw new Error('unstubbed fetch: ' + url); + }; + const n = await importMidiDrumTracksIntoSession('/tmp/slopsmith_midi_x/upload.mid', [ + { index: 9, name: 'Broken', notes: 3 }, + { index: 10, name: 'Good', notes: 3 }, + ]); + assert.strictEqual(n, 1, 'the good track still imported after the bad one errored'); + assert.strictEqual(drumArrangements(S.arrangements).length, 1); +}); + +await ta('guards: no session / no path / empty list import nothing and never fetch', async () => { + let fetched = 0; + globalThis.fetch = async () => { fetched += 1; throw new Error('should not fetch'); }; + seed({ sessionId: null }); + assert.strictEqual(await importMidiDrumTracksIntoSession('/tmp/x.mid', [{ index: 9, notes: 4 }]), 0, 'no session → 0'); + seed(); + assert.strictEqual(await importMidiDrumTracksIntoSession('', [{ index: 9, notes: 4 }]), 0, 'no path → 0'); + assert.strictEqual(await importMidiDrumTracksIntoSession('/tmp/x.mid', []), 0, 'empty list → 0'); + assert.strictEqual(fetched, 0, 'no backend call on any guarded path'); +}); + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); From a3f0a7e026d712ab3eae7cb2039befd72ef5d764 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Wed, 22 Jul 2026 17:52:14 -0500 Subject: [PATCH 2/2] Keep staged MIDI through drum auto-split --- routes.py | 32 ++++++++++++++++-------------- src/arrangement.js | 6 ++++-- src/create.js | 4 +++- src/import.js | 3 ++- tests/midi_drum_autosplit.test.mjs | 5 +++++ 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/routes.py b/routes.py index 1cd4b08..6a7ad98 100644 --- a/routes.py +++ b/routes.py @@ -7338,13 +7338,14 @@ def _convert_one(track_index, channel_filter): traceback.print_exc() return JSONResponse({"error": str(e)}, 500) - # Clean up the MIDI temp dir now that EVERY conversion is complete — - # the client no longer needs to reference midi_path after this response. - try: - shutil.rmtree(Path(midi_path).parent) - except OSError as _cleanup_err: - import warnings - warnings.warn(f"Could not clean up MIDI temp dir: {_cleanup_err}") + # A create flow that also selected drums reuses this staged file for + # the following drum conversions. Its last consumer owns cleanup. + if not data.get("keep_upload"): + try: + shutil.rmtree(Path(midi_path).parent) + except OSError as _cleanup_err: + import warnings + warnings.warn(f"Could not clean up MIDI temp dir: {_cleanup_err}") # Legacy single-track callers read `arrangement`; the batch form reads # `arrangements` (same order as the request's `tracks`). @@ -8532,14 +8533,15 @@ def _convert(): # swept by the opportunistic TTL cleanup, same as import_keys_midi. return JSONResponse({"error": str(e)}, 500) - # Clean up the MIDI temp dir now that conversion is complete — mirrors - # import_keys_midi which also rmtrees after a successful conversion so - # temp dirs don't accumulate between TTL cleanup runs. - try: - shutil.rmtree(_midi_tmp_dir) - except OSError as _cleanup_err: - import warnings - warnings.warn(f"Could not clean up drums MIDI temp dir: {_cleanup_err}") + # Multi-track create import reuses one staged file. Earlier conversions + # retain it; the final request removes it. Add-Drums remains the single + # consumer and therefore keeps the default cleanup behavior. + if not data.get("keep_upload"): + try: + shutil.rmtree(_midi_tmp_dir) + except OSError as _cleanup_err: + import warnings + warnings.warn(f"Could not clean up drums MIDI temp dir: {_cleanup_err}") # Defensive .get() reads — if a future converter shape ever leaves # `count` or `times` partially populated, return zero/empty rather diff --git a/src/arrangement.js b/src/arrangement.js index 910b524..e8402d4 100644 --- a/src/arrangement.js +++ b/src/arrangement.js @@ -493,9 +493,10 @@ export function _stashImportedDrumTab(tab, placeAt = 'keep') { // track is skipped, not fatal. Returns how many parts were imported. export async function importMidiDrumTracksIntoSession(midiPath, drumTracks, statusEl = null) { if (!midiPath || !S.sessionId || !Array.isArray(drumTracks) || !drumTracks.length) return 0; + const picked = drumTracks.filter(t => t && Number(t.notes) > 0); let imported = 0; - for (const t of drumTracks) { - if (!t || Number(t.notes) <= 0) continue; + for (let i = 0; i < picked.length; i++) { + const t = picked[i]; try { const resp = await fetch('/api/plugins/editor/import-drums-midi', { method: 'POST', @@ -505,6 +506,7 @@ export async function importMidiDrumTracksIntoSession(midiPath, drumTracks, stat track_index: Number(t.index) || 0, audio_offset: host.effectiveAudioOffset(), arrangement_name: t.name || 'Drums', + keep_upload: i < picked.length - 1, }), }); const data = await resp.json(); diff --git a/src/create.js b/src/create.js index 8aa1e31..a60c5e0 100644 --- a/src/create.js +++ b/src/create.js @@ -2015,7 +2015,9 @@ async function _editorDoMidiCreate() { if (createState.midiPath && (picked.length || drumPicked.length)) { // Pitched first so each drum part has a pitched sibling to sit beside. if (picked.length) { - await importMidiTracksIntoSession(createState.midiPath, picked, midiStatusEl); + await importMidiTracksIntoSession(createState.midiPath, picked, midiStatusEl, { + keepUpload: drumPicked.length > 0, + }); } if (drumPicked.length) { await importMidiDrumTracksIntoSession(createState.midiPath, drumPicked, midiStatusEl); diff --git a/src/import.js b/src/import.js index ab7792c..8fc16cf 100644 --- a/src/import.js +++ b/src/import.js @@ -393,7 +393,7 @@ async function _maybeRemoveMidiSeed() { // Create-window MIDI path: the unified table already listed and selected the // file's tracks, so import those exact rows without reopening a second picker. -export async function importMidiTracksIntoSession(midiPath, pickedList, statusEl = null) { +export async function importMidiTracksIntoSession(midiPath, pickedList, statusEl = null, opts = {}) { if (!midiPath || !S.sessionId || !Array.isArray(pickedList) || !pickedList.length) return false; if (statusEl) statusEl.textContent = 'Importing selected MIDI tracks…'; try { @@ -403,6 +403,7 @@ export async function importMidiTracksIntoSession(midiPath, pickedList, statusEl body: JSON.stringify({ midi_path: midiPath, audio_offset: host.effectiveAudioOffset(), + keep_upload: opts.keepUpload === true, tracks: pickedList.map(track => ({ index: Number(track.index) || 0, channel_filter: track.channel_filter == null ? null : Number(track.channel_filter), diff --git a/tests/midi_drum_autosplit.test.mjs b/tests/midi_drum_autosplit.test.mjs index 71c4190..7293d91 100644 --- a/tests/midi_drum_autosplit.test.mjs +++ b/tests/midi_drum_autosplit.test.mjs @@ -134,6 +134,10 @@ await ta('imports each channel-9 drum track as its own drums part, with the righ assert.strictEqual(reqs[1].body.track_index, 12); assert.strictEqual(reqs[1].body.arrangement_name, 'Aux Perc'); assert.strictEqual(reqs[0].body.midi_path, '/tmp/slopsmith_midi_x/upload.mid', 'the staged create-flow path is reused'); + assert.strictEqual(reqs[0].body.keep_upload, true, + 'the staged MIDI survives until every selected drum track is converted'); + assert.strictEqual(reqs[1].body.keep_upload, false, + 'the final drum conversion owns temp-file cleanup'); }); await ta('skips a note-less drum track without calling the backend', async () => { @@ -147,6 +151,7 @@ await ta('skips a note-less drum track without calling the backend', async () => const reqs = log.filter(r => r.url.includes('import-drums-midi')); assert.strictEqual(reqs.length, 1, 'the empty track was never fetched'); assert.strictEqual(reqs[0].body.track_index, 10); + assert.strictEqual(reqs[0].body.keep_upload, false, 'the only real track cleans up'); }); await ta('one failing track does not abort the rest', async () => {