Files
lanspread/CLEAN_CODE.md
T
ddidderr 41e9a0efc1 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
2026-05-18 21:25:20 +02:00

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 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.