ML-169: add subtasks

This commit is contained in:
Claudio Ortolina
2026-05-08 10:57:06 +01:00
parent 07d2e3fee3
commit 1247b89892
4 changed files with 168 additions and 0 deletions
@@ -0,0 +1,42 @@
---
id: ML-169.1
title: "Phase 3: Concurrent state change safety audit"
status: To Do
assignee: []
created_date: "2026-05-08 08:59"
labels:
- audit
- concurrency
dependencies: []
references:
- lib/music_library/worker/
- lib/music_library/records.ex
- lib/music_library_web/components/record_form.ex
parent_task_id: ML-169
priority: medium
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Audit whether background Oban worker record mutations are safely reflected in LiveView state without data corruption or stale UI.
Key risks to investigate:
1. Worker-broadcast consistency: do all record-modifying workers call Records.notify_update/1?
2. Form edit + background update race: editing in RecordForm modal while worker updates record
3. Double-update handling: two workers updating same record simultaneously
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 Every record-modifying Oban worker audited for notify_update/1 call after success (RefreshCover, PopulateGenres, GenerateRecordEmbedding, RecordRefreshMusicBrainzData, ImportFromMusicbrainzRelease, ImportFromMusicbrainzReleaseGroup, all RecordAll\* batch workers)
- [ ] #2 Workers that do NOT modify records confirmed as correctly NOT broadcasting (PruneAssets, ApplyScrobbleRules, all ArtistRefresh\*, etc.)
- [ ] #3 Form edit + background update race evaluated: trace what happens when RecordForm holds stale @record assign while worker broadcasts {:update, record} to parent Show page
- [ ] #4 Double-update race evaluated: confirm both handle_info calls apply safely (workers touch different fields, second wins)
- [ ] #5 ArtistLive.Show confirmed as intentionally not handling {:update, record} (artist updates use ArtistLive.Form → {:saved, artist_info} path)
- [ ] #6 Findings report written as a Backlog.md document with file:line references, severity ratings, and fix recommendations
<!-- AC:END -->
@@ -0,0 +1,49 @@
---
id: ML-169.2
title: "Phase 4: Performance low-hanging fruit audit"
status: To Do
assignee: []
created_date: "2026-05-08 08:59"
labels:
- audit
- performance
dependencies: []
references:
- lib/music_library_web/live/stats_live/index.ex
- lib/music_library_web/live_helpers/index_actions.ex
- lib/music_library_web/live_helpers/record_actions.ex
- .agents/skills/query-reporter/SKILL.md
parent_task_id: ML-169
priority: medium
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Identify unnecessary DB queries, redundant full-stream reloads, server round trips that could be client-side, missing optimistic UI opportunities, and blocking mount work that could be deferred.
Use QueryReporter skill to capture SQL traces for key page loads as evidence.
Key pre-flagged areas:
- StatsLive.Index: 5 synchronous queries in mount (TTFB impact)
- Display toggle (grid/list): full FTS query when CSS-only suffices
- ScrobbleRulesLive + OnlineStoreTemplateLive: redundant stream_insert + immediate full reload
- ScrobbledTracksLive + StatsLive: full stream reset on every scrobble batch instead of incremental insert
- Search inputs: verify phx-debounce is set
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 SQL traces captured via QueryReporter for /collection, /collection/:id, /stats, /artists/:id
- [ ] #2 Stream reset audit complete: every stream(:name, data, reset: true) call site evaluated for incremental-update alternatives
- [ ] #3 Display toggle (grid/list) evaluated: confirmed whether CSS-only toggle can replace full DB reload
- [ ] #4 StatsLive.Index mount blocking work quantified: 5 queries identified, assign_async feasibility assessed
- [ ] #5 Search input debounce verified: phx-debounce attribute presence checked on all search forms
- [ ] #6 Optimistic UI opportunities identified for delete, form save, toggle operations
- [ ] #7 N+1 audit complete: handle_params, stream rendering, and LiveComponent Repo calls checked
- [ ] #8 Findings report written as a Backlog.md document with file:line references, severity ratings, and fix recommendations
<!-- AC:END -->
@@ -0,0 +1,39 @@
---
id: ML-169.3
title: "Phase 2: PubSub subscription lifecycle audit"
status: To Do
assignee: []
created_date: "2026-05-08 08:59"
labels:
- audit
- pubsub
dependencies: []
references:
- lib/music_library/records.ex
- lib/music_library/listening_stats.ex
- lib/music_library_web/live_helpers/record_actions.ex
parent_task_id: ML-169
priority: medium
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Audit all PubSub subscribe/unsubscribe pairs for correctness. Verify no stale subscriptions or double-subscription risks.
Pre-flagged concern: ListeningStats.subscribe() is called in mount by StatsLive.Index and ScrobbledTracksLive.Index but never unsubscribed. Phoenix.PubSub monitors PID for auto-cleanup, but double-subscription on LiveView reconnect is possible and needs verification.
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 Records.subscribe/1 and Records.unsubscribe/1 call sites audited — confirmed only called via manage_subscription/2 in Show LiveViews
- [ ] #2 manage_subscription/2 verified correct across all navigation paths (direct URL, browser back/forward, push_navigate, push_patch)
- [ ] #3 ListeningStats.subscribe() double-subscription risk on reconnect evaluated — either confirmed safe by PID monitoring or flagged with fix
- [ ] #4 Termination cleanup verified: Phoenix.PubSub auto-cleans on PID death confirmed; any manual unsubscribe gaps documented
- [ ] #5 Records.notify_update/1 broadcast sites audited against intended behavior (coordinates with Phase 3 worker audit)
- [ ] #6 Findings report written as a Backlog.md document with file:line references, severity ratings, and fix recommendations
<!-- AC:END -->
@@ -0,0 +1,38 @@
---
id: ML-169.4
title: "Phase 1: Async message audit (LiveComponent → Parent handle_info coverage)"
status: To Do
assignee: []
created_date: "2026-05-08 08:59"
labels:
- audit
- async-messages
dependencies: []
references:
- lib/music_library_web/components/release.ex
- lib/music_library_web/live/collection_live/show.ex
- lib/music_library_web/live/scrobble_live/release_show.ex
parent_task_id: ML-169
priority: medium
---
## Description
<!-- SECTION:DESCRIPTION:BEGIN -->
Audit every LiveComponent message producer against its parent LiveView's handle_info clauses. Identify any missing or dead handlers.
Pre-flagged concern: the Release component renders in CollectionLive.Show with `on_release_loaded={:release_loaded}`, but CollectionLive.Show has no `handle_info({:release_loaded, _})` clause. Needs investigation — is this a dead message or a missing handler?
<!-- SECTION:DESCRIPTION:END -->
## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #1 Every send(self(), ...) call site in lib/music_library_web/components/ matched against handle_info in its consumer LiveViews
- [ ] #2 Release component's {:release*loaded, *} message traced end-to-end in CollectionLive.Show context — either confirmed harmless or flagged as bug
- [ ] #3 All 11 message-producing components verified: RecordForm, AddRecord, Chat, Release, ArtistLive.Form, RecordSetLive.Form, RecordSetLive.RecordPicker, ScrobbleRulesLive.Form, ScrobbledTracksLive.Form, OnlineStoreTemplateLive.Form, ScrobbleRulePicker
- [ ] #4 Any send/2 calls using non-{**MODULE**, \_} patterns found and verified
- [ ] #5 Findings report written as a Backlog.md document with file:line references, severity ratings, and fix recommendations
<!-- AC:END -->