Repository navigation
refactor(openrouteservice): isochrone file only - #528
dre-raygun wants to merge 4 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Integration tests✅ openrouteservice (8/8 tests pass)Passed (8) |
🔍 Integration Validation ResultsCommit:
✅ Structure Check output✅ Code Check output✅ Tests Check output✅ README Check output✅ Version Check output |
There was a problem hiding this comment.
| Result | P1 | P2 | P3 |
|---|---|---|---|
| Clean review | 0 | 0 | 0 |
Clean review
Summary
Reviewed commit: 7460c98
No actionable issues found in the isochrone file-only output, band summaries, or associated schema and documentation changes.
For an explanation, reply under a comment with @autohiveai and your question.
Limitations
- Tests were not run, as requested by the repository review settings.
Review powered by Autohive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7460c98b90
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| p1 = coords[lower] | ||
| p2 = coords[middle] | ||
| p3 = coords[upper] | ||
| area += math.radians(p3[0] - p1[0]) * math.sin(math.radians(p2[1])) |
There was a problem hiding this comment.
Normalize longitude deltas before computing band areas
For an isochrone whose polygon crosses the antimeridian, GeoJSON will contain adjacent longitudes near 179.x and -179.x; this subtraction treats the short crossing as an approximately 360° arc and can inflate area_sq_km by orders of magnitude. Normalize each longitude delta into the [-180, 180] range before converting it to radians so the compact band summary remains accurate for valid locations near ±180°.
Useful? React with 👍 / 👎.
Closes #527
Author commitment
Before opening an integration PR, I confirm that:
Summary
get_isochronecan return unsimplified drive-time polygons as a platform file only, so those rings do not enter the agent transcript. Callers that still want inline GeoJSON are unchanged.Version 1.3.0. Additive:
include_geojsondefaults to true.Behaviour
export_geojson=trueandinclude_geojson=false:geojsonis null, one file, plus abandssummary (no coordinates).include_geojson=falsewithout export:invalid_requestoninclude_geojson, no HeiGIT call.geojsonand setmessage. Geometry is never dropped.bands:time_minutes,geometry_type,polygon_count,vertex_count, geodesicarea_sq_km(holes subtracted; null if the calculation fails),bbox[west, south, east, north](null if any vertex is unusable; planar WGS84).export_geojson=true,include_geojson=false. Pass the file to the next action or script; readbandsin context.Tests
message; default call still returns inline GeoJSON plusbands; MultiPolygon / hole counts; ~1 km² square near 41°S within ±2%; bbox is null on NaN or truncated vertices.geojsonnull,bandspresent). The default live isochrone test assertsbands. Not run in this session (noOPENROUTESERVICE_API_KEYhere). Skip unless that key is set.validate_integration.pyandcheck_code.pypassed. Unit suite: 85 passed.Review standard
Reviewers are expected to hold integration PRs to this charter.
A PR may be sent back even if CI passes when the author has not demonstrated API understanding, intentional scope, safe permissions, meaningful tests, maintainable Python, or adequate self-review.
The goal is not to slow down integration development. The goal is to decrease time to develop and maintenance by avoiding shipping code we do not understand, cannot maintain long term, or cannot safely stand behind.
If you are unsure about anything on the above to help you move forward in the integration, reach out to a peer to get clarity