From fa8daafbffde708fba8768264dac414d3fb58c62 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Fri, 11 Oct 2024 15:38:19 -0700 Subject: [PATCH 1/4] fix: gracefully handle error when generative_qa_parameters is not provided Signed-off-by: Pavan Yekbote --- .../generative/GenerativeQAResponseProcessor.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java index 6e8a5544b3..f63ab3c722 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java @@ -126,6 +126,9 @@ public void processResponseAsync( } GenerativeQAParameters params = GenerativeQAParamUtil.getGenerativeQAParameters(request); + if (params == null) { + throw new IllegalArgumentException("generative_qa_parameters not found. Please provide ext.generative_qa_parameters to proceed."); + } Integer t = params.getTimeout(); if (t == null || t == GenerativeQAParameters.SIZE_NULL_VALUE) { From 400e16a02d74562f04601a60db64fe79dddf8cbd Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Fri, 11 Oct 2024 15:41:30 -0700 Subject: [PATCH 2/4] fix: spotless apply Signed-off-by: Pavan Yekbote --- .../generative/GenerativeQAResponseProcessor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java index f63ab3c722..37bd1db0e1 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java @@ -127,7 +127,9 @@ public void processResponseAsync( GenerativeQAParameters params = GenerativeQAParamUtil.getGenerativeQAParameters(request); if (params == null) { - throw new IllegalArgumentException("generative_qa_parameters not found. Please provide ext.generative_qa_parameters to proceed."); + throw new IllegalArgumentException( + "generative_qa_parameters not found. Please provide ext.generative_qa_parameters to proceed." + ); } Integer t = params.getTimeout(); From 290f89dd78da3deef3a6fc3703408903ca247102 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Mon, 14 Oct 2024 12:43:55 -0700 Subject: [PATCH 3/4] docs: adding documentation link to error message Signed-off-by: Pavan Yekbote --- .../generative/GenerativeQAResponseProcessor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java index 37bd1db0e1..203e99565a 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java @@ -129,6 +129,7 @@ public void processResponseAsync( if (params == null) { throw new IllegalArgumentException( "generative_qa_parameters not found. Please provide ext.generative_qa_parameters to proceed." + + "For more info, refer: https://opensearch.org/docs/latest/search-plugins/conversational-search/#step-6-use-the-pipeline-for-rag" ); } From a818939cf5d4fa71ec601e5cd543dda8577dc0ad Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Mon, 14 Oct 2024 14:16:17 -0700 Subject: [PATCH 4/4] tests: adding UT to test null params Signed-off-by: Pavan Yekbote --- .../GenerativeQAProcessorConstants.java | 4 ++ .../GenerativeQAResponseProcessor.java | 6 +- .../GenerativeQAResponseProcessorTests.java | 72 +++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java index ff71cadba2..7b3cf07db8 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java @@ -43,4 +43,8 @@ public class GenerativeQAProcessorConstants { .boolSetting("plugins.ml_commons.rag_pipeline_feature_enabled", true, Setting.Property.NodeScope, Setting.Property.Dynamic); public static final String FEATURE_NOT_ENABLED_ERROR_MSG = RAG_PIPELINE_FEATURE_ENABLED.getKey() + " is not enabled."; + + public static final String RAG_NULL_GEN_QA_PARAMS_ERROR_MSG = "generative_qa_parameters not found." + + " Please provide ext.generative_qa_parameters to proceed." + + " For more info, refer: https://opensearch.org/docs/latest/search-plugins/conversational-search/#step-6-use-the-pipeline-for-rag"; } diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java index 203e99565a..53a8439876 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessor.java @@ -18,6 +18,7 @@ package org.opensearch.searchpipelines.questionanswering.generative; import static org.opensearch.ingest.ConfigurationUtils.newConfigurationException; +import static org.opensearch.searchpipelines.questionanswering.generative.GenerativeQAProcessorConstants.RAG_NULL_GEN_QA_PARAMS_ERROR_MSG; import java.time.Duration; import java.time.Instant; @@ -127,10 +128,7 @@ public void processResponseAsync( GenerativeQAParameters params = GenerativeQAParamUtil.getGenerativeQAParameters(request); if (params == null) { - throw new IllegalArgumentException( - "generative_qa_parameters not found. Please provide ext.generative_qa_parameters to proceed." - + "For more info, refer: https://opensearch.org/docs/latest/search-plugins/conversational-search/#step-6-use-the-pipeline-for-rag" - ); + throw new IllegalArgumentException(RAG_NULL_GEN_QA_PARAMS_ERROR_MSG); } Integer t = params.getTimeout(); diff --git a/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessorTests.java b/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessorTests.java index a89b5c1731..4295ab450e 100644 --- a/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessorTests.java +++ b/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAResponseProcessorTests.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.opensearch.searchpipelines.questionanswering.generative.GenerativeQAProcessorConstants.RAG_NULL_GEN_QA_PARAMS_ERROR_MSG; import java.time.Instant; import java.util.Collections; @@ -646,6 +647,77 @@ public void testProcessResponseNullValueInteractions() throws Exception { })); } + public void testProcessResponseIllegalArgumentForNullParams() throws Exception { + exceptionRule.expect(IllegalArgumentException.class); + exceptionRule.expectMessage(RAG_NULL_GEN_QA_PARAMS_ERROR_MSG); + + Client client = mock(Client.class); + Map config = new HashMap<>(); + config.put(GenerativeQAProcessorConstants.CONFIG_NAME_MODEL_ID, "dummy-model"); + config.put(GenerativeQAProcessorConstants.CONFIG_NAME_CONTEXT_FIELD_LIST, List.of("text")); + + GenerativeQAResponseProcessor processor = (GenerativeQAResponseProcessor) new GenerativeQAResponseProcessor.Factory( + client, + alwaysOn + ).create(null, "tag", "desc", true, config, null); + + ConversationalMemoryClient memoryClient = mock(ConversationalMemoryClient.class); + List chatHistory = List + .of( + new Interaction( + "0", + Instant.now(), + "1", + "question", + "", + "answer", + "foo", + Collections.singletonMap("meta data", "some meta") + ) + ); + doAnswer(invocation -> { + ((ActionListener>) invocation.getArguments()[2]).onResponse(chatHistory); + return null; + }).when(memoryClient).getInteractions(any(), anyInt(), any()); + processor.setMemoryClient(memoryClient); + + SearchRequest request = new SearchRequest(); + SearchSourceBuilder sourceBuilder = new SearchSourceBuilder(); + GenerativeQAParamExtBuilder extBuilder = new GenerativeQAParamExtBuilder(); + extBuilder.setParams(null); + request.source(sourceBuilder); + sourceBuilder.ext(List.of(extBuilder)); + + int numHits = 10; + SearchHit[] hitsArray = new SearchHit[numHits]; + for (int i = 0; i < numHits; i++) { + XContentBuilder sourceContent = JsonXContent + .contentBuilder() + .startObject() + .field("_id", String.valueOf(i)) + .field("text", "passage" + i) + .field("title", "This is the title for document " + i) + .endObject(); + hitsArray[i] = new SearchHit(i, "doc" + i, Map.of(), Map.of()); + hitsArray[i].sourceRef(BytesReference.bytes(sourceContent)); + } + + SearchHits searchHits = new SearchHits(hitsArray, null, 1.0f); + SearchResponseSections internal = new SearchResponseSections(searchHits, null, null, false, false, null, 0); + SearchResponse response = new SearchResponse(internal, null, 1, 1, 0, 1, null, null, null); + + Llm llm = mock(Llm.class); + processor.setLlm(llm); + + processor + .processResponseAsync( + request, + response, + null, + ActionListener.wrap(r -> { assertTrue(r instanceof GenerativeSearchResponse); }, e -> {}) + ); + } + public void testProcessResponseIllegalArgument() throws Exception { exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage("llm_model cannot be null.");