spring-ai icon indicating copy to clipboard operation
spring-ai copied to clipboard

Published 20 hours ago • verified publisher icon spring-projects

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

Open ThomasVitale opened this issue 7 months ago • 3 comments
trafficstars

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: https://github.com/spring-projects/spring-ai/issues/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: https://github.com/spring-projects/spring-ai/issues/2339, https://github.com/spring-projects/spring-ai/issues/2355, https://github.com/spring-projects/spring-ai/issues/2408, https://github.com/spring-projects/spring-ai/issues/2612, https://github.com/spring-projects/spring-ai/issues/2631, https://github.com/spring-projects/spring-ai/issues/2437, https://github.com/spring-projects/spring-ai/discussions/2284, https://github.com/spring-projects/spring-ai/issues/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: https://github.com/spring-projects/spring-ai/discussions/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: https://github.com/spring-projects/spring-ai/issues/873, https://github.com/spring-projects/spring-ai/issues/2216, https://github.com/spring-projects/spring-ai/issues/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: https://github.com/spring-projects/spring-ai/issues/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: https://github.com/spring-projects/spring-ai/issues/355, https://github.com/spring-projects/spring-ai/issues/1687, https://github.com/spring-projects/spring-ai/issues/2448, https://github.com/spring-projects/spring-ai/issues/2456, https://github.com/spring-projects/spring-ai/issues/1849, https://github.com/spring-projects/spring-ai/issues/1428, https://github.com/spring-projects/spring-ai/pull/2468, https://github.com/spring-projects/spring-ai/pull/2020

ThomasVitale avatar Apr 06 '25 19:04 ThomasVitale