Stop perpetually writing mb_artistid, mb_albumartistid and albumtypes fields (#5540)

This PR fixes an issue where the `beet write` command repeatedly shows
differences for certain fields (`mb_artistid`, `mb_albumartistid`,
`albumtype`) even after writing the tags.
This happens because these fields are actually stored as lists in the
media files (`mb_artistids`, `mb_albumartistids`, `albumtypes`), but
beets maintains both single and list versions in its database.

This PR addresses this issue in a non-invasive way: the fix ensures
consistency between single fields and their list counterparts by:
1. When setting a single field value, making it the first element of the
corresponding list
2. When setting a list, using its first element as the single field
value

This resolves long-standing issues #5265, #5371, and #4715 where users
experienced persistent "differences" in these fields despite writing
tags multiple times.

Changes:
- Added `ensure_consistent_list_fields()` function to synchronize field
pairs
- Applied this function during metadata application
- Added tests for all field combinations

Fixes #5265, #5371, #4715

**Note**: your music needs to be reimported with `beet import -LI` or
synchronised with `beet mbsync` in order to fix these fields!

beets/autotag/__init__.py

@@ -18,9 +18,11 @@ from collections.abc import Mapping, Sequence
 from typing import Union
 
 from beets import config, logging
-from beets.library import Album, Item
+from beets.library import Album, Item, LibModel
 
 # Parts of external interface.
+from beets.util import unique_list
+
 from .hooks import AlbumInfo, AlbumMatch, Distance, TrackInfo, TrackMatch
 from .match import (
     Proposal,
@@ -119,6 +121,48 @@ def _apply_metadata(
         db_obj[field] = value
 
 
+def correct_list_fields(m: LibModel) -> None:
+    """Synchronise single and list values for the list fields that we use.
+
+    That is, ensure the same value in the single field and the first element
+    in the list.
+
+    For context, the value we set as, say, ``mb_artistid`` is simply ignored:
+    Under the current :class:`MediaFile` implementation, fields ``albumtype``,
+    ``mb_artistid`` and ``mb_albumartistid`` are mapped to the first element of
+    ``albumtypes``, ``mb_artistids`` and ``mb_albumartistids`` respectively.
+
+    This means setting ``mb_artistid`` has no effect. However, beets
+    functionality still assumes that ``mb_artistid`` is independent and stores
+    its value in the database. If ``mb_artistid`` != ``mb_artistids[0]``,
+    ``beet write`` command thinks that ``mb_artistid`` is modified and tries to
+    update the field in the file. Of course nothing happens, so the same diff
+    is shown every time the command is run.
+
+    We can avoid this issue by ensuring that ``mb_artistid`` has the same value
+    as ``mb_artistids[0]``, and that's what this function does.
+
+    Note: :class:`Album` model does not have ``mb_artistids`` and
+    ``mb_albumartistids`` fields therefore we need to check for their presence.
+    """
+
+    def ensure_first_value(single_field: str, list_field: str) -> None:
+        """Ensure the first ``list_field`` item is equal to ``single_field``."""
+        single_val, list_val = getattr(m, single_field), getattr(m, list_field)
+        if single_val:
+            setattr(m, list_field, unique_list([single_val, *list_val]))
+        elif list_val:
+            setattr(m, single_field, list_val[0])
+
+    ensure_first_value("albumtype", "albumtypes")
+
+    if hasattr(m, "mb_artistids"):
+        ensure_first_value("mb_artistid", "mb_artistids")
+
+    if hasattr(m, "mb_albumartistids"):
+        ensure_first_value("mb_albumartistid", "mb_albumartistids")
+
+
 def apply_item_metadata(item: Item, track_info: TrackInfo):
     """Set an item's metadata from its matched TrackInfo object."""
     item.artist = track_info.artist
@@ -136,6 +180,7 @@ def apply_item_metadata(item: Item, track_info: TrackInfo):
         item.mb_artistids = track_info.artists_ids
 
     _apply_metadata(track_info, item)
+    correct_list_fields(item)
 
     # At the moment, the other metadata is left intact (including album
     # and track number). Perhaps these should be emptied?
@@ -144,6 +189,7 @@ def apply_item_metadata(item: Item, track_info: TrackInfo):
 def apply_album_metadata(album_info: AlbumInfo, album: Album):
     """Set the album's metadata to match the AlbumInfo object."""
     _apply_metadata(album_info, album)
+    correct_list_fields(album)
 
 
 def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
@@ -267,3 +313,5 @@ def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
             item,
             nullable_fields=config["overwrite_null"]["track"].as_str_seq(),
         )
+
+        correct_list_fields(item)

beets/util/__init__.py

@@ -39,6 +39,7 @@ from typing import (
     Any,
     AnyStr,
     Callable,
+    Iterable,
     NamedTuple,
     TypeVar,
     Union,
@@ -1126,3 +1127,8 @@ def get_temp_filename(
 
     _, filename = tempfile.mkstemp(dir=tempdir, prefix=prefix, suffix=suffix)
     return bytestring_path(filename)
+
+
+def unique_list(elements: Iterable[T]) -> list[T]:
+    """Return a list with unique elements in the original order."""
+    return list(dict.fromkeys(elements))

docs/changelog.rst

@@ -19,6 +19,14 @@ Bug fixes:
   have before the introduction of Poetry.
   :bug:`5531`
   :bug:`5526`
+* :ref:`write-cmd`: Fix the issue where for certain files differences in
+  ``mb_artistid``, ``mb_albumartistid`` and ``albumtype`` fields are shown on
+  every attempt to write tags. Note: your music needs to be reimported with
+  ``beet import -LI`` or synchronised with ``beet mbsync`` in order to fix
+  this!
+  :bug:`5265`
+  :bug:`5371`
+  :bug:`4715`
 
 For packagers:
 

test/test_autotag.py

@@ -20,7 +20,7 @@ import unittest
 import pytest
 
 from beets import autotag, config
-from beets.autotag import AlbumInfo, TrackInfo, match
+from beets.autotag import AlbumInfo, TrackInfo, correct_list_fields, match
 from beets.autotag.hooks import Distance, string_dist
 from beets.library import Item
 from beets.test.helper import BeetsTestCase
@@ -1040,3 +1040,36 @@ class StringDistanceTest(unittest.TestCase):
     def test_accented_characters(self):
         dist = string_dist("\xe9\xe1\xf1", "ean")
         assert dist == 0.0
+
+
+@pytest.mark.parametrize(
+    "single_field,list_field",
+    [
+        ("mb_artistid", "mb_artistids"),
+        ("mb_albumartistid", "mb_albumartistids"),
+        ("albumtype", "albumtypes"),
+    ],
+)
+@pytest.mark.parametrize(
+    "single_value,list_value",
+    [
+        (None, []),
+        (None, ["1"]),
+        (None, ["1", "2"]),
+        ("1", []),
+        ("1", ["1"]),
+        ("1", ["1", "2"]),
+        ("1", ["2", "1"]),
+    ],
+)
+def test_correct_list_fields(
+    single_field, list_field, single_value, list_value
+):
+    """Ensure that the first value in a list field matches the single field."""
+    data = {single_field: single_value, list_field: list_value}
+    item = Item(**data)
+
+    correct_list_fields(item)
+
+    single_val, list_val = item[single_field], item[list_field]
+    assert (not single_val and not list_val) or single_val == list_val[0]