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..accde0d --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-school-autosuggest-design.md @@ -0,0 +1,239 @@ +# 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. + +Two things make this safe: Cloudflare replaces the header, so a browser cannot +forge it; and the backend is unreachable from outside the Docker network, so +nothing can reach it without passing through the proxy. The `X-Forwarded-For` +fallback *is* forgeable, but only by a caller already inside that network. + +### The part that is not free + +The shared bucket has been 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 that throttle: the origin becomes reachable at +60/min *per user* rather than 60/min in total. + +So the keying fix ships **with a global ceiling**, not instead of one: +slowapi's application-wide `default_limits`, set above any plausible real load +but below what would flood a single worker. **3000/minute**, against a +per-user suggest limit of 120/minute — so twenty-five simultaneously active +searchers are unaffected, while a runaway client or a scraper still meets a +wall. + +Per-user fairness and origin protection are different jobs and need different +limits. Conflating them is what produced the current behaviour. + +Both numbers are estimates, not measurements. They are a starting point to +revisit against real traffic once the keying is correct enough for real +traffic to be visible — which it is not today, because everyone shares 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 without letting one client alone consume the global ceiling. +- **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. The global ceiling is the +mitigation, and 1200/minute is a guess that should be revisited against real +traffic rather than treated as a considered number. + +**Cloudflare bypass.** If the origin is reachable without passing through +Cloudflare, `CF-Connecting-IP` is absent and the `X-Forwarded-For` fallback is +forgeable, so limits could be evaded per-request. Closing that properly means +Authenticated Origin Pulls or an origin firewall, which is infrastructure work +outside this change. Worth doing separately. + +**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.