-```
-
-## Verification
-After making changes:
-1. `make lint` — Go linting passes
-2. `make test` — Go tests pass
-3. `cd frontend && pnpm exec tsc --noEmit` — TypeScript type checking passes
diff --git a/.opencode/plans/refactoring-catalog.md b/.opencode/plans/refactoring-catalog.md
new file mode 100644
index 0000000..e82dcff
--- /dev/null
+++ b/.opencode/plans/refactoring-catalog.md
@@ -0,0 +1,209 @@
+# Refactoring Catalog
+
+Prioritized list of architectural improvements identified during a full codebase audit (Feb 2026). Items are grouped by priority — tackle P1 before adding major new features, P2 as convenient, P3 opportunistically.
+
+---
+
+## P1 — Should fix before adding major features
+
+### 1. Resolve `RequestPlay` dual-handler ambiguity
+
+**Problem:** Both `player.registerEventHandlers()` (`player.go`) and `queue.registerEventHandlers()` (`queue.go:182`) listen for the `RequestPlay` event. The player calls `Play()` (resume audio), while the queue calls `PlayFromStart()` (play from beginning if `currentIndex == -1`). Both fire on every `RequestPlay` event since Wails delivers to all listeners.
+
+**Why it matters:** This works by coincidence — `PlayFromStart` is a no-op when `currentIndex != -1`, so the two handlers don't conflict in the common case. But it's fragile and semantically confusing. A single event triggering two different actions in two packages is an anti-pattern that will cause bugs as the codebase grows.
+
+**Approach:** Remove the `RequestPlay` handler from the player. The queue should be the sole handler — it already calls `player.Play()` internally when needed. If the queue needs to distinguish "resume" from "play from start", add a separate event or an argument to the existing one.
+
+---
+
+### 2. Remove player from Wails `FEBindings` (or remove event handlers)
+
+**Problem:** The player is added to `FEBindings` in `app.go:163`, which generates JS bindings for all exported methods (`Play()`, `Pause()`, `LoadFile()`, `Seek()`, `SetVolume()`, etc.). However, the frontend exclusively uses events for player control. This creates two parallel APIs for the same operations.
+
+**Why it matters:** It exposes internal lifecycle methods (`SetContext()`, `SaveState()`, `RestoreState()`) to the frontend as callable JS functions. New developers won't know which API to use. Any method added to the player automatically becomes a frontend-callable binding.
+
+**Approach:** Remove the player from `FEBindings`. The frontend uses events exclusively and the player doesn't need direct bindings. If `GetCurrentTrackInfo()` is needed as a binding for some edge case, extract it to a separate small struct that only exposes that method.
+
+---
+
+### 3. Split `queue.go` (2254 lines)
+
+**Problem:** The queue package is a single 2254-line file containing types, state management, ~300 lines of event handler boilerplate, persistence logic, shuffle algorithms, and emit helpers.
+
+**Why it matters:** Hard to navigate, hard to review changes, easy to introduce bugs in unrelated sections.
+
+**Approach:** Split into focused files:
+- `queue.go` — Core types (`Track`, `State`, `Queue` struct), constructor, `SetContext`, `SetPlayer`
+- `handlers.go` — `registerEventHandlers()` and all `handle*` methods
+- `persistence.go` — `persistTracks`, `persistState`, `RestoreState`, `SaveState`, `lookupTrackMetaBatch`
+- `shuffle.go` — Shuffle order generation and navigation
+- `emit.go` — All `emit*` methods
+
+---
+
+### 4. Split `cover-grid.ts` (3740 lines)
+
+**Problem:** The largest frontend component by far. It likely handles album grid rendering, context menus, drag-and-drop, selection, sorting, resizing, and more — all in a single file.
+
+**Why it matters:** Difficult to understand, modify, or review. Changes to context menu logic risk breaking grid rendering and vice versa.
+
+**Approach:** Extract logical sections into separate files/components:
+- Context menu logic into a shared utility or sub-component
+- Selection logic already uses a `SelectionController` — verify it's fully extracted
+- Drag-and-drop setup into the existing `DragController` if not already
+- Grid rendering as the core component, delegating to these helpers
+
+---
+
+## P2 — Fix when convenient
+
+### 5. Delete `backend/models/` package (dead code)
+
+**Problem:** The `models` package (`files.go`, `music.go`, `art.go`) defines `AudioFile`, `AudioFileType`, `Album`, `Track`, `Artist`, and `Art` types. No package imports it anywhere.
+
+**Why it matters:** Dead code creates confusion — new contributors may think these are the canonical domain types, but the actual types are in `library/`, `queue/`, `playlist/`, and `sqlcgen/`.
+
+**Approach:** Delete the entire `backend/models/` directory.
+
+---
+
+### 6. Extract `SizedFilename` to a shared utility package
+
+**Problem:** `library.SizedFilename()` is a small string utility for generating thumbnail filenames. Both `player/player.go` and `playlist/playlist.go` import the entire `library` package solely for this function.
+
+**Why it matters:** Creates unnecessary coupling — `player` -> `library` and `playlist` -> `library` dependencies exist only for one utility function.
+
+**Approach:** Move `SizedFilename` to a shared package (e.g., `backend/coverart/` or `backend/fileutil/`). Update the three callers: `library/`, `player/`, and `playlist/`.
+
+---
+
+### 7. Consolidate `LibraryScanComplete` handling
+
+**Problem:** `LibraryScanComplete` is listened to directly in 10+ components (`genres-view.ts`, `artists-view.ts`, `cover-grid.ts`, `track-list.ts`, `playlist-view.ts`, `genre-details.ts`, `artist-details.ts`, `playlist-picker.ts`, `config-page.ts`, `library-manager.ts`) in addition to `library-store.ts` and `playlist-store.ts`. Each component independently re-fetches its data.
+
+**Why it matters:** The stores already invalidate their caches and notify subscribers on this event. Components that use the store controllers should get re-rendered automatically. The direct listeners exist because many components load data independently from the stores (calling Go bindings directly), which means the stores aren't serving their full purpose as centralized data sources.
+
+**Approach:** For components that already use `LibraryController`/`PlaylistController`, the store subscription should handle cache invalidation. The controller's `hostConnected` subscribes and `requestUpdate` triggers a re-render, which calls the async data getter, which will re-fetch since the cache was invalidated. Remove the redundant direct `EventsOn(LibraryScanComplete)` from components that go through stores. For components like `playlist-picker.ts` that call Go bindings directly (bypassing stores), either route them through the store or accept the direct listener as intentional.
+
+---
+
+### 8. Type the WebAwesome popup interactions (eliminate 49x `as any`)
+
+**Problem:** Every component with a context menu uses `(popup as any).anchor = ...` and `(popup as any).active = true`. This pattern appears 49 times across `track-list.ts`, `cover-grid.ts`, `queue-panel.ts`, `playlist-view.ts`, `genres-view.ts`, `artists-view.ts`.
+
+**Why it matters:** Type safety is completely bypassed for a core interaction pattern. Typos in property names (`actve` instead of `active`) would silently fail.
+
+**Approach:** Create a type declaration for the WebAwesome popup element (or find one in their package). Alternatively, write a small typed utility:
+```typescript
+function openPopup(popup: Element, anchor: Element | VirtualAnchor): void
+function closePopup(popup: Element): void
+```
+Replace all 49 `as any` casts with calls to these utilities.
+
+---
+
+### 9. Replace `GetCurrentTrackInfo` `map[string]interface{}` with a struct
+
+**Problem:** `player.GetCurrentTrackInfo()` returns `map[string]interface{}` with stringly-typed keys (`"fileName"`, `"filePath"`, `"state"`, `"title"`, etc.). The `emitTrackChanged()` method mutates this map by adding keys after the fact.
+
+**Why it matters:** No compile-time safety — typos in key names are silent bugs. The Wails binding generator would produce typed TypeScript if given a struct.
+
+**Approach:** Define a `TrackInfo` struct in the player package with all the fields. Return it from `GetCurrentTrackInfo`. Update `emitTrackChanged` to build the struct directly instead of mutating a map.
+
+---
+
+### 10. Move `FullRescan` orchestration from library to app
+
+**Problem:** `library.Library` holds references to the queue (`queueClearer` interface) and playlist service (`playlistRestorer` interface), set via `SetQueue()` and `SetPlaylistRestorer()`. The `FullRescan` method in `rescan.go` orchestrates clearing the queue and restoring playlists — cross-cutting concerns that aren't really library responsibilities.
+
+**Why it matters:** The library package shouldn't know about queue clearing or playlist restoration. This creates a dependency web (`app` -> `library` -> `queue`, `app` -> `library` -> `playlist`).
+
+**Approach:** Move the `FullRescan` orchestration to the `app` level. The app already has references to all three packages. The library would only expose `Scan()` and a `ClearAndRescan()` that handles only library concerns (clear DB, walk files, extract metadata). The app's `FullRescan` handler would call `queue.Clear()`, `library.ClearAndRescan()`, then `playlist.RestoreAll()`.
+
+---
+
+### 11. Fix double `LibraryScanStarted` event during FullRescan
+
+**Problem:** `rescan.go:22` emits `LibraryScanStarted`, then calls `Scan()` which emits `LibraryScanStarted` again at `library.go:191`. The frontend receives two `LibraryScanStarted` events for a single full rescan.
+
+**Why it matters:** Frontend components may show duplicate "scanning" UI state transitions or start/reset loading indicators twice.
+
+**Approach:** Remove the `LibraryScanStarted` emission from either `FullRescan` or `Scan`. Since `Scan` is also called independently, keep it in `Scan` and remove it from `FullRescan`.
+
+---
+
+### 12. Inconsistent communication patterns: queue (events) vs playlist (bindings)
+
+**Problem:** Queue operations use 14+ `Request*` events with manual `data[0].(type)` casting in ~300 lines of handler boilerplate. Playlist operations use direct Wails bindings with type-safe Go function signatures.
+
+**Why it matters:** Inconsistency makes the codebase harder to learn. The queue's event-only approach requires substantial boilerplate that the playlist avoids. New features on the queue require touching 4 files (Go event constant, TS event constant, Go handler, TS store method) vs 1-2 files for the playlist.
+
+**Approach:** This is a larger refactor. Two options:
+1. **Move queue to bindings** (recommended): Add the queue to `FEBindings`, expose typed methods, call them directly from the frontend store. Remove the event handlers and the `Request*` events. Keep the backend-to-frontend events (`QueueChanged`, etc.) for state push.
+2. **Accept the inconsistency**: Document the rationale (queue existed before playlists, events were the original pattern, bindings were adopted later). Add a comment in AGENTS.md.
+
+---
+
+## P3 — Fix opportunistically
+
+### 13. Dead player methods: `ChangeVolume`, `MuteToggle`, `CurrentPosition`
+
+**Problem:** `ChangeVolume()` (`player.go`), `MuteToggle()` (`player.go`), and `CurrentPosition()` (percentage-based, `player.go`) have zero callers anywhere in the codebase.
+
+**Approach:** Delete them, or keep them if you plan to add keyboard shortcuts / media key support soon.
+
+---
+
+### 14. Unused queue sentinels: `ErrEmptyQueue`, `ErrNoPlayer`
+
+**Problem:** Defined in `queue.go` but never returned or checked.
+
+**Approach:** Delete them, or wire them into the appropriate error paths if they were intended for future validation.
+
+---
+
+### 15. `SeekFailed` event emitted but never listened to
+
+**Problem:** `player.go` emits `SeekFailed` when seeking fails, but no frontend code subscribes to it. Users get no feedback on seek failure.
+
+**Approach:** Either add a frontend listener that shows a brief notification/toast, or remove the event emission if seek failure feedback isn't needed.
+
+---
+
+### 16. `path.Join` instead of `filepath.Join` in config
+
+**Problem:** `config/config.go:41` uses `path.Join` (POSIX paths) instead of `filepath.Join` (OS-aware paths) for constructing the config file path.
+
+**Approach:** Replace with `filepath.Join`. Single-line change.
+
+---
+
+### 17. Replace 200ms sleep with frontend-ready handshake
+
+**Problem:** `app.go:205-216` uses `time.Sleep(200 * time.Millisecond)` before emitting state to the frontend, assuming it will be ready by then.
+
+**Approach:** Have the frontend emit a "ready" event when its stores have initialized. The backend listens for this event and then emits the current state. Eliminates the timing assumption.
+
+---
+
+### 18. Custom `sortInts` in queue instead of `slices.Sort`
+
+**Problem:** `queue.go` has a hand-written insertion sort for int slices, but `slices.Sort()` is already used elsewhere in the same file.
+
+**Approach:** Replace the custom `sortInts` with `slices.Sort`. Single-line change.
+
+---
+
+### 19. `playlist-picker.ts` bypasses `playlistStore`
+
+**Problem:** `playlist-picker.ts` calls `GetAllPlaylists()` directly from the Go binding instead of going through `playlistStore`. It fetches only summaries (not `WithTracks`), which is why it doesn't use the store.
+
+**Approach:** Either add a `getSummaries()` method to the playlist store that caches just the summary list, or accept this as intentional since the picker only needs summaries and the full `WithTracks` fetch would be wasteful for this use case.
+
+---
+
+### 20. `library-manager.ts` and `config-page.ts` overlap
+
+**Problem:** Both components exist (different nav routes: "libraries" vs "settings"). `config-page.ts` has a comment saying scan metrics were "carried over from library-manager". They may have diverging copies of similar logic.
+
+**Approach:** Audit both components for duplicated logic. If the library manager's functionality is fully subsumed by the config page, consider removing it and redirecting the "libraries" nav route.
diff --git a/backend/player/player.go b/backend/player/player.go
index 5c3bb6d..b49cdb1 100644
--- a/backend/player/player.go
+++ b/backend/player/player.go
@@ -144,21 +144,6 @@ func (p *Player) registerEventHandlers() {
return
}
- runtime.EventsOn(
- ctx,
- events.RequestPlay,
- func(_ ...any) {
- p.logger.Info("Received RequestPlayEvent")
-
- if err := p.Play(); err != nil {
- p.logger.Warn(
- "Play requested but not ready",
- "err", err,
- )
- }
- },
- )
-
runtime.EventsOn(
ctx,
events.RequestPause,
diff --git a/backend/queue/queue.go b/backend/queue/queue.go
index a8b1834..ac263a2 100644
--- a/backend/queue/queue.go
+++ b/backend/queue/queue.go
@@ -181,7 +181,7 @@ func (q *Queue) registerEventHandlers() {
runtime.EventsOn(q.ctx, events.RequestPlay, func(_ ...any) {
q.logger.Info("Received RequestPlay")
- q.PlayFromStart()
+ q.Play()
})
runtime.EventsOn(q.ctx, events.RequestNext, func(_ ...any) {
@@ -1481,6 +1481,43 @@ func (q *Queue) Previous() {
q.emitIndexChanged()
}
+// Play handles a play request by either resuming the current track or
+// starting playback from the beginning of the queue. When a track is
+// already active (currentIndex != -1) the player is told to resume;
+// otherwise playback starts from the first track (or a random one when
+// shuffle is enabled).
+func (q *Queue) Play() {
+ q.mu.Lock()
+ defer q.mu.Unlock()
+
+ if len(q.tracks) == 0 {
+ return
+ }
+
+ // A track is already active — ask the player to resume.
+ if q.currentIndex != -1 {
+ if q.player == nil {
+ q.logger.Error(
+ "No player set, cannot resume",
+ )
+
+ return
+ }
+
+ if err := q.player.Play(); err != nil {
+ q.logger.Warn(
+ "Resume requested but player not ready",
+ "err", err,
+ )
+ }
+
+ return
+ }
+
+ // No active track — start from the beginning.
+ q.playFromStart()
+}
+
// PlayFromStart restarts playback from the beginning of the queue.
// If shuffle is enabled, a new shuffle order is generated and playback
// starts from a random track. This is a no-op when a track is already
@@ -1489,6 +1526,12 @@ func (q *Queue) PlayFromStart() {
q.mu.Lock()
defer q.mu.Unlock()
+ q.playFromStart()
+}
+
+// playFromStart is the lock-free inner implementation of PlayFromStart.
+// The caller must hold q.mu.
+func (q *Queue) playFromStart() {
if q.currentIndex != -1 {
return
}
diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts
index c9bc756..f0eca7a 100644
--- a/frontend/src/components/track-list/track-list.ts
+++ b/frontend/src/components/track-list/track-list.ts
@@ -122,6 +122,16 @@ export class TrackList extends LitElement implements SelectionHost {
typeof setTimeout
> | null = null;
+ // -- Memoisation caches for filtered / sorted tracks --
+ private cachedFilteredTracks: library.Track[] = [];
+ private cachedSortedTracks: library.Track[] = [];
+ private prevFilterTracks: library.Track[] = [];
+ private prevFilterTerm = '';
+ private prevFilterColIds = '';
+ private prevSortFiltered: library.Track[] = [];
+ private prevSortField: string | null = null;
+ private prevSortDir: SortDirection = 'asc';
+
private closeHandler = () => this.closeContextMenu();
private mousedownCloseHandler = (
@@ -181,11 +191,49 @@ export class TrackList extends LitElement implements SelectionHost {
private hasRestoredScroll = false;
// =================================================================
- // Filtered tracks (search)
+ // Filtered / sorted tracks (memoised)
// =================================================================
- private get filteredTracks(): library.Track[] {
- const term = this.searchCtrl.term.toLowerCase();
+ /**
+ * Recompute the filtered and sorted track caches when
+ * their inputs have changed. Called from willUpdate()
+ * so the caches are ready before render().
+ */
+ private recomputeTrackCaches() {
+ const term = this.searchCtrl.term;
+ const colIds =
+ this.trackListCtrl.columnIds.join(',');
+
+ if (
+ this.tracks !== this.prevFilterTracks ||
+ term !== this.prevFilterTerm ||
+ colIds !== this.prevFilterColIds
+ ) {
+ this.prevFilterTracks = this.tracks;
+ this.prevFilterTerm = term;
+ this.prevFilterColIds = colIds;
+ this.cachedFilteredTracks =
+ this.computeFilteredTracks();
+ }
+
+ if (
+ this.cachedFilteredTracks !==
+ this.prevSortFiltered ||
+ this.sortField !== this.prevSortField ||
+ this.sortDirection !== this.prevSortDir
+ ) {
+ this.prevSortFiltered =
+ this.cachedFilteredTracks;
+ this.prevSortField = this.sortField;
+ this.prevSortDir = this.sortDirection;
+ this.cachedSortedTracks =
+ this.computeSortedTracks();
+ }
+ }
+
+ private computeFilteredTracks(): library.Track[] {
+ const term =
+ this.searchCtrl.term.toLowerCase();
if (!term) return this.tracks;
@@ -201,12 +249,8 @@ export class TrackList extends LitElement implements SelectionHost {
);
}
- // =================================================================
- // Sorted tracks
- // =================================================================
-
- private get sortedTracks(): library.Track[] {
- const tracks = this.filteredTracks;
+ private computeSortedTracks(): library.Track[] {
+ const tracks = this.cachedFilteredTracks;
if (!this.sortField) return tracks;
@@ -227,11 +271,11 @@ export class TrackList extends LitElement implements SelectionHost {
// =================================================================
getItemKey(index: number): string | undefined {
- return this.sortedTracks[index]?.FilePath;
+ return this.cachedSortedTracks[index]?.FilePath;
}
getItemCount(): number {
- return this.sortedTracks.length;
+ return this.cachedSortedTracks.length;
}
onSelectionChanged(): void {
@@ -1003,6 +1047,8 @@ export class TrackList extends LitElement implements SelectionHost {
this.tracks = this.externalTracks;
this.selection.clear();
}
+
+ this.recomputeTrackCaches();
}
override firstUpdated() {
@@ -1664,7 +1710,7 @@ export class TrackList extends LitElement implements SelectionHost {
}
override render() {
- const visibleTracks = this.sortedTracks;
+ const visibleTracks = this.cachedSortedTracks;
const cols = this.activeColumns;
return html`