From a2bb98679c6a3aabb277b711f44953c7c160fd73 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Tue, 19 May 2026 07:38:37 +0100 Subject: [PATCH] ML-168.4: perform async messages audit --- ...t-→-Parent-handle_info-Coverage-Phase-1.md | 196 ++++++++++++++++++ ...Component-→-Parent-handle_info-coverage.md | 72 ++++++- 2 files changed, 258 insertions(+), 10 deletions(-) create mode 100644 backlog/docs/audits/phase1-async-message-coverage/doc-24 - Audit-Report-LiveComponent-→-Parent-handle_info-Coverage-Phase-1.md diff --git a/backlog/docs/audits/phase1-async-message-coverage/doc-24 - Audit-Report-LiveComponent-→-Parent-handle_info-Coverage-Phase-1.md b/backlog/docs/audits/phase1-async-message-coverage/doc-24 - Audit-Report-LiveComponent-→-Parent-handle_info-Coverage-Phase-1.md new file mode 100644 index 00000000..81a29f8c --- /dev/null +++ b/backlog/docs/audits/phase1-async-message-coverage/doc-24 - Audit-Report-LiveComponent-→-Parent-handle_info-Coverage-Phase-1.md @@ -0,0 +1,196 @@ +--- +id: doc-24 +title: "Audit Report: LiveComponent → Parent handle_info Coverage (Phase 1)" +type: other +created_date: "2026-05-19 06:35" +tags: + - audit + - liveview + - handle_info + - components +--- + +# Phase 1 Async Message Audit: LiveComponent → Parent handle_info Coverage + +**Date:** 2026-05-19 +**Scope:** All `send(self(), ...)` call sites in LiveComponents matched against `handle_info` clauses in parent LiveViews. + +## Executive Summary + +**Result: PASS — No critical bugs found.** All 11 message-producing components have their messages properly handled by their parent LiveViews. The pre-flagged Release component concern was a false positive — the component safely skips sending when `on_release_loaded` is nil. + +--- + +## Complete Producer → Consumer Matrix + +### Components (lib/music_library_web/components/) + +#### 1. RecordForm (`MusicLibraryWeb.Components.RecordForm`) + +- **File:** `lib/music_library_web/components/record_form.ex` +- **Send pattern:** `send(self(), {__MODULE__, {:saved, record}})` (lines 564, 603) +- **Consumers:** + +| LiveView | File | Line | Message | Status | +| -------------------- | ----------------------------------------------------- | -------- | ------------------- | -------------------------- | +| CollectionLive.Show | `lib/music_library_web/live/collection_live/show.ex` | 338, 461 | `{:saved, record}` | ✅ Full match | +| CollectionLive.Index | `lib/music_library_web/live/collection_live/index.ex` | 177, 291 | `{:saved, _record}` | ✅ Match (ignores payload) | +| WishlistLive.Show | `lib/music_library_web/live/wishlist_live/show.ex` | 274, 355 | `{:saved, record}` | ✅ Full match | +| WishlistLive.Index | `lib/music_library_web/live/wishlist_live/index.ex` | 144, 212 | `{:saved, _record}` | ✅ Match (ignores payload) | + +#### 2. AddRecord (`MusicLibraryWeb.Components.AddRecord`) + +- **File:** `lib/music_library_web/components/add_record.ex` +- **Send pattern:** `send(self(), {__MODULE__, msg})` (line 574) +- **Messages:** `{:imported_single, record}` (line 452), `{:imported_async, count}` (line 435) +- **Consumers:** + +| LiveView | File | Line | Message | Status | +| -------------------- | ----------------------------------------------------- | ------------- | ------------- | ------ | +| CollectionLive.Index | `lib/music_library_web/live/collection_live/index.ex` | 193, 295, 299 | Both messages | ✅ | +| WishlistLive.Index | `lib/music_library_web/live/wishlist_live/index.ex` | 160, 216, 220 | Both messages | ✅ | + +#### 3. Chat (`MusicLibraryWeb.Components.Chat`) + +- **File:** `lib/music_library_web/components/chat.ex` +- **Send pattern:** `send(self(), {__MODULE__, :chats_changed})` (lines 344, 431) +- **Consumers:** + +| LiveView | File | Line | Message | Status | +| -------------------- | ----------------------------------------------------- | -------- | ---------------- | ------ | +| CollectionLive.Show | `lib/music_library_web/live/collection_live/show.ex` | 322, 469 | `:chats_changed` | ✅ | +| CollectionLive.Index | `lib/music_library_web/live/collection_live/index.ex` | 225, 303 | `:chats_changed` | ✅ | +| WishlistLive.Show | `lib/music_library_web/live/wishlist_live/show.ex` | 258, 362 | `:chats_changed` | ✅ | +| ArtistLive.Show | `lib/music_library_web/live/artist_live/show.ex` | 477, 630 | `:chats_changed` | ✅ | + +#### 4. Release (`MusicLibraryWeb.Components.Release`) + +- **File:** `lib/music_library_web/components/release.ex` +- **Send pattern:** `send(self(), {tag, release})` (line 82) — **non-MODULE pattern** +- **Consumers:** + +| LiveView | File | Line | on_release_loaded | handle_info | Status | +| ------------------------ | ---------------------------------------------------------- | ------ | ------------------- | ---------------------------------------------- | ----------- | +| ScrobbleLive.ReleaseShow | `lib/music_library_web/live/scrobble_live/release_show.ex` | 41, 68 | `{:release_loaded}` | `handle_info({:release_loaded, release}, ...)` | ✅ | +| CollectionLive.Show | `lib/music_library_web/live/collection_live/show.ex` | 304 | (not set = nil) | N/A — no message sent | ✅ Harmless | + +**CollectionLive.Show analysis:** The component's `notify_release_loaded/2` checks `socket.assigns[:on_release_loaded]` before sending. When nil (as in CollectionLive.Show), it returns `:ok` without calling `send/2`. This is intentional — CollectionLive.Show doesn't need to react to release loading. **No dead message, no missing handler.** + +#### 5. ScrobbleRulePicker (`MusicLibraryWeb.ScrobbleRulePicker`) + +- **File:** `lib/music_library_web/components/scrobble_rule_picker.ex` +- **Send pattern:** `send(self(), {__MODULE__, {:rule_created, rule}})` (line 191/212) +- **Consumers:** + +| LiveView | File | Line | Status | +| ------------------------- | ----------------------------------------------------------- | -------- | ------ | +| ScrobbledTracksLive.Index | `lib/music_library_web/live/scrobbled_tracks_live/index.ex` | 215, 283 | ✅ | +| StatsLive.Index | `lib/music_library_web/live/stats_live/index.ex` | 57, 167 | ✅ | + +Note: Not rendered in ScrobbleRulesLive.Index — that LiveView only uses ScrobbleRulesLive.Form. + +--- + +### Form Components (lib/music_library_web/live/\*/form.ex) + +#### 6. ArtistLive.Form (`MusicLibraryWeb.ArtistLive.Form`) + +- **File:** `lib/music_library_web/live/artist_live/form.ex` +- **Messages:** `{:saved, artist_info}` (lines 279, 319) +- **Consumer:** `artist_live/show.ex:493, 639` ✅ + +#### 7. RecordSetLive.Form (`MusicLibraryWeb.RecordSetLive.Form`) + +- **File:** `lib/music_library_web/live/record_set_live/form.ex` +- **Messages:** `{:updated, record_set}` (line 74), `{:created, record_set}` (line 87) +- **Consumers:** + +| LiveView | File | Lines | Messages handled | Status | +| ------------------- | ----------------------------------------------------- | ------------ | ----------------------- | --------------------------------------------------------------------------------------- | +| RecordSetLive.Show | `lib/music_library_web/live/record_set_live/show.ex` | 185, 227 | `:updated` only | ✅ Correct — Show only renders Form in `:edit` mode (line 180: `@live_action == :edit`) | +| RecordSetLive.Index | `lib/music_library_web/live/record_set_live/index.ex` | 94, 192, 199 | `:created` + `:updated` | ✅ | + +#### 8. RecordSetLive.RecordPicker (`MusicLibraryWeb.RecordSetLive.RecordPicker`) + +- **File:** `lib/music_library_web/live/record_set_live/record_picker.ex` +- **Message:** `{:added, updated_set}` (line 191) +- **Consumers:** + - `record_set_live/show.ex:201, 237` ✅ + - `record_set_live/index.ex:108, 206` ✅ + +#### 9. ScrobbleRulesLive.Form (`MusicLibraryWeb.ScrobbleRulesLive.Form`) + +- **File:** `lib/music_library_web/live/scrobble_rules_live/form.ex` +- **Messages:** `{:updated, scrobble_rule}` (line 93), `{:created, scrobble_rule}` (line 106) +- **Consumer:** `scrobble_rules_live/index.ex:249, 259` ✅ + +#### 10. ScrobbledTracksLive.Form (`MusicLibraryWeb.ScrobbledTracksLive.Form`) + +- **File:** `lib/music_library_web/live/scrobbled_tracks_live/form.ex` +- **Message:** `{:saved, track}` (line 109) +- **Consumer:** `scrobbled_tracks_live/index.ex:271` ✅ + +#### 11. OnlineStoreTemplateLive.Form (`MusicLibraryWeb.OnlineStoreTemplateLive.Form`) + +- **File:** `lib/music_library_web/live/online_store_template_live/form.ex` +- **Message:** `{:saved, template}` (lines 117, 130) +- **Consumer:** `online_store_template_live/index.ex:199` ✅ + +--- + +## Non-MODULE Pattern Audit (Acceptance Criterion #4) + +Three non-`{__MODULE__, _}` patterns were found: + +1. **Release component** (`release.ex:82`) — `send(self(), {tag, release})` where tag is a dynamic assign + - **Severity:** LOW — design note only + - **Status:** Correctly handled in ScrobbleLive.ReleaseShow; safely no-ops in CollectionLive.Show + - **Risk:** If a new consumer sets a non-atom tag (e.g., a string), pattern matching could fail silently. The `handle_info` clauses pattern-match on atoms (`:release_loaded`). + - **Recommendation:** Consider using `{__MODULE__, {:loaded, release}}` for type safety, but not required. + +2. **ScrobbleLive.Index** (`scrobble_live/index.ex:123,139`) — `send(self(), {:perform_search, query})` + - **Severity:** INFO — self-send within same LiveView, not a component→parent message + - **Status:** Has matching `handle_info({:perform_search, query}, ...)` at line 145 ✅ + +3. **ShowToast hook** (`hooks/show_toast.ex:8`) — `send(self(), {:put_toast, type, message})` + - **Severity:** N/A — standard LiveView `attach_hook` pattern + - **Status:** Intercepted by `maybe_put_toast/2` hook before reaching `handle_info` ✅ + +--- + +## handle_info Clauses Without Corresponding Component Sends + +These `handle_info` clauses exist in LiveViews but receive messages from non-component sources (PubSub broadcast, Phoenix channels): + +| LiveView | Message | Source | +| ------------------------- | ------------------------ | ---------------------------------------------- | +| CollectionLive.Index | `:records_index_changed` | PubSub from Records context | +| WishlistLive.Index | `:records_index_changed` | PubSub from Records context | +| CollectionLive.Show | `{:update, record}` | PubSub from Records context (`records.ex:126`) | +| WishlistLive.Show | `{:update, record}` | PubSub from Records context (`records.ex:126`) | +| ScrobbledTracksLive.Index | `%{track_count: _}` | PubSub from ScrobbleActivity context | +| StatsLive.Index | `%{track_count: _}` | PubSub from ScrobbleActivity context | +| MaintenanceLive.Index | `:update_job_counts` | Self-send or timer | + +These are all correctly handled and outside the scope of this component audit. + +--- + +## Coverage Statistics + +- **11/11 components verified** ✅ +- **14 send(self()) call sites** across all components +- **22 handle_info clauses** in parent LiveViews matching component messages +- **0 missing handlers** found +- **0 dead messages** found +- **3 non-MODULE patterns** — all verified safe + +--- + +## Recommendations + +1. **LOW:** The Release component's dynamic tag pattern (`send(self(), {tag, release})`) works correctly but could be made type-safe by using `{__MODULE__, {:loaded, release}}` with a static atom. This would prevent accidental non-atom tags and make message tracing easier. **Not blocking.** + +2. **INFO:** All `.ex` form components under `lib/music_library_web/live/*/form.ex` follow an identical `notify_parent/1` → `{__MODULE__, msg}` pattern. This is consistent and well-structured. **No changes needed.** + +3. **INFO:** The `RecordSetLive.Show` only handles `{:updated, ...}` from `RecordSetLive.Form` — this is correct because the Show LiveView only renders the Form in `:edit` mode. No missing handler. **No changes needed.** 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 index 72ffc144..a6fb4d5c 100644 --- 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 @@ -1,18 +1,16 @@ --- id: ML-169.4 title: "Phase 1: Async message audit (LiveComponent → Parent handle_info coverage)" -status: To Do +status: Done assignee: [] created_date: "2026-05-08 08:59" -updated_date: "2026-05-11 06:46" +updated_date: "2026-05-19 06:35" labels: - audit - ready 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 + - audits/phase1-async-message-coverage parent_task_id: ML-169 priority: medium --- @@ -31,9 +29,63 @@ Pre-flagged concern: the Release component renders in CollectionLive.Show with ` -- [ ] #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 +- [x] #1 Every send(self(), ...) call site in lib/music_library_web/components/ matched against handle_info in its consumer LiveViews +- [x] #2 Release component's {:release*loaded, *} message traced end-to-end in CollectionLive.Show context — either confirmed harmless or flagged as bug +- [x] #3 All 11 message-producing components verified: RecordForm, AddRecord, Chat, Release, ArtistLive.Form, RecordSetLive.Form, RecordSetLive.RecordPicker, ScrobbleRulesLive.Form, ScrobbledTracksLive.Form, OnlineStoreTemplateLive.Form, ScrobbleRulePicker +- [x] #4 Any send/2 calls using non-{**MODULE**, \_} patterns found and verified +- [x] #5 Findings report written as a Backlog.md document with file:line references, severity ratings, and fix recommendations + +## Implementation Notes + + + +## Audit Execution Notes + +### Method + +- Grepped all `send(self(), ...)` call sites in `lib/music_library_web/components/` and `lib/music_library_web/live/` (excluding test files) +- Grepped all `handle_info` clauses in `lib/music_library_web/live/` +- Cross-referenced every `send(self())` call site against parent LiveView `handle_info` clauses +- Verified component render locations to confirm the consumer LiveView is the correct parent +- Verified all module aliases match (e.g., AddRecord = MusicLibraryWeb.Components.AddRecord) + +### Pre-flagged concern: Release component in CollectionLive.Show + +**RESULT: HARMLESS** — The `notify_release_loaded/2` function in release.ex checks `socket.assigns[:on_release_loaded]`. When nil (CollectionLive.Show doesn't set it), the function returns `:ok` without calling `send/2`. No dead message, no missing handler. The Release component is rendered at line 304 in CollectionLive.Show with `show_print?={true}` and `timezone={@timezone}` but no `on_release_loaded` attribute, which is intentional. + +### Key findings + +- 11/11 components fully verified with 0 missing handlers +- 3 non-MODULE patterns found, all verified safe +- Release component's dynamic tag pattern works but could benefit from type-safe refactoring (low priority) +- RecordSetLive.Show correctly only handles `:updated` since it renders Form only in `:edit` mode + +Full report at @audits/phase1-async-message-coverage + + + +## Final Summary + + + +## Result: PASS — No missing handlers or dead messages found + +### Coverage + +- **14 `send(self(), ...)` call sites** across 11 components all matched to corresponding `handle_info` clauses +- **0 missing handlers**, **0 dead messages** +- **3 non-`{__MODULE__, _}` patterns** all verified safe: + 1. Release component dynamic tag — safe, nil-guarded + 2. ScrobbleLive.Index self-send — has matching handler + 3. ShowToast hook — standard LiveView attach_hook pattern + +### Pre-flagged concern resolved + +Release component in CollectionLive.Show: **HARMLESS**. The `notify_release_loaded/2` checks `socket.assigns[:on_release_loaded]` and skips sending when nil. CollectionLive.Show intentionally doesn't set `on_release_loaded`. + +### Full report + +Documented at `audits/phase1-async-message-coverage` with file:line references, severity ratings, and all 11 components verified. + +