Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ All notable changes to this project are documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed
- `verify` paired each source playlist with the wrong Engine list in three ways,
every one of which invents discrepancies on a correct conversion — the failure
mode that trains you to ignore verify. Same-named playlists in different
folders collapsed onto one list (Engine's uniqueness constraint is
per-parent, so this is legal and common); duplicates within one folder all
compared against the first, because the writer renames the second to
`"Name (2)"` while both source lists keep the original name; and a missing
playlist could resolve to an unrelated one whose title merely started the
same way, so `"House"` was verified against `"House (old)"`.
- Playlists are now paired on their full folder path. Engine's
unique-name-per-folder renaming lives in one shared module used by both the
writer that applies the names and the verifier that has to predict them —
verify re-deriving that by hand is what produced all three defects.

### Changed
- **Breaking (text output).** Playlist discrepancies are keyed by folder path
rather than bare name: `playlist[Sets/Setlist].track_order`, previously
`playlist[Setlist].track_order`. Nothing parses these keys programmatically,
but scripts grepping verify's output will need updating.
- A playlist retitled in the database is now reported as missing instead of
being silently matched when its new title resembles a duplicate suffix. This
is divergence from the source and belongs in the report; classifying it as an
external edit rather than an absence is follow-up work.

## [0.3.2] - 2026-07-31

### Fixed
Expand Down
86 changes: 86 additions & 0 deletions src/rb2engine/playlist_naming.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
"""Sibling ordering and Engine's unique-name-per-folder renaming.

Shared by the writer, which applies these names, and by verify, which has to
predict them in order to pair each source playlist with the engine list it
became.

The two must travel together: the suffix a duplicate receives depends on the
sibling ordering, so a caller that re-derived only the rename would still
disagree with the writer whenever the ordering mattered. Verify re-deriving both
by hand is what produced the pairing defects this module exists to prevent.

Paths are tuples of titles rather than a joined string because a rekordbox
playlist name may itself contain a separator character; joining is for display
only.
"""

from __future__ import annotations

from collections import defaultdict
from collections.abc import Sequence

from rb2engine.ir import SourcePlaylist

_DISPLAY_SEP = "/"


def sibling_order(playlists: Sequence[SourcePlaylist]) -> dict[int, list[int]]:
"""``parent_rb_id`` → child rb_ids in ``(sort_order, rb_id)`` order."""
by_rb = {pl.rb_id: pl for pl in playlists}
groups: dict[int, list[int]] = defaultdict(list)
for pl in playlists:
groups[pl.parent_rb_id].append(pl.rb_id)
for rb_ids in groups.values():
rb_ids.sort(key=lambda r: (by_rb[r].sort_order, r))
return dict(groups)


def resolve_titles(playlists: Sequence[SourcePlaylist]) -> dict[int, str]:
"""``rb_id`` → the title the writer will give it.

rekordbox permits two playlists with the same name in one folder; Engine
does not (``C_NAME_UNIQUE_FOR_PARENT``). The second and later duplicates
become ``"Name (2)"``, ``"Name (3)"``, … in sibling order, which is
deterministic, so re-runs produce identical names.
"""
by_rb = {pl.rb_id: pl for pl in playlists}
titles: dict[int, str] = {}
for rb_ids in sibling_order(playlists).values():
seen: dict[str, int] = {}
for rb in rb_ids:
original = by_rb[rb].name
count = seen.get(original, 0)
seen[original] = count + 1
titles[rb] = original if count == 0 else f"{original} ({count + 1})"
return titles


def resolve_paths(
playlists: Sequence[SourcePlaylist],
) -> dict[int, tuple[str, ...]]:
"""``rb_id`` → its resolved titles from the root, root first.

A playlist is identified by its whole path: the same title may legally
appear under several folders, since Engine's uniqueness constraint is
per-parent.
"""
by_rb = {pl.rb_id: pl for pl in playlists}
titles = resolve_titles(playlists)
paths: dict[int, tuple[str, ...]] = {}
for rb in titles:
parts: list[str] = []
cur = rb
walked: set[int] = set()
# A parent cycle is rejected by the writer; guard anyway so verify
# cannot hang on a malformed source.
while cur != 0 and cur in by_rb and cur not in walked:
walked.add(cur)
parts.append(titles[cur])
cur = by_rb[cur].parent_rb_id
paths[rb] = tuple(reversed(parts))
return paths


