Skip to content

Commit 0630c0e

Browse files
committed
Clean duplicate MIDI Studio V2 data surfaces and establish canonical song model - PR_26146_045-midi-studio-v2-canonical-song-model-cleanup
1 parent 96c7f24 commit 0630c0e

7 files changed

Lines changed: 333 additions & 29 deletions

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# PR_26146_045 MIDI Studio V2 Canonical Song Model Cleanup Validation
2+
3+
Status: PASS
4+
5+
## Scope
6+
- Continued from PR_26146_044.
7+
- Audited MIDI Studio V2 primary workflow data/UI surfaces for duplicate song, instrument, section, and timeline state.
8+
- Removed the duplicate primary Song Sheet Summary panel.
9+
- Consolidated selected song state so `selectedSongId` derives from `payload.activeSongId`.
10+
- Kept instrument grid, song details, playback, diagnostics, manifest serialization, and save/export flows derived from the canonical selected song model.
11+
- Preserved piano-style octave grid, manifest import, multiple songs, GM controls, playback, Play/Stop, section/loop behavior, export Type + Save behavior, 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/SongSheetControl.js
20+
node --check tests/playwright/tools/MidiStudioV2.spec.mjs
21+
```
22+
23+
Additional guards:
24+
25+
```text
26+
HTML external-only guard passed
27+
CSS brace check passed
28+
```
29+
30+
## Targeted Playwright Validation
31+
PASS:
32+
33+
```text
34+
npx.cmd playwright test tests/playwright/tools/MidiStudioV2.spec.mjs -g "derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song|keeps selected song details editable and nests Song Sheet under the details panel|octave timeline freezes compact headers and note labels while active cells stay textless|fast octave note editing supports drag painting keyboard shortcuts selection and timeline scroll sync|renders timing ruler, section navigation, and loop region visualization" --config=codex_playwright_system_chrome.config.cjs --reporter=list --workers=1 --timeout=60000
35+
```
36+
37+
Result:
38+
39+
```text
40+
Running 5 tests using 1 worker
41+
ok 1 fast octave note editing supports drag painting keyboard shortcuts selection and timeline scroll sync
42+
ok 2 octave timeline freezes compact headers and note labels while active cells stay textless
43+
ok 3 keeps selected song details editable and nests Song Sheet under the details panel
44+
ok 4 derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song
45+
ok 5 renders timing ruler, section navigation, and loop region visualization
46+
5 passed
47+
```
48+
49+
## Required Assertions Covered
50+
- PASS: only one canonical selected song state is used (`selectedSongId` is derived from `payload.activeSongId`, not stored as an own app property).
51+
- PASS: song details update from the canonical selected song model.
52+
- PASS: instrument rows update from the canonical selected song model.
53+
- PASS: octave grid renders from the canonical selected song model.
54+
- PASS: playback reads playable events from the canonical selected song model.
55+
- PASS: diagnostics are read-only and derived.
56+
- PASS: duplicate Song Sheet Summary/stale duplicate primary panel is removed.
57+
- PASS: switching songs does not leave stale duplicated data visible.
58+
- PASS: Play and Stop still work through preserved targeted playback/editing tests.
59+
60+
## Diff Hygiene
61+
PASS:
62+
63+
```text
64+
git diff --check
65+
```
66+
67+
Git reported line-ending normalization warnings for touched files, but no whitespace errors.
68+
69+
## Not Run
70+
- Full samples smoke test was not run, per PR instructions.
71+
72+
## Result
73+
PR_26146_045 is marked PASS. No duplicate song/instrument/timeline state remains in the primary workflow based on changed-file implementation and targeted Playwright validation.
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# PR_26146_045 MIDI Studio V2 Duplicate Data Audit
2+
3+
Status: PASS
4+
5+
## Canonical Model Fields
6+
7+
MIDI Studio V2 now treats the normalized `payload` as the canonical runtime model:
8+
9+
- `payload.songs[]`
10+
- `payload.activeSongId`
11+
- `song.id`
12+
- `song.name`
13+
- `song.studioArrangement.tempo`
14+
- `song.studioArrangement.key`
15+
- `song.studioArrangement.style`
16+
- `song.studioArrangement.sections`
17+
- `song.studioArrangement.lanes`
18+
- `song.studioArrangement.previewInstruments`
19+
- `song.loop`
20+
- `song.rendered`
21+
- `song.defaultRuntimeFormat`
22+
- `song.sourceMidi`
23+
- `song.instrumentSet`
24+
- `song.director`
25+
- `song.tags`
26+
27+
The active song is resolved from `payload.activeSongId`. `selectedSongId` is no longer stored as independent app state.
28+
29+
## Duplicate Data Found
30+
31+
- `selectedSongId` duplicated `payload.activeSongId`.
32+
- `lastInstrumentGridResult` held unscoped parsed timeline state that could be mistaken for canonical song data.
33+
- `lastSongSheetResult` held parser state outside the selected song arrangement even though BPM/key/style belong to `song.studioArrangement`.
34+
- Guided Song Sheet inputs and Timeline source inputs could diverge from `song.studioArrangement` unless normalization pushed edits back into the selected song.
35+
36+
## Duplicate UI Found
37+
38+
- `#songSheetSummary` repeated parser-only section/chord metadata in the primary Song Setup flow while Timeline Diagnostics already reports the active normalized grid.
39+
- Previous raw Song Sheet and duplicate Song Sheet summary surfaces remain absent.
40+
- MIDI source inspection is kept as a read-only source diagnostic, not a primary runtime model.
41+
42+
## Removals Made
43+
44+
- Removed `#songSheetSummary` from Selected Song Details.
45+
- Removed required bootstrap dependency on `#songSheetSummary`; Song Sheet summary rendering is now optional.
46+
- Removed stored `selectedSongId` app state in favor of `payload.activeSongId`.
47+
- Removed stored `lastSongSheetResult` app state.
48+
- Replaced unscoped `lastInstrumentGridResult` storage with selected-song-scoped derived grid results.
49+
50+
## Canonical Derivation
51+
52+
- Song list derives from `payload.songs[]` and `payload.activeSongId`.
53+
- Selected Song Details derive from the selected canonical `song`.
54+
- Song Sheet controls derive from `song.studioArrangement`.
55+
- Instrument rows derive from `song.studioArrangement.lanes` and `song.studioArrangement.previewInstruments`.
56+
- Octave Timeline derives from parsing `song.studioArrangement` through `InstrumentGridParser`.
57+
- Playback derives from the selected song's current parsed grid result and selected lane settings.
58+
- Rendered export status derives from `song.rendered` and `song.defaultRuntimeFormat`.
59+
- ToolState export serializes the canonical `payload`.
60+
- Diagnostics are read-only derived views.
61+
62+
## Remaining Intentional Derived Views
63+
64+
- `InstrumentGridParser` output is retained as a selected-song-scoped derived view for rendering and playback scheduling.
65+
- `SongSheetParser` output is used transiently for validation/status and then synced into `song.studioArrangement`.
66+
- Audio Diagnostics derive from Preview Synth state, selected song, selected section, and selected instrument lane settings.
67+
- MIDI Source Details are read-only inspection results for the selected song source; imported MIDI remains a conversion/input source into the canonical song payload, not a second primary runtime model.
68+
69+
## UAT Risk
70+
71+
No UAT-blocking duplicate song, instrument, or timeline state remains in the primary workflow after this PR. If a future parser adds new conversion data, it must write into `payload.songs[]` / `song.studioArrangement` before becoming editable or playable.

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const uatManifestPath = path.resolve("tests/fixtures/midi-studio-v2/uat-midi-stu
88
const roadmapPath = path.resolve("docs/dev/roadmaps/MIDI_STUDIO_V2_ROADMAP.md");
99
const implementationAuditPath = path.resolve("docs/dev/reports/PR_26146_033-midi-studio-v2-implementation-audit.md");
1010
const implementationAuditValidationPath = path.resolve("docs/dev/reports/PR_26146_033-midi-studio-v2-implementation-audit_validation.md");
11+
const canonicalSongModelAuditPath = path.resolve("docs/dev/reports/PR_26146_045-midi-studio-v2-duplicate-data-audit.md");
1112

1213
const validManifest = {
1314
schema: "html-js-gaming.game-manifest",
@@ -1519,7 +1520,7 @@ test.describe("MIDI Studio V2", () => {
15191520
await expect(page.locator('[data-midi-studio-tab="song-setup"]')).toHaveAttribute("aria-selected", "true");
15201521
await expect(page.locator("#songDetailsContent")).toBeVisible();
15211522
await expect(page.locator("#songDetailsContent #songSheetContent")).toHaveCount(1);
1522-
await expect(page.locator("#songDetailsContent #songSheetSummary")).toHaveCount(1);
1523+
await expect(page.locator("#songDetailsContent #songSheetSummary")).toHaveCount(0);
15231524
await expect(page.locator("#songSheetSummaryContent")).toHaveCount(0);
15241525
await expect(page.locator("#songDetails input[data-song-detail-field='name']")).toHaveValue("Camptown Races UAT Reel");
15251526
await expect(page.locator("#songDetails input[data-song-detail-field='tempo']")).toHaveValue("144");
@@ -1570,6 +1571,102 @@ test.describe("MIDI Studio V2", () => {
15701571
}
15711572
});
15721573

1574+
test("derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song", async ({ page }) => {
1575+
const server = await openMidiStudioForImport(page);
1576+
try {
1577+
await page.locator("#toolImportManifestInput").setInputFiles(uatManifestPath);
1578+
await expect(page.locator("#songSheetSummary")).toHaveCount(0);
1579+
await expect(page.locator("#songSheetSummaryContent")).toHaveCount(0);
1580+
await expect(page.locator(".midi-studio-v2__advanced-song-sheet")).toHaveCount(0);
1581+
await expect(page.locator("#songSheetInput")).toHaveCount(0);
1582+
1583+
const initialState = await page.evaluate(() => {
1584+
const app = window.__midiStudioV2App;
1585+
const state = app.selectedSongState();
1586+
return {
1587+
activeSongId: app.payload.activeSongId,
1588+
gridSongId: state.songId,
1589+
hasOwnSelectedSongId: Object.hasOwn(app, "selectedSongId"),
1590+
instrumentCount: Object.keys(state.arrangement.lanes).length,
1591+
selectedSongId: app.selectedSongId,
1592+
title: state.song.name,
1593+
timelineCount: state.gridResult.timeline.length
1594+
};
1595+
});
1596+
expect(initialState).toEqual(expect.objectContaining({
1597+
activeSongId: "camptown-races-uat-reel",
1598+
gridSongId: "camptown-races-uat-reel",
1599+
hasOwnSelectedSongId: false,
1600+
selectedSongId: "camptown-races-uat-reel",
1601+
title: "Camptown Races UAT Reel"
1602+
}));
1603+
expect(initialState.instrumentCount).toBeGreaterThan(0);
1604+
expect(initialState.timelineCount).toBeGreaterThan(0);
1605+
await selectMidiStudioTab(page, "song-setup");
1606+
await expect(page.locator("#songDetails input[data-song-detail-field='name']")).toHaveValue("Camptown Races UAT Reel");
1607+
await expect(page.locator("#songDetails input[data-song-detail-field='tempo']")).toHaveValue("144");
1608+
await expect(page.locator("#songDetails input[data-song-detail-field='key']")).toHaveValue("G major");
1609+
await expect(page.locator("#songDetails input[data-song-detail-field='style']")).toHaveValue("public-domain-reel");
1610+
await expect(page.locator(".midi-studio-v2__instrument-row")).toHaveCount(initialState.instrumentCount);
1611+
await expect(octaveCell(page, "G4", 0)).toHaveAttribute("data-note-lanes", /lead/);
1612+
1613+
expect(await page.evaluate(() => {
1614+
const app = window.__midiStudioV2App;
1615+
app.selectSong("frog-hop-nursery-rhyme");
1616+
return {
1617+
activeSongId: app.payload.activeSongId,
1618+
nowPlaying: document.querySelector("#nowPlayingLabel").textContent,
1619+
selectedName: app.selectedSong()?.name || ""
1620+
};
1621+
})).toEqual({
1622+
activeSongId: "frog-hop-nursery-rhyme",
1623+
nowPlaying: "Selected: Frog Hop Nursery Rhyme UAT",
1624+
selectedName: "Frog Hop Nursery Rhyme UAT"
1625+
});
1626+
await expect(page.locator("#nowPlayingLabel")).toHaveText("Selected: Frog Hop Nursery Rhyme UAT");
1627+
await expect(page.locator("#songDetails input[data-song-detail-field='name']")).toHaveValue("Frog Hop Nursery Rhyme UAT");
1628+
await expect(page.locator("#instrumentGridSectionsInput")).toHaveValue("hop:2, home:2");
1629+
await expect(octaveCell(page, "C5", 0)).toHaveAttribute("data-note-lanes", /lead/);
1630+
await expect(octaveCell(page, "G4", 0)).not.toHaveAttribute("data-note-lanes", /lead/);
1631+
const frogState = await page.evaluate(() => {
1632+
const app = window.__midiStudioV2App;
1633+
const state = app.selectedSongState();
1634+
return {
1635+
activeSongId: app.payload.activeSongId,
1636+
detailTitle: document.querySelector("#songDetails input[data-song-detail-field='name']").value,
1637+
instrumentRows: document.querySelectorAll(".midi-studio-v2__instrument-row").length,
1638+
selectedSongId: app.selectedSongId,
1639+
stateSongId: state.songId,
1640+
timelineLeadZero: state.gridResult.timeline.some((event) => event.lane === "lead" && event.stepIndex === 0 && event.value === "C5")
1641+
};
1642+
});
1643+
expect(frogState).toEqual({
1644+
activeSongId: "frog-hop-nursery-rhyme",
1645+
detailTitle: "Frog Hop Nursery Rhyme UAT",
1646+
instrumentRows: Object.keys(await page.evaluate(() => window.__midiStudioV2App.selectedSongState().arrangement.lanes)).length,
1647+
selectedSongId: "frog-hop-nursery-rhyme",
1648+
stateSongId: "frog-hop-nursery-rhyme",
1649+
timelineLeadZero: true
1650+
});
1651+
1652+
const diagnostics = await audioDiagnosticsRows(page);
1653+
expect(diagnostics["Selected song"]).toBe("Frog Hop Nursery Rhyme UAT");
1654+
await expect(page.locator("#audioDiagnostics input, #audioDiagnostics textarea, #audioDiagnostics select, #audioDiagnostics button")).toHaveCount(0);
1655+
expect(await page.evaluate(() => window.__midiStudioV2App.playableEventSummary().count)).toBeGreaterThan(0);
1656+
await expect(page.locator("#playButton")).toBeEnabled();
1657+
await expect(page.locator("#stopButton")).toBeDisabled();
1658+
await expect(page.locator("#nowPlayingLabel")).toHaveText("Selected: Frog Hop Nursery Rhyme UAT");
1659+
1660+
await selectMidiStudioTab(page, "diagnostics");
1661+
await page.locator("#toolExportToolStateButton").click();
1662+
await expect(page.locator("#inspectorOutput")).toContainText('"activeSongId": "frog-hop-nursery-rhyme"');
1663+
expect(await fs.readFile(canonicalSongModelAuditPath, "utf8")).toContain("Canonical Model Fields");
1664+
} finally {
1665+
await workspaceV2CoverageReporter.stop(page);
1666+
await server.close();
1667+
}
1668+
});
1669+
15731670
test("roadmap and implementation audit exist with actual MIDI Studio V2 status markers", async () => {
15741671
const roadmap = await fs.readFile(roadmapPath, "utf8");
15751672
const audit = await fs.readFile(implementationAuditPath, "utf8");

tools/midi-studio-v2/index.html

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,6 @@ <h3>Song Sheet</h3>
187187
<button id="parseSongSheetButton" type="button">Parse Guided Song Sheet</button>
188188
</div>
189189
</section>
190-
<hr class="midi-studio-v2__details-divider">
191-
<dl id="songSheetSummary" class="midi-studio-v2__details"></dl>
192190
</div>
193191
</section>
194192

0 commit comments

Comments
 (0)