fix(hardcover): send the field count Hardcover's Book search requires (#1224)

Advanced title search, advanced title+author search, and the title
typeahead returned zero results every time, and the sort fallback added
in #1183 blamed the sort value for it.

Hardcover turns the `fields` search parameter into Typesense's
`query_by`
but keeps `num_typos` and `query_by_weights` as fixed-length presets per
query_type. For query_type=Book the preset expects exactly five fields,
so a shorter list is not searched loosely - the whole search is rejected
with a null results body. Confirmed against the live API: 1, 2, 3, 4 and
6 fields are all rejected, only 5 works, and weights must match
one-for-one when sent. Every Book-type list we sent was the wrong length
- the title typeahead and advanced title search sent 2, title+author
sent 3.

- Send BOOK_SEARCH_FIELDS (the full five) for every narrowed Book search
  and express the intent through weights instead. Weights only bias
  ranking - a field weighted 0 still matches - so a title search now
  ranks titles first rather than restricting to them. That is the
  closest behaviour Hardcover still allows, and there is no client-side
  filter to restore the old precision.
- Pin the field and weight counts in tests, since the failure mode is a
  silent zero results rather than an error.

The sort fallback from #1183 also misread these rejections:

- Select the `error` field on every search and log Hardcover's own
  explanation. The reason is only ever in that sibling field, so a
  rejection surfaced as "returned no result body" with nothing to act
  on. Reading it is what made the field-count rule findable.
- Drop `sort` entirely on the retry instead of sending an empty string.
  An empty sort is a value like any other and can be rejected too.
- Arm the 900s sticky window only after the sortless retry succeeds. It
  was armed before the retry and never rolled back, so one rejected
  typeahead disabled sorting process-wide for 15 minutes whatever the
  actual cause.

Verified against the live Hardcover API: advanced title search 0 -> 84
results, title+author 0 -> 139, title typeahead 0 -> 84 with the exact
title top. 2566 unit tests pass; ruff, basedpyright and vulture clean.

Refs #1183. The sort_by regression #1183 was written for is gone from
Hardcover's side - every sort value it rejected, including the one in
the report, is accepted again today. Two plain-search rejections in that
report (fields=None) remain unexplained: they could not be reproduced
under any per_page, page depth, sort value or query shape, and are most
likely transient upstream. They now self-report the reason if they
recur.

shelfmark/metadata_providers/hardcover.py

@@ -318,6 +318,7 @@ query SearchFieldOptions(
         fields: $fields,
         weights: $weights
     ) {
+        error
         results
     }
 }
@@ -536,13 +537,19 @@ SORT_MAPPING: dict[SortOrder, str] = {
     SortOrder.OLDEST: "release_year:asc",
 }
 
-# Mapping from abstract search type to Hardcover fields parameter
-SEARCH_TYPE_FIELDS: dict[SearchType, str] = {
-    SearchType.GENERAL: "title,isbns,series_names,author_names,alternative_titles",
-    SearchType.TITLE: "title,alternative_titles",
-    SearchType.AUTHOR: "author_names",
-    # ISBN is handled separately via search_by_isbn()
-}
+# `fields` becomes Typesense's `query_by`, but Hardcover keeps `num_typos` and
+# `query_by_weights` as fixed-length presets per query_type. Passing a different
+# number of fields than the preset expects makes Typesense reject the whole search,
+# complaining that the number of num_typos values does not match the number of
+# query_by fields. So a Book search may only ever narrow to *these five* names --
+# a shorter list is rejected outright rather than searched, and any weights sent
+# alongside must match one-for-one.
+# Weights only bias ranking: a field weighted 0 still matches, so `fields` can no
+# longer restrict which fields a Book query looks at.
+BOOK_SEARCH_FIELDS = "title,alternative_titles,author_names,series_names,isbns"
+BOOK_SEARCH_FIELD_COUNT = 5
+BOOK_TITLE_WEIGHTS = "5,1,0,0,0"
+BOOK_TITLE_AUTHOR_WEIGHTS = "5,1,3,0,0"
 
 SERIES_SEARCH_FIELDS = "name,books,author_name"
 SERIES_SEARCH_WEIGHTS = "2,1,1"
@@ -550,22 +557,28 @@ SERIES_SEARCH_SORT = "_text_match:desc,readers_count:desc"
 AUTHOR_SUGGESTION_FIELDS = "name,name_personal,alternate_names"
 AUTHOR_SUGGESTION_WEIGHTS = "4,3,2"
 AUTHOR_SUGGESTION_SORT = "_text_match:desc,books_count:desc"
