Skip to content

Commit c08e5fc

Browse files
committed
Build MIDI Studio V2 Instruments tab as the authoritative instrument editor - PR_26146_061-midi-studio-v2-instrument-editor-foundation
1 parent 45dca2a commit c08e5fc

8 files changed

Lines changed: 811 additions & 80 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# PR_26146_061 MIDI Studio V2 Instrument Editor Foundation Validation
2+
3+
Status: PASS
4+
5+
## Scope Validated
6+
7+
- Built the Instruments tab into the authoritative editor for selected instrument configuration.
8+
- Added selected-instrument buckets: Identity, Mix, Playback, Effects, and Advanced.
9+
- Kept GM Type and GM Instrument/Patch editing in Instruments only.
10+
- Wired display name, Volume, Pan/Balance, Mute default, Solo default, Octave range, Transpose, Velocity, and Duration into canonical `previewLaneSettings`.
11+
- Kept Effects and Advanced controls red/unwired with Not implemented tooltips/status.
12+
- Preserved the shared `selectedInstrumentId` state used by Instruments and Octave Timeline.
13+
- Preserved Octave Timeline quick controls for select, Mute, Solo, Hide/Show, and note editing without duplicating configuration fields.
14+
- Preserved the horizontal audition keyboard in Instruments only and made it follow the selected instrument octave range.
15+
- Preserved canvas timeline editing, manifest import, multiple songs, Play/Stop, launch NAV, red unwired behavior, and Export tab ownership.
16+
17+
## Validation Commands
18+
19+
```powershell
20+
node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js
21+
node --check tools/midi-studio-v2/js/MidiStudioV2App.js
22+
node --check tools/midi-studio-v2/js/bootstrap.js
23+
node --check tests/playwright/tools/MidiStudioV2.spec.mjs
24+
rg --pcre2 -n '<style\b|<script\b(?![^>]*\bsrc=)|\son[a-z]+\s=|style\s=' tools/midi-studio-v2/index.html; if ($LASTEXITCODE -eq 1) { exit 0 } else { exit $LASTEXITCODE }
25+
npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs --project=playwright -g 'canvas octave timeline edits canonical data|canvas note editing flow|enforces SSoT export ownership|keeps Export tab usable|keeps JSON wording|syncs PR060|builds PR061'
26+
git diff --check
27+
```
28+
29+
## Results
30+
31+
- JS syntax checks: PASS.
32+
- Inline script/style/event handler guard: PASS, no matches.
33+
- Targeted MIDI Studio V2 Playwright set: PASS, 7 passed.
34+
- `git diff --check`: PASS. Git reported line-ending normalization warnings only.
35+
- Full samples smoke test: not run, per PR instruction.
36+
37+
## Playwright Proof Points
38+
39+
- Instruments tab owns editable instrument configuration buckets.
40+
- Octave Timeline quick instruments contain no GM Type/Instrument dropdowns or editable configuration fields.
41+
- `selectedInstrumentId` stays synchronized from Octave Timeline to Instruments and back.
42+
- Audition keyboard remains in Instruments and follows Bass octave range changes.
43+
- Wired instrument controls update canonical song `studioArrangement.previewLaneSettings`.
44+
- Effects and Advanced controls are red/unwired with status/tooltips.
45+
- Canvas note editing, Play, and Stop still work.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# PR_26146_061 MIDI Studio V2 Instrument SSoT Map
2+
3+
## Canonical Instrument Store
4+
5+
- Editable instrument configuration is owned by the selected song's `studioArrangement.previewLaneSettings`.
6+
- `previewLaneSettings.instruments` is the canonical GM/Preview Synth instrument selection map.
7+
- `studioArrangement.previewInstruments` is maintained as a compatibility mirror of `previewLaneSettings.instruments`, not a second editable owner.
8+
- Active selection is shared through `InstrumentGridControl.selectedInstrumentId`; `selectedLane` remains an accessor for older internal call sites.
9+
10+
## Instruments Tab Ownership
11+
12+
| Bucket | Editable values | Canonical path |
13+
| --- | --- | --- |
14+
| Identity | Display name, GM Type, GM Instrument/Patch | `previewLaneSettings.displayNames`, `instrumentTypes`, `instruments` |
15+
| Mix | Volume, Pan/Balance, Mute default, Solo default | `previewLaneSettings.volumes`, `pans`, `muted`, `soloed` |
16+
| Playback | Octave range, Transpose, Velocity, Duration | `previewLaneSettings.octaveRanges`, `transposes`, `velocities`, `durations` |
17+
| Effects | Reverb, Chorus, Delay, Filter, Brightness/Tone | Red/unwired placeholders only; no data mutation |
18+
| Advanced | MIDI Channel, GM Program, Controller Values | Red/unwired placeholders only; no data mutation |
19+
20+
## Octave Timeline Ownership
21+
22+
- Octave Timeline owns canvas note editing and quick performance/edit controls only.
23+
- Timeline quick controls may select the active instrument and toggle Mute, Solo, and Hide/Show.
24+
- Timeline quick controls do not render GM Type, GM Instrument/Patch, display name, Volume, Pan, Octave range, Transpose, Velocity, Duration, Effects, or Advanced fields.
25+
- Hidden/visible state remains part of `previewLaneSettings.visible` because it directly affects timeline rendering and playback filtering.
26+
27+
## Audition Keyboard
28+
29+
- The horizontal audition keyboard is rendered only inside the Instruments tab.
30+
- It reads the current selected instrument from `selectedInstrumentId`.
31+
- It reads octave limits from `previewLaneSettings.octaveRanges`, falling back to the selected GM family defaults.
32+
- Key clicks audition through Preview Synth using the selected instrument mapping.
33+
34+
## Diagnostics And Export
35+
36+
- Diagnostics remains read-only/derived for instrument configuration.
37+
- Export remains the owner of rendered output controls.
38+
- No editable instrument configuration fields were added to Diagnostics, Export, Song Setup, MIDI Import, or Auto-Create Parts.

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 158 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,14 @@ function instrumentRow(page, lane) {
511511
return page.locator(`.midi-studio-v2__instrument-row[data-lane="${lane}"]`);
512512
}
513513

