-
Notifications
You must be signed in to change notification settings - Fork 64
Fix MCP stdio communication issues #326
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: dev
Are you sure you want to change the base?
Fix MCP stdio communication issues #326
Conversation
Kylie-dot-s
commented
Dec 26, 2025
- Fix rule_resume.py: Replace invalid eval_details/eval_status with proper EvalDetail fields (status, label, reason)
- Fix mcp_server.py: Add environment variables to force ThreadPool usage and prevent ProcessPool fd inheritance
- Set LOCAL_DEPLOYMENT_MODE=true to use ThreadPoolExecutor instead of ProcessPoolExecutor
- Set TQDM_DISABLE=1 to prevent progress bar output pollution
- Add stdout/stderr redirection with StringIO for better error handling
- Fix rule name extraction logic to handle both class and instance types
- Add defensive checks for Model.prompt_name_map access
- Change default rule group fallback from 'default' to 'sft' to avoid buggy Resume rules
- Fix rule_resume.py: Replace invalid eval_details/eval_status with proper EvalDetail fields (status, label, reason) - Fix mcp_server.py: Add environment variables to force ThreadPool usage and prevent ProcessPool fd inheritance * Set LOCAL_DEPLOYMENT_MODE=true to use ThreadPoolExecutor instead of ProcessPoolExecutor * Set TQDM_DISABLE=1 to prevent progress bar output pollution * Add stdout/stderr redirection with StringIO for better error handling * Fix rule name extraction logic to handle both class and instance types * Add defensive checks for Model.prompt_name_map access * Change default rule group fallback from 'default' to 'sft' to avoid buggy Resume rules
Summary of ChangesHello @Kylie-dot-s, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the stability and reliability of the MCP server's communication and Dingo evaluation processes. It addresses critical issues related to Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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 effectively addresses stdio communication issues in mcp_server.py by forcing single-threaded execution and redirecting output streams to prevent interference. It also introduces several valuable refactorings and robustness improvements, such as adding defensive checks, improving LLM configuration handling, and making rule processing more flexible. The changes align well with the PR description and significantly enhance the server's stability and maintainability. I have a few suggestions to further improve the code, mainly regarding a performance concern and some minor code duplication.
- Add noqa: E402 comments to imports that must stay after os.environ setup - Fix E261: Add proper spacing before inline comments - Fix F841: Remove unused variable 'inner_e' in exception handler - Fix E115: Correct indentation of FIX END comment
- Keep noqa: E402 comments for imports after os.environ setup - Maintain proper import order and formatting
|
/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 a series of fixes and refactorings primarily aimed at improving MCP stdio communication and adapting to a new Dingo evaluation architecture. The changes in rule_resume.py correctly align the EvalDetail object with its new structure. In mcp_server.py, you've made substantial improvements, including adding robust file logging, significantly enhancing input path resolution logic, and refactoring many functions to separate core logic from the MCP tool layer, which is great for maintainability. The logic for handling LLM and rule configurations has been heavily updated to support a new model where prompts are embedded within LLMs. While these are excellent changes, I've identified a few areas for improvement regarding exception handling, code duplication, and algorithmic efficiency, which are detailed in the comments below.