-TITLE_SUGGESTION_FIELDS = "title,alternative_titles"
-TITLE_SUGGESTION_WEIGHTS = "5,2"
+TITLE_SUGGESTION_FIELDS = BOOK_SEARCH_FIELDS
+TITLE_SUGGESTION_WEIGHTS = "5,2,0,0,0"
 TITLE_SUGGESTION_SORT = "_text_match:desc,users_count:desc"
 
 # Hardcover forwards `sort` to Typesense's `sort_by` and rejects the whole search
 # if it does not like the value -- an unknown field, a bare field name with no
 # direction, more than three keys. A rejected search comes back as HTTP 200 with
 # no GraphQL errors and a null `results` body, which is otherwise indistinguishable
-# from "nothing matched". An empty sort is always accepted, so fall back to it and
-# keep the fallback sticky for a while rather than paying for a doomed request on
-# every search.
-SORT_FALLBACK = ""
+# from "nothing matched"; the reason only shows up in the sibling `error` field,
+# so every search asks for it. Dropping `sort` from the request is the one shape
+# Hardcover always accepts -- an empty string is a value like any other and has
+# been rejected too -- so retry that way and keep the fallback sticky for a while
+# rather than paying for a doomed request on every search.
 SORT_FALLBACK_TTL = 900.0
 _sort_fallback_until = 0.0
 
 
+def _without_sort(variables: dict[str, Any]) -> dict[str, Any]:
+    """Drop `sort` entirely so Hardcover applies its own default ordering."""
+    return {key: value for key, value in variables.items() if key != "sort"}
+
+
 def _search_payload_rejected(result: dict[str, Any] | None) -> bool:
     """Report whether Hardcover answered a search with a null results body.
 
@@ -580,6 +593,17 @@ def _search_payload_rejected(result: dict[str, Any] | None) -> bool:
     return root["results"] is None
 
 
+def _search_rejection_reason(result: dict[str, Any] | None) -> str:
+    """Return Hardcover's explanation for a rejected search, if it sent one."""
+    if not isinstance(result, dict):
+        return ""
+    root = result.get("search", result)
+    if not isinstance(root, dict):
+        return ""
+    error = root.get("error")
+    return error.strip() if isinstance(error, str) else ""
+
+
 def _combine_headline_description(headline: str | None, description: str | None) -> str | None:
     """Combine headline (tagline) and description into a single description."""
     if headline and description:
@@ -1012,13 +1036,15 @@ class HardcoverProvider(MetadataProvider):
         """Build search query, fields, and weights based on provided values.
 
         Returns (query, fields, weights) tuple. Fields/weights are None for general search.
+        A narrowed search still sends all of BOOK_SEARCH_FIELDS -- Hardcover rejects a
+        shorter list outright -- and leans on the weights to rank the wanted field first.
         """
         if author and not title and not series:
             return author, None, None
         if title and not author and not series:
-            return title, "title,alternative_titles", "5,1"
+            return title, BOOK_SEARCH_FIELDS, BOOK_TITLE_WEIGHTS
         if author and title and not series:
-            return f"{title} {author}", "title,alternative_titles,author_names", "5,1,3"
+            return f"{title} {author}", BOOK_SEARCH_FIELDS, BOOK_TITLE_AUTHOR_WEIGHTS
         return default_query, None, None
 
     def _detect_list_url(self, query: str) -> tuple[str | None, str] | None:
@@ -2384,6 +2410,7 @@ class HardcoverProvider(MetadataProvider):
             graphql_query = """
             query SearchBooks($query: String!, $limit: Int!, $page: Int!, $sort: String, $fields: String, $weights: String) {
                 search(query: $query, query_type: "Book", per_page: $limit, page: $page, sort: $sort, fields: $fields, weights: $weights) {
+                    error
                     results
                 }
             }
@@ -2392,6 +2419,7 @@ class HardcoverProvider(MetadataProvider):
             graphql_query = """
             query SearchBooks($query: String!, $limit: Int!, $page: Int!, $sort: String) {
                 search(query: $query, query_type: "Book", per_page: $limit, page: $page, sort: $sort) {
+                    error
                     results
                 }
             }
@@ -2690,33 +2718,42 @@ class HardcoverProvider(MetadataProvider):
 
         sort = variables.get("sort")
         if sort and time.monotonic() < _sort_fallback_until:
