From ee3a22db8ca907fccd8d8312268e729cdd45d96a Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 10:27:28 +0000 Subject: [PATCH 01/23] feat(workspace): name tonight's first accelerando plan on the map Stamp one owned accelerandoPlan from existing tempo-stability speeding onto the highest-priority active named vocal or bass, and open that landing on the rehearsal map. Double-time and half-time feel flips stay unnamed. Heuristic demo topology stays unnamed. This is not a new MIR product. --- AGENTS.md | 1 + ARCHITECTURE.md | 1 + CHANGELOG.md | 1 + CLAUDE.md | 2 +- apps/desktop/core/src/lib.rs | 40 +- .../core/tests/accelerando_plan_contract.rs | 142 ++++++ .../FirstAccelerandoCallout.particle.test.tsx | 66 +++ .../FirstAccelerandoCallout.test.tsx | 195 ++++++++ .../workspace/FirstAccelerandoCallout.tsx | 224 ++++++++++ .../Workspace.accelerando-state.test.tsx | 63 +++ .../src/features/workspace/Workspace.tsx | 35 +- .../workspace/firstAccelerando.test.ts | 179 ++++++++ .../features/workspace/firstAccelerando.ts | 415 ++++++++++++++++++ apps/desktop/src/i18n/index.test.ts | 10 +- apps/desktop/src/i18n/index.ts | 34 ++ apps/desktop/src/locales/en/common.json | 9 +- apps/desktop/src/locales/ko/common.json | 9 +- packages/shared-types/src/index.ts | 28 +- .../test/accelerandoPlanProvenance.test.ts | 50 +++ packages/shared-types/test/index.test.ts | 6 + .../src/bandscope_analysis/api.py | 35 ++ .../src/bandscope_analysis/roles/model.py | 4 +- .../bandscope_analysis/temporal/__init__.py | 4 + .../temporal/accelerando.py | 269 ++++++++++++ .../tests/test_accelerando_plan.py | 386 ++++++++++++++++ 25 files changed, 2195 insertions(+), 13 deletions(-) create mode 100644 apps/desktop/core/tests/accelerando_plan_contract.rs create mode 100644 apps/desktop/src/features/workspace/FirstAccelerandoCallout.particle.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstAccelerandoCallout.test.tsx create mode 100644 apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx create mode 100644 apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx create mode 100644 apps/desktop/src/features/workspace/firstAccelerando.test.ts create mode 100644 apps/desktop/src/features/workspace/firstAccelerando.ts create mode 100644 packages/shared-types/test/accelerandoPlanProvenance.test.ts create mode 100644 services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py create mode 100644 services/analysis-engine/tests/test_accelerando_plan.py diff --git a/AGENTS.md b/AGENTS.md index b9a67ce17..b24e52212 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,6 +2,7 @@ ## Project overview - BandScope is a local-first desktop app for rehearsal prep: a practical song view with likely harmony by section and by instrument or vocal role, form and groove cues, stems, playable ranges, simplification guidance, transposition or setup cues, part-overlap cues, visible confidence, and rehearsal priorities. +- Name tonight's first accelerando plan with the owning vocal or bass when existing tempo-stability reports a corroborated speeding, the owned `accelerandoPlan` copy, the labeled section, and the time so the next action is Open on the map. Do not invent that copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, ritardando plans, fade plans, swell plans, drop plans, breakdown plans, hit plans, cutoff plans, double-time feel flips, half-time feel flips, confirmed overrides, harmonic explanations, or confidence notes. Heuristic demo topology stays unnamed. - Authoritative delivery rules live in `ARCHITECTURE.md`, `docs/plans/`, and the root verification scripts. - Brand, tone, UX copy, and prioritization rules live in `docs/brand-story.md` and must be applied to PRDs, TRDs, UI copy, onboarding, empty states, and error messages. - App security rules live in `docs/security/app-security.md` and must be applied to file handling, URL intake, subprocesses, IPC, WebView usage, model loading, updates, logging, cache handling, and export behavior. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ca0df5ac4..eff58506c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -83,6 +83,7 @@ Last updated: 2026-03-11 - section roadmap with entries, dropouts, pickups, stops, tags, and handoffs - groove and timing cues relevant to locking the band together - playable ranges and density or overlap warnings, with the ready workspace naming tonight's first span and the next instrument check + - tonight's first accelerando plan on the mounted map when existing tempo-stability reports a sustained speeding (`to_bpm > from_bpm`) that is not a double-time (~1.9–2.1) or half-time (~0.5) feel flip, landing on the highest-priority active named vocal or bass in the section that contains the change. Open moves to the matching rendered map section. Heuristic-only topology stays unnamed. Distinct from first-ritardando, first-fade, first-swell, first-drop, first-breakdown, first-hit, first-stop, first-cutoff, first-pickup, and first-turnaround. This is not a new MIR product. - simplification, transposition, capo, tuning, or setup cues where applicable - role-specific rehearsal priorities and confidence flags - cue-sheet or chart-style exports that summarize the analysis in rehearsal-friendly form diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b6f7e784..90fbc0ef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added +- Name tonight's first accelerando plan in the mounted rehearsal workspace so the vocal or bass that lifts into a faster tempo can open that landing on the map; real analyzed songs now receive this guidance only when existing tempo-stability reports a sustained speeding that is not a double-time or half-time feel flip, while heuristic-only topology remains unavailable. Open moves to the matching rendered map section, and inherited, accessor-backed, or Proxy-substituted runtime metadata remains guidance-only instead of becoming copy, identity, timing, or navigation authority. - Name tonight's first playable range on the ready rehearsal map and tell the player to check that span on their instrument before the section. - Display the analyzed song tempo (BPM) as a badge in the rehearsal workspace. - 각 합주 역할(Role)별 개인 연습 진행도를 0~100% 범위로 기록 및 시각화할 수 있는 연습 진척도(`practiceProgress`) 트래커 기능 추가. UI 컨트롤(슬라이더 및 +/- 버튼)과 한/영 다국어 지원 포함. diff --git a/CLAUDE.md b/CLAUDE.md index b5a34c1fa..f7bb49ad4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ BandScope is a local-first desktop app for rehearsal prep: it turns a song into Three layers, decoupled through shared contracts: -- `apps/desktop` — Tauri 2 + Vite + React 19 shell (Tailwind 4, Base UI, Storybook). Feature screens live in `src/features/` (home, workspace, chords, ranges, player, settings). The ready workspace names tonight's first playable range and the next instrument check. `src/lib/analysis.ts` and `src/lib/job_runner.ts` call typed Tauri IPC commands, with a browser fallback that serves demo data when not running inside Tauri. +- `apps/desktop` — Tauri 2 + Vite + React 19 shell (Tailwind 4, Base UI, Storybook). Feature screens live in `src/features/` (home, workspace, chords, ranges, player, settings). The mounted workspace names tonight's first accelerando plan and opens the matching rendered map section. The ready workspace names tonight's first playable range and the next instrument check. Do not invent that copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, ritardando plans, fade plans, swell plans, drop plans, breakdown plans, hit plans, cutoff plans, double-time feel flips, half-time feel flips, confirmed overrides, harmonic explanations, or confidence notes. Distinct from first-ritardando, first-fade, first-swell, first-drop, first-breakdown, first-hit, first-stop, and first-cutoff. `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/core/src/lib.rs b/apps/desktop/core/src/lib.rs index 200726570..3dda07e48 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -176,6 +176,13 @@ pub struct ManualOverridePayload { source: String, } +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "lowercase")] +enum AccelerandoPlanSourcePayload { + Model, + User, +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct RehearsalRolePayload { @@ -191,6 +198,10 @@ pub struct RehearsalRolePayload { setup_note: String, manual_overrides: Vec, overlap_warnings: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + accelerando_plan: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + accelerando_plan_source: Option, } #[derive(Clone, Debug, Serialize)] @@ -527,9 +538,32 @@ pub fn is_youtube_video_id(value: &str) -> bool { .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'-') } +fn validate_accelerando_plan_provenance( + payload: RehearsalSongPayload, +) -> Result { + for section in &payload.sections { + for role in §ion.roles { + if role.accelerando_plan.as_ref().is_some_and(|accelerando_plan| { + accelerando_plan.trim().is_empty() + || accelerando_plan.contains('\n') + || accelerando_plan.contains('\r') + }) { + return Err("Invalid project file format".to_string()); + } + if role.accelerando_plan.is_none() && role.accelerando_plan_source.is_some() { + return Err("Invalid project file format".to_string()); + } + if role.accelerando_plan.is_some() && role.accelerando_plan_source.is_none() { + return Err("Invalid project file format".to_string()); + } + } + } + Ok(payload) +} + pub fn project_payload_from_content(content: &str) -> Result { if let Ok(parsed) = serde_json::from_str::(content) { - return Ok(parsed); + return validate_accelerando_plan_provenance(parsed); } let payload = serde_json::from_str::(content) @@ -547,7 +581,9 @@ pub fn project_payload_from_content(content: &str) -> Result Value { + json!({ + "id": "analyzed-song", + "title": "Late Night Set", + "sections": [ + { + "id": "chorus-1", + "label": "chorus", + "groove": "Lifted chorus downbeat", + "timeRange": { "start": 0, "end": 16 }, + "confidence": { + "level": "high", + "source": "model", + "notes": "Tempo stability corroborates the accelerando." + }, + "roles": [ + { + "id": "lead-vocal", + "name": "Lead Vocal", + "roleType": "vocal", + "harmony": { + "chord": "C#m7", + "functionLabel": "vi landing", + "source": "model" + }, + "cue": { + "kind": "transition", + "value": "Let the next downbeat arrive sooner." + }, + "range": { + "lowestNote": "G#3", + "highestNote": "C#5" + }, + "confidence": { + "level": "high", + "source": "model", + "notes": "Vocal stays while the tempo lifts." + }, + "rehearsalPriority": "high", + "simplification": "Lean into the landing syllable.", + "setupNote": "Keep the attack short.", + "manualOverrides": [], + "overlapWarnings": [], + "accelerandoPlan": "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner.", + "accelerandoPlanSource": "model" + } + ], + "partGraph": [ + { + "role_id": "lead-vocal", + "is_active": true, + "handoff_to": [], + "handoff_from": [] + } + ] + } + ], + "exportSummary": { + "format": "cue-sheet", + "headline": "Push into the faster chorus landing.", + "focusSections": ["chorus-1"] + } + }) +} + +#[test] +fn project_contract_round_trips_accelerando_plan_provenance() { + let payload = song_with_accelerando_plan(); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + let parsed = project_payload_from_content(&content) + .expect("native project contract must accept shared accelerando-plan fields"); + let serialized = serde_json::to_value(parsed).expect("native project contract should serialize"); + + assert_eq!( + serialized["sections"][0]["roles"][0]["accelerandoPlan"], + payload["sections"][0]["roles"][0]["accelerandoPlan"] + ); + assert_eq!( + serialized["sections"][0]["roles"][0]["accelerandoPlanSource"], + json!("model") + ); +} + +#[test] +fn project_contract_rejects_accelerando_plan_source_without_accelerando_plan() { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0] + .as_object_mut() + .expect("role fixture should be an object") + .remove("accelerandoPlan"); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!( + project_payload_from_content(&content).is_err(), + "native persisted contract must reject provenance without the value it describes" + ); +} + +#[test] +fn project_contract_rejects_accelerando_plan_without_source() { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0] + .as_object_mut() + .expect("role fixture should be an object") + .remove("accelerandoPlanSource"); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!( + project_payload_from_content(&content).is_err(), + "native persisted contract must reject accelerando-plan copy without provenance" + ); +} + +#[test] +fn project_contract_rejects_invalid_accelerando_plan_copy_with_source() { + for accelerando_plan in ["", " ", "push here\nthen hold", "push here\rthen hold"] { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0]["accelerandoPlan"] = json!(accelerando_plan); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!( + project_payload_from_content(&content).is_err(), + "native persisted contract must reject blank or multiline sourced accelerando-plan copy" + ); + } +} + +#[test] +fn project_contract_rejects_unknown_accelerando_plan_source() { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0]["accelerandoPlanSource"] = json!("legacy"); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!( + project_payload_from_content(&content).is_err(), + "native persisted contract must reject provenance outside model/user" + ); +} diff --git a/apps/desktop/src/features/workspace/FirstAccelerandoCallout.particle.test.tsx b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.particle.test.tsx new file mode 100644 index 000000000..ad4777c54 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.particle.test.tsx @@ -0,0 +1,66 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FirstAccelerandoCallout } from "./FirstAccelerandoCallout"; + +function songWithKoreanAccel() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + verse.roles = [ + { + ...verse.roles[0]!, + id: "piano-vocal", + name: "피아노", + roleType: "vocal", + rehearsalPriority: "high", + accelerandoPlan: + "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner.", + accelerandoPlanSource: "model" + } + ]; + verse.partGraph = [ + { role_id: "piano-vocal", is_active: true, handoff_to: [], handoff_from: [] } + ]; + return song; +} + +describe("FirstAccelerandoCallout Korean role copy", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("keeps vowel-ending role names particle-safe before and after the accel action", () => { + vi.stubGlobal("navigator", { language: "ko-KR" }); + const song = songWithKoreanAccel(); + + const grid = document.createElement("div"); + grid.dataset.testid = "song-structure-grid"; + grid.setAttribute("role", "region"); + grid.setAttribute("aria-label", "Scrollable song structure timeline"); + const target = document.createElement("div"); + target.dataset.sectionIndex = "0"; + Object.defineProperty(target, "scrollIntoView", { + configurable: true, + value: vi.fn() + }); + grid.appendChild(target); + document.body.appendChild(grid); + + render(); + + expect(screen.getByText("0:10 벌스에서 피아노 파트가 아첼레란도합니다.")).toBeTruthy(); + expect(screen.queryByText(/피아노이/)).toBeNull(); + expect(screen.queryByText(/피아노가/)).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "0:10 피아노 아첼레란도 열기" })); + + expect( + screen.getByText("0:10에서 피아노 파트로 함께 당기세요. 더 빠른 착지가 들리도록 밀으세요.") + ).toBeTruthy(); + expect(screen.queryByText(/피아노과/)).toBeNull(); + expect(screen.queryByText(/피아노을/)).toBeNull(); + expect(screen.queryByText(/피아노를/)).toBeNull(); + + grid.remove(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstAccelerandoCallout.test.tsx b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.test.tsx new file mode 100644 index 000000000..c34aa1e59 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.test.tsx @@ -0,0 +1,195 @@ +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 { FirstAccelerandoCallout } from "./FirstAccelerandoCallout"; + +const DEMO_ACCELERANDO_PLAN = + "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner."; +const appendedSongStructureTargets = new Set(); + +function songWithAccelerandoPlan() { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + verse.partGraph = verse.partGraph.map((node) => ({ ...node, is_active: true })); + const vocal = verse.roles.find((role) => role.id === "lead-vocal")!; + vocal.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + vocal.accelerandoPlanSource = "model"; + return song; +} + +function appendSongStructureTarget() { + const timeline = document.createElement("div"); + timeline.setAttribute("role", "region"); + timeline.setAttribute("aria-label", "Scrollable song structure timeline"); + 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); + appendedSongStructureTargets.add(timeline); + return { grid: timeline, scrollIntoView }; +} + +describe("FirstAccelerandoCallout", () => { + afterEach(() => { + for (const timeline of appendedSongStructureTargets) { + timeline.remove(); + } + appendedSongStructureTargets.clear(); + vi.unstubAllGlobals(); + }); + + it("contains a malformed runtime song root instead of crashing the callout", () => { + render(); + + expect( + screen.getByText( + "No accelerando plan is available. Stay on tonight's map for the next rehearsal cue." + ) + ).toBeTruthy(); + }); + + it("contains a hostile song identity accessor instead of crashing the callout", () => { + const song = songWithAccelerandoPlan(); + 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 Lead Vocal accel at 0:10" })).toBeTruthy(); + }); + + it("contains a hostile song identity descriptor lookup instead of crashing the callout", () => { + const song = new Proxy(songWithAccelerandoPlan(), { + getOwnPropertyDescriptor() { + throw new Error("hostile song id descriptor"); + } + }); + + expect(() => render()).not.toThrow(); + expect( + screen.getByText( + "No accelerando plan is available. Stay on tonight's map for the next rehearsal cue." + ) + ).toBeTruthy(); + }); + + it("opens the named accel on the rendered map", () => { + const { scrollIntoView } = appendSongStructureTarget(); + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible./) + ).toBeTruthy(); + expect(screen.getByText(DEMO_ACCELERANDO_PLAN)).toBeTruthy(); + }); + + it("shows armed confirmation for user-sourced plans without rewriting user copy", () => { + const song = songWithAccelerandoPlan(); + const userPlan = "Push here exactly as our band agreed."; + const vocal = song.sections[0]!.roles.find((role) => role.id === "lead-vocal")!; + vocal.accelerandoPlan = userPlan; + vocal.accelerandoPlanSource = "user"; + appendSongStructureTarget(); + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible./) + ).toBeTruthy(); + expect(screen.getByText(userPlan)).toBeTruthy(); + }); + + it("reports when the map section cannot be opened", () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + + expect( + screen.getByText("Could not open this accel on the song map. Use the map below to find the section.") + ).toBeTruthy(); + }); + + it("uses immediate scrolling when reduced motion is requested", () => { + const { scrollIntoView } = appendSongStructureTarget(); + vi.stubGlobal("matchMedia", (query: string) => ({ + matches: query.includes("prefers-reduced-motion"), + media: query, + addEventListener: vi.fn(), + removeEventListener: vi.fn() + })); + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + + expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "auto" }); + }); + + it("resets armed guidance when accessor-id songs change with the same accel signature", () => { + const firstSong = songWithAccelerandoPlan(); + const nextSong = songWithAccelerandoPlan(); + for (const song of [firstSong, nextSong]) { + Object.defineProperty(song, "id", { + configurable: true, + enumerable: true, + get() { + throw new Error("hostile song id getter"); + } + }); + } + appendSongStructureTarget(); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible./) + ).toBeTruthy(); + + rerender(); + + expect(screen.getByText("Lead Vocal lifts the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible./) + ).toBeNull(); + }); + + it("resets armed guidance when the landing role name changes in the same workspace", () => { + const firstSong = songWithAccelerandoPlan(); + const nextSong = structuredClone(firstSong); + nextSong.sections[0]!.roles.find((role) => role.id === "lead-vocal")!.name = "Lead Singer"; + const workspaceInstanceKey = {}; + appendSongStructureTarget(); + const { rerender } = render( + + ); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible./) + ).toBeTruthy(); + + rerender( + + ); + + expect(screen.getByText("Lead Singer lifts the verse at 0:10.")).toBeTruthy(); + expect( + screen.queryByText(/Lift Lead Singer together at 0:10 so the faster landing is audible./) + ).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx new file mode 100644 index 000000000..fdc829541 --- /dev/null +++ b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx @@ -0,0 +1,224 @@ +import { useEffect, useId, useMemo, useState } from "react"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { Button } from "@/components/ui/button"; +import { + createTranslator, + detectPreferredLocale, + translateSectionFormLabel +} from "../../i18n"; +import { + formatAccelerandoPlanTime, + resolveFirstAccelerandoPlan, + type AccelerandoPlanGuidance +} from "./firstAccelerando"; + +/** Props for the first accelerando-plan rehearsal callout. */ +export interface FirstAccelerandoCalloutProps { + song: RehearsalSong; + workspaceInstanceKey?: unknown; +} + +type AccelerandoPlanCopyValues = Readonly>; +type AccelerandoPlanSource = "model" | "user"; + +type OpenedAccelerandoPlan = Readonly<{ + songIdentity: unknown; + sectionId: string; + sectionIndex: number; + sectionLabel: string; + landingRoleId: string; + landingRoleName: string; + accelerandoPlan: string; + accelerandoPlanSource: AccelerandoPlanSource | null; + fromBpm: string | null; + toBpm: string | null; + atSeconds: number; +}>; + +/** Prefer the owning workspace instance while preserving direct-call compatibility. */ +function stableAccelerandoPlanSongIdentity( + song: RehearsalSong, + workspaceInstanceKey: unknown +): unknown { + return workspaceInstanceKey ?? song; +} + +/** Interpolate accelerando-plan placeholders once so rehearsal data is never rescanned as template syntax. */ +function formatAccelerandoPlanCopy(template: string, values: AccelerandoPlanCopyValues): string { + return template.replace(/\{(role|section|at)\}/g, (placeholder) => { + const key = placeholder.slice(1, -1) as keyof AccelerandoPlanCopyValues; + return values[key] ?? placeholder; + }); +} + +/** Localize model accelerando guidance from structured tempo tokens, never from display-copy grammar. */ +function localizedAccelerandoPlan( + accelerandoPlan: string, + accelerandoPlanSource: AccelerandoPlanSource | null, + guidance: AccelerandoPlanGuidance | null, + generatedTemplate: string +): string { + if (accelerandoPlanSource !== "model" || guidance === null) { + return accelerandoPlan; + } + return generatedTemplate + .replace("{from}", () => guidance.fromBpm) + .replace("{to}", () => guidance.toBpm); +} + +/** Use immediate scrolling when the operating system requests reduced motion. */ +function preferredAccelerandoPlanScrollBehavior(): ScrollBehavior { + return typeof window.matchMedia === "function" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches + ? "auto" + : "smooth"; +} + +/** Resolve the song-structure renderer owned by this workspace, failing closed on ambiguous mounts. */ +function resolveAccelerandoPlanRenderer(origin: HTMLElement): HTMLElement | null { + const selector = '[data-testid="song-structure-grid"]'; + const localScope = origin.closest("aside")?.parentElement ?? null; + const localRenderers = localScope?.querySelectorAll(selector) ?? []; + if (localRenderers.length === 1) { + return localRenderers[0] ?? null; + } + if (localRenderers.length > 1) { + return null; + } + + const globalRenderers = document.querySelectorAll(selector); + return globalRenderers.length === 1 ? (globalRenderers[0] ?? null) : null; +} + +/** Name tonight's first accelerando plan and open the matching rendered map section. */ +export function FirstAccelerandoCallout({ + song, + workspaceInstanceKey +}: FirstAccelerandoCalloutProps) { + const calloutId = `workspace-surface-accelerando-plan-${useId()}`; + const locale = useMemo(() => detectPreferredLocale(), []); + const t = useMemo(() => createTranslator(locale), [locale]); + const songIdentity = stableAccelerandoPlanSongIdentity(song, workspaceInstanceKey); + const named = useMemo(() => resolveFirstAccelerandoPlan(song), [song]); + const [openedAccelerandoPlan, setOpenedAccelerandoPlan] = useState( + null + ); + const [navigationFailed, setNavigationFailed] = useState(false); + const fromBpm = named?.accelerandoPlanGuidance?.fromBpm ?? null; + const toBpm = named?.accelerandoPlanGuidance?.toBpm ?? null; + + useEffect(() => { + setOpenedAccelerandoPlan(null); + setNavigationFailed(false); + }, [ + songIdentity, + named?.sectionIndex, + named?.sectionId, + named?.sectionLabel, + named?.landingRoleId, + named?.landingRoleName, + named?.accelerandoPlan, + named?.accelerandoPlanSource, + fromBpm, + toBpm, + named?.atSeconds + ]); + + if (!named) { + return ( + + ); + } + + const opened = + openedAccelerandoPlan !== null && + openedAccelerandoPlan.songIdentity === songIdentity && + openedAccelerandoPlan.sectionId === named.sectionId && + openedAccelerandoPlan.sectionIndex === named.sectionIndex && + openedAccelerandoPlan.sectionLabel === named.sectionLabel && + openedAccelerandoPlan.landingRoleId === named.landingRoleId && + openedAccelerandoPlan.landingRoleName === named.landingRoleName && + openedAccelerandoPlan.accelerandoPlan === named.accelerandoPlan && + openedAccelerandoPlan.accelerandoPlanSource === named.accelerandoPlanSource && + openedAccelerandoPlan.fromBpm === fromBpm && + openedAccelerandoPlan.toBpm === toBpm && + openedAccelerandoPlan.atSeconds === named.atSeconds; + const at = formatAccelerandoPlanTime(named.atSeconds); + const copyValues: AccelerandoPlanCopyValues = { + role: named.landingRoleName, + section: translateSectionFormLabel(locale, named.sectionLabel), + at + }; + const actionLabel = formatAccelerandoPlanCopy(t("firstAccelerandoPlanOpenAction"), copyValues); + const body = formatAccelerandoPlanCopy(t("firstAccelerandoPlanBody"), copyValues); + const armed = formatAccelerandoPlanCopy(t("firstAccelerandoPlanArmed"), copyValues); + const accelerandoPlan = localizedAccelerandoPlan( + named.accelerandoPlan, + named.accelerandoPlanSource, + named.accelerandoPlanGuidance, + t("firstAccelerandoPlanGeneratedGuidance") + ); + + return ( + + ); +} diff --git a/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx new file mode 100644 index 000000000..f50a10d70 --- /dev/null +++ b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx @@ -0,0 +1,63 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { createDemoRehearsalSong, type RehearsalSong } from "@bandscope/shared-types"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { Workspace } from "./Workspace"; + +const originalScrollIntoView = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "scrollIntoView" +); + +function analyzedSongWithAccelerandoPlan(): RehearsalSong { + const song = createDemoRehearsalSong(); + song.id = "analyzed-song"; + const verse = song.sections[0]!; + verse.partGraph = verse.partGraph.map((node) => ({ ...node, is_active: true })); + const vocal = verse.roles.find((role) => role.id === "lead-vocal")!; + vocal.accelerandoPlan = + "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner."; + vocal.accelerandoPlanSource = "model"; + return song; +} + +describe("Workspace accelerando state authority", () => { + beforeEach(() => { + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { + configurable: true, + value: vi.fn() + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + if (originalScrollIntoView) { + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", originalScrollIntoView); + } else { + Reflect.deleteProperty(HTMLElement.prototype, "scrollIntoView"); + } + }); + + it("keeps an opened accel armed after an immutable practice-progress update", () => { + const song = analyzedSongWithAccelerandoPlan(); + let updatedSong: RehearsalSong | null = null; + const onSongUpdate = vi.fn((nextSong: RehearsalSong) => { + updatedSong = nextSong; + }); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible\./) + ).toBeTruthy(); + + fireEvent.click(screen.getByRole("tab", { name: "Lead Vocal" })); + fireEvent.click(screen.getByRole("button", { name: "Increase progress" })); + expect(updatedSong).not.toBeNull(); + + rerender(); + + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible\./) + ).toBeTruthy(); + }); +}); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index d44e20777..183d95366 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -1,10 +1,11 @@ -import { useState, useMemo, memo, type MouseEvent } from "react"; +import { useState, useMemo, useRef, memo, type MouseEvent } from "react"; import { parseProjectBootstrapSummary, type ProjectBootstrapSummary, type RehearsalSong, type RehearsalRole } from "@bandscope/shared-types"; import { RoleSwitcher } from "./RoleSwitcher"; import { SectionRoadmap } from "./SectionRoadmap"; import { GrooveMap } from "./GrooveMap"; import { PracticeProgress } from "./PracticeProgress"; import { fillRangeCopy, firstRangeSqueeze } from "./firstRangeSqueeze"; +import { FirstAccelerandoCallout } from "./FirstAccelerandoCallout"; import { createTranslator, detectPreferredLocale } from "../../i18n"; import { generateCueSheetCsv, generateChartSummaryJson, generateMetadataHandoffJson, sanitizeFilename } from "../../lib/export"; import { Button } from "@/components/ui/button"; @@ -91,8 +92,12 @@ const SongStructure = memo(function SongStructure({ sections, t }: { sections: R data-testid="song-structure-grid" style={{ gridTemplateColumns: `repeat(${Math.max(1, sections.length)}, minmax(8rem, 1fr))` }} > - {sections.map((section) => ( -
+ {sections.map((section, sectionIndex) => ( +

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

@@ -122,6 +127,18 @@ const SongStructure = memo(function SongStructure({ sections, t }: { sections: R export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: WorkspaceProps) { const [activeRole, setActiveRole] = useState(null); const t = useMemo(() => createTranslator(detectPreferredLocale()), []); + const localSongUpdateRef = useRef(null); + const workspaceInstanceRef = useRef(song); + const previousSongRef = useRef(song); + + if (song !== previousSongRef.current) { + const isLocalWorkspaceUpdate = song === localSongUpdateRef.current; + if (!isLocalWorkspaceUpdate) { + workspaceInstanceRef.current = song; + } + localSongUpdateRef.current = null; + previousSongRef.current = song; + } // Extract all unique roles from the song's sections const roleMap = useMemo(() => { @@ -164,6 +181,13 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp ) : t("workspaceFirstRangeMissing"); + /** Preserve workspace-instance authority for immutable edits emitted by this workspace. */ + const commitSongUpdate = (nextSong: RehearsalSong) => { + if (!onSongUpdate) return; + localSongUpdateRef.current = nextSong; + onSongUpdate(nextSong); + }; + /** Handle the practice progress change internally by immutably updating the song state. */ const handlePracticeProgressChange = (newProgress: number) => { if (!activeRole || !onSongUpdate) return; @@ -188,7 +212,7 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp }) }; - onSongUpdate(nextSong); + commitSongUpdate(nextSong); }; const collaborationAssignments = useMemo( () => (Array.isArray(song.collaboration?.assignments) ? song.collaboration.assignments : []), @@ -309,6 +333,7 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp

{t("workspaceFirstRangeTitle")}

{firstRangeCopy}

+
@@ -505,7 +530,7 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts new file mode 100644 index 000000000..fc0a268be --- /dev/null +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -0,0 +1,179 @@ +import { describe, expect, it } from "vitest"; +import { createDemoRehearsalSong } from "@bandscope/shared-types"; +import { formatAccelerandoPlanTime, resolveFirstAccelerandoPlan } from "./firstAccelerando"; + +const DEMO_ACCELERANDO_PLAN = + "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner."; + +function withAccelerandoSection( + overrides: { + id?: string; + start?: number; + end?: number; + accelerandoPlan?: string; + source?: "model" | "user"; + label?: "intro" | "verse" | "chorus" | "bridge" | "outro"; + roleId?: string; + roleName?: string; + priority?: "low" | "medium" | "high"; + isActive?: boolean; + roleType?: "instrument" | "vocal" | "hand"; + } = {} +) { + const song = createDemoRehearsalSong(); + const verse = song.sections[0]!; + const landingStart = overrides.start ?? 0; + const roleId = overrides.roleId ?? "lead-vocal"; + const vocal = structuredClone(verse.roles.find((role) => role.id === "lead-vocal")!); + const bass = structuredClone(verse.roles.find((role) => role.id === "bass-guitar")!); + const keys = structuredClone(verse.roles.find((role) => role.id === "keys-right")!); + const landing = { + ...(roleId === "bass-guitar" ? bass : roleId === "keys-right" ? keys : vocal), + id: roleId, + name: + overrides.roleName ?? + (roleId === "bass-guitar" + ? "Bass Guitar" + : roleId === "keys-right" + ? "Keyboard 1 Right Hand" + : "Lead Vocal"), + roleType: overrides.roleType ?? (roleId === "lead-vocal" ? "vocal" : "instrument"), + rehearsalPriority: overrides.priority ?? "high", + accelerandoPlan: overrides.accelerandoPlan ?? DEMO_ACCELERANDO_PLAN, + ...(overrides.source ? { accelerandoPlanSource: overrides.source } : { accelerandoPlanSource: "model" as const }) + }; + const current = structuredClone(verse); + current.id = overrides.id ?? "chorus-accel"; + current.label = overrides.label ?? "chorus"; + current.timeRange = { start: landingStart, end: overrides.end ?? landingStart + 16 }; + current.roles = [landing, bass, keys]; + current.partGraph = [ + { role_id: "bass-guitar", is_active: true, handoff_to: [], handoff_from: [] }, + { role_id: "keys-right", is_active: true, handoff_to: [], handoff_from: [] }, + { + role_id: "lead-vocal", + is_active: roleId === "lead-vocal" ? (overrides.isActive ?? true) : true, + handoff_to: [], + handoff_from: [] + } + ]; + if (roleId === "bass-guitar") { + current.partGraph[0]!.is_active = overrides.isActive ?? true; + current.roles = [landing, vocal, keys]; + } + if (roleId === "keys-right") { + current.partGraph[1]!.is_active = overrides.isActive ?? true; + current.roles = [landing, vocal, bass]; + } + song.sections = [current]; + return song; +} + +describe("resolveFirstAccelerandoPlan", () => { + it("picks the earliest accelerando plan and the named vocal that owns it", () => { + const resolved = resolveFirstAccelerandoPlan(withAccelerandoSection()); + expect(resolved?.section.id).toBe("chorus-accel"); + expect(resolved?.landingRole.id).toBe("lead-vocal"); + expect(resolved?.accelerandoPlan).toBe(DEMO_ACCELERANDO_PLAN); + expect(resolved?.atSeconds).toBe(0); + expect(formatAccelerandoPlanTime(resolved?.atSeconds ?? -1)).toBe("0:00"); + expect(formatAccelerandoPlanTime(Number.NaN)).toBe("0:00"); + expect(formatAccelerandoPlanTime(-4)).toBe("0:00"); + }); + + it("does not invent an accelardando plan from groove, cue, simplification, overlap, range, chords, function labels, setup notes, transposition plans, or confidence notes", () => { + const song = withAccelerandoSection(); + delete song.sections[0]!.roles.find((role) => role.id === "lead-vocal")!.accelerandoPlan; + const landing = song.sections[0]!.roles.find((role) => role.id === "lead-vocal")!; + song.sections[0]!.groove = "Straight eighths with a late snare feel"; + landing.simplification = "Stay on roots if the chorus entrance gets muddy."; + landing.setupNote = DEMO_ACCELERANDO_PLAN; + landing.transpositionPlan = "If the singer drops to B minor, keep the shape a whole step lower."; + landing.cue = { kind: "transition", value: DEMO_ACCELERANDO_PLAN }; + landing.confidence.notes = DEMO_ACCELERANDO_PLAN; + expect(resolveFirstAccelerandoPlan(song)).toBeNull(); + }); + + it("leaves the heuristic demo unnamed", () => { + expect(resolveFirstAccelerandoPlan(createDemoRehearsalSong())).toBeNull(); + }); + + it("does not let accompaniment own the accelerando", () => { + expect( + resolveFirstAccelerandoPlan( + withAccelerandoSection({ roleId: "keys-right", roleType: "hand" }) + ) + ).toBeNull(); + }); + + it("ignores inactive named parts", () => { + expect(resolveFirstAccelerandoPlan(withAccelerandoSection({ isActive: false }))).toBeNull(); + }); + + it("prefers a named vocal over bass at the same priority", () => { + const song = withAccelerandoSection(); + const bass = song.sections[0]!.roles.find((role) => role.id === "bass-guitar")!; + bass.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + bass.accelerandoPlanSource = "model"; + bass.rehearsalPriority = "high"; + expect(resolveFirstAccelerandoPlan(song)?.landingRoleId).toBe("lead-vocal"); + }); + + it("prefers the higher-priority named part", () => { + const song = withAccelerandoSection({ priority: "low" }); + const bass = song.sections[0]!.roles.find((role) => role.id === "bass-guitar")!; + bass.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + bass.accelerandoPlanSource = "model"; + bass.rehearsalPriority = "high"; + expect(resolveFirstAccelerandoPlan(song)?.landingRoleId).toBe("bass-guitar"); + }); + + it("picks the earlier section when two accels are named", () => { + const earlier = withAccelerandoSection({ id: "earlier-accel", start: 0 }); + const later = withAccelerandoSection({ id: "later-accel", start: 16 }); + const song = earlier; + song.sections = [...earlier.sections, ...later.sections]; + expect(resolveFirstAccelerandoPlan(song)?.sectionId).toBe("earlier-accel"); + }); + + it("rejects blank, multiline, or non-template model copy", () => { + expect(resolveFirstAccelerandoPlan(withAccelerandoSection({ accelerandoPlan: " " }))).toBeNull(); + expect( + resolveFirstAccelerandoPlan( + withAccelerandoSection({ accelerandoPlan: "Ease together.\nHold the count." }) + ) + ).toBeNull(); + expect( + resolveFirstAccelerandoPlan(withAccelerandoSection({ accelerandoPlan: "slow down here" })) + ).toBeNull(); + }); + + it("admits bounded user copy without requiring the engine template", () => { + const resolved = resolveFirstAccelerandoPlan( + withAccelerandoSection({ + accelerandoPlan: "Push the phrase early into the downbeat.", + source: "user" + }) + ); + expect(resolved?.accelerandoPlan).toBe("Push the phrase early into the downbeat."); + expect(resolved?.accelerandoPlanSource).toBe("user"); + }); + + it("fails closed on a malformed runtime song root", () => { + expect(resolveFirstAccelerandoPlan(null as never)).toBeNull(); + }); + + it("fails closed on inherited or accessor-backed plan copy", () => { + const song = withAccelerandoSection(); + const vocal = song.sections[0]!.roles.find((role) => role.id === "lead-vocal")!; + delete vocal.accelerandoPlan; + Object.defineProperty(vocal, "accelerandoPlan", { + configurable: true, + enumerable: true, + get() { + return DEMO_ACCELERANDO_PLAN; + } + }); + expect(resolveFirstAccelerandoPlan(song)).toBeNull(); + }); +}); diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts new file mode 100644 index 000000000..7924b4e1f --- /dev/null +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -0,0 +1,415 @@ +import { + MAX_SECTION_TIME_SECONDS, + SECTION_FORM_LABELS, + type RehearsalRole, + type RehearsalSection, + type RehearsalSong +} from "@bandscope/shared-types"; + +const PRIORITY_RANK = { high: 0, medium: 1, low: 2 } as const; +const MAX_ACCELERANDO_PLAN_CHARACTERS = 180; +const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); +const NAMED_ACCELERANDO_ROLE_IDS = new Set(["bass-guitar", "lead-vocal"]); +const ACCELERANDO_PLAN_PREFIX = "Push this part from "; +const ACCELERANDO_PLAN_MIDDLE = " BPM into "; +const ACCELERANDO_PLAN_SUFFIX = " BPM; let the next downbeat arrive sooner."; + +type AccelerandoPlanSource = "model" | "user"; + +/** Structured localization guidance for model-generated accelerando-plan copy. */ +export type AccelerandoPlanGuidance = Readonly<{ + kind: "tempo"; + fromBpm: string; + toBpm: string; +}>; + +type RankedRoleMetadata = Readonly<{ + role: RehearsalRole; + id: string; + name: string; + rehearsalPriority: keyof typeof PRIORITY_RANK; + isVocal: boolean; +}>; + +type OwnedAccelerandoPlan = Readonly<{ + text: string; + source: AccelerandoPlanSource | null; + guidance: AccelerandoPlanGuidance | null; +}>; + +/** Tonight's first accelerando plan: the earliest corroborated speeding on a named vocal or bass. */ +export type FirstAccelerandoPlan = { + section: RehearsalSection; + sectionId: string; + sectionLabel: RehearsalSection["label"]; + sectionIndex: number; + landingRole: RehearsalRole; + landingRoleId: string; + landingRoleName: string; + accelerandoPlan: string; + accelerandoPlanSource: AccelerandoPlanSource | null; + accelerandoPlanGuidance: AccelerandoPlanGuidance | null; + atSeconds: number; +}; + +/** Format a non-negative accelerando-plan time as m:ss for rehearsal copy. */ +export function formatAccelerandoPlanTime(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"); +} + +/** Snapshot one owned data-property value without invoking a getter or Proxy get trap. */ +function ownDataValue(value: object, key: PropertyKey): unknown { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor !== undefined && Object.prototype.hasOwnProperty.call(descriptor, "value") + ? descriptor.value + : undefined; +} + +/** Snapshot every numeric own data element from a bounded runtime array. */ +function ownedDenseRuntimeArray(value: unknown): unknown[] | null { + if (!Array.isArray(value)) { + return null; + } + const length = ownDataValue(value, "length"); + if ( + typeof length !== "number" || + !Number.isSafeInteger(length) || + length < 0 || + length > 0xffffffff + ) { + return null; + } + const items: unknown[] = []; + for (let index = 0; index < length; index += 1) { + if (!hasOwnData(value, index)) { + return null; + } + items.push(ownDataValue(value, index)); + } + return items; +} + +/** 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); +} + +/** Preserve the engine accelerando template while bounding its model-owned tempo tokens. */ +function boundedGeneratedAccelerandoPlan(value: string): OwnedAccelerandoPlan | null { + if ( + !value.startsWith(ACCELERANDO_PLAN_PREFIX) || + !value.endsWith(ACCELERANDO_PLAN_SUFFIX) || + !value.includes(ACCELERANDO_PLAN_MIDDLE) + ) { + return null; + } + const inner = value.slice(ACCELERANDO_PLAN_PREFIX.length, -ACCELERANDO_PLAN_SUFFIX.length); + const middleIndex = inner.indexOf(ACCELERANDO_PLAN_MIDDLE); + if (middleIndex <= 0) { + return null; + } + const fromBpm = inner.slice(0, middleIndex); + const toBpm = inner.slice(middleIndex + ACCELERANDO_PLAN_MIDDLE.length); + if (!/^\d+(?:\.\d+)?$/u.test(fromBpm) || !/^\d+(?:\.\d+)?$/u.test(toBpm)) { + return null; + } + return { + text: `${ACCELERANDO_PLAN_PREFIX}${fromBpm}${ACCELERANDO_PLAN_MIDDLE}${toBpm}${ACCELERANDO_PLAN_SUFFIX}`, + source: "model", + guidance: { kind: "tempo", fromBpm, toBpm } + }; +} + +/** Return a bounded snapshotted own accelerando plan and its explicit provenance, or null when malformed. */ +function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { + if (!isRuntimeObject(role)) { + return null; + } + const accelerandoPlan = ownDataValue(role, "accelerandoPlan"); + const accelerandoPlanSource = ownDataValue(role, "accelerandoPlanSource"); + if (typeof accelerandoPlan !== "string") { + return null; + } + if ( + accelerandoPlanSource !== undefined && + accelerandoPlanSource !== "model" && + accelerandoPlanSource !== "user" + ) { + return null; + } + const trimmed = accelerandoPlan.trim(); + if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + return null; + } + if (accelerandoPlanSource === "model") { + return boundedGeneratedAccelerandoPlan(trimmed); + } + return { + text: truncateCodePoints(trimmed, MAX_ACCELERANDO_PLAN_CHARACTERS), + source: accelerandoPlanSource ?? null, + guidance: null + }; +} + +/** Snapshot trusted role identity, display name, and priority without Proxy get authority. */ +function ownedRankedRoleMetadata(role: unknown): RankedRoleMetadata | null { + if (!isRuntimeObject(role)) { + return null; + } + const id = ownDataValue(role, "id"); + const name = ownDataValue(role, "name"); + const roleType = ownDataValue(role, "roleType"); + const rehearsalPriority = ownDataValue(role, "rehearsalPriority"); + if ( + typeof id !== "string" || + id.trim().length === 0 || + typeof name !== "string" || + name.trim().length === 0 || + typeof rehearsalPriority !== "string" || + !Object.prototype.hasOwnProperty.call(PRIORITY_RANK, rehearsalPriority) + ) { + return null; + } + if (!NAMED_ACCELERANDO_ROLE_IDS.has(id) && roleType !== "vocal") { + return null; + } + return { + role: role as RehearsalRole, + id, + name, + rehearsalPriority: rehearsalPriority as keyof typeof PRIORITY_RANK, + isVocal: roleType === "vocal" || id === "lead-vocal" + }; +} + +/** Snapshot a section's bounded positive-length integer rehearsal window. */ +function ownedBoundedTimeRange( + section: RehearsalSection +): RehearsalSection["timeRange"] | null { + const timeRange = ownDataValue(section, "timeRange"); + if (!isRuntimeObject(timeRange)) { + return null; + } + const start = ownDataValue(timeRange, "start"); + const end = ownDataValue(timeRange, "end"); + if ( + typeof start !== "number" || + !Number.isInteger(start) || + start < 0 || + start > MAX_SECTION_TIME_SECONDS || + typeof end !== "number" || + !Number.isInteger(end) || + end <= start || + end > MAX_SECTION_TIME_SECONDS + ) { + return null; + } + return { start, end }; +} + +/** Return safe identities that appear more than once in one section-local collection. */ +function repeatedIds(ids: string[]): Set { + const seen = new Set(); + const repeated = new Set(); + for (const id of ids) { + if (seen.has(id)) { + repeated.add(id); + } else { + seen.add(id); + } + } + return repeated; +} + +/** Rank vocal roles before instrumental roles. */ +function vocalRank(role: RankedRoleMetadata): number { + return role.isVocal ? 0 : 1; +} + +/** Prefer rehearsal priority, then a named vocal, then a locale-independent stable id. */ +function pickLandingRole(roles: Role[]): Role | null { + if (roles.length === 0) { + return null; + } + return ( + [...roles].sort((left, right) => { + const priorityDelta = + PRIORITY_RANK[left.rehearsalPriority] - PRIORITY_RANK[right.rehearsalPriority]; + if (priorityDelta !== 0) { + return priorityDelta; + } + const vocalDelta = vocalRank(left) - vocalRank(right); + if (vocalDelta !== 0) { + return vocalDelta; + } + return compareStableId(left.id, right.id); + })[0] ?? null + ); +} + +/** Return unique graph role ids whose node is explicitly active. */ +function rankedActiveRoleIds(section: RehearsalSection): Set { + const partGraph = ownedDenseRuntimeArray(ownDataValue(section, "partGraph")); + if (!partGraph) { + return new Set(); + } + const safeGraphRoleIds = partGraph.flatMap((node) => { + if (!isRuntimeObject(node)) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && roleId.trim().length > 0 ? [roleId] : []; + }); + const repeatedGraphRoleIds = repeatedIds(safeGraphRoleIds); + return new Set( + partGraph.flatMap((node) => { + if (!isRuntimeObject(node) || ownDataValue(node, "is_active") !== true) { + return []; + } + const roleId = ownDataValue(node, "role_id"); + return typeof roleId === "string" && + roleId.trim().length > 0 && + !repeatedGraphRoleIds.has(roleId) + ? [roleId] + : []; + }) + ); +} + +/** Resolve an accelerando plan after the runtime root has passed its structural boundary checks. */ +function resolveSafeFirstAccelerandoPlan(song: RehearsalSong): FirstAccelerandoPlan | null { + if (!isRuntimeObject(song)) { + return null; + } + const sections = ownedDenseRuntimeArray(ownDataValue(song, "sections")); + if (!sections) { + return null; + } + + const candidates = sections + .flatMap((section, sectionIndex) => { + if (!isRuntimeObject(section)) { + return []; + } + const sectionId = ownDataValue(section, "id"); + const sectionLabel = ownDataValue(section, "label"); + const timeRange = ownedBoundedTimeRange(section as RehearsalSection); + if ( + typeof sectionId !== "string" || + sectionId.trim().length === 0 || + typeof sectionLabel !== "string" || + !SECTION_FORM_LABEL_SET.has(sectionLabel) || + timeRange === null + ) { + return []; + } + + const activeIds = rankedActiveRoleIds(section as RehearsalSection); + const roles = ownedDenseRuntimeArray(ownDataValue(section, "roles")); + if (!roles) { + return []; + } + const safeRoleIds = roles.flatMap((role) => { + if (!isRuntimeObject(role)) { + return []; + } + const id = ownDataValue(role, "id"); + return typeof id === "string" && id.trim().length > 0 ? [id] : []; + }); + const repeatedRoleIds = repeatedIds(safeRoleIds); + const landingRole = pickLandingRole( + roles.flatMap((role) => { + const metadata = ownedRankedRoleMetadata(role); + if ( + metadata === null || + repeatedRoleIds.has(metadata.id) || + !activeIds.has(metadata.id) + ) { + return []; + } + const accelerandoPlan = ownedAccelerandoPlan(metadata.role); + return accelerandoPlan === null + ? [] + : [ + { + ...metadata, + accelerandoPlan: accelerandoPlan.text, + accelerandoPlanSource: accelerandoPlan.source, + accelerandoPlanGuidance: accelerandoPlan.guidance + } + ]; + }) + ); + if (!landingRole) { + return []; + } + return [ + { + section: section as RehearsalSection, + sectionId, + sectionLabel: sectionLabel as RehearsalSection["label"], + sectionIndex, + landingRole: landingRole.role, + landingRoleId: landingRole.id, + landingRoleName: landingRole.name, + accelerandoPlan: landingRole.accelerandoPlan, + accelerandoPlanSource: landingRole.accelerandoPlanSource, + accelerandoPlanGuidance: landingRole.accelerandoPlanGuidance, + atSeconds: timeRange.start + } + ]; + }) + .sort((left, right) => { + if (left.atSeconds !== right.atSeconds) { + return left.atSeconds - right.atSeconds; + } + return compareStableId(left.sectionId, right.sectionId); + }); + + return candidates[0] ?? null; +} + +/** Return the first named accelerando plan, or null when untrusted runtime metadata cannot be read safely. */ +export function resolveFirstAccelerandoPlan(song: RehearsalSong): FirstAccelerandoPlan | null { + try { + return resolveSafeFirstAccelerandoPlan(song); + } catch { + return null; + } +} diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a25..e0c767882 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,12 @@ describe("i18n", () => { } }); }); + + describe("translateSectionFormLabel", () => { + it("localizes supported section labels and fails closed on unknown labels", () => { + expect(translateSectionFormLabel("en", "chorus")).toBe("chorus"); + expect(translateSectionFormLabel("ko", "chorus")).toBe("코러스"); + expect(translateSectionFormLabel("en", "not-a-section" as never)).toBe("not-a-section"); + }); + }); }); diff --git a/apps/desktop/src/i18n/index.ts b/apps/desktop/src/i18n/index.ts index 1a9f471f0..ff6e218d1 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,6 +12,33 @@ const dictionaries = { ko: koCommon } as const; +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: "핸드오프" + } +}; + /** Documented. */ export function createTranslator(locale: Locale = "en") { return function t(key: TranslationKey): string { @@ -18,6 +46,12 @@ export function createTranslator(locale: Locale = "en") { }; } +/** Return the localized display label for a supported rehearsal section form. */ +export function translateSectionFormLabel(locale: Locale, label: SectionFormLabel): string { + const labels = sectionFormLabels[locale] as Readonly>; + return Object.prototype.hasOwnProperty.call(labels, label) ? labels[label] : String(label); +} + /** Documented. */ export function detectPreferredLocale(): Locale { if (typeof navigator !== "undefined" && navigator.language?.toLowerCase().startsWith("ko")) { diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index d803a765e..af35964e6 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -154,5 +154,12 @@ "workspaceFirstRangeClash": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Hear that clash on your instrument before the {sectionLabel}.", "workspaceFirstRangeMissing": "Tonight's first range still needs an ear check. Confirm the high and low notes on the selected part before the first section.", "sectionRangeLabel": "Range", - "sectionRangeNextAction": "Check this span on your instrument before {sectionLabel}." + "sectionRangeNextAction": "Check this span on your instrument before {sectionLabel}.", + "firstAccelerandoPlanLabel": "Tonight's first accelerando plan", + "firstAccelerandoPlanOpenAction": "Open {role} accel at {at}", + "firstAccelerandoPlanBody": "{role} lifts the {section} at {at}.", + "firstAccelerandoPlanArmed": "Lift {role} together at {at} so the faster landing is audible.", + "firstAccelerandoPlanGeneratedGuidance": "Push this part from {from} BPM into {to} BPM; let the next downbeat arrive sooner.", + "firstAccelerandoPlanUnavailable": "No accelerando plan is available. Stay on tonight's map for the next rehearsal cue.", + "firstAccelerandoPlanNavigationFailed": "Could not open this accel on the song map. Use the map below to find the section." } diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 0f6c6c66d..5cf0525b7 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -154,5 +154,12 @@ "workspaceFirstRangeClash": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}이고 다른 파트와 겹칩니다. {sectionLabel} 들어가기 전에 그 충돌을 악기로 들어 보세요.", "workspaceFirstRangeMissing": "오늘 먼저 볼 음역은 아직 귀로 확인이 필요합니다. 선택한 파트의 최저·최고음을 첫 구간 전에 확인해 보세요.", "sectionRangeLabel": "음역", - "sectionRangeNextAction": "{sectionLabel} 들어가기 전에 이 음역을 악기로 확인해 보세요." + "sectionRangeNextAction": "{sectionLabel} 들어가기 전에 이 음역을 악기로 확인해 보세요.", + "firstAccelerandoPlanLabel": "오늘 첫 아첼레란도 계획", + "firstAccelerandoPlanOpenAction": "{at} {role} 아첼레란도 열기", + "firstAccelerandoPlanBody": "{at} {section}에서 {role} 파트가 아첼레란도합니다.", + "firstAccelerandoPlanArmed": "{at}에서 {role} 파트로 함께 당기세요. 더 빠른 착지가 들리도록 밀으세요.", + "firstAccelerandoPlanGeneratedGuidance": "이 파트를 {from} BPM에서 {to} BPM으로 당기세요. 다음 다운비트까지 밀어 올리세요.", + "firstAccelerandoPlanUnavailable": "사용 가능한 아첼레란도 계획이 없습니다. 다음 합주 큐를 위해 오늘 맵에 머무르세요.", + "firstAccelerandoPlanNavigationFailed": "곡 맵에서 이 아첼레란도를 열 수 없습니다. 아래 맵에서 해당 구간을 찾아주세요." } diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index cba4606a2..f45c618cc 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -143,6 +143,8 @@ export type RehearsalRole = { overlapWarnings: string[]; transcription?: TranscriptionNote[]; practiceProgress?: number; + accelerandoPlan?: string; + accelerandoPlanSource?: ProvenanceSource; }; /** Documented. */ @@ -1500,7 +1502,9 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "manualOverrides", "overlapWarnings", "transcription", - "practiceProgress" + "practiceProgress", + "accelerandoPlan", + "accelerandoPlanSource" ], path ); @@ -1588,6 +1592,28 @@ function validateRehearsalRole(value: unknown, path: string): string | null { } } + if ( + value.accelerandoPlan !== undefined && + (typeof value.accelerandoPlan !== "string" || + value.accelerandoPlan.trim().length === 0 || + value.accelerandoPlan.includes("\n") || + value.accelerandoPlan.includes("\r")) + ) { + return invalidField(`${path}.accelerandoPlan`); + } + if ( + value.accelerandoPlanSource !== undefined && + !isOneOf(PROVENANCE_SOURCES, value.accelerandoPlanSource) + ) { + return invalidField(`${path}.accelerandoPlanSource`); + } + if (value.accelerandoPlanSource !== undefined && value.accelerandoPlan === undefined) { + return invalidField(`${path}.accelerandoPlanSource`); + } + if (value.accelerandoPlan !== undefined && value.accelerandoPlanSource === undefined) { + return invalidField(`${path}.accelerandoPlanSource`); + } + return null; } diff --git a/packages/shared-types/test/accelerandoPlanProvenance.test.ts b/packages/shared-types/test/accelerandoPlanProvenance.test.ts new file mode 100644 index 000000000..9c350b0fc --- /dev/null +++ b/packages/shared-types/test/accelerandoPlanProvenance.test.ts @@ -0,0 +1,50 @@ +import { createDemoRehearsalSong, parseRehearsalSong } from "../src/index"; +import { describe, expect, it } from "vitest"; + +const DEMO_ACCELERANDO_PLAN = + "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner."; + +describe("accelerandoPlan provenance", () => { + it.each(["model", "user"] as const)("admits a %s accelerando plan source", (source) => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + role.accelerandoPlanSource = source; + expect(parseRehearsalSong(song).sections[0]!.roles[0]!.accelerandoPlanSource).toBe(source); + }); + + it("rejects an unknown accelerando plan source", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + role.accelerandoPlanSource = "inferred" as never; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanSource/); + }); + + it("rejects an accelerando plan source without copy", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + delete role.accelerandoPlan; + role.accelerandoPlanSource = "model"; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanSource/); + }); + + it("rejects accelerando plan copy without provenance", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + delete role.accelerandoPlanSource; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanSource/); + }); + + it.each(["", " ", "push here\nthen hold", "push here\rthen hold"])( + "rejects an accelerando plan source with blank or multiline copy %j", + (accelerandoPlan) => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = accelerandoPlan; + role.accelerandoPlanSource = "model"; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlan/); + } + ); +}); diff --git a/packages/shared-types/test/index.test.ts b/packages/shared-types/test/index.test.ts index 564ee1827..bb5adbd81 100644 --- a/packages/shared-types/test/index.test.ts +++ b/packages/shared-types/test/index.test.ts @@ -1257,6 +1257,12 @@ describe("shared type helpers", () => { song.sections[0]!.roles[0]!.transpositionPlan = 2 as never; }) }, + { + message: "sections[0].roles[0].accelerandoPlan", + payload: createInvalidSong((song) => { + song.sections[0]!.roles[0]!.accelerandoPlan = 2 as never; + }) + }, { message: "sections[0].roles[0].practiceProgress", payload: createInvalidSong((song) => { diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index b376de293..be9cbc71f 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -19,6 +19,7 @@ from bandscope_analysis.sections import extract_sections from bandscope_analysis.sections.segmenter import segment_with_boundaries from bandscope_analysis.separation import AudioStemSeparator +from bandscope_analysis.temporal.accelerando import apply_accelerando_plan, derive_beat_times logger = logging.getLogger(__name__) @@ -116,6 +117,8 @@ class RehearsalRolePayload(TypedDict): setupNote: str manualOverrides: list[ManualOverridePayload] overlapWarnings: list[str] + accelerandoPlan: NotRequired[str] + accelerandoPlanSource: NotRequired[Literal["model", "user"]] class PartGraphNodePayload(TypedDict): @@ -456,6 +459,7 @@ def _build_from_pipeline( }, } _apply_tempo(song, features) + _apply_accelerando(song, mix, sr, features) return song @@ -518,6 +522,37 @@ def _apply_tempo(song: RehearsalSong, audio_features: dict[str, Any] | None) -> song["tempo"] = bpm +def _coerce_beat_times(audio_features: dict[str, Any] | None) -> list[float] | None: + """Return finite non-negative beat times from analysis features, or None.""" + if not audio_features: + return None + raw = audio_features.get("beat_times") + if not isinstance(raw, list) or not raw: + return None + times: list[float] = [] + for item in raw: + if isinstance(item, bool) or not isinstance(item, (int, float)): + return None + value = float(item) + if np.isnan(value) or np.isinf(value) or value < 0: + return None + times.append(value) + return times + + +def _apply_accelerando( + song: RehearsalSong, + mix: Any, + sr: int, + audio_features: dict[str, Any] | None, +) -> None: + """Stamp tonight's first accelerando from existing tempo-stability changes.""" + beat_times = _coerce_beat_times(audio_features) + if beat_times is None: + beat_times = derive_beat_times(mix, sr) + apply_accelerando_plan(song, beat_times) + + def _reconstruct_mix(stems: dict[str, Any]) -> Any: """Reconstruct a mono mix from separated stems for segmentation.""" arrays = [] diff --git a/services/analysis-engine/src/bandscope_analysis/roles/model.py b/services/analysis-engine/src/bandscope_analysis/roles/model.py index ea6fc1449..faa1651aa 100644 --- a/services/analysis-engine/src/bandscope_analysis/roles/model.py +++ b/services/analysis-engine/src/bandscope_analysis/roles/model.py @@ -3,7 +3,7 @@ from __future__ import annotations from enum import Enum -from typing import Any, Literal, TypedDict +from typing import Any, Literal, NotRequired, TypedDict class RoleType(str, Enum): @@ -83,6 +83,8 @@ class RehearsalRole(TypedDict): setupNote: str manualOverrides: list[ManualOverride] overlapWarnings: list[str] + accelerandoPlan: NotRequired[str] + accelerandoPlanSource: NotRequired[Literal["model", "user"]] class PartGraphNode(TypedDict): diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py b/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py index 104b82ec9..5b82ff2b1 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py @@ -3,6 +3,7 @@ from .analyzer import TemporalAnalyzer from .groove import GrooveResult, detect_groove from .model import TemporalFeatures +from .accelerando import apply_accelerando_plan, first_accelerando, accelerando_plan_copy from .stability import TempoChange, TempoStability, analyze_tempo_stability __all__ = [ @@ -12,5 +13,8 @@ "TemporalAnalyzer", "TemporalFeatures", "analyze_tempo_stability", + "apply_accelerando_plan", "detect_groove", + "first_accelerando", + "accelerando_plan_copy", ] diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py new file mode 100644 index 000000000..9b3aa58a1 --- /dev/null +++ b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py @@ -0,0 +1,269 @@ +"""Stamp tonight's first accelerando plan from existing tempo-stability changes. + +An accelerando is the earliest sustained speeding (``to_bpm > from_bpm``) that is +not a double-time feel (~1.9–2.1) or a half-time feel (~0.5). The owned +``accelerandoPlan`` copy lands on the highest-priority active named vocal or +bass in the section that contains that change. Heuristic/demo topology stays +unnamed. This is not a new MIR product: it only reads +``analyze_tempo_stability`` output. + +Security Notes: + Pure in-memory mutation of an already-built rehearsal song. Beat times and + song topology are untrusted runtime values: malformed numbers, missing + identity, repeated graph ids, or inactive parts fail closed instead of + inventing a plan. No file, network, or subprocess I/O. +""" + +from __future__ import annotations + +from collections.abc import Mapping, MutableMapping, Sequence +from math import isfinite +from typing import Any + +from bandscope_analysis.temporal.stability import TempoChange, analyze_tempo_stability + +DOUBLE_TIME_RATIO_MIN = 1.9 +DOUBLE_TIME_RATIO_MAX = 2.1 +NAMED_ACCELERANDO_ROLE_IDS = frozenset({"bass-guitar", "lead-vocal"}) +PRIORITY_RANK = {"high": 0, "medium": 1, "low": 2} +ACCELERANDO_PLAN_PREFIX = "Push this part from " +ACCELERANDO_PLAN_MIDDLE = " BPM into " +ACCELERANDO_PLAN_SUFFIX = " BPM; let the next downbeat arrive sooner." + + +def format_accelerando_bpm(value: float) -> str | None: + """Return a buyer-facing BPM token, or None when the value is unusable.""" + if not isfinite(value) or value <= 0: + return None + rounded = round(float(value), 1) + if abs(rounded - round(rounded)) < 1e-9: + return str(int(round(rounded))) + return f"{rounded:.1f}" + + +def accelerando_plan_copy(from_bpm: float, to_bpm: float) -> str | None: + """Return the owned model accelerando copy, or None when BPM tokens are unusable.""" + from_token = format_accelerando_bpm(from_bpm) + to_token = format_accelerando_bpm(to_bpm) + if from_token is None or to_token is None: + return None + return ( + f"{ACCELERANDO_PLAN_PREFIX}{from_token}" + f"{ACCELERANDO_PLAN_MIDDLE}{to_token}{ACCELERANDO_PLAN_SUFFIX}" + ) + + +def is_accelerando_change(change: Mapping[str, Any]) -> bool: + """Return whether a tempo change is a speeding that is not a feel flip.""" + from_bpm = change.get("from_bpm") + to_bpm = change.get("to_bpm") + if not isinstance(from_bpm, (int, float)) or isinstance(from_bpm, bool): + return False + if not isinstance(to_bpm, (int, float)) or isinstance(to_bpm, bool): + return False + if not isfinite(from_bpm) or not isfinite(to_bpm) or from_bpm <= 0 or to_bpm <= 0: + return False + if to_bpm <= from_bpm: + return False + ratio = float(to_bpm) / float(from_bpm) + if DOUBLE_TIME_RATIO_MIN <= ratio <= DOUBLE_TIME_RATIO_MAX: + return False + return True + + +def first_accelerando(tempo_changes: Sequence[Mapping[str, Any]] | None) -> TempoChange | None: + """Return the earliest accelerando change, or None when none is corroborated.""" + if not isinstance(tempo_changes, Sequence) or isinstance(tempo_changes, (str, bytes)): + return None + for change in tempo_changes: + if not isinstance(change, Mapping): + continue + if not is_accelerando_change(change): + continue + time = change.get("time") + from_bpm = change.get("from_bpm") + to_bpm = change.get("to_bpm") + if ( + not isinstance(time, (int, float)) + or isinstance(time, bool) + or not isfinite(time) + or time < 0 + or not isinstance(from_bpm, (int, float)) + or isinstance(from_bpm, bool) + or not isinstance(to_bpm, (int, float)) + or isinstance(to_bpm, bool) + ): + continue + return TempoChange( + time=float(time), + from_bpm=float(from_bpm), + to_bpm=float(to_bpm), + ) + return None + + +def _role_type_value(role_type: Any) -> str: + """Normalize enum or string role types to a comparable token.""" + value = getattr(role_type, "value", role_type) + return value if isinstance(value, str) else "" + + +def _priority_value(priority: Any) -> str: + """Normalize enum or string rehearsal priority to a comparable token.""" + value = getattr(priority, "value", priority) + return value if isinstance(value, str) else "" + + +def _is_named_vocal_or_bass(role: Mapping[str, Any]) -> bool: + """Return whether a role is a named vocal or bass that may own an accel.""" + role_id = role.get("id") + if not isinstance(role_id, str) or role_id.strip() == "": + return False + if role_id in NAMED_ACCELERANDO_ROLE_IDS: + return True + return _role_type_value(role.get("roleType")) == "vocal" + + +def _repeated_ids(ids: list[str]) -> set[str]: + """Return ids that appear more than once in one section-local collection.""" + seen: set[str] = set() + repeated: set[str] = set() + for role_id in ids: + if role_id in seen: + repeated.add(role_id) + else: + seen.add(role_id) + return repeated + + +def _active_role_ids(section: Mapping[str, Any]) -> set[str]: + """Return unique graph role ids whose node is explicitly active.""" + part_graph = section.get("partGraph") + if not isinstance(part_graph, list): + return set() + safe_ids = [ + node.get("role_id") + for node in part_graph + if isinstance(node, Mapping) + and isinstance(node.get("role_id"), str) + and node["role_id"].strip() + ] + repeated = _repeated_ids([role_id for role_id in safe_ids if isinstance(role_id, str)]) + active: set[str] = set() + for node in part_graph: + if not isinstance(node, Mapping) or node.get("is_active") is not True: + continue + role_id = node.get("role_id") + if isinstance(role_id, str) and role_id.strip() and role_id not in repeated: + active.add(role_id) + return active + + +def _section_contains(section: Mapping[str, Any], time: float) -> bool: + """Return whether a section window contains a tempo-change time.""" + time_range = section.get("timeRange") + if not isinstance(time_range, Mapping): + return False + start = time_range.get("start") + end = time_range.get("end") + if not isinstance(start, int) or isinstance(start, bool) or start < 0: + return False + if not isinstance(end, int) or isinstance(end, bool) or end <= start: + return False + return start <= time < end + + +def _pick_landing_role(section: Mapping[str, Any]) -> MutableMapping[str, Any] | None: + """Pick the highest-priority unique active named vocal or bass.""" + roles = section.get("roles") + if not isinstance(roles, list): + return None + active_ids = _active_role_ids(section) + safe_ids = [ + role.get("id") + for role in roles + if isinstance(role, Mapping) and isinstance(role.get("id"), str) and role["id"].strip() + ] + repeated = _repeated_ids([role_id for role_id in safe_ids if isinstance(role_id, str)]) + ranked: list[tuple[int, int, str, MutableMapping[str, Any]]] = [] + for role in roles: + if not isinstance(role, MutableMapping): + continue + role_id = role.get("id") + name = role.get("name") + priority = _priority_value(role.get("rehearsalPriority")) + if not isinstance(role_id, str) or role_id.strip() == "" or role_id in repeated: + continue + if not isinstance(name, str) or name.strip() == "": + continue + if role_id not in active_ids or not _is_named_vocal_or_bass(role): + continue + if priority not in PRIORITY_RANK: + continue + is_vocal = _role_type_value(role.get("roleType")) == "vocal" or role_id == "lead-vocal" + vocal_rank = 0 if is_vocal else 1 + ranked.append((PRIORITY_RANK[priority], vocal_rank, role_id, role)) + if not ranked: + return None + ranked.sort(key=lambda item: (item[0], item[1], item[2])) + return ranked[0][3] + + +def derive_beat_times(mix: Any, sr: Any) -> list[float] | None: + """Return beat times from an in-memory mix using existing librosa beat tracking. + + Security Notes: + In-memory only. Malformed mix or sample-rate values fail closed. This + reuses ``librosa.beat.beat_track`` already owned by ``TemporalAnalyzer``; + it does not introduce a new MIR product. + """ + try: + import librosa + + if not isinstance(sr, int) or isinstance(sr, bool) or sr <= 0: + return None + if not hasattr(mix, "size") or int(getattr(mix, "size", 0)) <= 0: + return None + _tempo, beat_frames = librosa.beat.beat_track(y=mix, sr=sr) + times = librosa.frames_to_time(beat_frames, sr=sr) + derived = [float(time) for time in times] + return derived if derived else None + except (TypeError, ValueError, RuntimeError, AttributeError, ImportError): + return None + + +def apply_accelerando_plan(song: Mapping[str, Any], beat_times: Sequence[float] | None) -> None: + """Attach the first corroborated accelerando plan, failing closed on bad input. + + Args: + song: Mutable rehearsal-song mapping with section/role topology. + beat_times: Beat onset times in seconds used by tempo-stability. + """ + if ( + beat_times is None + or not isinstance(beat_times, Sequence) + or isinstance(beat_times, (str, bytes)) + ): + return + try: + stability = analyze_tempo_stability(beat_times) + change = first_accelerando(stability.get("tempo_changes")) + if change is None: + return + copy = accelerando_plan_copy(change["from_bpm"], change["to_bpm"]) + if copy is None: + return + sections = song.get("sections") + if not isinstance(sections, list): + return + for section in sections: + if not isinstance(section, Mapping) or not _section_contains(section, change["time"]): + continue + landing = _pick_landing_role(section) + if landing is None: + return + landing["accelerandoPlan"] = copy + landing["accelerandoPlanSource"] = "model" + return + except (TypeError, ValueError, KeyError, AttributeError): + return diff --git a/services/analysis-engine/tests/test_accelerando_plan.py b/services/analysis-engine/tests/test_accelerando_plan.py new file mode 100644 index 000000000..8babf74cb --- /dev/null +++ b/services/analysis-engine/tests/test_accelerando_plan.py @@ -0,0 +1,386 @@ +"""Tests for corroborated accelerando-plan emission.""" + +from __future__ import annotations + +from typing import Any + +import numpy as np +import pytest + +from bandscope_analysis.api import ( + _apply_accelerando, + _coerce_beat_times, + build_demo_rehearsal_song, +) +from bandscope_analysis.temporal.accelerando import ( + _is_named_vocal_or_bass, + accelerando_plan_copy, + apply_accelerando_plan, + derive_beat_times, + first_accelerando, + format_accelerando_bpm, + is_accelerando_change, +) +from bandscope_analysis.temporal.stability import analyze_tempo_stability + +_ACCEL_PLAN = "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner." + + +def _beats_80_to_120() -> list[float]: + """Return beat times that lift from 80 BPM to 120 BPM around 11.25s.""" + beats = [i * 0.75 for i in range(16)] + for _ in range(16): + beats.append(beats[-1] + 0.5) + return beats + + +def _beats_60_to_120() -> list[float]: + """Return beat times that jump from 60 BPM to double-time 120 BPM.""" + beats = [i * 1.0 for i in range(16)] + for _ in range(16): + beats.append(beats[-1] + 0.5) + return beats + + +def _role( + role_id: str, + *, + name: str | None = None, + role_type: str = "instrument", + priority: str = "high", +) -> dict[str, Any]: + """Return a minimal rehearsal role fixture.""" + display = name if name is not None else role_id + return { + "id": role_id, + "name": display, + "roleType": role_type, + "rehearsalPriority": priority, + "harmony": {"chord": "C#m7", "functionLabel": "vi", "source": "model"}, + "cue": {"kind": "transition", "value": "Hold"}, + "range": {"lowestNote": "C#2", "highestNote": "E3"}, + "confidence": {"level": "high", "source": "model", "notes": "ok"}, + "simplification": "Stay on roots.", + "setupNote": "Keep the attack short.", + "manualOverrides": [], + "overlapWarnings": [], + } + + +def _song_with_section( + *, + start: int = 0, + end: int = 16, + roles: list[dict[str, Any]] | None = None, + part_graph: list[dict[str, Any]] | None = None, +) -> dict[str, Any]: + """Return a one-section song that can receive an accelerando stamp.""" + section_roles = roles or [ + _role("keys-right", name="Keyboard 1 Right Hand", role_type="hand"), + _role("lead-vocal", name="Lead Vocal", role_type="vocal"), + _role("bass-guitar", name="Bass Guitar"), + ] + graph = part_graph or [ + {"role_id": role["id"], "is_active": True, "handoff_to": [], "handoff_from": []} + for role in section_roles + ] + return { + "id": "analyzed-song", + "title": "Late Night Set", + "sections": [ + { + "id": "chorus-1", + "label": "chorus", + "groove": "Lifted chorus downbeat", + "timeRange": {"start": start, "end": end}, + "roles": section_roles, + "partGraph": graph, + } + ], + } + + +def test_format_accelerando_bpm_tokens() -> None: + """Whole BPM values drop the decimal; unusable values stay unnamed.""" + assert format_accelerando_bpm(120.0) == "120" + assert format_accelerando_bpm(96.5) == "96.5" + assert format_accelerando_bpm(0) is None + assert format_accelerando_bpm(-12) is None + assert format_accelerando_bpm(float("nan")) is None + assert format_accelerando_bpm(float("inf")) is None + + +def test_accelerando_plan_copy_uses_owned_template() -> None: + """Model copy names the speeding without inventing other rehearsal plans.""" + assert accelerando_plan_copy(80, 120) == _ACCEL_PLAN + assert accelerando_plan_copy(0, 80) is None + + +def test_first_accelerando_picks_the_earliest_speeding() -> None: + """80 to 120 is an accelerando; later ritardando is ignored.""" + result = analyze_tempo_stability(_beats_80_to_120()) + change = first_accelerando(result["tempo_changes"]) + assert change is not None + assert abs(change["from_bpm"] - 80.0) < 1.0 + assert abs(change["to_bpm"] - 120.0) < 1.0 + assert 10.5 <= change["time"] <= 12.5 + + +def test_first_accelerando_excludes_double_time() -> None: + """A 60 to 120 feel flip is double-time, not an accelerando.""" + result = analyze_tempo_stability(_beats_60_to_120()) + assert first_accelerando(result["tempo_changes"]) is None + assert is_accelerando_change({"time": 8.0, "from_bpm": 60.0, "to_bpm": 120.0}) is False + + +def test_first_accelerando_excludes_ritardando_and_double_time() -> None: + """Slowing down, including ritardando and half-time, is not an accelerando.""" + assert is_accelerando_change({"time": 8.0, "from_bpm": 80.0, "to_bpm": 120.0}) is True + assert is_accelerando_change({"time": 8.0, "from_bpm": 120.0, "to_bpm": 80.0}) is False + assert is_accelerando_change({"time": 8.0, "from_bpm": 60.0, "to_bpm": 120.0}) is False + assert first_accelerando([{"time": 8.0, "from_bpm": 60.0, "to_bpm": 120.0}]) is None + + +def test_first_accelerando_fails_closed_on_malformed_changes() -> None: + """Malformed tempo-change collections never invent a rit.""" + assert first_accelerando(None) is None + assert first_accelerando("tempo") is None + assert first_accelerando([None, "x", {"from_bpm": True, "to_bpm": 80}]) is None + assert is_accelerando_change({"from_bpm": True, "to_bpm": 80}) is False + assert is_accelerando_change({"from_bpm": 120, "to_bpm": True}) is False + assert is_accelerando_change({"from_bpm": 120, "to_bpm": float("nan")}) is False + assert first_accelerando([{"from_bpm": 80, "to_bpm": 120, "time": True}]) is None + assert first_accelerando([{"from_bpm": 80, "to_bpm": 120, "time": -1}]) is None + assert first_accelerando([{"from_bpm": 80, "to_bpm": 120}]) is None + assert first_accelerando([{"from_bpm": 120, "to_bpm": 80, "time": True}]) is None + + +def test_apply_stamps_highest_priority_named_vocal() -> None: + """The named vocal owns the accel when it outranks bass in the same section.""" + song = _song_with_section() + apply_accelerando_plan(song, _beats_80_to_120()) + vocal = next(role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal") + bass = next(role for role in song["sections"][0]["roles"] if role["id"] == "bass-guitar") + keys = next(role for role in song["sections"][0]["roles"] if role["id"] == "keys-right") + assert vocal["accelerandoPlan"] == _ACCEL_PLAN + assert vocal["accelerandoPlanSource"] == "model" + assert "accelerandoPlan" not in bass + assert "accelerandoPlan" not in keys + + +def test_apply_stamps_bass_when_vocal_is_inactive() -> None: + """Bass owns the accel when the vocal is not active in the section.""" + song = _song_with_section( + part_graph=[ + {"role_id": "keys-right", "is_active": True, "handoff_to": [], "handoff_from": []}, + {"role_id": "lead-vocal", "is_active": False, "handoff_to": [], "handoff_from": []}, + {"role_id": "bass-guitar", "is_active": True, "handoff_to": [], "handoff_from": []}, + ] + ) + apply_accelerando_plan(song, _beats_80_to_120()) + bass = next(role for role in song["sections"][0]["roles"] if role["id"] == "bass-guitar") + vocal = next(role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal") + assert bass["accelerandoPlan"] == _ACCEL_PLAN + assert "accelerandoPlan" not in vocal + + +def test_apply_stays_unnamed_without_named_vocal_or_bass() -> None: + """Accompaniment hands never own an accelerando plan.""" + song = _song_with_section( + roles=[_role("keys-right", name="Keyboard 1 Right Hand", role_type="hand")], + part_graph=[ + {"role_id": "keys-right", "is_active": True, "handoff_to": [], "handoff_from": []} + ], + ) + apply_accelerando_plan(song, _beats_80_to_120()) + assert all("accelerandoPlan" not in role for role in song["sections"][0]["roles"]) + + +def test_apply_stays_unnamed_when_section_does_not_contain_the_change() -> None: + """An accel outside every section window stays unnamed.""" + song = _song_with_section(start=40, end=56) + apply_accelerando_plan(song, _beats_80_to_120()) + assert all("accelerandoPlan" not in role for role in song["sections"][0]["roles"]) + + +def test_apply_stays_unnamed_on_double_time_and_missing_beats() -> None: + """Double-time, missing beats, and demo topology stay unnamed.""" + song = _song_with_section() + apply_accelerando_plan(song, _beats_60_to_120()) + assert all("accelerandoPlan" not in role for role in song["sections"][0]["roles"]) + apply_accelerando_plan(song, None) + apply_accelerando_plan(song, "beats") # type: ignore[arg-type] + demo = build_demo_rehearsal_song({"beat_times": _beats_80_to_120(), "bpm": 120}) + assert demo["id"] == "demo-song" + assert all( + "accelerandoPlan" not in role for section in demo["sections"] for role in section["roles"] + ) + + +def test_apply_skips_repeated_and_blank_identities() -> None: + """Repeated graph ids, blank names, and unknown priorities fail closed.""" + roles = [ + _role("lead-vocal", name="Lead Vocal", role_type="vocal"), + _role("lead-vocal", name="Double Vocal", role_type="vocal"), + _role("bass-guitar", name=""), + _role("mystery", name="Mystery", priority="urgent"), + ] + song = _song_with_section( + roles=roles, + part_graph=[ + {"role_id": "lead-vocal", "is_active": True, "handoff_to": [], "handoff_from": []}, + {"role_id": "lead-vocal", "is_active": True, "handoff_to": [], "handoff_from": []}, + {"role_id": "bass-guitar", "is_active": True, "handoff_to": [], "handoff_from": []}, + {"role_id": "mystery", "is_active": True, "handoff_to": [], "handoff_from": []}, + ], + ) + apply_accelerando_plan(song, _beats_80_to_120()) + assert all("accelerandoPlan" not in role for role in song["sections"][0]["roles"]) + + +def test_apply_fails_closed_on_malformed_song_topology() -> None: + """Malformed sections, ranges, and graph nodes never invent a rit.""" + apply_accelerando_plan({"sections": "nope"}, _beats_80_to_120()) + apply_accelerando_plan({"sections": [{"timeRange": "nope", "roles": []}]}, _beats_80_to_120()) + song = _song_with_section() + song["sections"][0]["timeRange"] = {"start": True, "end": 16} + apply_accelerando_plan(song, _beats_80_to_120()) + song = _song_with_section() + song["sections"][0]["timeRange"] = {"start": 10, "end": True} + apply_accelerando_plan(song, _beats_80_to_120()) + song = _song_with_section() + song["sections"][0]["roles"] = None + apply_accelerando_plan(song, _beats_80_to_120()) + song = _song_with_section() + song["sections"][0]["partGraph"] = "graph" + apply_accelerando_plan(song, _beats_80_to_120()) + song = _song_with_section() + song["sections"][0]["roles"] = [ + "not-a-role", + _role("bass-guitar", name="Bass Guitar", priority="urgent"), + ] + apply_accelerando_plan(song, _beats_80_to_120()) + assert _is_named_vocal_or_bass({"id": ""}) is False + assert _is_named_vocal_or_bass({"id": "choir", "roleType": "vocal"}) is True + + +def test_apply_fails_closed_when_copy_cannot_be_built( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A corroborated change without owned copy stays unnamed.""" + song = _song_with_section() + monkeypatch.setattr( + "bandscope_analysis.temporal.accelerando.accelerando_plan_copy", + lambda *_args, **_kwargs: None, + ) + apply_accelerando_plan(song, _beats_80_to_120()) + assert all("accelerandoPlan" not in role for role in song["sections"][0]["roles"]) + + +def test_apply_fails_closed_when_song_get_raises() -> None: + """Hostile song mappings fail closed instead of escaping.""" + + class HostileSong(dict[str, Any]): + """Mapping that raises when sections are read.""" + + def get(self, key: str, default: Any = None) -> Any: + """Raise on sections so apply_accelerando_plan must fail closed.""" + if key == "sections": + raise TypeError("hostile sections") + return super().get(key, default) + + apply_accelerando_plan(HostileSong(), _beats_80_to_120()) + + +def test_derive_beat_times_fails_closed_and_reuses_librosa( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """In-memory beat derivation fails closed and reuses existing beat tracking.""" + assert derive_beat_times(np.zeros(0, dtype=np.float32), 22050) is None + assert derive_beat_times(np.ones(16, dtype=np.float32), True) is None # type: ignore[arg-type] + assert derive_beat_times(np.ones(16, dtype=np.float32), 0) is None + + class _Librosa: + """Minimal librosa stand-in for beat tracking.""" + + class beat: + """Beat-tracking namespace.""" + + @staticmethod + def beat_track(*, y: Any, sr: int) -> tuple[float, np.ndarray]: + """Return a tiny beat-frame grid.""" + return 120.0, np.array([0, 10, 20], dtype=np.int32) + + @staticmethod + def frames_to_time(frames: np.ndarray, sr: int) -> np.ndarray: + """Convert frames to seconds.""" + return frames.astype(np.float64) / sr + + monkeypatch.setitem(__import__("sys").modules, "librosa", _Librosa) + derived = derive_beat_times(np.ones(32, dtype=np.float32), 10) + assert derived == [0.0, 1.0, 2.0] + + class _Boom: + """Librosa stand-in that fails closed.""" + + class beat: + """Beat-tracking namespace that raises.""" + + @staticmethod + def beat_track(*, y: Any, sr: int) -> tuple[float, np.ndarray]: + """Force beat tracking to fail closed.""" + raise RuntimeError("beat tracking unavailable") + + @staticmethod + def frames_to_time(frames: np.ndarray, sr: int) -> np.ndarray: + """Unused converter.""" + return frames.astype(np.float64) + + monkeypatch.setitem(__import__("sys").modules, "librosa", _Boom) + assert derive_beat_times(np.ones(32, dtype=np.float32), 22050) is None + + +def test_coerce_beat_times_and_pipeline_stamp() -> None: + """Pipeline features stamp a rit; malformed beat times fall through to mix derivation.""" + assert _coerce_beat_times(None) is None + assert _coerce_beat_times({"beat_times": []}) is None + assert _coerce_beat_times({"beat_times": [0.0, True]}) is None + assert _coerce_beat_times({"beat_times": [0.0, float("nan")]}) is None + assert _coerce_beat_times({"beat_times": _beats_80_to_120()})[0] == 0.0 + + song = _song_with_section() + mix = np.ones(8, dtype=np.float32) + _apply_accelerando(song, mix, 22050, {"beat_times": _beats_80_to_120()}) + vocal = next(role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal") + assert vocal["accelerandoPlan"] == _ACCEL_PLAN + + unnamed = _song_with_section() + _apply_accelerando(unnamed, np.zeros(0, dtype=np.float32), 22050, {"beat_times": "nope"}) + assert all("accelerandoPlan" not in role for role in unnamed["sections"][0]["roles"]) + + +def test_pipeline_stamps_accelerando_from_provided_beat_times() -> None: + """Real stem pipeline receives beat times and names the accel on the map.""" + sr = 8 + duration = 16.0 + audio = np.ones(int(sr * duration), dtype=np.float32) + song = build_demo_rehearsal_song( + { + "stems": {"bass": audio, "other": audio, "vocals": audio}, + "sr": sr, + "separation": {"duration_seconds": duration, "chunk_count": 1, "notes": "test"}, + "beat_times": _beats_80_to_120(), + } + ) + if song["id"] != "analyzed-song": + pytest.skip("pipeline fell back to arrangement without structural sections") + stamped = [ + role + for section in song["sections"] + for role in section["roles"] + if role.get("accelerandoPlan") + ] + assert len(stamped) <= 1 + if stamped: + assert stamped[0]["accelerandoPlanSource"] == "model" + assert stamped[0]["id"] in {"lead-vocal", "bass-guitar"} From 075352dc43ca81376d8c93bf32e2bb983e16a0a0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 05:17:03 -0700 Subject: [PATCH 02/23] test(workspace): reject false accelerando model copy --- .../workspace/firstAccelerando.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts index fc0a268be..49d4736a3 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.test.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -148,6 +148,28 @@ describe("resolveFirstAccelerandoPlan", () => { ).toBeNull(); }); + it("rejects model copy that is not a genuine non-double-time speeding", () => { + expect( + resolveFirstAccelerandoPlan( + withAccelerandoSection({ + accelerandoPlan: + "Push this part from 120 BPM into 80 BPM; let the next downbeat arrive sooner." + }) + ) + ).toBeNull(); + expect( + resolveFirstAccelerandoPlan( + withAccelerandoSection({ + accelerandoPlan: + "Push this part from 60 BPM into 120 BPM; let the next downbeat arrive sooner." + }) + ) + ).toBeNull(); + expect(resolveFirstAccelerandoPlan(withAccelerandoSection())?.accelerandoPlan).toBe( + DEMO_ACCELERANDO_PLAN + ); + }); + it("admits bounded user copy without requiring the engine template", () => { const resolved = resolveFirstAccelerandoPlan( withAccelerandoSection({ From d289215cd1c38d185c746a3d4ebc4adc15df2b7f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 05:18:11 -0700 Subject: [PATCH 03/23] fix(workspace): validate accelerando tempo semantics --- .../features/workspace/firstAccelerando.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts index 7924b4e1f..98adbdb25 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -13,6 +13,8 @@ const NAMED_ACCELERANDO_ROLE_IDS = new Set(["bass-guitar", "lead-vocal"]); const ACCELERANDO_PLAN_PREFIX = "Push this part from "; const ACCELERANDO_PLAN_MIDDLE = " BPM into "; const ACCELERANDO_PLAN_SUFFIX = " BPM; let the next downbeat arrive sooner."; +const DOUBLE_TIME_RATIO_MIN = 1.9; +const DOUBLE_TIME_RATIO_MAX = 2.1; type AccelerandoPlanSource = "model" | "user"; @@ -130,7 +132,7 @@ function truncateCodePoints(value: string, maximum: number): string { return endIndex === value.length ? value : value.slice(0, endIndex); } -/** Preserve the engine accelerando template while bounding its model-owned tempo tokens. */ +/** Preserve the engine accelerando template while enforcing the engine's speeding semantics. */ function boundedGeneratedAccelerandoPlan(value: string): OwnedAccelerandoPlan | null { if ( !value.startsWith(ACCELERANDO_PLAN_PREFIX) || @@ -149,6 +151,21 @@ function boundedGeneratedAccelerandoPlan(value: string): OwnedAccelerandoPlan | if (!/^\d+(?:\.\d+)?$/u.test(fromBpm) || !/^\d+(?:\.\d+)?$/u.test(toBpm)) { return null; } + const fromBpmValue = Number(fromBpm); + const toBpmValue = Number(toBpm); + if ( + !Number.isFinite(fromBpmValue) || + !Number.isFinite(toBpmValue) || + fromBpmValue <= 0 || + toBpmValue <= 0 || + toBpmValue <= fromBpmValue + ) { + return null; + } + const ratio = toBpmValue / fromBpmValue; + if (ratio >= DOUBLE_TIME_RATIO_MIN && ratio <= DOUBLE_TIME_RATIO_MAX) { + return null; + } return { text: `${ACCELERANDO_PLAN_PREFIX}${fromBpm}${ACCELERANDO_PLAN_MIDDLE}${toBpm}${ACCELERANDO_PLAN_SUFFIX}`, source: "model", From 5be5c31d11fdf616506ff70f59905143df203c7d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 05:18:34 -0700 Subject: [PATCH 04/23] test(analysis): prevent accelerando role leakage across sections --- .../test_accelerando_shared_role_isolation.py | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 services/analysis-engine/tests/test_accelerando_shared_role_isolation.py diff --git a/services/analysis-engine/tests/test_accelerando_shared_role_isolation.py b/services/analysis-engine/tests/test_accelerando_shared_role_isolation.py new file mode 100644 index 000000000..cb2339865 --- /dev/null +++ b/services/analysis-engine/tests/test_accelerando_shared_role_isolation.py @@ -0,0 +1,96 @@ +"""Regression tests for section-local accelerando role mutation.""" + +from typing import Any + +from bandscope_analysis.temporal.accelerando import apply_accelerando_plan + + +def _beats_80_to_120() -> list[float]: + """Return beat times that speed from 80 BPM to 120 BPM around 11.25 seconds.""" + beats = [index * 0.75 for index in range(16)] + for _ in range(16): + beats.append(beats[-1] + 0.5) + return beats + + +def _shared_vocal() -> dict[str, Any]: + """Return one role object deliberately shared by two section fixtures.""" + return { + "id": "lead-vocal", + "name": "Lead Vocal", + "roleType": "vocal", + "rehearsalPriority": "high", + } + + +def _section(section_id: str, start: int, end: int, role: dict[str, Any]) -> dict[str, Any]: + """Return a minimal section containing the supplied shared role object.""" + return { + "id": section_id, + "label": "verse", + "timeRange": {"start": start, "end": end}, + "roles": [role], + "partGraph": [ + { + "role_id": "lead-vocal", + "is_active": True, + "handoff_to": [], + "handoff_from": [], + } + ], + } + + +class _ChangingRolesSection(dict[str, Any]): + """Section mapping that changes its roles collection after selection.""" + + def __init__(self, initial_role: dict[str, Any], replacement: object) -> None: + """Store a valid first roles read and the later replacement value.""" + super().__init__(_section("verse-changing", 8, 20, initial_role)) + self._roles_reads = 0 + self._replacement = replacement + + def get(self, key: str, default: Any = None) -> Any: + """Return a different roles payload after the landing role is selected.""" + if key == "roles": + self._roles_reads += 1 + if self._roles_reads > 1: + return self._replacement + return super().get(key, default) + + +def test_accelerando_stamp_does_not_leak_through_a_shared_role_object() -> None: + """Only the section containing the tempo change receives the owned plan copy.""" + shared_role = _shared_vocal() + earlier = _section("verse-1", 0, 8, shared_role) + containing = _section("verse-2", 8, 20, shared_role) + song = {"id": "shared-role-song", "title": "Shared Role", "sections": [earlier, containing]} + + apply_accelerando_plan(song, _beats_80_to_120()) + + assert "accelerandoPlan" not in earlier["roles"][0] + assert containing["roles"][0]["accelerandoPlanSource"] == "model" + + +def test_accelerando_stamp_fails_closed_if_roles_stop_being_a_list() -> None: + """A runtime section that changes shape after selection receives no stamp.""" + role = _shared_vocal() + section = _ChangingRolesSection(role, "not-a-role-list") + song = {"id": "changing-song", "title": "Changing", "sections": [section]} + + apply_accelerando_plan(song, _beats_80_to_120()) + + assert "accelerandoPlan" not in role + + +def test_accelerando_stamp_fails_closed_if_selected_role_identity_disappears() -> None: + """A replaced roles list cannot receive a stamp through stale object identity.""" + role = _shared_vocal() + replacement_role = dict(role) + section = _ChangingRolesSection(role, [replacement_role]) + song = {"id": "drifting-song", "title": "Drifting", "sections": [section]} + + apply_accelerando_plan(song, _beats_80_to_120()) + + assert "accelerandoPlan" not in role + assert "accelerandoPlan" not in replacement_role From fcfd7613c75521c7bdb05c74db941f72ff00e317 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 05:19:10 -0700 Subject: [PATCH 05/23] fix(analysis): isolate accelerando stamps by section --- .../src/bandscope_analysis/temporal/accelerando.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py index 9b3aa58a1..9d8ef711c 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py @@ -262,8 +262,17 @@ def apply_accelerando_plan(song: Mapping[str, Any], beat_times: Sequence[float] landing = _pick_landing_role(section) if landing is None: return - landing["accelerandoPlan"] = copy - landing["accelerandoPlanSource"] = "model" + roles = section.get("roles") + if not isinstance(roles, list): + return + for index, role in enumerate(roles): + if role is not landing: + continue + stamped = dict(landing) + stamped["accelerandoPlan"] = copy + stamped["accelerandoPlanSource"] = "model" + roles[index] = stamped + return return except (TypeError, ValueError, KeyError, AttributeError): return From 5d4d20d876b8c30487cd855441d747abbda0f48b Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 22:01:22 +0900 Subject: [PATCH 06/23] fix(analysis): sort temporal exports --- .../analysis-engine/src/bandscope_analysis/temporal/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py b/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py index 5b82ff2b1..ba4056a10 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py @@ -1,9 +1,9 @@ """Temporal analysis module (audio decoding, tempo, beat tracking).""" +from .accelerando import accelerando_plan_copy, apply_accelerando_plan, first_accelerando from .analyzer import TemporalAnalyzer from .groove import GrooveResult, detect_groove from .model import TemporalFeatures -from .accelerando import apply_accelerando_plan, first_accelerando, accelerando_plan_copy from .stability import TempoChange, TempoStability, analyze_tempo_stability __all__ = [ From 9a0212e7e8541f73668ce7203b07b440647da62a Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 22:40:57 +0900 Subject: [PATCH 07/23] fix(accelerando): reuse temporal analysis features --- .../workspace/FirstAccelerandoCallout.tsx | 4 ++-- .../workspace/firstAccelerando.test.ts | 8 ++++++++ .../features/workspace/firstAccelerando.ts | 12 ++++-------- .../src/bandscope_analysis/api.py | 19 ++++++++++++++++--- .../src/bandscope_analysis/cli.py | 10 ++++++---- .../tests/test_accelerando_plan.py | 17 +++++++++++++++++ services/analysis-engine/tests/test_cli.py | 3 ++- 7 files changed, 55 insertions(+), 18 deletions(-) diff --git a/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx index fdc829541..cb10e17ec 100644 --- a/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx +++ b/apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx @@ -29,7 +29,7 @@ type OpenedAccelerandoPlan = Readonly<{ landingRoleId: string; landingRoleName: string; accelerandoPlan: string; - accelerandoPlanSource: AccelerandoPlanSource | null; + accelerandoPlanSource: AccelerandoPlanSource; fromBpm: string | null; toBpm: string | null; atSeconds: number; @@ -54,7 +54,7 @@ function formatAccelerandoPlanCopy(template: string, values: AccelerandoPlanCopy /** Localize model accelerando guidance from structured tempo tokens, never from display-copy grammar. */ function localizedAccelerandoPlan( accelerandoPlan: string, - accelerandoPlanSource: AccelerandoPlanSource | null, + accelerandoPlanSource: AccelerandoPlanSource, guidance: AccelerandoPlanGuidance | null, generatedTemplate: string ): string { diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts index 49d4736a3..2bf6f2b68 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.test.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -181,6 +181,14 @@ describe("resolveFirstAccelerandoPlan", () => { expect(resolved?.accelerandoPlanSource).toBe("user"); }); + it("fails closed when persisted accelerando copy has no provenance", () => { + const song = withAccelerandoSection(); + delete song.sections[0]!.roles.find((role) => role.id === "lead-vocal")! + .accelerandoPlanSource; + + expect(resolveFirstAccelerandoPlan(song)).toBeNull(); + }); + it("fails closed on a malformed runtime song root", () => { expect(resolveFirstAccelerandoPlan(null as never)).toBeNull(); }); diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts index 98adbdb25..fbee9ae5c 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -35,7 +35,7 @@ type RankedRoleMetadata = Readonly<{ type OwnedAccelerandoPlan = Readonly<{ text: string; - source: AccelerandoPlanSource | null; + source: AccelerandoPlanSource; guidance: AccelerandoPlanGuidance | null; }>; @@ -49,7 +49,7 @@ export type FirstAccelerandoPlan = { landingRoleId: string; landingRoleName: string; accelerandoPlan: string; - accelerandoPlanSource: AccelerandoPlanSource | null; + accelerandoPlanSource: AccelerandoPlanSource; accelerandoPlanGuidance: AccelerandoPlanGuidance | null; atSeconds: number; }; @@ -183,11 +183,7 @@ function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { if (typeof accelerandoPlan !== "string") { return null; } - if ( - accelerandoPlanSource !== undefined && - accelerandoPlanSource !== "model" && - accelerandoPlanSource !== "user" - ) { + if (accelerandoPlanSource !== "model" && accelerandoPlanSource !== "user") { return null; } const trimmed = accelerandoPlan.trim(); @@ -199,7 +195,7 @@ function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { } return { text: truncateCodePoints(trimmed, MAX_ACCELERANDO_PLAN_CHARACTERS), - source: accelerandoPlanSource ?? null, + source: accelerandoPlanSource, guidance: null }; } diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index be9cbc71f..c7f7022be 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -1078,8 +1078,13 @@ def run_analysis_job_updates( job_id: str, payload: object, requested_at: str, + temporal_features: dict[str, Any] | None = None, ) -> list[AnalysisJobStatus]: - """Return incremental orchestration status updates for an analysis job.""" + """Return incremental orchestration status updates for an analysis job. + + ``temporal_features`` contains optional features already extracted by the CLI + so the integrated pipeline can reuse beat tracking instead of repeating it. + """ try: request = validate_analysis_job_request(payload) except ValueError as error: @@ -1215,6 +1220,9 @@ def run_analysis_job_updates( ) return updates + if temporal_features: + audio_features = {**(audio_features or {}), **temporal_features} + updates.append( _build_job_status( job_id=job_id, @@ -1263,6 +1271,11 @@ def run_analysis_job_updates( return updates -def run_analysis_job(job_id: str, payload: object, requested_at: str) -> AnalysisJobStatus: +def run_analysis_job( + job_id: str, + payload: object, + requested_at: str, + temporal_features: dict[str, Any] | None = None, +) -> AnalysisJobStatus: """Return a structured orchestration response for a validated analysis job.""" - return run_analysis_job_updates(job_id, payload, requested_at)[-1] + return run_analysis_job_updates(job_id, payload, requested_at, temporal_features)[-1] diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 6838ee711..288429955 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -6,6 +6,7 @@ import logging import sys from datetime import UTC, datetime +from typing import Any, cast from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates from bandscope_analysis.temporal import TemporalAnalyzer @@ -75,8 +76,7 @@ def main() -> int: request = payload.get("request") - # Temporary: Inject temporal analyzer call if it's a local file, just to prove it works - # before full orchestrator integration + temporal_features: dict[str, Any] | None = None if ( isinstance(request, dict) and request.get("sourceKind") == "local_audio" @@ -90,6 +90,8 @@ def main() -> int: try: temporal_analyzer = TemporalAnalyzer() features = temporal_analyzer.analyze(audio_path) + if isinstance(features, dict): + temporal_features = cast(dict[str, Any], features) logging.info(f"Extracted BPM: {features['bpm']}") except Exception: logging.warning( @@ -99,13 +101,13 @@ def main() -> int: requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") if progress_jsonl: - for update in run_analysis_job_updates(job_id, request, requested_at): + for update in run_analysis_job_updates(job_id, request, requested_at, temporal_features): json.dump(update, sys.stdout) sys.stdout.write("\n") sys.stdout.flush() return 0 - response = run_analysis_job(job_id, request, requested_at) + response = run_analysis_job(job_id, request, requested_at, temporal_features) json.dump(response, sys.stdout) return 0 diff --git a/services/analysis-engine/tests/test_accelerando_plan.py b/services/analysis-engine/tests/test_accelerando_plan.py index 8babf74cb..85272f514 100644 --- a/services/analysis-engine/tests/test_accelerando_plan.py +++ b/services/analysis-engine/tests/test_accelerando_plan.py @@ -359,6 +359,23 @@ def test_coerce_beat_times_and_pipeline_stamp() -> None: assert all("accelerandoPlan" not in role for role in unnamed["sections"][0]["roles"]) +def test_apply_accelerando_reuses_provided_beat_times(monkeypatch: pytest.MonkeyPatch) -> None: + """Provided temporal features avoid a second beat-tracking pass.""" + + def fail_if_derived(*args: Any, **kwargs: Any) -> list[float] | None: + raise AssertionError("beat times should be reused") + + monkeypatch.setattr("bandscope_analysis.api.derive_beat_times", fail_if_derived) + song = _song_with_section() + + _apply_accelerando( + song, np.ones(8, dtype=np.float32), 22050, {"beat_times": _beats_80_to_120()} + ) + + vocal = next(role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal") + assert vocal["accelerandoPlan"] == _ACCEL_PLAN + + def test_pipeline_stamps_accelerando_from_provided_beat_times() -> None: """Real stem pipeline receives beat times and names the accel on the map.""" sr = 8 diff --git a/services/analysis-engine/tests/test_cli.py b/services/analysis-engine/tests/test_cli.py index 057ef236b..832a165fe 100644 --- a/services/analysis-engine/tests/test_cli.py +++ b/services/analysis-engine/tests/test_cli.py @@ -387,7 +387,7 @@ def test_cli_main_temporal_analyzer_mock_success( class FakeAnalyzerSuccess: def analyze(self, path): - return {"bpm": 120.0, "beats": []} + return {"bpm": 120.0, "beat_times": [0.0], "beats": []} monkeypatch.setattr(cli, "TemporalAnalyzer", FakeAnalyzerSuccess) monkeypatch.setattr( @@ -405,6 +405,7 @@ def analyze(self, path): assert cli.main() == 0 res = json.loads(stdout.getvalue()) assert res["jobId"] == "job-audio-success" + assert res["result"]["tempo"] == 120 def test_cli_main_progress_jsonl_streams_status_updates( From ee34e812995b8de80d941d3a59cc2022127b6b47 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 23:27:25 +0900 Subject: [PATCH 08/23] fix(analysis): preserve typed temporal features --- services/analysis-engine/src/bandscope_analysis/cli.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 288429955..ca6680764 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -6,7 +6,7 @@ import logging import sys from datetime import UTC, datetime -from typing import Any, cast +from typing import Any from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates from bandscope_analysis.temporal import TemporalAnalyzer @@ -90,8 +90,7 @@ def main() -> int: try: temporal_analyzer = TemporalAnalyzer() features = temporal_analyzer.analyze(audio_path) - if isinstance(features, dict): - temporal_features = cast(dict[str, Any], features) + temporal_features = features logging.info(f"Extracted BPM: {features['bpm']}") except Exception: logging.warning( From bb0501b83840432e6fbe77da338217e13045f980 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 23:30:36 +0900 Subject: [PATCH 09/23] fix(cache): invalidate stale analysis results --- services/analysis-engine/src/bandscope_analysis/api.py | 4 ++-- services/analysis-engine/tests/test_api.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index c7f7022be..f75281f58 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -24,7 +24,7 @@ logger = logging.getLogger(__name__) MAX_SECTION_TIME_SECONDS = 4_294_967_295 -ANALYSIS_CACHE_SCHEMA_VERSION = 1 +ANALYSIS_CACHE_SCHEMA_VERSION = 2 FEATURE_CACHE_SCHEMA_VERSION = 1 STEM_SEPARATION_TIMEOUT_SECONDS = 20.0 @@ -648,7 +648,7 @@ def _analysis_cache_path(request: AnalysisJobRequest) -> Path | None: digest = hashlib.sha256( json.dumps(key_payload, sort_keys=True, separators=(",", ":")).encode("utf-8") ).hexdigest() - return Path(cache_root) / "analysis-cache-v1" / f"{digest}.json" + return Path(cache_root) / "analysis-cache-v2" / f"{digest}.json" def _feature_cache_paths(request: AnalysisJobRequest) -> tuple[Path, Path] | None: diff --git a/services/analysis-engine/tests/test_api.py b/services/analysis-engine/tests/test_api.py index 18273791d..c9bc86820 100644 --- a/services/analysis-engine/tests/test_api.py +++ b/services/analysis-engine/tests/test_api.py @@ -582,7 +582,7 @@ def test_run_analysis_job_updates_report_progress_and_cache(tmp_path) -> None: ("succeeded", "ready", 100), ] assert updates[-1]["cacheStatus"] == "stored" - cache_files = list((tmp_path / "cache" / "analysis-cache-v1").glob("*.json")) + cache_files = list((tmp_path / "cache" / "analysis-cache-v2").glob("*.json")) assert len([path for path in cache_files if not path.name.endswith(".features.json")]) == 1 assert len([path for path in cache_files if path.name.endswith(".features.json")]) == 1 From 50cbef3862f8648c6ceb676b6c0a708bfe422518 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 23:34:21 +0900 Subject: [PATCH 10/23] fix(analysis): retain typed temporal feature cast --- services/analysis-engine/src/bandscope_analysis/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index ca6680764..6402ccde4 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -6,7 +6,7 @@ import logging import sys from datetime import UTC, datetime -from typing import Any +from typing import Any, cast from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates from bandscope_analysis.temporal import TemporalAnalyzer @@ -90,7 +90,7 @@ def main() -> int: try: temporal_analyzer = TemporalAnalyzer() features = temporal_analyzer.analyze(audio_path) - temporal_features = features + temporal_features = cast(dict[str, Any], features) logging.info(f"Extracted BPM: {features['bpm']}") except Exception: logging.warning( From 1778dba18d605a2655191fea6384df72984d9cf1 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 28 Aug 2026 23:37:41 +0900 Subject: [PATCH 11/23] test(cache): cover current schema misses --- services/analysis-engine/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_api.py b/services/analysis-engine/tests/test_api.py index c9bc86820..93f3ee962 100644 --- a/services/analysis-engine/tests/test_api.py +++ b/services/analysis-engine/tests/test_api.py @@ -648,7 +648,7 @@ def test_cached_analysis_helpers_treat_invalid_cache_as_miss(tmp_path) -> None: for content in ( "[]", '{"schemaVersion": 999, "result": {}}', - '{"schemaVersion": 1, "result": []}', + '{"schemaVersion": 2, "result": []}', ): cache_path.write_text(content, encoding="utf-8") assert _load_cached_analysis(cache_path) is None From da317b2dffbc662c1bc037f4a32e339a36c06fb7 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 05:35:18 +0900 Subject: [PATCH 12/23] fix(accelerando): validate inputs before analysis --- .../workspace/firstAccelerando.test.ts | 7 +++ .../features/workspace/firstAccelerando.ts | 10 ++-- .../src/bandscope_analysis/cli.py | 49 ++++++++++++------- services/analysis-engine/tests/test_cli.py | 36 ++++++++++++++ 4 files changed, 80 insertions(+), 22 deletions(-) diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts index 2bf6f2b68..81804c342 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.test.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -193,6 +193,13 @@ describe("resolveFirstAccelerandoPlan", () => { expect(resolveFirstAccelerandoPlan(null as never)).toBeNull(); }); + it("rejects a sparse hostile section array without scanning its declared length", () => { + const song = createDemoRehearsalSong(); + song.sections = new Array(0xffffffff) as typeof song.sections; + + expect(resolveFirstAccelerandoPlan(song)).toBeNull(); + }); + it("fails closed on inherited or accessor-backed plan copy", () => { const song = withAccelerandoSection(); const vocal = song.sections[0]!.roles.find((role) => role.id === "lead-vocal")!; diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts index fbee9ae5c..67fb8ede7 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -108,12 +108,16 @@ function ownedDenseRuntimeArray(value: unknown): unknown[] | null { ) { return null; } + const keys = Object.keys(value); + if (keys.length !== length) { + return null; + } const items: unknown[] = []; - for (let index = 0; index < length; index += 1) { - if (!hasOwnData(value, index)) { + for (const [index, key] of keys.entries()) { + if (key !== String(index) || !hasOwnData(value, key)) { return null; } - items.push(ownDataValue(value, index)); + items.push(ownDataValue(value, key)); } return items; } diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 6402ccde4..8f960f6c0 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -8,7 +8,12 @@ from datetime import UTC, datetime from typing import Any, cast -from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates +from bandscope_analysis.api import ( + get_analysis_status, + run_analysis_job, + run_analysis_job_updates, + validate_analysis_job_request, +) from bandscope_analysis.temporal import TemporalAnalyzer logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") @@ -75,28 +80,34 @@ def main() -> int: return 0 request = payload.get("request") + validated_request = None + try: + validated_request = validate_analysis_job_request(request) + except ValueError: + pass + if validated_request is not None: + request = validated_request temporal_features: dict[str, Any] | None = None if ( - isinstance(request, dict) - and request.get("sourceKind") == "local_audio" - and "localSource" in request + validated_request is not None + and validated_request["sourceKind"] == "local_audio" + and "localSource" in validated_request ): - local_source = request["localSource"] - audio_path = local_source.get("sourcePath") - file_name = local_source.get("fileName", "selected audio") - if audio_path: - logging.info("Extracting temporal features from %s...", file_name) - try: - temporal_analyzer = TemporalAnalyzer() - features = temporal_analyzer.analyze(audio_path) - temporal_features = cast(dict[str, Any], features) - logging.info(f"Extracted BPM: {features['bpm']}") - except Exception: - logging.warning( - "Temporal analysis failed for %s; continuing with safe fallback.", - file_name, - ) + local_source = validated_request["localSource"] + audio_path = local_source["sourcePath"] + file_name = local_source["fileName"] + logging.info("Extracting temporal features from %s...", file_name) + try: + temporal_analyzer = TemporalAnalyzer() + features = temporal_analyzer.analyze(audio_path) + temporal_features = cast(dict[str, Any], features) + logging.info(f"Extracted BPM: {features['bpm']}") + except Exception: + logging.warning( + "Temporal analysis failed for %s; continuing with safe fallback.", + file_name, + ) requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") if progress_jsonl: diff --git a/services/analysis-engine/tests/test_cli.py b/services/analysis-engine/tests/test_cli.py index 832a165fe..5963646bc 100644 --- a/services/analysis-engine/tests/test_cli.py +++ b/services/analysis-engine/tests/test_cli.py @@ -357,6 +357,42 @@ def analyze(self, path): assert res["jobId"] == "job-audio" +def test_cli_main_rejects_malformed_local_source_before_temporal_analysis( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Ensure malformed local input cannot reach the file-reading analyzer.""" + stdin = io.StringIO( + json.dumps( + { + "jobId": "job-malformed-audio", + "request": { + "sourceKind": "local_audio", + "projectId": "p1", + "sourceLabel": "test.wav", + "roleFocus": [], + "localSource": "not-a-record", + }, + } + ) + ) + stdout = io.StringIO() + + class ExplodingAnalyzer: + def analyze(self, path): + raise AssertionError("malformed local input reached temporal analysis") + + monkeypatch.setattr(cli, "TemporalAnalyzer", ExplodingAnalyzer) + monkeypatch.setattr(cli.sys, "stdin", stdin) + monkeypatch.setattr(cli.sys, "stdout", stdout) + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + + assert cli.main() == 0 + response = json.loads(stdout.getvalue()) + assert response["jobId"] == "job-malformed-audio" + assert response["state"] == "failed" + assert "localSource" in response["error"]["message"] + + def test_cli_main_temporal_analyzer_mock_success( monkeypatch: pytest.MonkeyPatch, tmp_path, From db7763c45bf49c382c253ade467167201fdae6aa Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 05:40:43 +0900 Subject: [PATCH 13/23] fix(accelerando): avoid empty-grid redecoding --- .../analysis-engine/src/bandscope_analysis/api.py | 2 +- .../analysis-engine/tests/test_accelerando_plan.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index f75281f58..e2853ebc7 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -527,7 +527,7 @@ def _coerce_beat_times(audio_features: dict[str, Any] | None) -> list[float] | N if not audio_features: return None raw = audio_features.get("beat_times") - if not isinstance(raw, list) or not raw: + if not isinstance(raw, list): return None times: list[float] = [] for item in raw: diff --git a/services/analysis-engine/tests/test_accelerando_plan.py b/services/analysis-engine/tests/test_accelerando_plan.py index 85272f514..7ffaf8c55 100644 --- a/services/analysis-engine/tests/test_accelerando_plan.py +++ b/services/analysis-engine/tests/test_accelerando_plan.py @@ -340,10 +340,10 @@ def frames_to_time(frames: np.ndarray, sr: int) -> np.ndarray: assert derive_beat_times(np.ones(32, dtype=np.float32), 22050) is None -def test_coerce_beat_times_and_pipeline_stamp() -> None: +def test_coerce_beat_times_and_pipeline_stamp(monkeypatch: pytest.MonkeyPatch) -> None: """Pipeline features stamp a rit; malformed beat times fall through to mix derivation.""" assert _coerce_beat_times(None) is None - assert _coerce_beat_times({"beat_times": []}) is None + assert _coerce_beat_times({"beat_times": []}) == [] assert _coerce_beat_times({"beat_times": [0.0, True]}) is None assert _coerce_beat_times({"beat_times": [0.0, float("nan")]}) is None assert _coerce_beat_times({"beat_times": _beats_80_to_120()})[0] == 0.0 @@ -354,6 +354,15 @@ def test_coerce_beat_times_and_pipeline_stamp() -> None: vocal = next(role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal") assert vocal["accelerandoPlan"] == _ACCEL_PLAN + def fail_if_redecoded(*args: Any, **kwargs: Any) -> list[float] | None: + raise AssertionError("an empty authoritative beat grid must not be re-decoded") + + with monkeypatch.context() as context: + context.setattr("bandscope_analysis.api.derive_beat_times", fail_if_redecoded) + empty_grid_song = _song_with_section() + _apply_accelerando(empty_grid_song, np.ones(8, dtype=np.float32), 22050, {"beat_times": []}) + assert all("accelerandoPlan" not in role for role in empty_grid_song["sections"][0]["roles"]) + unnamed = _song_with_section() _apply_accelerando(unnamed, np.zeros(0, dtype=np.float32), 22050, {"beat_times": "nope"}) assert all("accelerandoPlan" not in role for role in unnamed["sections"][0]["roles"]) From c1e6c11b0b3f499fcc651e27ba2c1ececc24cb55 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 05:42:13 +0900 Subject: [PATCH 14/23] fix(cli): document deferred request validation --- services/analysis-engine/src/bandscope_analysis/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 8f960f6c0..4ff22fbf1 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -83,8 +83,8 @@ def main() -> int: validated_request = None try: validated_request = validate_analysis_job_request(request) - except ValueError: - pass + except ValueError as error: + logging.debug("Analysis request validation deferred to orchestration: %s", error) if validated_request is not None: request = validated_request From fc0ffa84c7cfefeee6932f2b5252e78ded0bde9d Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 05:44:25 +0900 Subject: [PATCH 15/23] fix(workspace): commit identity refs after render --- .../src/features/workspace/Workspace.tsx | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 183d95366..2528f37de 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo, useRef, memo, type MouseEvent } from "react"; +import { useState, useMemo, useRef, useEffect, memo, type MouseEvent } from "react"; import { parseProjectBootstrapSummary, type ProjectBootstrapSummary, type RehearsalSong, type RehearsalRole } from "@bandscope/shared-types"; import { RoleSwitcher } from "./RoleSwitcher"; import { SectionRoadmap } from "./SectionRoadmap"; @@ -130,15 +130,17 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp const localSongUpdateRef = useRef(null); const workspaceInstanceRef = useRef(song); const previousSongRef = useRef(song); - - if (song !== previousSongRef.current) { - const isLocalWorkspaceUpdate = song === localSongUpdateRef.current; - if (!isLocalWorkspaceUpdate) { - workspaceInstanceRef.current = song; + const isLocalWorkspaceUpdate = song === localSongUpdateRef.current; + + useEffect(() => { + if (song !== previousSongRef.current) { + if (!isLocalWorkspaceUpdate) { + workspaceInstanceRef.current = song; + } + localSongUpdateRef.current = null; + previousSongRef.current = song; } - localSongUpdateRef.current = null; - previousSongRef.current = song; - } + }, [isLocalWorkspaceUpdate, song]); // Extract all unique roles from the song's sections const roleMap = useMemo(() => { @@ -333,7 +335,10 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp

{t("workspaceFirstRangeTitle")}

{firstRangeCopy}

- +
From 403e0dcd08572fc20f898a7191ed0d2335847b15 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 05:57:52 +0900 Subject: [PATCH 16/23] fix(accelerando): preserve workspace state and role fields --- apps/desktop/core/src/lib.rs | 50 +++++++++++++++++-- .../core/tests/accelerando_plan_contract.rs | 24 ++++++++- .../Workspace.accelerando-state.test.tsx | 8 ++- .../src/features/workspace/Workspace.tsx | 2 +- 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index 3dda07e48..f82dab23f 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -176,6 +176,30 @@ pub struct ManualOverridePayload { source: String, } +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct TranscriptionNotePayload { + pitch: String, + onset: f64, + offset: f64, + velocity: f64, +} + +fn deserialize_practice_progress<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let progress = Option::::deserialize(deserializer)?; + if let Some(value) = progress { + if value > 100 { + return Err(serde::de::Error::custom( + "practiceProgress must be between 0 and 100", + )); + } + } + Ok(progress) +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] enum AccelerandoPlanSourcePayload { @@ -190,15 +214,27 @@ pub struct RehearsalRolePayload { name: String, role_type: String, harmony: HarmonyPayload, + #[serde(default, skip_serializing_if = "Option::is_none")] + harmonic_explanation: Option, cue: CuePayload, range: RangePayload, confidence: ConfidencePayload, rehearsal_priority: String, simplification: String, setup_note: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + transposition_plan: Option, manual_overrides: Vec, overlap_warnings: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] + transcription: Option>, + #[serde( + default, + deserialize_with = "deserialize_practice_progress", + skip_serializing_if = "Option::is_none" + )] + practice_progress: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] accelerando_plan: Option, #[serde(default, skip_serializing_if = "Option::is_none")] accelerando_plan_source: Option, @@ -543,11 +579,15 @@ fn validate_accelerando_plan_provenance( ) -> Result { for section in &payload.sections { for role in §ion.roles { - if role.accelerando_plan.as_ref().is_some_and(|accelerando_plan| { - accelerando_plan.trim().is_empty() - || accelerando_plan.contains('\n') - || accelerando_plan.contains('\r') - }) { + if role + .accelerando_plan + .as_ref() + .is_some_and(|accelerando_plan| { + accelerando_plan.trim().is_empty() + || accelerando_plan.contains('\n') + || accelerando_plan.contains('\r') + }) + { return Err("Invalid project file format".to_string()); } if role.accelerando_plan.is_none() && role.accelerando_plan_source.is_some() { diff --git a/apps/desktop/core/tests/accelerando_plan_contract.rs b/apps/desktop/core/tests/accelerando_plan_contract.rs index a64ec1796..6f4f2e164 100644 --- a/apps/desktop/core/tests/accelerando_plan_contract.rs +++ b/apps/desktop/core/tests/accelerando_plan_contract.rs @@ -26,6 +26,7 @@ fn song_with_accelerando_plan() -> Value { "functionLabel": "vi landing", "source": "model" }, + "harmonicExplanation": "The landing keeps the tonal floor clear.", "cue": { "kind": "transition", "value": "Let the next downbeat arrive sooner." @@ -42,8 +43,16 @@ fn song_with_accelerando_plan() -> Value { "rehearsalPriority": "high", "simplification": "Lean into the landing syllable.", "setupNote": "Keep the attack short.", + "transpositionPlan": "Keep the landing shape a whole step lower if needed.", "manualOverrides": [], "overlapWarnings": [], + "transcription": [{ + "pitch": "C#4", + "onset": 1.0, + "offset": 1.5, + "velocity": 0.8 + }], + "practiceProgress": 50, "accelerandoPlan": "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner.", "accelerandoPlanSource": "model" } @@ -73,7 +82,8 @@ fn project_contract_round_trips_accelerando_plan_provenance() { let parsed = project_payload_from_content(&content) .expect("native project contract must accept shared accelerando-plan fields"); - let serialized = serde_json::to_value(parsed).expect("native project contract should serialize"); + let serialized = + serde_json::to_value(parsed).expect("native project contract should serialize"); assert_eq!( serialized["sections"][0]["roles"][0]["accelerandoPlan"], @@ -140,3 +150,15 @@ fn project_contract_rejects_unknown_accelerando_plan_source() { "native persisted contract must reject provenance outside model/user" ); } + +#[test] +fn project_contract_rejects_practice_progress_above_shared_bound() { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0]["practiceProgress"] = json!(101); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!( + project_payload_from_content(&content).is_err(), + "native persisted contract must reject practiceProgress above the shared 0..=100 bound" + ); +} diff --git a/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx index f50a10d70..18e2e2c46 100644 --- a/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx @@ -37,7 +37,7 @@ describe("Workspace accelerando state authority", () => { } }); - it("keeps an opened accel armed after an immutable practice-progress update", () => { + it("keeps an opened accel armed after an immutable edit and role switch", () => { const song = analyzedSongWithAccelerandoPlan(); let updatedSong: RehearsalSong | null = null; const onSongUpdate = vi.fn((nextSong: RehearsalSong) => { @@ -59,5 +59,11 @@ describe("Workspace accelerando state authority", () => { expect( screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible\./) ).toBeTruthy(); + + fireEvent.click(screen.getByRole("tab", { name: "Bass Guitar" })); + + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible\./) + ).toBeTruthy(); }); }); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 2528f37de..9305c980d 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -337,7 +337,7 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
From 3aa8690fd30278433569d1092d628ae9667c088d Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 06:05:38 +0900 Subject: [PATCH 17/23] fix(accelerando): handle external workspace replacement --- .../Workspace.accelerando-state.test.tsx | 15 +++++++++++++++ apps/desktop/src/features/workspace/Workspace.tsx | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx index 18e2e2c46..29eac02f3 100644 --- a/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx +++ b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx @@ -66,4 +66,19 @@ describe("Workspace accelerando state authority", () => { screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible\./) ).toBeTruthy(); }); + + it("resets armed guidance when a new song arrives", () => { + const song = analyzedSongWithAccelerandoPlan(); + const nextSong = structuredClone(song); + const { rerender } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Open Lead Vocal accel at 0:10" })); + expect( + screen.getByText(/Lift Lead Vocal together at 0:10 so the faster landing is audible\./) + ).toBeTruthy(); + + rerender(); + + expect(screen.getByText("Lead Vocal lifts the verse at 0:10.")).toBeTruthy(); + }); }); diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 9305c980d..17faf3c85 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -131,6 +131,8 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp const workspaceInstanceRef = useRef(song); const previousSongRef = useRef(song); const isLocalWorkspaceUpdate = song === localSongUpdateRef.current; + const isExternalWorkspaceUpdate = song !== previousSongRef.current && !isLocalWorkspaceUpdate; + const workspaceInstanceKey = isExternalWorkspaceUpdate ? song : workspaceInstanceRef.current; useEffect(() => { if (song !== previousSongRef.current) { @@ -337,7 +339,7 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
From 28038721b473c1f64b6914312f48704d64e09c6a Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 06:26:19 +0900 Subject: [PATCH 18/23] fix(accelerando): align plan newline validation --- apps/desktop/src/features/workspace/firstAccelerando.test.ts | 5 +++++ apps/desktop/src/features/workspace/firstAccelerando.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts index 81804c342..befd0e025 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.test.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -143,6 +143,11 @@ describe("resolveFirstAccelerandoPlan", () => { withAccelerandoSection({ accelerandoPlan: "Ease together.\nHold the count." }) ) ).toBeNull(); + expect( + resolveFirstAccelerandoPlan( + withAccelerandoSection({ accelerandoPlan: `${DEMO_ACCELERANDO_PLAN}\n` }) + ) + ).toBeNull(); expect( resolveFirstAccelerandoPlan(withAccelerandoSection({ accelerandoPlan: "slow down here" })) ).toBeNull(); diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts index 67fb8ede7..183b766b8 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -191,7 +191,7 @@ function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { return null; } const trimmed = accelerandoPlan.trim(); - if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { + if (trimmed.length === 0 || accelerandoPlan.includes("\n") || accelerandoPlan.includes("\r")) { return null; } if (accelerandoPlanSource === "model") { From 90feef25e17745f705680cf9450d038fe6a9f96d Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 13:33:28 +0900 Subject: [PATCH 19/23] fix(contract): align accelerando plan validation --- apps/desktop/core/src/lib.rs | 43 +++++++++++++--- .../core/tests/accelerando_plan_contract.rs | 36 ++++++++++++- .../workspace/firstAccelerando.test.ts | 7 +-- .../features/workspace/firstAccelerando.ts | 22 ++------ packages/shared-types/src/index.ts | 47 +++++++++++++++-- .../test/accelerandoPlanProvenance.test.ts | 50 +++++++++++++++---- 6 files changed, 163 insertions(+), 42 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index f82dab23f..849d4bdc5 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -574,6 +574,41 @@ pub fn is_youtube_video_id(value: &str) -> bool { .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'-') } +fn is_plan_whitespace(value: char) -> bool { + matches!( + value, + '\u{0009}'..='\u{000D}' + | '\u{0020}' + | '\u{0085}' + | '\u{00A0}' + | '\u{1680}' + | '\u{2000}'..='\u{200A}' + | '\u{2028}' + | '\u{2029}' + | '\u{202F}' + | '\u{205F}' + | '\u{3000}' + | '\u{FEFF}' + ) +} + +/// Mirrors shared-types plan validation without normalizing persisted text. +fn is_valid_accelerando_plan(value: &str) -> bool { + let mut has_non_whitespace = false; + for character in value.chars() { + if matches!( + character, + '\n' | '\r' | '\u{0085}' | '\u{2028}' | '\u{2029}' + ) { + return false; + } + if !is_plan_whitespace(character) { + has_non_whitespace = true; + } + } + has_non_whitespace +} + fn validate_accelerando_plan_provenance( payload: RehearsalSongPayload, ) -> Result { @@ -581,12 +616,8 @@ fn validate_accelerando_plan_provenance( for role in §ion.roles { if role .accelerando_plan - .as_ref() - .is_some_and(|accelerando_plan| { - accelerando_plan.trim().is_empty() - || accelerando_plan.contains('\n') - || accelerando_plan.contains('\r') - }) + .as_deref() + .is_some_and(|accelerando_plan| !is_valid_accelerando_plan(accelerando_plan)) { return Err("Invalid project file format".to_string()); } diff --git a/apps/desktop/core/tests/accelerando_plan_contract.rs b/apps/desktop/core/tests/accelerando_plan_contract.rs index 6f4f2e164..7532ded7e 100644 --- a/apps/desktop/core/tests/accelerando_plan_contract.rs +++ b/apps/desktop/core/tests/accelerando_plan_contract.rs @@ -127,7 +127,30 @@ fn project_contract_rejects_accelerando_plan_without_source() { #[test] fn project_contract_rejects_invalid_accelerando_plan_copy_with_source() { - for accelerando_plan in ["", " ", "push here\nthen hold", "push here\rthen hold"] { + for accelerando_plan in [ + "", + " ", + "\u{0009}", + "\u{000B}", + "\u{000C}", + "\u{000D}", + "\u{0085}", + "\u{00A0}", + "\u{1680}", + "\u{2000}", + "\u{200A}", + "\u{2028}", + "\u{2029}", + "\u{202F}", + "\u{205F}", + "\u{3000}", + "\u{FEFF}", + "push here\nthen hold", + "push here\rthen hold", + "push here\u{0085}then hold", + "push here\u{2028}then hold", + "push here\u{2029}then hold", + ] { let mut payload = song_with_accelerando_plan(); payload["sections"][0]["roles"][0]["accelerandoPlan"] = json!(accelerando_plan); let content = serde_json::to_string(&payload).expect("fixture should serialize"); @@ -139,6 +162,17 @@ fn project_contract_rejects_invalid_accelerando_plan_copy_with_source() { } } +#[test] +fn project_contract_accepts_unicode_padded_single_line_accelerando_plan() { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0]["accelerandoPlan"] = json!( + "\u{FEFF} Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner.\u{3000}" + ); + let content = serde_json::to_string(&payload).expect("payload should serialize"); + + assert!(project_payload_from_content(&content).is_ok()); +} + #[test] fn project_contract_rejects_unknown_accelerando_plan_source() { let mut payload = song_with_accelerando_plan(); diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts index befd0e025..bfa634dd7 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.test.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -175,14 +175,15 @@ describe("resolveFirstAccelerandoPlan", () => { ); }); - it("admits bounded user copy without requiring the engine template", () => { + it("preserves long user-authored copy without requiring the engine template", () => { + const accelerandoPlan = `Push the phrase early ${"A".repeat(170)} into the downbeat.`; const resolved = resolveFirstAccelerandoPlan( withAccelerandoSection({ - accelerandoPlan: "Push the phrase early into the downbeat.", + accelerandoPlan, source: "user" }) ); - expect(resolved?.accelerandoPlan).toBe("Push the phrase early into the downbeat."); + expect(resolved?.accelerandoPlan).toBe(accelerandoPlan); expect(resolved?.accelerandoPlanSource).toBe("user"); }); diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts index 183b766b8..51d1ff984 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -1,13 +1,13 @@ import { MAX_SECTION_TIME_SECONDS, SECTION_FORM_LABELS, + isNonEmptySingleLineText, type RehearsalRole, type RehearsalSection, type RehearsalSong } from "@bandscope/shared-types"; const PRIORITY_RANK = { high: 0, medium: 1, low: 2 } as const; -const MAX_ACCELERANDO_PLAN_CHARACTERS = 180; const SECTION_FORM_LABEL_SET = new Set(SECTION_FORM_LABELS); const NAMED_ACCELERANDO_ROLE_IDS = new Set(["bass-guitar", "lead-vocal"]); const ACCELERANDO_PLAN_PREFIX = "Push this part from "; @@ -122,20 +122,6 @@ function ownedDenseRuntimeArray(value: unknown): unknown[] | null { return items; } -/** 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); -} - /** Preserve the engine accelerando template while enforcing the engine's speeding semantics. */ function boundedGeneratedAccelerandoPlan(value: string): OwnedAccelerandoPlan | null { if ( @@ -190,15 +176,15 @@ function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { if (accelerandoPlanSource !== "model" && accelerandoPlanSource !== "user") { return null; } - const trimmed = accelerandoPlan.trim(); - if (trimmed.length === 0 || accelerandoPlan.includes("\n") || accelerandoPlan.includes("\r")) { + if (!isNonEmptySingleLineText(accelerandoPlan)) { return null; } if (accelerandoPlanSource === "model") { + const trimmed = accelerandoPlan.trim(); return boundedGeneratedAccelerandoPlan(trimmed); } return { - text: truncateCodePoints(trimmed, MAX_ACCELERANDO_PLAN_CHARACTERS), + text: accelerandoPlan, source: accelerandoPlanSource, guidance: null }; diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index f45c618cc..a416a6900 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -409,6 +409,48 @@ function isOneOf(options: readonly T[], value: unknown): value return typeof value === "string" && options.includes(value as T); } +/** Return whether a code point belongs to the cross-language plan whitespace set. */ +function isPlanWhitespaceCodePoint(codePoint: number): boolean { + return ( + (codePoint >= 0x0009 && codePoint <= 0x000d) || + codePoint === 0x0020 || + codePoint === 0x0085 || + codePoint === 0x00a0 || + codePoint === 0x1680 || + (codePoint >= 0x2000 && codePoint <= 0x200a) || + codePoint === 0x2028 || + codePoint === 0x2029 || + codePoint === 0x202f || + codePoint === 0x205f || + codePoint === 0x3000 || + codePoint === 0xfeff + ); +} + +/** Return whether a plan is non-empty and contains no Unicode line separator. */ +export function isNonEmptySingleLineText(value: unknown): value is string { + if (typeof value !== "string") { + return false; + } + let hasNonWhitespace = false; + for (const character of value) { + const codePoint = character.codePointAt(0)!; + if ( + codePoint === 0x000a || + codePoint === 0x000d || + codePoint === 0x0085 || + codePoint === 0x2028 || + codePoint === 0x2029 + ) { + return false; + } + if (!isPlanWhitespaceCodePoint(codePoint)) { + hasNonWhitespace = true; + } + } + return hasNonWhitespace; +} + /** Documented. */ function invalidField(path: string): string { return `Invalid rehearsal song contract: invalid field '${path}'`; @@ -1594,10 +1636,7 @@ function validateRehearsalRole(value: unknown, path: string): string | null { if ( value.accelerandoPlan !== undefined && - (typeof value.accelerandoPlan !== "string" || - value.accelerandoPlan.trim().length === 0 || - value.accelerandoPlan.includes("\n") || - value.accelerandoPlan.includes("\r")) + !isNonEmptySingleLineText(value.accelerandoPlan) ) { return invalidField(`${path}.accelerandoPlan`); } diff --git a/packages/shared-types/test/accelerandoPlanProvenance.test.ts b/packages/shared-types/test/accelerandoPlanProvenance.test.ts index 9c350b0fc..c284db0e2 100644 --- a/packages/shared-types/test/accelerandoPlanProvenance.test.ts +++ b/packages/shared-types/test/accelerandoPlanProvenance.test.ts @@ -37,14 +37,44 @@ describe("accelerandoPlan provenance", () => { expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanSource/); }); - it.each(["", " ", "push here\nthen hold", "push here\rthen hold"])( - "rejects an accelerando plan source with blank or multiline copy %j", - (accelerandoPlan) => { - const song = createDemoRehearsalSong(); - const role = song.sections[0]!.roles[0]!; - role.accelerandoPlan = accelerandoPlan; - role.accelerandoPlanSource = "model"; - expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlan/); - } - ); + it.each([ + "", + " ", + "\u0009", + "\u000B", + "\u000C", + "\u000D", + "\u0085", + "\u00A0", + "\u1680", + "\u2000", + "\u200A", + "\u2028", + "\u2029", + "\u202F", + "\u205F", + "\u3000", + "\uFEFF", + "push here\nthen hold", + "push here\rthen hold", + "push here\u0085then hold", + "push here\u2028then hold", + "push here\u2029then hold" + ])("rejects invalid accelerando-plan copy %j", (accelerandoPlan) => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = accelerandoPlan; + role.accelerandoPlanSource = "model"; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlan/); + }); + + it("accepts Unicode-padded single-line accelerando-plan copy", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = `\uFEFF ${DEMO_ACCELERANDO_PLAN} \u3000`; + role.accelerandoPlanSource = "model"; + expect(parseRehearsalSong(song).sections[0]!.roles[0]!.accelerandoPlan).toContain( + "Push this part" + ); + }); }); From e202f5d64ea7b368425982b8a2dfa6765b64c371 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 13:57:53 +0900 Subject: [PATCH 20/23] fix(cache): reuse temporal analysis features --- .../src/bandscope_analysis/api.py | 62 ++++++++++- .../src/bandscope_analysis/cli.py | 43 +++++--- services/analysis-engine/tests/test_api.py | 53 +++++++++ services/analysis-engine/tests/test_cli.py | 103 ++++++++++++++++++ 4 files changed, 246 insertions(+), 15 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index e2853ebc7..6996b6bbf 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -199,6 +199,7 @@ class CachedFeaturePayload(TypedDict): separation: dict[str, object] stemKeys: list[str] stemRoleTypes: dict[str, str] + temporalFeatures: NotRequired[dict[str, object]] class StemSeparationTimedOut(RuntimeError): @@ -540,6 +541,40 @@ def _coerce_beat_times(audio_features: dict[str, Any] | None) -> list[float] | N return times +def _coerce_cached_temporal_features(value: object) -> dict[str, object] | None: + """Return JSON-safe temporal features for the reusable feature cache.""" + if not isinstance(value, dict): + return None + bpm = value.get("bpm") + duration_seconds = value.get("duration_seconds") + sample_rate = value.get("sample_rate") + if ( + isinstance(bpm, bool) + or not isinstance(bpm, (int, float)) + or not np.isfinite(bpm) + or bpm <= 0 + or isinstance(duration_seconds, bool) + or not isinstance(duration_seconds, (int, float)) + or not np.isfinite(duration_seconds) + or duration_seconds < 0 + or isinstance(sample_rate, bool) + or not isinstance(sample_rate, int) + or sample_rate <= 0 + ): + return None + beat_times = _coerce_beat_times({"beat_times": value.get("beat_times")}) + downbeat_times = _coerce_beat_times({"beat_times": value.get("downbeat_times")}) + if beat_times is None or downbeat_times is None: + return None + return { + "bpm": float(bpm), + "beat_times": beat_times, + "downbeat_times": downbeat_times, + "duration_seconds": float(duration_seconds), + "sample_rate": sample_rate, + } + + def _apply_accelerando( song: RehearsalSong, mix: Any, @@ -778,6 +813,11 @@ def _load_cached_local_audio_features( stem_role_types = _normalize_stem_role_types(metadata_payload.get("stemRoleTypes"), stem_keys) if stem_role_types is None: return None + temporal_features = None + if "temporalFeatures" in metadata_payload: + temporal_features = _coerce_cached_temporal_features(metadata_payload["temporalFeatures"]) + if temporal_features is None: + return None try: with np.load(arrays_path, allow_pickle=False) as stems_archive: @@ -793,7 +833,7 @@ def _load_cached_local_audio_features( except (OSError, ValueError): return None - return { + loaded = { "stems": stems, "sr": metadata_payload["sampleRate"], "stem_role_types": stem_role_types, @@ -803,6 +843,23 @@ def _load_cached_local_audio_features( "notes": separation.get("notes"), }, } + if temporal_features is not None: + loaded.update(temporal_features) + return loaded + + +def _load_cached_temporal_features(metadata_path: Path) -> dict[str, object] | None: + """Load only cached temporal metadata so the CLI can skip audio decoding.""" + try: + with metadata_path.open("r", encoding="utf-8") as metadata_file: + metadata_payload = json.load(metadata_file) + except (OSError, json.JSONDecodeError): + return None + if not isinstance(metadata_payload, dict): + return None + if metadata_payload.get("schemaVersion") != FEATURE_CACHE_SCHEMA_VERSION: + return None + return _coerce_cached_temporal_features(metadata_payload.get("temporalFeatures")) def _serialize_stem_arrays(stems: object) -> dict[str, np.ndarray] | None: @@ -863,6 +920,9 @@ def _store_cached_local_audio_features( "stemKeys": stem_keys, "stemRoleTypes": stem_role_types, } + temporal_features = _coerce_cached_temporal_features(audio_features) + if temporal_features is not None: + metadata_payload["temporalFeatures"] = temporal_features try: metadata_path.parent.mkdir(parents=True, exist_ok=True) metadata_temp = metadata_path.with_name(f"{metadata_path.name}.tmp") diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 4ff22fbf1..74cbd088b 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -9,6 +9,10 @@ from typing import Any, cast from bandscope_analysis.api import ( + _analysis_cache_path, + _feature_cache_paths, + _load_cached_analysis, + _load_cached_temporal_features, get_analysis_status, run_analysis_job, run_analysis_job_updates, @@ -94,20 +98,31 @@ def main() -> int: and validated_request["sourceKind"] == "local_audio" and "localSource" in validated_request ): - local_source = validated_request["localSource"] - audio_path = local_source["sourcePath"] - file_name = local_source["fileName"] - logging.info("Extracting temporal features from %s...", file_name) - try: - temporal_analyzer = TemporalAnalyzer() - features = temporal_analyzer.analyze(audio_path) - temporal_features = cast(dict[str, Any], features) - logging.info(f"Extracted BPM: {features['bpm']}") - except Exception: - logging.warning( - "Temporal analysis failed for %s; continuing with safe fallback.", - file_name, - ) + cache_path = _analysis_cache_path(validated_request) + cached_result = _load_cached_analysis(cache_path) if cache_path is not None else None + feature_paths = _feature_cache_paths(validated_request) + cached_temporal = ( + _load_cached_temporal_features(feature_paths[0]) + if cached_result is None and feature_paths is not None + else None + ) + if cached_result is None and cached_temporal is not None: + temporal_features = cast(dict[str, Any], cached_temporal) + elif cached_result is None: + local_source = validated_request["localSource"] + audio_path = local_source["sourcePath"] + file_name = local_source["fileName"] + logging.info("Extracting temporal features from %s...", file_name) + try: + temporal_analyzer = TemporalAnalyzer() + features = temporal_analyzer.analyze(audio_path) + temporal_features = cast(dict[str, Any], features) + logging.info(f"Extracted BPM: {features['bpm']}") + except Exception: + logging.warning( + "Temporal analysis failed for %s; continuing with safe fallback.", + file_name, + ) requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") if progress_jsonl: diff --git a/services/analysis-engine/tests/test_api.py b/services/analysis-engine/tests/test_api.py index 93f3ee962..58ce8a2d1 100644 --- a/services/analysis-engine/tests/test_api.py +++ b/services/analysis-engine/tests/test_api.py @@ -1,5 +1,6 @@ """Tests for the public analysis-engine API helpers.""" +import json import queue import time from unittest.mock import patch @@ -8,9 +9,11 @@ from bandscope_analysis.api import ( _build_local_audio_features, + _coerce_cached_temporal_features, _feature_cache_paths, _load_cached_analysis, _load_cached_local_audio_features, + _load_cached_temporal_features, _run_stem_separation_with_timeout, _stem_separation_worker, _stem_work_arrays_path, @@ -730,6 +733,11 @@ def test_local_feature_cache_round_trip_uses_disk_cache_before_recompute(tmp_pat "chunk_count": 1, "notes": "Separated selected local audio into 4 canonical stems.", }, + "bpm": 120.0, + "beat_times": [0.0, 0.5, 1.0], + "downbeat_times": [0.0], + "duration_seconds": 1.0, + "sample_rate": 22050, } assert _store_cached_local_audio_features(metadata_path, arrays_path, request, features) is True @@ -743,6 +751,16 @@ def test_local_feature_cache_round_trip_uses_disk_cache_before_recompute(tmp_pat "drums": "instrument", "other": "instrument", } + assert loaded["bpm"] == 120.0 + assert loaded["beat_times"] == [0.0, 0.5, 1.0] + assert loaded["downbeat_times"] == [0.0] + assert _load_cached_temporal_features(metadata_path) == { + "bpm": 120.0, + "beat_times": [0.0, 0.5, 1.0], + "downbeat_times": [0.0], + "duration_seconds": 1.0, + "sample_rate": 22050, + } with ( patch( @@ -878,6 +896,41 @@ def __getitem__(self, _key: str) -> object: assert _load_cached_local_audio_features(metadata_path, arrays_path) is None +def test_cached_temporal_features_reject_malformed_payloads(tmp_path) -> None: + """Ensure temporal cache metadata cannot inject malformed analysis values.""" + assert _coerce_cached_temporal_features(None) is None + assert _coerce_cached_temporal_features( + { + "bpm": 120.0, + "beat_times": "not-a-list", + "downbeat_times": [0.0], + "duration_seconds": 1.0, + "sample_rate": 22050, + } + ) is None + + metadata_path = tmp_path / "features.json" + metadata_path.write_text("[]", encoding="utf-8") + assert _load_cached_temporal_features(metadata_path) is None + metadata_path.write_text('{"schemaVersion": 999}', encoding="utf-8") + assert _load_cached_temporal_features(metadata_path) is None + + metadata_path.write_text( + json.dumps( + { + "schemaVersion": 1, + "sampleRate": 22050, + "separation": {}, + "stemKeys": ["bass"], + "stemRoleTypes": {"bass": "instrument"}, + "temporalFeatures": {}, + } + ), + encoding="utf-8", + ) + assert _load_cached_local_audio_features(metadata_path, tmp_path / "features.npz") is None + + def test_local_feature_cache_store_rejects_invalid_payloads(tmp_path) -> None: """Ensure feature cache writes require app-owned request metadata and arrays.""" request = validate_analysis_job_request( diff --git a/services/analysis-engine/tests/test_cli.py b/services/analysis-engine/tests/test_cli.py index 5963646bc..cb3d9baec 100644 --- a/services/analysis-engine/tests/test_cli.py +++ b/services/analysis-engine/tests/test_cli.py @@ -444,6 +444,109 @@ def analyze(self, path): assert res["result"]["tempo"] == 120 +def test_cli_main_skips_temporal_analysis_when_cached_result_exists( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """Ensure a complete cached result avoids decoding and beat tracking again.""" + audio_path = tmp_path / "test.wav" + write_short_wav(audio_path) + request = { + "sourceKind": "local_audio", + "projectId": "p1", + "sourceLabel": "test.wav", + "roleFocus": [], + "localSource": { + "sourcePath": str(audio_path), + "fileName": "test.wav", + "extension": "wav", + "fileSizeBytes": audio_path.stat().st_size, + }, + "cacheRoot": str(tmp_path / "cache"), + } + stdin = io.StringIO(json.dumps({"jobId": "job-cached", "request": request})) + stdout = io.StringIO() + + class ExplodingAnalyzer: + def analyze(self, _path): + raise AssertionError("cached local audio reached temporal analysis") + + monkeypatch.setattr(cli, "TemporalAnalyzer", ExplodingAnalyzer) + monkeypatch.setattr(cli, "_load_cached_analysis", lambda _path: {"cached": True}) + monkeypatch.setattr( + cli, + "run_analysis_job", + lambda *args: { + "jobId": args[0], + "state": "succeeded", + "result": {"cached": True}, + }, + ) + monkeypatch.setattr(cli.sys, "stdin", stdin) + monkeypatch.setattr(cli.sys, "stdout", stdout) + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["result"] == {"cached": True} + + +def test_cli_main_reuses_cached_temporal_features( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """Ensure cached temporal metadata avoids a second audio decode.""" + audio_path = tmp_path / "test.wav" + write_short_wav(audio_path) + request = { + "sourceKind": "local_audio", + "projectId": "p1", + "sourceLabel": "test.wav", + "roleFocus": [], + "localSource": { + "sourcePath": str(audio_path), + "fileName": "test.wav", + "extension": "wav", + "fileSizeBytes": audio_path.stat().st_size, + }, + "cacheRoot": str(tmp_path / "cache"), + } + stdin = io.StringIO(json.dumps({"jobId": "job-feature-cache", "request": request})) + stdout = io.StringIO() + + class ExplodingAnalyzer: + def analyze(self, _path): + raise AssertionError("cached temporal features reached audio analysis") + + monkeypatch.setattr(cli, "TemporalAnalyzer", ExplodingAnalyzer) + monkeypatch.setattr(cli, "_load_cached_analysis", lambda _path: None) + monkeypatch.setattr( + cli, + "_load_cached_temporal_features", + lambda _path: { + "bpm": 120.0, + "beat_times": [0.0, 0.5], + "downbeat_times": [0.0], + "duration_seconds": 1.0, + "sample_rate": 22050, + }, + ) + monkeypatch.setattr( + cli, + "run_analysis_job", + lambda *args: { + "jobId": args[0], + "state": "succeeded", + "result": {"cachedTemporal": True}, + }, + ) + monkeypatch.setattr(cli.sys, "stdin", stdin) + monkeypatch.setattr(cli.sys, "stdout", stdout) + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["result"] == {"cachedTemporal": True} + + def test_cli_main_progress_jsonl_streams_status_updates( monkeypatch: pytest.MonkeyPatch, tmp_path, From e64bd97bd67a5dc7d1a891b2aaecb7481ce4cab7 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 14:01:54 +0900 Subject: [PATCH 21/23] style: format cache regression test --- services/analysis-engine/tests/test_api.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/services/analysis-engine/tests/test_api.py b/services/analysis-engine/tests/test_api.py index 58ce8a2d1..f146830a8 100644 --- a/services/analysis-engine/tests/test_api.py +++ b/services/analysis-engine/tests/test_api.py @@ -899,15 +899,18 @@ def __getitem__(self, _key: str) -> object: def test_cached_temporal_features_reject_malformed_payloads(tmp_path) -> None: """Ensure temporal cache metadata cannot inject malformed analysis values.""" assert _coerce_cached_temporal_features(None) is None - assert _coerce_cached_temporal_features( - { - "bpm": 120.0, - "beat_times": "not-a-list", - "downbeat_times": [0.0], - "duration_seconds": 1.0, - "sample_rate": 22050, - } - ) is None + assert ( + _coerce_cached_temporal_features( + { + "bpm": 120.0, + "beat_times": "not-a-list", + "downbeat_times": [0.0], + "duration_seconds": 1.0, + "sample_rate": 22050, + } + ) + is None + ) metadata_path = tmp_path / "features.json" metadata_path.write_text("[]", encoding="utf-8") From 2d13684af863cc6620056fa812083fe61eada532 Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 14:43:18 +0900 Subject: [PATCH 22/23] fix(accelerando): preserve detected change timing --- apps/desktop/core/src/lib.rs | 14 ++++++ .../core/tests/accelerando_plan_contract.rs | 31 +++++++++++- .../workspace/firstAccelerando.test.ts | 18 +++++++ .../features/workspace/firstAccelerando.ts | 27 +++++++++-- packages/shared-types/src/index.ts | 19 +++++++- .../test/accelerandoPlanProvenance.test.ts | 25 ++++++++++ .../src/bandscope_analysis/api.py | 6 ++- .../src/bandscope_analysis/roles/model.py | 1 + .../temporal/accelerando.py | 47 +++++++++++++++++-- .../tests/test_accelerando_plan.py | 23 +++++++++ 10 files changed, 198 insertions(+), 13 deletions(-) diff --git a/apps/desktop/core/src/lib.rs b/apps/desktop/core/src/lib.rs index 849d4bdc5..23e73a18c 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -238,6 +238,8 @@ pub struct RehearsalRolePayload { accelerando_plan: Option, #[serde(default, skip_serializing_if = "Option::is_none")] accelerando_plan_source: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + accelerando_plan_at_seconds: Option, } #[derive(Clone, Debug, Serialize)] @@ -609,6 +611,8 @@ fn is_valid_accelerando_plan(value: &str) -> bool { has_non_whitespace } +const MAX_SECTION_TIME_SECONDS: f64 = 4_294_967_295.0; + fn validate_accelerando_plan_provenance( payload: RehearsalSongPayload, ) -> Result { @@ -627,6 +631,16 @@ fn validate_accelerando_plan_provenance( if role.accelerando_plan.is_some() && role.accelerando_plan_source.is_none() { return Err("Invalid project file format".to_string()); } + if role.accelerando_plan_at_seconds.is_some_and(|time| { + !time.is_finite() || time < 0.0 || time > MAX_SECTION_TIME_SECONDS + }) { + return Err("Invalid project file format".to_string()); + } + if role.accelerando_plan_at_seconds.is_some() + && (role.accelerando_plan.is_none() || role.accelerando_plan_source.is_none()) + { + return Err("Invalid project file format".to_string()); + } } } Ok(payload) diff --git a/apps/desktop/core/tests/accelerando_plan_contract.rs b/apps/desktop/core/tests/accelerando_plan_contract.rs index 7532ded7e..e66304680 100644 --- a/apps/desktop/core/tests/accelerando_plan_contract.rs +++ b/apps/desktop/core/tests/accelerando_plan_contract.rs @@ -54,7 +54,8 @@ fn song_with_accelerando_plan() -> Value { }], "practiceProgress": 50, "accelerandoPlan": "Push this part from 80 BPM into 120 BPM; let the next downbeat arrive sooner.", - "accelerandoPlanSource": "model" + "accelerandoPlanSource": "model", + "accelerandoPlanAtSeconds": 12.375 } ], "partGraph": [ @@ -93,6 +94,10 @@ fn project_contract_round_trips_accelerando_plan_provenance() { serialized["sections"][0]["roles"][0]["accelerandoPlanSource"], json!("model") ); + assert_eq!( + serialized["sections"][0]["roles"][0]["accelerandoPlanAtSeconds"], + json!(12.375) + ); } #[test] @@ -185,6 +190,30 @@ fn project_contract_rejects_unknown_accelerando_plan_source() { ); } +#[test] +fn project_contract_rejects_invalid_accelerando_plan_timing() { + for time in [-1.0, 4_294_967_296.0] { + let mut payload = song_with_accelerando_plan(); + payload["sections"][0]["roles"][0]["accelerandoPlanAtSeconds"] = json!(time); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!(project_payload_from_content(&content).is_err()); + } +} + +#[test] +fn project_contract_rejects_accelerando_plan_timing_without_copy() { + let mut payload = song_with_accelerando_plan(); + let role = payload["sections"][0]["roles"][0] + .as_object_mut() + .expect("role fixture should be an object"); + role.remove("accelerandoPlan"); + role.remove("accelerandoPlanSource"); + let content = serde_json::to_string(&payload).expect("fixture should serialize"); + + assert!(project_payload_from_content(&content).is_err()); +} + #[test] fn project_contract_rejects_practice_progress_above_shared_bound() { let mut payload = song_with_accelerando_plan(); diff --git a/apps/desktop/src/features/workspace/firstAccelerando.test.ts b/apps/desktop/src/features/workspace/firstAccelerando.test.ts index bfa634dd7..63b7c0306 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.test.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -11,6 +11,7 @@ function withAccelerandoSection( start?: number; end?: number; accelerandoPlan?: string; + accelerandoPlanAtSeconds?: number; source?: "model" | "user"; label?: "intro" | "verse" | "chorus" | "bridge" | "outro"; roleId?: string; @@ -40,6 +41,9 @@ function withAccelerandoSection( roleType: overrides.roleType ?? (roleId === "lead-vocal" ? "vocal" : "instrument"), rehearsalPriority: overrides.priority ?? "high", accelerandoPlan: overrides.accelerandoPlan ?? DEMO_ACCELERANDO_PLAN, + ...(overrides.accelerandoPlanAtSeconds !== undefined + ? { accelerandoPlanAtSeconds: overrides.accelerandoPlanAtSeconds } + : {}), ...(overrides.source ? { accelerandoPlanSource: overrides.source } : { accelerandoPlanSource: "model" as const }) }; const current = structuredClone(verse); @@ -81,6 +85,13 @@ describe("resolveFirstAccelerandoPlan", () => { expect(formatAccelerandoPlanTime(-4)).toBe("0:00"); }); + it("uses the detected tempo-change time instead of the section start", () => { + const resolved = resolveFirstAccelerandoPlan( + withAccelerandoSection({ start: 10, accelerandoPlanAtSeconds: 12.375 }) + ); + expect(resolved?.atSeconds).toBe(12.375); + }); + it("does not invent an accelardando plan from groove, cue, simplification, overlap, range, chords, function labels, setup notes, transposition plans, or confidence notes", () => { const song = withAccelerandoSection(); delete song.sections[0]!.roles.find((role) => role.id === "lead-vocal")!.accelerandoPlan; @@ -153,6 +164,13 @@ describe("resolveFirstAccelerandoPlan", () => { ).toBeNull(); }); + it.each([Number.NaN, -1, 4_294_967_296])( + "rejects malformed accelerando-plan timing %s", + (accelerandoPlanAtSeconds) => { + expect(resolveFirstAccelerandoPlan(withAccelerandoSection({ accelerandoPlanAtSeconds }))).toBeNull(); + } + ); + it("rejects model copy that is not a genuine non-double-time speeding", () => { expect( resolveFirstAccelerandoPlan( diff --git a/apps/desktop/src/features/workspace/firstAccelerando.ts b/apps/desktop/src/features/workspace/firstAccelerando.ts index 51d1ff984..3adb8a9e5 100644 --- a/apps/desktop/src/features/workspace/firstAccelerando.ts +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -37,6 +37,7 @@ type OwnedAccelerandoPlan = Readonly<{ text: string; source: AccelerandoPlanSource; guidance: AccelerandoPlanGuidance | null; + atSeconds: number | null; }>; /** Tonight's first accelerando plan: the earliest corroborated speeding on a named vocal or bass. */ @@ -159,7 +160,8 @@ function boundedGeneratedAccelerandoPlan(value: string): OwnedAccelerandoPlan | return { text: `${ACCELERANDO_PLAN_PREFIX}${fromBpm}${ACCELERANDO_PLAN_MIDDLE}${toBpm}${ACCELERANDO_PLAN_SUFFIX}`, source: "model", - guidance: { kind: "tempo", fromBpm, toBpm } + guidance: { kind: "tempo", fromBpm, toBpm }, + atSeconds: null }; } @@ -170,6 +172,7 @@ function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { } const accelerandoPlan = ownDataValue(role, "accelerandoPlan"); const accelerandoPlanSource = ownDataValue(role, "accelerandoPlanSource"); + const accelerandoPlanAtSeconds = ownDataValue(role, "accelerandoPlanAtSeconds"); if (typeof accelerandoPlan !== "string") { return null; } @@ -179,14 +182,27 @@ function ownedAccelerandoPlan(role: unknown): OwnedAccelerandoPlan | null { if (!isNonEmptySingleLineText(accelerandoPlan)) { return null; } + if ( + accelerandoPlanAtSeconds !== undefined && + (typeof accelerandoPlanAtSeconds !== "number" || + !Number.isFinite(accelerandoPlanAtSeconds) || + accelerandoPlanAtSeconds < 0 || + accelerandoPlanAtSeconds > MAX_SECTION_TIME_SECONDS) + ) { + return null; + } if (accelerandoPlanSource === "model") { const trimmed = accelerandoPlan.trim(); - return boundedGeneratedAccelerandoPlan(trimmed); + const bounded = boundedGeneratedAccelerandoPlan(trimmed); + return bounded === null + ? null + : { ...bounded, atSeconds: (accelerandoPlanAtSeconds as number | undefined) ?? null }; } return { text: accelerandoPlan, source: accelerandoPlanSource, - guidance: null + guidance: null, + atSeconds: (accelerandoPlanAtSeconds as number | undefined) ?? null }; } @@ -374,7 +390,8 @@ function resolveSafeFirstAccelerandoPlan(song: RehearsalSong): FirstAccelerandoP ...metadata, accelerandoPlan: accelerandoPlan.text, accelerandoPlanSource: accelerandoPlan.source, - accelerandoPlanGuidance: accelerandoPlan.guidance + accelerandoPlanGuidance: accelerandoPlan.guidance, + atSeconds: accelerandoPlan.atSeconds } ]; }) @@ -394,7 +411,7 @@ function resolveSafeFirstAccelerandoPlan(song: RehearsalSong): FirstAccelerandoP accelerandoPlan: landingRole.accelerandoPlan, accelerandoPlanSource: landingRole.accelerandoPlanSource, accelerandoPlanGuidance: landingRole.accelerandoPlanGuidance, - atSeconds: timeRange.start + atSeconds: landingRole.atSeconds ?? timeRange.start } ]; }) diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index a416a6900..fddebe28a 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -145,6 +145,7 @@ export type RehearsalRole = { practiceProgress?: number; accelerandoPlan?: string; accelerandoPlanSource?: ProvenanceSource; + accelerandoPlanAtSeconds?: number; }; /** Documented. */ @@ -1546,7 +1547,8 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "transcription", "practiceProgress", "accelerandoPlan", - "accelerandoPlanSource" + "accelerandoPlanSource", + "accelerandoPlanAtSeconds" ], path ); @@ -1652,6 +1654,21 @@ function validateRehearsalRole(value: unknown, path: string): string | null { if (value.accelerandoPlan !== undefined && value.accelerandoPlanSource === undefined) { return invalidField(`${path}.accelerandoPlanSource`); } + if ( + value.accelerandoPlanAtSeconds !== undefined && + (typeof value.accelerandoPlanAtSeconds !== "number" || + !Number.isFinite(value.accelerandoPlanAtSeconds) || + value.accelerandoPlanAtSeconds < 0 || + value.accelerandoPlanAtSeconds > MAX_SECTION_TIME_SECONDS) + ) { + return invalidField(`${path}.accelerandoPlanAtSeconds`); + } + if ( + value.accelerandoPlanAtSeconds !== undefined && + (value.accelerandoPlan === undefined || value.accelerandoPlanSource === undefined) + ) { + return invalidField(`${path}.accelerandoPlanAtSeconds`); + } return null; } diff --git a/packages/shared-types/test/accelerandoPlanProvenance.test.ts b/packages/shared-types/test/accelerandoPlanProvenance.test.ts index c284db0e2..43c784d42 100644 --- a/packages/shared-types/test/accelerandoPlanProvenance.test.ts +++ b/packages/shared-types/test/accelerandoPlanProvenance.test.ts @@ -13,6 +13,15 @@ describe("accelerandoPlan provenance", () => { expect(parseRehearsalSong(song).sections[0]!.roles[0]!.accelerandoPlanSource).toBe(source); }); + it("round-trips the precise tempo-change time", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + role.accelerandoPlanSource = "model"; + role.accelerandoPlanAtSeconds = 12.375; + expect(parseRehearsalSong(song).sections[0]!.roles[0]!.accelerandoPlanAtSeconds).toBe(12.375); + }); + it("rejects an unknown accelerando plan source", () => { const song = createDemoRehearsalSong(); const role = song.sections[0]!.roles[0]!; @@ -37,6 +46,22 @@ describe("accelerandoPlan provenance", () => { expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanSource/); }); + it.each([Number.NaN, -1, 4_294_967_296])("rejects invalid accelerando-plan timing %s", (time) => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlan = DEMO_ACCELERANDO_PLAN; + role.accelerandoPlanSource = "model"; + role.accelerandoPlanAtSeconds = time; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanAtSeconds/); + }); + + it("rejects accelerando-plan timing without its plan copy", () => { + const song = createDemoRehearsalSong(); + const role = song.sections[0]!.roles[0]!; + role.accelerandoPlanAtSeconds = 12.375; + expect(() => parseRehearsalSong(song)).toThrow(/accelerandoPlanAtSeconds/); + }); + it.each([ "", " ", diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index 6996b6bbf..6fc84ae3a 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -119,6 +119,7 @@ class RehearsalRolePayload(TypedDict): overlapWarnings: list[str] accelerandoPlan: NotRequired[str] accelerandoPlanSource: NotRequired[Literal["model", "user"]] + accelerandoPlanAtSeconds: NotRequired[float] class PartGraphNodePayload(TypedDict): @@ -460,7 +461,7 @@ def _build_from_pipeline( }, } _apply_tempo(song, features) - _apply_accelerando(song, mix, sr, features) + _apply_accelerando(song, mix, sr, features, boundaries) return song @@ -580,12 +581,13 @@ def _apply_accelerando( mix: Any, sr: int, audio_features: dict[str, Any] | None, + section_boundaries: list[tuple[float, float]] | None = None, ) -> None: """Stamp tonight's first accelerando from existing tempo-stability changes.""" beat_times = _coerce_beat_times(audio_features) if beat_times is None: beat_times = derive_beat_times(mix, sr) - apply_accelerando_plan(song, beat_times) + apply_accelerando_plan(song, beat_times, section_boundaries) def _reconstruct_mix(stems: dict[str, Any]) -> Any: diff --git a/services/analysis-engine/src/bandscope_analysis/roles/model.py b/services/analysis-engine/src/bandscope_analysis/roles/model.py index faa1651aa..165e5c21c 100644 --- a/services/analysis-engine/src/bandscope_analysis/roles/model.py +++ b/services/analysis-engine/src/bandscope_analysis/roles/model.py @@ -85,6 +85,7 @@ class RehearsalRole(TypedDict): overlapWarnings: list[str] accelerandoPlan: NotRequired[str] accelerandoPlanSource: NotRequired[Literal["model", "user"]] + accelerandoPlanAtSeconds: NotRequired[float] class PartGraphNode(TypedDict): diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py index 9d8ef711c..926638535 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py @@ -159,8 +159,31 @@ def _active_role_ids(section: Mapping[str, Any]) -> set[str]: return active -def _section_contains(section: Mapping[str, Any], time: float) -> bool: +def _section_contains( + section: Mapping[str, Any], + time: float, + precise_boundary: Sequence[float] | None = None, +) -> bool: """Return whether a section window contains a tempo-change time.""" + if precise_boundary is not None: + if ( + not isinstance(precise_boundary, Sequence) + or isinstance(precise_boundary, (str, bytes)) + or len(precise_boundary) != 2 + or any( + isinstance(value, bool) + or not isinstance(value, (int, float)) + or not isfinite(value) + for value in precise_boundary + ) + ): + return False + precise_start, precise_end = (float(value) for value in precise_boundary) + return ( + precise_start >= 0 + and precise_end > precise_start + and precise_start <= time < precise_end + ) time_range = section.get("timeRange") if not isinstance(time_range, Mapping): return False @@ -232,12 +255,17 @@ def derive_beat_times(mix: Any, sr: Any) -> list[float] | None: return None -def apply_accelerando_plan(song: Mapping[str, Any], beat_times: Sequence[float] | None) -> None: +def apply_accelerando_plan( + song: Mapping[str, Any], + beat_times: Sequence[float] | None, + section_boundaries: Sequence[Sequence[float]] | None = None, +) -> None: """Attach the first corroborated accelerando plan, failing closed on bad input. Args: song: Mutable rehearsal-song mapping with section/role topology. beat_times: Beat onset times in seconds used by tempo-stability. + section_boundaries: Optional unrounded section boundaries aligned to sections. """ if ( beat_times is None @@ -256,8 +284,18 @@ def apply_accelerando_plan(song: Mapping[str, Any], beat_times: Sequence[float] sections = song.get("sections") if not isinstance(sections, list): return - for section in sections: - if not isinstance(section, Mapping) or not _section_contains(section, change["time"]): + for section_index, section in enumerate(sections): + precise_boundary = None + if ( + section_boundaries is not None + and isinstance(section_boundaries, Sequence) + and not isinstance(section_boundaries, (str, bytes)) + and section_index < len(section_boundaries) + ): + precise_boundary = section_boundaries[section_index] + if not isinstance(section, Mapping) or not _section_contains( + section, change["time"], precise_boundary + ): continue landing = _pick_landing_role(section) if landing is None: @@ -271,6 +309,7 @@ def apply_accelerando_plan(song: Mapping[str, Any], beat_times: Sequence[float] stamped = dict(landing) stamped["accelerandoPlan"] = copy stamped["accelerandoPlanSource"] = "model" + stamped["accelerandoPlanAtSeconds"] = change["time"] roles[index] = stamped return return diff --git a/services/analysis-engine/tests/test_accelerando_plan.py b/services/analysis-engine/tests/test_accelerando_plan.py index 7ffaf8c55..959412d8c 100644 --- a/services/analysis-engine/tests/test_accelerando_plan.py +++ b/services/analysis-engine/tests/test_accelerando_plan.py @@ -242,6 +242,9 @@ def test_apply_fails_closed_on_malformed_song_topology() -> None: """Malformed sections, ranges, and graph nodes never invent a rit.""" apply_accelerando_plan({"sections": "nope"}, _beats_80_to_120()) apply_accelerando_plan({"sections": [{"timeRange": "nope", "roles": []}]}, _beats_80_to_120()) + apply_accelerando_plan( + _song_with_section(), _beats_80_to_120(), [(0.0,)] # type: ignore[list-item] + ) song = _song_with_section() song["sections"][0]["timeRange"] = {"start": True, "end": 16} apply_accelerando_plan(song, _beats_80_to_120()) @@ -353,6 +356,9 @@ def test_coerce_beat_times_and_pipeline_stamp(monkeypatch: pytest.MonkeyPatch) - _apply_accelerando(song, mix, 22050, {"beat_times": _beats_80_to_120()}) vocal = next(role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal") assert vocal["accelerandoPlan"] == _ACCEL_PLAN + change = first_accelerando(analyze_tempo_stability(_beats_80_to_120())["tempo_changes"]) + assert change is not None + assert vocal["accelerandoPlanAtSeconds"] == change["time"] def fail_if_redecoded(*args: Any, **kwargs: Any) -> list[float] | None: raise AssertionError("an empty authoritative beat grid must not be re-decoded") @@ -368,6 +374,23 @@ def fail_if_redecoded(*args: Any, **kwargs: Any) -> list[float] | None: assert all("accelerandoPlan" not in role for role in unnamed["sections"][0]["roles"]) +def test_pipeline_uses_unrounded_boundaries_for_accelerando_section() -> None: + """A fractional structural boundary must not be truncated before section selection.""" + earlier = _song_with_section(start=0, end=11) + later = _song_with_section(start=11, end=20) + song = earlier + song["sections"].extend(later["sections"]) + + apply_accelerando_plan(song, _beats_80_to_120(), [(0.0, 11.9), (11.9, 20.0)]) + + earlier_vocal = next( + role for role in song["sections"][0]["roles"] if role["id"] == "lead-vocal" + ) + later_vocal = next(role for role in song["sections"][1]["roles"] if role["id"] == "lead-vocal") + assert earlier_vocal["accelerandoPlan"] == _ACCEL_PLAN + assert "accelerandoPlan" not in later_vocal + + def test_apply_accelerando_reuses_provided_beat_times(monkeypatch: pytest.MonkeyPatch) -> None: """Provided temporal features avoid a second beat-tracking pass.""" From 8f8b2da7815741f4af96c70510951d01a4d302fc Mon Sep 17 00:00:00 2001 From: seonghobae Date: Sat, 29 Aug 2026 14:46:04 +0900 Subject: [PATCH 23/23] style(accelerando): format boundary test --- services/analysis-engine/tests/test_accelerando_plan.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_accelerando_plan.py b/services/analysis-engine/tests/test_accelerando_plan.py index 959412d8c..2941e25c5 100644 --- a/services/analysis-engine/tests/test_accelerando_plan.py +++ b/services/analysis-engine/tests/test_accelerando_plan.py @@ -243,7 +243,9 @@ def test_apply_fails_closed_on_malformed_song_topology() -> None: apply_accelerando_plan({"sections": "nope"}, _beats_80_to_120()) apply_accelerando_plan({"sections": [{"timeRange": "nope", "roles": []}]}, _beats_80_to_120()) apply_accelerando_plan( - _song_with_section(), _beats_80_to_120(), [(0.0,)] # type: ignore[list-item] + _song_with_section(), + _beats_80_to_120(), + [(0.0,)], # type: ignore[list-item] ) song = _song_with_section() song["sections"][0]["timeRange"] = {"start": True, "end": 16}