Skip to content

Conversation

@ngoiyaeric
Copy link
Collaborator

@ngoiyaeric ngoiyaeric commented Aug 14, 2025

User description

This commit introduces a new drawingTool that allows me to draw GeoJSON features on the map.

The changes include:

  • A new Zod schema for validating GeoJSON data.
  • The drawingTool definition.
  • Integration of the new tool.
  • Updates to my actions to handle the tool's output.
  • Modifications to the Mapbox component to render the GeoJSON data I generate on the map.

PR Type

Enhancement


Description

  • Add drawing tool for GeoJSON features on map

  • Create Zod schema for GeoJSON validation

  • Integrate drawing tool with AI state management

  • Render GeoJSON features on Mapbox map


Diagram Walkthrough

flowchart LR
  A["Drawing Tool"] --> B["GeoJSON Schema"]
  B --> C["AI State"]
  C --> D["Mapbox Rendering"]
  A --> E["Tool Integration"]
  E --> C
Loading

File Walkthrough

Relevant files
Enhancement
drawing.ts
Add GeoJSON validation schemas                                                     

lib/schema/drawing.ts

  • Create comprehensive Zod schemas for GeoJSON geometries
  • Define Point, LineString, and Polygon geometry types
  • Export drawing schema for tool validation
+48/-0   
actions.tsx
Integrate drawing tool with actions                                           

app/actions.tsx

  • Handle DRAWING tool output in UI state
  • Add drawing tool case in tool result handling
  • Return null component for drawing tool UI
+12/-0   
mapbox-map.tsx
Render GeoJSON features on map                                                     

components/map/mapbox-map.tsx

  • Import AI state for accessing drawing messages
  • Add effect to render GeoJSON features from AI state
  • Create map layers for points, lines, and polygons
  • Handle different geometry types with appropriate styling
+71/-0   
drawing.tsx
Implement drawing tool definition                                               

lib/agents/tools/drawing.tsx

  • Create drawing tool with GeoJSON parameter schema
  • Return DRAWING type with geojson data
  • Add tool description for drawing features
+17/-0   
index.tsx
Register drawing tool in tools index                                         

lib/agents/tools/index.tsx

  • Import and register drawing tool
  • Add drawing tool to tools object
  • Export drawing tool for agent use
+4/-0     

Summary by CodeRabbit

  • New Features

    • Map now renders AI-generated drawings as overlays, showing points, lines and polygons with default styling.
    • Drawing results are recognized and applied to the map without opening a UI panel.
  • Chores

    • Added environment variable for database connection.
  • Chores

    • Chat persistence to backend has been disabled (chats are not saved).

This commit introduces a new `drawingTool` that allows me to draw GeoJSON features on the map.

The changes include:
- A new Zod schema for validating GeoJSON data.
- The `drawingTool` definition.
- Integration of the new tool.
- Updates to my actions to handle the tool's output.
- Modifications to the Mapbox component to render the GeoJSON data I generate on the map.
@vercel
Copy link

vercel bot commented Aug 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Project Deployment Preview Comments Updated (UTC)
qcx Ready Preview Comment Aug 14, 2025 6:46am

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 14, 2025

Walkthrough

Adds a drawing tool and GeoJSON validation schema, wires the tool into the tools registry, updates AI action handling to recognize DRAWING outputs (without persisting chat), and renders AI-provided GeoJSON as Mapbox sources/layers. Also adds a local DATABASE_URL entry and removes mcp from ToolProps/geospatialTool usage.

Changes

Cohort / File(s) Summary of changes
Environment config
/.env
Adds DATABASE_URL="postgresql://postgres:postgres@localhost:5432/postgres".
Tools registry & tool
/lib/agents/tools/index.tsx, /lib/agents/tools/drawing.tsx
Adds drawingTool and registers it in getTools; removes mcp from ToolProps and the geospatialTool call; drawingTool.execute returns { type: 'DRAWING', geojson }.
Drawing schema
/lib/schema/drawing.ts
Adds Zod-based drawingSchema validating GeoJSON FeatureCollection and exports Drawing type.
AI action handling
/app/actions.tsx
Comments out/preserves but disables saveChat(...); treats tool outputs with type === 'DRAWING' and produces UI entries with component: null (no UI rendered).
Map rendering (Mapbox)
/components/map/mapbox-map.tsx
Subscribes to AI state; on AI messages named drawing (role tool) parses message.content JSON, creates/updates per-message GeoJSON sources and three layers (points, lines, polygons) with styles; wraps in try/catch; no source/layer removal logic added.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Agent
  participant drawingTool
  participant AIState
  participant MapboxMap

  User->>Agent: Request a drawing
  Agent->>drawingTool: execute({ geojson })
  drawingTool-->>Agent: { type: "DRAWING", geojson }
  Agent-->>AIState: append tool message (name: "drawing", content: geojson)
  AIState-->>MapboxMap: state update / message visible
  MapboxMap->>MapboxMap: parse geojson, add/update sources and layers
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Possible security concern

Poem

