-
-
Notifications
You must be signed in to change notification settings - Fork 6
Implement enhancements for MCP visual rendering in Mapbox component #274
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,19 +1,15 @@ | ||||||||||||||||||||||||||||||||||||||
| 'use client'; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import React, { createContext, useContext, useState, ReactNode } from 'react'; | ||||||||||||||||||||||||||||||||||||||
| import { LngLatLike } from 'mapbox-gl'; // Import LngLatLike | ||||||||||||||||||||||||||||||||||||||
| import { LngLatLike } from 'mapbox-gl'; // Import LngLatLike\nimport type * as GeoJSON from 'geojson'; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Define the shape of the map data you want to share | ||||||||||||||||||||||||||||||||||||||
| export interface MapData { | ||||||||||||||||||||||||||||||||||||||
| targetPosition?: LngLatLike | null; // For flying to a location | ||||||||||||||||||||||||||||||||||||||
| // TODO: Add other relevant map data types later (e.g., routeGeoJSON, poiList) | ||||||||||||||||||||||||||||||||||||||
| mapFeature?: any | null; // Generic feature from MCP hook's processLocationQuery | ||||||||||||||||||||||||||||||||||||||
| drawnFeatures?: Array<{ // Added to store drawn features and their measurements | ||||||||||||||||||||||||||||||||||||||
| id: string; | ||||||||||||||||||||||||||||||||||||||
| type: 'Polygon' | 'LineString'; | ||||||||||||||||||||||||||||||||||||||
| measurement: string; | ||||||||||||||||||||||||||||||||||||||
| geometry: any; | ||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||
| targetPosition?: LngLatLike | null; | ||||||||||||||||||||||||||||||||||||||
| routeGeoJSON?: GeoJSON.Feature<GeoJSON.LineString> | null; | ||||||||||||||||||||||||||||||||||||||
| markers?: Array<{ lat: number; lng: number; name?: string; address?: string }>; | ||||||||||||||||||||||||||||||||||||||
| polygons?: Array<GeoJSON.Feature<GeoJSON.Polygon>>; | ||||||||||||||||||||||||||||||||||||||
| drawnFeatures?: Array<{ id: string; type: 'Polygon' | 'LineString'; measurement: string; geometry: any }>; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Tighten types: avoid Use GeoJSON types to prevent shape drift and improve editor tooling. export interface MapData {
targetPosition?: LngLatLike | null;
routeGeoJSON?: GeoJSON.Feature<GeoJSON.LineString> | null;
- markers?: Array<{ lat: number; lng: number; name?: string; address?: string }>;
- polygons?: Array<GeoJSON.Feature<GeoJSON.Polygon>>;
- drawnFeatures?: Array<{ id: string; type: 'Polygon' | 'LineString'; measurement: string; geometry: any }>;
+ markers?: Array<{ lat: number; lng: number; name?: string; address?: string }>;
+ polygons?: Array<GeoJSON.Feature<GeoJSON.Polygon>>;
+ drawnFeatures?: Array<{
+ id: string;
+ type: 'Polygon' | 'LineString';
+ measurement: string;
+ geometry: GeoJSON.Geometry; // replaces any
+ }>;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| interface MapDataContextType { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,24 +2,18 @@ | |||||
|
|
||||||
| import { useEffect } from 'react'; | ||||||
| // Removed useMCPMapClient as we'll use data passed via props | ||||||
| import { useMapData } from './map-data-context'; | ||||||
| import { useMapData, MapData } from './map-data-context'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainConfirm there are no other Residual references will fail type-checks after this refactor. 🏁 Script executed: #!/bin/bash
# Find and show remaining mapFeature usages with context
rg -nC2 "\bmapFeature\b"Length of output: 2145 Remove or update remaining
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| // Define the expected structure of the mcp_response from geospatialTool | ||||||
| interface McpResponseData { | ||||||
| location: { | ||||||
| latitude?: number; | ||||||
| longitude?: number; | ||||||
| place_name?: string; | ||||||
| address?: string; | ||||||
| }; | ||||||
| mapUrl?: string; | ||||||
| } | ||||||
| type McpResponseData = any; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Avoid This preserves safety while still allowing flexible shapes. -type McpResponseData = any;
+type McpResponseData = unknown;Follow-up: add small type guards per queryType to narrow when reading fields. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| interface GeospatialToolOutput { | ||||||
| type: string; // e.g., "MAP_QUERY_TRIGGER" | ||||||
| type: string; | ||||||
| originalUserInput: string; | ||||||
| timestamp: string; | ||||||
| queryType: string; | ||||||
| mcp_response: McpResponseData | null; | ||||||
| error?: string; | ||||||
| } | ||||||
|
|
||||||
| interface MapQueryHandlerProps { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -239,19 +239,8 @@ export const geospatialTool = ({ uiStream }: { uiStream: ReturnType<typeof creat | |||||||||||||||||||||||||||||||
| catch { console.warn('[GeospatialTool] Content is not JSON, using as string:', content); } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Process results | ||||||||||||||||||||||||||||||||
| if (typeof content === 'object' && content !== null) { | ||||||||||||||||||||||||||||||||
| const parsedData = content as any; | ||||||||||||||||||||||||||||||||
| 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 }; | ||||||||||||||||||||||||||||||||
| } 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 }; | ||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||
| throw new Error("Response missing required 'location' or 'results' field"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } else throw new Error('Unexpected response format from mapping service'); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| feedbackMessage = `Successfully processed ${queryType} query for: ${mcpData.location.place_name || JSON.stringify(params)}`; | ||||||||||||||||||||||||||||||||
| mcpData = content; | ||||||||||||||||||||||||||||||||
| feedbackMessage = `Successfully processed ${queryType} query`; | ||||||||||||||||||||||||||||||||
|
Comment on lines
+242
to
+243
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Don’t pass raw MCP content through; minimally normalize or wrap to a known shape. Downstream code expects consistent structures; passing arbitrary content will cause brittle consumers. At least distinguish “has location” vs “raw”. Apply this diff to minimally normalize while preserving raw payloads: - mcpData = content;
- feedbackMessage = `Successfully processed ${queryType} query`;
+ // Minimal normalization to reduce downstream shape assumptions
+ if (
+ content &&
+ typeof content === 'object' &&
+ 'location' in (content as any) &&
+ typeof (content as any).location?.latitude === 'number' &&
+ typeof (content as any).location?.longitude === 'number'
+ ) {
+ mcpData = content as any;
+ } else {
+ mcpData = { raw: content };
+ }
+ feedbackMessage = `Successfully processed ${queryType} query`;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| uiFeedbackStream.update(feedbackMessage); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| } catch (error: any) { | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split imports; current line likely includes a literal "\n" and will not parse.
Place the GeoJSON type-only import on its own line.
📝 Committable suggestion
🤖 Prompt for AI Agents