feat(gp2rs): emit lyrics + vocal_pitch sidecars from GP vocal tracks

GP files with a vocal track previously produced only the legacy vocals
arrangement XML; the melody sidecar converter existed but was never
wired, so imported paks had no karaoke-ready pitch data.

- convert_file() now calls _emit_vocal_sidecars() after writing the
  vocals XML (best-effort: a sidecar failure never breaks conversion,
  matching the notation-sidecar posture)
- lyrics sidecar is projected from the just-built vocals XML so timing
  stays in lockstep; the XML lyric convention's trailing '+' (join) is
  mapped to the spec 7.1 '-' (join) - a passthrough would have turned
  every joined word into a line break
- convert_vocal_track_to_pitch_sidecar() gains require_lyric=False for
  lyric-less melody tracks (pitch sidecar only)
- attach_vocal_sidecars_to_sloppak() added as the assembly-step helper
  (companion to attach_notation_to_sloppak): writes the sidecars into a
  directory-form pak and sets manifest lyrics/lyrics_source/vocal_pitch
  with no-clobber guards
- gp2rs_gpx registered in the spec-conformance READERS list

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>

CHANGELOG.md

@@ -8,20 +8,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
-- **Core reader for source rigs (feedpak 1.18.0).** A pack can declare what a
-  MIDI part should sound like by binding a rig; core now reads that binding and
-  hands it to the client instead of dropping it. Three parts: the
-  `tone_changes` WS message carries the pack's rig bindings (`base_rig`, and
-  `rig` per change) alongside the tone names it already sent; the manifest
-  `rigs:` key loads the pack's rig library (`rigs.json`, spec §7.9) verbatim;
-  and the binding precedence is resolved per spec §5.1/§5.2 — a manifest
-  arrangement entry's `tones` replaces the arrangement JSON's **wholesale**
-  (no field-level merge), while top-level `drum_tones` binds the primary drum
-  part as the fallback a `type: drums` entry's own `tones` outranks. Core
-  deliberately stops there: it does not select a realization or apply the
-  `intent.gm` floor, which belong to whatever actually voices the part. Packs
-  that bind no rig produce a byte-identical `tone_changes` payload, so existing
-  consumers are unaffected.
 - **Opt-in career venue packs (#122)** — higher-tier venue crowd media
   (`club`, `arena`) is no longer bundled; the app downloads each pack on demand
   from its release when you reach the venue (sha256-verified), keeping the
@@ -313,17 +299,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry).
 
 ### Fixed
