From 1f6c68d44389c049c7edb6dc1d08730117414709 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 16:22:13 +0000 Subject: [PATCH 1/5] feat(workspace): name the next action after project load or save fails Keep an open rehearsal map visible when save fails, and name Choose another project / Try saving again instead of the analysis error card. Security Notes: - Untrusted input is project file picker results and native load/save errors. - Trust boundary: Tauri load_project/save_project -> redacted error copy -> recovery action. - Recovery reuses the existing load/save pickers; it does not follow paths or start analysis. - Privacy: paths, URLs, and secret-shaped diagnostics stay redacted. --- AGENTS.md | 1 + ARCHITECTURE.md | 1 + CHANGELOG.md | 1 + CLAUDE.md | 2 +- apps/desktop/src/App.test.tsx | 255 ++++++++++++++++++ apps/desktop/src/App.tsx | 46 +++- .../features/workspace/WorkspaceStates.tsx | 41 +++ apps/desktop/src/locales/en/common.json | 4 + apps/desktop/src/locales/ko/common.json | 4 + docs/design-system/component-contract.md | 2 +- docs/design-system/product-design-handoff.md | 5 +- 11 files changed, 354 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fca448ce9..950265039 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,7 @@ This section applies to any agent (Claude, Codex, Cursor, opencode, ...) working - Do not add network-dependent runtime paths for local analysis. - Treat YouTube import as policy-constrained and fallback-friendly. - Treat files, URLs, metadata, model artifacts, and project files as untrusted input. +- Project load/save failures stay fail-closed and redacted, name the next local file action, and must not replace an open rehearsal map with the analysis error card. - Do not add generic exec/read/write APIs. - Use `shell=False`-style subprocess invocation with argument arrays only. - Keep local backend access on allowlisted IPC or `127.0.0.1` only, with strict schema validation. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3302a6fc3..445ad0327 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -115,6 +115,7 @@ Last updated: 2026-03-11 - Local audio intake bootstraps a project by validating a user-selected file in Rust, creating app-owned temp/cache/project roots, and referencing the original source file rather than copying it in this phase. - Those bootstrap roots should resolve from app-owned Tauri data/cache paths instead of the shared system temp namespace. - Product and UX decisions should prefer rehearsal-first simplicity while still maintaining high analytical accuracy. +- Project load and save failures keep the current rehearsal map when one exists and name **Choose another project** or **Try saving again** instead of replacing the workspace with the analysis error card. - Security decisions should prefer allowlisted narrow capabilities over generic convenience APIs. ## Verification model diff --git a/CHANGELOG.md b/CHANGELOG.md index eea696893..ad496b9fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Display the analyzed song tempo (BPM) as a badge in the rehearsal workspace. - 각 합주 역할(Role)별 개인 연습 진행도를 0~100% 범위로 기록 및 시각화할 수 있는 연습 진척도(`practiceProgress`) 트래커 기능 추가. UI 컨트롤(슬라이더 및 +/- 버튼)과 한/영 다국어 지원 포함. +- Name **Choose another project** after a project file fails to open, and **Try saving again** after tonight's rehearsal map fails to save, without hiding a song that is already open. ## [0.1.3] - 2026-04-29 diff --git a/CLAUDE.md b/CLAUDE.md index 82c2c704a..dc88bc515 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). `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. Project load/save failures keep an open rehearsal map visible and name **Choose another project** / **Try saving again**. - `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.test.tsx b/apps/desktop/src/App.test.tsx index 3eed386f8..2bc3506a3 100644 --- a/apps/desktop/src/App.test.tsx +++ b/apps/desktop/src/App.test.tsx @@ -1286,6 +1286,9 @@ describe("App", () => { await waitFor(() => { expect(screen.getByText(/Failed to load project: Corrupt file/i)).toBeTruthy(); }); + expect(screen.getByRole("heading", { name: /That project file can't open tonight/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + expect(screen.queryByText(/An error occurred during analysis/i)).toBeNull(); }); it("ignores cancellation when loading a project", async () => { @@ -1393,6 +1396,10 @@ describe("App", () => { await waitFor(() => { expect(screen.getByText(/Failed to save project: Permission denied/i)).toBeTruthy(); }); + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + expect(screen.getByRole("heading", { name: /Tonight's rehearsal map couldn't be saved/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /Try saving again/i })).toBeTruthy(); + expect(screen.queryByText(/An error occurred during analysis/i)).toBeNull(); }); it("ignores cancellation when saving a project with Error object", async () => { @@ -1536,6 +1543,254 @@ describe("App", () => { expect(mockSaveProject).not.toHaveBeenCalled(); }); + it("names Choose another project and retries a failed load from the recovery action", async () => { + mockLoadProject + .mockRejectedValueOnce(new Error("Corrupt file")) + .mockResolvedValueOnce(succeededResult().result); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /Choose another project/i })); + + await waitFor(() => { + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + expect(mockLoadProject).toHaveBeenCalledTimes(2); + expect(screen.queryByText(/That project file can't open tonight/i)).toBeNull(); + expect(screen.queryByText(/Failed to load project/i)).toBeNull(); + }); + + it("keeps Choose another project available when the replacement picker is cancelled", async () => { + mockLoadProject + .mockRejectedValueOnce(new Error("Corrupt file")) + .mockRejectedValueOnce(new Error("User cancelled")); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /Choose another project/i })); + + await waitFor(() => { + expect(mockLoadProject).toHaveBeenCalledTimes(2); + }); + expect(screen.getByRole("heading", { name: /That project file can't open tonight/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + }); + + it("keeps the open rehearsal map when opening another project fails", async () => { + mockLoadProject + .mockResolvedValueOnce(succeededResult().result) + .mockRejectedValueOnce(new Error("Corrupt file")); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByText(/Failed to load project: Corrupt file/i)).toBeTruthy(); + }); + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + expect(screen.queryByText(/An error occurred during analysis/i)).toBeNull(); + }); + + it("retries a failed save from Try saving again and clears recovery after success", async () => { + mockLoadProject.mockResolvedValueOnce(succeededResult().result); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + + mockSaveProject + .mockRejectedValueOnce(new Error("Permission denied")) + .mockResolvedValueOnce(undefined); + + fireEvent.click(screen.getByRole("button", { name: /save project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Try saving again/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /Try saving again/i })); + + await waitFor(() => { + expect(mockSaveProject).toHaveBeenCalledTimes(2); + }); + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + expect(screen.queryByText(/Tonight's rehearsal map couldn't be saved/i)).toBeNull(); + expect(screen.queryByText(/Failed to save project/i)).toBeNull(); + }); + + it("keeps Try saving again available when the save picker is cancelled", async () => { + mockLoadProject.mockResolvedValueOnce(succeededResult().result); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + + mockSaveProject + .mockRejectedValueOnce(new Error("Permission denied")) + .mockRejectedValueOnce(new Error("User cancelled")); + + fireEvent.click(screen.getByRole("button", { name: /save project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Try saving again/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /Try saving again/i })); + + await waitFor(() => { + expect(mockSaveProject).toHaveBeenCalledTimes(2); + }); + expect(screen.getByRole("heading", { name: /Tonight's rehearsal map couldn't be saved/i })).toBeTruthy(); + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + + it("does not put secret-shaped save diagnostics in the recovery title or action", async () => { + mockLoadProject.mockResolvedValueOnce(succeededResult().result); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + + mockSaveProject.mockRejectedValueOnce( + new Error("token=secret-token https://example.com/leak /Users/seongho/private.band") + ); + + fireEvent.click(screen.getByRole("button", { name: /save project/i })); + + await waitFor(() => { + expect(screen.getByRole("button", { name: /Try saving again/i })).toBeTruthy(); + }); + const title = screen.getByRole("heading", { name: /Tonight's rehearsal map couldn't be saved/i }); + const action = screen.getByRole("button", { name: /Try saving again/i }); + expect(title.textContent).not.toMatch(/secret-token|example\.com|\/Users\/seongho/i); + expect(action.textContent).not.toMatch(/secret-token|example\.com|\/Users\/seongho/i); + expect(screen.getByRole("alert").textContent).not.toMatch(/secret-token/i); + }); + + it("clears a failed load card when the musician chooses local audio instead", async () => { + mockLoadProject.mockRejectedValueOnce(new Error("Corrupt file")); + tauriInvoke.mockResolvedValueOnce(bootstrapResponse()); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /choose local audio/i })); + await waitFor(() => { + expect(screen.getByText(/late-night-set\.wav/i)).toBeTruthy(); + }); + expect(screen.queryByText(/That project file can't open tonight/i)).toBeNull(); + expect(screen.getByRole("heading", { name: /Ready to Analyze/i })).toBeTruthy(); + }); + + it("clears a failed load card after a successful YouTube import", async () => { + mockLoadProject.mockRejectedValueOnce(new Error("Corrupt file")); + tauriInvoke.mockResolvedValueOnce({ + projectId: "project-yt-1", + sourceMode: "reference", + projectRoot: "/tmp/bandscope/projects/project-yt-1", + cacheRoot: "/tmp/bandscope/cache/project-yt-1", + tempRoot: "/tmp/bandscope/temp/project-yt-1", + source: { + sourcePath: "/tmp/bandscope/temp/project-yt-1/youtube.wav", + fileName: "youtube.wav", + extension: "wav", + fileSizeBytes: 5000000 + } + }); + render(); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + }); + + fireEvent.change(screen.getByPlaceholderText(/YouTube URL.../i), { + target: { value: "https://youtube.com/watch?v=abc123DEF45" } + }); + fireEvent.click(screen.getByRole("button", { name: /Import YouTube/i })); + + await waitFor(() => { + expect(screen.getByText(/youtube\.wav/i)).toBeTruthy(); + }); + expect(screen.queryByText(/That project file can't open tonight/i)).toBeNull(); + }); + + it("clears a save-failure card when analysis starts again", async () => { + tauriInvoke + .mockResolvedValueOnce(bootstrapResponse()) + .mockResolvedValueOnce(jobStatusResponse({ + jobId: "job-1", + state: "queued", + progressLabel: "Queued for analysis" + })) + .mockResolvedValueOnce(succeededResult()) + .mockResolvedValueOnce(jobStatusResponse({ + jobId: "job-retry-1", + state: "queued", + progressLabel: "Queued for analysis" + })) + .mockResolvedValueOnce(succeededResult()); + + render(); + fireEvent.click(screen.getByRole("button", { name: /choose local audio/i })); + await waitFor(() => { + expect(screen.getByText(/late-night-set\.wav/i)).toBeTruthy(); + }); + fireEvent.click(screen.getByRole("button", { name: /start analysis/i })); + await waitFor(() => { + expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy(); + }); + + mockSaveProject.mockRejectedValueOnce(new Error("Permission denied")); + fireEvent.click(screen.getByRole("button", { name: /save project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Try saving again/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /start analysis/i })); + await waitFor(() => { + expect(screen.getByText(/queued for analysis/i)).toBeTruthy(); + }); + expect(screen.queryByText(/Tonight's rehearsal map couldn't be saved/i)).toBeNull(); + }); + + it("renders Korean recovery copy for a failed project load", async () => { + const languageSpy = vi.spyOn(window.navigator, "language", "get").mockReturnValue("ko-KR"); + mockLoadProject.mockRejectedValueOnce(new Error("Corrupt file")); + + try { + render(); + fireEvent.click(screen.getByRole("button", { name: /프로젝트 열기/i })); + await waitFor(() => { + expect(screen.getByRole("heading", { name: /그 프로젝트 파일로는 오늘 합주를 열 수 없습니다/i })).toBeTruthy(); + }); + expect(screen.getByRole("button", { name: /다른 프로젝트 선택하기/i })).toBeTruthy(); + expect(screen.queryByText(/An error occurred during analysis/i)).toBeNull(); + } finally { + languageSpy.mockRestore(); + } + }); + it("handles exception thrown by importYoutubeUrl itself", async () => { mockImportYoutubeUrlError = true; diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index f3d678454..43c0e46a1 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -46,7 +46,7 @@ import { import { createTranslator, detectPreferredLocale, type TranslationKey } from "./i18n"; import { ScoreView } from "./features/score/ScoreView"; import { Workspace } from "./features/workspace/Workspace"; -import { EmptyState, ErrorState, LoadingState } from "./features/workspace/WorkspaceStates"; +import { EmptyState, ErrorState, LoadingState, ProjectPersistenceError } from "./features/workspace/WorkspaceStates"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Progress } from "@/components/ui/progress"; @@ -59,6 +59,10 @@ const URL_PATTERN = /\bhttps?:\/\/[^\s"'<>]+/gi; const SECRET_ASSIGNMENT_PATTERN = /\b(token|secret|password|api[_-]?key|access[_-]?token)\s*[:=]\s*[^\s,;]+/gi; type RehearsalView = "workspace" | "score"; +type ProjectPersistenceFailure = { + kind: "load" | "save"; + message: string; +}; const NAV_ITEMS = [ { labelKey: "navWorkspace", icon: Home, view: "workspace" }, @@ -261,6 +265,7 @@ export function App() { const [activeAnalysisBootstrap, setActiveAnalysisBootstrap] = useState(null); const [selectionError, setSelectionError] = useState(null); const [selectionErrorSource, setSelectionErrorSource] = useState<"local" | "youtube" | null>(null); + const [projectError, setProjectError] = useState(null); const [youtubeUrl, setYoutubeUrl] = useState(""); const [isImporting, setIsImporting] = useState(false); const [activeView, setActiveView] = useState("workspace"); @@ -389,6 +394,7 @@ export function App() { const handleStartAnalysis = async () => { const submittedBootstrap = selectedBootstrap; setJobError(null); + setProjectError(null); setJobResult(null); setJobResultBootstrap(null); setJobStatus(null); @@ -420,6 +426,7 @@ export function App() { const selection = await selectLocalAudioSource(); if (selection.ok) { setSelectedBootstrap(selection.bootstrap); + setProjectError(null); return; } @@ -452,6 +459,7 @@ export function App() { if (selection.ok) { setSelectedBootstrap(selection.bootstrap); setYoutubeUrl(""); + setProjectError(null); } else { setSelectionError(safeErrorDetail(selection.error.message, t("youtubeImportFailed"))); setSelectionErrorSource("youtube"); @@ -477,12 +485,16 @@ export function App() { setJobResult(song); setJobResultBootstrap(null); setJobError(null); + setProjectError(null); setSelectedBootstrap(null); setActiveAnalysisBootstrap(null); setJobStatus(null); } catch (e) { if (!isUserCancellation(e)) { - setJobError(`${t("loadProjectFailedPrefix")}: ${safeErrorDetail(e, t("loadProjectFailedFallback"))}`); + setProjectError({ + kind: "load", + message: `${t("loadProjectFailedPrefix")}: ${safeErrorDetail(e, t("loadProjectFailedFallback"))}` + }); } } }; @@ -491,9 +503,13 @@ export function App() { const handleSaveProject = async () => { try { await saveProject(jobResult!); + setProjectError(null); } catch (e) { if (!isUserCancellation(e)) { - setJobError(`${t("saveProjectFailedPrefix")}: ${safeErrorDetail(e, t("saveProjectFailedFallback"))}`); + setProjectError({ + kind: "save", + message: `${t("saveProjectFailedPrefix")}: ${safeErrorDetail(e, t("saveProjectFailedFallback"))}` + }); } } }; @@ -512,7 +528,29 @@ export function App() { return ; } if (jobResult) { - return ; + return ( + <> + {projectError ? ( +
+ +
+ ) : null} + + + ); + } + if (projectError?.kind === "load") { + return ( + + ); } return ; }; diff --git a/apps/desktop/src/features/workspace/WorkspaceStates.tsx b/apps/desktop/src/features/workspace/WorkspaceStates.tsx index 8f9aba1b1..e60436cdf 100644 --- a/apps/desktop/src/features/workspace/WorkspaceStates.tsx +++ b/apps/desktop/src/features/workspace/WorkspaceStates.tsx @@ -1,5 +1,6 @@ import { createTranslator, detectPreferredLocale } from "../../i18n"; import { Card, CardContent } from "@/components/ui/card"; +import { Button } from "@/components/ui/button"; import { Loader2, Music, AlertCircle } from "lucide-react"; /** Documented. */ @@ -53,3 +54,43 @@ export function ErrorState({ error }: { error?: string }) { ); } + +/** Documented. */ +export function ProjectPersistenceError({ + kind, + detail, + onRetry +}: { + kind: "load" | "save"; + detail: string; + onRetry: () => void; +}) { + const t = createTranslator(detectPreferredLocale()); + const title = kind === "load" ? t("projectLoadFailedTitle") : t("projectSaveFailedTitle"); + const action = kind === "load" ? t("projectLoadFailedNextAction") : t("projectSaveFailedNextAction"); + + return ( + + +
+
+

{title}

+

{detail}

+ +
+
+ ); +} diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index 39f716d50..df1dde432 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -145,6 +145,10 @@ "loadProjectFailedFallback": "The selected project could not be loaded.", "saveProjectFailedPrefix": "Failed to save project", "saveProjectFailedFallback": "The project could not be saved.", + "projectLoadFailedTitle": "That project file can't open tonight", + "projectLoadFailedNextAction": "Choose another project", + "projectSaveFailedTitle": "Tonight's rehearsal map couldn't be saved", + "projectSaveFailedNextAction": "Try saving again", "practiceProgressRegionLabel": "Practice Progress", "practiceProgressLabel": "Practice Progress", "decreasePracticeProgressLabel": "Decrease progress", diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 371884abb..34e0e13ec 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -145,6 +145,10 @@ "loadProjectFailedFallback": "선택한 프로젝트를 불러올 수 없습니다.", "saveProjectFailedPrefix": "프로젝트를 저장하지 못했습니다", "saveProjectFailedFallback": "프로젝트를 저장할 수 없습니다.", + "projectLoadFailedTitle": "그 프로젝트 파일로는 오늘 합주를 열 수 없습니다", + "projectLoadFailedNextAction": "다른 프로젝트 선택하기", + "projectSaveFailedTitle": "오늘 합주 지도를 저장하지 못했습니다", + "projectSaveFailedNextAction": "다시 저장하기", "practiceProgressRegionLabel": "연습 진척도", "practiceProgressLabel": "연습 진척도", "decreasePracticeProgressLabel": "진척도 감소", diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 22602c313..960bccfad 100644 --- a/docs/design-system/component-contract.md +++ b/docs/design-system/component-contract.md @@ -34,7 +34,7 @@ The authoritative Figma view is `31 Component Contract Catalog`. This file mirro | Groove Map | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-526 | `apps/desktop/src/features/workspace/GrooveMap.tsx` | Use `notes?: TranscriptionNote[]` and `isLoading?: boolean`; preserve scrollable region semantics and note labels. | | Source Control Stack | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-655 | `apps/desktop/src/App.tsx` | Feature-local source controls for local audio, YouTube URL import, project actions, and Start Analysis; keep before metrics at 375px. | | 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()`. | +| 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, analysis-error, project persistence-error, and ready state routing; use before changing `renderWorkspaceState()`. | ## Prop And State Mapping diff --git a/docs/design-system/product-design-handoff.md b/docs/design-system/product-design-handoff.md index 5c8f9712b..3bd284bae 100644 --- a/docs/design-system/product-design-handoff.md +++ b/docs/design-system/product-design-handoff.md @@ -35,7 +35,8 @@ Out of scope for this handoff: new routes, cloud sharing, account settings, live | Workspace Home | No analyzed song and no active job | Choose local audio or import YouTube | Brand shell, source controls, pending metrics, actionable empty card | Empty card must explain next action; never show a blank canvas. | | Source Selected | Valid local or YouTube source exists | Start Analysis | Selected source pill, enabled start button, pending metrics | Invalid source messages stay in the source-control band. | | Analyzing | `isStarting`, queued, or running job | Wait; progress is informational | Loading card plus progress label/percent when available | Loading card uses live-region semantics. | -| Error | Job, import, load, or validation error | Choose another source, retry analysis, or load project | Safe redacted error copy | Error state uses alert semantics and must not leak local paths, URLs, or secrets. | +| Error | Job or validation error | Choose another source or retry analysis | Safe redacted error copy | Error state uses alert semantics and must not leak local paths, URLs, or secrets. | +| Project persistence error | Load or save of a project file failed | **Choose another project** or **Try saving again** | Keep the current rehearsal map when one exists | Do not reuse the analysis error card; recovery reuses the existing load/save pickers. | | Ready Workspace | `jobResult` exists | Review and export rehearsal output | Song header, export group, timeline, role switcher, groove map, section roadmap | Missing optional collaboration data uses copy, not empty modules. | ## Key Screens @@ -43,7 +44,7 @@ Out of scope for this handoff: new routes, cloud sharing, account settings, live 1. `Workspace Home` is the first screen and must prioritize source controls above metrics on mobile and desktop. 2. `Analyzing` must confirm work is in progress through both the workspace state card and the compact progress region when progress exists. 3. `Ready Workspace` is the production handoff screen for players and publishers; exports stay in the song header, not hidden below analysis modules. -4. `Error` is a recovery screen; the user must still see the source controls above it. +4. `Error` is a recovery screen; the user must still see the source controls above it. Project load/save failures use a dedicated recovery card and keep an open rehearsal map visible. ## Wireframes From 808d3a7fedc984c57a8ab531a7a6a4b0ec6f41ad Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 13:08:36 -0700 Subject: [PATCH 2/5] test: cover stale analysis error before project load recovery --- .../src/App.project-persistence.test.tsx | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 apps/desktop/src/App.project-persistence.test.tsx diff --git a/apps/desktop/src/App.project-persistence.test.tsx b/apps/desktop/src/App.project-persistence.test.tsx new file mode 100644 index 000000000..624f60cd9 --- /dev/null +++ b/apps/desktop/src/App.project-persistence.test.tsx @@ -0,0 +1,88 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { App } from "./App"; + +const mocks = vi.hoisted(() => ({ + getAnalysisJobStatus: vi.fn(), + importYoutubeUrl: vi.fn(), + loadProject: vi.fn(), + saveProject: vi.fn(), + selectLocalAudioSource: vi.fn(), + startAnalysisJob: vi.fn(), + subscribeToAnalysisJobUpdates: vi.fn() +})); + +vi.mock("./features/score/ScoreView", () => ({ + ScoreView: () => null +})); + +vi.mock("./lib/analysis", () => ({ + MAX_YOUTUBE_URL_LENGTH: 2000, + createDefaultAnalysisRequest: () => ({ + sourceKind: "demo", + sourceLabel: "Late Night Set", + roleFocus: ["bass-guitar", "keys-right", "lead-vocal"] + }), + getAnalysisJobStatus: mocks.getAnalysisJobStatus, + importYoutubeUrl: mocks.importYoutubeUrl, + isSupportedYoutubeUrl: () => true, + loadProject: mocks.loadProject, + saveProject: mocks.saveProject, + selectLocalAudioSource: mocks.selectLocalAudioSource, + startAnalysisJob: mocks.startAnalysisJob, + subscribeToAnalysisJobUpdates: mocks.subscribeToAnalysisJobUpdates +})); + +const localBootstrap = { + projectId: "project-local-1", + sourceMode: "reference", + projectRoot: "/tmp/bandscope/projects/project-local-1", + cacheRoot: "/tmp/bandscope/cache/project-local-1", + tempRoot: "/tmp/bandscope/temp/project-local-1", + source: { + sourcePath: "/tmp/bandscope/project-local-1/rehearsal.wav", + fileName: "rehearsal.wav", + extension: "wav", + fileSizeBytes: 1024 + } +}; + +describe("project persistence recovery", () => { + beforeEach(() => { + mocks.getAnalysisJobStatus.mockReset(); + mocks.importYoutubeUrl.mockReset(); + mocks.loadProject.mockReset(); + mocks.saveProject.mockReset(); + mocks.selectLocalAudioSource.mockReset(); + mocks.startAnalysisJob.mockReset(); + mocks.subscribeToAnalysisJobUpdates.mockReset(); + mocks.subscribeToAnalysisJobUpdates.mockResolvedValue(() => undefined); + }); + + it("replaces a stale analysis failure with failed project-load recovery", async () => { + mocks.selectLocalAudioSource.mockResolvedValue({ ok: true, bootstrap: localBootstrap }); + mocks.startAnalysisJob.mockRejectedValue(new Error("engine unavailable")); + mocks.loadProject.mockRejectedValue(new Error("Corrupt file")); + + render(); + + fireEvent.click(screen.getByRole("button", { name: /choose local audio/i })); + await waitFor(() => { + expect(screen.getByText(/rehearsal\.wav/i)).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /start analysis/i })); + await waitFor(() => { + expect(screen.getByRole("alert")).toHaveTextContent(/analysis could not start/i); + }); + + fireEvent.click(screen.getByRole("button", { name: /open project/i })); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Choose another project/i })).toBeTruthy(); + }); + + expect(screen.queryByText(/An error occurred during analysis/i)).toBeNull(); + expect(screen.getByRole("alert")).toHaveTextContent(/Failed to load project: Corrupt file/i); + }); +}); From 8207813b5ef7637946e6510e09fa734529b1f654 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 13:13:58 -0700 Subject: [PATCH 3/5] fix: surface project-load recovery after analysis failure --- apps/desktop/src/App.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index 43c0e46a1..4e2b5eca9 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -491,6 +491,7 @@ export function App() { setJobStatus(null); } catch (e) { if (!isUserCancellation(e)) { + setJobError(null); setProjectError({ kind: "load", message: `${t("loadProjectFailedPrefix")}: ${safeErrorDetail(e, t("loadProjectFailedFallback"))}` From 76a9d85f5472debabd0a443f42d626895767de06 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Wed, 26 Aug 2026 12:59:00 +0900 Subject: [PATCH 4/5] docs(review): record project persistence states in workspace routing order The recovery card added a projectError load branch before EmptyState and a save-failure banner above Workspace, but the component contract still listed only the original four routing states. Update the routing-order rule so the documented contract matches App.tsx. --- docs/design-system/component-contract.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md index 960bccfad..a6c5178c9 100644 --- a/docs/design-system/component-contract.md +++ b/docs/design-system/component-contract.md @@ -76,7 +76,7 @@ The authoritative Figma view is `31 Component Contract Catalog`. This file mirro ### Workspace States - Figma page `34 Workspace State Matrix` maps `EmptyState`, `LoadingState`, `ErrorState`, ready `Workspace`, `GrooveMap`, and Source Control Stack substates. -- `App.tsx` must preserve the current routing order: `jobError` -> `ErrorState`, `analysisInFlight || isStarting` -> `LoadingState`, `jobResult` -> `Workspace`, otherwise `EmptyState`. +- `App.tsx` must preserve the current routing order: `jobError` -> `ErrorState`, `analysisInFlight || isStarting` -> `LoadingState`, `jobResult` -> `Workspace` (a project save-failure recovery banner renders above `Workspace` while analyzed content stays visible), a load-only `projectError` -> `ProjectPersistenceError` recovery card, otherwise `EmptyState`. - `LoadingState` keeps `role="status"`, `aria-live="polite"`, `aria-atomic="true"`, and `aria-busy="true"`. - `ErrorState` keeps `role="alert"`, `aria-live="assertive"`, and visible safe error detail copy. - `EmptyState` must remain an actionable state card, not a blank placeholder panel. From bd324043e9065ca2d60ce3440b945aa9a9c41617 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Wed, 26 Aug 2026 13:30:50 +0900 Subject: [PATCH 5/5] fix(review): surface persistence recovery in the Score view The Save Project control stays reachable while the Score view is active, but renderWorkspaceState is bypassed there, so a failed save set projectError with no visible feedback. Hoist the shared recovery banner so it renders above ScoreView as well, and pin the behavior with a test. --- .../src/App.project-persistence.test.tsx | 30 +++++++++++++++ apps/desktop/src/App.tsx | 38 ++++++++++++------- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/App.project-persistence.test.tsx b/apps/desktop/src/App.project-persistence.test.tsx index 624f60cd9..8347926ac 100644 --- a/apps/desktop/src/App.project-persistence.test.tsx +++ b/apps/desktop/src/App.project-persistence.test.tsx @@ -1,5 +1,6 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; import { App } from "./App"; @@ -85,4 +86,33 @@ describe("project persistence recovery", () => { expect(screen.queryByText(/An error occurred during analysis/i)).toBeNull(); expect(screen.getByRole("alert")).toHaveTextContent(/Failed to load project: Corrupt file/i); }); + + it("keeps a failed save visible in the Score view where the Save control stays reachable", async () => { + mocks.selectLocalAudioSource.mockResolvedValue({ ok: true, bootstrap: localBootstrap }); + mocks.startAnalysisJob.mockResolvedValue({ + jobId: "job-1", + state: "succeeded", + result: createDemoRehearsalSong() + }); + mocks.saveProject.mockRejectedValue(new Error("disk full")); + + render(); + + fireEvent.click(screen.getByRole("button", { name: /choose local audio/i })); + await waitFor(() => { + expect(screen.getByText(/rehearsal\.wav/i)).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /start analysis/i })); + await waitFor(() => { + expect(screen.getByText(/Song Timeline/i)).toBeTruthy(); + }); + + fireEvent.click(screen.getAllByRole("button", { name: /^Score$/i })[0]); + fireEvent.click(screen.getByRole("button", { name: /save project/i })); + + await waitFor(() => { + expect(screen.getByRole("alert")).toHaveTextContent(/Failed to save project: disk full/i); + }); + }); }); diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index 4e2b5eca9..e52bf7a1e 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -520,6 +520,21 @@ export function App() { setJobResult(updatedSong); }; + /** + * Shared persistence-recovery banner so a failed save or load stays visible + * in every view that keeps the persistence controls reachable, including the + * Score view where renderWorkspaceState is bypassed. + */ + const persistenceErrorBanner = projectError ? ( +
+ +
+ ) : null; + /** Documented. */ const renderWorkspaceState = () => { if (jobError) { @@ -531,15 +546,7 @@ export function App() { if (jobResult) { return ( <> - {projectError ? ( -
- -
- ) : null} + {persistenceErrorBanner} ); @@ -882,11 +889,14 @@ export function App() {
{currentView === "score" && jobResult ? ( - + <> + {persistenceErrorBanner} + + ) : ( renderWorkspaceState() )}