diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dd5d1a..1766781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Multiple drum parts (feedpak 1.17.0 "drums as arrangements").** The sloppak + loader now reads `type: drums` arrangement entries carrying per-arrangement + `drum_tab` file pointers — a song can ship several drum charts (a second + drummer, an aux-percussion layer). Parts surface as `LoadedSloppak.drum_parts` + (primary first; the entry aliasing the song-level `drum_tab:` key is the + primary and is never loaded twice), the highway WS `song_info` gains a + `drum_parts` name list, and `?drum_part=` on the WS URL selects which + part's tab streams (`drum_tab` messages carry `part_id` when multiple parts + exist; unknown ids fall back to the primary). Pointer entries are **never** + loaded as fretted arrangements — the loader's file/notation gate keeps a drum + part out of the fretted pipeline (and out of note-detection grading), pinned + by test. Legacy single-drum packs read exactly as before, as a one-part list. - **`chart-transform` capability domain (#952)** — plugins can now remap the chart before rendering and scoring through a core-owned provider coordinator. Synchronous transforms run after difficulty filtering; host diff --git a/lib/routers/ws_highway.py b/lib/routers/ws_highway.py index 7950a6c..8fae471 100644 --- a/lib/routers/ws_highway.py +++ b/lib/routers/ws_highway.py @@ -143,9 +143,21 @@ def _sanitize_authors(manifest: dict | None) -> list[dict]: return out +def _drum_part_id_for_wire(drum_parts: list[dict] | None, selected_id: str | None) -> str | None: + """Expose a part id only when the pack genuinely has multiple parts.""" + return selected_id if selected_id is not None and len(drum_parts or []) > 1 else None + + @router.websocket("/ws/highway/{filename:path}") -async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1, naming_mode: str = "legacy"): - """Stream song data for the highway renderer over WebSocket.""" +async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1, + naming_mode: str = "legacy", drum_part: str = ""): + """Stream song data for the highway renderer over WebSocket. + + `drum_part` selects WHICH drum part's tab streams when the pack carries + several (feedpak 1.17.0 "drums as arrangements") — a part id from + song_info's `drum_parts`. Empty / unknown ids fall back to the primary, + so a stale or mistyped selection degrades to today's behavior instead of + silencing drums.""" await websocket.accept() structlog.contextvars.bind_contextvars(ws_conn_id=uuid.uuid4().hex[:8]) @@ -564,6 +576,15 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1, "has_drum_tab": bool( is_slop and loaded_slop is not None and loaded_slop.drum_tab is not None ), + # The song's DRUM PARTS (feedpak 1.17.0 "drums as arrangements"), + # primary first — names only; the selected part's payload streams + # as the `drum_tab`/`drum_hits` messages below. Always a list + # (empty when the pack has no drums, and a single entry for a + # legacy one-drum pack), so a part picker can bind unconditionally. + "drum_parts": [ + {"id": p["id"], "name": p["name"]} + for p in (loaded_slop.drum_parts or []) + ] if is_slop and loaded_slop is not None else [], "has_notation": bool( is_slop and loaded_slop is not None @@ -587,18 +608,36 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1, # client-side drums plugin keeps a fallback decoder for them. if is_slop and loaded_slop is not None and loaded_slop.drum_tab is not None: dt = loaded_slop.drum_tab + # Multiple drum parts: `?drum_part=` picks which part's tab + # streams; the default (and any unknown id) is the PRIMARY — + # exactly the pre-parts behavior, so legacy clients notice nothing. + _dt_part_id = None + if loaded_slop.drum_parts: + _dt_part_id = loaded_slop.drum_parts[0]["id"] + if drum_part: + for _p in loaded_slop.drum_parts: + if _p["id"] == drum_part: + dt = _p["drum_tab"] + _dt_part_id = _p["id"] + break kit = drums_mod.normalise_kit(dt.get("kit")) hits_wire = drums_mod.hits_to_wire(dt.get("hits") or []) _dt_name = dt.get("name") _dt_name = _dt_name if isinstance(_dt_name, str) and _dt_name else "Drums" + _dt_msg = { + "type": "drum_tab", + "version": int(dt.get("version", drums_mod.SCHEMA_VERSION)), + "name": _dt_name, + "kit": kit, + "total": len(hits_wire), + } + # Only multi-part packs identify a part on the wire. Legacy packs + # synthesize a one-item list internally but keep their old frame. + _wire_part_id = _drum_part_id_for_wire(loaded_slop.drum_parts, _dt_part_id) + if _wire_part_id is not None: + _dt_msg["part_id"] = _wire_part_id try: - await websocket.send_json({ - "type": "drum_tab", - "version": int(dt.get("version", drums_mod.SCHEMA_VERSION)), - "name": _dt_name, - "kit": kit, - "total": len(hits_wire), - }) + await websocket.send_json(_dt_msg) for i in range(0, len(hits_wire), 500): await websocket.send_json({ "type": "drum_hits", diff --git a/lib/sloppak.py b/lib/sloppak.py index dd806c4..396f770 100644 --- a/lib/sloppak.py +++ b/lib/sloppak.py @@ -730,6 +730,125 @@ class LoadedSloppak: # separated stems the moment one drops below 100% — demucs recombination is # lossy, so the mixdown is strictly the better audio when nothing is muted. full_mix: str | None = None + # The song's DRUM PARTS (feedpak 1.17.0 "drums as arrangements"): one dict + # {"id", "name", "drum_tab"} per part, primary FIRST. A part comes from a + # `type: drums` arrangement entry carrying a per-arrangement `drum_tab` + # file pointer and NO note `file` — entries this loader deliberately never + # turns into fretted Arrangements (see the file/notation gate in + # load_song; that skip IS the grading invariant). The primary part's + # payload is the SAME object as `drum_tab` above (the song-level key is + # its back-compat alias). None when the pack has no drums at all; a + # single-part list for a legacy pack with only the song-level key. + drum_parts: list[dict] | None = None + + +def _load_drum_tab_file(source_dir: Path, rel: str, label: str) -> dict | None: + """Load + schema-validate one drum-tab JSON named by a manifest-relative + path. Shared by the song-level `drum_tab:` key and the per-arrangement + drum-part pointers (feedpak 1.17.0), so every tab gets the same posture: + permissive — a missing file disables that part silently; a traversal, + parse, or validation failure disables it with a warning, never aborting + the load.""" + # Constrain to source_dir to prevent a crafted manifest from reading + # files outside the sloppak directory via path traversal (e.g. ../../etc). + # Wrap both resolve() calls in a broad handler: symlink loops and + # permission errors on .resolve() should disable drums, not abort load. + try: + dt_path = (source_dir / rel).resolve() + dt_path.relative_to(source_dir.resolve()) + except ValueError: + log.warning("sloppak: %s path %r escapes source_dir — skipped", label, rel) + return None + except OSError as e: + log.warning("sloppak: %s path resolution failed (%s) — skipped", label, e) + return None + if not dt_path.exists(): + return None + try: + raw = load_json(dt_path) + except Exception as e: + log.warning("sloppak: failed to parse %s %r: %s", label, rel, e) + return None + ok, reason = drums_mod.validate_drum_tab(raw) + if not ok: + log.warning("sloppak: %s %r failed validation: %s", label, rel, reason) + return None + return raw + + +def _resolve_drum_parts( + source_dir: Path, + drum_tab_rel: object, + drum_tab_data: dict | None, + drum_pointer_entries: list[dict], +) -> tuple[dict | None, list[dict] | None]: + """Resolve drum pointers into a primary-first list with unique ids.""" + if drum_tab_data is None and not drum_pointer_entries: + return drum_tab_data, None + + primary_id = "drums" + primary_name = None + extra_parts: list[dict] = [] + seen_rels: set[str] = set() + # Use the same canonical, traversal-safe identity as zip member lookup so + # equivalent spellings ("x.json", "./x.json", or backslashes) identify + # one file. Otherwise an alias pointer can reload and duplicate the primary. + primary_rel_key = ( + _zip_member_key(drum_tab_rel.strip()) + if isinstance(drum_tab_rel, str) and drum_tab_rel.strip() else None + ) + for entry in drum_pointer_entries: + rel = str(entry.get("drum_tab") or "").strip() + rel_key = _zip_member_key(rel) if rel else None + rel_identity = rel_key or rel + if not rel or rel_identity in seen_rels: + continue + seen_rels.add(rel_identity) + entry_id = str(entry.get("id") or "").strip() + entry_name = str(entry.get("name") or "").strip() + if primary_rel_key is not None and rel_key == primary_rel_key: + if entry_id: + primary_id = entry_id + if entry_name: + primary_name = entry_name + continue + tab = _load_drum_tab_file(source_dir, rel, f"drum part {entry_id or rel}") + if tab is None: + continue + tab_name = tab.get("name") + extra_parts.append({ + "id": entry_id, + "name": entry_name + or (tab_name if isinstance(tab_name, str) and tab_name else "Drums"), + "drum_tab": tab, + }) + + parts: list[dict] = [] + used_ids: set[str] = set() + if drum_tab_data is not None: + if primary_name is None: + tab_name = drum_tab_data.get("name") + primary_name = tab_name if isinstance(tab_name, str) and tab_name else "Drums" + parts.append({"id": primary_id, "name": primary_name, "drum_tab": drum_tab_data}) + used_ids.add(primary_id) + + next_generated_id = 2 + for part in extra_parts: + part_id = part["id"] + if not part_id or part_id in used_ids: + while f"drums-{next_generated_id}" in used_ids: + next_generated_id += 1 + part_id = f"drums-{next_generated_id}" + next_generated_id += 1 + part["id"] = part_id + used_ids.add(part_id) + parts.append(part) + + if not parts: + return drum_tab_data, None + if drum_tab_data is None: + drum_tab_data = parts[0]["drum_tab"] + return drum_tab_data, parts def load_song( @@ -754,6 +873,7 @@ def load_song( notation_acc: dict[str, dict] = {} any_notation = False arrangement_ids_acc: list[str | None] = [] # parallel to song.arrangements + drum_pointer_entries: list[dict] = [] # feedpak 1.17.0 drum-part pointers for entry in manifest.get("arrangements", []) or []: if not isinstance(entry, dict): log.warning("sloppak: non-dict arrangement entry skipped (%r)", type(entry).__name__) @@ -762,7 +882,30 @@ def load_song( rel = rel_raw.strip() if isinstance(rel_raw, str) else "" notation_raw = entry.get("notation") has_notation_key = isinstance(notation_raw, str) and bool(notation_raw.strip()) - if not rel and not has_notation_key: + _etype = str(entry.get("type") or "").strip().lower() + is_drums = _etype in ("drums", "drum") + # A drums-typed entry MUST NEVER become a fretted Arrangement (grading + # invariant, spec §5.2/§7.5): route on `type` FIRST, not on file + # absence — a malformed drums entry that also carries a note file/ + # notation would otherwise fall through and grade as garbage. + if is_drums or (not rel and not has_notation_key): + # A DRUM-PART POINTER entry (feedpak 1.17.0 "drums as + # arrangements"): `type: drums` with a per-arrangement `drum_tab` + # file. Collect it for the drum-parts load after this loop. + if is_drums and isinstance(entry.get("drum_tab"), str): + drum_pointer_entries.append(entry) + elif is_drums: + # Drums-typed but no drum_tab pointer — drop it (any note + # file/notation it carries is ignored), never fret it. + log.warning( + "sloppak: drums-typed arrangement entry %r has no drum_tab pointer — dropped", + entry.get("id"), + ) + elif isinstance(entry.get("drum_tab"), str): + log.warning( + "sloppak: arrangement entry has drum_tab %r but type=%r — ignored", + entry.get("drum_tab"), entry.get("type"), + ) continue data = None if rel: @@ -868,32 +1011,13 @@ def load_song( drum_tab_data: dict | None = None drum_tab_rel = manifest.get("drum_tab") if isinstance(drum_tab_rel, str) and drum_tab_rel: - # Constrain to source_dir to prevent a crafted manifest from reading - # files outside the sloppak directory via path traversal (e.g. ../../etc). - # Wrap both resolve() calls in a broad handler: symlink loops and - # permission errors on .resolve() should disable drums, not abort load. - try: - dt_path = (source_dir / drum_tab_rel).resolve() - dt_path.relative_to(source_dir.resolve()) - except ValueError: - log.warning("sloppak: drum_tab path %r escapes source_dir — skipped", drum_tab_rel) - dt_path = None - except OSError as e: - log.warning("sloppak: drum_tab path resolution failed (%s) — skipped", e) - dt_path = None - if dt_path is not None and dt_path.exists(): - try: - raw = load_json(dt_path) - except Exception as e: - log.warning("sloppak: failed to parse drum_tab %r: %s", drum_tab_rel, e) - raw = None - if raw is not None: - ok, reason = drums_mod.validate_drum_tab(raw) - if ok: - drum_tab_data = raw - else: - log.warning("sloppak: drum_tab %r failed validation: %s", - drum_tab_rel, reason) + drum_tab_data = _load_drum_tab_file(source_dir, drum_tab_rel, "drum_tab") + + # Keep the dense compatibility logic independently testable and guarantee + # ids are unique before the highway exposes them as selectors. + drum_tab_data, drum_parts = _resolve_drum_parts( + source_dir, drum_tab_rel, drum_tab_data, drum_pointer_entries, + ) # Drum-only sloppak: every GP track was percussion, so it ships a # drum_tab but no pitched arrangements. The highway WS rejects an empty @@ -1221,6 +1345,7 @@ def load_song( manifest=manifest, feedpak_version=_fpv if isinstance(_fpv, str) and _fpv else None, drum_tab=drum_tab_data, + drum_parts=drum_parts, song_timeline=song_timeline_data, tempos=tempos_data, time_signatures=time_sigs_data, diff --git a/tests/test_highway_ws_drum_parts.py b/tests/test_highway_ws_drum_parts.py new file mode 100644 index 0000000..1ded8b9 --- /dev/null +++ b/tests/test_highway_ws_drum_parts.py @@ -0,0 +1,17 @@ +"""Wire-compatibility coverage for selectable drum parts.""" + +from routers.ws_highway import _drum_part_id_for_wire + + +def test_single_synthesized_part_keeps_legacy_frame_without_part_id(): + parts = [{"id": "drums", "name": "Drums", "drum_tab": {}}] + assert _drum_part_id_for_wire(parts, "drums") is None + + +def test_multiple_parts_expose_selected_part_id(): + parts = [ + {"id": "drums", "name": "Drums", "drum_tab": {}}, + {"id": "drums-2", "name": "Aux", "drum_tab": {}}, + ] + assert _drum_part_id_for_wire(parts, "drums-2") == "drums-2" + assert _drum_part_id_for_wire(parts, None) is None diff --git a/tests/test_sloppak_drum_parts.py b/tests/test_sloppak_drum_parts.py new file mode 100644 index 0000000..b4d20f6 --- /dev/null +++ b/tests/test_sloppak_drum_parts.py @@ -0,0 +1,295 @@ +"""Loader coverage for MULTIPLE drum parts (feedpak 1.17.0 "drums as +arrangements"). + +A drum part rides the manifest as a `type: drums` arrangement entry carrying +a per-arrangement `drum_tab` file pointer and NO note `file`. The loader: + + - NEVER turns a pointer entry into a fretted Arrangement — that skip is + the grading invariant (an empty drum chart must not reach the fretted + pipeline, where note detection would grade it as garbage); + - resolves the parts into `LoadedSloppak.drum_parts`, primary FIRST: the + entry aliasing the song-level `drum_tab:` file contributes its id/name + but is never loaded twice (its payload IS `loaded.drum_tab`); + - loads each extra part's file with the same permissive posture as the + song-level tab (a bad part disables that part only, never the load); + - copes with a pointer-only pack (no song-level key): the first part + becomes the primary so every legacy consumer keeps working. +""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path + +import yaml + +import sloppak as sloppak_mod + + +def _tab(name: str, hits: list[dict] | None = None) -> dict: + return { + "version": 1, + "name": name, + "kit": [{"id": "kick", "name": "Kick"}], + "hits": hits if hits is not None else [{"t": 1.0, "p": "kick", "v": 100}], + } + + +def _write_pak(root: Path, manifest_extras: dict, files: dict[str, dict | str]) -> Path: + """A minimal directory-form sloppak with one Lead arrangement plus the + given extra files ({relpath: json-dict-or-raw-text}).""" + pak = root / f"{root.name}.sloppak" + pak.mkdir() + arr_dir = pak / "arrangements" + arr_dir.mkdir() + arr = { + "name": "Lead", "tuning": [0, 0, 0, 0, 0, 0], "capo": 0, + "notes": [], "chords": [], "anchors": [], "handshapes": [], + "templates": [], "beats": [], "sections": [], + } + (arr_dir / "lead.json").write_text(json.dumps(arr)) + manifest = { + "title": "Test", "artist": "Tester", "album": "", "year": 2026, + "duration": 10.0, + "arrangements": [ + {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}, + ], + "stems": [{"id": "full", "file": "stems/full.ogg", "default": True}], + } + manifest.update(manifest_extras) + (pak / "manifest.yaml").write_text(yaml.safe_dump(manifest, sort_keys=False)) + for rel, payload in files.items(): + text = payload if isinstance(payload, str) else json.dumps(payload) + (pak / rel).write_text(text) + return pak + + +def _load(pak_path: Path, tmp_path: Path): + dlc_root = pak_path.parent + cache = tmp_path / "cache" + cache.mkdir() + return sloppak_mod.load_song(pak_path.name, dlc_root, cache) + + +def _two_part_manifest() -> dict: + """The exact shape the editor writes: primary alias entry + one extra.""" + return { + "drum_tab": "drum_tab.json", + "arrangements": [ + {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}, + {"id": "drums", "name": "Drums", "type": "drums", + "drum_tab": "drum_tab.json"}, + {"id": "drums-2", "name": "Drums (Live)", "type": "drums", + "drum_tab": "drum_tab_drums-2.json"}, + ], + } + + +# ── The grading invariant ──────────────────────────────────────────────────── + +def test_pointer_entries_never_become_fretted_arrangements(tmp_path: Path): + pak = _write_pak(tmp_path, _two_part_manifest(), { + "drum_tab.json": _tab("Drums"), + "drum_tab_drums-2.json": _tab("Drums (Live)"), + }) + loaded = _load(pak, tmp_path) + # Only the Lead chart is an Arrangement — neither drum part enters the + # fretted pipeline (song.arrangements is what note detection grades). + assert [a.name for a in loaded.song.arrangements] == ["Lead"] + # And the ids list stays parallel to song.arrangements (skipped entries + # contribute nothing) — a misalignment here would remap every chart edit. + assert loaded.arrangement_ids == ["lead"] + + +def test_drums_typed_entry_with_note_file_never_frets(tmp_path: Path): + # A malformed entry: type:drums but ALSO carrying a note `file`. Keying the + # skip on file absence would let it through as a fretted, selectable, + # gradeable Arrangement (spec §5.2/§7.5 MUST-NOT). Routing on `type` first + # drops it instead — it never reaches song.arrangements. + bogus = { + "name": "Bogus", "tuning": [0, 0, 0, 0, 0, 0], "capo": 0, + "notes": [], "chords": [], "anchors": [], "handshapes": [], + "templates": [], "beats": [], "sections": [], + } + pak = _write_pak(tmp_path, { + "arrangements": [ + {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}, + {"id": "bad", "name": "Bogus", "type": "drums", + "file": "arrangements/bogus.json"}, + ], + }, {"arrangements/bogus.json": bogus}) + loaded = _load(pak, tmp_path) + assert [a.name for a in loaded.song.arrangements] == ["Lead"] + assert loaded.arrangement_ids == ["lead"] + + +# ── Parts resolution ───────────────────────────────────────────────────────── + +def test_two_parts_resolve_primary_first_with_alias_identity(tmp_path: Path): + pak = _write_pak(tmp_path, _two_part_manifest(), { + "drum_tab.json": _tab("Drums"), + "drum_tab_drums-2.json": _tab("Drums (Live)", [{"t": 2.0, "p": "kick", "v": 90}]), + }) + loaded = _load(pak, tmp_path) + assert loaded.drum_parts is not None + assert [(p["id"], p["name"]) for p in loaded.drum_parts] == [ + ("drums", "Drums"), ("drums-2", "Drums (Live)"), + ] + # The primary's payload IS the song-level tab — same object, loaded once. + assert loaded.drum_parts[0]["drum_tab"] is loaded.drum_tab + assert loaded.drum_parts[1]["drum_tab"]["hits"][0]["t"] == 2.0 + + +def test_primary_pointer_equivalent_path_is_not_duplicated(tmp_path: Path): + manifest = { + "drum_tab": "drum_tab.json", + "arrangements": [ + {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}, + {"id": "kit", "name": "Live Kit", "type": "drums", + "drum_tab": "./drum_tab.json"}, + ], + } + pak = _write_pak(tmp_path, manifest, {"drum_tab.json": _tab("Drums")}) + loaded = _load(pak, tmp_path) + assert loaded.drum_parts is not None + assert [(p["id"], p["name"]) for p in loaded.drum_parts] == [ + ("kit", "Live Kit"), + ] + assert loaded.drum_parts[0]["drum_tab"] is loaded.drum_tab + + +def test_legacy_single_drum_pack_gets_a_one_part_list(tmp_path: Path): + pak = _write_pak(tmp_path, {"drum_tab": "drum_tab.json"}, { + "drum_tab.json": _tab("Drums"), + }) + loaded = _load(pak, tmp_path) + assert loaded.drum_parts is not None and len(loaded.drum_parts) == 1 + assert loaded.drum_parts[0]["id"] == "drums" + assert loaded.drum_parts[0]["drum_tab"] is loaded.drum_tab + + +def test_no_drums_means_no_parts(tmp_path: Path): + pak = _write_pak(tmp_path, {}, {}) + loaded = _load(pak, tmp_path) + assert loaded.drum_parts is None + assert loaded.drum_tab is None + + +def test_pointer_only_pack_promotes_the_first_part_to_primary(tmp_path: Path): + # A writer that omitted the song-level alias: readers must cope (the + # spec keeps the alias, but a reader never crashes on its absence). + pak = _write_pak(tmp_path, { + "arrangements": [ + {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}, + {"id": "kit", "name": "Kit", "type": "drums", + "drum_tab": "drum_tab_kit.json"}, + ], + }, {"drum_tab_kit.json": _tab("Kit")}) + loaded = _load(pak, tmp_path) + assert loaded.drum_parts is not None and len(loaded.drum_parts) == 1 + # The part's tab becomes THE drum tab, so has_drum_tab / the default + # stream / the drum-only placeholder all keep working. + assert loaded.drum_tab is loaded.drum_parts[0]["drum_tab"] + assert loaded.drum_parts[0]["id"] == "kit" + + +# ── Permissive per-part failure ────────────────────────────────────────────── + +def test_a_bad_extra_part_disables_that_part_only(tmp_path: Path): + manifest = _two_part_manifest() + manifest["arrangements"].append( + {"id": "drums-3", "name": "Broken", "type": "drums", + "drum_tab": "drum_tab_broken.json"}) + pak = _write_pak(tmp_path, manifest, { + "drum_tab.json": _tab("Drums"), + "drum_tab_drums-2.json": _tab("Drums (Live)"), + "drum_tab_broken.json": "not json {{{", + }) + loaded = _load(pak, tmp_path) + assert [p["id"] for p in loaded.drum_parts] == ["drums", "drums-2"] + + +def test_a_traversal_part_path_is_skipped(tmp_path: Path): + manifest = _two_part_manifest() + manifest["arrangements"][2]["drum_tab"] = "../outside.json" + (tmp_path / "outside.json").write_text(json.dumps(_tab("Evil"))) + pak = _write_pak(tmp_path, manifest, {"drum_tab.json": _tab("Drums")}) + loaded = _load(pak, tmp_path) + assert [p["id"] for p in loaded.drum_parts] == ["drums"] + + +def test_duplicate_pointer_rels_load_once(tmp_path: Path): + manifest = _two_part_manifest() + manifest["arrangements"].append( + {"id": "drums-dup", "name": "Dup", "type": "drums", + "drum_tab": "drum_tab_drums-2.json"}) + pak = _write_pak(tmp_path, manifest, { + "drum_tab.json": _tab("Drums"), + "drum_tab_drums-2.json": _tab("Drums (Live)"), + }) + loaded = _load(pak, tmp_path) + assert [p["id"] for p in loaded.drum_parts] == ["drums", "drums-2"] + + +def test_duplicate_part_ids_are_made_unique(tmp_path: Path): + manifest = _two_part_manifest() + manifest["arrangements"][2]["id"] = "drums" + manifest["arrangements"].append( + {"id": "drums-2", "name": "Aux", "type": "drums", + "drum_tab": "drum_tab_aux.json"}) + pak = _write_pak(tmp_path, manifest, { + "drum_tab.json": _tab("Drums"), + "drum_tab_drums-2.json": _tab("Drums (Live)"), + "drum_tab_aux.json": _tab("Aux"), + }) + loaded = _load(pak, tmp_path) + assert [p["id"] for p in loaded.drum_parts] == ["drums", "drums-2", "drums-3"] + + +def test_drum_pointer_with_wrong_type_logs_warning(tmp_path: Path, caplog): + pak = _write_pak(tmp_path, { + "arrangements": [ + {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}, + {"id": "typo", "type": "druns", "drum_tab": "drum_tab_typo.json"}, + ], + }, {"drum_tab_typo.json": _tab("Typo")}) + # feedBack sets propagate=False, so pytest's root capture sees nothing from + # it — attach caplog's handler to the feedBack logger and pin WARNING + # regardless of ambient level (a sibling test can leak ERROR onto this tree). + lg = logging.getLogger("feedBack") + orig_level = lg.level + lg.addHandler(caplog.handler) + lg.setLevel(logging.WARNING) + try: + loaded = _load(pak, tmp_path) + finally: + lg.removeHandler(caplog.handler) + lg.setLevel(orig_level) + assert loaded.drum_parts is None + assert "has drum_tab" in caplog.text and "type='druns'" in caplog.text + + +# ── Drum-only pack with parts ──────────────────────────────────────────────── + +def test_drum_only_pack_with_pointer_entries_still_synthesizes_placeholder(tmp_path: Path): + # No pitched arrangements at all, drums via pointer entries only: the + # placeholder "Drums" arrangement must still appear so the highway WS + # proceeds and the tab reaches the drum highway. + pak = _write_pak(tmp_path, { + "arrangements": [ + {"id": "kit", "name": "Kit", "type": "drums", + "drum_tab": "drum_tab_kit.json"}, + ], + }, {"drum_tab_kit.json": _tab("Kit", [{"t": 5.0, "p": "kick", "v": 100}])}) + # Remove the Lead arrangement _write_pak added to the manifest. + manifest_path = pak / "manifest.yaml" + manifest = yaml.safe_load(manifest_path.read_text()) + manifest["arrangements"] = [e for e in manifest["arrangements"] if e.get("id") != "lead"] + manifest.pop("duration", None) + manifest_path.write_text(yaml.safe_dump(manifest, sort_keys=False)) + loaded = _load(pak, tmp_path) + assert [a.name for a in loaded.song.arrangements] == ["Drums"] + assert loaded.drum_parts is not None and loaded.drum_parts[0]["id"] == "kit" + # Song length derived from the last hit (the drum-only path's rule). + assert loaded.song.song_length > 5.0 diff --git a/tests/test_tuning_provider_isolation.py b/tests/test_tuning_provider_isolation.py index ff8149f..31ff5b9 100644 --- a/tests/test_tuning_provider_isolation.py +++ b/tests/test_tuning_provider_isolation.py @@ -60,12 +60,14 @@ def test_the_failure_is_actually_logged(registry, caplog): # capture_logger() context manager for this, but it is not importable from here: # pyproject pins pythonpath to [".", "lib"], so `tests` is not a package.) lg = logging.getLogger("feedBack") + orig_level = lg.level lg.addHandler(caplog.handler) lg.setLevel(logging.ERROR) try: registry.get_merged() finally: lg.removeHandler(caplog.handler) + lg.setLevel(orig_level) # restore, or ERROR leaks onto the feedBack tree assert any("bad-plugin" in r.getMessage() for r in caplog.records), ( "the raising provider was never named in the logs"