Import MusicBrainz composer/lyricist/arranger ids

[Maxr1998]

Description

Updates the MusicBrainz plugin to also import MBIDs for composers/lyricists/arrangers, and adds them as multi-valued fields.

Closes #5698.

To Do

• Documentation. (not required for this change)
• Changelog
• Tests

[github-actions]

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.

[Copilot]

Pull Request Overview

This PR enhances the MusicBrainz plugin by importing MBIDs for composers, lyricists, and arrangers and storing them as multi-valued fields. The changes span updates to the track information processing logic, the library schema, and the autotag hooks to accommodate the new fields.

• Updated track_info in the MusicBrainz plugin to collect both names and MBIDs for lyricists, composers, and arrangers.
• Modified the library model to add new multi-valued fields for the additional MBIDs.
• Adjusted autotag hooks and initialization to integrate the new fields into the metadata processing.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
beetsplug/musicbrainz.py	Updated to extract and store MBIDs alongside names for lyricists, composers, and arrangers
beets/library.py	Added new schema entries for multi-valued MBID fields
beets/autotag/hooks.py	Updated constructor parameters and initialization for new lyricist, composer, and arranger fields
beets/autotag/init.py	Mapped new track information fields to corresponding item fields

Comments suppressed due to low confidence (2)

beetsplug/musicbrainz.py:485

• [nitpick] Consider adding inline comments to explain the rationale for storing both the concatenated string (for legacy support) and the list of lyricists (for enhanced functionality).

        if lyricists:

beets/library.py:544

• Ensure that tests are added to verify the correct import and synchronization of the new multi-valued fields for lyricists, composers, and arrangers.

        "lyricists": types.MULTI_VALUE_DSV,

[semohr]

Heya, thank you for the PR!

I'm not too sure if we want to mirror more ids from musicbrainz in the autotag functionalities if they are not actively used in the lookup process. Especially since mb was moved into a plugin and other metadata sources do not supply the same information.

Maybe instead of specifically defining all different id properties, we can find a more generic way to allow adding arbitrary IDs (Optimally without the need to manually edit the autotag module)? Seems to me like we might have aquie some duplication for multivalue id fields.

I would be highly interested in @snejus thoughts on this. Let's discuss this a bit before we continue with the merge.

Best, Sebastian

[Maxr1998]

I'm not too sure if we want to mirror more ids from musicbrainz in the autotag functionalities if they are not actively used in the lookup process. Especially since mb was moved into a plugin and other metadata sources do not supply the same information.

Maybe instead of specifically defining all different id properties, we can find a more generic way to allow adding arbitrary IDs (Optimally without the need to manually edit the autotag module)? Seems to me like we might have aquie some duplication for multivalue id fields.

Yes, this is a good point. In my original issue in #5698 I wrote a separate plugin that contained all these properties inside, we can likely do the same for the new musicbrainz plugin. However, the we definitely need #5833 first because otherwise those fields can't store multiple values as a list.

[Maxr1998]

Rebased my changes now. Instead of adding the new fields to beets itself (library, hooks, autotag), putting them only in the plugin should be possible now after my previous PR has been merged.

@snejus what do you think? Additionally, I'm still a bit confused as to why there's both a artists_ids and mb_artistids field. Should this be mirrored for composers/lyricists/arrangers? What should be managed by the plugin, what shouldn't?

[semohr]

I'm still a bit confused as to why there's both a artists_ids and mb_artistids field. Should this be mirrored for composers/lyricists/arrangers? What should be managed by the plugin, what shouldn't?

There’s also mb_albumartistid. The main database tables are in need of a cleanup and unification; the naming is inconsistent, as you’ve probably noticed. From a database normalization perspective, we’re carrying around a lot of redundant fields that really should be consolidated.

As for composers/lyricists/arrangers: duplicating these fields across artist_ids vs. mb_* variants probably isn’t the right long-term direction. Ideally the core schema would handle the distinction cleanly, with plugins consuming or extending it rather than re-implementing.

For this specific case, I think it’s fine to add here for now and revisit later. I don’t think we should try to tackle that larger issue in this PR.

[Maxr1998]

Ok, that makes sense.

So, should the fields still be moved to the plugin, or can I keep them in the beets core for now?

[semohr]

In my opinion moving the fields to the plugin itself would be preferred and will spare us some work down the line. How would you achieve that?

[Maxr1998]

You can basically just declare them within the plugin and they'll be picked up by beets. I wonder though if that makes sense for all of these fields. A more generic composers_ids might still belong in the core so that other plugins can also provide that value. The redundant mb_composerids should be moved to the plugin, or eventually just removed (and maybe not even added in this PR).

[Maxr1998]

@semohr did you have some time to further look into this or discuss the changes? I can resolve the current conflicts, but I see there's some more refactoring going on presently (#6052), which would cause conflicts again if this can't be merged before.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.27%. Comparing base (4e08403) to head (39b8a8e).
⚠️ Report is 8 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5847   +/-   ##
=======================================
  Coverage   70.26%   70.27%           
=======================================
  Files         147      147           
  Lines       18752    18756    +4     
  Branches     3059     3059           
=======================================
+ Hits        13177    13181    +4     
  Misses       4927     4927           
  Partials      648      648           

Files with missing lines	Coverage Δ
beets/autotag/hooks.py	98.87% <100.00%> (+0.01%)	⬆️
beets/library/models.py	87.10% <ø> (ø)

🚀 New features to boost your workflow:

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

[Maxr1998]

I updated the PR once more and removed the duplicate mb_ tags for now, only adding the multi-valued, unbranded tags. (cc @semohr @snejus)

[Maxr1998]

Rebased the changes once more and fixed the changelog conflict 🙂

[Maxr1998]

Is there anything else I can do to get this reviewed and merged?

[Maxr1998]

Added remixer ids as well now. Gonna fix the tests in a bit.

[Maxr1998]

There we go. Everything is ready to review and merge now.

[snejus]

Cool, will have a look at this later today!

[snejus]

A pair of small comments.

[snejus]

Thanks!

[Maxr1998]

Thanks for the review and all the feedback!