fix: directions_tool turn-by-turn instructions never populated

CHANGELOG.md

@@ -23,6 +23,7 @@
 
 ### Bug Fixes
 
+- **`directions_tool`: turn-by-turn `instructions` now actually populate (#256).** 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.
 - **`static_map_image_tool` and place photos: URLs with several trailing dots are now checked like the plain hostname.** The URL check stripped only one trailing root-label dot, so `https://localhost../x.png` was compared as `localhost.` and passed, as did `app.localhost..`, dotless hosts such as `intranet..`, and IP literals such as `127.0.0.1..` and `169.254.169.254..`. It now strips them all. This covers custom-marker URLs in `static_map_image_tool` and photo URLs in `PlacePhotoResource`. Public hosts with a trailing dot (`example.com.`) are still accepted.
 
 ### Maintenance

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,14 @@ export function cleanResponseData(
           });
         }
 
-        // Process steps if they exist to collect voice instructions
+        // 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.voiceInstructions) {
-              step.voiceInstructions.forEach((instruction) => {
-                if (instruction.announcement) {
-                  routeAnnouncements.push(instruction.announcement);
-                }
-              });
+            if (step.maneuver?.instruction) {
+              routeInstructions.push(step.maneuver.instruction);
             }
           });
         }
@@ -312,10 +311,15 @@ 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 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 &&
+      routeInstructions.length <= MAX_INSTRUCTIONS
+    ) {
+      cleanedRoute.instructions = routeInstructions;
     }
 
     cleanedRoute.num_legs = route.legs?.length || 0;

test/tools/directions-tool/cleanResponseData.test.ts

@@ -228,26 +228,49 @@ describe('cleanResponseData', () => {
     );
   });
 
-  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 turn-by-turn instructions'
+            }
+          ]
+        }
+      ]
+    };
+
+    const result = cleanResponseData(mockInput, mockData);
+
+    expect(routesOf(result)[0].instructions).toEqual([
+      'Drive northeast on Main St.',
+      'Bear right onto Elm St. Continue on Elm St.',
+      'Arrive at your destination.'
+    ]);
+  });
+
+  it('ignores steps with no maneuver.instruction', () => {
+    const mockData = {
+      routes: [
+        {
+          legs: [
+            {
+              steps: [
+                { maneuver: { instruction: 'Drive north on Main St.' } },
+                { maneuver: {} },
+                {}
               ],
-              summary: 'Leg with voice instructions'
+              summary: 'Leg with a step missing an instruction'
             }
           ]
         }
@@ -257,27 +280,47 @@ describe('cleanResponseData', () => {
     const result = cleanResponseData(mockInput, mockData);
 
     expect(routesOf(result)[0].instructions).toEqual([
-      'Turn right in 100 meters',
-      'Turn right now',
-      'Continue straight for 500 meters'
+      'Drive north on Main St.'
     ]);
   });
 
-  it('should not include instructions when there are too many', () => {
+  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(routesOf(result)[0].instructions).toHaveLength(40);
+    expect(routesOf(result)[0].instructions?.[0]).toBe('Turn 1');
+    expect(routesOf(result)[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'
             }
           ]
         }
@@ -286,8 +329,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(routesOf(result)[0].instructions).toBeUndefined();
   });