Repository navigation
fix(importer): library scan keeps the best title match - #2978
Conversation
tryReconcileTitle now returns a hit with its score once the existing gates pass. reconcileByTitle collects every hit, sorts by score, and claims the leader when it is the only hit, when its normalised title is an exact match strictly ahead of the runner-up, or when it leads by at least 0.05. A closer pair is left unmatched, and a failed write applies the same rule to the hits that remain. The user guide and changelog describe that preference. Fixes vavallee#2941 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
github-actions
Bot
added
the
bindery-notified
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
vavallee
left a comment
vavallee
left a comment
There was a problem hiding this comment.
Thanks for jumping on this so fast @mvanhorn! The ranking is a clear improvement, the Dutch Harry Potter case is fixed, and the five new tests all fail on main and pass here. It still merges cleanly with today's main.
@foobarbigtime's two follow ups both hold, and I wrote failing tests for each:
-
The better match drops out of the comparison. A book that already has a file of that format, or was claimed earlier in this scan, never becomes a hit. So "Harry Potter" is the only hit and takes the file. I hit this with two copies in one scan, with the exact book already imported from an earlier scan, and with "…kind script.epub" being walked before the real file. Could those books stay in the ranking as blocked hits? If one of them leads, or is within the margin of the leader, set
claimBlockedand return false so the file stays Unmatched. Books that aren't candidates can come from the same poolsuggestionCatalogueuses. -
Fallback after a write error. When
claimTitleHitfails, just return false rather than moving on tohits[1:]. Repro:
database.Exec(fmt.Sprintf(`CREATE TRIGGER f BEFORE INSERT ON book_files
WHEN NEW.book_id = %d BEGIN SELECT RAISE(ABORT,'boom'); END`, exact.ID))
e.s.ScanLibrary(e.ctx) // "Harry Potter" ends up with the epubOne small thing: the changelog fragment should end with a thanks to foobarbigtime for the report and the follow up. That's how the other entries read.
With those two fixes and tests for them, happy to merge. Thanks again!
Keep existing and newly claimed format slots in the title comparison so weaker matches cannot take duplicate files. Stop after a title claim write fails instead of trying a runner-up. Cover the reported cases with regressions and thank @foobarbigtime for the report and follow up. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
|
Fixed in 1801ad0. Already-imported and already-claimed books stay in the ranking and block a weaker or near-tied match, and a failed write no longer falls through to the runner-up. Both of your repro cases are now regression tests, and the changelog fragment ends with thanks to @foobarbigtime. The new tests, gofmt, and golangci-lint on the importer pass locally. |
… first (#2941) (#3033) * fix(importer): bind an existing file to the best title match, not the first (#2941) FindExisting answered with the first library file whose title cleared titleWordsMatch, so "Harry Potter" took an untracked "Harry Potter en het vervloekte kind.epub" on the add author path, skipped its search, and left the exact book unbound once the file was owned. FindExisting now scores every matching file by Jaro-Winkler on the normalised titles, using the rule the library scan adopts in #2978: the best file answers when it is the only title, when it is an exact normalised match strictly ahead, or when it leads by 0.05. A closer pair answers nothing so the book stays Wanted. Files that differ only in digits (tracks, parts, formats) or share a numbered book folder are one book. Supplement ranking and the volume veto apply as before. FindExistingAmong adds the other direction: the chosen file is held to the same rule against the author's other catalogue titles, so a file titled exactly as another book is not taken by a shorter one. The add paths pass the author's books as rivals through handleNewWantedBook. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> * fix(importer): withdraw an existing file only for an exact rival title (#2941) Review of the first pass found the scored rival rule withdrew real matches. Any catalogue row sharing two words with the file and scoring within 0.05 of the wanted title blocked the bind, so "Project Hail Mary A Novel.epub" was refused to "Project Hail Mary" over a "Proyecto Hail Mary" row, "Mistborn.epub" to "Mistborn: The Final Empire" over the rest of the series, and the owned book was downloaded again. A rival now withdraws a file only when its normalised title is exactly the file's, which still keeps the Harry Potter case from the report fixed. Excluded rows are no longer rivals. A title group now answers with its best scoring member, so "Dune" gets "Dune.epub" rather than "Dune 2.epub" walked before it. The author sync passes the catalogue it already holds instead of listing the author again for every bound book. Other callers read the listing once per new book, only when a file was found. Also corrects the Scanner.FindExisting godoc and widens the changelog fragment to every add path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> * fix(importer): drop rival titled files before ranking existing files (#2941) A file titled exactly as another of the author's books was withdrawn after ranking, so when it outscored the wanted book's own file nothing answered: "The Way of Kings" found nothing beside "The Way of Kings Prime.epub", and "Dune" nothing beside "Dune Messiah.epub", where main bound the right file. Such files now leave the candidate set before bestExistingFile ranks, and the next best file can answer. Because dropping a file can also settle a near tie, the add path now always asks the rival aware lookup instead of only after a plain lookup found something. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> --------- Signed-off-by: vavallee <vavallee@protonmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Hey @mvanhorn, thanks for the update on this one, and sorry it has waited on me. This PR now conflicts with main in scanner.go (three hunks, from #2990, #3003 and #3107). Could you merge main into the branch? Once that is in I will take another look at the changes since my last review. One thing to check while you resolve it: #3003 made the scan refuse a file that another book already tracks, so it is worth looking at how claimBlocked interacts with that rule. |
Resolve internal/importer/scanner.go against the work that landed on main since 1801ad0. - The too-small gate (vavallee#2944) moves into claimTitleHit, after the ranking and claim checks and before AddBookFile. - fillCoverFromFile (vavallee#3094) stays after the reconcile log in claimTitleHit. - The recordUnmatchedUnits callback takes main's (rep, ev) signature and returns (candidates, reason). - The suggestion catalogue is built on first use again, shared by the title tier and the callback, instead of eagerly. - titleReconcileMargin is now findExistingMargin, one constant for the scan and the add path. - claimBlocked is set only when an earlier file of the pass claimed the book, as on main. A Skipped book in the ranking still blocks the hit but no longer turns a tiny sidecar into an already tracked file, which dropped its too small row. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
…the scan (vavallee#2941) A blocked hit (Skipped, or already holding a file) within the 0.05 margin sent the file back to Unmatched even when the wanted book's title was an exact match, so the wanted book stayed Wanted and was downloaded again. The scan now claims an exact, unblocked hit that is strictly ahead of every other hit, the same exact strictly ahead rule FindExisting uses. A blocked best, a tie, or a non exact best with a blocked hit in the margin still refuse. A claimed book whose root check turns a file away now still sets claimBlocked, so the file stays a sidecar as on main (vavallee#2188). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com>
vavallee
left a comment
vavallee
left a comment
There was a problem hiding this comment.
Looks good after the merge with main and the exact title fix, thanks.
|
Appreciate the review here, @vavallee. The library scan now keeps the best scoring title match and leaves close ties unmatched. |
Summary
tryReconcileTitle now returns a hit with its score once the existing gates pass. reconcileByTitle collects every hit, sorts by score, and claims the leader when it is the only hit, when its normalised title is an exact match strictly ahead of the runner-up, or when it leads by at least 0.05. A closer pair is left unmatched, and a failed write applies the same rule to the hits that remain. The user guide and changelog describe that preference.
A library scan can attach a file to the wrong wanted book for the same author. The first title that scores at least 0.85 takes the file, so a shorter partial title that appears earlier in the library receives it and the book with the exact title stays without a file. reconcileByTitle walks the author's wanted books in library order, and tryReconcileTitle claims a book as soon as it clears the title gates, including the 0.85 Jaro-Winkler floor. Later candidates are never compared with that first score, so an earlier partial match is kept even when a later title matches exactly.
Closes #2941
How it was verified
go test ./internal/importer/... succeeded.
Checklist
Signed-off-bythat matches its author (git commit -s), see Sign your work. Commits authored by a coding agent fail this check; author them as yourself.changelog.d/<issue-number>-<slug>.md, not an edit toCHANGELOG.md, see changelog.d/README.mddocs/DEPLOYMENT.mdupdated if env vars, config, or upgrade path changeddocs/updated if user-facing behaviour changedTHIRD_PARTY_LICENSES.mdis regeneratedTest plan
make check(or the individualgo test ./cmd/... ./internal/...andcd web && npm run buildsteps)Not claimed:
make checkis a different command from the local runs that passed,gofmt,go vetandgo test.go test ./internal/importer/... succeeded.
AI was used for assistance.