Skip to content

fix(importer): suggest every book by the matched author when adopting a file (#2879) - #2883

Merged
vavallee merged 1 commit into
mainfrom
fix/2879-adoption-suggestion-pool
Oct 1, 2026
Merged

vavallee merged 1 commit into
mainfrom
fix/2879-adoption-suggestion-pool

Conversation

@vavallee

@vavallee vavallee commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Closes #2879

Summary

The author's initials were not the cause. textutil.NormalizeAuthorName already folds "Sarah K. L. Wilson" and "Sarah K.L. Wilson" to the same key, and the scan's author resolution (matchingAuthors / resolveAuthors over authorNameTokens) matched the folder author to the catalogue author. That is why all three wrong suggestions were her books. The new test reproduces the report exactly: on main the unit is stored with Paths of Deception (0.667), Chase the Moon (0.643) and Mist of Power (0.622), all by the author resolved from the "K. L." folder. A Wanted "The Matsumoto" in the same layout is claimed automatically at jw=1 on main too.

The real cause: rankCandidates scored only wantedBooks, the reconcile pool from isReconcileCandidate (Wanted, or Imported with a format missing on disk). "The Matsumoto" would score 1.0, so it was not in that pool: Skipped, or already Imported with a file that still exists. The reporter's log fits the second case: files under both Sarah K.L. Wilson/ and Sarah K. L. Wilson/ with Calibre style (13105) folder ids and _ for :. Bindery's renamer renders {Author} from author.Name with ({Year}) folders, so it produces one consistent folder per author; those two spellings came from another tool.

Change 1, suggestion pool (unmatched_units.go, scanner.go): for a resolved author, suggestions are ranked from every catalogue book of that author, whatever its status. Built lazily, only when the scan has unmatched units. An exact title scores 1 and ranks first; between equal scores a reconcilable book goes ahead. Excluded books are still never suggested (the scan's BookRepo.List filters excluded = 0, the existing convention). Files with no readable author keep the old pool, since the whole library is too wide without an author. Cap stays at 3. isReconcileCandidate and the 0.85 automatic claim are unchanged.

Change 2, the outcome when the chosen book already has a file (adoption_adopt.go): adoption was already additive (AddBookFileIfMissing appends a book_files row, derivedFormatPath keeps rendering the lowest id live row, so the existing file stays the one shown; Undo removes only the added row). That is the outcome, now made explicit: the adopt response carries a message when the book already had a live file of that format.

Change 3, web (adoptionMatch.ts, AdoptionEditor.tsx, AdoptionRow.tsx): each suggestion in the editor shows the book's status pill (bookStatusBadge), and the row shows it for an Imported or Skipped top suggestion. Such a suggestion is never a one click Confirm, only a Possible match that opens the editor, where a note says the book already has its files. After adopting, the row says the file was added alongside.

Not done, stated:

  • Stale suggestions: they are still computed at scan time and hydrated from stored ids. Recomputing on view would need a second author resolution path outside the scan, or a created since query plus a copy of the matcher; not cheap enough to keep identical to the scan. A book added after the last scan shows up after the next scan, and the editor's library search finds it meanwhile. Documented in the user guide.
  • Undo of an adoption into a Skipped book leaves it Wanted (unmonitored, since skipping unmonitors) rather than Skipped, because the adoption record does not keep the prior status. That was already reachable through the editor's library search; it needs a column, so it is left for a follow up.
  • Conflicting middle initials and MatchAuthorName untouched.

How it was verified

  • New Go tests:
    • TestScanLibrary_SuggestsTheExactTitleWhateverItsStatus (catalogue Sarah K.L. Wilson, file at Sarah K. L. Wilson/The Matsumoto (13110)/The Matsumoto - Sarah K. L. Wilson.epub), subtests Skipped and Imported with an existing file elsewhere: The Matsumoto is the first suggestion with an exact score, the unit is stored (not auto reconciled), the book's status and monitored flag are unchanged, and the file is not registered on it.
    • TestScanLibrary_WantedExactTitleStillReconciles: a Wanted exact title is still claimed automatically, no unit.
    • TestRankCandidates_ReconcilableBookWinsATie.
    • TestAdopt_IntoABookThatAlreadyHasItsFile: 200, two ebook rows, still Imported, still rendering the existing file, message set; Undo leaves only the original and the book Imported; nothing touched on disk. TestAdopt_IntoAWantedBookSaysNothingExtra.
  • Web tests: adoptionHint.test.ts (Imported or Skipped is never strong), AdoptionView.test.tsx (Imported pill, no Confirm, editor note, "Added alongside" outcome).
  • Fail before, against main's code: both scan subtests fail with candidates = [{BookID:1 Score:0.667} {BookID:3 Score:0.643} {BookID:4 Score:0.622}], want The Matsumoto (book 2) first, the issue's three books. The Wanted test passes on main, as it should. Both web tests fail on main's sources (expected 'strong' to be 'possible'; Unable to find ... Possible match). The adopt test does not compile on main (no message constant); its file and undo assertions describe main's existing additive behaviour.
  • Mutations, each compiles and vets, and a named test fails:
    • suggestion catalogue filtered back to isReconcileCandidate: both scan subtests fail with the three wrong suggestions.
    • isReconcileCandidate widened to Skipped and Imported: both scan subtests fail with units = [], want the untracked Matsumoto left for a person to decide.
    • tie break removed: TestRankCandidates_ReconcilableBookWinsATie fails.
    • message never set / always set: the two adopt tests fail respectively.
  • go build ./... && go vet ./... clean; go test ./cmd/... ./internal/... exit 0; golangci-lint run ./internal/importer/... ./internal/api/... 0 issues. Web: tsc --noEmit clean, eslint on touched files clean, full vitest run 1175 passed, npm run build exit 0.

Checklist

  • Tests added
  • Docs (docs/User-Guide-Wiki.md, adoption section)
  • Changelog fragment (changelog.d/2879-adoption-suggestion-pool.md)

🤖 Generated with Claude Code

https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

… a file (#2879)

The author spelling was not the problem. "Sarah K. L. Wilson" and
"Sarah K.L. Wilson" already resolve to the same catalogue author, which is
why the three wrong suggestions were all her books. Suggestions on the
adoption page were ranked only from the reconcile pool (Wanted, or Imported
with a format missing on disk), so a Skipped or already Imported
"The Matsumoto" was never a candidate, and the best of the rest scored
0.667, 0.643 and 0.622.

For a resolved author, suggestions now come from every catalogue book of
that author, whatever its status. Excluded books stay out, as before (the
scan's book list never had them). An exact title ranks first, and between
equal scores a reconcilable book goes ahead. isReconcileCandidate and the
0.85 automatic claim are unchanged, so the scan still never attaches a file
to a Skipped book or one that already has its file. Authorless files keep
the old pool.

Adopting into a book that already has a live file of that format adds the
new file alongside it (book_files is additive, the existing file keeps
rendering) and the response now says so. In the web, every suggestion shows
its status, a Skipped or Imported suggestion is never a one click Confirm,
and the editor notes when the chosen book already has its files.

Signed-off-by: vavallee <vavallee@protonmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall clean fix. Root cause is correctly identified, the wantedBooks/catalogue split is sound, and the test coverage is solid. A few observations.

Logic

adoption_adopt.go:172–181 — hasLiveFile runs before register, so the message reflects the book's state at check time, not at completion time. A concurrent deletion in that window means the message fires without a true second copy being left. Data integrity is unaffected (the file still gets registered), but the informational message would be wrong in a race. Low likelihood, but worth knowing.

adoption_adopt.go:133 — checkOwnership(…, 0) correctly blocks the untracked file being already owned by any book before the book is resolved; the second check in register at line 337 uses bookID (allowing re-registration to the same book). The TOCTOU window between the two checks was pre-existing.

Frontend

AdoptionEditor.tsx:656 — the alreadyImported note only fires for status === 'imported'. A user who opens the editor and selects a Skipped suggestion gets no advisory that the book was deliberately skipped and the adoption will give it files. The status pill is visible, but a short note parallel to the imported one would reduce surprises. Not a blocker.

AdoptionRow.tsx:10 imports StatusPill from AdoptionEditor.tsx. No circular dependency, but the import direction is unusual (Row → Editor). If this component ever needs to be used elsewhere components/ would be the natural home.

No issues with:

  • The wantedBooks / catalogue separation keeping the auto-reconcile path unchanged.
  • Lazy initialisation of catalogueByAuthor (zero cost when nothing is unmatched).
  • Tie-break (reconcilable wins on equal score).
  • alreadySettled correctly blocking one-click Confirm for imported and skipped.
  • The Undo-of-Skipped limitation is documented in the PR body and in scope for a follow-up.

— 🤖 Bindery triage bot (automated). Reply to correct me; a human will see it.

@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.08108% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/api/adoption_adopt.go 64.70% 10 Missing and 2 partials ⚠️
internal/importer/unmatched_units.go 93.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@vavallee
vavallee merged commit 97b6b9b into main Oct 1, 2026
40 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import/adoption matching has trouble with authors with double initials with a space

1 participant