-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: enable MCP to models without function calling ability #1978
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
echo-yiyiyi
left a comment
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.
A very cool MCPAgent! Left some comments.
Also, I’d like to know—could you briefly explain what model_fc_available means and why we need these two modes?
| ```json | ||
| { | ||
| "server_idx": idx, | ||
| "tool_name": "tool_name", | ||
| "tool_args": { | ||
| "arg1": value1, | ||
| "arg2": value2, | ||
| ... | ||
| } | ||
| } | ||
| ``` |
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.
Camel provides modules for structured output and scraping. Maybe you can check if it's available to avoid reinventing the wheel.
https://github.com/camel-ai/camel/blob/master/examples/structured_response/structure_response_prompt_engineering.py
https://github.com/camel-ai/camel/blob/master/examples/structured_response/json_format_response.py
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.
In the existing implementation, Deepseek-R1, gemmas and other models without function calling ability can not be used.
The model_fc_avaible is to set whether the large model has the function call ability.
I will change it into function_calling_available and try to use the modules for structured output and scraping.
| model: Optional[BaseModelBackend] = None, | ||
| model_fc_available: bool = False, | ||
| ) -> None: | ||
| r"""Initialize the MCP agent. |
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.
Maybe also add the reference of MCP Agent design here if there is any?
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.
OK
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.
thanks @TommeyChang ! As @yiyiyi0817 mentioned it would be great if we can use structured output feature instead of using prompt-based method, added one more PR here: feel free to help with review~
camel/agents/mcp_agent.py
Outdated
| await self._mcp_toolkit.disconnect() | ||
|
|
||
| def add_mcp_tools(self): | ||
| assert self._mcp_toolkit.is_connected(), "Server is not connected." |
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.
better not use assert in production code
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.
OK, I will fix it.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
History messed up for review, moving to #2128. |
Pull request was closed
…ix for a clear git history) (#2128) Co-authored-by: tommey <tommeychang@163.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Wendong <w3ndong.fan@gmail.com> Co-authored-by: Wendong-Fan <133094783+Wendong-Fan@users.noreply.github.com>
|
hey @TommeyChang , this feature has been merged in PR #2128, thanks so much for the contribution! Feel free to close this PR at your convenience |
|
close as feature merged in another PR |
Description
In this commit, a model without built-in function calling ability can use a MCP tools from giving the tool_name and tool_args.
In this case, we can set agents to equip with the same type of tools reducing the search space.
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!