fix(collections): refresh smart collections per ROM, not per request

Recomputing every smart collection after a delete scanned the library
once per collection, synchronously on the event loop and inside a single
write transaction. Ask instead which of the changed ids each collection
matches: that resolves as a primary-key lookup, so only the collections
actually holding one of them pay for a recount.

Follow the same refresh into `update_rom` and the metadata reset. Both
change exactly the fields the saved filters match on, so cached counts
and cover mosaics no longer wait for the next scan to catch up.

Guard every call site. The ROM write is already committed by the time
the refresh runs, so a failure there must not be reported back as a
failed edit, and must not cost a bulk delete its per-ROM report.

Drop the handler-level test for leaving a collection untouched on read;
the endpoint test covers that invariant through the route the bug was
reported against.

Follow-up review of f4ac78c (#4029). Implemented with Claude Code
(Opus 5), then reviewed and verified by me.

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

Author: Spinnich

backend/endpoints/roms/__init__.py

@@ -119,6 +119,18 @@ def safe_int_or_none(value: Any) -> int | None:
     return safe_int(value)
 
 
+def refresh_affected_smart_collections(rom_ids: Sequence[int]) -> None:
+    """Follow a library change into the cached smart collection membership.
+
+    The ROM write has already been committed, so a stale count is the worst
+    this can cost, and reporting it back as a failed write would be a lie.
+    """
+    try:
+        db_collection_handler.refresh_smart_collections_for_roms(rom_ids)
+    except Exception as e:
+        log.error(f"Couldn't refresh smart collections for {rom_ids}: {e}")
+
+
 def build_unscoped_sidecar_cache_key(
     user_id: int,
     order_by: str,
@@ -1536,6 +1548,7 @@ async def update_rom(
             raise RomNotFoundInDatabaseException(id)
 
         db_rom_handler.invalidate_filter_values_cache()
+        refresh_affected_smart_collections([id])
         return DetailedRomSchema.from_orm_with_request(rom, request)
 
     provided_fields = form_data.model_fields_set
@@ -1915,6 +1928,7 @@ async def update_rom(
         fire_and_forget(meta_playmatch_handler.submit_manual_match_suggestion(rom))
 
     db_rom_handler.invalidate_filter_values_cache()
+    refresh_affected_smart_collections([id])
     return DetailedRomSchema.from_orm_with_request(rom, request)
 
 
@@ -1981,7 +1995,7 @@ async def delete_roms(
     perms = get_permissions(request)
     assert_can(perms, PermEntity.ROMS, PermAction.DELETE)
 
-    successful_items = 0
+    deleted_ids: list[int] = []
     failed_ids = []
     errors = []
 
@@ -2036,19 +2050,19 @@ async def delete_roms(
                     f"Couldn't find resources to delete for {hl(str(rom.name or 'ROM'), color=BLUE)}"
                 )
 
-            successful_items += 1
+            deleted_ids.append(id)
         except Exception as e:
             failed_ids.append(id)
             errors.append(f"Failed to delete ROM {id}: {str(e)}")
 
-    if successful_items:
+    if deleted_ids:
         db_rom_handler.invalidate_filter_values_cache()
         # Deleted ROMs would otherwise linger in the cached smart collection
         # membership until the next scan.
-        db_collection_handler.refresh_smart_collections()
+        refresh_affected_smart_collections(deleted_ids)
 
     return {
-        "successful_items": successful_items,
+        "successful_items": len(deleted_ids),
         "failed_ids": failed_ids,
         "errors": errors,
     }

backend/endpoints/sockets/scan.py

@@ -975,8 +975,12 @@ async def stop_scan():
         db_rom_handler.invalidate_filter_values_cache()
 
         # Smart collection membership is derived from the library, and is no
-        # longer recomputed while serving a gallery page.
-        db_collection_handler.refresh_smart_collections()
+        # longer recomputed while serving a gallery page. The scan itself is
+        # done, so a failure here must not report it as one.
+        try:
+            db_collection_handler.refresh_smart_collections()
+        except Exception as e:
+            log.error(f"Couldn't refresh smart collections after the scan: {e}")
 
         # Export metadata files if enabled in config
         config = cm.get_config()

backend/handler/database/collections_handler.py

@@ -632,3 +632,37 @@ def refresh_smart_collections(
             self.refresh_smart_collection(id, session=session)
 
         return len(ids)
+
+    @begin_session
+    def refresh_smart_collections_for_roms(
+        self,
+        rom_ids: Sequence[int],
+        session: Session = None,  # type: ignore
+    ) -> int:
+        """Refresh the collections a handful of edited or deleted ROMs touch.
+
+        Editing one ROM rarely moves any collection, and asking whether given
+        ids match is an indexed lookup, so only the collections that actually
+        hold one of them pay for a recount. Membership is then recomputed whole,
+        since a ROM that stayed a member can still have changed the stored order
+        or the cover mosaic.
+        """
+        from handler.database import db_rom_handler
+
+        if not rom_ids:
+            return 0
+
+        candidates = set(rom_ids)
+        refreshed = 0
+        for smart_collection in session.scalars(select(SmartCollection)).all():
+            matching = db_rom_handler.get_smart_collection_matches(
+                smart_collection=smart_collection,
+                rom_ids=candidates,
+                user_id=smart_collection.user_id,
+                session=session,
+            )
+            if matching or candidates & set(smart_collection.rom_ids):
+                self.refresh_smart_collection(smart_collection.id, session=session)
+                refreshed += 1
+
+        return refreshed

backend/handler/database/roms_handler.py

@@ -618,6 +618,32 @@ def build_smart_collection_query(
             **db_collection_handler.get_smart_collection_criteria(smart_collection),
         )
 
+    @begin_session
+    def get_smart_collection_matches(
+        self,
+        *,
+        smart_collection: SmartCollection,
+        rom_ids: Iterable[int],
+        user_id: int | None,
+        session: Session = None,  # type: ignore
+    ) -> set[int]:
+        """Which of `rom_ids` currently match the collection's criteria.
+
+        Restricting the criteria to a few ids keeps this an indexed lookup, so a
+        caller can ask whether one ROM moved without scanning the library.
+        """
+        query = self._join_rom_user(select(Rom.id), user_id).filter(Rom.id.in_(rom_ids))
+        return set(
+            session.scalars(
+                self.build_smart_collection_query(
+                    query=query,
+                    smart_collection=smart_collection,
+                    user_id=user_id,
+                    session=session,
+                )
+            )
+        )
+
     def _build_fulltext_boolean_query(self, term: str) -> str | None:
         words = FULLTEXT_BOOLEAN_OPERATORS_REGEX.sub(" ", term).split()
         if not words or any(len(word) < FULLTEXT_MIN_TOKEN_SIZE for word in words):

backend/tests/endpoints/roms/test_rom.py

@@ -640,6 +640,40 @@ def test_update_rom_adds_region_tag_on_rename(
     assert body["regions"] == ["Europe"]
 
 
+@patch.object(FSRomsHandler, "rename_fs_rom")
+@patch.object(IGDBHandler, "get_rom_by_id", return_value=IGDBRom(igdb_id=None))
+def test_update_rom_refreshes_smart_collection_membership(
+    rename_fs_rom_mock: AsyncMock,
+    get_rom_by_id_mock: AsyncMock,
+    client: TestClient,
+    access_token: str,
+    admin_user: User,
+    rom: Rom,
+):
+    # An edit changes what the saved filters match, so the cached counts have
+    # to follow it rather than wait for the next scan.
+    smart_collection = db_collection_handler.add_smart_collection(
+        SmartCollection(
+            name="European games",
+            description="",
+            user_id=admin_user.id,
+            filter_criteria={"regions": ["Europe"]},
+        )
+    )
+    db_collection_handler.refresh_smart_collection(smart_collection.id)
+
+    response = client.put(
+        f"/api/roms/{rom.id}",
+        headers={"Authorization": f"Bearer {access_token}"},
+        data={"fs_name": "test_rom (Europe).zip"},
+    )
+    assert response.status_code == status.HTTP_200_OK
+
+    refreshed = db_collection_handler.get_smart_collection(smart_collection.id)
+    assert refreshed is not None
+    assert refreshed.rom_ids == [rom.id]
+
+
 # Minimal valid PNG (1x1 transparent pixel)
 _PNG_BYTES = (
     b"\x89PNG\r\n\x1a\n"
@@ -700,6 +734,28 @@ def test_delete_roms(client: TestClient, access_token: str, rom: Rom):
     assert body["successful_items"] == 1
 
 
+def test_delete_roms_reports_results_when_the_refresh_fails(
+    client: TestClient, access_token: str, rom: Rom, mocker
+):
+    # The deletes are already committed by this point, so a failure updating
+    # cached smart collection membership must not cost the caller its report.
+    mocker.patch.object(
+        db_collection_handler,
+        "refresh_smart_collections_for_roms",
+        side_effect=RuntimeError("refresh exploded"),
+    )
+
+    response = client.post(
+        "/api/roms/delete",
+        headers={"Authorization": f"Bearer {access_token}"},
+        json={"roms": [rom.id], "delete_from_fs": []},
+    )
+
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json()["successful_items"] == 1
+    assert db_rom_handler.get_rom(rom.id) is None
+
+
 def test_delete_roms_reports_failed_ids(
     client: TestClient, access_token: str, rom: Rom
 ):

backend/tests/endpoints/test_collection.py

@@ -700,6 +700,9 @@ def test_update_recomputes_membership(
     def test_reading_a_smart_collection_does_not_touch_it(
         self, client, access_token: str, admin_user: User, rom: Rom
     ):
+        # Writing membership back while serving a page bumps `updated_at`, which
+        # is the `?ts=` cover cache-buster, so every visit re-downloaded
+        # thumbnails and every `updated_after` sync client saw a change.
         smart_collection = db_collection_handler.add_smart_collection(
             SmartCollection(
                 name="All roms",
@@ -724,3 +727,5 @@ def test_reading_a_smart_collection_does_not_touch_it(
         after = db_collection_handler.get_smart_collection(smart_collection.id)
         assert after is not None
         assert after.updated_at == before.updated_at
+        assert list(after.rom_ids) == list(before.rom_ids)
+        assert after.path_covers_small == before.path_covers_small

backend/tests/handler/database/test_smart_collections.py

@@ -88,27 +88,6 @@ def test_filter_roms_by_smart_collection_applies_criteria(
     assert {rom.id for rom in roms} == {rom.id for rom in matching}
 
 
-def test_filter_roms_by_smart_collection_leaves_the_collection_untouched(
-    platform: Platform, admin_user: User
-):
-    # Writing membership back while serving a page bumps `updated_at`, which is
-    # the `?ts=` cover cache-buster, so every visit re-downloads thumbnails.
-    _add_rom(platform, "Rally One", manual_metadata={"genres": ["Racing"]})
-    smart_collection = _add_smart_collection(admin_user, {"genres": ["Racing"]})
-    before = db_collection_handler.get_smart_collection(smart_collection.id)
-    assert before is not None
-    updated_at, rom_ids = before.updated_at, list(before.rom_ids)
-
-    db_rom_handler.get_roms_scalar(
-        smart_collection_id=smart_collection.id, user_id=admin_user.id
-    )
-
-    after = db_collection_handler.get_smart_collection(smart_collection.id)
-    assert after is not None
-    assert after.updated_at == updated_at
-    assert list(after.rom_ids) == rom_ids
-
-
 def test_filter_roms_by_smart_collection_narrows_with_the_gallery_filters(
     platform: Platform, admin_user: User
 ):
@@ -295,6 +274,42 @@ def test_refresh_smart_collections_covers_every_collection(
     assert refreshed_puzzle is not None and refreshed_puzzle.rom_count == 2
 
 
+def test_refresh_for_roms_updates_the_collections_the_rom_moved_between(
+    platform: Platform, admin_user: User
+):
+    racing = _add_smart_collection(admin_user, {"genres": ["Racing"]}, name="Racing")
+    puzzle = _add_smart_collection(admin_user, {"genres": ["Puzzle"]}, name="Puzzle")
+    _add_rom(platform, "Rally One", manual_metadata={"genres": ["Racing"]})
+    mover = _add_rom(platform, "Puzzler", manual_metadata={"genres": ["Puzzle"]})
+    db_collection_handler.refresh_smart_collections()
+
+    db_rom_handler.update_rom(mover.id, {"manual_metadata": {"genres": ["Racing"]}})
+    refreshed = db_collection_handler.refresh_smart_collections_for_roms([mover.id])
+
+    assert refreshed == 2
+    racing_after = db_collection_handler.get_smart_collection(racing.id)
+    puzzle_after = db_collection_handler.get_smart_collection(puzzle.id)
+    assert racing_after is not None and racing_after.rom_count == 2
+    assert puzzle_after is not None and puzzle_after.rom_count == 0
+
+
+def test_refresh_for_roms_leaves_untouched_collections_alone(
+    platform: Platform, admin_user: User, mocker
+):
+    # Recounting every collection for one edited ROM would scan the library
+    # once per collection, which is the cost this issue is about.
+    _add_smart_collection(admin_user, {"genres": ["Racing"]}, name="Racing")
+    _add_rom(platform, "Rally One", manual_metadata={"genres": ["Racing"]})
+    db_collection_handler.refresh_smart_collections()
+    bystander = _add_rom(platform, "Puzzler", manual_metadata={"genres": ["Puzzle"]})
+
+    refresh = mocker.spy(db_collection_handler, "refresh_smart_collection")
+    moved = db_collection_handler.refresh_smart_collections_for_roms([bystander.id])
+
+    assert moved == 0
+    assert refresh.call_count == 0
+
+
 def test_cached_membership_belongs_to_the_owner(
     platform: Platform, admin_user: User, editor_user: User
 ):