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>
This commit is contained in:
Kris Anderson
2026-07-13 12:14:08 -04:00
parent 1bf9fa1d49
commit 13eeca6ff8
2 changed files with 71 additions and 2 deletions
+53
View File
@@ -264,6 +264,59 @@ def test_verify_rejects_a_retained_mixdown_that_plays_on_open(tmp_path: Path):
assert mig.verify_zip(pak) == "full-stem-default-on"
@pytest.mark.parametrize(
"default, expected",
[
({"default": "off"}, "ok"), # the one safe, canonical shape
({"default": "OFF"}, "ok"), # case-insensitive
({"default": " off "}, "ok"), # surrounding whitespace tolerated
({}, "full-stem-default-not-off"), # MISSING — core defaults to True (ON)
({"default": ""}, "full-stem-default-not-off"), # empty → ON in core
({"default": False}, "full-stem-default-not-off"), # boolean, not the string
({"default": True}, "full-stem-default-on"), # boolean truthy → plays
({"default": "false"}, "full-stem-default-not-off"), # off-ish but non-canonical
({"default": "0"}, "full-stem-default-not-off"),
({"default": "no"}, "full-stem-default-not-off"),
({"default": "maybe"}, "full-stem-default-not-off"), # malformed
({"default": "on"}, "full-stem-default-on"),
({"default": "yes"}, "full-stem-default-on"),
({"default": "1"}, "full-stem-default-on"),
],
)
def test_verify_requires_an_explicit_off_on_a_retained_mixdown(tmp_path, default, expected):
"""Beside instrument stems, `full` is safe only with an explicit normalized
`off`. Core defaults an ABSENT `default` to ON and treats empty/unknown as
ON, so a missing or blank default is the double-audio hazard itself, not a
lesser one — `verify` must not certify it."""
m = _manifest(
stems=[
{"id": "full", "file": "stems/full.ogg", **default},
{"id": "guitar", "file": "stems/guitar.ogg", "default": "on"},
]
)
del m["original_audio"]
pak = _write_pack(
tmp_path / f"{tmp_path.name}.feedpak",
m,
files={"stems/full.ogg": b"M", "stems/guitar.ogg": b"g"},
)
assert mig.verify_zip(pak) == expected
def test_verify_ignores_default_on_a_sole_full_stem(tmp_path: Path):
"""A single `full` stem IS the audio — the len>1 gate means its default is
not policed, so an on/absent default is fine (off would mute the pack)."""
for default in ({"default": "on"}, {}, {"default": ""}):
m = _manifest(stems=[{"id": "full", "file": "stems/full.ogg", **default}])
del m["original_audio"]
pak = _write_pack(
tmp_path / f"{tmp_path.name}-{len(default)}.feedpak",
m,
files={"stems/full.ogg": b"M"},
)
assert mig.verify_zip(pak) == "ok"
def test_verify_rejects_an_unmigrated_pack(tmp_path: Path):
pak = _write_pack(tmp_path / "song.feedpak", _manifest())
assert mig.verify_zip(pak) == "still-has-key"