feat(hooks): support union types and list of types for add_hook#1719
feat(hooks): support union types and list of types for add_hook#1719
Conversation
|
Assessment: Approve Well-implemented feature that cleanly extends the hooks API to support union types and list-based event type registration. The implementation is clean, tests are comprehensive, and the change is backward compatible. Review Notes
Note: Per API_BAR_RAISING.md, since this PR modifies the public |
- Add union type support (A | B and Union[A, B]) for callback type hints - Allow passing list of event types to add_hook/add_callback - Remove unused **kwargs from Agent.add_hook() - Deduplicate event types when registering callbacks - Validate all types in unions and lists are BaseHookEvent subclasses - Error on None or invalid types in unions Resolves #1714
90c390a to
52f9f9d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
/strands |
- Use set() for deduplication instead of manual loop - Replace type: ignore with cast() for explicit type narrowing - Restore missing test case
|
Assessment: Approve ✅ All review feedback has been addressed:
Code is clean, tests pass with 100% coverage on modified lines. Ready to merge. |
|
Review Update: All feedback has been addressed ✅ Changes verified:
LGTM - ready for merge. |
Motivation
Hook providers often need to register the same callback for multiple event types, such as logging events before and after model calls. Currently, this requires either:
add_hook()calls for the same callbackThis change enables a cleaner API where a single callback can be registered for multiple event types by using union type hints or passing a list of types explicitly.
Resolves #1714
Public API Changes
Agent.add_hook()andHookRegistry.add_callback()now support union types and lists of event types:The
**kwargsparameter was removed fromAgent.add_hook()as it was unused.Use Cases