Address code review feedback for multi-value genres

- Add duplicate removal in Beatport plugin using dict.fromkeys()
- Add test for conflicting genre/genres values when disabled
- Add test for handling genres=None when feature is disabled
- Add library tests for genres field with template functions
- Fix lastgenre test case to include count parameter
- Make lastgenre plugin use global genre_separator for consistency

All changes ensure plugins use consistent separators and handle
edge cases properly while maintaining backward compatibility.

Author: claude

beetsplug/beatport.py

@@ -235,6 +235,8 @@ def __init__(self, data: JSONDict):
             self.artists = [(x["id"], str(x["name"])) for x in data["artists"]]
         if "genres" in data:
             genre_list = [str(x["name"]) for x in data["genres"]]
+            # Remove duplicates while preserving order
+            genre_list = list(dict.fromkeys(genre_list))
             if beets.config["multi_value_genres"]:
                 self.genres = genre_list
             else:
@@ -319,6 +321,9 @@ def __init__(self, data: JSONDict):
         else:
             genre_list = []
 
+        # Remove duplicates while preserving order
+        genre_list = list(dict.fromkeys(genre_list))
+
         if beets.config["multi_value_genres"]:
             # New behavior: populate both genres list and joined string
             separator = beets.config["genre_separator"].get(str)

test/plugins/test_lastgenre.py

@@ -409,6 +409,7 @@ def test_sort_by_depth(self):
                 "separator": "\u0000",
                 "canonical": False,
                 "prefer_specific": False,
+                "count": 10,
             },
             "Blues",
             {
@@ -567,6 +568,14 @@ def mock_fetch_artist_genre(self, obj):
     # Configure
     plugin.config.set(config_values)
     plugin.setup()  # Loads default whitelist and canonicalization tree
+
+    # If test specifies a separator, set it as the global genre_separator
+    # (when multi_value_genres is enabled, plugins use the global separator)
+    if "separator" in config_values:
+        from beets import config
+
+        config["genre_separator"] = config_values["separator"]
+
     item = _common.item()
     item.genre = item_genre
 

test/test_autotag.py

@@ -562,6 +562,17 @@ def test_sync_genres_priority_list_over_string(self):
         assert item.genres == ["Rock", "Alternative"]
         assert item.genre == "Rock, Alternative"
 
+    def test_sync_genres_disabled_conflicting_values(self):
+        """When multi_value_genres is disabled with conflicting values."""
+        config["multi_value_genres"] = False
+
+        item = Item(genre="Jazz", genres=["Rock", "Alternative"])
+        correct_list_fields(item)
+
+        # genre string should take priority and be added to front of list
+        assert item.genre == "Jazz"
+        assert item.genres == ["Jazz", "Rock", "Alternative"]
+
     def test_sync_genres_none_values(self):
         """Handle None values in genre/genres fields without errors."""
         config["multi_value_genres"] = True
@@ -588,3 +599,14 @@ def test_sync_genres_disabled_empty_genres(self):
         # Should handle empty list without errors
         assert item.genres == []
         assert item.genre == ""
+
+    def test_sync_genres_disabled_none_genres(self):
+        """Handle disabled config with genres=None."""
+        config["multi_value_genres"] = False
+
+        item = Item(genres=None)
+        correct_list_fields(item)
+
+        # Should handle None without errors
+        assert item.genres == []
+        assert item.genre == ""

test/test_library.py

@@ -701,6 +701,35 @@ def test_first_different_sep(self):
         self._setf("%first{Alice / Bob / Eve,2,0, / , & }")
         self._assert_dest(b"/base/Alice & Bob")
 
+    def test_first_genres_list(self):
+        # Test that setting genres as a list syncs to genre field properly
+        # and works with %first template function
+        from beets import config
+
+        config["genre_separator"] = "; "
+        from beets.autotag import correct_list_fields
+
+        self.i.genres = ["Pop", "Rock", "Classical Crossover"]
+        correct_list_fields(self.i)
+        # genre field should now be synced
+        assert self.i.genre == "Pop; Rock; Classical Crossover"
+        # %first should work on the synced genre field
+        self._setf("%first{$genre}")
+        self._assert_dest(b"/base/Pop")
+
+    def test_first_genres_list_skip(self):
+        # Test that genres list works with %first skip parameter
+        from beets import config
+
+        config["genre_separator"] = "; "
+        from beets.autotag import correct_list_fields
+
+        self.i.genres = ["Pop", "Rock", "Classical Crossover"]
+        correct_list_fields(self.i)
+        # %first with skip should work on the synced genre field
+        self._setf("%first{$genre,1,2}")
+        self._assert_dest(b"/base/Classical Crossover")
+
 
 class DisambiguationTest(BeetsTestCase, PathFormattingMixin):
     def setUp(self):