autonomous drawing #497
autonomous drawing #497
Conversation
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughAdds end-to-end map drawing: new CircleMode, a drawing tool + schema, MCP client helpers, ToolOutput types, pendingFeatures in MapData, and Mapbox/map-query wiring to accept programmatic drawing commands and geocoding (DRAWING_TRIGGER) for rendering on the map. Changes
Sequence DiagramsequenceDiagram
actor User
participant Researcher
participant DrawingTool
participant MCP as MCP Server
participant MapData as MapData Context
participant Mapbox
User->>Researcher: "Draw a circle at X" (user request)
Researcher->>DrawingTool: execute(params)
DrawingTool->>DrawingTool: validate params, start UI stream
alt location provided
DrawingTool->>MCP: forward_geocode_tool request
MCP-->>DrawingTool: returns coords
end
DrawingTool->>DrawingTool: build Turf geometry (circle/line/polygon)
DrawingTool-->>Researcher: return DRAWING_TRIGGER (features, params)
Researcher->>MapData: MapQueryHandler sets pendingFeatures & toggles map mode
MapData-->>Mapbox: updated mapData (pendingFeatures)
Mapbox->>Mapbox: flush pendingFeatures -> add to map, render labels/measurements
Mapbox-->>User: display drawn geometry
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||||||
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds valuable drawing capabilities, but there are a few correctness and reliability gaps: pendingFeatures can be dropped due to a race between mode switching and Draw initialization, and updateMapPosition now relies on a fixed timeout instead of 'moveend' synchronization. There are also several unsafe-but-type-valid patterns (any usage and unvalidated JSON.parse) that will make the drawing tool brittle as upstream responses change. Finally, the MCP utility currently logs partial secrets and the Google static map URL helper can emit key=undefined, both of which should be addressed.
Additional notes (1)
- Maintainability |
components/map/mapbox-map.tsx:275-275
This relies on direct DOM querying (document.querySelector('.mapbox-gl-draw_polygon')?.parentElement) and appends a button every timesetupDrawingTools()runs. SincesetupDrawingTools()is called whenever switching into DrawingMode, it can create duplicate Circle buttons if the control group persists or if MapboxDraw changes its internal markup.
It also uses innerHTML for SVG injection. While this string is constant (so risk is limited), it still sets a precedent for HTML injection and makes future changes riskier.
Finally, activateUIButton('circle') in CircleMode won’t automatically toggle this custom DOM button’s active state, so the UI may not reflect the current mode.
Summary of changes
Summary
This PR adds AI-assisted map drawing and a radial (circle) draw mode to the Mapbox UI.
Map drawing UI
- Added a custom Mapbox Draw mode in
components/map/draw-modes/circle-mode.ts(draw_circle) using Turf to generate circle polygons. - Updated
components/map/mapbox-map.tsxto:- Register the new draw mode via
modes: { ...MapboxDraw.modes, draw_circle: CircleMode }. - Inject a Circle button into the Draw control DOM and switch to
draw_circleon click. - Display circle measurements as
R: <radius>, A: <area>whenuser_isCirclemetadata exists.
- Register the new draw mode via
AI toolchain integration
- Added
drawingQueryTool(lib/agents/tools/drawing.tsx) with a Zod discriminated union schema (lib/schema/drawing.tsx) to generate GeoJSON features forcircle | polygon | line. - Updated
components/map/map-query-handler.tsxto handle a new tool output type (DRAWING_TRIGGER) and switch the UI intoDrawingMode, storing features intoMapDataContext. - Extended
MapDataContext(components/map/map-data-context.tsx) withpendingFeaturesfor programmatic drawing. - Updated
components/map/mapbox-map.tsxto consumependingFeatures, add them to Mapbox Draw, then clear the queue.
Shared utilities / refactors
- Extracted MCP client connection/teardown logic into
lib/utils/mcp.tsand reused it fromgeospatialToolanddrawingTool. - Moved Google Static Map URL helper to
lib/utils/index.ts.
Prompt updates
- Updated the researcher system prompt (
lib/agents/researcher.tsx) to requiredrawingQueryToolfor drawing/annotation requests.
Dependency bump
- Updated
nextfrom15.3.6to15.3.8inbun.lock.
| import * as turf from '@turf/turf'; | ||
|
|
||
| const CircleMode: any = { | ||
| onSetup: function(opts: any) { | ||
| const state: any = {}; | ||
| state.circle = this.newFeature({ | ||
| type: 'Feature', | ||
| properties: { | ||
| user_isCircle: true, | ||
| user_center: [] | ||
| }, | ||
| geometry: { | ||
| type: 'Polygon', | ||
| coordinates: [[]] | ||
| } | ||
| }); | ||
| this.addFeature(state.circle); | ||
| this.clearSelectedFeatures(); | ||
| this.updateUIClasses({ mouse: 'add' }); | ||
| this.activateUIButton('circle'); | ||
| this.setActionableState({ | ||
| trash: true | ||
| }); | ||
| return state; | ||
| }, | ||
|
|
||
| onTap: function(state: any, e: any) { | ||
| this.onClick(state, e); | ||
| }, | ||
|
|
||
| onClick: function(state: any, e: any) { | ||
| if (state.circle.properties.user_center.length === 0) { | ||
| state.circle.properties.user_center = [e.lngLat.lng, e.lngLat.lat]; | ||
| // Set initial point-like polygon | ||
| state.circle.setCoordinates([[ | ||
| [e.lngLat.lng, e.lngLat.lat], | ||
| [e.lngLat.lng, e.lngLat.lat], | ||
| [e.lngLat.lng, e.lngLat.lat], | ||
| [e.lngLat.lng, e.lngLat.lat] | ||
| ]]); | ||
| } else { | ||
| this.changeMode('simple_select', { featureIds: [state.circle.id] }); | ||
| } | ||
| }, | ||
|
|
||
| onMouseMove: function(state: any, e: any) { | ||
| if (state.circle.properties.user_center.length > 0) { | ||
| const center = state.circle.properties.user_center; | ||
| const distance = turf.distance(center, [e.lngLat.lng, e.lngLat.lat], { units: 'kilometers' }); | ||
| const circle = turf.circle(center, distance, { steps: 64, units: 'kilometers' }); | ||
| state.circle.setCoordinates(circle.geometry.coordinates); | ||
| state.circle.properties.user_radiusInKm = distance; | ||
| } | ||
| }, | ||
|
|
||
| onKeyUp: function(state: any, e: any) { | ||
| if (e.keyCode === 27) return this.changeMode('simple_select'); | ||
| }, | ||
|
|
||
| toDisplayFeatures: function(state: any, geojson: any, display: any) { | ||
| const isActive = geojson.id === state.circle.id; | ||
| geojson.properties.active = isActive ? 'true' : 'false'; | ||
| if (!isActive) return display(geojson); | ||
|
|
||
| // Only display if it has a center (and thus coordinates set) | ||
| if (geojson.properties.user_center && geojson.properties.user_center.length > 0) { | ||
| display(geojson); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
CircleMode is implemented with pervasive any (const CircleMode: any, opts: any, state: any, event e: any, and toDisplayFeatures args). This is type-valid but unsafe and makes it easy to break at runtime (e.g., assuming e.lngLat exists, or state.circle.properties.user_center is always an array).
Also, onSetup ignores opts, and activateUIButton('circle') may not correspond to a registered button name (you create a DOM button with class mapbox-gl-draw_circle, but Mapbox Draw’s internal UI button names typically follow built-in control IDs). If the UI button name is wrong, you may end up with inconsistent “active” styling.
Suggestion
Replace any with the Mapbox Draw mode types (or at least minimal structural types) and avoid relying on activateUIButton('circle') unless you’re sure it’s registered.
Example (minimal, still pragmatic):
import type { MapMouseEvent } from 'mapbox-gl';
import type { DrawCustomMode, DrawFeature } from '@mapbox/mapbox-gl-draw';
type CircleState = {
circle: DrawFeature;
};
type CircleProps = {
user_isCircle: true;
user_center: [number, number] | [];
user_radiusInKm?: number;
};
const CircleMode: DrawCustomMode<CircleState> = {
onSetup() {
const circle = this.newFeature({
type: 'Feature',
properties: { user_isCircle: true, user_center: [] } satisfies CircleProps,
geometry: { type: 'Polygon', coordinates: [[]] }
});
this.addFeature(circle);
this.clearSelectedFeatures();
this.updateUIClasses({ mouse: 'add' });
// Consider removing activateUIButton or ensure it matches the control id
this.setActionableState({ trash: true });
return { circle };
},
onClick(state, e: MapMouseEvent) { /* ... */ },
onMouseMove(state, e: MapMouseEvent) { /* ... */ },
/* ... */
};If you’d like, reply with "@CharlieHelps yes please" and I can add a commit with the typing + safer property handling changes.
| const updateMapPosition = useCallback(async (lat: number, lng: number) => { | ||
| if (map.current && !isUpdatingPositionRef.current) { | ||
| isUpdatingPositionRef.current = true | ||
| stopRotation() | ||
|
|
||
| try { | ||
| // Update our current map center ref | ||
| currentMapCenterRef.current.center = [longitude, latitude] | ||
|
|
||
| await new Promise<void>((resolve) => { | ||
| map.current?.flyTo({ | ||
| center: [longitude, latitude], | ||
| zoom: 12, | ||
| essential: true, | ||
| speed: 0.5, | ||
| curve: 1, | ||
| }) | ||
| map.current?.once('moveend', () => { | ||
| resolve() | ||
| }) | ||
| }) | ||
| setTimeout(() => { | ||
| if (mapType === MapToggleEnum.RealTimeMode) { | ||
| startRotation() | ||
| } | ||
| isUpdatingPositionRef.current = false | ||
| }, 500) | ||
| } catch (error) { | ||
| console.error('Error updating map position:', error) | ||
| map.current.flyTo({ | ||
| center: [lng, lat], | ||
| zoom: 15, | ||
| pitch: 45, | ||
| essential: true | ||
| }) | ||
| setTimeout(() => { | ||
| isUpdatingPositionRef.current = false | ||
| } | ||
| }, 2000) | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
updateMapPosition changed behavior significantly (zoom 12 → 15, adds pitch: 45, removes moveend synchronization, removes rotation stop/restart logic) and now uses a fixed setTimeout(2000) to clear isUpdatingPositionRef.
This can cause correctness issues:
- If
flyTotakes longer than 2s, the app may think updates are allowed while a move is still in progress. - If
flyTocompletes quickly, you unnecessarily block updates. - Removing
stopRotation()here means the rotation loop could fightflyToif it is running (you stop rotation on mode changes and user interaction, but programmatic moves can still happen while rotating).
Suggestion
Restore event-based synchronization and stop rotation during programmatic moves.
const updateMapPosition = useCallback(async (lat: number, lng: number) => {
if (!map.current || isUpdatingPositionRef.current) return;
isUpdatingPositionRef.current = true;
stopRotation();
await new Promise<void>((resolve) => {
map.current!.once('moveend', () => resolve());
map.current!.flyTo({
center: [lng, lat],
zoom: 15,
pitch: 45,
essential: true,
});
});
isUpdatingPositionRef.current = false;
if (mapType === MapToggleEnum.RealTimeMode) startRotation();
}, [mapType, startRotation, stopRotation]);If you prefer not to await, at least clear the flag on 'moveend' rather than a timeout. Reply with "@CharlieHelps yes please" and I can add a commit with this change.
| useEffect(() => { | ||
| if (mapData.pendingFeatures && mapData.pendingFeatures.length > 0 && drawRef.current) { | ||
| mapData.pendingFeatures.forEach(feature => { | ||
| drawRef.current?.add(feature); | ||
| }); | ||
| setMapData(prev => ({ ...prev, pendingFeatures: [] })); | ||
| setTimeout(updateMeasurementLabels, 100); | ||
| } | ||
| }, [mapData.pendingFeatures, updateMeasurementLabels, setMapData]); |
There was a problem hiding this comment.
pendingFeatures is only processed when drawRef.current exists. But MapQueryHandler sets pendingFeatures and switches to DrawingMode in the same effect. There’s a race where pendingFeatures is set before setupDrawingTools() creates drawRef.current, and then this effect won’t re-run because mapData.pendingFeatures won’t change.
Net effect: AI-produced features can be silently dropped (they remain queued in context, but never applied).
Suggestion
Include drawRef.current readiness as a reactive signal, or re-attempt when entering drawing mode / after setupDrawingTools().
One simple approach is to trigger processing when mapType becomes DrawingMode and when isMapReady is true:
useEffect(() => {
if (mapType !== MapToggleEnum.DrawingMode) return;
if (!drawRef.current) return;
const features = mapData.pendingFeatures ?? [];
if (features.length === 0) return;
for (const feature of features) drawRef.current.add(feature);
setMapData(prev => ({ ...prev, pendingFeatures: [] }));
setTimeout(updateMeasurementLabels, 0);
}, [mapType, isMapReady, mapData.pendingFeatures, setMapData, updateMeasurementLabels]);Alternatively, after setupDrawingTools() finishes, call a shared flushPendingFeatures().
Reply with "@CharlieHelps yes please" if you want me to add a commit to make pending feature flushing reliable.
| if (feature.geometry.type === 'Polygon') { | ||
| featureType = 'Polygon'; | ||
| // Calculate area for polygons | ||
| const area = turf.area(feature) | ||
| const formattedArea = formatMeasurement(area, true) | ||
| measurement = formattedArea; | ||
|
|
||
| // Get centroid for label placement | ||
| const isCircle = feature.properties?.user_isCircle; | ||
| const radiusInKm = feature.properties?.user_radiusInKm; | ||
|
|
||
| if (isCircle && radiusInKm) { | ||
| const formattedRadius = formatMeasurement(radiusInKm * 1000, false); | ||
| measurement = `R: ${formattedRadius}, A: ${formattedArea}`; | ||
| } else { | ||
| measurement = formattedArea; | ||
| } | ||
|
|
There was a problem hiding this comment.
Circle metadata written by the AI tool does not match what the map UI expects for displaying circle radius.
CircleModesetsproperties.user_radiusInKm(kilometers).drawingQueryToolsetsproperties.user_radius+user_radiusUnits.MapboxMap.updateMeasurementLabels()only checksuser_radiusInKmto display radius.
Result: circles created via AI will likely show area only (no radius label), despite having radius data.
Suggestion
Normalize circle properties across both drawing sources.
Options:
- Update
drawingQueryToolto also setuser_radiusInKm(convert based onunits). - Or update
updateMeasurementLabels()to fall back touser_radius+user_radiusUnitswhenuser_radiusInKmis missing.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing option (2) (UI-side backward-compatible).
lib/agents/tools/drawing.tsx
Outdated
| execute: async (params: z.infer<typeof drawingToolSchema>) => { | ||
| const { type } = params; | ||
| console.log('[DrawingTool] Execute called with:', params); | ||
|
|
||
| const uiFeedbackStream = createStreamableValue<string>(); | ||
| uiStream.append(<BotMessage content={uiFeedbackStream.value} />); | ||
|
|
||
| let feedbackMessage = `Preparing to draw ${type}... Connecting to mapping service...`; | ||
| uiFeedbackStream.update(feedbackMessage); | ||
|
|
||
| const mcpClient = await getConnectedMcpClient(); | ||
| if (!mcpClient) { | ||
| feedbackMessage = 'Drawing functionality is partially unavailable (geocoding failed). Please check configuration.'; | ||
| uiFeedbackStream.update(feedbackMessage); | ||
| uiFeedbackStream.done(); | ||
| return { type: 'DRAWING_TRIGGER', error: 'MCP client initialization failed' }; | ||
| } | ||
|
|
||
| try { | ||
| let features: any[] = []; | ||
| let center: [number, number] | null = null; | ||
|
|
||
| // Geocode location if provided | ||
| const locationToGeocode = (params as any).location; | ||
| if (locationToGeocode) { | ||
| feedbackMessage = `Geocoding location: ${locationToGeocode}...`; | ||
| uiFeedbackStream.update(feedbackMessage); | ||
|
|
||
| const toolCallResult = await mcpClient.callTool({ | ||
| name: 'forward_geocode_tool', | ||
| arguments: { searchText: locationToGeocode, maxResults: 1 } | ||
| }); | ||
|
|
||
| const serviceResponse = toolCallResult as { content?: Array<{ text?: string | null }> }; | ||
| const text = serviceResponse?.content?.[0]?.text; | ||
| if (text) { | ||
| const jsonMatch = text.match(/\`\`\`json\n([\s\S]*?)\n\`\`\`/); | ||
| const content = jsonMatch ? JSON.parse(jsonMatch[1]) : JSON.parse(text); | ||
| if (content.results?.[0]?.coordinates) { | ||
| const coords = content.results[0].coordinates; | ||
| center = [coords.longitude, coords.latitude]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (type === 'circle') { | ||
| const circleCenter = params.center ? [params.center.lng, params.center.lat] : center; | ||
| if (!circleCenter) throw new Error('Could not determine center for circle'); | ||
|
|
||
| feedbackMessage = `Generating circle around ${locationToGeocode || 'specified coordinates'} with radius ${params.radius} ${params.units}...`; | ||
| uiFeedbackStream.update(feedbackMessage); | ||
|
|
||
| const circle = turf.circle(circleCenter, params.radius, { | ||
| units: params.units as any, | ||
| steps: 64, | ||
| properties: { | ||
| user_isCircle: true, | ||
| user_radius: params.radius, | ||
| user_radiusUnits: params.units, | ||
| user_center: circleCenter, | ||
| user_label: params.label, | ||
| user_color: params.color | ||
| } | ||
| }); | ||
| features.push(circle); | ||
| } else if (type === 'polygon') { | ||
| const polyCoords = params.coordinates | ||
| ? [params.coordinates.map(c => [c.lng, c.lat])] | ||
| : null; // If no coords, we might want to use geocoded center but it's just a point | ||
|
|
||
| if (!polyCoords) { | ||
| if (center) { | ||
| // Fallback: draw a small square around the center if geocoded but no vertices | ||
| const buffered = turf.buffer(turf.point(center), 0.5, { units: 'kilometers' }); | ||
| if (buffered) { | ||
| buffered.properties = { ...buffered.properties, user_label: params.label, user_color: params.color }; | ||
| features.push(buffered); | ||
| } | ||
| } else { | ||
| throw new Error('No coordinates or location provided for polygon'); | ||
| } | ||
| } else { | ||
| // Ensure polygon is closed | ||
| if (polyCoords[0][0][0] !== polyCoords[0][polyCoords[0].length-1][0] || polyCoords[0][0][1] !== polyCoords[0][polyCoords[0].length-1][1]) { | ||
| polyCoords[0].push(polyCoords[0][0]); | ||
| } | ||
| const polygon = turf.polygon(polyCoords, { | ||
| user_label: params.label, | ||
| user_color: params.color |
There was a problem hiding this comment.
This tool parses MCP responses using JSON.parse(text) (or a regex-extracted code fence) without validating shape, and it uses multiple any casts ((params as any).location, params.units as any, error: any, features: any[]). Given the tool is exposed to LLM-driven inputs and a remote MCP server, this is brittle: a small change in server formatting will throw and you’ll return DRAWING_TRIGGER with an error.
Also, turf is imported but only used for geometry generation; in the polygon fallback comment you say “small square” but you actually create a circular buffer polygon.
Finally, the tool returns { type: 'DRAWING_TRIGGER', error: ... } without a timestamp, while your ToolOutput interface in MapQueryHandler requires timestamp: string (compiler may pass due to separate types, but the runtime payload shape becomes inconsistent for consumers/logging).
Suggestion
- Standardize the return payload shape (always include
timestamp, and includefeatures: []when failing). - Avoid
anyin parsing/params access; prefer discriminated union narrowing. - Parse MCP tool output defensively (try/catch around JSON parsing and validate minimal fields before using).
Sketch:
const now = () => new Date().toISOString();
const fail = (message: string) => ({
type: 'DRAWING_TRIGGER' as const,
timestamp: now(),
features: [],
error: message,
});
// ...
let locationToGeocode: string | undefined;
if (params.type !== 'circle') locationToGeocode = params.location;
else locationToGeocode = params.location;
// when parsing
let parsed: unknown;
try {
parsed = jsonMatch ? JSON.parse(jsonMatch[1]) : JSON.parse(text);
} catch {
return fail('Geocoding response was not valid JSON');
}
// validate minimally
const coords = (parsed as any)?.results?.[0]?.coordinates;
if (coords && typeof coords.longitude === 'number' && typeof coords.latitude === 'number') {
center = [coords.longitude, coords.latitude];
}Reply with "@CharlieHelps yes please" if you’d like me to add a commit that standardizes the tool output + hardens parsing while keeping behavior the same.
| export function getGoogleStaticMapUrl(latitude: number, longitude: number): string { | ||
| const apiKey = process.env.GOOGLE_MAPS_API_KEY; | ||
| return `https://maps.googleapis.com/maps/api/staticmap?center=${latitude},${longitude}&zoom=14&size=600x300&maptype=roadmap&markers=color:red%7C${latitude},${longitude}&key=${apiKey}`; | ||
| } |
There was a problem hiding this comment.
getGoogleStaticMapUrl now always reads process.env.GOOGLE_MAPS_API_KEY and unconditionally returns a URL even if the key is missing, which will generate requests like ...&key=undefined.
Previously you were checking NEXT_PUBLIC_GOOGLE_MAPS_API_KEY || GOOGLE_MAPS_API_KEY and returning an empty string if absent. This change can cause confusing UI behavior (broken images) and potentially leak that the key is missing via a visible URL.
Suggestion
Restore the guard and prefer the previous env fallback behavior:
export function getGoogleStaticMapUrl(latitude: number, longitude: number): string {
const apiKey = process.env.NEXT_PUBLIC_GOOGLE_MAPS_API_KEY ?? process.env.GOOGLE_MAPS_API_KEY;
if (!apiKey) return '';
return `https://maps.googleapis.com/maps/api/staticmap?center=${latitude},${longitude}` +
`&zoom=14&size=600x300&maptype=roadmap&markers=color:red%7C${latitude},${longitude}&key=${apiKey}`;
}Reply with "@CharlieHelps yes please" and I can add a commit with this fix.
| export async function getConnectedMcpClient(): Promise<McpClient | null> { | ||
| const composioApiKey = process.env.COMPOSIO_API_KEY; | ||
| const mapboxAccessToken = process.env.MAPBOX_ACCESS_TOKEN; | ||
| const composioUserId = process.env.COMPOSIO_USER_ID; | ||
|
|
||
| console.log('[MCP Utility] Environment check:', { | ||
| composioApiKey: composioApiKey ? `${composioApiKey.substring(0, 8)}...` : 'MISSING', | ||
| mapboxAccessToken: mapboxAccessToken ? `${mapboxAccessToken.substring(0, 8)}...` : 'MISSING', | ||
| composioUserId: composioUserId ? `${composioUserId.substring(0, 8)}...` : 'MISSING', | ||
| }); | ||
|
|
||
| if (!composioApiKey || !mapboxAccessToken || !composioUserId || !composioApiKey.trim() || !mapboxAccessToken.trim() || !composioUserId.trim()) { | ||
| console.error('[MCP Utility] Missing or empty required environment variables'); | ||
| return null; | ||
| } | ||
|
|
||
| // Build Composio MCP server URL | ||
| let serverUrlToUse: URL; | ||
| try { |
There was a problem hiding this comment.
getConnectedMcpClient logs partial API keys (substring(0, 8)) for COMPOSIO_API_KEY, MAPBOX_ACCESS_TOKEN, and COMPOSIO_USER_ID. Even truncated secrets are still secrets and can be used for correlation/leakage in logs.
Additionally, mapboxAccessToken is validated but never used to build the MCP URL or passed to the server, which suggests either (a) the check is unnecessary or (b) a missing required parameter is being ignored.
Suggestion
- Remove secret material from logs entirely (log boolean presence only).
- If
MAPBOX_ACCESS_TOKENis truly required, pass it to the server (e.g., as an extra query param or header) or remove the validation to avoid false negatives.
Example safe logging:
console.log('[MCP Utility] Env check', {
hasComposioApiKey: Boolean(composioApiKey?.trim()),
hasMapboxAccessToken: Boolean(mapboxAccessToken?.trim()),
hasComposioUserId: Boolean(composioUserId?.trim()),
});Reply with "@CharlieHelps yes please" if you want me to add a commit removing secret logging and clarifying/aligning the MAPBOX_ACCESS_TOKEN requirement.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/map/map-data-context.tsx (1)
15-28: 🧹 Nitpick | 🔵 TrivialType
pendingFeaturesto a concrete feature shape (avoidany[]).
pendingFeaturesfeeds map rendering; leaving it asany[]makes it easy to pass malformed shapes that only fail at runtime.Proposed refactor
+type PendingFeature = { + type: 'Feature'; + geometry: { type: string; coordinates: unknown }; + properties?: Record<string, unknown>; +}; + 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 @@ - pendingFeatures?: any[]; // For programmatic drawing commands + pendingFeatures?: PendingFeature[]; // For programmatic drawing commands markers?: Array<{ latitude: number; longitude: number; title?: string; }>; }lib/agents/tools/geospatial.tsx (1)
219-235: 🧹 Nitpick | 🔵 TrivialRetry logic doesn't re-establish MCP connection on failure.
The retry loop retries the
callTooloperation but doesn't handle cases where the MCP client connection itself becomes stale or disconnected. If the connection drops mid-retry, subsequent retries will fail with the same client instance.Consider adding connection health check in retry loop
while (retryCount < MAX_RETRIES) { try { toolCallResult = await Promise.race([ mcpClient.callTool({ name: toolName ?? 'unknown_tool', arguments: toolArgs }), new Promise((_, reject) => setTimeout(() => reject(new Error('Tool call timeout')), 30000)), ]); break; } catch (error: any) { retryCount++; - if (retryCount === MAX_RETRIES) throw new Error(`Tool call failed after ${MAX_RETRIES} retries: ${error.message}`); + if (retryCount === MAX_RETRIES) { + throw new Error(`Tool call failed after ${MAX_RETRIES} retries: ${error.message}`); + } console.warn(`[GeospatialTool] Retry ${retryCount}/${MAX_RETRIES}: ${error.message}`); - await new Promise(resolve => setTimeout(resolve, 1000)); + // Exponential backoff + await new Promise(resolve => setTimeout(resolve, 1000 * retryCount)); } }
🤖 Fix all issues with AI agents
In `@changes.diff`:
- Around line 1009-1014: The branch handling type === 'line' currently throws
when params.coordinates is missing (lineCoords), but the schema allows a
location-only input; either make coordinates required for 'line' in the schema
or add a location fallback like polygon: if params.coordinates is absent and
params.location exists, call your geocoding helper to resolve params.location
into an array of coordinates and assign to lineCoords (validate min 2 points)
before proceeding; update the 'line' schema (if chosen) to remove location and
require coordinates instead.
- Around line 1439-1442: The getGoogleStaticMapUrl function builds a URL with
process.env.GOOGLE_MAPS_API_KEY but doesn't validate it; update
getGoogleStaticMapUrl to read the API key into a local variable (apiKey), check
that apiKey is present and non-empty, and if not either throw a clear error or
return null/undefined per project convention (do not embed "undefined" in the
URL); then construct and return the URL using that validated apiKey. Ensure you
reference getGoogleStaticMapUrl and process.env.GOOGLE_MAPS_API_KEY in the fix.
- Around line 1418-1426: The circle schema currently allows a circle without
coordinates because both location and center are optional; add a Zod refinement
on the circle object (the schema containing fields location, center, radius,
units, label, color) using .refine(data => Boolean(data.location ||
data.center), { message: 'Either location or center must be provided' }) so
validation fails unless at least one of location or center is present; keep
radius required and leave other fields unchanged.
- Around line 173-197: The change removes diagnostic logs when toolOutput is
missing or invalid, reducing observability; restore minimal logging in
MapQueryHandler by logging a warning when toolOutput is falsy and when
MAP_QUERY_TRIGGER payload is missing required fields, and keep the existing
console.log for DRAWING_TRIGGER; specifically, in the MapQueryHandler before
returning on !toolOutput call processLogger.warn or console.warn with context
including "MapQueryHandler: missing toolOutput" and when handling
toolOutput.type === 'MAP_QUERY_TRIGGER' validate mcp_response.location and log a
warning like "MapQueryHandler: invalid MAP_QUERY_TRIGGER payload" (include
toolOutput and toolOutput.mcp_response) before skipping setMapData, while
leaving setMapData and MapToggleEnum.DrawingMode handling unchanged.
- Around line 999-1002: The closure check for polygons currently indexes
polyCoords[0] without validating it may be empty; update the logic around the
polygon handling (the block that ensures the polygon is closed) to first verify
polyCoords[0].length >= 3, then assign const first = polyCoords[0][0] and const
last = polyCoords[0][polyCoords[0].length - 1] and compare first and last before
pushing, and if length < 3 throw a clear Error('Polygon requires at least 3
coordinates') so you avoid out-of-bounds access and provide a meaningful failure
mode.
- Around line 1463-1467: The console.log for "[MCP Utility] Environment check"
currently prints truncated API keys (composioApiKey, mapboxAccessToken,
composioUserId); make this conditional on a debug flag or non-production
environment so keys are never logged in production: wrap the console.log in a
check (eg. process.env.DEBUG === 'true' or NODE_ENV !== 'production') or replace
with a proper logger that respects log levels and only emits the truncated key
info when debug/trace is enabled.
In `@components/map/draw-modes/circle-mode.ts`:
- Line 3: Replace the broad "any" on the CircleMode constant with precise
TypeScript interfaces: define a CircleModeState (with circle.id,
circle.properties.user_isCircle, user_center, optional user_radiusInKm, and
setCoordinates signature) and a DrawEvent (with lngLat {lng, lat}), then type
CircleMode as an object implementing the mapbox-gl-draw mode methods using those
interfaces (e.g., handlers like onMouseMove(event: DrawEvent), onClick(event:
DrawEvent), and state: CircleModeState). Update function and method signatures
inside CircleMode to use these types (referencing CircleMode, CircleModeState,
DrawEvent, and methods like setCoordinates) to improve IDE support and catch
type errors.
- Around line 46-54: onMouseMove in circle-mode.ts can produce a zero/near-zero
radius (using turf.distance and turf.circle) which yields degenerate geometry;
clamp the computed distance to a small minimum (e.g. minRadiusKm) before calling
turf.circle, set state.circle.properties.user_radiusInKm to the clamped value,
and then call state.circle.setCoordinates with the circle generated from the
clamped radius so the circle always has a visible area even when clicks are on
the same point.
- Around line 56-58: Add an onStop handler to the circle draw mode to perform
cleanup when the mode is exited (similar to how onKeyUp calls
this.changeMode('simple_select')); implement onStop on the same mode object and
make it cancel any in-progress draw action, remove temporary layers/markers,
reset cursor/interaction state, and clear any mode-specific state so incomplete
features aren’t left in the draw state when changeMode or programmatic exits
occur.
In `@components/map/mapbox-map.tsx`:
- Around line 291-303: The current DOM query for '.mapbox-gl-draw_polygon' and
appending a circle button is fragile; instead, if a proper MapboxDraw or control
API is available use that to register a custom control/button, otherwise add a
defensive null check and explicit error logging when you cannot find the target
container: check the result of document.querySelector('.mapbox-gl-draw_polygon')
and its parent before creating circleBtn, log a warning via console.warn or a
logger (include context like "mapbox draw control group not found") and avoid
silently failing, and ensure the created element still calls
drawRef.current?.changeMode('draw_circle') only when drawRef.current exists to
prevent runtime errors.
- Around line 515-523: The effect that drains mapData.pendingFeatures may miss
running when drawRef.current becomes available later; extract the
pending-features logic into a helper (e.g., processPendingFeatures) that
iterates over mapData.pendingFeatures, calls drawRef.current.add(feature),
clears pendingFeatures via setMapData, and calls updateMeasurementLabels, then
call that helper both from the existing useEffect (keep listening to
mapData.pendingFeatures, updateMeasurementLabels, setMapData) and from
setupDrawingTools right after drawRef/current is assigned/added so pending
features are processed as soon as drawing tools are ready.
In `@lib/agents/researcher.tsx`:
- Around line 81-86: The decision flow in the summary places geospatialQueryTool
before drawingQueryTool which creates ambiguity for mixed queries (e.g., "Draw a
1km circle around the Eiffel Tower"); update the logic in researcher.tsx so that
drawing-specific intents are checked before geospatial lookups or add an
explicit tie-break rule that if both drawingQueryTool and geospatialQueryTool
apply, prefer drawingQueryTool (and then call geospatialQueryTool as a substep
to resolve locations), ensuring the flow names retrieve, drawingQueryTool,
geospatialQueryTool, search remain referenced to locate the change.
In `@lib/agents/tools/drawing.tsx`:
- Around line 44-47: Update the mcpClient.callTool invocation for the
forward_geocode_tool (the call that uses name: 'forward_geocode_tool' and
arguments: { searchText: locationToGeocode, maxResults: 1 }) to pass
RequestOptions with a shorter timeout (e.g. 10000 ms) as the third parameter
instead of relying on an external wrapper; specifically, call
mcpClient.callTool(..., undefined, { timeout: 10000 }) so the SDK's built‑in
timeout is used for the interactive geocode request.
In `@lib/schema/drawing.tsx`:
- Around line 1-36: Update drawingToolSchema to enforce geographic bounds and
required shape data: for the polygon variant (type: 'polygon') require
coordinates with min length 3 and validate each point's lat ∈ [-90,90] and lng ∈
[-180,180]; for the line variant (type: 'line') require coordinates with min
length 2 and apply the same lat/lng bounds; for the circle variant (type:
'circle') require that either center (with lat/lng bounds) OR location is
present (use z.refine or z.union/transform to enforce the conditional), and
ensure radius is strictly positive (>0); apply the lat/lng numeric range checks
to all coordinate objects and replace .optional() on coordinate arrays/center
with the appropriate min()/refinement to prevent empty arrays/absent positioning
while keeping location optional where allowed.
In `@lib/utils/index.ts`:
- Around line 121-124: getGoogleStaticMapUrl currently embeds
process.env.GOOGLE_MAPS_API_KEY directly and can return a broken URL; update the
function getGoogleStaticMapUrl to first check that
process.env.GOOGLE_MAPS_API_KEY is set and throw a clear Error (or return a
controlled failure) if missing, then construct the URL using URL and
URLSearchParams (set center as `${latitude},${longitude}`, zoom, size, maptype,
markers and key via URLSearchParams) so query values are properly encoded and no
`key=undefined` can be returned.
In `@lib/utils/mcp.ts`:
- Around line 15-19: The console.log in lib/utils/mcp.ts currently prints
partial values of composioApiKey, mapboxAccessToken, and composioUserId; change
it to avoid any secret or identifier substrings and instead log only presence
(e.g., boolean or "PRESENT"/"MISSING") for composioApiKey, mapboxAccessToken,
and composioUserId so no prefixes or partial secrets are emitted by the
console.log in this module.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
changes.diffcomponents/map/draw-modes/circle-mode.tscomponents/map/map-data-context.tsxcomponents/map/map-query-handler.tsxcomponents/map/mapbox-map.tsxlib/agents/researcher.tsxlib/agents/tools/drawing.tsxlib/agents/tools/geospatial.tsxlib/agents/tools/index.tsxlib/schema/drawing.tsxlib/utils/index.tslib/utils/mcp.ts
🧰 Additional context used
🧬 Code graph analysis (8)
lib/utils/mcp.ts (3)
mapbox_mcp/index.ts (1)
testComposioConnection(3-52)mapbox_mcp/hooks.ts (3)
composioClientRef(106-121)initClient(60-72)composioClientRef(62-70)mapbox_mcp/composio-mapbox.ts (3)
initializeComposioMapbox(96-115)getComposioClient(121-132)authenticateToolkit(59-89)
components/map/map-data-context.tsx (2)
components/map/google-map.tsx (1)
features(38-52)components/map/google-geojson-layer.tsx (1)
GoogleGeoJsonLayerProps(6-8)
lib/schema/drawing.tsx (2)
lib/agents/resolution-search.tsx (1)
DrawnFeature(26-31)components/map-toggle-context.tsx (1)
MapToggleEnum(5-9)
lib/agents/tools/drawing.tsx (4)
lib/schema/drawing.tsx (1)
drawingToolSchema(3-36)components/message.tsx (1)
BotMessage(11-31)lib/utils/mcp.ts (2)
getConnectedMcpClient(10-79)closeClient(84-95)lib/actions/chat.ts (1)
updateDrawingContext(165-205)
components/map/map-query-handler.tsx (2)
components/map-toggle.tsx (1)
MapToggle(14-39)components/map-toggle-context.tsx (3)
MapToggleContextType(11-14)type(25-27)mapToggleState(22-34)
lib/agents/researcher.tsx (2)
lib/agents/resolution-search.tsx (1)
resolutionSearch(33-79)app/actions.tsx (1)
processResolutionSearch(90-187)
components/map/mapbox-map.tsx (3)
components/map-toggle-context.tsx (1)
useMapToggle(36-42)components/map/map-data-context.tsx (1)
useMapData(52-58)components/map-loading-context.tsx (1)
useMapLoading(20-26)
lib/agents/tools/index.tsx (1)
lib/agents/tools/drawing.tsx (1)
drawingTool(8-142)
🔇 Additional comments (8)
components/map/map-query-handler.tsx (1)
38-44: LGTM — drawing trigger switches to drawing mode and queues pending features.lib/agents/tools/index.tsx (1)
26-33: LGTM — drawing tool wired into the tool registry.lib/agents/researcher.tsx (1)
50-61: LGTM - Well-structured tool documentation.The new
drawingQueryToolsection follows the established pattern for other tools and provides clear guidance on when to use it. The behavioral instructions ("NEVER say 'drawing shape'") help ensure consistent AI responses.lib/agents/tools/geospatial.tsx (1)
11-12: LGTM - Good refactoring to shared utilities.Moving MCP client logic to a shared utility module (
lib/utils/mcp.ts) reduces code duplication and improves maintainability. The imports are correctly structured.components/map/mapbox-map.tsx (3)
6-9: LGTM - Clean import updates.Good switch from react-toastify to sonner (modern toast library) and proper integration of CircleMode.
81-89: LGTM - Good circle measurement labeling.The measurement label now shows both radius and area for circles (
R: X, A: Y), providing useful information to users. The conditional check foruser_isCircleanduser_radiusInKmcorrectly identifies circle features.
184-197: updateMapPosition no longer triggers rotation after flyTo in RealTimeMode.The previous implementation called
startRotation()after the position update animation completed when in RealTimeMode. This behavior has been removed, which changes the UX where the map would previously auto-rotate immediately after centering on the user's location. Rotation will now only resume after 30 seconds of user inactivity.Verify this is intentional. If rotation should resume after geolocation-based position updates in RealTimeMode, restore the removed
startRotation()call in theupdateMapPositionsetTimeout callback.changes.diff (1)
9-10: Next.js patch version upgrade looks fine.The Next.js upgrade from 15.3.6 to 15.3.8 is a patch release. Note that there have been security vulnerabilities disclosed in Next.js 15 - "Next.js versions 15 and 16, up to and including 15.0.4, 15.1.8, 15.2.5, 15.3.5, 15.4.7, 15.5.6" - this means 15.3.8 should include security patches. This is a good upgrade.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - if (toolOutput && toolOutput.mcp_response && toolOutput.mcp_response.location) { | ||
| + if (!toolOutput) return; | ||
| + | ||
| + if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) { | ||
| + console.log('MapQueryHandler: Received drawing data.', toolOutput.features); | ||
| + setMapType(MapToggleEnum.DrawingMode); | ||
| + setMapData(prevData => ({ | ||
| + ...prevData, | ||
| + pendingFeatures: toolOutput.features | ||
| + })); | ||
| + } else if (toolOutput.type === 'MAP_QUERY_TRIGGER' && toolOutput.mcp_response && toolOutput.mcp_response.location) { | ||
| const { latitude, longitude, place_name } = toolOutput.mcp_response.location; | ||
|
|
||
| if (typeof latitude === 'number' && typeof longitude === 'number') { | ||
| @@ -39,44 +50,14 @@ export const MapQueryHandler: React.FC<MapQueryHandlerProps> = ({ toolOutput }) | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| targetPosition: { lat: latitude, lng: longitude }, | ||
| - // 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 | ||
| } | ||
| })); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Removed error logging reduces observability.
The previous implementation logged warnings when toolOutput had missing or invalid data. The new implementation silently returns without any logging, making it harder to debug issues when the map doesn't update as expected.
Restore minimal logging for debugging
useEffect(() => {
if (!toolOutput) return;
if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) {
console.log('MapQueryHandler: Received drawing data.', toolOutput.features);
setMapType(MapToggleEnum.DrawingMode);
setMapData(prevData => ({
...prevData,
pendingFeatures: toolOutput.features
}));
} else if (toolOutput.type === 'MAP_QUERY_TRIGGER' && toolOutput.mcp_response && toolOutput.mcp_response.location) {
const { latitude, longitude, place_name } = toolOutput.mcp_response.location;
if (typeof latitude === 'number' && typeof longitude === 'number') {
console.log('MapQueryHandler: Panning map to:', place_name || `${latitude},${longitude}`);
setMapData(prevData => ({
...prevData,
targetPosition: { lat: latitude, lng: longitude },
mapFeature: {
place_name,
mapUrl: toolOutput.mcp_response?.mapUrl
}
}));
+ } else {
+ console.warn('MapQueryHandler: Invalid coordinates in toolOutput', { latitude, longitude });
}
+ } else if (toolOutput.type) {
+ console.warn('MapQueryHandler: Unhandled or incomplete toolOutput', { type: toolOutput.type, hasFeatures: !!toolOutput.features, hasMcpResponse: !!toolOutput.mcp_response });
}
}, [toolOutput, setMapData, setMapType]);📝 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.
| - if (toolOutput && toolOutput.mcp_response && toolOutput.mcp_response.location) { | |
| + if (!toolOutput) return; | |
| + | |
| + if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) { | |
| + console.log('MapQueryHandler: Received drawing data.', toolOutput.features); | |
| + setMapType(MapToggleEnum.DrawingMode); | |
| + setMapData(prevData => ({ | |
| + ...prevData, | |
| + pendingFeatures: toolOutput.features | |
| + })); | |
| + } else if (toolOutput.type === 'MAP_QUERY_TRIGGER' && toolOutput.mcp_response && toolOutput.mcp_response.location) { | |
| const { latitude, longitude, place_name } = toolOutput.mcp_response.location; | |
| if (typeof latitude === 'number' && typeof longitude === 'number') { | |
| @@ -39,44 +50,14 @@ export const MapQueryHandler: React.FC<MapQueryHandlerProps> = ({ toolOutput }) | |
| setMapData(prevData => ({ | |
| ...prevData, | |
| targetPosition: { lat: latitude, lng: longitude }, | |
| - // 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 | |
| } | |
| })); | |
| if (!toolOutput) return; | |
| if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) { | |
| console.log('MapQueryHandler: Received drawing data.', toolOutput.features); | |
| setMapType(MapToggleEnum.DrawingMode); | |
| setMapData(prevData => ({ | |
| ...prevData, | |
| pendingFeatures: toolOutput.features | |
| })); | |
| } else if (toolOutput.type === 'MAP_QUERY_TRIGGER' && toolOutput.mcp_response && toolOutput.mcp_response.location) { | |
| const { latitude, longitude, place_name } = toolOutput.mcp_response.location; | |
| if (typeof latitude === 'number' && typeof longitude === 'number') { | |
| console.log('MapQueryHandler: Panning map to:', place_name || `${latitude},${longitude}`); | |
| setMapData(prevData => ({ | |
| ...prevData, | |
| targetPosition: { lat: latitude, lng: longitude }, | |
| mapFeature: { | |
| place_name, | |
| mapUrl: toolOutput.mcp_response?.mapUrl | |
| } | |
| })); | |
| } else { | |
| console.warn('MapQueryHandler: Invalid coordinates in toolOutput', { latitude, longitude }); | |
| } | |
| } else if (toolOutput.type) { | |
| console.warn('MapQueryHandler: Unhandled or incomplete toolOutput', { type: toolOutput.type, hasFeatures: !!toolOutput.features, hasMcpResponse: !!toolOutput.mcp_response }); | |
| } |
🤖 Prompt for AI Agents
In `@changes.diff` around lines 173 - 197, The change removes diagnostic logs when
toolOutput is missing or invalid, reducing observability; restore minimal
logging in MapQueryHandler by logging a warning when toolOutput is falsy and
when MAP_QUERY_TRIGGER payload is missing required fields, and keep the existing
console.log for DRAWING_TRIGGER; specifically, in the MapQueryHandler before
returning on !toolOutput call processLogger.warn or console.warn with context
including "MapQueryHandler: missing toolOutput" and when handling
toolOutput.type === 'MAP_QUERY_TRIGGER' validate mcp_response.location and log a
warning like "MapQueryHandler: invalid MAP_QUERY_TRIGGER payload" (include
toolOutput and toolOutput.mcp_response) before skipping setMapData, while
leaving setMapData and MapToggleEnum.DrawingMode handling unchanged.
| + // Ensure polygon is closed | ||
| + if (polyCoords[0][0][0] !== polyCoords[0][polyCoords[0].length-1][0] || polyCoords[0][0][1] !== polyCoords[0][polyCoords[0].length-1][1]) { | ||
| + polyCoords[0].push(polyCoords[0][0]); | ||
| + } |
There was a problem hiding this comment.
Array bounds not checked before polygon closure validation.
The polygon closure check accesses array indices without verifying the array has elements, which could throw if polyCoords[0] is empty.
🛡️ Add bounds check
The diff shows (around line 1000 in the unified diff):
+ } else {
+ // Ensure polygon is closed
+ if (polyCoords[0][0][0] !== polyCoords[0][polyCoords[0].length-1][0] || polyCoords[0][0][1] !== polyCoords[0][polyCoords[0].length-1][1]) {Should be:
} else {
// Ensure polygon is closed
if (polyCoords[0].length >= 3) {
const first = polyCoords[0][0];
const last = polyCoords[0][polyCoords[0].length - 1];
if (first[0] !== last[0] || first[1] !== last[1]) {
polyCoords[0].push(polyCoords[0][0]);
}
} else {
throw new Error('Polygon requires at least 3 coordinates');
}🤖 Prompt for AI Agents
In `@changes.diff` around lines 999 - 1002, The closure check for polygons
currently indexes polyCoords[0] without validating it may be empty; update the
logic around the polygon handling (the block that ensures the polygon is closed)
to first verify polyCoords[0].length >= 3, then assign const first =
polyCoords[0][0] and const last = polyCoords[0][polyCoords[0].length - 1] and
compare first and last before pushing, and if length < 3 throw a clear
Error('Polygon requires at least 3 coordinates') so you avoid out-of-bounds
access and provide a meaningful failure mode.
| + } else if (type === 'line') { | ||
| + const lineCoords = params.coordinates | ||
| + ? params.coordinates.map(c => [c.lng, c.lat]) | ||
| + : null; | ||
| + | ||
| + if (!lineCoords) throw new Error('No coordinates provided for line'); |
There was a problem hiding this comment.
Line drawing requires coordinates but schema allows location-only.
The line type in the schema has coordinates as optional with location as an alternative, but the drawing tool throws if no coordinates are provided. However, if a user says "Draw a line at Central Park", the tool could potentially geocode the location but doesn't use it for lines (unlike circles/polygons).
Consider supporting geocoded location for lines or making coordinates required in schema
Either update the schema to make coordinates required for lines:
z.object({
type: z.literal('line'),
coordinates: z.array(z.object({
lat: z.number(),
lng: z.number()
})).min(2).describe('List of coordinates for the line segments (required)'),
// remove location for line type
label: z.string().optional(),
color: z.string().optional()
})Or handle the geocoded location in the line branch similar to polygon fallback.
🤖 Prompt for AI Agents
In `@changes.diff` around lines 1009 - 1014, The branch handling type === 'line'
currently throws when params.coordinates is missing (lineCoords), but the schema
allows a location-only input; either make coordinates required for 'line' in the
schema or add a location fallback like polygon: if params.coordinates is absent
and params.location exists, call your geocoding helper to resolve
params.location into an array of coordinates and assign to lineCoords (validate
min 2 points) before proceeding; update the 'line' schema (if chosen) to remove
location and require coordinates instead.
| + location: z.string().optional().describe('Name of the place to draw a circle around'), | ||
| + center: z.object({ | ||
| + lat: z.number(), | ||
| + lng: z.number() | ||
| + }).optional().describe('Center coordinates for the circle'), | ||
| + radius: z.number().describe('Radius of the circle'), | ||
| + units: z.enum(['meters', 'kilometers', 'miles', 'feet']).default('kilometers').describe('Units for the radius'), | ||
| + label: z.string().optional().describe('Label for the circle'), | ||
| + color: z.string().optional().describe('Color for the circle (e.g., "#00ff00")') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Schema allows circle without center or location.
The circle variant requires radius but both location and center are optional. This means a valid schema input could have neither, leading to a runtime error in the drawing tool. Consider adding a refinement to ensure at least one is provided.
Add schema refinement for circle center requirement
z.object({
type: z.literal('circle'),
location: z.string().optional().describe('Name of the place to draw a circle around'),
center: z.object({
lat: z.number(),
lng: z.number()
}).optional().describe('Center coordinates for the circle'),
radius: z.number().describe('Radius of the circle'),
units: z.enum(['meters', 'kilometers', 'miles', 'feet']).default('kilometers').describe('Units for the radius'),
label: z.string().optional().describe('Label for the circle'),
color: z.string().optional().describe('Color for the circle (e.g., "#00ff00")')
- })
+ }).refine(
+ data => data.location || data.center,
+ { message: 'Either location or center must be provided for circle' }
+ )
]);🤖 Prompt for AI Agents
In `@changes.diff` around lines 1418 - 1426, The circle schema currently allows a
circle without coordinates because both location and center are optional; add a
Zod refinement on the circle object (the schema containing fields location,
center, radius, units, label, color) using .refine(data => Boolean(data.location
|| data.center), { message: 'Either location or center must be provided' }) so
validation fails unless at least one of location or center is present; keep
radius required and leave other fields unchanged.
| + | ||
| +export function getGoogleStaticMapUrl(latitude: number, longitude: number): string { | ||
| + const apiKey = process.env.GOOGLE_MAPS_API_KEY; | ||
| + return `https://maps.googleapis.com/maps/api/staticmap?center=${latitude},${longitude}&zoom=14&size=600x300&maptype=roadmap&markers=color:red%7C${latitude},${longitude}&key=${apiKey}`; |
There was a problem hiding this comment.
Missing API key check in getGoogleStaticMapUrl.
The function doesn't check if the API key exists before constructing the URL. If GOOGLE_MAPS_API_KEY is undefined, the URL will include key=undefined, which will fail silently and may leak information in logs.
🛡️ Add API key validation
export function getGoogleStaticMapUrl(latitude: number, longitude: number): string {
const apiKey = process.env.GOOGLE_MAPS_API_KEY;
+ if (!apiKey) {
+ console.warn('[Utils] GOOGLE_MAPS_API_KEY not configured');
+ return '';
+ }
return `https://maps.googleapis.com/maps/api/staticmap?center=${latitude},${longitude}&zoom=14&size=600x300&maptype=roadmap&markers=color:red%7C${latitude},${longitude}&key=${apiKey}`;
}📝 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.
| + | |
| +export function getGoogleStaticMapUrl(latitude: number, longitude: number): string { | |
| + const apiKey = process.env.GOOGLE_MAPS_API_KEY; | |
| + return `https://maps.googleapis.com/maps/api/staticmap?center=${latitude},${longitude}&zoom=14&size=600x300&maptype=roadmap&markers=color:red%7C${latitude},${longitude}&key=${apiKey}`; | |
| export function getGoogleStaticMapUrl(latitude: number, longitude: number): string { | |
| const apiKey = process.env.GOOGLE_MAPS_API_KEY; | |
| if (!apiKey) { | |
| console.warn('[Utils] GOOGLE_MAPS_API_KEY not configured'); | |
| return ''; | |
| } | |
| return `https://maps.googleapis.com/maps/api/staticmap?center=${latitude},${longitude}&zoom=14&size=600x300&maptype=roadmap&markers=color:red%7C${latitude},${longitude}&key=${apiKey}`; | |
| } |
🤖 Prompt for AI Agents
In `@changes.diff` around lines 1439 - 1442, The getGoogleStaticMapUrl function
builds a URL with process.env.GOOGLE_MAPS_API_KEY but doesn't validate it;
update getGoogleStaticMapUrl to read the API key into a local variable (apiKey),
check that apiKey is present and non-empty, and if not either throw a clear
error or return null/undefined per project convention (do not embed "undefined"
in the URL); then construct and return the URL using that validated apiKey.
Ensure you reference getGoogleStaticMapUrl and process.env.GOOGLE_MAPS_API_KEY
in the fix.
| useEffect(() => { | ||
| if (mapData.pendingFeatures && mapData.pendingFeatures.length > 0 && drawRef.current) { | ||
| mapData.pendingFeatures.forEach(feature => { | ||
| drawRef.current?.add(feature); | ||
| }); | ||
| setMapData(prev => ({ ...prev, pendingFeatures: [] })); | ||
| setTimeout(updateMeasurementLabels, 100); | ||
| } | ||
| }, [mapData.pendingFeatures, updateMeasurementLabels, setMapData]); |
There was a problem hiding this comment.
Potential race condition with pendingFeatures processing.
This effect processes pendingFeatures when both the array has items AND drawRef.current exists. However, if pendingFeatures is set before drawing tools are set up (e.g., during initial mode switch to DrawingMode), the features may not be processed. The effect doesn't re-run when drawRef.current becomes available.
🐛 Proposed fix to handle timing
useEffect(() => {
- if (mapData.pendingFeatures && mapData.pendingFeatures.length > 0 && drawRef.current) {
+ if (!mapData.pendingFeatures || mapData.pendingFeatures.length === 0) return;
+
+ // If draw tools aren't ready, wait for them
+ if (!drawRef.current) {
+ console.log('[MapboxMap] Pending features waiting for draw tools to initialize');
+ return;
+ }
+
mapData.pendingFeatures.forEach(feature => {
drawRef.current?.add(feature);
});
setMapData(prev => ({ ...prev, pendingFeatures: [] }));
setTimeout(updateMeasurementLabels, 100);
- }
}, [mapData.pendingFeatures, updateMeasurementLabels, setMapData]);Additionally, consider triggering this processing in setupDrawingTools after the draw control is added.
🤖 Prompt for AI Agents
In `@components/map/mapbox-map.tsx` around lines 515 - 523, The effect that drains
mapData.pendingFeatures may miss running when drawRef.current becomes available
later; extract the pending-features logic into a helper (e.g.,
processPendingFeatures) that iterates over mapData.pendingFeatures, calls
drawRef.current.add(feature), clears pendingFeatures via setMapData, and calls
updateMeasurementLabels, then call that helper both from the existing useEffect
(keep listening to mapData.pendingFeatures, updateMeasurementLabels, setMapData)
and from setupDrawingTools right after drawRef/current is assigned/added so
pending features are processed as soon as drawing tools are ready.
…ed validation - Implemented custom CircleMode for MapboxDraw. - Added Circle button to Mapbox drawing controls. - Created drawingQueryToolSchema and drawingQueryTool for AI-controlled drawing. - Implemented lat/lng bounds validation and coordinate requirements in Zod schema. - Added location fallback for line drawings. - Refactored and secured MCP client logic in lib/utils/mcp.ts. - Updated MapboxMap and MapQueryHandler with improved logging and programmatic drawing support. - Enhanced getGoogleStaticMapUrl with API key validation and proper encoding. - Reordered researcher agent decision logic to prioritize drawingQueryTool. - Updated measurement labels to display radius and area for circles. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@components/map/map-query-handler.tsx`:
- Around line 18-25: Move the ToolOutput interface out of
components/map/map-query-handler.tsx into a shared types file (e.g., create
lib/types/tools.ts) so both producer and consumer reference the same contract;
update both components/map/map-query-handler.tsx and
lib/agents/tools/drawing.tsx to import ToolOutput from that shared module, and
tighten the features field from any[] to a GeoJSON Feature[] (importing Feature
from 'geojson') so features is strongly typed.
- Around line 35-39: The MapQueryHandler useEffect currently logs a warning
whenever toolOutput is falsy, which is expected on initial renders; update the
useEffect in MapQueryHandler to remove the unconditional console.warn or guard
it behind a debug flag/ENV check so it only logs when explicit debugging is
enabled (e.g., check a DEBUG_MAP_QUERY or process.env.NODE_ENV === 'development'
before logging). Locate the useEffect that references toolOutput and replace the
unconditional warn with a conditional log tied to the debug flag, or simply
remove the warning entirely to prevent spamming the console during normal
operation.
In `@lib/agents/tools/drawing.tsx`:
- Around line 115-125: The fallback that sets lineCoords to a ±0.01 degree span
around center produces an almost-invisible line at many zoom levels; update the
logic inside the fallback that assigns lineCoords (referencing lineCoords and
center) to either use a larger default span (e.g., configurable constant
DEFAULT_LINE_SPAN_METERS converted to degrees based on latitude), expose that
span as a parameter or option so callers can control it, and add/update the tool
description to document the fallback behavior; alternatively, if you prefer
strict behavior, replace the fallback with throwing the existing Error('No
coordinates or location provided for line') so consumers must provide explicit
coordinates — implement one clear approach and apply it where lineCoords is
computed.
- Around line 38-62: The code unsafely accesses params via (params as
any).location and parses the forward_geocode_tool response without validating
its shape; update to read location in a type-safe way from the incoming request
type (use the established params type/interface and optional chaining like
params.location) and replace the fragile parsing of
toolCallResult/serviceResponse with guarded validation: treat toolCallResult as
a typed envelope from mcpClient.callTool, check that content is an array and
content[0].text is a string before attempting JSON.parse, wrap JSON.parse in
try/catch, and validate the parsed object has results[0].coordinates with
numeric latitude/longitude before assigning center (references: params,
mcpClient.callTool, forward_geocode_tool, serviceResponse, center).
- Around line 72-83: The code uses an unsafe any cast for turf circle units;
update the turf.circle call to cast params.units to the specific Turf Units type
instead of any (i.e., change params.units as any to params.units as Units) and
add an import for Units from "@turf/helpers"; target the turf.circle invocation
and the params.units usage in the function that constructs the circle so the
value is typed as Units.
In `@lib/utils/mcp.ts`:
- Around line 84-95: The closeClient function currently returns Promise<void>
but swallows errors; update closeClient to explicitly accept an optional boolean
parameter (e.g., throwOnError = false) and document the behavior: when
throwOnError is true re-throw the caught error so callers can handle failures,
otherwise continue to log and return void; change the function signature (export
async function closeClient(client: McpClient | null, throwOnError = false):
Promise<void>) and modify the catch block in closeClient to either throw the
error or keep logging based on throwOnError, and add a short comment above the
function describing the default swallow-and-log behavior and the optional
rethrow behavior for callers that need to know about close failures.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/map/map-query-handler.tsxlib/agents/researcher.tsxlib/agents/tools/drawing.tsxlib/schema/drawing.tsxlib/utils/index.tslib/utils/mcp.ts
🧰 Additional context used
🧬 Code graph analysis (4)
lib/utils/index.ts (3)
lib/agents/tools/geospatial.tsx (3)
getGoogleStaticMapUrl(158-162)_(147-147)t(385-385)components/map/google-map.tsx (2)
apiKey(20-29)GoogleMapComponent(12-86)components/map/mapbox-map.tsx (1)
latitude(205-237)
lib/agents/researcher.tsx (3)
lib/agents/resolution-search.tsx (1)
resolutionSearch(33-79)lib/agents/tools/geospatial.tsx (1)
uiStream(167-426)app/actions.tsx (1)
processResolutionSearch(90-187)
lib/agents/tools/drawing.tsx (4)
lib/schema/drawing.tsx (1)
drawingToolSchema(8-39)components/message.tsx (1)
BotMessage(11-31)lib/utils/mcp.ts (2)
getConnectedMcpClient(10-79)closeClient(84-95)lib/agents/tools/index.tsx (2)
ToolProps(15-39)ToolProps(9-13)
lib/utils/mcp.ts (1)
lib/agents/tools/geospatial.tsx (2)
getConnectedMcpClient(39-137)closeClient(142-153)
🔇 Additional comments (5)
lib/agents/researcher.tsx (2)
50-61: Well-structured tool documentation with clear priority handling.The drawingQueryTool documentation clearly establishes:
- When to use the tool (drawing shapes)
- Priority over geospatialQueryTool for combined queries
- Internal geocoding behavior
- Response guidelines
This addresses the previous concern about ambiguous decision flow for mixed queries like "Draw a circle around Eiffel Tower".
134-134: Verify intent ofuiStream.update(null).Passing
nulltouiStream.update()clears any previous UI content. Confirm this is intentional to reset the stream before processing tool results, rather than an accidental removal of previous content.components/map/map-query-handler.tsx (1)
41-47: LGTM - DRAWING_TRIGGER handling is well-implemented.The handler correctly:
- Checks for the DRAWING_TRIGGER type and features existence
- Switches to DrawingMode via setMapType
- Updates map data with pendingFeatures
lib/utils/mcp.ts (1)
10-79: The code consolidation has already been completed—geospatial.tsx is already importing and usinggetConnectedMcpClientandcloseClientfrom the sharedlib/utils/mcputility. There is no code duplication to address.Likely an incorrect or invalid review comment.
lib/utils/index.ts (1)
121-138: No action needed — implementation is correct.The
getGoogleStaticMapUrlfunction is properly centralized inlib/utils/index.tsand correctly imported bylib/agents/tools/geospatial.tsx(line 12). There is no duplicate function. The implementation correctly validates the API key and throws an error if missing, usesURLSearchParamsfor proper parameter encoding, and is reused across the codebase as intended.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
components/map/map-query-handler.tsx
Outdated
| interface ToolOutput { | ||
| type: string; | ||
| originalUserInput?: string; | ||
| timestamp: string; | ||
| mcp_response: McpResponseData | null; | ||
| mcp_response?: McpResponseData | null; | ||
| features?: any[]; | ||
| error?: string | null; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting ToolOutput type to a shared location.
The ToolOutput interface is defined locally but represents the contract between the drawing tool (in lib/agents/tools/drawing.tsx) and this handler. Consider moving it to a shared types file (e.g., lib/types/tools.ts) to ensure both producer and consumer stay synchronized.
Additionally, features?: any[] loses type safety. Consider typing it as GeoJSON Feature array:
import type { Feature } from 'geojson';
// ...
features?: Feature[];🤖 Prompt for AI Agents
In `@components/map/map-query-handler.tsx` around lines 18 - 25, Move the
ToolOutput interface out of components/map/map-query-handler.tsx into a shared
types file (e.g., create lib/types/tools.ts) so both producer and consumer
reference the same contract; update both components/map/map-query-handler.tsx
and lib/agents/tools/drawing.tsx to import ToolOutput from that shared module,
and tighten the features field from any[] to a GeoJSON Feature[] (importing
Feature from 'geojson') so features is strongly typed.
…tion - Created shared ToolOutput type in lib/types/tools.ts with GeoJSON Feature typing. - Refined drawingToolSchema to require coordinates for lines and enforce lat/lng bounds. - Refactored drawingTool in lib/agents/tools/drawing.tsx for type-safe geocoding and robust response validation. - Enhanced getGoogleStaticMapUrl with API key validation and URLSearchParams for safe encoding. - Updated MapQueryHandler with guarded debug logging and shared types. - Enhanced closeClient in lib/utils/mcp.ts with throwOnError parameter and improved documentation. - Updated researcher agent system prompt for better drawing intent prioritization. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/agents/tools/drawing.tsx`:
- Around line 19-22: The console.log in the execute implementation of the
drawing tool currently prints the entire params object (from drawingToolSchema)
which may contain PII; update the execute function to stop logging raw params
and instead log only the non-sensitive shape type (the destructured type
variable) or wrap any debug logging behind a runtime debug/dev flag (e.g.,
process.env.DEBUG or a logger.isDebugEnabled check) so production logs never
contain user-provided location data.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/map/map-query-handler.tsxlib/agents/tools/drawing.tsxlib/schema/drawing.tsxlib/types/tools.tslib/utils/mcp.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/types/tools.ts (2)
lib/agents/tools/geospatial.tsx (2)
McpResponse(25-28)Location(18-23)mapbox_mcp/hooks.ts (1)
PlaceResult(31-39)
🔇 Additional comments (3)
lib/types/tools.ts (1)
1-19: Solid shared ToolOutput contract.
This centralizes tool outputs and geojson feature typing cleanly.components/map/map-query-handler.tsx (1)
16-52: LGTM for ToolOutput handling and drawing integration.
The guarded dev‑only warning and DRAWING_TRIGGER flow look solid.lib/utils/mcp.ts (1)
65-76: Add defensive cleanup on connection failure ingetConnectedMcpClient().While the MCP SDK documentation indicates
client.connect()handles cleanup internally on handshake failures, when usingPromise.race()to impose a 15-second timeout, the pendingclient.connect()promise is abandoned if the timeout fires. Addingawait closeClient(client);before returningnullprovides explicit, best-effort cleanup to ensure sockets and transports don't linger unexpectedly, especially in timeout scenarios.Note: Both consuming tools (
drawing.tsxandgeospatial.tsx) already properly clean up viafinallyblocks, but this defensive cleanup at the source—where the client is created—improves robustness.🔧 Suggested fix
try { console.log('[MCP Utility] Attempting to connect to MCP server...'); await Promise.race([ client.connect(transport), new Promise((_, reject) => setTimeout(() => reject(new Error('Connection timeout after 15 seconds')), 15000)), ]); console.log('[MCP Utility] Successfully connected to MCP server'); } catch (connectError: any) { console.error('[MCP Utility] MCP connection failed:', connectError.message); + await closeClient(client); return null; }
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Fixed HeaderSearchButton disabling logic to correctly support Google Maps and Mapbox. - Restored missing mobile-header-search-portal target in MobileIconsBar. - Implemented dynamic portal detection in HeaderSearchButton using MutationObserver to handle layout transitions. - Migrated HeaderSearchButton to use sonner for notifications. - Updated ResolutionImage to use theme-aware glassmorphism utilities (Tailwind) instead of a global CSS class. - Added infinite loop prevention to Chat router refresh logic using lastRefreshedMessageIdRef. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/chat.tsx`:
- Around line 20-21: The effect watching mapData.cameraState should debounce
calls to the server action updateDrawingContext and handle errors: replace the
direct call inside the effect that depends on mapData.cameraState (from
useMapData / MapDataProvider) with a debounced invocation (use a ref timer or a
debounce utility) so rapid cameraState changes only trigger updateDrawingContext
infrequently, and make the call async with proper error handling (await or
promise.then/.catch) and log or handle failures instead of letting them be
swallowed.
In `@components/header-search-button.tsx`:
- Around line 61-64: The guard currently only checks mapData.cameraState exists;
update the validation to ensure mapData.cameraState contains the needed
properties (either cameraState.range or cameraState.zoom) before proceeding
(when mapProvider === 'google'), and if neither is present show the same
toast.error and return; then, when deriving zoom from cameraState.range/zoom
(the variables used in the Google static map calculation), coerce/compute a
numeric zoom and clamp it to Google Maps allowed range (e.g., 0–21) before using
it in the static map URL to avoid invalid zoom values.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/chat.tsxcomponents/header-search-button.tsxcomponents/mobile-icons-bar.tsxcomponents/resolution-image.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/header-search-button.tsx (4)
components/map/google-map.tsx (2)
GoogleMapComponent(12-86)apiKey(20-29)components/map/map-query-handler.tsx (1)
toolOutput(33-73)app/actions.tsx (2)
processResolutionSearch(90-187)aiState(644-824)components/ui/toaster.tsx (1)
Toaster(6-26)
🔇 Additional comments (8)
components/resolution-image.tsx (2)
30-33: Styling update looks good.The new border/background/blur utilities are consistent and keep the trigger container visually clean.
45-55: DialogContent styling is consistent.The updated background + blur utilities align with the trigger styling and keep the modal readable.
components/mobile-icons-bar.tsx (1)
48-51: Portal host placement is OK.Provides a stable target for the portaled mobile search button.
components/header-search-button.tsx (3)
12-12: No comment.
28-54: Portal discovery via MutationObserver looks good.Keeps the button mount points in sync with DOM changes.
127-147: Availability gating aligns desktop and mobile buttons.The shared
isMapAvailablekeeps the enabled/disabled state consistent across layouts.components/chat.tsx (2)
44-83: Prevents repeated refresh loops.
The last‑refreshed ID guard makes the refresh conditional on a new response message, which should stop the loop.
123-164: Remove the redundantMapDataProviderwrapping;Chatis already wrapped by parent pages.The
useMapData()hook at line 85 receives context from the parentMapDataProviderinapp/page.tsxandapp/search/[id]/page.tsx, so the providers at lines 123 and 168 are unnecessary nesting. Remove the internal wrapping to simplify the component structure.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { MapDataProvider, useMapData } from './map/map-data-context'; | ||
| import { updateDrawingContext } from '@/lib/actions/chat'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -name "chat.tsx" -type fRepository: QueueLab/QCX
Length of output: 286
🏁 Script executed:
fd chat.tsx -type fRepository: QueueLab/QCX
Length of output: 226
🏁 Script executed:
fd chat.tsxRepository: QueueLab/QCX
Length of output: 75
🏁 Script executed:
cat -n components/chat.tsx | head -110Repository: QueueLab/QCX
Length of output: 4747
🏁 Script executed:
fd map-data-context -type fRepository: QueueLab/QCX
Length of output: 226
🏁 Script executed:
fd map-data-context -t fRepository: QueueLab/QCX
Length of output: 91
🏁 Script executed:
wc -l components/map/map-data-context.tsxRepository: QueueLab/QCX
Length of output: 94
🏁 Script executed:
cat -n components/map/map-data-context.tsxRepository: QueueLab/QCX
Length of output: 2276
🏁 Script executed:
rg "setMapData.*cameraState|cameraState.*setMapData" -A 3 -B 3Repository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
rg "setMapData" -B 2 -A 5Repository: QueueLab/QCX
Length of output: 15328
🏁 Script executed:
rg "updateDrawingContext" -B 3 -A 10Repository: QueueLab/QCX
Length of output: 4061
🏁 Script executed:
fd mapbox-map.tsx -t fRepository: QueueLab/QCX
Length of output: 85
🏁 Script executed:
wc -l components/map/mapbox-map.tsxRepository: QueueLab/QCX
Length of output: 89
🏁 Script executed:
rg "cameraState|'move'|'zoom'|'pitch'|'bearing'|'dragend'|'dragstart'" components/map/mapbox-map.tsx -B 2 -A 5Repository: QueueLab/QCX
Length of output: 879
🏁 Script executed:
rg "handleUserInteraction|on\('move'|on\('drag'|on\('zoom'" components/map/mapbox-map.tsx -B 5 -A 10Repository: QueueLab/QCX
Length of output: 1773
🏁 Script executed:
rg "captureMapCenter\s*=" components/map/mapbox-map.tsx -A 15Repository: QueueLab/QCX
Length of output: 634
Handle async errors and debounce cameraState writes to avoid rapid server action calls.
The effect calls updateDrawingContext with mapData.cameraState as a dependency, which updates on map interactions (pan, zoom, drag, wheel events via moveend). While moveend prevents firing during each frame, it still triggers on each interaction, and the server action is called without error handling. Missing .catch() silently swallows errors.
⏱️ Example debounce to reduce write frequency and handle errors
useEffect(() => {
- if (id && mapData.drawnFeatures && mapData.cameraState) {
- updateDrawingContext(id, {
- drawnFeatures: mapData.drawnFeatures,
- cameraState: mapData.cameraState,
- });
- }
+ if (!id || !mapData.drawnFeatures || !mapData.cameraState) return;
+ const handle = setTimeout(() => {
+ void updateDrawingContext(id, {
+ drawnFeatures: mapData.drawnFeatures,
+ cameraState: mapData.cameraState,
+ }).catch(console.error);
+ }, 300);
+ return () => clearTimeout(handle);
}, [id, mapData.drawnFeatures, mapData.cameraState]);🤖 Prompt for AI Agents
In `@components/chat.tsx` around lines 20 - 21, The effect watching
mapData.cameraState should debounce calls to the server action
updateDrawingContext and handle errors: replace the direct call inside the
effect that depends on mapData.cameraState (from useMapData / MapDataProvider)
with a debounced invocation (use a ref timer or a debounce utility) so rapid
cameraState changes only trigger updateDrawingContext infrequently, and make the
call async with proper error handling (await or promise.then/.catch) and log or
handle failures instead of letting them be swallowed.
| if (mapProvider === 'google' && !mapData.cameraState) { | ||
| toast.error('Google Maps state is not available. Try moving the map first.') | ||
| return | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and examine the context around lines 61-64
find . -name "header-search-button.tsx" -o -name "*.tsx" | head -20Repository: QueueLab/QCX
Length of output: 626
🏁 Script executed:
# Search for cameraState usage and assignments
rg -n "cameraState" --type tsx --type ts -C 3Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Look for where mapData is populated with cameraState
rg -n "setMapData" --type tsx --type ts -C 3 | head -100Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Search for Google map component and zoom calculation logic
rg -n "range.*zoom\|zoom.*range" --type tsx --type ts -C 2Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Read the header-search-button.tsx file
cat -n ./components/header-search-button.tsxRepository: QueueLab/QCX
Length of output: 6339
🏁 Script executed:
# Search for cameraState and zoom without type restrictions
rg "cameraState" -C 3Repository: QueueLab/QCX
Length of output: 6674
🏁 Script executed:
# Search for range and zoom calculations
rg "Math.log2.*range\|range.*40000000" -C 3Repository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
# Find where mapData is set
rg "setMapData" -C 3Repository: QueueLab/QCX
Length of output: 13539
🏁 Script executed:
# Find CameraState interface definition
rg "interface CameraState|type CameraState" -C 5Repository: QueueLab/QCX
Length of output: 707
🏁 Script executed:
# Check map-data-context.tsx for full type definitions
cat -n components/map/map-data-context.tsxRepository: QueueLab/QCX
Length of output: 2276
Validate cameraState has zoom or range before deriving Google static map zoom.
The guard at lines 61-64 only checks if cameraState exists, not whether it contains the required properties. When Mapbox populates cameraState with zoom but without range, the code at line 96 destructures range as undefined, causing Math.log2(40000000 / 1) to calculate an invalid zoom of ~25 (Google Maps max is 21). This causes the static map request to fail.
🔧 Proposed fix
- const { center, range } = mapData.cameraState
- const zoom = Math.round(Math.log2(40000000 / (range || 1)));
+ const { center, range, zoom: cameraZoom } = mapData.cameraState
+ const zoom =
+ typeof cameraZoom === 'number'
+ ? Math.round(cameraZoom)
+ : typeof range === 'number'
+ ? Math.round(Math.log2(40000000 / Math.max(range, 1)))
+ : null
+ if (zoom === null) {
+ toast.error('Google Maps zoom is not available.')
+ setIsAnalyzing(false)
+ return
+ }🤖 Prompt for AI Agents
In `@components/header-search-button.tsx` around lines 61 - 64, The guard
currently only checks mapData.cameraState exists; update the validation to
ensure mapData.cameraState contains the needed properties (either
cameraState.range or cameraState.zoom) before proceeding (when mapProvider ===
'google'), and if neither is present show the same toast.error and return; then,
when deriving zoom from cameraState.range/zoom (the variables used in the Google
static map calculation), coerce/compute a numeric zoom and clamp it to Google
Maps allowed range (e.g., 0–21) before using it in the static map URL to avoid
invalid zoom values.
|
@coderabbitai explain how the AI controlled drawings work |
|
Tip For best results, initiate chat on the files or code changes.
End-to-End Flow1. AI Decision MakingWhen a user asks to draw something (e.g., "draw a circle around downtown Seattle" or "highlight the route from A to B"), the AI researcher's system prompt has been updated with a new policy section that mandates the use of the 2. Drawing Tool ExecutionThe Circle: {
type: 'circle',
location?: string, // e.g., "Seattle"
center?: {lat, lng},
radius: number,
units: 'meters' | 'kilometers' | 'miles' | 'feet',
label?: string,
color?: string
}Polygon: {
type: 'polygon',
location?: string,
coordinates?: [{lat, lng}, ...],
label?: string,
color?: string
}Line: {
type: 'line',
location?: string,
coordinates?: [{lat, lng}, ...],
label?: string,
color?: string
}3. Geocoding & Feature CreationThe tool execution process:
4. Map IntegrationThe features flow to the map through:
5. User FeedbackThroughout execution, the tool uses
Key Features
The system is designed so users never see implementation details—they just ask to draw something, and the AI handles geocoding, geometry creation, and map rendering automatically. |
I've implemented the AI-controlled drawing feature using a coordinated flow between the AI agent and the Mapbox components:
This integration allows for seamless transitions between natural language drawing commands and manual fine-tuning. |
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat.tsx (1)
121-163:⚠️ Potential issue | 🔴 CriticalCritical: Nested
MapDataProvidershadows the outer context, breaking map↔chat data flow.
ChatcallsuseMapData()at line 85, which resolves to the outerMapDataProvider(fromapp/page.tsx). However, the newly added inner<MapDataProvider>at lines 123 and 168 creates a separate context scope. All map components (MapProvider→mapbox-map.tsx) inside this inner provider will read/write to the innermapDatastate.Consequence:
mapData.drawnFeaturesandmapData.cameraStatethatChatreads at line 95 will always be the initial empty state, because the map writes to the inner provider's state. TheupdateDrawingContextcall (line 96) will never see actual drawn features.Remove the inner
<MapDataProvider>wrappers — the outer provider fromapp/page.tsxalready provides the context that bothChatand the map components need.Proposed fix
if (isMobile) { return ( - <MapDataProvider> + <> <HeaderSearchButton /> ... - </MapDataProvider> + </> ); } return ( - <MapDataProvider> + <> <HeaderSearchButton /> ... - </MapDataProvider> + </> );Also applies to: 167-208
🤖 Fix all issues with AI agents
In `@components/chat.tsx`:
- Around line 74-83: The use of Array.prototype.findLast on aiState.messages is
incompatible with the project's ES2018 runtime target; replace the findLast call
in the useEffect with an ES2018-safe approach (e.g., iterate from the end or
create a reversed copy then use find) so responseMessage is resolved without
relying on ES2023 features, keeping the rest of the logic that checks
lastRefreshedMessageIdRef.current and calls router.refresh() unchanged (update
the code around aiState.messages.findLast, lastRefreshedMessageIdRef, and
router.refresh accordingly).
In `@components/header-search-button.tsx`:
- Line 25: The double-cast "as unknown as HeaderActions" bypasses TypeScript
checks; replace it by giving useActions the correct generic or by narrowing its
return with a type guard. Specifically, update the call to useActions<typeof
AI>() so it returns a properly typed actions object (e.g.,
useActions<HeaderActions>() or change the AI type parameter to match
HeaderActions) or add a runtime/type-guard function that validates the shape
before assigning to const actions; reference the useActions<typeof AI>() call
and the HeaderActions type in header-search-button.tsx to locate and correct the
typing.
- Around line 44-53: The MutationObserver created in this file (observer) is
watching document.body with subtree:true which is too broad; change findPortals
mounting logic to stop observing global DOM mutations: either narrow the
observer to the specific container element where the portal divs are expected
(attach observer to that parent instead of document.body) or replace the
observer with a short polling loop using requestAnimationFrame or setInterval
that calls findPortals until the two portals are found, then clears the
raf/interval; ensure the cleanup currently returning observer.disconnect() is
updated to cancel the interval/animation frame or disconnect the targeted
observer and keep references to the observer/handle so it is stopped as soon as
findPortals succeeds.
In `@components/map/map-query-handler.tsx`:
- Around line 24-26: Remove the unconditional console.log of drawing features
and geocoded place names in the MapQueryHandler branch that handles
toolOutput.type === 'DRAWING_TRIGGER' (and the other logging at line ~36) and
either delete the logs or wrap them with an environment/dev-only guard (e.g.,
check process.env.NODE_ENV !== 'production' or a isDev flag) so sensitive
location data is not emitted in production; update the statements near the
setMapType(MapToggleEnum.DrawingMode) and the place-name logging to use the
guard or be removed entirely.
- Around line 35-49: The warning logs in MapQueryHandler currently serialize the
entire toolOutput (and toolOutput.mcp_response) which may contain sensitive
user-provided locations; update the console.warn calls inside the
MAP_QUERY_TRIGGER handling to avoid logging full objects and instead log only
safe fields (e.g., toolOutput.type and a short summary string or boolean flags
indicating presence of mcp_response and mapUrl) and include the
place_name/coordinates only when they are validated; locate the block that calls
setMapData and the two console.warn invocations in map-query-handler.tsx and
replace those warns with minimal structured messages mentioning the symbol/tool
type and summary rather than the full toolOutput or mcp_response.
In `@lib/types/tools.ts`:
- Around line 13-20: Change ToolOutput.type from a free string to a
string-literal union so consumers can do exhaustiveness checks: update the
ToolOutput interface (symbol: ToolOutput) to use type: 'MAP_QUERY_TRIGGER' |
'DRAWING_TRIGGER' and, if needed, split into discriminated variants (e.g.,
MapQueryToolOutput and DrawingToolOutput) that extend a shared base to carry
type-specific fields (symbols: mcp_response, features, originalUserInput). Then
update callsites (e.g., map-query-handler.tsx) to use the discriminant for
switch/case logic and fix any type errors by narrowing on ToolOutput.type.
- Around line 3-11: Remove the duplicated local interfaces Location and
McpResponse in geospatial.tsx and import the shared McpResponseData type from
lib/types/tools.ts instead; update any references that used Location or
McpResponse (including the ToolOutput type) to use McpResponseData so the
component uses the centralized type and eliminate the local definitions.
In `@lib/utils/index.ts`:
- Around line 121-138: Update getGoogleStaticMapUrl to restore the original
behavior: read the API key using the fallback NEXT_PUBLIC_GOOGLE_MAPS_API_KEY ||
GOOGLE_MAPS_API_KEY (so client builds work), add the scale=2 parameter to the
URL params for HiDPI rendering, change the missing-key behavior to return an
empty string instead of throwing, and use the original default zoom and size
values (zoom 15 and size 640x480) while keeping markers and other params intact;
locate these changes in the getGoogleStaticMapUrl function in
lib/utils/index.ts.
In `@lib/utils/mcp.ts`:
- Around line 21-24: Simplify the redundant truthy + trim checks by replacing
the long compound condition with trimmed-falsy checks using optional chaining
for the three variables: composioApiKey, mapboxAccessToken, and composioUserId;
e.g., change the if condition to test !composioApiKey?.trim() ||
!mapboxAccessToken?.trim() || !composioUserId?.trim() so whitespace-only strings
are caught and you avoid duplicate checks while keeping the same error path in
the MCP utility that currently logs the console error and returns null.
- Around line 66-76: The Promise.race pattern with setTimeout in the
client.connect(transport) block (and similarly in closeClient) leaves a dangling
rejection when the connect/close promise resolves first; modify the logic to
create a cancellable timeout (store the timeout id) and clearTimeout when
client.connect or closeClient completes successfully, or implement an
AbortController-style token so the timeout's reject is not called after success;
update the connect code around client.connect(transport) and the closeClient
routine to cancel/clear the timer on success and only reject when the timer
actually elapses to avoid unhandled rejections.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
components/chat.tsxcomponents/header-search-button.tsxcomponents/map/map-query-handler.tsxcomponents/mobile-icons-bar.tsxcomponents/resolution-image.tsxlib/agents/researcher.tsxlib/agents/tools/drawing.tsxlib/schema/drawing.tsxlib/types/tools.tslib/utils/index.tslib/utils/mcp.ts
🧰 Additional context used
🧬 Code graph analysis (8)
components/mobile-icons-bar.tsx (1)
components/search-skeleton.tsx (1)
div(6-30)
lib/utils/index.ts (3)
lib/agents/tools/geospatial.tsx (3)
getGoogleStaticMapUrl(158-162)_(147-147)t(385-385)components/map/google-map.tsx (2)
apiKey(20-29)GoogleMapComponent(12-86)components/map/map-provider.tsx (1)
MapProvider(16-28)
lib/types/tools.ts (2)
lib/agents/tools/geospatial.tsx (3)
McpResponse(25-28)Location(18-23)uiStream(167-426)lib/agents/tools/index.tsx (1)
ToolProps(9-13)
components/chat.tsx (5)
components/map/map-data-context.tsx (1)
MapDataProvider(42-50)app/actions.tsx (1)
aiState(644-824)components/map/mapbox-map.tsx (3)
lastInteractionRef(194-203)map(67-168)prevData(167-167)components/map/map-3d.tsx (1)
prevData(35-45)app/page.tsx (1)
Page(9-18)
lib/utils/mcp.ts (2)
mapbox_mcp/hooks.ts (2)
composioClientRef(106-121)composioClientRef(62-70)mapbox_mcp/composio-mapbox.ts (3)
initializeComposioMapbox(96-115)getComposioClient(121-132)authenticateToolkit(59-89)
components/resolution-image.tsx (2)
components/ui/dialog.tsx (6)
props(20-29)props(102-108)props(35-53)props(70-81)props(56-67)props(87-96)components/ui/alert-dialog.tsx (4)
props(33-45)props(79-85)props(18-27)props(91-97)
lib/agents/tools/drawing.tsx (3)
lib/schema/drawing.tsx (1)
drawingToolSchema(8-38)lib/types/tools.ts (1)
ToolOutput(13-20)lib/utils/mcp.ts (2)
getConnectedMcpClient(10-79)closeClient(90-104)
lib/agents/researcher.tsx (3)
lib/agents/tools/geospatial.tsx (2)
uiStream(167-426)queryType(347-356)app/actions.tsx (1)
processResolutionSearch(90-187)lib/agents/resolution-search.tsx (1)
resolutionSearch(33-79)
🔇 Additional comments (8)
components/resolution-image.tsx (1)
30-30: Styling updates look good.The switch to explicit
border-border/40,bg-background/30, andbg-background/80withbackdrop-blur-mdis a clean approach, giving consistent translucency without relying on a sharedglassmorphicutility class. No functional concerns.Also applies to: 45-45
lib/agents/researcher.tsx (2)
50-87: Well-structured drawing/geospatial tool prioritization.The decision flow correctly places
drawingQueryToolbeforegeospatialQueryTool, and the priority rule at line 55 clearly resolves ambiguity for mixed queries. The behavioral guidelines (lines 57-61) appropriately instruct the model to avoid verbalizing drawing actions.
134-134: PassingnulltouiStream.update.
uiStream.update(null)clears the stream content. This is existing behavior (a comment was removed). Verify this is intentional and doesn't cause a React rendering issue —createStreamableUIfromai/rscshould acceptnull, but the type signature may expectReactNode.components/mobile-icons-bar.tsx (1)
48-51: Clean portal target placement.The portal target
divreplaces the inline search button, allowingHeaderSearchButtonto centrally manage both desktop and mobile search UIs. The emptydivwon't affect flex layout since it has no intrinsic size.lib/utils/mcp.ts (1)
10-78: Delegation to shared MCP utility confirmed.
geospatial.tsx(line 11) importsgetConnectedMcpClient,closeClient, andMcpClientfrom@/lib/utils/mcp. The MCP connection and instantiation logic (MCPClientClassandclient.connect()) exists only in the centralized utility; no duplication was found ingeospatial.tsx.lib/agents/tools/drawing.tsx (2)
99-123: Polygon branch: no validation that polygon has at least one positioning input.If a polygon with no
coordinatesand nolocationmakes it past schema validation (which currently allows it — see schema review), this code reaches line 112 and throws a generic error. The schema should catch this, but as a defense-in-depth measure the error message is appropriate. No action needed here if the schema is fixed.
37-152: Overall structure and error handling look solid.The try/finally ensures MCP cleanup and stream completion. Geocoding response parsing is properly guarded. The type-safe location access via narrowing on
params.typeis well done. Turf.js usage with properUnitstyping is correct.components/map/map-query-handler.tsx (1)
12-54: Component logic is well-structured overall.The type-based dispatch (
DRAWING_TRIGGER/MAP_QUERY_TRIGGER), validation guards on latitude/longitude types, callback-stylesetMapData, and proper dependency array are all good patterns.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| useEffect(() => { | ||
| if (aiState.messages[aiState.messages.length - 1]?.type === 'response') { | ||
| // Refresh the page to chat history updates | ||
| router.refresh() | ||
| // Check if there is a 'response' message in the history | ||
| const responseMessage = aiState.messages.findLast(m => m.type === 'response'); | ||
|
|
||
| if (responseMessage && responseMessage.id !== lastRefreshedMessageIdRef.current) { | ||
| console.log('Chat.tsx: refreshing router for message:', responseMessage.id); | ||
| lastRefreshedMessageIdRef.current = responseMessage.id; | ||
| router.refresh(); | ||
| } | ||
| }, [aiState, router]) | ||
| }, [aiState.messages, router]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check tsconfig.json for target and lib settings
find . -name "tsconfig.json" -type f | head -5Repository: QueueLab/QCX
Length of output: 373
🏁 Script executed:
# Look for tsconfig.json in common locations and check target/lib
if [ -f tsconfig.json ]; then
echo "=== tsconfig.json content ==="
cat tsconfig.json
fiRepository: QueueLab/QCX
Length of output: 751
🏁 Script executed:
# Search for findLast usage in the codebase
rg "findLast" -t ts -t tsx -C 2Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Check for polyfills or compatibility layers
rg "polyfill|shim|findLast" --type ts --type tsx --type json | head -30Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Search for findLast in TypeScript/React files
rg "findLast" --type tsRepository: QueueLab/QCX
Length of output: 158
🏁 Script executed:
# Check the specific file and lines mentioned in the review
if [ -f components/chat.tsx ]; then
echo "=== components/chat.tsx lines 70-85 ==="
sed -n '70,85p' components/chat.tsx
fiRepository: QueueLab/QCX
Length of output: 685
🏁 Script executed:
# Check for any polyfills or compatibility layers in the project
rg "Array.prototype" --type ts | head -10Repository: QueueLab/QCX
Length of output: 38
findLast is ES2023 — runtime compatibility issue with target ES2018.
The code at line 76 uses Array.prototype.findLast(), which is an ES2023 feature. The project's tsconfig.json sets target: "ES2018" while using lib: ["dom", "dom.iterable", "esnext"], allowing TypeScript to compile the code without errors. However, this will fail at runtime in environments without ES2023 support (older browsers, Node.js < 18.x).
Replace with an alternative that works in ES2018:
const responseMessage = [...aiState.messages].reverse().find(m => m.type === 'response');Or upgrade tsconfig.json to "target": "ES2023" if project requirements allow.
🤖 Prompt for AI Agents
In `@components/chat.tsx` around lines 74 - 83, The use of
Array.prototype.findLast on aiState.messages is incompatible with the project's
ES2018 runtime target; replace the findLast call in the useEffect with an
ES2018-safe approach (e.g., iterate from the end or create a reversed copy then
use find) so responseMessage is resolved without relying on ES2023 features,
keeping the rest of the logic that checks lastRefreshedMessageIdRef.current and
calls router.refresh() unchanged (update the code around
aiState.messages.findLast, lastRefreshedMessageIdRef, and router.refresh
accordingly).
| const { mapProvider } = useSettingsStore() | ||
| const { mapData } = useMapData() | ||
| // Cast the actions to our defined interface to avoid build errors | ||
| const actions = useActions<typeof AI>() as unknown as HeaderActions |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Double cast as unknown as HeaderActions bypasses type safety.
This silences any type mismatch between the actual actions returned by useActions<typeof AI>() and HeaderActions. If the shape changes, TypeScript won't catch it.
Consider properly typing the AI actions or using a type guard instead.
🤖 Prompt for AI Agents
In `@components/header-search-button.tsx` at line 25, The double-cast "as unknown
as HeaderActions" bypasses TypeScript checks; replace it by giving useActions
the correct generic or by narrowing its return with a type guard. Specifically,
update the call to useActions<typeof AI>() so it returns a properly typed
actions object (e.g., useActions<HeaderActions>() or change the AI type
parameter to match HeaderActions) or add a runtime/type-guard function that
validates the shape before assigning to const actions; reference the
useActions<typeof AI>() call and the HeaderActions type in
header-search-button.tsx to locate and correct the typing.
| const observer = new MutationObserver(() => { | ||
| findPortals() | ||
| }) | ||
|
|
||
| observer.observe(document.body, { | ||
| childList: true, | ||
| subtree: true | ||
| }) | ||
|
|
||
| return () => observer.disconnect() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
MutationObserver on document.body with subtree: true is overly broad.
This observer fires on every single DOM mutation (any element added/removed anywhere in the page), just to locate two portal divs that are rendered once. This can cause excessive re-renders and getElementById lookups on busy pages.
Consider a more targeted approach: observe only the direct parent where portals are expected, or simply retry with requestAnimationFrame / a short interval until found, then stop.
Example: polling with cleanup
useEffect(() => {
- const findPortals = () => {
- setDesktopPortal(document.getElementById('header-search-portal'))
- setMobilePortal(document.getElementById('mobile-header-search-portal'))
- }
-
- findPortals()
-
- const observer = new MutationObserver(() => {
- findPortals()
- })
-
- observer.observe(document.body, {
- childList: true,
- subtree: true
- })
-
- return () => observer.disconnect()
+ let rafId: number;
+ const findPortals = () => {
+ const desktop = document.getElementById('header-search-portal');
+ const mobile = document.getElementById('mobile-header-search-portal');
+ setDesktopPortal(desktop);
+ setMobilePortal(mobile);
+ if (!desktop || !mobile) {
+ rafId = requestAnimationFrame(findPortals);
+ }
+ };
+ findPortals();
+ return () => cancelAnimationFrame(rafId);
}, [])🤖 Prompt for AI Agents
In `@components/header-search-button.tsx` around lines 44 - 53, The
MutationObserver created in this file (observer) is watching document.body with
subtree:true which is too broad; change findPortals mounting logic to stop
observing global DOM mutations: either narrow the observer to the specific
container element where the portal divs are expected (attach observer to that
parent instead of document.body) or replace the observer with a short polling
loop using requestAnimationFrame or setInterval that calls findPortals until the
two portals are found, then clears the raf/interval; ensure the cleanup
currently returning observer.disconnect() is updated to cancel the
interval/animation frame or disconnect the targeted observer and keep references
to the observer/handle so it is stopped as soon as findPortals succeeds.
| if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) { | ||
| console.log('MapQueryHandler: Received drawing data.', toolOutput.features); | ||
| setMapType(MapToggleEnum.DrawingMode); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Production console.log statements leak feature data and place names.
Lines 25 and 36 log drawing features and geocoded place names unconditionally. While less sensitive than raw user input, this adds noise in production and could expose location data. Guard behind a development check or remove.
Proposed fix
if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) {
- console.log('MapQueryHandler: Received drawing data.', toolOutput.features);
+ if (process.env.NODE_ENV === 'development') {
+ console.log('MapQueryHandler: Received drawing data.', toolOutput.features);
+ }
setMapType(MapToggleEnum.DrawingMode);📝 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.
| if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) { | |
| console.log('MapQueryHandler: Received drawing data.', toolOutput.features); | |
| setMapType(MapToggleEnum.DrawingMode); | |
| if (toolOutput.type === 'DRAWING_TRIGGER' && toolOutput.features) { | |
| if (process.env.NODE_ENV === 'development') { | |
| console.log('MapQueryHandler: Received drawing data.', toolOutput.features); | |
| } | |
| setMapType(MapToggleEnum.DrawingMode); |
🤖 Prompt for AI Agents
In `@components/map/map-query-handler.tsx` around lines 24 - 26, Remove the
unconditional console.log of drawing features and geocoded place names in the
MapQueryHandler branch that handles toolOutput.type === 'DRAWING_TRIGGER' (and
the other logging at line ~36) and either delete the logs or wrap them with an
environment/dev-only guard (e.g., check process.env.NODE_ENV !== 'production' or
a isDev flag) so sensitive location data is not emitted in production; update
the statements near the setMapType(MapToggleEnum.DrawingMode) and the place-name
logging to use the guard or be removed entirely.
| if (typeof latitude === 'number' && typeof longitude === 'number') { | ||
| console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`); | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| targetPosition: { lat: latitude, lng: longitude }, | ||
| mapFeature: { | ||
| place_name, | ||
| mapUrl: toolOutput.mcp_response?.mapUrl | ||
| } | ||
| })); | ||
| } else { | ||
| console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload', { toolOutput, mcp_response: toolOutput.mcp_response }); | ||
| } | ||
| } else { | ||
| console.warn("MapQueryHandler: Invalid latitude/longitude in toolOutput.mcp_response:", toolOutput.mcp_response.location); | ||
| // Clear target position if data is invalid | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| targetPosition: null, | ||
| mapFeature: null | ||
| })); | ||
| } | ||
| } else { | ||
| // This case handles when toolOutput or its critical parts are missing. | ||
| // Depending on requirements, could fall back to originalUserInput and useMCPMapClient, | ||
| // or simply log that no valid data was provided from the tool. | ||
| // For this subtask, we primarily focus on using the new toolOutput. | ||
| if (toolOutput) { // It exists, but data is not as expected | ||
| console.warn("MapQueryHandler: toolOutput provided, but mcp_response or location data is missing.", toolOutput); | ||
| console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload', { toolOutput, mcp_response: toolOutput?.mcp_response }); |
There was a problem hiding this comment.
Warn logs include full toolOutput object — may contain user-provided locations.
Lines 46 and 49 log the entire toolOutput (and mcp_response) in warning messages. In production, this can serialize user-provided locations and other sensitive fields. Log only the type and a summary:
Proposed fix
- console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`);
+ if (process.env.NODE_ENV === 'development') {
+ console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`);
+ }
setMapData(prevData => ({
...prevData,
targetPosition: { lat: latitude, lng: longitude },
@@ ...
} else {
- console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload', { toolOutput, mcp_response: toolOutput.mcp_response });
+ console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload (missing valid coordinates)');
}
} else {
- console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload', { toolOutput, mcp_response: toolOutput?.mcp_response });
+ console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload (missing mcp_response)');
}📝 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.
| if (typeof latitude === 'number' && typeof longitude === 'number') { | |
| console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`); | |
| setMapData(prevData => ({ | |
| ...prevData, | |
| targetPosition: { lat: latitude, lng: longitude }, | |
| mapFeature: { | |
| place_name, | |
| mapUrl: toolOutput.mcp_response?.mapUrl | |
| } | |
| })); | |
| } else { | |
| console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload', { toolOutput, mcp_response: toolOutput.mcp_response }); | |
| } | |
| } else { | |
| console.warn("MapQueryHandler: Invalid latitude/longitude in toolOutput.mcp_response:", toolOutput.mcp_response.location); | |
| // Clear target position if data is invalid | |
| setMapData(prevData => ({ | |
| ...prevData, | |
| targetPosition: null, | |
| mapFeature: null | |
| })); | |
| } | |
| } else { | |
| // This case handles when toolOutput or its critical parts are missing. | |
| // Depending on requirements, could fall back to originalUserInput and useMCPMapClient, | |
| // or simply log that no valid data was provided from the tool. | |
| // For this subtask, we primarily focus on using the new toolOutput. | |
| if (toolOutput) { // It exists, but data is not as expected | |
| console.warn("MapQueryHandler: toolOutput provided, but mcp_response or location data is missing.", toolOutput); | |
| console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload', { toolOutput, mcp_response: toolOutput?.mcp_response }); | |
| if (typeof latitude === 'number' && typeof longitude === 'number') { | |
| if (process.env.NODE_ENV === 'development') { | |
| console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`); | |
| } | |
| setMapData(prevData => ({ | |
| ...prevData, | |
| targetPosition: { lat: latitude, lng: longitude }, | |
| mapFeature: { | |
| place_name, | |
| mapUrl: toolOutput.mcp_response?.mapUrl | |
| } | |
| })); | |
| } else { | |
| console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload (missing valid coordinates)'); | |
| } | |
| } else { | |
| console.warn('MapQueryHandler: invalid MAP_QUERY_TRIGGER payload (missing mcp_response)'); |
🤖 Prompt for AI Agents
In `@components/map/map-query-handler.tsx` around lines 35 - 49, The warning logs
in MapQueryHandler currently serialize the entire toolOutput (and
toolOutput.mcp_response) which may contain sensitive user-provided locations;
update the console.warn calls inside the MAP_QUERY_TRIGGER handling to avoid
logging full objects and instead log only safe fields (e.g., toolOutput.type and
a short summary string or boolean flags indicating presence of mcp_response and
mapUrl) and include the place_name/coordinates only when they are validated;
locate the block that calls setMapData and the two console.warn invocations in
map-query-handler.tsx and replace those warns with minimal structured messages
mentioning the symbol/tool type and summary rather than the full toolOutput or
mcp_response.
| export interface McpResponseData { | ||
| location: { | ||
| latitude?: number; | ||
| longitude?: number; | ||
| place_name?: string; | ||
| address?: string; | ||
| }; | ||
| mapUrl?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for McpResponse and Location interface definitions in the codebase
rg -n "interface McpResponse|interface Location" --type ts -B 1 -A 5Repository: QueueLab/QCX
Length of output: 1395
🏁 Script executed:
#!/bin/bash
# Check imports in geospatial.tsx and usage of McpResponse/Location
head -20 lib/agents/tools/geospatial.tsx
echo "---"
# Search for usages of McpResponse and Location in the file
rg -n "McpResponse|Location" lib/agents/tools/geospatial.tsx --type tsRepository: QueueLab/QCX
Length of output: 933
🏁 Script executed:
#!/bin/bash
# Get the full context of McpResponse usage in geospatial.tsx
rg -n "McpResponse" lib/agents/tools/geospatial.tsx --type ts -B 2 -A 2
echo "---"
# Check if McpResponseData is imported anywhere in geospatial.tsx
rg -n "McpResponseData\|from.*types" lib/agents/tools/geospatial.tsx --type ts
echo "---"
# Check if any other files import from lib/types/tools.ts
rg -n "from.*lib/types/tools|from.*tools" --type ts | head -20Repository: QueueLab/QCX
Length of output: 964
🏁 Script executed:
#!/bin/bash
# Compare the structures more carefully
echo "=== McpResponseData structure ==="
sed -n '3,11p' lib/types/tools.ts
echo ""
echo "=== Local McpResponse/Location in geospatial.tsx ==="
sed -n '15,25p' lib/agents/tools/geospatial.tsx
echo ""
echo "=== Check ToolOutput definition in types/tools.ts ==="
rg -n "interface ToolOutput|type ToolOutput" lib/types/tools.ts -A 8Repository: QueueLab/QCX
Length of output: 757
Replace local Location and McpResponse interfaces in geospatial.tsx with import of McpResponseData from lib/types/tools.ts.
The geospatial.tsx file defines its own Location and McpResponse interfaces (lines 15-25) that are structurally identical to McpResponseData in the shared types file. Since ToolOutput already references McpResponseData, consolidate by importing the shared type instead of maintaining duplicate definitions.
🤖 Prompt for AI Agents
In `@lib/types/tools.ts` around lines 3 - 11, Remove the duplicated local
interfaces Location and McpResponse in geospatial.tsx and import the shared
McpResponseData type from lib/types/tools.ts instead; update any references that
used Location or McpResponse (including the ToolOutput type) to use
McpResponseData so the component uses the centralized type and eliminate the
local definitions.
| export interface ToolOutput { | ||
| type: string; | ||
| originalUserInput?: string; | ||
| timestamp: string; | ||
| mcp_response?: McpResponseData | null; | ||
| features?: Feature[]; | ||
| error?: string | null; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider narrowing ToolOutput.type to a string literal union.
type: string loses type safety. The known trigger types are 'MAP_QUERY_TRIGGER' and 'DRAWING_TRIGGER'. A discriminated union enables exhaustive switch/case checks in consumers like map-query-handler.tsx.
Suggested change
export interface ToolOutput {
- type: string;
+ type: 'MAP_QUERY_TRIGGER' | 'DRAWING_TRIGGER';
originalUserInput?: string;
timestamp: string;
mcp_response?: McpResponseData | null;
features?: Feature[];
error?: string | 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.
| export interface ToolOutput { | |
| type: string; | |
| originalUserInput?: string; | |
| timestamp: string; | |
| mcp_response?: McpResponseData | null; | |
| features?: Feature[]; | |
| error?: string | null; | |
| } | |
| export interface ToolOutput { | |
| type: 'MAP_QUERY_TRIGGER' | 'DRAWING_TRIGGER'; | |
| originalUserInput?: string; | |
| timestamp: string; | |
| mcp_response?: McpResponseData | null; | |
| features?: Feature[]; | |
| error?: string | null; | |
| } |
🤖 Prompt for AI Agents
In `@lib/types/tools.ts` around lines 13 - 20, Change ToolOutput.type from a free
string to a string-literal union so consumers can do exhaustiveness checks:
update the ToolOutput interface (symbol: ToolOutput) to use type:
'MAP_QUERY_TRIGGER' | 'DRAWING_TRIGGER' and, if needed, split into discriminated
variants (e.g., MapQueryToolOutput and DrawingToolOutput) that extend a shared
base to carry type-specific fields (symbols: mcp_response, features,
originalUserInput). Then update callsites (e.g., map-query-handler.tsx) to use
the discriminant for switch/case logic and fix any type errors by narrowing on
ToolOutput.type.
| export function getGoogleStaticMapUrl(latitude: number, longitude: number): string { | ||
| const apiKey = process.env.GOOGLE_MAPS_API_KEY; | ||
| if (!apiKey) { | ||
| throw new Error('GOOGLE_MAPS_API_KEY is missing'); | ||
| } | ||
|
|
||
| const url = new URL('https://maps.googleapis.com/maps/api/staticmap'); | ||
| const params = new URLSearchParams({ | ||
| center: `${latitude},${longitude}`, | ||
| zoom: '14', | ||
| size: '600x300', | ||
| maptype: 'roadmap', | ||
| markers: `color:red|${latitude},${longitude}`, | ||
| key: apiKey | ||
| }); | ||
| url.search = params.toString(); | ||
| return url.toString(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all definitions and usages of getGoogleStaticMapUrl
rg -n "getGoogleStaticMapUrl" --type ts --type tsx -C 3Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Check the exact content of the function in lib/agents/tools/geospatial.tsx around lines 157-161
cat -n lib/agents/tools/geospatial.tsx | sed -n '150,170p'Repository: QueueLab/QCX
Length of output: 1366
🏁 Script executed:
#!/bin/bash
# Check the exact content of the function in lib/utils/index.ts around lines 121-138
cat -n lib/utils/index.ts | sed -n '115,145p'Repository: QueueLab/QCX
Length of output: 876
🏁 Script executed:
#!/bin/bash
# Search for getGoogleStaticMapUrl in all files
rg -n "getGoogleStaticMapUrl" -C 3Repository: QueueLab/QCX
Length of output: 3529
🏁 Script executed:
#!/bin/bash
# Check the entire lib/agents/tools/geospatial.tsx file for getGoogleStaticMapUrl
grep -n "getGoogleStaticMapUrl" lib/agents/tools/geospatial.tsxRepository: QueueLab/QCX
Length of output: 297
🏁 Script executed:
#!/bin/bash
# List all files that might contain this function
fd -e ts -e tsx -type f | xargs grep -l "getGoogleStaticMapUrl"Repository: QueueLab/QCX
Length of output: 228
Consolidation lost important features from the original implementation.
The refactoring moved getGoogleStaticMapUrl from a local function in geospatial.tsx to a shared utility in lib/utils/index.ts. However, the new utility is missing critical functionality:
- Environment variable fallback: The original checked
NEXT_PUBLIC_GOOGLE_MAPS_API_KEY || GOOGLE_MAPS_API_KEY; the new version only usesGOOGLE_MAPS_API_KEY. This breaks client-side usage. - Scale parameter: The original included
scale=2for sharper HiDPI display rendering. The new version omits this. - Error handling: The original gracefully returned an empty string on missing API key; the new version throws. This may break the geospatial tool's error flow.
- Default parameters: Zoom and size differ (14→15, 600×300→640×480).
Restore the original environment variable logic and scale parameter, or document why these changes were intentional.
🤖 Prompt for AI Agents
In `@lib/utils/index.ts` around lines 121 - 138, Update getGoogleStaticMapUrl to
restore the original behavior: read the API key using the fallback
NEXT_PUBLIC_GOOGLE_MAPS_API_KEY || GOOGLE_MAPS_API_KEY (so client builds work),
add the scale=2 parameter to the URL params for HiDPI rendering, change the
missing-key behavior to return an empty string instead of throwing, and use the
original default zoom and size values (zoom 15 and size 640x480) while keeping
markers and other params intact; locate these changes in the
getGoogleStaticMapUrl function in lib/utils/index.ts.
| if (!composioApiKey || !mapboxAccessToken || !composioUserId || !composioApiKey.trim() || !mapboxAccessToken.trim() || !composioUserId.trim()) { | ||
| console.error('[MCP Utility] Missing or empty required environment variables'); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant truthy + trim checks.
If composioApiKey is "", the !composioApiKey check already short-circuits. The .trim() guards only add value for whitespace-only strings. This works correctly, but can be simplified.
Optional simplification
- if (!composioApiKey || !mapboxAccessToken || !composioUserId || !composioApiKey.trim() || !mapboxAccessToken.trim() || !composioUserId.trim()) {
+ if (!composioApiKey?.trim() || !mapboxAccessToken?.trim() || !composioUserId?.trim()) {📝 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.
| if (!composioApiKey || !mapboxAccessToken || !composioUserId || !composioApiKey.trim() || !mapboxAccessToken.trim() || !composioUserId.trim()) { | |
| console.error('[MCP Utility] Missing or empty required environment variables'); | |
| return null; | |
| } | |
| if (!composioApiKey?.trim() || !mapboxAccessToken?.trim() || !composioUserId?.trim()) { | |
| console.error('[MCP Utility] Missing or empty required environment variables'); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In `@lib/utils/mcp.ts` around lines 21 - 24, Simplify the redundant truthy + trim
checks by replacing the long compound condition with trimmed-falsy checks using
optional chaining for the three variables: composioApiKey, mapboxAccessToken,
and composioUserId; e.g., change the if condition to test
!composioApiKey?.trim() || !mapboxAccessToken?.trim() || !composioUserId?.trim()
so whitespace-only strings are caught and you avoid duplicate checks while
keeping the same error path in the MCP utility that currently logs the console
error and returns null.
| try { | ||
| console.log('[MCP Utility] Attempting to connect to MCP server...'); | ||
| await Promise.race([ | ||
| client.connect(transport), | ||
| new Promise((_, reject) => setTimeout(() => reject(new Error('Connection timeout after 15 seconds')), 15000)), | ||
| ]); | ||
| console.log('[MCP Utility] Successfully connected to MCP server'); | ||
| } catch (connectError: any) { | ||
| console.error('[MCP Utility] MCP connection failed:', connectError.message); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Promise.race timeout leaves a dangling rejection when the main promise wins.
When client.connect(transport) resolves before the 15 s timer, the setTimeout callback still fires and calls reject(), producing an unhandled promise rejection. The same pattern exists in closeClient (line 93-96).
Fix using an AbortController-style cleanup
// Connect to server
try {
console.log('[MCP Utility] Attempting to connect to MCP server...');
- await Promise.race([
- client.connect(transport),
- new Promise((_, reject) => setTimeout(() => reject(new Error('Connection timeout after 15 seconds')), 15000)),
- ]);
+ await new Promise<void>((resolve, reject) => {
+ const timer = setTimeout(() => reject(new Error('Connection timeout after 15 seconds')), 15000);
+ client.connect(transport).then(
+ () => { clearTimeout(timer); resolve(); },
+ (err) => { clearTimeout(timer); reject(err); },
+ );
+ });
console.log('[MCP Utility] Successfully connected to MCP server');Apply the same pattern to closeClient:
- await Promise.race([
- client.close(),
- new Promise((_, reject) => setTimeout(() => reject(new Error('Close timeout after 5 seconds')), 5000)),
- ]);
+ await new Promise<void>((resolve, reject) => {
+ const timer = setTimeout(() => reject(new Error('Close timeout after 5 seconds')), 5000);
+ client.close().then(
+ () => { clearTimeout(timer); resolve(); },
+ (err) => { clearTimeout(timer); reject(err); },
+ );
+ });🤖 Prompt for AI Agents
In `@lib/utils/mcp.ts` around lines 66 - 76, The Promise.race pattern with
setTimeout in the client.connect(transport) block (and similarly in closeClient)
leaves a dangling rejection when the connect/close promise resolves first;
modify the logic to create a cancellable timeout (store the timeout id) and
clearTimeout when client.connect or closeClient completes successfully, or
implement an AbortController-style token so the timeout's reject is not called
after success; update the connect code around client.connect(transport) and the
closeClient routine to cancel/clear the timer on success and only reject when
the timer actually elapses to avoid unhandled rejections.
|
@coderabbitai 16:39:44.499 Running build in Portland, USA (West) – pdx1 |
I've fixed the build errors by providing explicit types for the |
- Fixed type error in `components/chat.tsx` by providing explicit types for `aiState` and `messages`. - Suppressed ESLint `exhaustive-deps` warning in `components/map/mapbox-map.tsx` for map initialization to prevent redundant re-renders. - Verified fix with `bun run build`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/map/mapbox-map.tsx (1)
450-475: 🧹 Nitpick | 🔵 TrivialDraw-control teardown logic is duplicated between
setupDrawingToolsand this effect.Lines 450–475 repeat almost the same cleanup sequence as lines 250–273 inside
setupDrawingTools(unsubscribe draw events →removeControl→ null refs → remove markers). Extract a sharedcleanupDrawingToolshelper to keep a single source of truth and reduce the risk of the two copies drifting apart.♻️ Sketch of extracted helper
+const cleanupDrawingTools = useCallback(() => { + if (!map.current) return; + if (drawRef.current) { + try { + map.current.off('draw.create', updateMeasurementLabels); + map.current.off('draw.delete', updateMeasurementLabels); + map.current.off('draw.update', updateMeasurementLabels); + map.current.removeControl(drawRef.current); + drawRef.current = null; + } catch (e) { + console.log('Error removing draw control:', e); + } + } + Object.values(polygonLabelsRef.current).forEach(m => m.remove()); + Object.values(lineLabelsRef.current).forEach(m => m.remove()); + polygonLabelsRef.current = {}; + lineLabelsRef.current = {}; + if (navControlRef.current) { + try { + map.current.removeControl(navControlRef.current); + navControlRef.current = null; + } catch (e) { + console.log('Error removing navigation control:', e); + } + } +}, [updateMeasurementLabels]);Then call
cleanupDrawingTools()in bothsetupDrawingToolsand the mode-switch effect.
🤖 Fix all issues with AI agents
In `@components/chat.tsx`:
- Line 81: Remove the noisy debug console.log in Chat.tsx that logs "Chat.tsx:
refreshing router for message:" with responseMessage.id; either delete that
console.log or wrap it behind a development-only guard (process.env.NODE_ENV ===
'development') so it does not run in production. Locate the statement in the
function handling AI responses (the code referencing responseMessage.id) and
update it accordingly to silence production logs.
In `@components/map/mapbox-map.tsx`:
- Around line 44-58: The measurement label DOM elements in the MapboxMap
component are being styled inline repeatedly (same background, padding,
borderRadius, fontSize, fontWeight, color, boxShadow, pointerEvents) — extract
those repeated style properties into a reusable constant (e.g., labelStyle) or,
preferably, a CSS class and apply it to every place you create label elements
instead of duplicating inline styles; update the code that creates measurement
labels (the DOM creation logic near formatMeasurement and the other label
creation sites) to reference the shared labelStyle or className so future style
changes are centralized.
- Around line 486-493: The effect watching mapData.targetPosition currently
includes mapData.mapFeature in its dependency array causing redundant re-runs
and extra flyTo calls; remove mapData.mapFeature from the dependency array so
the effect only reacts to mapData.targetPosition and updateMapPosition, or if
you actually want to re-fly on feature change, add a guard inside the effect
that compares the current targetPosition coords to the previous coords and only
calls updateMapPosition(lat, lng) when they differ; locate the useEffect that
references mapData.targetPosition/mapData.mapFeature and updateMapPosition and
either drop mapData.mapFeature from the deps or add the coordinate-equality
check before calling updateMapPosition.
- Around line 184-197: The current updateMapPosition sets isUpdatingPositionRef
via a fixed setTimeout after calling map.current.flyTo, which can misalign with
actual animation completion; instead, attach a one-time "moveend" (or
"idle"/"move" depending on your mapbox version) listener to map.current right
after calling map.current.flyTo and clear isUpdatingPositionRef.current inside
that event handler, and ensure you remove the listener on cleanup or if
updateMapPosition exits early; update the updateMapPosition implementation and
any related cleanup logic so the lock is released when the map emits the
completion event rather than after a hardcoded 2000ms.
- Line 232: The call to tzlookup(center.lat, center.lng) inside captureMapCenter
can throw for coordinates outside coverage (e.g., over oceans); wrap that call
in a try/catch inside the captureMapCenter function so an exception won't bubble
and crash the component — on error set timezone to null/undefined or leave the
previous value and optionally log the error via the component logger or
console.warn with context (include center lat/lng) to aid debugging; ensure
downstream code that consumes the timezone handles the null/undefined case
gracefully.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/chat.tsxcomponents/map/mapbox-map.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
components/chat.tsx (3)
app/actions.tsx (1)
AI(563-642)lib/types/index.ts (1)
AIMessage(59-77)components/map/map-data-context.tsx (1)
MapDataProvider(42-50)
components/map/mapbox-map.tsx (3)
components/map-toggle-context.tsx (1)
useMapToggle(36-42)components/map/map-data-context.tsx (1)
useMapData(52-58)components/map-loading-context.tsx (1)
useMapLoading(20-26)
🔇 Additional comments (2)
components/chat.tsx (2)
34-35: Good use of typed hooks and ref-based dedup guard.Typing
useUIState<typeof AI>()anduseAIState<typeof AI>()properly constrains the state shapes. ThelastRefreshedMessageIdRefis a clean approach to prevent repeatedrouter.refresh()calls for the same message.Also applies to: 46-48
87-103: Remove this review comment — the context issue is not as stated.
useMapData()at Line 87 does have access to aMapDataContextprovider:Chatis wrapped in a parentMapDataProviderat the page level (seeapp/page.tsxandapp/search/[id]/page.tsx). The hook will not fail or return undefined.However, there is a real issue with context shadowing:
- The
useMapData()call at Line 87 reads from the parentMapDataProvider(page level)- The
<MapDataProvider>rendered at Lines 125 and 170 create a separate context instance that shadows the parent for their descendantsMapProvider(a child of the inner<MapDataProvider>) uses its own isolatedmapDatastate- When
MapProviderupdates drawn features, themapDatain the effect (Lines 96–103) — which comes from the parent context — never sees those updates- Result:
updateDrawingContextreceives stale dataThe fix is to remove the inner
<MapDataProvider>wrappers and rely solely on the parent provider, or restructure so the effect reads from the correct context instance that the map components modify.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const responseMessage = aiState.messages.findLast((m: AIMessage) => m.type === 'response'); | ||
|
|
||
| if (responseMessage && responseMessage.id !== lastRefreshedMessageIdRef.current) { | ||
| console.log('Chat.tsx: refreshing router for message:', responseMessage.id); |
There was a problem hiding this comment.
Remove debug console.log before merging.
This log fires on every new AI response message and will be noisy in production. Consider removing it or gating it behind a NODE_ENV === 'development' check.
Proposed fix
if (responseMessage && responseMessage.id !== lastRefreshedMessageIdRef.current) {
- console.log('Chat.tsx: refreshing router for message:', responseMessage.id);
lastRefreshedMessageIdRef.current = responseMessage.id;
router.refresh();
}📝 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.
| console.log('Chat.tsx: refreshing router for message:', responseMessage.id); | |
| if (responseMessage && responseMessage.id !== lastRefreshedMessageIdRef.current) { | |
| lastRefreshedMessageIdRef.current = responseMessage.id; | |
| router.refresh(); | |
| } |
🤖 Prompt for AI Agents
In `@components/chat.tsx` at line 81, Remove the noisy debug console.log in
Chat.tsx that logs "Chat.tsx: refreshing router for message:" with
responseMessage.id; either delete that console.log or wrap it behind a
development-only guard (process.env.NODE_ENV === 'development') so it does not
run in production. Locate the statement in the function handling AI responses
(the code referencing responseMessage.id) and update it accordingly to silence
production logs.
| const formatMeasurement = useCallback((value: number, isArea = true) => { | ||
| if (isArea) { | ||
| // Area formatting | ||
| if (value >= 1000000) { | ||
| return `${(value / 1000000).toFixed(2)} km²` | ||
| return `${(value / 1000000).toFixed(2)} km²` | ||
| } else { | ||
| return `${value.toFixed(2)} m²` | ||
| return `${value.toFixed(2)} m²` | ||
| } | ||
| } else { | ||
| // Distance formatting | ||
| if (value >= 1000) { | ||
| return `${(value / 1000).toFixed(2)} km` | ||
| return `${(value / 1000).toFixed(2)} km` | ||
| } else { | ||
| return `${value.toFixed(0)} m` | ||
| return `${value.toFixed(0)} m` | ||
| } | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Measurement label DOM elements are created imperatively — consider extracting styles.
All label elements share the same inline styles (background, padding, border-radius, font-size, font-weight, color, box-shadow, pointer-events). Extracting a small utility or CSS class would reduce duplication and make future style changes easier.
Also applies to: 60-154
🤖 Prompt for AI Agents
In `@components/map/mapbox-map.tsx` around lines 44 - 58, The measurement label
DOM elements in the MapboxMap component are being styled inline repeatedly (same
background, padding, borderRadius, fontSize, fontWeight, color, boxShadow,
pointerEvents) — extract those repeated style properties into a reusable
constant (e.g., labelStyle) or, preferably, a CSS class and apply it to every
place you create label elements instead of duplicating inline styles; update the
code that creates measurement labels (the DOM creation logic near
formatMeasurement and the other label creation sites) to reference the shared
labelStyle or className so future style changes are centralized.
| const updateMapPosition = useCallback(async (lat: number, lng: number) => { | ||
| if (map.current && !isUpdatingPositionRef.current) { | ||
| isUpdatingPositionRef.current = true | ||
| stopRotation() | ||
|
|
||
| try { | ||
| // Update our current map center ref | ||
| currentMapCenterRef.current.center = [longitude, latitude] | ||
|
|
||
| await new Promise<void>((resolve) => { | ||
| map.current?.flyTo({ | ||
| center: [longitude, latitude], | ||
| zoom: 12, | ||
| essential: true, | ||
| speed: 0.5, | ||
| curve: 1, | ||
| }) | ||
| map.current?.once('moveend', () => { | ||
| resolve() | ||
| }) | ||
| }) | ||
| setTimeout(() => { | ||
| if (mapType === MapToggleEnum.RealTimeMode) { | ||
| startRotation() | ||
| } | ||
| isUpdatingPositionRef.current = false | ||
| }, 500) | ||
| } catch (error) { | ||
| console.error('Error updating map position:', error) | ||
| map.current.flyTo({ | ||
| center: [lng, lat], | ||
| zoom: 15, | ||
| pitch: 45, | ||
| essential: true | ||
| }) | ||
| setTimeout(() => { | ||
| isUpdatingPositionRef.current = false | ||
| } | ||
| }, 2000) | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
flyTo completion is approximated with a fixed 2-second timeout.
The isUpdatingPositionRef guard is released after a hardcoded setTimeout(…, 2000), regardless of actual flight duration. For long-distance flights the lock releases too early (allowing overlapping animations); for short ones it holds too long (blocking quick successive updates, e.g., fast geolocation updates in RealTimeMode). Consider using the map's moveend event to release the lock accurately:
♻️ Suggested approach
const updateMapPosition = useCallback(async (lat: number, lng: number) => {
if (map.current && !isUpdatingPositionRef.current) {
isUpdatingPositionRef.current = true
map.current.flyTo({
center: [lng, lat],
zoom: 15,
pitch: 45,
essential: true
})
- setTimeout(() => {
- isUpdatingPositionRef.current = false
- }, 2000)
+ map.current.once('moveend', () => {
+ isUpdatingPositionRef.current = false
+ })
}
}, [])📝 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 updateMapPosition = useCallback(async (lat: number, lng: number) => { | |
| if (map.current && !isUpdatingPositionRef.current) { | |
| isUpdatingPositionRef.current = true | |
| stopRotation() | |
| try { | |
| // Update our current map center ref | |
| currentMapCenterRef.current.center = [longitude, latitude] | |
| await new Promise<void>((resolve) => { | |
| map.current?.flyTo({ | |
| center: [longitude, latitude], | |
| zoom: 12, | |
| essential: true, | |
| speed: 0.5, | |
| curve: 1, | |
| }) | |
| map.current?.once('moveend', () => { | |
| resolve() | |
| }) | |
| }) | |
| setTimeout(() => { | |
| if (mapType === MapToggleEnum.RealTimeMode) { | |
| startRotation() | |
| } | |
| isUpdatingPositionRef.current = false | |
| }, 500) | |
| } catch (error) { | |
| console.error('Error updating map position:', error) | |
| map.current.flyTo({ | |
| center: [lng, lat], | |
| zoom: 15, | |
| pitch: 45, | |
| essential: true | |
| }) | |
| setTimeout(() => { | |
| isUpdatingPositionRef.current = false | |
| } | |
| }, 2000) | |
| } | |
| }, []) | |
| const updateMapPosition = useCallback(async (lat: number, lng: number) => { | |
| if (map.current && !isUpdatingPositionRef.current) { | |
| isUpdatingPositionRef.current = true | |
| map.current.flyTo({ | |
| center: [lng, lat], | |
| zoom: 15, | |
| pitch: 45, | |
| essential: true | |
| }) | |
| map.current.once('moveend', () => { | |
| isUpdatingPositionRef.current = false | |
| }) | |
| } | |
| }, []) |
🤖 Prompt for AI Agents
In `@components/map/mapbox-map.tsx` around lines 184 - 197, The current
updateMapPosition sets isUpdatingPositionRef via a fixed setTimeout after
calling map.current.flyTo, which can misalign with actual animation completion;
instead, attach a one-time "moveend" (or "idle"/"move" depending on your mapbox
version) listener to map.current right after calling map.current.flyTo and clear
isUpdatingPositionRef.current inside that event handler, and ensure you remove
the listener on cleanup or if updateMapPosition exits early; update the
updateMapPosition implementation and any related cleanup logic so the lock is
released when the map emits the completion event rather than after a hardcoded
2000ms.
| const bearing = map.current.getBearing(); | ||
| currentMapCenterRef.current = { center: [center.lng, center.lat], zoom, pitch }; | ||
|
|
||
| const timezone = tzlookup(center.lat, center.lng); |
There was a problem hiding this comment.
tzlookup can throw for coordinates over oceans or outside coverage, crashing the component.
tzlookup throws an error (e.g., "Invalid coordinates") for certain coordinate inputs, particularly over open ocean. Since captureMapCenter is bound to the moveend event, a user simply panning over an ocean will trigger an unhandled exception.
🛡️ Proposed fix — wrap in try/catch
- const timezone = tzlookup(center.lat, center.lng);
-
- setMapData(prevData => ({
- ...prevData,
- currentTimezone: timezone,
- cameraState: {
- center: { lat: center.lat, lng: center.lng },
- zoom,
- pitch,
- bearing
- }
- }));
+ let timezone: string | undefined;
+ try {
+ timezone = tzlookup(center.lat, center.lng);
+ } catch {
+ // Coordinates may be over ocean or outside tz-lookup coverage
+ }
+
+ setMapData(prevData => ({
+ ...prevData,
+ ...(timezone !== undefined && { currentTimezone: timezone }),
+ cameraState: {
+ center: { lat: center.lat, lng: center.lng },
+ zoom,
+ pitch,
+ bearing
+ }
+ }));🤖 Prompt for AI Agents
In `@components/map/mapbox-map.tsx` at line 232, The call to tzlookup(center.lat,
center.lng) inside captureMapCenter can throw for coordinates outside coverage
(e.g., over oceans); wrap that call in a try/catch inside the captureMapCenter
function so an exception won't bubble and crash the component — on error set
timezone to null/undefined or leave the previous value and optionally log the
error via the component logger or console.warn with context (include center
lat/lng) to aid debugging; ensure downstream code that consumes the timezone
handles the null/undefined case gracefully.
| useEffect(() => { | ||
| if (mapData.targetPosition && map.current) { | ||
| const { lat, lng } = mapData.targetPosition; | ||
| if (typeof lat === 'number' && typeof lng === '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 | ||
| // } | ||
| }, [mapData.targetPosition, mapData.mapFeature, updateMapPosition]); |
There was a problem hiding this comment.
Spurious dependency mapData.mapFeature may cause redundant flyTo calls.
mapData.mapFeature is listed in the dependency array but is never read in the effect body. When mapFeature changes (but targetPosition carries the same coordinates), this effect re-runs and triggers another flyTo to the same location, causing a jarring camera animation.
Remove it from the dependency array, or if the intent is to re-fly when the feature changes, guard against same-coordinates re-execution.
Proposed fix
useEffect(() => {
if (mapData.targetPosition && map.current) {
const { lat, lng } = mapData.targetPosition;
if (typeof lat === 'number' && typeof lng === 'number') {
updateMapPosition(lat, lng);
}
}
- }, [mapData.targetPosition, mapData.mapFeature, updateMapPosition]);
+ }, [mapData.targetPosition, updateMapPosition]);📝 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.
| useEffect(() => { | |
| if (mapData.targetPosition && map.current) { | |
| const { lat, lng } = mapData.targetPosition; | |
| if (typeof lat === 'number' && typeof lng === '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 | |
| // } | |
| }, [mapData.targetPosition, mapData.mapFeature, updateMapPosition]); | |
| useEffect(() => { | |
| if (mapData.targetPosition && map.current) { | |
| const { lat, lng } = mapData.targetPosition; | |
| if (typeof lat === 'number' && typeof lng === 'number') { | |
| updateMapPosition(lat, lng); | |
| } | |
| } | |
| }, [mapData.targetPosition, updateMapPosition]); |
🤖 Prompt for AI Agents
In `@components/map/mapbox-map.tsx` around lines 486 - 493, The effect watching
mapData.targetPosition currently includes mapData.mapFeature in its dependency
array causing redundant re-runs and extra flyTo calls; remove mapData.mapFeature
from the dependency array so the effect only reacts to mapData.targetPosition
and updateMapPosition, or if you actually want to re-fly on feature change, add
a guard inside the effect that compares the current targetPosition coords to the
previous coords and only calls updateMapPosition(lat, lng) when they differ;
locate the useEffect that references mapData.targetPosition/mapData.mapFeature
and updateMapPosition and either drop mapData.mapFeature from the deps or add
the coordinate-equality check before calling updateMapPosition.
ngoiyaeric
changed the title
2 participants
User description
This PR adds a radial drawing tool (Circle) to the Mapbox map and integrates drawing tools with the AI agent.
Key changes:
drawingQueryToolwith a Zod schema to allow the AI to draw polygons, lines, and circles.MapboxMapto handle programmatic drawing viaMapDataContext.pendingFeatures.lib/utils/mcp.ts.PR created automatically by Jules for task 15881425069546006738 started by @ngoiyaeric
PR Type
Enhancement, Refactoring
Description
Implemented custom
CircleModefor mapbox-gl-draw enabling radial drawing with center point and radius calculation using Turf.jsCreated new
drawingToolfor AI agent to draw shapes (circles, polygons, lines) on map with integrated geocoding via MCP clientRefactored shared MCP client logic into
lib/utils/mcp.tswith connection management and error handlingExtended
MapboxMapcomponent to handle programmatic drawing viaMapDataContext.pendingFeatureswith updated measurement labels for circlesUpdated
map-query-handler.tsxto support bothMAP_QUERY_TRIGGERandDRAWING_TRIGGERtool output typesAdded
drawingToolSchemaZod schema with discriminated union supporting polygon, line, and circle shape typesUpdated AI researcher system prompt with new "Map Drawing and Annotation" section documenting drawing tool usage
Added
getGoogleStaticMapUrl()utility function for generating Google Maps static image URLsUpdated Next.js dependency from 15.3.6 to 15.3.8
Diagram Walkthrough
File Walkthrough
2 files
mcp.ts
Refactored MCP client logic into shared utility modulelib/utils/mcp.ts
connection management
getConnectedMcpClient()function with environmentvalidation and error handling
closeClient()function for safe client cleanup with timeoutprotection
McpClienttype for use across the applicationgeospatial.tsx
Refactored to use shared MCP and utility moduleslib/agents/tools/geospatial.tsx
lib/utils/mcp.ts)getGoogleStaticMapUrl()function (moved tolib/utils/index.ts)MapboxConfiginterface11 files
circle-mode.ts
Added custom circle drawing mode for Mapboxcomponents/map/draw-modes/circle-mode.ts
CircleModefor mapbox-gl-draw supporting radialdrawing
Turf.js
user_isCircle,user_center, anduser_radiusInKmpropertiesradius, ESC to cancel)
index.ts
Added Google Maps static URL generation utilitylib/utils/index.ts
getGoogleStaticMapUrl()utility function for generating GoogleMaps static image URLs
placement
mapbox-map.tsx
Integrated circle drawing and AI-driven feature placementcomponents/map/mapbox-map.tsx
CircleModeinto drawing tools with custom circle button inUI
(e.g., "R: 5km, A: 78.5km²")
pendingFeatureseffect to handle programmatic drawing from AItool
updateMapPosition()to simplify animation logic and removedcomplex promise handling
react-toastifytosonnerlibrarydrawing.tsx
New AI drawing tool with geocoding and shape generationlib/agents/tools/drawing.tsx
drawingToolfor AI agent to draw shapes (circles,polygons, lines) on map
coordinates
styling and metadata
DRAWING_TRIGGERtype output with features array for mapintegration
operations
drawing.tsx
Added Zod schema for drawing tool parameterslib/schema/drawing.tsx
union pattern
polygon,line, andcirclewithtype-specific parameters
for vertices
for circles
map-query-handler.tsx
Extended map handler to support drawing triggerscomponents/map/map-query-handler.tsx
MAP_QUERY_TRIGGERandDRAWING_TRIGGERtool output types
DrawingModependingFeaturesin map context when drawing data is receivedtypes
map-data-context.tsx
Added pending features support to map data contextcomponents/map/map-data-context.tsx
pendingFeaturesoptional array field toMapDatainterface forprogrammatic drawing
pendingFeaturesas empty array in provider stateindex.tsx
Registered drawing tool in agent toolslib/agents/tools/index.tsx
drawingToolmoduledrawingQueryToolin tools object returned bygetTools()researcher.tsx
Integrate drawing tool into AI researcher system promptlib/agents/researcher.tsx
system prompt documenting the
drawingQueryToolfor drawing shapes onthe map
section, shifting location/geography queries to section 4
drawingQueryToolincluding geocoding andresponse guidelines
comment formatting
drawing.tsx
Define Zod schema for drawing tool parameterslib/schema/drawing.tsx
drawingToolSchemawith discriminated unionsupporting three shape types: polygon, line, and circle
label, and color properties
feet) parameters
DrawingToolParamstype derived from the schema for typesafety
index.tsx
Register drawing tool in agent tools configurationlib/agents/tools/index.tsx
drawingToolfrom './drawing' moduledrawingQueryToolin the tools object withuiStreamparameter
1 files
researcher.tsx
Updated AI researcher prompt with drawing tool guidancelib/agents/researcher.tsx
drawingQueryToolusagefor AI agent
geospatial queries
shape-related requests
automatically
1 files
bun.lock
Updated Next.js dependency to 15.3.8bun.lock
@next/envdependency to match Next.js version1 files
changes.diff
Complete changeset documentationchanges.diff
components
Summary by CodeRabbit
New Features
Style
Chores