Skip to content

Commit 7ba5765

Browse files
committed
Refine MIDI Studio V2 octave grid density and add proper multi-note chord editing - PR_26146_029-midi-studio-v2-octave-grid-density-and-chords
1 parent 5138bdf commit 7ba5765

5 files changed

Lines changed: 400 additions & 95 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# PR_26146_029 MIDI Studio V2 Octave Grid Density and Chords Validation
2+
3+
Status: PASS
4+
5+
## Scope
6+
7+
- Continued from PR_26146_028.
8+
- Refined the MIDI Studio V2 octave timeline editor density and instrument controls.
9+
- Removed decorative visible note-block spans from octave timeline cells.
10+
- Added additive multi-note timeline editing with `+`-joined cell tokens for notes and drums.
11+
- Expanded parser normalization so simultaneous notes become independent playable timeline events.
12+
- Preserved GM family type/instrument dropdown behavior.
13+
14+
## Changed Files
15+
16+
- `src/engine/audio/InstrumentGridParser.js`
17+
- `tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
18+
- `tools/midi-studio-v2/styles/midiStudioV2.css`
19+
- `tests/playwright/tools/MidiStudioV2.spec.mjs`
20+
- `docs/dev/reports/PR_26146_029-midi-studio-v2-octave-grid-density-and-chords_validation.md`
21+
22+
## Validation
23+
24+
- PASS: `node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
25+
- PASS: `node --check src/engine/audio/InstrumentGridParser.js`
26+
- PASS: `node --check tests/playwright/tools/MidiStudioV2.spec.mjs`
27+
- PASS: `npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs -g "octave timeline editor is the default editable and playable Studio workflow|octave grid density supports icon controls and simultaneous chord editing" --reporter=list --workers=1 --timeout=60000`
28+
- 2 passed.
29+
- PASS: `git diff --check`
30+
- No whitespace errors. Git emitted LF-to-CRLF working-copy warnings for touched text files.
31+
32+
## Required Proof Points
33+
34+
- PASS: octave timeline editor remains visible by default.
35+
- PASS: songs populate the left Songs column from the imported manifest.
36+
- PASS: instrument controls populate the left Instruments column.
37+
- PASS: visibility control uses icon-only button text with accessible labels.
38+
- PASS: mute, solo, eye, and delete controls stay on one horizontal row.
39+
- PASS: octave rows are denser/shorter.
40+
- PASS: visible octave note cells render direct text without `.midi-studio-v2__note-block` wrappers.
41+
- PASS: GM type dropdown and dependent instrument dropdown behavior is preserved.
42+
- PASS: multiple notes can exist in the same bar/beat column.
43+
- PASS: adding one note does not remove sibling notes in the same bar/beat.
44+
- PASS: chords/multi-note cells playback as simultaneous same-time oscillator starts.
45+
- PASS: drum rows support simultaneous same-time events.
46+
- PASS: selected instrument notes stay highlighted and non-selected notes stay dimmed.
47+
- PASS: show/hide eye behavior still removes and restores hidden-lane timeline notes.
48+
- PASS: playhead progression remains aligned to beat/bar columns.
49+
- PASS: Play and Stop work from visible octave timeline data.
50+
51+
## Notes
52+
53+
- Full samples smoke test was not run, per BUILD request.
54+
- SoundFont playback, export rendering, and MIDI recording/input were not implemented, per BUILD request.

