Fix imported routes collapsing to straight lines on heatmap#2291
Merged
robiningelbrecht merged 8 commits intoJul 21, 2026
Merged
Conversation
Polyline::simplify() defaulted to a tolerance of 0.4, but the tolerance is expressed in degrees (~44km). Douglas-Peucker therefore dropped every point within 0.4 degrees of the start-end chord, collapsing any normal GPS route to just its endpoints - a straight line on the heatmap. Affected all file imports (FIT/TCX/GPX), which run coordinates through simplify(); Strava import is unaffected as its polyline comes from the API. Lower the default tolerance to 0.0001 (~11m) and add a regression test pinning the default against a realistic GPS-scale route. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
We need a migration to fix the faulty activities (any activity with importSource=files) so people do not need to re-import those activities. I'll add some other small improvements as well. |
Activities already imported from FIT/TCX/GPX files have a collapsed 2-point polyline stored (the straight line caused by the simplify(0.4) bug). Recompute their polyline from the still-intact latlng stream, using the corrected simplify tolerance, so users don't have to re-import. Scoped to file imports (fitFile/tcxFile/gpxFile); Strava API polylines are left untouched. Irreversible data migration (down() is a no-op). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lowering the simplify() tolerance for the heatmap made the stored polyline denser. That polyline is also fed to the geosop binary for country detection, where the route WKT is passed as a single CLI argument; a dense, high-frequency route produces a WKT exceeding the OS MAX_ARG_STRLEN (~128KB) and crashes the import with "Failed to run geosop" (regression of dreeveapp#2230). Simplify the geosop input on its own (aggressive ~1.1km tolerance) so the argument stays bounded regardless of the stored polyline's density. The stored polyline keeps full fidelity for the heatmap. Add a regression test with a 40k-point route that previously overflowed the argument. Also pin the data migration's simplify() tolerance explicitly so the historical migration is frozen against future default changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
What type of PR is this? (check all applicable)
Description
Polyline::simplify()defaulted to a tolerance of 0.4, but the tolerance is expressed in degrees (~44km). Douglas-Peucker therefore dropped every point within 0.4 degrees of the start-end chord, collapsing any normal GPS route to just its endpoints - a straight line on the heatmap.Affected all file imports (FIT/TCX/GPX), which run coordinates through
simplify(); Strava import is unaffected as its polyline comes from the API.Lower the default tolerance to 0.0001 (~11m) and add a regression test pinning the default against a realistic GPS-scale route.
Fixes #2289