Skip to content

Commit a9a33ad

Browse files
Fix: Gracefully handle error when generative_qa_parameters is not provided (opensearch-project#3100) (opensearch-project#3111)
* fix: gracefully handle error when generative_qa_parameters is not provided Signed-off-by: Pavan Yekbote <mail2pavanyekbote@gmail.com> * fix: spotless apply Signed-off-by: Pavan Yekbote <mail2pavanyekbote@gmail.com> * docs: adding documentation link to error message Signed-off-by: Pavan Yekbote <mail2pavanyekbote@gmail.com> * tests: adding UT to test null params Signed-off-by: Pavan Yekbote <mail2pavanyekbote@gmail.com> --------- Signed-off-by: Pavan Yekbote <mail2pavanyekbote@gmail.com> (cherry picked from commit 0f7481e) Co-authored-by: Pavan Yekbote <mail2pavanyekbote@gmail.com>
1 parent 06c0c24 commit a9a33ad

File tree

3 files changed

+80
-0
lines changed

3 files changed

+80
-0
lines changed

search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java

+4
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,8 @@ public class GenerativeQAProcessorConstants {
4343
.boolSetting("plugins.ml_commons.rag_pipeline_feature_enabled", true, Setting.Property.NodeScope, Setting.Property.Dynamic);
4444

4545
public static final String FEATURE_NOT_ENABLED_ERROR_MSG = RAG_PIPELINE_FEATURE_ENABLED.getKey() + " is not enabled.";
46+
47+
public static final String RAG_NULL_GEN_QA_PARAMS_ERROR_MSG = "generative_qa_parameters not found."
48+
+ " Please provide ext.generative_qa_parameters to proceed."
49+
+ " For more info, refer: https://opensearch.org/docs/latest/search-plugins/conversational-search/#step-6-use-the-pipeline-for-rag";
4650
}

search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java

+4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.opensearch.searchpipelines.questionanswering.generative;
1919

2020
import static org.opensearch.ingest.ConfigurationUtils.newConfigurationException;
21+
import static org.opensearch.searchpipelines.questionanswering.generative.GenerativeQAProcessorConstants.RAG_NULL_GEN_QA_PARAMS_ERROR_MSG;
2122

2223
import java.time.Duration;
2324
import java.time.Instant;
@@ -126,6 +127,9 @@ public void processResponseAsync(
126127
}
127128

128129
GenerativeQAParameters params = GenerativeQAParamUtil.getGenerativeQAParameters(request);
130+
if (params == null) {
131+
throw new IllegalArgumentException(RAG_NULL_GEN_QA_PARAMS_ERROR_MSG);
132+
}
129133

130134
Integer t = params.getTimeout();
131135
if (t == null || t == GenerativeQAParameters.SIZE_NULL_VALUE) {

search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessorTests.java

+72
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.mockito.Mockito.mock;
2525
import static org.mockito.Mockito.verify;
2626
import static org.mockito.Mockito.when;
27+
import static org.opensearch.searchpipelines.questionanswering.generative.GenerativeQAProcessorConstants.RAG_NULL_GEN_QA_PARAMS_ERROR_MSG;
2728

2829
import java.time.Instant;
2930
import java.util.Collections;
@@ -646,6 +647,77 @@ public void testProcessResponseNullValueInteractions() throws Exception {
646647
}));
647648
}
648649

650+
public void testProcessResponseIllegalArgumentForNullParams() throws Exception {
651+
exceptionRule.expect(IllegalArgumentException.class);
652+
exceptionRule.expectMessage(RAG_NULL_GEN_QA_PARAMS_ERROR_MSG);
653+
654+
Client client = mock(Client.class);
655+
Map<String, Object> config = new HashMap<>();
656+
config.put(GenerativeQAProcessorConstants.CONFIG_NAME_MODEL_ID, "dummy-model");
657+
config.put(GenerativeQAProcessorConstants.CONFIG_NAME_CONTEXT_FIELD_LIST, List.of("text"));
658+
659+
GenerativeQAResponseProcessor processor = (GenerativeQAResponseProcessor) new GenerativeQAResponseProcessor.Factory(
660+
client,
661+
alwaysOn
662+
).create(null, "tag", "desc", true, config, null);
663+
664+
ConversationalMemoryClient memoryClient = mock(ConversationalMemoryClient.class);
665+
List<Interaction> chatHistory = List
666+
.of(
667+
new Interaction(
668+
"0",
669+
Instant.now(),
670+
"1",
671+
"question",
672+
"",
673+
"answer",
674+
"foo",
675+
Collections.singletonMap("meta data", "some meta")
676+
)
677+
);
678+
doAnswer(invocation -> {
679+
((ActionListener<List<Interaction>>) invocation.getArguments()[2]).onResponse(chatHistory);
680+
return null;
681+
}).when(memoryClient).getInteractions(any(), anyInt(), any());
682+
processor.setMemoryClient(memoryClient);
683+
684+
SearchRequest request = new SearchRequest();
685+
SearchSourceBuilder sourceBuilder = new SearchSourceBuilder();
686+
GenerativeQAParamExtBuilder extBuilder = new GenerativeQAParamExtBuilder();
687+
extBuilder.setParams(null);
688+
request.source(sourceBuilder);
689+
sourceBuilder.ext(List.of(extBuilder));
690+
691+
int numHits = 10;
692+
SearchHit[] hitsArray = new SearchHit[numHits];
693+
for (int i = 0; i < numHits; i++) {
694+
XContentBuilder sourceContent = JsonXContent
695+
.contentBuilder()
696+
.startObject()
697+
.field("_id", String.valueOf(i))
698+
.field("text", "passage" + i)
699+
.field("title", "This is the title for document " + i)
700+
.endObject();
701+
hitsArray[i] = new SearchHit(i, "doc" + i, Map.of(), Map.of());
702+
hitsArray[i].sourceRef(BytesReference.bytes(sourceContent));
703+
}
704+
705+
SearchHits searchHits = new SearchHits(hitsArray, null, 1.0f);
706+
SearchResponseSections internal = new SearchResponseSections(searchHits, null, null, false, false, null, 0);
707+
SearchResponse response = new SearchResponse(internal, null, 1, 1, 0, 1, null, null, null);
708+
709+
Llm llm = mock(Llm.class);
710+
processor.setLlm(llm);
711+
712+
processor
713+
.processResponseAsync(
714+
request,
715+
response,
716+
null,
717+
ActionListener.wrap(r -> { assertTrue(r instanceof GenerativeSearchResponse); }, e -> {})
718+
);
719+
}
720+
649721
public void testProcessResponseIllegalArgument() throws Exception {
650722
exceptionRule.expect(IllegalArgumentException.class);
651723
exceptionRule.expectMessage("llm_model cannot be null.");

0 commit comments

Comments
 (0)