-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Added support for passing tool_call_id via the RunContextWrapper #766
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
@rm-openai - This addressed the issue we discussed in another thread, would love you review :) |
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.
Sorry for delayed review. Was thinking about it, and it feels a bit bad to have tool_call_id
be nullable.
What if instead we had ToolContext
subclass RunContextWrapper. That way, existing usecases would just work since the downcast is fine. And tool_call_id
would be non-null always?
@rm-openai - I added a |
@rm-openai - bumping this 😄 |
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.
Looks good, please just fix the typecheck etc and I can merge
awesome thanks for implementing this! |
…nai#766) This PR fixes issue: openai#559 By adding the tool_call_id to the RunContextWrapper prior to calling tools. This gives the ability to access the tool_call_id in the implementation of the tool.
This PR fixes issue: #559
By adding the tool_call_id to the RunContextWrapper prior to calling tools. This gives the ability to access the tool_call_id in the implementation of the tool.