Inner advisor should handle output format instructions
Output format instructions should not be included until the very last advisor runs, otherwise there's a risk of templating failure if more than one advisor tries to render the prompt template. This change guarantees the output format instructions are always included right before calling the chat model, without the risk of previous advisors interfering with it. Signed-off-by: Thomas Vitale <ThomasVitale@users.noreply.github.com>
This commit is contained in:
committed by
Mark Pollack
parent
f4d4dddf9d
commit
90cab219d2
@@ -21,10 +21,7 @@ package org.springframework.ai.chat.client;
|
||||
*
|
||||
* @author Thomas Vitale
|
||||
* @since 1.0.0
|
||||
* @deprecated only introduced to smooth the transition to the new APIs and ensure
|
||||
* backward compatibility
|
||||
*/
|
||||
@Deprecated
|
||||
public enum ChatClientAttributes {
|
||||
|
||||
//@formatter:off
|
||||
@@ -33,7 +30,6 @@ public enum ChatClientAttributes {
|
||||
ADVISORS("spring.ai.chat.client.advisors"),
|
||||
@Deprecated // Only for backward compatibility until the next release.
|
||||
CHAT_MODEL("spring.ai.chat.client.model"),
|
||||
@Deprecated // Only for backward compatibility until the next release.
|
||||
OUTPUT_FORMAT("spring.ai.chat.client.output.format"),
|
||||
@Deprecated // Only for backward compatibility until the next release.
|
||||
USER_PARAMS("spring.ai.chat.client.user.params"),
|
||||
|
||||
@@ -496,11 +496,13 @@ public class DefaultChatClient implements ChatClient {
|
||||
|
||||
private ChatClientResponse doGetObservableChatClientResponse(ChatClientRequest chatClientRequest,
|
||||
@Nullable String outputFormat) {
|
||||
ChatClientRequest formattedChatClientRequest = StringUtils.hasText(outputFormat)
|
||||
? augmentPromptWithFormatInstructions(chatClientRequest, outputFormat) : chatClientRequest;
|
||||
|
||||
if (outputFormat != null) {
|
||||
chatClientRequest.context().put(ChatClientAttributes.OUTPUT_FORMAT.getKey(), outputFormat);
|
||||
}
|
||||
|
||||
ChatClientObservationContext observationContext = ChatClientObservationContext.builder()
|
||||
.request(formattedChatClientRequest)
|
||||
.request(chatClientRequest)
|
||||
.advisors(advisorChain.getCallAdvisors())
|
||||
.stream(false)
|
||||
.withFormat(outputFormat)
|
||||
@@ -510,7 +512,7 @@ public class DefaultChatClient implements ChatClient {
|
||||
DEFAULT_CHAT_CLIENT_OBSERVATION_CONVENTION, () -> observationContext, observationRegistry);
|
||||
var chatClientResponse = observation.observe(() -> {
|
||||
// Apply the advisor chain that terminates with the ChatModelCallAdvisor.
|
||||
return advisorChain.nextCall(formattedChatClientRequest);
|
||||
return advisorChain.nextCall(chatClientRequest);
|
||||
});
|
||||
return chatClientResponse != null ? chatClientResponse : ChatClientResponse.builder().build();
|
||||
}
|
||||
|
||||
@@ -16,14 +16,17 @@
|
||||
|
||||
package org.springframework.ai.chat.client.advisor;
|
||||
|
||||
import org.springframework.ai.chat.client.ChatClientAttributes;
|
||||
import org.springframework.ai.chat.client.ChatClientRequest;
|
||||
import org.springframework.ai.chat.client.ChatClientResponse;
|
||||
import org.springframework.ai.chat.client.advisor.api.CallAdvisor;
|
||||
import org.springframework.ai.chat.client.advisor.api.CallAroundAdvisorChain;
|
||||
import org.springframework.ai.chat.model.ChatModel;
|
||||
import org.springframework.ai.chat.model.ChatResponse;
|
||||
import org.springframework.ai.chat.prompt.Prompt;
|
||||
import org.springframework.core.Ordered;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
import java.util.Map;
|
||||
|
||||
@@ -46,9 +49,29 @@ public final class ChatModelCallAdvisor implements CallAdvisor {
|
||||
public ChatClientResponse adviseCall(ChatClientRequest chatClientRequest, CallAroundAdvisorChain chain) {
|
||||
Assert.notNull(chatClientRequest, "the chatClientRequest cannot be null");
|
||||
|
||||
ChatResponse chatResponse = chatModel.call(chatClientRequest.prompt());
|
||||
ChatClientRequest formattedChatClientRequest = augmentWithFormatInstructions(chatClientRequest);
|
||||
|
||||
ChatResponse chatResponse = chatModel.call(formattedChatClientRequest.prompt());
|
||||
return ChatClientResponse.builder()
|
||||
.chatResponse(chatResponse)
|
||||
.context(Map.copyOf(formattedChatClientRequest.context()))
|
||||
.build();
|
||||
}
|
||||
|
||||
private static ChatClientRequest augmentWithFormatInstructions(ChatClientRequest chatClientRequest) {
|
||||
String outputFormat = (String) chatClientRequest.context().get(ChatClientAttributes.OUTPUT_FORMAT.getKey());
|
||||
|
||||
if (!StringUtils.hasText(outputFormat)) {
|
||||
return chatClientRequest;
|
||||
}
|
||||
|
||||
Prompt augmentedPrompt = chatClientRequest.prompt()
|
||||
.augmentUserMessage(userMessage -> userMessage.mutate()
|
||||
.text(userMessage.getText() + System.lineSeparator() + outputFormat)
|
||||
.build());
|
||||
|
||||
return ChatClientRequest.builder()
|
||||
.prompt(augmentedPrompt)
|
||||
.context(Map.copyOf(chatClientRequest.context()))
|
||||
.build();
|
||||
}
|
||||
|
||||
@@ -163,6 +163,28 @@ public class QuestionAnswerAdvisorIT {
|
||||
evaluateRelevancy(question, chatResponse);
|
||||
}
|
||||
|
||||
@Test
|
||||
void qaOutputConverter() {
|
||||
String question = "Where does the adventure of Anacletus and Birba take place?";
|
||||
|
||||
QuestionAnswerAdvisor qaAdvisor = QuestionAnswerAdvisor.builder(this.pgVectorStore).build();
|
||||
|
||||
Answer answer = ChatClient.builder(this.openAiChatModel)
|
||||
.build()
|
||||
.prompt(question)
|
||||
.advisors(qaAdvisor)
|
||||
.call()
|
||||
.entity(Answer.class);
|
||||
|
||||
assertThat(answer).isNotNull();
|
||||
|
||||
System.out.println(answer);
|
||||
assertThat(answer.content()).containsIgnoringCase("Highlands");
|
||||
}
|
||||
|
||||
private record Answer(String content) {
|
||||
}
|
||||
|
||||
private void evaluateRelevancy(String question, ChatResponse chatResponse) {
|
||||
EvaluationRequest evaluationRequest = new EvaluationRequest(question,
|
||||
chatResponse.getMetadata().get(QuestionAnswerAdvisor.RETRIEVED_DOCUMENTS),
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
/*
|
||||
* Copyright 2023-2025 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.ai.integration.tests.client.advisor;
|
||||
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
|
||||
import org.springframework.ai.chat.client.ChatClient;
|
||||
import org.springframework.ai.chat.client.advisor.vectorstore.QuestionAnswerAdvisor;
|
||||
import org.springframework.ai.chat.model.ChatResponse;
|
||||
import org.springframework.ai.converter.BeanOutputConverter;
|
||||
import org.springframework.ai.document.Document;
|
||||
import org.springframework.ai.document.DocumentReader;
|
||||
import org.springframework.ai.integration.tests.TestApplication;
|
||||
import org.springframework.ai.openai.OpenAiChatModel;
|
||||
import org.springframework.ai.openai.OpenAiChatOptions;
|
||||
import org.springframework.ai.reader.markdown.MarkdownDocumentReader;
|
||||
import org.springframework.ai.reader.markdown.config.MarkdownDocumentReaderConfig;
|
||||
import org.springframework.ai.vectorstore.pgvector.PgVectorStore;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.core.io.Resource;
|
||||
import reactor.core.publisher.Flux;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Integration tests for {@link QuestionAnswerAdvisor} with streaming responses.
|
||||
*
|
||||
*/
|
||||
@SpringBootTest(classes = TestApplication.class)
|
||||
@EnabledIfEnvironmentVariable(named = "OPENAI_API_KEY", matches = ".*")
|
||||
public class QuestionAnswerAdvisorStreamIT {
|
||||
|
||||
private List<Document> knowledgeBaseDocuments;
|
||||
|
||||
@Autowired
|
||||
OpenAiChatModel openAiChatModel;
|
||||
|
||||
@Autowired
|
||||
PgVectorStore pgVectorStore;
|
||||
|
||||
@Value("${classpath:documents/knowledge-base.md}")
|
||||
Resource knowledgeBaseResource;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
DocumentReader markdownReader = new MarkdownDocumentReader(this.knowledgeBaseResource,
|
||||
MarkdownDocumentReaderConfig.defaultConfig());
|
||||
this.knowledgeBaseDocuments = markdownReader.read();
|
||||
this.pgVectorStore.add(this.knowledgeBaseDocuments);
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
void tearDown() {
|
||||
this.pgVectorStore.delete(this.knowledgeBaseDocuments.stream().map(Document::getId).toList());
|
||||
}
|
||||
|
||||
@Test
|
||||
void qaStreamBasic() {
|
||||
String question = "Where does the adventure of Anacletus and Birba take place?";
|
||||
|
||||
QuestionAnswerAdvisor qaAdvisor = QuestionAnswerAdvisor.builder(this.pgVectorStore).build();
|
||||
|
||||
// Test streaming with the QuestionAnswerAdvisor
|
||||
// This verifies the fix works in the streaming context too
|
||||
Flux<String> responseFlux = ChatClient.builder(this.openAiChatModel)
|
||||
.build()
|
||||
.prompt(question)
|
||||
.advisors(qaAdvisor)
|
||||
.options(OpenAiChatOptions.builder().streamUsage(true).build())
|
||||
.stream()
|
||||
.content();
|
||||
|
||||
// Collect the streamed responses
|
||||
String response = responseFlux.collectList().block().stream().collect(Collectors.joining());
|
||||
|
||||
// Verify the response contains the expected content
|
||||
assertThat(response).isNotEmpty();
|
||||
assertThat(response).containsIgnoringCase("Highlands");
|
||||
}
|
||||
|
||||
private record Answer(String content) {
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user