From 9075939668c1ef20e30e3b5ff03fafdb17b0339c Mon Sep 17 00:00:00 2001 From: gionnibgud Date: Thu, 23 Jul 2026 08:45:40 +0200 Subject: [PATCH] Load the pack's rig library from the manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit feedpak 1.18.0 lets a chart declare what a MIDI part should sound like by binding a rig id, but core had nothing to bind to: `rigs`, `base_rig` and `drum_tones` appeared nowhere in lib/, server.py or static/. The preceding commit carries the reference onto the wire; this adds the library it references. Read the manifest `rigs:` key into a new `LoadedSloppak.rigs`, alongside the other side-files rather than on Song — every side-file (drum_tab, song_timeline, keys, notation) hangs off the load result, and rigs is pack-level, not per-arrangement. Same permissive posture as its neighbours: missing, unreadable, malformed or traversing disables rigs with a warning and never fails the pack, which §7.9 requires outright. Rig objects pass through VERBATIM. §7.9 obliges a Reader to preserve unknown role/engine/kind values and `ext` namespaces, so validating block structure here would be wrong as well as premature — realization selection and the `intent.gm` floor belong to whatever voices the part. The only entries dropped are ones unreachable by construction: a rig is addressable solely by `id`, so a non-dict entry or one without a usable string id can never be referenced. Ids are stripped to match the reference side, and a duplicate id resolves first-wins with a warning, since ambiguity there would surface as the wrong sound rather than an error. Signed-off-by: gionnibgud --- lib/sloppak.py | 88 ++++++++++++++ tests/test_sloppak_rigs_load.py | 207 ++++++++++++++++++++++++++++++++ 2 files changed, 295 insertions(+) create mode 100644 tests/test_sloppak_rigs_load.py diff --git a/lib/sloppak.py b/lib/sloppak.py index 1081faa..cacd864 100644 --- a/lib/sloppak.py +++ b/lib/sloppak.py @@ -698,6 +698,14 @@ class LoadedSloppak: # absent / unreadable / malformed. Streamed over the highway WS as a # `keys` message; consumers (renderers, plugins) read it from there. keys: dict | None = None + # Parsed `rigs.json` payload (manifest `rigs:` key, spec §7.9) — the pack's + # library of engine-agnostic signal chains: effect chains and, since + # feedpak 1.18.0, MIDI-voiced sound sources. Arrangements bind rigs to time + # by referencing a rig `id` from `tones.base_rig` / `tones.changes[].rig` + # (§6.9), which `lib/tones.py` carries onto the wire. None when absent / + # unreadable / malformed. Rig objects are kept verbatim — this loader does + # not select realizations or apply the `intent.gm` floor. + rigs: dict | None = None # Sanitized song-level tempo + time-signature maps from `song_timeline.json` # (feedpak 1.2.0). `tempos`: [{time, bpm}]; `time_signatures`: [{time, ts}]. # None when absent/empty. Streamed over the highway WS (`tempos` / @@ -776,6 +784,77 @@ def _load_drum_tab_file(source_dir: Path, rel: str, label: str) -> dict | None: return raw +def _load_rigs_file(source_dir: Path, rel: str) -> dict | None: + """Load the pack's rig library (manifest `rigs:` key, spec §7.9). + + Returns `{"version": int, "rigs": [...]}` or None. Same permissive posture + as every other side-file: missing / unreadable / malformed -> None, never + fatal — spec §7.9 is explicit that a rig library a Reader can't use MUST NOT + fail the pack. + + Rig objects are kept **verbatim**. Only entries that could never be + addressed are dropped — a rig is reachable solely by `id` (from + `tones.base_rig` / `changes[].rig`), so a non-dict entry or one without a + usable string id is unreferenceable by construction. Everything else, + including unknown `role` / `engine` / `kind` values and `ext` namespaces, + passes through untouched, because this loader does not interpret rigs: + realization selection and the `intent.gm` fallback belong to whatever + voices the part. + """ + try: + r_path = (source_dir / rel).resolve() + r_path.relative_to(source_dir.resolve()) + except ValueError: + log.warning("sloppak: rigs path %r escapes source_dir — skipped", rel) + return None + except OSError as e: + log.warning("sloppak: rigs path resolution failed (%s) — skipped", e) + return None + if not r_path.exists(): + return None + try: + raw = load_json(r_path) + except Exception as e: + log.warning("sloppak: failed to parse rigs %r: %s", rel, e) + return None + if not isinstance(raw, dict): + log.warning("sloppak: rigs %r ignored — expected dict, got %s", + rel, type(raw).__name__) + return None + if not isinstance(raw.get("rigs"), list): + log.warning("sloppak: rigs %r ignored — 'rigs' must be a list", rel) + return None + + clean_rigs: list[dict] = [] + seen: set[str] = set() + for rig in raw["rigs"]: + if not isinstance(rig, dict): + continue + rid = rig.get("id") + if not isinstance(rid, str) or not rid.strip(): + continue + # Normalize the library side of the lookup the same way the reference + # side is normalized in lib/tones.py — otherwise a pack with padded ids + # fails to resolve against a stripped `base_rig` / `rig`. + rid = rid.strip() + # A duplicate id makes `tones.base_rig` ambiguous, which would surface + # as the wrong sound rather than an error. First wins, loudly. + if rid in seen: + log.warning("sloppak: rigs %r has duplicate rig id %r — later one ignored", + rel, rid) + continue + seen.add(rid) + clean_rigs.append({**rig, "id": rid}) + + # int only — a float version (incl. NaN/Inf, which json.loads accepts) + # would raise on int(); default rather than abort an optional side-file. + _ver = raw.get("version") + return { + "version": _ver if isinstance(_ver, int) and not isinstance(_ver, bool) else 1, + "rigs": clean_rigs, + } + + def _resolve_drum_parts( source_dir: Path, drum_tab_rel: object, @@ -1325,6 +1404,14 @@ def load_song( "events": clean_events, } + # Optional rigs.json — the pack's rig library (manifest `rigs:` key, + # spec §7.9). Loaded here so the highway WS can hand it to whatever voices + # the part; the bindings that reference it ride the arrangement's `tones`. + rigs_data: dict | None = None + rigs_rel = manifest.get("rigs") + if isinstance(rigs_rel, str) and rigs_rel: + rigs_data = _load_rigs_file(source_dir, rigs_rel) + _fpv = manifest.get("feedpak_version") # The pack's full mix. Normally the RESERVED `full` stem partitioned out # above (spec §5.3) — no path work needed, it was validated with the other @@ -1355,6 +1442,7 @@ def load_song( tempos=tempos_data, time_signatures=time_sigs_data, keys=keys_data, + rigs=rigs_data, notation_by_id=notation_by_id_data, arrangement_ids=arrangement_ids_acc, full_mix=full_mix_data, diff --git a/tests/test_sloppak_rigs_load.py b/tests/test_sloppak_rigs_load.py new file mode 100644 index 0000000..4ce5779 --- /dev/null +++ b/tests/test_sloppak_rigs_load.py @@ -0,0 +1,207 @@ +"""End-to-end test for the sloppak loader recognising a `rigs:` manifest key +(rigs.json — the pack-level library of engine-agnostic rigs, spec §7.9) and +surfacing the payload on the LoadedSloppak. + +The governing posture: rig objects pass through VERBATIM. This loader does not +select realizations or apply the `intent.gm` floor — it only makes the library +addressable by `id`, which is what `tones.base_rig` / `tones.changes[].rig` +reference.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import yaml + +import sloppak as sloppak_mod + + +def _write_dir_sloppak(root: Path, manifest_extras: dict, rigs_payload) -> Path: + """Minimal directory-form sloppak; writes rigs.json when a payload is given. + + Unique filename per test (tmp_path leaf) so the module-level + resolve_source_dir cache isn't poisoned across tests.""" + 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)) + + if rigs_payload is not None: + (pak / "rigs.json").write_text(json.dumps(rigs_payload)) + 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) + + +# ── Happy path ─────────────────────────────────────────────────────────────── + +def test_load_song_attaches_rigs_when_manifest_opts_in(tmp_path: Path): + """A source rig (spec §7.9 1.18.0) survives the load intact — including the + `soundfont` realization and the `intent.gm` floor a consumer needs to voice + the part.""" + payload = { + "version": 1, + "rigs": [ + { + "id": "grand-piano", + "name": "Grand Piano", + "instrument": "keys", + "blocks": [ + { + "role": "source", + "name": "Concert Grand", + "intent": {"kind": "instrument", "gm": {"program": 0}}, + "realizations": [ + {"engine": "soundfont", "format": "sf2", + "ref": "sounds/grand.sf2", "bank": 0, "program": 0}, + ], + }, + ], + }, + ], + } + pak = _write_dir_sloppak(tmp_path, {"rigs": "rigs.json"}, payload) + loaded = _load(pak, tmp_path) + assert loaded.rigs is not None + assert loaded.rigs["version"] == 1 + assert loaded.rigs["rigs"] == payload["rigs"] + + +def test_load_song_rigs_absent_without_manifest_key(tmp_path: Path): + """The file alone must not opt a pack in — the manifest is the opt-in + (spec §9.1, "manifest opt-in, file off to the side").""" + pak = _write_dir_sloppak(tmp_path, {}, {"version": 1, "rigs": []}) + assert _load(pak, tmp_path).rigs is None + + +# ── Verbatim passthrough ───────────────────────────────────────────────────── + +def test_load_song_preserves_unknown_rig_content(tmp_path: Path): + """Unknown `role` / `engine` / `kind` values and `ext` namespaces MUST + survive (spec §7.9) — core does not interpret rigs, so it must not prune + what a newer writer or a plugin put there.""" + payload = { + "version": 2, + "rigs": [ + { + "id": "future-rig", + "blocks": [ + {"role": "quantum-flux", "intent": {"kind": "not-yet-invented"}, + "realizations": [{"engine": "some-future-engine", "ref": "x.bin"}], + "ext": {"vendor.custom": {"anything": [1, 2, 3]}}}, + ], + "graph": {"nodes": ["input", "output"], "edges": [["input", "output"]]}, + "ext": {"vendor.rig": "kept"}, + }, + ], + } + pak = _write_dir_sloppak(tmp_path, {"rigs": "rigs.json"}, payload) + loaded = _load(pak, tmp_path) + assert loaded.rigs["version"] == 2 + assert loaded.rigs["rigs"] == payload["rigs"] + + +# ── Addressability ─────────────────────────────────────────────────────────── + +def test_load_song_drops_unaddressable_rigs_and_normalizes_ids(tmp_path: Path): + """A rig is reachable only by `id`, so entries without a usable one are + unreferenceable by construction. Ids are stripped to match the reference + side, which lib/tones.py strips before it reaches the wire.""" + payload = { + "rigs": [ + "not-a-dict", + {"name": "no id at all"}, + {"id": "", "name": "blank id"}, + {"id": " ", "name": "whitespace id"}, + {"id": 7, "name": "non-string id"}, + {"id": " padded-rig ", "name": "Padded"}, + {"id": "plain-rig", "name": "Plain"}, + ], + } + pak = _write_dir_sloppak(tmp_path, {"rigs": "rigs.json"}, payload) + loaded = _load(pak, tmp_path) + assert [r["id"] for r in loaded.rigs["rigs"]] == ["padded-rig", "plain-rig"] + # Everything except the normalized id is untouched. + assert loaded.rigs["rigs"][0]["name"] == "Padded" + # `version` defaults when the file omits it. + assert loaded.rigs["version"] == 1 + + +def test_load_song_first_rig_wins_on_duplicate_id(tmp_path: Path): + """A duplicate id makes `tones.base_rig` ambiguous, which would surface as + the wrong sound rather than an error.""" + payload = { + "rigs": [ + {"id": "dupe", "name": "First"}, + {"id": "dupe", "name": "Second"}, + {"id": " dupe ", "name": "Third, padded into a collision"}, + ], + } + pak = _write_dir_sloppak(tmp_path, {"rigs": "rigs.json"}, payload) + loaded = _load(pak, tmp_path) + assert len(loaded.rigs["rigs"]) == 1 + assert loaded.rigs["rigs"][0]["name"] == "First" + + +# ── Permissive posture (spec §7.9: never fail the pack) ────────────────────── + +def test_load_song_survives_malformed_rigs(tmp_path: Path): + """Malformed / missing / traversing rig libraries disable rigs, never the + pack — the song itself must still load.""" + cases = [ + {"version": 1, "rigs": "not-a-list"}, # wrong `rigs` type + ["top-level-not-a-dict"], # wrong document type + {"version": 1}, # no `rigs` key at all + ] + for i, payload in enumerate(cases): + sub = tmp_path / f"case{i}" + sub.mkdir() + pak = _write_dir_sloppak(sub, {"rigs": "rigs.json"}, payload) + loaded = _load(pak, sub) + assert loaded.rigs is None, f"case {i} should disable rigs" + assert loaded.song is not None, f"case {i} must not fail the pack" + + +def test_load_song_survives_unparseable_rigs(tmp_path: Path): + pak = _write_dir_sloppak(tmp_path, {"rigs": "rigs.json"}, None) + (pak / "rigs.json").write_text("{ not json at all ") + loaded = _load(pak, tmp_path) + assert loaded.rigs is None + assert loaded.song is not None + + +def test_load_song_survives_missing_rigs_file(tmp_path: Path): + pak = _write_dir_sloppak(tmp_path, {"rigs": "rigs.json"}, None) + loaded = _load(pak, tmp_path) + assert loaded.rigs is None + assert loaded.song is not None + + +def test_load_song_rejects_traversing_rigs_path(tmp_path: Path): + """A crafted manifest must not read outside the pack.""" + (tmp_path / "outside.json").write_text(json.dumps({"rigs": [{"id": "leaked"}]})) + pak = _write_dir_sloppak(tmp_path, {"rigs": "../outside.json"}, None) + loaded = _load(pak, tmp_path) + assert loaded.rigs is None + assert loaded.song is not None