Housekeeping: lint in CI, client lifecycle, and a truer breaker signal

Independent cleanups from the review, grouped because each is small.

A3 - utils.log_gathered_exceptions imported SerpQuery from serp.py purely
to read one attribute, so the lowest layer in the codebase depended on the
domain layer. It now takes any iterable of query strings, which also makes
it usable from the patent and OPS paths.

A4 - the shared httpx client was created at import and never closed; the
lifespan tore down the browser but left its connection pool open. It is
now closed on shutdown, including when Playwright failed to start. Two
lifespan tests cover this, and the existing one no longer closes the real
client out from under the rest of the session.

A5 - the circuit breaker could not tell "this query had no hits" from
"this backend is refusing us". query_ddg_search raises on an empty result
because a soft block can parse to zero results, but _search_one counted
every exception against the backend - and the breaker is a process-wide
singleton, so three unusual-but-legitimate queries in a row disabled the
primary backend for every user for a full cooldown. Empty results now get
their own exception type: still falls through to the next backend, no
longer counted as evidence the backend is unhealthy. A hard block normally
raises a real error, which still trips it.

A6 - the OPS fallback in search_patents awaited one query at a time while
the rest of the endpoint gathered. That path exists for when Google
Patents is unavailable, which is exactly when a request can least afford N
sequential round-trips to the slower backend. Gathered now.

A7 - ruff wired into both workflows with a deliberately small rule set.
Its first pass found the duplicate APIRouter import shadowing the other,
four unused imports, an f-string with no placeholders, seven exception
re-raises losing their cause chain, and two ambiguous zips - all fixed
rather than ignored. Also: playwright_open_page now closes its context
even when closing the page raises, instead of leaking it.

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

.github/workflows/deploy-to-hf.yml

@@ -24,6 +24,9 @@ jobs:
       - name: Install Playwright's Chromium
         run: playwright install --with-deps chromium
 
+      - name: Lint
+        run: ruff check .
+
       - name: Run tests
         run: pytest
 

.github/workflows/test.yml

@@ -29,5 +29,8 @@ jobs:
       - name: Install Playwright's Chromium
         run: playwright install --with-deps chromium
 
+      - name: Lint
+        run: ruff check .
+
       - name: Run tests
         run: pytest

CONTRIBUTING.md

@@ -26,10 +26,11 @@ dependency update broke this."
 ## Running tests
 
 ```bash
+ruff check .   # same lint CI runs
 pytest
 ```
 
-97 tests, a few seconds, fully offline. All outbound HTTP is mocked with
+199 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
@@ -77,6 +78,22 @@ changes should too:
    confirm the test actually catches it, then revert. This caught a real
    gap once already: a circuit breaker test that looked correct was
    actually vacuous until this step exposed it.
+
+   Applying this systematically is worth a pass of its own, and one such
+   pass is where most of the tests added since came from. Line coverage
+   was 93% and hid the problem completely: the suite killed five of six
+   mutants aimed at pure functions, and one of seven aimed at the I/O
+   boundary. Deleting the entire API-key middleware left it green, as did
+   replacing the Bing search URL with a constant. The lesson worth keeping
+   is *where* to point this technique - at the code that touches the
+   outside world, not the code that's easy to test.
+
+   A useful heuristic fell out of it: if a property has no behavioural
+   signature at all, no black-box test can pin it. `==` and
+   `secrets.compare_digest` return the same answer for every input and
+   differ only in timing, so that one test asserts structurally that the
+   comparison goes through `compare_digest`. Reach for that only when
+   there is genuinely nothing observable to assert on.
 6. **Watch for shared mutable state across tests.** A few characterization
    tests in this codebase turned out to be recording real state into a
    module-level singleton (a circuit breaker) with nothing resetting it
@@ -93,8 +110,13 @@ changes should too:
 
 ## CI / deployment
 
