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 integration when using tool calls #3615

Merged
merged 10 commits into from
Oct 14, 2024

Conversation

kwnath
Copy link
Contributor

@kwnath kwnath commented Oct 5, 2024

Description

If you've initialized Sentry with Anthropic integration, streaming responses with tool calls fail.

The reason is quite simple, the API introduced a new data model for a ContentBlockDeltaEvent. This new refactor was made in 0.27.0 because json was needed to be streamed back (so arguments were returned as json for tool calls, I think this was the initial reason). This introduced a new data type InputJsonDelta that had it's contents in partial_json instead of what is currently supported text (reference).

And you'll find in the original integration we try to get the text if the type is a content_block_delta, this doesn't exist if you're doing any sort of tool calling with streaming. This doesn't fail however if you turn off streaming.

More info on reproduction steps here

Side notes

  • Backwards compatibility is probably not that important but I'd rather be safe here.
  • I've only updated this for ContentBlockDeltaEvent, there are likely other incompatible areas but this is the most breaking one. (I can add a few follow up PR's)

Handles InputJsonDelta for anthropic >= 0.27.0 which broke tool calling in streams.
@kwnath
Copy link
Contributor Author

kwnath commented Oct 8, 2024

@antonpirker is this something you might have some time to look at 😅

@antonpirker
Copy link
Member

Will have a look at on Thursday

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.32%. Comparing base (759d6e9) to head (792e46b).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/integrations/anthropic.py 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3615   +/-   ##
=======================================
  Coverage   84.31%   84.32%           
=======================================
  Files         133      133           
  Lines       14056    14063    +7     
  Branches     2377     2379    +2     
=======================================
+ Hits        11851    11858    +7     
  Misses       1471     1471           
  Partials      734      734           
Files with missing lines Coverage Δ
sentry_sdk/integrations/anthropic.py 83.03% <90.00%> (+0.17%) ⬆️

... and 3 files with indirect coverage changes

@antonpirker antonpirker added the Trigger: tests using secrets PR code is safe; run CI label Oct 11, 2024
@github-actions github-actions bot removed the Trigger: tests using secrets PR code is safe; run CI label Oct 11, 2024
@antonpirker antonpirker added the Trigger: tests using secrets PR code is safe; run CI label Oct 11, 2024
@antonpirker antonpirker changed the title Fix Anthropic integration when streaming tool calls for Anthropic python sdk >= 0.27.0 Fix Anthropic integration when using tool calls Oct 11, 2024
@github-actions github-actions bot removed the Trigger: tests using secrets PR code is safe; run CI label Oct 11, 2024
@antonpirker antonpirker added the Trigger: tests using secrets PR code is safe; run CI label Oct 11, 2024
@antonpirker
Copy link
Member

Hey @kwnath !

Thanks again for your contribution! I finally had some time to look at your PR. I took the liberty to change some stuff to bring it over the finish line. When all the tests have passed, this is ready to be merged!

@antonpirker antonpirker enabled auto-merge (squash) October 11, 2024 13:00
@antonpirker antonpirker self-assigned this Oct 11, 2024
@kwnath
Copy link
Contributor Author

kwnath commented Oct 11, 2024

@antonpirker Looks like the codecov/patch is stuck 😅

@github-actions github-actions bot removed the Trigger: tests using secrets PR code is safe; run CI label Oct 14, 2024
@antonpirker antonpirker added the Trigger: tests using secrets PR code is safe; run CI label Oct 14, 2024
Copy link
Member

@antonpirker antonpirker 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!

@antonpirker antonpirker merged commit cbe0135 into getsentry:master Oct 14, 2024
137 of 139 checks passed
@antonpirker
Copy link
Member

@kwnath your fix has been released in this version: https://pypi.org/project/sentry-sdk/2.17.0/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Trigger: tests using secrets PR code is safe; run CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants