feat: simplify meta tool names to match Node SDK#19
Conversation
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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