-- **`.github/workflows/test.yml`** runs the full suite on every pull
-  request against `main`, so a change gets feedback before merge.
+- **`.github/workflows/test.yml`** runs `ruff check .` and then the full
+  suite on every pull request against `main`, so a change gets feedback
+  before merge. The lint rule set (see `pyproject.toml`) is deliberately
+  small - pyflakes-equivalent checks plus bugbear - because the first pass
+  over this repo found a duplicate import shadowing another, four unused
+  imports and an f-string with no placeholders. Widen it when the team
+  wants more rather than starting broad and accumulating ignores.
 - **`.github/workflows/deploy-to-hf.yml`** runs the same suite again
   (`test` job) on every push to `main`, and only then (`needs: test`)
   pushes the repo to the Hugging Face Space (`deploy` job) with

README.md

@@ -19,13 +19,15 @@ pinned: true
 ```
 pip install -r requirements.txt -r requirements-dev.txt
 playwright install chromium   # if not already present on the machine
+ruff check .
 pytest
 ```
 
 The suite mocks all outbound HTTP (via `respx`) and drives the Playwright-based
-scrapers against local fixture HTML instead of live search engines - see
-`tests/helpers.py` for how - so it runs fully offline and doesn't depend on,
-or risk tripping, any site's anti-bot measures.
+scrapers against local fixture HTML loaded straight into a real headless
+Chromium - see the `page_factory` fixture in `tests/conftest.py` - so it runs
+fully offline and doesn't depend on, or risk tripping, any site's anti-bot
+measures.
 
 See [`CONTRIBUTING.md`](CONTRIBUTING.md) for the development methodology
 (TDD workflow, dependency pinning policy, CI/deployment) behind this project.

app.py

@@ -4,18 +4,18 @@ import os
 import secrets
 from contextlib import asynccontextmanager
 from typing import Annotated, Optional
-from fastapi import APIRouter, FastAPI, HTTPException, Request
+from fastapi import FastAPI, HTTPException, Request
 from fastapi.responses import JSONResponse
 from fastapi.routing import APIRouter
 import httpx
 from httpx import HTTPStatusError
 from pydantic import BaseModel, Field, StringConstraints
-from playwright.async_api import async_playwright, Browser, BrowserContext, Page
+from playwright.async_api import async_playwright, Browser
 import uvicorn
 
 from circuit_breaker import CircuitBreaker, CircuitOpenError
 from scrap import PatentScrapBulkResponse, PatentScrapResult, scrap_patent_async, scrap_patent_bulk_async
-from serp import PATENT_ID_CORE, SerpQuery, SerpResults, query_arxiv, query_bing_search, query_brave_search, query_ddg_search, query_google_patents, query_google_scholar
+from serp import EmptyResultsError, PATENT_ID_CORE, SerpQuery, SerpResults, query_arxiv, query_bing_search, query_brave_search, query_ddg_search, query_google_patents, query_google_scholar
 from ops import OPSBulkResponse, OPSNotConfigured, ops_scrap_patent, ops_scrap_patent_bulk, ops_search, token_manager as ops_token_manager
 from utils import log_gathered_exceptions
 from mcp_server import mount_mcp_server
