Repository navigation
fix: repair invalid AOI geometry before using it to trim the task grid - #189
Merged
Merged
Conversation
Related to hotosm#6604 (not a confirmed full reproduction - see below). GridService.merge_to_multi_polygon() validates the merged AOI using geojson.MultiPolygon.is_valid, which only checks structure (ring closure etc), not real topology. A self-intersecting AOI (e.g. from a hand-drawn or imported boundary) can be structurally well-formed while still being topologically invalid, so it passes this check unrepaired. That invalid AOI then gets used directly in per-tile intersection() calls in trim_grid_to_aoi(). Intersecting with an invalid input geometry is undefined behavior in GEOS/Shapely and can produce corrupted (self-intersecting) output even from a clean square input tile. Fix: after building multi_polygon (a real Shapely geometry), check its actual topological validity and repair with buffer(0) if needed, before converting to geojson and running the existing checks. Also handles the case where buffer(0) collapses a MultiPolygon down to a plain Polygon (same thing _dissolve already guards against for unary_union's output a few lines below) - forces it back to MultiPolygon so the existing type check doesn't reject a valid repair. Investigation: pulled project 5349's actual AOI and the 7 flagged task geometries from the public production API (they're publicly accessible). Confirmed all 7 tasks are genuinely invalid (self-intersecting rings, matching the reported "weird lines" that don't change task area), and confirmed the project's stored AOI is itself invalid via Shapely's real validity check, while geojson.MultiPolygon.is_valid on the same geometry reports valid - exactly the gap this fix closes. Verified buffer(0) repairs the real AOI losslessly (identical area and bounds, valid=True after). Could not reproduce project 5349's exact corrupted task squares by constructing guessed grid tiles against the real AOI - I don't have access to the actual original tile geometries the grid-generation process used for that project, so I can't fully confirm this mechanism is what produced that project's specific corruption. This PR is a genuine, independently-justified correctness fix (using invalid geometry in intersection operations is unsafe regardless), not a confirmed fix for hotosm#6604 specifically. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Brian Bergstrom <dulcetberg@gmail.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
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.
Related to hotosm#6604 (not a confirmed full reproduction - see below).
GridService.merge_to_multi_polygon() validates the merged AOI using geojson.MultiPolygon.is_valid, which only checks structure (ring closure etc), not real topology. A self-intersecting AOI (e.g. from a hand-drawn or imported boundary) can be structurally well-formed while still being topologically invalid, so it passes this check unrepaired.
That invalid AOI then gets used directly in per-tile intersection() calls in trim_grid_to_aoi(). Intersecting with an invalid input geometry is undefined behavior in GEOS/Shapely and can produce corrupted (self-intersecting) output even from a clean square input tile.
Fix: after building multi_polygon (a real Shapely geometry), check its actual topological validity and repair with buffer(0) if needed, before converting to geojson and running the existing checks. Also handles the case where buffer(0) collapses a MultiPolygon down to a plain Polygon (same thing _dissolve already guards against for unary_union's output a few lines below) - forces it back to MultiPolygon so the existing type check doesn't reject a valid repair.
Investigation: pulled project 5349's actual AOI and the 7 flagged task geometries from the public production API (they're publicly accessible). Confirmed all 7 tasks are genuinely invalid (self-intersecting rings, matching the reported "weird lines" that don't change task area), and confirmed the project's stored AOI is itself invalid via Shapely's real validity check, while geojson.MultiPolygon.is_valid on the same geometry reports valid - exactly the gap this fix closes. Verified buffer(0) repairs the real AOI losslessly (identical area and bounds, valid=True after).
Could not reproduce project 5349's exact corrupted task squares by constructing guessed grid tiles against the real AOI - I don't have access to the actual original tile geometries the grid-generation process used for that project, so I can't fully confirm this mechanism is what produced that project's specific corruption. This PR is a genuine, independently-justified correctness fix (using invalid geometry in intersection operations is unsafe regardless), not a confirmed fix for hotosm#6604 specifically.
What type of PR is this? (check all applicable)
Related Issue
Example: Fixes #123
Describe this PR
A brief description of how this solves the issue.
Screenshots
Please provide screenshots of the change.
Alternative Approaches Considered
Did you attempt any other approaches that are not documented in code?
Review Guide
Notes for the reviewer. How to test this change?
Checklist before requesting a review
[optional] What gif best describes this PR or how it makes you feel?