-
Notifications
You must be signed in to change notification settings - Fork 8
fix: hooks integration pitfalls #183
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
Conversation
Signed-off-by: Amitha Dissanayake <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
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.
Pull request overview
This PR refactors the hooks integration mechanism in Agent Kernel, moving hook registration from the GlobalRuntime to a module-level API. The changes address two key issues: enabling hook attachment when using custom runtime contexts, and reducing exposure of runtime instances in application code. The PR also renames hook classes from Prehook/Posthook to PreHook/PostHook for better naming consistency.
Key changes:
- Introduces fluent API for hook registration via module methods (
pre_hook()andpost_hook()) - Moves hook storage from Runtime to individual Agent instances
- Renames hook classes to use PascalCase convention (PreHook, PostHook)
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
ak-py/src/agentkernel/core/hooks.py |
Renamed Prehook/Posthook to PreHook/PostHook, added TYPE_CHECKING imports for circular dependency handling |
ak-py/src/agentkernel/core/base.py |
Added hook storage properties and attachment methods to Agent class |
ak-py/src/agentkernel/core/runtime.py |
Updated Runtime.run() to read hooks from agents instead of internal dictionaries, removed register_*_hooks methods |
ak-py/src/agentkernel/core/module.py |
Added abstract pre_hook/post_hook methods and get_agent helper method |
ak-py/src/agentkernel/core/__init__.py |
Updated exports to use new PreHook/PostHook names |
ak-py/src/agentkernel/core/service.py |
Minor code style improvement (list initialization) |
ak-py/src/agentkernel/framework/openai/openai.py |
Implemented pre_hook/post_hook methods with fluent API support |
ak-py/src/agentkernel/framework/langgraph/langgraph.py |
Implemented pre_hook/post_hook methods with fluent API support |
ak-py/src/agentkernel/framework/crewai/crewai.py |
Implemented pre_hook/post_hook methods with fluent API support |
ak-py/src/agentkernel/framework/adk/adk.py |
Implemented pre_hook/post_hook methods with fluent API support |
examples/memory/key-value-cache/hooks.py |
Updated import to use PreHook class name |
examples/memory/key-value-cache/app.py |
Updated to use module.pre_hook() instead of runtime.register_pre_hooks() |
examples/api/openai/app.py |
Updated to use module.pre_hook() instead of runtime.register_pre_hooks() |
examples/api/hooks/hooks.py |
Updated imports and class names to PreHook/PostHook |
examples/api/hooks/demonstration.py |
Updated to use module.pre_hook() API |
examples/api/hooks/app.py |
Updated to use chained module.pre_hook().post_hook() API |
examples/api/hooks/README.md |
Updated documentation to reflect new class names and API |
docs/docs/integrations/hooks.md |
Comprehensive documentation update for new hook registration API |
docs/docs/core-concepts/session.md |
Updated hook class references in examples |
docs/docs/architecture/memory-management.md |
Updated hook class references in examples |
docs/blog/2025-12-18-hooks-and-smart-memory.md |
Updated blog post to reflect new hook registration API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Amitha Dissanayake <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
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.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Amitha Dissanayake <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
examples/api/hooks/demonstration.py:33
- This comment is now inaccurate. The hooks are registered on line 30, not after getting the runtime. The comment should be updated to reflect that we're only getting the runtime instance here, not registering hooks. Consider changing it to "Get runtime instance and create session".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
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.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
examples/api/hooks/demonstration.py:33
- The comment on line 32 states "Get runtime and register hooks" but the hooks have already been registered on line 30 via the module's pre_hook method. This comment is now outdated and misleading. It should be updated to something like "Get runtime for agent execution" or simply removed since the runtime is only being used to get agents and run them.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Hooks are currently being applied to via agents by Global Runtime instance. This creates two problems:
Type of Change
Changes Made
Testing
Checklist
Additional Notes
Key value store implementation should also be refactored similarly