Make ChatClient and Advisor APIs more robust, consistent, and flexible

[ThomasVitale]

Advisor API

The Advisor API went through some changes over time. Initially, there was a concept of "advisor type", with separate interfaces for "before", "after", and "around" advisors. Currently, there is only one type of advisor (around). Still, the naming of many APIs and the observation context include the legacy "around" concept. We should probably get rid of that to avoid confusion.

AdvisedRequest

The AdvisedRequest API contains the same information of a Prompt, but in a de-structured format. Furthermore, for the same information, multiple ways exist to store it. This causes lots of troubles in all consumers of an AdvisedRequest object (e.g. advisors and observations), in particular:

• A consumer doesn't know if some of the "de-structured" information has already been used to reach a more structured format. For example, there is no way to tell if the userParams have already been used to render a version of the userText. In that case, the workaround is for each consumer to render the userText, even if not needed. Some issues registered about this: Parameters not saved in ChatMemory when using PromptUserSpec #2618
• A consumer must be aware that prompt messages can be found in multiple places and must be aware of the rules for combining the information into a unified view. For example, the user prompt or system message might be found in one of the dedicated fields (userText, systemText) or in the more generic messages. In that case, the consumer must come up with a strategy to infer the actual user message and the actual system message, since there can be multiple ones. Furthermore, metadata for user and system messages are lost if passed via messages, because they are then converted internally to userText and systemText. Some issues registered about this: MessageChatMemoryAdvisor results in an error if prompt is initialized with List<Message> instead of userText #2339, Metadata Loss in AdvisedRequest.toPrompt() (Spring AI v1.0.0-M6) #2355, AdvisedRequest.userText is not populated under some ChatClient Configurations. #2408, Metadata Loss in DefaultChatClient.DefaultStreamResponseSpec When Converting UserMessage to AdvisedRequest #2612, spring.ai.chat.client.*.text span tags are not captured when using messages() #2631, MessageChatMemoryAdvisor and PromptChatMemoryAdvisor lack a way to pass custom Message metadata #2437, Relax text validation to support empty strings in multimodal scenarios #2284, UserMessage lost userParams after being processed by MessageChatMemoryAdvisor #2701
• A consumer must be aware that tools can be found in multiple places and must be aware of the rules for combining the information into a unified view. For example, tool names might be found in the dedicated field (toolNames) to in the more generic chatOptions. When both places contain toolNames, how should possible conflicts be handled? In some cases, the tools passed via chatOptions are completely overwritten by the ones passed via the dedicated APIs. Some issues registered about this: What's the difference between passing tools to the Prompt vs the ChatClient? #2530
• When both a systemText and a SystemMessage in messages are provided, it's not clear which one is used in the call to the chat model. Some issues registered about this: [ChatClient] Inconsistent handling of system messages #873, The Default System Prompt is not the first element in the messages array #2216, MistralAiChatModel returns an error if MessageChatMemoryAdvisor is used #2380

Furthermore, an AdvisedRequest contains a mutable ChatModel instance which is very confusing. Consumers of an AdvisedRequest have the power to replace the ChatModel instance, but that operation would have no effect. Actually, it could lead to wrong information provided downstream. If a subsequent consumer relies on that field to know which model is used by the chain of advisors, it could receive wrong information. It should probably be removed.

Similarly, an AdvisedRequest contains a mutable List<Advisor> which includes the list of advisors in the chain. Consumers of an AdvisedRequest have the power to replace the manipulate the List<Advisor> instance, but that operation would have no effect. Actually, it could lead to wrong information provided downstream. If a subsequent consumer relies on that field to know which advisors are being executed in the chain, it could receive wrong information. It should probably be removed.

Finally, an AdvisedRequest contains both advisorParams and adviseContext fields. However, the first one is only used at the beginning of an advisor chain to populate the second field, and never used anymore. It should probably be removed to avoid confusion and unpredictable behaviour.

AdvisedResponse

The AdvisedResponse API carries both an optional ChatResponse object returned from a chat model call and the context used throughout the chat client execution, including the advisor chain. This context includes useful information. For example, when performing a RAG operation, it contains the contextual documents retrieved by the vector store and used by the model for answering the user question. This context is currently hidden within the advisor chain, causing issues to users that would like to use that contextual information for several reasons, including for evaluation and validation. There have been some suggestions for expanding the ChatResponse so to include this extra context, but that solution doesn't really scale and dirties the lower-level Chat Model API. Instead, a better solution would be to return the context to the caller in a clean way.

Some issues registered about this: #1747

Observation

The observation context for the ChatClient operations is affected by the problems mentioned above regarding the AdvisedRequest. In particular, the information included in the observation context might be wrong or incomplete. For example, user messages, system messages, and tools are likely to be incomplete since we only consider one of the possible locations in the AdvisedRequest where they could be stored.

Also, the ChatClientObservationContext is strictly dependent on the DefaultChatClientRequestSpec, meaning that it only works with the default implementation of the ChatClient API.

Prompt Templating

The ChatClient API supports passing userParams and systemParams, which can be used to render the templates passed as userText and systemText. At a minimum, the templates are rendered right before calling the chat model, using a default PromptTemplate object. When using advisors, the rendering might be needed earlier. For example, it had to be included in QuestionAnswerAdvisor and RetrievalAugmentationAdvisor. That's part of the issues mentioned earlier in the context of AdvisedRequest.

The templating strategy is based on StringTemplate and cannot be changed. This creates problems in two main cases:

• When a user prompt doesn't contain templating, but includes curly braces, the call will fail.
• When a user prompt needs a different templating strategy, it's not possible.

The ChatClient API should provide the possibility to customise the prompt template rendering strategy.

Some issues registered about this: #355, #1687, #2448, #2456, #1849, #1428, #2468, #2020

[SpiReCZ]

The first issue with the current/old Advisor API I encountered was unavailable context information for the user (RAG document names), needed custom or extra advisor.
Then for chat UI it also didn't allow to dispatch events to the resulting stream before LLM inference.
Example:
SpiReCZ/spring-ai@main...SpiReCZ:spring-ai:ahead-stream-events

Users need more control and less encapsulation with advisors.

[SpiReCZ]

@ThomasVitale @tzolov What do you guys think? Can we achieve full control over Flux<ChatResponse> in 1.0.0 ?
The idea in the ahead stream events is to allow Advisors to publish events ahead of time before LLM inference..

Use case for this could be giving UI/frontend hints about what AI chatbot/agent is doing..
example UI hints: thinking, searching...

Then another use case is to sent out filenames of documents from retrieved RAG chunks.
If I could choose, I would publish all events/metadata ahead for LLM inference if available.