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: fixed support for groq (and deepseek?) #231

Merged
merged 1 commit into from
Oct 27, 2024
Merged

Conversation

ErikBjare
Copy link
Owner

@ErikBjare ErikBjare commented Oct 27, 2024

This also potentially unblocks: #180

Important

Refactored provider handling in msgs2dicts and related functions to improve support for different providers in llm_anthropic.py, llm_openai.py, and message.py.

  • Behavior:
    • Updated msgs2dicts function in message.py to use provider parameter instead of openai and anthropic boolean flags.
    • Added get_provider function in llm_openai.py to determine the provider based on base_url.
    • Modified chat and stream functions in llm_anthropic.py and llm_openai.py to use provider parameter.
  • Models:
    • Added ProvidersWithFiles list in message.py to specify providers supporting files.
    • Updated _content_files_list and to_dict methods in Message class to handle providers using ProvidersWithFiles.
  • Misc:
    • Removed openai and anthropic flags from msgs2dicts calls in llm_anthropic.py and llm_openai.py.

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

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 909d08a in 24 seconds

More details
  • Looked at 154 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. gptme/llm_openai.py:63
  • Draft comment:
    The get_provider function returns a string, but the type hint suggests it should return a Provider type. Ensure the return type matches the type hint for consistency.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. gptme/message.py:30
  • Draft comment:
    The ProvidersWithFiles list is defined here but used in multiple files. Consider centralizing this configuration to avoid duplication and potential inconsistencies.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The ProvidersWithFiles list is defined in message.py but is used in multiple files. It would be better to centralize this configuration to avoid duplication and potential inconsistencies.

Workflow ID: wflow_3f0LnZtm0QAEQL6H


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

@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 75.38%. Comparing base (65bdb8a) to head (909d08a).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
gptme/llm_openai.py 33.33% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
- Coverage   75.48%   75.38%   -0.10%     
==========================================
  Files          57       57              
  Lines        3590     3604      +14     
==========================================
+ Hits         2710     2717       +7     
- Misses        880      887       +7     
Flag Coverage Δ
anthropic/claude-3-haiku-20240307 74.30% <50.00%> (-0.21%) ⬇️
openai/gpt-4o-mini 73.91% <58.33%> (-0.10%) ⬇️

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.

@ErikBjare ErikBjare merged commit cea30cf into master Oct 27, 2024
7 checks passed
@ErikBjare ErikBjare deleted the dev/groq-fix branch October 27, 2024 20:46
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