v3 library: per-field pack overwrite (R4b) — fix wrong author values, with receipts [DRAFT: gated on spec §7]#733
Draft
ChrisBeWithYou wants to merge 2 commits into
Draft
v3 library: per-field pack overwrite (R4b) — fix wrong author values, with receipts [DRAFT: gated on spec §7]#733ChrisBeWithYou wants to merge 2 commits into
ChrisBeWithYou wants to merge 2 commits into
Conversation
… 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
…irror write-path guard)
The POST /api/song/{fn}/revert-original handler restored a .bak over its
target after only checking that a backup existed — unlike the write path
(gap-fill/overwrite), which refuses non-sloppak targets via is_sloppak. A
non-package path under DLC_DIR with a sibling .bak (or a plain directory
carrying manifest.yaml.bak) could therefore be mutated by a feature meant
only for song packages.
Add the same is_sloppak guard after path resolution and before any restore
or DB resync, returning the 404 the art/source endpoints use for a
non-package target. Existing behaviors (404 when no backup, .bak preserved
after a successful revert, demo-block, path-traversal safety) are intact.
Regression test: a non-package file under the DLC dir with a sibling .bak is
refused (404) and left byte-for-byte unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Integration note — from a full-queue dry-run that merged every open org PR into a next-version test build. Merges with two small adjustments:
Otherwise clean: the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Server (extends the merged gap-fill endpoints, doesn't fork)
GET /api/song/{fn}/gap-fillnow also returnsdiffers=[{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 — plusoverwrite_allowedandhas_backup.differsis empty unlessmatch_state == 'manual': a user-confirmed pin can authorize a replace; an auto-match never can.mbid/isrcare never indiffers(identity changes only via re-match).POSTacceptsoverwrite_keysalongside gap-fill'skeys; gated on theallow_pack_overwritesetting + manual state + the allowlist, proposals recomputed under the IO lock so a field that changed since preview is skipped. Every written key (gap-fill and overwrite) lands in a newwrite_logreceipts table, pruned to the newest 5000 rows.POST /api/song/{fn}/revert-originalrestores the pack from its.bak(dir + zip), re-syncs the DB; 404 when no backup; the.bakis preserved after revert. Demo-blocked.GET /api/song/{fn}/write-log— the song's receipts.allow_pack_overwrite(default OFF).The
.bakguaranteeUnchanged 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 "Overwrite existing fields" sub-block: per-key rows
Artist: "ACDC" → "AC/DC", all unticked by default, warning that it replaces the author's value and keeps a backup. Whendiffersexist but the setting is off, one muted line points to Settings. A "Revert file to original…" link appears whenever a backup exists. Settings gains theallow_pack_overwritecheckbox.Verification
tests/test_pack_overwrite.py— differs manual-only + identity-excluded + equal-skipped; refused when setting-off / not-manual / bad-keys; dir + zip overwrite;.bakstays 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 withtest_gap_fillgreen.node --checkclean; no new Tailwind classes.Merge sequence
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN