Refresh flexible MusicBrainz metadata on reimport #6064

arsaboo · 2025-10-02T12:25:38Z

Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)

…ta from new releases replaces stale values

Copilot

Pull Request Overview

This PR addresses issue #6036 by ensuring that flexible MusicBrainz metadata fields are refreshed during reimport operations, allowing format changes to be properly applied.

Adds specific MusicBrainz fields to the reimport fresh fields lists
Implements automatic extension of reimport fresh fields during plugin initialization
Updates changelog to document the bug fix

Reviewed Changes

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

File	Description
docs/changelog.rst	Documents the bug fix for refreshing flexible MusicBrainz metadata on reimport
beetsplug/musicbrainz.py	Implements the core functionality to extend reimport fresh fields with MusicBrainz-specific fields

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

sourcery-ai

Hey there - I've reviewed your changes and they look great!

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.}

beetsplug/musicbrainz.py

snejus

We need a different approach - this is not a good idea

beetsplug/musicbrainz.py

semohr · 2025-10-04T08:12:12Z

Do we want to rewrite this now? Sadly the current logic is how it was intended for some reason originally.

It alsso seems a bit strange to me that we have to define these field in the first place. An opt out list would be more appropriet.

We should also check why and when this was introduced.

snejus · 2025-10-04T08:55:25Z

We can (and should) simply extend those list constants explicitly with new fields. They aren't specific to musicbrainz only - for example, beetcamp returns both data_url and media.

JOJ0 · 2025-10-04T08:59:39Z

Do we want to rewrite this now? Sadly the current logic is how it was intended for some reason originally.

It alsso seems a bit strange to me that we have to define these field in the first place. An opt out list would be more appropriet.

AFAIR this is an opt out list: Everything should always be refreshed on a reimport, except not for those fields (because it would be just wrong for them)

We should also check why and when this was introduced.

I introduced this a couple of years ago. Not much time right now to reread what was all the reasons back then but I at least looked up the PR for you guys :-) #4735

snejus · 2025-10-04T09:06:45Z

I think I see this logic in play in my importlog:

Reimported album 9355. Not preserving flexible attributes ['data_source']. Path: /run/media/sarunas/music/Music/soma/uun
Reimported album 9355. Preserving attribute ['added']. Path: /run/media/sarunas/music/Music/soma/uun
Reimported album 9355. Preserving flexible attributes ['art_source']. Path: /run/media/sarunas/music/Music/soma/uun
Reimported item 43942. Preserving attribute ['added']. Path: /run/media/sarunas/music/Music/soma/uun/02_crueltyisnothingnew.flac
Reimported item 43942. Not preserving flexible attributes ['data_source']. Path: /run/media/sarunas/music/Music/soma/uun/02_crueltyisnothingnew.flac
Reimported item 43942. Preserving flexible attributes ['lastfm_play_count', 'bandcamp_artist_id', 'art_source', 'bandcamp_album_id', 'times_bought', 'rating', 'artpath', 'price', 'helicopta', 'play_count', 'art_url', 'bandcamp_track_id', 'currency', 'last_played']. Path: /run/media/sarunas/music/Music/soma/uun/02_crueltyisnothingnew.flac
Reimported item 43943. Preserving attribute ['added']. Path: /run/media/sarunas/music/Music/soma/uun/03_givethemrope.flac
Reimported item 43943. Not preserving flexible attributes ['data_source']. Path: /run/media/sarunas/music/Music/soma/uun/03_givethemrope.flac
Reimported item 43943. Preserving flexible attributes ['lastfm_play_count', 'skip_count', 'bandcamp_artist_id', 'art_source', 'bandcamp_album_id', 'times_bought', 'rating', 'artpath', 'price', 'play_count', 'art_url', 'bandcamp_track_id', 'currency', 'last_played']. Path: /run/media/sarunas/music/Music/soma/uun/03_givethemrope.flac

JOJ0 · 2025-10-04T09:09:12Z

We can (and should) simply extend those list constants explicitly with new fields. They aren't specific to musicbrainz only - for example, beetcamp returns both data_url and media.

Explicitly! Yes please! Keep it readable. If there would be something that is absolutely plugin specific, we could simply note it in a comment?

JOJ0 · 2025-10-04T09:10:36Z

I think I see this logic in play in my importlog:

exactly! this is it!

