diff --git a/backend/app.py b/backend/app.py index d352de5..9e60c1d 100644 --- a/backend/app.py +++ b/backend/app.py @@ -6,6 +6,7 @@ Uses real data from UK Government Compare School Performance downloads. import hashlib import re +import time from contextlib import asynccontextmanager from datetime import datetime, timezone from typing import Optional @@ -15,7 +16,7 @@ import pandas as pd from fastapi import FastAPI, HTTPException, Query, Request, Depends, Header from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware -from fastapi.responses import FileResponse, Response +from fastapi.responses import FileResponse, JSONResponse, Response from fastapi.staticfiles import StaticFiles from slowapi import Limiter, _rate_limit_exceeded_handler from slowapi.util import get_remote_address @@ -33,6 +34,7 @@ from .data_loader import ( get_supplementary_data, get_supplementary_data_batch, search_schools_typesense, + suggest_schools_typesense, ) from .data_loader import get_data_info as get_db_info from . import flags @@ -296,8 +298,101 @@ def clean_filter_values(series: pd.Series) -> list[str]: # SECURITY MIDDLEWARE & HELPERS # ============================================================================= -# Rate limiter -limiter = Limiter(key_func=get_remote_address) +def client_key(request: Request) -> str: + """The rate-limit bucket: the real caller, not the proxy in front of them. + + `get_remote_address` reads request.client.host. In staging and production + the backend has no published ports and sits on the internal network, so its + only caller is the Next proxy — meaning every browser user on the site + shared one bucket. Measured before this fix: 70 concurrent requests to + /api/schools returned 60 OK and 10 refused. + + CF-Connecting-IP first, because Cloudflare (in front of both environments) + sets it on every origin request and *overwrites* any client-supplied value, + which a parsed X-Forwarded-For chain does not guarantee. The XFF fallback is + forgeable, but only by a caller already inside the Docker network, which is + the one place nothing untrusted can reach. + """ + cf = request.headers.get("cf-connecting-ip") + if cf: + return cf.strip() + xff = request.headers.get("x-forwarded-for") + if xff: + return xff.split(",")[0].strip() + return get_remote_address(request) + + +# Per-client limiter. Paired with the global ceiling below — the two do +# different jobs and neither substitutes for the other. +limiter = Limiter(key_func=client_key) + + +# --- The ceiling no header can raise ---------------------------------------- +# +# client_key trusts CF-Connecting-IP, and nothing in this process can tell an +# edge-set header from an attacker-set one. That distinction can only be made +# at Cloudflare, with Authenticated Origin Pulls or an origin firewall. A +# caller reaching the origin directly could otherwise mint a fresh rate-limit +# bucket per request and evade per-client limits entirely — which would make +# correct keying a net regression against abuse, since the single shared bucket +# it replaced at least capped everyone at 60/minute together. +# +# So per-client limits give fairness, and this gives the origin a hard total. +# It does not make the header trustworthy; it bounds what trusting it can cost. +# The header problem itself is closed at Cloudflare, not here. +# +# [window_start_monotonic, count], or None before the first request. A fixed +# window is crude, which is right for a backstop: it has to be obviously +# correct rather than fair. +_global_window: Optional[list] = None + +# The container healthcheck runs `curl http://localhost:80/api/data-info` from +# inside the container. Starving it would fail the check, restart the +# container, and turn a load spike into an outage loop — the ceiling exists to +# protect the origin, not to kill it. +_LOCAL_HOSTS = frozenset({"127.0.0.1", "::1", "localhost"}) + + +def exempt_from_ceiling(request: Request) -> bool: + """Whether the ceiling should ignore this request. + + Its own function so the rule is testable without standing up a server — + and so the healthcheck exemption is somewhere a reader can find it. + """ + if not request.url.path.startswith("/api/"): + return True + # The peer address, never the Host header: Host is set by the caller and + # would hand every attacker an exemption. + return (request.client.host if request.client else "") in _LOCAL_HOSTS + + +class GlobalRateLimitMiddleware(BaseHTTPMiddleware): + """A cap on total /api/ traffic, independent of any client identity.""" + + async def dispatch(self, request: Request, call_next): + global _global_window + + if exempt_from_ceiling(request): + return await call_next(request) + + now = time.monotonic() + # One event loop, and no await between the read and the write, so this + # sequence is atomic without a lock. + if _global_window is None or now - _global_window[0] >= 60: + _global_window = [now, 0] + _global_window[1] += 1 + + if _global_window[1] > settings.global_rate_limit_per_minute: + return JSONResponse( + # Distinguishable from slowapi's per-client 429: an operator + # reading logs has to be able to tell "one noisy client" from + # "the origin is saturated". + {"detail": "The service is at capacity. Please retry shortly."}, + status_code=429, + headers={"Retry-After": + str(max(1, int(60 - (now - _global_window[0]))))}, + ) + return await call_next(request) class SecurityHeadersMiddleware(BaseHTTPMiddleware): @@ -355,6 +450,7 @@ CACHE_RULES: list[tuple[str, tuple[int, int, int]]] = [ ("/api/schools/", (300, 3600, 86400)), # /api/schools/{urn} ("/api/rankings", (60, 600, 3600)), ("/api/compare", (60, 600, 3600)), + ("/api/suggest", (60, 3600, 86400)), # autosuggest ("/api/schools", (30, 300, 1800)), # search list ] @@ -502,6 +598,10 @@ app.add_middleware(CacheAndETagMiddleware) app.add_middleware(SecurityHeadersMiddleware) app.add_middleware(RequestSizeLimitMiddleware) app.add_middleware(GZipMiddleware, minimum_size=512) +# Added last, so it is outermost and refuses before anything downstream does +# work. A ceiling that only applies after the expensive part has run is not a +# ceiling. +app.add_middleware(GlobalRateLimitMiddleware) # CORS middleware - restricted for production app.add_middleware( @@ -1271,6 +1371,38 @@ async def get_place(request: Request, kind: str, slug: str, } +# Two characters. One is not a query — it matches thousands of schools and the +# response is useless, so it is not worth a round trip. +SUGGEST_MIN_QUERY = 2 + + +@app.get("/api/suggest") +@limiter.limit("120/minute") +async def suggest_schools( + request: Request, + q: str = Query("", max_length=100), + limit: int = Query(8, ge=1, le=20), +): + """School name suggestions, from Typesense alone. + + Deliberately not a mode of /api/schools: that path filters and sorts the + full in-memory DataFrame, which is far too expensive to run per keystroke. + + Nothing here returns an error for ordinary input. A short query, no + matches, or Typesense being unreachable are all 200 with an empty list — + a dropdown that quietly does not appear is the right failure for a + keystroke path, and there is no DataFrame fallback because the 25,000-row + substring scan is precisely what this endpoint exists to avoid. + + 120/minute rather than the default 60: a 200 ms debounce makes typing + legitimately bursty. + """ + query = q.strip() + if len(query) < SUGGEST_MIN_QUERY: + return {"suggestions": []} + return {"suggestions": suggest_schools_typesense(query, limit)} + + @app.get("/api/flags") @limiter.limit(f"{settings.rate_limit_per_minute}/minute") async def get_feature_flags(request: Request): diff --git a/backend/config.py b/backend/config.py index 554efc9..f663ba7 100644 --- a/backend/config.py +++ b/backend/config.py @@ -35,6 +35,11 @@ class Settings(BaseSettings): # Security admin_api_key: str = Field(default_factory=lambda: secrets.token_urlsafe(32)) rate_limit_per_minute: int = 60 # Requests per minute per IP + # A ceiling on total /api/ traffic, independent of any client identity. + # client_key trusts headers only Cloudflare can vouch for, so a caller + # reaching the origin directly could otherwise mint a fresh bucket per + # request. See GlobalRateLimitMiddleware in backend/app.py. + global_rate_limit_per_minute: int = 3000 rate_limit_burst: int = 10 # Allow burst of requests max_request_size: int = 1024 * 1024 # 1MB max request size diff --git a/backend/data_loader.py b/backend/data_loader.py index 62aa4a5..8aff4df 100644 --- a/backend/data_loader.py +++ b/backend/data_loader.py @@ -100,6 +100,58 @@ def search_schools_typesense(query: str, limit: int = 250) -> List[int]: return [] +# The most a public endpoint will return in one response. +SUGGEST_MAX_LIMIT = 20 + +# Fields a suggestion row carries, and the default when the document omits an +# optional one. phase and school_type are optional in the Typesense schema. +_SUGGEST_FIELDS = ("school_name", "local_authority", "postcode", + "phase", "school_type") + + +def suggest_schools_typesense(query: str, limit: int = 8) -> List[dict]: + """Autosuggest rows straight from Typesense. Never raises. + + Returns documents rather than URNs, unlike search_schools_typesense, so the + caller needs no DataFrame. Every field below is already in the index — see + pipeline/scripts/sync_typesense.py — which is what makes this cheap enough + to run per keystroke. + """ + client = _get_typesense_client() + if client is None: + return [] + try: + result = client.collections["schools"].documents.search({ + "q": query, + "query_by": "school_name,local_authority", + "per_page": max(1, min(limit, SUGGEST_MAX_LIMIT)), + "typo_tokens_threshold": 1, + }) + except Exception: + # A dropdown that quietly stops appearing is the right failure here. + return [] + + rows = [] + for hit in result.get("hits", []) or []: + doc = (hit or {}).get("document") or {} + try: + urn = int(doc["urn"]) + except (KeyError, TypeError, ValueError): + # Skip the row, keep the rest. Typesense declares urn as int32 so + # this should be unreachable, but the index is a separate system + # that something other than this code can reindex — and "never + # raises" is a promise the keystroke path actually depends on. + # Dropping one malformed document is right; blanking the whole + # dropdown, or serving a suggestion pointing at /school/0, is not. + logging.getLogger(__name__).warning( + "skipping malformed suggestion document: %r", doc) + continue + row = {"urn": urn} + row.update({f: str(doc.get(f, "") or "") for f in _SUGGEST_FIELDS}) + rows.append(row) + return rows + + def normalize_school_type(school_type: Optional[str]) -> Optional[str]: """Convert cryptic school type codes to user-friendly names.""" if not school_type: diff --git a/backend/flags.py b/backend/flags.py index f843828..a06e93e 100644 --- a/backend/flags.py +++ b/backend/flags.py @@ -50,6 +50,13 @@ REGISTRY: dict[str, Flag] = { ), added=date(2026, 8, 23), ), + Flag( + name="school_autosuggest", + description=( + "School name suggestions as you type in the main search box." + ), + added=date(2026, 8, 26), + ), ) } diff --git a/backend/tests/test_rate_limit_key.py b/backend/tests/test_rate_limit_key.py new file mode 100644 index 0000000..2f0a12a --- /dev/null +++ b/backend/tests/test_rate_limit_key.py @@ -0,0 +1,147 @@ +"""The rate-limit bucket must be the caller, not the proxy in front of them. + +`get_remote_address` reads request.client.host. In staging and production the +backend has no published ports and its only caller is the Next proxy, so that +host is the Next container — one bucket for every browser user on the site. +Measured before this fix: 70 concurrent requests, 60 served and 10 refused. +""" + +from starlette.datastructures import Headers + +from backend.app import client_key + + +class _Req: + """Enough of a Request for the key function: headers and a client host.""" + + def __init__(self, headers: dict, host: str = "10.0.0.9"): + self.headers = Headers(headers) + self.client = type("C", (), {"host": host})() + self.scope = {"type": "http", "client": (host, 0), + "headers": [(k.lower().encode(), v.encode()) + for k, v in headers.items()]} + + +def test_cloudflare_header_wins(): + # Cloudflare sets CF-Connecting-IP and overwrites any client-supplied + # value, so it is trustworthy in a way a parsed XFF chain is not. + assert client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) == "203.0.113.7" + + +def test_forwarded_for_is_the_fallback_and_takes_the_first_entry(): + # Left-most is the original client; everything after it is proxies. + assert client_key( + _Req({"x-forwarded-for": "203.0.113.7, 10.0.0.2"})) == "203.0.113.7" + + +def test_remote_address_is_the_last_resort(): + assert client_key(_Req({}, host="10.0.0.9")) == "10.0.0.9" + + +def test_cloudflare_header_beats_forwarded_for(): + key = client_key(_Req({"cf-connecting-ip": "203.0.113.7", + "x-forwarded-for": "198.51.100.1"})) + assert key == "203.0.113.7" + + +def test_two_callers_get_two_buckets(): + # The whole point: one user exhausting their limit must not refuse another. + a = client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) + b = client_key(_Req({"cf-connecting-ip": "203.0.113.8"})) + assert a != b + + +def test_whitespace_is_stripped(): + # "a, b" split on comma leaves a leading space on every entry but the + # first; an unstripped key silently creates a second bucket per client. + assert client_key(_Req({"x-forwarded-for": " 203.0.113.7 ,10.0.0.2"})) \ + == "203.0.113.7" + + +# --------------------------------------------------------------------------- +# The ceiling that header rotation cannot raise. +# --------------------------------------------------------------------------- + +import pytest +from fastapi.testclient import TestClient + + +@pytest.fixture() +def api(monkeypatch): + from backend import app as app_module + from backend.config import settings + + monkeypatch.setattr(settings, "global_rate_limit_per_minute", 5) + monkeypatch.setattr(app_module, "_global_window", None) + return TestClient(app_module.app, raise_server_exceptions=False) + + + +def _ceiling_req(path: str, host: str): + """Enough of a Request for exempt_from_ceiling: a path and a peer host.""" + return type("R", (), { + "url": type("U", (), {"path": path})(), + "client": type("C", (), {"host": host})(), + })() + + +def _get(client, path="/api/flags", cf=None): + headers = {"cf-connecting-ip": cf} if cf else {} + return client.get(path, headers=headers) + + +def test_rotating_the_cloudflare_header_cannot_buy_unlimited_requests(api): + """The attack the per-client keying opened up. + + client_key trusts CF-Connecting-IP, and nothing in this process can tell an + edge-set header from an attacker-set one — that distinction can only be + made at Cloudflare, with Authenticated Origin Pulls or an origin firewall. + A caller reaching the origin directly can therefore mint a fresh + rate-limit bucket per request and evade per-client limits entirely. + + Per-client fairness is still the right default; this is the backstop that + bounds what evading it can achieve. Without it, correct keying would be a + net regression against abuse compared with the shared bucket it replaced. + """ + codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(8)] + assert codes.count(200) == 5 + assert codes.count(429) == 3 + + +def test_the_ceiling_says_which_limit_was_hit(api): + # Distinguishable from slowapi's per-client 429, or an operator reading + # logs cannot tell "one noisy client" from "the origin is saturated". + for i in range(5): + _get(api, cf=f"203.0.113.{i}") + refused = _get(api, cf="203.0.113.99") + assert refused.status_code == 429 + assert "capacity" in refused.json()["detail"].lower() + assert refused.headers.get("retry-after") + + +def test_traffic_below_the_ceiling_is_untouched(api): + codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(5)] + assert codes == [200] * 5 + + +def test_the_container_healthcheck_is_exempt(api): + """The healthcheck runs `curl http://localhost:80/api/data-info` inside the + container. If the ceiling could starve it, saturation would fail the + healthcheck, restart the container, and turn a load spike into an outage + loop — the ceiling has to protect the origin, not kill it. + """ + from backend.app import exempt_from_ceiling + + assert exempt_from_ceiling(_ceiling_req("/api/data-info", "127.0.0.1")) + assert exempt_from_ceiling(_ceiling_req("/api/data-info", "::1")) + # Everyone else is counted. + assert not exempt_from_ceiling(_ceiling_req("/api/data-info", "10.0.0.9")) + + +def test_the_ceiling_ignores_non_api_paths(): + # Sitemaps and robots.txt are served by this app too, and a crawler + # fetching them must not be refused because the API is busy. + from backend.app import exempt_from_ceiling + + assert exempt_from_ceiling(_ceiling_req("/sitemap.xml", "10.0.0.9")) + assert exempt_from_ceiling(_ceiling_req("/robots.txt", "10.0.0.9")) diff --git a/backend/tests/test_suggest.py b/backend/tests/test_suggest.py new file mode 100644 index 0000000..180b7c3 --- /dev/null +++ b/backend/tests/test_suggest.py @@ -0,0 +1,157 @@ +"""Tests for school autosuggest (spec 2026-08-26).""" + +from backend import data_loader + + +class _FakeDocs: + def __init__(self, hits, explode=False): + self._hits = hits + self._explode = explode + self.last_params = None + + def search(self, params): + self.last_params = params + if self._explode: + raise RuntimeError("typesense is down") + return {"hits": [{"document": d} for d in self._hits]} + + +class _FakeClient: + def __init__(self, hits, explode=False): + self.docs = _FakeDocs(hits, explode) + self.collections = {"schools": type("C", (), {"documents": self.docs})()} + + +_HIT = { + "urn": 100010, "school_name": "Brecknock Primary School", + "local_authority": "Camden", "postcode": "NW1 1AA", + "phase": "Primary", "school_type": "Community school", +} + + +def _use(monkeypatch, client): + monkeypatch.setattr(data_loader, "_get_typesense_client", lambda: client) + + +def test_returns_the_fields_a_suggestion_needs(monkeypatch): + # Local authority is not decoration: there are many schools called + # "St Mary's", and a list without it cannot be chosen between. + _use(monkeypatch, _FakeClient([_HIT])) + out = data_loader.suggest_schools_typesense("breck") + assert out == [{ + "urn": 100010, "school_name": "Brecknock Primary School", + "local_authority": "Camden", "postcode": "NW1 1AA", + "phase": "Primary", "school_type": "Community school", + }] + + +def test_a_missing_optional_field_becomes_an_empty_string(monkeypatch): + # phase and school_type are optional in the Typesense schema. A missing + # key must not KeyError in the keystroke path. + _use(monkeypatch, _FakeClient([{"urn": 1, "school_name": "X", + "local_authority": "Y", "postcode": "Z"}])) + out = data_loader.suggest_schools_typesense("x") + assert out[0]["phase"] == "" and out[0]["school_type"] == "" + + +def test_typesense_unavailable_gives_no_suggestions_rather_than_raising(monkeypatch): + _use(monkeypatch, None) + assert data_loader.suggest_schools_typesense("anything") == [] + + +def test_a_typesense_error_gives_no_suggestions_rather_than_raising(monkeypatch): + _use(monkeypatch, _FakeClient([], explode=True)) + assert data_loader.suggest_schools_typesense("anything") == [] + + +def test_the_limit_is_passed_through_and_clamped(monkeypatch): + client = _FakeClient([]) + _use(monkeypatch, client) + data_loader.suggest_schools_typesense("x", limit=500) + assert client.docs.last_params["per_page"] == 20 + + +def _client(monkeypatch, rows, *, blow_up_dataframe=False): + from fastapi.testclient import TestClient + from backend import app as app_module + + monkeypatch.setattr(app_module, "suggest_schools_typesense", + lambda q, limit=8: rows) + if blow_up_dataframe: + def _boom(): + raise AssertionError("the suggest path must not load the DataFrame") + monkeypatch.setattr(app_module, "load_school_data", _boom) + monkeypatch.setattr(app_module, "load_latest_school_data", _boom) + return TestClient(app_module.app, raise_server_exceptions=False) + + +def test_the_endpoint_returns_suggestions(monkeypatch): + body = _client(monkeypatch, [_HIT]).get("/api/suggest?q=breck").json() + assert body["suggestions"][0]["school_name"] == "Brecknock Primary School" + + +def test_the_endpoint_never_touches_the_dataframe(monkeypatch): + """The whole reason this is not a mode of /api/schools. + + That endpoint filters and sorts 25,000 rows of pandas per query, holding + the GIL. Per keystroke, that is the cost this endpoint exists to avoid. + """ + res = _client(monkeypatch, [_HIT], blow_up_dataframe=True).get("/api/suggest?q=breck") + assert res.status_code == 200 + assert res.json()["suggestions"] + + +def test_a_one_character_query_returns_nothing_and_does_not_error(monkeypatch): + # The keystroke path never errors on ordinary input. + res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=b") + assert res.status_code == 200 + assert res.json() == {"suggestions": []} + + +def test_a_blank_query_returns_nothing_and_does_not_error(monkeypatch): + res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=") + assert res.status_code == 200 + assert res.json() == {"suggestions": []} + + +def test_typesense_down_is_an_empty_list_not_a_500(monkeypatch): + res = _client(monkeypatch, []).get("/api/suggest?q=breck") + assert res.status_code == 200 + assert res.json() == {"suggestions": []} + + +def test_the_response_is_cacheable(monkeypatch): + # Prefix queries repeat enormously across users, and school names change + # once a year. Without this the endpoint pays full price every keystroke. + res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=breck") + assert "s-maxage" in res.headers.get("cache-control", "") + assert res.headers.get("etag") + + +def test_a_malformed_urn_does_not_raise(monkeypatch): + """The docstring promises "never raises"; the parsing loop sat outside the + try, so int(None) or int("abc") would have turned a keystroke into a 500. + + Typesense declares urn as int32, so this should be unreachable — but the + contract is what the caller relies on, and a search index is a separate + system that can be reindexed by something other than this code. + """ + _use(monkeypatch, _FakeClient([{"urn": None, "school_name": "X", + "local_authority": "Y", "postcode": "Z"}])) + assert data_loader.suggest_schools_typesense("x") == [] + + +def test_a_malformed_row_does_not_discard_the_good_ones(monkeypatch): + # One bad document must not blank the whole dropdown. + _use(monkeypatch, _FakeClient([ + {"urn": "not-a-number", "school_name": "Bad", "local_authority": "Y", + "postcode": "Z"}, + _HIT, + ])) + out = data_loader.suggest_schools_typesense("x") + assert [r["urn"] for r in out] == [100010] + + +def test_a_hit_with_no_document_does_not_raise(monkeypatch): + _use(monkeypatch, _FakeClient([{}])) + assert data_loader.suggest_schools_typesense("x") == [] diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 14b6f83..92353d4 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -151,6 +151,36 @@ needed), and fails the check only when a finding is rated **severe** (would break prod, leak data, or corrupt data). Minor findings are informational and never block a merge. +## Rate limiting, and the Cloudflare gap + +Two independent limits protect the API: + +- **Per client**, via slowapi, keyed on `CF-Connecting-IP` (falling back to + `X-Forwarded-For`, then the peer address). 60/minute by default; + `/api/suggest` gets 120/minute because typing is bursty. +- **Globally**, via `GlobalRateLimitMiddleware`: a fixed 60-second window over + all `/api/` traffic, `GLOBAL_RATE_LIMIT_PER_MINUTE` (default 3000), + independent of any client identity. Requests from `127.0.0.1` are exempt so + the container healthcheck cannot be starved into a restart loop. + +### Open: the origin must only accept Cloudflare + +`CF-Connecting-IP` is only meaningful for requests that actually reached the +origin through Cloudflare, and **the application cannot verify that they did**. +Anything able to reach the origin directly can set that header freely and, by +rotating it, mint a fresh rate-limit bucket per request — defeating per-client +limits on every endpoint. + +The global ceiling bounds the damage to total origin capacity. It does not fix +the underlying gap, and nothing in the code can. Closing it needs one of: + +- **Authenticated Origin Pulls** — Cloudflare presents a client certificate the + origin requires, so non-Cloudflare traffic is refused at TLS. +- **An origin firewall** restricted to Cloudflare's published IP ranges. + +Until one is in place, treat per-client limits as protection against accidents +and ordinary load, not against a determined caller. + ## Feature flags (Unleash) Flag state lives in a self-hosted Unleash instance, deployed as its own diff --git a/docs/superpowers/plans/2026-08-26-school-autosuggest.md b/docs/superpowers/plans/2026-08-26-school-autosuggest.md new file mode 100644 index 0000000..e57c19b --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-school-autosuggest.md @@ -0,0 +1,1359 @@ +# School Autosuggest Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Suggest schools by name as someone types in the site's main search box, so a parent who knows the school they want reaches it in one step. + +**Architecture:** A dedicated `/api/suggest` endpoint answers from Typesense alone, touching no pandas. The frontend adds an ARIA combobox to the existing `FilterBar` omni-input, behind the `school_autosuggest` flag. First, the rate limiter is fixed to key on the real caller rather than the Next container — without that, autosuggest 429s the site. + +**Tech Stack:** FastAPI, slowapi, Typesense, Next.js 15 App Router, React, Playwright, pytest, Jest. + +**Spec:** `docs/superpowers/specs/2026-08-26-school-autosuggest-design.md` + +## Global Constraints + +- **The rate-limit fix is not flagged.** It is a correctness fix that applies whether or not autosuggest is on. +- **`/api/suggest` is not flagged either.** Only the UI is. A live endpoint with the UI dark is deliberate — it lets the endpoint be smoke-tested before the feature is switched on. +- **The keystroke path never returns an error for ordinary input.** Short query, no matches, Typesense down — all `200` with `{"suggestions": []}`. +- **No DataFrame fallback in `/api/suggest`.** The 25,000-row substring scan `/api/schools` falls back to is exactly the cost this endpoint exists to avoid. +- **`Enter` with no active option must still submit the free-text search**, exactly as today. +- **The client fetch must not use `cache: "no-store"`** — it would discard the browser cache and the ETag 304s the existing middleware already provides. +- **Every flag defaults to `False`**, declared in `backend/flags.py` with a `name`/`description`/`added` triple. +- Backend tests run via: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` +- Never push to `main`. Branch, PR, let checks pass. +- Do not start a local server to test the application; it does not work. + +## File Structure + +| File | Responsibility | +|---|---| +| `backend/app.py` (modify) | `client_key`, the `Limiter` construction, `GET /api/suggest`, one `CACHE_RULES` row. | +| `backend/data_loader.py` (modify) | `suggest_schools_typesense` — returns documents, not URNs. | +| `backend/flags.py` (modify) | Declare `school_autosuggest`. | +| `backend/tests/test_rate_limit_key.py` (new) | Keying precedence and bucket separation. | +| `backend/tests/test_suggest.py` (new) | Endpoint behaviour, including that it never touches the DataFrame. | +| `nextjs-app/lib/suggest.ts` (new) | `fetchSuggestions(q, signal)` — one typed fetch. | +| `nextjs-app/hooks/useSchoolSuggest.ts` (new) | Debounce, abort, stale-response rejection, open/active state. | +| `nextjs-app/components/SuggestList.tsx` (new) | Presentational list + ARIA. No fetching. | +| `nextjs-app/components/SuggestList.module.css` (new) | Dropdown styling. | +| `nextjs-app/components/FilterBar.tsx` (modify) | Wire hook + list to the existing input and form. | +| `nextjs-app/components/HomeView.tsx` (modify) | Pass `autosuggest` through to both `FilterBar` instances. | +| `nextjs-app/app/page.tsx` (modify) | Read the flag server-side, pass to both `HomeView` render sites. | +| `e2e/tests/journeys.spec.ts` (modify) | Flag-gated journeys. | + +--- + +### Task 1: Key the rate limiter on the real caller + +**Files:** +- Modify: `backend/app.py` +- Create: `backend/tests/test_rate_limit_key.py` + +**Interfaces:** +- Produces: `backend.app.client_key(request: Request) -> str` + +> Measured on staging before this change: 70 concurrent requests to +> `/api/schools` returned exactly 60 × 200 and 10 × 429. One machine consumed +> the whole site's budget, because `get_remote_address` returns the Next +> container's IP for every browser user. + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/test_rate_limit_key.py`: + +```python +"""The rate-limit bucket must be the caller, not the proxy in front of them. + +`get_remote_address` reads request.client.host. In staging and production the +backend has no published ports and its only caller is the Next proxy, so that +host is the Next container — one bucket for every browser user on the site. +Measured before this fix: 70 concurrent requests, 60 served and 10 refused. +""" + +from starlette.datastructures import Headers + +from backend.app import client_key + + +class _Req: + """Enough of a Request for the key function: headers and a client host.""" + + def __init__(self, headers: dict, host: str = "10.0.0.9"): + self.headers = Headers(headers) + self.client = type("C", (), {"host": host})() + self.scope = {"type": "http", "client": (host, 0), + "headers": [(k.lower().encode(), v.encode()) + for k, v in headers.items()]} + + +def test_cloudflare_header_wins(): + # Cloudflare sets CF-Connecting-IP and overwrites any client-supplied + # value, so it is trustworthy in a way a parsed XFF chain is not. + assert client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) == "203.0.113.7" + + +def test_forwarded_for_is_the_fallback_and_takes_the_first_entry(): + # Left-most is the original client; everything after it is proxies. + assert client_key( + _Req({"x-forwarded-for": "203.0.113.7, 10.0.0.2"})) == "203.0.113.7" + + +def test_remote_address_is_the_last_resort(): + assert client_key(_Req({}, host="10.0.0.9")) == "10.0.0.9" + + +def test_cloudflare_header_beats_forwarded_for(): + key = client_key(_Req({"cf-connecting-ip": "203.0.113.7", + "x-forwarded-for": "198.51.100.1"})) + assert key == "203.0.113.7" + + +def test_two_callers_get_two_buckets(): + # The whole point: one user exhausting their limit must not refuse another. + a = client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) + b = client_key(_Req({"cf-connecting-ip": "203.0.113.8"})) + assert a != b + + +def test_whitespace_is_stripped(): + # "a, b" split on comma leaves a leading space on every entry but the + # first; an unstripped key silently creates a second bucket per client. + assert client_key(_Req({"x-forwarded-for": " 203.0.113.7 ,10.0.0.2"})) \ + == "203.0.113.7" +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_rate_limit_key.py -q` + +Expected: FAIL — `ImportError: cannot import name 'client_key' from 'backend.app'` + +- [ ] **Step 3: Implement the key function** + +In `backend/app.py`, replace: + +```python +# Rate limiter +limiter = Limiter(key_func=get_remote_address) +``` + +with: + +```python +def client_key(request: Request) -> str: + """The rate-limit bucket: the real caller, not the proxy in front of them. + + `get_remote_address` reads request.client.host. In staging and production + the backend has no published ports and sits on the internal network, so its + only caller is the Next proxy — meaning every browser user on the site + shared one bucket. Measured before this fix: 70 concurrent requests to + /api/schools returned 60 OK and 10 refused. + + CF-Connecting-IP first, because Cloudflare (in front of both environments) + sets it on every origin request and *overwrites* any client-supplied value, + which a parsed X-Forwarded-For chain does not guarantee. The XFF fallback is + forgeable, but only by a caller already inside the Docker network, which is + the one place nothing untrusted can reach. + """ + cf = request.headers.get("cf-connecting-ip") + if cf: + return cf.strip() + xff = request.headers.get("x-forwarded-for") + if xff: + return xff.split(",")[0].strip() + return get_remote_address(request) + + +# Rate limiter. No in-app global ceiling: slowapi's default_limits and +# application_limits are both keyed by key_func (so per-client, not global) +# and the latter only applies with SlowAPIMiddleware installed, which this app +# does not use. A global cap belongs at Cloudflare, which is already in the +# path. See the spec's §1 for why that is deliberate. +limiter = Limiter(key_func=client_key) +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_rate_limit_key.py -q` + +Expected: PASS, 6 tests. + +- [ ] **Step 5: Run the full backend suite** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` + +Expected: all pass. + +- [ ] **Step 6: Commit** + +```bash +git add backend/app.py backend/tests/test_rate_limit_key.py +git commit -m "fix(api): rate-limit per caller, not per proxy" +``` + +--- + +### Task 2: `suggest_schools_typesense` + +**Files:** +- Modify: `backend/data_loader.py` +- Create: `backend/tests/test_suggest.py` + +**Interfaces:** +- Produces: `backend.data_loader.suggest_schools_typesense(query: str, limit: int = 8) -> list[dict]`, each dict carrying `urn` (int), `school_name`, `local_authority`, `postcode`, `phase`, `school_type` (all str, `""` when the document omits an optional field). + +> The existing `search_schools_typesense` returns URNs only, which forces the +> caller to hydrate from the DataFrame. Suggestions need the fields Typesense +> already holds — see the schema in `pipeline/scripts/sync_typesense.py`. + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/test_suggest.py`: + +```python +"""Tests for school autosuggest (spec 2026-08-26).""" + +import pytest + +from backend import data_loader + + +class _FakeDocs: + def __init__(self, hits, explode=False): + self._hits = hits + self._explode = explode + self.last_params = None + + def search(self, params): + self.last_params = params + if self._explode: + raise RuntimeError("typesense is down") + return {"hits": [{"document": d} for d in self._hits]} + + +class _FakeClient: + def __init__(self, hits, explode=False): + self.docs = _FakeDocs(hits, explode) + self.collections = {"schools": type("C", (), {"documents": self.docs})()} + + +_HIT = { + "urn": 100010, "school_name": "Brecknock Primary School", + "local_authority": "Camden", "postcode": "NW1 1AA", + "phase": "Primary", "school_type": "Community school", +} + + +def _use(monkeypatch, client): + monkeypatch.setattr(data_loader, "_get_typesense_client", lambda: client) + + +def test_returns_the_fields_a_suggestion_needs(monkeypatch): + # Local authority is not decoration: there are many schools called + # "St Mary's", and a list without it cannot be chosen between. + _use(monkeypatch, _FakeClient([_HIT])) + out = data_loader.suggest_schools_typesense("breck") + assert out == [{ + "urn": 100010, "school_name": "Brecknock Primary School", + "local_authority": "Camden", "postcode": "NW1 1AA", + "phase": "Primary", "school_type": "Community school", + }] + + +def test_a_missing_optional_field_becomes_an_empty_string(monkeypatch): + # phase and school_type are optional in the Typesense schema. A missing + # key must not KeyError in the keystroke path. + _use(monkeypatch, _FakeClient([{"urn": 1, "school_name": "X", + "local_authority": "Y", "postcode": "Z"}])) + out = data_loader.suggest_schools_typesense("x") + assert out[0]["phase"] == "" and out[0]["school_type"] == "" + + +def test_typesense_unavailable_gives_no_suggestions_rather_than_raising(monkeypatch): + _use(monkeypatch, None) + assert data_loader.suggest_schools_typesense("anything") == [] + + +def test_a_typesense_error_gives_no_suggestions_rather_than_raising(monkeypatch): + _use(monkeypatch, _FakeClient([], explode=True)) + assert data_loader.suggest_schools_typesense("anything") == [] + + +def test_the_limit_is_passed_through_and_clamped(monkeypatch): + client = _FakeClient([]) + _use(monkeypatch, client) + data_loader.suggest_schools_typesense("x", limit=500) + assert client.docs.last_params["per_page"] == 20 +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_suggest.py -q` + +Expected: FAIL — `AttributeError: module 'backend.data_loader' has no attribute 'suggest_schools_typesense'` + +- [ ] **Step 3: Implement it** + +In `backend/data_loader.py`, directly below `search_schools_typesense`: + +```python +# The most a public endpoint will return in one response. +SUGGEST_MAX_LIMIT = 20 + +# Fields a suggestion row carries, and the default when the document omits an +# optional one. phase and school_type are optional in the Typesense schema. +_SUGGEST_FIELDS = ("school_name", "local_authority", "postcode", + "phase", "school_type") + + +def suggest_schools_typesense(query: str, limit: int = 8) -> List[dict]: + """Autosuggest rows straight from Typesense. Never raises. + + Returns documents rather than URNs, unlike search_schools_typesense, so the + caller needs no DataFrame. Every field below is already in the index — see + pipeline/scripts/sync_typesense.py — which is what makes this cheap enough + to run per keystroke. + """ + client = _get_typesense_client() + if client is None: + return [] + try: + result = client.collections["schools"].documents.search({ + "q": query, + "query_by": "school_name,local_authority", + "per_page": max(1, min(limit, SUGGEST_MAX_LIMIT)), + "typo_tokens_threshold": 1, + }) + except Exception: + # A dropdown that quietly stops appearing is the right failure here. + return [] + + rows = [] + for hit in result.get("hits", []): + doc = hit.get("document", {}) + row = {"urn": int(doc.get("urn", 0))} + row.update({f: str(doc.get(f, "") or "") for f in _SUGGEST_FIELDS}) + rows.append(row) + return rows +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_suggest.py -q` + +Expected: PASS, 5 tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/data_loader.py backend/tests/test_suggest.py +git commit -m "feat(suggest): Typesense rows for autosuggest, no DataFrame" +``` + +--- + +### Task 3: `GET /api/suggest` + +**Files:** +- Modify: `backend/app.py` +- Modify: `backend/tests/test_suggest.py` + +**Interfaces:** +- Consumes: `backend.data_loader.suggest_schools_typesense(query, limit) -> list[dict]` +- Produces: `GET /api/suggest?q=&limit=` → `200 {"suggestions": [...]}` + +- [ ] **Step 1: Write the failing tests** + +Append to `backend/tests/test_suggest.py`: + +```python +def _client(monkeypatch, rows, *, blow_up_dataframe=False): + from fastapi.testclient import TestClient + from backend import app as app_module + + monkeypatch.setattr(app_module, "suggest_schools_typesense", + lambda q, limit=8: rows) + if blow_up_dataframe: + def _boom(): + raise AssertionError("the suggest path must not load the DataFrame") + monkeypatch.setattr(app_module, "load_school_data", _boom) + monkeypatch.setattr(app_module, "load_latest_school_data", _boom) + return TestClient(app_module.app, raise_server_exceptions=False) + + +def test_the_endpoint_returns_suggestions(monkeypatch): + body = _client(monkeypatch, [_HIT]).get("/api/suggest?q=breck").json() + assert body["suggestions"][0]["school_name"] == "Brecknock Primary School" + + +def test_the_endpoint_never_touches_the_dataframe(monkeypatch): + """The whole reason this is not a mode of /api/schools. + + That endpoint filters and sorts 25,000 rows of pandas per query, holding + the GIL. Per keystroke, that is the cost this endpoint exists to avoid. + """ + res = _client(monkeypatch, [_HIT], blow_up_dataframe=True).get("/api/suggest?q=breck") + assert res.status_code == 200 + assert res.json()["suggestions"] + + +def test_a_one_character_query_returns_nothing_and_does_not_error(monkeypatch): + # The keystroke path never errors on ordinary input. + res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=b") + assert res.status_code == 200 + assert res.json() == {"suggestions": []} + + +def test_a_blank_query_returns_nothing_and_does_not_error(monkeypatch): + res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=") + assert res.status_code == 200 + assert res.json() == {"suggestions": []} + + +def test_typesense_down_is_an_empty_list_not_a_500(monkeypatch): + res = _client(monkeypatch, []).get("/api/suggest?q=breck") + assert res.status_code == 200 + assert res.json() == {"suggestions": []} + + +def test_the_response_is_cacheable(monkeypatch): + # Prefix queries repeat enormously across users, and school names change + # once a year. Without this the endpoint pays full price every keystroke. + res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=breck") + assert "s-maxage" in res.headers.get("cache-control", "") + assert res.headers.get("etag") +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_suggest.py -q` + +Expected: FAIL — the endpoint 404s, so `body["suggestions"]` raises `KeyError`. + +- [ ] **Step 3: Import the helper** + +In `backend/app.py`, add `suggest_schools_typesense` to the existing +`from .data_loader import (...)` block, alongside `search_schools_typesense`. + +- [ ] **Step 4: Add the cache rule** + +In `backend/app.py`, add to `CACHE_RULES` — before the `/api/schools` row, so +the longest-prefix match is unaffected: + +```python + ("/api/suggest", (60, 3600, 86400)), +``` + +- [ ] **Step 5: Add the endpoint** + +In `backend/app.py`, directly above `@app.get("/api/flags")`: + +```python +# Two characters. One is not a query — it matches thousands of schools and the +# response is useless, so it is not worth a round trip. +SUGGEST_MIN_QUERY = 2 + + +@app.get("/api/suggest") +@limiter.limit("120/minute") +async def suggest_schools( + request: Request, + q: str = Query("", max_length=100), + limit: int = Query(8, ge=1, le=20), +): + """School name suggestions, from Typesense alone. + + Deliberately not a mode of /api/schools: that path filters and sorts the + full in-memory DataFrame, which is far too expensive to run per keystroke. + + Nothing here returns an error for ordinary input. A short query, no + matches, or Typesense being unreachable are all 200 with an empty list — + a dropdown that quietly does not appear is the right failure for a + keystroke path, and there is no DataFrame fallback because the 25,000-row + substring scan is precisely what this endpoint exists to avoid. + + 120/minute rather than the default 60: a 200 ms debounce makes typing + legitimately bursty. + """ + query = q.strip() + if len(query) < SUGGEST_MIN_QUERY: + return {"suggestions": []} + return {"suggestions": suggest_schools_typesense(query, limit)} +``` + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` + +Expected: all pass. + +- [ ] **Step 7: Commit** + +```bash +git add backend/app.py backend/tests/test_suggest.py +git commit -m "feat(suggest): GET /api/suggest, cacheable and DataFrame-free" +``` + +--- + +### Task 4: The client and the hook + +**Files:** +- Create: `nextjs-app/lib/suggest.ts` +- Create: `nextjs-app/hooks/useSchoolSuggest.ts` +- Create: `nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx` + +**Interfaces:** +- Produces: + - `export interface Suggestion { urn: number; school_name: string; local_authority: string; postcode: string; phase: string; school_type: string }` + - `fetchSuggestions(q: string, signal?: AbortSignal): Promise` + - `useSchoolSuggest(query: string, enabled: boolean): { suggestions: Suggestion[]; open: boolean; activeIndex: number; setActiveIndex: (i: number) => void; close: () => void }` + +- [ ] **Step 1: Write `lib/suggest.ts`** + +```typescript +/** + * Client for /api/suggest. + * + * No `cache: "no-store"`. The compare modal's search uses it, and copying that + * here would discard both the browser cache and the ETag 304s the backend's + * CacheAndETagMiddleware already provides — on the one endpoint where prefix + * queries repeat most. + */ + +export interface Suggestion { + urn: number; + school_name: string; + local_authority: string; + postcode: string; + phase: string; + school_type: string; +} + +/** Below this the response is thousands of schools and worth no round trip. */ +export const SUGGEST_MIN_QUERY = 2; + +const API = process.env.NEXT_PUBLIC_API_URL || '/api'; + +/** Suggestions for `q`. Never throws: no suggestions is a fine outcome. */ +export async function fetchSuggestions( + q: string, signal?: AbortSignal, +): Promise { + if (q.trim().length < SUGGEST_MIN_QUERY) return []; + try { + const res = await fetch(`${API}/suggest?q=${encodeURIComponent(q.trim())}`, + { signal }); + if (!res.ok) return []; + const body = await res.json(); + return body.suggestions ?? []; + } catch { + // Includes AbortError, which is the normal path on every keystroke. + return []; + } +} +``` + +- [ ] **Step 2: Write the failing hook test** + +Create `nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx`: + +```tsx +import { renderHook, act, waitFor } from '@testing-library/react'; +import { useSchoolSuggest } from '@/hooks/useSchoolSuggest'; + +const realFetch = global.fetch; + +function mockFetch(rows: unknown[], delayMs = 0) { + global.fetch = jest.fn(async (_url: unknown, init?: { signal?: AbortSignal }) => { + if (delayMs) { + await new Promise((resolve, reject) => { + const t = setTimeout(resolve, delayMs); + init?.signal?.addEventListener('abort', () => { + clearTimeout(t); + reject(Object.assign(new Error('aborted'), { name: 'AbortError' })); + }); + }); + } + return { ok: true, json: async () => ({ suggestions: rows }) }; + }) as unknown as typeof fetch; +} + +const ROW = { + urn: 1, school_name: 'Brecknock Primary School', local_authority: 'Camden', + postcode: 'NW1 1AA', phase: 'Primary', school_type: 'Community school', +}; + +describe('useSchoolSuggest', () => { + beforeEach(() => { jest.useFakeTimers(); }); + afterEach(() => { jest.useRealTimers(); global.fetch = realFetch; }); + + it('does not fetch below the minimum query length', () => { + mockFetch([ROW]); + renderHook(() => useSchoolSuggest('b', true)); + act(() => { jest.advanceTimersByTime(500); }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('does not fetch at all when disabled', () => { + // The flag being off must mean no request, not a hidden dropdown. + mockFetch([ROW]); + renderHook(() => useSchoolSuggest('brecknock', false)); + act(() => { jest.advanceTimersByTime(500); }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('debounces rather than firing per keystroke', () => { + mockFetch([ROW]); + const { rerender } = renderHook( + ({ q }) => useSchoolSuggest(q, true), { initialProps: { q: 'br' } }); + rerender({ q: 'bre' }); + rerender({ q: 'brec' }); + act(() => { jest.advanceTimersByTime(199); }); + expect(global.fetch).not.toHaveBeenCalled(); + act(() => { jest.advanceTimersByTime(2); }); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('opens with results once they arrive', async () => { + mockFetch([ROW]); + const { result } = renderHook(() => useSchoolSuggest('brecknock', true)); + act(() => { jest.advanceTimersByTime(200); }); + await waitFor(() => expect(result.current.suggestions).toHaveLength(1)); + expect(result.current.open).toBe(true); + }); + + it('close() hides the list without clearing the query', async () => { + mockFetch([ROW]); + const { result } = renderHook(() => useSchoolSuggest('brecknock', true)); + act(() => { jest.advanceTimersByTime(200); }); + await waitFor(() => expect(result.current.open).toBe(true)); + act(() => { result.current.close(); }); + expect(result.current.open).toBe(false); + }); +}); +``` + +- [ ] **Step 3: Run it to verify it fails** + +Run: `cd nextjs-app && npx jest __tests__/hooks/useSchoolSuggest.test.tsx` + +Expected: FAIL — `Cannot find module '@/hooks/useSchoolSuggest'` + +- [ ] **Step 4: Write the hook** + +Create `nextjs-app/hooks/useSchoolSuggest.ts`: + +```typescript +'use client'; + +import { useEffect, useRef, useState } from 'react'; +import { fetchSuggestions, SUGGEST_MIN_QUERY, type Suggestion } from '@/lib/suggest'; + +/* + * Long enough that a fast typist does not fire a request per character, short + * enough that the list feels attached to the keyboard. + */ +const DEBOUNCE_MS = 200; + +export function useSchoolSuggest(query: string, enabled: boolean) { + const [suggestions, setSuggestions] = useState([]); + const [open, setOpen] = useState(false); + const [activeIndex, setActiveIndex] = useState(-1); + // Set when the user dismisses the list, so a re-render does not reopen it. + const dismissed = useRef(''); + + useEffect(() => { + const q = query.trim(); + if (!enabled || q.length < SUGGEST_MIN_QUERY || dismissed.current === q) { + setSuggestions([]); + setOpen(false); + return; + } + + /* + * Abort the superseded request on every keystroke. This is correctness, + * not economy: without it a slow response for "st" can land after the fast + * one for "st marys" and replace a correct list with a stale one. + */ + const controller = new AbortController(); + const timer = setTimeout(async () => { + const rows = await fetchSuggestions(q, controller.signal); + if (controller.signal.aborted) return; + setSuggestions(rows); + setActiveIndex(-1); + setOpen(rows.length > 0); + }, DEBOUNCE_MS); + + return () => { + clearTimeout(timer); + controller.abort(); + }; + }, [query, enabled]); + + return { + suggestions, + open, + activeIndex, + setActiveIndex, + close: () => { + dismissed.current = query.trim(); + setOpen(false); + setActiveIndex(-1); + }, + }; +} +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cd nextjs-app && npx jest __tests__/hooks/useSchoolSuggest.test.tsx && npx tsc --noEmit` + +Expected: PASS, 5 tests, typecheck clean. + +- [ ] **Step 6: Commit** + +```bash +git add nextjs-app/lib/suggest.ts nextjs-app/hooks/useSchoolSuggest.ts nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx +git commit -m "feat(suggest): debounced, abortable suggestion hook" +``` + +--- + +### Task 5: The list, with its ARIA + +**Files:** +- Create: `nextjs-app/components/SuggestList.tsx` +- Create: `nextjs-app/components/SuggestList.module.css` +- Create: `nextjs-app/__tests__/components/SuggestList.test.tsx` + +**Interfaces:** +- Consumes: `Suggestion` from `@/lib/suggest` +- Produces: ``, and `suggestOptionId(id: string, index: number): string` + +- [ ] **Step 1: Write the failing test** + +Create `nextjs-app/__tests__/components/SuggestList.test.tsx`: + +```tsx +import { render, screen } from '@testing-library/react'; +import { SuggestList, suggestOptionId } from '@/components/SuggestList'; + +const ROWS = [ + { urn: 1, school_name: "St Mary's Primary", local_authority: 'Camden', + postcode: 'NW1 1AA', phase: 'Primary', school_type: 'Voluntary aided school' }, + { urn: 2, school_name: "St Mary's Primary", local_authority: 'Barnet', + postcode: 'EN5 2AA', phase: 'Primary', school_type: 'Community school' }, +]; + +describe('SuggestList', () => { + it('is a listbox of options', () => { + render( {}} onHover={() => {}} />); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + expect(screen.getAllByRole('option')).toHaveLength(2); + }); + + it('shows the local authority, which is what tells two schools apart', () => { + // Both rows are "St Mary's Primary". Without the authority the list is + // unusable for exactly the query autosuggest exists to serve. + render( {}} onHover={() => {}} />); + expect(screen.getByText('Camden')).toBeInTheDocument(); + expect(screen.getByText('Barnet')).toBeInTheDocument(); + }); + + it('marks only the active option selected', () => { + render( {}} onHover={() => {}} />); + const options = screen.getAllByRole('option'); + expect(options[0]).toHaveAttribute('aria-selected', 'false'); + expect(options[1]).toHaveAttribute('aria-selected', 'true'); + }); + + it('gives each option the id the input will point at', () => { + // aria-activedescendant on the input has to name a real element id, or + // a screen reader announces nothing as the user arrows through. + render( {}} onHover={() => {}} />); + expect(screen.getAllByRole('option')[0]).toHaveAttribute( + 'id', suggestOptionId('s', 0)); + }); + + it('renders nothing when there is nothing to suggest', () => { + const { container } = render( {}} onHover={() => {}} />); + expect(container).toBeEmptyDOMElement(); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd nextjs-app && npx jest __tests__/components/SuggestList.test.tsx` + +Expected: FAIL — `Cannot find module '@/components/SuggestList'` + +- [ ] **Step 3: Write the component** + +Create `nextjs-app/components/SuggestList.tsx`: + +```tsx +'use client'; + +/** + * The autosuggest dropdown. Presentational only — it fetches nothing and owns + * no state, so the fetching rules and the ARIA rules can be read separately. + */ + +import type { Suggestion } from '@/lib/suggest'; +import styles from './SuggestList.module.css'; + +/** The id the input's aria-activedescendant points at. */ +export function suggestOptionId(id: string, index: number): string { + return `${id}-option-${index}`; +} + +interface Props { + /** Shared with the input's aria-controls. */ + id: string; + suggestions: Suggestion[]; + activeIndex: number; + onPick: (s: Suggestion) => void; + onHover: (index: number) => void; +} + +export function SuggestList({ id, suggestions, activeIndex, onPick, onHover }: Props) { + if (suggestions.length === 0) return null; + + return ( +
    + {suggestions.map((s, i) => ( +
  • { e.preventDefault(); onPick(s); }} + onMouseEnter={() => onHover(i)} + > + {s.school_name} + {/* Not decoration: there are many "St Mary's". */} + {s.local_authority} +
    • + ))} +
