Skip to content

Conversation

@Morikko
Copy link

@Morikko Morikko commented Dec 23, 2025

Description

This PR add support for aliases to releases, release-groups and recordings.

This PR is a must have (IMO at least) for people that listen to Japanese, Chinese and other songs that has other symbols for letters. With this, not only the artist name will use the alias if available, but now the album and track name will also use aliases.

Follow up from #5277 based on the recent new mechanism to query musicbrainz (#6052).

To Do

  • Documentation
  • Changelog
  • Tests

API examples

@Morikko Morikko requested a review from a team as a code owner December 23, 2025 11:19
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 2 issues, and left some high level feedback:

  • The logic for choosing an alias over the default title is now duplicated for recordings, releases, and release groups; consider extracting a small helper (e.g., _title_with_alias(obj, fallback_key="title")) to centralize this behavior and reduce the chance of inconsistent future changes.
  • In album_info, _preferred_alias(track["recording"].get("aliases", ())) is computed separately from the earlier track_info call that already chooses the recording alias; consider reusing or passing through that decision to avoid recomputing and to make the precedence between recording aliases and track titles clearer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The logic for choosing an alias over the default title is now duplicated for recordings, releases, and release groups; consider extracting a small helper (e.g., `_title_with_alias(obj, fallback_key="title")`) to centralize this behavior and reduce the chance of inconsistent future changes.
- In `album_info`, `_preferred_alias(track["recording"].get("aliases", ()))` is computed separately from the earlier `track_info` call that already chooses the recording alias; consider reusing or passing through that decision to avoid recomputing and to make the precedence between recording aliases and track titles clearer.

## Individual Comments

### Comment 1
<location> `test/plugins/test_musicbrainz.py:183-187` </location>
<code_context>
         disambiguation=None,
         remixer=False,
         multi_artist_credit=False,
+        aliases=None,
     ):
</code_context>

<issue_to_address>
**issue (testing):** Track alias tests may not exercise the actual alias lookup path (aliases are attached to the track, not the recording).

Production code reads aliases from `recording.get("aliases", ())`, but `_make_track` currently sets `track["aliases"]`, not `track["recording"]["aliases"]`. Unless something later moves this field, tests using `_make_track(..., aliases=...)` aren’t exercising the real alias lookup path. Consider either assigning aliases to `track["recording"]["aliases"]` in `_make_track`, or having tests build the `recording` dict (with an `aliases` field) explicitly so they validate `_preferred_alias(recording["aliases"])` as intended.
</issue_to_address>

### Comment 2
<location> `docs/changelog.rst:39` </location>
<code_context>
       resolve differences in metadata source styles.
 - :doc:`plugins/spotify`: Added support for multi-artist albums and tracks,
       saving all contributing artists to the respective fields.
+- :doc:`plugins/musicbrainz`: Use title alias for releases, release groups and
+     recordings.

 Bug fixes:
</code_context>

<issue_to_address>
**suggestion (typo):** Use plural "aliases" when referring to multiple releases, release groups, and recordings.

Consider updating the sentence to: “Use title aliases for releases, release groups, and recordings.” for clearer grammar.

```suggestion
- :doc:`plugins/musicbrainz`: Use title aliases for releases, release groups, and
```
</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.

Comment on lines 183 to 187
multi_artist_credit=False,
aliases=None,
):
track = {
"title": title,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (testing): Track alias tests may not exercise the actual alias lookup path (aliases are attached to the track, not the recording).

Production code reads aliases from recording.get("aliases", ()), but _make_track currently sets track["aliases"], not track["recording"]["aliases"]. Unless something later moves this field, tests using _make_track(..., aliases=...) aren’t exercising the real alias lookup path. Consider either assigning aliases to track["recording"]["aliases"] in _make_track, or having tests build the recording dict (with an aliases field) explicitly so they validate _preferred_alias(recording["aliases"]) as intended.

resolve differences in metadata source styles.
- :doc:`plugins/spotify`: Added support for multi-artist albums and tracks,
saving all contributing artists to the respective fields.
- :doc:`plugins/musicbrainz`: Use title alias for releases, release groups and
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (typo): Use plural "aliases" when referring to multiple releases, release groups, and recordings.

Consider updating the sentence to: “Use title aliases for releases, release groups, and recordings.” for clearer grammar.

Suggested change
- :doc:`plugins/musicbrainz`: Use title alias for releases, release groups and
- :doc:`plugins/musicbrainz`: Use title aliases for releases, release groups, and

@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.20%. Comparing base (5d1210a) to head (228f47e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6231      +/-   ##
==========================================
- Coverage   68.20%   68.20%   -0.01%     
==========================================
  Files         138      138              
  Lines       18775    18774       -1     
  Branches     3173     3171       -2     
==========================================
- Hits        12805    12804       -1     
  Misses       5296     5296              
  Partials      674      674              
Files with missing lines Coverage Δ
beetsplug/musicbrainz.py 81.10% <100.00%> (-0.05%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Morikko Morikko mentioned this pull request Dec 23, 2025
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.

1 participant