Searching for a book in a 2-in-1 edition never returns anything - #70
Open
JordanFromIT wants to merge 1 commit into
Open
Searching for a book in a 2-in-1 edition never returns anything#70JordanFromIT wants to merge 1 commit into
JordanFromIT wants to merge 1 commit into
Conversation
When a book's monitored edition is a compilation named after several works
("A Game of Thrones / A Clash of Kings"), that whole string is sent to the
indexers as the search query. No release is named after a compilation, so
every indexer returns nothing and the book can never be found.
Fall back to the book's own title when it is one of the slash-separated
works listed in the edition title. Titles where the slash is part of a
phrase ("Horror/Sci-Fi") produce no matching segment and are left alone.
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.
Description
A book whose monitored edition is a 2-in-1 can never be found. The edition title names both works — "A Game of Thrones / A Clash of Kings" — and that whole string is what gets sent to the indexers as the search query. Nothing is named after a compilation, so every indexer returns zero results, every time, for as long as that edition stays selected. The user sees an interactive search that finds nothing and no reason why. This makes the search use the book's own title when the book is one of the works the edition collects; the edition itself is left alone.
For a concrete sense of it: Goodreads lists A Clash of Kings under at least three compilation works —
#1-2,#1-4, and a 5-book boxed set — so a library that picks up any of them for the wrong reason silently loses that book. On a real instance, a book pinned to the#1-2edition returned 0 results across 5 indexers; with this patch the same book, same config, same indexers returned 3.Why it happens.
BookSearchCriteriaalready narrows a title down to its main section before querying, but it only knows two separators.SplitBookTitlesplits on:and(, so "A Clash of Kings (Part Two)" correctly becomes "A Clash of Kings" — while "A Game of Thrones / A Clash of Kings" is passed through whole.GetQueryTitlethen replaces the slash with the same+it uses for ordinary word breaks, welding the two titles into one query that matches nothing:A Clash of Kings (Part Two)A+Clash+of+KingsA Game of Thrones / A Clash of KingsA+Game+of+Thrones+A+Clash+of+KingsBoth rows are the same book by the same author. The only difference is which punctuation the edition happens to use.
Technical detail
ReleaseSearchService.BookSearchpicks the single monitored edition and hands its title to the search criteria:GetSearchBookTitleprefersselectedEdition.Titleoverbook.Title— deliberately, so a pinned edition is what actually gets searched for. That is the right default and is not changed here. The gap is only that a compilation edition names several works, so preferring its title asks for something that does not exist as a release.Downstream,
BookSearchCriteria.BookQueryisGetQueryTitle(GetMainSearchTitle(BookTitle, Author?.Name)).GetMainSearchTitledelegates toParser.SplitBookTitle, which builds itspartsarray fromIndexOf('(')andIndexOf(':')only and otherwise returns(book, string.Empty)— the title unsplit.GetQueryTitlethen runsNonWord.Replace(cleanTitle, "+"), and/is a non-word character, so it collapses to the same separator used between ordinary words.This adds
GetCollectedWorkTitle, called only fromGetSearchBookTitle. It splits the edition title on/, normalises each segment (collapse internal whitespace, trim) and compares case-insensitively against the book's own title. On a match it returnsbook.Title; otherwise it returns null and the caller keeps the edition title exactly as before.SplitBookTitleitself is untouched. It is shared withParsingServiceon the release-parsing side, and teaching it a new separator would change how release names are interpreted too — a much wider blast radius than this bug justifies.Deliberate choices worth flagging for review:
"Stories of Fantasy, Horror"and"Sci-Fi, and a Man Called Tuf", neither of which is any book's title, so nothing fires and the query is unchanged. A looser "contains" test would have broken that title. There is a test for exactly this case./is handled. Compilations also appear asBook One and Book Twoor with&,|,+. Those are far more ambiguous —&andandoccur inside ordinary titles constantly — and I did not want to guess at them inside a bugfix. If you would rather see a general separator list, say so and I will extend it.Clash of Kingsand the segment isA Clash of Kings, no match, no change. Conservative on purpose: a wrong fallback would send searches for the wrong book, which is worse than the status quo of sending none.GetCollectedWorkTitleis new and called only fromGetSearchBookTitle, which is called only fromBookSearch. Nothing else reads it, no shared parser is modified, and when the edition title has no/the method returns null on its second line — so for the overwhelming majority of libraries this code path is inert.Known gap, deliberately not addressed: the release matcher still validates candidates against the monitored edition title. So for a book genuinely pinned to a compilation, this patch makes standalone releases reachable — they now come back instead of not existing — but whether each one is then accepted still depends on that downstream comparison. Closing that properly means teaching the matcher about collected works too, which is a separate change against different code, and I would rather not fold it into a fix this small. Happy to follow up with it if you want them together.
Database Migration
NO. No schema changes, no new columns, no data written. The change reads
Book.TitleandEdition.Title, both of which already exist, and affects only the string handed to the indexer at search time. Nothing is persisted, so there is nothing to migrate or back-fill, and reverting the commit fully reverts the behaviour.How was this tested?
Built and tested on Linux, .NET 10, in the
mcr.microsoft.com/dotnet/sdk:10.0image. Ran the same stepsbuild.ymluses, including the guards that run before compilation:git grepmerge-conflict markerspackage.jsonparsesversion_guard.py syncVersion sync OK for working tree: 0.9.929version_guard.py monotonic --compare-ref origin/developVersion monotonic OK: 0.9.929version_guard.py commit-hygiene --compare-ref origin/developOK for range origin/develop..HEADdotnet build src/Chaptarr.NoTests.sln -c Releasedotnet test src/Chaptarr.Core.Test -c ReleaseTest counts. Clean
origin/developat5713d83: 2838 passed. This branch: 2845 passed (+7), 0 failing. Both measured from a hard-reset,git cleaned tree with_output/_testsremoved, because an earlier run on a dirty tree reported the same number for both and I did not want to quote it.No frontend files are touched — 2 files changed, both
.cs— so the yarn steps that live outsidebuild.ymlare not applicable here.The 7 new tests, in
ReleaseSearchServiceTitleSelectionFixture: the spaced separator, the unspaced separator, case and padding differences, the book title being blank, a slash that is part of a phrase (Horror/Sci-Fi), a marketplace listing where no segment matches the book, and one end-to-end assertion thatBookQuerycomes out as a single work.Confirmed the tests actually catch the bug by reverting only
ReleaseSearchService.csto itsdevelopversion while keeping the new tests, then re-running:Restoring the fix returns
Passed! - Failed: 0, Passed: 14. The three guard tests pass in both states, which is what confirms the patch does not change behaviour for titles it should leave alone.End to end, Docker on Linux, against a copy of a real library where a book was pinned to the
A Game of Thrones / A Clash of Kingsedition. Stock0.9.929and a build of this branch, same config, same 5 indexers, run through the interactive-search endpoint:totalResultsQueries recorded on the indexer proxy confirm the change reaches the wire:
A Game of Thrones A Clash of KingsA Clash of KingsScreenshots (UI changes only)
None — no UI changes.