diff --git a/AGENTS.md b/AGENTS.md index b9a67ce17..fc77dc66d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,6 +83,7 @@ This section applies to any agent (Claude, Codex, Cursor, opencode, ...) working - Keep UI and analysis engine decoupled through shared contracts. - Prefer minimal, test-first changes for production code. - Prefer practical, friendly, rehearsal-first wording over academic or authority-heavy language. +- Name tonight's first pickup plan with the owning part when an active role is corroborated, the owned `pickupPlan` copy, the labeled section, and the time so the next action is obvious. Do not invent that copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, vamp plans, fill plans, tuning plans, dynamics plans, articulation plans, hook plans, solo plans, pad plans, hit plans, cutoff plans, turnaround plans, confirmed overrides, harmonic explanations, or confidence notes. - Do not reduce the product to a chord analyzer when form, timing, player coordination, playable ranges, simplification, and setup cues are the real rehearsal blockers. - Do not frame usability as a reason to accept weak analysis quality; BandScope should aim for both easy use and high accuracy. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ca0df5ac4..08844ea58 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1,10 +1,11 @@ # ARCHITECTURE.md -Last updated: 2026-03-11 +Last updated: 2026-08-27 ## Brand source - Product identity, UX tone, copy rules, and prioritization tie-breakers live in `docs/brand-story.md`. +- The mounted workspace copy for tonight's first pickup plan must name the owning part when corroborated, the owned `pickupPlan` text, the labeled section, and the time so the next action is obvious. Open moves to the matching rendered map section. Do not invent that copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, vamp plans, fill plans, tuning plans, dynamics plans, articulation plans, hook plans, solo plans, pad plans, hit plans, cutoff plans, turnaround plans, confirmed overrides, harmonic explanations, or confidence notes. Distinct from first-turnaround-plan, first-cutoff-plan, first-hit-plan, first-vamp-plan, first-pad-plan, first-solo-plan, first-hook-plan, first-fill-plan, first-setup-note, first-transposition-plan, first-tuning-plan, first-articulation-plan, and the labeled-pickup section guide. - Future PRDs, TRDs, onboarding copy, empty states, error messages, and marketing copy should use that document as the single brand source of truth. ## Security source diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b6f7e784..102f7ea87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first pickup plan in the mounted rehearsal workspace so a part that was resting can lead into the next downbeat on the map; real analyzed songs now receive this guidance only when section-level stem activity shows that part becoming active after an abutting rest, with at least one other distinct source already on the landing downbeat, while heuristic-only topology remains unavailable. Open moves to the matching rendered map section, and inherited, accessor-backed, or Proxy-substituted runtime metadata remains guidance-only instead of becoming copy, identity, timing, or navigation authority. - Name tonight's first playable range on the ready rehearsal map and tell the player to check that span on their instrument before the section. - Display the analyzed song tempo (BPM) as a badge in the rehearsal workspace. - 각 합주 역할(Role)별 개인 연습 진행도를 0~100% 범위로 기록 및 시각화할 수 있는 연습 진척도(`practiceProgress`) 트래커 기능 추가. UI 컨트롤(슬라이더 및 +/- 버튼)과 한/영 다국어 지원 포함. diff --git a/CLAUDE.md b/CLAUDE.md index b5a34c1fa..72557bd8d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ BandScope is a local-first desktop app for rehearsal prep: it turns a song into Three layers, decoupled through shared contracts: -- `apps/desktop` — Tauri 2 + Vite + React 19 shell (Tailwind 4, Base UI, Storybook). Feature screens live in `src/features/` (home, workspace, chords, ranges, player, settings). The ready workspace names tonight's first playable range and the next instrument check. `src/lib/analysis.ts` and `src/lib/job_runner.ts` call typed Tauri IPC commands, with a browser fallback that serves demo data when not running inside Tauri. +- `apps/desktop` — Tauri 2 + Vite + React 19 shell (Tailwind 4, Base UI, Storybook). Feature screens live in `src/features/` (home, workspace, chords, ranges, player, settings). The mounted workspace names tonight's first pickup plan and opens the matching rendered map section. The ready workspace names tonight's first playable range and the next instrument check. Do not invent that copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, vamp plans, fill plans, tuning plans, dynamics plans, articulation plans, hook plans, solo plans, pad plans, hit plans, cutoff plans, turnaround plans, confirmed overrides, harmonic explanations, or confidence notes. Distinct from first-turnaround-plan, first-cutoff-plan, first-hit-plan, first-vamp-plan, first-pad-plan, first-solo-plan, first-hook-plan, first-fill-plan, first-setup-note, first-transposition-plan, first-tuning-plan, first-articulation-plan, and the labeled-pickup section guide. `src/lib/analysis.ts` and `src/lib/job_runner.ts` call typed Tauri IPC commands, with a browser fallback that serves demo data when not running inside Tauri. - `apps/desktop/src-tauri/src/main.rs` — the Rust orchestration boundary. Tauri commands (`start_analysis_job`, `get_analysis_job_status`, `select_local_audio_source`, `import_youtube_url`) validate untrusted input (project IDs, file paths, URLs) and spawn the Python engine as a subprocess. There is no loopback HTTP listener and no network path for local analysis. - `services/analysis-engine` — Python package `bandscope_analysis` (librosa/numpy). Entry point `cli.py` reads a JSON job request on stdin and prints a structured job-status JSON envelope on stdout (`--progress-jsonl` streams progress lines). `api.py` orchestrates the pipeline across the `separation`, `sections`, `roles`, `chords`, `ranges`, `temporal`, `transcription`, and `youtube` modules. diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.identity.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.identity.test.tsx new file mode 100644 index 000000000..54b4aa7d3 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.identity.test.tsx @@ -0,0 +1,21 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { expect, it } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +it("gives co-mounted pickup-plan callouts distinct DOM identities", () => { + render( + <> + + + > + ); + + const callouts = screen.getAllByRole("complementary", { + name: "Tonight's first pickup plan" + }); + const ids = callouts.map((callout) => callout.id); + + expect(ids.every((id) => id.length > 0)).toBe(true); + expect(new Set(ids).size).toBe(callouts.length); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.memoization.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.memoization.test.tsx new file mode 100644 index 000000000..f4c002568 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.memoization.test.tsx @@ -0,0 +1,46 @@ +import { render } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithPickupPlan() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + return song; +} + +describe("FirstPickupPlanCallout resolver reuse", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("does not rescan role metadata when a parent rerenders the same song object", () => { + const song = songWithPickupPlan(); + const role = song.sections[1]!.roles.find((candidate) => candidate.id === "bass-guitar")!; + const descriptorSpy = vi.spyOn(Object, "getOwnPropertyDescriptor"); + + const { rerender } = render(); + const firstScanCount = descriptorSpy.mock.calls.filter(([target]) => target === role).length; + expect(firstScanCount).toBeGreaterThan(0); + + rerender(); + const secondScanCount = descriptorSpy.mock.calls.filter(([target]) => target === role).length; + + expect(secondScanCount).toBe(firstScanCount); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.navigation-failure.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.navigation-failure.test.tsx new file mode 100644 index 000000000..71fbb9d31 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.navigation-failure.test.tsx @@ -0,0 +1,42 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithPickupPlan() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + return song; +} + +describe("FirstPickupPlanCallout navigation failure", () => { + it("tells the user when the named pickup cannot be opened on the rendered map", () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + + expect( + screen.getByText( + "Could not open this pickup on the song map. Use the map below to find the section." + ) + ).toBeTruthy(); + expect( + screen.queryByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.particle.test.tsx new file mode 100644 index 000000000..aa5cf847b --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.particle.test.tsx @@ -0,0 +1,139 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithKoreanPickup( + pickupPlan: string, + pickupPlanSource?: "model" | "user" +) { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const companion = verse.roles.find((role) => role.id === "bass-guitar")!; + verse.roles = [ + { + ...verse.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + pickupPlan, + ...(pickupPlanSource ? { pickupPlanSource } : {}) + }, + companion + ]; + verse.partGraph = [ + { role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] } + ]; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "piano" + })); + song.sections = [intro, verse]; + return song; +} + +describe("FirstPickupPlanCallout Korean role copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending role names particle-safe before and after the pickup action", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithKoreanPickup( + "Play this pickup with Lead Vocal on the verse last beat; land the chorus downbeat together." + ); + + const grid = document.createElement("div"); + grid.dataset.testid = "song-structure-grid"; + grid.setAttribute("role", "region"); + grid.setAttribute("aria-label", "Scrollable song structure timeline"); + const target = document.createElement("div"); + target.dataset.sectionIndex = "1"; + Object.defineProperty(target, "scrollIntoView", { + configurable: true, + value: vi.fn() + }); + grid.appendChild(target); + document.body.appendChild(grid); + + render(); + + expect(screen.getByText("0:10 벌스에서 피아노 파트의 픽업 계획이 있습니다.")).toBeTruthy(); + expect(screen.queryByText(/피아노이/)).toBeNull(); + expect(screen.queryByText(/피아노가/)).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "0:10 피아노 픽업 열기" })); + + expect(screen.getByText("0:10에서 피아노 파트의 픽업을 넣은 다음 합주를 시작하세요.")).toBeTruthy(); + expect(screen.queryByText(/피아노과/)).toBeNull(); + + grid.remove(); + }); + + it("localizes the analysis-engine pickup template instead of exposing English guidance", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithKoreanPickup( + "Play this pickup with Lead Vocal; land the downbeat together.", + "model" + ); + + render(); + + expect( + screen.getByText("Lead Vocal 파트와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.") + ).toBeTruthy(); + expect( + screen.queryByText("Play this pickup with Lead Vocal; land the downbeat together.") + ).toBeNull(); + }); + + it("localizes the rest-of-band pickup template instead of exposing English guidance", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithKoreanPickup( + "Play this pickup with the rest of the band; land the downbeat together.", + "model" + ); + + render(); + + expect( + screen.getByText("나머지 밴드와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.") + ).toBeTruthy(); + expect( + screen.queryByText("Play this pickup with the rest of the band; land the downbeat together.") + ).toBeNull(); + }); + + it("preserves the generated template shape when long target names are bounded", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const targetRole = `Lead-${"A".repeat(180)}`; + const song = songWithKoreanPickup( + `Play this pickup with ${targetRole}; land the downbeat together.`, + "model" + ); + const landing = song.sections[1]!; + landing.roles[1] = { ...landing.roles[1]!, name: targetRole }; + + render(); + + expect(screen.queryByText(/^Play this pickup with /)).toBeNull(); + expect( + screen.getByText( + (content) => + content.startsWith("Lead-") && + content.endsWith("파트와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.") + ) + ).toBeTruthy(); + }); +}); \ No newline at end of file diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.provenance.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.provenance.test.tsx new file mode 100644 index 000000000..39ad32ad3 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.provenance.test.tsx @@ -0,0 +1,89 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithKoreanPickup( + pickupPlan: string, + pickupPlanSource?: "model" | "user" +) { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const companion = verse.roles.find((role) => role.id === "bass-guitar")!; + verse.roles = [ + { + ...verse.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + pickupPlan, + ...(pickupPlanSource ? { pickupPlanSource } : {}) + }, + companion + ]; + verse.partGraph = [ + { role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] } + ]; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "piano" + })); + song.sections = [intro, verse]; + return song; +} + +describe("FirstPickupPlanCallout pickup-plan provenance", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("preserves user pickup guidance that happens to match the engine sentence shape", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const customPlan = "Play this pickup with Lead Vocal; land the downbeat together."; + const song = songWithKoreanPickup(customPlan, "user"); + + render(); + + expect(screen.getByText(customPlan)).toBeTruthy(); + expect( + screen.queryByText("Bass Guitar 파트와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.") + ).toBeNull(); + }); + + it("does not infer model authority when persisted pickup guidance has no source", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const legacyPlan = "Play this pickup with Lead Vocal; land the downbeat together."; + const song = songWithKoreanPickup(legacyPlan); + + render(); + + expect(screen.getByText(legacyPlan)).toBeTruthy(); + expect( + screen.queryByText("Bass Guitar 파트와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.") + ).toBeNull(); + }); + + it("localizes model guidance from structured landing topology instead of display sentence wording", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const changedDisplayCopy = "Pickup display wording changed upstream."; + const song = songWithKoreanPickup(changedDisplayCopy, "model"); + + render(); + + expect( + screen.getByText("Bass Guitar 파트와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.") + ).toBeTruthy(); + expect(screen.queryByText(changedDisplayCopy)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.reduced-motion.test.tsx new file mode 100644 index 000000000..1cfa653f5 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.reduced-motion.test.tsx @@ -0,0 +1,64 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithPickupPlan() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + return song; +} + +describe("FirstPickupPlanCallout reduced motion", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("scrolls immediately when the operating system requests reduced motion", () => { + vi.stubGlobal("matchMedia", (query: string) => ({ + matches: query === "(prefers-reduced-motion: reduce)", + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn() + })); + + const grid = document.createElement("div"); + grid.dataset.testid = "song-structure-grid"; + grid.setAttribute("role", "region"); + grid.setAttribute("aria-label", "Scrollable song structure timeline"); + const target = document.createElement("div"); + target.dataset.sectionIndex = "1"; + const scrollIntoView = vi.fn(); + Object.defineProperty(target, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + grid.appendChild(target); + document.body.appendChild(grid); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.test.tsx new file mode 100644 index 000000000..c0bf60469 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.test.tsx @@ -0,0 +1,272 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong, type RehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +const DEMO_PICKUP_PLAN = "Play this pickup with Lead Vocal; land the downbeat together."; +const appendedSongStructureTargets = new Set(); + +function songWithPickupPlan() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + return song; +} + +function appendSongStructureTarget(ariaLabel = "Scrollable song structure timeline") { + const timeline = document.createElement("div"); + timeline.setAttribute("role", "region"); + timeline.setAttribute("aria-label", ariaLabel); + const grid = document.createElement("div"); + grid.dataset.testid = "song-structure-grid"; + const target = document.createElement("div"); + target.dataset.sectionIndex = "1"; + const scrollIntoView = vi.fn(); + Object.defineProperty(target, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + grid.appendChild(target); + timeline.appendChild(grid); + document.body.appendChild(timeline); + appendedSongStructureTargets.add(timeline); + return { grid: timeline, scrollIntoView }; +} + +describe("FirstPickupPlanCallout", () => { + afterEach(() => { + for (const timeline of appendedSongStructureTargets) { + timeline.remove(); + } + appendedSongStructureTargets.clear(); + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText("No pickup plan is available. Stay on tonight's map for the next rehearsal cue.") + ).toBeTruthy(); + }); + + it("contains a hostile song identity accessor instead of crashing the callout", () => { + const song = songWithPickupPlan(); + Object.defineProperty(song, "id", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile song id getter"); + } + }); + + expect(() => render()).not.toThrow(); + expect(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })).toBeTruthy(); + }); + + it("contains a hostile song identity descriptor lookup instead of crashing the callout", () => { + const song = new Proxy(songWithPickupPlan(), { + getOwnPropertyDescriptor() { + throw new Error("hostile song id descriptor"); + } + }); + + expect(() => render()).not.toThrow(); + expect( + screen.getByText("No pickup plan is available. Stay on tonight's map for the next rehearsal cue.") + ).toBeTruthy(); + }); + + it("resets armed guidance when accessor-id songs change with the same pickup signature", () => { + const firstSong = songWithPickupPlan(); + const nextSong = songWithPickupPlan(); + for (const song of [firstSong, nextSong]) { + Object.defineProperty(song, "id", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile song id getter"); + } + }); + } + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + expect( + screen.getByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeTruthy(); + + rerender(); + + expect(screen.getByText("Bass Guitar has a pickup into the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeNull(); + + grid.remove(); + }); + + it("preserves armed guidance across immutable edits of the same owned song", () => { + const song = songWithPickupPlan(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + expect( + screen.getByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeTruthy(); + + rerender(); + + expect( + screen.getByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeTruthy(); + expect(screen.queryByText("Bass Guitar has a pickup into the verse at 0:10.")).toBeNull(); + + grid.remove(); + }); + + it("does not show another part's pickup plan under the named landing part", () => { + const song = songWithPickupPlan(); + song.sections[1]!.roles[0]!.pickupPlan = "Bass leftover pickup that must not appear."; + song.sections[1]!.roles[0]!.rehearsalPriority = "low"; + song.sections[1]!.roles[2]!.pickupPlan = "Leave the vocal on the last lyric while the pickup lands."; + song.sections[1]!.roles[2]!.rehearsalPriority = "low"; + song.sections[1]!.roles[1]!.pickupPlan = + "Play this pickup with Lead Vocal; land the downbeat together."; + song.sections[0]!.partGraph = song.sections[0]!.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "keys-right" + })); + + render(); + + expect( + screen.getByText("Keyboard 1 Right Hand has a pickup into the verse at 0:10.") + ).toBeTruthy(); + expect( + screen.getByText("Play this pickup with Lead Vocal; land the downbeat together.") + ).toBeTruthy(); + expect(screen.queryByText("Leave the vocal on the last lyric while the pickup lands.")).toBeNull(); + expect(screen.queryByText("Bass leftover pickup that must not appear.")).toBeNull(); + }); + + it("names the first pickup plan as map navigation, scrolls to its rendered section, and arms that action", () => { + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + expect(screen.getByText(DEMO_PICKUP_PLAN)).toBeTruthy(); + const action = screen.getByRole("button", { + name: "Open Bass Guitar pickup at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeTruthy(); + + grid.remove(); + }); + + it("keeps map navigation stable when the renderer accessible name is localized", () => { + const { grid, scrollIntoView } = appendSongStructureTarget("스크롤 가능한 곡 구조 타임라인"); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeTruthy(); + + grid.remove(); + }); + + it("does not claim map navigation completed when the rendered section target is missing", () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + + expect(screen.getByText("Bass Guitar has a pickup into the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeNull(); + }); + + it("navigates by renderer-owned section position instead of untrusted analysis ids", () => { + const song = songWithPickupPlan(); + song.sections[1]!.id = "analysis section / duplicate"; + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + + grid.remove(); + }); + + it("fails closed when one workspace owns more than one song-structure renderer", () => { + const firstScroll = vi.fn(); + const secondScroll = vi.fn(); + const { container } = render( + + + + + + + + + + ); + const targets = container.querySelectorAll('[data-section-index="1"]'); + Object.defineProperty(targets[0], "scrollIntoView", { + configurable: true, + value: firstScroll + }); + Object.defineProperty(targets[1], "scrollIntoView", { + configurable: true, + value: secondScroll + }); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + + expect(firstScroll).not.toHaveBeenCalled(); + expect(secondScroll).not.toHaveBeenCalled(); + expect(screen.getByText("Bass Guitar has a pickup into the verse at 0:10.")).toBeTruthy(); + }); + + it("fails closed when more than one song-structure renderer is mounted globally", () => { + const first = appendSongStructureTarget(); + const second = appendSongStructureTarget(); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar pickup at 0:10" })); + + expect(first.scrollIntoView).not.toHaveBeenCalled(); + expect(second.scrollIntoView).not.toHaveBeenCalled(); + expect(screen.getByText("Bass Guitar has a pickup into the verse at 0:10.")).toBeTruthy(); + + first.grid.remove(); + second.grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.tsx new file mode 100644 index 000000000..663450c08 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.tsx @@ -0,0 +1,226 @@ +import { useEffect, useId, useMemo, useState } from "react"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { Button } from "@/components/ui/button"; +import { + createTranslator, + detectPreferredLocale, + translateSectionFormLabel +} from "../../i18n"; +import { + formatPickupPlanTime, + resolveFirstPickupPlan, + type PickupPlanGuidance +} from "./firstPickupPlan"; + +/** Props for the first pickup-plan rehearsal callout. */ +export interface FirstPickupPlanCalloutProps { + song: RehearsalSong; +} + +type PickupPlanCopyValues = Readonly>; +type PickupPlanSource = "model" | "user"; + +type OpenedPickupPlan = Readonly<{ + songIdentity: unknown; + sectionId: string; + sectionIndex: number; + landingRoleId: string; + pickupPlan: string; + pickupPlanSource: PickupPlanSource | null; + pickupPlanGuidanceKind: PickupPlanGuidance["kind"] | null; + pickupPlanTargetRoleName: string | null; + atSeconds: number; +}>; + +/** Read a stable owned song id, falling back to object identity for untrusted identity metadata. */ +function stablePickupPlanSongIdentity(song: RehearsalSong): unknown { + if (song === null || typeof song !== "object" || Array.isArray(song)) { + return song; + } + let descriptor: PropertyDescriptor | undefined; + try { + descriptor = Object.getOwnPropertyDescriptor(song, "id"); + } catch { + return song; + } + return descriptor !== undefined && + Object.prototype.hasOwnProperty.call(descriptor, "value") && + typeof descriptor.value === "string" && + descriptor.value.trim().length > 0 + ? descriptor.value + : song; +} + +/** Interpolate pickup-plan placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatPickupPlanCopy(template: string, values: PickupPlanCopyValues): string { + return template.replace(/\{(role|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof PickupPlanCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Localize model pickup guidance from structured landing topology, never from display-copy grammar. */ +function localizedPickupPlan( + pickupPlan: string, + pickupPlanSource: PickupPlanSource | null, + guidance: PickupPlanGuidance | null, + generatedTemplate: string, + generatedBandTemplate: string +): string { + if (pickupPlanSource !== "model" || guidance === null) { + return pickupPlan; + } + return guidance.kind === "band" + ? generatedBandTemplate + : generatedTemplate.replace("{target}", () => guidance.targetRoleName); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredPickupPlanScrollBehavior(): ScrollBehavior { + return typeof window.matchMedia === "function" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches + ? "auto" + : "smooth"; +} + +/** Resolve the song-structure renderer owned by this workspace, failing closed on ambiguous mounts. */ +function resolvePickupPlanRenderer(origin: HTMLElement): HTMLElement | null { + const selector = '[data-testid="song-structure-grid"]'; + const localScope = origin.closest("aside")?.parentElement ?? null; + const localRenderers = localScope?.querySelectorAll(selector) ?? []; + if (localRenderers.length === 1) { + return localRenderers[0] ?? null; + } + if (localRenderers.length > 1) { + return null; + } + + const globalRenderers = document.querySelectorAll(selector); + return globalRenderers.length === 1 ? (globalRenderers[0] ?? null) : null; +} + +/** Name tonight's first pickup plan and open the matching rendered map section. */ +export function FirstPickupPlanCallout({ song }: FirstPickupPlanCalloutProps) { + const calloutId = `workspace-surface-pickup-plan-${useId()}`; + const locale = useMemo(() => detectPreferredLocale(), []); + const t = useMemo(() => createTranslator(locale), [locale]); + const songIdentity = stablePickupPlanSongIdentity(song); + const named = useMemo(() => resolveFirstPickupPlan(song), [song]); + const [openedPickupPlan, setOpenedPickupPlan] = useState(null); + const [navigationFailed, setNavigationFailed] = useState(false); + const guidanceKind = named?.pickupPlanGuidance?.kind ?? null; + const guidanceTargetRoleName = + named?.pickupPlanGuidance?.kind === "role" + ? named.pickupPlanGuidance.targetRoleName + : null; + + useEffect(() => { + setOpenedPickupPlan(null); + setNavigationFailed(false); + }, [ + songIdentity, + named?.sectionIndex, + named?.sectionId, + named?.landingRoleId, + named?.pickupPlan, + named?.pickupPlanSource, + guidanceKind, + guidanceTargetRoleName, + named?.atSeconds + ]); + + if (!named) { + return ( + + ); + } + + const opened = + openedPickupPlan !== null && + openedPickupPlan.songIdentity === songIdentity && + openedPickupPlan.sectionId === named.sectionId && + openedPickupPlan.sectionIndex === named.sectionIndex && + openedPickupPlan.landingRoleId === named.landingRoleId && + openedPickupPlan.pickupPlan === named.pickupPlan && + openedPickupPlan.pickupPlanSource === named.pickupPlanSource && + openedPickupPlan.pickupPlanGuidanceKind === guidanceKind && + openedPickupPlan.pickupPlanTargetRoleName === guidanceTargetRoleName && + openedPickupPlan.atSeconds === named.atSeconds; + const at = formatPickupPlanTime(named.atSeconds); + const copyValues: PickupPlanCopyValues = { + role: named.landingRoleName, + section: translateSectionFormLabel(locale, named.sectionLabel), + at + }; + const actionLabel = formatPickupPlanCopy(t("firstPickupPlanOpenAction"), copyValues); + const body = formatPickupPlanCopy(t("firstPickupPlanBody"), copyValues); + const armed = formatPickupPlanCopy(t("firstPickupPlanArmed"), copyValues); + const pickupPlan = localizedPickupPlan( + named.pickupPlan, + named.pickupPlanSource, + named.pickupPlanGuidance, + t("firstPickupPlanGeneratedGuidance"), + t("firstPickupPlanGeneratedBandGuidance") + ); + + return ( + + ); +} diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.unavailable-copy.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.unavailable-copy.test.tsx new file mode 100644 index 000000000..a39a3ab0d --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.unavailable-copy.test.tsx @@ -0,0 +1,38 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithoutPickupPlan() { + const song = createDemoRehearsalSong(); + for (const section of song.sections) { + for (const role of section.roles) { + role.pickupPlan = ""; + } + } + return song; +} + +describe("FirstPickupPlanCallout unavailable copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("does not assert why the English pickup plan is unavailable", () => { + render(); + + expect( + screen.getByText("No pickup plan is available. Stay on tonight's map for the next rehearsal cue.") + ).toBeTruthy(); + }); + + it("does not assert why the Korean pickup plan is unavailable", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + + render(); + + expect( + screen.getByText("사용 가능한 픽업 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요.") + ).toBeTruthy(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstPickupPlanCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.workspace-scope.test.tsx new file mode 100644 index 000000000..4b958b86b --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstPickupPlanCallout.workspace-scope.test.tsx @@ -0,0 +1,75 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it, vi } from "vitest"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; + +function songWithPickupPlan(id: string) { + const song = createDemoRehearsalSong(); + song.id = id; + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = `${id}-intro`; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + return song; +} + +describe("FirstPickupPlanCallout workspace scope", () => { + it("opens the song-structure renderer owned by the current workspace", () => { + const firstSong = songWithPickupPlan("first-workspace-song"); + const secondSong = songWithPickupPlan("second-workspace-song"); + + const { container } = render( + <> + + + + + + + + + + + + + > + ); + + const targets = container.querySelectorAll('[data-section-index="1"]'); + expect(targets).toHaveLength(2); + const firstScrollIntoView = vi.fn(); + const secondScrollIntoView = vi.fn(); + Object.defineProperty(targets[0]!, "scrollIntoView", { + configurable: true, + value: firstScrollIntoView + }); + Object.defineProperty(targets[1]!, "scrollIntoView", { + configurable: true, + value: secondScrollIntoView + }); + + const actions = screen.getAllByRole("button", { + name: "Open Bass Guitar pickup at 0:10" + }); + expect(actions).toHaveLength(2); + fireEvent.click(actions[1]!); + + expect(firstScrollIntoView).not.toHaveBeenCalled(); + expect(secondScrollIntoView).toHaveBeenCalledWith({ + block: "nearest", + behavior: "smooth" + }); + }); +}); diff --git a/apps/desktop/src/features/workspace/Workspace.test.tsx b/apps/desktop/src/features/workspace/Workspace.test.tsx index 7837bf80e..c61e8f021 100644 --- a/apps/desktop/src/features/workspace/Workspace.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.test.tsx @@ -326,4 +326,48 @@ describe("Workspace", () => { expect(screen.getByText("합주 우선순위")).toBeTruthy(); expect(screen.getByText("역할과 화성")).toBeTruthy(); }); + + it("names tonight's first pickup plan as workspace navigation", () => { + setNavigatorLanguage("en-US"); + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + + render(); + + const target = screen.getByTestId("song-structure-grid").children.item(1); + expect(target).toBeTruthy(); + const scrollIntoView = vi.fn(); + Object.defineProperty(target!, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + + expect( + screen.getAllByText("Play this pickup with Lead Vocal; land the downbeat together.").length + ).toBeGreaterThan(0); + const action = screen.getByRole("button", { + name: "Open Bass Guitar pickup at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Play that pickup on Bass Guitar at 0:10 before the downbeat lands./) + ).toBeTruthy(); + }); }); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index d44e20777..db914ee68 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -4,6 +4,7 @@ import { RoleSwitcher } from "./RoleSwitcher"; import { SectionRoadmap } from "./SectionRoadmap"; import { GrooveMap } from "./GrooveMap"; import { PracticeProgress } from "./PracticeProgress"; +import { FirstPickupPlanCallout } from "./FirstPickupPlanCallout"; import { fillRangeCopy, firstRangeSqueeze } from "./firstRangeSqueeze"; import { createTranslator, detectPreferredLocale } from "../../i18n"; import { generateCueSheetCsv, generateChartSummaryJson, generateMetadataHandoffJson, sanitizeFilename } from "../../lib/export"; @@ -91,8 +92,12 @@ const SongStructure = memo(function SongStructure({ sections, t }: { sections: R data-testid="song-structure-grid" style={{ gridTemplateColumns: `repeat(${Math.max(1, sections.length)}, minmax(8rem, 1fr))` }} > - {sections.map((section) => ( - + {sections.map((section, sectionIndex) => ( + {section.label} · {formatTimelineTime(section.timeRange.start)}–{formatTimelineTime(section.timeRange.end)} @@ -353,6 +358,8 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp + + diff --git a/apps/desktop/src/features/workspace/coverageContract.test.ts b/apps/desktop/src/features/workspace/coverageContract.test.ts new file mode 100644 index 000000000..3d4bfdba5 --- /dev/null +++ b/apps/desktop/src/features/workspace/coverageContract.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { DESKTOP_OWNED_PRODUCTION_COVERAGE } from "../../../vite.config"; + +describe("desktop owned production coverage", () => { + it("keeps the first pickup-plan resolver and callout inside the coverage gate", () => { + expect(DESKTOP_OWNED_PRODUCTION_COVERAGE).toEqual( + expect.arrayContaining([ + "src/features/workspace/firstPickupPlan.ts", + "src/features/workspace/FirstPickupPlanCallout.tsx" + ]) + ); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.continuation.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.continuation.test.ts new file mode 100644 index 000000000..e3c8fabcf --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.continuation.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +describe("resolveFirstPickupPlan landing authority", () => { + it("does not name a pickup when the landing role is the only source on the downbeat", () => { + const song = createDemoRehearsalSong(); + const verse = structuredClone(song.sections[0]!); + const landingRole = verse.roles.find((role) => role.id === "bass-guitar")!; + landingRole.pickupPlan = "Play this pickup with Lead Vocal; land the downbeat together."; + verse.roles = [landingRole]; + verse.partGraph = verse.partGraph.map((node) => ({ + ...node, + is_active: node.role_id === landingRole.id + })); + + const intro = structuredClone(verse); + intro.id = "intro-lone"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ ...node, is_active: false })); + song.sections = [intro, verse]; + + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("does not name a pickup when the rest and landing windows leave a gap", () => { + const song = createDemoRehearsalSong(); + const verse = structuredClone(song.sections[0]!); + verse.roles[0]!.pickupPlan = "Play this pickup with Lead Vocal; land the downbeat together."; + const intro = structuredClone(verse); + intro.id = "intro-gap"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start - 1 }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.demo.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.demo.test.ts new file mode 100644 index 000000000..a10adbfb5 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.demo.test.ts @@ -0,0 +1,35 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +describe("demo pickup plan", () => { + it("does not name a demo pickup without a predecessor rest", () => { + expect(resolveFirstPickupPlan(createDemoRehearsalSong())).toBeNull(); + }); + + it("keeps the demo pickup actionable across a real predecessor rest", () => { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + + expect(resolveFirstPickupPlan(song)).toMatchObject({ + sectionId: "verse-1", + landingRoleId: "bass-guitar", + atSeconds: 10 + }); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.inherited-metadata.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.inherited-metadata.test.ts new file mode 100644 index 000000000..975527ddb --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.inherited-metadata.test.ts @@ -0,0 +1,120 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +function songWithPickupPlan() { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + section.id = "pickup-own"; + section.roles = [ + { + ...section.roles[0]!, + id: "bass-guitar", + name: "Bass Guitar", + rehearsalPriority: "high", + pickupPlan: "Play this pickup with Lead Vocal; land the downbeat together." + }, + { + ...section.roles[2]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "medium" + } + ]; + delete section.roles[1]!.pickupPlan; + delete section.roles[1]!.pickupPlanSource; + section.partGraph = [ + { role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "lead-vocal", is_active: true, handoff_to: [], handoff_from: [] } + ]; + const previous = structuredClone(section); + previous.id = "pickup-rest"; + previous.label = "intro"; + previous.timeRange = { start: 0, end: section.timeRange.start }; + previous.roles = previous.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + previous.partGraph = previous.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [previous, section]; + return { song, section, previous }; +} + +describe("resolveFirstPickupPlan inherited metadata", () => { + it("rejects a song or section whose required metadata is inherited", () => { + const { song, section } = songWithPickupPlan(); + const inheritedSong = Object.create({ sections: song.sections }) as typeof song; + expect(resolveFirstPickupPlan(inheritedSong)).toBeNull(); + + const inheritedSection = Object.create(section) as typeof section; + song.sections = [song.sections[0]!, inheritedSection]; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("rejects inherited timing fields", () => { + const { song, section } = songWithPickupPlan(); + section.timeRange = Object.create({ start: 10, end: 30 }) as typeof section.timeRange; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("contains exceptions from own runtime accessors instead of trusting them", () => { + const { song, section } = songWithPickupPlan(); + Object.defineProperty(section.roles[0]!, "pickupPlan", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile pickupPlan getter"); + } + }); + + expect(() => resolveFirstPickupPlan(song)).not.toThrow(); + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("does not treat own accessors as stable pickup-plan identity authority", () => { + const { song, section } = songWithPickupPlan(); + Object.defineProperty(section, "id", { + configurable: true, + enumerable: true, + get() { + return "pickup-own"; + } + }); + + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("does not let inherited pickup plans establish the named copy", () => { + const { song, section } = songWithPickupPlan(); + const companion = { ...section.roles[1]! }; + const inheritedRole = Object.create({ + pickupPlan: "Inherited pickup plan" + }) as (typeof section.roles)[0]; + Object.defineProperties(inheritedRole, { + id: { configurable: true, enumerable: true, value: "bass-guitar" }, + name: { configurable: true, enumerable: true, value: "Bass Guitar" }, + rehearsalPriority: { configurable: true, enumerable: true, value: "high" } + }); + section.roles = [inheritedRole, companion]; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("does not let inherited role or graph metadata establish the landing part", () => { + const { song, section } = songWithPickupPlan(); + const node = section.partGraph[0]!; + section.partGraph = [Object.create(node) as typeof node]; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("rejects arrays masquerading as section records", () => { + const { song, section } = songWithPickupPlan(); + const arraySection = Object.assign([], section) as unknown as typeof section; + song.sections = [song.sections[0]!, arraySection]; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.proxy-authority.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.proxy-authority.test.ts new file mode 100644 index 000000000..ff6a9bb73 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.proxy-authority.test.ts @@ -0,0 +1,98 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +const DEMO_PICKUP_PLAN = "Play this pickup with Lead Vocal; land the downbeat together."; + +function songWithPredecessorPickup() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + return song; +} + +describe("resolveFirstPickupPlan own-data authority", () => { + it("uses the snapshotted own-data pickup plan instead of a Proxy get trap", () => { + const song = songWithPredecessorPickup(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + const roleIndex = section?.roles.findIndex((role) => role.id === "bass-guitar") ?? -1; + const role = roleIndex >= 0 ? section?.roles[roleIndex] : undefined; + expect(section).toBeDefined(); + expect(role).toBeDefined(); + if (!section || !role || roleIndex < 0) { + throw new Error("Demo pickup-plan fixture is missing the expected Bass Guitar role."); + } + + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "pickupPlan") { + return "Injected proxy pickup."; + } + return Reflect.get(target, property, receiver); + } + }); + + expect(resolveFirstPickupPlan(song)?.pickupPlan).toBe(DEMO_PICKUP_PLAN); + }); + + it("uses the snapshotted own-data time range instead of a Proxy get trap", () => { + const song = songWithPredecessorPickup(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + expect(section).toBeDefined(); + if (!section) { + throw new Error("Demo pickup-plan fixture is missing the expected verse section."); + } + const expectedStart = section.timeRange.start; + section.timeRange = new Proxy(section.timeRange, { + get(target, property, receiver) { + if (property === "start") { + return expectedStart + 15; + } + return Reflect.get(target, property, receiver); + } + }); + + expect(resolveFirstPickupPlan(song)?.atSeconds).toBe(expectedStart); + }); + + it("returns snapshotted role identity and display copy instead of Proxy get values", () => { + const song = songWithPredecessorPickup(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + const roleIndex = section?.roles.findIndex((role) => role.id === "bass-guitar") ?? -1; + const role = roleIndex >= 0 ? section?.roles[roleIndex] : undefined; + expect(section).toBeDefined(); + expect(role).toBeDefined(); + if (!section || !role || roleIndex < 0) { + throw new Error("Demo pickup-plan fixture is missing the expected Bass Guitar role."); + } + const expectedId = role.id; + const expectedName = role.name; + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "name") { + return "Injected proxy role"; + } + return Reflect.get(target, property, receiver); + } + }); + + const resolved = resolveFirstPickupPlan(song); + expect(resolved?.pickupPlan).toBe(DEMO_PICKUP_PLAN); + expect(resolved?.landingRoleId).toBe(expectedId); + expect(resolved?.landingRoleName).toBe(expectedName); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.section-label.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.section-label.test.ts new file mode 100644 index 000000000..5b208bdae --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.section-label.test.ts @@ -0,0 +1,28 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +describe("resolveFirstPickupPlan section-label authority", () => { + it("fails closed when runtime metadata supplies a label outside the shared SectionFormLabel contract", () => { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const intro = structuredClone(verse); + intro.id = "intro-1"; + intro.label = "intro"; + intro.timeRange = { start: 0, end: verse.timeRange.start }; + intro.roles = intro.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + intro.partGraph = intro.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + song.sections = [intro, verse]; + (verse as unknown as { label: string }).label = "verse-legacy"; + + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.snapshot.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.snapshot.test.ts new file mode 100644 index 000000000..ec2c7378f --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.snapshot.test.ts @@ -0,0 +1,63 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +describe("resolveFirstPickupPlan descriptor snapshots", () => { + it("uses the pickup-plan snapshot that admitted the role", () => { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + const role = section.roles[0]!; + const companion = section.roles[2]!; + const roleId = role.id; + let pickupPlanDescriptorReads = 0; + const proxiedRole = new Proxy(role, { + getOwnPropertyDescriptor(target, key) { + if (key === "pickupPlan") { + pickupPlanDescriptorReads += 1; + return { + configurable: true, + enumerable: true, + writable: true, + value: + pickupPlanDescriptorReads === 1 + ? "Play this pickup with Lead Vocal; land the downbeat together." + : "Changed after validation." + }; + } + return Reflect.getOwnPropertyDescriptor(target, key); + } + }); + + const previous = structuredClone({ + ...section, + roles: [role, companion], + partGraph: [ + { role_id: roleId, is_active: false, handoff_to: [], handoff_from: [] }, + { role_id: companion.id, is_active: true, handoff_to: [], handoff_from: [] } + ] + }); + previous.id = "pickup-rest"; + previous.label = "intro"; + previous.timeRange = { start: 0, end: section.timeRange.start }; + previous.roles = previous.roles.map((candidate) => { + const clone = { ...candidate }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + + section.roles = [proxiedRole, companion]; + delete companion.pickupPlan; + delete companion.pickupPlanSource; + section.partGraph = [ + { role_id: roleId, is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: companion.id, is_active: true, handoff_to: [], handoff_from: [] } + ]; + song.sections = [previous, section]; + + expect(resolveFirstPickupPlan(song)?.pickupPlan).toBe( + "Play this pickup with Lead Vocal; land the downbeat together." + ); + expect(pickupPlanDescriptorReads).toBe(1); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.test.ts new file mode 100644 index 000000000..3c4bba9ad --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.test.ts @@ -0,0 +1,431 @@ +import { describe, expect, it } from "vitest"; +import { MAX_SECTION_TIME_SECONDS, createDemoRehearsalSong } from "@bandscope/shared-types"; +import { formatPickupPlanTime, resolveFirstPickupPlan } from "./firstPickupPlan"; + +const DEMO_PICKUP_PLAN = "Play this pickup with Lead Vocal; land the downbeat together."; + +function withPickupSection( + overrides: { + id?: string; + start?: number; + end?: number; + previousStart?: number; + pickupPlan?: string; + label?: "intro" | "verse" | "pre-chorus" | "chorus" | "bridge" | "outro" | "tag" | "pickup" | "stop" | "handoff"; + roleId?: string; + roleName?: string; + priority?: "low" | "medium" | "high"; + isActive?: boolean; + wasActive?: boolean; + functionLabel?: string; + } = {} +) { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const landingStart = overrides.start ?? 10; + const previousStart = overrides.previousStart ?? 0; + const roleId = overrides.roleId ?? "lead-vocal"; + const companionRoleId = roleId === "bass-guitar" ? "keys-right" : "bass-guitar"; + const companionRole = structuredClone(verse.roles.find((role) => role.id === companionRoleId)!); + delete companionRole.pickupPlan; + delete companionRole.pickupPlanSource; + + const current = structuredClone(verse); + current.id = overrides.id ?? "verse-pickup"; + current.label = overrides.label ?? "verse"; + current.groove = "Straight eighths with a late snare feel"; + current.timeRange = { start: landingStart, end: overrides.end ?? landingStart + 20 }; + current.roles = [ + { + ...verse.roles[2]!, + id: roleId, + name: overrides.roleName ?? "Lead Vocal", + rehearsalPriority: overrides.priority ?? "medium", + cue: { kind: "lyric", value: "city lights" }, + range: { lowestNote: "G#3", highestNote: "C#5" }, + setupNote: "Watch the breath before the last line of the verse.", + simplification: "Keep the sustained note centered; skip the ad-lib on the first pass.", + overlapWarnings: ["Melodic overlap: competing with Keyboard 1 Right Hand."], + harmony: { + chord: "C#m7", + functionLabel: overrides.functionLabel ?? "vi melodic pull", + source: "model" + }, + harmonicExplanation: + "The melody leans on the ninth over vi, so the vocal line should feel like a lift rather than a strict chord-tone outline.", + confidence: { + level: "high", + source: "user", + notes: "Singer confirmed the pickup phrasing in rehearsal notes." + }, + pickupPlan: overrides.pickupPlan ?? DEMO_PICKUP_PLAN, + manualOverrides: [] + }, + companionRole + ]; + current.partGraph = [ + { + role_id: roleId, + is_active: overrides.isActive ?? true, + handoff_to: [], + handoff_from: [] + }, + { + role_id: companionRole.id, + is_active: true, + handoff_to: [], + handoff_from: [] + } + ]; + + const previous = structuredClone(current); + previous.id = `${current.id}-rest`; + previous.label = "intro"; + previous.timeRange = { start: previousStart, end: landingStart }; + previous.roles = previous.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + previous.partGraph = previous.partGraph.map((node) => ({ + ...node, + is_active: node.role_id === roleId ? (overrides.wasActive ?? false) : true + })); + + song.sections = [previous, current]; + return song; +} + +describe("resolveFirstPickupPlan", () => { + it("picks the earliest pickup plan and the part that leads into the downbeat", () => { + const resolved = resolveFirstPickupPlan(withPickupSection()); + expect(resolved?.section.id).toBe("verse-pickup"); + expect(resolved?.landingRole.id).toBe("lead-vocal"); + expect(resolved?.pickupPlan).toBe(DEMO_PICKUP_PLAN); + expect(resolved?.atSeconds).toBe(10); + expect(formatPickupPlanTime(resolved?.atSeconds ?? -1)).toBe("0:10"); + expect(formatPickupPlanTime(Number.NaN)).toBe("0:00"); + expect(formatPickupPlanTime(-4)).toBe("0:00"); + }); + + it("does not invent a pickup plan from groove, cue, simplification, overlap, range, chords, function labels, setup notes, transposition plans, vamp plans, fill plans, tuning plans, dynamics plans, articulation plans, hook plans, solo plans, pad plans, hit plans, cutoff plans, turnaround plans, confirmed overrides, harmonic explanations, or confidence notes", () => { + const song = withPickupSection(); + delete song.sections[1]!.roles[0]!.pickupPlan; + song.sections[1]!.groove = "Straight eighths with a late snare feel"; + song.sections[1]!.roles[0]!.simplification = "Keep the sustained note centered."; + song.sections[1]!.roles[0]!.setupNote = DEMO_PICKUP_PLAN; + song.sections[1]!.roles[0]!.transpositionPlan = + "If the singer drops to B minor, keep the shape a whole step lower."; + (song.sections[1]!.roles[0] as { vampPlan?: string }).vampPlan = + "Keep this part going until Lead Vocal enters in the next section."; + (song.sections[1]!.roles[0] as { fillPlan?: string }).fillPlan = + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty."; + (song.sections[1]!.roles[0] as { tuningPlan?: string }).tuningPlan = + "Tune the E string down to D so the verse riff sits on the open fifth."; + (song.sections[1]!.roles[0] as { dynamicsPlan?: string }).dynamicsPlan = + "Keep the verse under the vocal so the chorus still has somewhere to lift."; + (song.sections[1]!.roles[0] as { articulationPlan?: string }).articulationPlan = + "Shorten the last chorus vowel so the band can hear the pickup."; + (song.sections[1]!.roles[0] as { hookPlan?: string }).hookPlan = + "Lead vocal carries the chorus hook; lock the melody before anyone stacks harmony."; + (song.sections[1]!.roles[0] as { soloPlan?: string }).soloPlan = + "Hold the verse solo; everyone else drops to a two-bar pad so the run can land."; + (song.sections[1]!.roles[0] as { padPlan?: string }).padPlan = + "Drop to a two-bar pad so the Keyboard 1 Right Hand run can land."; + (song.sections[1]!.roles[0] as { hitPlan?: string }).hitPlan = + "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup."; + (song.sections[1]!.roles[0] as { cutoffPlan?: string }).cutoffPlan = + "Cut this off with Lead Vocal on the verse last beat; don't linger past the pickup."; + (song.sections[1]!.roles[0] as { turnaroundPlan?: string }).turnaroundPlan = + "Turn these last bars with Lead Vocal; land the downbeat together."; + song.sections[1]!.roles[0]!.cue = { kind: "lyric", value: "city lights" }; + song.sections[1]!.roles[0]!.range = { lowestNote: "G#3", highestNote: "C#5" }; + song.sections[1]!.roles[0]!.overlapWarnings = ["Melodic overlap: competing with Keyboard 1 Right Hand."]; + song.sections[1]!.roles[0]!.harmony = { + chord: "C#m7", + functionLabel: "vi melodic pull", + source: "user" + }; + song.sections[1]!.roles[0]!.harmonicExplanation = "The ninth is the reason this lift works."; + song.sections[1]!.roles[0]!.manualOverrides = [ + { + field: "harmony", + value: { + chord: "C#m11", + functionLabel: "vi suspended lift", + source: "user" + }, + source: "user" + } + ]; + song.sections[1]!.roles[0]!.confidence = { + level: "high", + source: "user", + notes: DEMO_PICKUP_PLAN + }; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("skips a blank pickup plan", () => { + expect(resolveFirstPickupPlan(withPickupSection({ pickupPlan: " " }))).toBeNull(); + }); + + it("skips a multi-line pickup plan", () => { + expect( + resolveFirstPickupPlan(withPickupSection({ pickupPlan: "Keep the melody centered.\nLeave the stack." })) + ).toBeNull(); + }); + + it("prefers the earlier of two pickup plans", () => { + const song = withPickupSection({ + id: "verse-late-pickup", + start: 40, + end: 56, + previousStart: 24, + roleId: "keys-right", + pickupPlan: "Late pickup." + }); + const earlier = structuredClone(song.sections[1]!); + const earlierCompanion = structuredClone(earlier.roles[1]!); + delete earlierCompanion.pickupPlan; + delete earlierCompanion.pickupPlanSource; + earlier.id = "verse-early"; + earlier.roles = [ + { + ...earlier.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "low", + pickupPlan: "Earlier pickup." + }, + earlierCompanion + ]; + earlier.timeRange = { start: 8, end: 24 }; + earlier.partGraph = [ + { role_id: "lead-vocal", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: earlierCompanion.id, is_active: true, handoff_to: [], handoff_from: [] } + ]; + const earlierRest = structuredClone(earlier); + earlierRest.id = "intro-before-early"; + earlierRest.label = "intro"; + earlierRest.timeRange = { start: 0, end: 8 }; + earlierRest.roles = earlierRest.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + earlierRest.partGraph = earlier.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "lead-vocal" + })); + song.sections = [earlierRest, earlier, song.sections[0]!, song.sections[1]!]; + + const resolved = resolveFirstPickupPlan(song); + expect(resolved?.section.id).toBe("verse-early"); + expect(resolved?.landingRole.id).toBe("lead-vocal"); + expect(resolved?.pickupPlan).toBe("Earlier pickup."); + expect(resolved?.atSeconds).toBe(8); + }); + + it("breaks same-time pickup-plan ties with locale-independent id ordering", () => { + const song = withPickupSection({ id: "ä-pickup", start: 10, end: 26 }); + const umlautRest = song.sections[0]!; + const umlaut = song.sections[1]!; + const asciiRest = structuredClone(umlautRest); + asciiRest.id = "z-pickup-rest"; + const ascii = structuredClone(umlaut); + ascii.id = "z-pickup"; + song.sections = [umlautRest, umlaut, asciiRest, ascii]; + + expect(resolveFirstPickupPlan(song)?.section.id).toBe("z-pickup"); + }); + + it("prefers a high-priority pickup part over a low-priority part in the same section", () => { + const song = withPickupSection({ + roleId: "keys-right", + roleName: "Keys", + priority: "low", + pickupPlan: "Low-priority pickup." + }); + const section = song.sections[1]!; + const highRole = { + ...section.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "high" as const, + pickupPlan: "High-priority pickup." + }; + section.roles = [section.roles[0]!, highRole]; + section.partGraph = [ + { role_id: "keys-right", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "lead-vocal", is_active: true, handoff_to: [], handoff_from: [] } + ]; + const previous = song.sections[0]!; + previous.roles = section.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + previous.partGraph = section.partGraph.map((node) => ({ ...node, is_active: false })); + + expect(resolveFirstPickupPlan(song)?.landingRole.id).toBe("lead-vocal"); + expect(resolveFirstPickupPlan(song)?.pickupPlan).toBe("High-priority pickup."); + }); + + it("breaks equal-priority role ties with locale-independent id ordering", () => { + const song = withPickupSection({ roleId: "ä-role", roleName: "Umlaut role", priority: "high" }); + const section = song.sections[1]!; + const asciiRole = { + ...section.roles[0]!, + id: "z-role", + name: "ASCII role", + pickupPlan: "ASCII pickup." + }; + section.roles = [section.roles[0]!, asciiRole]; + section.partGraph = [ + { role_id: "ä-role", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "z-role", is_active: true, handoff_to: [], handoff_from: [] } + ]; + const previous = song.sections[0]!; + previous.roles = section.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + previous.partGraph = section.partGraph.map((node) => ({ ...node, is_active: false })); + + expect(resolveFirstPickupPlan(song)?.landingRole.id).toBe("z-role"); + expect(resolveFirstPickupPlan(song)?.pickupPlan).toBe("ASCII pickup."); + }); + + it("skips a pickup plan whose graph node is inactive", () => { + expect(resolveFirstPickupPlan(withPickupSection({ isActive: false }))).toBeNull(); + }); + + it("skips a pickup plan whose previous graph node was already active", () => { + expect(resolveFirstPickupPlan(withPickupSection({ wasActive: true }))).toBeNull(); + }); + + it("skips a pickup plan whose rest and landing windows do not abut", () => { + const song = withPickupSection({ start: 12 }); + song.sections[0]!.timeRange = { start: 0, end: 10 }; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("skips a pickup plan whose rehearsal window is unbounded", () => { + expect(resolveFirstPickupPlan(withPickupSection({ start: Number.NaN, end: 30 }))).toBeNull(); + }); + + it("skips a pickup plan whose end precedes its start", () => { + expect(resolveFirstPickupPlan(withPickupSection({ start: 30, end: 10 }))).toBeNull(); + }); + + it("skips a zero-length pickup-plan window", () => { + expect(resolveFirstPickupPlan(withPickupSection({ start: 10, end: 10 }))).toBeNull(); + }); + + it("skips a pickup plan whose endpoint overflows the shared timing bound", () => { + expect( + resolveFirstPickupPlan( + withPickupSection({ + start: MAX_SECTION_TIME_SECONDS, + end: MAX_SECTION_TIME_SECONDS + 1 + }) + ) + ).toBeNull(); + }); + + it("returns null for a non-object song root", () => { + expect(resolveFirstPickupPlan(null as never)).toBeNull(); + }); + + it("skips non-object roles and graph nodes without inventing a landing part", () => { + const song = withPickupSection(); + song.sections[1]!.roles = [ + null as never, + song.sections[1]!.roles[0]!, + song.sections[1]!.roles[1]! + ]; + song.sections[1]!.partGraph = [ + null as never, + song.sections[1]!.partGraph[0]!, + song.sections[1]!.partGraph[1]! + ]; + expect(resolveFirstPickupPlan(song)?.landingRole.id).toBe("lead-vocal"); + }); + + it("returns null when the runtime section collection is sparse", () => { + const song = withPickupSection(); + const sparseSections: typeof song.sections = new Array(2); + sparseSections[1] = song.sections[1]!; + song.sections = sparseSections; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("keeps the pickup plan unnamed when role identities are duplicated", () => { + const song = withPickupSection(); + const role = song.sections[1]!.roles[0]!; + song.sections[1]!.roles = [role, { ...role }]; + song.sections[1]!.partGraph = [ + { role_id: role.id, is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: role.id, is_active: true, handoff_to: [], handoff_from: [] } + ]; + expect(resolveFirstPickupPlan(song)).toBeNull(); + }); + + it("bounds the pickup plan to 180 Unicode code points", () => { + const song = withPickupSection({ pickupPlan: `${"G".repeat(200)}` }); + const resolved = resolveFirstPickupPlan(song); + expect(resolved?.pickupPlan.length).toBe(180); + }); + + it("does not split a Unicode surrogate pair at the pickup-plan boundary", () => { + const song = withPickupSection({ pickupPlan: `${"a".repeat(179)}😀tail` }); + const resolved = resolveFirstPickupPlan(song); + expect(Array.from(resolved?.pickupPlan ?? "")).toHaveLength(180); + expect(resolved?.pickupPlan.endsWith("😀")).toBe(true); + }); + + it("keeps the generated activity sentence recognizable after bounding a long partner name", () => { + const target = `Lead-${"A".repeat(180)}`; + const song = withPickupSection({ + pickupPlan: `Play this pickup with ${target}; land the downbeat together.` + }); + const resolved = resolveFirstPickupPlan(song); + expect(resolved?.pickupPlan.startsWith("Play this pickup with Lead-")).toBe(true); + expect(resolved?.pickupPlan.endsWith("; land the downbeat together.")).toBe(true); + expect(Array.from(resolved?.pickupPlan ?? "").length).toBeLessThanOrEqual(180); + }); + + it("preserves a short generated shared-pickup sentence", () => { + const song = withPickupSection({ + pickupPlan: "Play this pickup with Lead Vocal; land the downbeat together." + }); + expect(resolveFirstPickupPlan(song)?.pickupPlan).toBe( + "Play this pickup with Lead Vocal; land the downbeat together." + ); + }); + + it("does not treat an empty generated partner as structured guidance", () => { + const song = withPickupSection({ + pickupPlan: "Play this pickup with ; land the downbeat together." + }); + expect(resolveFirstPickupPlan(song)?.pickupPlan).toBe( + "Play this pickup with ; land the downbeat together." + ); + }); + + it("contains exceptions from the runtime root instead of crashing", () => { + const song = new Proxy(withPickupSection(), { + getOwnPropertyDescriptor() { + throw new Error("hostile descriptor"); + } + }); + expect(() => resolveFirstPickupPlan(song as never)).not.toThrow(); + expect(resolveFirstPickupPlan(song as never)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.ts b/apps/desktop/src/features/workspace/firstPickupPlan.ts new file mode 100644 index 000000000..5a535001a --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.ts @@ -0,0 +1,451 @@ +import { + MAX_SECTION_TIME_SECONDS, + SECTION_FORM_LABELS, + type RehearsalRole, + type RehearsalSection, + type RehearsalSong +} from "@bandscope/shared-types"; + +const PRIORITY_RANK = { high: 0, medium: 1, low: 2 } as const; +const MAX_PICKUP_PLAN_CHARACTERS = 180; +const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); +const ACCOMPANIMENT_SOURCE_ROLE_IDS = new Set([ + "keys-left", + "keys-right", + "acoustic-guitar" +]); +const ACCOMPANIMENT_SOURCE_ID = "other"; +const PICKUP_PLAN_PREFIX = "Play this pickup with "; +const PICKUP_PLAN_SUFFIX = "; land the downbeat together."; +const PICKUP_PLAN_BAND_TARGET = "the rest of the band"; + +type PickupPlanSource = "model" | "user"; + +/** Structured localization guidance for model-generated pickup-plan copy. */ +export type PickupPlanGuidance = + | Readonly<{ kind: "role"; targetRoleName: string }> + | Readonly<{ kind: "band" }>; + +type RankedRoleMetadata = Readonly<{ + role: RehearsalRole; + id: string; + name: string; + rehearsalPriority: keyof typeof PRIORITY_RANK; +}>; + +type OwnedPickupPlan = Readonly<{ + text: string; + source: PickupPlanSource | null; + guidance: PickupPlanGuidance | null; +}>; + +/** Tonight's first pickup plan: the earliest labeled downbeat a resting part leads into. */ +export type FirstPickupPlan = { + section: RehearsalSection; + sectionId: string; + sectionLabel: RehearsalSection["label"]; + sectionIndex: number; + landingRole: RehearsalRole; + landingRoleId: string; + landingRoleName: string; + pickupPlan: string; + pickupPlanSource: PickupPlanSource | null; + pickupPlanGuidance: PickupPlanGuidance | null; + atSeconds: number; +}; + +/** Format a non-negative pickup-plan time as m:ss for rehearsal copy. */ +export function formatPickupPlanTime(totalSeconds: number): string { + const safeSeconds = Number.isFinite(totalSeconds) && totalSeconds >= 0 ? totalSeconds : 0; + const minutes = Math.floor(safeSeconds / 60); + const seconds = Math.floor(safeSeconds % 60) + .toString() + .padStart(2, "0"); + return `${minutes}:${seconds}`; +} + +/** Compare opaque ids by Unicode code units so tie-breaking never depends on host locale. */ +function compareStableId(left: string, right: string): number { + if (left < right) { + return -1; + } + if (left > right) { + return 1; + } + return 0; +} + +/** Return whether an untrusted runtime value can be inspected as a record. */ +function isRuntimeObject(value: unknown): value is object { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +/** Return whether a runtime record owns a stable data property rather than inherited/accessor state. */ +function hasOwnData(value: object, key: PropertyKey): boolean { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor !== undefined && Object.prototype.hasOwnProperty.call(descriptor, "value"); +} + +/** Snapshot one owned data-property value without invoking a getter or Proxy get trap. */ +function ownDataValue(value: object, key: PropertyKey): unknown { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor !== undefined && Object.prototype.hasOwnProperty.call(descriptor, "value") + ? descriptor.value + : undefined; +} + +/** Snapshot every numeric own data element from a bounded runtime array. */ +function ownedDenseRuntimeArray(value: unknown): unknown[] | null { + if (!Array.isArray(value)) { + return null; + } + const length = ownDataValue(value, "length"); + if ( + typeof length !== "number" || + !Number.isSafeInteger(length) || + length < 0 || + length > 0xffffffff + ) { + return null; + } + const items: unknown[] = []; + for (let index = 0; index < length; index += 1) { + if (!hasOwnData(value, index)) { + return null; + } + items.push(ownDataValue(value, index)); + } + return items; +} + +/** Bound buyer-visible text by Unicode code points without splitting a surrogate pair. */ +function truncateCodePoints(value: string, maximum: number): string { + let codePoints = 0; + let endIndex = 0; + for (const character of value) { + if (codePoints >= maximum) { + break; + } + endIndex += character.length; + codePoints += 1; + } + return endIndex === value.length ? value : value.slice(0, endIndex); +} + +/** Preserve the engine pickup template while bounding its model-owned target and localization guidance. */ +function boundedGeneratedPickupPlan(value: string): OwnedPickupPlan | null { + if (!value.startsWith(PICKUP_PLAN_PREFIX) || !value.endsWith(PICKUP_PLAN_SUFFIX)) { + return null; + } + const target = value.slice(PICKUP_PLAN_PREFIX.length, -PICKUP_PLAN_SUFFIX.length); + if (target.trim().length === 0) { + return null; + } + const fixedLength = Array.from(PICKUP_PLAN_PREFIX + PICKUP_PLAN_SUFFIX).length; + const boundedTarget = truncateCodePoints(target, MAX_PICKUP_PLAN_CHARACTERS - fixedLength); + return { + text: `${PICKUP_PLAN_PREFIX}${boundedTarget}${PICKUP_PLAN_SUFFIX}`, + source: "model", + guidance: + target === PICKUP_PLAN_BAND_TARGET + ? { kind: "band" } + : { kind: "role", targetRoleName: boundedTarget } + }; +} + +/** Return a bounded snapshotted own pickup plan and its explicit provenance, or null when malformed. */ +function ownedPickupPlan(role: unknown): OwnedPickupPlan | null { + if (!isRuntimeObject(role)) { + return null; + } + const pickupPlan = ownDataValue(role, "pickupPlan"); + const pickupPlanSource = ownDataValue(role, "pickupPlanSource"); + if (typeof pickupPlan !== "string") { + return null; + } + if ( + pickupPlanSource !== undefined && + pickupPlanSource !== "model" && + pickupPlanSource !== "user" + ) { + return null; + } + const trimmed = pickupPlan.trim(); + if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + return null; + } + if (pickupPlanSource === "model") { + const generatedPlan = boundedGeneratedPickupPlan(trimmed); + if (generatedPlan !== null) { + return generatedPlan; + } + } + return { + text: truncateCodePoints(trimmed, MAX_PICKUP_PLAN_CHARACTERS), + source: pickupPlanSource ?? null, + guidance: null + }; +} + +/** Snapshot trusted role identity, display name, and priority without Proxy get authority. */ +function ownedRankedRoleMetadata(role: unknown): RankedRoleMetadata | null { + if (!isRuntimeObject(role)) { + return null; + } + const id = ownDataValue(role, "id"); + const name = ownDataValue(role, "name"); + const rehearsalPriority = ownDataValue(role, "rehearsalPriority"); + if ( + typeof id !== "string" || + id.trim().length === 0 || + typeof name !== "string" || + name.trim().length === 0 || + typeof rehearsalPriority !== "string" || + !Object.prototype.hasOwnProperty.call(PRIORITY_RANK, rehearsalPriority) + ) { + return null; + } + return { + role: role as RehearsalRole, + id, + name, + rehearsalPriority: rehearsalPriority as keyof typeof PRIORITY_RANK + }; +} + +/** Snapshot a section's bounded positive-length integer rehearsal window. */ +function ownedBoundedTimeRange( + section: RehearsalSection +): RehearsalSection["timeRange"] | null { + const timeRange = ownDataValue(section, "timeRange"); + if (!isRuntimeObject(timeRange)) { + return null; + } + const start = ownDataValue(timeRange, "start"); + const end = ownDataValue(timeRange, "end"); + if ( + typeof start !== "number" || + !Number.isInteger(start) || + start < 0 || + start > MAX_SECTION_TIME_SECONDS || + typeof end !== "number" || + !Number.isInteger(end) || + end <= start || + end > MAX_SECTION_TIME_SECONDS + ) { + return null; + } + return { start, end }; +} + +/** Return safe identities that appear more than once in one section-local collection. */ +function repeatedIds(ids: string[]): Set { + const seen = new Set(); + const repeated = new Set(); + for (const id of ids) { + if (seen.has(id)) { + repeated.add(id); + } else { + seen.add(id); + } + } + return repeated; +} + +/** Map canonical accompaniment roles back to their shared source-separation stem. */ +function pickupSourceId(roleId: string): string { + return ACCOMPANIMENT_SOURCE_ROLE_IDS.has(roleId) ? ACCOMPANIMENT_SOURCE_ID : roleId; +} + +/** Prefer rehearsal priority, then a locale-independent stable id. */ +function pickLandingRole(roles: Role[]): Role | null { + if (roles.length === 0) { + return null; + } + return ( + [...roles].sort((left, right) => { + const priorityDelta = + PRIORITY_RANK[left.rehearsalPriority] - PRIORITY_RANK[right.rehearsalPriority]; + if (priorityDelta !== 0) { + return priorityDelta; + } + return compareStableId(left.id, right.id); + })[0] ?? null + ); +} + +/** Return ranked roles whose unique graph node is explicitly active. */ +function rankedActiveRoles(section: RehearsalSection): RankedRoleMetadata[] { + const roles = ownedDenseRuntimeArray(ownDataValue(section, "roles")); + const partGraph = ownedDenseRuntimeArray(ownDataValue(section, "partGraph")); + if (!roles || !partGraph) { + return []; + } + + const safeRoleIds = roles.flatMap((role) => { + if (!isRuntimeObject(role)) { + return []; + } + const id = ownDataValue(role, "id"); + return typeof id === "string" && id.trim().length > 0 ? [id] : []; + }); + const safeGraphRoleIds = partGraph.flatMap((node) => { + if (!isRuntimeObject(node)) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && roleId.trim().length > 0 ? [roleId] : []; + }); + const repeatedRoleIds = repeatedIds(safeRoleIds); + const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); + const activeIds = new Set( + partGraph.flatMap((node) => { + if (!isRuntimeObject(node) || ownDataValue(node, "is_active") !== true) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && + roleId.trim().length > 0 && + !repeatedGraphRoleIds.has(roleId) + ? [roleId] + : []; + }) + ); + + return roles.flatMap((role) => { + const metadata = ownedRankedRoleMetadata(role); + return metadata !== null && + !repeatedRoleIds.has(metadata.id) && + activeIds.has(metadata.id) + ? [metadata] + : []; + }); +} + +/** Return unique graph role ids whose node is explicitly inactive. */ +function rankedInactiveRoleIds(section: RehearsalSection): Set { + const partGraph = ownedDenseRuntimeArray(ownDataValue(section, "partGraph")); + if (!partGraph) { + return new Set(); + } + const safeGraphRoleIds = partGraph.flatMap((node) => { + if (!isRuntimeObject(node)) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && roleId.trim().length > 0 ? [roleId] : []; + }); + const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); + return new Set( + partGraph.flatMap((node) => { + if (!isRuntimeObject(node) || ownDataValue(node, "is_active") !== false) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && + roleId.trim().length > 0 && + !repeatedGraphRoleIds.has(roleId) + ? [roleId] + : []; + }) + ); +} + +/** Resolve a pickup plan after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstPickupPlan(song: RehearsalSong): FirstPickupPlan | null { + if (!isRuntimeObject(song)) { + return null; + } + const sections = ownedDenseRuntimeArray(ownDataValue(song, "sections")); + if (!sections) { + return null; + } + + const candidates = sections + .flatMap((section, sectionIndex) => { + if (!isRuntimeObject(section) || sectionIndex === 0) { + return []; + } + const previousSection = sections[sectionIndex - 1]; + if (!isRuntimeObject(previousSection)) { + return []; + } + const sectionId = ownDataValue(section, "id"); + const sectionLabel = ownDataValue(section, "label"); + const timeRange = ownedBoundedTimeRange(section as RehearsalSection); + const previousTimeRange = ownedBoundedTimeRange(previousSection as RehearsalSection); + if ( + typeof sectionId !== "string" || + sectionId.trim().length === 0 || + typeof sectionLabel !== "string" || + !SECTION_FORM_LABEL_SET.has(sectionLabel) || + timeRange === null || + previousTimeRange === null || + previousTimeRange.end !== timeRange.start + ) { + return []; + } + + const activeRoles = rankedActiveRoles(section as RehearsalSection); + const previousInactiveIds = rankedInactiveRoleIds(previousSection as RehearsalSection); + const landingSourceCount = new Set(activeRoles.map((metadata) => pickupSourceId(metadata.id))) + .size; + if (landingSourceCount < 2) { + return []; + } + + const landingRole = pickLandingRole( + activeRoles.flatMap((metadata) => { + if (!previousInactiveIds.has(metadata.id)) { + return []; + } + const pickupPlan = ownedPickupPlan(metadata.role); + return pickupPlan === null + ? [] + : [ + { + ...metadata, + pickupPlan: pickupPlan.text, + pickupPlanSource: pickupPlan.source, + pickupPlanGuidance: pickupPlan.guidance + } + ]; + }) + ); + if (!landingRole) { + return []; + } + return [ + { + section: section as RehearsalSection, + sectionId, + sectionLabel: sectionLabel as RehearsalSection["label"], + sectionIndex, + landingRole: landingRole.role, + landingRoleId: landingRole.id, + landingRoleName: landingRole.name, + pickupPlan: landingRole.pickupPlan, + pickupPlanSource: landingRole.pickupPlanSource, + pickupPlanGuidance: landingRole.pickupPlanGuidance, + atSeconds: timeRange.start + } + ]; + }) + .sort((left, right) => { + if (left.atSeconds !== right.atSeconds) { + return left.atSeconds - right.atSeconds; + } + return compareStableId(left.sectionId, right.sectionId); + }); + + return candidates[0] ?? null; +} + +/** Return the first named pickup plan, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstPickupPlan(song: RehearsalSong): FirstPickupPlan | null { + try { + return resolveSafeFirstPickupPlan(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/features/workspace/firstPickupPlan.user-provenance.test.ts b/apps/desktop/src/features/workspace/firstPickupPlan.user-provenance.test.ts new file mode 100644 index 000000000..12519e0f5 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstPickupPlan.user-provenance.test.ts @@ -0,0 +1,66 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstPickupPlan } from "./firstPickupPlan"; + +const GENERATED_PREFIX = "Play this pickup with "; +const GENERATED_SUFFIX = "; land the downbeat together."; + +function userPlanSong(pickupPlan: string) { + const song = createDemoRehearsalSong(); + const current = structuredClone(song.sections[0]!); + current.id = "user-pickup"; + current.timeRange = { start: 10, end: 30 }; + current.roles = [ + { + ...current.roles[0]!, + id: "bass-guitar", + name: "Bass Guitar", + rehearsalPriority: "high", + pickupPlan, + pickupPlanSource: "user" + }, + { + ...current.roles[2]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "medium" + } + ]; + delete current.roles[1]!.pickupPlan; + delete current.roles[1]!.pickupPlanSource; + current.partGraph = [ + { role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "lead-vocal", is_active: true, handoff_to: [], handoff_from: [] } + ]; + + const previous = structuredClone(current); + previous.id = "user-pickup-rest"; + previous.label = "intro"; + previous.timeRange = { start: 0, end: 10 }; + previous.roles = previous.roles.map((role) => { + const clone = { ...role }; + delete clone.pickupPlan; + delete clone.pickupPlanSource; + return clone; + }); + previous.partGraph = previous.partGraph.map((node) => ({ + ...node, + is_active: node.role_id !== "bass-guitar" + })); + + song.sections = [previous, current]; + return song; +} + +describe("resolveFirstPickupPlan user provenance", () => { + it("bounds user-authored generated-shape copy as user text instead of rewriting the target", () => { + const pickupPlan = `${GENERATED_PREFIX}${"A".repeat(170)}${GENERATED_SUFFIX}`; + const expected = Array.from(pickupPlan).slice(0, 180).join(""); + + const resolved = resolveFirstPickupPlan(userPlanSong(pickupPlan)); + + expect(resolved?.pickupPlanSource).toBe("user"); + expect(resolved?.pickupPlan).toBe(expected); + expect(resolved?.pickupPlan?.endsWith(GENERATED_SUFFIX)).toBe(false); + }); +}); diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..55b180a81 100644 --- a/apps/desktop/src/i18n/index.test.ts +++ b/apps/desktop/src/i18n/index.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { createTranslator, detectPreferredLocale } from "./index"; +import { createTranslator, detectPreferredLocale, translateSectionFormLabel } from "./index"; import koCommon from "../locales/ko/common.json"; describe("i18n", () => { @@ -75,4 +75,52 @@ describe("i18n", () => { } }); }); + + describe("translateSectionFormLabel", () => { + it("localizes every supported Korean section form label", () => { + expect( + [ + "intro", + "verse", + "pre-chorus", + "chorus", + "bridge", + "outro", + "tag", + "pickup", + "stop", + "handoff" + ].map((label) => translateSectionFormLabel("ko", label as never)) + ).toEqual([ + "인트로", + "벌스", + "프리코러스", + "코러스", + "브리지", + "아웃트로", + "태그", + "픽업", + "스톱", + "핸드오프" + ]); + }); + + it("preserves every supported English section form label", () => { + expect(translateSectionFormLabel("en", "verse")).toBe("verse"); + expect(translateSectionFormLabel("en", "pre-chorus")).toBe("pre-chorus"); + }); + + it("does not read inherited Object keys as section labels", () => { + const inheritedKey = "toString" as never; + expect(translateSectionFormLabel("en", inheritedKey)).toBe("toString"); + expect(translateSectionFormLabel("ko", inheritedKey)).toBe("toString"); + }); + + it("keeps Korean first-pickup-plan next-action copy particle-safe", () => { + const t = createTranslator("ko"); + expect(t("firstPickupPlanOpenAction")).toBe("{at} {role} 픽업 열기"); + expect(t("firstPickupPlanBody")).toBe("{at} {section}에서 {role} 파트의 픽업 계획이 있습니다."); + expect(t("firstPickupPlanArmed")).toBe("{at}에서 {role} 파트의 픽업을 넣은 다음 합주를 시작하세요."); + }); + }); }); diff --git a/apps/desktop/src/i18n/index.ts b/apps/desktop/src/i18n/index.ts index 1a9f471f0..f5656ce01 100644 --- a/apps/desktop/src/i18n/index.ts +++ b/apps/desktop/src/i18n/index.ts @@ -1,3 +1,4 @@ +import type { SectionFormLabel } from "@bandscope/shared-types"; import enCommon from "../locales/en/common.json"; import koCommon from "../locales/ko/common.json"; @@ -11,13 +12,46 @@ const dictionaries = { ko: koCommon } as const; -/** Documented. */ +const sectionFormLabels: Readonly>>> = { + en: { + intro: "intro", + verse: "verse", + "pre-chorus": "pre-chorus", + chorus: "chorus", + bridge: "bridge", + outro: "outro", + tag: "tag", + pickup: "pickup", + stop: "stop", + handoff: "handoff" + }, + ko: { + intro: "인트로", + verse: "벌스", + "pre-chorus": "프리코러스", + chorus: "코러스", + bridge: "브리지", + outro: "아웃트로", + tag: "태그", + pickup: "픽업", + stop: "스톱", + handoff: "핸드오프" + } +}; + +/** Create a translator for the requested locale, falling back to the English dictionary for missing entries. */ export function createTranslator(locale: Locale = "en") { return function t(key: TranslationKey): string { return dictionaries[locale][key] ?? dictionaries.en[key]; }; } +/** Return the localized display label for a supported rehearsal section form. */ +export function translateSectionFormLabel(locale: Locale, label: SectionFormLabel): string { + const labels = sectionFormLabels[locale] as Readonly>; + return Object.prototype.hasOwnProperty.call(labels, label) ? labels[label] : String(label); +} + /** Documented. */ export function detectPreferredLocale(): Locale { if (typeof navigator !== "undefined" && navigator.language?.toLowerCase().startsWith("ko")) { diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index d803a765e..162351f4c 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -149,6 +149,14 @@ "practiceProgressLabel": "Practice Progress", "decreasePracticeProgressLabel": "Decrease progress", "increasePracticeProgressLabel": "Increase progress", + "firstPickupPlanLabel": "Tonight's first pickup plan", + "firstPickupPlanOpenAction": "Open {role} pickup at {at}", + "firstPickupPlanBody": "{role} has a pickup into the {section} at {at}.", + "firstPickupPlanArmed": "Play that pickup on {role} at {at} before the downbeat lands.", + "firstPickupPlanGeneratedGuidance": "Play this pickup with {target}; land the downbeat together.", + "firstPickupPlanGeneratedBandGuidance": "Play this pickup with the rest of the band; land the downbeat together.", + "firstPickupPlanUnavailable": "No pickup plan is available. Stay on tonight's map for the next rehearsal cue.", + "firstPickupPlanNavigationFailed": "Could not open this pickup on the song map. Use the map below to find the section.", "workspaceFirstRangeTitle": "Tonight's first range", "workspaceFirstRangeCheck": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Check that span on your instrument before the {sectionLabel}.", "workspaceFirstRangeClash": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Hear that clash on your instrument before the {sectionLabel}.", diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 0f6c6c66d..2724b048f 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -149,6 +149,14 @@ "practiceProgressLabel": "연습 진척도", "decreasePracticeProgressLabel": "진척도 감소", "increasePracticeProgressLabel": "진척도 증가", + "firstPickupPlanLabel": "오늘 첫 픽업 계획", + "firstPickupPlanOpenAction": "{at} {role} 픽업 열기", + "firstPickupPlanBody": "{at} {section}에서 {role} 파트의 픽업 계획이 있습니다.", + "firstPickupPlanArmed": "{at}에서 {role} 파트의 픽업을 넣은 다음 합주를 시작하세요.", + "firstPickupPlanGeneratedGuidance": "{target} 파트와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.", + "firstPickupPlanGeneratedBandGuidance": "나머지 밴드와 이 픽업을 맞추세요. 첫 박에 함께 들어가세요.", + "firstPickupPlanUnavailable": "사용 가능한 픽업 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요.", + "firstPickupPlanNavigationFailed": "곡 맵에서 이 픽업을 열 수 없습니다. 아래 맵에서 해당 구간을 찾아주세요.", "workspaceFirstRangeTitle": "오늘 먼저 볼 음역", "workspaceFirstRangeCheck": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}입니다. {sectionLabel} 들어가기 전에 그 음역을 악기로 확인해 보세요.", "workspaceFirstRangeClash": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}이고 다른 파트와 겹칩니다. {sectionLabel} 들어가기 전에 그 충돌을 악기로 들어 보세요.", diff --git a/apps/desktop/vite.config.ts b/apps/desktop/vite.config.ts index f1db6f2b8..45b812b22 100644 --- a/apps/desktop/vite.config.ts +++ b/apps/desktop/vite.config.ts @@ -6,6 +6,18 @@ import { fileURLToPath } from "node:url"; const configDirectory = path.dirname(fileURLToPath(import.meta.url)); +/** Production files whose V8 coverage is owned by the desktop test gate. */ +export const DESKTOP_OWNED_PRODUCTION_COVERAGE = [ + "src/App.tsx", + "src/lib/export.ts", + "src/i18n/index.ts", + "src/features/score/ScoreViewer.tsx", + "src/features/score/ScoreView.tsx", + "src/features/score/scoreStorage.ts", + "src/features/workspace/firstPickupPlan.ts", + "src/features/workspace/FirstPickupPlanCallout.tsx" +]; + export default defineConfig({ plugins: [react(), tailwindcss()], resolve: { @@ -19,14 +31,7 @@ export default defineConfig({ setupFiles: ["./src/setupTests.ts"], coverage: { provider: "v8", - include: [ - "src/App.tsx", - "src/lib/export.ts", - "src/i18n/index.ts", - "src/features/score/ScoreViewer.tsx", - "src/features/score/ScoreView.tsx", - "src/features/score/scoreStorage.ts" - ], + include: DESKTOP_OWNED_PRODUCTION_COVERAGE, thresholds: { lines: 90, functions: 90, diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 22602c313..1ec1bc76a 100644 --- a/docs/design-system/component-contract.md +++ b/docs/design-system/component-contract.md @@ -30,6 +30,7 @@ The authoritative Figma view is `31 Component Contract Catalog`. This file mirro | Status Pill | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-283 | `apps/desktop/src/features/workspace/Workspace.tsx` | Design pattern only. Current code uses `formatStatusLabel(status)` inside local badge-like markup. | | Role Switcher | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-337 | `apps/desktop/src/features/workspace/RoleSwitcher.tsx` | Use `roles`, `activeRole`, and `onRoleChange`; `null` means all roles. | | Section Roadmap Card | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-402 | `apps/desktop/src/features/workspace/SectionRoadmap.tsx` | Use `song`, `activeRole`, and optional `onSongUpdate`; avoid rebuilding its internal card layout. | +| First Pickup Plan Callout (feature-local pattern) | No dedicated Figma node exists yet; keep feature-local until the design catalog is updated. | `apps/desktop/src/features/workspace/FirstPickupPlanCallout.tsx` | Name the owning part when an active graph node corroborates it after an abutting rest, the owned `pickupPlan` copy, the labeled section start, and the time. Do not invent that copy from `groove`, cue text, `simplification`, overlap warnings, range copy, `harmony.chord`, `harmony.functionLabel`, `setupNote`, `transpositionPlan`, `vampPlan`, `fillPlan`, `tuningPlan`, `dynamicsPlan`, `articulationPlan`, `hookPlan`, `soloPlan`, `padPlan`, `hitPlan`, `cutoffPlan`, `turnaroundPlan`, confirmed overrides, `harmonicExplanation`, or confidence notes. Open scrolls the renderer-owned song-structure section. Keep the unavailable state guidance-only. Distinct from first-turnaround-plan, first-cutoff-plan, first-hit-plan, first-vamp-plan, first-pad-plan, first-solo-plan, first-hook-plan, first-fill-plan, first-setup-note, first-transposition-plan, first-tuning-plan, first-articulation-plan, and the labeled-pickup section guide. | | Song Structure Timeline | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-457 | `apps/desktop/src/features/workspace/Workspace.tsx` | Feature-local `SongStructure({ sections, t })` memo component; not exported. | | Groove Map | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-526 | `apps/desktop/src/features/workspace/GrooveMap.tsx` | Use `notes?: TranscriptionNote[]` and `isLoading?: boolean`; preserve scrollable region semantics and note labels. | | Source Control Stack | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-655 | `apps/desktop/src/App.tsx` | Feature-local source controls for local audio, YouTube URL import, project actions, and Start Analysis; keep before metrics at 375px. | diff --git a/docs/doctoring/reduced-motion-first-pickup-plan-navigation.md b/docs/doctoring/reduced-motion-first-pickup-plan-navigation.md new file mode 100644 index 000000000..6522ae0ea --- /dev/null +++ b/docs/doctoring/reduced-motion-first-pickup-plan-navigation.md @@ -0,0 +1,3 @@ +# Reduced-motion first pickup-plan navigation + +Open tonight's first pickup plan with `behavior: "auto"` when `prefers-reduced-motion: reduce` matches. Do not keep a smooth scroll for that next action. diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index cba4606a2..423e58228 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -125,7 +125,7 @@ export type ManualOverride = source: "user"; }; -/** Documented. */ +/** An active rehearsal role on one section of tonight's map, carrying optional activity-backed guidance metadata. */ export type RehearsalRole = { id: string; name: string; @@ -139,6 +139,8 @@ export type RehearsalRole = { simplification: string; setupNote: string; transpositionPlan?: string; + pickupPlan?: string; + pickupPlanSource?: ProvenanceSource; manualOverrides: ManualOverride[]; overlapWarnings: string[]; transcription?: TranscriptionNote[]; @@ -474,6 +476,7 @@ const demoRehearsalSongSeed: RehearsalSong = { simplification: "Stay on roots if the chorus entrance gets muddy.", setupNote: "Keep the attack short so the verse breathes.", transpositionPlan: "If the singer drops to B minor, keep the shape a whole step lower and let keys keep the color tones.", + pickupPlan: "Play this pickup with Lead Vocal; land the downbeat together.", manualOverrides: [], overlapWarnings: [ "Density warning: competing with Keyboard Left Hand in low register." @@ -1497,6 +1500,8 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "simplification", "setupNote", "transpositionPlan", + "pickupPlan", + "pickupPlanSource", "manualOverrides", "overlapWarnings", "transcription", @@ -1552,6 +1557,18 @@ function validateRehearsalRole(value: unknown, path: string): string | null { if (value.transpositionPlan !== undefined && typeof value.transpositionPlan !== "string") { return invalidField(`${path}.transpositionPlan`); } + if (value.pickupPlan !== undefined && typeof value.pickupPlan !== "string") { + return invalidField(`${path}.pickupPlan`); + } + if ( + value.pickupPlanSource !== undefined && + !isOneOf(PROVENANCE_SOURCES, value.pickupPlanSource) + ) { + return invalidField(`${path}.pickupPlanSource`); + } + if (value.pickupPlanSource !== undefined && value.pickupPlan === undefined) { + return invalidField(`${path}.pickupPlanSource`); + } if (!isDenseArray(value.manualOverrides)) { return invalidField(`${path}.manualOverrides`); } diff --git a/packages/shared-types/test/index.test.ts b/packages/shared-types/test/index.test.ts index 564ee1827..b77746c5c 100644 --- a/packages/shared-types/test/index.test.ts +++ b/packages/shared-types/test/index.test.ts @@ -738,6 +738,7 @@ describe("shared type helpers", () => { expect(song.sections[0]?.roles[2]?.harmony?.source).toBe("model"); expect(song.sections[0]?.roles[0]?.harmonicExplanation).toContain("tonal floor"); expect(song.sections[0]?.roles[0]?.transpositionPlan).toContain("whole step lower"); + expect(song.sections[0]?.roles[0]?.pickupPlan).toContain("Lead Vocal"); expect(song.collaboration?.assignments).toHaveLength(2); expect(song.collaboration?.comments[0]?.status).toBe("open"); expect(song.sections[0]?.roles[2]?.manualOverrides?.[0]).toMatchObject({ @@ -1257,6 +1258,12 @@ describe("shared type helpers", () => { song.sections[0]!.roles[0]!.transpositionPlan = 2 as never; }) }, + { + message: "sections[0].roles[0].pickupPlan", + payload: createInvalidSong((song) => { + song.sections[0]!.roles[0]!.pickupPlan = 2 as never; + }) + }, { message: "sections[0].roles[0].practiceProgress", payload: createInvalidSong((song) => { diff --git a/packages/shared-types/test/pickupPlanProvenance.test.ts b/packages/shared-types/test/pickupPlanProvenance.test.ts new file mode 100644 index 000000000..04dfcecfe --- /dev/null +++ b/packages/shared-types/test/pickupPlanProvenance.test.ts @@ -0,0 +1,30 @@ +import { createDemoRehearsalSong, parseRehearsalSong } from "../src/index"; + +describe("pickup plan provenance", () => { + it.each(["model", "user"] as const)("preserves explicit %s provenance", (source) => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.pickupPlan = "Play this pickup with Lead Vocal; land the downbeat together."; + role.pickupPlanSource = source; + + expect(parseRehearsalSong(song).sections[0]!.roles[0]!.pickupPlanSource).toBe(source); + }); + + it("rejects unsupported provenance values", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]! as unknown as Record; + role.pickupPlan = "Play this pickup with Lead Vocal; land the downbeat together."; + role.pickupPlanSource = "inferred"; + + expect(() => parseRehearsalSong(song)).toThrow(/pickupPlanSource/); + }); + + it("rejects provenance without a pickup plan", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + delete role.pickupPlan; + role.pickupPlanSource = "model"; + + expect(() => parseRehearsalSong(song)).toThrow(/pickupPlanSource/); + }); +}); diff --git a/services/analysis-engine/src/bandscope_analysis/roles/extractor.py b/services/analysis-engine/src/bandscope_analysis/roles/extractor.py index a0f092213..2cc992d02 100644 --- a/services/analysis-engine/src/bandscope_analysis/roles/extractor.py +++ b/services/analysis-engine/src/bandscope_analysis/roles/extractor.py @@ -22,6 +22,12 @@ logger = logging.getLogger(__name__) +_OTHER_STEM_ROLE_IDS = frozenset({"keys-left", "keys-right", "acoustic-guitar"}) +_OTHER_STEM_SOURCE_LABEL = "Keys / guitar" +_PICKUP_PLAN_PREFIX = "Play this pickup with " +_PICKUP_PLAN_SUFFIX = "; land the downbeat together." +_PICKUP_PLAN_BAND_TARGET = "the rest of the band" + class RoleExtractor: """Extracts roles and builds the part graph for song sections.""" @@ -71,8 +77,13 @@ def extract( # Real activity-based topology current_activity = activity_maps[i] next_activity = activity_maps[i + 1] if i + 1 < len(activity_maps) else None + previous_activity = activity_maps[i - 1] if i > 0 else None topology = self._build_activity_topology( - section_id, roles, current_activity, next_activity + section_id, + roles, + current_activity, + next_activity, + previous_activity, ) else: # Fallback to heuristic-based topology @@ -330,12 +341,67 @@ def _build_roles( "acoustic_guitar": acoustic_guitar_role, } + @staticmethod + def _activity_pickup_plan( + role_id: str, + roles: dict[str, RehearsalRole], + role_activity: dict[str, bool], + previous_role_activity: dict[str, bool] | None, + ) -> str | None: + """Return bounded pickup guidance only for a corroborated lead-in. + + A pickup plan is emitted only when real stem activity shows this role + becoming active after an abutting rest, with at least one other distinct + source already active on the landing downbeat. Mixed-source landing is + the evidence. Heuristic fallback topology and first-section (no previous + activity) produce no plan. A lone entrance is not a pickup. + """ + if ( + previous_role_activity is None + or previous_role_activity.get(role_id, False) + or not role_activity.get(role_id, False) + ): + return None + + landing_role_ids = [ + candidate_id for candidate_id, is_active in role_activity.items() if is_active + ] + named_source_ids = [ + candidate_id + for candidate_id in landing_role_ids + if candidate_id not in _OTHER_STEM_ROLE_IDS + ] + other_stem_landing = any( + candidate_id in _OTHER_STEM_ROLE_IDS for candidate_id in landing_role_ids + ) + source_count = len(named_source_ids) + (1 if other_stem_landing else 0) + if source_count < 2: + return None + if source_count >= 3: + return f"{_PICKUP_PLAN_PREFIX}{_PICKUP_PLAN_BAND_TARGET}{_PICKUP_PLAN_SUFFIX}" + + partner_ids = [candidate_id for candidate_id in named_source_ids if candidate_id != role_id] + other_name: str | None = None + if partner_ids: + other_id = partner_ids[0] + other_name = next( + (role["name"] for role in roles.values() if role["id"] == other_id), + None, + ) + elif other_stem_landing and role_id not in _OTHER_STEM_ROLE_IDS: + other_name = _OTHER_STEM_SOURCE_LABEL + + if other_name is None: + return None + return f"{_PICKUP_PLAN_PREFIX}{other_name}{_PICKUP_PLAN_SUFFIX}" + def _build_activity_topology( self, section_id: str, roles: dict[str, RehearsalRole], role_activity: dict[str, bool], next_role_activity: dict[str, bool] | None, + previous_role_activity: dict[str, bool] | None = None, ) -> SectionRoleTopology: """Build topology from real stem activity detection.""" handoffs = compute_handoffs(role_activity, next_role_activity) @@ -357,7 +423,18 @@ def _build_activity_topology( handoff_to, handoff_from = handoffs.get(role_id, ([], [])) if is_active: - active_roles.append(roles[role_key]) + role = roles[role_key] + pickup_plan = self._activity_pickup_plan( + role_id, + roles, + role_activity, + previous_role_activity, + ) + if pickup_plan is not None: + role = role.copy() + role["pickupPlan"] = pickup_plan + role["pickupPlanSource"] = "model" + active_roles.append(role) part_graph.append( { diff --git a/services/analysis-engine/src/bandscope_analysis/roles/model.py b/services/analysis-engine/src/bandscope_analysis/roles/model.py index ea6fc1449..5e9b0c06c 100644 --- a/services/analysis-engine/src/bandscope_analysis/roles/model.py +++ b/services/analysis-engine/src/bandscope_analysis/roles/model.py @@ -3,7 +3,7 @@ from __future__ import annotations from enum import Enum -from typing import Any, Literal, TypedDict +from typing import Any, Literal, NotRequired, TypedDict class RoleType(str, Enum): @@ -83,6 +83,8 @@ class RehearsalRole(TypedDict): setupNote: str manualOverrides: list[ManualOverride] overlapWarnings: list[str] + pickupPlan: NotRequired[str] + pickupPlanSource: NotRequired[Literal["model", "user"]] class PartGraphNode(TypedDict): diff --git a/services/analysis-engine/tests/test_pickup_plan_provenance.py b/services/analysis-engine/tests/test_pickup_plan_provenance.py new file mode 100644 index 000000000..ba9ef64ab --- /dev/null +++ b/services/analysis-engine/tests/test_pickup_plan_provenance.py @@ -0,0 +1,42 @@ +"""Regression coverage for pickup-plan provenance.""" + +from bandscope_analysis.roles.extractor import RoleExtractor + + +def test_activity_pickup_plan_marks_generated_copy_as_model_owned() -> None: + """Generated pickup copy carries explicit provenance into the role contract.""" + extractor = RoleExtractor() + empty_range = {"lowestNote": "", "highestNote": ""} + roles = extractor._build_roles("", empty_range, "", empty_range) + previous = { + "bass-guitar": True, + "keys-left": False, + "keys-right": False, + "lead-vocal": False, + "acoustic-guitar": False, + } + current = { + "bass-guitar": True, + "keys-left": False, + "keys-right": False, + "lead-vocal": True, + "acoustic-guitar": False, + } + + topology = extractor._build_activity_topology("verse-1", roles, current, None, previous) + generated_roles = [role for role in topology["active_roles"] if "pickupPlan" in role] + + assert generated_roles + assert all(role["pickupPlanSource"] == "model" for role in generated_roles) + + +def test_heuristic_topology_does_not_invent_pickup_provenance() -> None: + """Fallback topology must not claim provenance when it has no pickup plan.""" + extractor = RoleExtractor() + empty_range = {"lowestNote": "", "highestNote": ""} + roles = extractor._build_roles("", empty_range, "", empty_range) + + topology = extractor._build_topology("verse-1", True, roles) + + assert all("pickupPlan" not in role for role in topology["active_roles"]) + assert all("pickupPlanSource" not in role for role in topology["active_roles"]) diff --git a/services/analysis-engine/tests/test_pickup_plan_source_label.py b/services/analysis-engine/tests/test_pickup_plan_source_label.py new file mode 100644 index 000000000..30b412205 --- /dev/null +++ b/services/analysis-engine/tests/test_pickup_plan_source_label.py @@ -0,0 +1,15 @@ +"""Regression coverage for cross-layer pickup-plan source labels.""" + +from bandscope_analysis.roles.extractor import RoleExtractor + + +def test_activity_pickup_plan_names_shared_accompaniment_source_consistently() -> None: + """Generated pickup copy must use the same shared-source label as the workspace UI.""" + pickup_plan = RoleExtractor._activity_pickup_plan( + "bass-guitar", + {}, + {"bass-guitar": True, "keys-right": True}, + {"bass-guitar": False, "keys-right": True}, + ) + + assert pickup_plan == "Play this pickup with Keys / guitar; land the downbeat together." diff --git a/services/analysis-engine/tests/test_roles.py b/services/analysis-engine/tests/test_roles.py index 45a2ddada..3ee3c43b2 100644 --- a/services/analysis-engine/tests/test_roles.py +++ b/services/analysis-engine/tests/test_roles.py @@ -133,3 +133,164 @@ def test_role_extractor_falls_back_when_activity_detection_fails() -> None: assert result["topologies"][0]["section_id"] == "verse-1" assert result["topologies"][0]["part_graph"][0]["role_id"] == "bass-guitar" + + +def _extract_with_activity( + stem_activity: list[dict[str, bool]], + section_ids: list[str] | None = None, +) -> list[dict[str, dict[str, object]]]: + """Run RoleExtractor against a patched stem-activity map.""" + extractor = RoleExtractor() + sections = [{"id": section_id} for section_id in (section_ids or ["intro", "verse-1"])] + audio_features = { + "stems": {"bass": np.ones(200, dtype=np.float32)}, + "sr": 10, + "boundaries": [ + (float(index * 10), float((index + 1) * 10)) for index in range(len(sections)) + ], + } + with ( + patch( + "bandscope_analysis.roles.extractor.detect_stem_activity", + return_value=stem_activity, + ), + patch.object( + RoleExtractor, + "_extract_features", + return_value=( + {"lowestNote": "", "highestNote": ""}, + "", + {"lowestNote": "E1", "highestNote": "E3"}, + "Em", + ), + ), + ): + result = extractor.extract(sections, audio_features) + return [ + {role["id"]: role for role in topology["active_roles"]} for topology in result["topologies"] + ] + + +def test_role_extractor_emits_activity_corroborated_pickup_plan() -> None: + """Emit a pickup plan only when a resting part leads into a shared downbeat.""" + topologies = _extract_with_activity( + [ + {"bass": True, "vocals": False, "other": False}, + {"bass": True, "vocals": True, "other": False}, + ] + ) + assert topologies[0]["bass-guitar"].get("pickupPlan") is None + assert topologies[1]["lead-vocal"]["pickupPlan"] == ( + "Play this pickup with Bass Guitar; land the downbeat together." + ) + assert "pickupPlan" not in topologies[1]["bass-guitar"] + + +def test_role_extractor_groups_shared_other_stem_landing_for_pickup_plan() -> None: + """Name the shared accompaniment stem without inventing a specific instrument.""" + topologies = _extract_with_activity( + [ + {"bass": False, "vocals": False, "other": True}, + {"bass": True, "vocals": False, "other": True}, + ] + ) + assert topologies[1]["bass-guitar"]["pickupPlan"] == ( + "Play this pickup with Accompaniment; land the downbeat together." + ) + + +def test_role_extractor_keeps_mixed_landings_as_shared_pickup_evidence() -> None: + """Mixed landing is the pickup evidence, not an ambiguity.""" + topologies = _extract_with_activity( + [ + {"bass": True, "vocals": False, "other": True}, + {"bass": True, "vocals": True, "other": True}, + ] + ) + assert topologies[1]["lead-vocal"]["pickupPlan"] == ( + "Play this pickup with the rest of the band; land the downbeat together." + ) + + +def test_role_extractor_keeps_lone_entrance_pickup_plan_unnamed() -> None: + """A lone entrance is not a pickup with the band.""" + topologies = _extract_with_activity( + [ + {"bass": False, "vocals": False, "other": False}, + {"bass": True, "vocals": False, "other": False}, + ] + ) + assert "pickupPlan" not in topologies[1]["bass-guitar"] + + +def test_role_extractor_keeps_continuing_role_pickup_plan_unnamed() -> None: + """A part that was already playing is not leading in with a pickup.""" + topologies = _extract_with_activity( + [ + {"bass": True, "vocals": True, "other": False}, + {"bass": True, "vocals": True, "other": False}, + ] + ) + assert all("pickupPlan" not in role for role in topologies[1].values()) + + +def test_role_extractor_keeps_first_section_pickup_plan_unnamed() -> None: + """Without a previous section there is no rest-then-enter evidence.""" + extractor = RoleExtractor() + sections = [{"id": "intro"}] + audio_features = { + "stems": {"bass": np.ones(100, dtype=np.float32)}, + "sr": 10, + "boundaries": [(0.0, 10.0)], + } + with ( + patch( + "bandscope_analysis.roles.extractor.detect_stem_activity", + return_value=[{"bass": True, "vocals": True, "other": True}], + ), + patch.object( + RoleExtractor, + "_extract_features", + return_value=( + {"lowestNote": "", "highestNote": ""}, + "", + {"lowestNote": "E1", "highestNote": "E3"}, + "Em", + ), + ), + ): + result = extractor.extract(sections, audio_features) + intro_roles = {role["id"]: role for role in result["topologies"][0]["active_roles"]} + assert all("pickupPlan" not in role for role in intro_roles.values()) + + +def test_role_extractor_keeps_heuristic_pickup_plan_unnamed() -> None: + """Heuristic fallback topology must not invent a pickup.""" + extractor = RoleExtractor() + result = extractor.extract([{"id": "intro"}, {"id": "verse-1"}]) + intro_roles = {role["id"]: role for role in result["topologies"][0]["active_roles"]} + verse_roles = {role["id"]: role for role in result["topologies"][1]["active_roles"]} + assert all("pickupPlan" not in role for role in intro_roles.values()) + assert all("pickupPlan" not in role for role in verse_roles.values()) + + +def test_activity_pickup_plan_fails_closed_without_a_named_partner() -> None: + """Unknown landing partners stay unnamed instead of inventing copy.""" + assert ( + RoleExtractor._activity_pickup_plan( + "lead-vocal", + {}, + {"bass-guitar": True, "lead-vocal": True}, + {"bass-guitar": True, "lead-vocal": False}, + ) + is None + ) + assert ( + RoleExtractor._activity_pickup_plan( + "keys-right", + {}, + {"keys-right": True, "lead-vocal": True}, + {"keys-right": False, "lead-vocal": True}, + ) + is None + )
{section.label} · {formatTimelineTime(section.timeRange.start)}–{formatTimelineTime(section.timeRange.end)}