mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-04 22:05:45 +01:00
"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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
]
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user