fix(scanner): move updated_at when the upsert takes its conflict branch

backend/app/services/scanner.py

@@ -10,7 +10,7 @@
 from pathlib import Path
 from typing import Any
 
-from sqlalchemy import delete, select
+from sqlalchemy import delete, func, select
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.config import AUDIO_EXTENSIONS, settings
@@ -719,6 +719,17 @@ async def _create_track(
         upsert_stmt = insert_stmt.on_conflict_do_update(
             index_elements=["file_path"],
             set_={
+                # `Track.updated_at` carries `onupdate=func.now()`, and SQLAlchemy applies that to
+                # ORM flushes and Core `update()` — but *not* to an `on_conflict_do_update` `set_`
+                # clause, which is emitted verbatim. So this branch rewrote 22 columns and left the
+                # timestamp reading whenever the row was last touched by some other path.
+                #
+                # Narrow, but real: this branch fires only when two scans race on the same
+                # `file_path`, since the ordinary rescan goes through `_update_track` and is a plain
+                # ORM update. It matters because the timestamp is the cursor an incremental client
+                # would page from — a row whose tags changed but whose `updated_at` did not is
+                # invisible to a delta, and invisible in the silent direction.
+                "updated_at": func.now(),
                 "file_hash": insert_stmt.excluded.file_hash,
                 "file_size": insert_stmt.excluded.file_size,
                 "file_modified_at": insert_stmt.excluded.file_modified_at,
@@ -763,7 +774,7 @@ async def detect_compilation_albums(self) -> dict[str, int]:
         Uses case-insensitive album matching to handle variations like
         "Alice In Ultraland" vs "Alice in Ultraland".
         """
-        from sqlalchemy import func, update
+        from sqlalchemy import update
 
         # Find albums with multiple artists (compilation candidates)
         # Only consider tracks where album_artist is not already set

backend/tests/test_scanner.py

@@ -26,8 +26,10 @@
       epic_boss_battle.mp3
 """
 
+import asyncio
 import shutil
 import tempfile
+from datetime import timedelta
 from pathlib import Path
 
 import pytest
@@ -696,3 +698,120 @@ async def test_stale_lock_detection_via_heartbeat(self, clean_db):
         finally:
             r.delete("familiar:sync:lock")
             r.delete("familiar:sync:progress")
+
+
+@pytest.mark.asyncio(loop_scope="function")
+class TestUpdatedAtCursor:
+    """`tracks.updated_at` as a cursor an incremental client could page from.
+
+    Nothing consumes it yet — `TrackResponse` does not expose it and `list_tracks` takes no `since`
+    parameter — but ADR-0011 proposes both, and a cursor that fails to move is wrong in the silent
+    direction: the row is simply absent from the delta, and the client cannot tell.
+
+    The rescan path is a plain ORM update (`_update_track`), so SQLAlchemy applies the column's
+    `onupdate=func.now()`. That is the behaviour these tests pin, because it is load-bearing for a
+    design that does not exist yet and would otherwise be easy to break by "optimising" the rescan
+    into a bulk statement.
+    """
+
+    async def _read_fresh(self):
+        """Read the row on its own engine, so no session snapshot or identity map can answer."""
+        from sqlalchemy import text
+        from sqlalchemy.ext.asyncio import create_async_engine
+
+        from app.config import settings
+
+        engine = create_async_engine(settings.database_url, echo=False)
+        async with engine.connect() as conn:
+            row = (
+                await conn.execute(text("SELECT title, updated_at FROM tracks LIMIT 1"))
+            ).one()
+        await engine.dispose()
+        return row[0], row[1]
+
+    async def test_retagging_a_file_moves_updated_at(self, clean_db):
+        """A tag change is exactly the edit a delta must not miss."""
+        from mutagen.easyid3 import EasyID3
+
+        with tempfile.TemporaryDirectory() as tmpdir:
+            tmp_path = Path(tmpdir)
+            target = tmp_path / "track.mp3"
+            shutil.copy(FIXTURES_DIR / "electronic_short.mp3", target)
+
+            await LibraryScanner(clean_db).scan(tmp_path)
+            await clean_db.commit()
+            _, before = await self._read_fresh()
+
+            # Wide enough that the second timestamp cannot be confused for the first, and wide
+            # enough to survive a slow CI runner.
+            await asyncio.sleep(2.0)
+
+            tags = EasyID3(target)
+            tags["title"] = "Retagged By The Test"
+            tags.save()
+
+            clean_db.expire_all()
+            await LibraryScanner(clean_db).scan(tmp_path)
+            await clean_db.commit()
+            title, after = await self._read_fresh()
+
+            assert title == "Retagged By The Test", "the rescan did not pick up the new tag"
+            assert after > before + timedelta(seconds=1.5), (
+                f"updated_at did not move with the retag ({before} -> {after}); "
+                "a delta cursor built on it would silently skip this row"
+            )
+
+    async def test_the_upsert_conflict_branch_moves_updated_at(self, clean_db):
+        """The race path: two scans reaching the same `file_path` at once.
+
+        `_create_track`'s `on_conflict_do_update` is emitted verbatim, so unlike the ORM rescan it
+        gets no `onupdate` applied for free — the timestamp has to be in the `set_` clause. Reached
+        by calling `_create_track` twice, which is what the race produces.
+        """
+        from app.utils.time import utcnow
+
+        with tempfile.TemporaryDirectory() as tmpdir:
+            tmp_path = Path(tmpdir)
+            target = tmp_path / "track.mp3"
+            shutil.copy(FIXTURES_DIR / "electronic_short.mp3", target)
+
+            scanner = LibraryScanner(clean_db)
+            await scanner._create_track(target, "hash-one", utcnow(), 1234)
+            await clean_db.commit()
+            _, before = await self._read_fresh()
+
+            await asyncio.sleep(2.0)
+
+            # Same path, second writer: takes the conflict branch rather than inserting.
+            clean_db.expire_all()
+            await scanner._create_track(target, "hash-two", utcnow(), 5678)
+            await clean_db.commit()
+            _, after = await self._read_fresh()
+
+            assert after > before + timedelta(seconds=1.5), (
+                f"the conflict branch left updated_at at {before} (now {after}); "
+                "`set_` is emitted verbatim, so the timestamp must be listed in it"
+            )
+
+    async def test_an_unchanged_rescan_leaves_updated_at_alone(self, clean_db):
+        """The other half: a cursor that moves for every scan carries the whole library every time."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            tmp_path = Path(tmpdir)
+            shutil.copy(FIXTURES_DIR / "electronic_short.mp3", tmp_path / "track.mp3")
+
+            await LibraryScanner(clean_db).scan(tmp_path)
+            await clean_db.commit()
+            _, before = await self._read_fresh()
+
+            await asyncio.sleep(1.0)
+
+            clean_db.expire_all()
+            results = await LibraryScanner(clean_db).scan(tmp_path)
+            await clean_db.commit()
+            _, after = await self._read_fresh()
+
+            assert results["unchanged"] == 1
+            assert after == before, (
+                f"an unchanged rescan moved updated_at ({before} -> {after}); "
+                "every delta would then return the entire library"
+            )