src/engine/audio/InstrumentGridParser.js

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,12 @@ export class InstrumentGridParser {
240240
if (lane === "drums" || this.isDrumToken(value)) {
241241
return this.normalizeDrums({ cell, timing, value });
242242
}
243+
if (value.includes("+")) {
244+
return this.normalizeNotes({ cell, timing, value });
245+
}
246+
if (NOTE_PATTERN.test(value)) {
247+
return { cell, events: [this.eventForCell({ cell, kind: "note", value })], ok: true, warnings: [] };
248+
}
243249
if (lane === "chords") {
244250
if (!CHORD_PATTERN.test(value)) {
245251
return { message: `Invalid chord token "${value}" in ${lane} at bar ${timing.bar}, beat ${timing.beat}.`, ok: false };
@@ -249,10 +255,24 @@ export class InstrumentGridParser {
249255
if (lane === "pad" && CHORD_PATTERN.test(value)) {
250256
return { cell, events: [this.eventForCell({ cell, kind: "chord", value })], ok: true, warnings: [] };
251257
}
252-
if (!NOTE_PATTERN.test(value)) {
253-
return { message: `Invalid note token "${value}" in ${lane} at bar ${timing.bar}, beat ${timing.beat}. Use notes like C4 or rests (-).`, ok: false };
258+
return { message: `Invalid note token "${value}" in ${lane} at bar ${timing.bar}, beat ${timing.beat}. Use notes like C4, C4+E4, or rests (-).`, ok: false };
259+
}
260+
261+
normalizeNotes({ cell, timing, value }) {
262+
const parts = value.split("+").map((part) => part.trim()).filter(Boolean);
263+
const invalid = parts.filter((part) => !NOTE_PATTERN.test(part));
264+
if (!parts.length || invalid.length) {
265+
return {
266+
message: `Invalid multi-note token "${value}" in ${cell.lane} at bar ${timing.bar}, beat ${timing.beat}. Use notes like C4+E4+G4.`,
267+
ok: false
268+
};
254269
}
255-
return { cell, events: [this.eventForCell({ cell, kind: "note", value })], ok: true, warnings: [] };
270+
return {
271+
cell,
272+
events: parts.map((part) => this.eventForCell({ cell, kind: "note", value: part })),
273+
ok: true,
274+
warnings: []
275+
};
256276
}
257277

258278
normalizeDrums({ cell, timing, value }) {

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 154 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -353,13 +353,17 @@ function octaveCell(page, rowToken, stepIndex) {
353353
}
354354

355355
function octaveNoteBlock(page, lane) {
356-
return page.locator(`.midi-studio-v2__note-block[data-lane="${lane}"]`);
356+
return page.locator(`.midi-studio-v2__octave-note-cell[data-note-lanes~="${lane}"]`);
357357
}
358358

359359
function instrumentRow(page, lane) {
360360
return page.locator(`.midi-studio-v2__instrument-row[data-lane="${lane}"]`);
361361
}
362362

363+
async function selectInstrumentRow(page, lane) {
364+
await instrumentRow(page, lane).locator(`[data-lane-label="${lane}"]`).click();
365+
}
366+
363367
function instrumentTypeSelect(page, lane) {
364368
return instrumentRow(page, lane).locator("[data-lane-instrument-type-select]");
365369
}
@@ -449,21 +453,24 @@ test.describe("MIDI Studio V2", () => {
449453
]);
450454
await instrumentTypeSelect(page, "lead").selectOption("Synth Lead");
451455
await instrumentSelect(page, "lead").selectOption("retro-pulse-lead");
452-
await instrumentRow(page, "lead").click();
453-
await expect(octaveNoteBlock(page, "lead").first()).toHaveClass(/midi-studio-v2__note-block--selected/);
454-
await expect(octaveNoteBlock(page, "chords").first()).toHaveClass(/midi-studio-v2__note-block--dimmed/);
456+
await selectInstrumentRow(page, "lead");
457+
await expect(octaveNoteBlock(page, "lead").first()).toHaveClass(/midi-studio-v2__grid-cell--lane-selected/);
458+
await expect(octaveNoteBlock(page, "chords").first()).toHaveClass(/midi-studio-v2__grid-cell--lane-dimmed/);
455459
await expect(page.locator(".midi-studio-v2__octave-row-label[data-octave='5']")).not.toHaveCount(0);
456-
await expect(octaveCell(page, "G4", 0).locator('[data-lane="lead"]')).toContainText("G4");
457-
await instrumentRow(page, "bass").click();
458-
await expect(octaveCell(page, "G2", 0).locator('[data-lane="bass"]')).toContainText("G2");
460+
await expect(octaveCell(page, "G4", 0)).toContainText("G4");
461+
await expect(octaveCell(page, "G4", 0)).toHaveAttribute("data-note-lanes", /lead/);
462+
await selectInstrumentRow(page, "bass");
463+
await expect(octaveCell(page, "G2", 0)).toContainText("G2");
464+
await expect(octaveCell(page, "G2", 0)).toHaveAttribute("data-note-lanes", /bass/);
459465
await instrumentRow(page, "bass").locator("[data-toggle-instrument-visibility='bass']").click();
460466
await expect(octaveNoteBlock(page, "bass")).toHaveCount(0);
461467
await expect(page.locator("#statusLog")).toHaveValue(/INFO Lane hidden: Bass\./);
462468
await instrumentRow(page, "bass").locator("[data-toggle-instrument-visibility='bass']").click();
463469
await expect(octaveNoteBlock(page, "bass").first()).toBeVisible();
464-
await instrumentRow(page, "lead").click();
470+
await selectInstrumentRow(page, "lead");
465471
await octaveCell(page, "C5", 1).click();
466-
await expect(octaveCell(page, "C5", 1).locator('[data-lane="lead"]')).toHaveText("C5");
472+
await expect(octaveCell(page, "C5", 1)).toHaveText("C5");
473+
await expect(octaveCell(page, "C5", 1)).toHaveAttribute("data-note-lanes", /lead/);
467474
expect(await page.evaluate(() => window.__midiStudioV2App.lastInstrumentGridResult.timeline.some((event) => event.lane === "lead" && event.value === "C5" && event.stepIndex === 1))).toBe(true);
468475

469476
await page.locator("#playButton").click();
@@ -482,9 +489,11 @@ test.describe("MIDI Studio V2", () => {
482489
await expect(page.locator("#playButton")).toBeEnabled();
483490

484491
await page.locator('[data-song-id="frog-hop-nursery-rhyme"]').click();
485-
await expect(octaveCell(page, "C5", 0).locator('[data-lane="lead"]')).toContainText("C5");
492+
await expect(octaveCell(page, "C5", 0)).toContainText("C5");
493+
await expect(octaveCell(page, "C5", 0)).toHaveAttribute("data-note-lanes", /lead/);
486494
await page.locator('[data-song-id="quiet-village-pad"]').click();
487-
await expect(octaveCell(page, "D5", 0).locator('[data-lane="lead"]')).toContainText("D5");
495+
await expect(octaveCell(page, "D5", 0)).toContainText("D5");
496+
await expect(octaveCell(page, "D5", 0)).toHaveAttribute("data-note-lanes", /lead/);
488497

489498
await selectMidiStudioTab(page, "midi-import");
490499
await page.locator("#midiSourceFileInput").setInputFiles({
@@ -496,9 +505,11 @@ test.describe("MIDI Studio V2", () => {
496505
await expect(page.locator("#songList")).toContainText("Local Import");
497506
await selectMidiStudioTab(page, "studio");
498507
await expect(instrumentRow(page, "track-2-ch-1")).toBeVisible();
499-
await expect(octaveCell(page, "C4", 0).locator('[data-lane="track-2-ch-1"]')).toContainText("C4");
508+
await expect(octaveCell(page, "C4", 0)).toContainText("C4");
509+
await expect(octaveCell(page, "C4", 0)).toHaveAttribute("data-note-lanes", /track-2-ch-1/);
500510
await octaveCell(page, "D4", 1).click();
501-
await expect(octaveCell(page, "D4", 1).locator('[data-lane="track-2-ch-1"]')).toHaveText("D4");
511+
await expect(octaveCell(page, "D4", 1)).toHaveText("D4");
512+
await expect(octaveCell(page, "D4", 1)).toHaveAttribute("data-note-lanes", /track-2-ch-1/);
502513
expect(await page.evaluate(() => window.__midiStudioV2App.selectedSong().studioArrangement.lanes["track-2-ch-1"])).toContain("D4");
503514
await page.evaluate(() => {
504515
window.__midiStudioPreviewSynthEvents = [];
@@ -514,6 +525,136 @@ test.describe("MIDI Studio V2", () => {
514525
}
515526
});
516527

528+
test("octave grid density supports icon controls and simultaneous chord editing", async ({ page }) => {
529+
const server = await openMidiStudioForImport(page);
530+
try {
531+
await page.locator("#toolImportManifestInput").setInputFiles(uatManifestPath);
532+
await selectInstrumentRow(page, "lead");
533+
534+
const leadVisibility = instrumentRow(page, "lead").locator("[data-toggle-instrument-visibility='lead']");
535+
await expect(leadVisibility).toHaveText("");
536+
await expect(leadVisibility).toHaveAttribute("aria-pressed", "true");
537+
await expect(leadVisibility).toHaveAttribute("aria-label", /Hide Lead/);
538+
const controlAlignment = await instrumentRow(page, "lead").locator(".midi-studio-v2__instrument-control-row").evaluate((row) => {
539+
const controls = [
540+
row.querySelector('[aria-label="Mute Lead"]')?.closest("label"),
541+
row.querySelector('[aria-label="Solo Lead"]')?.closest("label"),
542+
row.querySelector("[data-toggle-instrument-visibility='lead']"),
543+
row.querySelector("[data-delete-instrument-row='lead']")
544+
];
545+
if (controls.some((control) => !control)) {
546+
return { maxCenterDelta: 999, missing: true };
547+
}
548+
const centers = controls.map((control) => {
549+
const rect = control.getBoundingClientRect();
550+
return rect.top + rect.height / 2;
551+
});
552+
return {
553+
maxCenterDelta: Math.max(...centers) - Math.min(...centers),
554+
missing: false
555+
};
556+
});
557+
expect(controlAlignment.missing).toBe(false);
558+
expect(controlAlignment.maxCenterDelta).toBeLessThanOrEqual(4);
559+
560+
const octaveCellHeight = await octaveCell(page, "C5", 0).evaluate((cell) => cell.getBoundingClientRect().height);
561+
expect(octaveCellHeight).toBeLessThanOrEqual(32);
562+
await expect(page.locator(".midi-studio-v2__octave-timeline .midi-studio-v2__note-block")).toHaveCount(0);
563+
564+
const chordStep = await page.evaluate(() => {
565+
const result = window.__midiStudioV2App.lastInstrumentGridResult;
566+
const target = new Set(["C5", "E5", "G5"]);
567+
for (let stepIndex = 0; stepIndex < result.totalSteps; stepIndex += 1) {
568+
const leadValues = result.timeline
569+
.filter((event) => event.lane === "lead" && event.kind === "note" && event.stepIndex === stepIndex)
570+
.map((event) => event.value);
571+
if (leadValues.length && leadValues.every((value) => !target.has(value))) {
572+
return stepIndex;
573+
}
574+
}
575+
return 0;
576+
});
577+
const initialLeadValues = await page.evaluate((stepIndex) => window.__midiStudioV2App.lastInstrumentGridResult.timeline
578+
.filter((event) => event.lane === "lead" && event.kind === "note" && event.stepIndex === stepIndex)
579+
.map((event) => event.value), chordStep);
580+
await octaveCell(page, "C5", chordStep).click();
581+
await octaveCell(page, "E5", chordStep).click();
582+
await expect(octaveCell(page, "C5", chordStep)).toHaveText("C5");
583+
await expect(octaveCell(page, "E5", chordStep)).toHaveText("E5");
584+
await octaveCell(page, "G5", chordStep).click();
585+
const leadChordValues = await page.evaluate((stepIndex) => window.__midiStudioV2App.lastInstrumentGridResult.timeline
586+
.filter((event) => event.lane === "lead" && event.kind === "note" && event.stepIndex === stepIndex)
587+
.map((event) => event.value)
588+
.sort(), chordStep);
589+
expect(leadChordValues).toEqual(expect.arrayContaining(["C5", "E5", "G5"]));
590+
expect(leadChordValues).toEqual(expect.arrayContaining(initialLeadValues));
591+
await expect(octaveCell(page, "C5", chordStep)).toHaveClass(/midi-studio-v2__grid-cell--lane-selected/);
592+
await expect(octaveNoteBlock(page, "chords").first()).toHaveClass(/midi-studio-v2__grid-cell--lane-dimmed/);
593+
594+
await leadVisibility.click();
595+
await expect(octaveNoteBlock(page, "lead")).toHaveCount(0);
596+
await leadVisibility.click();
597+
await expect(octaveCell(page, "C5", chordStep)).toHaveAttribute("data-note-lanes", /lead/);
598+
599+
await selectInstrumentRow(page, "drums");
600+
const drumPlan = await page.evaluate(() => {
601+
const result = window.__midiStudioV2App.lastInstrumentGridResult;
602+
const target = ["kick", "snare", "hat"];
603+
for (let stepIndex = 0; stepIndex < result.totalSteps; stepIndex += 1) {
604+
const values = result.timeline
605+
.filter((event) => event.lane === "drums" && event.kind === "drum" && event.stepIndex === stepIndex)
606+
.map((event) => event.value);
607+
const missing = target.filter((value) => !values.includes(value));
608+
if (missing.length) {
609+
return { missing, stepIndex };
610+
}
611+
}
612+
return { missing: [], stepIndex: 0 };
613+
});
614+
for (const rowToken of drumPlan.missing) {
615+
await octaveCell(page, rowToken, drumPlan.stepIndex).click();
616+
}
617+
const drumValues = await page.evaluate((stepIndex) => window.__midiStudioV2App.lastInstrumentGridResult.timeline
618+
.filter((event) => event.lane === "drums" && event.kind === "drum" && event.stepIndex === stepIndex)
619+
.map((event) => event.value)
620+
.sort(), drumPlan.stepIndex);
621+
expect(drumValues).toEqual(expect.arrayContaining(["hat", "kick", "snare"]));
622+
623+
await page.evaluate(() => {
624+
window.__midiStudioPreviewSynthEvents = [];
625+
});
626+
await page.locator("#playButton").click();
627+
await expect(page.locator("#stopButton")).toBeEnabled();
628+
await expect(page.locator('.midi-studio-v2__grid-cell--playhead-active[data-step-index="0"]').first()).toBeVisible();
629+
const playbackEvidence = await page.evaluate(() => {
630+
const maxSameTime = (action) => {
631+
const counts = new Map();
632+
window.__midiStudioPreviewSynthEvents
633+
.filter((event) => event.action === action)
634+
.forEach((event) => {
635+
const key = Number(event.time).toFixed(3);
636+
counts.set(key, (counts.get(key) || 0) + 1);
637+
});
638+
return Math.max(0, ...counts.values());
639+
};
640+
return {
641+
bufferStartsAtSameTime: maxSameTime("buffer-start"),
642+
oscillatorStartsAtSameTime: maxSameTime("oscillator-start")
643+
};
644+
});
645+
expect(playbackEvidence.oscillatorStartsAtSameTime).toBeGreaterThanOrEqual(3);
646+
expect(playbackEvidence.bufferStartsAtSameTime).toBeGreaterThanOrEqual(2);
647+
await page.waitForFunction(() => window.__midiStudioV2App.instrumentGrid.playheadStep > 0);
648+
const activeStep = await page.evaluate(() => window.__midiStudioV2App.instrumentGrid.playheadStep);
649+
await expect(page.locator(`.midi-studio-v2__grid-cell--timing-header.midi-studio-v2__grid-cell--playhead-active[data-step-index="${activeStep}"]`)).toHaveCount(1);
650+
await page.locator("#stopButton").click();
651+
await expect(page.locator("#playButton")).toBeEnabled();
652+
} finally {
653+
await workspaceV2CoverageReporter.stop(page);
654+
await server.close();
655+
}
656+
});
657+
517658
test("imports UAT manifest and plays the upbeat multi-instrument studio arrangement", async ({ page }) => {
518659
const server = await openMidiStudioForImport(page);
519660
try {

0 commit comments

Comments
 (0)