-
-
Couldn't load subscription status.
- Fork 6
Implement Search Mode Selector and Agent Personas #328
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
base: main
Are you sure you want to change the base?
Conversation
- Adds a new UI component in the chat panel to allow users to select a search mode (Standard, Geospatial, or Web Search). - The selected mode is passed to the AI agent with each request. - The researcher agent now adopts a "persona" based on the selected mode, which dynamically adjusts its system prompt and restricts its available tools to improve relevance and accuracy. - When the geospatial tool returns multiple locations, they are now rendered as a list of clickable "fly to" links in the chat, allowing the user to navigate the map to each location.
|
👋 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 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.
|
|
|
WalkthroughAdds a user-selectable searchMode to the chat UI and form submission, threads it through app/actions to the researcher agent, and updates the researcher to use mode-specific prompts and tool sets. Introduces multi-location handling for map queries via a new LocationLinks component and refactors map-query-handler to support single vs. multiple locations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ChatPanel as Chat Panel
participant Actions as app/actions.tsx
participant Researcher as lib/agents/researcher.tsx
participant Tools as Tools Registry
User->>ChatPanel: Select searchMode + submit message
ChatPanel->>Actions: POST form (message, files, searchMode)
Actions->>Researcher: researcher({ ..., searchMode })
Researcher->>Researcher: Build systemPrompt based on searchMode
Researcher->>Tools: getTools()
Tools-->>Researcher: allTools
Researcher->>Researcher: Derive availableTools per searchMode
Researcher->>Researcher: nonexperimental_streamText(systemPrompt, availableTools)
Researcher-->>Actions: Streamed response (incl. tool calls/results)
Actions-->>ChatPanel: UI stream updates
ChatPanel-->>User: Render assistant output
sequenceDiagram
autonumber
participant ToolOut as Tool Output (mcp_response)
participant MapHandler as components/map/map-query-handler.tsx
participant LocationLinks as components/map/location-links.tsx
participant MapState as useMapData()
ToolOut-->>MapHandler: locations[] / location
alt Multiple locations
MapHandler->>LocationLinks: Render list (place_name)
User->>LocationLinks: Click location
LocationLinks->>MapState: set targetPosition [lon, lat], mapFeature.place_name
else Single valid location
MapHandler->>MapState: set targetPosition [lon, lat], mapFeature { place_name, mapUrl }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (5)
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:
|
||||||||||||||||
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.
- The chat panel uses invalid
idvalues with spaces, breaking label–input associations; fix to space-free ids. Also make theRadioGroupcontrolled and add accessible labels. dynamicSystemPromptis ignored in the researcher, likely a functional regression; integrate it with the persona prompt. Also avoidanyfor tools and consider not over-restricting tool availability.- Map handler no longer clears stale map state when no valid location is present; add a fallback reset. Duplicated
Locationtypes should be centralized. - Use stable React keys in
LocationLinksinstead of array indexes.
Additional notes (2)
-
Maintainability |
components/chat-panel.tsx:174-178
The RadioGroup is semi-controlled: you set local state viaonValueChange, but rely ondefaultValuerather than bindingvalue. This can lead to subtle UI state issues during re-renders or hydration. Prefer a fully controlled component by passingvalue={searchMode}and removingdefaultValue. -
Maintainability |
components/map/map-query-handler.tsx:7-11
Locationis defined in bothmap-query-handler.tsxandlocation-links.tsx. Duplicated types tend to drift over time. Centralizing the type improves maintainability.
Summary of changes
- Added search mode selection UI in
components/chat-panel.tsxusing aRadioGroupwith Standard, Geospatial, and Web Search options, and posted it via a hiddensearchModeinput. - Captured
searchModeinapp/actions.tsxfromFormDataand forwarded it to theresearcheragent. - Updated
lib/agents/researcher.tsxto introduce personas (Standard/Geospatial/Web Search), adjust the system prompt accordingly, and restrict available tools per persona. - Enhanced map behavior in
components/map/map-query-handler.tsxto handle multiple locations; when multiple are returned, rendersLocationLinksto let users "fly to" individual locations. - Introduced
components/map/location-links.tsxto render clickable links for multiple locations. - Minor refactor to file input ref handling in the chat panel.
| <RadioGroupItem value="Web Search" id="Web Search" className="sr-only" /> | ||
| <label | ||
| htmlFor="Web Search" | ||
| className={`cursor-pointer rounded-full px-3 py-1 text-sm ${ | ||
| searchMode === 'Web Search' ? 'bg-background text-foreground' : 'text-muted-foreground' | ||
| }`} | ||
| > | ||
| <Globe className="h-4 w-4" /> | ||
| </label> |
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.
id attributes must not contain spaces. Using id="Web Search" can break the label–input association and CSS selectors, leading to the labels not toggling the radio inputs reliably. This is a correctness bug in the new UI.
Suggestion
Use space-free ids and keep the display values as-is. Also bind value to state for consistency (see next comment). For example:
<RadioGroup
value={searchMode}
onValueChange={setSearchMode}
defaultValue="Standard"
className="flex bg-muted rounded-full p-1"
>
<RadioGroupItem value="Standard" id="standard" className="sr-only" />
<label htmlFor="standard" className={`cursor-pointer rounded-full px-3 py-1 text-sm ${searchMode === 'Standard' ? 'bg-background text-foreground' : 'text-muted-foreground'}`}>
<Search className="h-4 w-4" />
</label>
<RadioGroupItem value="Geospatial" id="geospatial" className="sr-only" />
<label htmlFor="geospatial" className={`cursor-pointer rounded-full px-3 py-1 text-sm ${searchMode === 'Geospatial' ? 'bg-background text-foreground' : 'text-muted-foreground'}`}>
<MapPin className="h-4 w-4" />
</label>
<RadioGroupItem value="Web Search" id="web-search" className="sr-only" />
<label htmlFor="web-search" className={`cursor-pointer rounded-full px-3 py-1 text-sm ${searchMode === 'Web Search' ? 'bg-background text-foreground' : 'text-muted-foreground'}`}>
<Globe className="h-4 w-4" />
</label>
</RadioGroup>Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| <label | ||
| htmlFor="Standard" | ||
| className={`cursor-pointer rounded-full px-3 py-1 text-sm ${ | ||
| searchMode === 'Standard' ? 'bg-background text-foreground' : 'text-muted-foreground' | ||
| }`} | ||
| > | ||
| <Search className="h-4 w-4" /> | ||
| </label> | ||
| <RadioGroupItem value="Geospatial" id="Geospatial" className="sr-only" /> | ||
| <label | ||
| htmlFor="Geospatial" | ||
| className={`cursor-pointer rounded-full px-3 py-1 text-sm ${ | ||
| searchMode === 'Geospatial' ? 'bg-background text-foreground' : 'text-muted-foreground' | ||
| }`} | ||
| > | ||
| <MapPin className="h-4 w-4" /> | ||
| </label> | ||
| <RadioGroupItem value="Web Search" id="Web Search" className="sr-only" /> | ||
| <label | ||
| htmlFor="Web Search" | ||
| className={`cursor-pointer rounded-full px-3 py-1 text-sm ${ | ||
| searchMode === 'Web Search' ? 'bg-background text-foreground' : 'text-muted-foreground' | ||
| }`} | ||
| > | ||
| <Globe className="h-4 w-4" /> | ||
| </label> |
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.
The icon-only labels lack accessible names. Screen-reader users won’t know what each control does. Provide accessible labels (e.g., aria-label) or include visually hidden text.
Suggestion
Add aria-label to each label or include an sr-only span:
<label htmlFor="standard" aria-label="Standard mode" className={...}>
<Search className="h-4 w-4" />
</label>
<label htmlFor="geospatial" aria-label="Geospatial mode" className={...}>
<MapPin className="h-4 w-4" />
</label>
<label htmlFor="web-search" aria-label="Web Search mode" className={...}>
<Globe className="h-4 w-4" />
</label>Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| let systemPrompt = `Current date and time: ${currentDate}. Match the language of your response to the user's language. Always aim to directly address the user's question. If using information from a tool (like web search), cite the source URL.`; | ||
|
|
||
| const standardPrompt = `As a comprehensive AI assistant, you can search the web, retrieve information from URLs, and understand geospatial queries to assist the user and display information on a map. When tools are not needed, provide direct, helpful answers based on your knowledge. | ||
| Tool Usage Guide: | ||
| - For general web searches: Use the 'search' tool. | ||
| - For retrieving content from specific URLs: Use the 'retrieve' tool. | ||
| - For any questions involving locations, places, or directions: You MUST use the 'geospatialQueryTool'.`; | ||
|
|
||
| const geospatialPrompt = `You are a specialized Geospatial AI assistant. Your primary function is to understand and respond to geospatial queries. You MUST prioritize using the 'geospatialQueryTool' for any questions involving locations, places, addresses, geographical features, businesses, points of interest, distances, or directions. Only use other tools if geospatial queries are not applicable.`; | ||
|
|
||
| const webSearchPrompt = `You are a specialized Web Search AI assistant. Your primary function is to search the web and retrieve information from URLs to answer user questions. You MUST prioritize using the 'search' and 'retrieve' tools. Only use other tools if web searches are not applicable.`; | ||
|
|
||
| switch (searchMode) { | ||
| case 'Geospatial': | ||
| systemPrompt = `${geospatialPrompt}\n${systemPrompt}`; | ||
| break; | ||
| case 'Web Search': | ||
| systemPrompt = `${webSearchPrompt}\n${systemPrompt}`; | ||
| break; | ||
| default: | ||
| systemPrompt = `${standardPrompt}\n${systemPrompt}`; | ||
| break; | ||
| } |
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.
dynamicSystemPrompt is now ignored. This is a functional regression from the previous implementation where a caller-provided system prompt could shape the agent’s behavior. The new persona logic should augment or allow override by dynamicSystemPrompt, not discard it.
Suggestion
Combine the persona prompt with dynamicSystemPrompt, giving the dynamic prompt precedence if present:
let systemPrompt = `Current date and time: ${currentDate}. Match the language of your response to the user's language. Always aim to directly address the user's question. If using information from a tool (like web search), cite the source URL.`;
// Build personaPrompt based on searchMode as you do now...
// If a dynamic system prompt is provided, prepend or override
if (dynamicSystemPrompt && dynamicSystemPrompt.trim() !== '') {
systemPrompt = `${dynamicSystemPrompt.trim()}\n\n${systemPrompt}`;
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| const allTools = getTools({ uiStream, fullResponse }); | ||
| let availableTools: any = allTools; | ||
|
|
||
| if (searchMode === 'Geospatial') { | ||
| availableTools = { geospatialQueryTool: allTools.geospatialQueryTool }; | ||
| } else if (searchMode === 'Web Search') { | ||
| availableTools = { search: allTools.search, retrieve: allTools.retrieve }; | ||
| } | ||
|
|
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.
Avoid any for availableTools. It weakens type safety and risks passing malformed tool definitions to the model at runtime. You can keep the correct shape with a Partial<typeof allTools> and narrow explicitly.
Suggestion
Preserve types by using Partial<typeof allTools> and explicit construction:
const allTools = getTools({ uiStream, fullResponse });
let availableTools: Partial<typeof allTools> = allTools;
if (searchMode === 'Geospatial') {
availableTools = { geospatialQueryTool: allTools.geospatialQueryTool };
} else if (searchMode === 'Web Search') {
availableTools = { search: allTools.search, retrieve: allTools.retrieve };
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| if (searchMode === 'Geospatial') { | ||
| availableTools = { geospatialQueryTool: allTools.geospatialQueryTool }; | ||
| } else if (searchMode === 'Web Search') { | ||
| availableTools = { search: allTools.search, retrieve: allTools.retrieve }; | ||
| } |
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.
Over-restricting tools by removing them entirely can degrade fallback behavior. For example, the Geospatial persona might still benefit from retrieve to cite sources. Consider keeping complementary tools available and relying on the persona prompt to prioritize usage, rather than hard removal.
Suggestion
Keep complementary tools available for personas while instructing prioritization via the prompt. For example, allow retrieve with Geospatial:
if (searchMode === 'Geospatial') {
availableTools = {
geospatialQueryTool: allTools.geospatialQueryTool,
retrieve: allTools.retrieve
};
}Alternatively, leave all tools available and rely solely on the persona prompt to guide selection.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| if (toolOutput && toolOutput.mcp_response) { | ||
| const { locations, location } = toolOutput.mcp_response; | ||
|
|
||
| if (typeof latitude === 'number' && typeof longitude === 'number') { | ||
| console.log(`MapQueryHandler: Received data from geospatialTool. Place: ${place_name}, Lat: ${latitude}, Lng: ${longitude}`); | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| // Ensure coordinates are in [lng, lat] format for MapboxGL | ||
| targetPosition: [longitude, latitude], | ||
| // Optionally store more info from mcp_response if needed by MapboxMap component later | ||
| mapFeature: { | ||
| place_name, | ||
| // Potentially add mapUrl or other details from toolOutput.mcp_response | ||
| mapUrl: toolOutput.mcp_response?.mapUrl | ||
| } | ||
| })); | ||
| if (locations && locations.length > 1) { | ||
| // Multiple locations: handled by LocationLinks, no automatic fly-to | ||
| } else { | ||
| console.warn("MapQueryHandler: Invalid latitude/longitude in toolOutput.mcp_response:", toolOutput.mcp_response.location); | ||
| // Clear target position if data is invalid | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| targetPosition: null, | ||
| mapFeature: null | ||
| })); | ||
| const singleLocation = locations && locations.length === 1 ? locations[0] : location; | ||
| if (singleLocation && typeof singleLocation.latitude === 'number' && typeof singleLocation.longitude === 'number') { | ||
| setMapData(prevData => ({ | ||
| ...prevData, | ||
| targetPosition: [singleLocation.longitude, singleLocation.latitude], | ||
| mapFeature: { | ||
| place_name: singleLocation.place_name, | ||
| mapUrl: toolOutput.mcp_response?.mapUrl | ||
| } | ||
| })); | ||
| } | ||
| } |
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.
When the tool output is invalid (no valid coordinates), the previous implementation cleared targetPosition/mapFeature. The new version silently does nothing, which can leave stale map state visible. Consider resetting the map state when no valid single location is present and there aren’t multiple locations to show.
Suggestion
Add a fallback reset when no valid single location is available and there aren’t multiple locations:
if (locations && locations.length > 1) {
// handled by LocationLinks
} else {
const singleLocation = locations && locations.length === 1 ? locations[0] : location;
if (singleLocation && typeof singleLocation.latitude === 'number' && typeof singleLocation.longitude === 'number') {
setMapData(prevData => ({
...prevData,
targetPosition: [singleLocation.longitude, singleLocation.latitude],
mapFeature: {
place_name: singleLocation.place_name,
mapUrl: toolOutput.mcp_response?.mapUrl
}
}));
} else {
setMapData(prevData => ({ ...prevData, targetPosition: null, mapFeature: null }));
}
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| <Button | ||
| key={index} | ||
| variant="outline" | ||
| className="justify-start" | ||
| onClick={() => handleFlyTo(location)} | ||
| > | ||
| <MapPin className="mr-2 h-4 w-4" /> | ||
| {location.place_name} | ||
| </Button> |
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.
Using the array index as a React key can cause unnecessary re-renders or state mismatches if the list changes. Even if the list is static per response, a stable key is preferable.
Suggestion
Use a stable key derived from the location itself, e.g., coordinates or place_name:
{locations.map((location) => (
<Button
key={`${location.longitude},${location.latitude}`}
variant="outline"
className="justify-start"
onClick={() => handleFlyTo(location)}
>
<MapPin className="mr-2 h-4 w-4" />
{location.place_name}
</Button>
))}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| const searchMode = formData?.get('searchMode') as string || 'Standard' | ||
|
|
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.
You’re trusting the client-provided searchMode string without validation. This allows unexpected values to flow into your agent logic and could lead to inconsistent prompts/tool availability. Validate and normalize it to a known set of modes before use.
Suggestion
Validate the value before using it:
const rawSearchMode = (formData?.get('searchMode') as string) ?? 'Standard'
const allowedModes = new Set(['Standard', 'Geospatial', 'Web Search'])
const searchMode = allowedModes.has(rawSearchMode) ? rawSearchMode : 'Standard'Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
User description
This feature introduces a search mode selector that allows users to guide the AI agent's tool usage. By selecting a persona like "Geospatial" or "Web Search," the agent will prioritize relevant tools, leading to more accurate and context-aware responses. The update also enhances the map interaction by displaying multiple locations as a list of "fly to" links.
PR created automatically by Jules for task 9273094999750174910
PR Type
Enhancement, Bug fix
Description
Adds search mode selector UI with three personas: Standard, Geospatial, and Web Search
Implements dynamic system prompts and tool filtering based on selected persona
Enhances geospatial tool to support multiple locations with clickable "fly to" links
Refactors map query handler to process both single and multiple location responses
Diagram Walkthrough
File Walkthrough
chat-panel.tsx
Add search mode selector UI componentcomponents/chat-panel.tsx
Geospatial, Web Search)
actions.tsx
Pass search mode to researcher agentapp/actions.tsx
researcher.tsx
Implement agent personas with dynamic prompts and tool filteringlib/agents/researcher.tsx
only geospatialQueryTool, Web Search uses search/retrieve)
map-query-handler.tsx
Support multiple locations in map query responsescomponents/map/map-query-handler.tsx
location-links.tsx
Add location links component for multi-location displaycomponents/map/location-links.tsx
buttons
Summary by CodeRabbit