fix(metadata): ISBN cache scope, supplement translations, OpenLibrary cover_edition, rebind editions

changelog.d/1779-openlibrary-cover-edition.md

@@ -0,0 +1,2 @@
+### Fixed
+- **OpenLibrary works take their language and cover from the edition OpenLibrary features** (#1779). When a work had no language or cover of its own, Bindery read a handful of editions in whatever order OpenLibrary returned them, so a work whose first few happened to be translations could be filed as foreign language or shown with a translated cover. It now uses the work's featured edition, the one its openlibrary.org page shows, and only samples other editions for what that edition lacks, preferring a cover in the language it settled on. A single book fetched from OpenLibrary, as on Re-bind, also takes the featured edition's cover when the work has none. This costs up to two extra OpenLibrary requests per work that needs sampling, and one extra request for a single book only when its work has no cover.

changelog.d/2781-rebind-old-editions.md

@@ -0,0 +1,2 @@
+### Fixed
+- **Re-binding a book drops the old record's editions** (#2781). A book re-bound from the wrong metadata record kept that record's provider editions, so searches could keep using the wrong work's ISBN. A re-bind to a different record now removes them. Editions imported from Calibre or Audiobookshelf, the edition you selected, and editions a download or Calibre delivery points at are kept. Thanks to magrhino for the report.

changelog.d/2869-isbn-lookup-cache.md

@@ -0,0 +1,2 @@
+### Fixed
+- **ISBN and Audible ASIN lookups no longer hand out a stale or shared record** (#2869). An ISBN or ASIN answer was cached for a day under a key that ignored the metadata provider settings, so changing the primary provider or the Hardcover token kept serving the old answer, and every caller was handed the same cached book. Each lookup now gets its own copy, a settings change fetches a fresh answer, and a record built while an enrichment provider was failing is kept for five minutes instead of a day so it is retried. Thanks to magrhino for the report.

changelog.d/3091-supplement-translations.md

@@ -0,0 +1,2 @@
+### Fixed
+- **Translations from Hardcover no longer slip past the language filter when OpenLibrary is the primary provider** (#3091). Hardcover's edition languages were only checked when Hardcover was the primary provider, so with OpenLibrary primary its translated works arrived with no language and the default "pass" setting let them in, often monitored and wanted. They are now checked whenever a Hardcover token is set, and a work whose only known editions are in other languages is skipped. A work whose title is in a different script from the author's other titles no longer inherits the author's language, and OpenLibrary no longer spends a request per Hardcover work looking for editions it does not have. Thanks to magrhino for the report.

docs/Metadata-Editing-Wiki.md

@@ -29,6 +29,13 @@ Two actions clear locks on their own, deliberately: **Re-bind** and metadata
 **re-map**. Both mean "this is the wrong record, take the new one", so
 Bindery applies the new provider record wholesale.
 
+A re-bind to a different record also drops the editions the metadata provider
+listed for the old record, so their ISBNs no longer steer searches for the
+book. Editions created by the Calibre or Audiobookshelf import for your files
+are kept, and so is the edition you selected for the book and any edition a
+download or a Calibre delivery recorded. Re-binding to the record the book is
+already bound to changes no editions.
+
 ## What you cannot edit: an author's sort name
 
 The value that decides which letter an author files under, in the Authors list

docs/User-Guide-Wiki.md

@@ -791,15 +791,26 @@ like everything else on that tab that names server paths.
   selector stays disabled until you save one.
 
   When a metadata profile restricts languages, Bindery checks Hardcover's
-  editions for each author work in one batched request. A translated default
-  edition is not treated as the language of the whole work: any edition in an
-  allowed language keeps the work, while a work is rejected as non-allowed
-  only when the lookup completes and finds no allowed edition. This filtering
-  evidence does not rewrite the displayed language, which remains the
+  editions for each author work in one batched request. This happens whether
+  Hardcover is the primary provider or supplements OpenLibrary, so the
+  Hardcover works merged into an OpenLibrary author are checked too. A
+  translated default edition is not treated as the language of the whole work:
+  any edition in an allowed language keeps the work, while a work is rejected
+  as non-allowed only when the lookup completes and finds no allowed edition
+  and some edition, default or not, records another language. A work with no
+  language on any edition stays unknown. This filtering evidence does not
+  rewrite the displayed language, which remains the
   provider's preferred/default language or the user's locked value. If the
   evidence is indeterminate or its lookup fails, normal refreshes fall through
   to the existing edition-sampled, author-majority, and scalar language before
-  applying **When book language is unknown**. **Reconcile catalogue** treats a
+  applying **When book language is unknown**. The author-majority fallback is
+  not applied to a work whose title is written in a different script from the
+  author's other titles (a Cyrillic title in an English author's catalogue, for
+  example), so such a work stays unknown. OpenLibrary edition sampling only
+  runs for OpenLibrary works, and takes the language and a missing cover from
+  the edition OpenLibrary features on the work's own page first, falling back
+  to a small sample of its editions (and a cover in the sampled language) only
+  for what that edition lacks. **Reconcile catalogue** treats a
   failed lookup as indeterminate rather than offering the row for removal.
 - **Google Books** (free API key) and **Audnexus/Audible** (audiobook
   narrator, duration, by ASIN) enrich further.

internal/api/author_majority_language_test.go

@@ -0,0 +1,91 @@
+package api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/vavallee/bindery/internal/db"
+	"github.com/vavallee/bindery/internal/metadata"
+	"github.com/vavallee/bindery/internal/models"
+)
+
+// The author-majority language fallback stamps the author's main language on
+// works nothing else could resolve. It must not stamp it on a title written in
+// another script: that is a translation, and stamping "eng" on it let it past
+// even unknown_language_behavior = fail (#3091). A work its provider proved is
+// in a non-allowed language is rejected on that evidence, and is not stamped
+// either.
+func TestFetchAuthorBooks_MajorityLanguageSkipsUnresolvableTranslations(t *testing.T) {
+	database, err := db.OpenMemory()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer database.Close()
+
+	ctx := context.Background()
+	authorRepo := db.NewAuthorRepo(database)
+	bookRepo := db.NewBookRepo(database)
+	profileRepo := db.NewMetadataProfileRepo(database)
+	profile, err := profileRepo.GetByID(ctx, models.DefaultMetadataProfileID)
+	if err != nil || profile == nil {
+		t.Fatalf("GetByID(default profile): profile=%+v err=%v", profile, err)
+	}
+	profile.AllowedLanguages = "eng"
+	profile.UnknownLanguageBehavior = models.UnknownLanguageFail
+	if err := profileRepo.Update(ctx, profile); err != nil {
+		t.Fatal(err)
+	}
+	author := &models.Author{
+		ForeignID: "hc:orson-scott-card", Name: "Orson Scott Card", SortName: "Card, Orson Scott",
+		MetadataProvider: "hardcover", Monitored: false,
+	}
+	if err := authorRepo.Create(ctx, author); err != nil {
+		t.Fatal(err)
+	}
+	work := func(id, title, language string) models.Book {
+		return models.Book{
+			ForeignID: id, Title: title, SortTitle: title, Language: language,
+			MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "hardcover",
+		}
+	}
+	provider := &languageEvidenceMetaProvider{
+		stubMetaProvider: stubMetaProvider{name: "hardcover", works: []models.Book{
+			work("hc:enders-game", "Ender's Game", "eng"),
+			work("hc:speaker-for-the-dead", "Speaker for the Dead", "eng"),
+			work("hc:xenocide", "Xenocide", "eng"),
+			work("hc:children-of-the-mind", "Children of the Mind", "eng"),
+			work("hc:seventh-son", "Seventh Son", "eng"),
+			// The provider found only Spanish editions.
+			work("hc:el-septimo-hijo", "El séptimo hijo", ""),
+			// No evidence either way, but the title is in another script.
+			work("hc:rozhbi-na-saznanieto", "Рожби на съзнанието", ""),
+			// No evidence and a Latin title: the fallback still applies.
+			work("hc:red-prophet", "Red Prophet", ""),
+		}},
+		evidence: map[string]metadata.AuthorWorkLanguageEvidence{
+			"hc:el-septimo-hijo": {State: metadata.AuthorWorkLanguageNotAllowed, Language: "spa"},
+		},
+	}
+	h := NewAuthorHandler(authorRepo, nil, bookRepo, nil, metadata.NewAggregator(provider), nil, profileRepo, nil)
+	h.FetchAuthorBooks(author, false, models.MediaTypeEbook)
+
+	books, err := bookRepo.ListByAuthor(ctx, author.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := map[string]string{}
+	for _, b := range books {
+		got[b.ForeignID] = b.Language
+	}
+	for _, translation := range []string{"hc:el-septimo-hijo", "hc:rozhbi-na-saznanieto"} {
+		if language, added := got[translation]; added {
+			t.Errorf("%s added with language %q; a translation must not inherit the author's language", translation, language)
+		}
+	}
+	if got["hc:red-prophet"] != "eng" {
+		t.Errorf("hc:red-prophet language = %q, want eng from the author majority (books: %v)", got["hc:red-prophet"], got)
+	}
+	if len(books) != 6 {
+		t.Errorf("added %d books, want the five English works and Red Prophet: %v", len(books), got)
+	}
+}

internal/api/authors.go

@@ -1977,7 +1977,7 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut
 	// OpenLibrary metadata gap it has no way to tell apart from an actual
 	// foreign-language book.
 	if len(allowedLangs) > 0 {
-		applyAuthorMajorityLanguageFallback(books)
+		applyAuthorMajorityLanguageFallback(books, languageEvidence)
 	}
 
 	// Everything above can take minutes for a prolific author (works fetch,
@@ -3698,7 +3698,17 @@ const authorMajorityLanguageThreshold = 0.9
 // least authorMajorityLanguageMinSample resolved works. Mutates books in
 // place. A no-op when too few works have a resolved language yet, or when
 // no single language dominates strongly enough to trust.
-func applyAuthorMajorityLanguageFallback(books []models.Book) {
+//
+// A blank work whose title is written in a different script from the
+// majority's titles is left blank (#3091): "Рожби на съзнанието" in an English
+// author's catalogue is a translation, and stamping "eng" on it let it past
+// even unknown_language_behavior = fail. It keeps an unknown language, so the
+// profile's unknown-language rule decides it. So is a work its provider has
+// already proved is in a language the profile does not allow: the filter
+// rejects it on that evidence anyway, and the skipped sample should not show
+// it as English. Indeterminate evidence still gets the fallback, as it did
+// before: it means the provider found no edition in any language (#2754).
+func applyAuthorMajorityLanguageFallback(books []models.Book, evidence map[string]metadata.AuthorWorkLanguageEvidence) {
 	counts := make(map[string]int, 4)
 	resolved := 0
 	for _, b := range books {
@@ -3721,10 +3731,18 @@ func applyAuthorMajorityLanguageFallback(books []models.Book) {
 	if float64(majorityCount)/float64(resolved) < authorMajorityLanguageThreshold {
 		return
 	}
+	majorityScript := majorityTitleScript(books, majorityLang)
 	for i := range books {
-		if books[i].Language == "" {
-			books[i].Language = majorityLang
+		if books[i].Language != "" {
+			continue
+		}
+		if found, ok := evidence[strings.TrimSpace(books[i].ForeignID)]; ok && found.State == metadata.AuthorWorkLanguageNotAllowed {
+			continue
+		}
+		if script := titleScript(books[i].Title); script != "" && majorityScript != "" && script != majorityScript {
+			continue
 		}
+		books[i].Language = majorityLang
 	}
 }
 

internal/api/authors_test.go

@@ -5551,7 +5551,7 @@ func TestApplyAuthorMajorityLanguageFallback(t *testing.T) {
 			{ForeignID: "5", Language: ""}, // unresolved: should be backfilled
 			{ForeignID: "6", Language: ""}, // unresolved: should be backfilled
 		}
-		applyAuthorMajorityLanguageFallback(books)
+		applyAuthorMajorityLanguageFallback(books, nil)
 		for _, b := range books {
 			if b.Language != "eng" {
 				t.Errorf("book %s: Language = %q, want eng", b.ForeignID, b.Language)
@@ -5567,7 +5567,7 @@ func TestApplyAuthorMajorityLanguageFallback(t *testing.T) {
 			{ForeignID: "4", Language: "ger"},
 			{ForeignID: "5", Language: ""},
 		}
-		applyAuthorMajorityLanguageFallback(books)
+		applyAuthorMajorityLanguageFallback(books, nil)
 		if books[4].Language != "" {
 			t.Errorf("Language = %q, want unchanged (empty) — no language clears the dominance threshold", books[4].Language)
 		}
@@ -5579,7 +5579,7 @@ func TestApplyAuthorMajorityLanguageFallback(t *testing.T) {
 			{ForeignID: "2", Language: "eng"},
 			{ForeignID: "3", Language: ""},
 		}
-		applyAuthorMajorityLanguageFallback(books)
+		applyAuthorMajorityLanguageFallback(books, nil)
 		if books[2].Language != "" {
 			t.Errorf("Language = %q, want unchanged (empty) — only 2 resolved works is below the minimum sample", books[2].Language)
 		}
@@ -5590,7 +5590,7 @@ func TestApplyAuthorMajorityLanguageFallback(t *testing.T) {
 			{ForeignID: "1", Language: ""},
 			{ForeignID: "2", Language: ""},
 		}
-		applyAuthorMajorityLanguageFallback(books)
+		applyAuthorMajorityLanguageFallback(books, nil)
 		for _, b := range books {
 			if b.Language != "" {
 				t.Errorf("book %s: Language = %q, want unchanged (empty)", b.ForeignID, b.Language)

internal/api/books.go

@@ -1302,6 +1302,17 @@ func (h *BookHandler) Rebind(w http.ResponseWriter, r *http.Request) {
 		writeJSON(w, http.StatusConflict, map[string]string{"error": "This book changed while it was being rebound. Try again.", "reason": conflictReasonChanged})
 		return
 	}
+	// The previous work's provider editions describe the wrong work: their
+	// ISBN would keep steering search, and their unique ids keep them from
+	// the book they belong to (#2781). Only when the identity actually
+	// changed, and never an imported, selected or referenced edition.
+	if h.editions != nil && (oldForeignID != book.ForeignID || oldProvider != book.MetadataProvider) {
+		if n, err := h.editions.DeleteProviderEditionsAfterRebind(r.Context(), book.ID, book.ForeignID, book.MetadataProvider); err != nil {
+			slog.Warn("rebind: failed to remove the previous work's editions", "bookId", book.ID, "error", err)
+		} else if n > 0 {
+			slog.Info("rebind: removed the previous work's provider editions", "bookId", book.ID, "removed", n, "oldForeignId", oldForeignID)
+		}
+	}
 	// MediaType is in the preserved-fields list above because it belongs to the
 	// user, so a rebind is exactly the case the pin exists for: hydration must
 	// not widen the format the user keeps (#2768).