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..23e73a18c 100644 --- a/apps/desktop/core/src/lib.rs +++ b/apps/desktop/core/src/lib.rs @@ -176,6 +176,37 @@ 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 { + Model, + User, +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct RehearsalRolePayload { @@ -183,14 +214,32 @@ 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, + #[serde(default, skip_serializing_if = "Option::is_none")] + accelerando_plan_at_seconds: Option, } #[derive(Clone, Debug, Serialize)] @@ -527,9 +576,79 @@ 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 +} + +const MAX_SECTION_TIME_SECONDS: f64 = 4_294_967_295.0; + +fn validate_accelerando_plan_provenance( + payload: RehearsalSongPayload, +) -> Result { + for section in &payload.sections { + for role in §ion.roles { + if role + .accelerando_plan + .as_deref() + .is_some_and(|accelerando_plan| !is_valid_accelerando_plan(accelerando_plan)) + { + 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()); + } + 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) +} + 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 +666,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" + }, + "harmonicExplanation": "The landing keeps the tonal floor clear.", + "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.", + "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", + "accelerandoPlanAtSeconds": 12.375 + } + ], + "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") + ); + assert_eq!( + serialized["sections"][0]["roles"][0]["accelerandoPlanAtSeconds"], + json!(12.375) + ); +} + +#[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 [ + "", + " ", + "\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"); + + assert!( + project_payload_from_content(&content).is_err(), + "native persisted contract must reject blank or multiline sourced accelerando-plan copy" + ); + } +} + +#[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(); + 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" + ); +} + +#[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(); + 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/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..cb10e17ec --- /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; + 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, + 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..29eac02f3 --- /dev/null +++ b/apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx @@ -0,0 +1,84 @@ +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 edit and role switch", () => { + 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(); + + 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(); + }); + + 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 d44e20777..17faf3c85 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, 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"; 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,22 @@ 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); + const isLocalWorkspaceUpdate = song === localSongUpdateRef.current; + const isExternalWorkspaceUpdate = song !== previousSongRef.current && !isLocalWorkspaceUpdate; + const workspaceInstanceKey = isExternalWorkspaceUpdate ? song : workspaceInstanceRef.current; + + useEffect(() => { + if (song !== previousSongRef.current) { + if (!isLocalWorkspaceUpdate) { + workspaceInstanceRef.current = song; + } + localSongUpdateRef.current = null; + previousSongRef.current = song; + } + }, [isLocalWorkspaceUpdate, song]); // Extract all unique roles from the song's sections const roleMap = useMemo(() => { @@ -164,6 +185,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 +216,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 +337,10 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp

{t("workspaceFirstRangeTitle")}

{firstRangeCopy}

+
@@ -505,7 +537,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..63b7c0306 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstAccelerando.test.ts @@ -0,0 +1,240 @@ +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; + accelerandoPlanAtSeconds?: number; + 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.accelerandoPlanAtSeconds !== undefined + ? { accelerandoPlanAtSeconds: overrides.accelerandoPlanAtSeconds } + : {}), + ...(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("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; + 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: `${DEMO_ACCELERANDO_PLAN}\n` }) + ) + ).toBeNull(); + expect( + resolveFirstAccelerandoPlan(withAccelerandoSection({ accelerandoPlan: "slow down here" })) + ).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( + 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("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, + source: "user" + }) + ); + expect(resolved?.accelerandoPlan).toBe(accelerandoPlan); + 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(); + }); + + 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")!; + 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..3adb8a9e5 --- /dev/null +++ b/apps/desktop/src/features/workspace/firstAccelerando.ts @@ -0,0 +1,435 @@ +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 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."; +const DOUBLE_TIME_RATIO_MIN = 1.9; +const DOUBLE_TIME_RATIO_MAX = 2.1; + +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; + guidance: AccelerandoPlanGuidance | null; + atSeconds: number | 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; + 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 keys = Object.keys(value); + if (keys.length !== length) { + return null; + } + const items: unknown[] = []; + for (const [index, key] of keys.entries()) { + if (key !== String(index) || !hasOwnData(value, key)) { + return null; + } + items.push(ownDataValue(value, key)); + } + return items; +} + +/** Preserve the engine accelerando template while enforcing the engine's speeding semantics. */ +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; + } + 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", + guidance: { kind: "tempo", fromBpm, toBpm }, + atSeconds: null + }; +} + +/** 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"); + const accelerandoPlanAtSeconds = ownDataValue(role, "accelerandoPlanAtSeconds"); + if (typeof accelerandoPlan !== "string") { + return null; + } + if (accelerandoPlanSource !== "model" && accelerandoPlanSource !== "user") { + return 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(); + const bounded = boundedGeneratedAccelerandoPlan(trimmed); + return bounded === null + ? null + : { ...bounded, atSeconds: (accelerandoPlanAtSeconds as number | undefined) ?? null }; + } + return { + text: accelerandoPlan, + source: accelerandoPlanSource, + guidance: null, + atSeconds: (accelerandoPlanAtSeconds as number | undefined) ?? 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, + atSeconds: accelerandoPlan.atSeconds + } + ]; + }) + ); + 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: landingRole.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..fddebe28a 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -143,6 +143,9 @@ export type RehearsalRole = { overlapWarnings: string[]; transcription?: TranscriptionNote[]; practiceProgress?: number; + accelerandoPlan?: string; + accelerandoPlanSource?: ProvenanceSource; + accelerandoPlanAtSeconds?: number; }; /** Documented. */ @@ -407,6 +410,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}'`; @@ -1500,7 +1545,10 @@ function validateRehearsalRole(value: unknown, path: string): string | null { "manualOverrides", "overlapWarnings", "transcription", - "practiceProgress" + "practiceProgress", + "accelerandoPlan", + "accelerandoPlanSource", + "accelerandoPlanAtSeconds" ], path ); @@ -1588,6 +1636,40 @@ function validateRehearsalRole(value: unknown, path: string): string | null { } } + if ( + value.accelerandoPlan !== undefined && + !isNonEmptySingleLineText(value.accelerandoPlan) + ) { + 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`); + } + 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 new file mode 100644 index 000000000..43c784d42 --- /dev/null +++ b/packages/shared-types/test/accelerandoPlanProvenance.test.ts @@ -0,0 +1,105 @@ +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("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]!; + 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([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([ + "", + " ", + "\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" + ); + }); +}); 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..6fc84ae3a 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -19,11 +19,12 @@ 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__) 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 @@ -116,6 +117,9 @@ class RehearsalRolePayload(TypedDict): setupNote: str manualOverrides: list[ManualOverridePayload] overlapWarnings: list[str] + accelerandoPlan: NotRequired[str] + accelerandoPlanSource: NotRequired[Literal["model", "user"]] + accelerandoPlanAtSeconds: NotRequired[float] class PartGraphNodePayload(TypedDict): @@ -196,6 +200,7 @@ class CachedFeaturePayload(TypedDict): separation: dict[str, object] stemKeys: list[str] stemRoleTypes: dict[str, str] + temporalFeatures: NotRequired[dict[str, object]] class StemSeparationTimedOut(RuntimeError): @@ -456,6 +461,7 @@ def _build_from_pipeline( }, } _apply_tempo(song, features) + _apply_accelerando(song, mix, sr, features, boundaries) return song @@ -518,6 +524,72 @@ 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): + 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 _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, + 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, section_boundaries) + + def _reconstruct_mix(stems: dict[str, Any]) -> Any: """Reconstruct a mono mix from separated stems for segmentation.""" arrays = [] @@ -613,7 +685,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: @@ -743,6 +815,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: @@ -758,7 +835,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, @@ -768,6 +845,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: @@ -828,6 +922,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") @@ -1043,8 +1140,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: @@ -1180,6 +1282,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, @@ -1228,6 +1333,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..74cbd088b 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -6,8 +6,18 @@ import logging import sys from datetime import UTC, datetime - -from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates +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, + validate_analysis_job_request, +) from bandscope_analysis.temporal import TemporalAnalyzer logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") @@ -74,22 +84,39 @@ def main() -> int: return 0 request = payload.get("request") + validated_request = None + try: + validated_request = validate_analysis_job_request(request) + except ValueError as error: + logging.debug("Analysis request validation deferred to orchestration: %s", error) + if validated_request is not None: + request = validated_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" - 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: + 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( @@ -99,13 +126,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/src/bandscope_analysis/roles/model.py b/services/analysis-engine/src/bandscope_analysis/roles/model.py index ea6fc1449..165e5c21c 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,9 @@ class RehearsalRole(TypedDict): setupNote: str manualOverrides: list[ManualOverride] 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/__init__.py b/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py index 104b82ec9..ba4056a10 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/__init__.py @@ -1,5 +1,6 @@ """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 @@ -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..926638535 --- /dev/null +++ b/services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py @@ -0,0 +1,317 @@ +"""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, + 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 + 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, + 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 + 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_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: + return + 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" + stamped["accelerandoPlanAtSeconds"] = change["time"] + roles[index] = stamped + return + 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..2941e25c5 --- /dev/null +++ b/services/analysis-engine/tests/test_accelerando_plan.py @@ -0,0 +1,437 @@ +"""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()) + 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()) + 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(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": []}) == [] + 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 + 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") + + 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"]) + + +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.""" + + 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 + 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"} 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 diff --git a/services/analysis-engine/tests/test_api.py b/services/analysis-engine/tests/test_api.py index 18273791d..f146830a8 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, @@ -582,7 +585,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 @@ -648,7 +651,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 @@ -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,44 @@ 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 057ef236b..cb3d9baec 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, @@ -387,7 +423,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 +441,110 @@ 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_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(