fix(anthropic): upgrade supported version of anthropic SDK#604
Merged
nirga merged 1 commit intotraceloop:mainfrom Jun 10, 2025
Merged
fix(anthropic): upgrade supported version of anthropic SDK#604nirga merged 1 commit intotraceloop:mainfrom
nirga merged 1 commit intotraceloop:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 8322a20 in 2 minutes and 8 seconds. Click for details.
- Reviewed
504lines of code in3files - Skipped
1files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/instrumentation-anthropic/package.json:48
- Draft comment:
Dependency upgrade to '@anthropic-ai/sdk' (^0.50.4) looks good. Please ensure that any breaking changes in the new SDK version are fully compatible with the instrumentation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure compatibility with a dependency upgrade, which violates the rule against asking for confirmation or verification of intentions. It does not provide a specific suggestion or point out a specific issue.
2. packages/instrumentation-anthropic/src/instrumentation.ts:64
- Draft comment:
Passing the moduleExports parameter to patchAnthropic improves compatibility with the new SDK. Verify that all call sites now supply the correct module reference. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/instrumentation-anthropic/src/instrumentation.ts:380
- Draft comment:
The use of 'any' casts (e.g. for promise.responsePromise and promise.parseResponse) might hide potential type safety issues. Consider using proper typings if available. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment identifies a real issue with type safety. However, looking at the code more carefully, theseanycasts appear to be necessary to work with the Anthropic SDK's internal API. The properties being accessed (responsePromiseandparseResponse) seem to be internal implementation details of the SDK that aren't exposed in the public types. This suggests the casts may be unavoidable without changes to the SDK itself. I could be wrong about whether proper types exist in the SDK - there might be a way to access these properties in a type-safe way that I'm missing. Even if better types exist, the comment doesn't provide specific guidance on how to fix it. And if the types don't exist in the SDK, this isn't something the PR author can easily fix. The comment should be deleted because it points out an issue that likely can't be fixed without SDK changes, and doesn't provide actionable guidance.
4. packages/instrumentation-anthropic/test/instrumentation.test.ts:234
- Draft comment:
The previously skipped streaming test for messages is now enabled with an updated model. Confirm that the test expectations match the new SDK behavior. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_umYvvyBjEAmtJIdF
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| completion: "", | ||
| stop_reason: null, | ||
| }; | ||
| for await (const chunk of await promise) { |
Contributor
There was a problem hiding this comment.
In the _streamingWrapPromise iterateStream function, the completion branch uses for await (const chunk of await promise) while the chat branch iterates over stream. This inconsistency may lead to unexpected behavior. Consider replacing it with for await (const chunk of stream) for consistency.
f01aee3 to
2c71715
Compare
- supports newer series of Anthropic SDKs which use overriden Promise classes and Stream classes - enables the mostly-finished streaming chat completion support in the implementation and tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Upgrade Anthropic SDK to v0.50.4 and update instrumentation for new streaming capabilities.
@anthropic-ai/sdkfrom^0.32.1to^0.50.4inpackage.json.patchAnthropic()ininstrumentation.tsto handle newAPIPromiseandStreamclasses._streamingWrapPromise()to support new streaming chat completion.patch()andunpatch()methods to accommodate SDK changes.instrumentation.test.ts.This description was created by
for 8322a20. You can customize this summary. It will automatically update as commits are pushed.