Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions components/map/map-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,20 @@ const GoogleMapComponent = dynamic(
{ ssr: false, loading: () => <div className="h-full w-full bg-gray-200 animate-pulse" /> }
)

const OSMMap = dynamic(
() => import('./osm-map').then(mod => mod.OSMMap),
{ ssr: false, loading: () => <div className="h-full w-full bg-gray-200 animate-pulse" /> }
)

export function MapProvider({ position }: { position?: { latitude: number; longitude: number; } }) {
const { mapProvider } = useSettingsStore()

return (
<>
{mapProvider === 'google' ? (
<GoogleMapComponent />
) : mapProvider === 'osm' ? (
<OSMMap />
) : (
<Mapbox position={position} />
)}
Expand Down
11 changes: 11 additions & 0 deletions components/map/osm-map.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.leaflet-measurement-label {
background: rgba(255, 255, 255, 0.8);
padding: 4px 8px;
border-radius: 4px;
font-size: 12px;
font-weight: bold;
color: #333333;
box-shadow: 0 2px 4px rgba(0,0,0,0.2);
pointer-events: none;
white-space: nowrap;
}
187 changes: 187 additions & 0 deletions components/map/osm-map.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
'use client'

import { useEffect, useRef, useCallback } from 'react'
import { MapContainer, TileLayer, FeatureGroup, useMap, useMapEvents } from 'react-leaflet'
import L from 'leaflet'
import 'leaflet/dist/leaflet.css'
import 'leaflet-draw/dist/leaflet.draw.css'
import * as turf from '@turf/turf'
import { useMapData } from './map-data-context'
import { useMapLoading } from '../map-loading-context'
import './osm-map.css'

// Leaflet's default icon path is not set up correctly in Next.js by default.
// This fix ensures that the marker icons are loaded correctly.
delete (L.Icon.Default.prototype as any)._getIconUrl
L.Icon.Default.mergeOptions({
iconRetinaUrl: require('leaflet/dist/images/marker-icon-2x.png').default,
iconUrl: require('leaflet/dist/images/marker-icon.png').default,
shadowUrl: require('leaflet/dist/images/marker-shadow.png').default,
})
Comment on lines +13 to +20

Choose a reason for hiding this comment

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

This file mutates global Leaflet defaults at module scope:

  • delete (L.Icon.Default.prototype as any)._getIconUrl
  • L.Icon.Default.mergeOptions(...)

Because osm-map.tsx is dynamically imported, this will run each time the chunk is evaluated; in dev/HMR it may execute multiple times. It also uses as any and require(...).default, which is brittle and makes the behavior harder to reason about.

This should be guarded to run only once and avoid the any escape hatch if possible.

Suggestion

Wrap the Leaflet icon patch in a one-time guard and avoid any by narrowing with an interface for the private field.

Example:

// osm-map.tsx (module scope)
let leafletIconPatched = false;

function patchLeafletDefaultIconOnce() {
  if (leafletIconPatched) return;
  leafletIconPatched = true;

  type IconDefaultWithPrivate = L.Icon.Default & { _getIconUrl?: unknown };
  const proto = L.Icon.Default.prototype as IconDefaultWithPrivate;
  delete proto._getIconUrl;

  L.Icon.Default.mergeOptions({
    iconRetinaUrl: new URL('leaflet/dist/images/marker-icon-2x.png', import.meta.url).toString(),
    iconUrl: new URL('leaflet/dist/images/marker-icon.png', import.meta.url).toString(),
    shadowUrl: new URL('leaflet/dist/images/marker-shadow.png', import.meta.url).toString(),
  });
}

patchLeafletDefaultIconOnce();

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.


// Formats the area or distance for display, consistent with other map components.
const formatMeasurement = (value: number, isArea = true) => {
if (isArea) {
return value >= 1000000
? `${(value / 1000000).toFixed(2)} km²`
: `${value.toFixed(2)} m²`
} else {
return value >= 1000
? `${(value / 1000).toFixed(2)} km`
: `${value.toFixed(0)} m`
}
}

const DrawControl = () => {
const map = useMap()
const { setMapData } = useMapData()
const featureGroupRef = useRef<L.FeatureGroup>(new L.FeatureGroup())
const labelsRef = useRef<{ [id: number]: L.Marker }>({})

const updateMeasurementLabels = useCallback(() => {
const layers = featureGroupRef.current.getLayers() as (L.Polygon | L.Polyline)[]
const currentDrawnFeatures: any[] = []

// Clear existing labels
Object.values(labelsRef.current).forEach(marker => marker.remove())
labelsRef.current = {}

layers.forEach(layer => {
const id = L.Util.stamp(layer)
const geojson = layer.toGeoJSON()
let measurement = ''
let labelPos: L.LatLngExpression | undefined

if (geojson.geometry.type === 'Polygon') {
const area = turf.area(geojson)
measurement = formatMeasurement(area, true)
const center = turf.centroid(geojson)
labelPos = [center.geometry.coordinates[1], center.geometry.coordinates[0]]
} else if (geojson.geometry.type === 'LineString') {
const length = turf.length(geojson, { units: 'meters' })
measurement = formatMeasurement(length, false)
const line = geojson.geometry.coordinates
const midpoint = line[Math.floor(line.length / 2)]
labelPos = [midpoint[1], midpoint[0]]
}

if (measurement && labelPos) {
const label = L.marker(labelPos, {
icon: L.divIcon({
className: 'leaflet-measurement-label',
html: `<span>${measurement}</span>`,
}),
}).addTo(map)
labelsRef.current[id] = label
}

currentDrawnFeatures.push({
id: id.toString(),
type: geojson.geometry.type,
measurement,
geometry: geojson.geometry,
});
})

setMapData(prev => ({ ...prev, drawnFeatures: currentDrawnFeatures }))
}, [map, setMapData])
Comment on lines +41 to +87

Choose a reason for hiding this comment

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

updateMeasurementLabels rebuilds all measurement markers on every create/edit/delete by removing all labels and re-adding them. For a small number of shapes this is fine, but it does not scale and will feel laggy with many features.

Also, the midpoint calculation for LineString uses the middle vertex (coordinates[Math.floor(n/2)]) rather than the midpoint along the line length, which produces incorrect labels for uneven segment lengths.

Suggestion

Improve both correctness and performance:

  1. Compute a true mid-point along the line using Turf:
const lengthKm = turf.length(geojson, { units: 'kilometers' });
const mid = turf.along(geojson, lengthKm / 2, { units: 'kilometers' });
labelPos = [mid.geometry.coordinates[1], mid.geometry.coordinates[0]];
  1. Update labels incrementally by layer id (add/update/remove) instead of clearing all markers each time. A simple first step is to only clear/recreate the label for the edited/created layer by using the event payload (e.layers, e.layer) from Leaflet Draw.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.


useEffect(() => {
const featureGroup = featureGroupRef.current
map.addLayer(featureGroup)

const drawControl = new L.Control.Draw({
edit: { featureGroup },
draw: {
polygon: {},
polyline: {},
rectangle: false,
circle: false,
marker: false,
circlemarker: false,
},
})
map.addControl(drawControl)

const onDrawCreated = (e: any) => {
const layer = e.layer
featureGroup.addLayer(layer)
updateMeasurementLabels()
}
Comment on lines +42 to +110

Choose a reason for hiding this comment

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

currentDrawnFeatures is typed as any[] and the draw event is typed as any. This is type-valid but reduces maintainability and increases the chance of runtime mistakes (e.g., assuming e.layer exists for all events).

Since this is core state (drawnFeatures) used elsewhere, it’s worth giving it an explicit shape and narrowing the event types.

Suggestion

Introduce a small local type for the drawn feature payload and minimally type the draw event(s) you use.

Example:

type DrawnFeature = {
  id: string;
  type: 'Polygon' | 'LineString';
  measurement: string;
  geometry: GeoJSON.Polygon | GeoJSON.LineString;
};

const currentDrawnFeatures: DrawnFeature[] = [];

const onDrawCreated = (e: L.LeafletEvent & { layer: L.Layer }) => {
  const layer = e.layer as L.Polygon | L.Polyline;
  featureGroup.addLayer(layer);
  updateMeasurementLabels();
};

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.


const onDrawEdited = () => updateMeasurementLabels()
const onDrawDeleted = () => updateMeasurementLabels()

map.on(L.Draw.Event.CREATED, onDrawCreated)
map.on(L.Draw.Event.EDITED, onDrawEdited)
map.on(L.Draw.Event.DELETED, onDrawDeleted)

return () => {
map.off(L.Draw.Event.CREATED, onDrawCreated)
map.off(L.Draw.Event.EDITED, onDrawEdited)
map.off(L.Draw.Event.DELETED, onDrawDeleted)
if (map.hasLayer(featureGroup)) {
map.removeLayer(featureGroup)
}
map.removeControl(drawControl)
}
}, [map, updateMeasurementLabels])
Comment on lines +119 to +128
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 | 🟠 Major

Memory leak: measurement labels not cleaned up on unmount.

The cleanup function removes the feature group and draw control, but measurement label markers stored in labelsRef.current are not removed. When DrawControl unmounts, these markers remain orphaned on the map.

Proposed fix
     return () => {
+      // Clean up measurement labels
+      Object.values(labelsRef.current).forEach(marker => marker.remove())
+      labelsRef.current = {}
+
       map.off(L.Draw.Event.CREATED, onDrawCreated)
       map.off(L.Draw.Event.EDITED, onDrawEdited)
       map.off(L.Draw.Event.DELETED, onDrawDeleted)
       if (map.hasLayer(featureGroup)) {
         map.removeLayer(featureGroup)
       }
       map.removeControl(drawControl)
     }
🤖 Prompt for AI Agents
In `@components/map/osm-map.tsx` around lines 119 - 128, The cleanup currently
removes draw listeners, the featureGroup and drawControl but forgets to remove
measurement label markers stored in labelsRef.current; iterate over
labelsRef.current (if any), remove each marker from the map (e.g.,
marker.remove() or map.removeLayer(marker)), then clear labelsRef.current (set
to empty array or null) before returning so no orphaned label markers remain
after unmount; apply this change in the same effect cleanup where map.off(...)
and map.removeControl(drawControl) are called.


return null
}

const MapEvents = ({ onMoveEnd }: { onMoveEnd: (map: L.Map) => void }) => {
const map = useMapEvents({
moveend: () => onMoveEnd(map),
zoomend: () => onMoveEnd(map),
});
return null;
};

export function OSMMap() {
const { mapData, setMapData } = useMapData()
const { setIsMapLoaded } = useMapLoading()

const initialCenter = mapData.cameraState
? [mapData.cameraState.center.lat, mapData.cameraState.center.lng] as [number, number]
: ([51.505, -0.09] as [number, number])
const initialZoom = mapData.cameraState ? mapData.cameraState.zoom : 13

useEffect(() => {
setIsMapLoaded(true)
return () => setIsMapLoaded(false)
}, [setIsMapLoaded])

Comment on lines +150 to +154

Choose a reason for hiding this comment

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

setIsMapLoaded(true) is called immediately on mount, not when Leaflet tiles/layers have actually loaded. That can cause downstream UI (or tools) to assume the map is ready while tiles are still blank or the map size hasn’t stabilized.

Leaflet exposes map events like load and tile-layer events like load/tileload that are more accurate signals.

Suggestion

Tie setIsMapLoaded(true) to a real Leaflet readiness event.

Example using whenReady:

<MapContainer
  ...
  whenReady={() => setIsMapLoaded(true)}
>

And keep the cleanup to set it false on unmount.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

const handleMapMoveEnd = useCallback((map: L.Map) => {
const center = map.getCenter()
const zoom = map.getZoom()
setMapData(prev => ({
...prev,
cameraState: {
...prev.cameraState,
center: { lat: center.lat, lng: center.lng },
zoom,
},
}))
}, [setMapData])

return (
<>
<MapContainer
center={initialCenter}
zoom={initialZoom}
scrollWheelZoom={true}
className="h-full w-full"
>
<TileLayer
attribution='&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors'
url="https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
/>
<FeatureGroup>
<DrawControl />
</FeatureGroup>
<MapEvents onMoveEnd={handleMapMoveEnd} />
Comment on lines +170 to +183

Choose a reason for hiding this comment

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

Unnecessary nested FeatureGroup and potential layer ownership confusion

You render a React <FeatureGroup> but the draw logic uses an independent featureGroupRef (new L.FeatureGroup()) that you manually add to the map. The React <FeatureGroup> wrapper is unused for controlling the layers and may confuse future maintainers (it implies children will be in that group, but DrawControl adds layers to the manually-managed group).

This can also complicate cleanup and make it harder to extend (e.g., if someone expects drawn layers to be in the React-managed group).

Suggestion

Either (a) remove the React <FeatureGroup> wrapper entirely, or (b) wire leaflet-draw to the React FeatureGroup instance instead of creating your own.

Simplest option is removing the wrapper:

<MapContainer ...>
  <TileLayer ... />
  <DrawControl />
  <MapEvents onMoveEnd={handleMapMoveEnd} />
</MapContainer>

Reply with "@CharlieHelps yes please" if you'd like me to add a commit applying this cleanup.

</MapContainer>
</>
)
}
4 changes: 4 additions & 0 deletions components/settings/components/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@ export function Settings({ initialTab = "system-prompt" }: SettingsProps) {
<RadioGroupItem value="google" id="google" />
<Label htmlFor="google">Google Maps</Label>
</div>
<div className="flex items-center space-x-2">
<RadioGroupItem value="osm" id="osm" />
<Label htmlFor="osm">OpenStreetMap</Label>
</div>
</RadioGroup>
</CardContent>
</Card>
Expand Down
2 changes: 1 addition & 1 deletion lib/agents/tools/geospatial.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ Uses the Mapbox Search Box Text Search API endpoint to power searching for and g

const selectedModel = await getSelectedModel();

if (selectedModel?.includes('gemini') && mapProvider === 'google') {
if (selectedModel?.includes('gemini') && (mapProvider === 'google' || mapProvider === 'osm')) {
let feedbackMessage = `Processing geospatial query with Gemini...`;
uiFeedbackStream.update(feedbackMessage);

Expand Down
2 changes: 1 addition & 1 deletion lib/store/settings.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { create } from 'zustand'

export type MapProvider = 'mapbox' | 'google'
export type MapProvider = 'mapbox' | 'google' | 'osm'

interface SettingsState {
mapProvider: MapProvider
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@
"@tailwindcss/typography": "^0.5.16",
"@tavily/core": "^0.6.4",
"@turf/turf": "^7.2.0",
"@types/leaflet": "^1.9.21",
"@types/leaflet-draw": "^1.0.13",
"@types/mapbox__mapbox-gl-draw": "^1.4.8",
"@types/pg": "^8.15.4",
"@types/react-leaflet": "^3.0.0",
Comment on lines +51 to +55
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 | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

react-leaflet v5 TypeScript definitions bundled included

💡 Result:

Yes — react-leaflet v5 includes/bundles its own TypeScript definitions. You still need to install Leaflet's types (e.g. @types/leaflet) as a dev dependency. [1][2]

Sources:

  • React Leaflet docs (installation / TypeScript support). [1]
  • React Leaflet v5 core API (exports types/interfaces). [2]

🌐 Web query:

@types/react-leaflet v3 react-leaflet v5 type conflicts compatibility

💡 Result:

Short answer: don't install @types/react-leaflet with react-leaflet v5 — it's a deprecated stub for older releases and will conflict with the built-in typings in react-leaflet v5. Remove @types/react-leaflet and keep only @types/leaflet (and a modern TS version). [1][2][3][4]

Why / action items:

  • react-leaflet v5 ships its own TypeScript definitions; the docs instruct using those and installing @types/leaflet only. [1][3]
  • @types/react-leaflet on npm is a deprecated stub (for older v3-era definitions) and can cause duplicate/type conflicts. Remove it. [2]
  • Also ensure your TypeScript version supports newer syntax (e.g. "export type" etc. — TS ≥3.8/4.x depending on your toolchain) to avoid related compiler errors. [4]

If you want, tell me your package.json (react-leaflet, @types/react-leaflet, @types/leaflet, typescript versions) and I’ll give exact commands to fix it.


Remove @types/react-leaflet from package.json — it conflicts with react-leaflet v5's bundled types.

react-leaflet v5.0.0 includes its own TypeScript definitions and should not be paired with @types/react-leaflet. The @types/react-leaflet package is a deprecated stub for older releases and will cause type conflicts. Remove line 55 ("@types/react-leaflet": "^3.0.0"); keep only @types/leaflet.

🤖 Prompt for AI Agents
In `@package.json` around lines 51 - 55, The package.json lists a conflicting
types package: remove the "@types/react-leaflet" dependency entry so it no
longer coexists with react-leaflet v5's bundled types; edit package.json to
delete the line containing "@types/react-leaflet": "^3.0.0" and run npm/yarn
install to update lockfile and node_modules, keeping "@types/leaflet" intact.

"@upstash/redis": "^1.35.0",
"@vercel/analytics": "^1.5.0",
"@vercel/speed-insights": "^1.2.0",
Expand All @@ -69,6 +72,8 @@
"geotiff": "^2.1.4-beta.1",
"glassmorphic": "^0.0.3",
"katex": "^0.16.22",
"leaflet": "^1.9.4",
"leaflet-draw": "^1.0.4",
"lodash": "^4.17.21",
"lottie-react": "^2.4.1",
"lucide-react": "^0.507.0",
Expand All @@ -83,6 +88,7 @@
"react-dom": "19.1.2",
"react-hook-form": "^7.56.2",
"react-icons": "^5.5.0",
"react-leaflet": "^5.0.0",
"react-markdown": "^9.1.0",
"react-textarea-autosize": "^8.5.9",
"react-toastify": "^10.0.6",
Expand Down