From c74573f5bd08f0c4a832f02925db8e0a8260cef0 Mon Sep 17 00:00:00 2001 From: vavallee Date: Sat, 26 Sep 2026 22:04:24 -0300 Subject: [PATCH 1/3] fix(indexer): run the #2502 relevance guards on the search paths that grab filterRelevant and filterRelevantDebug were two copies of the relevance filter. #2502 added its guards, conflictingTitleAuthor and the title identity phrase gate, to filterRelevant only, and pinned them with tests that call filterRelevant directly. Production never runs filterRelevant. The scheduler calls SearchBookWithOutcomes, which projects SearchBookWithDebug, and the interactive search calls SearchBookWithDebug; both run filterRelevantDebug. filterRelevant is reached only through the scheduler's fallback for searchers that cannot report outcomes, which in practice is test stubs. So every release #2502 was written to reject was still kept, and automatic search could still grab it. filterRelevantDetailed is now the one implementation and returns the kept results plus a reason for each drop. filterRelevant and filterRelevantDebug are wrappers over it. The drop record now distinguishes a conflicting author and a split title phrase from a plain keyword miss. The #2502 tables run through both wrappers, and a new test drives SearchBook, SearchBookWithDebug and SearchBookWithOutcomes against one fake indexer serving the wrong releases beside the right ones. On main both production entry points keep all four wrong releases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee --- .../relevance-filter-single-implementation.md | 2 + internal/indexer/debug.go | 73 +------------ .../indexer/relevance_entrypoints_test.go | 101 ++++++++++++++++++ internal/indexer/searcher.go | 48 +++++++-- internal/indexer/title_identity_test.go | 46 +++++--- 5 files changed, 181 insertions(+), 89 deletions(-) create mode 100644 changelog.d/relevance-filter-single-implementation.md create mode 100644 internal/indexer/relevance_entrypoints_test.go diff --git a/changelog.d/relevance-filter-single-implementation.md b/changelog.d/relevance-filter-single-implementation.md new file mode 100644 index 000000000..117e802e4 --- /dev/null +++ b/changelog.d/relevance-filter-single-implementation.md @@ -0,0 +1,2 @@ +### Fixed +- **Automatic search no longer grabs a same titled book by a different author**: the release checks added in v1.35 to stop grabs like Edward Luttwak's "Coup d'État" for Ben Coes's "Coup d'Etat", "The Power of Writing It Down" for "Power Down", or "12 More Rules for Life" for "12 Rules for Life" never reached a real search. The relevance filter existed as two copies, the guards went into one, and both automatic search and the interactive Search button run the other. There is now a single implementation, so every guard applies to every search. The interactive search panel also says which check dropped a release: a different author for this title, or title words split by words the title does not have. diff --git a/internal/indexer/debug.go b/internal/indexer/debug.go index 82150e5f7..be9767936 100644 --- a/internal/indexer/debug.go +++ b/internal/indexer/debug.go @@ -255,75 +255,12 @@ func filterUsenetJunkDebug(results []newznab.SearchResult) ([]newznab.SearchResu return out, dropped } -// filterRelevantDebug is filterRelevant instrumented to record each drop with -// the keyword set that failed to match. +// filterRelevantDebug returns the relevance filter's kept results together with +// a record of every drop and its reason, for the interactive search panel and +// for the automatic search that projects it (SearchBookWithOutcomes). It shares +// its implementation with filterRelevant; see filterRelevantDetailed. func filterRelevantDebug(results []newznab.SearchResult, title, author string, aliases []string) ([]newznab.SearchResult, []FilterDebug) { - // Strip edition qualifiers ("(German Edition)" etc.) and normalize smart - // quotes before tokenizing — mirrors the first step of filterRelevant so - // both paths produce identical keyword sets. - title = newznab.NormalizeQueryTitle(title) - // Strip possessive author prefix before keyword extraction (mirrors filterRelevant). - title = stripPossessivePrefix(title, author) - fullKws := newznab.SigWords(title) - primaryKws := newznab.SigWords(primaryTitle(title)) - authorKws := newznab.SigWords(author) - // Elision fallback, identical to filterRelevant: this path serves the - // interactive search AND auto-grab (SearchBookWithOutcomes projects - // SearchBookWithDebug), so a fix applied to one filter and not the other - // is invisible here and live there. See newznab.SigWordsElided. - fullElided := newznab.SigWordsElided(title) - primaryElided := newznab.SigWordsElided(primaryTitle(title)) - - authorTokenSets := latinAliasTokenSets(author, aliases) - - tryMatch := func(n string, kws []string) bool { - for _, toks := range authorTokenSets { - if titleMatchesResult(n, kws, toks, true) { - return true - } - } - return false - } - - // tryMatchElided retries a FAILED strict match with the apostrophe- - // separated reading. Never tried first, skipped when the readings agree. - tryMatchElided := func(n string, elided, kws []string) bool { - if len(elided) == 0 || sameKws(elided, kws) { - return false - } - return tryMatch(n, elided) - } - - if len(fullKws) == 0 && len(primaryKws) == 0 && len(authorKws) == 0 { - return results, nil - } - - normTitles := make([]string, len(results)) - for i, r := range results { - normTitles[i] = NormalizeRelease(r.Title) - } - - filtered := make([]newznab.SearchResult, 0, len(results)) - var dropped []FilterDebug - for i, r := range results { - n := normTitles[i] - fullOK := tryMatch(n, fullKws) || tryMatchElided(n, fullElided, fullKws) - primaryOK := false - if !fullOK && len(primaryKws) > 0 && !sameKws(primaryKws, fullKws) { - primaryOK = tryMatch(n, primaryKws) || tryMatchElided(n, primaryElided, primaryKws) - } - if fullOK || primaryOK { - filtered = append(filtered, r) - continue - } - dropped = append(dropped, FilterDebug{ - Title: r.Title, - IndexerName: r.IndexerName, - Stage: "relevance", - Reason: "title/author keywords did not match release name", - }) - } - return filtered, dropped + return filterRelevantDetailed(results, title, author, aliases) } // SearchBookWithOutcomes is SearchBook plus a per-indexer account of what diff --git a/internal/indexer/relevance_entrypoints_test.go b/internal/indexer/relevance_entrypoints_test.go new file mode 100644 index 000000000..2efc5e150 --- /dev/null +++ b/internal/indexer/relevance_entrypoints_test.go @@ -0,0 +1,101 @@ +package indexer + +import ( + "context" + "fmt" + "html" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + + "github.com/vavallee/bindery/internal/models" +) + +// feedOf renders release titles as a newznab RSS response. +func feedOf(titles ...string) string { + var b strings.Builder + fmt.Fprintf(&b, ` + + + `, len(titles)) + for i, title := range titles { + fmt.Fprintf(&b, ` + + %s + g%d + + `, html.EscapeString(title), i, i) + } + b.WriteString("\n \n") + return b.String() +} + +// TestRelevanceGuardsReachEveryProductionEntrypoint drives the three real search +// entry points against one fake indexer serving #2502's wrong releases next to +// the right ones. +// +// #2502's guards lived in filterRelevant, and its tests called that function +// directly. But SearchBookWithOutcomes (automatic grabbing, via the scheduler) +// and SearchBookWithDebug (interactive search, via the API) both run +// filterRelevantDebug, which did not have them, so every release #2502 was +// written to reject was still grabbed. A unit test on one filter function +// cannot see that; only running the entry points can. +func TestRelevanceGuardsReachEveryProductionEntrypoint(t *testing.T) { + cases := []struct { + title, author string + right, wrong []string + }{ + { + title: "Coup d'Etat", author: "Ben Coes", + right: []string{"Coup D'Etat - Ben Coes EPUB"}, + wrong: []string{"Coup D'Etat - Edward Luttwak", "Coup D'Etat by Edward Luttwak EPUB"}, + }, + { + title: "Power Down", author: "Ben Coes", + right: []string{"Power Down by Ben Coes EPUB"}, + wrong: []string{"The Power of Writing It Down by Ben Coes EPUB"}, + }, + { + title: "12 Rules for Life", author: "Jordan B. Peterson", + right: []string{"12 Rules for Life by Jordan B. Peterson EPUB"}, + wrong: []string{"Beyond Order: 12 More Rules for Life by Jordan B. Peterson EPUB"}, + }, + } + for _, c := range cases { + t.Run(c.title, func(t *testing.T) { + feed := feedOf(append(slices.Clone(c.right), c.wrong...)...) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/xml") + _, _ = w.Write([]byte(feed)) + })) + defer srv.Close() + + idxs := []models.Indexer{{ID: 1, Name: "test", URL: srv.URL, Enabled: true, Categories: []int{7020}}} + crit := MatchCriteria{Title: c.title, Author: c.author} + ctx := context.Background() + + plain := newTestSearcher().SearchBook(ctx, idxs, crit) + debugged, _ := newTestSearcher().SearchBookWithDebug(ctx, idxs, crit) + outcomes, _ := newTestSearcher().SearchBookWithOutcomes(ctx, idxs, crit) + + for name, got := range map[string][]string{ + "SearchBook": resultTitles(plain), + "SearchBookWithDebug": resultTitles(debugged), + "SearchBookWithOutcomes": resultTitles(outcomes), + } { + for _, w := range c.wrong { + if slices.Contains(got, w) { + t.Errorf("%s kept the wrong release %q for %q by %s", name, w, c.title, c.author) + } + } + for _, r := range c.right { + if !slices.Contains(got, r) { + t.Errorf("%s dropped the right release %q for %q by %s", name, r, c.title, c.author) + } + } + } + }) + } +} diff --git a/internal/indexer/searcher.go b/internal/indexer/searcher.go index ece76a689..0d1f764cc 100644 --- a/internal/indexer/searcher.go +++ b/internal/indexer/searcher.go @@ -787,6 +787,26 @@ func titleMatchesResult(normResult string, titleKws []string, authorToks []strin // dropped when an abbreviated result set the gate — e.g. "Name.Wind.epub" // enabling strict mode that then rejected "Name.of.the.Wind.epub". func filterRelevant(results []newznab.SearchResult, title, author string, aliases []string) []newznab.SearchResult { + kept, _ := filterRelevantDetailed(results, title, author, aliases) + return kept +} + +// filterRelevantDetailed is the one implementation of the relevance filter, +// returning the kept results and a record of each result it dropped and why. +// filterRelevant and filterRelevantDebug are both thin wrappers over it. +// +// There used to be two copies, and they drifted. The guards added for #2502 +// (a release naming a different author for the same title, and title words +// that match only when scattered through a longer title) went into +// filterRelevant alone. But every production search runs the other copy: +// the scheduler reaches SearchBookWithOutcomes, which projects +// SearchBookWithDebug, and the interactive search calls SearchBookWithDebug +// directly. filterRelevant was reached only by the scheduler's fallback for +// searchers that cannot report outcomes, which in practice means test stubs. +// So #2502's fix was pinned by tests that never ran the production path, and +// automatic search kept grabbing the exact releases it was written to reject. +// One implementation means a guard cannot land in one path and miss the other. +func filterRelevantDetailed(results []newznab.SearchResult, title, author string, aliases []string) ([]newznab.SearchResult, []FilterDebug) { // Strip edition qualifiers ("(German Edition)" etc.) and normalize // smart quotes before tokenizing, so they don't become spurious keywords. title = newznab.NormalizeQueryTitle(title) @@ -847,7 +867,7 @@ func filterRelevant(results []newznab.SearchResult, title, author string, aliase } if len(fullKws) == 0 && len(primaryKws) == 0 && len(authorKws) == 0 { - return results + return results, nil } // Pre-normalize all result titles once. @@ -856,11 +876,17 @@ func filterRelevant(results []newznab.SearchResult, title, author string, aliase normTitles[i] = NormalizeRelease(r.Title) } + drop := func(r newznab.SearchResult, reason string) FilterDebug { + return FilterDebug{Title: r.Title, IndexerName: r.IndexerName, Stage: "relevance", Reason: reason} + } + filtered := make([]newznab.SearchResult, 0, len(results)) + var dropped []FilterDebug for i, r := range results { n := normTitles[i] if conflictingTitleAuthor(r.Title, title, authorTokenSets) || conflictingTitleAuthor(r.Title, primaryTitle(title), authorTokenSets) { + dropped = append(dropped, drop(r, "release names a different author for this title")) continue } // Only insignificant connecting words may separate title keywords. @@ -869,18 +895,26 @@ func filterRelevant(results []newznab.SearchResult, title, author string, aliase // allowFallback=true: each result gets phrase match first, then keyword // fallback if the phrase fails. No batch-level gate. - fullOK := (tryMatch(n, fullKws) || tryMatchElided(n, fullElided, fullKws)) && - identityOK(identity, fullIdentity, fullIdentityElided) - primaryOK := false + fullKeywords := tryMatch(n, fullKws) || tryMatchElided(n, fullElided, fullKws) + fullOK := fullKeywords && identityOK(identity, fullIdentity, fullIdentityElided) + primaryKeywords, primaryOK := false, false if !fullOK && len(primaryKws) > 0 && !sameKws(primaryKws, fullKws) { - primaryOK = (tryMatch(n, primaryKws) || tryMatchElided(n, primaryElided, primaryKws)) && - identityOK(identity, primaryIdentity, primaryIdentityElided) + primaryKeywords = tryMatch(n, primaryKws) || tryMatchElided(n, primaryElided, primaryKws) + primaryOK = primaryKeywords && identityOK(identity, primaryIdentity, primaryIdentityElided) } if fullOK || primaryOK { filtered = append(filtered, r) + continue } + reason := "title/author keywords did not match release name" + if fullKeywords || primaryKeywords { + // The words are all there but not as this title: other words sit + // between them, as "12 Rules for Life" inside "12 More Rules for Life". + reason = "title words appear, but split by words that are not in this title" + } + dropped = append(dropped, drop(r, reason)) } - return filtered + return filtered, dropped } // latinAliasTokenSets builds the candidate author token sets used to anchor a diff --git a/internal/indexer/title_identity_test.go b/internal/indexer/title_identity_test.go index 19dd9262a..2835457aa 100644 --- a/internal/indexer/title_identity_test.go +++ b/internal/indexer/title_identity_test.go @@ -6,6 +6,20 @@ import ( "github.com/vavallee/bindery/internal/indexer/newznab" ) +// relevanceFilters is every entry point to the relevance filter. The #2502 +// guards were once pinned against filterRelevant alone while production ran +// filterRelevantDebug, so every table here runs through both. +var relevanceFilters = []struct { + name string + fn func([]newznab.SearchResult, string, string, []string) []newznab.SearchResult +}{ + {"filterRelevant", filterRelevant}, + {"filterRelevantDebug", func(r []newznab.SearchResult, title, author string, aliases []string) []newznab.SearchResult { + kept, _ := filterRelevantDebug(r, title, author, aliases) + return kept + }}, +} + // These releases were auto-grabbed and imported as three different books. func TestFilterRelevantConflictingBookIdentity(t *testing.T) { cases := []struct{ title, author, release string }{ @@ -13,13 +27,15 @@ func TestFilterRelevantConflictingBookIdentity(t *testing.T) { {"12 Rules for Life", "Jordan B. Peterson", "Beyond Order: 12 More Rules for Life by Jordan B. Peterson EPUB"}, {"Coup d'Etat", "Ben Coes", "Coup D'Etat - Edward Luttwak"}, } - for _, c := range cases { - t.Run(c.title, func(t *testing.T) { - got := filterRelevant([]newznab.SearchResult{{Title: c.release}}, c.title, c.author, nil) - if len(got) != 0 { - t.Fatalf("wrong release accepted: %q for %q by %s", c.release, c.title, c.author) - } - }) + for _, f := range relevanceFilters { + for _, c := range cases { + t.Run(f.name+"/"+c.title, func(t *testing.T) { + got := f.fn([]newznab.SearchResult{{Title: c.release}}, c.title, c.author, nil) + if len(got) != 0 { + t.Fatalf("%s accepted the wrong release: %q for %q by %s", f.name, c.release, c.title, c.author) + } + }) + } } } @@ -53,12 +69,14 @@ func TestFilterRelevantIdentityCompatibility(t *testing.T) { {"Death by Black Hole", "Neil deGrasse Tyson", "Death by Black Hole by Neil deGrasse Tyson EPUB", true}, {"Death by Black Hole", "Neil deGrasse Tyson", "Death by Black Hole by Edward Luttwak EPUB", false}, } - for _, c := range cases { - t.Run(c.release, func(t *testing.T) { - got := filterRelevant(toResults(c.release), c.title, c.author, nil) - if (len(got) == 1) != c.want { - t.Fatalf("filterRelevant(%q, %q, %q): kept=%v, want %v", c.release, c.title, c.author, len(got) == 1, c.want) - } - }) + for _, f := range relevanceFilters { + for _, c := range cases { + t.Run(f.name+"/"+c.release, func(t *testing.T) { + got := f.fn(toResults(c.release), c.title, c.author, nil) + if (len(got) == 1) != c.want { + t.Fatalf("%s(%q, %q, %q): kept=%v, want %v", f.name, c.release, c.title, c.author, len(got) == 1, c.want) + } + }) + } } } From cf6a3071ed6fdeed8ecfaedec3a3f9e410dad7ea Mon Sep 17 00:00:00 2001 From: vavallee Date: Sat, 26 Sep 2026 22:10:20 -0300 Subject: [PATCH 2/3] fix(indexer): match a "Series N: Title" book on its own title A book stored as "Feral Mage 4: The Mining Company Contract" was matched on two readings: the whole name, which needs "feral" and "mage", and primaryTitle's part before the colon, which is the series and position rather than the book. A release named "The Mining Company Contract by Chase Kilgore" satisfies neither, so both automatic and interactive search dropped it. newznab.SeriesPositionTitle returns the part after the colon when the part before it ends in a position: digits, optionally after "#" or before a period, with at least one word ahead. An ordinary "Title: Subtitle" has no such number and is untouched; roman numerals and a bare leading number do not count. The relevance filter tries that reading only after the full and primary readings fail, requires at least two significant words, and applies the same phrase gate and conflicting author check as the others. The query tier gate in the newznab client gets the same reading, so a tier whose results carry only the book's title is not taken for a canned feed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee --- .../series-position-title-relevance.md | 2 + internal/indexer/newznab/client.go | 12 ++- internal/indexer/newznab/words.go | 38 +++++++++ internal/indexer/newznab/words_series_test.go | 46 ++++++++++ internal/indexer/searcher.go | 24 +++++- .../indexer/series_position_title_test.go | 83 +++++++++++++++++++ 6 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 changelog.d/series-position-title-relevance.md create mode 100644 internal/indexer/newznab/words_series_test.go create mode 100644 internal/indexer/series_position_title_test.go diff --git a/changelog.d/series-position-title-relevance.md b/changelog.d/series-position-title-relevance.md new file mode 100644 index 000000000..d8fb19ba5 --- /dev/null +++ b/changelog.d/series-position-title-relevance.md @@ -0,0 +1,2 @@ +### Fixed +- **Books stored as "Series 4: Title" now match releases that name only the title**: a book whose title carries its series and position ahead of a colon, like "Feral Mage 4: The Mining Company Contract", was only ever matched on the whole name or on the series half, so a release called "The Mining Company Contract by Chase Kilgore" was dropped by both automatic and interactive search. The book's own title is now tried as well when the part before the colon ends in a number ("Feral Mage 4", "Book 8", "Discworld #3"), after the stricter readings fail, and it has to be at least two meaningful words, keep its words in order, and not name a different author. An ordinary "Title: Subtitle" is unaffected. Thanks OMegaliterosMalakas for the report. diff --git a/internal/indexer/newznab/client.go b/internal/indexer/newznab/client.go index 77d94e5b1..e39b90147 100644 --- a/internal/indexer/newznab/client.go +++ b/internal/indexer/newznab/client.go @@ -662,9 +662,15 @@ func titleHasRelevantResult(queryTitle string, results []SearchResult) bool { // // Gated on a non-empty word list: SigWordsElided returns nil for a title // with no apostrophe, and an empty list means "nothing checkable" (true) - // upstream — letting it through here would accept junk for every title. - if elided := SigWordsElided(queryTitle); len(elided) > 0 { - return wordsPresentInAll(elided, combined) + // upstream; letting it through here would accept junk for every title. + if elided := SigWordsElided(queryTitle); len(elided) > 0 && wordsPresentInAll(elided, combined) { + return true + } + // A "Series N: Title" name is judged on the book's own title as well, + // since release names carry the title without the series position. Same + // two word floor as the relevance filter's fallback reading. + if series := SigWords(SeriesPositionTitle(queryTitle)); len(series) >= 2 && wordsPresentInAll(series, combined) { + return true } return false } diff --git a/internal/indexer/newznab/words.go b/internal/indexer/newznab/words.go index 0a4c66463..5370c1d4c 100644 --- a/internal/indexer/newznab/words.go +++ b/internal/indexer/newznab/words.go @@ -175,3 +175,41 @@ var queryUmlautReplacer = strings.NewReplacer( func TransliterateQuery(s string) string { return queryUmlautReplacer.Replace(norm.NFC.String(s)) } + +// SeriesPositionTitle returns the book's own title from a "Series N: Title" +// name, or "" when title does not have that shape. +// +// Providers often store a series entry with the series and its position ahead +// of the colon ("Feral Mage 4: The Mining Company Contract"), while release +// names carry only the book's own title ("The Mining Company Contract by Chase +// Kilgore"). The full title then needs "feral" and "mage", and the part before +// the colon is the series, not the book, so neither reading can match a +// release that names the book correctly. +// +// The shape is recognised only when the part before the colon ends in a +// position: a number, optionally prefixed with "#" or followed by a period +// ("Feral Mage 4", "Book 8", "Discworld #3", "Vol. 2."), with at least one word +// ahead of it. An ordinary "Title: Subtitle" has no such number, so it is left +// to the existing readings. The returned title is a fallback reading for the +// relevance checks and must never replace the stricter ones. +func SeriesPositionTitle(title string) string { + i := strings.Index(title, ":") + if i <= 0 { + return "" + } + head := strings.Fields(title[:i]) + tail := strings.TrimSpace(title[i+1:]) + if len(head) < 2 || tail == "" { + return "" + } + pos := strings.TrimSuffix(strings.TrimPrefix(head[len(head)-1], "#"), ".") + if pos == "" { + return "" + } + for _, r := range pos { + if r < '0' || r > '9' { + return "" + } + } + return tail +} diff --git a/internal/indexer/newznab/words_series_test.go b/internal/indexer/newznab/words_series_test.go new file mode 100644 index 000000000..8f95f74d2 --- /dev/null +++ b/internal/indexer/newznab/words_series_test.go @@ -0,0 +1,46 @@ +package newznab + +import "testing" + +func TestSeriesPositionTitle(t *testing.T) { + cases := []struct{ in, want string }{ + {"Feral Mage 4: The Mining Company Contract", "The Mining Company Contract"}, + {"Cradle Book 8: Wintersteel", "Wintersteel"}, + {"Discworld #3: Equal Rites", "Equal Rites"}, + {"Dungeon Crawler Carl 07: This Inevitable Ruin", "This Inevitable Ruin"}, + {"The Wandering Inn Vol. 2.: Volume Two", "Volume Two"}, + // No series position before the colon: an ordinary subtitle, left alone. + {"12 Rules for Life: An Antidote to Chaos", ""}, + {"Dune: Messiah", ""}, + {"The Lord of the Rings: The Fellowship of the Ring", ""}, + // A bare number is a title, not a series with a position. + {"1984: A Novel", ""}, + // Roman numerals are ambiguous with words ("I"), so they do not count. + {"Rocky IV: The Novel", ""}, + {"Feral Mage 4:", ""}, + {"No colon here 4", ""}, + {"", ""}, + } + for _, c := range cases { + if got := SeriesPositionTitle(c.in); got != c.want { + t.Errorf("SeriesPositionTitle(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestTitleHasRelevantResult_SeriesPositionTitle covers the tier gate. A tier +// whose only results name the book's own title is relevant; without this it is +// read as a canned feed and the cascade walks on past it. +func TestTitleHasRelevantResult_SeriesPositionTitle(t *testing.T) { + q := "Feral Mage 4: The Mining Company Contract" + if !titleHasRelevantResult(q, []SearchResult{{Title: "The Mining Company Contract by Chase Kilgore [ENG / EPUB]"}}) { + t.Error("a release naming the book's own title should make the tier relevant") + } + if titleHasRelevantResult(q, []SearchResult{{Title: "Wulfe Untamed: Feral Warriors, Book 8 by Pamela Palmer"}}) { + t.Error("a release naming neither the series nor the book's own title must not") + } + // A one word book title is below the floor, so it does not open the gate. + if titleHasRelevantResult("Cradle Book 8: Wintersteel", []SearchResult{{Title: "Wintersteel by Someone"}}) { + t.Error("a one word series position title must not be enough on its own") + } +} diff --git a/internal/indexer/searcher.go b/internal/indexer/searcher.go index 0d1f764cc..344d53d2d 100644 --- a/internal/indexer/searcher.go +++ b/internal/indexer/searcher.go @@ -829,6 +829,18 @@ func filterRelevantDetailed(results []newznab.SearchResult, title, author string fullIdentityElided := titleIdentityWords(newznab.ElideApostrophes(title)) primaryIdentityElided := titleIdentityWords(newznab.ElideApostrophes(primaryTitle(title))) authorKws := newznab.SigWords(author) + // A "Series N: Title" name also gets the book's own title as a reading, + // because releases name the book, not its series position ("The Mining + // Company Contract" for "Feral Mage 4: The Mining Company Contract"). It + // needs two significant words so a one word title cannot turn the author's + // whole catalogue into matches, and it is tried only after the full and + // primary readings both fail. See newznab.SeriesPositionTitle. + seriesTitle := newznab.SeriesPositionTitle(title) + seriesKws := newznab.SigWords(seriesTitle) + if len(seriesKws) < 2 || sameKws(seriesKws, fullKws) || sameKws(seriesKws, primaryKws) { + seriesTitle, seriesKws = "", nil + } + seriesIdentity := titleIdentityWords(seriesTitle) authorTokenSets := latinAliasTokenSets(author, aliases) @@ -885,7 +897,8 @@ func filterRelevantDetailed(results []newznab.SearchResult, title, author string for i, r := range results { n := normTitles[i] if conflictingTitleAuthor(r.Title, title, authorTokenSets) || - conflictingTitleAuthor(r.Title, primaryTitle(title), authorTokenSets) { + conflictingTitleAuthor(r.Title, primaryTitle(title), authorTokenSets) || + (seriesTitle != "" && conflictingTitleAuthor(r.Title, seriesTitle, authorTokenSets)) { dropped = append(dropped, drop(r, "release names a different author for this title")) continue } @@ -902,12 +915,17 @@ func filterRelevantDetailed(results []newznab.SearchResult, title, author string primaryKeywords = tryMatch(n, primaryKws) || tryMatchElided(n, primaryElided, primaryKws) primaryOK = primaryKeywords && identityOK(identity, primaryIdentity, primaryIdentityElided) } - if fullOK || primaryOK { + seriesKeywords, seriesOK := false, false + if !fullOK && !primaryOK && len(seriesKws) > 0 { + seriesKeywords = tryMatch(n, seriesKws) + seriesOK = seriesKeywords && (len(seriesIdentity) < 2 || ContainsPhrase(identity, seriesIdentity)) + } + if fullOK || primaryOK || seriesOK { filtered = append(filtered, r) continue } reason := "title/author keywords did not match release name" - if fullKeywords || primaryKeywords { + if fullKeywords || primaryKeywords || seriesKeywords { // The words are all there but not as this title: other words sit // between them, as "12 Rules for Life" inside "12 More Rules for Life". reason = "title words appear, but split by words that are not in this title" diff --git a/internal/indexer/series_position_title_test.go b/internal/indexer/series_position_title_test.go new file mode 100644 index 000000000..eeafaa12f --- /dev/null +++ b/internal/indexer/series_position_title_test.go @@ -0,0 +1,83 @@ +package indexer + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/vavallee/bindery/internal/indexer/newznab" + "github.com/vavallee/bindery/internal/models" +) + +// A book stored as "Series N: Title" was only ever matched on the whole name or +// on the series half, so a release naming the book itself was dropped. This +// release is the one from the Discord report. +func TestFilterRelevant_SeriesPositionTitle(t *testing.T) { + const title, author = "Feral Mage 4: The Mining Company Contract", "Chase Kilgore" + cases := []struct { + release string + want bool + }{ + {"The Mining Company Contract by Chase Kilgore [ENG / EPUB] [VIP]", true}, + {"Chase Kilgore - The Mining Company Contract (epub)", true}, + // Already matched before this change, and still does. + {"Chase Kilgore - Feral Mage 4 - The Mining Company Contract (epub)", true}, + // The author is still required. + {"The Mining Company Contract by Someone Else [ENG / EPUB]", false}, + {"The Mining Company Contract [ENG / EPUB]", true}, + // A different book by the same author. + {"The Silver Company Contract by Chase Kilgore [ENG / EPUB]", false}, + // Words present but split by words the title does not have. + {"The Mining Company Breach of Contract by Chase Kilgore", false}, + {"Wulfe Untamed: Feral Warriors, Book 8 by Pamela Palmer [ENG / MP3]", false}, + } + for _, f := range relevanceFilters { + for _, c := range cases { + t.Run(f.name+"/"+c.release, func(t *testing.T) { + got := f.fn([]newznab.SearchResult{{Title: c.release}}, title, author, nil) + if (len(got) == 1) != c.want { + t.Fatalf("%s kept=%v, want %v for %q", f.name, len(got) == 1, c.want, c.release) + } + }) + } + } +} + +// A one word book title after a series position is below the floor, so the +// fallback reading does not open up every release by the author that contains +// that word. +func TestFilterRelevant_SeriesPositionTitleNeedsTwoWords(t *testing.T) { + got := filterRelevant([]newznab.SearchResult{{Title: "Wintersteel Anthology by Will Wight"}}, + "Cradle Book 8: Wintersteel", "Will Wight", nil) + for _, r := range got { + t.Errorf("kept %q through a one word series position title", r.Title) + } +} + +// Drives the production entry points so the reading is proven on the paths +// that search and grab, not only on the filter function. +func TestSeriesPositionTitle_ReachesProductionEntrypoints(t *testing.T) { + const right = "The Mining Company Contract by Chase Kilgore [ENG / EPUB] [VIP]" + const wrong = "Wulfe Untamed: Feral Warriors, Book 8 by Pamela Palmer [ENG / MP3]" + feed := feedOf(right, wrong) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/xml") + _, _ = w.Write([]byte(feed)) + })) + defer srv.Close() + + idxs := []models.Indexer{{ID: 1, Name: "test", URL: srv.URL, Enabled: true, Categories: []int{7020}}} + crit := MatchCriteria{Title: "Feral Mage 4: The Mining Company Contract", Author: "Chase Kilgore"} + ctx := context.Background() + debugged, _ := newTestSearcher().SearchBookWithDebug(ctx, idxs, crit) + outcomes, _ := newTestSearcher().SearchBookWithOutcomes(ctx, idxs, crit) + for name, got := range map[string][]string{ + "SearchBookWithDebug": resultTitles(debugged), + "SearchBookWithOutcomes": resultTitles(outcomes), + } { + if len(got) != 1 || got[0] != right { + t.Errorf("%s kept %v, want only %q", name, got, right) + } + } +} From cc9d92bd3cf0d24da50e1acc1b24fc334fbb65fb Mon Sep 17 00:00:00 2001 From: vavallee Date: Fri, 2 Oct 2026 01:58:50 -0300 Subject: [PATCH 3/3] fix(indexer): treat a zero padded number as the same number in title matching Putting the title identity gate on the production path (previous commits) started dropping correct releases that pad a position: "Vol 07" for "Volume 7", "Shield Hero 07" for "Shield Hero 7", "Book 07" for "Book 7". The gate compared numbers as literal words. keywordPattern now renders an all digit keyword as 0* followed by the number without its padding, so the phrase, keyword and in order matchers and the identity gate (ContainsPhrase) all accept "7", "07" and "007" as one number, in either direction. The callers' word boundaries keep "7" from matching "17" or "70". volumeNumberAgrees already ignored padding through sameVolumeNumber. conflictingTitleAuthor compares whole normalized strings, so it would have read "Book 07 - Other Author" as a different title and skipped the attribution. foldVolumeMarkers becomes foldTitleTokens and also strips padding, so that release is still caught. A digit and a spelled out number ("7" and "Seven") stay different; that case is pinned as a known gap. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee --- ...-relevance-filter-single-implementation.md | 2 +- internal/indexer/release.go | 12 +++ internal/indexer/title_identity.go | 30 +++--- internal/indexer/volume_marker_test.go | 93 +++++++++++++++++-- 4 files changed, 118 insertions(+), 19 deletions(-) diff --git a/changelog.d/2812-relevance-filter-single-implementation.md b/changelog.d/2812-relevance-filter-single-implementation.md index 583f15e1f..5690d68b0 100644 --- a/changelog.d/2812-relevance-filter-single-implementation.md +++ b/changelog.d/2812-relevance-filter-single-implementation.md @@ -1,2 +1,2 @@ ### Fixed -- **Automatic search no longer grabs a longer book that contains your title** (#2812): the check added in v1.35 that stops "The Power of Writing It Down" passing for "Power Down", or "Beyond Order: 12 More Rules for Life" for "12 Rules for Life", only ran on a fallback path, so automatic search and the interactive Search button never used it. The same check also turns away a release for the wrong volume when both spell it the same way, such as "Volume 16" for a book titled "Volume 17". The relevance filter is now one piece of code shared by every search, so a check cannot reach one path and miss the other again. The interactive search panel also says which check dropped a release: a different author for this title, or the title's words with other words or numbers among them. +- **Automatic search no longer grabs a longer book that contains your title** (#2812): the check added in v1.35 that stops "The Power of Writing It Down" passing for "Power Down", or "Beyond Order: 12 More Rules for Life" for "12 Rules for Life", only ran on a fallback path, so automatic search and the interactive Search button never used it. The same check also turns away a release for the wrong volume when both spell it the same way, such as "Volume 16" for a book titled "Volume 17". A number padded with zeros still counts as the same number, so "Vol 07" or "Book 07" matches a book numbered 7, while "17" or "70" does not. The relevance filter is now one piece of code shared by every search, so a check cannot reach one path and miss the other again. The interactive search panel also says which check dropped a release: a different author for this title, or the title's words with other words or numbers among them. diff --git a/internal/indexer/release.go b/internal/indexer/release.go index affd6c651..83703ce58 100644 --- a/internal/indexer/release.go +++ b/internal/indexer/release.go @@ -190,6 +190,12 @@ const volumeMarkerPattern = `(?:vol(?:ume)?s?)` // "vol" still cannot match inside "volcano" or "revolution". Which VOLUME a // cross spelling match names is checked separately, see volumeNumberAgrees. // +// A keyword made only of digits matches the same number with any zero +// padding: "7", "07" and "007" are one number, because release names pad +// series and volume positions ("Vol 07", "Book 07") while titles usually do +// not. The callers' word boundaries keep "7" from matching inside "17" or +// "70". This is the same rule sameVolumeNumber applies to volume numbers. +// // Every other keyword is QuoteMeta'd with the umlaut flex, exactly as before. // lower is the keyword lowercased for the marker lookup only; the fragment // keeps the caller's own casing so the umlaut flex sees what it always saw. @@ -197,6 +203,12 @@ func keywordPattern(kw, lower string) string { if newznab.IsVolumeMarker(lower) { return volumeMarkerPattern } + if isAllDigits(kw) { + if n := strings.TrimLeft(kw, "0"); n != "" { + return `0*` + n + } + return `0+` + } return umlautFlexRegex(regexp.QuoteMeta(kw)) } diff --git a/internal/indexer/title_identity.go b/internal/indexer/title_identity.go index 8e4c0efe8..73e9917cb 100644 --- a/internal/indexer/title_identity.go +++ b/internal/indexer/title_identity.go @@ -22,20 +22,21 @@ func titleIdentityWords(s string) []string { // the left side names the requested title. Title-only releases remain valid; // release metadata and narrator credits are not treated as authors. // -// Both sides are compared with volume markers folded to one spelling -// (foldVolumeMarkers), because the keyword matchers accept "Vol 17" for a -// "Volume 17" title: without the fold, "Title Vol 17 - Other Author" would be -// accepted on its title while its attribution went unread. +// Both sides are compared with volume markers folded to one spelling and +// numbers stripped of zero padding (foldTitleTokens), because the keyword +// matchers accept "Vol 07" for a "Volume 7" title: without the fold, +// "Title Vol 07 - Other Author" would be accepted on its title while its +// attribution went unread. func conflictingTitleAuthor(release, title string, authorSets [][]string) bool { - normalizedTitle := foldVolumeMarkers(NormalizeRelease(title)) - normalizedRelease := foldVolumeMarkers(NormalizeRelease(release)) + normalizedTitle := foldTitleTokens(NormalizeRelease(title)) + normalizedRelease := foldTitleTokens(NormalizeRelease(release)) var attribution string if after, ok := strings.CutPrefix(normalizedRelease, normalizedTitle+" by "); ok { attribution = after } else { for _, separator := range []string{" - ", " – ", " — "} { before, after, ok := strings.Cut(release, separator) - if ok && foldVolumeMarkers(NormalizeRelease(before)) == normalizedTitle { + if ok && foldTitleTokens(NormalizeRelease(before)) == normalizedTitle { attribution = NormalizeRelease(after) break } @@ -68,16 +69,23 @@ func conflictingTitleAuthor(release, title string, authorSets [][]string) bool { return knownAuthor } -// foldVolumeMarkers rewrites every volume marker spelling in a -// NormalizeRelease string to "vol", so two strings that differ only in how -// they spell the marker compare equal. Used for equality tests only. -func foldVolumeMarkers(s string) string { +// foldTitleTokens rewrites every volume marker spelling in a +// NormalizeRelease string to "vol" and strips zero padding from every number +// ("07" to "7", "00" to "0"), so two strings that differ only in how they +// spell the marker or pad a number compare equal. This mirrors what +// keywordPattern accepts. Used for equality tests only. +func foldTitleTokens(s string) string { toks := strings.Fields(s) changed := false for i, tok := range toks { if newznab.IsVolumeMarker(tok) && tok != "vol" { toks[i] = "vol" changed = true + } else if len(tok) > 1 && tok[0] == '0' && isAllDigits(tok) { + if toks[i] = strings.TrimLeft(tok, "0"); toks[i] == "" { + toks[i] = "0" + } + changed = true } } if !changed { diff --git a/internal/indexer/volume_marker_test.go b/internal/indexer/volume_marker_test.go index 98dbafe56..58e2a684d 100644 --- a/internal/indexer/volume_marker_test.go +++ b/internal/indexer/volume_marker_test.go @@ -66,16 +66,13 @@ func TestFilterRelevantVolumeMarkerSpelling(t *testing.T) { want: true, }, { - // KNOWN GAP, not a goal: this is the right release and it is - // dropped. The title identity gate compares "7" and "07" as - // different words (the volume guard itself ignores zero padding). - // Production search kept it until #2812 put the identity gate on - // that path too; flip this to true when the gate learns to - // ignore leading zeros. + // The title identity gate used to compare "7" and "07" as + // different words. A number now matches with any zero padding + // (keywordPattern), the same as the volume guard. name: "zero padded volume number in the release", title: "The Rising of the Shield Hero Volume 7", release: "Aneko Yusagi - The Rising of the Shield Hero Vol 07 - epub", - want: false, + want: true, }, { name: "mid title format qualifier is not a keyword", @@ -146,3 +143,85 @@ func TestFilterRelevantVolumeMarkerSpelling(t *testing.T) { }) } } + +// TestFilterRelevantNumberPadding pins that a number in a title matches the +// same number with zero padding in a release, and the other way round, and +// that the padding rule cannot make one number match another. Release names +// pad series and volume positions ("Vol 07", "Book 07"); titles usually do +// not. Every case runs through both filter wrappers, which share one +// implementation. +func TestFilterRelevantNumberPadding(t *testing.T) { + cases := []struct { + name, title, author, release string + want bool + }{ + {"Volume 7 title, Vol 07 release", "The Rising of the Shield Hero Volume 7", "Aneko Yusagi", + "Aneko Yusagi - The Rising of the Shield Hero Vol 07 - epub", true}, + {"bare 7 title, 07 release", "The Rising of the Shield Hero 7", "Aneko Yusagi", + "Aneko Yusagi - The Rising of the Shield Hero 07 - epub", true}, + {"Book 7 title, Book 07 release", "Dungeon Crawler Carl Book 7", "Matt Dinniman", + "Matt Dinniman - Dungeon Crawler Carl Book 07 - This Inevitable Ruin", true}, + {"padded title, bare release", "Dungeon Crawler Carl Book 07", "Matt Dinniman", + "Matt Dinniman - Dungeon Crawler Carl Book 7 - This Inevitable Ruin", true}, + {"triple padded release", "The Rising of the Shield Hero 7", "Aneko Yusagi", + "Aneko Yusagi - The Rising of the Shield Hero 007 - epub", true}, + + // Padding never makes one number match another. + {"Volume 16 release for Volume 17 title", "The Rising of the Shield Hero Volume 17", "Aneko Yusagi", + "Aneko Yusagi - The Rising of the Shield Hero Volume 16 - epub", false}, + {"7 does not match 17", "The Rising of the Shield Hero 7", "Aneko Yusagi", + "Aneko Yusagi - The Rising of the Shield Hero 17 - epub", false}, + {"7 does not match 70", "The Rising of the Shield Hero 7", "Aneko Yusagi", + "Aneko Yusagi - The Rising of the Shield Hero 70 - epub", false}, + {"Book 7 does not match Book 017", "Dungeon Crawler Carl Book 7", "Matt Dinniman", + "Matt Dinniman - Dungeon Crawler Carl Book 017", false}, + {"padded release for another author is still caught", "Dungeon Crawler Carl Book 7", "Matt Dinniman", + "Dungeon Crawler Carl Book 07 - Some Other Writer", false}, + + // KNOWN GAP, not a goal: this is the right release and it is + // dropped. A digit and a spelled out number ("7" and "Seven") are + // different words to the title identity gate, and only zero padding + // is treated as equal. Production search kept it until #2812 put the + // identity gate on that path. + {"7 does not match Seven", "The 7 Habits of Highly Effective People", "Stephen R. Covey", + "Stephen R. Covey - The Seven Habits of Highly Effective People", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + in := []newznab.SearchResult{{Title: tc.release}} + gotPlain := len(filterRelevant(in, tc.title, tc.author, nil)) == 1 + kept, _ := filterRelevantDebug(in, tc.title, tc.author, nil) + gotDebug := len(kept) == 1 + if gotPlain != tc.want { + t.Errorf("filterRelevant(%q, %q) kept=%v, want %v", tc.title, tc.release, gotPlain, tc.want) + } + if gotDebug != tc.want { + t.Errorf("filterRelevantDebug(%q, %q) kept=%v, want %v", tc.title, tc.release, gotDebug, tc.want) + } + }) + } +} + +// TestContainsPhraseNumberPadding pins the number rule in keywordPattern at +// the matcher level, where the word boundaries do the work. +func TestContainsPhraseNumberPadding(t *testing.T) { + cases := []struct { + haystack string + phrase []string + want bool + }{ + {"book 07", []string{"book", "7"}, true}, + {"book 7", []string{"book", "07"}, true}, + {"book 007", []string{"book", "7"}, true}, + {"book 0", []string{"book", "00"}, true}, + {"book 17", []string{"book", "7"}, false}, + {"book 70", []string{"book", "7"}, false}, + {"book 107", []string{"book", "07"}, false}, + {"book 0", []string{"book", "7"}, false}, + } + for _, c := range cases { + if got := ContainsPhrase(c.haystack, c.phrase); got != c.want { + t.Errorf("ContainsPhrase(%q, %q) = %v, want %v", c.haystack, c.phrase, got, c.want) + } + } +}