Empty metadata support for autotagger plugins

[semohr]

Description

It is possible for an metadata lookup to be performed with an empty string for both artist and title/album. This PR add handling for this edgecase for the metadata lookup of musibrainz, spotify, discogs and beatport.

Seems like the issue was not catched earlier, since the typehints were not propagated correctly in the metadata_plugin.item_candidates function.

closes #6060 #5965 might have helped here too

[codecov[bot]]

Codecov Report

:x: Patch coverage is 6.25000% with 15 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 66.93%. Comparing base (c26c342) to head (ddca7c4). :warning: Report is 11 commits behind head on master. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/beatport.py	14.28%	6 Missing :warning:
beetsplug/discogs.py	0.00%	3 Missing :warning:
beetsplug/musicbrainz.py	0.00%	2 Missing and 1 partial :warning:
beetsplug/spotify.py	0.00%	2 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6065      +/-   ##
==========================================
- Coverage   66.98%   66.93%   -0.06%     
==========================================
  Files         118      118              
  Lines       18189    18206      +17     
  Branches     3079     3084       +5     
==========================================
+ Hits        12184    12186       +2     
- Misses       5345     5359      +14     
- Partials      660      661       +1     

Files with missing lines	Coverage Δ
beetsplug/discogs.py	70.25% <0.00%> (-0.54%)	:arrow_down:
beetsplug/musicbrainz.py	68.94% <0.00%> (-0.55%)	:arrow_down:
beetsplug/spotify.py	46.12% <0.00%> (-0.48%)	:arrow_down:
beetsplug/beatport.py	42.85% <14.28%> (-1.05%)	:arrow_down:

... and 1 file with indirect coverage changes

:rocket: New features to boost your workflow:

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

[snejus]

Seems like the issue was not catched earlier, since the typehints were not propagated correctly in the metadata_plugin.item_candidates function.

I may be missing something, but it seems to me they have been correct? Both candidates functions always received strings - including empty ones.

[semohr]

Seems like the issue was not catched earlier, since the typehints were not propagated correctly in the metadata_plugin.item_candidates function.

I may be missing something, but it seems to me they have been correct? Both candidates functions always received strings - including empty ones.

see https://github.com/beetbox/beets/pull/6065#discussion_r2401359869

[semohr]

@snejus How do we want to continue here?

The main concern was the duplication as far as I can remember. It might be possible to move the duplication check into an api layer (or shared session) at some point but right now I dont see how we can dedupe this otherwise.

[snejus]

Forgot about this PR! My main concern is that chroma plugin is very different from the rest of metadata source plugins - and whether we gain any value from forcing it to implement MetadataSourcePlugin? It does not implement track_for_id and album_for_id which also indicates that this interface may not the best fit for it.

I'm leaning towards making it inherit BeetsPlugin instead:

1. Define data_source explicitly to make sure it's still treated as a data source
2. Centralise the empty search handling for MetadataSourcePlugin subclasses

[semohr]

I think the proposed change creates more problems than it solves. Maybe we are stuck here without a bigger refactor.

Chroma does define track_for_id and album_for_id they just return empty results. And that actually makes sense: the plugin supports the interface contract, but simply doesn’t produce lookups because fingerprinting doesn’t map cleanly to ID-based metadata sources (which it could in theory). That’s a perfectly valid implementation and doesn’t justify removing it from MetadataSourcePlugin.

Switching it to inherit directly from BeetsPlugin' blurs the separation between the two interfaces. MetadataSourcePlugin has a clear purpose imo ID-based lookup and search semantics, while BeetsPlugin is intentionally generic. Moving chroma out of the metadata-source abstraction weakens that clarity and makes the inheritance structure harder to reason about.

It also goes against one of beets strengths: being open and flexible in how plugins can be implemented. We shouldn’t impose rules that don’t fit every plugin. Chroma isn’t the only case, there are external metadata plugins that also purely work on Item data without necessary requiring titles and artist (e.g. aisauce or audible).

I agree that from a maintainer standpoint, the proposal may simplify internal maintenance slightly. But it adds ambiguity for plugin authors, increases the risk of mismatched expectations in future core changes, and could even break existing workflows for users.

---

TLDR: chroma fits the interface well enough, and beets has always thrived by not forcing plugin authors into rigid patterns. This change would move in the opposite direction.