Clean code notes

Running notes on architectural smells in the codebase: things that work today but are shaped wrong, and what a cleaner design would look like. Each entry should explain the smell, why the current shape exists, and the warning sign that says "now is the time to refactor."

Resolved: local library and operation UI signals are split

Context. Commit a9f9845 ("fix(peer): suppress duplicate local game updates") added a last_local_update_key cache in Ctx, keyed on (revision, digest, active_operations). That worked, but it made scan emission responsible for deduplicating operation-state changes that were actually owned by command handlers.

The root issue

update_and_announce_games already knows whether the library actually changed: LocalLibraryState::update_from_scan returns Option<LibraryDelta>, where None means "nothing changed." That fact gates peer LibraryDelta announcements. Active operation status has a different source of truth: Ctx::active_operations, mutated by operation start, handoff, end, liveness cancellation, and guard cleanup.

The old LocalGamesUpdated { games, active_operations } event mixed those two sources of truth. The dedup key was a symptom of that overload.

Current shape

The peer runtime now emits two separate facts:

LocalLibraryChanged { games } is emitted from scans when the local library state changes. A real SetGameDir path change may force one local snapshot for the UI even when the library digest matches the previous path, because Tauri has already cleared local flags for the old path.
ActiveOperationsChanged { active_operations } is emitted when the operation table changes. Normal mutations go through begin_operation, transition_download_to_install, and end_operation; liveness cancellation and OperationGuard cleanup publish the same snapshot when they clear state.

Tauri is the join boundary. It stores the latest game DB and latest active operation snapshot, then keeps emitting the existing frontend games-list-updated payload. The frontend did not need to learn the peer runtime's internal event split.

Invariants to protect

Do not reintroduce a scan-level dedup cache for operation state. If a new path mutates Ctx::active_operations, route it through the operation publisher or explicitly document why it is not UI-visible. If a new local scan reason needs a UI snapshot without a peer delta, model that as an explicit scan policy like the path-change forced snapshot, not as cache invalidation.

How to add to this file

When a code review uncovers a "this works but it's bolted on" pattern, write it up here. Structure:

Context — what commit/PR introduced the pattern, one-paragraph summary.
Root issue — the underlying invariant the code is working around.
Why the obvious fix doesn't work — what constraints forced the current shape.
The tell — concrete code shapes (scattered invalidations, repeated special cases, dedup keys that re-derive existing facts) that signal the smell.
Clean shape — what the code would look like without the constraint.
Warning signs — what observations in future work mean "do the refactor now."

Keep entries narrative, not bulleted to death. The point is to preserve the reasoning so future contributors can decide whether the trade-off still holds.

3.3 KiB Raw Blame History