-            variables = {**variables, "sort": SORT_FALLBACK}
+            variables = _without_sort(variables)
             sort = None
 
         result = self._execute_query(query, variables)
         if not _search_payload_rejected(result):
             return result
 
+        reason = _search_rejection_reason(result)
         if not sort:
             logger.error(
-                "Hardcover rejected this search (query_type=%s, fields=%s) and returned "
-                "no result body",
+                "Hardcover rejected this search (query_type=%s, fields=%s): %s",
                 variables.get("queryType", "Book"),
                 variables.get("fields"),
+                reason or "no error message",
+            )
+            return None
+
+        retry = self._execute_query(query, _without_sort(variables))
+        if _search_payload_rejected(retry):
+            # The sort was not the culprit, so leave sorting alone for other searches.
+            logger.error(
+                "Hardcover rejected this search (query_type=%s, fields=%s) with and without "
+                "a sort order: %s",
+                variables.get("queryType", "Book"),
+                variables.get("fields"),
+                _search_rejection_reason(retry) or reason or "no error message",
             )
             return None
 
         logger.warning(
-            "Hardcover rejected sort '%s'; retrying searches without a sort order for %ss",
+            "Hardcover rejected sort '%s' (%s); dropping the sort order from searches for %ss",
             sort,
+            reason or "no error message",
             int(SORT_FALLBACK_TTL),
         )
         _sort_fallback_until = time.monotonic() + SORT_FALLBACK_TTL
-
-        retry = self._execute_query(query, {**variables, "sort": SORT_FALLBACK})
-        if _search_payload_rejected(retry):
-            logger.error("Hardcover rejected this search even without a sort order")
-            return None
         return retry
 
     def _parse_search_result(self, item: dict) -> BookMetadata | None:

tests/metadata/test_hardcover_field_options.py

@@ -1,4 +1,8 @@
-from shelfmark.metadata_providers.hardcover import HardcoverProvider
+from shelfmark.metadata_providers.hardcover import (
+    TITLE_SUGGESTION_FIELDS,
+    TITLE_SUGGESTION_WEIGHTS,
+    HardcoverProvider,
+)
 
 
 class TestHardcoverFieldOptions:
@@ -130,8 +134,10 @@ class TestHardcoverFieldOptions:
             "limit": 7,
             "page": 1,
             "sort": "_text_match:desc,users_count:desc",
-            "fields": "title,alternative_titles",
-            "weights": "5,2",
+            # Hardcover rejects a Book search that narrows to fewer fields than its
+            # preset expects, so the typeahead sends the full list and leans on weights.
+            "fields": TITLE_SUGGESTION_FIELDS,
+            "weights": TITLE_SUGGESTION_WEIGHTS,
         }
 
     def test_get_search_field_options_skips_short_text_queries(self):

tests/metadata/test_hardcover_search_fields.py

