ML-169.8: tighten task scope

This commit is contained in:
Claudio Ortolina
2026-05-20 07:55:36 +01:00
parent f6792e08aa
commit d2def9d0d3
@@ -4,6 +4,7 @@ title: "Fix: StatsLive.Index 10 sync queries blocking TTFB"
status: To Do
assignee: []
created_date: "2026-05-19 11:48"
updated_date: "2026-05-20 06:54"
labels:
- perf
- fix
@@ -14,6 +15,8 @@ references:
Phase-4-Performance-Audit-Low-Hanging-Fruit-Findings.md
modified_files:
- lib/music_library_web/live/stats_live/index.ex
- lib/music_library/collection.ex
- test/music_library_web/live/stats_live/index_test.exs
parent_task_id: ML-169
priority: high
ordinal: 29000
@@ -23,41 +26,52 @@ ordinal: 29000
<!-- SECTION:DESCRIPTION:BEGIN -->
Fix performance issue where the StatsLive.Index mount callback executes 10 synchronous Ecto queries before returning, blocking the initial render (TTFB) on the home page.
Fix performance issue where `StatsLive.Index.mount/3` executes a block of synchronous Ecto queries before returning, blocking the initial render (TTFB) on the home page.
## Problem
The mount callback executes 10 synchronous queries before returning `{:ok, socket}`:
The query trace at `/tmp/queries.sql` confirms the Stats mount/helper block at lines 14-63. It executes 10 synchronous application queries before the page is ready:
1. `Collection.get_latest_record()` — ORDER BY purchased_at LIMIT 1
2. `Collection.count_records_by_artist(limit: 20)` — json_each + GROUP BY
3. `Collection.count_records_by_genre(limit: 20)` — json_each + GROUP BY + subquery exclusion
3. `Collection.count_records_by_genre(limit: 20)` — json_each + GROUP BY + excluded-genre subquery
4. `Collection.count_records_by_release_year(limit: 20)` — substr + GROUP BY
5. `Collection.get_records_on_this_day(date)` — strftime + ORDER BY
6. `Collection.count_records_by_format()` — GROUP BY format
7. `Collection.count_records_by_type()` — GROUP BY type
8. `Wishlist.count()`FTS search count
9. `ListeningStats.recent_activity(timezone)`4 correlated subqueries × 100 rows
8. `Wishlist.count()`count over the FTS-backed `SearchIndex` with `purchased_at IS NULL`
9. `ListeningStats.recent_activity(timezone)`3 top-level correlated scalar subqueries per returned row
10. `ListeningStats.scrobble_count()` — simple COUNT
The trace reported about 35.6ms total query time for these 10 queries on the captured run. `recent_activity` was not the slowest query in that trace, but it remains structurally risky because its correlated subqueries scale with the 100-row activity limit.
## Impact
10 blocking queries on the home page is the worst TTFB bottleneck in the app. The `assign_async` pattern is already in use for TopAlbums/TopArtists on the same page — apply it consistently.
The home page does non-critical data loading before the initial render. The existing async pattern used by `StatsLive.TopAlbums` and `StatsLive.TopArtists` should be extended, but the implementation must preserve LiveView stream conventions and use true scalar counters for the immediately visible badges.
## Files
- `lib/music_library_web/live/stats_live/index.ex:67-109` — mount callback
- `lib/music_library_web/live/stats_live/index.ex` — mount/render async orchestration
- `lib/music_library/collection.ex` — add a direct scalar collection count if format/type grouped stats move async
- `test/music_library_web/live/stats_live/index_test.exs` — update page tests to wait for async sections where needed
## Fix
Use `assign_async` to defer non-critical queries. The simple ones (collection_count, wishlist_count, scrobble_count for the counter badges) can stay synchronous since they're fast. Move the heavy ones to `assign_async`:
Keep only true scalar badge counters synchronous: collection count, wishlist count, and scrobble count. Do not derive `collection_count` from `count_records_by_format/0` if format stats move async; add `Collection.count/0` or an equivalent direct aggregate.
Move non-critical sections out of synchronous mount work:
- `count_records_by_artist` / `count_records_by_genre` / `count_records_by_release_year`
- `get_records_on_this_day`
- `recent_activity` (the most expensive — 4 correlated subqueries)
- `get_latest_record`
- `count_records_by_artist` / `count_records_by_genre` / `count_records_by_release_year`
- `count_records_by_format` / `count_records_by_type` if they are no longer needed to derive the badge count
- `get_records_on_this_day`
- `recent_activity`
Show skeleton/loading placeholders while async data loads.
Use `assign_async` for value assigns rendered through `<.async_result>` with loading and failed states. For scrobble activity, preserve the existing `@streams.recent_tracks` and `@streams.recent_albums` contract: use `start_async` + `handle_async` to stream both lists, or use `stream_async` carefully if it can support both streams without converting them to list assigns.
Remember that LiveView async tasks only start once the socket is connected. The disconnected render must have safe initial assigns/loading states and must not render helpers that expect plain lists or records against `%AsyncResult{}` values.
Show skeleton/loading placeholders for async sections and graceful failed states without flashing empty data containers.
<!-- SECTION:DESCRIPTION:END -->
@@ -65,9 +79,31 @@ Show skeleton/loading placeholders while async data loads.
<!-- AC:BEGIN -->
- [ ] #1 StatsLive.Index mount completes in under 200ms (sync queries only)
- [ ] #2 Charts and activity feed load with loading skeleton placeholders
- [ ] #3 Counter badges (collection, wishlist, scrobbles) show immediately
- [ ] #4 No visual flash of empty data containers
- [ ] #5 Assigns that are now async handle nil/failure states gracefully
- [ ] #1 StatsLive.Index initial mount runs only true scalar synchronous queries needed for immediately visible counters and returns under 200ms on the audited environment
- [ ] #2 Counter badges for collection, wishlist, and scrobbles render immediately from scalar counts
- [ ] #3 Latest purchase, format/type stats, collection charts, on-this-day records, and scrobble activity load asynchronously with skeleton/loading placeholders
- [ ] #4 Scrobble activity async loading preserves LiveView streams for recent tracks and recent albums rather than converting stream-backed UI to plain list assigns
- [ ] #5 Async sections render graceful failed or nil states and do not flash empty data containers
- [ ] #6 Tests cover immediate counter rendering and use render_async() for async-loaded Stats page content
- [ ] #7 A follow-up query trace or equivalent query-budget check confirms the synchronous Stats mount query count is reduced from the original 10-query block
<!-- AC:END -->
## Implementation Plan
<!-- SECTION:PLAN:BEGIN -->
1. Add or identify a direct scalar collection count so `collection_count` can stay synchronous without depending on grouped format stats.
2. Split Stats mount assigns into synchronous scalar counters plus explicit async loading state for each non-critical section.
3. Wrap non-stream async value sections in `<.async_result>` with loading and failed slots; update helpers so they receive plain loaded values only.
4. Load scrobble activity asynchronously while preserving `@streams.recent_tracks` and `@streams.recent_albums`, and keep `last_updated_uts` updates compatible with TopAlbums/TopArtists.
5. Update Stats LiveView tests to assert counters before async completion and async sections after `render_async()`.
6. Re-run the targeted tests and capture or inspect the Stats mount query block to verify the sync query budget dropped.
<!-- SECTION:PLAN:END -->
## Implementation Notes
<!-- SECTION:NOTES:BEGIN -->
Review correction from `/tmp/queries.sql`: the trace supports the 10 synchronous Stats queries claim, but not the statement that `recent_activity` was the slowest query in that run. It also shows `collection_count` currently depends on grouped format stats, so the task needs a direct scalar collection count before moving format/type grouped stats async.
<!-- SECTION:NOTES:END -->