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