From 7ef52cdd66e44e6edfd371648ef2217cc683a52f Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Thu, 2 Jul 2026 13:51:57 -0500 Subject: [PATCH] fix: Edit Metadata persists into .feedpak files (suffix gate predated the rename) (#725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_song_metadata dispatched zip-form packages on `suffix == ".sloppak"` only, while core reads both suffixes everywhere else (sloppak.SONG_EXTS, .feedpak being the current write extension). Editing a zip-form .feedpak's title/artist/album/year therefore silently fell back to a DB-only update, which looked fine until the next full library rescan re-derived metadata from the file and reverted the edit — the exact failure this module exists to prevent. Directory-form packages were unaffected (manifest-presence dispatch, not suffix). Gate on SONG_EXTS, add TestWriteSongMetadata regression coverage (both zip suffixes, mixed-case suffix, directory form, unknown-suffix fallback), and correct the stale scan_worker comment claiming .sloppak-suffix-only detection (the code already accepts both via is_sloppak). Claude-Session: https://claude.ai/code/session_01H1ZBEcZoJinde9ms5fAjwc Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 1 + lib/scan_worker.py | 13 +++++----- lib/songmeta.py | 13 ++++++---- tests/test_songmeta.py | 56 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8918ea4..47ed7da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Player frame-time hotspots removed (trace-backed) + weak-hardware hardening.** A Chrome performance trace of a 3D-highway session surfaced two core per-frame layout-thrash sources, now fixed: the highway's visibility check read `canvas.offsetParent` every rAF frame (forces style/layout recalc — now sampled every 10th frame with a cached value, force-refreshed on init/canvas-replace/resize/override-clear), and the v3 player chrome loop called `matches(':hover')` per frame and unconditionally rewrote the Up-Next pill's `textContent`/bar width at 6 Hz (now hover-tracked via mouseenter/mouseleave, DOM writes only on value change, progress bar moved from `width` to compositor-only `scaleX`). The 3D highway pre-warms shader programs (`ren.compile`) and deterministic label textures at init — and chart-dependent chord/section label textures on first draw — so first-appearance shader-compile/texture-upload frame spikes move into the load spinner. For weaker hardware: the per-frame renderer bundle is now a single reused object instead of a fresh ~35-field allocation per frame (object identity is stable and meaningless; array fields still swap reference on chart changes), custom viz get `bundle.lowerBoundT`/`bundle.lowerBoundTime` binary-search helpers for visible-window culling, the default 2D highway's beat lines no longer scan every beat in the song per frame, and the 3D highway stops reading `localStorage` per frame (1 Hz poll) and caches its lyrics text-measurement layout per displayed line instead of re-measuring every syllable every frame. A second, throttled-CPU trace pass additionally removed: shader-program re-resolution churn from label texture swaps (`material.needsUpdate` is now only set on a null↔texture transition — swapping between two cached label textures never changes the compiled program), the 3D highway's per-frame `getBoundingClientRect` layout read in its canvas-size self-check (now every 10th frame, still immediate on backing-store change), and the core 60 Hz HUD clock rewriting `textContent` on every tick (now write-on-change, ~1/s). The dominant residual — steady `getParameters` shader-program re-resolution (~4% of throttled main thread) — turned out to be Three r158+'s transparent-DoubleSide two-pass rendering, which sets `material.needsUpdate` twice per object per frame; all 18 of the 3D highway's transparent DoubleSide materials are flat unlit quads (labels, rails, chord frames, lanes), so they now declare `forceSinglePass: true`, eliminating the recompile churn and halving those objects' draw calls. ### Fixed +- **Edit Metadata now writes into `.feedpak` files, not just legacy `.sloppak` ones.** `lib/songmeta.py`'s suffix gate predated the format rename — core reads both suffixes everywhere else (`sloppak.SONG_EXTS`), but the metadata writer only dispatched on `.sloppak`, so editing a zip-form `.feedpak`'s title/artist/album/year silently fell back to a DB-only update. That looked fine until the next **full library rescan** re-derived metadata from the file and reverted the edit (directory-form packages were unaffected — they dispatch on manifest presence, not suffix). The gate now accepts both package suffixes. Tests: `tests/test_songmeta.py` `TestWriteSongMetadata` (both zip suffixes, mixed-case suffix, directory form, unknown-suffix fallback). - **3D Drum & Keys highways now re-frame on fullscreen/layout drift under splitscreen.** The guitar/bass `highway_3d` self-detects when its panel canvas changes size and re-runs `applySize()` every frame, because the splitscreen host overrides `hw.resize` and never calls `renderer.resize()`. The drum and keys highways lacked that fallback — they only re-framed when the host explicitly called `resize(w, h)` — so their panels stayed framed for the pre-fullscreen size while the guitar/bass panels adapted (visible as a too-small, off-center highway after maximizing a split-screen session). Both draw loops now port `highway_3d`'s per-frame drift check: they re-apply on backing-store change (`canvas.width/height`) AND on CSS-box drift (`clientWidth/clientHeight` vs the last applied logical size, throttled to every 10th frame), and reset the tracking in `destroy()` so a reused instance re-frames on the next song. `plugins/drum_highway_3d` → 0.3.1, `plugins/keys_highway_3d` → 0.1.1. Tests: `tests/js/drum_keys_highway_3d_resize_reframe.test.js`. - **Tuner: finished the "remove unused settings" cleanup and fixed the sidebar panel position.** The Floating Button and Tuning Visibility settings sections were removed, but their config was still live: `disabledTunings` still filtered the tuner menu (with no UI left to re-enable a hidden tuning — a one-way trap) and `showFloatingButton` still gated the floating launcher. Both are now fully retired — the enforcement paths in `plugins/tuner/screen.js`/`utils/ui.js` and the persistence in `plugins/tuner/routes.py` are gone (and `routes.py` strips the retired keys on write, so stale values are purged). The tuner panel opened from the v3 sidebar Plugins rail popover now anchors beside it via the host's stable plugin-control slot API (falling back to the popover id), is **clamped to the viewport** so it can't open off the right/bottom edge on narrow/short windows, and re-anchors on window resize. `plugins/tuner` → 1.3.3. diff --git a/lib/scan_worker.py b/lib/scan_worker.py index f119a9b..5715e9b 100644 --- a/lib/scan_worker.py +++ b/lib/scan_worker.py @@ -109,13 +109,14 @@ def _extract_meta_for_file(path: Path, dlc_root=None) -> dict: the root it already resolved; in-process callers can pass the resolver itself (e.g. `_get_dlc_dir`) to keep the lookup lazy. - FeedBack reads only its own `.sloppak` format and loose-folder XML - songs. Encrypted/proprietary archive formats are not supported and are - silently ignored (empty metadata) rather than decrypted. + FeedBack reads only its own song-package format (`.feedpak` / legacy + `.sloppak`) and loose-folder XML songs. Encrypted/proprietary archive + formats are not supported and are silently ignored (empty metadata) + rather than decrypted. """ - # Sloppak is detected by `.sloppak` suffix only (cheap), so check it - # first — that way a user's loose folder named `foo.sloppak` still wins - # the sloppak branch instead of being misclassified. + # Packages are detected by suffix only (`.feedpak`/`.sloppak`, cheap), so + # check that first — that way a user's loose folder named `foo.feedpak` + # still wins the package branch instead of being misclassified. if sloppak_mod.is_sloppak(path): return _extract_meta_sloppak(path) if loosefolder_mod.is_loose_song(path): diff --git a/lib/songmeta.py b/lib/songmeta.py index 7815c64..87301b7 100644 --- a/lib/songmeta.py +++ b/lib/songmeta.py @@ -10,9 +10,9 @@ source of truth, so the change survives both incremental and full rescans. only the keys present are overwritten, so an edit of just the title can't blank out the artist. -Only feedBack's own ``.sloppak`` format (zip- or directory-form) is writable. -Unknown / unsupported shapes return False and the caller keeps the DB-only -update. +Only feedBack's own song-package format (zip- or directory-form, ``.feedpak`` +or the legacy ``.sloppak`` suffix) is writable. Unknown / unsupported shapes +return False and the caller keeps the DB-only update. """ from __future__ import annotations @@ -110,16 +110,19 @@ def write_sloppak_metadata(path: Path, fields: dict) -> bool: def write_song_metadata(path: Path, fields: dict) -> bool: """Persist edited title/artist/album/year into the song's file. - Dispatches by shape: ``.sloppak`` files and sloppak directories + Dispatches by shape: zip-form song packages (``.feedpak`` / legacy + ``.sloppak``, per ``sloppak.SONG_EXTS``) and package directories (manifest.yaml present). Loose-folder and unknown shapes return False (caller keeps the DB-only update). Returns True if the file was modified. """ + from sloppak import SONG_EXTS + path = Path(path) suffix = path.suffix.lower() if path.is_dir(): if (path / "manifest.yaml").exists() or (path / "manifest.yml").exists(): return write_sloppak_metadata(path, fields) return False - if suffix == ".sloppak": + if suffix in SONG_EXTS: return write_sloppak_metadata(path, fields) return False diff --git a/tests/test_songmeta.py b/tests/test_songmeta.py index 90123c5..5628bda 100644 --- a/tests/test_songmeta.py +++ b/tests/test_songmeta.py @@ -1,13 +1,29 @@ """Unit tests for lib/songmeta.py — metadata file-persistence helpers.""" from __future__ import annotations +import zipfile + +import yaml + # songmeta lives in lib/ which is on PYTHONPATH via conftest / pyproject from songmeta import ( _apply_to_sloppak_manifest, _coerce_year, + write_song_metadata, ) +def _make_zip_pak(path, manifest: dict) -> None: + with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as z: + z.writestr("manifest.yaml", yaml.safe_dump(manifest)) + z.writestr("arrangements/lead.json", "{}") + + +def _read_zip_manifest(path) -> dict: + with zipfile.ZipFile(path) as z: + return yaml.safe_load(z.read("manifest.yaml")) + + class TestCoerceYear: def test_int_passthrough(self): assert _coerce_year(2024) == 2024 @@ -62,3 +78,43 @@ class TestApplyToSloppakManifest: manifest = {"title": "T"} dirty = _apply_to_sloppak_manifest(manifest, {}) assert not dirty + + +class TestWriteSongMetadata: + """Suffix dispatch — zip-form packages are writable under BOTH the current + ``.feedpak`` suffix and the legacy ``.sloppak`` one. The ``.feedpak`` gate + was missed when the format was renamed, silently downgrading Edit Metadata + to a DB-only update that a full rescan reverts.""" + + def test_zip_feedpak_persists(self, tmp_path): + pak = tmp_path / "song.feedpak" + _make_zip_pak(pak, {"title": "Old", "artist": "A"}) + assert write_song_metadata(pak, {"title": "New"}) + assert _read_zip_manifest(pak)["title"] == "New" + + def test_zip_sloppak_persists(self, tmp_path): + pak = tmp_path / "song.sloppak" + _make_zip_pak(pak, {"title": "Old", "artist": "A"}) + assert write_song_metadata(pak, {"title": "New"}) + assert _read_zip_manifest(pak)["title"] == "New" + + def test_zip_suffix_case_insensitive(self, tmp_path): + pak = tmp_path / "song.FeedPak" + _make_zip_pak(pak, {"title": "Old"}) + assert write_song_metadata(pak, {"title": "New"}) + assert _read_zip_manifest(pak)["title"] == "New" + + def test_directory_form_persists(self, tmp_path): + pak = tmp_path / "song.feedpak" + pak.mkdir() + (pak / "manifest.yaml").write_text( + yaml.safe_dump({"title": "Old"}), encoding="utf-8" + ) + assert write_song_metadata(pak, {"title": "New"}) + got = yaml.safe_load((pak / "manifest.yaml").read_text(encoding="utf-8")) + assert got["title"] == "New" + + def test_unknown_suffix_returns_false(self, tmp_path): + f = tmp_path / "song.txt" + f.write_text("not a package") + assert not write_song_metadata(f, {"title": "New"})