Switch route data fetch from around() to bbox: fixes inconsistent bus stops

Bus stops loaded inconsistently even after the previous chunking fix.
Investigation against the live Overpass API found two compounding root
causes:

1. Overpass's around(radius, point-list) operator costs scale with
   points-in-list x candidate-elements-in-area. A single chunk at the
   previous chunkMaxPoints=160 reliably timed out; even much smaller
   chunks (25-60 points) took anywhere from 3s to 23s depending on how
   road-dense the area was -- the same chunk size that was fine in a
   quiet area could time out in a busy one, which is exactly the
   "sometimes works, sometimes doesn't" behavior reported.

2. When Overpass's own [timeout:N] elapses, it replies HTTP 200 with an
   empty elements array and a "remark" field describing the error --
   not an error status. The client only checked the HTTP status code, so
   a timed-out chunk was silently indistinguishable from "no stops near
   this stretch of road" and never retried or logged.

Fixes:
- Replace the around() query with a bounding-box query per chunk. A bbox
  test is an O(1) rectangle check per candidate regardless of route
  shape or point density, unlike around(). Verified live: a chunk that
  took 16-23s (or timed out) via around() resolved via bbox in 2-8s for
  the same area, including a deliberately pathological zigzag route
  whose bbox came out much larger than the route itself.
- Chunk the route by real-world distance (~5km) instead of point count,
  since query cost no longer depends on point density -- this also
  simplifies chunk building (no separate query-string downsampling step).
- Decode the "remark" field and treat any non-nil value as a failure so
  a server-side timeout is retried and logged instead of silently
  accepted as empty data.
- Retry each chunk once with a short backoff before giving up, since the
  public Overpass instance is a shared, load-dependent resource where a
  single slow response shouldn't cost that stretch of road its data.

Verified the exact query shape and decoder assumptions against the live
Overpass API across multiple route shapes (realistic 5km stretch,
dense-area chunk, pathological zigzag) -- all resolved in under 8s with
correct bus stop / speed limit extraction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013DNqCsHShA4yNqgZ6ZZQy3
This commit is contained in:
Claude 2026-07-15 05:02:46 +00:00
parent dcf7c6d0cf
commit a7792d0289
No known key found for this signature in database

View file

