GeoJSON Upload and Tool Ingestion Pipeline #472
Conversation
- Extend MapData context to support uploaded GeoJSON layers - Update ChatPanel to support GeoJSON file selection - Implement MapDataUpdater component for automatic context sync and map framing - Update Mapbox and Google Maps components to render uploaded GeoJSON - Enable AI tools to ingest GeoJSON into the map pipeline via MapQueryHandler - Ensure persistence of GeoJSON data across chat sessions via database sync - Add test IDs to key components for improved observability Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
|
WalkthroughAdds end-to-end GeoJSON handling: detect inline or uploaded GeoJSON on submit, emit a Changes
Sequence DiagramsequenceDiagram
actor User
participant ChatPanel as Chat Panel
participant Actions as app/actions
participant AIState as AI State
participant MapUpdater as MapDataUpdater
participant MapContext as MapData Context
participant MapRenderer as Map Components
User->>ChatPanel: paste or upload GeoJSON
ChatPanel->>Actions: submit(text/file)
Actions->>Actions: detect & parse GeoJSON
Actions->>AIState: push 'geojson_upload' message (id, filename, data)
Actions->>MapUpdater: stream MapDataUpdater component
MapUpdater->>MapContext: add uploadedGeoJson entry {id, filename, data, visible:true}
MapUpdater->>MapUpdater: compute bbox (turf.bbox)
MapUpdater->>MapRenderer: call fitBounds(bbox)
MapRenderer->>User: render GeoJsonLayer(s)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/actions/chat.ts (1)
211-215:⚠️ Potential issue | 🟡 MinorRedis client instantiation may fail silently or cause runtime errors.
The Redis client is instantiated at module load time with potentially empty environment variables. If
UPSTASH_REDIS_REST_URLis not configured, this will create a client with an empty URL that will fail on first use. Consider:
- Lazy initialization (create client only when needed)
- Guard the instantiation with environment variable checks
- Or remove if Redis functionality is being deprecated (as suggested by the TODO comments)
🛡️ Suggested fix with lazy initialization
-// `@ts-ignore` - Ignoring Redis import error for now as it might be removed or replaced -import { Redis } from '@upstash/redis'; // This will cause issues if REDIS_URL is not configured. -const redis = new Redis({ - url: process.env.UPSTASH_REDIS_REST_URL?.trim() || '', - token: process.env.UPSTASH_REDIS_REST_TOKEN || '' -}); +import { Redis } from '@upstash/redis'; + +let _redis: Redis | null = null; +function getRedis(): Redis { + if (!_redis) { + const url = process.env.UPSTASH_REDIS_REST_URL?.trim(); + const token = process.env.UPSTASH_REDIS_REST_TOKEN; + if (!url || !token) { + throw new Error('Redis configuration missing: UPSTASH_REDIS_REST_URL and UPSTASH_REDIS_REST_TOKEN are required'); + } + _redis = new Redis({ url, token }); + } + return _redis; +}Then update usages:
- await redis.set(`system_prompt:${userId}`, prompt) + await getRedis().set(`system_prompt:${userId}`, prompt)
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 326-340: The current branch that creates a geojson upload message
(around geoJson/type check, nanoid generation and aiState.update that pushes a
message with type 'geojson_upload' and content from file.name) should validate
the GeoJSON more thoroughly before updating aiState: implement or call a
lightweight validator (e.g., a new validateGeoJSON function or an
is-geojson/geojson-validation library) to verify FeatureCollection.features is
an array and each Feature has valid properties (type==='Feature', geometry with
type and coordinates) and that FeatureCollection has valid bbox/crs if present;
if validation fails, avoid pushing the message, log the error (or surface a
user-facing error) and return early. Ensure you reference/replace the simple
geoJson.type check with this validator in the block that generates geoJsonId and
calls aiState.update so only validated GeoJSON gets stored as type
'geojson_upload'.
- Around line 319-350: The GeoJSON parse failure is only console.logged and the
raw file content may still be prepended to messages; modify the catch block
around JSON.parse(textContent) to (1) prevent adding the MapDataUpdater/uiStream
entry or the geojson_upload assistant message when parsing fails and (2) surface
a user-facing error via aiState.update (add an assistant or system message with
id like geoJsonId or similar explaining the file failed to parse) and optionally
append a UI error component instead of MapDataUpdater; reference the existing
symbols JSON.parse, geoJsonId, aiState.update, uiStream.append, and
MapDataUpdater to locate and change behavior.
In `@components/chat.tsx`:
- Around line 92-102: The effect's condition always becomes true once
cameraState exists because arrays are truthy; change the condition in the
useEffect that calls updateDrawingContext so it only runs when id exists,
cameraState exists, and at least one of mapData.drawnFeatures or
mapData.uploadedGeoJson contains items (check .length > 0 or truthy length via
optional chaining), e.g., use mapData.drawnFeatures?.length > 0 ||
mapData.uploadedGeoJson?.length > 0 before calling updateDrawingContext(id,
...); keep the same payload shape and dependency array ([id,
mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]).
In `@components/map/google-map.tsx`:
- Around line 84-86: The map callback currently uses a logical && which can push
false values into the JSX array; update the rendering of mapData.uploadedGeoJson
so you either filter out invisible items first (e.g.,
uploadedGeoJson.filter(item => item.visible).map(item => <GoogleGeoJsonLayer
key={item.id} data={item.data} />)) or use a ternary to return null for
invisible items (item.visible ? <GoogleGeoJsonLayer key={item.id}
data={item.data} /> : null); apply this change where mapData.uploadedGeoJson is
iterated to ensure only valid React nodes (no false values) are returned.
In `@components/map/map-data-context.tsx`:
- Around line 32-37: The uploadedGeoJson entry uses data: any which loses type
safety; update the type in map-data-context.tsx to use GeoJSON's
FeatureCollection (import FeatureCollection from 'geojson' or the project's
GeoJSON types) so uploadedGeoJson?: Array<{ id: string; filename: string; data:
FeatureCollection; visible: boolean; }>; reference the existing
GeoJsonLayerProps in geojson-layer.tsx to ensure the FeatureCollection generic
matches the layer's expected geometry/props types and adjust imports
accordingly.
In `@components/map/map-data-updater.tsx`:
- Around line 9-13: The MapDataUpdaterProps interface uses a loose any for the
data prop; change it to an explicit GeoJSON union (e.g.,
GeoJSON.FeatureCollection | GeoJSON.Feature) by importing the GeoJSON types and
replacing the data: any declaration in MapDataUpdaterProps with the appropriate
union type; also update any usages of the data prop (in the MapDataUpdater
component and callers) to satisfy the stronger type (narrow with type guards
where you expect Feature vs FeatureCollection).
- Around line 22-25: The normalization blindly treats non-FeatureCollection
inputs as Features and can produce invalid GeoJSON; update the logic around the
featureCollection variable to validate the incoming data.type: if data.type ===
'FeatureCollection' keep it, if data.type === 'Feature' use it directly, if
data.type is a GeoJSON Geometry type (e.g., 'Point','Polygon','LineString',
etc.) wrap it in a Feature object ({ type: 'Feature', geometry: data,
properties: {} }), and otherwise handle invalid input by logging an error via
the same logger and returning or falling back to an empty FeatureCollection ({
type: 'FeatureCollection', features: [] }) so downstream consumers only ever
receive a valid FeatureCollection of Feature objects. Ensure this uses the same
variable names (featureCollection, data) so the change is easy to locate.
In `@lib/actions/chat.ts`:
- Around line 165-166: Remove the redundant inner 'use server' directive inside
the updateDrawingContext function; the module already declares 'use server' at
the top, so delete the string literal within the updateDrawingContext function
body (leave the top-level directive intact) to clean up the function declaration
and avoid a no-op directive.
In `@lib/agents/tools/geospatial.tsx`:
- Around line 406-411: When parsedData is a GeoJSON Feature or FeatureCollection
in the geospatial tool, do not set mcpData.location to an empty object because
MapQueryHandler (and its latitude/longitude checks and targetPosition handling)
treats empty objects as invalid; instead either derive a centroid from
parsedData.bbox and populate mcpData.location.latitude and
mcpData.location.longitude, or set mcpData.location to null/undefined to signal
that location should be derived from the GeoJSON later; update the branch
handling parsedData.type === 'FeatureCollection' || 'Feature' to compute a
centroid from parsedData.bbox (or leave location null) and attach geoJson to
mcpData so MapQueryHandler can process it correctly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
app/actions.tsxcomponents/chat-panel.tsxcomponents/chat.tsxcomponents/map/google-map.tsxcomponents/map/map-data-context.tsxcomponents/map/map-data-updater.tsxcomponents/map/map-query-handler.tsxcomponents/map/mapbox-map.tsxcomponents/message.tsxcomponents/user-message.tsxlib/actions/chat.tslib/agents/tools/geospatial.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
lib/actions/chat.ts (2)
lib/actions/chat-db.ts (1)
msg(117-121)components/sidebar/chat-history-client.tsx (1)
ChatHistoryClientProps(24-26)
components/user-message.tsx (2)
tests/chat.spec.ts (2)
markdownMessage(46-62)test(4-89)components/header-search-button.tsx (1)
currentMessages(51-57)
components/map/map-data-context.tsx (2)
components/map/google-geojson-layer.tsx (4)
map(14-57)GoogleGeoJsonLayerProps(6-8)GoogleGeoJsonLayer(10-60)feature(27-44)components/map/geojson-layer.tsx (2)
map(24-80)GeoJsonLayerProps(8-11)
components/chat.tsx (1)
lib/actions/chat.ts (1)
updateDrawingContext(165-205)
app/actions.tsx (3)
components/map/map-data-updater.tsx (1)
MapDataUpdater(15-63)components/map/google-geojson-layer.tsx (2)
map(14-57)feature(27-44)components/map/geojson-layer.tsx (3)
map(16-97)GeoJsonLayer(13-100)map(24-80)
components/message.tsx (1)
tests/chat.spec.ts (1)
markdownMessage(46-62)
components/map/map-query-handler.tsx (1)
components/map/map-data-updater.tsx (1)
MapDataUpdater(15-63)
components/map/map-data-updater.tsx (3)
components/map/map-data-context.tsx (1)
useMapData(61-67)components/map/map-context.tsx (1)
useMap(24-30)components/map/geojson-layer.tsx (1)
map(16-97)
components/map/google-map.tsx (1)
components/map/google-geojson-layer.tsx (3)
GoogleGeoJsonLayer(10-60)feature(27-44)newLayer(52-56)
lib/agents/tools/geospatial.tsx (1)
mapbox_mcp/hooks.ts (3)
LocationResult(12-20)DistanceResult(22-29)PlaceResult(31-39)
🔇 Additional comments (10)
components/user-message.tsx (1)
35-35: LGTM!The
data-testidattribute addition aligns with the test selectors intests/chat.spec.tsand enables proper test automation without affecting component behavior.components/message.tsx (1)
21-21: LGTM!The
data-testidattribute addition matches the test selectors intests/chat.spec.tsand supports automated testing.components/chat-panel.tsx (1)
216-217: LGTM!The file input changes correctly support GeoJSON uploads:
- Both the
.geojsonextension andapplication/geo+jsonMIME type are accepted- The
data-testidattribute enables test automationGeoJSON files will be shown in the attachment preview (lines 294-305) and sent via FormData for server-side processing.
components/map/map-data-context.tsx (1)
48-52: LGTM!Initializing the arrays explicitly in the default state prevents potential undefined access errors when components try to map over these arrays.
lib/actions/chat.ts (1)
180-180: Remove the unnecessary cast and update the schema comment to reflect actual supported roles.The
'data'role is already defined in theAIMessagetype and is actively handled in the codebase (seeapp/actions.tsx:817). The database schema usesvarchar(50), which accepts any string. The cast is redundant since'data'is already part of the union type. The actual issue is that the schema comment atlib/db/schema.ts:30is outdated—it lists only'user', 'assistant', 'system', 'tool'but should include'data'and'function'to match the actualAIMessagetype definition.components/map/mapbox-map.tsx (1)
573-575: No issues found—GeoJsonLayer is correctly within MapContext provider scope.The
GeoJsonLayercomponents at lines 573-575 are rendered within theMapboxcomponent, which successfully uses theuseMap()hook at line 23. SinceMapboxcallsuseMap()without errors, theMapContextProvider(imported inapp/layout.tsxline 18) definitively wraps the entire component tree. TheGeoJsonLayerinstances, being rendered withinMapbox's JSX, are therefore within provider scope and can safely calluseMap()to access the map instance. The code is correct.components/map/map-query-handler.tsx (1)
81-91: LGTM!The conditional rendering of
MapDataUpdaterwhen GeoJSON is available is well-implemented. The use oftoolOutput.timestampas the unique identifier and the fallback filename logic are appropriate.lib/agents/tools/geospatial.tsx (1)
385-394: LGTM on GeoJSON extraction from results.The extraction logic correctly handles both
geoJsonandgeojsonproperty name variants at both the response level and individual result level. This defensive approach handles inconsistent API responses well.app/actions.tsx (2)
751-757: LGTM!The
geojson_uploadmessage type handler correctly reconstructs theMapDataUpdatercomponent from persisted state, enabling GeoJSON layers to reappear when chats are reloaded.
817-836: LGTM ondatamessage handling.The handling of
datamessages withuploadedGeoJsonarrays correctly iterates and rendersMapDataUpdatercomponents for each item. Error handling is appropriate.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| } else if (file.type === 'text/plain' || file.name.endsWith('.geojson') || file.type === 'application/geo+json') { | ||
| const textContent = Buffer.from(buffer).toString('utf-8') | ||
| const isGeoJson = file.name.endsWith('.geojson') || file.type === 'application/geo+json' | ||
|
|
||
| if (isGeoJson) { | ||
| try { | ||
| const geoJson = JSON.parse(textContent) | ||
| if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') { | ||
| const geoJsonId = nanoid() | ||
| // Add a special message to track the GeoJSON upload | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
| ...aiState.get().messages, | ||
| { | ||
| id: geoJsonId, | ||
| role: 'assistant', | ||
| content: JSON.stringify({ data: geoJson, filename: file.name }), | ||
| type: 'geojson_upload' | ||
| } | ||
| ] | ||
| }) | ||
|
|
||
| // Immediately append the updater to the UI stream | ||
| uiStream.append( | ||
| <MapDataUpdater id={geoJsonId} data={geoJson} filename={file.name} /> | ||
| ) | ||
| } | ||
| } catch (e) { | ||
| console.error('Failed to parse GeoJSON:', e) | ||
| } | ||
| } |
There was a problem hiding this comment.
Silent error handling may confuse users when GeoJSON parsing fails.
When JSON.parse(textContent) fails on Line 325, the error is only logged to the console. The file content still gets prepended to the message (lines 352-357), which may lead to confusing AI responses. Consider providing user feedback:
🛡️ Proposed improvement
} catch (e) {
console.error('Failed to parse GeoJSON:', e)
+ // Optionally notify the user
+ uiStream.append(
+ <BotMessage content={createStreamableValue(`⚠️ Failed to parse ${file.name} as valid GeoJSON. The file will be processed as plain text.`).value} />
+ )
}🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 319 - 350, The GeoJSON parse failure is only
console.logged and the raw file content may still be prepended to messages;
modify the catch block around JSON.parse(textContent) to (1) prevent adding the
MapDataUpdater/uiStream entry or the geojson_upload assistant message when
parsing fails and (2) surface a user-facing error via aiState.update (add an
assistant or system message with id like geoJsonId or similar explaining the
file failed to parse) and optionally append a UI error component instead of
MapDataUpdater; reference the existing symbols JSON.parse, geoJsonId,
aiState.update, uiStream.append, and MapDataUpdater to locate and change
behavior.
| if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') { | ||
| const geoJsonId = nanoid() | ||
| // Add a special message to track the GeoJSON upload | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
| ...aiState.get().messages, | ||
| { | ||
| id: geoJsonId, | ||
| role: 'assistant', | ||
| content: JSON.stringify({ data: geoJson, filename: file.name }), | ||
| type: 'geojson_upload' | ||
| } | ||
| ] | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating GeoJSON structure more thoroughly.
The current validation only checks geoJson.type. Invalid GeoJSON (e.g., a FeatureCollection with malformed features) could still pass through and cause rendering issues downstream. Consider using a lightweight validation:
🛡️ Optional: Add feature validation
if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') {
+ // Basic validation for FeatureCollection
+ if (geoJson.type === 'FeatureCollection' && !Array.isArray(geoJson.features)) {
+ console.warn('Invalid FeatureCollection: missing features array')
+ return
+ }
const geoJsonId = nanoid()📝 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.
| if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') { | |
| const geoJsonId = nanoid() | |
| // Add a special message to track the GeoJSON upload | |
| aiState.update({ | |
| ...aiState.get(), | |
| messages: [ | |
| ...aiState.get().messages, | |
| { | |
| id: geoJsonId, | |
| role: 'assistant', | |
| content: JSON.stringify({ data: geoJson, filename: file.name }), | |
| type: 'geojson_upload' | |
| } | |
| ] | |
| }) | |
| if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') { | |
| // Basic validation for FeatureCollection | |
| if (geoJson.type === 'FeatureCollection' && !Array.isArray(geoJson.features)) { | |
| console.warn('Invalid FeatureCollection: missing features array') | |
| return | |
| } | |
| const geoJsonId = nanoid() | |
| // Add a special message to track the GeoJSON upload | |
| aiState.update({ | |
| ...aiState.get(), | |
| messages: [ | |
| ...aiState.get().messages, | |
| { | |
| id: geoJsonId, | |
| role: 'assistant', | |
| content: JSON.stringify({ data: geoJson, filename: file.name }), | |
| type: 'geojson_upload' | |
| } | |
| ] | |
| }) |
🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 326 - 340, The current branch that creates a
geojson upload message (around geoJson/type check, nanoid generation and
aiState.update that pushes a message with type 'geojson_upload' and content from
file.name) should validate the GeoJSON more thoroughly before updating aiState:
implement or call a lightweight validator (e.g., a new validateGeoJSON function
or an is-geojson/geojson-validation library) to verify
FeatureCollection.features is an array and each Feature has valid properties
(type==='Feature', geometry with type and coordinates) and that
FeatureCollection has valid bbox/crs if present; if validation fails, avoid
pushing the message, log the error (or surface a user-facing error) and return
early. Ensure you reference/replace the simple geoJson.type check with this
validator in the block that generates geoJsonId and calls aiState.update so only
validated GeoJSON gets stored as type 'geojson_upload'.
| // useEffect to call the server action when drawnFeatures or uploadedGeoJson changes | ||
| useEffect(() => { | ||
| if (id && mapData.drawnFeatures && mapData.cameraState) { | ||
| console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures); | ||
| if (id && (mapData.drawnFeatures || mapData.uploadedGeoJson) && mapData.cameraState) { | ||
| console.log('Chat.tsx: map data changed, calling updateDrawingContext'); | ||
| updateDrawingContext(id, { | ||
| drawnFeatures: mapData.drawnFeatures, | ||
| drawnFeatures: mapData.drawnFeatures || [], | ||
| cameraState: mapData.cameraState, | ||
| uploadedGeoJson: mapData.uploadedGeoJson || [] | ||
| }); | ||
| } | ||
| }, [id, mapData.drawnFeatures, mapData.cameraState]); | ||
| }, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]); |
There was a problem hiding this comment.
Condition may trigger excessive database writes.
The condition (mapData.drawnFeatures || mapData.uploadedGeoJson) && mapData.cameraState will always evaluate to true once cameraState is set, because drawnFeatures and uploadedGeoJson are initialized as empty arrays (truthy values).
This means updateDrawingContext will be called on every camera state change (pan, zoom, etc.), even when there are no actual drawn features or uploaded GeoJSON to persist.
🐛 Suggested fix to only save when there's actual data
// useEffect to call the server action when drawnFeatures or uploadedGeoJson changes
useEffect(() => {
- if (id && (mapData.drawnFeatures || mapData.uploadedGeoJson) && mapData.cameraState) {
+ const hasDrawnFeatures = mapData.drawnFeatures && mapData.drawnFeatures.length > 0;
+ const hasUploadedGeoJson = mapData.uploadedGeoJson && mapData.uploadedGeoJson.length > 0;
+
+ if (id && (hasDrawnFeatures || hasUploadedGeoJson) && mapData.cameraState) {
console.log('Chat.tsx: map data changed, calling updateDrawingContext');
updateDrawingContext(id, {
drawnFeatures: mapData.drawnFeatures || [],
cameraState: mapData.cameraState,
uploadedGeoJson: mapData.uploadedGeoJson || []
});
}
}, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]);📝 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.
| // useEffect to call the server action when drawnFeatures or uploadedGeoJson changes | |
| useEffect(() => { | |
| if (id && mapData.drawnFeatures && mapData.cameraState) { | |
| console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures); | |
| if (id && (mapData.drawnFeatures || mapData.uploadedGeoJson) && mapData.cameraState) { | |
| console.log('Chat.tsx: map data changed, calling updateDrawingContext'); | |
| updateDrawingContext(id, { | |
| drawnFeatures: mapData.drawnFeatures, | |
| drawnFeatures: mapData.drawnFeatures || [], | |
| cameraState: mapData.cameraState, | |
| uploadedGeoJson: mapData.uploadedGeoJson || [] | |
| }); | |
| } | |
| }, [id, mapData.drawnFeatures, mapData.cameraState]); | |
| }, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]); | |
| // useEffect to call the server action when drawnFeatures or uploadedGeoJson changes | |
| useEffect(() => { | |
| const hasDrawnFeatures = mapData.drawnFeatures && mapData.drawnFeatures.length > 0; | |
| const hasUploadedGeoJson = mapData.uploadedGeoJson && mapData.uploadedGeoJson.length > 0; | |
| if (id && (hasDrawnFeatures || hasUploadedGeoJson) && mapData.cameraState) { | |
| console.log('Chat.tsx: map data changed, calling updateDrawingContext'); | |
| updateDrawingContext(id, { | |
| drawnFeatures: mapData.drawnFeatures || [], | |
| cameraState: mapData.cameraState, | |
| uploadedGeoJson: mapData.uploadedGeoJson || [] | |
| }); | |
| } | |
| }, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]); |
🤖 Prompt for AI Agents
In `@components/chat.tsx` around lines 92 - 102, The effect's condition always
becomes true once cameraState exists because arrays are truthy; change the
condition in the useEffect that calls updateDrawingContext so it only runs when
id exists, cameraState exists, and at least one of mapData.drawnFeatures or
mapData.uploadedGeoJson contains items (check .length > 0 or truthy length via
optional chaining), e.g., use mapData.drawnFeatures?.length > 0 ||
mapData.uploadedGeoJson?.length > 0 before calling updateDrawingContext(id,
...); keep the same payload shape and dependency array ([id,
mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]).
| {mapData.uploadedGeoJson?.map(item => ( | ||
| item.visible && <GoogleGeoJsonLayer key={item.id} data={item.data} /> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Conditional rendering inside .map() can return false to the array.
When item.visible is falsy, the expression item.visible && <GoogleGeoJsonLayer ... /> returns false, which React will include in the rendered output (though it won't display anything). For cleaner behavior, filter before mapping or use a ternary with null:
♻️ Suggested refactor
- {mapData.uploadedGeoJson?.map(item => (
- item.visible && <GoogleGeoJsonLayer key={item.id} data={item.data} />
- ))}
+ {mapData.uploadedGeoJson
+ ?.filter(item => item.visible)
+ .map(item => (
+ <GoogleGeoJsonLayer key={item.id} data={item.data} />
+ ))}📝 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.
| {mapData.uploadedGeoJson?.map(item => ( | |
| item.visible && <GoogleGeoJsonLayer key={item.id} data={item.data} /> | |
| ))} | |
| {mapData.uploadedGeoJson | |
| ?.filter(item => item.visible) | |
| .map(item => ( | |
| <GoogleGeoJsonLayer key={item.id} data={item.data} /> | |
| ))} |
🤖 Prompt for AI Agents
In `@components/map/google-map.tsx` around lines 84 - 86, The map callback
currently uses a logical && which can push false values into the JSX array;
update the rendering of mapData.uploadedGeoJson so you either filter out
invisible items first (e.g., uploadedGeoJson.filter(item =>
item.visible).map(item => <GoogleGeoJsonLayer key={item.id} data={item.data}
/>)) or use a ternary to return null for invisible items (item.visible ?
<GoogleGeoJsonLayer key={item.id} data={item.data} /> : null); apply this change
where mapData.uploadedGeoJson is iterated to ensure only valid React nodes (no
false values) are returned.
| uploadedGeoJson?: Array<{ | ||
| id: string; | ||
| filename: string; | ||
| data: any; // FeatureCollection | ||
| visible: boolean; | ||
| }>; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider typing data as FeatureCollection for type safety.
The data: any type loses type safety. Based on the GeoJSON layer components (e.g., GeoJsonLayerProps in geojson-layer.tsx), this should be a GeoJSON FeatureCollection.
♻️ Suggested type improvement
+import type { FeatureCollection } from 'geojson';
+
// ... in MapData interface
uploadedGeoJson?: Array<{
id: string;
filename: string;
- data: any; // FeatureCollection
+ data: FeatureCollection;
visible: boolean;
}>;📝 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.
| uploadedGeoJson?: Array<{ | |
| id: string; | |
| filename: string; | |
| data: any; // FeatureCollection | |
| visible: boolean; | |
| }>; | |
| uploadedGeoJson?: Array<{ | |
| id: string; | |
| filename: string; | |
| data: FeatureCollection; | |
| visible: boolean; | |
| }>; |
🤖 Prompt for AI Agents
In `@components/map/map-data-context.tsx` around lines 32 - 37, The
uploadedGeoJson entry uses data: any which loses type safety; update the type in
map-data-context.tsx to use GeoJSON's FeatureCollection (import
FeatureCollection from 'geojson' or the project's GeoJSON types) so
uploadedGeoJson?: Array<{ id: string; filename: string; data: FeatureCollection;
visible: boolean; }>; reference the existing GeoJsonLayerProps in
geojson-layer.tsx to ensure the FeatureCollection generic matches the layer's
expected geometry/props types and adjust imports accordingly.
| interface MapDataUpdaterProps { | ||
| id: string; | ||
| data: any; // FeatureCollection or Feature | ||
| filename: string; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider stronger typing for the data prop.
Using any here loses type safety. Consider using a union type to be explicit about expected inputs:
+import type { Feature } from 'geojson';
+
interface MapDataUpdaterProps {
id: string;
- data: any; // FeatureCollection or Feature
+ data: FeatureCollection | Feature | null;
filename: string;
}🤖 Prompt for AI Agents
In `@components/map/map-data-updater.tsx` around lines 9 - 13, The
MapDataUpdaterProps interface uses a loose any for the data prop; change it to
an explicit GeoJSON union (e.g., GeoJSON.FeatureCollection | GeoJSON.Feature) by
importing the GeoJSON types and replacing the data: any declaration in
MapDataUpdaterProps with the appropriate union type; also update any usages of
the data prop (in the MapDataUpdater component and callers) to satisfy the
stronger type (narrow with type guards where you expect Feature vs
FeatureCollection).
| // Ensure it's a FeatureCollection for consistency | ||
| const featureCollection: FeatureCollection = data.type === 'FeatureCollection' | ||
| ? data | ||
| : { type: 'FeatureCollection', features: [data] }; |
There was a problem hiding this comment.
Potential silent failure with invalid GeoJSON data.
The normalization logic assumes data is either a FeatureCollection or a valid Feature. If data has an unexpected structure (e.g., data.type is "Point" or "Polygon" — raw geometries rather than Features), wrapping it as { type: 'FeatureCollection', features: [data] } will produce an invalid FeatureCollection that may cause downstream rendering issues.
Consider adding validation:
🛡️ Proposed validation
+ // Validate that data is a Feature or FeatureCollection
+ if (data.type !== 'FeatureCollection' && data.type !== 'Feature') {
+ console.warn('MapDataUpdater: Invalid GeoJSON type, expected Feature or FeatureCollection:', data.type);
+ return;
+ }
+
// Ensure it's a FeatureCollection for consistency
const featureCollection: FeatureCollection = data.type === 'FeatureCollection'
? data
: { type: 'FeatureCollection', features: [data] };📝 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.
| // Ensure it's a FeatureCollection for consistency | |
| const featureCollection: FeatureCollection = data.type === 'FeatureCollection' | |
| ? data | |
| : { type: 'FeatureCollection', features: [data] }; | |
| // Validate that data is a Feature or FeatureCollection | |
| if (data.type !== 'FeatureCollection' && data.type !== 'Feature') { | |
| console.warn('MapDataUpdater: Invalid GeoJSON type, expected Feature or FeatureCollection:', data.type); | |
| return; | |
| } | |
| // Ensure it's a FeatureCollection for consistency | |
| const featureCollection: FeatureCollection = data.type === 'FeatureCollection' | |
| ? data | |
| : { type: 'FeatureCollection', features: [data] }; |
🤖 Prompt for AI Agents
In `@components/map/map-data-updater.tsx` around lines 22 - 25, The normalization
blindly treats non-FeatureCollection inputs as Features and can produce invalid
GeoJSON; update the logic around the featureCollection variable to validate the
incoming data.type: if data.type === 'FeatureCollection' keep it, if data.type
=== 'Feature' use it directly, if data.type is a GeoJSON Geometry type (e.g.,
'Point','Polygon','LineString', etc.) wrap it in a Feature object ({ type:
'Feature', geometry: data, properties: {} }), and otherwise handle invalid input
by logging an error via the same logger and returning or falling back to an
empty FeatureCollection ({ type: 'FeatureCollection', features: [] }) so
downstream consumers only ever receive a valid FeatureCollection of Feature
objects. Ensure this uses the same variable names (featureCollection, data) so
the change is easy to locate.
| // Fly to the extent of the GeoJSON | ||
| if (map && featureCollection.features.length > 0) { | ||
| try { | ||
| const bbox = turf.bbox(featureCollection); | ||
| map.fitBounds(bbox as [number, number, number, number], { | ||
| padding: 50, | ||
| maxZoom: 15, | ||
| duration: 2000 | ||
| }); | ||
| } catch (e) { | ||
| console.error('Failed to fit bounds for GeoJSON:', e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check how useMap is implemented and what map instance it returns
ast-grep --pattern 'export const useMap = $_'Repository: QueueLab/QCX
Length of output: 489
🏁 Script executed:
# Check for fitBounds usage and context
rg -n 'fitBounds' --type=ts --type=tsx -C3Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Look for Google Maps imports or setup
rg -n 'google\.maps|@react-google-maps|GoogleMap' --type=ts --type=tsxRepository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Check the context around useMap hook to understand the provider
rg -n 'useMap' --type=ts --type=tsx -B2 -A2 | head -50Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Find MapContextType definition
rg -n 'MapContextType' -A5 | head -30Repository: QueueLab/QCX
Length of output: 1138
🏁 Script executed:
# Find MapProvider implementation
rg -n 'MapProvider|export.*Provider' components/map/ -A10 | head -50Repository: QueueLab/QCX
Length of output: 2955
🏁 Script executed:
# Search for fitBounds without type filter
rg -n 'fitBounds' -C3Repository: QueueLab/QCX
Length of output: 584
🏁 Script executed:
# Search for Google Maps references
rg -n 'google.maps|@react-google-maps|GoogleMap'Repository: QueueLab/QCX
Length of output: 3033
🏁 Script executed:
# Check package.json to see what mapping libraries are used
cat package.json | grep -A5 -B5 'mapbox\|google\|map'Repository: QueueLab/QCX
Length of output: 1895
🏁 Script executed:
# Check what map-data-updater.tsx imports and which useMap it uses
head -20 components/map/map-data-updater.tsxRepository: QueueLab/QCX
Length of output: 584
🏁 Script executed:
# Find the Mapbox component to see if it sets map in context
rg -n 'export.*Mapbox|function Mapbox' components/map/ -A15Repository: QueueLab/QCX
Length of output: 1615
🏁 Script executed:
# Check GoogleMapComponent implementation
rg -n 'export function GoogleMapComponent' components/map/ -A30Repository: QueueLab/QCX
Length of output: 1955
🏁 Script executed:
# Check if there's any conditional handling for different map types
rg -n 'MapboxMap|google.maps.Map' components/map/map-data-updater.tsxRepository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
# Get more of GoogleMapComponent implementation
cat components/map/google-map.tsx | head -80Repository: QueueLab/QCX
Length of output: 2406
🏁 Script executed:
# Check if google-geojson-layer uses a different approach
cat components/map/google-geojson-layer.tsxRepository: QueueLab/QCX
Length of output: 1414
🏁 Script executed:
# Verify what providers are actually selectable in settings
rg -n 'mapProvider.*google|mapProvider.*mapbox' components/ -iRepository: QueueLab/QCX
Length of output: 434
🏁 Script executed:
# Check where MapDataUpdater is imported/used
rg -n 'MapDataUpdater' components/Repository: QueueLab/QCX
Length of output: 398
🏁 Script executed:
# Check if there are any other places attempting to call fitBounds or similar
rg -n 'fitBounds|flyTo|panTo' components/ -B2 -A2Repository: QueueLab/QCX
Length of output: 750
🏁 Script executed:
# Verify the actual provider switching logic
cat components/map/map-provider.tsxRepository: QueueLab/QCX
Length of output: 843
Implement bounds-fitting for Google Maps or restrict MapDataUpdater to Mapbox.
When using the Google Maps provider, the map object in context is null because GoogleMapComponent manages maps independently via @vis.gl/react-google-maps. This causes the fitBounds call (line 51) to silently skip due to the if (map && ...) guard, leaving uploaded GeoJSON uncentered in Google Maps.
Either:
- Implement bounds-fitting for Google Maps using its native API (e.g.,
google.maps.LatLngBounds) - Restrict
MapDataUpdaterto Mapbox only - Pass the Google Maps instance through context alongside Mapbox
The current code works without errors only because the bounds-fitting is silently skipped for Google Maps.
| export async function updateDrawingContext(chatId: string, contextData: { drawnFeatures: any[], cameraState: any, uploadedGeoJson?: any[] }) { | ||
| 'use server'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant 'use server' directive.
The 'use server' directive on line 166 is unnecessary since the file already has 'use server' at the top (line 1). This inner directive has no effect and should be removed for clarity.
🧹 Suggested cleanup
export async function updateDrawingContext(chatId: string, contextData: { drawnFeatures: any[], cameraState: any, uploadedGeoJson?: any[] }) {
- 'use server';
console.log('[Action] updateDrawingContext called for chatId:', chatId);📝 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 async function updateDrawingContext(chatId: string, contextData: { drawnFeatures: any[], cameraState: any, uploadedGeoJson?: any[] }) { | |
| 'use server'; | |
| export async function updateDrawingContext(chatId: string, contextData: { drawnFeatures: any[], cameraState: any, uploadedGeoJson?: any[] }) { | |
| console.log('[Action] updateDrawingContext called for chatId:', chatId); |
🤖 Prompt for AI Agents
In `@lib/actions/chat.ts` around lines 165 - 166, Remove the redundant inner 'use
server' directive inside the updateDrawingContext function; the module already
declares 'use server' at the top, so delete the string literal within the
updateDrawingContext function body (leave the top-level directive intact) to
clean up the function declaration and avoid a no-op directive.
| } else if (parsedData.type === 'FeatureCollection' || parsedData.type === 'Feature') { | ||
| // Direct GeoJSON response | ||
| mcpData = { | ||
| location: {}, // Will be derived from bbox if needed, or left empty | ||
| geoJson: parsedData | ||
| }; |
There was a problem hiding this comment.
Empty location object may cause issues downstream.
When handling direct GeoJSON responses, location is set to an empty object. However, MapQueryHandler (lines 36-59) checks for latitude and longitude in the location and logs warnings when they're invalid. This will trigger the warning path and potentially clear targetPosition.
Consider deriving a centroid from the GeoJSON bbox or leaving location as undefined/null to signal that location data should be derived from the GeoJSON itself:
🛡️ Proposed fix
} else if (parsedData.type === 'FeatureCollection' || parsedData.type === 'Feature') {
// Direct GeoJSON response
mcpData = {
- location: {}, // Will be derived from bbox if needed, or left empty
+ location: {
+ place_name: 'GeoJSON Result'
+ },
geoJson: parsedData
};📝 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.
| } else if (parsedData.type === 'FeatureCollection' || parsedData.type === 'Feature') { | |
| // Direct GeoJSON response | |
| mcpData = { | |
| location: {}, // Will be derived from bbox if needed, or left empty | |
| geoJson: parsedData | |
| }; | |
| } else if (parsedData.type === 'FeatureCollection' || parsedData.type === 'Feature') { | |
| // Direct GeoJSON response | |
| mcpData = { | |
| location: { | |
| place_name: 'GeoJSON Result' | |
| }, | |
| geoJson: parsedData | |
| }; |
🤖 Prompt for AI Agents
In `@lib/agents/tools/geospatial.tsx` around lines 406 - 411, When parsedData is a
GeoJSON Feature or FeatureCollection in the geospatial tool, do not set
mcpData.location to an empty object because MapQueryHandler (and its
latitude/longitude checks and targetPosition handling) treats empty objects as
invalid; instead either derive a centroid from parsedData.bbox and populate
mcpData.location.latitude and mcpData.location.longitude, or set
mcpData.location to null/undefined to signal that location should be derived
from the GeoJSON later; update the branch handling parsedData.type ===
'FeatureCollection' || 'Feature' to compute a centroid from parsedData.bbox (or
leave location null) and attach geoJson to mcpData so MapQueryHandler can
process it correctly.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
GeoJSON uploads currently bloat LLM prompts because the full file contents are still appended as a text message in submit, even when successfully ingested into the map pipeline. UI hydration has a correctness risk: geojson_upload is parsed without a try/catch, and replaying uploadedGeoJson via MapDataUpdater can cause repeated fitBounds camera jumps (especially on chat reload). Tool ingestion uses timestamp as an id, which is not reliably unique/stable and can lead to unintended de-duping. Several places rely on any for GeoJSON/state payloads, making the pipeline easier to break silently.
Additional notes (1)
- Performance |
app/actions.tsx:316-354
GeoJSON uploads are still appended into the user prompt
In submit, GeoJSON files are treated as text/plain and their full contents are appended into messageParts as a text message even when they’re successfully parsed and handled via geojson_upload + MapDataUpdater.
That has several real downsides:
- Token bloat / cost: large GeoJSONs will inflate the prompt dramatically.
- Leaky / noisy context: the model sees raw coordinates and properties that it doesn’t need to answer many questions.
- Duplicated behavior: you already persist/render GeoJSON via map state; repeating it in chat input is redundant.
If the intended UX is “upload GeoJSON to the map”, the text payload should typically not be sent to the LLM unless the user explicitly wants it.
Summary of changes
What changed
GeoJSON ingestion pipeline
- Added support for uploading and ingesting GeoJSON (
.geojson,application/geo+json) through the chat file upload flow. - Introduced a new headless client component
MapDataUpdaterto:- normalize input to a
FeatureCollection - add items to
MapDataContext.uploadedGeoJson - auto-
fitBoundsusing Turf.js
- normalize input to a
Map state + rendering
- Extended
MapDatawithuploadedGeoJson(id/filename/data/visible) and initialized it inMapDataProvider. - Render uploaded GeoJSON layers in both:
components/map/mapbox-map.tsxvia<GeoJsonLayer />components/map/google-map.tsxvia<GoogleGeoJsonLayer />
Tool integration
- Extended
geospatialToolparsing to pass throughgeoJsonfrom tool output. - Updated
MapQueryHandlerto render aMapDataUpdaterwhen tool output includes GeoJSON.
Persistence / UI reconstruction
- Updated
getUIStateFromAIStateto:- handle a new
geojson_uploadassistant message type - handle
datamessages that includeuploadedGeoJsonand replay viaMapDataUpdater
- handle a new
Testability
- Added
data-testidattributes to file input, bot messages, and user messages.
| const geoJsonId = nanoid() | ||
| // Add a special message to track the GeoJSON upload | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
| ...aiState.get().messages, | ||
| { | ||
| id: geoJsonId, | ||
| role: 'assistant', | ||
| content: JSON.stringify({ data: geoJson, filename: file.name }), | ||
| type: 'geojson_upload' | ||
| } | ||
| ] | ||
| }) | ||
|
|
||
| // Immediately append the updater to the UI stream | ||
| uiStream.append( | ||
| <MapDataUpdater id={geoJsonId} data={geoJson} filename={file.name} /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Server action appends a React client component into the UI stream
submit (a server action) is directly appending <MapDataUpdater /> into uiStream. That means a server-side path is deciding to render a client component as a side effect of file parsing.
This is fragile for a few reasons:
- It tightly couples server parsing with a specific client synchronization mechanism.
- It risks odd replay behavior (e.g., streaming replays, retries) causing multiple
MapDataUpdatermounts. - It makes the UI update dependent on server streaming rather than the persisted AI state reconstruction (which you already added via
geojson_uploadingetUIStateFromAIState).
Given you already insert a geojson_upload message into aiState, UI reconstruction can naturally render the updater from state without also pushing it immediately.
Suggestion
Remove the immediate uiStream.append(<MapDataUpdater ... />) and rely on the geojson_upload message being rendered via getUIStateFromAIState.
If you need instant feedback before the assistant message is produced, consider appending a lightweight server-renderable placeholder (e.g., “Uploaded X”) and let the client handle map updates based on AI state.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit making this change.
| case 'geojson_upload': { | ||
| const { data, filename } = JSON.parse(content as string) | ||
| return { | ||
| id, | ||
| component: <MapDataUpdater id={id} data={data} filename={filename} /> | ||
| } | ||
| } |
There was a problem hiding this comment.
Unvalidated JSON.parse for geojson_upload can crash UI rendering
getUIStateFromAIState parses content for geojson_upload without a try/catch. Any malformed content (db corruption, older message format, partial writes) will throw and can break UI state mapping.
You already wrap parsing for other message kinds; this one should be consistent.
Suggestion
Wrap the JSON.parse in a try/catch (similar to the tool and data cases) and return { id, component: null } on failure.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this defensive parsing.
| case 'data': | ||
| try { | ||
| const contextData = JSON.parse(content as string) | ||
| if (contextData.uploadedGeoJson && Array.isArray(contextData.uploadedGeoJson)) { | ||
| return { | ||
| id, | ||
| component: ( | ||
| <> | ||
| {contextData.uploadedGeoJson.map((item: any) => ( | ||
| <MapDataUpdater key={item.id} id={item.id} data={item.data} filename={item.filename} /> | ||
| ))} | ||
| </> | ||
| ) | ||
| } | ||
| } | ||
| return { id, component: null } | ||
| } catch (e) { | ||
| console.error('Error parsing data message:', e) | ||
| return { id, component: null } | ||
| } |
There was a problem hiding this comment.
data message rendering uses any and replays side effects without guarding
In the data case you iterate uploadedGeoJson.map((item: any) => <MapDataUpdater ... />). Two concerns:
- Using
anyhere makes it easy to introduce unsafe shapes that still compile. This is UI-state hydration code; it should be strict. - Rendering many
MapDataUpdaters will attemptfitBoundsfor each layer; on chat reload this can cause multiple sequential camera jumps. The currentMapDataUpdateronly de-dupes byidfor inserting into context, but it does not de-dupe thefitBoundsside effect. So reload may result in the map fitting to the last item every time, even if the user previously set a different view.
Suggestion
-
Replace
anywith a minimal structural type (e.g.,{ id: string; data: unknown; filename: string }) and validate required fields before rendering. -
Add a flag to
MapDataUpdaterlikeshouldFlyTo?: booleanand set it tofalsewhen replaying persisteduploadedGeoJsonfrom adatamessage, or add internal logic to onlyfitBoundson first-ever insert (e.g., only when it was newly added to context).
Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing shouldFlyTo and tightening the hydration type.
| return ( | ||
| <> | ||
| {toolOutput?.mcp_response?.geoJson && ( | ||
| <MapDataUpdater | ||
| id={toolOutput.timestamp} | ||
| data={toolOutput.mcp_response.geoJson} | ||
| filename={toolOutput.mcp_response.location?.place_name || 'Tool Result'} | ||
| /> | ||
| )} | ||
| </> | ||
| ); |
There was a problem hiding this comment.
Tool-derived GeoJSON id is not stable/unique enough
MapQueryHandler uses toolOutput.timestamp as the MapDataUpdater id. If two tool calls happen in the same timestamp granularity (or if timestamps are reused across reloads), this can:
- cause unintended de-dupe (layer never appears)
- overwrite expectations when persisting/replaying
Also, timestamps aren’t semantically tied to the GeoJSON payload.
Suggestion
Generate a unique id for each tool GeoJSON emission (e.g., nanoid() on the client, or use a deterministic hash of the GeoJSON if you want stable ids), and keep timestamp as metadata.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit updating the id generation strategy.
| useEffect(() => { | ||
| if (!data) return; | ||
|
|
||
| // Ensure it's a FeatureCollection for consistency | ||
| const featureCollection: FeatureCollection = data.type === 'FeatureCollection' | ||
| ? data | ||
| : { type: 'FeatureCollection', features: [data] }; | ||
|
|
||
| // Update MapData context | ||
| setMapData(prev => { | ||
| // Avoid duplicate entries | ||
| const alreadyExists = prev.uploadedGeoJson?.some(item => item.id === id); | ||
| if (alreadyExists) return prev; | ||
|
|
||
| return { | ||
| ...prev, | ||
| uploadedGeoJson: [ | ||
| ...(prev.uploadedGeoJson || []), | ||
| { | ||
| id, | ||
| filename, | ||
| data: featureCollection, | ||
| visible: true | ||
| } | ||
| ] | ||
| }; | ||
| }); | ||
|
|
||
| // Fly to the extent of the GeoJSON | ||
| if (map && featureCollection.features.length > 0) { | ||
| try { | ||
| const bbox = turf.bbox(featureCollection); | ||
| map.fitBounds(bbox as [number, number, number, number], { | ||
| padding: 50, | ||
| maxZoom: 15, | ||
| duration: 2000 | ||
| }); | ||
| } catch (e) { | ||
| console.error('Failed to fit bounds for GeoJSON:', e); | ||
| } | ||
| } | ||
| }, [id, data, filename, setMapData, map]); |
There was a problem hiding this comment.
MapDataUpdater triggers camera movement on every mount; this will be disruptive
MapDataUpdater always fitBounds whenever it mounts with data. This is likely fine for a single upload, but it becomes disruptive when:
- multiple layers are replayed (e.g., from persisted state) → multiple consecutive camera jumps
- the component remounts due to UI state reconciliation
Because this is a headless “sync” component, controlling side effects is crucial.
Suggestion
Add an explicit prop (e.g., flyTo?: boolean defaulting to true) and/or only run fitBounds if the layer was newly inserted into uploadedGeoJson (i.e., inside the setMapData updater when alreadyExists === false).
That way, replay/hydration can set flyTo={false} and avoid surprising camera behavior.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| // Fly to the extent of the GeoJSON | ||
| if (map && featureCollection.features.length > 0) { | ||
| try { | ||
| const bbox = turf.bbox(featureCollection); | ||
| map.fitBounds(bbox as [number, number, number, number], { | ||
| padding: 50, | ||
| maxZoom: 15, | ||
| duration: 2000 | ||
| }); |
There was a problem hiding this comment.
MapDataUpdater uses map.fitBounds(bbox as [number, number, number, number], ...) with a Turf bbox. Two concerns:
fitBoundsAPIs differ across providers. IfuseMap()can point at multiple map implementations, this can become a runtime hazard.fitBoundsgenerally expects aLngLatBoundsLike(often[[west, south],[east, north]]), while Turf returns[minX, minY, maxX, maxY]. Some libs accept the 4-tuple; others do not.
Right now you’re relying on a cast to force it through.
Suggestion
Normalize the bounds shape and isolate provider differences:
- Convert Turf bbox to a canonical bounds object/tuple your
useMap()abstraction guarantees. - If
useMap()is Mapbox-only, pass[[minX, minY],[maxX, maxY]]to be explicit.
Example:
const [minX, minY, maxX, maxY] = turf.bbox(featureCollection)
const bounds: [[number, number],[number, number]] = [[minX, minY], [maxX, maxY]]
map.fitBounds(bounds, { padding: 50, maxZoom: 15, duration: 2000 })Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change (and optionally a small adapter in useMap).
| // useEffect to call the server action when drawnFeatures or uploadedGeoJson changes | ||
| useEffect(() => { | ||
| if (id && mapData.drawnFeatures && mapData.cameraState) { | ||
| console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures); | ||
| if (id && (mapData.drawnFeatures || mapData.uploadedGeoJson) && mapData.cameraState) { | ||
| console.log('Chat.tsx: map data changed, calling updateDrawingContext'); | ||
| updateDrawingContext(id, { | ||
| drawnFeatures: mapData.drawnFeatures, | ||
| drawnFeatures: mapData.drawnFeatures || [], | ||
| cameraState: mapData.cameraState, | ||
| uploadedGeoJson: mapData.uploadedGeoJson || [] | ||
| }); | ||
| } | ||
| }, [id, mapData.drawnFeatures, mapData.cameraState]); | ||
| }, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]); |
There was a problem hiding this comment.
This effect logs every map-data change and triggers a server action whenever uploadedGeoJson changes. Because uploadedGeoJson elements contain full feature collections, any update that changes object identity (even if semantically the same) can cause repeated writes.
Also, sending full GeoJSON on every change is likely heavy. If MapDataUpdater adds one layer, this effect will upload the entire uploadedGeoJson array back to the server each time.
Suggestion
Reduce redundant writes and payload size:
- Remove the
console.log(or gate it behind a debug flag). - Persist only deltas (the newly added layer) instead of the whole array, or persist only IDs + references if you already store the GeoJSON elsewhere.
- If you keep this approach, debounce the
updateDrawingContextcall (e.g., 300–500ms) and/or compute a stable hash ofuploadedGeoJsonto avoid writes on identity-only changes.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing a debounce + change detection.
| uploadedGeoJson?: Array<{ | ||
| id: string; | ||
| filename: string; | ||
| data: any; // FeatureCollection | ||
| visible: boolean; | ||
| }>; |
There was a problem hiding this comment.
uploadedGeoJson is typed as data: any in context and MapDataUpdaterProps, and geoJson?: any in tool output. Since this pipeline crosses persistence boundaries and executes viewport operations, this is a good place to tighten runtime validation and narrow types.
Even a lightweight validation (checking type and features array) would prevent downstream runtime errors and reduce the surface area for malicious/accidental payloads.
Suggestion
Replace any with Feature | FeatureCollection (from geojson) and add a small runtime validator at ingestion points (upload + tool) that rejects anything else.
Example types:
import type { Feature, FeatureCollection } from 'geojson'
data: FeatureCollection | FeatureReply with "@CharlieHelps yes please" if you’d like me to add a commit that tightens types and adds a shared isGeoJsonFeatureOrCollection() helper.
- Extend MapData context to support uploaded GeoJSON layers - Update ChatPanel to support GeoJSON file selection and add test IDs - Implement MapDataUpdater component for automatic context sync and map framing - Update Mapbox and Google Maps components to render uploaded GeoJSON - Enable AI tools to ingest GeoJSON into the map pipeline via MapQueryHandler - Ensure persistence of GeoJSON data across chat sessions via database sync - Update AIMessage type to support 'geojson_upload' - Fix ESLint warnings to ensure clean build performance Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/map/mapbox-map.tsx (1)
356-458:⚠️ Potential issue | 🟠 MajorMap re-initialization on camera updates (Line 458).
IncludingmapData.cameraStateandpositionin this effect’s deps will destroy and recreate the map on every camera update (since cleanup runs before re-run). This can cause flicker, lost state, and heavy performance regressions.🛠️ Proposed fix: keep initialization effect stable
- }, [setMap, setIsMapLoaded, captureMapCenter, handleUserInteraction, stopRotation, mapData.cameraState, position?.latitude, position?.longitude]) + }, [setMap, setIsMapLoaded, captureMapCenter, handleUserInteraction, stopRotation])
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/chat-panel.tsxcomponents/map/mapbox-map.tsxlib/types/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
components/map/mapbox-map.tsx (1)
components/map/geojson-layer.tsx (4)
GeoJsonLayer(13-100)map(24-80)map(89-96)GeoJsonLayerProps(8-11)
🔇 Additional comments (5)
components/chat-panel.tsx (2)
133-156: Good dependency fix for debounced suggestions.
IncludingsetSuggestionsavoids stale closure risk without changing behavior.
210-218: GeoJSON accept + test id looks good.
The accept filter now supports GeoJSON uploads and adds a stable selector for tests.lib/types/index.ts (1)
65-77: Type union extension is consistent.
Adding'geojson_upload'aligns with the new message flow.components/map/mapbox-map.tsx (2)
13-16: No issues to flag here.
573-575: GeoJSON layer rendering is properly gated.
Rendering only visible layers keeps behavior predictable and aligns with the new upload pipeline.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@coderabbitai its stuck refreshing and i need to be able to copy geojson into the chat input |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8151 🏁 Script executed: Length of output: 1690 🏁 Script executed: Length of output: 6910 🏁 Script executed: Length of output: 3123 🏁 Script executed: Length of output: 5986 🏁 Script executed: Length of output: 180 🏁 Script executed: Length of output: 2966
🔴 Issue 1: Infinite Refresh LoopThe loop occurs because:
Solution:Fix 1: Add a ref to track if data is being restored in // Add near the top with other useState/useRef hooks
const isRestoringRef = useRef(false);
// Modify the useEffect that calls updateDrawingContext:
useEffect(() => {
// Don't call updateDrawingContext during initial restoration
if (isRestoringRef.current) {
isRestoringRef.current = false;
return;
}
if (id && (mapData.drawnFeatures || mapData.uploadedGeoJson) && mapData.cameraState) {
console.log('Chat.tsx: map data changed, calling updateDrawingContext');
updateDrawingContext(id, {
drawnFeatures: mapData.drawnFeatures || [],
cameraState: mapData.cameraState,
uploadedGeoJson: mapData.uploadedGeoJson || []
});
}
}, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]);Fix 2: Update export function MapDataUpdater({ id, data, filename }: MapDataUpdaterProps) {
const { setMapData } = useMapData();
const { map } = useMap();
const hasUpdatedRef = useRef(false); // Add this
useEffect(() => {
if (!data || hasUpdatedRef.current) return; // Prevent re-runs
hasUpdatedRef.current = true; // Mark as updated
// Ensure it's a FeatureCollection for consistency
const featureCollection: FeatureCollection = data.type === 'FeatureCollection'
? data
: { type: 'FeatureCollection', features: [data] };
// Update MapData context
setMapData(prev => {
// Avoid duplicate entries
const alreadyExists = prev.uploadedGeoJson?.some(item => item.id === id);
if (alreadyExists) return prev;
return {
...prev,
uploadedGeoJson: [
...(prev.uploadedGeoJson || []),
{
id,
filename,
data: featureCollection,
visible: true
}
]
};
});
// Fly to the extent of the GeoJSON (only if not already present)
if (map && featureCollection.features.length > 0) {
try {
const bbox = turf.bbox(featureCollection);
map.fitBounds(bbox as [number, number, number, number], {
padding: 50,
maxZoom: 15,
duration: 2000
});
} catch (e) {
console.error('Failed to fit bounds for GeoJSON:', e);
}
}
}, [id, data, filename, setMapData, map]);
return null;
}🟢 Issue 2: Paste GeoJSON into Chat InputAdd GeoJSON detection from pasted text in // After the file handling block (around line 330), add:
// Check if userInput contains GeoJSON
if (userInput && !file) {
try {
const trimmedInput = userInput.trim();
// Try to parse as JSON
if ((trimmedInput.startsWith('{') && trimmedInput.endsWith('}')) ||
(trimmedInput.startsWith('[') && trimmedInput.endsWith(']'))) {
const possibleGeoJson = JSON.parse(trimmedInput);
// Check if it's valid GeoJSON
if (possibleGeoJson.type === 'FeatureCollection' || possibleGeoJson.type === 'Feature') {
const geoJsonId = nanoid();
// Add a special message to track the GeoJSON upload
aiState.update({
...aiState.get(),
messages: [
...aiState.get().messages,
{
id: geoJsonId,
role: 'assistant',
content: JSON.stringify({
data: possibleGeoJson,
filename: 'Pasted GeoJSON'
}),
type: 'geojson_upload'
}
]
});
// Immediately append the updater to the UI stream
uiStream.append(
<MapDataUpdater
id={geoJsonId}
data={possibleGeoJson}
filename="Pasted GeoJSON"
/>
);
// Clear the input since we've handled it as GeoJSON
// Don't add it to messages as regular text
return {
id: nanoid(),
isGenerating: isGenerating.value,
component: uiStream.value,
isCollapsed: isCollapsed.value
};
}
}
} catch (e) {
// Not valid JSON, continue with normal text handling
}
}Would you like me to create a plan to implement these fixes? 💡 Use |
I've implemented support for pasting GeoJSON directly into the chat input. I've also added a synchronization guard to prevent redundant calls to |
- Add support for pasting GeoJSON directly into the chat input - Implement synchronization guard in Chat.tsx to prevent redundant state saves - Ensure 'geojson_upload' message type is correctly handled in AI state - Improve GeoJSON extraction from geospatial tool responses Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/actions.tsx (1)
348-386:⚠️ Potential issue | 🟡 MinorUploaded GeoJSON file content is also sent as text to the AI model.
Similar to pasted GeoJSON, after successfully parsing a GeoJSON file (lines 352-375), the raw text content is still prepended to
messageParts(lines 381-386). For large GeoJSON files, this could significantly increase token usage and processing time.Consider conditionally skipping the text prepending when GeoJSON is successfully processed:
🛡️ Proposed fix
+ let geoJsonHandled = false if (isGeoJson) { try { const geoJson = JSON.parse(textContent) if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') { const geoJsonId = nanoid() // Add a special message to track the GeoJSON upload aiState.update({ ...aiState.get(), messages: [ ...aiState.get().messages, { id: geoJsonId, role: 'assistant', content: JSON.stringify({ data: geoJson, filename: file.name }), type: 'geojson_upload' } ] }) // Immediately append the updater to the UI stream uiStream.append( <MapDataUpdater id={geoJsonId} data={geoJson} filename={file.name} /> ) + geoJsonHandled = true } } catch (e) { console.error('Failed to parse GeoJSON:', e) } } - const existingTextPart = messageParts.find(p => p.type === 'text') - if (existingTextPart) { - existingTextPart.text = `${textContent}\n\n${existingTextPart.text}` - } else { - messageParts.push({ type: 'text', text: textContent }) + // Only add raw text if GeoJSON wasn't successfully handled + if (!geoJsonHandled) { + const existingTextPart = messageParts.find(p => p.type === 'text') + if (existingTextPart) { + existingTextPart.text = `${textContent}\n\n${existingTextPart.text}` + } else { + messageParts.push({ type: 'text', text: textContent }) + } }
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 780-786: The JSON.parse in the 'geojson_upload' case can throw for
malformed content; wrap the parse of content in a try-catch (the case handling
that returns component: <MapDataUpdater ... />) similar to the other role
branches: on success return the existing object with id and component:
<MapDataUpdater id={id} data={data} filename={filename} />, on parse failure
catch the error, log or report it, and return a safe fallback entry (e.g., an
object with id and a lightweight error/display component or null component) so
UI state reconstruction doesn't crash; reference the 'geojson_upload' case,
JSON.parse(content as string), and MapDataUpdater when applying the fix.
- Around line 214-241: The pasted GeoJSON block currently handles the data
(using trimmedInput, JSON.parse, aiState.update and uiStream.append with
MapDataUpdater) but does not stop further processing, so the raw JSON still gets
added to messageParts; fix this by short-circuiting after successful GeoJSON
handling — either clear userInput or set a local flag (e.g. processedGeoJson)
and return/skip the subsequent messageParts construction/send logic so the raw
JSON is not appended or sent to the AI model.
In `@components/chat.tsx`:
- Around line 105-109: The current sync flow calls updateDrawingContext (in
components/chat.tsx) which—via lib/actions/chat.ts—creates a new message record
each time and embeds the full uploadedGeoJson array, causing duplicate large
payloads; change this to either (A) send only lightweight metadata (id,
filename, visible, maybe a geoJsonRef) from the UI and persist full GeoJSON in a
separate store/table with a stable key, or (B) modify
updateDrawingContext/lib/actions/chat.ts to detect existing drawing-context
messages (by drawing id or message type) and perform an update/patch of the
existing message instead of inserting a new one; update the callsites in
components/chat.tsx to pass the reduced metadata or reference key accordingly
and ensure read paths fetch full GeoJSON by reference when rendering.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
app/actions.tsxcomponents/chat.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
app/actions.tsx
🧬 Code graph analysis (2)
app/actions.tsx (1)
components/map/map-data-updater.tsx (1)
MapDataUpdater(15-63)
components/chat.tsx (6)
lib/actions/chat.ts (1)
updateDrawingContext(165-205)components/map/mapbox-map.tsx (2)
prevData(166-166)prevData(330-339)components/map/map-data-context.tsx (3)
MapData(15-32)mapData(41-49)MapDataContextType(34-37)components/map/map-3d.tsx (1)
prevData(35-45)components/map/google-map.tsx (1)
features(38-52)components/map/map-query-handler.tsx (2)
prevData(52-56)prevData(39-48)
🔇 Additional comments (1)
app/actions.tsx (1)
846-865: LGTM!The
datarole handling correctly parses context data with proper try-catch error handling and rendersMapDataUpdatercomponents for each uploaded GeoJSON item.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (userInput) { | ||
| try { | ||
| const trimmedInput = userInput.trim() | ||
| if ((trimmedInput.startsWith('{') && trimmedInput.endsWith('}')) || (trimmedInput.startsWith('[') && trimmedInput.endsWith(']'))) { | ||
| const geoJson = JSON.parse(trimmedInput) | ||
| if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') { | ||
| const geoJsonId = nanoid() | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
| ...aiState.get().messages, | ||
| { | ||
| id: geoJsonId, | ||
| role: 'assistant', | ||
| content: JSON.stringify({ data: geoJson, filename: 'Pasted GeoJSON' }), | ||
| type: 'geojson_upload' | ||
| } | ||
| ] | ||
| }) | ||
| uiStream.append( | ||
| <MapDataUpdater id={geoJsonId} data={geoJson} filename="Pasted GeoJSON" /> | ||
| ) | ||
| } | ||
| } | ||
| } catch (e) { | ||
| // Not a valid JSON, ignore | ||
| } | ||
| } |
There was a problem hiding this comment.
Pasted GeoJSON is processed twice — once as upload and again as text input.
After successfully detecting and handling GeoJSON (lines 219-236), the code continues execution without returning or clearing userInput. The raw JSON text is still added to messageParts at line 334 and sent to the AI model, wasting tokens and potentially confusing the model with raw coordinate data.
Consider skipping the text message when GeoJSON is successfully parsed:
🐛 Proposed fix to prevent double processing
if (userInput) {
try {
const trimmedInput = userInput.trim()
if ((trimmedInput.startsWith('{') && trimmedInput.endsWith('}')) || (trimmedInput.startsWith('[') && trimmedInput.endsWith(']'))) {
const geoJson = JSON.parse(trimmedInput)
if (geoJson.type === 'FeatureCollection' || geoJson.type === 'Feature') {
const geoJsonId = nanoid()
aiState.update({
...aiState.get(),
messages: [
...aiState.get().messages,
{
id: geoJsonId,
role: 'assistant',
content: JSON.stringify({ data: geoJson, filename: 'Pasted GeoJSON' }),
type: 'geojson_upload'
}
]
})
uiStream.append(
<MapDataUpdater id={geoJsonId} data={geoJson} filename="Pasted GeoJSON" />
)
+ // Skip normal text processing for GeoJSON input
+ isGenerating.done(false)
+ uiStream.done()
+ aiState.done(aiState.get())
+ return {
+ id: nanoid(),
+ isGenerating: isGenerating.value,
+ component: uiStream.value,
+ isCollapsed: isCollapsed.value
+ }
}
}
} catch (e) {
// Not a valid JSON, ignore
}
}🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 214 - 241, The pasted GeoJSON block currently
handles the data (using trimmedInput, JSON.parse, aiState.update and
uiStream.append with MapDataUpdater) but does not stop further processing, so
the raw JSON still gets added to messageParts; fix this by short-circuiting
after successful GeoJSON handling — either clear userInput or set a local flag
(e.g. processedGeoJson) and return/skip the subsequent messageParts
construction/send logic so the raw JSON is not appended or sent to the AI model.
| case 'geojson_upload': { | ||
| const { data, filename } = JSON.parse(content as string) | ||
| return { | ||
| id, | ||
| component: <MapDataUpdater id={id} data={data} filename={filename} /> | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing error handling for malformed geojson_upload content.
The JSON.parse call on line 781 will throw if content is malformed, potentially breaking the entire UI state reconstruction. Other similar cases in this function (e.g., tool role at line 790) wrap parsing in try-catch.
🛡️ Proposed fix
case 'geojson_upload': {
+ try {
const { data, filename } = JSON.parse(content as string)
return {
id,
component: <MapDataUpdater id={id} data={data} filename={filename} />
}
+ } catch (e) {
+ console.error('Error parsing geojson_upload content:', e)
+ return { id, component: null }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case 'geojson_upload': { | |
| const { data, filename } = JSON.parse(content as string) | |
| return { | |
| id, | |
| component: <MapDataUpdater id={id} data={data} filename={filename} /> | |
| } | |
| } | |
| case 'geojson_upload': { | |
| try { | |
| const { data, filename } = JSON.parse(content as string) | |
| return { | |
| id, | |
| component: <MapDataUpdater id={id} data={data} filename={filename} /> | |
| } | |
| } catch (e) { | |
| console.error('Error parsing geojson_upload content:', e) | |
| return { id, component: null } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 780 - 786, The JSON.parse in the
'geojson_upload' case can throw for malformed content; wrap the parse of content
in a try-catch (the case handling that returns component: <MapDataUpdater ...
/>) similar to the other role branches: on success return the existing object
with id and component: <MapDataUpdater id={id} data={data} filename={filename}
/>, on parse failure catch the error, log or report it, and return a safe
fallback entry (e.g., an object with id and a lightweight error/display
component or null component) so UI state reconstruction doesn't crash; reference
the 'geojson_upload' case, JSON.parse(content as string), and MapDataUpdater
when applying the fix.
|
@coderabbitai the application is refreshing and losing context |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 8582 🏁 Script executed: Length of output: 1690 🏁 Script executed: Length of output: 27 🏁 Script executed: Length of output: 13211 🏁 Script executed: Length of output: 9832 Let me search for all router.refresh calls with the correct command: 🏁 Script executed: Length of output: 1935 🏁 Script executed: Length of output: 4533 🏁 Script executed: Length of output: 6299
🔴 Critical Issues Found1. router.refresh() causing context lossIn useEffect(() => {
if (aiState.messages[aiState.messages.length - 1]?.type === 'response') {
// Refresh the page to chat history updates
router.refresh() // ← This resets ALL React state!
}
}, [aiState, router])This completely refreshes the page and loses all React context state (MapData, etc.). 2. Infinite database message creation
const savedMessage = await dbCreateMessage(messageToSave); // Creates new message each time!3. MapDataUpdater missing guard
🔄 The Infinite Loop
✅ Complete FixI need to make three changes. Would you like me to create a plan to implement these fixes? Fix 1: Remove the problematic router.refresh() // components/chat.tsx - REMOVE OR COMMENT OUT lines 76-80
// useEffect(() => {
// if (aiState.messages[aiState.messages.length - 1]?.type === 'response') {
// router.refresh()
// }
// }, [aiState, router])Fix 2: Add hasUpdatedRef to MapDataUpdater // components/map/map-data-updater.tsx
const hasUpdatedRef = useRef(false);
useEffect(() => {
if (!data || hasUpdatedRef.current) return;
hasUpdatedRef.current = true;
// ... rest of code
}, [id, data, filename, setMapData, map]);Fix 3: Skip updateDrawingContext during restoration // components/chat.tsx
const isRestoringRef = useRef(true);
useEffect(() => {
// Skip during initial restoration
if (isRestoringRef.current) {
isRestoringRef.current = false;
return;
}
// ... rest of existing code
}, [id, mapData.drawnFeatures, mapData.cameraState, mapData.uploadedGeoJson]);Would you like me to create a complete implementation plan for these fixes? 💡 Use |
This commit implements a comprehensive pipeline for GeoJSON data overlays: - Updated `ChatPanel` to accept `.geojson` files and handle pasted GeoJSON text. - Added `MapDataUpdater` headless component to sync GeoJSON to map context and automatically fit viewport bounds using Turf.js. - Fixed an infinite refresh loop in `Chat.tsx` by introducing `lastRefreshedMessageIdRef` to guard `router.refresh()`. - Prevented redundant camera movement loops in `MapDataUpdater` using `hasZoomedRef`. - Updated database schema and message mapping to include `type`, `attachments`, and tool metadata, ensuring map overlays persist across page refreshes. - Implemented `onConflictDoUpdate` (upsert) for saving messages to prevent primary key collisions. - Supported GeoJSON ingestion from both file uploads, manual text input, and agent tools. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/actions.tsx (1)
879-886:⚠️ Potential issue | 🟡 MinorAdd explicit return after the outer switch to handle unmapped type combinations.
The
case 'user'andcase 'assistant'branches contain inner switches without default cases. When a message type doesn't match any inner switch case, execution falls through the outer switch'sbreakstatement and returnsundefinedinstead of a value. The subsequent.filter(message => message !== null)will not remove theseundefinedentries.🛠️ Proposed fix
.map((message, index) => { const { role, content, id, type, name } = message ... switch (role) { ... } + return null }) - .filter(message => message !== null) as UIState + .filter((message): message is UIState[number] => message != null)
🤖 Fix all issues with AI agents
In `@app/search/`[id]/page.tsx:
- Around line 51-55: The mapping of DB records to AIMessage in page.tsx
currently casts nullable columns directly (dbMsg.type, dbMsg.toolName) which can
leave nulls where AIMessage expects undefined; update the mapping in the
function that constructs the AIMessage object (the block assigning
role/content/createdAt/type/name) to convert nullable values to undefined (e.g.,
use null-coalescing like dbMsg.type ?? undefined and dbMsg.toolName ?? undefined
or conditional expressions) so type and name are undefined when the DB returns
null.
In `@drizzle/migrations/0001_aromatic_ultimatum.sql`:
- Around line 1-12: This migration uses gen_random_uuid() in the calendar_notes
table but doesn't enable the pgcrypto extension; add a CREATE EXTENSION IF NOT
EXISTS "pgcrypto"; statement at the top of the first migration (before the
CREATE TABLE "calendar_notes") so gen_random_uuid() is available when the
migration runs.
In `@lib/actions/chat-db.ts`:
- Around line 123-136: The upsert currently uses
tx.insert(messages).values(messagesToInsert).onConflictDoUpdate targeting
messages.id which can overwrite rows across chats/users; modify the
onConflictDoUpdate call to include a setWhere clause that restricts updates to
rows where messages.chat_id = excluded.chat_id (or messages.chatId =
excluded.chatId) and messages.user_id = excluded.user_id (or messages.userId =
excluded.userId) so updates only apply when the incoming row matches the same
chatId and userId as the existing row; keep the same set mapping (content, role,
type, tool_name, tool_call_id, attachments) but add the setWhere guard to the
onConflictDoUpdate invocation on tx.insert/messages/messagesToInsert.
In `@lib/actions/chat.ts`:
- Around line 188-205: The current update branch in save/update logic uses
dbGetMessagesByChatId and existingMessages.find(...) which returns the first
(oldest) 'data' message and can leave the UI showing the last 'data' entry
out-of-sync with getUIStateFromAIState; change the selection to target the most
recent 'data' message (e.g., pick the last matching element by timestamp or id)
or iterate and update all messages with role === 'data' so the DB update against
messages (used in the db.update(...) block) updates the latest/each 'data'
record rather than the first one found; update references:
dbGetMessagesByChatId, existingMessages, dataMessage, and the db.update(...)
call accordingly.
In `@lib/db/schema.ts`:
- Around line 33-36: The attachments column is declared with
jsonb('attachments') which yields an untyped any; define and use a concrete type
for the JSON shape (e.g., Attachment or Attachment[] depending on expected
payload) and pass it into jsonb to lock down typings (e.g.,
jsonb<Attachment[]>('attachments')), or if null is not meaningful set a typed
default value instead; update the schema declaration that references attachments
and any related uses of toolName/toolCallId/type to use the new Attachment type
for downstream safety and remove unnecessary null checks.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
app/actions.tsxapp/search/[id]/page.tsxcomponents/chat.tsxcomponents/map/map-data-updater.tsxdrizzle/migrations/0001_aromatic_ultimatum.sqldrizzle/migrations/meta/0000_snapshot.jsondrizzle/migrations/meta/0001_snapshot.jsondrizzle/migrations/meta/_journal.jsonlib/actions/chat-db.tslib/actions/chat.tslib/db/schema.tspackage.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
app/actions.tsx
🧬 Code graph analysis (5)
lib/actions/chat-db.ts (1)
lib/db/schema.ts (1)
messages(26-37)
lib/db/schema.ts (1)
lib/types/index.ts (1)
Chat(49-57)
app/search/[id]/page.tsx (1)
lib/types/index.ts (1)
AIMessage(59-78)
components/chat.tsx (3)
components/map/map-data-context.tsx (2)
MapData(15-32)mapData(41-49)lib/actions/chat.ts (1)
updateDrawingContext(165-222)components/map/mapbox-map.tsx (3)
prevData(166-166)prevData(330-339)feature(78-164)
components/map/map-data-updater.tsx (2)
components/map/map-data-context.tsx (4)
useMapData(61-67)mapData(41-49)MapData(15-32)MapDataContextType(34-37)components/map/map-context.tsx (1)
useMap(24-30)
🪛 Biome (2.3.13)
app/actions.tsx
[error] 698-698: This callback passed to map() iterable method should always return a value.
Add missing return statements so that this callback returns a value on all execution paths.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (10)
drizzle/migrations/meta/0000_snapshot.json (1)
1-178: Snapshot metadata update looks consistent.drizzle/migrations/meta/0001_snapshot.json (1)
1-305: Snapshot looks consistent with the updated schema.package.json (1)
65-65: No action required—drizzle-orm 0.45.1 has no breaking changes.drizzle-orm 0.45.1 (released December 10) contains only a single bug fix for pg-native Pool detection and has no breaking changes to migrations or query APIs. The current drizzle-kit 0.31.1 is compatible with this version; ideally, both should be upgraded together, but this is an optional alignment—not a blocker.
drizzle/migrations/meta/_journal.json (1)
11-18: Journal entry addition looks consistent.app/actions.tsx (3)
26-26: Import wiring is fine.
693-703: Filtering to the latest data message makes restoration deterministic.
859-878: Data-message GeoJSON reconstruction looks good.components/chat.tsx (3)
75-88: Refresh guard looks solid.The last-message ID guard should prevent repeated refresh cycles on the same response.
96-115: Snapshot-based map sync reduces redundant updates.
120-174: Layout-only wrapper changes.Also applies to: 180-238
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| role: dbMsg.role as AIMessage['role'], | ||
| content: dbMsg.content, | ||
| createdAt: dbMsg.createdAt ? new Date(dbMsg.createdAt) : undefined, | ||
| // 'type' and 'name' are not in the basic Drizzle 'messages' schema. | ||
| // These would be undefined unless specific logic is added to derive them. | ||
| // For instance, if a message with role 'tool' should have a 'name', | ||
| // or if some messages have a specific 'type' based on content or other flags. | ||
| // This mapping assumes standard user/assistant messages primarily. | ||
| type: dbMsg.type as AIMessage['type'], | ||
| name: dbMsg.toolName as string, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd app/search && find . -type f -name "page.tsx" | head -5Repository: QueueLab/QCX
Length of output: 71
🏁 Script executed:
cd app && find . -type f -name "*.tsx" -o -name "*.ts" | grep -E "(page|types|message)" | head -20Repository: QueueLab/QCX
Length of output: 89
🏁 Script executed:
cat -n app/search/\[id\]/page.tsx | sed -n '40,65p'Repository: QueueLab/QCX
Length of output: 1170
🏁 Script executed:
# Find AIMessage type definition
rg -n "type AIMessage|interface AIMessage" --type=ts --type=tsxRepository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Check dbMsg source and context
rg -n "dbMsg" app/search/\[id\]/page.tsx -B 5 -A 2Repository: QueueLab/QCX
Length of output: 607
🏁 Script executed:
rg -n "type AIMessage|interface AIMessage" -A 10Repository: QueueLab/QCX
Length of output: 1274
🏁 Script executed:
rg -n "type DrizzleMessage|interface DrizzleMessage" -A 10Repository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
# Search for where these types are defined
fd -e ts -e tsx | xargs rg "export (type|interface) AIMessage"Repository: QueueLab/QCX
Length of output: 100
🏁 Script executed:
fd -e ts -e tsx | xargs rg "export (type|interface) DrizzleMessage"Repository: QueueLab/QCX
Length of output: 40
🏁 Script executed:
# Check for downstream usages of message.name and message.type
rg "\.name\s*\?" -t=ts -t=tsx | head -20Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
# Look for optional chaining or null checks on message fields
rg "message\.(name|type|toolName)" | head -30Repository: QueueLab/QCX
Length of output: 501
🏁 Script executed:
# Find DrizzleMessage or Message type from Drizzle schema
rg -n "export type Message|export type DrizzleMessage" -A 15Repository: QueueLab/QCX
Length of output: 1189
🏁 Script executed:
# Check the database schema or Drizzle definitions
fd -e ts -e tsx | xargs rg "messages.*table\|export const messages"Repository: QueueLab/QCX
Length of output: 40
🏁 Script executed:
# Look at the Drizzle schema file
find . -name "*schema*" -o -name "*drizzle*" | head -10Repository: QueueLab/QCX
Length of output: 549
🏁 Script executed:
# Check db directory structure
ls -la db/ 2>/dev/null || ls -la lib/db/ 2>/dev/null || find . -type d -name "db" | head -3Repository: QueueLab/QCX
Length of output: 349
🏁 Script executed:
cat -n lib/db/schema.tsRepository: QueueLab/QCX
Length of output: 4470
🏁 Script executed:
# Check the actual schema definition for the messages table
cat -n lib/schema/messages.ts 2>/dev/null || cat -n lib/db/schema.ts | grep -A 30 "messages"Repository: QueueLab/QCX
Length of output: 3827
🏁 Script executed:
# Check the AIMessage type definition more completely
cat -n lib/types/index.ts | sed -n '59,75p'Repository: QueueLab/QCX
Length of output: 614
🏁 Script executed:
# Check if there are any null checks on message.type or message.name in the codebase
rg "message\.(type|name)\s*\?" -A 2 -B 2Repository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
# Check how the initialMessages are used downstream
rg "initialMessages" app/search/\[id\]/page.tsx -A 5 -B 5Repository: QueueLab/QCX
Length of output: 836
🏁 Script executed:
# Check how the type and name fields are accessed in downstream code
rg "\.type\s*==|\.type\s*!=|\.type\s*\?|\.name\s*==|\.name\s*!=" app/ -A 2 -B 2Repository: QueueLab/QCX
Length of output: 3401
🏁 Script executed:
# Look at the AI component to see how it uses these fields
rg -n "initialAIState" app/ -A 10 | head -50Repository: QueueLab/QCX
Length of output: 1910
🏁 Script executed:
# Check if there are assertions or strict equality checks on type/name
rg "(type|name)\s*!==" app/ -B 2 -A 2Repository: QueueLab/QCX
Length of output: 981
Convert nullable database fields to undefined when mapping to AIMessage.
Lines 54–55 cast dbMsg.type and dbMsg.toolName without converting null values to undefined. Since the database schema defines these columns as nullable (no .notNull() constraint) and the AIMessage type expects undefined for optional fields, assign null-coalesced values to maintain type consistency.
Recommended fix
- type: dbMsg.type as AIMessage['type'],
- name: dbMsg.toolName as string,
+ type: dbMsg.type ?? undefined,
+ name: dbMsg.toolName ?? undefined,📝 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.
| role: dbMsg.role as AIMessage['role'], | |
| content: dbMsg.content, | |
| createdAt: dbMsg.createdAt ? new Date(dbMsg.createdAt) : undefined, | |
| // 'type' and 'name' are not in the basic Drizzle 'messages' schema. | |
| // These would be undefined unless specific logic is added to derive them. | |
| // For instance, if a message with role 'tool' should have a 'name', | |
| // or if some messages have a specific 'type' based on content or other flags. | |
| // This mapping assumes standard user/assistant messages primarily. | |
| type: dbMsg.type as AIMessage['type'], | |
| name: dbMsg.toolName as string, | |
| role: dbMsg.role as AIMessage['role'], | |
| content: dbMsg.content, | |
| createdAt: dbMsg.createdAt ? new Date(dbMsg.createdAt) : undefined, | |
| type: dbMsg.type ?? undefined, | |
| name: dbMsg.toolName ?? undefined, |
🤖 Prompt for AI Agents
In `@app/search/`[id]/page.tsx around lines 51 - 55, The mapping of DB records to
AIMessage in page.tsx currently casts nullable columns directly (dbMsg.type,
dbMsg.toolName) which can leave nulls where AIMessage expects undefined; update
the mapping in the function that constructs the AIMessage object (the block
assigning role/content/createdAt/type/name) to convert nullable values to
undefined (e.g., use null-coalescing like dbMsg.type ?? undefined and
dbMsg.toolName ?? undefined or conditional expressions) so type and name are
undefined when the DB returns null.
| CREATE TABLE "calendar_notes" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "chat_id" uuid, | ||
| "date" timestamp with time zone NOT NULL, | ||
| "content" text NOT NULL, | ||
| "location_tags" jsonb, | ||
| "user_tags" text[], | ||
| "map_feature_id" text, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL, | ||
| "updated_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "CREATE EXTENSION.*pgcrypto|gen_random_uuid" drizzle/migrationsRepository: QueueLab/QCX
Length of output: 1209
🏁 Script executed:
rg -n "CREATE EXTENSION" drizzle/Repository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
head -20 drizzle/migrations/0000_sweet_metal_master.sqlRepository: QueueLab/QCX
Length of output: 763
🏁 Script executed:
find . -maxdepth 2 -type f \( -name "README*" -o -name "*.md" -o -name "init*" -o -name "setup*" \) | head -20Repository: QueueLab/QCX
Length of output: 356
🏁 Script executed:
rg -i "pgcrypto|database.*init|extension" README.md ./docs/ ./CLOUD_BUILD_SETUP.md 2>/dev/null | head -20Repository: QueueLab/QCX
Length of output: 38
Ensure pgcrypto is enabled before using gen_random_uuid().
gen_random_uuid() requires the pgcrypto extension; it is not created in any migration file. This migration will fail unless pgcrypto is already enabled in the database. Add CREATE EXTENSION IF NOT EXISTS "pgcrypto"; at the top of the first migration, or confirm it exists in your database setup.
🛠️ Proposed fix
+CREATE EXTENSION IF NOT EXISTS "pgcrypto";
+
CREATE TABLE "calendar_notes" (📝 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.
| CREATE TABLE "calendar_notes" ( | |
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | |
| "user_id" uuid NOT NULL, | |
| "chat_id" uuid, | |
| "date" timestamp with time zone NOT NULL, | |
| "content" text NOT NULL, | |
| "location_tags" jsonb, | |
| "user_tags" text[], | |
| "map_feature_id" text, | |
| "created_at" timestamp with time zone DEFAULT now() NOT NULL, | |
| "updated_at" timestamp with time zone DEFAULT now() NOT NULL | |
| ); | |
| CREATE EXTENSION IF NOT EXISTS "pgcrypto"; | |
| CREATE TABLE "calendar_notes" ( | |
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | |
| "user_id" uuid NOT NULL, | |
| "chat_id" uuid, | |
| "date" timestamp with time zone NOT NULL, | |
| "content" text NOT NULL, | |
| "location_tags" jsonb, | |
| "user_tags" text[], | |
| "map_feature_id" text, | |
| "created_at" timestamp with time zone DEFAULT now() NOT NULL, | |
| "updated_at" timestamp with time zone DEFAULT now() NOT NULL | |
| ); |
🤖 Prompt for AI Agents
In `@drizzle/migrations/0001_aromatic_ultimatum.sql` around lines 1 - 12, This
migration uses gen_random_uuid() in the calendar_notes table but doesn't enable
the pgcrypto extension; add a CREATE EXTENSION IF NOT EXISTS "pgcrypto";
statement at the top of the first migration (before the CREATE TABLE
"calendar_notes") so gen_random_uuid() is available when the migration runs.
| // Use upsert to avoid duplicate primary keys and prevent redundant appends | ||
| await tx.insert(messages) | ||
| .values(messagesToInsert) | ||
| .onConflictDoUpdate({ | ||
| target: messages.id, | ||
| set: { | ||
| content: sql`excluded.content`, | ||
| role: sql`excluded.role`, | ||
| type: sql`excluded.type`, | ||
| toolName: sql`excluded.tool_name`, | ||
| toolCallId: sql`excluded.tool_call_id`, | ||
| attachments: sql`excluded.attachments` | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the chat-db.ts file
find . -name "chat-db.ts" -type fRepository: QueueLab/QCX
Length of output: 80
🏁 Script executed:
# Read the file to see the full context around lines 123-136
cat -n lib/actions/chat-db.ts | sed -n '100,150p'Repository: QueueLab/QCX
Length of output: 2263
🏁 Script executed:
# Check the messages table schema definition
fd -e ts -e js | xargs grep -l "messages.*Table\|export.*messages" | head -5Repository: QueueLab/QCX
Length of output: 95
🏁 Script executed:
# Examine the messages table schema
cat -n lib/db/schema.ts | head -150Repository: QueueLab/QCX
Length of output: 4470
🏁 Script executed:
# Search specifically for the messages table definition
grep -n "messages.*=" lib/db/schema.ts | head -10Repository: QueueLab/QCX
Length of output: 176
🏁 Script executed:
# Check if onConflictDoUpdate with where clause is used elsewhere in the codebase
rg "onConflictDoUpdate" -A 5 lib/Repository: QueueLab/QCX
Length of output: 391
🏁 Script executed:
# Check Drizzle ORM version in package.json to understand API capabilities
cat package.json | grep -A 2 -B 2 "drizzle"Repository: QueueLab/QCX
Length of output: 230
🌐 Web query:
Drizzle ORM onConflictDoUpdate where clause support version 0.45
💡 Result:
In Drizzle ORM v0.45.x, .onConflictDoUpdate() does support WHERE clauses, but not via a single where field. Instead you use:
targetWhere: addsWHERE ...to the conflict target (for partial indexes /ON CONFLICT (...) WHERE ...)setWhere: addsWHERE ...to the DO UPDATE part (... DO UPDATE SET ... WHERE ...)
Example:
await db.insert(employees).values({...}).onConflictDoUpdate({
target: employees.employeeId,
targetWhere: sql`name <> 'John Doe'`,
set: { name: sql`excluded.name` },
});
await db.insert(employees).values({...}).onConflictDoUpdate({
target: employees.employeeId,
set: { name: 'John Doe' },
setWhere: sql`name <> 'John Doe'`,
});This split (where → targetWhere + setWhere) was introduced earlier (v0.30.8 for Postgres; v0.30.9 for SQLite), so it’s available in v0.45.0 as well. [1] [2]
Notes:
- This applies to PostgreSQL and SQLite upserts (
ON CONFLICT ...). [1] - MySQL uses
.onDuplicateKeyUpdate(...)(and doesn’t have the sameON CONFLICT ... WHERE ...structure). [1]
Sources:
[1] Drizzle ORM Insert docs (where clauses, targetWhere/setWhere)
[2] Drizzle ORM v0.30.8 release note (split into setWhere and targetWhere)
🏁 Script executed:
# Check if there are any imports of `and`, `eq` functions from drizzle-orm in the file
grep -n "import.*from.*drizzle" lib/actions/chat-db.tsRepository: QueueLab/QCX
Length of output: 172
🌐 Web query:
Drizzle ORM setWhere syntax and condition with eq and chatId userId
💡 Result:
In Drizzle ORM:
1) Normal UPDATE / SELECT / DELETE: use .where(...) (not setWhere)
import { and, eq } from "drizzle-orm";
await db
.update(chatMembers)
.set({ lastReadAt: new Date() })
.where(and(
eq(chatMembers.chatId, chatId),
eq(chatMembers.userId, userId),
));(.where(...) takes a single condition; combine multiple with and(...).) (orm.drizzle.team)
2) INSERT ... onConflictDoUpdate(...): setWhere is for the DO UPDATE’s WHERE
import { and, eq } from "drizzle-orm";
await db.insert(chatMembers).values({ chatId, userId, ... })
.onConflictDoUpdate({
target: [chatMembers.chatId, chatMembers.userId],
set: { lastReadAt: new Date() },
setWhere: and(
eq(chatMembers.chatId, chatId),
eq(chatMembers.userId, userId),
),
});setWhere / targetWhere are specific to onConflictDoUpdate. (orm.drizzle.team)
Citations:
- 1: https://orm.drizzle.team/docs/update?utm_source=openai
- 2: https://orm.drizzle.team/docs/insert?utm_source=openai
Add setWhere guard to prevent cross-chat/user overwrites in upsert.
The upsert targets messages.id alone without guarding against conflicts. If an id collision occurs (or a malicious id is supplied), this can overwrite a message belonging to another chat or user. Add a setWhere clause that restricts updates to rows matching the same chatId and userId being inserted.
🔒 Guarded onConflict update
await tx.insert(messages)
.values(messagesToInsert)
.onConflictDoUpdate({
target: messages.id,
set: {
content: sql`excluded.content`,
role: sql`excluded.role`,
type: sql`excluded.type`,
toolName: sql`excluded.tool_name`,
toolCallId: sql`excluded.tool_call_id`,
attachments: sql`excluded.attachments`
- }
+ },
+ setWhere: and(
+ eq(messages.chatId, sql`excluded.chat_id`),
+ eq(messages.userId, sql`excluded.user_id`)
+ )
});🤖 Prompt for AI Agents
In `@lib/actions/chat-db.ts` around lines 123 - 136, The upsert currently uses
tx.insert(messages).values(messagesToInsert).onConflictDoUpdate targeting
messages.id which can overwrite rows across chats/users; modify the
onConflictDoUpdate call to include a setWhere clause that restricts updates to
rows where messages.chat_id = excluded.chat_id (or messages.chatId =
excluded.chatId) and messages.user_id = excluded.user_id (or messages.userId =
excluded.userId) so updates only apply when the incoming row matches the same
chatId and userId as the existing row; keep the same set mapping (content, role,
type, tool_name, tool_call_id, attachments) but add the setWhere guard to the
onConflictDoUpdate invocation on tx.insert/messages/messagesToInsert.
| try { | ||
| // We need to ensure the message is associated with the chat. | ||
| // dbCreateMessage requires chatId. | ||
| // Check if a 'data' message already exists to avoid duplication | ||
| const existingMessages = await dbGetMessagesByChatId(chatId); | ||
| const dataMessage = existingMessages.find(m => m.role === 'data'); | ||
|
|
||
| if (dataMessage) { | ||
| // Update existing message content using direct db call since chat-db.ts doesn't have an updateMessage | ||
| const { db } = await import('@/lib/db'); | ||
| const { messages } = await import('@/lib/db/schema'); | ||
| const { eq } = await import('drizzle-orm'); | ||
|
|
||
| await db.update(messages) | ||
| .set({ content: JSON.stringify(contextData) }) | ||
| .where(eq(messages.id, dataMessage.id)); | ||
|
|
||
| console.log('Drawing context updated for chat:', chatId, 'messageId:', dataMessage.id); | ||
| return { success: true, messageId: dataMessage.id }; | ||
| } |
There was a problem hiding this comment.
Update the most recent data message to stay in sync with UI restoration.
getUIStateFromAIState renders the last data message, but this update path uses find() which may pick the oldest entry if duplicates exist (legacy data). That can leave the rendered context stale after reloads. Prefer updating the latest data message (or updating all data messages for the chat) to match restoration behavior.
🐛 Proposed fix (pick latest data message)
- const dataMessage = existingMessages.find(m => m.role === 'data');
+ const dataMessage = [...existingMessages]
+ .filter(m => m.role === 'data')
+ .sort(
+ (a, b) =>
+ new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()
+ )[0];📝 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.
| try { | |
| // We need to ensure the message is associated with the chat. | |
| // dbCreateMessage requires chatId. | |
| // Check if a 'data' message already exists to avoid duplication | |
| const existingMessages = await dbGetMessagesByChatId(chatId); | |
| const dataMessage = existingMessages.find(m => m.role === 'data'); | |
| if (dataMessage) { | |
| // Update existing message content using direct db call since chat-db.ts doesn't have an updateMessage | |
| const { db } = await import('@/lib/db'); | |
| const { messages } = await import('@/lib/db/schema'); | |
| const { eq } = await import('drizzle-orm'); | |
| await db.update(messages) | |
| .set({ content: JSON.stringify(contextData) }) | |
| .where(eq(messages.id, dataMessage.id)); | |
| console.log('Drawing context updated for chat:', chatId, 'messageId:', dataMessage.id); | |
| return { success: true, messageId: dataMessage.id }; | |
| } | |
| try { | |
| // Check if a 'data' message already exists to avoid duplication | |
| const existingMessages = await dbGetMessagesByChatId(chatId); | |
| const dataMessage = [...existingMessages] | |
| .filter(m => m.role === 'data') | |
| .sort( | |
| (a, b) => | |
| new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime() | |
| )[0]; | |
| if (dataMessage) { | |
| // Update existing message content using direct db call since chat-db.ts doesn't have an updateMessage | |
| const { db } = await import('@/lib/db'); | |
| const { messages } = await import('@/lib/db/schema'); | |
| const { eq } = await import('drizzle-orm'); | |
| await db.update(messages) | |
| .set({ content: JSON.stringify(contextData) }) | |
| .where(eq(messages.id, dataMessage.id)); | |
| console.log('Drawing context updated for chat:', chatId, 'messageId:', dataMessage.id); | |
| return { success: true, messageId: dataMessage.id }; | |
| } |
🤖 Prompt for AI Agents
In `@lib/actions/chat.ts` around lines 188 - 205, The current update branch in
save/update logic uses dbGetMessagesByChatId and existingMessages.find(...)
which returns the first (oldest) 'data' message and can leave the UI showing the
last 'data' entry out-of-sync with getUIStateFromAIState; change the selection
to target the most recent 'data' message (e.g., pick the last matching element
by timestamp or id) or iterate and update all messages with role === 'data' so
the DB update against messages (used in the db.update(...) block) updates the
latest/each 'data' record rather than the first one found; update references:
dbGetMessagesByChatId, existingMessages, dataMessage, and the db.update(...)
call accordingly.
| attachments: jsonb('attachments'), | ||
| toolName: varchar('tool_name', { length: 100 }), | ||
| toolCallId: varchar('tool_call_id', {length: 100}), | ||
| type: varchar('type', { length: 50 }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider typing attachments to avoid any propagation.
Line 33 uses jsonb('attachments') without a type, which makes downstream typings less safe. If null isn’t meaningful, a typed default also removes null checks.
♻️ Suggested typing (adjust shape as needed)
- attachments: jsonb('attachments'),
+ attachments: jsonb('attachments').$type<unknown[]>().default([]),📝 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.
| attachments: jsonb('attachments'), | |
| toolName: varchar('tool_name', { length: 100 }), | |
| toolCallId: varchar('tool_call_id', {length: 100}), | |
| type: varchar('type', { length: 50 }) | |
| attachments: jsonb('attachments').$type<unknown[]>().default([]), | |
| toolName: varchar('tool_name', { length: 100 }), | |
| toolCallId: varchar('tool_call_id', {length: 100}), | |
| type: varchar('type', { length: 50 }) |
🤖 Prompt for AI Agents
In `@lib/db/schema.ts` around lines 33 - 36, The attachments column is declared
with jsonb('attachments') which yields an untyped any; define and use a concrete
type for the JSON shape (e.g., Attachment or Attachment[] depending on expected
payload) and pass it into jsonb to lock down typings (e.g.,
jsonb<Attachment[]>('attachments')), or if null is not meaningful set a typed
default value instead; update the schema declaration that references attachments
and any related uses of toolName/toolCallId/type to use the new Attachment type
for downstream safety and remove unnecessary null checks.
ngoiyaeric
added
help wanted
User description
This change enables users to upload GeoJSON files (.geojson, application/geo+json) directly through the chat interface. Uploaded data is automatically rendered on the map (both Mapbox and Google Maps) and the viewport is adjusted to frame the data's extent using Turf.js.
The implementation includes:
uploadedGeoJsonfield inMapDataContext.submitaction.MapDataUpdatercomponent that manages state synchronization and "fly to" behavior.geospatialToolto allow AI agents to return GeoJSON that is processed through the same pipeline.data-testidattributes.PR created automatically by Jules for task 11124993269366174034 started by @ngoiyaeric
PR Type
Enhancement
Description
Enable GeoJSON file uploads (.geojson, application/geo+json) through chat interface
Automatically render uploaded GeoJSON on maps with viewport framing using Turf.js
Persist GeoJSON layers in database for chat session reload capability
Allow AI agents to return GeoJSON through geospatial tool pipeline
Add MapDataUpdater component for state synchronization and map navigation
Diagram Walkthrough
File Walkthrough
10 files
Add uploadedGeoJson field to context updateImplement GeoJSON file parsing and upload pipelineAdd GeoJSON file type to upload input acceptSync uploadedGeoJson to server via updateDrawingContextAdd uploadedGeoJson array to MapData interfaceCreate headless component for GeoJSON context syncIntegrate MapDataUpdater for tool GeoJSON resultsRender uploaded GeoJSON layers on MapboxRender uploaded GeoJSON layers on Google MapsExtract and pass GeoJSON from geospatial tool results2 files
Add data-testid attribute to bot messageAdd data-testid attribute to user messageSummary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.