-
-
Notifications
You must be signed in to change notification settings - Fork 191
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: support for OpenAI/Anthropic tools
APIs
#300
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.
👍 Looks good to me! Reviewed everything up to 67f8f1c in 1 minute and 19 seconds
More details
- Looked at
2517
lines of code in33
files - Skipped
1
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. pyproject.toml:58
- Draft comment:
Ensure 'json-repair' is included in the dependencies to avoid runtime errors. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_zuIZ3VgodsYA1onL
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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 988020e in 46 seconds
More details
- Looked at
249
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. gptme/llm/llm_anthropic.py:15
- Draft comment:
The importPromptCachingBetaToolParam
is unused and can be removed to clean up the code. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forPromptCachingBetaToolParam
is not used in the filegptme/llm/llm_anthropic.py
. It should be removed to clean up the code.
2. gptme/llm/llm_anthropic.py:105
- Draft comment:
Avoid usingprint
statements for logging. Consider usinglogger.debug
or another appropriate logging level instead. - Reason this comment was not posted:
Confidence changes required:50%
Theprint
statement in thestream
function is not necessary for production code and should be removed or replaced with a logging statement.
3. gptme/server/api.py:24
- Draft comment:
Reorder imports to follow PEP8 guidelines. Groupget_model
with other imports fromgptme.llm
. - Reason this comment was not posted:
Confidence changes required:10%
The import order ingptme/server/api.py
is not following PEP8 guidelines. The import ofget_model
should be grouped with other imports fromgptme.llm
.
4. gptme/server/api.py:178
- Draft comment:
Avoid usingprint
statements for logging. Consider usinglogger.debug
or another appropriate logging level instead. - Reason this comment was not posted:
Confidence changes required:50%
Theprint
statement in thegenerate
function is not necessary for production code and should be removed or replaced with a logging statement.
5. gptme/tools/python.py:18
- Draft comment:
The importConfirmFunc
is unused and can be removed to clean up the code. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forConfirmFunc
ingptme/tools/python.py
is not used in the file. It should be removed to clean up the code.
Workflow ID: wflow_lUpuTWWpnegXDqJY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #300 +/- ##
==========================================
+ Coverage 72.52% 73.93% +1.40%
==========================================
Files 65 65
Lines 4386 4604 +218
==========================================
+ Hits 3181 3404 +223
+ Misses 1205 1200 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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 4ed70f1 in 28 seconds
More details
- Looked at
82
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. gptme/chat.py:54
- Draft comment:
The return type annotation for thechat
function should beNone
instead of-> None:
. Remove the colon after the closing parenthesis. - Reason this comment was not posted:
Confidence changes required:50%
The return type annotation for thechat
function was missing aNone
return type. This is a minor issue but should be corrected for clarity and consistency.
Workflow ID: wflow_okw7h830aOwz2fDV
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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 11b74f1 in 39 seconds
More details
- Looked at
122
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. gptme/llm/llm_anthropic.py:62
- Draft comment:
Ensure that theanthropic
module is available in the environment where this function is executed, as it is imported conditionally within the function. - Reason this comment was not posted:
Confidence changes required:50%
The import statements foranthropic
andopenai
modules are conditionally imported within functions. This is a good practice to avoid unnecessary imports when the module is not used, but it can lead to issues if the module is not available when the function is called. It's important to ensure that these modules are available in the environment where the functions are executed.
2. gptme/llm/llm_openai.py:179
- Draft comment:
Ensure that theopenai
module is available in the environment where this function is executed, as it is imported conditionally within the function. - Reason this comment was not posted:
Confidence changes required:50%
The import statements foranthropic
andopenai
modules are conditionally imported within functions. This is a good practice to avoid unnecessary imports when the module is not used, but it can lead to issues if the module is not available when the function is called. It's important to ensure that these modules are available in the environment where the functions are executed.
Workflow ID: wflow_jPvuTWg7yzNsUm3X
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
- Enhance anthropic streaming to handle different chunk types - Rename global anthropic to _anthropic - Fix imports and formatting in various files Part of the tools API refactoring work.
11b74f1
to
9ee19a0
Compare
tools
APIs
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 1647814 in 29 seconds
More details
- Looked at
34
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. gptme/llm/llm_openai.py:172
- Draft comment:
Consider moving the imports ofChoiceDeltaToolCall
andChoiceDeltaToolCallFunction
back to the top of the file to avoid repeated imports during function calls. - Reason this comment was not posted:
Confidence changes required:50%
The import statements forChoiceDeltaToolCall
andChoiceDeltaToolCallFunction
were moved inside thestream
function. This change is unnecessary and can lead to performance issues due to repeated imports during function calls. It's better to keep imports at the top of the file unless there's a specific reason to import them inside a function.
Workflow ID: wflow_lqrbgLavSVZ9iVY0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
This is passing now! My limited local testing suggests this is good enough to merge. Any surprises will be handled in follow-up PRs. Thanks a lot for the huge effort @jrmi! 🥇 |
Rebased and testing #280 by @jrmi
Important
Adds support for OpenAI and Anthropic
tools
APIs with configurable tool parsing methods and updates related tests and dependencies.tool_format
parameter tochat()
andstep()
inchat.py
to support different tool parsing methods ('markdown', 'xml', 'tool').cli.py
to include--tool-format
option for specifying tool parsing method.reply()
inllm/__init__.py
to handle tools based ontool_format
.ToolSpec
andToolUse
intools/base.py
to support newtool_format
.examples()
functions in various tool files to generate examples based ontool_format
.tools/*.py
to handletool_format
.test_tool_use.py
andtest_prompt_tools.py
to verify tool output formatting.test_cli.py
to cover new--tool-format
option.json-repair
dependency inpyproject.toml
for JSON handling.This description was created by for 1647814. It will automatically update as commits are pushed.