-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add tool call parameters for on_tool_start
hook
#253
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
base: main
Are you sure you want to change the base?
Conversation
@yanmxa thanks for doing this work, very useful! |
fccd3fa
to
656f211
Compare
much needed feature! |
I learned that if you use Example: result = Runner.run_streamed(
agent,
input,
)
async for event in result.stream_events():
if event.type == "run_item_stream_event":
if event.name == "tool_called":
print(
f"-- Tool {event.item.raw_item.name} was called with args: {event.item.raw_item.arguments} "
)
elif event.name == "reasoning_item_created":
summary = event.item.raw_item.summary
# there’s at least one summary part
# o-series models will send this every response, it is sometimes blank
if summary:
text = summary[0].text
print(f"-- Reasoning item created: {text}")
print("--")
elif event.name == "message_output_created":
print(event.item.raw_item.content[0].text)
else:
pass # Ignore other event types |
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.
Apologies for missing this PR. It looks good, but is a breaking change. We could certainly make a breaking change but I'd like to avoid it unless absolutely necessary. Another option could be to add a new method for the args. Thoughts on which would be better?
Thanks @rm-openai! I tend to favor the breaking change approach because:
The community feedback shows this is needed. A clean break now might be better than API fragmentation. What do you think? |
@rm-openai do you have any ETA on this PR? |
Signed-off-by: myan <[email protected]> fix: resolve lint issues - Fix import sorting in multiple files - Remove duplicate ResponseComputerToolCall import - Organize imports according to ruff rules 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> fix: resolve lint issues - Fix import sorting in multiple files - Remove duplicate ResponseComputerToolCall import - Organize imports according to ruff rules 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> fix: restore correct context parameter for function tool hooks - Function tools should use tool_context (ToolContext) - Computer/shell tools use context_wrapper (RunContextWrapper) - This maintains consistency with original codebase 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> chore: remove VSCode configuration files from PR - Remove .vscode/ directory and all IDE-specific configuration - These files should not be included in the repository 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
019c5bb
to
86ec7f0
Compare
## Background Currently, the `RunHooks` lifecycle (`on_tool_start`, `on_tool_end`) exposes the `Tool` and `ToolContext`, but does not include the actual arguments passed to the tool call. resolves #939 ## Solution This implementation is inspired by [PR #1598](#1598). * Add a new `tool_arguments` field to `ToolContext` and populate it via from_agent_context with tool_call.arguments. * Update `lifecycle_example.py` to demonstrate tool_arguments in hooks * Unlike the proposal in [PR #253](#253), this solution is not expected to introduce breaking changes, making it easier to adopt.
Resolve: #252