From 57e5c385a6c80b75cb3ee03b96ba0abbb3fabbe2 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Tue, 21 Jul 2026 08:24:59 -0500 Subject: [PATCH 1/7] =?UTF-8?q?feat(editor):=20track=20regions=20=E2=80=94?= =?UTF-8?q?=20place=20+=20delete=20commands=20(the=20import-region=20found?= =?UTF-8?q?ation)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PlaceRegionCmd drops freshly-added track content onto the timeline as a BOUNDED, selected, draggable block: it slides the content so its first onset lands on the snapped startBeat (a musical move — beats preserved on a varying grid, mirroring MoveRegionCmd's remap with the constant-tempo fast path) and writes an explicit-length region covering it. Even a bar-1 placement stays bounded — never the implicit default — so the block is a distinct addressable object from birth. DeleteRegionCmd removes exactly the notes/hits a region's end-exclusive window owns plus the region entry, leaving neighbours untouched. Both round-trip: rollback restores a verbatim content snapshot (the beatOf∘timeOf round trip is not bit-reversible), the track's raw `regions` key (including deleting one that was never there), and the prior selection. Node-runnable with no DOM, like the move command. _nextRegionIdPure allocates the next free region:N, counting the implicit default region:1 even when unpersisted so a placed region can never collide with it. This is the command layer for "Add Track from File → placed region" (TRACK-REGIONS-DESIGN.md PR 3); the import front door wires in next. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix --- src/region-commands.js | 203 +++++++++++++++++++++++- src/region.js | 14 ++ tests/region_place_delete.test.mjs | 246 +++++++++++++++++++++++++++++ 3 files changed, 462 insertions(+), 1 deletion(-) create mode 100644 tests/region_place_delete.test.mjs diff --git a/src/region-commands.js b/src/region-commands.js index f4e480b4..e02d0557 100644 --- a/src/region-commands.js +++ b/src/region-commands.js @@ -22,10 +22,47 @@ // ════════════════════════════════════════════════════════════════════ import { beatOf, timeOf } from './beats.js'; import { - _regionContainsBeatPure, _regionRemapPure, _trackRegionsNormalizePure, _trackRegionsResolvePure, + _nextRegionIdPure, _regionContainsBeatPure, _regionRemapPure, + _trackRegionsNormalizePure, _trackRegionsResolvePure, } from './region.js'; import { S } from './state.js'; +// A placed region's window must strictly CONTAIN the last onset it owns +// (membership is [startBeat, startBeat+lenBeat) — end-exclusive). When every +// contained note has zero sustain the content end equals the last onset, so the +// naive span would put that onset exactly on the excluded edge; pad the length +// by this sub-cent-of-a-beat guard so the region keeps owning its own notes. +const REGION_LEN_GUARD = 1e-4; + +// ── Content/track access shared by the placement commands ───────────── +// (MoveRegionCmd predates these and keeps its own copies as methods; leaving it +// untouched avoids churning shipped, tested code.) +function _contentList(kind, arrIdx) { + if (kind === 'drums') { + return (S.drumTab && Array.isArray(S.drumTab.hits)) ? S.drumTab.hits : null; + } + const arr = S.arrangements && S.arrangements[arrIdx]; + return arr && Array.isArray(arr.notes) ? arr.notes : null; +} +function _timeOfItem(kind, item) { return kind === 'drums' ? item.t : item.time; } +function _findTrack(trackId) { + const tracks = S.trackSession && Array.isArray(S.trackSession.tracks) ? S.trackSession.tracks : null; + return tracks ? tracks.find(t => t && t.id === trackId) : null; +} +// Snapshot a track's raw `regions` (absent or an array) so rollback restores it +// EXACTLY — including deleting a key that was never there. +function _snapRegions(track) { + return { + taken: true, + hadKey: Object.prototype.hasOwnProperty.call(track, 'regions'), + value: track.regions, + }; +} +function _restoreRegions(track, snap) { + if (!snap.taken || !track) return; + if (snap.hadKey) track.regions = snap.value; else delete track.regions; +} + export class MoveRegionCmd { // `kind`: 'notation' shifts S.arrangements[arrIdx].notes; 'drums' shifts // S.drumTab.hits. `region` is the resolved region object being moved (its id @@ -155,3 +192,167 @@ export class MoveRegionCmd { else delete track.regions; } } + +// ════════════════════════════════════════════════════════════════════ +// PlaceRegionCmd — drop freshly-added track content onto the timeline as a +// BOUNDED, selectable, draggable block (the "Add Track from File" driver). +// +// The import adopts the new part at its source timing (typically beat 0); this +// command then, as ONE undoable edit: (1) slides all of that content so its +// first onset lands on `startBeat` (a MUSICAL move — mirrors MoveRegionCmd, so +// a varying grid preserves beats, not seconds), and (2) writes a bounded region +// covering it onto the track and selects it. Placing at bar 1 (startBeat 0) +// still yields a bounded region (explicit lenBeat) — never the implicit default +// — so the block is a distinct, addressable object you can drag/delete, unlike +// a whole-track default slide. +// +// Undo restores a verbatim content snapshot (the beat round trip isn't bit- +// reversible), the track's raw `regions` (incl. deleting a key that wasn't +// there), and the prior selection. Browser surface: NONE (node-runnable). +// ════════════════════════════════════════════════════════════════════ +export class PlaceRegionCmd { + // `kind`: 'notation' places S.arrangements[arrIdx].notes; 'drums' places + // S.drumTab.hits. `startBeat` is the snapped bar/beat the block lands on. + // `regionId`/`name` are optional (id defaults to the track's next free one). + constructor({ kind, arrIdx, trackId, startBeat, regionId, name } = {}) { + this.kind = kind; + this.arrIdx = arrIdx; + this.trackId = trackId; + this.startBeat = Math.max(0, Number(startBeat) || 0); + this.regionId = (typeof regionId === 'string' && regionId.trim()) ? regionId.trim() : null; + this.name = (typeof name === 'string' && name.trim()) ? name.trim().slice(0, 120) : null; + // A placement changes only WHEN content plays, never pitch — passes the + // read-only-roll lock like MoveRegionCmd. Drum content is song-level. + this.pitchPreserving = true; + this.songScope = kind === 'drums'; + this._before = null; // ref-order snapshot of the content array + this._snap = null; // [{ item, time, sustain }] verbatim pre-move + this._regionBefore = { taken: false, hadKey: false, value: undefined }; + this._sel = { taken: false, track: '', region: '' }; + this._placedId = ''; + } + + exec() { + const list = _contentList(this.kind, this.arrIdx); + if (!list || !list.length) return; // nothing to place + this._before = list.slice(); + const isDrums = this.kind === 'drums'; + const times = list.map(it => Number(_timeOfItem(this.kind, it)) || 0); + const sustains = list.map(it => (isDrums ? 0 : Math.max(0, Number(it.sustain) || 0))); + // Beat extent of the content as imported (independent of the shift — a + // musical move preserves beat span, so lenBeat is computed once here). + let minBeat = Infinity; let maxOnset = -Infinity; let maxEnd = -Infinity; + for (let i = 0; i < times.length; i++) { + const ob = beatOf(S.beats, times[i]); + const eb = sustains[i] > 0 ? beatOf(S.beats, times[i] + sustains[i]) : ob; + if (ob < minBeat) minBeat = ob; + if (ob > maxOnset) maxOnset = ob; + if (eb > maxEnd) maxEnd = eb; + } + if (!Number.isFinite(minBeat)) return; + const dBeat = this.startBeat - minBeat; + this._snap = list.map((it, i) => ({ item: it, time: times[i], sustain: sustains[i] })); + // Slide the content (skip the beat round trip when it wouldn't move — + // routing dBeat 0 through beats perturbs every note by an epsilon). + if (dBeat !== 0) { + const { times: nt, sustains: ns } = _regionRemapPure(times, sustains, dBeat, S.beats, beatOf, timeOf); + if (isDrums) { + list.forEach((h, i) => { h.t = nt[i]; }); + list.sort((a, b) => (a.t || 0) - (b.t || 0)); + } else { + list.forEach((n, i) => { n.time = nt[i]; n.sustain = ns[i]; }); + list.sort((a, b) => (a.time || 0) - (b.time || 0)); + } + } + if (isDrums) S.drumTabDirty = true; + // The bounded window covering the placed content (guarded so a zero- + // sustain tail onset stays strictly inside — see REGION_LEN_GUARD). + const onsetSpan = Math.max(0, maxOnset - minBeat); + let lenBeat = maxEnd - minBeat; + if (!(lenBeat > onsetSpan)) lenBeat = onsetSpan + REGION_LEN_GUARD; + const track = _findTrack(this.trackId); + if (track) { + this._regionBefore = _snapRegions(track); + this._placedId = this.regionId || _nextRegionIdPure(track.regions); + const region = { id: this._placedId, startBeat: this.startBeat, lenBeat }; + if (this.name) region.name = this.name; + track.regions = _trackRegionsNormalizePure([..._trackRegionsNormalizePure(track.regions), region]); + } + // Land selected — a placed region arrives ready to drag (design R3). + this._sel = { taken: true, track: S.selectedTrackId, region: S.selectedRegionId }; + S.selectedTrackId = this.trackId; + S.selectedRegionId = this._placedId; + } + + rollback() { + const list = _contentList(this.kind, this.arrIdx); + if (list && this._snap) { + if (this.kind === 'drums') { + for (const s of this._snap) s.item.t = s.time; + S.drumTabDirty = true; + } else { + for (const s of this._snap) { s.item.time = s.time; s.item.sustain = s.sustain; } + } + list.length = 0; + for (const it of this._before) list.push(it); + } + _restoreRegions(_findTrack(this.trackId), this._regionBefore); + if (this._sel.taken) { S.selectedTrackId = this._sel.track; S.selectedRegionId = this._sel.region; } + } +} + +// ════════════════════════════════════════════════════════════════════ +// DeleteRegionCmd — remove a region block: the content its window owns AND the +// region entry, as one undoable edit. Membership is by beat through the single +// tempo map (the same predicate the move/place commands use), so it deletes +// exactly the notes/hits under the block and leaves neighbours untouched. +// Rollback re-adds the removed items in their original array order and restores +// the track's raw `regions` and the prior selection. Browser surface: NONE. +// ════════════════════════════════════════════════════════════════════ +export class DeleteRegionCmd { + constructor({ kind, arrIdx, trackId, region } = {}) { + this.kind = kind; + this.arrIdx = arrIdx; + this.trackId = trackId; + this.region = region || {}; + // Removing a block changes no surviving note's pitch; song-level for drums. + this.pitchPreserving = true; + this.songScope = kind === 'drums'; + this._before = null; + this._regionBefore = { taken: false, hadKey: false, value: undefined }; + this._sel = { taken: false, track: '', region: '' }; + } + + exec() { + const list = _contentList(this.kind, this.arrIdx); + if (!list) return; + this._before = list.slice(); + const keep = []; + for (const it of list) { + const beat = beatOf(S.beats, Number(_timeOfItem(this.kind, it)) || 0); + if (!_regionContainsBeatPure(this.region, beat)) keep.push(it); + } + list.length = 0; + for (const it of keep) list.push(it); + if (this.kind === 'drums') S.drumTabDirty = true; + const track = _findTrack(this.trackId); + if (track) { + this._regionBefore = _snapRegions(track); + const remaining = _trackRegionsNormalizePure(track.regions).filter(r => r.id !== this.region.id); + track.regions = _trackRegionsNormalizePure(remaining); + } + this._sel = { taken: true, track: S.selectedTrackId, region: S.selectedRegionId }; + if (S.selectedRegionId === this.region.id) S.selectedRegionId = ''; + } + + rollback() { + const list = _contentList(this.kind, this.arrIdx); + if (list && this._before) { + list.length = 0; + for (const it of this._before) list.push(it); + if (this.kind === 'drums') S.drumTabDirty = true; + } + _restoreRegions(_findTrack(this.trackId), this._regionBefore); + if (this._sel.taken) { S.selectedTrackId = this._sel.track; S.selectedRegionId = this._sel.region; } + } +} diff --git a/src/region.js b/src/region.js index 6b033619..c19578a0 100644 --- a/src/region.js +++ b/src/region.js @@ -108,6 +108,20 @@ export function _trackRegionsResolvePure(raw) { return norm.length ? norm : [_defaultRegionPure()]; } +// The next free `region:N` id for a track, given its persisted regions[] (or +// absent). N is one past the highest numeric suffix in use, and the implicit +// default (DEFAULT_REGION_ID = `region:1`) is always counted even when it isn't +// persisted — so a placed region never collides with it. A fresh track (no +// regions) yields `region:2`. Non-numeric ids don't participate in the count. +export function _nextRegionIdPure(raw) { + let max = 1; // DEFAULT_REGION_ID occupies region:1 + for (const region of _trackRegionsNormalizePure(raw)) { + const m = /^region:(\d+)$/.exec(region.id); + if (m) { const n = Number(m[1]); if (n > max) max = n; } + } + return 'region:' + (max + 1); +} + // Membership predicate: does a beat fall inside the region's window? startBeat // inclusive, startBeat+lenBeat exclusive; lenBeat null = open to the end of // content. This is the primitive a later move/trim command uses to select "the diff --git a/tests/region_place_delete.test.mjs b/tests/region_place_delete.test.mjs new file mode 100644 index 00000000..8faac247 --- /dev/null +++ b/tests/region_place_delete.test.mjs @@ -0,0 +1,246 @@ +/* + * Region PLACE + DELETE (PR 3 / R3b): the "Add Track from File" driver's + * command layer — drop imported content onto the timeline as a bounded, + * selectable, draggable block, and remove a block (content + window) as one + * undoable edit. + * + * Pinned here: + * - PlaceRegionCmd slides content so its first onset lands on `startBeat` + * (a MUSICAL move — beats preserved on a varying grid), writes a BOUNDED + * region covering it (explicit lenBeat, never the implicit default — even + * at bar 1 / startBeat 0), and lands it SELECTED. + * - the placed region is a real draggable block: a subsequent MoveRegionCmd + * rides its window and shifts only its notes. + * - DeleteRegionCmd removes exactly the notes/hits its window owns (membership + * by beat) and drops the region entry, leaving neighbours untouched. + * - round-trips: exec → rollback restores the model EXACTLY (deep-equal notes/ + * hits AND regions[] AND selection), and redo reproduces the post-exec state + * — for a notation arrangement AND for the drum tab, incl. through the real + * S.history.exec path. + * - _nextRegionIdPure never collides with the implicit default region:1. + * + * This suite fails on main: PlaceRegionCmd / DeleteRegionCmd / _nextRegionIdPure + * do not exist there. + * + * Run: node tests/region_place_delete.test.mjs + */ +import assert from 'node:assert'; + +import { beatOf } from '../src/beats.js'; +import { _nextRegionIdPure, _regionsAreDefaultPure } from '../src/region.js'; +import { DeleteRegionCmd, MoveRegionCmd, PlaceRegionCmd } from '../src/region-commands.js'; +import { EditHistory } from '../src/history.js'; +import { S } from '../src/state.js'; +import { seedState, trackHooks } from './_history_env.mjs'; + +let pass = 0; let fail = 0; +const tests = []; +const t = (name, fn) => tests.push([name, fn]); +const clone = (v) => JSON.parse(JSON.stringify(v)); + +// Constant 120 BPM (0.5 s/beat), 4 beats/bar. +function constGrid() { + const b = []; + for (let i = 0; i < 13; i++) b.push({ time: i * 0.5, measure: i % 4 === 0 ? i / 4 + 1 : 0 }); + return b; +} +// Varying: beats 0..4 @ 0.5 s/beat, beats 4..12 @ 1.0 s/beat. +function varyGrid() { + const b = []; + for (let i = 0; i <= 4; i++) b.push({ time: i * 0.5, measure: i % 4 === 0 ? i / 4 + 1 : 0 }); + for (let i = 5; i <= 12; i++) b.push({ time: 2.0 + (i - 4) * 1.0, measure: i % 4 === 0 ? i / 4 + 1 : 0 }); + return b; +} +const note = (time, sustain = 0, string = 0, fret = 0) => ({ time, sustain, string, fret, techniques: {} }); + +function seedNotation({ beats = constGrid(), notes, regions } = {}) { + const arr = { name: 'Lead', notes }; + const trackSession = { + version: 3, tracks: [ + { id: 'transcription:Lead', type: 'transcription', targetId: 'Lead', ...(regions ? { regions } : {}) }, + ], removedSourceIds: [], tempoGuideSourceId: '', tempoGuideLocked: false, tempoGuideMode: 'audio', + }; + seedState({ arrangements: [arr], currentArr: 0, beats, drumTab: null, trackSession, + selectedTrackId: '', selectedRegionId: '' }); + S.history = new EditHistory(); + trackHooks(); + return arr; +} + +function seedDrums({ beats = constGrid(), hits } = {}) { + const drumTab = { version: 1, name: 'Drums', kit: [], hits }; + seedState({ arrangements: [], currentArr: 0, beats, drumTab, + trackSession: { version: 3, tracks: [{ id: 'transcription:drums', type: 'transcription', targetId: 'drums' }], + removedSourceIds: [], tempoGuideSourceId: '', tempoGuideLocked: false, tempoGuideMode: 'audio' }, + selectedTrackId: '', selectedRegionId: '' }); + S.history = new EditHistory(); + trackHooks(); + S.drumTabDirty = false; + return drumTab; +} + +// ── _nextRegionIdPure ───────────────────────────────────────────────── +t('_nextRegionIdPure: fresh track → region:2; never collides with the default', () => { + assert.strictEqual(_nextRegionIdPure(undefined), 'region:2', 'no regions → region:2 (past the implicit region:1)'); + assert.strictEqual(_nextRegionIdPure([]), 'region:2'); + assert.strictEqual(_nextRegionIdPure([{ id: 'region:1', startBeat: 0, lenBeat: null }]), 'region:2', 'default counted'); + assert.strictEqual(_nextRegionIdPure([{ id: 'region:2', startBeat: 4, lenBeat: 4 }]), 'region:3'); + assert.strictEqual(_nextRegionIdPure([{ id: 'region:5', startBeat: 8, lenBeat: 4 }]), 'region:6', 'one past the max'); + assert.strictEqual(_nextRegionIdPure([{ id: 'weird', startBeat: 0, lenBeat: 2 }]), 'region:2', 'non-numeric ignored'); +}); + +// ── PlaceRegionCmd: notation ────────────────────────────────────────── +t('place: slides content to startBeat, writes a bounded region, lands selected; round-trips', () => { + // Notes at beats 1, 2, 3 (last sustains 1 beat → content ends at beat 4). + const arr = seedNotation({ notes: [note(0.5), note(1.0), note(1.5, 0.5)] }); + const track = S.trackSession.tracks[0]; + const before = clone(arr.notes); + const cmd = new PlaceRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', startBeat: 4 }); + cmd.exec(); + // minBeat 1 → startBeat 4 is dBeat +3 = +1.5 s @120. + assert.deepStrictEqual(arr.notes.map(n => n.time), [2.0, 2.5, 3.0], 'onsets rode +3 beats to bar 2'); + assert.strictEqual(arr.notes[2].sustain, 0.5, 'constant-tempo shift keeps the sustain'); + assert.deepStrictEqual(track.regions, [{ id: 'region:2', startBeat: 4, lenBeat: 3 }], 'bounded region covers the block'); + assert.strictEqual(S.selectedTrackId, 'transcription:Lead', 'track selected'); + assert.strictEqual(S.selectedRegionId, 'region:2', 'placed region selected'); + const after = clone(arr.notes); + cmd.rollback(); + assert.deepStrictEqual(arr.notes, before, 'rollback restores notes EXACTLY'); + assert.ok(!('regions' in track), 'rollback removes the regions key that was never there'); + assert.strictEqual(S.selectedRegionId, '', 'prior (empty) selection restored'); + cmd.exec(); + assert.deepStrictEqual(arr.notes, after, 'redo reproduces the placed notes'); + assert.deepStrictEqual(track.regions, [{ id: 'region:2', startBeat: 4, lenBeat: 3 }], 'redo reproduces the region'); +}); + +t('place at bar 1 (startBeat 0) is still BOUNDED, never the implicit default', () => { + const arr = seedNotation({ notes: [note(1.0), note(1.5)] }); // beats 2, 3 + const track = S.trackSession.tracks[0]; + new PlaceRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', startBeat: 0 }).exec(); + assert.deepStrictEqual(arr.notes.map(n => n.time), [0.0, 0.5], 'content pulled to beat 0'); + assert.strictEqual(track.regions.length, 1, 'one region'); + assert.ok(track.regions[0].lenBeat > 0, 'explicit length → a real bounded block'); + assert.strictEqual(_regionsAreDefaultPure(track.regions), false, 'NOT the implicit default region'); +}); + +t('place: zero-sustain tail onset stays strictly inside the region window', () => { + const arr = seedNotation({ notes: [note(0.5), note(1.0)] }); // beats 1, 2, both sustain 0 + const track = S.trackSession.tracks[0]; + new PlaceRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', startBeat: 0 }).exec(); + const r = track.regions[0]; + const lastBeat = beatOf(S.beats, arr.notes[arr.notes.length - 1].time); + assert.ok(lastBeat < r.startBeat + r.lenBeat, 'end-exclusive window still owns its last onset'); +}); + +t('place preserves musical position on a VARYING grid (beats, not seconds)', () => { + const arr = seedNotation({ beats: varyGrid(), notes: [note(0.5), note(1.5, 0.5)] }); // beats 1, 3 + new PlaceRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', startBeat: 5 }).exec(); + // minBeat 1 → startBeat 5 = +4 beats. Onsets should land on beats 5 and 7. + const beats = S.beats; + assert.ok(Math.abs(beatOf(beats, arr.notes[0].time) - 5) < 1e-9, 'first onset rode to beat 5'); + assert.ok(Math.abs(beatOf(beats, arr.notes[1].time) - 7) < 1e-9, 'second onset rode to beat 7'); +}); + +t('a PLACED region is a real draggable block: MoveRegionCmd rides it', () => { + const arr = seedNotation({ notes: [note(0.5), note(1.0, 0.5)] }); // beats 1, 2 + const track = S.trackSession.tracks[0]; + S.history.exec(new PlaceRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', startBeat: 4 })); + const placed = clone(track.regions[0]); + const notesAfterPlace = clone(arr.notes); + S.history.exec(new MoveRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', region: track.regions[0], dBeat: 4 })); + assert.strictEqual(track.regions[0].startBeat, placed.startBeat + 4, 'the placed window rode the drag +4 beats'); + S.history.doUndo(); // undo the move + assert.deepStrictEqual(arr.notes, notesAfterPlace, 'undo restores the placed notes'); + assert.deepStrictEqual(track.regions[0], placed, 'undo restores the placed window'); +}); + +// ── PlaceRegionCmd: drums ───────────────────────────────────────────── +t('place drums: hits slide, a region lands on the drum track, tab dirtied; round-trips', () => { + const drumTab = seedDrums({ hits: [{ t: 0.5, p: 36 }, { t: 1.0, p: 38 }, { t: 1.5, p: 42 }] }); // beats 1,2,3 + const track = S.trackSession.tracks[0]; + const before = clone(drumTab.hits); + const cmd = new PlaceRegionCmd({ kind: 'drums', trackId: 'transcription:drums', startBeat: 4 }); + cmd.exec(); + assert.deepStrictEqual(drumTab.hits.map(h => h.t), [2.0, 2.5, 3.0], 'hits rode +3 beats'); + assert.strictEqual(S.drumTabDirty, true, 'placing dirties the tab'); + assert.strictEqual(track.regions.length, 1, 'a region landed on the drum track'); + assert.strictEqual(S.selectedRegionId, track.regions[0].id, 'placed drum region selected'); + cmd.rollback(); + assert.deepStrictEqual(drumTab.hits, before, 'rollback restores hits EXACTLY'); + assert.ok(!('regions' in track), 'and removes the region key'); +}); + +// ── DeleteRegionCmd: notation ───────────────────────────────────────── +t('delete: removes only the block’s notes + its entry; leaves neighbours; round-trips', () => { + // A owns beat 1; B owns beats 4, 5. + const regions = [{ id: 'A', startBeat: 0, lenBeat: 4 }, { id: 'B', startBeat: 4, lenBeat: 4 }]; + const arr = seedNotation({ notes: [note(0.5), note(2.0, 0.25), note(2.5)], regions }); // beats 1, 4, 5 + const track = S.trackSession.tracks[0]; + S.selectedTrackId = 'transcription:Lead'; + S.selectedRegionId = 'B'; + const notesBefore = clone(arr.notes); + const regionsBefore = clone(track.regions); + const cmd = new DeleteRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', region: { id: 'B', startBeat: 4, lenBeat: 4 } }); + cmd.exec(); + assert.deepStrictEqual(arr.notes.map(n => n.time), [0.5], 'only A’s note survives'); + assert.deepStrictEqual(track.regions, [{ id: 'A', startBeat: 0, lenBeat: 4 }], 'B’s entry dropped, A kept'); + assert.strictEqual(S.selectedRegionId, '', 'the deleted region is deselected'); + cmd.rollback(); + assert.deepStrictEqual(arr.notes, notesBefore, 'notes restored exactly (order + values)'); + assert.deepStrictEqual(track.regions, regionsBefore, 'regions[] restored exactly'); + assert.strictEqual(S.selectedRegionId, 'B', 'selection restored'); + cmd.exec(); + assert.deepStrictEqual(arr.notes.map(n => n.time), [0.5], 'redo re-deletes'); +}); + +t('delete respects the end-exclusive window (a note ON the boundary is a neighbour’s)', () => { + // Region [0,4): owns beats 0..3; the note at beat 4 belongs to the next block. + const regions = [{ id: 'A', startBeat: 0, lenBeat: 4 }, { id: 'B', startBeat: 4, lenBeat: 4 }]; + const arr = seedNotation({ notes: [note(1.5), note(2.0)], regions }); // beats 3, 4 + new DeleteRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', region: { id: 'A', startBeat: 0, lenBeat: 4 } }).exec(); + assert.deepStrictEqual(arr.notes.map(n => n.time), [2.0], 'beat 3 deleted; beat 4 (B’s edge) kept'); +}); + +// ── DeleteRegionCmd: drums ──────────────────────────────────────────── +t('delete drums: removes the windowed hits + entry; round-trips and dirties', () => { + const drumTab = seedDrums({ hits: [{ t: 0.5, p: 36 }, { t: 2.0, p: 38 }, { t: 2.5, p: 42 }] }); // beats 1, 4, 5 + const track = S.trackSession.tracks[0]; + track.regions = [{ id: 'B', startBeat: 4, lenBeat: 4 }]; // owns beats 4, 5 + const before = clone(drumTab.hits); + const cmd = new DeleteRegionCmd({ kind: 'drums', trackId: 'transcription:drums', region: { id: 'B', startBeat: 4, lenBeat: 4 } }); + cmd.exec(); + assert.deepStrictEqual(drumTab.hits.map(h => h.t), [0.5], 'the two windowed hits removed'); + assert.strictEqual(S.drumTabDirty, true, 'delete dirties the tab'); + assert.deepStrictEqual(track.regions, [], 'the region entry is gone'); + cmd.rollback(); + assert.deepStrictEqual(drumTab.hits, before, 'rollback restores hits EXACTLY'); +}); + +// ── Through EditHistory (the real exec path) ────────────────────────── +t('via S.history.exec: place → delete, undo/redo restore both', () => { + const arr = seedNotation({ notes: [note(0.5), note(1.0)] }); // beats 1, 2 + const track = S.trackSession.tracks[0]; + const empty = clone(arr.notes); + S.history.exec(new PlaceRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', startBeat: 4 })); + const placedNotes = clone(arr.notes); + const region = clone(track.regions[0]); + S.history.exec(new DeleteRegionCmd({ kind: 'notation', arrIdx: 0, trackId: 'transcription:Lead', region: track.regions[0] })); + assert.deepStrictEqual(arr.notes, [], 'the placed block was deleted'); + assert.deepStrictEqual(track.regions, [], 'its entry gone'); + S.history.doUndo(); // undo delete + assert.deepStrictEqual(arr.notes, placedNotes, 'undo restores the placed notes'); + assert.deepStrictEqual(track.regions[0], region, 'undo restores the region'); + S.history.doUndo(); // undo place + assert.deepStrictEqual(arr.notes, empty, 'undo the place restores the pre-place notes'); + assert.ok(!('regions' in track), 'and the regions key'); + S.history.doRedo(); // redo place + assert.deepStrictEqual(arr.notes, placedNotes, 'redo the place'); +}); + +for (const [name, fn] of tests) { + try { await fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); From 7f421831428a62aecdef389c94dae5f2f7100c15 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Tue, 21 Jul 2026 08:32:15 -0500 Subject: [PATCH 2/7] fix(editor): region commands act on the drum part the track names, not the active tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A song can hold several drum parts now (#339): each type:"drums" arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid target. The region commands and the parts-view drag arming predate that (#334) — kind:'drums' always operated on S.drumTab, and a materialized extra part ('drums-2'…) armed as 'notation' over its empty shell notes[], so dragging its block moved nothing (or, for the primary while another part was active, moved the WRONG part's hits). Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part whose track carries the region → its own .drumTab; arrIdx < 0 stays the legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared _contentList, and parts-view arms kind:'drums' for any drum-arrangement row. Tests drive a NON-active part and pin that the active tab never moves, plus the legacy no-arrIdx fallback. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix --- src/parts-view.js | 7 ++- src/region-commands.js | 40 +++++++++-------- tests/region_place_delete.test.mjs | 71 ++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/src/parts-view.js b/src/parts-view.js index 8d7c95de..5557ee27 100644 --- a/src/parts-view.js +++ b/src/parts-view.js @@ -415,7 +415,12 @@ export function _partsViewOnMouseDown(e, x, y) { trackId: row.id, regionId: hitRegion, region, - kind: row.targetId === 'drums' ? 'drums' : 'notation', + // Any drum part's row (its own arrangement, or the legacy + // unmaterialized 'drums' target) moves DRUM content — the + // command resolves the part's own tab from arrIdx, so dragging + // a non-active part never shifts the active grid's hits. + kind: (rArrIdx >= 0 && isDrumArrangement(S.arrangements[rArrIdx])) || row.targetId === 'drums' + ? 'drums' : 'notation', arrIdx: rArrIdx, origStart: span.t0, spanW: Math.max(0, span.t1 - span.t0), diff --git a/src/region-commands.js b/src/region-commands.js index e02d0557..bac68bb8 100644 --- a/src/region-commands.js +++ b/src/region-commands.js @@ -3,9 +3,9 @@ // // A region is a WINDOW over content, never a copy (src/region.js). "Move // region" therefore shifts the CONTENT the window covers — a notation region -// shifts the arrangement's contained notes, a drum region shifts S.drumTab's -// contained hits — as one undoable edit, with the window's own placement riding -// along for a BOUNDED region. It goes through S.history.exec (NOT the +// shifts the arrangement's contained notes, a drum region shifts its own +// part's tab hits (see _drumTabFor) — as one undoable edit, with the window's +// own placement riding along for a BOUNDED region. It goes through S.history.exec (NOT the // track-session commit() path): commit() only marks the session dirty, so an // in-place time shift would leave the coverage/chord/lint memos stale — exec() // bumps editGen, which is what forces them to recompute. @@ -34,12 +34,21 @@ import { S } from './state.js'; // by this sub-cent-of-a-beat guard so the region keeps owning its own notes. const REGION_LEN_GUARD = 1e-4; -// ── Content/track access shared by the placement commands ───────────── -// (MoveRegionCmd predates these and keeps its own copies as methods; leaving it -// untouched avoids churning shipped, tested code.) +// ── Content/track access shared by all region commands ──────────────── +// A song can hold SEVERAL drum parts (each type:"drums" arrangement OWNS its +// `.drumTab`; `S.drumTab` is only the ACTIVE grid target) — so a drums region +// command must act on the tab of the part whose TRACK carries the region, not +// whichever part happens to be active. `arrIdx >= 0` names that part; +// `arrIdx < 0` is the legacy unmaterialized tab (create-mode compose), which +// IS `S.drumTab`. Same resolution the lane silhouette paints by. +function _drumTabFor(arrIdx) { + const arr = Number.isInteger(arrIdx) && arrIdx >= 0 && S.arrangements ? S.arrangements[arrIdx] : null; + return (arr && arr.drumTab) || S.drumTab; +} function _contentList(kind, arrIdx) { if (kind === 'drums') { - return (S.drumTab && Array.isArray(S.drumTab.hits)) ? S.drumTab.hits : null; + const tab = _drumTabFor(arrIdx); + return (tab && Array.isArray(tab.hits)) ? tab.hits : null; } const arr = S.arrangements && S.arrangements[arrIdx]; return arr && Array.isArray(arr.notes) ? arr.notes : null; @@ -64,10 +73,11 @@ function _restoreRegions(track, snap) { } export class MoveRegionCmd { - // `kind`: 'notation' shifts S.arrangements[arrIdx].notes; 'drums' shifts - // S.drumTab.hits. `region` is the resolved region object being moved (its id - // locates it in the track's regions[]); `dBeat` is the snapped bar/beat - // distance of the drag. + // `kind`: 'notation' shifts S.arrangements[arrIdx].notes; 'drums' shifts the + // hits of the drum part `arrIdx` names (its own `.drumTab`; arrIdx < 0 = the + // legacy unmaterialized S.drumTab — see _drumTabFor). `region` is the + // resolved region object being moved (its id locates it in the track's + // regions[]); `dBeat` is the snapped bar/beat distance of the drag. constructor({ kind, arrIdx, trackId, region, dBeat }) { this.kind = kind; this.arrIdx = arrIdx; @@ -95,13 +105,7 @@ export class MoveRegionCmd { return r.lenBeat != null || (Number(r.startBeat) || 0) > 0 || r.srcIn != null; } - _list() { - if (this.kind === 'drums') { - return (S.drumTab && Array.isArray(S.drumTab.hits)) ? S.drumTab.hits : null; - } - const arr = S.arrangements && S.arrangements[this.arrIdx]; - return arr && Array.isArray(arr.notes) ? arr.notes : null; - } + _list() { return _contentList(this.kind, this.arrIdx); } _timeOf(item) { return this.kind === 'drums' ? item.t : item.time; } diff --git a/tests/region_place_delete.test.mjs b/tests/region_place_delete.test.mjs index 8faac247..007648c5 100644 --- a/tests/region_place_delete.test.mjs +++ b/tests/region_place_delete.test.mjs @@ -216,6 +216,77 @@ t('delete drums: removes the windowed hits + entry; round-trips and dirties', () assert.deepStrictEqual(drumTab.hits, before, 'rollback restores hits EXACTLY'); }); +// ── Multi-drum seam: commands act on the part the TRACK names ───────── +// A song can hold several drum parts; S.drumTab is only the ACTIVE grid +// target. A region command must resolve the tab from arrIdx (the part whose +// track carries the region) — never assume the active tab. +function seedTwoDrumParts() { + const primaryTab = { version: 1, name: 'Drums', kit: [], hits: [{ t: 0.5, p: 36 }, { t: 1.0, p: 38 }] }; + const extraTab = { version: 1, name: 'Drums 2', kit: [], hits: [{ t: 0.5, p: 42 }, { t: 1.5, p: 42 }] }; + const arrs = [ + { id: 'lead', name: 'Lead', notes: [{ time: 0.5, sustain: 0, string: 0, fret: 0, techniques: {} }], chords: [] }, + { id: 'drums', name: 'Drums', type: 'drums', drumTab: primaryTab, notes: [], chords: [] }, + { id: 'drums-2', name: 'Drums 2', type: 'drums', drumTab: extraTab, notes: [], chords: [] }, + ]; + seedState({ arrangements: arrs, currentArr: 0, beats: constGrid(), drumTab: primaryTab, + trackSession: { version: 3, tracks: [ + { id: 'transcription:lead', type: 'transcription', targetId: 'lead' }, + { id: 'transcription:drums', type: 'transcription', targetId: 'drums' }, + { id: 'transcription:drums-2', type: 'transcription', targetId: 'drums-2' }, + ], removedSourceIds: [], tempoGuideSourceId: '', tempoGuideLocked: false, tempoGuideMode: 'audio' }, + selectedTrackId: '', selectedRegionId: '' }); + S.history = new EditHistory(); + trackHooks(); + S.drumTabDirty = false; + return { primaryTab, extraTab }; +} + +t('move on a NON-ACTIVE drum part shifts ITS hits; the active tab is untouched', () => { + const { primaryTab, extraTab } = seedTwoDrumParts(); + const primaryBefore = clone(primaryTab.hits); + const extraBefore = clone(extraTab.hits); + const cmd = new MoveRegionCmd({ kind: 'drums', arrIdx: 2, trackId: 'transcription:drums-2', + region: { id: 'region:1', startBeat: 0, lenBeat: null }, dBeat: 2 }); + cmd.exec(); + assert.deepStrictEqual(extraTab.hits.map(h => h.t), [1.5, 2.5], 'the part’s own hits rode +2 beats'); + assert.deepStrictEqual(primaryTab.hits, primaryBefore, 'the ACTIVE tab (primary) never moved'); + cmd.rollback(); + assert.deepStrictEqual(extraTab.hits, extraBefore, 'rollback restores the part’s hits exactly'); + assert.deepStrictEqual(primaryTab.hits, primaryBefore, 'primary still untouched after rollback'); +}); + +t('place on a NON-ACTIVE drum part slides ITS hits + regions land on ITS track', () => { + const { primaryTab, extraTab } = seedTwoDrumParts(); + const primaryBefore = clone(primaryTab.hits); + const track = S.trackSession.tracks[2]; + const cmd = new PlaceRegionCmd({ kind: 'drums', arrIdx: 2, trackId: 'transcription:drums-2', startBeat: 4 }); + cmd.exec(); + assert.deepStrictEqual(extraTab.hits.map(h => h.t), [2.0, 3.0], 'the part’s hits placed at bar 2'); + assert.deepStrictEqual(primaryTab.hits, primaryBefore, 'the ACTIVE tab (primary) never moved'); + assert.strictEqual(track.regions.length, 1, 'the region landed on the part’s OWN track'); + assert.ok(!('regions' in S.trackSession.tracks[1]), 'not on the primary’s track'); + cmd.rollback(); + assert.ok(!('regions' in track), 'rollback clears it'); +}); + +t('delete on a NON-ACTIVE drum part removes ITS windowed hits only', () => { + const { primaryTab, extraTab } = seedTwoDrumParts(); + const primaryBefore = clone(primaryTab.hits); + S.trackSession.tracks[2].regions = [{ id: 'B', startBeat: 0, lenBeat: 2 }]; // owns beats 0..1 + new DeleteRegionCmd({ kind: 'drums', arrIdx: 2, trackId: 'transcription:drums-2', + region: { id: 'B', startBeat: 0, lenBeat: 2 } }).exec(); + assert.deepStrictEqual(extraTab.hits.map(h => h.t), [1.5], 'only the windowed hit (beat 1) removed'); + assert.deepStrictEqual(primaryTab.hits, primaryBefore, 'the ACTIVE tab untouched'); +}); + +t('legacy fallback: kind drums with NO arrIdx still targets S.drumTab', () => { + const { primaryTab } = seedTwoDrumParts(); + const cmd = new MoveRegionCmd({ kind: 'drums', trackId: 'transcription:drums', + region: { id: 'region:1', startBeat: 0, lenBeat: null }, dBeat: 2 }); + cmd.exec(); + assert.deepStrictEqual(primaryTab.hits.map(h => h.t), [1.5, 2.0], 'no arrIdx → the active tab moved (legacy path)'); +}); + // ── Through EditHistory (the real exec path) ────────────────────────── t('via S.history.exec: place → delete, undo/redo restore both', () => { const arr = seedNotation({ notes: [note(0.5), note(1.0)] }); // beats 1, 2 From 6891d0a0120bbd7b1e1c3272b51eaaf0173753da Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Tue, 21 Jul 2026 08:38:47 -0500 Subject: [PATCH 3/7] feat(editor): the drum import lands as a placed, selected region (Add Track from File) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The R3b payoff: importing a drum part into an existing project now surfaces it in the Tracks view as a SELECTED region, ready to drag. The Add-Drums dialog gains an inline "Place at" choice — Keep source timing (default: the file's own timing, no content motion, no persisted window), Bar 1, or Playhead — the latter two sliding the whole part there as one undoable PlaceRegionCmd (bar-snapped, beat-preserving). placeImportedPartAsRegion (track-session.js) is the orchestrator: it re-normalizes the session so the fresh part's row exists (the import just created the part — its row is synthesized here), resolves the track from the part's arr: target, then places or just selects. The import flow (arrangement.js) reaches it through the host table — track-session imports arrangement, so the call inverts through the usual seam. _placeAtStartBeatPure (region.js) resolves the dialog choice to a startBeat, converter-free like the other region pures. Create mode keeps replace semantics: the Place-at row hides, the choice is forced to 'keep', and with no materialized arrangement the hook no-ops gracefully. Tests drive the real orchestrator: row synthesis + placement + selection + undo restore, keep-writes-no-window, playhead snapping, and the pure's resolution table. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix --- CHANGELOG.md | 15 ++++++ screen.html | 14 ++++++ src/arrangement.js | 20 +++++++- src/host.js | 7 +++ src/main.js | 5 +- src/region.js | 17 +++++++ src/track-session.js | 35 ++++++++++++- tests/region_place_delete.test.mjs | 81 +++++++++++++++++++++++++++++- 8 files changed, 189 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db27570..c5cd8fc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Track regions — import a drum part into an existing project as a placed, + movable region.** The Add-Drums import (New Track ▸ Drums ▸ from a file, or + Track ▸ Drums…) now lands the imported part in the Tracks view **selected as + a region**, ready to drag — and a new **"Place at"** choice in the dialog + says where: **Keep source timing** (the default — the file's own timing, no + content motion), **Bar 1**, or **Playhead** (the whole part slides there as + one undoable step, snapped to the bar, beats preserved across tempo + changes). Under the hood: `PlaceRegionCmd` / `DeleteRegionCmd` (bounded + window + verbatim-snapshot undo, mirroring the move command) and a + `placeImportedPartAsRegion` orchestrator the import flows reach through the + host table. Also fixes the region commands and the Tracks drag-arming for + **multi-drum songs**: a region on a drum part's track now always acts on + that part's own tab — never whichever part happens to be active in the drum + grid (dragging an extra part's block used to move nothing at all). + - **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 diff --git a/screen.html b/screen.html index c9ff139d..102d27ec 100644 --- a/screen.html +++ b/screen.html @@ -953,6 +953,20 @@
+ +
+ + +