lastgenre: Use albumartists field to improve last.fm results

[JOJ0]

Often last.fm does not find artist genres for delimiter-separated artist names (eg. "Artist One, Artist Two") or where multiple artists are combined with "concatenation words" like "and" , "+", "featuring" and so on.

This fix gathers each artist's last.fm genre separately by using Beets' mutli-valued albumartists field to improve the likeliness of finding genres in the artist genre fetching stage.

Refactoring was done along the existing genre fetching helper functions (fetch_album_genre, fetch_track_genre, ...):

• last.fm can be asked for genre for these combinations of metadata:

  • albumartist/album
  • artist/track
  • artist

• Instead of passing Album or Item objects directly to these helpers., generalize them and pass the (string) metadata directly.

• Passing "what's to fetch" in the callers instead of hiding it in the methods also helps readability in _get_genre()

• And reduces the requirement at hand for another additional method (or adaptation) to support "multi-albumartist genre fetching"

• Documentation

• Changelog

• Tests

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[codecov]

Codecov Report

❌ Patch coverage is 27.77778% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.43%. Comparing base (f79c125) to head (138f77d).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/lastgenre/__init__.py	27.77%	12 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5981      +/-   ##
==========================================
- Coverage   67.46%   67.43%   -0.03%     
==========================================
  Files         136      136              
  Lines       18535    18541       +6     
  Branches     3130     3133       +3     
==========================================
- Hits        12504    12503       -1     
- Misses       5366     5373       +7     
  Partials      665      665              

Files with missing lines	Coverage Δ
beetsplug/lastgenre/__init__.py	69.81% <27.77%> (-1.93%)	⬇️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sourcery-ai]

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `beetsplug/lastgenre/__init__.py:431-437` </location>
<code_context>
-                new_genres = self.fetch_album_artist_genre(obj)
+                new_genres = self.fetch_artist_genre(obj.albumartist)
                 stage_label = "album artist"