-- **Count-in follows the song's meter and its pickup measure.** The count-in
-  (loop wrap, section practice, and the "Countdown before song" setting) always
-  clicked exactly four beats, so a 3/4 song was counted in 4/4, and a song
-  opening with a pickup (anacrusis) had the pickup enter where the downbeat
-  belonged — putting the player a beat ahead for the whole song. The bar length
-  now comes from the `song_timeline` beats already on the highway
-  (`measure >= 0` marks downbeats; no new plumbing, since the `time_signatures`
-  map is streamed to plugins rather than stored in the frontend), and a first
-  bar shorter than that meter shortens the count by its length: a 1-beat pickup
-  in 4/4 counts "1 2 3" and the music enters on 4. Songs without beats — pre-chart,
-  minigames, synthetic highways — still get four.
 - **GP8 asset resolution honours the directory the registry named.**
   `<EmbeddedFilePath>` is matched on filename stem so a format variant of the
   same recording can win (an `.ogg` beside the declared `.mp3` is copied out

CLAUDE.md

@@ -690,7 +690,7 @@ The highway WebSocket at `/ws/highway/{filename}?arrangement={index}` streams th
 | `anchors` | `{ type, data: [{ time, fret, width }] }` | Fret zoom anchors |
 | `chord_templates` | `{ type, data: [{ name, frets: [6] }] }` | Named chord shapes |
 | `lyrics` | `{ type, data: [{ w, t, d }], source }` | Syllables: `w`=word, `t`=time, `d`=duration. `-` joins to previous, `+` = line break. `source` is one of `"xml"`, `"whisperx"`, `"user"` — UI can use it to render an "auto-transcribed" badge for `whisperx`. Sloppaks always include `source` (legacy sloppaks without a `lyrics_source` manifest key default to `"xml"` at load time). Loose folders set it based on which extractor matched. Absent only when no lyrics fired the message at all |
-| `tone_changes` | `{ type: 'tone_changes', base, base_rig?, data: [{ t, name, rig? }] }` | Optional — tone change events relative to the arrangement base tone; only sent if tones were found. Note the time key is **`t`**, not `time` (both the sloppak path and the legacy XML path emit `t`). `base_rig` and each entry's `rig` are the pack's **rig bindings** — ids into [`rigs.json`](https://github.com/got-feedback/feedpak-spec/blob/main/spec/feedpak-v1.md#79-rigsjson) (feedpak §6.9/§7.9), carried through verbatim and **not** resolved by core: selecting a realization and applying the `intent.gm` floor belong to whatever voices the part. Both are **omitted entirely** when the chart binds no rig, so consumers predating the rig model see the payload they always did. |
+| `tone_changes` | `{ type: 'tone_changes', base, data: [{ time, name }] }` | Optional — tone change events relative to the arrangement base tone; only sent if tones were found |
 | `notes` | `{ type, data: [{ t, s, f, sus, ho, po, sl, bn, ... }] }` | Single notes |
 | `chords` | `{ type, data: [{ t, notes: [{ s, f, sus, ... }] }] }` | Chord events |
 | `phrases` | `{ type, data: [{ start_time, end_time, max_difficulty, levels: [{ difficulty, notes, chords, anchors, handshapes }] }], total }` | Optional — per-phrase difficulty ladder for master-difficulty slider (feedBack#48). Only sent when the source chart carries multi-level phrase data (phrase-aware sloppak). Sent in chunks (`data` is a batch, `total` is the full count across messages) to avoid multi-MB single frames. Absent for GP imports and legacy sloppak; consumers must treat missing message as "single fixed difficulty — slider disabled". |

lib/gp2rs_gpx.py

@@ -12,6 +12,7 @@ Both are called transparently by gp2rs.py when the file extension is .gpx.
 Do not call this module directly; use gp2rs.list_tracks / gp2rs.convert_file.
 """
 
+import json
 import logging
 import re
 import struct
@@ -1186,6 +1187,7 @@ def convert_vocal_track_to_pitch_sidecar(
     *,
     tempo_bpm: float = 120.0,
     audio_offset: float = 0.0,
+    require_lyric: bool = True,
 ) -> dict:
     """
     Extract per-syllable pitch from a GPX vocal track as a vocal_pitch.json dict.
@@ -1196,14 +1198,17 @@ def convert_vocal_track_to_pitch_sidecar(
 
         {"version": 1, "notes": [{"t": float, "d": float, "midi": int}, ...]}
 
-    This is complementary to convert_vocal_track() which produces arrangement XML.
-    NOTE: nothing in this module calls this helper yet — convert_file() does not
-    invoke it, so no vocal_pitch.json is emitted automatically. A caller wanting
-    the pitch ribbon must call this itself and persist the returned dict (e.g.
-    write it as vocal_pitch.json into the sloppak). When wiring vocals into a
-    sloppak, call both:
-        - convert_vocal_track() → vocals arrangement XML (karaoke highway)
-        - convert_vocal_track_to_pitch_sidecar() → vocal_pitch.json (pitch ribbon)
+    This is complementary to convert_vocal_track() which produces arrangement
+    XML. convert_file() calls both for every vocal track and writes the result
+    as a ``<stem>.vocal_pitch.json`` sidecar next to the vocals XML (see
+    _emit_vocal_sidecars); the sloppak assembly step then attaches it via
+    attach_vocal_sidecars_to_sloppak.
+
+    ``require_lyric`` (default True) keeps the feedpak-spec §7.2 alignment:
+    only beats carrying a lyric emit a note, so the pitch ribbon mirrors
+    lyrics.json token-for-token. Pass False for a lyric-less vocal track
+    (authored melody, no lyric text) to emit every pitched beat instead —
+    there are no lyric tokens to stay aligned with.
 
     Pitch source is the tab author's authored notes (exact), not AI audio
     analysis — so this is more accurate than pYIN/CREPE for well-authored tabs.
@@ -1258,13 +1263,15 @@ def convert_vocal_track_to_pitch_sidecar(
                         # Only emit notes that have a lyric — unvoiced beats
                         # (rests, instrumental fills) are excluded so the
                         # pitch ribbon stays aligned with lyric tokens.
+                        # (Relaxed via require_lyric=False for lyric-less
+                        # vocal tracks, where every pitched beat counts.)
                         lyric_el = beat_el.find('Lyrics')
                         has_lyric = (
                             lyric_el is not None
                             and lyric_el.find('Line') is not None
                             and (lyric_el.find('Line').text or '').strip()
                         )
-                        if not has_lyric:
+                        if require_lyric and not has_lyric:
                             voice_time += dur
                             continue
 
@@ -1682,6 +1689,26 @@ def convert_file(
                 raise ValueError(f"unsafe output filename from track name: {track['name']!r}")
             filepath.write_text(xml_str, encoding="utf-8")
             output_files.append(str(filepath))
+
+            # Vocal tracks additionally get karaoke sidecars next to the XML
+            # (`<stem>.lyrics.json` + `<stem>.vocal_pitch.json`, feedpak spec
+            # §7.1/§7.2) so the sloppak assembly step can attach the `lyrics` /
+            # `vocal_pitch` manifest keys without re-walking the GP file —
+            # same pattern as the keys notation sidecar below. Best-effort: a
+            # sidecar bug must never break the vocals XML conversion itself.
+            try:
+                _emit_vocal_sidecars(
+                    filepath, xml_str,
+                    root, track, raw_idx,
+                    masterbars, bars_by_id, voices_dict, beats_dict,
+                    notes_dict, rhythms_dict,
+                    tempo_bpm=tempo_bpm, audio_offset=audio_offset,
+                )
+            except Exception:
+                _log.exception(
+                    "gp2rs_gpx: vocal sidecar emission failed for track %r "
+                    "— vocals XML is unaffected", track['name'],
+                )
             continue
 
         # Iterate all masterbars and collect notes for this track
@@ -2403,6 +2430,195 @@ def _build_vocals_xml(
     return dom.toprettyxml(indent='  ', encoding=None)
 
 
+# ---------------------------------------------------------------------------
+# Vocal karaoke sidecars + manifest wiring (feedpak spec §7.1/§7.2)
+# Mirrors the gp2notation sidecar pattern: convert_file writes the payloads
+# next to the vocals XML (arrangement ids / the pak don't exist yet at convert
+# time), and the sloppak assembly step moves them into the pak root + manifest
+# via attach_vocal_sidecars_to_sloppak.
+# ---------------------------------------------------------------------------
+
+def lyrics_sidecar_path(xml_path: str | Path) -> Path:
+    """``Voice_Vocals.xml`` → ``Voice_Vocals.lyrics.json`` (next to the XML)."""
+    p = Path(xml_path)
+    return p.with_name(p.stem + ".lyrics.json")
+
+
+def vocal_pitch_sidecar_path(xml_path: str | Path) -> Path:
+    """``Voice_Vocals.xml`` → ``Voice_Vocals.vocal_pitch.json`` (next to the XML)."""
+    p = Path(xml_path)
+    return p.with_name(p.stem + ".vocal_pitch.json")
+
+
+def _vocals_xml_to_lyrics(xml_str: str) -> list[dict]:
+    """Project a ``<vocals>`` arrangement XML into the flat lyrics.json shape
+    (feedpak spec §7.1): ``[{"t": float, "d": float, "w": str}, ...]``.
+
+    Deriving from the XML (rather than re-walking the GP tree) guarantees the
+    two stay in lockstep — same tie extension, same rounding, same beats.
+
+    Suffix conversion: the XML lyric convention and feedpak disagree on ``+``.
+    In the vocals XML a trailing ``+`` means "connect to next token" (a join),
+    while feedpak ``+`` marks the last syllable of a LINE — so a pass-through
+    would turn every joined syllable into a line break. Joins map to feedpak's
+    trailing ``-`` instead; a trailing ``-`` already means the same thing in
+    both. Line-end ``+`` markers are never emitted: GP stores lyrics per beat
+    with no line structure, so there is nothing to derive them from.
+    """
+    try:
+        root = ET.fromstring(xml_str)
+    except ET.ParseError:
+        return []
+    lyrics: list[dict] = []
+    for v in root.iter('vocal'):
+        w = (v.get('lyric') or '').strip()
+        if w.endswith('+'):
+            w = w[:-1] + '-'
+        # A bare joiner token isn't a syllable (spec: suffixes ride on real
+        # syllables, never standalone entries) — skip it.
+        if not w or w in ('-', '+'):
+            continue
+        try:
+            t = float(v.get('time', ''))
+            d = float(v.get('length', ''))
+        except (TypeError, ValueError):
+            continue
+        lyrics.append({'t': round(t, 3), 'd': round(d, 3), 'w': w})
+    return lyrics
+
+
+def _emit_vocal_sidecars(
+    xml_path: Path,
+    xml_str: str,
+    root: ET.Element,
+    track: dict,
+    raw_idx: int,
+    masterbars: list,
+    bars_by_id: dict,
+    voices_dict: dict,
+    beats_dict: dict,
+    notes_dict: dict,
+    rhythms_dict: dict,
+    *,
+    tempo_bpm: float = 120.0,
+    audio_offset: float = 0.0,
+) -> list[Path]:
+    """Write the karaoke sidecars for one converted vocal track.
+
+    ``<stem>.lyrics.json`` — only when the track actually carries lyric text
+    (derived from the vocals XML just built, so timings match exactly).
+    ``<stem>.vocal_pitch.json`` — whenever the track has pitched beats. With
+    lyrics present the notes stay lyric-aligned (spec §7.2: one entry per
+    syllable); for a lyric-less melody track the lyric gate is dropped so the
+    authored pitch still ships.
+
+    Returns the sidecar paths written (possibly empty).
+    """
+    written: list[Path] = []
+
+    lyrics = _vocals_xml_to_lyrics(xml_str)
+    if lyrics:
+        side = lyrics_sidecar_path(xml_path)
+        side.write_text(json.dumps(lyrics, separators=(",", ":")),
+                        encoding="utf-8")
+        written.append(side)
+
+    pitch = convert_vocal_track_to_pitch_sidecar(
+        root, track, raw_idx,
+        masterbars, bars_by_id, voices_dict, beats_dict,
+        notes_dict, rhythms_dict,
+        tempo_bpm=tempo_bpm, audio_offset=audio_offset,
+        require_lyric=bool(lyrics),
+    )
+    if pitch.get('notes'):
+        side = vocal_pitch_sidecar_path(xml_path)
+        side.write_text(json.dumps(pitch, separators=(",", ":")),
+                        encoding="utf-8")
+        written.append(side)
+
+    return written
+
+
+def attach_vocal_sidecars_to_sloppak(
+    sloppak_dir: str | Path,
+    *,
+    lyrics: list | None = None,
+    vocal_pitch: dict | None = None,
+    lyrics_source: str = "authored",
+) -> list[Path]:
+    """Write ``lyrics.json`` / ``vocal_pitch.json`` into a directory-form
+    sloppak and point the top-level manifest ``lyrics`` / ``lyrics_source`` /
+    ``vocal_pitch`` keys at them (feedpak spec §7.1/§7.2).
+
+    Vocal companion to gp2notation.attach_notation_to_sloppak, with the same
+    manifest round-trip caveat (PyYAML ``safe_load`` + ``safe_dump`` — key
+    order survives, comments don't). GP-derived payloads are ``authored``
+    provenance, so no ``lyric_transcription`` / ``pitch_extraction`` blocks
+    are written (the spec reserves those for automated engines).
+
+    Never clobbers: a payload whose manifest key is already set (or whose
+    target file already exists) is skipped, so a pak that already carries
+    lyrics/pitch — hand-edited or machine-extracted — is left alone.
+    Raises ``ValueError`` on a malformed payload, an unknown
+    ``lyrics_source``, or a manifest that isn't a mapping. Returns the paths
+    actually written.
+    """
+    import yaml
+
+    if lyrics_source not in ("authored", "transcribed", "user"):
+        raise ValueError(
+            f"lyrics_source must be authored/transcribed/user, got {lyrics_source!r}")
+    if lyrics is not None and not (
+        isinstance(lyrics, list) and all(
+            isinstance(e, dict)
+            and isinstance(e.get('w'), str)
+            and isinstance(e.get('t'), (int, float))
+            and isinstance(e.get('d'), (int, float))
+            for e in lyrics
+        )
+    ):
+        raise ValueError("lyrics must be a list of {t, d, w} syllable dicts")
+    if vocal_pitch is not None and not (
+        isinstance(vocal_pitch, dict)
+        and isinstance(vocal_pitch.get('notes'), list)
+    ):
+        raise ValueError("vocal_pitch must be a dict with a `notes` list")
+
+    pak = Path(sloppak_dir)
+    manifest_path = pak / "manifest.yaml"
+    manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8"))
+    if not isinstance(manifest, dict):
+        raise ValueError(f"{manifest_path} is not a mapping")
+
+    written: list[Path] = []
+
+    if lyrics and not manifest.get("lyrics") and not (pak / "lyrics.json").exists():
+        (pak / "lyrics.json").write_text(
+            json.dumps(lyrics, separators=(",", ":")), encoding="utf-8")
+        manifest["lyrics"] = "lyrics.json"
+        manifest["lyrics_source"] = lyrics_source
+        written.append(pak / "lyrics.json")
+
+    if (vocal_pitch and vocal_pitch.get("notes")
+            and not manifest.get("vocal_pitch")
+            and not (pak / "vocal_pitch.json").exists()):
+        (pak / "vocal_pitch.json").write_text(
+            json.dumps(vocal_pitch, separators=(",", ":")), encoding="utf-8")
+        manifest["vocal_pitch"] = "vocal_pitch.json"
+        written.append(pak / "vocal_pitch.json")
+
+    if written:
+        # Stamp the format version while we're rewriting the manifest (spec
+        # §4), without downgrading an existing declared version.
+        from sloppak import FEEDPAK_VERSION
+        manifest.setdefault("feedpak_version", FEEDPAK_VERSION)
+        manifest_path.write_text(
+            yaml.safe_dump(manifest, sort_keys=False, allow_unicode=True),
+            encoding="utf-8",
+        )
+    return written
+
+
 def _gpx_tuning(track: dict) -> list[int]:
     """Compute RS tuning offsets (semitones from standard) from GPX string pitches."""
     from gp2rs import STANDARD_TUNING_GUITAR, STANDARD_TUNING_BASS

lib/routers/ws_highway.py

@@ -774,29 +774,20 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1,
         # (Arrangement.tones, populated by the converter), so read it straight
         # off `arr` rather than walking for XML that doesn't exist.
         if is_slop:
-            # `sloppak_tone_changes` builds the (base, base_rig, sorted
-            # changes) triple from `Arrangement.tones`, skipping non-string
-            # names, non-finite/non-numeric times, and unusable rig ids —
-            # unit-tested in test_tones.py.
+            # `sloppak_tone_changes` builds the (base, sorted changes) pair
+            # from `Arrangement.tones`, skipping non-string names and
+            # non-finite/non-numeric times — unit-tested in test_tones.py.
             from tones import sloppak_tone_changes
-            base_name, base_rig, tone_changes = sloppak_tone_changes(
-                getattr(arr, "tones", None)
-            )
+            base_name, tone_changes = sloppak_tone_changes(getattr(arr, "tones", None))
             # Send when there's a base tone OR timed changes — a single-tone
             # arrangement has a base but no switches, and the highway should
             # still be able to show the initial tone.
             if tone_changes or base_name:
-                payload = {
+                await websocket.send_json({
                     "type": "tone_changes",
                     "base": base_name,
                     "data": tone_changes,
-                }
-                # `base_rig` is additive (feedpak-spec §6.9) — omitted entirely
-                # when the chart binds no rig, so consumers that predate the rig
-                # model see the exact payload they always did.
-                if base_rig:
-                    payload["base_rig"] = base_rig
-                await websocket.send_json(payload)
+                })
         else:
             xml_paths = sorted(_xml_walk("*.xml"))
 

lib/sloppak.py

@@ -121,41 +121,6 @@ def partition_stems(stems: list[dict]) -> tuple[dict | None, list[dict]]:
     return full, [s for s in stems if str(s.get("id", "")) != FULL_MIX_STEM_ID]
 
 
-def _resolve_pack_path(source_dir: Path, rel: str, label: str) -> Path | None:
-    """Resolve a manifest-relative path, contained inside the pack. None if not.
-
-    Every manifest key that names a file routes through here. A crafted manifest
-    must not read outside the sloppak directory via path traversal
-    (e.g. `../../etc`), and a symlink loop or permission error on `.resolve()`
-    must disable that one file rather than abort the whole load — so both
-    failures are caught, and both are warnings rather than raises.
-
-    The two branches log differently on purpose: a `ValueError` means the path
-    resolved *outside* the pack (a crafted or broken manifest), an `OSError`
-    means it could not be resolved at all (symlink loop, permissions). Reading
-    "escapes source_dir" in the logs and reading "resolution failed" lead an
-    operator to very different places, so the distinction is worth two lines.
-
-    Returns the resolved path — **existence is NOT checked here**. Callers
-    differ on that deliberately: a missing optional side-file is silent, while a
-    missing arrangement skips an entry, so each caller keeps its own `.exists()`
-    (or `.is_file()`) test and its own control flow.
-
-    `label` names the manifest key in the log message ("keys", "song_timeline",
-    a drum part's id, …).
-    """
-    try:
-        p = (source_dir / rel).resolve()
-        p.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
-    return p
-
-
 def _legacy_full_mix(manifest: dict, source_dir: Path) -> str | None:
     """Full mix from the DEPRECATED `original_audio:` manifest key, or None.
 
@@ -187,8 +152,16 @@ def _legacy_full_mix(manifest: dict, source_dir: Path) -> str | None:
     if not isinstance(rel_raw, str) or not rel_raw.strip():
         return None
     rel = rel_raw.strip()
-    target = _resolve_pack_path(source_dir, rel, "original_audio")
-    if target is None or not target.is_file():
+    try:
+        target = (source_dir / rel).resolve()
+        target.relative_to(source_dir.resolve())
+    except ValueError:
+        log.warning("sloppak: original_audio path %r escapes source_dir — skipped", rel)
+        return None
+    except OSError as e:
+        log.warning("sloppak: original_audio path resolution failed (%s) — skipped", e)
+        return None
+    if not target.is_file():
         return None
     log.info(
         "sloppak: pack uses the deprecated `original_audio:` key (%r) — the full mix "
@@ -725,14 +698,6 @@ 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` /
@@ -784,8 +749,20 @@ def _load_drum_tab_file(source_dir: Path, rel: str, label: str) -> dict | None:
     permissive — a missing file disables that part silently; a traversal,
     parse, or validation failure disables it with a warning, never aborting
     the load."""
-    dt_path = _resolve_pack_path(source_dir, rel, label)
-    if dt_path is None or not dt_path.exists():
+    # 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)
@@ -799,117 +776,18 @@ 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 _entry_tones(entry: dict) -> dict | None:
-    """A manifest entry's `tones` binding, or None when it doesn't carry one.
-
-    Spec §5.2: a manifest arrangement entry's `tones` overrides the arrangement
-    JSON's `tones` **wholesale** — no field-level merge. This normalizes the
-    "does it carry one" test for both the arrangement path and the drum path.
-
-    An empty dict reads as *absent*, not as "override to silence": it is what a
-    Writer emits by accident, `arrangement_from_wire` already normalizes the
-    in-JSON `{}` to None the same way, and treating it as an override would let
-    a stray empty object silently unbind a part's sound.
-    """
-    tones = entry.get("tones")
-    return tones if isinstance(tones, dict) and tones else None
-
-
 def _resolve_drum_parts(
     source_dir: Path,
     drum_tab_rel: object,
     drum_tab_data: dict | None,
     drum_pointer_entries: list[dict],
-    drum_tones: dict | None = None,
 ) -> tuple[dict | None, list[dict] | None]:
-    """Resolve drum pointers into a primary-first list with unique ids.
-
-    Also binds each part's sound (feedpak 1.18.0). The precedence mirrors the
-    `drum_tab` alias rule this function already implements: a `type: drums`
-    entry's own `tones` wins for that part, and the song-level `drum_tones` is
-    the fallback for the **primary** part only. A Reader MUST NOT apply both to
-    the same part (spec §5.1/§5.2), which is why the primary picks one or the
-    other here rather than merging them.
-    """
+    """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
-    # The primary's own binding, lifted from its alias pointer entry when it has
-    # one. Stays None if no entry claims the primary — `drum_tones` fills in.
-    primary_tones = None
     extra_parts: list[dict] = []
     seen_rels: set[str] = set()
     # Use the same canonical, traversal-safe identity as zip member lookup so
@@ -933,11 +811,6 @@ def _resolve_drum_parts(
                 primary_id = entry_id
             if entry_name:
                 primary_name = entry_name
-            # This entry IS the primary (an alias pointer at the same file), so
-            # its binding is the primary's — and it outranks `drum_tones`.
-            _alias_tones = _entry_tones(entry)
-            if _alias_tones is not None:
-                primary_tones = _alias_tones
             continue
         tab = _load_drum_tab_file(source_dir, rel, f"drum part {entry_id or rel}")
         if tab is None:
@@ -948,9 +821,6 @@ def _resolve_drum_parts(
             "name": entry_name
                 or (tab_name if isinstance(tab_name, str) and tab_name else "Drums"),
             "drum_tab": tab,
-            # Non-primary parts bind through their own entry only; `drum_tones`
-            # is explicitly the primary's fallback, never theirs.
-            "tones": _entry_tones(entry),
         })
 
     parts: list[dict] = []
@@ -959,14 +829,7 @@ def _resolve_drum_parts(
         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,
-            # Entry `tones` takes precedence; `drum_tones` is the fallback. One
-            # or the other, never both on the same part (spec §5.1).
-            "tones": primary_tones if primary_tones is not None else drum_tones,
-        })
+        parts.append({"id": primary_id, "name": primary_name, "drum_tab": drum_tab_data})
         used_ids.add(primary_id)
 
     next_generated_id = 2
@@ -1046,8 +909,16 @@ def load_song(
             continue
         data = None
         if rel:
-            arr_path = _resolve_pack_path(source_dir, rel, "arrangement")
-            if arr_path is None or not arr_path.exists():
+            try:
+                arr_path = (source_dir / rel).resolve()
+                arr_path.relative_to(source_dir.resolve())
+            except ValueError:
+                log.warning("sloppak: arrangement path %r escapes source_dir — skipped", rel)
+                continue
+            except OSError as e:
+                log.warning("sloppak: arrangement path resolution failed (%s) — skipped", e)
+                continue
+            if not arr_path.exists():
                 continue
             try:
                 data = load_json(arr_path)
@@ -1077,14 +948,6 @@ def load_song(
             # _finite_float keeps a malformed manifest NaN/Infinity from
             # poisoning the song_info JSON (same guard as the wire path).
             arr.cent_offset = _finite_float(entry["centOffset"])
-        # `tones` overrides WHOLESALE, unlike the field-level overrides above:
-        # the entry's object replaces the arrangement JSON's entirely, with no
-        # per-field merge (spec §5.2). A Writer SHOULD NOT emit both, but when
-        # one does, a half-merged sound — this pack's base with that pack's
-        # changes — would be worse than either source alone.
-        _entry_tone_block = _entry_tones(entry)
-        if _entry_tone_block is not None:
-            arr.tones = _entry_tone_block
 
         # Beats/sections can live on the arrangement itself in the wire format.
         # If the manifest-level arrangement JSON carries them, pull them onto
@@ -1117,7 +980,15 @@ def load_song(
         notation_rel = notation_rel.strip()
         if not notation_rel:
             continue
-        nt_path = _resolve_pack_path(source_dir, notation_rel, "notation")
+        try:
+            nt_path = (source_dir / notation_rel).resolve()
+            nt_path.relative_to(source_dir.resolve())
+        except ValueError:
+            log.warning("sloppak: notation path %r escapes source_dir — skipped", notation_rel)
+            nt_path = None
+        except OSError as e:
+            log.warning("sloppak: notation path resolution failed (%s) — skipped", e)
+            nt_path = None
         raw_nt = None
         if nt_path is not None and nt_path.exists():
             try:
@@ -1149,15 +1020,8 @@ def load_song(
 
     # Keep the dense compatibility logic independently testable and guarantee
     # ids are unique before the highway exposes them as selectors.
-    # Top-level `drum_tones` (spec §5.1) binds the song-level drum part — the
-    # fallback for packs without `type: drums` arrangements. Same shape as an
-    # arrangement entry's `tones`; `_resolve_drum_parts` owns the precedence.
-    _raw_drum_tones = manifest.get("drum_tones")
-    drum_tones_data = _raw_drum_tones if isinstance(_raw_drum_tones, dict) and _raw_drum_tones else None
-
     drum_tab_data, drum_parts = _resolve_drum_parts(
         source_dir, drum_tab_rel, drum_tab_data, drum_pointer_entries,
-        drum_tones_data,
     )
 
     # Drum-only sloppak: every GP track was percussion, so it ships a
@@ -1197,7 +1061,15 @@ def load_song(
     time_sigs_data: list | None = None
     song_timeline_rel = manifest.get("song_timeline")
     if isinstance(song_timeline_rel, str) and song_timeline_rel:
-        st_path = _resolve_pack_path(source_dir, song_timeline_rel, "song_timeline")
+        try:
+            st_path = (source_dir / song_timeline_rel).resolve()
+            st_path.relative_to(source_dir.resolve())
+        except ValueError:
+            log.warning("sloppak: song_timeline path %r escapes source_dir — skipped", song_timeline_rel)
+            st_path = None
+        except OSError as e:
+            log.warning("sloppak: song_timeline path resolution failed (%s) — skipped", e)
+            st_path = None
         if st_path is not None and st_path.exists():
             try:
                 raw = load_json(st_path)
@@ -1287,7 +1159,15 @@ def load_song(
     # downstream through the WS path.
     lyrics_rel = manifest.get("lyrics")
     if isinstance(lyrics_rel, str) and lyrics_rel:
-        lyr_path = _resolve_pack_path(source_dir, lyrics_rel, "lyrics")
+        try:
+            lyr_path = (source_dir / lyrics_rel).resolve()
+            lyr_path.relative_to(source_dir.resolve())
+        except ValueError:
+            log.warning("sloppak: lyrics path %r escapes source_dir — skipped", lyrics_rel)
+            lyr_path = None
+        except OSError as e:
+            log.warning("sloppak: lyrics path resolution failed (%s) — skipped", e)
+            lyr_path = None
         if lyr_path is not None and lyr_path.exists():
             try:
                 raw = load_json(lyr_path)
@@ -1392,7 +1272,15 @@ def load_song(
     keys_data: dict | None = None
     keys_rel = manifest.get("keys")
     if isinstance(keys_rel, str) and keys_rel:
-        k_path = _resolve_pack_path(source_dir, keys_rel, "keys")
+        try:
+            k_path = (source_dir / keys_rel).resolve()
+            k_path.relative_to(source_dir.resolve())
+        except ValueError:
+            log.warning("sloppak: keys path %r escapes source_dir — skipped", keys_rel)
+            k_path = None
+        except OSError as e:
+            log.warning("sloppak: keys path resolution failed (%s) — skipped", e)
+            k_path = None
         if k_path is not None and k_path.exists():
             try:
                 raw = load_json(k_path)
@@ -1437,14 +1325,6 @@ 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
@@ -1475,7 +1355,6 @@ 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,

lib/tones.py

@@ -32,29 +32,20 @@ def tokens(s: str) -> set[str]:
     return {t for t in re.split(r"[^a-z0-9]+", (s or "").lower()) if t}
 
 
-def sloppak_tone_changes(arr_tones) -> tuple[str, str, list[dict]]:
+def sloppak_tone_changes(arr_tones) -> tuple[str, list[dict]]:
     """Build the highway tone-change payload from an arrangement's tone block.
 
     Given ``Arrangement.tones`` (the dict embedded in the sloppak, or ``None``),
-    returns ``(base, base_rig, changes)`` where ``base`` is the initial tone
-    name, ``base_rig`` is the ``rigs.json`` rig id bound to it (feedpak-spec
-    §6.9; ``""`` when absent), and ``changes`` is a time-sorted
-    ``[{"t", "name", "rig"?}]`` list. Non-string names, non-dict entries, and
-    non-numeric / non-finite times are skipped — a hand-edited or third-party
-    sloppak must not crash the highway WebSocket or emit NaN/inf (which the
-    client's ``JSON.parse`` rejects).
-
-    ``rig`` / ``base_rig`` are carried through but NOT resolved against
-    ``rigs.json`` here: this builder only preserves the binding the chart
-    declared. Realization selection and the ``intent.gm`` fallback (§7.9) belong
-    to the consumer that actually voices the part.
+    returns ``(base, changes)`` where ``base`` is the initial tone name and
+    ``changes`` is a time-sorted ``[{"t", "name"}]`` list. Non-string names,
+    non-dict entries, and non-numeric / non-finite times are skipped — a
+    hand-edited or third-party sloppak must not crash the highway WebSocket
+    or emit NaN/inf (which the client's ``JSON.parse`` rejects).
     """
     if not isinstance(arr_tones, dict):
-        return "", "", []
+        return "", []
     base_val = arr_tones.get("base", "")
     base = base_val.strip() if isinstance(base_val, str) else ""
-    base_rig_val = arr_tones.get("base_rig", "")
-    base_rig = base_rig_val.strip() if isinstance(base_rig_val, str) else ""
 
     changes: list[dict] = []
     raw_changes = arr_tones.get("changes")
@@ -74,13 +65,6 @@ def sloppak_tone_changes(arr_tones) -> tuple[str, str, list[dict]]:
             continue
         if not math.isfinite(t):
             continue
-        change = {"t": round(t, 3), "name": name}
-        # ponytail: `rig` only when it's a usable id — a non-string or blank
-        # value is dropped rather than forwarded, so a consumer can treat
-        # presence of the key as "this change binds a rig".
-        rig = c.get("rig")
-        if isinstance(rig, str) and rig.strip():
-            change["rig"] = rig.strip()
-        changes.append(change)
+        changes.append({"t": round(t, 3), "name": name})
     changes.sort(key=lambda x: x["t"])
-    return base, base_rig, changes
+    return base, changes

static/js/count-in.js

@@ -1,4 +1,4 @@
-// Count-in — the one-bar click before playback, plus the song-credits overlay that
+// Count-in — the 1-2-3-4 click before playback, plus the song-credits overlay that
 // shares its lifecycle and timers.
 //
 // The third slice out of app.js's strongly-connected core, and the first that had to
@@ -40,75 +40,6 @@ export function playClick(high = false) {
     osc.stop(_audioCtx.currentTime + 0.08);
 }
 
-// ── How many clicks lead into `startT` ──────────────────────────────────
-// One bar, derived from the song_timeline beats: `window.highway.getBeats()`
-// is the only meter data the frontend holds (the `time_signatures` map is
-// streamed to plugins, not stored here). Beats carry `measure >= 0` on
-// downbeats, so the gap between consecutive downbeats IS the bar length —
-// which is why a 3/4 song no longer gets four clicks.
-//
-// A first bar shorter than that is a pickup (anacrusis), and the count is
-// shortened by its length so the music enters on its real beat: a 1-beat
-// pickup in 4/4 counts "1 2 3" and the pickup lands on 4. Counting a full
-// four there puts the pickup where the downbeat belongs, and the player comes
-// in a beat late for the whole song.
-export function countInBeats(startT) {
-    const DEFAULT = 4;   // pre-chart, synthetic highway (minigames), or no beats
-    let beats = null;
-    try {
-        if (window.highway && typeof window.highway.getBeats === 'function') {
-            beats = window.highway.getBeats();
-        }
-    } catch (_) { /* fall through to the default */ }
-    if (!Array.isArray(beats) || beats.length < 2) return DEFAULT;
-
-    const downbeats = [];
-    for (let i = 0; i < beats.length; i++) {
-        if (beats[i] && beats[i].measure >= 0) downbeats.push(i);
-    }
-    if (downbeats.length < 2) return DEFAULT;
-
-    // Bar length = the most common gap between downbeats. The mode rather than
-    // the first gap: it ignores a short pickup bar and a short final bar, and
-    // survives an isolated meter change mid-song. The beats trailing the last
-    // downbeat count as a candidate too — otherwise a song of pickup + one bar
-    // offers only the pickup's own gap and the count collapses to it.
-    const gapCounts = new Map();
-    const addGap = (gap) => gapCounts.set(gap, (gapCounts.get(gap) || 0) + 1);
-    for (let k = 1; k < downbeats.length; k++) {
-        addGap(downbeats[k] - downbeats[k - 1]);
-    }
-    addGap(beats.length - downbeats[downbeats.length - 1]);
-    let barLen = DEFAULT;
-    let bestCount = 0;
-    for (const [gap, n] of gapCounts) {
-        // Tie → the longer bar: a pickup's short gap must not outvote the
-        // real meter when the song is too short to repeat it.
-        if (n > bestCount || (n === bestCount && gap > barLen)) {
-            barLen = gap;
-            bestCount = n;
-        }
-    }
-
-    // The beat playback resumes on. The 50 ms tolerance matches the seek
-    // precision the loop-wrap path already assumes.
-    const startIdx = beats.findIndex(b => b && b.time >= startT - 0.05);
-    if (startIdx === -1) return barLen;                  // past the last beat
-    if (!(beats[startIdx].measure >= 0)) return barLen;  // resuming mid-bar
-
-    const nextDownbeat = downbeats.find(d => d > startIdx);
-    if (nextDownbeat === undefined) return barLen;       // the last downbeat
-    const thisBar = nextDownbeat - startIdx;
-    if (thisBar <= 0) return barLen;
-
-    // Only the song's FIRST bar can be a pickup. A short bar anywhere else is
-    // a meter change (or a truncated final bar), and counting it as a pickup
-    // would leave almost no count-in at all — so elsewhere we simply count
-    // that bar's own length, which is also what a mid-song meter change wants.
-    if (startIdx === downbeats[0] && thisBar < barLen) return barLen - thisBar;
-    return thisBar;
-}
-
 let _countingIn = false;
 let _countOverlay = null;
 // Generation token so teardown can cancel an in-progress count-in. Each
@@ -342,15 +273,12 @@ export async function startCountIn(opts = {}) {
     function beginCount() {
         const bpm = window.highway.getBPM(loopA);
         const beatInterval = 60 / bpm;
-        // One bar of the meter at loop A (a short bar there is counted short,
-        // same as the song-start pickup).
-        const clicks = countInBeats(loopA);
         let count = 0;
 
         function tick() {
             if (gen !== _countInGen) return; // teardown mid-count
             count++;
-            if (count > clicks) {
+            if (count > 4) {
                 hideCountOverlay();
                 _countingIn = false;
                 if (window._juceMode) {
@@ -392,7 +320,7 @@ export async function startCountIn(opts = {}) {
     }
 }
 
-// Start-of-song count-in: a one-bar click before playback begins, gated by the
+// Start-of-song count-in: a 4-beat click before playback begins, gated by the
 // "Countdown before song" setting (Gameplay tab). Mirrors the loop count-in's
 // overlay + click + gen-token cancellation, but counts from the song's current
 // position (0 at song start) with no loop A/B rewind. startCountIn() is loop-
@@ -412,15 +340,14 @@ export async function startSongCountIn() {
     if (gen !== _countInGen) return; // teardown during pause
     const startT = S.lastAudioTime || 0;
     let bpm = window.highway.getBPM(startT);
-    // Pre-chart / malformed-tempo fallback: 120 BPM (500 ms per beat).
+    // Pre-chart / malformed-tempo fallback: 4 beats at 120 BPM (500 ms each).
     if (!Number.isFinite(bpm) || bpm <= 0) bpm = 120;
     const beatInterval = 60 / bpm;
-    const clicks = countInBeats(startT);
     let count = 0;
     function tick() {
         if (gen !== _countInGen) return; // teardown mid-count
         count++;
-        if (count > clicks) {
+        if (count > 4) {
             hideCountOverlay();
             _countingIn = false;
             // Hand off to the normal play path — togglePlay() flips isPlaying,

tests/js/count_in_beats.test.js

@@ -1,189 +0,0 @@
-// Verify `countInBeats(startT)` in static/js/count-in.js sizes the count-in
-// to the song's own bar rather than a hardcoded four clicks.
-//
-// Two behaviours are under test:
-//   1. Meter — a 3/4 song gets three clicks, not four.
-//   2. Pickup (anacrusis) — a first bar shorter than the meter shortens the
-//      count so the pickup enters on its real beat (1-beat pickup in 4/4 →
-//      "1 2 3", music on 4). A full four there puts the pickup where the
-//      downbeat belongs and the player comes in a beat late all song.
-//
-// The meter is read from the song_timeline beats (`window.highway.getBeats()`,
-// `measure >= 0` on downbeats) because that is the only meter data the
-// frontend holds — the `time_signatures` map is streamed to plugins, not
-// stored here.
-//
-// Same extraction approach as loop_restart.test.js: pull the function source
-// out of the module and evaluate it in a vm sandbox with a stubbed highway,
-// rather than loading the ESM module and its DOM-coupled imports.
-
-const { test } = require('node:test');
-const assert = require('node:assert/strict');
-const fs = require('node:fs');
-const path = require('node:path');
-const vm = require('node:vm');
-
-const COUNT_IN_JS = path.join(__dirname, '..', '..', 'static', 'js', 'count-in.js');
-
-// Brace-match the function body out of the source. Brittle by design:
-// a rename fails loudly here rather than silently skipping coverage.
-function extractFunction(src, signature) {
-    const start = src.indexOf(signature);
-    if (start === -1) throw new Error(`extractFunction: '${signature}' not found`);
-    const openBrace = src.indexOf('{', start + signature.length);
-    let depth = 1;
-    let i = openBrace + 1;
-    while (i < src.length && depth > 0) {
-        const ch = src[i];
-        if (ch === '{') depth++;
-        else if (ch === '}') depth--;
-        i++;
-    }
-    if (depth !== 0) throw new Error(`extractFunction: unbalanced braces after '${signature}'`);
-    return src.slice(start, i);
-}
-
-const src = fs.readFileSync(COUNT_IN_JS, 'utf8');
-// Drop the `export` keyword so the body evaluates as a plain declaration.
-const fnSrc = extractFunction(src, 'export function countInBeats')
-    .replace(/^export\s+/, '');
-
-// `beats` is the song_timeline shape: {time, measure}, measure >= 0 only on
-// downbeats. `getBeats` may also be absent entirely (pre-chart / minigame).
-function load(beats) {
-    const sandbox = {
-        window: beats === undefined
-            ? { highway: {} }
-            : { highway: { getBeats: () => beats } },
-    };
-    vm.createContext(sandbox);
-    vm.runInContext(`${fnSrc}; globalThis.__fn = countInBeats;`, sandbox);
-    return sandbox.__fn;
-}
-
-// Build a beats array: `bars` full bars of `beatsPerBar`, optionally preceded
-// by a pickup of `pickup` beats. One beat per 0.5 s throughout.
-function makeBeats({ beatsPerBar = 4, bars = 4, pickup = 0 } = {}) {
-    const out = [];
-    let t = 0;
-    let measure = 0;
-    if (pickup > 0) {
-        for (let i = 0; i < pickup; i++) {
-            out.push({ time: t, measure: i === 0 ? measure : -1 });
-            t += 0.5;
-        }
-        measure++;
-    }
-    for (let b = 0; b < bars; b++) {
-        for (let i = 0; i < beatsPerBar; i++) {
-            out.push({ time: t, measure: i === 0 ? measure : -1 });
-            t += 0.5;
-        }
-        measure++;
-    }
-    return out;
-}
-
-// ── Meter ────────────────────────────────────────────────────────────────
-
-test('countInBeats counts a full bar in 4/4', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 4 }));
-    assert.equal(countInBeats(0), 4);
-});
-
-test('countInBeats counts three in 3/4 (was hardcoded four)', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 3 }));
-    assert.equal(countInBeats(0), 3);
-});
-
-test('countInBeats counts six in 6/8', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 6 }));
-    assert.equal(countInBeats(0), 6);
-});
-
-// ── Pickup (anacrusis) ───────────────────────────────────────────────────
-
-test('countInBeats shortens the count by a 1-beat pickup in 4/4', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 4, pickup: 1 }));
-    assert.equal(countInBeats(0), 3, 'counts 1-2-3 so the pickup lands on 4');
-});
-
-test('countInBeats shortens the count by a 2-beat pickup in 4/4', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 4, pickup: 2 }));
-    assert.equal(countInBeats(0), 2);
-});
-
-test('countInBeats handles a pickup in 3/4', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 3, pickup: 1 }));
-    assert.equal(countInBeats(0), 2);
-});
-
-test('countInBeats finds the meter when the song is only a pickup plus one bar', () => {
-    // Gap counts tie (one 1-beat gap, one 4-beat gap) — the longer bar is the
-    // meter, so this must be 3 rather than 0.
-    const countInBeats = load(makeBeats({ beatsPerBar: 4, bars: 1, pickup: 1 }));
-    assert.equal(countInBeats(0), 3);
-});
-
-// ── Resuming somewhere other than the song top ───────────────────────────
-
-test('countInBeats counts a full bar at a mid-song downbeat, pickup notwithstanding', () => {
-    const beats = makeBeats({ beatsPerBar: 4, pickup: 1 });
-    const countInBeats = load(beats);
-    // Index 5 is the downbeat of the second full bar (1 pickup + 4 beats).
-    assert.equal(beats[5].measure >= 0, true, 'fixture sanity: index 5 is a downbeat');
-    assert.equal(countInBeats(beats[5].time), 4);
-});
-
-test('countInBeats counts a mid-song meter change by that bar, not as a pickup', () => {
-    // 4/4 throughout, except one 3-beat bar at index 8. Treating a short bar
-    // anywhere but the song's first as a pickup would count a single click.
-    const beats = [];
-    let t = 0;
-    const push = (n, measure) => {
-        for (let i = 0; i < n; i++) { beats.push({ time: t, measure: i === 0 ? measure : -1 }); t += 0.5; }
-    };
-    push(4, 0); push(4, 1); push(3, 2); push(4, 3); push(4, 4);
-    const countInBeats = load(beats);
-    assert.equal(beats[8].measure, 2, 'fixture sanity: index 8 opens the 3-beat bar');
-    assert.equal(countInBeats(beats[8].time), 3, 'counts the short bar itself');
-    assert.equal(countInBeats(0), 4, 'the 4/4 opening is unaffected');
-});
-
-test('countInBeats counts a full bar when resuming mid-bar', () => {
-    const beats = makeBeats({ beatsPerBar: 4 });
-    const countInBeats = load(beats);
-    assert.equal(countInBeats(beats[2].time), 4);   // third beat of bar 1
-});
-
-test('countInBeats tolerates a start time slightly past the beat (seek slop)', () => {
-    const countInBeats = load(makeBeats({ beatsPerBar: 4, pickup: 1 }));
-    assert.equal(countInBeats(0.02), 3);
-});
-
-// ── Fallbacks ────────────────────────────────────────────────────────────
-
-test('countInBeats falls back to four without a beats array', () => {
-    assert.equal(load(undefined)(0), 4, 'no getBeats (pre-chart / minigame)');
-    assert.equal(load([])(0), 4, 'empty beats');
-    assert.equal(load(null)(0), 4, 'null beats');
-});
-
-test('countInBeats falls back to four when beats carry no downbeat labels', () => {
-    const beats = [0, 0.5, 1.0, 1.5, 2.0].map(time => ({ time, measure: -1 }));
-    assert.equal(load(beats)(0), 4);
-});
-
-test('countInBeats falls back to four with only one downbeat', () => {
-    const beats = [
-        { time: 0, measure: 0 },
-        { time: 0.5, measure: -1 },
-        { time: 1.0, measure: -1 },
-    ];
-    assert.equal(load(beats)(0), 4);
-});
-
-test('countInBeats counts a full bar past the last beat', () => {
-    const beats = makeBeats({ beatsPerBar: 3 });
-    assert.equal(load(beats)(9999), 3);
-});

tests/js/loop_restart.test.js

@@ -88,10 +88,6 @@ function buildSandbox() {
         playClick: () => {},
         showCountOverlay: () => {},
         hideCountOverlay: () => {},
-        // beginCount sizes the count to the bar at loop A; the wrap-path
-        // assertions below don't depend on how many clicks it decides on.
-        // Covered directly in count_in_beats.test.js.
-        countInBeats: () => 4,
 
         // Stubbed DOM access. Anything querying for a button just gets a
         // permissive object that ignores writes.

tests/test_gp2rs_gpx.py

@@ -7,6 +7,7 @@ the PR; here we pin the input-validation guards and the conversion helpers
 that are easy to drive without a fixture.
 """
 
+import json
 import struct
 import xml.etree.ElementTree as ET
 
@@ -29,6 +30,10 @@ from gp2rs_gpx import (
     _GPX_MAX_DECOMPRESSED,
     _find_piano_pairs,
     convert_vocal_track_to_pitch_sidecar,
+    _vocals_xml_to_lyrics,
+    attach_vocal_sidecars_to_sloppak,
+    lyrics_sidecar_path,
+    vocal_pitch_sidecar_path,
     _collect_tone_events,
     _inject_tones,
     _resolve_pending_slides,
@@ -386,6 +391,192 @@ def test_vocal_pitch_sidecar_skips_beat_without_lyric():
     assert out == {"version": 1, "notes": []}
 
 
+def test_vocal_pitch_sidecar_require_lyric_false_emits_melody():
+    # Lyric-less vocal track: with the gate relaxed the authored pitch still
+    # ships (there are no lyric tokens to stay aligned with).
+    out = convert_vocal_track_to_pitch_sidecar(
+        **_vocal_sidecar_args(with_lyric=False), require_lyric=False)
+    assert out == {"version": 1, "notes": [{"t": 0.0, "d": 0.5, "midi": 60}]}
+
+
+# ── _vocals_xml_to_lyrics ───────────────────────────────────────────────────
+
+def test_vocals_xml_to_lyrics_shape_and_suffixes():
+    xml = (
+        '<vocals count="4">'
+        '<vocal time="0.000" note="60" length="0.500" lyric="Hel-"/>'
+        '<vocal time="0.500" note="62" length="0.500" lyric="lo"/>'
+        '<vocal time="1.000" note="64" length="0.250" lyric="sing+"/>'
+        '<vocal time="1.250" note="64" length="0.250" lyric="ing"/>'
+        '</vocals>'
+    )
+    out = _vocals_xml_to_lyrics(xml)
+    assert out == [
+        # "-" means the same join in both conventions — passed through.
+        {"t": 0.0, "d": 0.5, "w": "Hel-"},
+        {"t": 0.5, "d": 0.5, "w": "lo"},
+        # XML "+" is a JOIN; feedpak "+" is a LINE END — joins become "-".
+        {"t": 1.0, "d": 0.25, "w": "sing-"},
+        {"t": 1.25, "d": 0.25, "w": "ing"},
+    ]
+
+
+def test_vocals_xml_to_lyrics_skips_bare_joiners_and_bad_xml():
+    xml = (
+        '<vocals count="2">'
+        '<vocal time="0.000" note="0" length="0.500" lyric="+"/>'
+        '<vocal time="0.500" note="60" length="0.500" lyric="la"/>'
+        '</vocals>'
+    )
+    assert _vocals_xml_to_lyrics(xml) == [{"t": 0.5, "d": 0.5, "w": "la"}]
+    assert _vocals_xml_to_lyrics("not xml <<<") == []
+
+
+# ── convert_file end-to-end: vocal karaoke sidecars ─────────────────────────
+# A vocal track must emit `<stem>.lyrics.json` + `<stem>.vocal_pitch.json`
+# next to the vocals XML; non-vocal tracks must not; a lyric-less vocal track
+# emits the pitch sidecar only.
+
+_GPIF_VOCAL = """
+<GPIF>
+  <Score><Title>T</Title><Artist>A</Artist></Score>
+  <Tracks>
+    <Track id="0"><Name>Vocals</Name>
+      <Property name="Tuning"><Pitches>60</Pitches></Property></Track>
+  </Tracks>
+  <MasterBars><MasterBar><Time>4/4</Time><Bars>0</Bars></MasterBar></MasterBars>
+  <Bars><Bar id="0"><Voices>0</Voices></Bar></Bars>
+  <Voices><Voice id="0"><Beats>0 1</Beats></Voice></Voices>
+  <Beats>
+    <Beat id="0"><Rhythm ref="r0"/><Lyrics><Line>Hel-</Line></Lyrics><Notes>0</Notes></Beat>
+    <Beat id="1"><Rhythm ref="r0"/><Lyrics><Line>lo</Line></Lyrics><Notes>1</Notes></Beat>
+  </Beats>
+  <Notes>
+    <Note id="0">
+      <Property name="String"><String>0</String></Property>
+      <Property name="Fret"><Fret>0</Fret></Property></Note>
+    <Note id="1">
+      <Property name="String"><String>0</String></Property>
+      <Property name="Fret"><Fret>2</Fret></Property></Note>
+  </Notes>
+  <Rhythms><Rhythm id="r0"><NoteValue>Quarter</NoteValue></Rhythm></Rhythms>
+</GPIF>
+"""
+
+# Same melody, no <Lyrics> anywhere.
+_GPIF_VOCAL_NO_LYRICS = _GPIF_VOCAL.replace(
+    "<Lyrics><Line>Hel-</Line></Lyrics>", "").replace(
+    "<Lyrics><Line>lo</Line></Lyrics>", "")
+
+
+def test_convert_file_vocal_track_emits_both_sidecars(tmp_path, monkeypatch):
+    monkeypatch.setattr(gp2rs_gpx, "_load_gpif",
+                        lambda _p: ET.fromstring(_GPIF_VOCAL))
+    out_files = convert_file("dummy.gpx", str(tmp_path), track_indices=[0])
+    assert len(out_files) == 1
+    assert ET.parse(out_files[0]).getroot().tag == "vocals"
+
+    # lyrics.json sidecar: flat [{t, d, w}] (spec §7.1), timings from the XML.
+    lyr = json.loads(lyrics_sidecar_path(out_files[0]).read_text(encoding="utf-8"))
+    assert lyr == [
+        {"t": 0.0, "d": 0.5, "w": "Hel-"},
+        {"t": 0.5, "d": 0.5, "w": "lo"},
+    ]
+
+    # vocal_pitch.json sidecar: {version, notes:[{t, d, midi}]} (spec §7.2),
+    # lyric-aligned (one note per syllable) at the authored pitches.
+    pitch = json.loads(vocal_pitch_sidecar_path(out_files[0]).read_text(encoding="utf-8"))
+    assert pitch == {"version": 1, "notes": [
+        {"t": 0.0, "d": 0.5, "midi": 60},
+        {"t": 0.5, "d": 0.5, "midi": 62},
+    ]}
+
+
+def test_convert_file_no_vocal_track_no_sidecars(tmp_path, monkeypatch):
+    monkeypatch.setattr(gp2rs_gpx, "_load_gpif",
+                        lambda _p: ET.fromstring(_GPIF_GUITAR_ASCENDING))
+    convert_file("dummy.gp", str(tmp_path),
+                 track_indices=[0], arrangement_names={0: "Lead"})
+    assert not list(tmp_path.glob("*.lyrics.json"))
+    assert not list(tmp_path.glob("*.vocal_pitch.json"))
+
+
+def test_convert_file_lyricless_vocal_track_pitch_sidecar_only(tmp_path, monkeypatch):
+    monkeypatch.setattr(gp2rs_gpx, "_load_gpif",
+                        lambda _p: ET.fromstring(_GPIF_VOCAL_NO_LYRICS))
+    out_files = convert_file("dummy.gpx", str(tmp_path), track_indices=[0])
+    assert len(out_files) == 1
+    # No lyric text anywhere -> no lyrics.json; the authored melody still
+    # ships as vocal_pitch.json (lyric gate relaxed for lyric-less tracks).
+    assert not lyrics_sidecar_path(out_files[0]).exists()
+    pitch = json.loads(vocal_pitch_sidecar_path(out_files[0]).read_text(encoding="utf-8"))
+    assert pitch == {"version": 1, "notes": [
+        {"t": 0.0, "d": 0.5, "midi": 60},
+        {"t": 0.5, "d": 0.5, "midi": 62},
+    ]}
+
+
+# ── attach_vocal_sidecars_to_sloppak ────────────────────────────────────────
+
+_LYRICS_PAYLOAD = [{"t": 0.0, "d": 0.5, "w": "Hel-"}, {"t": 0.5, "d": 0.5, "w": "lo"}]
+_PITCH_PAYLOAD = {"version": 1, "notes": [{"t": 0.0, "d": 0.5, "midi": 60}]}
+
+
+def _make_pak(tmp_path, manifest: dict):
+    import yaml
+    pak = tmp_path / "pak"
+    pak.mkdir()
+    (pak / "manifest.yaml").write_text(
+        yaml.safe_dump(manifest, sort_keys=False), encoding="utf-8")
+    return pak
+
+
+def test_attach_vocal_sidecars_writes_files_and_manifest(tmp_path):
+    import yaml
+    pak = _make_pak(tmp_path, {"title": "T", "arrangements": []})
+    written = attach_vocal_sidecars_to_sloppak(
+        pak, lyrics=_LYRICS_PAYLOAD, vocal_pitch=_PITCH_PAYLOAD)
+    assert written == [pak / "lyrics.json", pak / "vocal_pitch.json"]
+    assert json.loads((pak / "lyrics.json").read_text(encoding="utf-8")) == _LYRICS_PAYLOAD
+    assert json.loads((pak / "vocal_pitch.json").read_text(encoding="utf-8")) == _PITCH_PAYLOAD
+    manifest = yaml.safe_load((pak / "manifest.yaml").read_text(encoding="utf-8"))
+    assert manifest["lyrics"] == "lyrics.json"
+    assert manifest["lyrics_source"] == "authored"      # GP tab = authored chart
+    assert manifest["vocal_pitch"] == "vocal_pitch.json"
+    # No automated-engine provenance for authored payloads (spec §7.1.1/§7.2.1).
+    assert "lyric_transcription" not in manifest
+    assert "pitch_extraction" not in manifest
+    assert "feedpak_version" in manifest
+
+
+def test_attach_vocal_sidecars_never_clobbers(tmp_path):
+    import yaml
+    pak = _make_pak(tmp_path, {
+        "title": "T",
+        "lyrics": "existing_lyrics.json",
+        "lyrics_source": "user",
+    })
+    written = attach_vocal_sidecars_to_sloppak(
+        pak, lyrics=_LYRICS_PAYLOAD, vocal_pitch=_PITCH_PAYLOAD)
+    # lyrics already claimed by the manifest -> skipped entirely; pitch is new.
+    assert written == [pak / "vocal_pitch.json"]
+    assert not (pak / "lyrics.json").exists()
+    manifest = yaml.safe_load((pak / "manifest.yaml").read_text(encoding="utf-8"))
+    assert manifest["lyrics"] == "existing_lyrics.json"
+    assert manifest["lyrics_source"] == "user"
+
+
+def test_attach_vocal_sidecars_rejects_bad_payloads(tmp_path):
+    pak = _make_pak(tmp_path, {"title": "T"})
+    with pytest.raises(ValueError):
+        attach_vocal_sidecars_to_sloppak(pak, lyrics=[{"t": 0.0}])   # missing d/w
+    with pytest.raises(ValueError):
+        attach_vocal_sidecars_to_sloppak(pak, vocal_pitch={"version": 1})  # no notes
+    with pytest.raises(ValueError):
+        attach_vocal_sidecars_to_sloppak(
+            pak, lyrics=_LYRICS_PAYLOAD, lyrics_source="whisperx")  # not spec enum
+
+
 # ── _collect_tone_events ────────────────────────────────────────────────────
 
 def _tone_args(banks, tempo_map=((0, 120.0),)):

tests/test_sloppak_rigs_load.py

@@ -1,207 +0,0 @@
-"""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

tests/test_sloppak_tones_precedence.py

@@ -1,230 +0,0 @@
-"""Loader coverage for the manifest-vs-in-JSON `tones` precedence cascade
-(feedpak 1.18.0, spec §5.1 / §5.2).
-
-Two rules, both about *which* sound binding wins, neither about interpreting it:
-
-  - A manifest arrangement entry's `tones` replaces the arrangement JSON's
-    `tones` **WHOLESALE** — no field-level merge. A half-merged block (this
-    source's `base` with that source's `changes`) would be a sound nobody
-    authored, so the two never blend.
-  - Top-level `drum_tones` binds the song-level (primary) drum part and is the
-    fallback; a `type: drums` entry's own `tones` takes precedence, and a
-    Reader MUST NOT apply both to the same part.
-"""
-
-from __future__ import annotations
-
-import json
-from pathlib import Path
-
-import yaml
-
-import sloppak as sloppak_mod
-
-
-IN_JSON_TONES = {
-    "base": "In-JSON Clean",
-    "base_rig": "injson-clean",
-    "changes": [{"t": 5.0, "name": "In-JSON Lead", "rig": "injson-lead"}],
-}
-ENTRY_TONES = {
-    "base": "Entry Grand",
-    "base_rig": "entry-grand",
-    "changes": [{"t": 9.0, "name": "Entry Rhodes", "rig": "entry-rhodes"}],
-}
-
-
-def _tab(name: str) -> dict:
-    return {
-        "version": 1,
-        "name": name,
-        "kit": [{"id": "kick", "name": "Kick"}],
-        "hits": [{"t": 1.0, "p": "kick", "v": 100}],
-    }
-
-
-def _write_pak(root: Path, manifest_extras: dict, arr_tones: dict | None = None,
-               files: dict[str, dict] | None = None) -> Path:
-    """Directory-form sloppak with one Lead arrangement, optionally carrying an
-    in-JSON `tones` block, plus any extra files."""
-    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": [],
-    }
-    if arr_tones is not None:
-        arr["tones"] = arr_tones
-    (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 or {}).items():
-        (pak / rel).write_text(json.dumps(payload))
-    return pak
-
-
-def _load(pak_path: Path, tmp_path: Path):
-    cache = tmp_path / "cache"
-    cache.mkdir()
-    return sloppak_mod.load_song(pak_path.name, pak_path.parent, cache)
-
-
-# ── Arrangement entry vs in-JSON (§5.2) ──────────────────────────────────────
-
-def test_entry_tones_replaces_in_json_wholesale(tmp_path: Path):
-    """The entry object replaces the in-JSON one entirely — no key survives
-    from the loser, not even ones the winner doesn't define."""
-    entry_tones = {"base": "Entry Only"}          # no base_rig, no changes
-    pak = _write_pak(
-        tmp_path,
-        {"arrangements": [{"id": "lead", "name": "Lead",
-                           "file": "arrangements/lead.json",
-                           "tones": entry_tones}]},
-        arr_tones=IN_JSON_TONES,
-    )
-    arr = _load(pak, tmp_path).song.arrangements[0]
-    assert arr.tones == entry_tones
-    # The in-JSON `base_rig` and `changes` must NOT have been merged in.
-    assert "base_rig" not in arr.tones
-    assert "changes" not in arr.tones
-
-
-def test_in_json_tones_survive_when_entry_has_none(tmp_path: Path):
-    pak = _write_pak(tmp_path, {}, arr_tones=IN_JSON_TONES)
-    assert _load(pak, tmp_path).song.arrangements[0].tones == IN_JSON_TONES
-
-
-def test_empty_entry_tones_is_absent_not_an_override(tmp_path: Path):
-    """`{}` reads as "didn't specify", not "override to silence" — otherwise a
-    stray empty object silently unbinds the part's sound."""
-    pak = _write_pak(
-        tmp_path,
-        {"arrangements": [{"id": "lead", "name": "Lead",
-                           "file": "arrangements/lead.json", "tones": {}}]},
-        arr_tones=IN_JSON_TONES,
-    )
-    assert _load(pak, tmp_path).song.arrangements[0].tones == IN_JSON_TONES
-
-
-def test_malformed_entry_tones_is_ignored(tmp_path: Path):
-    """A non-dict `tones` must not override, and must not crash the load."""
-    pak = _write_pak(
-        tmp_path,
-        {"arrangements": [{"id": "lead", "name": "Lead",
-                           "file": "arrangements/lead.json",
-                           "tones": ["not", "a", "dict"]}]},
-        arr_tones=IN_JSON_TONES,
-    )
-    assert _load(pak, tmp_path).song.arrangements[0].tones == IN_JSON_TONES
-
-
-def test_entry_tones_binds_a_notation_only_arrangement(tmp_path: Path):
-    """§5.2: entry `tones` is available whether or not the arrangement has a
-    `file` — a keys part is a notation-only entry, and binding its sound is the
-    whole point of the 1.18.0 work."""
-    notation = {"version": 1, "measures": []}
-    pak = _write_pak(
-        tmp_path,
-        {"arrangements": [{"id": "keys", "name": "Keys",
-                           "notation": "notation_keys.json",
-                           "tones": ENTRY_TONES}]},
-        files={"notation_keys.json": notation},
-    )
-    arr = _load(pak, tmp_path).song.arrangements[0]
-    assert arr.name == "Keys"
-    assert arr.tones == ENTRY_TONES
-
-
-# ── drum_tones vs entry tones (§5.1) ─────────────────────────────────────────
-
-def test_drum_tones_binds_the_primary_part(tmp_path: Path):
-    pak = _write_pak(
-        tmp_path,
-        {"drum_tab": "drum_tab.json", "drum_tones": ENTRY_TONES},
-        files={"drum_tab.json": _tab("Drums")},
-    )
-    parts = _load(pak, tmp_path).drum_parts
-    assert len(parts) == 1
-    assert parts[0]["tones"] == ENTRY_TONES
-
-
-def test_entry_tones_outrank_drum_tones_on_the_primary(tmp_path: Path):
-    """An alias pointer entry naming the same file IS the primary, so its own
-    binding wins — and `drum_tones` must not also be applied."""
-    alias_tones = {"base": "Alias Kit", "base_rig": "alias-kit"}
-    pak = _write_pak(
-        tmp_path,
-        {
-            "drum_tab": "drum_tab.json",
-            "drum_tones": ENTRY_TONES,
-            "arrangements": [
-                {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"},
-                {"id": "drums", "name": "Drums", "type": "drums",
-                 "drum_tab": "drum_tab.json", "tones": alias_tones},
-            ],
-        },
-        files={"drum_tab.json": _tab("Drums")},
-    )
-    parts = _load(pak, tmp_path).drum_parts
-    assert len(parts) == 1
-    assert parts[0]["tones"] == alias_tones
-
-
-def test_drum_tones_does_not_leak_to_secondary_parts(tmp_path: Path):
-    """`drum_tones` is the PRIMARY's fallback only. A second drummer with no
-    binding of its own gets None — not the primary's kit."""
-    live_tones = {"base": "Live Kit", "base_rig": "live-kit"}
-    pak = _write_pak(
-        tmp_path,
-        {
-            "drum_tab": "drum_tab.json",
-            "drum_tones": ENTRY_TONES,
-            "arrangements": [
-                {"id": "lead", "name": "Lead", "file": "arrangements/lead.json"},
-                {"id": "drums-live", "name": "Drums (Live)", "type": "drums",
-                 "drum_tab": "drum_tab_live.json", "tones": live_tones},
-                {"id": "drums-prog", "name": "Drums (Prog)", "type": "drums",
-                 "drum_tab": "drum_tab_prog.json"},
-            ],
-        },
-        files={
-            "drum_tab.json": _tab("Drums"),
-            "drum_tab_live.json": _tab("Drums Live"),
-            "drum_tab_prog.json": _tab("Drums Prog"),
-        },
-    )
-    parts = {p["id"]: p for p in _load(pak, tmp_path).drum_parts}
-    assert parts["drums"]["tones"] == ENTRY_TONES        # primary, from drum_tones
-    assert parts["drums-live"]["tones"] == live_tones    # own entry
-    assert parts["drums-prog"]["tones"] is None          # no binding, no leak
-
-
-def test_drum_parts_carry_none_when_pack_binds_nothing(tmp_path: Path):
-    """A pack with drums and no sound binding at all still loads, with the key
-    present and None — consumers can read `part["tones"]` unconditionally."""
-    pak = _write_pak(
-        tmp_path,
-        {"drum_tab": "drum_tab.json"},
-        files={"drum_tab.json": _tab("Drums")},
-    )
-    parts = _load(pak, tmp_path).drum_parts
-    assert parts[0]["tones"] is None
-
-
-def test_malformed_drum_tones_is_ignored(tmp_path: Path):
-    pak = _write_pak(
-        tmp_path,
-        {"drum_tab": "drum_tab.json", "drum_tones": "not-a-dict"},
-        files={"drum_tab.json": _tab("Drums")},
-    )
-    assert _load(pak, tmp_path).drum_parts[0]["tones"] is None

tests/test_tones.py

@@ -6,17 +6,16 @@ from tones import sloppak_tone_changes
 # ── sloppak_tone_changes (highway payload builder) ───────────────────────────
 
 def test_sloppak_tone_changes_sorts_and_returns_base():
-    base, base_rig, changes = sloppak_tone_changes({
+    base, changes = sloppak_tone_changes({
         "base": "Clean",
         "changes": [{"t": 12.5, "name": "Drive"}, {"t": 3.0, "name": "Clean"}],
     })
     assert base == "Clean"
-    assert base_rig == ""
     assert changes == [{"t": 3.0, "name": "Clean"}, {"t": 12.5, "name": "Drive"}]
 
 
 def test_sloppak_tone_changes_skips_malformed_markers():
-    _, _, changes = sloppak_tone_changes({
+    _, changes = sloppak_tone_changes({
         "changes": [
             {"t": "nan", "name": "BadStr"},
             {"t": float("inf"), "name": "Inf"},
@@ -30,66 +29,18 @@ def test_sloppak_tone_changes_skips_malformed_markers():
 
 
 def test_sloppak_tone_changes_handles_none_and_bad_base():
-    assert sloppak_tone_changes(None) == ("", "", [])
-    base, base_rig, changes = sloppak_tone_changes({"base": 123, "changes": []})
-    assert base == "" and base_rig == "" and changes == []
+    assert sloppak_tone_changes(None) == ("", [])
+    base, changes = sloppak_tone_changes({"base": 123, "changes": []})
+    assert base == "" and changes == []
 
 
 def test_sloppak_tone_changes_non_dict_input():
     """A truthy non-dict payload must not crash."""
-    assert sloppak_tone_changes(["not", "a", "dict"]) == ("", "", [])
-    assert sloppak_tone_changes("nope") == ("", "", [])
+    assert sloppak_tone_changes(["not", "a", "dict"]) == ("", [])
+    assert sloppak_tone_changes("nope") == ("", [])
 
 
 def test_sloppak_tone_changes_non_list_changes():
     """A truthy non-list `changes` value must not raise on iteration."""
-    base, _, changes = sloppak_tone_changes({"base": "Clean", "changes": 1})
+    base, changes = sloppak_tone_changes({"base": "Clean", "changes": 1})
     assert base == "Clean" and changes == []
-
-
-# ── rig bindings (feedpak-spec 1.18.0 §6.9) ──────────────────────────────────
-
-def test_sloppak_tone_changes_carries_rig_bindings():
-    """`base_rig` and per-change `rig` reach the wire — the binding a chart
-    declares is what core must hand the consumer that voices the part."""
-    base, base_rig, changes = sloppak_tone_changes({
-        "base": "Clean Rhythm",
-        "base_rig": "clean-rhythm",
-        "changes": [
-            {"t": 12.5, "name": "Lead Drive", "rig": "lead-drive"},
-            {"t": 48.0, "name": "Clean Rhythm", "rig": "clean-rhythm"},
-        ],
-    })
-    assert base == "Clean Rhythm"
-    assert base_rig == "clean-rhythm"
-    assert changes == [
-        {"t": 12.5, "name": "Lead Drive", "rig": "lead-drive"},
-        {"t": 48.0, "name": "Clean Rhythm", "rig": "clean-rhythm"},
-    ]
-
-
-def test_sloppak_tone_changes_omits_unusable_rig_ids():
-    """A non-string or blank `rig` is dropped rather than forwarded, so a
-    consumer can treat presence of the key as "this change binds a rig"."""
-    _, base_rig, changes = sloppak_tone_changes({
-        "base_rig": "   ",
-        "changes": [
-            {"t": 1.0, "name": "A", "rig": 7},
-            {"t": 2.0, "name": "B", "rig": ""},
-            {"t": 3.0, "name": "C", "rig": None},
-            {"t": 4.0, "name": "D", "rig": "  padded-id  "},
-        ],
-    })
-    assert base_rig == ""
-    assert changes == [
-        {"t": 1.0, "name": "A"},
-        {"t": 2.0, "name": "B"},
-        {"t": 3.0, "name": "C"},
-        {"t": 4.0, "name": "D", "rig": "padded-id"},
-    ]
-
-
-def test_sloppak_tone_changes_non_string_base_rig():
-    """A non-string `base_rig` must not crash or leak a non-id onto the wire."""
-    _, base_rig, _ = sloppak_tone_changes({"base": "Clean", "base_rig": 42})
-    assert base_rig == ""

tools/check_spec_conformance.py

@@ -58,6 +58,7 @@ READERS = [
     "lib/enrichment.py",
     "lib/songmeta.py",
     "lib/gp2notation.py",        # rewrites manifest.yaml; stamps feedpak_version
+    "lib/gp2rs_gpx.py",          # attach_vocal_sidecars_to_sloppak: lyrics/vocal_pitch keys
     "lib/routers/ws_highway.py", # reads `authors` off a feedpak manifest
     "lib/routers/chart.py",      # Get-info panel: binds `m = load_manifest(...)`
     "lib/routers/song.py",       # enrichment gap-fill: reads the manifest directly