- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.2k
[router] web_search_preview tool basic implementation #12290
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
Conversation
| /gemini review | 
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.
Code Review
This pull request introduces web search capabilities as a preview feature. It accomplishes this by adding a special web_search_preview tool type that is handled differently from other MCP tools. The implementation adds a new is_web_search boolean flag that is passed through many layers of the application to control logic flow. While this works, it introduces a code smell that could impact maintainability. My review includes a suggestion to refactor this using a strategy pattern to make the design more robust and extensible. I've also pointed out a few places where magic strings should be replaced with constants to improve code quality, and a small bug in an example configuration file. Overall, this is a good start for the feature, and addressing these points will make it more solid.
| /gemini review | 
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.
Code Review
This pull request introduces support for a web_search_preview tool, which is handled as a simplified version of an MCP tool. The changes are extensive, touching MCP handling, response generation, and streaming logic. The introduction of the ToolContext enum is a good design choice to differentiate between regular MCP tools and the new web search tool.
My review focuses on improving maintainability by addressing magic strings and code duplication. I've pointed out several places where string literals should be replaced with constants and where repeated blocks of code could be refactored into helper functions. These changes will make the code easier to read, less prone to errors, and simpler to modify in the future.
| Hi @slin1237 this pr is ready for review | 
Motivation
filters,search_context_size,user_locationis not supported yetModifications
Architecture:
Request Handling (router.rs):
Response Transformation (responses.rs, streaming.rs):
Streaming Events (streaming.rs):
Tool Execution (mcp.rs):
Tests
Start router using --mcp-config-path
Test non stream output
Test streaming response
Benchmarking and Profiling
Checklist