+                if not new_genres:
+                    self._tunelog(
+                        'No album artist genre found for "{}", '
+                        "trying multi-valued field...",
+                        obj.albumartist,
+                    )
+                    for albumartist in obj.albumartists:
+                        self._tunelog(
+                            'Fetching artist genre for "{}"', albumartist
</code_context>

<issue_to_address>
**suggestion:** Potential duplication of genres when aggregating from multiple album artists.

Deduplicate the genre list before returning or further processing to avoid repeated entries.

```suggestion
                    for albumartist in obj.albumartists:
                        self._tunelog(
                            'Fetching artist genre for "{}"', albumartist
                        )
                        new_genres += self.fetch_artist_genre(albumartist)
                    # Deduplicate genres while preserving order
                    if new_genres:
                        new_genres = list(dict.fromkeys(new_genres))
                        stage_label = "multi-valued album artist"
```
</issue_to_address>

### Comment 2
<location> `beetsplug/lastgenre/__init__.py:441` </location>
<code_context>
             else:
                 # For "Various Artists", pick the most popular track genre.
                 item_genres = []
+                assert isinstance(obj, Album)  # Type narrowing for mypy
                 for item in obj.items():
                     item_genre = None
</code_context>

<issue_to_address>
**suggestion:** Use runtime type checking instead of assert for production code.

Consider replacing the assert with an explicit isinstance check and appropriate error handling, since asserts can be disabled in production environments.

```suggestion
                if not isinstance(obj, Album):
                    raise TypeError(f"Expected obj to be an Album, got {type(obj).__name__}")
```
</issue_to_address>

### Comment 3
<location> `test/plugins/test_lastgenre.py:555` </location>
<code_context>
         return mock_genres["album"]

-    def mock_fetch_artist_genre(self, obj):
+    def mock_fetch_artist_genre(self, artist):
         return mock_genres["artist"]

</code_context>

<issue_to_address>
**suggestion (testing):** No test coverage for multi-valued albumartists genre fetching.

Please add a test that creates an Album with multiple albumartists and verifies that genres are fetched for each and aggregated correctly.

Suggested implementation:

```python
    def mock_fetch_artist_genre(self, artist):

```

```python
    def test_album_with_multiple_albumartists_genre_aggregation(self):
        # Setup mock genres for two artists
        artist1 = "Artist One"
        artist2 = "Artist Two"
        genre_artist1 = ["rock", "indie"]
        genre_artist2 = ["pop", "electronic"]

        # Patch the genre fetcher to return different genres for each artist
        def genre_fetcher(artist):
            if artist == artist1:
                return genre_artist1
            elif artist == artist2:
                return genre_artist2
            return []

        # Assume Album and LastGenrePlugin are available in the test context
        album = Album(albumartist=[artist1, artist2], albumtitle="Test Album")
        plugin = LastGenrePlugin()
        plugin.fetch_artist_genre = genre_fetcher

        # Fetch genres for the album
        genres = []
        for artist in album.albumartist:
            genres.extend(plugin.fetch_artist_genre(artist))
        # Aggregate and deduplicate genres
        aggregated_genres = sorted(set(genres))

        # Assert that all genres from both artists are present
        expected_genres = sorted(set(genre_artist1 + genre_artist2))
        assert aggregated_genres == expected_genres, f"Expected {expected_genres}, got {aggregated_genres}"

```
</issue_to_address>

### Comment 4
<location> `test/plugins/test_lastgenre.py:552-553` </location>
<code_context>
         return mock_genres["track"]

-    def mock_fetch_album_genre(self, obj):
+    def mock_fetch_album_genre(self, albumartist, albumtitle):
         return mock_genres["album"]

</code_context>

<issue_to_address>
**suggestion (testing):** No test for fallback when album artist genre is missing.

Please add a test that simulates an empty result from the primary albumartist genre fetch and confirms the fallback logic is executed correctly.

```suggestion
    def mock_fetch_album_genre(self, albumartist, albumtitle):
        return mock_genres["album"]

def test_album_genre_fallback(monkeypatch):
    """Test fallback logic when albumartist genre is missing."""

    class DummyGenreFetcher:
        def fetch_album_genre(self, albumartist, albumtitle):
            # Simulate missing genre for albumartist
            return None

        def fetch_albumtitle_genre(self, albumtitle):
            # Simulate fallback genre for albumtitle
            return "FallbackGenre"

    fetcher = DummyGenreFetcher()

    # Patch the primary fetch to return None, and fallback to albumtitle genre
    monkeypatch.setattr(fetcher, "fetch_album_genre", lambda albumartist, albumtitle: None)
    monkeypatch.setattr(fetcher, "fetch_albumtitle_genre", lambda albumtitle: "FallbackGenre")

    # Simulate the logic that tries albumartist first, then falls back to albumtitle
    genre = fetcher.fetch_album_genre("MissingArtist", "TestAlbum")
    if genre is None:
        genre = fetcher.fetch_albumtitle_genre("TestAlbum")

    assert genre == "FallbackGenre"
```
</issue_to_address>

### Comment 5
<location> `test/plugins/test_lastgenre.py:549` </location>
<code_context>
     """Test _get_genre with various configurations."""

-    def mock_fetch_track_genre(self, obj=None):
+    def mock_fetch_track_genre(self, trackartist, tracktitle):
         return mock_genres["track"]

</code_context>

<issue_to_address>
**suggestion (testing):** Tests do not cover delimiter-separated or concatenated artist names.

Please add tests with delimiter-separated or concatenated artist names in the albumartists field to ensure genres are correctly fetched for each artist.
</issue_to_address>

### Comment 6
<location> `docs/changelog.rst:50` </location>
<code_context>
   endpoints. Previously, due to single-quotes (ie. string literal) in the SQL
   query, the query eg. `GET /item/values/albumartist` would return the literal
   "albumartist" instead of a list of unique album artists.
+- :doc:`plugins/lastgenre`: Fix the issue were last.fm does not give a result in
+  the artist genre stage because multi-artists "concatenation" words (like
+  "feat." "+", or "&" prevent exact matches. Using the albumartists list field
</code_context>

<issue_to_address>
**issue (typo):** Typo: 'were' should be 'where'.

Please change 'were' to 'where' in the documentation for correct grammar.

```suggestion
- :doc:`plugins/lastgenre`: Fix the issue where last.fm does not give a result in
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion: Use runtime type checking instead of assert for production code.

Consider replacing the assert with an explicit isinstance check and appropriate error handling, since asserts can be disabled in production environments.

Suggested change

	assert isinstance(obj, Album) # Type narrowing for mypy
	if not isinstance(obj, Album):
	raise TypeError(f"Expected obj to be an Album, got {type(obj).__name__}")

[Copilot]

Pull Request Overview

This PR enhances the lastgenre plugin to better handle multi-artist scenarios when fetching genres from last.fm. The main improvement is that when last.fm doesn't return results for concatenated artist names (like "Artist A & Artist B"), the plugin now falls back to querying each artist individually using the albumartists multi-valued field.

Key changes:

• Refactored genre fetching methods to accept string parameters instead of Item/Album objects for better flexibility
• Added fallback logic to query individual artists from the albumartists field when the concatenated albumartist yields no results
• Updated test mocks to match the new method signatures

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
beetsplug/lastgenre/init.py	Refactored fetch methods to accept strings; added multi-valued albumartists fallback logic for improved genre matching
test/plugins/test_lastgenre.py	Updated test mock signatures to match refactored fetch methods
docs/changelog.rst	Added changelog entry describing the bug fix (contains typos)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Typo: "were" should be "where"

Suggested change

	- :doc:`plugins/lastgenre`: Fix the issue were last.fm does not give a result in
	- :doc:`plugins/lastgenre`: Fix the issue where last.fm does not give a result in

[JOJ0]

This was quite tricky to fix. The original mypy error was:

$ mypy beetsplug/lastgenre/__init__.py --show-error-codes
beetsplug/lastgenre/__init__.py:443: error: "tuple[str, Any]" has no attribute "artist"  [attr-defined]
beetsplug/lastgenre/__init__.py:443: error: "tuple[str, Any]" has no attribute "title"  [attr-defined]
beetsplug/lastgenre/__init__.py:446: error: "tuple[str, Any]" has no attribute "artist"  [attr-defined]
Found 3 errors in 1 file (checked 1 source file)

With the assert I'm convincing mypy to consider obj being an Album object, otherwise it would have detected it as an [Iterable[tuple[str, Any]]] which LibModel.items() returns.

This seemed to be the most easy fix. I tried to fiddle around with changing types within models.py:

beets/beets/library/models.py

Line 37 in f79c125

class LibModel(dbcore.Model["Library"]):

but I'm out of my depth here. Some advice for a better fix or if that's a good enough one already please @semohr ?