diff --git a/AGENTS.md b/AGENTS.md index b9a67ce17..5b2f2c823 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,7 +1,7 @@ # AGENTS.md ## Project overview -- BandScope is a local-first desktop app for rehearsal prep: a practical song view with likely harmony by section and by instrument or vocal role, form and groove cues, stems, playable ranges, simplification guidance, transposition or setup cues, part-overlap cues, visible confidence, and rehearsal priorities. +- BandScope is a local-first desktop app for rehearsal prep: a practical song view with likely harmony by section and by instrument or vocal role, form and groove cues, stems, playable ranges, simplification guidance, transposition or setup cues, part-overlap cues, visible confidence, and rehearsal priorities. The ready workspace names tonight's first playable range and offers a next action that finds that section on the timeline. - Authoritative delivery rules live in `ARCHITECTURE.md`, `docs/plans/`, and the root verification scripts. - Brand, tone, UX copy, and prioritization rules live in `docs/brand-story.md` and must be applied to PRDs, TRDs, UI copy, onboarding, empty states, and error messages. - App security rules live in `docs/security/app-security.md` and must be applied to file handling, URL intake, subprocesses, IPC, WebView usage, model loading, updates, logging, cache handling, and export behavior. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ca0df5ac4..c61902132 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -82,7 +82,7 @@ Last updated: 2026-03-11 - likely harmony by section and by role - section roadmap with entries, dropouts, pickups, stops, tags, and handoffs - groove and timing cues relevant to locking the band together - - playable ranges and density or overlap warnings, with the ready workspace naming tonight's first span and the next instrument check + - playable ranges and density or overlap warnings, with the ready workspace naming tonight's first span, offering a timeline find control, and naming the next instrument check - simplification, transposition, capo, tuning, or setup cues where applicable - role-specific rehearsal priorities and confidence flags - cue-sheet or chart-style exports that summarize the analysis in rehearsal-friendly form diff --git a/CHANGELOG.md b/CHANGELOG.md index 34331fb86..c2e8da0f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added -- Name tonight's first playable range on the ready rehearsal map and tell the player to check that span on their instrument before the section. +- Name tonight's first playable range on the ready rehearsal map, offer Find {section} at {clock} on the timeline and Find {section} for {role} on the section roadmap, and tell the player to check that span on their instrument before the section. - Display the analyzed song tempo (BPM) as a badge in the rehearsal workspace. - 각 합주 역할(Role)별 개인 연습 진행도를 0~100% 범위로 기록 및 시각화할 수 있는 연습 진척도(`practiceProgress`) 트래커 기능 추가. UI 컨트롤(슬라이더 및 +/- 버튼)과 한/영 다국어 지원 포함. diff --git a/CLAUDE.md b/CLAUDE.md index b5a34c1fa..b4b99fca0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ BandScope is a local-first desktop app for rehearsal prep: it turns a song into Three layers, decoupled through shared contracts: -- `apps/desktop` — Tauri 2 + Vite + React 19 shell (Tailwind 4, Base UI, Storybook). Feature screens live in `src/features/` (home, workspace, chords, ranges, player, settings). The ready workspace names tonight's first playable range and the next instrument check. `src/lib/analysis.ts` and `src/lib/job_runner.ts` call typed Tauri IPC commands, with a browser fallback that serves demo data when not running inside Tauri. +- `apps/desktop` — Tauri 2 + Vite + React 19 shell (Tailwind 4, Base UI, Storybook). Feature screens live in `src/features/` (home, workspace, chords, ranges, player, settings). The ready workspace names tonight's first playable range, offers a control that finds that section on the timeline, and tells the player to check that span on their instrument. `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/App.projectNavigation.regression.test.tsx b/apps/desktop/src/App.projectNavigation.regression.test.tsx new file mode 100644 index 000000000..54f8db07d --- /dev/null +++ b/apps/desktop/src/App.projectNavigation.regression.test.tsx @@ -0,0 +1,52 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { App } from "./App"; + +vi.mock("./features/score/pdfjs", () => ({ + configureScorePdfWorker: vi.fn(), + loadScorePdf: vi.fn(() => ({ + promise: Promise.resolve({ numPages: 1, getPage: vi.fn() }), + destroy: vi.fn(() => Promise.resolve()) + })) +})); + +const mockLoadProject = vi.fn(); + +vi.mock("./lib/analysis", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + loadProject: () => mockLoadProject(), + subscribeToAnalysisJobUpdates: vi.fn(async () => () => undefined) + }; +}); + +describe("loaded-project rehearsal navigation identity", () => { + beforeEach(() => { + mockLoadProject.mockReset(); + }); + + it("clears timeline and roadmap focus when a second saved analysis reuses analyzed-song", async () => { + const firstProject = { ...createDemoRehearsalSong(), id: "analyzed-song", title: "First saved analysis" }; + const secondProject = { ...createDemoRehearsalSong(), id: "analyzed-song", title: "Second saved analysis" }; + mockLoadProject.mockResolvedValueOnce(firstProject).mockResolvedValueOnce(secondProject); + + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => expect(screen.getByRole("heading", { name: "First saved analysis" })).toBeInTheDocument()); + + fireEvent.click(screen.getByRole("button", { name: /Find verse at 0:10–0:30 on the timeline/i })); + fireEvent.click(screen.getByRole("button", { name: /Find verse for Bass Guitar on the roadmap/i })); + expect(screen.getByTestId("song-structure-section-verse-1")).toHaveAttribute("aria-current", "location"); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).toHaveAttribute("aria-current", "true"); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => expect(screen.getByRole("heading", { name: "Second saved analysis" })).toBeInTheDocument()); + + expect(screen.getByTestId("song-structure-section-verse-1")).not.toHaveAttribute("aria-current"); + expect(screen.getByTestId("section-roadmap-section-verse-1")).not.toHaveAttribute("aria-current"); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).not.toHaveAttribute("aria-current"); + }); +}); diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index f3d678454..97d8fd286 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -254,6 +254,7 @@ export function App() { const [jobStatus, setJobStatus] = useState(null); const [jobResult, setJobResult] = useState(null); const [jobResultBootstrap, setJobResultBootstrap] = useState(null); + const [loadedProjectRevision, setLoadedProjectRevision] = useState(0); const [jobError, setJobError] = useState(null); const [renderedProgressPercent, setRenderedProgressPercent] = useState(undefined); const [isStarting, setIsStarting] = useState(false); @@ -474,6 +475,7 @@ export function App() { const handleLoadProject = async () => { try { const song = await loadProject(); + setLoadedProjectRevision((revision) => revision + 1); setJobResult(song); setJobResultBootstrap(null); setJobError(null); @@ -512,7 +514,17 @@ export function App() { return ; } if (jobResult) { - return ; + const workspaceInstanceKey = jobResultBootstrap?.projectId + ? `analysis-project-${jobResultBootstrap.projectId}` + : `loaded-project-${loadedProjectRevision}`; + return ( + + ); } return ; }; diff --git a/apps/desktop/src/features/workspace/SectionRoadmap.tsx b/apps/desktop/src/features/workspace/SectionRoadmap.tsx index 834d1e8f0..042c31fe1 100644 --- a/apps/desktop/src/features/workspace/SectionRoadmap.tsx +++ b/apps/desktop/src/features/workspace/SectionRoadmap.tsx @@ -1,5 +1,5 @@ import type { RehearsalSong, RehearsalRole } from "@bandscope/shared-types"; -import { useId, useMemo } from "react"; +import { useEffect, useId, useMemo, useRef } from "react"; import { createTranslator, detectPreferredLocale } from "../../i18n"; import { ConfidenceBadge } from "./ConfidenceBadge"; import { fillRangeCopy, playableRange } from "./firstRangeSqueeze"; @@ -12,13 +12,49 @@ interface SectionRoadmapProps { song: RehearsalSong; activeRole: string | null; // null means all roles onSongUpdate?: (song: RehearsalSong) => void; + focusSectionId?: string | null; + focusRoleId?: string | null; + focusRequestSequence?: number; +} + +/** Encode the exact section/role pair without delimiter-collision ambiguity. */ +function roadmapRoleFocusKey(sectionId: string, roleId: string): string { + return JSON.stringify([sectionId, roleId]); } /** Documented. */ -export function SectionRoadmap({ song, activeRole, onSongUpdate }: SectionRoadmapProps) { +export function SectionRoadmap({ + song, + activeRole, + onSongUpdate, + focusSectionId = null, + focusRoleId = null, + focusRequestSequence = 0 +}: SectionRoadmapProps) { const sectionRoadmapTitleId = useId(); const locale = useMemo(() => detectPreferredLocale(), []); const t = useMemo(() => createTranslator(locale), [locale]); + const sectionCardRefs = useRef(new Map()); + const roleCardRefs = useRef(new Map()); + + useEffect(() => { + if (!focusSectionId || focusRequestSequence < 1) { + return; + } + const focusTarget = focusRoleId + ? roleCardRefs.current.get(roadmapRoleFocusKey(focusSectionId, focusRoleId)) + : sectionCardRefs.current.get(focusSectionId); + if (focusTarget && typeof focusTarget.scrollIntoView === "function") { + const reducedMotionPreferred = + typeof window.matchMedia === "function" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches; + focusTarget.scrollIntoView({ + behavior: reducedMotionPreferred ? "auto" : "smooth", + inline: "center", + block: "nearest" + }); + } + }, [focusRequestSequence, focusRoleId, focusSectionId]); /** Documented. */ const editChordLabel = (role: RehearsalRole, sectionLabel: string): string => { @@ -104,11 +140,29 @@ export function SectionRoadmap({ song, activeRole, onSongUpdate }: SectionRoadma tabIndex={0} aria-labelledby={sectionRoadmapTitleId} > - {song.sections.map((section) => ( - { + const sectionFocused = focusSectionId === section.id; + return ( +
{ + if (sectionNode) { + sectionCardRefs.current.set(section.id, sectionNode); + } else { + sectionCardRefs.current.delete(section.id); + } + }} + data-testid={`section-roadmap-section-${section.id}`} + aria-current={sectionFocused ? "location" : undefined} + className="w-80 flex-none shrink-0 snap-start" + > + @@ -127,10 +181,23 @@ export function SectionRoadmap({ song, activeRole, onSongUpdate }: SectionRoadma .filter(role => !activeRole || role.id === activeRole) .map(role => { const validatedRange = playableRange(role.range.lowestNote, role.range.highestNote); + const roleFocused = sectionFocused && focusRoleId === role.id; + const roleFocusKey = roadmapRoleFocusKey(section.id, role.id); return (
{ + if (roleNode) { + roleCardRefs.current.set(roleFocusKey, roleNode); + } else { + roleCardRefs.current.delete(roleFocusKey); + } + }} + data-testid={`section-roadmap-role-${section.id}-${role.id}`} + aria-current={roleFocused ? "true" : undefined} + className={`rounded-xl border-l-4 p-4 transition-all hover:translate-x-1 ${getPriorityColor(role.rehearsalPriority)}${ + roleFocused ? " ring-2 ring-fuchsia-300" : "" + }`} >
@@ -228,7 +295,9 @@ export function SectionRoadmap({ song, activeRole, onSongUpdate }: SectionRoadma })} - ))} +
+ ); + })}
); diff --git a/apps/desktop/src/features/workspace/Workspace.test.tsx b/apps/desktop/src/features/workspace/Workspace.test.tsx index 7837bf80e..f97fb0087 100644 --- a/apps/desktop/src/features/workspace/Workspace.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.test.tsx @@ -140,6 +140,39 @@ describe("Workspace", () => { expect(screen.getByText(/Verse harmony pass/i)).toBeTruthy(); }); + it("finds tonight's first range on the structure timeline", () => { + setNavigatorLanguage("en-US"); + const song = createDemoRehearsalSong(); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })); + + expect(screen.getByTestId("song-structure-section-verse-1")).toHaveAttribute("aria-current", "location"); + expect(screen.getByTestId("song-structure-grid").querySelector("[aria-current='location']")).toHaveTextContent(/verse · 0:10–0:30/i); + }); + + it("hides the timeline find control when the named section clock is unusable", () => { + setNavigatorLanguage("en-US"); + const song = createDemoRehearsalSong(); + song.sections[0].timeRange = { + start: Number.NaN, + end: Number.POSITIVE_INFINITY + }; + + render(); + + expect(screen.queryByRole("button", { name: /Find .+ on the timeline/ })).toBeNull(); + }); + + it("localizes the first-range timeline find control", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + + render(); + + expect(screen.getByRole("button", { name: "타임라인에서 0:10–0:30 verse 찾기" })).toBeTruthy(); + }); + it("names tonight's first playable range and the next instrument check", () => { setNavigatorLanguage("en-US"); const song = createDemoRehearsalSong(); @@ -151,6 +184,7 @@ describe("Workspace", () => { expect(callout).toHaveTextContent( "Bass Guitar sits C#2–E3 in verse. Hear that clash on your instrument before the verse." ); + expect(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })).toBeTruthy(); }); it("asks for an ear check when the selected part has no named span", () => { @@ -167,6 +201,7 @@ describe("Workspace", () => { expect(screen.getByTestId("first-range-squeeze")).toHaveTextContent( "Tonight's first range still needs an ear check. Confirm the high and low notes on the selected part before the first section." ); + expect(screen.queryByRole("button", { name: /Find .+ on the timeline/ })).toBeNull(); }); it("limits the range callout to the selected role", () => { diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index d44e20777..7ca1f7b34 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -1,10 +1,17 @@ -import { useState, useMemo, memo, type MouseEvent } from "react"; +import { useState, useMemo, useEffect, useRef, memo, type MouseEvent } from "react"; import { parseProjectBootstrapSummary, type ProjectBootstrapSummary, type RehearsalSong, type RehearsalRole } from "@bandscope/shared-types"; import { RoleSwitcher } from "./RoleSwitcher"; import { SectionRoadmap } from "./SectionRoadmap"; import { GrooveMap } from "./GrooveMap"; import { PracticeProgress } from "./PracticeProgress"; -import { fillRangeCopy, firstRangeSqueeze } from "./firstRangeSqueeze"; +import { + fillRangeCopy, + firstRangeRoadmap, + firstRangeSqueeze, + firstRangeTimeline, + hasUniqueRoadmapNavigationTarget, + hasUniqueSectionNavigationTarget +} from "./firstRangeSqueeze"; import { createTranslator, detectPreferredLocale } from "../../i18n"; import { generateCueSheetCsv, generateChartSummaryJson, generateMetadataHandoffJson, sanitizeFilename } from "../../lib/export"; import { Button } from "@/components/ui/button"; @@ -17,6 +24,21 @@ interface WorkspaceProps { onSongUpdate?: (song: RehearsalSong) => void; } +/** Request identity for a user-initiated structure-timeline focus action. */ +type TimelineFocusRequest = { + rehearsalSourceIdentity: string; + sectionId: string; + requestSequence: number; +}; + +/** Request identity for a user-initiated section-roadmap focus action. */ +type RoadmapFocusRequest = { + rehearsalSourceIdentity: string; + sectionId: string; + roleId: string; + requestSequence: number; +}; + /** Documented. */ function formatTimelineTime(totalSeconds: number): string { const safeSeconds = Number.isFinite(totalSeconds) && totalSeconds >= 0 ? totalSeconds : 0; @@ -72,7 +94,36 @@ function safeProjectBootstrapSummary(value: ProjectBootstrapSummary | null): Pro } /** Documented. */ -const SongStructure = memo(function SongStructure({ sections, t }: { sections: RehearsalSong["sections"]; t: Translator }) { +const SongStructure = memo(function SongStructure({ + sections, + t, + focusSectionId, + focusRequestSequence +}: { + sections: RehearsalSong["sections"]; + t: Translator; + focusSectionId: string | null; + focusRequestSequence: number; +}) { + const cellRefs = useRef(new Map()); + + useEffect(() => { + if (!focusSectionId || focusRequestSequence < 1) { + return; + } + const sectionCell = cellRefs.current.get(focusSectionId); + if (sectionCell && typeof sectionCell.scrollIntoView === "function") { + const reducedMotionPreferred = + typeof window.matchMedia === "function" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches; + sectionCell.scrollIntoView({ + behavior: reducedMotionPreferred ? "auto" : "smooth", + inline: "center", + block: "nearest" + }); + } + }, [focusRequestSequence, focusSectionId]); + return (
@@ -91,14 +142,33 @@ 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) => ( -
-

- {section.label} · {formatTimelineTime(section.timeRange.start)}–{formatTimelineTime(section.timeRange.end)} -

-

{section.groove}

-
- ))} + {sections.map((section) => { + const sectionFocused = focusSectionId === section.id; + return ( +
{ + if (sectionNode) { + cellRefs.current.set(section.id, sectionNode); + } else { + cellRefs.current.delete(section.id); + } + }} + data-testid={`song-structure-section-${section.id}`} + aria-current={sectionFocused ? "location" : undefined} + className={ + sectionFocused + ? "border-r border-fuchsia-300/40 bg-fuchsia-300/15 px-3 py-3 last:border-r-0 ring-2 ring-inset ring-fuchsia-300" + : "border-r border-white/10 bg-cyan-300/[0.05] px-3 py-3 last:border-r-0" + } + > +

+ {section.label} · {formatTimelineTime(section.timeRange.start)}–{formatTimelineTime(section.timeRange.end)} +

+

{section.groove}

+
+ ); + })}
@@ -353,7 +525,12 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
- +
@@ -506,6 +683,9 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp song={song} activeRole={activeRole} onSongUpdate={onSongUpdate} + focusSectionId={roadmapFocusedSectionId} + focusRoleId={roadmapFocusedRoleId} + focusRequestSequence={roadmapFocusRequestSequence} />
diff --git a/apps/desktop/src/features/workspace/firstRangeRoadmap.regression.test.tsx b/apps/desktop/src/features/workspace/firstRangeRoadmap.regression.test.tsx new file mode 100644 index 000000000..23880e954 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstRangeRoadmap.regression.test.tsx @@ -0,0 +1,196 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong, type ProjectBootstrapSummary } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { Workspace } from "./Workspace"; +import { firstRangeRoadmap, firstRangeSqueeze } from "./firstRangeSqueeze"; + +const originalLanguage = navigator.language; +const originalMatchMedia = window.matchMedia; +const originalScrollIntoView = HTMLElement.prototype.scrollIntoView; + +function installScrollRecorder() { + const scrollRequests = vi.fn(); + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { configurable: true, value: scrollRequests }); + return scrollRequests; +} + +function setReducedMotionPreference(reducedMotionPreferred: boolean) { + Object.defineProperty(window, "matchMedia", { configurable: true, value: vi.fn().mockReturnValue({ matches: reducedMotionPreferred }) }); +} + +function setNavigatorLanguage(language: string) { + Object.defineProperty(navigator, "language", { configurable: true, value: language }); +} + +function projectBootstrap(projectId: string): ProjectBootstrapSummary { + return { + projectId, + sourceMode: "reference", + projectRoot: `/tmp/bandscope/projects/${projectId}`, + cacheRoot: `/tmp/bandscope/projects/${projectId}/cache`, + tempRoot: `/tmp/bandscope/projects/${projectId}/tmp`, + source: { sourcePath: `/tmp/bandscope/projects/${projectId}/source.wav`, fileName: "source.wav", extension: "wav", fileSizeBytes: 1024 } + }; +} + +describe("first-range roadmap interaction regressions", () => { + afterEach(() => { + setNavigatorLanguage(originalLanguage); + if (originalScrollIntoView) { + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { configurable: true, value: originalScrollIntoView }); + } else { + delete (HTMLElement.prototype as { scrollIntoView?: typeof HTMLElement.prototype.scrollIntoView }).scrollIntoView; + } + Object.defineProperty(window, "matchMedia", { configurable: true, value: originalMatchMedia }); + vi.restoreAllMocks(); + }); + + it("requests roadmap focus again when Find is activated twice for the same part", () => { + const scrollRequests = installScrollRecorder(); + const rehearsalSong = createDemoRehearsalSong(); + render(); + const findRoadmapButton = screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" }); + fireEvent.click(findRoadmapButton); + fireEvent.click(findRoadmapButton); + expect(scrollRequests).toHaveBeenCalledTimes(2); + }); + + it("scrolls the exact requested role rather than only its section wrapper", () => { + const rehearsalSong = createDemoRehearsalSong(); + render(); + const sectionCard = screen.getByTestId("section-roadmap-section-verse-1"); + const roleCard = screen.getByTestId("section-roadmap-role-verse-1-bass-guitar"); + const sectionScroll = vi.fn(); + const roleScroll = vi.fn(); + Object.defineProperty(sectionCard, "scrollIntoView", { configurable: true, value: sectionScroll }); + Object.defineProperty(roleCard, "scrollIntoView", { configurable: true, value: roleScroll }); + + fireEvent.click(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })); + + expect(roleScroll).toHaveBeenCalledTimes(1); + expect(sectionScroll).not.toHaveBeenCalled(); + }); + + it("does not carry a focused roadmap cell into a replacement rehearsal song", () => { + installScrollRecorder(); + const rehearsalSong = createDemoRehearsalSong(); + const replacementSong = { ...createDemoRehearsalSong(), id: "replacement-song" }; + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })); + expect(screen.getByTestId("section-roadmap-section-verse-1")).toHaveAttribute("aria-current", "location"); + renderedWorkspace.rerender(); + expect(screen.getByTestId("section-roadmap-section-verse-1")).not.toHaveAttribute("aria-current"); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).not.toHaveAttribute("aria-current"); + }); + + it("does not carry roadmap focus across projects that reuse analyzed-song", () => { + installScrollRecorder(); + const analyzedSong = { ...createDemoRehearsalSong(), id: "analyzed-song" }; + const replacementAnalysis = { ...createDemoRehearsalSong(), id: "analyzed-song" }; + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })); + expect(screen.getByTestId("section-roadmap-section-verse-1")).toHaveAttribute("aria-current", "location"); + renderedWorkspace.rerender(); + expect(screen.getByTestId("section-roadmap-section-verse-1")).not.toHaveAttribute("aria-current"); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).not.toHaveAttribute("aria-current"); + }); + + it("does not conflate a project identity with a fallback song identity of the same spelling", () => { + installScrollRecorder(); + const projectSong = { ...createDemoRehearsalSong(), id: "shared-source-id" }; + const replacementSong = { ...createDemoRehearsalSong(), id: "shared-source-id" }; + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).toHaveAttribute("aria-current", "true"); + + renderedWorkspace.rerender(); + + expect(screen.getByTestId("section-roadmap-section-verse-1")).not.toHaveAttribute("aria-current"); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).not.toHaveAttribute("aria-current"); + }); + + it("drops an existing roadmap request when a same-source song update makes the target role ambiguous", () => { + installScrollRecorder(); + const rehearsalSong = createDemoRehearsalSong(); + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })); + expect(screen.getByTestId("section-roadmap-role-verse-1-bass-guitar")).toHaveAttribute("aria-current", "true"); + + const ambiguousSong = createDemoRehearsalSong(); + ambiguousSong.sections[0]!.roles.push({ ...ambiguousSong.sections[0]!.roles[0]!, name: "Bass Guitar Double" }); + renderedWorkspace.rerender(); + + expect(screen.getByTestId("section-roadmap-section-verse-1")).not.toHaveAttribute("aria-current"); + expect(screen.getAllByTestId("section-roadmap-role-verse-1-bass-guitar").every((roleCard) => !roleCard.hasAttribute("aria-current"))).toBe(true); + }); + + it("keeps repeated labels and display names navigable when IDs remain unique", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections.push({ ...rehearsalSong.sections[0]!, id: "verse-2" }); + rehearsalSong.sections[0]!.roles.push({ ...rehearsalSong.sections[0]!.roles[0]!, id: "bass-guitar-double" }); + expect(firstRangeRoadmap(rehearsalSong, firstRangeSqueeze(rehearsalSong))).toEqual({ sectionId: "verse-1", roleId: "bass-guitar", sectionLabel: "verse", roleName: "Bass Guitar" }); + render(); + expect(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })).toBeInTheDocument(); + }); + + it("fails closed when the target section identifier is duplicated elsewhere", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections.push({ ...rehearsalSong.sections[0]!, id: rehearsalSong.sections[0]!.id, label: "chorus" }); + expect(firstRangeRoadmap(rehearsalSong, firstRangeSqueeze(rehearsalSong))).toBeNull(); + render(); + expect(screen.queryByRole("button", { name: /Find .+ on the roadmap/ })).toBeNull(); + }); + + it("fails closed when the target role identifier is duplicated on the section", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections[0]!.roles.push({ ...rehearsalSong.sections[0]!.roles[0]!, name: "Bass Guitar Double" }); + expect(firstRangeRoadmap(rehearsalSong, firstRangeSqueeze(rehearsalSong))).toBeNull(); + render(); + expect(screen.queryByRole("button", { name: /Find .+ on the roadmap/ })).toBeNull(); + }); + + it("keeps playable-range evidence visible when noncanonical identity disables navigation", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections[0]!.id = " verse-1 "; + + render(); + + expect(screen.getByTestId("first-range-squeeze")).toHaveTextContent(/Bass Guitar.*C#2.*E3/i); + expect(screen.queryByRole("button", { name: /Find .+ on the timeline/ })).toBeNull(); + expect(screen.queryByRole("button", { name: /Find .+ on the roadmap/ })).toBeNull(); + }); + + it("hides the roadmap control when no role has a named playable range", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections = rehearsalSong.sections.map((section) => ({ + ...section, + roles: section.roles.map((role) => ({ + ...role, + range: { lowestNote: "", highestNote: "none" }, + overlapWarnings: [] + })) + })); + + render(); + + expect(screen.queryByRole("button", { name: /Find .+ on the roadmap/ })).toBeNull(); + }); + + it("localizes the roadmap find control in Korean", () => { + setNavigatorLanguage("ko-KR"); + const rehearsalSong = createDemoRehearsalSong(); + + render(); + + expect(screen.getByRole("button", { name: "로드맵에서 Bass Guitar verse 찾기" })).toBeInTheDocument(); + }); + + it("avoids smooth scrolling when reduced motion is preferred", () => { + const scrollRequests = installScrollRecorder(); + setReducedMotionPreference(true); + const rehearsalSong = createDemoRehearsalSong(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse for Bass Guitar on the roadmap" })); + expect(scrollRequests).toHaveBeenCalledWith(expect.objectContaining({ behavior: "auto" })); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts b/apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts index 643935954..701088f40 100644 --- a/apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts +++ b/apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts @@ -1,6 +1,6 @@ import { createDemoRehearsalSong, type RehearsalSong } from "@bandscope/shared-types"; import { describe, expect, it } from "vitest"; -import { fillRangeCopy, firstRangeSqueeze, meaningfulRangeText, playableRange } from "./firstRangeSqueeze"; +import { fillRangeCopy, firstRangeRoadmap, firstRangeSqueeze, firstRangeTimeline, formatRangeClock, meaningfulRangeText, playableRange } from "./firstRangeSqueeze"; function blankRoleRange(song: RehearsalSong): RehearsalSong { return { @@ -48,10 +48,10 @@ describe("playableRange", () => { describe("firstRangeSqueeze", () => { it("prefers the first named span that also carries a clash warning", () => { - const squeeze = firstRangeSqueeze(createDemoRehearsalSong()); - - expect(squeeze).toEqual({ + expect(firstRangeSqueeze(createDemoRehearsalSong())).toEqual({ + sectionId: "verse-1", sectionLabel: "verse", + roleId: "bass-guitar", roleName: "Bass Guitar", lowestNote: "C#2", highestNote: "E3", @@ -67,7 +67,9 @@ describe("firstRangeSqueeze", () => { })); expect(firstRangeSqueeze(song)).toEqual({ + sectionId: "verse-1", sectionLabel: "verse", + roleId: "bass-guitar", roleName: "Bass Guitar", lowestNote: "C#2", highestNote: "E3", @@ -82,7 +84,6 @@ describe("firstRangeSqueeze", () => { range: { lowestNote: "none", highestNote: "E3" }, overlapWarnings: ["Density warning: competing with Keyboard Left Hand in low register."] }; - expect(firstRangeSqueeze(song)?.roleName).toBe("Keyboard 1 Right Hand"); }); @@ -94,7 +95,6 @@ describe("firstRangeSqueeze", () => { const song = createDemoRehearsalSong(); const selectedRole = song.sections[0]!.roles[0]!; selectedRole.range = range; - expect(firstRangeSqueeze(song, selectedRole.id)).toBeNull(); } }); @@ -111,10 +111,10 @@ describe("firstRangeSqueeze", () => { roles: [null, { ...validRole, range: null }, validRole] }; - expect( - firstRangeSqueeze({ ...song, sections: [malformedSection] } as unknown as RehearsalSong) - ).toEqual({ + expect(firstRangeSqueeze({ ...song, sections: [malformedSection] } as unknown as RehearsalSong)).toEqual({ + sectionId: "verse-1", sectionLabel: "verse", + roleId: "bass-guitar", roleName: "Bass Guitar", lowestNote: "C#2", highestNote: "E3", @@ -123,10 +123,10 @@ describe("firstRangeSqueeze", () => { }); it("limits the squeeze to the selected role", () => { - const squeeze = firstRangeSqueeze(createDemoRehearsalSong(), "lead-vocal"); - - expect(squeeze).toEqual({ + expect(firstRangeSqueeze(createDemoRehearsalSong(), "lead-vocal")).toEqual({ + sectionId: "verse-1", sectionLabel: "verse", + roleId: "lead-vocal", roleName: "Lead Vocal", lowestNote: "G#3", highestNote: "C#5", @@ -138,30 +138,144 @@ describe("firstRangeSqueeze", () => { expect(firstRangeSqueeze(blankRoleRange(createDemoRehearsalSong()))).toBeNull(); expect(firstRangeSqueeze(createDemoRehearsalSong(), "missing-role")).toBeNull(); }); + + it("preserves playable-range evidence while noncanonical identity disables navigation", () => { + const spacedSection = createDemoRehearsalSong(); + spacedSection.sections[0]!.id = " verse-1 "; + const sectionSqueeze = firstRangeSqueeze(spacedSection); + expect(sectionSqueeze).toMatchObject({ + sectionId: " verse-1 ", + roleId: "bass-guitar", + roleName: "Bass Guitar", + lowestNote: "C#2", + highestNote: "E3" + }); + expect(firstRangeTimeline(spacedSection, sectionSqueeze)).toBeNull(); + expect(firstRangeRoadmap(spacedSection, sectionSqueeze)).toBeNull(); + + const spacedRole = createDemoRehearsalSong(); + spacedRole.sections[0]!.roles[0]!.id = " bass-guitar "; + const roleSqueeze = firstRangeSqueeze(spacedRole); + expect(roleSqueeze).toMatchObject({ + sectionId: "verse-1", + roleId: " bass-guitar ", + roleName: "Bass Guitar", + lowestNote: "C#2", + highestNote: "E3" + }); + expect(firstRangeRoadmap(spacedRole, roleSqueeze)).toBeNull(); + }); +}); + +describe("formatRangeClock", () => { + it("formats finite non-negative seconds as m:ss", () => { + expect(formatRangeClock(0)).toBe("0:00"); + expect(formatRangeClock(10)).toBe("0:10"); + expect(formatRangeClock(30)).toBe("0:30"); + expect(formatRangeClock(75)).toBe("1:15"); + }); + + it("fails closed on unusable clocks", () => { + expect(formatRangeClock(Number.NaN)).toBeNull(); + expect(formatRangeClock(Number.POSITIVE_INFINITY)).toBeNull(); + expect(formatRangeClock(-1)).toBeNull(); + expect(formatRangeClock("10")).toBeNull(); + }); +}); + +describe("firstRangeTimeline", () => { + it("names the unique first-range section clock", () => { + const song = createDemoRehearsalSong(); + expect(firstRangeTimeline(song, firstRangeSqueeze(song))).toEqual({ + sectionId: "verse-1", + sectionLabel: "verse", + startClock: "0:10", + endClock: "0:30" + }); + }); + + it("allows repeated form labels when the selected section identifier remains unique", () => { + const song = createDemoRehearsalSong(); + song.sections.push({ ...song.sections[0]!, id: "verse-2", timeRange: { start: 90, end: 110 } }); + + expect(firstRangeTimeline(song, firstRangeSqueeze(song))).toEqual({ + sectionId: "verse-1", + sectionLabel: "verse", + startClock: "0:10", + endClock: "0:30" + }); + }); + + it("fails closed when the squeeze is missing or the selected section identifier is duplicated", () => { + const song = createDemoRehearsalSong(); + const duplicateId = { + ...song, + sections: [song.sections[0]!, { ...song.sections[0]!, label: "chorus" }] + }; + expect(firstRangeTimeline(song, null)).toBeNull(); + expect(firstRangeTimeline(duplicateId, firstRangeSqueeze(song))).toBeNull(); + }); + + it("fails closed on malformed, zero-duration, inverted, or non-finite section times", () => { + const song = createDemoRehearsalSong(); + const squeeze = firstRangeSqueeze(song); + song.sections[0]!.timeRange = { start: Number.NaN, end: 30 }; + expect(firstRangeTimeline(song, squeeze)).toBeNull(); + song.sections[0]!.timeRange = { start: 10, end: 10 }; + expect(firstRangeTimeline(song, squeeze)).toBeNull(); + song.sections[0]!.timeRange = { start: 30, end: 10 }; + expect(firstRangeTimeline(song, squeeze)).toBeNull(); + song.sections[0]!.id = " "; + song.sections[0]!.timeRange = { start: 10, end: 30 }; + expect(firstRangeTimeline(song, squeeze)).toBeNull(); + }); +}); + +describe("firstRangeRoadmap", () => { + it("returns the selected section and part by unique identifiers", () => { + const song = createDemoRehearsalSong(); + expect(firstRangeRoadmap(song, firstRangeSqueeze(song))).toEqual({ + sectionId: "verse-1", + roleId: "bass-guitar", + sectionLabel: "verse", + roleName: "Bass Guitar" + }); + }); + + it("allows repeated labels and names when section and role identifiers remain unique", () => { + const song = createDemoRehearsalSong(); + song.sections.push({ ...song.sections[0]!, id: "verse-2" }); + song.sections[0]!.roles.push({ ...song.sections[0]!.roles[0]!, id: "bass-guitar-double" }); + + expect(firstRangeRoadmap(song, firstRangeSqueeze(song))).toEqual({ + sectionId: "verse-1", + roleId: "bass-guitar", + sectionLabel: "verse", + roleName: "Bass Guitar" + }); + }); + + it("fails closed when the selected section or role identifier is duplicated", () => { + const sectionDuplicate = createDemoRehearsalSong(); + sectionDuplicate.sections.push({ ...sectionDuplicate.sections[0]!, label: "chorus" }); + expect(firstRangeRoadmap(sectionDuplicate, firstRangeSqueeze(sectionDuplicate))).toBeNull(); + + const roleDuplicate = createDemoRehearsalSong(); + roleDuplicate.sections[0]!.roles.push({ ...roleDuplicate.sections[0]!.roles[0]!, name: "Bass Guitar Double" }); + expect(firstRangeRoadmap(roleDuplicate, firstRangeSqueeze(roleDuplicate))).toBeNull(); + }); }); describe("fillRangeCopy", () => { it("replaces every token occurrence", () => { - expect( - fillRangeCopy("{roleName} in {sectionLabel} before the {sectionLabel}.", { - roleName: "Bass Guitar", - sectionLabel: "verse" - }) - ).toBe("Bass Guitar in verse before the verse."); + expect(fillRangeCopy("{roleName} in {sectionLabel} before the {sectionLabel}.", { roleName: "Bass Guitar", sectionLabel: "verse" })).toBe("Bass Guitar in verse before the verse."); }); it("keeps replacement tokens and placeholder-shaped rehearsal values literal", () => { - expect( - fillRangeCopy("{roleName} in {sectionLabel}.", { - roleName: "Bass $& {sectionLabel}", - sectionLabel: "verse" - }) - ).toBe("Bass $& {sectionLabel} in verse."); + expect(fillRangeCopy("{roleName} in {sectionLabel}.", { roleName: "Bass $& {sectionLabel}", sectionLabel: "verse" })).toBe("Bass $& {sectionLabel} in verse."); }); it("does not satisfy tokens with inherited object members", () => { - expect( - fillRangeCopy("Check {toString} before {missingToken}.", { sectionLabel: "verse" }) - ).toBe("Check {toString} before {missingToken}."); + expect(fillRangeCopy("Check {toString} before {missingToken}.", { sectionLabel: "verse" })).toBe("Check {toString} before {missingToken}."); }); }); diff --git a/apps/desktop/src/features/workspace/firstRangeSqueeze.ts b/apps/desktop/src/features/workspace/firstRangeSqueeze.ts index 47270d2a9..95e83bf84 100644 --- a/apps/desktop/src/features/workspace/firstRangeSqueeze.ts +++ b/apps/desktop/src/features/workspace/firstRangeSqueeze.ts @@ -2,13 +2,31 @@ import type { RehearsalSong } from "@bandscope/shared-types"; /** Tonight's first named playable span on the rehearsal map. */ export type FirstRangeSqueeze = { + sectionId?: string; sectionLabel: string; + roleId: string; roleName: string; lowestNote: string; highestNote: string; overlapWarning?: string; }; +/** Clocked structure cell for tonight's first playable span. */ +export type FirstRangeTimeline = { + sectionId: string; + sectionLabel: string; + startClock: string; + endClock: string; +}; + +/** Trusted roadmap cell for tonight's first playable span. */ +export type FirstRangeRoadmap = { + sectionId: string; + roleId: string; + sectionLabel: string; + roleName: string; +}; + const NATURAL_PITCH_CLASS = { C: 0, D: 2, @@ -29,32 +47,135 @@ const ACCIDENTAL_OFFSET: Record = { const NOTE_PATTERN = /^([A-Ga-g])([#b♯♭]?)(-?\d{1,2})$/u; -/** Return whether an untrusted runtime value is a plain object record. */ -function isRuntimeObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); +/** Return whether an untrusted runtime candidate is a plain object record. */ +function isRuntimeObject(runtimeCandidate: unknown): runtimeCandidate is Record { + return typeof runtimeCandidate === "object" && runtimeCandidate !== null && !Array.isArray(runtimeCandidate); +} + +/** Return finite non-negative seconds, or fail closed. */ +function finiteNonNegativeSeconds(secondsCandidate: unknown): number | null { + if ( + typeof secondsCandidate !== "number" || + !Number.isFinite(secondsCandidate) || + secondsCandidate < 0 + ) { + return null; + } + return secondsCandidate; } -/** Return trimmed copy that is not a blank or `none` sentinel. */ -export function meaningfulRangeText(value: unknown): string | undefined { - if (typeof value !== "string") { +/** Return trimmed range copy that is not a blank or `none` sentinel. */ +export function meaningfulRangeText(rangeTextCandidate: unknown): string | undefined { + if (typeof rangeTextCandidate !== "string") { return undefined; } - const trimmed = value.trim(); - if (!trimmed || /^none$/i.test(trimmed)) { + const trimmedRangeText = rangeTextCandidate.trim(); + if (!trimmedRangeText || /^none$/i.test(trimmedRangeText)) { return undefined; } - return trimmed; + return trimmedRangeText; +} + +/** + * Admit a navigation identity only when its serialized spelling is already canonical. + * + * DOM refs and shared-type identities are keyed by the exact value. Trimming an + * untrusted ID would create a control whose target does not exist, so surrounding + * whitespace is rejected rather than silently normalized. + */ +function exactNavigationIdentity(identityCandidate: unknown): string | null { + if (typeof identityCandidate !== "string" || !identityCandidate) { + return null; + } + return identityCandidate.trim() === identityCandidate ? identityCandidate : null; +} + +/** Resolve one exact section identity only when it occurs once in the current song. */ +function uniqueSectionByIdentity( + rehearsalSong: RehearsalSong, + sectionIdentity: string +): Record | null { + const runtimeSong: unknown = rehearsalSong; + if (!isRuntimeObject(runtimeSong) || !Array.isArray(runtimeSong.sections)) { + return null; + } + + let sectionIdOccurrences = 0; + let targetSection: Record | null = null; + for (const sectionValue of runtimeSong.sections) { + if (!isRuntimeObject(sectionValue)) { + continue; + } + if (exactNavigationIdentity(sectionValue.id) === sectionIdentity) { + sectionIdOccurrences += 1; + targetSection = sectionValue; + } + } + + return sectionIdOccurrences === 1 ? targetSection : null; +} + +/** Resolve one exact role identity only when it occurs once inside the target section. */ +function uniqueRoleByIdentity( + targetSection: Record, + roleIdentity: string +): Record | null { + if (!Array.isArray(targetSection.roles)) { + return null; + } + + let roleIdOccurrences = 0; + let targetRole: Record | null = null; + for (const roleValue of targetSection.roles) { + if (!isRuntimeObject(roleValue)) { + continue; + } + if (exactNavigationIdentity(roleValue.id) === roleIdentity) { + roleIdOccurrences += 1; + targetRole = roleValue; + } + } + + return roleIdOccurrences === 1 ? targetRole : null; +} + +/** Revalidate an existing timeline focus request against the current song identity graph. */ +export function hasUniqueSectionNavigationTarget( + rehearsalSong: RehearsalSong, + sectionIdentityCandidate: unknown +): boolean { + const sectionIdentity = exactNavigationIdentity(sectionIdentityCandidate); + return sectionIdentity !== null && uniqueSectionByIdentity(rehearsalSong, sectionIdentity) !== null; +} + +/** Revalidate an existing roadmap focus request against the current song identity graph. */ +export function hasUniqueRoadmapNavigationTarget( + rehearsalSong: RehearsalSong, + sectionIdentityCandidate: unknown, + roleIdentityCandidate: unknown +): boolean { + const sectionIdentity = exactNavigationIdentity(sectionIdentityCandidate); + const roleIdentity = exactNavigationIdentity(roleIdentityCandidate); + if (sectionIdentity === null || roleIdentity === null) { + return false; + } + const targetSection = uniqueSectionByIdentity(rehearsalSong, sectionIdentity); + return targetSection !== null && uniqueRoleByIdentity(targetSection, roleIdentity) !== null; } /** Convert a bounded scientific-pitch label into chromatic ordering. */ -function notePitchValue(note: string): number | null { - const match = NOTE_PATTERN.exec(note); - if (!match) { +function notePitchValue(noteLabel: string): number | null { + const noteMatch = NOTE_PATTERN.exec(noteLabel); + if (!noteMatch) { return null; } - const letter = match[1].toUpperCase() as keyof typeof NATURAL_PITCH_CLASS; - const octave = Number(match[3]); - return (octave + 1) * 12 + NATURAL_PITCH_CLASS[letter] + ACCIDENTAL_OFFSET[match[2]]; + const noteLetter = noteMatch[1].toUpperCase() as keyof typeof NATURAL_PITCH_CLASS; + const noteOctave = Number(noteMatch[3]); + return ( + (noteOctave + 1) * 12 + + NATURAL_PITCH_CLASS[noteLetter] + + ACCIDENTAL_OFFSET[noteMatch[2]] + ); } /** @@ -84,30 +205,48 @@ export function playableRange( return { lowestNote, highestNote }; } +/** + * Format a rehearsal clock as `m:ss`, or fail closed on unusable values. + * + * Unlike the structure-grid fallback that renders `0:00` for NaN times, the + * first-range find control must not invent a clock the player cannot trust. + */ +export function formatRangeClock(clockSecondsCandidate: unknown): string | null { + const safeSeconds = finiteNonNegativeSeconds(clockSecondsCandidate); + if (safeSeconds === null) { + return null; + } + const clockMinutes = Math.floor(safeSeconds / 60); + const clockSeconds = Math.floor(safeSeconds % 60) + .toString() + .padStart(2, "0"); + return `${clockMinutes}:${clockSeconds}`; +} + /** * Pick the first playable range a player should check before the next section. * - * Prefers a named span that also carries a clash warning so the board names - * the squeeze that will waste rehearsal time. Falls back to the first named - * span when no clash is present. Runtime roots and collection members are - * treated as untrusted; malformed evidence is isolated instead of crashing - * the buyer-visible workspace or becoming playable-range authority. + * Playable-range truth does not depend on whether the originating record can + * also become UI navigation authority. Exact section/role identity evidence is + * preserved here and validated separately by the timeline/roadmap resolvers, + * so an unsafe ID can hide Find without erasing valid range guidance. */ export function firstRangeSqueeze( - song: RehearsalSong, - activeRole: string | null = null + rehearsalSong: RehearsalSong, + activeRoleId: string | null = null ): FirstRangeSqueeze | null { - const runtimeSong: unknown = song; + const runtimeSong: unknown = rehearsalSong; if (!isRuntimeObject(runtimeSong) || !Array.isArray(runtimeSong.sections)) { return null; } - let fallback: FirstRangeSqueeze | null = null; + let fallbackRange: FirstRangeSqueeze | null = null; for (const sectionValue of runtimeSong.sections) { if (!isRuntimeObject(sectionValue) || !Array.isArray(sectionValue.roles)) { continue; } + const sectionId = typeof sectionValue.id === "string" ? sectionValue.id : undefined; const sectionLabel = meaningfulRangeText(sectionValue.label); if (!sectionLabel) { continue; @@ -117,24 +256,27 @@ export function firstRangeSqueeze( if (!isRuntimeObject(roleValue)) { continue; } - const roleId = meaningfulRangeText(roleValue.id); + const roleId = typeof roleValue.id === "string" ? roleValue.id : ""; const roleName = meaningfulRangeText(roleValue.name); - if (!roleId || !roleName || (activeRole && roleId !== activeRole)) { + if (!roleName || (activeRoleId && roleId !== activeRoleId)) { continue; } if (!isRuntimeObject(roleValue.range)) { continue; } - const range = playableRange(roleValue.range.lowestNote, roleValue.range.highestNote); - if (!range) { + const playableRoleRange = playableRange( + roleValue.range.lowestNote, + roleValue.range.highestNote + ); + if (!playableRoleRange) { continue; } let overlapWarning: string | undefined; if (Array.isArray(roleValue.overlapWarnings)) { - for (const warning of roleValue.overlapWarnings) { - const meaningfulWarning = meaningfulRangeText(warning); + for (const overlapWarningValue of roleValue.overlapWarnings) { + const meaningfulWarning = meaningfulRangeText(overlapWarningValue); if (meaningfulWarning) { overlapWarning = meaningfulWarning; break; @@ -142,31 +284,136 @@ export function firstRangeSqueeze( } } - const candidate: FirstRangeSqueeze = { + const rangeCandidate: FirstRangeSqueeze = { + sectionId, sectionLabel, + roleId, roleName, - ...range, + ...playableRoleRange, overlapWarning }; if (overlapWarning) { - return candidate; + return rangeCandidate; } - if (!fallback) { - fallback = candidate; + if (!fallbackRange) { + fallbackRange = rangeCandidate; } } } - return fallback; + return fallbackRange; +} + +/** + * Offer the originating first-range section only when its identity and clock are trusted. + * + * Display labels may repeat in ordinary song form and therefore are not + * navigation authority. The selected section ID must occur exactly once on + * the current map; its current label and time range are then derived from that + * cell. Does not start playback; #961 owns the rehearsal player. + */ +export function firstRangeTimeline( + rehearsalSong: RehearsalSong, + rangeSqueeze: FirstRangeSqueeze | null +): FirstRangeTimeline | null { + if (!rangeSqueeze) { + return null; + } + + const targetSectionId = exactNavigationIdentity(rangeSqueeze.sectionId); + if (!targetSectionId) { + return null; + } + + const targetSection = uniqueSectionByIdentity(rehearsalSong, targetSectionId); + if (!targetSection || !isRuntimeObject(targetSection.timeRange)) { + return null; + } + + const sectionLabel = meaningfulRangeText(targetSection.label); + const startSeconds = finiteNonNegativeSeconds(targetSection.timeRange.start); + const endSeconds = finiteNonNegativeSeconds(targetSection.timeRange.end); + const startClock = formatRangeClock(startSeconds); + const endClock = formatRangeClock(endSeconds); + if ( + !sectionLabel || + startSeconds === null || + endSeconds === null || + startClock === null || + endClock === null || + endSeconds <= startSeconds + ) { + return null; + } + + return { + sectionId: targetSectionId, + sectionLabel, + startClock, + endClock + }; +} + +/** + * Offer the originating first-range section and part only when their IDs remain unique and trusted. + * + * Section labels and role display names may repeat. The selected section ID + * must occur exactly once across the map and the selected role ID exactly once + * inside that section; current presentation copy is derived from those cells. + * Does not start playback; #961 owns the rehearsal player. + */ +export function firstRangeRoadmap( + rehearsalSong: RehearsalSong, + rangeSqueeze: FirstRangeSqueeze | null +): FirstRangeRoadmap | null { + if (!rangeSqueeze) { + return null; + } + + const targetSectionId = exactNavigationIdentity(rangeSqueeze.sectionId); + const targetRoleId = exactNavigationIdentity(rangeSqueeze.roleId); + if (!targetSectionId || !targetRoleId) { + return null; + } + + const targetSection = uniqueSectionByIdentity(rehearsalSong, targetSectionId); + if (!targetSection) { + return null; + } + const targetRole = uniqueRoleByIdentity(targetSection, targetRoleId); + if (!targetRole) { + return null; + } + + const sectionLabel = meaningfulRangeText(targetSection.label); + const roleName = meaningfulRangeText(targetRole.name); + if (!sectionLabel || !roleName) { + return null; + } + + return { + sectionId: targetSectionId, + roleId: targetRoleId, + sectionLabel, + roleName + }; } /** Fill trusted `{token}` placeholders once while keeping rehearsal values literal. */ -export function fillRangeCopy(template: string, values: Record): string { - return template.replace(/\{([A-Za-z][A-Za-z0-9]*)\}/g, (placeholder, token: string) => { - // Own-property lookup only: inherited members such as `toString` must - // never satisfy a token, or the raw function source would be rendered. - return Object.prototype.hasOwnProperty.call(values, token) ? values[token] : placeholder; - }); +export function fillRangeCopy( + copyTemplate: string, + copyValues: Record +): string { + return copyTemplate.replace( + /\{([A-Za-z][A-Za-z0-9]*)\}/g, + (copyPlaceholder, copyToken: string) => { + // Own-property lookup only: inherited members such as `toString` must + // never satisfy a token, or the raw function source would be rendered. + return Object.prototype.hasOwnProperty.call(copyValues, copyToken) + ? copyValues[copyToken] + : copyPlaceholder; + } + ); } diff --git a/apps/desktop/src/features/workspace/firstRangeTimeline.regression.test.tsx b/apps/desktop/src/features/workspace/firstRangeTimeline.regression.test.tsx new file mode 100644 index 000000000..a105cda97 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstRangeTimeline.regression.test.tsx @@ -0,0 +1,118 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong, type ProjectBootstrapSummary } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { Workspace } from "./Workspace"; +import { firstRangeSqueeze, firstRangeTimeline } from "./firstRangeSqueeze"; + +const originalMatchMedia = window.matchMedia; +const originalScrollIntoView = HTMLElement.prototype.scrollIntoView; + +function installScrollRecorder() { + const scrollRequests = vi.fn(); + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { configurable: true, value: scrollRequests }); + return scrollRequests; +} + +function setReducedMotionPreference(reducedMotionPreferred: boolean) { + Object.defineProperty(window, "matchMedia", { configurable: true, value: vi.fn().mockReturnValue({ matches: reducedMotionPreferred }) }); +} + +function projectBootstrap(projectId: string): ProjectBootstrapSummary { + return { + projectId, + sourceMode: "reference", + projectRoot: `/tmp/bandscope/projects/${projectId}`, + cacheRoot: `/tmp/bandscope/projects/${projectId}/cache`, + tempRoot: `/tmp/bandscope/projects/${projectId}/tmp`, + source: { + sourcePath: `/tmp/bandscope/projects/${projectId}/source.wav`, + fileName: "source.wav", + extension: "wav", + fileSizeBytes: 1024 + } + }; +} + +describe("first-range timeline interaction regressions", () => { + afterEach(() => { + if (originalScrollIntoView) { + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { configurable: true, value: originalScrollIntoView }); + } else { + delete (HTMLElement.prototype as { scrollIntoView?: typeof HTMLElement.prototype.scrollIntoView }).scrollIntoView; + } + Object.defineProperty(window, "matchMedia", { configurable: true, value: originalMatchMedia }); + vi.restoreAllMocks(); + }); + + it("requests timeline focus again when Find is activated twice for the same section", () => { + const scrollRequests = installScrollRecorder(); + const rehearsalSong = createDemoRehearsalSong(); + render(); + const findSectionButton = screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" }); + fireEvent.click(findSectionButton); + fireEvent.click(findSectionButton); + expect(scrollRequests).toHaveBeenCalledTimes(2); + }); + + it("does not carry a focused section into a replacement rehearsal song", () => { + installScrollRecorder(); + const rehearsalSong = createDemoRehearsalSong(); + const replacementSong = { ...createDemoRehearsalSong(), id: "replacement-song" }; + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })); + expect(screen.getByTestId("song-structure-section-verse-1")).toHaveAttribute("aria-current", "location"); + renderedWorkspace.rerender(); + expect(screen.getByTestId("song-structure-section-verse-1")).not.toHaveAttribute("aria-current"); + }); + + it("does not carry timeline focus across projects that reuse analyzed-song", () => { + installScrollRecorder(); + const analyzedSong = { ...createDemoRehearsalSong(), id: "analyzed-song" }; + const replacementAnalysis = { ...createDemoRehearsalSong(), id: "analyzed-song" }; + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })); + expect(screen.getByTestId("song-structure-section-verse-1")).toHaveAttribute("aria-current", "location"); + renderedWorkspace.rerender(); + expect(screen.getByTestId("song-structure-section-verse-1")).not.toHaveAttribute("aria-current"); + }); + + it("drops an existing focus request when a same-source song update makes the target identifier ambiguous", () => { + installScrollRecorder(); + const rehearsalSong = createDemoRehearsalSong(); + const renderedWorkspace = render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })); + expect(screen.getByTestId("song-structure-section-verse-1")).toHaveAttribute("aria-current", "location"); + + const ambiguousSong = createDemoRehearsalSong(); + ambiguousSong.sections.push({ ...ambiguousSong.sections[0]!, label: "chorus", timeRange: { start: 90, end: 110 } }); + renderedWorkspace.rerender(); + + expect(screen.getByTestId("song-structure-grid").querySelectorAll("[aria-current='location']")).toHaveLength(0); + }); + + it("keeps repeated form labels navigable when IDs remain unique", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections.push({ ...rehearsalSong.sections[0]!, id: "verse-2", timeRange: { start: 90, end: 110 } }); + expect(firstRangeTimeline(rehearsalSong, firstRangeSqueeze(rehearsalSong))).toEqual({ sectionId: "verse-1", sectionLabel: "verse", startClock: "0:10", endClock: "0:30" }); + render(); + expect(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })).toBeInTheDocument(); + }); + + it("fails closed when the target section identifier is duplicated elsewhere", () => { + const rehearsalSong = createDemoRehearsalSong(); + rehearsalSong.sections[1] = { ...rehearsalSong.sections[1]!, id: rehearsalSong.sections[0]!.id, label: "chorus" }; + expect(firstRangeTimeline(rehearsalSong, firstRangeSqueeze(rehearsalSong))).toBeNull(); + render(); + expect(screen.queryByRole("button", { name: /Find .+ on the timeline/ })).toBeNull(); + expect(screen.getByTestId("song-structure-grid").querySelectorAll("[aria-current='location']")).toHaveLength(0); + }); + + it("avoids smooth scrolling when reduced motion is preferred", () => { + const scrollRequests = installScrollRecorder(); + setReducedMotionPreference(true); + const rehearsalSong = createDemoRehearsalSong(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Find verse at 0:10–0:30 on the timeline" })); + expect(scrollRequests).toHaveBeenCalledWith(expect.objectContaining({ behavior: "auto" })); + }); +}); diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index d803a765e..5c726041b 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -153,6 +153,8 @@ "workspaceFirstRangeCheck": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Check that span on your instrument before the {sectionLabel}.", "workspaceFirstRangeClash": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Hear that clash on your instrument before the {sectionLabel}.", "workspaceFirstRangeMissing": "Tonight's first range still needs an ear check. Confirm the high and low notes on the selected part before the first section.", + "workspaceFirstRangeFindSection": "Find {sectionLabel} at {startClock}–{endClock} on the timeline", + "workspaceFirstRangeFindRoadmap": "Find {sectionLabel} for {roleName} on the roadmap", "sectionRangeLabel": "Range", "sectionRangeNextAction": "Check this span on your instrument before {sectionLabel}." } diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 0f6c6c66d..6e7aaec44 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -153,6 +153,8 @@ "workspaceFirstRangeCheck": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}입니다. {sectionLabel} 들어가기 전에 그 음역을 악기로 확인해 보세요.", "workspaceFirstRangeClash": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}이고 다른 파트와 겹칩니다. {sectionLabel} 들어가기 전에 그 충돌을 악기로 들어 보세요.", "workspaceFirstRangeMissing": "오늘 먼저 볼 음역은 아직 귀로 확인이 필요합니다. 선택한 파트의 최저·최고음을 첫 구간 전에 확인해 보세요.", + "workspaceFirstRangeFindSection": "타임라인에서 {startClock}–{endClock} {sectionLabel} 찾기", + "workspaceFirstRangeFindRoadmap": "로드맵에서 {roleName} {sectionLabel} 찾기", "sectionRangeLabel": "음역", "sectionRangeNextAction": "{sectionLabel} 들어가기 전에 이 음역을 악기로 확인해 보세요." } diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index e7e56d311..0f232454e 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -11,7 +11,7 @@ It is technically defined as a rehearsal-analysis product, not a single-output c - likely harmony by section and by role - section roadmap with entries, dropouts, pickups, stops, and handoffs - groove and timing cues -- role ranges, overlap warnings, and simplification guidance +- role ranges, overlap warnings, and simplification guidance, with the ready workspace naming tonight's first span and offering controls that find the section on the timeline and the exact part on the section roadmap - transposition, capo, tuning, or setup cues where relevant - role-specific confidence and rehearsal priority