docs(suggest): design for school autosuggest
The load-bearing finding is not about autosuggest. The rate limiter keys on request.client.host, which in staging and prod is the Next container — so all browser users share one 60/min bucket per route. Measured against staging: 70 concurrent requests gave exactly 60 x 200 and 10 x 429. Eight concurrent searchers would 429 the site once each keystroke costs a request, so the keying fix is part of this work. Both environments are behind Cloudflare, which sets CF-Connecting-IP and overwrites any client-supplied value — trustworthy in a way a parsed X-Forwarded-For chain is not, and the backend is unreachable except through the Next proxy. Named honestly: the shared bucket has been an accidental global throttle on a single-process backend, so correct per-user keying removes a protection. A global ceiling ships with it rather than instead of it. Suggestions come from Typesense alone. The existing search path filters a 25,000-row DataFrame per query, which is exactly the cost a keystroke endpoint cannot pay, so there is deliberately no DataFrame fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
This commit is contained in:
1 parent
e953ee7c5f
commit
22c113fc29
1 file changed
+239
@@ -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=<query>&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.
|
||||
Reference in new issue
Block a user