Skip to content
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

fix: cleanup and fixes for tools API #303

Merged
merged 4 commits into from
Dec 3, 2024
Merged

Conversation

ErikBjare
Copy link
Owner

@ErikBjare ErikBjare commented Dec 3, 2024

Progress on #302

  • Improve save/append tools with better error handling and parameter descriptions
  • Remove debug print in llm_anthropic.py
  • Code formatting improvements

Important

Enhance tools API with better error handling and parameter descriptions, remove debug prints, and improve code formatting.

  • Tools API Improvements:
    • Enhanced error handling and parameter descriptions in execute_save() and execute_append() in save.py.
    • Updated Parameter descriptions in base.py.
  • Debug and Logging:
    • Removed debug print statement in stream() in llm_anthropic.py.
  • Code Formatting:
    • Improved code formatting in chat.py and base.py.
    • Reordered imports in prompts.py and util/__init__.py.

This description was created by Ellipsis for 77593d0. It will automatically update as commits are pushed.

- Improve save/append tools with better error handling and parameter descriptions
- Remove debug print in llm_anthropic.py
- Code formatting improvements
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 b426f13 in 16 seconds

More details
  • Looked at 245 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. gptme/llm/llm_anthropic.py:110
  • Draft comment:
    Remove the commented-out debug print statement to clean up the code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR description mentions removing a debug print statement in llm_anthropic.py. The commented-out print statement should be removed entirely to clean up the code.
2. gptme/tools/base.py:34
  • Draft comment:
    The regex pattern in toolcall_re was updated to allow for optional whitespace after the colon. Ensure that any related documentation or comments are updated to reflect this change.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The regex pattern in toolcall_re was updated to allow for optional whitespace after the colon. This change should be reflected in the comment or documentation if it exists.

Workflow ID: wflow_jj4iW4u5HUcOD7i0


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 83.18584% with 19 lines in your changes missing coverage. Please review.

Project coverage is 73.89%. Comparing base (d83f723) to head (77593d0).
Report is 4 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
gptme/llm/llm_anthropic.py 71.87% 9 Missing ⚠️
gptme/tools/base.py 90.38% 5 Missing ⚠️
gptme/tools/save.py 66.66% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #303      +/-   ##
==========================================
- Coverage   73.91%   73.89%   -0.02%     
==========================================
  Files          65       65              
  Lines        4604     4662      +58     
==========================================
+ Hits         3403     3445      +42     
- Misses       1201     1217      +16     
Flag Coverage Δ
anthropic/claude-3-haiku-20240307 71.98% <81.41%> (+<0.01%) ⬆️
openai/gpt-4o-mini 71.51% <63.71%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Improve toolcall regex and JSON parsing
- Update parameter order in tools (path before content)
- Add better error handling and logging
- Improve streaming output handling
- Add test cases for tool use parsing
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 5c18ec9 in 26 seconds

More details
  • Looked at 577 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. gptme/llm/llm_anthropic.py:122
  • Draft comment:
    Replace print statements with logger calls for better control over logging and to maintain consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in gptme/llm/llm_anthropic.py uses print statements for debugging purposes, which should be replaced with proper logging to maintain consistency and control over output verbosity.
2. gptme/llm/llm_openai.py:199
  • Draft comment:
    Replace print statements with logger calls for better control over logging and to maintain consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in gptme/llm/llm_openai.py uses print statements for debugging purposes, which should be replaced with proper logging to maintain consistency and control over output verbosity.
3. gptme/tools/save.py:97
  • Draft comment:
    Store the result of p.diff_minimal() in a variable and reuse it to avoid redundant calls.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The execute_save function in gptme/tools/save.py has a redundant call to p.diff_minimal() which can be optimized by storing the result in a variable and reusing it.
4. gptme/tools/base.py:35
  • Draft comment:
    Ensure toolcall_re handles cases with leading characters before @tool: to avoid missing tool calls.
  • Reason this comment was not posted:
    Confidence changes required: 40%
    The toolcall_re regex pattern in gptme/tools/base.py might not handle cases where there are leading characters before the @tool: pattern. This could lead to missed tool calls in some cases.

Workflow ID: wflow_73LJTGb5rn5I82AO


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

gptme/llm/llm_openai.py Outdated Show resolved Hide resolved
@ErikBjare ErikBjare mentioned this pull request Dec 3, 2024
3 tasks
@ErikBjare ErikBjare force-pushed the dev/more-tools-api-fixes branch from f30a53b to e5f077b Compare December 3, 2024 14:03
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 e5f077b in 18 seconds

More details
  • Looked at 69 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. tests/test_cli.py:279
  • Draft comment:
    Remove unnecessary print statement to keep test output clean.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The test case test_chain in tests/test_cli.py is printing the result output, which is unnecessary for a test case unless debugging. This should be removed to keep the test output clean.
2. tests/test_prompt_tools.py:47
  • Draft comment:
    Simplify multi-line string to a single line for better readability and consistency.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The test case test_tool_format_option in tests/test_prompt_tools.py is using a multi-line string for expected output, which can be simplified to a single line for better readability and consistency with other test cases.

Workflow ID: wflow_GTGlRc8aZ9szcxzM


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 77593d0 in 13 seconds

More details
  • Looked at 19 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_D4QHEWiG4fJrEtOW


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@ErikBjare ErikBjare merged commit fe6730c into master Dec 3, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants