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."""