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
429 changes: 145 additions & 284 deletions app/actions.tsx

Large diffs are not rendered by default.

177 changes: 177 additions & 0 deletions app/api/elevation/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import { NextRequest, NextResponse } from 'next/server';
import * as turf from '@turf/turf';

/**
* GET /api/elevation
*
* Fetches elevation data for a grid of points within the given bounds.
* Uses Mapbox Terrain vector tileset to get elevation values.
*/
export async function GET(req: NextRequest) {
try {
const { searchParams } = new URL(req.url);
const boundsParam = searchParams.get('bounds');
const gridSizeParam = searchParams.get('gridSize');
const geometryParam = searchParams.get('geometry');

if (!boundsParam) {
return NextResponse.json(
{ error: 'Missing required parameter: bounds' },
{ status: 400 }
);
}

let bounds;
try {
bounds = JSON.parse(boundsParam);
} catch (e) {
return NextResponse.json({ error: 'Invalid bounds parameter' }, { status: 400 });
}

const gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20;
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

No upper-bound validation on gridSize — potential DoS vector.

A caller can pass gridSize=500, generating 501² = 251,001 Mapbox API calls from a single request. Add a reasonable cap (e.g., 50) and validate that gridSize is a positive integer.

🛡️ Proposed fix
-    const gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20;
+    const gridSize = Math.min(Math.max(gridSizeParam ? parseInt(gridSizeParam, 10) : 20, 1), 50);
📝 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 gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20;
const gridSize = Math.min(Math.max(gridSizeParam ? parseInt(gridSizeParam, 10) : 20, 1), 50);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` at line 31, Validate and cap gridSize after
parsing: replace the raw parseInt usage of gridSizeParam -> gridSize with logic
that ensures it's a positive integer and does not exceed a MAX (e.g., const
MAX_GRID_SIZE = 50). Use Number.isFinite/Number.isInteger on the parsed value
and if it's <=0 or NaN return a 400 error; if it exceeds MAX_GRID_SIZE either
clamp it to MAX_GRID_SIZE or return 400 per API policy (prefer clamping for
backward compatibility). Update references to gridSize in route.ts (where
gridSizeParam and gridSize are used) to rely on this validated/capped value.

const geometry = geometryParam ? JSON.parse(geometryParam) : null;
Comment on lines +31 to +32
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

Missing try/catch around JSON.parse(geometryParam) — will crash with an unhandled 500.

boundsParam parsing is correctly wrapped in a try/catch (Lines 25-29), but geometryParam parsing on Line 32 is not. A malformed geometry query string will throw and be caught by the outer catch, returning a generic 500 instead of a descriptive 400.

🛡️ Proposed fix
-    const geometry = geometryParam ? JSON.parse(geometryParam) : null;
+    let geometry = null;
+    if (geometryParam) {
+      try {
+        geometry = JSON.parse(geometryParam);
+      } catch (e) {
+        return NextResponse.json({ error: 'Invalid geometry parameter' }, { status: 400 });
+      }
+    }
📝 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 gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20;
const geometry = geometryParam ? JSON.parse(geometryParam) : null;
const gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20;
let geometry = null;
if (geometryParam) {
try {
geometry = JSON.parse(geometryParam);
} catch (e) {
return NextResponse.json({ error: 'Invalid geometry parameter' }, { status: 400 });
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 31 - 32, Wrap the JSON.parse call
for geometryParam in a try/catch (same pattern as boundsParam) so a malformed
geometry query returns a 400 with a descriptive error instead of throwing a 500;
specifically, update the logic around geometryParam and the geometry variable in
route handler (where geometryParam is parsed into geometry) to catch SyntaxError
from JSON.parse and call return new NextResponse("Invalid geometry parameter", {
status: 400 }) (or the project’s existing bad-request response helper).


const [west, south, east, north] = bounds;

const points: Array<{ lng: number; lat: number; elevation: number | null }> = [];
const lonStep = (east - west) / gridSize;
const latStep = (north - south) / gridSize;

const polygon = geometry ? turf.polygon(geometry.coordinates) : null;

Comment on lines +31 to +41

Choose a reason for hiding this comment

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

geometryParam is parsed without a try/catch. If the query contains invalid JSON (which is plausible given the current client-side URL construction), this will throw and return a 500 rather than a 400.

Separately, turf.polygon(geometry.coordinates) assumes the geometry is a Polygon and that coordinates is shaped correctly. If geometry.type is not Polygon or coordinates are malformed, this will also throw and bubble to the outer catch (500).

Suggestion

Harden request parsing:

  • Wrap JSON.parse(geometryParam) in a try/catch and return 400 on failure.
  • Validate geometry.type === 'Polygon' before calling turf.polygon(...).

Reply with "@CharlieHelps yes please" if you'd like me to add a commit adding defensive parsing/validation and consistent 400 responses.

for (let i = 0; i <= gridSize; i++) {
for (let j = 0; j <= gridSize; j++) {
const lng = west + (lonStep * i);
const lat = south + (latStep * j);

if (polygon) {
const point = turf.point([lng, lat]);
if (!turf.booleanPointInPolygon(point, polygon)) {
continue;
}
}

points.push({ lng, lat, elevation: null });
}
}
Comment on lines +42 to +56
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

Unbounded concurrent HTTP requests to Mapbox will hit rate limits and cause failures.

With the default gridSize=20, the grid generates (20+1)² = 441 points, each firing a separate fetch to the Mapbox Tilequery API via Promise.all (Line 88) with zero concurrency control. The Mapbox Tilequery API has rate limits (typically 600 requests/minute on standard plans). A single call to this endpoint nearly exhausts that budget, and concurrent users will easily exceed it — leading to 429s, dropped data points, and degraded UX.

🐛 Proposed fix — batch with concurrency limit

Use a concurrency-limited approach (e.g., a simple chunked Promise.all or a library like p-limit):

+import pLimit from 'p-limit';

+const limit = pLimit(10); // max 10 concurrent requests

-const elevationPromises = points.map(async (point) => {
+const elevationPromises = points.map((point) => limit(async () => {
     try {
       const response = await fetch(
         `https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2/tilequery/${point.lng},${point.lat}.json?access_token=${token}`
       );
       // ... rest unchanged
     } catch (error) {
       console.error(`Error fetching elevation for ${point.lng},${point.lat}:`, error);
     }
     return point;
-});
+}));

Alternatively, consider using the Mapbox Terrain-RGB raster tiles directly (fewer requests by fetching tile images and decoding pixel values) instead of one tilequery per point.

Also applies to: 62-86, 88-88

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 42 - 56, The code currently builds a
large points array in route.ts (points from the nested loops that push { lng,
lat, elevation: null }) and then fires unbounded concurrent fetches to the
Mapbox Tilequery API (the Promise.all at the fetch stage), which will hit rate
limits; update the fetch phase to use a concurrency-limited strategy (e.g.,
process points in chunks or use a concurrency utility like p-limit) so only N
requests run concurrently (choose a safe default like 5–10), and replace the
direct Promise.all over all points with batched Promise.all calls or a p-limit
wrapped fetch; ensure you reference the points array and the function or inline
block that performs the Tilequery fetch to apply the concurrency control and
handle/retry 429 responses gracefully.


const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
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

Mapbox access token fallback and missing-token check.

If neither MAPBOX_ACCESS_TOKEN nor NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN is set, token will be undefined, and all Mapbox requests will fail silently (caught by the per-point catch). Return an early 500 if the token is missing to surface the misconfiguration clearly.

🛡️ Proposed fix
     const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
+    if (!token) {
+      return NextResponse.json({ error: 'Mapbox access token not configured' }, { status: 500 });
+    }
📝 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 token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;
if (!token) {
return NextResponse.json({ error: 'Mapbox access token not configured' }, { status: 500 });
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` at line 58, Check for a missing Mapbox token
immediately after the token is resolved (the const token = ... line) and return
an early 500 response if token is undefined; update the API handler in route.ts
to validate token before making any per-point Mapbox requests (so
functions/blocks that use token for fetches don’t run), and send a clear error
message stating the Mapbox access token is not configured.


// Fetch elevation data using Mapbox Terrain vector tiles (v2)
// This tileset contains contour lines with 'ele' property
const elevationPromises = points.map(async (point) => {
try {
const response = await fetch(
`https://api.mapbox.com/v4/mapbox.mapbox-terrain-v2/tilequery/${point.lng},${point.lat}.json?access_token=${token}`
);
Comment on lines +31 to +66

Choose a reason for hiding this comment

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

This endpoint allows unbounded gridSize from a query param, which can be abused to trigger massive fan-out to Mapbox and tie up server resources (DoS-by-proxy). Even with concurrency limiting, you should validate input and enforce ceilings.

Also, if MAPBOX_ACCESS_TOKEN is missing, the endpoint still attempts requests; better to fail fast with a clear 500/503 error.

Suggestion

Add server-side validation and fail-fast checks:

  • Clamp or reject gridSize outside a safe range.
  • Validate bounds array length and numeric ranges.
  • If no Mapbox token is configured, return an explicit error response.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing validation + clearer error handling.


if (response.ok) {
const data = await response.json();
if (data.features && data.features.length > 0) {
// Find the feature with the highest elevation in this point's vicinity
// or just take the first one if it has 'ele'
const elevations = data.features
.map((f: any) => f.properties.ele)
.filter((e: any) => e !== undefined);

if (elevations.length > 0) {
point.elevation = Math.max(...elevations);
}
}
}
} catch (error) {
console.error(`Error fetching elevation for ${point.lng},${point.lat}:`, error);
}
return point;
});

const elevationData = await Promise.all(elevationPromises);
const validPoints = elevationData.filter(p => p.elevation !== null);
Comment on lines +36 to +89

Choose a reason for hiding this comment

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

This endpoint fires one Mapbox Tilequery request per grid point ((gridSize+1)^2). With the default gridSize=20, that’s 441 external requests per call, all launched concurrently via Promise.all. That’s a major reliability and cost issue:

  • High chance of hitting Mapbox rate limits / request bursts.
  • Large fan-out can starve server resources.
  • Latency will be dominated by the slowest requests.

This needs concurrency limiting and/or a different sampling strategy (vector tile fetch, coarser grid, adaptive sampling, or batch where possible).

Suggestion

Add a concurrency limiter (e.g. p-limit) around the per-point fetches and consider capping gridSize server-side. Example approach:

  • Validate gridSize to a safe range (e.g. 5..30) and reject/clip larger values.
  • Use pLimit(8) (or similar) to process points with bounded concurrency.
  • Optionally add caching keyed by (bounds, gridSize, geometryHash) to avoid repeated fan-out.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with concurrency limiting + gridSize validation.


if (validPoints.length === 0) {
return NextResponse.json({
success: true,
points: [],
statistics: { min: 0, max: 0, average: 0, count: 0 },
bounds,
gridSize,
});
}

const elevations = validPoints.map(p => p.elevation as number);
const minElevation = Math.min(...elevations);
const maxElevation = Math.max(...elevations);
const avgElevation = elevations.reduce((sum, e) => sum + e, 0) / elevations.length;

return NextResponse.json({
success: true,
points: validPoints,
statistics: {
min: minElevation,
max: maxElevation,
average: Math.round(avgElevation * 10) / 10,
count: validPoints.length,
},
bounds,
gridSize,
});

} catch (error) {
console.error('Error fetching elevation data:', error);
return NextResponse.json(
{
error: 'Failed to fetch elevation data',
details: error instanceof Error ? error.message : 'Unknown error'
},
{ status: 500 }
);
}
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.

🧹 Nitpick | 🔵 Trivial

Avoid leaking internal error details to the client.

Line 124 sends error.message in the response, which could expose internal file paths, stack traces, or dependency details. Return a generic message instead.

♻️ Proposed fix
       {
         error: 'Failed to fetch elevation data',
-        details: error instanceof Error ? error.message : 'Unknown error'
       },

The same applies to the POST handler at Line 172.

📝 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
} catch (error) {
console.error('Error fetching elevation data:', error);
return NextResponse.json(
{
error: 'Failed to fetch elevation data',
details: error instanceof Error ? error.message : 'Unknown error'
},
{ status: 500 }
);
}
} catch (error) {
console.error('Error fetching elevation data:', error);
return NextResponse.json(
{
error: 'Failed to fetch elevation data'
},
{ status: 500 }
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 119 - 128, The catch blocks in the
GET and POST handlers (the try/catch surrounding the elevation fetch/processing
in route.ts) currently return error instanceof Error ? error.message to the
client; change these to return a generic error message (e.g. "Internal server
error" or "Failed to fetch elevation data") instead while keeping the detailed
error logged server-side via console.error(error). Update both the GET catch and
the POST catch so NextResponse.json responses do not include error.message or
other internal details and still return status: 500.

}

export async function POST(req: NextRequest) {
try {
const body = await req.json();
const { features, gridSize = 20 } = body;

if (!features || !Array.isArray(features)) {
return NextResponse.json(
{ error: 'Missing or invalid features array' },
{ status: 400 }
);
}

const results = await Promise.all(
features.map(async (feature: any) => {
if (feature.geometry.type !== 'Polygon') {
return null;
}

const bbox = turf.bbox(feature);
const params = new URLSearchParams({
bounds: JSON.stringify(bbox),
gridSize: gridSize.toString(),
geometry: JSON.stringify(feature.geometry),
});

const url = new URL(`/api/elevation?${params}`, req.url);
const response = await GET(new NextRequest(url));
return await response.json();
})
Comment on lines +143 to +159
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

POST handler: missing null-check on feature.geometry and no token check.

Line 145 accesses feature.geometry.type without verifying feature.geometry exists, which will throw a TypeError for malformed input. Additionally, the POST handler constructs internal GET requests but doesn't share the token validation — if the token is missing, each internal GET silently fails.

🛡️ Proposed fix
       features.map(async (feature: any) => {
-        if (feature.geometry.type !== 'Polygon') {
+        if (!feature.geometry || feature.geometry.type !== 'Polygon') {
           return null;
         }
📝 Committable suggestion

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

Suggested change
const results = await Promise.all(
features.map(async (feature: any) => {
if (feature.geometry.type !== 'Polygon') {
return null;
}
const bbox = turf.bbox(feature);
const params = new URLSearchParams({
bounds: JSON.stringify(bbox),
gridSize: gridSize.toString(),
geometry: JSON.stringify(feature.geometry),
});
const url = new URL(`/api/elevation?${params}`, req.url);
const response = await GET(new NextRequest(url));
return await response.json();
})
const results = await Promise.all(
features.map(async (feature: any) => {
if (!feature.geometry || feature.geometry.type !== 'Polygon') {
return null;
}
const bbox = turf.bbox(feature);
const params = new URLSearchParams({
bounds: JSON.stringify(bbox),
gridSize: gridSize.toString(),
geometry: JSON.stringify(feature.geometry),
});
const url = new URL(`/api/elevation?${params}`, req.url);
const response = await GET(new NextRequest(url));
return await response.json();
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/elevation/route.ts` around lines 143 - 159, The POST handler's loop
assumes feature.geometry exists and constructs internal GETs without
passing/validating the auth token; add a guard in the features.map callback to
return null if !feature || !feature.geometry or if feature.geometry.type is
missing before accessing .type, and ensure the internal request to GET (created
with new NextRequest and URL in this block) includes the same
authorization/token check used by the outer POST handler (either validate the
token up front and abort if missing, or attach the token header/query param to
the URL/NextRequest) so internal GETs don't silently fail; update the code that
builds params/const url and new NextRequest to reuse the POST handler's token
validation and include token in the internal request.

);
Comment on lines +131 to +160

Choose a reason for hiding this comment

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

POST /api/elevation calls GET by constructing a new NextRequest(url) and then calling GET(new NextRequest(url)). This is an unusual pattern and can behave differently than a direct function call because it creates a synthetic request (and may not carry headers, cookies, IP, etc.). It also makes error handling and status codes harder to manage because you always call response.json() without checking response.ok.

If you want an internal helper, extract the shared logic into a pure function (e.g., queryElevation({bounds, gridSize, geometry})) and call it from both handlers.

Suggestion

Refactor shared logic into a helper and avoid synthetic NextRequest objects:

async function queryElevation({ bounds, gridSize, geometry }: { bounds: number[]; gridSize: number; geometry?: any }) {
  // existing GET logic, but return a plain object
}

export async function GET(req: NextRequest) {
  // parse params; return NextResponse.json(await queryElevation(...))
}

export async function POST(req: NextRequest) {
  // parse body; call queryElevation per feature
  // ensure you propagate per-item errors/statuses appropriately
}

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


return NextResponse.json({
success: true,
results: results.filter(r => r !== null),
});

} catch (error) {
console.error('Error in batch elevation query:', error);
return NextResponse.json(
{
error: 'Failed to process batch elevation query',
details: error instanceof Error ? error.message : 'Unknown error'
},
{ status: 500 }
);
}
}
23 changes: 2 additions & 21 deletions app/layout.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Metadata, Viewport } from 'next'
import Script from 'next/script'
import { Inter as FontSans, Poppins } from 'next/font/google'
import './globals.css'
import 'katex/dist/katex.min.css';
Expand Down Expand Up @@ -66,27 +67,7 @@ export default function RootLayout({
return (
<html lang="en" suppressHydrationWarning>
<head>
<script
dangerouslySetInnerHTML={{
__html: `
(function() {
const htmxEvents = [
'sseError', 'sseOpen', 'swapError', 'targetError', 'timeout',
'validation:validate', 'validation:failed', 'validation:halted',
'xhr:abort', 'xhr:loadend', 'xhr:loadstart'
];
htmxEvents.forEach(event => {
const funcName = 'func ' + event;
if (typeof window[funcName] === 'undefined') {
window[funcName] = function() {
console.warn('HTMX event handler "' + funcName + '" was called but not defined. Providing safety fallback.');
};
}
});
})();
`,
}}
/>
<Script src="/htmx-fallback.js" strategy="beforeInteractive" />
</head>
<body
className={cn(
Expand Down
9 changes: 9 additions & 0 deletions app_actions_final_fix.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<<<<<<< SEARCH
import dynamic from 'next/dynamic'
import { ResolutionCarousel } from '@/components/resolution-carousel'

const ElevationHeatmapLayer = dynamic(() => import('@/components/map/elevation-heatmap-layer').then(mod => mod.ElevationHeatmapLayer), { ssr: false })
=======
import { ElevationHeatmapLayer } from '@/components/map/elevation-heatmap-layer'
import { ResolutionCarousel } from '@/components/resolution-carousel'
>>>>>>> REPLACE
Comment on lines +1 to +9
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

Development artifact committed to repository — should be removed.

app_actions_final_fix.patch is an aider-style SEARCH/REPLACE patch file generated during development. It's already been applied (the final app/actions.tsx imports MapResultsContainer rather than ElevationHeatmapLayer directly), so this file serves no further purpose and adds noise to the repository.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app_actions_final_fix.patch` around lines 1 - 9, This file is a leftover
SEARCH/REPLACE patch (contains conflict markers and duplicate imports for
ElevationHeatmapLayer and ResolutionCarousel); delete the file
app_actions_final_fix.patch from the commit/repo (or revert the commit that
added it) so the repository no longer contains the development artifact, and
verify no other files contain the same SEARCH/REPLACE/<<<<<<< markers
referencing ElevationHeatmapLayer or ResolutionCarousel.

6 changes: 3 additions & 3 deletions components/header-search-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Button } from '@/components/ui/button'
import { Search } from 'lucide-react'
import { useMap } from './map/map-context'
import { useActions, useUIState } from 'ai/rsc'
import { AI } from '@/app/actions'

import { nanoid } from '@/lib/utils'
import { UserMessage } from './user-message'
import { toast } from 'sonner'
Expand All @@ -24,8 +24,8 @@ export function HeaderSearchButton() {
const { mapProvider } = useSettingsStore()
const { mapData } = useMapData()
// Cast the actions to our defined interface to avoid build errors
const actions = useActions<typeof AI>() as unknown as HeaderActions
const [, setMessages] = useUIState<typeof AI>()
const actions = useActions<any>() as unknown as HeaderActions
const [, setMessages] = useUIState<any>()
const [isAnalyzing, setIsAnalyzing] = useState(false)
const [desktopPortal, setDesktopPortal] = useState<HTMLElement | null>(null)
const [mobilePortal, setMobilePortal] = useState<HTMLElement | null>(null)
Expand Down
143 changes: 143 additions & 0 deletions components/map/elevation-heatmap-layer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
'use client'

import { useEffect, useState } from 'react'
import { useMap } from './map-context'

interface ElevationPoint {
lng: number;
lat: number;
elevation: number;
}

interface ElevationHeatmapLayerProps {
id: string;
points: ElevationPoint[];
statistics?: {
min: number;
max: number;
average: number;
count: number;
};
}

export function ElevationHeatmapLayer({ id, points, statistics }: ElevationHeatmapLayerProps) {
const { map } = useMap()
const [mapboxgl, setMapboxgl] = useState<any>(null)

useEffect(() => {
import('mapbox-gl').then(mod => {
setMapboxgl(mod.default)
})
}, [])

useEffect(() => {
if (!map || !points || points.length === 0 || !mapboxgl) return

const sourceId = `elevation-heatmap-source-${id}`
const heatmapLayerId = `elevation-heatmap-layer-${id}`
const pointsLayerId = `elevation-points-layer-${id}`

const geojson: any = {
type: 'FeatureCollection',
features: points.map(point => ({
type: 'Feature',
geometry: {
type: 'Point',
coordinates: [point.lng, point.lat]
},
properties: {
elevation: point.elevation,
intensity: statistics && statistics.max !== statistics.min
? (point.elevation - statistics.min) / (statistics.max - statistics.min)
: 0.5
}
}))
}

const onMapLoad = () => {
if (!map.getSource(sourceId)) {
map.addSource(sourceId, {
type: 'geojson',
data: geojson
})

map.addLayer({
id: heatmapLayerId,
type: 'heatmap',
source: sourceId,
paint: {
'heatmap-weight': ['interpolate', ['linear'], ['get', 'intensity'], 0, 0, 1, 1],
'heatmap-intensity': ['interpolate', ['linear'], ['zoom'], 0, 1, 15, 3],
'heatmap-color': [
'interpolate',
['linear'],
['heatmap-density'],
0, 'rgba(33,102,172,0)',
0.2, 'rgb(103,169,207)',
0.4, 'rgb(209,229,240)',
0.6, 'rgb(253,219,199)',
0.8, 'rgb(239,138,98)',
1, 'rgb(178,24,43)'
],
'heatmap-radius': ['interpolate', ['linear'], ['zoom'], 0, 2, 15, 20],
'heatmap-opacity': ['interpolate', ['linear'], ['zoom'], 7, 0.7, 15, 0.5]
}
})

map.addLayer({
id: pointsLayerId,
type: 'circle',
source: sourceId,
minzoom: 14,
paint: {
'circle-radius': ['interpolate', ['linear'], ['zoom'], 14, 3, 22, 8],
'circle-color': [
'interpolate',
['linear'],
['get', 'intensity'],
0, '#2166ac',
0.25, '#67a9cf',
0.5, '#f7f7f7',
0.75, '#ef8a62',
1, '#b2182b'
],
'circle-stroke-color': 'white',
'circle-stroke-width': 1,
'circle-opacity': ['interpolate', ['linear'], ['zoom'], 14, 0, 15, 0.8]
}
})

const clickHandler = (e: any) => {
if (!e.features || e.features.length === 0) return
const elevation = e.features[0].properties?.elevation
if (elevation !== undefined) {
new mapboxgl.Popup()
.setLngLat(e.lngLat)
.setHTML(`<strong>Elevation:</strong> ${elevation}m`)
.addTo(map)
}
}

map.on('click', pointsLayerId, clickHandler)
map.on('mouseenter', pointsLayerId, () => { map.getCanvas().style.cursor = 'pointer' })
map.on('mouseleave', pointsLayerId, () => { map.getCanvas().style.cursor = '' })
}
}

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

return () => {
if (map) {
if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}
Comment on lines +133 to +139
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

Cleanup lacks isStyleLoaded() guard — may throw if map style is unloaded.

The existing GeoJsonLayer component (see components/map/geojson-layer.tsx, Lines 90-97) wraps removeLayer/removeSource in an if (map.isStyleLoaded()) check. Without this guard, calling removeLayer/removeSource on an already-disposed map or during a style change can throw.

🛡️ Proposed fix
     return () => {
-      if (map) {
+      if (map && map.isStyleLoaded()) {
         if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
         if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
         if (map.getSource(sourceId)) map.removeSource(sourceId)
       }
     }
📝 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
return () => {
if (map) {
if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}
return () => {
if (map && map.isStyleLoaded()) {
if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
if (map.getSource(sourceId)) map.removeSource(sourceId)
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/map/elevation-heatmap-layer.tsx` around lines 183 - 189, The
cleanup currently removes layers/sources directly and can throw if the map style
is unloaded; update the return cleanup in elevation-heatmap-layer.tsx to first
check map && map.isStyleLoaded() before calling
map.getLayer(pointsLayerId)/getLayer(heatmapLayerId)/getSource(sourceId) and
removing them; ensure you keep the same checks for existence (map.getLayer(...)
/ map.getSource(...)) but only run them when map.isStyleLoaded() returns true to
mirror the pattern used in GeoJsonLayer.

Comment on lines +127 to +139
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

map.once('load', onMapLoad) is never deregistered — orphaned layers on early unmount.

When map.isStyleLoaded() is false, onMapLoad is queued via map.once('load', ...) (line 130). If the component unmounts (or the effect re-runs) before the load event fires, the cleanup on lines 133–139 executes with no layers/source to remove, then later onMapLoad fires and adds source + layers that are never cleaned up because no further cleanup will run.

Fix: cancel the pending listener at the top of cleanup, and add the missing isStyleLoaded() guard (previously flagged) in the same pass.

🐛 Proposed fix
     return () => {
-      if (map) {
+      map.off('load', onMapLoad) // cancel pending once-listener before it fires
+      if (map && map.isStyleLoaded()) {
         if (map.getLayer(pointsLayerId)) map.removeLayer(pointsLayerId)
         if (map.getLayer(heatmapLayerId)) map.removeLayer(heatmapLayerId)
         if (map.getSource(sourceId)) map.removeSource(sourceId)
       }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/map/elevation-heatmap-layer.tsx` around lines 127 - 139, The
cleanup currently removes layers/sources but does not deregister the pending
listener added with map.once('load', onMapLoad'), which can cause orphaned
layers if the effect unmounts before the load event; update the effect cleanup
to call map.off('load', onMapLoad) (or cancel the once listener) to remove the
queued handler and also guard removals with map.isStyleLoaded() before removing
layers/sources; locate the listener registration and onMapLoad function in
elevation-heatmap-layer.tsx and add the deregistration plus the isStyleLoaded()
check in the returned cleanup block.

}, [map, id, points, statistics, mapboxgl])

return null
}
Loading