From 9018d4f16baaf508cd76e0ea74cc0360bb2e19b9 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Thu, 27 Aug 2026 09:42:26 -0400 Subject: [PATCH 1/2] fix: directions_tool turn-by-turn instructions never populated The tool has always requested steps=true and documented its response as including "turn-by-turn instructions", but the extraction read step.voiceInstructions, which the Directions API only returns when the separate voice_instructions=true parameter is set -- never sent by this tool. So `instructions` was silently empty for every call. Switched to step.maneuver.instruction, a human-readable instruction string already present on every step whenever steps=true is set, no extra parameter needed. Raised the output cap from 10 to 200: the old cap was sized for the old source (which could emit several announcements per turn), while maneuver.instruction is one entry per real turn, so a normal route -- even a long one with dozens of turns -- stays well under it. Confirmed live against the real Directions API. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 2 + .../directions-tool/cleanResponseData.ts | 42 +++++---- .../directions-tool/cleanResponseData.test.ts | 91 +++++++++++++------ 3 files changed, 93 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 497f0780..2b117d0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ - **`render_map_tool`: `baseStyle` (e.g. `"standard-satellite"`) was silently ignored on every real call.** `baseStyle` was only ever applied at map-construction time in `mapAppHtml.ts`'s `initMap()`, seeded from a server-embedded `#initial-data` script tag — but `MapAppUIResource` never actually passes `initialData` when serving that HTML, so the tag is always empty and the map always constructs with the default `"standard"` style. The real payload (with `baseStyle`) arrives afterward via the `ui/notifications/tool-result` → `render(payload)` path, which only ever handled `baseMapConfig` (color/theme properties via `setConfigProperty`), never `baseStyle` itself. Caught live: asking for a `"standard-satellite"` render produced a map correctly labeled "satellite view" that was actually still the standard vector style. `render()` now detects a `baseStyle` change from what the map is currently on and switches via `map.setStyle(...)`, deferring the rest of the render until `style.load` fires (matching how a style is normally swapped in GL JS, since it replaces sources/layers). Added regression tests reproducing the original silent-ignore bug (confirmed to fail without the fix) and covering the no-op case when a render repeats the already-active style. +- **`directions_tool`: turn-by-turn `instructions` now actually populate.** The tool has always requested `steps=true` and documented its response as including "turn-by-turn instructions," but the extraction code read announcements from `step.voiceInstructions`, which the Directions API only returns when the separate `voice_instructions=true` parameter is set — never sent by this tool. So `instructions` was silently empty for every call, regardless of route length. Switched to `step.maneuver.instruction` (e.g. `"Bear right onto Great Portland Street/A4201. Continue on A4201."`), a human-readable instruction already present on every step whenever `steps=true` is set, no extra parameter required. Also raised the output cap from 10 to 200: the old cap was sized for the old (broken) source, which could emit several announcements per turn; `maneuver.instruction` is one entry per real turn, so a normal route — even a long one with dozens of turns — stays well under the new cap. Confirmed live against the real Directions API. + ### New Features - **`search_and_geocode_tool`: description now mentions Search Box API's natural-language query support.** Mapbox is rolling out NL understanding on the underlying `/forward` endpoint (category + place + attribute phrasing, e.g. "quiet coffee shops with wifi near downtown"), verified working well in our own testing. The tool's description and `q` field now mention this so callers know richer free-text phrasing is worth trying, with a caveat that big-box retail brand + address queries (Costco, Walmart, Target) are currently unreliable and return unrelated results — verified via a 202-query test set against the live API, after first ruling out a CDN-caching artifact in our own test methodology that had produced misleading results. diff --git a/src/tools/directions-tool/cleanResponseData.ts b/src/tools/directions-tool/cleanResponseData.ts index 9078724b..585f887a 100644 --- a/src/tools/directions-tool/cleanResponseData.ts +++ b/src/tools/directions-tool/cleanResponseData.ts @@ -42,13 +42,13 @@ interface RawIncident { [key: string]: unknown; } -interface RawVoiceInstruction { - announcement?: string; +interface RawManeuver { + instruction?: string; [key: string]: unknown; } interface RawStep { - voiceInstructions?: RawVoiceInstruction[]; + maneuver?: RawManeuver; [key: string]: unknown; } @@ -200,8 +200,8 @@ export function cleanResponseData( length?: number; }> = []; - // Collect voice instruction announcements from all steps - const routeAnnouncements: string[] = []; + // Collect turn-by-turn instructions from all steps + const routeInstructions: string[] = []; let totalDistanceWeightedSpeed = 0; // Sum of (speed × distance) for each segment let sumDistanceMeters = 0; @@ -284,15 +284,17 @@ export function cleanResponseData( }); } - // Process steps if they exist to collect voice instructions + // Process steps if they exist to collect turn-by-turn instructions. + // `maneuver.instruction` (e.g. "Bear right onto Great Portland + // Street/A4201") is present on every step whenever steps=true is + // requested, unlike `step.voiceInstructions`, which requires the + // separate voice_instructions=true parameter -- never sent by this + // tool, so that field was always empty and `instructions` never + // actually populated. if (leg.steps) { leg.steps.forEach((step) => { - if (step.voiceInstructions) { - step.voiceInstructions.forEach((instruction) => { - if (instruction.announcement) { - routeAnnouncements.push(instruction.announcement); - } - }); + if (step.maneuver?.instruction) { + routeInstructions.push(step.maneuver.instruction); } }); } @@ -312,10 +314,18 @@ export function cleanResponseData( // Add all incidents with the specified fields as a new property on the route cleanedRoute.incidents_summary = routeIncidents; - // Add voice instruction announcements only if there are 1 to 10 of them - // If there are more than 10, it's just too many, and if there is 0 then we don't have them. - if (routeAnnouncements.length >= 1 && routeAnnouncements.length <= 10) { - cleanedRoute.instructions = routeAnnouncements; + // One instruction per real maneuver (unlike the old voice-announcement + // source, which could repeat several announcements per turn), so a + // normal route -- even a long one with dozens of turns -- stays well + // within a generous cap. The cap here is just a backstop against a + // pathological case (e.g. many waypoints each with a complex urban leg), + // not a limit expected to bite in practice. + const MAX_INSTRUCTIONS = 200; + if ( + routeInstructions.length >= 1 && + routeInstructions.length <= MAX_INSTRUCTIONS + ) { + cleanedRoute.instructions = routeInstructions; } cleanedRoute.num_legs = route.legs?.length || 0; diff --git a/test/tools/directions-tool/cleanResponseData.test.ts b/test/tools/directions-tool/cleanResponseData.test.ts index a257bc96..c100afce 100644 --- a/test/tools/directions-tool/cleanResponseData.test.ts +++ b/test/tools/directions-tool/cleanResponseData.test.ts @@ -213,26 +213,22 @@ describe('cleanResponseData', () => { expect(result.routes[0].incidents_summary[0].extra_field).toBeUndefined(); }); - it('should collect voice instructions when within limits', () => { + it('should collect turn-by-turn instructions from step.maneuver.instruction', () => { const mockData = { routes: [ { legs: [ { steps: [ + { maneuver: { instruction: 'Drive northeast on Main St.' } }, { - voiceInstructions: [ - { announcement: 'Turn right in 100 meters' }, - { announcement: 'Turn right now' } - ] + maneuver: { + instruction: 'Bear right onto Elm St. Continue on Elm St.' + } }, - { - voiceInstructions: [ - { announcement: 'Continue straight for 500 meters' } - ] - } + { maneuver: { instruction: 'Arrive at your destination.' } } ], - summary: 'Leg with voice instructions' + summary: 'Leg with turn-by-turn instructions' } ] } @@ -242,27 +238,72 @@ describe('cleanResponseData', () => { const result = cleanResponseData(mockInput, mockData); expect(result.routes[0].instructions).toEqual([ - 'Turn right in 100 meters', - 'Turn right now', - 'Continue straight for 500 meters' + 'Drive northeast on Main St.', + 'Bear right onto Elm St. Continue on Elm St.', + 'Arrive at your destination.' ]); }); - it('should not include instructions when there are too many', () => { + it('ignores steps with no maneuver.instruction', () => { + const mockData = { + routes: [ + { + legs: [ + { + steps: [ + { maneuver: { instruction: 'Drive north on Main St.' } }, + { maneuver: {} }, + {} + ], + summary: 'Leg with a step missing an instruction' + } + ] + } + ] + }; + + const result = cleanResponseData(mockInput, mockData); + + expect(result.routes[0].instructions).toEqual(['Drive north on Main St.']); + }); + + it('still includes turn-by-turn instructions for a long route with dozens of real turns', () => { + // Mirrors a real long driving route (confirmed live: London->Edinburgh + // has ~40 steps in a single leg) -- well within the new cap, unlike the + // old 10-instruction cap (calibrated for a different, always-empty + // voice-announcement source) which would have excluded this outright. + const mockData = { + routes: [ + { + legs: [ + { + steps: Array.from({ length: 40 }, (_, i) => ({ + maneuver: { instruction: `Turn ${i + 1}` } + })), + summary: 'Leg with many real turns' + } + ] + } + ] + }; + + const result = cleanResponseData(mockInput, mockData); + + expect(result.routes[0].instructions).toHaveLength(40); + expect(result.routes[0].instructions?.[0]).toBe('Turn 1'); + expect(result.routes[0].instructions?.[39]).toBe('Turn 40'); + }); + + it('excludes instructions entirely past the pathological-case cap', () => { const mockData = { routes: [ { legs: [ { - steps: Array(6) - .fill(0) - .map(() => ({ - voiceInstructions: [ - { announcement: 'Instruction 1' }, - { announcement: 'Instruction 2' } - ] - })), - summary: 'Leg with many instructions' + steps: Array.from({ length: 201 }, (_, i) => ({ + maneuver: { instruction: `Turn ${i + 1}` } + })), + summary: 'Leg with an unreasonable number of turns' } ] } @@ -271,8 +312,6 @@ describe('cleanResponseData', () => { const result = cleanResponseData(mockInput, mockData); - // With 6 steps and 2 instructions each, we'd have 12 instructions total - // The function should exclude them since it's > 10 expect(result.routes[0].instructions).toBeUndefined(); }); From 4b20de55ae8e7fa4b96cbfe843d178156ea796cc Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 27 Oct 2026 14:14:10 -0400 Subject: [PATCH 2/2] docs: describe what the instruction code does, not its history From review on #256. Co-Authored-By: Claude Opus 5.5 --- .../directions-tool/cleanResponseData.ts | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/tools/directions-tool/cleanResponseData.ts b/src/tools/directions-tool/cleanResponseData.ts index 585f887a..5613057f 100644 --- a/src/tools/directions-tool/cleanResponseData.ts +++ b/src/tools/directions-tool/cleanResponseData.ts @@ -284,13 +284,10 @@ export function cleanResponseData( }); } - // Process steps if they exist to collect turn-by-turn instructions. - // `maneuver.instruction` (e.g. "Bear right onto Great Portland - // Street/A4201") is present on every step whenever steps=true is - // requested, unlike `step.voiceInstructions`, which requires the - // separate voice_instructions=true parameter -- never sent by this - // tool, so that field was always empty and `instructions` never - // actually populated. + // One instruction per step, e.g. "Bear right onto Great Portland + // Street/A4201". `maneuver.instruction` comes back whenever + // steps=true; `step.voiceInstructions` would need + // voice_instructions=true, which this tool doesn't send. if (leg.steps) { leg.steps.forEach((step) => { if (step.maneuver?.instruction) { @@ -314,12 +311,9 @@ export function cleanResponseData( // Add all incidents with the specified fields as a new property on the route cleanedRoute.incidents_summary = routeIncidents; - // One instruction per real maneuver (unlike the old voice-announcement - // source, which could repeat several announcements per turn), so a - // normal route -- even a long one with dozens of turns -- stays well - // within a generous cap. The cap here is just a backstop against a - // pathological case (e.g. many waypoints each with a complex urban leg), - // not a limit expected to bite in practice. + // One instruction per maneuver, so even a long route with dozens of + // turns fits. The cap only guards against extreme cases, like many + // waypoints each with a complex urban leg. const MAX_INSTRUCTIONS = 200; if ( routeInstructions.length >= 1 &&