Skip to content

Add bitrate-based 'upgrade' duplicate_action#6842

Open
arsaboo wants to merge 9 commits into
beetbox:masterfrom
arsaboo:upgrade
Open

Add bitrate-based 'upgrade' duplicate_action#6842
arsaboo wants to merge 9 commits into
beetbox:masterfrom
arsaboo:upgrade

Conversation

@arsaboo

@arsaboo arsaboo commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Adds an upgrade option for duplicate_action (config-only, not offered in the interactive ask prompt). When set, each newly imported track is compared against any existing duplicate with the same duplicate_keys:

  • A track only replaces its old counterpart if it has a higher bitrate.
  • Tracks with no old counterpart (e.g., filling in a previously incomplete album) are always added.
  • The rest of the existing album is left untouched, and the kept tracks join that same album rather than forming a new, separate one.

This picks up the idea from #4471 by @Serene-Arc, which stalled. The importer has been substantially refactored since then, so many of the issues identified earlier are not relevant, but the design follows what was agreed there: config-only, bitrate-based comparison, no per-track album splitting as a decision mechanism.

Would appreciate some 👀 on this.

  • 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.)

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.23529% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.69%. Comparing base (8778d0e) to head (4667c76).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
beets/importer/tasks.py 87.77% 4 Missing and 7 partials ⚠️
beets/importer/actions.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6842      +/-   ##
==========================================
+ Coverage   75.63%   75.69%   +0.05%     
==========================================
  Files         163      163              
  Lines       21317    21417     +100     
  Branches     3361     3385      +24     
==========================================
+ Hits        16124    16211      +87     
- Misses       4403     4408       +5     
- Partials      790      798       +8     
Files with missing lines Coverage Δ
beets/importer/stages.py 93.41% <100.00%> (+0.37%) ⬆️
beets/importer/actions.py 93.75% <50.00%> (+0.20%) ⬆️
beets/importer/tasks.py 90.75% <87.77%> (-0.49%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arsaboo
arsaboo marked this pull request as ready for review July 15, 2026 22:59
@arsaboo
arsaboo requested a review from a team as a code owner July 15, 2026 22:59
@arsaboo arsaboo closed this Jul 16, 2026
@arsaboo arsaboo reopened this Jul 16, 2026
@arsaboo
arsaboo marked this pull request as draft July 16, 2026 12:40
@arsaboo arsaboo mentioned this pull request Jul 16, 2026
3 tasks
@Serene-Arc Serene-Arc self-assigned this Jul 17, 2026
@arsaboo
arsaboo marked this pull request as ready for review July 18, 2026 23:18
Comment thread beets/importer/tasks.py Outdated
Comment thread beets/importer/tasks.py
Comment on lines +435 to +437
# else: a second old album also has survivors; an item can
# only belong to one album, so leave it untouched rather than
# arbitrarily choosing between two candidates.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need a better option than just ignoring it. If you have multiple albums, it won't be clear which gets upgraded and it might result in a downgrade in some situations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the safest behavior would be to automatically decline the upgrade in that case (falling back to skip), because any merge would otherwise pick an arbitrary destination. What is the preferred rule you have in mind here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One option would be to keep the assignments for duplicate albums from the upgrade. That means that multiple, different files could be upgraded in different on-filesystem albums, even if they are duplicate items

For example, let's say that we have two existing albums: Album A, which is mostly 320kbps with some 24bit FLAC; and Album B, which is just 320kbps. Now if we import a 16bit FLAC album as a replacement with upgrade, then we will have a problem. Almost all of the files will be counted as an upgrade with both albums. But because of Album B, all of the files will be included in the upgrade list.

Now, if Album A is returned first because of the dict (which I believe will actually be somewhat random) then we will actually be downgrading all those 24bit FLAC files because we can't distinguish between which versions of existing albums we should be upgrading.

But if we keep those assignments, we can upgrade each duplicate album separately to ensure that, overall, there is no downgrades.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went with your suggested approach: resolve_upgrade now runs independently against each duplicate album (as if it were the only one), and only the album with the most overlap (most superseded tracks, then most kept tracks, then lowest album ID as a tie-breaker) is treated as the upgrade target. Every other candidate album, including any higher-bitrate tracks in it, is untouched, since a given new track can only ever join one physical album.

For your example: Album A (mostly 320kbps + some 24-bit FLAC) and Album B (all 320kbps) both matching a 16-bit FLAC reimport — Album B would be resolved and upgraded on its own since it has more overlap, and Album A's 24-bit FLAC tracks are never touched or considered for supersession at all.

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.

2 participants