Stop the search chain treating an empty answer as an answer

`/serp/search` returned zero results and the explanation "All backends are
rate-limited." - a claim with nothing behind it. Seen once against the
live deployment, then not reproducible, which is itself the problem: the
message described a diagnosis rather than what happened.

Two causes, both fixed here.

A backend that returned an empty list without raising ended the fallback
chain and was recorded as a success. Brave does that whenever its block
page lacks the word "suspicious", and Bing whenever no result item yields
a title and href - so one quiet backend could stop the chain before a
working one was ever tried. This is the same bug class fixed for
DuckDuckGo in d341a7a, which raises on empty; the other two never got the
same treatment. Fixing it in the chain rather than in each scraper covers
every backend, including any added later, and leaves the single-backend
endpoints free to report an honest empty result.

An empty answer still counts neither for nor against the backend, as an
exception-raised empty already did: recording it as a success would reset
a backend part-way to tripping its breaker, which is how a soft-blocking
backend stays in rotation forever.

The error now names what each backend did - "no results", "failed
(cause)", or "skipped (circuit open)". Those are different problems that
lead somewhere different, and collapsing them into one invented diagnosis
sent whoever read it looking for throttling that may not have existed:

No results for query 'widget' (DuckDuckGo: no results; Brave Search:
failed (connection reset by peer); Bing: skipped (Circuit open for
'Bing'; retry after 60s))

Mutation testing showed `search`'s own fallback text was unreachable once
_search_one always explains itself - which is how the rate-limit guess
survived unnoticed. Rather than delete the defensive branch, there is now
a test that reaches it, so it has to say something defensible too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSNSYnceqvdVz7Csis4K9e

CONTRIBUTING.md

