From 5058b7668ceebd64f33a783a51a11eb0da2bac0d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 18:36:53 +0000 Subject: [PATCH 1/7] feat(workspace): name tonight's first hit plan on the map Shared hits are a rehearsal coordination point: parts that activate together need a named next action on tonight's map. Emit owned hitPlan copy only when real stem activity shows at least two distinct sources becoming active in the same section. Mixed simultaneous activation is the evidence; heuristic topology and lone entrances stay unnamed. --- AGENTS.md | 1 + ARCHITECTURE.md | 3 +- CHANGELOG.md | 1 + CLAUDE.md | 2 +- .../FirstHitPlanCallout.identity.test.tsx | 21 + .../FirstHitPlanCallout.memoization.test.tsx | 25 ++ .../FirstHitPlanCallout.particle.test.tsx | 129 +++++++ ...irstHitPlanCallout.reduced-motion.test.tsx | 43 +++ .../workspace/FirstHitPlanCallout.test.tsx | 246 ++++++++++++ .../workspace/FirstHitPlanCallout.tsx | 198 ++++++++++ ...stHitPlanCallout.unavailable-copy.test.tsx | 34 ++ ...rstHitPlanCallout.workspace-scope.test.tsx | 54 +++ .../src/features/workspace/Workspace.test.tsx | 30 ++ .../src/features/workspace/Workspace.tsx | 11 +- .../workspace/coverageContract.test.ts | 13 + .../firstHitPlan.inherited-metadata.test.ts | 94 +++++ .../firstHitPlan.proxy-authority.test.ts | 83 ++++ .../firstHitPlan.section-label.test.ts | 14 + .../features/workspace/firstHitPlan.test.ts | 334 ++++++++++++++++ .../src/features/workspace/firstHitPlan.ts | 360 ++++++++++++++++++ apps/desktop/src/i18n/index.test.ts | 50 ++- apps/desktop/src/i18n/index.ts | 36 +- apps/desktop/src/locales/en/common.json | 9 +- apps/desktop/src/locales/ko/common.json | 9 +- apps/desktop/vite.config.ts | 21 +- docs/design-system/component-contract.md | 1 + ...educed-motion-first-hit-plan-navigation.md | 3 + packages/shared-types/src/index.ts | 6 + packages/shared-types/test/index.test.ts | 7 + .../src/bandscope_analysis/roles/extractor.py | 81 +++- .../src/bandscope_analysis/roles/model.py | 3 +- services/analysis-engine/tests/test_roles.py | 159 ++++++++ 32 files changed, 2062 insertions(+), 19 deletions(-) create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.identity.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.memoization.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.reduced-motion.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.unavailable-copy.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.workspace-scope.test.tsx create mode 100644 apps/desktop/src/features/workspace/coverageContract.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHitPlan.inherited-metadata.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHitPlan.proxy-authority.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHitPlan.section-label.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHitPlan.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHitPlan.ts create mode 100644 docs/doctoring/reduced-motion-first-hit-plan-navigation.md diff --git a/AGENTS.md b/AGENTS.md index fca448ce9..e4ff9272c 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 hit plan with the owning part when an active role is corroborated, the owned `hitPlan` 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, confirmed overrides, harmonic explanations, or confidence notes. - Do not reduce the product to a chord analyzer when form, timing, player coordination, 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 3302a6fc3..0dde9331c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1,10 +1,11 @@ # ARCHITECTURE.md -Last updated: 2026-03-11 +Last updated: 2026-08-26 ## 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 hit plan must name the owning part when corroborated, the owned `hitPlan` 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, confirmed overrides, harmonic explanations, or confidence notes. Distinct from 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-dynamics-plan, and first-articulation-plan. - 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 eea696893..484b7ab84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first hit plan in the mounted rehearsal workspace so parts that land a shared hit together can be named on the map; real analyzed songs now receive this guidance only when section-level stem activity shows at least two distinct sources becoming active together after a previous section, while mixed simultaneous activation is the evidence and 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. - 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 82c2c704a..d1adcb80c 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). `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 hit plan and opens 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, confirmed overrides, harmonic explanations, or confidence notes. Distinct from 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-dynamics-plan, and first-articulation-plan. `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/FirstHitPlanCallout.identity.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.identity.test.tsx new file mode 100644 index 000000000..ab1884411 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.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 { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +it("gives co-mounted hit-plan callouts distinct DOM identities", () => { + render( + <> + + + + ); + + const callouts = screen.getAllByRole("complementary", { + name: "Tonight's first hit 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/FirstHitPlanCallout.memoization.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.memoization.test.tsx new file mode 100644 index 000000000..3aae8b1d1 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.memoization.test.tsx @@ -0,0 +1,25 @@ +import { render } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +describe("FirstHitPlanCallout resolver reuse", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("does not rescan role metadata when a parent rerenders the same song object", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles.find((candidate) => candidate.id === "lead-vocal")!; + 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/FirstHitPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx new file mode 100644 index 000000000..ddd1a98c8 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx @@ -0,0 +1,129 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +describe("FirstHitPlanCallout Korean role copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending role names particle-safe before and after the hit action", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const seed = song.sections[0]!; + seed.roles = [ + { + ...seed.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + hitPlan: "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup." + } + ]; + seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + + 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 = "0"; + 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 hit template instead of exposing English guidance", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const seed = song.sections[0]!; + seed.roles = [ + { + ...seed.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + hitPlan: "Land this hit with Lead Vocal; don't drift past the downbeat." + } + ]; + seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + + render(); + + expect( + screen.getByText("Lead Vocal 파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") + ).toBeTruthy(); + expect( + screen.queryByText("Land this hit with Lead Vocal; don't drift past the downbeat.") + ).toBeNull(); + }); + + it("localizes the rest-of-band hit template instead of exposing English guidance", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const seed = song.sections[0]!; + seed.roles = [ + { + ...seed.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + hitPlan: "Land this hit with the rest of the band; don't drift past the downbeat." + } + ]; + seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + + render(); + + expect( + screen.getByText("나머지 밴드와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") + ).toBeTruthy(); + expect( + screen.queryByText("Land this hit with the rest of the band; don't drift past the downbeat.") + ).toBeNull(); + }); + + it("preserves the generated template shape when long target names are bounded", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const seed = song.sections[0]!; + const targetRole = `Lead-${"A".repeat(180)}`; + seed.roles = [ + { + ...seed.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + hitPlan: `Land this hit with ${targetRole}; don't drift past the downbeat.` + } + ]; + seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + + render(); + + expect(screen.queryByText(/^Land this hit with /)).toBeNull(); + expect( + screen.getByText( + (content) => + content.startsWith("Lead-") && content.endsWith("파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") + ) + ).toBeTruthy(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.reduced-motion.test.tsx new file mode 100644 index 000000000..4d4e2c5ec --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.reduced-motion.test.tsx @@ -0,0 +1,43 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +describe("FirstHitPlanCallout 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 = "0"; + 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 hit at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx new file mode 100644 index 000000000..a955ba5ab --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx @@ -0,0 +1,246 @@ +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 { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +const DEMO_HIT_PLAN = + "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup."; + +function songWithHitPlan() { + return createDemoRehearsalSong(); +} + +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 = "0"; + const scrollIntoView = vi.fn(); + Object.defineProperty(target, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + grid.appendChild(target); + timeline.appendChild(grid); + document.body.appendChild(timeline); + return { grid: timeline, scrollIntoView }; +} + +describe("FirstHitPlanCallout", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText("No hit 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 = songWithHitPlan(); + 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 hit at 0:10" })).toBeTruthy(); + }); + + it("contains a hostile song identity descriptor lookup instead of crashing the callout", () => { + const song = new Proxy(songWithHitPlan(), { + getOwnPropertyDescriptor() { + throw new Error("hostile song id descriptor"); + } + }); + + expect(() => render()).not.toThrow(); + expect( + screen.getByText("No hit 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 hit signature", () => { + const firstSong = songWithHitPlan(); + const nextSong = songWithHitPlan(); + 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 hit at 0:10" })); + expect( + screen.getByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect(screen.getByText("Bass Guitar has a shared hit in the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).toBeNull(); + + grid.remove(); + }); + + it("preserves armed guidance across immutable edits of the same owned song", () => { + const song = songWithHitPlan(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar hit at 0:10" })); + expect( + screen.getByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect( + screen.getByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + expect(screen.queryByText("Bass Guitar has a shared hit in the verse at 0:10.")).toBeNull(); + + grid.remove(); + }); + + it("does not show another part's hit plan under the named landing part", () => { + const song = songWithHitPlan(); + song.sections[0]!.roles[0]!.hitPlan = ""; + song.sections[0]!.roles[0]!.rehearsalPriority = "low"; + song.sections[0]!.roles[2]!.hitPlan = "Leave the vocal on the last lyric while the hit lands."; + song.sections[0]!.roles[2]!.rehearsalPriority = "low"; + song.sections[0]!.roles[1]!.hitPlan = + "Land this hit with Lead Vocal; don't drift past the downbeat."; + + render(); + + expect( + screen.getByText("Keyboard 1 Right Hand has a shared hit in the verse at 0:10.") + ).toBeTruthy(); + expect( + screen.getByText("Land this hit with Lead Vocal; don't drift past the downbeat.") + ).toBeTruthy(); + expect(screen.queryByText("Leave the vocal on the last lyric while the hit lands.")).toBeNull(); + expect(screen.queryByText(DEMO_HIT_PLAN)).toBeNull(); + }); + + it("names the first hit plan as map navigation, scrolls to its rendered section, and arms that action", () => { + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + expect(screen.getByText(DEMO_HIT_PLAN)).toBeTruthy(); + const action = screen.getByRole("button", { + name: "Open Bass Guitar hit at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).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 hit at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).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 hit at 0:10" })); + + expect(screen.getByText("Bass Guitar has a shared hit in the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).toBeNull(); + }); + + it("navigates by renderer-owned section position instead of untrusted analysis ids", () => { + const song = songWithHitPlan(); + song.sections[0]!.id = "analysis section / duplicate"; + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar hit 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="0"]'); + 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 hit at 0:10" })); + + expect(firstScroll).not.toHaveBeenCalled(); + expect(secondScroll).not.toHaveBeenCalled(); + expect(screen.getByText("Bass Guitar has a shared hit in 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 hit at 0:10" })); + + expect(first.scrollIntoView).not.toHaveBeenCalled(); + expect(second.scrollIntoView).not.toHaveBeenCalled(); + expect(screen.getByText("Bass Guitar has a shared hit in the verse at 0:10.")).toBeTruthy(); + + first.grid.remove(); + second.grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx new file mode 100644 index 000000000..05839bbdc --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx @@ -0,0 +1,198 @@ +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 { formatHitPlanTime, resolveFirstHitPlan } from "./firstHitPlan"; + +/** Props for the first hit-plan rehearsal callout. */ +export interface FirstHitPlanCalloutProps { + song: RehearsalSong; +} + +type HitPlanCopyValues = Readonly>; + +type OpenedHitPlan = Readonly<{ + songIdentity: unknown; + sectionId: string; + sectionIndex: number; + landingRoleId: string; + hitPlan: string; + atSeconds: number; +}>; + +const GENERATED_ACTIVITY_HIT_PLAN = + /^Land this hit with (.+); don't drift past the downbeat\.$/u; +const GENERATED_ACTIVITY_HIT_PLAN_BAND_TARGET = "the rest of the band"; + +/** Read a stable owned song id, falling back to object identity for untrusted identity metadata. */ +function stableHitPlanSongIdentity(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 hit-plan placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatHitPlanCopy(template: string, values: HitPlanCopyValues): string { + return template.replace(/\{(role|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof HitPlanCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Localize the analysis-engine-owned hit sentence while preserving custom role-owned guidance verbatim. */ +function localizedHitPlan( + hitPlan: string, + generatedTemplate: string, + generatedBandTemplate: string +): string { + const match = GENERATED_ACTIVITY_HIT_PLAN.exec(hitPlan); + const targetRole = match?.[1]?.trim() ?? ""; + if (targetRole.length === 0) { + return hitPlan; + } + if (targetRole === GENERATED_ACTIVITY_HIT_PLAN_BAND_TARGET) { + return generatedBandTemplate; + } + return generatedTemplate.replace("{target}", () => targetRole); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredHitPlanScrollBehavior(): 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 resolveHitPlanRenderer(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 hit plan and open the matching rendered map section. */ +export function FirstHitPlanCallout({ song }: FirstHitPlanCalloutProps) { + const calloutId = `workspace-surface-hit-plan-${useId()}`; + const locale = detectPreferredLocale(); + const t = createTranslator(locale); + const songIdentity = stableHitPlanSongIdentity(song); + const named = useMemo(() => resolveFirstHitPlan(song), [song]); + const [openedHitPlan, setOpenedHitPlan] = useState(null); + + useEffect(() => { + setOpenedHitPlan(null); + }, [ + songIdentity, + named?.sectionIndex, + named?.sectionId, + named?.landingRoleId, + named?.hitPlan, + named?.atSeconds + ]); + + if (!named) { + return ( + + ); + } + + const opened = + openedHitPlan !== null && + openedHitPlan.songIdentity === songIdentity && + openedHitPlan.sectionId === named.sectionId && + openedHitPlan.sectionIndex === named.sectionIndex && + openedHitPlan.landingRoleId === named.landingRoleId && + openedHitPlan.hitPlan === named.hitPlan && + openedHitPlan.atSeconds === named.atSeconds; + const at = formatHitPlanTime(named.atSeconds); + const copyValues: HitPlanCopyValues = { + role: named.landingRoleName, + section: translateSectionFormLabel(locale, named.sectionLabel), + at + }; + const actionLabel = formatHitPlanCopy(t("firstHitPlanOpenAction"), copyValues); + const body = formatHitPlanCopy(t("firstHitPlanBody"), copyValues); + const armed = formatHitPlanCopy(t("firstHitPlanArmed"), copyValues); + const hitPlan = localizedHitPlan( + named.hitPlan, + t("firstHitPlanGeneratedGuidance"), + t("firstHitPlanGeneratedBandGuidance") + ); + + return ( + + ); +} diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.unavailable-copy.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.unavailable-copy.test.tsx new file mode 100644 index 000000000..84618d73f --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.unavailable-copy.test.tsx @@ -0,0 +1,34 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +function songWithoutHitPlan() { + const song = createDemoRehearsalSong(); + for (const section of song.sections) { + for (const role of section.roles) { + role.hitPlan = ""; + } + } + return song; +} + +describe("FirstHitPlanCallout unavailable copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("does not assert why the English hit plan is unavailable", () => { + render(); + + expect(screen.getByText("No hit plan is available. Stay on tonight's map for the next rehearsal cue.")).toBeTruthy(); + }); + + it("does not assert why the Korean hit plan is unavailable", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + + render(); + + expect(screen.getByText("사용 가능한 히트 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요.")).toBeTruthy(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.workspace-scope.test.tsx new file mode 100644 index 000000000..e25b2b62b --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.workspace-scope.test.tsx @@ -0,0 +1,54 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it, vi } from "vitest"; +import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +describe("FirstHitPlanCallout workspace scope", () => { + it("opens the song-structure renderer owned by the current workspace", () => { + const firstSong = createDemoRehearsalSong(); + const secondSong = createDemoRehearsalSong(); + secondSong.id = "second-workspace-song"; + + const { container } = render( + <> +
+ +
+
+
+
+
+ +
+
+
+
+ + ); + + const targets = container.querySelectorAll('[data-section-index="0"]'); + 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 hit 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 a3da5ffe6..966a697a0 100644 --- a/apps/desktop/src/features/workspace/Workspace.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.test.tsx @@ -270,4 +270,34 @@ describe("Workspace", () => { expect(screen.getByText("합주 우선순위")).toBeTruthy(); expect(screen.getByText("역할과 화성")).toBeTruthy(); }); + + it("names tonight's first hit plan as workspace navigation", () => { + setNavigatorLanguage("en-US"); + const song = createDemoRehearsalSong(); + + render(); + + const target = screen.getByTestId("song-structure-grid").children.item(0); + expect(target).toBeTruthy(); + const scrollIntoView = vi.fn(); + Object.defineProperty(target!, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + + expect( + screen.getAllByText( + "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup." + ).length + ).toBeGreaterThan(0); + const action = screen.getByRole("button", { + name: "Open Bass Guitar hit at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Land that hit on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + }); }); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 71546b524..a3d8e8e3b 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 { FirstHitPlanCallout } from "./FirstHitPlanCallout"; import { createTranslator, detectPreferredLocale } from "../../i18n"; import { generateCueSheetCsv, generateChartSummaryJson, generateMetadataHandoffJson, sanitizeFilename } from "../../lib/export"; import { Button } from "@/components/ui/button"; @@ -90,8 +91,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)}

@@ -331,6 +336,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..495213c1e --- /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 hit-plan resolver and callout inside the coverage gate", () => { + expect(DESKTOP_OWNED_PRODUCTION_COVERAGE).toEqual( + expect.arrayContaining([ + "src/features/workspace/firstHitPlan.ts", + "src/features/workspace/FirstHitPlanCallout.tsx" + ]) + ); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHitPlan.inherited-metadata.test.ts b/apps/desktop/src/features/workspace/firstHitPlan.inherited-metadata.test.ts new file mode 100644 index 000000000..30759f5f6 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHitPlan.inherited-metadata.test.ts @@ -0,0 +1,94 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstHitPlan } from "./firstHitPlan"; + +function songWithHitPlan() { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + section.id = "hit-own"; + section.roles = [ + { + ...section.roles[0]!, + id: "bass-guitar", + name: "Bass Guitar", + rehearsalPriority: "high", + hitPlan: "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup." + } + ]; + section.partGraph = [{ role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] }]; + song.sections = [section]; + return { song, section }; +} + +describe("resolveFirstHitPlan inherited metadata", () => { + it("rejects a song or section whose required metadata is inherited", () => { + const { song, section } = songWithHitPlan(); + const inheritedSong = Object.create({ sections: song.sections }) as typeof song; + expect(resolveFirstHitPlan(inheritedSong)).toBeNull(); + + const inheritedSection = Object.create(section) as typeof section; + song.sections = [inheritedSection]; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("rejects inherited timing fields", () => { + const { song, section } = songWithHitPlan(); + section.timeRange = Object.create({ start: 10, end: 30 }) as typeof section.timeRange; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("contains exceptions from own runtime accessors instead of trusting them", () => { + const { song, section } = songWithHitPlan(); + Object.defineProperty(section.roles[0]!, "hitPlan", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile hitPlan getter"); + } + }); + + expect(() => resolveFirstHitPlan(song)).not.toThrow(); + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("does not treat own accessors as stable hit-plan identity authority", () => { + const { song, section } = songWithHitPlan(); + Object.defineProperty(section, "id", { + configurable: true, + enumerable: true, + get() { + return "hit-own"; + } + }); + + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("does not let inherited hit plans establish the named copy", () => { + const { song, section } = songWithHitPlan(); + const inheritedRole = Object.create({ + hitPlan: "Inherited hit plan" + }) as (typeof section.roles)[0]; + Object.defineProperties(inheritedRole, { + id: { configurable: true, enumerable: true, value: "lead-vocal" }, + name: { configurable: true, enumerable: true, value: "Lead Vocal" }, + rehearsalPriority: { configurable: true, enumerable: true, value: "high" } + }); + section.roles = [inheritedRole]; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("does not let inherited role or graph metadata establish the landing part", () => { + const { song, section } = songWithHitPlan(); + const node = section.partGraph[0]!; + section.partGraph = [Object.create(node) as typeof node]; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("rejects arrays masquerading as section records", () => { + const { song, section } = songWithHitPlan(); + const arraySection = Object.assign([], section) as unknown as typeof section; + song.sections = [arraySection]; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHitPlan.proxy-authority.test.ts b/apps/desktop/src/features/workspace/firstHitPlan.proxy-authority.test.ts new file mode 100644 index 000000000..dad68ce2a --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHitPlan.proxy-authority.test.ts @@ -0,0 +1,83 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstHitPlan } from "./firstHitPlan"; + +const DEMO_HIT_PLAN = + "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup."; + +describe("resolveFirstHitPlan own-data authority", () => { + it("uses the snapshotted own-data hit plan instead of a Proxy get trap", () => { + const song = createDemoRehearsalSong(); + 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 hit-plan fixture is missing the expected Bass Guitar role."); + } + + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "hitPlan") { + return "Injected proxy hit."; + } + return Reflect.get(target, property, receiver); + } + }); + + expect(resolveFirstHitPlan(song)?.hitPlan).toBe(DEMO_HIT_PLAN); + }); + + it("uses the snapshotted own-data time range instead of a Proxy get trap", () => { + const song = createDemoRehearsalSong(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + expect(section).toBeDefined(); + if (!section) { + throw new Error("Demo hit-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(resolveFirstHitPlan(song)?.atSeconds).toBe(expectedStart); + }); + + it("returns snapshotted role identity and display copy instead of Proxy get values", () => { + const song = createDemoRehearsalSong(); + 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 hit-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 = resolveFirstHitPlan(song) as + | (ReturnType & { + landingRoleId?: string; + landingRoleName?: string; + }) + | null; + expect(resolved?.hitPlan).toBe(DEMO_HIT_PLAN); + expect(resolved?.landingRoleId).toBe(expectedId); + expect(resolved?.landingRoleName).toBe(expectedName); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHitPlan.section-label.test.ts b/apps/desktop/src/features/workspace/firstHitPlan.section-label.test.ts new file mode 100644 index 000000000..8d09a1bb4 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHitPlan.section-label.test.ts @@ -0,0 +1,14 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstHitPlan } from "./firstHitPlan"; + +describe("resolveFirstHitPlan section-label authority", () => { + it("fails closed when runtime metadata supplies a label outside the shared SectionFormLabel contract", () => { + const song = createDemoRehearsalSong(); + const section = song.sections[0]!; + + (section as unknown as { label: string }).label = "verse-legacy"; + + expect(resolveFirstHitPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHitPlan.test.ts b/apps/desktop/src/features/workspace/firstHitPlan.test.ts new file mode 100644 index 000000000..6999812c8 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHitPlan.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, it } from "vitest"; +import { MAX_SECTION_TIME_SECONDS, createDemoRehearsalSong } from "@bandscope/shared-types"; +import { formatHitPlanTime, resolveFirstHitPlan } from "./firstHitPlan"; + +const DEMO_HIT_PLAN = + "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup."; + +function withHitSection( + overrides: { + id?: string; + start?: number; + end?: number; + hitPlan?: string; + label?: "intro" | "verse" | "pre-chorus" | "chorus" | "bridge" | "outro" | "tag" | "pickup" | "stop" | "handoff"; + roleId?: string; + roleName?: string; + priority?: "low" | "medium" | "high"; + isActive?: boolean; + functionLabel?: string; + } = {} +) { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const section = structuredClone(verse); + section.id = overrides.id ?? "verse-hit"; + section.label = overrides.label ?? "verse"; + section.groove = "Straight eighths with a late snare feel"; + section.timeRange = { start: overrides.start ?? 10, end: overrides.end ?? 30 }; + const roleId = overrides.roleId ?? "lead-vocal"; + section.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." + }, + hitPlan: overrides.hitPlan ?? DEMO_HIT_PLAN, + manualOverrides: [] + } + ]; + section.partGraph = [ + { + role_id: roleId, + is_active: overrides.isActive ?? true, + handoff_to: [], + handoff_from: [] + } + ]; + song.sections = [section]; + return song; +} + +describe("resolveFirstHitPlan", () => { + it("picks the demo song's earliest hit plan and the part that lands it", () => { + const resolved = resolveFirstHitPlan(createDemoRehearsalSong()); + expect(resolved?.section.id).toBe("verse-1"); + expect(resolved?.landingRole.id).toBe("bass-guitar"); + expect(resolved?.hitPlan).toBe(DEMO_HIT_PLAN); + expect(resolved?.atSeconds).toBe(10); + expect(formatHitPlanTime(resolved?.atSeconds ?? -1)).toBe("0:10"); + expect(formatHitPlanTime(Number.NaN)).toBe("0:00"); + expect(formatHitPlanTime(-4)).toBe("0:00"); + }); + + it("does not invent a hit 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, confirmed overrides, harmonic explanations, or confidence notes", () => { + const song = withHitSection(); + delete song.sections[0]!.roles[0]!.hitPlan; + song.sections[0]!.groove = "Straight eighths with a late snare feel"; + song.sections[0]!.roles[0]!.simplification = "Keep the sustained note centered."; + song.sections[0]!.roles[0]!.setupNote = DEMO_HIT_PLAN; + song.sections[0]!.roles[0]!.transpositionPlan = + "If the singer drops to B minor, keep the shape a whole step lower."; + (song.sections[0]!.roles[0] as { vampPlan?: string }).vampPlan = + "Keep this part going until Lead Vocal enters in the next section."; + (song.sections[0]!.roles[0] as { fillPlan?: string }).fillPlan = + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty."; + (song.sections[0]!.roles[0] as { tuningPlan?: string }).tuningPlan = + "Tune the E string down to D so the verse riff sits on the open fifth."; + (song.sections[0]!.roles[0] as { dynamicsPlan?: string }).dynamicsPlan = + "Keep the verse under the vocal so the chorus still has somewhere to lift."; + (song.sections[0]!.roles[0] as { articulationPlan?: string }).articulationPlan = + "Shorten the last chorus vowel so the band can hear the cutoff."; + (song.sections[0]!.roles[0] as { hookPlan?: string }).hookPlan = + "Lead vocal carries the chorus hook; lock the melody before anyone stacks harmony."; + (song.sections[0]!.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[0]!.roles[0] as { padPlan?: string }).padPlan = + "Drop to a two-bar pad so the Keyboard 1 Right Hand run can land."; + song.sections[0]!.roles[0]!.cue = { kind: "lyric", value: "city lights" }; + song.sections[0]!.roles[0]!.range = { lowestNote: "G#3", highestNote: "C#5" }; + song.sections[0]!.roles[0]!.overlapWarnings = ["Melodic overlap: competing with Keyboard 1 Right Hand."]; + song.sections[0]!.roles[0]!.harmony = { + chord: "C#m7", + functionLabel: "vi melodic pull", + source: "user" + }; + song.sections[0]!.roles[0]!.harmonicExplanation = "The ninth is the reason this lift works."; + song.sections[0]!.roles[0]!.manualOverrides = [ + { + field: "harmony", + value: { + chord: "C#m11", + functionLabel: "vi suspended lift", + source: "user" + }, + source: "user" + } + ]; + song.sections[0]!.roles[0]!.confidence = { + level: "high", + source: "user", + notes: DEMO_HIT_PLAN + }; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("skips a blank hit plan", () => { + expect(resolveFirstHitPlan(withHitSection({ hitPlan: " " }))).toBeNull(); + }); + + it("skips a multi-line hit plan", () => { + expect( + resolveFirstHitPlan(withHitSection({ hitPlan: "Keep the melody centered.\nLeave the stack." })) + ).toBeNull(); + }); + + it("prefers the earlier of two hit plans", () => { + const song = withHitSection({ + id: "verse-late-hit", + start: 40, + end: 56, + roleId: "keys-right", + hitPlan: "Late hit." + }); + const earlier = structuredClone(song.sections[0]!); + earlier.id = "verse-early"; + earlier.roles = [ + { + ...earlier.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "low", + hitPlan: "Earlier hit." + } + ]; + earlier.timeRange = { start: 8, end: 24 }; + earlier.partGraph = [{ role_id: "lead-vocal", is_active: true, handoff_to: [], handoff_from: [] }]; + song.sections = [song.sections[0]!, earlier]; + + const resolved = resolveFirstHitPlan(song); + expect(resolved?.section.id).toBe("verse-early"); + expect(resolved?.landingRole.id).toBe("lead-vocal"); + expect(resolved?.hitPlan).toBe("Earlier hit."); + expect(resolved?.atSeconds).toBe(8); + }); + + it("breaks same-time hit-plan ties with locale-independent id ordering", () => { + const song = withHitSection({ id: "ä-hit", start: 10, end: 26 }); + const ascii = structuredClone(song.sections[0]!); + ascii.id = "z-hit"; + song.sections = [song.sections[0]!, ascii]; + + expect(resolveFirstHitPlan(song)?.section.id).toBe("z-hit"); + }); + + it("prefers a high-priority hit part over a low-priority part in the same section", () => { + const song = withHitSection({ + roleId: "keys-right", + roleName: "Keys", + priority: "low", + hitPlan: "Low-priority hit." + }); + const section = song.sections[0]!; + const highRole = { + ...section.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "high" as const, + hitPlan: "High-priority hit." + }; + 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: [] } + ]; + + expect(resolveFirstHitPlan(song)?.landingRole.id).toBe("lead-vocal"); + expect(resolveFirstHitPlan(song)?.hitPlan).toBe("High-priority hit."); + }); + + it("breaks equal-priority role ties with locale-independent id ordering", () => { + const song = withHitSection({ roleId: "ä-role", roleName: "Umlaut role", priority: "high" }); + const section = song.sections[0]!; + const asciiRole = { + ...section.roles[0]!, + id: "z-role", + name: "ASCII role", + hitPlan: "ASCII hit." + }; + 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: [] } + ]; + + expect(resolveFirstHitPlan(song)?.landingRole.id).toBe("z-role"); + expect(resolveFirstHitPlan(song)?.hitPlan).toBe("ASCII hit."); + }); + + it("skips a hit plan whose graph node is inactive", () => { + expect(resolveFirstHitPlan(withHitSection({ isActive: false }))).toBeNull(); + }); + + it("skips a hit plan whose rehearsal window is unbounded", () => { + expect(resolveFirstHitPlan(withHitSection({ start: Number.NaN, end: 30 }))).toBeNull(); + }); + + it("skips a hit plan whose end precedes its start", () => { + expect(resolveFirstHitPlan(withHitSection({ start: 30, end: 10 }))).toBeNull(); + }); + + it("skips a zero-length hit-plan window", () => { + expect(resolveFirstHitPlan(withHitSection({ start: 10, end: 10 }))).toBeNull(); + }); + + it("skips a hit plan whose endpoint overflows the shared timing bound", () => { + expect( + resolveFirstHitPlan( + withHitSection({ + start: MAX_SECTION_TIME_SECONDS, + end: MAX_SECTION_TIME_SECONDS + 1 + }) + ) + ).toBeNull(); + }); + + it("returns null for a non-object song root", () => { + expect(resolveFirstHitPlan(null as never)).toBeNull(); + }); + + it("skips non-object roles and graph nodes without inventing a landing part", () => { + const song = withHitSection(); + song.sections[0]!.roles = [null as never, song.sections[0]!.roles[0]!]; + song.sections[0]!.partGraph = [null as never, song.sections[0]!.partGraph[0]!]; + expect(resolveFirstHitPlan(song)?.landingRole.id).toBe("lead-vocal"); + }); + + it("returns null when the runtime section collection is sparse", () => { + const song = withHitSection(); + const sparseSections: typeof song.sections = new Array(2); + sparseSections[1] = song.sections[0]!; + song.sections = sparseSections; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("keeps the hit plan unnamed when role identities are duplicated", () => { + const song = withHitSection(); + const role = song.sections[0]!.roles[0]!; + song.sections[0]!.roles = [role, { ...role }]; + song.sections[0]!.partGraph = [ + { role_id: role.id, is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: role.id, is_active: true, handoff_to: [], handoff_from: [] } + ]; + expect(resolveFirstHitPlan(song)).toBeNull(); + }); + + it("bounds the hit plan to 180 Unicode code points", () => { + const song = withHitSection({ hitPlan: `${"G".repeat(200)}` }); + const resolved = resolveFirstHitPlan(song); + expect(resolved?.hitPlan.length).toBe(180); + }); + + it("does not split a Unicode surrogate pair at the hit-plan boundary", () => { + const song = withHitSection({ hitPlan: `${"a".repeat(179)}😀tail` }); + const resolved = resolveFirstHitPlan(song); + expect(Array.from(resolved?.hitPlan ?? "")).toHaveLength(180); + expect(resolved?.hitPlan.endsWith("😀")).toBe(true); + }); + + it("keeps the generated activity sentence recognizable after bounding a long partner name", () => { + const target = `Lead-${"A".repeat(180)}`; + const song = withHitSection({ + hitPlan: `Land this hit with ${target}; don't drift past the downbeat.` + }); + const resolved = resolveFirstHitPlan(song); + expect(resolved?.hitPlan.startsWith("Land this hit with Lead-")).toBe(true); + expect(resolved?.hitPlan.endsWith("; don't drift past the downbeat.")).toBe(true); + expect(Array.from(resolved?.hitPlan ?? "").length).toBeLessThanOrEqual(180); + }); + + it("preserves a short generated shared-hit sentence", () => { + const song = withHitSection({ + hitPlan: "Land this hit with Lead Vocal; don't drift past the downbeat." + }); + expect(resolveFirstHitPlan(song)?.hitPlan).toBe( + "Land this hit with Lead Vocal; don't drift past the downbeat." + ); + }); + + it("does not treat an empty generated partner as structured guidance", () => { + const song = withHitSection({ + hitPlan: "Land this hit with ; don't drift past the downbeat." + }); + expect(resolveFirstHitPlan(song)?.hitPlan).toBe( + "Land this hit with ; don't drift past the downbeat." + ); + }); + + it("contains exceptions from the runtime root instead of crashing", () => { + const song = new Proxy(withHitSection(), { + getOwnPropertyDescriptor() { + throw new Error("hostile descriptor"); + } + }); + expect(() => resolveFirstHitPlan(song as never)).not.toThrow(); + expect(resolveFirstHitPlan(song as never)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHitPlan.ts b/apps/desktop/src/features/workspace/firstHitPlan.ts new file mode 100644 index 000000000..86f841a95 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHitPlan.ts @@ -0,0 +1,360 @@ +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_HIT_PLAN_CHARACTERS = 180; +const GENERATED_ACTIVITY_HIT_PLAN_PREFIX = "Land this hit with "; +const GENERATED_ACTIVITY_HIT_PLAN_SUFFIX = "; don't drift past the downbeat."; +const GENERATED_ACTIVITY_HIT_PLAN_FIXED_CHARACTERS = Array.from( + GENERATED_ACTIVITY_HIT_PLAN_PREFIX + GENERATED_ACTIVITY_HIT_PLAN_SUFFIX +).length; +const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); + +type RankedRoleMetadata = Readonly<{ + role: RehearsalRole; + id: string; + name: string; + rehearsalPriority: keyof typeof PRIORITY_RANK; +}>; + +/** Tonight's first hit plan: the earliest labeled section and the part that lands it. */ +export type FirstHitPlan = { + section: RehearsalSection; + sectionId: string; + sectionLabel: RehearsalSection["label"]; + sectionIndex: number; + landingRole: RehearsalRole; + landingRoleId: string; + landingRoleName: string; + hitPlan: string; + atSeconds: number; +}; + +/** Format a non-negative hit-plan time as m:ss for rehearsal copy. */ +export function formatHitPlanTime(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); +} + +/** Keep a bounded engine-owned hit sentence structurally recognizable for localization. */ +function boundedGeneratedActivityHitPlan(value: string): string | null { + if ( + !value.startsWith(GENERATED_ACTIVITY_HIT_PLAN_PREFIX) || + !value.endsWith(GENERATED_ACTIVITY_HIT_PLAN_SUFFIX) + ) { + return null; + } + const target = value + .slice( + GENERATED_ACTIVITY_HIT_PLAN_PREFIX.length, + value.length - GENERATED_ACTIVITY_HIT_PLAN_SUFFIX.length + ) + .trim(); + if (target.length === 0) { + return null; + } + const boundedTarget = truncateCodePoints( + target, + MAX_HIT_PLAN_CHARACTERS - GENERATED_ACTIVITY_HIT_PLAN_FIXED_CHARACTERS + ); + return `${GENERATED_ACTIVITY_HIT_PLAN_PREFIX}${boundedTarget}${GENERATED_ACTIVITY_HIT_PLAN_SUFFIX}`; +} + +/** Return a bounded snapshotted own hit plan, or null when it cannot be shown. */ +function ownedHitPlan(role: unknown): string | null { + if (!isRuntimeObject(role)) { + return null; + } + const hitPlan = ownDataValue(role, "hitPlan"); + if (typeof hitPlan !== "string") { + return null; + } + const trimmed = hitPlan.trim(); + if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + return null; + } + return ( + boundedGeneratedActivityHitPlan(trimmed) ?? + truncateCodePoints(trimmed, MAX_HIT_PLAN_CHARACTERS) + ); +} + +/** 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; +} + +/** Prefer the earlier ranked role, then rehearsal priority, then a locale-independent id. */ +function pickLandingRole(roles: RankedRoleMetadata[]): RankedRoleMetadata | 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] + : []; + }); +} + +/** Resolve a hit plan after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstHitPlan(song: RehearsalSong): FirstHitPlan | 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)) { + return []; + } + const sectionId = ownDataValue(section, "id"); + const sectionLabel = ownDataValue(section, "label"); + const timeRange = ownedBoundedTimeRange(section as RehearsalSection); + if ( + typeof sectionId !== "string" || + sectionId.trim().length === 0 || + typeof sectionLabel !== "string" || + !SECTION_FORM_LABEL_SET.has(sectionLabel) || + timeRange === null + ) { + return []; + } + + const landingRole = pickLandingRole( + rankedActiveRoles(section as RehearsalSection).filter( + (metadata) => ownedHitPlan(metadata.role) !== null + ) + ); + if (!landingRole) { + return []; + } + const hitPlan = ownedHitPlan(landingRole.role); + if (!hitPlan) { + return []; + } + return [ + { + section: section as RehearsalSection, + sectionId, + sectionLabel: sectionLabel as RehearsalSection["label"], + sectionIndex, + landingRole: landingRole.role, + landingRoleId: landingRole.id, + landingRoleName: landingRole.name, + hitPlan, + 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 hit plan, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstHitPlan(song: RehearsalSong): FirstHitPlan | null { + try { + return resolveSafeFirstHitPlan(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..c5596f1c4 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-hit-plan next-action copy particle-safe", () => { + const t = createTranslator("ko"); + expect(t("firstHitPlanOpenAction")).toBe("{at} {role} 히트 열기"); + expect(t("firstHitPlanBody")).toBe("{at} {section}에서 {role} 파트의 히트 계획이 있습니다."); + expect(t("firstHitPlanArmed")).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 39f716d50..e0042ac2e 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -148,5 +148,12 @@ "practiceProgressRegionLabel": "Practice Progress", "practiceProgressLabel": "Practice Progress", "decreasePracticeProgressLabel": "Decrease progress", - "increasePracticeProgressLabel": "Increase progress" + "increasePracticeProgressLabel": "Increase progress", + "firstHitPlanLabel": "Tonight's first hit plan", + "firstHitPlanOpenAction": "Open {role} hit at {at}", + "firstHitPlanBody": "{role} has a shared hit in the {section} at {at}.", + "firstHitPlanArmed": "Land that hit on {role} at {at} before the room starts.", + "firstHitPlanGeneratedGuidance": "Land this hit with {target}; don't drift past the downbeat.", + "firstHitPlanGeneratedBandGuidance": "Land this hit with the rest of the band; don't drift past the downbeat.", + "firstHitPlanUnavailable": "No hit plan is available. Stay on tonight's map for the next rehearsal cue." } diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 371884abb..a2bebf426 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -148,5 +148,12 @@ "practiceProgressRegionLabel": "연습 진척도", "practiceProgressLabel": "연습 진척도", "decreasePracticeProgressLabel": "진척도 감소", - "increasePracticeProgressLabel": "진척도 증가" + "increasePracticeProgressLabel": "진척도 증가", + "firstHitPlanLabel": "오늘 첫 히트 계획", + "firstHitPlanOpenAction": "{at} {role} 히트 열기", + "firstHitPlanBody": "{at} {section}에서 {role} 파트의 히트 계획이 있습니다.", + "firstHitPlanArmed": "{at}에서 {role} 파트의 히트를 맞춘 다음 합주를 시작하세요.", + "firstHitPlanGeneratedGuidance": "{target} 파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.", + "firstHitPlanGeneratedBandGuidance": "나머지 밴드와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.", + "firstHitPlanUnavailable": "사용 가능한 히트 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요." } diff --git a/apps/desktop/vite.config.ts b/apps/desktop/vite.config.ts index f1db6f2b8..0bd5e0840 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/firstHitPlan.ts", + "src/features/workspace/FirstHitPlanCallout.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..b27fcb6e1 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 Hit Plan Callout | workspace next-action pattern | `apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx` | Name the owning part when an active graph node corroborates it, the owned `hitPlan` 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`, confirmed overrides, `harmonicExplanation`, or confidence notes. Open scrolls the renderer-owned song-structure section. Keep the unavailable state guidance-only. Distinct from 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-dynamics-plan, and first-articulation-plan. | | 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-hit-plan-navigation.md b/docs/doctoring/reduced-motion-first-hit-plan-navigation.md new file mode 100644 index 000000000..533826845 --- /dev/null +++ b/docs/doctoring/reduced-motion-first-hit-plan-navigation.md @@ -0,0 +1,3 @@ +# Reduced-motion first hit-plan navigation + +Open tonight's first hit 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..8a957b7d7 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -143,6 +143,7 @@ export type RehearsalRole = { overlapWarnings: string[]; transcription?: TranscriptionNote[]; practiceProgress?: number; + hitPlan?: string; }; /** Documented. */ @@ -474,6 +475,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.", + hitPlan: "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup.", manualOverrides: [], overlapWarnings: [ "Density warning: competing with Keyboard Left Hand in low register." @@ -1497,6 +1499,7 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "simplification", "setupNote", "transpositionPlan", + "hitPlan", "manualOverrides", "overlapWarnings", "transcription", @@ -1552,6 +1555,9 @@ function validateRehearsalRole(value: unknown, path: string): string | null { if (value.transpositionPlan !== undefined && typeof value.transpositionPlan !== "string") { return invalidField(`${path}.transpositionPlan`); } + if (value.hitPlan !== undefined && typeof value.hitPlan !== "string") { + return invalidField(`${path}.hitPlan`); + } 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..ae6893f50 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]?.hitPlan).toContain("verse downbeat"); 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].hitPlan", + payload: createInvalidSong((song) => { + song.sections[0]!.roles[0]!.hitPlan = 2 as never; + }) + }, { message: "sections[0].roles[0].practiceProgress", payload: createInvalidSong((song) => { diff --git a/services/analysis-engine/src/bandscope_analysis/roles/extractor.py b/services/analysis-engine/src/bandscope_analysis/roles/extractor.py index a0f092213..f050b6918 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 = "Accompaniment" +_HIT_PLAN_PREFIX = "Land this hit with " +_HIT_PLAN_SUFFIX = "; don't drift past the downbeat." +_HIT_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,68 @@ def _build_roles( "acoustic_guitar": acoustic_guitar_role, } + @staticmethod + def _activity_hit_plan( + role_id: str, + roles: dict[str, RehearsalRole], + previous_role_activity: dict[str, bool] | None, + role_activity: dict[str, bool], + ) -> str | None: + """Return bounded hit guidance only for a shared simultaneous activation. + + A hit plan is emitted only when real stem activity shows this role + becoming active with at least one other distinct source in the same + section. Mixed-source simultaneous activation is the evidence. Heuristic + fallback topology and first-section (no previous activity) produce no plan. + """ + if ( + previous_role_activity is None + or not role_activity.get(role_id, False) + or previous_role_activity.get(role_id, False) + ): + return None + + activating_role_ids = [ + candidate_id + for candidate_id, is_active in role_activity.items() + if is_active and not previous_role_activity.get(candidate_id, False) + ] + named_source_ids = [ + candidate_id + for candidate_id in activating_role_ids + if candidate_id not in _OTHER_STEM_ROLE_IDS + ] + other_stem_activating = any( + candidate_id in _OTHER_STEM_ROLE_IDS for candidate_id in activating_role_ids + ) + source_count = len(named_source_ids) + (1 if other_stem_activating else 0) + if source_count < 2: + return None + if source_count >= 3: + return f"{_HIT_PLAN_PREFIX}{_HIT_PLAN_BAND_TARGET}{_HIT_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_activating and role_id not in _OTHER_STEM_ROLE_IDS: + other_name = _OTHER_STEM_SOURCE_LABEL + + if other_name is None: + return None + return f"{_HIT_PLAN_PREFIX}{other_name}{_HIT_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 +424,17 @@ 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] + hit_plan = self._activity_hit_plan( + role_id, + roles, + previous_role_activity, + role_activity, + ) + if hit_plan is not None: + role = role.copy() + role["hitPlan"] = hit_plan + 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..0630d6a51 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,7 @@ class RehearsalRole(TypedDict): setupNote: str manualOverrides: list[ManualOverride] overlapWarnings: list[str] + hitPlan: NotRequired[str] class PartGraphNode(TypedDict): diff --git a/services/analysis-engine/tests/test_roles.py b/services/analysis-engine/tests/test_roles.py index 45a2ddada..c1a7842ac 100644 --- a/services/analysis-engine/tests/test_roles.py +++ b/services/analysis-engine/tests/test_roles.py @@ -133,3 +133,162 @@ 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, +) -> 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 ["verse-1", "chorus-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 result["topologies"][-1]["active_roles"]} + + +def test_role_extractor_emits_activity_corroborated_hit_plan() -> None: + """Emit a hit plan only when two distinct sources activate together.""" + chorus_roles = _extract_with_activity( + [ + {"bass": False, "vocals": False, "other": False}, + {"bass": True, "vocals": True, "other": False}, + ] + ) + assert chorus_roles["bass-guitar"]["hitPlan"] == ( + "Land this hit with Lead Vocal; don't drift past the downbeat." + ) + assert chorus_roles["lead-vocal"]["hitPlan"] == ( + "Land this hit with Bass Guitar; don't drift past the downbeat." + ) + + +def test_role_extractor_groups_shared_other_stem_activation_for_hit_plan() -> None: + """Name the shared accompaniment stem without inventing a specific instrument.""" + chorus_roles = _extract_with_activity( + [ + {"bass": False, "vocals": False, "other": False}, + {"bass": True, "vocals": False, "other": True}, + ] + ) + assert chorus_roles["bass-guitar"]["hitPlan"] == ( + "Land this hit with Accompaniment; don't drift past the downbeat." + ) + assert chorus_roles["keys-right"]["hitPlan"] == ( + "Land this hit with Bass Guitar; don't drift past the downbeat." + ) + + +def test_role_extractor_keeps_mixed_activations_as_shared_hit_evidence() -> None: + """Mixed simultaneous activation is the shared-hit evidence, not an ambiguity.""" + chorus_roles = _extract_with_activity( + [ + {"bass": False, "vocals": False, "other": False}, + {"bass": True, "vocals": True, "other": True}, + ] + ) + assert chorus_roles["bass-guitar"]["hitPlan"] == ( + "Land this hit with the rest of the band; don't drift past the downbeat." + ) + assert chorus_roles["lead-vocal"]["hitPlan"] == ( + "Land this hit with the rest of the band; don't drift past the downbeat." + ) + assert chorus_roles["acoustic-guitar"]["hitPlan"] == ( + "Land this hit with the rest of the band; don't drift past the downbeat." + ) + + +def test_role_extractor_keeps_single_entrance_hit_plan_unnamed() -> None: + """A lone entrance is not a shared hit.""" + chorus_roles = _extract_with_activity( + [ + {"bass": True, "vocals": False, "other": False}, + {"bass": True, "vocals": True, "other": False}, + ] + ) + assert "hitPlan" not in chorus_roles["bass-guitar"] + assert "hitPlan" not in chorus_roles["lead-vocal"] + + +def test_role_extractor_keeps_first_section_hit_plan_unnamed() -> None: + """Without a previous section there is no shared-activation 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 "hitPlan" not in intro_roles["bass-guitar"] + + +def test_role_extractor_keeps_heuristic_hit_plan_unnamed() -> None: + """Heuristic fallback topology must not invent a shared hit.""" + 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("hitPlan" not in role for role in intro_roles.values()) + assert all("hitPlan" not in role for role in verse_roles.values()) + + +def test_activity_hit_plan_fails_closed_without_a_named_partner() -> None: + """Unknown activation partners stay unnamed instead of inventing copy.""" + assert ( + RoleExtractor._activity_hit_plan( + "bass-guitar", + {}, + {"bass-guitar": False, "lead-vocal": False}, + {"bass-guitar": True, "lead-vocal": True}, + ) + is None + ) + assert ( + RoleExtractor._activity_hit_plan( + "keys-right", + {}, + {"keys-right": False, "lead-vocal": False}, + {"keys-right": True, "lead-vocal": True}, + ) + is None + ) From ec1d3077434c698e8731f2e31a7be66aa8c79207 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 13:07:15 -0700 Subject: [PATCH 2/7] test(workspace): clean hit-plan timeline fixtures --- .../src/features/workspace/FirstHitPlanCallout.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx index a955ba5ab..b5fe928f7 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.test.tsx @@ -5,6 +5,7 @@ import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; const DEMO_HIT_PLAN = "Land this hit with Lead Vocal on the verse downbeat; don't drift past the pickup."; +const appendedSongStructureTargets = new Set(); function songWithHitPlan() { return createDemoRehearsalSong(); @@ -26,11 +27,16 @@ function appendSongStructureTarget(ariaLabel = "Scrollable song structure timeli grid.appendChild(target); timeline.appendChild(grid); document.body.appendChild(timeline); + appendedSongStructureTargets.add(timeline); return { grid: timeline, scrollIntoView }; } describe("FirstHitPlanCallout", () => { afterEach(() => { + for (const timeline of appendedSongStructureTargets) { + timeline.remove(); + } + appendedSongStructureTargets.clear(); vi.unstubAllGlobals(); }); From cc9feea6994c88893b972ea5d127ce35e2faf41d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:40:24 -0700 Subject: [PATCH 3/7] test(workspace): preserve custom hit copy provenance --- .../FirstHitPlanCallout.provenance.test.tsx | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx new file mode 100644 index 000000000..0f1749afa --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx @@ -0,0 +1,34 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstHitPlanCallout } from "./FirstHitPlanCallout"; + +describe("FirstHitPlanCallout hit-plan provenance", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("preserves custom hit guidance that happens to match the engine sentence shape", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const section = song.sections[0]!; + const customPlan = "Land this hit with Lead Vocal; don't drift past the downbeat."; + section.roles = [ + { + ...section.roles[2]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + hitPlan: customPlan + } + ]; + section.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + + render(); + + expect(screen.getByText(customPlan)).toBeTruthy(); + expect( + screen.queryByText("Lead Vocal 파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") + ).toBeNull(); + }); +}); From 708b87001f0afcf4f83cefd8a4abe21687a5f1c1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 16:34:04 -0700 Subject: [PATCH 4/7] perf(workspace): memoize hit plan translator --- apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx index 05839bbdc..c58184b40 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx @@ -99,8 +99,8 @@ function resolveHitPlanRenderer(origin: HTMLElement): HTMLElement | null { /** Name tonight's first hit plan and open the matching rendered map section. */ export function FirstHitPlanCallout({ song }: FirstHitPlanCalloutProps) { const calloutId = `workspace-surface-hit-plan-${useId()}`; - const locale = detectPreferredLocale(); - const t = createTranslator(locale); + const locale = useMemo(() => detectPreferredLocale(), []); + const t = useMemo(() => createTranslator(locale), [locale]); const songIdentity = stableHitPlanSongIdentity(song); const named = useMemo(() => resolveFirstHitPlan(song), [song]); const [openedHitPlan, setOpenedHitPlan] = useState(null); From 4e3d91a91e68a6d67e1c523148aafc7ffaf944e8 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Wed, 26 Aug 2026 21:57:39 +0900 Subject: [PATCH 5/7] fix(hit-plan): stop re-localizing custom shaped guidance; describe RehearsalRole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit localizedHitPlan re-localized any sentence matching the engine shape, so role-owned copy that happened to match was swapped for the ko template — exactly the coupling Devin flagged and the provenance test pins. The engine only names a lineup part (possibly bounded by the resolver) or the whole band, so localization now requires the target to equal or prefix-match a name in this section's lineup; everything else passes through verbatim. Also adds descriptive JSDoc on RehearsalRole/hitPlan per CodeRabbit. Evidence: hit-plan suites 12 files, 58/58. --- .../FirstHitPlanCallout.particle.test.tsx | 22 +++++++- .../FirstHitPlanCallout.provenance.test.tsx | 19 +++++++ .../workspace/FirstHitPlanCallout.tsx | 51 ++++++++++++++++++- packages/shared-types/src/index.ts | 3 +- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx index ddd1a98c8..710b9e2a6 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.particle.test.tsx @@ -61,9 +61,18 @@ describe("FirstHitPlanCallout Korean role copy", () => { name: "피아노", rehearsalPriority: "high", hitPlan: "Land this hit with Lead Vocal; don't drift past the downbeat." + }, + { + ...seed.roles[2]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "medium" } ]; - seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + seed.partGraph = [ + { role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "lead-vocal", is_active: true, handoff_to: [], handoff_from: [] } + ]; render(); @@ -112,9 +121,18 @@ describe("FirstHitPlanCallout Korean role copy", () => { name: "피아노", rehearsalPriority: "high", hitPlan: `Land this hit with ${targetRole}; don't drift past the downbeat.` + }, + { + ...seed.roles[2]!, + id: "long-part", + name: targetRole, + rehearsalPriority: "medium" } ]; - seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; + seed.partGraph = [ + { role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "long-part", is_active: true, handoff_to: [], handoff_from: [] } + ]; render(); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx index 0f1749afa..512afd279 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx @@ -31,4 +31,23 @@ describe("FirstHitPlanCallout hit-plan provenance", () => { screen.queryByText("Lead Vocal 파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") ).toBeNull(); }); + + it("localizes engine-shaped guidance whose target names a part in this lineup", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const section = song.sections[0]!; + for (const role of section.roles) { + role.hitPlan = ""; + } + const engineLikePlan = + "Land this hit with Keyboard 1 Right Hand; don't drift past the downbeat."; + section.roles[1]!.hitPlan = engineLikePlan; + + render(); + + expect( + screen.getByText("Keyboard 1 Right Hand 파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") + ).toBeTruthy(); + expect(screen.queryByText(engineLikePlan)).toBeNull(); + }); }); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx index c58184b40..7729a1dc9 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx @@ -59,7 +59,8 @@ function formatHitPlanCopy(template: string, values: HitPlanCopyValues): string function localizedHitPlan( hitPlan: string, generatedTemplate: string, - generatedBandTemplate: string + generatedBandTemplate: string, + knownSectionRoleNames: ReadonlySet ): string { const match = GENERATED_ACTIVITY_HIT_PLAN.exec(hitPlan); const targetRole = match?.[1]?.trim() ?? ""; @@ -69,9 +70,49 @@ function localizedHitPlan( if (targetRole === GENERATED_ACTIVITY_HIT_PLAN_BAND_TARGET) { return generatedBandTemplate; } + // The engine only ever names a part from this section's lineup or the whole + // band. A shaped sentence naming an absent part is role-owned copy and must + // stay verbatim instead of being re-localized. + // The engine only ever names a part from this section's lineup or the whole + // band; the resolver may have bounded a long partner name, so lineup names + // count as matches when they start with the plan's target. Anything else is + // role-owned copy and must stay verbatim instead of being re-localized. + const matchesLineup = Array.from(knownSectionRoleNames).some((name) => + name.startsWith(targetRole) + ); + if (!matchesLineup) { + return hitPlan; + } return generatedTemplate.replace("{target}", () => targetRole); } +/** Collect this section's role names through own-data reads so target checks never invoke accessors. */ +function collectSectionRoleNames(section: RehearsalSection): ReadonlySet { + const names = new Set(); + try { + const rolesDescriptor = Object.getOwnPropertyDescriptor(section as object, "roles"); + const roles = + rolesDescriptor && "value" in rolesDescriptor ? rolesDescriptor.value : undefined; + if (!Array.isArray(roles)) { + return names; + } + for (const role of roles) { + if (role === null || typeof role !== "object") { + continue; + } + const nameDescriptor = Object.getOwnPropertyDescriptor(role, "name"); + const name = + nameDescriptor && "value" in nameDescriptor ? nameDescriptor.value : undefined; + if (typeof name === "string" && name.trim().length > 0) { + names.add(name.trim()); + } + } + } catch { + return new Set(); + } + return names; +} + /** Use immediate scrolling when the operating system requests reduced motion. */ function preferredHitPlanScrollBehavior(): ScrollBehavior { return typeof window.matchMedia === "function" && @@ -103,6 +144,11 @@ export function FirstHitPlanCallout({ song }: FirstHitPlanCalloutProps) { const t = useMemo(() => createTranslator(locale), [locale]); const songIdentity = stableHitPlanSongIdentity(song); const named = useMemo(() => resolveFirstHitPlan(song), [song]); + // Derived from the memoized plan so rerenders never rescan role metadata. + const sectionRoleNames = useMemo( + () => (named ? collectSectionRoleNames(named.section) : new Set()), + [named] + ); const [openedHitPlan, setOpenedHitPlan] = useState(null); useEffect(() => { @@ -151,7 +197,8 @@ export function FirstHitPlanCallout({ song }: FirstHitPlanCalloutProps) { const hitPlan = localizedHitPlan( named.hitPlan, t("firstHitPlanGeneratedGuidance"), - t("firstHitPlanGeneratedBandGuidance") + t("firstHitPlanGeneratedBandGuidance"), + sectionRoleNames ); return ( diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index 8a957b7d7..f82b6a483 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 as surfaced on one section of tonight's map. */ export type RehearsalRole = { id: string; name: string; @@ -143,6 +143,7 @@ export type RehearsalRole = { overlapWarnings: string[]; transcription?: TranscriptionNote[]; practiceProgress?: number; + /** Optional activity-backed guidance shown for this role in this section; absent when the engine has no hitPlan for it. */ hitPlan?: string; }; From 5c3b1702241aea9cc208f227c8fa187dc7046421 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Wed, 26 Aug 2026 22:20:20 +0900 Subject: [PATCH 6/7] fix(hit-plan): localize engine source labels absent from lineup The lineup gate missed the engine's own 'Accompaniment' source label (no such section role), leaving that generated plan English under ko. Engine-owned targets are now accepted explicitly, with the band target keeping its dedicated template. --- .../FirstHitPlanCallout.provenance.test.tsx | 18 ++++++++++++++++++ .../features/workspace/FirstHitPlanCallout.tsx | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx index 512afd279..66ca4937b 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.provenance.test.tsx @@ -50,4 +50,22 @@ describe("FirstHitPlanCallout hit-plan provenance", () => { ).toBeTruthy(); expect(screen.queryByText(engineLikePlan)).toBeNull(); }); + + it("localizes the engine's Accompaniment source label even without a lineup match", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const section = song.sections[0]!; + for (const role of section.roles) { + role.hitPlan = ""; + } + section.roles[1]!.hitPlan = + "Land this hit with Accompaniment; don't drift past the downbeat."; + + render(); + + expect( + screen.getByText("Accompaniment 파트와 이 히트를 맞추세요. 다운비트 뒤로 밀리지 마세요.") + ).toBeTruthy(); + expect(screen.queryByText(/Land this hit with Accompaniment/)).toBeNull(); + }); }); diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx index 7729a1dc9..fc54dda5a 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx @@ -27,6 +27,11 @@ type OpenedHitPlan = Readonly<{ const GENERATED_ACTIVITY_HIT_PLAN = /^Land this hit with (.+); don't drift past the downbeat\.$/u; const GENERATED_ACTIVITY_HIT_PLAN_BAND_TARGET = "the rest of the band"; +/** Engine-owned source labels that can appear as targets without being section lineup names. */ +const GENERATED_ACTIVITY_HIT_PLAN_ENGINE_TARGETS = new Set([ + GENERATED_ACTIVITY_HIT_PLAN_BAND_TARGET, + "Accompaniment" +]); /** Read a stable owned song id, falling back to object identity for untrusted identity metadata. */ function stableHitPlanSongIdentity(song: RehearsalSong): unknown { @@ -70,6 +75,9 @@ function localizedHitPlan( if (targetRole === GENERATED_ACTIVITY_HIT_PLAN_BAND_TARGET) { return generatedBandTemplate; } + if (GENERATED_ACTIVITY_HIT_PLAN_ENGINE_TARGETS.has(targetRole)) { + return generatedTemplate.replace("{target}", () => targetRole); + } // The engine only ever names a part from this section's lineup or the whole // band. A shaped sentence naming an absent part is role-owned copy and must // stay verbatim instead of being re-localized. From c3f3b4ec64dbd62afa679366dc0965501df42693 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Wed, 26 Aug 2026 22:49:52 +0900 Subject: [PATCH 7/7] fix(types): import RehearsalSection for collectSectionRoleNames annotation --- apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx index fc54dda5a..969413bff 100644 --- a/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstHitPlanCallout.tsx @@ -1,5 +1,5 @@ import { useEffect, useId, useMemo, useState } from "react"; -import type { RehearsalSong } from "@bandscope/shared-types"; +import type { RehearsalSection, RehearsalSong } from "@bandscope/shared-types"; import { Button } from "@/components/ui/button"; import { createTranslator,