-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add CodeAct module #8222
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
base: main
Are you sure you want to change the base?
Add CodeAct module #8222
Conversation
a6ef78b
to
5ab5d99
Compare
dspy/predict/react.py
Outdated
@@ -47,6 +47,7 @@ def __init__(self, signature, tools: list[Callable], max_iters=5): | |||
|
|||
for idx, tool in enumerate(tools.values()): | |||
instr.append(f"({idx + 1}) {tool}") | |||
instr.append("You should pass the tool argument in JSON format") |
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 revise to "When providing next_tool_args, the value inside the field must be in JSON format".
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.
Also I'm confused about line 37, which has
"When selecting the next_tool_name and its next_tool_args, the tool must be one of:\n",
I thought that was removed? Am I thinking of a different PR?
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.
Line 37 will be removed in #8190 👍
Example: | ||
|
||
```python | ||
from dspy.predict import CodeAct |
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.
Can we follow up with a code tutorial demonstrating a use case of CodeAct?
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.
Yup, I will file a follow up PR for documentation
if any( | ||
not inspect.isfunction(tool.func) for tool in tools | ||
): | ||
raise ValueError("CodeAct only accepts functions and not callable objects.") |
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.
curious - why do we have this constraint?
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.
Since it is hard to define the callable object in the python interpreter. Unlike functions, we need to define class and what parameters are passed to the object initialization, which is not visible.
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.
Awesome work!
f"Your goal is to generate executable Python code that collects any necessary information for producing {outputs}.\n" | ||
"For each iteration, you will generate a code snippet that either solves the task or progresses towards the solution.\n" | ||
"Ensure any output you wish to extract from the code is printed to the console. The code should be enclosed in a fenced code block.\n" | ||
f"When all information for producing the outputs ({outputs}) are available to be extracted, mark `finished=True` besides the final Python code.\n" |
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.
should that be "besides the final Python code" or finished=True
and generated_code is not None
be mutual exclusive?
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.
Since we have an extractor at last, I designed it in a way that the code passed with finished=True
is executed to minimize the interaction count.
def forward(self, **kwargs): | ||
# Define the tool funcitons in the interpreter | ||
for tool in self.tools.values(): | ||
self.interpreter(inspect.getsource(tool.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.
shall we move it to init, and only run this line for call-time tools? (the new change in ReAct)
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.
Since python interpreter is shutdown every forward
, this needs to happen in forward
too.
return a + b | ||
|
||
@skip_if_deno_not_available | ||
def test_codeact_tool_validation(): |
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.
We may also want to test the input flow - use a mock litellm.completion to capture the prompt, and validate our tool + code information are correctly included in the prompt.
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.
Code looks good to me! I will need to play with this new Module a bit today or tomorrow.
I encountered an error when the function arg is of pydantic type:
Error:
Looks like we also need to register the custom types to the interpreter. I tried to change
|
@chenmoneygithub Thanks for trying this out. That is a known limitation where Tool functions can only depend on built-in naming. In other words, tools should be self-contained. We could re-register non-tool variables in |
This PR add a new module called CodeAct, which adopts the CodeAct architecture to manipulate the Python interpreter with predefined tools.