feat(plugins): improve plugin creation devex with @hook and @tool decorators#1740
feat(plugins): improve plugin creation devex with @hook and @tool decorators#1740
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
d652f8e to
00f08ea
Compare
00f08ea to
655ffd3
Compare
- Create @hook decorator for declarative hook registration in plugins - Convert Plugin from Protocol to base class (breaking change) - Add auto-discovery of @hook and @tool decorated methods in Plugin.__init__() - Add auto-registration of hooks and tools in Plugin.init_plugin() - Support union types for multiple event types (e.g., BeforeModelCallEvent | AfterModelCallEvent) - Export hook from strands.plugins and strands namespaces - Update existing tests to use inheritance-based approach - Add comprehensive test coverage for new functionality BREAKING CHANGE: Plugin is now a base class instead of a Protocol. Existing plugins must inherit from Plugin instead of just implementing the protocol.
655ffd3 to
f2f74e4
Compare
mkmeral
left a comment
There was a problem hiding this comment.
Also couple of pointers from my agent 😅
My main things are the other two reviews though, making tools/hooks public and moving the logic to registry to simplify init plugin devx (getting rid of the extra super call for plumbing)
Type annotation bug in _WrappedHookCallable
src/strands/plugins/decorator.py:25 — _hook_event_types is list[TEvent] but stores classes, not instances. Should be list[type[TEvent]]. Confirmed at runtime: type(handler._hook_event_types[0]) is <class 'type'>.
# Fix
_hook_event_types: list[type[TEvent]]@hook not exported from top-level strands package
from strands import tool works but from strands import hook does not. These are symmetrical decorators plugin authors will use together. Add to src/strands/__init__.py:
from .plugins import Plugin, hookHook discovery order is alphabetical, not definition order
plugin.py:82 uses dir(self) which returns alphabetically. A plugin with z_validate and a_log hooks on the same event registers a_log first. Surprising when ordering matters. Fix with __mro__ + vars() (Python 3.7+ guarantees dict insertion order), or at minimum document the behavior.
No public API for plugin.hooks / plugin.tools
Discovered hooks/tools live in _hooks/_tools — private mutable lists with no public accessors. Users can mutate them (p._hooks.clear(), p._hooks.append(...)) but it's undocumented and fragile. Either:
- Document that customization goes through
init_pluginoverride (minimum for this PR) - Add read-only properties returning tuples (follow-up)
Docstring examples use @tool without showing its import
plugins/__init__.py:19 and plugin.py:43 show @tool alongside @hook but never import it. Users copy-pasting get a NameError. Add from strands.tools.decorator import tool to the examples.
Docstring typo in _WrappedHookCallable
decorator.py:24 says "includes a _hook_event_types argument" — should be "attribute".
DevX before/after samples
Simple plugin:
# BEFORE — 3 lines of boilerplate
class LoggingPlugin(Plugin):
name = "logging"
def init_plugin(self, agent):
agent.add_hook(self._log, BeforeModelCallEvent)
def _log(self, event: BeforeModelCallEvent) -> None:
print(f"Model call for {event.agent.name}")
# AFTER
class LoggingPlugin(Plugin):
name = "logging"
@hook
def log(self, event: BeforeModelCallEvent) -> None:
print(f"Model call for {event.agent.name}")Multiple hooks — no registration list to keep in sync:
class AuditPlugin(Plugin):
name = "audit"
@hook
def before_model(self, event: BeforeModelCallEvent) -> None: ...
@hook
def after_model(self, event: AfterModelCallEvent) -> None: ...
@hook
def before_tool(self, event: BeforeToolCallEvent) -> None: ...Union types — one annotation, two registrations:
class MonitorPlugin(Plugin):
name = "monitor"
@hook
def on_model(self, event: BeforeModelCallEvent | AfterModelCallEvent) -> None:
print(f"Model event: {type(event).__name__}")Hooks + tools — both auto-discovered:
class MetricsPlugin(Plugin):
name = "metrics"
def __init__(self):
super().__init__()
self.calls = 0
@hook
def count(self, event: BeforeModelCallEvent) -> None:
self.calls += 1
@tool
def get_metrics(self) -> str:
"""Get call count."""
return f"Calls: {self.calls}"Hybrid — decorated + conditional manual hooks:
class ConfigPlugin(Plugin):
name = "config"
def __init__(self, verbose=False):
super().__init__()
self.verbose = verbose
@hook
def always_on(self, event: BeforeModelCallEvent) -> None: ...
def verbose_hook(self, event: BeforeInvocationEvent) -> None: ...
def init_plugin(self, agent):
super().init_plugin(agent) # registers @hook methods
if self.verbose:
agent.add_hook(self.verbose_hook, BeforeInvocationEvent)|
/strands |
…properties Address PR feedback: - Add public 'hooks' and 'tools' properties returning tuples for user customization - Move hook/tool auto-registration from Plugin.init_plugin() to _PluginRegistry.add_and_init() - Remove need for super().init_plugin(agent) - users only implement custom logic - Update steering handler to use new simpler pattern - Update all tests to use registry-based registration This simplifies plugin development: - Before: Users had to call super().init_plugin(agent) for auto-registration - After: init_plugin() is purely for custom logic, registry handles auto-registration
|
/strands consider these comments, and my reply on immutable tools and hooks commentsType annotation bug:
|
Address additional PR feedback: - Make hooks and tools properties return mutable lists for filtering/customization - Fix type annotation: _hook_event_types is list[type[TEvent]] not list[TEvent] - Export @hook from top-level strands package (from strands import hook) - Fix docstring typo: 'argument' -> 'attribute' - Add tests for filtering hooks and tools
src/strands/plugins/plugin.py
Outdated
| Attributes: | ||
| name: A stable string identifier for the plugin | ||
| name: A stable string identifier for the plugin (must be provided by subclass) | ||
| hooks: Tuple of discovered @hook decorated methods (read-only) |
There was a problem hiding this comment.
comment is out of date, we should remove read-only here
There was a problem hiding this comment.
Fixed! Updated docstring to say "mutable for filtering" instead of "read-only".
There was a problem hiding this comment.
Updated to simpler wording: "List of hooks/tools the plugin provides, auto-discovered from decorated methods"
|
/strands |
- Fix Plugin class docstring: 'read-only' -> 'mutable for filtering' - Add _type_inference.py to AGENTS.md hooks directory listing
|
/strands don't say mutable or immutable for in the comment. just say this is a list of hooks/tools the plugin provides, autogenerated from decorators |
Remove mutable/immutable language, just describe what they are: - List of hooks/tools the plugin provides - Auto-discovered from decorated methods
|
/strands for discovering decorated methods, instead of using dir, let's use MRO (method resolution order), so we keep the order of hooks as they are defined in the class. See example code def _discover_decorated_methods(self) -> None:
"""Scan class for @hook and @tool decorated methods in declaration order."""
seen = set()
# Walk MRO so parent class hooks come first, child overrides win
for cls in reversed(type(self).__mro__):
for name, attr in cls.__dict__.items():
if name in seen:
continue
seen.add(name)
# Get the bound method from self
try:
bound = getattr(self, name)
except Exception:
continue
if hasattr(bound, "_hook_event_types") and callable(bound):
self._hooks.append(bound)
if isinstance(bound, DecoratedFunctionTool):
self._tools.append(bound) |
- Replace dir() with __mro__ iteration to maintain declaration order - Parent class hooks come first, child overrides win - Add test verifying definition order is preserved (not alphabetical)
|
Updated hook/tool discovery to use MRO instead of |
Motivation
Currently, plugin authors must manually register hooks in their
init_plugin()method, which is verbose and error-prone:This PR enables declarative hook registration using a
@hookdecorator, making plugin development more intuitive and reducing boilerplate:Resolves: #1739
Public API Changes
New
@hookdecoratorThe
@hookdecorator marks methods for automatic registration: