From c722645f38335b9e76f2624e7134d0a9e590679e Mon Sep 17 00:00:00 2001 From: gch ahcg Date: Thu, 1 Oct 2026 20:09:09 -0400 Subject: [PATCH 1/4] fix(series): count split editions of an owned book as covered, not missing (#2524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some Hardcover catalogues list a novel's own split-into-parts editions beside the whole novel: Stormlight's "The Way of Kings" at position 1, with "Part 1"/"Part 2" at 1.1/1.2. Already owning the whole novel left those parts reading as missing volumes, counted against the series, and sitting behind an add button that would create and queue a download for text already on the shelf. buildHardcoverDiff now checks each unclaimed catalogue row against the series' own Present rows: a row at a fractional position whose integer floor is a Present whole work, and whose title is that work's title plus a "Part N" marker, is reported as Covered instead of Missing. Both signals are required — position alone would also catch a real novella at a neighbouring fractional position (Edgedancer at 2.5 beside Words of Radiance at 2), and title alone says nothing about position. A series represented ONLY by its split parts (Wheel of Time's "Two Volume Edition", no unsplit row to speak of) is unaffected: there is nothing Present to cover the parts, so they stay Missing. Both Fill write paths iterate diff.Missing, so Covered rows stop being created and queued automatically, with no change needed there. internal/seriesmatch gains IntegerFloorPosition and SplitPartOf, plus the HasPartMarker/StripPartMarker pair moved from internal/importer (one regex, one package, now that series.go needs them too). The Series page shows covered rows in their own collapsed section, linked to the book that already covers them, with no add button. Refs #2524 (not Closes: this is the agreed first narrow slice — the broader work-identity check the issue asks for stays open). Signed-off-by: gch ahcg --- changelog.d/2524-split-editions.md | 2 + docs/Hardcover-Series-Wiki.md | 4 +- internal/api/series.go | 66 +++++- internal/api/series_test.go | 337 +++++++++++++++++++++++++++++ internal/importer/volume.go | 20 +- internal/seriesmatch/match.go | 61 ++++++ internal/seriesmatch/match_test.go | 65 ++++++ web/src/api/series.ts | 5 + web/src/pages/SeriesPage.test.tsx | 74 +++++++ web/src/pages/SeriesPage.tsx | 30 +++ 10 files changed, 642 insertions(+), 22 deletions(-) create mode 100644 changelog.d/2524-split-editions.md diff --git a/changelog.d/2524-split-editions.md b/changelog.d/2524-split-editions.md new file mode 100644 index 000000000..0a30cd629 --- /dev/null +++ b/changelog.d/2524-split-editions.md @@ -0,0 +1,2 @@ +### Fixed +- **Split-edition catalogue rows no longer count as missing books you don't have** (#2524): some Hardcover catalogs list a novel's own split-into-parts editions alongside the whole novel — Brandon Sanderson's The Way of Kings at position 1, with its "Part 1" and "Part 2" at 1.1 and 1.2. Already owning the whole novel left those parts reading as missing volumes on the Series page, counted against the series and sitting behind an add button that would create and queue a download for text already on the shelf. They now show up in their own collapsed section instead, linked to the book that already covers them, and Fill gaps no longer creates them. A real novella at a neighbouring fractional position, such as Edgedancer beside Words of Radiance, is unaffected, and a series represented only by its split parts, with no unsplit whole to speak of, is unaffected too. Thanks magrhino for the Stormlight and Wheel of Time catalogue data that shaped the rule. diff --git a/docs/Hardcover-Series-Wiki.md b/docs/Hardcover-Series-Wiki.md index 7b89ced87..017feef07 100644 --- a/docs/Hardcover-Series-Wiki.md +++ b/docs/Hardcover-Series-Wiki.md @@ -13,7 +13,7 @@ When enhanced Hardcover series data is enabled, Bindery can: - search Hardcover for the matching catalog series - link or unlink a local series to a Hardcover series - compare the local series against the Hardcover catalog -- show present, missing, local-only, and uncertain catalog entries +- show present, missing, local-only, uncertain, and covered (split-edition) catalog entries - add one missing Hardcover book, or add all missing books for a linked series - create wanted and monitored book rows for missing catalog entries and queue searches for them @@ -87,6 +87,8 @@ Missing-book fill uses the linked Hardcover catalog as the source of truth for m Catalog entries whose titles name a box set rather than a book ("box set", "boxed set", "collection set", "3 Books Set") are never created by fill, the same way they are pruned from an author catalogue on ingestion. They are still listed in the diff, so clicking **add** on one does nothing and reports nothing queued. +Some catalogs also list a novel's own split-into-parts editions beside the whole novel — Brandon Sanderson's The Way of Kings sits at position 1, with its "Part 1" and "Part 2" editions at 1.1 and 1.2. If the whole novel is already present, its parts are reported as **covered**, not missing: they are not counted, not offered an **add** button, and not created by fill, because the book is already on the shelf. They appear in their own collapsed section on the Series page ("N split editions of books you own"), each linking to the whole work that covers it. The rule needs both a fractional position under a present whole AND a title that is the whole's title plus a "Part N" marker, because a fractional position alone is not enough: a real novella, such as Edgedancer at position 2.5 beside Words of Radiance at 2, carries no "Part N" marker and is correctly left in Missing. A catalog that represents a work ONLY as its split parts, with no unsplit row at the integer position at all — Robert Jordan's Wheel of Time "Two Volume Edition" catalog is like this — never triggers the rule either, because there is nothing present to cover the parts; they are the real volumes there and stay in Missing. + 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. diff --git a/internal/api/series.go b/internal/api/series.go index 9f2720990..52d343559 100644 --- a/internal/api/series.go +++ b/internal/api/series.go @@ -914,14 +914,19 @@ type seriesHardcoverDiffBook struct { } type seriesHardcoverDiffResponse struct { - SeriesID int64 `json:"seriesId"` - Link *models.SeriesHardcoverLink `json:"link"` - Present []seriesHardcoverDiffBook `json:"present"` - Missing []seriesHardcoverDiffBook `json:"missing"` - LocalOnly []seriesHardcoverDiffBook `json:"localOnly"` - Uncertain []seriesHardcoverDiffBook `json:"uncertain"` - PresentCount int `json:"presentCount"` - MissingCount int `json:"missingCount"` + SeriesID int64 `json:"seriesId"` + Link *models.SeriesHardcoverLink `json:"link"` + Present []seriesHardcoverDiffBook `json:"present"` + Missing []seriesHardcoverDiffBook `json:"missing"` + LocalOnly []seriesHardcoverDiffBook `json:"localOnly"` + Uncertain []seriesHardcoverDiffBook `json:"uncertain"` + // Covered is a catalogue row that is not a missing volume but a split + // edition (#2524) of a book already present: the row's own LocalBookID/ + // LocalTitle point at the whole work that covers it, not at a row of its + // own. Never counted in MissingCount, never offered to Fill. + Covered []seriesHardcoverDiffBook `json:"covered"` + PresentCount int `json:"presentCount"` + MissingCount int `json:"missingCount"` } var ( @@ -1432,6 +1437,7 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s Missing: []seriesHardcoverDiffBook{}, LocalOnly: []seriesHardcoverDiffBook{}, Uncertain: []seriesHardcoverDiffBook{}, + Covered: []seriesHardcoverDiffBook{}, } // matchedCatalog doubles as the exclusion set for bestCatalogMatch: a // catalog entry a previous local book already claimed is off the table for @@ -1553,6 +1559,13 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s if title := firstNonEmpty(book.Book.Title, book.Title); metadata.IsUnambiguousBundleTitle(title) { continue } + if whole, ok := splitEditionWhole(book, diff.Present); ok { + item := catalogDiffBook(book, catalog.AuthorName) + item.LocalBookID = whole.LocalBookID + item.LocalTitle = whole.LocalTitle + diff.Covered = append(diff.Covered, item) + continue + } item := catalogDiffBook(book, catalog.AuthorName) enrichMissingDiffBook(ctx, books, userID, book, &item) diff.Missing = append(diff.Missing, item) @@ -1562,6 +1575,43 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s return diff } +// splitEditionWhole reports whether book is a split-edition part of some +// already-present whole work, and if so returns that whole work's diff row. +// +// #2524: some Hardcover catalogues list a novel AND its split parts at +// fractional positions under the novel's own position — Stormlight's "The +// Way of Kings" at 1 alongside "…, Part 1" and "…, Part 2" at 1.1 and 1.2. +// Owning the whole novel should not leave its own parts reading as missing +// volumes, counting against the series, and sitting behind an Add button +// that would create and queue a download for text already on the shelf. +// +// Position alone is not enough to call that: a novella can sit at a +// fractional position too (Edgedancer at 2.5, beside Words of Radiance at +// 2), and a novella is a real, separate book. So both signals are required: +// book's position must be the fractional child of a Present row's integer +// position (seriesmatch.IntegerFloorPosition), AND book's title must be that +// Present row's title plus a "Part N" marker (seriesmatch.SplitPartOf). A +// series represented ONLY by its split parts — Wheel of Time's "Two Volume +// Edition", where there is no unsplit whole at the integer position — never +// matches this, because there is no Present row to match against; its parts +// are the real volumes and stay Missing. +func splitEditionWhole(book metadata.SeriesCatalogBook, present []seriesHardcoverDiffBook) (seriesHardcoverDiffBook, bool) { + floor, ok := seriesmatch.IntegerFloorPosition(book.Position) + if !ok { + return seriesHardcoverDiffBook{}, false + } + partTitle := firstNonEmpty(book.Book.Title, book.Title) + for _, whole := range present { + if whole.Position == "" || !seriesmatch.SamePosition(whole.Position, floor) { + continue + } + if seriesmatch.SplitPartOf(partTitle, whole.Title) { + return whole, true + } + } + return seriesHardcoverDiffBook{}, false +} + // logDiffDecision records which catalogue entry a local series book bound // to, and how. The diff matcher had no logging at all, so a wrong binding // could only be diagnosed by rebuilding the library in a scratch container diff --git a/internal/api/series_test.go b/internal/api/series_test.go index a20fb97f2..a8195d93c 100644 --- a/internal/api/series_test.go +++ b/internal/api/series_test.go @@ -1567,6 +1567,83 @@ func TestSeriesFillCreatesMissingHardcoverBook(t *testing.T) { } } +// TestSeriesFillSkipsOwnedSplitEditions is the write-path form of #2524: +// both createMissingHardcoverBooks (bulk Fill) and createMissingHardcoverBook +// (per-book Fill) iterate diff.Missing, so once buildHardcoverDiff moves a +// split-edition row to Covered instead, Fill stops creating — and so stops +// queuing a download for — a book already on the shelf. Every genuinely +// missing book in the same series must still be created. +func TestSeriesFillSkipsOwnedSplitEditions(t *testing.T) { + catalog := splitEditionCatalog() + searcher := newMockBookSearcher() + h, seriesRepo, authorRepo, bookRepo := seriesFixtureWithProvider(t, &stubSeriesProvider{ + catalogs: map[string]*metadata.SeriesCatalog{catalog.ForeignID: catalog}, + }, searcher) + ctx := context.Background() + + author := &models.Author{ForeignID: "hc:brandon-sanderson", Name: "Brandon Sanderson", SortName: "Sanderson, Brandon"} + if err := authorRepo.Create(ctx, author); err != nil { + t.Fatal(err) + } + owned := &models.Book{ + ForeignID: "hc:the-way-of-kings", + AuthorID: author.ID, + Title: "The Way of Kings", + SortTitle: "The Way of Kings", + Status: models.BookStatusImported, + Genres: []string{}, + } + if err := bookRepo.Create(ctx, owned); err != nil { + t.Fatal(err) + } + + series := &models.Series{ForeignID: "ol-series:stormlight", Title: "The Stormlight Archive"} + if err := seriesRepo.Create(ctx, series); err != nil { + t.Fatal(err) + } + if err := seriesRepo.LinkBook(ctx, series.ID, owned.ID, "1", true); err != nil { + t.Fatal(err) + } + link := &models.SeriesHardcoverLink{ + SeriesID: series.ID, + HardcoverSeriesID: catalog.ForeignID, + HardcoverProviderID: catalog.ProviderID, + HardcoverTitle: catalog.Title, + HardcoverAuthorName: catalog.AuthorName, + HardcoverBookCount: catalog.BookCount, + Confidence: 1, + LinkedBy: "manual", + } + if err := seriesRepo.UpsertHardcoverLink(ctx, link); err != nil { + t.Fatal(err) + } + + rec := httptest.NewRecorder() + h.Fill(rec, withURLParam(httptest.NewRequest(http.MethodPost, "/api/v1/series/1/fill", nil), "id", strconv.FormatInt(series.ID, 10))) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + + for _, foreignID := range []string{"hc:the-way-of-kings-part-1", "hc:the-way-of-kings-part-2"} { + created, err := bookRepo.GetByForeignID(ctx, foreignID) + if err != nil { + t.Fatal(err) + } + if created != nil { + t.Fatalf("split-edition part %q must not be created when the whole work is owned", foreignID) + } + } + for _, foreignID := range []string{"hc:words-of-radiance", "hc:edgedancer"} { + created, err := bookRepo.GetByForeignID(ctx, foreignID) + if err != nil { + t.Fatal(err) + } + if created == nil { + t.Fatalf("genuinely missing book %q should still be created by Fill", foreignID) + } + } +} + // TestSeriesFillHonoursRequestedMediaType covers #1124: the per-book "add" // selector must create the missing Hardcover book with the chosen media type // rather than always defaulting to ebook. @@ -2958,6 +3035,266 @@ func stormlightCatalog() *metadata.SeriesCatalog { } } +// splitEditionCatalog builds on stormlightCatalog with the real-data shape +// from the #2524 report: "The Way of Kings" at position 1 is split into its +// own "Part 1"/"Part 2" at 1.1/1.2, "Words of Radiance" sits at position 2, +// and "Edgedancer" — a real novella, not a split edition of anything — sits +// at the neighbouring fractional position 2.5. +func splitEditionCatalog() *metadata.SeriesCatalog { + catalog := stormlightCatalog() + author := catalog.Books[0].Book.Author + catalog.Books = append(catalog.Books, + metadata.SeriesCatalogBook{ + ForeignID: "hc:the-way-of-kings-part-1", + ProviderID: "101-1", + Title: "The Way of Kings, Part 1", + Position: "1.1", + Book: models.Book{ + ForeignID: "hc:the-way-of-kings-part-1", + Title: "The Way of Kings, Part 1", + Author: author, + }, + }, + metadata.SeriesCatalogBook{ + ForeignID: "hc:the-way-of-kings-part-2", + ProviderID: "101-2", + Title: "The Way of Kings, Part 2", + Position: "1.2", + Book: models.Book{ + ForeignID: "hc:the-way-of-kings-part-2", + Title: "The Way of Kings, Part 2", + Author: author, + }, + }, + metadata.SeriesCatalogBook{ + ForeignID: "hc:words-of-radiance", + ProviderID: "102", + Title: "Words of Radiance", + Position: "2", + Book: models.Book{ + ForeignID: "hc:words-of-radiance", + Title: "Words of Radiance", + Author: author, + }, + }, + metadata.SeriesCatalogBook{ + ForeignID: "hc:edgedancer", + ProviderID: "102-5", + Title: "Edgedancer", + Position: "2.5", + Book: models.Book{ + ForeignID: "hc:edgedancer", + Title: "Edgedancer", + Author: author, + }, + }, + ) + catalog.BookCount = len(catalog.Books) + return catalog +} + +// TestSplitEditionWhole unit-tests the #2524 suppression rule directly +// against crafted Present rows, independent of how a book came to be +// Present. That sidesteps engineering a fuzzy title score into the 70-89 +// Uncertain band: a whole work that is Uncertain or Missing simply never +// appears in the `present` slice this function is handed, which is exactly +// the real-world shape (buildHardcoverDiff only passes diff.Present). +func TestSplitEditionWhole(t *testing.T) { + wayOfKingsID := int64(1) + present := []seriesHardcoverDiffBook{{ + ForeignBookID: "hc:the-way-of-kings", + Title: "The Way of Kings", + Position: "1", + LocalBookID: &wayOfKingsID, + LocalTitle: "The Way of Kings", + }} + + cases := []struct { + name string + book metadata.SeriesCatalogBook + present []seriesHardcoverDiffBook + wantOK bool + }{ + { + name: "first part of an owned whole", + book: metadata.SeriesCatalogBook{ + Position: "1.1", + Title: "The Way of Kings, Part 1", + Book: models.Book{Title: "The Way of Kings, Part 1"}, + }, + present: present, + wantOK: true, + }, + { + name: "second part of an owned whole", + book: metadata.SeriesCatalogBook{ + Position: "1.2", + Title: "The Way of Kings, Part 2", + Book: models.Book{Title: "The Way of Kings, Part 2"}, + }, + present: present, + wantOK: true, + }, + { + name: "novella at a fractional position is not a split edition", + book: metadata.SeriesCatalogBook{ + Position: "1.5", + Title: "Dawnshard", + Book: models.Book{Title: "Dawnshard"}, + }, + present: present, + wantOK: false, + }, + { + name: "whole number position never matches, even with a part title", + book: metadata.SeriesCatalogBook{ + Position: "2", + Title: "The Way of Kings, Part 1", + Book: models.Book{Title: "The Way of Kings, Part 1"}, + }, + present: present, + wantOK: false, + }, + { + name: "fractional position but no Present row at the floor (Two Volume Edition shape)", + book: metadata.SeriesCatalogBook{ + Position: "1.1", + Title: "The Eye of the World, Part 1 of 2", + Book: models.Book{Title: "The Eye of the World, Part 1 of 2"}, + }, + present: nil, + wantOK: false, + }, + { + name: "whole is bound only as Uncertain or Missing, not Present", + book: metadata.SeriesCatalogBook{ + Position: "1.1", + Title: "The Way of Kings, Part 1", + Book: models.Book{Title: "The Way of Kings, Part 1"}, + }, + present: nil, // the whole never made it into the Present slice + wantOK: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := splitEditionWhole(tc.book, tc.present) + if ok != tc.wantOK { + t.Fatalf("splitEditionWhole() ok = %v, want %v (got %+v)", ok, tc.wantOK, got) + } + if ok && (got.LocalBookID == nil || *got.LocalBookID != wayOfKingsID) { + t.Fatalf("splitEditionWhole() local = %v, want %d", got.LocalBookID, wayOfKingsID) + } + }) + } +} + +// TestBuildHardcoverDiffCoversSplitEditionsOfOwnedWork is the #2524 +// acceptance case from the issue thread: Stormlight's "The Way of Kings" is +// filed at position 1 alongside its own split parts at 1.1/1.2, and a real +// novella sits at the neighbouring fractional position 2.5 under an unowned +// "Words of Radiance". Owning the whole novel must not leave its own parts +// reading as missing volumes; the novella, owning nothing to its name, must +// not be swept up just because it shares the fractional-position shape. +func TestBuildHardcoverDiffCoversSplitEditionsOfOwnedWork(t *testing.T) { + catalog := splitEditionCatalog() + series := &models.Series{ + ID: 1, + Title: "The Stormlight Archive", + Books: []models.SeriesBook{ + localSeriesBook(1, "The Way of Kings"), + }, + } + link := &models.SeriesHardcoverLink{SeriesID: series.ID, HardcoverSeriesID: catalog.ForeignID} + + diff := buildHardcoverDiff(context.Background(), nil, 0, series, link, catalog) + + covered := map[string]seriesHardcoverDiffBook{} + for _, row := range diff.Covered { + covered[row.ForeignBookID] = row + } + if len(diff.Covered) != 2 { + t.Fatalf("Covered = %v, want exactly the two Way of Kings parts", diffForeignIDs(diff.Covered)) + } + for _, id := range []string{"hc:the-way-of-kings-part-1", "hc:the-way-of-kings-part-2"} { + row, ok := covered[id] + if !ok { + t.Fatalf("expected %q in Covered, got %v", id, diffForeignIDs(diff.Covered)) + } + if row.LocalBookID == nil || *row.LocalBookID != 1 { + t.Fatalf("%q LocalBookID = %v, want 1 (The Way of Kings)", id, row.LocalBookID) + } + } + + missing := map[string]bool{} + for _, row := range diff.Missing { + missing[row.ForeignBookID] = true + } + for _, id := range []string{"hc:the-way-of-kings-part-1", "hc:the-way-of-kings-part-2"} { + if missing[id] { + t.Fatalf("%q was covered but still counted in Missing: %v", id, diffForeignIDs(diff.Missing)) + } + } + if !missing["hc:words-of-radiance"] || !missing["hc:edgedancer"] { + t.Fatalf("Words of Radiance and the Edgedancer novella are genuinely missing, want both in Missing: %v", diffForeignIDs(diff.Missing)) + } + if diff.MissingCount != len(diff.Missing) { + t.Fatalf("missingCount %d disagrees with list length %d", diff.MissingCount, len(diff.Missing)) + } + if diff.MissingCount != 2 { + t.Fatalf("MissingCount = %d, want 2 (Words of Radiance, Edgedancer)", diff.MissingCount) + } +} + +// TestBuildHardcoverDiffSplitEditionsNeedAnOwnedWhole is the Wheel of Time +// "Two Volume Edition" trap from the #2524 thread: a catalogue can represent +// a work ONLY as its split parts, with no unsplit row at the integer +// position at all. There is nothing Present to cover the parts, so they +// must stay Missing — they are the real volumes here, not duplicates of one. +func TestBuildHardcoverDiffSplitEditionsNeedAnOwnedWhole(t *testing.T) { + author := &models.Author{ForeignID: "hc:robert-jordan", Name: "Robert Jordan"} + catalog := &metadata.SeriesCatalog{ + ForeignID: "hc-series:two-volume", + ProviderID: "7000", + Title: "The Wheel of Time (Two Volume Edition)", + AuthorName: "Robert Jordan", + Books: []metadata.SeriesCatalogBook{ + { + ForeignID: "hc:the-eye-of-the-world-part-1-of-2", + Title: "The Eye of the World, Part 1 of 2", + Position: "1.1", + Book: models.Book{ + ForeignID: "hc:the-eye-of-the-world-part-1-of-2", + Title: "The Eye of the World, Part 1 of 2", + Author: author, + }, + }, + { + ForeignID: "hc:the-eye-of-the-world-part-2-of-2", + Title: "The Eye of the World, Part 2 of 2", + Position: "1.2", + Book: models.Book{ + ForeignID: "hc:the-eye-of-the-world-part-2-of-2", + Title: "The Eye of the World, Part 2 of 2", + Author: author, + }, + }, + }, + } + catalog.BookCount = len(catalog.Books) + series := &models.Series{ID: 2, Title: "The Wheel of Time (Two Volume Edition)"} + link := &models.SeriesHardcoverLink{SeriesID: series.ID, HardcoverSeriesID: catalog.ForeignID} + + diff := buildHardcoverDiff(context.Background(), nil, 0, series, link, catalog) + + if len(diff.Covered) != 0 { + t.Fatalf("Covered = %v, want empty: there is no unsplit whole for these parts to cover", diffForeignIDs(diff.Covered)) + } + if len(diff.Missing) != 2 || diff.MissingCount != 2 { + t.Fatalf("Missing = %v (count %d), want both parts (they are the real volumes here)", diffForeignIDs(diff.Missing), diff.MissingCount) + } +} + // TestSeriesHandler_LifetimeCtxFallsBackToBackground is the #846 follow-up // guard for fanOutSeriesSearches. Same contract as BookHandler.bgCtx(). func TestSeriesHandler_LifetimeCtxFallsBackToBackground(t *testing.T) { diff --git a/internal/importer/volume.go b/internal/importer/volume.go index 9a9437122..e25751833 100644 --- a/internal/importer/volume.go +++ b/internal/importer/volume.go @@ -2,15 +2,10 @@ package importer import ( "regexp" - "strings" "github.com/vavallee/bindery/internal/seriesmatch" ) -// partMarkerRe matches a "Part N" marker, with an optional "of M": "Part 1", -// "Pt. 2", "Part 1 of 3". -var partMarkerRe = regexp.MustCompile(`(?i)\b(?:part|pt)\.?\s*\d+(?:\.\d+)?(?:\s+of\s+\d+)?\b`) - // volumeTitles returns a and b ready for a volume-number comparison. // // seriesmatch.VolumeNumber counts "Part N" as a volume marker, which is right @@ -22,22 +17,21 @@ var partMarkerRe = regexp.MustCompile(`(?i)\b(?:part|pt)\.?\s*\d+(?:\.\d+)?(?:\s // (The Stormlight Archive, Book 4)", the "Part 1" read as volume 1 and vetoed // the book's own files (#2810 review). So a Part marker on one side only is // dropped before the comparison, and kept when both sides carry one. +// +// partMarkerRe/stripPartMarker moved to seriesmatch.HasPartMarker / +// seriesmatch.StripPartMarker (#2524), which also needs them to recognize a +// split-edition catalogue row — one regex, one package, instead of two. func volumeTitles(a, b string) (string, string) { - ap, bp := partMarkerRe.MatchString(a), partMarkerRe.MatchString(b) + ap, bp := seriesmatch.HasPartMarker(a), seriesmatch.HasPartMarker(b) switch { case ap && !bp: - a = stripPartMarker(a) + a = seriesmatch.StripPartMarker(a) case bp && !ap: - b = stripPartMarker(b) + b = seriesmatch.StripPartMarker(b) } return a, b } -func stripPartMarker(s string) string { - s = partMarkerRe.ReplaceAllString(s, " ") - return strings.TrimRight(strings.TrimSpace(s), " ,.:;-_") -} - // differentVolumes is volumesDisagree after volumeTitles: true only when both // titles carry a volume number and the numbers disagree. func differentVolumes(a, b string) bool { diff --git a/internal/seriesmatch/match.go b/internal/seriesmatch/match.go index e462a48c4..25883961e 100644 --- a/internal/seriesmatch/match.go +++ b/internal/seriesmatch/match.go @@ -27,6 +27,24 @@ func SamePosition(a, b string) bool { return aerr == nil && berr == nil && math.Abs(af-bf) < 0.001 } +// IntegerFloorPosition reports the integer floor of a fractional series +// position, and whether pos actually IS fractional. "1.1" -> ("1", true); a +// whole number, "", or anything that does not parse as a number all report +// (_, false) — this exists specifically to find the WHOLE work a split-part +// position like 1.1 or 1.2 sits under (#2524), so a position that is already +// whole has no "floor" worth reporting. +func IntegerFloorPosition(pos string) (string, bool) { + f, err := strconv.ParseFloat(strings.TrimSpace(pos), 64) + if err != nil { + return "", false + } + floor := math.Floor(f) + if math.Abs(f-floor) < 1e-9 { + return "", false + } + return strconv.FormatFloat(floor, 'f', -1, 64), true +} + // NormalizeSeriesName reduces a series name for comparison: the canonical // dedup key, minus a redundant trailing collective noun ("… Series", "… // Trilogy"). @@ -275,6 +293,49 @@ func CleanTitle(title string) string { return strings.Join(out, " ") } +// partMarkerRe matches a "Part N" marker, with an optional "of M": "Part 1", +// "Pt. 2", "Part 1 of 3". +var partMarkerRe = regexp.MustCompile(`(?i)\b(?:part|pt)\.?\s*\d+(?:\.\d+)?(?:\s+of\s+\d+)?\b`) + +// HasPartMarker reports whether s carries an explicit "Part N" / "Pt. N" +// marker. +func HasPartMarker(s string) bool { + return partMarkerRe.MatchString(s) +} + +// StripPartMarker removes a "Part N" marker from s and cleans up the +// whitespace and punctuation left behind: "The Way of Kings, Part 1" -> +// "The Way of Kings". +func StripPartMarker(s string) string { + s = partMarkerRe.ReplaceAllString(s, " ") + return strings.TrimRight(strings.TrimSpace(s), " ,.:;-_") +} + +// SplitPartOf reports whether partTitle names one split-edition part of +// wholeTitle: the text before a "Part N" marker, with trailing punctuation +// trimmed, is wholeTitle under CleanTitle. Any subtitle AFTER the marker is +// ignored, because a catalogue sometimes hangs one off the part itself — +// "The Great Hunt, Part 2 of 2: New Threads in the Pattern" is still part 2 +// of "The Great Hunt". +// +// A bare marker with nothing before it ("Part 1" alone) never matches: an +// empty prefix can only equal an equally blank wholeTitle. A title that +// carries no marker at all — a novella like "Edgedancer" — is never a split +// part of anything, however short its name or close its series position; +// that is what keeps a fractional-position novella out of the #2524 rule +// this exists for (see IntegerFloorPosition). +func SplitPartOf(partTitle, wholeTitle string) bool { + loc := partMarkerRe.FindStringIndex(partTitle) + if loc == nil { + return false + } + prefix := strings.TrimRight(strings.TrimSpace(partTitle[:loc[0]]), " ,:;-") + if prefix == "" { + return false + } + return CleanTitle(prefix) == CleanTitle(wholeTitle) +} + // volumeNumberRe matches an EXPLICIT volume marker followed by a number: // "Vol. 3", "Volume 3", "Book 3", "Part 3", "#3". A bare trailing number is // deliberately not matched — "Fahrenheit 451" and "Catch 22" are titles, not diff --git a/internal/seriesmatch/match_test.go b/internal/seriesmatch/match_test.go index 053c5ee2c..0dea2e6e5 100644 --- a/internal/seriesmatch/match_test.go +++ b/internal/seriesmatch/match_test.go @@ -285,3 +285,68 @@ func TestDifferentVolumesCatchesBareTrailingNumbers(t *testing.T) { }) } } + +// TestIntegerFloorPosition pins the #2524 fractional-position test: only a +// genuinely fractional position reports a floor, and a whole number, blank, +// or non-numeric position reports none. +func TestIntegerFloorPosition(t *testing.T) { + cases := []struct { + name string + pos string + wantFloor string + wantOK bool + }{ + {name: "simple fraction", pos: "1.1", wantFloor: "1", wantOK: true}, + {name: "second fraction", pos: "1.2", wantFloor: "1", wantOK: true}, + {name: "novella position", pos: "2.5", wantFloor: "2", wantOK: true}, + {name: "whole number", pos: "1", wantOK: false}, + {name: "whole number written as float", pos: "1.0", wantOK: false}, + {name: "blank", pos: "", wantOK: false}, + {name: "non numeric", pos: "prelude", wantOK: false}, + {name: "trims whitespace", pos: " 1.1 ", wantFloor: "1", wantOK: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + floor, ok := IntegerFloorPosition(tc.pos) + if ok != tc.wantOK { + t.Fatalf("IntegerFloorPosition(%q) ok = %v, want %v", tc.pos, ok, tc.wantOK) + } + if ok && floor != tc.wantFloor { + t.Fatalf("IntegerFloorPosition(%q) floor = %q, want %q", tc.pos, floor, tc.wantFloor) + } + }) + } +} + +// TestSplitPartOf pins the #2524 title half of the split-edition rule: the +// text before a "Part N" marker, not the marker or anything after it, is +// what must match the whole work's title. +func TestSplitPartOf(t *testing.T) { + cases := []struct { + name string + part string + whole string + want bool + }{ + {name: "simple part", part: "The Way of Kings, Part 1", whole: "The Way of Kings", want: true}, + {name: "second part", part: "The Way of Kings, Part 2", whole: "The Way of Kings", want: true}, + {name: "pt spelling", part: "The Way of Kings, Pt. 2", whole: "The Way of Kings", want: true}, + { + name: "part carries its own subtitle", + part: "The Great Hunt, Part 2 of 2: New Threads in the Pattern", + whole: "The Great Hunt", + want: true, + }, + {name: "wrong whole work", part: "Words of Radiance, Part 1", whole: "The Way of Kings", want: false}, + {name: "novella has no marker at all", part: "Edgedancer", whole: "Words of Radiance", want: false}, + {name: "bare marker, no prefix to compare", part: "Part 1", whole: "The Way of Kings", want: false}, + {name: "case and punctuation folded", part: "the way of kings: part 1", whole: "The Way Of Kings", want: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := SplitPartOf(tc.part, tc.whole); got != tc.want { + t.Fatalf("SplitPartOf(%q, %q) = %v, want %v", tc.part, tc.whole, got, tc.want) + } + }) + } +} diff --git a/web/src/api/series.ts b/web/src/api/series.ts index 6431cbcb5..802f2aee5 100644 --- a/web/src/api/series.ts +++ b/web/src/api/series.ts @@ -70,6 +70,11 @@ export interface SeriesHardcoverDiff { missing: SeriesHardcoverDiffBook[] localOnly: SeriesHardcoverDiffBook[] uncertain: SeriesHardcoverDiffBook[] + // covered is a split edition of a book already in Present (#2524) — e.g. a + // "Part 1"/"Part 2" catalogue row for a novel you already own whole. Its + // localBookId/localTitle point at that whole work, not at a row of its own. + // Never counted in missingCount, never offered an add button. + covered: SeriesHardcoverDiffBook[] presentCount: number missingCount: number } diff --git a/web/src/pages/SeriesPage.test.tsx b/web/src/pages/SeriesPage.test.tsx index 744d1b150..666d7cffe 100644 --- a/web/src/pages/SeriesPage.test.tsx +++ b/web/src/pages/SeriesPage.test.tsx @@ -433,6 +433,7 @@ describe('SeriesPage', () => { ], localOnly: [], uncertain: [], + covered: [], presentCount: 2, missingCount: 1, }) @@ -818,6 +819,7 @@ describe('SeriesPage', () => { ], localOnly: [], uncertain: [], + covered: [], presentCount: 0, missingCount: 1, }) @@ -879,6 +881,7 @@ describe('SeriesPage', () => { ], localOnly: [], uncertain: [], + covered: [], presentCount: 0, missingCount: 1, }) @@ -952,6 +955,7 @@ describe('SeriesPage', () => { ], localOnly: [], uncertain: [], + covered: [], presentCount: 0, missingCount: 1, }) @@ -966,6 +970,74 @@ describe('SeriesPage', () => { expect(row).not.toBeInTheDocument() }) + // #2524: a split-edition catalogue row is reported under `covered`, not + // `missing`, so it must render in its own collapsed section, link to the + // whole work that covers it, and never show an add button. + it('shows a covered split-edition row in its own section, linked to the owned whole', async () => { + const hardcoverLink = { + id: 1, + seriesId: 43, + hardcoverSeriesId: 'hc-series:42', + hardcoverProviderId: '42', + hardcoverTitle: 'The Stormlight Archive', + hardcoverAuthorName: 'Brandon Sanderson', + hardcoverBookCount: 3, + confidence: 1, + linkedBy: 'manual', + linkedAt: '2026-01-01T00:00:00Z', + createdAt: '2026-01-01T00:00:00Z', + updatedAt: '2026-01-01T00:00:00Z', + } + const series: Series = { + id: 43, + foreignSeriesId: 'series-43', + title: 'The Stormlight Archive', + description: '', + monitored: true, + books: [], + hardcoverLink, + } + vi.mocked(api.getSeriesHardcoverDiff).mockResolvedValue({ + seriesId: 43, + link: hardcoverLink, + present: [ + { + foreignBookId: 'hc:the-way-of-kings', + providerId: '101', + title: 'The Way of Kings', + position: '1', + authorName: 'Brandon Sanderson', + localBookId: 7, + }, + ], + missing: [], + localOnly: [], + uncertain: [], + covered: [ + { + foreignBookId: 'hc:the-way-of-kings-part-1', + providerId: '101-1', + title: 'The Way of Kings, Part 1', + position: '1.1', + authorName: 'Brandon Sanderson', + localBookId: 7, + localTitle: 'The Way of Kings', + }, + ], + presentCount: 1, + missingCount: 0, + }) + + renderSeriesPage([series]) + + fireEvent.click(await screen.findByRole('heading', { name: 'The Stormlight Archive' })) + fireEvent.click(await screen.findByText('1 split edition of a book you own')) + + const partLink = await screen.findByRole('link', { name: /The Way of Kings, Part 1/ }) + expect(partLink).toHaveAttribute('href', '/book/7') + expect(within(partLink).queryByRole('button', { name: 'add' })).not.toBeInTheDocument() + }) + it('keeps a Hardcover missing row without a library match non-clickable', async () => { const hardcoverLink = { id: 1, @@ -1005,6 +1077,7 @@ describe('SeriesPage', () => { ], localOnly: [], uncertain: [], + covered: [], presentCount: 0, missingCount: 1, }) @@ -1054,6 +1127,7 @@ describe('SeriesPage Hardcover links out (#1708)', () => { missing: [], localOnly: [], uncertain: [], + covered: [], presentCount: 0, missingCount: 0, } diff --git a/web/src/pages/SeriesPage.tsx b/web/src/pages/SeriesPage.tsx index 29ee1c2b5..f7e217d89 100644 --- a/web/src/pages/SeriesPage.tsx +++ b/web/src/pages/SeriesPage.tsx @@ -742,6 +742,36 @@ export default function SeriesPage() { )} )} + {/* #2524: a split edition (e.g. "Part 1"/"Part 2") of a book + already present is not a missing volume, so it is never + counted or offered an add button — just linked to the + whole work that covers it. Collapsed by default since + this is the exception, not the thing a user came to + check. */} + {diff && diff.covered.length > 0 && ( +
+ + {diff.covered.length} split edition{diff.covered.length === 1 ? ' of a book you own' : 's of books you own'} + +
+ {diff.covered.map(book => ( + + + #{book.position || '?'} + +
+

{book.title}

+

Part of {book.localTitle}, already in your library

+
+ + ))} +
+
+ )} )} From eae980071210c3ada8bbc1360cb0c098e7bcb1be Mon Sep 17 00:00:00 2001 From: gch ahcg Date: Thu, 1 Oct 2026 22:36:44 -0400 Subject: [PATCH 2/4] test(seriesmatch): cover HasPartMarker/StripPartMarker directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving them here from internal/importer (#2524) left them reached only transitively through importer's own tests, which this package's own coverage profile does not credit — Codecov flagged both at 0% on the #2524 PR despite internal/importer's existing tests exercising them on every run. Direct table tests fix the gap without changing behaviour. Signed-off-by: gch ahcg --- internal/seriesmatch/match_test.go | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/internal/seriesmatch/match_test.go b/internal/seriesmatch/match_test.go index 0dea2e6e5..01bcd3ba5 100644 --- a/internal/seriesmatch/match_test.go +++ b/internal/seriesmatch/match_test.go @@ -286,6 +286,51 @@ func TestDifferentVolumesCatchesBareTrailingNumbers(t *testing.T) { } } +// TestHasPartMarker and TestStripPartMarker exercise the two helpers +// directly: moving them here from internal/importer (#2524, so series.go +// could use them too) left them reached only transitively through +// internal/importer's own tests, which a per-package coverage profile does +// not credit to this package. +func TestHasPartMarker(t *testing.T) { + cases := []struct { + name string + in string + want bool + }{ + {name: "part", in: "The Way of Kings, Part 1", want: true}, + {name: "pt abbreviation", in: "The Way of Kings, Pt. 2", want: true}, + {name: "part of", in: "The Great Hunt, Part 2 of 2", want: true}, + {name: "no marker", in: "The Way of Kings", want: false}, + {name: "blank", in: "", want: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := HasPartMarker(tc.in); got != tc.want { + t.Fatalf("HasPartMarker(%q) = %v, want %v", tc.in, got, tc.want) + } + }) + } +} + +func TestStripPartMarker(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {name: "trailing comma and marker", in: "The Way of Kings, Part 1", want: "The Way of Kings"}, + {name: "part of", in: "The Great Hunt, Part 2 of 2", want: "The Great Hunt"}, + {name: "no marker is unchanged", in: "The Way of Kings", want: "The Way of Kings"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := StripPartMarker(tc.in); got != tc.want { + t.Fatalf("StripPartMarker(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + // TestIntegerFloorPosition pins the #2524 fractional-position test: only a // genuinely fractional position reports a floor, and a whole number, blank, // or non-numeric position reports none. From ff98dcfb9b119dcd6354c2f67573b0afaa45211e Mon Sep 17 00:00:00 2001 From: gch ahcg Date: Wed, 7 Oct 2026 21:29:34 -0400 Subject: [PATCH 3/4] fix(web): move the covered-section strings into en.json (#2524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the new covered section on SeriesPage had hardcoded English and a hand-rolled plural, while the rest of the page goes through t(). Moved both strings to series.hardcover.coveredSection (i18next _one/_other) and series.hardcover.coveredSubtitle. Also drops the em dash from the changelog fragment and trims it — review feedback that it read as release notes rather than a changelog bullet. Signed-off-by: gch ahcg --- changelog.d/2524-split-editions.md | 2 +- web/src/i18n/locales/en.json | 5 ++++- web/src/pages/SeriesPage.test.tsx | 13 ++++++++++++- web/src/pages/SeriesPage.tsx | 6 ++++-- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/changelog.d/2524-split-editions.md b/changelog.d/2524-split-editions.md index 0a30cd629..4190c218d 100644 --- a/changelog.d/2524-split-editions.md +++ b/changelog.d/2524-split-editions.md @@ -1,2 +1,2 @@ ### Fixed -- **Split-edition catalogue rows no longer count as missing books you don't have** (#2524): some Hardcover catalogs list a novel's own split-into-parts editions alongside the whole novel — Brandon Sanderson's The Way of Kings at position 1, with its "Part 1" and "Part 2" at 1.1 and 1.2. Already owning the whole novel left those parts reading as missing volumes on the Series page, counted against the series and sitting behind an add button that would create and queue a download for text already on the shelf. They now show up in their own collapsed section instead, linked to the book that already covers them, and Fill gaps no longer creates them. A real novella at a neighbouring fractional position, such as Edgedancer beside Words of Radiance, is unaffected, and a series represented only by its split parts, with no unsplit whole to speak of, is unaffected too. Thanks magrhino for the Stormlight and Wheel of Time catalogue data that shaped the rule. +- **Split-edition catalogue rows no longer count as missing books you already own** (#2524): some Hardcover catalogs list a novel's own split-into-parts editions beside the whole novel (The Way of Kings at position 1, with "Part 1" and "Part 2" at 1.1 and 1.2). Already owning the whole novel left those parts listed as missing and offered an add button that queued a duplicate download. They now show in their own collapsed section, linked to the book that covers them, and Fill gaps no longer creates them. Thanks magrhino for the Stormlight and Wheel of Time catalogue data that shaped the rule. diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 093e87d16..bf5f87ac5 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -858,7 +858,10 @@ "add": "add", "addTitle": "Add this missing Hardcover book and search indexers", "moreMissing_one": "{{count}} more missing book", - "moreMissing_other": "{{count}} more missing books" + "moreMissing_other": "{{count}} more missing books", + "coveredSection_one": "{{count}} split edition of a book you own", + "coveredSection_other": "{{count}} split editions of books you own", + "coveredSubtitle": "Part of {{title}}, already in your library" }, "nameModal": { "name": "Name", diff --git a/web/src/pages/SeriesPage.test.tsx b/web/src/pages/SeriesPage.test.tsx index 666d7cffe..e6cdded76 100644 --- a/web/src/pages/SeriesPage.test.tsx +++ b/web/src/pages/SeriesPage.test.tsx @@ -1023,6 +1023,15 @@ describe('SeriesPage', () => { localBookId: 7, localTitle: 'The Way of Kings', }, + { + foreignBookId: 'hc:the-way-of-kings-part-2', + providerId: '101-2', + title: 'The Way of Kings, Part 2', + position: '1.2', + authorName: 'Brandon Sanderson', + localBookId: 7, + localTitle: 'The Way of Kings', + }, ], presentCount: 1, missingCount: 0, @@ -1031,11 +1040,13 @@ describe('SeriesPage', () => { renderSeriesPage([series]) fireEvent.click(await screen.findByRole('heading', { name: 'The Stormlight Archive' })) - fireEvent.click(await screen.findByText('1 split edition of a book you own')) + // Plural form (i18next _other), not a hand-rolled "s" suffix. + fireEvent.click(await screen.findByText('2 split editions of books you own')) const partLink = await screen.findByRole('link', { name: /The Way of Kings, Part 1/ }) expect(partLink).toHaveAttribute('href', '/book/7') expect(within(partLink).queryByRole('button', { name: 'add' })).not.toBeInTheDocument() + expect(within(partLink).getByText('Part of The Way of Kings, already in your library')).toBeInTheDocument() }) it('keeps a Hardcover missing row without a library match non-clickable', async () => { diff --git a/web/src/pages/SeriesPage.tsx b/web/src/pages/SeriesPage.tsx index f7e217d89..ffd193297 100644 --- a/web/src/pages/SeriesPage.tsx +++ b/web/src/pages/SeriesPage.tsx @@ -751,7 +751,7 @@ export default function SeriesPage() { {diff && diff.covered.length > 0 && (
- {diff.covered.length} split edition{diff.covered.length === 1 ? ' of a book you own' : 's of books you own'} + {t('series.hardcover.coveredSection', { count: diff.covered.length })}
{diff.covered.map(book => ( @@ -765,7 +765,9 @@ export default function SeriesPage() {

{book.title}

-

Part of {book.localTitle}, already in your library

+

+ {t('series.hardcover.coveredSubtitle', { title: book.localTitle })} +

))} From b53f844066b0fc0ff47f42134c27e93ab2827a23 Mon Sep 17 00:00:00 2001 From: gch ahcg Date: Sat, 10 Oct 2026 13:51:52 -0400 Subject: [PATCH 4/4] fix(series): consolidate split-edition detection with #3109 (#2524) #3109 landed on main while this was open and introduced its own copy of the same two-signal rule (seriesmatch.SplitEditionPartOf, db.ListCoveredSplitEditionParts) for rows already stored, asking whichever PR landed second to line up with the other. - splitEditionWhole now calls seriesmatch.SplitEditionPartOf instead of the separate IntegerFloorPosition/SplitPartOf pair, which are removed; HasPartMarker/StripPartMarker keep their own tests but now share split_edition.go's splitEditionMarkerRe instead of a second copy of the same regex. - The eligibility gate ListCoveredSplitEditionParts applies to a stored whole (not excluded, imported or monitored) now gates a Present row's eligibility to cover a catalogue-diff part too, via a new eligibleSplitEditionWhole helper cross-referenced from both sides of the SQL/Go boundary, with a test for each half of the gate (excluded, wanted-and-unmonitored, wanted-and-monitored). - Merged the wiki's two split-edition paragraphs into one and dropped the em dashes. Review feedback from vavallee. Signed-off-by: gch ahcg --- docs/Hardcover-Series-Wiki.md | 4 +- internal/api/series.go | 87 +++++++++++---- internal/api/series_test.go | 170 +++++++++++++++++++++++------ internal/db/split_edition_parts.go | 7 +- internal/seriesmatch/match.go | 54 +-------- internal/seriesmatch/match_test.go | 66 +---------- 6 files changed, 210 insertions(+), 178 deletions(-) diff --git a/docs/Hardcover-Series-Wiki.md b/docs/Hardcover-Series-Wiki.md index 017feef07..4fd778be8 100644 --- a/docs/Hardcover-Series-Wiki.md +++ b/docs/Hardcover-Series-Wiki.md @@ -87,15 +87,13 @@ Missing-book fill uses the linked Hardcover catalog as the source of truth for m Catalog entries whose titles name a box set rather than a book ("box set", "boxed set", "collection set", "3 Books Set") are never created by fill, the same way they are pruned from an author catalogue on ingestion. They are still listed in the diff, so clicking **add** on one does nothing and reports nothing queued. -Some catalogs also list a novel's own split-into-parts editions beside the whole novel — Brandon Sanderson's The Way of Kings sits at position 1, with its "Part 1" and "Part 2" editions at 1.1 and 1.2. If the whole novel is already present, its parts are reported as **covered**, not missing: they are not counted, not offered an **add** button, and not created by fill, because the book is already on the shelf. They appear in their own collapsed section on the Series page ("N split editions of books you own"), each linking to the whole work that covers it. The rule needs both a fractional position under a present whole AND a title that is the whole's title plus a "Part N" marker, because a fractional position alone is not enough: a real novella, such as Edgedancer at position 2.5 beside Words of Radiance at 2, carries no "Part N" marker and is correctly left in Missing. A catalog that represents a work ONLY as its split parts, with no unsplit row at the integer position at all — Robert Jordan's Wheel of Time "Two Volume Edition" catalog is like this — never triggers the rule either, because there is nothing present to cover the parts; they are the real volumes there and stay in Missing. - 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. +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 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" (a subtitle after the marker is ignored). Position alone is not enough: a real novella at a fractional position, such as "Edgedancer" at 2.5 beside "Words of Radiance" at 2, has no "Part N" marker and stays a normal book, and a catalog that represents a work only as its split parts, with no unsplit row at the integer position at all (Robert Jordan's Wheel of Time "Two Volume Edition" catalog is like this), never triggers the rule either, since there is nothing to cover the parts. On the Hardcover catalog diff a covered part is reported as covered, not missing: it is not counted, not offered an **add** button, and not created by fill, and appears in its own collapsed section on the Series page ("N split editions of books you own") linking to the whole that covers it. The same rule covers rows already stored locally: once the whole is imported, or monitored and not excluded, fill no longer re-queues a stored part and the scheduled wanted search no longer searches it, so a part of a book you already have is never downloaded. Stored parts are marked **Split part** on the series card and are not counted as missing. If a fill from an older version left one monitored, **Unmonitor N split parts** on the series card unmonitors them in one click. Nothing is deleted, and an imported part is never touched. 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. diff --git a/internal/api/series.go b/internal/api/series.go index 52d343559..c2d7fbd36 100644 --- a/internal/api/series.go +++ b/internal/api/series.go @@ -1446,6 +1446,12 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s // and pushed the second local's real catalog entry into Missing behind an // Add button that ensureHardcoverCatalogBook's guards then refuse. matchedCatalog := make(map[int]struct{}) + // presentWholes collects the subset of Present rows eligible to cover a + // split-edition part (#2524/#3048): not excluded, and imported or + // monitored, the same gate db.ListCoveredSplitEditionParts applies to + // stored rows, so the diff and that sweep never disagree about which + // wholes cover which parts. + presentWholes := make([]presentWhole, 0, len(series.Books)) // Identities bind first (#2553). Assigning in library order let an // earlier local's fuzzy title match claim the catalogue slot that a later // local owned by foreign ID: "He Who Fights with Monsters 4" took volume @@ -1540,6 +1546,14 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s if match.score >= 90 || match.foreignID { diff.Present = append(diff.Present, item) matchedCatalog[match.index] = struct{}{} + if eligibleSplitEditionWhole(local.Book) { + presentWholes = append(presentWholes, presentWhole{ + position: item.Position, + title: item.Title, + localBookID: local.Book.ID, + localTitle: local.Book.Title, + }) + } } else if match.score >= 70 { diff.Uncertain = append(diff.Uncertain, item) matchedCatalog[match.index] = struct{}{} @@ -1559,10 +1573,11 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s if title := firstNonEmpty(book.Book.Title, book.Title); metadata.IsUnambiguousBundleTitle(title) { continue } - if whole, ok := splitEditionWhole(book, diff.Present); ok { + if whole, ok := splitEditionWhole(book, presentWholes); ok { item := catalogDiffBook(book, catalog.AuthorName) - item.LocalBookID = whole.LocalBookID - item.LocalTitle = whole.LocalTitle + id := whole.localBookID + item.LocalBookID = &id + item.LocalTitle = whole.localTitle diff.Covered = append(diff.Covered, item) continue } @@ -1575,41 +1590,65 @@ func buildHardcoverDiff(ctx context.Context, books *db.BookRepo, userID int64, s return diff } +// presentWhole is the subset of a Present row that splitEditionWhole needs: +// a catalogue diff row the library holds, eligible to cover a split-edition +// part (#2524/#3048). Kept separate from seriesHardcoverDiffBook so the +// eligibility gate (not excluded, imported or monitored) is decided once, +// where local.Book is still in scope, rather than re-derived from the API +// response shape. +type presentWhole struct { + position string + title string + localBookID int64 + localTitle string +} + +// eligibleSplitEditionWhole reports whether book is a book the user actually +// has or wants, so it is allowed to cover a split-edition part (#3048): not +// excluded, and imported or monitored. +// +// This MUST stay in agreement with the SQL predicate in +// db.ListCoveredSplitEditionParts (internal/db/split_edition_parts.go), +// which applies the identical rule to rows already stored. The two cannot +// share one literal implementation across the SQL/Go boundary, so an +// eligibility change here needs the same change made there, and vice versa, +// or the catalogue diff and the stored-row sweep will disagree about which +// wholes cover which parts. +func eligibleSplitEditionWhole(b *models.Book) bool { + return !b.Excluded && (b.Status == models.BookStatusImported || b.Monitored) +} + // splitEditionWhole reports whether book is a split-edition part of some -// already-present whole work, and if so returns that whole work's diff row. +// already-present, eligible whole work, and if so returns that whole. // -// #2524: some Hardcover catalogues list a novel AND its split parts at +// #2524/#3048: some Hardcover catalogues list a novel AND its split parts at // fractional positions under the novel's own position — Stormlight's "The // Way of Kings" at 1 alongside "…, Part 1" and "…, Part 2" at 1.1 and 1.2. // Owning the whole novel should not leave its own parts reading as missing // volumes, counting against the series, and sitting behind an Add button // that would create and queue a download for text already on the shelf. // -// Position alone is not enough to call that: a novella can sit at a -// fractional position too (Edgedancer at 2.5, beside Words of Radiance at -// 2), and a novella is a real, separate book. So both signals are required: -// book's position must be the fractional child of a Present row's integer -// position (seriesmatch.IntegerFloorPosition), AND book's title must be that -// Present row's title plus a "Part N" marker (seriesmatch.SplitPartOf). A +// The two-signal rule itself is seriesmatch.SplitEditionPartOf, shared with +// db.ListCoveredSplitEditionParts (#3048), which applies the same rule to +// rows already stored for an install that filled before this landed. A // series represented ONLY by its split parts — Wheel of Time's "Two Volume // Edition", where there is no unsplit whole at the integer position — never -// matches this, because there is no Present row to match against; its parts -// are the real volumes and stay Missing. -func splitEditionWhole(book metadata.SeriesCatalogBook, present []seriesHardcoverDiffBook) (seriesHardcoverDiffBook, bool) { - floor, ok := seriesmatch.IntegerFloorPosition(book.Position) - if !ok { - return seriesHardcoverDiffBook{}, false - } +// matches this, because there is no eligible whole to match against; its +// parts are the real volumes and stay Missing. +// +// wholes is pre-filtered to Present rows that are also eligible (not +// excluded, imported or monitored), so an excluded or purely-wanted, +// unmonitored Present row covers nothing — the same gate the stored-row +// sweep applies, so the two never disagree about which wholes cover which +// parts. +func splitEditionWhole(book metadata.SeriesCatalogBook, wholes []presentWhole) (presentWhole, bool) { partTitle := firstNonEmpty(book.Book.Title, book.Title) - for _, whole := range present { - if whole.Position == "" || !seriesmatch.SamePosition(whole.Position, floor) { - continue - } - if seriesmatch.SplitPartOf(partTitle, whole.Title) { + for _, whole := range wholes { + if seriesmatch.SplitEditionPartOf(partTitle, book.Position, whole.title, whole.position) { return whole, true } } - return seriesHardcoverDiffBook{}, false + return presentWhole{}, false } // logDiffDecision records which catalogue entry a local series book bound diff --git a/internal/api/series_test.go b/internal/api/series_test.go index a8195d93c..95c7a7932 100644 --- a/internal/api/series_test.go +++ b/internal/api/series_test.go @@ -3099,21 +3099,26 @@ func splitEditionCatalog() *metadata.SeriesCatalog { // Uncertain band: a whole work that is Uncertain or Missing simply never // appears in the `present` slice this function is handed, which is exactly // the real-world shape (buildHardcoverDiff only passes diff.Present). +// TestSplitEditionWhole unit-tests the matching loop directly against +// crafted presentWhole candidates, independent of how a candidate came to be +// eligible — that eligibility gate (not excluded, imported or monitored) is +// decided by the caller before a whole ever reaches this function (see the +// buildHardcoverDiff-level tests for the gate itself), so an absent whole +// here stands for either "no Present row at this position" (the Two Volume +// Edition trap) or "a Present row that failed the eligibility gate." func TestSplitEditionWhole(t *testing.T) { - wayOfKingsID := int64(1) - present := []seriesHardcoverDiffBook{{ - ForeignBookID: "hc:the-way-of-kings", - Title: "The Way of Kings", - Position: "1", - LocalBookID: &wayOfKingsID, - LocalTitle: "The Way of Kings", + wholes := []presentWhole{{ + position: "1", + title: "The Way of Kings", + localBookID: 1, + localTitle: "The Way of Kings", }} cases := []struct { - name string - book metadata.SeriesCatalogBook - present []seriesHardcoverDiffBook - wantOK bool + name string + book metadata.SeriesCatalogBook + wholes []presentWhole + wantOK bool }{ { name: "first part of an owned whole", @@ -3122,8 +3127,8 @@ func TestSplitEditionWhole(t *testing.T) { Title: "The Way of Kings, Part 1", Book: models.Book{Title: "The Way of Kings, Part 1"}, }, - present: present, - wantOK: true, + wholes: wholes, + wantOK: true, }, { name: "second part of an owned whole", @@ -3132,8 +3137,8 @@ func TestSplitEditionWhole(t *testing.T) { Title: "The Way of Kings, Part 2", Book: models.Book{Title: "The Way of Kings, Part 2"}, }, - present: present, - wantOK: true, + wholes: wholes, + wantOK: true, }, { name: "novella at a fractional position is not a split edition", @@ -3142,8 +3147,8 @@ func TestSplitEditionWhole(t *testing.T) { Title: "Dawnshard", Book: models.Book{Title: "Dawnshard"}, }, - present: present, - wantOK: false, + wholes: wholes, + wantOK: false, }, { name: "whole number position never matches, even with a part title", @@ -3152,38 +3157,28 @@ func TestSplitEditionWhole(t *testing.T) { Title: "The Way of Kings, Part 1", Book: models.Book{Title: "The Way of Kings, Part 1"}, }, - present: present, - wantOK: false, + wholes: wholes, + wantOK: false, }, { - name: "fractional position but no Present row at the floor (Two Volume Edition shape)", + name: "no eligible whole at all (Two Volume Edition shape, or one the caller already filtered out)", book: metadata.SeriesCatalogBook{ Position: "1.1", Title: "The Eye of the World, Part 1 of 2", Book: models.Book{Title: "The Eye of the World, Part 1 of 2"}, }, - present: nil, - wantOK: false, - }, - { - name: "whole is bound only as Uncertain or Missing, not Present", - book: metadata.SeriesCatalogBook{ - Position: "1.1", - Title: "The Way of Kings, Part 1", - Book: models.Book{Title: "The Way of Kings, Part 1"}, - }, - present: nil, // the whole never made it into the Present slice - wantOK: false, + wholes: nil, + wantOK: false, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - got, ok := splitEditionWhole(tc.book, tc.present) + got, ok := splitEditionWhole(tc.book, tc.wholes) if ok != tc.wantOK { t.Fatalf("splitEditionWhole() ok = %v, want %v (got %+v)", ok, tc.wantOK, got) } - if ok && (got.LocalBookID == nil || *got.LocalBookID != wayOfKingsID) { - t.Fatalf("splitEditionWhole() local = %v, want %d", got.LocalBookID, wayOfKingsID) + if ok && got.localBookID != 1 { + t.Fatalf("splitEditionWhole() local = %d, want 1", got.localBookID) } }) } @@ -3295,6 +3290,111 @@ func TestBuildHardcoverDiffSplitEditionsNeedAnOwnedWhole(t *testing.T) { } } +// TestBuildHardcoverDiffExcludedWholeDoesNotCoverParts is the #3109 review +// follow-up: db.ListCoveredSplitEditionParts only lets a not-excluded whole +// cover a stored part, and the catalogue diff has to apply the same gate so +// the two never disagree about which wholes cover which parts. An excluded +// Present row — the user said "not this edition" — covers nothing; its parts +// fall back to Missing like any other unclaimed catalogue row. +func TestBuildHardcoverDiffExcludedWholeDoesNotCoverParts(t *testing.T) { + catalog := splitEditionCatalog() + series := &models.Series{ + ID: 1, + Title: "The Stormlight Archive", + Books: []models.SeriesBook{{ + BookID: 1, + Book: &models.Book{ + ID: 1, + ForeignID: "calibre:book:1", + Title: "The Way of Kings", + SortTitle: "The Way of Kings", + Status: models.BookStatusImported, + Excluded: true, + }, + }}, + } + link := &models.SeriesHardcoverLink{SeriesID: series.ID, HardcoverSeriesID: catalog.ForeignID} + + diff := buildHardcoverDiff(context.Background(), nil, 0, series, link, catalog) + + if len(diff.Covered) != 0 { + t.Fatalf("Covered = %v, want empty: an excluded whole must not cover its parts", diffForeignIDs(diff.Covered)) + } + missing := map[string]bool{} + for _, row := range diff.Missing { + missing[row.ForeignBookID] = true + } + for _, id := range []string{"hc:the-way-of-kings-part-1", "hc:the-way-of-kings-part-2"} { + if !missing[id] { + t.Fatalf("%q should fall back to Missing when its whole is excluded: %v", id, diffForeignIDs(diff.Missing)) + } + } +} + +// TestBuildHardcoverDiffUnmonitoredWantedWholeDoesNotCoverParts pins the +// other half of the #3109 gate: a Present whole that is neither imported nor +// monitored (Wanted and unmonitored, perhaps added and then turned off) is +// not a book the user actually has or wants, so it must not cover parts +// either. TestBuildHardcoverDiffCoversSplitEditionsOfOwnedWork already pins +// the imported case; this and the next test pin both halves of the "or +// monitored" branch. +func TestBuildHardcoverDiffUnmonitoredWantedWholeDoesNotCoverParts(t *testing.T) { + catalog := splitEditionCatalog() + series := &models.Series{ + ID: 1, + Title: "The Stormlight Archive", + Books: []models.SeriesBook{{ + BookID: 1, + Book: &models.Book{ + ID: 1, + ForeignID: "calibre:book:1", + Title: "The Way of Kings", + SortTitle: "The Way of Kings", + Status: models.BookStatusWanted, + Monitored: false, + }, + }}, + } + link := &models.SeriesHardcoverLink{SeriesID: series.ID, HardcoverSeriesID: catalog.ForeignID} + + diff := buildHardcoverDiff(context.Background(), nil, 0, series, link, catalog) + + if len(diff.Covered) != 0 { + t.Fatalf("Covered = %v, want empty: a wanted-but-unmonitored whole must not cover its parts", diffForeignIDs(diff.Covered)) + } +} + +// TestBuildHardcoverDiffMonitoredWantedWholeCoversParts is the positive half +// of the "or monitored" branch: a Present whole that is Wanted but monitored +// — the user asked Bindery to go find it — is exactly the #3048 wiki case +// ("monitored and wanted"), and does cover its parts even though it is not +// yet imported. +func TestBuildHardcoverDiffMonitoredWantedWholeCoversParts(t *testing.T) { + catalog := splitEditionCatalog() + series := &models.Series{ + ID: 1, + Title: "The Stormlight Archive", + Books: []models.SeriesBook{{ + BookID: 1, + Book: &models.Book{ + ID: 1, + ForeignID: "calibre:book:1", + Title: "The Way of Kings", + SortTitle: "The Way of Kings", + Status: models.BookStatusWanted, + Monitored: true, + }, + }}, + } + link := &models.SeriesHardcoverLink{SeriesID: series.ID, HardcoverSeriesID: catalog.ForeignID} + + diff := buildHardcoverDiff(context.Background(), nil, 0, series, link, catalog) + + if len(diff.Covered) != 2 { + t.Fatalf("Covered = %v, want exactly the two Way of Kings parts (whole is wanted and monitored)", diffForeignIDs(diff.Covered)) + } +} + // TestSeriesHandler_LifetimeCtxFallsBackToBackground is the #846 follow-up // guard for fanOutSeriesSearches. Same contract as BookHandler.bgCtx(). func TestSeriesHandler_LifetimeCtxFallsBackToBackground(t *testing.T) { diff --git a/internal/db/split_edition_parts.go b/internal/db/split_edition_parts.go index c99e963d2..a4c97941b 100644 --- a/internal/db/split_edition_parts.go +++ b/internal/db/split_edition_parts.go @@ -35,8 +35,11 @@ type SplitEditionPart struct { // Only a part that is not imported is reported: an imported part is already on // disk and there is nothing to stop searching for. The whole has to be a book // the user actually has or wants, so it must be imported or monitored, and not -// excluded. Both rows must belong to the same owner, so one user's library -// never decides what another user's sweep searches for. +// excluded — the WHERE clause below and api.eligibleSplitEditionWhole +// (internal/api/series.go) apply the identical rule to the catalogue diff; +// an eligibility change here needs the same change made there. Both rows +// must belong to the same owner, so one user's library never decides what +// another user's sweep searches for. func (r *BookRepo) ListCoveredSplitEditionParts(ctx context.Context, seriesID int64) ([]SplitEditionPart, error) { query := ` SELECT sp.series_id, sp.book_id, sp.position_in_series, bp.title, bp.monitored, diff --git a/internal/seriesmatch/match.go b/internal/seriesmatch/match.go index 25883961e..8b33a989e 100644 --- a/internal/seriesmatch/match.go +++ b/internal/seriesmatch/match.go @@ -27,24 +27,6 @@ func SamePosition(a, b string) bool { return aerr == nil && berr == nil && math.Abs(af-bf) < 0.001 } -// IntegerFloorPosition reports the integer floor of a fractional series -// position, and whether pos actually IS fractional. "1.1" -> ("1", true); a -// whole number, "", or anything that does not parse as a number all report -// (_, false) — this exists specifically to find the WHOLE work a split-part -// position like 1.1 or 1.2 sits under (#2524), so a position that is already -// whole has no "floor" worth reporting. -func IntegerFloorPosition(pos string) (string, bool) { - f, err := strconv.ParseFloat(strings.TrimSpace(pos), 64) - if err != nil { - return "", false - } - floor := math.Floor(f) - if math.Abs(f-floor) < 1e-9 { - return "", false - } - return strconv.FormatFloat(floor, 'f', -1, 64), true -} - // NormalizeSeriesName reduces a series name for comparison: the canonical // dedup key, minus a redundant trailing collective noun ("… Series", "… // Trilogy"). @@ -293,49 +275,21 @@ func CleanTitle(title string) string { return strings.Join(out, " ") } -// partMarkerRe matches a "Part N" marker, with an optional "of M": "Part 1", -// "Pt. 2", "Part 1 of 3". -var partMarkerRe = regexp.MustCompile(`(?i)\b(?:part|pt)\.?\s*\d+(?:\.\d+)?(?:\s+of\s+\d+)?\b`) - // HasPartMarker reports whether s carries an explicit "Part N" / "Pt. N" -// marker. +// marker. Shares splitEditionMarkerRe with SplitEditionPartOf (#3048, #2524) +// rather than keeping a second copy of the same pattern. func HasPartMarker(s string) bool { - return partMarkerRe.MatchString(s) + return splitEditionMarkerRe.MatchString(s) } // StripPartMarker removes a "Part N" marker from s and cleans up the // whitespace and punctuation left behind: "The Way of Kings, Part 1" -> // "The Way of Kings". func StripPartMarker(s string) string { - s = partMarkerRe.ReplaceAllString(s, " ") + s = splitEditionMarkerRe.ReplaceAllString(s, " ") return strings.TrimRight(strings.TrimSpace(s), " ,.:;-_") } -// SplitPartOf reports whether partTitle names one split-edition part of -// wholeTitle: the text before a "Part N" marker, with trailing punctuation -// trimmed, is wholeTitle under CleanTitle. Any subtitle AFTER the marker is -// ignored, because a catalogue sometimes hangs one off the part itself — -// "The Great Hunt, Part 2 of 2: New Threads in the Pattern" is still part 2 -// of "The Great Hunt". -// -// A bare marker with nothing before it ("Part 1" alone) never matches: an -// empty prefix can only equal an equally blank wholeTitle. A title that -// carries no marker at all — a novella like "Edgedancer" — is never a split -// part of anything, however short its name or close its series position; -// that is what keeps a fractional-position novella out of the #2524 rule -// this exists for (see IntegerFloorPosition). -func SplitPartOf(partTitle, wholeTitle string) bool { - loc := partMarkerRe.FindStringIndex(partTitle) - if loc == nil { - return false - } - prefix := strings.TrimRight(strings.TrimSpace(partTitle[:loc[0]]), " ,:;-") - if prefix == "" { - return false - } - return CleanTitle(prefix) == CleanTitle(wholeTitle) -} - // volumeNumberRe matches an EXPLICIT volume marker followed by a number: // "Vol. 3", "Volume 3", "Book 3", "Part 3", "#3". A bare trailing number is // deliberately not matched — "Fahrenheit 451" and "Catch 22" are titles, not diff --git a/internal/seriesmatch/match_test.go b/internal/seriesmatch/match_test.go index 01bcd3ba5..53969acb7 100644 --- a/internal/seriesmatch/match_test.go +++ b/internal/seriesmatch/match_test.go @@ -331,67 +331,5 @@ func TestStripPartMarker(t *testing.T) { } } -// TestIntegerFloorPosition pins the #2524 fractional-position test: only a -// genuinely fractional position reports a floor, and a whole number, blank, -// or non-numeric position reports none. -func TestIntegerFloorPosition(t *testing.T) { - cases := []struct { - name string - pos string - wantFloor string - wantOK bool - }{ - {name: "simple fraction", pos: "1.1", wantFloor: "1", wantOK: true}, - {name: "second fraction", pos: "1.2", wantFloor: "1", wantOK: true}, - {name: "novella position", pos: "2.5", wantFloor: "2", wantOK: true}, - {name: "whole number", pos: "1", wantOK: false}, - {name: "whole number written as float", pos: "1.0", wantOK: false}, - {name: "blank", pos: "", wantOK: false}, - {name: "non numeric", pos: "prelude", wantOK: false}, - {name: "trims whitespace", pos: " 1.1 ", wantFloor: "1", wantOK: true}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - floor, ok := IntegerFloorPosition(tc.pos) - if ok != tc.wantOK { - t.Fatalf("IntegerFloorPosition(%q) ok = %v, want %v", tc.pos, ok, tc.wantOK) - } - if ok && floor != tc.wantFloor { - t.Fatalf("IntegerFloorPosition(%q) floor = %q, want %q", tc.pos, floor, tc.wantFloor) - } - }) - } -} - -// TestSplitPartOf pins the #2524 title half of the split-edition rule: the -// text before a "Part N" marker, not the marker or anything after it, is -// what must match the whole work's title. -func TestSplitPartOf(t *testing.T) { - cases := []struct { - name string - part string - whole string - want bool - }{ - {name: "simple part", part: "The Way of Kings, Part 1", whole: "The Way of Kings", want: true}, - {name: "second part", part: "The Way of Kings, Part 2", whole: "The Way of Kings", want: true}, - {name: "pt spelling", part: "The Way of Kings, Pt. 2", whole: "The Way of Kings", want: true}, - { - name: "part carries its own subtitle", - part: "The Great Hunt, Part 2 of 2: New Threads in the Pattern", - whole: "The Great Hunt", - want: true, - }, - {name: "wrong whole work", part: "Words of Radiance, Part 1", whole: "The Way of Kings", want: false}, - {name: "novella has no marker at all", part: "Edgedancer", whole: "Words of Radiance", want: false}, - {name: "bare marker, no prefix to compare", part: "Part 1", whole: "The Way of Kings", want: false}, - {name: "case and punctuation folded", part: "the way of kings: part 1", whole: "The Way Of Kings", want: true}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if got := SplitPartOf(tc.part, tc.whole); got != tc.want { - t.Fatalf("SplitPartOf(%q, %q) = %v, want %v", tc.part, tc.whole, got, tc.want) - } - }) - } -} +// The #2524 fractional-position and title-prefix rules are now +// seriesmatch.SplitEditionPartOf (#3048), tested in split_edition_test.go.