diff --git a/backend/data_loader.py b/backend/data_loader.py index 1816745..79ba386 100644 --- a/backend/data_loader.py +++ b/backend/data_loader.py @@ -84,34 +84,55 @@ def _get_typesense_client(): return None -def search_schools_typesense(query: str) -> Optional[List[int]]: - """Return all matching URNs in relevance order; None means unavailable. +SEARCH_PAGE_SIZE = 250 +# Search results are filtered again by the API (authority, phase, postcode, +# etc.), so one page is too small for scoped searches. Keep the candidate set +# bounded, though: a broad query must not turn into an unbounded sequence of +# Typesense requests. Four pages is enough to preserve useful scoped matches +# while putting a hard ceiling on latency and upstream load. +SEARCH_MAX_CANDIDATES = 1_000 - Filtering and user pagination happen in the API after this search. Returning - only the first search page would silently discard valid local matches. - Never return a partial candidate set if a later page fails. + +def search_schools_typesense(query: str) -> Optional[List[int]]: + """Return a bounded set of matching URNs in relevance order. + + ``None`` means Typesense is unavailable; ``[]`` is a valid zero-match + result. The API applies its remaining filters after this search, so the + first few pages are fetched rather than only the first page. Once the + candidate ceiling is reached, the relevance-ordered prefix is returned on + purpose; fetching every match would make common or adversarial queries + unbounded. """ client = _get_typesense_client() if client is None: return None - urns = [] + urns: list[int] = [] + fetched = 0 try: page = 1 - while True: + while fetched < SEARCH_MAX_CANDIDATES: + page_size = min(SEARCH_PAGE_SIZE, SEARCH_MAX_CANDIDATES - fetched) result = client.collections["schools"].documents.search({ "q": query, "query_by": "school_name,local_authority,postcode", - "per_page": 250, + "per_page": page_size, "page": page, "typo_tokens_threshold": 1, }) hits = result.get("hits", []) urns.extend(int(h["document"]["urn"]) for h in hits) - if len(urns) >= result.get("found", len(urns)): + fetched += len(hits) + if fetched >= result.get("found", fetched): return list(dict.fromkeys(urns)) if not hits: raise ValueError("Search pagination ended before all matches arrived") page += 1 + logging.getLogger(__name__).info( + "Typesense search capped at %d candidates for query %r", + SEARCH_MAX_CANDIDATES, + query, + ) + return list(dict.fromkeys(urns)) except Exception: logging.getLogger(__name__).exception("School search unavailable") return None diff --git a/backend/tests/test_search_completeness.py b/backend/tests/test_search_completeness.py index e170a8b..b32634a 100644 --- a/backend/tests/test_search_completeness.py +++ b/backend/tests/test_search_completeness.py @@ -23,6 +23,47 @@ def test_search_returns_matches_beyond_first_page(monkeypatch): assert pages == [1, 2] +def test_search_caps_broad_queries_at_a_bounded_number_of_pages(monkeypatch): + requests = [] + + def search(params): + requests.append(params) + return { + 'found': 10_000, + 'hits': [ + {'document': {'urn': 100000 + params['page'] * 1000 + i}} + for i in range(params['per_page']) + ], + } + + client_for(monkeypatch, search) + result = data_loader.search_schools_typesense('school') + + assert len(result) == data_loader.SEARCH_MAX_CANDIDATES + assert len(requests) == data_loader.SEARCH_MAX_CANDIDATES // data_loader.SEARCH_PAGE_SIZE + assert all(request['per_page'] == data_loader.SEARCH_PAGE_SIZE for request in requests) + assert requests[-1]['page'] == len(requests) + + +def test_search_uses_a_smaller_final_page_when_the_cap_is_not_a_page_multiple(monkeypatch): + monkeypatch.setattr(data_loader, 'SEARCH_MAX_CANDIDATES', 251) + requests = [] + + def search(params): + requests.append(params) + return { + 'found': 10_000, + 'hits': [{'document': {'urn': 100000 + len(requests) * 1000 + i}} + for i in range(params['per_page'])], + } + + client_for(monkeypatch, search) + result = data_loader.search_schools_typesense('school') + + assert len(result) == 251 + assert [request['per_page'] for request in requests] == [250, 1] + + def test_later_page_failure_does_not_return_partial_results(monkeypatch): def search(params): if params['page'] == 2: diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a00995e..a94087d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -104,8 +104,10 @@ alias-update response must never cause deletion of a potentially live index. The backend snapshot swap is process-local and assumes the current single-worker deployment. It is not an atomic transaction spanning PostgreSQL marts, Typesense and Next.js caches. Next.js caches are not explicitly purged by the pipeline. -School search retrieves every Typesense candidate before applying API filters; -only a dependency failure invokes substring fallback, not a valid empty match set. +School search retrieves a relevance-ordered candidate prefix (currently capped at +1,000 URNs) before applying API filters. This keeps scoped searches useful while +putting a hard ceiling on Typesense round trips; only a dependency failure invokes +substring fallback, not a valid empty match set. ## Deployment references