-
Notifications
You must be signed in to change notification settings - Fork 501
fix: persist container_id in codeInterpreterTool history #260
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: persist container_id in codeInterpreterTool history #260
Conversation
🦋 Changeset detectedLatest commit: 9fdb8d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
Hi @vrtnis, thanks for sending this pull request. If I understand this pull request correctly,
For these two reasons, I don't think this change should be merged. |
|
Thanks so much for the thoughtful guidance @seratch and context on the camelCase convention 😄! really appreciate you taking the time to clarify. You are right that the Essentially, after the first run, history holds While keeping everything snake case, i'm thinking that we can patch getInputItems in const containerId =
providerData.containerId ?? providerData.container_id;This keeps the key in snake_case the whole time, avoids any new helpers or conversions, and ensures the replay works as expected. Would be happy to open a separate (and much slimmer!) PR for this fix if that sounds good. Let me know! |
|
Thanks for pointing the cause out. Definitely, the line https://github.com/openai/openai-agents-js/blob/v0.0.13/packages/agents-openai/src/openaiResponsesModel.ts#L544 is the cause of the issue. No need to consider Having the single line of change should resolve the issue. So, including the change and changeset file in this PR would be appreciated. |
a2893e2 to
9fdb8d3
Compare
|
Just pushed an update, thanks again for the helpful feedback! I’ve removed the camelCase handling and made the one-line fix to directly use |
seratch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Replaying a full run history with the
codeInterpreterToolfails because the API’scontainer_idstayed in snake_case and never got mapped toproviderData.containerId. As such, I've added a smallsnakeToCamelCasehelper so incomingcontainer_idbecomescontainerId, and we still convert it back when building the next request. Now the container ID survives in history and a secondrunner.run(...)won’t throw “missing container_id” anymore.Changes:
snakeToCamelCaseutilityconvertToOutputItemgetInputItemsWith these changes, replaying the full
result.historynow succeeds on the secondrunner.run(...)and thecontainer_idis correctly preserved across runs.Resolves #253