Repository navigation
Add mapbox isochrone - #44
mattpodwysocki wants to merge 4 commits into
Conversation
bd00deb to
16e44ed
Compare
Third of the Navigation-category APIs with no prior CLI coverage. Same shape as directions/isochrone (#43, #44): hand-authored into custom-openapi/ since openapi-specs has no spec for this API either, reusing ARG_NAME_OVERRIDES for the same profile-vs-global-flag collision (third row, not a third mechanism). Excludes POST, for the same documented reason directions route does: this spec format can't express "GET or POST, caller's choice" for one operationId, and the API's own POST exists specifically for a trace too long for a URL (~8100 bytes) — a real gap, not a design choice. Smoke-tested against production: a three-point San Francisco trace returned a real match with legs/steps/geometry, including a null tracepoint for a point too far from the road network to match — the documented shape for that case, not a bug. 486 tests, fmt and clippy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Third of the Navigation-category APIs with no prior CLI coverage. Same shape as directions/isochrone (#43, #44): hand-authored into custom-openapi/ since openapi-specs has no spec for this API either, reusing ARG_NAME_OVERRIDES for the same profile-vs-global-flag collision (third row, not a third mechanism). Excludes POST, for the same documented reason directions route does: this spec format can't express "GET or POST, caller's choice" for one operationId, and the API's own POST exists specifically for a trace too long for a URL (~8100 bytes) — a real gap, not a design choice. Smoke-tested against production: a three-point San Francisco trace returned a real match with legs/steps/geometry, including a null tracepoint for a point too far from the road network to match — the documented shape for that case, not a bug. 486 tests, fmt and clippy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
63629b3 to
dfc6759
Compare
16e44ed to
17ae5e4
Compare
Third of the Navigation-category APIs with no prior CLI coverage. Same shape as directions/isochrone (#43, #44): hand-authored into custom-openapi/ since openapi-specs has no spec for this API either, reusing ARG_NAME_OVERRIDES for the same profile-vs-global-flag collision (third row, not a third mechanism). Excludes POST, for the same documented reason directions route does: this spec format can't express "GET or POST, caller's choice" for one operationId, and the API's own POST exists specifically for a trace too long for a URL (~8100 bytes) — a real gap, not a design choice. Smoke-tested against production: a three-point San Francisco trace returned a real match with legs/steps/geometry, including a null tracepoint for a point too far from the road network to match — the documented shape for that case, not a bug. 486 tests, fmt and clippy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zmofei
left a comment
There was a problem hiding this comment.
A few parameter descriptions don't match the API docs (https://docs.mapbox.com/api/navigation/isochrone/), and denoise is reversed. Please fix these in both the YAML and docs/commands.md. Also, the "Also included" section of the PR description says this PR fixes eight directions.yaml descriptions, but that change is in #43 (cc7486d), not in this diff.
| # `0.0-1.0:` on its own left `--help` showing just `0`. | ||
| # `--schema` and `docs/commands.md` still show it in full. | ||
| description: >- | ||
| A smaller value removes more of the smaller contours, 0.0-1.0, |
There was a problem hiding this comment.
This is reversed. The docs say 1.0 keeps only the largest contour, so a higher value removes more. Also, first_sentence() still cuts at the . in 0.0, so --help shows "…the smaller contours, 0". Maybe "Drops contours smaller than this fraction of the largest one; 1 (the default) keeps only the largest. From 0 to 1."
There was a problem hiding this comment.
Fixed, and confirmed the direction against docs.mapbox.com directly: a larger value is more aggressive (1.0 keeps only the largest contour, 0.5 drops anything under half its area). Also fixed the truncation bug you caught — spelled the bound as "0 to 1"/"1" instead of "0.0-1.0"/"1.0" so first_sentence() has no "." to cut on before the end.
There was a problem hiding this comment.
Fixed, it was backwards. Now: "A larger value removes more of the smaller contours, 0 to 1, defaulting to 1. A value of 1 returns only the largest contour for each level; 0.5 drops any contour under half the largest's area." Spelled "0 to 1"/"1" instead of "0.0-1.0"/"1.0" because first_sentence() in src/main.rs cuts --help text at the first literal "." anywhere in the string, not just at the start, so a decimal bound further into the sentence was truncating the help text too. Left a comment explaining this so it doesn't get undone later.
| in: query | ||
| required: false | ||
| description: >- | ||
| Douglas-Peucker simplification tolerance in meters. A higher |
There was a problem hiding this comment.
The docs don't say a higher value makes the contour smaller, only coarser. Please drop "smaller".
There was a problem hiding this comment.
Dropped "smaller" — confirmed the docs only say coarser.
There was a problem hiding this comment.
Dropped "smaller", thanks. Now just "A higher value gives a coarser contour."
| in: query | ||
| required: false | ||
| description: >- | ||
| Road types to route around, comma-separated. Options are |
There was a problem hiding this comment.
The docs say all five values work only with mapbox/driving and mapbox/driving-traffic. Worth saying so.
There was a problem hiding this comment.
Confirmed against docs.mapbox.com and added: all five are scoped to mapbox/driving and mapbox/driving-traffic.
There was a problem hiding this comment.
Added the driving/driving-traffic scoping: "all five only available for mapbox/driving and mapbox/driving-traffic."
| in: query | ||
| required: false | ||
| description: >- | ||
| Departure time, ISO 8601 — for `mapbox/driving-traffic`, which |
There was a problem hiding this comment.
Nit: the Isochrone docs don't limit depart_at to mapbox/driving-traffic. They say the contours reflect traffic at that time, and it defaults to now.
There was a problem hiding this comment.
You're right, dropped the restriction and added the actual default (now, in the coordinates' own timezone).
There was a problem hiding this comment.
Dropped the driving-traffic restriction and described the real default/traffic behavior instead: "Departure time, ISO 8601, defaulting to now in the coordinates' own timezone, the contours reflect traffic conditions at this time."
| polygons. This response is a real GeoJSON `FeatureCollection`, unlike | ||
| `mapbox directions`'s response, but isochrone isn't one of the three | ||
| services (`search`, `geocoder`, `tilequery`) this CLI has a bespoke | ||
| list-per-feature rendering for yet (`output.rs`'s `list_rendering` is an |
There was a problem hiding this comment.
Nit: output.rs no longer exists. list_rendering is in src/output/render.rs.
There was a problem hiding this comment.
Fixed, now points at output/render.rs.
There was a problem hiding this comment.
Fixed, docs/commands.md now points at src/output/render.rs.
dfc6759 to
ed4fd38
Compare
Second of the Navigation-category APIs with no prior CLI coverage. Same shape as mapbox directions route (#43): hand-authored into custom-openapi/ since openapi-specs has no spec for this API either, and reuses that PR's fix for a spec parameter named `profile` colliding with the global --profile flag (ARG_NAME_OVERRIDES gets a second row, not a second mechanism). `contours_minutes` and `contours_meters` are mutually exclusive but neither is individually required by this CLI's own validation — same "not enforced before the request goes out" precedent search category already uses for its own proximity/near/bbox/route disjunction. The API answers 422 if both or neither are given. Smoke-tested against production: real contour polygons and linestrings for driving and walking profiles, --polygons, --contours-minutes with multiple values, verified to return the documented GeoJSON shape. Also fixes a self-inflicted --help regression found while writing this: first_sentence() in src/main.rs cuts a --help line at the first '.', and several profile-scoped parameter descriptions in directions.yaml (already merged in this branch) led with a complete sentence before the substantive content, e.g. "`mapbox/driving` only." — eating everything after it in --help. isochrone.yaml's own `denoise` had the same shape ("0.0-1.0: ...") and would have rendered as literally "0". Both fixed by moving the qualifier to the end of the description instead of the front. 485 tests, fmt and clippy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cascades the fix already shipped on mapbox directions: isochrone's API also has exactly one operation, so mapbox isochrone <args> replaces mapbox isochrone contours <args>, the same shape mapbox usage already has (spec::FLATTENED_SERVICES). And isochrone's own profile path parameter had the same closed four-value enum, which would reject an OEM account's undocumented profiles client-side; it's now free-form and reaches the URL unescaped via UNESCAPED_PATH_PARAMS, same mechanism as directions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
denoise was described as "a smaller value removes more of the smaller contours," which is the opposite of the real API: a larger value is more aggressive (1.0 keeps only the largest contour; 0.5 drops anything under half its area), confirmed against docs.mapbox.com/api/navigation/isochrone/. Also rewrote it to avoid a second bug: `0.0-1.0`/`1.0` still has a literal `.` that `first_sentence()` truncates on no matter where in the sentence it sits, so the earlier "move the qualifier to the end" fix didn't actually fix this one (confirmed live: --help rendered "...smaller contours, 0"). Spelled as "0 to 1"/"1" instead, with the exact 0.5 figure in a second sentence `--schema` and docs/commands.md still show in full. polygons now says a contour that doesn't form a ring stays a linestring either way, matching the real API rather than implying every contour becomes a polygon. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…c path generalize: dropped "smaller" — the docs say a higher value gives a coarser contour, not a smaller one. exclude: all five values are only available for mapbox/driving and mapbox/driving-traffic, confirmed against docs.mapbox.com, now says so. depart_at: the docs don't restrict it to mapbox/driving-traffic, and it defaults to now in the coordinates' own timezone when omitted — neither was in the description before. docs/commands.md also still pointed at output.rs, which moved to output/render.rs a while back. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
daf27f9 to
fcb18fe
Compare
Third of the Navigation-category APIs with no prior CLI coverage. Same shape as directions/isochrone (#43, #44): hand-authored into custom-openapi/ since openapi-specs has no spec for this API either, reusing ARG_NAME_OVERRIDES for the same profile-vs-global-flag collision (third row, not a third mechanism). Excludes POST, for the same documented reason directions route does: this spec format can't express "GET or POST, caller's choice" for one operationId, and the API's own POST exists specifically for a trace too long for a URL (~8100 bytes) — a real gap, not a design choice. Smoke-tested against production: a three-point San Francisco trace returned a real match with legs/steps/geometry, including a null tracepoint for a point too far from the road network to match — the documented shape for that case, not a bug. 486 tests, fmt and clippy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This reuses the flattening mechanism (
spec::FLATTENED_SERVICES),ARG_NAME_OVERRIDES, and thepath_segment_for/UNESCAPED_PATH_PARAMSfix #43 introduces, since both APIs'
profilepath parameter hits thesame global-flag collision and the same free-form-profile requirement.
Once #43 merges, this should be retargeted to
main(gh pr edit --base main) rather than reviewed against it as a diff. Everything below isscoped to what this PR actually adds on top of #43.
What
mapbox isochrone, the second Navigation-category API with no prior CLIcoverage. Same shape as #43: hand-authored into
custom-openapi/sinceopenapi-specs publishes no spec for this API either.
No subcommand: like
mapbox directions, this API has one operation, sothere's nothing a second word (the old
contours) would disambiguate.Same shape
mapbox usagealready has.How far you can get from a point in a given time or distance, for driving
(with or without live traffic), walking, or cycling, as GeoJSON polygons
or linestrings.
contours_minutesandcontours_metersare mutually exclusive but neitheris individually required by this CLI's own validation, the same "not
enforced before the request goes out" precedent
search categoryalreadyuses for its own proximity/near/bbox/route disjunction. The API answers 422
if both or neither are given.
Routing profile is free-form here too
Same fix as #43: an earlier version of this command validated
profileagainst the four documented values client-side, via a clap enum. Some OEM
accounts have additional profiles that aren't published, so that
validation would have broken this command for exactly the accounts that
most need it.
profileis now sent exactly as typed, and reaches the URLunescaped through
UNESCAPED_PATH_PARAMS(("isochrone", "profile"))rather than relying on the enum-implies-safe assumption that stopped
holding once the enum came off.
Fixed in review
denoise's description was backwards: it said a smaller value removesmore of the smaller contours, when the real API works the other way (a
larger value is more aggressive — 1.0 keeps only the largest contour,
0.5 drops anything under half its area), confirmed against
docs.mapbox.com/api/navigation/isochrone/. Also rewrote it to avoid a
second bug while I was in there:
0.0-1.0/1.0still has a literal.that
first_sentence()truncates on no matter where in the sentence itsits, so the "move the qualifier to the end" fix the directions PR (Add mapbox directions route #43)
used for its own eight descriptions doesn't actually fix this one —
confirmed live,
--helprendered "...smaller contours, 0". Spelled as"0 to 1"/"1" instead.
polygons's description implied every contour becomes a polygon; thereal API only does that for a contour that forms a ring, and says so
now.
(The eight
directions.yamldescription fixes mentioned in earlier revieware #43's own commit, inherited through this branch's stack — not
something this PR adds.)
Verification
Smoke-tested against production: real contour polygons and linestrings for
mapbox/drivingandmapbox/walking,--polygons, and--contours-minuteswith multiple values, all verified to return thedocumented GeoJSON shape, plus the bare
mapbox isochronecommand shapeitself (no subcommand accepted,
--schemareflects the new name).Full suite,
cargo fmt --checkandcargo clippy --all-targets -- -D warningsall clean.🤖 Generated with Claude Code