@@ -0,0 +1,82 @@
+"""Guards on the shape of Hardcover's `fields`/`weights` search parameters.
+
+Hardcover turns `fields` into Typesense's `query_by` but keeps `num_typos` and
+`query_by_weights` as fixed-length presets per query_type. A field list of the
+wrong length is not searched loosely -- the whole search is rejected with a null
+results body, which used to surface as "0 results". These tests pin the counts
+so a narrower field list cannot silently ship again.
+"""
+
+import pytest
+
+from shelfmark.metadata_providers.hardcover import (
+    AUTHOR_SUGGESTION_FIELDS,
+    AUTHOR_SUGGESTION_WEIGHTS,
+    BOOK_SEARCH_FIELD_COUNT,
+    BOOK_SEARCH_FIELDS,
+    BOOK_TITLE_AUTHOR_WEIGHTS,
+    BOOK_TITLE_WEIGHTS,
+    SERIES_SEARCH_FIELDS,
+    SERIES_SEARCH_WEIGHTS,
+    TITLE_SUGGESTION_FIELDS,
+    TITLE_SUGGESTION_WEIGHTS,
+    HardcoverProvider,
+)
+
+
+def _count(value: str) -> int:
+    return len([part for part in value.split(",") if part.strip()])
+
+
+class TestBookSearchFieldCounts:
+    def test_book_field_list_matches_hardcovers_preset_length(self):
+        assert _count(BOOK_SEARCH_FIELDS) == BOOK_SEARCH_FIELD_COUNT
+
+    @pytest.mark.parametrize(
+        ("label", "weights"),
+        [
+            ("title", BOOK_TITLE_WEIGHTS),
+            ("title+author", BOOK_TITLE_AUTHOR_WEIGHTS),
+            ("title typeahead", TITLE_SUGGESTION_WEIGHTS),
+        ],
+    )
+    def test_book_weights_line_up_with_the_field_list(self, label, weights):
+        assert _count(weights) == BOOK_SEARCH_FIELD_COUNT, label
+
+    def test_title_typeahead_uses_the_full_book_field_list(self):
+        assert TITLE_SUGGESTION_FIELDS == BOOK_SEARCH_FIELDS
+
+
+class TestNonBookSearchFieldCounts:
+    @pytest.mark.parametrize(
+        ("fields", "weights"),
+        [
+            (AUTHOR_SUGGESTION_FIELDS, AUTHOR_SUGGESTION_WEIGHTS),
+            (SERIES_SEARCH_FIELDS, SERIES_SEARCH_WEIGHTS),
+        ],
+    )
+    def test_weights_line_up_with_their_field_list(self, fields, weights):
+        assert _count(fields) == _count(weights)
+
+
+class TestBuildSearchParams:
+    @pytest.mark.parametrize(
+        ("author", "title", "series"),
+        [
+            ("", "Dune", ""),
+            ("Herbert", "Dune", ""),
+            ("Herbert", "", ""),
+            ("", "", ""),
+        ],
+    )
+    def test_every_branch_sends_a_usable_field_weight_pair(self, author, title, series):
+        provider = HardcoverProvider(api_key="test-token")
+
+        _query, fields, weights = provider._build_search_params("dune", author, title, series)
+
+        if fields is None:
+            # No override: Hardcover applies its own preset, so weights must be absent too.
+            assert weights is None
+            return
+        assert _count(fields) == BOOK_SEARCH_FIELD_COUNT
+        assert _count(weights) == BOOK_SEARCH_FIELD_COUNT

tests/metadata/test_hardcover_sort_fallback.py

@@ -1,3 +1,4 @@
+import logging
 from typing import Any
 
 import pytest
@@ -6,9 +7,10 @@ from shelfmark.metadata_providers import MetadataSearchOptions
 from shelfmark.metadata_providers.hardcover import HardcoverProvider
 
 # Hardcover answers a rejected search with HTTP 200, no GraphQL errors, and a
-# null results body. A search that genuinely matched nothing still returns a
-# results object with found: 0.
-REJECTED = {"search": {"results": None}}
+# null results body; the reason shows up in the sibling error field. A search
+# that genuinely matched nothing still returns a results object with found: 0.
+REJECTED = {"search": {"error": "Parameter `sort_by` is malformed.", "results": None}}
+REJECTED_SILENTLY = {"search": {"results": None}}
 EMPTY = {"search": {"results": {"hits": [], "found": 0}}}
 ONE_HIT = {"search": {"results": {"hits": [{"document": {"id": 7, "title": "Dune"}}], "found": 1}}}
 
@@ -19,6 +21,29 @@ def _reset_sort_fallback(monkeypatch):
     monkeypatch.setattr("shelfmark.metadata_providers.hardcover._sort_fallback_until", 0.0)
 
 
+@pytest.fixture
+def hardcover_logs():
+    """Collect Hardcover log messages.
+
+    The provider's logger is built outside the standard hierarchy, so its
+    records never reach the root handler that caplog installs.
+    """
+    from shelfmark.metadata_providers import hardcover
+
+    messages: list[str] = []
+
+    class _Capture(logging.Handler):
+        def emit(self, record: logging.LogRecord) -> None:
+            messages.append(record.getMessage())
+
+    handler = _Capture()
+    hardcover.logger.addHandler(handler)
+    try:
+        yield messages
+    finally:
+        hardcover.logger.removeHandler(handler)
+
+
 def _reject_sorted(calls: list[dict[str, Any]], *, success=ONE_HIT):
     """Build an _execute_query stand-in that rejects any request carrying a sort."""
 
@@ -38,7 +63,9 @@ class TestHardcoverSortFallback:
         result = provider._execute_search_query("query", {"query": "dune", "sort": "relevance"})
 
         assert result == ONE_HIT
-        assert [call["sort"] for call in calls] == ["relevance", ""]
+        # The retry drops sort entirely -- an empty sort is a value Hardcover can reject too.
+        assert [call.get("sort") for call in calls] == ["relevance", None]
+        assert "sort" not in calls[1]
 
     def test_treats_an_empty_result_set_as_success(self, monkeypatch):
         provider = HardcoverProvider(api_key="test-token")
