Skip to content

Implement Mapbox route rendering via GeoJsonLayer #562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ngoiyaeric wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement Mapbox route rendering via GeoJsonLayer #562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions components/map/geojson-layer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export function GeoJsonLayer({ id, data }: GeoJsonLayerProps) {
const pointLayerId = `geojson-point-layer-${id}`
const polygonLayerId = `geojson-polygon-layer-${id}`
const polygonOutlineLayerId = `geojson-polygon-outline-layer-${id}`
const lineStringLayerId = `geojson-linestring-layer-${id}`

const onMapLoad = () => {
// Add source if it doesn't exist
Expand Down Expand Up @@ -62,6 +63,25 @@ export function GeoJsonLayer({ id, data }: GeoJsonLayerProps) {
})
}

// Add linestring layer for routes
if (!map.getLayer(lineStringLayerId)) {
map.addLayer({
id: lineStringLayerId,
type: 'line',
source: sourceId,
filter: ['any', ['==', '$type', 'LineString'], ['==', '$type', 'MultiLineString']],
layout: {
'line-join': 'round',
'line-cap': 'round'
},
paint: {
'line-color': '#3b82f6', // blue-500
'line-width': 4,
'line-opacity': 0.8
}
})
}

// Add point layer for circles
if (!map.getLayer(pointLayerId)) {
map.addLayer({
Expand Down Expand Up @@ -91,6 +111,7 @@ export function GeoJsonLayer({ id, data }: GeoJsonLayerProps) {
if (map.getLayer(pointLayerId)) map.removeLayer(pointLayerId)
if (map.getLayer(polygonLayerId)) map.removeLayer(polygonLayerId)
if (map.getLayer(polygonOutlineLayerId)) map.removeLayer(polygonOutlineLayerId)
if (map.getLayer(lineStringLayerId)) map.removeLayer(lineStringLayerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}
Expand Down
9 changes: 7 additions & 2 deletions components/map/map-data-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@ export interface MapData {
targetPosition?: { lat: number; lng: number } | null; // For flying to a location
cameraState?: CameraState; // For saving camera state
currentTimezone?: string; // Current timezone identifier
// TODO: Add other relevant map data types later (e.g., routeGeoJSON, poiList)
mapFeature?: any | null; // Generic feature from MCP hook's processLocationQuery
// TODO: Add other relevant map data types later (e.g., poiList)
mapFeature?: {
place_name?: string;
mapUrl?: string;
routeGeoJSON?: any;
[key: string]: any;
} | null; // Generic feature from MCP hook's processLocationQuery
Comment on lines +19 to +25
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider tightening the mapFeature type.

The index signature [key: string]: any is overly permissive and could mask type errors. If additional properties are needed, consider explicitly defining them or using a discriminated union. Also, routeGeoJSON could benefit from a proper GeoJSON Geometry type for better type safety.

♻️ Suggested type improvement 
+import type { Geometry } from 'geojson';
+
 export interface MapData {
   targetPosition?: { lat: number; lng: number } | null;
   cameraState?: CameraState;
   currentTimezone?: string;
   // TODO: Add other relevant map data types later (e.g., poiList)
   mapFeature?: {
     place_name?: string;
     mapUrl?: string;
-    routeGeoJSON?: any;
-    [key: string]: any;
+    routeGeoJSON?: Geometry;
   } | null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// TODO: Add other relevant map data types later (e.g., poiList)
mapFeature?: {
place_name?: string;
mapUrl?: string;
routeGeoJSON?: any;
[key: string]: any;
} | null; // Generic feature from MCP hook's processLocationQuery
import type { Geometry } from 'geojson';
export interface MapData {
targetPosition?: { lat: number; lng: number } | null;
cameraState?: CameraState;
currentTimezone?: string;
// TODO: Add other relevant map data types later (e.g., poiList)
mapFeature?: {
place_name?: string;
mapUrl?: string;
routeGeoJSON?: Geometry;
} | null; // Generic feature from MCP hook's processLocationQuery
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/map/map-data-context.tsx` around lines 19 - 25, Tighten the
overly-permissive mapFeature shape by replacing the broad index signature with
an explicit interface (e.g., MapFeature) that lists known optional properties
(place_name, mapUrl) and types routeGeoJSON as a GeoJSON Geometry/Feature (use
GeoJSON.Geometry | GeoJSON.Feature | null) instead of any; if you still need
extensibility, use a safer type like Record<string, unknown> or a discriminated
union for different feature kinds rather than [key: string]: any so type errors
are not masked (update the mapFeature type declaration and any usages that
reference mapFeature/routeGeoJSON).

drawnFeatures?: Array<{ // Added to store drawn features and their measurements
id: string;
type: 'Polygon' | 'LineString';
Expand Down
5 changes: 3 additions & 2 deletions components/map/map-query-handler.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface McpResponseData {
address?: string;
};
mapUrl?: string;
routeGeoJSON?: any;
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using a more specific type for routeGeoJSON.

Using any loses type safety. Since this represents GeoJSON geometry from the Mapbox Directions API, consider using a proper GeoJSON type.

♻️ Suggested type improvement 
+import type { Geometry } from 'geojson';
+
 interface McpResponseData {
   location: {
     latitude?: number;
     longitude?: number;
     place_name?: string;
     address?: string;
   };
   mapUrl?: string;
-  routeGeoJSON?: any;
+  routeGeoJSON?: Geometry;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
routeGeoJSON?: any;
import type { Geometry } from 'geojson';
interface McpResponseData {
location: {
latitude?: number;
longitude?: number;
place_name?: string;
address?: string;
};
mapUrl?: string;
routeGeoJSON?: Geometry;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/map/map-query-handler.tsx` at line 16, routeGeoJSON is currently
typed as any which removes type safety; change its type to an appropriate
GeoJSON type (e.g., GeoJSON.FeatureCollection |
GeoJSON.Feature<GeoJSON.LineString> | GeoJSON.Geometry) to match the Mapbox
Directions API output. Import the GeoJSON types from the 'geojson' package (or
use your project’s geo types) and update the routeGeoJSON declaration and any
usages in the MapQueryHandler component / functions to use the chosen specific
type, adjusting casts or parsing where necessary.

}

interface GeospatialToolOutput {
Expand Down Expand Up @@ -42,8 +43,8 @@ export const MapQueryHandler: React.FC<MapQueryHandlerProps> = ({ toolOutput })
// Optionally store more info from mcp_response if needed by MapboxMap component later
mapFeature: {
place_name,
// Potentially add mapUrl or other details from toolOutput.mcp_response
mapUrl: toolOutput.mcp_response?.mapUrl
mapUrl: toolOutput.mcp_response?.mapUrl,
routeGeoJSON: toolOutput.mcp_response?.routeGeoJSON
}
}));
} else {
Expand Down
23 changes: 17 additions & 6 deletions components/map/mapbox-map.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import 'mapbox-gl/dist/mapbox-gl.css'
import '@mapbox/mapbox-gl-draw/dist/mapbox-gl-draw.css'
import { useMapToggle, MapToggleEnum } from '../map-toggle-context'
import { useMapData } from './map-data-context'; // Add this import
import { useMapLoading } from '../map-loading-context'; // Import useMapLoading
import { useMapLoading } from '../map-loading-context'
import { GeoJsonLayer } from './geojson-layer'; // Import useMapLoading
Comment on lines +14 to +15
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the misleading inline comment.

The comment says "Import useMapLoading" but the line actually imports GeoJsonLayer. The useMapLoading import is on line 14.

📝 Proposed fix 
 import { useMapLoading } from '../map-loading-context'
-import { GeoJsonLayer } from './geojson-layer'; // Import useMapLoading
+import { GeoJsonLayer } from './geojson-layer';
 import { useMap } from './map-context'
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/map/mapbox-map.tsx` around lines 14 - 15, The inline comment on
the import of GeoJsonLayer is incorrect; update the comment (or remove it) so it
accurately describes that the line imports GeoJsonLayer rather than
useMapLoading — locate the import statements for useMapLoading and GeoJsonLayer
in the module (symbols: useMapLoading, GeoJsonLayer) and change the comment to
either "// Import GeoJsonLayer" or delete the misleading comment.

import { useMap } from './map-context'

mapboxgl.accessToken = process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN as string;
Expand Down Expand Up @@ -550,11 +551,8 @@ export const Mapbox: React.FC<{ position?: { latitude: number; longitude: number
updateMapPosition(lat, lng);
}
}
// TODO: Handle mapData.mapFeature for drawing routes, polygons, etc. in a future step.
// For example:
// if (mapData.mapFeature && mapData.mapFeature.route_geometry && typeof drawRoute === 'function') {
// drawRoute(mapData.mapFeature.route_geometry); // Implement drawRoute function if needed
// }
// mapFeature route rendering is now handled declaratively by GeoJsonLayer
// mounted conditionally in the component return
}, [mapData.targetPosition, mapData.mapFeature, updateMapPosition]);

// Long-press handlers
Expand Down Expand Up @@ -593,6 +591,19 @@ export const Mapbox: React.FC<{ position?: { latitude: number; longitude: number
onMouseUp={handleMouseUp}
onMouseLeave={handleMouseUp} // Clear timer if mouse leaves container while pressed
/>
{mapData.mapFeature?.routeGeoJSON && (
<GeoJsonLayer
id="map-feature-route"
data={{
type: 'FeatureCollection',
features: [{
type: 'Feature',
geometry: mapData.mapFeature.routeGeoJSON,
properties: {}
}]
}}
/>
)}
</div>
)
}
27 changes: 24 additions & 3 deletions lib/agents/tools/geospatial.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface Location {
interface McpResponse {
location: Location;
mapUrl?: string;
routeGeoJSON?: any;
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using a proper GeoJSON Geometry type.

This is consistent with other files, but using any loses type safety. The GeoJSON Geometry type would provide better validation.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/agents/tools/geospatial.tsx` at line 28, The property routeGeoJSON is
typed as any which loses type safety; replace its type with the proper GeoJSON
Geometry type (import Geometry from the 'geojson' package or `@types/geojson`) and
update the declaration of routeGeoJSON?: any to routeGeoJSON?: Geometry so
callers and consumers (e.g., in geospatial.tsx) have correct shape validation
and autocompletion.

}

interface MapboxConfig {
Expand Down Expand Up @@ -393,13 +394,33 @@ Uses the Mapbox Search Box Text Search API endpoint to power searching for and g
// Process results
if (typeof content === 'object' && content !== null) {
const parsedData = content as any;

// Extract route geometry if available (Directions tool)
let routeGeoJSON = undefined;
if (parsedData.geometry) {
routeGeoJSON = parsedData.geometry;
} else if (parsedData.routes && parsedData.routes.length > 0 && parsedData.routes[0].geometry) {
routeGeoJSON = parsedData.routes[0].geometry;
}

if (parsedData.results?.length > 0) {
const firstResult = parsedData.results[0];
mcpData = { location: { latitude: firstResult.coordinates?.latitude, longitude: firstResult.coordinates?.longitude, place_name: firstResult.name || firstResult.place_name, address: firstResult.full_address || firstResult.address }, mapUrl: parsedData.mapUrl };
mcpData = { location: { latitude: firstResult.coordinates?.latitude, longitude: firstResult.coordinates?.longitude, place_name: firstResult.name || firstResult.place_name, address: firstResult.full_address || firstResult.address }, mapUrl: parsedData.mapUrl, routeGeoJSON };
} else if (parsedData.location) {
mcpData = { location: { latitude: parsedData.location.latitude, longitude: parsedData.location.longitude, place_name: parsedData.location.place_name || parsedData.location.name, address: parsedData.location.address || parsedData.location.formatted_address }, mapUrl: parsedData.mapUrl || parsedData.map_url };
mcpData = { location: { latitude: parsedData.location.latitude, longitude: parsedData.location.longitude, place_name: parsedData.location.place_name || parsedData.location.name, address: parsedData.location.address || parsedData.location.formatted_address }, mapUrl: parsedData.mapUrl || parsedData.map_url, routeGeoJSON };
} else if (parsedData.routes && parsedData.routes.length > 0) {
// It's a routing response, pick first route coordinates for map center
const route = parsedData.routes[0];
let lat, lng;
if (route.geometry && route.geometry.coordinates && route.geometry.coordinates.length > 0) {
// Pick a point roughly in the middle, or the start
const coords = route.geometry.coordinates[Math.floor(route.geometry.coordinates.length / 2)];
lng = coords[0];
lat = coords[1];
}
mcpData = { location: { latitude: lat, longitude: lng, place_name: "Route", address: "Generated Route" }, routeGeoJSON };
Comment on lines +411 to +421
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Route center coordinates may be undefined, causing downstream issues.

When extracting the route center point, lat and lng remain undefined if route.geometry.coordinates is empty or missing. This undefined location will be passed to MapQueryHandler, which checks typeof latitude === 'number' and will reject the data, clearing mapFeature entirely—including the valid routeGeoJSON. This means routes with empty coordinate arrays will silently fail to render.

🛠️ Proposed fix to handle empty coordinates 
         } else if (parsedData.routes && parsedData.routes.length > 0) {
           // It's a routing response, pick first route coordinates for map center
           const route = parsedData.routes[0];
-          let lat, lng;
+          let lat: number | undefined, lng: number | undefined;
           if (route.geometry && route.geometry.coordinates && route.geometry.coordinates.length > 0) {
             // Pick a point roughly in the middle, or the start
             const coords = route.geometry.coordinates[Math.floor(route.geometry.coordinates.length / 2)];
-            lng = coords[0];
-            lat = coords[1];
+            if (Array.isArray(coords) && coords.length >= 2) {
+              lng = coords[0];
+              lat = coords[1];
+            }
+          }
+          // Fallback: if no valid center, use first coordinate pair
+          if (lat === undefined || lng === undefined) {
+            const firstCoord = route.geometry?.coordinates?.[0];
+            if (Array.isArray(firstCoord) && firstCoord.length >= 2) {
+              lng = firstCoord[0];
+              lat = firstCoord[1];
+            }
           }
           mcpData = { location: { latitude: lat, longitude: lng, place_name: "Route", address: "Generated Route" }, routeGeoJSON };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else if (parsedData.routes && parsedData.routes.length > 0) {
// It's a routing response, pick first route coordinates for map center
const route = parsedData.routes[0];
let lat, lng;
if (route.geometry && route.geometry.coordinates && route.geometry.coordinates.length > 0) {
// Pick a point roughly in the middle, or the start
const coords = route.geometry.coordinates[Math.floor(route.geometry.coordinates.length / 2)];
lng = coords[0];
lat = coords[1];
}
mcpData = { location: { latitude: lat, longitude: lng, place_name: "Route", address: "Generated Route" }, routeGeoJSON };
} else if (parsedData.routes && parsedData.routes.length > 0) {
// It's a routing response, pick first route coordinates for map center
const route = parsedData.routes[0];
let lat: number | undefined, lng: number | undefined;
if (route.geometry && route.geometry.coordinates && route.geometry.coordinates.length > 0) {
// Pick a point roughly in the middle, or the start
const coords = route.geometry.coordinates[Math.floor(route.geometry.coordinates.length / 2)];
if (Array.isArray(coords) && coords.length >= 2) {
lng = coords[0];
lat = coords[1];
}
}
// Fallback: if no valid center, use first coordinate pair
if (lat === undefined || lng === undefined) {
const firstCoord = route.geometry?.coordinates?.[0];
if (Array.isArray(firstCoord) && firstCoord.length >= 2) {
lng = firstCoord[0];
lat = firstCoord[1];
}
}
mcpData = { location: { latitude: lat, longitude: lng, place_name: "Route", address: "Generated Route" }, routeGeoJSON };
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/agents/tools/geospatial.tsx` around lines 411 - 421, The route center
extraction leaves lat/lng undefined when route.geometry.coordinates is missing
or empty, causing MapQueryHandler to reject the whole mcpData (including valid
routeGeoJSON); update the block that handles parsedData.routes (around
parsedData.routes[0], route.geometry.coordinates, and the assignment to mcpData)
to defensively set a fallback center: if coordinates array is empty or missing,
use the first available waypoint or omit the location field entirely (or set a
safe default like 0/0 or a bounding-box center) so that routeGeoJSON is still
preserved and passed to MapQueryHandler; ensure mcpData.location is only
included when latitude and longitude are valid numbers.

} else {
throw new Error("Response missing required 'location' or 'results' field");
throw new Error("Response missing required 'location', 'results' or 'routes' field");
}
} else throw new Error('Unexpected response format from mapping service');

Expand Down