Skip to content
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

Rework and improve Agent and ChatModel API #14818

Closed
sdirix opened this issue Feb 3, 2025 · 0 comments · Fixed by #14859
Closed

Rework and improve Agent and ChatModel API #14818

sdirix opened this issue Feb 3, 2025 · 0 comments · Fixed by #14859
Labels

Comments

@sdirix
Copy link
Member

sdirix commented Feb 3, 2025

The agent inheritance tree is quite large already. It must be checked whether it can be simplified, including using composition patterns over inheritance.


Before declaring the API somewhat final, we should have another look at the interfaces and abstract classes in ai-chat and ai-core.There may be more but here is what I felt is worth improving so far:

The AbstractChatAgent and its sub-classes are very important classes for custom chat agent implementations. However, it is a bit clunky and unclear how to extend it.

For instance, if the agent wants to run preparatory LLM requests (e.g. user intent detection, etc.), before finally streaming a response to the chat, it needs to weirdly overwrite addContentsToResponse or similar. But then it is clunky to reuse the quite dense code of AbstractStreamParsingChatAgent.addContentsToResponse. Much of that code could actually just be a function, such as a versatile AbstractStreamParsingChatAgent.addContentsToResponse() and AbstractStreamParsingChatAgent.parse().

Also there is currently a clear assumption that there is exactly one system message in the AbstractChatAgent, which may not be the case and then implementations need to return an empty string or avoid using certain methods of the super class. We should check AbstractChatAgent.invoke() and rethink which code we want to reuse and how to design this class.

In general declaring and obtaining prompt templates would be nicer if the agent declares a map of named entries and then have a method to obtain again by name filled with a certain object. This code is repeated currently across agents.


Chat Agents

  • The AbstractChatAgent should implement ChatAgent, otherwise all subclasses would have to do it explicitly
  • The whole initialization of the plethora of fields a chat agent "must" implement is very tedious and feels random, sometimes I have to override (e.g. id), because the parent happens to already specify this field, sometimes I have to just implement (e.g. name), and I always have to initialize with empty arrays the "optional" fields (e.g. variables, agentSpecificVariables, functions).
  • The constructor-based approach of creating chat agents seems very Java-like and doesn't really feel idiomatic. Unless I need to run logic, I'd always prefer just specifying the field values in the properties.
  • To just specify the system message I always have to copy over two pretty long lines of code, instead of just specifying the prompt template id.
  • It feels a bit out of place that I (1) have to specify the "defaultLanguageModelPurpose` and (2) add a languageModelRequirement matching this purpose. If you don't look at the code of the superclass there is hardly a way to get this right.

Chat Model

The chat model implementation classes are somewhat cumbersome as they often unnecessarily rely on private fields which are just as is exposed via getters and setters and specified via lengthy constructor arguments, where often several of them are optional.
I would find it more concise and easier to maintain if

  • we use wherever possible just public fields and only if needed introduce own getters and setters
  • use objects for initialization that follow an interface, this is more convenient especially with optional fields

It would be nice to have an explicit "fire update notification", because some of the getters are notifying an update (causing an update on the UI) and others don't. But there is then no way to trigger an update of the UI.

@sdirix sdirix changed the title The agent inheritance tree is quite large already. It must be checked whether it can be simplified, including using composition patterns over inheritance. Rework and improve Agent API Feb 3, 2025
@sdirix sdirix changed the title Rework and improve Agent API Rework and improve Agent and ChatModel API Feb 3, 2025
@sdirix sdirix added the TheiaAI label Feb 4, 2025
eneufeld added a commit to eclipsesource/theia that referenced this issue Feb 6, 2025
eneufeld added a commit to eclipsesource/theia that referenced this issue Feb 6, 2025
eneufeld added a commit to eclipsesource/theia that referenced this issue Feb 6, 2025
eneufeld added a commit to eclipsesource/theia that referenced this issue Feb 12, 2025
sdirix pushed a commit that referenced this issue Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant