From f429211cf32b46a3ae9147f8c3abe4a97a6d6f06 Mon Sep 17 00:00:00 2001 From: jrgutier Date: Wed, 29 Jul 2026 08:52:55 -0500 Subject: [PATCH 1/2] Catch a wrong playlist write before it is published MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A conversion on a real stick published two playlists that each contained one track appearing nowhere in the corresponding source playlist, and `convert` still exited 0. Only a later `verify` found it. Re-running convert over the same export.pdb produced a clean database, which destroyed the evidence and left the cause unknown. The root cause is NOT fixed here — it is not identified. What is fixed is that this class of corruption could be published silently. What was ruled out ------------------ The reader is a pure function of the file bytes (`data = path.read_bytes()`, no mmap; `_walk_table_pages` walks that buffer), and repeated parses of the stick returned identical playlist membership every time. The writer only ever drops entries relative to `pl.track_rb_ids`; it cannot add one. rb ids on this library are exactly 1..3673 with no skips, so the identity map returned by `insert_tracks` (EngineTrack carries no rb_id) is correct here, and the positional repair in build.py is correct when ids have gaps — not the bug. Four faithful rebuilds with artwork on, through the full copy-to-USB path, produced byte-identical playlist chains. The failure did not recur. Two gates --------- `insert_playlists` now reads its own nextEntityId chains back and raises if they disagree with what it meant to write. The walk follows the chain rather than row order because the spurious row was spliced second-to-last, where an `ORDER BY id` comparison cannot see it. Every row in the table is checked, not just the lists we filled: a stray row on a folder or an empty playlist is exactly as wrong, and scoping to intended lists would leave that invisible. That check runs inside the writing transaction, so it can only prove SQLite agreed at that moment. build.py therefore re-checks the copy that actually crossed to the target volume, after fsync and before `os.replace`. A failure there leaves the user's previous m.db byte-for-byte intact and surfaces through the existing FatalError path as exit 2. Reproduction harness -------------------- tools/repro_playlist_determinism.py bisects the pipeline so a recurrence is attributable rather than mysterious: the reader stage hashes the raw export.pdb bytes alongside the parse, separating an unreliable read from a non-deterministic parser; the writer stage reads once and rebuilds N times from that provably identical input. It preserves the baseline and any diverging m.db, since a re-run is what lost the original evidence. It does not use a symlinked shadow root. `engine_track_path` resolves both paths, so a symlink collapsed to the real stick, `relative_to` raised, and mapper/track.py degraded to the raw path — every track still "converted" and the harness reported a confident verdict about a configuration nobody ships. It now runs against the real drive root, redirects only the output directory, and asserts the written paths still look like a real conversion's. Tests ----- The corruption tests splice a row mid-chain, orphan a row, delete an entry and repair its predecessor, and place a row on an unintended list. The wiring is covered too: removing the gate call from insert_playlists turns the suite red, which was not true of the first version of these tests. 693 passing (687 before), ruff clean. Verified end to end on a 3,673-track stick: convert wrote 45 playlists and verify reports 0 discrepancies, so neither gate misfires on a real library. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AxFS98fHa9GwJzPnfLPVQr --- src/rb2engine/writer/build.py | 27 ++- src/rb2engine/writer/playlists.py | 101 +++++++- tests/unit/test_write_playlists.py | 193 +++++++++++++++- tools/repro_playlist_determinism.py | 343 ++++++++++++++++++++++++++++ 4 files changed, 661 insertions(+), 3 deletions(-) create mode 100644 tools/repro_playlist_determinism.py diff --git a/src/rb2engine/writer/build.py b/src/rb2engine/writer/build.py index 213c15f..46fff6e 100644 --- a/src/rb2engine/writer/build.py +++ b/src/rb2engine/writer/build.py @@ -23,6 +23,7 @@ import sqlite3 import sys import tempfile +from collections.abc import Sequence from pathlib import Path from rb2engine.errors import FatalError @@ -190,6 +191,7 @@ def build_library( # Late imports keep this module importable while sibling writer modules # land (concurrent workers). Contract signatures are fixed. from rb2engine.writer import database as database_mod + from rb2engine.writer import playlists as playlists_mod from rb2engine.writer.playlists import insert_playlists conn: sqlite3.Connection | None = None @@ -315,8 +317,12 @@ def build_library( # --- playlists --------------------------------------------------------- if on_progress is not None: on_progress("playlists", 0, 0) + intended_playlists: dict[int, Sequence[int]] = {} n_playlists = insert_playlists( - conn, lib.playlists, track_id_map=track_id_map or {} + conn, + lib.playlists, + track_id_map=track_id_map or {}, + intended_out=intended_playlists, ) report.counters.playlists_converted = n_playlists @@ -343,6 +349,25 @@ def build_library( _fsync_file(tmp_path) _fsync_dir(db2) + # Re-check the copy that actually crossed to the target volume. + # + # The check inside insert_playlists runs in the writing transaction, so + # it can only prove SQLite agreed with us at that moment — it cannot see + # the commit, the half-gigabyte copy over USB, or this volume's driver. + # A conversion once published playlists containing a track that was in + # no source playlist and still exited 0, and the staged database is + # discarded before anyone can compare it, so this is the last point + # where that class of corruption is still catchable. Running it before + # os.replace means a failure leaves the user's previous m.db in place. + if intended_playlists: + check_conn = sqlite3.connect(f"file:{tmp_path}?mode=ro", uri=True) + try: + playlists_mod.assert_entities_match_intent( + check_conn, intended_playlists + ) + finally: + check_conn.close() + os.replace(tmp_path, m_db_path) _fsync_dir(db2) diff --git a/src/rb2engine/writer/playlists.py b/src/rb2engine/writer/playlists.py index 1ef5341..57fc95d 100644 --- a/src/rb2engine/writer/playlists.py +++ b/src/rb2engine/writer/playlists.py @@ -20,7 +20,7 @@ import logging import sqlite3 from collections import defaultdict -from collections.abc import Mapping, Sequence +from collections.abc import Mapping, MutableMapping, Sequence from rb2engine.ir import SourcePlaylist @@ -39,6 +39,7 @@ def insert_playlists( playlists: Sequence[SourcePlaylist], *, track_id_map: Mapping[int, int], + intended_out: MutableMapping[int, Sequence[int]] | None = None, ) -> int: """Insert Playlist + PlaylistEntity rows for *playlists*. @@ -53,6 +54,11 @@ def insert_playlists( track_id_map: ``SourceTrack.rb_id`` → Engine ``Track.id``. Entries whose rb_id is missing are skipped (soft track skips must not fail playlist write). + intended_out: + Optional sink receiving ``listId`` → intended track order. The caller + needs it to re-check the database *after* it has been committed and + copied to the target volume; the in-transaction check below cannot see + that far. Returns ------- @@ -194,6 +200,9 @@ def insert_playlists( next_entity_id = 1 duplicate_entries = 0 + # listId → the exact track order this function intends to write. Kept so + # the write can be read back and checked against intent (see below). + intended: dict[int, list[int]] = {} for rb_id, pl in by_rb.items(): list_id = engine_id_of[rb_id] # Preserve source order; drop members whose tracks were skipped. @@ -219,6 +228,7 @@ def insert_playlists( continue n = len(engine_track_ids) + intended[list_id] = engine_track_ids entity_ids = list(range(next_entity_id, next_entity_id + n)) next_entity_id += n for i, track_id in enumerate(engine_track_ids): @@ -240,6 +250,10 @@ def insert_playlists( ) _bump_sequence(conn, "PlaylistEntity", next_entity_id - 1) + assert_entities_match_intent(conn, intended) + if intended_out is not None: + intended_out.update(intended) + if duplicate_entries: logger.warning( "dropped %d duplicate playlist entrie(s): rekordbox allows the same " @@ -250,6 +264,91 @@ def insert_playlists( return len(playlists) +def _walk_entity_chain( + list_id: int, rows: Sequence[tuple[int, int, int]] +) -> list[int]: + """Track order for one list, walked through ``nextEntityId`` like Engine. + + Walking the chain rather than reading the raw rows is deliberate: a row that + exists but is unreachable, or one spliced into the middle, is invisible to a + plain ``SELECT ... ORDER BY id`` and is exactly the defect this must catch. + """ + by_next: dict[int, tuple[int, int]] = {} + for eid, tid, nxt in rows: + # Two rows sharing a successor would silently collapse into one dict + # entry. The count check below would still fire, but it would blame the + # wrong thing, so name this corruption for what it is. + if int(nxt) in by_next: + raise RuntimeError( + f"playlist listId={list_id}: two PlaylistEntity rows share " + f"nextEntityId={int(nxt)} — chain is forked" + ) + by_next[int(nxt)] = (int(eid), int(tid)) + + order: list[int] = [] + curr = _NO_NEXT + seen: set[int] = set() + while curr in by_next: + eid, track_id = by_next[curr] + if eid in seen: # corrupt chain must not spin forever + break + seen.add(eid) + order.insert(0, track_id) + curr = eid + + # A row that no chain walk reaches is still a row Engine may honour; make + # the count mismatch loud instead of letting the walk hide it. + if len(order) != len(rows): + raise RuntimeError( + f"playlist listId={list_id}: {len(rows)} PlaylistEntity rows but the " + f"nextEntityId chain reaches {len(order)} — chain is inconsistent" + ) + return order + + +def assert_entities_match_intent( + conn: sqlite3.Connection, intended: Mapping[int, Sequence[int]] +) -> None: + """Read every written chain back and fail if it differs from intent. + + The writer builds each chain in memory and inserts it in one pass, so in + principle the database must agree. In practice a conversion shipped two + playlists containing a track that appeared nowhere in the source, and + ``convert`` still exited 0 — the corruption was only found later by + ``verify``. Checking here turns that class of silent wrong-database into a + build-time failure, before the new m.db is swapped into place. + + Every row in the table is examined, not just the lists we meant to fill: a + spurious row landing on a folder or an empty playlist is precisely as wrong + as one landing on a playlist we wrote, and scoping the check to *intended* + would leave that whole class invisible. + """ + grouped: dict[int, list[tuple[int, int, int]]] = defaultdict(list) + for list_id, eid, tid, nxt in conn.execute( + "SELECT listId, id, trackId, nextEntityId FROM PlaylistEntity" + ): + grouped[int(list_id)].append((int(eid), int(tid), int(nxt))) + + unexpected_lists = sorted(set(grouped) - set(intended)) + if unexpected_lists: + raise RuntimeError( + f"PlaylistEntity rows exist for listId(s) {unexpected_lists} that " + "no playlist write intended" + ) + + for list_id, expected in intended.items(): + actual = _walk_entity_chain(list_id, grouped.get(list_id, [])) + if list(actual) != list(expected): + extra = sorted(set(actual) - set(expected)) + missing = sorted(set(expected) - set(actual)) + raise RuntimeError( + f"playlist listId={list_id}: written entries do not match the " + f"{len(expected)} intended ({len(actual)} written; " + f"unexpected track ids {extra or 'none'}; " + f"absent track ids {missing or 'none'})" + ) + + def _bump_sequence(conn: sqlite3.Connection, table: str, seq: int) -> None: """Ensure sqlite_sequence.seq >= *seq* after explicit PRIMARY KEY inserts. diff --git a/tests/unit/test_write_playlists.py b/tests/unit/test_write_playlists.py index ac98d73..256155a 100644 --- a/tests/unit/test_write_playlists.py +++ b/tests/unit/test_write_playlists.py @@ -17,9 +17,16 @@ import sqlite3 from pathlib import Path +import pytest + from rb2engine.ir import SourcePlaylist +from rb2engine.writer import playlists as playlists_mod from rb2engine.writer import schema as schema_mod -from rb2engine.writer.playlists import insert_playlists +from rb2engine.writer.playlists import ( + _walk_entity_chain, + assert_entities_match_intent, + insert_playlists, +) # Engine / libdjinterop sentinel: tail of every next*Id chain points at 0. _NO_NEXT = 0 @@ -261,3 +268,187 @@ def test_returns_zero_for_empty_input(tmp_path: Path) -> None: assert conn.execute("SELECT COUNT(*) FROM Playlist").fetchone()[0] == 0 finally: conn.close() + + +# --------------------------------------------------------------------------- +# Post-write integrity gate +# +# A real conversion shipped two playlists each containing one track that +# appeared nowhere in the corresponding source playlist, and `convert` still +# exited 0 — only a later `verify` caught it. insert_playlists now reads its +# own chains back and refuses to hand over a database that disagrees with what +# it meant to write. A check that cannot fail would be worthless, so these +# tests corrupt the chain deliberately and require the failure. +# --------------------------------------------------------------------------- + + +def _seed_one_playlist(tmp_path: Path) -> tuple[sqlite3.Connection, int, list[int]]: + """Write a single 3-track playlist and return (conn, list_id, track_ids).""" + playlists = [ + SourcePlaylist( + rb_id=1, + parent_rb_id=0, + name="PL", + sort_order=0, + is_folder=False, + track_rb_ids=[10, 20, 30], + ) + ] + conn = _open_empty_db(tmp_path) + insert_playlists(conn, playlists, track_id_map={10: 101, 20: 102, 30: 103}) + conn.commit() + list_id = conn.execute("SELECT id FROM Playlist").fetchone()[0] + return conn, int(list_id), [101, 102, 103] + + +def _chain(conn: sqlite3.Connection, list_id: int) -> list[int]: + rows = conn.execute( + "SELECT id, trackId, nextEntityId FROM PlaylistEntity WHERE listId = ?", + (list_id,), + ).fetchall() + return _walk_entity_chain(list_id, [(int(a), int(b), int(c)) for a, b, c in rows]) + + +def test_integrity_gate_accepts_a_faithful_write(tmp_path: Path) -> None: + """The gate must stay silent on a correct chain, or it is just noise.""" + conn, list_id, tracks = _seed_one_playlist(tmp_path) + try: + assert _chain(conn, list_id) == tracks + assert_entities_match_intent(conn, {list_id: tracks}) + finally: + conn.close() + + +def test_insert_playlists_actually_invokes_the_gate( + tmp_path: Path, monkeypatch +) -> None: + """The wiring itself must be able to fail, not just the helper. + + Without this, deleting the call from insert_playlists leaves every other + test in this file green — the gate would be dead code and nothing would say + so. + """ + monkeypatch.setattr( + playlists_mod, + "_walk_entity_chain", + lambda _list_id, rows: [*(int(t) for _, t, _ in rows), 4242], + ) + playlists = [ + SourcePlaylist( + rb_id=1, + parent_rb_id=0, + name="PL", + sort_order=0, + is_folder=False, + track_rb_ids=[10], + ) + ] + conn = _open_empty_db(tmp_path) + try: + with pytest.raises(RuntimeError, match="4242"): + insert_playlists(conn, playlists, track_id_map={10: 101}) + finally: + conn.close() + + +def test_integrity_gate_catches_row_on_an_unintended_list(tmp_path: Path) -> None: + """A spurious row on a folder or empty playlist is just as wrong.""" + conn, list_id, tracks = _seed_one_playlist(tmp_path) + try: + conn.execute( + "INSERT INTO PlaylistEntity (id, listId, trackId, databaseUuid, " + "nextEntityId, membershipReference) VALUES (?, ?, ?, ?, 0, 0)", + (9003, list_id + 500, 997, "test-uuid-playlists"), + ) + conn.commit() + with pytest.raises(RuntimeError, match="no playlist write intended"): + assert_entities_match_intent(conn, {list_id: tracks}) + finally: + conn.close() + + +def test_integrity_gate_catches_extra_track_spliced_into_chain( + tmp_path: Path, +) -> None: + """The observed defect: one track present that the source never had. + + The spurious row is spliced *into* the chain rather than appended, which is + where it was found on the real stick (second to last), so a check that only + compared lengths at the tail would miss it. + """ + conn, list_id, tracks = _seed_one_playlist(tmp_path) + try: + # Point the middle entity at a new row, and that row at the old tail. + tail_id = conn.execute( + "SELECT id FROM PlaylistEntity WHERE listId = ? AND nextEntityId = 0", + (list_id,), + ).fetchone()[0] + prev_id = conn.execute( + "SELECT id FROM PlaylistEntity WHERE listId = ? AND nextEntityId = ?", + (list_id, tail_id), + ).fetchone()[0] + conn.execute( + "INSERT INTO PlaylistEntity (id, listId, trackId, databaseUuid, " + "nextEntityId, membershipReference) VALUES (?, ?, ?, ?, ?, 0)", + (9001, list_id, 999, "test-uuid-playlists", tail_id), + ) + conn.execute( + "UPDATE PlaylistEntity SET nextEntityId = ? WHERE id = ?", + (9001, prev_id), + ) + conn.commit() + + assert 999 in _chain(conn, list_id) + with pytest.raises(RuntimeError, match="999"): + assert_entities_match_intent(conn, {list_id: tracks}) + finally: + conn.close() + + +def test_integrity_gate_catches_unreachable_row(tmp_path: Path) -> None: + """A row no chain walk reaches is still a row Engine may honour.""" + conn, list_id, tracks = _seed_one_playlist(tmp_path) + try: + conn.execute( + "INSERT INTO PlaylistEntity (id, listId, trackId, databaseUuid, " + "nextEntityId, membershipReference) VALUES (?, ?, ?, ?, ?, 0)", + (9002, list_id, 998, "test-uuid-playlists", 4242), + ) + conn.commit() + with pytest.raises(RuntimeError, match="chain is inconsistent"): + assert_entities_match_intent(conn, {list_id: tracks}) + finally: + conn.close() + + +def test_integrity_gate_catches_dropped_track(tmp_path: Path) -> None: + """Losing an entry must fail as loudly as gaining one. + + The row is removed from the database and the predecessor is re-pointed at + its successor, so this is a genuine dropped entry with an intact chain — + not merely a corrupted expectation. + """ + conn, list_id, tracks = _seed_one_playlist(tmp_path) + try: + tail_id, tail_track = conn.execute( + "SELECT id, trackId FROM PlaylistEntity " + "WHERE listId = ? AND nextEntityId = 0", + (list_id,), + ).fetchone() + prev_id = conn.execute( + "SELECT id FROM PlaylistEntity WHERE listId = ? AND nextEntityId = ?", + (list_id, tail_id), + ).fetchone()[0] + # Drop the DELETE trigger's rewiring out of the picture by repairing the + # predecessor ourselves, so the chain stays well-formed. + conn.execute("DELETE FROM PlaylistEntity WHERE id = ?", (tail_id,)) + conn.execute( + "UPDATE PlaylistEntity SET nextEntityId = 0 WHERE id = ?", (prev_id,) + ) + conn.commit() + + assert tail_track not in _chain(conn, list_id) + with pytest.raises(RuntimeError, match="absent track ids"): + assert_entities_match_intent(conn, {list_id: tracks}) + finally: + conn.close() diff --git a/tools/repro_playlist_determinism.py b/tools/repro_playlist_determinism.py new file mode 100644 index 0000000..61ff10a --- /dev/null +++ b/tools/repro_playlist_determinism.py @@ -0,0 +1,343 @@ +#!/usr/bin/env python3 +"""Reproduction harness for non-deterministic playlist output. + +WHY THIS EXISTS +--------------- +A real conversion produced two spurious ``PlaylistEntity`` rows (one each in +two playlists) that corresponded to no source playlist entry. Re-running +``convert`` over the *same* ``export.pdb`` produced a clean database. So the +writer's determinism guarantee did not hold, but a single re-run destroys the +evidence. One observation cannot tell you the reproduction rate, and it cannot +tell you which half of the pipeline is at fault. + +This harness answers both questions by bisecting the pipeline: + +* **Reader stage** — parse ``export.pdb`` N times and fingerprint the result. + Any variation here is a reader bug. The raw bytes are hashed too, so an + unreliable read is distinguishable from a non-deterministic parse. +* **Writer stage** — read the library *once*, then run ``build_library`` N + times from that single immutable input. Any variation here is a writer bug, + because the input is provably identical across runs. + +If both stages are stable the non-determinism is neither reader nor writer, and +the next suspect is the commit/copy/replace path or the environment. + +WHY NOT A SYMLINKED SHADOW ROOT +------------------------------- +An earlier version built each run against a temp directory holding symlinks to +the real ``PIONEER/`` and ``Contents/``. That was wrong and quietly so: +``engine_track_path`` calls ``.resolve()`` on both the track path and the drive +root (``writer/paths.py``), so the symlink collapsed to the real stick and +``relative_to`` raised. ``mapper/track.py`` swallows that error and degrades to +the raw path, so every track still "converted" and the harness reported a +confident verdict about a configuration nobody ships. The stage now runs +against the real drive root and redirects only the *output* directory, and +``_assert_paths_are_faithful`` fails loudly if the written paths ever stop +looking like a real conversion's. + +SAFETY +------ +``PIONEER/`` and ``Contents/`` are only ever read. Builds write to a scratch +``Engine Library.repro/`` beside the real library, which is removed after each +run; the real ``Engine Library/`` is never opened for writing. + +USAGE +----- + python tools/repro_playlist_determinism.py "/Volumes/USB DISK" --runs 5 + python tools/repro_playlist_determinism.py "/Volumes/USB DISK" --stage reader +""" + +from __future__ import annotations + +import argparse +import hashlib +import json +import shutil +import sqlite3 +import sys +import tempfile +from collections import Counter +from pathlib import Path + +# Engine sentinel: tail of a nextEntityId chain. +_NO_NEXT = 0 + +# Scratch library name used for builds. Distinct from "Engine Library" so a +# crash can never leave the user's real library half-written. +_SCRATCH_LIBRARY = "Engine Library.repro" + + +# -------------------------------------------------------------------------- +# fingerprints +# -------------------------------------------------------------------------- +def _hash(payload: object) -> str: + blob = json.dumps(payload, sort_keys=True, separators=(",", ":")).encode() + return hashlib.sha256(blob).hexdigest()[:16] + + +def source_fingerprint(lib) -> dict[str, object]: + """Everything about the source that can move a playlist chain. + + Membership alone is not enough: chains are built from ``track_id_map``, + whose ids come from the sorted set of tracks that resolved to a path. A + reader that non-deterministically drops one track shifts every Engine + Track.id and therefore every chain, while playlist membership looks + untouched. + """ + return { + "playlists": {str(pl.rb_id): list(pl.track_rb_ids) for pl in lib.playlists}, + "tracks": sorted(lib.tracks), + "resolved": { + str(rb): (str(t.resolved_path) if t.resolved_path else None) + for rb, t in lib.tracks.items() + }, + } + + +def entity_chains(m_db: Path) -> dict[str, list[int]]: + """Written-side playlist order, keyed by playlist title. + + Order is reconstructed through ``nextEntityId`` exactly as Engine reads it, + so a chain that is corrupt in a way row order would hide still shows up. + """ + conn = sqlite3.connect(f"file:{m_db}?mode=ro", uri=True) + try: + titles = {int(i): str(t) for i, t in conn.execute("SELECT id, title FROM Playlist")} + out: dict[str, list[int]] = {} + for list_id, title in titles.items(): + rows = conn.execute( + "SELECT id, trackId, nextEntityId FROM PlaylistEntity WHERE listId = ?", + (list_id,), + ).fetchall() + by_next = {int(n): (int(e), int(t)) for e, t, n in rows} + order: list[int] = [] + curr, seen = _NO_NEXT, set() + while curr in by_next: + eid, track_id = by_next[curr] + if eid in seen: # cycle guard — a corrupt chain must not hang + break + seen.add(eid) + order.insert(0, track_id) + curr = eid + # Rows unreachable from the chain are themselves a defect; surface + # them rather than reporting a shorter, tidier list. + if len(order) != len(rows): + order.append(-len(rows)) # sentinel makes the mismatch visible + out[f"{title}#{list_id}"] = order + return out + finally: + conn.close() + + +def _assert_paths_are_faithful(m_db: Path, expected_tracks: int) -> None: + """Fail unless the written Track paths look like a real conversion's. + + ``map_track`` degrades to the raw absolute path instead of raising when + relative-path arithmetic fails, so a misconfigured run still reports every + track converted. Without this check that degradation is invisible and the + harness would draw a confident conclusion from the wrong configuration. + """ + conn = sqlite3.connect(f"file:{m_db}?mode=ro", uri=True) + try: + total, relative = conn.execute( + "SELECT COUNT(*), SUM(CASE WHEN path LIKE '../%' THEN 1 ELSE 0 END) " + "FROM Track" + ).fetchone() + total, relative = int(total), int(relative or 0) + finally: + conn.close() + if total != expected_tracks or relative != total: + sample = sqlite3.connect(f"file:{m_db}?mode=ro", uri=True) + try: + bad = sample.execute( + "SELECT path FROM Track WHERE path NOT LIKE '../%' LIMIT 3" + ).fetchall() + finally: + sample.close() + raise SystemExit( + f"harness broken: {total} Track rows (expected {expected_tracks}), " + f"{total - relative} with non-relative paths e.g. {[b[0] for b in bad]} " + "— the run did not reproduce a real conversion's path arithmetic" + ) + + +# -------------------------------------------------------------------------- +# stages +# -------------------------------------------------------------------------- +def run_reader_stage(root: Path, runs: int) -> None: + """Parse the pdb `runs` times; report distinct fingerprints.""" + from rb2engine.reader.pdb import parse_export_pdb + from rb2engine.reader.scan import scan_drive + + print(f"\n=== reader stage: parsing export.pdb x{runs} ===") + seen: Counter[str] = Counter() + byte_hashes: Counter[str] = Counter() + first: dict[str, object] | None = None + for i in range(1, runs + 1): + pdb_path = scan_drive(root).export_pdb + # Hash the bytes as read. If the parse varies while these agree, the + # parser is at fault; if these vary, the read itself is unreliable. + byte_hashes[hashlib.sha256(pdb_path.read_bytes()).hexdigest()[:16]] += 1 + lib = parse_export_pdb(pdb_path, root) + fingerprint = source_fingerprint(lib) + fp = _hash(fingerprint) + seen[fp] += 1 + if first is None: + first = fingerprint + entries = sum(len(v) for v in fingerprint["playlists"].values()) # type: ignore[union-attr] + print(f" run {i}: fp={fp} playlists={len(fingerprint['playlists'])} " # type: ignore[arg-type] + f"tracks={len(fingerprint['tracks'])} entries={entries}") # type: ignore[arg-type] + else: + same = fp == _hash(first) + print(f" run {i}: fp={fp} {'same' if same else '*** DIFFERENT ***'}") + if not same: + report_membership_diff( + first["playlists"], fingerprint["playlists"] # type: ignore[index,arg-type] + ) + print(f" distinct fingerprints: {len(seen)} ->", dict(seen)) + print(f" distinct export.pdb byte hashes: {len(byte_hashes)} ->", dict(byte_hashes)) + if len(byte_hashes) > 1: + print(" VERDICT: *** export.pdb read returned different bytes — I/O layer ***") + elif len(seen) > 1: + print(" VERDICT: *** parser is non-deterministic on identical bytes ***") + else: + print(" VERDICT: reader is deterministic across these runs") + + +def run_writer_stage( + root: Path, runs: int, *, artwork: bool, keep_dir: Path +) -> None: + """Read once, build `runs` times from that identical input. + + *artwork* mirrors the real ``convert`` default. It is far slower (every + audio file is opened) but it is the configuration the observed failure + actually ran under, so a repro attempt that skips it is not faithful. + """ + from rb2engine.reader.library import read_library + from rb2engine.report import ConversionReport + from rb2engine.writer import build as build_mod + + print(f"\n=== writer stage: build_library x{runs} " + f"(artwork={'on' if artwork else 'off'}) from one immutable read ===") + # ANLZ never affects playlist chains, so it stays off regardless. + lib = read_library(root, with_anlz=False, with_artwork=artwork) + src_entries = sum(len(pl.track_rb_ids) for pl in lib.playlists) + print(f" source: {len(lib.tracks)} tracks, {len(lib.playlists)} playlists, " + f"{src_entries} playlist entries") + + scratch = root / _SCRATCH_LIBRARY + seen: Counter[str] = Counter() + baseline: dict[str, list[int]] | None = None + original_dirname = build_mod.ENGINE_LIBRARY_DIRNAME + try: + # Redirect only the output. drive_root stays the real stick so that + # .resolve()-based path arithmetic matches a real conversion exactly. + build_mod.ENGINE_LIBRARY_DIRNAME = _SCRATCH_LIBRARY + for i in range(1, runs + 1): + shutil.rmtree(scratch, ignore_errors=True) + report = ConversionReport() + m_db = build_mod.build_library( + lib, drive_root=root, report=report, with_artwork=artwork + ) + _assert_paths_are_faithful(m_db, len(lib.tracks)) + + chains = entity_chains(m_db) + fp = _hash(chains) + seen[fp] += 1 + written = sum(len(v) for v in chains.values()) + if baseline is None: + baseline = chains + # Keep the first database. Without a reference copy a later + # divergence can only be described, not diffed — which is + # exactly how the original evidence was lost. + keep_dir.mkdir(parents=True, exist_ok=True) + shutil.copyfile(m_db, keep_dir / "baseline.m.db") + print(f" run {i}: fp={fp} entries_written={written} " + f"(source {src_entries}, delta {written - src_entries:+d})") + print(f" baseline preserved -> {keep_dir / 'baseline.m.db'}") + else: + same = fp == _hash(baseline) + print(f" run {i}: fp={fp} entries_written={written} " + f"{'same' if same else '*** DIFFERENT ***'}") + if not same: + report_chain_diff(baseline, chains) + kept = keep_dir / f"diverged-run{i}.m.db" + keep_dir.mkdir(parents=True, exist_ok=True) + shutil.copyfile(m_db, kept) + print(f" *** DIVERGING DB PRESERVED -> {kept} ***") + finally: + build_mod.ENGINE_LIBRARY_DIRNAME = original_dirname + shutil.rmtree(scratch, ignore_errors=True) + + print(f" distinct fingerprints: {len(seen)} ->", dict(seen)) + if len(seen) == 1: + print(" VERDICT: writer is deterministic across these runs") + else: + print(" VERDICT: *** writer is NON-deterministic — reproduced ***") + + +# -------------------------------------------------------------------------- +# diffs +# -------------------------------------------------------------------------- +def report_membership_diff(a: dict[str, list[int]], b: dict[str, list[int]]) -> None: + for key in sorted(set(a) | set(b)): + va, vb = a.get(key, []), b.get(key, []) + if va != vb: + print(f" playlist rb_id={key}: {len(va)} -> {len(vb)} entries") + print(f" only in A: {sorted((Counter(va) - Counter(vb)).elements())}") + print(f" only in B: {sorted((Counter(vb) - Counter(va)).elements())}") + + +def report_chain_diff(a: dict[str, list[int]], b: dict[str, list[int]]) -> None: + for key in sorted(set(a) | set(b)): + va, vb = a.get(key, []), b.get(key, []) + if va != vb: + print(f" {key}: {len(va)} -> {len(vb)} entries") + print(f" only in baseline: {sorted((Counter(va) - Counter(vb)).elements())}") + print(f" only in this run: {sorted((Counter(vb) - Counter(va)).elements())}") + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("drive_root", type=Path) + ap.add_argument("--runs", type=int, default=5) + ap.add_argument("--stage", choices=("reader", "writer", "both"), default="both") + ap.add_argument( + "--no-artwork", + action="store_true", + help="skip artwork extraction: much faster, but no longer the " + "configuration the observed failure ran under", + ) + ap.add_argument( + "--keep-dir", + type=Path, + default=None, + help="where a baseline and any diverging m.db are preserved " + "(default: a fresh temp directory, reported on stdout)", + ) + args = ap.parse_args() + + if args.runs < 1: + print("--runs must be >= 1", file=sys.stderr) + return 2 + + root = args.drive_root + if not (root / "PIONEER").is_dir(): + print(f"no PIONEER/ under {root}", file=sys.stderr) + return 2 + + # Databases are hundreds of MB and contain the user's library; never + # default to dropping them into the working tree. + keep_dir = args.keep_dir or Path(tempfile.mkdtemp(prefix="rb2engine-repro-")) + + if args.stage in ("reader", "both"): + run_reader_stage(root, args.runs) + if args.stage in ("writer", "both"): + run_writer_stage( + root, args.runs, artwork=not args.no_artwork, keep_dir=keep_dir + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 685e059a77fae20deb321f31bcba5c1feb0cb56c Mon Sep 17 00:00:00 2001 From: jrgutier Date: Wed, 29 Jul 2026 09:09:04 -0500 Subject: [PATCH 2/2] Make verify and the writer agree on a broken chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two oracles disagreed about what a valid PlaylistEntity chain is. The writer's new gate treats a row the chain never reaches as fatal and refuses to publish. verify walked the same structure, returned whatever it reached, and reported the library clean — so verify would pass a database convert would refuse to write. On a real stick that eventually shows up as one tool contradicting the other about the same file. One walker ---------- rb2engine/chain.py is now the only implementation. It raises ChainInconsistent (a RuntimeError subclass, so the writer's documented contract is unchanged) when the chain forks or fails to account for every row. They still need different reactions, and that is the point of raising rather than returning a flag: the writer must abort before publishing, while verify must record the finding and keep checking the rest of the library. What they can no longer do is disagree about whether there is anything to react to. verify now reports it --------------------- _entity_track_order returns (order, problem). A problem becomes its own `playlist[NAME].chain` discrepancy rather than surfacing indirectly as whatever track order a truncated walk happened to produce — and it is reported in its own right, because a broken chain is a defect even when the set of tracks still matches what the source expected. This removes the fourth copy of the walk as well; the harness uses the shared one and folds any chain problem into its fingerprint, so a run that corrupts a chain can no longer compare equal to a run that did not. Tests ----- Both chain tests were confirmed red against the old lenient walker and green after: the pre-existing cycle test (which now names the orphaned row instead of asserting on a truncated order) and a new test that strands a row behind a successor id that does not exist, leaving the rest of the chain well-formed so only the row count reveals it. 694 passing (693 before), ruff clean. verify on a 3,673-track stick still reports 0 discrepancies, so the stricter walk does not misfire on a real library. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AxFS98fHa9GwJzPnfLPVQr --- src/rb2engine/chain.py | 87 +++++++++++++++++++++++++++++ src/rb2engine/verify.py | 62 ++++++++++++-------- src/rb2engine/writer/playlists.py | 45 +-------------- tests/unit/test_verify.py | 42 +++++++++++++- tests/unit/test_write_playlists.py | 6 +- tools/repro_playlist_determinism.py | 39 ++++++------- 6 files changed, 189 insertions(+), 92 deletions(-) create mode 100644 src/rb2engine/chain.py diff --git a/src/rb2engine/chain.py b/src/rb2engine/chain.py new file mode 100644 index 0000000..201c7dd --- /dev/null +++ b/src/rb2engine/chain.py @@ -0,0 +1,87 @@ +"""The one place that reconstructs an Engine ``nextEntityId`` chain. + +Engine stores playlist membership as a singly linked list: each +``PlaylistEntity`` row points at its successor and the tail points at 0. Track +order is whatever that chain says, so anything wanting to know "what does this +playlist actually contain" has to walk it. + +WHY THIS IS SHARED +------------------ +The walk existed in three places that had drifted apart. The writer's copy +treated a row the chain never reaches as fatal; ``verify``'s copy silently +returned the shorter, tidier list and reported no problem. Two oracles that +disagree about what "correct" means will eventually contradict each other on a +real stick — one refusing to publish a database the other calls clean. + +They still need different *reactions*: the writer must abort before publishing, +while ``verify`` must record the finding and carry on checking. So the walk +raises, and each caller decides what that means. What they can no longer do is +disagree about whether there is something to react to. +""" + +from __future__ import annotations + +from collections.abc import Sequence + +# Engine / libdjinterop sentinel: tail of every next*Id chain points at 0. +NO_NEXT = 0 + + +class ChainInconsistent(RuntimeError): + """A ``nextEntityId`` chain does not account for every row in its list. + + Subclasses ``RuntimeError`` because the writer's contract is to raise + ``RuntimeError`` on a database it refuses to publish, and callers that only + care about that broader promise should not need to know this type exists. + """ + + def __init__(self, list_id: int, message: str) -> None: + super().__init__(f"playlist listId={list_id}: {message}") + self.list_id = list_id + + +def walk_entity_chain( + list_id: int, rows: Sequence[tuple[int, int, int]] +) -> list[int]: + """Track ids for one list in chain order, from ``(id, trackId, nextEntityId)``. + + Walking the chain rather than reading rows in id order is deliberate: a row + spliced into the middle, or one no walk reaches, is invisible to a plain + ``ORDER BY id`` comparison. A real conversion published a spurious entry + second-to-last, exactly where row order would have hidden it. + + Raises ``ChainInconsistent`` if the chain forks or fails to reach every row. + """ + by_next: dict[int, tuple[int, int]] = {} + for eid, tid, nxt in rows: + # Two rows sharing a successor would silently collapse into one dict + # entry. The count check below would still fire, but it would blame the + # wrong thing, so name this corruption for what it is. + if int(nxt) in by_next: + raise ChainInconsistent( + list_id, + f"two PlaylistEntity rows share nextEntityId={int(nxt)} — " + "chain is forked", + ) + by_next[int(nxt)] = (int(eid), int(tid)) + + order: list[int] = [] + curr = NO_NEXT + seen: set[int] = set() + while curr in by_next: + eid, track_id = by_next[curr] + if eid in seen: # corrupt chain must not spin forever + break + seen.add(eid) + order.insert(0, track_id) + curr = eid + + # A row that no chain walk reaches is still a row Engine may honour; make + # the count mismatch loud instead of letting the walk hide it. + if len(order) != len(rows): + raise ChainInconsistent( + list_id, + f"{len(rows)} PlaylistEntity rows but the nextEntityId chain " + f"reaches {len(order)} — chain is inconsistent", + ) + return order diff --git a/src/rb2engine/verify.py b/src/rb2engine/verify.py index 3017cae..a02c499 100644 --- a/src/rb2engine/verify.py +++ b/src/rb2engine/verify.py @@ -14,6 +14,7 @@ from pathlib import Path from typing import Any +from rb2engine.chain import ChainInconsistent, walk_entity_chain from rb2engine.errors import FatalError from rb2engine.ir import SourceLibrary, SourcePlaylist, SourceTrack from rb2engine.ir_engine import artwork_content_hash @@ -34,9 +35,6 @@ # Empty-slot sentinel shared with writer/blobs and ir_engine. _EMPTY_SAMPLE = -1.0 -# Engine / libdjinterop: tail of every nextEntityId chain. -_NO_NEXT = 0 - @dataclass(frozen=True, slots=True) class Discrepancy: @@ -563,7 +561,18 @@ def _compare_playlists( expected_track_ids = _expected_entity_track_ids( pl, source, by_path=by_path, drive_root=drive_root, engine_lib=engine_lib ) - actual_track_ids = _entity_track_order(conn, list_id) + actual_track_ids, chain_problem = _entity_track_order(conn, list_id) + if chain_problem is not None: + # Report it in its own right: a broken chain is a defect even when + # the set of tracks happens to match what the source expected. + discrepancies.append( + Discrepancy( + track_id=None, + field=f"playlist[{pl.name}].chain", + expected="every row reachable from the nextEntityId chain", + actual=chain_problem, + ) + ) if expected_track_ids != actual_track_ids: discrepancies.append( Discrepancy( @@ -615,25 +624,32 @@ def _expected_entity_track_ids( return out -def _entity_track_order(conn: sqlite3.Connection, list_id: int) -> list[int]: - """Reconstruct track order from nextEntityId (tail sentinel = 0).""" - rows = conn.execute( - "SELECT id, trackId, nextEntityId FROM PlaylistEntity WHERE listId = ?", - (list_id,), - ).fetchall() - by_next = {int(next_id): (int(eid), int(track_id)) for eid, track_id, next_id in rows} - order: list[int] = [] - curr = _NO_NEXT - # Guard against cycles in a corrupted chain. - seen_entity: set[int] = set() - while curr in by_next: - eid, track_id = by_next[curr] - if eid in seen_entity: - break - seen_entity.add(eid) - order.insert(0, track_id) - curr = eid - return order +def _entity_track_order( + conn: sqlite3.Connection, list_id: int +) -> tuple[list[int], str | None]: + """Track order from the nextEntityId chain, plus any inconsistency found. + + Returns ``(order, problem)``. ``problem`` is None when the chain accounts + for every row; otherwise it describes what is wrong and ``order`` holds the + rows in id order as a best effort. + + This used to swallow an inconsistent chain and return the shorter, tidier + list, which reported a clean library while the writer's own gate would + refuse to publish that exact database. Both now walk the same code + (``rb2engine.chain``); they differ only in how they react, because ``verify`` + must record the finding and keep checking rather than abort. + """ + rows = [ + (int(eid), int(track_id), int(next_id)) + for eid, track_id, next_id in conn.execute( + "SELECT id, trackId, nextEntityId FROM PlaylistEntity WHERE listId = ?", + (list_id,), + ) + ] + try: + return walk_entity_chain(list_id, rows), None + except ChainInconsistent as exc: + return [track_id for _, track_id, _ in rows], str(exc) def _compare_artwork( diff --git a/src/rb2engine/writer/playlists.py b/src/rb2engine/writer/playlists.py index 57fc95d..8978515 100644 --- a/src/rb2engine/writer/playlists.py +++ b/src/rb2engine/writer/playlists.py @@ -22,6 +22,7 @@ from collections import defaultdict from collections.abc import Mapping, MutableMapping, Sequence +from rb2engine.chain import walk_entity_chain from rb2engine.ir import SourcePlaylist # Engine / libdjinterop sentinel: tail of every next*Id chain. @@ -264,48 +265,6 @@ def insert_playlists( return len(playlists) -def _walk_entity_chain( - list_id: int, rows: Sequence[tuple[int, int, int]] -) -> list[int]: - """Track order for one list, walked through ``nextEntityId`` like Engine. - - Walking the chain rather than reading the raw rows is deliberate: a row that - exists but is unreachable, or one spliced into the middle, is invisible to a - plain ``SELECT ... ORDER BY id`` and is exactly the defect this must catch. - """ - by_next: dict[int, tuple[int, int]] = {} - for eid, tid, nxt in rows: - # Two rows sharing a successor would silently collapse into one dict - # entry. The count check below would still fire, but it would blame the - # wrong thing, so name this corruption for what it is. - if int(nxt) in by_next: - raise RuntimeError( - f"playlist listId={list_id}: two PlaylistEntity rows share " - f"nextEntityId={int(nxt)} — chain is forked" - ) - by_next[int(nxt)] = (int(eid), int(tid)) - - order: list[int] = [] - curr = _NO_NEXT - seen: set[int] = set() - while curr in by_next: - eid, track_id = by_next[curr] - if eid in seen: # corrupt chain must not spin forever - break - seen.add(eid) - order.insert(0, track_id) - curr = eid - - # A row that no chain walk reaches is still a row Engine may honour; make - # the count mismatch loud instead of letting the walk hide it. - if len(order) != len(rows): - raise RuntimeError( - f"playlist listId={list_id}: {len(rows)} PlaylistEntity rows but the " - f"nextEntityId chain reaches {len(order)} — chain is inconsistent" - ) - return order - - def assert_entities_match_intent( conn: sqlite3.Connection, intended: Mapping[int, Sequence[int]] ) -> None: @@ -337,7 +296,7 @@ def assert_entities_match_intent( ) for list_id, expected in intended.items(): - actual = _walk_entity_chain(list_id, grouped.get(list_id, [])) + actual = walk_entity_chain(list_id, grouped.get(list_id, [])) if list(actual) != list(expected): extra = sorted(set(actual) - set(expected)) missing = sorted(set(expected) - set(actual)) diff --git a/tests/unit/test_verify.py b/tests/unit/test_verify.py index adc279d..137ab01 100644 --- a/tests/unit/test_verify.py +++ b/tests/unit/test_verify.py @@ -1104,10 +1104,14 @@ def test_verify_catches_extra_playlist_count( def test_verify_playlist_chain_cycle_does_not_hang( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """A cyclic nextEntityId chain must terminate and still flag bad order. + """A cyclic nextEntityId chain must terminate and still be flagged. WHY: Without a cycle guard, a corrupted stick could hang verify forever — worse than reporting a discrepancy. + + The cycle orphans e3, so it surfaces as a `.chain` discrepancy naming the + unreachable row. It used to be reported only indirectly, as whatever track + order the truncated walk happened to produce. """ from rb2engine.verify import verify_library @@ -1134,7 +1138,41 @@ def test_verify_playlist_chain_cycle_does_not_hang( result = verify_library(drive, with_artwork=False) assert result.ok is False - assert "playlist[Main Set].track_order" in _fields(result.discrepancies) + assert "playlist[Main Set].chain" in _fields(result.discrepancies) + + +def test_verify_reports_unreachable_entity_row( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An orphaned row must be reported, not quietly dropped from the order. + + WHY: verify used to walk the chain and return whatever it reached, so a row + Engine may still honour vanished from the comparison and the library was + reported clean — while the writer's own gate refuses to publish that exact + database. Both now share rb2engine.chain, so they cannot disagree about + whether there is a defect. + """ + from rb2engine.verify import verify_library + + drive, lib, m_db = _build_fixture(tmp_path) + _patch_read_library(monkeypatch, lib) + + conn = sqlite3.connect(str(m_db)) + entities = conn.execute( + "SELECT id FROM PlaylistEntity WHERE listId = 1 ORDER BY id" + ).fetchall() + e2, e3 = entities[1][0], entities[2][0] + # Make e2 the tail and strand e3 behind a successor id that does not exist. + # The chain is otherwise well-formed, so only the row count reveals e3. + conn.execute("UPDATE PlaylistEntity SET nextEntityId = 0 WHERE id = ?", (e2,)) + conn.execute("UPDATE PlaylistEntity SET nextEntityId = 8888 WHERE id = ?", (e3,)) + conn.commit() + conn.close() + + result = verify_library(drive, with_artwork=False) + assert result.ok is False + fields = _fields(result.discrepancies) + assert "playlist[Main Set].chain" in fields def test_verify_playlist_skips_unknown_and_unresolved_source_tracks( diff --git a/tests/unit/test_write_playlists.py b/tests/unit/test_write_playlists.py index 256155a..b097400 100644 --- a/tests/unit/test_write_playlists.py +++ b/tests/unit/test_write_playlists.py @@ -19,11 +19,11 @@ import pytest +from rb2engine.chain import walk_entity_chain from rb2engine.ir import SourcePlaylist from rb2engine.writer import playlists as playlists_mod from rb2engine.writer import schema as schema_mod from rb2engine.writer.playlists import ( - _walk_entity_chain, assert_entities_match_intent, insert_playlists, ) @@ -306,7 +306,7 @@ def _chain(conn: sqlite3.Connection, list_id: int) -> list[int]: "SELECT id, trackId, nextEntityId FROM PlaylistEntity WHERE listId = ?", (list_id,), ).fetchall() - return _walk_entity_chain(list_id, [(int(a), int(b), int(c)) for a, b, c in rows]) + return walk_entity_chain(list_id, [(int(a), int(b), int(c)) for a, b, c in rows]) def test_integrity_gate_accepts_a_faithful_write(tmp_path: Path) -> None: @@ -330,7 +330,7 @@ def test_insert_playlists_actually_invokes_the_gate( """ monkeypatch.setattr( playlists_mod, - "_walk_entity_chain", + "walk_entity_chain", lambda _list_id, rows: [*(int(t) for _, t, _ in rows), 4242], ) playlists = [ diff --git a/tools/repro_playlist_determinism.py b/tools/repro_playlist_determinism.py index 61ff10a..d1f417f 100644 --- a/tools/repro_playlist_determinism.py +++ b/tools/repro_playlist_determinism.py @@ -59,9 +59,6 @@ from collections import Counter from pathlib import Path -# Engine sentinel: tail of a nextEntityId chain. -_NO_NEXT = 0 - # Scratch library name used for builds. Distinct from "Engine Library" so a # crash can never leave the user's real library half-written. _SCRATCH_LIBRARY = "Engine Library.repro" @@ -100,29 +97,29 @@ def entity_chains(m_db: Path) -> dict[str, list[int]]: Order is reconstructed through ``nextEntityId`` exactly as Engine reads it, so a chain that is corrupt in a way row order would hide still shows up. """ + from rb2engine.chain import ChainInconsistent, walk_entity_chain + conn = sqlite3.connect(f"file:{m_db}?mode=ro", uri=True) try: titles = {int(i): str(t) for i, t in conn.execute("SELECT id, title FROM Playlist")} out: dict[str, list[int]] = {} for list_id, title in titles.items(): - rows = conn.execute( - "SELECT id, trackId, nextEntityId FROM PlaylistEntity WHERE listId = ?", - (list_id,), - ).fetchall() - by_next = {int(n): (int(e), int(t)) for e, t, n in rows} - order: list[int] = [] - curr, seen = _NO_NEXT, set() - while curr in by_next: - eid, track_id = by_next[curr] - if eid in seen: # cycle guard — a corrupt chain must not hang - break - seen.add(eid) - order.insert(0, track_id) - curr = eid - # Rows unreachable from the chain are themselves a defect; surface - # them rather than reporting a shorter, tidier list. - if len(order) != len(rows): - order.append(-len(rows)) # sentinel makes the mismatch visible + rows = [ + (int(e), int(t), int(n)) + for e, t, n in conn.execute( + "SELECT id, trackId, nextEntityId FROM PlaylistEntity " + "WHERE listId = ?", + (list_id,), + ) + ] + try: + order = walk_entity_chain(list_id, rows) + except ChainInconsistent as exc: + # Fold the problem into the fingerprint so a run that corrupts a + # chain cannot compare equal to a run that did not. + order = [t for _, t, _ in rows] + out[f"{title}#{list_id}!chain"] = [len(rows), len(order)] + print(f" chain problem on {title!r}: {exc}") out[f"{title}#{list_id}"] = order return out finally: