-
-
Notifications
You must be signed in to change notification settings - Fork 730
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
Added example for using unions of models, useful for agents #299
Conversation
WalkthroughThe latest update introduces a suite of Pydantic models that streamline the process of selecting and executing actions within a Python application. These models enable the application to search, lookup, and complete tasks, with an overarching Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- examples/let_the_llm_choose_a_tool.py (1 hunks)
Additional comments: 4
examples/let_the_llm_choose_a_tool.py (4)
6-12: The
Search
class is well-defined with a clear docstring and a process method. The use of theField
function for thequery
attribute provides additional metadata which can be useful for documentation or schema generation.14-20: The
Lookup
class is consistent with theSearch
class in structure and implementation, which is good for maintainability and readability.22-28: The
Finish
class maintains the consistency of design seen in theSearch
andLookup
classes, which is beneficial for a unified approach to action classes.30-36: The
TakeAction
class correctly uses a union type for theaction
attribute, allowing for polymorphic behavior. The process method is concise and delegates the call to the contained action's process method, which is a clean implementation of the strategy pattern.
# Enables `response_model` | ||
client = instructor.patch(OpenAI()) | ||
action = client.chat.completions.create( | ||
model="gpt-3.5-turbo", | ||
response_model=TakeAction, | ||
messages=[ | ||
{"role": "user", "content": "Please choose one action"}, | ||
] | ||
) | ||
assert isinstance(action, TakeAction) | ||
print(action.process()) |
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.
Consider adding error handling around the API call and the type assertion to gracefully handle any exceptions that may occur, ensuring the robustness of the example.
+ try:
client = instructor.patch(OpenAI())
action = client.chat.completions.create(
model="gpt-3.5-turbo",
response_model=TakeAction,
messages=[
{"role": "user", "content": "Please choose one action"},
]
)
assert isinstance(action, TakeAction)
print(action.process())
+ except (SomeSpecificException, AssertionError) as e:
+ print(f"An error occurred: {e}")
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
# Enables `response_model` | |
client = instructor.patch(OpenAI()) | |
action = client.chat.completions.create( | |
model="gpt-3.5-turbo", | |
response_model=TakeAction, | |
messages=[ | |
{"role": "user", "content": "Please choose one action"}, | |
] | |
) | |
assert isinstance(action, TakeAction) | |
print(action.process()) | |
try: | |
client = instructor.patch(OpenAI()) | |
action = client.chat.completions.create( | |
model="gpt-3.5-turbo", | |
response_model=TakeAction, | |
messages=[ | |
{"role": "user", "content": "Please choose one action"}, | |
] | |
) | |
assert isinstance(action, TakeAction) | |
print(action.process()) | |
except (SomeSpecificException, AssertionError) as e: | |
print(f"An error occurred: {e}") |
do you mind:
|
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- docs/concepts/union.md (1 hunks)
- examples/union/run.py (1 hunks)
Files skipped from review due to trivial changes (1)
- docs/concepts/union.md
Additional comments: 5
examples/union/run.py (5)
1-5: Ensure that all imported modules are used within the file to avoid unnecessary imports that can lead to confusion and performance issues.
6-12: The
Search
class is well-defined with a clear docstring and a process method. The use of Pydantic'sField
for additional metadata is good practice.14-20: The
Lookup
class follows the same pattern as theSearch
class, with appropriate use of Pydantic'sField
and a clear process method.22-28: The
Finish
class is consistent with the other action classes in structure and implementation, ensuring uniformity across the different action types.30-36: The
TakeAction
class correctly uses aUnion
type for theaction
field, allowing for dynamic action processing. This aligns with the PR's objective to demonstrate the use of unions.
try: | ||
# Enables `response_model` | ||
client = instructor.patch(OpenAI()) | ||
action = client.chat.completions.create( | ||
model="gpt-3.5-turbo", | ||
response_model=TakeAction, | ||
messages=[ | ||
{"role": "user", "content": "Please choose one action"}, | ||
] | ||
) | ||
assert isinstance(action, TakeAction), "The action is not TakeAction" | ||
print(action.process()) | ||
except Exception as e: | ||
print(f"An error occurred: {e}") |
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.
The main execution block includes error handling and an assertion to ensure the correct type is returned. However, consider adding more specific exception handling to provide clearer error messages for different failure scenarios.
try:
# Existing code...
except SpecificException as e:
print(f"A specific error occurred: {e}")
except Exception as e:
print(f"An error occurred: {e}")
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
try: | |
# Enables `response_model` | |
client = instructor.patch(OpenAI()) | |
action = client.chat.completions.create( | |
model="gpt-3.5-turbo", | |
response_model=TakeAction, | |
messages=[ | |
{"role": "user", "content": "Please choose one action"}, | |
] | |
) | |
assert isinstance(action, TakeAction), "The action is not TakeAction" | |
print(action.process()) | |
except Exception as e: | |
print(f"An error occurred: {e}") | |
try: | |
# Enables `response_model` | |
client = instructor.patch(OpenAI()) | |
action = client.chat.completions.create( | |
model="gpt-3.5-turbo", | |
response_model=TakeAction, | |
messages=[ | |
{"role": "user", "content": "Please choose one action"}, | |
] | |
) | |
assert isinstance(action, TakeAction), "The action is not TakeAction" | |
print(action.process()) | |
except SpecificException as e: | |
print(f"A specific error occurred: {e}") | |
except Exception as e: | |
print(f"An error occurred: {e}") |
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 want the example to be executable.
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.
💡 NOTE
Deep insights is a beta feature.
@zby, it seems that the instructor
and openai
modules are not part of the codebase and might be external dependencies. To ensure the example is executable, you would need to have these packages installed in your environment. If they are already installed and you're encountering issues, could you please provide more details on the errors you're facing? This will help me assist you further in making the example executable.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the 'instructor' module exists in the codebase
fd 'instructor.py'
# Check if the 'openai' module exists in the codebase
fd 'openai.py'
# Check if the 'OpenAI' class is defined within the 'openai' module
ast-grep --lang python --pattern $'class OpenAI {
$$$
}'
Length of output: 538
Script:
#!/bin/bash
# Correcting the ast-grep pattern to search for the 'OpenAI' class definition
ast-grep --lang python --pattern $'class OpenAI { $$$ }'
Length of output: 500
I would rename the concept to Agent - because this is what people are looking for. |
This is the example from #296
I searched for Union in the sources - and it seems that there are no examples of this approach.
Summary by CodeRabbit