JBAI-13757. Replace EventHandler with the event-handler feature #30

sdubov · 2025-05-12T12:05:38Z

PR contains the following changes:

Migrate handlers from EventHandler into AgentPipeline
Add the event-handler feature to features module
Replace usages of the EventHandler class with the new event-handle feature
Update the tracing feature with handling new events in an agent from the EventHandler class

…andle feature - Delete EventHandler class and update its usages; - Move logic related to the feature functionality from core into features module. Restored modules dependencies; - Update logic in products and tests to use a new event-handler feature instead of an old EventHandler.

… agent from the EventHandler class - Update logic to set JSON config for remote writers in agent features to correctly process with dependencies; - Add new events from the EventHandler class; - Update all tests.

devcrocod · 2025-05-13T10:41:01Z

...otlin/ai/grazie/code/agents/local/features/eventHandler/feature/EventHandlerFeatureConfig.kt

+
+    //region Trigger Agent Handlers
+
+    var onAgentCreated: suspend (strategy: LocalAgentStrategy, agent: AIAgentBase) -> Unit =


I don't think that API using setters is convenient to use

@devcrocod, could you please give some suggestions here?

I think using functions would be a little better
for example

private var agentStarted: suspend (strategyName: String) -> Unit = { strategyName: String -> } fun onAgentStarted(block: suspend (strategyName: String) -> Unit) { agentStarted = block }

So it's like a "glorified setter" /s? But agreed, seems like it is more idiomatic Kotlin

@devcrocod, we also need getters here since we need to execute this block of code on a particular interceptor call (please see EventHandler#install()). Please let me know if I miss anything.

Rizzen · 2025-05-13T11:18:29Z

agents/agents-core/src/commonMain/kotlin/ai/grazie/code/agents/core/api/LocalAIAgentExt.kt

    toolRegistry: ToolRegistry? = null,
    maxIterations: Int = 50,
-): AIAgentBase {
+installFeatures: suspend AIAgentBase.FeatureContext.() -> Unit = {}


Minor: formatting is broken here

Rizzen · 2025-05-13T11:48:46Z

agents/agents-core/src/commonMain/kotlin/ai/grazie/code/agents/core/agent/AIAgentBase.kt

-            } catch (e: Exception) {
-                logger.error(e) { "Tool \"${tool.name}\" failed to run call handler with arguments: ${content.toolArgs}" }
-            }
+            pipeline.onToolCall(stage = stage, tool = tool, toolArgs = toolArgs)