def format_path(path: Sequence[str]) -> str:
"""Human-readable form of a playlist path, for report field names."""
return _DISPLAY_SEP.join(path)
60 changes: 39 additions & 21 deletions src/rb2engine/verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from rb2engine.ir import SourceLibrary, SourcePlaylist, SourceTrack
from rb2engine.ir_engine import artwork_content_hash
from rb2engine.mapper.track import map_track
from rb2engine.playlist_naming import format_path, resolve_paths
from rb2engine.reader.library import read_library
from rb2engine.writer.blobs import (
decode_beat_data,
Expand Down Expand Up @@ -537,21 +538,23 @@ def _compare_playlists(
)
)

# Title → list id (first match; Engine renames duplicates with " (N)").
title_to_id: dict[str, int] = {}
for row in conn.execute("SELECT id, title FROM Playlist"):
title_to_id[str(row[1])] = int(row[0])
# Pair on the whole path, not the title: the same title may legally exist
# under several folders (Engine's constraint is per-parent), and duplicates
# within one folder are renamed by the writer. Both are predicted by
# playlist_naming, which the writer uses to assign the names in the first
# place.
path_to_id = _db_playlist_paths(conn)
source_paths = resolve_paths(source.playlists)

for pl in source.playlists:
list_id = title_to_id.get(pl.name)
if list_id is None:
# Renamed duplicate titles — try ordered suffix scan.
list_id = _find_playlist_id(title_to_id, pl)
path = source_paths[pl.rb_id]
label = format_path(path)
list_id = path_to_id.get(path)
if list_id is None:
discrepancies.append(
Discrepancy(
track_id=None,
field=f"playlist[{pl.name}].missing",
field=f"playlist[{label}].missing",
expected="present",
actual="absent",
)
Expand All @@ -568,7 +571,7 @@ def _compare_playlists(
discrepancies.append(
Discrepancy(
track_id=None,
field=f"playlist[{pl.name}].chain",
field=f"playlist[{label}].chain",
expected="every row reachable from the nextEntityId chain",
actual=chain_problem,
)
Expand All @@ -577,23 +580,38 @@ def _compare_playlists(
discrepancies.append(
Discrepancy(
track_id=None,
field=f"playlist[{pl.name}].track_order",
field=f"playlist[{label}].track_order",
expected=expected_track_ids,
actual=actual_track_ids,
)
)


def _find_playlist_id(
title_to_id: dict[str, int], pl: SourcePlaylist
) -> int | None:
if pl.name in title_to_id:
return title_to_id[pl.name]
# insert_playlists renames duplicates to "Name (2)", "Name (3)", …
for title, lid in title_to_id.items():
if title == pl.name or title.startswith(f"{pl.name} ("):
return lid
return None
def _db_playlist_paths(
conn: sqlite3.Connection,
) -> dict[tuple[str, ...], int]:
"""Engine playlist path (root first) → list id.

Built by walking ``parentListId`` to the root, so a title is only ever
matched within its own folder.
"""
rows: dict[int, tuple[str, int]] = {
int(r[0]): (str(r[1]), int(r[2]))
for r in conn.execute("SELECT id, title, parentListId FROM Playlist")
}
paths: dict[tuple[str, ...], int] = {}
for list_id in rows:
parts: list[str] = []
cur = list_id
walked: set[int] = set()
# Guard against a cyclic parent chain in a database we did not write.
while cur != 0 and cur in rows and cur not in walked:
walked.add(cur)
title, parent = rows[cur]
parts.append(title)
cur = parent
paths[tuple(reversed(parts))] = list_id
return paths


def _expected_entity_track_ids(
Expand Down
23 changes: 9 additions & 14 deletions src/rb2engine/writer/playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

from rb2engine.chain import walk_entity_chain
from rb2engine.ir import SourcePlaylist
from rb2engine.playlist_naming import resolve_titles

# Engine / libdjinterop sentinel: tail of every next*Id chain.
_NO_NEXT = 0
Expand Down Expand Up @@ -142,20 +143,14 @@ def insert_playlists(
# happened. The suffix is assigned in the already-deterministic sibling
# order (sort_order, rb_id), so re-runs produce identical names and the
# determinism guarantee holds.
title_of: dict[int, str] = {}
renamed: list[tuple[str, str]] = []
for group in siblings.values():
seen: dict[str, int] = {}
for rb in group: # already sorted by (sort_order, rb_id)
original = by_rb[rb].name
count = seen.get(original, 0)
seen[original] = count + 1
if count == 0:
title_of[rb] = original
else:
new_title = f"{original} ({count + 1})"
title_of[rb] = new_title
renamed.append((original, new_title))
# The algorithm lives in playlist_naming so verify predicts exactly what we
# write; it must not be reimplemented on either side.
title_of = resolve_titles(playlists)
renamed: list[tuple[str, str]] = [
(by_rb[rb].name, title_of[rb])
for rb in by_rb
if title_of[rb] != by_rb[rb].name
]
if renamed:
logger.warning(
"renamed %d playlist(s) to satisfy Engine's unique-name-per-folder "
Expand Down
95 changes: 95 additions & 0 deletions tests/unit/test_playlist_naming.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
"""Contract tests for the naming shared by the writer and verify.

WHY these are direct rather than only exercised through both callers: the whole
point of the module is that two independent consumers agree. If either drifts,
an integration test on one side can stay green while the pair silently
disagrees — which is the class of defect the module was extracted to end.
"""

from __future__ import annotations

from rb2engine.ir import SourcePlaylist
from rb2engine.playlist_naming import format_path, resolve_paths, resolve_titles


def _pl(rb_id: int, name: str, *, parent: int = 0, sort: int = 0) -> SourcePlaylist:
return SourcePlaylist(
rb_id=rb_id,
parent_rb_id=parent,
name=name,
sort_order=sort,
is_folder=False,
track_rb_ids=[],
)


def test_suffixes_follow_sibling_order_not_input_order() -> None:
"""The suffix depends on (sort_order, rb_id), never on list position.

WHY: this is why the ordering and the rename must live in one function. A
caller that re-derived only the rename would suffix whichever duplicate it
happened to see first and disagree with the writer.
"""
playlists = [
_pl(3, "Setlist", sort=2),
_pl(1, "Setlist", sort=0),
_pl(2, "Setlist", sort=1),
]

titles = resolve_titles(playlists)

assert titles[1] == "Setlist"
assert titles[2] == "Setlist (2)"
assert titles[3] == "Setlist (3)"


def test_duplicate_names_in_different_folders_are_not_renamed() -> None:
"""Engine's constraint is per-parent, so siblings-only collisions rename."""
playlists = [
_pl(1, "Folder A"),
_pl(2, "Folder B", sort=1),
_pl(3, "Chill", parent=1),
_pl(4, "Chill", parent=2),
]

titles = resolve_titles(playlists)

assert titles[3] == "Chill"
assert titles[4] == "Chill"


def test_paths_distinguish_same_named_playlists() -> None:
playlists = [
_pl(1, "Folder A"),
_pl(2, "Folder B", sort=1),
_pl(3, "Chill", parent=1),
_pl(4, "Chill", parent=2),
]

paths = resolve_paths(playlists)

assert paths[3] == ("Folder A", "Chill")
assert paths[4] == ("Folder B", "Chill")
assert paths[3] != paths[4]


def test_path_is_a_tuple_so_a_name_containing_the_separator_stays_distinct() -> None:
"""A "/" in a playlist name must not merge two different playlists.

WHY: rekordbox permits "/" in names. Matching on a joined string would make
a playlist literally named "A/B" collide with "B" inside folder "A"; only
the display form joins.
"""
playlists = [
_pl(1, "A"),
_pl(2, "B", parent=1),
_pl(3, "A/B", sort=1),
]

paths = resolve_paths(playlists)

assert paths[2] == ("A", "B")
assert paths[3] == ("A/B",)
assert paths[2] != paths[3]
# Display collapses them; matching does not.
assert format_path(paths[2]) == format_path(paths[3]) == "A/B"
Loading
Loading