From ea7d685990126d48752df4b6197493f7303c927c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 17:16:12 +0000 Subject: [PATCH 01/25] feat(workspace): name tonight's first harmonic explanation on the map Name the owning part, owned harmonicExplanation copy, labeled section, and time so the room can play from that reason before rehearsal. Open scrolls the renderer-owned map section. Do not invent the copy from function labels, groove, cues, setup, simplification, overlap, range, chords, confirmed overrides, or confidence notes. --- AGENTS.md | 1 + ARCHITECTURE.md | 3 +- CHANGELOG.md | 1 + CLAUDE.md | 2 +- ...rmonicExplanationCallout.particle.test.tsx | 52 ++++ ...ExplanationCallout.reduced-motion.test.tsx | 43 +++ .../FirstHarmonicExplanationCallout.test.tsx | 268 +++++++++++++++++ .../FirstHarmonicExplanationCallout.tsx | 169 +++++++++++ .../src/features/workspace/Workspace.test.tsx | 32 +- .../src/features/workspace/Workspace.tsx | 11 +- ...onicExplanation.inherited-metadata.test.ts | 94 ++++++ .../firstHarmonicExplanation.test.ts | 262 +++++++++++++++++ .../workspace/firstHarmonicExplanation.ts | 274 ++++++++++++++++++ apps/desktop/src/i18n/index.test.ts | 51 +++- apps/desktop/src/i18n/index.ts | 36 ++- apps/desktop/src/locales/en/common.json | 7 +- apps/desktop/src/locales/ko/common.json | 7 +- docs/design-system/component-contract.md | 1 + ...n-first-harmonic-explanation-navigation.md | 5 + 19 files changed, 1310 insertions(+), 9 deletions(-) create mode 100644 apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.particle.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.reduced-motion.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx create mode 100644 apps/desktop/src/features/workspace/firstHarmonicExplanation.inherited-metadata.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHarmonicExplanation.test.ts create mode 100644 apps/desktop/src/features/workspace/firstHarmonicExplanation.ts create mode 100644 docs/doctoring/reduced-motion-first-harmonic-explanation-navigation.md diff --git a/AGENTS.md b/AGENTS.md index fca448ce9..7747d2067 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 harmonic explanation with the owning part when an active role is corroborated, the owned `harmonicExplanation` copy, the labeled section, and the time so the next action is obvious. Do not invent that copy from groove, cue, setup, simplification, overlap, range, chord labels, function labels, confirmed overrides, 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..9e1e4d081 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 harmonic explanation must name the owning part when corroborated, the owned `harmonicExplanation` 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, setup, simplification, overlap, range, chord labels, function labels, confirmed overrides, or confidence notes. - 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..8516bfac6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first harmonic explanation in the mounted rehearsal workspace so the room can play from the owned harmony reason 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..30e80e052 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 harmonic explanation and opens the matching rendered map section. Do not invent that copy from groove, cue, setup, simplification, overlap, range, chord labels, function labels, confirmed overrides, or confidence notes. `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/FirstHarmonicExplanationCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.particle.test.tsx new file mode 100644 index 000000000..7b4f165a9 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.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 { FirstHarmonicExplanationCallout } from "./FirstHarmonicExplanationCallout"; + +describe("FirstHarmonicExplanationCallout Korean role copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending dynamic role names particle-safe before and after the explanation 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", + harmonicExplanation: "The bass holds the vi center." + } + ]; + 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/FirstHarmonicExplanationCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.reduced-motion.test.tsx new file mode 100644 index 000000000..3d94b5e0e --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.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 { FirstHarmonicExplanationCallout } from "./FirstHarmonicExplanationCallout"; + +describe("FirstHarmonicExplanationCallout 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 explanation at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.test.tsx b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.test.tsx new file mode 100644 index 000000000..dac0d6803 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.test.tsx @@ -0,0 +1,268 @@ +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 { FirstHarmonicExplanationCallout } from "./FirstHarmonicExplanationCallout"; + +const DEMO_EXPLANATION = + "The bass holds the vi center so the rest of the section can lean into the pickup without losing the tonal floor."; + +function songWithHarmonicExplanation() { + 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("FirstHarmonicExplanationCallout", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText( + "Nothing still has a harmonic explanation. Stay on tonight's map until a part owns rehearsal-facing harmony copy." + ) + ).toBeTruthy(); + }); + + it("contains a hostile song identity accessor instead of crashing the callout", () => { + const song = songWithHarmonicExplanation(); + 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 explanation at 0:10" })).toBeTruthy(); + }); + + it("resets armed guidance when accessor-id songs change with the same explanation signature", () => { + const firstSong = songWithHarmonicExplanation(); + const nextSong = songWithHarmonicExplanation(); + 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 explanation at 0:10" })); + expect( + screen.getByText(/Play from that explanation on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect( + screen.getByText("Bass Guitar still has a harmonic explanation in the verse at 0:10.") + ).toBeTruthy(); + expect( + screen.queryByText(/Play from that explanation 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 = songWithHarmonicExplanation(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar explanation at 0:10" })); + expect( + screen.getByText(/Play from that explanation on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + rerender(); + + expect( + screen.getByText(/Play from that explanation on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + expect( + screen.queryByText("Bass Guitar still has a harmonic explanation in the verse at 0:10.") + ).toBeNull(); + + grid.remove(); + }); + + it("does not show another part's explanation under the named holding part", () => { + const song = songWithHarmonicExplanation(); + song.sections[0]!.roles[0]!.harmonicExplanation = ""; + song.sections[0]!.roles[0]!.rehearsalPriority = "low"; + song.sections[0]!.roles[1]!.harmonicExplanation = "Check the keyboard voicing instead."; + song.sections[0]!.roles[2]!.harmonicExplanation = "The ninth is the reason this lift works."; + + render(); + + expect( + screen.getByText("Keyboard 1 Right Hand still has a harmonic explanation in the verse at 0:10.") + ).toBeTruthy(); + expect(screen.getByText("Check the keyboard voicing instead.")).toBeTruthy(); + expect(screen.queryByText("The ninth is the reason this lift works.")).toBeNull(); + expect(screen.queryByText(DEMO_EXPLANATION)).toBeNull(); + }); + + it("names the first harmonic explanation as map navigation, scrolls to its rendered section, and arms that action", () => { + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + expect(screen.getByText(DEMO_EXPLANATION)).toBeTruthy(); + const action = screen.getByRole("button", { + name: "Open Bass Guitar explanation at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Play from that explanation 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 explanation at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Play from that explanation 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 explanation at 0:10" })); + + expect( + screen.getByText("Bass Guitar still has a harmonic explanation in the verse at 0:10.") + ).toBeTruthy(); + expect( + screen.queryByText(/Play from that explanation 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 = songWithHarmonicExplanation(); + song.sections[0]!.id = "analysis section / duplicate"; + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar explanation 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 explanation 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 harmonic explanation changes or returns later", () => { + const initialSong = songWithHarmonicExplanation(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + fireEvent.click(screen.getByRole("button", { name: "Open Bass Guitar explanation at 0:10" })); + expect( + screen.getByText(/Play from that explanation on Bass Guitar at 0:10 before the room starts./) + ).toBeTruthy(); + + const nextSong = songWithHarmonicExplanation(); + nextSong.id = "next-song"; + nextSong.sections[0]!.timeRange = { start: 20, end: 40 }; + rerender(); + expect( + screen.getByText("Bass Guitar still has a harmonic explanation in the verse at 0:20.") + ).toBeTruthy(); + + grid.remove(); + }); + + it("keeps an unavailable harmonic explanation guidance-only", () => { + const song = songWithHarmonicExplanation(); + for (const role of song.sections[0]!.roles) { + delete role.harmonicExplanation; + } + render(); + expect(screen.queryByRole("button")).toBeNull(); + expect( + screen.getByText( + "Nothing still has a harmonic explanation. Stay on tonight's map until a part owns rehearsal-facing harmony copy." + ) + ).toBeTruthy(); + }); + + it("localizes the harmonic-explanation form label instead of exposing its raw enum in Korean copy", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithHarmonicExplanation(); + song.sections[0]!.roles[0]!.name = "베이스"; + + render(); + + expect(screen.getByText("0:10 벌스에서 베이스 파트의 화성 설명이 있습니다.")).toBeTruthy(); + expect(screen.queryByText(/verse에서/)).toBeNull(); + }); + + it("renders the owned explanation as a text node instead of template syntax", () => { + const song = songWithHarmonicExplanation(); + song.sections[0]!.roles[1]!.harmonicExplanation = undefined; + song.sections[0]!.roles[2]!.harmonicExplanation = undefined; + song.sections[0]!.roles[0]!.harmonicExplanation = "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/FirstHarmonicExplanationCallout.tsx b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx new file mode 100644 index 000000000..2f309f56d --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx @@ -0,0 +1,169 @@ +import { useEffect, useState } from "react"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { Button } from "@/components/ui/button"; +import { + createTranslator, + detectPreferredLocale, + translateSectionFormLabel +} from "../../i18n"; +import { + formatHarmonicExplanationTime, + resolveFirstHarmonicExplanation +} from "./firstHarmonicExplanation"; + +/** Props for the first harmonic-explanation rehearsal callout. */ +export interface FirstHarmonicExplanationCalloutProps { + song: RehearsalSong; +} + +type HarmonicExplanationCopyValues = Readonly>; + +type OpenedHarmonicExplanation = Readonly<{ + songIdentity: unknown; + sectionId: string; + sectionIndex: number; + holdingRoleId: string; + explanation: string; + atSeconds: number; +}>; + +/** Read a stable owned song id, falling back to object identity for untrusted identity metadata. */ +function stableHarmonicExplanationSongIdentity(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 harmonic-explanation placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatHarmonicExplanationCopy( + template: string, + values: HarmonicExplanationCopyValues +): string { + return template.replace(/\{(role|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof HarmonicExplanationCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredHarmonicExplanationScrollBehavior(): ScrollBehavior { + return typeof window.matchMedia === "function" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches + ? "auto" + : "smooth"; +} + +/** Name tonight's first harmonic explanation and open the matching rendered map section. */ +export function FirstHarmonicExplanationCallout({ song }: FirstHarmonicExplanationCalloutProps) { + const locale = detectPreferredLocale(); + const t = createTranslator(locale); + const songIdentity = stableHarmonicExplanationSongIdentity(song); + const runtimeSong = song as unknown as Partial | null; + const named = resolveFirstHarmonicExplanation(song); + const namedSectionIndex = + named && Array.isArray(runtimeSong?.sections) + ? runtimeSong.sections.indexOf(named.section) + : -1; + const [openedHarmonicExplanation, setOpenedHarmonicExplanation] = + useState(null); + + useEffect(() => { + setOpenedHarmonicExplanation(null); + }, [ + songIdentity, + namedSectionIndex, + named?.section.id, + named?.holdingRole.id, + named?.explanation, + named?.atSeconds + ]); + + if (!named) { + return ( + + ); + } + + const opened = + openedHarmonicExplanation !== null && + openedHarmonicExplanation.songIdentity === songIdentity && + openedHarmonicExplanation.sectionId === named.section.id && + openedHarmonicExplanation.sectionIndex === namedSectionIndex && + openedHarmonicExplanation.holdingRoleId === named.holdingRole.id && + openedHarmonicExplanation.explanation === named.explanation && + openedHarmonicExplanation.atSeconds === named.atSeconds; + const at = formatHarmonicExplanationTime(named.atSeconds); + const copyValues: HarmonicExplanationCopyValues = { + role: named.holdingRole.name, + section: translateSectionFormLabel(locale, named.section.label), + at + }; + const actionLabel = formatHarmonicExplanationCopy(t("firstHarmonicExplanationOpenAction"), copyValues); + const body = formatHarmonicExplanationCopy(t("firstHarmonicExplanationBody"), copyValues); + const armed = formatHarmonicExplanationCopy(t("firstHarmonicExplanationArmed"), copyValues); + + return ( + + ); +} diff --git a/apps/desktop/src/features/workspace/Workspace.test.tsx b/apps/desktop/src/features/workspace/Workspace.test.tsx index a3da5ffe6..b78a544fc 100644 --- a/apps/desktop/src/features/workspace/Workspace.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.test.tsx @@ -134,7 +134,7 @@ describe("Workspace", () => { fireEvent.click(screen.getByRole("tab", { name: "Bass Guitar" })); - expect(screen.getByText(/The bass holds the vi center/i)).toBeTruthy(); + expect(screen.getAllByText(/The bass holds the vi center/i).length).toBeGreaterThan(0); expect(screen.getByText(/whole step lower/i)).toBeTruthy(); expect(screen.getByText(/Lock the bass entrance against the pickup/i)).toBeTruthy(); expect(screen.getByText(/Verse harmony pass/i)).toBeTruthy(); @@ -270,4 +270,34 @@ describe("Workspace", () => { expect(screen.getByText("합주 우선순위")).toBeTruthy(); expect(screen.getByText("역할과 화성")).toBeTruthy(); }); + + it("names tonight's first harmonic explanation 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( + "The bass holds the vi center so the rest of the section can lean into the pickup without losing the tonal floor." + ).length + ).toBeGreaterThan(0); + const action = screen.getByRole("button", { + name: "Open Bass Guitar explanation at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Play from that explanation 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..00c1aa0f9 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 { FirstHarmonicExplanationCallout } from "./FirstHarmonicExplanationCallout"; 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/firstHarmonicExplanation.inherited-metadata.test.ts b/apps/desktop/src/features/workspace/firstHarmonicExplanation.inherited-metadata.test.ts new file mode 100644 index 000000000..d342ec3b4 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHarmonicExplanation.inherited-metadata.test.ts @@ -0,0 +1,94 @@ +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstHarmonicExplanation } from "./firstHarmonicExplanation"; + +function songWithHarmonicExplanation() { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + section.id = "explained-own"; + section.roles = [ + { + ...section.roles[0]!, + id: "bass-guitar", + name: "Bass Guitar", + rehearsalPriority: "high", + harmonicExplanation: "The bass holds the vi center." + } + ]; + section.partGraph = [{ role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] }]; + song.sections = [section]; + return { song, section }; +} + +describe("resolveFirstHarmonicExplanation inherited metadata", () => { + it("rejects a song or section whose required metadata is inherited", () => { + const { song, section } = songWithHarmonicExplanation(); + const inheritedSong = Object.create({ sections: song.sections }) as typeof song; + expect(resolveFirstHarmonicExplanation(inheritedSong)).toBeNull(); + + const inheritedSection = Object.create(section) as typeof section; + song.sections = [inheritedSection]; + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("rejects inherited timing fields", () => { + const { song, section } = songWithHarmonicExplanation(); + section.timeRange = Object.create({ start: 10, end: 30 }) as typeof section.timeRange; + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("contains exceptions from own runtime accessors instead of trusting them", () => { + const { song, section } = songWithHarmonicExplanation(); + Object.defineProperty(section.roles[0]!, "harmonicExplanation", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile harmonicExplanation getter"); + } + }); + + expect(() => resolveFirstHarmonicExplanation(song)).not.toThrow(); + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("does not treat own accessors as stable explanation identity authority", () => { + const { song, section } = songWithHarmonicExplanation(); + Object.defineProperty(section, "id", { + configurable: true, + enumerable: true, + get() { + return "explained-own"; + } + }); + + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("does not let inherited explanations establish the named copy", () => { + const { song, section } = songWithHarmonicExplanation(); + const inheritedRole = Object.create({ + harmonicExplanation: "Inherited harmonic explanation" + }) 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(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("does not let inherited role or graph metadata establish the holding part", () => { + const { song, section } = songWithHarmonicExplanation(); + const node = section.partGraph[0]!; + section.partGraph = [Object.create(node) as typeof node]; + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("rejects arrays masquerading as section records", () => { + const { song, section } = songWithHarmonicExplanation(); + const arraySection = Object.assign([], section) as unknown as typeof section; + song.sections = [arraySection]; + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHarmonicExplanation.test.ts b/apps/desktop/src/features/workspace/firstHarmonicExplanation.test.ts new file mode 100644 index 000000000..c1d3d3b55 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHarmonicExplanation.test.ts @@ -0,0 +1,262 @@ +import { describe, expect, it } from "vitest"; +import { MAX_SECTION_TIME_SECONDS, createDemoRehearsalSong } from "@bandscope/shared-types"; +import { formatHarmonicExplanationTime, resolveFirstHarmonicExplanation } from "./firstHarmonicExplanation"; + +function withExplainedSection( + overrides: { + id?: string; + start?: number; + end?: number; + explanation?: 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-explained"; + 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: + overrides.explanation ?? + "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." + }, + manualOverrides: [] + } + ]; + section.partGraph = [ + { + role_id: roleId, + is_active: overrides.isActive ?? true, + handoff_to: [], + handoff_from: [] + } + ]; + song.sections = [section]; + return song; +} + +describe("resolveFirstHarmonicExplanation", () => { + it("picks the demo song's earliest high-priority explanation and the part that owns it", () => { + const resolved = resolveFirstHarmonicExplanation(createDemoRehearsalSong()); + expect(resolved?.section.id).toBe("verse-1"); + expect(resolved?.holdingRole.id).toBe("bass-guitar"); + expect(resolved?.explanation).toBe( + "The bass holds the vi center so the rest of the section can lean into the pickup without losing the tonal floor." + ); + expect(resolved?.atSeconds).toBe(10); + expect(formatHarmonicExplanationTime(resolved?.atSeconds ?? -1)).toBe("0:10"); + expect(formatHarmonicExplanationTime(Number.NaN)).toBe("0:00"); + expect(formatHarmonicExplanationTime(-4)).toBe("0:00"); + }); + + it("does not invent an explanation from function labels, groove, cue, setup, simplification, overlap, range, confidence, or confirmed chords", () => { + const song = withExplainedSection(); + delete song.sections[0]!.roles[0]!.harmonicExplanation; + song.sections[0]!.groove = "Straight eighths with a late snare feel"; + song.sections[0]!.roles[0]!.setupNote = "Watch the breath before the last line of the verse."; + song.sections[0]!.roles[0]!.simplification = "Keep the sustained note centered."; + 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]!.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: "The ninth is the reason this lift works." + }; + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("skips a blank harmonic explanation", () => { + expect(resolveFirstHarmonicExplanation(withExplainedSection({ explanation: " " }))).toBeNull(); + }); + + it("prefers the earlier of two harmonic explanations", () => { + const song = withExplainedSection({ + id: "verse-late", + start: 40, + end: 56, + roleId: "keys-right", + explanation: "Late keyboard color." + }); + const earlier = structuredClone(song.sections[0]!); + earlier.id = "verse-early"; + earlier.roles = [ + { + ...earlier.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "low", + harmonicExplanation: "Earlier vocal lift." + } + ]; + 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 = resolveFirstHarmonicExplanation(song); + expect(resolved?.section.id).toBe("verse-early"); + expect(resolved?.holdingRole.id).toBe("lead-vocal"); + expect(resolved?.explanation).toBe("Earlier vocal lift."); + expect(resolved?.atSeconds).toBe(8); + }); + + it("breaks same-time explanation ties with locale-independent id ordering", () => { + const song = withExplainedSection({ id: "ä-explained", start: 10, end: 26 }); + const ascii = structuredClone(song.sections[0]!); + ascii.id = "z-explained"; + song.sections = [song.sections[0]!, ascii]; + + expect(resolveFirstHarmonicExplanation(song)?.section.id).toBe("z-explained"); + }); + + it("prefers a high-priority explained part over a low-priority part in the same section", () => { + const song = withExplainedSection({ + roleId: "keys-right", + roleName: "Keys", + priority: "low", + explanation: "Low-priority color." + }); + const section = song.sections[0]!; + const highRole = { + ...section.roles[0]!, + id: "lead-vocal", + name: "Lead Vocal", + rehearsalPriority: "high" as const, + harmonicExplanation: "High-priority vocal lift." + }; + 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(resolveFirstHarmonicExplanation(song)?.holdingRole.id).toBe("lead-vocal"); + expect(resolveFirstHarmonicExplanation(song)?.explanation).toBe("High-priority vocal lift."); + }); + + it("breaks equal-priority role ties with locale-independent id ordering", () => { + const song = withExplainedSection({ roleId: "ä-role", roleName: "Umlaut role", priority: "high" }); + const section = song.sections[0]!; + const asciiRole = { + ...section.roles[0]!, + id: "z-role", + name: "ASCII role", + harmonicExplanation: "ASCII explanation." + }; + 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(resolveFirstHarmonicExplanation(song)?.holdingRole.id).toBe("z-role"); + expect(resolveFirstHarmonicExplanation(song)?.explanation).toBe("ASCII explanation."); + }); + + it("skips an explanation whose graph node is inactive", () => { + expect(resolveFirstHarmonicExplanation(withExplainedSection({ isActive: false }))).toBeNull(); + }); + + it("skips an explanation whose rehearsal window is unbounded", () => { + expect(resolveFirstHarmonicExplanation(withExplainedSection({ start: Number.NaN, end: 30 }))).toBeNull(); + }); + + it("skips an explanation whose end precedes its start", () => { + expect(resolveFirstHarmonicExplanation(withExplainedSection({ start: 30, end: 10 }))).toBeNull(); + }); + + it("skips a zero-length explanation window", () => { + expect(resolveFirstHarmonicExplanation(withExplainedSection({ start: 10, end: 10 }))).toBeNull(); + }); + + it("skips an explanation whose endpoint overflows the shared timing bound", () => { + expect( + resolveFirstHarmonicExplanation( + withExplainedSection({ + start: MAX_SECTION_TIME_SECONDS, + end: MAX_SECTION_TIME_SECONDS + 1 + }) + ) + ).toBeNull(); + }); + + it("returns null for a non-object song root", () => { + expect(resolveFirstHarmonicExplanation(null as never)).toBeNull(); + }); + + it("returns null when the runtime section collection is sparse", () => { + const song = withExplainedSection(); + const sparseSections: typeof song.sections = new Array(2); + sparseSections[1] = song.sections[0]!; + song.sections = sparseSections; + expect(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("keeps the explanation unnamed when role identities are duplicated", () => { + const song = withExplainedSection(); + 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(resolveFirstHarmonicExplanation(song)).toBeNull(); + }); + + it("bounds the explanation to 180 Unicode code points", () => { + const song = withExplainedSection({ explanation: `${"G".repeat(200)}` }); + const resolved = resolveFirstHarmonicExplanation(song); + expect(resolved?.explanation.length).toBe(180); + }); + + it("does not split a Unicode surrogate pair at the explanation boundary", () => { + const song = withExplainedSection({ explanation: `${"a".repeat(179)}😀tail` }); + const resolved = resolveFirstHarmonicExplanation(song); + expect(Array.from(resolved?.explanation ?? "")).toHaveLength(180); + expect(resolved?.explanation.endsWith("😀")).toBe(true); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstHarmonicExplanation.ts b/apps/desktop/src/features/workspace/firstHarmonicExplanation.ts new file mode 100644 index 000000000..278707cc4 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstHarmonicExplanation.ts @@ -0,0 +1,274 @@ +import { + MAX_SECTION_TIME_SECONDS, + type RehearsalRole, + type RehearsalSection, + type RehearsalSong +} from "@bandscope/shared-types"; + +const PRIORITY_RANK = { high: 0, medium: 1, low: 2 } as const; +const MAX_HARMONIC_EXPLANATION_CHARACTERS = 180; + +/** Tonight's first harmonic explanation: the earliest labeled section and the part that owns it. */ +export type FirstHarmonicExplanation = { + section: RehearsalSection; + holdingRole: RehearsalRole; + explanation: string; + atSeconds: number; +}; + +/** Format a non-negative harmonic-explanation time as m:ss for rehearsal copy. */ +export function formatHarmonicExplanationTime(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 harmonic explanation, or null when it cannot be shown. */ +function ownedHarmonicExplanation(role: unknown): string | null { + if (!isRuntimeObject(role) || !hasOwnData(role, "harmonicExplanation")) { + return null; + } + const explanation = (role as { harmonicExplanation?: unknown }).harmonicExplanation; + if (typeof explanation !== "string") { + return null; + } + const trimmed = explanation.trim(); + if (trimmed.length === 0) { + return null; + } + return truncateCodePoints(trimmed, MAX_HARMONIC_EXPLANATION_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 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 harmonic explanation after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstHarmonicExplanation(song: RehearsalSong): FirstHarmonicExplanation | null { + if (!isRuntimeObject(song) || !hasOwnData(song, "sections") || !isDenseRuntimeArray(song.sections)) { + return null; + } + + const candidates = song.sections + .filter( + (section) => + isRuntimeObject(section) && + hasOwnData(section, "label") && + typeof section.label === "string" && + section.label.trim().length > 0 && + hasOwnData(section, "id") && + typeof section.id === "string" && + section.id.trim().length > 0 && + hasBoundedTimeRange(section) + ) + .flatMap((section) => { + const holdingRole = pickHoldingRole( + rankedActiveRoles(section).filter((role) => ownedHarmonicExplanation(role) !== null) + ); + if (!holdingRole) { + return []; + } + const explanation = ownedHarmonicExplanation(holdingRole); + if (!explanation) { + return []; + } + return [ + { + section, + holdingRole, + explanation, + 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 harmonic explanation, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstHarmonicExplanation(song: RehearsalSong): FirstHarmonicExplanation | null { + try { + return resolveSafeFirstHarmonicExplanation(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..4c01eafd8 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,53 @@ describe("i18n", () => { } }); }); + + describe("translateSectionFormLabel", () => { + it("localizes every supported Korean section form label", () => { + expect( + [ + "intro", + "verse", + "pre-chorus", + "chorus", + "bridge", + "outro", + "tag", + "pickup", + "stop", + "handoff" + ].map((label) => translateSectionFormLabel("ko", label as never)) + ).toEqual([ + "인트로", + "벌스", + "프리코러스", + "코러스", + "브리지", + "아웃트로", + "태그", + "픽업", + "스톱", + "핸드오프" + ]); + }); + + it("preserves every supported English section form label", () => { + expect(translateSectionFormLabel("en", "verse")).toBe("verse"); + expect(translateSectionFormLabel("en", "outro")).toBe("outro"); + }); + + it("does not treat inherited object keys as localized section labels", () => { + const inheritedKey = "toString" as never; + expect(translateSectionFormLabel("ko", inheritedKey)).toBe("toString"); + }); + + it("keeps Korean first-harmonic-explanation next-action copy particle-safe", () => { + const t = createTranslator("ko"); + expect(t("firstHarmonicExplanationOpenAction")).toBe("{at} {role} 화성 설명 위치 열기"); + expect(t("firstHarmonicExplanationBody")).toBe("{at} {section}에서 {role} 파트의 화성 설명이 있습니다."); + expect(t("firstHarmonicExplanationArmed")).toBe( + "{at}에서 {role} 파트의 화성 설명을 확인한 다음 합주를 시작하세요." + ); + }); + }); }); diff --git a/apps/desktop/src/i18n/index.ts b/apps/desktop/src/i18n/index.ts index 1a9f471f0..352eff65e 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 locale-aware translation lookup that falls back to English copy. */ export function createTranslator(locale: Locale = "en") { return function t(key: TranslationKey): string { return dictionaries[locale][key] ?? dictionaries.en[key]; }; } +/** Return the localized display label for a supported rehearsal section form. */ +export function translateSectionFormLabel(locale: Locale, label: SectionFormLabel): string { + const labels = sectionFormLabels[locale] as Readonly>; + return Object.prototype.hasOwnProperty.call(labels, label) ? labels[label] : String(label); +} + /** Documented. */ export function detectPreferredLocale(): Locale { if (typeof navigator !== "undefined" && navigator.language?.toLowerCase().startsWith("ko")) { diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index 39f716d50..722150fe3 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", + "firstHarmonicExplanationLabel": "Tonight's first harmonic explanation", + "firstHarmonicExplanationOpenAction": "Open {role} explanation at {at}", + "firstHarmonicExplanationBody": "{role} still has a harmonic explanation in the {section} at {at}.", + "firstHarmonicExplanationArmed": "Play from that explanation on {role} at {at} before the room starts.", + "firstHarmonicExplanationUnavailable": "Nothing still has a harmonic explanation. Stay on tonight's map until a part owns rehearsal-facing harmony copy." } diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 371884abb..274e4141c 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": "진척도 증가", + "firstHarmonicExplanationLabel": "오늘 첫 화성 설명", + "firstHarmonicExplanationOpenAction": "{at} {role} 화성 설명 위치 열기", + "firstHarmonicExplanationBody": "{at} {section}에서 {role} 파트의 화성 설명이 있습니다.", + "firstHarmonicExplanationArmed": "{at}에서 {role} 파트의 화성 설명을 확인한 다음 합주를 시작하세요.", + "firstHarmonicExplanationUnavailable": "아직 화성 설명이 없습니다. 파트가 합주용 화성 설명을 가질 때까지 오늘 지도에 머무르세요." } diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 22602c313..d185ab136 100644 --- a/docs/design-system/component-contract.md +++ b/docs/design-system/component-contract.md @@ -32,6 +32,7 @@ The authoritative Figma view is `31 Component Contract Catalog`. This file mirro | 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. | | 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. | +| First Harmonic Explanation Callout | workspace next-action pattern | `apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx` | Name the owning part when an active graph node corroborates it, the owned `harmonicExplanation` copy, the labeled section start, and the time. Do not invent that copy from `groove`, cue text, `setupNote`, `simplification`, overlap warnings, range copy, `harmony.chord`, `harmony.functionLabel`, confirmed overrides, or confidence notes. Open scrolls the renderer-owned song-structure section. Keep the unavailable state guidance-only. Distinct from first-confirmed-harmony, first-ear-check, first-overlap, first-groove, and first-form-label work. | | 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. | | Export Action Group | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-731 | `apps/desktop/src/features/workspace/Workspace.tsx` | Feature-local export buttons call `handleExportCueSheet`, `handleExportChart`, and `handleExportHandoff`. | | Workspace State Matrix | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=99-560 | `apps/desktop/src/features/workspace/WorkspaceStates.tsx`, `apps/desktop/src/App.tsx` | Whole-workspace empty, loading, error, and ready state routing; use before changing `renderWorkspaceState()`. | diff --git a/docs/doctoring/reduced-motion-first-harmonic-explanation-navigation.md b/docs/doctoring/reduced-motion-first-harmonic-explanation-navigation.md new file mode 100644 index 000000000..ebea19f1a --- /dev/null +++ b/docs/doctoring/reduced-motion-first-harmonic-explanation-navigation.md @@ -0,0 +1,5 @@ +# Reduced-motion first harmonic explanation navigation + +When `prefers-reduced-motion: reduce` matches, `FirstHarmonicExplanationCallout` scrolls the renderer-owned song-structure section with `behavior: "auto"`. Otherwise it uses `behavior: "smooth"`. + +Open still names the owning part, labeled section, and time. Analysis `section.id` is never DOM-ID authority. From 61151a2e4f59420a25fadb8e4b60f57891ff245c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 11:10:04 -0700 Subject: [PATCH 02/25] test(workspace): scope harmonic explanation navigation --- ...xplanationCallout.workspace-scope.test.tsx | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.workspace-scope.test.tsx diff --git a/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.workspace-scope.test.tsx b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.workspace-scope.test.tsx new file mode 100644 index 000000000..98bcf3b5f --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.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 { FirstHarmonicExplanationCallout } from "./FirstHarmonicExplanationCallout"; + +describe("FirstHarmonicExplanationCallout 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 explanation at 0:10" + }); + expect(actions).toHaveLength(2); + fireEvent.click(actions[1]!); + + expect(firstScrollIntoView).not.toHaveBeenCalled(); + expect(secondScrollIntoView).toHaveBeenCalledWith({ + block: "nearest", + behavior: "smooth" + }); + }); +}); From f179bac33aae78d8c0d9df7d1b993843d59fe321 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 11:10:42 -0700 Subject: [PATCH 03/25] fix(workspace): scope harmonic explanation navigation --- .../FirstHarmonicExplanationCallout.tsx | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx index 2f309f56d..1400ffaa7 100644 --- a/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstHarmonicExplanationCallout.tsx @@ -65,6 +65,22 @@ function preferredHarmonicExplanationScrollBehavior(): ScrollBehavior { : "smooth"; } +/** Resolve the song-structure renderer owned by this workspace, failing closed on ambiguous mounts. */ +function resolveHarmonicExplanationRenderer(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 harmonic explanation and open the matching rendered map section. */ export function FirstHarmonicExplanationCallout({ song }: FirstHarmonicExplanationCalloutProps) { const locale = detectPreferredLocale(); @@ -137,8 +153,8 @@ export function FirstHarmonicExplanationCallout({ song }: FirstHarmonicExplanati