-
Notifications
You must be signed in to change notification settings - Fork 0
feat: simplify meta tool names to match Node SDK #19
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
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.
cubic analysis
2 issues found across 9 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| else: | ||
| func = partial(self.execute, kwargs if kwargs else None) | ||
|
|
||
| return await asyncio.get_event_loop().run_in_executor(None, func) |
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.
asyncio.get_event_loop() is deprecated inside coroutines as of Python 3.11 and can raise a RuntimeError if no loop is set. Use asyncio.get_running_loop() (or asyncio.to_thread) to obtain the current loop safely.
Prompt for AI agents
Address the following comment on stackone_ai/models.py at line 277:
<comment>`asyncio.get_event_loop()` is deprecated inside coroutines as of Python 3.11 and can raise a `RuntimeError` if no loop is set. Use `asyncio.get_running_loop()` (or `asyncio.to_thread`) to obtain the current loop safely.</comment>
<file context>
@@ -214,6 +216,66 @@ def execute(self, arguments: str | JsonDict | None = None) -> JsonDict:
) from e
raise StackOneError(f"Request failed: {e}") from e
+ def call(self, *args: Any, **kwargs: Any) -> JsonDict:
+ """Call the tool with the given arguments
+
+ This method provides a more intuitive way to execute tools directly.
+
+ Args:
</file context>
| return await asyncio.get_event_loop().run_in_executor(None, func) | |
| return await asyncio.get_running_loop().run_in_executor(None, func) |
| if args: | ||
| if len(args) > 1: | ||
| raise ValueError("Only one positional argument is allowed") | ||
| return self.execute(args[0]) |
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.
If the single positional argument is not a str or dict, execute() will attempt to treat it as a mapping and fail later with an obscure AttributeError when calling .items(). Validate the argument type here and raise a clear ValueError instead to avoid confusing downstream errors.
Prompt for AI agents
Address the following comment on stackone_ai/models.py at line 245:
<comment>If the single positional argument is not a `str` or `dict`, `execute()` will attempt to treat it as a mapping and fail later with an obscure `AttributeError` when calling `.items()`. Validate the argument type here and raise a clear `ValueError` instead to avoid confusing downstream errors.</comment>
<file context>
@@ -214,6 +216,66 @@ def execute(self, arguments: str | JsonDict | None = None) -> JsonDict:
) from e
raise StackOneError(f"Request failed: {e}") from e
+ def call(self, *args: Any, **kwargs: Any) -> JsonDict:
+ """Call the tool with the given arguments
+
+ This method provides a more intuitive way to execute tools directly.
+
+ Args:
</file context>
|
oh wait i'll merge it |
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.
cubic analysis
1 issue found across 5 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
stackone_ai/models.py
Outdated
| return [tool.to_langchain() for tool in self.tools] | ||
|
|
||
| def meta_tools(self) -> "Tools": | ||
| def meta_search_tools(self) -> "Tools": |
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.
Renaming the public API from meta_tools() to meta_search_tools() outright breaks existing user code that still calls the old method; no compatibility shim or deprecated alias is provided. This contradicts the PR description which promises “Maintains backward compatibility”.
Prompt for AI agents
Address the following comment on stackone_ai/models.py at line 483:
<comment>Renaming the public API from meta_tools() to meta_search_tools() outright breaks existing user code that still calls the old method; no compatibility shim or deprecated alias is provided. This contradicts the PR description which promises “Maintains backward compatibility”.</comment>
<file context>
@@ -480,7 +480,7 @@ def to_langchain(self) -> Sequence[BaseTool]:
"""
return [tool.to_langchain() for tool in self.tools]
- def meta_tools(self) -> "Tools":
+ def meta_search_tools(self) -> "Tools":
"""Return meta tools for tool discovery and execution
</file context>
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.
I think the aim was to change the tool name to
meta_search_tools
meta_execute_tool
this doesnt do that?
- Rename create_meta_search_tools_filter_tool() to create_meta_search_tools() - Rename create_meta_search_tools_execute_tool() to create_meta_execute_tool() - Update file names: meta_search_tools.py to meta_tools.py - Update method name: meta_search_tools() to meta_tools() - Standardize terminology: use "meta tools" in comments/docs - Keep tool names as meta_search_tools and meta_execute_tool This aligns the Python SDK with the Node SDK naming conventions while maintaining the same functionality.
1978e5c to
411ef06
Compare
|
@mattzcarey i think it is ready |
Summary
This PR simplifies meta tool function names to match the Node SDK naming conventions, improving consistency between the Python and Node.js SDKs.
Breaking Changes
Function Names
create_meta_search_tools_filter_tool()→create_meta_search_tools()create_meta_search_tools_execute_tool()→create_meta_execute_tool()File Names
meta_search_tools.py→meta_tools.pytest_meta_search_tools.py→test_meta_tools.pymeta_search_tools_example.py→meta_tools_example.pyMethod Names
Tools.meta_search_tools()→Tools.meta_tools()No Changes to Tool Names
The actual tool names remain the same for API compatibility:
meta_search_tools(search/discovery tool)meta_execute_tool(execution tool)Migration Guide
If you are using the meta tools directly:
Additional Improvements
This change brings the Python SDK in line with the Node SDK changes from StackOneHQ/stackone-ai-node#86