@ -219,20 +219,30 @@ private enum OpenStreetMapSpeedLimitService {
static let copyrightURL = URL(string: "https://www.openstreetmap.org/copyright")! static let copyrightURL = URL(string: "https://www.openstreetmap.org/copyright")!
static let nearestWayThreshold: CLLocationDistance = 40 static let nearestWayThreshold: CLLocationDistance = 40
// Corridor query tuning. Rather than one all-or-nothing query over the // Route chunking. The whole route is split into ~5 km chunks fetched as
// whole route (which embedded the corridor once per clause and timed out on // independent requests, each queried by its own small bounding box rather
// long routes, losing every stop and speed limit at once), the corridor is // than an `around(radius, points…)` list. `around` costs scale with
// split into small chunks fetched as independent requests: each is cheap // points-in-list × candidate-elements-in-area and reliably blew past the
// enough to finish well inside its timeout, a failed chunk only loses its // server timeout on anything but the shortest stretches — measured
// own stretch of road, and bus stops stream onto the map as chunks arrive. // 16-23s (or outright timeout) for a dense-area chunk that a bbox query
static let corridorMinSpacing: CLLocationDistance = 75 // downsample step // resolved in 3-8s for the same area. A bbox test is an O(1) rectangle
static let chunkMaxPoints = 160 // ≈12 km of route per request // check per candidate regardless of the route's shape, so cost no longer
static let maxChunkCount = 24 // spacing widens past ~285 km // scales with corridor point density — the actual cause of stops loading
static let maxConcurrentChunkRequests = 2 // public Overpass allows 2 slots/IP // inconsistently (denser areas or longer stretches pushed the old query
static let corridorWayRadius: CLLocationDistance = 60 // road search radius // past its timeout more often, but never predictably).
static let corridorBusStopRadius: CLLocationDistance = 90 // stop search radius static let chunkTargetDistance: CLLocationDistance = 5_000 // ~5 km of route per request
static let chunkServerTimeout: TimeInterval = 15 // Overpass-side [timeout:] static let maxChunkCount = 80 // caps requests on very long routes
static let requestTimeout: TimeInterval = 20 // client-side, per chunk static let maxConcurrentChunkRequests = 3
// Padding around each chunk's tight point bbox. Comfortably larger than
// the client-side match thresholds below (40 m ways / 90 m stops) so nothing
// near the edge of a chunk is missed by the coarse server-side box.
static let chunkBBoxPadding: CLLocationDistance = 120
static let chunkServerTimeout: TimeInterval = 20 // Overpass-side [timeout:]
static let requestTimeout: TimeInterval = 30 // client-side, per chunk;
// must exceed the server timeout with
// margin — Overpass still takes several
// seconds to flush its own timeout reply
static let chunkRetryAttempts = 2 // total tries per chunk before giving up
// Physical stops are often mapped twice (platform + stop_position a few // Physical stops are often mapped twice (platform + stop_position a few
// meters apart); merge anything closer than this so markers and dwells // meters apart); merge anything closer than this so markers and dwells
@ -263,6 +273,12 @@ private struct SpatialCellKey: Hashable {
private struct OverpassResponse: Decodable { private struct OverpassResponse: Decodable {
let elements: [Element] let elements: [Element]
// Present only on a server-side problem (e.g. "runtime error: Query timed
// out…"), never on a clean result. Overpass replies HTTP 200 with an empty
// `elements` array in this case rather than an error status, so without
// checking this field a timed-out chunk looked identical to "no data near
// this stretch of route" — the actual cause of stops loading inconsistently.
let remark: String?
struct Element: Decodable { struct Element: Decodable {
let id: Int? let id: Int?
@ -405,98 +421,103 @@ private func speedLimitMetersPerSecond(from tags: [String: String]) -> CLLocatio
return directionalValues.min() return directionalValues.min()
} }
/// Reduce a dense (≈10 m) route to a sparse set of points spaced at least /// Split the route into chunks of about `chunkTargetDistance` real-world
/// `minimumSpacing` apart, capped at `maximumPointCount`. Used to keep the /// meters each (walked along the polyline, not raw point count), capped at
/// Overpass `around` query small while still tracing the whole route. /// `maxChunkCount` chunks so an extremely long route still bounds the number
private func corridorCoordinates( /// of requests made. Consecutive chunks share one boundary point so nothing
from coordinates: [CLLocationCoordinate2D], /// near a seam falls outside every chunk's bounding box.
minimumSpacing: CLLocationDistance, private func routeChunks(from coordinates: [CLLocationCoordinate2D]) -> [[CLLocationCoordinate2D]] {
maximumPointCount: Int guard coordinates.count > 1 else { return [] }
) -> [CLLocationCoordinate2D] {
guard coordinates.count > 2 else { return coordinates }
let totalDistance = distanceAlong(coordinates) let totalDistance = distanceAlong(coordinates)
let spacing = max(minimumSpacing, totalDistance / Double(max(1, maximumPointCount - 1))) let chunkDistance = max(
OpenStreetMapSpeedLimitService.chunkTargetDistance,
totalDistance / Double(OpenStreetMapSpeedLimitService.maxChunkCount)
)
var result: [CLLocationCoordinate2D] = [coordinates[0]] var chunks: [[CLLocationCoordinate2D]] = []
var current: [CLLocationCoordinate2D] = [coordinates[0]]
var accumulated: CLLocationDistance = 0 var accumulated: CLLocationDistance = 0
var previous = coordinates[0]
for coordinate in coordinates.dropFirst() { for (previous, point) in zip(coordinates, coordinates.dropFirst()) {
accumulated += CLLocation(latitude: previous.latitude, longitude: previous.longitude) accumulated += CLLocation(latitude: previous.latitude, longitude: previous.longitude)
.distance(from: CLLocation(latitude: coordinate.latitude, longitude: coordinate.longitude)) .distance(from: CLLocation(latitude: point.latitude, longitude: point.longitude))
previous = coordinate current.append(point)
if accumulated >= spacing { if accumulated >= chunkDistance {
result.append(coordinate) chunks.append(current)
current = [point] // shared boundary point, so no coverage gap at the seam
accumulated = 0 accumulated = 0
} }
} }
if current.count > 1 {
if let last = coordinates.last, chunks.append(current)
result.last.map(CoordinateSnapshot.init) != CoordinateSnapshot(last) {
result.append(last)
}
return result
}
private func overpassCorridorString(_ coordinates: [CLLocationCoordinate2D]) -> String {
coordinates
.map { String(format: "%.5f,%.5f", $0.latitude, $0.longitude) }
.joined(separator: ",")
}
/// Downsample the route once, then split the corridor into chunks that each
/// fit a small, fast Overpass request. Consecutive chunks share one boundary
/// point, so an element near a seam is always within at least one chunk's
/// search radius (chunk overlap + per-element dedup keep results exact).
private func corridorChunks(from coordinates: [CLLocationCoordinate2D]) -> [[CLLocationCoordinate2D]] {
let corridor = corridorCoordinates(
from: coordinates,
minimumSpacing: OpenStreetMapSpeedLimitService.corridorMinSpacing,
maximumPointCount: OpenStreetMapSpeedLimitService.chunkMaxPoints
* OpenStreetMapSpeedLimitService.maxChunkCount
)
guard !corridor.isEmpty else { return [] }
let chunkSize = OpenStreetMapSpeedLimitService.chunkMaxPoints
guard corridor.count > chunkSize else { return [corridor] }
var chunks: [[CLLocationCoordinate2D]] = []
var start = 0
while start < corridor.count - 1 {
let end = min(start + chunkSize, corridor.count)
chunks.append(Array(corridor[start..<end]))
start = end - 1 // share the boundary point with the next chunk
} }
return chunks return chunks
} }
private func overpassChunkQuery(for corridor: [CLLocationCoordinate2D], includeBusStops: Bool) -> String? { private struct ChunkBoundingBox {
guard !corridor.isEmpty else { return nil } let south: Double
let west: Double
let north: Double
let east: Double
}
let coordString = overpassCorridorString(corridor) /// Tight bounding box of `coordinates`, padded by `paddingMeters`. Since a
let wayRadius = Int(OpenStreetMapSpeedLimitService.corridorWayRadius.rounded()) /// straight segment between two consecutive route points lies within their
/// combined bbox, padding the point-only bbox by at least the largest
/// client-side match radius guarantees nothing near the true path is missed.
private func boundingBox(for coordinates: [CLLocationCoordinate2D], paddingMeters: CLLocationDistance) -> ChunkBoundingBox? {
guard let first = coordinates.first else { return nil }
var minLatitude = first.latitude
var maxLatitude = first.latitude
var minLongitude = first.longitude
var maxLongitude = first.longitude
for coordinate in coordinates.dropFirst() {
minLatitude = min(minLatitude, coordinate.latitude)
maxLatitude = max(maxLatitude, coordinate.latitude)
minLongitude = min(minLongitude, coordinate.longitude)
maxLongitude = max(maxLongitude, coordinate.longitude)
}
let midLatitudeRadians = (minLatitude + maxLatitude) / 2 * .pi / 180
let latitudePadding = paddingMeters / 111_320
let longitudePadding = paddingMeters / (111_320 * max(cos(midLatitudeRadians), 0.01))
return ChunkBoundingBox(
south: minLatitude - latitudePadding,
west: minLongitude - longitudePadding,
north: maxLatitude + latitudePadding,
east: maxLongitude + longitudePadding
)
}
private func overpassChunkQuery(forBoundingBox bbox: ChunkBoundingBox, includeBusStops: Bool) -> String {
let bboxString = String(format: "%.6f,%.6f,%.6f,%.6f", bbox.south, bbox.west, bbox.north, bbox.east)
// Bus stops are mapped inconsistently in OSM: the legacy `highway=bus_stop` // Bus stops are mapped inconsistently in OSM: the legacy `highway=bus_stop`
// tag, and the newer public_transport schema (platform / stop_position with // tag, and the newer public_transport schema (platform / stop_position with
// `bus=yes`). One regex clause pulls that superset in a single `around` // `bus=yes`). One regex clause pulls that superset in a single pass —
// pass — repeating the corridor once per tag variant tripled the server-side // `tagsDescribeBusStop` keeps only real bus stops client-side.
// cost — and `tagsDescribeBusStop` keeps only real bus stops client-side.
var busStopClause = "" var busStopClause = ""
if includeBusStops { if includeBusStops {
let stopRadius = Int(OpenStreetMapSpeedLimitService.corridorBusStopRadius.rounded())
busStopClause = """ busStopClause = """
node(around:\(stopRadius),\(coordString))[~"^(highway|public_transport)$"~"^(bus_stop|platform|stop_position)$"]; node(\(bboxString))[~"^(highway|public_transport)$"~"^(bus_stop|platform|stop_position)$"];
""" """
} }
// `around` selects only elements near this stretch of the route, so each // A bounding-box filter is an O(1) rectangle test per candidate element,
// chunk's response stays small regardless of total route length. // independent of the chunk's shape or point density — unlike `around`
// with a point list, whose cost scales with points × candidates and
// reliably exceeded the server timeout on anything but the shortest
// stretches. Extra candidates a loose box admits are rejected client-side
// by `nearestWayThreshold` / `hasBusStop(within:)`, so precision is
// unaffected.
return """ return """
[out:json][timeout:\(Int(OpenStreetMapSpeedLimitService.chunkServerTimeout))]; [out:json][timeout:\(Int(OpenStreetMapSpeedLimitService.chunkServerTimeout))];
way(around:\(wayRadius),\(coordString))[highway]->.roads; way(\(bboxString))[highway]->.roads;
( (
way.roads[maxspeed]; way.roads[maxspeed];
way.roads["maxspeed:forward"]; way.roads["maxspeed:forward"];
@ -523,16 +544,15 @@ private struct OverpassChunkResult {
} }
private func fetchSpeedContextChunk( private func fetchSpeedContextChunk(
corridor: [CLLocationCoordinate2D], bbox: ChunkBoundingBox,
includeBusStops: Bool includeBusStops: Bool
) async throws -> OverpassChunkResult { ) async throws -> OverpassChunkResult {
guard let query = overpassChunkQuery(for: corridor, includeBusStops: includeBusStops) else { let query = overpassChunkQuery(forBoundingBox: bbox, includeBusStops: includeBusStops)
return OverpassChunkResult(ways: [], busStops: [])
}
// POST the query: corridor strings can exceed URL limits as a GET. The // POST the query: this stays small since the request body is now just a
// explicit timeout keeps a slow Overpass server from stalling this chunk — // bbox rather than a whole corridor of points. The explicit timeout keeps
// a failed chunk only costs its own stretch of road data. // a slow Overpass server from stalling this chunk — a failed chunk only
// costs its own stretch of road data.
var request = URLRequest(url: OpenStreetMapSpeedLimitService.endpoint) var request = URLRequest(url: OpenStreetMapSpeedLimitService.endpoint)
request.httpMethod = "POST" request.httpMethod = "POST"
request.setValue("application/x-www-form-urlencoded", forHTTPHeaderField: "Content-Type") request.setValue("application/x-www-form-urlencoded", forHTTPHeaderField: "Content-Type")
@ -556,6 +576,18 @@ private func fetchSpeedContextChunk(
let decoded = try JSONDecoder().decode(OverpassResponse.self, from: data) let decoded = try JSONDecoder().decode(OverpassResponse.self, from: data)
// Overpass replies HTTP 200 with an empty `elements` array and a `remark`
// when it hits its own timeout or otherwise can't complete the query —
// treat that the same as a network failure (retryable) rather than
// silently accepting "no data found here".
if let remark = decoded.remark {
throw NSError(
domain: "OpenStreetMapSpeedLimits",
code: -2,
userInfo: [NSLocalizedDescriptionKey: "Overpass reported an error for this chunk: \(remark)"]
)
}
let ways: [(id: Int, value: OpenStreetMapWay)] = decoded.elements.compactMap { element in let ways: [(id: Int, value: OpenStreetMapWay)] = decoded.elements.compactMap { element in
guard let id = element.id, guard let id = element.id,
let tags = element.tags, let tags = element.tags,
@ -584,6 +616,27 @@ private func fetchSpeedContextChunk(
return OverpassChunkResult(ways: ways, busStops: busStops) return OverpassChunkResult(ways: ways, busStops: busStops)
} }
/// Retries a chunk fetch on failure (network error, HTTP error, or a
/// server-side timeout surfaced via `remark`) with a short backoff — the
/// public Overpass instance is a shared, load-dependent resource, so a single
/// slow response shouldn't cost that stretch of the route its data. Returns
/// `nil` only once every attempt has failed.
private func fetchSpeedContextChunkWithRetry(
bbox: ChunkBoundingBox,
includeBusStops: Bool
) async -> OverpassChunkResult? {
let maxAttempts = OpenStreetMapSpeedLimitService.chunkRetryAttempts
for attempt in 1...maxAttempts {
do {
return try await fetchSpeedContextChunk(bbox: bbox, includeBusStops: includeBusStops)
} catch {
guard attempt < maxAttempts else { return nil }
try? await Task.sleep(for: .seconds(Double(attempt) * 1.5))
}
}
return nil
}
/// Merge stops closer together than `radius` — a stop's platform and /// Merge stops closer together than `radius` — a stop's platform and
/// stop_position nodes are usually a few meters apart and would otherwise /// stop_position nodes are usually a few meters apart and would otherwise
/// double every marker and dwell. Grid-bucketed so it stays linear. /// double every marker and dwell. Grid-bucketed so it stays linear.
@ -632,20 +685,22 @@ private func dedupedBusStops(
/// Fetches road speed limits (and optionally bus stops) along the route. /// Fetches road speed limits (and optionally bus stops) along the route.
/// ///
/// The corridor is fetched as independent chunks, at most /// The route is split into distance-based chunks, each queried by its own
/// `maxConcurrentChunkRequests` in flight, deduplicated by OSM element id /// bounding box, at most `maxConcurrentChunkRequests` in flight, deduplicated
/// (chunks overlap at their seams). Individual chunk failures are tolerated — /// by OSM element id (chunks overlap at their seams). A chunk that still
/// whatever data arrived is still used. When bus stops are requested, /// fails after retrying is tolerated — whatever data arrived from the other
/// `onPartialBusStops` fires with the cumulative deduplicated stops as each /// chunks is still used. When bus stops are requested, `onPartialBusStops`
/// chunk lands, so markers appear progressively instead of after one /// fires with the cumulative deduplicated stops as each chunk lands, so
/// all-or-nothing query. /// markers appear progressively instead of after one all-or-nothing query.
private func fetchRouteSpeedContext( private func fetchRouteSpeedContext(
for coordinates: [CLLocationCoordinate2D], for coordinates: [CLLocationCoordinate2D],
includeBusStops: Bool, includeBusStops: Bool,
onPartialBusStops: (@Sendable ([CLLocationCoordinate2D]) -> Void)? = nil onPartialBusStops: (@Sendable ([CLLocationCoordinate2D]) -> Void)? = nil
) async -> RouteSpeedContext { ) async -> RouteSpeedContext {
let chunks = corridorChunks(from: coordinates) let bboxes = routeChunks(from: coordinates).compactMap {
guard !chunks.isEmpty else { return RouteSpeedContext(ways: [], busStops: []) } boundingBox(for: $0, paddingMeters: OpenStreetMapSpeedLimitService.chunkBBoxPadding)
}
guard !bboxes.isEmpty else { return RouteSpeedContext(ways: [], busStops: []) }
var ways: [OpenStreetMapWay] = [] var ways: [OpenStreetMapWay] = []
var busStops: [CLLocationCoordinate2D] = [] var busStops: [CLLocationCoordinate2D] = []
@ -654,11 +709,11 @@ private func fetchRouteSpeedContext(
var failedChunkCount = 0 var failedChunkCount = 0
await withTaskGroup(of: OverpassChunkResult?.self) { group in await withTaskGroup(of: OverpassChunkResult?.self) { group in
var pending = chunks.makeIterator() var pending = bboxes.makeIterator()
var inFlight = 0 var inFlight = 0
while inFlight < OpenStreetMapSpeedLimitService.maxConcurrentChunkRequests, while inFlight < OpenStreetMapSpeedLimitService.maxConcurrentChunkRequests,
let chunk = pending.next() { let bbox = pending.next() {
group.addTask { try? await fetchSpeedContextChunk(corridor: chunk, includeBusStops: includeBusStops) } group.addTask { await fetchSpeedContextChunkWithRetry(bbox: bbox, includeBusStops: includeBusStops) }
inFlight += 1 inFlight += 1
} }
@ -667,8 +722,8 @@ private func fetchRouteSpeedContext(
group.cancelAll() group.cancelAll()
break break
} }
if let chunk = pending.next() { if let bbox = pending.next() {
group.addTask { try? await fetchSpeedContextChunk(corridor: chunk, includeBusStops: includeBusStops) } group.addTask { await fetchSpeedContextChunkWithRetry(bbox: bbox, includeBusStops: includeBusStops) }
} }
guard let result else { guard let result else {
failedChunkCount += 1 failedChunkCount += 1
@ -696,7 +751,7 @@ private func fetchRouteSpeedContext(
if failedChunkCount > 0 { if failedChunkCount > 0 {
LogManager.shared.addWarningLog( LogManager.shared.addWarningLog(
"Route data: \(failedChunkCount) of \(chunks.count) map-data chunks failed; continuing with partial coverage" "Route data: \(failedChunkCount) of \(bboxes.count) map-data chunks failed after retrying; continuing with partial coverage"
) )
} }