@@ -64,6 +91,24 @@ class TestHardcoverSortFallback:
         assert result is None
         assert len(calls) == 2
 
+    def test_keeps_sorting_when_the_sort_was_not_the_culprit(self, monkeypatch):
+        """A rejection that survives dropping the sort must not disable sorting globally."""
+        provider = HardcoverProvider(api_key="test-token")
+        calls: list[dict[str, Any]] = []
+        monkeypatch.setattr(
+            provider, "_execute_query", lambda query, variables: calls.append(variables) or REJECTED
+        )
+
+        provider._execute_search_query("query", {"query": "dune", "sort": "rating:desc"})
+        provider._execute_search_query("query", {"query": "hyperion", "sort": "rating:desc"})
+
+        assert [call.get("sort") for call in calls] == [
+            "rating:desc",
+            None,
+            "rating:desc",
+            None,
+        ]
+
     def test_reports_failure_for_an_unsorted_rejection(self, monkeypatch):
         provider = HardcoverProvider(api_key="test-token")
         calls: list[dict[str, Any]] = []
@@ -76,6 +121,24 @@ class TestHardcoverSortFallback:
         assert result is None
         assert len(calls) == 1
 
+    def test_logs_the_reason_hardcover_gave(self, monkeypatch, hardcover_logs):
+        provider = HardcoverProvider(api_key="test-token")
+        monkeypatch.setattr(provider, "_execute_query", lambda query, variables: REJECTED)
+
+        provider._execute_search_query("query", {"query": "dune", "sort": ""})
+
+        assert any("Parameter `sort_by` is malformed." in message for message in hardcover_logs)
+
+    def test_falls_back_to_a_placeholder_when_hardcover_says_nothing(
+        self, monkeypatch, hardcover_logs
+    ):
+        provider = HardcoverProvider(api_key="test-token")
+        monkeypatch.setattr(provider, "_execute_query", lambda query, variables: REJECTED_SILENTLY)
+
+        provider._execute_search_query("query", {"query": "dune", "sort": ""})
+
+        assert any("no error message" in message for message in hardcover_logs)
+
     def test_skips_the_doomed_request_on_later_searches(self, monkeypatch):
         provider = HardcoverProvider(api_key="test-token")
         calls: list[dict[str, Any]] = []
@@ -84,7 +147,7 @@ class TestHardcoverSortFallback:
         provider._execute_search_query("query", {"query": "dune", "sort": "relevance"})
         provider._execute_search_query("query", {"query": "hyperion", "sort": "relevance"})
 
-        assert [call["sort"] for call in calls] == ["relevance", "", ""]
+        assert [call.get("sort") for call in calls] == ["relevance", None, None]
 
     def test_search_returns_results_despite_a_rejected_sort(self, monkeypatch):
         provider = HardcoverProvider(api_key="test-token")
@@ -97,7 +160,7 @@ class TestHardcoverSortFallback:
 
         assert result.total_found == 1
         assert [book.title for book in result.books] == ["Dune"]
-        assert [call["sort"] for call in calls] == ["_text_match:desc,users_count:desc", ""]
+        assert [call.get("sort") for call in calls] == ["_text_match:desc,users_count:desc", None]
 
 
 class TestSearchPayloadRejection:
@@ -105,9 +168,19 @@ class TestSearchPayloadRejection:
         from shelfmark.metadata_providers.hardcover import _search_payload_rejected
 
         assert _search_payload_rejected(REJECTED) is True
+        assert _search_payload_rejected(REJECTED_SILENTLY) is True
         assert _search_payload_rejected(EMPTY) is False
         assert _search_payload_rejected(ONE_HIT) is False
         assert _search_payload_rejected(None) is False
         assert _search_payload_rejected({}) is False
         # Non-search payloads (list lookups, book fetches) must pass through.
         assert _search_payload_rejected({"series": [{"id": 1}]}) is False
+
+    def test_reads_the_error_hardcover_attached(self):
+        from shelfmark.metadata_providers.hardcover import _search_rejection_reason
+
+        assert _search_rejection_reason(REJECTED) == "Parameter `sort_by` is malformed."
+        assert _search_rejection_reason(REJECTED_SILENTLY) == ""
+        assert _search_rejection_reason(EMPTY) == ""
+        assert _search_rejection_reason(None) == ""
+        assert _search_rejection_reason({"search": {"error": None, "results": None}}) == ""