514+
function instrumentLaneDomId(lane) {
515+
return String(lane || "")
516+
.split(/[^A-Za-z0-9]+/)
517+
.filter(Boolean)
518+
.map((part) => `${part.charAt(0).toUpperCase()}${part.slice(1)}`)
519+
.join("");
520+
}
521+
514522
async function selectInstrumentRow(page, lane) {
515523
await instrumentRow(page, lane).locator(`[data-lane-label="${lane}"]`).click();
516524
}
@@ -520,11 +528,11 @@ function timelineQuickInstrumentRow(page, lane) {
520528
}
521529

522530
function instrumentTypeSelect(page, lane) {
523-
return instrumentRow(page, lane).locator("[data-lane-instrument-type-select]");
531+
return page.locator(`#previewInstrumentType${instrumentLaneDomId(lane)}Select`);
524532
}
525533

526534
function instrumentSelect(page, lane) {
527-
return instrumentRow(page, lane).locator("[data-lane-instrument-select]");
535+
return page.locator(`#previewInstrument${instrumentLaneDomId(lane)}Select`);
528536
}
529537

530538
function instrumentToggle(page, lane, kind) {
@@ -556,6 +564,20 @@ async function controlColors(page, selector) {
556564
});
557565
}
558566

567+
async function setInputValue(page, selector, value, eventType = "input") {
568+
await page.locator(selector).evaluate((input, { dispatchType, nextValue }) => {
569+
input.value = String(nextValue);
570+
input.dispatchEvent(new Event(dispatchType, { bubbles: true }));
571+
}, { dispatchType: eventType, nextValue: value });
572+
}
573+
574+
async function setCheckboxValue(page, selector, checked) {
575+
await page.locator(selector).evaluate((input, nextChecked) => {
576+
input.checked = Boolean(nextChecked);
577+
input.dispatchEvent(new Event("change", { bubbles: true }));
578+
}, checked);
579+
}
580+
559581
async function prepareInstrumentScrollSentinel(page) {
560582
await page.evaluate(() => {
561583
const leftPanel = document.querySelector(".tool-starter__panel--left");
@@ -808,19 +830,21 @@ test.describe("MIDI Studio V2", () => {
808830
expect(keyboardAxisPixel[3]).toBeGreaterThan(0);
809831
expect(keyboardAxisPixel.slice(0, 3).some((channel) => channel > 0)).toBe(true);
810832

811-
await clickCanvasCell(page, "C6", 2);
812-
await expect(page.locator("#statusLog")).toHaveValue(/OK Painted C6 for Lead across the timeline; playback data updated\./);
813-
const canonicalEdit = await page.evaluate(() => {
833+
const editableTarget = await emptyCanvasRun(page, { lane: "lead", length: 1 });
834+
expect(editableTarget).toBeTruthy();
835+
await clickCanvasCell(page, editableTarget.rowToken, editableTarget.stepIndex);
836+
await expect(page.locator("#statusLog")).toHaveValue(/OK Painted .* for Lead across the timeline; playback data updated\./);
837+
const canonicalEdit = await page.evaluate((target) => {
814838
const app = window.__midiStudioV2App;
815839
return {
816-
gridHasEdit: app.lastInstrumentGridResult.timeline.some((event) => event.lane === "lead" && event.stepIndex === 2 && event.value === "C6"),
840+
gridHasEdit: app.lastInstrumentGridResult.timeline.some((event) => event.lane === "lead" && event.stepIndex === target.stepIndex && event.value === target.rowToken),
817841
leadLane: app.selectedSong().studioArrangement.lanes.lead,
818842
selectedCell: app.instrumentGrid.timelineCanvasState().selectedCell
819843
};
820-
});
844+
}, editableTarget);
821845
expect(canonicalEdit.gridHasEdit).toBe(true);
822-
expect(canonicalEdit.leadLane).toContain("C6");
823-
expect(canonicalEdit.selectedCell).toEqual({ rowToken: "C6", stepIndex: 2 });
846+
expect(canonicalEdit.leadLane).toContain(editableTarget.rowToken);
847+
expect(canonicalEdit.selectedCell).toEqual({ rowToken: editableTarget.rowToken, stepIndex: editableTarget.stepIndex });
824848

825849
await page.locator("#instrumentGridOutput").evaluate((output) => {
826850
window.__midiStudioGridClassMutations = [];
@@ -850,7 +874,7 @@ test.describe("MIDI Studio V2", () => {
850874
await page.locator("#playButton").click();
851875
await expect(page.locator("#stopButton")).toBeEnabled();
852876
await expect(page.locator("#playButton")).toBeDisabled();
853-
expect(await page.evaluate(() => window.__midiStudioV2App.__lastPreviewGridValues)).toContain("C6");
877+
expect(await page.evaluate(() => window.__midiStudioV2App.__lastPreviewGridValues)).toContain(editableTarget.rowToken);
854878
expect(await page.evaluate(() => window.__midiStudioPreviewSynthEvents.some((event) => event.action === "oscillator-start"))).toBe(true);
855879
await page.waitForFunction(() => window.__midiStudioV2App.instrumentGrid.playheadStep > 0);
856880
const playbackCanvasState = await canvasTimelineState(page);
@@ -2420,26 +2444,26 @@ test.describe("MIDI Studio V2", () => {
24202444
await expect(page.locator("#instrumentAuditionKeyboard")).not.toHaveAttribute("data-midi-studio-unwired");
24212445
await expect(page.locator("#previewVolumeLeadInput")).toBeVisible();
24222446
await expect(page.locator("#previewPanLeadInput")).toBeVisible();
2447+
await expect(page.locator("#previewOctaveLowLeadInput")).toBeVisible();
2448+
await expect(page.locator("#previewTransposeLeadInput")).toBeVisible();
24232449
const instrumentFutureControls = await page.locator('[data-midi-studio-tab-panel="instruments"] [data-midi-studio-future-control]').evaluateAll((controls) => controls.map((control) => ({
2450+
label: (control.getAttribute("aria-label") || control.placeholder || control.textContent).replace(/\s*\(Not implemented\)$/, "").trim(),
24242451
disabled: control.disabled,
24252452
status: control.dataset.midiStudioUnwired,
24262453
text: control.textContent.trim(),
24272454
title: control.title
24282455
})));
2429-
expect(instrumentFutureControls).toHaveLength(11);
2456+
expect(instrumentFutureControls).toHaveLength(8);
24302457
expect(instrumentFutureControls.every((control) => control.disabled && control.status === "not-implemented" && control.title.includes("Not implemented:"))).toBe(true);
2431-
expect(instrumentFutureControls.map((control) => control.text)).toEqual(expect.arrayContaining([
2432-
"Effects",
2458+
expect(instrumentFutureControls.map((control) => control.label)).toEqual(expect.arrayContaining([
24332459
"Reverb",
24342460
"Chorus",
24352461
"Delay",
24362462
"Filter",
24372463
"Brightness/Tone",
2438-
"Octave Range",
2439-
"Transpose",
2440-
"Velocity",
2441-
"Duration",
2442-
"Instrument Settings"
2464+
"MIDI Channel",
2465+
"GM Program",
2466+
"Controller Values"
24432467
]));
24442468
await selectInstrumentRow(page, "lead");
24452469
await page.evaluate(() => {
@@ -2711,6 +2735,122 @@ test.describe("MIDI Studio V2", () => {
27112735
}
27122736
});
27132737

2738+
test("builds PR061 instrument editor buckets from selectedInstrumentId", async ({ page }) => {
2739+
const server = await openMidiStudioForImport(page);
2740+
try {
2741+
await page.locator("#toolImportManifestInput").setInputFiles(uatManifestPath);
2742+
2743+
await selectMidiStudioTab(page, "studio");
2744+
await waitForCanvasRender(page);
2745+
await expect(page.locator("#timelineInstrumentQuickList")).toBeVisible();
2746+
await expect(timelineQuickInstrumentRow(page, "bass")).toBeVisible();
2747+
await expect(timelineQuickInstrumentRow(page, "bass").locator("[data-timeline-quick-mute='bass']")).toBeVisible();
2748+
await expect(timelineQuickInstrumentRow(page, "bass").locator("[data-timeline-quick-solo='bass']")).toBeVisible();
2749+
await expect(timelineQuickInstrumentRow(page, "bass").locator("[data-toggle-instrument-visibility='bass']")).toBeVisible();
2750+
await expect(page.locator("#timelineInstrumentQuickList select, #timelineInstrumentQuickList input:not([type='checkbox'])")).toHaveCount(0);
2751+
await timelineQuickInstrumentRow(page, "bass").click();
2752+
expect(await page.evaluate(() => window.__midiStudioV2App.instrumentGrid.selectedInstrumentId)).toBe("bass");
2753+
2754+
await selectMidiStudioTab(page, "instruments");
2755+
await expect(instrumentRow(page, "bass")).toHaveClass(/is-selected/);
2756+
const editor = page.locator("#selectedInstrumentEditor");
2757+
await expect(editor).toHaveAttribute("data-selected-instrument-id", "bass");
2758+
await expect(editor.locator("[data-instrument-editor-bucket='identity']")).toContainText("Identity");
2759+
await expect(editor.locator("[data-instrument-editor-bucket='mix']")).toContainText("Mix");
2760+
await expect(editor.locator("[data-instrument-editor-bucket='playback']")).toContainText("Playback");
2761+
await expect(editor.locator("[data-instrument-editor-bucket='effects']")).toContainText("Effects");
2762+
await expect(editor.locator("[data-instrument-editor-bucket='advanced']")).toContainText("Advanced");
2763+
2764+
await page.locator("#previewDisplayNameBassInput").fill("Bass Anchor");
2765+
await instrumentTypeSelect(page, "bass").selectOption("Bass");
2766+
await instrumentSelect(page, "bass").selectOption("gm-electric-bass-finger");
2767+
await setInputValue(page, "#previewVolumeBassInput", "0.65");
2768+
await setInputValue(page, "#previewPanBassInput", "-0.4");
2769+
await setCheckboxValue(page, "#previewMuteBassToggle", true);
2770+
expect(await page.evaluate(() => window.__midiStudioV2App.selectedSong().studioArrangement.previewLaneSettings.muted.bass)).toBe(true);
2771+
await setCheckboxValue(page, "#previewMuteBassToggle", false);
2772+
await setCheckboxValue(page, "#previewSoloBassToggle", true);
2773+
await setInputValue(page, "#previewOctaveLowBassInput", "2");
2774+
await setInputValue(page, "#previewOctaveHighBassInput", "4");
2775+
await setInputValue(page, "#previewTransposeBassInput", "12");
2776+
await setInputValue(page, "#previewVelocityBassInput", "96");
2777+
await setInputValue(page, "#previewDurationBassInput", "1.5");
2778+
2779+
expect(await page.evaluate(() => {
2780+
const app = window.__midiStudioV2App;
2781+
const settings = app.selectedSong().studioArrangement.previewLaneSettings;
2782+
return {
2783+
displayName: settings.displayNames.bass,
2784+
duration: settings.durations.bass,
2785+
instrument: settings.instruments.bass,
2786+
instrumentType: settings.instrumentTypes.bass,
2787+
muted: settings.muted.bass,
2788+
octaveRange: settings.octaveRanges.bass,
2789+
pan: settings.pans.bass,
2790+
selectedInstrumentId: app.instrumentGrid.selectedInstrumentId,
2791+
soloed: settings.soloed.bass,
2792+
transpose: settings.transposes.bass,
2793+
velocity: settings.velocities.bass,
2794+
volume: settings.volumes.bass
2795+
};
2796+
})).toEqual({
2797+
displayName: "Bass Anchor",
2798+
duration: 1.5,
2799+
instrument: "gm-electric-bass-finger",
2800+
instrumentType: "Bass",
2801+
muted: false,
2802+
octaveRange: { high: 4, low: 2 },
2803+
pan: -0.4,
2804+
selectedInstrumentId: "bass",
2805+
soloed: true,
2806+
transpose: 12,
2807+
velocity: 96,
2808+
volume: 0.65
2809+
});
2810+
2811+
const keyboard = page.locator("#instrumentAuditionKeyboard");
2812+
await expect(keyboard).toBeVisible();
2813+
await expect(keyboard).toHaveAttribute("data-selected-lane", "bass");
2814+
await expect(keyboard).toHaveAttribute("data-octave-min", "2");
2815+
await expect(keyboard).toHaveAttribute("data-octave-max", "4");
2816+
expect(await keyboard.locator("[data-audition-note]").count()).toBe(36);
2817+
await expect(keyboard.locator("[data-audition-note='C1']")).toHaveCount(0);
2818+
await expect(keyboard.locator("[data-audition-note='C4']")).toHaveCount(1);
2819+
2820+
const effectControls = await editor.locator("[data-instrument-editor-bucket='effects'] [data-midi-studio-future-control]").evaluateAll((controls) => controls.map((control) => ({
2821+
label: (control.getAttribute("aria-label") || control.placeholder).replace(/\s*\(Not implemented\)$/, "").trim(),
2822+
status: control.dataset.midiStudioUnwired,
2823+
title: control.title
2824+
})));
2825+
expect(effectControls).toHaveLength(5);
2826+
expect(effectControls.map((control) => control.label)).toEqual(["Reverb", "Chorus", "Delay", "Filter", "Brightness/Tone"]);
2827+
expect(effectControls.every((control) => control.status === "not-implemented" && control.title.includes("Not implemented:"))).toBe(true);
2828+
const advancedControls = await editor.locator("[data-instrument-editor-bucket='advanced'] [data-midi-studio-future-control]").evaluateAll((controls) => controls.map((control) => ({
2829+
label: (control.getAttribute("aria-label") || control.placeholder).replace(/\s*\(Not implemented\)$/, "").trim(),
2830+
status: control.dataset.midiStudioUnwired,
2831+
title: control.title
2832+
})));
2833+
expect(advancedControls).toHaveLength(3);
2834+
expect(advancedControls.map((control) => control.label)).toEqual(["MIDI Channel", "GM Program", "Controller Values"]);
2835+
expect(advancedControls.every((control) => control.status === "not-implemented" && control.title.includes("Not implemented:"))).toBe(true);
2836+
2837+
await selectInstrumentRow(page, "lead");
2838+
expect(await page.evaluate(() => window.__midiStudioV2App.instrumentGrid.selectedInstrumentId)).toBe("lead");
2839+
await selectMidiStudioTab(page, "studio");
2840+
await expect(timelineQuickInstrumentRow(page, "lead")).toHaveClass(/is-selected/);
2841+
2842+
await page.locator("#playButton").click();
2843+
await expect(page.locator("#playButton")).toBeDisabled();
2844+
await expect(page.locator("#stopButton")).toBeEnabled();
2845+
await page.locator("#stopButton").click();
2846+
await expect(page.locator("#stopButton")).toBeDisabled();
2847+
await expect(page.locator("#playButton")).toBeEnabled();
2848+
} finally {
2849+
await workspaceV2CoverageReporter.stop(page);
2850+
await server.close();
2851+
}
2852+
});
2853+
27142854
test("derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song", async ({ page }) => {
27152855
const server = await openMidiStudioForImport(page);
27162856
try {

0 commit comments

Comments
 (0)