Restore and persist durable geospatial drawing context#683
Conversation
- Restored drawing and geospatial context to resolution carousel re-trigger in `components/resolution-carousel.tsx`. - Implemented durable drawing context in `app/actions.tsx` by capturing `drawing_context` messages in AI state and merging features across turns. - Seeded `MapDataProvider` from persisted state in `app/search/[id]/page.tsx` for hydrated chat reloads. - Resolved nested provider shadowing in `components/chat.tsx` to ensure consistent map state resolution. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
|
|
Caution Review failedPull request was closed or merged during review WalkthroughThe PR persists drawing history in AI state, restores saved map data into the search UI, forwards map context into QCX analysis, and aligns anonymous-user handling across search, chat, and auth helpers. ChangesMap state persistence and anonymous-user handling
Sequence Diagram(s)sequenceDiagram
participant SearchPage
participant MapDataProvider
participant Chat
participant ResolutionCarousel
SearchPage->>MapDataProvider: initialData from latest 'data' message
MapDataProvider->>Chat: mapData via useMapData
MapDataProvider->>ResolutionCarousel: mapData via useMapData
ResolutionCarousel->>ResolutionCarousel: build QCX-TERRA FormData
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
|
2026-06-26 10:05:56.040 [error] updateDrawingContext: Could not get current user ID. |
The resolution search completely breaks the code. These are the errors. |
There was a problem hiding this comment.
Blocking feedback
- Reload hydration can pick a non-drawing
datapayload and clear map context — app/search/[id]/page.tsx#L63-L74
If you'd like me to push fixes, reply with the item number (for example: please fix 1).
| // PERSISTENCE: Seed map context from latest persisted drawing data | ||
| const latestDrawingContext = [...dbMessages] | ||
| .reverse() | ||
| .find(m => m.role === 'data'); |
There was a problem hiding this comment.
latestDrawingContext currently assumes every latest role: "data" message has the { drawnFeatures, cameraState } shape, but this PR also writes drawing_context entries as raw arrays in app/actions.tsx (content: JSON.stringify(drawnFeatures)).
When that array payload (or another data payload like calendar metadata) is newest, this parse path falls back to drawnFeatures: [] and missing cameraState, so reopened chats lose hydrated map context.
Suggested fix: select only drawing-context-shaped payloads (or support both array + object shapes defensively), and ideally align the write format so role: "data" messages use one schema.
For example:
const latestDrawingContext = [...dbMessages].reverse().find((m) => {
if (m.role !== 'data') return false;
try {
const parsed = JSON.parse(m.content as string);
return Array.isArray(parsed) || Array.isArray(parsed?.drawnFeatures);
} catch {
return false;
}
});
const parsed = latestDrawingContext ? JSON.parse(latestDrawingContext.content as string) : null;
const drawnFeatures = Array.isArray(parsed) ? parsed : parsed?.drawnFeatures ?? [];
const cameraState = Array.isArray(parsed) ? undefined : parsed?.cameraState;There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/actions.tsx`:
- Around line 69-83: The historical drawing merge in the action that builds
mergedDrawnFeatures is currently first-wins, so older snapshots override newer
ones for duplicate feature IDs. Update the merge logic in app/actions.tsx so
historicalDrawingContexts are applied in chronological order with later
snapshots replacing earlier entries for the same id, then apply the current
drawnFeatures last so the active request wins over all history. Use the existing
mergedDrawnFeatures / historicalDrawingContexts flow and ensure resolutionSearch
and researcher receive the latest geometry and measurements for each
DrawnFeature.
- Line 393: The `drawing_context` filter is only applied on the main `messages`
path, so the fallback that rebuilds from `aiState.get().messages` can still pass
those entries into `writer`. Update the fallback filtering in `writer`/the
specific-API path so it also excludes messages with `role: 'data'` and `type:
'drawing_context'`, matching the existing message filtering logic in
`actions.tsx`.
- Around line 53-66: The drawing context persistence in the aiState.update block
is appending duplicate drawing snapshots whenever drawnFeatures repeats, which
inflates history and parsing cost. Add a guard before pushing the new message in
app/actions.tsx that compares the current drawnFeatures payload against the most
recent drawing_context entry in aiState.get().messages, and skip the update when
the serialized geometry is unchanged. Keep the fix localized around the
drawnFeatures append logic and the aiState.get()/aiState.update flow.
In `@app/search/`[id]/page.tsx:
- Around line 62-78: The map rehydration logic is parsing a persisted payload
with the wrong shape: latestDrawingContext in app/search/[id]/page.tsx expects
an object with drawnFeatures and cameraState, but the data written from
app/actions.tsx is a bare array. Update the persistence and loading flow to use
the same schema end-to-end, and in the page.tsx parsing path keep using the
latestDrawingContext/content but read the fields from the actual saved object
shape so initialMapData is populated correctly.
In `@components/map/map-data-context.tsx`:
- Around line 41-42: MapDataProvider is passing initialData through verbatim, so
MapData.markers can stay undefined when app/search/[id]/page.tsx omits it.
Update the MapDataProvider initialization to merge initialData with the default
MapData shape, ensuring markers (and any other missing fields) always fall back
to an empty array. Use the MapDataProvider and its useState<MapData> setup to
locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad500b55-f130-4703-a7cb-762447089292
📒 Files selected for processing (5)
app/actions.tsxapp/search/[id]/page.tsxcomponents/chat.tsxcomponents/map/map-data-context.tsxcomponents/resolution-carousel.tsx
📜 Review details
🔇 Additional comments (4)
components/resolution-carousel.tsx (1)
57-64: 🎯 Functional CorrectnessThis issue is no longer valid.
The
MapDatainterface inmap-data-context.tsxexplicitly definescurrentTimezone?: string,targetPosition?: { lat: number; lng: number }, andcameraState?: { center: { lat: number; lng: number } }. BothcameraState.centerandtargetPositionshare the expected{ lat, lng }shape, making the fallback logic and subsequent property accesses type-safe.> Likely an incorrect or invalid review comment.components/chat.tsx (1)
140-140: 🩺 Stability & AvailabilityNo issue: All
Chatrender sites are correctly wrapped inMapDataProvider.Verification confirms
Chatis only rendered inapp/page.tsx, which wraps it inMapDataProvider. No other direct render sites exist that could cause runtime errors from missing provider context. The original concern about unprovidered render paths is no longer valid.> Likely an incorrect or invalid review comment.app/actions.tsx (2)
121-129: LGTM!Also applies to: 151-151
576-576: LGTM!
| // PERSISTENCE: Always append drawing_context for durable feature history | ||
| if (drawnFeatures.length > 0) { | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
| ...aiState.get().messages, | ||
| { | ||
| id: nanoid(), | ||
| role: 'data', | ||
| content: JSON.stringify(drawnFeatures), | ||
| type: 'drawing_context' | ||
| } | ||
| ] | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid appending unchanged drawing snapshots.
Carousel re-triggers can submit the same drawnFeatures repeatedly, and this stores a full duplicate drawing_context each time. Since Line 71 later scans every historical context, large geometries will grow AI state and parsing cost unnecessarily.
Proposed guard for unchanged drawing context
- if (drawnFeatures.length > 0) {
+ const serializedDrawnFeatures =
+ drawnFeatures.length > 0 ? JSON.stringify(drawnFeatures) : ''
+ const latestDrawingContext = [...aiState.get().messages]
+ .reverse()
+ .find(m => m.type === 'drawing_context')
+
+ if (
+ serializedDrawnFeatures &&
+ latestDrawingContext?.content !== serializedDrawnFeatures
+ ) {
aiState.update({
...aiState.get(),
messages: [
...aiState.get().messages,
{
id: nanoid(),
role: 'data',
- content: JSON.stringify(drawnFeatures),
+ content: serializedDrawnFeatures,
type: 'drawing_context'
}
]
});
}📝 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.
| // PERSISTENCE: Always append drawing_context for durable feature history | |
| if (drawnFeatures.length > 0) { | |
| aiState.update({ | |
| ...aiState.get(), | |
| messages: [ | |
| ...aiState.get().messages, | |
| { | |
| id: nanoid(), | |
| role: 'data', | |
| content: JSON.stringify(drawnFeatures), | |
| type: 'drawing_context' | |
| } | |
| ] | |
| }); | |
| // PERSISTENCE: Always append drawing_context for durable feature history | |
| const serializedDrawnFeatures = | |
| drawnFeatures.length > 0 ? JSON.stringify(drawnFeatures) : '' | |
| const latestDrawingContext = [...aiState.get().messages] | |
| .reverse() | |
| .find(m => m.type === 'drawing_context') | |
| if ( | |
| serializedDrawnFeatures && | |
| latestDrawingContext?.content !== serializedDrawnFeatures | |
| ) { | |
| aiState.update({ | |
| ...aiState.get(), | |
| messages: [ | |
| ...aiState.get().messages, | |
| { | |
| id: nanoid(), | |
| role: 'data', | |
| content: serializedDrawnFeatures, | |
| type: 'drawing_context' | |
| } | |
| ] | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/actions.tsx` around lines 53 - 66, The drawing context persistence in the
aiState.update block is appending duplicate drawing snapshots whenever
drawnFeatures repeats, which inflates history and parsing cost. Add a guard
before pushing the new message in app/actions.tsx that compares the current
drawnFeatures payload against the most recent drawing_context entry in
aiState.get().messages, and skip the update when the serialized geometry is
unchanged. Keep the fix localized around the drawnFeatures append logic and the
aiState.get()/aiState.update flow.
| // PERSISTENCE: Build merged drawing set from all historical drawing_context messages | ||
| const mergedDrawnFeatures = [...drawnFeatures]; | ||
| const historicalDrawingContexts = aiState.get().messages.filter(m => m.type === 'drawing_context'); | ||
| historicalDrawingContexts.forEach(m => { | ||
| try { | ||
| const historicalFeatures = JSON.parse(m.content as string) as DrawnFeature[]; | ||
| historicalFeatures.forEach(hf => { | ||
| if (!mergedDrawnFeatures.some(f => f.id === hf.id)) { | ||
| mergedDrawnFeatures.push(hf); | ||
| } | ||
| }); | ||
| } catch (e) { | ||
| console.error('Failed to parse historical drawing context:', e); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make historical feature merging latest-wins.
When drawnFeatures is empty or stale, duplicate feature IDs from historical contexts currently keep the earliest geometry/measurement. That can send outdated drawing context to resolutionSearch/researcher; historical contexts should be applied in order with later snapshots overriding earlier ones, then the current request overriding both.
Proposed latest-wins merge
- const mergedDrawnFeatures = [...drawnFeatures];
+ const featureById = new Map<string, DrawnFeature>();
const historicalDrawingContexts = aiState.get().messages.filter(m => m.type === 'drawing_context');
historicalDrawingContexts.forEach(m => {
try {
const historicalFeatures = JSON.parse(m.content as string) as DrawnFeature[];
- historicalFeatures.forEach(hf => {
- if (!mergedDrawnFeatures.some(f => f.id === hf.id)) {
- mergedDrawnFeatures.push(hf);
- }
- });
+ if (Array.isArray(historicalFeatures)) {
+ historicalFeatures.forEach(hf => {
+ if (hf?.id) featureById.set(hf.id, hf);
+ });
+ }
} catch (e) {
console.error('Failed to parse historical drawing context:', e);
}
});
+ drawnFeatures.forEach(f => {
+ if (f?.id) featureById.set(f.id, f);
+ });
+ const mergedDrawnFeatures = Array.from(featureById.values());📝 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.
| // PERSISTENCE: Build merged drawing set from all historical drawing_context messages | |
| const mergedDrawnFeatures = [...drawnFeatures]; | |
| const historicalDrawingContexts = aiState.get().messages.filter(m => m.type === 'drawing_context'); | |
| historicalDrawingContexts.forEach(m => { | |
| try { | |
| const historicalFeatures = JSON.parse(m.content as string) as DrawnFeature[]; | |
| historicalFeatures.forEach(hf => { | |
| if (!mergedDrawnFeatures.some(f => f.id === hf.id)) { | |
| mergedDrawnFeatures.push(hf); | |
| } | |
| }); | |
| } catch (e) { | |
| console.error('Failed to parse historical drawing context:', e); | |
| } | |
| }); | |
| // PERSISTENCE: Build merged drawing set from all historical drawing_context messages | |
| const featureById = new Map<string, DrawnFeature>(); | |
| const historicalDrawingContexts = aiState.get().messages.filter(m => m.type === 'drawing_context'); | |
| historicalDrawingContexts.forEach(m => { | |
| try { | |
| const historicalFeatures = JSON.parse(m.content as string) as DrawnFeature[]; | |
| if (Array.isArray(historicalFeatures)) { | |
| historicalFeatures.forEach(hf => { | |
| if (hf?.id) featureById.set(hf.id, hf); | |
| }); | |
| } | |
| } catch (e) { | |
| console.error('Failed to parse historical drawing context:', e); | |
| } | |
| }); | |
| drawnFeatures.forEach(f => { | |
| if (f?.id) featureById.set(f.id, f); | |
| }); | |
| const mergedDrawnFeatures = Array.from(featureById.values()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/actions.tsx` around lines 69 - 83, The historical drawing merge in the
action that builds mergedDrawnFeatures is currently first-wins, so older
snapshots override newer ones for duplicate feature IDs. Update the merge logic
in app/actions.tsx so historicalDrawingContexts are applied in chronological
order with later snapshots replacing earlier entries for the same id, then apply
the current drawnFeatures last so the active request wins over all history. Use
the existing mergedDrawnFeatures / historicalDrawingContexts flow and ensure
resolutionSearch and researcher receive the latest geometry and measurements for
each DrawnFeature.
| // PERSISTENCE: Seed map context from latest persisted drawing data | ||
| const latestDrawingContext = [...dbMessages] | ||
| .reverse() | ||
| .find(m => m.role === 'data'); | ||
|
|
||
| let initialMapData = undefined; | ||
| if (latestDrawingContext) { | ||
| try { | ||
| const parsed = JSON.parse(latestDrawingContext.content as string); | ||
| initialMapData = { | ||
| drawnFeatures: parsed.drawnFeatures || [], | ||
| cameraState: parsed.cameraState, | ||
| }; | ||
| } catch (e) { | ||
| console.error('Failed to parse latest drawing context:', e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how drawing_context messages are created/persisted: role + type used.
rg -nP -C4 "drawing_context" app/actions.tsx
# Inspect updateDrawingContext server action and its persisted role
rg -nP -C6 "updateDrawingContext" --type=ts --type=tsx
# Confirm role values written to the messages table
rg -nP -C4 "role:\s*'data'|role:\s*\"data\"" --type=ts --type=tsxRepository: QueueLab/QCX
Length of output: 1817
Mismatched data structure prevents map rehydration
While the role is correctly set to 'data' in app/actions.tsx (line 61), the data shape saved differs from what app/search/[id]/page.tsx expects to read.
- Persistence (
app/actions.tsxline 62): SavescontentasJSON.stringify(drawnFeatures), which is a bare array. - Rehydration (
app/search/[id]/page.tsxlines 70–74): Parses the content and expects an object with adrawnFeatureskey (parsed.drawnFeatures).
Because the saved data is an array but the loader treats it as an object, parsed.drawnFeatures evaluates to undefined, and the map defaults to an empty list on reload.
Align the structures so persistence stores the same object shape used for seeding:
// In app/actions.tsx (line 62)
- content: JSON.stringify(drawnFeatures),
+ content: JSON.stringify({ drawnFeatures, cameraState: ... }), // Include cameraState if needed
// In app/search/[id]/page.tsx (lines 70-72)
- const parsed = JSON.parse(latestDrawingContext.content as string);
- initialMapData = { drawnFeatures: parsed.drawnFeatures || [], ... };
+ const parsed = JSON.parse(latestDrawingContext.content as string) as any[];
+ initialMapData = { drawnFeatures: parsed, ... };
// OR update the save side to match the load sideEnsure the content schema is consistent end-to-end to restore persistence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/search/`[id]/page.tsx around lines 62 - 78, The map rehydration logic is
parsing a persisted payload with the wrong shape: latestDrawingContext in
app/search/[id]/page.tsx expects an object with drawnFeatures and cameraState,
but the data written from app/actions.tsx is a bare array. Update the
persistence and loading flow to use the same schema end-to-end, and in the
page.tsx parsing path keep using the latestDrawingContext/content but read the
fields from the actual saved object shape so initialMapData is populated
correctly.
| export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => { | ||
| const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [] }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
initialData may drop markers, leaving mapData.markers undefined after reload.
When initialData is provided (from app/search/[id]/page.tsx, which builds { drawnFeatures, cameraState } only), it is used verbatim, so markers is never defaulted to []. Any consumer doing mapData.markers.map(...) will fail after a chat reload, unlike the no-initialData path which guarantees markers: [].
🛡️ Proposed fix to preserve defaults
- const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [] });
+ const [mapData, setMapData] = useState<MapData>({
+ drawnFeatures: [],
+ markers: [],
+ ...initialData,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => { | |
| const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [] }); | |
| export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => { | |
| const [mapData, setMapData] = useState<MapData>({ | |
| drawnFeatures: [], | |
| markers: [], | |
| ...initialData, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/map/map-data-context.tsx` around lines 41 - 42, MapDataProvider is
passing initialData through verbatim, so MapData.markers can stay undefined when
app/search/[id]/page.tsx omits it. Update the MapDataProvider initialization to
merge initialData with the default MapData shape, ensuring markers (and any
other missing fields) always fall back to an empty array. Use the
MapDataProvider and its useState<MapData> setup to locate the fix.
I have implemented several fixes to address these errors:
|
Resolved. See the summary of fixes in my reply to the previous comment. These changes address the UUID validation issues and the async cookie handling requirements in Next.js 15. |
…DB/Auth issues - Restored drawing and geospatial context to resolution carousel re-trigger in `components/resolution-carousel.tsx`. - Implemented durable drawing context in `app/actions.tsx` by capturing `drawing_context` messages in AI state and merging features across turns. - Seeded `MapDataProvider` from persisted state in `app/search/[id]/page.tsx` for hydrated chat reloads. - Resolved nested provider shadowing in `components/chat.tsx` to ensure consistent map state resolution. - Fixed `invalid input syntax for type uuid` by validating against 'anonymous' userId in `lib/actions/chat.ts` and `lib/actions/chat-db.ts`. - Fixed `TypeError: Cannot read properties of undefined (reading 'searchParams')` by correctly awaiting `cookies()` in `lib/auth/get-current-user.ts`. - Optimized agent imports in `app/actions.tsx` to avoid circular dependencies. - Ensured `sanitizedHistory` (with image placeholders) is saved to the database. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
This PR restores and persists the durable geospatial drawing context across the application.
Key changes:
components/resolution-carousel.tsxnow captures and sendsdrawnFeatures,latitude,longitude, andtimezonewhen re-triggering analysis, matching the behavior of the initial search.app/actions.tsxnow storesdrawing_contextmessages inaiStateand merges features from all historical context messages before invoking agents, ensuring a cumulative view of all drawn features.app/search/[id]/page.tsxnow seeds theMapDataProviderwith the latest persisted drawing data when a chat is reopened.MapDataProviderwrappers incomponents/chat.tsxwere removed to ensureuseMapData()resolves to the correctly hydrated provider.PR created automatically by Jules for task 10282829163014573234 started by @ngoiyaeric
Summary by CodeRabbit