Add "Rescan directory" import prompt choice - #6945
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6945 +/- ##
=========================================
Coverage ? 76.91%
=========================================
Files ? 163
Lines ? 21644
Branches ? 3349
=========================================
Hits ? 16648
Misses ? 4180
Partials ? 816
🚀 New features to boost your workflow:
|
|
Hi! Thanks for the PR! Is there an active issue / feature request this is related to? |
Hey @henry-oberholtzer! I didn't find one at first, but I looked again and found what looks like an old issue: #166 (google-code-export/beets#384). I updated the PR description to reflect this. This may be a niche situation, but it's been plaguing me lately. It's been super helpful for me, and hopefully others will find it helpful as well. I also added an example in the PR description to show it in use. |
|
Possibly #6527 could be related |
|
@henry-oberholtzer is there anything else needed for this PR or is it good to be merged? |
When beets can't confidently match an album, the interactive import prompt now offers a "Rescan directory" option alongside Skip/Use as-is/etc. It re-reads the task's directories from disk and re-runs the match, so files can be cleaned up (duplicates, junk) mid-import without restarting the whole `beet import` run. - New Action.RESCAN enum member (actions.py) and set_choice wiring (tasks.py). - New "r" PromptChoice, shown for album tasks with a toppath, added to both the "no candidates" and "pick a candidate" prompt variants since they share the same choice list (session.py). - New rescan_tasks() generator and a RESCAN branch in the user_query pipeline stage that re-extends the pipeline with freshly-discovered tasks, mirroring the existing TRACKS/ALBUMS branches (stages.py). - Tests covering added/removed/all files between the initial scan and the rescan (test_importer.py). - Documents the new choice in the auto-tagger guide (docs/guides/tagger.rst). - Changelog entry. Also syncs uv.lock's recorded package version (2.13.0 -> 2.13.1) to match pyproject.toml; it was drifting and getting silently regenerated by every uv/poe invocation, which was tripping up pre-commit's file-modification detection.
- rescan_tasks(): assert task.toppath is not None before passing to ImportTaskFactory (it's only reachable when the "r" choice was offered, which already requires a truthy toppath), and use distinct variable names for the singleton/album branches so mypy doesn't unify them into an incompatible union type. - test_plugins.py: the new built-in "r" (Rescan directory) choice collided with a test plugin's example "r" choice, which the short-letter conflict resolution silently drops in favor of built-ins. Moved the test plugin's choice to "z" and added "Rescan directory" to the expected option tuples across the four TestPromptChoices cases that exercise the album prompt.
- rescan_tasks(): remove the singleton-mode branch. It was dead code -- the "Rescan directory" choice is only ever offered on album tasks, and album tasks only exist when the session isn't in singleton mode, so the branch could never actually run. - test_importer.py: the existing rescan tests mutated the album directory *before* calling importer.run(), so the initial scan (not just the rescan) already saw the mutated state -- the tests passed, but didn't actually exercise re-reading a directory that changed mid-import. Added ImportRescanTest._run_with_cleanup_before_first_prompt(), which performs the filesystem edit exactly when the first prompt is answered (via a choose_match side_effect), and switched all existing rescan tests to use it. - Added two more tests: rescanning a directory left with only an unreadable file (covers factory.album() returning None), and rescanning after the user splits one messy directory into two proper album subdirectories.
68251bb to
ee066e2
Compare
I don't find this too niche actually. This is plaguing me since I started using beets. On batch imports of old and untidy music collections this can happen all the time and I think is a very simple and handy approach instead of "skip, remember, cleanup files, rerun". I love it and looking through it think it's ready to merge. Maybe a quick final look from @snejus? What do you think? |
snejus
left a comment
snejus
left a comment
There was a problem hiding this comment.
What a clean PR!! Looks good to me, just a small consistency fix needed.
Thank you :) Co-authored-by: Šarūnas Nejus <snejus@protonmail.com>
| assert task.toppath is not None | ||
| factory = ImportTaskFactory(task.toppath, session) | ||
| found = False | ||
| for directory in task.paths: |
There was a problem hiding this comment.
I think this walks the same files more than once because task.paths is not a list of independent album roots. For a multi-disc album, it can contain the album root and each disc directory. Rescanning each path emits the combined album first and then each disc again. I reproduced this with a two-disc album: the rescan produced three tasks and eventually failed during duplicate handling with NotFoundError.
This also bypasses ImportTaskFactory.paths(), so a flat import gets split into separate directory albums, while rescanning a single-file album finds nothing.
Could we reuse the normal discovery behavior here while keeping it scoped to the current task? I think it would also be worth adding multi-disc, flat, and single-file rescan tests.
There was a problem hiding this comment.
from beets import importer
from test.test_importer import ImportRescanTest
class TestRescanImportModes(ImportRescanTest):
def test_rescan_preserves_flat_album_grouping(self):
self.prepare_album_for_import(2, album_id=2)
self.setup_importer(flat=True)
self.importer.add_choice(importer.Action.RESCAN)
self.importer.add_choice(importer.Action.APPLY)
self.importer.add_choice(importer.Action.APPLY)
self._run_with_cleanup_before_first_prompt(lambda: None)
assert len(self.lib.albums()) == 1
def test_rescan_preserves_multidisc_album_grouping(self):
for track in self.album_path.glob("*.mp3"):
track.unlink()
self.prepare_album_for_import(1, album_path=self.album_path / "CD1")
self.prepare_album_for_import(1, album_path=self.album_path / "CD2")
self.importer.add_choice(importer.Action.RESCAN)
self.importer.add_choice(importer.Action.APPLY)
self.importer.add_choice(importer.Action.APPLY)
self._run_with_cleanup_before_first_prompt(lambda: None)
assert len(self.lib.albums()) == 1
rescan_tasks() walked each entry in task.paths independently with albums_in_dir(), but task.paths isn't a list of independent album roots -- for a multi-disc album it holds the album root plus each disc subdirectory. Rescanning every entry re-walked the same files more than once, producing duplicate tasks (the combined album plus each disc on its own), which could fail during duplicate handling. It also bypassed ImportTaskFactory.paths(), so a flat import got split into separate directory albums on rescan, and rescanning a single-file album found nothing. Fix: dedupe task.paths down to top-level roots, then scope a fresh ImportTaskFactory to each root and reuse its normal .paths() discovery, which already handles flat mode, multi-disc collapsing, and single-file imports correctly. Also fixes test_plugin_choices_in_ui_input_options_album, which expected a stale 'baR' choice label after the plugin's own choice was relabeled to 'baZ' in a prior commit -- this was failing CI. Co-authored-by: Šarūnas Nejus <snejus@protonmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011NmPv7JDYZi9UqeKE6z312
| # Reuse the normal discovery logic (respecting `flat` mode, | ||
| # multi-disc collapsing, and single-file imports), scoped to just | ||
| # this directory rather than the whole original `toppath`. | ||
| sub_factory = ImportTaskFactory(directory, session) |
There was a problem hiding this comment.
I think using directory as the factory's toppath changes more than just the discovery scope. ImportTaskFactory.album() passes its own toppath to every task it creates, but the original import root is still needed for cleanup and resume bookkeeping.
I reproduced two regressions here:
- With
move=True, rescanning and importing leaves the now-empty source album directory behind because pruning treats that directory as the root that must not be removed. - With
resume=True, the completed import leaves stale progress recorded under the album directory because the final sentinel only resets progress for the original import root.
Could we scope discovery to directory while preserving task.toppath on the tasks that are emitted? I think it would be worth adding regression tests for moving and resume state as well.
There was a problem hiding this comment.
Good catch, thanks — reproduced both. Fixed by keeping a factory scoped to the original task.toppath for constructing the emitted tasks, and using a directory-scoped factory only for discovery. Added regression tests for both the move-pruning and resume-state cases in 9869ff79b.
Scoping discovery to each rescanned directory (previous commit) had a side effect: ImportTaskFactory.album() stamps its own toppath onto every task it creates, so tasks emitted by rescan_tasks() ended up with the rescanned directory as their toppath instead of the original import root. That broke two things that key off toppath: - Pruning treats toppath as the boundary it must not remove past, so with move=True the emptied source album directory was left behind. - Resume progress is recorded (and later reset) keyed by toppath, so with resume=True progress got recorded under the album directory and was never reset, leaving stale state behind. Fix: keep a single factory scoped to the original task.toppath for actually constructing tasks (as before), and use a separate, directory-scoped factory only for discovery (walking the filesystem and respecting flat mode / multi-disc collapsing / single-file imports). Co-authored-by: Šarūnas Nejus <snejus@protonmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011NmPv7JDYZi9UqeKE6z312
There was a problem hiding this comment.
Pull request overview
grug see PR add new import prompt choice: “Rescan directory”. this let user clean folder mid-import, then re-run match without restart.
Changes:
- add
Action.RESCANand wire it through prompt choice + task choice storage. - add
rescan_tasks()and newuser_querybranch to re-discover tasks from disk and re-run lookup/prompt. - add tests + docs + changelog, and sync
uv.lockbeets version.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | bump locked beets version to 2.13.1 to match project metadata |
| test/test_plugins.py | adjust prompt-choice tests for new default “Rescan directory” option |
| test/test_importer.py | add importer tests covering rescan behavior across directory changes |
| docs/guides/tagger.rst | document new “R” rescan choice in interactive prompt |
| docs/changelog.rst | add changelog entry for rescan import prompt option |
| beets/ui/commands/import_/session.py | add “r” choice for album tasks with toppath; allow returning RESCAN from prompt loop |
| beets/importer/tasks.py | allow Action.RESCAN in ImportTask.set_choice() |
| beets/importer/stages.py | implement rescan_tasks() and RESCAN pipeline extension in user_query |
| beets/importer/actions.py | introduce new Action.RESCAN enum member |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I think filtering I found two cases where this still splits one album into multiple tasks:
I don't think the original discovery scope can be reconstructed from |
Description
Fixes #166
When beets can't confidently match an album, the interactive import prompt now offers a "Rescan directory" option alongside Skip/Use as-is/etc. It re-reads the task's directories from disk and re-runs the match, so files can be cleaned up (duplicates, junk) mid-import without restarting the whole
beet importrun.Also syncs uv.lock's recorded package version (2.13.0 -> 2.13.1) to match pyproject.toml; it was drifting and getting silently regenerated by every uv/poe invocation, which was tripping up pre-commit's file-modification detection.
To Do
Example
Don McLean has multiple albums named "The Best of Don McLean". In this case 2 of them were smashed together in the same directory.