I hop, I draw, I trace a line,
GeoJSON carrots, crisp and fine.
Layers rise in carrot-red,
A rabbit's map where I have tread.
Hop on—these drawings are mine. 🥕

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cbfe14d and 9eb8ec3.

📒 Files selected for processing (3)
  • app/actions.tsx (2 hunks)
  • components/map/mapbox-map.tsx (3 hunks)
  • lib/schema/drawing.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/actions.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
components/map/mapbox-map.tsx (4)
app/actions.tsx (1)
  • AIState (251-255)
components/chat.tsx (3)
  • Chat (20-123)
  • aiState (56-61)
  • id (67-72)
app/page.tsx (1)
  • Page (9-18)
lib/actions/chat.ts (1)
  • msg (119-127)
lib/schema/drawing.ts (2)
components/map/map-data-context.tsx (1)
  • MapData (7-17)
lib/actions/chat.ts (1)
  • updateDrawingContext (165-205)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/llm-drawing-tool

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@qodo-merge-pro
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

AI message parsing assumes JSON string in message.content and specific shape; if message.content is already an object or differs, this will throw and silently skip rendering. Consider robust parsing and validating with the Zod schema before adding sources/layers.

  if (map.current && aiState.messages) {
    aiState.messages.forEach(message => {
      if (message.name === 'drawing' && message.role === 'tool') {
        try {
          const { geojson } = JSON.parse(message.content);
          const sourceId = `geojson-source-${message.id}`;
          const layerId = `geojson-layer-${message.id}`;

          if (map.current?.getSource(sourceId)) {
            // Source already exists, no need to re-add
            return;
          }

          map.current?.addSource(sourceId, {
            type: 'geojson',
            data: geojson,
          });

          geojson.features.forEach((feature: any) => {
            const featureLayerId = `${layerId}-${feature.geometry.type}`;
            if (feature.geometry.type === 'Point' || feature.geometry.type === 'MultiPoint') {
              map.current?.addLayer({
                id: featureLayerId,
                type: 'circle',
                source: sourceId,
                paint: {
                  'circle-radius': 6,
                  'circle-color': '#B42222',
                },
                filter: ['==', '$type', 'Point'],
              });
            } else if (feature.geometry.type === 'LineString' || feature.geometry.type === 'MultiLineString') {
              map.current?.addLayer({
                id: featureLayerId,
                type: 'line',
                source: sourceId,
                layout: {
                  'line-join': 'round',
                  'line-cap': 'round',
                },
                paint: {
                  'line-color': '#B42222',
                  'line-width': 4,
                },
                filter: ['==', '$type', 'LineString'],
              });
            } else if (feature.geometry.type === 'Polygon' || feature.geometry.type === 'MultiPolygon') {
              map.current?.addLayer({
                id: featureLayerId,
                type: 'fill',
                source: sourceId,
                paint: {
                  'fill-color': '#B42222',
                  'fill-opacity': 0.5,
                },
                filter: ['==', '$type', 'Polygon'],
              });
            }
          });
        } catch (error) {
          console.error('Error parsing or adding GeoJSON data:', error);
        }
      }
    });
  }
}, [aiState.messages, map.current]);
Resource Leaks

Sources and layers are added per message without corresponding cleanup or id collision handling; on rerenders or AI state changes, layers may duplicate or persist, and Multi* geometry filters may not match. Add existence checks per layer id, remove on unmount/update, and use geometry-type-agnostic layers or correct filters.

map.current?.addSource(sourceId, {
  type: 'geojson',
  data: geojson,
});

geojson.features.forEach((feature: any) => {
  const featureLayerId = `${layerId}-${feature.geometry.type}`;
  if (feature.geometry.type === 'Point' || feature.geometry.type === 'MultiPoint') {
    map.current?.addLayer({
      id: featureLayerId,
      type: 'circle',
      source: sourceId,
      paint: {
        'circle-radius': 6,
        'circle-color': '#B42222',
      },
      filter: ['==', '$type', 'Point'],
    });
  } else if (feature.geometry.type === 'LineString' || feature.geometry.type === 'MultiLineString') {
    map.current?.addLayer({
      id: featureLayerId,
      type: 'line',
      source: sourceId,
      layout: {
        'line-join': 'round',
        'line-cap': 'round',
      },
      paint: {
        'line-color': '#B42222',
        'line-width': 4,
      },
      filter: ['==', '$type', 'LineString'],
    });
  } else if (feature.geometry.type === 'Polygon' || feature.geometry.type === 'MultiPolygon') {
    map.current?.addLayer({
      id: featureLayerId,
      type: 'fill',
      source: sourceId,
      paint: {
        'fill-color': '#B42222',
        'fill-opacity': 0.5,
      },
      filter: ['==', '$type', 'Polygon'],
    });
  }
});
Schema Gaps

GeoJSON schema lacks validation for MultiPoint, MultiLineString, MultiPolygon, bounding boxes, CRS, and ring closure/orientation for polygons. Map layer code handles Multi* types but schema does not; align schema to accept/validate these geometries.

const PointSchema = z.tuple([z.number(), z.number()]);
const LineStringSchema = z.array(PointSchema);
const PolygonSchema = z.array(LineStringSchema);

