Commit Graph

2 Commits

Author SHA1 Message Date
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
ChrisBeWithYou
8e953e8bc4
library: opt-in gap-fill — write confirmed missing metadata into the pack (R4a) (#724)
* library: opt-in gap-fill — write confirmed missing metadata into the pack (R4a)

The write-back contract agreed with the spec chair (alignment doc §7),
made executable, now that feedpak-spec 1.14.0 (mbid/isrc) is merged:
opt-in + user-initiated, adds ABSENT keys only, spec'd-keys allowlist,
values only from a CONFIRMED identity, atomic write + .bak. Single-song
only — batch write-back stays an open question with the chair.

- songmeta.gap_fill_sloppak: append-only manifest writer. Every added
  key is absent by definition, so the new lines are APPENDED — the
  author's existing bytes (key order, comments, formatting) survive
  verbatim, unlike the metadata editor's full re-serialize. Directory
  form gets a one-time manifest.yaml.bak + temp + atomic replace; zip
  form reuses the editor's backup/temp/replace rewriter. Raises on any
  already-present key: the never-clobber rule lives in the writer, not
  just the callers.
- GET /api/song/{fn}/gap-fill: read-only preview — which of
  album/year/genres/mbid/isrc are missing from the file (absent or
  empty; year 0 = empty), with the values the enrichment match
  supplies. Only a CONFIRMED identity is eligible (matched or a user
  pin); review-tier rows are refused until a human confirms —
  wrong-match > fast, same as everywhere else in the enrichment layer.
- POST /api/song/{fn}/gap-fill {keys}: writes the user-confirmed
  subset. Proposals are RECOMPUTED under _song_io_lock, so a key that
  gained an author value between preview and confirm is skipped, never
  replaced. mbid/isrc written in canonical form only (validated).
  DB stays scanner-consistent (album/year/genre columns + mtime/size
  re-stat, cache invalidation + scan kick — the metadata editor's
  contract). Demo mode blocks the write.
- Details drawer (Identity section): "Write missing info to file…" →
  per-key checkbox confirm ("Only adds what's missing — nothing already
  in the file is changed. A backup (.bak) is kept.") → written
  confirmation; not-eligible states explain themselves. v3 only; no
  new tailwind classes.
- Rides along: _manifest_exact_ids now strips ISRC display separators
  (spec 1.14.0's strip rule) — a hand-authored "AU-AP0-90-00045" hits
  the exact-match tier instead of silently falling back to text.

Tests: tests/test_gap_fill.py (10) — preview eligibility incl.
review-refusal + empty-as-gap, author-bytes-preserved-verbatim on dir
AND zip (with .bak content pinned), skip-not-replace on the mixed
request, the writer's ValueError guard, key validation, demo block,
DB sync; +1 hyphenated-ISRC test in test_mb_enrichment.py. 46 targeted
green; full-suite failure set A/B-identical to the main base (39
env/pre-existing).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* gap-fill: align preview with append-only writer (no cleared-value 500)

The R4a preview offered present-but-empty manifest values (album: '',
genres: [], year: 0) as gaps, but the append-only writer's never-clobber
guard raises on ANY key already present — so a user-confirmed POST for
those keys turned into a 500 "write failed" instead of filling the gap.
Appending can't fill an empty-but-present key anyway (it would duplicate
the YAML key).

Fix: _gap_fill_manifest_absent now treats only genuinely-MISSING keys as
gaps; a present-but-empty value is left to the metadata editor (which
re-serializes and can replace in place). This closes the preview→POST
mismatch — the preview never offers what the writer would refuse.

Tests: test_preview_treats_empty_values_as_gaps replaced by
test_preview_excludes_present_but_empty_keys (present-but-empty not
offered; genuinely-absent still offered) + test_write_present_but_empty_
key_is_refused_not_500 (POST → clean 409, file untouched, no .bak; a
genuinely-absent key alongside still writes). Closes the write-path blind
spot in the original empty-value test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
2026-07-02 20:52:11 +02:00