Catch a wrong playlist write before it is published

[jrgutier]

What happened

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 caught 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, because it is not identified. What is fixed is that this class of corruption could be published silently, and that the two oracles disagreed about what "correct" even means.

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). Repeated parses returned identical playlist membership every time, with matching byte hashes.
• insert_playlists can only ever drop 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; the positional repair in build.py is also 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.

One initial condition could not be recreated: the failing run was the only conversion that started from the older pre-existing database, and that artifact is gone.

The two gates

1. In-transaction (writer/playlists.py) — insert_playlists reads its own nextEntityId chains back and raises if they disagree with intent. It walks 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 empty playlist is exactly as wrong.

2. Post-copy, pre-swap (writer/build.py) — the in-transaction check can only prove SQLite agreed at that moment, so the copy that actually crossed to the target volume is re-checked after fsync and before os.replace. A failure there leaves the previous m.db byte-for-byte intact and surfaces through the existing FatalError path as exit 2.

One chain walker

The walk existed in four places that had drifted apart. The writer treated an unreachable row as fatal; verify returned the shorter, tidier list and reported the library clean — so verify would pass a database convert refuses to write.

rb2engine/chain.py is now the only implementation. It raises ChainInconsistent (a RuntimeError subclass, so the writer's contract is unchanged) when a chain forks or fails to account for every row. Raising rather than returning a flag is deliberate: the callers need different reactions — the writer aborts before publishing, verify records the finding and keeps checking — but they can no longer disagree about whether there is anything to react to.

verify now reports it as its own playlist[NAME].chain discrepancy, in its own right, because a broken chain is a defect even when the set of tracks still matches the source.

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 exactly what lost the original evidence.

It deliberately 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.

Testing

Every check here was confirmed to fail before it passed:

• Removing the gate call from insert_playlists turns the suite red (it did not in the first version of these tests).
• Both chain tests were confirmed red against the old lenient walker.
• Corruption tests splice a row mid-chain, orphan a row behind a nonexistent successor, delete an entry and repair its predecessor, fork a chain, and place a row on an unintended list.
• 694 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 none of the stricter checks misfire on a real library.

One pre-existing test changed meaning: the cycle test now asserts the .chain discrepancy naming the orphaned row, instead of whatever track order the truncated walk happened to produce.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AxFS98fHa9GwJzPnfLPVQr