refactor(peer): split local library and operation UI events

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
This commit is contained in:
2026-05-18 21:25:20 +02:00
parent be00a7a298
commit 41e9a0efc1
14 changed files with 657 additions and 255 deletions
+76
View File
@@ -0,0 +1,76 @@
# 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:
1. **Context** — what commit/PR introduced the pattern, one-paragraph summary.
2. **Root issue** — the underlying invariant the code is working around.
3. **Why the obvious fix doesn't work** — what constraints forced the current
shape.
4. **The tell** — concrete code shapes (scattered invalidations, repeated
special cases, dedup keys that re-derive existing facts) that signal the
smell.
5. **Clean shape** — what the code would look like without the constraint.
6. **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.