@@ -36,7 +36,9 @@ logging.basicConfig(
 playwright = None
 pw_browser: Optional[Browser] = None
 
-# httpx client
+# httpx client. Constructed at import so module-level handlers can close
+# over it, but its lifetime is owned by `api_lifespan`, which closes it on
+# shutdown alongside the browser.
 httpx_client = httpx.AsyncClient(timeout=30, limits=httpx.Limits(
     max_connections=30, max_keepalive_connections=20))
 
@@ -89,6 +91,7 @@ async def api_lifespan(app: FastAPI):
         await pw_browser.close()
     if playwright:
         await playwright.stop()
+    await httpx_client.aclose()
 
 app = FastAPI(lifespan=api_lifespan, docs_url="/",
               title="SERPent", description=_load_docs())
@@ -149,7 +152,7 @@ async def _run_serp_queries(fn, params: SerpQuery, context: str) -> SerpResults:
     """
     results = await asyncio.gather(
         *[fn(q, params.n_results) for q in params.queries], return_exceptions=True)
-    log_gathered_exceptions(results, context, params)
+    log_gathered_exceptions(results, context, params.queries)
     return _shape_serp_results(results)
 
 
@@ -178,18 +181,30 @@ async def search_patents(params: SerpQuery) -> SerpResults:
     """
     logging.info(f"Searching Google Patents for queries: {params.queries}")
     results = await asyncio.gather(*[query_google_patents(pw_browser, q, params.n_results) for q in params.queries], return_exceptions=True)
-    log_gathered_exceptions(results, "google patent search", params)
+    log_gathered_exceptions(results, "google patent search", params.queries)
 
     # Fall back to OPS for queries that errored or returned no results.
+    # Gathered rather than awaited one at a time: this path exists for when
+    # Google Patents is unavailable, so it is exactly when a request is
+    # least able to afford N sequential round-trips to the slower backend.
     if ops_token_manager.configured:
-        for i, res in enumerate(results):
-            if isinstance(res, Exception) or not res:
-                try:
-                    logging.info(
-                        f"Google Patents empty for `{params.queries[i]}`, trying OPS.")
-                    results[i] = await ops_search(httpx_client, params.queries[i], params.n_results)
-                except Exception as e:
-                    logging.warning(f"OPS fallback failed: {e}")
+        needs_fallback = [
+            i for i, res in enumerate(results)
+            if isinstance(res, Exception) or not res]
+        if needs_fallback:
+            logging.info(
+                f"Google Patents empty for {len(needs_fallback)} quer(y/ies), trying OPS.")
+            fallback_results = await asyncio.gather(
+                *[ops_search(httpx_client, params.queries[i], params.n_results)
+                  for i in needs_fallback],
+                return_exceptions=True)
+            # strict: gather returns exactly one result per index, so a
+            # length mismatch here would be a bug worth surfacing.
+            for i, res in zip(needs_fallback, fallback_results, strict=True):
+                if isinstance(res, Exception):
+                    logging.warning(f"OPS fallback failed for `{params.queries[i]}`: {res}")
+                else:
+                    results[i] = res
 
     return _shape_serp_results(results)
 
@@ -247,6 +262,15 @@ async def _search_one(q: str, n_results: int) -> tuple[str, list[dict], Optional
             result = await call()
             _backend_circuit_breaker.record_success(name)
             return q, result, None
+        except EmptyResultsError as e:
+            # 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 process-wide, so
+            # counting it would let a few unusual-but-legitimate queries
+            # disable the primary backend for every user. A hard block
+            # normally raises a real error, which is handled below.
+            logging.info(f"{name} returned no results for query `{q}`")
+            last_error = e
         except Exception as e:
             logging.error(f"Failed to query {name} with query `{q}`: {e}")
             _backend_circuit_breaker.record_failure(name)
@@ -331,14 +355,14 @@ async def scrap_patent(patent_id: PatentId) -> PatentScrapResult:
         if _is_not_found(google_error) and _is_not_found(e):
             raise HTTPException(
                 status_code=404,
-                detail=f"Patent '{patent_id}' not found on Google Patents or EPO OPS.")
+                detail=f"Patent '{patent_id}' not found on Google Patents or EPO OPS.") from e
         if isinstance(e, HTTPStatusError):
             raise HTTPException(
                 status_code=_upstream_status(e),
-                detail=f"EPO OPS returned {e.response.status_code} for '{patent_id}'.")
+                detail=f"EPO OPS returned {e.response.status_code} for '{patent_id}'.") from e
         raise HTTPException(
             status_code=_upstream_status(e),
-            detail=f"EPO OPS request failed for '{patent_id}': {e}")
+            detail=f"EPO OPS request failed for '{patent_id}': {e}") from e
 
 
 # Same reasoning as MAX_QUERIES_PER_REQUEST in serp.py: one accepted
