Fix LiteRT-LM MCP Server based on code review#9
Conversation
- Removed empty try-except block and unused `subprocess` import. - Added input validation for `prompt` (required) and `backend` (enum). - Implemented robust binary existence check using `shutil.which`. - Fixed indentation in multimodal error block. - Updated README example to be text-only to align with current capabilities. - Added explicit logging for Windows fallback and writer drain errors. Co-authored-by: groupthinking <154503486+groupthinking@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @groupthinking, 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 significantly expands the repository's capabilities by integrating a new LiteRT-LM MCP server, which facilitates local LLM inference. Concurrently, it lays down foundational guidelines for autonomous AI agents, defining their operational scope and interaction protocols. The changes reflect a commitment to robust implementation, clear communication through documentation, and addressing specific feedback from code review to enhance reliability and maintainability. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
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.
Code Review
This pull request effectively addresses feedback from a prior code review for the new LiteRT-LM MCP server. The changes significantly improve the server's robustness by adding input validation, a more reliable binary existence check, and better error handling, particularly for cross-platform scenarios. The documentation has also been updated to reflect the server's current capabilities. I have one suggestion to further enhance maintainability by centralizing the configuration for supported backends, which is currently duplicated in two places.
I am having trouble creating individual review comments. Click here to see my feedback.
mcp-servers/litert-mcp/server.py (163-164)
The list of valid backends {"cpu", "gpu", "npu"} is hardcoded here and also duplicated in the tool schema definition on line 112. This violates the DRY (Don't Repeat Yourself) principle and could lead to future maintenance issues if the lists fall out of sync.
To centralize this configuration, I recommend defining it as a class attribute on MCPServer and referencing it in both places.
Example:
class MCPServer:
VALID_BACKENDS = {"cpu", "gpu", "npu"}
# ...
def _handle_tools_list(self, request_id):
# ...
"enum": sorted(list(self.VALID_BACKENDS)),
# ...
async def _run_inference(self, args: Dict[str, Any]) -> Dict[str, Any]:
# ...
if backend not in self.VALID_BACKENDS:
# ... if backend not in self.VALID_BACKENDS:
subprocessimport.prompt(required) andbackend(enum).shutil.which.