Fix: Resolve React Hook usage error in getTools #195
Conversation
Refactored the `getTools` function and its callers to correctly handle the `useGeospatialToolMcp` hook, preventing it from being called in a non-React component context. - Modified `getTools` to accept `mcp` as a parameter. - Updated `researcher` function to accept and pass `mcp`. - Updated components calling the `submit` action to fetch `mcp` using the hook and pass it down. - Made `mcp` optional in `ToolProps` as not all tools require it. The build still fails due to an unresolved import for `createStreamableValue` when its usage is included, and a missing `DATABASE_URL` (which is being disregarded per user instruction). The primary React Hook linting error is resolved.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a new optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UIComponent
participant useGeospatialToolMcp
participant submit
participant researcher
participant getTools
User->>UIComponent: Submit form
UIComponent->>useGeospatialToolMcp: Obtain mcp
UIComponent->>submit: submit(formData, skip, mcp)
submit->>researcher: researcher(..., mcp)
researcher->>getTools: getTools({..., mcp})
getTools-->>researcher: Tools configured with mcp
researcher-->>submit: Research results
submit-->>UIComponent: Submission response
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
PR Reviewer Guide 🔍(Review updated until commit fde6685)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
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: 0
🔭 Outside diff range comments (1)
lib/agents/tools/geospatial.tsx (1)
14-17: Security risk: Hardcoded API credentials exposed in source code.The API key is hardcoded and exposed in the source code, which is a security vulnerability. The commented environment variable approach should be used instead.
Apply this diff to use environment variables:
export function useGeospatialToolMcp() { - const mcpServerUrl = - 'https://server.smithery.ai/mapbox-mcp-server/mcp?api_key=705b0222-a657-4cd2-b180-80c406cf6179&profile=smooth-lemur-vfUbUE'; - // Alternative: Dynamic URL (uncomment if using environment variables) - // const mcpServerUrl = `https://server.smithery.ai/@ngoiyaeric/mapbox-mcp-server/mcp?profile=${process.env.SMITHERY_PROFILE_ID}&api_key=${process.env.SMITHERY_API_KEY}`; + const mcpServerUrl = `https://server.smithery.ai/@ngoiyaeric/mapbox-mcp-server/mcp?profile=${process.env.SMITHERY_PROFILE_ID}&api_key=${process.env.SMITHERY_API_KEY}`;
🧹 Nitpick comments (7)
lib/agents/tools/geospatial.tsx (1)
1-1: Unnecessary comment addition.The comment clarification is redundant since the import path is self-explanatory.
components/chat-panel.tsx (1)
24-24: Consider conditional hook usage for better performance.The hook is called unconditionally on every render, which establishes an MCP connection even when geospatial functionality may not be needed. Consider adding a flag or context to conditionally enable geospatial tools.
components/search-related.tsx (1)
26-26: Consider centralized MCP management for better architecture.Since all UI components now call
useGeospatialToolMcp(), consider implementing a context provider or higher-order component to manage the MCP connection centrally. This would reduce duplication and provide better control over when geospatial functionality is available.For example, you could create a
GeospatialProvider:const GeospatialContext = createContext<ReturnType<typeof useMcp> | null>(null); export function GeospatialProvider({ children }) { const mcp = useGeospatialToolMcp(); return ( <GeospatialContext.Provider value={mcp}> {children} </GeospatialContext.Provider> ); } export function useGeospatialContext() { return useContext(GeospatialContext); }app/actions.tsx (2)
15-15: Remove unused import.The
useGeospatialToolMcpimport is not used in this file since the hook is now called in React components rather than in server actions.-import { useGeospatialToolMcp } from '@/lib/agents/tools/geospatial'; // Added import for the hook
33-33: Consider improving type safety for the mcp parameter.While the parameter passing approach correctly fixes the React Hook usage error, consider defining a proper type interface for
mcpinstead of usinganyto improve type safety and code maintainability.-async function submit(formData?: FormData, skip?: boolean, mcp?: any) { // Added mcp as a parameter +async function submit(formData?: FormData, skip?: boolean, mcp?: GeospatialToolMcp) { // Added mcp as a parameterlib/agents/researcher.tsx (1)
19-19: Consider improving type safety for the mcp parameter.The parameter addition correctly fixes the React Hook usage error. However, consider using a more specific type instead of
anyfor better type safety.- mcp: any, // Moved mcp before optional parameter + mcp: GeospatialToolMcp, // Moved mcp before optional parameterlib/agents/tools/index.tsx (1)
13-13: Excellent fix for the React Hook usage error.The refactor successfully addresses the React Hook rules violation by:
- Accepting
mcpas a parameter instead of callinguseGeospatialToolMcp()internally- Using the passed
mcpvalue directly in the geospatial tool configurationThis maintains the same functionality while ensuring hooks are only called in React components.
Consider improving type safety by replacing
anywith a specific type:-export const getTools = ({ uiStream, fullResponse, mcp }: ToolProps) => { +export const getTools = ({ uiStream, fullResponse, mcp }: ToolProps) => {And update the interface:
- mcp?: any // Made mcp optional + mcp?: GeospatialToolMcp // Made mcp optionalAlso applies to: 27-27
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/actions.tsx(3 hunks)components/chat-panel.tsx(3 hunks)components/copilot.tsx(3 hunks)components/followup-panel.tsx(2 hunks)components/search-related.tsx(3 hunks)lib/agents/researcher.tsx(2 hunks)lib/agents/tools/geospatial.tsx(1 hunks)lib/agents/tools/index.tsx(2 hunks)
🔇 Additional comments (7)
components/chat-panel.tsx (1)
7-7: LGTM! Hook usage correctly moved to component level.The changes properly fix the React Hook usage error by moving
useGeospatialToolMcpto the component level and passingmcpthrough the call chain. This ensures hooks are only called from React components.Also applies to: 24-24, 61-61
components/followup-panel.tsx (1)
7-7: LGTM! Consistent hook usage pattern implemented.The implementation correctly follows the same pattern as other components, properly moving the hook call to the component level to fix the React Hook usage error.
Also applies to: 15-15, 28-28
components/copilot.tsx (1)
11-11: LGTM! Consistent implementation across components.The changes maintain consistency with other components in fixing the React Hook usage error. The hook is properly called at the component level and
mcpis correctly passed to the submit function.Also applies to: 35-35, 82-82
components/search-related.tsx (1)
14-14: LGTM! Final component completes consistent pattern.This component correctly implements the same pattern as the other UI components, completing the fix for the React Hook usage error across the application.
Also applies to: 26-26, 49-49
app/actions.tsx (1)
149-149: LGTM: Parameter properly passed through the call chain.The
mcpparameter is correctly passed to theresearcherfunction, maintaining the flow from UI components through the submission pipeline.lib/agents/researcher.tsx (1)
59-59: LGTM: Parameter correctly forwarded to getTools.The
mcpparameter is properly passed to thegetToolsfunction, completing the parameter flow from the UI components.lib/agents/tools/index.tsx (1)
10-10: LGTM: Making mcp optional is the right approach.Making
mcpoptional in theToolPropsinterface makes sense since not all tools require geospatial functionality. This provides good flexibility for the tools system.
![codiumai-pr-agent-free[bot]](https://avatars.githubusercontent.com/in/354216?s=60&v=4){kind=small}
![qodo-merge-pro[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
@coderabbitai the geospatial mcp is still not working |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll investigate the geospatial MCP implementation to identify why it's still not working. Let me examine the code structure and flow. 🏁 Script executed: Length of output: 16853 Let me examine more specific details about how the parameters are being passed and check for type mismatches: 🏁 Script executed: Length of output: 3923 Now I can see the issues! There are several problems with the current implementation: 🏁 Script executed: Length of output: 2077 Now I can see the issues! There are several problems with the current implementation: @ngoiyaeric I found several issues with the geospatial MCP implementation: Problem 1: Parameter MismatchThe Current code in geospatialQueryTool: geospatialTool({
uiStream,
fullResponse, // ❌ This parameter is not accepted by geospatialTool
mcp
})Expected by export const geospatialTool = ({ uiStream, mcp }: ToolProps & { mcp: ReturnType<typeof useMcp> })Problem 2: Type MismatchThe Problem 3: Missing useMcp ImportThe type Solution:Fix Geospatial MCP Issues
The main issues are parameter and type mismatches. Once these fixes are applied, the geospatial MCP should work properly. The key fix is removing the ⏳ Preparing changes... |
|
A summary of the edits CodeRabbit can apply:
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets): ✏️ lib/agents/tools/geospatial.tsxAdd a proper type definition for the MCP client after the imports. Insert this after line 5 and before the dotenv config comment: // Define proper MCP type
export type McpClient = ReturnType<typeof useMcp>;Update the geospatialTool function signature on line 29 to properly handle the MCP parameter and remove the complex type intersection. Replace: export const geospatialTool = ({ uiStream, mcp }: ToolProps & { mcp: ReturnType<typeof useMcp> }) => ({with: export const geospatialTool = ({ uiStream, mcp }: { uiStream: ToolProps['uiStream'], mcp: McpClient | null }) => ({Add MCP validation at the beginning of the execute function, right after the UI feedback setup. Insert this code after line 44 (after // Check if MCP client is available
if (!mcp) {
console.warn('[GeospatialTool] MCP client is not available, cannot proceed with geospatial query');
const errorStream = createStreamableValue<string>();
errorStream.done('Geospatial functionality is currently unavailable. Please ensure MCP is properly configured.');
uiStream.append(<BotMessage content={errorStream.value} />);
return {
type: 'MAP_QUERY_TRIGGER',
originalUserInput: query,
timestamp: new Date().toISOString(),
mcp_response: null,
error: 'MCP client not available'
};
}Update the MCP state check on lines 69-72 to handle the case where mcp might be null. Replace the existing if statement: if (mcp.state !== 'ready') {
console.warn(`MCP client not ready (state: ${mcp.state}), cannot proceed with tool call.`);
throw new Error(`MCP client not ready (state: ${mcp.state})`);
}with: if (!mcp || mcp.state !== 'ready') {
const stateMsg = mcp ? `state: ${mcp.state}` : 'mcp client is null';
console.warn(`[GeospatialTool] MCP client not ready (${stateMsg}), cannot proceed with tool call.`);
throw new Error(`MCP client not ready (${stateMsg})`);
}Update the final MCP state check on lines 125-127 in the finally block to handle null mcp. Replace: if (mcp.state === 'ready') {
console.log('\nMCP client is ready; no explicit close method available for useMcp.');
}with: if (mcp && mcp.state === 'ready') {
console.log('\n[GeospatialTool] MCP client is ready; no explicit close method available for useMcp.');
}Add better logging prefix to all console statements for consistency. Update line 67 from: console.log(`Attempting to connect to MCP server...`);to: console.log(`[GeospatialTool] Attempting to connect to MCP server...`);And update line 74 from: console.log('✅ Successfully connected to MCP server.');to: console.log('[GeospatialTool] ✅ Successfully connected to MCP server.');Fix the import statements at the top of the file. Reorganize imports and add proper type definition: import { useMcp } from 'use-mcp/react';
import { createStreamableUI, createStreamableValue } from 'ai/rsc';
import { BotMessage } from '@/components/message';
import { geospatialQuerySchema } from '@/lib/schema/geospatial';
// Define proper MCP type
export type McpClient = ReturnType<typeof useMcp>;Remove any duplicate useMcp imports and ensure the order is correct. Update the geospatialTool function signature to only accept the parameters it actually needs, removing the dependency on ToolProps which includes the unsupported fullResponse parameter: export const geospatialTool = ({ uiStream, mcp }: {
uiStream: ReturnType<typeof createStreamableUI>,
mcp: McpClient | null
}) => ({
description: `Use this tool for any queries that involve locations, places, addresses, distances between places, directions, or finding points of interest on a map. This includes questions like:
- 'Where is [place name/address]?'
- 'Show me [place name/address] on the map.'
- 'What's the latitude and longitude of [place name]?'
- 'How far is it from [location A] to [location B]?'
- 'Give me directions from [location A] to [location B].'
- 'Find cafes near [current location/specified location].'
- 'What's around the [specific landmark]?'
- Any query that implies needing a map or spatial understanding.`,
parameters: geospatialQuerySchema,This removes the ToolProps dependency and makes the function signature explicit about what parameters it accepts. Add comprehensive MCP validation at the beginning of the execute function, right after the UI feedback: execute: async ({ query }: { query: string }) => {
// Provide immediate UI feedback
const uiFeedbackStream = createStreamableValue<string>();
uiFeedbackStream.done(`Looking up map information for: "${query}"...`);
uiStream.append(<BotMessage content={uiFeedbackStream.value} />);
// Check if MCP client is available
if (!mcp) {
console.warn('MCP client is not available, cannot proceed with geospatial query');
const errorStream = createStreamableValue<string>();
errorStream.done('Geospatial functionality is currently unavailable.');
uiStream.append(<BotMessage content={errorStream.value} />);
return {
type: 'MAP_QUERY_TRIGGER',
originalUserInput: query,
timestamp: new Date().toISOString(),
mcp_response: null,
error: 'MCP client not available'
};
}
// Log environment variables for debugging (with API key masked)
console.log(`[GeospatialTool] SMITHERY_PROFILE_ID: "${process.env.SMITHERY_PROFILE_ID ?? 'undefined'}"`);
console.log(
`[GeospatialTool] SMITHERY_API_KEY: ${
process.env.SMITHERY_API_KEY
? `****${process.env.SMITHERY_API_KEY.slice(-4)} (masked)`
: 'undefined'
}`
);This adds proper null checking for the mcp parameter before any usage. Update the MCP state checking logic to handle the null case properly, replacing the existing mcp.state check: try {
console.log(`Attempting to connect to MCP server...`);
if (mcp.state !== 'ready') {
console.warn(`MCP client not ready (state: ${mcp.state}), cannot proceed with tool call.`);
const errorStream = createStreamableValue<string>();
errorStream.done(`MCP client not ready (state: ${mcp.state}). Please try again.`);
uiStream.append(<BotMessage content={errorStream.value} />);
return {
type: 'MAP_QUERY_TRIGGER',
originalUserInput: query,
timestamp: new Date().toISOString(),
mcp_response: null,
error: `MCP client not ready (state: ${mcp.state})`
};
}
console.log('✅ Successfully connected to MCP server.');
// ... rest of existing MCP logicThis improves error handling and provides better user feedback when MCP is not ready. Also create a separate fix for lib/agents/tools/index.tsx to resolve the parameter mismatch in the getTools function. Remove the fullResponse parameter from the geospatialTool call: // In getTools function, change:
geospatialQueryTool: geospatialTool({
uiStream,
fullResponse, // Remove this line
mcp: mcp || null // Use the passed mcp with fallback
})
// To:
geospatialQueryTool: geospatialTool({
uiStream,
mcp: mcp || null // Use the passed mcp with fallback
})This ensures that only the parameters expected by geospatialTool are passed to it. ✏️ lib/agents/tools/index.tsxRemove the Add proper fallback handling for the mcp parameter by changing Update the geospatialTool call to be: geospatialQueryTool: geospatialTool({
uiStream,
mcp: mcp || null
})This removes the unsupported fullResponse parameter and provides a safe fallback for the mcp parameter. Locate the getTools function definition in the file and find the line where geospatialQueryTool is being called. This should look something like: geospatialQueryTool: geospatialTool({
uiStream,
fullResponse,
mcp
})Remove the Replace the geospatialTool call with the corrected version: geospatialQueryTool: geospatialTool({
uiStream,
mcp: mcp || null
})This ensures only the expected parameters (uiStream and mcp) are passed, and provides a null fallback for mcp to handle cases where it might be undefined. Verify that the ToolProps interface (if defined in this file) properly handles the mcp parameter. If the interface exists and defines mcp as optional ( Ensure that other tool functions in the getTools function maintain their correct parameter signatures. The search and retrieve tools should continue to receive Add any necessary imports for MCP types if the file references
|
|
Creating a commit in the current branch to apply the edits... The changes have been committed to the current branch. Commit ID: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/agents/tools/geospatial.tsx (1)
68-73: Inconsistent logging format.The logging format differs between lines 69-73 and lines 74-80. Consider using a consistent format for better debugging experience.
Apply this diff for consistency:
- console.log( - `[GeospatialTool] SMITHERY_PROFILE_ID: "${ - process.env.SMITHERY_PROFILE_ID ?? 'undefined' - }"` - ); + console.log( + `[GeospatialTool] SMITHERY_PROFILE_ID: ${ + process.env.SMITHERY_PROFILE_ID ?? 'undefined' + }` + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/agents/tools/geospatial.tsx(8 hunks)lib/agents/tools/index.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/agents/tools/index.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
lib/agents/tools/geospatial.tsx
[error] 203-203: Expected a statement but instead found '}
} catch (error)'.
Expected a statement here.
(parse)
[error] 205-205: Expected a statement but instead found 'finally'.
Expected a statement here.
(parse)
[error] 212-218: Illegal return statement outside of a function
(parse)
[error] 218-218: Expected a statement but instead found '},
})'.
Expected a statement here.
(parse)
[error] 203-205: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 205-210: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (7)
lib/agents/tools/geospatial.tsx (7)
1-2: LGTM: Environment configuration setup.The early dotenv configuration ensures environment variables are loaded before other imports, which is a good practice.
9-10: Good type safety improvement.Defining
McpClientas an explicit type alias improves code clarity and type safety throughout the module.
28-34: Excellent function signature refactoring.The updated function signature with explicit parameter types replaces the generic
ToolPropsusage, providing better type safety and making the MCP dependency clear.
51-66: Robust MCP availability validation.The early check for MCP client availability with proper error handling and user feedback is a significant improvement that prevents runtime failures.
98-113: Improved MCP readiness validation.The enhanced MCP state checking with user feedback provides better error handling compared to the previous implementation.
118-126: Enhanced logging for debugging.The detailed logging with emojis and structured parameter logging will help with debugging MCP integration issues.
149-157: Good JSON parsing validation.The additional validation checks for expected fields and format improve robustness of the MCP response handling.
Also applies to: 167-175
|
@coderabbitai can your previous commit has broken the codebase its not building |
Refactored `getTools` and its callers to correctly handle the `useGeospatialToolMcp` hook, preventing calls in non-React component contexts. This resolves the primary linting error. - Modified `getTools` to accept `mcp` as a parameter. - Updated `researcher` function to accept and pass `mcp`. - Updated components calling the `submit` action to use the hook and pass `mcp`. - Made `mcp` optional in `ToolProps`. - Corrected a syntax error in `lib/agents/tools/geospatial.tsx` caused by a duplicated code block during previous edits. The build, when `createStreamableValue` is used in `geospatial.tsx`, still fails due to an unresolved import for it (despite user confirmation of the path `ai/rsc`). The `DATABASE_URL` error also persists but is being disregarded per user instruction.
|
Tip For best results, initiate chat on the files or code changes. Creating a commit in the current branch to apply the edits... The changes have been committed to the current branch. Commit ID: |
2 participants
User description
Refactored the
getToolsfunction and its callers to correctly handle theuseGeospatialToolMcphook, preventing it from being called in a non-React component context.getToolsto acceptmcpas a parameter.researcherfunction to accept and passmcp.submitaction to fetchmcpusing the hook and pass it down.mcpoptional inToolPropsas not all tools require it.The build still fails due to an unresolved import for
createStreamableValuewhen its usage is included, and a missingDATABASE_URL(which is being disregarded per user instruction). The primary React Hook linting error is resolved.PR Type
Bug fix
Description
Fix React Hook usage error in
getToolsfunctionRefactor
mcpparameter passing through component hierarchyUpdate tool initialization to accept external
mcpparameterRemove unused
useToolsfunction implementationChanges walkthrough 📝
8 files
Add mcp parameter to submit functionUse hook and pass mcp to submitUse hook and pass mcp to submitUse hook and pass mcp to submitUse hook and pass mcp to submitAccept mcp parameter and pass to getToolsEnsure correct import path for createStreamableValueRefactor getTools to accept mcp parameterSummary by CodeRabbit