Skip to content

Commit fb67d8a

Browse files
authored
fix(sync): union same-named collections instead of overwriting in finalize (#1533)
Two enabled collections sharing a display name overwrote each other's member set in the name-keyed finalize accumulator, so the single by-name Steam collection reflected only the last-synced one. Reachable on a single-user server (RomM has no cross-table name constraint, so a user and a smart/franchise collection can share a name) and wider on shared servers (list endpoints return own + public). The accumulator is re-keyed to a collision-free (collection_kind, collection_id) identity carrying the name, and the reporter unions same-named collections' resolved appIds when building the by-name Steam map — the frontend contract is byte-for-byte unchanged. No owner filter (a QAM own/all toggle is tracked as #1532). Side benefit: the #742 stamp now records each collection's own membership. Closes #1503
1 parent a303ba7 commit fb67d8a

10 files changed

Lines changed: 244 additions & 67 deletions

File tree

docs/architecture/backend-architecture.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,20 @@ sibling row holding `shortcut_app_id` is the group's **active version**.
500500
to its group's bound sibling's appId — so collecting or favouriting any version puts the game's single shortcut in the
501501
collection.
502502

503+
**Same-named collections union into one Steam collection (#1503).** RomM enforces name uniqueness only per-table and
504+
per-`(name, user_id)`, so two enabled collections can share a display name — a user collection and a smart/franchise
505+
collection, or (multi-user) another account's public collection the list endpoints return. Steam's collection namespace
506+
is **by-name** (`RomM: [<name>] (host)`), so both must resolve to the one Steam collection. The finalize accumulator
507+
(`_state.py` `pending_collection_memberships`) is therefore keyed by a collision-free `(collection_kind, collection_id)`
508+
identity — the same identity the #742 completion stamp uses — with the display name carried in the
509+
`CollectionMembership` value; the real-sync and preview write paths share one key builder so they cannot drift. The
510+
reporter (`_resolve_collection_memberships`) then groups the accumulator's entries by name and **unions** their resolved
511+
appId sets (order-preserving, de-duplicated across collections; each collection's own resolution already dedups within),
512+
emitting the unchanged by-name `romm_collection_app_ids: {name → [appId]}` contract — a single-collection name unions a
513+
set of one and is byte-for-byte the pre-#1503 output. The plugin **unions rather than owner-filters** here: RomM
514+
deliberately exposes public collections across accounts, so syncing them unfiltered is the established behaviour (an
515+
own/all QAM toggle is a separate concern).
516+
503517
**Incremental skip — the per-platform completion stamp is the sole authority.** A platform unit skips only when its
504518
`PlatformSyncState` stamp exists
505519
([ADR-0023](https://github.com/danielcopper/decky-romm-sync/blob/main/docs/adr/0023-chunked-per-unit-apply.md)); the

docs/user-guide/syncing-your-library.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,11 @@ collections, or use the paired **Enable All** / **Disable All** buttons to bulk-
229229
**Show collection games in platform groups** toggle controls whether games pulled in via a collection also get added to
230230
their platform's Steam group.
231231

232+
If two enabled collections share the same name — for example a personal collection and a smart or franchise collection
233+
called the same thing, or (on a shared server) another account's public collection — they **merge into a single Steam
234+
collection** carrying the combined set of games. RomM allows collections to share a name, but Steam identifies a
235+
collection by its name, so the plugin unions their members rather than dropping one.
236+
232237
## Artwork
233238

234239
Each synced game gets up to five types of artwork:

py_modules/domain/sync_diff.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,16 +270,21 @@ def select_stale_removals(
270270

271271

272272
def compute_collection_diff(
273-
collection_memberships: dict[str, list[int]],
273+
current_collection_names: set[str],
274274
last_synced_collections: list[str],
275275
) -> dict[str, Any]:
276276
"""Diff enabled collections (by name) against the last-synced set.
277277
278-
Returns ``{"has_changes": bool, "added": [...], "removed": [...]}``.
279-
``has_changes`` is True if there are any added/removed collections, or
280-
if there are any current collections at all (covers first-sync case).
278+
``current_collection_names`` is the set of DISTINCT display names present in
279+
this run's collection accumulator — a name counts as present iff at least one
280+
collection carries it, so two same-named collections (RomM permits them across
281+
kinds/users, #1503) collapse to one entry, matching the by-name Steam
282+
collection they merge into. Returns
283+
``{"has_changes": bool, "added": [...], "removed": [...]}``; ``has_changes`` is
284+
True if there are any added/removed collections, or if there are any current
285+
collections at all (covers first-sync case).
281286
"""
282-
current = set(collection_memberships.keys())
287+
current = current_collection_names
283288
previous = set(last_synced_collections)
284289
added = sorted(current - previous)
285290
removed = sorted(previous - current)

py_modules/services/library/_state.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@ def _default_progress() -> dict[str, Any]:
4242
}
4343

4444

45+
@dataclass(frozen=True)
46+
class CollectionMembership:
47+
"""One enabled collection's full member rom_ids, tagged with its display name.
48+
49+
The value type of :attr:`LibrarySyncStateBox.pending_collection_memberships`,
50+
whose key is a collision-free ``(collection_kind, collection_id)`` identity so
51+
two collections that merely SHARE a display name never overwrite each other's
52+
members in the finalize accumulator. The name rides here in the value because
53+
Steam's collection namespace is by-name: the reporter groups same-named
54+
memberships and UNIONs their resolved appIds into the one
55+
``RomM: [<name>] (host)`` Steam collection (RomM permits same-named collections
56+
across kinds/users, so the plugin merges rather than owner-filters, #1503).
57+
"""
58+
59+
name: str
60+
rom_ids: list[int]
61+
62+
4563
@dataclass(frozen=True)
4664
class AbandonedChunk:
4765
"""A heartbeat-timed-out apply chunk, stashed for a late-ack commit.
@@ -98,7 +116,13 @@ class LibrarySyncStateBox:
98116
# abandon window so a late ack still stamps the confirmed values.
99117
pending_cover_sources: dict[int, str] = field(default_factory=dict)
100118
pending_delta: PreviewDelta | None = None
101-
pending_collection_memberships: dict[str, list[int]] = field(default_factory=dict)
119+
# Per-run collection-membership accumulator, keyed by a collision-free
120+
# ``(collection_kind, collection_id)`` identity (never the display name), so
121+
# two collections that share a name each keep their own members. The name
122+
# travels in the :class:`CollectionMembership` value; the reporter unions
123+
# same-named memberships when it builds the by-name Steam-collection map
124+
# (#1503).
125+
pending_collection_memberships: dict[tuple[str, str], CollectionMembership] = field(default_factory=dict)
102126
pending_platform_rom_ids: set[int] | None = None
103127
# Per-unit pipeline coordination. ``unit_complete_event`` is set by
104128
# :meth:`SyncReporter.report_unit_results` when the frontend acks the

py_modules/services/library/reporter.py

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
from domain.collection_sync_state import CollectionSyncState
3838
from domain.platform_sync_state import PlatformSyncState
3939
from domain.sync_run import SyncRun
40-
from services.library._state import LibrarySyncStateBox
40+
from services.library._state import CollectionMembership, LibrarySyncStateBox
4141
from services.protocols import (
4242
ArtworkManager,
4343
Clock,
@@ -117,7 +117,7 @@ def _build_collection_app_ids(
117117
self,
118118
uow: UnitOfWork,
119119
pending_platform_rom_ids: set[int] | None,
120-
pending_collection_memberships: dict[str, list[int]],
120+
pending_collection_memberships: dict[tuple[str, str], CollectionMembership],
121121
platform_names: dict[str, str],
122122
) -> tuple[dict[str, list[int]], dict[str, list[int]]]:
123123
"""Build platform_app_ids and romm_collection_app_ids from ``uow.roms``.
@@ -128,14 +128,17 @@ def _build_collection_app_ids(
128128
resolved from *platform_names* (the work-queue), falling back to
129129
the slug when absent.
130130
131-
RomM collections keep the per-run membership accumulator and resolve
132-
each member ``rom_id`` to a Steam appId with a **sibling-group
133-
fallback** (ADR-0021): a bound member uses its own binding; an unbound
134-
member maps to its group's bound sibling's appId, so favouriting /
135-
collecting ANY version of a game puts the game's single shortcut into
136-
the Steam collection. Per-collection appIds are de-duplicated — several
137-
siblings of one group collapse onto the one shortcut. The platform loop
138-
still excludes rows whose ``shortcut_app_id`` is ``None``.
131+
RomM collections keep the per-run membership accumulator (keyed by a
132+
collision-free ``(collection_kind, collection_id)`` identity, name in the
133+
value) and resolve each member ``rom_id`` to a Steam appId with a
134+
**sibling-group fallback** (ADR-0021): a bound member uses its own
135+
binding; an unbound member maps to its group's bound sibling's appId, so
136+
favouriting / collecting ANY version of a game puts the game's single
137+
shortcut into the Steam collection. Per-collection appIds are
138+
de-duplicated — several siblings of one group collapse onto the one
139+
shortcut — and same-named collections UNION into the one by-name Steam
140+
collection (#1503). The platform loop still excludes rows whose
141+
``shortcut_app_id`` is ``None``.
139142
"""
140143
platform_app_ids, group_bound_app_id = self._scan_bound_rows(uow, pending_platform_rom_ids, platform_names)
141144
romm_collection_app_ids = self._resolve_collection_memberships(
@@ -179,27 +182,33 @@ def _note_group_binding(group_bound: dict[str, tuple[int, int]], rom: Rom) -> No
179182
def _resolve_collection_memberships(
180183
self,
181184
uow: UnitOfWork,
182-
pending_collection_memberships: dict[str, list[int]],
185+
pending_collection_memberships: dict[tuple[str, str], CollectionMembership],
183186
group_bound_app_id: dict[str, int],
184187
) -> dict[str, list[int]]:
185-
"""Resolve each collection's member rom_ids to de-duplicated appIds.
186-
187-
A bound member uses its own binding; an unbound member falls back to
188-
its sibling group's bound appId (ADR-0021). Collections that resolve
189-
to no appId are omitted.
188+
"""Resolve each collection's member rom_ids to de-duplicated appIds, UNION by name.
189+
190+
A bound member uses its own binding; an unbound member falls back to its
191+
sibling group's bound appId (ADR-0021). Steam's collection namespace is
192+
by-name, so same-named RomM collections (permitted across kinds/users,
193+
#1503) merge into one entry: their resolved appIds are UNIONed,
194+
order-preserving and de-duplicated ACROSS collections (each collection's
195+
own resolution already dedups within). The accumulator is keyed by a
196+
collision-free identity, so a same-named pair never overwrote either
197+
member set upstream. Names that resolve to no appId are omitted; the
198+
common single-collection case unions a set of one and is byte-for-byte
199+
identical to the pre-#1503 output.
190200
"""
191201
romm_collection_app_ids: dict[str, list[int]] = {}
192-
for coll_name, rom_ids in pending_collection_memberships.items():
193-
seen: set[int] = set()
194-
app_ids: list[int] = []
195-
for rid in rom_ids:
202+
seen_by_name: dict[str, set[int]] = {}
203+
for membership in pending_collection_memberships.values():
204+
app_ids = romm_collection_app_ids.setdefault(membership.name, [])
205+
seen = seen_by_name.setdefault(membership.name, set())
206+
for rid in membership.rom_ids:
196207
app_id = self._member_app_id(uow, rid, group_bound_app_id)
197208
if app_id is not None and app_id not in seen:
198209
seen.add(app_id)
199210
app_ids.append(app_id)
200-
if app_ids:
201-
romm_collection_app_ids[coll_name] = app_ids
202-
return romm_collection_app_ids
211+
return {name: app_ids for name, app_ids in romm_collection_app_ids.items() if app_ids}
203212

204213
@staticmethod
205214
def _member_app_id(uow: UnitOfWork, rid: int, group_bound_app_id: dict[str, int]) -> int | None:
@@ -217,7 +226,7 @@ def _member_app_id(uow: UnitOfWork, rid: int, group_bound_app_id: dict[str, int]
217226

218227
def _finalize_per_unit_run_io(
219228
self,
220-
pending_collection_memberships: dict[str, list[int]],
229+
pending_collection_memberships: dict[tuple[str, str], CollectionMembership],
221230
pending_platform_rom_ids: set[int] | None,
222231
platform_names: dict[str, str],
223232
stale_rom_ids: list[int] | None = None,
@@ -263,7 +272,7 @@ def _finalize_per_unit_run_io(
263272

264273
async def finalize_per_unit_run(
265274
self,
266-
pending_collection_memberships: dict[str, list[int]],
275+
pending_collection_memberships: dict[tuple[str, str], CollectionMembership],
267276
pending_platform_rom_ids: set[int] | None,
268277
platform_names: dict[str, str] | None = None,
269278
stale_rom_ids: list[int] | None = None,

py_modules/services/library/service.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from typing import TYPE_CHECKING, Any
1919

2020
from lib.late_binding import LateBinding
21-
from services.library._state import LibrarySyncStateBox
21+
from services.library._state import CollectionMembership, LibrarySyncStateBox
2222
from services.library.fetcher import LibraryFetcher, LibraryFetcherConfig
2323
from services.library.reporter import SyncReporter, SyncReporterConfig
2424
from services.library.sync_orchestrator import SyncOrchestrator, SyncOrchestratorConfig
@@ -228,11 +228,11 @@ def _pending_delta(self, value: PreviewDelta | None) -> None:
228228
self._box.pending_delta = value
229229

230230
@property
231-
def _pending_collection_memberships(self) -> dict[str, list[int]]:
231+
def _pending_collection_memberships(self) -> dict[tuple[str, str], CollectionMembership]:
232232
return self._box.pending_collection_memberships
233233

234234
@_pending_collection_memberships.setter
235-
def _pending_collection_memberships(self, value: dict[str, list[int]]) -> None:
235+
def _pending_collection_memberships(self, value: dict[tuple[str, str], CollectionMembership]) -> None:
236236
self._box.pending_collection_memberships = value
237237

238238
@property

0 commit comments

Comments
 (0)