diff --git a/changelog.d/2235-min-edition-count.md b/changelog.d/2235-min-edition-count.md new file mode 100644 index 000000000..d3d5b2166 --- /dev/null +++ b/changelog.d/2235-min-edition-count.md @@ -0,0 +1,2 @@ +### Added +- **Minimum edition count on metadata profiles** ([#2235](https://github.com/vavallee/bindery/issues/2235)): an opt in **Min editions** floor under Settings, Metadata that keeps thin, one off records out of an author's catalogue when it syncs. A work is judged on the best edition count among the works sharing its title, so one well known record is enough to keep the title. Only OpenLibrary reports edition counts, so the filter only acts on OpenLibrary records: a work with no known count passes, and a Hardcover catalogue is never touched. Books already in your library are never dropped. It is off by default (0). Thanks gchahcg. diff --git a/docs/User-Guide-Wiki.md b/docs/User-Guide-Wiki.md index 80f057e22..f5bc5b5ba 100644 --- a/docs/User-Guide-Wiki.md +++ b/docs/User-Guide-Wiki.md @@ -671,8 +671,11 @@ When metadata is wrong, you have three levels of fix: never overwrite them ([guide](Metadata-Editing-Wiki.md)). 2. **Re-bind** the book, or **relink** the author ("Find better match"), to a different provider record when the match itself is wrong. -3. A **metadata profile** (languages, minimum page count, skip part books) - filters what a catalogue sync lets in. +3. A **metadata profile** (languages, minimum page count, minimum edition + count, skip part books) filters what a catalogue sync lets in. Filling a + series skips every metadata profile filter today, the edition count included + ([#2208](https://github.com/vavallee/bindery/issues/2208)), so a filled + series can still bring in thin works. Box sets need no setting. A work whose title plainly names a bundle ("... Box Set", "3 Books Set", "Carton of 10 Signed Copies") is dropped from every diff --git a/internal/api/authors.go b/internal/api/authors.go index 984adf9fd..23114c7a4 100644 --- a/internal/api/authors.go +++ b/internal/api/authors.go @@ -1875,6 +1875,7 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut skipPartBooks := h.resolveSkipPartBooks(ctx, author) skipMissingDate := h.resolveSkipMissingDate(ctx, author) minPages, skipMissingISBN := h.resolveEditionFilters(ctx, author) + minEditionCount := h.resolveMinEditionCount(ctx, author) var languageEvidence map[string]metadata.AuthorWorkLanguageEvidence if !singleWork { var languageEvidenceErr error @@ -2133,7 +2134,7 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut // the write. Both were invisible, which made Total minus everything else // look like a hole. var added, matched, failed, skippedLang, skippedJunk, skippedMediaType, skippedNotAccepted, skippedExcluded int - var skippedPartBooks, skippedMissingDate, skippedMinPages, skippedMissingISBN int + var skippedPartBooks, skippedMissingDate, skippedMinPages, skippedMissingISBN, skippedThinCluster int // Names of the first few language-rejected works, reported to the user // alongside the count (#1889): "65 books skipped" is alarming, but it is // the titles and their language codes that tell them whether the profile @@ -2144,6 +2145,20 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut // again for these filters specifically in PR review, vavallee). var skippedPartBooksSample, skippedMissingDateSample []models.AuthorSyncSkippedBook var skippedMinPagesSample, skippedMissingISBNSample []models.AuthorSyncSkippedBook + var skippedThinClusterSample []models.AuthorSyncSkippedBook + // clusterEditions maps a normalised title to the highest known edition + // count among the works carrying that title, built as the candidates + // accumulate below. It backs the MinEditionCount filter in the loop that + // follows (#2235): a title whose works all report fewer editions than the + // profile's floor is the noise shape that filter drops. It is only + // populated when the filter is actually on, so a profile without it pays + // nothing. A title absent from the map, or mapped to 0, means "no work in + // the cluster reported an edition count" — unknown, not zero — and the + // filter lets it through, the same unknown-passes semantics MinPages uses. + var clusterEditions map[string]int + if minEditionCount > 0 { + clusterEditions = make(map[string]int, len(books)) + } // candidates accumulates every work that survives the free (in-memory) // filters below and would otherwise reach the MinPages/SkipMissingISBN // check. Splitting the loop here lets the edition lookups those two @@ -2220,6 +2235,9 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut continue } + if clusterEditions != nil && b.EditionCount > clusterEditions[normalizedTitle] { + clusterEditions[normalizedTitle] = b.EditionCount + } candidates = append(candidates, b) } @@ -2317,6 +2335,37 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut continue } + // Filter works whose title cluster is thinner than the profile's + // MinEditionCount floor (#2235). Edition count only separates real + // works from one-off noise once records are grouped by title, so the + // judgement is made on the cluster's best-known count, not each work's + // own: a title with at least one well-editioned work is a real book + // even if this particular record is thin. The cluster map was built in + // the loop above from works that already survived the junk and + // language filters. + // + // The exemptions mirror the other discovery filters: a book the user + // already owns (existing != nil) is maintained, never dropped, and a + // single-work run is exempt per #1612 — an explicit add of one + // specific work must not be vetoed by catalogue-sync heuristics. + // A cluster with no known edition count (map miss, or 0) is unknown, + // not zero, and passes: only OpenLibrary search results populate + // EditionCount, so a Hardcover-primary author's catalogue — where + // every work reports none — must survive a profile that turned this + // filter on. + if existing == nil && !singleWork && minEditionCount > 0 { + clusterKey := strings.ToLower(strings.TrimSpace(b.Title)) + if count, ok := clusterEditions[clusterKey]; ok && count > 0 && count < minEditionCount { + skippedThinCluster++ + if len(skippedThinClusterSample) < authorSyncSkippedSampleLimit { + skippedThinClusterSample = append(skippedThinClusterSample, models.AuthorSyncSkippedBook{Title: b.Title}) + } + slog.Debug("skipping work from a title cluster below the edition-count floor", + "title", b.Title, "foreignId", b.ForeignID, "clusterEditions", count, "minEditionCount", minEditionCount) + continue + } + } + // MinPages / SkipMissingISBN both need edition data (page count and // ISBN live on Edition, not Book) fetched in the prefetch pass above. // A book with no entry in editionsByForeignID is either not gated by @@ -2702,6 +2751,8 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut SkippedMinPagesSample: skippedMinPagesSample, SkippedMissingISBN: skippedMissingISBN, SkippedMissingISBNSample: skippedMissingISBNSample, + SkippedThinCluster: skippedThinCluster, + SkippedThinClusterSample: skippedThinClusterSample, AllowedLanguages: allowedLangs, UnknownLanguageFail: unknownFail, SkippedLanguageSample: skippedLangSample, @@ -2719,6 +2770,7 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut "skipped_part_books", skippedPartBooks, "skipped_missing_date", skippedMissingDate, "skipped_min_pages", skippedMinPages, "skipped_missing_isbn", skippedMissingISBN, + "skipped_thin_cluster", skippedThinCluster, "total", len(books), } // The metadata filters dropping works is the surprising case and stays at @@ -2730,7 +2782,7 @@ func (h *AuthorHandler) runCatalogueSync(ctx context.Context, author *models.Aut // filter dropping a work is a setting doing its job, while a create that // lost its write is the run failing at the thing it exists to do. if failed+skippedLang+skippedJunk+skippedMediaType+skippedPartBooks+skippedMissingDate+ - skippedMinPages+skippedMissingISBN > 0 { + skippedMinPages+skippedMissingISBN+skippedThinCluster > 0 { slog.Warn("author books synced", logArgs...) return added, nil } @@ -3509,6 +3561,22 @@ func (h *AuthorHandler) resolveEditionFilters(ctx context.Context, author *model return p.MinPages, p.SkipMissingISBN } +// resolveMinEditionCount returns the author's effective metadata profile's +// MinEditionCount floor (#2235). Defaults to 0 (filter disabled) on any +// lookup failure, so an unresolvable profile never turns into unexpected +// catalogue loss. +func (h *AuthorHandler) resolveMinEditionCount(ctx context.Context, author *models.Author) int { + id := models.DefaultMetadataProfileID + if author.MetadataProfileID != nil { + id = *author.MetadataProfileID + } + p, err := h.profiles.GetByID(ctx, id) + if err != nil || p == nil { + return 0 + } + return p.MinEditionCount +} + // anyEditionHasISBN reports whether any edition carries an ISBN-13 or // ISBN-10. Returns false for a nil or empty slice — a work with no editions // to check has no ISBN to confirm. diff --git a/internal/api/authors_test.go b/internal/api/authors_test.go index d4d737dc3..7c98cfad8 100644 --- a/internal/api/authors_test.go +++ b/internal/api/authors_test.go @@ -7687,6 +7687,274 @@ func TestFetchAuthorBooks_SkipMissingISBNExemptsAlreadyTrackedBook(t *testing.T) } } +// thinClusterTestWorks is the catalogue for the MinEditionCount tests (#2235). +// "Thin Novel" sits below a floor of 3 and is the work the filter drops. +// "Fat Novel" clears it. "Unknown Novel" reports no edition count at all — +// the Hardcover-primary shape — and must pass. "Clustered Novel" appears as +// two works under one title, one thin (1) and one well-editioned (5): the +// filter judges the cluster's best-known count, so both survive. +func thinClusterTestWorks() []models.Book { + return []models.Book{ + {ForeignID: "OL990W", Title: "Thin Novel", SortTitle: "thin novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary", + EditionCount: 1}, + {ForeignID: "OL991W", Title: "Fat Novel", SortTitle: "fat novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary", + EditionCount: 5}, + {ForeignID: "OL992W", Title: "Unknown Novel", SortTitle: "unknown novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary"}, + {ForeignID: "OL993W", Title: "Clustered Novel", SortTitle: "clustered novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary", + EditionCount: 1}, + {ForeignID: "OL994W", Title: "Clustered Novel", SortTitle: "clustered novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary", + EditionCount: 5}, + } +} + +// TestFetchAuthorBooks_SkipsThinCluster verifies that once a metadata +// profile's MinEditionCount is set, works whose title cluster reports fewer +// editions than the floor are dropped, while works clearing the floor, works +// with no known edition count (unknown, not zero), and a title cluster that +// carries at least one well-editioned work all come through. +func TestFetchAuthorBooks_SkipsThinCluster(t *testing.T) { + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + + authorRepo := db.NewAuthorRepo(database) + bookRepo := db.NewBookRepo(database) + profileRepo := db.NewMetadataProfileRepo(database) + settingsRepo := db.NewSettingsRepo(database) + ctx := context.Background() + + profile, err := profileRepo.GetByID(ctx, models.DefaultMetadataProfileID) + if err != nil || profile == nil { + t.Fatalf("GetByID(default profile) failed: %v", err) + } + profile.MinEditionCount = 3 + if err := profileRepo.Update(ctx, profile); err != nil { + t.Fatal(err) + } + + author := &models.Author{ + ForeignID: "OL990A", Name: "Editions Author", SortName: "Author, Editions", + MetadataProvider: "openlibrary", Monitored: false, + } + if err := authorRepo.Create(ctx, author); err != nil { + t.Fatal(err) + } + + works := thinClusterTestWorks() + agg := metadata.NewAggregator(&stubMetaProvider{works: works}) + h := NewAuthorHandler(authorRepo, nil, bookRepo, nil, agg, settingsRepo, profileRepo, nil) + h.FetchAuthorBooks(author, false, models.MediaTypeEbook) + + got, err := bookRepo.ListByAuthor(ctx, author.ID) + if err != nil { + t.Fatal(err) + } + byTitle := make(map[string]bool, len(got)) + for _, b := range got { + byTitle[b.Title] = true + } + + if byTitle["Thin Novel"] { + t.Error("work below the edition-count floor should have been skipped, but was created") + } + for _, want := range []string{"Fat Novel", "Unknown Novel", "Clustered Novel"} { + if !byTitle[want] { + t.Errorf("work %q should have been created, but was skipped", want) + } + } + + summary := h.syncSummaries.get(author.ID) + if summary == nil { + t.Fatal("expected a recorded sync summary, got nil") + } + if want := 1; summary.SkippedThinCluster != want { + t.Errorf("summary.SkippedThinCluster = %d, want %d", summary.SkippedThinCluster, want) + } + if summary.SkippedTotal() < summary.SkippedThinCluster { + t.Errorf("summary.SkippedTotal() = %d should include SkippedThinCluster = %d", summary.SkippedTotal(), summary.SkippedThinCluster) + } + sampleTitles := make(map[string]bool, len(summary.SkippedThinClusterSample)) + for _, b := range summary.SkippedThinClusterSample { + sampleTitles[b.Title] = true + } + if !sampleTitles["Thin Novel"] { + t.Errorf("expected %q in summary.SkippedThinClusterSample, got %+v", "Thin Novel", summary.SkippedThinClusterSample) + } +} + +// TestFetchAuthorBooks_ThinClusterKeptWhenZero verifies the inverse of +// TestFetchAuthorBooks_SkipsThinCluster: with MinEditionCount left at its +// default (0), no work is dropped by the edition-count filter — it is +// opt-in, and a profile without it pays nothing. +func TestFetchAuthorBooks_ThinClusterKeptWhenZero(t *testing.T) { + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + + authorRepo := db.NewAuthorRepo(database) + bookRepo := db.NewBookRepo(database) + profileRepo := db.NewMetadataProfileRepo(database) + settingsRepo := db.NewSettingsRepo(database) + ctx := context.Background() + + author := &models.Author{ + ForeignID: "OL991A", Name: "Editions Author Two", SortName: "Author, Editions Two", + MetadataProvider: "openlibrary", Monitored: false, + } + if err := authorRepo.Create(ctx, author); err != nil { + t.Fatal(err) + } + + works := thinClusterTestWorks() + agg := metadata.NewAggregator(&stubMetaProvider{works: works}) + h := NewAuthorHandler(authorRepo, nil, bookRepo, nil, agg, settingsRepo, profileRepo, nil) + h.FetchAuthorBooks(author, false, models.MediaTypeEbook) + + got, err := bookRepo.ListByAuthor(ctx, author.ID) + if err != nil { + t.Fatal(err) + } + // Four books, not five: the two "Clustered Novel" works share a title, + // and the sync's pre-existing title dedup folds the second into the + // first (matched, not created). That merge is independent of the + // edition-count filter — the point here is that nothing is DROPPED. + if len(got) != 4 { + t.Errorf("got %d books, want 4 (MinEditionCount defaults to 0, nothing should be dropped; the twin Clustered Novel works merge via title dedup)", len(got)) + } + if summary := h.syncSummaries.get(author.ID); summary != nil && summary.SkippedThinCluster != 0 { + t.Errorf("summary.SkippedThinCluster = %d, want 0 (filter is opt-in)", summary.SkippedThinCluster) + } +} + +// TestFetchAuthorBooks_ThinClusterExemptsAlreadyTrackedBook verifies that the +// edition-count filter screens works out of discovery without stopping the +// maintenance of a book the user already owns: a thin work that resolves to +// an existing row is refreshed, not dropped, and not counted as skipped. +func TestFetchAuthorBooks_ThinClusterExemptsAlreadyTrackedBook(t *testing.T) { + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + + authorRepo := db.NewAuthorRepo(database) + bookRepo := db.NewBookRepo(database) + profileRepo := db.NewMetadataProfileRepo(database) + settingsRepo := db.NewSettingsRepo(database) + ctx := context.Background() + + profile, err := profileRepo.GetByID(ctx, models.DefaultMetadataProfileID) + if err != nil || profile == nil { + t.Fatalf("GetByID(default profile) failed: %v", err) + } + profile.MinEditionCount = 3 + if err := profileRepo.Update(ctx, profile); err != nil { + t.Fatal(err) + } + + author := &models.Author{ + ForeignID: "OL992A", Name: "Editions Author Three", SortName: "Author, Editions Three", + MetadataProvider: "openlibrary", Monitored: false, + } + if err := authorRepo.Create(ctx, author); err != nil { + t.Fatal(err) + } + + owned := &models.Book{ + ForeignID: "OL990W", Title: "Thin Novel", SortTitle: "thin novel", + AuthorID: author.ID, Language: "eng", Status: models.BookStatusWanted, + MediaType: models.MediaTypeEbook, MetadataProvider: "openlibrary", + } + if err := bookRepo.Create(ctx, owned); err != nil { + t.Fatal(err) + } + + works := []models.Book{ + {ForeignID: "OL990W", Title: "Thin Novel", SortTitle: "thin novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary", + EditionCount: 1, AverageRating: 4.2, RatingsCount: 250}, + } + agg := metadata.NewAggregator(&stubMetaProvider{works: works}) + h := NewAuthorHandler(authorRepo, nil, bookRepo, nil, agg, settingsRepo, profileRepo, nil) + h.FetchAuthorBooks(author, false, models.MediaTypeEbook) + + updated, err := bookRepo.GetByForeignID(ctx, "OL990W") + if err != nil { + t.Fatal(err) + } + if updated == nil { + t.Fatal("already-tracked thin work was deleted, want it kept") + } + if updated.RatingsCount != 250 { + t.Errorf("RatingsCount = %d, want 250 (already-tracked book should still receive updates)", updated.RatingsCount) + } + + if summary := h.syncSummaries.get(author.ID); summary != nil && summary.SkippedThinCluster != 0 { + t.Errorf("summary.SkippedThinCluster = %d, want 0 (already-tracked book must not be counted as skipped)", summary.SkippedThinCluster) + } +} + +// TestFetchAuthorBooks_ThinClusterSingleWorkExempt verifies the #1612 +// exemption on the edition-count filter: an explicit add of one specific +// work must not be vetoed by catalogue-sync heuristics, so a single-work run +// creates the requested work even when it sits below the profile's floor. +func TestFetchAuthorBooks_ThinClusterSingleWorkExempt(t *testing.T) { + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + + authorRepo := db.NewAuthorRepo(database) + bookRepo := db.NewBookRepo(database) + profileRepo := db.NewMetadataProfileRepo(database) + settingsRepo := db.NewSettingsRepo(database) + ctx := context.Background() + + profile, err := profileRepo.GetByID(ctx, models.DefaultMetadataProfileID) + if err != nil || profile == nil { + t.Fatalf("GetByID(default profile) failed: %v", err) + } + profile.MinEditionCount = 3 + if err := profileRepo.Update(ctx, profile); err != nil { + t.Fatal(err) + } + + author := &models.Author{ + ForeignID: "OL993A", Name: "Editions Author Four", SortName: "Author, Editions Four", + MetadataProvider: "openlibrary", Monitored: false, + } + if err := authorRepo.Create(ctx, author); err != nil { + t.Fatal(err) + } + + works := []models.Book{ + {ForeignID: "OL990W", Title: "Thin Novel", SortTitle: "thin novel", Language: "eng", + MediaType: models.MediaTypeEbook, Status: models.BookStatusWanted, MetadataProvider: "openlibrary", + EditionCount: 1}, + } + agg := metadata.NewAggregator(&stubMetaProvider{works: works}) + h := NewAuthorHandler(authorRepo, nil, bookRepo, nil, agg, settingsRepo, profileRepo, nil) + h.fetchAuthorBooks(ctx, author, catalogueSyncOptions{mediaType: models.MediaTypeEbook, onlyForeignID: "OL990W"}) + + created, err := bookRepo.GetByForeignID(ctx, "OL990W") + if err != nil { + t.Fatal(err) + } + if created == nil { + t.Fatal("explicitly added single work was vetoed by the edition-count filter, want it created (#1612)") + } +} + // TestIsPartBookTitle is a fast, DB-free check of partBookTitleRe against // every noise pattern denoise_author.py was built to catch (confirmed // against real OpenLibrary titles pulled during development) plus the diff --git a/internal/db/metadata_profiles.go b/internal/db/metadata_profiles.go index ad25eba77..97b63053b 100644 --- a/internal/db/metadata_profiles.go +++ b/internal/db/metadata_profiles.go @@ -18,9 +18,9 @@ func NewMetadataProfileRepo(db *sql.DB) *MetadataProfileRepo { func (r *MetadataProfileRepo) List(ctx context.Context) ([]models.MetadataProfile, error) { rows, err := r.db.QueryContext(ctx, ` - SELECT id, name, min_popularity, min_pages, skip_missing_date, skip_missing_isbn, - skip_part_books, allowed_languages, unknown_language_behavior, created_at, - COALESCE(owner_user_id, 0) + SELECT id, name, min_popularity, min_pages, min_edition_count, skip_missing_date, + skip_missing_isbn, skip_part_books, allowed_languages, unknown_language_behavior, + created_at, COALESCE(owner_user_id, 0) FROM metadata_profiles ORDER BY id`) if err != nil { return nil, fmt.Errorf("list metadata profiles: %w", err) @@ -40,9 +40,9 @@ func (r *MetadataProfileRepo) List(ctx context.Context) ([]models.MetadataProfil func (r *MetadataProfileRepo) GetByID(ctx context.Context, id int64) (*models.MetadataProfile, error) { rows, err := r.db.QueryContext(ctx, ` - SELECT id, name, min_popularity, min_pages, skip_missing_date, skip_missing_isbn, - skip_part_books, allowed_languages, unknown_language_behavior, created_at, - COALESCE(owner_user_id, 0) + SELECT id, name, min_popularity, min_pages, min_edition_count, skip_missing_date, + skip_missing_isbn, skip_part_books, allowed_languages, unknown_language_behavior, + created_at, COALESCE(owner_user_id, 0) FROM metadata_profiles WHERE id=?`, id) if err != nil { return nil, fmt.Errorf("get metadata profile %d: %w", id, err) @@ -88,11 +88,11 @@ func (r *MetadataProfileRepo) create(ctx context.Context, p *models.MetadataProf ownerArg = ownerUserID } result, err := r.db.ExecContext(ctx, ` - INSERT INTO metadata_profiles (name, min_popularity, min_pages, skip_missing_date, - skip_missing_isbn, skip_part_books, allowed_languages, - unknown_language_behavior, owner_user_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, - p.Name, p.MinPopularity, p.MinPages, + INSERT INTO metadata_profiles (name, min_popularity, min_pages, min_edition_count, + skip_missing_date, skip_missing_isbn, skip_part_books, + allowed_languages, unknown_language_behavior, owner_user_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + p.Name, p.MinPopularity, p.MinPages, p.MinEditionCount, p.SkipMissingDate, p.SkipMissingISBN, p.SkipPartBooks, p.AllowedLanguages, p.UnknownLanguageBehavior, ownerArg) if err != nil { @@ -112,11 +112,11 @@ func (r *MetadataProfileRepo) Update(ctx context.Context, p *models.MetadataProf p.UnknownLanguageBehavior = models.UnknownLanguagePass } _, err := r.db.ExecContext(ctx, ` - UPDATE metadata_profiles SET name=?, min_popularity=?, min_pages=?, skip_missing_date=?, - skip_missing_isbn=?, skip_part_books=?, allowed_languages=?, - unknown_language_behavior=? + UPDATE metadata_profiles SET name=?, min_popularity=?, min_pages=?, min_edition_count=?, + skip_missing_date=?, skip_missing_isbn=?, skip_part_books=?, + allowed_languages=?, unknown_language_behavior=? WHERE id=?`, - p.Name, p.MinPopularity, p.MinPages, + p.Name, p.MinPopularity, p.MinPages, p.MinEditionCount, p.SkipMissingDate, p.SkipMissingISBN, p.SkipPartBooks, p.AllowedLanguages, p.UnknownLanguageBehavior, p.ID) if err != nil { @@ -137,7 +137,7 @@ func scanMetadataProfile(rows *sql.Rows) (models.MetadataProfile, error) { var p models.MetadataProfile var skipDate, skipISBN, skipPart int err := rows.Scan( - &p.ID, &p.Name, &p.MinPopularity, &p.MinPages, + &p.ID, &p.Name, &p.MinPopularity, &p.MinPages, &p.MinEditionCount, &skipDate, &skipISBN, &skipPart, &p.AllowedLanguages, &p.UnknownLanguageBehavior, &p.CreatedAt, &p.OwnerUserID, ) diff --git a/internal/db/migrations/094_metadata_profile_min_edition_count.sql b/internal/db/migrations/094_metadata_profile_min_edition_count.sql new file mode 100644 index 000000000..0ac655356 --- /dev/null +++ b/internal/db/migrations/094_metadata_profile_min_edition_count.sql @@ -0,0 +1,6 @@ +-- min_edition_count on metadata_profiles (#2235): opt-in floor on the +-- edition count of a work's title cluster during author sync. 0 (the +-- default) disables the filter. Only OpenLibrary search results populate +-- Book.EditionCount; a work with no count is treated as unknown and passes, +-- the same "unknown is not zero" semantics MinPages uses for page counts. +ALTER TABLE metadata_profiles ADD COLUMN min_edition_count INTEGER NOT NULL DEFAULT 0; diff --git a/internal/models/author.go b/internal/models/author.go index 62e10b7e4..19f196a7f 100644 --- a/internal/models/author.go +++ b/internal/models/author.go @@ -161,8 +161,9 @@ type AuthorSyncSummary struct { // the user meant. SkippedLanguageSample []AuthorSyncSkippedBook `json:"skippedLanguageSample,omitempty"` - // The five fields below back the metadata-profile filters wired into - // author sync by PRs #1968, #2005, #2006, #2007, and #2008. Landed here + // The six fields below back the metadata-profile filters wired into + // author sync by PRs #1968, #2005, #2006, #2007, #2008, and the + // min_edition_count filter (#2235). Landed here // first (#1889-shaped review feedback, vavallee) so those PRs rebase onto // fields and UI that already exist rather than each adding — and each // needing rebased around the others adding — the same summary/notice @@ -192,6 +193,13 @@ type AuthorSyncSummary struct { // profile's SkipMissingISBN setting enabled. SkippedMissingISBN int `json:"skippedMissingIsbn,omitempty"` SkippedMissingISBNSample []AuthorSyncSkippedBook `json:"skippedMissingIsbnSample,omitempty"` + // SkippedThinCluster is the number of works dropped because every work + // sharing their title reports fewer editions than the metadata profile's + // MinEditionCount floor, with that setting enabled. A work with no known + // edition count is unknown, not zero, and its title cluster passes + // (same semantics as SkippedMinPages). + SkippedThinCluster int `json:"skippedThinCluster,omitempty"` + SkippedThinClusterSample []AuthorSyncSkippedBook `json:"skippedThinClusterSample,omitempty"` } // AccountedFor is the number of works the summary can name an outcome for. @@ -263,7 +271,8 @@ func (s *AuthorSyncSummary) SkippedTotal() int { return 0 } return s.SkippedLanguage + s.SkippedJunk + s.SkippedMediaType + s.SkippedNotAccepted + - s.SkippedPartBooks + s.SkippedMissingDate + s.SkippedMinPages + s.SkippedMissingISBN + s.SkippedPartBooks + s.SkippedMissingDate + s.SkippedMinPages + s.SkippedMissingISBN + + s.SkippedThinCluster } // AuthorProviderFromForeignID returns the metadata provider implied by a diff --git a/internal/models/metadata_profile.go b/internal/models/metadata_profile.go index e06219317..63f0bd288 100644 --- a/internal/models/metadata_profile.go +++ b/internal/models/metadata_profile.go @@ -11,10 +11,15 @@ const ( ) type MetadataProfile struct { - ID int64 `json:"id"` - Name string `json:"name"` - MinPopularity int `json:"minPopularity"` - MinPages int `json:"minPages"` + ID int64 `json:"id"` + Name string `json:"name"` + MinPopularity int `json:"minPopularity"` + MinPages int `json:"minPages"` + // MinEditionCount is an opt-in floor on the edition count of a work's + // title cluster during author sync (see #2235). Zero disables the filter. + // Works with no known edition count pass, matching MinPages' "unknown is + // not zero" semantics. + MinEditionCount int `json:"minEditionCount"` SkipMissingDate bool `json:"skipMissingDate"` SkipMissingISBN bool `json:"skipMissingIsbn"` SkipPartBooks bool `json:"skipPartBooks"` diff --git a/web/src/api/authors.ts b/web/src/api/authors.ts index 3dd989cbc..794c7e0a4 100644 --- a/web/src/api/authors.ts +++ b/web/src/api/authors.ts @@ -88,9 +88,10 @@ export interface AuthorSyncSummary { // The first few dropped titles, capped server-side. skippedLanguageSample?: AuthorSyncSkippedBook[] - // The five fields below back the metadata-profile filters wired into - // author sync by PRs #1968, #2005, #2006, #2007, and #2008. Landed here - // first so those PRs rebase onto a type and notice that already exist. + // The six fields below back the metadata-profile filters wired into + // author sync by PRs #1968, #2005, #2006, #2007, #2008, and the + // min_edition_count filter (#2235). Landed here first so those PRs + // rebase onto a type and notice that already exist. skippedPartBooks?: number skippedPartBooksSample?: AuthorSyncSkippedBook[] skippedMissingDate?: number @@ -101,6 +102,11 @@ export interface AuthorSyncSummary { skippedMinPagesSample?: AuthorSyncSkippedBook[] skippedMissingIsbn?: number skippedMissingIsbnSample?: AuthorSyncSkippedBook[] + // Works dropped because their title cluster reported fewer editions than + // the profile's minEditionCount floor. A work with no known edition count + // passes (unknown, not zero), matching skippedMinPages' semantics. + skippedThinCluster?: number + skippedThinClusterSample?: AuthorSyncSkippedBook[] } export interface AuthorSyncSkippedBook { diff --git a/web/src/api/profiles.ts b/web/src/api/profiles.ts index 2e0699c87..8c489a502 100644 --- a/web/src/api/profiles.ts +++ b/web/src/api/profiles.ts @@ -16,6 +16,7 @@ export interface MetadataProfile { name: string minPopularity: number minPages: number + minEditionCount: number skipMissingDate: boolean skipMissingIsbn: boolean skipPartBooks: boolean diff --git a/web/src/components/AuthorSyncNotice.tsx b/web/src/components/AuthorSyncNotice.tsx index c23c6e399..8796fdc5c 100644 --- a/web/src/components/AuthorSyncNotice.tsx +++ b/web/src/components/AuthorSyncNotice.tsx @@ -56,6 +56,7 @@ export default function AuthorSyncNotice({ sync }: { sync?: AuthorSyncSummary }) const missingDate = sync.skippedMissingDate ?? 0 const minPages = sync.skippedMinPages ?? 0 const missingIsbn = sync.skippedMissingIsbn ?? 0 + const thinCluster = sync.skippedThinCluster ?? 0 const failed = sync.failed ?? 0 // skippedExcluded is deliberately absent from this sum and from the list // below. The user excluded that book on purpose, so it is not something the @@ -69,7 +70,8 @@ export default function AuthorSyncNotice({ sync }: { sync?: AuthorSyncSummary }) partBooks + missingDate + minPages + - missingIsbn + missingIsbn + + thinCluster // A sync that dropped nothing and lost nothing has nothing to explain; the // book list already says what happened. A failed write is worth saying even // when no filter fired, because it is the one outcome here that is a fault @@ -98,6 +100,7 @@ export default function AuthorSyncNotice({ sync }: { sync?: AuthorSyncSummary }) (sync.skippedMissingDateSample ?? []).map(b => b.title), (sync.skippedMinPagesSample ?? []).map(b => b.title), (sync.skippedMissingIsbnSample ?? []).map(b => b.title), + (sync.skippedThinClusterSample ?? []).map(b => b.title), ], authorSyncSampleLimit, ) @@ -209,6 +212,15 @@ export default function AuthorSyncNotice({ sync }: { sync?: AuthorSyncSummary }) })} )} + {thinCluster > 0 && ( +
  • + {t('authorDetail.lastSync.thinCluster', { + count: thinCluster, + defaultValue: + '{{count}} skipped because their title cluster reported fewer editions than the minimum', + })} +
  • + )} {sample.length > 0 && (
    diff --git a/web/src/components/EditAuthorModal.test.tsx b/web/src/components/EditAuthorModal.test.tsx index b94ddfa3b..c2b4b3d07 100644 --- a/web/src/components/EditAuthorModal.test.tsx +++ b/web/src/components/EditAuthorModal.test.tsx @@ -43,8 +43,8 @@ const qualityProfiles: QualityProfile[] = [ ] const metadataProfiles: MetadataProfile[] = [ - { id: 10, name: 'Standard', minPopularity: 0, minPages: 0, skipMissingDate: false, skipMissingIsbn: false, skipPartBooks: false, allowedLanguages: 'eng', unknownLanguageBehavior: 'pass' }, - { id: 11, name: 'English Only', minPopularity: 0, minPages: 0, skipMissingDate: false, skipMissingIsbn: false, skipPartBooks: false, allowedLanguages: 'eng', unknownLanguageBehavior: 'fail' }, + { id: 10, name: 'Standard', minPopularity: 0, minPages: 0, minEditionCount: 0, skipMissingDate: false, skipMissingIsbn: false, skipPartBooks: false, allowedLanguages: 'eng', unknownLanguageBehavior: 'pass' }, + { id: 11, name: 'English Only', minPopularity: 0, minPages: 0, minEditionCount: 0, skipMissingDate: false, skipMissingIsbn: false, skipPartBooks: false, allowedLanguages: 'eng', unknownLanguageBehavior: 'fail' }, ] const rootFolders: RootFolder[] = [ diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index f84997573..00f17832d 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -451,6 +451,7 @@ "missingDate": "{{count}} skipped for having no release date", "minPages": "{{count}} skipped for falling below the minimum page count", "missingIsbn": "{{count}} skipped for having no ISBN on any edition", + "thinCluster": "{{count}} skipped because their title reported fewer editions than the minimum", "examples": "For example: {{titles}}", "sampleUnknownLanguage": "{{title}} (no language)", "anyLanguage": "any", @@ -1002,6 +1003,7 @@ "empty": "No metadata profiles configured.", "description": "Books in languages outside the profile's allowed list are filtered out when an author is added or refreshed. Leave the list empty to accept any language. A metadata profile decides which books enter an author's catalogue. It does not filter releases: to stop a search grabbing another language, set Preferred language under General.", "minPages": "Min pages:", + "minEditionCount": "Min editions:", "languages": "Languages:", "skipMissingDate": "Skip missing date", "skipMissingIsbn": "Skip missing ISBN", @@ -1016,6 +1018,8 @@ "formUnknownLanguageFail": "Fail — skip the book", "formUnknownLanguageHint": "Some metadata sources (notably OpenLibrary) omit the language on some works. Pass imports them silently; Fail rejects them with a log entry.", "formMinPages": "Min pages", + "formMinEditionCount": "Min editions", + "formMinEditionCountHint": "Only cataloged when the work’s title reports at least this many editions; works with no known edition count pass, so a Hardcover-only catalogue is never dropped. 0 disables the filter.", "formSkipMissingDate": "Skip missing release date", "formSkipMissingIsbn": "Skip missing ISBN", "formSkipPartBooks": "Skip part books", diff --git a/web/src/pages/settings/MetadataTab.tsx b/web/src/pages/settings/MetadataTab.tsx index 2b78d0fc7..6cd2a4a01 100644 --- a/web/src/pages/settings/MetadataTab.tsx +++ b/web/src/pages/settings/MetadataTab.tsx @@ -285,6 +285,7 @@ export default function MetadataTab() {

    {p.name}

    {t('settings.metadata.minPages')} {p.minPages === 0 ? 'none' : p.minPages} + {t('settings.metadata.minEditionCount')} {p.minEditionCount === 0 ? 'none' : p.minEditionCount} {t('settings.metadata.languages')} {formatLanguageList(p.allowedLanguages)}
    @@ -331,6 +332,7 @@ function MetadataProfileForm({ profile, onClose, onSaved }: { profile?: Metadata // shape as #2373, which kept quality_profiles.cutoff for the same reason. const minPopularity = profile?.minPopularity ?? 0 const [minPages, setMinPages] = useState(profile?.minPages ?? 0) + const [minEditionCount, setMinEditionCount] = useState(profile?.minEditionCount ?? 0) const [skipMissingDate, setSkipMissingDate] = useState(profile?.skipMissingDate ?? false) const [skipMissingIsbn, setSkipMissingIsbn] = useState(profile?.skipMissingIsbn ?? false) const [skipPartBooks, setSkipPartBooks] = useState(profile?.skipPartBooks ?? false) @@ -355,6 +357,7 @@ function MetadataProfileForm({ profile, onClose, onSaved }: { profile?: Metadata name: name.trim(), minPopularity, minPages, + minEditionCount, skipMissingDate, skipMissingIsbn, skipPartBooks, @@ -422,7 +425,16 @@ function MetadataProfileForm({ profile, onClose, onSaved }: { profile?: Metadata setMinPages(Number(e.target.value))} className={inputCls} />
    +
    + + setMinEditionCount(Number(e.target.value))} className={inputCls} /> +
    + {minEditionCount > 0 && ( +

    + {t('settings.metadata.formMinEditionCountHint')} +

    + )}