+ ); +} +``` + +- [ ] **Step 4: Write the stylesheet** + +Create `nextjs-app/components/SuggestList.module.css`: + +```css +/* Anchored to the search field's wrapper, which is position: relative. */ +.list { + position: absolute; + top: calc(100% + 4px); + left: 0; + right: 0; + z-index: 40; + margin: 0; + padding: 4px; + list-style: none; + max-height: 320px; + overflow-y: auto; + background: var(--color-surface, #fff); + border: 1px solid var(--color-border, #d8d8d8); + border-radius: 10px; + box-shadow: 0 10px 30px rgb(0 0 0 / 12%); +} + +.option { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 12px; + padding: 10px 12px; + border-radius: 6px; + cursor: pointer; +} + +/* Hover and keyboard share one style: the active option is the active option + however it became active. */ +.option:hover, +.active { + background: var(--color-surface-hover, #f1f1f1); +} + +.name { + font-weight: 500; +} + +.meta { + font-size: 0.85em; + color: var(--color-text-muted, #666); + white-space: nowrap; +} +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cd nextjs-app && npx jest __tests__/components/SuggestList.test.tsx && npx tsc --noEmit` + +Expected: PASS, 5 tests, typecheck clean. + +- [ ] **Step 6: Commit** + +```bash +git add nextjs-app/components/SuggestList.tsx nextjs-app/components/SuggestList.module.css nextjs-app/__tests__/components/SuggestList.test.tsx +git commit -m "feat(suggest): the dropdown, with combobox ARIA" +``` + +--- + +### Task 6: Wire it into the search box, behind the flag + +**Files:** +- Modify: `backend/flags.py` +- Modify: `nextjs-app/app/page.tsx` +- Modify: `nextjs-app/components/HomeView.tsx` +- Modify: `nextjs-app/components/FilterBar.tsx` +- Modify: `nextjs-app/components/FilterBar.module.css` +- Create: `nextjs-app/__tests__/components/FilterBarSuggest.test.tsx` + +**Interfaces:** +- Consumes: `useSchoolSuggest`, `SuggestList`, `suggestOptionId`, `schoolUrl` from `@/lib/utils`, `getFlags` from `@/lib/flags`. + +- [ ] **Step 1: Declare the flag** + +In `backend/flags.py`, add to the `REGISTRY` tuple, after the +`admission_distance` entry: + +```python + Flag( + name="school_autosuggest", + description=( + "School name suggestions as you type in the main search box." + ), + added=date(2026, 8, 26), + ), +``` + +- [ ] **Step 2: Write the failing test** + +Create `nextjs-app/__tests__/components/FilterBarSuggest.test.tsx`: + +```tsx +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { FilterBar } from '@/components/FilterBar'; + +const push = jest.fn(); +jest.mock('next/navigation', () => ({ + useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }), + usePathname: () => '/', + useSearchParams: () => new URLSearchParams(), +})); + +const FILTERS = { + local_authorities: [], school_types: [], years: [], phases: [], + genders: [], admissions_policies: [], +}; + +const realFetch = global.fetch; +beforeEach(() => { + global.fetch = jest.fn(async () => ({ + ok: true, + json: async () => ({ suggestions: [{ + urn: 100010, school_name: 'Brecknock Primary School', + local_authority: 'Camden', postcode: 'NW1 1AA', + phase: 'Primary', school_type: 'Community school' }] }), + })) as unknown as typeof fetch; + push.mockClear(); +}); +afterEach(() => { global.fetch = realFetch; }); + +describe('FilterBar autosuggest', () => { + it('is a combobox only when the flag is on', () => { + const { rerender } = render(); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + rerender(); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + }); + + it('makes no request while the flag is off', async () => { + // Off means off: no listener, no fetch, no markup. + render(); + await userEvent.type(screen.getByPlaceholderText(/School name or postcode/i), + 'brecknock'); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('shows suggestions and navigates when one is chosen', async () => { + render(); + await userEvent.type(screen.getByRole('combobox'), 'brecknock'); + const option = await screen.findByRole('option', { name: /Brecknock/ }); + await userEvent.click(option); + expect(push).toHaveBeenCalledWith( + expect.stringContaining('/school/100010')); + }); + + it('suppresses suggestions once the value is a postcode', async () => { + // The box takes a name OR a postcode; suggestions must get out of the way. + // + // fireEvent.change, not userEvent.type: typing sets "N", "NW", "NW1"... and + // "NW1" is not a postcode, so a request for it is correct behaviour. Only + // the settled value is the assertion, so set it in one go. + render(); + fireEvent.change(screen.getByRole('combobox'), { target: { value: 'NW1 1AA' } }); + await new Promise((r) => setTimeout(r, 300)); // past the 200ms debounce + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('Enter with no active option still submits the free-text search', async () => { + // The existing behaviour is preserved, not replaced. + render(); + const input = screen.getByRole('combobox'); + await userEvent.type(input, 'brecknock{Enter}'); + // updateURL pushes inside startTransition, so the call is not synchronous. + await waitFor(() => expect(push).toHaveBeenCalledWith( + expect.stringContaining('search=brecknock'))); + }); +}); +``` + +- [ ] **Step 3: Run it to verify it fails** + +Run: `cd nextjs-app && npx jest __tests__/components/FilterBarSuggest.test.tsx` + +Expected: FAIL — `autosuggest` is not a prop, so no combobox is rendered. + +- [ ] **Step 4: Wire the hook into FilterBar** + +In `nextjs-app/components/FilterBar.tsx`: + +Add to the imports: + +```typescript +import { useSchoolSuggest } from "@/hooks/useSchoolSuggest"; +import { SuggestList, suggestOptionId } from "./SuggestList"; +import { schoolUrl } from "@/lib/utils"; +import type { Suggestion } from "@/lib/suggest"; +``` + +Add to `FilterBarProps`: + +```typescript + /** Server-read feature flag. Off means no listener, no fetch, no markup. */ + autosuggest?: boolean; +``` + +Add `autosuggest = false,` to the destructured parameters of `FilterBar`. + +Inside the component, after the `omniValue` state declaration: + +```typescript + const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`; + // Suppressed once the value parses as a postcode: the box takes a school + // name OR a postcode, and suggesting schools during postcode entry fights + // the user rather than helping them. + const suggestEnabled = autosuggest && !isValidPostcode(omniValue); + const { suggestions, open, activeIndex, setActiveIndex, close } = + useSchoolSuggest(omniValue, suggestEnabled); + + const pickSuggestion = (s: Suggestion) => { + close(); + track('search_submitted', { + query: s.school_name.toLowerCase(), + via: 'suggestion', + urn: s.urn, + has_postcode: false, + filters_active: '', + filters_count: 0, + }); + router.push(schoolUrl(s.urn, s.school_name)); + }; + + const handleOmniKeyDown = (e: React.KeyboardEvent) => { + if (!open) return; + if (e.key === "ArrowDown") { + e.preventDefault(); + setActiveIndex(activeIndex + 1 >= suggestions.length ? 0 : activeIndex + 1); + } else if (e.key === "ArrowUp") { + e.preventDefault(); + setActiveIndex(activeIndex <= 0 ? suggestions.length - 1 : activeIndex - 1); + } else if (e.key === "Escape") { + close(); + } else if (e.key === "Enter" && activeIndex >= 0) { + // Only when an option is active. With none, the event falls through to + // the form's submit handler and searches the typed text, as it does now. + e.preventDefault(); + pickSuggestion(suggestions[activeIndex]); + } + }; +``` + +- [ ] **Step 5: Wire the markup** + +In `nextjs-app/components/FilterBar.tsx`, replace the existing omni `` +element with: + +```tsx + setOmniValue(e.target.value)} + onKeyDown={handleOmniKeyDown} + onBlur={close} + placeholder="School name or postcode" + className={styles.omniInput} + {...(autosuggest ? { + role: "combobox", + "aria-expanded": open, + "aria-controls": suggestId, + "aria-autocomplete": "list" as const, + "aria-activedescendant": + activeIndex >= 0 ? suggestOptionId(suggestId, activeIndex) : undefined, + autoComplete: "off", + } : {})} + /> +``` + +And directly after the closing `` of the search submit button, still +inside the wrapper element that holds the input: + +```tsx + {autosuggest && open && ( + + )} +``` + +- [ ] **Step 6: Anchor the dropdown** + +The list is `position: absolute`, so its nearest positioned ancestor must be +the wrapper around the input and the submit button. That is +`.omniBoxContainer`, and it **does not currently declare `position: relative`** +— without this the dropdown anchors to the page and lands in the wrong place. + +In `nextjs-app/components/FilterBar.module.css`, change: + +```css +.omniBoxContainer { + display: flex; + align-items: center; + gap: 0.5rem; +} +``` + +to: + +```css +.omniBoxContainer { + display: flex; + align-items: center; + gap: 0.5rem; + /* The suggestion dropdown is absolutely positioned against this box. */ + position: relative; +} +``` + +- [ ] **Step 7: Thread the flag from the server** + +In `nextjs-app/app/page.tsx`, add the import: + +```typescript +import { getFlags } from '@/lib/flags'; +``` + +Inside `HomePage`, before the `try`: + +```typescript + // Server-read: no flag value reaches the browser bundle. + const flags = await getFlags(); + const autosuggest = flags.school_autosuggest === true; +``` + +Add `autosuggest={autosuggest}` to **both** `` render sites — +the success path and the `catch` fallback. Missing the second means the flag +silently does nothing whenever the API call fails. + +In `nextjs-app/components/HomeView.tsx`, add `autosuggest?: boolean` to its +props interface, destructure it with a default of `false`, and pass +`autosuggest={autosuggest}` to **both** `` instances — hero +and sticky. + +- [ ] **Step 8: Run the tests to verify they pass** + +Run: `cd nextjs-app && npx jest && npx tsc --noEmit && npm run build` + +Expected: all pass, typecheck clean, build green. + +- [ ] **Step 9: Run the backend suite** + +The flag registry gained an entry, so the registry tests must still pass. + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` + +Expected: all pass. + +- [ ] **Step 10: Commit** + +```bash +git add backend/flags.py nextjs-app/app/page.tsx nextjs-app/components/HomeView.tsx nextjs-app/components/FilterBar.tsx nextjs-app/components/FilterBar.module.css nextjs-app/__tests__/components/FilterBarSuggest.test.tsx +git commit -m "feat(suggest): wire autosuggest into the search box behind a flag" +``` + +--- + +### Task 7: E2E journeys + +**Files:** +- Modify: `e2e/tests/journeys.spec.ts` + +- [ ] **Step 1: Write the journeys** + +`/api/flags` is denied to the public, so feature state is read from its +observable effect — the same approach the distance journeys use. + +Append to `e2e/tests/journeys.spec.ts`: + +```typescript +/* + * School autosuggest (spec 2026-08-26). + */ +async function autosuggestIsOn(page: Page): Promise { + await page.goto('/'); + return (await page.getByRole('combobox').count()) > 0; +} + +test('the suggest endpoint answers from Typesense', async ({ page }) => { + // Not flagged — the endpoint is live even while the UI is dark, so it can + // be smoke-tested before the feature is switched on. + const res = await page.request.get('/api/suggest?q=brecknock'); + expect(res.ok()).toBeTruthy(); + const { suggestions } = await res.json(); + expect(Array.isArray(suggestions)).toBeTruthy(); + if (suggestions.length) { + // Local authority is what tells two "St Mary's" apart. + expect(suggestions[0]).toHaveProperty('school_name'); + expect(suggestions[0]).toHaveProperty('local_authority'); + } +}); + +test('a one-character query is answered, not rejected', async ({ page }) => { + // The keystroke path never errors on ordinary input. + const res = await page.request.get('/api/suggest?q=b'); + expect(res.status()).toBe(200); + expect((await res.json()).suggestions).toEqual([]); +}); + +test('the suggest response is cacheable', async ({ page }) => { + const res = await page.request.get('/api/suggest?q=brecknock'); + expect(res.headers()['cache-control'] ?? '').toContain('s-maxage'); +}); + +test('typing a school name suggests it, and choosing it opens that school', async ({ page }) => { + test.skip(!(await autosuggestIsOn(page)), + 'the school_autosuggest flag is off in this environment'); + + // A school certain to exist in any environment with data. + const { schools } = await (await page.request.get('/api/schools?page_size=1')).json(); + test.skip(!schools?.length, 'no schools in this environment'); + const name = schools[0].school_name as string; + + await page.goto('/'); + await page.getByRole('combobox').first().fill(name.slice(0, 12)); + const option = page.getByRole('option').first(); + await expect(option).toBeVisible(); + await option.click(); + await expect(page).toHaveURL(/\/school\/\d+/); +}); + +test('with autosuggest off, the search box is a plain input', async ({ page }) => { + test.skip(await autosuggestIsOn(page), + 'the school_autosuggest flag is on in this environment'); + + await page.goto('/'); + await expect(page.getByRole('combobox')).toHaveCount(0); + // And the box still works: the existing search must be untouched. + await page.getByPlaceholder(/School name or postcode/i).first().fill('abbey'); + await page.getByRole('button', { name: /Search/i }).first().click(); + await expect(page).toHaveURL(/search=abbey/); +}); +``` + +- [ ] **Step 2: Verify the spec compiles and the tests are collected** + +Run: `cd e2e && npx playwright test --list` + +Expected: the five new titles appear; total rises by 5. + +- [ ] **Step 3: Commit** + +```bash +git add e2e/tests/journeys.spec.ts +git commit -m "test(e2e): autosuggest journeys, gated on the flag" +``` + +--- + +### Task 8: Verify and open the PR + +- [ ] **Step 1: Run everything** + +```bash +uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q +cd nextjs-app && npx jest && npx tsc --noEmit && npm run build +cd ../e2e && npx playwright test --list +``` + +Expected: backend green, frontend green, typecheck clean, build green, e2e collects. + +- [ ] **Step 2: Open the PR against `main`** + +The body must state: the rate-limit keying change applies unflagged and +site-wide; autosuggest itself is off until toggled in Unleash; and that +`/api/suggest` is live regardless, deliberately. + +## Notes for the executor + +**Task 1 changes rate limiting for every endpoint.** It is the one change here +that is not behind a flag, and it is the one worth the most review attention. +Before it, everyone shares one 60/minute bucket; after it, each caller gets +their own, bounded by a global ceiling. The header it keys on is only +trustworthy for traffic that actually passed through Cloudflare, which this +app cannot verify — closing that needs Authenticated Origin Pulls or an origin +firewall, and is the most valuable follow-up in the spec's Risks. + +**The in-app global ceiling is required, and slowapi cannot express it.** +Code review found that `client_key` trusts `CF-Connecting-IP` with no way to +verify the request reached the origin through Cloudflare — so rotating that +header mints a fresh bucket per request and defeats per-client limits entirely, +which is worse against abuse than the shared bucket it replaced. The ceiling +(`GlobalRateLimitMiddleware`) bounds that, and is written by hand because +slowapi's `default_limits` and `application_limits` are both keyed by +`key_func` — per-client, not global — and the latter needs `SlowAPIMiddleware`, +which this app does not install. See spec §1.1. + +**`onMouseDown`, not `onClick`, on the options.** The input's `onBlur` closes +the list and blur fires first, so a click handler never runs. This is the +classic bug where the dropdown works by keyboard and is dead to the mouse. + +**Both render sites, twice over.** `page.tsx` renders `HomeView` in the success +path *and* the catch fallback; `HomeView` renders `FilterBar` as hero *and* +sticky. Missing any of the four makes the flag silently do nothing somewhere. diff --git a/docs/superpowers/specs/2026-08-26-school-autosuggest-design.md b/docs/superpowers/specs/2026-08-26-school-autosuggest-design.md new file mode 100644 index 0000000..bc79dad --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-school-autosuggest-design.md @@ -0,0 +1,294 @@ +# School Autosuggest — Design + +**Date:** 2026-08-26 +**Status:** approved for planning +**Depends on:** the feature-flag layer (PR #125, merged) + +## Goal + +Suggest schools by name as someone types in the site's main search box, so a +parent who knows the school they want reaches it in one step instead of +searching, scanning a result list, and clicking. + +Scope is **schools only**. Places and postcodes were considered and excluded — +see *Out of scope*. + +## The finding that shapes everything + +The site's rate limiter does not do what it looks like it does. + +`limiter = Limiter(key_func=get_remote_address)` with `60/minute` reads +`request.client.host`. In staging and production the backend has no published +ports and sits on the internal `backend` network, so its only caller is the +Next proxy — and `request.client.host` is therefore **the Next container**, for +every browser user on the site. + +Measured against staging: 70 concurrent requests to `/api/schools` returned +**60 × 200 and 10 × 429**. One machine consumed the whole site's budget for +that minute. + +Autosuggest is the worst possible feature to build on that. One person typing +"st marys primary" produces six to eight debounced requests; **eight concurrent +searchers would 429 the site.** The compare modal's search-as-you-type already +shares this bucket, so the exposure exists today — autosuggest makes it +certain. + +Fixing the keying is therefore part of this work, not a follow-up. + +## 1. Rate-limit keying + +Both environments sit behind Cloudflare (`server: cloudflare`, `cf-ray` present +on staging and production). Cloudflare sets `CF-Connecting-IP` on every request +to the origin and **overwrites any client-supplied value**, which makes it +trustworthy in a way a parsed `X-Forwarded-For` chain is not. + +```python +def client_key(request: Request) -> str: + """Rate-limit bucket: the real caller, not the proxy in front of them.""" + cf = request.headers.get("cf-connecting-ip") + if cf: + return cf.strip() + xff = request.headers.get("x-forwarded-for") + if xff: + return xff.split(",")[0].strip() + return get_remote_address(request) +``` + +`nextjs-app/app/api/[...path]/route.ts` already forwards every inbound header +except `host` and `connection`, so `CF-Connecting-IP` reaches the backend with +no proxy change. + +**This header is trustworthy only for traffic that actually passed through +Cloudflare, and nothing in the application can verify that it did.** An earlier +draft of this section claimed Cloudflare "replaces the header, so a browser +cannot forge it", and that only the `X-Forwarded-For` fallback was forgeable. +That was wrong. Cloudflare does overwrite the header *on requests it handles* — +but a caller reaching the origin directly sets whatever it likes, and this +process cannot distinguish an edge-set header from an attacker-set one. Both +headers are equally forgeable in that scenario. + +The consequence is sharper than a weakened defence. An attacker rotating +`CF-Connecting-IP` per request mints a fresh rate-limit bucket every time and +evades per-client limits entirely — including on the DataFrame-heavy +`/api/schools`. Against abuse that is *worse* than the shared bucket it +replaced, which at least capped everyone at 60/minute together. + +Two mitigations, and they are not interchangeable: + +1. **The real fix is at Cloudflare** — Authenticated Origin Pulls, or an origin + firewall that refuses connections not from Cloudflare's ranges. Only the + edge can vouch for its own header. This is infrastructure work and is not + part of this change; it is the thing that makes the header mean anything. +2. **The ceiling in §1.1 bounds what evading the keying can achieve** while + that remains open. It does not make the header trustworthy — it makes + trusting it survivable. + +The backend being unreachable from outside the Docker network is a real second +layer, but it depends on the ingress path in front of the frontend, which this +design does not control and should not assume. + +### 1.1 The ceiling, which is back + +The shared bucket was acting as an accidental global throttle on a +single-process uvicorn backend that filters a 25,000-row DataFrame in-process. +Correct per-user keying removes it: the origin becomes reachable at 60/min *per +user* rather than 60/min in total, and — per above — at an unbounded rate by +anyone willing to rotate a header. + +An earlier draft dropped the in-app ceiling, arguing it belonged at Cloudflare. +That argument assumed the keying was sound. It is not, so the ceiling is +load-bearing rather than redundant, and it ships here: + +`GlobalRateLimitMiddleware` counts all `/api/` requests in a fixed 60-second +window against `global_rate_limit_per_minute` (3000), independent of any client +identity, and refuses with a 429 that names capacity rather than the client — +an operator has to be able to tell "one noisy client" from "the origin is +saturated". It is registered last so it is outermost: a ceiling that applies +after the expensive work has run is not a ceiling. + +slowapi cannot express this. `default_limits` and `application_limits` are both +evaluated with the same `key_func`, making them per-client rather than global, +and `application_limits` only apply with `SlowAPIMiddleware` installed, which +this app does not use. Hence the explicit middleware — about thirty lines, and +obviously correct, which is what a backstop needs to be. + +Requests from `127.0.0.1` are exempt. The container healthcheck runs +`curl http://localhost:80/api/data-info` from inside the container, and +starving it would fail the check, restart the container, and turn a load spike +into an outage loop. The exemption keys on the peer address, never the `Host` +header, which the caller sets. + +3000/minute is an estimate, not a measurement, and worth revisiting against +real traffic. + +### Per-user limits + +Per-user fairness and origin protection are different jobs, and this design now +does both separately: the ceiling above for the origin, and per-route limits +for fairness. Conflating them is what produced the original behaviour, where +one bucket served the whole internet. + +The existing 60/minute default is unchanged, and `/api/suggest` gets +120/minute. Both are estimates rather than measurements, and are a starting +point to revisit once the keying is correct enough for real per-user traffic to +be visible — which it was not before, because everyone shared one bucket. + +## 2. `GET /api/suggest` + +A dedicated endpoint, not a mode of `/api/schools`. + +The existing search path calls Typesense for URNs and then filters, ranks and +sorts the full in-memory DataFrame — a pandas pass per keystroke, holding the +GIL and blocking other requests in the same worker. Suggestions need none of +it: `urn`, `school_name`, `phase`, `school_type`, `local_authority`, +`postcode` and `ofsted_rating` are all already in the Typesense document +(`pipeline/scripts/sync_typesense.py`). + +``` +GET /api/suggest?q=&limit=8 +→ 200 {"suggestions": [ + {"urn": 100010, "school_name": "Brecknock Primary School", + "local_authority": "Camden", "postcode": "NW1 1AA", + "phase": "Primary", "school_type": "Community school"} + ]} +``` + +- **Under two characters** returns `{"suggestions": []}` with 200. The + keystroke path never returns an error for ordinary input. +- **Typesense unavailable** returns `{"suggestions": []}` with 200. There is + deliberately **no DataFrame fallback**: the substring scan `/api/schools` + falls back to is precisely the cost this endpoint exists to avoid, and a + silent 25,000-row scan per keystroke is worse than no suggestions. +- **`limit` is clamped** to 20. It is a public endpoint. +- **Rate limit `120/minute`** per client, not the default 60. A 200 ms + debounce tops out near 5 requests/second while someone is actively typing, + but averages far below that across a real search; 120 leaves headroom for + bursts while still bounding one client. +- **Local authority is part of the payload, not decoration.** There are many + schools called "St Mary's"; a suggestion list without the authority is + unusable for exactly the queries autosuggest is meant to serve. + +### Caching + +`CACHE_RULES` gains `("/api/suggest", (60, 3600, 86400))`. Prefix queries +repeat enormously across users and school names change once a year. + +The client fetch must **not** use `cache: "no-store"`. The compare modal does, +and copying that pattern would throw away both the browser cache and the ETag +304s the existing `CacheAndETagMiddleware` already provides. + +Both environments currently report `cf-cache-status: DYNAMIC` — Cloudflare +ignores the `Cache-Control` the API already sends, because it does not cache +dynamic paths by default. **A Cloudflare Cache Rule for `/api/suggest*` would +let the edge absorb most of this traffic and never reach the origin.** That is +a dashboard change, it is optional, and nothing here depends on it. + +## 3. The combobox + +This is an ARIA combobox, not a text input with a list underneath. + +**Files.** `FilterBar.tsx` is already long. The work splits three ways: +`hooks/useSchoolSuggest.ts` owns fetching, debouncing and cancellation; +`components/SuggestList.tsx` owns rendering and ARIA; `FilterBar.tsx` wires +them to the existing input and form. + +**Fetching.** 200 ms debounce; minimum two characters; an `AbortController` +cancels the superseded request on every keystroke. Cancellation is not an +optimisation — without it, a slow response for `"st"` can land after the fast +one for `"st marys"` and replace a correct list with a stale one. + +**Suppressed during postcode entry.** The box takes a school name *or* a +postcode, and `isValidPostcode` already distinguishes them. Suggestions do not +appear once the value parses as a postcode. + +**Keyboard.** `ArrowDown`/`ArrowUp` move the active option, `Escape` closes and +keeps the typed text, `Tab` closes. `Enter` **with an option active** navigates +to that school's page. `Enter` **with none active** submits the free-text +search exactly as it does today — the existing behaviour is preserved, not +replaced. + +**ARIA.** `role="combobox"` with `aria-expanded` and `aria-controls` on the +input, `aria-activedescendant` pointing at the active option, `role="listbox"` +on the list and `role="option"` on each row. + +**Both instances get it.** `HomeView` renders `FilterBar` twice — hero and +sticky — from one component, so there is one implementation. + +## 4. Behind a flag + +Flag `school_autosuggest`, declared in `backend/flags.py`, default off. + +This is the most-used control on the site and the first change to it in a +while. `app/page.tsx` is an async server component, so it reads the flag and +threads it to `FilterBar` through `HomeView` — two prop hops, explicit, no +client-side flag read. + +Off means the input behaves exactly as it does today: no listener, no fetch, no +markup. Not a rendered-then-hidden dropdown. + +The rate-limit keying is **not** flagged. It is a correctness fix that should +apply whether or not autosuggest is on, and flagging it would mean shipping a +known-wrong limiter into production deliberately. + +## 5. Analytics + +`search_submitted` already carries `via: 'input'`. Selecting a suggestion fires +it with `via: 'suggestion'` plus the chosen `urn`, so the obvious question — +does this actually help, or do people ignore it — has an answer in the data +rather than an opinion. + +## 6. Testing + +**Backend.** `client_key` prefers `CF-Connecting-IP`, falls back through +`X-Forwarded-For` to the remote address, and two different values get two +different buckets. `/api/suggest` returns matches, returns empty below two +characters, returns empty and 200 when Typesense is unavailable, and clamps +`limit`. That it never touches the DataFrame is asserted by making +`load_school_data` raise and requiring the endpoint to answer anyway. + +**Frontend.** The hook debounces, aborts superseded requests, and drops a +late-arriving response for a stale query. The list renders the ARIA +attributes. Keyboard navigation moves the active option; `Enter` on an option +navigates; `Enter` on none submits the search. + +**E2E.** With the flag on, typing a known school name shows it and selecting it +lands on that school's page. With the flag off, no combobox markup exists. +Gated on the flag the same way the distance journeys are — read the observable +effect, since `/api/flags` is denied to the public. + +## 7. Risks + +**Removing the accidental throttle.** Covered in §1. Correct per-user keying +means the origin is reachable at 60/minute *per user* where it was 60/minute +in total, and no in-app global cap replaces it — that job goes to Cloudflare, +which is not done as part of this change. Until it is, a determined caller +with many source addresses can put more load on a single-process origin than +they can today. Against this site's traffic that is a theoretical risk rather +than a live one, but it is a real one and it is the price of the fix. + +**Cloudflare bypass — the open one.** If the origin is reachable without +passing through Cloudflare, `CF-Connecting-IP` is attacker-controlled, and +rotating it per request defeats per-client limits on every endpoint. The +ceiling in §1.1 bounds the damage to the origin's total capacity; it does not +restore per-client fairness under attack, and it cannot. Closing this properly +means Authenticated Origin Pulls or an origin firewall restricted to +Cloudflare's published ranges — infrastructure work, outside this change, and +the single most valuable follow-up here. + +**Typesense becomes user-visible.** Today a Typesense outage degrades search to +a slow substring match. With autosuggest it also means the dropdown silently +stops appearing. That is the correct failure — quiet, not broken — but it makes +Typesense health worth monitoring in a way it was not before. + +## Out of scope + +- **Place suggestions.** The 2,646 town, authority and outcode pages are a + strong candidate and would route people onto the pages W2 built, but they + live in the place registry rather than Typesense, so it is a second index and + a ranking rule for comparing two kinds of result. Worth its own change. +- **Postcode completion.** Would put postcodes.io in the keystroke path, with + its own latency and rate limits. +- **The compare modal.** It already has search-as-you-type. Converting it to + this component is a reasonable follow-up, not part of this. +- **Recent or popular searches.** No storage for either, and no evidence yet + that they are wanted. diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index e99174d..db3fab7 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -2111,3 +2111,66 @@ test('the rankings page still orders by score, not name', async ({ page }) => { .filter((v: number | null) => v != null); expect(scores).toEqual([...scores].sort((a: number, b: number) => b - a)); }); + +/* + * School autosuggest (spec 2026-08-26). + */ +async function autosuggestIsOn(page: Page): Promise { + await page.goto('/'); + return (await page.getByRole('combobox').count()) > 0; +} + +test('the suggest endpoint answers from Typesense', async ({ page }) => { + // Not flagged — the endpoint is live even while the UI is dark, so it can + // be smoke-tested before the feature is switched on. + const res = await page.request.get('/api/suggest?q=brecknock'); + expect(res.ok()).toBeTruthy(); + const { suggestions } = await res.json(); + expect(Array.isArray(suggestions)).toBeTruthy(); + if (suggestions.length) { + // Local authority is what tells two "St Mary's" apart. + expect(suggestions[0]).toHaveProperty('school_name'); + expect(suggestions[0]).toHaveProperty('local_authority'); + } +}); + +test('a one-character query is answered, not rejected', async ({ page }) => { + // The keystroke path never errors on ordinary input. + const res = await page.request.get('/api/suggest?q=b'); + expect(res.status()).toBe(200); + expect((await res.json()).suggestions).toEqual([]); +}); + +test('the suggest response is cacheable', async ({ page }) => { + const res = await page.request.get('/api/suggest?q=brecknock'); + expect(res.headers()['cache-control'] ?? '').toContain('s-maxage'); +}); + +test('typing a school name suggests it, and choosing it opens that school', async ({ page }) => { + test.skip(!(await autosuggestIsOn(page)), + 'the school_autosuggest flag is off in this environment'); + + // A school certain to exist in any environment with data. + const { schools } = await (await page.request.get('/api/schools?page_size=1')).json(); + test.skip(!schools?.length, 'no schools in this environment'); + const name = schools[0].school_name as string; + + await page.goto('/'); + await page.getByRole('combobox').first().fill(name.slice(0, 12)); + const option = page.getByRole('option').first(); + await expect(option).toBeVisible(); + await option.click(); + await expect(page).toHaveURL(/\/school\/\d+/); +}); + +test('with autosuggest off, the search box is a plain input', async ({ page }) => { + test.skip(await autosuggestIsOn(page), + 'the school_autosuggest flag is on in this environment'); + + await page.goto('/'); + await expect(page.getByRole('combobox')).toHaveCount(0); + // And the box still works: the existing search must be untouched. + await page.getByPlaceholder(/School name or postcode/i).first().fill('abbey'); + await page.getByRole('button', { name: /Search/i }).first().click(); + await expect(page).toHaveURL(/search=abbey/); +}); diff --git a/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx b/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx new file mode 100644 index 0000000..70ec8ac --- /dev/null +++ b/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx @@ -0,0 +1,76 @@ +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { FilterBar } from '@/components/FilterBar'; + +const push = jest.fn(); +jest.mock('next/navigation', () => ({ + useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }), + usePathname: () => '/', + useSearchParams: () => new URLSearchParams(), +})); + +const FILTERS = { + local_authorities: [], school_types: [], years: [], phases: [], + genders: [], admissions_policies: [], +}; + +const realFetch = global.fetch; +beforeEach(() => { + global.fetch = jest.fn(async () => ({ + ok: true, + json: async () => ({ suggestions: [{ + urn: 100010, school_name: 'Brecknock Primary School', + local_authority: 'Camden', postcode: 'NW1 1AA', + phase: 'Primary', school_type: 'Community school' }] }), + })) as unknown as typeof fetch; + push.mockClear(); +}); +afterEach(() => { global.fetch = realFetch; }); + +describe('FilterBar autosuggest', () => { + it('is a combobox only when the flag is on', () => { + const { rerender } = render(); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + rerender(); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + }); + + it('makes no request while the flag is off', async () => { + // Off means off: no listener, no fetch, no markup. + render(); + await userEvent.type(screen.getByPlaceholderText(/School name or postcode/i), + 'brecknock'); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('shows suggestions and navigates when one is chosen', async () => { + render(); + await userEvent.type(screen.getByRole('combobox'), 'brecknock'); + const option = await screen.findByRole('option', { name: /Brecknock/ }); + await userEvent.click(option); + expect(push).toHaveBeenCalledWith( + expect.stringContaining('/school/100010')); + }); + + it('suppresses suggestions once the value is a postcode', async () => { + // The box takes a name OR a postcode; suggestions must get out of the way. + // + // fireEvent.change, not userEvent.type: typing sets "N", "NW", "NW1"... and + // "NW1" is not a postcode, so a request for it is correct behaviour. Only + // the settled value is the assertion, so set it in one go. + render(); + fireEvent.change(screen.getByRole('combobox'), { target: { value: 'NW1 1AA' } }); + await new Promise((r) => setTimeout(r, 300)); // past the 200ms debounce + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('Enter with no active option still submits the free-text search', async () => { + // The existing behaviour is preserved, not replaced. + render(); + const input = screen.getByRole('combobox'); + await userEvent.type(input, 'brecknock{Enter}'); + // updateURL pushes inside startTransition, so the call is not synchronous. + await waitFor(() => expect(push).toHaveBeenCalledWith( + expect.stringContaining('search=brecknock'))); + }); +}); diff --git a/nextjs-app/__tests__/components/SuggestList.test.tsx b/nextjs-app/__tests__/components/SuggestList.test.tsx new file mode 100644 index 0000000..fbc68e2 --- /dev/null +++ b/nextjs-app/__tests__/components/SuggestList.test.tsx @@ -0,0 +1,50 @@ +import { render, screen } from '@testing-library/react'; +import { SuggestList, suggestOptionId } from '@/components/SuggestList'; + +const ROWS = [ + { urn: 1, school_name: "St Mary's Primary", local_authority: 'Camden', + postcode: 'NW1 1AA', phase: 'Primary', school_type: 'Voluntary aided school' }, + { urn: 2, school_name: "St Mary's Primary", local_authority: 'Barnet', + postcode: 'EN5 2AA', phase: 'Primary', school_type: 'Community school' }, +]; + +describe('SuggestList', () => { + it('is a listbox of options', () => { + render( {}} onHover={() => {}} />); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + expect(screen.getAllByRole('option')).toHaveLength(2); + }); + + it('shows the local authority, which is what tells two schools apart', () => { + // Both rows are "St Mary's Primary". Without the authority the list is + // unusable for exactly the query autosuggest exists to serve. + render( {}} onHover={() => {}} />); + expect(screen.getByText('Camden')).toBeInTheDocument(); + expect(screen.getByText('Barnet')).toBeInTheDocument(); + }); + + it('marks only the active option selected', () => { + render( {}} onHover={() => {}} />); + const options = screen.getAllByRole('option'); + expect(options[0]).toHaveAttribute('aria-selected', 'false'); + expect(options[1]).toHaveAttribute('aria-selected', 'true'); + }); + + it('gives each option the id the input will point at', () => { + // aria-activedescendant on the input has to name a real element id, or + // a screen reader announces nothing as the user arrows through. + render( {}} onHover={() => {}} />); + expect(screen.getAllByRole('option')[0]).toHaveAttribute( + 'id', suggestOptionId('s', 0)); + }); + + it('renders nothing when there is nothing to suggest', () => { + const { container } = render( {}} onHover={() => {}} />); + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx b/nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx new file mode 100644 index 0000000..4681b86 --- /dev/null +++ b/nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx @@ -0,0 +1,73 @@ +import { renderHook, act, waitFor } from '@testing-library/react'; +import { useSchoolSuggest } from '@/hooks/useSchoolSuggest'; + +const realFetch = global.fetch; + +function mockFetch(rows: unknown[], delayMs = 0) { + global.fetch = jest.fn(async (_url: unknown, init?: { signal?: AbortSignal }) => { + if (delayMs) { + await new Promise((resolve, reject) => { + const t = setTimeout(resolve, delayMs); + init?.signal?.addEventListener('abort', () => { + clearTimeout(t); + reject(Object.assign(new Error('aborted'), { name: 'AbortError' })); + }); + }); + } + return { ok: true, json: async () => ({ suggestions: rows }) }; + }) as unknown as typeof fetch; +} + +const ROW = { + urn: 1, school_name: 'Brecknock Primary School', local_authority: 'Camden', + postcode: 'NW1 1AA', phase: 'Primary', school_type: 'Community school', +}; + +describe('useSchoolSuggest', () => { + beforeEach(() => { jest.useFakeTimers(); }); + afterEach(() => { jest.useRealTimers(); global.fetch = realFetch; }); + + it('does not fetch below the minimum query length', () => { + mockFetch([ROW]); + renderHook(() => useSchoolSuggest('b', true)); + act(() => { jest.advanceTimersByTime(500); }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('does not fetch at all when disabled', () => { + // The flag being off must mean no request, not a hidden dropdown. + mockFetch([ROW]); + renderHook(() => useSchoolSuggest('brecknock', false)); + act(() => { jest.advanceTimersByTime(500); }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('debounces rather than firing per keystroke', () => { + mockFetch([ROW]); + const { rerender } = renderHook( + ({ q }) => useSchoolSuggest(q, true), { initialProps: { q: 'br' } }); + rerender({ q: 'bre' }); + rerender({ q: 'brec' }); + act(() => { jest.advanceTimersByTime(199); }); + expect(global.fetch).not.toHaveBeenCalled(); + act(() => { jest.advanceTimersByTime(2); }); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('opens with results once they arrive', async () => { + mockFetch([ROW]); + const { result } = renderHook(() => useSchoolSuggest('brecknock', true)); + act(() => { jest.advanceTimersByTime(200); }); + await waitFor(() => expect(result.current.suggestions).toHaveLength(1)); + expect(result.current.open).toBe(true); + }); + + it('close() hides the list without clearing the query', async () => { + mockFetch([ROW]); + const { result } = renderHook(() => useSchoolSuggest('brecknock', true)); + act(() => { jest.advanceTimersByTime(200); }); + await waitFor(() => expect(result.current.open).toBe(true)); + act(() => { result.current.close(); }); + expect(result.current.open).toBe(false); + }); +}); diff --git a/nextjs-app/app/page.tsx b/nextjs-app/app/page.tsx index dfcdeda..f791b95 100644 --- a/nextjs-app/app/page.tsx +++ b/nextjs-app/app/page.tsx @@ -8,6 +8,7 @@ import type { Metadata } from 'next'; import { fetchSchools, fetchFilters, fetchDataInfo } from '@/lib/api'; import { formatAcademicYear } from '@/lib/utils'; import { HomeView } from '@/components/HomeView'; +import { getFlags } from '@/lib/flags'; import { HowItWorksSection } from '@/components/HowItWorksSection'; import { EditorialSection } from '@/components/EditorialSection'; @@ -63,6 +64,11 @@ export default async function HomePage({ searchParams }: HomePageProps) { // Await search params (Next.js 15 requirement) const params = await searchParams; + // Server-read: no flag value reaches the browser bundle. Threaded down to + // both FilterBar instances via HomeView. + const flags = await getFlags(); + const autosuggest = flags.school_autosuggest === true; + // Parse search params const page = parseInt(params.page || '1'); const radius = params.radius ? parseFloat(params.radius) : undefined; @@ -111,6 +117,7 @@ export default async function HomePage({ searchParams }: HomePageProps) { const years = dataInfo?.years_available ?? []; return ( void; geoState?: "idle" | "requesting" | "error"; geoError?: string | null; + /** Server-read feature flag. Off means no listener, no fetch, no markup. */ + autosuggest?: boolean; } /** @@ -48,6 +53,7 @@ export function FilterBar({ onNearMe, geoState = "idle", geoError, + autosuggest = false, }: FilterBarProps) { const router = useRouter(); const pathname = usePathname(); @@ -62,6 +68,45 @@ export function FilterBar({ const [omniValue, setOmniValue] = useState(initialOmniValue); + const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`; + // Suppressed once the value parses as a postcode: the box takes a school + // name OR a postcode, and suggesting schools during postcode entry fights + // the user rather than helping them. + const suggestEnabled = autosuggest && !isValidPostcode(omniValue); + const { suggestions, open, activeIndex, setActiveIndex, close } = + useSchoolSuggest(omniValue, suggestEnabled); + + const pickSuggestion = (s: Suggestion) => { + close(); + track('search_submitted', { + query: s.school_name.toLowerCase(), + via: 'suggestion', + urn: s.urn, + has_postcode: false, + filters_active: '', + filters_count: 0, + }); + router.push(schoolUrl(s.urn, s.school_name)); + }; + + const handleOmniKeyDown = (e: React.KeyboardEvent) => { + if (!open) return; + if (e.key === "ArrowDown") { + e.preventDefault(); + setActiveIndex(activeIndex + 1 >= suggestions.length ? 0 : activeIndex + 1); + } else if (e.key === "ArrowUp") { + e.preventDefault(); + setActiveIndex(activeIndex <= 0 ? suggestions.length - 1 : activeIndex - 1); + } else if (e.key === "Escape") { + close(); + } else if (e.key === "Enter" && activeIndex >= 0) { + // Only when an option is active. With none, the event falls through to + // the form's submit handler and searches the typed text, as it does now. + e.preventDefault(); + pickSuggestion(suggestions[activeIndex]); + } + }; + const currentLA = searchParams.get("local_authority") || ""; const currentType = searchParams.get("school_type") || ""; const currentPhase = searchParams.get("phase") || ""; @@ -227,8 +272,19 @@ export function FilterBar({ type="search" value={omniValue} onChange={(e) => setOmniValue(e.target.value)} + onKeyDown={handleOmniKeyDown} + onBlur={close} placeholder="School name or postcode" className={styles.omniInput} + {...(autosuggest ? { + role: "combobox", + "aria-expanded": open, + "aria-controls": suggestId, + "aria-autocomplete": "list" as const, + "aria-activedescendant": + activeIndex >= 0 ? suggestOptionId(suggestId, activeIndex) : undefined, + autoComplete: "off", + } : {})} /> + {autosuggest && open && ( + + )} {isHero && ( <> diff --git a/nextjs-app/components/HomeView.tsx b/nextjs-app/components/HomeView.tsx index 205c626..fd90e77 100644 --- a/nextjs-app/components/HomeView.tsx +++ b/nextjs-app/components/HomeView.tsx @@ -29,6 +29,8 @@ interface HomeViewProps { // show (e.g. an active search). howItWorks?: React.ReactNode; editorial?: React.ReactNode; + /** Server-read feature flag, threaded to both FilterBar instances. */ + autosuggest?: boolean; } function daysUntil(month: number, day: number): number { @@ -193,7 +195,7 @@ const VALUE_PROPS: ValueProp[] = [ }, ]; -export function HomeView({ initialSchools, filters, totalSchools, howItWorks, editorial }: HomeViewProps) { +export function HomeView({ initialSchools, filters, totalSchools, howItWorks, editorial, autosuggest = false }: HomeViewProps) { const searchParams = useSearchParams(); const router = useRouter(); const pathname = usePathname(); @@ -462,6 +464,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed onNearMe={handleNearMe} geoState={geoState} geoError={geoError} + autosuggest={autosuggest} /> @@ -500,6 +503,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed onNearMe={handleNearMe} geoState={geoState} geoError={geoError} + autosuggest={autosuggest} /> )} diff --git a/nextjs-app/components/SuggestList.module.css b/nextjs-app/components/SuggestList.module.css new file mode 100644 index 0000000..a785220 --- /dev/null +++ b/nextjs-app/components/SuggestList.module.css @@ -0,0 +1,53 @@ +/* + * Anchored to .omniBoxContainer, which is position: relative for this reason. + * + * Every colour is a token, so the dropdown follows the theme. The dark theme + * redefines --bg-card, --border, --text-muted and --shadow-soft, and this + * inherits all four without a second rule. + */ +.list { + position: absolute; + top: calc(100% + 4px); + left: 0; + right: 0; + /* Above the sticky filter bar (10) and the hero layers (0–2), below the + skip-link (10000) and the modal overlay (1000). */ + z-index: 40; + margin: 0; + padding: 4px; + list-style: none; + max-height: 320px; + overflow-y: auto; + background: var(--bg-card); + border: 1px solid var(--border); + border-radius: var(--radius-md); + box-shadow: var(--shadow-soft); +} + +.option { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 12px; + padding: 10px 12px; + border-radius: var(--radius-sm); + cursor: pointer; + color: var(--text-primary); +} + +/* Hover and keyboard share one style: the active option is the active option + however it became active. Two rules would drift. */ +.option:hover, +.active { + background: var(--bg-secondary); +} + +.name { + font-weight: 500; +} + +.meta { + font-size: 0.85em; + color: var(--text-muted); + white-space: nowrap; +} diff --git a/nextjs-app/components/SuggestList.tsx b/nextjs-app/components/SuggestList.tsx new file mode 100644 index 0000000..18071c4 --- /dev/null +++ b/nextjs-app/components/SuggestList.tsx @@ -0,0 +1,53 @@ +'use client'; + +/** + * The autosuggest dropdown. Presentational only — it fetches nothing and owns + * no state, so the fetching rules and the ARIA rules can be read separately. + */ + +import type { Suggestion } from '@/lib/suggest'; +import styles from './SuggestList.module.css'; + +/** The id the input's aria-activedescendant points at. */ +export function suggestOptionId(id: string, index: number): string { + return `${id}-option-${index}`; +} + +interface Props { + /** Shared with the input's aria-controls. */ + id: string; + suggestions: Suggestion[]; + activeIndex: number; + onPick: (s: Suggestion) => void; + onHover: (index: number) => void; +} + +export function SuggestList({ id, suggestions, activeIndex, onPick, onHover }: Props) { + if (suggestions.length === 0) return null; + + return ( +
    + {suggestions.map((s, i) => ( +
  • { e.preventDefault(); onPick(s); }} + onMouseEnter={() => onHover(i)} + > + {s.school_name} + {/* Not decoration: there are many "St Mary's". */} + {s.local_authority} +
    • + ))} +
+ ); +} diff --git a/nextjs-app/hooks/useSchoolSuggest.ts b/nextjs-app/hooks/useSchoolSuggest.ts new file mode 100644 index 0000000..53ba171 --- /dev/null +++ b/nextjs-app/hooks/useSchoolSuggest.ts @@ -0,0 +1,58 @@ +'use client'; + +import { useEffect, useRef, useState } from 'react'; +import { fetchSuggestions, SUGGEST_MIN_QUERY, type Suggestion } from '@/lib/suggest'; + +/* + * Long enough that a fast typist does not fire a request per character, short + * enough that the list feels attached to the keyboard. + */ +const DEBOUNCE_MS = 200; + +export function useSchoolSuggest(query: string, enabled: boolean) { + const [suggestions, setSuggestions] = useState([]); + const [open, setOpen] = useState(false); + const [activeIndex, setActiveIndex] = useState(-1); + // Set when the user dismisses the list, so a re-render does not reopen it. + const dismissed = useRef(''); + + useEffect(() => { + const q = query.trim(); + if (!enabled || q.length < SUGGEST_MIN_QUERY || dismissed.current === q) { + setSuggestions([]); + setOpen(false); + return; + } + + /* + * Abort the superseded request on every keystroke. This is correctness, + * not economy: without it a slow response for "st" can land after the fast + * one for "st marys" and replace a correct list with a stale one. + */ + const controller = new AbortController(); + const timer = setTimeout(async () => { + const rows = await fetchSuggestions(q, controller.signal); + if (controller.signal.aborted) return; + setSuggestions(rows); + setActiveIndex(-1); + setOpen(rows.length > 0); + }, DEBOUNCE_MS); + + return () => { + clearTimeout(timer); + controller.abort(); + }; + }, [query, enabled]); + + return { + suggestions, + open, + activeIndex, + setActiveIndex, + close: () => { + dismissed.current = query.trim(); + setOpen(false); + setActiveIndex(-1); + }, + }; +} diff --git a/nextjs-app/lib/suggest.ts b/nextjs-app/lib/suggest.ts new file mode 100644 index 0000000..5a17d9d --- /dev/null +++ b/nextjs-app/lib/suggest.ts @@ -0,0 +1,39 @@ +/** + * Client for /api/suggest. + * + * No `cache: "no-store"`. The compare modal's search uses it, and copying that + * here would discard both the browser cache and the ETag 304s the backend's + * CacheAndETagMiddleware already provides — on the one endpoint where prefix + * queries repeat most. + */ + +export interface Suggestion { + urn: number; + school_name: string; + local_authority: string; + postcode: string; + phase: string; + school_type: string; +} + +/** Below this the response is thousands of schools and worth no round trip. */ +export const SUGGEST_MIN_QUERY = 2; + +const API = process.env.NEXT_PUBLIC_API_URL || '/api'; + +/** Suggestions for `q`. Never throws: no suggestions is a fine outcome. */ +export async function fetchSuggestions( + q: string, signal?: AbortSignal, +): Promise { + if (q.trim().length < SUGGEST_MIN_QUERY) return []; + try { + const res = await fetch(`${API}/suggest?q=${encodeURIComponent(q.trim())}`, + { signal }); + if (!res.ok) return []; + const body = await res.json(); + return body.suggestions ?? []; + } catch { + // Includes AbortError, which is the normal path on every keystroke. + return []; + } +}