-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Pass Mapbox state to the MCP to provide context #224
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces MapData into the chat flow: map state (center, zoom, pitch, drawn features) is captured in the map component, stored in context, serialized into form submission, parsed in the server action, and passed to the researcher agent and geospatial tools, replacing the previous MCP-based parameter. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatPanel
participant MapDataContext
participant MapboxMap
participant ServerAction as app/actions.submit
participant Researcher as researcher()
participant Tools as getTools()/geospatialTool
MapboxMap->>MapDataContext: setMapData({ center, zoom, pitch, drawnFeatures })
User->>ChatPanel: Submit message
ChatPanel->>MapDataContext: read mapData
ChatPanel->>ServerAction: FormData(message, map_data?)
ServerAction->>ServerAction: parse map_data JSON
ServerAction->>Researcher: researcher(dynamicSystemPrompt, uiStream, streamText, messages, mapData, useSpecificAPI)
Researcher->>Researcher: build map-aware system prompt (if mapData)
Researcher->>Tools: getTools({ uiStream, mapData })
Tools->>Tools: geospatialTool({ uiStream, mapData })
Tools-->>Researcher: tool result
Researcher-->>User: streamed response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
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.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
lib/agents/tools/geospatial.tsx (1)
145-270: Unused mapData parameter — use it or remove itgeospatialTool currently accepts mapData but never reads it in execute. Verified references show mapData is passed into the tool and is used elsewhere as map context, so either wire it into the tool or remove the param/calls.
Files to review:
- lib/agents/tools/geospatial.tsx — signature at line ~145 accepts { uiStream, mapData } but mapData is unused.
- lib/agents/tools/index.tsx — passes mapData into geospatialTool (lines ~24–26).
- lib/agents/researcher.tsx — builds mapContext from mapData (lines ~34–44) (center, zoom, drawnFeatures).
- components/map/map-data-context.tsx — MapData shape and provider (defines targetPosition, drawnFeatures, etc.).
- components/map/map-query-handler.tsx — consumes geospatialTool output and sets mapData (lines ~36–40); geospatialTool could return mapFeature/targetPosition to populate this.
Suggested actions (pick one):
- Preferred: Use mapData inside geospatialTool.execute to improve results (examples: if params lack coordinates use mapData.center as proximity/default; use mapData.zoom to set search radius; include drawnFeatures as bounding polygon or radius when present; return mapFeature or targetPosition in mcp_response).
- Alternative: If mapData is intentionally unused, remove it from geospatialTool signature and stop passing it from index.tsx to avoid confusion.
lib/agents/researcher.tsx (1)
64-69: Make dynamicSystemPrompt optional or give it a default to match the fallback logicThe function currently declares dynamicSystemPrompt as a required string but the code treats empty/absent prompts by falling back to default_system_prompt. Either make the parameter optional or give it a default value for clarity/type-safety.
Findings:
- Caller(s): app/actions.tsx calls researcher(currentSystemPrompt, uiStream, streamText, ...) — no callers found that omit the parameter.
- Declarations that should be aligned with the fallback behavior:
- lib/agents/researcher.tsx
- lib/agents/writer.tsx
Suggested minimal fixes (example diffs):
lib/agents/researcher.tsx
- export async function researcher(
- dynamicSystemPrompt: string, // New parameter
- export async function researcher(
- dynamicSystemPrompt: string = '', // New parameter (default to empty)
lib/agents/writer.tsx
- export async function writer(
- dynamicSystemPrompt: string, // New parameter
- export async function writer(
- dynamicSystemPrompt: string = '', // New parameter (default to empty)
Apply one of the two options:
- Prefer dynamicSystemPrompt: string = '' to keep the type non-optional and ensure .trim() is safe.
- Or use dynamicSystemPrompt?: string if you want it explicitly optional.
🧹 Nitpick comments (5)
components/map/map-data-context.tsx (1)
17-19: Document tuple order and consider adding 'bearing' for completeness
- Add a short comment to clarify the center tuple is [lng, lat] to avoid confusion.
- If you intend to preserve full camera state like you do with pitch/zoom, consider adding bearing as well.
Apply this small doc tweak within the selected range:
- center?: [number, number]; + center?: [number, number]; // [lng, lat] zoom?: number; pitch?: number;If you decide to capture bearing too, update the interface accordingly (outside the selected range):
export interface MapData { // ... center?: [number, number]; // [lng, lat] zoom?: number; pitch?: number; bearing?: number; // optional: map camera bearing in degrees }components/chat-panel.tsx (1)
63-65: Be defensive when serializing LngLatLike and avoid redundant check
- targetPosition is typed as LngLatLike, which could be a class instance. While you currently store arrays, a future update could pass a LngLat object and lead to unexpected JSON output. Consider a replacer to normalize LngLat-like objects to [lng, lat] and keep the payload predictable.
- Minor: mapData is always defined by the provider, so the if (mapData) guard is redundant.
Here’s an inline replacer to normalize any LngLat-like object and keep the change scoped:
- if (mapData) { - formData.append('map_data', JSON.stringify(mapData)) - } + if (mapData) { + const replacer = (key: string, value: any) => { + // Normalize Mapbox's LngLat-like objects to [lng, lat] + if (key === 'targetPosition' && value && typeof value === 'object' && 'lng' in value && 'lat' in value) { + return [value.lng, value.lat] + } + return value + } + formData.append('map_data', JSON.stringify(mapData, replacer)) + }Optional: Consider excluding map_data from the user-visible message content on the server to avoid bloating message history with large map state (see app/actions.tsx suggestion).
components/map/mapbox-map.tsx (1)
321-331: Good: persist camera state; consider also capturing bearing and avoiding no-op updates
- LGTM capturing center/zoom/pitch and pushing to context. This enables downstream consumers to tailor behavior based on current view.
- Enhancement: capture bearing as well (map.current.getBearing()) to preserve full camera state.
- Minor optimization: compare against currentMapCenterRef before calling setMapData to avoid no-op context updates on small moves.
Apply the bearing addition and a minimal change-detection (bearing requires interface update in MapData):
- const newCenter: [number, number] = [center.lng, center.lat]; - currentMapCenterRef.current = { center: newCenter, zoom, pitch }; - setMapData(prevData => ({ - ...prevData, - center: newCenter, - zoom: zoom, - pitch: pitch, - })); + const newCenter: [number, number] = [center.lng, center.lat] + const bearing = map.current.getBearing() + const prev = currentMapCenterRef.current + const changed = + prev.center[0] !== newCenter[0] || + prev.center[1] !== newCenter[1] || + prev.zoom !== zoom || + prev.pitch !== pitch + if (changed) { + currentMapCenterRef.current = { center: newCenter, zoom, pitch } + setMapData(prevData => ({ + ...prevData, + center: newCenter, + zoom, + pitch, + // bearing, // uncomment after adding to MapData + })) + }lib/agents/tools/index.tsx (1)
11-12: Plumbing looks correct
- ToolProps growth to include mapData and wiring it through to geospatialQueryTool is consistent.
- After switching to a type-only import, this remains type-safe and server-friendly.
If geospatialTool doesn’t need fullResponse, you can drop it from ToolProps and callers for minimal surface area. Not a blocker.
Also applies to: 14-15, 25-27
lib/agents/researcher.tsx (1)
42-44: Consider improving drawnFeatures display format.While the current JSON.stringify approach works, the output might be verbose and less readable in the system prompt. Consider a more concise format.
- if (mapData.drawnFeatures && mapData.drawnFeatures.length > 0) { - mapContext += `- Drawn features on map: ${JSON.stringify(mapData.drawnFeatures.map(f => ({ type: f.type, measurement: f.measurement })))} \n`; - } + if (mapData.drawnFeatures && mapData.drawnFeatures.length > 0) { + const features = mapData.drawnFeatures.map(f => `${f.type} (${f.measurement})`).join(', '); + mapContext += `- Drawn features on map: ${features}\n`; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
app/actions.tsx(2 hunks)components/chat-panel.tsx(3 hunks)components/map/map-data-context.tsx(1 hunks)components/map/mapbox-map.tsx(1 hunks)lib/agents/researcher.tsx(3 hunks)lib/agents/tools/geospatial.tsx(2 hunks)lib/agents/tools/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
components/chat-panel.tsx (4)
components/map/map-data-context.tsx (1)
useMapData(39-45)components/chat.tsx (2)
Chat(20-123)id(67-72)components/map/map-query-handler.tsx (1)
useMapData(30-83)app/page.tsx (1)
Page(9-18)
components/map/map-data-context.tsx (1)
components/map/map-query-handler.tsx (2)
prevData(39-49)MapQueryHandlerProps(25-28)
components/map/mapbox-map.tsx (1)
components/map/map-query-handler.tsx (2)
useMapData(30-83)prevData(39-49)
app/actions.tsx (1)
components/map/map-query-handler.tsx (4)
prevData(39-49)McpResponseData(8-16)toolOutput(33-74)prevData(53-57)
lib/agents/tools/index.tsx (2)
components/map/map-data-context.tsx (1)
MapData(7-20)components/map/map-query-handler.tsx (4)
MapQueryHandlerProps(25-28)GeospatialToolOutput(18-23)useMapData(30-83)toolOutput(33-74)
lib/agents/tools/geospatial.tsx (2)
components/map/map-data-context.tsx (1)
MapData(7-20)components/map/map-query-handler.tsx (3)
GeospatialToolOutput(18-23)toolOutput(33-74)prevData(39-49)
lib/agents/researcher.tsx (2)
components/map/map-data-context.tsx (1)
MapData(7-20)components/map/map-query-handler.tsx (4)
prevData(39-49)useMapData(30-83)toolOutput(33-74)prevData(53-57)
🔇 Additional comments (9)
app/actions.tsx (1)
155-156: LGTM: researcher receives mapDataPassing mapData down the stack aligns with the PR’s goal and matches the updated researcher signature.
lib/agents/tools/geospatial.tsx (3)
5-5: LGTM - Good addition of MapData import.The import of MapData interface provides proper type safety for the new mapData parameter.
145-145: LGTM - Function signature updated appropriately.The function signature now accepts mapData as an optional parameter with proper typing. The destructuring pattern is clean and maintains backwards compatibility.
146-152: Enhanced tool description looks comprehensive.The expanded description covers a broader range of geospatial capabilities including distance calculations, direction queries, and geographic information lookup, which aligns well with the tool's actual functionality.
lib/agents/researcher.tsx (5)
14-14: LGTM - Appropriate import addition.The MapData import is correctly added to support the new parameter.
16-23: Function signature update looks good.The signature changes are well-structured:
- Added
dynamicSystemPrompt: stringparameter for flexibility- Added
mapData: MapData | undefinedparameter with proper optional typing- Parameters are logically ordered and maintain backwards compatibility where possible
33-45: Well-implemented mapContext construction.The logic for building map context from mapData is clean and handles optional properties appropriately:
- Properly checks for mapData existence before processing
- Handles optional properties (center, zoom, drawnFeatures) with individual checks
- Uses appropriate formatting for coordinate display and feature serialization
50-62: Comprehensive tool usage guide with good examples.The updated system prompt provides clear guidance on when to use the geospatialQueryTool with relevant examples. The integration of mapContext at the beginning ensures the AI has proper map state awareness.
71-75: Proper mapData propagation to tools.The mapData parameter is correctly passed through to the getTools function, ensuring tools have access to the current map context.
| const mapDataString = formData?.get('map_data') as string | undefined; | ||
| const mapData = mapDataString ? JSON.parse(mapDataString) : undefined; | ||
|
|
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.
Untrusted JSON.parse on client-provided map_data can crash the action
map_data comes from the browser. A malformed payload will throw and break the request. Wrap in try/catch and default to undefined.
Apply this defensive parsing:
- const mapDataString = formData?.get('map_data') as string | undefined;
- const mapData = mapDataString ? JSON.parse(mapDataString) : undefined;
+ const mapDataString = formData?.get('map_data') as string | undefined
+ let mapData: unknown = undefined
+ if (mapDataString) {
+ try {
+ mapData = JSON.parse(mapDataString)
+ } catch (e) {
+ console.warn('submit(): Ignoring invalid map_data JSON in formData', e)
+ mapData = undefined
+ }
+ }Optional follow-up: avoid including the raw map_data blob in the persisted user message content (JSON.stringify(Object.fromEntries(formData))) to prevent inflating message history and storage. Instead, construct the user-visible content from a filtered copy of FormData that excludes map_data. I can provide a patch if you want that change now.
📝 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.
| const mapDataString = formData?.get('map_data') as string | undefined; | |
| const mapData = mapDataString ? JSON.parse(mapDataString) : undefined; | |
| const mapDataString = formData?.get('map_data') as string | undefined | |
| let mapData: unknown = undefined | |
| if (mapDataString) { | |
| try { | |
| mapData = JSON.parse(mapDataString) | |
| } catch (e) { | |
| console.warn('submit(): Ignoring invalid map_data JSON in formData', e) | |
| mapData = undefined | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/actions.tsx around lines 66 to 68, the code directly calls JSON.parse on
client-provided map_data which can throw on malformed input; update the code to
wrap the JSON.parse call in a try/catch and set mapData to undefined on parse
errors (optionally log/debug the error), and when building persisted user
message content (JSON.stringify(Object.fromEntries(formData))) exclude the raw
map_data entry by creating a filtered copy of FormData that omits 'map_data' so
the large blob isn't stored.
| import { videoSearchTool } from './video-search' | ||
| import { geospatialTool } from './geospatial' // Removed useGeospatialToolMcp import | ||
| import { geospatialTool } from './geospatial' | ||
| import { MapData } from '@/components/map/map-data-context' |
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.
Import MapData as a type to avoid bundling a client module on the server
MapData is declared in a 'use client' file under components. Importing it as a value risks pulling a client-only module (and its deps like mapbox-gl) into server bundles. Use a type-only import, or better, move the type to a shared, non-client module (e.g., lib/types/map.ts) and import from there.
Apply the minimal fix:
-import { MapData } from '@/components/map/map-data-context'
+import type { MapData } from '@/components/map/map-data-context'Longer-term, extract MapData into a server-safe shared types module (e.g., lib/types/map.ts) and update all imports in lib/* to reference that instead of a client component file.
📝 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.
| import { MapData } from '@/components/map/map-data-context' | |
| import type { MapData } from '@/components/map/map-data-context' |
🤖 Prompt for AI Agents
In lib/agents/tools/index.tsx around line 6, the file imports MapData from a
client-side component which can pull client-only modules into server bundles;
update the import to be type-only by replacing the value import with a type
import (e.g., "import type { MapData } from
'@/components/map/map-data-context'") or, preferably for a longer-term fix, move
the MapData type to a shared server-safe module like lib/types/map.ts and import
the type from there across lib/* so no client component is bundled on the
server.
PR Type
Enhancement
Description
Pass Mapbox map state to MCP for context
Add map center, zoom, pitch tracking
Include drawn features in context data
Update geospatial tool with map context
Diagram Walkthrough
File Walkthrough
actions.tsx
Extract and pass map data to researcherapp/actions.tsx
chat-panel.tsx
Integrate map context in chat submissionscomponents/chat-panel.tsx
map-data-context.tsx
Extend MapData interface with map statecomponents/map/map-data-context.tsx
mapbox-map.tsx
Track and update map state changescomponents/map/mapbox-map.tsx
researcher.tsx
Add map context to researcher system promptlib/agents/researcher.tsx
geospatial.tsx
Accept map data in geospatial toollib/agents/tools/geospatial.tsx
index.tsx
Update tools interface for map datalib/agents/tools/index.tsx
Summary by CodeRabbit
New Features
Refactor