Skip to content

Conversation

@asardaes
Copy link
Contributor

@asardaes asardaes commented Dec 30, 2025

Description

Supersedes #6163, but the same improvements apply:

  • Support reimport via the album_info_received event
  • Support multiple pseudo-releases in timid mode

See also my comment below.

To Do

  • Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
  • Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
  • Tests. (Very much encouraged but not strictly required.)

@asardaes asardaes requested a review from snejus December 30, 2025 16:06
@asardaes asardaes requested review from a team and semohr as code owners December 30, 2025 16:06
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 7 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `beets/metadata_plugins.py:39-42` </location>
<code_context>

 @notify_info_yielded("albuminfo_received")
-def candidates(*args, **kwargs) -> Iterable[AlbumInfo]:
+def candidates(items, *args, **kwargs) -> Iterable[AlbumInfo]:
     """Return matching album candidates from all metadata source plugins."""
     for plugin in find_metadata_source_plugins():
-        yield from plugin.candidates(*args, **kwargs)
+        for candidate in plugin.candidates(items, *args, **kwargs):
+            send("album_info_received", items=items, album_info=candidate)
+            yield candidate
</code_context>

<issue_to_address>
**issue (bug_risk):** Passing `items` positionally to plugin.candidates can break existing metadata source plugins.

This new signature requires all metadata source plugins to accept `items` as the first positional argument. Any existing plugin still using the old positional signature (e.g. `candidates(artist, album, va_likely, ...)`) will now fail with a `TypeError`.

To keep this backwards compatible, detect whether a plugin supports `items` (e.g. via `inspect.signature` or a feature flag like `supports_items`) and only pass `items` when supported, falling back to the previous call pattern otherwise.
</issue_to_address>

### Comment 2
<location> `beets/metadata_plugins.py:51-54` </location>
<code_context>
         yield from plugin.item_candidates(*args, **kwargs)


-def album_for_id(_id: str) -> AlbumInfo | None:
+def album_for_id(_id: str, items: Iterable[Item]) -> AlbumInfo | None:
     """Get AlbumInfo object for the given ID string.

</code_context>

<issue_to_address>
**issue (bug_risk):** Changing the public album_for_id signature may break existing callers that don’t provide items.

Because `metadata_plugins.album_for_id` is a public helper, external callers that use `metadata_plugins.album_for_id(mbid)` without `items` will now raise errors.

To preserve compatibility, you could give `items` a default of `None` and only call `send("album_info_received", ...)` when `items` is not `None`. That keeps the new behavior for internal callers while avoiding a breaking API change.
</issue_to_address>

### Comment 3
<location> `test/plugins/test_musicbrainz_pseudo.py:132-141` </location>
<code_context>
+class TestMBPseudoReleases(TestMBPseudoMixin):
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding tests for `_handle_main_pseudo_release` fallback paths (no relation and no desired script cases).

The current tests only cover pseudo-releases that point to an official release with a desired script via `_handle_intercepted_pseudo_releases`. Two branches in `_handle_main_pseudo_release` remain untested:

1. Pseudo-release with no `transl-tracklisting` backward relation (should return the pseudo-release as-is).
2. Pseudo-release with a backward relation where `_has_desired_script` is `False` (should return the merged pseudo+official `AlbumInfo` via `_merge_pseudo_and_actual_album`).

Targeted tests for these cases would better safeguard behavior when MB data is incomplete or uses an unexpected script.

Suggested implementation:

```python
class TestMBPseudoReleaseFallbacks(TestMBPseudoMixin):
    def test_main_pseudo_release_no_back_relation_returns_pseudo(
        self,
        musicbrainz_plugin: MusicBrainzPlugin,
        pseudo_release_info: AlbumInfo,
        monkeypatch,
    ):
        # Force the "no transl-tracklisting backward relation" path
        # by making the internal relation lookup return None.
        # _handle_main_pseudo_release should then return the pseudo-release as-is.
        def fake_get_back_relation(pseudo: AlbumInfo):
            assert pseudo is pseudo_release_info
            return None

        # Adjust attribute name here if the plugin uses a different helper
        monkeypatch.setattr(
            musicbrainz_plugin,
            "_get_transl_tracklisting_backward_relation",
            fake_get_back_relation,
            raising=False,
        )

        result = musicbrainz_plugin._handle_main_pseudo_release(pseudo_release_info)
        assert result is pseudo_release_info

    def test_main_pseudo_release_no_desired_script_merges_pseudo_and_official(
        self,
        musicbrainz_plugin: MusicBrainzPlugin,
        official_release_info: AlbumInfo,
        pseudo_release_info: AlbumInfo,
        monkeypatch,
    ):
        # Force the "has backward relation but no desired script" path:
        # 1. Provide a synthetic transl-tracklisting backward relation.
        # 2. Make _has_desired_script return False.
        # 3. Stub _merge_pseudo_and_actual_album and assert it is used.
        def fake_get_back_relation(pseudo: AlbumInfo):
            assert pseudo is pseudo_release_info
            # Adjust structure to match whatever the real code expects.
            return {"release": {"id": official_release_info.album_id}}

        monkeypatch.setattr(
            musicbrainz_plugin,
            "_get_transl_tracklisting_backward_relation",
            fake_get_back_relation,
            raising=False,
        )

        def fake_has_desired_script(relation):
            # We explicitly force the "no desired script" path
            return False

        monkeypatch.setattr(
            musicbrainz_plugin,
            "_has_desired_script",
            fake_has_desired_script,
            raising=True,
        )

        merged_sentinel = object()

        def fake_merge_pseudo_and_actual_album(
            pseudo: AlbumInfo,
            actual: AlbumInfo,
        ) -> AlbumInfo:
            assert pseudo is pseudo_release_info
            assert actual is official_release_info
            return merged_sentinel  # type: ignore[return-value]

        monkeypatch.setattr(
            musicbrainz_plugin,
            "_merge_pseudo_and_actual_album",
            fake_merge_pseudo_and_actual_album,
            raising=True,
        )

        result = musicbrainz_plugin._handle_main_pseudo_release(pseudo_release_info)
        assert result is merged_sentinel


class TestMBPseudoReleases(TestMBPseudoMixin):

```

These tests assume a couple of internal helper names and relation shapes that may differ slightly in your code:

1. If the helper used by `_handle_main_pseudo_release` to obtain the backward transl-tracklisting relation is not named `_get_transl_tracklisting_backward_relation`, adjust the `monkeypatch.setattr` targets `"_get_transl_tracklisting_backward_relation"` in both tests to the actual helper name.
2. If `_handle_main_pseudo_release` reads the related official release ID from a different structure than `{"release": {"id": official_release_info.album_id}}`, update `fake_get_back_relation` to return a dict (or object) that matches what the real implementation expects.
3. If `_handle_main_pseudo_release` does not call `_get_transl_tracklisting_backward_relation` at all, but instead reads relations directly from `AlbumInfo` / `pseudo_release_info`, you can instead:
   - Remove the monkeypatch of `_get_transl_tracklisting_backward_relation`.
   - Modify `pseudo_release_info` (or create a copy) so that its backing MB JSON has either:
     a) no `transl-tracklisting` backward relation (for the first test), or  
     b) exactly one such relation pointing to `official_release_info` (for the second test).
4. Ensure that `_handle_main_pseudo_release` is indeed a method on `musicbrainz_plugin`. If it is instead a free function or lives on another object, adjust the call sites accordingly.
</issue_to_address>

### Comment 4
<location> `test/plugins/test_musicbrainz_pseudo.py:210-173` </location>
<code_context>
-        assert not isinstance(album_info, PseudoAlbumInfo)
-        assert album_info.data_source == "MusicBrainzPseudoRelease"
-
-    def test_interception(
-        self,
-        mbpseudo_plugin: MusicBrainzPseudoReleasePlugin,
-        official_release: JSONDict,
-    ):
-        album_info = mbpseudo_plugin.album_info(official_release)
-        assert isinstance(album_info, PseudoAlbumInfo)
-        assert album_info.data_source == "MusicBrainzPseudoRelease"
-
</code_context>

<issue_to_address>
**suggestion (testing):** Add tests that explicitly exercise `_intercept_mb_release` when no scripts are configured and when release-relations are absent.

Current tests cover mismatched relation fields and scripts, but not these additional branches of `_intercept_mb_release`:

* `self._scripts` is empty → should immediately return `[]`.
* Release has no `release-relations` → interception should be a no-op.

Please add tests that construct these cases, assert `_intercept_mb_release` returns `[]`, and verify that `album_for_id` falls back to using only the main release.

Suggested implementation:

```python
        album_info = self.get_album_info(musicbrainz_plugin, official_release)
        assert not isinstance(album_info, PseudoAlbumInfo)
        assert album_info.data_source == "MusicBrainz"

    def test_intercept_mb_release_no_scripts(
        self,
        mbpseudo_plugin: MusicBrainzPseudoReleasePlugin,
        musicbrainz_plugin: MusicBrainzPlugin,
        official_release: JSONDict,
    ):
        # When no scripts are configured, interception should immediately return [].
        mbpseudo_plugin._scripts = {}

        intercepted = mbpseudo_plugin._intercept_mb_release(
            musicbrainz_plugin, official_release
        )
        assert intercepted == []

        # album_for_id should fall back to using only the main release.
        albums = list(
            musicbrainz_plugin.album_for_id(official_release["id"], None, None)
        )
        assert albums
        assert all(not isinstance(a, MultiPseudoAlbumInfo) for a in albums)

    def test_intercept_mb_release_no_release_relations(
        self,
        mbpseudo_plugin: MusicBrainzPseudoReleasePlugin,
        musicbrainz_plugin: MusicBrainzPlugin,
        official_release: JSONDict,
    ):
        # When the release has no "release-relations", interception should be a no-op.
        release_without_relations = dict(official_release)
        release_without_relations.pop("release-relations", None)

        intercepted = mbpseudo_plugin._intercept_mb_release(
            musicbrainz_plugin, release_without_relations
        )
        assert intercepted == []

        # album_for_id should again fall back to using only the main release.
        albums = list(
            musicbrainz_plugin.album_for_id(official_release["id"], None, None)
        )
        assert albums
        assert all(not isinstance(a, MultiPseudoAlbumInfo) for a in albums)


import pytest

```

These changes assume:

1. `MusicBrainzPseudoReleasePlugin`, `MusicBrainzPlugin`, `MultiPseudoAlbumInfo`, and `PseudoAlbumInfo` are already imported in this test module.  
   If any are missing, add them alongside the existing imports, for example:
   - `from beetsplug.musicbrainz_pseudo import MusicBrainzPseudoReleasePlugin, PseudoAlbumInfo`
   - `from beetsplug.musicbrainz import MusicBrainzPlugin, MultiPseudoAlbumInfo`

2. `musicbrainz_plugin.album_for_id` accepts `(mbid, includes, release_status)` (or similar) and yields/returns an iterable of `AlbumInfo` / `MultiPseudoAlbumInfo`.  
   If your signature differs, adjust the `album_for_id` call arguments accordingly.

3. If `mbpseudo_plugin._scripts` is not a dict, set it to the appropriate “empty” value that makes the `_scripts` falsy check pass (e.g. `[]` instead of `{}`) to hit the branch `if not self._scripts: return []`.
</issue_to_address>

### Comment 5
<location> `test/plugins/test_musicbrainz_pseudo.py:263-272` </location>
<code_context>
+class TestMBMultiplePseudoReleases(PluginMixin):
</code_context>

<issue_to_address>
**suggestion (testing):** Extend `MultiPseudoAlbumInfo` tests to cover deepcopy and data integrity after cloning.

`TestMBMultiplePseudoReleases` currently only checks `unwrap()` and ordering, but not the custom `__deepcopy__` on `MultiPseudoAlbumInfo`. Since beets frequently clones `AlbumInfo` instances, a regression here could be hard to spot.

Please add a test that:
- Builds a `MultiPseudoAlbumInfo` via the plugin,
- Calls `deepcopy()` on it,
- Verifies the copy has the same metadata and `unwrap()` contents, and
- Confirms mutations to the copy or its wrapped infos do not affect the original.

This will better validate the deepcopy behavior and guard against subtle regressions.

Suggested implementation:

```python
import copy

class TestMBMultiplePseudoReleases(PluginMixin):

```

```python
    @pytest.fixture(autouse=True)
    def patch_get_release(
        self,
        monkeypatch,
        official_release: JSONDict,
        pseudo_release: JSONDict,
    ):
        """Patch the MusicBrainz client to return both the official and pseudo releases.

        This ensures the plugin can build a MultiPseudoAlbumInfo from real-looking
        MusicBrainz JSON while keeping the tests fully deterministic.
        """

        def mock_get_release(_, album_id: str):
            if album_id == "official":
                return official_release
            if album_id == "pseudo":
                return pseudo_release

            raise AssertionError(f"Unexpected album_id requested in test: {album_id!r}")

        # NOTE: the exact target here may differ depending on how the client is imported
        # in the musicbrainz plugin; adjust the dotted path if necessary.
        monkeypatch.setattr(
            "beetsplug.musicbrainz.MusicBrainzClient.get_release",
            mock_get_release,
            raising=True,
        )

    def test_multi_pseudo_albuminfo_deepcopy_independent(self, library, tmp_path):
        """MultiPseudoAlbumInfo deepcopy keeps data but fully detaches references.

        The musicbrainz plugin frequently clones AlbumInfo instances; this test
        ensures that MultiPseudoAlbumInfo's custom __deepcopy__ implementation
        preserves metadata and unwrap() contents while preventing mutations on
        the copy from affecting the original.
        """
        from beets.autotag.hooks import MultiPseudoAlbumInfo

        # Obtain the plugin instance via the PluginMixin facilities.
        # PluginMixin typically exposes a mapping of loaded plugins keyed by name.
        musicbrainz_plugin = self.plugins[self.plugin]

        # Build a MultiPseudoAlbumInfo via the plugin. The exact public surface
        # used here should mirror how the plugin actually exposes pseudo releases.
        # For example, this might internally resolve both official and pseudo
        # releases using the patched MusicBrainz client above.
        multi_info: MultiPseudoAlbumInfo = musicbrainz_plugin._get_pseudo_album_info(
            album_id="official"
        )
        assert isinstance(multi_info, MultiPseudoAlbumInfo)

        # Sanity‑check that unwrap() returns underlying AlbumInfo objects and that
        # they are consistent before we start mutating anything.
        original_wrapped = multi_info.unwrap()
        assert original_wrapped
        original_album_titles = [ai.album for ai in original_wrapped]
        original_ids = [ai.album_id for ai in original_wrapped]

        # Perform a deepcopy through the custom __deepcopy__ implementation.
        multi_info_copy = copy.deepcopy(multi_info)
        assert isinstance(multi_info_copy, MultiPseudoAlbumInfo)

        # The copy should expose equivalent metadata and unwrap() contents.
        copy_wrapped = multi_info_copy.unwrap()
        assert [ai.album for ai in copy_wrapped] == original_album_titles
        assert [ai.album_id for ai in copy_wrapped] == original_ids

        # Mutate top‑level attributes on the copy and ensure the original is unaffected.
        multi_info_copy.album = "Changed Album Title"
        multi_info_copy.album_id = "changed-id"
        assert multi_info.album != multi_info_copy.album
        assert multi_info.album_id != multi_info_copy.album_id

        # Mutate the wrapped AlbumInfo objects in the copy and confirm the originals
        # remain unchanged, verifying that deepcopy correctly duplicated the list and
        # its contents rather than sharing references.
        copy_wrapped[0].album = "Wrapped Changed"
        copy_wrapped[0].album_id = "wrapped-changed-id"

        # Re-fetch unwrap() from the original to avoid relying on previous references.
        original_wrapped_after = multi_info.unwrap()
        assert [ai.album for ai in original_wrapped_after] == original_album_titles
        assert [ai.album_id for ai in original_wrapped_after] == original_ids

```

1. The `monkeypatch.setattr` target (`"beetsplug.musicbrainz.MusicBrainzClient.get_release"`) may differ in your codebase depending on how the MusicBrainz client is imported; update the dotted path to match the actual symbol used by the plugin to fetch releases.
2. The call to `musicbrainz_plugin._get_pseudo_album_info(album_id="official")` assumes the plugin exposes a helper that builds a `MultiPseudoAlbumInfo` from an official release plus related pseudo releases. If the actual helper has a different name or signature, replace this call with the correct plugin API that returns a `MultiPseudoAlbumInfo` constructed from the patched MusicBrainz responses.
3. If `PluginMixin` exposes the plugin instance differently (e.g., `self.plugin` already being the instance or a `self.musicbrainz` attribute), adjust `musicbrainz_plugin = self.plugins[self.plugin]` accordingly.
4. If `MultiPseudoAlbumInfo` is imported elsewhere in the file already, move or deduplicate the `from beets.autotag.hooks import MultiPseudoAlbumInfo` import to the file top-level to comply with your project’s import conventions.
</issue_to_address>

### Comment 6
<location> `docs/plugins/musicbrainz.rst:179-180` </location>
<code_context>
+pseudo-releases. For ``artist`` in particular, keep in mind that even
+pseudo-releases might specify it with the original script, so you should also
+configure import :ref:`languages` to give artist aliases more priority.
+Therefore, the minimum configuration to enable this functionality like this:
+
+.. code-block:: yaml
</code_context>

<issue_to_address>
**suggestion (typo):** Sentence is grammatically awkward; likely missing a word such as "is" or could be rephrased.

The phrase "the minimum configuration to enable this functionality like this" is grammatically incomplete. Consider something like "Therefore, the minimum configuration to enable this functionality is as follows:" or "…looks like this:" for clarity.

```suggestion
configure import :ref:`languages` to give artist aliases more priority.
Therefore, the minimum configuration to enable this functionality is as follows:
```
</issue_to_address>

### Comment 7
<location> `docs/plugins/musicbrainz.rst:197-199` </location>
<code_context>
+pseudo-releases with desired scripts.
+
+A release may have multiple pseudo-releases, for example when there is both a
+transliteration and a translation available. By default, only 1 pseudo-release
+per original release is emitted as candidate, using the languages from the
+configuration to decide which one has most priority. If you're importing in
+timid mode and you would like to receive all valid pseudo-releases as additional
</code_context>

<issue_to_address>
**suggestion (typo):** Missing article in "emitted as candidate"; should be "emitted as a candidate".

Here it should read "emitted as a candidate."

```suggestion
A release may have multiple pseudo-releases, for example when there is both a
transliteration and a translation available. By default, only 1 pseudo-release
per original release is emitted as a candidate, using the languages from the
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.86%. Comparing base (b2335b9) to head (6c84ede).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6255      +/-   ##
==========================================
- Coverage   68.95%   68.86%   -0.10%     
==========================================
  Files         140      139       -1     
  Lines       18685    18531     -154     
  Branches     3058     3028      -30     
==========================================
- Hits        12885    12762     -123     
+ Misses       5153     5131      -22     
+ Partials      647      638       -9     
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch from d1973e4 to 84dd0e2 Compare December 30, 2025 16:22
Comment on lines 527 to 428
if isinstance(album_info, PseudoAlbumInfo):
for item in items:
# particularly relevant for reimport but could also happen during import
if "mb_albumid" in item:
del item["mb_albumid"]
if "mb_trackid" in item:
del item["mb_trackid"]
Copy link
Contributor Author

@asardaes asardaes Dec 30, 2025

Choose a reason for hiding this comment

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

If you reimport because you want to switch from an official release to a pseudo-release, the MB ids from the official release introduce a lot of penalty here, that's why I delete them. I think this is fine as long as it isn't flushed to disk/DB, right?

@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch 2 times, most recently from 75e0337 to 2b43058 Compare December 30, 2025 20:52
@asardaes asardaes added this to the 2.6.0 milestone Dec 30, 2025
@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch 3 times, most recently from 6614da8 to 3314445 Compare January 7, 2026 18:30
@JOJ0 JOJ0 added the plugin Pull requests that are plugins related label Jan 10, 2026
@snejus snejus self-assigned this Jan 13, 2026
@snejus
Copy link
Member

snejus commented Jan 13, 2026

Well done for taking this on!!! I think splitting it into two sequential PRs would make the review process much smoother and keep the git history easier to follow.

PR #1: Migration Only

First, let's do a clean refactoring that moves the mbpseudo plugin into musicbrainz without any new features:

  • Move the PseudoAlbumInfo class to beetsplug/musicbrainz.py
  • Migrate the configuration structure from mbpseudo: to musicbrainz.pseudo_releases:
  • Rename and update the tests accordingly
  • Update documentation, CODEOWNERS, and the plugin index
  • Keep all existing behavior exactly the same

This way, it's purely mechanical - just consolidating the code without changing functionality.

PR #2: New Features and Improvements

Once PR #1 is opened, you can create a second PR with all the enhancements:

  • Implement the new album_info_received event
  • Add reimport support with proper MusicBrainz ID handling
  • Introduce MultiPseudoAlbumInfo for handling multiple pseudo-releases
  • Add the multiple_allowed configuration option
  • Improve the search fallback logic

You can use PR #1 as the base branch for PR #2, so you don't need to wait for the first one to merge.

JSON Test Files

One small thing - could you preserve the original field order in the JSON test files? When fields get rearranged, it's harder to see what actually changed in the diff. You can use tools like jq --sort-keys or json.dumps(..., sort_keys=True, indent=2) to keep things consistent and readable.

Does this approach work for you?

@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch from 3314445 to f541d62 Compare January 17, 2026 14:06
@asardaes asardaes changed the title Merge mbpseudo functionality into musicbrainz and improve Merge mbpseudo functionality into musicbrainz Jan 17, 2026
@asardaes
Copy link
Contributor Author

@snejus I moved the improvements to #6298, if you want you can see the diff between the 2 PRs here.

@snejus
Copy link
Member

snejus commented Jan 20, 2026

I've been looking at this and my underlying thought is: there is a lot of complexity for the sake of making sure the distance is adjusted for translations.

What if we keep the distance calculation as it was? I'm aware this will penalize the translations and will move them down the candidate list, however they will still be returned and seen by the users.

The way this is currently designed also only handles a single translation - ideally I'd want to simplify this all to something like

for translation in release.get("translations", []):
    yield apply_translation(info.copy(), translation)

yield info

@asardaes
Copy link
Contributor Author

asardaes commented Jan 20, 2026

Handling multiple translations is added in the other PR. And not avoiding penalization would mean that only timid mode would be usable for this use case.

EDIT: for the most part, avoiding penalization mostly comes down to the custom class + the call to _adjust_final_album_match, I would argue that's not a lot of extra complexity. If you replace all PseudoAlbumInfo with AlbumInfo and remove the final adjustment, the code wouldn't look so different.

@snejus
Copy link
Member

snejus commented Jan 20, 2026

Since I didn't get to review the original PR, and we're merging it into musicbrainz plugin here, I'm using this as an opportunity to review the original design. It's also much easier to reason about it in the context of the rest of the plugin when it's all in a single place.

How does it handle multiple translations?

And in the quick mode, what happens when we have original release + translation, given that both matches yield the same distance? Which one is going to get prioritised?

@asardaes
Copy link
Contributor Author

I'll answer your 2nd question first. It's simply a matter of yielding order: we emit the pseudo-release first, and if it's a good enough candidate, it gets chosen even if other candidates are equally good.

Now to your 1st question. In the current version you can imagine PseudoAlbumInfo is a container for 2 releases, and both are yielded as candidates. To handle multiple releases (in the other PR) I use another class that can contain the main release + any number of pseudo-releases, and they all get yielded. To choose which one is emitted first, I look at the languages configured by the user and sort accordingly. In #6163 I showed this example:

Finding tags for album "YOASOBI - 夜に駆ける".
  Candidates:
  1. (97.6%) YOASOBI - Yoru ni Kakeru
             ≠ media
             MusicBrainz, Digital Media, 2019, XW, None, YOASOBI-001, None
  2. (97.6%) YOASOBI - Racing Into The Night
             ≠ media
             MusicBrainz, Digital Media, 2019, XW, None, YOASOBI-001, None
  3. (97.6%) YOASOBI - 夜に駆ける
             ≠ media
             MusicBrainz, Digital Media, 2019, XW, None, YOASOBI-001, None

Those are:

  1. A transliteration (pseudo-release)
  2. A translation (pseudo-release)
  3. Original (official release)

By specifying languages as jp en, I got the transliteration first. If I changed to en jp I should get the translation first.

@snejus
Copy link
Member

snejus commented Jan 21, 2026

Makes sense, thanks!

I guess, my largest concern here is separation of responsibilities. The way things stand, musicbrainz took over some responsibilities from distance regarding prioritising matches. The prioritisation should ideally be happening within the distance() function, where the solution is generic enough to take care of reimports as well.

One idea I'm thinking of is: what if we extend AlbumInfo and TrackInfo with fields artist_alt, title_alt etc. that store the alternatives/translations while we keep the original artist and title in tact to make sure translations are not penalized. Then, when we apply metadata, we can set artist to artist_alt when it's available, same for other fields.

Does this sort of direction make sense or should I expand a bit more?

@asardaes
Copy link
Contributor Author

@snejus I get the point, but I feel there's more nuance to it. In my head I see it as different prioritisations. Distance calculations are concerned with the match between existing file metadata and data provided by some online source, whereas the logic here is more about prioritisation within a release group from MusicBrainz.

I think "release group" is important to think about here. Concretely, I cared more about translations and transliterations, even though MB has such groups also for remasters and other variations. To me, moving responsibility for this kind of prioritisation outside of the plugin makes sense if that's a valid use case for different data sources. If none of the other data sources can benefit from supporting translation/transliteration priorities in a generic way, then I think it's valid to leave that responsibility inside the plugin because it's only sensible for that plugin's data source.

I'm not very familiar with all the data sources out there, so maybe they do have similar concepts for release groups and pseudo releases, but do you know if that's the case?

@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch from f541d62 to 7cc5f0f Compare January 23, 2026 20:48
@snejus snejus modified the milestones: 2.6.0, 2.7.0 Feb 1, 2026
@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch 2 times, most recently from 25ad01b to 647949f Compare February 1, 2026 15:44
@asardaes asardaes force-pushed the feature/musicbrainz-pseudo-releases branch from 647949f to 6c84ede Compare February 1, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

musicbrainz plugin Pull requests that are plugins related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants