feat(core): adopt feedpak_version — read on load + stamp on manifest writes (spec §4) (#530)

Core never read or emitted the manifest `feedpak_version` field. Adopt it:

- sloppak.py: `FEEDPAK_VERSION = "1.2.0"` constant (the format version this build
  targets); `LoadedSloppak.feedpak_version` read from the manifest on load
  (string, else None for legacy/absent).
- Stamp the version on the two core manifest-rewrite paths, without downgrading
  an existing (possibly higher) declared version:
  - gp2notation: `setdefault` before its notation-add rewrite.
  - songmeta: opportunistically when a metadata field is supplied (gated on the
    existing `dirty` flag, so never a standalone rewrite).

Core has no create-from-scratch path (RS-free repo) — the editor plugin's
create-mode save stamping FEEDPAK_VERSION is a follow-up in that repo. Internal
"sloppak" naming is intentionally left as-is (a rename is out of scope / risky).

Codex-reviewed: no P1/P2. +6 tests (read present/absent/non-string; metadata-write
stamp-when-absent / preserve-existing / no-op-no-stamp) + updated the gp2notation
key-order test for the appended version. 197 sloppak/songmeta/gp2notation tests pass.

Closes #527. Part of got-feedback/feedback#334.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos 2026-06-20 21:59:04 +02:00 committed by GitHub
parent 587fbbea81
commit b8382139ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 115 additions and 2 deletions

View File

@ -522,6 +522,10 @@ def attach_notation_to_sloppak(sloppak_dir: str | Path, arr_id: str, payload: di
json.dumps(payload, separators=(",", ":")), encoding="utf-8"
)
entry["notation"] = filename
# Stamp the format version while we're rewriting the manifest (spec §4),
# without downgrading an existing (possibly higher) 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",

View File

@ -24,6 +24,11 @@ from pathlib import Path
log = logging.getLogger("slopsmith.lib.sloppak")
# The feedpak format version this build targets / writes (manifest
# `feedpak_version`, a semver string per spec §4). Readers tolerate any version
# (additive/MINOR compatibility); writers stamp this.
FEEDPAK_VERSION = "1.2.0"
import yaml
from safepath import safe_join
@ -316,6 +321,9 @@ class LoadedSloppak:
stems: list[dict] # [{"id": str, "file": str, "default": bool}]
source_dir: Path
manifest: dict
# The pack's declared format version (manifest `feedpak_version`, a semver
# string per spec §4). None when absent (legacy / pre-versioning packs).
feedpak_version: str | None = None
# Parsed `drum_tab.json` payload when the manifest carries a `drum_tab:`
# key pointing at a readable, schema-valid file. None otherwise (older
# sloppaks, sloppaks without drums, sloppaks whose drum tab failed to
@ -789,11 +797,13 @@ def load_song(
"events": clean_events,
}
_fpv = manifest.get("feedpak_version")
return LoadedSloppak(
song=song,
stems=stems,
source_dir=source_dir,
manifest=manifest,
feedpak_version=_fpv if isinstance(_fpv, str) and _fpv else None,
drum_tab=drum_tab_data,
song_timeline=song_timeline_data,
tempos=tempos_data,

View File

@ -49,6 +49,15 @@ def _apply_to_sloppak_manifest(manifest: dict, fields: dict) -> bool:
if "year" in fields:
manifest["year"] = _coerce_year(fields["year"])
dirty = True
# Opportunistically declare the format version (spec §4) when we're already
# rewriting because a metadata field was supplied. Gated on `dirty` (i.e. a
# field was given) so this never forces a *standalone* rewrite with no fields
# passed, and `not in` so an existing (possibly higher) version is preserved,
# never downgraded. NB `dirty` here means "a field was supplied" — a
# supplied-but-identical value already triggers a rewrite (pre-existing).
if dirty and "feedpak_version" not in manifest:
from sloppak import FEEDPAK_VERSION
manifest["feedpak_version"] = FEEDPAK_VERSION
return dirty

View File

@ -502,8 +502,12 @@ def test_attach_notation_to_sloppak(tmp_path):
entries = {e["id"]: e for e in rewritten["arrangements"]}
assert entries["keys"]["notation"] == "notation_keys.json"
assert "notation" not in entries["lead"]
# Key order preserved (sort_keys=False round-trip).
assert list(rewritten.keys()) == ["title", "artist", "arrangements", "stems"]
# Original key order preserved (sort_keys=False round-trip); the manifest
# rewrite also stamps feedpak_version (spec §4), appended at the end.
assert list(rewritten.keys()) == [
"title", "artist", "arrangements", "stems", "feedpak_version"]
from sloppak import FEEDPAK_VERSION
assert rewritten["feedpak_version"] == FEEDPAK_VERSION
def test_attach_notation_unknown_arrangement_raises(tmp_path):

View File

@ -0,0 +1,86 @@
"""feedpak_version (spec §4): read on load + opportunistic stamp on a metadata
write. Core has no create-from-scratch path (RS-free repo); the editor plugin's
create-mode save stamping the version is a separate follow-up."""
from __future__ import annotations
import json
from pathlib import Path
import yaml
import sloppak as sloppak_mod
from sloppak import FEEDPAK_VERSION
from songmeta import write_sloppak_metadata
def _write_dir_sloppak(root: Path, manifest_extras: dict) -> Path:
pak = root / f"{root.name}.sloppak"
pak.mkdir()
arr_dir = pak / "arrangements"
arr_dir.mkdir()
(arr_dir / "lead.json").write_text(json.dumps({
"name": "Lead", "tuning": [0, 0, 0, 0, 0, 0], "capo": 0,
"notes": [], "chords": [], "anchors": [], "handshapes": [],
"templates": [], "beats": [], "sections": [],
}))
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))
return pak
def _load(pak: Path, tmp_path: Path):
cache = tmp_path / "cache"
cache.mkdir()
return sloppak_mod.load_song(pak.name, pak.parent, cache)
def _manifest(pak: Path) -> dict:
return yaml.safe_load((pak / "manifest.yaml").read_text(encoding="utf-8"))
# ── read ─────────────────────────────────────────────────────────────────────
def test_feedpak_version_read_from_manifest(tmp_path: Path):
pak = _write_dir_sloppak(tmp_path, {"feedpak_version": "1.2.0"})
assert _load(pak, tmp_path).feedpak_version == "1.2.0"
def test_feedpak_version_none_when_absent(tmp_path: Path):
pak = _write_dir_sloppak(tmp_path, {})
assert _load(pak, tmp_path).feedpak_version is None
def test_feedpak_version_none_when_not_a_string(tmp_path: Path):
pak = _write_dir_sloppak(tmp_path, {"feedpak_version": 12})
assert _load(pak, tmp_path).feedpak_version is None
# ── opportunistic stamp on a metadata write ──────────────────────────────────
def test_metadata_write_stamps_version_when_absent(tmp_path: Path):
pak = _write_dir_sloppak(tmp_path, {})
assert "feedpak_version" not in _manifest(pak)
assert write_sloppak_metadata(pak, {"title": "New"}) is True
m = _manifest(pak)
assert m["title"] == "New"
assert m["feedpak_version"] == FEEDPAK_VERSION
def test_metadata_write_preserves_existing_version(tmp_path: Path):
pak = _write_dir_sloppak(tmp_path, {"feedpak_version": "9.9.9"})
write_sloppak_metadata(pak, {"artist": "X"})
assert _manifest(pak)["feedpak_version"] == "9.9.9" # not downgraded
def test_metadata_no_change_does_not_add_version(tmp_path: Path):
# A no-op metadata write must NOT stamp a version (no rewrite happens).
pak = _write_dir_sloppak(tmp_path, {})
assert write_sloppak_metadata(pak, {}) is False
assert "feedpak_version" not in _manifest(pak)