fix: message integration #3269
fix: message integration #3269
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
thanks @fengju0213 and @nitpicker55555 , left some comments below
| if isinstance(item, FunctionTool): | ||
| enhanced_tools.append( | ||
| FunctionTool( | ||
| func=enhanced_method, | ||
| openai_tool_schema=item.get_openai_tool_schema(), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
seems here is using original tool schema? then why we handle like this, would this be same with enhanced_tools.append(FunctionTool(enhanced_method))?
| if function_names is None or func.__name__ in function_names: | ||
| enhanced_func = self._add_messaging_to_tool(func) | ||
| enhanced_tools.append(FunctionTool(enhanced_func)) | ||
| # If func is a bound toolkit method, route to register_toolkits | ||
| if hasattr(func, '__self__') and isinstance( | ||
| func.__self__, BaseToolkit | ||
| ): |
There was a problem hiding this comment.
seems we didn't consider the case that the toolkit already been enhanced
Wendong-Fan
added
the
Waiting for Update
fengju0213
added
Review Required
| # Enhance the specific method on the toolkit instance | ||
| enhanced_toolkit = self.register_toolkits( | ||
| toolkit_instance, tool_names=[method_name] | ||
| ) |
There was a problem hiding this comment.
this will lead to the whole toolkit be registered
There was a problem hiding this comment.
@Wendong-Fan after our discussion，i re-check the code，finding i just pass in tool_names=[method_name] when use register_toolkits,so the whole toolkit will not be registered, only one function
so i think the current way is ok
There was a problem hiding this comment.
self.register_toolkits would register the whole toolkit, i will fix this
There was a problem hiding this comment.
you mean it should be？
Wendong-Fan
added
Waiting for Update
fengju0213
added
Review Required
| # Enhance the specific method on the toolkit instance | ||
| enhanced_toolkit = self.register_toolkits( | ||
| toolkit_instance, tool_names=[method_name] | ||
| ) |
There was a problem hiding this comment.
@Wendong-Fan after our discussion，i re-check the code，finding i just pass in tool_names=[method_name] when use register_toolkits,so the whole toolkit will not be registered, only one function
so i think the current way is ok
|
please review PR: #3331 |
3 participants
Description
Describe your changes in detail (optional if the linked issue already contains a detailed description of the changes).
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!