feat(geocode): Places autocomplete, Barbados-bounded fallback chain, and a required located address

[sajclarke]

Closes #2679

Address lookup now finds places by name, the way Google Maps does ("kensin" → Kensington Oval, Kensington Mall…). An address that routes an application must be located: picked from the list, or placed on a map when it can't be found.

API (apps/api/src/geocode)

• Ordered list of providers, set by config. GEOCODE_PROVIDERS (default google,nominatim) sets the order. Google is skipped until GOOGLE_GEOCODING_API_KEY is set, so an environment without the key keeps today's behaviour (Nominatim only). An unknown provider name stops the API at startup.
• Google suggestions use Places API (New).
  • Places Autocomplete, restricted to Barbados (includedRegionCodes: ["bb"] plus the island's bounds). Each result returns the place name, where it is, the matched characters and a placeId.
  • Predictions for the country itself are dropped: their centre point would send the case to an arbitrary polyclinic.
• GET /geocode/place/:placeId?session= looks up the coordinate and parish of a picked place (Place Details, location and address parts only).
  • The browser sends a session token, which ties the keystrokes and the pick into one billed Places session.
  • The place id is validated before it is sent to Google, and a result outside Barbados is refused.
• GET /geocode/reverse?lat=&lon= returns the address at a map pin, from Google's Geocoding API with Nominatim /reverse as the fallback. It always returns the parish, read off the parish map when no provider names one or none can answer.
• GET /geocode/polyclinic?lat=&lon= names the polyclinic a located address routes to (404 where none serves), so the field can say so.
• Fallback and errors.
  • A provider that fails (quota, bad key, outage) falls through to the next one.
  • For search, an empty answer is final. For reverse it is not: Google often has only a plus code for a rural pin, so the next provider is asked.
  • When no provider can answer, search and place return 503; previously search returned [].
• Parish. A located address's parish is read-only on the form, so the API always supplies one: a search result or place with a coordinate but no parish gets it from the parish map (parishAt).
  • parishAt takes the nearest parish outline only within 1 km, because the outlines are coarse and the real shore is up to ~630 m outside them (Bottom Bay). A point further out is at sea and has no parish.
  • A place or search result with no parish is therefore offshore: /geocode/place/:id answers 404 (the field offers the map) and search drops it.
• Timeout. Provider calls time out after 5 s, so a provider that hangs falls through to the next.
• Cache. Search answers are cached for an hour only when they come from the first provider and that provider's terms allow it. Google's do not (§3.2.3(b)), so only Nominatim's answers are cached. A fallback's answers are never cached, so they do not outlive an outage.
  • Results outside Barbados are always dropped.
• Address parsing. Google writes the end of an address without commas (Saint Michael BB120004Barbados). The parser now removes the country, postcode and plus code (493G+XXX) before splitting into line 1, line 2 and parish, so the parish no longer ends up in line 2.
• Coordinates are rounded to 6 decimal places.
• Logs. A failed search logs the query's length, not the typed address; a failed reverse lookup logs the pin rounded to ~1 km.

Forms address field

• The suggestion list is built in the forms app, using the design-system combobox classes and the WAI-ARIA editable-combobox pattern.
  • Each row shows the place name with the typed part in bold, then where it is in muted text.
  • When Google results are shown, the list carries a "Google Maps" attribution, styled to Google's text-attribution rules.
  • The listbox keeps the <id>-listbox id that smoke specs target.
• Picking a place fills line 1 with its name, line 2 with the street and town, and the parish, then fetches the coordinate. If that fetch fails, the pin map appears.
• An address that routes the application must be located. On fields that write a routing coordinate, an address that is typed but never picked from the list or placed on the map is refused at Continue: "Choose the address from the list, or find it on the map". Fields that don't route still accept typed addresses.
• When nothing matches, the field shows "We cannot find that address. Find it on the map."
• The pin map (Leaflet, lazy-loaded):
  • The pin can be dragged, clicked into place, or moved with the arrow keys, and it stays inside Barbados.
  • When it settles, the address at that point fills lines 1 and 2 and the parish. The coordinate stays the pin's own.
  • The pin is the location and the text only describes it, so editing line 1 after placing a pin keeps the pin, its coordinate and the parish.
  • "Hide the map" closes it, and picking a suggestion also closes it.
  • A pin in the sea (no parish there) locates nothing: the field says "The pin is in the sea. Move it to where the address is."
  • If the map fails to load, the page stays and the field says "We cannot show the map at the moment. Refresh the page to try again."
  • The forms CSP now allows https://tile.openstreetmap.org in img-src; without it the deployed map had no tiles.
  • The suggestion list now sits above the map; Leaflet's layers are isolated.
• Once located, the field says where the application goes: "Your request will be routed to ."
• Keeping the details consistent. There is no confirmation prompt.
  • The parish is filled by the location and is shown but locked; the applicant cannot change it by hand.
  • Editing a picked address drops its coordinate and parish (line 2 is kept), so it has to be picked or placed again.
  • Clearing line 1 clears line 2, the parish, the coordinate and the pin.
  • Pressing Continue while a picked place's coordinate is still loading waits for it, rather than refusing the pick.

Submission (apps/api)

• An empty parish is filled from the coordinate before validation. If the lookup that writes the locked parish fails on the client (network error, 429), the submission would otherwise be refused for a field the applicant cannot change. The existing fill in the other direction (coordinate from parish, for typed addresses and old clients) is unchanged.
• trust proxy is set to one hop (the ALB). Without it, ThrottlerGuard kept one bucket for everyone on a task, and one address is several geocode calls. ⚠️ This applies to every throttled endpoint: limits that were shared by all applicants become per applicant. Only the ALB's entry in X-Forwarded-For is trusted, so a client cannot spoof its address.

Testing

• API:
  • Provider specs use real Places and Geocoding response shapes: autocomplete, Place Details, reverse lookup, failures.
  • Service specs cover fallback, 503 and 404, bounds checks, session-token validation, and place-id validation.
  • Submission specs cover the parish filled from the coordinate, and the parish still required when there is none.
  • 1901 tests pass (DB_HOST= vitest run).
• Forms: 992 tests pass. They cover:
  • suggestion rows and attribution, picking a place, the session token, and a late coordinate after an edit
  • editing a picked address, editing after a pin (pin kept), and clearing line 1
  • the no-match link, the pin map, the reverse fill and closing the map
  • the located-address validator, including Continue waiting for a pick's coordinate
• Builds and checks: nx run-many -t build --exclude=landing and tsc -b --force both pass.
• Smoke: the hairdresser test "free-typed address… still routes" is replaced by "refuses an unfound typed address, then routes one placed on the map".
  • Run locally, the address step passed (refused at Continue, pin placed, coordinate filled).
  • The run then stopped at file upload, which needs AWS credentials locally. The final submit-and-route step still needs a sandbox run.
• Live Google checks (local API with a key):
  • "kensin" and "gun hill" return the expected places, and "National Botanical" finds the gardens.
  • Picking Kensington Oval fills Kensington Oval / President Kennedy Drive, Bridgetown / St. Michael / 13.104995,-59.622524.
  • Dropping the pin in St. Thomas fills Hwy 3A / Applewhaites / St. Thomas.

Before release

• Google key: set GOOGLE_GEOCODING_API_KEY per environment (waiting on govtech-bb/case-management#368).
  • It needs Places API (New) and the Geocoding API enabled.
  • It must have no HTTP-referrer restriction, because the calls are made from the server.
  • It isn't wired into ECS or Secrets Manager yet.
• ⚠️ Google terms, needs a decision. Section 3.2.3(e) of the Service Specific Terms says Google services must not be used "with or near a non-Google map". Today the Google suggestions and the Google reverse lookup sit beside the OpenStreetMap pin map. The options:
  • (A) switch the pin map to Google tiles;
  • (B) keep OpenStreetMap, and use Nominatim only while the map is open, including for the reverse lookup;
  • (C) confirm with whoever owns the Google Maps contract.
  • Caching is no longer part of this question: Google search answers are not cached.
• Google budget cap. Set a daily budget or quota on the key when it is provisioned. The app-side throttle is per applicant, so only Google's cap bounds total cost.
• No circuit-breaker on quota. When Google's quota is used up, every uncached query still tries Google first, then falls back.

🤖 Generated with Claude Code

[amazon-inspector-n-virginia]

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

[amazon-inspector-n-virginia]

✅ I finished the code review, and didn't find any security or code quality issues.

[github-actions]

Amplify PR previews

App	Status	URL
forms	✅ Ready	https://geocoding-provider-fallback-2679.d1j7z1k0h7u5nb.amplifyapp.com
landing	✅ Ready	https://geocoding-provider-fallback-2679.d3kbl8o0ovutw4.amplifyapp.com
chat	⏭ Not affected	—
form-builder	✅ Ready	https://geocoding-provider-fallback-2679.d16oo4n0w76zwn.amplifyapp.com
analytics	✅ Ready	https://geocoding-provider-fallback-2679.d363pae4e63dc6.amplifyapp.com

Commit: d9b5ae1 · Branches auto-delete when this PR closes.

ℹ️ feature-flagging uses a single shared preview slot at a fixed URL (preview.feature-flagging.sandbox.alpha.gov.bb) — GitHub login works, but only one FF preview exists at a time (the most recent FF PR build wins). It reads/writes sandbox flag data.

[Zainab980]

@sajclarke big, careful piece of work. One fix before merge, plus two things to settle before release.

Verified: I read the PR, all 16 commits and the diff (0 behind main, merges clean), and traced pick → pin → reverse → polyclinic → parish lock/validation, plus throttling, caching, the server parish handling and the build assets. Tests at head pass (API 280, forms 455), and both .geojson files land in dist. I wrote a throwaway probe spec for #1 (not committed). I didn't run smoke or hit Google.

What's strong

• Provider chain from GEOCODE_PROVIDERS, with an unknown name failing at boot, and every provider's results bounded to Barbados.
• 503 instead of [] when lookup is down, so "no such address" and "lookup unavailable" can't be confused, and the field offers the pin.
• reverse() never fails server-side: if every provider is down, it still returns the pin's coordinate plus the parish from the map.
• parishAt falls back to the nearest vertex for points just off the generalised coastline, and the specs use real Places/Geocoding response shapes.

---

1. A pinned address can reach submit with no parish — fix before merge (inline)

2. One throttle bucket for every citizen — fix before release (inline)

3. Caching Google predictions may breach the terms — add to the (C) decision (inline)

4. The description no longer matches the code — nit
It still describes a native confirm() and "changing the parish drops the coordinate", and says the applicant picks a missing parish. Since 068c9221 / 1895c3e0 the parish is locked, no prompt fires, and /geocode/polyclinic plus "routed to …" are new. QA will test what the description says. ConfirmDialog and its CSS also ship unused ("for when it returns"); fine if it's coming back soon.

Security: no injection; place id and session token are validated, and the redirect target and Barbados bounds are server-side. Cost exposure is #2. Performance: Leaflet is lazy-loaded ✓. Conventions: OK.

[Zainab980]

Fix before merge: the parish is locked even when it's empty. If the pin's reverse lookup fails (address-lookup-field.tsx L285 .catch(() => {})), the coordinate is written but the parish never is. This select is disabled, validation-builder skips it, and the API still requires it (Parish is required). Nothing server-side fills parish from the coordinate (fillParishRoutingCoordinate only goes the other way), so submit 422s on a field the applicant can't touch. Verified: with the spec's default rejected reverseGeocode, a placed pin writes 13.100000,-59.600000 and never writes event-parish.

Smallest fix: lock only a filled parish, and validate it when it's empty:

-        {...(filledByAddress && {
+        {...(filledByAddress && selectValue && {

plus making the filledByAddressLookup skip in validation-builder.ts apply only when the value is set. Better still, fill it server-side from parishAt(coordinate) when it arrives empty, so any client failure is covered.

[sajclarke]

Resolved in 8465b7a, on the API side.

• The API now fills an empty parish from the coordinate. It does this before validation: fillRoutingParish in parish-routing-point.ts, called at the start of SubmissionPipelineService.run.
• Thus a failed pin lookup on the client does not stop a submission.
• If there is no coordinate, the API still requires the parish.
• Specs: parish-routing-point.spec.ts and submission-pipeline.service.spec.ts.

We did not unlock the empty select. renderSelectField cannot keep state. If we unlock the empty field, it locks again after the applicant selects a parish. Then the applicant cannot correct a wrong selection.

[Zainab980]

Fix before release: apps/api doesn't set trust proxy (see the note in journey.controller.ts), so ThrottlerGuard keys on the ALB hop: 30 req / 10s here is shared by every applicant on a task. One address is now several calls (debounced searches + place + polyclinic + reverse), so a busy hour returns 429s. Previously a typed address was an acceptable fallback; now a located address is mandatory, and a 429 on reverse is the likely trigger for the empty-parish dead end. Options: app.set("trust proxy", 1) (fixes every throttle), or a per-IP WAF rule for /geocode*.

[sajclarke]

Resolved in 8465b7a.

• main.ts now sets trust proxy to 1. The ALB is the only proxy. The DNS records for the API go directly to the ALB, with no CloudFront.
• ThrottlerGuard now uses the address of the applicant. Each applicant has a different bucket.
• Express uses only the last entry in X-Forwarded-For. The ALB writes that entry. Thus a client cannot change its address with this header.
• Caution: This change applies to all throttled endpoints, not only to /geocode.
• The comments in journey.controller.ts and throttler-config.spec.ts now agree with this change. The journey endpoint still skips the throttle.

[Zainab980]

Before release: this caches Google Autocomplete predictions (names and addresses) in memory for an hour. As I read the Maps Platform terms (§3.2.3(b) "No caching", plus the Places carve-outs), only place IDs, and lat/lng for up to 30 days, may be cached. Worth adding to the option (C) check alongside the "near a non-Google map" question. Skipping the cache for Google results (or caching only Nominatim's) would avoid it.

[sajclarke]

Resolved in 5c485be.

• Each provider now tells if its terms permit a cache (cacheable on GeocodeProvider). Only Nominatim permits a cache.
• The API does not cache Google search results.
• The API caches a result only from the first provider. Thus fallback results do not stay in the cache after an outage.
• Spec: "does not cache a provider whose terms forbid it" in geocode.service.spec.ts.
• Option (C) in the description now covers only the "near a non-Google map" question.

[ihtishamtanveer]

Strong, security-conscious work — recommend approve. This is a big change (48 files / +3.5k), so I went deep on the API + security surface and the new client contract, and sampled the rest.

Verified

• Input validation before anything hits an upstream URL: PLACE_ID (^[\w-]{1,512}$) and SESSION_TOKEN (^[\w-]{1,64}$) gate resolve/session; placeId is also encodeURIComponent'd; the user query goes in the Places POST body, not a URL — no injection. The new regexes (incl. ROUTING_COORDINATE) are all linear — no ReDoS.
• Bounds everywhere: isWithinBarbados on search results, resolved places, and reverse input (which also absorbs the controller's Number(lat)→NaN case → 400). Country predictions dropped so routing can't centroid to an arbitrary polyclinic.
• Key stays server-side (forms → API → Google); Places autocomplete + details send it via the X-Goog-Api-Key header, not the URL.
• Cost/abuse controls are good: controller @Throttle (30/10s, 200/60s), MIN_QUERY_LENGTH=3, 1h bounded cache (200), and Places session tokens.
• Graceful config: no key ⇒ Google skipped, runs on Nominatim (today's behaviour); unknown GEOCODE_PROVIDERS name fails at startup — so merging is safe before the key is provisioned (feature is dark until then).
• Validator correct: the "must be located" rule only applies when htmlType === "address-lookup" and geocodeTargets.coordinatesFieldId is set (routing fields); non-routing address fields still accept typed input; lookup-filled parish is read-only.
• 503 contract: searchAddresses throws on !res.ok and the field handles it — intentional, tested.

Non-blocking notes

1. Google key in the reverse URL query. Unavoidable for the classic Geocoding API, and current logging is safe (only error.message). To keep it safe: never log the axios error's config/request/toJSON() for that call (would expose ?key=), and note the key appears in any upstream/proxy access logs for that host. A separately-restricted key for Geocoding would be belt-and-braces.
2. Set a Google-side daily budget cap / quota when provisioning the key (ties to the case-management#368 gate) — the app-side throttle is per-client, so a cost-DoS across many clients is only truly bounded by a Google budget/quota.
3. 503 is a new contract for /geocode (was []). The address field handles the throw — just double-check no other consumer still expects [].

Scope: deep-read the geocode API (controller/service/providers), the forms geocode client, and the located-address validator; did not line-read every frontend file (combobox a11y, Leaflet pin-map, the 13 smoke-spec updates) — lower security risk and well-covered (API 1816 / forms 982 pass).

Correct, well-tested, cost- and security-aware; the only real pre-release items (Google key + budget cap) are already called out.

[IsaiahSama]

Reviewed at 8465b7a2. The two latest commits fix most of what I'd drafted against fc229020:

• Empty locked parish: search now fills it with parishAt, and fillRoutingParish fills it before validation in the submission pipeline. Both have specs.
• Search cache: only the primary provider's answer is cached.
• Reverse lookup: a lookup with no address now moves on to the next provider instead of stopping.
• Dead code and duplication: ConfirmDialog and its CSS are gone. inRing, BARBADOS_BOUNDS and ROUTING_COORDINATE are each defined once and shared.

The API side and the combobox/ARIA work are solid, and CI is green. One thing still blocks. Update: all addressed at d9b5ae1a, see the approval below.

Blocking (resolved in 8ab4863)

The pin map has no tiles in any deployed environment. The apps/forms CSP at amplify.yml:115 is still img-src 'self' data:. Only landing's CSP (line 43) allows https://tile.openstreetmap.org, so the browser blocks every tile location-pin-map.tsx requests. The applicant gets a grey box to click on. On routing forms that box is the only way past an address the lookup can't find, and a place that fails to resolve also opens it. Neither local runs nor the smoke tests catch this, because vite dev sends no CSP and the smoke spec clicks the map without looking at it. Fix: add the tile host to img-src on line 115. The inline comment is on the tile URL.

Non-blocking (inline)

• Race: a late resolvePlace answer can overwrite a pin placed after it.
• Map loading: the lazy-loaded map has no error boundary.
• Fetching: HttpModule has no timeout, so a provider that hangs never falls through to the next.
• Sea pins: a pin dropped in the sea still gets the nearest parish.
• Nits:
  • the session-token length check
  • an empty provider chain fails silently
  • the test-only data-value attribute
  • aria-controls is only set while the list is open
  • the smoke spec clicks the centre of the map

PR description is out of date

Please update it before merge:

• Edit confirmation: it still says this uses native confirm(), but nothing calls confirm() and the dialog has been deleted.
• Parish: "Changing the parish… drops the coordinate" and "the applicant picks the parish" no longer apply, because the parish is disabled.
• Tests: "the confirmation prompt and cancel" and "the parish change" aren't covered by any test.
• Reverse lookup status: "When no provider can answer, the endpoints return 503", but /geocode/reverse returns 200 with only the parish.
• Missing items: GET /geocode/polyclinic, the "routed to …" notice, the trust proxy change in main.ts, and the submission-side parish fill aren't mentioned.

Out of scope — follow-ups, not blockers

• Provider policies: OSM's tile usage policy and Nominatim's ban on autocomplete-as-you-type both apply to a production government service. They belong with the Google ToS A/B/C decision. Nominatim autocomplete predates this PR.
• Smoke coverage: every smoke test now mocks /geocode*, so none of them reaches a real provider. A scheduled Nominatim-only check would cover that.
• Map tests: location-pin-map.tsx has none. clampToBarbados is pure and easy to test.
• Pre-existing: search logs the applicant's typed query in its warn line, which puts address PII in the logs. The cache never drops expired entries. Both were the same at base.

[IsaiahSama]

Blocking. The forms CSP (amplify.yml:115, img-src 'self' data:) blocks this host, so no tiles load in any deployed environment. Only landing's CSP (line 43) allows https://tile.openstreetmap.org. Please add it to the forms img-src in this PR. vite dev sends no CSP, and the smoke spec clicks the map without needing tiles, so neither catches this.

[sajclarke]

Resolved in 8ab4863.

• The forms CSP (amplify.yml:115) now has img-src 'self' data: https://tile.openstreetmap.org. This agrees with the landing CSP.
• The tile layer was the only image host that the CSP blocked. The pin icon is a divIcon and needs no image.
• Not tested yet: tiles in the Amplify preview. Please look at the map on the preview after the build.

[IsaiahSama]

Non-blocking (race). placePin doesn't clear pendingPlace. The "Find it on the map" link can still be on screen after an earlier invalid Continue while a place's resolvePlace is in flight. If the applicant drops a pin then, the late answer overwrites the pin's coordinate, and the pin and its reverse-filled address stay as they were. locatingPlace also makes Continue wait for that stale lookup. Fix: add pendingPlace.current = null; at the top of placePin.

[sajclarke]

Resolved in 8ab4863.

• placePin now sets pendingPlace.current = null before it writes the pin. Thus a late resolvePlace answer does not overwrite the pin.
• New spec: "keeps a pin placed while the pick's coordinate was still loading". It fails without the fix.
• Continue can still wait for the old lookup. The wait is short, and after the wait the validator reads the pin's coordinate. Thus the result is correct.

[IsaiahSama]

Non-blocking. If the lazy chunk fails to load (for example a stale hash after a deploy), the error goes past this Suspense to the route error component, which replaces the whole form page. On routing forms the map is the only way through. A small error boundary here that falls back to the "suggestions unavailable" hint would keep the applicant on the page.

[sajclarke]

Resolved in 8ab4863.

• A small error boundary (MapLoadBoundary) is now around the Suspense of the map.
• If the map chunk does not load, the form page stays. The field shows: "We cannot show the map at the moment. Refresh the page to try again."
• We did not use the "suggestions unavailable" hint. That hint tells the applicant to type the address, but a routing field does not accept a typed address.
• New spec: "keeps the page when the map cannot load".

[IsaiahSama]

Suggestion, non-blocking. The fallback relies on a failing provider failing fast. HttpModule has no timeout (it had none at base either), so if Google hangs, the request hangs and Nominatim is never tried. HttpModule.register({ timeout: 5000 }) turns a hang into a rejection that falls through to the next provider.

[sajclarke]

Resolved in 8ab4863.

• GeocodeModule now imports HttpModule.register({ timeout: 5000 }).
• If a provider does not answer in 5 seconds, the request fails. Then the chain tries the next provider.

[IsaiahSama]

Decision, non-blocking. The nearest-vertex fallback has no distance cap, so any point inside the bounding box gets a parish, open sea included: parishAt(13.34, -59.41) returns st-john. With this PR that matters in more places. Search results and fillRoutingParish both use it now, so a pin in the sea routes on the guessed parish. If that isn't intended, cap the fallback at a few hundred metres and return "" beyond it.

[sajclarke]

Resolved in 8ab4863. Decision: cap the fallback at 1 km.

• parishAt now finds the nearest parish outline by the distance to each segment, not to each vertex. It gives a parish only if that distance is 1 km or less. A point further out is at sea, and parishAt returns "".
• Why 1 km, not a few hundred metres: The outlines are coarse (1,010 vertices, segments of up to 8 km). On the St Philip coast, the real shore is outside the outline. We measured Crane Beach at 526 m and Bottom Bay at 630 m from it. A 500 m cap refused those addresses.
• Sea points: parishAt(13.34, -59.41) is 18 km from the outline and now returns "". A point 2 km off the west coast (13.17, -59.66) also returns "".
• Pin in the sea: The reverse lookup gives no parish. The field then removes the coordinate and shows: "The pin is in the sea. Move it to where the address is." Thus Continue refuses the address until the applicant moves the pin.
• Specs: parish-at.spec.ts (two beaches, two sea points) and "locates nothing with a pin in the sea, and says to move it".

[IsaiahSama]

Nit. Places API (New) documents a session token of at most 36 characters. A token of 37–64 characters passes this check, Google returns 400, and search silently falls back. The forms app sends a v4 UUID (36 characters), so only direct callers are affected. {1,36} matches the documented limit.

[sajclarke]

Resolved in 8ab4863.

• SESSION_TOKEN is now ^[\w-]{1,36}$. This agrees with the documented limit.
• New spec: a 36-character token (a v4 UUID) goes to the provider. A 37-character token does not.

[IsaiahSama]

Nit. An unknown provider name stops startup, but an empty chain doesn't. GEOCODE_PROVIDERS="", or google alone without a key, gives [], and every search then returns 503 with nothing in the log to say why. A startup error or warning when providers.length === 0 would make that visible. Also, buildProviders takes env, but NominatimProvider still reads process.env.NOMINATIM_BASE_URL directly.

[sajclarke]

Resolved in 8ab4863.

• Empty chain: buildProviders now logs a warning at startup if the chain is empty. The warning gives the value of GEOCODE_PROVIDERS and tells that Google needs GOOGLE_GEOCODING_API_KEY.
• We used a warning, not an error. An error stops the API. An environment without a key must still start.
• NOMINATIM_BASE_URL: buildProviders now reads it from env and gives it to the NominatimProvider constructor. The provider does not read process.env.
• New specs: "warns at startup when no provider is configured" and "passes NOMINATIM_BASE_URL to the Nominatim provider".

[IsaiahSama]

Nit. data-value exists only for tests. A disabled <select> still exposes .value. select-field.spec.tsx already asserts toHaveValue("st-michael") on the disabled control, and Playwright's toHaveValue works on disabled selects too. Consider dropping it; the smoke edits that switched to it could then be reverted.

[sajclarke]

Resolved in 8ab4863.

• We removed data-value from the locked parish select.
• The smoke specs now read the value of the disabled select again, as on main: select[id=…] with .not.toHaveValue(""). In eho-frederick-miller-local, the spec uses .inputValue() again.
• The hairdresser spec does not go back to main, because the parish is now locked there. It uses .not.toHaveValue("") with the step timeout.
• select-field.spec.tsx keeps toHaveValue("st-michael") on the disabled control.

[IsaiahSama]

Nit. In the ARIA 1.2 combobox pattern, aria-controls stays on the input whether or not the popup is open. The APG examples keep it set and hide the listbox. Optional.

[sajclarke]

Resolved in 8ab4863.

• aria-controls is now always on the input.
• The listbox stays in the DOM. It has the hidden attribute while the list is closed. A CSS rule (.address-combobox__popup[hidden]) keeps it hidden, because the popup class sets display: flex.
• New spec: "keeps aria-controls on the input while the list is closed".

[IsaiahSama]

Nit (fragility). map.click() clicks the container's centre, which is CENTRE, where the default pin is anchored. A click on the marker doesn't fire the map's click, so this could miss. Clicking at an offset (e.g. { position: { x: 60, y: 60 } }) avoids that.

[sajclarke]

Resolved in 8ab4863.

• The spec now clicks the map at { x: 120, y: 120 }, not at the centre.
• This point is not on the unplaced pin at the centre. It is also not on the zoom control at the top-left. The map is 20rem (320 px) high.

[sajclarke]

Changes after the reviews

This comment gives the changes since the reviews on fc22902. The changes are in three commits: 63e5089, 8465b7a and 5c485be.

Items from @Zainab980

1. Empty parish after a pin (fix before merge)

• Problem: The pin lookup can fail on the client, for example with a 429. Then the form writes the coordinate, but not the parish. The parish field is locked. Thus the API refused the submission.
• Change: The API now fills an empty parish from the coordinate. It does this before validation (fillRoutingParish, called in submission-pipeline.service.ts).
• Result: A failed client lookup does not stop a submission. If there is no coordinate, the API still requires the parish.
• Not changed: The empty parish field stays locked. renderSelectField cannot keep state. If we unlock the empty field, it locks again after the applicant selects a parish. Then the applicant cannot correct a wrong selection.

2. One throttle bucket for all applicants (fix before release)

• Change: main.ts sets trust proxy to 1. The ALB is the only proxy. The DNS records for the API go directly to the ALB, with no CloudFront.
• Result: ThrottlerGuard now uses the address of the applicant, not the address of the ALB. Each applicant has a different bucket.
• Caution: This change applies to all throttled endpoints, not only to /geocode.
• Security: Express uses only the last entry in X-Forwarded-For. The ALB writes that entry. Thus a client cannot change its address with this header.

3. Cache of Google predictions (before release)

• Change: Each provider now tells if its terms permit a cache. Only Nominatim permits a cache. The API does not cache Google search results.
• The API caches a result only from the first provider. Thus fallback results do not stay in the cache after an outage.
• Option (C) now covers only the "near a non-Google map" question.

4. PR description (nit)

• The description now agrees with the code. We removed the text about confirm() and about parish changes.
• We removed ConfirmDialog and its CSS.

Other fixes from an internal review (63e5089)

• Search: A result with a coordinate but no parish gets the parish from the parish map.
• Reverse lookup: If a provider has no address at the pin, the API asks the next provider. Google often gives only a plus code for a rural pin.
• Pin and line 1: The pin stays when the applicant edits line 1. The pin gives the location, and the text only describes it. Before, an edit removed the pin, and Continue refused the address.
• Continue: If the coordinate of a selected place is not available yet, Continue waits for it. Before, Continue refused the address for a short time.
• Session token: The form uses the uuid package, not crypto.randomUUID(). crypto.randomUUID() fails on a page without HTTPS. Then the lookup stopped, and the applicant got no message.
• Lookup after a pick: Each keystroke now starts a lookup. Before, a pick could block the next lookup.
• Keyboard pin: The keyboard controls now attach when Leaflet adds the pin to the map. Before, they could attach late.
• Duplicates: The Barbados bounds, inRing and ROUTING_COORDINATE now have one definition each.

Notes from @ihtishamtanveer

• Key in the reverse URL: The logs record only the error message, not the request. No change.
• Budget cap: We added it to "Before release" in the description.
• 503 contract: Only the forms address field calls /geocode. It handles a 503.

Tests

• API: 1901 tests pass (DB_HOST= vitest run). New tests cover the parish fill, the reverse fallback and the cache rules.
• Forms: 992 tests pass. New tests cover the pin after an edit, the lookup after a pick and Continue during a place lookup.
• nx run-many -t build --exclude=landing, tsc -b and lint pass.
• Not done: a smoke run on sandbox, and a keyboard test of the pin in a browser.

[ihtishamtanveer]

Re-reviewed the delta since my last pass (head 8ab48637). The follow-ups are thorough and the blocker IsaiahSama raised is resolved.

Resolved ✅

• 🔴 Blocker — map tiles CSP: amplify.yml:115 (forms CSP) now includes https://tile.openstreetmap.org in img-src, so the pin map renders in deployed envs rather than a grey box.
• trust proxy = 1 — trusts exactly the ALB hop, so req.ip is the real client and X-Forwarded-For can't be spoofed (correct, not a blind true); keeps the throttler sound.
• HTTP timeout — HttpModule.register({ timeout: 5000 }), so a hanging provider times out and the chain falls through.
• Caching is now ToS-safe — only the primary, cacheable provider is cached (index === 0 && provider.cacheable), so Google Places answers are never cached and a fallback's answer can't outlive the outage that caused it.
• Empty locked parish filled from the coordinate (search results + submission pipeline); reverse now continues past an empty answer (rural plus-code pins); session token tightened to {1,36}; empty provider chain warns loudly instead of silently 503-ing.
• Race handled — latest-wins sequence counter + AbortController so a late resolvePlace/search can't overwrite a newer pin; lazy map has a load-failure fallback.
• PR description updated (no more confirm() / parish-picks claims).

Still standing (non-blocking)

• Logging hygiene (low-sev): the reverse key is in the Geocoding URL — never log the axios config/toJSON() for that call; and search logs the applicant's typed query (lookup failed for "${query}"), which is address PII. Flagged by IsaiahSama as pre-existing, but this PR widens the surface (Google now in the chain) — redacting to a length/hash would close it.
• Google-side budget cap/quota on the key (ties to case-management#368) — the app throttle is per-client, so only a Google budget bounds a multi-client cost-DoS.
• A few nits remain for @IsaiahSama to clear (sea-pin → nearest parish, aria-controls only while open, the data-value test attr) — none blocking.

Net: materially improved — the blocker is gone and the follow-ups are correct. Only logging-hygiene + the ops budget cap are worth tidying, and neither blocks. reviewDecision is still CHANGES_REQUESTED only because @IsaiahSama's review hasn't been re-submitted; his blocker is addressed in code, so it's ready for a re-review to clear.

[Zainab980]

Re-reviewed at 8ab4863. All three of my items are fixed, and I verified Isaiah's tile blocker on the deployed preview. LGTM from me 👍

Verified: I read all 13 threads, both reviews and the summary comment, and checked each fix at head (6 behind main, clean, CI green). I didn't run suites or smoke.

Resolved

• Empty locked parish: fillRoutingParish runs before validation. All 14 locked parishes are the routing parish, so the fill covers each one.
• Throttle bucket: trust proxy = 1 keys on the citizen, and the last XFF hop is the ALB's, so it can't be spoofed.
• Google cache: only index === 0 && provider.cacheable is cached.
• Map tiles (Isaiah's blocker): on the forms preview at 8ab4863, the served CSP is img-src 'self' data: https://tile.openstreetmap.org, and an OSM tile loads under it. I didn't open the real map on a routed form.
• The description now matches the code.

Before merge, cheap: this closes #2679, so Isaiah's out-of-scope items will only live in a closed PR. Please file one follow-up issue: OSM tile and Nominatim usage policies (Nominatim is the only provider until the key lands), the typed query in the search warn log (PII), expired cache entries never evicted, location-pin-map tests, and a real-provider smoke check.

One small edge, non-blocking (inline): since the 1 km sea cap, a picked place with no parish keeps its coordinate, unlike a sea pin.

[Zainab980]

Non-blocking, a knock-on from the 1 km cap on parish-at.ts: the pin path treats "no parish" as "in the sea" and drops the coordinate, but a picked place doesn't. A parish-less place more than 1 km beyond the outline now returns parish: "" here, the client keeps the coordinate, the locked parish stays empty, and fillRoutingParish gets "" too. That's a 422 the applicant can't fix. Rare, but the same rule closes it:

const parish = place.parish || parishAt(Number(place.lat), Number(place.lon));
// No parish means it's offshore, as for a sea pin: routes nowhere, so the field offers the map.
if (!parish) throw new NotFoundException("Place not found");
return { ...place, parish };

[sajclarke]

Resolved in 6c2abbe.

• resolve() now uses your rule. A place with no parish is offshore, and resolve() answers 404. The address field then offers the map, as for a sea pin.
• search() uses the same rule. It drops a result that has a coordinate but no parish. Thus the applicant cannot select an offshore result.
• Specs: "refuses a place offshore, where there is no parish" and "drops a result offshore, where there is no parish".

[sajclarke]

Changes after the second reviews

This comment gives the changes in 6c2abbe.

Offshore place (@Zainab980, inline)

• resolve() answers 404 for a place with no parish. The address field then offers the map.
• search() drops a result with a coordinate but no parish.

Logging (@ihtishamtanveer, @IsaiahSama)

• The search failure warning gives the length of the query, not the query. A typed address is personal data.
• The reverse failure warning rounds the pin to about 1 km.
• The API key: the logs record only the error message, never the request config. No change.

Follow-up issue (@Zainab980)

• Address lookup follow-ups from #2941: provider policies, cache eviction, map tests, real-provider smoke #3062 now tracks the items that are out of scope: the OSM tile and Nominatim usage policies, cache eviction, tests for location-pin-map.tsx, and a smoke check with a real provider.

Not in code

• The Google budget cap stays in "Before release" in the description. Set it when the key is provisioned.

Tests: API 1912 pass. The API build, tsc -b and lint pass.

@IsaiahSama, the CSP blocker and all the inline items are resolved. Please review again.

[sajclarke]

@IsaiahSama, thank you for the review. This comment tells what we fixed. The fixes are in 8ab4863 and 6c2abbe.

Blocker

• Map tiles: The forms CSP now permits https://tile.openstreetmap.org in img-src. @Zainab980 checked the preview. The tiles load.

Inline items

• Race: A pin now stops the pending place lookup. A late answer does not change the pin.
• Map load: If the map does not load, the form page stays. The field tells the applicant to refresh the page.
• Timeout: Each provider call stops after 5 seconds. Then the next provider answers.
• Sea pins: A point more than 1 km from the parish outlines has no parish. A pin there does not locate the address. The field tells the applicant to move the pin. We used 1 km, not less, because some real beach addresses are 630 m outside the outlines.
• Session token: The limit is now 36 characters.
• Empty provider list: The API logs a warning at startup. NOMINATIM_BASE_URL now comes from env.
• data-value: We removed it. The smoke tests read the value of the select again.
• aria-controls: It is always on the input. The list stays in the page and is hidden when closed.
• Smoke click: The test clicks the map away from the centre.

Other items from your review

• Description: It now agrees with the code.
• Typed query in the logs: The log now gives only the length of the query.
• Out of scope: Issue Address lookup follow-ups from #2941: provider policies, cache eviction, map tests, real-provider smoke #3062 tracks the OSM and Nominatim policies, cache eviction, map tests and a real-provider smoke check.

Each inline comment has a reply with the details. Please review again.

[IsaiahSama]

Re-reviewed at d9b5ae1a. Approving: every earlier thread is closed, and what's left below is non-blocking.

My earlier threads, all closed:

• CSP (was blocking): the forms img-src now allows https://tile.openstreetmap.org (amplify.yml:115).
• Race: placePin clears pendingPlace. Spec: "keeps a pin placed while the pick's coordinate was still loading".
• Map loading: MapLoadBoundary wraps the map's Suspense. Spec: "keeps the page when the map cannot load".
• Timeout: HttpModule.register({ timeout: 5000 }).
• Sea pins: parishAt is capped at 1 km from the outline. Run at this commit: open sea and 2 km off the west coast return "", and Crane Beach keeps st-philip.
• Nits:
  • the session token is now {1,36}
  • an empty provider chain warns at startup
  • data-value is gone
  • aria-controls is always set, with the listbox hidden
  • the smoke clicks off-centre (the mocked reverse lookup gives it a parish, so the pin is accepted)

Zainab980's threads also check out: server-side parish fill, trust proxy, no caching of Google answers, and an offshore place answers 404.

The PR description now matches the code.

Non-blocking (inline):

• pinOffshore isn't reset when a pick or a cleared address drops the pin.
• From the main merge: collectStepErrorCodes doesn't skip the locked parish.

Still to check by eye: map tiles on the Amplify preview.

[IsaiahSama]

Nit, non-blocking. pinOffshore is only reset in placePin (L307). Here and in clearFoundDetails (L215), the pin is dropped but the flag stays set. If the map then reopens (e.g. resolvePlace fails after this pick and calls setShowPinMap(true)), the hint at L400 says "The pin is in the sea…" with no pin on the map. Fix: add setPinOffshore(false) next to each setPin(null).

[IsaiahSama]

Non-blocking, from the main merge. stepPassesValidation (L378, #2812/#2937) judges off-screen steps with collectStepErrorCodes, which doesn't have this skip.

• Scenario: the pin's reverse lookup failed, so the locked parish is empty (the API fills it in at submission).
• On the return from a Change, that step counts as failing, and the applicant is sent back to it.
• Its own Continue then passes, so this is a detour, not a dead end.

Fix: add field.filledByAddressLookup || to the continue condition at L339–343, so the two validators agree.