DefaultChatClientClass is assuming that the last message in the messages array is of type "USER". If the last message is not of type USER, userText is not being set and the subsequent null check will fail, although a USER message has been provided:
Pseudo-Code that causes the problem:
chatClient
.prompt(
new Prompt(
List.of(
createUserMessage(request),
createSystemMessage()
)))
.call()
.chatResponse();
will result in :
java.lang.IllegalArgumentException: userText cannot be null or empty
at org.springframework.util.Assert.hasText(Assert.java:240) ~[spring-core-6.1.10.jar:6.1.10]
at org.springframework.ai.chat.client.advisor.api.AdvisedRequest.
Offending lines from DefaultChatClient, method toAdvisedRequest ll 109ff
List<Message> messages = inputRequest.messages;
if (!StringUtils.hasText(userText) && !CollectionUtils.isEmpty(messages)) {
Message lastMessage = (Message)messages.get(messages.size() - 1);
if (lastMessage.getMessageType() == MessageType.USER) {
UserMessage userMessage = (UserMessage)lastMessage;
if (StringUtils.hasText(userMessage.getContent())) {
userText = lastMessage.getContent();
}
Please make no assumptions about the order of messages an external caller to your API may provide.
Comment From: ThomasVitale
@github2atauhiq thanks for reporting this. Could you elaborate a bit more on this specific use case where there is no user message as the last one in a list of messages sent to an LLM?
ChatClient requires each AI workflow to have a user message well identified. Either directly (via .user()) or as part of a list of messages (via .prompt() or .messages()). In the latter case, ChatClient would try to find the user message as the last element of the list of messages (which is also what an LLM provider would do in the presence of multiple messages). If neither places contain a user message, the ChatClient cannot guarantee the integrity and consistency of its behaviour, so it's important to get the exception thrown. By design, a user message needs to be provided. Or else, all the augmentations that it could provide like memory, RAG or guardrails would not be predictable nor robust enough.
Besides ensuring a clear and consistent behaviour for ChatClient, this requirement also helps preventing mistakes when interacting with a model. LLMs expect to get a user message. If there's a list of messages, LLMs look for the user message to answer to as the last one in the list. If that's not true, they would still answer something, but the outcome is even more unpredictable than it usually is, because the order of messages matters and therefore changes the LLM behaviour, sometimes in a very significant way. Some models would not process the misplaced user message at all and answer completely unexpected things.
Comment From: github2atauhiq
Hello Thomas,
thank you for your feedback. I wasn't aware of any particular order that the message must to be supplied in. There is no hint in the documentation that specifies that the prompts have to be ordered in a certain way. There weren't any issues so far with providing the user-message as the first message in the list. Since spring-ai is meant to be an abstraction, why do I have to care about the order of the messages that the underlying LLM is expecting anyways? I also find it odd, that i am able to construct an instance of Prompt although the order is incorrect. I would have expected the API to fail immediately and throw a more specific exception.
Of course, it is no problem for us to change the order (we are already doing that), but the behavior of spring-ai regarding this topic clearly changed between last week and this week and that was reason enough for me to indicate this to you.
Best regards
Manuel
Comment From: ThomasVitale
Thanks a lot for providing additional context. Before, this same behaviour was happening, but in case no user message was found, one was created with empty content, which was leading to unpredictable outcomes (especially when using Advisors). Now, there is a check to ensure a proper user message is identified, so to ensure consistency and no surprises in the resulting behaviour. You can blame me for this change :) I'm also working on some additional documentation and release notes before this change is released in the upcoming milestone.
The underlying ChatModel abstraction doesn't assume anything, so you can build the messages any way you want. The ChatClient is a higher-level API that brings some boundaries to be able to build such AI workflows in a robust way. For example, when using RAG via an Advisor, it will not work as expected if the user message is not identified correctly.
I wonder if there could be a way to make this more flexible, while maintaining the null-safety and predictability of the API. For example, would it work having ChatClient accept a lambda to use for customising the "user message identification" strategy? What do you think?
Comment From: DHKIM-0511
@ThomasVitale What do you think about applying the builder pattern to the Prompt class? It seems that developers would find it easier to understand the message insertion order by looking at the API. Also, if we add the UserMessage to the list during the build() process, wouldn't it be possible to handle the order correctly internally?
Comment From: DHKIM-0511
Sorry for the mistake, this issue was mentioned by accident.
Comment From: markpollack
Some good ideas here, going to close an concentrate the discussion in #873