Skip to content

fix(hooks): handle kwargs in hook calls #1002

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Quratulain-bilal
Copy link

@Quratulain-bilal Quratulain-bilal commented Jul 3, 2025

Issue 1: Ensure Fallback Name Handling in input_guardrail

Problem

The input_guardrail logic previously allowed the name attribute to be None if not explicitly provided. This caused issues in downstream systems such as logging, tracing, or serialization that expect a valid string name.

Changes

Introduced fallback logic to assign a default name if name is None.
Guarantees that all function metadata includes a valid string name.
Prevents unpredictable behavior or runtime errors in downstream processes.

Impact

Improves system robustness and fault tolerance.
Ensures consistent behavior in tools relying on function names.
No breaking changes introduced.

Issue 2: Standardize Hook Callback Signatures with **kwargs

Problem

Lifecycle hooks in AgentHooks and RunnerHooks were inconsistent:
on_handoff used **kwargs.
Other hooks like on_agent_start, on_tool_end, etc., used positional arguments.
This inconsistency led to:
Poor developer experience (requiring memorization of argument order).
Risk of bugs and lower maintainability.

Changes

Refactored all hook signatures to accept **kwargs.
Affected hooks include:
on_agent_start
on_agent_end
on_tool_start
on_tool_end
Internal invocation already uses named arguments, so no functional change occurred.

Impact

Improves consistency across the SDK.
Simplifies custom hook implementations.
Enables forward-compatible API evolution.

@seratch seratch added bug Something isn't working feature:core labels Jul 8, 2025
@seratch seratch requested a review from rm-openai July 8, 2025 13:21
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, the issue 1 should be resolved

@seratch
Copy link
Member

seratch commented Jul 8, 2025

It seems the CI builds are failing

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test failures need to be resolved first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants