Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
DATABASE_URL="postgresql://user:password@host:port/db"
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1IjoiZHVtbXkiLCJhIjoiY2p1bW15In0.ZHVtbXk=
Comment on lines 1 to +2
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

Remove committed Mapbox token from tracked .env.

Even public Mapbox tokens can be abused and tied to account quotas; committed .env files are a common leak vector. Keep the real token in deployment secrets and use a placeholder (or move to .env.example) in the repo.

🛠️ Suggested placeholder
-NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1IjoiZHVtbXkiLCJhIjoiY2p1bW15In0.ZHVtbXk=
+NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=YOUR_MAPBOX_PUBLIC_TOKEN
📝 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://user:password@host:port/db"
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1IjoiZHVtbXkiLCJhIjoiY2p1bW15In0.ZHVtbXk=
DATABASE_URL="postgresql://user:password@host:port/db"
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=YOUR_MAPBOX_PUBLIC_TOKEN
🧰 Tools
🪛 dotenv-linter (4.0.0)

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

(QuoteCharacter)

🤖 Prompt for AI Agents
In @.env around lines 1 - 2, Remove the committed Mapbox token by replacing the
real value for NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN with a placeholder (e.g.
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=YOUR_MAPBOX_TOKEN_HERE) or move the current .env
contents into .env.example, add .env to .gitignore so secrets aren’t tracked,
and update deployment/config to load the real token from environment/secret
storage; reference the NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN variable in the repo to
locate and update all usages and rotate the exposed token in your Mapbox
account.

13 changes: 12 additions & 1 deletion app/search/[id]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@ export default async function SearchPage({ params }: SearchPageProps) {
};
});

// Extract initial map data from the latest 'data' message
const latestDataMessage = [...dbMessages].reverse().find(m => m.role === 'data');
let initialMapData = undefined;
if (latestDataMessage) {
try {
initialMapData = JSON.parse(latestDataMessage.content);
} catch (e) {
console.error('Failed to parse initial map data:', e);
}
}
Comment on lines +62 to +71
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 | 🟡 Minor

Validate parsed map data structure before use.

The parsed JSON is passed directly to MapDataProvider without validating that it conforms to the expected MapData shape ({ drawnFeatures, markers, imageOverlays }). Malformed data in the database could cause runtime errors in child components that expect these arrays.

Consider adding schema validation or at minimum structural checks:

🛡️ Proposed fix to add basic validation
   // Extract initial map data from the latest 'data' message
   const latestDataMessage = [...dbMessages].reverse().find(m => m.role === 'data');
   let initialMapData = undefined;
   if (latestDataMessage) {
     try {
-      initialMapData = JSON.parse(latestDataMessage.content);
+      const parsed = JSON.parse(latestDataMessage.content);
+      // Basic validation: ensure expected arrays exist
+      if (
+        parsed &&
+        typeof parsed === 'object' &&
+        Array.isArray(parsed.drawnFeatures) &&
+        Array.isArray(parsed.imageOverlays)
+      ) {
+        initialMapData = parsed;
+      } else {
+        console.warn('Parsed map data has unexpected shape, ignoring');
+      }
     } catch (e) {
       console.error('Failed to parse initial map data:', e);
     }
   }
🤖 Prompt for AI Agents
In `@app/search/`[id]/page.tsx around lines 62 - 71, The parsed JSON assigned to
initialMapData (from latestDataMessage) must be validated to ensure it matches
the expected MapData shape before passing to MapDataProvider; update the parsing
block (latestDataMessage / initialMapData) to: try parsing, then check that the
result is an object and that drawnFeatures, markers, and imageOverlays exist and
are arrays (or coerce to empty arrays as a safe default); on validation failure,
log a clear error and set initialMapData to a safe fallback object ({
drawnFeatures: [], markers: [], imageOverlays: [] }) so MapDataProvider and
downstream components never receive malformed data.


return (
<AI
initialAIState={{
Expand All @@ -68,7 +79,7 @@ export default async function SearchPage({ params }: SearchPageProps) {
// If needed for styling or other logic, it can be set.
}}
>
<MapDataProvider>
<MapDataProvider initialData={initialMapData}>
<Chat id={id} />
</MapDataProvider>
</AI>
Expand Down
19 changes: 10 additions & 9 deletions components/chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,22 @@ export function Chat({ id }: ChatProps) {
}
}, [isSubmitting])

// useEffect to call the server action when drawnFeatures changes
// useEffect to call the server action when map data changes
useEffect(() => {
if (id && mapData.drawnFeatures && mapData.cameraState) {
console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures);
if (id && (mapData.drawnFeatures || mapData.imageOverlays) && mapData.cameraState) {
console.log('Chat.tsx: map data changed, calling updateDrawingContext');
updateDrawingContext(id, {
drawnFeatures: mapData.drawnFeatures,
drawnFeatures: mapData.drawnFeatures || [],
imageOverlays: mapData.imageOverlays || [],
cameraState: mapData.cameraState,
});
}
}, [id, mapData.drawnFeatures, mapData.cameraState]);
}, [id, mapData.drawnFeatures, mapData.imageOverlays, mapData.cameraState]);

Choose a reason for hiding this comment

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

The server action is now triggered for any change to drawnFeatures/imageOverlays and requires cameraState. Two correctness concerns:

  1. mapData.drawnFeatures || mapData.imageOverlays will be truthy even when both are empty arrays (because [] is truthy), which can cause unnecessary writes on initialization.
  2. On rapid marker drags, you’ll call updateDrawingContext on every intermediate state, likely creating many data messages and hammering the server/DB.

This should be debounced/throttled and should distinguish “data actually changed” from “arrays exist”.

Suggestion

Debounce persistence and tighten the condition so it only runs when there is meaningful content to persist.

Example approach:

  • Compute counts: const hasMapEdits = (mapData.drawnFeatures?.length ?? 0) > 0 || (mapData.imageOverlays?.length ?? 0) > 0
  • Debounce the call (e.g., 300–800ms) and cancel on unmount.

Pseudo-code:

useEffect(() => {
  if (!id || !mapData.cameraState) return
  const hasMapEdits = (mapData.drawnFeatures?.length ?? 0) > 0 || (mapData.imageOverlays?.length ?? 0) > 0
  if (!hasMapEdits) return

  const t = setTimeout(() => {
    updateDrawingContext(id, {
      drawnFeatures: mapData.drawnFeatures ?? [],
      imageOverlays: mapData.imageOverlays ?? [],
      cameraState: mapData.cameraState,
    })
  }, 500)

  return () => clearTimeout(t)
}, [id, mapData.drawnFeatures, mapData.imageOverlays, mapData.cameraState])

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


// Mobile layout
if (isMobile) {
return (
<MapDataProvider> {/* Add Provider */}
<>
<HeaderSearchButton />
<div className="mobile-layout-container">
<div className="mobile-map-section">
Expand Down Expand Up @@ -157,13 +158,13 @@ export function Chat({ id }: ChatProps) {
)}
</div>
</div>
</MapDataProvider>
</>
);
}

// Desktop layout
return (
<MapDataProvider> {/* Add Provider */}
<>
<HeaderSearchButton />
<div className="flex justify-start items-start">
{/* This is the new div for scrolling */}
Expand Down Expand Up @@ -221,6 +222,6 @@ export function Chat({ id }: ChatProps) {
{activeView ? <SettingsView /> : <MapProvider />}
</div>
</div>
</MapDataProvider>
</>
);
}
167 changes: 167 additions & 0 deletions components/map/image-overlay-layer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
'use client'

import { useEffect, useRef, useCallback } from 'react'
import mapboxgl from 'mapbox-gl'
import { useMap } from './map-context'
import { useMapData, ImageOverlay } from './map-data-context'
import { useMapToggle, MapToggleEnum } from '../map-toggle-context'
import { X } from 'lucide-react'
import { Button } from '../ui/button'
import { createRoot } from 'react-dom/client'

interface ImageOverlayLayerProps {
overlay: ImageOverlay;
}

export function ImageOverlayLayer({ overlay }: ImageOverlayLayerProps) {
const { map } = useMap()
const { setMapData } = useMapData()
const { mapType } = useMapToggle()
const isDrawingMode = mapType === MapToggleEnum.DrawingMode
const markersRef = useRef<mapboxgl.Marker[]>([])
const deleteMarkerRef = useRef<mapboxgl.Marker | null>(null)
const deleteRootRef = useRef<any>(null)

const sourceId = `image-source-${overlay.id}`
const layerId = `image-layer-${overlay.id}`

// Update overlay coordinates in global state
const updateCoordinates = useCallback((index: number, lngLat: [number, number]) => {
setMapData(prev => {
const overlays = prev.imageOverlays || []
const newOverlays = overlays.map(o => {
if (o.id === overlay.id) {
const newCoords = [...o.coordinates] as [[number, number], [number, number], [number, number], [number, number]]
newCoords[index] = lngLat
return { ...o, coordinates: newCoords }
}
return o
})
return { ...prev, imageOverlays: newOverlays }
})
}, [overlay.id, setMapData])

const removeOverlay = useCallback(() => {
setMapData(prev => ({
...prev,
imageOverlays: (prev.imageOverlays || []).filter(o => o.id !== overlay.id)
}))
}, [overlay.id, setMapData])

useEffect(() => {
if (!map) return

const onMapLoad = () => {
if (!map.getSource(sourceId)) {
map.addSource(sourceId, {
type: 'image',
url: overlay.url,
coordinates: overlay.coordinates
})

map.addLayer({
id: layerId,
type: 'raster',
source: sourceId,
paint: {
'raster-opacity': overlay.opacity || 0.7
}
})
Comment on lines +62 to +69
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 | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -name "image-overlay-layer.tsx" -type f

Repository: QueueLab/QCX

Length of output: 96


🏁 Script executed:

wc -l ./components/map/image-overlay-layer.tsx

Repository: QueueLab/QCX

Length of output: 100


🏁 Script executed:

sed -n '55,75p' ./components/map/image-overlay-layer.tsx

Repository: QueueLab/QCX

Length of output: 507


🏁 Script executed:

sed -n '90,105p' ./components/map/image-overlay-layer.tsx

Repository: QueueLab/QCX

Length of output: 446


🏁 Script executed:

grep -n 'overlay.opacity' ./components/map/image-overlay-layer.tsx

Repository: QueueLab/QCX

Length of output: 234


🏁 Script executed:

cat -n ./components/map/image-overlay-layer.tsx | sed -n '1,30p'

Repository: QueueLab/QCX

Length of output: 1366


🏁 Script executed:

grep -A 5 'interface ImageOverlay' ./components/map/map-data-context.tsx

Repository: QueueLab/QCX

Length of output: 246


Allow explicit opacity = 0.

overlay.opacity || 0.7 treats 0 as falsy and forces 0.7, preventing fully transparent overlays. Use ?? instead.

Suggested fix
-            'raster-opacity': overlay.opacity || 0.7
+            'raster-opacity': overlay.opacity ?? 0.7
-      map.setPaintProperty(layerId, 'raster-opacity', overlay.opacity || 0.7)
+      map.setPaintProperty(layerId, 'raster-opacity', overlay.opacity ?? 0.7)

Applies to lines 67 and 98.

🤖 Prompt for AI Agents
In `@components/map/image-overlay-layer.tsx` around lines 62 - 69, The raster
layer's opacity uses a truthy fallback (overlay.opacity || 0.7) which treats 0
as false and prevents fully transparent overlays; update both uses (the paint
'raster-opacity' in the map.addLayer call where layerId is added and the other
occurrence around line 98) to use the nullish coalescing operator
(overlay.opacity ?? 0.7) so that explicit 0 is honored while still defaulting to
0.7 when opacity is undefined or null.

}
}

if (map.isStyleLoaded()) {
onMapLoad()
} else {
map.on('load', onMapLoad)
}

return () => {
if (map.getLayer(layerId)) map.removeLayer(layerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}, [map, sourceId, layerId, overlay.url]) // coordinates handled separately
Comment on lines +51 to +83

Choose a reason for hiding this comment

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

The effect that adds the Mapbox source/layer subscribes to map.on('load', onMapLoad) when style isn’t loaded, but it never unregisters the listener. If the component unmounts before load fires, the handler can still run later and attempt to add sources/layers for an overlay that no longer exists.

You should call map.off('load', onMapLoad) in cleanup when you register it.

Suggestion

Track whether you registered the load handler and unregister it in cleanup.

Example:

let subscribed = false
if (!map.isStyleLoaded()) {
  map.on('load', onMapLoad)
  subscribed = true
} else {
  onMapLoad()
}

return () => {
  if (subscribed) map.off('load', onMapLoad)
  if (map.getLayer(layerId)) map.removeLayer(layerId)
  if (map.getSource(sourceId)) map.removeSource(sourceId)
}

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

Comment on lines +51 to +83
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

🏁 Script executed:

fd "image-overlay-layer.tsx" --type f

Repository: QueueLab/QCX

Length of output: 94


🏁 Script executed:

cat -n components/map/image-overlay-layer.tsx

Repository: QueueLab/QCX

Length of output: 6094


Fix Mapbox and React resource leaks in overlay lifecycle.

Three issues cause resource leaks when overlays are added/removed:

  1. The map.on('load', onMapLoad) listener (line 76) isn't detached in cleanup, leaving orphaned listeners if the component unmounts before the map style loads.
  2. The React root for the delete button (line 137–138) is never unmounted in cleanup (line 160–163), leaking the React tree.
  3. Opacity uses || instead of ?? (lines 67, 98), so setting opacity to 0 incorrectly defaults to 0.7 instead.

Additionally, consider wrapping forEach callbacks in explicit blocks for clarity (lines 107, 161).

Suggested fixes

Line 79–82: Add listener cleanup

     return () => {
+      map.off('load', onMapLoad)
       if (map.getLayer(layerId)) map.removeLayer(layerId)
       if (map.getSource(sourceId)) map.removeSource(sourceId)
     }

Lines 160–163: Unmount React root

     return () => {
       markersRef.current.forEach(m => m.remove())
       if (deleteMarkerRef.current) deleteMarkerRef.current.remove()
+      if (deleteRootRef.current) {
+        deleteRootRef.current.unmount()
+        deleteRootRef.current = null
+      }
     }

Lines 67, 98: Use nullish coalescing

-            'raster-opacity': overlay.opacity || 0.7
+            'raster-opacity': overlay.opacity ?? 0.7

Lines 107, 161: Wrap forEach callbacks explicitly

-    markersRef.current.forEach(m => m.remove())
+    markersRef.current.forEach(m => { m.remove() })
📝 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
useEffect(() => {
if (!map) return
const onMapLoad = () => {
if (!map.getSource(sourceId)) {
map.addSource(sourceId, {
type: 'image',
url: overlay.url,
coordinates: overlay.coordinates
})
map.addLayer({
id: layerId,
type: 'raster',
source: sourceId,
paint: {
'raster-opacity': overlay.opacity || 0.7
}
})
}
}
if (map.isStyleLoaded()) {
onMapLoad()
} else {
map.on('load', onMapLoad)
}
return () => {
if (map.getLayer(layerId)) map.removeLayer(layerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}, [map, sourceId, layerId, overlay.url]) // coordinates handled separately
useEffect(() => {
if (!map) return
const onMapLoad = () => {
if (!map.getSource(sourceId)) {
map.addSource(sourceId, {
type: 'image',
url: overlay.url,
coordinates: overlay.coordinates
})
map.addLayer({
id: layerId,
type: 'raster',
source: sourceId,
paint: {
'raster-opacity': overlay.opacity ?? 0.7
}
})
}
}
if (map.isStyleLoaded()) {
onMapLoad()
} else {
map.on('load', onMapLoad)
}
return () => {
map.off('load', onMapLoad)
if (map.getLayer(layerId)) map.removeLayer(layerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}, [map, sourceId, layerId, overlay.url]) // coordinates handled separately
🤖 Prompt for AI Agents
In `@components/map/image-overlay-layer.tsx` around lines 51 - 83, The useEffect's
onMapLoad listener isn't removed on cleanup, the image layer opacity uses ||
causing 0 to default to 0.7, and the React root created for the delete button is
never unmounted; fix by (1) storing onMapLoad and, in the cleanup return, call
map.off('load', onMapLoad) if you previously attached it (or always call map.off
to be safe) alongside removing layer/source in useEffect, (2) change any
'raster-opacity': overlay.opacity || 0.7 (and the other occurrence) to use
nullish coalescing 'raster-opacity': overlay.opacity ?? 0.7, and (3) keep the
createRoot instance (e.g., deleteButtonRoot or similar variable used where you
call createRoot) in scope and call its unmount() in the same cleanup; also wrap
forEach callbacks in explicit { } blocks for clarity.


// Sync coordinates with map source
useEffect(() => {
if (!map) return
const source = map.getSource(sourceId) as mapboxgl.ImageSource
if (source) {
source.setCoordinates(overlay.coordinates)
}
}, [map, overlay.coordinates, sourceId])

// Sync opacity
useEffect(() => {
if (!map) return
if (map.getLayer(layerId)) {
map.setPaintProperty(layerId, 'raster-opacity', overlay.opacity || 0.7)
}
}, [map, overlay.opacity, layerId])

// Draggable markers for corners
useEffect(() => {
if (!map) return

// Clean up existing markers
markersRef.current.forEach(m => m.remove())
markersRef.current = []
if (deleteMarkerRef.current) {
deleteMarkerRef.current.remove()
deleteMarkerRef.current = null
}

if (isDrawingMode) {
// Add 4 corner markers
overlay.coordinates.forEach((coord, index) => {
const el = document.createElement('div')
el.className = 'w-4 h-4 bg-white border-2 border-primary rounded-full cursor-move shadow-md'

const marker = new mapboxgl.Marker({
element: el,
draggable: true
})
.setLngLat(coord)
.addTo(map)

marker.on('drag', () => {
const newLngLat = marker.getLngLat()
updateCoordinates(index, [newLngLat.lng, newLngLat.lat])
})

markersRef.current.push(marker)
})

// Add delete button near top-right corner
const deleteEl = document.createElement('div')
const root = createRoot(deleteEl)
deleteRootRef.current = root
root.render(
<Button
variant="destructive"
size="icon"
className="w-6 h-6 rounded-full shadow-lg"
onClick={removeOverlay}
>
<X size={14} />
</Button>
)

const deleteMarker = new mapboxgl.Marker({
element: deleteEl,
offset: [0, -20]
})
.setLngLat(overlay.coordinates[1]) // Near Top-right
.addTo(map)

deleteMarkerRef.current = deleteMarker
}

return () => {
markersRef.current.forEach(m => m.remove())
if (deleteMarkerRef.current) deleteMarkerRef.current.remove()
}
Comment on lines +23 to +163

Choose a reason for hiding this comment

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

deleteRootRef is typed as any and never unmounted. Because you create a React root inside a Mapbox marker element, you should call root.unmount() during cleanup; otherwise you risk leaking React trees and event handlers when toggling drawing mode, changing overlay coordinates, or unmounting the layer.

Also, you remove the marker element in cleanup but don’t clear deleteRootRef.current, which makes repeated toggles harder to reason about.

Suggestion

Avoid any and always unmount the React root you create.

Suggested changes:

  • Type: const deleteRootRef = useRef<import('react-dom/client').Root | null>(null)
  • Cleanup:
return () => {
  markersRef.current.forEach(m => m.remove())
  markersRef.current = []

  deleteMarkerRef.current?.remove()
  deleteMarkerRef.current = null

  deleteRootRef.current?.unmount()
  deleteRootRef.current = null
}

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

}, [map, isDrawingMode, overlay.coordinates, updateCoordinates, removeOverlay])

Comment on lines +102 to +165

Choose a reason for hiding this comment

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

You set the delete marker location to overlay.coordinates[1] and comment it as “Near Top-right”, but the coordinate ordering is documented as [NW, NE, SE, SW], so [1] is exactly NE. That’s fine, but the marker won’t follow when coordinates change unless drawing mode is re-entered because you only create the delete marker inside the drawing-mode effect.

Since that effect depends on overlay.coordinates, it will re-run and recreate the delete marker on every drag (because you update global state on every drag). Combined with the lack of debouncing, this means you’re constantly destroying/recreating the React root and marker during drags.

This is avoidable churn.

Suggestion

Don’t recreate the delete marker/root on every coordinate update.

Option A: Create markers once when entering drawing mode, then update their positions in a separate effect that runs on overlay.coordinates.

Option B: Keep the current effect but move the delete marker position update to a lighter-weight branch:

// when already exists
deleteMarkerRef.current?.setLngLat(overlay.coordinates[1])

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

return null
}
12 changes: 10 additions & 2 deletions components/map/map-data-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ export interface CameraState {
heading?: number;
}

export interface ImageOverlay {
id: string;
url: string;
coordinates: [[number, number], [number, number], [number, number], [number, number]]; // [NW, NE, SE, SW]
opacity: number;
}

export interface MapData {
targetPosition?: { lat: number; lng: number } | null; // For flying to a location
cameraState?: CameraState; // For saving camera state
Expand All @@ -29,6 +36,7 @@ export interface MapData {
longitude: number;
title?: string;
}>;
imageOverlays?: ImageOverlay[];
}

interface MapDataContextType {
Expand All @@ -38,8 +46,8 @@ interface MapDataContextType {

const MapDataContext = createContext<MapDataContextType | undefined>(undefined);

export const MapDataProvider: React.FC<{ children: ReactNode }> = ({ children }) => {
const [mapData, setMapData] = useState<MapData>({ drawnFeatures: [], markers: [] });
export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => {
const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [], imageOverlays: [] });
Comment on lines +49 to +50

Choose a reason for hiding this comment

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

MapDataProvider initializes state from initialData verbatim, which can leave drawnFeatures, markers, and imageOverlays as undefined depending on what the persisted payload contains. You partially work around this elsewhere with || [], but leaving the canonical state shape inconsistent tends to create subtle bugs (e.g., code that maps without guarding, or merge logic that assumes arrays exist).

Because initialData comes from JSON.parse, it’s especially important to normalize defaults at the boundary.

Suggestion

Normalize initialData into a full MapData object with array defaults rather than storing it raw.

For example:

export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => {
  const normalized: MapData = {
    drawnFeatures: [],
    markers: [],
    imageOverlays: [],
    ...initialData,
    drawnFeatures: initialData?.drawnFeatures ?? [],
    markers: initialData?.markers ?? [],
    imageOverlays: initialData?.imageOverlays ?? [],
  }

  const [mapData, setMapData] = useState<MapData>(normalized)
  ...
}

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

Comment on lines +49 to +50
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 | 🟡 Minor

Merge defaults when initialData is supplied.

initialData bypasses defaults, so partial data (older chats) can leave drawnFeatures, markers, or imageOverlays undefined. Merge with defaults to keep array invariants stable.

✅ Suggested fix
-export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => {
-  const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [], imageOverlays: [] });
+export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => {
+  const [mapData, setMapData] = useState<MapData>(() => ({
+    drawnFeatures: [],
+    markers: [],
+    imageOverlays: [],
+    ...initialData
+  }));
📝 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
export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => {
const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [], imageOverlays: [] });
export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => {
const [mapData, setMapData] = useState<MapData>(() => ({
drawnFeatures: [],
markers: [],
imageOverlays: [],
...initialData
}));
🤖 Prompt for AI Agents
In `@components/map/map-data-context.tsx` around lines 49 - 50, MapDataProvider
currently sets state to initialData directly which can omit
drawnFeatures/markers/imageOverlays; change the initialization in the
MapDataProvider (where mapData and setMapData are created) to merge initialData
with a default MapData object so missing fields fall back to empty arrays (e.g.
create a default like { drawnFeatures: [], markers: [], imageOverlays: [] } and
shallow-merge with initialData), and ensure any later setters that replace
mapData preserve those array defaults when merging updates.


return (
<MapDataContext.Provider value={{ mapData, setMapData }}>
Expand Down
4 changes: 4 additions & 0 deletions components/map/mapbox-map.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { useMapToggle, MapToggleEnum } from '../map-toggle-context'
import { useMapData } from './map-data-context'; // Add this import
import { useMapLoading } from '../map-loading-context'; // Import useMapLoading
import { useMap } from './map-context'
import { ImageOverlayLayer } from './image-overlay-layer'

mapboxgl.accessToken = process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN as string;

Expand Down Expand Up @@ -569,6 +570,9 @@ export const Mapbox: React.FC<{ position?: { latitude: number; longitude: number
onMouseUp={handleMouseUp}
onMouseLeave={handleMouseUp} // Clear timer if mouse leaves container while pressed
/>
{mapData.imageOverlays?.map(overlay => (
<ImageOverlayLayer key={overlay.id} overlay={overlay} />
))}
</div>
)
}
57 changes: 56 additions & 1 deletion components/user-message.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
'use client'

import React from 'react'
import Image from 'next/image'
import { ChatShare } from './chat-share'
import { Button } from './ui/button'
import { MapPin } from 'lucide-react'
import { useMap } from './map/map-context'
import { useMapData } from './map/map-data-context'
import { useMapToggle, MapToggleEnum } from './map-toggle-context'
import { nanoid } from 'nanoid'

type UserMessageContentPart =
| { type: 'text'; text: string }
Expand All @@ -17,6 +25,9 @@ export const UserMessage: React.FC<UserMessageProps> = ({
chatId,
showShare = false
}) => {
const { map } = useMap()
const { setMapData } = useMapData()
const { setMapType } = useMapToggle()
Comment on lines +28 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

# Search for UserMessage component usage
rg -n '<UserMessage' --type tsx --type ts -C 3

Repository: QueueLab/QCX

Length of output: 83


🏁 Script executed:

# Find the hook implementations to see if they throw errors
fd -e tsx -e ts | xargs rg -l 'export.*useMap|export.*useMapData|export.*useMapToggle' | head -10

Repository: QueueLab/QCX

Length of output: 260


🏁 Script executed:

# Look for provider setup in the app
rg -n 'MapProvider|MapDataProvider|MapToggleProvider' --type tsx --type ts -C 5

Repository: QueueLab/QCX

Length of output: 83


🏁 Script executed:

# Search for UserMessage component usage
rg -n '<UserMessage' -C 3

Repository: QueueLab/QCX

Length of output: 2371


🏁 Script executed:

# Find app.tsx or page.tsx to see provider setup
fd -name 'page.tsx' -o -name 'app.tsx' -o -name 'layout.tsx'

Repository: QueueLab/QCX

Length of output: 286


🏁 Script executed:

# Look at the hook implementations
cat components/map/map-context.tsx | head -50

Repository: QueueLab/QCX

Length of output: 860


🏁 Script executed:

# Check MapDataContext
cat components/map/map-data-context.tsx | head -50

Repository: QueueLab/QCX

Length of output: 1734


🏁 Script executed:

# Check MapToggleContext
cat components/map-toggle-context.tsx | head -50

Repository: QueueLab/QCX

Length of output: 1154


🏁 Script executed:

# Search for app.tsx or main layout/page setup
fd -name 'page.tsx' | head -5

Repository: QueueLab/QCX

Length of output: 286


🏁 Script executed:

# Check the app page structure
cat app/page.tsx | head -100

Repository: QueueLab/QCX

Length of output: 470


🏁 Script executed:

# Find and view useMapData hook implementation
rg -n 'export const useMapData' -A 10

Repository: QueueLab/QCX

Length of output: 544


🏁 Script executed:

# Check if there's a root layout with providers
find . -name 'layout.tsx' -type f

Repository: QueueLab/QCX

Length of output: 72


🏁 Script executed:

# Check the root layout for providers
cat app/layout.tsx | head -150

Repository: QueueLab/QCX

Length of output: 2861


🏁 Script executed:

# Check the Chat component to see what it renders and where UserMessage is used
cat components/chat.tsx | head -100

Repository: QueueLab/QCX

Length of output: 3498


🏁 Script executed:

# Search for where MapProvider and MapToggleProvider are used
rg -n 'MapProvider|MapToggleProvider' -B 2 -A 2

Repository: QueueLab/QCX

Length of output: 8703


🏁 Script executed:

# Check where search-related.tsx and header-search-button.tsx are used
rg -n 'search-related|header-search-button' --type jsx --type js

Repository: QueueLab/QCX

Length of output: 83


🏁 Script executed:

# Check followup-panel.tsx context
rg -n 'FollowupPanel|followup-panel' --type jsx --type js -B 2 -A 2

Repository: QueueLab/QCX

Length of output: 83


🏁 Script executed:

# Check if MapDataProvider is wrapped globally or only in page.tsx
rg -n 'MapDataProvider'

Repository: QueueLab/QCX

Length of output: 842


🏁 Script executed:

# View the complete user-message.tsx file to see all hook usage
cat components/user-message.tsx

Repository: QueueLab/QCX

Length of output: 3414


🏁 Script executed:

# Check where search-related.tsx is rendered
rg -n 'search-related|SearchRelated' -B 2 -A 2

Repository: QueueLab/QCX

Length of output: 2123


🏁 Script executed:

# Check where header-search-button.tsx is rendered
rg -n 'header-search-button|HeaderSearchButton' -B 2 -A 2

Repository: QueueLab/QCX

Length of output: 1232


🏁 Script executed:

# Check where followup-panel.tsx is rendered
rg -n 'followup-panel|FollowupPanel' -B 2 -A 2

Repository: QueueLab/QCX

Length of output: 1546


🏁 Script executed:

# View header.tsx to see if it's at root level
cat components/header.tsx | head -80

Repository: QueueLab/QCX

Length of output: 2481


🏁 Script executed:

# Verify if Chat component is used anywhere outside app/page.tsx and app/search/[id]/page.tsx
rg -n 'import.*Chat|<Chat' | grep -v 'ChatPanel\|ChatMessages\|ChatShare'

Repository: QueueLab/QCX

Length of output: 1700


🏁 Script executed:

# Double-check: are there any other entry points that render Chat without MapDataProvider?
fd -name 'page.tsx' -o -name 'layout.tsx' | xargs rg -l 'Chat'

Repository: QueueLab/QCX

Length of output: 1004


🏁 Script executed:

# Check if app/actions.tsx is a server action file and see how UserMessage is used there
head -10 app/actions.tsx

Repository: QueueLab/QCX

Length of output: 273


🏁 Script executed:

# Check the context where UserMessage components are rendered - verify they're all within Chat
grep -n "component:" app/actions.tsx | head -20

Repository: QueueLab/QCX

Length of output: 848


🏁 Script executed:

# Verify HeaderSearchButton is only used in Chat
rg -n 'HeaderSearchButton' -B 3 -A 1

Repository: QueueLab/QCX

Length of output: 1254


🏁 Script executed:

# Final check: verify "Place on Map" button is always rendered or only when specific conditions are met
rg -n 'Place on Map|handlePlaceOnMap' -B 5 -A 2

Repository: QueueLab/QCX

Length of output: 1280


All three hooks are currently used safely within the Chat context.

UserMessage is always rendered through components within the Chat component, which is wrapped with MapDataProvider in both entry points (app/page.tsx and app/search/[id]/page.tsx). MapProvider and MapToggleProvider wrap the entire application in the root layout. Therefore, useMap(), useMapData(), and useMapToggle() will not throw errors in current usage.

The "Place on Map" button is only rendered when an image is attached (imagePart exists), and the handler has guard clauses (if (!map || !imagePart) return), so the feature gracefully handles edge cases.

If UserMessage is ever extracted and used outside the Chat component in the future, the hooks would fail. Consider adopting defensive context access if you anticipate broader reusability.

🤖 Prompt for AI Agents
In `@components/user-message.tsx` around lines 28 - 30, The current hooks (useMap,
useMapData, useMapToggle) assume their contexts always exist which will break if
UserMessage is used outside Chat; change the hooks to be defensive (return
null/undefined or safe no-op functions instead of throwing when their Provider
is missing) and update UserMessage to safely read from them (e.g., const mapCtx
= useMap(); const map = mapCtx?.map; and const setMapDataFn =
useMapData()?.setMapData; const setMapTypeFn = useMapToggle()?.setMapType) and
keep the existing guard in the Place on Map handler that checks for map and
imagePart before using them. Ensure hook return types reflect optional values so
callers (UserMessage) can null-check before invoking methods like setMapData and
setMapType.

const enableShare = process.env.ENABLE_SHARE === 'true'

// Normalize content to an array
Expand All @@ -31,18 +42,62 @@ export const UserMessage: React.FC<UserMessageProps> = ({
(part): part is { type: 'image'; image: string } => part.type === 'image'
)?.image

const handlePlaceOnMap = () => {
if (!map || !imagePart) return

const bounds = map.getBounds()
if (!bounds) return
const north = bounds.getNorth()
const south = bounds.getSouth()
const east = bounds.getEast()
const west = bounds.getWest()

const latStep = (north - south) / 4
const lngStep = (east - west) / 4

const nw: [number, number] = [west + lngStep, north - latStep]
const ne: [number, number] = [east - lngStep, north - latStep]
const se: [number, number] = [east - lngStep, south + latStep]
const sw: [number, number] = [west + lngStep, south + latStep]

const newOverlay = {
id: nanoid(),
url: imagePart,
coordinates: [nw, ne, se, sw] as [[number, number], [number, number], [number, number], [number, number]],
opacity: 0.7
}

setMapData(prev => ({
...prev,
imageOverlays: [...(prev.imageOverlays || []), newOverlay]
}))

setMapType(MapToggleEnum.DrawingMode)
}

return (
<div className="flex items-start w-full space-x-3 mt-2">
<div className="flex-1 space-y-2">
{imagePart && (
<div className="p-2 border rounded-lg bg-muted w-fit">
<div className="group relative p-2 border rounded-lg bg-muted w-fit">
<Image
src={imagePart}
alt="attachment"
width={300}
height={300}
className="max-w-xs max-h-64 rounded-md object-contain"
/>
<div className="absolute top-4 right-4 opacity-0 group-hover:opacity-100 transition-opacity">
<Button
variant="secondary"
size="sm"
className="h-8 px-2 shadow-md"
onClick={handlePlaceOnMap}
>
<MapPin size={14} className="mr-1" />
Place on Map
</Button>
</div>
</div>
Comment on lines +82 to 101
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider touch device accessibility for the hover-revealed button.

The button only becomes visible on hover (opacity-0 group-hover:opacity-100), which works for mouse users but may not be discoverable on touch devices where hover states don't exist.

Consider adding group-focus-within:opacity-100 or making the button always visible (perhaps with reduced prominence):

♿ Suggested accessibility improvement
- <div className="absolute top-4 right-4 opacity-0 group-hover:opacity-100 transition-opacity">
+ <div className="absolute top-4 right-4 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-within:opacity-100 transition-opacity">
📝 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
<div className="group relative p-2 border rounded-lg bg-muted w-fit">
<Image
src={imagePart}
alt="attachment"
width={300}
height={300}
className="max-w-xs max-h-64 rounded-md object-contain"
/>
<div className="absolute top-4 right-4 opacity-0 group-hover:opacity-100 transition-opacity">
<Button
variant="secondary"
size="sm"
className="h-8 px-2 shadow-md"
onClick={handlePlaceOnMap}
>
<MapPin size={14} className="mr-1" />
Place on Map
</Button>
</div>
</div>
<div className="group relative p-2 border rounded-lg bg-muted w-fit">
<Image
src={imagePart}
alt="attachment"
width={300}
height={300}
className="max-w-xs max-h-64 rounded-md object-contain"
/>
<div className="absolute top-4 right-4 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-within:opacity-100 transition-opacity">
<Button
variant="secondary"
size="sm"
className="h-8 px-2 shadow-md"
onClick={handlePlaceOnMap}
>
<MapPin size={14} className="mr-1" />
Place on Map
</Button>
</div>
</div>
🤖 Prompt for AI Agents
In `@components/user-message.tsx` around lines 82 - 101, The hover-only reveal for
the Place on Map button is not accessible on touch devices; update the wrapper
and button visibility so the control is reachable via keyboard and touch by
adding focus-based styles (e.g., include group-focus-within:opacity-100
alongside group-hover:opacity-100 on the absolute button container), ensure the
container is keyboard-focusable (add a tabindex or use a semantic button
wrapper) and confirm handlePlaceOnMap remains the click/keyboard activation
handler on the Button component (refer to the Button element with
onClick={handlePlaceOnMap} and the surrounding div using classes "group" and
"absolute top-4 right-4 ...").

)}
{textPart && <div className="text-xl break-words">{textPart}</div>}
Expand Down
2 changes: 1 addition & 1 deletion lib/actions/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export async function saveChat(chat: OldChatType, userId: string): Promise<strin
// return null;
// }

export async function updateDrawingContext(chatId: string, contextData: { drawnFeatures: any[], cameraState: any }) {
export async function updateDrawingContext(chatId: string, contextData: { drawnFeatures?: any[], imageOverlays?: any[], cameraState: any }) {

Choose a reason for hiding this comment

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

updateDrawingContext now accepts imageOverlays, but the signature still uses any[] and cameraState: any. While tsc passes, this is an unsafe-but-type-valid boundary: you’re persisting untyped blobs to the DB and later JSON.parse them into runtime structures.

Given this feature’s complexity (Mapbox sources/layers depend on coordinate ordering and URL), it’s worth strongly typing the persisted payload to prevent silent corruption.

Suggestion

Replace any with your shared types (MapData, ImageOverlay, CameraState) for the server action payload and validate minimally before writing.

Example:

import type { CameraState, ImageOverlay } from '@/components/map/map-data-context'

export async function updateDrawingContext(
  chatId: string,
  contextData: {
    drawnFeatures?: MapData['drawnFeatures']
    imageOverlays?: ImageOverlay[]
    cameraState: CameraState
  }
) { ... }

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

'use server';
console.log('[Action] updateDrawingContext called for chatId:', chatId);

Expand Down