@@ -381,20 +405,20 @@ async def ops_get_patent(patent_id: PatentId) -> PatentScrapResult:
             detail="EPO OPS is not configured (OPS_CONSUMER_KEY / OPS_CONSUMER_SECRET missing).")
     try:
         return await ops_scrap_patent(httpx_client, patent_id)
-    except OPSNotConfigured:
-        raise HTTPException(status_code=503, detail="EPO OPS is not configured.")
+    except OPSNotConfigured as e:
+        raise HTTPException(status_code=503, detail="EPO OPS is not configured.") from e
     except HTTPStatusError as e:
         if e.response.status_code == 404:
             raise HTTPException(
-                status_code=404, detail=f"Patent '{patent_id}' not found in EPO OPS.")
+                status_code=404, detail=f"Patent '{patent_id}' not found in EPO OPS.") from e
         raise HTTPException(
             status_code=_upstream_status(e),
-            detail=f"EPO OPS returned {e.response.status_code} for '{patent_id}'.")
+            detail=f"EPO OPS returned {e.response.status_code} for '{patent_id}'.") from e
     except Exception as e:
         logging.warning(f"Failed to retrieve patent {patent_id} from OPS: {e}")
         raise HTTPException(
             status_code=_upstream_status(e),
-            detail=f"EPO OPS request failed for '{patent_id}': {e}")
+            detail=f"EPO OPS request failed for '{patent_id}': {e}") from e
 
 
 @ops_router.post("/scrap_patents_bulk", response_model=OPSBulkResponse)

pyproject.toml

@@ -0,0 +1,17 @@
+# Lint configuration. The rule set is deliberately small: this repo had no
+# linter, and the first pass found a duplicate import shadowing another, four
+# unused imports and an f-string with no placeholders - all of which a
+# default pyflakes-equivalent catches. Widen the selection when the team
+# wants more, rather than starting from a large set and adding ignores.
+[tool.ruff]
+line-length = 120
+
+[tool.ruff.lint]
+select = [
+    "F",    # pyflakes: unused/duplicate imports, f-strings without placeholders
+    "E9",   # syntax and IO errors
+    "B",    # bugbear: likely-wrong constructs
+]
+ignore = [
+    "B008",  # FastAPI's Depends()/Field() in defaults is the documented idiom
+]

requirements-dev.txt

@@ -10,3 +10,5 @@
 pytest>=8,<10
 pytest-asyncio>=1.4.0,<1.5
 respx>=0.23.1,<0.24
+# Lint. Pinned like everything else; `ruff check .` is what CI runs.
+ruff>=0.14.6,<0.15

scrap.py

@@ -1,5 +1,4 @@
 import asyncio
-import logging
 import re
 from typing import Optional
 from httpx import AsyncClient

serp.py

@@ -3,9 +3,8 @@ from typing import Optional
 from duckduckgo_search import DDGS
 import httpx
 from pydantic import BaseModel, Field, field_validator
-from playwright.async_api import Browser, BrowserContext, Page, TimeoutError
+from playwright.async_api import Browser, BrowserContext, Page
 from urllib.parse import quote_plus
-import logging
 import re
 from lxml import etree
 import asyncio
@@ -61,14 +60,26 @@ class BrowserUnavailableError(Exception):
         super().__init__("Playwright browser is not available (it failed to start at startup).")
 
 
