Skip to content

Conversation

@torresmateo
Copy link
Collaborator

This adds a docstring to get_arcade_tools that documents and implements the tools parameter so not everything in a toolkit is included.

@torresmateo torresmateo requested a review from a team April 2, 2025 18:15
Copy link
Member

@EricGustin EricGustin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Spartee the get_arcade_tools does not behave the way that I would expect (irrespective of this current PR).

tools = await get_arcade_tools(
    client, 
    toolkits=["github"], 
    tools=["Google.ListEmails"]
)
# Expected: All github tools + list emails tool
# Actual: All github tools (list emails tool is missing)

Seems to me that we are missing a call to client.tools.formatted.get. i.e.,

for tool in tools:
    tool_format = await client.tools.formatted.get(
        name=tool, 
        format="openai"
    )
    tool_formats.append(tool_format)

This is how langchain-arcade and crewai-arcade behave at least.

@torresmateo
Copy link
Collaborator Author

torresmateo commented Apr 2, 2025

@EricGustin Hmm... from the code here I thought the second parameter was a "filter" for the toolkit. I will implement the same behavior as in the other packages for consistency. It's a better DX that way. Making this a draft until I push that!

@torresmateo torresmateo marked this pull request as draft April 2, 2025 18:51
@torresmateo torresmateo marked this pull request as ready for review April 3, 2025 00:06
@torresmateo torresmateo requested a review from EricGustin April 3, 2025 00:07
@sdserranog
Copy link

This is from the docs, but I’m not sure if we want to change it to match how we do it in other packages.

Screenshot 2025-04-04 at 10 13 51 AM Screenshot 2025-04-04 at 10 12 53 AM

@torresmateo
Copy link
Collaborator Author

@sdserranog I think we should update the docs to follow changes in this PR, as the API will be consistent with how langchain-arcade and crewai-arcade behave.

I don't know which order is best to approve these PRs though.

@torresmateo
Copy link
Collaborator Author

@sdserranog On this commit I addressed all of the comments from the other PR.

Copy link

@sdserranog sdserranog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go ahead and approve this since the core functionality is working well. We'll still need to clean up/fix the documentation, but we can handle that in a follow-up PR

@torresmateo torresmateo merged commit 0a8e563 into main Apr 10, 2025
1 check passed
@torresmateo torresmateo deleted the tools-filter branch May 29, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants