diff --git a/src/data/areaResolver.js b/src/data/areaResolver.js index 2d483cf80..0cbcd4894 100644 --- a/src/data/areaResolver.js +++ b/src/data/areaResolver.js @@ -150,10 +150,11 @@ function stableCandidateHash(value) { function candidateIdOf(admin) { const prefix = `ne:${admin.kind}:`; - const slug = normalizeAreaQuery(`${admin.country} ${admin.name}`).replace( - / /g, - '-', - ); + // The region code keeps same-named units apart: "Washington" is both WA + // and DC, which would otherwise share one candidate and one stored area. + const slug = normalizeAreaQuery( + `${admin.country} ${admin.name} ${admin.regionCode || ''}`, + ).replace(/ /g, '-'); const full = `${prefix}${slug}`; if (full.length <= MAX_AREA_CANDIDATE_ID_LENGTH) return full; const suffix = `-${stableCandidateHash(slug)}`; @@ -175,6 +176,10 @@ function describeCandidate(admin) { candidateId: candidateIdOf(admin), name: admin.name, level: ADMIN_LEVEL[admin.kind] || null, + // Tells same-named units apart when spoken: Washington WA vs DC. + ...(admin.regionCode && admin.kind !== 'country' + ? { region: String(admin.regionCode) } + : {}), ...(admin.country && admin.kind !== 'country' ? { country: admin.country } : {}), @@ -275,12 +280,24 @@ export function createAreaResolver({ async function lookup({ query, level, within }, { signal }) { const { level: levelHint } = splitLevelWord(query); const searchText = within ? `${query}, ${within}` : query; + let bundledRefusal = null; // Offline first: a named natural region, unless an admin level or a // country/state qualifier was given (the pack cannot check `within`). if ((!level || level === 'natural') && !within) { const ne = await findNaturalRegion(query, { signal }).catch(() => null); signal?.throwIfAborted?.(); - if (ne?.polygons?.length) { + // A region named like a country or state ("India" the peninsula, + // "Punjab" the desert) never hides it; asked for by level, it answers. + // Only a unit that really has this name counts: "Great Britain" finds + // the United Kingdom by alias, which is not a Great Britain. + const shadowsAdmin = + ne?.polygons?.length && + level !== 'natural' && + (await findAdminCandidates(query, { signal }).catch(() => [])).some( + (admin) => !bundledAdminIdentityRefusal(admin, query, null), + ); + signal?.throwIfAborted?.(); + if (ne?.polygons?.length && !shadowsAdmin) { const record = store.put({ geometry: ne.polygons.map((ring) => [ring]), name: ne.name, @@ -314,8 +331,14 @@ export function createAreaResolver({ places = checked .filter(({ refused }) => !refused) .map(({ admin }) => admin); - if (!places.length && checked.length) - return checked.find(({ refused }) => refused)?.refused; + if (!places.length && checked.length) { + const refused = checked.find((entry) => entry.refused)?.refused; + // A retired or unverifiable unit is final. A different unit that + // merely shares the name ("Azad Kashmir" for "Kashmir") is not an + // answer, so the outline ladder still gets to look. + if (refused?.code !== 'AREA_IDENTITY_MISMATCH') return refused; + bundledRefusal = refused; + } if (places.length === 1) { return storeAdmin(places[0], 0.95, identity); } @@ -354,11 +377,13 @@ export function createAreaResolver({ }); signal?.throwIfAborted?.(); if (!outline?.ok) - return { - ok: false, - code: outline?.code || 'AREA_NOT_FOUND', - error: outline?.error || `No boundary found for "${query}".`, - }; + return ( + bundledRefusal || { + ok: false, + code: outline?.code || 'AREA_NOT_FOUND', + error: outline?.error || `No boundary found for "${query}".`, + } + ); const record = store.put({ geometry: outline.polygons, name: outline.name || query, diff --git a/src/data/areaResolver.test.mjs b/src/data/areaResolver.test.mjs index f27dd8fbd..2e371ca87 100644 --- a/src/data/areaResolver.test.mjs +++ b/src/data/areaResolver.test.mjs @@ -623,3 +623,105 @@ test('a name that normalizes to nothing is never cached or shared', async () => await resolver.resolve({ query: '???' }); assert.equal(calls, 2, 'two punctuation-only asks are two lookups'); }); + +test('a natural region never hides a country or state of the same name', async () => { + const desert = { + name: 'Punjab', + kind: 'natural', + polygons: [square(70, 28, 76, 33)], + }; + const { resolver } = makeResolver({ + findNaturalRegion: async (q) => (/^punjab$/i.test(q) ? desert : null), + }); + const bare = await resolver.resolve({ query: 'Punjab' }); + assert.equal(bare.ok, false); + assert.equal(bare.code, 'AMBIGUOUS_AREA', 'the two states are asked about'); + const natural = await resolver.resolve({ query: 'Punjab', level: 'natural' }); + assert.equal(natural.ok, true); + assert.equal(natural.record.level, 'natural', 'asked for, the region wins'); +}); + +test('same-named states in one country get their own candidate and area', async () => { + const wa = admin( + 'state', + 'Washington', + 'United States of America', + [-124, 45.5, -117, 49], + { regionCode: 'WA' }, + ); + const dc = admin( + 'state', + 'Washington', + 'United States of America', + [-77.2, 38.8, -76.9, 39], + { regionCode: 'DC' }, + ); + const { resolver } = makeResolver({ + findAdminCandidates: async (q) => (/^washington$/i.test(q) ? [wa, dc] : []), + }); + const asked = await resolver.resolve({ query: 'Washington' }); + assert.equal(asked.code, 'AMBIGUOUS_AREA'); + const ids = asked.candidates.map((c) => c.candidateId); + assert.equal(new Set(ids).size, 2, `distinct ids: ${ids.join(', ')}`); + assert.deepEqual( + asked.candidates.map((c) => c.region), + ['WA', 'DC'], + 'the choices can be told apart', + ); + const picked = await Promise.all( + ids.map((candidateId) => resolver.resolve({ candidateId })), + ); + assert.ok(picked.every((p) => p.ok)); + assert.notEqual( + picked[0].record.areaId, + picked[1].record.areaId, + 'two areas, not one overwritten', + ); + assert.equal( + pointInPreparedArea(picked[0].record.prepared, 47, -120), + true, + 'Washington State', + ); + assert.equal( + pointInPreparedArea(picked[1].record.prepared, 38.9, -77), + true, + 'District of Columbia', + ); +}); + +test('a bundled name mismatch falls through to the outline ladder', async () => { + const azad = admin('state', 'Azad Kashmir', 'Pakistan', [73, 33, 75, 35.5]); + const kashmir = { + ok: true, + name: 'Kashmir', + polygons: [[square(73, 32, 80, 37)]], + source: 'osm', + rung: 'nominatim', + }; + let outlineOk = true; + const { resolver } = makeResolver({ + findAdminCandidates: async (q) => (/^kashmir$/i.test(q) ? [azad] : []), + resolveNamedOutline: async () => + outlineOk + ? kashmir + : { ok: false, code: 'AREA_NOT_FOUND', error: 'none' }, + }); + const found = await resolver.resolve({ query: 'Kashmir' }); + assert.equal(found.ok, true, found.error); + assert.equal(found.record.name, 'Kashmir'); + outlineOk = false; + const { resolver: offline } = makeResolver({ + findAdminCandidates: async () => [azad], + resolveNamedOutline: async () => ({ + ok: false, + code: 'AREA_NOT_FOUND', + error: 'none', + }), + }); + const refused = await offline.resolve({ query: 'Kashmir' }); + assert.equal( + refused.code, + 'AREA_IDENTITY_MISMATCH', + 'the bundled refusal is kept', + ); +}); diff --git a/src/voice/actionSchemas.test.mjs b/src/voice/actionSchemas.test.mjs index 6c12ba62a..165ed8737 100644 --- a/src/voice/actionSchemas.test.mjs +++ b/src/voice/actionSchemas.test.mjs @@ -103,7 +103,7 @@ test('every emitted area candidate round-trips through the production schema', a true, ); } - assert.equal(ambiguous.candidates[0].candidateId.length, 41); + assert.equal(ambiguous.candidates[0].candidateId.length, 44); const grammar = new RegExp(candidateSchema.pattern, 'u'); for (const malformed of [ 'ne:state:bad slug', diff --git a/src/voice/resultDisplay.js b/src/voice/resultDisplay.js index 5f4dd6006..3d44bf95e 100644 --- a/src/voice/resultDisplay.js +++ b/src/voice/resultDisplay.js @@ -80,7 +80,9 @@ function candidateLines(candidates) { ? t(LEVEL_WORDS[candidate.level]) : candidate.level : null; - const detail = [level, candidate?.country].filter(Boolean).join(', '); + const detail = [level, candidate?.region, candidate?.country] + .filter(Boolean) + .join(', '); return `${i + 1} · ${spokenLabel(candidate?.name || 'Unnamed', 40)}${detail ? ` — ${detail}` : ''}`; }); }