From b7b23d7d6e3906efcaae612da496248b4ed88e89 Mon Sep 17 00:00:00 2001 From: Seongho Bae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:11:07 +0000 Subject: [PATCH 01/17] feat(workspace): name tonight's first fill plan on the map Show the earliest corroborated fill so a part can lock the walk-in before rehearsal. Open moves to the matching rendered map section. Do not invent fill copy from groove, cue, simplification, overlap, range, chords, setup, transposition, tuning, dynamics, or articulation. --- AGENTS.md | 1 + ARCHITECTURE.md | 3 +- CHANGELOG.md | 1 + CLAUDE.md | 2 +- .../FirstFillPlanCallout.identity.test.tsx | 21 ++ .../FirstFillPlanCallout.particle.test.tsx | 52 ++++ ...rstFillPlanCallout.reduced-motion.test.tsx | 43 +++ .../workspace/FirstFillPlanCallout.test.tsx | 262 ++++++++++++++++ .../workspace/FirstFillPlanCallout.tsx | 179 +++++++++++ ...tFillPlanCallout.unavailable-copy.test.tsx | 32 ++ ...stFillPlanCallout.workspace-scope.test.tsx | 54 ++++ .../src/features/workspace/Workspace.test.tsx | 30 ++ .../src/features/workspace/Workspace.tsx | 11 +- .../firstFillPlan.inherited-metadata.test.ts | 94 ++++++ .../firstFillPlan.section-label.test.ts | 14 + .../features/workspace/firstFillPlan.test.ts | 279 +++++++++++++++++ .../src/features/workspace/firstFillPlan.ts | 283 ++++++++++++++++++ apps/desktop/src/i18n/index.test.ts | 50 +++- apps/desktop/src/i18n/index.ts | 37 ++- apps/desktop/src/locales/en/common.json | 7 +- apps/desktop/src/locales/ko/common.json | 7 +- docs/design-system/component-contract.md | 1 + ...duced-motion-first-fill-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/FirstFillPlanCallout.identity.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.particle.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.reduced-motion.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.unavailable-copy.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.workspace-scope.test.tsx create mode 100644 apps/desktop/src/features/workspace/firstFillPlan.inherited-metadata.test.ts create mode 100644 apps/desktop/src/features/workspace/firstFillPlan.section-label.test.ts create mode 100644 apps/desktop/src/features/workspace/firstFillPlan.test.ts create mode 100644 apps/desktop/src/features/workspace/firstFillPlan.ts create mode 100644 docs/doctoring/reduced-motion-first-fill-plan-navigation.md diff --git a/AGENTS.md b/AGENTS.md index fca448ce9..5072a3833 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 fill plan with the owning part when an active role is corroborated, the owned `fillPlan` 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..b521f9b8d 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 fill plan must name the owning part when corroborated, the owned `fillPlan` 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..86774725b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first fill plan in the mounted rehearsal workspace so a part can lock the owned fill 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..6f59b122d 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 fill 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/FirstFillPlanCallout.identity.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.identity.test.tsx new file mode 100644 index 000000000..1c7929b57 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +it("gives co-mounted fill-plan callouts distinct DOM identities", () => { + render( + <> + + + + ); + + const callouts = screen.getAllByRole("complementary", { + name: "Tonight's first fill 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/FirstFillPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.particle.test.tsx new file mode 100644 index 000000000..8e6fed802 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +describe("FirstFillPlanCallout Korean role copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending role names particle-safe before and after the fill action", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + const seed = song.sections[0]!; + seed.roles = [ + { + ...seed.roles[0]!, + id: "piano", + name: "피아노", + rehearsalPriority: "high", + fillPlan: "Walk eight notes into the chorus downbeat; leave the vocal pickup empty." + } + ]; + 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/FirstFillPlanCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.reduced-motion.test.tsx new file mode 100644 index 000000000..ce19f243b --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +describe("FirstFillPlanCallout reduced motion", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("scrolls immediately when the operating system requests reduced motion", () => { + vi.stubGlobal("matchMedia", (query: string) => ({ + matches: query === "(prefers-reduced-motion: reduce)", + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn() + })); + + const grid = document.createElement("div"); + grid.dataset.testid = "song-structure-grid"; + grid.setAttribute("role", "region"); + grid.setAttribute("aria-label", "Scrollable song structure timeline"); + const target = document.createElement("div"); + target.dataset.sectionIndex = "0"; + const scrollIntoView = vi.fn(); + Object.defineProperty(target, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + grid.appendChild(target); + document.body.appendChild(grid); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.test.tsx new file mode 100644 index 000000000..35cff8378 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +const DEMO_FILL_PLAN = + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty."; + +function songWithFillPlan() { + 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("FirstFillPlanCallout", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText("No fill 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 = songWithFillPlan(); + Object.defineProperty(song, "id", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile song id getter"); + } + }); + + expect(() => render()).not.toThrow(); + expect(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })).toBeTruthy(); + }); + + it("resets armed guidance when accessor-id songs change with the same fill signature", () => { + const firstSong = songWithFillPlan(); + const nextSong = songWithFillPlan(); + for (const song of [firstSong, nextSong]) { + Object.defineProperty(song, "id", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile song id getter"); + } + }); + } + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })); + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect(screen.getByText("Bass Guitar still has a fill plan in the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeNull(); + + grid.remove(); + }); + + it("preserves armed guidance across immutable edits of the same owned song", () => { + const song = songWithFillPlan(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })); + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + expect(screen.queryByText("Bass Guitar still has a fill plan in the verse at 0:10.")).toBeNull(); + + grid.remove(); + }); + + it("does not show another part's fill plan under the named holding part", () => { + const song = songWithFillPlan(); + song.sections[0]!.roles[0]!.fillPlan = ""; + song.sections[0]!.roles[0]!.rehearsalPriority = "low"; + song.sections[0]!.roles[1]!.fillPlan = + "Tune the patch a half step down so the chorus still sits under the vocal."; + song.sections[0]!.roles[2]!.fillPlan = "Keep concert pitch even if the band drops the last chorus."; + + render(); + + expect( + screen.getByText("Keyboard 1 Right Hand still has a fill plan in the verse at 0:10.") + ).toBeTruthy(); + expect( + screen.getByText("Tune the patch a half step down so the chorus still sits under the vocal.") + ).toBeTruthy(); + expect(screen.queryByText("Keep concert pitch even if the band drops the last chorus.")).toBeNull(); + expect(screen.queryByText(DEMO_FILL_PLAN)).toBeNull(); + }); + + it("names the first fill plan as map navigation, scrolls to its rendered section, and arms that action", () => { + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + expect(screen.getByText(DEMO_FILL_PLAN)).toBeTruthy(); + const action = screen.getByRole("button", { + name: "Open Bass Guitar fill at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + grid.remove(); + }); + + it("keeps map navigation stable when the renderer accessible name is localized", () => { + const { grid, scrollIntoView } = appendSongStructureTarget("스크롤 가능한 곡 구조 타임라인"); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + grid.remove(); + }); + + it("does not claim map navigation completed when the rendered section target is missing", () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })); + + expect(screen.getByText("Bass Guitar still has a fill plan in the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeNull(); + }); + + it("navigates by renderer-owned section position instead of untrusted analysis ids", () => { + const song = songWithFillPlan(); + song.sections[0]!.id = "analysis section / duplicate"; + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill 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 Bass Guitar fill 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 fill plan changes or returns later", () => { + const initialSong = songWithFillPlan(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar fill at 0:10" })); + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + const nextSong = songWithFillPlan(); + nextSong.id = "next-song"; + nextSong.sections[0]!.timeRange = { start: 20, end: 40 }; + rerender(); + expect(screen.getByText("Bass Guitar still has a fill plan in the verse at 0:20.")).toBeTruthy(); + + grid.remove(); + }); + + it("keeps an unavailable fill plan guidance-only", () => { + const song = songWithFillPlan(); + for (const role of song.sections[0]!.roles) { + role.fillPlan = ""; + } + render(); + expect(screen.queryByRole("button")).toBeNull(); + expect( + screen.getByRole("complementary", { name: "Tonight's first fill plan" }) + ).toBeTruthy(); + expect( + screen.getByText("No fill plan is available. Stay on tonight's map for the next rehearsal cue.") + ).toBeTruthy(); + }); + + it("localizes the fill-plan form label instead of exposing its raw enum in Korean copy", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithFillPlan(); + song.sections[0]!.roles[0]!.name = "베이스"; + + render(); + + expect(screen.getByText("0:10 벌스에서 베이스 파트의 필인 계획이 있습니다.")).toBeTruthy(); + expect(screen.queryByText(/verse에서/)).toBeNull(); + }); + + it("renders the owned fill plan as a text node instead of template syntax", () => { + const song = songWithFillPlan(); + song.sections[0]!.roles[1]!.fillPlan = ""; + song.sections[0]!.roles[2]!.fillPlan = ""; + song.sections[0]!.roles[0]!.fillPlan = "Check {role} at {at}"; + render(); + expect(screen.getByText("Check {role} at {at}")).toBeTruthy(); + expect(screen.queryByText("Check Bass Guitar at 0:10")).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx new file mode 100644 index 000000000..87855daa2 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { formatFillPlanTime, resolveFirstFillPlan } from "./firstFillPlan"; + +/** Props for the first fill-plan rehearsal callout. */ +export interface FirstFillPlanCalloutProps { + song: RehearsalSong; +} + +type FillPlanCopyValues = Readonly>; + +type OpenedFillPlan = Readonly<{ + songIdentity: unknown; + sectionId: string; + sectionIndex: number; + holdingRoleId: string; + fillPlan: string; + atSeconds: number; +}>; + +/** Read a stable owned song id, falling back to object identity for untrusted identity metadata. */ +function stableFillPlanSongIdentity(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 fill-plan placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatFillPlanCopy(template: string, values: FillPlanCopyValues): string { + return template.replace(/\{(role|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof FillPlanCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredFillPlanScrollBehavior(): 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 resolveFillPlanRenderer(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 fill plan and open the matching rendered map section. */ +export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { + const calloutId = `workspace-surface-fill-plan-${useId()}`; + const locale = detectPreferredLocale(); + const t = createTranslator(locale); + const songIdentity = stableFillPlanSongIdentity(song); + const runtimeSong = song as unknown as Partial | null; + const named = resolveFirstFillPlan(song); + const namedSectionIndex = + named && Array.isArray(runtimeSong?.sections) + ? runtimeSong.sections.indexOf(named.section) + : -1; + const [openedFillPlan, setOpenedFillPlan] = useState(null); + + useEffect(() => { + setOpenedFillPlan(null); + }, [ + songIdentity, + namedSectionIndex, + named?.section.id, + named?.holdingRole.id, + named?.fillPlan, + named?.atSeconds + ]); + + if (!named) { + return ( + + ); + } + + const opened = + openedFillPlan !== null && + openedFillPlan.songIdentity === songIdentity && + openedFillPlan.sectionId === named.section.id && + openedFillPlan.sectionIndex === namedSectionIndex && + openedFillPlan.holdingRoleId === named.holdingRole.id && + openedFillPlan.fillPlan === named.fillPlan && + openedFillPlan.atSeconds === named.atSeconds; + const at = formatFillPlanTime(named.atSeconds); + const copyValues: FillPlanCopyValues = { + role: named.holdingRole.name, + section: translateSectionFormLabel(locale, named.section.label), + at + }; + const actionLabel = formatFillPlanCopy(t("firstFillPlanOpenAction"), copyValues); + const body = formatFillPlanCopy(t("firstFillPlanBody"), copyValues); + const armed = formatFillPlanCopy(t("firstFillPlanArmed"), copyValues); + + return ( + + ); +} \ No newline at end of file diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.unavailable-copy.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.unavailable-copy.test.tsx new file mode 100644 index 000000000..7469b2ed4 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +function songWithoutFillPlan() { + const song = createDemoRehearsalSong(); + for (const role of song.sections[0]!.roles) { + role.fillPlan = ""; + } + return song; +} + +describe("FirstFillPlanCallout unavailable copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("does not assert why the English fill plan is unavailable", () => { + render(); + + expect(screen.getByText("No fill plan is available. Stay on tonight's map for the next rehearsal cue.")).toBeTruthy(); + }); + + it("does not assert why the Korean fill plan is unavailable", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + + render(); + + expect(screen.getByText("사용 가능한 필인 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요.")).toBeTruthy(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.workspace-scope.test.tsx new file mode 100644 index 000000000..1a7f5c8bd --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +describe("FirstFillPlanCallout workspace scope", () => { + it("opens the song-structure renderer owned by the current workspace", () => { + const firstSong = createDemoRehearsalSong(); + const secondSong = createDemoRehearsalSong(); + secondSong.id = "second-workspace-song"; + + const { container } = render( + <> +
+ +
+
+
+
+
+ +
+
+
+
+ + ); + + const targets = container.querySelectorAll('[data-section-index="0"]'); + expect(targets).toHaveLength(2); + const firstScrollIntoView = vi.fn(); + const secondScrollIntoView = vi.fn(); + Object.defineProperty(targets[0]!, "scrollIntoView", { + configurable: true, + value: firstScrollIntoView + }); + Object.defineProperty(targets[1]!, "scrollIntoView", { + configurable: true, + value: secondScrollIntoView + }); + + const actions = screen.getAllByRole("button", { + name: "Open Bass Guitar fill 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..6c6745f8b 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 fill 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( + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty." + ).length + ).toBeGreaterThan(0); + const action = screen.getByRole("button", { + name: "Open Bass Guitar fill at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lock that fill on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + }); }); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 71546b524..1cdfe2b86 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 { FirstFillPlanCallout } from "./FirstFillPlanCallout"; 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/firstFillPlan.inherited-metadata.test.ts b/apps/desktop/src/features/workspace/firstFillPlan.inherited-metadata.test.ts new file mode 100644 index 000000000..92f275b10 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstFillPlan.inherited-metadata.test.ts @@ -0,0 +1,94 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstFillPlan } from "./firstFillPlan"; + +function songWithFillPlan() { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + section.id = "fill-own"; + section.roles = [ + { + ...section.roles[0]!, + id: "bass-guitar", + name: "Bass Guitar", + rehearsalPriority: "high", + fillPlan: "Walk eight notes into the chorus downbeat; leave the vocal pickup empty." + } + ]; + section.partGraph = [{ role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] }]; + song.sections = [section]; + return { song, section }; +} + +describe("resolveFirstFillPlan inherited metadata", () => { + it("rejects a song or section whose required metadata is inherited", () => { + const { song, section } = songWithFillPlan(); + const inheritedSong = Object.create({ sections: song.sections }) as typeof song; + expect(resolveFirstFillPlan(inheritedSong)).toBeNull(); + + const inheritedSection = Object.create(section) as typeof section; + song.sections = [inheritedSection]; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("rejects inherited timing fields", () => { + const { song, section } = songWithFillPlan(); + section.timeRange = Object.create({ start: 10, end: 30 }) as typeof section.timeRange; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("contains exceptions from own runtime accessors instead of trusting them", () => { + const { song, section } = songWithFillPlan(); + Object.defineProperty(section.roles[0]!, "fillPlan", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile fillPlan getter"); + } + }); + + expect(() => resolveFirstFillPlan(song)).not.toThrow(); + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("does not treat own accessors as stable fill-plan identity authority", () => { + const { song, section } = songWithFillPlan(); + Object.defineProperty(section, "id", { + configurable: true, + enumerable: true, + get() { + return "fill-own"; + } + }); + + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("does not let inherited fill plans establish the named copy", () => { + const { song, section } = songWithFillPlan(); + const inheritedRole = Object.create({ + fillPlan: "Inherited fill plan" + }) as (typeof section.roles)[0]; + Object.defineProperties(inheritedRole, { + id: { configurable: true, enumerable: true, value: "bass-guitar" }, + name: { configurable: true, enumerable: true, value: "Bass Guitar" }, + rehearsalPriority: { configurable: true, enumerable: true, value: "high" } + }); + section.roles = [inheritedRole]; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("does not let inherited role or graph metadata establish the holding part", () => { + const { song, section } = songWithFillPlan(); + const node = section.partGraph[0]!; + section.partGraph = [Object.create(node) as typeof node]; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("rejects arrays masquerading as section records", () => { + const { song, section } = songWithFillPlan(); + const arraySection = Object.assign([], section) as unknown as typeof section; + song.sections = [arraySection]; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstFillPlan.section-label.test.ts b/apps/desktop/src/features/workspace/firstFillPlan.section-label.test.ts new file mode 100644 index 000000000..4e9f3d6a7 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstFillPlan.section-label.test.ts @@ -0,0 +1,14 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstFillPlan } from "./firstFillPlan"; + +describe("resolveFirstFillPlan 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(resolveFirstFillPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstFillPlan.test.ts b/apps/desktop/src/features/workspace/firstFillPlan.test.ts new file mode 100644 index 000000000..ba0a8f020 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstFillPlan.test.ts @@ -0,0 +1,279 @@ +import { describe, expect, it } from "vitest"; +import { MAX_SECTION_TIME_SECONDS, createDemoRehearsalSong } from "@bandscope/shared-types"; +import { formatFillPlanTime, resolveFirstFillPlan } from "./firstFillPlan"; + +const DEMO_FILL_PLAN = + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty."; + +function withFillSection( + overrides: { + id?: string; + start?: number; + end?: number; + fillPlan?: 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-fill"; + 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." + }, + fillPlan: + overrides.fillPlan ?? + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty.", + manualOverrides: [] + } + ]; + section.partGraph = [ + { + role_id: roleId, + is_active: overrides.isActive ?? true, + handoff_to: [], + handoff_from: [] + } + ]; + song.sections = [section]; + return song; +} + +describe("resolveFirstFillPlan", () => { + it("picks the demo song's earliest high-priority fill plan and the part that owns it", () => { + const resolved = resolveFirstFillPlan(createDemoRehearsalSong()); + expect(resolved?.section.id).toBe("verse-1"); + expect(resolved?.holdingRole.id).toBe("bass-guitar"); + expect(resolved?.fillPlan).toBe(DEMO_FILL_PLAN); + expect(resolved?.atSeconds).toBe(10); + expect(formatFillPlanTime(resolved?.atSeconds ?? -1)).toBe("0:10"); + expect(formatFillPlanTime(Number.NaN)).toBe("0:00"); + expect(formatFillPlanTime(-4)).toBe("0:00"); + }); + + it("does not invent a fill 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 = withFillSection(); + delete song.sections[0]!.roles[0]!.fillPlan; + 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 = + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty."; + 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]!.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: "Walk eight notes into the chorus downbeat; leave the vocal pickup empty." + }; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("skips a blank fill plan", () => { + expect(resolveFirstFillPlan(withFillSection({ fillPlan: " " }))).toBeNull(); + }); + + it("skips a multi-line fill plan", () => { + expect( + resolveFirstFillPlan(withFillSection({ fillPlan: "Drop under the vocal.\nKeep the pickup." })) + ).toBeNull(); + }); + + it("prefers the earlier of two fill plans", () => { + const song = withFillSection({ + id: "verse-late", + start: 40, + end: 56, + roleId: "keys-right", + fillPlan: "Late fill." + }); + const earlier = structuredClone(song.sections[0]!); + earlier.id = "verse-early"; + earlier.roles = [ + { + ...earlier.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "low", + fillPlan: "Earlier fill." + } + ]; + 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 = resolveFirstFillPlan(song); + expect(resolved?.section.id).toBe("verse-early"); + expect(resolved?.holdingRole.id).toBe("lead-vocal"); + expect(resolved?.fillPlan).toBe("Earlier fill."); + expect(resolved?.atSeconds).toBe(8); + }); + + it("breaks same-time fill-plan ties with locale-independent id ordering", () => { + const song = withFillSection({ id: "ä-fill", start: 10, end: 26 }); + const ascii = structuredClone(song.sections[0]!); + ascii.id = "z-fill"; + song.sections = [song.sections[0]!, ascii]; + + expect(resolveFirstFillPlan(song)?.section.id).toBe("z-fill"); + }); + + it("prefers a high-priority fill part over a low-priority part in the same section", () => { + const song = withFillSection({ + roleId: "keys-right", + roleName: "Keys", + priority: "low", + fillPlan: "Low-priority fill." + }); + const section = song.sections[0]!; + const highRole = { + ...section.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "high" as const, + fillPlan: "High-priority fill." + }; + 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(resolveFirstFillPlan(song)?.holdingRole.id).toBe("lead-vocal"); + expect(resolveFirstFillPlan(song)?.fillPlan).toBe("High-priority fill."); + }); + + it("breaks equal-priority role ties with locale-independent id ordering", () => { + const song = withFillSection({ roleId: "ä-role", roleName: "Umlaut role", priority: "high" }); + const section = song.sections[0]!; + const asciiRole = { + ...section.roles[0]!, + id: "z-role", + name: "ASCII role", + fillPlan: "ASCII fill." + }; + 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(resolveFirstFillPlan(song)?.holdingRole.id).toBe("z-role"); + expect(resolveFirstFillPlan(song)?.fillPlan).toBe("ASCII fill."); + }); + + it("skips a fill plan whose graph node is inactive", () => { + expect(resolveFirstFillPlan(withFillSection({ isActive: false }))).toBeNull(); + }); + + it("skips a fill plan whose rehearsal window is unbounded", () => { + expect(resolveFirstFillPlan(withFillSection({ start: Number.NaN, end: 30 }))).toBeNull(); + }); + + it("skips a fill plan whose end precedes its start", () => { + expect(resolveFirstFillPlan(withFillSection({ start: 30, end: 10 }))).toBeNull(); + }); + + it("skips a zero-length fill-plan window", () => { + expect(resolveFirstFillPlan(withFillSection({ start: 10, end: 10 }))).toBeNull(); + }); + + it("skips a fill plan whose endpoint overflows the shared timing bound", () => { + expect( + resolveFirstFillPlan( + withFillSection({ + start: MAX_SECTION_TIME_SECONDS, + end: MAX_SECTION_TIME_SECONDS + 1 + }) + ) + ).toBeNull(); + }); + + it("returns null for a non-object song root", () => { + expect(resolveFirstFillPlan(null as never)).toBeNull(); + }); + + it("returns null when the runtime section collection is sparse", () => { + const song = withFillSection(); + const sparseSections: typeof song.sections = new Array(2); + sparseSections[1] = song.sections[0]!; + song.sections = sparseSections; + expect(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("keeps the fill plan unnamed when role identities are duplicated", () => { + const song = withFillSection(); + 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(resolveFirstFillPlan(song)).toBeNull(); + }); + + it("bounds the fill plan to 180 Unicode code points", () => { + const song = withFillSection({ fillPlan: `${"G".repeat(200)}` }); + const resolved = resolveFirstFillPlan(song); + expect(resolved?.fillPlan.length).toBe(180); + }); + + it("does not split a Unicode surrogate pair at the fill-plan boundary", () => { + const song = withFillSection({ fillPlan: `${"a".repeat(179)}😀tail` }); + const resolved = resolveFirstFillPlan(song); + expect(Array.from(resolved?.fillPlan ?? "")).toHaveLength(180); + expect(resolved?.fillPlan.endsWith("😀")).toBe(true); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstFillPlan.ts b/apps/desktop/src/features/workspace/firstFillPlan.ts new file mode 100644 index 000000000..c06c64d33 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstFillPlan.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_FILL_PLAN_CHARACTERS = 180; +const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); + +/** Tonight's first fill plan: the earliest labeled section and the part that owns it. */ +export type FirstFillPlan = { + section: RehearsalSection; + holdingRole: RehearsalRole; + fillPlan: string; + atSeconds: number; +}; + +/** Format a non-negative fill-plan time as m:ss for rehearsal copy. */ +export function formatFillPlanTime(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 fill plan, or null when it cannot be shown. */ +function ownedFillPlan(role: unknown): string | null { + if (!isRuntimeObject(role) || !hasOwnData(role, "fillPlan")) { + return null; + } + const fillPlan = (role as { fillPlan?: unknown }).fillPlan; + if (typeof fillPlan !== "string") { + return null; + } + const trimmed = fillPlan.trim(); + if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + return null; + } + return truncateCodePoints(trimmed, MAX_FILL_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 fill plan after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstFillPlan(song: RehearsalSong): FirstFillPlan | 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) => ownedFillPlan(role) !== null) + ); + if (!holdingRole) { + return []; + } + const fillPlan = ownedFillPlan(holdingRole); + if (!fillPlan) { + return []; + } + return [ + { + section, + holdingRole, + fillPlan, + 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 fill plan, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstFillPlan(song: RehearsalSong): FirstFillPlan | null { + try { + return resolveSafeFirstFillPlan(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..5721951d7 100644 --- a/apps/desktop/src/i18n/index.test.ts +++ b/apps/desktop/src/i18n/index.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { createTranslator, detectPreferredLocale } from "./index"; +import { createTranslator, detectPreferredLocale, translateSectionFormLabel } from "./index"; import koCommon from "../locales/ko/common.json"; describe("i18n", () => { @@ -75,4 +75,52 @@ describe("i18n", () => { } }); }); + + describe("translateSectionFormLabel", () => { + it("localizes every supported Korean section form label", () => { + expect( + [ + "intro", + "verse", + "pre-chorus", + "chorus", + "bridge", + "outro", + "tag", + "pickup", + "stop", + "handoff" + ].map((label) => translateSectionFormLabel("ko", label as never)) + ).toEqual([ + "인트로", + "벌스", + "프리코러스", + "코러스", + "브리지", + "아웃트로", + "태그", + "픽업", + "스톱", + "핸드오프" + ]); + }); + + it("preserves every supported English section form label", () => { + expect(translateSectionFormLabel("en", "verse")).toBe("verse"); + expect(translateSectionFormLabel("en", "pre-chorus")).toBe("pre-chorus"); + }); + + it("does not read inherited Object keys as section labels", () => { + const inheritedKey = "toString" as never; + expect(translateSectionFormLabel("en", inheritedKey)).toBe("toString"); + expect(translateSectionFormLabel("ko", inheritedKey)).toBe("toString"); + }); + + it("keeps Korean first-fill-plan next-action copy particle-safe", () => { + const t = createTranslator("ko"); + expect(t("firstFillPlanOpenAction")).toBe("{at} {role} 필인 열기"); + expect(t("firstFillPlanBody")).toBe("{at} {section}에서 {role} 파트의 필인 계획이 있습니다."); + expect(t("firstFillPlanArmed")).toBe("{at}에서 {role} 파트의 필인을 맞춘 다음 합주를 시작하세요."); + }); + }); }); diff --git a/apps/desktop/src/i18n/index.ts b/apps/desktop/src/i18n/index.ts index 1a9f471f0..18e34b831 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")) { @@ -26,3 +60,4 @@ export function detectPreferredLocale(): Locale { return "en"; } + diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index 39f716d50..395dfa77e 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", + "firstFillPlanLabel": "Tonight's first fill plan", + "firstFillPlanOpenAction": "Open {role} fill at {at}", + "firstFillPlanBody": "{role} still has a fill plan in the {section} at {at}.", + "firstFillPlanArmed": "Lock that fill on {role} at {at} before the room starts.", + "firstFillPlanUnavailable": "No fill 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..d47d53541 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": "진척도 증가", + "firstFillPlanLabel": "오늘 첫 필인 계획", + "firstFillPlanOpenAction": "{at} {role} 필인 열기", + "firstFillPlanBody": "{at} {section}에서 {role} 파트의 필인 계획이 있습니다.", + "firstFillPlanArmed": "{at}에서 {role} 파트의 필인을 맞춘 다음 합주를 시작하세요.", + "firstFillPlanUnavailable": "사용 가능한 필인 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요." } diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 22602c313..d49e6ec61 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 Fill Plan Callout | workspace next-action pattern | `apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx` | Name the owning part when an active graph node corroborates it, the owned `fillPlan` 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-fill-plan-navigation.md b/docs/doctoring/reduced-motion-first-fill-plan-navigation.md new file mode 100644 index 000000000..71bb9d168 --- /dev/null +++ b/docs/doctoring/reduced-motion-first-fill-plan-navigation.md @@ -0,0 +1,3 @@ +# Reduced-motion first fill-plan navigation + +Open tonight's first fill 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..1eda7d99d 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; + fillPlan?: string; manualOverrides: ManualOverride[]; overlapWarnings: string[]; transcription?: TranscriptionNote[]; @@ -474,6 +475,7 @@ const demoRehearsalSongSeed: RehearsalSong = { simplification: "Stay on roots if the chorus entrance gets muddy.", setupNote: "Keep the attack short so the verse breathes.", transpositionPlan: "If the singer drops to B minor, keep the shape a whole step lower and let keys keep the color tones.", + fillPlan: "Walk eight notes into the chorus downbeat; leave the vocal pickup empty.", manualOverrides: [], overlapWarnings: [ "Density warning: competing with Keyboard Left Hand in low register." @@ -1497,6 +1499,7 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "simplification", "setupNote", "transpositionPlan", + "fillPlan", "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.fillPlan !== undefined && typeof value.fillPlan !== "string") { + return invalidField(`${path}.fillPlan`); + } 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..e706dfd8b 100644 --- a/packages/shared-types/test/index.test.ts +++ b/packages/shared-types/test/index.test.ts @@ -738,6 +738,7 @@ describe("shared type helpers", () => { expect(song.sections[0]?.roles[2]?.harmony?.source).toBe("model"); expect(song.sections[0]?.roles[0]?.harmonicExplanation).toContain("tonal floor"); expect(song.sections[0]?.roles[0]?.transpositionPlan).toContain("whole step lower"); + expect(song.sections[0]?.roles[0]?.fillPlan).toContain("chorus downbeat"); expect(song.collaboration?.assignments).toHaveLength(2); expect(song.collaboration?.comments[0]?.status).toBe("open"); expect(song.sections[0]?.roles[2]?.manualOverrides?.[0]).toMatchObject({ @@ -1257,6 +1258,12 @@ describe("shared type helpers", () => { song.sections[0]!.roles[0]!.transpositionPlan = 2 as never; }) }, + { + message: "sections[0].roles[0].fillPlan", + payload: createInvalidSong((song) => { + song.sections[0]!.roles[0]!.fillPlan = 2 as never; + }) + }, { message: "sections[0].roles[0].practiceProgress", payload: createInvalidSong((song) => { From 7a7b0553a9a5e33d9ae4bba412465a19d87a0e19 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:15:57 -0700 Subject: [PATCH 02/17] test(workspace): lock fill-plan own-data authority --- .../firstFillPlan.proxy-authority.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts diff --git a/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts b/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts new file mode 100644 index 000000000..e3e9165b9 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts @@ -0,0 +1,31 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstFillPlan } from "./firstFillPlan"; + +const DEMO_FILL_PLAN = + "Walk eight notes into the chorus downbeat; leave the vocal pickup empty."; + +describe("resolveFirstFillPlan own-data authority", () => { + it("uses the snapshotted own-data fill plan instead of a Proxy get trap", () => { + const song = createDemoRehearsalSong(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + const roleIndex = section?.roles.findIndex((role) => role.id === "bass-guitar") ?? -1; + const role = roleIndex >= 0 ? section?.roles[roleIndex] : undefined; + expect(section).toBeDefined(); + expect(role).toBeDefined(); + if (!section || !role || roleIndex < 0) { + throw new Error("Demo fill-plan fixture is missing the expected Bass Guitar role."); + } + + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "fillPlan") { + return "Injected proxy fill."; + } + return Reflect.get(target, property, receiver); + } + }); + + expect(resolveFirstFillPlan(song)?.fillPlan).toBe(DEMO_FILL_PLAN); + }); +}); From 070fceff58a5ab87089981c7e8424ccd75a4c05b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:17:10 -0700 Subject: [PATCH 03/17] fix(workspace): snapshot owned fill-plan data --- apps/desktop/src/features/workspace/firstFillPlan.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstFillPlan.ts b/apps/desktop/src/features/workspace/firstFillPlan.ts index c06c64d33..808402cf5 100644 --- a/apps/desktop/src/features/workspace/firstFillPlan.ts +++ b/apps/desktop/src/features/workspace/firstFillPlan.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 fill plan, or null when it cannot be shown. */ +/** Return a bounded snapshotted own fill plan, or null when it cannot be shown. */ function ownedFillPlan(role: unknown): string | null { - if (!isRuntimeObject(role) || !hasOwnData(role, "fillPlan")) { + if (!isRuntimeObject(role)) { return null; } - const fillPlan = (role as { fillPlan?: unknown }).fillPlan; + const descriptor = Object.getOwnPropertyDescriptor(role, "fillPlan"); + if (descriptor === undefined || !Object.prototype.hasOwnProperty.call(descriptor, "value")) { + return null; + } + const fillPlan = descriptor.value; if (typeof fillPlan !== "string") { return null; } From 4e74faa430dc0b45266811a582094f58f255d9a8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:37:54 -0700 Subject: [PATCH 04/17] test(workspace): require fill plan coverage ownership --- .../src/features/workspace/coverageContract.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 apps/desktop/src/features/workspace/coverageContract.test.ts diff --git a/apps/desktop/src/features/workspace/coverageContract.test.ts b/apps/desktop/src/features/workspace/coverageContract.test.ts new file mode 100644 index 000000000..0c32ede5f --- /dev/null +++ b/apps/desktop/src/features/workspace/coverageContract.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { DESKTOP_OWNED_PRODUCTION_COVERAGE } from "../../../vite.config"; + +describe("desktop owned production coverage", () => { + it("keeps the first fill-plan resolver and callout inside the coverage gate", () => { + expect(DESKTOP_OWNED_PRODUCTION_COVERAGE).toEqual( + expect.arrayContaining([ + "src/features/workspace/firstFillPlan.ts", + "src/features/workspace/FirstFillPlanCallout.tsx" + ]) + ); + }); +}); From 8bdd5f5ca17baa002b97dfdd6868aae51b10edfb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:38:22 -0700 Subject: [PATCH 05/17] fix(coverage): include fill plan 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..38746d3b9 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/firstFillPlan.ts", + "src/features/workspace/FirstFillPlanCallout.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 ddeee9a7271ac1132087295879b863a209467a4b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:39:07 -0700 Subject: [PATCH 06/17] test(workspace): require fill plan resolver reuse --- .../FirstFillPlanCallout.memoization.test.tsx | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.memoization.test.tsx diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.memoization.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.memoization.test.tsx new file mode 100644 index 000000000..538d08014 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.memoization.test.tsx @@ -0,0 +1,25 @@ +import { render } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +describe("FirstFillPlanCallout resolver reuse", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("does not rescan role metadata when a parent rerenders the same song object", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + const descriptorSpy = vi.spyOn(Object, "getOwnPropertyDescriptor"); + + const { rerender } = render(); + const firstScanCount = descriptorSpy.mock.calls.filter(([target]) => target === role).length; + expect(firstScanCount).toBeGreaterThan(0); + + rerender(); + const secondScanCount = descriptorSpy.mock.calls.filter(([target]) => target === role).length; + + expect(secondScanCount).toBe(firstScanCount); + }); +}); From ca6862c09b59b375e1deece5f7f145c46a42f4c0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 04:39:36 -0700 Subject: [PATCH 07/17] perf(workspace): memoize first fill plan resolution --- .../desktop/src/features/workspace/FirstFillPlanCallout.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx index 87855daa2..612f77cf8 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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 { @@ -82,7 +82,7 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { const t = createTranslator(locale); const songIdentity = stableFillPlanSongIdentity(song); const runtimeSong = song as unknown as Partial | null; - const named = resolveFirstFillPlan(song); + const named = useMemo(() => resolveFirstFillPlan(song), [song]); const namedSectionIndex = named && Array.isArray(runtimeSong?.sections) ? runtimeSong.sections.indexOf(named.section) @@ -176,4 +176,4 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { ); -} \ No newline at end of file +} From f45a2fcf100c3a7ddb3f6dbc87fca2d6faafaf59 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 09:26:11 -0700 Subject: [PATCH 08/17] test(workspace): reject fill-plan root proxy reread --- .../FirstFillPlanCallout.proxy-root.test.tsx | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 apps/desktop/src/features/workspace/FirstFillPlanCallout.proxy-root.test.tsx diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.proxy-root.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.proxy-root.test.tsx new file mode 100644 index 000000000..18696ad3c --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.proxy-root.test.tsx @@ -0,0 +1,22 @@ +import { render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { expect, it } from "vitest"; +import { FirstFillPlanCallout } from "./FirstFillPlanCallout"; + +it("does not read root sections through a Proxy get trap after validating the fill plan", () => { + const source = createDemoRehearsalSong(); + const song = new Proxy(source, { + get(target, property, receiver) { + if (property === "sections") { + throw new Error("root sections must be consumed from owned data authority"); + } + return Reflect.get(target, property, receiver); + } + }); + + render(); + + expect( + screen.getByText("Walk eight notes into the chorus downbeat; leave the vocal pickup empty.") + ).toBeTruthy(); +}); From b5b6a0c228638966ab731daeb741b06b2b2e74b6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 09:26:48 -0700 Subject: [PATCH 09/17] fix(workspace): snapshot fill-plan section authority --- .../src/features/workspace/firstFillPlan.ts | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstFillPlan.ts b/apps/desktop/src/features/workspace/firstFillPlan.ts index 808402cf5..1ff3ba6e9 100644 --- a/apps/desktop/src/features/workspace/firstFillPlan.ts +++ b/apps/desktop/src/features/workspace/firstFillPlan.ts @@ -13,6 +13,7 @@ const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); /** Tonight's first fill plan: the earliest labeled section and the part that owns it. */ export type FirstFillPlan = { section: RehearsalSection; + sectionIndex: number; holdingRole: RehearsalRole; fillPlan: string; atSeconds: number; @@ -50,6 +51,14 @@ function hasOwnData(value: object, key: PropertyKey): boolean { return descriptor !== undefined && Object.prototype.hasOwnProperty.call(descriptor, "value"); } +/** Snapshot one owned data-property value without invoking an accessor or Proxy get trap. */ +function ownDataValue(value: object, key: PropertyKey): unknown { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor !== undefined && Object.prototype.hasOwnProperty.call(descriptor, "value") + ? descriptor.value + : undefined; +} + /** 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)) { @@ -233,13 +242,19 @@ function rankedActiveRoles(section: RehearsalSection): RehearsalRole[] { /** Resolve a fill plan after the runtime root has passed its structural boundary checks. */ function resolveSafeFirstFillPlan(song: RehearsalSong): FirstFillPlan | null { - if (!isRuntimeObject(song) || !hasOwnData(song, "sections") || !isDenseRuntimeArray(song.sections)) { + if (!isRuntimeObject(song)) { + return null; + } + const sectionsValue = ownDataValue(song, "sections"); + if (!isDenseRuntimeArray(sectionsValue)) { return null; } + const sections = sectionsValue as RehearsalSection[]; - const candidates = song.sections + const candidates = sections + .map((section, sectionIndex) => ({ section, sectionIndex })) .filter( - (section) => + ({ section }) => isRuntimeObject(section) && hasSupportedSectionLabel(section) && hasOwnData(section, "id") && @@ -247,7 +262,7 @@ function resolveSafeFirstFillPlan(song: RehearsalSong): FirstFillPlan | null { section.id.trim().length > 0 && hasBoundedTimeRange(section) ) - .flatMap((section) => { + .flatMap(({ section, sectionIndex }) => { const holdingRole = pickHoldingRole( rankedActiveRoles(section).filter((role) => ownedFillPlan(role) !== null) ); @@ -261,6 +276,7 @@ function resolveSafeFirstFillPlan(song: RehearsalSong): FirstFillPlan | null { return [ { section, + sectionIndex, holdingRole, fillPlan, atSeconds: section.timeRange.start From e1d531cc99bfb26abb4fd555ef9b602c52e2dbae Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 09:27:11 -0700 Subject: [PATCH 10/17] fix(workspace): reuse validated fill-plan section index --- .../desktop/src/features/workspace/FirstFillPlanCallout.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx index 612f77cf8..b8a11d4da 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx @@ -81,12 +81,8 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { const locale = detectPreferredLocale(); const t = createTranslator(locale); const songIdentity = stableFillPlanSongIdentity(song); - const runtimeSong = song as unknown as Partial | null; const named = useMemo(() => resolveFirstFillPlan(song), [song]); - const namedSectionIndex = - named && Array.isArray(runtimeSong?.sections) - ? runtimeSong.sections.indexOf(named.section) - : -1; + const namedSectionIndex = named?.sectionIndex ?? -1; const [openedFillPlan, setOpenedFillPlan] = useState(null); useEffect(() => { From e8dad78da9aeb294d4df088f7a69852b9772b792 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 15:48:17 -0700 Subject: [PATCH 11/17] test(workspace): pin fill-plan runtime authority --- .../firstFillPlan.proxy-authority.test.ts | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts b/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts index e3e9165b9..fa7f3c503 100644 --- a/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts +++ b/apps/desktop/src/features/workspace/firstFillPlan.proxy-authority.test.ts @@ -28,4 +28,56 @@ describe("resolveFirstFillPlan own-data authority", () => { expect(resolveFirstFillPlan(song)?.fillPlan).toBe(DEMO_FILL_PLAN); }); + + it("uses the snapshotted own-data time range instead of a Proxy get trap", () => { + const song = createDemoRehearsalSong(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + expect(section).toBeDefined(); + if (!section) { + throw new Error("Demo fill-plan fixture is missing the expected verse section."); + } + const expectedStart = section.timeRange.start; + section.timeRange = new Proxy(section.timeRange, { + get(target, property, receiver) { + if (property === "start") { + return expectedStart + 15; + } + return Reflect.get(target, property, receiver); + } + }); + + expect(resolveFirstFillPlan(song)?.atSeconds).toBe(expectedStart); + }); + + it("returns snapshotted role identity and display copy instead of Proxy get values", () => { + const song = createDemoRehearsalSong(); + const section = song.sections.find((candidate) => candidate.id === "verse-1"); + const roleIndex = section?.roles.findIndex((role) => role.id === "bass-guitar") ?? -1; + const role = roleIndex >= 0 ? section?.roles[roleIndex] : undefined; + expect(section).toBeDefined(); + expect(role).toBeDefined(); + if (!section || !role || roleIndex < 0) { + throw new Error("Demo fill-plan fixture is missing the expected Bass Guitar role."); + } + const expectedId = role.id; + const expectedName = role.name; + section.roles[roleIndex] = new Proxy(role, { + get(target, property, receiver) { + if (property === "name") { + return "Injected proxy role"; + } + return Reflect.get(target, property, receiver); + } + }); + + const resolved = resolveFirstFillPlan(song) as + | (ReturnType & { + holdingRoleId?: string; + holdingRoleName?: string; + }) + | null; + expect(resolved?.fillPlan).toBe(DEMO_FILL_PLAN); + expect(resolved?.holdingRoleId).toBe(expectedId); + expect(resolved?.holdingRoleName).toBe(expectedName); + }); }); From f917bff0f7771677f07c43d31a77d21c49797669 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 15:48:55 -0700 Subject: [PATCH 12/17] fix(workspace): snapshot fill-plan runtime authority --- .../src/features/workspace/firstFillPlan.ts | 261 ++++++++++-------- 1 file changed, 143 insertions(+), 118 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstFillPlan.ts b/apps/desktop/src/features/workspace/firstFillPlan.ts index 1ff3ba6e9..5c48aa4bf 100644 --- a/apps/desktop/src/features/workspace/firstFillPlan.ts +++ b/apps/desktop/src/features/workspace/firstFillPlan.ts @@ -10,11 +10,22 @@ const PRIORITY_RANK = { high: 0, medium: 1, low: 2 } as const; const MAX_FILL_PLAN_CHARACTERS = 180; const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); +type RankedRoleMetadata = Readonly<{ + role: RehearsalRole; + id: string; + name: string; + rehearsalPriority: keyof typeof PRIORITY_RANK; +}>; + /** Tonight's first fill plan: the earliest labeled section and the part that owns it. */ export type FirstFillPlan = { section: RehearsalSection; + sectionId: string; + sectionLabel: RehearsalSection["label"]; sectionIndex: number; holdingRole: RehearsalRole; + holdingRoleId: string; + holdingRoleName: string; fillPlan: string; atSeconds: number; }; @@ -59,21 +70,28 @@ function ownDataValue(value: object, key: PropertyKey): unknown { : undefined; } -/** Return whether every numeric index is an own data element in a bounded runtime array. */ -function isDenseRuntimeArray(value: unknown): value is unknown[] { +/** Snapshot every numeric own data element from a bounded runtime array. */ +function ownedDenseRuntimeArray(value: unknown): unknown[] | null { if (!Array.isArray(value)) { - return false; + return null; } - const length = Number(value.length); - if (!Number.isSafeInteger(length) || length < 0 || length > 0xffffffff) { - return false; + const length = ownDataValue(value, "length"); + if ( + typeof length !== "number" || + !Number.isSafeInteger(length) || + length < 0 || + length > 0xffffffff + ) { + return null; } + const items: unknown[] = []; for (let index = 0; index < length; index += 1) { if (!hasOwnData(value, index)) { - return false; + return null; } + items.push(ownDataValue(value, index)); } - return true; + return items; } /** Bound buyer-visible text by Unicode code points without splitting a surrogate pair. */ @@ -95,11 +113,7 @@ function ownedFillPlan(role: unknown): string | null { if (!isRuntimeObject(role)) { return null; } - const descriptor = Object.getOwnPropertyDescriptor(role, "fillPlan"); - if (descriptor === undefined || !Object.prototype.hasOwnProperty.call(descriptor, "value")) { - return null; - } - const fillPlan = descriptor.value; + const fillPlan = ownDataValue(role, "fillPlan"); if (typeof fillPlan !== "string") { return null; } @@ -110,49 +124,55 @@ function ownedFillPlan(role: unknown): string | null { return truncateCodePoints(trimmed, MAX_FILL_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) - ); +/** Snapshot trusted role identity, display name, and priority without Proxy get authority. */ +function ownedRankedRoleMetadata(role: unknown): RankedRoleMetadata | null { + if (!isRuntimeObject(role)) { + return null; + } + const id = ownDataValue(role, "id"); + const name = ownDataValue(role, "name"); + const rehearsalPriority = ownDataValue(role, "rehearsalPriority"); + if ( + typeof id !== "string" || + id.trim().length === 0 || + typeof name !== "string" || + name.trim().length === 0 || + typeof rehearsalPriority !== "string" || + !Object.prototype.hasOwnProperty.call(PRIORITY_RANK, rehearsalPriority) + ) { + return null; + } + return { + role: role as RehearsalRole, + id, + name, + rehearsalPriority: rehearsalPriority as keyof typeof PRIORITY_RANK + }; } -/** Return whether a section owns a bounded, positive-length integer rehearsal window. */ -function hasBoundedTimeRange(section: RehearsalSection): boolean { - if (!hasOwnData(section, "timeRange")) { - return false; +/** Snapshot a section's bounded positive-length integer rehearsal window. */ +function ownedBoundedTimeRange( + section: RehearsalSection +): RehearsalSection["timeRange"] | null { + const timeRange = ownDataValue(section, "timeRange"); + if (!isRuntimeObject(timeRange)) { + return null; } - const timeRange = section.timeRange as Partial | null; - if (!isRuntimeObject(timeRange) || !hasOwnData(timeRange, "start") || !hasOwnData(timeRange, "end")) { - return false; + const start = ownDataValue(timeRange, "start"); + const end = ownDataValue(timeRange, "end"); + if ( + typeof start !== "number" || + !Number.isInteger(start) || + start < 0 || + start > MAX_SECTION_TIME_SECONDS || + typeof end !== "number" || + !Number.isInteger(end) || + end <= start || + end > MAX_SECTION_TIME_SECONDS + ) { + return null; } - - 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 { start, end }; } /** Return safe identities that appear more than once in one section-local collection. */ @@ -170,13 +190,14 @@ function repeatedIds(ids: string[]): Set { } /** Prefer the earlier ranked role, then rehearsal priority, then a locale-independent id. */ -function pickHoldingRole(roles: RehearsalRole[]): RehearsalRole | null { +function pickHoldingRole(roles: RankedRoleMetadata[]): RankedRoleMetadata | null { if (roles.length === 0) { return null; } return ( [...roles].sort((left, right) => { - const priorityDelta = PRIORITY_RANK[left.rehearsalPriority] - PRIORITY_RANK[right.rehearsalPriority]; + const priorityDelta = + PRIORITY_RANK[left.rehearsalPriority] - PRIORITY_RANK[right.rehearsalPriority]; if (priorityDelta !== 0) { return priorityDelta; } @@ -186,58 +207,51 @@ function pickHoldingRole(roles: RehearsalRole[]): RehearsalRole | 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) - ) { +function rankedActiveRoles(section: RehearsalSection): RankedRoleMetadata[] { + const roles = ownedDenseRuntimeArray(ownDataValue(section, "roles")); + const partGraph = ownedDenseRuntimeArray(ownDataValue(section, "partGraph")); + if (!roles || !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 safeRoleIds = roles.flatMap((role) => { + if (!isRuntimeObject(role)) { + return []; + } + const id = ownDataValue(role, "id"); + return typeof id === "string" && id.trim().length > 0 ? [id] : []; + }); + const safeGraphRoleIds = partGraph.flatMap((node) => { + if (!isRuntimeObject(node)) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && roleId.trim().length > 0 ? [roleId] : []; + }); const repeatedRoleIds = repeatedIds(safeRoleIds); const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); const activeIds = new Set( - 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) + partGraph.flatMap((node) => { + if (!isRuntimeObject(node) || ownDataValue(node, "is_active") !== true) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && + roleId.trim().length > 0 && + !repeatedGraphRoleIds.has(roleId) + ? [roleId] + : []; + }) ); - return section.roles.filter( - (role) => - isRuntimeObject(role) && - hasRankedPriority(role) && - !repeatedRoleIds.has(role.id) && - activeIds.has(role.id) - ); + return roles.flatMap((role) => { + const metadata = ownedRankedRoleMetadata(role); + return metadata !== null && + !repeatedRoleIds.has(metadata.id) && + activeIds.has(metadata.id) + ? [metadata] + : []; + }); } /** Resolve a fill plan after the runtime root has passed its structural boundary checks. */ @@ -245,41 +259,52 @@ function resolveSafeFirstFillPlan(song: RehearsalSong): FirstFillPlan | null { if (!isRuntimeObject(song)) { return null; } - const sectionsValue = ownDataValue(song, "sections"); - if (!isDenseRuntimeArray(sectionsValue)) { + const sections = ownedDenseRuntimeArray(ownDataValue(song, "sections")); + if (!sections) { return null; } - const sections = sectionsValue as RehearsalSection[]; const candidates = sections - .map((section, sectionIndex) => ({ section, sectionIndex })) - .filter( - ({ section }) => - isRuntimeObject(section) && - hasSupportedSectionLabel(section) && - hasOwnData(section, "id") && - typeof section.id === "string" && - section.id.trim().length > 0 && - hasBoundedTimeRange(section) - ) - .flatMap(({ section, sectionIndex }) => { + .flatMap((section, sectionIndex) => { + if (!isRuntimeObject(section)) { + return []; + } + const sectionId = ownDataValue(section, "id"); + const sectionLabel = ownDataValue(section, "label"); + const timeRange = ownedBoundedTimeRange(section as RehearsalSection); + if ( + typeof sectionId !== "string" || + sectionId.trim().length === 0 || + typeof sectionLabel !== "string" || + !SECTION_FORM_LABEL_SET.has(sectionLabel) || + timeRange === null + ) { + return []; + } + const holdingRole = pickHoldingRole( - rankedActiveRoles(section).filter((role) => ownedFillPlan(role) !== null) + rankedActiveRoles(section as RehearsalSection).filter( + (metadata) => ownedFillPlan(metadata.role) !== null + ) ); if (!holdingRole) { return []; } - const fillPlan = ownedFillPlan(holdingRole); + const fillPlan = ownedFillPlan(holdingRole.role); if (!fillPlan) { return []; } return [ { - section, + section: section as RehearsalSection, + sectionId, + sectionLabel: sectionLabel as RehearsalSection["label"], sectionIndex, - holdingRole, + holdingRole: holdingRole.role, + holdingRoleId: holdingRole.id, + holdingRoleName: holdingRole.name, fillPlan, - atSeconds: section.timeRange.start + atSeconds: timeRange.start } ]; }) @@ -287,7 +312,7 @@ function resolveSafeFirstFillPlan(song: RehearsalSong): FirstFillPlan | null { 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 7f65226876700cf1563523d127db383fadd05c94 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 15:49:19 -0700 Subject: [PATCH 13/17] fix(workspace): consume fill-plan authority snapshots --- .../workspace/FirstFillPlanCallout.tsx | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx index b8a11d4da..57a89a98b 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx @@ -82,16 +82,15 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { const t = createTranslator(locale); const songIdentity = stableFillPlanSongIdentity(song); const named = useMemo(() => resolveFirstFillPlan(song), [song]); - const namedSectionIndex = named?.sectionIndex ?? -1; const [openedFillPlan, setOpenedFillPlan] = useState(null); useEffect(() => { setOpenedFillPlan(null); }, [ songIdentity, - namedSectionIndex, - named?.section.id, - named?.holdingRole.id, + named?.sectionIndex, + named?.sectionId, + named?.holdingRoleId, named?.fillPlan, named?.atSeconds ]); @@ -114,15 +113,15 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { const opened = openedFillPlan !== null && openedFillPlan.songIdentity === songIdentity && - openedFillPlan.sectionId === named.section.id && - openedFillPlan.sectionIndex === namedSectionIndex && - openedFillPlan.holdingRoleId === named.holdingRole.id && + openedFillPlan.sectionId === named.sectionId && + openedFillPlan.sectionIndex === named.sectionIndex && + openedFillPlan.holdingRoleId === named.holdingRoleId && openedFillPlan.fillPlan === named.fillPlan && openedFillPlan.atSeconds === named.atSeconds; const at = formatFillPlanTime(named.atSeconds); const copyValues: FillPlanCopyValues = { - role: named.holdingRole.name, - section: translateSectionFormLabel(locale, named.section.label), + role: named.holdingRoleName, + section: translateSectionFormLabel(locale, named.sectionLabel), at }; const actionLabel = formatFillPlanCopy(t("firstFillPlanOpenAction"), copyValues); @@ -146,9 +145,9 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { onClick={(event) => { const renderer = resolveFillPlanRenderer(event.currentTarget); const target = - namedSectionIndex >= 0 + named.sectionIndex >= 0 ? (renderer?.querySelector( - `[data-section-index="${namedSectionIndex}"]` + `[data-section-index="${named.sectionIndex}"]` ) ?? null) : null; if (typeof target?.scrollIntoView !== "function") { @@ -160,9 +159,9 @@ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { }); setOpenedFillPlan({ songIdentity, - sectionId: named.section.id, - sectionIndex: namedSectionIndex, - holdingRoleId: named.holdingRole.id, + sectionId: named.sectionId, + sectionIndex: named.sectionIndex, + holdingRoleId: named.holdingRoleId, fillPlan: named.fillPlan, atSeconds: named.atSeconds }); From 38e7d5709faa1e09267a34b8ea7fd37432b8eb22 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 25 Aug 2026 04:18:10 -0700 Subject: [PATCH 14/17] perf(workspace): memoize fill-plan translator --- apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx index 57a89a98b..ea6357fd4 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx @@ -78,8 +78,8 @@ function resolveFillPlanRenderer(origin: HTMLElement): HTMLElement | null { /** Name tonight's first fill plan and open the matching rendered map section. */ export function FirstFillPlanCallout({ song }: FirstFillPlanCalloutProps) { const calloutId = `workspace-surface-fill-plan-${useId()}`; - const locale = detectPreferredLocale(); - const t = createTranslator(locale); + const locale = useMemo(() => detectPreferredLocale(), []); + const t = useMemo(() => createTranslator(locale), [locale]); const songIdentity = stableFillPlanSongIdentity(song); const named = useMemo(() => resolveFirstFillPlan(song), [song]); const [openedFillPlan, setOpenedFillPlan] = useState(null); From a4948df82ad075a0b743e92b985992aa04815565 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Wed, 26 Aug 2026 21:57:10 +0900 Subject: [PATCH 15/17] docs(shared-types): describe RehearsalRole and its fillPlan contract CodeRabbit flagged the exported declaration as lacking descriptive JSDoc per coding guidelines. Comment-only change. --- packages/shared-types/src/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index 1eda7d99d..aad0fd7a6 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -125,7 +125,7 @@ export type ManualOverride = source: "user"; }; -/** Documented. */ +/** An active rehearsal role as surfaced on one section of tonight's map. */ export type RehearsalRole = { id: string; name: string; @@ -139,6 +139,7 @@ export type RehearsalRole = { simplification: string; setupNote: string; transpositionPlan?: string; + /** Optional activity-backed guidance shown for this role in this section; absent when the engine has no fillPlan for it. */ fillPlan?: string; manualOverrides: ManualOverride[]; overlapWarnings: string[]; From 15882a01b3358e60aad4e99bcda217f50cbb9166 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 02:33:00 +0900 Subject: [PATCH 16/17] fix(workspace): keep fill plan contract and navigation stable --- apps/desktop/core/src/lib.rs | 37 ++++++++++++++++++- .../FirstFillPlanCallout.particle.test.tsx | 2 +- ...rstFillPlanCallout.reduced-motion.test.tsx | 2 +- .../workspace/FirstFillPlanCallout.test.tsx | 2 +- .../workspace/FirstFillPlanCallout.tsx | 2 +- ...stFillPlanCallout.workspace-scope.test.tsx | 4 +- .../src/features/workspace/Workspace.tsx | 1 + 7 files changed, 42 insertions(+), 8 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index 200726570..ef30abeb8 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")] + fill_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_fill_plan(parsed); } let payload = serde_json::from_str::(content) @@ -547,7 +549,22 @@ pub fn project_payload_from_content(content: &str) -> Result Result { + for section in &payload.sections { + for role in §ion.roles { + if role.fill_plan.as_ref().is_some_and(|fill_plan| { + fill_plan.trim().is_empty() || fill_plan.contains('\n') || fill_plan.contains('\r') + }) { + return Err("Invalid project file format".to_string()); + } + } + } + Ok(payload) } #[derive(Clone, Debug, Serialize)] @@ -752,6 +769,7 @@ mod tests { "rehearsalPriority": "high", "simplification": "Stay on roots if the chorus entrance gets muddy.", "setupNote": "Keep the attack short so the verse breathes.", + "fillPlan": "Walk eight notes into the chorus downbeat; leave the vocal pickup empty.", "manualOverrides": [], "overlapWarnings": [ "Density warning: competing with Keyboard Left Hand in low register." @@ -784,6 +802,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].fill_plan.as_deref(), + Some("Walk eight notes into the chorus downbeat; leave the vocal pickup empty.") + ); } #[test] @@ -894,6 +916,17 @@ mod tests { assert_eq!(error, "Invalid project file format"); } + #[test] + fn project_payload_from_content_rejects_invalid_fill_plan() { + for fill_plan in ["", " ", "fill here\nthen move", "fill here\rthen move"] { + let mut payload = shared_contract_payload(json!({ "start": 10, "end": 30 })); + payload["sections"][0]["roles"][0]["fillPlan"] = json!(fill_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/FirstFillPlanCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.particle.test.tsx index 8e6fed802..4601c75b5 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.particle.test.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.particle.test.tsx @@ -24,7 +24,7 @@ describe("FirstFillPlanCallout 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/FirstFillPlanCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.reduced-motion.test.tsx index ce19f243b..64fc72c95 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.reduced-motion.test.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.reduced-motion.test.tsx @@ -21,7 +21,7 @@ describe("FirstFillPlanCallout 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/FirstFillPlanCallout.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.test.tsx index 35cff8378..9bc349222 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.test.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.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/FirstFillPlanCallout.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx index ea6357fd4..756667b3b 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.tsx @@ -61,7 +61,7 @@ function preferredFillPlanScrollBehavior(): ScrollBehavior { /** Resolve the song-structure renderer owned by this workspace, failing closed on ambiguous mounts. */ function resolveFillPlanRenderer(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/FirstFillPlanCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstFillPlanCallout.workspace-scope.test.tsx index 1a7f5c8bd..db2d7787f 100644 --- a/apps/desktop/src/features/workspace/FirstFillPlanCallout.workspace-scope.test.tsx +++ b/apps/desktop/src/features/workspace/FirstFillPlanCallout.workspace-scope.test.tsx @@ -13,13 +13,13 @@ describe("FirstFillPlanCallout workspace scope", () => { <>
-
+
-
+
diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index c9f9af436..7953bee41 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 a70896b11b9d93e6d50b555473cc3b9c183dc07f Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 11:53:09 +0900 Subject: [PATCH 17/17] fix(contract): align Unicode fill validation --- apps/desktop/core/src/lib.rs | 61 +++++++++++++++++-- .../features/workspace/firstFillPlan.test.ts | 9 +++ .../src/features/workspace/firstFillPlan.ts | 5 +- packages/shared-types/src/index.ts | 44 ++++++++++++- packages/shared-types/test/index.test.ts | 24 ++++++++ 5 files changed, 136 insertions(+), 7 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index ef30abeb8..82baa3d20 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -554,12 +554,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 fill guidance without normalizing user text. +fn is_valid_fill_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_fill_plan(payload: RehearsalSongPayload) -> Result { for section in &payload.sections { for role in §ion.roles { - if role.fill_plan.as_ref().is_some_and(|fill_plan| { - fill_plan.trim().is_empty() || fill_plan.contains('\n') || fill_plan.contains('\r') - }) { + if role + .fill_plan + .as_deref() + .is_some_and(|fill_plan| !is_valid_fill_plan(fill_plan)) + { return Err("Invalid project file format".to_string()); } } @@ -918,13 +956,28 @@ mod tests { #[test] fn project_payload_from_content_rejects_invalid_fill_plan() { - for fill_plan in ["", " ", "fill here\nthen move", "fill here\rthen move"] { + for fill_plan in [ + "", + " ", + "\u{FEFF}", + "\u{0085}", + "fill here\nthen move", + "fill here\rthen move", + "fill here\u{0085}then move", + "fill here\u{2028}then move", + "fill here\u{2029}then move", + ] { let mut payload = shared_contract_payload(json!({ "start": 10, "end": 30 })); payload["sections"][0]["roles"][0]["fillPlan"] = json!(fill_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]["fillPlan"] = json!("\u{FEFF}Fill the string\u{FEFF}"); + let content = serde_json::to_string(&payload).expect("payload should serialize"); + assert!(project_payload_from_content(&content).is_ok()); } #[test] diff --git a/apps/desktop/src/features/workspace/firstFillPlan.test.ts b/apps/desktop/src/features/workspace/firstFillPlan.test.ts index ba0a8f020..17eb77744 100644 --- a/apps/desktop/src/features/workspace/firstFillPlan.test.ts +++ b/apps/desktop/src/features/workspace/firstFillPlan.test.ts @@ -131,6 +131,15 @@ describe("resolveFirstFillPlan", () => { ).toBeNull(); }); + it("skips Unicode line separators and accepts BOM-padded fill text", () => { + for (const fillPlan of ["Fill\u0085here", "Fill\u2028here", "Fill\u2029here"]) { + expect(resolveFirstFillPlan(withFillSection({ fillPlan }))).toBeNull(); + } + expect(resolveFirstFillPlan(withFillSection({ fillPlan: "\uFEFF Fill here \uFEFF" }))?.fillPlan).toBe( + "Fill here" + ); + }); + it("prefers the earlier of two fill plans", () => { const song = withFillSection({ id: "verse-late", diff --git a/apps/desktop/src/features/workspace/firstFillPlan.ts b/apps/desktop/src/features/workspace/firstFillPlan.ts index 5c48aa4bf..6cfea5d3c 100644 --- a/apps/desktop/src/features/workspace/firstFillPlan.ts +++ b/apps/desktop/src/features/workspace/firstFillPlan.ts @@ -1,6 +1,7 @@ import { MAX_SECTION_TIME_SECONDS, SECTION_FORM_LABELS, + isNonEmptySingleLineText, type RehearsalRole, type RehearsalSection, type RehearsalSong @@ -117,10 +118,10 @@ function ownedFillPlan(role: unknown): string | null { if (typeof fillPlan !== "string") { return null; } - const trimmed = fillPlan.trim(); - if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + if (!isNonEmptySingleLineText(fillPlan)) { return null; } + const trimmed = fillPlan.trim(); return truncateCodePoints(trimmed, MAX_FILL_PLAN_CHARACTERS); } diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index aad0fd7a6..742ba6b90 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -409,6 +409,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}'`; @@ -1556,7 +1598,7 @@ function validateRehearsalRole(value: unknown, path: string): string | null { if (value.transpositionPlan !== undefined && typeof value.transpositionPlan !== "string") { return invalidField(`${path}.transpositionPlan`); } - if (value.fillPlan !== undefined && typeof value.fillPlan !== "string") { + if (value.fillPlan !== undefined && !isNonEmptySingleLineText(value.fillPlan)) { return invalidField(`${path}.fillPlan`); } if (!isDenseArray(value.manualOverrides)) { diff --git a/packages/shared-types/test/index.test.ts b/packages/shared-types/test/index.test.ts index e706dfd8b..9d860c385 100644 --- a/packages/shared-types/test/index.test.ts +++ b/packages/shared-types/test/index.test.ts @@ -764,6 +764,30 @@ describe("shared type helpers", () => { expect(second.collaboration?.assignments).toHaveLength(2); }); + it("keeps optional fill plans aligned with Rust project loading", () => { + for (const fillPlan of [ + "", + " ", + "\uFEFF", + "\u0085", + "fill here\nthen move", + "fill here\rthen move", + "fill here\u0085then move", + "fill here\u2028then move", + "fill here\u2029then move" + ]) { + const song = createDemoRehearsalSong(); + song.sections[0]!.roles[0]!.fillPlan = fillPlan; + + expect(isRehearsalSong(song)).toBe(false); + expect(() => parseRehearsalSong(song)).toThrow("sections[0].roles[0].fillPlan"); + } + + const paddedPlan = createDemoRehearsalSong(); + paddedPlan.sections[0]!.roles[0]!.fillPlan = "\uFEFF Fill the string \uFEFF"; + expect(isRehearsalSong(paddedPlan)).toBe(true); + }); + it("validates and parses rehearsal song payloads", () => { const song = createDemoRehearsalSong(); const malformedSong = createDemoRehearsalSong() as unknown as {