From 812d60a93cf4ee5ac9cdceb4f96ce4b9f09cf7b5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:12:44 +0000 Subject: [PATCH 01/14] feat(workspace): name tonight's first voicing plan on the map Surface the owned voicing plan as the next rehearsal action and open the matching rendered map section. Do not invent voicing from groove, cues, simplification, overlap, range, chords, function labels, setup notes, transposition, tuning, dynamics, articulation, overrides, harmonic explanations, or confidence notes. --- AGENTS.md | 1 + ARCHITECTURE.md | 3 +- CHANGELOG.md | 1 + CLAUDE.md | 2 +- .../FirstVoicingPlanCallout.identity.test.tsx | 21 ++ .../FirstVoicingPlanCallout.particle.test.tsx | 52 ++++ ...VoicingPlanCallout.reduced-motion.test.tsx | 43 +++ .../FirstVoicingPlanCallout.test.tsx | 262 ++++++++++++++++ .../workspace/FirstVoicingPlanCallout.tsx | 179 +++++++++++ ...icingPlanCallout.unavailable-copy.test.tsx | 32 ++ ...oicingPlanCallout.workspace-scope.test.tsx | 54 ++++ .../src/features/workspace/Workspace.test.tsx | 30 ++ .../src/features/workspace/Workspace.tsx | 11 +- ...irstVoicingPlan.inherited-metadata.test.ts | 94 ++++++ .../firstVoicingPlan.section-label.test.ts | 14 + .../workspace/firstVoicingPlan.test.ts | 281 +++++++++++++++++ .../features/workspace/firstVoicingPlan.ts | 283 ++++++++++++++++++ apps/desktop/src/i18n/index.test.ts | 49 ++- apps/desktop/src/i18n/index.ts | 36 ++- apps/desktop/src/locales/en/common.json | 7 +- apps/desktop/src/locales/ko/common.json | 7 +- docs/design-system/component-contract.md | 1 + ...ed-motion-first-voicing-plan-navigation.md | 3 + packages/shared-types/src/index.ts | 6 + packages/shared-types/test/index.test.ts | 7 + 25 files changed, 1471 insertions(+), 8 deletions(-) create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.identity.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.unavailable-copy.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.inherited-metadata.test.ts create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.section-label.test.ts create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.test.ts create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.ts create mode 100644 docs/doctoring/reduced-motion-first-voicing-plan-navigation.md diff --git a/AGENTS.md b/AGENTS.md index fca448ce9..ab82124b4 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 voicing plan with the owning part when an active role is corroborated, the owned `voicingPlan` 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, tuning plans, dynamics plans, articulation 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..7fb151b30 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1,10 +1,11 @@ # ARCHITECTURE.md -Last updated: 2026-03-11 +Last updated: 2026-08-24 ## 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 voicing plan must name the owning part when corroborated, the owned `voicingPlan` 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, tuning plans, dynamics plans, articulation plans, confirmed overrides, harmonic explanations, or confidence notes. Distinct from 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..6db7f282b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first voicing plan in the mounted rehearsal workspace so a part can lock the owned voicing before rehearsal; the Open action moves to the matching rendered map section, while inherited or accessor-backed runtime metadata remains guidance-only instead of becoming 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..656d8ad3e 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 voicing 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, tuning plans, dynamics plans, articulation plans, confirmed overrides, harmonic explanations, or confidence notes. Distinct from 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/FirstVoicingPlanCallout.identity.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.identity.test.tsx new file mode 100644 index 000000000..0ec2aa0d7 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.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 { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; + +it("gives co-mounted voicing-plan callouts distinct DOM identities", () => { + render( + <> + + + + ); + + const callouts = screen.getAllByRole("complementary", { + name: "Tonight's first voicing 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/FirstVoicingPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx new file mode 100644 index 000000000..7213d0f0a --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx @@ -0,0 +1,52 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; + +describe("FirstVoicingPlanCallout Korean role copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending role names particle-safe before and after the voicing action", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const seed = song.sections[0]!; + seed.roles = [ + { + ...seed.roles[1]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + voicingPlan: "Keep the verse voicing in first inversion so the top line still sings over the guitars." + } + ]; + 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(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx new file mode 100644 index 000000000..93d7ac0af --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.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 { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; + +describe("FirstVoicingPlanCallout 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 Keyboard 1 Right Hand voicing at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx new file mode 100644 index 000000000..f0ffcc5aa --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx @@ -0,0 +1,262 @@ +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 { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; + +const DEMO_VOICING_PLAN = + "Keep the verse voicing in first inversion so the top line still sings over the guitars."; + +function songWithVoicingPlan() { + 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("FirstVoicingPlanCallout", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText("No voicing 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 = songWithVoicingPlan(); + 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 Keyboard 1 Right Hand voicing at 0:10" })).toBeTruthy(); + }); + + it("resets armed guidance when accessor-id songs change with the same voicing signature", () => { + const firstSong = songWithVoicingPlan(); + const nextSong = songWithVoicingPlan(); + 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 Keyboard 1 Right Hand voicing at 0:10" })); + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect(screen.getByText("Keyboard 1 Right Hand still has a voicing plan in the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Lock that voicing on Keyboard 1 Right Hand at 0:10 before the room starts./) + ).toBeNull(); + + grid.remove(); + }); + + it("preserves armed guidance across immutable edits of the same owned song", () => { + const song = songWithVoicingPlan(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Keyboard 1 Right Hand voicing at 0:10" })); + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand at 0:10 before the room starts./) + ).toBeTruthy(); + expect(screen.queryByText("Keyboard 1 Right Hand still has a voicing plan in the verse at 0:10.")).toBeNull(); + + grid.remove(); + }); + + it("does not show another part's voicing plan under the named holding part", () => { + const song = songWithVoicingPlan(); + song.sections[0]!.roles[1]!.voicingPlan = ""; + song.sections[0]!.roles[1]!.rehearsalPriority = "low"; + song.sections[0]!.roles[0]!.voicingPlan = + "Stay on roots under the vocal so the chorus still has space for the fifth."; + song.sections[0]!.roles[2]!.voicingPlan = "Keep concert pitch even if the band drops the last chorus."; + + render(); + + expect( + screen.getByText("Bass Guitar still has a voicing plan in the verse at 0:10.") + ).toBeTruthy(); + expect( + screen.getByText("Stay on roots under the vocal so the chorus still has space for the fifth.") + ).toBeTruthy(); + expect(screen.queryByText("Keep concert pitch even if the band drops the last chorus.")).toBeNull(); + expect(screen.queryByText(DEMO_VOICING_PLAN)).toBeNull(); + }); + + it("names the first voicing plan as map navigation, scrolls to its rendered section, and arms that action", () => { + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + expect(screen.getByText(DEMO_VOICING_PLAN)).toBeTruthy(); + const action = screen.getByRole("button", { + name: "Open Keyboard 1 Right Hand voicing at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand 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 Keyboard 1 Right Hand voicing at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand 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 Keyboard 1 Right Hand voicing at 0:10" })); + + expect(screen.getByText("Keyboard 1 Right Hand still has a voicing plan in the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Lock that voicing on Keyboard 1 Right Hand at 0:10 before the room starts./) + ).toBeNull(); + }); + + it("navigates by renderer-owned section position instead of untrusted analysis ids", () => { + const song = songWithVoicingPlan(); + song.sections[0]!.id = "analysis section / duplicate"; + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Keyboard 1 Right Hand voicing at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + + grid.remove(); + }); + + it("scopes map navigation to the song-structure renderer when another surface reuses an index", () => { + const decoy = document.createElement("div"); + decoy.dataset.sectionIndex = "0"; + const decoyScrollIntoView = vi.fn(); + Object.defineProperty(decoy, "scrollIntoView", { + configurable: true, + value: decoyScrollIntoView + }); + document.body.appendChild(decoy); + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Keyboard 1 Right Hand voicing at 0:10" })); + + expect(decoyScrollIntoView).not.toHaveBeenCalled(); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + + decoy.remove(); + grid.remove(); + }); + + it("shows fresh guidance when the first voicing plan changes or returns later", () => { + const initialSong = songWithVoicingPlan(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + fireEvent.click(screen.getByRole("button", { name: "Open Keyboard 1 Right Hand voicing at 0:10" })); + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand at 0:10 before the room starts./) + ).toBeTruthy(); + + const nextSong = songWithVoicingPlan(); + nextSong.id = "next-song"; + nextSong.sections[0]!.timeRange = { start: 20, end: 40 }; + rerender(); + expect(screen.getByText("Keyboard 1 Right Hand still has a voicing plan in the verse at 0:20.")).toBeTruthy(); + + grid.remove(); + }); + + it("keeps an unavailable voicing plan guidance-only", () => { + const song = songWithVoicingPlan(); + for (const role of song.sections[0]!.roles) { + role.voicingPlan = ""; + } + render(); + expect(screen.queryByRole("button")).toBeNull(); + expect( + screen.getByRole("complementary", { name: "Tonight's first voicing plan" }) + ).toBeTruthy(); + expect( + screen.getByText("No voicing plan is available. Stay on tonight's map for the next rehearsal cue.") + ).toBeTruthy(); + }); + + it("localizes the voicing-plan form label instead of exposing its raw enum in Korean copy", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithVoicingPlan(); + song.sections[0]!.roles[1]!.name = "키보드"; + + render(); + + expect(screen.getByText("0:10 벌스에서 키보드 파트의 보이싱 계획이 있습니다.")).toBeTruthy(); + expect(screen.queryByText(/verse에서/)).toBeNull(); + }); + + it("renders the owned voicing plan as a text node instead of template syntax", () => { + const song = songWithVoicingPlan(); + song.sections[0]!.roles[0]!.voicingPlan = undefined; + song.sections[0]!.roles[2]!.voicingPlan = undefined; + song.sections[0]!.roles[1]!.voicingPlan = "Check {role} at {at}"; + render(); + expect(screen.getByText("Check {role} at {at}")).toBeTruthy(); + expect(screen.queryByText("Check Keyboard 1 Right Hand at 0:10")).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx new file mode 100644 index 000000000..3899dd7c7 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx @@ -0,0 +1,179 @@ +import { useEffect, useId, useState } from "react"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { Button } from "@/components/ui/button"; +import { + createTranslator, + detectPreferredLocale, + translateSectionFormLabel +} from "../../i18n"; +import { formatVoicingPlanTime, resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +/** Props for the first voicing-plan rehearsal callout. */ +export interface FirstVoicingPlanCalloutProps { + song: RehearsalSong; +} + +type VoicingPlanCopyValues = Readonly>; + +type OpenedVoicingPlan = Readonly<{ + songIdentity: unknown; + sectionId: string; + sectionIndex: number; + holdingRoleId: string; + voicingPlan: string; + atSeconds: number; +}>; + +/** Read a stable owned song id, falling back to object identity for untrusted identity metadata. */ +function stableVoicingPlanSongIdentity(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 voicing-plan placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatVoicingPlanCopy(template: string, values: VoicingPlanCopyValues): string { + return template.replace(/\{(role|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof VoicingPlanCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredVoicingPlanScrollBehavior(): 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 resolveVoicingPlanRenderer(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 voicing plan and open the matching rendered map section. */ +export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) { + const calloutId = `workspace-surface-voicing-plan-${useId()}`; + const locale = detectPreferredLocale(); + const t = createTranslator(locale); + const songIdentity = stableVoicingPlanSongIdentity(song); + const runtimeSong = song as unknown as Partial | null; + const named = resolveFirstVoicingPlan(song); + const namedSectionIndex = + named && Array.isArray(runtimeSong?.sections) + ? runtimeSong.sections.indexOf(named.section) + : -1; + const [openedVoicingPlan, setOpenedVoicingPlan] = useState(null); + + useEffect(() => { + setOpenedVoicingPlan(null); + }, [ + songIdentity, + namedSectionIndex, + named?.section.id, + named?.holdingRole.id, + named?.voicingPlan, + named?.atSeconds + ]); + + if (!named) { + return ( + + ); + } + + const opened = + openedVoicingPlan !== null && + openedVoicingPlan.songIdentity === songIdentity && + openedVoicingPlan.sectionId === named.section.id && + openedVoicingPlan.sectionIndex === namedSectionIndex && + openedVoicingPlan.holdingRoleId === named.holdingRole.id && + openedVoicingPlan.voicingPlan === named.voicingPlan && + openedVoicingPlan.atSeconds === named.atSeconds; + const at = formatVoicingPlanTime(named.atSeconds); + const copyValues: VoicingPlanCopyValues = { + role: named.holdingRole.name, + section: translateSectionFormLabel(locale, named.section.label), + at + }; + const actionLabel = formatVoicingPlanCopy(t("firstVoicingPlanOpenAction"), copyValues); + const body = formatVoicingPlanCopy(t("firstVoicingPlanBody"), copyValues); + const armed = formatVoicingPlanCopy(t("firstVoicingPlanArmed"), copyValues); + + return ( + + ); +} diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.unavailable-copy.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.unavailable-copy.test.tsx new file mode 100644 index 000000000..3e8b14117 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.unavailable-copy.test.tsx @@ -0,0 +1,32 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; + +function songWithoutVoicingPlan() { + const song = createDemoRehearsalSong(); + for (const role of song.sections[0]!.roles) { + role.voicingPlan = ""; + } + return song; +} + +describe("FirstVoicingPlanCallout unavailable copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("does not assert why the English voicing plan is unavailable", () => { + render(); + + expect(screen.getByText("No voicing plan is available. Stay on tonight's map for the next rehearsal cue.")).toBeTruthy(); + }); + + it("does not assert why the Korean voicing plan is unavailable", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + + render(); + + expect(screen.getByText("사용 가능한 보이싱 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요.")).toBeTruthy(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx new file mode 100644 index 000000000..a17c2dc6e --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.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 { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; + +describe("FirstVoicingPlanCallout 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 Keyboard 1 Right Hand voicing 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..da62f1a32 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 voicing 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( + "Keep the verse voicing in first inversion so the top line still sings over the guitars." + ).length + ).toBeGreaterThan(0); + const action = screen.getByRole("button", { + name: "Open Keyboard 1 Right Hand voicing at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lock that voicing on Keyboard 1 Right Hand 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..8dcda7857 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 { FirstVoicingPlanCallout } from "./FirstVoicingPlanCallout"; 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/firstVoicingPlan.inherited-metadata.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.inherited-metadata.test.ts new file mode 100644 index 000000000..12acf782b --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.inherited-metadata.test.ts @@ -0,0 +1,94 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +function songWithVoicingPlan() { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + section.id = "voicing-own"; + section.roles = [ + { + ...section.roles[1]!, + id: "keys-right", + name: "Keyboard 1 Right Hand", + rehearsalPriority: "high", + voicingPlan: "Keep the verse voicing in first inversion so the top line still sings over the guitars." + } + ]; + section.partGraph = [{ role_id: "keys-right", is_active: true, handoff_to: [], handoff_from: [] }]; + song.sections = [section]; + return { song, section }; +} + +describe("resolveFirstVoicingPlan inherited metadata", () => { + it("rejects a song or section whose required metadata is inherited", () => { + const { song, section } = songWithVoicingPlan(); + const inheritedSong = Object.create({ sections: song.sections }) as typeof song; + expect(resolveFirstVoicingPlan(inheritedSong)).toBeNull(); + + const inheritedSection = Object.create(section) as typeof section; + song.sections = [inheritedSection]; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("rejects inherited timing fields", () => { + const { song, section } = songWithVoicingPlan(); + section.timeRange = Object.create({ start: 10, end: 30 }) as typeof section.timeRange; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("contains exceptions from own runtime accessors instead of trusting them", () => { + const { song, section } = songWithVoicingPlan(); + Object.defineProperty(section.roles[0]!, "voicingPlan", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile voicingPlan getter"); + } + }); + + expect(() => resolveFirstVoicingPlan(song)).not.toThrow(); + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("does not treat own accessors as stable voicing-plan identity authority", () => { + const { song, section } = songWithVoicingPlan(); + Object.defineProperty(section, "id", { + configurable: true, + enumerable: true, + get() { + return "voicing-own"; + } + }); + + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("does not let inherited voicing plans establish the named copy", () => { + const { song, section } = songWithVoicingPlan(); + const inheritedRole = Object.create({ + voicingPlan: "Inherited voicing plan" + }) as (typeof section.roles)[0]; + Object.defineProperties(inheritedRole, { + id: { configurable: true, enumerable: true, value: "keys-right" }, + name: { configurable: true, enumerable: true, value: "Keyboard 1 Right Hand" }, + rehearsalPriority: { configurable: true, enumerable: true, value: "high" } + }); + section.roles = [inheritedRole]; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("does not let inherited role or graph metadata establish the holding part", () => { + const { song, section } = songWithVoicingPlan(); + const node = section.partGraph[0]!; + section.partGraph = [Object.create(node) as typeof node]; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("rejects arrays masquerading as section records", () => { + const { song, section } = songWithVoicingPlan(); + const arraySection = Object.assign([], section) as unknown as typeof section; + song.sections = [arraySection]; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.section-label.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.section-label.test.ts new file mode 100644 index 000000000..d13062783 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.section-label.test.ts @@ -0,0 +1,14 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +describe("resolveFirstVoicingPlan 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(resolveFirstVoicingPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts new file mode 100644 index 000000000..78f3866b1 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts @@ -0,0 +1,281 @@ +import { describe, expect, it } from "vitest"; +import { MAX_SECTION_TIME_SECONDS, createDemoRehearsalSong } from "@bandscope/shared-types"; +import { formatVoicingPlanTime, resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +const DEMO_VOICING_PLAN = + "Keep the verse voicing in first inversion so the top line still sings over the guitars."; + +function withVoicingSection( + overrides: { + id?: string; + start?: number; + end?: number; + voicingPlan?: 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-voicing"; + 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." + }, + voicingPlan: + overrides.voicingPlan ?? + "Keep the verse voicing in first inversion so the top line still sings over the guitars.", + manualOverrides: [] + } + ]; + section.partGraph = [ + { + role_id: roleId, + is_active: overrides.isActive ?? true, + handoff_to: [], + handoff_from: [] + } + ]; + song.sections = [section]; + return song; +} + +describe("resolveFirstVoicingPlan", () => { + it("picks the demo song's earliest high-priority voicing plan and the part that owns it", () => { + const resolved = resolveFirstVoicingPlan(createDemoRehearsalSong()); + expect(resolved?.section.id).toBe("verse-1"); + expect(resolved?.holdingRole.id).toBe("keys-right"); + expect(resolved?.voicingPlan).toBe(DEMO_VOICING_PLAN); + expect(resolved?.atSeconds).toBe(10); + expect(formatVoicingPlanTime(resolved?.atSeconds ?? -1)).toBe("0:10"); + expect(formatVoicingPlanTime(Number.NaN)).toBe("0:00"); + expect(formatVoicingPlanTime(-4)).toBe("0:00"); + }); + + it("does not invent a voicing plan from groove, cue, simplification, overlap, range, chords, function labels, setup notes, transposition plans, tuning plans, dynamics plans, articulation plans, confirmed overrides, harmonic explanations, or confidence notes", () => { + const song = withVoicingSection(); + delete song.sections[0]!.roles[0]!.voicingPlan; + 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 = + "Keep the verse voicing in first inversion so the top line still sings over the guitars."; + 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 { 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 = + "Keep the verse attack short so the chorus still has a longer sustain to land on."; + 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: "Keep the verse voicing in first inversion so the top line still sings over the guitars." + }; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("skips a blank voicing plan", () => { + expect(resolveFirstVoicingPlan(withVoicingSection({ voicingPlan: " " }))).toBeNull(); + }); + + it("skips a multi-line voicing plan", () => { + expect( + resolveFirstVoicingPlan(withVoicingSection({ voicingPlan: "Drop under the vocal.\nKeep the pickup." })) + ).toBeNull(); + }); + + it("prefers the earlier of two voicing plans", () => { + const song = withVoicingSection({ + id: "verse-late", + start: 40, + end: 56, + roleId: "keys-right", + voicingPlan: "Late voicing." + }); + const earlier = structuredClone(song.sections[0]!); + earlier.id = "verse-early"; + earlier.roles = [ + { + ...earlier.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "low", + voicingPlan: "Earlier voicing." + } + ]; + 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 = resolveFirstVoicingPlan(song); + expect(resolved?.section.id).toBe("verse-early"); + expect(resolved?.holdingRole.id).toBe("lead-vocal"); + expect(resolved?.voicingPlan).toBe("Earlier voicing."); + expect(resolved?.atSeconds).toBe(8); + }); + + it("breaks same-time voicing-plan ties with locale-independent id ordering", () => { + const song = withVoicingSection({ id: "ä-voicing", start: 10, end: 26 }); + const ascii = structuredClone(song.sections[0]!); + ascii.id = "z-voicing"; + song.sections = [song.sections[0]!, ascii]; + + expect(resolveFirstVoicingPlan(song)?.section.id).toBe("z-voicing"); + }); + + it("prefers a high-priority voicing part over a low-priority part in the same section", () => { + const song = withVoicingSection({ + roleId: "keys-right", + roleName: "Keys", + priority: "low", + voicingPlan: "Low-priority voicing." + }); + const section = song.sections[0]!; + const highRole = { + ...section.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "high" as const, + voicingPlan: "High-priority voicing." + }; + 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(resolveFirstVoicingPlan(song)?.holdingRole.id).toBe("lead-vocal"); + expect(resolveFirstVoicingPlan(song)?.voicingPlan).toBe("High-priority voicing."); + }); + + it("breaks equal-priority role ties with locale-independent id ordering", () => { + const song = withVoicingSection({ roleId: "ä-role", roleName: "Umlaut role", priority: "high" }); + const section = song.sections[0]!; + const asciiRole = { + ...section.roles[0]!, + id: "z-role", + name: "ASCII role", + voicingPlan: "ASCII voicing." + }; + 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(resolveFirstVoicingPlan(song)?.holdingRole.id).toBe("z-role"); + expect(resolveFirstVoicingPlan(song)?.voicingPlan).toBe("ASCII voicing."); + }); + + it("skips a voicing plan whose graph node is inactive", () => { + expect(resolveFirstVoicingPlan(withVoicingSection({ isActive: false }))).toBeNull(); + }); + + it("skips a voicing plan whose rehearsal window is unbounded", () => { + expect(resolveFirstVoicingPlan(withVoicingSection({ start: Number.NaN, end: 30 }))).toBeNull(); + }); + + it("skips a voicing plan whose end precedes its start", () => { + expect(resolveFirstVoicingPlan(withVoicingSection({ start: 30, end: 10 }))).toBeNull(); + }); + + it("skips a zero-length voicing-plan window", () => { + expect(resolveFirstVoicingPlan(withVoicingSection({ start: 10, end: 10 }))).toBeNull(); + }); + + it("skips a voicing plan whose endpoint overflows the shared timing bound", () => { + expect( + resolveFirstVoicingPlan( + withVoicingSection({ + start: MAX_SECTION_TIME_SECONDS, + end: MAX_SECTION_TIME_SECONDS + 1 + }) + ) + ).toBeNull(); + }); + + it("returns null for a non-object song root", () => { + expect(resolveFirstVoicingPlan(null as never)).toBeNull(); + }); + + it("returns null when the runtime section collection is sparse", () => { + const song = withVoicingSection(); + const sparseSections: typeof song.sections = new Array(2); + sparseSections[1] = song.sections[0]!; + song.sections = sparseSections; + expect(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("keeps the voicing plan unnamed when role identities are duplicated", () => { + const song = withVoicingSection(); + 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(resolveFirstVoicingPlan(song)).toBeNull(); + }); + + it("bounds the voicing plan to 180 Unicode code points", () => { + const song = withVoicingSection({ voicingPlan: `${"G".repeat(200)}` }); + const resolved = resolveFirstVoicingPlan(song); + expect(resolved?.voicingPlan.length).toBe(180); + }); + + it("does not split a Unicode surrogate pair at the voicing-plan boundary", () => { + const song = withVoicingSection({ voicingPlan: `${"a".repeat(179)}😀tail` }); + const resolved = resolveFirstVoicingPlan(song); + expect(Array.from(resolved?.voicingPlan ?? "")).toHaveLength(180); + expect(resolved?.voicingPlan.endsWith("😀")).toBe(true); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.ts new file mode 100644 index 000000000..c36bcb074 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.ts @@ -0,0 +1,283 @@ +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_VOICING_PLAN_CHARACTERS = 180; +const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); + +/** Tonight's first voicing plan: the earliest labeled section and the part that owns it. */ +export type FirstVoicingPlan = { + section: RehearsalSection; + holdingRole: RehearsalRole; + voicingPlan: string; + atSeconds: number; +}; + +/** Format a non-negative voicing-plan time as m:ss for rehearsal copy. */ +export function formatVoicingPlanTime(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"); +} + +/** Return whether every numeric index is an own data element in a bounded runtime array. */ +function isDenseRuntimeArray(value: unknown): value is unknown[] { + if (!Array.isArray(value)) { + return false; + } + const length = Number(value.length); + if (!Number.isSafeInteger(length) || length < 0 || length > 0xffffffff) { + return false; + } + for (let index = 0; index < length; index += 1) { + if (!hasOwnData(value, index)) { + return false; + } + } + return true; +} + +/** 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); +} + +/** Return a bounded own voicing plan, or null when it cannot be shown. */ +function ownedVoicingPlan(role: unknown): string | null { + if (!isRuntimeObject(role) || !hasOwnData(role, "voicingPlan")) { + return null; + } + const voicingPlan = (role as { voicingPlan?: unknown }).voicingPlan; + if (typeof voicingPlan !== "string") { + return null; + } + const trimmed = voicingPlan.trim(); + if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + return null; + } + return truncateCodePoints(trimmed, MAX_VOICING_PLAN_CHARACTERS); +} + +/** Return true when the role has safe owned identity/copy and ranked rehearsal priority. */ +function hasRankedPriority(role: RehearsalRole): boolean { + return ( + hasOwnData(role, "id") && + typeof role.id === "string" && + role.id.trim().length > 0 && + hasOwnData(role, "name") && + typeof role.name === "string" && + role.name.trim().length > 0 && + hasOwnData(role, "rehearsalPriority") && + Object.prototype.hasOwnProperty.call(PRIORITY_RANK, role.rehearsalPriority) + ); +} + +/** Return whether a section owns a canonical form label from the shared contract. */ +function hasSupportedSectionLabel(section: RehearsalSection): boolean { + return ( + hasOwnData(section, "label") && + typeof section.label === "string" && + SECTION_FORM_LABEL_SET.has(section.label) + ); +} + +/** Return whether a section owns a bounded, positive-length integer rehearsal window. */ +function hasBoundedTimeRange(section: RehearsalSection): boolean { + if (!hasOwnData(section, "timeRange")) { + return false; + } + const timeRange = section.timeRange as Partial | null; + if (!isRuntimeObject(timeRange) || !hasOwnData(timeRange, "start") || !hasOwnData(timeRange, "end")) { + return false; + } + + const start = timeRange.start ?? -1; + const end = timeRange.end ?? -1; + return ( + Number.isInteger(start) && + start >= 0 && + start <= MAX_SECTION_TIME_SECONDS && + Number.isInteger(end) && + end > start && + end <= MAX_SECTION_TIME_SECONDS + ); +} + +/** 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 pickHoldingRole(roles: RehearsalRole[]): RehearsalRole | 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): RehearsalRole[] { + if ( + !hasOwnData(section, "roles") || + !hasOwnData(section, "partGraph") || + !isDenseRuntimeArray(section.roles) || + !isDenseRuntimeArray(section.partGraph) + ) { + return []; + } + + const safeRoleIds = section.roles + .filter( + (role) => + isRuntimeObject(role) && + hasOwnData(role, "id") && + typeof role.id === "string" && + role.id.trim().length > 0 + ) + .map((role) => role.id); + const safeGraphRoleIds = section.partGraph + .filter( + (node) => + isRuntimeObject(node) && + hasOwnData(node, "role_id") && + typeof node.role_id === "string" && + node.role_id.trim().length > 0 + ) + .map((node) => node.role_id); + const repeatedRoleIds = repeatedIds(safeRoleIds); + const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); + const activeIds = new Set( + section.partGraph + .filter( + (node) => + isRuntimeObject(node) && + hasOwnData(node, "is_active") && + node.is_active === true && + hasOwnData(node, "role_id") && + typeof node.role_id === "string" && + node.role_id.trim().length > 0 && + !repeatedGraphRoleIds.has(node.role_id) + ) + .map((node) => node.role_id) + ); + + return section.roles.filter( + (role) => + isRuntimeObject(role) && + hasRankedPriority(role) && + !repeatedRoleIds.has(role.id) && + activeIds.has(role.id) + ); +} + +/** Resolve a voicing plan after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstVoicingPlan(song: RehearsalSong): FirstVoicingPlan | null { + if (!isRuntimeObject(song) || !hasOwnData(song, "sections") || !isDenseRuntimeArray(song.sections)) { + return null; + } + + const candidates = song.sections + .filter( + (section) => + isRuntimeObject(section) && + hasSupportedSectionLabel(section) && + hasOwnData(section, "id") && + typeof section.id === "string" && + section.id.trim().length > 0 && + hasBoundedTimeRange(section) + ) + .flatMap((section) => { + const holdingRole = pickHoldingRole( + rankedActiveRoles(section).filter((role) => ownedVoicingPlan(role) !== null) + ); + if (!holdingRole) { + return []; + } + const voicingPlan = ownedVoicingPlan(holdingRole); + if (!voicingPlan) { + return []; + } + return [ + { + section, + holdingRole, + voicingPlan, + atSeconds: section.timeRange.start + } + ]; + }) + .sort((left, right) => { + if (left.atSeconds !== right.atSeconds) { + return left.atSeconds - right.atSeconds; + } + return compareStableId(left.section.id, right.section.id); + }); + + return candidates[0] ?? null; +} + +/** Return the first named voicing plan, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstVoicingPlan(song: RehearsalSong): FirstVoicingPlan | null { + try { + return resolveSafeFirstVoicingPlan(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..3e35726cf 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,51 @@ 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", "outro")).toBe("outro"); + }); + + it("does not treat inherited object keys as localized section labels", () => { + const inheritedKey = "toString" as never; + expect(translateSectionFormLabel("ko", inheritedKey)).toBe("toString"); + }); + + it("keeps Korean first-voicing-plan next-action copy particle-safe", () => { + const t = createTranslator("ko"); + expect(t("firstVoicingPlanOpenAction")).toBe("{at} {role} 보이싱 열기"); + expect(t("firstVoicingPlanBody")).toBe("{at} {section}에서 {role} 파트의 보이싱 계획이 있습니다."); + expect(t("firstVoicingPlanArmed")).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..8dc7cc683 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -148,5 +148,10 @@ "practiceProgressRegionLabel": "Practice Progress", "practiceProgressLabel": "Practice Progress", "decreasePracticeProgressLabel": "Decrease progress", - "increasePracticeProgressLabel": "Increase progress" + "increasePracticeProgressLabel": "Increase progress", + "firstVoicingPlanLabel": "Tonight's first voicing plan", + "firstVoicingPlanOpenAction": "Open {role} voicing at {at}", + "firstVoicingPlanBody": "{role} still has a voicing plan in the {section} at {at}.", + "firstVoicingPlanArmed": "Lock that voicing on {role} at {at} before the room starts.", + "firstVoicingPlanUnavailable": "No voicing 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..ecfe86fa9 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -148,5 +148,10 @@ "practiceProgressRegionLabel": "연습 진척도", "practiceProgressLabel": "연습 진척도", "decreasePracticeProgressLabel": "진척도 감소", - "increasePracticeProgressLabel": "진척도 증가" + "increasePracticeProgressLabel": "진척도 증가", + "firstVoicingPlanLabel": "오늘 첫 보이싱 계획", + "firstVoicingPlanOpenAction": "{at} {role} 보이싱 열기", + "firstVoicingPlanBody": "{at} {section}에서 {role} 파트의 보이싱 계획이 있습니다.", + "firstVoicingPlanArmed": "{at}에서 {role} 파트의 보이싱을 맞춘 다음 합주를 시작하세요.", + "firstVoicingPlanUnavailable": "사용 가능한 보이싱 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요." } diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 22602c313..bda60db6f 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 Voicing Plan Callout | workspace next-action pattern | `apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx` | Name the owning part when an active graph node corroborates it, the owned `voicingPlan` 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`, `tuningPlan`, `dynamicsPlan`, `articulationPlan`, confirmed overrides, `harmonicExplanation`, or confidence notes. Open scrolls the renderer-owned song-structure section. Keep the unavailable state guidance-only. Distinct from 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-voicing-plan-navigation.md b/docs/doctoring/reduced-motion-first-voicing-plan-navigation.md new file mode 100644 index 000000000..5f7afa371 --- /dev/null +++ b/docs/doctoring/reduced-motion-first-voicing-plan-navigation.md @@ -0,0 +1,3 @@ +# Reduced-motion first voicing-plan navigation + +Open tonight's first voicing 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..543ded6cd 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -139,6 +139,7 @@ export type RehearsalRole = { simplification: string; setupNote: string; transpositionPlan?: string; + voicingPlan?: string; manualOverrides: ManualOverride[]; overlapWarnings: string[]; transcription?: TranscriptionNote[]; @@ -506,6 +507,7 @@ const demoRehearsalSongSeed: RehearsalSong = { simplification: "Drop the top extension if the chorus turnaround still feels busy.", setupNote: "Keep the patch bright enough to stay over the guitars.", transpositionPlan: "If the band rehearses in D, keep the voicing in first inversion so the top line still sings.", + voicingPlan: "Keep the verse voicing in first inversion so the top line still sings over the guitars.", manualOverrides: [], overlapWarnings: [ "Melodic overlap: top notes conflict with Lead Vocal range." @@ -1497,6 +1499,7 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "simplification", "setupNote", "transpositionPlan", + "voicingPlan", "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.voicingPlan !== undefined && typeof value.voicingPlan !== "string") { + return invalidField(`${path}.voicingPlan`); + } 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..a4d84347c 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[1]?.voicingPlan).toContain("first inversion"); 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].voicingPlan", + payload: createInvalidSong((song) => { + song.sections[0]!.roles[0]!.voicingPlan = 2 as never; + }) + }, { message: "sections[0].roles[0].practiceProgress", payload: createInvalidSong((song) => { From 7f79b6d7d445da5e10bad5f5e8ec30d5873613d1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:24:59 -0700 Subject: [PATCH 02/14] test(workspace): lock voicing-plan own-data authority --- .../firstVoicingPlan.proxy-authority.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.proxy-authority.test.ts diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.proxy-authority.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.proxy-authority.test.ts new file mode 100644 index 000000000..cc980269e --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.proxy-authority.test.ts @@ -0,0 +1,31 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +const DEMO_VOICING_PLAN = + "Keep the verse voicing in first inversion so the top line still sings over the guitars."; + +describe("resolveFirstVoicingPlan own-data authority", () => { + it("uses the snapshotted own-data voicing 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 === "keys-right") ?? -1; + const role = roleIndex >= 0 ? section?.roles[roleIndex] : undefined; + expect(section).toBeDefined(); + expect(role).toBeDefined(); + if (!section || !role || roleIndex < 0) { + throw new Error("Demo voicing-plan fixture is missing the expected Keyboard 1 Right Hand role."); + } + + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "voicingPlan") { + return "Injected proxy voicing."; + } + return Reflect.get(target, property, receiver); + } + }); + + expect(resolveFirstVoicingPlan(song)?.voicingPlan).toBe(DEMO_VOICING_PLAN); + }); +}); From b3e8922bc9507593698d86a6eb57fd73259391e1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:25:35 -0700 Subject: [PATCH 03/14] fix(workspace): snapshot owned voicing-plan data --- .../desktop/src/features/workspace/firstVoicingPlan.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.ts index c36bcb074..9d36dfc52 100644 --- a/apps/desktop/src/features/workspace/firstVoicingPlan.ts +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.ts @@ -81,12 +81,16 @@ function truncateCodePoints(value: string, maximum: number): string { return endIndex === value.length ? value : value.slice(0, endIndex); } -/** Return a bounded own voicing plan, or null when it cannot be shown. */ +/** Return a bounded snapshotted own voicing plan, or null when it cannot be shown. */ function ownedVoicingPlan(role: unknown): string | null { - if (!isRuntimeObject(role) || !hasOwnData(role, "voicingPlan")) { + if (!isRuntimeObject(role)) { return null; } - const voicingPlan = (role as { voicingPlan?: unknown }).voicingPlan; + const descriptor = Object.getOwnPropertyDescriptor(role, "voicingPlan"); + if (descriptor === undefined || !Object.prototype.hasOwnProperty.call(descriptor, "value")) { + return null; + } + const voicingPlan = descriptor.value; if (typeof voicingPlan !== "string") { return null; } From 13e0c43c06ff1384cff8c550cfa48ec7f20b75b8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:48:38 -0700 Subject: [PATCH 04/14] test(workspace): require voicing plan coverage ownership --- .../workspace/voicingCoverageContract.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 apps/desktop/src/features/workspace/voicingCoverageContract.test.ts diff --git a/apps/desktop/src/features/workspace/voicingCoverageContract.test.ts b/apps/desktop/src/features/workspace/voicingCoverageContract.test.ts new file mode 100644 index 000000000..46dd6d279 --- /dev/null +++ b/apps/desktop/src/features/workspace/voicingCoverageContract.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 voicing-plan resolver and callout inside the coverage gate", () => { + expect(DESKTOP_OWNED_PRODUCTION_COVERAGE).toEqual( + expect.arrayContaining([ + "src/features/workspace/firstVoicingPlan.ts", + "src/features/workspace/FirstVoicingPlanCallout.tsx" + ]) + ); + }); +}); From a0285ef59d84f27cf109b1f4b26cd92d45f13c70 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:48:57 -0700 Subject: [PATCH 05/14] fix(coverage): include voicing production paths --- apps/desktop/vite.config.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/apps/desktop/vite.config.ts b/apps/desktop/vite.config.ts index f1db6f2b8..51457feba 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/firstVoicingPlan.ts", + "src/features/workspace/FirstVoicingPlanCallout.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, From a6b5f828af1611cb86f825112553e8156d28c415 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 04:20:26 -0700 Subject: [PATCH 06/14] perf(workspace): memoize voicing-plan resolution --- .../src/features/workspace/FirstVoicingPlanCallout.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx index 3899dd7c7..1c20b4ab3 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId, useState } from "react"; +import { useEffect, useId, useMemo, useState } from "react"; import type { RehearsalSong } from "@bandscope/shared-types"; import { Button } from "@/components/ui/button"; import { @@ -78,11 +78,11 @@ function resolveVoicingPlanRenderer(origin: HTMLElement): HTMLElement | null { /** Name tonight's first voicing plan and open the matching rendered map section. */ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) { const calloutId = `workspace-surface-voicing-plan-${useId()}`; - const locale = detectPreferredLocale(); - const t = createTranslator(locale); + const locale = useMemo(() => detectPreferredLocale(), []); + const t = useMemo(() => createTranslator(locale), [locale]); const songIdentity = stableVoicingPlanSongIdentity(song); const runtimeSong = song as unknown as Partial | null; - const named = resolveFirstVoicingPlan(song); + const named = useMemo(() => resolveFirstVoicingPlan(song), [song]); const namedSectionIndex = named && Array.isArray(runtimeSong?.sections) ? runtimeSong.sections.indexOf(named.section) From 5bedd5fd1825a1426b5bade6d02b3cbd5613f895 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:26:53 -0700 Subject: [PATCH 07/14] test(workspace): lock voicing role authority --- .../firstVoicingPlan.role-authority.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.role-authority.test.ts diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.role-authority.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.role-authority.test.ts new file mode 100644 index 000000000..d82abd09b --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.role-authority.test.ts @@ -0,0 +1,44 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +describe("resolveFirstVoicingPlan role authority", () => { + it("uses snapshotted own role identity and priority instead of Proxy get traps", () => { + const song = createDemoRehearsalSong(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + const roleIndex = section?.roles.findIndex((role) => role.id === "keys-right") ?? -1; + const role = roleIndex >= 0 ? section?.roles[roleIndex] : undefined; + expect(section).toBeDefined(); + expect(role).toBeDefined(); + if (!section || !role || roleIndex < 0) { + throw new Error("Demo voicing-plan fixture is missing the expected Keyboard 1 Right Hand role."); + } + + const expectedId = Object.getOwnPropertyDescriptor(role, "id")?.value; + const expectedName = Object.getOwnPropertyDescriptor(role, "name")?.value; + const expectedPriority = Object.getOwnPropertyDescriptor(role, "rehearsalPriority")?.value; + expect(expectedId).toBe("keys-right"); + expect(expectedName).toBeTypeOf("string"); + expect(expectedPriority).toBe("high"); + + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "id") { + return "bass"; + } + if (property === "name") { + return "Injected proxy role"; + } + if (property === "rehearsalPriority") { + return "low"; + } + return Reflect.get(target, property, receiver); + } + }); + + const resolved = resolveFirstVoicingPlan(song); + expect(resolved?.holdingRoleId).toBe(expectedId); + expect(resolved?.holdingRoleName).toBe(expectedName); + expect(resolved?.voicingPlan).toContain("first inversion"); + }); +}); From ea95d6c4eb4b0c5d541bffd3f2c1c211f921240c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:28:56 -0700 Subject: [PATCH 08/14] fix(workspace): snapshot voicing role authority --- .../features/workspace/firstVoicingPlan.ts | 180 +++++++++++------- 1 file changed, 108 insertions(+), 72 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.ts index 9d36dfc52..cf74e782d 100644 --- a/apps/desktop/src/features/workspace/firstVoicingPlan.ts +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.ts @@ -10,10 +10,26 @@ const PRIORITY_RANK = { high: 0, medium: 1, low: 2 } as const; const MAX_VOICING_PLAN_CHARACTERS = 180; const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); +type RehearsalPriority = keyof typeof PRIORITY_RANK; + +type RankedRoleSnapshot = Readonly<{ + role: RehearsalRole; + id: string; + name: string; + rehearsalPriority: RehearsalPriority; +}>; + +type VoicingRoleSnapshot = RankedRoleSnapshot & + Readonly<{ + voicingPlan: string; + }>; + /** Tonight's first voicing plan: the earliest labeled section and the part that owns it. */ export type FirstVoicingPlan = { section: RehearsalSection; holdingRole: RehearsalRole; + holdingRoleId: string; + holdingRoleName: string; voicingPlan: string; atSeconds: number; }; @@ -44,10 +60,29 @@ function isRuntimeObject(value: unknown): value is object { return value !== null && typeof value === "object" && !Array.isArray(value); } +/** Return an own data-property descriptor, rejecting inherited or accessor state. */ +function ownDataDescriptor(value: object, key: PropertyKey): PropertyDescriptor | null { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor !== undefined && Object.prototype.hasOwnProperty.call(descriptor, "value") + ? descriptor + : null; +} + /** 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"); + return ownDataDescriptor(value, key) !== null; +} + +/** Snapshot one non-blank owned string without consulting a Proxy get trap. */ +function ownedNonBlankString(value: unknown, key: PropertyKey): string | null { + if (!isRuntimeObject(value)) { + return null; + } + const descriptor = ownDataDescriptor(value, key); + if (descriptor === null || typeof descriptor.value !== "string") { + return null; + } + return descriptor.value.trim().length > 0 ? descriptor.value : null; } /** Return whether every numeric index is an own data element in a bounded runtime array. */ @@ -86,33 +121,46 @@ function ownedVoicingPlan(role: unknown): string | null { if (!isRuntimeObject(role)) { return null; } - const descriptor = Object.getOwnPropertyDescriptor(role, "voicingPlan"); - if (descriptor === undefined || !Object.prototype.hasOwnProperty.call(descriptor, "value")) { + const descriptor = ownDataDescriptor(role, "voicingPlan"); + if (descriptor === null || typeof descriptor.value !== "string") { return null; } - const voicingPlan = descriptor.value; - if (typeof voicingPlan !== "string") { - return null; - } - const trimmed = voicingPlan.trim(); + const trimmed = descriptor.value.trim(); if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { return null; } return truncateCodePoints(trimmed, MAX_VOICING_PLAN_CHARACTERS); } -/** Return true when the role has safe owned identity/copy and ranked rehearsal priority. */ -function hasRankedPriority(role: RehearsalRole): boolean { - return ( - hasOwnData(role, "id") && - typeof role.id === "string" && - role.id.trim().length > 0 && - hasOwnData(role, "name") && - typeof role.name === "string" && - role.name.trim().length > 0 && - hasOwnData(role, "rehearsalPriority") && - Object.prototype.hasOwnProperty.call(PRIORITY_RANK, role.rehearsalPriority) - ); +/** Snapshot owned role identity and priority without later untrusted property reads. */ +function snapshotRankedRole(role: unknown): RankedRoleSnapshot | null { + if (!isRuntimeObject(role)) { + return null; + } + const id = ownedNonBlankString(role, "id"); + const name = ownedNonBlankString(role, "name"); + const priorityDescriptor = ownDataDescriptor(role, "rehearsalPriority"); + const rehearsalPriority = priorityDescriptor?.value; + if ( + id === null || + name === null || + typeof rehearsalPriority !== "string" || + !Object.prototype.hasOwnProperty.call(PRIORITY_RANK, rehearsalPriority) + ) { + return null; + } + return { + role: role as RehearsalRole, + id, + name, + rehearsalPriority: rehearsalPriority as RehearsalPriority + }; +} + +/** Attach one snapshotted owned voicing plan to an already validated role snapshot. */ +function snapshotVoicingRole(role: RankedRoleSnapshot): VoicingRoleSnapshot | null { + const voicingPlan = ownedVoicingPlan(role.role); + return voicingPlan === null ? null : { ...role, voicingPlan }; } /** Return whether a section owns a canonical form label from the shared contract. */ @@ -160,14 +208,15 @@ function repeatedIds(ids: string[]): Set { return repeated; } -/** Prefer the earlier ranked role, then rehearsal priority, then a locale-independent id. */ -function pickHoldingRole(roles: RehearsalRole[]): RehearsalRole | null { +/** Prefer rehearsal priority, then a locale-independent stable role id. */ +function pickHoldingRole(roles: VoicingRoleSnapshot[]): VoicingRoleSnapshot | null { if (roles.length === 0) { return null; } return ( [...roles].sort((left, right) => { - const priorityDelta = PRIORITY_RANK[left.rehearsalPriority] - PRIORITY_RANK[right.rehearsalPriority]; + const priorityDelta = + PRIORITY_RANK[left.rehearsalPriority] - PRIORITY_RANK[right.rehearsalPriority]; if (priorityDelta !== 0) { return priorityDelta; } @@ -176,8 +225,8 @@ function pickHoldingRole(roles: RehearsalRole[]): RehearsalRole | null { ); } -/** Return ranked roles whose unique graph node is explicitly active. */ -function rankedActiveRoles(section: RehearsalSection): RehearsalRole[] { +/** Return snapshotted ranked roles whose unique graph node is explicitly active. */ +function rankedActiveRoles(section: RehearsalSection): RankedRoleSnapshot[] { if ( !hasOwnData(section, "roles") || !hasOwnData(section, "partGraph") || @@ -187,48 +236,35 @@ function rankedActiveRoles(section: RehearsalSection): RehearsalRole[] { return []; } - const safeRoleIds = section.roles - .filter( - (role) => - isRuntimeObject(role) && - hasOwnData(role, "id") && - typeof role.id === "string" && - role.id.trim().length > 0 - ) - .map((role) => role.id); - const safeGraphRoleIds = section.partGraph - .filter( - (node) => - isRuntimeObject(node) && - hasOwnData(node, "role_id") && - typeof node.role_id === "string" && - node.role_id.trim().length > 0 - ) - .map((node) => node.role_id); + const safeRoleIds = section.roles.flatMap((role) => { + const id = ownedNonBlankString(role, "id"); + return id === null ? [] : [id]; + }); + const safeGraphRoleIds = section.partGraph.flatMap((node) => { + const roleId = ownedNonBlankString(node, "role_id"); + return roleId === null ? [] : [roleId]; + }); const repeatedRoleIds = repeatedIds(safeRoleIds); const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); const activeIds = new Set( - section.partGraph - .filter( - (node) => - isRuntimeObject(node) && - hasOwnData(node, "is_active") && - node.is_active === true && - hasOwnData(node, "role_id") && - typeof node.role_id === "string" && - node.role_id.trim().length > 0 && - !repeatedGraphRoleIds.has(node.role_id) - ) - .map((node) => node.role_id) + section.partGraph.flatMap((node) => { + if (!isRuntimeObject(node)) { + return []; + } + const roleId = ownedNonBlankString(node, "role_id"); + const isActive = ownDataDescriptor(node, "is_active")?.value; + return roleId !== null && isActive === true && !repeatedGraphRoleIds.has(roleId) + ? [roleId] + : []; + }) ); - return section.roles.filter( - (role) => - isRuntimeObject(role) && - hasRankedPriority(role) && - !repeatedRoleIds.has(role.id) && - activeIds.has(role.id) - ); + return section.roles.flatMap((role) => { + const snapshot = snapshotRankedRole(role); + return snapshot !== null && !repeatedRoleIds.has(snapshot.id) && activeIds.has(snapshot.id) + ? [snapshot] + : []; + }); } /** Resolve a voicing plan after the runtime root has passed its structural boundary checks. */ @@ -248,21 +284,21 @@ function resolveSafeFirstVoicingPlan(song: RehearsalSong): FirstVoicingPlan | nu hasBoundedTimeRange(section) ) .flatMap((section) => { - const holdingRole = pickHoldingRole( - rankedActiveRoles(section).filter((role) => ownedVoicingPlan(role) !== null) - ); + const voicingRoles = rankedActiveRoles(section).flatMap((role) => { + const candidate = snapshotVoicingRole(role); + return candidate === null ? [] : [candidate]; + }); + const holdingRole = pickHoldingRole(voicingRoles); if (!holdingRole) { return []; } - const voicingPlan = ownedVoicingPlan(holdingRole); - if (!voicingPlan) { - return []; - } return [ { section, - holdingRole, - voicingPlan, + holdingRole: holdingRole.role, + holdingRoleId: holdingRole.id, + holdingRoleName: holdingRole.name, + voicingPlan: holdingRole.voicingPlan, atSeconds: section.timeRange.start } ]; From 392b27e97dcd68d21bd574ce1d3c85d199378bc9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:29:26 -0700 Subject: [PATCH 09/14] fix(workspace): use snapshotted voicing role copy --- .../src/features/workspace/FirstVoicingPlanCallout.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx index 1c20b4ab3..8d1a0229f 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx @@ -95,7 +95,7 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) songIdentity, namedSectionIndex, named?.section.id, - named?.holdingRole.id, + named?.holdingRoleId, named?.voicingPlan, named?.atSeconds ]); @@ -120,12 +120,12 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) openedVoicingPlan.songIdentity === songIdentity && openedVoicingPlan.sectionId === named.section.id && openedVoicingPlan.sectionIndex === namedSectionIndex && - openedVoicingPlan.holdingRoleId === named.holdingRole.id && + openedVoicingPlan.holdingRoleId === named.holdingRoleId && openedVoicingPlan.voicingPlan === named.voicingPlan && openedVoicingPlan.atSeconds === named.atSeconds; const at = formatVoicingPlanTime(named.atSeconds); const copyValues: VoicingPlanCopyValues = { - role: named.holdingRole.name, + role: named.holdingRoleName, section: translateSectionFormLabel(locale, named.section.label), at }; @@ -166,7 +166,7 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) songIdentity, sectionId: named.section.id, sectionIndex: namedSectionIndex, - holdingRoleId: named.holdingRole.id, + holdingRoleId: named.holdingRoleId, voicingPlan: named.voicingPlan, atSeconds: named.atSeconds }); From 37d8751d19b1685c502411e512654dead252a51e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:42:18 -0700 Subject: [PATCH 10/14] test(workspace): lock voicing section authority --- ...firstVoicingPlan.section-authority.test.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 apps/desktop/src/features/workspace/firstVoicingPlan.section-authority.test.ts diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.section-authority.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.section-authority.test.ts new file mode 100644 index 000000000..5f09eb917 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.section-authority.test.ts @@ -0,0 +1,43 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstVoicingPlan } from "./firstVoicingPlan"; + +describe("resolveFirstVoicingPlan section authority", () => { + it("uses snapshotted own section identity, label, and time instead of Proxy get traps", () => { + const song = createDemoRehearsalSong(); + const sectionIndex = song.sections.findIndex((section) => section.id === "verse-1"); + const section = sectionIndex >= 0 ? song.sections[sectionIndex] : undefined; + expect(section).toBeDefined(); + if (!section || sectionIndex < 0) { + throw new Error("Demo voicing-plan fixture is missing the expected verse section."); + } + + const expectedId = Object.getOwnPropertyDescriptor(section, "id")?.value; + const expectedLabel = Object.getOwnPropertyDescriptor(section, "label")?.value; + const expectedRange = Object.getOwnPropertyDescriptor(section, "timeRange")?.value; + expect(expectedId).toBe("verse-1"); + expect(expectedLabel).toBe("verse"); + expect(expectedRange?.start).toBe(10); + + song.sections[sectionIndex] = new Proxy(section, { + get(target, property, receiver) { + if (property === "id") { + return "chorus-injected"; + } + if (property === "label") { + return "chorus"; + } + if (property === "timeRange") { + return { start: 25, end: 40 }; + } + return Reflect.get(target, property, receiver); + } + }); + + const resolved = resolveFirstVoicingPlan(song); + expect(resolved).not.toBeNull(); + expect((resolved as unknown as { sectionId?: string })?.sectionId).toBe(expectedId); + expect((resolved as unknown as { sectionLabel?: string })?.sectionLabel).toBe(expectedLabel); + expect(resolved?.atSeconds).toBe(expectedRange?.start); + }); +}); From 5bf7f1b2042a0232f062c540157f9ac30739b0e0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:43:08 -0700 Subject: [PATCH 11/14] fix(workspace): snapshot voicing section authority --- .../features/workspace/firstVoicingPlan.ts | 200 ++++++++++-------- 1 file changed, 112 insertions(+), 88 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.ts index cf74e782d..16ffee46c 100644 --- a/apps/desktop/src/features/workspace/firstVoicingPlan.ts +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.ts @@ -24,9 +24,28 @@ type VoicingRoleSnapshot = RankedRoleSnapshot & voicingPlan: string; }>; +type GraphNodeSnapshot = Readonly<{ + roleId: string; + isActive: boolean; +}>; + +type SectionSnapshot = Readonly<{ + section: RehearsalSection; + sectionIndex: number; + id: string; + label: RehearsalSection["label"]; + start: number; + end: number; + roles: unknown[]; + partGraph: unknown[]; +}>; + /** Tonight's first voicing plan: the earliest labeled section and the part that owns it. */ export type FirstVoicingPlan = { section: RehearsalSection; + sectionIndex: number; + sectionId: string; + sectionLabel: RehearsalSection["label"]; holdingRole: RehearsalRole; holdingRoleId: string; holdingRoleName: string; @@ -68,11 +87,6 @@ function ownDataDescriptor(value: object, key: PropertyKey): PropertyDescriptor : null; } -/** Return whether a runtime record owns a stable data property rather than inherited/accessor state. */ -function hasOwnData(value: object, key: PropertyKey): boolean { - return ownDataDescriptor(value, key) !== null; -} - /** Snapshot one non-blank owned string without consulting a Proxy get trap. */ function ownedNonBlankString(value: unknown, key: PropertyKey): string | null { if (!isRuntimeObject(value)) { @@ -85,21 +99,25 @@ function ownedNonBlankString(value: unknown, key: PropertyKey): string | null { return descriptor.value.trim().length > 0 ? descriptor.value : null; } -/** Return whether every numeric index is an own data element in a bounded runtime array. */ -function isDenseRuntimeArray(value: unknown): value is unknown[] { +/** Snapshot a bounded dense array through own descriptors without later index reads. */ +function snapshotDenseRuntimeArray(value: unknown): unknown[] | null { if (!Array.isArray(value)) { - return false; + return null; } - const length = Number(value.length); + const lengthDescriptor = ownDataDescriptor(value, "length"); + const length = lengthDescriptor?.value; if (!Number.isSafeInteger(length) || length < 0 || length > 0xffffffff) { - return false; + return null; } + const items: unknown[] = []; for (let index = 0; index < length; index += 1) { - if (!hasOwnData(value, index)) { - return false; + const descriptor = ownDataDescriptor(value, index); + if (descriptor === null) { + return null; } + items.push(descriptor.value); } - return true; + return items; } /** Bound buyer-visible text by Unicode code points without splitting a surrogate pair. */ @@ -163,35 +181,58 @@ function snapshotVoicingRole(role: RankedRoleSnapshot): VoicingRoleSnapshot | nu return voicingPlan === null ? null : { ...role, voicingPlan }; } -/** Return whether a section owns a canonical form label from the shared contract. */ -function hasSupportedSectionLabel(section: RehearsalSection): boolean { - return ( - hasOwnData(section, "label") && - typeof section.label === "string" && - SECTION_FORM_LABEL_SET.has(section.label) - ); +/** Snapshot one graph node's role identity and activity flag through own descriptors. */ +function snapshotGraphNode(node: unknown): GraphNodeSnapshot | null { + if (!isRuntimeObject(node)) { + return null; + } + const roleId = ownedNonBlankString(node, "role_id"); + const isActive = ownDataDescriptor(node, "is_active")?.value; + return roleId !== null && typeof isActive === "boolean" ? { roleId, isActive } : null; } -/** Return whether a section owns a bounded, positive-length integer rehearsal window. */ -function hasBoundedTimeRange(section: RehearsalSection): boolean { - if (!hasOwnData(section, "timeRange")) { - return false; +/** Snapshot one section's buyer/navigation authority without later untrusted property reads. */ +function snapshotSection(section: unknown, sectionIndex: number): SectionSnapshot | null { + if (!isRuntimeObject(section)) { + return null; } - const timeRange = section.timeRange as Partial | null; - if (!isRuntimeObject(timeRange) || !hasOwnData(timeRange, "start") || !hasOwnData(timeRange, "end")) { - return false; + const id = ownedNonBlankString(section, "id"); + const label = ownDataDescriptor(section, "label")?.value; + const timeRange = ownDataDescriptor(section, "timeRange")?.value; + const roles = snapshotDenseRuntimeArray(ownDataDescriptor(section, "roles")?.value); + const partGraph = snapshotDenseRuntimeArray(ownDataDescriptor(section, "partGraph")?.value); + if ( + id === null || + typeof label !== "string" || + !SECTION_FORM_LABEL_SET.has(label) || + !isRuntimeObject(timeRange) || + roles === null || + partGraph === null + ) { + return null; } - - const start = timeRange.start ?? -1; - const end = timeRange.end ?? -1; - return ( - Number.isInteger(start) && - start >= 0 && - start <= MAX_SECTION_TIME_SECONDS && - Number.isInteger(end) && - end > start && - end <= MAX_SECTION_TIME_SECONDS - ); + const start = ownDataDescriptor(timeRange, "start")?.value; + const end = ownDataDescriptor(timeRange, "end")?.value; + if ( + !Number.isInteger(start) || + start < 0 || + start > MAX_SECTION_TIME_SECONDS || + !Number.isInteger(end) || + end <= start || + end > MAX_SECTION_TIME_SECONDS + ) { + return null; + } + return { + section: section as RehearsalSection, + sectionIndex, + id, + label: label as RehearsalSection["label"], + start, + end, + roles, + partGraph + }; } /** Return safe identities that appear more than once in one section-local collection. */ @@ -226,65 +267,45 @@ function pickHoldingRole(roles: VoicingRoleSnapshot[]): VoicingRoleSnapshot | nu } /** Return snapshotted ranked roles whose unique graph node is explicitly active. */ -function rankedActiveRoles(section: RehearsalSection): RankedRoleSnapshot[] { - if ( - !hasOwnData(section, "roles") || - !hasOwnData(section, "partGraph") || - !isDenseRuntimeArray(section.roles) || - !isDenseRuntimeArray(section.partGraph) - ) { - return []; - } - - const safeRoleIds = section.roles.flatMap((role) => { - const id = ownedNonBlankString(role, "id"); - return id === null ? [] : [id]; +function rankedActiveRoles(section: SectionSnapshot): RankedRoleSnapshot[] { + const roleSnapshots = section.roles.flatMap((role) => { + const snapshot = snapshotRankedRole(role); + return snapshot === null ? [] : [snapshot]; }); - const safeGraphRoleIds = section.partGraph.flatMap((node) => { - const roleId = ownedNonBlankString(node, "role_id"); - return roleId === null ? [] : [roleId]; + const graphSnapshots = section.partGraph.flatMap((node) => { + const snapshot = snapshotGraphNode(node); + return snapshot === null ? [] : [snapshot]; }); - const repeatedRoleIds = repeatedIds(safeRoleIds); - const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); + const repeatedRoleIds = repeatedIds(roleSnapshots.map((role) => role.id)); + const repeatedGraphRoleIds = repeatedIds(graphSnapshots.map((node) => node.roleId)); const activeIds = new Set( - section.partGraph.flatMap((node) => { - if (!isRuntimeObject(node)) { - return []; - } - const roleId = ownedNonBlankString(node, "role_id"); - const isActive = ownDataDescriptor(node, "is_active")?.value; - return roleId !== null && isActive === true && !repeatedGraphRoleIds.has(roleId) - ? [roleId] - : []; - }) + graphSnapshots.flatMap((node) => + node.isActive && !repeatedGraphRoleIds.has(node.roleId) ? [node.roleId] : [] + ) ); - return section.roles.flatMap((role) => { - const snapshot = snapshotRankedRole(role); - return snapshot !== null && !repeatedRoleIds.has(snapshot.id) && activeIds.has(snapshot.id) - ? [snapshot] - : []; - }); + return roleSnapshots.filter( + (role) => !repeatedRoleIds.has(role.id) && activeIds.has(role.id) + ); } /** Resolve a voicing plan after the runtime root has passed its structural boundary checks. */ function resolveSafeFirstVoicingPlan(song: RehearsalSong): FirstVoicingPlan | null { - if (!isRuntimeObject(song) || !hasOwnData(song, "sections") || !isDenseRuntimeArray(song.sections)) { + if (!isRuntimeObject(song)) { + return null; + } + const sections = snapshotDenseRuntimeArray(ownDataDescriptor(song, "sections")?.value); + if (sections === null) { return null; } - const candidates = song.sections - .filter( - (section) => - isRuntimeObject(section) && - hasSupportedSectionLabel(section) && - hasOwnData(section, "id") && - typeof section.id === "string" && - section.id.trim().length > 0 && - hasBoundedTimeRange(section) - ) - .flatMap((section) => { - const voicingRoles = rankedActiveRoles(section).flatMap((role) => { + const candidates = sections + .flatMap((section, sectionIndex) => { + const sectionSnapshot = snapshotSection(section, sectionIndex); + if (sectionSnapshot === null) { + return []; + } + const voicingRoles = rankedActiveRoles(sectionSnapshot).flatMap((role) => { const candidate = snapshotVoicingRole(role); return candidate === null ? [] : [candidate]; }); @@ -294,12 +315,15 @@ function resolveSafeFirstVoicingPlan(song: RehearsalSong): FirstVoicingPlan | nu } return [ { - section, + section: sectionSnapshot.section, + sectionIndex: sectionSnapshot.sectionIndex, + sectionId: sectionSnapshot.id, + sectionLabel: sectionSnapshot.label, holdingRole: holdingRole.role, holdingRoleId: holdingRole.id, holdingRoleName: holdingRole.name, voicingPlan: holdingRole.voicingPlan, - atSeconds: section.timeRange.start + atSeconds: sectionSnapshot.start } ]; }) @@ -307,7 +331,7 @@ function resolveSafeFirstVoicingPlan(song: RehearsalSong): FirstVoicingPlan | nu if (left.atSeconds !== right.atSeconds) { return left.atSeconds - right.atSeconds; } - return compareStableId(left.section.id, right.section.id); + return compareStableId(left.sectionId, right.sectionId); }); return candidates[0] ?? null; From 065a239277c78275f6613821e7e0f4cd68c64b3a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 15:43:31 -0700 Subject: [PATCH 12/14] fix(workspace): use snapshotted voicing section data --- .../workspace/FirstVoicingPlanCallout.tsx | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx index 8d1a0229f..ecc326156 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx @@ -81,20 +81,15 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) const locale = useMemo(() => detectPreferredLocale(), []); const t = useMemo(() => createTranslator(locale), [locale]); const songIdentity = stableVoicingPlanSongIdentity(song); - const runtimeSong = song as unknown as Partial | null; const named = useMemo(() => resolveFirstVoicingPlan(song), [song]); - const namedSectionIndex = - named && Array.isArray(runtimeSong?.sections) - ? runtimeSong.sections.indexOf(named.section) - : -1; const [openedVoicingPlan, setOpenedVoicingPlan] = useState(null); useEffect(() => { setOpenedVoicingPlan(null); }, [ songIdentity, - namedSectionIndex, - named?.section.id, + named?.sectionIndex, + named?.sectionId, named?.holdingRoleId, named?.voicingPlan, named?.atSeconds @@ -118,15 +113,15 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) const opened = openedVoicingPlan !== null && openedVoicingPlan.songIdentity === songIdentity && - openedVoicingPlan.sectionId === named.section.id && - openedVoicingPlan.sectionIndex === namedSectionIndex && + openedVoicingPlan.sectionId === named.sectionId && + openedVoicingPlan.sectionIndex === named.sectionIndex && openedVoicingPlan.holdingRoleId === named.holdingRoleId && openedVoicingPlan.voicingPlan === named.voicingPlan && openedVoicingPlan.atSeconds === named.atSeconds; const at = formatVoicingPlanTime(named.atSeconds); const copyValues: VoicingPlanCopyValues = { role: named.holdingRoleName, - section: translateSectionFormLabel(locale, named.section.label), + section: translateSectionFormLabel(locale, named.sectionLabel), at }; const actionLabel = formatVoicingPlanCopy(t("firstVoicingPlanOpenAction"), copyValues); @@ -150,11 +145,9 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) onClick={(event) => { const renderer = resolveVoicingPlanRenderer(event.currentTarget); const target = - namedSectionIndex >= 0 - ? (renderer?.querySelector( - `[data-section-index="${namedSectionIndex}"]` - ) ?? null) - : null; + renderer?.querySelector( + `[data-section-index="${named.sectionIndex}"]` + ) ?? null; if (typeof target?.scrollIntoView !== "function") { return; } @@ -164,8 +157,8 @@ export function FirstVoicingPlanCallout({ song }: FirstVoicingPlanCalloutProps) }); setOpenedVoicingPlan({ songIdentity, - sectionId: named.section.id, - sectionIndex: namedSectionIndex, + sectionId: named.sectionId, + sectionIndex: named.sectionIndex, holdingRoleId: named.holdingRoleId, voicingPlan: named.voicingPlan, atSeconds: named.atSeconds From 7a42053263c6a8a1759dd594b37f108e89e54d40 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 02:44:48 +0900 Subject: [PATCH 13/14] fix(workspace): keep voicing plan contract and navigation stable --- apps/desktop/core/src/lib.rs | 39 ++++++++++++++++++- .../FirstVoicingPlanCallout.particle.test.tsx | 2 +- ...VoicingPlanCallout.reduced-motion.test.tsx | 2 +- .../FirstVoicingPlanCallout.test.tsx | 2 +- .../workspace/FirstVoicingPlanCallout.tsx | 2 +- ...oicingPlanCallout.workspace-scope.test.tsx | 4 +- .../src/features/workspace/Workspace.tsx | 1 + 7 files changed, 44 insertions(+), 8 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index 200726570..2f2bb5ed3 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -189,6 +189,8 @@ pub struct RehearsalRolePayload { rehearsal_priority: String, simplification: String, setup_note: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + voicing_plan: Option, manual_overrides: Vec, overlap_warnings: Vec, } @@ -529,7 +531,7 @@ pub fn is_youtube_video_id(value: &str) -> bool { pub fn project_payload_from_content(content: &str) -> Result { if let Ok(parsed) = serde_json::from_str::(content) { - return Ok(parsed); + return validate_voicing_plan(parsed); } let payload = serde_json::from_str::(content) @@ -547,7 +549,24 @@ pub fn project_payload_from_content(content: &str) -> Result Result { + for section in &payload.sections { + for role in §ion.roles { + if role.voicing_plan.as_ref().is_some_and(|voicing_plan| { + voicing_plan.trim().is_empty() + || voicing_plan.contains('\n') + || voicing_plan.contains('\r') + }) { + return Err("Invalid project file format".to_string()); + } + } + } + Ok(payload) } #[derive(Clone, Debug, Serialize)] @@ -752,6 +771,7 @@ mod tests { "rehearsalPriority": "high", "simplification": "Stay on roots if the chorus entrance gets muddy.", "setupNote": "Keep the attack short so the verse breathes.", + "voicingPlan": "Keep the verse voicing in first inversion so the top line still sings over the guitars.", "manualOverrides": [], "overlapWarnings": [ "Density warning: competing with Keyboard Left Hand in low register." @@ -784,6 +804,10 @@ mod tests { .expect("shared rehearsal song contract should deserialize in Tauri"); assert_eq!(parsed.sections[0].id, "verse-1"); + assert_eq!( + parsed.sections[0].roles[0].voicing_plan.as_deref(), + Some("Keep the verse voicing in first inversion so the top line still sings over the guitars.") + ); } #[test] @@ -894,6 +918,17 @@ mod tests { assert_eq!(error, "Invalid project file format"); } + #[test] + fn project_payload_from_content_rejects_invalid_voicing_plan() { + for voicing_plan in ["", " ", "voice here\nthen move", "voice here\rthen move"] { + let mut payload = shared_contract_payload(json!({ "start": 10, "end": 30 })); + payload["sections"][0]["roles"][0]["voicingPlan"] = json!(voicing_plan); + let content = serde_json::to_string(&payload).expect("payload should serialize"); + + assert!(project_payload_from_content(&content).is_err()); + } + } + #[test] fn youtube_url_validation_requires_exact_video_ids() { assert!(is_supported_youtube_url( diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx index 7213d0f0a..e756f78f6 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx @@ -24,7 +24,7 @@ describe("FirstVoicingPlanCallout Korean role copy", () => { seed.partGraph = [{ role_id: "piano", is_active: true, handoff_to: [], handoff_from: [] }]; const grid = document.createElement("div"); - grid.dataset.testid = "song-structure-grid"; + grid.id = "workspace-song-structure-grid"; grid.setAttribute("role", "region"); grid.setAttribute("aria-label", "Scrollable song structure timeline"); const target = document.createElement("div"); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx index 93d7ac0af..7ab8537dc 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx @@ -21,7 +21,7 @@ describe("FirstVoicingPlanCallout reduced motion", () => { })); const grid = document.createElement("div"); - grid.dataset.testid = "song-structure-grid"; + grid.id = "workspace-song-structure-grid"; grid.setAttribute("role", "region"); grid.setAttribute("aria-label", "Scrollable song structure timeline"); const target = document.createElement("div"); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx index f0ffcc5aa..f2437a284 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx @@ -15,7 +15,7 @@ function appendSongStructureTarget(ariaLabel = "Scrollable song structure timeli timeline.setAttribute("role", "region"); timeline.setAttribute("aria-label", ariaLabel); const grid = document.createElement("div"); - grid.dataset.testid = "song-structure-grid"; + grid.id = "workspace-song-structure-grid"; const target = document.createElement("div"); target.dataset.sectionIndex = "0"; const scrollIntoView = vi.fn(); diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx index ecc326156..bb5438e0f 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx @@ -61,7 +61,7 @@ function preferredVoicingPlanScrollBehavior(): ScrollBehavior { /** Resolve the song-structure renderer owned by this workspace, failing closed on ambiguous mounts. */ function resolveVoicingPlanRenderer(origin: HTMLElement): HTMLElement | null { - const selector = '[data-testid="song-structure-grid"]'; + const selector = "#workspace-song-structure-grid"; const localScope = origin.closest("aside")?.parentElement ?? null; const localRenderers = localScope?.querySelectorAll(selector) ?? []; if (localRenderers.length === 1) { diff --git a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx index a17c2dc6e..5e9d960e0 100644 --- a/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx +++ b/apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx @@ -13,13 +13,13 @@ describe("FirstVoicingPlanCallout workspace scope", () => { <>
-
+
-
+
diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 3fb3f9556..38927577c 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -89,6 +89,7 @@ const SongStructure = memo(function SongStructure({ sections, t }: { sections: R >
From 0cf6e6454c6b4655d4f2df27f33685cf3be66de0 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 11:43:54 +0900 Subject: [PATCH 14/14] fix(contract): align voicing payload boundaries --- apps/desktop/core/src/lib.rs | 134 +++++++++++++++++- .../workspace/firstVoicingPlan.test.ts | 9 ++ .../features/workspace/firstVoicingPlan.ts | 5 +- packages/shared-types/src/index.ts | 44 +++++- packages/shared-types/test/index.test.ts | 22 +++ 5 files changed, 205 insertions(+), 9 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index 2f2bb5ed3..2d35ed754 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -176,6 +176,30 @@ pub struct ManualOverridePayload { source: String, } +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct TranscriptionNotePayload { + pitch: String, + onset: f64, + offset: f64, + velocity: f64, +} + +fn deserialize_practice_progress<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let progress = Option::::deserialize(deserializer)?; + if let Some(value) = progress { + if value > 100 { + return Err(serde::de::Error::custom( + "practiceProgress must be between 0 and 100", + )); + } + } + Ok(progress) +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct RehearsalRolePayload { @@ -190,7 +214,19 @@ pub struct RehearsalRolePayload { simplification: String, setup_note: String, #[serde(default, skip_serializing_if = "Option::is_none")] + harmonic_explanation: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + transposition_plan: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] voicing_plan: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + transcription: Option>, + #[serde( + default, + deserialize_with = "deserialize_practice_progress", + skip_serializing_if = "Option::is_none" + )] + practice_progress: Option, manual_overrides: Vec, overlap_warnings: Vec, } @@ -554,14 +590,50 @@ pub fn project_payload_from_content(content: &str) -> Result bool { + matches!( + value, + '\u{0009}'..='\u{000D}' + | '\u{0020}' + | '\u{0085}' + | '\u{00A0}' + | '\u{1680}' + | '\u{2000}'..='\u{200A}' + | '\u{2028}' + | '\u{2029}' + | '\u{202F}' + | '\u{205F}' + | '\u{3000}' + | '\u{FEFF}' + ) +} + +/// Reject blank or Unicode line-separated voicing guidance without normalizing user text. +fn is_valid_voicing_plan(value: &str) -> bool { + let mut has_non_whitespace = false; + for character in value.chars() { + if matches!( + character, + '\n' | '\r' | '\u{0085}' | '\u{2028}' | '\u{2029}' + ) { + return false; + } + if !is_plan_whitespace(character) { + has_non_whitespace = true; + } + } + has_non_whitespace +} + fn validate_voicing_plan(payload: RehearsalSongPayload) -> Result { for section in &payload.sections { for role in §ion.roles { - if role.voicing_plan.as_ref().is_some_and(|voicing_plan| { - voicing_plan.trim().is_empty() - || voicing_plan.contains('\n') - || voicing_plan.contains('\r') - }) { + if role + .voicing_plan + .as_deref() + .is_some_and(|voicing_plan| !is_valid_voicing_plan(voicing_plan)) + { return Err("Invalid project file format".to_string()); } } @@ -755,6 +827,7 @@ mod tests { "functionLabel": "vi pedal anchor", "source": "model" }, + "harmonicExplanation": "The landing keeps the tonal floor clear.", "cue": { "kind": "transition", "value": "Hold through the pickup before the downbeat." @@ -771,7 +844,15 @@ mod tests { "rehearsalPriority": "high", "simplification": "Stay on roots if the chorus entrance gets muddy.", "setupNote": "Keep the attack short so the verse breathes.", + "transpositionPlan": "Keep the landing shape a whole step lower if needed.", "voicingPlan": "Keep the verse voicing in first inversion so the top line still sings over the guitars.", + "transcription": [{ + "pitch": "C#4", + "onset": 1.0, + "offset": 1.5, + "velocity": 0.8 + }], + "practiceProgress": 50, "manualOverrides": [], "overlapWarnings": [ "Density warning: competing with Keyboard Left Hand in low register." @@ -808,6 +889,22 @@ mod tests { parsed.sections[0].roles[0].voicing_plan.as_deref(), Some("Keep the verse voicing in first inversion so the top line still sings over the guitars.") ); + assert_eq!( + parsed.sections[0].roles[0].harmonic_explanation.as_deref(), + Some("The landing keeps the tonal floor clear.") + ); + assert_eq!( + parsed.sections[0].roles[0].transposition_plan.as_deref(), + Some("Keep the landing shape a whole step lower if needed.") + ); + assert_eq!( + parsed.sections[0].roles[0] + .transcription + .as_ref() + .map(Vec::len), + Some(1) + ); + assert_eq!(parsed.sections[0].roles[0].practice_progress, Some(50)); } #[test] @@ -920,13 +1017,38 @@ mod tests { #[test] fn project_payload_from_content_rejects_invalid_voicing_plan() { - for voicing_plan in ["", " ", "voice here\nthen move", "voice here\rthen move"] { + for voicing_plan in [ + "", + " ", + "\u{FEFF}", + "\u{0085}", + "voice here\nthen move", + "voice here\rthen move", + "voice here\u{0085}then move", + "voice here\u{2028}then move", + "voice here\u{2029}then move", + ] { let mut payload = shared_contract_payload(json!({ "start": 10, "end": 30 })); payload["sections"][0]["roles"][0]["voicingPlan"] = json!(voicing_plan); let content = serde_json::to_string(&payload).expect("payload should serialize"); assert!(project_payload_from_content(&content).is_err()); } + + let mut payload = shared_contract_payload(json!({ "start": 10, "end": 30 })); + payload["sections"][0]["roles"][0]["voicingPlan"] = + json!("\u{FEFF}Voice the string\u{FEFF}"); + let content = serde_json::to_string(&payload).expect("padded plan should serialize"); + assert!(project_payload_from_content(&content).is_ok()); + } + + #[test] + fn project_payload_from_content_rejects_practice_progress_above_shared_bound() { + let mut payload = shared_contract_payload(json!({ "start": 10, "end": 30 })); + payload["sections"][0]["roles"][0]["practiceProgress"] = json!(101); + let content = serde_json::to_string(&payload).expect("payload should serialize"); + + assert!(project_payload_from_content(&content).is_err()); } #[test] diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts index 78f3866b1..d768e9c16 100644 --- a/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.test.ts @@ -133,6 +133,15 @@ describe("resolveFirstVoicingPlan", () => { ).toBeNull(); }); + it("skips Unicode line separators and accepts BOM-padded voicing text", () => { + for (const voicingPlan of ["Play\u0085here", "Play\u2028here", "Play\u2029here"]) { + expect(resolveFirstVoicingPlan(withVoicingSection({ voicingPlan }))).toBeNull(); + } + expect(resolveFirstVoicingPlan(withVoicingSection({ voicingPlan: "\uFEFF Play here \uFEFF" }))?.voicingPlan).toBe( + "Play here" + ); + }); + it("prefers the earlier of two voicing plans", () => { const song = withVoicingSection({ id: "verse-late", diff --git a/apps/desktop/src/features/workspace/firstVoicingPlan.ts b/apps/desktop/src/features/workspace/firstVoicingPlan.ts index 16ffee46c..887023757 100644 --- a/apps/desktop/src/features/workspace/firstVoicingPlan.ts +++ b/apps/desktop/src/features/workspace/firstVoicingPlan.ts @@ -1,6 +1,7 @@ import { MAX_SECTION_TIME_SECONDS, SECTION_FORM_LABELS, + isNonEmptySingleLineText, type RehearsalRole, type RehearsalSection, type RehearsalSong @@ -143,10 +144,10 @@ function ownedVoicingPlan(role: unknown): string | null { if (descriptor === null || typeof descriptor.value !== "string") { return null; } - const trimmed = descriptor.value.trim(); - if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + if (!isNonEmptySingleLineText(descriptor.value)) { return null; } + const trimmed = descriptor.value.trim(); return truncateCodePoints(trimmed, MAX_VOICING_PLAN_CHARACTERS); } diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index 543ded6cd..330c99ebe 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -408,6 +408,48 @@ function isOneOf(options: readonly T[], value: unknown): value return typeof value === "string" && options.includes(value as T); } +/** Return whether a code point is in the cross-language plan whitespace set. */ +function isPlanWhitespaceCodePoint(codePoint: number): boolean { + return ( + (codePoint >= 0x0009 && codePoint <= 0x000d) || + codePoint === 0x0020 || + codePoint === 0x0085 || + codePoint === 0x00a0 || + codePoint === 0x1680 || + (codePoint >= 0x2000 && codePoint <= 0x200a) || + codePoint === 0x2028 || + codePoint === 0x2029 || + codePoint === 0x202f || + codePoint === 0x205f || + codePoint === 0x3000 || + codePoint === 0xfeff + ); +} + +/** Apply one explicit cross-language Unicode whitespace policy to plan text. */ +export function isNonEmptySingleLineText(value: unknown): value is string { + if (typeof value !== "string") { + return false; + } + let hasNonWhitespace = false; + for (const character of value) { + const codePoint = character.codePointAt(0)!; + if ( + codePoint === 0x000a || + codePoint === 0x000d || + codePoint === 0x0085 || + codePoint === 0x2028 || + codePoint === 0x2029 + ) { + return false; + } + if (!isPlanWhitespaceCodePoint(codePoint)) { + hasNonWhitespace = true; + } + } + return hasNonWhitespace; +} + /** Documented. */ function invalidField(path: string): string { return `Invalid rehearsal song contract: invalid field '${path}'`; @@ -1555,7 +1597,7 @@ function validateRehearsalRole(value: unknown, path: string): string | null { if (value.transpositionPlan !== undefined && typeof value.transpositionPlan !== "string") { return invalidField(`${path}.transpositionPlan`); } - if (value.voicingPlan !== undefined && typeof value.voicingPlan !== "string") { + if (value.voicingPlan !== undefined && !isNonEmptySingleLineText(value.voicingPlan)) { return invalidField(`${path}.voicingPlan`); } if (!isDenseArray(value.manualOverrides)) { diff --git a/packages/shared-types/test/index.test.ts b/packages/shared-types/test/index.test.ts index a4d84347c..22793d74f 100644 --- a/packages/shared-types/test/index.test.ts +++ b/packages/shared-types/test/index.test.ts @@ -764,6 +764,28 @@ describe("shared type helpers", () => { expect(second.collaboration?.assignments).toHaveLength(2); }); + it("keeps optional voicing plans aligned with Rust project loading", () => { + for (const voicingPlan of [ + "", + " ", + "\uFEFF", + "\u0085", + "voice here\nthen move", + "voice here\rthen move", + "voice here\u0085then move" + ]) { + const song = createDemoRehearsalSong(); + song.sections[0]!.roles[0]!.voicingPlan = voicingPlan; + + expect(isRehearsalSong(song)).toBe(false); + expect(() => parseRehearsalSong(song)).toThrow("sections[0].roles[0].voicingPlan"); + } + + const paddedPlan = createDemoRehearsalSong(); + paddedPlan.sections[0]!.roles[0]!.voicingPlan = "\uFEFF Voice the string \uFEFF"; + expect(isRehearsalSong(paddedPlan)).toBe(true); + }); + it("validates and parses rehearsal song payloads", () => { const song = createDemoRehearsalSong(); const malformedSong = createDemoRehearsalSong() as unknown as {