fix(series): skip split edition parts of owned books and apply metadata profile filters to series fill (#3048, #2208)

changelog.d/2208-series-fill-profile-filters.md

@@ -0,0 +1,2 @@
+### Fixed
+- Fill gaps and add all on a series now respect the author's metadata profile, so box sets, undated works and anything else the profile filters out are no longer created by a series fill, and a fill no longer puts stored books the profile rejects back on Wanted (#2208). The result line says how many the profile skipped. Adding a single book from a series row still works for anything you pick by hand. Thanks to ianepreston for the report.

changelog.d/3048-split-edition-parts.md

@@ -0,0 +1,2 @@
+### Fixed
+- Split edition parts of a book you already have, like "The Way of Kings, Part 1" and "Part 2" next to The Way of Kings itself, are no longer put back on Wanted by Fill gaps or searched by the scheduled wanted search, so Bindery stops downloading parts of a book on your shelf (#3048). The series card marks them as split parts, leaves them out of the missing count, and offers to unmonitor any a fill from an older version left monitored. Nothing is deleted.

cmd/bindery/main.go

@@ -793,6 +793,7 @@ func main() {
 		WithHardcoverFeatureSettings(settingsRepo, cfg.EnhancedHardcoverAPI).
 		WithFinder(importScanner).
 		WithEditionHydration(editionRepo).
+		WithMetadataProfiles(metadataProfileRepo).
 		WithLifetimeCtx(appCtx)
 	importListHandler := api.NewImportListHandler(importListRepo, settingsRepo, hcSyncer, userRepo)
 	metadataProfileHandler := api.NewMetadataProfileHandler(metadataProfileRepo)

cmd/bindery/requester_routes_test.go

@@ -486,3 +486,8 @@ func TestRequesterGuard_OPDSModeNeverElevatesARequester(t *testing.T) {
 		}
 	}
 }
+
+// UnmonitorSplitEditionParts completes the series route surface (#3048).
+func (s universalStub) UnmonitorSplitEditionParts(w http.ResponseWriter, _ *http.Request) {
+	s.h(w)
+}

cmd/bindery/series_routes.go

@@ -28,6 +28,7 @@ type seriesRouteHandler interface {
 	PutHardcoverLink(http.ResponseWriter, *http.Request)
 	DeleteHardcoverLink(http.ResponseWriter, *http.Request)
 	HardcoverDiff(http.ResponseWriter, *http.Request)
+	UnmonitorSplitEditionParts(http.ResponseWriter, *http.Request)
 }
 
 func registerSeriesRoutes(r chi.Router, seriesHandler seriesRouteHandler) {
@@ -48,6 +49,7 @@ func registerSeriesRoutes(r chi.Router, seriesHandler seriesRouteHandler) {
 		r.Delete("/series/{id}/books/{bookId}", seriesHandler.RemoveBook)
 		r.Put("/series/{id}/books/{bookId}/primary", seriesHandler.SetPrimaryBook)
 		r.Post("/series/{id}/fill", seriesHandler.Fill)
+		r.Post("/series/{id}/split-parts/unmonitor", seriesHandler.UnmonitorSplitEditionParts)
 		r.Put("/series/{id}/genres", seriesHandler.ApplyGenres)
 		r.Delete("/series/{id}/genres", seriesHandler.ClearGenres)
 		r.Post("/series/{id}/hardcover-link/auto", seriesHandler.AutoLinkHardcover)

cmd/bindery/series_routes_test.go

@@ -95,6 +95,10 @@ func (h *stubSeriesRouteHandler) HardcoverDiff(w http.ResponseWriter, _ *http.Re
 	h.record("hardcover-diff", w)
 }
 
+func (h *stubSeriesRouteHandler) UnmonitorSplitEditionParts(w http.ResponseWriter, _ *http.Request) {
+	h.record("unmonitor-split-edition-parts", w)
+}
+
 func TestSeriesMutationRoutesRequireAdmin(t *testing.T) {
 	tests := []struct {
 		name   string
@@ -110,6 +114,7 @@ func TestSeriesMutationRoutesRequireAdmin(t *testing.T) {
 		{name: "remove book", method: http.MethodDelete, path: "/series/1/books/2"},
 		{name: "set primary book", method: http.MethodPut, path: "/series/1/books/2/primary"},
 		{name: "fill series", method: http.MethodPost, path: "/series/1/fill"},
+		{name: "unmonitor split edition parts", method: http.MethodPost, path: "/series/1/split-parts/unmonitor"},
 		{name: "auto link hardcover", method: http.MethodPost, path: "/series/1/hardcover-link/auto"},
 		{name: "put hardcover link", method: http.MethodPut, path: "/series/1/hardcover-link"},
 		{name: "delete hardcover link", method: http.MethodDelete, path: "/series/1/hardcover-link"},

docs/API.md

@@ -302,7 +302,8 @@ GET    /api/v1/book/{id}/calibre                  where the book stands in the C
 ```
 GET    /api/v1/series                             list series with their linked books
 GET    /api/v1/series/{id}                        one series
-POST   /api/v1/series/{id}/fill                   add the series' missing books as wanted (admin)
+POST   /api/v1/series/{id}/fill                   add the series' missing books as wanted (admin); answers {queued, skippedByProfile, skippedSplitParts}
+POST   /api/v1/series/{id}/split-parts/unmonitor  unmonitor the split edition parts of books already in the series, listed as splitEditionPartBookIds on the series (admin); answers {unmonitored}
 PATCH  /api/v1/series/{id}                        monitor / unmonitor (admin)
 POST   /api/v1/series/{id}/merge                  merge other series into this one: {"sourceIds":[..],"title":"optional rename","dryRun":true} previews (admin)
 ```

docs/Hardcover-Series-Wiki.md

@@ -89,6 +89,12 @@ Catalog entries whose titles name a box set rather than a book ("box set", "boxe
 
 The fill action may create new authors and books from Hardcover metadata when the catalog entry is not already in Bindery. Those books are linked back to the series with the catalog position.
 
+**Fill gaps** and **add all** apply the author's metadata profile to every book they would create, the same filters an author catalogue sync applies: skip part books, skip missing release date, minimum editions, allowed languages, minimum pages and skip missing ISBN. A book a filter rejects is not created, and the result line says how many the profile skipped. An author new to the library gets the default profile. The single row **add** is an explicit pick of one book, so it is not filtered, the same way adding one work from an author page is not. A book that is already in the library is linked to the series whatever the profile says.
+
+When fill re-queues the books already in the series, it also leaves out any that are unmonitored and not imported and that the profile's skip part books or skip missing release date filter rejects, so a fill no longer puts those back on Wanted. A book that is already monitored, for example one you added by hand, is queued as before. A book with no known language is not rejected by the language filter during a fill, whatever the profile does with unknown languages, because Hardcover series entries carry no language; it is resolved later. The other filters need a fresh provider lookup; use **More → Reconcile catalogue…** on the author to apply every filter to books already stored.
+
+Some catalogs list a novel's own split edition beside it, such as "The Way of Kings, Part 1" and "Part 2" at 1.1 and 1.2 under "The Way of Kings" at 1. A book in the series counts as such a split part when both hold: its position is a fraction under the whole's position, and its title is the whole's title followed by "Part N". When the whole is imported, or monitored and wanted, fill does not re-queue the parts and the scheduled wanted search does not search them, so a part of a book you already have is never downloaded. The parts are marked **Split part** on the series card and are not counted as missing. If a fill from an older version left them monitored, **Unmonitor N split parts** on the series card unmonitors them in one click. Nothing is deleted, and an imported part is never touched. A real novella at a fractional position ("Edgedancer" at 2.5) has no "Part N" marker and stays a normal book, and parts with no whole at the integer position are the real volumes and are searched as usual.
+
 The format dropdown beside **add all** sets the media type of every book the fill creates. Pick **Ebook** and the created books are ebook only, even when Hardcover lists an audiobook edition of the same work, so only one search is queued per book. Pick **Audiobook** for audio only. Pick **Both** if you want Bindery to look for both formats. Books that are already in the series keep whatever media type they were added with, so change those on the book itself.
 
 ## Merging Series
@@ -120,7 +126,7 @@ Undoing an import (ABS, Calibre) made before a merge does not take the merged bo
 - Enhanced controls are missing: save a Hardcover API token, then enable **Enhanced Hardcover series** in `Settings -> API Keys`. If your operator disabled the deployment-wide feature flag, they must remove `BINDERY_ENHANCED_HARDCOVER_API=false` and restart Bindery.
 - Hardcover test fails: verify the token at [Hardcover API settings](https://hardcover.app/account/api) and make sure Bindery can reach `hardcover.app`. Both `hc_pat_` personal access tokens and the older JWTs are accepted. The test tells you which side failed: `token rejected (HTTP 401: ...)` is a token to reissue, while `HTTP 500 (upstream returned a non-JSON response ...)` is a Hardcover outage to wait out. See [Troubleshooting](./Troubleshooting-Wiki.md#hardcover-fails-with-http-500-or-the-token-test-shows-a-hardcover-error-page).
 - Search finds the wrong series: choose a different result manually, or remove the link and search again with a more specific local series name.
-- Fill gaps adds nothing: check whether the missing books already exist locally, are excluded, or whether the linked Hardcover catalog has no missing entries relative to the local series.
+- Fill gaps adds nothing: check whether the missing books already exist locally, are excluded, are filtered out by the author's metadata profile (the result line counts those), or whether the linked Hardcover catalog has no missing entries relative to the local series.
 - Searches are not queued after fill: check the **Auto-grab** toggle in `Settings -> General` first, since it suppresses fill's searches deliberately, then verify your indexers and download-client search flow are working for normal wanted books.
 
 ## See Also

docs/User-Guide-Wiki.md

@@ -869,10 +869,12 @@ When metadata is wrong, you have three levels of fix:
 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, 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.
+   count, skip part books) filters what a catalogue sync lets in, and what
+   **Fill gaps** or **add all** on a series creates
+   ([#2208](https://github.com/vavallee/bindery/issues/2208)). Adding a single
+   row from a series is an explicit pick and is not filtered. Filters screen
+   books as they arrive; **Reconcile catalogue** on an author applies them to
+   books already stored.
 
 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

internal/api/profile_filters.go

@@ -0,0 +1,145 @@
+package api
+
+import (
+	"context"
+	"log/slog"
+	"strings"
+
+	"github.com/vavallee/bindery/internal/db"
+	"github.com/vavallee/bindery/internal/metadata"
+	"github.com/vavallee/bindery/internal/models"
+)
+
+// Reasons a metadata profile filter rejects a book. They travel in API
+// responses, so they are stable identifiers rather than prose.
+const (
+	profileFilterPartBook        = "partBook"
+	profileFilterMissingDate     = "missingDate"
+	profileFilterLanguage        = "language"
+	profileFilterMinEditionCount = "minEditionCount"
+	profileFilterMinPages        = "minPages"
+	profileFilterMissingISBN     = "missingIsbn"
+)
+
+// effectiveMetadataProfile returns the metadata profile that governs an
+// author: its own, or the seeded default. Nil when profiles is nil or the
+// lookup fails, which every caller treats as "filter nothing", the same
+// fallback the author sync's resolve helpers use, so an unresolvable profile
+// never turns into unexpected catalogue loss.
+func effectiveMetadataProfile(ctx context.Context, profiles *db.MetadataProfileRepo, author *models.Author) *models.MetadataProfile {
+	if profiles == nil {
+		return nil
+	}
+	id := models.DefaultMetadataProfileID
+	if author != nil && author.MetadataProfileID != nil {
+		id = *author.MetadataProfileID
+	}
+	p, err := profiles.GetByID(ctx, id)
+	if err != nil {
+		slog.Debug("metadata profile lookup failed; not filtering", "profileID", id, "error", err)
+		return nil
+	}
+	return p
+}
+
+// storedBookProfileFilter reports which metadata profile filter rejects b,
+// judged only from what a stored book row already carries: its title and its
+// release date. "" means it passes. Series fill uses it so a fill does not put
+// a stored row the profile rejects back on Wanted (#2208).
+//
+// These are the two filters the author sync applies without a provider round
+// trip. The others need data a stored row does not hold reliably: language is
+// decided from provider edition evidence that is not stored, and MinPages,
+// SkipMissingISBN and MinEditionCount need a fresh edition lookup. Judging
+// those from the row would skip books the sync correctly kept. Reconcile
+// catalogue (catalogue_reconciliation.go) is the place that applies every
+// filter to stored rows, with the provider lookups that needs.
+func storedBookProfileFilter(p *models.MetadataProfile, b *models.Book) string {
+	if p == nil || b == nil {
+		return ""
+	}
+	if p.SkipPartBooks && isPartBookTitle(b.Title) {
+		return profileFilterPartBook
+	}
+	if p.SkipMissingDate && b.ReleaseDate == nil {
+		return profileFilterMissingDate
+	}
+	return ""
+}
+
+// catalogBookProfileFilter reports which metadata profile filter rejects a
+// book a series fill is about to create, or "" when it passes. It applies the
+// same filters, with the same "unknown passes" semantics, as the author sync's
+// discovery loop in fetchAuthorBooks (#2208):
+//
+//   - SkipPartBooks and SkipMissingDate from the title and release date.
+//   - MinEditionCount against the work's own edition count; zero is unknown
+//     and passes.
+//   - The language allow list, against the provider's language. A language the
+//     list rejects is checked once more against the provider's edition
+//     evidence, which the author sync also consults, so a work whose display
+//     language is foreign but which has an allowed language edition is kept.
+//     A blank language, or an evidence lookup that fails, passes.
+//   - MinPages and SkipMissingISBN from an edition lookup, made only when the
+//     profile turns either on. A failed lookup enforces neither.
+func catalogBookProfileFilter(ctx context.Context, meta *metadata.Aggregator, p *models.MetadataProfile, b *models.Book) string {
+	if p == nil || b == nil {
+		return ""
+	}
+	if reason := storedBookProfileFilter(p, b); reason != "" {
+		return reason
+	}
+	if p.MinEditionCount > 0 && b.EditionCount > 0 && b.EditionCount < p.MinEditionCount {
+		return profileFilterMinEditionCount
+	}
+	// A blank language is unknown and passes here whatever the profile's
+	// unknown language behaviour says. Hardcover series catalogue entries
+	// carry no language at all, so honouring "fail" would reject every book
+	// of every series fill; the language is resolved later, on hydration or
+	// import. Same unknown passes rule as MinPages and SkipMissingISBN.
+	if allowed := models.ParseAllowedLanguages(p.AllowedLanguages); len(allowed) > 0 && strings.TrimSpace(b.Language) != "" {
+		if !models.IsLanguageAllowed(b.Language, allowed, false) {
+			ok, err := languageEvidenceAllows(ctx, meta, b, allowed)
+			if err != nil {
+				// No answer is not a "no": a lookup that failed (or was
+				// held back) does not reject the book.
+				slog.Debug("language evidence lookup failed; not enforcing the language filter for this book",
+					"title", b.Title, "foreignId", b.ForeignID, "error", err)
+			} else if !ok {
+				return profileFilterLanguage
+			}
+		}
+	}
+	if (p.MinPages > 0 || p.SkipMissingISBN) && meta != nil && b.ForeignID != "" {
+		editions, err := meta.GetEditions(ctx, b.ForeignID)
+		if err != nil {
+			slog.Debug("edition lookup failed while checking MinPages/SkipMissingISBN; not enforcing for this book",
+				"title", b.Title, "foreignId", b.ForeignID, "error", err)
+			return ""
+		}
+		if p.SkipMissingISBN && !anyEditionHasISBN(editions) {
+			return profileFilterMissingISBN
+		}
+		if p.MinPages > 0 && !passesMinPagesFilter(editions, p.MinPages) {
+			return profileFilterMinPages
+		}
+	}
+	return ""
+}
+
+// languageEvidenceAllows asks the provider whether any edition of b is in an
+// allowed language. Only a definitive "allowed" answer counts as true; an
+// indeterminate answer, or a primary provider with no evidence to offer,
+// leaves the book's own language verdict standing. A failed lookup is
+// returned as an error so the caller can treat it as unknown.
+func languageEvidenceAllows(ctx context.Context, meta *metadata.Aggregator, b *models.Book, allowed []string) (bool, error) {
+	if meta == nil || b.ForeignID == "" {
+		return false, nil
+	}
+	evidence, err := meta.GetAuthorWorkLanguageEvidence(ctx, []models.Book{*b}, allowed)
+	if err != nil {
+		return false, err
+	}
+	resolved, found := evidence[strings.TrimSpace(b.ForeignID)]
+	return found && resolved.State == metadata.AuthorWorkLanguageAllowed, nil
+}

internal/api/profile_filters_test.go

@@ -0,0 +1,66 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/vavallee/bindery/internal/metadata"
+	"github.com/vavallee/bindery/internal/models"
+)
+
+// evidenceSeriesProvider is a primary provider that answers language evidence
+// queries, the way Hardcover does, with a fixed state or a fixed error.
+type evidenceSeriesProvider struct {
+	*stubSeriesProvider
+	state metadata.AuthorWorkLanguageEvidenceState
+	err   error
+}
+
+func (p *evidenceSeriesProvider) GetAuthorWorkLanguageEvidence(_ context.Context, books []models.Book, _ []string) (map[string]metadata.AuthorWorkLanguageEvidence, error) {
+	if p.err != nil {
+		return nil, p.err
+	}
+	out := make(map[string]metadata.AuthorWorkLanguageEvidence, len(books))
+	for _, b := range books {
+		out[b.ForeignID] = metadata.AuthorWorkLanguageEvidence{State: p.state}
+	}
+	return out, nil
+}
+
+// TestCatalogBookProfileFilterLanguage pins the language rules a series fill
+// applies (#2208 review). Hardcover series catalogue entries carry no
+// language, so a blank language must pass even under "unknown fails", or add
+// all rejects every book whenever OpenLibrary is primary and no evidence can
+// rescue it. A failed evidence lookup (an outage, or a held request) is no
+// answer and must pass too; only a definitive "not allowed" rejects.
+func TestCatalogBookProfileFilterLanguage(t *testing.T) {
+	ctx := context.Background()
+	profile := &models.MetadataProfile{AllowedLanguages: "eng", UnknownLanguageBehavior: models.UnknownLanguageFail}
+	openLibraryLike := metadata.NewAggregator(&stubSeriesProvider{}).WithAudnexClient(nil)
+	evidence := func(state metadata.AuthorWorkLanguageEvidenceState, err error) *metadata.Aggregator {
+		return metadata.NewAggregator(&evidenceSeriesProvider{stubSeriesProvider: &stubSeriesProvider{}, state: state, err: err}).WithAudnexClient(nil)
+	}
+	cases := []struct {
+		name     string
+		meta     *metadata.Aggregator
+		language string
+		want     string
+	}{
+		{"blank language, no evidence provider", openLibraryLike, "", ""},
+		{"blank language, evidence lookup fails", evidence(0, errors.New("held")), "", ""},
+		{"allowed language", openLibraryLike, "eng", ""},
+		{"foreign language, no evidence provider", openLibraryLike, "spa", profileFilterLanguage},
+		{"foreign language, evidence lookup fails", evidence(0, errors.New("held")), "spa", ""},
+		{"foreign language, evidence says allowed", evidence(metadata.AuthorWorkLanguageAllowed, nil), "spa", ""},
+		{"foreign language, evidence says not allowed", evidence(metadata.AuthorWorkLanguageNotAllowed, nil), "spa", profileFilterLanguage},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			b := &models.Book{ForeignID: "hc:book", Title: "A Book", Language: tc.language}
+			if got := catalogBookProfileFilter(ctx, tc.meta, profile, b); got != tc.want {
+				t.Fatalf("catalogBookProfileFilter = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}