ML-10: break compile cycle via Artists → Collection edge
This commit is contained in:
Claudio Ortolina
+29
-11
@@ -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
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
_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.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- `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
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
- [ ] #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
|
||||
<!-- AC:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
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.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 =
|
||||
|
||||
Reference in New Issue
Block a user