From 1955128a8ad1fa344c2130760318a36598d99bfb Mon Sep 17 00:00:00 2001 From: Wei Khang Chong Date: Fri, 11 Sep 2026 13:54:04 +0800 Subject: [PATCH 1/4] fix: route plan_route stops by resolved waypoints and add text-search fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit planRoute resolved every stop twice, with two different resolvers, and then zipped the results by array index: the display labels came from the Geocoding API, while the distances came from the Routes API independently re-resolving the same raw strings. When the two disagreed, the tool returned success and its legs described a different place than its own labels. Resolve each stop once and pass the result to computeRoutes as a structured place ID waypoint, falling back to coordinates when the resolved place has no place ID. Routes then routes to the same place the label names, so the two can no longer diverge. Google recommends place IDs over coordinates and address strings for exactly this reason: they are unambiguous and let Routes use the place's own access points, while coordinates are snapped to the nearest road. Coordinate stops keep the caller's exact point and no longer depend on the Geocoding API at all: the point needs no resolving, so geocoding is used only to fetch a display address and its failure is not fatal. A coordinate-shaped string outside valid latitude/longitude range now fails immediately, without an API call, instead of being text-searched. Also add a Places Text Search fallback for input Geocoding cannot resolve. The failing class is non-address free text, such as "精進湖 他根浜", which previously threw in both planRoute and exploreArea; landmark names geocode normally and are unaffected. computeRoutes takes optional originLabel/destinationLabel so its no-route and no-transit errors keep naming the stop the caller asked for rather than the resolved place ID. No tool schema changed, so the tool checklist in CLAUDE.md does not apply. --- src/services/PlacesSearcher.ts | 148 ++++++++++++++--- src/services/RoutesService.ts | 48 +++++- tests/locationResolution.unit.test.ts | 220 ++++++++++++++++++++++++++ tests/routesWaypoints.unit.test.ts | 162 +++++++++++++++++++ 4 files changed, 545 insertions(+), 33 deletions(-) create mode 100644 tests/locationResolution.unit.test.ts create mode 100644 tests/routesWaypoints.unit.test.ts diff --git a/src/services/PlacesSearcher.ts b/src/services/PlacesSearcher.ts index 9d33635..007017f 100644 --- a/src/services/PlacesSearcher.ts +++ b/src/services/PlacesSearcher.ts @@ -1,6 +1,13 @@ import { GoogleMapsTools } from "./toolclass.js"; import { NewPlacesService } from "./NewPlacesService.js"; -import { RoutesService, parseDuration, formatDistance, formatDuration } from "./RoutesService.js"; +import { + RoutesService, + parseDuration, + formatDistance, + formatDuration, + COORDINATE_STRING_PATTERN, + type RouteWaypoint, +} from "./RoutesService.js"; interface SearchResponse { success: boolean; @@ -100,6 +107,44 @@ interface ElevationResponse { }>; } +interface ResolvedLocation { + originalName: string; + address: string; + lat: number; + lng: number; + placeId: string; +} + +/** + * Parse a "latitude,longitude" input, or null when it is not one or is out of range. + */ +function parseCoordinateInput(input: string): { lat: number; lng: number } | null { + const match = input.match(COORDINATE_STRING_PATTERN); + if (!match) { + return null; + } + const lat = parseFloat(match[1]); + const lng = parseFloat(match[2]); + if (!Number.isFinite(lat) || !Number.isFinite(lng)) { + return null; + } + if (lat < -90 || lat > 90 || lng < -180 || lng > 180) { + return null; + } + return { lat, lng }; +} + +/** + * Place ID over coordinates: Routes snaps coordinates to the nearest road, which + * may not be an entrance. Text Search can return a result without a place ID. + */ +function toRouteWaypoint(loc: ResolvedLocation): RouteWaypoint { + if (loc.placeId) { + return { placeId: loc.placeId }; + } + return { latLng: { latitude: loc.lat, longitude: loc.lng } }; +} + export class PlacesSearcher { private mapsTools: GoogleMapsTools; private newPlacesService: NewPlacesService; @@ -449,6 +494,65 @@ export class PlacesSearcher { // --------------- Composite Tools --------------- + /** + * Resolve a location string to one concrete place, exactly once: geocoding + * first, then Places Text Search for informal names it does not index. + * + * Throws rather than returning { success: false } — the plan_route and + * explore_area actions JSON.stringify(result.data) without checking the flag, + * so a failure object would surface as an empty success. + */ + private async resolveLocation(input: string): Promise { + if (COORDINATE_STRING_PATTERN.test(input)) { + const coordinates = parseCoordinateInput(input); + if (!coordinates) { + throw new Error(`Failed to resolve location: ${input} (not a valid "latitude,longitude" pair)`); + } + // Already exact, so geocoding only supplies a display address and its failure + // must not fail the stop. No place ID: the nearest addressable place would + // silently move the caller's point. geocode() over reverseGeocode() because it + // labels sparse areas better — "Canada" rather than a bare Plus Code. + const displayGeocode = await this.geocode(input); + return { + originalName: input, + address: displayGeocode.success && displayGeocode.data ? displayGeocode.data.formatted_address : input, + lat: coordinates.lat, + lng: coordinates.lng, + placeId: "", + }; + } + + const geo = await this.geocode(input); + if (geo.success && geo.data) { + return { + originalName: input, + address: geo.data.formatted_address, + lat: geo.data.location.lat, + lng: geo.data.location.lng, + placeId: geo.data.place_id, + }; + } + + // Any geocoding failure falls through, not just zero results: geocode() flattens + // every error into { success: false } with no status code. A systemic failure + // (bad key, exhausted quota) fails the text search too, and both errors are thrown. + const textSearch = await this.searchText({ query: input }); + if (textSearch.success && textSearch.data && textSearch.data.length > 0) { + const topMatch = textSearch.data[0]; + return { + originalName: input, + address: topMatch.address || topMatch.name, + lat: topMatch.location.lat, + lng: topMatch.location.lng, + placeId: topMatch.place_id || "", + }; + } + + throw new Error( + `Failed to resolve location: ${input} (geocoding: ${geo.error || "no result"}; places text search: ${textSearch.error || "no result"})` + ); + } + async exploreArea(params: { location: string; types?: string[]; radius?: number; topN?: number }): Promise { // "tourist_attraction" is the Places API (New) type name; a bare // "attraction" is rejected with INVALID_ARGUMENT: Unsupported types. @@ -456,10 +560,9 @@ export class PlacesSearcher { const radius = params.radius || 1000; const topN = params.topN || 3; - // 1. Geocode - const geo = await this.geocode(params.location); - if (!geo.success || !geo.data) throw new Error(geo.error || "Geocode failed"); - const { lat, lng } = geo.data.location; + // 1. Resolve location + const resolved = await this.resolveLocation(params.location); + const { lat, lng } = resolved; // 2. Search each type const categories: any[] = []; @@ -493,7 +596,7 @@ export class PlacesSearcher { return { success: true, data: { - location: { address: geo.data.formatted_address, lat, lng }, + location: { address: resolved.address, lat, lng }, radius, categories, }, @@ -512,23 +615,16 @@ export class PlacesSearcher { const stops = params.stops; if (stops.length < 2) throw new Error("Need at least 2 stops"); - // 1. Geocode all stops for display addresses - const geocoded: Array<{ originalName: string; address: string; lat: number; lng: number }> = []; + // 1. Resolve all stops for display addresses and waypoints + const resolvedStops: ResolvedLocation[] = []; for (const stop of stops) { - const geo = await this.geocode(stop); - if (!geo.success || !geo.data) throw new Error(`Failed to geocode: ${stop}`); - geocoded.push({ - originalName: stop, - address: geo.data.formatted_address, - lat: geo.data.location.lat, - lng: geo.data.location.lng, - }); + resolvedStops.push(await this.resolveLocation(stop)); } // 2. Single Routes API call handles optimization + all leg directions - const origin = stops[0]; - const destination = stops[stops.length - 1]; - const intermediates = stops.length > 2 ? stops.slice(1, -1) : undefined; + const origin = toRouteWaypoint(resolvedStops[0]); + const destination = toRouteWaypoint(resolvedStops[resolvedStops.length - 1]); + const intermediates = stops.length > 2 ? resolvedStops.slice(1, -1).map((s) => toRouteWaypoint(s)) : undefined; // Optimize if requested, > 2 stops, and not transit (transit doesn't support intermediates for optimization) const shouldOptimize = params.optimize !== false && stops.length > 2 && mode !== "transit"; @@ -538,6 +634,8 @@ export class PlacesSearcher { mode, intermediates, optimizeWaypointOrder: shouldOptimize, + originLabel: stops[0], + destinationLabel: stops[stops.length - 1], ...(params.departure_time ? { departureTime: new Date(params.departure_time) } : {}), ...(params.avoid_tolls !== undefined ? { avoidTolls: params.avoid_tolls } : {}), ...(params.avoid_highways !== undefined ? { avoidHighways: params.avoid_highways } : {}), @@ -547,17 +645,17 @@ export class PlacesSearcher { const routeLegs = route?.legs || []; // 3. Determine ordered stops based on optimization result - let orderedStops: typeof geocoded; + let orderedStops: typeof resolvedStops; if (shouldOptimize && routeResult.optimizedIntermediateWaypointIndex) { const optimizedOrder = routeResult.optimizedIntermediateWaypointIndex; - const intermediateGeocoded = geocoded.slice(1, -1); + const intermediateResolved = resolvedStops.slice(1, -1); orderedStops = [ - geocoded[0], - ...optimizedOrder.map((i: number) => intermediateGeocoded[i]), - geocoded[geocoded.length - 1], + resolvedStops[0], + ...optimizedOrder.map((i: number) => intermediateResolved[i]), + resolvedStops[resolvedStops.length - 1], ]; } else { - orderedStops = geocoded; + orderedStops = resolvedStops; } // 4. Build legs from Routes API response diff --git a/src/services/RoutesService.ts b/src/services/RoutesService.ts index 6f6721d..52707f0 100644 --- a/src/services/RoutesService.ts +++ b/src/services/RoutesService.ts @@ -67,11 +67,36 @@ export function formatDuration(seconds: number): string { return `${mins} min${mins !== 1 ? "s" : ""}`; } +export const COORDINATE_STRING_PATTERN = /^\s*(-?\d+\.?\d*)\s*,\s*(-?\d+\.?\d*)\s*$/; + +export type RouteWaypoint = { latLng: { latitude: number; longitude: number } } | { placeId: string }; +export type WaypointInput = string | RouteWaypoint; + +/** + * Format a waypoint for human-readable error messages. + */ +function describeWaypoint(waypoint: WaypointInput): string { + if (typeof waypoint === "string") { + return waypoint; + } + if ("placeId" in waypoint) { + return `placeId:${waypoint.placeId}`; + } + return `${waypoint.latLng.latitude},${waypoint.latLng.longitude}`; +} + /** - * Convert address/coordinates string to Routes API Waypoint. + * Convert address/coordinates string or structured waypoint to Routes API Waypoint. + * + * NOTE: must stay synchronous and take exactly one argument. computeRoutes below + * calls params.intermediates.map(toWaypoint), so a second parameter would silently + * bind the array index, and returning a promise would serialise as {}. */ -function toWaypoint(location: string): any { - const coordMatch = location.match(/^\s*(-?\d+\.?\d*)\s*,\s*(-?\d+\.?\d*)\s*$/); +function toWaypoint(location: WaypointInput): any { + if (typeof location !== "string") { + return "placeId" in location ? { placeId: location.placeId } : { location: { latLng: location.latLng } }; + } + const coordMatch = location.match(COORDINATE_STRING_PATTERN); if (coordMatch) { return { location: { @@ -127,15 +152,18 @@ export class RoutesService { * Returns response compatible with existing DirectionsResponse.data interface. */ async computeRoutes(params: { - origin: string; - destination: string; + origin: WaypointInput; + destination: WaypointInput; mode?: string; departureTime?: Date; arrivalTime?: Date; - intermediates?: string[]; + intermediates?: WaypointInput[]; optimizeWaypointOrder?: boolean; avoidTolls?: boolean; avoidHighways?: boolean; + /** What to call the endpoints in error messages. Defaults to the waypoints themselves. */ + originLabel?: string; + destinationLabel?: string; }): Promise<{ routes: any[]; summary: string; @@ -208,12 +236,16 @@ export class RoutesService { const mode = params.mode || "driving"; if (mode === "transit") { throw new Error( - `No transit route found from "${params.origin}" to "${params.destination}". ` + + `No transit route found from "${params.originLabel ?? describeWaypoint(params.origin)}" ` + + `to "${params.destinationLabel ?? describeWaypoint(params.destination)}". ` + `The Google Routes API does not support transit directions in some regions (notably Japan and India). ` + `Try using mode "driving" or "walking" instead, or use a regional transit service for public transportation details.` ); } - throw new Error(`No route found from "${params.origin}" to "${params.destination}" with mode: ${mode}`); + throw new Error( + `No route found from "${params.originLabel ?? describeWaypoint(params.origin)}" ` + + `to "${params.destinationLabel ?? describeWaypoint(params.destination)}" with mode: ${mode}` + ); } const route = data.routes[0]; diff --git a/tests/locationResolution.unit.test.ts b/tests/locationResolution.unit.test.ts new file mode 100644 index 0000000..a1056a4 --- /dev/null +++ b/tests/locationResolution.unit.test.ts @@ -0,0 +1,220 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { PlacesSearcher } from "../src/services/PlacesSearcher.js"; + +const FIXTURES: Record< + string, + { + geocode?: { location: { lat: number; lng: number }; formatted_address: string; place_id: string }; + places?: { location: { lat: number; lng: number }; address: string; place_id: string; name: string }; + } +> = { + "Tokyo Tower": { + geocode: { + location: { lat: 35.6585805, lng: 139.7454329 }, + formatted_address: "Tokyo Tower, Tokyo, Japan", + place_id: "place-tokyo-tower", + }, + }, + 河口湖駅: { + geocode: { + location: { lat: 35.4985, lng: 138.769 }, + formatted_address: "Kawaguchiko Station, Yamanashi, Japan", + place_id: "place-kawaguchiko", + }, + }, + "35.6585805,139.7454329": { + geocode: { + location: { lat: 35.6586, lng: 139.7455 }, + formatted_address: "4-chome-2-8 Shibakoen, Minato City, Tokyo", + place_id: "place-nearest-street-address", + }, + }, + "Place Without Id": { + places: { + location: { lat: 1.23, lng: 4.56 }, + address: "Somewhere with no place ID", + place_id: "", + name: "Somewhere with no place ID", + }, + }, + "精進湖 他根浜": { + places: { + location: { lat: 35.4906841, lng: 138.6046066 }, + address: "Tatego-Hama Beach, Fujikawaguchiko", + place_id: "ChIJ13ziPsznG2ARIqd4uJcjN0s", + name: "Tatego-Hama Beach", + }, + }, +}; + +function createSearcher(captured: { textQueries: string[]; routeParams: any; nearbyCenters: string[] }) { + const searcher = new PlacesSearcher("test-api-key"); + + searcher.geocode = async (address: string) => { + const fixture = FIXTURES[address]; + if (fixture?.geocode) { + return { success: true, data: fixture.geocode }; + } + return { success: false, error: `No location found for: ${address}` }; + }; + + searcher.searchText = async (params: { query: string }) => { + captured.textQueries.push(params.query); + const fixture = FIXTURES[params.query]; + if (fixture?.places) { + return { success: true, data: [fixture.places] }; + } + return { success: true, data: [] }; + }; + + (searcher as unknown as { routesService: any }).routesService = { + computeRoutes: async (params: any) => { + captured.routeParams = params; + return { + routes: [ + { + description: "Mock Route", + distanceMeters: 1000, + duration: "600s", + legs: [ + { distanceMeters: 500, duration: "300s" }, + { distanceMeters: 500, duration: "300s" }, + ], + }, + ], + }; + }, + }; + + searcher.searchNearby = async (params: any) => { + if (params.center?.value) { + captured.nearbyCenters.push(params.center.value); + } + return { success: true, data: [] }; + }; + + return searcher; +} + +test("planRoute sends resolved waypoints, not raw strings", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + await searcher.planRoute({ stops: ["Tokyo Tower", "河口湖駅"] }); + + assert.deepEqual(captured.routeParams.origin, { placeId: "place-tokyo-tower" }); + assert.deepEqual(captured.routeParams.destination, { placeId: "place-kawaguchiko" }); +}); + +test("planRoute routes to the same place it labels", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + const result = await searcher.planRoute({ + stops: ["Tokyo Tower", "精進湖 他根浜", "河口湖駅"], + optimize: false, + }); + + assert.deepEqual(captured.routeParams.intermediates, [{ placeId: "ChIJ13ziPsznG2ARIqd4uJcjN0s" }]); + assert.equal(result.data.stops[1], "精進湖 他根浜 (Tatego-Hama Beach, Fujikawaguchiko)"); +}); + +test("planRoute falls back to Places Text Search when geocoding finds nothing", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + await searcher.planRoute({ stops: ["Tokyo Tower", "精進湖 他根浜"] }); + + assert.deepEqual(captured.textQueries, ["精進湖 他根浜"]); +}); + +test("planRoute throws when neither geocoding nor text search resolves a stop", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + await assert.rejects( + async () => { + await searcher.planRoute({ stops: ["Tokyo Tower", "NonExistentUnknownPlace123"] }); + }, + (err: Error) => { + return err.message.includes("NonExistentUnknownPlace123"); + } + ); +}); + +test("planRoute rejects unusable coordinates without calling any API", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + let geocodeCalls = 0; + searcher.geocode = async () => { + geocodeCalls += 1; + return { success: false, error: "should not be called" }; + }; + + // Out of range, so it is not a usable point. It must not reach Text Search + // either, which would happily return an unrelated place for the numbers. + await assert.rejects( + async () => { + await searcher.planRoute({ stops: ["99.9,99.9", "Tokyo Tower"] }); + }, + (err: Error) => err.message.includes("99.9,99.9") + ); + assert.equal(geocodeCalls, 0); + assert.deepEqual(captured.textQueries, []); +}); + +test("exploreArea falls back to Places Text Search when geocoding finds nothing", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + const result = await searcher.exploreArea({ location: "精進湖 他根浜" }); + + assert.deepEqual(captured.nearbyCenters, [ + "35.4906841,138.6046066", + "35.4906841,138.6046066", + "35.4906841,138.6046066", + ]); + assert.equal(result.data.location.address, "Tatego-Hama Beach, Fujikawaguchiko"); + assert.equal(result.data.location.lat, 35.4906841); + assert.equal(result.data.location.lng, 138.6046066); +}); + +test("planRoute falls back to coordinates when a resolved place has no place ID", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + await searcher.planRoute({ stops: ["Tokyo Tower", "Place Without Id"] }); + + assert.deepEqual(captured.routeParams.destination, { + latLng: { latitude: 1.23, longitude: 4.56 }, + }); +}); + +test("planRoute routes coordinate stops through the caller's exact point", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + + await searcher.planRoute({ stops: ["35.6585805,139.7454329", "河口湖駅"] }); + + // Not { placeId: "place-nearest-street-address" }: geocoding a coordinate + // only supplies a display address, it must not move the waypoint. + assert.deepEqual(captured.routeParams.origin, { + latLng: { latitude: 35.6585805, longitude: 139.7454329 }, + }); +}); + +test("planRoute routes coordinate stops even when geocoding is unavailable", async () => { + const captured = { textQueries: [], routeParams: null as any, nearbyCenters: [] }; + const searcher = createSearcher(captured); + searcher.geocode = async () => ({ success: false, error: "Geocoding API has not been used in project" }); + + const result = await searcher.planRoute({ stops: ["35.6585805,139.7454329", "35.7147651,139.7966553"] }); + + assert.deepEqual(captured.routeParams.origin, { + latLng: { latitude: 35.6585805, longitude: 139.7454329 }, + }); + // No display address available, so the input stands in for it. + assert.equal(result.data.stops[0], "35.6585805,139.7454329 (35.6585805,139.7454329)"); + assert.deepEqual(captured.textQueries, []); +}); diff --git a/tests/routesWaypoints.unit.test.ts b/tests/routesWaypoints.unit.test.ts new file mode 100644 index 0000000..64c6bee --- /dev/null +++ b/tests/routesWaypoints.unit.test.ts @@ -0,0 +1,162 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { RoutesService } from "../src/services/RoutesService.js"; + +const MOCK_ROUTES_RESPONSE = { + routes: [ + { + description: "Route description", + distanceMeters: 5000, + duration: "600s", + legs: [{ distanceMeters: 5000, duration: "600s" }], + }, + ], +}; + +const MOCK_MATRIX_RESPONSE = [ + { + originIndex: 0, + destinationIndex: 0, + distanceMeters: 1000, + duration: "120s", + status: {}, + }, +]; + +test("structured waypoints forward unchanged", async () => { + const originalFetch = globalThis.fetch; + let capturedBody: any = null; + + globalThis.fetch = async (_url: any, init: any) => { + capturedBody = JSON.parse(init.body); + return new Response(JSON.stringify(MOCK_ROUTES_RESPONSE), { status: 200 }); + }; + + try { + const service = new RoutesService("test-api-key"); + await service.computeRoutes({ + origin: { latLng: { latitude: 35.6585, longitude: 139.7454 } }, + destination: { placeId: "test-dest-place-id" }, + intermediates: [{ latLng: { latitude: 35.5, longitude: 138.7 } }, { placeId: "test-intermediate-place-id" }], + }); + + assert.deepEqual(capturedBody.origin, { + location: { latLng: { latitude: 35.6585, longitude: 139.7454 } }, + }); + assert.deepEqual(capturedBody.destination, { + placeId: "test-dest-place-id", + }); + assert.deepEqual(capturedBody.intermediates, [ + { location: { latLng: { latitude: 35.5, longitude: 138.7 } } }, + { placeId: "test-intermediate-place-id" }, + ]); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("plain strings still work", async () => { + const originalFetch = globalThis.fetch; + let capturedBody: any = null; + + globalThis.fetch = async (_url: any, init: any) => { + capturedBody = JSON.parse(init.body); + return new Response(JSON.stringify(MOCK_ROUTES_RESPONSE), { status: 200 }); + }; + + try { + const service = new RoutesService("test-api-key"); + await service.computeRoutes({ + origin: "Tokyo Tower", + destination: "35.6,139.7", + }); + + assert.deepEqual(capturedBody.origin, { address: "Tokyo Tower" }); + assert.deepEqual(capturedBody.destination, { + location: { latLng: { latitude: 35.6, longitude: 139.7 } }, + }); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("computeRouteMatrix still works with strings", async () => { + const originalFetch = globalThis.fetch; + let capturedBody: any = null; + + globalThis.fetch = async (_url: any, init: any) => { + capturedBody = JSON.parse(init.body); + return new Response(JSON.stringify(MOCK_MATRIX_RESPONSE), { status: 200 }); + }; + + try { + const service = new RoutesService("test-api-key"); + await service.computeRouteMatrix({ + origins: ["Point A"], + destinations: ["Point B"], + }); + + assert.deepEqual(capturedBody.origins, [{ waypoint: { address: "Point A" } }]); + assert.deepEqual(capturedBody.destinations, [{ waypoint: { address: "Point B" } }]); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("no-route error describes structured waypoints", async () => { + const originalFetch = globalThis.fetch; + + globalThis.fetch = async () => { + return new Response(JSON.stringify({ routes: [] }), { status: 200 }); + }; + + try { + const service = new RoutesService("test-api-key"); + await assert.rejects( + async () => { + await service.computeRoutes({ + origin: { latLng: { latitude: 35.6585, longitude: 139.7454 } }, + destination: { placeId: "test-place-123" }, + }); + }, + (err: Error) => { + assert.equal(err.message.includes("[object Object]"), false); + assert.equal(err.message.includes("35.6585,139.7454"), true); + assert.equal(err.message.includes("placeId:test-place-123"), true); + return true; + } + ); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("no-route error prefers caller-supplied labels over waypoints", async () => { + const originalFetch = globalThis.fetch; + + globalThis.fetch = async () => { + return new Response(JSON.stringify({ routes: [] }), { status: 200 }); + }; + + try { + const service = new RoutesService("test-api-key"); + await assert.rejects( + async () => { + await service.computeRoutes({ + origin: { placeId: "test-origin-123" }, + destination: { placeId: "test-dest-456" }, + originLabel: "Tokyo Station", + destinationLabel: "Shibuya Station", + }); + }, + (err: Error) => { + assert.equal(err.message.includes("Tokyo Station"), true); + assert.equal(err.message.includes("Shibuya Station"), true); + assert.equal(err.message.includes("test-origin-123"), false); + return true; + } + ); + } finally { + globalThis.fetch = originalFetch; + } +}); From 47ccba13fea02d43bbf0cda8886c2e62db862ee5 Mon Sep 17 00:00:00 2001 From: Andrei Kazakou Date: Wed, 2 Sep 2026 17:10:09 -0500 Subject: [PATCH 2/4] fix: require at least 2 intermediates for waypoint optimization 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. --- src/services/PlacesSearcher.ts | 4 +- src/services/RoutesService.ts | 4 +- src/tools/maps/planRoute.ts | 4 +- tests/planRoute.unit.test.ts | 106 +++++++++++++++++++++++++++++++++ tests/smoke.test.ts | 89 +++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 6 deletions(-) create mode 100644 tests/planRoute.unit.test.ts diff --git a/src/services/PlacesSearcher.ts b/src/services/PlacesSearcher.ts index 007017f..5bc06a4 100644 --- a/src/services/PlacesSearcher.ts +++ b/src/services/PlacesSearcher.ts @@ -625,8 +625,8 @@ export class PlacesSearcher { const origin = toRouteWaypoint(resolvedStops[0]); const destination = toRouteWaypoint(resolvedStops[resolvedStops.length - 1]); const intermediates = stops.length > 2 ? resolvedStops.slice(1, -1).map((s) => toRouteWaypoint(s)) : undefined; - // Optimize if requested, > 2 stops, and not transit (transit doesn't support intermediates for optimization) - const shouldOptimize = params.optimize !== false && stops.length > 2 && mode !== "transit"; + // Optimize if requested, > 3 stops (at least 2 intermediates), and not transit (transit doesn't support intermediates for optimization) + const shouldOptimize = params.optimize !== false && stops.length > 3 && mode !== "transit"; const routeResult = await this.routesService.computeRoutes({ origin, diff --git a/src/services/RoutesService.ts b/src/services/RoutesService.ts index 52707f0..7921e63 100644 --- a/src/services/RoutesService.ts +++ b/src/services/RoutesService.ts @@ -204,11 +204,11 @@ export class RoutesService { requestBody.intermediates = params.intermediates.map(toWaypoint); } - // Waypoint optimization (not supported for TRANSIT) + // Waypoint optimization (not supported for TRANSIT, requires at least 2 intermediates) if ( params.optimizeWaypointOrder && params.intermediates && - params.intermediates.length > 0 && + params.intermediates.length > 1 && travelMode !== "TRANSIT" ) { requestBody.optimizeWaypointOrder = true; diff --git a/src/tools/maps/planRoute.ts b/src/tools/maps/planRoute.ts index 10a2a92..43e93aa 100644 --- a/src/tools/maps/planRoute.ts +++ b/src/tools/maps/planRoute.ts @@ -4,7 +4,7 @@ import { getCurrentApiKey } from "../../utils/requestContext.js"; const NAME = "maps_plan_route"; const DESCRIPTION = - "Plan an optimized multi-stop route in one call — geocodes all stops, uses Routes API waypoint optimization (up to 25 intermediate stops) to find the most efficient visit order, and returns directions for each leg. Use when the user says 'visit these 5 places efficiently', 'plan a route through A, B, C', or needs a multi-stop itinerary. Replaces the manual chain of geocode → distance-matrix → directions. For multi-day trips: create one plan_route call per day with stops that follow a geographic arc (e.g. east→west) rather than mixing distant areas. After results, call static_map to visualize the route."; + "Plan an optimized multi-stop route in one call — geocodes all stops, uses Routes API waypoint optimization (2 to 25 intermediate stops) to find the most efficient visit order, and returns directions for each leg. Use when the user says 'visit these 5 places efficiently', 'plan a route through A, B, C', or needs a multi-stop itinerary. Replaces the manual chain of geocode → distance-matrix → directions. Waypoint optimization requires at least 4 stops (2 intermediates); with 2 or 3 stops the route is returned in the original order. For multi-day trips: create one plan_route call per day with stops that follow a geographic arc (e.g. east→west) rather than mixing distant areas. After results, call static_map to visualize the route."; const SCHEMA = { stops: z.array(z.string()).min(2).describe("List of addresses or landmarks to visit (minimum 2)"), @@ -13,7 +13,7 @@ const SCHEMA = { .boolean() .optional() .describe( - "Auto-optimize visit order via Routes API waypoint optimization (default: true). Set false to keep original order. Not available for transit mode." + "Auto-optimize visit order via Routes API waypoint optimization (default: true). Requires at least 4 stops (2 intermediates) — ignored for 2-3 stops. Set false to keep original order. Not available for transit mode." ), departure_time: z .string() diff --git a/tests/planRoute.unit.test.ts b/tests/planRoute.unit.test.ts new file mode 100644 index 0000000..50c3482 --- /dev/null +++ b/tests/planRoute.unit.test.ts @@ -0,0 +1,106 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { PlacesSearcher } from "../src/services/PlacesSearcher.js"; + +interface RouteStub { + optimizedIntermediateWaypointIndex?: number[]; + legCount: number; +} + +/** + * Build a PlacesSearcher whose network dependencies are stubbed so planRoute + * can be exercised without hitting the Geocoding or Routes APIs. + */ +function createSearcher(routeStub: RouteStub): PlacesSearcher { + const searcher = new PlacesSearcher("test-api-key"); + + // Deterministic geocode: echo the requested stop as originalName. + searcher.geocode = async (address: string) => ({ + success: true, + data: { + location: { lat: 0, lng: 0 }, + formatted_address: `${address} (formatted)`, + place_id: `place-${address}`, + }, + }); + + // Stub the Routes API response. + ( + searcher as unknown as { routesService: { computeRoutes: (p: unknown) => Promise } } + ).routesService.computeRoutes = async () => ({ + routes: [ + { + legs: Array.from({ length: routeStub.legCount }, () => ({ + distanceMeters: 1000, + duration: "600s", + })), + }, + ], + summary: "", + total_distance: { value: 0, text: "" }, + total_duration: { value: 0, text: "" }, + arrival_time: "", + departure_time: "", + ...(routeStub.optimizedIntermediateWaypointIndex + ? { optimizedIntermediateWaypointIndex: routeStub.optimizedIntermediateWaypointIndex } + : {}), + }); + + return searcher; +} + +test("planRoute handles 2 stops (optimize: true, no intermediates)", async () => { + const searcher = createSearcher({ legCount: 1 }); + const result = await searcher.planRoute({ stops: ["A", "B"] }); + + assert.equal(result.data.optimized, false); // <= 3 stops => no optimization + assert.deepEqual(result.data.stops, ["A (A (formatted))", "B (B (formatted))"]); + assert.equal(result.data.legs.length, 1); +}); + +test("planRoute handles 2 stops (optimize: false)", async () => { + const searcher = createSearcher({ legCount: 1 }); + const result = await searcher.planRoute({ stops: ["A", "B"], optimize: false }); + + assert.equal(result.data.optimized, false); + assert.equal(result.data.legs.length, 1); +}); + +test("planRoute handles 3 stops (optimize: true) — skips optimization (too few intermediates)", async () => { + // Optimization is skipped because it requires at least 4 stops (2 intermediates) + const searcher = createSearcher({ legCount: 2 }); + const result = await searcher.planRoute({ stops: ["A", "B", "C"], optimize: true }); + + assert.equal(result.data.optimized, false); + assert.deepEqual( + result.data.stops.map((s: string) => s.split(" (")[0]), + ["A", "B", "C"] + ); + assert.equal(result.data.legs.length, 2); +}); + +test("planRoute handles 3 stops (optimize: false) — keeps original order", async () => { + const searcher = createSearcher({ legCount: 2 }); + const result = await searcher.planRoute({ stops: ["A", "B", "C"], optimize: false }); + + assert.equal(result.data.optimized, false); + assert.deepEqual( + result.data.stops.map((s: string) => s.split(" (")[0]), + ["A", "B", "C"] + ); +}); + +test("planRoute applies a valid optimized waypoint order (4 stops / 2 intermediates)", async () => { + const searcher = createSearcher({ legCount: 3, optimizedIntermediateWaypointIndex: [1, 0] }); + const result = await searcher.planRoute({ + stops: ["Start", "I0", "I1", "End"], + optimize: true, + }); + + assert.equal(result.data.optimized, true); + assert.deepEqual( + result.data.stops.map((s: string) => s.split(" (")[0]), + ["Start", "I1", "I0", "End"] + ); + assert.equal(result.data.legs.length, 3); +}); diff --git a/tests/smoke.test.ts b/tests/smoke.test.ts index ce32e5a..6b23508 100644 --- a/tests/smoke.test.ts +++ b/tests/smoke.test.ts @@ -1148,6 +1148,94 @@ async function testTransitDetailsField(session: McpSession): Promise { ); } +async function testPlanRoute(session: McpSession): Promise { + console.log("\n🧪 Test 9: Plan route (waypoint optimization thresholds)"); + + if (!API_KEY) { + console.log(" ⏭️ Skipped (no GOOGLE_MAPS_API_KEY)"); + return; + } + + // Case A: 3 stops (1 intermediate) — optimization must be bypassed. + // Regression guard: the Routes API returns an unusable + // optimizedIntermediateWaypointIndex of [-1] for a single intermediate, which + // previously crashed with "Cannot read properties of undefined (reading 'originalName')". + const threeStops = ["Shibuya Station, Tokyo", "Tokyo Tower", "Shinjuku Station, Tokyo"]; + const threeStopResult = await sendRequest(session, "tools/call", { + name: "maps_plan_route", + arguments: { stops: threeStops, mode: "driving", optimize: true }, + }); + + const threeContent = threeStopResult?.result?.content ?? []; + assert(threeContent.length > 0, "plan_route (3 stops) returns content"); + + if (threeContent.length > 0) { + const text = threeContent[0]?.text ?? ""; + const isError = threeStopResult?.result?.isError === true; + + assert(!isError, "plan_route (3 stops, optimize: true) succeeds", `got: ${text.slice(0, 200)}`); + assert(!text.includes("originalName"), "plan_route (3 stops) does not surface an originalName crash"); + + if (!isError) { + let parsed: any; + try { + parsed = JSON.parse(text); + } catch { + assert(false, "plan_route (3 stops) returns valid JSON", `got: ${text.slice(0, 200)}`); + return; + } + + assert(parsed?.optimized === false, "plan_route (3 stops) skips optimization", `optimized: ${parsed?.optimized}`); + assert(parsed?.stops?.length === 3, "plan_route (3 stops) returns 3 stops", `got: ${parsed?.stops?.length}`); + assert(parsed?.legs?.length === 2, "plan_route (3 stops) returns 2 legs", `got: ${parsed?.legs?.length}`); + assert( + threeStops.every((stop, i) => (parsed?.stops?.[i] ?? "").startsWith(stop)), + "plan_route (3 stops) preserves the original stop order", + `got: ${JSON.stringify(parsed?.stops)}` + ); + } + } + + // Case B: 4 stops (2 intermediates) — optimization must engage. + const fourStops = ["Tokyo Station", "Ueno Park", "Asakusa", "Shibuya Crossing"]; + const fourStopResult = await sendRequest(session, "tools/call", { + name: "maps_plan_route", + arguments: { stops: fourStops, mode: "driving", optimize: true }, + }); + + const fourContent = fourStopResult?.result?.content ?? []; + assert(fourContent.length > 0, "plan_route (4 stops) returns content"); + if (fourContent.length === 0) return; + + const fourText = fourContent[0]?.text ?? ""; + const fourIsError = fourStopResult?.result?.isError === true; + assert(!fourIsError, "plan_route (4 stops, optimize: true) succeeds", `got: ${fourText.slice(0, 200)}`); + if (fourIsError) return; + + let fourParsed: any; + try { + fourParsed = JSON.parse(fourText); + } catch { + assert(false, "plan_route (4 stops) returns valid JSON", `got: ${fourText.slice(0, 200)}`); + return; + } + + assert( + fourParsed?.optimized === true, + "plan_route (4 stops) applies waypoint optimization", + `optimized: ${fourParsed?.optimized}` + ); + assert(fourParsed?.stops?.length === 4, "plan_route (4 stops) returns 4 stops", `got: ${fourParsed?.stops?.length}`); + assert(fourParsed?.legs?.length === 3, "plan_route (4 stops) returns 3 legs", `got: ${fourParsed?.legs?.length}`); + // The optimized order is decided by Google, so only assert every stop survived + // the geocode → optimized-order remap (no dropped or undefined entries). + assert( + fourStops.every((stop) => (fourParsed?.stops ?? []).some((s: string) => s.startsWith(stop))), + "plan_route (4 stops) retains every requested stop after optimization", + `got: ${JSON.stringify(fourParsed?.stops)}` + ); +} + // --------------- Main --------------- async function main() { @@ -1173,6 +1261,7 @@ async function main() { await testPlaceDetailsPhotos(session); await testTransitErrorMessages(session); await testTransitDetailsField(session); + await testPlanRoute(session); await testMultiSession(); } catch (err) { console.error("\n💥 Fatal error:", err); From 33fc73cfb07c5b45a0c8602ac14e4f98b2573f6a Mon Sep 17 00:00:00 2001 From: CabLate <85614048+cablate@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:00:43 +0800 Subject: [PATCH 3/4] test: cover waypoint threshold in CI --- .github/workflows/ci.yml | 1 + tests/routesWaypoints.unit.test.ts | 31 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f43365f..c66c914 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,7 @@ jobs: - run: npm run build - run: npm run lint - run: npm run format:check + - run: npm run test:unit - name: Smoke Test run: npm test diff --git a/tests/routesWaypoints.unit.test.ts b/tests/routesWaypoints.unit.test.ts index 64c6bee..9cd2569 100644 --- a/tests/routesWaypoints.unit.test.ts +++ b/tests/routesWaypoints.unit.test.ts @@ -55,6 +55,37 @@ test("structured waypoints forward unchanged", async () => { } }); +test("waypoint optimization requires at least two intermediates", async () => { + const originalFetch = globalThis.fetch; + const capturedBodies: any[] = []; + + globalThis.fetch = async (_url: any, init: any) => { + capturedBodies.push(JSON.parse(init.body)); + return new Response(JSON.stringify(MOCK_ROUTES_RESPONSE), { status: 200 }); + }; + + try { + const service = new RoutesService("test-api-key"); + await service.computeRoutes({ + origin: { placeId: "origin" }, + destination: { placeId: "destination" }, + intermediates: [{ placeId: "only-intermediate" }], + optimizeWaypointOrder: true, + }); + await service.computeRoutes({ + origin: { placeId: "origin" }, + destination: { placeId: "destination" }, + intermediates: [{ placeId: "intermediate-1" }, { placeId: "intermediate-2" }], + optimizeWaypointOrder: true, + }); + + assert.equal(capturedBodies[0].optimizeWaypointOrder, undefined); + assert.equal(capturedBodies[1].optimizeWaypointOrder, true); + } finally { + globalThis.fetch = originalFetch; + } +}); + test("plain strings still work", async () => { const originalFetch = globalThis.fetch; let capturedBody: any = null; From d38344dbd2ff5360f34571f8763e5526045a76b7 Mon Sep 17 00:00:00 2001 From: CabLate <85614048+cablate@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:02:37 +0800 Subject: [PATCH 4/4] test: skip live place photos without API key --- tests/smoke.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/smoke.test.ts b/tests/smoke.test.ts index 6b23508..ca6f5c6 100644 --- a/tests/smoke.test.ts +++ b/tests/smoke.test.ts @@ -598,6 +598,11 @@ async function testToolCalls(session: McpSession): Promise { async function testPlaceDetailsPhotos(session: McpSession): Promise { console.log("\n🧪 Test 4b: Place details with photos"); + if (!API_KEY) { + console.log(" ⏭️ Skipped (no GOOGLE_MAPS_API_KEY)"); + return; + } + // First search for a place to get a place_id const searchResult = await sendRequest(session, "tools/call", { name: "maps_search_places",