Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactored core state management with new public UIState and AIState types, added server-side user handling for chat persistence, introduced elevation API routes for terrain data, and created new map visualization components (ElevationHeatmapLayer, MapResultsContainer). Simplified component type constraints across multiple files. Added elevation utility functions and external HTMX fallback script. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as Client (Browser)
participant Server as Server (Actions)
participant ElevAPI as Elevation API
participant Mapbox as Mapbox Terrain v2
participant MapUI as Map Component
User->>Client: Request with elevation data
Client->>Server: Call AI action with bounds/gridSize
Server->>ElevAPI: POST /api/elevation with features
ElevAPI->>Mapbox: Query terrain RGB for grid points
Mapbox-->>ElevAPI: Return elevation data per point
ElevAPI-->>Server: Return aggregated elevation statistics
Server->>Server: Attach elevation data to AIState
Server-->>Client: Stream UIState with MapResultsContainer
Client->>MapUI: Render ElevationHeatmapLayer
MapUI->>Mapbox: Add heatmap source & layers
Mapbox-->>MapUI: Render elevation visualization
MapUI-->>User: Display elevation heat map
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
This re-implementation adds the elevation heatmap feature, but there are several high-impact problems: the elevation API currently fans out to hundreds of external requests per call (likely rate-limit/latency failures), chat persistence was simplified in a way that can break invariants and save data under 'anonymous', and getUIStateFromAIState now drops user messages (likely a UI regression when loading chats). Additionally, elevation fetch URLs are built via raw string concatenation without encoding, and some client components removed type safety by switching useActions/useUIState to any. These should be addressed before merging to avoid regressions and production instability.
Additional notes (4)
- Maintainability |
components/header-search-button.tsx:12-12
Several components replaceduseActions<typeof AI>()/useUIState<typeof AI>()withany. This removes type safety around action names and return shapes and makes it easy to ship runtime errors (e.g. calling a missing action, or assumingsubmitreturns a certain UI state shape).
Given this PR also changes UIState/AIState shapes, this is the worst time to drop types.
-
Maintainability |
components/mobile-icons-bar.tsx:26-32
useActions<any>()/useUIState<any>()repeats the same type-safety regression. This makes it easy to silently breakclearChat/submitinteractions. -
Maintainability |
components/resolution-carousel.tsx:17-17
useActions<any>()/useUIState<any>()again—this spreadsanythrough core user flows.
Also, actions is cast to any anyway; consider defining a minimal ResolutionActions interface (like you did in header-search-button) and using that instead of any to at least constrain the surface area.
- Maintainability |
components/search-related.tsx:22-22
SearchRelatednow typessetMessagescallback arg asany, which is another type-safety drop and makes it easy to break message rendering.
This component is part of the core chat loop; it should keep the UI state type consistent with UIState from app/actions.tsx.
Summary of changes
Summary
This PR re-introduces elevation heatmap support and refactors map result rendering.
Key changes
-
Server actions / chat flow (
app/actions.tsx)- Adds
location+timezoneparsing fromFormData. - Swaps
GeoJsonLayerrendering for a newMapResultsContainerthat can render both GeoJSON overlays and elevation heatmaps. - Fetches elevation points from a new endpoint (
/api/elevation) whenanalysisResult.elevationData.requestedis set. - Changes chat persistence behavior:
- Uses
getCurrentUserIdOnServer()in more places. - Simplifies
onSetAIStatetosaveChat({ id, messages }, userId).
- Uses
- Refactors
getUIStateFromAIStateto primarily render assistant messages bytypeand skipuser/systemroles.
- Adds
-
New elevation API (
app/api/elevation/route.ts)- Adds a GET endpoint that samples a grid of points within a bbox and queries Mapbox Terrain tilequery for
ele. - Adds a POST endpoint for batch elevation queries over polygon features.
- Adds a GET endpoint that samples a grid of points within a bbox and queries Mapbox Terrain tilequery for
-
New map UI pieces
components/map/elevation-heatmap-layer.tsx: Mapbox GL heatmap + point layer with popups.components/map/map-results-container.tsx: Wrapper that conditionally renders GeoJSON + elevation layers.
-
Script loading (
app/layout.tsx,public/htmx-fallback.js)- Replaces inline
dangerouslySetInnerHTMLwithnext/scriptloading a static fallback file.
- Replaces inline
-
Resolution search schema/tooling (
lib/agents/resolution-search.tsx)- Extends schema with
elevationDatainstructions (bounds/gridSize/requested).
- Extends schema with
-
Geospatial tool config (
lib/agents/tools/geospatial.tsx)- Removes optional
mapbox_mcp_config.jsonloading and uses a simple fallback config.
- Removes optional
-
Client hooks typing
- Several components switch
useActions<typeof AI>()/useUIState<typeof AI>()toany.
- Several components switch
| // Handle elevation heat map if requested | ||
| let elevationPointsData = null; | ||
| if (analysisResult.elevationData?.requested && analysisResult.elevationData.bounds) { | ||
| try { | ||
| const elevationResponse = await fetch( | ||
| `${process.env.NEXT_PUBLIC_BASE_URL || 'http://localhost:3000'}/api/elevation?` + | ||
| `bounds=${JSON.stringify(analysisResult.elevationData.bounds)}&gridSize=${analysisResult.elevationData.gridSize || 20}${ | ||
| drawnFeatures.length > 0 && drawnFeatures[0].geometry | ||
| ? `&geometry=${JSON.stringify(drawnFeatures[0].geometry)}` | ||
| : '' | ||
| }` | ||
| ); | ||
|
|
||
| if (elevationResponse.ok) { | ||
| const elevationData = await elevationResponse.json(); | ||
| if (elevationData.success && elevationData.points.length > 0) { | ||
| elevationPointsData = elevationData; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Error fetching elevation data:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
The elevation request is built by concatenating JSON.stringify(...) into the querystring. This will routinely break due to unescaped characters ([, ], {, }, quotes) and can also easily exceed typical URL length limits for larger geometries. It’s also using NEXT_PUBLIC_BASE_URL || 'http://localhost:3000', which is brittle in server actions (multi-env, previews, proxies).
This is a correctness issue: a large polygon or even just the bbox JSON can yield a 414/400 and silently skip heatmap rendering.
Suggestion
Use URLSearchParams (or new URL()) to properly encode parameters, and strongly consider switching this call to POST /api/elevation with a JSON body to avoid URL length limits and encoding pitfalls.
Example:
const url = new URL('/api/elevation', process.env.NEXT_PUBLIC_BASE_URL)
url.searchParams.set('gridSize', String(analysisResult.elevationData.gridSize ?? 20))
url.searchParams.set('bounds', JSON.stringify(analysisResult.elevationData.bounds))
if (drawnFeatures[0]?.geometry) url.searchParams.set('geometry', JSON.stringify(drawnFeatures[0].geometry))
const elevationResponse = await fetch(url)Or better:
const elevationResponse = await fetch(`${baseUrl}/api/elevation`, {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ features: drawnFeatures.map(f => ({ geometry: f.geometry })), gridSize })
})Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing this safely (including a server-side base URL helper).
| export function getUIStateFromAIState(aiState: Chat) { | ||
| return aiState.messages | ||
| .map((message, index) => { | ||
| const { role, content, id, type, name } = message | ||
|
|
||
| if ( | ||
| !type || | ||
| type === 'end' || | ||
| (isSharePage && type === 'related') || | ||
| (isSharePage && type === 'followup') | ||
| ) | ||
| if (role === 'user' || role === 'system') { | ||
| return null | ||
| } | ||
|
|
There was a problem hiding this comment.
getUIStateFromAIState now drops all role === 'user' messages. That’s a behavioral change from the previous implementation where user inputs were rendered into the UI state. If Chat.messages includes user messages (likely), then loading an existing chat would show only assistant messages, losing the conversational context in the UI.
This is a user-visible regression unless another component reconstructs user messages elsewhere.
Suggestion
Keep user messages in the UI state mapping (as before), and only skip system (and maybe internal-only message types like end). For example:
if (role === 'system' || type === 'end') return null
if (role === 'user') {
return { id, component: <UserMessage content={parsedUserContent} chatId={chatId} showShare={index === 0} /> }
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit that restores user-message rendering while preserving your new map/elevation rendering paths.
| onSetAIState: async ({ state, done }) => { | ||
| 'use server' | ||
|
|
||
| const aiState = getAIState() as AIState | ||
| if (aiState) { | ||
| const uiState = getUIStateFromAIState(aiState) | ||
| return uiState | ||
| } | ||
| return initialUIState | ||
| }, | ||
| onSetAIState: async ({ state }) => { | ||
| 'use server' | ||
|
|
||
| if (!state.messages.some(e => e.type === 'response')) { | ||
| return | ||
| } | ||
|
|
||
| const { chatId, messages } = state | ||
| const createdAt = new Date() | ||
| const path = `/search/${chatId}` | ||
|
|
||
| let title = 'Untitled Chat' | ||
| if (messages.length > 0) { | ||
| const firstMessageContent = messages[0].content | ||
| if (typeof firstMessageContent === 'string') { | ||
| try { | ||
| const parsedContent = JSON.parse(firstMessageContent) | ||
| title = parsedContent.input?.substring(0, 100) || 'Untitled Chat' | ||
| } catch (e) { | ||
| title = firstMessageContent.substring(0, 100) | ||
| } | ||
| } else if (Array.isArray(firstMessageContent)) { | ||
| const textPart = ( | ||
| firstMessageContent as { type: string; text?: string }[] | ||
| ).find(p => p.type === 'text') | ||
| title = | ||
| textPart && textPart.text | ||
| ? textPart.text.substring(0, 100) | ||
| : 'Image Message' | ||
| } | ||
| } | ||
|
|
||
| const updatedMessages: AIMessage[] = [ | ||
| ...messages, | ||
| { | ||
| id: nanoid(), | ||
| role: 'assistant', | ||
| content: `end`, | ||
| type: 'end' | ||
| } | ||
| ] | ||
|
|
||
| const { getCurrentUserIdOnServer } = await import( | ||
| '@/lib/auth/get-current-user' | ||
| ) | ||
| const actualUserId = await getCurrentUserIdOnServer() | ||
|
|
||
| if (!actualUserId) { | ||
| console.error('onSetAIState: User not authenticated. Chat not saved.') | ||
| return | ||
| } | ||
|
|
||
| const chat: Chat = { | ||
| id: chatId, | ||
| createdAt, | ||
| userId: actualUserId, | ||
| path, | ||
| title, | ||
| messages: updatedMessages | ||
| if (done) { | ||
| const userId = (await getCurrentUserIdOnServer()) || 'anonymous' | ||
| const { chatId, messages } = state as AIState | ||
| saveChat({ id: chatId, messages } as any, userId).catch(e => | ||
| console.error('Failed to save chat:', e) | ||
| ) | ||
| } | ||
| await saveChat(chat, actualUserId) | ||
| } |
There was a problem hiding this comment.
onSetAIState now saves { id: chatId, messages } as any and uses userId || 'anonymous'. This is a large change from the previous logic which:
- appended an
endmarker, - computed
title,createdAt,path, - refused to save if unauthenticated.
Two concrete risks visible here:
- Persisting chats under
'anonymous'can mix multiple anonymous sessions together depending on how storage is keyed, and may violate expected access control semantics. - Saving a partial
Chatshape viaas anybypasses invariants (title/path/createdAt), leading to inconsistent downstream behavior when listing chats/search pages expect those fields.
Suggestion
Avoid as any and restore/centralize the chat persistence contract:
- Either require authentication before saving (return early if no user), or generate a per-session anonymous user id.
- Build a full
Chatobject includingcreatedAt,path, and a reasonabletitle. - If you intentionally changed the persistence model, update
saveChat’s signature to accept the new shape instead of casting.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit that restores the previous persistence invariants while keeping your new onSetAIState(done) gating.
| 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}` | ||
| ); | ||
|
|
||
| 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); | ||
|
|
There was a problem hiding this comment.
The elevation API performs a Mapbox Tilequery request for every grid point concurrently via Promise.all(points.map(...)). With the default (gridSize+1)^2 points, gridSize=20 yields 441 outbound requests per API call, which is very likely to trigger rate limits and will also be slow/expensive. Additionally, there’s no guard if MAPBOX_ACCESS_TOKEN is missing—requests will still be attempted with access_token=null/undefined.
This endpoint will be unstable under real traffic and can cause cascading latency issues.
Suggestion
Add (1) a required-token check, and (2) concurrency limiting + sane caps:
- Reject or return
{ success: false }with 500/503 if token is missing. - Cap
gridSize(e.g.<= 30) and/or total points. - Limit concurrency with a pool (e.g.
p-limit) and consider caching results by rounded lat/lng.
Sketch:
if (!token) return NextResponse.json({ error: 'Mapbox token missing' }, { status: 500 })
const gridSize = Math.min(parsedGridSize, 30)
const limit = pLimit(10)
const elevationData = await Promise.all(points.map(p => limit(() => fetchElevation(p))))Reply with "@CharlieHelps yes please" if you'd like me to add a commit adding token validation + concurrency limiting and a gridSize cap.
| 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; | ||
|
|
There was a problem hiding this comment.
geometryParam is parsed with JSON.parse without a try/catch, while boundsParam is guarded. A single malformed geometry query parameter will throw and return a 500 instead of a 400.
Also, turf.polygon(geometry.coordinates) assumes the incoming geometry always matches what turf.polygon expects (array of linear rings). If geometry is a GeoJSON Polygon object, you should pass coordinates but validate geometry.type === 'Polygon' first to avoid hard failures.
Suggestion
Wrap geometry parsing in a try/catch and validate the geometry type before constructing a turf polygon:
let geometry: any = null
if (geometryParam) {
try { geometry = JSON.parse(geometryParam) } catch { return NextResponse.json({ error: 'Invalid geometry parameter' }, { status: 400 }) }
if (geometry?.type !== 'Polygon' || !Array.isArray(geometry.coordinates)) {
return NextResponse.json({ error: 'geometry must be a GeoJSON Polygon' }, { status: 400 })
}
}
const polygon = geometry ? turf.polygon(geometry.coordinates) : nullReply with "@CharlieHelps yes please" if you'd like me to add a commit implementing this validation.
| 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(); | ||
| }) |
There was a problem hiding this comment.
In POST, the handler calls GET(new NextRequest(url)). This is an unusual pattern and can be problematic because it bypasses normal Next.js request handling/middleware and can lose request context. It also re-parses URL params and repeats validation paths.
This is better implemented by extracting the core elevation sampling logic into a shared function and calling it from both GET and POST.
Suggestion
Refactor to a shared internal function like computeElevation({ bounds, gridSize, geometry }) and call it from both handlers. This avoids constructing synthetic requests and keeps behavior consistent.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit performing this refactor.
| 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) | ||
| } | ||
| } |
There was a problem hiding this comment.
The new ElevationHeatmapLayer registers event handlers (click, mouseenter, mouseleave) but the cleanup only removes layers/sources. If the layer is removed without also removing handlers, Mapbox can keep references around and leak handlers (or throw warnings).
Additionally, the onMapLoad function is registered via map.once('load', onMapLoad) but is never removed on cleanup if the component unmounts before load fires.
Suggestion
Store handlers and unregister them in cleanup, and cancel the pending load handler:
const clickHandler = (e: mapboxgl.MapLayerMouseEvent) => { ... }
const enterHandler = () => { map.getCanvas().style.cursor = 'pointer' }
const leaveHandler = () => { map.getCanvas().style.cursor = '' }
map.on('click', pointsLayerId, clickHandler)
map.on('mouseenter', pointsLayerId, enterHandler)
map.on('mouseleave', pointsLayerId, leaveHandler)
return () => {
map.off('click', pointsLayerId, clickHandler)
map.off('mouseenter', pointsLayerId, enterHandler)
map.off('mouseleave', pointsLayerId, leaveHandler)
map.off('load', onMapLoad)
...remove layers/sources...
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit addressing the handler cleanup properly.
| interface MapResultsContainerProps { | ||
| id: string | ||
| geoJson?: FeatureCollection | ||
| elevationPoints?: { | ||
| points: any[] | ||
| statistics: any | ||
| } | ||
| } | ||
|
|
||
| export function MapResultsContainer({ id, geoJson, elevationPoints }: MapResultsContainerProps) { | ||
| return ( | ||
| <> | ||
| {geoJson && <GeoJsonLayer id={id} data={geoJson} />} | ||
| {elevationPoints && elevationPoints.points && elevationPoints.points.length > 0 && ( | ||
| <ElevationHeatmapLayer | ||
| id={`${id}-elevation`} | ||
| points={elevationPoints.points} | ||
| statistics={elevationPoints.statistics} | ||
| /> |
There was a problem hiding this comment.
elevationPoints is typed as { points: any[]; statistics: any }, and the child layer takes points with a defined shape. Using any here makes it easy to pass the wrong structure and only discover it at runtime.
Since the API response structure is known (points, statistics, etc.), define a shared ElevationResponse type and use it here and in actions.tsx.
Suggestion
Introduce a shared type, e.g. in lib/types/elevation.ts:
export interface ElevationPoint { lng: number; lat: number; elevation: number }export interface ElevationStatistics { min; max; average; count }export interface ElevationResponse { success: boolean; points: ElevationPoint[]; statistics: ElevationStatistics; bounds: ...; gridSize: number }
Then update MapResultsContainerProps and the JSON parsing in actions.tsx.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit that replaces these any usages with shared types end-to-end.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/actions.tsx (2)
240-243:⚠️ Potential issue | 🔴 Critical
isCollapsedalso not finalized inprocessResolutionSearch.Same issue as the
processEventsfunction: thefinallyblock on lines 240-243 callsisGenerating.done(false)anduiStream.done()but neverisCollapsed.done(false).Proposed fix
} finally { isGenerating.done(false) + isCollapsed.done(false) uiStream.done() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/actions.tsx` around lines 240 - 243, The finally block in processResolutionSearch fails to finalize isCollapsed, mirroring the same bug from processEvents; update the finally block inside processResolutionSearch to call isCollapsed.done(false) in addition to isGenerating.done(false) and uiStream.done() so isCollapsed is always cleaned up when the function completes or errors.
456-522:⚠️ Potential issue | 🔴 Critical
isCollapsedStreamableValue is never finalized outside theinquirebranch.
isCollapsedis created on line 56 but onlydone()'d in the inquire path (line 460). In the researcher/writer path (lines 473-522) and the error path (lines 485-489),isCollapsedis left open. An unfinalizedStreamableValuecan cause the client to hang waiting for further updates.Proposed fix
if (hasError) { isGenerating.done(false) + isCollapsed.done(false) uiStream.done() return }And at the end of the
elseblock:isGenerating.done(false) + isCollapsed.done(false) uiStream.done() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/actions.tsx` around lines 456 - 522, The isCollapsed StreamableValue is only finalized in the inquire branch; add isCollapsed.done(false) in the researcher/writer and error paths so the client isn't left waiting: in the hasError branch (after isGenerating.done(false) and before uiStream.done() / return) call isCollapsed.done(false), and in the successful else flow call isCollapsed.done(false) before isGenerating.done(false) and uiStream.done() (i.e., finalize isCollapsed just prior to ending generation and closing uiStream in the code that follows researcher/writer/querySuggestor).components/header-search-button.tsx (2)
119-132:⚠️ Potential issue | 🔴 CriticalFormData keys
file_mapboxandfile_googledon't match what the server reads.The server action (app/actions.tsx lines 79-80) reads
formData.get('mapboxImage')andformData.get('googleImage')as strings, but this component appends blobs under'file_mapbox'and'file_google'. The mapbox/google images will never reach theResolutionCarousel.Either align the keys, or convert the blobs to data URLs on the client and append them as string values under the expected keys.
Proposed fix — add string data URLs for the expected keys
if (mapboxBlob) formData.append('file_mapbox', mapboxBlob, 'mapbox_capture.png') if (googleBlob) formData.append('file_google', googleBlob, 'google_capture.png') + // Provide data URLs for the server to pass through to the carousel + if (mapboxBlob) { + const mapboxDataUrl = await blobToDataURL(mapboxBlob) + formData.append('mapboxImage', mapboxDataUrl) + } + if (googleBlob) { + const googleDataUrl = await blobToDataURL(googleBlob) + formData.append('googleImage', googleDataUrl) + } + // Keep 'file' for backward compatibility formData.append('file', (mapboxBlob || googleBlob)!, 'map_capture.png')Where
blobToDataURLconverts a Blob to a base64 data URL string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/header-search-button.tsx` around lines 119 - 132, The client is appending blobs as 'file_mapbox' and 'file_google' but the server (app/actions.tsx reading mapboxImage and googleImage) expects string fields named 'mapboxImage' and 'googleImage'; fix header-search-button.tsx by converting mapboxBlob and googleBlob to base64 data URLs (use a blobToDataURL helper) and append those strings to formData under the keys 'mapboxImage' and 'googleImage' (keep the existing blob keys if backward compatibility is desired), ensuring you still append 'file' only if needed and preserve other formData fields like action, timezone, and drawnFeatures.
150-167:⚠️ Potential issue | 🟠 MajorDesktop and mobile buttons are permanently disabled when
mapProvideris'google'.Both buttons use
disabled={isAnalyzing || !map || !actions}, butmapcomes fromuseMap()which is hardcoded to Mapbox (typed asMapboxMap | null). WhenmapProvider === 'google',mapis alwaysnull, making the button permanently disabled — even though lines 96-117 contain a fully functional Google Maps code path.The fix is to check
mapProviderbefore treating a missingmapas a disable condition:Fix
- disabled={isAnalyzing || !map || !actions} + disabled={isAnalyzing || (mapProvider === 'mapbox' && !map) || !actions}Apply to both the desktop button (line 155) and mobile button (line 167).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/header-search-button.tsx` around lines 150 - 167, desktopButton and mobileButton are being disabled because useMap() returns a Mapbox-typed `map` which is null when mapProvider === 'google'; update the disabled checks in the `desktopButton` and `mobileButton` JSX (where `disabled={isAnalyzing || !map || !actions}` is used) to account for `mapProvider` so that a missing `map` only disables the button when the provider is Mapbox (e.g., change the condition to require `map` only when `mapProvider !== 'google'`, keeping the `isAnalyzing` and `!actions` checks unchanged) and keep `handleResolutionSearch` as the click handler.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
app/actions.tsxapp/api/elevation/route.tsapp/layout.tsxcomponents/header-search-button.tsxcomponents/map/elevation-heatmap-layer.tsxcomponents/map/map-results-container.tsxcomponents/mobile-icons-bar.tsxcomponents/resolution-carousel.tsxcomponents/search-related.tsxlib/agents/resolution-search.tsxlib/agents/tools/geospatial.tsxlib/utils/elevation.tspublic/htmx-fallback.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-08T16:50:33.791Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-02-08T16:50:33.791Z
Learning: The PR branch implements RAG (Retrieval-Augmented Generation) via `lib/actions/rag.ts` which uses Supabase RPC functions `generate_embedding()` and `hybrid_search()` to retrieve relevant context from past conversations. However, there's a bug on line 433 of `app/actions.tsx` where `currentSystemPrompt` is passed to the researcher instead of `augmentedSystemPrompt`, preventing the retrieved context from being used.
Applied to files:
app/actions.tsx
🧬 Code graph analysis (6)
lib/utils/elevation.ts (3)
components/map/geojson-layer.tsx (3)
map(16-97)GeoJsonLayer(13-100)map(24-80)components/map/mapbox-map.tsx (1)
feature(79-165)components/map/map-3d-types.ts (1)
Map3DElement(23-30)
app/api/elevation/route.ts (1)
app/api/embeddings/route.ts (2)
GET(126-276)findFileForLocation(59-88)
components/search-related.tsx (4)
components/chat-messages.tsx (1)
ChatMessagesProps(7-9)components/chat.tsx (1)
aiState(78-83)components/sidebar/chat-history-client.tsx (1)
ChatHistoryClientProps(26-28)components/chat-panel.tsx (1)
ChatPanelProps(18-23)
lib/agents/tools/geospatial.tsx (3)
mapbox_mcp/hooks.ts (5)
initClient(60-72)isConnected(49-282)composioClientRef(62-70)composioClientRef(106-121)setIsLoading(75-104)mapbox_mcp/index.ts (1)
testComposioConnection(3-52)mapbox_mcp/composio-mapbox.ts (1)
initializeComposioMapbox(96-115)
components/header-search-button.tsx (1)
components/chat.tsx (1)
aiState(78-83)
app/actions.tsx (3)
lib/types/index.ts (2)
AIMessage(59-77)Chat(49-57)components/map/map-results-container.tsx (1)
MapResultsContainer(19-32)lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)
🔇 Additional comments (5)
public/htmx-fallback.js (1)
1-15: LGTM — clean IIFE safety fallback.The guarded stub pattern correctly avoids overwriting any already-defined handlers and the bracket-notation property names (
"func validation:validate", etc.) are valid JavaScript.lib/utils/elevation.ts (1)
42-44: Remove unuseddecodeMapboxTerrainRGBfunction fromlib/utils/elevation.ts.The function targets raster Terrain-RGB encoding, but the elevation API uses the vector
mapbox.mapbox-terrain-v2tileset and reads elevation directly fromf.properties.ele(contour lines). The function is not imported or called anywhere in the codebase, making it dead code that could confuse developers about which encoding method is actually in use.app/actions.tsx (3)
33-42: LGTM — Clean public type definitions.The
UIStateandAIStatetypes are well-structured and properly exported for use across the codebase.
474-483: No issues to address. The review comment references a concern aboutaugmentedSystemPromptand RAG infrastructure, but neither exists in this codebase. Theresearcher()function accepts adynamicSystemPromptparameter and is correctly called withcurrentSystemPrompton line 476. There is no pre-existing bug here.Likely an incorrect or invalid review comment.
551-559:saveChatreceives an incompleteChatobject.
saveChatis called with{ id: chatId, messages }cast viaas any, missing fields liketitle,createdAt, andpathfrom theChatinterface. WhileuserIdis passed as a separate argument, the incomplete object may cause persistence failures depending onsaveChat's implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/actions.tsx`:
- Around line 603-628: The 'resolution_search_result' branch currently calls
JSON.parse(content) without error handling; wrap the parse of content into a
try/catch (same pattern used in the 'tool' case) to catch JSON parse errors, log
or surface the error and return a fallback/empty response instead of throwing;
specifically surround the JSON.parse into a try block, assign
analysisResult/geoJson/image/mapboxImage/googleImage/elevationPoints inside it,
and in the catch log the error (or create an error component) and return an
appropriate object instead of rendering ResolutionCarousel and
MapResultsContainer.
- Around line 572-588: In getUIStateFromAIState's switch mapping, wrap the
JSON.parse calls in the 'related' and 'resolution_search_result' cases in a
try/catch so malformed JSON can't throw and break the whole mapping; for each
case (the 'related' case that assigns relatedQueries / relatedStream using
createStreamableValue and the 'resolution_search_result' case that parses
resolution queries) catch parse errors, optionally log or swallow them, and fall
back to a safe value (e.g., the original content string or an empty array)
before calling createStreamableValue and returning the component—this mirrors
the existing error-handling used in the 'tool' case and prevents uncaught
exceptions.
- Around line 78-90: The form keys are mismatched: app/actions.tsx reads
formData.get('mapboxImage') and formData.get('googleImage') while
HeaderSearchButton appends blobs as 'file_mapbox' and 'file_google', causing
mapbox/google images to be null; fix by making the keys consistent—either update
HeaderSearchButton to append the blobs under 'mapboxImage' and 'googleImage'
(preferred) or update app/actions.tsx to read 'file_mapbox' and 'file_google'
instead, and ensure ResolutionCarousel receives the same property names
(mapboxDataUrl/googleDataUrl) so the comparison slide and individual satellite
images render.
- Around line 134-156: Replace the server-side HTTP self-fetch of /api/elevation
with a direct call to a shared elevation function (e.g., create a
getElevationData/getElevationPoints utility) and call that from the server
action instead of using fetch with NEXT_PUBLIC_BASE_URL; if you must keep the
HTTP approach as a quick patch, stop using stringified JSON inline and build the
query with URLSearchParams or encodeURIComponent for
analysisResult.elevationData.bounds, analysisResult.elevationData.gridSize and
drawnFeatures[0].geometry, and avoid defaulting to 'http://localhost:3000' so
the server action does not rely on a client base URL. Ensure elevationPointsData
is assigned from the utility/encoded-response handling used.
In `@app/api/elevation/route.ts`:
- Around line 62-88: The current code creates elevationPromises for every point
and calls Promise.all(elevationPromises), which issues up to 441 concurrent
Mapbox tilequery requests; replace this with controlled concurrency/batching to
avoid rate limits and timeouts by processing points in chunks (e.g., BATCH_SIZE
= 50) or using a concurrency limiter (like p-limit) so only a limited number of
fetches run concurrently; specifically, change the logic around points,
elevationPromises and elevationData to iterate over points in slices
(points.slice(i, i + BATCH_SIZE)) or wrap the fetch logic in a limited-run
queue, await each batch (or limited-run promises) before continuing, and
accumulate results into elevationData instead of firing all requests at once.
- Around line 144-147: The POST handler's features.map callback accesses
feature.geometry.type without null-checking, which throws when geometry is
missing; update the mapping in the POST handler (the async callback passed to
features.map) to defensively check for feature and feature.geometry (e.g., use
optional chaining or explicit null checks) before reading .type and skip/return
null for malformed features so Promise.all won't reject on a TypeError.
- Line 32: The code parses geometryParam inline (const geometry = geometryParam
? JSON.parse(geometryParam) : null;) which will throw on invalid JSON and bubble
to the outer catch returning 500; wrap the JSON.parse for geometryParam in the
same guarded try/catch used for boundsParam so that invalid geometry JSON
returns a 400 with a clear error message. Locate geometryParam/geometry in the
route handler (route.ts) and change the assignment to parse inside a try block,
set geometry to null or the parsed object on success, and on JSON parse failure
call the same bad request response flow used for boundsParam.
- Line 34: The destructuring const [west, south, east, north] = bounds can
produce undefined values if bounds.length < 4; update the route handler to
validate bounds is an array of length 4 (in addition to current type checks)
before destructuring, and return a 400/appropriate error when the length is
invalid; reference the bounds variable and the destructuring expression (const
[west, south, east, north] = bounds) so you add the length check immediately
prior to that line and avoid proceeding to coordinate parsing and Mapbox fetches
when the check fails.
- Around line 58-86: The code does not guard against a missing Mapbox token (the
token variable used in the fetch URL), causing every request in
elevationPromises (points.map(...)) to silently fail; add an early check before
creating elevationPromises to validate token (from MAPBOX_ACCESS_TOKEN or
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN) and short-circuit the handler with an explicit
error/failed response (or throw) if absent so you don't perform the per-point
fetches and return a false-success payload; update the handler entrypoint that
consumes elevationPromises to rely on this guard and ensure the error is logged
and returned to the client instead of letting the per-point catch blocks hide
401 failures.
In `@app/layout.tsx`:
- Line 70: The Script instance used for the HTMX fallback stub (the <Script ...
/> in app layout) should use strategy="afterInteractive" and be moved out of the
manual <head> JSX into a sibling of the <body> element per Next.js canonical
pattern; update the Script component invocation that references
"/htmx-fallback.js" to change strategy to "afterInteractive" and relocate that
JSX so it is rendered alongside (not inside) the <body> element.
In `@components/map/elevation-heatmap-layer.tsx`:
- Line 25: The state for the dynamically imported mapbox module loses type
safety; update the useState declaration for the mapboxgl variable to a proper
Mapbox GL type (e.g., use a typeof import('mapbox-gl') or an explicit MapboxGL
type) so setMapboxgl/getMapboxgl are strongly typed. Add an import type for the
mapbox-gl module (import type mapboxgl from 'mapbox-gl' or use the
import('mapbox-gl') type inline) and change const [mapboxgl, setMapboxgl] =
useState<any>(null) to useState<typeof import('mapbox-gl') | null>(null) (or
useState<mapboxgl | null>(null)) so all usages of mapboxgl and setMapboxgl have
correct types.
- Around line 110-123: The map event handlers are registered but never removed:
hoist the handlers out of the inline callbacks by defining clickHandler,
mouseEnterHandler, and mouseLeaveHandler in the same useEffect scope where you
call map.on(pointsLayerId, ...), replace the anonymous mouseenter/mouseleave
arrow functions with mouseEnterHandler and mouseLeaveHandler, and then in the
useEffect cleanup call map.off('click', pointsLayerId, clickHandler),
map.off('mouseenter', pointsLayerId, mouseEnterHandler), and
map.off('mouseleave', pointsLayerId, mouseLeaveHandler) so the handlers are
properly unregistered when the component unmounts (ensure you still remove the
popup behavior within clickHandler as before).
- Around line 127-139: The cleanup currently removes layers/sources but doesn't
cancel a pending load listener, so if map.once('load', onMapLoad) was registered
and the component unmounts the onMapLoad callback can still run and add orphaned
layers; update the cleanup to also call map.off('load', onMapLoad) (or remove
the listener registered) to ensure the onMapLoad handler is deregistered when
the effect cleans up, keeping references to the same onMapLoad function and
leaving existing removal of pointsLayerId, heatmapLayerId and sourceId intact.
In `@components/map/map-results-container.tsx`:
- Around line 13-16: The prop type for elevationPoints currently uses any/any[]
which bypasses type safety; update the prop to reuse the typed shapes from
ElevationHeatmapLayer by importing/exporting its ElevationPoint interface and
the statistics type (e.g., ElevationPoint[] for points and the exported
statistics type) and replace the anonymous any types in the elevationPoints prop
declaration with those imported types so MapResultsContainer uses the exact
ElevationHeatmapLayer types.
In `@components/search-related.tsx`:
- Line 25: Replace the untyped useUIState usage with the exported UIState type:
change useUIState<any>() to useUIState<UIState>() for the declaration that
provides setMessages (symbol: setMessages) and update the callback parameter
type where setMessages is used (the callback at the location that updates
messages around line 38) to accept UIState instead of any; apply the same
pattern to the other files noted by replacing useUIState<any>() with
useUIState<UIState>() in mobile-icons-bar.tsx, resolution-carousel.tsx, and
header-search-button.tsx so all useUIState usages and their callbacks are
strongly typed to UIState.
In `@lib/agents/resolution-search.tsx`:
- Around line 34-35: Validate the "bounds" schema to ensure exactly four numeric
entries (west, south, east, north) instead of an open array, and constrain
"gridSize" to a positive integer greater than zero; update the zod schema in
resolution-search.tsx to use a fixed-length tuple or .min/.max length check for
bounds and use .int().positive() (and optionally a default) for gridSize so the
destructuring const [west, south, east, north] receives defined numbers and grid
generation cannot receive zero/negative values.
In `@lib/agents/tools/geospatial.tsx`:
- Around line 55-56: The local variable config and the MapboxConfig interface
are dead after the simplification: remove the unused const config and the
MapboxConfig type if mapboxAccessToken/version/name are not actually forwarded;
alternatively, if getConnectedMcpClient should pass mapboxAccessToken through,
update getConnectedMcpClient to construct and forward a config object (including
mapboxAccessToken) to the MCP client instead of using hardcoded literals —
locate the unused config declaration and the MapboxConfig interface and either
delete them or wire them into getConnectedMcpClient so the token/version/name
are passed to the MCP client constructor.
In `@lib/utils/elevation.ts`:
- Line 1: The file imports the entire `@turf/turf` namespace but never uses it;
replace the dead import by using turf.bbox in getBoundsFromFeatures (or remove
the import entirely). Update getBoundsFromFeatures to call
turf.bbox(featureCollectionOrArray) to compute [minX,minY,maxX,maxY] which
handles all GeoJSON types (including Multi* geometries) instead of manual
coordinate loops, leaving decodeMapboxTerrainRGB and getElevationColor
unchanged; if you prefer not to add the turf dependency, simply remove the
import line and keep the existing manual bounds logic.
- Around line 50-66: In getElevationColor, clamp the computed normalized value
to the [0,1] range (e.g., normalized = Math.min(1, Math.max(0, normalized)))
before branching so out-of-range elevations cannot produce invalid RGB math, and
optionally clamp each interpolant t = ... to [0,1] as well; update the
normalized/t calculations inside getElevationColor (and any usages of variable
t) so all color channel math operates on clamped values.
---
Outside diff comments:
In `@app/actions.tsx`:
- Around line 240-243: The finally block in processResolutionSearch fails to
finalize isCollapsed, mirroring the same bug from processEvents; update the
finally block inside processResolutionSearch to call isCollapsed.done(false) in
addition to isGenerating.done(false) and uiStream.done() so isCollapsed is
always cleaned up when the function completes or errors.
- Around line 456-522: The isCollapsed StreamableValue is only finalized in the
inquire branch; add isCollapsed.done(false) in the researcher/writer and error
paths so the client isn't left waiting: in the hasError branch (after
isGenerating.done(false) and before uiStream.done() / return) call
isCollapsed.done(false), and in the successful else flow call
isCollapsed.done(false) before isGenerating.done(false) and uiStream.done()
(i.e., finalize isCollapsed just prior to ending generation and closing uiStream
in the code that follows researcher/writer/querySuggestor).
In `@components/header-search-button.tsx`:
- Around line 119-132: The client is appending blobs as 'file_mapbox' and
'file_google' but the server (app/actions.tsx reading mapboxImage and
googleImage) expects string fields named 'mapboxImage' and 'googleImage'; fix
header-search-button.tsx by converting mapboxBlob and googleBlob to base64 data
URLs (use a blobToDataURL helper) and append those strings to formData under the
keys 'mapboxImage' and 'googleImage' (keep the existing blob keys if backward
compatibility is desired), ensuring you still append 'file' only if needed and
preserve other formData fields like action, timezone, and drawnFeatures.
- Around line 150-167: desktopButton and mobileButton are being disabled because
useMap() returns a Mapbox-typed `map` which is null when mapProvider ===
'google'; update the disabled checks in the `desktopButton` and `mobileButton`
JSX (where `disabled={isAnalyzing || !map || !actions}` is used) to account for
`mapProvider` so that a missing `map` only disables the button when the provider
is Mapbox (e.g., change the condition to require `map` only when `mapProvider
!== 'google'`, keeping the `isAnalyzing` and `!actions` checks unchanged) and
keep `handleResolutionSearch` as the click handler.
---
Duplicate comments:
In `@components/header-search-button.tsx`:
- Around line 27-28: The code weakens types by casting useActions<any>() to
HeaderActions and using useUIState<any>() for setMessages; replace the any casts
with precise types: call useActions<HeaderActions>() (or adjust HeaderActions to
match) and useUIState<YourUIStateType>() so setMessages gets the correct message
type, updating any related handlers in the HeaderActions and the component to
use the exact types instead of any.
In `@components/mobile-icons-bar.tsx`:
- Line 29: The state hook in mobile-icons-bar.tsx uses an unsafe any for
useUIState (const [, setMessages] = useUIState<any>()); replace the any with the
proper messages type (e.g., Message[] or the shared MessagesState type used in
search-related.tsx) by importing or reusing that type and updating the generic
passed to useUIState so setMessages has correct typings; ensure you update any
related code in the mobile-icons-bar component to match the new type signature
(useUIState<MessagesType> or equivalent).
In `@components/resolution-carousel.tsx`:
- Around line 28-29: The double cast is redundant: remove the unnecessary "as
any" on the actions declaration so it reads using only useActions<any>() (or
preferably give a concrete type to useActions instead of any); update the line
declaring actions (symbol: actions, call: useActions) to drop the extra cast and
keep setMessages/useUIState unchanged.
| const file = formData?.get('file') as File; | ||
| const mapboxImage = formData?.get('mapboxImage') as string; | ||
| const googleImage = formData?.get('googleImage') as string; | ||
|
|
||
| if (!file) { | ||
| throw new Error('No file provided for resolution search.'); | ||
| isGenerating.done(false); | ||
| return { id: nanoid(), isGenerating: isGenerating.value, component: null, isCollapsed: isCollapsed.value }; | ||
| } | ||
|
|
||
| const mapboxBuffer = file_mapbox ? await file_mapbox.arrayBuffer() : null; | ||
| const mapboxDataUrl = mapboxBuffer ? `data:${file_mapbox.type};base64,${Buffer.from(mapboxBuffer).toString('base64')}` : null; | ||
|
|
||
| const googleBuffer = file_google ? await file_google.arrayBuffer() : null; | ||
| const googleDataUrl = googleBuffer ? `data:${file_google.type};base64,${Buffer.from(googleBuffer).toString('base64')}` : null; | ||
|
|
||
| const buffer = await file.arrayBuffer(); | ||
| const dataUrl = `data:${file.type};base64,${Buffer.from(buffer).toString('base64')}`; | ||
| const mapboxDataUrl = mapboxImage || null; | ||
| const googleDataUrl = googleImage || null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all formData.append operations to understand what keys are being used
rg -n "formData\.append|formData\.get" --type ts --type tsx -A 2 -B 2 | head -100Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Search for formData operations in all TypeScript/TSX files
rg -n "formData\.(append|get)" -A 2 -B 2 | head -150Repository: QueueLab/QCX
Length of output: 3664
🏁 Script executed:
#!/bin/bash
# Check the app/actions.tsx file around lines 78-90
cat -n app/actions.tsx | sed -n '70,100p'Repository: QueueLab/QCX
Length of output: 1414
🏁 Script executed:
#!/bin/bash
# Search for any formData.append with 'mapboxImage' or 'googleImage' keys
rg -n "mapboxImage|googleImage" --type-add 'ts:*.ts' --type-add 'tsx:*.tsx' -B 2 -A 2Repository: QueueLab/QCX
Length of output: 4613
🏁 Script executed:
#!/bin/bash
# Search for any place where 'mapboxImage' or 'googleImage' is appended to formData
rg -n "append.*mapboxImage|append.*googleImage"Repository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Look at how mapboxDataUrl and googleDataUrl are used after being set to null
cat -n app/actions.tsx | sed -n '88,260p'Repository: QueueLab/QCX
Length of output: 7014
Form data key mismatch prevents mapbox/google images from displaying.
The server reads formData.get('mapboxImage') and formData.get('googleImage') (app/actions.tsx, lines 79-80), but HeaderSearchButton appends blobs under keys file_mapbox and file_google (header-search-button.tsx, lines 124-125). These values will always be null when submitted from that component. As a result, ResolutionCarousel receives null values and cannot display the comparison slide or individual mapbox/google satellite images—only the uploaded image is shown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/actions.tsx` around lines 78 - 90, The form keys are mismatched:
app/actions.tsx reads formData.get('mapboxImage') and
formData.get('googleImage') while HeaderSearchButton appends blobs as
'file_mapbox' and 'file_google', causing mapbox/google images to be null; fix by
making the keys consistent—either update HeaderSearchButton to append the blobs
under 'mapboxImage' and 'googleImage' (preferred) or update app/actions.tsx to
read 'file_mapbox' and 'file_google' instead, and ensure ResolutionCarousel
receives the same property names (mapboxDataUrl/googleDataUrl) so the comparison
slide and individual satellite images render.
| // Handle elevation heat map if requested | ||
| let elevationPointsData = null; | ||
| if (analysisResult.elevationData?.requested && analysisResult.elevationData.bounds) { | ||
| try { | ||
| const elevationResponse = await fetch( | ||
| `${process.env.NEXT_PUBLIC_BASE_URL || 'http://localhost:3000'}/api/elevation?` + | ||
| `bounds=${JSON.stringify(analysisResult.elevationData.bounds)}&gridSize=${analysisResult.elevationData.gridSize || 20}${ | ||
| drawnFeatures.length > 0 && drawnFeatures[0].geometry | ||
| ? `&geometry=${JSON.stringify(drawnFeatures[0].geometry)}` | ||
| : '' | ||
| }` | ||
| ); | ||
|
|
||
| if (elevationResponse.ok) { | ||
| const elevationData = await elevationResponse.json(); | ||
| if (elevationData.success && elevationData.points.length > 0) { | ||
| elevationPointsData = elevationData; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Error fetching elevation data:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if elevation route logic can be imported directly instead of fetching via HTTP
echo "=== Elevation-related files ==="
fd "elevation" --type f \( -name "*.ts" -o -name "*.tsx" \) -E node_modules | head -20
echo "=== Elevation route handlers ==="
fd "route.ts" --type f -E node_modules | xargs rg -l "elevation" 2>/dev/null || echo "No elevation in route handlers found"
echo "=== Exportable elevation functions ==="
rg -n "export.*function.*elevation|export.*const.*elevation|export.*async.*function" --type ts --type tsx -g '!node_modules' | grep -i elevation
echo "=== API directory structure ==="
fd . "app/api" --type f -name "*.ts" | head -20Repository: QueueLab/QCX
Length of output: 725
🏁 Script executed:
#!/bin/bash
echo "=== Content of elevation route handler ==="
cat -n "app/api/elevation/route.ts"
echo -e "\n=== Content of app/actions.tsx (lines 130-160) ==="
sed -n '130,160p' "app/actions.tsx" | cat -n
echo -e "\n=== Check if elevation logic exists as a separate utility ==="
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -path "*/app/*" ! -path "*/node_modules/*" ! -path "*/.next/*" | xargs grep -l "elevation" 2>/dev/nullRepository: QueueLab/QCX
Length of output: 8442
Use encodeURIComponent() for query parameters, or refactor to avoid server-to-self HTTP calls.
The elevation data fetch has two issues:
-
Self-fetch anti-pattern: Calling
/api/elevationvia HTTP from a server action is unreliable.NEXT_PUBLIC_BASE_URLwon't resolve correctly in serverless/container environments, andlocalhost:3000fails in production. Consider extracting the elevation logic to a shared utility function instead of going through HTTP. -
Unencoded query parameters:
JSON.stringify()output contains unencoded characters ({,},",:) that should be percent-encoded in URLs. The POST handler in the same route file correctly usesURLSearchParamsfor this—apply the same pattern here usingencodeURIComponent().
Immediate fix for URL encoding
- `bounds=${JSON.stringify(analysisResult.elevationData.bounds)}&gridSize=${analysisResult.elevationData.gridSize || 20}${
+ `bounds=${encodeURIComponent(JSON.stringify(analysisResult.elevationData.bounds))}&gridSize=${analysisResult.elevationData.gridSize || 20}${
drawnFeatures.length > 0 && drawnFeatures[0].geometry
- ? `&geometry=${JSON.stringify(drawnFeatures[0].geometry)}`
+ ? `&geometry=${encodeURIComponent(JSON.stringify(drawnFeatures[0].geometry))}`
: ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/actions.tsx` around lines 134 - 156, Replace the server-side HTTP
self-fetch of /api/elevation with a direct call to a shared elevation function
(e.g., create a getElevationData/getElevationPoints utility) and call that from
the server action instead of using fetch with NEXT_PUBLIC_BASE_URL; if you must
keep the HTTP approach as a quick patch, stop using stringified JSON inline and
build the query with URLSearchParams or encodeURIComponent for
analysisResult.elevationData.bounds, analysisResult.elevationData.gridSize and
drawnFeatures[0].geometry, and avoid defaulting to 'http://localhost:3000' so
the server action does not rely on a client base URL. Ensure elevationPointsData
is assigned from the utility/encoded-response handling used.
| switch (type) { | ||
| case 'response': | ||
| const botMessageStream = createStreamableValue(content as string) | ||
| botMessageStream.done(content as string) | ||
| return { | ||
| id, | ||
| component: <BotMessage content={botMessageStream.value} /> | ||
| } | ||
| break | ||
| case 'assistant': | ||
| const answer = createStreamableValue(content as string) | ||
| answer.done(content as string) | ||
| switch (type) { | ||
| case 'response': | ||
| return { | ||
| id, | ||
| component: ( | ||
| <Section title="response"> | ||
| <BotMessage content={answer.value} /> | ||
| </Section> | ||
| ) | ||
| } | ||
| case 'related': | ||
| const relatedQueries = createStreamableValue<RelatedQueries>({ | ||
| items: [] | ||
| }) | ||
| relatedQueries.done(JSON.parse(content as string)) | ||
| return { | ||
| id, | ||
| component: ( | ||
| <Section title="Related" separator={true}> | ||
| <SearchRelated relatedQueries={relatedQueries.value} /> | ||
| </Section> | ||
| ) | ||
| } | ||
| case 'followup': | ||
| return { | ||
| id, | ||
| component: ( | ||
| <Section title="Follow-up" className="pb-8"> | ||
| <FollowupPanel /> | ||
| </Section> | ||
| ) | ||
| } | ||
| case 'resolution_search_result': { | ||
| const analysisResult = JSON.parse(content as string); | ||
| const geoJson = analysisResult.geoJson as FeatureCollection; | ||
| const image = analysisResult.image as string; | ||
| const mapboxImage = analysisResult.mapboxImage as string; | ||
| const googleImage = analysisResult.googleImage as string; | ||
| case 'related': | ||
| const relatedQueries = JSON.parse(content as string) | ||
| const relatedStream = createStreamableValue(relatedQueries) | ||
| relatedStream.done(relatedQueries) | ||
| return { | ||
| id, | ||
| component: <SearchRelated relatedQueries={relatedStream.value} />, | ||
| isCollapsed: true | ||
| } |
There was a problem hiding this comment.
Missing try/catch around JSON.parse for 'related' and 'resolution_search_result' cases.
Lines 581 and 604 call JSON.parse(content as string) without error handling. If content is malformed, this throws an uncaught exception that crashes the entire getUIStateFromAIState mapping. The 'tool' case on line 630 correctly wraps JSON.parse in try/catch — these cases should be consistent.
Proposed fix for 'related' case
case 'related':
- const relatedQueries = JSON.parse(content as string)
- const relatedStream = createStreamableValue(relatedQueries)
- relatedStream.done(relatedQueries)
- return {
- id,
- component: <SearchRelated relatedQueries={relatedStream.value} />,
- isCollapsed: true
- }
+ try {
+ const relatedQueries = JSON.parse(content as string)
+ const relatedStream = createStreamableValue(relatedQueries)
+ relatedStream.done(relatedQueries)
+ return {
+ id,
+ component: <SearchRelated relatedQueries={relatedStream.value} />,
+ isCollapsed: true
+ }
+ } catch (error) {
+ console.error('Error parsing related content:', error)
+ return { id, component: null }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/actions.tsx` around lines 572 - 588, In getUIStateFromAIState's switch
mapping, wrap the JSON.parse calls in the 'related' and
'resolution_search_result' cases in a try/catch so malformed JSON can't throw
and break the whole mapping; for each case (the 'related' case that assigns
relatedQueries / relatedStream using createStreamableValue and the
'resolution_search_result' case that parses resolution queries) catch parse
errors, optionally log or swallow them, and fall back to a safe value (e.g., the
original content string or an empty array) before calling createStreamableValue
and returning the component—this mirrors the existing error-handling used in the
'tool' case and prevents uncaught exceptions.
| case 'resolution_search_result': { | ||
| const analysisResult = JSON.parse(content as string); | ||
| const geoJson = analysisResult.geoJson as FeatureCollection; | ||
| const image = analysisResult.image as string; | ||
| const mapboxImage = analysisResult.mapboxImage as string; | ||
| const googleImage = analysisResult.googleImage as string; | ||
| const elevationPoints = analysisResult.elevationPoints; | ||
|
|
||
| return { | ||
| id, | ||
| component: ( | ||
| <> | ||
| <ResolutionCarousel | ||
| mapboxImage={mapboxImage} | ||
| googleImage={googleImage} | ||
| initialImage={image} | ||
| /> | ||
| {geoJson && ( | ||
| <GeoJsonLayer id={id} data={geoJson} /> | ||
| )} | ||
| </> | ||
| ) | ||
| } | ||
| } | ||
| return { | ||
| id, | ||
| component: ( | ||
| <> | ||
| <ResolutionCarousel | ||
| mapboxImage={mapboxImage} | ||
| googleImage={googleImage} | ||
| initialImage={image} | ||
| /> | ||
| <MapResultsContainer | ||
| id={id} | ||
| geoJson={geoJson} | ||
| elevationPoints={elevationPoints} | ||
| /> | ||
| </> | ||
| ) | ||
| } | ||
| break | ||
| } |
There was a problem hiding this comment.
Same JSON.parse without try/catch in the 'resolution_search_result' case.
Line 604 parses content without error handling. Wrap it in try/catch for consistency with the 'tool' case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/actions.tsx` around lines 603 - 628, The 'resolution_search_result'
branch currently calls JSON.parse(content) without error handling; wrap the
parse of content into a try/catch (same pattern used in the 'tool' case) to
catch JSON parse errors, log or surface the error and return a fallback/empty
response instead of throwing; specifically surround the JSON.parse into a try
block, assign
analysisResult/geoJson/image/mapboxImage/googleImage/elevationPoints inside it,
and in the catch log the error (or create an error component) and return an
appropriate object instead of rendering ResolutionCarousel and
MapResultsContainer.
| } | ||
|
|
||
| const gridSize = gridSizeParam ? parseInt(gridSizeParam) : 20; | ||
| const geometry = geometryParam ? JSON.parse(geometryParam) : null; |
There was a problem hiding this comment.
geometryParam JSON parse is unguarded — invalid JSON returns 500 instead of 400.
boundsParam has a dedicated try/catch returning a 400, but geometryParam is parsed inline on line 32 without a guard. An invalid JSON geometry string falls through to the outer catch and returns a generic 500 error.
🐛 Proposed fix
- const geometry = geometryParam ? JSON.parse(geometryParam) : null;
+ let geometry = null;
+ if (geometryParam) {
+ try {
+ geometry = JSON.parse(geometryParam);
+ } catch {
+ 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` at line 32, The code parses geometryParam inline
(const geometry = geometryParam ? JSON.parse(geometryParam) : null;) which will
throw on invalid JSON and bubble to the outer catch returning 500; wrap the
JSON.parse for geometryParam in the same guarded try/catch used for boundsParam
so that invalid geometry JSON returns a 400 with a clear error message. Locate
geometryParam/geometry in the route handler (route.ts) and change the assignment
to parse inside a try block, set geometry to null or the parsed object on
success, and on JSON parse failure call the same bad request response flow used
for boundsParam.
| }) => { | ||
| const { submit } = useActions() | ||
| const [, setMessages] = useUIState<typeof AI>() | ||
| const [, setMessages] = useUIState<any>() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, check if UIState is actually exported from app/actions
rg -n "export.*UIState" --type ts --type tsx | head -20
echo "=== Checking imports from app/actions ==="
rg -n "from.*app/actions" --type ts --type tsx | head -30Repository: QueueLab/QCX
Length of output: 170
🏁 Script executed:
# Check the actual implementation of search-related.tsx
fd -e tsx -e ts -path "*/search-related*"Repository: QueueLab/QCX
Length of output: 224
🏁 Script executed:
# Check the other components mentioned in the review
fd -e tsx -e ts -path "*mobile-icons-bar*"
fd -e tsx -e ts -path "*resolution-carousel*"
fd -e tsx -e ts -path "*header-search-button*"Repository: QueueLab/QCX
Length of output: 562
🏁 Script executed:
# Look at the structure of app/actions.tsx to understand the export situation
fd -e tsx -e ts -path "*app/actions*"Repository: QueueLab/QCX
Length of output: 224
🏁 Script executed:
# Find TypeScript files and check for UIState export
find . -name "*.tsx" -o -name "*.ts" | grep -E "(app/actions|search-related|mobile-icons-bar|resolution-carousel|header-search-button)" | head -20Repository: QueueLab/QCX
Length of output: 214
🏁 Script executed:
# Search for UIState export using correct rg syntax
rg "export.*UIState" -g "*.ts" -g "*.tsx"Repository: QueueLab/QCX
Length of output: 229
🏁 Script executed:
# Check imports from app/actions
rg "from.*app/actions" -g "*.ts" -g "*.tsx" | head -30Repository: QueueLab/QCX
Length of output: 402
🏁 Script executed:
# Find and display search-related.tsx
find . -name "search-related.tsx"Repository: QueueLab/QCX
Length of output: 87
🏁 Script executed:
# Read search-related.tsx to see current implementation
cat -n ./components/search-related.tsxRepository: QueueLab/QCX
Length of output: 2351
🏁 Script executed:
# Check mobile-icons-bar.tsx
cat -n ./components/mobile-icons-bar.tsx | head -50Repository: QueueLab/QCX
Length of output: 2022
🏁 Script executed:
# Check resolution-carousel.tsx
cat -n ./components/resolution-carousel.tsx | head -50Repository: QueueLab/QCX
Length of output: 1939
🏁 Script executed:
# Check header-search-button.tsx
cat -n ./components/header-search-button.tsx | head -50Repository: QueueLab/QCX
Length of output: 2210
Replace any with the exported UIState type in useUIState declarations.
UIState is already exported from app/actions.tsx and used successfully in other components (e.g., chat-panel.tsx, chat-messages.tsx). Use useUIState<UIState>() instead of useUIState<any>() at line 25, and type the callback parameter at line 38. The same applies to mobile-icons-bar.tsx (line 29), resolution-carousel.tsx (line 29), and header-search-button.tsx (line 28).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/search-related.tsx` at line 25, Replace the untyped useUIState
usage with the exported UIState type: change useUIState<any>() to
useUIState<UIState>() for the declaration that provides setMessages (symbol:
setMessages) and update the callback parameter type where setMessages is used
(the callback at the location that updates messages around line 38) to accept
UIState instead of any; apply the same pattern to the other files noted by
replacing useUIState<any>() with useUIState<UIState>() in mobile-icons-bar.tsx,
resolution-carousel.tsx, and header-search-button.tsx so all useUIState usages
and their callbacks are strongly typed to UIState.
| bounds: z.array(z.number()).optional().describe('Bounding box for elevation query [west, south, east, north]'), | ||
| gridSize: z.number().optional().describe('Grid resolution for elevation sampling (default: 20)'), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
bounds array is not validated for length; gridSize has no positivity constraint.
z.array(z.number()) accepts any number of elements. The API route destructures const [west, south, east, north] = bounds, so a wrong-length array will silently produce undefined values and broken coordinates. Similarly, z.number() allows 0 or negative gridSize, which would generate an empty or single-point grid.
♻️ Suggested fix
- bounds: z.array(z.number()).optional().describe('Bounding box for elevation query [west, south, east, north]'),
- gridSize: z.number().optional().describe('Grid resolution for elevation sampling (default: 20)'),
+ bounds: z.tuple([z.number(), z.number(), z.number(), z.number()]).optional().describe('Bounding box for elevation query [west, south, east, north]'),
+ gridSize: z.number().int().positive().optional().describe('Grid resolution for elevation sampling (default: 20)'),📝 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.
| bounds: z.array(z.number()).optional().describe('Bounding box for elevation query [west, south, east, north]'), | |
| gridSize: z.number().optional().describe('Grid resolution for elevation sampling (default: 20)'), | |
| bounds: z.tuple([z.number(), z.number(), z.number(), z.number()]).optional().describe('Bounding box for elevation query [west, south, east, north]'), | |
| gridSize: z.number().int().positive().optional().describe('Grid resolution for elevation sampling (default: 20)'), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/resolution-search.tsx` around lines 34 - 35, Validate the "bounds"
schema to ensure exactly four numeric entries (west, south, east, north) instead
of an open array, and constrain "gridSize" to a positive integer greater than
zero; update the zod schema in resolution-search.tsx to use a fixed-length tuple
or .min/.max length check for bounds and use .int().positive() (and optionally a
default) for gridSize so the destructuring const [west, south, east, north]
receives defined numbers and grid generation cannot receive zero/negative
values.
| // Use fallback config (mapbox_mcp_config.json is optional and may be missing) | ||
| const config = { mapboxAccessToken, version: '1.0.0', name: 'mapbox-mcp-server' }; |
There was a problem hiding this comment.
config is assigned but never referenced — dead variable after the simplification.
config.mapboxAccessToken, config.version, and config.name are not used anywhere in getConnectedMcpClient(). The MCP client is instantiated with hardcoded literals on line 93, and the Composio URL is built independently. If mapboxAccessToken genuinely needs to be forwarded to the MCP server, this should be verified; otherwise the variable (and the MapboxConfig interface on lines 30–34) can be removed.
♻️ Proposed fix
- // Use fallback config (mapbox_mcp_config.json is optional and may be missing)
- const config = { mapboxAccessToken, version: '1.0.0', name: 'mapbox-mcp-server' };
-
// Build Composio MCP server URL📝 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.
| // Use fallback config (mapbox_mcp_config.json is optional and may be missing) | |
| const config = { mapboxAccessToken, version: '1.0.0', name: 'mapbox-mcp-server' }; | |
| // Build Composio MCP server URL |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/tools/geospatial.tsx` around lines 55 - 56, The local variable
config and the MapboxConfig interface are dead after the simplification: remove
the unused const config and the MapboxConfig type if
mapboxAccessToken/version/name are not actually forwarded; alternatively, if
getConnectedMcpClient should pass mapboxAccessToken through, update
getConnectedMcpClient to construct and forward a config object (including
mapboxAccessToken) to the MCP client instead of using hardcoded literals —
locate the unused config declaration and the MapboxConfig interface and either
delete them or wire them into getConnectedMcpClient so the token/version/name
are passed to the MCP client constructor.
| @@ -0,0 +1,67 @@ | |||
| import * as turf from '@turf/turf'; | |||
There was a problem hiding this comment.
turf is imported but never used — dead import.
None of the three exported functions (getBoundsFromFeatures, decodeMapboxTerrainRGB, getElevationColor) call any turf.* API, yet the full @turf/turf namespace is imported.
getBoundsFromFeatures could be simplified to use the already-available turf.bbox(), which handles all GeoJSON geometry types (including MultiPolygon, MultiLineString, MultiPoint that the manual implementation silently skips). Either use turf or remove the import.
♻️ Option A — use turf.bbox (removes manual coordinate loops)
import * as turf from '@turf/turf';
import type { Position } from 'geojson';
export function getBoundsFromFeatures(features: Array<{ geometry: any }>): [number, number, number, number] | null {
if (!features || features.length === 0) return null;
- const allCoordinates: Position[] = [];
- features.forEach(feature => {
- if (feature.geometry.type === 'Polygon') {
- feature.geometry.coordinates.forEach((ring: Position[]) => {
- allCoordinates.push(...ring);
- });
- } else if (feature.geometry.type === 'LineString') {
- allCoordinates.push(...feature.geometry.coordinates);
- } else if (feature.geometry.type === 'Point') {
- allCoordinates.push(feature.geometry.coordinates);
- }
- });
- if (allCoordinates.length === 0) return null;
- const lngs = allCoordinates.map(coord => coord[0]);
- const lats = allCoordinates.map(coord => coord[1]);
- return [
- Math.min(...lngs),
- Math.min(...lats),
- Math.max(...lngs),
- Math.max(...lats),
- ];
+ try {
+ const fc = turf.featureCollection(features.map(f => ({ type: 'Feature', geometry: f.geometry, properties: {} })));
+ const bbox = turf.bbox(fc);
+ return bbox as [number, number, number, number];
+ } catch {
+ return null;
+ }
}Option B — remove the import if turf is not needed:
-import * as turf from '@turf/turf';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/utils/elevation.ts` at line 1, The file imports the entire `@turf/turf`
namespace but never uses it; replace the dead import by using turf.bbox in
getBoundsFromFeatures (or remove the import entirely). Update
getBoundsFromFeatures to call turf.bbox(featureCollectionOrArray) to compute
[minX,minY,maxX,maxY] which handles all GeoJSON types (including Multi*
geometries) instead of manual coordinate loops, leaving decodeMapboxTerrainRGB
and getElevationColor unchanged; if you prefer not to add the turf dependency,
simply remove the import line and keep the existing manual bounds logic.
| export function getElevationColor(elevation: number, min: number, max: number): string { | ||
| const normalized = (elevation - min) / (max - min || 1); | ||
|
|
||
| // Color scale: blue -> cyan -> green -> yellow -> red | ||
| if (normalized < 0.25) { | ||
| const t = normalized * 4; | ||
| return `rgb(${Math.round(33 + (103-33) * t)}, ${Math.round(102 + (169-102) * t)}, 172)`; | ||
| } else if (normalized < 0.5) { | ||
| const t = (normalized - 0.25) * 4; | ||
| return `rgb(${Math.round(103 + (209-103) * t)}, ${Math.round(169 + (229-169) * t)}, ${Math.round(207 + (240-207) * t)})`; | ||
| } else if (normalized < 0.75) { | ||
| const t = (normalized - 0.5) * 4; | ||
| return `rgb(${Math.round(209 + (253-209) * t)}, ${Math.round(229 + (219-229) * t)}, ${Math.round(240 + (199-240) * t)})`; | ||
| } else { | ||
| const t = (normalized - 0.75) * 4; | ||
| return `rgb(${Math.round(253 + (178-253) * t)}, ${Math.round(219 + (24-219) * t)}, ${Math.round(199 + (43-199) * t)})`; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
getElevationColor does not clamp normalized — out-of-range elevations produce garbage color values.
If elevation < min or elevation > max, normalized is negative or greater than 1. In the last branch (line 64), t = (normalized - 0.75) * 4 can exceed 1, causing Math.round(253 + (178-253)*t) to go negative (e.g., t=1.25 → 253 - 93.75 = 159, t=2 → 253 - 150 = 103). While the API route should normally bound elevation within [min, max], defensive clamping prevents visual artifacts.
♻️ Proposed fix
export function getElevationColor(elevation: number, min: number, max: number): string {
- const normalized = (elevation - min) / (max - min || 1);
+ const normalized = Math.min(1, Math.max(0, (elevation - min) / (max - min || 1)));📝 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.
| export function getElevationColor(elevation: number, min: number, max: number): string { | |
| const normalized = (elevation - min) / (max - min || 1); | |
| // Color scale: blue -> cyan -> green -> yellow -> red | |
| if (normalized < 0.25) { | |
| const t = normalized * 4; | |
| return `rgb(${Math.round(33 + (103-33) * t)}, ${Math.round(102 + (169-102) * t)}, 172)`; | |
| } else if (normalized < 0.5) { | |
| const t = (normalized - 0.25) * 4; | |
| return `rgb(${Math.round(103 + (209-103) * t)}, ${Math.round(169 + (229-169) * t)}, ${Math.round(207 + (240-207) * t)})`; | |
| } else if (normalized < 0.75) { | |
| const t = (normalized - 0.5) * 4; | |
| return `rgb(${Math.round(209 + (253-209) * t)}, ${Math.round(229 + (219-229) * t)}, ${Math.round(240 + (199-240) * t)})`; | |
| } else { | |
| const t = (normalized - 0.75) * 4; | |
| return `rgb(${Math.round(253 + (178-253) * t)}, ${Math.round(219 + (24-219) * t)}, ${Math.round(199 + (43-199) * t)})`; | |
| } | |
| export function getElevationColor(elevation: number, min: number, max: number): string { | |
| const normalized = Math.min(1, Math.max(0, (elevation - min) / (max - min || 1))); | |
| // Color scale: blue -> cyan -> green -> yellow -> red | |
| if (normalized < 0.25) { | |
| const t = normalized * 4; | |
| return `rgb(${Math.round(33 + (103-33) * t)}, ${Math.round(102 + (169-102) * t)}, 172)`; | |
| } else if (normalized < 0.5) { | |
| const t = (normalized - 0.25) * 4; | |
| return `rgb(${Math.round(103 + (209-103) * t)}, ${Math.round(169 + (229-169) * t)}, ${Math.round(207 + (240-207) * t)})`; | |
| } else if (normalized < 0.75) { | |
| const t = (normalized - 0.5) * 4; | |
| return `rgb(${Math.round(209 + (253-209) * t)}, ${Math.round(229 + (219-229) * t)}, ${Math.round(240 + (199-240) * t)})`; | |
| } else { | |
| const t = (normalized - 0.75) * 4; | |
| return `rgb(${Math.round(253 + (178-253) * t)}, ${Math.round(219 + (24-219) * t)}, ${Math.round(199 + (43-199) * t)})`; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/utils/elevation.ts` around lines 50 - 66, In getElevationColor, clamp the
computed normalized value to the [0,1] range (e.g., normalized = Math.min(1,
Math.max(0, normalized))) before branching so out-of-range elevations cannot
produce invalid RGB math, and optionally clamp each interpolant t = ... to [0,1]
as well; update the normalized/t calculations inside getElevationColor (and any
usages of variable t) so all color channel math operates on clamped values.
1 participant
This PR re-implements the functionality from PR #533 on the current main branch. It includes elevation heatmap support and related geospatial tool improvements.
Summary by CodeRabbit
Release Notes
New Features
Improvements