From ec95c8df2523981c38a9f7782001f10409d620ce Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 23 Nov 2025 09:45:12 -0500 Subject: [PATCH 01/12] preserve the order in which queries were specified in the configuration --- beetsplug/smartplaylist.py | 50 ++++++------- docs/changelog.rst | 4 ++ test/plugins/test_smartplaylist.py | 110 ++++++++++++++++++++++++----- 3 files changed, 123 insertions(+), 41 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 8203ce4ef..c0404b7d6 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -19,8 +19,7 @@ from urllib.parse import quote from urllib.request import pathname2url from beets import ui -from beets.dbcore import OrQuery -from beets.dbcore.query import MultipleSort, ParsingError +from beets.dbcore.query import ParsingError from beets.library import Album, Item, parse_query_string from beets.plugins import BeetsPlugin from beets.plugins import send as send_event @@ -190,25 +189,12 @@ class SmartPlaylistPlugin(BeetsPlugin): elif len(qs) == 1: query_and_sort = parse_query_string(qs[0], model_cls) else: - # multiple queries and sorts - queries, sorts = zip( - *(parse_query_string(q, model_cls) for q in qs) + # multiple queries and sorts - preserve order + # Store tuple of (query, sort) tuples for hashability + queries_and_sorts = tuple( + parse_query_string(q, model_cls) for q in qs ) - query = OrQuery(queries) - final_sorts = [] - for s in sorts: - if s: - if isinstance(s, MultipleSort): - final_sorts += s.sorts - else: - final_sorts.append(s) - if not final_sorts: - sort = None - elif len(final_sorts) == 1: - (sort,) = final_sorts - else: - sort = MultipleSort(final_sorts) - query_and_sort = query, sort + query_and_sort = queries_and_sorts, None playlist_data += (query_and_sort,) @@ -221,10 +207,17 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists.add(playlist_data) def matches(self, model, query, album_query): - if album_query and isinstance(model, Album): + # Handle tuple/list of queries (multiple queries preserving order) + if isinstance(album_query, (list, tuple)) and isinstance(model, Album): + return any(q.match(model) for q, _ in album_query) + elif album_query and isinstance(model, Album): return album_query.match(model) - if query and isinstance(model, Item): + + if isinstance(query, (list, tuple)) and isinstance(model, Item): + return any(q.match(model) for q, _ in query) + elif query and isinstance(model, Item): return query.match(model) + return False def db_change(self, lib, model): @@ -270,9 +263,18 @@ class SmartPlaylistPlugin(BeetsPlugin): self._log.info("Creating playlist {}", name) items = [] - if query: + # Handle tuple/list of queries (preserves order) + if isinstance(query, (list, tuple)): + for q, sort in query: + items.extend(lib.items(q, sort)) + elif query: items.extend(lib.items(query, q_sort)) - if album_query: + + if isinstance(album_query, (list, tuple)): + for q, sort in album_query: + for album in lib.albums(q, sort): + items.extend(album.items()) + elif album_query: for album in lib.albums(album_query, a_q_sort): items.extend(album.items()) diff --git a/docs/changelog.rst b/docs/changelog.rst index 366af9ff0..af9012a23 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,10 @@ New features: Bug fixes: +- :doc:`/plugins/smartplaylist`: Fixed an issue where multiple queries in a + playlist configuration were not preserving their order, causing items to + appear in database order rather than the order specified in the config. + :bug:`6183` - When hardlinking from a symlink (e.g. importing a symlink with hardlinking enabled), dereference the symlink then hardlink, rather than creating a new (potentially broken) symlink :bug:`5676` diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index d3569d836..a16313b28 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -22,7 +22,6 @@ from unittest.mock import MagicMock, Mock, PropertyMock import pytest from beets import config -from beets.dbcore import OrQuery from beets.dbcore.query import FixedFieldSort, MultipleSort, NullSort from beets.library import Album, Item, parse_query_string from beets.test.helper import BeetsTestCase, PluginTestCase @@ -54,16 +53,15 @@ class SmartPlaylistTest(BeetsTestCase): foo_foo = parse_query_string("FOO foo", Item) baz_baz = parse_query_string("BAZ baz", Item) baz_baz2 = parse_query_string("BAZ baz", Album) - bar_bar = OrQuery( - ( - parse_query_string("BAR bar1", Album)[0], - parse_query_string("BAR bar2", Album)[0], - ) - ) + # Multiple queries are now stored as a tuple of (query, sort) tuples + bar_queries = tuple([ + parse_query_string("BAR bar1", Album), + parse_query_string("BAR bar2", Album), + ]) assert spl._unmatched_playlists == { ("foo", foo_foo, (None, None)), ("baz", baz_baz, baz_baz2), - ("bar", (None, None), (bar_bar, None)), + ("bar", (None, None), (bar_queries, None)), } def test_build_queries_with_sorts(self): @@ -86,19 +84,28 @@ class SmartPlaylistTest(BeetsTestCase): ) spl.build_queries() - sorts = {name: sort for name, (_, sort), _ in spl._unmatched_playlists} + + # Multiple queries now return a tuple of (query, sort) tuples, not combined + sorts = {} + for name, (query_data, sort), _ in spl._unmatched_playlists: + if isinstance(query_data, tuple): + # Tuple of queries - each has its own sort + sorts[name] = [s for _, s in query_data] + else: + sorts[name] = sort sort = FixedFieldSort # short cut since we're only dealing with this assert sorts["no_sort"] == NullSort() assert sorts["one_sort"] == sort("year") - assert sorts["only_empty_sorts"] is None - assert sorts["one_non_empty_sort"] == sort("year") - assert sorts["multiple_sorts"] == MultipleSort( - [sort("year"), sort("genre", False)] - ) - assert sorts["mixed"] == MultipleSort( - [sort("year"), sort("genre"), sort("id", False)] - ) + # Multiple queries store individual sorts in the tuple + assert sorts["only_empty_sorts"] == [NullSort(), NullSort()] + assert sorts["one_non_empty_sort"] == [sort("year"), NullSort()] + assert sorts["multiple_sorts"] == [sort("year"), sort("genre", False)] + assert sorts["mixed"] == [ + sort("year"), + NullSort(), + MultipleSort([sort("genre"), sort("id", False)]), + ] def test_matches(self): spl = SmartPlaylistPlugin() @@ -122,6 +129,15 @@ class SmartPlaylistTest(BeetsTestCase): assert spl.matches(i, query, a_query) assert spl.matches(a, query, a_query) + # Test with list of queries + q1 = Mock() + q1.match.return_value = False + q2 = Mock() + q2.match.side_effect = {i: True}.__getitem__ + queries_list = [(q1, None), (q2, None)] + assert spl.matches(i, queries_list, None) + assert not spl.matches(a, queries_list, None) + def test_db_changes(self): spl = SmartPlaylistPlugin() @@ -327,6 +343,66 @@ class SmartPlaylistTest(BeetsTestCase): assert content == b"http://beets:8337/item/3/file\n" + def test_playlist_update_multiple_queries_preserve_order(self): + """Test that multiple queries preserve their order in the playlist.""" + spl = SmartPlaylistPlugin() + + # Create three mock items + i1 = Mock(path=b"/item1.mp3") + i1.evaluate_template.return_value = "ordered.m3u" + i2 = Mock(path=b"/item2.mp3") + i2.evaluate_template.return_value = "ordered.m3u" + i3 = Mock(path=b"/item3.mp3") + i3.evaluate_template.return_value = "ordered.m3u" + + lib = Mock() + lib.replacements = CHAR_REPLACE + lib.albums.return_value = [] + + # Set up lib.items to return different items for different queries + q1 = Mock() + q2 = Mock() + q3 = Mock() + + def items_side_effect(query, sort): + if query == q1: + return [i1] + elif query == q2: + return [i2] + elif query == q3: + return [i3] + return [] + + lib.items.side_effect = items_side_effect + + # Create playlist with multiple queries (stored as tuple) + queries_and_sorts = ((q1, None), (q2, None), (q3, None)) + pl = "ordered.m3u", (queries_and_sorts, None), (None, None) + spl._matched_playlists = [pl] + + dir = mkdtemp() + config["smartplaylist"]["relative_to"] = False + config["smartplaylist"]["playlist_dir"] = str(dir) + try: + spl.update_playlists(lib) + except Exception: + rmtree(syspath(dir)) + raise + + # Verify that lib.items was called with queries in the correct order + assert lib.items.call_count == 3 + lib.items.assert_any_call(q1, None) + lib.items.assert_any_call(q2, None) + lib.items.assert_any_call(q3, None) + + m3u_filepath = Path(dir, "ordered.m3u") + assert m3u_filepath.exists() + content = m3u_filepath.read_bytes() + rmtree(syspath(dir)) + + # Items should be in order: i1, i2, i3 + assert content == b"/item1.mp3\n/item2.mp3\n/item3.mp3\n" + class SmartPlaylistCLITest(PluginTestCase): plugin = "smartplaylist" From 0511c4f2021e1962c82e1a180b5342a3a22be6fe Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 23 Nov 2025 09:50:53 -0500 Subject: [PATCH 02/12] cleanup --- beetsplug/smartplaylist.py | 21 ++++++++++++--------- test/plugins/test_smartplaylist.py | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index c0404b7d6..392c635f0 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -159,7 +159,7 @@ class SmartPlaylistPlugin(BeetsPlugin): """ Instantiate queries for the playlists. - Each playlist has 2 queries: one or items one for albums, each with a + Each playlist has 2 queries: one for items, one for albums, each with a sort. We must also remember its name. _unmatched_playlists is a set of tuples (name, (q, q_sort), (album_q, album_q_sort)). @@ -168,7 +168,7 @@ class SmartPlaylistPlugin(BeetsPlugin): More precisely - it will be NullSort when a playlist query ('query' or 'album_query') is a single item or a list with 1 element - - it will be None when there are multiple items i a query + - it will be None when there are multiple items in a query """ self._unmatched_playlists = set() self._matched_playlists = set() @@ -207,16 +207,19 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists.add(playlist_data) def matches(self, model, query, album_query): - # Handle tuple/list of queries (multiple queries preserving order) - if isinstance(album_query, (list, tuple)) and isinstance(model, Album): - return any(q.match(model) for q, _ in album_query) - elif album_query and isinstance(model, Album): + # Handle single query object for Album + if album_query and not isinstance(album_query, (list, tuple)) and isinstance(model, Album): return album_query.match(model) + # Handle tuple/list of queries for Album + elif isinstance(album_query, (list, tuple)) and isinstance(model, Album): + return any(q.match(model) for q, _ in album_query) - if isinstance(query, (list, tuple)) and isinstance(model, Item): - return any(q.match(model) for q, _ in query) - elif query and isinstance(model, Item): + # Handle single query object for Item + if query and not isinstance(query, (list, tuple)) and isinstance(model, Item): return query.match(model) + # Handle tuple/list of queries for Item + elif isinstance(query, (list, tuple)) and isinstance(model, Item): + return any(q.match(model) for q, _ in query) return False diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index a16313b28..79055986f 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -98,7 +98,7 @@ class SmartPlaylistTest(BeetsTestCase): assert sorts["no_sort"] == NullSort() assert sorts["one_sort"] == sort("year") # Multiple queries store individual sorts in the tuple - assert sorts["only_empty_sorts"] == [NullSort(), NullSort()] + assert all(isinstance(x, NullSort) for x in sorts["only_empty_sorts"]) assert sorts["one_non_empty_sort"] == [sort("year"), NullSort()] assert sorts["multiple_sorts"] == [sort("year"), sort("genre", False)] assert sorts["mixed"] == [ From f00bf83f05c83a72f8cbea83ec59c4a46dff1918 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 23 Nov 2025 09:52:26 -0500 Subject: [PATCH 03/12] lint --- beetsplug/smartplaylist.py | 16 +++++++++++++--- test/plugins/test_smartplaylist.py | 10 ++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 392c635f0..47d7414f4 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -208,14 +208,24 @@ class SmartPlaylistPlugin(BeetsPlugin): def matches(self, model, query, album_query): # Handle single query object for Album - if album_query and not isinstance(album_query, (list, tuple)) and isinstance(model, Album): + if ( + album_query + and not isinstance(album_query, (list, tuple)) + and isinstance(model, Album) + ): return album_query.match(model) # Handle tuple/list of queries for Album - elif isinstance(album_query, (list, tuple)) and isinstance(model, Album): + elif isinstance(album_query, (list, tuple)) and isinstance( + model, Album + ): return any(q.match(model) for q, _ in album_query) # Handle single query object for Item - if query and not isinstance(query, (list, tuple)) and isinstance(model, Item): + if ( + query + and not isinstance(query, (list, tuple)) + and isinstance(model, Item) + ): return query.match(model) # Handle tuple/list of queries for Item elif isinstance(query, (list, tuple)) and isinstance(model, Item): diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index 79055986f..1b994c094 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -54,10 +54,12 @@ class SmartPlaylistTest(BeetsTestCase): baz_baz = parse_query_string("BAZ baz", Item) baz_baz2 = parse_query_string("BAZ baz", Album) # Multiple queries are now stored as a tuple of (query, sort) tuples - bar_queries = tuple([ - parse_query_string("BAR bar1", Album), - parse_query_string("BAR bar2", Album), - ]) + bar_queries = tuple( + [ + parse_query_string("BAR bar1", Album), + parse_query_string("BAR bar2", Album), + ] + ) assert spl._unmatched_playlists == { ("foo", foo_foo, (None, None)), ("baz", baz_baz, baz_baz2), From 71f4cc181435157cacae4cc5f797c8df7eb534ab Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 23 Nov 2025 09:59:34 -0500 Subject: [PATCH 04/12] Remove duplicate tracks --- beetsplug/smartplaylist.py | 14 ++++++-- test/plugins/test_smartplaylist.py | 58 ++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 47d7414f4..5fbc4785b 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -277,16 +277,26 @@ class SmartPlaylistPlugin(BeetsPlugin): items = [] # Handle tuple/list of queries (preserves order) + # Track seen items to avoid duplicates when an item matches + # multiple queries + seen_ids = set() + if isinstance(query, (list, tuple)): for q, sort in query: - items.extend(lib.items(q, sort)) + for item in lib.items(q, sort): + if item.id not in seen_ids: + items.append(item) + seen_ids.add(item.id) elif query: items.extend(lib.items(query, q_sort)) if isinstance(album_query, (list, tuple)): for q, sort in album_query: for album in lib.albums(q, sort): - items.extend(album.items()) + for item in album.items(): + if item.id not in seen_ids: + items.append(item) + seen_ids.add(item.id) elif album_query: for album in lib.albums(album_query, a_q_sort): items.extend(album.items()) diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index 1b994c094..ad9f056cc 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -350,11 +350,11 @@ class SmartPlaylistTest(BeetsTestCase): spl = SmartPlaylistPlugin() # Create three mock items - i1 = Mock(path=b"/item1.mp3") + i1 = Mock(path=b"/item1.mp3", id=1) i1.evaluate_template.return_value = "ordered.m3u" - i2 = Mock(path=b"/item2.mp3") + i2 = Mock(path=b"/item2.mp3", id=2) i2.evaluate_template.return_value = "ordered.m3u" - i3 = Mock(path=b"/item3.mp3") + i3 = Mock(path=b"/item3.mp3", id=3) i3.evaluate_template.return_value = "ordered.m3u" lib = Mock() @@ -405,6 +405,58 @@ class SmartPlaylistTest(BeetsTestCase): # Items should be in order: i1, i2, i3 assert content == b"/item1.mp3\n/item2.mp3\n/item3.mp3\n" + def test_playlist_update_multiple_queries_no_duplicates(self): + """Test that items matching multiple queries only appear once.""" + spl = SmartPlaylistPlugin() + + # Create two mock items + i1 = Mock(path=b"/item1.mp3", id=1) + i1.evaluate_template.return_value = "dedup.m3u" + i2 = Mock(path=b"/item2.mp3", id=2) + i2.evaluate_template.return_value = "dedup.m3u" + + lib = Mock() + lib.replacements = CHAR_REPLACE + lib.albums.return_value = [] + + # Set up lib.items so both queries return overlapping items + q1 = Mock() + q2 = Mock() + + def items_side_effect(query, sort): + if query == q1: + return [i1, i2] # Both items match q1 + elif query == q2: + return [i2] # Only i2 matches q2 + return [] + + lib.items.side_effect = items_side_effect + + # Create playlist with multiple queries (stored as tuple) + queries_and_sorts = ((q1, None), (q2, None)) + pl = "dedup.m3u", (queries_and_sorts, None), (None, None) + spl._matched_playlists = [pl] + + dir = mkdtemp() + config["smartplaylist"]["relative_to"] = False + config["smartplaylist"]["playlist_dir"] = str(dir) + try: + spl.update_playlists(lib) + except Exception: + rmtree(syspath(dir)) + raise + + m3u_filepath = Path(dir, "dedup.m3u") + assert m3u_filepath.exists() + content = m3u_filepath.read_bytes() + rmtree(syspath(dir)) + + # i2 should only appear once even though it matches both queries + # Order should be: i1 (from q1), i2 (from q1, skipped in q2) + assert content == b"/item1.mp3\n/item2.mp3\n" + # Verify i2 is not duplicated + assert content.count(b"/item2.mp3") == 1 + class SmartPlaylistCLITest(PluginTestCase): plugin = "smartplaylist" From 67d6e7dd62d3a7b3ad17bf3e5863915d01d904ed Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 26 Nov 2025 13:27:44 -0500 Subject: [PATCH 05/12] feat(types): Add type hints to smartplaylist.py --- beetsplug/smartplaylist.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 5fbc4785b..40b6d9f80 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -15,6 +15,7 @@ """Generates smart playlists based on beets queries.""" import os +from typing import Any, List, Optional, Set, Tuple, Union from urllib.parse import quote from urllib.request import pathname2url @@ -35,7 +36,7 @@ from beets.util import ( class SmartPlaylistPlugin(BeetsPlugin): - def __init__(self): + def __init__(self) -> None: super().__init__() self.config.add( { @@ -60,7 +61,7 @@ class SmartPlaylistPlugin(BeetsPlugin): if self.config["auto"]: self.register_listener("database_change", self.db_change) - def commands(self): + def commands(self) -> List[ui.Subcommand]: spl_update = ui.Subcommand( "splupdate", help="update the smart playlists. Playlist names may be " @@ -123,7 +124,7 @@ class SmartPlaylistPlugin(BeetsPlugin): spl_update.func = self.update_cmd return [spl_update] - def update_cmd(self, lib, opts, args): + def update_cmd(self, lib: Any, opts: Any, args: List[str]) -> None: self.build_queries() if args: args = set(args) @@ -150,12 +151,12 @@ class SmartPlaylistPlugin(BeetsPlugin): self.__apply_opts_to_config(opts) self.update_playlists(lib, opts.pretend) - def __apply_opts_to_config(self, opts): + def __apply_opts_to_config(self, opts: Any) -> None: for k, v in opts.__dict__.items(): if v is not None and k in self.config: self.config[k] = v - def build_queries(self): + def build_queries(self) -> None: """ Instantiate queries for the playlists. @@ -206,7 +207,7 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists.add(playlist_data) - def matches(self, model, query, album_query): + def matches(self, model: Union[Item, Album], query: Any, album_query: Any) -> bool: # Handle single query object for Album if ( album_query @@ -233,7 +234,7 @@ class SmartPlaylistPlugin(BeetsPlugin): return False - def db_change(self, lib, model): + def db_change(self, lib: Any, model: Union[Item, Album]) -> None: if self._unmatched_playlists is None: self.build_queries() @@ -246,7 +247,7 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists -= self._matched_playlists - def update_playlists(self, lib, pretend=False): + def update_playlists(self, lib: Any, pretend: bool = False) -> None: if pretend: self._log.info( "Showing query results for {} smart playlists...", @@ -266,7 +267,7 @@ class SmartPlaylistPlugin(BeetsPlugin): relative_to = normpath(relative_to) # Maps playlist filenames to lists of track filenames. - m3us = {} + m3us: "dict[str, list[PlaylistItem]]" = {} for playlist in self._matched_playlists: name, (query, q_sort), (album_query, a_q_sort) = playlist @@ -373,6 +374,6 @@ class SmartPlaylistPlugin(BeetsPlugin): class PlaylistItem: - def __init__(self, item, uri): + def __init__(self, item: Item, uri: bytes) -> None: self.item = item self.uri = uri From 028401ac286a82754115adf19a844208abec2d41 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 26 Nov 2025 13:33:07 -0500 Subject: [PATCH 06/12] lint --- beetsplug/smartplaylist.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 40b6d9f80..856e575e8 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -207,7 +207,9 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists.add(playlist_data) - def matches(self, model: Union[Item, Album], query: Any, album_query: Any) -> bool: + def matches( + self, model: Union[Item, Album], query: Any, album_query: Any + ) -> bool: # Handle single query object for Album if ( album_query From b9de8f9aabac0fd7713555b915def704f1752985 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 26 Nov 2025 13:39:02 -0500 Subject: [PATCH 07/12] Remove duplication in matches method --- beetsplug/smartplaylist.py | 49 +++++++++++++++----------------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 856e575e8..3a9f630ed 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -14,8 +14,10 @@ """Generates smart playlists based on beets queries.""" +from __future__ import annotations + import os -from typing import Any, List, Optional, Set, Tuple, Union +from typing import Any from urllib.parse import quote from urllib.request import pathname2url @@ -61,7 +63,7 @@ class SmartPlaylistPlugin(BeetsPlugin): if self.config["auto"]: self.register_listener("database_change", self.db_change) - def commands(self) -> List[ui.Subcommand]: + def commands(self) -> list[ui.Subcommand]: spl_update = ui.Subcommand( "splupdate", help="update the smart playlists. Playlist names may be " @@ -124,7 +126,7 @@ class SmartPlaylistPlugin(BeetsPlugin): spl_update.func = self.update_cmd return [spl_update] - def update_cmd(self, lib: Any, opts: Any, args: List[str]) -> None: + def update_cmd(self, lib: Any, opts: Any, args: list[str]) -> None: self.build_queries() if args: args = set(args) @@ -207,36 +209,23 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists.add(playlist_data) - def matches( - self, model: Union[Item, Album], query: Any, album_query: Any - ) -> bool: - # Handle single query object for Album - if ( - album_query - and not isinstance(album_query, (list, tuple)) - and isinstance(model, Album) - ): - return album_query.match(model) - # Handle tuple/list of queries for Album - elif isinstance(album_query, (list, tuple)) and isinstance( - model, Album - ): - return any(q.match(model) for q, _ in album_query) - - # Handle single query object for Item - if ( - query - and not isinstance(query, (list, tuple)) - and isinstance(model, Item) - ): - return query.match(model) - # Handle tuple/list of queries for Item - elif isinstance(query, (list, tuple)) and isinstance(model, Item): + def _matches_query(self, model: Item | Album, query: Any) -> bool: + if not query: + return False + if isinstance(query, (list, tuple)): return any(q.match(model) for q, _ in query) + return query.match(model) + def matches( + self, model: Item | Album, query: Any, album_query: Any + ) -> bool: + if isinstance(model, Album): + return self._matches_query(model, album_query) + if isinstance(model, Item): + return self._matches_query(model, query) return False - def db_change(self, lib: Any, model: Union[Item, Album]) -> None: + def db_change(self, lib: Any, model: Item | Album) -> None: if self._unmatched_playlists is None: self.build_queries() @@ -269,7 +258,7 @@ class SmartPlaylistPlugin(BeetsPlugin): relative_to = normpath(relative_to) # Maps playlist filenames to lists of track filenames. - m3us: "dict[str, list[PlaylistItem]]" = {} + m3us: dict[str, list[PlaylistItem]] = {} for playlist in self._matched_playlists: name, (query, q_sort), (album_query, a_q_sort) = playlist From 002a051d06686c8e3abd2304aa9d1d354625c174 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 26 Nov 2025 13:44:29 -0500 Subject: [PATCH 08/12] fix(smartplaylist): Resolve mypy type errors and update tests --- beetsplug/smartplaylist.py | 15 ++++++++------- test/plugins/test_smartplaylist.py | 16 ++++++++-------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 3a9f630ed..82739a995 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -57,8 +57,8 @@ class SmartPlaylistPlugin(BeetsPlugin): ) self.config["prefix"].redact = True # May contain username/password. - self._matched_playlists = None - self._unmatched_playlists = None + self._matched_playlists: set[tuple[Any, Any, Any]] = set() + self._unmatched_playlists: set[tuple[Any, Any, Any]] = set() if self.config["auto"]: self.register_listener("database_change", self.db_change) @@ -129,15 +129,15 @@ class SmartPlaylistPlugin(BeetsPlugin): def update_cmd(self, lib: Any, opts: Any, args: list[str]) -> None: self.build_queries() if args: - args = set(args) - for a in list(args): + args_set = set(args) + for a in list(args_set): if not a.endswith(".m3u"): - args.add(f"{a}.m3u") + args_set.add(f"{a}.m3u") playlists = { (name, q, a_q) for name, q, a_q in self._unmatched_playlists - if name in args + if name in args_set } if not playlists: unmatched = [name for name, _, _ in self._unmatched_playlists] @@ -185,6 +185,7 @@ class SmartPlaylistPlugin(BeetsPlugin): try: for key, model_cls in (("query", Item), ("album_query", Album)): qs = playlist.get(key) + query_and_sort: tuple[Any, Any] if qs is None: query_and_sort = None, None elif isinstance(qs, str): @@ -353,7 +354,7 @@ class SmartPlaylistPlugin(BeetsPlugin): ) f.write(comment.encode("utf-8") + entry.uri + b"\n") # Send an event when playlists were updated. - send_event("smartplaylist_update") + send_event("smartplaylist_update") # type: ignore if pretend: self._log.info( diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index ad9f056cc..8ec2c74ce 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -33,8 +33,8 @@ from beetsplug.smartplaylist import SmartPlaylistPlugin class SmartPlaylistTest(BeetsTestCase): def test_build_queries(self): spl = SmartPlaylistPlugin() - assert spl._matched_playlists is None - assert spl._unmatched_playlists is None + assert spl._matched_playlists == set() + assert spl._unmatched_playlists == set() config["smartplaylist"]["playlists"].set([]) spl.build_queries() @@ -182,7 +182,7 @@ class SmartPlaylistTest(BeetsTestCase): q = Mock() a_q = Mock() pl = b"$title-my.m3u", (q, None), (a_q, None) - spl._matched_playlists = [pl] + spl._matched_playlists = {pl} dir = mkdtemp() config["smartplaylist"]["relative_to"] = False @@ -224,7 +224,7 @@ class SmartPlaylistTest(BeetsTestCase): q = Mock() a_q = Mock() pl = b"$title-my.m3u", (q, None), (a_q, None) - spl._matched_playlists = [pl] + spl._matched_playlists = {pl} dir = mkdtemp() config["smartplaylist"]["output"] = "extm3u" @@ -274,7 +274,7 @@ class SmartPlaylistTest(BeetsTestCase): q = Mock() a_q = Mock() pl = b"$title-my.m3u", (q, None), (a_q, None) - spl._matched_playlists = [pl] + spl._matched_playlists = {pl} dir = mkdtemp() config["smartplaylist"]["output"] = "extm3u" @@ -319,7 +319,7 @@ class SmartPlaylistTest(BeetsTestCase): q = Mock() a_q = Mock() pl = b"$title-my.m3u", (q, None), (a_q, None) - spl._matched_playlists = [pl] + spl._matched_playlists = {pl} dir = mkdtemp() tpl = "http://beets:8337/item/$id/file" @@ -380,7 +380,7 @@ class SmartPlaylistTest(BeetsTestCase): # Create playlist with multiple queries (stored as tuple) queries_and_sorts = ((q1, None), (q2, None), (q3, None)) pl = "ordered.m3u", (queries_and_sorts, None), (None, None) - spl._matched_playlists = [pl] + spl._matched_playlists = {pl} dir = mkdtemp() config["smartplaylist"]["relative_to"] = False @@ -435,7 +435,7 @@ class SmartPlaylistTest(BeetsTestCase): # Create playlist with multiple queries (stored as tuple) queries_and_sorts = ((q1, None), (q2, None)) pl = "dedup.m3u", (queries_and_sorts, None), (None, None) - spl._matched_playlists = [pl] + spl._matched_playlists = {pl} dir = mkdtemp() config["smartplaylist"]["relative_to"] = False From 6bfe7cfbc9daf8f30850f4434f32af9f894886f9 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 26 Nov 2025 13:47:18 -0500 Subject: [PATCH 09/12] refactor(smartplaylist): Improve type safety in query building --- beetsplug/smartplaylist.py | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 82739a995..8c61ee1fc 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -158,6 +158,20 @@ class SmartPlaylistPlugin(BeetsPlugin): if v is not None and k in self.config: self.config[k] = v + def _parse_one_query( + self, playlist: dict[str, Any], key: str, model_cls: type + ) -> tuple[Any, Any]: + qs = playlist.get(key) + if qs is None: + return None, None + if isinstance(qs, str): + return parse_query_string(qs, model_cls) + if len(qs) == 1: + return parse_query_string(qs[0], model_cls) + + queries_and_sorts = tuple(parse_query_string(q, model_cls) for q in qs) + return queries_and_sorts, None + def build_queries(self) -> None: """ Instantiate queries for the playlists. @@ -181,34 +195,16 @@ class SmartPlaylistPlugin(BeetsPlugin): self._log.warning("playlist configuration is missing name") continue - playlist_data = (playlist["name"],) try: - for key, model_cls in (("query", Item), ("album_query", Album)): - qs = playlist.get(key) - query_and_sort: tuple[Any, Any] - if qs is None: - query_and_sort = None, None - elif isinstance(qs, str): - query_and_sort = parse_query_string(qs, model_cls) - elif len(qs) == 1: - query_and_sort = parse_query_string(qs[0], model_cls) - else: - # multiple queries and sorts - preserve order - # Store tuple of (query, sort) tuples for hashability - queries_and_sorts = tuple( - parse_query_string(q, model_cls) for q in qs - ) - query_and_sort = queries_and_sorts, None - - playlist_data += (query_and_sort,) - + q_match = self._parse_one_query(playlist, "query", Item) + a_match = self._parse_one_query(playlist, "album_query", Album) except ParsingError as exc: self._log.warning( "invalid query in playlist {}: {}", playlist["name"], exc ) continue - self._unmatched_playlists.add(playlist_data) + self._unmatched_playlists.add((playlist["name"], q_match, a_match)) def _matches_query(self, model: Item | Album, query: Any) -> bool: if not query: From b7541bedbd9df4b29ea1a3e63cf170d2d2d7998d Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Tue, 2 Dec 2025 09:10:06 -0500 Subject: [PATCH 10/12] Annotated handlers to accept a Library instead of Any and added typed playlist helpers --- beetsplug/smartplaylist.py | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 8c61ee1fc..40eb591b1 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -17,13 +17,13 @@ from __future__ import annotations import os -from typing import Any +from typing import Any, TypeAlias from urllib.parse import quote from urllib.request import pathname2url from beets import ui -from beets.dbcore.query import ParsingError -from beets.library import Album, Item, parse_query_string +from beets.dbcore.query import ParsingError, Query, Sort +from beets.library import Album, Item, Library, parse_query_string from beets.plugins import BeetsPlugin from beets.plugins import send as send_event from beets.util import ( @@ -36,6 +36,14 @@ from beets.util import ( syspath, ) +QueryAndSort = tuple[Query, Sort] +PlaylistQuery = Query | tuple[QueryAndSort, ...] | None +PlaylistMatch: TypeAlias = tuple[ + str, + tuple[PlaylistQuery, Sort | None], + tuple[PlaylistQuery, Sort | None], +] + class SmartPlaylistPlugin(BeetsPlugin): def __init__(self) -> None: @@ -57,8 +65,8 @@ class SmartPlaylistPlugin(BeetsPlugin): ) self.config["prefix"].redact = True # May contain username/password. - self._matched_playlists: set[tuple[Any, Any, Any]] = set() - self._unmatched_playlists: set[tuple[Any, Any, Any]] = set() + self._matched_playlists: set[PlaylistMatch] = set() + self._unmatched_playlists: set[PlaylistMatch] = set() if self.config["auto"]: self.register_listener("database_change", self.db_change) @@ -126,7 +134,7 @@ class SmartPlaylistPlugin(BeetsPlugin): spl_update.func = self.update_cmd return [spl_update] - def update_cmd(self, lib: Any, opts: Any, args: list[str]) -> None: + def update_cmd(self, lib: Library, opts: Any, args: list[str]) -> None: self.build_queries() if args: args_set = set(args) @@ -160,7 +168,7 @@ class SmartPlaylistPlugin(BeetsPlugin): def _parse_one_query( self, playlist: dict[str, Any], key: str, model_cls: type - ) -> tuple[Any, Any]: + ) -> tuple[PlaylistQuery, Sort | None]: qs = playlist.get(key) if qs is None: return None, None @@ -169,7 +177,9 @@ class SmartPlaylistPlugin(BeetsPlugin): if len(qs) == 1: return parse_query_string(qs[0], model_cls) - queries_and_sorts = tuple(parse_query_string(q, model_cls) for q in qs) + queries_and_sorts: tuple[QueryAndSort, ...] = tuple( + parse_query_string(q, model_cls) for q in qs + ) return queries_and_sorts, None def build_queries(self) -> None: @@ -206,7 +216,7 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists.add((playlist["name"], q_match, a_match)) - def _matches_query(self, model: Item | Album, query: Any) -> bool: + def _matches_query(self, model: Item | Album, query: PlaylistQuery) -> bool: if not query: return False if isinstance(query, (list, tuple)): @@ -214,7 +224,10 @@ class SmartPlaylistPlugin(BeetsPlugin): return query.match(model) def matches( - self, model: Item | Album, query: Any, album_query: Any + self, + model: Item | Album, + query: PlaylistQuery, + album_query: PlaylistQuery, ) -> bool: if isinstance(model, Album): return self._matches_query(model, album_query) @@ -222,7 +235,7 @@ class SmartPlaylistPlugin(BeetsPlugin): return self._matches_query(model, query) return False - def db_change(self, lib: Any, model: Item | Album) -> None: + def db_change(self, lib: Library, model: Item | Album) -> None: if self._unmatched_playlists is None: self.build_queries() @@ -235,7 +248,7 @@ class SmartPlaylistPlugin(BeetsPlugin): self._unmatched_playlists -= self._matched_playlists - def update_playlists(self, lib: Any, pretend: bool = False) -> None: + def update_playlists(self, lib: Library, pretend: bool = False) -> None: if pretend: self._log.info( "Showing query results for {} smart playlists...", From 03283aaf27a717237406bb181dc6805a1a01d305 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Tue, 2 Dec 2025 09:19:09 -0500 Subject: [PATCH 11/12] update lint From 20d9b6a13670049a5a76c888ee614c95bcc8cb5b Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Tue, 2 Dec 2025 09:25:42 -0500 Subject: [PATCH 12/12] Fix URL-encoding path conversion --- beetsplug/smartplaylist.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 40eb591b1..ed417f2b9 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -319,7 +319,9 @@ class SmartPlaylistPlugin(BeetsPlugin): if self.config["forward_slash"].get(): item_uri = path_as_posix(item_uri) if self.config["urlencode"]: - item_uri = bytestring_path(pathname2url(item_uri)) + item_uri = bytestring_path( + pathname2url(os.fsdecode(item_uri)) + ) item_uri = prefix + item_uri if item_uri not in m3us[m3u_name]: