mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-22 21:01:40 +00:00
* feat(library): metadata override + lock store, enforced by enrichment (popup slices 1–2)
Backend foundation for the Fix-metadata popup. Not yet surfaced in the UI (the
display + 3-tab popup are the next slices); no PR until it's user-visible.
Slice 1 — the store:
- `song_field_override(filename, field, value, locked)` table: a reversible
DISPLAY overlay (never written to the pack), filename-keyed so it survives a
rescan (never purged by delete_missing) and is dropped only with the song.
- DB methods (partial upsert that drops empty+unlocked rows; batch map) +
`GET`/`PUT /api/song/{fn}/overrides` (field allowlist title/artist/album/
year/genre; clearing rides PUT since DELETE /api/song/{path} shadows sub-
routes; PUT demo-blocked).
Slice 2 — locks respected by enrichment:
- The auto-matcher composes a per-song `_compose_lock_filter` onto the global
apply-filter, so a match still applies IDENTITY (mbid/release → art) but never
re-canonicalizes a LOCKED display field.
- Gap-fill (write-to-file) skips locked album/year/genre — writing the matched
value would be exactly the clobber the lock exists to prevent.
- Review/manual picks bypass the filter (an explicit confirm overrides a lock).
Tests: store semantics + rescan-survival + API; the lock filter + reader; an
auto-match leaving a locked field un-canonicalized; gap-fill excluding locked
keys.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(library): show per-song overrides in the grid (popup slice 3)
The grid now displays the user's per-song title/artist/album/year override
in place of the pack value ("grid shows only overrides") — a matched
MusicBrainz canon never silently re-titles a card; canon stays in the
Details drawer + art. Overlaid in Python over the visible window, keyset-safe
like the P4 artist-alias re-label: the seek still runs on the raw column, and
the one overridable keyset column (title) stashes its raw value for the cursor
so paging never skips/dupes. The private stash is dropped from the payload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(library): 3-tab Fix-metadata popup — Details / Cover art / Match (slice 4)
Turns the thin single-song fix-match modal into the Plex-style metadata
editor reached from a card's "Fix metadata…" menu:
- Details tab: type + lock the displayed title/artist/album/year. Values
ride the reversible override store (GET/PUT /api/song/{fn}/overrides); each
field sits on its pack value (Yours/Pack provenance + revert-to-pack), a lock
pins it against auto-match, and Save repaints the grid via library:changed
(slice-3 overlay). This is the real tool for the blank-artist city-pop pile
MusicBrainz can't surface — you just type the right title.
- Cover art tab: hands off to the shared image picker (its own modal); the
pick refreshes the thumbnail everywhere.
- Match tab: the existing MusicBrainz search + candidate/pick flow, refactored
into shared body/footer helpers (the queue-review flow is untouched).
Backend: GET /overrides now also returns the pack baseline so the Details tab
can pre-fill + show provenance. tailwind.min.css regenerated (build-tailwind.sh)
for the popup's new utility classes.
Identify-by-audio (AcoustID) is deferred: it lives in unmerged PR #759, off
main — the Match tab gains the button once #759 lands and this branch rebases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(library): wire "Identify by audio" in the tabbed popup's Match tab
The AcoustID Identify button (#759) merged in referencing an out-of-scope
`panel` in the wiring — a leftover from the pre-popup fix-match modal that my
tab refactor renamed to `root`. Under strict mode that threw, so the handler
never attached and the button did nothing. Scope it to `root` (the tab body),
which is where the search-results area it renders into lives.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(library): make "Identify by audio" outcomes unmistakable
An empty AcoustID result read the same as a broken button. Each state now says
plainly which outcome it is — ✓ fingerprinted-but-no-match vs no-audio vs off vs
unavailable — and, in the popup, points at the manual fallback (Search, or set
the album in Details + cover in Cover art by hand). A ✓ marks the states that
actually ran, so "worked, found nothing" no longer looks like a failure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
275 lines
12 KiB
Python
275 lines
12 KiB
Python
"""Tests for the R4a gap-fill write-back — the §7 contract made executable:
|
|
user-initiated, adds ABSENT keys only (author bytes preserved verbatim),
|
|
spec'd-keys allowlist, values only from a CONFIRMED match, atomic + .bak.
|
|
|
|
No network anywhere: matches are seeded straight into the enrichment cache
|
|
(as the P8 matcher would have written them).
|
|
"""
|
|
|
|
import importlib
|
|
import sys
|
|
import zipfile
|
|
|
|
import pytest
|
|
import yaml
|
|
from fastapi.testclient import TestClient
|
|
|
|
|
|
@pytest.fixture()
|
|
def server(tmp_path, monkeypatch, isolate_logging):
|
|
monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
|
|
dlc = tmp_path / "dlc"
|
|
dlc.mkdir()
|
|
monkeypatch.setenv("DLC_DIR", str(dlc))
|
|
monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
|
|
sys.modules.pop("server", None)
|
|
srv = importlib.import_module("server")
|
|
try:
|
|
yield srv
|
|
finally:
|
|
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
|
|
if conn is not None:
|
|
getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
|
|
conn.close()
|
|
sys.modules.pop("server", None)
|
|
|
|
|
|
@pytest.fixture()
|
|
def client(server):
|
|
return TestClient(server.app)
|
|
|
|
|
|
BASE_MANIFEST = ("# my hand-made pack\n"
|
|
"title: Thunderstruck\n"
|
|
"artist: AC/DC # the real one\n"
|
|
"duration: 292\n"
|
|
"arrangements: []\n"
|
|
"stems: []\n")
|
|
|
|
|
|
def make_dir_sloppak(server, name, manifest=BASE_MANIFEST):
|
|
d = server.DLC_DIR / name
|
|
d.mkdir(parents=True)
|
|
(d / "manifest.yaml").write_text(manifest, encoding="utf-8")
|
|
_put_db(server, name)
|
|
return d
|
|
|
|
|
|
def make_zip_sloppak(server, name, manifest=BASE_MANIFEST):
|
|
p = server.DLC_DIR / name
|
|
with zipfile.ZipFile(p, "w", zipfile.ZIP_DEFLATED) as z:
|
|
z.writestr("manifest.yaml", manifest)
|
|
z.writestr("stems/full.ogg", b"OggS-fake")
|
|
_put_db(server, name)
|
|
return p
|
|
|
|
|
|
def _put_db(server, name):
|
|
server.meta_db.put(name, 0, 0, {
|
|
"title": "Thunderstruck", "artist": "AC/DC", "album": "", "year": "",
|
|
"duration": 292, "arrangements": [{"name": "Lead", "index": 0}],
|
|
})
|
|
|
|
|
|
def seed_match(server, fn, state="matched", **overrides):
|
|
"""Seed a confirmed enrichment row as the P8 matcher would have."""
|
|
cand = {"recording_id": "12345678-abcd-4ef0-9876-0123456789ab",
|
|
"release_id": "rel-1", "artist_id": "art-1",
|
|
"artist": "AC/DC", "title": "Thunderstruck",
|
|
"album": "The Razors Edge", "year": "1990",
|
|
"genres": ["hard rock", "rock"], "isrc": "AUAP09000045"}
|
|
cand.update(overrides)
|
|
song = server.meta_db.enrichment_song_row(fn)
|
|
h = server.meta_db.enrichment_content_hash(
|
|
song["artist"], song["title"], song["album"], song["duration"])
|
|
server.meta_db.apply_enrichment_match(
|
|
fn, h, state, source="text", score=1.0, cand=cand,
|
|
candidates=([cand] if state == "review" else None))
|
|
|
|
|
|
# ── preview ───────────────────────────────────────────────────────────────────
|
|
|
|
def test_preview_offers_only_absent_confirmed_keys(server, client):
|
|
make_dir_sloppak(server, "a.sloppak")
|
|
seed_match(server, "a.sloppak")
|
|
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
|
assert d["eligible"] is True
|
|
got = {m["key"]: m["value"] for m in d["missing"]}
|
|
assert got == {"album": "The Razors Edge", "year": 1990,
|
|
"genres": ["hard rock", "rock"],
|
|
"mbid": "12345678-abcd-4ef0-9876-0123456789ab",
|
|
"isrc": "AUAP09000045"}
|
|
|
|
|
|
def test_preview_excludes_author_set_keys(server, client):
|
|
make_dir_sloppak(server, "a.sloppak",
|
|
BASE_MANIFEST + "album: Live Bootleg\nyear: 1991\n")
|
|
seed_match(server, "a.sloppak")
|
|
got = {m["key"] for m in client.get("/api/song/a.sloppak/gap-fill").json()["missing"]}
|
|
assert "album" not in got and "year" not in got
|
|
assert {"genres", "mbid", "isrc"} <= got
|
|
|
|
|
|
def test_preview_excludes_locked_fields(server, client):
|
|
"""A field LOCKED in the Fix-metadata popup is never gap-filled — writing
|
|
the matched value would be exactly the clobber the lock prevents — even
|
|
though the match has a value and the manifest lacks it."""
|
|
make_dir_sloppak(server, "a.sloppak")
|
|
seed_match(server, "a.sloppak")
|
|
server.meta_db.set_song_override("a.sloppak", "album", locked=True)
|
|
server.meta_db.set_song_override("a.sloppak", "year", locked=True)
|
|
got = {m["key"] for m in client.get("/api/song/a.sloppak/gap-fill").json()["missing"]}
|
|
assert "album" not in got and "year" not in got
|
|
assert {"genres", "mbid", "isrc"} <= got # unlocked keys still offered
|
|
|
|
|
|
def test_preview_excludes_present_but_empty_keys(server, client):
|
|
"""Gap-fill is append-only, so a present-but-empty value (album: '',
|
|
year: 0) is NOT a gap the writer can fill — appending would duplicate the
|
|
key, and the never-clobber guard refuses any present key. The preview must
|
|
therefore not offer it (those are the metadata editor's job to re-serialize),
|
|
while genuinely-absent keys are still offered."""
|
|
make_dir_sloppak(server, "a.sloppak", BASE_MANIFEST + "album: ''\nyear: 0\n")
|
|
seed_match(server, "a.sloppak")
|
|
got = {m["key"] for m in client.get("/api/song/a.sloppak/gap-fill").json()["missing"]}
|
|
assert "album" not in got and "year" not in got
|
|
assert {"genres", "mbid", "isrc"} <= got
|
|
|
|
|
|
def test_write_present_but_empty_key_is_refused_not_500(server, client):
|
|
"""The preview↔writer contract must agree: a POST for a present-but-empty
|
|
key is turned away with a clean 409 (never offered → skipped), never a 500
|
|
from the writer's never-clobber guard, and the file is left untouched."""
|
|
d = make_dir_sloppak(server, "a.sloppak", BASE_MANIFEST + "album: ''\nyear: 0\n")
|
|
seed_match(server, "a.sloppak")
|
|
before = (d / "manifest.yaml").read_text(encoding="utf-8")
|
|
r = client.post("/api/song/a.sloppak/gap-fill", json={"keys": ["album", "year"]})
|
|
assert r.status_code == 409
|
|
assert sorted(r.json()["skipped"]) == ["album", "year"]
|
|
assert (d / "manifest.yaml").read_text(encoding="utf-8") == before
|
|
assert not (d / "manifest.yaml.bak").exists() # nothing written → no backup
|
|
# A genuinely-absent key alongside the empty ones still writes cleanly.
|
|
r = client.post("/api/song/a.sloppak/gap-fill", json={"keys": ["album", "genres"]})
|
|
assert r.status_code == 200
|
|
assert r.json() == {"ok": True, "written": {"genres": ["hard rock", "rock"]},
|
|
"skipped": ["album"]}
|
|
|
|
|
|
def test_preview_requires_confirmed_match(server, client):
|
|
make_dir_sloppak(server, "a.sloppak")
|
|
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
|
assert d["eligible"] is False and d["reason"] == "no-match"
|
|
seed_match(server, "a.sloppak", state="review")
|
|
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
|
assert d["eligible"] is False and d["reason"] == "review"
|
|
# A user-pinned match is confirmed.
|
|
seed_match(server, "a.sloppak", state="manual")
|
|
assert client.get("/api/song/a.sloppak/gap-fill").json()["eligible"] is True
|
|
|
|
|
|
# ── writing ───────────────────────────────────────────────────────────────────
|
|
|
|
def test_write_dir_form_appends_and_preserves_author_bytes(server, client):
|
|
d = make_dir_sloppak(server, "a.sloppak")
|
|
seed_match(server, "a.sloppak")
|
|
r = client.post("/api/song/a.sloppak/gap-fill",
|
|
json={"keys": ["album", "year", "genres", "mbid", "isrc"]})
|
|
assert r.status_code == 200
|
|
body = r.json()
|
|
assert set(body["written"]) == {"album", "year", "genres", "mbid", "isrc"}
|
|
text = (d / "manifest.yaml").read_text(encoding="utf-8")
|
|
# The author's original bytes — comments included — survive verbatim as a
|
|
# prefix; the additions are appended after them.
|
|
assert text.startswith(BASE_MANIFEST)
|
|
manifest = yaml.safe_load(text)
|
|
assert manifest["album"] == "The Razors Edge"
|
|
assert manifest["year"] == 1990
|
|
assert manifest["genres"] == ["hard rock", "rock"]
|
|
assert manifest["mbid"] == "12345678-abcd-4ef0-9876-0123456789ab"
|
|
assert manifest["isrc"] == "AUAP09000045"
|
|
# Backup + DB sync (the row must match what the scanner would derive).
|
|
assert (d / "manifest.yaml.bak").read_text(encoding="utf-8") == BASE_MANIFEST
|
|
row = client.get("/api/song/a.sloppak").json()
|
|
assert row["album"] == "The Razors Edge"
|
|
assert str(row["year"]) == "1990"
|
|
|
|
|
|
def test_write_zip_form_appends_with_backup(server, client):
|
|
p = make_zip_sloppak(server, "a.sloppak")
|
|
seed_match(server, "a.sloppak")
|
|
r = client.post("/api/song/a.sloppak/gap-fill", json={"keys": ["album", "mbid"]})
|
|
assert r.status_code == 200
|
|
with zipfile.ZipFile(p) as z:
|
|
text = z.read("manifest.yaml").decode("utf-8")
|
|
assert text.startswith(BASE_MANIFEST)
|
|
manifest = yaml.safe_load(text)
|
|
assert manifest["album"] == "The Razors Edge"
|
|
assert manifest["mbid"] == "12345678-abcd-4ef0-9876-0123456789ab"
|
|
assert "year" not in manifest # unrequested keys untouched
|
|
assert z.read("stems/full.ogg") == b"OggS-fake"
|
|
bak = p.with_name(p.name + ".bak")
|
|
assert bak.exists()
|
|
with zipfile.ZipFile(bak) as z:
|
|
assert z.read("manifest.yaml").decode("utf-8") == BASE_MANIFEST
|
|
|
|
|
|
def test_write_never_replaces_author_values(server, client):
|
|
d = make_dir_sloppak(server, "a.sloppak", BASE_MANIFEST + "album: Live Bootleg\n")
|
|
seed_match(server, "a.sloppak")
|
|
# Requesting a present key: skipped, not replaced; nothing else requested
|
|
# → 409 and the file is untouched.
|
|
r = client.post("/api/song/a.sloppak/gap-fill", json={"keys": ["album"]})
|
|
assert r.status_code == 409
|
|
assert r.json()["skipped"] == ["album"]
|
|
assert (d / "manifest.yaml").read_text(encoding="utf-8").endswith("album: Live Bootleg\n")
|
|
# Mixed request: the gap is written, the author value survives.
|
|
r = client.post("/api/song/a.sloppak/gap-fill", json={"keys": ["album", "year"]})
|
|
assert r.status_code == 200
|
|
assert r.json() == {"ok": True, "written": {"year": 1990}, "skipped": ["album"]}
|
|
manifest = yaml.safe_load((d / "manifest.yaml").read_text(encoding="utf-8"))
|
|
assert manifest["album"] == "Live Bootleg"
|
|
assert manifest["year"] == 1990
|
|
|
|
|
|
def test_writer_last_line_guard(server):
|
|
"""The lib-level never-clobber guard holds even if a caller skips the
|
|
proposal check."""
|
|
import songmeta
|
|
d = make_dir_sloppak(server, "a.sloppak", BASE_MANIFEST + "album: Kept\n")
|
|
with pytest.raises(ValueError):
|
|
songmeta.gap_fill_sloppak(d, {"album": "Clobber"})
|
|
assert "Kept" in (d / "manifest.yaml").read_text(encoding="utf-8")
|
|
|
|
|
|
def test_write_validates_keys(server, client):
|
|
make_dir_sloppak(server, "a.sloppak")
|
|
seed_match(server, "a.sloppak")
|
|
assert client.post("/api/song/a.sloppak/gap-fill",
|
|
json={"keys": ["title"]}).status_code == 400
|
|
assert client.post("/api/song/a.sloppak/gap-fill",
|
|
json={"keys": []}).status_code == 400
|
|
assert client.post("/api/song/a.sloppak/gap-fill", json={}).status_code == 400
|
|
|
|
|
|
def test_demo_mode_blocks_write(tmp_path, monkeypatch, isolate_logging):
|
|
"""The middleware turns the write route away before any handler runs —
|
|
demo visitors can never rewrite pack files."""
|
|
monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
|
|
dlc = tmp_path / "dlc"
|
|
dlc.mkdir()
|
|
monkeypatch.setenv("DLC_DIR", str(dlc))
|
|
monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
|
|
monkeypatch.setenv("FEEDBACK_DEMO_MODE", "1")
|
|
sys.modules.pop("server", None)
|
|
srv = importlib.import_module("server")
|
|
try:
|
|
r = TestClient(srv.app).post("/api/song/a.sloppak/gap-fill",
|
|
json={"keys": ["album"]})
|
|
assert r.status_code == 403
|
|
finally:
|
|
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
|
|
if conn is not None:
|
|
getattr(__import__("sys").modules.get("server"), "_join_background_db_threads", lambda: None)()
|
|
conn.close()
|
|
sys.modules.pop("server", None)
|