-class DuckDuckGoBlockedException(Exception):
+class EmptyResultsError(Exception):
+    """A backend returned zero results.
+
+    Deliberately distinct from a backend erroring, because zero results is
+    ambiguous: it can mean the query genuinely has no hits, or that the
+    backend served an anti-bot page that parsed to nothing. Callers (the
+    `/serp/search` fallback chain in particular) rely on an exception to
+    move on to the next backend, but should not treat this one as evidence
+    that the backend itself is unhealthy.
+    """
+
+
+class DuckDuckGoBlockedException(EmptyResultsError):
     """Raised when DuckDuckGo returns zero results.
 
     The duckduckgo_search library doesn't always raise when its request is
     blocked/rate-limited — it can also just parse zero results from the
-    anti-bot response page and return an empty list. Since callers (the
-    `/serp/search` fallback chain in particular) rely on an exception to know
-    a backend failed, treat an empty result as a failure too.
+    anti-bot response page and return an empty list. A hard block usually
+    does raise (and that still counts against the backend); this covers the
+    soft case, which is indistinguishable from a genuinely empty search.
     """
 
     def __init__(self, *args):
@@ -89,8 +100,12 @@ async def playwright_open_page(browser: Optional[Browser]):
         try:
             yield page
         finally:
-            await page.close()
-            await context.close()
+            # Close the context even if closing the page fails (a crashed
+            # renderer), otherwise the context leaks for the process's life.
+            try:
+                await page.close()
+            finally:
+                await context.close()
 
 
 async def _block_stylesheet_and_image_resources(route, request):