-            val (result, serializedResult) = try {
+            // Tool Execution
+            val (toolResult, serializedResult) = try {
                @Suppress("UNCHECKED_CAST")
-                (tool as Tool<Tool.Args, ToolResult>).executeAndSerialize(args, toolEnabler)
-            } catch (e: ToolException) {
-                with(eventHandler.toolValidationFailureListener) {
-                    AgentHandlerContext(strategy.name).handle(stage, tool, args, e.message)
-                }
+                (tool as Tool<Tool.Args, ToolResult>).executeAndSerialize(toolArgs, toolEnabler)
+            }
+            catch (e: ToolException) {
+
+                pipeline.onToolValidationError(stage = stage, tool = tool, toolArgs = toolArgs, error = e.message)
+
                return toolResult(
                    message = e.message,
                    toolCallId = content.toolCallId,
                    toolName = content.toolName,
                    agentId = strategy.name,
                    result = null
                )
-            } catch (e: Exception) {
-                with(eventHandler.toolExceptionListener) {
-                    AgentHandlerContext(strategy.name).handle(stage, tool, args, e)
-                }
+            }
+            catch (e: Exception) {


I have some concern regarding tools calls reporting. In the case of multiple tool calling we have following order of events:

onBeforeToolCalls

onToolCall

onToolCallResult

onAfterToolCalls

So, question #1 is - why do we have onBeforeToolCalls/onAfterToolCalls only for the bunch of calls, not for each. Moreover - onToolCall is called before actual call.
Second question is - do we really need that many events on each tool call, do we use it?

As we discussed, I've dropped onBeforeToolCalls and onAfterToolCalls handlers.

Rizzen · 2025-05-13T11:53:26Z

...i/grazie/code/agents/core/feature/remote/client/config/AgentFeatureClientConnectionConfig.kt

+class AgentFeatureClientConnectionConfig(
+    host: String,
+    port: Int,
+    protocol: String = "https",


I would suggest make an enum for protocols.

Or we can reuse URLProtocol from Ktor? But there are more protocols that we probably won't need, such as socks or web sockets

Fixed. Replaced with URLProtocol ktor type.

Rizzen · 2025-05-13T11:54:47Z

...n/ai/grazie/code/agents/local/features/common/remote/client/config/ClientConnectionConfig.kt

@@ -23,7 +23,7 @@ import kotlin.time.Duration.Companion.seconds
 * @property healthCheckUrl A computed property that constructs the URL endpoint for health check requests.
 * @property messageUrl A computed property that constructs the URL endpoint for sending or receiving messages.
 */
-data class ClientConnectionConfig(
+abstract class ClientConnectionConfig(
    val host: String,
    val port: Int,
    val protocol: String = "https",


Same here with protocol enum

EugeneTheDev

I left a few suggestions. Also, I've noticed some hardcoded dependencies that are not in libs.versions.toml. Feel free to disregard these, I just noticed this and wanted to mention it, maybe we can fix it later

EugeneTheDev · 2025-05-13T13:17:48Z

...s/agents-examples/src/test/kotlin/ai/grazie/code/agents/example/calculator/CalculatorTest.kt

+        ) {
+            install(EventHandlerFeature) {
+                onToolCall = { stage, tool, arguments ->
+                    println("[DEBUG_LOG] Tool `${tool.name}` was called with arguments $arguments")


Can you please clean up Junie logs?

This log was originally in the event handler. I moved it to the new implementation. We can change it if needed outside of the current PR if you don't mind.

EugeneTheDev · 2025-05-13T13:21:26Z

...nts-core/src/commonMain/kotlin/ai/grazie/code/agents/core/feature/handler/StrategyHandler.kt

@@ -0,0 +1,41 @@
+package ai.grazie.code.agents.core.feature.handler
+
+class StrategyHandler<FeatureT : Any>(val feature: FeatureT) {


nit: TFeature sounds a bit better and more aligned with standard convetions

Ok, I can rename it later, if needed. I used the same type naming as in other classes we have (AgentHandler).

EugeneTheDev · 2025-05-13T13:24:05Z

...i/grazie/code/agents/core/feature/remote/client/config/AgentFeatureClientConnectionConfig.kt

+class AgentFeatureClientConnectionConfig(
+    host: String,
+    port: Int,
+    protocol: String = "https",


Or we can reuse URLProtocol from Ktor? But there are more protocols that we probably won't need, such as socks or web sockets

EugeneTheDev · 2025-05-13T13:25:55Z

agents/agents-features/agents-features-common/build.gradle.kts

@@ -12,7 +12,6 @@ kotlin {
    sourceSets {
        commonMain {
            dependencies {
-                api(project(":agents:agents-core"))
                api("ai.jetbrains.code.files:code-files-model:1.0.0-beta.55+0.4.45")


Just noticed, shouldn't we move it to libs.versions.toml?

I think yes, but let's do it outside of this PR. This dependency was here already for a while. I agree in general. Good catch! Thank you @EugeneTheDev

...ommon/src/commonMain/kotlin/ai/grazie/code/agents/local/features/common/message/Closeable.kt

agents/agents-features/agents-features-memory/build.gradle.kts

...e/src/commonMain/kotlin/ai/grazie/code/agents/local/features/tracing/feature/TraceFeature.kt

examples/build.gradle.kts

...otlin/ai/grazie/code/agents/local/features/eventHandler/feature/EventHandlerFeatureConfig.kt

sdubov requested review from Ololoshechkin and Rizzen May 12, 2025 12:05

sdubov added the refactoring Code improvements that enhance structure, readability, and quality without changing functionality label May 12, 2025

sdubov self-assigned this May 12, 2025

sdubov force-pushed the sdubov/JBAI-13757 branch from 846dacd to 8d6c469 Compare May 12, 2025 12:35

sdubov requested review from devcrocod and removed request for Ololoshechkin May 12, 2025 12:36

sdubov force-pushed the sdubov/JBAI-13757 branch 3 times, most recently from 5678a01 to bc379d4 Compare May 12, 2025 15:19

sdubov added 2 commits May 12, 2025 20:29

JBAI-13757. Add handlers from EventHandler into AgentPipeline

db74aae

JBAI-13757. Add the event-handler feature to features module

1d783b0

sdubov force-pushed the sdubov/JBAI-13757 branch 4 times, most recently from 58b3a14 to df0f840 Compare May 12, 2025 22:00

sdubov force-pushed the sdubov/JBAI-13757 branch from df0f840 to 329b53a Compare May 12, 2025 22:57

sdubov force-pushed the sdubov/JBAI-13757 branch from 329b53a to bb044e5 Compare May 13, 2025 08:46

tiginamaria mentioned this pull request May 13, 2025

Rename [AI|Local]Agent... to just Agent #36

Closed

sdubov requested a review from EugeneTheDev May 13, 2025 09:29

devcrocod reviewed May 13, 2025

View reviewed changes

Rizzen reviewed May 13, 2025

View reviewed changes

JBAI-13757. Replace protocol parameter with URLProtocol type

cf56e7d

EugeneTheDev reviewed May 13, 2025

View reviewed changes

JBAI-13757. Drop onBeforeToolCalls and onAfterToolCalls handlers

6ebfa09

sdubov force-pushed the sdubov/JBAI-13757 branch from 7659ac6 to 6ebfa09 Compare May 13, 2025 14:18

Rizzen approved these changes May 13, 2025

View reviewed changes

sdubov merged commit 48eb3a1 into main May 13, 2025
3 checks passed

devcrocod reviewed May 13, 2025

View reviewed changes

...otlin/ai/grazie/code/agents/local/features/eventHandler/feature/EventHandlerFeatureConfig.kt Show resolved Hide resolved

sproshev deleted the sdubov/JBAI-13757 branch May 21, 2025 17:17


		//region Trigger Agent Handlers

		var onAgentCreated: suspend (strategy: LocalAgentStrategy, agent: AIAgentBase) -> Unit =

		@@ -0,0 +1,41 @@
		package ai.grazie.code.agents.core.feature.handler

		class StrategyHandler<FeatureT : Any>(val feature: FeatureT) {

JBAI-13757. Replace EventHandler with the event-handler feature #30

JBAI-13757. Replace EventHandler with the event-handler feature #30

Uh oh!

Conversation

sdubov commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EugeneTheDev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!