chroma: gate candidates on musicbrainz plugin being enabled#6532
Open
AnonymousAAArdvark wants to merge 2 commits intobeetbox:masterfrom
Open
chroma: gate candidates on musicbrainz plugin being enabled#6532AnonymousAAArdvark wants to merge 2 commits intobeetbox:masterfrom
AnonymousAAArdvark wants to merge 2 commits intobeetbox:masterfrom
Conversation
The chroma plugin uses Acoustid fingerprinting, which returns MusicBrainz
release and recording IDs. It then unconditionally resolved those IDs
through a privately instantiated MusicBrainzPlugin, so MusicBrainz-sourced
candidates appeared during tagging even when the user had not enabled the
musicbrainz plugin.
Replace the direct MusicBrainzPlugin() instantiation with a lookup through
the metadata-source registry via get_metadata_source("musicbrainz"). When
that returns None, short-circuit both candidates() and item_candidates()
to return empty. This also fixes an incidental issue where the private
instance bypassed any plugin that swaps the musicbrainz plugin at runtime
(e.g. mbpseudo).
Acoustid fingerprinting itself is unaffected — acoustid_id and
acoustid_fingerprint item fields are still populated as before.
Fixes beetbox#6212
8 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6532 +/- ##
==========================================
+ Coverage 70.67% 71.12% +0.44%
==========================================
Files 150 150
Lines 18918 19130 +212
Branches 3078 3081 +3
==========================================
+ Hits 13371 13606 +235
+ Misses 4896 4867 -29
- Partials 651 657 +6
🚀 New features to boost your workflow:
|
snejus
requested changes
Apr 13, 2026
Member
snejus
left a comment
snejus
left a comment
There was a problem hiding this comment.
This is great, thanks. Just a pair of small comments.
Member
|
I think this also fixed another issue related to mbpseudo as well? Is there a reported GitHub issue for that? |
semohr
reviewed
Apr 13, 2026
test/plugins/test_chroma.py
Outdated
| metadata_plugins.get_metadata_source.cache_clear() | ||
|
|
||
|
|
||
| def _load_plugins(*names: str) -> None: |
Contributor
There was a problem hiding this comment.
- Use @cached_property named `mb` (per snejus/semohr) instead of a helper method, reducing the diff relative to the original code - Move changelog entry to the Unreleased section (per snejus) - Refactor tests to use PluginMixin from beets.test.helper (per semohr) instead of manually managing beets.plugins._instances
snejus
reviewed
Apr 14, 2026
Comment on lines
+87
to
+91
| # ----------------------------------------------------------------------------- | ||
| # Regression tests for issue #6212: chroma must respect which metadata source | ||
| # plugins are enabled. When the musicbrainz plugin is not loaded, chroma must | ||
| # not produce any MusicBrainz-sourced candidates. | ||
| # ----------------------------------------------------------------------------- |
Member
There was a problem hiding this comment.
Is it possible to use a single test class and include this in its docstring?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6212. Split out from #6522 per @semohr's suggestion.
The
chromaplugin unconditionally instantiated its own privateMusicBrainzPlugininstance and calledalbum_for_id/track_for_idon it regardless of the user's plugin configuration.This meant MusicBrainz-sourced candidates appeared during tagging even
when the user had not enabled the
musicbrainzplugin.This PR replaces the direct instantiation with a lookup through the
metadata-source registry (
get_metadata_source("musicbrainz")).When the musicbrainz plugin is not loaded, both
candidatesanditem_candidatesshort-circuit and return empty. Acoustidfingerprinting itself is unaffected —
acoustid_idandacoustid_fingerprintfields are still populated as before.Side-effect fix
The previous
cached_property mb = MusicBrainzPlugin()patterncreated a second instance outside the plugin registry. This bypassed
any plugin that swaps the musicbrainz instance at runtime (e.g.
mbpseudo). Routing throughget_metadata_sourcefixes this —chroma now uses the same shared instance as the rest of beets.
Scope
This PR is intentionally scoped to the gating fix only. The richer
cross-reference routing (extracting Spotify/Discogs/etc. IDs from
MusicBrainz
url-relationsand dispatching to enabled externalplugins) is being developed separately in #6522.
Test plan
test/plugins/test_chroma.py:candidates/item_candidates× with / without musicbrainz.chromasearchtests still pass.metadata_plugins, autotag/match, autotag/distance).
ruff check+ruff format --checkclean.mypy beetsplug/chroma.pyclean.sphinx-lint+docstrfmt(Python 3.10) clean.