Fix #283 #291 #300 migrate ai-sdk/provider to v2 #356
Conversation
seratch
added
enhancement
🦋 Changeset detectedLatest commit: adf64aa The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@openai/agents-extensions': minor | |||
There was a problem hiding this comment.
@dkundel-openai Since this could be a breaking change to existing users, I think it's worth bumping the minor version. As for the compatibility with Python SDK, it's already v0.2 for different reasons (e.g., adding realtime agents). So, this project can decide when to upgrade the version now. Do you agree?
There was a problem hiding this comment.
is AI SDK v2 stable for us to go and fully cut over or is there any value to maintain both for now? Like should we copy the code and expose aisdk.v1(...) and aisdk.v2(...). Not opposed to a minor bump more concerned if one breaks the other.
There was a problem hiding this comment.
@dkundel-openai a challenge here is that you cannot have both v1 and v2 as peer dependencies for the same package. If we want to support both, we need to have two separate sub packages like @openai/agents-ai-sdk-v1 and @openai/agents-ai-sdk-v2.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@openai/agents-extensions': minor | |||
There was a problem hiding this comment.
is AI SDK v2 stable for us to go and fully cut over or is there any value to maintain both for now? Like should we copy the code and expose aisdk.v1(...) and aisdk.v2(...). Not opposed to a minor bump more concerned if one breaks the other.
| type: 'file', | ||
| file: c.file, | ||
| mimeType: 'application/octet-stream', | ||
| mediaType: 'application/octet-stream', |
There was a problem hiding this comment.
Might want to use the original mediaType prop here - this caused a bug for us when trying to upload PDFs where Anthropic returned "application/octet-stream" isn't supported
There was a problem hiding this comment.
Thanks for the comment. At this moment, this could be only a PDF file, but we may support other file formats as well, so we hesitate to change this just for the provider. Also, the code here does not have the media/mimeType for the file input here. Enabling users to explicitly pass mediaType for the compatibility may be worth considering, but I am not sure if we can include the enhancement this time.
There was a problem hiding this comment.
No worries, just wanted to make you guys aware of it if not already. I actually was already using a custom version of this file anyways so our workaround was just to pass the mime type through the provider data which seems to be working fine (we are using PDFs but just using them from a web URL instead of base64 data url).
Cheers
seratch
force-pushed
the
issue-300-ai-sdk-provider-v2
branch
from
0b71d61 to
90456ec
Compare
|
@dkundel-openai I've added v1 module under examples: https://github.com/openai/openai-agents-js/blob/issue-300-ai-sdk-provider-v2/examples/ai-sdk-v1/index.ts After migrating our latest package to provider v2, we can share the v1 example for developers who need v1 for a certain reason. |
| const sleep = (ms: number) => | ||
| new Promise((resolve) => setTimeout(resolve, ms)); |
There was a problem hiding this comment.
It would be better if we add this function to utils. we are repeating it in 3 diffrent places.
seratch
force-pushed
the
issue-300-ai-sdk-provider-v2
branch
from
adf64aa to
78cdd43
Compare
This pull request resolves the following issues:
Resolves #283
Resolves #291
Resolves #300
I've done with basic tests with a few providers, but if anyone help us do more testing before releasing this change, it'd be greatly appreciated. To verify if it works for your use case, you can simply copy the
aiSdk.tsand put the file in your project.