Give un-addressed POIs a street and settlement for Places Nearby - #1046
Merged
Conversation
davecraig
had a problem deploying
to
development
August 28, 2026 11:04 — with 
GitHub Actions
Failure
GitHub Actions
Failure
Places Nearby lists every POI in the loaded grid, and OSM never names most street furniture, so the screen fills with interchangeable "Post Box" and "Bench" rows distinguishable only by their distance. Each now carries the street and settlement it sits in, e.g. "London Road, Bridgeton". The spatial association is done once per grid rebuild, in GridState's new attachNearestWays, and hung off the feature as MvtFeature.nearestWay and nearestSettlement. It then lives as long as the grid does, so the screen can be opened, scrolled, re-sorted and re-opened for free, and any other consumer gets it without recomputing. No strings are built there - turning the association into text stays lazy in LocationDescription.process(), which already runs per visible row, so neither list composable nor either ViewModel changes. A POI is matched to the nearest way that identifies itself (name, else ref) within 30m, roads first and only then paths: ROADS deliberately excludes footways, so a POI on a pavement is described by the street, not the pavement. Distances are measured from the way back to the POI rather than from the POI's centre, because a large feature's centre can be well outside the threshold while the feature is right beside the road - a Milngavie playground's middle is 35m from the street its fence is 10m from. Settlements aren't in the high-zoom tiles at all (no "place" layer), so GeoEngine hands the main grid a provider backed by the low-zoom settlement grid, and updates that grid first so it's populated before the main one rebuilds. Lookups are bucketed into ~110m cells; the cascade's smallest radius is 1km, so that can't change the answer, and it takes most of the work out of what is otherwise four rtree queries per POI. The Nominatim-radii cascade itself was duplicated in OfflineGeocoder and travellingReverseGeocodeName, and is now shared as nearestSettlement(). POIs which carry their own addr:street still skip the way lookup but keep the settlement: OSM addresses routinely stop at addr:street with no addr:city, which is how "Kersland Drive Car Park" presented as a bare "Kersland Drive" with no town. process() folds the settlement in as the city when the tags don't supply one, so the existing AddressFormatter produces the whole address. Selecting a row now fills in the full offline-geocoded address, house number included, via a new suspend GeoEngine.getOfflineAddress - deliberately the offline geocoder rather than whichever MultiGeocoder would pick, since the place is already in the loaded grid. The details screen shows the street line immediately and upgrades when the geocode arrives. Reverse geocoding a bare point no longer produces "just before"/"just after"/"until"/"since". Those place a location relative to the direction of travel, which describeLocation only knows from a heading - without one it fills ahead and behind in arbitrarily, so they were as likely as not naming the wrong side. "Between" is symmetric and stands either way, and "near" replaces "just before"/"just after" for a feature right beside the location. Callouts with a heading are unchanged. On a 3x3 z14 Glasgow grid the association takes ~117ms (~52ms for the 2x2 grid used in production, against the ~200ms the rtree build beside it already costs), and gives 3181 of 3905 un-named POIs a street and all 3905 a settlement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Ecp7eBi3ozoQqzk5H8v5n
OSM rarely names the path along a river or round a reservoir, so the app announces a bare "Path" where a sighted map reader sees "the riverside path". The water is the landmark those paths are known by locally, and it's already in the tiles. An un-named way that demonstrably follows one is now described by it, e.g. "Path next to Allander Water" or "Path next to Craigmaddie Reservoir", matching the phrasing of the existing pavement confection. Half the data was already there: named "water" polygons have been kept in TreeId.NAMED_WATER_POLYGONS since the firth-crossing work. Linear waterways were not - extractCrossings reads that layer but keeps only crossing metadata keyed by road osmId and discards the geometry. extractNamedWaterways now retains the named river/canal/stream lines in their own tree. Its class filter is deliberately wider than significantWaterwayClasses, because the two answer different questions: that one asks whether crossing something is worth announcing, where a culverted stream is not, while this asks whether following something for hundreds of metres identifies the path, and a burn very much does. Culverted segments (brunnel-tagged) are skipped either way - a path isn't "next to" a watercourse buried under it. Waterways are excluded from TreeId.POIS, or every river turns up in "What's around me". Being near water isn't enough, since a road crossing a river is near it too. The test is an absolute length rather than a proportion: at least 80m of the way, or all of it if it's shorter, must lie within 25m of the same water. A path that follows the bank for a couple of hundred metres and then turns inland is still the path next to the river, so what earns the water its mention is the distance spent beside it, not what fraction of the path that represents. A crossing is beside the river only either side of the bridge and fails. Distances are measured to a lake's shore rather than its centroid - the Craigmaddie Reservoir path is 10-24m from the water and several hundred from the middle. Candidates are grouped by name first, because one body of water is many features: a river is a chain of LineStrings and a loch spanning the grid arrives as several tile-clipped polygons, and testing fragments separately would let a path fail against every fragment of the very river it runs along. The 80m figure is calibrated, not guessed: the path at 55.931961,-4.305300 runs 99m of its 124m within 25m of Allander Water, so the rounder 100m would have missed the very example that prompted this. Over the whole Milngavie grid it names 56 of 2773 ways in 18ms, every one of them a genuine waterside path. confectNamesForRoad now returns the name it confected instead of working purely by mutation. It assigned to Way.name, which getName() only picks up on its *next* call, so the first callout for a way said a bare "Path" and the real description arrived afterwards - the first one is the one the user hears. That also applies the pavement confection a call earlier, which no existing expectation depended on. Water is tried after pavements and before POI destinations: a path that hugs a river for its whole length is better described by the river than by whatever happens to sit at its two ends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Ecp7eBi3ozoQqzk5H8v5n
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Places Nearby lists every POI in the loaded grid, and OSM never names most street furniture, so the screen fills with interchangeable "Post Box" and "Bench" rows distinguishable only by their distance. Each now carries the street and settlement it sits in, e.g. "London Road, Bridgeton".
The spatial association is done once per grid rebuild, in GridState's new attachNearestWays, and hung off the feature as MvtFeature.nearestWay and nearestSettlement. It then lives as long as the grid does, so the screen can be opened, scrolled, re-sorted and re-opened for free, and any other consumer gets it without recomputing. No strings are built there - turning the association into text stays lazy in LocationDescription.process(), which already runs per visible row, so neither list composable nor either ViewModel changes.
A POI is matched to the nearest way that identifies itself (name, else ref) within 30m, roads first and only then paths: ROADS deliberately excludes footways, so a POI on a pavement is described by the street, not the pavement. Distances are measured from the way back to the POI rather than from the POI's centre, because a large feature's centre can be well outside the threshold while the feature is right beside the road - a Milngavie playground's middle is 35m from the street its fence is 10m from.
Settlements aren't in the high-zoom tiles at all (no "place" layer), so GeoEngine hands the main grid a provider backed by the low-zoom settlement grid, and updates that grid first so it's populated before the main one rebuilds. Lookups are bucketed into ~110m cells; the cascade's smallest radius is 1km, so that can't change the answer, and it takes most of the work out of what is otherwise four rtree queries per POI. The Nominatim-radii cascade itself was duplicated in OfflineGeocoder and travellingReverseGeocodeName, and is now shared as nearestSettlement().
POIs which carry their own addr:street still skip the way lookup but keep the settlement: OSM addresses routinely stop at addr:street with no addr:city, which is how "Kersland Drive Car Park" presented as a bare "Kersland Drive" with no town. process() folds the settlement in as the city when the tags don't supply one, so the existing AddressFormatter produces the whole address.
Selecting a row now fills in the full offline-geocoded address, house number included, via a new suspend GeoEngine.getOfflineAddress - deliberately the offline geocoder rather than whichever MultiGeocoder would pick, since the place is already in the loaded grid. The details screen shows the street line immediately and upgrades when the geocode arrives.
Reverse geocoding a bare point no longer produces "just before"/"just after"/"until"/"since". Those place a location relative to the direction of travel, which describeLocation only knows from a heading - without one it fills ahead and behind in arbitrarily, so they were as likely as not naming the wrong side. "Between" is symmetric and stands either way, and "near" replaces "just before"/"just after" for a feature right beside the location. Callouts with a heading are unchanged.
On a 3x3 z14 Glasgow grid the association takes ~117ms (~52ms for the 2x2 grid used in production, against the ~200ms the rtree build beside it already costs), and gives 3181 of 3905 un-named POIs a street and all 3905 a settlement.
Claude-Session: https://claude.ai/code/session_013Ecp7eBi3ozoQqzk5H8v5n