fix: preserve album track order during ATProto sync (#592)

* fix: preserve album track order during ATProto sync (#587)

when syncing album list records to ATProto, the background sync was
always ordering tracks by created_at, overwriting any custom order
the user had set via the frontend reorder feature.

now the sync:
1. fetches the existing ATProto list record order (if any)
2. preserves that order for existing tracks
3. appends any new tracks at the end (sorted by created_at)

this prevents user-reordered albums from reverting on login.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: use unique DID and cleanup in test to avoid CI conflicts

the test was sharing fixtures with fixed DIDs causing foreign key
constraint violations during teardown when running in parallel with xdist.

now uses:
- unique DID with uuid suffix for test isolation
- explicit cleanup in finally block (tracks → albums → artists order)
- properly typed capture function for lint compliance

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

authored by zzstoatzz.io Claude Opus 4.5 and committed by GitHub 2ffe1437 77dbde30

Changed files
+205 -4
backend
src
backend
_internal
atproto
tests
+68 -4
backend/src/backend/_internal/atproto/sync.py
··· 6 6 7 7 from backend._internal import Session as AuthSession 8 8 from backend._internal.atproto.records.fm_plyr import ( 9 + get_record_public, 9 10 upsert_album_list_record, 10 11 upsert_liked_list_record, 11 12 upsert_profile_record, ··· 15 16 logger = logging.getLogger(__name__) 16 17 17 18 19 + async def _get_existing_track_order( 20 + album_atproto_uri: str, 21 + artist_pds_url: str | None, 22 + ) -> list[str]: 23 + """fetch existing track URIs from ATProto list record. 24 + 25 + returns list of track URIs in the order stored in ATProto, 26 + or empty list if fetch fails. 27 + """ 28 + if not album_atproto_uri or not artist_pds_url: 29 + return [] 30 + 31 + try: 32 + record_data = await get_record_public( 33 + record_uri=album_atproto_uri, 34 + pds_url=artist_pds_url, 35 + ) 36 + items = record_data.get("value", {}).get("items", []) 37 + return [ 38 + item.get("subject", {}).get("uri") 39 + for item in items 40 + if item.get("subject", {}).get("uri") 41 + ] 42 + except Exception as e: 43 + logger.debug(f"could not fetch existing ATProto record order: {e}") 44 + return [] 45 + 46 + 18 47 async def sync_atproto_records( 19 48 auth_session: AuthSession, 20 49 user_did: str, ··· 44 73 45 74 # query and sync album list records 46 75 async with db_session() as session: 76 + # fetch artist for PDS URL (needed for ATProto record fetch) 77 + artist_result = await session.execute( 78 + select(Artist).where(Artist.did == user_did) 79 + ) 80 + artist = artist_result.scalar_one_or_none() 81 + artist_pds_url = artist.pds_url if artist else None 82 + 47 83 albums_result = await session.execute( 48 84 select(Album).where(Album.artist_did == user_did) 49 85 ) ··· 51 87 52 88 for album in albums: 53 89 tracks_result = await session.execute( 54 - select(Track) 55 - .where( 90 + select(Track).where( 56 91 Track.album_id == album.id, 57 92 Track.atproto_record_uri.isnot(None), 58 93 Track.atproto_record_cid.isnot(None), 59 94 ) 60 - .order_by(Track.created_at.asc()) 61 95 ) 62 - tracks = tracks_result.scalars().all() 96 + tracks = list(tracks_result.scalars().all()) 63 97 64 98 if tracks: 99 + # if album has existing ATProto record, preserve its track order 100 + # this prevents overwriting user's custom reordering 101 + existing_order = await _get_existing_track_order( 102 + album.atproto_record_uri, artist_pds_url 103 + ) 104 + 105 + if existing_order: 106 + # order tracks by existing ATProto order, append new tracks at end 107 + track_by_uri = {t.atproto_record_uri: t for t in tracks} 108 + ordered_tracks = [] 109 + seen_uris = set() 110 + 111 + # first: tracks in existing order 112 + for uri in existing_order: 113 + if uri in track_by_uri: 114 + ordered_tracks.append(track_by_uri[uri]) 115 + seen_uris.add(uri) 116 + 117 + # then: any new tracks not in existing order (by created_at) 118 + new_tracks = [ 119 + t for t in tracks if t.atproto_record_uri not in seen_uris 120 + ] 121 + new_tracks.sort(key=lambda t: t.created_at) 122 + ordered_tracks.extend(new_tracks) 123 + 124 + tracks = ordered_tracks 125 + else: 126 + # no existing order - use created_at (default for new albums) 127 + tracks.sort(key=lambda t: t.created_at) 128 + 65 129 track_refs = [ 66 130 {"uri": t.atproto_record_uri, "cid": t.atproto_record_cid} 67 131 for t in tracks
+137
backend/tests/api/test_list_record_sync.py
··· 336 336 await sync_atproto_records(mock_session, "did:plc:testartist123") 337 337 338 338 339 + async def test_sync_preserves_existing_album_track_order( 340 + db_session: AsyncSession, 341 + ): 342 + """test that sync preserves user's custom track order from ATProto. 343 + 344 + regression test for issue #587: album track reorder reverts. 345 + when an album already has an ATProto list record with a custom order, 346 + sync should preserve that order instead of overwriting with created_at. 347 + """ 348 + import uuid 349 + from datetime import UTC, datetime, timedelta 350 + 351 + from sqlalchemy import delete 352 + 353 + from backend._internal.atproto import sync_atproto_records 354 + 355 + # use unique DID to avoid conflicts with other tests 356 + unique_did = f"did:plc:ordertest{uuid.uuid4().hex[:8]}" 357 + 358 + # create artist with pds_url (required for ATProto record fetch) 359 + artist = Artist( 360 + did=unique_did, 361 + handle="ordertest.bsky.social", 362 + display_name="Order Test Artist", 363 + pds_url="https://test.pds", 364 + ) 365 + db_session.add(artist) 366 + await db_session.flush() 367 + 368 + # create album with existing ATProto record 369 + album = Album( 370 + artist_did=unique_did, 371 + title="Reordered Album", 372 + slug="reordered-album", 373 + atproto_record_uri=f"at://{unique_did}/fm.plyr.list/existing123", 374 + atproto_record_cid="bafyexisting123", 375 + ) 376 + db_session.add(album) 377 + await db_session.flush() 378 + 379 + # create tracks with staggered created_at (track3 oldest, track1 newest) 380 + base_time = datetime.now(UTC) 381 + track1 = Track( 382 + title="Track 1", 383 + file_id=f"preserve-order-1-{uuid.uuid4().hex[:8]}", 384 + file_type="audio/mpeg", 385 + artist_did=unique_did, 386 + album_id=album.id, 387 + atproto_record_uri=f"at://{unique_did}/fm.plyr.track/t1", 388 + atproto_record_cid="cid1", 389 + created_at=base_time, # newest 390 + ) 391 + track2 = Track( 392 + title="Track 2", 393 + file_id=f"preserve-order-2-{uuid.uuid4().hex[:8]}", 394 + file_type="audio/mpeg", 395 + artist_did=unique_did, 396 + album_id=album.id, 397 + atproto_record_uri=f"at://{unique_did}/fm.plyr.track/t2", 398 + atproto_record_cid="cid2", 399 + created_at=base_time - timedelta(hours=1), 400 + ) 401 + track3 = Track( 402 + title="Track 3", 403 + file_id=f"preserve-order-3-{uuid.uuid4().hex[:8]}", 404 + file_type="audio/mpeg", 405 + artist_did=unique_did, 406 + album_id=album.id, 407 + atproto_record_uri=f"at://{unique_did}/fm.plyr.track/t3", 408 + atproto_record_cid="cid3", 409 + created_at=base_time - timedelta(hours=2), # oldest 410 + ) 411 + db_session.add_all([track1, track2, track3]) 412 + await db_session.commit() 413 + 414 + album_id = album.id 415 + mock_session = MockSession(did=unique_did) 416 + 417 + # mock ATProto record fetch to return custom order: track2, track1, track3 418 + # (user reordered via frontend - different from created_at order) 419 + mock_existing_record = { 420 + "value": { 421 + "items": [ 422 + {"subject": {"uri": track2.atproto_record_uri, "cid": "cid2"}}, 423 + {"subject": {"uri": track1.atproto_record_uri, "cid": "cid1"}}, 424 + {"subject": {"uri": track3.atproto_record_uri, "cid": "cid3"}}, 425 + ] 426 + } 427 + } 428 + 429 + captured_track_refs: list[dict[str, str]] = [] 430 + 431 + async def capture_album_sync(*args: object, **kwargs: object) -> tuple[str, str]: 432 + track_refs: list[dict[str, str]] = kwargs.get("track_refs", []) # type: ignore[assignment] 433 + captured_track_refs.extend(track_refs) 434 + assert album.atproto_record_uri is not None 435 + return (album.atproto_record_uri, "bafynewcid") 436 + 437 + try: 438 + with ( 439 + patch( 440 + "backend._internal.atproto.sync.upsert_profile_record", 441 + new_callable=AsyncMock, 442 + return_value=None, 443 + ), 444 + patch( 445 + "backend._internal.atproto.sync.get_record_public", 446 + new_callable=AsyncMock, 447 + return_value=mock_existing_record, 448 + ), 449 + patch( 450 + "backend._internal.atproto.sync.upsert_album_list_record", 451 + new_callable=AsyncMock, 452 + side_effect=capture_album_sync, 453 + ), 454 + patch( 455 + "backend._internal.atproto.sync.upsert_liked_list_record", 456 + new_callable=AsyncMock, 457 + return_value=None, 458 + ), 459 + ): 460 + await sync_atproto_records(mock_session, unique_did) 461 + 462 + # verify track order was preserved (track2, track1, track3) 463 + # NOT created_at order (which would be track3, track2, track1) 464 + assert len(captured_track_refs) == 3 465 + assert captured_track_refs[0]["uri"] == track2.atproto_record_uri 466 + assert captured_track_refs[1]["uri"] == track1.atproto_record_uri 467 + assert captured_track_refs[2]["uri"] == track3.atproto_record_uri 468 + finally: 469 + # clean up in correct order (tracks before albums before artists) 470 + await db_session.execute(delete(Track).where(Track.album_id == album_id)) 471 + await db_session.execute(delete(Album).where(Album.id == album_id)) 472 + await db_session.execute(delete(Artist).where(Artist.did == unique_did)) 473 + await db_session.commit() 474 + 475 + 339 476 async def test_login_callback_triggers_background_sync( 340 477 test_app: FastAPI, 341 478 db_session: AsyncSession,