-
-
Notifications
You must be signed in to change notification settings - Fork 626
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
feat: anthropic batching code #1064
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 everything up to 1bdc2d5 in 39 seconds
More details
- Looked at
692
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. docs/cli/batch.md:1
- Draft comment:
If this is a new file, ensure it's added to mkdocs.yml for documentation consistency. - Reason this comment was not posted:
Confidence changes required:80%
The mkdocs.yml file needs to be updated to include the new batch.md file for documentation consistency.
2. instructor/batch.py:114
- Draft comment:
Add comments to explain the logic increate_from_messages
, especially around handling different modes for OpenAI and Anthropic. - Reason this comment was not posted:
Confidence changes required:80%
The functioncreate_from_messages
inbatch.py
is quite complex and would benefit from comments explaining the logic, especially around the handling of different modes for OpenAI and Anthropic.
3. instructor/cli/batch.py:39
- Draft comment:
The use ofstr()
aroundgetattr
calls is unnecessary and can be removed for cleaner code. This applies to lines 39, 40, and 41. - Reason this comment was not posted:
Confidence changes required:50%
Thegenerate_table
function inbatch.py
usesgetattr
to access attributes, which is a good practice to avoid errors if the attribute doesn't exist. However, the use ofstr()
around these calls is unnecessary and can be removed for cleaner code.
4. instructor/cli/batch.py:104
- Draft comment:
Avoid usingeval
for parsing JSON data. Usejson.loads
instead for security reasons. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_0XQLXZjyvf4ZN6vE
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
"[bold green]Creating Anthropic batch job...", spinner="dots" | ||
): | ||
with open(file_path, "r") as file: | ||
requests = [eval(line) for line in file] |
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.
Using eval
to parse JSON is a security risk. Consider using json.loads(line)
instead.
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.
👍 Looks good to me! Incremental review on fbf81a7 in 20 seconds
More details
- Looked at
56
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. instructor/cli/batch.py:103
- Draft comment:
Usingeval
to parse file content is a security risk. Consider usingjson.loads
instead for safer parsing. - Reason this comment was not posted:
Comment was on unchanged code.
2. instructor/cli/batch.py:13
- Draft comment:
list[Any]
is not compatible with Python versions below 3.9. UseList[Any]
for compatibility. - Reason this comment was not posted:
Confidence changes required:80%
Thegenerate_table
function useslist[Any]
which is not compatible with Python versions below 3.9. It should be replaced withList[Any]
for compatibility.
3. instructor/cli/batch.py:13
- Draft comment:
For compatibility with Python versions below 3.9, useList[Any]
instead oflist[Any]
. This also applies to other instances in the codebase. - Reason this comment was not posted:
Confidence changes required:80%
Thegenerate_table
function useslist[Any]
which is not compatible with Python versions below 3.9. It should be updated for compatibility.
Workflow ID: wflow_j3F1tpwsSjPIJ9Ne
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
I'm pretty sure this works, but I would love some time for someone else to verify some of the loose ends. I don't know how to test this yet.
Important
Adds support for managing batch jobs on Anthropic alongside OpenAI, updating CLI and batch processing logic.
--use-anthropic
flag inbatch.py
to switch between OpenAI and Anthropic.create_from_file
,cancel
, anddownload_file
commands to support Anthropic API.generate_table
andget_jobs
to handle Anthropic batch jobs.BatchJob
class inbatch.py
to parse and create batch jobs for both OpenAI and Anthropic.openai_models
type and generalizedmodel
handling.RequestBody
andBatchModel
to accommodate Anthropic requests.batch.md
to include instructions for using Anthropic API.This description was created by for fbf81a7. It will automatically update as commits are pushed.