[LastGenre] Canonicalize existing genres

[Nukesor]

Description

Fixes #6305

Implements a new canonicalize_existing config flag. The added documentation should explain its behavior. If there're any unclarities, we need to adjust the docs :)

To Do

• Documentation.
• Changelog.
• Tests.

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

[Copilot]

Pull request overview

This PR implements a new canonicalize_existing configuration flag for the lastgenre plugin to enable whitelist canonicalization of existing genres when force: no, canonical: yes, and whitelist: yes are set.

Changes:

• Adds a new canonicalize_existing boolean configuration option with a default value of False
• Updates the genre resolution logic to canonicalize existing genres before returning them when the new flag is enabled
• Adds documentation explaining the new flag's behavior and requirements

Reviewed changes

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

File	Description
beetsplug/lastgenre/init.py	Adds the canonicalize_existing config option and implements canonicalization logic for existing genres
docs/plugins/lastgenre.rst	Documents the new canonicalize_existing configuration option
docs/changelog.rst	Adds changelog entry for the new feature
test/plugins/test_lastgenre.py	Adds test case to verify canonicalization of existing genres

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The documentation should clarify what 'fails' means in this context. Does it mean no canonical genres are found, or that the canonicalization process encounters an error? This ambiguity could confuse users.

Suggested change

	genres. If this fails, the ``fallback`` is used. Default: ``no`` (disabled).
	genres. If canonicalization cannot be applied or does not produce a canonical
	genre (for example, because no mapping is found or an error occurs), the
	``fallback`` is used instead. Default: ``no`` (disabled).

[Nukesor]

This really doesn't match the style of the remaining documentation + it's overly verbose.
For the user the inner workings aren't really important.

[codecov]

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.86%. Comparing base (a327c28) to head (c5eb470).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/lastgenre/__init__.py	42.85%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6317      +/-   ##
==========================================
- Coverage   68.87%   68.86%   -0.01%     
==========================================
  Files         140      140              
  Lines       18666    18673       +7     
  Branches     3063     3066       +3     
==========================================
+ Hits        12856    12859       +3     
- Misses       5162     5165       +3     
- Partials      648      649       +1     

Files with missing lines	Coverage Δ
beetsplug/lastgenre/__init__.py	69.47% <42.85%> (-0.68%)	⬇️

🚀 New features to boost your workflow:

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

[JOJ0]

Hm interesting feature and useful, but I'm not sure if this should maybe be called something containing the word "cleanup". It might even include whitelist filtering without canonicalization too.

I see it as a "cleanup existing only" kind of functionality.

And are you sure this can't be included in the force branch somehow? We are moving away from force:no being what one would expect from it. let's brainstorm some more here please

[JOJ0]

@Nukesor I adjusted my wording above slightly. I think it was weird. Maybe still is...

[Nukesor]

I didn't read your first response, but the edited doesn't seem weird at all :D

And are you sure this can't be included in the force branch somehow?

The force-branch already canonicalizes local genres (if keep_existing is set), so that case is already covered. Right now, it's just not possible to canonicalize genres in non-force mode.

I see it as a "cleanup existing only" kind of functionality.

It's more of a "cleanup stuff from lastfm, but please also cleanup local stuff that isn't force-overwritten/merged" :D

Regarding "cleanup", I would be up for it, although I think that canonicalization better conveys what actually happens. If we were to change the wording, we should do it for the whole plugin though.

We are moving away from force:no being what one would expect from it.

Neat. How would that look like?
The current flag handling is definitely a bit confusing. A different approach to configuration would probably be easier to understand and use. Although I'm not sure how that would look like yet. Maybe enum-based configuration, where sensible combinations of behavior are given a dedicated name?