lastgenre: Refactor genre applying and pretend mode #6021

JOJ0 · 2025-09-17T06:01:46Z

Description

Shorten final album/genre-apply code. This was longish code already and since the --pretend option is here it is even longer, thus I'd like to smarten/shorten that. Also behavior is changed so that --pretend is automatically enabling force mode (-p or -f is what the user usually expects to being the opposite of each other, but we can discuss that of course)

To Do

~~Documentation.~~ Not required. No behavioral changes
Changelog.
Test refactored to pytest

sourcery-ai · 2025-09-17T06:01:53Z

Reviewer's Guide

Refactors genre assignment and pretend-mode handling by extracting common logging/apply logic into a decorator and dedicated methods, updating command and import hooks to use them, auto-enabling force on pretend mode, and streamlining tests with pytest-mock.

File-Level Changes

Change	Details	Files
Extract logging and pretend-mode branching into a decorator	Define log_and_pretend decorator Wrap apply functions to log messages Check config["pretend"] and prefix logs with “Pretend:” Skip actual apply when in pretend mode	`beetsplug/lastgenre/__init__.py`
Consolidate genre application into two dedicated methods	Create _apply_album_genre with decorated logging/pretend support Create _apply_item_genre similarly Move genre assignment and store logic into these methods	`beetsplug/lastgenre/__init__.py`
Use new apply methods in commands() and imported() hooks and auto-enable force on pretend	Replace inline genre assignment in lastgenre_func with calls to apply* methods In commands(), set config["force"] when opts.pretend Update imported() hook to call apply* for albums and items	`beetsplug/lastgenre/__init__.py`
Introduce pretend configuration default and update changelog	Add "pretend": False to default config Record behavior changes in docs/changelog.rst	`beetsplug/lastgenre/__init__.py` `docs/changelog.rst`
Revise pretend-mode tests and add pytest-mock dependency	Remove old pretend-mode test Add new pytest-mock based test for pretend behavior Add pytest-mock to pyproject.toml dependencies	`test/plugins/test_lastgenre.py` `pyproject.toml`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey there - I've reviewed your changes - here's some feedback:

The fallback-to-album genre behavior is currently hard-coded; consider adding a config option to enable or disable track-level fallback for more flexibility.
In the track processing loop you check if item.genre: (the old value) instead of testing the lookup result (item_genre), which can cause incorrect logging or assignment—update that condition to use item_genre.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The fallback-to-album genre behavior is currently hard-coded; consider adding a config option to enable or disable track-level fallback for more flexibility.
- In the track processing loop you check `if item.genre:` (the old value) instead of testing the lookup result (`item_genre`), which can cause incorrect logging or assignment—update that condition to use `item_genre`.

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

JOJ0 · 2025-09-17T06:14:00Z

Setting this to "ready to review" since I'd like to get this merged before any new feature I have in draft already and to get a sourcery review already..

JOJ0 · 2025-09-17T08:07:40Z

@sourcery-ai guide

@sourcery-ai summary

snejus · 2025-09-17T08:18:24Z

I think it would be a good idea to add --pretend flag in a separate PR, and we already have a PR open: #6008. Would you be happy to adjust this PR to not add the --pretend flag and keep it focused on the refactor?

arsaboo · 2025-09-17T09:56:45Z

@snejus if it's not too much of inconvenience, I would rather have @JOJ0 make changes to last genre. They are much closer to it and have other features planned. I'm happy to close the other PR.

JOJ0 · 2025-09-17T10:26:48Z

@snejus the refactor does not really make sense without the pretend. it's easier this way.

i would like to close @arsaboo 's PR

what i can do here is remove the track to album fallback feature.

would that be ok with you @snejus ?

Ah let me reconsider @snejus: I see you already reviewed the pretend PR from @arsaboo, so if you think this is ready to merge soon, please go ahead you both and finishe it!

I can work from there an adapt this PR to only make pretend conditional smarter/shorter with my decorator refactor idea here!

I think I misunderstood earlier and you ment it that way @snejus right?

JOJ0 · 2025-09-18T04:58:44Z

@snejus the refactor does not really make sense without the pretend. it's easier this way.
i would like to close @arsaboo 's PR
what i can do here is remove the track to album fallback feature.
would that be ok with you @snejus ?

Ah let me reconsider @snejus: I see you already reviewed the pretend PR from @arsaboo, so if you think this is ready to merge soon, please go ahead you both and finishe it!

I can work from there an adapt this PR to only make pretend conditional smarter/shorter with my decorator refactor idea here!

I think I misunderstood earlier and you ment it that way @snejus right?

@snejus ? Was this your intention?

snejus · 2025-09-18T08:39:16Z

@JOJ0 indeed - it's mostly finished so it's we can finish it there and you can base your work on top of it!

sourcery-ai

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

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `beetsplug/lastgenre/__init__.py:591` </location>
<code_context>
+                                    item,
+                                )
+                            else:
+                                self._apply_item_genre(item, label, item.genre)
+                                if write:
+                                    item.try_write()
</code_context>

<issue_to_address>
**issue (bug_risk):** Passing item.genre instead of item_genre may result in incorrect genre assignment.

Pass item_genre to _apply_item_genre to ensure the genre field is updated correctly.
</issue_to_address>

### Comment 2
<location> `test/plugins/test_lastgenre.py:175-184` </location>
<code_context>
+def test_pretend_option_skips_library_updates(mocker):
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding test coverage for pretend mode with both album and singleton (track) operations.

Please add a test for pretend mode with singleton tracks to verify correct behavior and logging.
</issue_to_address>

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/lastgenre/__init__.py

JOJ0 · 2025-09-25T08:43:25Z

Consider adding a configuration flag to enable or disable the automatic fallback to album genre, so users can opt out of inherited genres if desired.

The feature was removed from this PR.

Add unit tests specifically for the fallback-to-album-genre path to ensure tracks only inherit album genres under the correct conditions.

Removed this feature from this PR. Just refactoring of genre applying/pretend handling now.

You might rename the label parameter to something like source for clearer alignment with the existing genre-source terminology.

Terminology changed to "logging label / stage label" a while ago already. This is fine and consistent with other parts of the code.

JOJ0 · 2025-09-25T20:05:53Z

@semohr or @snejus I pulled out the test of the unittest class to a self-contained pytest and installed pytest-mock. I didn't like the cascaded patch.object statements. Test is similar in length now (altough self-contained) but I think a bit easier to read. What do you think?

snejus

Added a comment regarding making the test simpler and a couple of suggestions how to deduplicate item and album logic further

test/plugins/test_lastgenre.py

beetsplug/lastgenre/__init__.py

snejus · 2025-10-13T07:08:37Z

@JOJ0 shall I commit what I've got here? I refactored things exactly as I suggested as I reviewed the PR.

JOJ0 · 2025-10-13T07:11:12Z

I was about to finally do it this week. Was again out of town this weekend. Yes please, it would help speed up things. Thank you!!! 🙏

- Move item and genre apply to separate helper functions. Have one function for each to not overcomplicate implementation! - Use a decorator log_and_pretend that logs and does the right thing depending on wheter --pretend was passed or not. - Sets --force (internally) automatically if --pretend is given (this is a behavirol change needing discussion)

only one arg is passed to the info log anymore.

Replace the log_and_pretend decorator with a more robust implementation using singledispatchmethod. This simplifies the genre application logic by consolidating logging and processing into dedicated methods. Key changes: - Remove log_and_pretend decorator in favor of explicit dispatch - Add _fetch_and_log_genre method to centralize genre fetching and logging - Log user-configured full object representation instead of specific attributes - Introduce _process singledispatchmethod with type-specific handlers - Use LibModel type hint for broader compatibility - Simplify command handler by removing duplicate album/item logic - Replace manual genre application with try_sync for consistency

snejus · 2025-10-15T09:10:14Z

I tried to make the output a bit more user-friendly and used

str(obj) to log the object using the user configured format_item / format_album values
ui.show_model_changes to show the change in genre with colors

Non-verbose output

Verbose output

JOJ0

Thanks @snejus! Let's merge it. Can't press approve since I'm the initial author but LGTM. Great idea with the colors and the singledispatch 🤩

snejus · 2025-10-15T21:46:45Z

🫡

snejus

weird self approval

This comment was marked as resolved.

Sign in to view

JOJ0 changed the title ~~lastgenre: Refactor final genre apply and --pretend option~~ lastgenre: Refactor final genre apply, --pretend option, item to album genre fallback Sep 17, 2025

JOJ0 marked this pull request as ready for review September 17, 2025 06:12

JOJ0 requested a review from a team as a code owner September 17, 2025 06:12

Copilot AI review requested due to automatic review settings September 17, 2025 06:12

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

sourcery-ai bot reviewed Sep 17, 2025

View reviewed changes

JOJ0 force-pushed the lastgenre_pretend branch 3 times, most recently from 6adab6b to 097591e Compare September 17, 2025 07:50

This comment was marked as outdated.

Sign in to view

This comment was marked as duplicate.

Sign in to view

arsaboo mentioned this pull request Sep 17, 2025

lastgenre: Add --pretend option for previewing genre changes #6008

Merged

3 tasks

semohr added the lastgenre label Sep 17, 2025

JOJ0 marked this pull request as draft September 18, 2025 04:57

JOJ0 force-pushed the lastgenre_pretend branch from 097591e to d7a3a1e Compare September 21, 2025 06:35

JOJ0 marked this pull request as ready for review September 21, 2025 06:41

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

JOJ0 changed the title ~~lastgenre: Refactor final genre apply, --pretend option, item to album genre fallback~~ lastgenre: Refactor final genre apply and item to album genre fallback Sep 21, 2025

JOJ0 force-pushed the lastgenre_pretend branch from d7a3a1e to 5b2cf75 Compare September 23, 2025 03:52

JOJ0 marked this pull request as ready for review September 25, 2025 08:34

sourcery-ai bot reviewed Sep 25, 2025

View reviewed changes

beetsplug/lastgenre/__init__.py Outdated Show resolved Hide resolved

JOJ0 force-pushed the lastgenre_pretend branch from b76cd5b to 93610e3 Compare September 25, 2025 08:37

JOJ0 added a commit to JOJ0/beets that referenced this pull request Sep 25, 2025

lastgenre: Changelog for beetbox#6021 refactor apply/pretend

51fd8fa

JOJ0 force-pushed the lastgenre_pretend branch from 93610e3 to aaa9be1 Compare September 25, 2025 19:39

snejus requested changes Sep 27, 2025

View reviewed changes

snejus force-pushed the lastgenre_pretend branch from af0c576 to 8fbdd6e Compare October 15, 2025 08:51

JOJ0 and others added 8 commits October 15, 2025 09:52

lastgenre: Use apply methods during import

1acec39

lastgenre: Fix test_pretend_option

d617e67

only one arg is passed to the info log anymore.

lastgenre: Refactor test_pretend to pytest

654c144

Add pytest-mock to poetry test dependencies group

65f5dd5

Update test

c2d5c1f

Add _process_album and _process_item methods

ee28984

snejus force-pushed the lastgenre_pretend branch 2 times, most recently from a4a5890 to 15d6b05 Compare October 15, 2025 09:07

snejus force-pushed the lastgenre_pretend branch from 15d6b05 to 54a82c6 Compare October 15, 2025 10:12

Show genre change using show_model_changes

88011a7

snejus force-pushed the lastgenre_pretend branch from 54a82c6 to 88011a7 Compare October 15, 2025 10:14

JOJ0 commented Oct 15, 2025

View reviewed changes

snejus approved these changes Oct 15, 2025

View reviewed changes

snejus merged commit becb073 into beetbox:master Oct 15, 2025
17 checks passed

JOJ0 deleted the lastgenre_pretend branch October 16, 2025 05:20

lastgenre: Refactor genre applying and pretend mode #6021

lastgenre: Refactor genre applying and pretend mode #6021

Uh oh!

Conversation

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

Uh oh!

Description

To Do

Uh oh!

sourcery-ai bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

This comment was marked as outdated.

JOJ0 commented Sep 17, 2025

Uh oh!

snejus commented Sep 17, 2025

Uh oh!

arsaboo commented Sep 17, 2025

Uh oh!

This comment was marked as duplicate.

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

Uh oh!

Uh oh!

JOJ0 commented Sep 18, 2025

Uh oh!

snejus commented Sep 18, 2025

Uh oh!

This comment was marked as resolved.

This comment was marked as outdated.

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JOJ0 commented Sep 25, 2025

Uh oh!

JOJ0 commented Sep 25, 2025

Uh oh!

snejus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snejus commented Oct 13, 2025

Uh oh!

JOJ0 commented Oct 13, 2025

Uh oh!

snejus commented Oct 15, 2025

Non-verbose output

Verbose output

Uh oh!

JOJ0 left a comment

Choose a reason for hiding this comment

Uh oh!

snejus commented Oct 15, 2025

Uh oh!

snejus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JOJ0 commented Sep 17, 2025 •

edited

Loading

sourcery-ai bot commented Sep 17, 2025 •

edited

Loading

JOJ0 commented Sep 17, 2025 •

edited

Loading

JOJ0 commented Sep 17, 2025 •

edited

Loading