From 2e38412a845617d2cdafa0cd856e06b631ad50b7 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Fri, 24 Apr 2026 10:49:47 +0100 Subject: [PATCH] =?UTF-8?q?ML-10:=20break=20compile=20cycle=20via=20Artist?= =?UTF-8?q?s=20=E2=86=92=20Collection=20edge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...→-Collection-→-Records-static-alias-cycle.md | 40 ++++++++++++++----- lib/music_library/artists.ex | 7 +--- .../live/artist_live/show.ex | 6 ++- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/backlog/tasks/ml-10 - Break-Records-→-Artists-→-Collection-→-Records-static-alias-cycle.md b/backlog/tasks/ml-10 - Break-Records-→-Artists-→-Collection-→-Records-static-alias-cycle.md index de437eb3..38889447 100644 --- a/backlog/tasks/ml-10 - Break-Records-→-Artists-→-Collection-→-Records-static-alias-cycle.md +++ b/backlog/tasks/ml-10 - Break-Records-→-Artists-→-Collection-→-Records-static-alias-cycle.md @@ -1,42 +1,50 @@ --- id: ML-10 title: Break Records → Artists → Collection → Records static alias cycle -status: To Do +status: Done assignee: [] created_date: '2026-04-20 08:49' +updated_date: '2026-04-24 09:49' labels: [] dependencies: [] references: - 'https://github.com/cloud8421/music_library/issues/173' priority: low +ordinal: 1000 --- ## Description + _GitHub: created 2026-04-16 · updated 2026-04-16_ ## Summary -A three-way compile-time alias cycle exists between the three context facades. `mix xref graph --format cycles` surfaces it. No runtime cycle (the Records → Artists edge is Oban-async). +A three-way compile-connected alias cycle exists between the three context facades. `mix xref graph --format cycles` surfaces it. No runtime cycle (the Records → Artists edge is Oban-async). ## Evidence ``` Records → Artists: lib/music_library/records.ex:10 (alias) - Artists.refresh_artist_info_async/1 records.ex:385 - Artists.prune_artist_info_async/1 records.ex:415 + Artists.refresh_artist_info_async/1 records.ex:387 + Artists.prune_artist_info_async/1 records.ex:417 Artists → Collection: lib/music_library/artists.ex:10 (alias) Collection.collected_artist_ids/0 artists.ex:29 (used inside get_similar_artists/1) -Collection → Records: lib/music_library/collection.ex:9 (alias) +Collection → Records: lib/music_library/collection.ex:9 (alias) + line 7 (import) Records.search_records/4, Records.search_records_count/2, Records.essential_fields/0 + import MusicLibrary.Records, only: [order_alphabetically: 0] ``` +The compile-time edge in the cycle is `Collection → Records` via `import ..., only: [order_alphabetically: 0]`. `order_alphabetically` is a **macro** (`records.ex:56`), so the import is unavoidably compile-time. `mix xref graph --format cycles --label compile` reports this as a cycle of 15 nodes (3 contexts + `Records.Similarity` + 11 workers), meaning any change to any of those 15 modules triggers recompilation of the entire cycle. + +Breaking any other edge in the triangle (including `Artists → Collection`) removes the cycle without needing to eliminate the macro import. + ## Relation to #112 -#112 fixed the earlier `Records ↔ Artists` bidirectional dependency by moving `collected_artist_ids/0` from `Artists` to `Collection`. That fix made the shape `Records → Artists → Collection → Records` — unidirectional in hops, but still a compile-time cycle. +#112 fixed the earlier `Records ↔ Artists` bidirectional dependency by moving `collected_artist_ids/0` from `Artists` to `Collection`. That fix made the shape `Records → Artists → Collection → Records` — unidirectional in hops, but still a compile-connected cycle. ## Fix @@ -53,14 +61,24 @@ collected = Collection.collected_artist_ids() similar = Artists.get_similar_artists(artist, collected) ``` -This removes the `Artists → Collection` edge without moving any behaviour, and the "which are in the collection?" filtering becomes a caller concern. +This removes the `Artists → Collection` edge without moving any behaviour. Within `lib/music_library/`, only `Artists` aliases `Collection` — everything else referencing `Collection` lives in `lib/music_library_web/` (outside the cycle), so this single change is sufficient to break the cycle. + ## Acceptance Criteria -- `mix xref graph --format cycles` reports no cycles involving these three modules +- `mix xref graph --format cycles --label compile` reports no cycle involving Records/Artists/Collection - `ArtistLive.Show` tests still pass - -- [ ] #1 `mix xref graph --format cycles` reports no cycles involving these three modules -- [ ] #2 `ArtistLive.Show` tests still pass +- [x] #1 `mix xref graph --format cycles --label compile` reports no cycle involving Records/Artists/Collection +- [x] #2 `ArtistLive.Show` tests still pass + +## Final Summary + + +Removed `alias MusicLibrary.Collection` from `MusicLibrary.Artists` and changed `get_similar_artists/1` to `get_similar_artists/2`, taking the collected artist id set as an argument instead of fetching it inline. The only caller (`ArtistLive.Show`) now computes the set via `Collection.collected_artist_ids()` and passes it in. + +`mix xref graph --format cycles --label compile` now reports **no cycles**. The 3-way cycle is gone, and the 15-module compile-connected cycle that previously forced recompilation cascades is broken. A 14-module runtime-only cycle (Records ↔ Artists via Oban-async edges + workers) remains as expected and does not trigger recompilation. + +Full test suite (836 tests including 10 in `ArtistLive.ShowTest`) passes. Format and Credo clean. + diff --git a/lib/music_library/artists.ex b/lib/music_library/artists.ex index f3dab1e4..9bc8d85d 100644 --- a/lib/music_library/artists.ex +++ b/lib/music_library/artists.ex @@ -7,7 +7,6 @@ defmodule MusicLibrary.Artists do alias MusicLibrary.Artists.ArtistInfo alias MusicLibrary.Assets - alias MusicLibrary.Collection alias MusicLibrary.Records.ArtistRecord alias MusicLibrary.{Repo, Worker} @@ -22,12 +21,10 @@ defmodule MusicLibrary.Artists do Repo.one!(q) end - @spec get_similar_artists(map()) :: {:ok, [map()]} | {:error, term()} - def get_similar_artists(artist) do + @spec get_similar_artists(map(), MapSet.t(String.t())) :: {:ok, [map()]} | {:error, term()} + def get_similar_artists(artist, collected_artist_ids) do case LastFm.get_similar_artists(artist.musicbrainz_id, artist.name) do {:ok, artists} -> - collected_artist_ids = Collection.collected_artist_ids() - {:ok, Enum.filter(artists, fn a -> MapSet.member?(collected_artist_ids, a.musicbrainz_id) diff --git a/lib/music_library_web/live/artist_live/show.ex b/lib/music_library_web/live/artist_live/show.ex index dedbbc04..5c845140 100644 --- a/lib/music_library_web/live/artist_live/show.ex +++ b/lib/music_library_web/live/artist_live/show.ex @@ -4,7 +4,7 @@ defmodule MusicLibraryWeb.ArtistLive.Show do import MusicLibraryWeb.RecordComponents, only: [record_grid: 1, country_label: 1, artist_image: 1] - alias MusicLibrary.{Artists, Chats, ListeningStats, Records} + alias MusicLibrary.{Artists, Chats, Collection, ListeningStats, Records} alias MusicLibrary.Artists.ArtistInfo alias MusicLibraryWeb.ArtistLive.Biography alias MusicLibraryWeb.ErrorMessages @@ -659,7 +659,9 @@ defmodule MusicLibraryWeb.ArtistLive.Show do |> assign(:country, ArtistInfo.country(artist_info)) |> maybe_assign_lastfm_artist_info(artist) |> assign_async(:similar_artists, fn -> - with {:ok, similar_artists} <- Artists.get_similar_artists(artist) do + collected_artist_ids = Collection.collected_artist_ids() + + with {:ok, similar_artists} <- Artists.get_similar_artists(artist, collected_artist_ids) do artist_image_hashes = Artists.get_image_hashes(similar_artists) similar_artists =