Drop name field of hooks (#67)

Instead of solving the collision of names bwetween internal/external
hooks lets remove these all together, these exist such that we have
something to pass to LC, but other that that these don't serve any
usercase (except in debug logs, where we have been printing these
names).

This change removes the name field of hooks, and while converting to LC
we generate a random uuid4 to name these hooks.

To not loose the DEBUG logging experience we infer a name for logs for these
from the class/function name.

Author: mateusz834

splunklib/ai/agent.py

@@ -137,9 +137,6 @@ def __init__(
             logger=logger,
         )
 
-        if duplicate_hook_names := _find_duplicate_hook_names(self.hooks):
-            raise ValueError(f"Duplicate hook names found: {duplicate_hook_names!r}")
-
         self._use_mcp_tools = use_mcp_tools
         self._tool_filters = tool_filters
         self._service = service
@@ -215,19 +212,3 @@ async def _load_tools_from_mcp(
     )
 
     return mcp_tools
-
-
-def _find_duplicate_hook_names(hooks: Sequence[AgentHook] | None) -> set[str]:
-    seen: set[str] = set()
-    duplicates: set[str] = set()
-
-    if not hooks:
-        return set()
-
-    for hook in hooks:
-        if hook.name in seen:
-            duplicates.add(hook.name)
-        else:
-            seen.add(hook.name)
-
-    return duplicates

splunklib/ai/engines/langchain.py

@@ -62,6 +62,7 @@
 from splunklib.ai.hooks import (
     AgentHook,
     AgentState,
+    FunctionHook,
     after_model as hook_after_model,
     before_model as hook_before_model,
 )
@@ -229,8 +230,6 @@ async def create_agent(
 def _debugging_middleware(
     logger: logging.Logger,
 ) -> tuple[list[AgentHook], list[AgentHook], list[LC_AgentMiddleware]]:
-    # TODO: These names can conflict with user-provided names.
-
     # TODO: replace this with ours middleware, once we add them.
     @wrap_tool_call  # pyright: ignore[reportArgumentType, reportCallIssue, reportUntypedFunctionDecorator]
     async def _tool_call(
@@ -489,15 +488,25 @@ def _convert_hook_to_middleware(
     model: BaseChatModel,
     logger: logging.Logger | None = None,
 ) -> LC_AgentMiddleware:
+    # Inspect the hook to generate a useful name for debug log messages.
+    hook_name = hook.__class__.__name__
+    if isinstance(hook, FunctionHook):
+        hook_name = hook.func.__name__
+
+    # Generate a random name to name this hook in langchain.
+    # We can't use the hook_name, derived above, since it might not be unique, we
+    # also don't want to force the users to name these hooks, as langchain does.
+    lc_hook_name = str(uuid.uuid4())
+
     match hook.type:
         case "before_model":
-            wrapper = before_model(can_jump_to=["end"], name=hook.name)
+            wrapper = before_model(can_jump_to=["end"], name=lc_hook_name)
         case "after_model":
-            wrapper = after_model(can_jump_to=["end"], name=hook.name)
+            wrapper = after_model(can_jump_to=["end"], name=lc_hook_name)
         case "before_agent":
-            wrapper = before_agent(can_jump_to=["end"], name=hook.name)
+            wrapper = before_agent(can_jump_to=["end"], name=lc_hook_name)
         case "after_agent":
-            wrapper = after_agent(can_jump_to=["end"], name=hook.name)
+            wrapper = after_agent(can_jump_to=["end"], name=lc_hook_name)
         case _:
             raise AssertionError(f"Unsupported middleware type: {hook.type}")
 
@@ -517,7 +526,7 @@ async def _middleware(
         sdk_state = _convert_agent_state_from_langchain(state, model)
 
         if logger:
-            logger.debug(f"Executing {hook.type} hook {hook.name}")
+            logger.debug(f"Executing {hook.type} hook {hook_name}")
 
         res = hook(sdk_state)
         if isawaitable(res):

splunklib/ai/hooks.py

@@ -31,8 +31,6 @@ class AgentHook(Protocol):
     """
 
     type: HookType
-    # Name of the middleware must be unique
-    name: str
 
     def __call__(self, state: AgentState) -> None | Awaitable[None]:
         """Called at specific points during the agent execution, depending on the hook type."""
@@ -63,48 +61,54 @@ def __init__(self, timeout_seconds: float) -> None:
         super().__init__(f"Timed out after {timeout_seconds} seconds.")
 
 
-def _create_hook(
-    type: HookType,
-    func: Callable[[AgentState], None | Awaitable[None]],
-    name: str | None = None,
-) -> AgentHook:
-    mw_name = name or func.__name__
-    mw_type = type
+@final
+class FunctionHook(AgentHook):
+    """
+    Implementation of AgentHook that wraps a single callable function.
+
+    FunctionHook allows creation of a hook from a plain function instead of
+    defining a full AgentHook subclass.
+
+    Use helper decorators: before_model, after_model, before_agent, after_agent to
+    construct such hook.
+    """
 
-    @final
-    class CustomHook(AgentHook):
-        type = mw_type
-        name = mw_name
+    type: HookType
+    func: Callable[[AgentState], None | Awaitable[None]]
 
-        @override
-        def __call__(self, state: AgentState) -> None | Awaitable[None]:
-            return func(state)
+    def __init__(
+        self, hookType: HookType, func: Callable[[AgentState], None | Awaitable[None]]
+    ) -> None:
+        self.type = hookType
+        self.func = func
 
-    return CustomHook()
+    @override
+    def __call__(self, state: AgentState) -> None | Awaitable[None]:
+        return self.func(state)
 
 
 def before_model(func: Callable[[AgentState], None | Awaitable[None]]) -> AgentHook:
     """This hook is called before each model call."""
 
-    return _create_hook("before_model", func)
+    return FunctionHook("before_model", func)
 
 
 def after_model(func: Callable[[AgentState], None | Awaitable[None]]) -> AgentHook:
     """This hook is called after each model call."""
 
-    return _create_hook("after_model", func)
+    return FunctionHook("after_model", func)
 
 
 def before_agent(func: Callable[[AgentState], None | Awaitable[None]]) -> AgentHook:
     """This hook is called once per agent invocation. Before any model calls."""
 
-    return _create_hook("before_agent", func)
+    return FunctionHook("before_agent", func)
 
 
 def after_agent(func: Callable[[AgentState], None | Awaitable[None]]) -> AgentHook:
     """This hook is called once per agent invocation. After all model calls."""
 
-    return _create_hook("after_agent", func)
+    return FunctionHook("after_agent", func)
 
 
 def token_limit(limit: float) -> AgentHook:
@@ -114,7 +118,7 @@ def _token_limit_hook(state: AgentState) -> None:
         if state.token_count > limit:
             raise TokenLimitExceededException(token_limit=limit)
 
-    return _create_hook("before_model", _token_limit_hook, name="builtin_token_limit")
+    return FunctionHook("before_model", _token_limit_hook)
 
 
 def step_limit(limit: int) -> AgentHook:
@@ -124,7 +128,7 @@ def _step_limit_hook(state: AgentState) -> None:
         if state.total_steps >= limit:
             raise StepsLimitExceededException(steps_limit=limit)
 
-    return _create_hook("before_model", _step_limit_hook, name="builtin_step_limit")
+    return FunctionHook("before_model", _step_limit_hook)
 
 
 def timeout_limit(seconds: float) -> AgentHook:
@@ -137,6 +141,4 @@ def _timeout_limit_hook(_state: AgentState) -> None:
         if monotonic() >= timeout:
             raise TimeoutExceededException(timeout_seconds=seconds)
 
-    return _create_hook(
-        "before_model", _timeout_limit_hook, name="builtin_timeout_limit"
-    )
+    return FunctionHook("before_model", _timeout_limit_hook)

tests/integration/ai/test_hooks.py

@@ -38,21 +38,6 @@
 
 
 class TestHook(AITestCase):
-    @pytest.mark.asyncio
-    async def test_agent_hooks_duplicated(self):
-        pytest.importorskip("langchain_openai")
-
-        with pytest.raises(
-            ValueError, match="Duplicate hook names found: {'builtin_step_limit'}"
-        ):
-            async with Agent(
-                model=(await self.model()),
-                system_prompt="Your name is stefan",
-                service=self.service,
-                hooks=[step_limit(5), step_limit(10)],
-            ) as agent:
-                ...
-
     @pytest.mark.asyncio
     async def test_agent_hook(self):
         pytest.importorskip("langchain_openai")
@@ -62,7 +47,6 @@ async def test_agent_hook(self):
         @final
         class TestHook(AgentHook):
             type = "before_model"
-            name = "test_async_hook"
 
             @override
             def __call__(self, state: AgentState) -> None:
@@ -73,7 +57,6 @@ def __call__(self, state: AgentState) -> None:
         @final
         class TestAsyncHook(AgentHook):
             type = "before_model"
-            name = "test_hook"
 
             @override
             async def __call__(self, state: AgentState) -> None: