Kotlin AI Agent simplification

[Rizzen]

No description provided.

[sdubov]

I would keep Base an abstract class with basic implementation and add some concrete default implementation, e.g. SimpleAiAgent as a class.

[Rizzen]

The only reason we needed this base class is that we had IdeFormer implementation. Currently AIAgentBase contains all the necessary stuff and it's open, why create two classes instead?

[sdubov]

I'm not sure I understand the reason to keep AiAgentBase class open in that case. I would assume it is final.

[sdubov]

It can be private.

[sdubov]

Maybe, we can merge AgentEnvironment fully into AIAgent interface? Wdyt?

Sorry, but I still do not understand why do we extract this set of methods into a separate interface "agent environment":

executeTools(toolCalls: List<Message.Tool.Call>): List<ReceivedToolResult>
reportProblem(exception: Throwable)
sendTermination(result: String?)

• executeTools can be a part of AgentNode
• reportProblem, sendTermination are a part of AgentStrategy (based on usages). Or... in event handler + agent

[Rizzen]

AgentEnvironment is "environment outside of agent", not the agent itself.

Existense of AIAgent interface was under heavy discussion but in the end we decided to leave that interface to make room for users to extend the library. I'm still not completely sure if we need AIAgent interface.

executeTools can be a part of AgentNode
reportProblem, sendTermination are a part of AgentStrategy (based on usages). Or... in event handler + agent

Absolutely not, AgentNode and AgentStrategy are graph-level entities and should not handle such things.

[sdubov]

AiAgentBase implements the executeTool method (due to inheriting from AgentEnvironment interface). However, in fact, the implementation of the method is just - getting a tool name from a message, fetching this tool from a ToolRegistry and calling execute method. So, based on this, I would say that tool calling ability is seems for me a part of an agent logic. Maybe?

sendTerminatin is kinda an event about agent finish it's work. It is just used to notify that agent is done. Our implementation inside AiAgentBase is not necessary at all, because we create termination message by ourselves, than in a private terminate method we check this message, verify it in a different ways and complete a private completable deferred object that we use to await for an agent finish. We can move this into a event handler (pipeline in future) if needed.

reportProblem is the same for me. Can be converted into an "agent problem listener". In that case agent can send events about termination/problems and everyone who want to know this ("some environment") can listen for these events and react.

Just in case, we can change it later as a separate PR, for sure (as an option if you like proposals)

Wdyt?

[Rizzen]

AiAgentBase implements the executeTool method (due to inheriting from AgentEnvironment interface). However, in fact, the implementation of the method is just - getting a tool name from a message, fetching this tool from a ToolRegistry and calling execute method. So, based on this, I would say that tool calling ability is seems for me a part of an agent logic. Maybe?

sendTerminatin is kinda an event about agent finish it's work. It is just used to notify that agent is done. Our implementation inside AiAgentBase is not necessary at all, because we create termination message by ourselves, than in a private terminate method we check this message, verify it in a different ways and complete a private completable deferred object that we use to await for an agent finish. We can move this into a event handler (pipeline in future) if needed.

reportProblem is the same for me. Can be converted into an "agent problem listener". In that case agent can send events about termination/problems and everyone who want to know this ("some environment") can listen for these events and react.

All of these things are management of agent's lifecycle and providing interactions with environment, which is the responsibility of AiAgent class. We don't want to spread it all over codebase with listeners etc.
To be honest, I don't understand what problem we're trying to solve with such moves.

We can discuss it on tech sync later.

[sdubov]

To be honest, I don't understand what problem we're trying to solve with such moves.

The main goal is to make a simple API. We have number of abstractions in our framework that seems unnecessary and makes understanding more complicated from my view. AgentEnvironment is one of the things that I feel is redundant here. It does not really interact with anything outside of an agent. It is just an internal implementation because all what it do:

1. Search for a tools in the internal (!) ToolRegistry and call execute for a selected tool (I do not see any external interactions here)
2. Set a private (!) flag about termination to wait for an agent to finish (no external interactions)
3. Throw an exception in case of agent execution error (no external interactions)

I don't feel that we need to create a separate abstractions for these three points above. Sure, let's discuss this out of the scope of this ticket on a tech sync or in the channel 🤝

[sdubov]

[To discuss] Maybe we should use AIAgent interface here. It might be we can add general agent properties into interface, wdyt?:

interface AIAgent {
    val name: String
    val config: LocalAgentConfig
    val promptExecutor: PromptExecutor
    val toolRegistry: ToolRegistry
    ...
}

[Rizzen]

Kinda make sense, not sure if we want to make AIAgent dependent on PromptExecutor

[sdubov]

Based on Kotlin doc, they ask to keep parameters with default values at the end to keep compatibility with Java that does not have named parameters.

[sdubov]

Actually, parameters can be in any order in Kotlin files if you explicitly specify names for them.

[sdubov]

Thank you for fixing this. I approve to merge this to not block further changes and leave some comments to discuss. We can fix them later in further commits if needed.