// GeoJSON Geometry Schemas
const PointGeometrySchema = z.object({
  type: z.literal('Point'),
  coordinates: PointSchema,
});

const LineStringGeometrySchema = z.object({
  type: z.literal('LineString'),
  coordinates: LineStringSchema,
});

const PolygonGeometrySchema = z.object({
  type: z.literal('Polygon'),
  coordinates: PolygonSchema,
});

const GeometrySchema = z.union([
  PointGeometrySchema,
  LineStringGeometrySchema,
  PolygonGeometrySchema,
]);

// GeoJSON Feature Schema
const FeatureSchema = z.object({
  type: z.literal('Feature'),
  geometry: GeometrySchema,
  properties: z.record(z.any()).optional(),
});

// GeoJSON FeatureCollection Schema
const FeatureCollectionSchema = z.object({
  type: z.literal('FeatureCollection'),
  features: z.array(FeatureSchema),
});

// The schema for the drawing tool, which will accept a FeatureCollection
export const drawingSchema = z.object({
  geojson: FeatureCollectionSchema.describe("A valid GeoJSON FeatureCollection object to be drawn on the map."),
});

@codiumai-pr-agent-free
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Memory Leak

The effect that renders GeoJSON features doesn't clean up map sources and layers when components unmount or when messages change. This could lead to duplicate layers and memory leaks.

useEffect(() => {
  if (map.current && aiState.messages) {
    aiState.messages.forEach(message => {
      if (message.name === 'drawing' && message.role === 'tool') {
        try {
          const { geojson } = JSON.parse(message.content);
          const sourceId = `geojson-source-${message.id}`;
          const layerId = `geojson-layer-${message.id}`;

          if (map.current?.getSource(sourceId)) {
            // Source already exists, no need to re-add
            return;
          }

          map.current?.addSource(sourceId, {
            type: 'geojson',
            data: geojson,
          });

          geojson.features.forEach((feature: any) => {
            const featureLayerId = `${layerId}-${feature.geometry.type}`;
            if (feature.geometry.type === 'Point' || feature.geometry.type === 'MultiPoint') {
              map.current?.addLayer({
                id: featureLayerId,
                type: 'circle',
                source: sourceId,
                paint: {
                  'circle-radius': 6,
                  'circle-color': '#B42222',
                },
                filter: ['==', '$type', 'Point'],
              });
            } else if (feature.geometry.type === 'LineString' || feature.geometry.type === 'MultiLineString') {
              map.current?.addLayer({
                id: featureLayerId,
                type: 'line',
                source: sourceId,
                layout: {
                  'line-join': 'round',
                  'line-cap': 'round',
                },
                paint: {
                  'line-color': '#B42222',
                  'line-width': 4,
                },
                filter: ['==', '$type', 'LineString'],
              });
            } else if (feature.geometry.type === 'Polygon' || feature.geometry.type === 'MultiPolygon') {
              map.current?.addLayer({
                id: featureLayerId,
                type: 'fill',
                source: sourceId,
                paint: {
                  'fill-color': '#B42222',
                  'fill-opacity': 0.5,
                },
                filter: ['==', '$type', 'Polygon'],
              });
            }
          });
        } catch (error) {
          console.error('Error parsing or adding GeoJSON data:', error);
        }
      }
    });
  }
}, [aiState.messages, map.current]);
Type Safety Concern

The properties field in FeatureSchema uses z.any() which bypasses type checking. Consider using a more specific schema for properties to maintain type safety.

  properties: z.record(z.any()).optional(),
});
Error Handling

The code catches errors when parsing GeoJSON but only logs them. Consider adding user-facing error handling or fallback behavior when GeoJSON data is invalid.

} catch (error) {
  console.error('Error parsing or adding GeoJSON data:', error);
}

@codiumai-pr-agent-free
Copy link
Contributor

codiumai-pr-agent-free bot commented Aug 14, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix MultiPoint filtering

The filter condition only matches 'Point' but not 'MultiPoint', causing
MultiPoint features to be invisible. Update the filter to handle both geometry
types correctly.

