-
-
Notifications
You must be signed in to change notification settings - Fork 8
Restore and persist durable geospatial drawing context #683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,11 +14,14 @@ export interface SearchPageProps { | |
| } | ||
|
|
||
| export async function generateMetadata({ params }: SearchPageProps) { | ||
| const { id } = await params; // Keep as is for now | ||
| // TODO: Metadata generation might need authenticated user if chats are private | ||
| // For now, assuming getChat can be called or it handles anon access for metadata appropriately | ||
| const userId = await getCurrentUserIdOnServer(); // Attempt to get user for metadata | ||
| const chat = await getChat(id, userId || 'anonymous'); // Pass userId or 'anonymous' if none | ||
| const { id } = await params; | ||
| const userId = await getCurrentUserIdOnServer(); | ||
|
|
||
| if (!userId) { | ||
| return { title: 'Search' }; | ||
| } | ||
|
|
||
| const chat = await getChat(id, userId); | ||
| return { | ||
| title: chat?.title?.toString().slice(0, 50) || 'Search', | ||
| }; | ||
|
|
@@ -59,16 +62,32 @@ export default async function SearchPage({ params }: SearchPageProps) { | |
| }; | ||
| }); | ||
|
|
||
| // 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When that array payload (or another Suggested fix: select only drawing-context-shaped payloads (or support both array + object shapes defensively), and ideally align the write format so 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; |
||
|
|
||
| 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); | ||
| } | ||
| } | ||
|
Comment on lines
+65
to
+81
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ 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
Because the saved data is an array but the loader treats it as an object, 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 🤖 Prompt for AI Agents |
||
|
|
||
| return ( | ||
| <AI | ||
| initialAIState={{ | ||
| chatId: chat.id, | ||
| messages: initialMessages, // Use the transformed messages from the database | ||
| // isSharePage: true, // This was in PR#533, but share functionality is removed. | ||
| // If needed for styling or other logic, it can be set. | ||
| messages: initialMessages, | ||
| }} | ||
| > | ||
| <MapDataProvider> | ||
| <MapDataProvider initialData={initialMapData}> | ||
| <Chat id={id} /> | ||
| </MapDataProvider> | ||
| </AI> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -38,8 +38,8 @@ interface MapDataContextType { | |||||||||||||||||
|
|
||||||||||||||||||
| const MapDataContext = createContext<MapDataContextType | undefined>(undefined); | ||||||||||||||||||
|
|
||||||||||||||||||
| export const MapDataProvider: React.FC<{ children: ReactNode }> = ({ children }) => { | ||||||||||||||||||
| const [mapData, setMapData] = useState<MapData>({ drawnFeatures: [], markers: [] }); | ||||||||||||||||||
| export const MapDataProvider: React.FC<{ children: ReactNode; initialData?: MapData }> = ({ children, initialData }) => { | ||||||||||||||||||
| const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [] }); | ||||||||||||||||||
|
Comment on lines
+41
to
+42
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
When 🛡️ Proposed fix to preserve defaults- const [mapData, setMapData] = useState<MapData>(initialData || { drawnFeatures: [], markers: [] });
+ const [mapData, setMapData] = useState<MapData>({
+ drawnFeatures: [],
+ markers: [],
+ ...initialData,
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| return ( | ||||||||||||||||||
| <MapDataContext.Provider value={{ mapData, setMapData }}> | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid appending unchanged drawing snapshots.
Carousel re-triggers can submit the same
drawnFeaturesrepeatedly, and this stores a full duplicatedrawing_contexteach 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
📝 Committable suggestion
🤖 Prompt for AI Agents