Skip to content

Commit 6875965

Browse files
committed
Move MIDI import controls to the MIDI Import tab and reduce missing-section warning noise - PR_26146_046-midi-studio-v2-tab-cleanup-and-section-warning-repair
1 parent 0630c0e commit 6875965

8 files changed

Lines changed: 192 additions & 33 deletions

File tree

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# PR_26146_046 MIDI Studio V2 Tab Cleanup And Section Warning Repair Validation
2+
3+
Status: PASS
4+
5+
## Scope
6+
- Continued from PR_26146_045.
7+
- Moved MIDI source/import/inspect controls out of the Songs/Selected Song Details workflow into a dedicated MIDI Import accordion on the MIDI Import tab.
8+
- Removed Source MIDI and Instrument set editable rows from Selected Song Details; their display now belongs to MIDI Import.
9+
- Preserved canonical selected song model behavior from PR045.
10+
- Repaired quick section warning noise by disabling unavailable preset buttons and showing compact inline guidance instead of repeated WARN log entries.
11+
- Preserved valid section selection, loop-region feedback, Play Section, Play Loop, Studio octave timeline, piano keyboard, playback, Play/Stop, GM instrument controls, export Type + Save, and roadmap status rules.
12+
13+
## Changed-File Syntax Checks
14+
PASS:
15+
16+
```text
17+
node --check tools/midi-studio-v2/js/MidiStudioV2App.js
18+
node --check tools/midi-studio-v2/js/bootstrap.js
19+
node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js
20+
node --check tools/midi-studio-v2/js/controls/SongDetailsControl.js
21+
node --check tests/playwright/tools/MidiStudioV2.spec.mjs
22+
```
23+
24+
Additional guards:
25+
26+
```text
27+
HTML external-only guard passed
28+
CSS brace check passed
29+
```
30+
31+
## Targeted Playwright Validation
32+
PASS:
33+
34+
```text
35+
npx.cmd playwright test tests/playwright/tools/MidiStudioV2.spec.mjs -g "imports a local MIDI source from the MIDI Import tab without default HTTP 404 noise|keeps selected song details editable and nests Song Sheet under the details panel|derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song|renders timing ruler, section navigation, and loop region visualization|reports invalid section and invalid loop handling|fast octave note editing supports drag painting keyboard shortcuts selection and timeline scroll sync" --config=codex_playwright_system_chrome.config.cjs --reporter=list --workers=1 --timeout=60000
36+
```
37+
38+
Result:
39+
40+
```text
41+
Running 6 tests using 1 worker
42+
ok 1 fast octave note editing supports drag painting keyboard shortcuts selection and timeline scroll sync
43+
ok 2 imports a local MIDI source from the MIDI Import tab without default HTTP 404 noise
44+
ok 3 keeps selected song details editable and nests Song Sheet under the details panel
45+
ok 4 derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song
46+
ok 5 renders timing ruler, section navigation, and loop region visualization
47+
ok 6 reports invalid section and invalid loop handling
48+
6 passed
49+
```
50+
51+
## Required Assertions Covered
52+
- PASS: MIDI source/import/inspect controls are visible on the MIDI Import tab.
53+
- PASS: Songs/Selected Song Details no longer contain MIDI import controls.
54+
- PASS: selected song details remain editable for title, BPM, key, style, loop settings, runtime format, tags, and song id.
55+
- PASS: missing Intro/Loop/Bridge/Boss/Victory quick sections are visibly disabled or unavailable without repeated WARN log spam.
56+
- PASS: unavailable sections show compact inline guidance.
57+
- PASS: valid sections still update selected section and loop-region feedback.
58+
- PASS: Play Section and Play Loop still use valid selected ranges.
59+
- PASS: Play and Stop still work through the preserved fast octave editing/playback test.
60+
61+
## Diff Hygiene
62+
PASS:
63+
64+
```text
65+
git diff --check
66+
```
67+
68+
Git reported line-ending normalization warnings for touched files, but no whitespace errors.
69+
70+
## Not Run
71+
- Full samples smoke test was not run, per PR instructions.
72+
73+
## Result
74+
PR_26146_046 is marked PASS. MIDI import controls are no longer duplicated in Songs/Selected Song Details, and quick section buttons no longer create repeated missing-section warning spam.

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,15 +1318,18 @@ test.describe("MIDI Studio V2", () => {
13181318
await page.locator("#toolImportManifestInput").setInputFiles(uatManifestPath);
13191319

13201320
await expect(page.locator("#statusLog")).toHaveValue(/OK Loaded 3 MIDI songs from Import JSON Manifest:.*uat-midi-studio-v2\.game\.manifest\.json via manifest\.music\./);
1321-
await expect(page.locator('[data-midi-studio-tab="midi-import"]')).toHaveCount(0);
1321+
await expect(page.locator('[data-midi-studio-tab="midi-import"]')).toHaveCount(1);
13221322
await expect(page.locator('[data-midi-studio-tab="export"]')).toHaveCount(0);
13231323
await expect(page.locator(".midi-studio-v2__advanced-song-sheet")).toHaveCount(0);
13241324
await expect(page.locator("#songSheetInput")).toHaveCount(0);
13251325
await expect(page.locator("#parseRawSongSheetButton")).toHaveCount(0);
13261326
await expect(page.locator("#midiSourceFileInput")).toHaveAttribute("accept", /\.mid,.midi/);
1327+
await selectMidiStudioTab(page, "midi-import");
1328+
await expect(page.locator("#midiImportContent")).toBeVisible();
1329+
await expect(page.locator("#songDetailsContent #midiImportContent")).toHaveCount(0);
1330+
await expect(page.locator("#midiSourceDetails")).toContainText("No MIDI source inspected.");
13271331
await selectMidiStudioTab(page, "song-setup");
1328-
await expect(page.locator("#songDetailsContent #midiImportContent")).toBeVisible();
1329-
await expect(page.locator("#songDetailsContent #midiSourceDetails")).toContainText("No MIDI source inspected.");
1332+
await expect(page.locator("#midiImportContent")).toBeHidden();
13301333
await expect(page.locator("#statusLog")).not.toHaveValue(/HTTP 404/);
13311334
await expect(page.locator("#songList [data-song-id]")).toHaveCount(3);
13321335
await expect(page.locator("#songList")).toContainText("Camptown Races UAT Reel");
@@ -1468,13 +1471,16 @@ test.describe("MIDI Studio V2", () => {
14681471
}
14691472
});
14701473

1471-
test("imports a local MIDI source from Selected Song Details without default HTTP 404 noise", async ({ page }) => {
1474+
test("imports a local MIDI source from the MIDI Import tab without default HTTP 404 noise", async ({ page }) => {
14721475
const server = await openMidiStudioForImport(page);
14731476
try {
1474-
await expect(page.locator('[data-midi-studio-tab="midi-import"]')).toHaveCount(0);
1477+
await expect(page.locator('[data-midi-studio-tab="midi-import"]')).toHaveCount(1);
14751478
await expect(page.locator('[data-midi-studio-tab="export"]')).toHaveCount(0);
1476-
await selectMidiStudioTab(page, "song-setup");
1477-
await expect(page.locator("#songDetailsContent #midiImportContent")).toBeVisible();
1479+
await selectMidiStudioTab(page, "midi-import");
1480+
await expect(page.locator("#midiImportContent")).toBeVisible();
1481+
await expect(page.locator("#songDetailsContent #midiImportContent")).toHaveCount(0);
1482+
await expect(page.locator("#songDetails input[data-song-detail-field='sourceMidi']")).toHaveCount(0);
1483+
await expect(page.locator("#songDetails input[data-song-detail-field='instrumentSet']")).toHaveCount(0);
14781484
await expect(page.locator("#midiSourceFileInput")).toHaveAttribute("accept", /\.mid,.midi/);
14791485
await expect(page.locator("#statusLog")).not.toHaveValue(/HTTP 404/);
14801486

@@ -1490,19 +1496,21 @@ test.describe("MIDI Studio V2", () => {
14901496
await expect(page.locator("#songList")).toContainText("Local Import");
14911497
await expect(page.locator("#songSourceField")).toHaveValue("local-import.midi");
14921498
await expect(page.locator("#statusLog")).toHaveValue(/OK Imported MIDI source local-import\.midi: format 1, 2 tracks\./);
1493-
await expect(page.locator("#statusLog")).toHaveValue(/WARN MIDI note conversion to editable timeline rows is not implemented yet\./);
1499+
await expect(page.locator("#statusLog")).toHaveValue(/OK Normalized 1 MIDI note into editable octave timeline data\./);
14941500
await expect(page.locator("#statusLog")).not.toHaveValue(/HTTP 404/);
14951501
expect(await page.evaluate(() => {
14961502
const song = window.__midiStudioV2App.selectedSong();
14971503
return {
14981504
activeSongId: window.__midiStudioV2App.payload.activeSongId,
14991505
hasArrangement: Boolean(song.studioArrangement),
1506+
importedNoteCount: song.studioArrangement?.importedNoteCount,
15001507
name: song.name,
15011508
sourceMidi: song.sourceMidi
15021509
};
15031510
})).toEqual({
15041511
activeSongId: "local-import",
1505-
hasArrangement: false,
1512+
hasArrangement: true,
1513+
importedNoteCount: 1,
15061514
name: "Local Import",
15071515
sourceMidi: "local-import.midi"
15081516
});
@@ -1529,8 +1537,13 @@ test.describe("MIDI Studio V2", () => {
15291537
await expect(page.locator("#songDetails input[data-song-detail-field='loopEnabled']")).toBeChecked();
15301538
await expect(page.locator("#songDetails input[data-song-detail-field='loopStartSeconds']")).toHaveValue("0");
15311539
await expect(page.locator("#songDetails input[data-song-detail-field='loopEndSeconds']")).toHaveValue("14");
1532-
await expect(page.locator("#songDetails input[data-song-detail-field='sourceMidi']")).toHaveValue("assets/music/midi/camptown-races-uat-reel.mid");
1540+
await expect(page.locator("#songDetails input[data-song-detail-field='sourceMidi']")).toHaveCount(0);
1541+
await expect(page.locator("#songDetails input[data-song-detail-field='instrumentSet']")).toHaveCount(0);
15331542
await expect(page.locator("#songDetails input[data-song-detail-field='tags']")).toHaveValue("uat, upbeat, traditional, public-domain");
1543+
await selectMidiStudioTab(page, "midi-import");
1544+
await expect(page.locator("#songSourceField")).toHaveValue("assets/music/midi/camptown-races-uat-reel.mid");
1545+
await expect(page.locator("#instrumentSetField")).toHaveValue("Preview Synth public-domain UAT band");
1546+
await selectMidiStudioTab(page, "song-setup");
15341547

15351548
await page.locator("#songDetails input[data-song-detail-field='name']").fill("Edited UAT Reel");
15361549
await expect(page.locator("#nowPlayingLabel")).toHaveText("Selected: Edited UAT Reel");
@@ -2303,6 +2316,8 @@ test.describe("MIDI Studio V2", () => {
23032316
await expect(page.locator(".midi-studio-v2__grid-cell--bar")).toHaveCount(5);
23042317
await expect(page.locator(".midi-studio-v2__grid-cell--ruler").first()).toContainText("1");
23052318
await expect(page.locator(".midi-studio-v2__grid-cell--beat-header.midi-studio-v2__grid-cell--playhead-active")).toHaveAttribute("data-section", "intro");
2319+
await expect(page.locator("#instrumentGridSectionAvailability")).toHaveText("Quick sections available.");
2320+
await expect(page.locator(".midi-studio-v2__section-preset:disabled")).toHaveCount(0);
23062321
await expect(page.locator("#instrumentGridSectionSelect")).toContainText("bridge");
23072322
await page.locator("#instrumentGridLoopStartSelect").selectOption("loop");
23082323
await page.locator("#instrumentGridLoopEndSelect").selectOption("boss");
@@ -2497,8 +2512,18 @@ test.describe("MIDI Studio V2", () => {
24972512
try {
24982513
await fillInstrumentGrid(page);
24992514
await page.locator("#normalizeInstrumentGridButton").click();
2500-
await page.locator('[data-section-preset="bridge"]').click();
2501-
await expect(page.locator("#statusLog")).toHaveValue(/WARN section Bridge does not exist\. Normalize a section map containing that label or choose a listed custom section\./);
2515+
await expect(page.locator('[data-section-preset="bridge"]')).toBeDisabled();
2516+
await expect(page.locator('[data-section-preset="bridge"]')).toHaveClass(/is-unavailable/);
2517+
await expect(page.locator("#instrumentGridSectionAvailability")).toContainText("Section not available: Bridge, Boss, Victory.");
2518+
const warningCountBefore = await page.locator("#statusLog").evaluate((log) => (log.value.match(/WARN section Bridge does not exist/g) || []).length);
2519+
await page.locator('[data-section-preset="bridge"]').evaluate((button) => {
2520+
button.click();
2521+
button.click();
2522+
button.click();
2523+
});
2524+
await expect(page.locator("#instrumentGridTransportState")).not.toContainText("Section not available: Bridge");
2525+
const warningCountAfter = await page.locator("#statusLog").evaluate((log) => (log.value.match(/WARN section Bridge does not exist/g) || []).length);
2526+
expect(warningCountAfter).toBe(warningCountBefore);
25022527
await page.locator("#instrumentGridLoopStartSelect").selectOption("loop");
25032528
await page.locator("#instrumentGridLoopEndSelect").selectOption("intro");
25042529
await page.locator("#playLoopButton").click();

tools/midi-studio-v2/index.html

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -84,25 +84,30 @@ <h2 class="tools-platform-frame__eyebrow">First-Class Tools Surface V2</h2>
8484
</button>
8585
<div id="songListContent" class="accordion-v2__content">
8686
<div id="songList" class="midi-studio-v2__song-list" role="listbox" aria-label="MIDI songs"></div>
87-
<section id="midiImportContent" class="midi-studio-v2__stack midi-studio-v2__midi-import-panel" aria-label="MIDI Import">
88-
<h3>MIDI Import</h3>
89-
<div class="midi-studio-v2__midi-import-grid">
90-
<div class="tool-starter__field tool-starter__field--not-editable">
91-
<span>.mid instruction source</span>
92-
<input id="songSourceField" type="text" readonly value="No song selected">
93-
</div>
94-
<div class="tool-starter__field tool-starter__field--not-editable">
95-
<span>Instrument set</span>
96-
<input id="instrumentSetField" type="text" readonly value="No song selected">
97-
</div>
87+
</div>
88+
</section>
89+
<section class="accordion-v2 tool-starter__accordion is-open" data-accordion-v2-open="true" data-midi-studio-tab-panel="midi-import">
90+
<button class="accordion-v2__header" type="button" aria-expanded="true" aria-controls="midiImportContent">
91+
<span>MIDI Import</span>
92+
<span class="accordion-v2__icon" aria-hidden="true">+</span>
93+
</button>
94+
<div id="midiImportContent" class="accordion-v2__content midi-studio-v2__stack midi-studio-v2__midi-import-panel">
95+
<div class="midi-studio-v2__midi-import-grid">
96+
<div class="tool-starter__field tool-starter__field--not-editable">
97+
<span>.mid instruction source</span>
98+
<input id="songSourceField" type="text" readonly value="No song selected">
9899
</div>
99-
<div class="midi-studio-v2__midi-import-actions">
100-
<button id="importMidiSourceButton" type="button">Import MIDI Source</button>
101-
<button id="inspectMidiSourceButton" type="button" disabled>Inspect MIDI Source</button>
102-
<input id="midiSourceFileInput" type="file" accept=".mid,.midi,audio/midi,audio/x-midi" hidden>
100+
<div class="tool-starter__field tool-starter__field--not-editable">
101+
<span>Instrument set</span>
102+
<input id="instrumentSetField" type="text" readonly value="No song selected">
103103
</div>
104-
<dl id="midiSourceDetails" class="midi-studio-v2__details"></dl>
105-
</section>
104+
</div>
105+
<div class="midi-studio-v2__midi-import-actions">
106+
<button id="importMidiSourceButton" type="button">Import MIDI Source</button>
107+
<button id="inspectMidiSourceButton" type="button" disabled>Inspect MIDI Source</button>
108+
<input id="midiSourceFileInput" type="file" accept=".mid,.midi,audio/midi,audio/x-midi" hidden>
109+
</div>
110+
<dl id="midiSourceDetails" class="midi-studio-v2__details"></dl>
106111
</div>
107112
</section>
108113
<section class="accordion-v2 tool-starter__accordion is-open" data-accordion-v2-open="true" data-midi-studio-tab-panel="studio instruments">
@@ -272,6 +277,7 @@ <h2 id="instrumentGridHeading">Octave Timeline</h2>
272277
<button class="midi-studio-v2__section-preset" type="button" data-section-preset="boss">Boss</button>
273278
<button class="midi-studio-v2__section-preset" type="button" data-section-preset="victory">Victory</button>
274279
</div>
280+
<p id="instrumentGridSectionAvailability" class="midi-studio-v2__section-availability" aria-live="polite"></p>
275281
<div class="midi-studio-v2__transport-fields">
276282
<label class="tool-starter__field" for="instrumentGridSectionSelect">
277283
<span>Custom section</span>

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export class MidiStudioV2App {
3939
this.manifestLoader = manifestLoader;
4040
this.midiSourceDetails = midiSourceDetails;
4141
this.midiSourceInspection = midiSourceInspection;
42+
this.missingSectionWarnings = new Set();
43+
this.missingSectionWarningKey = "";
4244
this.payload = null;
4345
this.playback = playback;
4446
this.playbackControl = playbackControl;
@@ -281,6 +283,7 @@ export class MidiStudioV2App {
281283
if (!song) {
282284
return;
283285
}
286+
this.resetMissingSectionWarnings(result);
284287
if (result?.ok) {
285288
this.instrumentGridResults.set(song, result);
286289
return;
@@ -293,6 +296,16 @@ export class MidiStudioV2App {
293296
if (song) {
294297
this.instrumentGridResults.delete(song);
295298
}
299+
this.resetMissingSectionWarnings(null);
300+
}
301+
302+
resetMissingSectionWarnings(result) {
303+
const labels = result?.ok ? result.sections.map((section) => section.label).join("|") : "";
304+
const key = `${this.selectedSongId}:${labels}`;
305+
if (key !== this.missingSectionWarningKey) {
306+
this.missingSectionWarnings.clear();
307+
this.missingSectionWarningKey = key;
308+
}
296309
}
297310

298311
selectSong(songId) {
@@ -690,7 +703,12 @@ export class MidiStudioV2App {
690703

691704
async handleInstrumentGridTransport(action, detail = {}) {
692705
if (action === "invalid-section") {
693-
this.statusLog.warn(`section ${detail.label} does not exist. Normalize a section map containing that label or choose a listed custom section.`);
706+
const label = String(detail.label || "(none)");
707+
const warningKey = `${this.missingSectionWarningKey}:${label.toLowerCase()}`;
708+
if (!this.missingSectionWarnings.has(warningKey)) {
709+
this.missingSectionWarnings.add(warningKey);
710+
this.statusLog.warn(`section ${label} does not exist. Normalize a section map containing that label or choose a listed custom section.`);
711+
}
694712
return;
695713
}
696714
if (action === "invalid-loop") {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ window.addEventListener("DOMContentLoaded", () => {
8787
padInput: requireElement("#instrumentGridPadInput"),
8888
playLoopButton: requireElement("#playLoopButton"),
8989
playSectionButton: requireElement("#playSectionButton"),
90+
sectionAvailability: requireElement("#instrumentGridSectionAvailability"),
9091
sectionPresetButtons: Array.from(document.querySelectorAll(".midi-studio-v2__section-preset")),
9192
sectionSelect: requireElement("#instrumentGridSectionSelect"),
9293
selectionDetails: requireElement("#timelineSelectionDetails"),

0 commit comments

Comments
 (0)