@@ -158,7 +173,7 @@ async def _extract_google_patents_results(page: Page, n_results: int) -> list[di
     # Wait for at least one search result item to appear
     # This ensures the page has loaded enough to start scraping
     await page.wait_for_function(
-        f"""() => document.querySelectorAll('search-result-item').length >= 1""",
+        "() => document.querySelectorAll('search-result-item').length >= 1",
         timeout=30_000
     )
 

tests/test_app.py

@@ -243,6 +243,10 @@ async def test_api_lifespan_launches_chromium_without_a_sandbox(monkeypatch):
     monkeypatch.setattr(app_module, "pw_browser", None)
     monkeypatch.setattr(app_module, "playwright", None)
     monkeypatch.setattr(app_module, "async_playwright", lambda: FakePlaywrightContextManager())
+    # The lifespan also closes the shared httpx client on shutdown; swap in
+    # a stand-in so this test doesn't close the real one for the rest of
+    # the session.
+    monkeypatch.setattr(app_module, "httpx_client", _RecordingClient())
 
     async with app_module.api_lifespan(app_module.app):
         pass
@@ -251,6 +255,54 @@ async def test_api_lifespan_launches_chromium_without_a_sandbox(monkeypatch):
     assert "--no-sandbox" in launch_calls[0].get("args", [])
 
 
+class _RecordingClient:
+    def __init__(self):
+        self.closed = False
+
+    async def aclose(self):
+        self.closed = True
+
+
+async def test_api_lifespan_closes_the_http_client_on_shutdown(monkeypatch):
+    """The client was created at import and never closed - the browser was
+    torn down on shutdown but its connection pool was not.
+    """
+    class FakePlaywright:
+        chromium = None
+
+        async def stop(self):
+            pass
+
+    recording_client = _RecordingClient()
+    monkeypatch.setattr(app_module, "pw_browser", None)
+    monkeypatch.setattr(app_module, "playwright", None)
+    monkeypatch.setattr(app_module, "httpx_client", recording_client)
+    # Playwright failing to start is the documented degraded mode; the
+    # client must still be closed cleanly on the way out.
+    monkeypatch.setattr(
+        app_module, "async_playwright", lambda: (_ for _ in ()).throw(RuntimeError("no browser")))
+
+    async with app_module.api_lifespan(app_module.app):
+        pass
+
+    assert recording_client.closed is True
+
+
+async def test_api_lifespan_survives_playwright_failing_to_start(monkeypatch):
+    """Documented behaviour: the app still comes up, pw_browser stays None,
+    and the browser-backed endpoints fail individually rather than the whole
+    deployment refusing to boot.
+    """
+    monkeypatch.setattr(app_module, "pw_browser", None)
+    monkeypatch.setattr(app_module, "playwright", None)
+    monkeypatch.setattr(app_module, "httpx_client", _RecordingClient())
+    monkeypatch.setattr(
+        app_module, "async_playwright", lambda: (_ for _ in ()).throw(RuntimeError("no browser")))
+
+    async with app_module.api_lifespan(app_module.app):
+        assert app_module.pw_browser is None
+
+
 # --------------------------------- _search_one fallback ---------------------------------
 
 
@@ -419,3 +471,87 @@ async def test_search_endpoint_falls_through_backends_per_query(client, monkeypa
     data = resp.json()
     assert data["results"] == [{"title": "brave has a"}]
     assert data["error"] is None
+
+
+# --------------------- what counts as a backend failure ---------------------
+
+
+async def test_a_zero_result_query_does_not_open_the_backends_circuit(monkeypatch):
+    """An empty result is ambiguous - it can mean the query genuinely has
+    no hits, or that the backend served an anti-bot page that parsed to
+    nothing. The circuit breaker is a process-wide singleton, so counting
+    the ambiguous case meant three unusual-but-legitimate queries in a row
+    could disable the primary backend for every user of the deployment for
+    a full cooldown.
+    """
+    from circuit_breaker import CircuitBreaker
+    from serp import DuckDuckGoBlockedException
+
+    breaker = CircuitBreaker(failure_threshold=1, cooldown_seconds=60)
+    monkeypatch.setattr(app_module, "_backend_circuit_breaker", breaker)
+
+    ddg_calls = []
+
+    async def empty_ddg(q, n):
+        ddg_calls.append(q)
+        raise DuckDuckGoBlockedException()
+
+    async def fake_brave(browser, q, n):
+        return [{"title": "from brave"}]
+
+    monkeypatch.setattr(app_module, "query_ddg_search", empty_ddg)
+    monkeypatch.setattr(app_module, "query_brave_search", fake_brave)
+
+    await app_module._search_one("obscure query", 5)
+    await app_module._search_one("another obscure query", 5)
+
+    # Still being tried: an empty result is not evidence the backend is down.
+    assert ddg_calls == ["obscure query", "another obscure query"]
+
+
+async def test_an_empty_result_still_falls_through_to_the_next_backend(monkeypatch):
+    """Not counting it against the backend must not stop the fallback: the
+    caller still needs results from somewhere."""
+    from serp import DuckDuckGoBlockedException
+
+    async def empty_ddg(q, n):
+        raise DuckDuckGoBlockedException()
+
+    async def fake_brave(browser, q, n):
+        return [{"title": "from brave"}]
+
+    monkeypatch.setattr(app_module, "query_ddg_search", empty_ddg)
+    monkeypatch.setattr(app_module, "query_brave_search", fake_brave)
+
+    _q, results, err = await app_module._search_one("widget", 5)
+
+    assert results == [{"title": "from brave"}]
+    assert err is None
+
+
+async def test_a_real_backend_error_still_opens_the_circuit(monkeypatch):
+    """The distinction has to cut both ways: a backend that raises a
+    genuine error (a rate limit, a transport failure) is still evidence,
+    and must still trip the breaker.
+    """
+    from circuit_breaker import CircuitBreaker
+
+    breaker = CircuitBreaker(failure_threshold=1, cooldown_seconds=60)
+    monkeypatch.setattr(app_module, "_backend_circuit_breaker", breaker)
+
+    ddg_calls = []
+
+    async def failing_ddg(q, n):
+        ddg_calls.append(q)
+        raise RuntimeError("rate limited")
+
+    async def fake_brave(browser, q, n):
+        return [{"title": "from brave"}]
+
+    monkeypatch.setattr(app_module, "query_ddg_search", failing_ddg)
+    monkeypatch.setattr(app_module, "query_brave_search", fake_brave)
+
+    await app_module._search_one("first", 5)
+    await app_module._search_one("second", 5)
+
+    assert ddg_calls == ["first"]  # skipped the second time

tests/test_utils.py

@@ -1,15 +1,13 @@
 import logging
 
-from serp import SerpQuery
 from utils import log_gathered_exceptions
 
 
 def test_logs_only_the_exceptions_with_their_query(caplog):
-    params = SerpQuery(queries=["q1", "q2", "q3"])
     results = ["ok result", ValueError("boom"), "another ok"]
 
     with caplog.at_level(logging.WARNING):
-        log_gathered_exceptions(results, "test context", params)
+        log_gathered_exceptions(results, "test context", ["q1", "q2", "q3"])
 
     warnings = [r for r in caplog.records if r.levelno == logging.WARNING]
     assert len(warnings) == 1
@@ -19,11 +17,8 @@ def test_logs_only_the_exceptions_with_their_query(caplog):
 
 
 def test_logs_nothing_when_all_results_succeeded(caplog):
-    params = SerpQuery(queries=["q1", "q2"])
-    results = ["ok", "also ok"]
-
     with caplog.at_level(logging.WARNING):
-        log_gathered_exceptions(results, "test context", params)
+        log_gathered_exceptions(["ok", "also ok"], "test context", ["q1", "q2"])
 
     assert [r for r in caplog.records if r.levelno == logging.WARNING] == []
 
@@ -33,8 +28,29 @@ def test_uses_its_own_logger_not_the_asyncio_one(caplog):
     misattributed these warnings to the 'asyncio' logging namespace instead
     of the app's own. Fixed to `logging.getLogger(__name__)`.
     """
-    params = SerpQuery(queries=["q1"])
     with caplog.at_level(logging.WARNING):
-        log_gathered_exceptions([ValueError("boom")], "ctx", params)
+        log_gathered_exceptions([ValueError("boom")], "ctx", ["q1"])
 
     assert caplog.records[0].name == "utils"
+
+
+def test_takes_a_plain_sequence_of_queries(caplog):
+    """This helper is the lowest layer in the codebase; it used to import
+    SerpQuery from serp.py purely to read one attribute, inverting the
+    dependency. It now accepts any iterable of query strings, so the patent
+    and OPS paths can use it too.
+    """
+    with caplog.at_level(logging.WARNING):
+        log_gathered_exceptions([ValueError("boom")], "ctx", iter(["only-query"]))
+
+    assert "only-query" in caplog.records[0].message
+
+
+def test_extra_results_without_a_matching_query_are_ignored(caplog):
+    """zip stops at the shorter sequence - pinning that so a mismatch can't
+    raise inside an error-logging path.
+    """
+    with caplog.at_level(logging.WARNING):
+        log_gathered_exceptions([ValueError("a"), ValueError("b")], "ctx", ["q1"])
+
+    assert len([r for r in caplog.records if r.levelno == logging.WARNING]) == 1

utils.py

@@ -1,12 +1,20 @@
 import logging
-
-from serp import SerpQuery
+from typing import Iterable
 
 logger = logging.getLogger(__name__)
 
 
-def log_gathered_exceptions(results: list, context: str, params: SerpQuery):
-    """Logs gathered exceptions with context and parameters."""
-    for exc, q in zip(results, params.queries):
+def log_gathered_exceptions(results: list, context: str, queries: Iterable[str]):
+    """Log the exceptions in a `return_exceptions=True` gather, named by the
+    query that produced each.
+
+    Takes the queries themselves rather than a SerpQuery: this is the
+    lowest layer in the codebase and importing the domain model inverted
+    the dependency for the sake of one attribute access. It also means the
+    patent and OPS paths can use it.
+    """
+    # strict=False deliberately: this is an error-logging path and must
+    # never raise on a length mismatch of its own.
+    for exc, q in zip(results, queries, strict=False):
         if isinstance(exc, Exception):
             logger.warning(f"Error during {context} for query '{q}': {exc}")