#905 (root cause): Navidrome reconcile read current tracks via missing t.id -> playlists doubled

reconcile_playlist read the existing playlist's track ids with str(t.id), but NavidromeTrack
exposes the Subsonic song id as .ratingKey and has NO .id attribute (append_to_playlist already
reads ratingKey — reconcile was the straggler). So current_ids came back EMPTY every time:
plan_playlist_reconcile saw an 'empty' playlist, re-added the entire matched set, and removed
nothing. Result: the playlist grew by the full track count on every sync (warl0ck: 5 songs, one
removed -> 9), in reconcile mode and whenever reconcile is the active mode.

Fix: read current ids via ratingKey, matching append_to_playlist.

Verified: tests/test_navidrome_reconcile.py drives the real reconcile_playlist with a stubbed
server — reverting the one-char change flips 3 tests red (they show it re-adding all 4 tracks),
the fix flips them green. Covers no-op resync, a removed track (remove, don't re-add all), and an
added track (append once).

core/navidrome_client.py

@@ -1192,7 +1192,11 @@ class NavidromeClient(MediaServerClient):
 
             primary = existing_playlists[0]
             existing_tracks = self.get_playlist_tracks(primary.id)
-            current_ids = [str(t.id) for t in existing_tracks if getattr(t, 'id', None)]
+            # #905: NavidromeTrack exposes the Subsonic song id as `ratingKey` (NOT `.id`,
+            # which doesn't exist) — same as append_to_playlist reads it. Reading `t.id` here
+            # made current_ids ALWAYS empty, so reconcile thought the playlist was empty and
+            # re-added every track each sync (playlists doubling) while removing nothing.
+            current_ids = [str(t.ratingKey) for t in existing_tracks if getattr(t, 'ratingKey', None)]
             desired_ids = []
             for t in tracks:
                 tid = (str(t.ratingKey) if hasattr(t, 'ratingKey')

tests/test_navidrome_reconcile.py

@@ -0,0 +1,74 @@
+"""Navidrome reconcile must read the CURRENT playlist via ratingKey (#905).
+
+NavidromeTrack exposes the Subsonic song id as ``ratingKey`` — it has no ``.id``.
+reconcile_playlist used ``t.id``, so the current-track list came back empty, the
+add/remove plan saw "nothing is here", and every sync re-added the whole matched
+set (playlists doubling). These tests drive the real reconcile_playlist with a
+stubbed client and assert the Subsonic params reflect the true delta.
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+from core.navidrome_client import NavidromeClient, NavidromeTrack
+
+
+def _track(song_id):
+    """A NavidromeTrack as get_playlist_tracks / the matcher produce them."""
+    return NavidromeTrack({'id': song_id, 'title': f'Song {song_id}'}, client=None)
+
+
+def test_navidrome_track_exposes_ratingkey_not_id():
+    # Root cause: the attribute is ratingKey, NOT id.
+    t = _track('42')
+    assert t.ratingKey == '42'
+    assert not hasattr(t, 'id')
+
+
+def _client_with_existing(existing_ids):
+    """A NavidromeClient whose server playlist already holds `existing_ids`,
+    capturing the Subsonic params reconcile sends."""
+    c = NavidromeClient.__new__(NavidromeClient)
+    c.ensure_connection = lambda: True
+    c.get_playlists_by_name = lambda name: [SimpleNamespace(id='PL1')]
+    c.get_playlist_tracks = lambda pid: [_track(i) for i in existing_ids]
+    captured = {}
+
+    def _req(method, params):
+        captured['method'] = method
+        captured['params'] = params
+        return {'status': 'ok'}
+
+    c._make_request = _req
+    return c, captured
+
+
+def test_no_change_resync_is_a_noop():
+    # Playlist already == desired → reconcile must add/remove NOTHING.
+    c, captured = _client_with_existing(['1', '2', '3', '4'])
+    desired = [_track('1'), _track('2'), _track('3'), _track('4')]
+    assert c.reconcile_playlist('My Playlist', desired) is True
+    # plan empty → early return, updatePlaylist never called (no re-add).
+    assert 'params' not in captured
+
+
+def test_removed_source_track_is_removed_not_everything_readded():
+    # warl0ck's exact case: server has 1..5, source now has 1..4 (5 removed).
+    c, captured = _client_with_existing(['1', '2', '3', '4', '5'])
+    desired = [_track('1'), _track('2'), _track('3'), _track('4')]
+    assert c.reconcile_playlist('My Playlist', desired) is True
+    params = captured['params']
+    assert params['playlistId'] == 'PL1'
+    assert 'songIdToAdd' not in params           # the bug: would re-add 1..4 → doubling
+    assert params['songIndexToRemove'] == [4]    # only song '5' (index 4) is removed
+
+
+def test_added_source_track_is_appended_once():
+    # Server has 1..3, source now has 1..4 → add only '4'.
+    c, captured = _client_with_existing(['1', '2', '3'])
+    desired = [_track('1'), _track('2'), _track('3'), _track('4')]
+    assert c.reconcile_playlist('My Playlist', desired) is True
+    params = captured['params']
+    assert params['songIdToAdd'] == ['4']
+    assert 'songIndexToRemove' not in params