Skip to content

Re-implementation of PR #533 (Full Features) #542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ngoiyaeric wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Re-implementation of PR #533 (Full Features) #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop

Large diffs are not rendered by default.

177 changes: 177 additions & 0 deletions app/api/elevation/route.ts
View file
Open in desktop
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;
const geometry = geometryParam ? JSON.parse(geometryParam) : null;

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 +24 to +41
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

geometry is JSON.parse(geometryParam) without a try/catch. A malformed geometry query will throw and result in a 500 instead of a controlled 400, unlike bounds which is validated.

Similarly, gridSize isn’t validated for NaN, negative, or huge values (which directly impacts request fan-out).

Suggestion

Validate inputs consistently:

  • Wrap geometry parsing in try/catch and return 400 on failure.
  • Validate gridSize: default to 20 when NaN, clamp to a maximum, and require >= 1.
  • Validate bounds is an array of 4 finite numbers and west<east, south<north.

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

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 });
}
}

const token = process.env.MAPBOX_ACCESS_TOKEN || process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;

// 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 +58 to +66
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

The access token is read from env, but if it’s missing/empty the code still issues requests like ...access_token=null, causing many failing external calls and slow responses. This should fail fast with a clear 500/400 response.

Additionally, you’re interpolating access_token directly into a URL string. It’s safer to construct using URL/URLSearchParams to avoid accidental injection/encoding issues.

Suggestion

Add an early guard:

  • If !token, return NextResponse.json({ error: 'Missing MAPBOX access token' }, { status: 500 }).
  • Build the request with const u = new URL(...); u.searchParams.set('access_token', token);.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this guard + safer URL construction.


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);

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,
});

Comment on lines +36 to +118
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

/api/elevation performs one Mapbox Tilequery request per grid point (worst case (gridSize+1)^2). With the default gridSize=20, that’s 441 external requests per request, and it scales quadratically. This is likely to hit Mapbox rate limits, slow responses significantly, and increase server costs.

Also, POST calls GET repeatedly (via GET(new NextRequest(url))) which multiplies the above per-feature; this can become explosive under batch usage.

Suggestion

Consider switching to a raster-based approach (Mapbox Terrain-RGB tiles) or otherwise drastically reducing external calls:

  • Prefer fetching a small set of terrain tiles and sampling pixels server-side.
  • Enforce hard limits: cap gridSize (e.g. max 30), cap number of points, and cap number of features in POST.
  • Add caching (e.g. unstable_cache/KV) keyed by {bounds, gridSize, geometryHash}.
  • If you keep tilequery, throttle concurrency (e.g. p-limit) and consider a single request per cell centroid at lower grid sizes.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing caps + concurrency limiting + basic caching hooks.

} 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 }
);
}
}

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 +131 to +160
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

In POST, you construct new URL(/api/elevation?${params}, req.url) and then call GET(new NextRequest(url)). This is an unusual pattern and can have subtle issues:

  • req.url includes the current path; using it as a base may produce unexpected results if the route is not at the same origin/path.
  • You lose the original request context (headers, geo, etc.).
  • You duplicate JSON parsing/validation behavior and create a tight coupling between handlers.

Prefer extracting the elevation computation into a shared function and calling it from both handlers.

Suggestion

Refactor to a shared internal function, e.g. computeElevation({ bounds, gridSize, geometry }), used by both GET and POST. POST can compute bbox/geometry then call the shared function directly (no synthetic NextRequest).

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


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
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions components/header-search-button.tsx
View file
Open in desktop
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)
Comment on lines 24 to 29
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

Several client components replaced useActions<typeof AI>() / useUIState<typeof AI>() with any.

Even if tsc passes, this is unsafe but type-valid and removes compile-time guarantees for action names/return shapes. It also increases the chance of runtime issues when actions change.

Given this PR already exports AIState/UIState types, the better fix is to reintroduce correct generic typing (or export a dedicated AppAI type) rather than widening to any.

Suggestion

Avoid any here by restoring the AI type binding:

  • Re-add import { AI } from '@/app/actions' and use useActions<typeof AI>() / useUIState<typeof AI>().
  • If circular deps were the reason, export a type AppAI = typeof AI from a non-RSC module or a types.ts file and import only the type.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit that fixes these to use proper generics (type-only imports to avoid runtime cycles).

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
View file
Open in desktop
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 +57 to +139
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

ElevationHeatmapLayer registers map event handlers (click, mouseenter, mouseleave) but cleanup only removes layers/sources. If the effect re-runs (e.g. points updated, style reload), you can accumulate handlers or leave handlers referencing removed layers.

Also, the handler functions are created inside the effect, but never deregistered via map.off.

Suggestion

Store handler references and remove them during cleanup:

  • map.off('click', pointsLayerId, clickHandler)
  • likewise for mouseenter/mouseleave handlers
  • Consider updating the source data when it already exists ((map.getSource(sourceId) as GeoJSONSource).setData(geojson)) rather than no-op when present.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit that adds proper event cleanup and updates existing sources on re-render.

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

return null
}
Loading