components/map/mapbox-map.tsx [549-561]

 geojson.features.forEach((feature: any) => {
   const featureLayerId = `${layerId}-${feature.geometry.type}`;
   if (feature.geometry.type === 'Point' || feature.geometry.type === 'MultiPoint') {
     map.current?.addLayer({
       id: featureLayerId,
       type: 'circle',
       source: sourceId,
       paint: {
         'circle-radius': 6,
         'circle-color': '#B42222',
       },
-      filter: ['==', '$type', 'Point'],
+      filter: ['in', '$type', 'Point', 'MultiPoint'],
     });
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix, as the code intends to handle MultiPoint geometries but the filter ['==', '$type', 'Point'] would prevent them from being rendered on the map.

High
Support multi-geometry types

Similar to the Point issue, the filters for LineString and Polygon don't handle
their Multi variants. Update the filters to include both single and multi
geometry types.

components/map/mapbox-map.tsx [562-587]

 } else if (feature.geometry.type === 'LineString' || feature.geometry.type === 'MultiLineString') {
   map.current?.addLayer({
     id: featureLayerId,
     type: 'line',
     source: sourceId,
     layout: {
       'line-join': 'round',
       'line-cap': 'round',
     },
     paint: {
       'line-color': '#B42222',
       'line-width': 4,
     },
-    filter: ['==', '$type', 'LineString'],
+    filter: ['in', '$type', 'LineString', 'MultiLineString'],
   });
 } else if (feature.geometry.type === 'Polygon' || feature.geometry.type === 'MultiPolygon') {
   map.current?.addLayer({
     id: featureLayerId,
     type: 'fill',
     source: sourceId,
     paint: {
       'fill-color': '#B42222',
       'fill-opacity': 0.5,
     },
-    filter: ['==', '$type', 'Polygon'],
+    filter: ['in', '$type', 'Polygon', 'MultiPolygon'],
   });
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical bug where MultiLineString and MultiPolygon geometries would not be rendered due to incorrect layer filters, despite the code's intent to handle them.

High
Handle both string and object

The code assumes message.content is a JSON string, but in AI tool responses,
content might already be parsed. Add a check to handle both string and object
formats to prevent JSON parsing errors.

components/map/mapbox-map.tsx [530-542]

 useEffect(() => {
   if (map.current && aiState.messages) {
     aiState.messages.forEach(message => {
       if (message.name === 'drawing' && message.role === 'tool') {
         try {
-          const { geojson } = JSON.parse(message.content);
+          const geojson = typeof message.content === 'string' 
+            ? JSON.parse(message.content).geojson 
+            : message.content.geojson;
           const sourceId = `geojson-source-${message.id}`;
           const layerId = `geojson-layer-${message.id}`;
 
           if (map.current?.getSource(sourceId)) {
             // Source already exists, no need to re-add
             return;
           }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This suggestion correctly identifies a potential runtime error if message.content is not a string and improves code robustness by handling both string and object types.

Low
  • Update

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 14, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix effect deps and cleanup
Suggestion Impact:The commit partially addressed the dependency array issue mentioned in the suggestion by changing the dependency from map.current to just aiState, which is more stable. It also fixed the type safety by adding proper type casting for aiState.

code diff:

-    if (map.current && aiState.messages) {
-      aiState.messages.forEach(message => {
+    if (map.current && aiState && (aiState as AIState).messages) {
+      (aiState as AIState).messages.forEach(message => {
         if (message.name === 'drawing' && message.role === 'tool') {
           try {
             const { geojson } = JSON.parse(message.content);
@@ -593,7 +593,7 @@
         }
       });
     }
-  }, [aiState.messages, map.current]);
+  }, [aiState]);

Accessing map.current in the dependency array will cause the effect to re-run on
every render without stability guarantees. Also, layers are not cleaned up,
leading to leaks and duplicate IDs across message updates. Use map as a stable
ref in deps, and add a cleanup/removal path for layers/sources tied to processed
message IDs.

components/map/mapbox-map.tsx [530-596]

 useEffect(() => {
-  if (map.current && aiState.messages) {
-    aiState.messages.forEach(message => {
-      if (message.name === 'drawing' && message.role === 'tool') {
-        try {
-          const { geojson } = JSON.parse(message.content);
-          const sourceId = `geojson-source-${message.id}`;
-          const layerId = `geojson-layer-${message.id}`;
+  if (!map.current || !aiState.messages) return;
 
-          if (map.current?.getSource(sourceId)) {
-            // Source already exists, no need to re-add
-            return;
-          }
+  aiState.messages.forEach(message => {
+    if (message.name === 'drawing' && message.role === 'tool') {
+      try {
+        const parsed = typeof message.content === 'string' ? JSON.parse(message.content) : message.content;
+        const geojson = parsed?.geojson;
+        if (!geojson || !geojson.features) return;
 
-          map.current?.addSource(sourceId, {
+        const sourceId = `geojson-source-${message.id}`;
+        const layerPrefix = `geojson-layer-${message.id}`;
+
+        if (!map.current.getSource(sourceId)) {
+          map.current.addSource(sourceId, {
             type: 'geojson',
             data: geojson,
           });
 
-          geojson.features.forEach((feature: any) => {
-            const featureLayerId = `${layerId}-${feature.geometry.type}`;
-            if (feature.geometry.type === 'Point' || feature.geometry.type === 'MultiPoint') {
-              map.current?.addLayer({
-                id: featureLayerId,
-                type: 'circle',
-                source: sourceId,
-                paint: {
-                  'circle-radius': 6,
-                  'circle-color': '#B42222',
-                },
-                filter: ['==', '$type', 'Point'],
-              });
-            } else if (feature.geometry.type === 'LineString' || feature.geometry.type === 'MultiLineString') {
-              map.current?.addLayer({
-                id: featureLayerId,
-                type: 'line',
-                source: sourceId,
-                layout: {
-                  'line-join': 'round',
-                  'line-cap': 'round',
-                },
-                paint: {
-                  'line-color': '#B42222',
-                  'line-width': 4,
-                },
-                filter: ['==', '$type', 'LineString'],
-              });
-            } else if (feature.geometry.type === 'Polygon' || feature.geometry.type === 'MultiPolygon') {
-              map.current?.addLayer({
-                id: featureLayerId,
-                type: 'fill',
-                source: sourceId,
-                paint: {
-                  'fill-color': '#B42222',
-                  'fill-opacity': 0.5,
-                },
-                filter: ['==', '$type', 'Polygon'],
-              });
+          // Add one layer per geometry type and filter by feature type
+          const ensureLayer = (id: string, layer: mapboxgl.AnyLayer) => {
+            if (!map.current!.getLayer(id)) {
+              map.current!.addLayer(layer);
             }
+          };
+
+          ensureLayer(`${layerPrefix}-point`, {
+            id: `${layerPrefix}-point`,
+            type: 'circle',
+            source: sourceId,
+            paint: { 'circle-radius': 6, 'circle-color': '#B42222' },
+            filter: ['==', '$type', 'Point'],
           });
-        } catch (error) {
-          console.error('Error parsing or adding GeoJSON data:', error);
+
+          ensureLayer(`${layerPrefix}-line`, {
+            id: `${layerPrefix}-line`,
+            type: 'line',
+            source: sourceId,
+            layout: { 'line-join': 'round', 'line-cap': 'round' },
+            paint: { 'line-color': '#B42222', 'line-width': 4 },
+            filter: ['==', '$type', 'LineString'],
+          });
+
+          ensureLayer(`${layerPrefix}-polygon`, {
+            id: `${layerPrefix}-polygon`,
+            type: 'fill',
+            source: sourceId,
+            paint: { 'fill-color': '#B42222', 'fill-opacity': 0.5 },
+            filter: ['==', '$type', 'Polygon'],
+          });
+        }
+      } catch (error) {
+        console.error('Error parsing or adding GeoJSON data:', error);
+      }
+    }
+  });
+
+  return () => {
+    if (!map.current || !aiState.messages) return;
+    aiState.messages.forEach(message => {
+      if (message.name === 'drawing' && message.role === 'tool') {
+        const sourceId = `geojson-source-${message.id}`;
+        const layerPrefix = `geojson-layer-${message.id}`;
+        ['point', 'line', 'polygon'].forEach(suffix => {
+          const id = `${layerPrefix}-${suffix}`;
+          if (map.current!.getLayer(id)) {
+            map.current!.removeLayer(id);
+          }
+        });
+        if (map.current.getSource(sourceId)) {
+          map.current.removeSource(sourceId);
         }
       }
     });
-  }
-}, [aiState.messages, map.current]);
+  };
+}, [aiState.messages]);

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical issue where map layers and sources are not cleaned up, leading to memory leaks and visual bugs, and provides a robust fix with a cleanup function. It also correctly refactors the layer creation logic to prevent crashes from duplicate layer IDs.

High
Safely parse tool payload
Suggestion Impact:The commit implemented a similar safety check by adding a condition to verify if geojson and geojson.features exist before proceeding, which addresses the core concern in the suggestion about preventing runtime crashes.

code diff:

+          const { geojson } = JSON.parse(message.content);
+          if (!geojson || !geojson.features) return;

message.content may already be an object depending on upstream usage, and
parsing without a type guard can throw. Add a safe parse that accepts objects
and validates presence of geojson before proceeding to avoid runtime crashes.

components/map/mapbox-map.tsx [535]

-const { geojson } = JSON.parse(message.content);
+const parsed = typeof message.content === 'string' ? JSON.parse(message.content) : message.content;
+const geojson = parsed?.geojson;
+if (!geojson || !geojson.features) {
+  return;
+}

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: This suggestion correctly identifies that JSON.parse could fail if message.content is not a string, and adds necessary checks to prevent a runtime crash, making the code more robust.

Medium
General
Preserve UI state shape

Returning null without isCollapsed can break UI assumptions where other tool
handlers include collapse state. Include an explicit isCollapsed boolean to keep
state shape consistent and prevent downstream UI errors.

app/actions.tsx [433-438]

 if (toolOutput.type === 'DRAWING') {
   return {
     id,
     component: null, // Replace with a new DrawingHandler component if needed
+    isCollapsed: true,
   };
 }
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion improves the consistency of the returned UI state object by adding the isCollapsed property, which is good practice for maintainability, although the property is optional in the type definition.

Low
  • More

This commit introduces a new `drawingTool` that allows me to draw GeoJSON features on the map.

The changes include:
- A new Zod schema for validating GeoJSON data.
- The `drawingTool` definition, which adds to my capabilities.
- Integration of this new capability so I can use it.
- Updates to my internal logic to handle the drawing output.
- Modifications to the Mapbox component to render the GeoJSON data I generate on the map.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🔭 Outside diff range comments (1)
app/actions.tsx (1)

423-431: Fix type mismatch: isCollapsed should be a StreamableValue, not a boolean

The UIState type defines isCollapsed?: StreamableValue. Returning a raw boolean here breaks the contract and can cause runtime/type issues.

Apply this diff:

-              return {
-                id, // message id
-                component: <MapQueryHandler toolOutput={toolOutput} />, 
-                isCollapsed: false, // Allow handler to be active
-              };
+              const collapsed = createStreamableValue<boolean>();
+              collapsed.done(false); // Allow handler to be active
+              return {
+                id, // message id
+                component: <MapQueryHandler toolOutput={toolOutput} />,
+                isCollapsed: collapsed.value,
+              };
🧹 Nitpick comments (5)
lib/agents/tools/index.tsx (1)

15-33: Clean up leftover mcp comment and consider typing the tools map

  • There’s a stale commented-out argument in geospatialQueryTool’s call. Remove it to avoid confusion.
  • tools is typed as any. Consider defining a ToolMap type (if available from your AI tooling) or at least Record<string, unknown> to keep the surface typed.

Apply this minimal cleanup:

     geospatialQueryTool: geospatialTool({
-      uiStream
-      // mcp: mcp || null // Removed mcp argument
+      uiStream
     }),
     drawing: drawingTool({
       uiStream
     })

Optional typing idea (outside this hunk):

// e.g., import type { ToolDefinition } from 'ai' (if such type exists)
// type ToolMap = Record<string, ToolDefinition>;
const tools: any = { ... } // -> const tools: ToolMap = { ... }
lib/schema/drawing.ts (2)

9-22: Allow foreign members and bbox on geometries

GeoJSON allows foreign members and optional bbox. Passing through unknown keys increases interoperability.

Apply this diff:

-const PointGeometrySchema = z.object({
+const PointGeometrySchema = z.object({
   type: z.literal('Point'),
-  coordinates: PointSchema,
-});
+  coordinates: PositionSchema,
+  bbox: z.array(z.number()).length(4).optional(),
+}).passthrough();
 
-const LineStringGeometrySchema = z.object({
+const LineStringGeometrySchema = z.object({
   type: z.literal('LineString'),
   coordinates: LineStringSchema,
-});
+  bbox: z.array(z.number()).length(4).optional(),
+}).passthrough();
 
-const PolygonGeometrySchema = z.object({
+const PolygonGeometrySchema = z.object({
   type: z.literal('Polygon'),
   coordinates: PolygonSchema,
-});
+  bbox: z.array(z.number()).length(4).optional(),
+}).passthrough();

38-46: FeatureCollection: add bbox and passthrough; keep schema descriptive

Add bbox at the collection level and allow foreign members.

Apply this diff:

-const FeatureCollectionSchema = z.object({
+const FeatureCollectionSchema = z.object({
   type: z.literal('FeatureCollection'),
   features: z.array(FeatureSchema),
-});
+  bbox: z.array(z.number()).length(4).optional(),
+}).passthrough();
 
 export const drawingSchema = z.object({
   geojson: FeatureCollectionSchema.describe("A valid GeoJSON FeatureCollection object to be drawn on the map."),
 });
components/map/mapbox-map.tsx (2)

14-15: Import AIState as a type-only import to avoid unnecessary bundling

This keeps the client bundle lean and avoids pulling server-only types into the client graph.

-import { AIState } from '@/app/actions';
+import type { AIState } from '@/app/actions';

37-37: Minor: Consider typing the hook to avoid later type casts

If useAIState supports generics, prefer const [aiState] = useAIState<AIState>() and remove downstream as AIState casts. If it doesn't, current usage is fine.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20bb0e5 and cbfe14d.

📒 Files selected for processing (6)
  • .env (1 hunks)
  • app/actions.tsx (2 hunks)
  • components/map/mapbox-map.tsx (3 hunks)
  • lib/agents/tools/drawing.tsx (1 hunks)
  • lib/agents/tools/index.tsx (2 hunks)
  • lib/schema/drawing.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
.env (1)
lib/db/migrate.ts (1)
  • runMigrations (8-34)
lib/agents/tools/drawing.tsx (1)
lib/schema/drawing.ts (1)
  • drawingSchema (44-46)
lib/schema/drawing.ts (2)
components/map/map-data-context.tsx (1)
  • MapData (7-17)
lib/actions/chat.ts (1)
  • updateDrawingContext (165-205)
lib/agents/tools/index.tsx (2)
lib/agents/tools/drawing.tsx (1)
  • drawingTool (4-17)
lib/agents/tools/geospatial.tsx (2)
  • uiStream (146-306)
  • query (159-305)
components/map/mapbox-map.tsx (3)
app/actions.tsx (1)
  • AIState (251-255)
components/chat.tsx (2)
  • Chat (20-123)
  • id (67-72)
components/map/map-data-context.tsx (1)
  • MapData (7-17)
🪛 dotenv-linter (3.3.0)
.env

[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)

🔇 Additional comments (1)
lib/agents/tools/index.tsx (1)

6-6: Wiring the drawing tool looks good

Importing and exposing the drawing tool aligns with the new schema and map rendering flow.

@@ -0,0 +1 @@
DATABASE_URL="postgresql://postgres:postgres@localhost:5432/postgres"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Do not commit .env with credentials; replace with .env.example and ignore .env

Committing environment files with secrets (even local defaults) is a security risk and makes accidental leaks likely. Also, dotenv-linter flags the quoted value. Recommended:

  • Remove .env from version control and add it to .gitignore.
  • Add a .env.example with placeholders for developers to copy locally.

Proposed changes:

Delete the committed secret (leaves the file empty if you don’t remove the file outright):

-DATABASE_URL="postgresql://postgres:postgres@localhost:5432/postgres"

Add a .env.example (outside this hunk) with placeholders:

# .env.example
# Copy to .env and fill real credentials locally
DATABASE_URL=postgresql://USER:PASSWORD@HOST:5432/DBNAME

And ignore .env (outside this hunk):

# .gitignore
.env

If, for some reason, this file must stay, at minimum remove quotes to satisfy dotenv-linter:

-DATABASE_URL="postgresql://postgres:postgres@localhost:5432/postgres"
+DATABASE_URL=postgresql://postgres:postgres@localhost:5432/postgres

I can open a follow-up PR to codify this workflow (add .env.example, update docs).

📝 Committable suggestion

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

Suggested change
DATABASE_URL="postgresql://postgres:postgres@localhost:5432/postgres"
🧰 Tools
🪛 dotenv-linter (3.3.0)

[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)

🤖 Prompt for AI Agents
In .env around lines 1 to 1, the repository contains a committed environment
file with hardcoded DB credentials and quoted value (security and dotenv-linter
issues); remove the sensitive .env from version control (git rm --cached if you
need to keep locally), add .env to .gitignore, create a .env.example with
placeholder values for DATABASE_URL for developers to copy, and if you must keep
a .env file in the repo temporarily remove the quotes around the value to
satisfy dotenv-linter (preferred: delete committed secret and replace with
.env.example).

Comment on lines +9 to +17
description: 'Draw GeoJSON features on the map. Use this tool to draw points, lines, and polygons.',
parameters: drawingSchema,
execute: async ({ geojson }: { geojson: any }) => {
return {
type: 'DRAWING',
geojson,
};
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Type the execute input and validate at runtime as a safety net

parameters uses drawingSchema for LLM tool calling, but adding an explicit runtime parse inside execute makes the tool robust against malformed inputs and keeps TypeScript happy.

Apply this diff:

-import { drawingSchema } from '@/lib/schema/drawing';
+import { drawingSchema } from '@/lib/schema/drawing';
+import type { z } from 'zod';

@@
-  execute: async ({ geojson }: { geojson: any }) => {
-    return {
-      type: 'DRAWING',
-      geojson,
-    };
-  },
+  execute: async (input: z.infer<typeof drawingSchema>) => {
+    // Safety net: validate again at runtime
+    const parsed = drawingSchema.parse(input);
+    return {
+      type: 'DRAWING' as const,
+      geojson: parsed.geojson,
+    };
+  },

If you prefer a softer failure mode, use safeParse and return an error to the UI stream instead of throwing.

📝 Committable suggestion

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

Suggested change
description: 'Draw GeoJSON features on the map. Use this tool to draw points, lines, and polygons.',
parameters: drawingSchema,
execute: async ({ geojson }: { geojson: any }) => {
return {
type: 'DRAWING',
geojson,
};
},
});
import { drawingSchema } from '@/lib/schema/drawing';
import type { z } from 'zod';
description: 'Draw GeoJSON features on the map. Use this tool to draw points, lines, and polygons.',
parameters: drawingSchema,
execute: async (input: z.infer<typeof drawingSchema>) => {
// Safety net: validate again at runtime
const parsed = drawingSchema.parse(input);
return {
type: 'DRAWING' as const,
geojson: parsed.geojson,
};
},
});
🤖 Prompt for AI Agents
In lib/agents/tools/drawing.tsx around lines 9 to 17, the execute handler
currently trusts the incoming { geojson } without runtime validation; update the
function signature to type the input explicitly (e.g., ({ geojson }: { geojson:
unknown }) or matching drawingSchema input type) and perform a runtime
validation using drawingSchema.parse or drawingSchema.safeParse before
proceeding; on successful parse return the existing DRAWING payload with the
validated geojson, and on failure either throw a clear error or (preferred
softer mode) return an error result that the UI can stream back to the user.

Comment on lines 4 to 7
const PointSchema = z.tuple([z.number(), z.number()]);
const LineStringSchema = z.array(PointSchema);
const PolygonSchema = z.array(LineStringSchema);

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Support altitude and proper minimums for coordinates

Per RFC 7946, positions may include additional elements (e.g., altitude). Lines should have at least 2 positions; rings should have at least 4.

Apply this diff:

-const PointSchema = z.tuple([z.number(), z.number()]);
-const LineStringSchema = z.array(PointSchema);
-const PolygonSchema = z.array(LineStringSchema);
+// Position supports [lng, lat, ...extras], e.g., altitude
+const PositionSchema = z.tuple([z.number(), z.number()]).rest(z.number());
+const LineStringSchema = z.array(PositionSchema).min(2);
+// Polygon is an array of LinearRings (first is outer, subsequent are holes)
+const LinearRingSchema = z.array(PositionSchema).min(4);
+const PolygonSchema = z.array(LinearRingSchema);

Optionally, enforce ring closure (first == last) via a superRefine on LinearRingSchema.

📝 Committable suggestion

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

Suggested change
const PointSchema = z.tuple([z.number(), z.number()]);
const LineStringSchema = z.array(PointSchema);
const PolygonSchema = z.array(LineStringSchema);
// Position supports [lng, lat, ...extras], e.g., altitude
const PositionSchema = z.tuple([z.number(), z.number()]).rest(z.number());
const LineStringSchema = z.array(PositionSchema).min(2);
// Polygon is an array of LinearRings (first is outer, subsequent are holes)
const LinearRingSchema = z.array(PositionSchema).min(4);
const PolygonSchema = z.array(LinearRingSchema);
🤖 Prompt for AI Agents
In lib/schema/drawing.ts around lines 4 to 7, the coordinate schemas currently
only allow 2-element positions and don't enforce minimum lengths for LineStrings
and LinearRings; update PointSchema to accept z.tuple([z.number(),
z.number()]).rest(z.number().optional()) (or equivalent) so positions can
include optional altitude, change LineStringSchema to
z.array(PointSchema).min(2) to require at least two positions, and replace
PolygonSchema with z.array(LinearRingSchema) where LinearRingSchema is
z.array(PointSchema).min(4) (and optionally add a superRefine on
LinearRingSchema to assert the first and last position are equal to enforce ring
closure).

Comment on lines 30 to 35
// GeoJSON Feature Schema
const FeatureSchema = z.object({
type: z.literal('Feature'),
geometry: GeometrySchema,
properties: z.record(z.any()).optional(),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Feature schema: allow id, nullable properties, bbox, and foreign members

These are common in real-world GeoJSON and increase compatibility with external tools.

Apply this diff:

-const FeatureSchema = z.object({
+const FeatureSchema = z.object({
   type: z.literal('Feature'),
   geometry: GeometrySchema,
-  properties: z.record(z.any()).optional(),
-});
+  properties: z.record(z.any()).nullable().optional(),
+  id: z.union([z.string(), z.number()]).optional(),
+  bbox: z.array(z.number()).length(4).optional(),
+}).passthrough();
📝 Committable suggestion

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

Suggested change
// GeoJSON Feature Schema
const FeatureSchema = z.object({
type: z.literal('Feature'),
geometry: GeometrySchema,
properties: z.record(z.any()).optional(),
});
// GeoJSON Feature Schema
const FeatureSchema = z.object({
type: z.literal('Feature'),
geometry: GeometrySchema,
properties: z.record(z.any()).nullable().optional(),
id: z.union([z.string(), z.number()]).optional(),
bbox: z.array(z.number()).length(4).optional(),
}).passthrough();
🤖 Prompt for AI Agents
In lib/schema/drawing.ts around lines 30 to 35, the FeatureSchema is too strict
for real-world GeoJSON; update it to accept an optional id (string or number),
make properties optional and nullable, add an optional bbox (array of numbers),
and allow foreign members by letting the object accept additional unknown keys
(use passthrough/catchall). Concretely, extend the FeatureSchema to include id?:
string|number, properties?: Record<string, any> | null, bbox?: number[], and
enable passthrough or catchall(z.any()) on the object so extra GeoJSON members
are preserved.

I've been given a new `drawingTool` that allows me to draw GeoJSON features on the map.

The changes include:
- A new Zod schema for validating GeoJSON data.
- The `drawingTool` definition that grants me this new ability.
- Updates to handle the tool's output.
- Modifications to the Mapbox component to render the GeoJSON data I generate on the map.

Note: The `saveChat` function call in `app/actions.tsx` has been temporarily commented out to resolve a runtime issue where the application would hang due to a missing database connection. The `DATABASE_URL` environment variable needs to be properly configured for the chat history saving to work.
…mentation of the drawing tool:

1.  **Revised Drawing Schema**: The schema in `lib/schema/drawing.ts` has been updated to be more comprehensive, including support for `MultiPoint`, `MultiLineString`, and `MultiPolygon` geometries, with stricter validation rules aligned with GeoJSON standards.

2.  **Fixed Duplicate Handling**: The redundant `case 'drawing'` in `app/actions.tsx` has been removed to streamline the logic.

3.  **Refactored Map Rendering**: The map rendering logic in `components/map/mapbox-map.tsx` has been completely refactored to:
    -   Ensure the map style is loaded before adding sources/layers.
    -   Update existing sources with new data instead of skipping them.
    -   Use a single layer per geometry type per message to prevent layer ID conflicts.
    -   Corrected the `useEffect` dependency array to remove linting warnings.

4.  **Runtime Responsiveness Fix**: The `saveChat` function call in `app/actions.tsx` remains commented out to prevent the application from hanging due to a missing database connection. I included a note about this in the PR description for you.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants