Replace old cover art instead of creating suffixed new entries / fetchart

[snejus]

Summary

• Fix duplicate album art files (cover.2.jpg, cover.3.jpg, ...) accumulating when re-importing albums with the fetchart plugin enabled.
• ImportTask.remove_duplicates now operates at the album level and explicitly deletes album art files from disk when removing duplicate albums during import.
• Album.set_art now unconditionally removes old art and any existing file at the destination, instead of appending a numeric suffix via unique_path. This matches the method's documented behavior of "replacing any existing art".
• SingletonImportTask gets its own remove_duplicates override preserving the original item-level behavior, since find_duplicates returns items (not albums) for singletons.

Root cause

Two issues contributed to the bug:

1. remove_duplicates never deleted album art files.

When a user imports a duplicate album and chooses "Remove old", remove_duplicates iterated over items and called item.remove(). When the last item was removed, this cascaded to Album.remove(delete=False), which skipped art deletion due to delete=False. The old cover.jpg remained on disk as an orphan.

2. set_art created unique paths instead of replacing.

set_art only removed the old art file when oldart == artdest (exact byte equality). When oldart was None (new album, no inherited artpath) or pointed to a different path (e.g. different extension), the existing file was left in place and unique_path generated cover.2.jpg.

Fixes #6205

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

[codecov]

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.77%. Comparing base (7edf27b) to head (4445830).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
beets/importer/tasks.py	91.30%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6554      +/-   ##
==========================================
+ Coverage   71.76%   71.77%   +0.01%     
==========================================
  Files         159      159              
  Lines       20515    20531      +16     
  Branches     3262     3266       +4     
==========================================
+ Hits        14722    14737      +15     
  Misses       5105     5105              
- Partials      688      689       +1     

Files with missing lines	Coverage Δ
beets/library/models.py	87.01% <100.00%> (ø)
beets/importer/tasks.py	90.82% <91.30%> (+0.08%)	⬆️

🚀 New features to boost your workflow:

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

[Copilot]

Pull request overview

PR try stop cover.2.jpg / cover.3.jpg pile-up when re-import with fetchart. PR make old art get removed instead of making suffixed new file.

Changes:

• Change Album.set_art to always remove old art and any existing destination file (no unique_path).
• Change ImportTask.remove_duplicates to work per-album and also delete album art file on disk when removing duplicate albums.
• Add/adjust tests + add changelog entry for duplicate art fix.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
beets/library/models.py	Make set_art actually replace art at destination (no suffixed duplicates).
beets/importer/tasks.py	Remove duplicate albums at album level; delete album art file + prune dirs.
test/test_importer.py	Add test for “remove duplicate album also deletes art”.
test/test_files.py	Update/add tests for replacing conflicting art + replacing old art with different extension.
docs/changelog.rst	Document bug fix for duplicate cover files on re-import with fetchart.

Comments suppressed due to low confidence (1)

test/test_importer.py:1019

• grug see @pytest.mark.xfail still here, but PR say bug fixed. if test now pass, remove xfail so CI catch future break.

def album_candidates_mock(*args, **kwargs):
    """Create an AlbumInfo object for testing."""
    yield AlbumInfo(

[ShimmerGlass]

What do you think of removing any file matching art_filename in Album.set_art (for example with a list of known image extention)?
This would handle the case where, for example, an album already has cover.png and we Album.set_art("cover.jpg").

[snejus]

What do you think of removing any file matching art_filename in Album.set_art (for example with a list of known image extention)? This would handle the case where, for example, an album already has cover.png and we Album.set_art("cover.jpg").

I think that's what the new logic does? Now it removes both oldart and artdest paths before continuing.

[snejus]

On the other hand, I think it will be up to the users to clean up their album directories. Presumably, many users have a similar situation to mine:

$ fd 'cover.[0-9]' $MUSIC_DIR | head -20
/mnt/music/Music/delsin/randomxs_giveyourbody/cover.1.jpg
/mnt/music/Music/gaurasange/cutyourhair_cryoutloud/cover.1.jpg
/mnt/music/Music/underzone/variosartistasviii/cover.1.jpg
/mnt/music/Music/tsa/irrevocable/cover.1.jpg
/mnt/music/Music/hproductions/hpx49_tonyrohr_oddlantikavenue/cover.1.jpg
/mnt/music/Music/warnerbros/pendulum_immersion/cover.1.jpg
/mnt/music/Music/spektator/spektator5_djvalentimes_descent/cover.1.jpg
/mnt/music/Music/greenfetish/benzeenvol3/cover.1.jpg
/mnt/music/Music/takemetothehospital/hospcds02_theprodigy_omen/cover.1.jpg
/mnt/music/Music/d_vision/benassibros_pumphonia/cover.2.jpg
/mnt/music/Music/diffusereality/te0047_simonhalsberghe_beginnersmindsignalfunk/cover.1.jpg
/mnt/music/Music/gaul/90seditsseries/cover.1.jpg
/mnt/music/Music/diffusereality/smforma_smforma/cover.1.jpg
/mnt/music/Music/diffusereality/drss345_tkg_buster/cover.1.jpg
/mnt/music/Music/rnd/rndr036_sydneyvalette_gorodbolit/cover.1.jpg
/mnt/music/Music/weirdnxcr/weirdnxcract6/cover.1.jpg
/mnt/music/Music/mord/charlton_inplainsight/cover.1.jpg
/mnt/music/Music/sweatitout/sweatds041w_parachuteyouth_cantgetbetterthanthisep/cover.1.jpg
/mnt/music/Music/trip/romazuckerman_stageofloyalty/cover.1.jpg
/mnt/music/Music/concreteberlin/crva02_concretev_avol2/cover.1.jpg

I would like to add an automatic cleanup to this PR (another migration subclass), but we should probably not risk making a mess in users' album directories in case they've added images there manually.

[ShimmerGlass]

I think that's what the new logic does? Now it removes both oldart and artdest paths before continuing.

This works if the old file has the same name as the new file, or is known to beets.
But it doesn't work if the old file has a different name than the new one and is unknown to beets.

This can happen when using the convert feature of fetchart:

• You add cover.png manually to an album dir
• You setup fetchart to convert to jpeg
• You run fetchart, its filesystem sources picks up cover.png, converts it to cover.jpg and imports it
• You now have both cover.png and cover.jpg

[ShimmerGlass]

On the other hand, I think it will be up to the users to clean up their album directories. Presumably, many users have a similar situation to mine

cleanup can be achieved in a single command for people on unix: find MUSIC_DIR -regex '.*/cover\.[0-9]+\.[a-z]+' -delete

[snejus]

and is unknown to beets

This is precisely why I'm not very comfortable about the idea of removing such files - see my previous comment. In this specific use case, I imagine this should have an opt-in configuration specific to fetchart?