-
-
Notifications
You must be signed in to change notification settings - Fork 602
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 support for synchrous Reka using the OpenAI SDK format. #624
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.
❌ Changes requested.
- Reviewed the entire pull request up to b1f4f1e
- Looked at
293
lines of code in7
files - Took 1 minute and 54 seconds to review
More info
- Skipped
0
files when reviewing. - Skipped posting
1
additional comments because they didn't meet confidence threshold of85%
.
1. /instructor/client_reka.py:31
:
- Assessed confidence :
66%
- Comment:
Consider allowing more modes if the Reka-specific JSON mode testing involves other modes or if future expansions are planned. Currently, onlyMode.MD_JSON
is allowed, which might be too restrictive. - Reasoning:
The PR description mentions that a Reka-specific JSON mode is being tested, but the code only allows forMode.MD_JSON
. This could be a limitation if other modes are intended to be supported in the future or if the testing involves other modes.
Workflow ID: wflow_CLfriM6wLxeIeIy2
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. We'll respond in a few minutes. Learn more here.
instructor/client_reka.py
Outdated
) -> instructor.Instructor: ... | ||
|
||
|
||
def from_reka(api_key: Optional[str] =None, |
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.
Please add an assertion or check to prevent the use of async methods with Reka since async support is not yet implemented. This will help avoid runtime errors if such methods are attempted to be used.
@@ -0,0 +1,46 @@ | |||
# Structured Outputs using Reka |
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.
Please update the mkdocs.yml
file to include the new reka.md
file in the site navigation. This ensures that the documentation is accessible and visible in the generated site.
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.
+1 let me know if you need help
temperature=0, | ||
) | ||
|
||
print(user_info.name) |
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 variable user_info
is not defined in the example code. You should replace user_info
with resp
to match the variable that holds the response from the Reka client.
print(user_info.name) | |
print(resp.name) |
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.
+1
|
||
print(user_info.name) | ||
#> John Doe | ||
print(user_info.age) |
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 variable user_info
is not defined in the example code. You should replace user_info
with resp
to match the variable that holds the response from the Reka client.
print(user_info.age) | |
print(resp.age) |
docs/examples/reka.md
Outdated
pip install instructor reka-api pydantic | ||
``` | ||
|
||
``` |
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.
``` |
instructor/client_reka.py
Outdated
|
||
def reka_chat_wrapper(default_model, **kwargs): | ||
model = kwargs.pop('model',default_model) | ||
kwargs['model_name']=model |
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.
run ruff
What's the status of this? Is this ready to merge if the tests pass? ? |
Will be adding async and am testing a Reka specific json mode
Summary:
This PR introduces support for the Reka AI model in the Instructor library, including a new client, example usage, test case, and documentation.
Key points:
RekaClient
andfrom_reka
function ininstructor
module.client.py
andutils.py
to support Reka client.test_new_client.py
.reka.py
.reka.md
.Generated with ❤️ by ellipsis.dev