Skip to content

tidal: respect search_limit config option in candidates()#6781

Open
kveni12 wants to merge 6 commits into
beetbox:masterfrom
kveni12:fix/tidal-search-limit
Open

tidal: respect search_limit config option in candidates()#6781
kveni12 wants to merge 6 commits into
beetbox:masterfrom
kveni12:fix/tidal-search-limit

Conversation

@kveni12

@kveni12 kveni12 commented Jun 27, 2026

Copy link
Copy Markdown

Fixes #6770

Problem

TidalPlugin extends MetadataSourcePlugin, which sets a default search_limit: 5 config value. However, candidates() and item_candidates() called search_albums_by_query() and search_tracks_by_query() without any limit, so the config option was silently ignored.

Fix

  • Add an optional limit: int | None = None parameter to search_albums_by_query() — slices album_ids before passing to search_albums_by_ids()
  • Add an optional limit: int | None = None parameter to search_tracks_by_query() — slices the track relationships list before iterating
  • candidates() reads self.config["search_limit"].get(int) and passes it through
  • item_candidates() does the same

The fix is intentionally minimal — no changes to api.py or the Tidal HTTP request; results are simply sliced client-side, consistent with how other direct-implementation plugins cap their results.

Tests

Added TestSearchLimit with two tests:

  • test_candidates_respects_search_limit — sets search_limit: 1, mocks search_results returning 2 album IDs, asserts only 1 candidate is returned
  • test_item_candidates_respects_search_limit — same pattern for tracks

TidalPlugin.candidates() and item_candidates() called search_albums_by_query()
and search_tracks_by_query() without passing any limit, ignoring the
search_limit config value inherited from MetadataSourcePlugin.

Add an optional limit parameter to both query methods and slice the result
list before fetching full records, matching the pattern used by other
direct-implementation plugins.

Fixes beetbox#6770

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kveni12 kveni12 requested review from a team and semohr as code owners June 27, 2026 05:43
@github-actions

Copy link
Copy Markdown

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.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.85%. Comparing base (bf879d5) to head (c33a520).
⚠️ Report is 7 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6781   +/-   ##
=======================================
  Coverage   74.84%   74.85%           
=======================================
  Files         163      163           
  Lines       20949    20946    -3     
  Branches     3300     3297    -3     
=======================================
- Hits        15680    15679    -1     
+ Misses       4511     4510    -1     
+ Partials      758      757    -1     
Files with missing lines Coverage Δ
beetsplug/tidal/__init__.py 89.33% <100.00%> (-0.05%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@semohr semohr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for already taking a look here!

We should be able to resolve all search limit based logic within the candidates and item_candidates function. I don't really see the need for the extensive drilling.

The search_tracks_by_query and search_albums_by_query functions are returning iterators. We should be able to break them early and limit the number of requests that way.

Additionally the search_limit currently does not respect the _album_queries and _item_queries loop. We might want to have a round robin approach here.


I will have a more detailed look next week.

@snejus

snejus commented Jun 27, 2026

Copy link
Copy Markdown
Member

I've added it to my fork using a slightly simpler solution 626963d

Instead of drilling a limit parameter into search_albums_by_query and
search_tracks_by_query, handle the cap entirely inside candidates() and
item_candidates() by consuming each query's iterator in a round-robin loop
and stopping as soon as search_limit results have been collected.

This keeps the query functions pure and correctly interleaves results
across multiple queries (e.g. multiple artist/album combinations from
_album_queries) rather than exhausting one query before moving to the next.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kveni12

kveni12 commented Jun 27, 2026

Copy link
Copy Markdown
Author

hi @semohr thank you for the feedback! I completely agree, so moved the limit logic entirely into candidates() and item_candidates() using a round-robin loop that early-breaks across query iterators, so search_albums_by_query and search_tracks_by_query stay clean. Also added a test covering the multi-query interleaving behaviour.

- Wrap query iterables with iter() so next() satisfies mypy's
  SupportsNext constraint
- Shorten two docstrings that exceeded the 88-char line limit
- Reformat test file to satisfy ruff

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread beetsplug/tidal/__init__.py Outdated
Comment thread beetsplug/tidal/__init__.py Outdated
kveni12 added 2 commits June 28, 2026 09:06
Use cached_property for search_limit and replace manual round-robin loops
with itertools.chain.from_iterable, zip_longest, and islice.
Use None as the zip_longest fill value so mypy can narrow candidate
types instead of inferring object from an object() sentinel.

for query in self._item_queries(item):
candidates += self.search_tracks_by_query(query)
candidates = list(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets make the round robin logic a helper function. Should make the flow here more readable and should allow to dedupe some code.

(
candidate
for candidate in itertools.chain.from_iterable(
itertools.zip_longest(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is zip_longest here for? Also, you can use filter(None, ... instead of checking for None values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tidal: search_limit is ignored

3 participants