From 3833d38a8769130eabfc5b420962d9c996dbdb4d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 14:17:52 +0000 Subject: [PATCH 01/10] feat(workspace): name tonight's first pending approval on the map Name the owner and owned scope so the room can sign off the waiting approval. Open moves to the uniquely named rendered map section. Assignments, comments, and already-approved scopes stay guidance-only. --- AGENTS.md | 1 + ARCHITECTURE.md | 3 +- CHANGELOG.md | 1 + CLAUDE.md | 2 +- .../FirstApprovalCallout.particle.test.tsx | 43 +++ ...rstApprovalCallout.reduced-motion.test.tsx | 43 +++ .../workspace/FirstApprovalCallout.test.tsx | 215 +++++++++++++ .../workspace/FirstApprovalCallout.tsx | 148 +++++++++ .../src/features/workspace/Workspace.test.tsx | 13 +- .../src/features/workspace/Workspace.tsx | 3 + .../firstApproval.inherited-metadata.test.ts | 107 +++++++ .../features/workspace/firstApproval.test.ts | 215 +++++++++++++ .../src/features/workspace/firstApproval.ts | 290 ++++++++++++++++++ apps/desktop/src/i18n/index.test.ts | 19 +- apps/desktop/src/i18n/index.ts | 38 ++- apps/desktop/src/locales/en/common.json | 12 +- apps/desktop/src/locales/ko/common.json | 12 +- docs/design-system/component-contract.md | 1 + ...educed-motion-first-approval-navigation.md | 14 + 19 files changed, 1172 insertions(+), 8 deletions(-) create mode 100644 apps/desktop/src/features/workspace/FirstApprovalCallout.particle.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstApprovalCallout.reduced-motion.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstApprovalCallout.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstApprovalCallout.tsx create mode 100644 apps/desktop/src/features/workspace/firstApproval.inherited-metadata.test.ts create mode 100644 apps/desktop/src/features/workspace/firstApproval.test.ts create mode 100644 apps/desktop/src/features/workspace/firstApproval.ts create mode 100644 docs/doctoring/reduced-motion-first-approval-navigation.md diff --git a/AGENTS.md b/AGENTS.md index fca448ce9..9fbbf3348 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 pending approval with the owner, the owned scope, and the uniquely named labeled section and time so the next action is obvious. Do not invent a sign-off from assignments, comments, or already-approved scopes. - 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..d0578aae9 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1,11 +1,12 @@ # ARCHITECTURE.md -Last updated: 2026-03-11 +Last updated: 2026-08-23 ## Brand source - Product identity, UX tone, copy rules, and prioritization tie-breakers live in `docs/brand-story.md`. - Future PRDs, TRDs, onboarding copy, empty states, error messages, and marketing copy should use that document as the single brand source of truth. +- The mounted workspace copy for tonight's first pending approval must name the owner, the owned scope, and the uniquely named labeled section and time so the next action is obvious. Open moves to the matching rendered map section. Do not invent a sign-off from assignments, comments, or already-approved scopes. ## Security source diff --git a/CHANGELOG.md b/CHANGELOG.md index eea696893..9244d3f83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first pending approval in the mounted rehearsal workspace so the room can sign off the scope already waiting; the Open action moves to the uniquely named rendered map section, while assignments, comments, already-approved scopes, or inherited runtime metadata remain 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..e0e484eb7 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 pending approval and opens the uniquely named rendered map section. Do not invent a sign-off from assignments, comments, or already-approved scopes. `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/FirstApprovalCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstApprovalCallout.particle.test.tsx new file mode 100644 index 000000000..27abb721c --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstApprovalCallout.particle.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 { FirstApprovalCallout } from "./FirstApprovalCallout"; + +describe("FirstApprovalCallout Korean owner copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending owner names particle-safe before and after the approval action", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = createDemoRehearsalSong(); + song.collaboration!.approvals[0]!.owner = "미나"; + + 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 벌스의 Verse harmony pass 승인을 기다리고 있습니다.")).toBeTruthy(); + expect(screen.queryByText(/미나이/)).toBeNull(); + expect(screen.queryByText(/미나가/)).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "0:10 벌스 승인 위치 열기" })); + + expect(screen.getByText("0:10 벌스에서 Verse harmony pass 승인을 이어서 하세요.")).toBeTruthy(); + expect(screen.queryByText(/미나과/)).toBeNull(); + expect(screen.queryByText(/미나를/)).toBeNull(); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstApprovalCallout.reduced-motion.test.tsx b/apps/desktop/src/features/workspace/FirstApprovalCallout.reduced-motion.test.tsx new file mode 100644 index 000000000..642c7eb5d --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstApprovalCallout.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 { FirstApprovalCallout } from "./FirstApprovalCallout"; + +describe("FirstApprovalCallout 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 verse approval at 0:10" })); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstApprovalCallout.test.tsx b/apps/desktop/src/features/workspace/FirstApprovalCallout.test.tsx new file mode 100644 index 000000000..6c57a5fc5 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstApprovalCallout.test.tsx @@ -0,0 +1,215 @@ +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 { FirstApprovalCallout } from "./FirstApprovalCallout"; + +function songWithApproval() { + 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("FirstApprovalCallout", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText("No pending approval yet. Stay on tonight's map until a scope needs a sign-off.") + ).toBeTruthy(); + }); + + it("contains a hostile song identity accessor instead of crashing the callout", () => { + const song = songWithApproval(); + 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 verse approval at 0:10" })).toBeTruthy(); + }); + + it("resets armed guidance when accessor-id songs change with the same approval signature", () => { + const firstSong = songWithApproval(); + const nextSong = songWithApproval(); + 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 verse approval at 0:10" })); + expect(screen.getByText(/Keep the verse approval moving at 0:10. Sign it off together./)).toBeTruthy(); + + rerender(); + + expect(screen.getByText("MD still needs to sign off on Verse harmony pass in the verse at 0:10.")).toBeTruthy(); + expect(screen.queryByText(/Keep the verse approval moving at 0:10. Sign it off together./)).toBeNull(); + + grid.remove(); + }); + + it("names the first approval as map navigation, scrolls to its rendered section, and arms that action", () => { + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + expect(screen.getByText("MD still needs to sign off on Verse harmony pass in the verse at 0:10.")).toBeTruthy(); + const action = screen.getByRole("button", { + name: "Open verse approval at 0:10" + }); + expect(action).toBeTruthy(); + fireEvent.click(action); + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect(screen.getByText(/Keep the verse approval moving at 0:10. Sign it off together./)).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 verse approval at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect(screen.getByText(/Keep the verse approval moving at 0:10. Sign it off together./)).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 verse approval at 0:10" })); + + expect(screen.getByText("MD still needs to sign off on Verse harmony pass in the verse at 0:10.")).toBeTruthy(); + expect(screen.queryByText(/Keep the verse approval moving at 0:10. Sign it off together./)).toBeNull(); + }); + + it("navigates by renderer-owned section position instead of untrusted analysis ids", () => { + const song = songWithApproval(); + song.sections[0]!.id = "analysis section / duplicate"; + const { grid, scrollIntoView } = appendSongStructureTarget(); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open verse approval 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 verse approval 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 approval changes or returns later", () => { + const initialSong = songWithApproval(); + const { grid } = appendSongStructureTarget(); + const { rerender } = render(); + fireEvent.click(screen.getByRole("button", { name: "Open verse approval at 0:10" })); + expect(screen.getByText(/Keep the verse approval moving at 0:10. Sign it off together./)).toBeTruthy(); + + const nextSong = songWithApproval(); + nextSong.id = "next-song"; + nextSong.sections[0]!.timeRange = { start: 20, end: 40 }; + rerender(); + expect(screen.getByText("MD still needs to sign off on Verse harmony pass in the verse at 0:20.")).toBeTruthy(); + + grid.remove(); + }); + + it("keeps an unavailable approval guidance-only", () => { + const song = songWithApproval(); + song.collaboration = undefined; + render(); + expect(screen.queryByRole("button")).toBeNull(); + expect( + screen.getByText("No pending approval yet. Stay on tonight's map until a scope needs a sign-off.") + ).toBeTruthy(); + }); + + it("names a band-wide approval when the scope does not uniquely name a section", () => { + const song = songWithApproval(); + song.collaboration!.approvals[0]!.scope = "Whole-set mix pass"; + render(); + expect(screen.queryByRole("button")).toBeNull(); + expect(screen.getByText("MD still needs to sign off on Whole-set mix pass.")).toBeTruthy(); + }); + + it("names a changes-requested approval as the next pass", () => { + const song = songWithApproval(); + song.collaboration!.approvals[0]!.status = "changes_requested"; + render(); + expect(screen.getByText("MD asked for another pass on Verse harmony pass in the verse at 0:10.")).toBeTruthy(); + }); + + it("localizes the approval form label instead of exposing its raw enum in Korean copy", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithApproval(); + + render(); + + expect(screen.getByText("MD님이 0:10 벌스의 Verse harmony pass 승인을 기다리고 있습니다.")).toBeTruthy(); + expect(screen.queryByText(/verse의/)).toBeNull(); + }); + + it("renders the owned approval scope as a text node instead of template syntax", () => { + const song = songWithApproval(); + song.collaboration!.approvals[0]!.scope = "Lock {owner} at {at} in {section}"; + song.collaboration!.approvals[1]!.status = "approved"; + render(); + expect(screen.getByText("MD still needs to sign off on Lock {owner} at {at} in {section}.")).toBeTruthy(); + expect(screen.queryByText("MD still needs to sign off on Lock MD at 0:10 in verse.")).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx b/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx new file mode 100644 index 000000000..dc3e43dbe --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx @@ -0,0 +1,148 @@ +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 { formatApprovalTime, resolveFirstApproval } from "./firstApproval"; + +/** Props for the first-approval rehearsal callout. */ +export interface FirstApprovalCalloutProps { + song: RehearsalSong; +} + +type ApprovalCopyValues = Readonly>; + +type OpenedApproval = Readonly<{ + songIdentity: unknown; + sectionId: string | null; + sectionIndex: number; + approvalId: string; + atSeconds: number | null; +}>; + +/** Interpolate approval placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatApprovalCopy(template: string, values: ApprovalCopyValues): string { + return template.replace(/\{(owner|scope|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof ApprovalCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredApprovalScrollBehavior(): ScrollBehavior { + return typeof window.matchMedia === "function" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches + ? "auto" + : "smooth"; +} + +/** Name tonight's first pending approval and open the matching rendered map section. */ +export function FirstApprovalCallout({ song }: FirstApprovalCalloutProps) { + const locale = detectPreferredLocale(); + const t = createTranslator(locale); + const songIdentity: unknown = song; + const runtimeSong = song as unknown as Partial | null; + const approval = resolveFirstApproval(song); + const approvalSectionIndex = + approval?.section && Array.isArray(runtimeSong?.sections) + ? runtimeSong.sections.indexOf(approval.section) + : -1; + const [openedApproval, setOpenedApproval] = useState(null); + + useEffect(() => { + setOpenedApproval(null); + }, [ + songIdentity, + approvalSectionIndex, + approval?.section?.id, + approval?.approval.id, + approval?.atSeconds + ]); + + if (!approval) { + return ( + + ); + } + + const hasSection = approval.section !== null && approval.atSeconds !== null; + const opened = + openedApproval !== null && + openedApproval.songIdentity === songIdentity && + openedApproval.sectionId === (approval.section?.id ?? null) && + openedApproval.sectionIndex === approvalSectionIndex && + openedApproval.approvalId === approval.approval.id && + openedApproval.atSeconds === approval.atSeconds; + const at = hasSection ? formatApprovalTime(approval.atSeconds ?? 0) : ""; + const copyValues: ApprovalCopyValues = { + owner: approval.approval.owner, + scope: approval.scope, + section: approval.section ? translateSectionFormLabel(locale, approval.section.label) : "", + at + }; + const isChangesRequested = approval.approval.status === "changes_requested"; + const bodyKey = hasSection + ? isChangesRequested + ? "firstApprovalBodyChanges" + : "firstApprovalBody" + : isChangesRequested + ? "firstApprovalBodyChangesBand" + : "firstApprovalBodyBand"; + const armedKey = hasSection ? "firstApprovalArmed" : "firstApprovalArmedBand"; + const body = formatApprovalCopy(t(bodyKey), copyValues); + const armed = formatApprovalCopy(t(armedKey), copyValues); + const actionLabel = hasSection + ? formatApprovalCopy(t("firstApprovalOpenAction"), copyValues) + : ""; + + return ( + + ); +} diff --git a/apps/desktop/src/features/workspace/Workspace.test.tsx b/apps/desktop/src/features/workspace/Workspace.test.tsx index a3da5ffe6..2341db81b 100644 --- a/apps/desktop/src/features/workspace/Workspace.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.test.tsx @@ -137,7 +137,7 @@ describe("Workspace", () => { expect(screen.getByText(/The bass holds the vi center/i)).toBeTruthy(); 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(); + expect(screen.getAllByText(/Verse harmony pass/i).length).toBeGreaterThan(0); }); it("falls back from blank planning copy and tolerates partial collaboration payloads", () => { @@ -270,4 +270,15 @@ describe("Workspace", () => { expect(screen.getByText("합주 우선순위")).toBeTruthy(); expect(screen.getByText("역할과 화성")).toBeTruthy(); }); + + it("names tonight's first pending approval on the mounted map", () => { + setNavigatorLanguage("en-US"); + render(); + + expect(screen.getByText("Tonight's first approval")).toBeTruthy(); + expect( + screen.getByText("MD still needs to sign off on Verse harmony pass in the verse at 0:10.") + ).toBeTruthy(); + expect(screen.getByRole("button", { name: "Open verse approval at 0:10" })).toBeTruthy(); + }); }); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 71546b524..13538f02d 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 { FirstApprovalCallout } from "./FirstApprovalCallout"; import { createTranslator, detectPreferredLocale } from "../../i18n"; import { generateCueSheetCsv, generateChartSummaryJson, generateMetadataHandoffJson, sanitizeFilename } from "../../lib/export"; import { Button } from "@/components/ui/button"; @@ -331,6 +332,8 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp + +
diff --git a/apps/desktop/src/features/workspace/firstApproval.inherited-metadata.test.ts b/apps/desktop/src/features/workspace/firstApproval.inherited-metadata.test.ts new file mode 100644 index 000000000..5473d65af --- /dev/null +++ b/apps/desktop/src/features/workspace/firstApproval.inherited-metadata.test.ts @@ -0,0 +1,107 @@ +import { createDemoRehearsalSong, type RehearsalApproval } from "@bandscope/shared-types"; +import { describe, expect, it } from "vitest"; +import { resolveFirstApproval } from "./firstApproval"; + +function songWithApproval() { + const song = createDemoRehearsalSong(); + const section = structuredClone(song.sections[0]!); + section.id = "approve-own"; + song.sections = [section]; + song.collaboration = { + syncMode: "local_only", + syncNote: "Keep approvals local for now.", + assignments: [], + comments: [], + approvals: [ + { + id: "approval-harmony-pass", + scope: "Verse harmony pass", + owner: "MD", + status: "pending" + } + ] + }; + return { song, section }; +} + +describe("resolveFirstApproval inherited metadata", () => { + it("rejects a song or collaboration whose required metadata is inherited", () => { + const { song } = songWithApproval(); + const inheritedSong = Object.create({ collaboration: song.collaboration, sections: song.sections }) as typeof song; + expect(resolveFirstApproval(inheritedSong)).toBeNull(); + + const inheritedCollaboration = Object.create(song.collaboration!) as NonNullable; + song.collaboration = inheritedCollaboration; + expect(resolveFirstApproval(song)).toBeNull(); + }); + + it("rejects inherited approval fields", () => { + const { song } = songWithApproval(); + song.collaboration!.approvals = [ + Object.create(song.collaboration!.approvals[0]!) as (typeof song.collaboration.approvals)[number] + ]; + expect(resolveFirstApproval(song)).toBeNull(); + }); + + it("rejects inherited timing fields when a unique section is required", () => { + const { song, section } = songWithApproval(); + section.timeRange = Object.create({ start: 10, end: 30 }) as typeof section.timeRange; + const resolved = resolveFirstApproval(song); + expect(resolved?.approval.id).toBe("approval-harmony-pass"); + expect(resolved?.section).toBeNull(); + expect(resolved?.atSeconds).toBeNull(); + }); + + it("contains exceptions from own runtime accessors instead of trusting them", () => { + const { song } = songWithApproval(); + Object.defineProperty(song.collaboration!.approvals[0]!, "scope", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile scope getter"); + } + }); + + expect(() => resolveFirstApproval(song)).not.toThrow(); + expect(resolveFirstApproval(song)).toBeNull(); + }); + + it("does not treat own accessors as stable approval identity authority", () => { + const { song } = songWithApproval(); + Object.defineProperty(song.collaboration!.approvals[0]!, "id", { + configurable: true, + enumerable: true, + get() { + return "approval-harmony-pass"; + } + }); + + expect(resolveFirstApproval(song)).toBeNull(); + }); + + it("does not let inherited section metadata host the approval", () => { + const { song, section } = songWithApproval(); + const inheritedSection = Object.create(section) as typeof section; + song.sections = [inheritedSection]; + const resolved = resolveFirstApproval(song); + expect(resolved?.approval.id).toBe("approval-harmony-pass"); + expect(resolved?.section).toBeNull(); + }); + + it("rejects arrays masquerading as section records", () => { + const { song, section } = songWithApproval(); + const arraySection = Object.assign([], section) as unknown as typeof section; + song.sections = [arraySection]; + const resolved = resolveFirstApproval(song); + expect(resolved?.approval.id).toBe("approval-harmony-pass"); + expect(resolved?.section).toBeNull(); + }); + + it("rejects sparse approval arrays", () => { + const { song } = songWithApproval(); + const sparse: RehearsalApproval[] = []; + sparse[1] = song.collaboration!.approvals[0]!; + song.collaboration!.approvals = sparse; + expect(resolveFirstApproval(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstApproval.test.ts b/apps/desktop/src/features/workspace/firstApproval.test.ts new file mode 100644 index 000000000..3b248c3d8 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstApproval.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, it } from "vitest"; +import { + createDemoRehearsalSong, + type RehearsalApproval, + type SectionFormLabel +} from "@bandscope/shared-types"; +import { formatApprovalTime, resolveFirstApproval } from "./firstApproval"; + +function withApproval( + overrides: { + approvalId?: string; + scope?: string; + owner?: string; + status?: RehearsalApproval["status"]; + sectionId?: string; + start?: number; + end?: number; + label?: SectionFormLabel; + } = {} +) { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const section = structuredClone(verse); + section.id = overrides.sectionId ?? "verse-approve"; + section.label = overrides.label ?? "verse"; + section.timeRange = { start: overrides.start ?? 10, end: overrides.end ?? 30 }; + song.sections = [section]; + song.collaboration = { + syncMode: "local_only", + syncNote: "Keep approvals local for now.", + assignments: [], + comments: [], + approvals: [ + { + id: overrides.approvalId ?? "approval-harmony-pass", + scope: overrides.scope ?? "Verse harmony pass", + owner: overrides.owner ?? "MD", + status: overrides.status ?? "pending" + } + ] + }; + return song; +} + +describe("resolveFirstApproval", () => { + it("picks the demo song's pending verse harmony sign-off", () => { + const resolved = resolveFirstApproval(createDemoRehearsalSong()); + expect(resolved?.approval.id).toBe("approval-harmony-pass"); + expect(resolved?.approval.owner).toBe("MD"); + expect(resolved?.scope).toBe("Verse harmony pass"); + expect(resolved?.section?.id).toBe("verse-1"); + expect(resolved?.atSeconds).toBe(10); + expect(formatApprovalTime(resolved?.atSeconds ?? -1)).toBe("0:10"); + expect(formatApprovalTime(Number.NaN)).toBe("0:00"); + expect(formatApprovalTime(-4)).toBe("0:00"); + }); + + it("does not invent an approval from assignments, comments, approved scopes, or empty scope", () => { + const song = withApproval({ scope: " " }); + song.collaboration!.assignments = [ + { + id: "assign-bass-entrance", + assignee: "Rhythm Section", + summary: "Lock the bass entrance against the pickup so the chorus lift lands together.", + sectionId: song.sections[0]!.id, + roleId: "bass-guitar", + status: "in_progress" + } + ]; + song.collaboration!.comments = [ + { + id: "comment-keys-color", + author: "MD", + body: "Keep the keyboard color tone gentle on the first pass so the vocal cue stays forward.", + sectionId: song.sections[0]!.id, + roleId: "keys-right", + status: "open" + } + ]; + song.collaboration!.approvals = [ + { + id: "approval-vocal-shape", + scope: "Lead vocal simplification", + owner: "Lead Vocal", + status: "approved" + } + ]; + expect(resolveFirstApproval(song)).toBeNull(); + }); + + it("does not treat an empty or whitespace scope as a named approval", () => { + expect(resolveFirstApproval(withApproval({ scope: "" }))).toBeNull(); + expect(resolveFirstApproval(withApproval({ scope: " \n\t " }))).toBeNull(); + }); + + it("skips already-approved scopes instead of treating them as tonight's next action", () => { + expect(resolveFirstApproval(withApproval({ status: "approved" }))).toBeNull(); + }); + + it("prefers a changes-requested approval over an earlier pending one", () => { + const song = withApproval({ + approvalId: "approval-late", + start: 40, + end: 56, + label: "chorus", + status: "changes_requested", + scope: "Chorus lift pass" + }); + const earlier = structuredClone(song.sections[0]!); + earlier.id = "verse-early"; + earlier.label = "verse"; + earlier.timeRange = { start: 8, end: 24 }; + song.sections = [song.sections[0]!, earlier]; + song.collaboration!.approvals = [ + song.collaboration!.approvals[0]!, + { + id: "approval-early-pending", + scope: "Verse harmony pass", + owner: "Lead Vocal", + status: "pending" + } + ]; + + const resolved = resolveFirstApproval(song); + expect(resolved?.approval.id).toBe("approval-late"); + expect(resolved?.atSeconds).toBe(40); + }); + + it("prefers the earlier of two pending approvals", () => { + const song = withApproval({ + approvalId: "approval-late", + start: 40, + end: 56, + label: "chorus", + scope: "Chorus lift pass" + }); + const earlier = structuredClone(song.sections[0]!); + earlier.id = "verse-early"; + earlier.label = "verse"; + earlier.timeRange = { start: 8, end: 24 }; + song.sections = [song.sections[0]!, earlier]; + song.collaboration!.approvals = [ + song.collaboration!.approvals[0]!, + { + id: "approval-early", + scope: "Verse harmony pass", + owner: "Lead Vocal", + status: "pending" + } + ]; + + const resolved = resolveFirstApproval(song); + expect(resolved?.approval.id).toBe("approval-early"); + expect(resolved?.atSeconds).toBe(8); + }); + + it("does not invent a section from chorus when the scope names pre-chorus", () => { + const song = withApproval({ + scope: "Pre-chorus lift pass", + label: "pre-chorus", + start: 24, + end: 32 + }); + const chorus = structuredClone(song.sections[0]!); + chorus.id = "chorus-1"; + chorus.label = "chorus"; + chorus.timeRange = { start: 40, end: 56 }; + song.sections = [song.sections[0]!, chorus]; + + const resolved = resolveFirstApproval(song); + expect(resolved?.section?.id).toBe("verse-approve"); + expect(resolved?.section?.label).toBe("pre-chorus"); + expect(resolved?.atSeconds).toBe(24); + }); + + it("keeps the approval band-wide when two verse sections share the named form", () => { + const song = withApproval({ scope: "Verse harmony pass" }); + const second = structuredClone(song.sections[0]!); + second.id = "verse-2"; + second.timeRange = { start: 40, end: 56 }; + song.sections = [song.sections[0]!, second]; + + const resolved = resolveFirstApproval(song); + expect(resolved?.approval.id).toBe("approval-harmony-pass"); + expect(resolved?.section).toBeNull(); + expect(resolved?.atSeconds).toBeNull(); + }); + + it("does not invent a section from assignments, comments, or Korean scope tokens", () => { + const song = withApproval({ scope: "벌스 화성 패스" }); + expect(resolveFirstApproval(song)?.section).toBeNull(); + expect(resolveFirstApproval(song)?.scope).toBe("벌스 화성 패스"); + }); + + it("bounds a long owned scope without splitting a surrogate pair", () => { + const song = withApproval({ + scope: `${"a".repeat(179)}\uD83D\uDE80trailing` + }); + expect(resolveFirstApproval(song)?.scope).toBe(`${"a".repeat(179)}\uD83D\uDE80`); + }); + + it("ties equal pending times with a stable id", () => { + const song = withApproval({ approvalId: "z-late", scope: "Verse later pass" }); + song.collaboration!.approvals = [ + song.collaboration!.approvals[0]!, + { + id: "a-early", + scope: "Verse earlier pass", + owner: "MD", + status: "pending" + } + ]; + expect(resolveFirstApproval(song)?.approval.id).toBe("a-early"); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstApproval.ts b/apps/desktop/src/features/workspace/firstApproval.ts new file mode 100644 index 000000000..4f94693c4 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstApproval.ts @@ -0,0 +1,290 @@ +import { + MAX_SECTION_TIME_SECONDS, + SECTION_FORM_LABELS, + type CollaborationApprovalStatus, + type RehearsalApproval, + type RehearsalSection, + type RehearsalSong, + type SectionFormLabel +} from "@bandscope/shared-types"; + +const ACTIONABLE_STATUS_RANK = { changes_requested: 0, pending: 1 } as const; +const MAX_APPROVAL_SCOPE_CHARACTERS = 180; +const FORM_LABELS_BY_LENGTH = [...SECTION_FORM_LABELS].sort((left, right) => right.length - left.length); + +/** Tonight's first named approval: the earliest owned sign-off and the unique section it names. */ +export type FirstApproval = { + section: RehearsalSection | null; + approval: RehearsalApproval; + atSeconds: number | null; + scope: string; +}; + +/** Format a non-negative approval time as m:ss for rehearsal copy. */ +export function formatApprovalTime(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 owned approval scope, or null when the field cannot be shown. */ +function ownedApprovalScope(approval: RehearsalApproval): string | null { + if (!hasOwnData(approval, "scope") || typeof approval.scope !== "string") { + return null; + } + const scope = approval.scope.trim(); + if (scope.length === 0) { + return null; + } + return truncateCodePoints(scope, MAX_APPROVAL_SCOPE_CHARACTERS); +} + +/** Return true when the approval owns identity, owner, status, and a named scope. */ +function isActionableApproval(approval: RehearsalApproval): boolean { + return ( + isRuntimeObject(approval) && + hasOwnData(approval, "id") && + typeof approval.id === "string" && + approval.id.trim().length > 0 && + hasOwnData(approval, "owner") && + typeof approval.owner === "string" && + approval.owner.trim().length > 0 && + hasOwnData(approval, "status") && + Object.prototype.hasOwnProperty.call(ACTIONABLE_STATUS_RANK, approval.status) && + ownedApprovalScope(approval) !== null + ); +} + +/** 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 + ); +} + +/** Map unique owned ids onto their records; duplicated ids are not authority. */ +function uniqueOwnedById( + items: T[], + readId: (item: T) => string | null +): Map { + const unique = new Map(); + const repeated = new Set(); + for (const item of items) { + if (!isRuntimeObject(item)) { + continue; + } + const id = readId(item); + if (id === null || repeated.has(id)) { + continue; + } + if (unique.has(id)) { + unique.delete(id); + repeated.add(id); + continue; + } + unique.set(id, item); + } + return unique; +} + +/** Collect canonical form labels that appear as whole tokens in an owned scope. */ +function matchedFormLabels(scope: string): Set { + const normalized = scope.toLowerCase(); + const occupied = Array.from({ length: normalized.length }, () => false); + const matched = new Set(); + + for (const label of FORM_LABELS_BY_LENGTH) { + let from = 0; + while (from <= normalized.length - label.length) { + const index = normalized.indexOf(label, from); + if (index === -1) { + break; + } + const beforeOk = index === 0 || /[^a-z]/.test(normalized[index - 1] ?? ""); + const afterIndex = index + label.length; + const afterOk = afterIndex === normalized.length || /[^a-z]/.test(normalized[afterIndex] ?? ""); + let alreadyOccupied = false; + for (let cursor = index; cursor < afterIndex; cursor += 1) { + if (occupied[cursor]) { + alreadyOccupied = true; + break; + } + } + if (beforeOk && afterOk && !alreadyOccupied) { + matched.add(label); + for (let cursor = index; cursor < afterIndex; cursor += 1) { + occupied[cursor] = true; + } + } + from = index + 1; + } + } + + return matched; +} + +/** Return owned sections that can host an approval. */ +function uniqueReadySections(song: RehearsalSong): Map { + if (!isRuntimeObject(song) || !hasOwnData(song, "sections") || !isDenseRuntimeArray(song.sections)) { + return new Map(); + } + + return uniqueOwnedById( + 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) + ), + (section) => (hasOwnData(section, "id") && typeof section.id === "string" ? section.id : null) + ); +} + +/** Return the unique ready section named by the owned scope, or null when the pointer is ambiguous. */ +function resolveNamedSection(scope: string, sections: Map): RehearsalSection | null { + const labels = matchedFormLabels(scope); + if (labels.size === 0) { + return null; + } + const named = [...sections.values()].filter( + (section) => hasOwnData(section, "label") && labels.has(section.label) + ); + return named.length === 1 ? (named[0] ?? null) : null; +} + +/** Resolve an approval after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstApproval(song: RehearsalSong): FirstApproval | null { + if ( + !isRuntimeObject(song) || + !hasOwnData(song, "collaboration") || + !isRuntimeObject(song.collaboration) || + !hasOwnData(song.collaboration, "approvals") || + !isDenseRuntimeArray(song.collaboration.approvals) + ) { + return null; + } + + const sections = uniqueReadySections(song); + const uniqueApprovals = uniqueOwnedById( + song.collaboration.approvals.filter((approval) => isActionableApproval(approval)), + (approval) => (hasOwnData(approval, "id") && typeof approval.id === "string" ? approval.id : null) + ); + + const candidates = [...uniqueApprovals.values()] + .flatMap((approval) => { + const scope = ownedApprovalScope(approval); + if (scope === null) { + return []; + } + const section = resolveNamedSection(scope, sections); + return [ + { + section, + approval, + atSeconds: section ? section.timeRange.start : null, + scope + } + ]; + }) + .sort((left, right) => { + const statusDelta = + ACTIONABLE_STATUS_RANK[left.approval.status as Exclude] - + ACTIONABLE_STATUS_RANK[right.approval.status as Exclude]; + if (statusDelta !== 0) { + return statusDelta; + } + const leftTime = left.atSeconds ?? Number.POSITIVE_INFINITY; + const rightTime = right.atSeconds ?? Number.POSITIVE_INFINITY; + if (leftTime !== rightTime) { + return leftTime - rightTime; + } + return compareStableId(left.approval.id, right.approval.id); + }); + + return candidates[0] ?? null; +} + +/** Return the first named approval, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstApproval(song: RehearsalSong): FirstApproval | null { + try { + return resolveSafeFirstApproval(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..6212c0a0a 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,21 @@ describe("i18n", () => { } }); }); + + describe("translateSectionFormLabel", () => { + it("localizes Korean section form labels without treating inherited keys as labels", () => { + expect(translateSectionFormLabel("ko", "verse")).toBe("벌스"); + expect(translateSectionFormLabel("ko", "pre-chorus")).toBe("프리코러스"); + expect(translateSectionFormLabel("en", "verse")).toBe("verse"); + const inheritedKey = "toString" as never; + expect(translateSectionFormLabel("ko", inheritedKey)).toBe("toString"); + }); + + it("keeps Korean first-approval next-action copy particle-safe", () => { + const t = createTranslator("ko"); + expect(t("firstApprovalOpenAction")).toBe("{at} {section} 승인 위치 열기"); + expect(t("firstApprovalBody")).toBe("{owner}님이 {at} {section}의 {scope} 승인을 기다리고 있습니다."); + expect(t("firstApprovalArmed")).toBe("{at} {section}에서 {scope} 승인을 이어서 하세요."); + }); + }); }); diff --git a/apps/desktop/src/i18n/index.ts b/apps/desktop/src/i18n/index.ts index 1a9f471f0..28d9bfd95 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,14 +12,47 @@ 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]; }; } -/** Documented. */ +/** 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); +} + +/** Detect Korean from the runtime navigator, otherwise English. */ export function detectPreferredLocale(): Locale { if (typeof navigator !== "undefined" && navigator.language?.toLowerCase().startsWith("ko")) { return "ko"; diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index 39f716d50..91878f30b 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -148,5 +148,15 @@ "practiceProgressRegionLabel": "Practice Progress", "practiceProgressLabel": "Practice Progress", "decreasePracticeProgressLabel": "Decrease progress", - "increasePracticeProgressLabel": "Increase progress" + "increasePracticeProgressLabel": "Increase progress", + "firstApprovalLabel": "Tonight's first approval", + "firstApprovalOpenAction": "Open {section} approval at {at}", + "firstApprovalOpenActionBand": "Open the first approval", + "firstApprovalBody": "{owner} still needs to sign off on {scope} in the {section} at {at}.", + "firstApprovalBodyBand": "{owner} still needs to sign off on {scope}.", + "firstApprovalBodyChanges": "{owner} asked for another pass on {scope} in the {section} at {at}.", + "firstApprovalBodyChangesBand": "{owner} asked for another pass on {scope}.", + "firstApprovalArmed": "Keep the {section} approval moving at {at}. Sign it off together.", + "firstApprovalArmedBand": "Keep the approval moving. Sign it off together.", + "firstApprovalUnavailable": "No pending approval yet. Stay on tonight's map until a scope needs a sign-off." } diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 371884abb..010a6ff2f 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -148,5 +148,15 @@ "practiceProgressRegionLabel": "연습 진척도", "practiceProgressLabel": "연습 진척도", "decreasePracticeProgressLabel": "진척도 감소", - "increasePracticeProgressLabel": "진척도 증가" + "increasePracticeProgressLabel": "진척도 증가", + "firstApprovalLabel": "오늘 첫 승인", + "firstApprovalOpenAction": "{at} {section} 승인 위치 열기", + "firstApprovalOpenActionBand": "첫 승인 위치 열기", + "firstApprovalBody": "{owner}님이 {at} {section}의 {scope} 승인을 기다리고 있습니다.", + "firstApprovalBodyBand": "{owner}님이 {scope} 승인을 기다리고 있습니다.", + "firstApprovalBodyChanges": "{owner}님이 {at} {section}의 {scope}를 다시 봐 달라고 했습니다.", + "firstApprovalBodyChangesBand": "{owner}님이 {scope}를 다시 봐 달라고 했습니다.", + "firstApprovalArmed": "{at} {section}에서 {scope} 승인을 이어서 하세요.", + "firstApprovalArmedBand": "{scope} 승인을 이어서 하세요.", + "firstApprovalUnavailable": "아직 대기 중인 승인이 없습니다. 범위가 사인오프될 때까지 오늘 지도에 머무르세요." } diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 22602c313..0f4042263 100644 --- a/docs/design-system/component-contract.md +++ b/docs/design-system/component-contract.md @@ -35,6 +35,7 @@ The authoritative Figma view is `31 Component Contract Catalog`. This file mirro | 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()`. | +| First Approval Callout | workspace next-action pattern | `apps/desktop/src/features/workspace/FirstApprovalCallout.tsx` | Name the owner, the owned scope, and the uniquely named labeled section and time. Prefer `changes_requested` over `pending`. Do not invent a sign-off from assignments, comments, already-approved scopes, empty/whitespace scopes, or inherited runtime metadata. Open scrolls the renderer-owned song-structure section when the owned scope uniquely names a form label. Keep the unavailable and band-wide states guidance-only. Distinct from first-assignment (#996), first-open-comment (#997), first-count (#995), first-lyric (#913), and export-and-priority (#900). | ## Prop And State Mapping diff --git a/docs/doctoring/reduced-motion-first-approval-navigation.md b/docs/doctoring/reduced-motion-first-approval-navigation.md new file mode 100644 index 000000000..192db607d --- /dev/null +++ b/docs/doctoring/reduced-motion-first-approval-navigation.md @@ -0,0 +1,14 @@ +# Reduced-motion first-approval navigation + +Workspace map navigation for tonight's first pending approval follows the operating-system reduced-motion preference. + +When `prefers-reduced-motion: reduce` matches, `FirstApprovalCallout` scrolls the renderer-owned song-structure section with `behavior: "auto"`. Otherwise it uses `behavior: "smooth"`. + +This is a presentation contract only. Approval resolution and analysis-id isolation stay unchanged. + +## Security Notes + +- Untrusted input: song, collaboration, approval identity/owner/scope/status, section, time-range, and form-label tokens inside an owned scope are runtime data; inherited properties and arrays masquerading as record metadata are not authority. +- Trust boundary: approval resolution accepts required fields only when the inspected record owns them, while renderer-owned song-structure children remain the only navigation targets; analysis `section.id` is never DOM-ID authority. The owned approval scope is interpolated once as copy and is never rescanned as template syntax. Assignments, comments, and already-approved scopes cannot invent a pending approval. Canonical English form-label tokens may uniquely name a section; Korean or free-text scope copy cannot invent navigation. +- Mitigations: runtime record guards reject arrays, dense collections require own indexed elements, required metadata fields must be own properties, `matchMedia` is read-only, scroll targets come from renderer child index, copy interpolation runs once, and the approval scope is bounded to 180 Unicode code points. +- Test points: inherited song/collaboration/approval/section/timing metadata is rejected, array-backed section records are rejected, reduced-motion scroll uses `auto`, and default motion uses `smooth`. From ab8e2c859b21f890aed3e9e4ae1c99aa70340d19 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 08:15:21 -0700 Subject: [PATCH 02/10] test(workspace): reproduce approval navigation and scope defects --- ...pprovalCallout.review-regressions.test.tsx | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 apps/desktop/src/features/workspace/FirstApprovalCallout.review-regressions.test.tsx diff --git a/apps/desktop/src/features/workspace/FirstApprovalCallout.review-regressions.test.tsx b/apps/desktop/src/features/workspace/FirstApprovalCallout.review-regressions.test.tsx new file mode 100644 index 000000000..e5ce5d6ae --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstApprovalCallout.review-regressions.test.tsx @@ -0,0 +1,62 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstApprovalCallout } from "./FirstApprovalCallout"; +import { resolveFirstApproval } from "./firstApproval"; +import { Workspace } from "./Workspace"; + +describe("FirstApprovalCallout current review regressions", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("opens the real song-structure cell rendered by Workspace", () => { + const song = createDemoRehearsalSong(); + render(); + const renderer = screen.getByTestId("song-structure-grid"); + const firstCell = renderer.children.item(0) as HTMLElement | null; + expect(firstCell).not.toBeNull(); + const scrollIntoView = vi.fn(); + Object.defineProperty(firstCell!, "scrollIntoView", { + configurable: true, + value: scrollIntoView + }); + + fireEvent.click(screen.getByRole("button", { name: "Open verse approval at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + }); + + it("keeps scopes naming multiple canonical form labels band-wide", () => { + const song = createDemoRehearsalSong(); + song.sections = [song.sections[0]!]; + song.collaboration!.approvals[0]!.scope = "Verse / Chorus transition"; + + const approval = resolveFirstApproval(song); + + expect(approval).not.toBeNull(); + expect(approval?.section).toBeNull(); + expect(approval?.atSeconds).toBeNull(); + }); + + it("keeps the pending owner and scope visible after opening the approval", () => { + const song = createDemoRehearsalSong(); + const grid = document.createElement("div"); + grid.dataset.testid = "song-structure-grid"; + const target = document.createElement("div"); + 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 verse approval at 0:10" })); + + expect(screen.getByText(/MD still needs to sign off on Verse harmony pass/i)).toBeTruthy(); + expect(screen.getByText(/Keep the verse approval moving at 0:10/i)).toBeTruthy(); + grid.remove(); + }); +}); From 3ef4b75449594de9e9c5dc0538e40c284373b6f4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 08:15:58 -0700 Subject: [PATCH 03/10] fix(workspace): make approval Open use rendered section order --- .../src/features/workspace/FirstApprovalCallout.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx b/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx index dc3e43dbe..d51954fd0 100644 --- a/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstApprovalCallout.tsx @@ -112,18 +112,17 @@ export function FirstApprovalCallout({ song }: FirstApprovalCalloutProps) { aria-label={t("firstApprovalLabel")} >

{t("firstApprovalLabel")}

-

{opened ? armed : body}

+

{body}

+ {opened ?

{armed}

: null} {hasSection ? (