Skip to content

Commit 1a29ef2

Browse files
committed
Improve MIDI Studio V2 instrument duplication, ordering, and ownership workflow - PR_26146_067-midi-studio-v2-instrument-management-workflow
1 parent 3d00f1c commit 1a29ef2

7 files changed

Lines changed: 554 additions & 40 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# PR_26146_067 MIDI Studio V2 Instrument Management Workflow Validation
2+
3+
## Scope
4+
5+
PASS - Continued from PR_26146_066 and kept the change focused on MIDI Studio V2 instrument management around duplicate, order, delete confirmation, and instrument-owned data.
6+
7+
## Implementation
8+
9+
- PASS - Duplicate remains in the Octave Timeline Instruments header as the primary creation workflow.
10+
- PASS - Duplicate creates readable names and ids such as `Lead 1` / `lead-1` without GUID-style display names.
11+
- PASS - Duplicate copies owned lane notes, playback settings, instrument configuration, volume, pan, and effects/advanced placeholder data.
12+
- PASS - Duplicated instrument becomes the shared selected instrument and receives visible confirmation styling.
13+
- PASS - Duplicated/selected instruments are scrolled into view through the existing selected instrument reveal flow.
14+
- PASS - Move Up and Move Down update the canonical `studioArrangement.lanes` ordering.
15+
- PASS - Delete now requires an inline visible confirmation before removing an instrument.
16+
- PASS - The final remaining instrument cannot be deleted and shows a visible blocked state.
17+
- PASS - Selected instrument synchronization between Octave Timeline and Instruments is preserved.
18+
19+
## Validation
20+
21+
- PASS - `node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
22+
- PASS - `node --check tools/midi-studio-v2/js/MidiStudioV2App.js`
23+
- PASS - `node --check tools/midi-studio-v2/js/bootstrap.js`
24+
- PASS - `node --check tests/playwright/tools/MidiStudioV2.spec.mjs`
25+
- PASS - `npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs --project=playwright --grep "PR067"` passed 1 test.
26+
- PASS - `npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs --project=playwright --grep "PR0(60|61|62|63|64|65|66|67)"` passed 8 tests.
27+
- PASS - `rg --pcre2 -n "<script(?![^>]*src=)|<style\\b|\\sstyle=|\\son[a-z]+=" tools/midi-studio-v2/index.html` returned no inline script/style/event handler matches.
28+
- PASS - `git diff --check`
29+
- PASS - `npm run codex:review-artifacts`
30+
31+
## Samples
32+
33+
SKIP - Full samples smoke test was not run per PR instruction.

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 161 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,8 @@ async function visibleMidiStudioControlOwnership(page, activeTabId) {
621621
instrumentSetField: { canonical: "music.songs[].instrumentSet", kind: "readonly", owner: "MIDI Import", wired: "wired" },
622622
jumpToSectionButton: { canonical: "timing preview section state", kind: "workflow-state", owner: "Octave Timeline", wired: "wired" },
623623
loopToggle: { canonical: "playback loop state", kind: "workflow-state", owner: "Octave Timeline", wired: "wired" },
624+
moveInstrumentDownButton: { canonical: "music.songs[].studioArrangement.lanes order", kind: "canonical-action", owner: "Octave Timeline", wired: "wired" },
625+
moveInstrumentUpButton: { canonical: "music.songs[].studioArrangement.lanes order", kind: "canonical-action", owner: "Octave Timeline", wired: "wired" },
624626
normalizeInstrumentGridButton: { canonical: "music.songs[].studioArrangement", kind: "canonical-action", owner: "Auto-Create Parts", wired: "wired" },
625627
parseSongSheetButton: { canonical: "music.songs[].studioArrangement", kind: "canonical-action", owner: "Song Setup", wired: "wired" },
626628
playButton: { canonical: "playback from selected canonical song model", kind: "action", owner: "Global NAV", wired: "wired" },
@@ -3675,7 +3677,7 @@ test.describe("MIDI Studio V2", () => {
36753677
await timelineQuickInstrumentRow(page, "lead").locator("[data-timeline-quick-solo='lead']").click();
36763678
await timelineQuickInstrumentRow(page, "lead").locator("[data-toggle-instrument-visibility='lead']").click();
36773679
await page.locator("#duplicateInstrumentRowButton").click();
3678-
await expect(page.locator("#statusLog")).toHaveValue(/OK Duplicated instrument row Lead as Lead Copy; playback data updated\./);
3680+
await expect(page.locator("#statusLog")).toHaveValue(/OK Duplicated instrument row Lead as Lead 1; playback data updated\./);
36793681

36803682
const duplicateState = await page.evaluate((leadLaneSource) => {
36813683
const app = window.__midiStudioV2App;
@@ -3698,7 +3700,7 @@ test.describe("MIDI Studio V2", () => {
36983700
}, beforeDuplicate.leadLane);
36993701
expect(duplicateState).toEqual({
37003702
copiedLaneData: true,
3701-
duplicateDisplayName: "Lead Copy Source Copy",
3703+
duplicateDisplayName: "Lead 1",
37023704
duplicateInstrument: "gm-electric-bass-finger",
37033705
duplicateInstrumentType: "Bass",
37043706
duplicatePan: -0.3,
@@ -3707,18 +3709,18 @@ test.describe("MIDI Studio V2", () => {
37073709
duplicateSoloed: true,
37083710
duplicateVolume: 0.6,
37093711
hasUniqueId: true,
3710-
selected: "lead-copy"
3712+
selected: "lead-1"
37113713
});
3712-
await expect(timelineQuickInstrumentRow(page, "lead-copy")).toHaveClass(/is-selected/);
3714+
await expect(timelineQuickInstrumentRow(page, "lead-1")).toHaveClass(/is-selected/);
37133715

37143716
await selectMidiStudioTab(page, "instruments");
3715-
await expect(instrumentRow(page, "lead-copy")).toHaveClass(/is-selected/);
3717+
await expect(instrumentRow(page, "lead-1")).toHaveClass(/is-selected/);
37163718
await expect(page.locator("#selectedInstrumentEditor")).not.toContainText("Mute default");
37173719
await expect(page.locator("#selectedInstrumentEditor")).not.toContainText("Solo default");
37183720

37193721
await selectMidiStudioTab(page, "studio");
3720-
await timelineQuickInstrumentRow(page, "lead-copy").locator("[data-timeline-quick-mute='lead-copy']").click();
3721-
await timelineQuickInstrumentRow(page, "lead-copy").locator("[data-toggle-instrument-visibility='lead-copy']").click();
3722+
await timelineQuickInstrumentRow(page, "lead-1").locator("[data-timeline-quick-mute='lead-1']").click();
3723+
await timelineQuickInstrumentRow(page, "lead-1").locator("[data-toggle-instrument-visibility='lead-1']").click();
37223724
await page.locator("#playButton").click();
37233725
await expect(page.locator("#playButton")).toBeDisabled();
37243726
await expect(page.locator("#stopButton")).toBeEnabled();
@@ -3731,6 +3733,158 @@ test.describe("MIDI Studio V2", () => {
37313733
}
37323734
});
37333735

3736+
test("manages PR067 instrument duplication ordering and guarded deletion", async ({ page }) => {
3737+
await page.setViewportSize({ width: 1600, height: 900 });
3738+
const server = await openMidiStudioForImport(page);
3739+
try {
3740+
await page.locator("#toolImportManifestInput").setInputFiles(uatManifestPath);
3741+
3742+
await selectMidiStudioTab(page, "instruments");
3743+
await selectInstrumentRow(page, "lead");
3744+
await instrumentTypeSelect(page, "lead").selectOption("Bass");
3745+
await instrumentSelect(page, "lead").selectOption("gm-electric-bass-finger");
3746+
await setInputValue(page, "#previewVolumeLeadInput", "0.62");
3747+
await setInputValue(page, "#previewPanLeadInput", "-0.25");
3748+
await setInputValue(page, "#previewTransposeLeadInput", "7");
3749+
await setInputValue(page, "#previewVelocityLeadInput", "91");
3750+
await setInputValue(page, "#previewDurationLeadInput", "1.4");
3751+
await page.evaluate(() => {
3752+
const app = window.__midiStudioV2App;
3753+
const state = app.instrumentGrid.previewLaneState.lead;
3754+
state.effects = { ...state.effects, reverb: "future-room" };
3755+
state.advanced = { ...state.advanced, midiChannel: 3 };
3756+
app.syncSelectedArrangementFromGridInput(app.instrumentGrid.readInput());
3757+
});
3758+
const leadState = await page.evaluate(() => {
3759+
const app = window.__midiStudioV2App;
3760+
const song = app.selectedSong();
3761+
const settings = song.studioArrangement.previewLaneSettings;
3762+
return {
3763+
advanced: settings.advanced.lead,
3764+
effects: settings.effects.lead,
3765+
laneText: song.studioArrangement.lanes.lead,
3766+
settings: {
3767+
duration: settings.durations.lead,
3768+
instrument: settings.instruments.lead,
3769+
instrumentType: settings.instrumentTypes.lead,
3770+
pan: settings.pans.lead,
3771+
transpose: settings.transposes.lead,
3772+
velocity: settings.velocities.lead,
3773+
volume: settings.volumes.lead
3774+
}
3775+
};
3776+
});
3777+
3778+
await selectMidiStudioTab(page, "studio");
3779+
await waitForCanvasRender(page);
3780+
await page.locator("#duplicateInstrumentRowButton").click();
3781+
await expect(page.locator("#statusLog")).toHaveValue(/OK Duplicated instrument row Lead as Lead 1; playback data updated\./);
3782+
await expect(timelineQuickInstrumentRow(page, "lead-1")).toHaveClass(/is-selected/);
3783+
await expect(timelineQuickInstrumentRow(page, "lead-1")).toHaveAttribute("data-duplicate-confirmation", "true");
3784+
3785+
const duplicateState = await page.evaluate((expectedLeadState) => {
3786+
const app = window.__midiStudioV2App;
3787+
const song = app.selectedSong();
3788+
const settings = song.studioArrangement.previewLaneSettings;
3789+
const selected = app.instrumentGrid.selectedInstrumentId;
3790+
return {
3791+
copiedAdvanced: settings.advanced[selected],
3792+
copiedEffects: settings.effects[selected],
3793+
copiedLaneText: song.studioArrangement.lanes[selected] === expectedLeadState.laneText,
3794+
copiedSettings: {
3795+
duration: settings.durations[selected],
3796+
instrument: settings.instruments[selected],
3797+
instrumentType: settings.instrumentTypes[selected],
3798+
pan: settings.pans[selected],
3799+
transpose: settings.transposes[selected],
3800+
velocity: settings.velocities[selected],
3801+
volume: settings.volumes[selected]
3802+
},
3803+
displayName: settings.displayNames[selected],
3804+
hasUniqueId: selected === "lead-1" && Object.hasOwn(song.studioArrangement.lanes, selected),
3805+
order: Object.keys(song.studioArrangement.lanes),
3806+
selected
3807+
};
3808+
}, leadState);
3809+
expect(duplicateState).toEqual({
3810+
copiedAdvanced: leadState.advanced,
3811+
copiedEffects: leadState.effects,
3812+
copiedLaneText: true,
3813+
copiedSettings: leadState.settings,
3814+
displayName: "Lead 1",
3815+
hasUniqueId: true,
3816+
order: expect.arrayContaining(["lead", "lead-1"]),
3817+
selected: "lead-1"
3818+
});
3819+
const duplicateOrder = duplicateState.order;
3820+
const duplicateIndex = duplicateOrder.indexOf("lead-1");
3821+
expect(duplicateIndex).toBeGreaterThan(0);
3822+
expect(duplicateOrder[duplicateIndex - 1]).toBe("lead");
3823+
3824+
await selectMidiStudioTab(page, "instruments");
3825+
await expect(instrumentRow(page, "lead-1")).toHaveClass(/is-selected/);
3826+
await selectMidiStudioTab(page, "studio");
3827+
await page.locator("#moveInstrumentUpButton").click();
3828+
await expect(page.locator("#statusLog")).toHaveValue(/OK Moved instrument row Lead 1 up; canonical order updated\./);
3829+
const orderAfterMoveUp = await page.evaluate(() => Object.keys(window.__midiStudioV2App.selectedSong().studioArrangement.lanes));
3830+
expect(orderAfterMoveUp.indexOf("lead-1")).toBe(duplicateIndex - 1);
3831+
await page.locator("#moveInstrumentDownButton").click();
3832+
await expect(page.locator("#statusLog")).toHaveValue(/OK Moved instrument row Lead 1 down; canonical order updated\./);
3833+
const orderAfterMoveDown = await page.evaluate(() => Object.keys(window.__midiStudioV2App.selectedSong().studioArrangement.lanes));
3834+
expect(orderAfterMoveDown).toEqual(duplicateOrder);
3835+
await expect(timelineQuickInstrumentRow(page, "lead-1")).toHaveClass(/is-selected/);
3836+
const expectedSelectionAfterDelete = orderAfterMoveDown[orderAfterMoveDown.indexOf("lead-1") + 1]
3837+
|| orderAfterMoveDown[orderAfterMoveDown.indexOf("lead-1") - 1]
3838+
|| "";
3839+
3840+
await page.locator("#playButton").click();
3841+
await expect(page.locator("#playButton")).toBeDisabled();
3842+
await expect(page.locator("#stopButton")).toBeEnabled();
3843+
await page.locator("#stopButton").click();
3844+
await expect(page.locator("#stopButton")).toBeDisabled();
3845+
await expect(page.locator("#playButton")).toBeEnabled();
3846+
3847+
await selectMidiStudioTab(page, "instruments");
3848+
await instrumentRow(page, "lead-1").locator("[data-delete-instrument-row='lead-1']").click();
3849+
await expect(page.locator("[data-delete-confirmation-lane='lead-1']")).toBeVisible();
3850+
expect(await page.evaluate(() => Object.hasOwn(window.__midiStudioV2App.selectedSong().studioArrangement.lanes, "lead-1"))).toBe(true);
3851+
await page.locator("[data-confirm-delete-instrument-row='lead-1']").click();
3852+
await expect(instrumentRow(page, "lead-1")).toHaveCount(0);
3853+
const afterDelete = await page.evaluate(() => {
3854+
const app = window.__midiStudioV2App;
3855+
return {
3856+
hasDeletedLane: Object.hasOwn(app.selectedSong().studioArrangement.lanes, "lead-1"),
3857+
selected: app.instrumentGrid.selectedInstrumentId
3858+
};
3859+
});
3860+
expect(afterDelete).toEqual({ hasDeletedLane: false, selected: expectedSelectionAfterDelete });
3861+
3862+
await page.evaluate(() => {
3863+
const app = window.__midiStudioV2App;
3864+
const song = app.selectedSong();
3865+
const settings = song.studioArrangement.previewLaneSettings;
3866+
song.studioArrangement.lanes = { lead: song.studioArrangement.lanes.lead };
3867+
Object.keys(settings).forEach((key) => {
3868+
if (settings[key] && typeof settings[key] === "object" && !Array.isArray(settings[key])) {
3869+
settings[key] = { lead: settings[key].lead };
3870+
}
3871+
});
3872+
song.studioArrangement.previewInstruments = { lead: settings.instruments.lead };
3873+
app.applySelectedSongArrangement("final instrument delete guard");
3874+
});
3875+
await selectMidiStudioTab(page, "instruments");
3876+
await expect(instrumentRow(page, "lead")).toHaveCount(1);
3877+
await instrumentRow(page, "lead").locator("[data-delete-instrument-row='lead']").click();
3878+
await expect(page.locator("[data-delete-blocked-lane='lead']")).toBeVisible();
3879+
await expect(page.locator("[data-delete-blocked-lane='lead']")).toContainText("Final instrument cannot be deleted");
3880+
expect(await page.evaluate(() => Object.keys(window.__midiStudioV2App.selectedSong().studioArrangement.lanes))).toEqual(["lead"]);
3881+
await expect(page.locator("#statusLog")).toHaveValue(/WARN Final instrument cannot be deleted: Lead\./);
3882+
} finally {
3883+
await workspaceV2CoverageReporter.stop(page);
3884+
await server.close();
3885+
}
3886+
});
3887+
37343888
test("derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song", async ({ page }) => {
37353889
const server = await openMidiStudioForImport(page);
37363890
try {

tools/midi-studio-v2/index.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ <h2 class="tools-platform-frame__eyebrow">First-Class Tools Surface V2</h2>
8383
<span>Instruments</span>
8484
<span class="midi-studio-v2__instrument-header-actions">
8585
<button id="duplicateInstrumentRowButton" class="midi-studio-v2__instrument-header-button midi-studio-v2__duplicate-instrument-button" type="button" aria-label="Duplicate selected instrument" title="Duplicate selected instrument"></button>
86+
<button id="moveInstrumentUpButton" class="midi-studio-v2__instrument-header-button midi-studio-v2__move-instrument-button" type="button" aria-label="Move selected instrument up" title="Move selected instrument up">^</button>
87+
<button id="moveInstrumentDownButton" class="midi-studio-v2__instrument-header-button midi-studio-v2__move-instrument-button" type="button" aria-label="Move selected instrument down" title="Move selected instrument down">v</button>
8688
<button id="timelineAddInstrumentRowButton" class="midi-studio-v2__instrument-header-button" type="button" aria-label="Add instrument" title="Add instrument">Add</button>
8789
<button id="timelineCloseInstrumentPanelButton" class="midi-studio-v2__instrument-header-button" type="button" aria-label="Collapse Octave Timeline Instruments panel" title="Collapse Instruments panel">X</button>
8890
</span>

tools/midi-studio-v2/js/MidiStudioV2App.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,7 @@ export class MidiStudioV2App {
948948
return;
949949
}
950950
this.setCurrentInstrumentGridResult(result);
951-
if (["add-lane", "delete-lane", "duplicate-lane"].includes(detail.action)) {
951+
if (["add-lane", "delete-lane", "duplicate-lane", "move-lane-down", "move-lane-up"].includes(detail.action)) {
952952
this.instrumentGrid.render(result);
953953
} else {
954954
this.instrumentGrid.syncEditedGridResult(result);
@@ -959,6 +959,8 @@ export class MidiStudioV2App {
959959
this.statusLog.ok(`Added instrument row ${detail.laneLabel || detail.lane}; playback data updated.`);
960960
} else if (detail.action === "duplicate-lane") {
961961
this.statusLog.ok(`Duplicated instrument row ${detail.sourceLaneLabel || detail.sourceLane || "instrument"} as ${detail.laneLabel || detail.lane}; playback data updated.`);
962+
} else if (detail.action === "move-lane-up" || detail.action === "move-lane-down") {
963+
this.statusLog.ok(`Moved instrument row ${detail.laneLabel || detail.lane} ${detail.direction || "in order"}; canonical order updated.`);
962964
} else if (detail.action === "delete-lane") {
963965
this.statusLog.ok(`Deleted instrument row ${detail.laneLabel || detail.lane}; playback data updated.`);
964966
} else if (detail.action === "delete-selected-note") {
@@ -1201,6 +1203,21 @@ export class MidiStudioV2App {
12011203
this.updateAudioDiagnostics();
12021204
return;
12031205
}
1206+
if (kind === "delete-confirmation") {
1207+
this.statusLog.warn(`Confirm delete requested for ${detail.laneLabel}. Confirming will remove its notes and instrument settings.`);
1208+
this.updateAudioDiagnostics();
1209+
return;
1210+
}
1211+
if (kind === "delete-blocked") {
1212+
this.statusLog.warn(`Final instrument cannot be deleted: ${detail.laneLabel}.`);
1213+
this.updateAudioDiagnostics();
1214+
return;
1215+
}
1216+
if (kind === "delete-cancelled") {
1217+
this.statusLog.info(`Delete cancelled for ${detail.laneLabel}.`);
1218+
this.updateAudioDiagnostics();
1219+
return;
1220+
}
12041221
if (kind === "volume") {
12051222
this.persistInstrumentSettings("volume");
12061223
this.statusLog.info(`Lane volume set for ${detail.laneLabel}: ${detail.value}.`);

tools/midi-studio-v2/js/bootstrap.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ window.addEventListener("DOMContentLoaded", () => {
104104
leadInput: requireElement("#instrumentGridLeadInput"),
105105
loopEndSelect: requireElement("#instrumentGridLoopEndSelect"),
106106
loopStartSelect: requireElement("#instrumentGridLoopStartSelect"),
107+
moveInstrumentDownButton: requireElement("#moveInstrumentDownButton"),
108+
moveInstrumentUpButton: requireElement("#moveInstrumentUpButton"),
107109
normalizeButton: requireElement("#normalizeInstrumentGridButton"),
108110
padInput: requireElement("#instrumentGridPadInput"),
109111
playLoopButton: requireElement("#playLoopButton"),

0 commit comments

Comments
 (0)