Retarget cross-provider duplicate book rows to the grabbed book on shared edition identity - #113
Open
jbob06 wants to merge 1 commit into
Open
Conversation
…ared edition identity RetargetSameWorkMatchesToGrabbedBook only retargeted an import decision back to the grabbed book row when the two rows shared a work-level provider ID (WorkIdMatcher.WorkProviderIdMatches). Two rows created for the same real book via different metadata providers (e.g. one Hardcover-sourced, one Goodreads-sourced) never share a work ID, so a completed, tracked download could permanently fail to import if the matcher happened to resolve to the wrong (unmonitored duplicate) row - no retry ever succeeds, since the matcher makes the same choice every time. Add a second, independent signal: if the matched edition already exists under the grabbed book row via a shared edition-level identifier (ISBN, ASIN, Audible ASIN, or a provider edition ID), that's sufficient proof the two rows represent the same book, since two different books cannot share an ISBN/ASIN. The retarget still only ever lands on a concrete edition the grabbed row already owns - no cloning or synthesis. The existing same-author and same-media-type guards are unchanged. Fixes Chaptarr#111
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 #111
Problem
RetargetSameWorkMatchesToGrabbedBookonly retargets an import decision back to the grabbed book row when the two rows share a work-level provider ID (WorkIdMatcher.WorkProviderIdMatches-hc:/gr:/ol:intersection). Two rows created for the same real book via different metadata providers (one Hardcover-sourced, one Goodreads-sourced, with no cross-reference between them) never share a work ID.Concrete repro from my own library: a monitored book (BookId 29083,
hc:585413) was grabbed and downloaded successfully. A separate, never-monitored duplicate row for the same real book (BookId 29100,gr:200474660) existed from an unrelated author-bibliography re-discovery pass. Import matched the completed download to BookId 29100 instead of BookId 29083, and since there's no work-ID overlap between the two rows, the retarget safety net declined to fix it up - the import was permanently rejected:No retry ever succeeds, since the matcher makes the same choice every time. Full writeup in #111.
Fix
Add a second, independent signal alongside the existing work-ID check: if the matched edition already exists under the grabbed book row via a shared edition-level identifier (ISBN, ASIN, Audible ASIN, or a provider edition ID), that's sufficient proof the two rows represent the same book - two different books can't share an ISBN/ASIN. The retarget still only ever lands on a concrete edition the grabbed row already owns (via the existing
FindEquivalentEditionForTargetBooklookup) - no cloning or synthesis of editions. The existing same-author and same-media-type guards in the baseline identity check are unchanged.WorkIdMatcher.csitself is untouched, so its other callers (EbookColocateOnAudiobookImportHandler,EbookColocationPlanner,BookService,BookController,ReleaseController) are unaffected. The change is scoped to one method inDownloadedBooksImportService.cs.Known limitation
This only rescues the case where the duplicate rows' editions actually share an edition-level identifier. If they share neither a work ID nor any edition ID, the import still (correctly, conservatively) gets rejected - no safe signal remains to retarget on, and no regression from that path.
Testing
Two tests added to
DownloadedBooksImportServiceFixture.cs, using the real incident's BookIds:should_retarget_cross_provider_duplicate_row_match_to_grabbed_book_when_edition_identity_matches- the repro; fails ondevelop, passes with this fix.should_not_retarget_cross_provider_row_match_when_no_shared_work_or_edition_identity- the safety boundary: no work-ID overlap and no edition-ID overlap still rejects, with the original error message, and the decision's book is left untouched (no misfiling).Verified via Docker (
mcr.microsoft.com/dotnet/sdk:10.0):dotnet build src/Chaptarr.sln- clean, 0 errors.dotnet test src/Chaptarr.Core.Test/Chaptarr.Core.Test.csproj- 2854/2854 passing, 0 failures (full suite, no regressions).