From f16fc2db0de213a4f45ac072005d4910fa88740b Mon Sep 17 00:00:00 2001 From: daikaginza Date: Fri, 28 Aug 2026 18:00:01 +0700 Subject: [PATCH 1/3] feat(search): fall back to keyless Photon geocoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Place search was Google-only: searchAndFlyTo threw without a key, and with a key whose Geocoding API is not enabled it returned nothing. Both cases leave the LOCATION box and the fly_to_location voice tool unable to resolve a name, even though every other layer in the app works keyless. src/keylessGeocoder.js adapts Photon (komoot, over OpenStreetMap) into the shape the Google path already produces — lat/lng, label, Google-style types, {southwest,northeast} bounds — so geocodeNavigationMode, regionFramingPlan and the region-swath cap keep running on unchanged inputs. The translation lives at the boundary and nowhere else; Google remains primary and its branch is unchanged when it answers. Detection is the empty result, not an exception: an unenabled Geocoding API answers HTTP 200 with REQUEST_DENIED. Places near-view recovery is now attempted only when a key exists, since Places is a Google service too. Three translations are load-bearing, and each was found by querying the live service rather than by reasoning about OSM tagging: - Google's `bounds` PREFERS results inside the box; Photon's `bbox` FILTERS to it. Sent as a bbox, searching "Ho Guom, Ha Noi" while looking at Austin returns features: []. Sent as Photon's lat/lon proximity bias it finds the lake, and "Sixth Street" still resolves to Austin's over Austin and to England's over London. - Photon's `extent` is [west, north, east, south]. Read as [w,s,e,n] it yields an inverted but finite box that frames the camera at nothing. - A specific OSM key=value rule must beat the coarse `type`. Photon tags a lake `water=lake` with `type: 'other'` — unmapped, that framed Hoan Kiem Lake as a precise place and put the camera 26 m over the water — and tags a town square `type: 'locality'`, which framed Times Square as if it were a city. searchAndFlyTo no longer throws when no key is present. Both callers already read a null return as not-found (ui.js renders its miss branch, gevActions reports ok: false), so the removed throw has no consumer. Verified end to end through the real search box, with a key Google refuses: one denied Google geocode, one Photon request, camera from Austin (30.2700, -97.7400) to 21.0288, 105.8524 — Hoan Kiem Lake to within metres — and "Ha Noi" framed at province scale. npm test 2604/2604; npm run build clean; npm run test:track 97/3, the same three failures unmodified main produces on this machine. --- CHANGELOG.md | 8 ++ DATA_SOURCES.md | 1 + docs/CURRENT-STATE.md | 6 + src/data/dataCredits.js | 8 ++ src/keylessGeocoder.js | 250 +++++++++++++++++++++++++++++++++++ src/keylessGeocoder.test.mjs | 199 ++++++++++++++++++++++++++++ src/locations.js | 32 +++-- 7 files changed, 496 insertions(+), 8 deletions(-) create mode 100644 src/keylessGeocoder.js create mode 100644 src/keylessGeocoder.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 7398d8418..40c545a2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,14 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md - Existing cached refusals are now ignored immediately, including during stale-data fallback. Concurrent identical requests share the same last-good fallback when all mirrors refuse, without duplicating upstream requests. +### Added + +- Keyless place search. The LOCATION search box and the `fly_to_location` voice + tool now resolve place names through Photon (komoot, over OpenStreetMap) when + no Google Maps key is configured — previously the lookup threw. Google stays + the primary path and is unchanged when it answers; the fallback also covers a + key whose Geocoding API is not enabled, which Google reports as HTTP 200 with + `REQUEST_DENIED`, so an empty result is the detector rather than an error. ## [0.1.1] — 2026-09-01 — Installation and live-data fixes diff --git a/DATA_SOURCES.md b/DATA_SOURCES.md index faa351dca..650d3fe6b 100644 --- a/DATA_SOURCES.md +++ b/DATA_SOURCES.md @@ -27,6 +27,7 @@ How to read this: | **OpenStreetMap (Overpass API)** | Road geometry for traffic | ODbL 1.0 | "© OpenStreetMap contributors" | | **TomTom Traffic API** (flow vector tiles) | Live congestion coloring for the traffic layer (optional, BYOK) | [TomTom for Developers terms](https://developer.tomtom.com) (proprietary, your own key; free tier currently 200K tile requests/month — see [current pricing](https://docs.tomtom.com/pricing/)) | "Traffic flow data © TomTom" — registered when live mode activates | | **OpenStreetMap (Overpass API)** | Viewport-bounded mapped installation context for Global Context | ODbL 1.0 | "© OpenStreetMap contributors" (incomplete mapped context) | +| **Photon** (komoot) | Keyless place search — the fallback when no Google Maps key is configured, or when Google declines the Geocoding request | Service: free public instance, [fair use](https://photon.komoot.io) (no bulk/heavy use); underlying data: **ODbL 1.0** | "Photon (komoot)" + "© OpenStreetMap contributors" | | **OpenStreetMap (Nominatim)** | Reverse-geocoded place label in the cockpit Local Info page | ODbL 1.0 + Nominatim usage policy | "© OpenStreetMap contributors" | | **Open-Meteo** | Current weather in the cockpit Local Info page and cockpit-local dynamic atmospheric effects | [CC BY 4.0 data licence and adjacent-link attribution requirement](https://open-meteo.com/en/licence) | Linked "Weather data by Open-Meteo.com" beside the displayed local data | | **Google News RSS** | Primary locality-matched headlines in the cockpit Regional News page | [Google News Terms of Service](https://www.google.com/intl/en_us/terms_google_news.html) restrict use to personal, noncommercial use; linked articles remain third-party publisher content and retain publisher terms | "Google News RSS" plus each article's linked publisher/domain | diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index a2bb9dad1..00003a260 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2181,6 +2181,12 @@ silently demoting every later lookup for the session. - Input is the live basemap label context (place/street/nearby-place labels + enabled layers) — the model is instructed not to infer from coordinates. - Output is sanitized to exactly five words; falls back to the deterministic telemetry summary on error/timeout (5s abort); typewriter animation on update. +### Keyless place search (August 2026) + +- `src/keylessGeocoder.js` is a Photon (komoot/OpenStreetMap) adapter used by `searchAndFlyTo` when the Google geocode produces no result: no key configured, or a key whose Geocoding API is not enabled — Google answers HTTP 200 with `REQUEST_DENIED`, so the empty result is the detector, not an error. Google stays the primary path and is byte-unchanged when it answers, and the Places near-view recovery is only attempted when a key exists, since Places is a Google service. +- The adapter normalizes into the shape the Google path already produces — `lat`/`lng`, label, Google-style `types`, `{southwest,northeast}` bounds — so `geocodeNavigationMode`, `regionFramingPlan` and the region-swath cap keep running unchanged. Two translations are load-bearing and are pinned by tests: Photon's `extent` is `[west, north, east, south]`, and a specific OSM `key=value` rule beats the coarse `type` (Photon reports a lake as `water=lake` with `type: 'other'`, which framed at building range put the camera 26 m over the water; it reports a town square as `type: 'locality'`, which framed a plaza as a city). +- The shared `viewportBias()` string is sent as Photon's `lat`/`lon` PROXIMITY bias, never as `bbox`: Google's `bounds` prefers, Photon's `bbox` filters, and the literal translation makes every off-screen search return nothing. One result per query, memoized by query+bias (misses included) to respect komoot's fair use. + ### Map Stack Switcher (June 2026) - `src/mapStackController.js` switches between Google Photorealistic 3D (`photoreal`, the default when a Google or ion key is present), keyless Esri World Imagery (the zero-key default landing, with keyless terrain), Bing Aerial / Aerial-with-Labels via Cesium ion world imagery (require `CESIUM_ION_TOKEN`), and OSM tile fallback. Bing Road is **retired**: it is gone from `MAP_STACKS`, from the `set_map_stack` enum, and from the voice aliases (road phrasings now resolve to OSM, the one shipped road basemap). An old `map=bing-road` link is simply an unknown id and takes `setStack()`'s existing photoreal fallback with the Google 3D tile lit — pinned live in `scripts/qa-map-source-tray.mjs`. diff --git a/src/data/dataCredits.js b/src/data/dataCredits.js index b35da9e00..009ff6d01 100644 --- a/src/data/dataCredits.js +++ b/src/data/dataCredits.js @@ -73,6 +73,14 @@ export const DATA_CREDITS = [ '© OpenStreetMap contributors ' + '(ODbL 1.0)', }, + { + key: 'photon-geocoder', + html: + 'Keyless place search: ' + + 'Photon (komoot) over ' + + '© OpenStreetMap contributors ' + + '(ODbL 1.0)', + }, { key: 'military-installations-osm', html: diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js new file mode 100644 index 000000000..9de6faea9 --- /dev/null +++ b/src/keylessGeocoder.js @@ -0,0 +1,250 @@ +/** + * Keyless place search — Photon (komoot), an OpenStreetMap geocoder. + * + * The app's place search is Google Geocoding, which needs a billing-enabled + * key. This module is the fallback for the two cases where that key produces + * nothing: no key configured at all, and a key whose Geocoding API is not + * enabled (Google answers HTTP 200 with `REQUEST_DENIED`, so the status — not + * the response code — is what decides). + * + * It is an ADAPTER, not a second search stack. Results are normalized into the + * exact shape `searchAndFlyTo` already consumes from Google — `lat`/`lng`, a + * label, Google-style `types`, and `{southwest,northeast}` bounds — so every + * downstream framing decision (`geocodeNavigationMode`, `regionFramingPlan`, + * the region-swath cap) keeps running unchanged on unchanged inputs. The + * translation lives here, at the boundary, and nowhere else. + * + * Photon is free, keyless, and CORS-open, so this runs in the browser on the + * same path Google does. It asks for one hit and caches by query+bias to stay + * inside komoot's fair-use expectations. + */ + +const PHOTON_ENDPOINT = 'https://photon.komoot.io/api/'; + +/** Photon is a courtesy service; fail fast rather than hold the search open. */ +const PHOTON_TIMEOUT_MS = 6000; + +/** Bounded memo — a search box re-issues the same query on every keystroke. */ +const PHOTON_CACHE_MAX = 64; +const photonCache = new Map(); + +/** + * Photon's own coarse class → the Google Place Type `geocodeNavigationMode` + * reads. Only types that function consults are worth emitting; anything else + * falls through to `precise-place`, which is the correct default for a house + * or a POI. + */ +const PHOTON_TYPE_TO_GOOGLE = Object.freeze({ + country: 'country', + state: 'administrative_area_level_1', + county: 'administrative_area_level_2', + city: 'locality', + district: 'sublocality', + locality: 'locality', + street: 'route', +}); + +/** + * OSM `key=value` → Google Place Type. `null` means "this tag is a precise + * place" and, like any other entry, stops the coarse class from being consulted. + * + * Two failure directions matter, and both were found by querying the live + * service rather than by reasoning about OSM tagging: + * + * - Too coarse: a park or lake that arrives without an area type frames at + * building range, which is the exact bug `geocodeNavigationMode`'s area types + * exist to prevent — searching "Ho Guom" put the camera 26 m over the water. + * - Too broad: Photon classifies a town square as `type: 'locality'`, so + * deferring to the coarse class frames Times Square as if it were a city. + * The explicit `null` below is what stops that. + */ +const OSM_TAG_TO_GOOGLE = Object.freeze({ + 'leisure=park': 'park', + 'leisure=nature_reserve': 'park', + 'leisure=garden': 'park', + 'leisure=stadium': 'stadium', + 'boundary=national_park': 'park', + 'boundary=protected_area': 'park', + 'tourism=zoo': 'zoo', + 'tourism=theme_park': 'amusement_park', + 'amenity=university': 'university', + 'amenity=college': 'university', + 'amenity=grave_yard': 'cemetery', + 'landuse=cemetery': 'cemetery', + 'landuse=forest': 'natural_feature', + 'aeroway=aerodrome': 'airport', + 'shop=mall': 'shopping_mall', + 'place=region': 'natural_feature', + 'place=suburb': 'sublocality', + 'place=neighbourhood': 'sublocality', + 'place=quarter': 'sublocality', + 'place=borough': 'sublocality', + 'place=square': null, +}); + +/** + * OSM keys whose features are areas whatever their value — lakes, rivers, + * peaks, woods, protected land. Photon reports every one of these with + * `type: 'other'`, so the key is the only signal that they are not buildings. + */ +const AREA_OSM_KEYS = Object.freeze(new Set(['natural', 'water', 'waterway', 'landuse'])); + +/** + * Google-style types for one Photon feature's properties. + * + * A specific `key=value` mapping is authoritative and ends the lookup; the + * coarse `type` is consulted only when no tag rule matched. Deferring to the + * coarse class after a tag rule is what mis-framed town squares as cities. + * @param {object} properties - Photon feature `properties`. + * @returns {string[]} Google Place Types, possibly empty (a precise place). + */ +export function photonResultTypes(properties) { + const key = String(properties?.osm_key || ''); + const value = String(properties?.osm_value || ''); + const tag = `${key}=${value}`; + + if (Object.prototype.hasOwnProperty.call(OSM_TAG_TO_GOOGLE, tag)) { + const mapped = OSM_TAG_TO_GOOGLE[tag]; + return mapped ? [mapped] : []; + } + if (AREA_OSM_KEYS.has(key)) return ['natural_feature']; + if (key === 'highway') return ['route']; + + const coarse = PHOTON_TYPE_TO_GOOGLE[String(properties?.type || '')]; + return coarse ? [coarse] : []; +} + +/** + * Photon `extent` → the `{southwest,northeast}` bounds the flight code frames. + * Photon orders it [west, north, east, south]; a naive [w,s,e,n] read produces + * an inverted box that still looks like a valid viewport. + * @param {number[]} extent - Photon `properties.extent`. + * @returns {?{southwest:{lat:number,lng:number}, northeast:{lat:number,lng:number}}} + */ +export function photonExtentToBounds(extent) { + if (!Array.isArray(extent) || extent.length !== 4) return null; + const [west, north, east, south] = extent.map(Number); + if (![west, north, east, south].every(Number.isFinite)) return null; + if (Math.abs(north) > 90 || Math.abs(south) > 90) return null; + if (Math.abs(west) > 180 || Math.abs(east) > 180) return null; + return { + southwest: { lat: Math.min(north, south), lng: Math.min(west, east) }, + northeast: { lat: Math.max(north, south), lng: Math.max(west, east) }, + }; +} + +/** + * Human label for a Photon feature — its name plus the coarsest containing + * places that are not already in the name, mirroring the single-line address + * Google's `formatted_address` provides. + * @param {object} properties - Photon feature `properties`. + * @returns {string} + */ +export function photonResultLabel(properties) { + const name = String(properties?.name || '').trim(); + const parts = [name]; + for (const field of ['district', 'city', 'state', 'country']) { + const part = String(properties?.[field] || '').trim(); + if (part && !parts.includes(part)) parts.push(part); + } + return parts.filter(Boolean).join(', '); +} + +/** + * Photon GeoJSON feature → the normalized geocode result `searchAndFlyTo` + * consumes. Returns null for a feature without usable coordinates. + * @param {object} feature - One entry of Photon's `features` array. + * @returns {?{lat:number, lng:number, label:string, types:string[], viewport:?object}} + */ +export function normalizePhotonFeature(feature) { + const [lng, lat] = feature?.geometry?.coordinates || []; + if (!Number.isFinite(lat) || !Number.isFinite(lng)) return null; + if (Math.abs(lat) > 90 || Math.abs(lng) > 180) return null; + + const properties = feature.properties || {}; + return { + lat, + lng, + label: photonResultLabel(properties) || String(properties.name || ''), + types: photonResultTypes(properties), + viewport: photonExtentToBounds(properties.extent), + }; +} + +/** + * Photon request URL for one query. + * + * `bias` is the SAME `"swLat,swLng|neLat,neLng"` string `viewportBias()` builds + * for the Google call, so the two paths cannot drift apart — but it is sent as + * Photon's `lat`/`lon` PROXIMITY bias, not as `bbox`. + * + * That distinction is the whole correctness of this function. Google's `bounds` + * prefers results inside the box and still returns ones outside it; Photon's + * `bbox` is a hard filter. Translating the rectangle literally makes every + * search for somewhere off-screen return nothing — searching "Ho Guom, Ha Noi" + * while looking at Austin answers `features: []`. Sent as `lat`/`lon` it finds + * Hoan Kiem Lake, while "Sixth Street" still resolves to Austin's over Austin + * and to England's over London. Soft bias is the semantics Google gives, so + * soft bias is what the adapter must produce. + * @param {string} query - Free-text place name. + * @param {{bias?: ?string}} [options] + * @returns {string} + */ +export function photonSearchUrl(query, { bias = null } = {}) { + const url = new URL(PHOTON_ENDPOINT); + url.searchParams.set('q', String(query ?? '')); + url.searchParams.set('limit', '1'); + + const corners = String(bias ?? '').split('|'); + if (corners.length === 2) { + const [swLat, swLng] = corners[0].split(',').map(Number); + const [neLat, neLng] = corners[1].split(',').map(Number); + if ([swLat, swLng, neLat, neLng].every(Number.isFinite)) { + url.searchParams.set('lat', String((swLat + neLat) / 2)); + url.searchParams.set('lon', String((swLng + neLng) / 2)); + } + } + return url.toString(); +} + +/** Evict the oldest memo entries until the cache is within its cap. */ +function trimPhotonCache() { + while (photonCache.size > PHOTON_CACHE_MAX) { + const oldest = photonCache.keys().next().value; + if (oldest === undefined) break; + photonCache.delete(oldest); + } +} + +/** + * Geocode a place name without any API key. Never throws: a network failure, + * a timeout, or an empty result all resolve to null, which the caller reads as + * "not found" exactly as it reads a Google miss. + * @param {string} query - Free-text place name. + * @param {{bias?: ?string, fetchImpl?: Function}} [options] + * @returns {Promise} + */ +export async function geocodeKeyless(query, { bias = null, fetchImpl = fetch } = {}) { + const trimmed = String(query ?? '').trim(); + if (!trimmed) return null; + + const url = photonSearchUrl(trimmed, { bias }); + if (photonCache.has(url)) return photonCache.get(url); + + let result = null; + try { + const response = await fetchImpl(url, { signal: AbortSignal.timeout(PHOTON_TIMEOUT_MS) }); + if (response.ok) { + const body = await response.json(); + result = normalizePhotonFeature(body?.features?.[0]); + } + } catch { + result = null; + } + + // Misses are cached too: a typo re-issued on every keystroke should cost one + // request, not one per stroke. + photonCache.set(url, result); + trimPhotonCache(); + return result; +} diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs new file mode 100644 index 000000000..922acd9d0 --- /dev/null +++ b/src/keylessGeocoder.test.mjs @@ -0,0 +1,199 @@ +// KEYLESS GEOCODER — the Photon adapter that keeps place search working +// without a Google key. +// +// The whole value of this module is that its output is INDISTINGUISHABLE to +// everything downstream: `geocodeNavigationMode` and the framing code must keep +// making the same decisions they make on a Google result. So these tests check +// the translation, not the network — three things that go wrong silently: +// +// 1. Photon's `extent` is [west, north, east, south]. Read as [w,s,e,n] it +// yields an inverted-but-plausible box, and the camera frames nonsense. +// 2. Photon calls a national park `type: 'other'`. Dropped, a park geocodes +// as a precise place and the camera lands on a random patch of grass — +// the exact bug geocodeNavigationMode's area types exist to prevent. +// 3. A miss must be `null`, never a throw: both callers read null as +// "not found" and a throw would surface as a broken search box. +// +// Run with: npm test +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + geocodeKeyless, + normalizePhotonFeature, + photonExtentToBounds, + photonResultLabel, + photonResultTypes, + photonSearchUrl, +} from './keylessGeocoder.js'; +import { geocodeNavigationMode } from './locations.js'; + +/** Photon's real answer for "Hanoi", trimmed to the fields we consume. */ +const HANOI = { + geometry: { type: 'Point', coordinates: [105.854041, 21.0283334] }, + properties: { + osm_key: 'place', + osm_value: 'city', + type: 'city', + name: 'Hà Nội', + country: 'Việt Nam', + countrycode: 'VN', + extent: [105.2889615, 21.3854176, 106.0200407, 20.5645154], + }, +}; + +test('a city normalizes into the shape the Google path produces', () => { + const result = normalizePhotonFeature(HANOI); + + assert.equal(result.lat, 21.0283334); + assert.equal(result.lng, 105.854041); + assert.equal(result.label, 'Hà Nội, Việt Nam'); + assert.deepEqual(result.types, ['locality']); + assert.equal(geocodeNavigationMode(result.types), 'city-overview'); +}); + +test('the extent is read as [west, north, east, south], not as [w,s,e,n]', () => { + const bounds = photonExtentToBounds(HANOI.properties.extent); + + // Hanoi spans roughly 20.56..21.39 N and 105.29..106.02 E. A [w,s,e,n] read + // would put north at 21.385 and south at 106.02 — a box that still has four + // finite numbers and would frame the camera at nothing. + assert.equal(bounds.southwest.lat, 20.5645154); + assert.equal(bounds.northeast.lat, 21.3854176); + assert.equal(bounds.southwest.lng, 105.2889615); + assert.equal(bounds.northeast.lng, 106.0200407); + assert.ok(bounds.northeast.lat > bounds.southwest.lat, 'north must exceed south'); + assert.ok(bounds.northeast.lng > bounds.southwest.lng, 'east must exceed west'); +}); + +test('a malformed or out-of-range extent yields no bounds rather than a bad box', () => { + assert.equal(photonExtentToBounds(undefined), null); + assert.equal(photonExtentToBounds([1, 2, 3]), null); + assert.equal(photonExtentToBounds([0, 95, 10, -95]), null, 'latitude past the pole'); + assert.equal(photonExtentToBounds([-200, 10, 10, 0]), null, 'longitude past the antimeridian'); +}); + +test('area features keep an area type so the camera frames them, not a rooftop', () => { + // Every row is a real Photon answer, captured from the live service. The + // lake rows are why: Photon tags water as `water=lake`, not `natural=water`, + // and the guess cost a camera 26 m over Hoan Kiem Lake. + const cases = [ + [{ type: 'other', osm_key: 'water', osm_value: 'lake' }, 'natural_feature', 'area-overview'], + [{ type: 'other', osm_key: 'leisure', osm_value: 'park' }, 'park', 'area-overview'], + [{ type: 'other', osm_key: 'place', osm_value: 'region' }, 'natural_feature', 'area-overview'], + [{ type: 'other', osm_key: 'natural', osm_value: 'peak' }, 'natural_feature', 'area-overview'], + [{ type: 'other', osm_key: 'waterway', osm_value: 'river' }, 'natural_feature', 'area-overview'], + [{ type: 'house', osm_key: 'aeroway', osm_value: 'aerodrome' }, 'airport', 'area-overview'], + [{ type: 'house', osm_key: 'amenity', osm_value: 'university' }, 'university', 'area-overview'], + [{ type: 'street', osm_key: 'highway', osm_value: 'motorway' }, 'route', 'street-corridor'], + [{ type: 'country', osm_key: 'place', osm_value: 'country' }, 'country', 'region-overview'], + [{ type: 'state', osm_key: 'place', osm_value: 'state' }, 'administrative_area_level_1', 'region-overview'], + [{ type: 'district', osm_key: 'place', osm_value: 'suburb' }, 'sublocality', 'neighborhood-close'], + ]; + + for (const [properties, expectedType, expectedMode] of cases) { + const types = photonResultTypes(properties); + assert.ok( + types.includes(expectedType), + `${properties.osm_key}=${properties.osm_value} must carry ${expectedType}, got ${JSON.stringify(types)}`, + ); + assert.equal(geocodeNavigationMode(types), expectedMode); + } +}); + +test('a tag rule wins over the coarse class — a town square is not a city', () => { + // Photon reports Times Square and Quang truong Ba Dinh as place=square with + // type: 'locality'. Falling through to the coarse class framed a plaza as if + // it were a whole city. + const square = { type: 'locality', osm_key: 'place', osm_value: 'square' }; + assert.deepEqual(photonResultTypes(square), []); + assert.equal(geocodeNavigationMode(photonResultTypes(square)), 'precise-place'); +}); + +test('an ordinary building stays a precise place', () => { + const types = photonResultTypes({ type: 'house', osm_key: 'building', osm_value: 'yes' }); + assert.equal(geocodeNavigationMode(types), 'precise-place'); +}); + +test('the label names the place, then only containers it does not already name', () => { + assert.equal( + photonResultLabel({ name: 'Hoàn Kiếm Lake', district: 'Hoàn Kiếm', city: 'Hà Nội', country: 'Việt Nam' }), + 'Hoàn Kiếm Lake, Hoàn Kiếm, Hà Nội, Việt Nam', + ); + assert.equal( + photonResultLabel({ name: 'Hà Nội', city: 'Hà Nội', country: 'Việt Nam' }), + 'Hà Nội, Việt Nam', + 'a city must not repeat its own name', + ); +}); + +test('the Google bias becomes a SOFT proximity bias, never a hard bbox', () => { + const url = new URL(photonSearchUrl('Sixth Street', { bias: '30.2000,-97.8000|30.3000,-97.7000' })); + + assert.equal(url.searchParams.get('q'), 'Sixth Street'); + assert.equal(url.searchParams.get('limit'), '1'); + assert.equal(url.searchParams.get('lat'), '30.25', 'centre of the biased rectangle'); + assert.equal(url.searchParams.get('lon'), '-97.75'); + + // Measured against the live service: Photon's bbox FILTERS, Google's bounds + // PREFERS. Sending the rectangle as a bbox makes every off-screen search + // return features: [] — "Ho Guom, Ha Noi" while looking at Austin finds + // nothing at all. This assertion is the guard against that regression. + assert.equal(url.searchParams.get('bbox'), null, 'a bbox would filter out every off-screen place'); +}); + +test('an absent or unparsable bias drops the bias instead of sending a broken one', () => { + for (const bias of [undefined, 'nonsense', 'a,b|c,d']) { + const url = new URL(photonSearchUrl('Hanoi', bias === undefined ? {} : { bias })); + assert.equal(url.searchParams.get('lat'), null); + assert.equal(url.searchParams.get('lon'), null); + assert.equal(url.searchParams.get('bbox'), null); + } +}); + +test('a network failure resolves to null — callers read null as not-found', async () => { + const rejecting = () => Promise.reject(new Error('offline')); + assert.equal(await geocodeKeyless('Hanoi', { fetchImpl: rejecting }), null); + + const notOk = () => Promise.resolve({ ok: false, json: () => Promise.resolve({}) }); + assert.equal(await geocodeKeyless('somewhere else', { fetchImpl: notOk }), null); + + const empty = () => Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [] }) }); + assert.equal(await geocodeKeyless('third distinct query', { fetchImpl: empty }), null); +}); + +test('an empty query never reaches the network', async () => { + let calls = 0; + const counting = () => { calls += 1; return Promise.reject(new Error('should not run')); }; + + assert.equal(await geocodeKeyless(' ', { fetchImpl: counting }), null); + assert.equal(await geocodeKeyless(null, { fetchImpl: counting }), null); + assert.equal(calls, 0); +}); + +test('repeat queries are served from the memo, misses included', async () => { + let calls = 0; + const counting = () => { + calls += 1; + return Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [HANOI] }) }); + }; + + const first = await geocodeKeyless('Ha Noi memo probe', { fetchImpl: counting }); + const second = await geocodeKeyless('Ha Noi memo probe', { fetchImpl: counting }); + assert.equal(calls, 1, 'the second lookup must not hit the network'); + assert.deepEqual(second, first); + + const missing = () => { + calls += 1; + return Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [] }) }); + }; + await geocodeKeyless('typo memo probe', { fetchImpl: missing }); + await geocodeKeyless('typo memo probe', { fetchImpl: missing }); + assert.equal(calls, 2, 'a miss must be memoized too, or a typo costs one request per keystroke'); +}); + +test('a feature without usable coordinates is rejected', () => { + assert.equal(normalizePhotonFeature(undefined), null); + assert.equal(normalizePhotonFeature({ properties: { name: 'Nowhere' } }), null); + assert.equal(normalizePhotonFeature({ geometry: { coordinates: [200, 10] } }), null); + assert.equal(normalizePhotonFeature({ geometry: { coordinates: [10, 91] } }), null); +}); diff --git a/src/locations.js b/src/locations.js index 36a12263a..20e2cfbcf 100644 --- a/src/locations.js +++ b/src/locations.js @@ -1,5 +1,6 @@ import * as Cesium from 'cesium'; import { viewportBias, placesNearViewRecovery } from './annotations/annotationResolver.js'; +import { geocodeKeyless } from './keylessGeocoder.js'; /** * Points of Interest per city. @@ -348,7 +349,6 @@ export const CANCELLED_SEARCH = Object.freeze({ cancelled: true }); */ export async function searchAndFlyTo(viewer, query, options = {}) { const apiKey = window.__GOOGLE_MAPS_API_KEY__ || import.meta.env.GOOGLE_MAPS_API_KEY; - if (!apiKey) throw new Error('No Google Maps API key available for geocoding'); const beforeFly = typeof options.beforeFly === 'function' ? options.beforeFly : null; const mayFly = () => beforeFly === null || beforeFly() !== false; @@ -356,13 +356,17 @@ export async function searchAndFlyTo(viewer, query, options = {}) { // Viewport-biased geocode — the same bias annotationResolver's geocodePlace uses: // "Sixth Street" spoken over Austin must prefer the Sixth Street on screen, not a // same-named road in another city (or the wrong end of town — the W 6th vs E 6th bug). - let url = `https://maps.googleapis.com/maps/api/geocode/json?address=${encodeURIComponent(query)}&key=${apiKey}`; const bias = viewportBias(viewer); - if (bias) url += `&bounds=${bias}`; - const response = await fetch(url); - const data = await response.json(); + let result = null; + + if (apiKey) { + let url = `https://maps.googleapis.com/maps/api/geocode/json?address=${encodeURIComponent(query)}&key=${apiKey}`; + if (bias) url += `&bounds=${bias}`; + const response = await fetch(url); + const data = await response.json(); + result = (data.status === 'OK' && data.results?.length) ? data.results[0] : null; + } - const result = (data.status === 'OK' && data.results?.length) ? data.results[0] : null; let lat = result?.geometry.location.lat; let lng = result?.geometry.location.lng; let label = result ? result.formatted_address : null; @@ -372,7 +376,10 @@ export async function searchAndFlyTo(viewer, query, options = {}) { // Places-near-view recovery (annotationResolver's twin): a missed geocode, or one // that landed implausibly far from the view centre, snaps back to a view-biased // Places hit within the trust bound — "the Capitol" means the one on screen. - const recovered = await placesNearViewRecovery(viewer, query, result ? { lat, lon: lng } : null); + // Places is a Google service, so it is only worth asking when Google answered. + const recovered = apiKey + ? await placesNearViewRecovery(viewer, query, result ? { lat, lon: lng } : null) + : null; if (recovered) { lat = recovered.lat; lng = recovered.lon; @@ -380,7 +387,16 @@ export async function searchAndFlyTo(viewer, query, options = {}) { types = recovered.types || []; viewport = placesViewportToBounds(recovered.viewport) || viewport; } else if (!result) { - return null; + // Keyless fallback. Reached when no key is configured, and also when a key + // exists but Google declined — an unenabled Geocoding API answers HTTP 200 + // with REQUEST_DENIED, so the miss above is what detects it, not an error. + const keyless = await geocodeKeyless(query, { bias }); + if (!keyless) return null; + lat = keyless.lat; + lng = keyless.lng; + label = keyless.label || query; + types = keyless.types; + viewport = keyless.viewport; } const requestedRange = finitePositive(options.range); From acac1e4842d01878ef50082a3b6b73b84a5c0254 Mon Sep 17 00:00:00 2001 From: daikaginza Date: Sat, 29 Aug 2026 15:50:25 +0700 Subject: [PATCH 2/3] fix(search): keep proximity bias from choosing a different place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Photon scores proximity against the name, so a viewport bias could outrank an exact name match by distance alone. Searching "Hue" from Austin returned Hutto, Texas; "Ha Long" returned Long Branch. Measured against the live service over twelve Vietnamese place names, three resolved to the wrong continent. Google's `bounds` prefers a result inside the box without changing what qualifies as a result, and that is the semantics this adapter promised to reproduce. So a biased answer is now accepted only when a candidate leads with the name searched for, and the query otherwise re-runs unbiased — one extra request, on exactly the searches that were going to be wrong. Matching leads rather than contains, which is what separates "Hue" from "Nguyen Hue Road", and compares unaccented so "Hue" and "Huế" are one name. On the unbiased pass the head of a "place, region" query is matched first; the tail only says where to look. Nearby searches are unaffected: "Sixth Street" and "Barton Springs" over Austin still take the biased answer in a single request. --- src/keylessGeocoder.js | 99 +++++++++++++++++++++++++++++++----- src/keylessGeocoder.test.mjs | 93 ++++++++++++++++++++++++++++++++- 2 files changed, 177 insertions(+), 15 deletions(-) diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index 9de6faea9..8b8b24e4a 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -26,6 +26,12 @@ const PHOTON_TIMEOUT_MS = 6000; /** Bounded memo — a search box re-issues the same query on every keystroke. */ const PHOTON_CACHE_MAX = 64; + +/** + * Candidates requested per pass. One is not enough: proximity bias reorders the + * list, so the name actually asked for can sit below a nearer near-miss. + */ +const PHOTON_RESULT_LIMIT = 5; const photonCache = new Map(); /** @@ -187,13 +193,13 @@ export function normalizePhotonFeature(feature) { * and to England's over London. Soft bias is the semantics Google gives, so * soft bias is what the adapter must produce. * @param {string} query - Free-text place name. - * @param {{bias?: ?string}} [options] + * @param {{bias?: ?string, limit?: number}} [options] * @returns {string} */ -export function photonSearchUrl(query, { bias = null } = {}) { +export function photonSearchUrl(query, { bias = null, limit = PHOTON_RESULT_LIMIT } = {}) { const url = new URL(PHOTON_ENDPOINT); url.searchParams.set('q', String(query ?? '')); - url.searchParams.set('limit', '1'); + url.searchParams.set('limit', String(limit)); const corners = String(bias ?? '').split('|'); if (corners.length === 2) { @@ -207,6 +213,49 @@ export function photonSearchUrl(query, { bias = null } = {}) { return url.toString(); } +/** + * Place name reduced to comparable form: unaccented, lowercased, punctuation + * collapsed to single spaces. "Huế" and "Hue" have to compare equal, because a + * user typing either means the same city. + * @param {string} text + * @returns {string} + */ +export function normalizeToponym(text) { + return String(text ?? '') + .normalize('NFD') + .replace(/[\u0300-\u036f]/g, '') + .toLowerCase() + .replace(/[^a-z0-9]+/g, ' ') + .trim(); +} + +/** + * Pick the candidate whose name actually answers `target`. + * + * Matching leads rather than merely contains: "Hue" must select `Huế` and not + * `Nguyen Hue Road`, which contains the word but names a different thing. + * `allowContains` relaxes that to a substring match, used only as a second + * choice once no candidate leads with the name. + * @param {object[]} features - Photon features, in the order returned. + * @param {string} target - Normalized name being looked for. + * @param {{allowContains?: boolean}} [options] + * @returns {?object} + */ +export function selectPhotonFeature(features, target, { allowContains = false } = {}) { + if (!target) return null; + const names = (Array.isArray(features) ? features : []) + .map((feature) => [feature, normalizeToponym(feature?.properties?.name)]); + + for (const [feature, name] of names) { + if (name === target || name.startsWith(`${target} `)) return feature; + } + if (!allowContains) return null; + for (const [feature, name] of names) { + if (name.includes(target)) return feature; + } + return null; +} + /** Evict the oldest memo entries until the cache is within its cap. */ function trimPhotonCache() { while (photonCache.size > PHOTON_CACHE_MAX) { @@ -228,23 +277,45 @@ export async function geocodeKeyless(query, { bias = null, fetchImpl = fetch } = const trimmed = String(query ?? '').trim(); if (!trimmed) return null; - const url = photonSearchUrl(trimmed, { bias }); - if (photonCache.has(url)) return photonCache.get(url); + const memoKey = `${bias ?? ''}\n${trimmed}`; + if (photonCache.has(memoKey)) return photonCache.get(memoKey); - let result = null; - try { - const response = await fetchImpl(url, { signal: AbortSignal.timeout(PHOTON_TIMEOUT_MS) }); - if (response.ok) { - const body = await response.json(); - result = normalizePhotonFeature(body?.features?.[0]); + const ask = async (url) => { + try { + const response = await fetchImpl(url, { signal: AbortSignal.timeout(PHOTON_TIMEOUT_MS) }); + if (!response.ok) return null; + return (await response.json())?.features || []; + } catch { + return null; } - } catch { - result = null; + }; + + // Bias decides WHICH match wins, never WHAT counts as a match. Photon scores + // proximity against the name, so from Austin "Huế" came back as Hutto, Texas + // and "Hạ Long" as Long Branch — near misses beating the exact name by + // distance alone. Measured over twelve Vietnamese place names, three resolved + // to the wrong continent. So a biased answer is only accepted when it leads + // with the name asked for; otherwise the same query runs unbiased, which is + // what a user searching for somewhere off-screen meant in the first place. + let feature = null; + if (bias) { + feature = selectPhotonFeature(await ask(photonSearchUrl(trimmed, { bias })), normalizeToponym(trimmed)); } + if (!feature) { + const anywhere = await ask(photonSearchUrl(trimmed, { bias: null })); + // "Hoan Kiem Lake, Hanoi" names a place and then the region holding it, the + // convention every geocoder's free-text field follows. The head segment is + // the thing being searched for; the tail only says where to look. + const head = normalizeToponym(trimmed.split(',')[0]); + feature = selectPhotonFeature(anywhere, head, { allowContains: true }) || anywhere?.[0]; + } + + const result = normalizePhotonFeature(feature); + // Misses are cached too: a typo re-issued on every keystroke should cost one // request, not one per stroke. - photonCache.set(url, result); + photonCache.set(memoKey, result); trimPhotonCache(); return result; } diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index 922acd9d0..b7e0a0078 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -20,10 +20,12 @@ import assert from 'node:assert/strict'; import { geocodeKeyless, normalizePhotonFeature, + normalizeToponym, photonExtentToBounds, photonResultLabel, photonResultTypes, photonSearchUrl, + selectPhotonFeature, } from './keylessGeocoder.js'; import { geocodeNavigationMode } from './locations.js'; @@ -130,7 +132,9 @@ test('the Google bias becomes a SOFT proximity bias, never a hard bbox', () => { const url = new URL(photonSearchUrl('Sixth Street', { bias: '30.2000,-97.8000|30.3000,-97.7000' })); assert.equal(url.searchParams.get('q'), 'Sixth Street'); - assert.equal(url.searchParams.get('limit'), '1'); + // Five, not one: proximity reorders the list, so the name actually asked for + // can sit below a nearer near-miss and must still be reachable. + assert.equal(url.searchParams.get('limit'), '5'); assert.equal(url.searchParams.get('lat'), '30.25', 'centre of the biased rectangle'); assert.equal(url.searchParams.get('lon'), '-97.75'); @@ -197,3 +201,90 @@ test('a feature without usable coordinates is rejected', () => { assert.equal(normalizePhotonFeature({ geometry: { coordinates: [200, 10] } }), null); assert.equal(normalizePhotonFeature({ geometry: { coordinates: [10, 91] } }), null); }); + +// ── Bias may choose among matches; it may not change what counts as one ────── + +/** Photon's real answers for "Huế": Austin-biased, then unbiased. */ +const HUTTO = { + geometry: { type: 'Point', coordinates: [-97.6842, 30.5427] }, + properties: { osm_key: 'place', osm_value: 'town', type: 'city', name: 'Hutto', country: 'United States' }, +}; +const HUE = { + geometry: { type: 'Point', coordinates: [107.5908, 16.4674] }, + properties: { + osm_key: 'place', osm_value: 'city', type: 'city', name: 'Huế', country: 'Việt Nam', + extent: [107.3763, 16.7739, 108.0518, 16.1276], + }, +}; + +/** Answers biased requests with `nearby`, unbiased ones with `anywhere`. */ +function photonPair(nearby, anywhere) { + const urls = []; + const impl = (url) => { + urls.push(url); + const biased = new URL(url).searchParams.has('lat'); + return Promise.resolve({ ok: true, json: () => Promise.resolve({ features: biased ? nearby : anywhere }) }); + }; + impl.urls = urls; + return impl; +} + +test('a biased near-miss is refused, and the unbiased answer is taken instead', async () => { + // Measured from Austin against the live service: "Huế" scored Hutto, Texas + // above the city, and "Hạ Long" scored Long Branch. Distance alone must not + // outrank the name — three of twelve Vietnamese names crossed a continent. + const fetchImpl = photonPair([HUTTO], [HUE]); + + const result = await geocodeKeyless('Huế', { bias: '30.20,-97.80|30.30,-97.70', fetchImpl }); + + assert.equal(result.label.startsWith('Huế'), true, `got ${result.label}`); + assert.equal(Math.round(result.lat), 16); + assert.equal(fetchImpl.urls.length, 2, 'the refused bias costs exactly one extra request'); +}); + +test('a biased match that leads with the name is taken without a second request', async () => { + // The other half of the rule, and the reason bias exists at all: "Sixth + // Street" over Austin must stay Austin's. + const sixth = { + geometry: { type: 'Point', coordinates: [-97.7431, 30.2672] }, + properties: { osm_key: 'highway', osm_value: 'residential', type: 'street', name: 'Sixth Street', country: 'United States' }, + }; + const fetchImpl = photonPair([sixth], [HUE]); + + const result = await geocodeKeyless('Sixth Street', { bias: '30.20,-97.80|30.30,-97.70', fetchImpl }); + + assert.equal(result.label.startsWith('Sixth Street'), true); + assert.equal(fetchImpl.urls.length, 1); +}); + +test('a name is matched at its head, so a street that merely contains it loses', () => { + const road = { properties: { name: 'Nguyen Hue Road' } }; + const city = { properties: { name: 'Huế' } }; + + assert.equal(selectPhotonFeature([road, city], 'hue'), city); + // Relaxing to substring is a second choice only, never a first one. + assert.equal(selectPhotonFeature([road], 'hue'), null); + assert.equal(selectPhotonFeature([road], 'hue', { allowContains: true }), road); + assert.equal(selectPhotonFeature([city], ''), null); + assert.equal(selectPhotonFeature(undefined, 'hue'), null); +}); + +test('a "place, region" query is matched on the place, not the region', async () => { + const lake = { + geometry: { type: 'Point', coordinates: [105.8524, 21.0287] }, + properties: { osm_key: 'natural', osm_value: 'water', water: 'lake', name: 'Hoàn Kiếm Lake', country: 'Việt Nam' }, + }; + const street = { geometry: { type: 'Point', coordinates: [105.85, 21.03] }, properties: { name: 'Hoan Kiem District Road' } }; + const fetchImpl = photonPair([], [street, lake]); + + const result = await geocodeKeyless('Hoan Kiem Lake, Hanoi', { fetchImpl }); + + assert.equal(result.label.startsWith('Hoàn Kiếm Lake'), true, `got ${result.label}`); +}); + +test('accents and punctuation do not change what a name compares as', () => { + assert.equal(normalizeToponym('Huế'), 'hue'); + assert.equal(normalizeToponym('Hạ Long Bay'), 'ha long bay'); + assert.equal(normalizeToponym(" St. John's-Ravenscourt "), 'st john s ravenscourt'); + assert.equal(normalizeToponym(undefined), ''); +}); From e21c174f76f1d4772449c739e2a12728cacb5eaa Mon Sep 17 00:00:00 2001 From: daikaginza Date: Sat, 5 Sep 2026 09:45:55 +0700 Subject: [PATCH 3/3] feat(search): finish the keyless fallback at every geocode call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keyless fallback landed on the LOCATION search box only. Two other places forward-geocode a name and neither had it: the annotation resolver returned null, so an annotation silently failed to anchor, and the Radio layer THREW, which surfaced as a failed voice turn rather than as a station it could not place. src/keylessGeocoder.test.mjs now COUNTS the call sites instead of checking them file by file, so a second seam added to a file that already has a fallback fails the suite too. The annotation path carries the feature's own name out of Photon. Footprint scoring matches OSM on that canonical name rather than on the user's words, so incidental locality tokens cannot win: "the monument near the Texas Capitol, Austin" scored on the raw utterance picks a downtown building that shares only "Texas", "Capitol" and "Austin". Photon's country is deliberately NOT re-exported. It comes back in the feature's own language — "Việt Nam", "日本", "Россия" — and normalizeRadioCountryInput fails CLOSED on a name it cannot map, so forwarding it would filter the station list to nothing for exactly the places the lookup had just resolved. The composed label still names the country. A keyless lookup must not apply a filter the keyed one never applies. A not-found is now remembered only when every source consulted returned a verdict. Photon not replying is a network blip, not "no such place": memoizing it kept the query answering not-found for the rest of the session on a network that had since recovered — the same conflation the Overpass disk cache had to be repaired for. geocodeMissIsDefinitive states the rule once and both entry points use it, and the adapter's own memo follows it as well. Reverse geocoding stays out of scope: the street-label reader consumes Google's ranked address_components, and Photon's /reverse cannot reproduce that shape. --- CHANGELOG.md | 13 ++ docs/CURRENT-STATE.md | 6 +- src/annotations/annotationResolver.js | 78 ++++++++- src/annotations/annotationResolver.test.mjs | 184 +++++++++++++++++++ src/keylessGeocoder.js | 97 +++++++--- src/keylessGeocoder.test.mjs | 185 ++++++++++++++++++++ src/voice/gevActions.js | 43 +++-- src/voice/gevActions.test.mjs | 118 +++++++++++++ 8 files changed, 681 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40c545a2a..231ab050e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,12 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md - Existing cached refusals are now ignored immediately, including during stale-data fallback. Concurrent identical requests share the same last-good fallback when all mirrors refuse, without duplicating upstream requests. +- A keyless place lookup no longer remembers a network failure as "no such + place". A blip while Photon was answering used to be memoized for the rest of + the session, so the query kept returning not-found from memory on a network + that had since recovered. A miss is now cached only when every source + consulted actually returned a verdict. + ### Added - Keyless place search. The LOCATION search box and the `fly_to_location` voice @@ -25,6 +31,13 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md the primary path and is unchanged when it answers; the fallback also covers a key whose Geocoding API is not enabled, which Google reports as HTTP 200 with `REQUEST_DENIED`, so an empty result is the detector rather than an error. +- The same keyless fallback now covers the remaining two place lookups: map + annotations ("annotate the botanical garden") and the Radio layer's + "near \" selection. Radio previously threw without a key, which + surfaced as a failed voice turn rather than as a station it could not place; + annotations silently failed to anchor. Annotation footprints match OSM on the + resolved feature's canonical name, so locality words in the request cannot + pull the outline onto a neighbouring building. ## [0.1.1] — 2026-09-01 — Installation and live-data fixes diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 00003a260..6fcd02b0c 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2183,7 +2183,11 @@ silently demoting every later lookup for the session. ### Keyless place search (August 2026) -- `src/keylessGeocoder.js` is a Photon (komoot/OpenStreetMap) adapter used by `searchAndFlyTo` when the Google geocode produces no result: no key configured, or a key whose Geocoding API is not enabled — Google answers HTTP 200 with `REQUEST_DENIED`, so the empty result is the detector, not an error. Google stays the primary path and is byte-unchanged when it answers, and the Places near-view recovery is only attempted when a key exists, since Places is a Google service. +- `src/keylessGeocoder.js` is a Photon (komoot/OpenStreetMap) adapter used when the Google geocode produces no result: no key configured, or a key whose Geocoding API is not enabled — Google answers HTTP 200 with `REQUEST_DENIED`, so the empty result is the detector, not an error. Google stays the primary path and is byte-unchanged when it answers, and the Places near-view recovery is only attempted when a key exists, since Places is a Google service. +- The fallback covers **all three** client call sites that forward-geocode a name, not just the search box: `searchAndFlyTo` (`src/locations.js`), the annotation resolver's `geocodePlace` (`src/annotations/annotationResolver.js`) and the Radio layer's "near \" lookup (`resolveRadioLocation`, `src/voice/gevActions.js`). The last of those used to THROW without a key, which surfaced as a failed voice turn rather than as a station it could not place. `src/keylessGeocoder.test.mjs` pins the inventory by counting the call sites, so a second seam added to a file that already has a fallback fails the suite too. Reverse geocoding (`?latlng=`) is deliberately NOT covered: the street-label reader consumes Google's ranked `address_components`, and Photon's `/reverse` cannot reproduce that shape. +- For the annotation resolver the adapter also carries the feature's own `name`. OSM footprint scoring matches on that canonical name rather than on the user's words, so incidental locality tokens cannot win — "the monument near the Texas Capitol, Austin" scored on the raw utterance picks a downtown building that merely shares "Texas", "Capitol" and "Austin" (the same failure as field test 7). +- Photon's `country` is NOT re-exported as a field. It comes back in the feature's own language ("Việt Nam", "日本", "Россия"), and `normalizeRadioCountryInput` fails CLOSED on a name it cannot map — forwarding it would filter the station list to nothing for exactly the places the lookup just resolved. The composed label still names the country. +- A not-found is remembered only when every source consulted returned a verdict (`geocodeMissIsDefinitive`). Photon not replying is a network blip, not "no such place": caching it would keep an annotation unplaceable for the rest of the session on a network that has since recovered — the same conflation the Overpass disk cache had to be repaired for. The adapter's own memo follows the same rule, so an unanswered lookup is retried rather than replayed. - The adapter normalizes into the shape the Google path already produces — `lat`/`lng`, label, Google-style `types`, `{southwest,northeast}` bounds — so `geocodeNavigationMode`, `regionFramingPlan` and the region-swath cap keep running unchanged. Two translations are load-bearing and are pinned by tests: Photon's `extent` is `[west, north, east, south]`, and a specific OSM `key=value` rule beats the coarse `type` (Photon reports a lake as `water=lake` with `type: 'other'`, which framed at building range put the camera 26 m over the water; it reports a town square as `type: 'locality'`, which framed a plaza as a city). - The shared `viewportBias()` string is sent as Photon's `lat`/`lon` PROXIMITY bias, never as `bbox`: Google's `bounds` prefers, Photon's `bbox` filters, and the literal translation makes every off-screen search return nothing. One result per query, memoized by query+bias (misses included) to respect komoot's fair use. diff --git a/src/annotations/annotationResolver.js b/src/annotations/annotationResolver.js index 3bcc143c2..dce8fc2e0 100644 --- a/src/annotations/annotationResolver.js +++ b/src/annotations/annotationResolver.js @@ -2,6 +2,7 @@ import * as Cesium from 'cesium'; import { lookupNeighborhoodRing } from '../data/neighborhoodPolygons.js'; import { lookupNaturalRegionOutline, findNaturalRegion } from '../data/naturalEarthRegions.js'; import { registerDynamicCredit, NATURAL_EARTH_CREDIT } from '../data/dataCredits.js'; +import { geocodeKeylessWithOutcome } from '../keylessGeocoder.js'; import { isPickedWorldPosition } from '../data/scenePick.js'; /** @@ -599,17 +600,19 @@ function ringAreaM2(ring) { } /** - * Forward-geocode a place name via Google Geocoding, biased to the current - * viewport so "the marina" resolves near where the user is looking. + * Forward-geocode a place name, biased to the current viewport so "the marina" + * resolves near where the user is looking. Google first; Photon when there is + * no key, or when Google returns nothing. */ async function geocodePlace(query, biasRect, signal) { const apiKey = window.__GOOGLE_MAPS_API_KEY__ || import.meta.env.GOOGLE_MAPS_API_KEY; - if (!apiKey) return null; const cacheKey = `${query.toLowerCase()}|${biasRect || ''}`; const cached = cacheRead(geocodeCache, cacheKey); if (cached !== undefined) return cached; + if (!apiKey) return keylessPlace(query, biasRect, cacheKey, signal, { googleConsulted: false }); + let url = `https://maps.googleapis.com/maps/api/geocode/json?address=${encodeURIComponent(query)}&key=${apiKey}`; if (biasRect) url += `&bounds=${biasRect}`; @@ -617,10 +620,13 @@ async function geocodePlace(query, biasRect, signal) { const response = await fetch(url, { signal }); const data = await response.json(); if (data.status !== 'OK' || !data.results?.length) { - // ZERO_RESULTS is a definitive not-found (cacheable); OVER_QUERY_LIMIT / - // REQUEST_DENIED / UNKNOWN_ERROR are transient → don't poison the cache. - negCache(geocodeCache, cacheKey, signal, data?.status === 'ZERO_RESULTS'); - return null; + // Google found nothing, or declined. An unenabled Geocoding API answers + // HTTP 200 with REQUEST_DENIED, so this branch is also how a + // half-configured key looks — either way the keyless path is the answer. + return keylessPlace(query, biasRect, cacheKey, signal, { + googleConsulted: true, + googleSaysNoSuchPlace: data?.status === 'ZERO_RESULTS', + }); } const result = data.results[0]; const place = { @@ -644,6 +650,64 @@ async function geocodePlace(query, biasRect, signal) { } } +/** + * Whether a combined Google+Photon not-found may be REMEMBERED as "no such + * place", or must be left open for the next attempt. + * + * One rule for both entry points: a miss is definitive only when EVERY source + * that was consulted returned a verdict. With no key Photon is the only source, + * so its answer decides. With a key, Google declining (REQUEST_DENIED on an + * unenabled API, OVER_QUERY_LIMIT) or Photon being unreachable each leave the + * question open — and Photon holds plenty of places Google does not, which is + * the reason this fallback exists at all. Caching either would keep an + * annotation unplaceable for the rest of the session on a network that has + * since recovered. + * + * Exported for the unit tests: the no-key branch cannot be driven from + * `node --test`, because the key expression reads `import.meta.env`. + * @param {{photonAnswered:boolean, googleConsulted:boolean, + * googleSaysNoSuchPlace?:boolean}} sources + * @returns {boolean} + */ +export function geocodeMissIsDefinitive({ photonAnswered, googleConsulted, googleSaysNoSuchPlace = false }) { + if (!photonAnswered) return false; + return !googleConsulted || Boolean(googleSaysNoSuchPlace); +} + +/** + * Photon fallback for `geocodePlace`, in the shape the annotation pipeline + * already consumes. + * + * `primaryName` is the feature's own name rather than a name extracted from a + * formatted address, which is what the OSM name-matching downstream wants — + * Photon reports it directly, so nothing has to be parsed back out of a label. + * @param {string} query Free-text place name. + * @param {?string} biasRect `"swLat,swLng|neLat,neLng"` viewport bias. + * @param {string} cacheKey Shared cache key, so both paths memoise together. + * @param {?AbortSignal} signal Caller's abort signal. + * @param {{googleConsulted:boolean, googleSaysNoSuchPlace?:boolean}} google What + * the Google leg contributed: whether it ran at all, and whether it answered + * ZERO_RESULTS rather than declining. + * @returns {Promise} Resolved place, or null. + */ +async function keylessPlace(query, biasRect, cacheKey, signal, google) { + const { place: keyless, answered } = await geocodeKeylessWithOutcome(query, { bias: biasRect || null }); + if (!keyless) { + negCache(geocodeCache, cacheKey, signal, geocodeMissIsDefinitive({ ...google, photonAnswered: answered })); + return null; + } + const place = { + lat: keyless.lat, + lon: keyless.lng, + label: shortLabel(keyless.label), + primaryName: keyless.name || null, + types: keyless.types, + viewport: normalizeGeocodeViewport(keyless.viewport), + }; + cacheWrite(geocodeCache, cacheKey, place); + return place; +} + /** Geocoding returns {southwest:{lat,lng},northeast:{lat,lng}}; normalize to the Places * {low,high} lat/lng shape the rest of the pipeline (disc sizing, framing) consumes. */ function normalizeGeocodeViewport(vp) { diff --git a/src/annotations/annotationResolver.test.mjs b/src/annotations/annotationResolver.test.mjs index 1ab378683..0741b56b8 100644 --- a/src/annotations/annotationResolver.test.mjs +++ b/src/annotations/annotationResolver.test.mjs @@ -12,6 +12,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { + geocodeMissIsDefinitive, resolveAnnotationTarget, selectFootprint, refineScope, @@ -362,3 +363,186 @@ test('ask-side admin bypass: admin level 2/3 result types never grant a township 'both township-level admin result types stay guarded', ); }); + +// ── Keyless geocoding ──────────────────────────────────────────────────────── +// +// `geocodePlace` was Google-only: no key returned null, and so did a key whose +// Geocoding API is not enabled (Google answers HTTP 200 with REQUEST_DENIED). +// Either way the annotation silently failed to place. Both now fall through to +// Photon. These drive the second case, because it reaches the SAME fallback +// through a running Google branch — the no-key branch cannot be driven from +// `node --test`, since the key expression reads `import.meta.env`, which only +// Vite defines. + +/** Photon's GeoJSON shape, trimmed to the properties the adapter consumes. */ +function photonFeature({ name, lat, lon, tags = {}, extent = null, ...rest }) { + return { + geometry: { type: 'Point', coordinates: [lon, lat] }, + properties: { name, ...tags, ...rest, ...(extent ? { extent } : {}) }, + }; +} + +const GOOGLE_FOUND_NOTHING = { ok: true, json: async () => ({ status: 'ZERO_RESULTS', results: [] }) }; + +test('keyless: a key that geocodes to nothing still anchors the annotation', async (t) => { + const requests = []; + installGoogleMocks(t, async (url) => { + requests.push(String(url)); + if (String(url).startsWith('https://maps.googleapis.com/')) return GOOGLE_FOUND_NOTHING; + return { + ok: true, + json: async () => ({ + features: [photonFeature({ + name: 'Lady Bird Lake', + lat: 30.25, + lon: -97.73, + city: 'Austin', + state: 'Texas', + country: 'United States', + tags: { osm_key: 'natural', osm_value: 'water' }, + // Photon orders extent [west, north, east, south] — a naive [w,s,e,n] + // read would invert the box and still look like a valid viewport. + extent: [-97.8, 30.28, -97.68, 30.24], + })], + }), + }; + }); + + const resolved = await resolveAnnotationTarget({ + viewer: closeViewportViewer(), + target: 'Lady Bird Lake', + }); + + assert.ok(resolved, 'a Google miss must not leave the annotation unplaced'); + assert.equal(resolved.source, 'geocode'); + assert.deepEqual([resolved.lat, resolved.lon], [30.25, -97.73]); + // The label is shortened the same way a Google `formatted_address` is. + assert.equal(resolved.label, 'Lady Bird Lake'); + // Photon's extent reaches the framing code in the Places `{low,high}` shape, + // right way up — this is what sizes a grounds disc and the flyTo box. + assert.deepEqual(resolved.viewport, { + low: { latitude: 30.24, longitude: -97.8 }, + high: { latitude: 30.28, longitude: -97.68 }, + }); + // Google is asked first and exactly once; one Photon call answers it. + assert.equal(requests.filter((url) => url.includes('maps.googleapis.com')).length, 1); + assert.equal(requests.filter((url) => url.includes('photon.komoot.io')).length, 1); +}); + +test('keyless: OSM matching uses the feature\'s canonical name, not the user\'s words', async (t) => { + // The reason `primaryName` is carried out of Photon at all. The ask names a + // landmark AND its surroundings; the geocoded feature is called just "Tejano + // Monument". Score the OSM candidates on the raw utterance and the locality + // tokens win — this is the Thompson-Austin bug (field test 7) reached through + // the keyless path. Score them on the canonical name and the monument wins. + const decoy = squareWay(ANCHOR, -250, -250, 3000, { + building: 'yes', tourism: 'hotel', name: 'Texas Capitol Austin Visitor Center', + }); + const monument = squareWay(ANCHOR, 5, 5, 300, { tourism: 'artwork', name: 'Tejano Monument' }); + + installGoogleMocks(t, async (url, init) => { + const href = String(url); + if (href.startsWith('https://maps.googleapis.com/')) return GOOGLE_FOUND_NOTHING; + if (href.startsWith('/api/google/text-search')) return { ok: true, json: async () => ({ places: [] }) }; + if (href.startsWith('https://photon.komoot.io/')) { + return { + ok: true, + json: async () => ({ + features: [photonFeature({ + name: 'Tejano Monument', + lat: ANCHOR.lat, + lon: ANCHOR.lon, + city: 'Austin', + tags: { osm_key: 'historic', osm_value: 'memorial' }, + })], + }), + }; + } + assert.equal(href, '/api/overpass'); + assert.equal(init?.method, 'POST'); + return { ok: true, status: 200, headers: { get: () => null }, json: async () => ({ elements: [decoy, monument] }) }; + }); + + const resolved = await resolveAnnotationTarget({ + viewer: closeViewportViewer(), + target: 'the monument near the Texas Capitol, Austin', + footprint: true, + }); + + assert.ok(resolved.ring, 'the monument polygon is monument-scale and exactly named — it must outline'); + // The anchor re-centres on the chosen polygon, so its position IS the verdict: + // the monument sits on the anchor, the decoy 354 m south-west of it. + assert.ok( + approximateMetres(resolved.lat, resolved.lon, ANCHOR.lat, ANCHOR.lon) < 50, + `outlined a polygon ${Math.round(approximateMetres(resolved.lat, resolved.lon, ANCHOR.lat, ANCHOR.lon))} m from the monument`, + ); +}); + +/** Metres between two coordinates — enough precision for a few hundred metres. */ +function approximateMetres(lat1, lon1, lat2, lon2) { + const mLat = 111320; + const dy = (lat1 - lat2) * mLat; + const dx = (lon1 - lon2) * mLat * Math.cos((lat1 * Math.PI) / 180); + return Math.hypot(dx, dy); +} + +test('keyless: a Photon outage is not remembered as "no such place"', async (t) => { + // Google answering ZERO_RESULTS is a verdict about GOOGLE. Photon holds + // plenty of places Google does not — that asymmetry is why this fallback + // exists — so a Google verdict plus a Photon timeout must leave the query + // open. Caching it would keep an annotation unplaceable for the rest of the + // session, on a network that has since recovered. + const photonUp = { + ok: true, + json: async () => ({ + features: [photonFeature({ + name: 'Waller Creek', lat: 30.2665, lon: -97.7385, city: 'Austin', + tags: { osm_key: 'waterway', osm_value: 'stream' }, + })], + }), + }; + let photonReachable = false; + const requests = []; + installGoogleMocks(t, async (url) => { + requests.push(String(url)); + if (String(url).startsWith('https://maps.googleapis.com/')) return GOOGLE_FOUND_NOTHING; + if (!photonReachable) throw new Error('network down'); + return photonUp; + }); + + const target = 'Waller Creek'; + assert.equal(await resolveAnnotationTarget({ viewer: closeViewportViewer(), target }), null); + const duringOutage = requests.length; + + photonReachable = true; + const retried = await resolveAnnotationTarget({ viewer: closeViewportViewer(), target }); + + assert.ok(retried, 'the same target must resolve once the network is back'); + assert.deepEqual([retried.lat, retried.lon], [30.2665, -97.7385]); + assert.ok( + requests.length > duringOutage, + 'the retry was served from a poisoned cache instead of asking again', + ); +}); + +test('a not-found is remembered only when every source consulted gave a verdict', () => { + // The full truth table, because the no-key row is the one the behavioural + // tests above cannot reach: `import.meta.env` only exists under Vite, so + // `node --test` can never take the branch that skips Google entirely. + const cases = [ + // no key: Photon is the only source, so Photon alone decides + [{ googleConsulted: false, photonAnswered: true }, true], + [{ googleConsulted: false, photonAnswered: false }, false], + // with a key: Google must have said "no such place", AND Photon must have replied + [{ googleConsulted: true, googleSaysNoSuchPlace: true, photonAnswered: true }, true], + [{ googleConsulted: true, googleSaysNoSuchPlace: true, photonAnswered: false }, false], + // Google declined (REQUEST_DENIED / OVER_QUERY_LIMIT) — never a verdict + [{ googleConsulted: true, googleSaysNoSuchPlace: false, photonAnswered: true }, false], + [{ googleConsulted: true, googleSaysNoSuchPlace: false, photonAnswered: false }, false], + ]; + for (const [sources, expected] of cases) { + assert.equal(geocodeMissIsDefinitive(sources), expected, JSON.stringify(sources)); + } + // An omitted Google verdict defaults to "did not say no such place". + assert.equal(geocodeMissIsDefinitive({ googleConsulted: true, photonAnswered: true }), false); +}); diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index 8b8b24e4a..8f2f89413 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -157,10 +157,21 @@ export function photonResultLabel(properties) { } /** - * Photon GeoJSON feature → the normalized geocode result `searchAndFlyTo` - * consumes. Returns null for a feature without usable coordinates. + * Photon GeoJSON feature → the normalized geocode result its callers consume. + * Returns null for a feature without usable coordinates. + * + * `name` is carried alongside the composed `label` because two call sites want + * different halves of the same answer: the search box shows the full label, + * while the annotation resolver matches OSM features on the canonical name + * alone. Deriving it here keeps one shape rather than two parsers of the same + * string. + * + * `properties.country` is deliberately NOT re-exported. Photon answers in the + * feature's own language ("Việt Nam", "日本", "Россия"), which no consumer in + * this app can match; the composed label is the only honest use for it. * @param {object} feature - One entry of Photon's `features` array. - * @returns {?{lat:number, lng:number, label:string, types:string[], viewport:?object}} + * @returns {?{lat:number, lng:number, name:string, label:string, + * types:string[], viewport:?object}} */ export function normalizePhotonFeature(feature) { const [lng, lat] = feature?.geometry?.coordinates || []; @@ -168,10 +179,12 @@ export function normalizePhotonFeature(feature) { if (Math.abs(lat) > 90 || Math.abs(lng) > 180) return null; const properties = feature.properties || {}; + const name = String(properties.name || ''); return { lat, lng, - label: photonResultLabel(properties) || String(properties.name || ''), + name, + label: photonResultLabel(properties) || name, types: photonResultTypes(properties), viewport: photonExtentToBounds(properties.extent), }; @@ -266,20 +279,33 @@ function trimPhotonCache() { } /** - * Geocode a place name without any API key. Never throws: a network failure, - * a timeout, or an empty result all resolve to null, which the caller reads as - * "not found" exactly as it reads a Google miss. + * Geocode a place name without any API key, reporting whether the service + * ANSWERED as well as what it found. + * + * A caller that remembers negative results needs both halves. "Photon has no + * such place" is a verdict and may be cached; "Photon did not reply" is a + * network blip, and caching THAT keeps the app answering "not found" from + * memory long after the network came back — a whole session, for a key that + * would now resolve. Conflating the two is the defect this app already had to + * repair once in the Overpass disk cache, where refusals were persisted with + * normal data TTLs. + * + * Never throws: a failure, a timeout, and an empty result all resolve to a null + * `place`, which the caller reads exactly as it reads a Google miss. * @param {string} query - Free-text place name. * @param {{bias?: ?string, fetchImpl?: Function}} [options] - * @returns {Promise} + * @returns {Promise<{place: ?{lat:number, lng:number, name:string, label:string, + * types:string[], viewport:?object}, answered:boolean}>} */ -export async function geocodeKeyless(query, { bias = null, fetchImpl = fetch } = {}) { +export async function geocodeKeylessWithOutcome(query, { bias = null, fetchImpl = fetch } = {}) { const trimmed = String(query ?? '').trim(); - if (!trimmed) return null; + if (!trimmed) return { place: null, answered: true }; const memoKey = `${bias ?? ''}\n${trimmed}`; - if (photonCache.has(memoKey)) return photonCache.get(memoKey); + // Only answered outcomes are memoised, so a hit is always an answer. + if (photonCache.has(memoKey)) return { place: photonCache.get(memoKey), answered: true }; + /** One request: the feature array Photon returned, or null if it did not answer. */ const ask = async (url) => { try { const response = await fetchImpl(url, { signal: AbortSignal.timeout(PHOTON_TIMEOUT_MS) }); @@ -298,24 +324,49 @@ export async function geocodeKeyless(query, { bias = null, fetchImpl = fetch } = // with the name asked for; otherwise the same query runs unbiased, which is // what a user searching for somewhere off-screen meant in the first place. let feature = null; + let everyAskAnswered = true; if (bias) { - feature = selectPhotonFeature(await ask(photonSearchUrl(trimmed, { bias })), normalizeToponym(trimmed)); + const biased = await ask(photonSearchUrl(trimmed, { bias })); + if (biased === null) everyAskAnswered = false; + else feature = selectPhotonFeature(biased, normalizeToponym(trimmed)); } if (!feature) { const anywhere = await ask(photonSearchUrl(trimmed, { bias: null })); - // "Hoan Kiem Lake, Hanoi" names a place and then the region holding it, the - // convention every geocoder's free-text field follows. The head segment is - // the thing being searched for; the tail only says where to look. - const head = normalizeToponym(trimmed.split(',')[0]); - feature = selectPhotonFeature(anywhere, head, { allowContains: true }) || anywhere?.[0]; + if (anywhere === null) everyAskAnswered = false; + else { + // "Hoan Kiem Lake, Hanoi" names a place and then the region holding it, the + // convention every geocoder's free-text field follows. The head segment is + // the thing being searched for; the tail only says where to look. + const head = normalizeToponym(trimmed.split(',')[0]); + feature = selectPhotonFeature(anywhere, head, { allowContains: true }) || anywhere[0]; + } } - const result = normalizePhotonFeature(feature); + const place = normalizePhotonFeature(feature); + // A found place is self-evidently an answer; a null one only counts as an + // answer when every request it rests on actually came back. + const answered = Boolean(place) || everyAskAnswered; - // Misses are cached too: a typo re-issued on every keystroke should cost one - // request, not one per stroke. - photonCache.set(memoKey, result); - trimPhotonCache(); - return result; + // Answered misses are cached too: a typo re-issued on every keystroke should + // cost one request, not one per stroke. Unanswered ones are not, so the next + // attempt retries instead of replaying a network blip. + if (answered) { + photonCache.set(memoKey, place); + trimPhotonCache(); + } + return { place, answered }; +} + +/** + * Geocode a place name without any API key. Never throws: a network failure, a + * timeout, or an empty result all resolve to null, which the caller reads as + * "not found" exactly as it reads a Google miss. + * @param {string} query - Free-text place name. + * @param {{bias?: ?string, fetchImpl?: Function}} [options] + * @returns {Promise} + */ +export async function geocodeKeyless(query, options) { + return (await geocodeKeylessWithOutcome(query, options)).place; } diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index b7e0a0078..74f0a06bb 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -17,8 +17,12 @@ // Run with: npm test import { test } from 'node:test'; import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import { geocodeKeyless, + geocodeKeylessWithOutcome, normalizePhotonFeature, normalizeToponym, photonExtentToBounds, @@ -29,6 +33,8 @@ import { } from './keylessGeocoder.js'; import { geocodeNavigationMode } from './locations.js'; +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + /** Photon's real answer for "Hanoi", trimmed to the fields we consume. */ const HANOI = { geometry: { type: 'Point', coordinates: [105.854041, 21.0283334] }, @@ -288,3 +294,182 @@ test('accents and punctuation do not change what a name compares as', () => { assert.equal(normalizeToponym(" St. John's-Ravenscourt "), 'st john s ravenscourt'); assert.equal(normalizeToponym(undefined), ''); }); + +// ── The seam inventory ─────────────────────────────────────────────────────── +// +// A fallback is only worth having where the app actually geocodes, and the app +// geocodes in more than one place: the LOCATION search box, the Radio layer's +// "near " lookup, and the annotation resolver. Fixing one and leaving +// the others is the failure this pins — the first version of this work fixed +// the search box alone, so a keyless install still threw on "play radio near +// Hanoi" while the search box beside it worked. +// +// It reads the source rather than the behaviour on purpose. The defect is not +// that a particular function is wrong; it is that a NEW call site can be added +// without one, and no behavioural test of the existing three would notice. + +/** Client modules that forward-geocode a place name through Google. */ +const GOOGLE_FORWARD_GEOCODE = /maps\.googleapis\.com\/maps\/api\/geocode\/json\?address/; +const CLIENT_SOURCES = fs.readdirSync(path.join(ROOT, 'src'), { recursive: true }) + .filter((entry) => typeof entry === 'string' && entry.endsWith('.js')) + .map((entry) => ({ file: path.join('src', entry), body: fs.readFileSync(path.join(ROOT, 'src', entry), 'utf8') })); + +/** + * Every forward-geocode call site in the client, by file. Reverse geocoding + * (`?latlng=`) is deliberately absent: Google returns ranked + * `address_components` that the street-label reader consumes, and Photon's + * `/reverse` cannot reproduce that shape — an honest gap, not an oversight. + */ +const FORWARD_GEOCODE_SEAMS = Object.freeze({ + 'src/locations.js': 1, + 'src/annotations/annotationResolver.js': 1, + 'src/voice/gevActions.js': 1, +}); + +test('the seam inventory is exactly the call sites we know about', () => { + // Counted, not merely listed. A file-level check would pass a SECOND seam + // added to a file that already has a fallback — which is how the first + // version of this work shipped: the search box had one, and the two call + // sites beside it did not. + assert.ok(CLIENT_SOURCES.length > 50, `only ${CLIENT_SOURCES.length} client sources scanned`); + const found = {}; + for (const { file, body } of CLIENT_SOURCES) { + const count = (body.match(new RegExp(GOOGLE_FORWARD_GEOCODE, 'g')) || []).length; + if (count) found[file] = count; + } + // A new seam is not a failure — it is a prompt to give it the same fallback + // and record it here. + assert.deepEqual(found, { ...FORWARD_GEOCODE_SEAMS }); +}); + +test('every place that forward-geocodes through Google can also answer without a key', () => { + for (const { file, body } of CLIENT_SOURCES) { + if (!GOOGLE_FORWARD_GEOCODE.test(body)) continue; + assert.match(body, /geocodeKeyless/, `${file} geocodes through Google with no keyless fallback`); + } +}); + +test('a missing key is never a thrown error on the client', () => { + // It was one, in the Radio layer: a keyless install surfaced "play radio near + // Hanoi" as a failed voice turn rather than as a station it could not place. + for (const { file, body } of CLIENT_SOURCES) { + assert.doesNotMatch(body, /throw new Error\([^)]*No Google Maps API key/, `${file} throws on a missing key`); + } +}); + +// ── The shape the three call sites share ───────────────────────────────────── + +test('the normalized result carries the canonical name its callers need', () => { + // Two consumers want different halves of one answer: the search box shows + // `label`, while the annotation resolver matches OSM features on `name` + // alone — a canonical name beats a parsed label there, because "Hà Nội, + // Việt Nam" would let the country token win the footprint scoring. + const result = normalizePhotonFeature(HANOI); + + assert.equal(result.name, 'Hà Nội'); + assert.equal(result.label.startsWith('Hà Nội'), true); + assert.equal(result.label.includes(result.name), true, 'the label is built from the name, not beside it'); +}); + +test('a feature with no name yields an empty string, never undefined', () => { + const bare = { geometry: { coordinates: [105.85, 21.03] }, properties: { osm_key: 'place', osm_value: 'city' } }; + const result = normalizePhotonFeature(bare); + + assert.equal(result.name, ''); + assert.equal(typeof result.label, 'string'); +}); + +test("Photon's own country name stays inside the label — it is not a field", () => { + // Photon answers in the feature's LANGUAGE: this fixture is the live service's + // real reply for "Hanoi", and it says "Việt Nam", not "Vietnam". Tokyo answers + // "日本", Moscow "Россия". Re-exporting that as a `country` field invites a + // consumer to match on it, and the one consumer that would — the Radio + // layer's `normalizeRadioCountryInput` — fails CLOSED on a name it cannot + // map, turning "radio near Ha Long" into an empty station list. The composed + // label is the only honest use for a localized country name. + assert.equal(HANOI.properties.country, 'Việt Nam'); + const result = normalizePhotonFeature(HANOI); + + assert.equal('country' in result, false, 'a localized country name must not become a matchable field'); + assert.match(result.label, /Việt Nam$/); +}); + +// ── An answer and a silence are not the same miss ──────────────────────────── +// +// Both arrive as `null`, and remembering the wrong one is the defect this app +// has already had to repair once: the Overpass disk cache persisted refusals +// with normal data TTLs, so an install that hit a bad minute kept serving the +// refusal for a month. A geocoder that memoises a network blip does the same +// thing for a session — every later keystroke replays it without asking. + +test('an unanswered request is never memoized, so the next attempt really retries', async () => { + let attempts = 0; + const offline = () => { attempts += 1; return Promise.reject(new Error('offline')); }; + const online = () => { + attempts += 1; + return Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [HANOI] }) }); + }; + + assert.equal(await geocodeKeyless('blip probe', { fetchImpl: offline }), null); + assert.equal(attempts, 1); + + const recovered = await geocodeKeyless('blip probe', { fetchImpl: online }); + assert.equal(attempts, 2, 'the retry must reach the network, not a memoized blip'); + assert.equal(recovered.name, 'Hà Nội'); +}); + +test('an HTTP refusal counts as silence, not as "no such place"', async () => { + let attempts = 0; + const refusing = () => { + attempts += 1; + return Promise.resolve({ ok: false, status: 429, json: () => Promise.resolve({}) }); + }; + + await geocodeKeyless('throttle probe', { fetchImpl: refusing }); + await geocodeKeyless('throttle probe', { fetchImpl: refusing }); + assert.equal(attempts, 2, 'a 429 must not be remembered as a verdict'); +}); + +test('the outcome reports which of the two a null result was', async () => { + const answeredMiss = () => Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [] }) }); + const failure = () => Promise.reject(new Error('offline')); + const hit = () => Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [HANOI] }) }); + + assert.deepEqual( + await geocodeKeylessWithOutcome('outcome probe a', { fetchImpl: answeredMiss }), + { place: null, answered: true }, + ); + assert.deepEqual( + await geocodeKeylessWithOutcome('outcome probe b', { fetchImpl: failure }), + { place: null, answered: false }, + ); + + const found = await geocodeKeylessWithOutcome('outcome probe c', { fetchImpl: hit }); + assert.equal(found.answered, true); + assert.equal(found.place.name, 'Hà Nội'); + + // A memo hit is by definition an answer — only answers are stored. + assert.deepEqual( + await geocodeKeylessWithOutcome('outcome probe a', { fetchImpl: failure }), + { place: null, answered: true }, + ); +}); + +test('a biased pass that never answered leaves the whole lookup unanswered', async () => { + // The unbiased retry can report an honest empty while the biased request that + // preceded it timed out. The lookup as a whole still rests on a request that + // never came back, so its null is not a verdict. + const urls = []; + const halfDown = (url) => { + urls.push(url); + return new URL(url).searchParams.has('lat') + ? Promise.reject(new Error('timeout')) + : Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [] }) }); + }; + + const outcome = await geocodeKeylessWithOutcome('half-down probe', { + bias: '30.20,-97.80|30.30,-97.70', fetchImpl: halfDown, + }); + assert.deepEqual(outcome, { place: null, answered: false }); + assert.equal(urls.length, 2); +}); diff --git a/src/voice/gevActions.js b/src/voice/gevActions.js index a56c840e7..a64f4b44d 100644 --- a/src/voice/gevActions.js +++ b/src/voice/gevActions.js @@ -17,6 +17,7 @@ import militaryAwarenessLayer, { import { initCameraVerbs, moveCamera, flyRoute, interruptCameraMotion, adjustOrbitRange } from '../cameraVerbs.js'; import { cachedGroundFloor, warmGroundFloor } from '../data/groundFloor.js'; import { isPickedWorldPosition } from '../data/scenePick.js'; +import { geocodeKeyless } from '../keylessGeocoder.js'; import { resolveRegionRingForQuery } from '../annotations/annotationResolver.js'; import { normalizeRadioCountryInput } from '../data/radioCountry.js'; import { TR3B_CLASS } from '../data/tr3bRegistry.js'; @@ -1265,25 +1266,43 @@ async function resolveRadioLocation(args = {}, coordinates = radioCoordinatePair if (known) return known; if (!query) return null; const apiKey = window.__GOOGLE_MAPS_API_KEY__ || import.meta.env.GOOGLE_MAPS_API_KEY; - if (!apiKey) throw new Error('No Google Maps API key available for Radio location search'); const controller = new AbortController(); const cancelFromTurn = () => controller.abort(); if (options.signal?.aborted) throw radioAbortError(); options.signal?.addEventListener('abort', cancelFromTurn, { once: true }); const timer = setTimeout(() => controller.abort(), 6000); try { - const url = `https://maps.googleapis.com/maps/api/geocode/json?address=${encodeURIComponent(query)}&key=${apiKey}`; - const response = await fetch(url, { signal: controller.signal }); - const body = await response.json(); + if (apiKey) { + const url = `https://maps.googleapis.com/maps/api/geocode/json?address=${encodeURIComponent(query)}&key=${apiKey}`; + const response = await fetch(url, { signal: controller.signal }); + const body = await response.json(); + if (!radioActionIsCurrent(options)) throw radioAbortError(); + const result = body.status === 'OK' ? body.results?.[0] : null; + if (result?.geometry?.location) { + return { + lat: result.geometry.location.lat, + lon: result.geometry.location.lng, + label: result.formatted_address || query, + country: '', + }; + } + } + // Same fallback rule as the LOCATION search box: reached with no key, and + // also when a key exists but Google returned nothing. A missing key used to + // throw here, which surfaced as a failed voice turn rather than as a radio + // station that could not be placed. + // Unbiased, matching the Google call above: a radio location is named + // outright ("radio near Hanoi"), not relative to what the camera shows. + const keyless = await geocodeKeyless(query); if (!radioActionIsCurrent(options)) throw radioAbortError(); - const result = body.status === 'OK' ? body.results?.[0] : null; - if (!result?.geometry?.location) return null; - return { - lat: result.geometry.location.lat, - lon: result.geometry.location.lng, - label: result.formatted_address || query, - country: '', - }; + if (!keyless) return null; + // `country` stays empty, exactly as the Google branch above leaves it. Photon + // does report a country, but in the feature's own language ("Việt Nam", + // "日本"), and `normalizeRadioCountryInput` fails CLOSED on a name it cannot + // map — so forwarding it would turn "radio near Ha Long" into an empty + // station list rather than the nearest station. A keyless lookup must not + // apply a filter the keyed one never applies. + return { lat: keyless.lat, lon: keyless.lng, label: keyless.label || query, country: '' }; } finally { clearTimeout(timer); options.signal?.removeEventListener('abort', cancelFromTurn); diff --git a/src/voice/gevActions.test.mjs b/src/voice/gevActions.test.mjs index 6d61e2d4d..a1bd44688 100644 --- a/src/voice/gevActions.test.mjs +++ b/src/voice/gevActions.test.mjs @@ -6,6 +6,7 @@ import { getContextStore, registerEntityContext } from '../data/contextStore.js' import { DataLayerManager } from '../data/manager.js'; import { getActiveCameraMotion, interruptCameraMotion, moveCamera } from '../cameraVerbs.js'; import { reassertNavigationHandoff, runExplicitNavigation } from '../navigationPolicy.js'; +import { normalizeRadioCountryInput } from '../data/radioCountry.js'; import { TR3B_CLASS } from '../data/tr3bRegistry.js'; import { controlCctv, @@ -3020,3 +3021,120 @@ test('front5: 0.99 km due EAST is the subject, though a degree box rejects it', assert.equal(result.window.centeredOn, 'N546PC'); }); }); + +// ── Keyless Radio location ─────────────────────────────────────────────────── +// +// `resolveRadioLocation` used to be Google-only: no key threw, and a key that +// geocoded to nothing returned null, which the caller reports as "Could not +// resolve Radio location". Both now fall through to Photon. These drive the +// second case, because it reaches the SAME fallback through a running Google +// branch — the no-key branch cannot be driven here, since the key expression +// reads `import.meta.env`, which only Vite defines. + +/** Photon's GeoJSON shape, trimmed to the properties the adapter consumes. */ +function photonFeature({ name, lat, lon, city = '', country = '' }) { + return { + geometry: { type: 'Point', coordinates: [lon, lat] }, + properties: { name, city, country, osm_key: 'place', osm_value: 'city' }, + }; +} + +/** A Radio layer that records what it was asked to select. */ +function radioSelectionHarness() { + let enabled = false; + const calls = []; + const state = { + stationCount: 4, filter: 'all', selected: null, + audioState: 'stopped', volume: 0.8, voiceDucked: false, + }; + const radio = { + getUIState: () => ({ ...state }), + selectRequestedStation(criteria, options) { + calls.push({ criteria, options }); + state.selected = { id: 'kl-1', name: 'Keyless FM' }; + return state.selected; + }, + }; + return { + calls, + dataManager: { + layers: new Map([['radio', { module: radio }]]), + isEnabled: () => enabled, + async setEnabled(_id, value) { enabled = value; }, + }, + }; +} + +/** Install a Google key plus a fetch stub, restoring both afterwards. */ +function installKeyedFetch(t, handler) { + globalThis.window = globalThis.window || { clearTimeout, setTimeout, requestIdleCallback: null }; + const priorKey = globalThis.window.__GOOGLE_MAPS_API_KEY__; + const priorFetch = globalThis.fetch; + globalThis.window.__GOOGLE_MAPS_API_KEY__ = 'unit-test-key'; + globalThis.fetch = handler; + t.after(() => { + globalThis.fetch = priorFetch; + if (priorKey === undefined) delete globalThis.window.__GOOGLE_MAPS_API_KEY__; + else globalThis.window.__GOOGLE_MAPS_API_KEY__ = priorKey; + }); +} + +test('voice Radio: a key that geocodes to nothing still places the station, keylessly', async (t) => { + const { calls, dataManager } = radioSelectionHarness(); + const requests = []; + installKeyedFetch(t, async (url) => { + requests.push(String(url)); + if (String(url).startsWith('https://maps.googleapis.com/')) { + return { ok: true, json: async () => ({ status: 'ZERO_RESULTS', results: [] }) }; + } + assert.match(String(url), /^https:\/\/photon\.komoot\.io\/api\/\?/); + return { + ok: true, + json: async () => ({ + features: [photonFeature({ + name: 'Hạ Long Bay', lat: 20.9101, lon: 107.1839, city: 'Hạ Long', country: 'Việt Nam', + })], + }), + }; + }); + + const result = await controlRadio({}, dataManager, { + action: 'select', locationQuery: 'Hạ Long Bay', + }); + + // Before the fallback existed this was `ok: false, "Could not resolve Radio location"`. + assert.equal(result.ok, true); + assert.equal(result.requestedLocation, 'Hạ Long Bay, Hạ Long, Việt Nam'); + assert.equal(calls.length, 1); + assert.ok(Math.abs(calls[0].criteria.anchor.lat - 20.9101) < 1e-9); + assert.ok(Math.abs(calls[0].criteria.anchor.lon - 107.1839) < 1e-9); + // Google is asked first and exactly once; Photon answers unbiased, in one call. + assert.equal(requests.filter((url) => url.includes('maps.googleapis.com')).length, 1); + assert.equal(requests.filter((url) => url.includes('photon.komoot.io')).length, 1); + assert.match(requests.at(-1), /[?&]q=H%E1%BA%A1\+Long\+Bay/); + assert.doesNotMatch(requests.at(-1), /[?&](lat|lon|bbox)=/, 'a named radio location is not viewport-biased'); +}); + +test('voice Radio: the keyless path applies no country filter the keyed path would not', async (t) => { + // Photon reports the country in the feature's own language ("Việt Nam"), and + // `rankRadioStationsForRequest` fails CLOSED on a country it cannot map — + // returning NO stations. Forwarding it would make a keyless install answer + // "No Radio station matched" for exactly the places it just resolved, while a + // keyed install placed a station. The label may carry it; the filter may not. + const { calls, dataManager } = radioSelectionHarness(); + installKeyedFetch(t, async (url) => (String(url).startsWith('https://maps.googleapis.com/') + ? { ok: true, json: async () => ({ status: 'ZERO_RESULTS', results: [] }) } + : { + ok: true, + json: async () => ({ + features: [photonFeature({ name: 'Kraków', lat: 50.0614, lon: 19.9366, country: 'Polska' })], + }), + })); + + const result = await controlRadio({}, dataManager, { action: 'select', locationQuery: 'Kraków' }); + + assert.equal(result.ok, true); + assert.equal(calls[0].criteria.country, '', 'a localized country name must never reach the station filter'); + assert.equal(normalizeRadioCountryInput('Polska').valid, false, 'and this is why: it would match nothing'); + assert.equal(result.requestedLocation, 'Kraków, Polska', 'the label still names the country honestly'); +});