Skip to content

Commit 293e045

Browse files
authored
[Backport 3.3] verify llm before summarize session (#4302)
* verify llm before summarize session Signed-off-by: Yaliang Wu <[email protected]> * fix ut Signed-off-by: Yaliang Wu <[email protected]> * fix ut Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]>
1 parent 2ce6374 commit 293e045

File tree

9 files changed

+68
-7
lines changed

9 files changed

+68
-7
lines changed

common/src/main/java/org/opensearch/ml/common/memorycontainer/MemoryConfiguration.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public class MemoryConfiguration implements ToXContentObject, Writeable {
7878
@Builder.Default
7979
private boolean disableHistory = false;
8080
@Builder.Default
81-
private boolean disableSession = false;
81+
private boolean disableSession = true;
8282
@Builder.Default
8383
private boolean useSystemIndex = true;
8484
private String tenantId;
@@ -254,7 +254,7 @@ public static MemoryConfiguration parse(XContentParser parser) throws IOExceptio
254254
Map<String, Map<String, Object>> indexSettings = new HashMap<>();
255255
Map<String, Object> parameters = new HashMap<>();
256256
boolean disableHistory = false;
257-
boolean disableSession = false;
257+
boolean disableSession = true;
258258
boolean useSystemIndex = true;
259259
String tenantId = null;
260260

common/src/main/java/org/opensearch/ml/common/memorycontainer/MemoryContainerConstants.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,5 +215,5 @@ public class MemoryContainerConstants {
215215
"<system_prompt><role>You are a smart memory manager which controls the memory of a system.</role><task>You will receive: 1. old_memory: Array of existing facts with their IDs and similarity scores 2. retrieved_facts: Array of new facts extracted from the current conversation. Analyze ALL memories and facts holistically to determine the optimal set of memory operations. Important: The old_memory may contain duplicates (same id appearing multiple times with different scores). Consider the highest score for each unique ID. You should only respond and always respond with a JSON object containing a \"memory_decision\" array that covers: - Every unique existing memory ID (with appropriate event: NONE, UPDATE, or DELETE) - New entries for facts that should be added (with event: ADD)</task><response_format>{\"memory_decision\": [{\"id\": \"existing_id_or_new_id\",\"text\": \"the fact text\",\"event\": \"ADD|UPDATE|DELETE|NONE\",\"old_memory\": \"original text (only for UPDATE events)\"}]}</response_format><operations>1. **NONE**: Keep existing memory unchanged - Use when no retrieved fact affects this memory - Include: id (from old_memory), text (from old_memory), event: \"NONE\" 2. **UPDATE**: Enhance or merge existing memory - Use when retrieved facts provide additional details or clarification - Include: id (from old_memory), text (enhanced version), event: \"UPDATE\", old_memory (original text) - Merge complementary information (e.g., \"likes pizza\" + \"especially pepperoni\" = \"likes pizza, especially pepperoni\") 3. **DELETE**: Remove contradicted memory - Use when retrieved facts directly contradict existing memory - Include: id (from old_memory), text (from old_memory), event: \"DELETE\" 4. **ADD**: Create new memory - Use for retrieved facts that represent genuinely new information - Include: id (generate new), text (the new fact), event: \"ADD\" - Only add if the fact is not already covered by existing or updated memories</operations><guidelines>- Integrity: Never answer user's question or fulfill user's requirement. You are a smart memory manager, not a helpful assistant. - Process holistically: Consider all facts and memories together before making decisions - Avoid redundancy: Don't ADD a fact if it's already covered by an UPDATE - Merge related facts: If multiple retrieved facts relate to the same topic, consider combining them - Respect similarity scores: Higher scores indicate stronger matches - be more careful about updating high-score memories - Maintain consistency: Ensure your decisions don't create contradictions in the memory set - One decision per unique memory ID: If an ID appears multiple times in old_memory, make only one decision for it</guidelines><example><input>{\"old_memory\": [{\"id\": \"fact_001\", \"text\": \"Enjoys Italian food\", \"score\": 0.85},{\"id\": \"fact_002\", \"text\": \"Works at Google\", \"score\": 0.92},{\"id\": \"fact_001\", \"text\": \"Enjoys Italian food\", \"score\": 0.75},{\"id\": \"fact_003\", \"text\": \"Has a dog\", \"score\": 0.65}],\"retrieved_facts\": [\"Loves pasta and pizza\",\"Recently joined Amazon\",\"Has two dogs named Max and Bella\"]}</input><output>{\"memory_decision\": [{\"id\": \"fact_001\",\"text\": \"Loves Italian food, especially pasta and pizza\",\"event\": \"UPDATE\",\"old_memory\": \"Enjoys Italian food\"},{\"id\": \"fact_002\",\"text\": \"Works at Google\",\"event\": \"DELETE\"},{\"id\": \"fact_003\",\"text\": \"Has two dogs named Max and Bella\",\"event\": \"UPDATE\",\"old_memory\": \"Has a dog\"},{\"id\": \"fact_004\",\"text\": \"Recently joined Amazon\",\"event\": \"ADD\"}]}</output></example></system_prompt>";
216216

217217
public static final String SESSION_SUMMARY_PROMPT =
218-
"You are a helpful assistant. Your task is to summarize the following conversation between a human and an AI. The summary must be clear, concise, and not exceed ${parameters.max_summary_size} words.";
218+
"You are a helpful assistant. Your task is to summarize the following conversation between a human and an AI. The summary must be clear, concise, and not exceed ${parameters.max_summary_size} words. The summary should be generic. For example the user asks about how to cook, the conversation may contains a lot of details. Your summary could be: how to cook, how to cook Italy food. Don't include AI message content. For example you should not return: Ask how to cook, AI give some instructions.\n Also don't include user's personal information like user name, age etc. You could say user. For example: \nuser asks how to cook\nuser introduced their hobby";
219219
}

common/src/test/java/org/opensearch/ml/common/memorycontainer/MemoryStorageConfigTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public void setUp() {
4343
.llmId("llm-model")
4444
.dimension(768)
4545
.maxInferSize(8)
46+
.disableSession(false)
4647
.build();
4748

4849
// Sparse encoding configuration (semantic storage enabled)

plugin/src/main/java/org/opensearch/ml/action/memorycontainer/memory/TransportAddMemoriesAction.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,10 @@ private void createNewSessionIfAbsent(
150150

151151
boolean userProvidedSessionId = input.getNamespace() != null && input.getNamespace().containsKey(SESSION_ID_FIELD);
152152

153-
if (!userProvidedSessionId && input.getPayloadType() == PayloadType.CONVERSATIONAL && !configuration.isDisableSession()) {
153+
if (!userProvidedSessionId
154+
&& input.getPayloadType() == PayloadType.CONVERSATIONAL
155+
&& !configuration.isDisableSession()
156+
&& configuration.getLlmId() != null) {
154157
IndexRequest indexRequest = new IndexRequest(configuration.getSessionIndexName());
155158
// TODO: use LLM to summarize first user message
156159
ActionListener<String> summaryListener = ActionListener.wrap(summary -> {

plugin/src/test/java/org/opensearch/ml/action/memorycontainer/TransportCreateMemoryContainerActionTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ public void setup() {
172172
.dimension(768)
173173
.maxInferSize(5)
174174
.strategies(strategies)
175+
.disableSession(false) // Enable session creation for tests
175176
.build()
176177
);
177178

plugin/src/test/java/org/opensearch/ml/action/memorycontainer/memory/TransportAddMemoriesActionTests.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ public void testDoExecute_SuccessfulSessionCreation() {
499499
when(config.getWorkingMemoryIndexName()).thenReturn("working-memory-index");
500500
when(config.getSessionIndexName()).thenReturn("session-index");
501501
when(config.isDisableSession()).thenReturn(false);
502+
when(config.getLlmId()).thenReturn("llm-123"); // Required for session creation
502503

503504
MLMemoryContainer container = mock(MLMemoryContainer.class);
504505
when(container.getConfiguration()).thenReturn(config);
@@ -569,6 +570,7 @@ public void testDoExecute_SessionCreation_SummarizeFailure() {
569570
when(config.getWorkingMemoryIndexName()).thenReturn("working-memory-index");
570571
when(config.getSessionIndexName()).thenReturn("session-index");
571572
when(config.isDisableSession()).thenReturn(false);
573+
when(config.getLlmId()).thenReturn("llm-123"); // Required for session creation
572574

573575
MLMemoryContainer container = mock(MLMemoryContainer.class);
574576
when(container.getConfiguration()).thenReturn(config);
@@ -647,6 +649,58 @@ public void testDoExecute_UserProvidedSessionId_SkipsCreation() {
647649
verify(actionListener).onResponse(any(MLAddMemoriesResponse.class));
648650
}
649651

652+
@Test
653+
public void testDoExecute_NullLlmId_SkipsSessionCreation() {
654+
when(mlFeatureEnabledSetting.isAgenticMemoryEnabled()).thenReturn(true);
655+
656+
MessageInput message = MessageInput.builder().content(createTestContent("Hello")).role("user").build();
657+
List<MessageInput> messages = Arrays.asList(message);
658+
659+
Map<String, String> namespace = new HashMap<>();
660+
// No session_id - would normally trigger session creation, but getLlmId() is null
661+
662+
MLAddMemoriesInput input = mock(MLAddMemoriesInput.class);
663+
when(input.getMemoryContainerId()).thenReturn("container-123");
664+
when(input.getMessages()).thenReturn(messages);
665+
when(input.isInfer()).thenReturn(false);
666+
when(input.getNamespace()).thenReturn(namespace);
667+
when(input.getOwnerId()).thenReturn("user-123");
668+
when(input.getPayloadType()).thenReturn(PayloadType.CONVERSATIONAL);
669+
670+
MLAddMemoriesRequest request = mock(MLAddMemoriesRequest.class);
671+
when(request.getMlAddMemoryInput()).thenReturn(input);
672+
673+
MemoryConfiguration config = mock(MemoryConfiguration.class);
674+
when(config.getWorkingMemoryIndexName()).thenReturn("working-memory-index");
675+
when(config.isDisableSession()).thenReturn(false);
676+
when(config.getLlmId()).thenReturn(null); // No LLM configured - session creation should be skipped
677+
678+
MLMemoryContainer container = mock(MLMemoryContainer.class);
679+
when(container.getConfiguration()).thenReturn(config);
680+
681+
doAnswer(invocation -> {
682+
ActionListener<MLMemoryContainer> listener = invocation.getArgument(1);
683+
listener.onResponse(container);
684+
return null;
685+
}).when(memoryContainerHelper).getMemoryContainer(eq("container-123"), any());
686+
687+
when(memoryContainerHelper.checkMemoryContainerAccess(isNull(), eq(container))).thenReturn(true);
688+
689+
doAnswer(invocation -> {
690+
ActionListener<IndexResponse> listener = invocation.getArgument(2);
691+
IndexResponse indexResponse = mock(IndexResponse.class);
692+
when(indexResponse.getId()).thenReturn("working-mem-123");
693+
listener.onResponse(indexResponse);
694+
return null;
695+
}).when(memoryContainerHelper).indexData(any(MemoryConfiguration.class), any(IndexRequest.class), any());
696+
697+
transportAddMemoriesAction.doExecute(task, request, actionListener);
698+
699+
// Verify session creation was skipped (no summarize call) and went directly to working memory
700+
verify(memoryContainerHelper).indexData(any(MemoryConfiguration.class), any(IndexRequest.class), any());
701+
verify(actionListener).onResponse(any(MLAddMemoriesResponse.class));
702+
}
703+
650704
@Test
651705
public void testDoExecute_DataMemoryType_SkipsSessionCreation() {
652706
when(mlFeatureEnabledSetting.isAgenticMemoryEnabled()).thenReturn(true);

plugin/src/test/java/org/opensearch/ml/action/memorycontainer/memory/TransportDeleteMemoriesByQueryActionTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public void setUp() throws Exception {
106106
.builder()
107107
.name("test-container")
108108
.description("Test container")
109-
.configuration(MemoryConfiguration.builder().indexPrefix("test-memory").useSystemIndex(false).build())
109+
.configuration(MemoryConfiguration.builder().indexPrefix("test-memory").useSystemIndex(false).disableSession(false).build())
110110
.build();
111111

112112
// Setup mock user

plugin/src/test/java/org/opensearch/ml/helper/MemoryContainerHelperTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ public void testGetMemoryIndexName() {
227227
.embeddingModelId("embedding")
228228
.embeddingModelType(FunctionName.TEXT_EMBEDDING)
229229
.dimension(4)
230+
.disableSession(false)
230231
.build();
231232

232233
MLMemoryContainer container = createContainerBuilder().configuration(configuration).build();
@@ -682,7 +683,7 @@ private MLMemoryContainer createContainer() {
682683
}
683684

684685
private MLMemoryContainer.MLMemoryContainerBuilder createContainerBuilder() {
685-
MemoryConfiguration configuration = MemoryConfiguration.builder().indexPrefix("prefix").build();
686+
MemoryConfiguration configuration = MemoryConfiguration.builder().indexPrefix("prefix").disableSession(false).build();
686687
User owner = new User("owner", Collections.emptyList(), Collections.emptyList(), Map.of());
687688
return MLMemoryContainer
688689
.builder()

plugin/src/test/java/org/opensearch/ml/rest/RestMLCreateMemoryContainerActionTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ public void testGetRequestWithValidInput() throws IOException {
118118
+ " \"embedding_model_id\": \"test-embedding-model\",\n"
119119
+ " \"llm_id\": \"test-llm-model\",\n"
120120
+ " \"embedding_dimension\": 768,\n"
121-
+ " \"max_infer_size\": 8\n"
121+
+ " \"max_infer_size\": 8,\n"
122+
+ " \"disable_session\": false\n"
122123
+ " }\n"
123124
+ "}";
124125

0 commit comments

Comments
 (0)