snejus · 2025-10-04T09:25:22Z

For now, I think, we can just extend those lists and merge this PR. Longer term, we need to find why flexible attribute update logic differs from db attribute update.

DB attributes are always updated with values other than None. None indicates absence of a field, and thus this field is ignored in updates. This can be changed by overwrite_null.album and overwrite_null.track configuration: when a field is present in this list, None means empty and the previous value is cleared on updates.

arsaboo · 2025-10-05T22:12:59Z

Here's another discussion on this topic #4788

So, how do we want to resolve this PR? I am happy if one of you would like to take this forward.

arsaboo · 2025-10-06T14:57:45Z

@snejus, I have updated the list for now. We can merge this for now and have a separate discussion about the changes required to fix the underlying issue. It may also not be a bad idea for you to fix this in #6052
@semohr I can create a separate PR for #6076 once this one is done (so I know what is needed)

snejus

Remove these changes from musicbrainz.py and add the fields to the lists in importer.tasks instead.

arsaboo · 2025-10-07T22:13:45Z

@snejus check now.

This may be beyond the scope of the current PR, but I assume that title, album, albumartist, etc. are still not updated on reimport. I don't see any reason why that should be the case. Like you said

Longer term, we need to find why flexible attribute update logic differs from db attribute update.

snejus · 2025-10-07T22:51:04Z

@arsaboo it's impossible to have a flexible attribute named after any of the database fields. If you try to set item.title = "new_title", you will update the database field item.title. See the following logic under class Model:

    def _setitem(self, key, value):
        """Assign the value for a field, return whether new and old value
        differ.
        """
        # Choose where to place the value.
        if key in self._fields:
            source = self._values_fixed
        else:
            source = self._values_flex

_values_fixed property holds database fields like album, title, albumartist, artist etc., while _values_flex contains everything else.

beets/importer/tasks.py

…M and REIMPORT_FRESH_FIELDS_ALBUM definitions

semohr · 2025-10-08T11:05:25Z

Longer term, we need to find why flexible attribute update logic differs from db attribute update.

Before merging, could you please create an issue summarizing the situation, what’s different, and how or why we might want to change it? That way we can keep track of it and organize our thoughts for future work. 😉

arsaboo · 2025-10-08T12:38:39Z

Longer term, we need to find why flexible attribute update logic differs from db attribute update.

Before merging, could you please create an issue summarizing the situation, what’s different, and how or why we might want to change it? That way we can keep track of it and organize our thoughts for future work. 😉

HAHA....I wish I had that clarity 🤣

I honestly did not even completely follow @snejus comment. @JOJ0 and @snejus are the closest to this, and I will let them summarize and resolve this in a subsequent PR.

semohr · 2025-10-08T12:41:08Z

I kinda lost the sauce too. That's why im asking :D

snejus · 2025-10-08T13:01:13Z

I often fail to communicate properly... I hope that this helps, guys, see: https://www.perplexity.ai/search/can-you-explain-what-is-snejus-eO3.NbE8TM.d.1FOARnPqA

arsaboo · 2025-10-08T13:10:04Z

I often fail to communicate properly... I hope that this helps, guys, see: https://www.perplexity.ai/search/can-you-explain-what-is-snejus-eO3.NbE8TM.d.1FOARnPqA

It was not your explanation, but my lack of understanding. This helps. To summarize (and for posterity),

Database Fields vs. Flexible Attributes: Beets treats two types of fields differently:

Database Fields (_values_fixed):

These are predefined schema fields like title, album, albumartist, artist, etc.
Stored directly in the SQLite database tables as columns
Always updated during reimport when new values are found (unless the value is None)
Updates are controlled by the overwrite_null configuration for handling None values

Flexible Attributes (_values_flex)

Custom fields that users can add (like mood, play_count, etc.)
Stored in separate key-value tables (album_attributes and item_attributes)
Follow different update logic during reimport operations
Subject to the REIMPORT_FRESH_FIELDS lists that determine which flexible attributes get refreshed

The REIMPORT_FRESH_FIELDS lists act as an allowlist - only flexible attributes in these lists get updated during reimport, while all others are preserved.

Let me know if anything else is required in this PR.

snejus · 2025-10-08T13:32:25Z

There's no need to add releasegroup_id - see the last section in https://www.perplexity.ai/search/can-you-explain-what-is-snejus-eO3.NbE8TM.d.1FOARnPqA#2.

