feedBack/tests/test_gap_fill.py
ChrisBeWithYou 4db3db4622 v3 library: per-field pack overwrite (R4b) — fix wrong author values, with receipts
DRAFT — gated on the feedpak-spec §7 amendment (got-feedback/feedpak-spec#47).
Gap-fill (#724) fills absent keys; this fixes WRONG author-set values (a
typo'd artist, a wrong year) one field at a time, with provenance and an undo.
Built fully compliant with the amendment shape so it's ready the moment §7
is amended.

Server (extends the merged gap-fill endpoints, doesn't fork):
- Preview GET /api/song/{fn}/gap-fill now also returns `differs` =
  [{key, current, proposed}] over the allowlist (title/artist/album/year/
  genres) where the manifest HAS an author value and the confirmed match
  supplies a DIFFERENT one — plus `overwrite_allowed` (the Settings gate) and
  `has_backup`. differs is EMPTY unless match_state == 'manual': a
  user-confirmed pin can authorize a replace; an auto-match never can. mbid/
  isrc are never in differs (identity changes only via re-match).
- POST accepts `overwrite_keys` alongside gap-fill's `keys`; gated on the
  allow_pack_overwrite setting + manual state + the allowlist, proposals
  recomputed under _song_io_lock so a field that changed since preview is
  skipped. Writes via the re-serialize writer (extended to handle the genres
  list). Every written key (gap-fill AND overwrite) lands in a new write_log
  receipts table (filename, key, old, new, source, score, ts), pruned to the
  newest 5000 rows.
- POST /api/song/{fn}/revert-original restores the pack from its .bak (dir +
  zip forms), re-stats + re-syncs the DB; 404 when no backup; the .bak is
  PRESERVED after revert (re-appliable). Demo-blocked, like the write.
- GET /api/song/{fn}/write-log — the song's receipts, newest first.
- Setting allow_pack_overwrite (default OFF).

The .bak safety property is unchanged from the shipped writer: the backup is
written ONCE (first write) and is always the pristine author original — a
second overwrite never clobbers it, so Revert always returns the untouched
pack.

Frontend (Details drawer, extends the gap-fill block): when the preview
returns differs AND overwriting is enabled, an "Overwrite existing fields"
sub-block renders per-key rows `Artist: "ACDC" -> "AC/DC"`, ALL UNTICKED by
default, with the warning that it replaces what the author wrote and the
original is kept as a backup; the Write button carries the ticked keys. When
differs exist but the setting is off, one muted line points to Settings. A
"Revert file to original…" link (confirm dialog) appears whenever a backup
exists. Settings gains the allow_pack_overwrite checkbox.

Tests: tests/test_pack_overwrite.py — differs manual-only + identity-excluded
+ equal-skipped, refused when setting-off / not-manual / bad keys, dir + zip
overwrite, .bak stays the pristine original across a second write, write_log
records old+new (and gap-fills), prune at cap, revert dir+zip+404, demo
blocks write & revert. 30 pass with test_gap_fill green. node --check clean;
no new Tailwind classes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
2026-07-02 20:53:36 -05:00

271 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 time
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:
# A write endpoint kicks a background scan (which then kicks the
# enrichment worker); both daemon threads share meta_db's sqlite
# connection. Drain them before closing it — closing mid-scan
# crashes the thread with an access violation on Windows. Network
# is off (FEEDBACK_SKIP_STARTUP_TASKS), so both drain in ms.
deadline = time.time() + 10
while time.time() < deadline and (
srv._scan_status.get("running")
or srv._enrich_status.get("running")):
time.sleep(0.05)
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
if conn is not 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_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:
conn.close()
sys.modules.pop("server", None)