From 0712db989127962c2c41e8632de706b6e73a81b7 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Mon, 2 Nov 2026 09:03:36 -0800 Subject: [PATCH] address the final review of the voice outline fixes - A building word ("Burj Khalifa Tower", "Sagrada Familia Church") passes the last-word rule on a building-like result; enclosures stay out. - The anchor chooses only among the best-named outlines, so a worse name never wins by containment alone. - An ask without a quadrant takes only the quadrant of the matching street nearest the point; a leading quadrant ("SE Main St") may go unsaid too. - On an unbuffered tile edge a fragment shows as an edge along the boundary, so a whole building touching it at a corner is not cut. - Drop two orphaned comments and correct the place-name key wording. --- changelog.d/842-voice-outlines.md | 7 +- docs/CURRENT-STATE.md | 2 +- server/providers/regional/outlineSelect.js | 61 ++++++++++--- server/providers/regional/place.js | 6 +- src/sources/openFreeMapOutlines.js | 85 ++++++++++++++----- src/sources/openFreeMapOutlines.test.mjs | 30 +++++++ src/sources/openFreeMapOutlinesEdges.test.mjs | 11 +++ src/tooling/nominatimOutlineRoute.test.mjs | 34 ++++++++ 8 files changed, 194 insertions(+), 42 deletions(-) diff --git a/changelog.d/842-voice-outlines.md b/changelog.d/842-voice-outlines.md index ca8457d8e..b53460e0b 100644 --- a/changelog.d/842-voice-outlines.md +++ b/changelog.d/842-voice-outlines.md @@ -42,9 +42,10 @@ pick the same-name Nominatim outline that holds the resolved place, and keep a new outline when an identical older mark fails to clean up (samehkhamis, #842). -- Tighten the remaining voice outline review items: one place-name key for - server and tiles ("St. Paul" = "Saint Paul"), "E Street" no longer matching - "East Street" while "E St NW" answers "E Street", no neighbour standing in +- Tighten the remaining voice outline review items: Nominatim name checks use + the bundled packs' place-name key ("St. Paul" = "Saint Paul"), "E Street" no + longer matches "East Street" while "E St NW" answers "E Street" near it, + "Burj Khalifa Tower" still finds the tower, no neighbour standing in for a building cut at the tile edge, no cut area answering grounds, https-only PHOTON_URL, an interrupted Nominatim dispatch reclaimed after ten minutes, no node:sqlite startup warning, and lazily decoded shared OpenFreeMap layers diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index fc62c0abe..438c69c43 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5466,7 +5466,7 @@ request viewbox and continues searching without that geographic preference. - Runtime entry: `src/main.js` calls `initAnnotations({ viewer, tileset })`, exposes `window.__gevAnnotations`, and passes the engine into the voice action runner. - Engine contract: `src/annotations/annotationEngine.js` owns annotation state, TTL/fade lifecycle, concurrent anchor resolution, duplicate detection keyed on geometry, cancellation on clear/newer generations, and a hard cap of 120 live marks. Deferred outline upgrades drain FIFO at concurrency 2; queued work retains the owning abort controller and is discarded on a generation change before it can fetch. -- Resolver contract: `src/annotations/resolver.js` converts names, coordinates and screen pixels into world anchors and optional geometry. Google Geocode/Places supplies a centroid and scope; route requests use `/api/route`; ambiguous or distant results are rejected or recovered near the current view instead of drawing misleading blobs. Outline resolution keeps this order: bundled admin/place/neighborhood/Natural Earth geometry; OpenFreeMap street or pointed-building geometry where applicable; the configured OSM/Overpass admin/place/footprint/street/enclosing-area ladder; guarded Nominatim for eligible named cities, admin units, landmarks, buildings and grounds; then an OpenFreeMap building fallback that accepts a named building only when its resolved anchor lies in the selected filled footprint. Pointed-at building requests retain nearby selection within 40 m. Tile building and grounds answers also need the tile holding the anchor: when that tile fails to load and only neighbours arrive, no neighbouring building or area stands in. Tile grounds answer a named grounds ask only from an area of the same name, or, when the ask's head noun names a kind of grounds (school, university/college, hospital, cemetery, stadium, zoo; "campus" alone names none), from the part of the smallest unnamed area of that class that holds the anchor (a class word elsewhere in a name, as in "University Avenue Park", does not count); a differently named area never answers. If no rung can supply honest geometry, the mark remains a point or reports the capability unavailable. A real polygon, definitive miss, configured-ladder approximation, rate limit or transient result keeps its existing finality; Nominatim is only tried after higher-priority configured sources cannot answer. Building tile selection returns the selected component and its holes; a shared vector feature ID does not establish that unrelated components are one building. Other trusted multi-part outlines and holes are retained in `anno.polygons`, with `ring` anchored to the part whose filled area contains the resolved point, outside its holes, when possible. Guarded Nominatim keeps every accepted result for an ask, ranks an exact name first, then a name holding every asked word, then a partial match; a partial landmark match whose name lacks the ask's last word is refused unless that word appears in the result's address or type (the place around a feature, such as a park around its zoo, is not the feature, while "Space Needle Seattle" still finds the Space Needle). Each ask is answered with the cached result whose filled area holds the resolved anchor (read across the antimeridian), else the first. Server and tiles fold place names with one key, so "St. Paul" and "Saint Paul" agree. A street ribbon that holds neither its ring mean nor the anchor is centred on the street's nearest point to the anchor when the street runs within 100 m of it. In tile street names a lone letter is a direction only beside another name word ("N Lamar Blvd"), so "E Street" is not "East Street", and a trailing quadrant ("E St NW") may go unsaid in the ask. A building or area cut at the tile edge (with or without a tile buffer) is not complete: a cut building nearer the point than any whole one leaves the point, and a cut area does not answer grounds. Missing capability never invents a radius outline. Only explicitly country/state/county-scoped asks bypass near-view recovery and proximity gating; state scope requires a leading `state of …`/`the state of …` phrase, while bare names, proper names ending in “State,” and administrative geocode result types alone remain guarded. Overpass throttles remain distinct from normal transients: `Retry-After` is honored for one retry, and a repeated throttle ends only that mark's outline upgrade. +- Resolver contract: `src/annotations/resolver.js` converts names, coordinates and screen pixels into world anchors and optional geometry. Google Geocode/Places supplies a centroid and scope; route requests use `/api/route`; ambiguous or distant results are rejected or recovered near the current view instead of drawing misleading blobs. Outline resolution keeps this order: bundled admin/place/neighborhood/Natural Earth geometry; OpenFreeMap street or pointed-building geometry where applicable; the configured OSM/Overpass admin/place/footprint/street/enclosing-area ladder; guarded Nominatim for eligible named cities, admin units, landmarks, buildings and grounds; then an OpenFreeMap building fallback that accepts a named building only when its resolved anchor lies in the selected filled footprint. Pointed-at building requests retain nearby selection within 40 m. Tile building and grounds answers also need the tile holding the anchor: when that tile fails to load and only neighbours arrive, no neighbouring building or area stands in. Tile grounds answer a named grounds ask only from an area of the same name, or, when the ask's head noun names a kind of grounds (school, university/college, hospital, cemetery, stadium, zoo; "campus" alone names none), from the part of the smallest unnamed area of that class that holds the anchor (a class word elsewhere in a name, as in "University Avenue Park", does not count); a differently named area never answers. If no rung can supply honest geometry, the mark remains a point or reports the capability unavailable. A real polygon, definitive miss, configured-ladder approximation, rate limit or transient result keeps its existing finality; Nominatim is only tried after higher-priority configured sources cannot answer. Building tile selection returns the selected component and its holes; a shared vector feature ID does not establish that unrelated components are one building. Other trusted multi-part outlines and holes are retained in `anno.polygons`, with `ring` anchored to the part whose filled area contains the resolved point, outside its holes, when possible. Guarded Nominatim keeps every accepted result for an ask, ranks an exact name first, then a name holding every asked word, then a partial match; a partial landmark match whose name lacks the ask's last word is refused unless that word appears in the result's address or type (the place around a feature, such as a park around its zoo, is not the feature, while "Space Needle Seattle" still finds the Space Needle). A building word ("Burj Khalifa Tower") also passes on a building-like result. Each ask is answered with the cached result among the best-named ones whose filled area holds the resolved anchor (read across the antimeridian), else the first; a worse name never wins by containment alone. Nominatim name checks use the bundled packs' place-name key, so "St. Paul" and "Saint Paul" agree there. A street ribbon that holds neither its ring mean nor the anchor is centred on the street's nearest point to the anchor when the street runs within 100 m of it. In tile street names a lone letter is a direction only beside another name word ("N Lamar Blvd"), so "E Street" is not "East Street", and a quadrant ("E St NW", "SE Main St") may go unsaid in the ask, which then takes only the quadrant of the matching street nearest the point. A building or area cut at the tile edge is not complete (past a tile buffer any vertex on the cut line marks it; on an unbuffered edge, an edge running along the boundary does, not a corner that merely touches it): a cut building nearer the point than any whole one leaves the point, and a cut area does not answer grounds. Missing capability never invents a radius outline. Only explicitly country/state/county-scoped asks bypass near-view recovery and proximity gating; state scope requires a leading `state of …`/`the state of …` phrase, while bare names, proper names ending in “State,” and administrative geocode result types alone remain guarded. Overpass throttles remain distinct from normal transients: `Retry-After` is honored for one retry, and a repeated throttle ends only that mark's outline upgrade. - With no detailed-feature capability, annotations retain their pins and say "Detailed outline unavailable"; location focus reports the same result without retrying. Missing capability never synthesizes an outline from a radius. diff --git a/server/providers/regional/outlineSelect.js b/server/providers/regional/outlineSelect.js index 635bb4006..1fb0dd0ee 100644 --- a/server/providers/regional/outlineSelect.js +++ b/server/providers/regional/outlineSelect.js @@ -234,8 +234,6 @@ function adminLevelFits(kind, row) { /** Largest footprint (m²) a non-`building` result may have for a building ask. */ const BUILDING_MAX_AREA_M2 = 150_000; -/** Approximate area (m²) of a [lon, lat] ring. */ - /** Whether a result's class/type suits the ask kind. */ export function acceptsOutlineClass(kind, osmClass, osmType) { const rules = OUTLINE_ACCEPT[kind]; @@ -259,6 +257,36 @@ function queryWords(query) { .filter((word) => word && !STOP_WORDS.has(word)); } +/** + * Words naming what a building is, which OSM rarely carries in the name or + * type ("Burj Khalifa", building=yes). Enclosures (zoo, garden, arch, + * conservatory) are left out: those name a feature inside a park. + */ +const BUILDING_WORDS = [ + 'building', + 'tower', + 'church', + 'cathedral', + 'basilica', + 'chapel', + 'temple', + 'mosque', + 'synagogue', + 'shrine', + 'museum', + 'palace', + 'castle', + 'hall', + 'library', +]; +const BUILDING_LIKE_CLASSES = new Set([ + 'building', + 'amenity', + 'man_made', + 'tourism', + 'historic', +]); + /** How well a result's names answer the ask. */ const NAME_MISS = 0; const NAME_OVERLAP = 1; @@ -293,7 +321,8 @@ function outlineNameMatch(row, query, kind) { const unspaced = UNSPACED_SCRIPT_RE.test(whole); // The last word usually names the feature ("Zoo", "Arch"): a name without // it is the place around the feature ("Central Park"), unless the word is - // where the result is ("Space Needle Seattle") or what it is ("… Bridge"). + // where the result is ("Space Needle Seattle"), what it is ("… Bridge"), or + // a building word on a building-like result ("Burj Khalifa Tower"). const context = new Set( normalizePlaceName( [ @@ -308,6 +337,8 @@ function outlineNameMatch(row, query, kind) { .replace(/_/g, ' '), ).split(' '), ); + if (BUILDING_LIKE_CLASSES.has(String(row?.category || row?.class))) + for (const word of BUILDING_WORDS) context.add(word); let match = NAME_MISS; for (const name of names) { if ( @@ -356,8 +387,6 @@ function cleanRing(ring) { return out; } -/** Planar ring area in squared degrees (only used to rank parts). */ - /** Polygon/MultiPolygon GeoJSON → `[[outer, ...holes], …]`, largest first. */ function polygonsFromGeoJson(geometry) { const raw = @@ -565,21 +594,29 @@ export function selectOutlineResult(rows, { kind, query }) { }); } // Stable: equal matches keep upstream order. - const outlines = found - .sort((a, b) => b.nameMatch - a.nameMatch) - .map((entry) => entry.outline); - return { outline: outlines[0] || null, outlines, skipped }; + found.sort((a, b) => b.nameMatch - a.nameMatch); + const outlines = found.map((entry) => entry.outline); + const ranks = found.map((entry) => entry.nameMatch); + return { outline: outlines[0] || null, outlines, ranks, skipped }; } /** - * The outline holding `lat, lon` (the resolved anchor), else the first. + * Among the best-named outlines (`ranks` from selectOutlineResult; all of + * them without), the one holding `lat, lon` (the resolved anchor), else the + * first. A worse name never wins by containment alone: an exact building + * that misses a pin by metres is not replaced by the plaza around it. * Without an anchor the first answers, as before. */ -export function outlineAt(outlines, lat, lon) { +export function outlineAt(outlines, lat, lon, ranks = null) { const list = Array.isArray(outlines) ? outlines : []; if (!finiteLat(lat) || !finiteLon(lon)) return list[0] || null; + const best = Array.isArray(ranks) ? ranks[0] : undefined; return ( - list.find((outline) => containsAnchor(outline.polygons, lat, lon)) || + list.find( + (outline, index) => + (best === undefined || ranks[index] === best) && + containsAnchor(outline.polygons, lat, lon), + ) || list[0] || null ); diff --git a/server/providers/regional/place.js b/server/providers/regional/place.js index 82768ca75..aa5ce2c2e 100644 --- a/server/providers/regional/place.js +++ b/server/providers/regional/place.js @@ -242,7 +242,7 @@ export function createNominatimOutlineProvider({ const answer = (payload, anchorLat, anchorLon, cached) => ({ status: payload.status, - outline: outlineAt(payload.outlines, anchorLat, anchorLon), + outline: outlineAt(payload.outlines, anchorLat, anchorLon, payload.ranks), ...(payload.skipped ? { skipped: payload.skipped } : {}), ...(cached ? { cached: true } : {}), }); @@ -277,12 +277,12 @@ export function createNominatimOutlineProvider({ { signal: sharedSignal, maxBytes: OUTLINE_MAX_RESPONSE_BYTES }, ); if (!Array.isArray(rows)) throw malformed(); - const { outlines, skipped } = selectOutlineResult(rows, { + const { outlines, ranks, skipped } = selectOutlineResult(rows, { kind, query: sent, }); const found = outlines.length - ? { status: 'OK', outlines } + ? { status: 'OK', outlines, ranks } : { status: 'ZERO_RESULTS', outlines: [], skipped }; await cache.set( cacheKey, diff --git a/src/sources/openFreeMapOutlines.js b/src/sources/openFreeMapOutlines.js index 17187134a..ee226f246 100644 --- a/src/sources/openFreeMapOutlines.js +++ b/src/sources/openFreeMapOutlines.js @@ -204,14 +204,28 @@ function decodePolygons( } const [westCut, northCut] = pixelToLonLat(minX, minY, extent, z, x, y); const [eastCut, southCut] = pixelToLonLat(maxX, maxY, extent, z, x, y); - // A side with no buffer was cut at the tile edge itself, so a polygon that - // touches it is a fragment too. + // Past a buffer, any vertex on the cut line marks a fragment. A side with + // no buffer was cut at the tile edge itself, where a whole building may + // also just touch the edge: there a fragment shows as an edge running + // along the boundary, not a single vertex on it. const eps = 1e-9; - const onCut = ([lon, lat]) => - Math.abs(lon - westCut) < eps || - Math.abs(lon - eastCut) < eps || - Math.abs(lat - northCut) < eps || - Math.abs(lat - southCut) < eps; + const near = (a, b) => Math.abs(a - b) < eps; + const onBufferedCut = ([lon, lat]) => + (minX < 0 && near(lon, westCut)) || + (maxX > extent && near(lon, eastCut)) || + (minY < 0 && near(lat, northCut)) || + (maxY > extent && near(lat, southCut)); + const alongEdge = ([lonA, latA], [lonB, latB]) => + (minX === 0 && near(lonA, westCut) && near(lonB, westCut)) || + (maxX === extent && near(lonA, eastCut) && near(lonB, eastCut)) || + (minY === 0 && near(latA, northCut) && near(latB, northCut)) || + (maxY === extent && near(latA, southCut) && near(latB, southCut)); + const isCut = (ring) => + ring.some( + (point, index) => + onBufferedCut(point) || + (index > 0 && alongEdge(ring[index - 1], point)), + ); const out = []; for (let i = 0; i < layer.length; i++) { const feature = layer.feature(i); @@ -245,7 +259,7 @@ function decodePolygons( ), ); const clippedParts = polygons.map( - (rings, part) => incompleteParts[part] || rings[0].some(onCut), + (rings, part) => incompleteParts[part] || isCut(rings[0]), ); for (let part = 0; part < polygons.length; part++) { const rings = polygons[part]; @@ -364,14 +378,24 @@ export function streetNameMatches(candidate, asked) { }; const have = normalizeStreetName(candidate); if (answers(have)) return true; + const quadrant = streetQuadrant(have); + if (!quadrant || streetQuadrant(want)) return false; const words = have.split(' '); - return ( - QUADRANTS.has(words.at(-1)) && - !QUADRANTS.has(want.split(' ').at(-1)) && - answers(words.slice(0, -1).join(' ')) + return answers( + (quadrant.leading ? words.slice(1) : words.slice(0, -1)).join(' '), ); } +/** A normalized street name's quadrant ("e street northwest", "southeast main street"). */ +function streetQuadrant(name) { + const words = name.split(' '); + if (words.length < 2) return null; + if (QUADRANTS.has(words.at(-1))) + return { name: words.at(-1), leading: false }; + if (QUADRANTS.has(words[0])) return { name: words[0], leading: true }; + return null; +} + const lengthM = (line) => { let total = 0; for (let i = 1; i < line.length; i++) @@ -399,23 +423,38 @@ export function streetFromTiles( const pieces = []; let matchedName = null; const extent = boxAround(lat, lon, radiusM); + const matches = []; for (const tile of tiles || []) { for (const street of tile?.streets || []) { - if ( - !streetNameMatches(street.name, name) && - !(street.nameEn && streetNameMatches(street.nameEn, name)) - ) - continue; + const spelled = streetNameMatches(street.name, name) + ? street.name + : street.nameEn && streetNameMatches(street.nameEn, name) + ? street.nameEn + : null; + if (!spelled) continue; // Segment distance, not vertex distance: a long straight piece through // the point has no vertex near it. - if (distanceToLineM(lon, lat, street.coordinates) > radiusM) continue; - // Only what lies within the search extent is drawn. - const clipped = clipTileLine(street.coordinates, extent); - if (!clipped.length) continue; - matchedName ||= street.name; - pieces.push(...clipped); + const distanceM = distanceToLineM(lon, lat, street.coordinates); + if (distanceM > radiusM) continue; + const quadrant = + streetQuadrant(normalizeStreetName(spelled))?.name || null; + matches.push({ street, distanceM, quadrant }); } } + // An ask without a quadrant ("E Street" where E St NW and E St NE both + // run) means the one beside the point, not every quadrant's street. + const nearest = matches.reduce( + (best, match) => (!best || match.distanceM < best.distanceM ? match : best), + null, + ); + for (const { street, quadrant } of matches) { + if (quadrant !== nearest.quadrant) continue; + // Only what lies within the search extent is drawn. + const clipped = clipTileLine(street.coordinates, extent); + if (!clipped.length) continue; + matchedName ||= street.name; + pieces.push(...clipped); + } if (!pieces.length) return null; const chains = stitchLines( pieces.filter((piece) => piece.length >= 2), diff --git a/src/sources/openFreeMapOutlines.test.mjs b/src/sources/openFreeMapOutlines.test.mjs index 5ee5ddd7a..a81522267 100644 --- a/src/sources/openFreeMapOutlines.test.mjs +++ b/src/sources/openFreeMapOutlines.test.mjs @@ -84,6 +84,36 @@ test('a lone letter is a direction only beside a name; quadrants may go unsaid', assert.ok(!streetNameMatches('E St NE', 'E Street Northwest')); assert.ok(!streetNameMatches('E Street', 'E Street Northwest')); assert.ok(!streetNameMatches('F Street Northwest', 'E Street')); + // A leading quadrant may go unsaid too (Portland's "SE Main St"). + assert.ok(streetNameMatches('SE Main St', 'Main Street')); + assert.ok(!streetNameMatches('SE Main St', 'NE Main Street')); +}); + +test('an ask without a quadrant takes the quadrant beside the point', () => { + const street = (name, lat) => ({ + name, + coordinates: [ + [-77.03, lat], + [-77.02, lat], + ], + }); + const tiles = [ + { + streets: [ + street('E Street Northwest', 38.8962), + street('E Street Northeast', 38.8972), + street('E Street Southeast', 38.8822), + ], + }, + ]; + const near = streetFromTiles(tiles, { + name: 'E Street', + lat: 38.8963, + lon: -77.025, + }); + assert.equal(near.name, 'E Street Northwest'); + assert.equal(near.lines.length, 1, 'one quadrant, not three streets'); + assert.ok(near.lines[0].every(([, lat]) => lat === 38.8962)); }); test('same-name segments merge into polylines around the point (Congress Avenue)', () => { diff --git a/src/sources/openFreeMapOutlinesEdges.test.mjs b/src/sources/openFreeMapOutlinesEdges.test.mjs index 842768bf6..f4da0ac17 100644 --- a/src/sources/openFreeMapOutlinesEdges.test.mjs +++ b/src/sources/openFreeMapOutlinesEdges.test.mjs @@ -217,6 +217,17 @@ test('grounds touching an unbuffered tile edge are cut, not complete', () => { const inside = decode([square(-179.999, 0, 0.01)]); assert.equal(inside.areas[0].clipped, false); assert.ok(enclosingAreaFromTiles([inside], ask)); + // A whole shape that only touches the edge at one corner is not cut. + const tangent = decode([ + [ + [-180, 0.005], + [-179.995, 0], + [-179.99, 0.005], + [-179.995, 0.01], + [-180, 0.005], + ], + ]); + assert.equal(tangent.areas[0].clipped, false); }); test('a point in a courtyard is not inside the building', () => { diff --git a/src/tooling/nominatimOutlineRoute.test.mjs b/src/tooling/nominatimOutlineRoute.test.mjs index cffdfbf12..2dcf38cab 100644 --- a/src/tooling/nominatimOutlineRoute.test.mjs +++ b/src/tooling/nominatimOutlineRoute.test.mjs @@ -1347,6 +1347,40 @@ test('a landmark ask is not answered by the place around it', () => { landmark('Royal Botanic Park'), ); assert.equal(every.outline.name, 'Royal Botanic Historic Park'); + // A building word passes on a building-like result, not on a park. + for (const [query, result] of [ + [ + 'Burj Khalifa Tower', + at('Burj Khalifa', 'Burj Khalifa, Downtown Dubai, Dubai', { + category: 'building', + type: 'yes', + }), + ], + [ + 'Sagrada Familia Church', + at('Basílica de la Sagrada Família', 'Basílica, Barcelona, Spain', { + category: 'amenity', + type: 'place_of_worship', + }), + ], + ]) + assert.ok(selectOutlineResult([result], landmark(query)).outline, query); + assert.equal( + selectOutlineResult([park('Central Park')], landmark('Central Park Tower')) + .outline, + null, + ); +}); + +test('a worse name never wins the anchor by containment alone', () => { + const building = { + osm: 'way/1', + polygons: [square(0, 0, 0.0001)], + }; + const plaza = { osm: 'way/2', polygons: [square(-0.001, -0.001, 0.003)] }; + const anchor = [0.0002, 0.0002]; + assert.equal(outlineAt([building, plaza], ...anchor, [3, 2]).osm, 'way/1'); + assert.equal(outlineAt([building, plaza], ...anchor, [3, 3]).osm, 'way/2'); }); test('outlineAt reads a ring crossing the antimeridian in its own frame', () => {