-
-
Notifications
You must be signed in to change notification settings - Fork 7
Merge Resolution Search Enhancement (f89e3f7) into Main #490
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 44 commits
6eeae28
160e082
ae085cf
73c385c
1508c04
c8c028b
e2c0615
43179e7
7d45e02
5623831
44e86b6
0a48018
6acfbe5
8ae549a
3495da1
ba36e58
6431b41
8a04d49
10ab3fe
250283e
6554775
e95a25b
f80c87f
c4278e9
813d264
c868dcd
3b5ed27
3984b9b
f45f687
67c26d5
a842df1
184f678
895bf37
dc345b9
86013ed
2ba1f9e
dda7a32
23a1d3f
885dbbe
dd812c1
00c2e1a
54d9d6e
c9bd9b2
9ba4dfd
b49f7ca
8e37c34
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 |
|---|---|---|
|
|
@@ -13,27 +13,25 @@ import { Spinner } from '@/components/ui/spinner' | |
| import { Section } from '@/components/section' | ||
| import { FollowupPanel } from '@/components/followup-panel' | ||
| import { inquire, researcher, taskManager, querySuggestor, resolutionSearch, type DrawnFeature } from '@/lib/agents' | ||
| // Removed import of useGeospatialToolMcp as it no longer exists and was incorrectly used here. | ||
| // The geospatialTool (if used by agents like researcher) now manages its own MCP client. | ||
| import { writer } from '@/lib/agents/writer' | ||
| import { saveChat, getSystemPrompt } from '@/lib/actions/chat' // Added getSystemPrompt | ||
| import { saveChat, getSystemPrompt } from '@/lib/actions/chat' | ||
| import { Chat, AIMessage } from '@/lib/types' | ||
| import { UserMessage } from '@/components/user-message' | ||
| import { BotMessage } from '@/components/message' | ||
| import { SearchSection } from '@/components/search-section' | ||
| import SearchRelated from '@/components/search-related' | ||
| import { GeoJsonLayer } from '@/components/map/geojson-layer' | ||
| import { ResolutionImage } from '@/components/resolution-image' | ||
| import { CopilotDisplay } from '@/components/copilot-display' | ||
| import RetrieveSection from '@/components/retrieve-section' | ||
| import { VideoSearchSection } from '@/components/video-search-section' | ||
| import { MapQueryHandler } from '@/components/map/map-query-handler' // Add this import | ||
| import { MapQueryHandler } from '@/components/map/map-query-handler' | ||
|
|
||
| // Define the type for related queries | ||
| type RelatedQueries = { | ||
| items: { query: string }[] | ||
| } | ||
|
|
||
| // Removed mcp parameter from submit, as geospatialTool now handles its client. | ||
| async function submit(formData?: FormData, skip?: boolean) { | ||
| 'use server' | ||
|
|
||
|
|
@@ -43,16 +41,17 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| const isCollapsed = createStreamableValue(false) | ||
|
|
||
| const action = formData?.get('action') as string; | ||
| const drawnFeaturesString = formData?.get('drawnFeatures') as string; | ||
| let drawnFeatures: DrawnFeature[] = []; | ||
| try { | ||
| drawnFeatures = drawnFeaturesString ? JSON.parse(drawnFeaturesString) : []; | ||
| } catch (e) { | ||
| console.error('Failed to parse drawnFeatures:', e); | ||
| } | ||
|
|
||
| if (action === 'resolution_search') { | ||
| const file = formData?.get('file') as File; | ||
| const timezone = (formData?.get('timezone') as string) || 'UTC'; | ||
| const drawnFeaturesString = formData?.get('drawnFeatures') as string; | ||
| let drawnFeatures: DrawnFeature[] = []; | ||
| try { | ||
| drawnFeatures = drawnFeaturesString ? JSON.parse(drawnFeaturesString) : []; | ||
| } catch (e) { | ||
| console.error('Failed to parse drawnFeatures:', e); | ||
| } | ||
|
|
||
| if (!file) { | ||
| throw new Error('No file provided for resolution search.'); | ||
|
|
@@ -61,7 +60,6 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| const buffer = await file.arrayBuffer(); | ||
| const dataUrl = `data:${file.type};base64,${Buffer.from(buffer).toString('base64')}`; | ||
|
|
||
| // Get the current messages, excluding tool-related ones. | ||
| const messages: CoreMessage[] = [...(aiState.get().messages as any[])].filter( | ||
| message => | ||
| message.role !== 'tool' && | ||
|
|
@@ -71,16 +69,12 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| message.type !== 'resolution_search_result' | ||
| ); | ||
|
|
||
| // The user's prompt for this action is static. | ||
| const userInput = 'Analyze this map view.'; | ||
|
|
||
| // Construct the multimodal content for the user message. | ||
| const content: CoreMessage['content'] = [ | ||
| { type: 'text', text: userInput }, | ||
| { type: 'image', image: dataUrl, mimeType: file.type } | ||
| ]; | ||
|
|
||
| // Add the new user message to the AI state. | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
|
|
@@ -90,12 +84,11 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| }); | ||
| messages.push({ role: 'user', content }); | ||
|
|
||
| // Create a streamable value for the summary. | ||
| const summaryStream = createStreamableValue<string>(''); | ||
| const groupeId = nanoid(); | ||
|
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. 🧹 Nitpick | 🔵 Trivial Consider renaming The variable name ✏️ Suggested rename- const groupeId = nanoid();
+ const groupId = nanoid();Apply the same change at line 203 and update all references. 🤖 Prompt for AI Agents |
||
|
|
||
| async function processResolutionSearch() { | ||
| try { | ||
| // Call the simplified agent, which now returns a stream. | ||
| const streamResult = await resolutionSearch(messages, timezone, drawnFeatures); | ||
|
|
||
| let fullSummary = ''; | ||
|
|
@@ -107,10 +100,17 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| } | ||
|
|
||
| const analysisResult = await streamResult.object; | ||
|
|
||
| // Mark the summary stream as done with the result. | ||
| summaryStream.done(analysisResult.summary || 'Analysis complete.'); | ||
|
|
||
| if (analysisResult.geoJson) { | ||
| uiStream.append( | ||
| <GeoJsonLayer | ||
| id={groupeId} | ||
| data={analysisResult.geoJson as FeatureCollection} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| messages.push({ role: 'assistant', content: analysisResult.summary || 'Analysis complete.' }); | ||
|
|
||
| const sanitizedMessages: CoreMessage[] = messages.map(m => { | ||
|
|
@@ -132,8 +132,6 @@ async function submit(formData?: FormData, skip?: boolean) { | |
|
|
||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||
|
|
||
| const groupeId = nanoid(); | ||
|
|
||
| aiState.done({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
|
|
@@ -147,7 +145,10 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| { | ||
| id: groupeId, | ||
| role: 'assistant', | ||
| content: JSON.stringify(analysisResult), | ||
| content: JSON.stringify({ | ||
| ...analysisResult, | ||
| image: dataUrl | ||
| }), | ||
|
Comment on lines
188
to
194
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. You’re storing the full This is particularly risky because static map screenshots can be large, and every resolution search will permanently embed the image bytes into chat history. SuggestionAvoid persisting base64 data URLs in chat history. Prefer storing:
Minimal improvement if you can’t add storage yet: store only the original capture parameters (provider, center/zoom/bounds) and re-fetch on render. Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this suggestion.
Comment on lines
+191
to
+194
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. 🧹 Nitpick | 🔵 Trivial Double JSON.stringify creates nested string encoding. The image data is stringified twice: once at line 173 and again as part of the outer ♻️ Alternative approach content: JSON.stringify({
...analysisResult,
- image: JSON.stringify({ mapbox: mapboxDataUrl, google: googleDataUrl })
+ image: { mapbox: mapboxDataUrl, google: googleDataUrl }
}),Then update the read side: - const imageData = analysisResult.image as string;
+ const imageData = analysisResult.image;
let mapboxSrc = '';
let googleSrc = '';
if (imageData) {
- try {
- const parsed = JSON.parse(imageData);
- mapboxSrc = parsed.mapbox || '';
- googleSrc = parsed.google || '';
- } catch (e) {
- mapboxSrc = imageData;
- }
+ if (typeof imageData === 'string') {
+ // Legacy format fallback
+ mapboxSrc = imageData;
+ } else {
+ mapboxSrc = imageData.mapbox || '';
+ googleSrc = imageData.google || '';
+ }
}🤖 Prompt for AI Agents |
||
| type: 'resolution_search_result' | ||
| }, | ||
| { | ||
|
|
@@ -173,12 +174,11 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| } | ||
|
Comment on lines
211
to
216
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. Missing In the 🐛 Proposed fix }
]
})
+ isGenerating.done(false)
+ uiStream.done()
} catch (error) {
console.error('Failed to process resolution search:', error);
summaryStream.done('An error occurred during analysis.');
isGenerating.done(false);
uiStream.done();
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| // Start the background process without awaiting it. | ||
| processResolutionSearch(); | ||
|
|
||
| // Immediately update the UI stream with the BotMessage component. | ||
| uiStream.update( | ||
| <Section title="response"> | ||
| <ResolutionImage src={dataUrl} /> | ||
| <BotMessage content={summaryStream.value} /> | ||
| </Section> | ||
| ); | ||
|
|
@@ -243,7 +243,6 @@ async function submit(formData?: FormData, skip?: boolean) { | |
|
|
||
| uiStream.append(answerSection); | ||
|
|
||
| const groupeId = nanoid(); | ||
| const relatedQueries = { items: [] }; | ||
|
|
||
| aiState.done({ | ||
|
|
@@ -327,7 +326,6 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| } | ||
|
|
||
| const hasImage = messageParts.some(part => part.type === 'image') | ||
| // Properly type the content based on whether it contains images | ||
| const content: CoreMessage['content'] = hasImage | ||
| ? messageParts as CoreMessage['content'] | ||
| : messageParts.map(part => part.text).join('\n') | ||
|
|
@@ -361,7 +359,6 @@ async function submit(formData?: FormData, skip?: boolean) { | |
|
|
||
| const userId = 'anonymous' | ||
| const currentSystemPrompt = (await getSystemPrompt(userId)) || '' | ||
|
|
||
| const mapProvider = formData?.get('mapProvider') as 'mapbox' | 'google' | ||
|
|
||
| async function processEvents() { | ||
|
|
@@ -410,7 +407,8 @@ async function submit(formData?: FormData, skip?: boolean) { | |
| streamText, | ||
| messages, | ||
| mapProvider, | ||
| useSpecificAPI | ||
| useSpecificAPI, | ||
| drawnFeatures | ||
| ) | ||
| answer = fullResponse | ||
| toolOutputs = toolResponses | ||
|
|
@@ -643,12 +641,10 @@ export const getUIStateFromAIState = (aiState: AIState): UIState => { | |
| case 'input_related': | ||
| let messageContent: string | any[] | ||
| try { | ||
| // For backward compatibility with old messages that stored a JSON string | ||
| const json = JSON.parse(content as string) | ||
| messageContent = | ||
| type === 'input' ? json.input : json.related_query | ||
| } catch (e) { | ||
| // New messages will store the content array or string directly | ||
| messageContent = content | ||
| } | ||
| return { | ||
|
|
@@ -669,8 +665,8 @@ export const getUIStateFromAIState = (aiState: AIState): UIState => { | |
| } | ||
| break | ||
| case 'assistant': | ||
| const answer = createStreamableValue() | ||
| answer.done(content) | ||
| const answer = createStreamableValue(content as string) | ||
| answer.done(content as string) | ||
| switch (type) { | ||
| case 'response': | ||
| return { | ||
|
|
@@ -682,7 +678,9 @@ export const getUIStateFromAIState = (aiState: AIState): UIState => { | |
| ) | ||
| } | ||
| case 'related': | ||
| const relatedQueries = createStreamableValue<RelatedQueries>() | ||
| const relatedQueries = createStreamableValue<RelatedQueries>({ | ||
| items: [] | ||
| }) | ||
| relatedQueries.done(JSON.parse(content as string)) | ||
| return { | ||
| id, | ||
|
|
@@ -704,11 +702,13 @@ export const getUIStateFromAIState = (aiState: AIState): UIState => { | |
| case 'resolution_search_result': { | ||
| const analysisResult = JSON.parse(content as string); | ||
| const geoJson = analysisResult.geoJson as FeatureCollection; | ||
| const image = analysisResult.image as string; | ||
|
|
||
| return { | ||
| id, | ||
| component: ( | ||
|
Comment on lines
771
to
774
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.
You already handle backward compatibility in the user message case; this should get similar protection. SuggestionWrap parsing with a guard and fail soft: case 'resolution_search_result': {
let analysisResult: any
try {
analysisResult = JSON.parse(content as string)
} catch {
return { id, component: null }
}
const geoJson = analysisResult.geoJson as FeatureCollection
const image = analysisResult.image as string
// ...
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion. |
||
| <> | ||
| {image && <ResolutionImage src={image} />} | ||
| {geoJson && ( | ||
| <GeoJsonLayer id={id} data={geoJson} /> | ||
| )} | ||
|
|
@@ -721,21 +721,37 @@ export const getUIStateFromAIState = (aiState: AIState): UIState => { | |
| case 'tool': | ||
| try { | ||
| const toolOutput = JSON.parse(content as string) | ||
| const isCollapsed = createStreamableValue() | ||
| const isCollapsed = createStreamableValue(true) | ||
| isCollapsed.done(true) | ||
|
|
||
| if ( | ||
| toolOutput.type === 'MAP_QUERY_TRIGGER' && | ||
| name === 'geospatialQueryTool' | ||
| ) { | ||
| const mapUrl = toolOutput.mcp_response?.mapUrl; | ||
| const placeName = toolOutput.mcp_response?.location?.place_name; | ||
|
|
||
| return { | ||
| id, | ||
| component: <MapQueryHandler toolOutput={toolOutput} />, | ||
| component: ( | ||
| <> | ||
| {mapUrl && ( | ||
| <ResolutionImage | ||
| src={mapUrl} | ||
| className="mb-0" | ||
| alt={placeName ? `Map of ${placeName}` : 'Map Preview'} | ||
| /> | ||
| )} | ||
| <MapQueryHandler toolOutput={toolOutput} /> | ||
| </> | ||
| ), | ||
| isCollapsed: false | ||
| } | ||
| } | ||
|
|
||
| const searchResults = createStreamableValue() | ||
| const searchResults = createStreamableValue( | ||
| JSON.stringify(toolOutput) | ||
| ) | ||
| searchResults.done(JSON.stringify(toolOutput)) | ||
| switch (name) { | ||
| case 'search': | ||
|
|
||
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.
drawnFeaturesis parsed at the top-level ofsubmit()even for actions that never need it. More importantly, the value is computed from client-provided JSON without any size/shape guardrails. A large payload (or maliciously large JSON) can increase server work and bloat logs/state.Even if you trust the source, it’s cheap to add a minimal cap (e.g., max length of the raw string / max number of features) and only parse when needed (or when present).
Suggestion
Consider lazy-parsing
drawnFeaturesonly when required (e.g., whenaction === 'resolution_search'or when you’re about to callresearcher(...)), and add basic guardrails:drawnFeaturesString.lengthexceeds a reasonable threshold (e.g., 50–200KB)drawnFeatures.length(e.g., max 200) and drop oversizedmeasurementstringsExample sketch:
Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this suggestion.