From a331616a9d31b40f7e4a642770c9ab55803b4d49 Mon Sep 17 00:00:00 2001 From: manjunath Date: Mon, 19 Oct 2026 11:01:18 +0530 Subject: [PATCH 1/3] fix(location): preserve geocoded places and region provenance --- docs/CURRENT-STATE.md | 8 + scripts/qa-voice-routing.mjs | 8 +- src/annotations/adminOutline.test.mjs | 152 +++++++++++++ src/annotations/regionRing.test.mjs | 106 +++++++++ src/annotations/resolver.js | 95 +++++++- src/data/analystEngine.js | 16 +- src/data/analystEngine.test.mjs | 303 ++++++++++++++++++++++++++ src/data/naturalEarthRegions.js | 41 ++-- src/data/naturalEarthRegions.test.mjs | 37 +++- src/locations.js | 64 +++++- src/locations.test.mjs | 202 +++++++++++++++++ 11 files changed, 999 insertions(+), 33 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 9562dc9f5..fa6d301e1 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,5 +1,13 @@ # God's Eye View Current State +Location search preserves successful geocoder results, including contemporary +places that share a Natural Earth name. After geocoding and nearby-place recovery +miss, a natural region can resolve from the current view only when exactly one +matching region contains it. Annotation outlines use the same containment rule +for view-based recovery. Analyst queries retain their offline unique-name lookup; +duplicate names use a geocoded anchor, and follow-ups retain boundary provenance. +Cancelled region lookups preserve the previous analyst cohort and boundary source. + Shared distance checks use `src/geoDistance.js` with `{ lat, lon }` points and the IUGG mean spherical Earth radius, 6371.0088 km. `greatCircleKm` is canonical; `greatCircleMeters` is its exact metre conversion. Missing, diff --git a/scripts/qa-voice-routing.mjs b/scripts/qa-voice-routing.mjs index f1b2b0ced..1c5c2eb83 100644 --- a/scripts/qa-voice-routing.mjs +++ b/scripts/qa-voice-routing.mjs @@ -1745,8 +1745,9 @@ async function runBehaviorLayer() { 'behavior: analyst follow-up re-filters the remembered set', `count=${r?.count} followUp=${r?.coverage?.followUp}`, ); - // A marine region resolves from the bundled Natural Earth pack in the page, - // without the slower geocode fallback. + // Require the bundled boundary source and the existing whole-query limit. + // This elapsed time includes record acquisition and filtering; the region + // resolver has its own separate fallback budget. const gulfStarted = Date.now(); r = await run('analyst_query', { layers: ['flights'], @@ -1757,9 +1758,10 @@ async function runBehaviorLayer() { report( r?.ok === true && String(r?.coverage?.scope || '').includes('Gulf of Mexico') && + r?.coverage?.scopeSource === 'natural-earth' && gulfMs < 3000, 'behavior: analyst resolves the Gulf of Mexico from the bundled pack', - `scope=${r?.coverage?.scope} ms=${gulfMs} err=${r?.error || ''}`, + `scope=${r?.coverage?.scope} source=${r?.coverage?.scopeSource} ms=${gulfMs} err=${r?.error || ''}`, ); // (5) zoom_to_globe is ABSOLUTE full-earth (>12,000 km band) diff --git a/src/annotations/adminOutline.test.mjs b/src/annotations/adminOutline.test.mjs index 29761a2e3..d3d1cc4b9 100644 --- a/src/annotations/adminOutline.test.mjs +++ b/src/annotations/adminOutline.test.mjs @@ -120,6 +120,123 @@ test('Georgia resolves from camera geography without a geocoder', async () => { assert.deepEqual(geocodes, []); }); +test('the Alps under the view resolve before an unrelated remote geocode', async () => { + const { resolve, lookups, geocodes } = harness({ + lat: -27.96, + lng: 152.2, + label: 'The Alps, Queensland', + name: 'The Alps', + types: ['locality', 'political'], + }); + const resolved = await resolve('the Alps', viewerAt(46.5, 10, 248_000)); + assert.equal(resolved?.source, 'bundled'); + assert.equal(resolved.naturalRegion, 'Alps'); + assert.equal(resolved.synthesized, false); + assert.equal(resolved.footprintKind, 'area'); + assert.ok(resolved.ring.length >= 8); + assert.deepEqual(resolved.ring[0], resolved.ring.at(-1)); + assert.equal(resolved.resolveOutline, undefined); + assert.deepEqual(geocodes, []); + assert.deepEqual(lookups, []); +}); + +test('Gold Coast in Australia cannot acquire its West African namesake outline', async () => { + for (const types of [['locality', 'political'], ['natural_feature']]) { + const { resolve, geocodes } = harness({ + lat: -28.0167, + lng: 153.4, + label: 'Gold Coast, Australia', + name: 'Gold Coast', + types, + }); + const resolved = await resolve('Gold Coast', viewerAt(-28.0167, 153.4)); + assert.equal(resolved?.source, 'geocode'); + assert.equal(resolved.lat, -28.0167); + assert.equal(resolved.lon, 153.4); + assert.equal(resolved.naturalRegion, undefined); + const outline = await resolved.resolveOutline(); + assert.equal(outline?.naturalRegion, undefined); + assert.deepEqual(geocodes, ['Gold Coast']); + } +}); + +test('unique natural names without a containing view still need a geocoded anchor', async () => { + for (const viewer of [AUSTIN, {}]) { + const miss = harness(); + assert.equal(await miss.resolve('the Alps', viewer), null); + assert.ok(miss.geocodes.includes('the Alps')); + } + const anchored = harness({ + lat: 46.5, + lng: 10, + label: 'Alps', + name: 'Alps', + types: ['natural_feature'], + }); + const resolved = await anchored.resolve('the Alps', {}); + assert.equal(resolved.source, 'geocode'); + const outline = await resolved.resolveOutline(); + assert.equal(outline.naturalRegion, 'Alps'); + assert.ok(outline.ring.length >= 8); +}); + +test('overlapping natural candidates continue through geocoding without choosing a ring', async () => { + const { resolve, geocodes } = harness({ + lat: 45.6336, + lng: -74.86493, + label: 'Canadian Shield', + name: 'Canadian Shield', + types: ['natural_feature'], + }); + const resolved = await resolve( + 'Canadian Shield', + viewerAt(45.6336, -74.86493), + ); + assert.equal(resolved?.source, 'geocode'); + assert.equal((await resolved.resolveOutline())?.naturalRegion, undefined); + assert.deepEqual(geocodes, ['Canadian Shield']); +}); + +test('natural view recovery preserves point, explicit-coordinate, entity and around intent', async () => { + for (const extra of [ + { footprint: false }, + { latitude: 46.5, longitude: 10 }, + { entityKind: 'building' }, + { entityKind: 'compound' }, + { entityKind: 'district' }, + { entityKind: 'street' }, + { entityKind: 'point_feature' }, + { intent: 'around_the_thing' }, + ]) { + const { resolve, geocodes } = harness({ + lat: 46.5, + lng: 10, + label: 'Alps', + name: 'Alps', + types: + extra.entityKind === 'district' ? ['locality'] : ['natural_feature'], + }); + const resolved = await resolve('the Alps', viewerAt(46.5, 10), extra); + assert.notEqual(resolved?.source, 'bundled'); + assert.equal(resolved?.naturalRegion, undefined); + if (extra.entityKind === 'district') + assert.ok(geocodes.includes('the Alps')); + if (extra.intent === 'around_the_thing' || extra.entityKind === 'district') + assert.equal((await resolved.resolveOutline())?.naturalRegion, undefined); + } +}); + +test('cancellation while a natural view lookup is pending cannot publish its outline', async () => { + const controller = new AbortController(); + const { resolve, geocodes } = harness(); + const pending = resolve('the Alps', viewerAt(46.5, 10), { + signal: controller.signal, + }); + controller.abort(); + await assert.rejects(pending, { name: 'AbortError' }); + assert.deepEqual(geocodes, []); +}); + test('country asks and geocoded country scopes use the bundled outline', async () => { const direct = harness(); for (const name of ['Switzerland', 'Iran', 'France', 'Japan']) { @@ -234,3 +351,38 @@ test('the engine keeps every part on the mark, drawn at once or upgraded', async ); engine.destroy(); }); + +test('cancellation after the admin miss rejects the resumed natural view lookup', async () => { + const controller = new AbortController(); + const signal = controller.signal; + const throwIfAborted = signal.throwIfAborted.bind(signal); + const checkpoints = []; + let abortCheckpoint = null; + // This exact path has entry, post-admin-miss and post-natural checkpoints. + // Queue at the known post-admin checkpoint: after this callback returns, + // the real natural lookup runs synchronously to its await before the queued + // abort executes. No timeout or guessed number of promise turns is used. + Object.defineProperty(signal, 'throwIfAborted', { + value() { + checkpoints.push(signal.aborted); + throwIfAborted(); + if (checkpoints.length === 2) { + queueMicrotask(() => { + abortCheckpoint = checkpoints.length; + controller.abort(); + }); + } + }, + }); + const { resolve, lookups, geocodes } = harness(); + const pending = resolve('the Alps', viewerAt(46.5, 10), { signal }); + await assert.rejects(pending, { name: 'AbortError' }); + assert.equal( + abortCheckpoint, + 2, + 'abort follows admin miss, before natural resume', + ); + assert.deepEqual(checkpoints, [false, false, true]); + assert.deepEqual(lookups, []); + assert.deepEqual(geocodes, []); +}); diff --git a/src/annotations/regionRing.test.mjs b/src/annotations/regionRing.test.mjs index cd8ad6447..b1a5ac524 100644 --- a/src/annotations/regionRing.test.mjs +++ b/src/annotations/regionRing.test.mjs @@ -44,10 +44,115 @@ test('region ring: Natural Earth names resolve without a geocoder', async () => for (const name of ['Gulf of Mexico', 'the Alps']) { const region = await resolveRegionRingForQuery(name); assert.ok(region?.ring?.length >= 3, `${name} resolves to a ring`); + assert.equal(region.source, 'natural-earth'); assert.equal(region.error, undefined); } }); +test('region ring: duplicate natural names need a containing geocoded anchor', async () => { + const { resolveRegionRingForQuery } = createAnnotationResolver({ + featureSource: featureSource(), + }); + const miss = geocoderFor(null); + assert.equal( + await resolveRegionRingForQuery('Cordillera Oriental', undefined, miss), + null, + ); + assert.deepEqual(miss.calls, ['Cordillera Oriental']); + + const inside = geocoderFor({ + lat: 5, + lng: -73, + label: 'Cordillera Oriental', + name: 'Cordillera Oriental', + types: ['natural_feature'], + }); + const region = await resolveRegionRingForQuery( + 'Cordillera Oriental', + undefined, + inside, + ); + assert.equal(region?.source, 'natural-earth'); + assert.equal(region.name, 'Cordillera Oriental'); + assert.ok(region.ring.length >= 8); + assert.deepEqual(inside.calls, ['Cordillera Oriental']); +}); + +test('region ring: outside and overlapping geocoded natural anchors stay unresolved', async () => { + const { resolveRegionRingForQuery } = createAnnotationResolver({ + featureSource: featureSource(), + }); + for (const [name, lat, lng] of [ + ['Cordillera Oriental', 30.27, -97.74], + ['Canadian Shield', 45.6336, -74.86493], + ]) { + const geocoder = geocoderFor({ + lat, + lng, + label: name, + name, + types: ['natural_feature'], + }); + assert.equal( + await resolveRegionRingForQuery(name, undefined, geocoder), + null, + ); + assert.deepEqual(geocoder.calls, [name]); + } +}); + +test('region ring: geocoded administrative scope wins over duplicate natural geography', async () => { + const lookups = []; + const { resolveRegionRingForQuery } = createAnnotationResolver({ + featureSource: featureSource({ + getAdministrativeAreas: async (point) => { + lookups.push(point); + return []; + }, + }), + }); + const geo = geocoderFor({ + lat: 5, + lng: -73, + label: 'Cordillera Oriental', + name: 'Cordillera Oriental', + types: ['locality'], + }); + const resolved = await resolveRegionRingForQuery( + 'Cordillera Oriental', + undefined, + geo, + ); + assert.equal( + resolved, + null, + 'an unavailable city boundary is not a natural-region outline', + ); + assert.deepEqual(lookups, [{ lat: 5, lon: -73 }]); + assert.deepEqual(geo.calls, ['Cordillera Oriental']); +}); + +test('region ring: cancellation during bundled lookup and geocode never returns a region', async () => { + const { resolveRegionRingForQuery } = createAnnotationResolver({ + featureSource: featureSource(), + }); + const controller = new AbortController(); + const pending = resolveRegionRingForQuery('the Alps', controller.signal); + controller.abort(); + await assert.rejects(pending, { name: 'AbortError' }); + + const later = new AbortController(); + await assert.rejects( + resolveRegionRingForQuery('Cordillera Oriental', later.signal, { + async geocode() { + later.abort(); + return { place: null }; + }, + }), + { name: 'AbortError' }, + ); +}); + test('region ring: a slow admin-boundary lookup returns region-timeout within the budget', async () => { let adminSignal; let release; @@ -119,6 +224,7 @@ test('region ring: bundled states and counties resolve without a geocoder', asyn ]) { const region = await resolveRegionRingForQuery(query, undefined, geocoder); assert.equal(region?.name, name, query); + assert.equal(region.source, 'bundled-admin'); assert.ok(region.ring.length >= 8, `${query} has a real ring`); } assert.deepEqual(geocoder.calls, [], 'no geocoding'); diff --git a/src/annotations/resolver.js b/src/annotations/resolver.js index 7d5f55d0d..ce22b44f2 100644 --- a/src/annotations/resolver.js +++ b/src/annotations/resolver.js @@ -202,6 +202,22 @@ export function createAnnotationResolver({ ); signal?.throwIfAborted(); if (admin) return bundledAdminTarget(viewer, admin, trace.query); + + // A unique pack name can still name an unrelated modern place. Recover + // natural outlines only from a containing view, including unique names. + if ( + center && + entityKind !== 'district' && + intent !== 'around_the_thing' + ) { + const natural = await lookupNaturalRegionOutline( + trace.query, + center.lat, + center.lon, + ).catch(() => null); + signal?.throwIfAborted(); + if (natural) return bundledNaturalTarget(viewer, natural, trace.query); + } } if (!Number.isFinite(lat) || !Number.isFinite(lon)) { @@ -444,6 +460,7 @@ export function createAnnotationResolver({ * AFTER the anchor mark is drawn and upgrade the mark in place. */ const resolveOutline = async () => { + signal?.throwIfAborted(); let scope = baseScope; const isAdmin = scope === 'country' || @@ -469,6 +486,7 @@ export function createAnnotationResolver({ const ne = await lookupNaturalRegionOutline(target, lat, lon).catch( () => null, ); + signal?.throwIfAborted(); if (ne) { registerDynamicCredit(viewer, NATURAL_EARTH_CREDIT); const neCentroid = ringCentroid(ne.ring); @@ -838,6 +856,37 @@ export function createAnnotationResolver({ }; } + /** A complete annotation target for a natural ring containing the view. */ + function bundledNaturalTarget(viewer, natural, query) { + const ring = closeRing([...natural.ring]); + const centroid = ringCentroid(ring); + if (!centroid) return null; + registerDynamicCredit(viewer, NATURAL_EARTH_CREDIT); + const lons = ring.map(([lon]) => lon); + const lats = ring.map(([, lat]) => lat); + console.log( + `[Resolver] "${query}": bundled ${natural.kind} "${natural.name}" ` + + '(natural-earth, view-contained) → FINAL source=bundled', + ); + return { + lon: centroid.lon, + lat: centroid.lat, + height: sampleGroundHeight(viewer, centroid.lon, centroid.lat), + ring, + footprintKind: 'area', + buildingHeight: null, + label: natural.name, + source: 'bundled', + synthesized: false, + outlineUnavailable: false, + naturalRegion: natural.name, + viewport: { + low: { latitude: Math.min(...lats), longitude: Math.min(...lons) }, + high: { latitude: Math.max(...lats), longitude: Math.max(...lons) }, + }, + }; + } + /** * Synthesize an approximate circular AREA by buffering a label point. Returns a ring * marked `synthesized:true` so the renderers draw it "approximate" (dashed/feathered), @@ -2004,8 +2053,8 @@ export function createAnnotationResolver({ /** * Region ring for ANALYST queries ("how many flights over Texas / the Alps") — * a name-only entry point that reuses this module's boundary machinery - * without the annotation pipeline. Natural Earth pack first (offline, - * instant; largest-area match is correct for a global name-only ask), then + * without the annotation pipeline. Unique Natural Earth names resolve first + * (offline); duplicate names require a geocoded containment anchor, then * geocode + admin boundary for states/countries/counties (Tier A disk-cached * Overpass, so repeat asks are instant). Returns null when the name doesn't * resolve to a region-like boundary — the analyst engine reports that @@ -2020,7 +2069,7 @@ export function createAnnotationResolver({ * @param {AbortSignal} [signal] * @param {object} [placeSearch] * @param {{budgetMs?: number}} [options] `Infinity` waits for the lookup. - * @returns {Promise<{name:string, ring:Array<[number,number]>}|{name:string, ring:null, error:'region-timeout'}|null>} + * @returns {Promise<{name:string, ring:Array<[number,number]>, source:string}|{name:string, ring:null, error:'region-timeout'}|null>} */ async function resolveRegionRingForQuery( name, @@ -2037,16 +2086,24 @@ export function createAnnotationResolver({ const q = String(name || '').trim(); if (!q) return null; const ne = await findNaturalRegion(q).catch(() => null); - if (ne?.polygons?.length) { + signal?.throwIfAborted(); + if (ne?.candidates === 1 && ne.polygons?.length) { // Largest ring carries the query scope; multi-ring regions (Andes) keep // their main cordillera — good enough for containment counting. const ring = [...ne.polygons].sort((a, b) => b.length - a.length)[0]; - if (ring?.length >= 3) return { name: ne.name, ring }; + if (ring?.length >= 3) + return { name: ne.name, ring, source: 'natural-earth' }; } // Bundled states/provinces/counties by name (offline); the main part only, // like the Natural Earth rung above. const admin = await findAdminArea(q).catch(() => null); - if (admin) return { name: admin.name, ring: [...admin.ring] }; + signal?.throwIfAborted(); + if (admin) + return { + name: admin.name, + ring: [...admin.ring], + source: 'bundled-admin', + }; const lookup = resolveAdminRegionRing(q, signal, placeSearch); if (!Number.isFinite(budgetMs)) return lookup; let timer; @@ -2067,8 +2124,26 @@ export function createAnnotationResolver({ const geo = await geocodePlace(q, null, signal, placeSearch).catch( () => null, ); + signal?.throwIfAborted(); if (!geo) return null; const scope = scopeFromTypes(geo.types); + if ( + scope === 'auto' || + (scope === 'compound' && geo.types?.includes('natural_feature')) + ) { + const natural = await lookupNaturalRegionOutline( + q, + geo.lat, + geo.lon, + ).catch(() => null); + signal?.throwIfAborted(); + if (natural) + return { + name: natural.name, + ring: natural.ring, + source: 'natural-earth', + }; + } if (!['country', 'state', 'county', 'city'].includes(scope)) return null; if (scope === 'state' || scope === 'county' || scope === 'country') { const admin = await findAdminAreaAt( @@ -2077,12 +2152,16 @@ export function createAnnotationResolver({ geo.lon, scope, ).catch(() => null); - if (admin) return { name: q, ring: [...admin.ring] }; + signal?.throwIfAborted(); + if (admin) + return { name: q, ring: [...admin.ring], source: 'bundled-admin' }; } const fp = await fetchAdminArea(geo.lat, geo.lon, q, scope, signal).catch( () => null, ); - if (fp?.ring?.length >= 3) return { name: q, ring: fp.ring }; + signal?.throwIfAborted(); + if (fp?.ring?.length >= 3) + return { name: q, ring: fp.ring, source: 'remote-boundary' }; return null; } diff --git a/src/data/analystEngine.js b/src/data/analystEngine.js index 6c3361aff..59a631421 100644 --- a/src/data/analystEngine.js +++ b/src/data/analystEngine.js @@ -881,6 +881,7 @@ export function createAnalystEngine(providers) { followUp && !spec.scope ? lastResult.distanceOf || null : null; let resolvedScope = null; let scopeNote = rememberedScope?.coverage || 'anywhere'; + let scopeSource = rememberedScope?.source || null; // Human phrasing for the same scope, so every spoken count can name what it // measured ("8 in view", "about 30 within 250 km of Austin") instead of // arriving as a bare number that contradicts the panel. @@ -890,8 +891,16 @@ export function createAnalystEngine(providers) { // further when a new scope is given. const scope = spec.scope || { kind: followUp ? 'anywhere' : 'view' }; if (scope.kind === 'region') { - const region = await providers.resolveRegionRing(scope.name, signal); - if (!isCurrent()) return cancelled(); + let region; + try { + region = await providers.resolveRegionRing(scope.name, signal); + } catch (error) { + // Cancellation belongs to the query owner, not an error's name: a + // resource-local AbortError from a current lookup still propagates. + if (signal?.aborted || !isCurrent()) return cancelled(); + throw error; + } + if (signal?.aborted || !isCurrent()) return cancelled(); if (region?.error === 'region-timeout') { return { ok: false, @@ -910,6 +919,7 @@ export function createAnalystEngine(providers) { } resolvedScope = region; scopeNote = `region:${region.name}`; + scopeSource = region.source || null; scopeLabel = `over ${region.name}`; scopeDetail = scopeLabel; } else if (scope.kind === 'radius') { @@ -1049,6 +1059,7 @@ export function createAnalystEngine(providers) { layersQueried, records: totals, scope: scopeNote, + ...(scopeSource ? { scopeSource } : {}), ...(rememberedScope?.note ? { note: rememberedScope.note } : {}), ...(queriedSnapshots.length ? { feedProvenance: feedProvenanceEnvelope(queriedSnapshots) } @@ -1081,6 +1092,7 @@ export function createAnalystEngine(providers) { label: memoryOverride?.scopeLabel || result.scopeLabel, detail: memoryOverride?.scopeDetail || result.display.scope, coverage: committedCoverage.scope, + source: committedCoverage.scopeSource || null, note: committedCoverage.note || null, }, }; diff --git a/src/data/analystEngine.test.mjs b/src/data/analystEngine.test.mjs index 8da85a4d6..b7aaa2a45 100644 --- a/src/data/analystEngine.test.mjs +++ b/src/data/analystEngine.test.mjs @@ -1,6 +1,8 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { createAnalystEngine, applyScope, haversineKm } from './analystEngine.js'; +import { createAnnotationResolver } from '../annotations/resolver.js'; +import { FEATURE_SOURCE_METHODS } from '../sources/featureSource.js'; // Stub world: a square "Texland" region, flights + ships + fires around it. const TEXLAND = { name: 'Texland', ring: [[-100, 28], [-94, 28], [-94, 33], [-100, 33]] }; @@ -131,6 +133,175 @@ test('analyst: count flights over a region', async () => { assert.ok(r.coverage.scope.includes('Texland')); }); +test('analyst: region coverage identifies the source of the boundary used for counting', async () => { + for (const source of ['natural-earth', 'bundled-admin', 'remote-boundary']) { + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: async () => ({ ...TEXLAND, source }), + }); + const result = await engine.query({ + layers: ['flights'], + scope: { kind: 'region', name: 'Texland' }, + }); + + assert.equal(result.ok, true); + assert.equal(result.count, 3, 'the boundary excludes the Oregon flight'); + assert.equal(result.coverage.scope, 'region:Texland'); + assert.equal(result.coverage.scopeSource, source); + } +}); + +test('analyst: filter-only follow-ups retain the region source without resolving again', async () => { + const lookups = []; + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: async (name) => { + lookups.push(name); + return { ...TEXLAND, source: 'natural-earth' }; + }, + }); + const first = await engine.query({ + layers: ['flights'], + scope: { kind: 'region', name: 'Texland' }, + }); + const airborne = await engine.query({ + followUp: true, + filters: [{ field: 'onGround', op: 'eq', value: false }], + }); + const military = await engine.query({ + followUp: true, + filters: [{ field: 'military', op: 'eq', value: true }], + }); + + assert.equal(first.count, 3); + assert.equal(airborne.count, 2); + assert.deepEqual(military.items.map((item) => item.id), ['RCH01']); + for (const result of [first, airborne, military]) { + assert.equal(result.coverage.scope, 'region:Texland'); + assert.equal(result.scopeLabel, 'over Texland'); + assert.equal(result.coverage.scopeSource, 'natural-earth'); + } + assert.deepEqual(lookups, ['Texland']); +}); + +test('analyst: an explicit replacement region replaces retained provenance', async () => { + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: async (name) => name === 'Texland' + ? { ...TEXLAND, source: 'natural-earth' } + : { + name, + ring: [[-98, 30.5], [-96, 30.5], [-96, 31.5], [-98, 31.5]], + source: 'remote-boundary', + }, + }); + await engine.query({ + layers: ['flights'], + scope: { kind: 'region', name: 'Texland' }, + }); + const replacement = await engine.query({ + followUp: true, + scope: { kind: 'region', name: 'Northern Texland' }, + }); + const followUp = await engine.query({ followUp: true }); + + for (const result of [replacement, followUp]) { + assert.deepEqual(result.items.map((item) => item.id), ['RCH01']); + assert.equal(result.coverage.scope, 'region:Northern Texland'); + assert.equal(result.coverage.scopeSource, 'remote-boundary'); + } +}); + +test('analyst: an explicit scope without provenance clears the retained region source', async () => { + for (const scope of [ + { kind: 'anywhere' }, + { kind: 'view' }, + { kind: 'radius', km: 150 }, + { kind: 'region', name: 'Uncredited Texland' }, + ]) { + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: async (name) => ({ + ...TEXLAND, + name, + ...(name === 'Texland' ? { source: 'natural-earth' } : {}), + }), + getViewContext: () => ({ lat: 30.27, lon: -97.74, viewRadiusKm: 150 }), + }); + const first = await engine.query({ + layers: ['flights'], + scope: { kind: 'region', name: 'Texland' }, + }); + const replacement = await engine.query({ followUp: true, scope }); + const followUp = await engine.query({ followUp: true }); + + assert.equal(first.coverage.scopeSource, 'natural-earth'); + for (const result of [replacement, followUp]) { + assert.equal(result.ok, true, scope.kind); + assert.equal(result.count, 3, scope.kind); + assert.equal(Object.hasOwn(result.coverage, 'scopeSource'), false, scope.kind); + assert.notEqual(result.coverage.scope, first.coverage.scope, scope.kind); + } + } +}); + +for (const [label, committedScope, scopeSource] of [ + ['replacement boundary source', 'region:Presented Texland', 'remote-boundary'], + ['window without a boundary source', 'window:presented', null], +]) { + test(`analyst: pre-commit displayed cohort retains ${label} and its feed provenance`, async () => { + const committedSnapshot = { + id: 'flights', enabled: true, feedState: 'stale', + source: 'displayed-cohort-feed', lastUpdate: 1_800_000_000_000, + }; + let reads = 0; + let lookups = 0; + const engine = createAnalystEngine({ + getRecords: () => { reads++; return FLIGHTS; }, + getLayerSnapshot: () => ({ + id: 'flights', enabled: true, feedState: 'nominal', source: 'current-feed', + }), + resolveRegionRing: async () => { + lookups++; + return { ...TEXLAND, source: 'natural-earth' }; + }, + }); + await engine.query( + { layers: ['flights'], scope: { kind: 'region', name: 'Texland' } }, + { + beforeCommit: async (result) => { + assert.equal(result.count, 3); + assert.equal(result.coverage.scopeSource, 'natural-earth'); + const { scopeSource: _originalSource, ...coverage } = result.coverage; + return { + matched: [{ key: 'flights', rows: [FLIGHTS[1]] }], + scopeLabel: 'the presented cohort', + scopeDetail: 'the presented cohort window', + coverage: { + ...coverage, + scope: committedScope, + ...(scopeSource ? { scopeSource } : {}), + feedProvenance: { layers: [committedSnapshot] }, + }, + }; + }, + }, + ); + const followUp = await engine.query({ followUp: true }); + assert.equal(followUp.count, 1); + assert.deepEqual(followUp.items.map((item) => item.id), ['RCH01']); + assert.equal(followUp.scopeLabel, 'the presented cohort'); + assert.equal(followUp.display.scope, 'the presented cohort window'); + assert.equal(followUp.coverage.scope, committedScope); + if (scopeSource) assert.equal(followUp.coverage.scopeSource, scopeSource); + else assert.equal(Object.hasOwn(followUp.coverage, 'scopeSource'), false); + assert.equal(followUp.coverage.feedProvenance.overall, 'stale'); + assert.deepEqual(followUp.coverage.feedProvenance.layers, [committedSnapshot]); + assert.equal(reads, 1, 'displayed cohort is not replaced by another feed read'); + assert.equal(lookups, 1, 'follow-up retains the committed scope without resolving again'); + }); +} + test('analyst: attribute filter — above 40,000 ft (~12,192 m)', async () => { const r = await makeEngine().query({ layers: ['flights'], scope: { kind: 'anywhere' }, @@ -317,6 +488,138 @@ test('analyst: region lookup receives and obeys the owning action signal', async assert.equal(result.cancelled, true); }); +for (const [label, reason, trackAuthority] of [ + ['default abort reason', undefined, false], + ['custom error reason', new Error('operator stopped'), false], + ['primitive reason and revoked authority', 'operator stopped', true], +]) { + test(`analyst: real region resolver cancellation preserves memory (${label})`, async () => { + const started = Promise.withResolvers(); + const geocode = Promise.withResolvers(); + const controller = new AbortController(); + const lookups = []; + const { resolveRegionRingForQuery } = createAnnotationResolver({ + featureSource: Object.fromEntries(FEATURE_SOURCE_METHODS.map((method) => [ + method, + async () => { throw new Error('offline region query reached feature source'); }, + ])), + }); + const placeSearch = { + geocode(name, { signal }) { + lookups.push(name); + assert.equal(signal, controller.signal); + started.resolve(); + return geocode.promise; + }, + }; + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: (name, signal) => resolveRegionRingForQuery( + name, signal, placeSearch, { budgetMs: Infinity }, + ), + }); + const first = await engine.query({ + layers: ['flights'], scope: { kind: 'region', name: 'Texas' }, + }); + assert.equal(first.ok, true); + assert.equal(first.coverage.scopeSource, 'bundled-admin'); + assert.deepEqual(first.items.map((item) => item.id), ['SWA1', 'RCH01', 'GND1']); + let commits = 0; + const pending = engine.query( + { layers: ['flights'], scope: { kind: 'region', name: 'Cordillera Oriental' } }, + { + signal: controller.signal, + ...(trackAuthority ? { isCurrent: () => !controller.signal.aborted } : {}), + beforeCommit: async () => { commits++; }, + }, + ); + await started.promise; + controller.abort(reason); + // Let an actual late geocode result cross the resolver's post-await check. + geocode.resolve({ place: { + lat: 5, lng: -73, label: 'Cordillera Oriental', + name: 'Cordillera Oriental', types: ['natural_feature'], + } }); + const result = await pending; + assert.equal(result.ok, false); + assert.equal(result.code, 'CANCELLED'); + assert.equal(result.cancelled, true); + assert.equal(commits, 0); + const followUp = await engine.query({ followUp: true }); + assert.deepEqual(followUp.items.map((item) => item.id), first.items.map((item) => item.id)); + assert.equal(followUp.coverage.scope, first.coverage.scope); + assert.equal(followUp.coverage.scopeSource, 'bundled-admin'); + assert.equal(followUp.scopeLabel, first.scopeLabel); + assert.deepEqual(lookups, ['Cordillera Oriental'], 'follow-up does not resolve again'); + }); +} + +test('analyst: a rejected superseded region lookup returns cancellation and retains its prior cohort', async () => { + const lookup = Promise.withResolvers(); + let current = true; + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: (name) => name === 'Texland' + ? { ...TEXLAND, source: 'natural-earth' } + : lookup.promise, + }); + const first = await engine.query({ layers: ['flights'], scope: { kind: 'region', name: 'Texland' } }); + const pending = engine.query( + { layers: ['flights'], scope: { kind: 'region', name: 'Replacement' } }, + { isCurrent: () => current }, + ); + current = false; + lookup.reject(new Error('late lookup failed')); + assert.equal((await pending).code, 'CANCELLED'); + const followUp = await engine.query({ followUp: true }); + assert.deepEqual(followUp.items.map((item) => item.id), first.items.map((item) => item.id)); + assert.equal(followUp.coverage.scopeSource, 'natural-earth'); + assert.equal(followUp.coverage.scope, first.coverage.scope); +}); + +test('analyst: an aborted owner cannot commit a region provider that returns a late success', async () => { + const controller = new AbortController(); + const lookup = Promise.withResolvers(); + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: () => lookup.promise, + }); + const pending = engine.query( + { layers: ['flights'], scope: { kind: 'region', name: 'Texland' } }, + { signal: controller.signal }, + ); + controller.abort(); + lookup.resolve({ ...TEXLAND, source: 'natural-earth' }); + assert.equal((await pending).code, 'CANCELLED'); + assert.equal(engine.hasMemory(), false); +}); + +for (const error of [new Error('live lookup failed'), new DOMException('resource stopped', 'AbortError')]) { + test(`analyst: live region ${error.name} propagates without changing memory`, async () => { + const controller = new AbortController(); + const engine = createAnalystEngine({ + getRecords: () => FLIGHTS, + resolveRegionRing: async (name) => { + if (name === 'Texland') return { ...TEXLAND, source: 'natural-earth' }; + throw error; + }, + }); + const first = await engine.query({ layers: ['flights'], scope: { kind: 'region', name: 'Texland' } }); + await assert.rejects( + engine.query( + { layers: ['flights'], scope: { kind: 'region', name: 'Replacement' } }, + { signal: controller.signal }, + ), + (thrown) => thrown === error, + ); + assert.equal(controller.signal.aborted, false); + const followUp = await engine.query({ followUp: true }); + assert.deepEqual(followUp.items.map((item) => item.id), first.items.map((item) => item.id)); + assert.equal(followUp.coverage.scope, first.coverage.scope); + assert.equal(followUp.coverage.scopeSource, 'natural-earth'); + }); +} + test('analyst: route fields queryable from cached enrichment only', async () => { const r = await makeEngine().query({ layers: ['flights'], scope: { kind: 'anywhere' }, diff --git a/src/data/naturalEarthRegions.js b/src/data/naturalEarthRegions.js index c40e180ac..a060d2260 100644 --- a/src/data/naturalEarthRegions.js +++ b/src/data/naturalEarthRegions.js @@ -189,7 +189,7 @@ const loadIndex = createRetryableLoader(async () => { return index; }); -function toResult(entry) { +function toResult(entry, candidates = 1) { return { name: entry.name, featurecla: entry.featurecla, @@ -197,6 +197,7 @@ function toResult(entry) { polygons: entry.polygons, bboxDiagonalKm: entry.bboxDiagonalKm, areaKm2: entry.areaKm2, + candidates, }; } @@ -209,7 +210,9 @@ function toResult(entry) { * @param {string} query e.g. "the Alps", "Rockies", "Gulf of Mexico" * @returns {Promise<{name:string, featurecla:string, kind:'natural'|'marine', * polygons:Array>, bboxDiagonalKm:number, - * areaKm2:number}|null>} largest-area match, or null when nothing matches. + * areaKm2:number, candidates:number}|null>} largest-area match plus the + * number of same-name entries, or null when nothing matches. Uniqueness in + * this pack alone does not establish the user's intended place. */ export async function findNaturalRegion(query) { const norm = normalizeName(query); @@ -223,7 +226,7 @@ export async function findNaturalRegion(query) { for (const key of candidates) { if (!key) continue; const list = index.get(key); - if (list && list.length) return toResult(list[0]); + if (list && list.length) return toResult(list[0], list.length); } return null; } @@ -270,9 +273,8 @@ export function pointInRing(ring, lat, lon) { /** * Resolve a natural-region OUTLINE ring for the annotation resolver's first * rung. Stricter than `findNaturalRegion`: walks ALL entries sharing the - * matched name (duplicate names included — the US and Spanish "Sierra - * Nevada" both exist upstream) and returns the single ring that CONTAINS the - * geocoded anchor. Containment is simultaneously the disambiguator and the + * matched name (duplicate names included) and returns a ring only when exactly + * one indexed entry CONTAINS the anchor. Containment is the disambiguator and the * wrong-place guard: when no ring contains the anchor this returns null and * the resolver's normal ladder continues unchanged. * @@ -292,24 +294,27 @@ export async function lookupNaturalRegionOutline(query, lat, lon) { ...suffixVariants(ALIASES[norm] || norm), ]; const seen = new Set(); + const containingEntries = new Map(); for (const key of candidates) { if (!key || seen.has(key)) continue; seen.add(key); for (const entry of index.get(key) || []) { - for (const ring of entry.polygons) { - if (pointInRing(ring, lat, lon)) { - return { - name: entry.name, - kind: entry.kind, - featurecla: entry.featurecla, - ring, - areaKm2: entry.areaKm2, - }; - } - } + if (containingEntries.has(entry)) continue; + const ring = entry.polygons.find((candidate) => + pointInRing(candidate, lat, lon), + ); + if (ring) containingEntries.set(entry, ring); } } - return null; + if (containingEntries.size !== 1) return null; + const [[entry, ring]] = containingEntries; + return { + name: entry.name, + kind: entry.kind, + featurecla: entry.featurecla, + ring, + areaKm2: entry.areaKm2, + }; } /** Resolve the smallest containing bundled physical region without a network geocoder. */ diff --git a/src/data/naturalEarthRegions.test.mjs b/src/data/naturalEarthRegions.test.mjs index 5dd78adea..9d58fa6e0 100644 --- a/src/data/naturalEarthRegions.test.mjs +++ b/src/data/naturalEarthRegions.test.mjs @@ -33,7 +33,10 @@ test('marquee ranges resolve with sane areas (owner acceptance: Alps + Rockies)' }); test('aliases and articles: "the Alps", "Rockies", "Sahara Desert", "Himalaya"', async () => { - assert.equal((await findNaturalRegion('the Alps'))?.name, 'Alps'); + const alps = await findNaturalRegion('the Alps'); + assert.equal(alps?.name, 'Alps'); + assert.equal(alps?.candidates, 1); + assert.equal((await findNaturalRegion('Alps mountains'))?.name, 'Alps'); assert.equal((await findNaturalRegion('Rockies'))?.name, 'Rocky Mountains'); assert.equal((await findNaturalRegion('the Rockies'))?.name, 'Rocky Mountains'); assert.equal((await findNaturalRegion('Sahara Desert'))?.name, 'Sahara'); @@ -41,6 +44,11 @@ test('aliases and articles: "the Alps", "Rockies", "Sahara Desert", "Himalaya"', assert.equal((await findNaturalRegion(' THE ALPS '))?.name, 'Alps'); }); +test('duplicate natural-region names expose ambiguity instead of claiming uniqueness', async () => { + assert.ok((await findNaturalRegion('Cordillera Oriental'))?.candidates > 1); + assert.ok((await findNaturalRegion('Canadian Shield'))?.candidates > 1); +}); + test('major deserts/ranges resolve with sane areas', async () => { const sahara = await findNaturalRegion('Sahara'); assert.ok(sahara && sahara.areaKm2 > 8_000_000 && sahara.areaKm2 < 12_000_000, @@ -134,12 +142,39 @@ test('outline lookup: duplicate names disambiguate by anchor containment', async const us = await lookupNaturalRegionOutline('Sierra Nevada', 37.2, -119.0); assert.ok(us, 'US anchor must match a Sierra Nevada'); assert.ok(pointInRing(us.ring, 37.2, -119.0), 'returned ring contains the US anchor'); + const colombia = await lookupNaturalRegionOutline('Cordillera Oriental', 5, -73); + assert.ok(colombia, 'a single containing candidate resolves a duplicate name'); + assert.ok(pointInRing(colombia.ring, 5, -73)); +}); + +test('outline lookup: overlapping duplicate candidates remain unresolved', async () => { + assert.equal(await lookupNaturalRegionOutline('Canadian Shield', 45.6336, -74.86493), null); }); test('outline lookup: anchor outside every ring → null (wrong-place guard)', async () => { assert.equal(await lookupNaturalRegionOutline('the Alps', 30.26, -97.77), null); }); +test('outline lookup: a unique physical-region name does not override an outside contemporary place', async () => { + for (const [name, lat, lon] of [ + ['Gold Coast', -28, 153.4], + ['Santa Cruz', 37, -122], + ['Isabela', 17, 122], + ['India', 28, 95], // Northeast India lies outside the physical peninsula. + ['Korea', 33.5, 126.5], // Jeju lies outside the physical peninsula. + ]) { + assert.equal((await findNaturalRegion(name))?.candidates, 1, `${name} has a unique physical namesake`); + assert.equal(await lookupNaturalRegionOutline(name, lat, lon), null, `${name} requires containment`); + } +}); + +test('outline lookup: aliases retain containment checks', async () => { + const alps = await lookupNaturalRegionOutline('Alps mountains', 46.5, 10); + assert.equal(alps?.name, 'Alps'); + assert.ok(pointInRing(alps.ring, 46.5, 10)); + assert.equal(await lookupNaturalRegionOutline('Alps mountains', -28, 153.4), null); +}); + test('outline lookup: non-region names never match', async () => { assert.equal(await lookupNaturalRegionOutline('Zilker Park', 30.26, -97.77), null); assert.equal(await lookupNaturalRegionOutline('Texas', 31.0, -99.0), null); diff --git a/src/locations.js b/src/locations.js index d17186905..80777dbbe 100644 --- a/src/locations.js +++ b/src/locations.js @@ -7,11 +7,13 @@ import { greatCircleKm } from './geoDistance.js'; import { viewportBias, placesNearViewRecovery, + pickWorldFromScreen, } from './annotations/annotationResolver.js'; import { unavailablePlaceSearch } from './search/placeSearch.js'; import { cancelCameraArrival } from './data/cameraArrival.js'; import { guardCameraAboveGround } from './cameraGroundGuard.js'; import { CITY_POIS } from './cityPois.js'; +import { lookupNaturalRegionOutline } from './data/naturalEarthRegions.js'; /** Newest landmark arrival; an older post-arrival ground guard yields to it. */ let arrivalGeneration = 0; @@ -299,6 +301,47 @@ export function findPoiByName(query) { /** Distinguishes an authority veto from a genuine not-found result. */ export const CANCELLED_SEARCH = Object.freeze({ cancelled: true }); +/** Frame a natural ring only after its name and view containment are resolved. */ +function naturalRegionSearchPlace(region) { + const points = region?.ring || []; + if (points.length < 3) return null; + const latitudes = points.map(([, lat]) => lat).filter(Number.isFinite); + const longitudes = points + .map(([lon]) => lon) + .filter(Number.isFinite) + .map((lon) => ((lon % 360) + 360) % 360) + .sort((a, b) => a - b); + if (latitudes.length < 3 || longitudes.length < 3) return null; + let gapIndex = longitudes.length - 1; + let largestGap = longitudes[0] + 360 - longitudes.at(-1); + for (let index = 0; index < longitudes.length - 1; index += 1) { + const gap = longitudes[index + 1] - longitudes[index]; + if (gap > largestGap) { + largestGap = gap; + gapIndex = index; + } + } + const viewport = { + southwest: { + lat: Math.min(...latitudes), + lng: wrapLongitude(longitudes[(gapIndex + 1) % longitudes.length]), + }, + northeast: { + lat: Math.max(...latitudes), + lng: wrapLongitude(longitudes[gapIndex]), + }, + }; + const metrics = viewportMetrics(viewport); + if (!metrics) return null; + return { + lat: metrics.centerLat, + lng: metrics.centerLng, + label: region.name, + types: ['natural_feature'], + viewport, + }; +} + /** * Geocode a place name through the supplied service, then fly there at a scale * appropriate to the request. Countries and cities use their viewport by @@ -344,7 +387,26 @@ export async function searchAndFlyTo(viewer, query, options = {}) { label = recovered.label || label; types = recovered.types || []; viewport = placesViewportToBounds(recovered.viewport) || viewport; - } else if (!result) return null; + } else if (!result) { + // A pack name may also name a contemporary city or country. Only rescue a + // missing geocode, and only with a ring containing what the user is viewing. + const carto = viewer?.camera?.positionCartographic; + const center = + pickWorldFromScreen(viewer, 0.5, 0.5) || + (carto && { + lat: Cesium.Math.toDegrees(carto.latitude), + lon: Cesium.Math.toDegrees(carto.longitude), + }); + const natural = + center && + (await lookupNaturalRegionOutline(query, center.lat, center.lon).catch( + () => null, + )); + signal?.throwIfAborted(); + const place = naturalRegionSearchPlace(natural); + if (!place) return null; + ({ lat, lng, label, types, viewport } = place); + } const requestedRange = finitePositive(options.range); const duration = finitePositive(options.duration) || 3.0; diff --git a/src/locations.test.mjs b/src/locations.test.mjs index c225488a3..65384a158 100644 --- a/src/locations.test.mjs +++ b/src/locations.test.mjs @@ -152,6 +152,208 @@ test('regionFramingPlan: E-W elongated region looks along the east axis', () => assert.equal(plan.headingDeg, 90); }); +// Synthetic service responses exercise collisions with the physical-region +// pack. These are authored fixtures, not captured geocoder responses. +for (const [query, label, lat, lng, type, bounds] of [ + ['Gold Coast', 'Gold Coast, Queensland, Australia', -28, 153.4, 'locality', [-28.2, 153.2, -27.8, 153.6]], + ['Santa Cruz', 'Santa Cruz, California, USA', 37, -122, 'locality', [36.9, -122.1, 37.1, -121.9]], + ['Isabela', 'Isabela, Philippines', 17, 122, 'administrative_area_level_1', [16.5, 121.5, 17.5, 122.5]], + ['India', 'India', 22, 79, 'country', [7, 67, 37, 91]], + ['Korea', 'South Korea', 36, 128, 'country', [33, 126, 39, 130]], +]) { + test(`search keeps the geocoded ${label} over its Natural Earth namesake`, async () => { + const viewer = stubViewer(); + const geocodes = []; + const result = await searchAndFlyTo(viewer, query, { + placeSearch: { + async geocode(request) { + geocodes.push(request); + return { place: { lat, lng, label, types: [type, 'political'], viewport: boxOf(...bounds) } }; + }, + }, + recoverNearView: async () => null, + }); + assert.deepEqual(geocodes, [query]); + assert.equal(result.label, label); + assert.equal(result.navigationMode, type === 'locality' ? 'city-overview' : 'region-overview'); + assert.equal(viewer.flights.length, 1); + const flown = flownRectangleDegrees(viewer); + assert.ok(Math.abs((flown.south + flown.north) / 2 - lat) < 0.000001, `${query}: latitude remains geocoded`); + assert.ok(Math.abs((flown.west + flown.east) / 2 - lng) < 0.000001, `${query}: longitude remains geocoded`); + assert.ok(flown.south <= bounds[0] && flown.north >= bounds[2], 'frames the geocoded north-south extent'); + assert.ok(flown.west <= bounds[1] && flown.east >= bounds[3], 'frames the geocoded east-west extent'); + assert.ok(flown.widthDeg < (bounds[3] - bounds[1]) * 1.6, 'keeps the geocoded place scale'); + }); +} + +function alpsViewer() { + const viewer = stubViewer(); + Object.assign(viewer.camera.positionCartographic, { + latitude: Cesium.Math.toRadians(46.5), + longitude: Cesium.Math.toRadians(10), + height: 248_000, + }); + return viewer; +} + +for (const answered of [true, false]) { + test(`search recovers contained Alps only after geocoder ${answered ? 'miss' : 'unavailability'} and nearby recovery miss`, async () => { + const viewer = alpsViewer(); + const order = []; + const result = await searchAndFlyTo(viewer, 'the Alps', { + placeSearch: { async geocode() { order.push('geocode'); return { place: null, answered }; } }, + recoverNearView: async () => { order.push('nearby'); return null; }, + }); + assert.deepEqual(order, ['geocode', 'nearby']); + assert.equal(result?.label, 'Alps'); + assert.equal(result?.navigationMode, 'natural-region-swath'); + assert.equal(viewer.flights.length, 1); + const target = Cesium.Cartographic.fromCartesian(viewer.flights[0].sphere.center); + assert.ok(Cesium.Math.toDegrees(target.latitude) > 43 && Cesium.Math.toDegrees(target.latitude) < 49); + assert.ok(Cesium.Math.toDegrees(target.longitude) > 5 && Cesium.Math.toDegrees(target.longitude) < 17); + }); +} + +test('a successful nearby recovery keeps precedence over a contained natural region', async () => { + const viewer = alpsViewer(); + const result = await searchAndFlyTo(viewer, 'the Alps', { + placeSearch: { async geocode() { return { place: null, answered: true }; } }, + recoverNearView: async () => ({ + lat: 46.6, + lon: 10.1, + label: 'The Alps local landmark', + types: ['locality', 'political'], + viewport: { low: { latitude: 46.5, longitude: 10 }, high: { latitude: 46.7, longitude: 10.2 } }, + }), + }); + assert.equal(result.label, 'The Alps local landmark'); + assert.equal(result.navigationMode, 'city-overview'); + assert.equal(viewer.flights.length, 1); +}); + +test('missing searches never choose a natural namesake outside the view or without a view anchor', async () => { + const withoutAnchor = alpsViewer(); + delete withoutAnchor.camera.positionCartographic; + for (const [query, viewer] of [['the Alps', stubViewer()], ['not a real location', alpsViewer()], ['the Alps', withoutAnchor]]) { + const result = await searchAndFlyTo(viewer, query, { + placeSearch: { async geocode() { return { place: null, answered: true }; } }, + recoverNearView: async () => null, + }); + assert.equal(result, null, query); + assert.equal(viewer.flights.length, 0, query); + } +}); + +test('a duplicate natural name can recover when exactly one candidate contains the view', async () => { + const viewer = stubViewer(); + Object.assign(viewer.camera.positionCartographic, { + latitude: Cesium.Math.toRadians(5), + longitude: Cesium.Math.toRadians(-73), + }); + const result = await searchAndFlyTo(viewer, 'Cordillera Oriental', { + placeSearch: { async geocode() { return { place: null, answered: true }; } }, + recoverNearView: async () => null, + }); + assert.equal(result?.label, 'Cordillera Oriental'); + assert.equal(result?.navigationMode, 'natural-region-swath'); + assert.equal(viewer.flights.length, 1); + const target = Cesium.Cartographic.fromCartesian(viewer.flights[0].sphere.center); + assert.ok(Cesium.Math.toDegrees(target.latitude) > 0, 'frames the northern candidate'); +}); + +test('an overlapping natural name stays unresolved after geocoder and nearby recovery miss', async () => { + const viewer = stubViewer(); + Object.assign(viewer.camera.positionCartographic, { + latitude: Cesium.Math.toRadians(45.6336), + longitude: Cesium.Math.toRadians(-74.86493), + }); + const result = await searchAndFlyTo(viewer, 'Canadian Shield', { + placeSearch: { async geocode() { return { place: null, answered: true }; } }, + recoverNearView: async () => null, + }); + assert.equal(result, null); + assert.equal(viewer.flights.length, 0); +}); + +test('a geocoder rejection propagates without natural-region recovery or a flight', async () => { + const viewer = alpsViewer(); + const failure = new Error('geocoder failed'); + let recoveries = 0; + await assert.rejects(searchAndFlyTo(viewer, 'the Alps', { + placeSearch: { async geocode() { throw failure; } }, + recoverNearView: async () => { recoveries += 1; return null; }, + }), (error) => error === failure); + assert.equal(recoveries, 0); + assert.equal(viewer.flights.length, 0); +}); + +test('search cancellation before or during geocoding cannot issue a fallback flight', async () => { + for (const preAborted of [true, false]) { + const viewer = alpsViewer(); + const controller = new AbortController(); + if (preAborted) controller.abort(); + let geocodes = 0; + let recoveries = 0; + await assert.rejects(searchAndFlyTo(viewer, 'the Alps', { + signal: controller.signal, + placeSearch: { + async geocode(_query, { signal }) { + geocodes += 1; + assert.equal(signal, controller.signal); + controller.abort(); + return { place: null, answered: true }; + }, + }, + recoverNearView: async () => { recoveries += 1; return null; }, + }), { name: 'AbortError' }); + assert.equal(geocodes, preAborted ? 0 : 1); + assert.equal(recoveries, 0); + assert.equal(viewer.flights.length, 0); + } +}); + +test('cancellation while the bundled fallback is resolving cannot issue a flight', async () => { + const viewer = alpsViewer(); + const carto = viewer.camera.positionCartographic; + const controller = new AbortController(); + let recoveryFinished = false; + let fallbackRead = false; + let authorityChecks = 0; + Object.defineProperty(viewer.camera, 'positionCartographic', { + get() { + if (recoveryFinished && !fallbackRead) { + fallbackRead = true; + // The fallback reads the view after nearby recovery, then awaits the + // bundled lookup. Abort in that async gap, even when the pack is warm. + queueMicrotask(() => controller.abort()); + } + return carto; + }, + }); + await assert.rejects(searchAndFlyTo(viewer, 'the Alps', { + signal: controller.signal, + placeSearch: { async geocode() { return { place: null, answered: true }; } }, + recoverNearView: async () => { recoveryFinished = true; return null; }, + beforeFly: () => { authorityChecks += 1; return true; }, + }), { name: 'AbortError' }); + assert.equal(fallbackRead, true, 'reached the bundled recovery'); + assert.equal(authorityChecks, 0); + assert.equal(viewer.flights.length, 0); +}); + +test('a final authority veto blocks a contained natural-region recovery flight', async () => { + const viewer = alpsViewer(); + const order = []; + const result = await searchAndFlyTo(viewer, 'the Alps', { + placeSearch: { async geocode() { order.push('geocode'); return { place: null, answered: true }; } }, + recoverNearView: async () => { order.push('nearby'); return null; }, + beforeFly: () => { order.push('veto'); return false; }, + }); + assert.deepEqual(order, ['geocode', 'nearby', 'veto']); + assert.equal(result, CANCELLED_SEARCH); + assert.equal(viewer.flights.length, 0); +}); + test('regionFramingPlan: park- and lake-scale viewports keep full framing', () => { // Zilker Park (~3 km). assert.equal(regionFramingPlan({ From 17631433371a61d5dc9bedda3f5b89f77d0011a9 Mon Sep 17 00:00:00 2001 From: manjunath Date: Mon, 19 Oct 2026 11:02:30 +0530 Subject: [PATCH 2/3] docs(changelog): record location resolution fix --- changelog.d/1235-location-resolution.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/1235-location-resolution.md diff --git a/changelog.d/1235-location-resolution.md b/changelog.d/1235-location-resolution.md new file mode 100644 index 000000000..9d67783c0 --- /dev/null +++ b/changelog.d/1235-location-resolution.md @@ -0,0 +1 @@ +- Preserve successful place-search matches, require an unambiguous containing natural region for recovery, and retain analyst boundary provenance through follow-ups (manjunath22466, #1235). From 803cddd5cb23e39161be4e1356d4d02057c341b9 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Tue, 20 Oct 2026 09:48:49 -0700 Subject: [PATCH 3/3] docs(analyst): describe region resolution as current state --- changelog.d/1235-location-resolution.md | 4 +++- docs/CURRENT-STATE.md | 17 ++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/changelog.d/1235-location-resolution.md b/changelog.d/1235-location-resolution.md index 85cb62cae..160688005 100644 --- a/changelog.d/1235-location-resolution.md +++ b/changelog.d/1235-location-resolution.md @@ -1 +1,3 @@ -- Require a geocoded containment anchor for ambiguous analyst region names, preserve boundary provenance and cancellation safety, and retain exact-name precedence for natural outlines (manjunath22466, #1235). +- Require a geocoded containment anchor for ambiguous analyst region names, + preserve boundary provenance and cancellation safety, and retain exact-name + precedence for natural outlines (manjunath22466, #1235). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 391b4b3d6..ee070b25f 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,14 +1,13 @@ # God's Eye View Current State -Analyst region queries retain the existing offline lookup for unique Natural -Earth names without geocoding: Gold Coast and Ivory Coast still name the West -African natural regions. Duplicate names require a geocoded containment anchor. -When multiple entries for the same name contain that anchor, the lookup now stays -unresolved instead of selecting the largest entry. Natural outlines try exact -names before aliases and broader suffix variants. Follow-ups retain boundary provenance, and -cancelled lookups preserve the previous analyst cohort and boundary source. -Navigation keeps main's existing successful-geocode precedence; this change adds -no search or view-based annotation recovery. +Analyst region queries resolve unique Natural Earth names offline without +geocoding, so Gold Coast and Ivory Coast name the West African natural regions. +Duplicate names require a geocoded containment anchor; when several entries for +the same name contain that anchor, the lookup stays unresolved rather than +selecting the largest entry. Natural outlines try exact names before aliases and +broader suffix variants. Follow-ups retain boundary provenance, and cancelled +lookups preserve the previous analyst cohort and boundary source. Navigation +search keeps a successful geocode, with no search or view-based region recovery. Shared distance checks use `src/geoDistance.js` with `{ lat, lon }` points and the IUGG mean spherical Earth radius, 6371.0088 km. `greatCircleKm` is