Skip to content

Pair playlists on their folder path, not their title - #3

Merged
jrgutier merged 1 commit into
mainfrom
fix/verify-playlist-pairing
Jul 31, 2026
Merged

Pair playlists on their folder path, not their title#3
jrgutier merged 1 commit into
mainfrom
fix/verify-playlist-pairing

Conversation

@jrgutier

Copy link
Copy Markdown
Owner

verify compared some source playlists against the wrong Engine list. Every instance of that invents discrepancies on a conversion that is correct — the worse direction for this tool to fail in, since a verifier that cries wolf gets ignored and the next real defect goes unread.

Found while investigating the "phantom playlist entry" incident. These defects can manufacture that exact symptom (two playlists, one extra track each) on their own, independent of whatever caused the original report.

Three ways it mispaired

# Defect Effect
1 title_to_id keyed on title alone, last-write-wins Two playlists named Chill in different folders collapse onto one list. Engine's uniqueness constraint is per-parent, so this is legal and common.
2 Exact-title lookup succeeds for every same-folder duplicate The writer renames the second to Name (2), but both source lists still carry the original name — so all N duplicates resolve to list #1. The suffix scan meant to handle this was unreachable.
3 Suffix scan matched startswith("Name (") A playlist absent from the database resolved to any unrelated list beginning the same way: House was verified against House (old), reported as wrong tracks rather than missing.

One naming implementation

playlist_naming.py now owns the sibling ordering and the rename, used by both the writer that applies the names and the verifier that must predict them.

The two have to travel together: the suffix a duplicate receives depends on the sibling order, so extracting the rename alone would leave verify re-deriving the ordering by hand — the same coupling that produced these defects.

Pairing uses the whole path as a tuple of titles rather than a joined string, because rekordbox permits / in a playlist name; joining is display-only.

⚠️ Breaking change (text output)

Playlist discrepancy keys are now path-qualified:

playlist[Sets/Setlist].track_order    # was: playlist[Setlist].track_order

Nothing parses these keys programmatically (report.py prints d.field verbatim; VerifyResult has no JSON serialization), but scripts grepping verify's output need updating.

A rewritten test — please scrutinize this one

A pre-existing test asserted that a source Main Set should still match a database list retitled Main Set (2), and this PR makes that report as missing instead.

That test was defect #3 in test form. With a single source playlist the writer can never emit a suffix, so that database state means something else modified it — and test_verify_catches_playlist_rename already asserts an external retitle is reported. The old test demanded the opposite outcome for the same operation whenever the new title happened to resemble a rename.

The case it claimed to defend is now covered properly, by a test that builds two same-named playlists and asserts each pairs with the list it actually became.

Verification

  • 696 tests (688 before + 8 new), 6 real-stick deselected
  • All three defect tests were confirmed red first, for the right reasons
  • playlist_naming.py at 100% line + branch coverage; total 87.79% (floor 85%)
  • ruff + mypy clean
  • Independent review pass: sound; extraction proven behaviour-equivalent to the writer's previous inline grouping

Not included

Remaining workstreams from the investigation are specified in .omc/plans/playlist-phantom-entry-root-cause.md and deliberately left out of this PR: source provenance + torn-file gate + inline pre-publish oracle (W1), external-edit classifier (W3), and the num_rows_large reader probe (W4, needs a real large export.pdb).

🤖 Generated with Claude Code

https://claude.ai/code/session_01V3aF4y4GdsuJta2bfYUZ8w

verify compared some source playlists against the wrong Engine list, and every
instance of that invents discrepancies on a conversion that is correct. That is
the worse direction for this tool to fail in: a verifier that cries wolf gets
ignored, and the next real defect goes unread.

Three ways it mispaired
-----------------------
The lookup was keyed on title alone with last-write-wins, so two playlists named
"Chill" in different folders collapsed onto one list. Engine's uniqueness
constraint is per-parent, so that is legal and common.

Within one folder, rekordbox allows duplicate names and Engine does not, so the
writer renames the second to "Name (2)". Both source lists still carry the
original name, and the exact-title lookup therefore succeeded for every one of
them, resolving all N duplicates to the first list. The suffix scan meant to
handle this was unreachable.

That scan also matched on startswith("Name ("), so a playlist absent from the
database resolved to any unrelated list whose title merely began the same way —
"House" was verified against "House (old)", reported as wrong tracks instead of
as missing, pointing at the wrong playlist entirely.

One naming implementation
-------------------------
playlist_naming.py now owns the sibling ordering and the rename, and both the
writer that applies the names and the verifier that must predict them use it.
The two have to travel together: the suffix a duplicate receives depends on the
sibling order, so extracting the rename alone would leave verify re-deriving the
ordering by hand — the same coupling that produced these defects.

Pairing is on the whole path as a tuple of titles, not a joined string, because
rekordbox permits "/" in a playlist name; joining is for display only.

A retitled list is now reported
-------------------------------
A test asserted that a source "Main Set" should still match a database list
retitled "Main Set (2)". That was the startswith bug in test form: with one
source playlist the writer can never emit a suffix, so that database state means
something else changed it, and test_verify_catches_playlist_rename already
asserts an external retitle is reported. The old test demanded the opposite
outcome for the same operation whenever the new title happened to look like a
rename. It is replaced by one asserting the retitle is reported, and the case it
claimed to defend is covered properly by a test that builds two same-named
playlists and checks each pairs with the list it became.

Breaking: playlist discrepancy keys are now path-qualified.

696 tests (8 new), 88% branch coverage, playlist_naming at 100%, ruff + mypy clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3aF4y4GdsuJta2bfYUZ8w
@jrgutier
jrgutier merged commit 03e06b5 into main Jul 31, 2026
9 checks passed
@jrgutier
jrgutier deleted the fix/verify-playlist-pairing branch July 31, 2026 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant