importer: improve handling of duplicates#6702
Open
snejus wants to merge 9 commits into
Open
Conversation
|
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 Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6702 +/- ##
==========================================
- Coverage 72.77% 72.66% -0.12%
==========================================
Files 162 163 +1
Lines 20810 20825 +15
Branches 3294 3284 -10
==========================================
- Hits 15145 15133 -12
- Misses 4938 4972 +34
+ Partials 727 720 -7
🚀 New features to boost your workflow:
|
- Store duplicate decisions on `ImportTask.duplicate_action` instead of separate `should_remove_duplicates` and `should_merge_duplicates` flags. - Update duplicate resolution, stage branching, and duplicate logging to read from the enum value consistently. - This centralizes duplicate action state and prevents behavior drift across importer flow.
- Move configured duplicate-action lookup into ImportSession.get_duplicate_action. - Let TerminalImportSession prompt only when the configured action is ASK. - Simplify duplicate stage flow and remove obsolete fixture override logic.
snejus
force-pushed
the
improve-import-task-duplicates-handling
branch
from
cd6dfe9 to
252e414
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
grug see PR want make duplicate handling live in one place (session API) so stages and UI stop copy-paste policy logic. this fit importer pipeline: one config decision, then task carry that decision.
Changes:
- move duplicate policy resolution behind
ImportSession.get_duplicate_action()and store result onImportTask. - simplify duplicate resolution in importer stages to call session API.
- update tests/fixtures to drive duplicate behavior via config instead of fixture-only flags.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
test/test_importer.py |
update duplicate tests to set import.duplicate_action via config. |
beets/ui/commands/import_/session.py |
make terminal session prompt only when policy is ASK; refactor duplicate prompt output. |
beets/test/helper.py |
adjust importer test helper defaults for new duplicate behavior (but current diff sets wrong config key). |
beets/library/models.py |
add AnyLibModel typing helper. |
beets/library/__init__.py |
export AnyLibModel. |
beets/importer/tasks.py |
add duplicate_action state to task; update skip logic to account for duplicate skip. |
beets/importer/stages.py |
simplify duplicate resolution to use session API and duplicate_action. |
beets/importer/session.py |
add get_duplicate_action() default config lookup; update logging for duplicate replace. |
beets/importer/__init__.py |
re-export DuplicateAction. |
.git-blame-ignore-revs |
add ignore rev entry. |
Comment on lines
521
to
525
| "resume": False, | ||
| "singletons": False, | ||
| "timid": True, | ||
| "import": {"duplicate_action": "remove"}, | ||
| } |
Comment on lines
+152
to
+156
| def report_item_summary(self, items: list[Item], is_album: bool) -> None: | ||
| ui.print_(f"Old: {summarize_items(items, not is_album)}") | ||
| if self.config["duplicate_verbose_prompt"].get(bool): | ||
| for dup in items: | ||
| print(f" {dup}") |
| sel = ui.input_options( | ||
| ("Skip new", "Keep all", "Remove old", "Merge all") | ||
| ) | ||
| self.report_item_summary(task.imported_items(), is_album) |
Comment on lines
+345
to
349
| task.duplicate_action = session.get_duplicate_action( | ||
| task, found_duplicates | ||
| ) | ||
| log.debug("default action for duplicates: {}", duplicate_action) | ||
|
|
||
| if duplicate_action == "s": | ||
| # Skip new. | ||
| task.set_choice(Action.SKIP) | ||
| elif duplicate_action == "k": | ||
| # Keep both. Do nothing; leave the choice intact. | ||
| pass | ||
| elif duplicate_action == "r": | ||
| # Remove old. | ||
| task.should_remove_duplicates = True | ||
| elif duplicate_action == "m": | ||
| # Merge duplicates together | ||
| task.should_merge_duplicates = True | ||
| else: | ||
| # No default action set; ask the session. | ||
| session.resolve_duplicate(task, found_duplicates) | ||
|
|
||
| session.log_choice(task, True) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change centralizes duplicate handling in the import pipeline.
ImportTasknow carries a singleduplicate_actionstate instead of separate duplicate flags, so duplicate decisions are represented in one place.ImportSession.get_duplicate_action()becomes the shared entry point for resolving the configured duplicate policy.TerminalImportSessionnow only prompts the user when the configured action isASK; otherwise it reuses the shared session logic.beets/importer/stages.pyis simplified to call the session API instead of re-implementing config lookup and branching.High-level impact