snejus

Thanks for your patience @arsaboo. We ended up having a very involved discussion relative to the size of this tiny change.

This only proves that this is one of the most complicated and opaque parts of the codebase and that we need to prioritize improving this to make it clearer to everyone.

arsaboo · 2025-10-09T18:58:03Z

Thanks for your patience @arsaboo. We ended up having a very involved discussion relative to the size of this tiny change.

This only proves that this is one of the most complicated and opaque parts of the codebase and that we need to prioritize improving this to make it clearer to everyone.

Thanks for patiently explaining the issue. I feel I have a (slightly) better understanding :)

Extends the importer’s “fresh on reimport” lists so album flex metada…

93c8950

…ta from new releases replaces stale values

arsaboo requested a review from a team as a code owner October 2, 2025 12:25

Copilot AI review requested due to automatic review settings October 2, 2025 12:25

Copilot AI reviewed Oct 2, 2025

View reviewed changes

sourcery-ai bot reviewed Oct 2, 2025

View reviewed changes

update imports

20820fd

arsaboo mentioned this pull request Oct 2, 2025

import: Media type won't get updated when retagging album #6036

Closed

semohr reviewed Oct 2, 2025

View reviewed changes

beetsplug/musicbrainz.py Outdated Show resolved Hide resolved

semohr approved these changes Oct 2, 2025

View reviewed changes

snejus requested changes Oct 4, 2025

View reviewed changes

snejus reviewed Oct 4, 2025

View reviewed changes

beetsplug/musicbrainz.py Outdated Show resolved Hide resolved

snejus reviewed Oct 4, 2025

View reviewed changes

beetsplug/musicbrainz.py Outdated Show resolved Hide resolved

semohr mentioned this pull request Oct 6, 2025

$discogs_albumid field not updating in database #6076

Open

Added releasegroup_id to _MB_REIMPORT_FRESH_FIELDS_ITEM list

c5f5ffc

snejus requested changes Oct 7, 2025

View reviewed changes

arsaboo added 2 commits October 7, 2025 18:02

Added the MusicBrainz flex fields directly to the importer

c59aff9

Merge remote-tracking branch 'upstream/master' into mb_fix

6c06f2a

snejus reviewed Oct 7, 2025

View reviewed changes

beets/importer/tasks.py Outdated Show resolved Hide resolved

beets/importer/tasks.py Outdated Show resolved Hide resolved

Refactor reimport fresh fields: consolidate REIMPORT_FRESH_FIELDS_ITE…

8787495

…M and REIMPORT_FRESH_FIELDS_ALBUM definitions

Merge remote-tracking branch 'upstream/master' into mb_fix

79494b8

remove releasegroup_id

92a2233

snejus approved these changes Oct 9, 2025

View reviewed changes

snejus merged commit fe98841 into beetbox:master Oct 9, 2025
17 checks passed

arsaboo deleted the mb_fix branch October 9, 2025 18:58

Refresh flexible MusicBrainz metadata on reimport #6064

Refresh flexible MusicBrainz metadata on reimport #6064

Uh oh!

Conversation

arsaboo commented Oct 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

snejus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

semohr commented Oct 4, 2025

Uh oh!

snejus commented Oct 4, 2025

Uh oh!

JOJ0 commented Oct 4, 2025

Uh oh!

snejus commented Oct 4, 2025

Uh oh!

JOJ0 commented Oct 4, 2025

Uh oh!

JOJ0 commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snejus commented Oct 4, 2025

Uh oh!

arsaboo commented Oct 5, 2025

Uh oh!

arsaboo commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snejus left a comment

Choose a reason for hiding this comment

Uh oh!

arsaboo commented Oct 7, 2025

Uh oh!

snejus commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

semohr commented Oct 8, 2025

Uh oh!

arsaboo commented Oct 8, 2025

Uh oh!

semohr commented Oct 8, 2025

Uh oh!

snejus commented Oct 8, 2025

Uh oh!

arsaboo commented Oct 8, 2025

Uh oh!

snejus commented Oct 8, 2025

Uh oh!

snejus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arsaboo commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JOJ0 commented Oct 4, 2025 •

edited

Loading

arsaboo commented Oct 6, 2025 •

edited

Loading

snejus commented Oct 7, 2025 •

edited

Loading