From 1247b8989269ddba998ebb247aaedc60fadbb7a0 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Fri, 8 May 2026 10:57:06 +0100 Subject: [PATCH] ML-169: add subtasks --- ...-3-Concurrent-state-change-safety-audit.md | 42 ++++++++++++++++ ...e-4-Performance-low-hanging-fruit-audit.md | 49 +++++++++++++++++++ ...e-2-PubSub-subscription-lifecycle-audit.md | 39 +++++++++++++++ ...Component-→-Parent-handle_info-coverage.md | 38 ++++++++++++++ 4 files changed, 168 insertions(+) create mode 100644 backlog/tasks/ml-169.1 - Phase-3-Concurrent-state-change-safety-audit.md create mode 100644 backlog/tasks/ml-169.2 - Phase-4-Performance-low-hanging-fruit-audit.md create mode 100644 backlog/tasks/ml-169.3 - Phase-2-PubSub-subscription-lifecycle-audit.md create mode 100644 backlog/tasks/ml-169.4 - Phase-1-Async-message-audit-LiveComponent-→-Parent-handle_info-coverage.md diff --git a/backlog/tasks/ml-169.1 - Phase-3-Concurrent-state-change-safety-audit.md b/backlog/tasks/ml-169.1 - Phase-3-Concurrent-state-change-safety-audit.md new file mode 100644 index 00000000..f92a6087 --- /dev/null +++ b/backlog/tasks/ml-169.1 - Phase-3-Concurrent-state-change-safety-audit.md @@ -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 + + + +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 + + +## Acceptance Criteria + + + +- [ ] #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 + diff --git a/backlog/tasks/ml-169.2 - Phase-4-Performance-low-hanging-fruit-audit.md b/backlog/tasks/ml-169.2 - Phase-4-Performance-low-hanging-fruit-audit.md new file mode 100644 index 00000000..aee452ac --- /dev/null +++ b/backlog/tasks/ml-169.2 - Phase-4-Performance-low-hanging-fruit-audit.md @@ -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 + + + +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 + + +## Acceptance Criteria + + + +- [ ] #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 + diff --git a/backlog/tasks/ml-169.3 - Phase-2-PubSub-subscription-lifecycle-audit.md b/backlog/tasks/ml-169.3 - Phase-2-PubSub-subscription-lifecycle-audit.md new file mode 100644 index 00000000..2e30e78c --- /dev/null +++ b/backlog/tasks/ml-169.3 - Phase-2-PubSub-subscription-lifecycle-audit.md @@ -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 + + + +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. + + + +## Acceptance Criteria + + + +- [ ] #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 + diff --git a/backlog/tasks/ml-169.4 - Phase-1-Async-message-audit-LiveComponent-→-Parent-handle_info-coverage.md b/backlog/tasks/ml-169.4 - Phase-1-Async-message-audit-LiveComponent-→-Parent-handle_info-coverage.md new file mode 100644 index 00000000..9c2811f6 --- /dev/null +++ b/backlog/tasks/ml-169.4 - Phase-1-Async-message-audit-LiveComponent-→-Parent-handle_info-coverage.md @@ -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 + + + +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? + + + +## Acceptance Criteria + + + +- [ ] #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 +