diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 989f072..40cd504 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -8,6 +8,7 @@ - ✅ **v1.0 Consolidation** — Phases 1-8 (shipped 2026-03-05) — [archive](milestones/v1.0-ROADMAP.md) - 🔄 **v1.1 Multi-Library Support** — Phase 9 complete, Phases 10-13 in progress +- 🔄 **Performance Optimization** — Phase 14 (cross-cutting, parallel to v1.1) ## Phases @@ -109,6 +110,23 @@ Plans: 5. When a library is removed, its tracks in playlists become phantom entries — visually distinguished (greyed out / icon) with preserved title, artist, album metadata instead of disappearing **Plans:** TBD +### Phase 14: Performance Optimization +**Goal:** Scrolling, navigation, and rendering are as smooth and fast as possible — scrolling feels like a native animation, navigation is instant, no unnecessary re-renders +**Depends on:** Nothing (cross-cutting, can execute in parallel with v1.1 phases) +**Requirements:** PERF-SCROLL-01, PERF-SCROLL-02, PERF-SCROLL-03, PERF-NAV-01, PERF-NAV-02, PERF-RENDER-01, PERF-RENDER-02, PERF-DIAG-01 +**Success Criteria** (what must be TRUE): + 1. Scrolling in all views (tracks, albums, artists, genres, queue, playlists) is smooth at 60fps — no jank, no stuttering, no blank areas + 2. Navigating between primary views (tracks, albums, artists, genres, playlists, settings) is near-instant — no component destruction/recreation, scroll positions preserved + 3. Render hot paths (renderTrackRow, renderTrackItem) create zero new closures per frame — all event handling uses delegation + 4. Store notifications are batched (queueMicrotask) and components only re-render when their relevant data changes + 5. A profiling guide documents how to diagnose performance issues using pprof (backend) and DevTools (frontend) +**Plans:** 4 plans +Plans: +- [ ] 14-01-PLAN.md — CSS containment + GPU layer promotion on all scroll containers +- [ ] 14-02-PLAN.md — View caching navigation system (replace innerHTML destruction) +- [ ] 14-03-PLAN.md — Render hot-path optimization (closure elimination, store granularity) +- [ ] 14-04-PLAN.md — Scroll event optimization, profiling guide, performance verification checkpoint + ## Progress | Phase | Milestone | Plans Complete | Status | Completed | @@ -126,7 +144,8 @@ Plans: | 11. Per-Library Scan Pipeline | 3/3 | Complete | 2026-03-09 | - | | 12. Library CRUD & Data Integrity | v1.1 | 1/2 | In Progress | - | | 13. Library Views & Phantom Tracks | v1.1 | 0/? | Not started | - | +| 14. Performance Optimization | v1.1 | 0/4 | Not started | - | --- *Roadmap created: 2026-02-27* -*Last updated: 2026-03-08 — Multi-library phases 10-13 created from 23 requirements* +*Last updated: 2026-03-14 — Phase 14 (Performance Optimization) added with 4 plans* diff --git a/.planning/phases/14-performance-optimization/14-01-PLAN.md b/.planning/phases/14-performance-optimization/14-01-PLAN.md new file mode 100644 index 0000000..db6e50a --- /dev/null +++ b/.planning/phases/14-performance-optimization/14-01-PLAN.md @@ -0,0 +1,166 @@ +--- +phase: 14-performance-optimization +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - frontend/src/components/cover-grid/cover-grid-styles.ts + - frontend/src/components/track-list/track-list.ts + - frontend/src/components/queue-panel/queue-panel.ts + - frontend/src/components/artists-view/artists-view.ts + - frontend/src/components/genres-view/genres-view.ts + - frontend/src/components/playlist-view/playlist-view.ts + - frontend/index.css +autonomous: true +requirements: [PERF-SCROLL-01, PERF-SCROLL-02] + +must_haves: + truths: + - "All scroll containers use CSS contain to limit browser layout/paint scope" + - "Virtualizer scroll containers are GPU-promoted for composited scrolling" + - "The main content area uses CSS containment to isolate layout from sidebar/header/footer" + - "Album cards use content-visibility auto to skip rendering when off-screen" + artifacts: + - path: "frontend/src/components/cover-grid/cover-grid-styles.ts" + provides: "CSS contain and will-change on scroll containers, content-visibility on album cards" + contains: "contain:" + - path: "frontend/src/components/track-list/track-list.ts" + provides: "CSS contain and will-change on virtualizer host" + contains: "contain:" + - path: "frontend/src/components/queue-panel/queue-panel.ts" + provides: "CSS contain on queue panel scroll area" + contains: "contain:" + - path: "frontend/index.css" + provides: "CSS containment on .main-panel and .content-area" + contains: "contain:" + key_links: + - from: "frontend/index.css" + to: ".main-panel" + via: "CSS contain: strict on layout boundary" + pattern: "contain:\\s*(strict|layout)" + - from: "cover-grid-styles.ts" + to: ".grid-scroll-container" + via: "will-change: transform for GPU compositing" + pattern: "will-change" +--- + + +Add CSS containment, GPU layer promotion, and content-visibility to all scroll containers and layout boundaries for dramatically smoother scrolling performance. + +Purpose: The browser currently cannot optimize layout/paint for any component — no `contain`, no `will-change`, no `content-visibility` anywhere. Adding these CSS properties allows the browser to skip layout recalculation for off-screen content and use GPU-composited scrolling for list containers. + +Output: All scroll-heavy components have CSS containment; scrolling moves to the compositor thread where possible. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@frontend/index.css +@frontend/src/components/cover-grid/cover-grid-styles.ts + + + + + + Task 1: Add CSS containment to app shell layout boundaries + frontend/index.css + +Add CSS containment properties to the app shell layout to isolate layout recalculation boundaries: + +1. On `.content-area`: Add `contain: layout style;` — isolates the main content + queue panel from affecting header/sidebar/footer layout. Do NOT use `contain: strict` here because strict includes size containment which would break the flex layout. + +2. On `.main-panel`: Add `contain: strict;` — the main panel has explicit dimensions (flex: 1, overflow: hidden) so strict containment (layout + size + paint + style) is safe and maximally beneficial. This means any DOM changes inside the main panel cannot trigger layout recalculation outside it. + +3. On `.main-panel > *`: Add `contain: layout style paint;` — each view component inside main-panel gets paint containment (creates new stacking context, isolates paint) plus layout containment. Do NOT add size containment since height: 100% needs to resolve from parent. + +4. On `body div.sidebar`: Add `contain: layout style paint;` — sidebar is a fixed-width element that shouldn't affect main panel layout. + +5. On `.bottom-bar`: Add `contain: layout style;` — footer has fixed height, isolate from content reflows. + +Do NOT add `will-change` to the app shell elements — those are for scroll containers only (Task 2). + + + The app builds successfully: `cd frontend && npx vite build --mode development 2>&1 | tail -5` + Visual check: all layout areas still render correctly (no collapsed panels, no overflow issues). + + App shell layout boundaries have CSS containment isolating layout recalculation between header, sidebar, main panel, and footer. + + + + Task 2: Add GPU promotion and containment to all scroll containers + + frontend/src/components/cover-grid/cover-grid-styles.ts + frontend/src/components/track-list/track-list.ts + frontend/src/components/queue-panel/queue-panel.ts + frontend/src/components/artists-view/artists-view.ts + frontend/src/components/genres-view/genres-view.ts + frontend/src/components/playlist-view/playlist-view.ts + + +Add CSS containment and GPU layer promotion to every scroll container and virtualized list component. The goal is to make scrolling happen on the GPU compositor thread rather than the main thread. + +**cover-grid-styles.ts:** +- On `:host`: Add `contain: layout style;` (already has `overflow: hidden`) +- On `.grid-scroll-container`: Add `contain: paint;` and `will-change: transform;` — this is the actual scroll container for the album grid. `will-change: transform` promotes it to its own GPU layer so scrolling is composited. `contain: paint` creates a new stacking context. +- On `.album-card`: Add `content-visibility: auto;` with `contain-intrinsic-size: auto var(--card-width, 176px) auto calc(var(--card-width, 176px) + 40px);` — this tells the browser to skip rendering album cards that are not in the viewport. The intrinsic size hint prevents layout shift. Note: lit-virtualizer already handles virtualization, but content-visibility provides an additional browser-native layer for cards near the viewport edges that are rendered but not visible. + +**track-list.ts (in static styles):** +- On `:host`: Add `contain: layout style;` +- On `lit-virtualizer`: Add `contain: paint;` and `will-change: transform;` — the virtualizer element is the scroller for the track list. + +**queue-panel.ts (in static styles):** +- On `:host` or the scroll container: Add `contain: layout style paint;` +- On `lit-virtualizer`: Add `contain: paint;` and `will-change: transform;` + +**artists-view.ts (in static styles):** +- On `:host`: Add `contain: layout style;` +- On the grid virtualizer parent scroll container: Add `contain: paint;` and `will-change: transform;` + +**genres-view.ts (in static styles):** +- Same pattern as artists-view. + +**playlist-view.ts (in static styles):** +- On `:host`: Add `contain: layout style;` +- On `.playlist-list` (the native scroll container): Add `contain: paint;` and `will-change: transform;` — even though this isn't virtualized, GPU compositing still helps scrolling. + +**Important:** Do NOT add `will-change: transform` to `:host` elements — only to actual scroll containers. `will-change` on non-scrolling elements wastes GPU memory. Only apply it to elements with `overflow-y: auto/scroll`. + +**Important:** Verify that `contain: paint` doesn't clip absolutely-positioned tooltips/popups that need to overflow. Context menus and popups use `wa-popup` which are appended to the shadow root, so they should still work. But verify this. + + + `cd frontend && npx vite build --mode development 2>&1 | tail -5` completes without errors. + Run the app and test: (1) scroll the track list rapidly — should feel smoother, (2) scroll the album grid — should feel smoother, (3) right-click a track — context menu should still appear correctly and not be clipped, (4) open a cover grid album dropdown — should still work and not be clipped by contain: paint. + + All 6 scroll-heavy components have CSS containment on hosts and will-change: transform on scroll containers for GPU-composited scrolling. Content-visibility on album cards skips rendering for off-viewport cards. + + + + + +After both tasks: +1. `cd frontend && npx vite build --mode development` builds without errors +2. App starts and all views render correctly +3. Scrolling in track list, album grid, queue panel, artists, genres, playlists all work without visual artifacts +4. Context menus, popups, and tooltips are not clipped by paint containment +5. Album grid dropdown (expanded album) still renders correctly between grid splits + + + +- CSS `contain` property present on all 6 scroll component `:host` elements +- CSS `will-change: transform` present on all 6 scroll containers (not hosts) +- CSS `contain: strict` on `.main-panel` in index.css +- CSS `content-visibility: auto` on `.album-card` in cover-grid-styles +- No visual regressions (popups, context menus, dropdowns all work) +- Build succeeds + + + +After completion, create `.planning/phases/14-performance-optimization/14-01-SUMMARY.md` + diff --git a/.planning/phases/14-performance-optimization/14-02-PLAN.md b/.planning/phases/14-performance-optimization/14-02-PLAN.md new file mode 100644 index 0000000..6bb5ba8 --- /dev/null +++ b/.planning/phases/14-performance-optimization/14-02-PLAN.md @@ -0,0 +1,187 @@ +--- +phase: 14-performance-optimization +plan: 02 +type: execute +wave: 1 +depends_on: [] +files_modified: + - frontend/index.ts + - frontend/index.html + - frontend/index.css +autonomous: true +requirements: [PERF-NAV-01, PERF-NAV-02] + +must_haves: + truths: + - "Navigating between views does not destroy and recreate components" + - "Previously visited views retain their DOM and internal state (scroll position, expanded items)" + - "Only the active view is visible; inactive views are hidden with display: none" + - "Navigation between cached views feels instant (no data refetch, no virtualizer reinit)" + artifacts: + - path: "frontend/index.ts" + provides: "View cache manager that creates views once and toggles visibility" + contains: "display" + - path: "frontend/index.css" + provides: "Hidden state for inactive views" + contains: "display: none" + key_links: + - from: "frontend/index.ts" + to: "#main-content" + via: "View cache toggling active/hidden children" + pattern: "display" + - from: "frontend/index.ts" + to: "navigate event" + via: "Show cached view or create new one" + pattern: "navigate" +--- + + +Replace the innerHTML destruction/recreation navigation pattern with a view caching system that keeps previously visited views in the DOM (hidden) and toggles visibility on navigation. + +Purpose: Currently, every navigation event destroys the active view via `innerHTML = ''` and creates a new component from scratch. This means virtualizers reinitialize, data refetches from cache, scroll positions must be restored, and cover art images reload. By caching views in the DOM and toggling `display: none` / `display: block`, navigation becomes instant. + +Output: Navigation between primary views (tracks, albums, artists, genres, playlists, settings) is instant — no component destruction, no virtualizer reinit, no scroll position loss. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@frontend/index.ts +@frontend/index.html +@frontend/index.css + + + + +From frontend/index.ts: +- Navigation via CustomEvent('navigate', { detail: { view: '...' } }) +- Primary views: 'albums', 'tracks', 'playlists', 'artists', 'genres', 'settings' +- Detail views: 'artist-details', 'playlist-details', 'genre-details' (take extra attributes) +- mainContent = document.getElementById('main-content') +- Currently: mainContent.innerHTML = '' for each view + +From frontend/index.html: +-
+- Default view is track-list (rendered in HTML) + +From frontend/index.css: +- .main-panel > * { height: 100%; } + + + + + + Task 1: Implement view caching navigation system + frontend/index.ts, frontend/index.html, frontend/index.css + +Replace the `innerHTML`-based navigation with a view cache that keeps views alive in the DOM. + +**Design:** +- Maintain a `Map` called `viewCache` for primary views (tracks, albums, artists, genres, playlists, settings). +- Detail views (artist-details, playlist-details, genre-details) are NOT cached — they are ephemeral and created fresh each time (because they depend on specific IDs/names that change on each navigation). When navigating to a detail view, remove any existing detail view element and create a fresh one. +- On navigation, hide the current view (`display: none`), then show the target view. If the target view isn't in the cache yet, create it and add to the cache. + +**Implementation in index.ts:** + +1. Add a `viewCache` Map and a `currentView` string variable at module scope: +```typescript +const viewCache = new Map(); +let currentViewEl: HTMLElement | null = null; +let currentDetailEl: HTMLElement | null = null; +``` + +2. Define a `VIEW_TAGS` mapping for cacheable primary views: +```typescript +const VIEW_TAGS: Record = { + tracks: 'track-list', + albums: 'cover-grid', + artists: 'artists-view', + genres: 'genres-view', + playlists: 'playlist-view', + settings: 'config-page', +}; +``` + +3. Replace the entire `document.addEventListener('navigate', ...)` handler with a new one that: + + a. For primary views (key exists in VIEW_TAGS): + - If a detail view element exists, remove it from DOM and set `currentDetailEl = null` + - If the view is already in `viewCache`, retrieve it; otherwise create it via `document.createElement(VIEW_TAGS[view])` and add to both `viewCache` and `mainContent` + - Hide `currentViewEl` by setting `style.display = 'none'` + - Show the target element by setting `style.display = ''` (empty string restores the default) + - Update `currentViewEl` reference + + b. For detail views (artist-details, playlist-details, genre-details): + - Hide `currentViewEl` (set display: none) + - If `currentDetailEl` exists, remove it from DOM + - Create the detail element, set attributes (artistId, artistName, etc.), append to `mainContent` + - Set `currentDetailEl` to the new element + +4. Initialize the default view (track-list) from the HTML: +```typescript +const initialTrackList = mainContent.querySelector('track-list'); +if (initialTrackList) { + viewCache.set('tracks', initialTrackList as HTMLElement); + currentViewEl = initialTrackList as HTMLElement; +} +``` + +**In index.html:** No changes needed — `` remains the default. + +**In index.css:** Add a rule for hidden views: +```css +.main-panel > [data-view-hidden] { + display: none !important; +} +``` + +Actually, simpler to just use inline `style.display` since we control it in JS. No CSS changes needed for this. BUT we should add `contain: layout style paint` to hidden views to ensure they don't participate in layout even if display:none is somehow bypassed (belt and suspenders — Plan 14-01 already adds containment but this is the navigation-specific insurance). + +**Important considerations:** +- The `searchStore.setCurrentView(view)` call must remain at the top of the handler +- View elements get `connectedCallback` called only once (on first creation), and `disconnectedCallback` is never called (they stay in DOM). This is fine — Lit components handle this correctly. Controllers subscribe in `hostConnected` and unsubscribe in `hostDisconnected`, so subscriptions stay active. This is INTENDED for cached views. +- Memory consideration: We're keeping at most 6 primary view components alive. Each is a single custom element with its own shadow DOM. The data they hold (tracks, albums, etc.) is already in the store cache regardless. The only extra memory is the DOM nodes for the virtualizer's rendered items (typically ~20-50 visible items per view). This is negligible. +- Scroll position: With view caching, scroll positions are naturally preserved because the DOM is never destroyed. This means the existing scroll save/restore logic in each component (`visibilityChanged` handlers, `scrollToIndex` calls) can eventually be simplified, but DO NOT remove them in this plan — they still serve as fallback for data invalidation scenarios. + + + `cd frontend && npx vite build --mode development 2>&1 | tail -5` builds without errors. + Functional test: Start app → see tracks (default) → click Albums → albums appear → click Tracks → tracks reappear instantly WITH scroll position preserved → click Artists → artists appear → click back to Albums → albums still show previously loaded content → navigate to artist-details → back to Artists → artists view preserved. + + +- Primary views (tracks, albums, artists, genres, playlists, settings) are created once and cached in DOM +- Navigation toggles visibility instead of destroying/recreating +- Detail views (artist-details, playlist-details, genre-details) are still created fresh (they're parameterized) +- Scroll positions naturally preserved by keeping DOM alive +- No component destruction means no virtualizer reinit, no data refetch, no image reload + + + + + + +1. Build succeeds +2. Navigate tracks → albums → tracks: track list shows same scroll position +3. Navigate to albums → scroll down → navigate tracks → back to albums: scroll position preserved +4. Navigate to artist-details → back button → artists view: artists view preserved +5. Navigate to playlist-details → back → playlists: playlists preserved +6. Library scan completes → views update correctly (data invalidation still triggers refetch) +7. Search works across all views (search state preserved per view) + + + +- No `innerHTML = '<...>'` patterns remain in index.ts for primary views +- `viewCache` Map maintains at most 6 cached view elements +- Navigation between cached views takes <16ms (one frame) +- All existing navigation paths still work (primary views, detail views, settings) +- No memory leaks (view count bounded, no listener accumulation) + + + +After completion, create `.planning/phases/14-performance-optimization/14-02-SUMMARY.md` + diff --git a/.planning/phases/14-performance-optimization/14-03-PLAN.md b/.planning/phases/14-performance-optimization/14-03-PLAN.md new file mode 100644 index 0000000..6439669 --- /dev/null +++ b/.planning/phases/14-performance-optimization/14-03-PLAN.md @@ -0,0 +1,418 @@ +--- +phase: 14-performance-optimization +plan: 03 +type: execute +wave: 2 +depends_on: ["14-01"] +files_modified: + - frontend/src/components/track-list/track-list.ts + - frontend/src/components/queue-panel/queue-panel.ts + - frontend/src/components/cover-grid/cover-grid.ts + - frontend/src/store/queue-store.ts + - frontend/src/store/library-store.ts + - frontend/src/store/controllers/library-controller.ts +autonomous: true +requirements: [PERF-RENDER-01, PERF-RENDER-02] + +must_haves: + truths: + - "Scrolling does not create new arrow function closures per rendered item" + - "Queue store notifications are batched via queueMicrotask (matching library store pattern)" + - "Library store subscribers can subscribe to specific data types (tracks, albums, etc.) and only update when their data changes" + artifacts: + - path: "frontend/src/components/track-list/track-list.ts" + provides: "Bound method references instead of inline arrow closures in renderTrackRow" + contains: "this.onTrackRowClick" + - path: "frontend/src/components/queue-panel/queue-panel.ts" + provides: "Bound method references instead of inline closures in renderTrackItem" + contains: "this.onQueueTrackClick" + - path: "frontend/src/store/queue-store.ts" + provides: "queueMicrotask-based notification batching" + contains: "queueMicrotask" + - path: "frontend/src/store/library-store.ts" + provides: "Granular subscription by data type" + contains: "subscribeToTracks" + key_links: + - from: "frontend/src/store/library-store.ts" + to: "frontend/src/store/controllers/library-controller.ts" + via: "Granular subscription replacing blanket subscribe" + pattern: "subscribeTo" + - from: "frontend/src/store/queue-store.ts" + to: "notify" + via: "queueMicrotask batching" + pattern: "queueMicrotask" +--- + + +Eliminate per-item closure allocation during scroll rendering and reduce unnecessary component re-renders by adding notification batching and granular store subscriptions. + +Purpose: Every scroll frame, `renderTrackRow` and `renderTrackItem` create new arrow function closures for click, dblclick, contextmenu, and dragstart handlers. This causes GC pressure during rapid scrolling. Additionally, the queue store notifies synchronously (not batched), and the library store sends blanket notifications for any data change even if the subscribing component only cares about one data type. + +Output: Scroll rendering is GC-friendly with stable function references; store notifications are batched and granular. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@frontend/src/components/track-list/track-list.ts +@frontend/src/components/queue-panel/queue-panel.ts +@frontend/src/store/queue-store.ts +@frontend/src/store/library-store.ts +@frontend/src/store/controllers/library-controller.ts + + + + +```typescript +private renderTrackRow = (track: library.Track, index: number): unknown => { + // Creates new closures every render: + // @click=${(e: MouseEvent) => this.onTrackRowClick(e, track, index)} + // @dblclick=${() => this.onTrackRowDblClick(track)} + // @contextmenu=${(e: MouseEvent) => this.onTrackContextMenu(e, track)} + // @dragstart=${(e: DragEvent) => this.onTrackDragStart(e, track)} +}; +``` + + +```typescript +private notify(): void { + for (const sub of this.subscribers) { + sub(); + } +} +``` + + +```typescript +// LibraryController subscribes to ALL store changes +hostConnected(): void { + this.unsubscribe = libraryStore.subscribe(() => { + this.host.requestUpdate(); + }); +} +``` + + +```typescript +private notify(): void { + if (this.notifyScheduled) return; + this.notifyScheduled = true; + queueMicrotask(() => { + this.notifyScheduled = false; + for (const sub of this.subscribers) { + sub(); + } + }); +} +``` + + + + + + Task 1: Eliminate per-item closure allocation in render hot paths + + frontend/src/components/track-list/track-list.ts + frontend/src/components/queue-panel/queue-panel.ts + frontend/src/components/cover-grid/cover-grid.ts + + +Replace inline arrow function closures in renderItem callbacks with event delegation or stable bound references. The key insight: lit-virtualizer's renderItem is called for each visible item on every scroll frame — closures created here are immediately GC pressure. + +**track-list.ts — renderTrackRow:** + +The current pattern creates 5 new closures per row per render: `@click`, `@dblclick`, `@contextmenu`, `@dragstart`, and the fav-icon `@click`. + +Convert to **data-attribute event delegation**: + +1. Add `data-index="${index}"` to the `.track-row` div. + +2. Instead of per-row `@click`, `@dblclick`, `@contextmenu`, `@dragstart` closures, use a single delegated event handler pattern. Add stable (bound once) event handlers on the `lit-virtualizer` element itself in `firstUpdated()`: + +```typescript +override firstUpdated() { + // ... existing firstUpdated code ... + + const virt = this.virtualizer; + if (virt) { + virt.addEventListener('click', this.onDelegatedClick); + virt.addEventListener('dblclick', this.onDelegatedDblClick); + virt.addEventListener('contextmenu', this.onDelegatedContextMenu); + // dragstart delegated on track-row (draggable="true" is on each row) + } +} +``` + +3. Create stable bound methods that extract the index from `data-index`: + +```typescript +private onDelegatedClick = (e: MouseEvent) => { + const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null; + if (!row) return; + const idx = Number(row.dataset.index); + const track = this.cachedSortedTracks[idx]; + if (!track) return; + + // Check if click was on fav-icon + const favEl = (e.target as HTMLElement).closest('.fav-icon'); + if (favEl) { + e.stopPropagation(); + void this.favCtrl.toggleFavorite(track.FilePath); + return; + } + + this.onTrackRowClick(e, track, idx); +}; + +private onDelegatedDblClick = (e: MouseEvent) => { + const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null; + if (!row) return; + const idx = Number(row.dataset.index); + const track = this.cachedSortedTracks[idx]; + if (track) this.onTrackRowDblClick(track); +}; + +private onDelegatedContextMenu = (e: MouseEvent) => { + const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null; + if (!row) return; + const idx = Number(row.dataset.index); + const track = this.cachedSortedTracks[idx]; + if (track) this.onTrackContextMenu(e, track); +}; +``` + +4. For `@dragstart`: Keep it inline on the row element but use the data-index delegation pattern. Since `draggable="true"` must be on the individual row, the dragstart event naturally targets the row. Add a single delegated handler: + +```typescript +private onDelegatedDragStart = (e: DragEvent) => { + const row = (e.target as HTMLElement).closest('.track-row') as HTMLElement | null; + if (!row) return; + const idx = Number(row.dataset.index); + const track = this.cachedSortedTracks[idx]; + if (track) this.onTrackDragStart(e, track); +}; +``` + +5. Update `renderTrackRow` to remove ALL inline closures: +```typescript +private renderTrackRow = (track: library.Track, index: number): unknown => { + // ... classMap, isFav, etc. remain the same ... + return html` +
+ +
+ +
+ ${/* column rendering stays the same */} +
+ `; +}; +``` + +6. Add `@dragend` as a single stable handler on the virtualizer too (it's already `this.onTrackDragEnd` which is stable). + +**CRITICAL:** Event delegation must work through shadow DOM. Since the virtualizer and its children are all within the same shadow root, `event.target.closest('.track-row')` works correctly. But verify with `composedPath()` if needed. + +**queue-panel.ts — renderTrackItem:** + +Apply the same event delegation pattern: +1. Add `data-index="${track.position}"` (or the loop index) to each `.track-item` +2. Register delegated handlers on the virtualizer in firstUpdated +3. Extract item index via `closest('.track-item')?.dataset.index` +4. Remove all inline closures from the template + +**cover-grid.ts — renderAlbumCard/renderGridEntry:** + +The cover grid already uses some delegation (it reads `data-index` for some operations). Verify that all click handlers on album cards use delegation. If any inline closures remain in `renderGridEntry` or `renderAlbumCard`, convert them. + +**Key rule:** After this change, `renderTrackRow` and `renderTrackItem` should create ZERO new function objects. Every handler reference should be stable (either a bound class method or a property arrow function defined once in the class body). + + + `cd frontend && npx vite build --mode development 2>&1 | tail -5` builds without errors. + Functional test: (1) Click a track → plays correctly, (2) Double-click → plays, (3) Right-click → context menu appears with correct track, (4) Drag a track → drag image shows, drop works, (5) Click favorite icon → toggles correctly, (6) Multi-select with Shift/Ctrl → works, (7) Queue panel: click, dblclick, drag, context menu all work. + + renderTrackRow and renderTrackItem create zero inline closures. All event handling uses delegation via data-index attributes and stable bound handlers. + + + + Task 2: Add notification batching to queue store and granular subscriptions to library store + + frontend/src/store/queue-store.ts + frontend/src/store/library-store.ts + frontend/src/store/controllers/library-controller.ts + + +**queue-store.ts — Add queueMicrotask batching:** + +The library store already uses `queueMicrotask` batching (added in Phase 8). The queue store does NOT — it calls all subscribers synchronously on every state change. This means rapid queue mutations (e.g., adding multiple tracks) trigger multiple synchronous re-renders. + +Add the same batching pattern from library-store: + +```typescript +private notifyScheduled = false; + +private notify(): void { + if (this.notifyScheduled) return; + this.notifyScheduled = true; + queueMicrotask(() => { + this.notifyScheduled = false; + for (const sub of this.subscribers) { + sub(); + } + }); +} +``` + +This coalesces multiple synchronous `notify()` calls into a single subscriber notification per microtask tick. Safe because Lit's `requestUpdate()` already deduplicates internally, but this prevents the overhead of even invoking all subscriber callbacks multiple times. + +**library-store.ts — Add granular data-type subscriptions:** + +Currently `subscribe()` registers a callback that fires on ANY store change (tracks, albums, artists, genres, cover size, loading state). This means: +- Track list component gets notified when albums change (unnecessary requestUpdate) +- Album grid gets notified when genres change (unnecessary requestUpdate) +- All components get notified when any loading flag changes + +Add type-specific subscriptions alongside the existing blanket `subscribe()`: + +```typescript +type DataType = 'tracks' | 'albums' | 'artists' | 'genres' | 'coverSize'; + +private typedSubscribers = new Map>(); + +subscribeTo(type: DataType, callback: Subscriber): () => void { + if (!this.typedSubscribers.has(type)) { + this.typedSubscribers.set(type, new Set()); + } + const subs = this.typedSubscribers.get(type)!; + subs.add(callback); + return () => subs.delete(callback); +} + +private notifyType(type: DataType): void { + const subs = this.typedSubscribers.get(type); + if (subs) { + for (const sub of subs) { + sub(); + } + } +} +``` + +Then update the data access methods to use `notifyType`: +- `getTracks()` finally block: call `notifyType('tracks')` instead of `notify()` +- `getAlbums()` finally block: call `notifyType('albums')` instead of `notify()` +- `getArtists()` finally block: call `notifyType('artists')` instead of `notify()` +- `getGenres()` finally block: call `notifyType('genres')` instead of `notify()` +- `setCoverSize()`: call `notifyType('coverSize')` instead of `notify()` +- `invalidate()`: Keep calling `notify()` (blanket) since invalidation affects everything + +Wait — this is tricky. The `notify()` method uses `queueMicrotask` batching. If we have both typed and blanket notifications in the same microtask, we need to be careful. + +**Simpler approach:** Instead of typed subscriptions on the store, make `LibraryController` smarter. The controller already has access to `cachedTracks`, `cachedAlbums`, etc. On each store notification, the controller can CHECK if the data it cares about actually changed before calling `requestUpdate()`: + +```typescript +// In LibraryController +hostConnected(): void { + // Track the references we last saw + let lastTracks = libraryStore.getCachedTracks(); + let lastAlbums = libraryStore.getCachedAlbums(); + + this.unsubscribe = libraryStore.subscribe(() => { + const newTracks = libraryStore.getCachedTracks(); + const newAlbums = libraryStore.getCachedAlbums(); + const newArtists = libraryStore.getCachedArtists(); + const newGenres = libraryStore.getCachedGenres(); + + // Only request update if data this host cares about changed + // Since we don't know what the host uses, check all and requestUpdate + // if ANY changed. But crucially, skip if loading state just toggled. + if (newTracks !== lastTracks || + newAlbums !== lastAlbums || + newArtists !== this.lastArtists || + newGenres !== this.lastGenres) { + lastTracks = newTracks; + lastAlbums = newAlbums; + // ... etc + this.host.requestUpdate(); + } + }); +} +``` + +Actually, this is still checking everything. The real win is: **don't call requestUpdate when only loading state changed**. The loading state toggling on/off during eagerFetch causes 8+ unnecessary requestUpdate calls across all components. + +**Refined approach for library-store.ts:** +Add a `changeGeneration` counter. Increment it only when actual data changes (not loading flags): + +```typescript +private changeGen = 0; + +// In getTracks, getAlbums, etc. — after setting this.tracks = tracks: +this.changeGen++; + +// In invalidate — after clearing caches: +this.changeGen++; +``` + +Then in `notify()`, also expose the generation. In the controller: + +```typescript +hostConnected(): void { + let lastGen = libraryStore.changeGeneration; + this.unsubscribe = libraryStore.subscribe(() => { + const gen = libraryStore.changeGeneration; + if (gen !== lastGen) { + lastGen = gen; + this.host.requestUpdate(); + } + }); +} +``` + +Add a public `get changeGeneration(): number` to the store. + +This means: loading flag changes trigger `notify()` but subscribers skip the update because `changeGeneration` hasn't changed. Only when actual data arrives (or is invalidated) do components re-render. + +Use this approach. It's simpler, backward-compatible, and eliminates the biggest source of unnecessary re-renders. + + + `cd frontend && npx vite build --mode development 2>&1 | tail -5` builds without errors. + Functional test: (1) App starts → all views load data correctly, (2) Trigger a library scan → views update when scan completes, (3) Queue operations (add, remove, reorder) work without lag, (4) Rapid queue additions don't cause visual stuttering. + + Queue store uses queueMicrotask batching. Library store has changeGeneration counter. LibraryController skips requestUpdate when only loading state changed. Result: fewer unnecessary component re-renders during data loading. + + + + + +After both tasks: +1. Build succeeds +2. All click/dblclick/contextmenu/drag interactions work on track list and queue panel +3. Selection (click, Shift+click, Ctrl+click) still works +4. Queue operations are responsive +5. Library scan invalidation still triggers view updates +6. No regressions in any view's functionality + + + +- renderTrackRow creates 0 inline closures (all delegation) +- renderTrackItem creates 0 inline closures (all delegation) +- queue-store.ts contains queueMicrotask batching +- library-store.ts has changeGeneration counter +- LibraryController checks changeGeneration before requestUpdate +- All existing interactions work correctly + + + +After completion, create `.planning/phases/14-performance-optimization/14-03-SUMMARY.md` + diff --git a/.planning/phases/14-performance-optimization/14-04-PLAN.md b/.planning/phases/14-performance-optimization/14-04-PLAN.md new file mode 100644 index 0000000..5d7f775 --- /dev/null +++ b/.planning/phases/14-performance-optimization/14-04-PLAN.md @@ -0,0 +1,294 @@ +--- +phase: 14-performance-optimization +plan: 04 +type: execute +wave: 2 +depends_on: ["14-01", "14-02"] +files_modified: + - frontend/src/components/cover-grid/scroll-manager.ts + - frontend/src/components/queue-panel/queue-panel.ts + - docs/PROFILING.md +autonomous: false +requirements: [PERF-SCROLL-03, PERF-DIAG-01] + +must_haves: + truths: + - "Cover grid scroll manager uses RAF-throttled scroll events instead of debounced saves" + - "Queue panel scroll correction monkey-patch is replaced with a cleaner CSS/layout solution" + - "A profiling guide documents how to diagnose frontend and backend performance issues" + - "User verifies scrolling smoothness across all views" + artifacts: + - path: "frontend/src/components/cover-grid/scroll-manager.ts" + provides: "RAF-throttled scroll position saves" + contains: "requestAnimationFrame" + - path: "docs/PROFILING.md" + provides: "Performance diagnosis guide" + contains: "pprof" + key_links: + - from: "docs/PROFILING.md" + to: "scripts/profile.sh" + via: "References profiling script usage" + pattern: "profile.sh" +--- + + +Polish scroll performance with targeted fixes to the cover grid scroll manager and queue panel, then create a performance profiling guide and verify all optimizations with user. + +Purpose: The cover grid scroll manager uses a 100ms debounced scroll save (fires after scrolling stops, not ideal for position tracking during rapid scrolling). The queue panel has a monkey-patched `_correctScrollError` which is a band-aid for lit-virtualizer's scroll correction fighting the native scrollbar. Both need cleaner solutions. Additionally, the user wants guidance on diagnosing performance issues using the existing pprof infrastructure. + +Output: Cleaner scroll handling, profiling documentation, and user-verified scroll smoothness. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@frontend/src/components/cover-grid/scroll-manager.ts +@frontend/src/components/queue-panel/queue-panel.ts +@scripts/profile.sh +@backend/profiling/profiling.go + + + + + + Task 1: Optimize scroll event handling and clean up queue panel scroll hack + + frontend/src/components/cover-grid/scroll-manager.ts + frontend/src/components/queue-panel/queue-panel.ts + + +**scroll-manager.ts — RAF-throttled scroll position saving:** + +The current scroll position save uses a 100ms debounce timer. This is suboptimal because: +1. During continuous scrolling, position is never saved (debounce resets on each scroll event) +2. When scrolling stops, there's a 100ms delay before the position is recorded +3. If the user navigates away during scrolling (before debounce fires), position is lost + +Replace with **requestAnimationFrame throttling**: save the position once per animation frame. This fires at most once per ~16ms (60fps), captures position during scrolling (not just after), and naturally aligns with the browser's paint cycle. + +Pattern: +```typescript +private scrollRAFId: number | null = null; + +private onScroll = () => { + if (this.scrollRAFId !== null) return; + this.scrollRAFId = requestAnimationFrame(() => { + this.scrollRAFId = null; + // save current scroll position + this.saveScrollPosition(); + }); +}; +``` + +Find the existing debounced scroll handler in scroll-manager.ts and replace it with this RAF-throttled version. Make sure to: +- Cancel any pending RAF in `destroy()` or cleanup method +- Keep the same `saveScrollPosition()` logic (storing to library store) + +**queue-panel.ts — Replace _correctScrollError monkey-patch:** + +The queue panel currently monkey-patches lit-virtualizer's internal `_correctScrollError` method to prevent it from fighting the native scrollbar during drag scrolling. This was documented as a fix for the "scroll bar not following" issue. + +Instead of monkey-patching an internal API (which could break on lit-virtualizer updates), use CSS `overflow-anchor: none` on the virtualizer's scroll container. This CSS property tells the browser NOT to automatically adjust scroll position when content changes above the viewport — which is the same thing `_correctScrollError` does but from the browser side. + +Add to the queue panel's lit-virtualizer CSS: +```css +lit-virtualizer { + overflow-anchor: none; +} +``` + +Then check if the monkey-patch can be removed. If `overflow-anchor: none` alone resolves the scrollbar desync, remove the monkey-patch code entirely. If the monkey-patch is still needed for a specific scenario (like the gutter click detection for native scrollbar drag), keep only the gutter detection part and remove the scroll error correction override. + +**Important:** Test the queue panel with a large queue (5000+ tracks) and verify: +1. Native scrollbar drag works smoothly (no jumping/fighting) +2. Keyboard navigation (arrow keys) doesn't cause scroll jumps +3. Auto-scroll to current track works +4. The queue panel scrolls smoothly when dragging tracks to reorder + +If `overflow-anchor: none` doesn't fully replace the monkey-patch, keep the monkey-patch but add a comment explaining WHY it's needed and what the CSS alone doesn't handle. + + + `cd frontend && npx vite build --mode development 2>&1 | tail -5` builds without errors. + Cover grid: scroll position saves continuously during scrolling (not just after stopping). + Queue panel: scrollbar drag on 5000+ item queue works without scroll fighting. + + Cover grid scroll position saves are RAF-throttled (once per frame). Queue panel scroll handling is cleaned up (overflow-anchor or documented monkey-patch). + + + + Task 2: Create performance profiling guide + docs/PROFILING.md + +Create a practical profiling guide at `docs/PROFILING.md` that documents how to diagnose performance issues in YellowJacket. This should be a concise, actionable reference — not a textbook. + +Structure: + +## 1. Backend Profiling (Go / pprof) + +**Setup:** `make dev` starts the app with pprof server on `:6060`. + +**Quick Start:** +- `./scripts/profile.sh` — interactive menu +- `./scripts/profile.sh cpu` — 30s CPU profile (flame graph in browser) +- `./scripts/profile.sh heap` — current memory usage +- `./scripts/profile.sh health` — goroutine count, heap, GC stats + +**When to use each profile type:** +| Profile | Use When | What It Shows | +|---------|----------|---------------| +| CPU | Something is slow | Time spent in each function (flame graph) | +| Heap | Memory growing | Current allocations by location | +| Allocs | GC pressure | Where allocations happen (even freed) | +| Goroutine | Hangs/deadlocks | All goroutines and their stack traces | +| Block | Lock contention | Where goroutines block on mutexes/channels | +| Mutex | Mutex bottleneck | Mutex contention hotspots | +| Trace | Scheduling issues | Timeline of goroutine scheduling, GC pauses, syscalls | + +**Reading flame graphs:** +- Wide bars = more time spent +- Look for unexpected width (functions taking more time than expected) +- Bottom of stack = entry points, top = leaf functions where time is actually spent +- Use the search box to find specific packages (e.g., "library", "queue", "database") + +**Common YellowJacket hotspots:** +- `database.GetAllTracks` — large library, check SQL query time +- `library.extractMetadata` — scan performance, check per-format timing in scan metrics +- `queue.SetQueue` — Phase 1/2 dedup, check with large queues +- `coverart.Generate*` — thumbnail generation, check per-tier timing + +## 2. Frontend Profiling (Chrome DevTools) + +Since YellowJacket uses Wails (WebView2/WebKit), you can use Chrome DevTools for frontend profiling. + +**Opening DevTools:** +- On Wails dev builds, press `Ctrl+Shift+I` (or right-click → Inspect) + +**Performance Panel (scrolling/rendering):** +1. Open Performance panel +2. Click Record +3. Perform the action (scroll, navigate, etc.) +4. Stop recording +5. Look at the Main thread timeline: + - Long yellow bars = JavaScript execution (too long = jank) + - Purple bars = rendering/layout + - Green bars = painting + - Grey bars = idle +6. Target: each frame should be <16ms for 60fps scrolling + +**Key metrics for scroll smoothness:** +- Frame time: Should be consistently <16ms +- Layout recalculation: Should not happen during scrolling (if it does, `contain` CSS isn't working) +- Paint: Should be minimal and composited (green bars should be thin) +- JS execution during scroll: Should be minimal — lit-virtualizer does most work, but renderItem callbacks add up + +**Memory Panel:** +1. Take heap snapshot before/after an action +2. Compare snapshots to find leaks +3. Look for growing arrays of detached DOM nodes (sign of view not cleaning up) + +**What to look for in YellowJacket:** +| Symptom | Likely Cause | Check | +|---------|-------------|-------| +| Scroll jank | Layout thrashing | Performance panel → check for "Layout" bars during scroll | +| Slow navigation | View recreation | Performance panel → look for long constructors after navigate | +| Memory growth | Listener leaks | Memory panel → compare snapshots, filter "Detached" | +| Slow initial load | Blocking JS | Performance panel → check DOMContentLoaded to first paint | + +## 3. Profiling Workflow for Specific Issues + +**"Scrolling feels janky":** +1. Open DevTools Performance panel +2. Record while scrolling the problematic view +3. Look at frame times — are any >16ms? +4. If JS is the bottleneck: check renderItem callback time +5. If Layout is the bottleneck: check if `contain` CSS is present +6. If Paint is the bottleneck: check if `will-change: transform` is on the scroll container + +**"Navigation is slow":** +1. Open DevTools Performance panel +2. Record while navigating between views +3. Look for long JS tasks between navigate event and first paint +4. Check if the view is being destroyed/recreated (look for constructor calls) +5. After Phase 14 view caching: navigation between cached views should show almost no activity + +**"Library operations feel slow":** +1. `./scripts/profile.sh cpu` — capture during the operation +2. Check the flame graph for the specific Go function +3. For database operations: check if SQL queries are optimal +4. For scan operations: check scan metrics (they're already logged) +5. `./scripts/profile.sh trace` — for detailed timing of goroutine scheduling + + + `test -f docs/PROFILING.md && echo "File exists"` outputs "File exists". + The file contains sections on Backend Profiling, Frontend Profiling, and Profiling Workflow. + + docs/PROFILING.md exists with practical guidance on using pprof, Chrome DevTools Performance panel, and specific diagnostic workflows for scrolling, navigation, and library operation performance issues. + + + + Task 3: Verify performance improvements + none + +Phase 14 performance optimizations across 4 plans: +- CSS containment + GPU layer promotion on all scroll containers (Plan 01) +- View caching navigation (no more innerHTML destruction) (Plan 02) +- Render closure elimination + store notification optimization (Plan 03) +- Scroll event handling cleanup + profiling guide (Plan 04) + +Verification steps: +1. Run `make dev` to start the app +2. **Test scrolling smoothness:** + - Open the track list view → scroll rapidly up and down → should feel smooth, no jank or stuttering + - Open the album grid → scroll rapidly → should be smooth, no blank areas appearing + - Open the queue panel → add 1000+ tracks → scroll rapidly → should be smooth + - Open artists view → scroll → smooth + - Open genres view → scroll → smooth +3. **Test navigation speed:** + - Click Tracks → Albums → Tracks rapidly → should feel instant (no flash of loading) + - Click Albums → Artists → Genres → Playlists → Settings → Tracks → each transition should be near-instant + - Navigate to an artist detail → back to artists → artists view should still have its scroll position +4. **Test that nothing broke:** + - Play a track from track list (double-click) + - Right-click → context menu works + - Drag tracks to queue + - Multi-select with Shift/Ctrl+click + - Search filters correctly + - Album dropdown (click album in grid → tracks show) + - Queue reorder via drag +5. **Read profiling guide:** + - Open `docs/PROFILING.md` + - Does it make sense? Any confusing parts? + - Try `./scripts/profile.sh health` — does it connect? + + User approves scrolling smoothness and navigation speed + User has verified that scrolling is smooth, navigation is instant, and no interactions are broken. Type "approved" or describe issues found. + + + + + +Full Phase 14 verification: +1. Scrolling is measurably smoother across all views +2. Navigation between views is near-instant (cached views) +3. No visual regressions (context menus, popups, dropdowns, drag-drop) +4. Build succeeds: `cd frontend && npx vite build --mode development` +5. Go backend builds: `go build -tags webkit2_41 ./...` +6. PROFILING.md provides actionable guidance + + + +- Cover grid scroll position saves use RAF throttling +- Queue panel scroll handling is cleaner (overflow-anchor or documented hack) +- docs/PROFILING.md exists with Backend, Frontend, and Workflow sections +- User approves scrolling smoothness in the human-verify checkpoint + + + +After completion, create `.planning/phases/14-performance-optimization/14-04-SUMMARY.md` +