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: anthropic provider on raw message #507

Merged
merged 9 commits into from
Nov 18, 2024
Merged

Conversation

teocns
Copy link
Contributor

@teocns teocns commented Nov 14, 2024

🔍 Review Summary

Purpose:
Enhance reliability and response handling logic in the AnthropicProvider class within the agentops/llms/anthropic.py file.

Key Changes:

  • Refactor: Reorganized import statements for improved clarity and moved import json to the top. Improved response handling logic to correctly parse and log data for APIResponse and LegacyAPIResponse types.
  • Enhancement: Enhanced logic to handle different response structures, including extracting model and token usage information.
  • Style: Made minor formatting changes for better code readability and maintainability.

Impact:
Improves the reliability of the AnthropicProvider class by ensuring correct parsing and logging of response data and enhancing the handling of different response structures.

Original Description

No existing description found

@teocns teocns changed the title refactor(anthropic): reorganize imports and improve response handling fix: anthropic provider on raw message Nov 14, 2024
Maybe [bedrock,vertex] is not what we're looking for, but picking these
for now as its what anthropic's computer vision demo uses
Copy link

codecov bot commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 11.53846% with 23 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agentops/llms/anthropic.py 11.53% 23 Missing ⚠️
Flag Coverage Δ
unittests 54.77% <11.53%> (-0.17%) ⬇️

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

Files with missing lines Coverage Δ
agentops/llms/anthropic.py 11.29% <11.53%> (-0.29%) ⬇️

@teocns
Copy link
Contributor Author

teocns commented Nov 15, 2024

Blocked by #510

Only static analysis checks are failing (black); free to merge afterwards


Edit 17/11
Free to merge

teocns and others added 2 commits November 17, 2024 18:19
Copy link
Contributor

@areibman areibman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to ask @the-praxs to double check this but looks good

agentops/llms/anthropic.py Outdated Show resolved Hide resolved
Copy link
Member

@the-praxs the-praxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes needed.

I would remove requirement for the Bedrock and Vertex clients. But we can add support for them in a different PR.

agentops/llms/anthropic.py Outdated Show resolved Hide resolved
agentops/llms/anthropic.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@teocns teocns enabled auto-merge (squash) November 18, 2024 10:29
Copy link
Member

@the-praxs the-praxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@teocns teocns merged commit 7e1c32f into main Nov 18, 2024
10 of 11 checks passed
@teocns teocns deleted the anthropic-fix-raw-message branch November 18, 2024 11:12
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.

3 participants