@@ -30,7 +30,7 @@ ruff check .   # same lint CI runs
 pytest
 ```
 
-264 tests, a few seconds, fully offline. All outbound HTTP is mocked with
+271 tests, a few seconds, fully offline. All outbound HTTP is mocked with
 `respx`; the Playwright-driven scrapers (Bing, Brave, Google Scholar,
 Google Patents search) run against a real headless Chromium but navigate to
 local fixture HTML instead of the live sites — see `tests/helpers.py` for

services.py

@@ -202,42 +202,66 @@ class SearchService:
 
     async def _search_one(self, q: str, n_results: int) -> tuple[str, list[dict], Optional[str]]:
         """Try DDG, then Brave, then Bing for a single query; stop at the
-        first success."""
+        first backend that actually returns something.
+
+        A backend that answers with an empty list has not answered the
+        question - Brave does that whenever its block page lacks the word
+        "suspicious", and Bing whenever no result item yields a title and
+        href - so the chain continues rather than reporting success with
+        zero results.
+
+        When nothing produces results, the error names what each backend
+        did. "Failed", "returned nothing" and "never asked" are different
+        problems that lead somewhere different, and collapsing them into
+        one guess is how this previously reported rate limiting it had no
+        evidence for.
+        """
         backends = [
             ("DuckDuckGo", lambda: query_ddg_search(q, n_results)),
             ("Brave Search", lambda: query_brave_search(self.browser, q, n_results)),
             ("Bing", lambda: query_bing_search(self.browser, q, n_results)),
         ]
-        last_error: Optional[Exception] = None
+        outcomes: list[str] = []
         for name, call in backends:
             try:
                 self._breaker.before_call(name)
             except CircuitOpenError as e:
                 logger.info(f"Skipping {name} for query `{q}`: {e}")
-                last_error = e
+                outcomes.append(f"{name}: skipped ({e})")
                 continue
 
             try:
                 logger.info(f"Querying {name} with query: `{q}`")
                 result = await call()
-                self._breaker.record_success(name)
-                return q, result, None
-            except EmptyResultsError as e:
+            except EmptyResultsError:
                 # Zero results is ambiguous - no hits, or a soft block that
-                # parsed to nothing. Move on to the next backend, but don't
-                # hold it against this one: the breaker is shared across
-                # every request, so counting it would let a few
-                # unusual-but-legitimate queries disable the primary backend
-                # for everyone. A hard block normally raises a real error,
-                # which is handled below.
+                # parsed to nothing. Move on, but don't hold it against
+                # this backend: the breaker is shared across every request,
+                # so counting it would let a few unusual-but-legitimate
+                # queries disable the primary backend for everyone. A hard
+                # block normally raises a real error, handled below.
                 logger.info(f"{name} returned no results for query `{q}`")
-                last_error = e
+                outcomes.append(f"{name}: no results")
+                continue
             except Exception as e:
                 logger.error(f"Failed to query {name} with query `{q}`: {e}")
                 self._breaker.record_failure(name)
-                last_error = e
+                outcomes.append(f"{name}: failed ({e})")
+                continue
+
+            if result:
+                self._breaker.record_success(name)
+                return q, result, None
+
+            # Answered, but with nothing. Same ambiguity as the exception
+            # above, so it counts neither for nor against the backend -
+            # recording it as a success would reset one part-way to
+            # tripping its breaker, which is how a soft-blocking backend
+            # stays in rotation forever.
+            logger.info(f"{name} returned an empty result set for query `{q}`")
+            outcomes.append(f"{name}: no results")
 
-        return q, [], f"All backends failed for query '{q}': {last_error}"
+        return q, [], f"No results for query '{q}' ({'; '.join(outcomes)})"
 
     async def search(self, params: SerpQuery) -> SerpResults:
         """Search every query across the full backend fallback chain."""
@@ -252,9 +276,13 @@ class SearchService:
                 errors.append(err)
 
         if len(results) == 0:
+            # _search_one always explains itself when it produces nothing,
+            # so `errors` is populated here; the fallback text is a plain
+            # statement of fact rather than the invented rate-limit
+            # diagnosis it replaced.
             return SerpResults(
                 results=[],
-                error="; ".join(errors) if errors else "All backends are rate-limited.")
+                error="; ".join(errors) if errors else "No results found.")
 
         return SerpResults(results=results, error="; ".join(errors) if errors else None)
 

tests/test_search_service.py

@@ -245,7 +245,11 @@ async def test_reports_an_error_when_every_backend_fails(monkeypatch):
     q, results, err = await make_service()._search_one("widget", 5)
 
     assert results == []
-    assert "All backends failed for query 'widget'" in err
+    assert "widget" in err
+    # Each backend is named with what it did, rather than collapsed into a
+    # single "all backends failed" with only the last error attached.
+    for backend in ("DuckDuckGo", "Brave Search", "Bing"):
+        assert f"{backend}: failed" in err
     assert "blocked" in err
 
 
@@ -408,3 +412,148 @@ async def test_a_scholar_block_is_reported_as_such_to_the_caller(monkeypatch):
     assert result.results == []
     assert "blocked" in result.error.lower()
     assert "timeout" not in result.error.lower()
+
+
+# ------------------- an empty answer is not an answer -------------------
+# A backend returning [] without raising used to end the fallback chain and
+# be reported as success. Brave does that whenever its block page lacks the
+# word "suspicious", and Bing whenever no result item yields a title+href.
+# The caller then got zero results and the invented explanation "All
+# backends are rate-limited." - a guess, not evidence. Observed once
+# against the live deployment.
+
+
+async def test_an_empty_backend_result_falls_through_to_the_next_backend(monkeypatch):
+    calls = []
+
+    async def empty_ddg(q, n):
+        calls.append("ddg")
+        return []
+
+    async def fake_brave(browser, q, n):
+        calls.append("brave")
+        return [{"title": "from brave"}]
+
+    monkeypatch.setattr(services_module, "query_ddg_search", empty_ddg)
+    monkeypatch.setattr(services_module, "query_brave_search", fake_brave)
+
+    _q, results, err = await make_service()._search_one("widget", 5)
+
+    assert calls == ["ddg", "brave"], "an empty result ended the chain"
+    assert results == [{"title": "from brave"}]
+    assert err is None
+
+
+async def test_every_backend_empty_reports_that_and_not_rate_limiting(monkeypatch):
+    """The message has to describe what happened. "Rate-limited" was
+    asserted without evidence, and sent whoever read it looking for a
+    throttling problem that may not exist.
+    """
+    async def empty(*args):
+        return []
+
+    for name in ("query_ddg_search", "query_brave_search", "query_bing_search"):
+        monkeypatch.setattr(services_module, name, empty)
+
+    _q, results, err = await make_service()._search_one("widget", 5)
+
+    assert results == []
+    assert "rate-limit" not in err.lower()
+    for backend in ("DuckDuckGo", "Brave Search", "Bing"):
+        assert backend in err, f"{backend}'s outcome is missing from {err!r}"
+
+
+async def test_the_error_distinguishes_failing_from_returning_nothing(monkeypatch):
+    """"Failed" and "answered with nothing" are different problems and lead
+    somewhere different - one is an outage, the other may just be an
+    obscure query."""
+    async def failing_ddg(q, n):
+        raise RuntimeError("connection reset")
+
+    async def empty_brave(browser, q, n):
+        return []
+
+    async def failing_bing(browser, q, n):
+        raise RuntimeError("connection reset")
+
+    monkeypatch.setattr(services_module, "query_ddg_search", failing_ddg)
+    monkeypatch.setattr(services_module, "query_brave_search", empty_brave)
+    monkeypatch.setattr(services_module, "query_bing_search", failing_bing)
+
+    _q, _results, err = await make_service()._search_one("widget", 5)
+
+    assert "connection reset" in err, "a real failure must still surface its cause"
+    assert "no results" in err.lower(), "an empty answer must be described as such"
+
+
+async def test_a_skipped_backend_is_reported_as_skipped_not_failed(monkeypatch):
+    """A backend whose circuit is open was never asked, so saying it failed
+    for this query is wrong."""
+    async def empty(*args):
+        return []
+
+    for name in ("query_ddg_search", "query_brave_search", "query_bing_search"):
+        monkeypatch.setattr(services_module, name, empty)
+
+    breaker = CircuitBreaker(failure_threshold=1, cooldown_seconds=60)
+    breaker.record_failure("Bing")
+    service = make_service(breaker=breaker)
+
+    _q, _results, err = await service._search_one("widget", 5)
+
+    assert "skipped" in err.lower()
+
+
+async def test_an_empty_answer_does_not_clear_an_accumulating_failure_count(monkeypatch):
+    """Empty is ambiguous, so it is evidence neither way. Recording it as a
+    success would reset a backend part-way to tripping its breaker, which
+    is how a soft-blocking backend stays in rotation forever.
+    """
+    breaker = CircuitBreaker(failure_threshold=2, cooldown_seconds=60)
+    breaker.record_failure("DuckDuckGo")  # one strike already
+
+    async def empty_ddg(q, n):
+        return []
+
+    monkeypatch.setattr(services_module, "query_ddg_search", empty_ddg)
+    monkeypatch.setattr(
+        services_module, "query_brave_search", lambda b, q, n: _ok([{"title": "brave"}]))
+
+    await make_service(breaker=breaker)._search_one("widget", 5)
+
+    assert breaker._states["DuckDuckGo"].failure_count == 1, (
+        "an empty result cleared DuckDuckGo's failure count")
+
+
+async def test_search_never_invents_a_rate_limit_explanation(monkeypatch):
+    """The endpoint-level guard for the same thing."""
+    async def empty(*args):
+        return []
+
+    for name in ("query_ddg_search", "query_brave_search", "query_bing_search"):
+        monkeypatch.setattr(services_module, name, empty)
+
+    result = await make_service().search(SerpQuery(queries=["a", "b"]))
+
+    assert result.results == []
+    assert result.error and "rate-limit" not in result.error.lower()
+
+
+async def test_the_aggregate_fallback_message_states_a_fact_not_a_diagnosis():
+    """`_search_one` always explains itself, so `search`'s fallback text is
+    unreachable in practice - which is exactly how the rate-limit guess
+    survived unnoticed for so long. Reach it directly so the defensive
+    branch says something defensible too.
+    """
+    service = make_service()
+
+    async def silent_search_one(q, n_results):
+        return q, [], None
+
+    service._search_one = silent_search_one
+
+    result = await service.search(SerpQuery(queries=["a"]))
+
+    assert result.results == []
+    assert "rate-limit" not in result.error.lower()
+    assert result.error, "an empty result set must still carry an explanation"