fix: require at least 2 intermediates for waypoint optimization - #86
Open
akazakou wants to merge 1 commit into
Open
fix: require at least 2 intermediates for waypoint optimization#86akazakou wants to merge 1 commit into
akazakou wants to merge 1 commit into
Conversation
The Routes API rejects waypoint optimization requests with a single intermediate stop, so only enable `optimizeWaypointOrder` when there are more than 3 stops (2+ intermediates). Also add unit tests for `planRoute` covering 2, 3 and more stops with optimization enabled and disabled, using stubbed geocoding and Routes API responses.
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.
Summary
maps_plan_routecrashing withCannot read properties of undefined (reading 'originalName')on every 3-stop route withoptimize: true(reported as completely unusable).optimizedIntermediateWaypointIndex: [-1]. Indexing the geocoded stops with-1yieldedundefined, which then crashed on.originalName.Changes
src/services/PlacesSearcher.tsplanRoute():shouldOptimizenow requiresstops.length > 3(was> 2), so optimization is only requested with 2+ intermediates.src/services/RoutesService.tscomputeRoutes(): guard changed toparams.intermediates.length > 1(was> 0) before settingoptimizeWaypointOrder— enforces the constraint at the API-client layer too.src/tools/maps/planRoute.tsDESCRIPTIONnow reads "2 to 25 intermediate stops" and states optimization needs 4+ stops (2 intermediates), with 2–3 stops keeping the original order;optimizeparam.describe()updated to match.tests/planRoute.unit.test.tsgeocode+computeRoutes) covering 2 stops (optimize true/false), 3 stops (optimize true/false), and 4 stops with a valid optimized order.tests/smoke.test.tsmaps_plan_routeinvocation: 3-stop regression case (succeeds, nooriginalName,optimized: false, original order) and 4-stop case (optimized: true, all stops retained). Registered inmain().Test plan
npm run buildsucceedsnpm run lint— 0 errors (177 pre-existing warnings; none new, noanyadded)npm test— 211 passed, 0 failed (full live smoke suite; was 198 before, +13 new assertions)Additional checks:
npm run format:check— passes (this was failing on the new unit test file and would have broken CI; fixed with Prettier)npm run test:unit— 7/7 passVerified the new smoke test actually catches the bug. Temporarily reverting both parts of the fix and re-running produces:
With the fix in place, live API behavior is correct:
Changelog entry
Notes for reviewers
Behavior change:
optimize: trueon a 2- or 3-stop route is now silently ignored (returnsoptimized: false) instead of crashing. The response'soptimizedflag is the source of truth for whether reordering actually happened.CI cost: Test 9 adds 2
plan_routecalls per run (7 geocodes + 2 route computations). Waypoint optimization bills on the higher Compute Routes Pro SKU — small, but it runs on every PR and every push tomain.