Replace the `a9f9845` local-update dedup cache with explicit peer event semantics. Local scans now emit `LocalLibraryChanged` when the library changes, while operation mutations emit `ActiveOperationsChanged` from the mutation path. Tauri keeps joining those facts into the existing `games-list-updated` payload, so the frontend contract stays stable. This removes the cache/invalidation coupling between scan emission and operation state. The remaining forced local snapshot is explicit: accepted game directory changes can refresh the UI for an equivalent new path without sending a peer library delta. Operation guard cleanup and liveness cancellation now publish the same active operation snapshot as normal command-handler transitions. The peer CLI JSONL events follow the same split with `local-library-changed` and `active-operations-changed`. Test Plan: - `just fmt` - `CARGO_BUILD_RUSTC_WRAPPER= just test` - `CARGO_BUILD_RUSTC_WRAPPER= just clippy` - `git diff --check` Refs: CLEAN_CODE_PLAN_1.md
3.3 KiB
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 realSetGameDirpath 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 throughbegin_operation,transition_download_to_install, andend_operation; liveness cancellation andOperationGuardcleanup 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.