From 1a61d02b7da5339898a16fd3e4682014df4ee329 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Mon, 12 Oct 2026 10:59:51 -0700 Subject: [PATCH 01/12] fix(geocode): a 'place, region' query lands in the region, not on a namesake From Denver, 'Paris, Texas' flew to a pub in Budapest at rooftop range. The camera-biased Photon answer listed the town first, but the pub's name equals the whole query, and the biased path matched names only. A query with a region now takes a candidate named for the place whose city, county, state, country or country code (or USA/UK for US/GB) contains every word of the region, in both the biased and the unbiased pass; with no such candidate the existing name rules decide. A query with no region after its comma keeps the biased name match. Co-Authored-By: Claude Opus 5.5 --- src/keylessGeocoder.js | 51 +++++++++++++++++++++++++++++- src/keylessGeocoder.test.mjs | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index 6033a6776..fb82ac84e 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -266,6 +266,50 @@ export function selectPhotonFeature( return null; } +/** Short country names people type that Photon's fields don't carry. */ +const COUNTRY_ALIASES = Object.freeze({ US: 'usa', GB: 'uk' }); + +/** The region part of a "place, region" query, normalized ('' if none). */ +function queryRegion(query) { + const [, ...rest] = String(query ?? '').split(','); + return normalizeToponym(rest.join(' ')); +} + +/** + * For a "place, region" query, pick the candidate named for the place that + * actually lies in the region. "Paris, Texas" is also the name of a pub in + * Budapest: matched as one name it beat the town, so the region has to be + * checked against where each candidate is, not against its name. + * @param {object[]} features - Photon features, in the order returned. + * @param {string} query - The query as typed. + * @returns {?object} null when the query names no region or nothing fits. + */ +export function selectQualifiedFeature(features, query) { + const [headText] = String(query ?? '').split(','); + const region = queryRegion(query); + const head = normalizeToponym(headText); + if (!head || !region) return null; + const words = region.split(' '); + return ( + (Array.isArray(features) ? features : []).find((feature) => { + const properties = feature?.properties ?? {}; + const name = normalizeToponym(properties.name); + if (name !== head && !name.startsWith(`${head} `)) return false; + const where = ` ${normalizeToponym( + [ + properties.city, + properties.county, + properties.state, + properties.country, + properties.countrycode, + COUNTRY_ALIASES[String(properties.countrycode ?? '').toUpperCase()], + ].join(' '), + )} `; + return words.every((word) => where.includes(` ${word} `)); + }) ?? null + ); +} + /** Evict the oldest memo entries until the cache is within its cap. */ function trimPhotonCache(cache) { while (cache.size > PHOTON_CACHE_MAX) { @@ -344,12 +388,16 @@ export async function geocodeKeylessWithOutcome( // 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. + // A query naming a region is only answered nearby by a match inside it. let feature = null; let everyAskAnswered = true; if (bias) { const biased = await ask(photonSearchUrl(trimmed, { bias, endpoint })); if (biased === null) everyAskAnswered = false; - else feature = selectPhotonFeature(biased, normalizeToponym(trimmed)); + else + feature = queryRegion(trimmed) + ? selectQualifiedFeature(biased, trimmed) + : selectPhotonFeature(biased, normalizeToponym(trimmed)); } signal?.throwIfAborted(); @@ -364,6 +412,7 @@ export async function geocodeKeylessWithOutcome( // the thing being searched for; the tail only says where to look. const head = normalizeToponym(trimmed.split(',')[0]); feature = + selectQualifiedFeature(anywhere, trimmed) || selectPhotonFeature(anywhere, head, { allowContains: true }) || anywhere[0]; } diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index 2ce8a70b4..bd6b96f59 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -30,6 +30,7 @@ import { photonResultTypes, photonSearchUrl, selectPhotonFeature, + selectQualifiedFeature, } from './keylessGeocoder.js'; import { geocodeNavigationMode } from './locations.js'; @@ -288,6 +289,52 @@ test('a "place, region" query is matched on the place, not the region', async () assert.equal(result.label.startsWith('Hoàn Kiếm Lake'), true, `got ${result.label}`); }); +/** Photon's real answers for "Paris, Texas" biased to Denver, then unbiased. */ +const PARIS_TX = { + geometry: { type: 'Point', coordinates: [-95.5555, 33.6609] }, + properties: { osm_key: 'place', osm_value: 'town', type: 'city', name: 'Paris', state: 'Texas', country: 'United States', countrycode: 'US' }, +}; +const PARIS_TEXAS_PUB = { + geometry: { type: 'Point', coordinates: [19.0606, 47.4868] }, + properties: { osm_key: 'amenity', osm_value: 'pub', type: 'house', name: 'Paris, Texas', city: 'Budapest', country: 'Magyarország', countrycode: 'HU' }, +}; +const PARIS_TEXAS_SALON = { + geometry: { type: 'Point', coordinates: [153.03, -27.47] }, + properties: { osm_key: 'shop', osm_value: 'hairdresser', type: 'house', name: 'Paris Texas', city: 'Brisbane', state: 'Queensland', country: 'Australia', countrycode: 'AU' }, +}; + +test('"Paris, Texas" is the town in Texas, not a pub of that name in Budapest', async () => { + // Measured from Denver against the live service: the biased answer listed the + // town first, but the pub's name equals the whole query and won, flying the + // camera to Budapest at rooftop range. + const fetchImpl = photonPair([PARIS_TX, PARIS_TEXAS_PUB, PARIS_TEXAS_SALON], [PARIS_TX]); + + const result = await geocodeKeyless('Paris, Texas', { bias: '39.70,-104.90|39.80,-104.80', fetchImpl }); + + assert.equal(Math.round(result.lat), 34, `got ${result.label}`); + assert.equal(fetchImpl.urls.length, 1); +}); + +test('a region is checked against where a candidate is, not its name', () => { + const pubFirst = [PARIS_TEXAS_PUB, PARIS_TEXAS_SALON, PARIS_TX]; + assert.equal(selectQualifiedFeature(pubFirst, 'Paris, Texas'), PARIS_TX); + assert.equal(selectQualifiedFeature(pubFirst, 'Paris, United States'), PARIS_TX); + assert.equal(selectQualifiedFeature(pubFirst, 'Paris, US'), PARIS_TX); + // No region, or a region nothing lies in, leaves the choice to the name rules. + assert.equal(selectQualifiedFeature(pubFirst, 'Paris'), null); + assert.equal(selectQualifiedFeature(pubFirst, 'Paris, Ontario'), null); + assert.equal(selectQualifiedFeature(undefined, 'Paris, Texas'), null); +}); + +test('a region nearby fits nothing, so the unbiased answer decides', async () => { + const fetchImpl = photonPair([PARIS_TEXAS_PUB], [PARIS_TX, PARIS_TEXAS_PUB]); + + const result = await geocodeKeyless('Paris, Texas', { bias: '47.40,19.00|47.50,19.10', fetchImpl }); + + assert.equal(Math.round(result.lat), 34, `got ${result.label}`); + assert.equal(fetchImpl.urls.length, 2); +}); + 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'); @@ -452,3 +499,16 @@ function readGeocodingConsumer(file) { ? [file, 'annotations/resolver.js'] : [file]; return files.map((name) => fs.readFileSync(path.join(ROOT, 'src', name), 'utf8')).join('\n'); } + +test('"USA" and "UK" name their countries, and a trailing comma keeps the nearby answer', async () => { + const pubFirst = [PARIS_TEXAS_PUB, PARIS_TX]; + assert.equal(selectQualifiedFeature(pubFirst, 'Paris, Texas, USA'), PARIS_TX); + // "Paris," names no region: the camera-biased pass still answers. + const fetchImpl = photonPair([PARIS_TX], []); + const result = await geocodeKeyless('Paris,', { + bias: '33.60,-95.60|33.70,-95.50', + fetchImpl, + }); + assert.equal(Math.round(result.lat), 34, `got ${result.label}`); + assert.equal(fetchImpl.urls.length, 1); +}); From d7986309b6d0b89c8d493378e96f209780f08768 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:24:41 -0700 Subject: [PATCH 02/12] fix(terrain): pause upstream calls after a 429 A rate-limited upstream got asked again by every page request for new points (about once a second while the camera moves), each with its own retries, which kept the limit in force and logged "refresh incomplete" each time. After a 429 the proxy now leaves the upstream alone for its Retry-After (at most 10 minutes), or 60 s when it sends none, serving cached points meanwhile. The warning that starts the pause says "(HTTP 429; pausing upstream for N s)"; requests refused during the pause don't log. Co-Authored-By: Claude Opus 5.5 --- server/providers/terrain.js | 44 ++++++++++++++++--- src/tooling/environmentProviders.test.mjs | 52 +++++++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/server/providers/terrain.js b/server/providers/terrain.js index f4817f602..4d122d7ac 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -6,6 +6,7 @@ import { parseTerrainPoints, resolveTerrainHeightRequest, terrainPointKey, + terrainRetryAfterMs, validTerrainResult, } from '../../src/data/terrainHeightsProxy.js'; import { makeRateLimiter, clientKey } from './common/rate-limit.js'; @@ -55,6 +56,15 @@ export function terrainHeightsProxy(options = {}) { // 2026-08-21). 64 points costs 5.6-12 s, inside the timeout at both ends of // that range. const UPSTREAM_CHUNK = 64; + // After a 429 the upstream is left alone for its Retry-After (at most + // RATE_LIMIT_PAUSE_MAX_MS, so a far-off date can't stop terrain until a + // restart), or RATE_LIMIT_PAUSE_MS when it gives none. Every page poll for + // new points (about once a second while the camera moves) otherwise asks + // again with its own retries, which keeps the limit in force and logs each + // time. + const RATE_LIMIT_PAUSE_MS = 60_000; + const RATE_LIMIT_PAUSE_MAX_MS = 10 * 60_000; + let rateLimitedUntil = 0; const MAX_POINTS = UPSTREAM_CHUNK; /** * Entry ceiling for the point cache, and with it the file written from it. @@ -211,15 +221,31 @@ export function terrainHeightsProxy(options = {}) { * @returns {Promise>} */ async function fetchUpstreamAll(points) { + if (Date.now() < rateLimitedUntil) { + // Rate-limited: asking again only extends the limit. Cached points are + // still served; the rest resolve on the page through its geoid. + const error = new Error('HTTP 429'); + error.coolingDown = true; + throw error; + } const results = []; let firstError = null; for (let i = 0; i < points.length; i += UPSTREAM_CHUNK) { const chunk = points.slice(i, i + UPSTREAM_CHUNK); let chunkResults = []; - try { - chunkResults = await fetchTerrainChunkWithRetry(chunk); - } catch (error) { - firstError = firstError || error; + if (Date.now() >= rateLimitedUntil) { + try { + chunkResults = await fetchTerrainChunkWithRetry(chunk); + } catch (error) { + firstError = firstError || error; + if (error?.message === 'HTTP 429') + rateLimitedUntil = + Date.now() + + Math.min( + terrainRetryAfterMs(error.retryAfter) ?? RATE_LIMIT_PAUSE_MS, + RATE_LIMIT_PAUSE_MAX_MS, + ); + } } // Keep later chunks aligned even if a malformed upstream response omits // trailing positions. The resolver will reject each null individually. @@ -292,9 +318,17 @@ export function terrainHeightsProxy(options = {}) { evictOverflow(); diskDirty = true; } - if (outcome.upstreamError) { + if (outcome.upstreamError?.coolingDown) { + // Logged once, when the pause began. + } else if (outcome.upstreamError) { + const pause = + outcome.upstreamError.message === 'HTTP 429' && + Date.now() < rateLimitedUntil + ? ` (HTTP 429; pausing upstream for ${Math.ceil((rateLimitedUntil - Date.now()) / 1000)} s)` + : ''; console.warn( '[terrain-heights-proxy] refresh incomplete' + + pause + ' — serving stale points when available', ); } else if (outcome.absentPoints > 0) { diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index 5e786e408..dc0bc6ab7 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -411,3 +411,55 @@ test('terrain middleware retains successful chunks around a failure and retries ); assert.equal(requests.length, 4); }); + +test('after a 429 the terrain proxy leaves the upstream alone and logs once', async (t) => { + isolate(t); + const warnings = []; + t.mock.method(console, 'warn', (...args) => warnings.push(args.join(' '))); + let calls = 0; + t.mock.method(globalThis, 'fetch', async () => { + calls += 1; + return new Response('', { status: 429 }); + }); + const request = install(terrainHeightsProxy()); + await request('/?points=1,2'); + const afterFirst = calls; + assert.ok(afterFirst >= 1); + assert.equal(warnings.length, 1); + assert.match(warnings[0], /HTTP 429; pausing upstream for 60 s/); + // New points during the pause: no upstream call, no new log line. + const paused = await request('/?points=3,4'); + assert.equal(calls, afterFirst); + assert.equal(paused.status, 502); + assert.equal(warnings.length, 1); +}); + +test('a far-off Retry-After pauses the terrain upstream for at most 10 minutes', async (t) => { + isolate(t); + t.mock.method(console, 'warn', () => {}); + t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 }); + let limited = true; + t.mock.method(globalThis, 'fetch', async () => + limited + ? new Response('', { status: 429, headers: { 'retry-after': '86400' } }) + : Response.json({ results: [{ ellipsoid: 100 }] }), + ); + const request = install(terrainHeightsProxy()); + // The first request spends its retry budget on 429s, then the pause begins. + let settled = false; + const first = request('/?points=1,2').then((res) => { + settled = true; + return res; + }); + while (!settled) { + await new Promise((resolve) => setImmediate(resolve)); + t.mock.timers.tick(1_000); + } + assert.equal((await first).status, 502); + limited = false; + const pausedAt = Date.now(); + t.mock.timers.setTime(pausedAt + 9 * 60_000); + assert.equal((await request('/?points=3,4')).status, 502, 'still paused'); + t.mock.timers.setTime(pausedAt + 10 * 60_000 + 1); + assert.equal((await request('/?points=3,4')).status, 200, 'pause over'); +}); From d1bdae20d3d3df101ae74882e4eaaad717168ef7 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:38:20 -0700 Subject: [PATCH 03/12] docs(changelog): note #1402 Co-Authored-By: Claude Opus 5.5 --- changelog.d/1402-terrain-429-pause.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/1402-terrain-429-pause.md diff --git a/changelog.d/1402-terrain-429-pause.md b/changelog.d/1402-terrain-429-pause.md new file mode 100644 index 000000000..fb7e14f10 --- /dev/null +++ b/changelog.d/1402-terrain-429-pause.md @@ -0,0 +1,3 @@ +- The terrain proxy stops asking the upstream for a while after a 429 + (its Retry-After, at most 10 minutes, or 60 s) instead of retrying on every + page request (quanticsoul4772, #1402). From 5ef78b975caf64650e2498ce4a0bb5f862e3fa62 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:38:19 -0700 Subject: [PATCH 04/12] docs(changelog): note #1401 Co-Authored-By: Claude Opus 5.5 --- changelog.d/1401-geocode-place-region.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/1401-geocode-place-region.md diff --git a/changelog.d/1401-geocode-place-region.md b/changelog.d/1401-geocode-place-region.md new file mode 100644 index 000000000..d3aa5c6b8 --- /dev/null +++ b/changelog.d/1401-geocode-place-region.md @@ -0,0 +1,2 @@ +- A "place, region" search such as "Paris, Texas" now lands on the place in + that region instead of a namesake elsewhere (quanticsoul4772, #1401). From 7775f6007dcd889e666446baa842b7ce28c168c8 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:56:33 -0700 Subject: [PATCH 05/12] fix(geocode): a region match takes the place itself, not a name starting with it A name that only started with the place word also matched, so from Lyon "Paris, France" picked "Paris Gare de Lyon" in the biased pass, where main fell through to the unbiased answer and reached Paris. Only an exact name matches now. Adds a test for the UK alias. Co-Authored-By: Claude Opus 5.5 --- src/keylessGeocoder.js | 4 ++- src/keylessGeocoder.test.mjs | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index fb82ac84e..5c69a5aa0 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -294,7 +294,9 @@ export function selectQualifiedFeature(features, query) { (Array.isArray(features) ? features : []).find((feature) => { const properties = feature?.properties ?? {}; const name = normalizeToponym(properties.name); - if (name !== head && !name.startsWith(`${head} `)) return false; + // Exactly the place: "Paris Gare de Lyon" is in France too, but it + // is not what "Paris, France" asks for. + if (name !== head) return false; const where = ` ${normalizeToponym( [ properties.city, diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index bd6b96f59..81e910fe5 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -512,3 +512,51 @@ test('"USA" and "UK" name their countries, and a trailing comma keeps the nearby assert.equal(Math.round(result.lat), 34, `got ${result.label}`); assert.equal(fetchImpl.urls.length, 1); }); + +test('a region match takes the place itself, not a place whose name starts with it', () => { + const station = { + geometry: { type: 'Point', coordinates: [4.86, 45.76] }, + properties: { + osm_key: 'railway', + name: 'Paris Gare de Lyon', + city: 'Lyon', + country: 'France', + countrycode: 'FR', + }, + }; + const paris = { + geometry: { type: 'Point', coordinates: [2.35, 48.86] }, + properties: { + osm_key: 'place', + name: 'Paris', + state: 'Île-de-France', + country: 'France', + countrycode: 'FR', + }, + }; + assert.equal(selectQualifiedFeature([station, paris], 'Paris, France'), paris); + assert.equal(selectQualifiedFeature([station], 'Paris, France'), null); +}); + +test('"UK" names the United Kingdom', () => { + const london = { + geometry: { type: 'Point', coordinates: [-0.13, 51.51] }, + properties: { + osm_key: 'place', + name: 'London', + country: 'United Kingdom', + countrycode: 'GB', + }, + }; + const ontario = { + geometry: { type: 'Point', coordinates: [-81.25, 42.98] }, + properties: { + osm_key: 'place', + name: 'London', + state: 'Ontario', + country: 'Canada', + countrycode: 'CA', + }, + }; + assert.equal(selectQualifiedFeature([ontario, london], 'London, UK'), london); +}); From 3ac986882c46451c7270dc2fb133bea43cb2b15d Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:56:47 -0700 Subject: [PATCH 06/12] test(terrain): bound the 429 pause test's settle loop Co-Authored-By: Claude Opus 5.5 --- src/tooling/environmentProviders.test.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index dc0bc6ab7..52dcd8351 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -451,10 +451,12 @@ test('a far-off Retry-After pauses the terrain upstream for at most 10 minutes', settled = true; return res; }); - while (!settled) { + // Bounded: the retry budget is 10 s, so 60 simulated seconds is plenty. + for (let step = 0; step < 60 && !settled; step += 1) { await new Promise((resolve) => setImmediate(resolve)); t.mock.timers.tick(1_000); } + assert.ok(settled, 'the first request settles within its retry budget'); assert.equal((await first).status, 502); limited = false; const pausedAt = Date.now(); From 91769d74b81148a52179c91f022b1549ddf8becd Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:07:46 -0700 Subject: [PATCH 07/12] fix(geocode): keep a nearby whole-name match as the last choice, and document the region rule A query with a comma counted as "place, region", so a nearby place whose own name has a comma ("Bed, Bath & Beyond") no longer won the nearby pass and the unbiased pass could fly to a namesake far away. Its whole-name match is now kept as the last choice, after the region checks of both passes. CURRENT-STATE describes the place-region rule. Co-Authored-By: Claude Opus 5.5 --- docs/CURRENT-STATE.md | 6 +++++- src/keylessGeocoder.js | 18 +++++++++++++----- src/keylessGeocoder.test.mjs | 23 +++++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index cc4e7b861..529ffb6e6 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5191,7 +5191,11 @@ HTTP refusals and outages remain retryable. Searches share a 12-second total deadline, with Photon requests capped at six seconds each. Caller/application cancellation stops retries and late cache writes. Replacing a location search cancels the previous lookup; disposing its controls cancels the active lookup. Photon uses a soft proximity bias, up to five -candidates, and an unbiased retry for name mismatches. Invalid/wrapped bounds +candidates, and an unbiased retry for name mismatches. A "place, region" query +only accepts a candidate named exactly for the place whose city, county, state, +country or country code (or USA/UK) contains the region; nearby results that +fail that check are retried unbiased, and a nearby place whose whole name is the +query (one with a comma in it) is the last choice. Invalid/wrapped bounds are omitted rather than framing the wrong part of the globe. ### Map Stack Switcher (June 2026) diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index 5c69a5aa0..2712dd423 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -390,16 +390,22 @@ export async function geocodeKeylessWithOutcome( // 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. - // A query naming a region is only answered nearby by a match inside it. + // A query naming a region is answered nearby only by a match inside it. + // A nearby place whose own name has a comma ("Bed, Bath & Beyond") and + // equals the whole query is kept as the last choice, after the region + // checks of both passes. let feature = null; + let nearbyNameMatch = null; let everyAskAnswered = true; if (bias) { const biased = await ask(photonSearchUrl(trimmed, { bias, endpoint })); if (biased === null) everyAskAnswered = false; - else - feature = queryRegion(trimmed) - ? selectQualifiedFeature(biased, trimmed) - : selectPhotonFeature(biased, normalizeToponym(trimmed)); + else if (queryRegion(trimmed)) { + feature = selectQualifiedFeature(biased, trimmed); + nearbyNameMatch = selectPhotonFeature(biased, normalizeToponym(trimmed)); + } else { + feature = selectPhotonFeature(biased, normalizeToponym(trimmed)); + } } signal?.throwIfAborted(); @@ -415,9 +421,11 @@ export async function geocodeKeylessWithOutcome( const head = normalizeToponym(trimmed.split(',')[0]); feature = selectQualifiedFeature(anywhere, trimmed) || + nearbyNameMatch || selectPhotonFeature(anywhere, head, { allowContains: true }) || anywhere[0]; } + if (!feature) feature = nearbyNameMatch; } signal?.throwIfAborted(); diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index 81e910fe5..7759d5226 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -560,3 +560,26 @@ test('"UK" names the United Kingdom', () => { }; assert.equal(selectQualifiedFeature([ontario, london], 'London, UK'), london); }); + +test('a nearby place whose own name has a comma still wins when nothing matches as place and region', async () => { + const store = (lon, lat, city) => ({ + geometry: { type: 'Point', coordinates: [lon, lat] }, + properties: { + osm_key: 'shop', + osm_value: 'houseware', + name: 'Bed, Bath & Beyond', + city, + countrycode: 'US', + }, + }); + const nearby = store(-104.94, 39.66, 'Denver'); + const fetchImpl = photonPair([nearby], [store(-73.99, 40.74, 'New York')]); + + const result = await geocodeKeyless('Bed, Bath & Beyond', { + bias: '39.60,-105.00|39.70,-104.90', + fetchImpl, + }); + + assert.match(result.label, /Denver/); + assert.equal(fetchImpl.urls.length, 2, 'the region checks ran first'); +}); From 923211c24ac5f38d0ac04a2ab554792385d472e6 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:08:26 -0700 Subject: [PATCH 08/12] fix(terrain): key the 429 pause on status, never shorten it, and document it Review follow-ups: the upstream error now carries its HTTP status and the pause checks error.status instead of the message text; concurrent 429s extend the pause with Math.max rather than a shorter Retry-After cutting a longer one short; the per-chunk re-check says it is for multi-chunk requests; the first 429 test runs its backoff on mocked timers. CURRENT-STATE describes the pause. Co-Authored-By: Claude Opus 5.5 --- docs/CURRENT-STATE.md | 6 +++++- server/providers/terrain.js | 20 +++++++++++++------- src/data/terrainHeightsProxy.js | 1 + src/tooling/environmentProviders.test.mjs | 12 +++++++++++- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index cc4e7b861..d0ead090b 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -3005,7 +3005,11 @@ down | auth-failed` (plus the unchanged `missing-key`/`unsupported`) with > usable per point, while an uncached absent height still returns 502 rather > than becoming a fabricated ground value. A position the upstream answers > with a null ellipsoid is reported as an absent height rather than a refresh -> failure, and is left uncached so a later poll re-asks it. Client geoid +> failure, and is left uncached so a later poll re-asks it. After a 429 that +> outlasts its retries, the proxy pauses upstream calls for the `Retry-After` +> (60 seconds when none, at most 10 minutes): cached and stale points are still +> served, uncached points get 502 and use the page's geoid fallback, and the +> pause is logged once by the request that started it. Client geoid > fallbacks wait 60 seconds before retrying and self-heal to Re:Earth on the > first later successful fetch. Smaller chunks reduce timeout risk; complete > camera batches still wait for their sequential requests, and unresolved diff --git a/server/providers/terrain.js b/server/providers/terrain.js index 4d122d7ac..b217cd900 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -225,6 +225,7 @@ export function terrainHeightsProxy(options = {}) { // Rate-limited: asking again only extends the limit. Cached points are // still served; the rest resolve on the page through its geoid. const error = new Error('HTTP 429'); + error.status = 429; error.coolingDown = true; throw error; } @@ -233,18 +234,23 @@ export function terrainHeightsProxy(options = {}) { for (let i = 0; i < points.length; i += UPSTREAM_CHUNK) { const chunk = points.slice(i, i + UPSTREAM_CHUNK); let chunkResults = []; + // Re-checked per chunk for when a request spans several chunks again; + // today requests are capped at one chunk (MAX_POINTS). if (Date.now() >= rateLimitedUntil) { try { chunkResults = await fetchTerrainChunkWithRetry(chunk); } catch (error) { firstError = firstError || error; - if (error?.message === 'HTTP 429') - rateLimitedUntil = + // Concurrent 429s extend the pause, never cut it short. + if (error?.status === 429) + rateLimitedUntil = Math.max( + rateLimitedUntil, Date.now() + - Math.min( - terrainRetryAfterMs(error.retryAfter) ?? RATE_LIMIT_PAUSE_MS, - RATE_LIMIT_PAUSE_MAX_MS, - ); + Math.min( + terrainRetryAfterMs(error.retryAfter) ?? RATE_LIMIT_PAUSE_MS, + RATE_LIMIT_PAUSE_MAX_MS, + ), + ); } } // Keep later chunks aligned even if a malformed upstream response omits @@ -322,7 +328,7 @@ export function terrainHeightsProxy(options = {}) { // Logged once, when the pause began. } else if (outcome.upstreamError) { const pause = - outcome.upstreamError.message === 'HTTP 429' && + outcome.upstreamError.status === 429 && Date.now() < rateLimitedUntil ? ` (HTTP 429; pausing upstream for ${Math.ceil((rateLimitedUntil - Date.now()) / 1000)} s)` : ''; diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index c83b30a87..562d4d7c1 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -145,6 +145,7 @@ export async function fetchTerrainChunkWithRetry( const res = await fetchImpl(url, signal ? { signal } : {}); if (!res.ok) { const error = new Error(`HTTP ${res.status}`); + error.status = res.status; error.retryable = res.status === 429 || res.status >= 500; error.retryAfter = res.headers?.get?.('retry-after') ?? null; throw error; diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index 52dcd8351..e541a8069 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -421,8 +421,18 @@ test('after a 429 the terrain proxy leaves the upstream alone and logs once', as calls += 1; return new Response('', { status: 429 }); }); + t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 }); const request = install(terrainHeightsProxy()); - await request('/?points=1,2'); + // The retries' backoff runs on mocked time. + let settled = false; + const first = request('/?points=1,2').then(() => { + settled = true; + }); + for (let step = 0; step < 60 && !settled; step += 1) { + await new Promise((resolve) => setImmediate(resolve)); + t.mock.timers.tick(1_000); + } + await first; const afterFirst = calls; assert.ok(afterFirst >= 1); assert.equal(warnings.length, 1); From 5410c973072acd958130da7d7d3ff16281eac3a0 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:55:17 -0700 Subject: [PATCH 09/12] test(terrain): fail the first 429 test instead of hanging if its request never settles Co-Authored-By: Claude Opus 5.5 --- src/tooling/environmentProviders.test.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index e541a8069..45940e00b 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -432,6 +432,7 @@ test('after a 429 the terrain proxy leaves the upstream alone and logs once', as await new Promise((resolve) => setImmediate(resolve)); t.mock.timers.tick(1_000); } + assert.ok(settled, 'the first request settles within its retry budget'); await first; const afterFirst = calls; assert.ok(afterFirst >= 1); From 2c16dd6601bd48278ae5a9f5dfab0f4310c02e34 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:55:35 -0700 Subject: [PATCH 10/12] fix(geocode): match the nearby whole-name fallback exactly Co-Authored-By: Claude Opus 5.5 --- src/keylessGeocoder.js | 8 +++++++- src/keylessGeocoder.test.mjs | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index d2b6b7f45..d33c9d0ce 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -417,7 +417,13 @@ export async function geocodeKeylessWithOutcome( if (biased === null) everyAskAnswered = false; else if (queryRegion(trimmed)) { feature = selectQualifiedFeature(biased, trimmed); - nearbyNameMatch = selectPhotonFeature(biased, normalizeToponym(trimmed)); + // Exactly the whole query: "Springfield IL Auto" only starts with it. + const whole = normalizeToponym(trimmed); + nearbyNameMatch = + biased.find( + (candidate) => + normalizeToponym(candidate?.properties?.name) === whole, + ) ?? null; } else { feature = selectPhotonFeature(biased, normalizeToponym(trimmed)); } diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index b7078d9d0..22a1d257e 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -594,3 +594,30 @@ test('wrapped view bias stays near the antimeridian and malformed coordinates ar assert.equal(new URL(photonSearchUrl('island', { bias })).searchParams.has('lon'), false); } }); + +test('the nearby whole-name fallback takes an exact name, not one that only starts with the query', async () => { + const place = (name, lon, lat, props) => ({ + geometry: { type: 'Point', coordinates: [lon, lat] }, + properties: { osm_key: 'place', name, ...props }, + }); + const nearby = [ + place('Springfield IL Auto', -89.6, 39.8, { + osm_key: 'shop', + city: 'Springfield', + state: 'Illinois', + countrycode: 'US', + }), + ]; + const anywhere = [ + place('Springfield', -89.65, 39.78, { state: 'Illinois', countrycode: 'US' }), + ]; + const fetchImpl = photonPair(nearby, anywhere); + + const result = await geocodeKeyless('Springfield, IL', { + bias: '39.70,-89.70|39.90,-89.50', + fetchImpl, + }); + + assert.doesNotMatch(result.label, /Auto/); + assert.match(result.label, /^Springfield/); +}); From c1820aea4950b259605f4335e35e4fea231e6aa5 Mon Sep 17 00:00:00 2001 From: Said Turksever Date: Wed, 28 Oct 2026 14:09:12 +0000 Subject: [PATCH 11/12] fix(terrain): end retries early on a long Retry-After; cover the 429 pause A Retry-After longer than the 10 s retry budget used to sleep the whole budget out and then give up, so the proxy's 429 pause started 10 s late and every new-point poll in that window still went upstream and logged. The retry loop now stops at once in that case. Tests: the retry loop no longer sleeps for an unreachable Retry-After; fully cached points, including stale ones, are served during the pause with no upstream call; a 503 does not pause; a shorter concurrent Retry-After never cuts the pause short. CURRENT-STATE now says which requests are still served and which log. --- changelog.d/1402-terrain-429-pause.md | 4 +- docs/CURRENT-STATE.md | 11 +-- server/providers/terrain.js | 3 +- src/data/terrainHeightsProxy.js | 6 +- src/data/terrainHeightsProxy.test.mjs | 30 ++++++++ src/tooling/environmentProviders.test.mjs | 88 +++++++++++++++++++++++ 6 files changed, 135 insertions(+), 7 deletions(-) diff --git a/changelog.d/1402-terrain-429-pause.md b/changelog.d/1402-terrain-429-pause.md index fb7e14f10..05b18680b 100644 --- a/changelog.d/1402-terrain-429-pause.md +++ b/changelog.d/1402-terrain-429-pause.md @@ -1,3 +1,5 @@ - The terrain proxy stops asking the upstream for a while after a 429 (its Retry-After, at most 10 minutes, or 60 s) instead of retrying on every - page request (quanticsoul4772, #1402). + page request, and a Retry-After longer than its 10 s retry budget now ends + the retries at once instead of sleeping the budget out (quanticsoul4772, + #1402). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index df37c6805..a8f3646c8 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -3078,11 +3078,14 @@ down | auth-failed` (plus the unchanged `missing-key`/`unsupported`) with > usable per point, while an uncached absent height still returns 502 rather > than becoming a fabricated ground value. A position the upstream answers > with a null ellipsoid is reported as an absent height rather than a refresh -> failure, and is left uncached so a later poll re-asks it. After a 429 that +> failure, and is left uncached so a later poll re-asks it. A `Retry-After` +> longer than the 10 s retry budget ends the retries at once. After a 429 that > outlasts its retries, the proxy pauses upstream calls for the `Retry-After` -> (60 seconds when none, at most 10 minutes): cached and stale points are still -> served, uncached points get 502 and use the page's geoid fallback, and the -> pause is logged once by the request that started it. Client geoid +> (60 seconds when none, at most 10 minutes). A request whose points are all +> cached, fresh or stale, is still served; a request with any uncached point +> gets 502 and the page uses its geoid fallback for that chunk. Each request +> that was waiting on the upstream when the 429 came back logs one warning; +> requests refused during the pause do not log. Client geoid > fallbacks wait 60 seconds before retrying and self-heal to Re:Earth on the > first later successful fetch. Smaller chunks reduce timeout risk; complete > camera batches still wait for their sequential requests, and unresolved diff --git a/server/providers/terrain.js b/server/providers/terrain.js index b217cd900..35606308a 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -209,7 +209,8 @@ export function terrainHeightsProxy(options = {}) { * * The first try keeps its empirically required 30s timeout; network errors, * 429, and 5xx receive up to three jittered retries sharing a 10s added-time - * budget, with Retry-After honored within that bound. + * budget, with Retry-After honored within that bound; a longer Retry-After + * ends the retries at once so the 429 pause below starts without delay. * * A chunk that still fails yields nulls for its own positions rather than * rejecting: with several chunks per request, throwing would discard every diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index 562d4d7c1..f06ff3671 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -107,7 +107,10 @@ export function terrainRetryAfterMs(value, nowMs = Date.now()) { /** * Fetch one upstream chunk with jittered exponential backoff. Network errors, * 429, and 5xx retry; other HTTP failures and malformed successes fail fast. - * Retry sleeps and retry attempts share a 10s budget after attempt one. + * Retry sleeps and retry attempts share a 10s budget after attempt one. A + * Retry-After longer than the budget left ends the retries at once: sleeping + * out the budget cannot reach the time the upstream asked for, and it would + * delay the caller's own back-off (the proxy's 429 pause) by up to 10 s. * * Dependencies are injectable for deterministic offline tests. * @param {Array<[number, number]>} points @@ -168,6 +171,7 @@ export async function fetchTerrainChunkWithRetry( if (remaining <= 0) break; const retryAfterMs = terrainRetryAfterMs(error?.retryAfter, now()); + if (retryAfterMs != null && retryAfterMs > remaining) break; const backoffMs = Math.round( 350 * 2 ** attempt * (0.75 + random() * 0.5), ); diff --git a/src/data/terrainHeightsProxy.test.mjs b/src/data/terrainHeightsProxy.test.mjs index 50e45b732..c9e143337 100644 --- a/src/data/terrainHeightsProxy.test.mjs +++ b/src/data/terrainHeightsProxy.test.mjs @@ -91,6 +91,36 @@ test('429 retry honors Retry-After before the next attempt', async () => { assert.equal(results[0].id, 'recovered'); }); +test('a Retry-After longer than the retry budget ends the retries without sleeping', async () => { + let clock = 3_000_000; + let attempts = 0; + const sleeps = []; + await assert.rejects( + fetchTerrainChunkWithRetry([[1, 1]], { + now: () => clock, + random: () => 0.5, + makeSignal: () => undefined, + sleep: async (ms) => { + sleeps.push(ms); + clock += ms; + }, + fetchImpl: async () => { + attempts += 1; + return { + ok: false, + status: 429, + headers: { + get: (name) => (name.toLowerCase() === 'retry-after' ? '30' : null), + }, + }; + }, + }), + (error) => error.status === 429 && error.retryAfter === '30', + ); + assert.equal(attempts, 1); + assert.deepEqual(sleeps, [], 'no sleep that cannot reach the Retry-After'); +}); + test('permanently failing upstream exhausts retries and returns 502 without fake results', async () => { let clock = 2_000_000; let attempts = 0; diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index 45940e00b..d2f827838 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -476,3 +476,91 @@ test('a far-off Retry-After pauses the terrain upstream for at most 10 minutes', t.mock.timers.setTime(pausedAt + 10 * 60_000 + 1); assert.equal((await request('/?points=3,4')).status, 200, 'pause over'); }); + +/** Drive mocked timers until `promise` settles; the retry budget is 10 s. */ +async function settleOnMockedTime(t, promise) { + let settled = false; + const tracked = promise.finally(() => { + settled = true; + }); + for (let step = 0; step < 60 && !settled; step += 1) { + await new Promise((resolve) => setImmediate(resolve)); + t.mock.timers.tick(1_000); + } + assert.ok(settled, 'the request settles within its retry budget'); + return tracked; +} + +test('during a terrain 429 pause fully cached points, even stale ones, are still served', async (t) => { + isolate(t); + t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 }); + let calls = 0; + let limited = false; + t.mock.method(globalThis, 'fetch', async () => { + calls += 1; + return limited + ? new Response('', { status: 429, headers: { 'retry-after': '300' } }) + : Response.json({ results: [{ ellipsoid: 100 }] }); + }); + const request = install(terrainHeightsProxy()); + assert.equal((await request('/?points=5,6')).status, 200); + // Past the 30-day TTL, so 5,6 is stale and would normally be refreshed. + t.mock.timers.setTime(Date.now() + 31 * 24 * 3600_000); + limited = true; + const first = await settleOnMockedTime(t, request('/?points=1,2')); + assert.equal(first.status, 502); + assert.equal(calls, 2, 'a Retry-After past the budget is not retried'); + const stale = await request('/?points=5,6'); + assert.equal(stale.status, 200); + assert.deepEqual(json(stale).results, [{ ellipsoid: 100 }]); + assert.equal((await request('/?points=5,6;7,8')).status, 502); + assert.equal(calls, 2, 'no upstream call during the pause'); +}); + +test('a terrain upstream failure other than 429 does not pause later requests', async (t) => { + isolate(t); + t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 }); + let calls = 0; + t.mock.method(globalThis, 'fetch', async () => { + calls += 1; + return new Response('', { status: 503 }); + }); + const request = install(terrainHeightsProxy()); + const first = await settleOnMockedTime(t, request('/?points=1,2')); + assert.equal(first.status, 502); + const afterFirst = calls; + const second = await settleOnMockedTime(t, request('/?points=3,4')); + assert.equal(second.status, 502); + assert.ok(calls > afterFirst, 'new points still go upstream after a 503'); +}); + +test('a shorter concurrent Retry-After never cuts a terrain 429 pause short', async (t) => { + isolate(t); + t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 }); + let calls = 0; + t.mock.method(globalThis, 'fetch', async (raw) => { + calls += 1; + const points = new URL(raw).searchParams.get('points'); + if (points.startsWith('0.')) + return Response.json({ results: [{ ellipsoid: 1 }] }); + const retryAfter = points.startsWith('1.') ? '300' : '30'; + return new Response('', { + status: 429, + headers: { 'retry-after': retryAfter }, + }); + }); + const request = install(terrainHeightsProxy()); + // Load the disk cache first so both requests below reach the upstream in + // order: the 300 s answer lands first, the 30 s answer after it. + assert.equal((await request('/?points=0,0')).status, 200); + const [long, short] = await settleOnMockedTime( + t, + Promise.all([request('/?points=1,2'), request('/?points=3,4')]), + ); + assert.equal(long.status, 502); + assert.equal(short.status, 502); + const afterBoth = calls; + t.mock.timers.setTime(Date.now() + 120_000); + assert.equal((await request('/?points=5,6')).status, 502); + assert.equal(calls, afterBoth, 'still paused by the 300 s Retry-After'); +}); From 7788168d730e39418b022a73459534f60053bd84 Mon Sep 17 00:00:00 2001 From: Said Turksever Date: Wed, 28 Oct 2026 14:09:56 +0000 Subject: [PATCH 12/12] fix(geocode): prefer the settlement when a POI also fits the region When several candidates fit a "place, region" query, take a settlement or administrative area over a POI of the same name, so a cafe called "Paris" in Lyon cannot win "Paris, France" from the nearby pass. Tests: region queries without a camera bias, whole-word and every-word region matching, city and county regions, the nearby whole-name answer when the unbiased request fails, and the settlement preference. The new tests sit with the other Paris, Texas tests. The station fixture uses its real city. CURRENT-STATE now describes the order the code applies. --- docs/CURRENT-STATE.md | 11 +- src/keylessGeocoder.js | 23 ++- src/keylessGeocoder.test.mjs | 305 ++++++++++++++++++++++------------- 3 files changed, 218 insertions(+), 121 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index d4a3da982..92220f6fb 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5268,11 +5268,12 @@ deadline, with Photon requests capped at six seconds each. Caller/application cancellation stops retries and late cache writes. Replacing a location search cancels the previous lookup; disposing its controls cancels the active lookup. Photon uses a soft proximity bias, up to five candidates, and an unbiased retry for name mismatches. A "place, region" query -only accepts a candidate named exactly for the place whose city, county, state, -country or country code (or USA/UK) contains the region; nearby results that -fail that check are retried unbiased, and a nearby place whose whole name is the -query (one with a comma in it) is the last choice. Invalid or wrapped bounds -returned by providers +prefers a candidate named exactly for the place whose city, county, state, +country or country code (or USA/UK) contains every word of the region, and a +settlement or administrative area over a POI when both fit. Nearby results that +fail that check are retried unbiased. After the region checks of both passes, a +nearby place whose whole name is the query (one with a comma in it) comes next, +then the unbiased name match. Invalid or wrapped bounds returned by providers are omitted rather than framing the wrong part of the globe. A location search that finds nothing, or fails, leaves the field open with the diff --git a/src/keylessGeocoder.js b/src/keylessGeocoder.js index d33c9d0ce..4e01f1c74 100644 --- a/src/keylessGeocoder.js +++ b/src/keylessGeocoder.js @@ -284,6 +284,13 @@ export function selectPhotonFeature( /** Short country names people type that Photon's fields don't carry. */ const COUNTRY_ALIASES = Object.freeze({ US: 'usa', GB: 'uk' }); +/** + * OSM keys of settlements and administrative areas. Among candidates that fit + * a "place, region" query, these are what the region names a place in: a cafe + * called "Paris" in Lyon is in France too, but "Paris, France" means the city. + */ +const SETTLEMENT_OSM_KEYS = Object.freeze(new Set(['place', 'boundary'])); + /** The region part of a "place, region" query, normalized ('' if none). */ function queryRegion(query) { const [, ...rest] = String(query ?? '').split(','); @@ -294,7 +301,8 @@ function queryRegion(query) { * For a "place, region" query, pick the candidate named for the place that * actually lies in the region. "Paris, Texas" is also the name of a pub in * Budapest: matched as one name it beat the town, so the region has to be - * checked against where each candidate is, not against its name. + * checked against where each candidate is, not against its name. When several + * candidates fit, a settlement or administrative area wins over a POI. * @param {object[]} features - Photon features, in the order returned. * @param {string} query - The query as typed. * @returns {?object} null when the query names no region or nothing fits. @@ -305,8 +313,8 @@ export function selectQualifiedFeature(features, query) { const head = normalizeToponym(headText); if (!head || !region) return null; const words = region.split(' '); - return ( - (Array.isArray(features) ? features : []).find((feature) => { + const qualified = (Array.isArray(features) ? features : []).filter( + (feature) => { const properties = feature?.properties ?? {}; const name = normalizeToponym(properties.name); // Exactly the place: "Paris Gare de Lyon" is in France too, but it @@ -323,7 +331,14 @@ export function selectQualifiedFeature(features, query) { ].join(' '), )} `; return words.every((word) => where.includes(` ${word} `)); - }) ?? null + }, + ); + return ( + qualified.find((feature) => + SETTLEMENT_OSM_KEYS.has(feature?.properties?.osm_key), + ) ?? + qualified[0] ?? + null ); } diff --git a/src/keylessGeocoder.test.mjs b/src/keylessGeocoder.test.mjs index 22a1d257e..95098a10c 100644 --- a/src/keylessGeocoder.test.mjs +++ b/src/keylessGeocoder.test.mjs @@ -335,6 +335,199 @@ test('a region nearby fits nothing, so the unbiased answer decides', async () => assert.equal(fetchImpl.urls.length, 2); }); +test('"USA" and "UK" name their countries, and a trailing comma keeps the nearby answer', async () => { + const pubFirst = [PARIS_TEXAS_PUB, PARIS_TX]; + assert.equal(selectQualifiedFeature(pubFirst, 'Paris, Texas, USA'), PARIS_TX); + // "Paris," names no region: the camera-biased pass still answers. + const fetchImpl = photonPair([PARIS_TX], []); + const result = await geocodeKeyless('Paris,', { + bias: '33.60,-95.60|33.70,-95.50', + fetchImpl, + }); + assert.equal(Math.round(result.lat), 34, `got ${result.label}`); + assert.equal(fetchImpl.urls.length, 1); +}); + +test('a region match takes the place itself, not a place whose name starts with it', () => { + const station = { + geometry: { type: 'Point', coordinates: [4.86, 45.76] }, + properties: { + osm_key: 'railway', + name: 'Paris Gare de Lyon', + city: 'Paris', + country: 'France', + countrycode: 'FR', + }, + }; + const paris = { + geometry: { type: 'Point', coordinates: [2.35, 48.86] }, + properties: { + osm_key: 'place', + name: 'Paris', + state: 'Île-de-France', + country: 'France', + countrycode: 'FR', + }, + }; + assert.equal(selectQualifiedFeature([station, paris], 'Paris, France'), paris); + assert.equal(selectQualifiedFeature([station], 'Paris, France'), null); +}); + +test('"UK" names the United Kingdom', () => { + const london = { + geometry: { type: 'Point', coordinates: [-0.13, 51.51] }, + properties: { + osm_key: 'place', + name: 'London', + country: 'United Kingdom', + countrycode: 'GB', + }, + }; + const ontario = { + geometry: { type: 'Point', coordinates: [-81.25, 42.98] }, + properties: { + osm_key: 'place', + name: 'London', + state: 'Ontario', + country: 'Canada', + countrycode: 'CA', + }, + }; + assert.equal(selectQualifiedFeature([ontario, london], 'London, UK'), london); +}); + +test('a nearby place whose own name has a comma still wins when nothing matches as place and region', async () => { + const store = (lon, lat, city) => ({ + geometry: { type: 'Point', coordinates: [lon, lat] }, + properties: { + osm_key: 'shop', + osm_value: 'houseware', + name: 'Bed, Bath & Beyond', + city, + countrycode: 'US', + }, + }); + const nearby = store(-104.94, 39.66, 'Denver'); + const fetchImpl = photonPair([nearby], [store(-73.99, 40.74, 'New York')]); + + const result = await geocodeKeyless('Bed, Bath & Beyond', { + bias: '39.60,-105.00|39.70,-104.90', + fetchImpl, + }); + + assert.match(result.label, /Denver/); + // Main answered this from the nearby pass alone. The second request is the + // accepted cost of checking the region unbiased first. + assert.equal(fetchImpl.urls.length, 2, 'the region checks ran first'); +}); + +test('the nearby whole-name fallback takes an exact name, not one that only starts with the query', async () => { + const place = (name, lon, lat, props) => ({ + geometry: { type: 'Point', coordinates: [lon, lat] }, + properties: { osm_key: 'place', name, ...props }, + }); + const nearby = [ + place('Springfield IL Auto', -89.6, 39.8, { + osm_key: 'shop', + city: 'Springfield', + state: 'Illinois', + countrycode: 'US', + }), + ]; + const anywhere = [ + place('Springfield', -89.65, 39.78, { state: 'Illinois', countrycode: 'US' }), + ]; + const fetchImpl = photonPair(nearby, anywhere); + + const result = await geocodeKeyless('Springfield, IL', { + bias: '39.70,-89.70|39.90,-89.50', + fetchImpl, + }); + + assert.doesNotMatch(result.label, /Auto/); + assert.match(result.label, /^Springfield/); +}); + +test('a region query without a camera bias still takes the place in the region', async () => { + // Unbiased, Photon can list the pub first; main took it because its name + // starts with "Paris". + const fetchImpl = photonPair([], [PARIS_TEXAS_PUB, PARIS_TX]); + + const result = await geocodeKeyless('Paris, Texas', { fetchImpl }); + + assert.equal(Math.round(result.lat), 34, `got ${result.label}`); + assert.equal(fetchImpl.urls.length, 1); +}); + +test('a region is matched by whole words, and every word has to fit', () => { + const londonUk = { + geometry: { type: 'Point', coordinates: [-0.13, 51.51] }, + properties: { osm_key: 'place', name: 'London', state: 'England', country: 'United Kingdom', countrycode: 'GB' }, + }; + // "on" is inside "london" and "england", but it is not a word of either. + assert.equal(selectQualifiedFeature([londonUk], 'London, ON'), null); + // "Texas" fits, "Ontario" does not, so the town does not qualify. + assert.equal(selectQualifiedFeature([PARIS_TX], 'Paris, Texas, Ontario'), null); + assert.equal(selectQualifiedFeature([PARIS_TX], 'Paris, Tex'), null); +}); + +test('a region can be the city or the county a candidate lies in', () => { + const street = { + geometry: { type: 'Point', coordinates: [-105.27, 40.02] }, + properties: { osm_key: 'highway', name: 'Main Street', city: 'Boulder', countrycode: 'US' }, + }; + const town = { + geometry: { type: 'Point', coordinates: [-95.5555, 33.6609] }, + properties: { osm_key: 'place', name: 'Paris', county: 'Lamar', countrycode: 'US' }, + }; + assert.equal(selectQualifiedFeature([street], 'Main Street, Boulder'), street); + assert.equal(selectQualifiedFeature([town], 'Paris, Lamar'), town); +}); + +test('when a POI and the place both fit the region, the place wins', async () => { + // A cafe named "Paris" in Lyon is in France too. Nearby it can rank above the + // city, and it would frame at rooftop range. + const cafe = { + geometry: { type: 'Point', coordinates: [4.84, 45.76] }, + properties: { osm_key: 'amenity', osm_value: 'cafe', name: 'Paris', city: 'Lyon', country: 'France', countrycode: 'FR' }, + }; + const paris = { + geometry: { type: 'Point', coordinates: [2.35, 48.86] }, + properties: { osm_key: 'place', osm_value: 'city', type: 'city', name: 'Paris', state: 'Île-de-France', country: 'France', countrycode: 'FR' }, + }; + const fetchImpl = photonPair([cafe, paris], [paris]); + + const result = await geocodeKeyless('Paris, France', { bias: '45.70,4.80|45.80,4.90', fetchImpl }); + + assert.equal(Math.round(result.lat), 49, `got ${result.label}`); + assert.equal(fetchImpl.urls.length, 1); + // A POI alone still qualifies: "Main Street, Denver" names a street. + assert.equal(selectQualifiedFeature([cafe], 'Paris, France'), cafe); +}); + +test('if the unbiased request fails, the nearby whole-name match is still the answer', async () => { + const store = { + geometry: { type: 'Point', coordinates: [-104.94, 39.66] }, + properties: { osm_key: 'shop', name: 'Bed, Bath & Beyond', city: 'Denver', countrycode: 'US' }, + }; + const urls = []; + const fetchImpl = (url) => { + urls.push(url); + if (!new URL(url).searchParams.has('lat')) return Promise.resolve({ ok: false, status: 503 }); + return Promise.resolve({ ok: true, json: () => Promise.resolve({ features: [store] }) }); + }; + + const outcome = await geocodeKeylessWithOutcome('Bed, Bath & Beyond', { + bias: '39.60,-105.00|39.70,-104.90', + fetchImpl, + cache: new Map(), + }); + + assert.match(outcome.place.label, /Denver/); + assert.equal(outcome.answered, true); + assert.equal(urls.length, 2); +}); + 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'); @@ -500,91 +693,6 @@ function readGeocodingConsumer(file) { return files.map((name) => fs.readFileSync(path.join(ROOT, 'src', name), 'utf8')).join('\n'); } -test('"USA" and "UK" name their countries, and a trailing comma keeps the nearby answer', async () => { - const pubFirst = [PARIS_TEXAS_PUB, PARIS_TX]; - assert.equal(selectQualifiedFeature(pubFirst, 'Paris, Texas, USA'), PARIS_TX); - // "Paris," names no region: the camera-biased pass still answers. - const fetchImpl = photonPair([PARIS_TX], []); - const result = await geocodeKeyless('Paris,', { - bias: '33.60,-95.60|33.70,-95.50', - fetchImpl, - }); - assert.equal(Math.round(result.lat), 34, `got ${result.label}`); - assert.equal(fetchImpl.urls.length, 1); -}); - -test('a region match takes the place itself, not a place whose name starts with it', () => { - const station = { - geometry: { type: 'Point', coordinates: [4.86, 45.76] }, - properties: { - osm_key: 'railway', - name: 'Paris Gare de Lyon', - city: 'Lyon', - country: 'France', - countrycode: 'FR', - }, - }; - const paris = { - geometry: { type: 'Point', coordinates: [2.35, 48.86] }, - properties: { - osm_key: 'place', - name: 'Paris', - state: 'Île-de-France', - country: 'France', - countrycode: 'FR', - }, - }; - assert.equal(selectQualifiedFeature([station, paris], 'Paris, France'), paris); - assert.equal(selectQualifiedFeature([station], 'Paris, France'), null); -}); - -test('"UK" names the United Kingdom', () => { - const london = { - geometry: { type: 'Point', coordinates: [-0.13, 51.51] }, - properties: { - osm_key: 'place', - name: 'London', - country: 'United Kingdom', - countrycode: 'GB', - }, - }; - const ontario = { - geometry: { type: 'Point', coordinates: [-81.25, 42.98] }, - properties: { - osm_key: 'place', - name: 'London', - state: 'Ontario', - country: 'Canada', - countrycode: 'CA', - }, - }; - assert.equal(selectQualifiedFeature([ontario, london], 'London, UK'), london); -}); - -test('a nearby place whose own name has a comma still wins when nothing matches as place and region', async () => { - const store = (lon, lat, city) => ({ - geometry: { type: 'Point', coordinates: [lon, lat] }, - properties: { - osm_key: 'shop', - osm_value: 'houseware', - name: 'Bed, Bath & Beyond', - city, - countrycode: 'US', - }, - }); - const nearby = store(-104.94, 39.66, 'Denver'); - const fetchImpl = photonPair([nearby], [store(-73.99, 40.74, 'New York')]); - - const result = await geocodeKeyless('Bed, Bath & Beyond', { - bias: '39.60,-105.00|39.70,-104.90', - fetchImpl, - }); - - assert.match(result.label, /Denver/); - assert.equal(fetchImpl.urls.length, 2, 'the region checks ran first'); -}); - - test('wrapped view bias stays near the antimeridian and malformed coordinates are omitted', () => { for (const [bias, expected] of [['-20,170|-10,-171', 179.5], ['50,175|60,-165', -175], ['0,180|10,-180', 180], ['0,-180|10,180', 0]]) { const params = new URL(photonSearchUrl('island', { bias })).searchParams; @@ -594,30 +702,3 @@ test('wrapped view bias stays near the antimeridian and malformed coordinates ar assert.equal(new URL(photonSearchUrl('island', { bias })).searchParams.has('lon'), false); } }); - -test('the nearby whole-name fallback takes an exact name, not one that only starts with the query', async () => { - const place = (name, lon, lat, props) => ({ - geometry: { type: 'Point', coordinates: [lon, lat] }, - properties: { osm_key: 'place', name, ...props }, - }); - const nearby = [ - place('Springfield IL Auto', -89.6, 39.8, { - osm_key: 'shop', - city: 'Springfield', - state: 'Illinois', - countrycode: 'US', - }), - ]; - const anywhere = [ - place('Springfield', -89.65, 39.78, { state: 'Illinois', countrycode: 'US' }), - ]; - const fetchImpl = photonPair(nearby, anywhere); - - const result = await geocodeKeyless('Springfield, IL', { - bias: '39.70,-89.70|39.90,-89.50', - fetchImpl, - }); - - assert.doesNotMatch(result.label, /Auto/); - assert.match(result.label, /^Springfield/); -});