fix(sloppak): the full mix is a stem — drop the invented original_audio key (#946)

* fix(sloppak): the full mix is a stem — drop the invented `original_audio` key (#933)

Core read, served, and depended on `original_audio:` — a top-level manifest key
this repo invented in #583 that the feedpak spec never defined. The format
already had a home for the pre-separation mixdown: it is a stem. feedpak 1.15.0
(feedpak-spec#53) RESERVES the id `full` for it, so read it from there.

The key existed to work around a bug in our own reader. The packer's comment
said so plainly: "we must NOT list the full mix as a playable stem — the player
sums every entry in `stems` and does not gate playback on `default`, so a listed
full mix plays on top of the stems". Faced with a reader that would double the
song, the packer put the mixdown outside `stems` and invented a key to point at
it. The fix belongs in the reader, and that is what this is.

load_song() now partitions the stem list: `full` comes out as
LoadedSloppak.full_mix, the instruments stay in .stems. Nothing that sums stems
or draws one fader per stem can see the mixdown, so retaining it is safe — which
is what lets the packer put it where the format says it goes.

- ws_highway: `song_info` gains full_mix_url / has_full_mix. The old
  original_audio_url / has_original_audio remain as deprecated aliases for one
  release so an older stems plugin keeps working (#945).
- `stems` on the wire, and stem_ids / stem_count in the library index, are now
  INSTRUMENT stems only — a separated pack that retains its mixdown no longer
  advertises a bogus "full" chip or an inflated stem count.
- enrichment: fingerprint against the mixdown wherever it lives. This widens
  coverage — _song_audio_file() previously returned None for any pack without
  the invented key, so fingerprinting silently did nothing for nearly every pack.
- sloppak: `original_audio:` is still READ as a deprecated fallback, because
  every pack in the wild carries it and would otherwise lose its pristine mix.
  tools/migrate_full_mix_stem.py rewrites those packs into the spec shape
  (original/full.ogg -> stems/full.ogg, add the `full` stem at default:off, drop
  the key); the fallback and the aliases die with #945.

The spec gate keeps the debt honest: the grandfather entry now tracks #945, and
the gate fails if it goes stale.

Verified: spec gate OK (4/4, incl. ingesting the spec's new example pack that
retains `full`); 2493 python tests, 995 js tests; migrator round-tripped over
real packs from the library and the results pass the spec's reference validator.

* fix(migrate): discover directory-form packs instead of silently skipping them

iter_packs() searched only files, so a directory-form pack (`song.sloppak/`, the
authoring shape) was walked INTO and never yielded — silently missed by a run
that's meant to be exhaustive. Discover suffix-named directories too (yielded
whole, not descended into), and route packs through migrate_pack/verify_pack.

Directory packs are REPORTED as `dir-form-unsupported`, not rewritten in place:
a single-file pack is replaced atomically (a fully-built temp archive swapped in
with one os.replace), but a populated directory can't be swapped that way, so an
interrupted in-place rewrite could leave an authoring pack half-migrated. The
status is a problem status, so it counts against the run's exit code and shows
in the summary — the operator re-packs or migrates it as a `.feedpak` instead of
it vanishing from the report. Addresses a CodeRabbit review finding.

Signed-off-by: Kris Anderson <topkoa@gmail.com>

* fix(migrate): verify requires an explicit `off` on a retained full mix

verify_zip accepted any non-truthy `default` on a multi-stem `full` (missing,
empty, boolean, `false`/`no`/`0`, malformed) as "ok". But core defaults an ABSENT
`default` to True — ON (lib/sloppak.py: `s.get("default", True)`) — and treats an
empty/unrecognized string as ON too, so a migrated-shape pack whose `full` stem
has a missing or blank default beside instrument stems would actually play the
mixdown on open and double the song. verify was certifying that as safe.

Require an explicit normalized `off` beside instrument stems: `on`-ish values are
reported `full-stem-default-on` (actively plays), everything that is not a
normalized `off` is reported `full-stem-default-not-off`. The migrator already
writes the literal `off`, so its own output is unaffected; this also certifies
the pack is in the tool's canonical, most-portable shape. The len>1 gate is kept,
so a sole `full` stem (which IS the audio) is not policed.

Adds parametrized coverage for missing / empty / boolean / off-ish / malformed
defaults, and a sole-full-stem case. Addresses a CodeRabbit review finding.

Signed-off-by: Kris Anderson <topkoa@gmail.com>

---------

Signed-off-by: Kris Anderson <topkoa@gmail.com>
Co-authored-by: Kris Anderson <topkoa@gmail.com>

lib/enrichment.py

@@ -368,10 +368,12 @@ def _acoustid_gate() -> "JSONResponse | None":
 
 def _song_audio_file(filename: str) -> "str | None":
     """Resolve a LIBRARY song (by filename/id) to a local master-audio file for
-    fingerprinting: the full-mix `original_audio` extracted from a sloppak, or a
-    loose folder's audio. None when the song can't be found or ships no full-mix
-    audio (some packs carry only stems). Mirrors serve_sloppak_file's containment
-    guards so a crafted filename can't read outside DLC_DIR / the pack."""
+    fingerprinting: a sloppak's complete mixdown, or a loose folder's audio. None
+    when the song can't be found or carries no mixdown (a pack that kept only its
+    separated stems — an acoustic fingerprint of one re-summed from them would not
+    match the recording, so we decline rather than submit a lossy reconstruction).
+    Mirrors serve_sloppak_file's containment guards so a crafted filename can't
+    read outside DLC_DIR / the pack."""
     dlc = _get_dlc_dir()
     if not dlc:
         return None
@@ -383,7 +385,20 @@ def _song_audio_file(filename: str) -> "str | None":
             canon = resolved.relative_to(dlc.resolve()).as_posix()
         except ValueError:
             return None
-        rel = (sloppak_mod.load_manifest(resolved) or {}).get("original_audio")
+        manifest = sloppak_mod.load_manifest(resolved) or {}
+        # The mixdown is the RESERVED `full` stem (spec §5.3). Unlike playback,
+        # fingerprinting wants it even when it is the pack's ONLY stem — a
+        # single-mix pack is exactly the master audio we want to fingerprint —
+        # so this asks find_full_mix() rather than partition_stems().
+        stems = manifest.get("stems") or []
+        full = sloppak_mod.find_full_mix(
+            [s for s in stems if isinstance(s, dict)]
+        )
+        rel = full.get("file") if full else None
+        # DEPRECATED fallback: packs written before the spec reserved `full` put
+        # the mixdown behind a top-level `original_audio:` key instead (#933).
+        if not isinstance(rel, str) or not rel.strip():
+            rel = manifest.get("original_audio")
         if not isinstance(rel, str) or not rel.strip():
             return None
         src = sloppak_mod.get_cached_source_dir(canon)

lib/routers/ws_highway.py

@@ -321,11 +321,16 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1,
         audio_url = None
         audio_error: str | None = None  # Surfaced in song_info when audio_url is None
         stems_payload: list[dict] = []
-        # URL of the single full-mix audio (sloppak `original_audio:`), when the
-        # pack ships one. The stems plugin uses this to play the untouched mix
-        # while every stem slider is at unity; None otherwise (separate stems
-        # only, loose folder, or archive).
-        original_audio_url: str | None = None
+        # URL of the pack's complete mixdown — the RESERVED `full` stem (spec
+        # §5.3), which sloppak.load_song() lifts out of `stems` because it is a
+        # mixdown, not a layer. The stems plugin plays it while every stem slider
+        # is at unity (separation is lossy, so it beats re-summing the stems) and
+        # crosses to the separated stems as soon as one is attenuated.
+        #
+        # None when the pack has no mixdown to offer separately from its stems:
+        # a single-mix pack (its one stem IS the mixdown), a loose folder, or an
+        # archive.
+        full_mix_url: str | None = None
         if is_loose:
             # Loose folder filenames are relative paths (artist/album/song).
             # Hash the *canonical* dlc-relative path (so two URL spellings
@@ -365,21 +370,25 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1,
                 url = f"/api/sloppak/{q_fn}/file/{quote(s['file'])}"
                 stems_payload.append({"id": s["id"], "url": url, "default": s["default"]})
             # Full-mix URL (served by the same /api/sloppak/.../file/ endpoint).
-            if loaded_slop is not None and loaded_slop.original_audio:
-                original_audio_url = (
-                    f"/api/sloppak/{q_fn}/file/{quote(loaded_slop.original_audio)}"
+            if loaded_slop is not None and loaded_slop.full_mix:
+                full_mix_url = (
+                    f"/api/sloppak/{q_fn}/file/{quote(loaded_slop.full_mix)}"
                 )
             if stems_payload:
                 # Stems present: keep the core <audio> pointed at stem[0]. This
                 # URL is only ever heard in the degraded path (stems plugin
                 # refuses takeover / decode fails); the full-mix↔stems switch is
-                # driven client-side by `original_audio_url`, not `audio_url`.
+                # driven client-side by `full_mix_url`, not `audio_url`.
                 audio_url = stems_payload[0]["url"]
-            elif original_audio_url:
+            elif full_mix_url:
                 # Stem-less full-mix pack: nothing to separate, so play the full
                 # mix natively through the core <audio>. The stems plugin's
                 # onSongReady returns early on an empty stems list (no graph).
-                audio_url = original_audio_url
+                # Reachable only via the deprecated `original_audio:` key, whose
+                # packs put the mixdown outside `stems` — a pack that carries its
+                # mixdown as the `full` stem has it IN `stems`, so it lands in the
+                # branch above with stems_payload == [full].
+                audio_url = full_mix_url
             else:
                 audio_error = "This sloppak has no playable stems."
         else:
@@ -521,16 +530,31 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1,
             # for the credits overlay, so minigames / synthetic highway uses
             # (no manifest) never trigger it.
             "authors": _sanitize_authors(loaded_slop.manifest) if (is_slop and loaded_slop is not None) else [],
+            # Instrument stems ONLY. The pack's complete mixdown (the RESERVED
+            # `full` stem, spec §5.3) is deliberately NOT in this list: consumers
+            # sum `stems` into one mix and render one fader per entry, and the
+            # mixdown is neither a layer nor an instrument — summing it would
+            # double the whole song. It is surfaced separately, below.
             "stems": stems_payload,
-            # Full-mix audio (sloppak `original_audio:`) served alongside the
-            # separate `stems`. The stems plugin plays this single file while
-            # every stem slider is at unity and switches to the separate stems
-            # the moment one drops below 100%. None when the pack ships stems
-            # only. `has_*` flags mirror the has_drum_tab/has_keys convention so
-            # a client can branch without re-deriving from the URLs.
-            "original_audio_url": original_audio_url,
-            "has_original_audio": bool(original_audio_url),
+            # The complete mixdown, served by the same /api/sloppak/.../file/
+            # endpoint as the stems. The stems plugin plays this single file
+            # while every stem slider is at unity and crosses to the separated
+            # stems the moment one drops below 100% — separation is lossy, so the
+            # mixdown is strictly better audio when nothing is muted. None when
+            # the pack has no mixdown apart from its stems. The `has_*` flags
+            # mirror the has_drum_tab/has_keys convention so a client can branch
+            # without re-deriving from the URLs.
+            "full_mix_url": full_mix_url,
+            "has_full_mix": bool(full_mix_url),
             "has_stems": bool(stems_payload),
+            # DEPRECATED aliases of the two keys above, kept so a client built
+            # against the old frame keeps working across one release. They were
+            # named after `original_audio:` — a manifest key this repo invented
+            # and the feedpak spec never had (#933). The key is gone; the mixdown
+            # is a stem. Remove these once the shipped stems plugin reads
+            # `full_mix_url` (#945).
+            "original_audio_url": full_mix_url,
+            "has_original_audio": bool(full_mix_url),
             # Surface a drum_tab presence flag so the visualization picker
             # can auto-activate the drums plugin even when the chosen
             # arrangement isn't named "Drums" (drum_tab.json lives next

lib/sloppak.py

@@ -35,6 +35,21 @@ FEEDPAK_EXT = ".feedpak"
 SLOPPAK_EXT = ".sloppak"
 SONG_EXTS = (FEEDPAK_EXT, SLOPPAK_EXT)  # accepted on read/discovery
 
+# ── The full mix ──────────────────────────────────────────────────────────────
+#
+# Spec §5.3 RESERVES the stem id `full` for the song's complete mixdown: the
+# whole song in one file, as heard before source separation. It is a stem — it
+# lives in `stems` like every other audio file in a pack — but it is a *mixdown,
+# not a layer*. A reader that sums stems must never include it in the sum: it
+# already contains every instrument, so summing it doubles the whole song and
+# muting `guitar` still leaves guitar audible inside it.
+#
+# Keeping it matters because separation is lossy: re-summing guitar+bass+drums+
+# vocals does NOT reproduce the file they came from. The mixdown is the only
+# faithful rendering of the song a pack can carry, so we play it whenever every
+# stem sits at unity and nothing is muted.
+FULL_MIX_STEM_ID = "full"
+
 import yaml
 
 from jsonc import load_json
@@ -52,6 +67,97 @@ import drums as drums_mod
 import notation as notation_mod
 
 
+def find_full_mix(stems: list[dict]) -> dict | None:
+    """The RESERVED `full` stem (spec §5.3) — the pack's complete mixdown — or None.
+
+    Answers "what is this pack's master audio", which is what fingerprinting
+    wants. For playback use partition_stems() instead: a pack whose *only* stem
+    is `full` has no mixdown to play *separately from* its stems, and this
+    function still returns it.
+    """
+    return next(
+        (s for s in stems if str(s.get("id", "")) == FULL_MIX_STEM_ID), None
+    )
+
+
+def partition_stems(stems: list[dict]) -> tuple[dict | None, list[dict]]:
+    """Split stem descriptors into (mixdown, instrument_stems) for PLAYBACK.
+
+    The mixdown is lifted OUT of the stem list because every consumer of `stems`
+    treats that list as layers to sum or to show as mixer channels, and `full` is
+    neither (spec §5.3). Leaving it in is precisely the bug that made the packer
+    invent `original_audio` in the first place: a listed full mix plays on top of
+    the stems.
+
+    A pack whose only stem is `full` is a single-mix pack, not a separated one:
+    there are no instruments to be pristine *against*, so `full` stays the sole
+    playable stem and no mixdown is surfaced. That keeps the freshly-converted
+    single-stem pack — much the most common shape — behaving exactly as before.
+
+    EVERY entry with the reserved id is removed, not just the one we surface. A
+    malformed pack that lists `full` twice would otherwise leave a copy of the
+    whole song behind in the stem list, to be summed with the instruments — the
+    precise failure this function exists to prevent, reintroduced by a duplicate.
+    """
+    if len(stems) < 2:
+        return None, stems
+    full = find_full_mix(stems)
+    if full is None:
+        return None, stems
+    return full, [s for s in stems if str(s.get("id", "")) != FULL_MIX_STEM_ID]
+
+
+def _legacy_full_mix(manifest: dict, source_dir: Path) -> str | None:
+    """Full mix from the DEPRECATED `original_audio:` manifest key, or None.
+
+    Before feedpak 1.15.0 reserved `full`, §5.3 said the mixdown was "commonly
+    replaced" by the per-instrument stems on splitting — so it had nowhere to
+    live, and this repo invented a top-level key pointing at a parallel
+    `original/` directory (#583) to hold it. That key was never in the spec, and
+    #933 removed our dependence on it: the mixdown is a stem.
+
+    We still READ it, because every pack written before the spec caught up
+    carries `original_audio: original/full.ogg` and would otherwise lose its full
+    mix. We never write it. Delete this once those packs are migrated (#945);
+    `tools/migrate_full_mix_stem.py` is the migration.
+
+    NOTE the string literal below. tools/check_spec_conformance.py AST-scans for
+    `manifest.get("<literal>")` to prove every manifest key core reads is one the
+    spec declares. Hoisting "original_audio" into a named constant would hide
+    this read from that scan — the gate would conclude core no longer touches the
+    key, and the grandfather entry that documents this debt would go stale. The
+    literal is what keeps the deprecation honest and visible to CI. Leave it.
+
+    Same permissive, path-traversal-guarded posture as the optional side-files: a
+    missing / escaping / unreadable file leaves the pack without a full mix (the
+    player falls back to the separated stems) rather than aborting the load.
+    Returns the manifest-relative string, so callers build its URL exactly as
+    they build a stem's.
+    """
+    rel_raw = manifest.get("original_audio")
+    if not isinstance(rel_raw, str) or not rel_raw.strip():
+        return None
+    rel = rel_raw.strip()
+    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 "
+        "is a stem (id `full`, feedpak spec §5.3). Re-pack with "
+        "tools/migrate_full_mix_stem.py; support for this key will be removed.",
+        rel,
+    )
+    return rel
+
+
 # ── Format detection ──────────────────────────────────────────────────────────
 
 def is_sloppak(path: Path) -> bool:
@@ -595,14 +701,21 @@ class LoadedSloppak:
     # song.arrangements (not to manifest["arrangements"]) — skipped entries are
     # absent so indexing by song.arrangements index is safe.
     arrangement_ids: list[str | None] = field(default_factory=list)
-    # Manifest-relative path to the single full-mix audio file, taken from the
-    # manifest `original_audio:` key (e.g. "original/full.ogg"). This is the
-    # pre-separation mixdown that exists alongside the per-instrument `stems`.
-    # None when the key is absent, points outside source_dir, or the file is
-    # missing on disk. Served to the front-end via the highway WS as
-    # `original_audio_url`; the stems plugin uses it to play the untouched mix
-    # when every stem slider is at unity (and the separate stems otherwise).
-    original_audio: str | None = None
+    # Manifest-relative path to the pack's complete mixdown — the whole song in
+    # one file, as heard before source separation. This is the RESERVED `full`
+    # stem (spec §5.3), lifted out of `stems` above precisely because it is NOT
+    # an instrument layer: summing it with the per-instrument stems it was split
+    # into would double the entire song. See partition_stems().
+    #
+    # None when the pack has no mixdown to offer *separately* from its stems —
+    # which includes the common single-mix pack, whose only stem IS the mixdown
+    # (there is nothing to be pristine against, so it stays in `stems`).
+    #
+    # Served to the front-end via the highway WS as `full_mix_url`; the stems
+    # plugin plays it while every stem slider sits at unity and crosses to the
+    # separated stems the moment one drops below 100% — demucs recombination is
+    # lossy, so the mixdown is strictly the better audio when nothing is muted.
+    full_mix: str | None = None
 
 
 def load_song(
@@ -994,6 +1107,13 @@ def load_song(
             default_on = bool(default_val)
         stems.append({"id": sid, "file": sfile, "default": default_on})
 
+    # The complete mixdown is a stem (spec §5.3), but it is not a *layer*: lift
+    # it out so that no consumer of `stems` — the mixer, the library's stem
+    # chips, the WS payload — sums it with, or lists it beside, the instruments
+    # it was separated into. `full_mix_stem` is None for a single-mix pack,
+    # whose only stem IS the mixdown and stays in the list.
+    full_mix_stem, stems = partition_stems(stems)
+
     # Optional keys.json — song-level, instrument-independent key/scale track
     # (manifest `keys:` key, spec §7.7). Permissive like the other side-files:
     # missing / unreadable / malformed -> None, never fatal. Stored as a
@@ -1056,28 +1176,22 @@ def load_song(
                     }
 
     _fpv = manifest.get("feedpak_version")
-    # Optional full-mix audio — manifest `original_audio:` key. The single
-    # pre-separation mixdown that ships alongside the per-instrument stems.
-    # Same permissive, path-traversal-guarded posture as drum_tab above: a
-    # missing/escaping/absent file simply leaves the full mix unavailable (the
-    # player falls back to the separate stems) rather than aborting the load.
-    # We store the manifest-relative string so server.py can build its URL the
-    # same way it builds stem URLs (via the /api/sloppak/.../file/ endpoint).
-    original_audio_data: str | None = None
-    original_audio_rel = manifest.get("original_audio")
-    if isinstance(original_audio_rel, str) and original_audio_rel.strip():
-        rel = original_audio_rel.strip()
-        try:
-            oa_path = (source_dir / rel).resolve()
-            oa_path.relative_to(source_dir.resolve())
-        except ValueError:
-            log.warning("sloppak: original_audio path %r escapes source_dir — skipped", rel)
-            oa_path = None
-        except OSError as e:
-            log.warning("sloppak: original_audio path resolution failed (%s) — skipped", e)
-            oa_path = None
-        if oa_path is not None and oa_path.is_file():
-            original_audio_data = rel
+    # 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
+    # stems and its URL is built the same way. Only when the pack has no `full`
+    # stem do we fall back to the DEPRECATED `original_audio:` key, which is the
+    # shape every pack written before feedpak 1.15.0 uses.
+    if full_mix_stem is not None:
+        full_mix_data: str | None = full_mix_stem["file"]
+    elif find_full_mix(stems) is not None:
+        # Single-mix pack: its ONE stem is the mixdown, so there is no mixdown to
+        # offer *apart from* the stems. Never fall through to the legacy key here
+        # — a pack that both carries a `full` stem and names the old key would
+        # otherwise surface the mixdown twice (once as the stem the player is
+        # already playing, once as a "pristine" track to cross to).
+        full_mix_data = None
+    else:
+        full_mix_data = _legacy_full_mix(manifest, source_dir)
 
     return LoadedSloppak(
         song=song,
@@ -1092,7 +1206,7 @@ def load_song(
         keys=keys_data,
         notation_by_id=notation_by_id_data,
         arrangement_ids=arrangement_ids_acc,
-        original_audio=original_audio_data,
+        full_mix=full_mix_data,
     )
 
 
@@ -1137,7 +1251,7 @@ def extract_meta(path: Path) -> dict:
     tuning_offsets = _tuning_for_meta(arr_list)
 
     stems_list = manifest.get("stems", []) or []
-    stem_ids: list[str] = []
+    valid_stems: list[dict] = []
     for s in stems_list:
         if not isinstance(s, dict):
             continue
@@ -1151,7 +1265,13 @@ def extract_meta(path: Path) -> dict:
             isinstance(sid, str) and sid
             and isinstance(sfile, str) and sfile
         ):
-            stem_ids.append(sid)
+            valid_stems.append({"id": sid, "file": sfile})
+    # Partition exactly as load_song() does, for the same reason the library
+    # filter must not lie: `full` is the mixdown, not an instrument (spec §5.3).
+    # A separated pack that retains it would otherwise offer the user a "full"
+    # stem chip alongside guitar/bass/drums and count it as a seventh stem.
+    _full, instrument_stems = partition_stems(valid_stems)
+    stem_ids = [s["id"] for s in instrument_stems]
     stem_count = len(stem_ids)
 
     return {