feat(library): deep rescan for the whole library, not just one folder - #460
Merged
Conversation
Reported in #457: genre edits made in Mp3tag never showed up after "Rescan the library", while an earlier artist rename had worked. The scan fast path skips a file whose `(mtime, size)` is unchanged. Mp3tag preserves the modification date, and an ID3 rewrite usually fits in the existing padding so the size doesn't move either — the file looks untouched and its new tags are never read. (The artist rename likely grew the tag past its padding, changing the size.) The bypass for this already existed — `scan_folder(deep: true)`, added for the same root cause in #366 — but only per folder. `rescan_library` hard-coded `false`, so the library-wide button users actually reach for could never see these edits. Thread `deep` through `rescan_library` and add a second header button next to Rescan, mirroring the per-folder one. Separate control rather than a modifier on the existing button: a full re-read is markedly slower, so it should be chosen deliberately. i18n across all 17 locales. Docs: library.md claimed a tag edit "is already detected, since it moves that file's mtime". That is the assumption this bug disproves, so it now states the caveat and documents both deep-rescan entry points. Refs #457
InstaZDLL
added
scope: frontend
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLe PR ajoute un rescan profond optionnel, disponible par dossier et pour toutes les bibliothèques. Le paramètre est propagé jusqu’au scanner backend, avec prévention des chevauchements, progression localisée et documentation du comportement. ChangesRescan profond
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant LibraryView
participant LibraryContext
participant TauriAPI
participant Scanner
participant ScanProgressToast
LibraryView->>LibraryContext: rescanLibrary(libraryId, true)
LibraryContext->>TauriAPI: rescan_library({ libraryId, deep: true })
TauriAPI->>Scanner: scan_folder_inner(..., deep)
Scanner-->>TauriAPI: RescanSummary
TauriAPI-->>LibraryContext: refresh()
Scanner-->>ScanProgressToast: progression et erreurs
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/library.md`:
- Around line 35-38: Update the “Two entry points” documentation in
docs/features/library.md to distinguish the locations: document the per-folder
magnifier under My music → Folders, and document the whole-library button next
to Rescan in the My music header independently of the active tab.
In `@src/components/views/LibraryView.tsx`:
- Around line 153-155: Unify the rescan locking in LibraryView by deriving a
shared isAnyRescanActive state from both isDeepRescanning and
deepRescanFolderId, then use it for the global and folder rescan controls,
including the referenced handlers or disabled states. Ensure either rescan mode
disables both actions and prevents concurrent scan_folder_inner writes.
- Around line 445-451: Modify the deep-rescan loop in LibraryView so each
rescanLibrary(lib.id, true) call has its own error handling, allowing subsequent
libraries to continue after one fails. Track or report the failed library
errors, while retaining the final setIsDeepRescanning(false) cleanup and
avoiding a loop-level catch that aborts iteration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 009d081c-d820-4cc6-8647-1224d2d8c42d
📒 Files selected for processing (23)
docs/features/library.mdsrc-tauri/crates/app/src/commands/library.rssrc/components/views/LibraryView.tsxsrc/contexts/LibraryContext.tsxsrc/hooks/useLibrary.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.jsonsrc/lib/tauri/library.ts
Three review findings on #460. **Concurrent scans were possible.** The two header buttons only guarded against each other, leaving the per-folder deep-rescan button free to start a second scan alongside a library-wide one. `scan_folder_inner` writes and SQLite takes one writer at a time, so that is a real overlap, not just a confusing UI. All three controls now derive their disabled state and their early-return guard from one `isAnyRescanActive`. **One failing library stranded the rest.** The loop wrapped every iteration in a single try, so a drive that went away aborted the pass for the libraries after it. Errors are handled per library now. The plain rescan loop had the identical flaw right next to it and gets the same treatment — leaving one of two adjacent loops with the bug would be worse than the small extra diff. **Docs placed both entry points in the same screen.** The per-folder magnifier is under My music → Folders, but the library-wide button sits in the My music header and is reachable from any tab. Refs #457
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/views/LibraryView.tsx`:
- Around line 456-464: Update the library rescan loops around rescanLibrary so
they inspect each returned RescanSummary.errors, not only Promise rejections.
Aggregate or display the number of errors for each library, while preserving the
existing catch handling for rejected rescans and continuing to process other
libraries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b0eb8912-f459-4081-b1cf-a8377dfa2a32
📒 Files selected for processing (2)
docs/features/library.mdsrc/components/views/LibraryView.tsx
Review finding on #460. `rescan_library` folds per-folder failures into `summary.errors` and resolves anyway, so awaiting it only catches an outright rejection: a pass where half the folders failed looked exactly like a clean one, in both loops. Both now read the summary and log the aggregate folder-error count. The count was invisible in the UI too. `scan:progress` has carried `errors` all along and the toast never rendered it, so a partially failed scan announced "Scan complete" with no qualification. It now appends the failed-file count in amber when there is one — which covers every scan path, not just this button. New i18n key across all 17 locales, with the CLDR plural categories Russian and Arabic need (a missing `_other` on ru is what makes i18next fall back to English). Refs #457
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/common/ScanProgressToast.tsx`:
- Around line 112-119: Update the completed-scan rendering in ScanProgressToast
so done && errors > 0 uses scanProgress.doneErrors as the primary completion
message instead of the normal success title/subtitle. Also switch the completion
icon and visual styling from success green to an error or warning treatment for
partial failures, while preserving the existing clean-success appearance when
errors is zero.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40960c12-1890-40b7-8bd3-0783a4ac5a1e
📒 Files selected for processing (19)
src/components/common/ScanProgressToast.tsxsrc/components/views/LibraryView.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.json
Review finding on #460. Appending an amber line under a green checkmark and "Scan complete" still presents a partially failed scan as a clean one — the headline is what people read. When the scan finishes with errors, the failure count becomes the headline, the icon switches to an amber warning, and the title takes the amber tone. The added/updated/skipped line stays underneath as context, and a clean scan looks exactly as before. Refs #457
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
1 participant
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.
Fixes #457 — @jo-el414 changed genres on several albums in Mp3tag, hit Rescan the library, and the genres never updated. An earlier artist rename through the same workflow had worked, which is what made it look inconsistent.
Cause
The scan fast path skips any file whose
(mtime, size)is unchanged. Mp3tag preserves the file's modification date, and an ID3 rewrite usually fits inside the existing padding so the size doesn't move either — the file looks untouched, and its new tags are never read.That also explains why the artist rename worked:
Celine→Célinegrows the tag by a byte, which likely pushed it past the padding and changed the file size.Why the existing fix didn't help
The bypass already existed:
scan_folder(deep: true), added for this exact root cause in #366, reachable from the magnifier button on a folder row.But
rescan_libraryhard-codeddeep = false:So the library-wide button — the one you actually reach for after retagging a batch of albums — could never see mtime-preserving edits. The escape hatch existed but not where the problem is met.
Change
deepthreaded throughrescan_library(defaulting tofalse, so existing callers such as Settings are untouched), plus a second header button next to Rescan mirroring the per-folder one.A separate control rather than a modifier on the existing button: a full re-read of the library is markedly slower, so it should be chosen deliberately, not triggered by a stray modifier key. New i18n keys in all 17 locales.
Docs
docs/features/library.mdasserted:That is precisely the assumption this bug disproves. It now carries the caveat and documents both deep-rescan entry points, including the interaction with Split this artist (a deep pass re-reads tags authoritatively and undoes an in-place split).
Verification
cargo check -p waveflow --all-targets,bun run typecheck,bun run lint, prettier — all clean (exit codes read directly).Not visually verified — the app doesn't run outside the Tauri shell here. Worth confirming on the reporter's own case: retag a genre in Mp3tag with "preserve modification time" on, then use the new button.
Note on scope
This makes the workaround reachable; it doesn't make a normal rescan notice mtime-preserving edits. Doing that would mean hashing every file on every scan — the cost the fast path exists to avoid. If it turns out users hit this often, the middle ground would be hashing only files whose tag-region size changed, but that's a bigger change than this bug warrants.
Summary by CodeRabbit