From 1b17fe179abdef8fe2733efecc4f13a07a7f8d57 Mon Sep 17 00:00:00 2001 From: CaliBrain Date: Fri, 11 Sep 2026 22:26:28 -0400 Subject: [PATCH] fix(irc): rank a surname-only result as partial, not wrong (#1332) (#1334) "David Petrie" as "D. Petrie", then ranked the answer by the full name to recover the precision the surname gave up. The two halves disagreed. author_affinity needs two agreeing tokens before it calls a name the same person, so "Petrie" - the name on the filenames a surname search exists to reach - matched one and came back AUTHOR_MISMATCH. It therefore sorted below "Unknown" and level with "Gordon Petrie", a different author who merely shares the surname. The widened query pulled those rows in and the ranker buried them. Falling short of agreement is now separated from disagreeing with it. A name whose every token fits the one asked for is an abbreviation of it and ranks AUTHOR_PARTIAL, between agreement and "no author reported"; a name carrying a token that fits nothing still ranks AUTHOR_MISMATCH. Nothing that agreed before changes tier - "Homer"/"Homer Simpson" is still a match, since the extra token must not demote a mononym that already met its one-token requirement - so Prowlarr's #1293 ordering is unchanged except that a tracker listing a bare surname stops being read as the wrong author. Measured on the issue's own case, wanted "David Petrie": before: D Petrie, Unknown, Petrie, Gordon Petrie after: D Petrie, Petrie, Unknown, Gordon Petrie Second fix, same release: a book with no title posted the surname on its own. _build_query fell back to book.search_title or book.title, which is empty on exactly the path where the plan has no title variants, so the line reaching the channel was "@search Petrie" - not a search for anything, and the kind of bare over-broad post is_available refuses unaddressed queries to avoid. It now returns "" and the existing "No search query could be built" guard takes it. Tested with make python-lint, python-format, python-dead-code, python-typecheck and python-test. --- shelfmark/core/author_match.py | 34 ++++++++++++++++++++----- shelfmark/release_sources/irc/source.py | 7 +++++ tests/irc/test_search_query.py | 16 ++++++++++++ tests/prowlarr/test_author_matching.py | 28 +++++++++++++++++--- 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/shelfmark/core/author_match.py b/shelfmark/core/author_match.py index d8e82153..ceac8796 100644 --- a/shelfmark/core/author_match.py +++ b/shelfmark/core/author_match.py @@ -15,8 +15,9 @@ _AUTHOR_NOISE_TOKENS = frozenset( # Ordering tiers for author agreement between the requested book and what an # indexer reported. Lower sorts first. AUTHOR_MATCH = 0 -AUTHOR_UNKNOWN = 1 -AUTHOR_MISMATCH = 2 +AUTHOR_PARTIAL = 1 +AUTHOR_UNKNOWN = 2 +AUTHOR_MISMATCH = 3 # A mononym ("Homer") can only ever agree on one token; a longer name needs a # given name and a surname to agree before it counts as the same person. @@ -45,9 +46,15 @@ def author_affinity(wanted: object, offered: object) -> int: agree, while a transliteration ("Dostoevsky"/"Dostoyevsky") is merely sorted last instead of being hidden. - Three-way on purpose: an indexer that reports no author at all must not sort - below one that reports a wrong author, so "no metadata" ranks between - agreement and disagreement rather than counting as either. + Graded, not binary, because the ways of falling short are not equally bad. + An indexer that reports no author at all must not sort below one that reports + a wrong author, so "no metadata" ranks between agreement and disagreement. And + a name that merely says *less* than the one asked for is not evidence of a + different person: "Petrie" contradicts nothing about "David Petrie", while + "Gordon Petrie" does. That gap matters most where a source is searched by + surname alone (#1331) - the filenames such a search is meant to reach are + exactly the ones filed under a bare surname, and ranking them as wrong put + them below every result that named someone else entirely. """ wanted_tokens = _author_tokens(wanted) offered_tokens = _author_tokens(offered) @@ -63,7 +70,22 @@ def author_affinity(wanted: object, offered: object) -> int: ) ) required = min(_AUTHOR_TOKENS_REQUIRED, len(wanted_tokens)) - return AUTHOR_MATCH if matched >= required else AUTHOR_MISMATCH + if matched >= required: + return AUTHOR_MATCH + + # Too little agreement to call it the same person, so the question is whether + # what was offered *disagrees*. A name every token of which fits the wanted + # name is an abbreviation of it; one carrying a token that fits nothing is a + # different name that happens to share a surname. + if all( + any( + _author_tokens_compatible(wanted_token, offered_token) for wanted_token in wanted_tokens + ) + for offered_token in offered_tokens + ): + return AUTHOR_PARTIAL + + return AUTHOR_MISMATCH def search_surname(author: object) -> str: diff --git a/shelfmark/release_sources/irc/source.py b/shelfmark/release_sources/irc/source.py index 3820eee3..571b7354 100644 --- a/shelfmark/release_sources/irc/source.py +++ b/shelfmark/release_sources/irc/source.py @@ -419,9 +419,16 @@ class IRCReleaseSource(ReleaseSource): carries `author=""` on purpose (search_plan.py:246), and so does a manual query, so reading `plan.author` here would append a surname to searches that deliberately have none. + + Returns "" without a title, so the caller reports "no query" rather than + posting one. A surname on its own is not a search: `@search Petrie` asks + the bot for every Petrie on the channel, and a bare over-broad line is the + kind of post `is_available` refuses queries to avoid being banned for. """ variant = plan.title_variants[0] if plan.title_variants else None title = variant.title if variant else (book.search_title or book.title) + if not title: + return "" author = variant.author if variant else plan.author parts = [part for part in (title, search_surname(author)) if part] return " ".join(parts) diff --git a/tests/irc/test_search_query.py b/tests/irc/test_search_query.py index 8021a371..4a1458f8 100644 --- a/tests/irc/test_search_query.py +++ b/tests/irc/test_search_query.py @@ -89,6 +89,17 @@ class TestQueryPostedToChannel: assert IRCReleaseSource()._build_query(book, build_release_search_plan(book)) == "Beowulf" + def test_a_book_with_no_title_is_not_searched_by_surname_alone(self): + # "@search Petrie" asks the bot for every Petrie it holds. It is not a + # search for anything, and a bare over-broad line posted to a public + # channel is what gets the nick banned - so no query is built at all. + book = BookMetadata(provider="hardcover", provider_id="1", title="", authors=["D. Petrie"]) + plan = build_release_search_plan(book) + + assert plan.title_variants == [] + assert plan.author == "D. Petrie" + assert IRCReleaseSource()._build_query(book, plan) == "" + class TestAuthorOrdersTheAnswer: def test_requested_author_leads_and_the_rest_stay_visible(self): @@ -99,6 +110,10 @@ class TestAuthorOrdersTheAnswer: # What the parser writes when a filename has no " - " separator; it # must not sort below a result that named a different author. _release("Unknown"), + # The shape the surname query exists to reach: the channel filed this + # under the surname alone, so it agrees with everything known about + # "David Petrie" and contradicts none of it. + _release("Petrie"), _release("D Petrie"), ] @@ -106,6 +121,7 @@ class TestAuthorOrdersTheAnswer: assert [release.extra["author"] for release in ranked] == [ "D Petrie", + "Petrie", "Unknown", "Gordon Petrie", ] diff --git a/tests/prowlarr/test_author_matching.py b/tests/prowlarr/test_author_matching.py index d782a4e9..adb6523b 100644 --- a/tests/prowlarr/test_author_matching.py +++ b/tests/prowlarr/test_author_matching.py @@ -11,6 +11,7 @@ import pytest from shelfmark.core.author_match import ( AUTHOR_MATCH, AUTHOR_MISMATCH, + AUTHOR_PARTIAL, AUTHOR_UNKNOWN, author_affinity, ) @@ -62,12 +63,33 @@ class TestAuthorAffinity: # An indexer that reports no author must not sort below one that reports # the wrong author, so this tier sits between the two. assert author_affinity(wanted, offered) == AUTHOR_UNKNOWN - assert AUTHOR_MATCH < AUTHOR_UNKNOWN < AUTHOR_MISMATCH + assert AUTHOR_MATCH < AUTHOR_PARTIAL < AUTHOR_UNKNOWN < AUTHOR_MISMATCH - def test_a_surname_alone_is_not_enough_for_a_full_name(self): - # "Ferriss" appearing under some other given name is a different person. + def test_another_given_name_under_the_same_surname_is_a_different_person(self): assert author_affinity("Timothy Ferriss", "Bruce Ferriss") == AUTHOR_MISMATCH + @pytest.mark.parametrize( + ("wanted", "offered"), + [ + ("Timothy Ferriss", "Ferriss"), + ("David Petrie", "Petrie"), + ("Ursula K. Le Guin", "Guin"), + ("Timothy Ferriss", "T."), + ], + ) + def test_a_name_that_says_less_is_not_a_name_that_disagrees(self, wanted, offered): + # Every token offered fits the name asked for; there is just not enough of + # it to confirm the person. Weaker evidence than full agreement, stronger + # than none - and far from the wrong-author tier these used to land in, + # which is where a surname-only search put most of its own answer (#1331). + assert author_affinity(wanted, offered) == AUTHOR_PARTIAL + assert AUTHOR_MATCH < AUTHOR_PARTIAL < AUTHOR_UNKNOWN < AUTHOR_MISMATCH + + def test_a_mononym_still_agrees_with_a_longer_name_that_contains_it(self): + # The one-token requirement for a mononym is unchanged: the extra token + # must not turn an agreement into a partial. + assert author_affinity("Homer", "Homer Simpson") == AUTHOR_MATCH + class _EnrichedIndexerClient: """Stands in for a Prowlarr with MyAnonamouse enabled."""