Add search and singular APIs to conversation memory #1504

HenryL27 · 2023-10-12T18:02:54Z

Description

Adds search and singular apis to conversation memory as per this spec

The lower level architecture is identical to the rest of the conversation memory architecture - conversations index + interactions index; private-mode security. APIs are basically just wrappers around normal OpenSearch document APIs

Issues Resolved

#1268

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

HenryL27 · 2023-10-12T19:06:14Z

failed expected flakies in linux (20)

Tests with failures:
 - org.opensearch.ml.action.prediction.PredictionITTests.testPredictionWithDataFrame_LinearRegression
 - org.opensearch.ml.action.prediction.PredictionITTests.testPredictionWithoutDataset_KMeans

and these integtests in windows (20)

Tests with failures:
51 tests completed, 8 failed, 9 skipped
 - org.opensearch.ml.rest.RestMLDeployModelActionIT.testReDeployModel
 - org.opensearch.ml.rest.RestMLDeployModelActionIT.classMethod
 - org.opensearch.ml.rest.RestMLGetModelActionIT.testGetModelAPI_EmptyResources
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testOpenAIModerationsModel
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testCohereGenerateTextModel
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testOpenAIEditsModel
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testCohereClassifyModel
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testPredictRemoteModel

all unrelated to this PR

austintlee

I think you can reduce a lot of duplicate code (constructors and doExecute routines, e.g.) by using a base class and an interface with default method.

plugin/build.gradle

...in/java/org/opensearch/ml/memory/action/conversation/SearchConversationsTransportAction.java

...ain/java/org/opensearch/ml/memory/action/conversation/SearchInteractionsTransportAction.java

austintlee · 2023-10-16T21:19:39Z

memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java

+                    newQuery.must(originalQuery);
+                    newQuery.must(new TermQueryBuilder(ConversationalIndexConstants.INTERACTIONS_CONVERSATION_ID_FIELD, conversationId));
+                    request.source().query(newQuery);
+                    client.admin().indices().refresh(Requests.refreshRequest(indexName), ActionListener.wrap(refreshResponse -> {


Is 'refresh' necessary? Does it force OpenSearch to do something every time or is it smart enough not to do anything if the index hasn't changed.

memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java

austintlee · 2023-10-16T21:29:23Z

memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java

+        if (!clusterService.state().metadata().hasIndex(indexName)) {
+            listener.onFailure(new IndexNotFoundException("cannot get interaction since the interactions index does not exist", indexName));
+            return;
+        }


Wouldn't OS throw this error anyway when you execute the Get request below?

sure, but this way I shortcut the access control request

memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java

HenryL27 · 2023-11-01T15:30:46Z

@jngz-es @dhrubo-os @ylwu-amzn or someone can I get some eyes on this? Thanks

HenryL27 · 2023-11-13T21:16:09Z

@Zhangxunmt @ylwu-amzn this PR for memory search API

Zhangxunmt

@HenryL27 , I downloaded this PR and looks like there are still tests compile errors. Can you please update the tests and pass the CI?

Signed-off-by: HenryL27 <[email protected]>

codecov · 2023-11-29T18:01:24Z

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (df644ff) 80.83% compared to head (1f4bfdb) 81.19%.

Files	Patch %	Lines
...nversation/SearchConversationsTransportAction.java	86.36%	3 Missing ⚠️
...onversation/SearchInteractionsTransportAction.java	86.36%	3 Missing ⚠️
...nsearch/ml/memory/index/ConversationMetaIndex.java	96.47%	0 Missing and 3 partials ⚠️
.../opensearch/ml/memory/index/InteractionsIndex.java	97.36%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1504      +/-   ##
============================================
+ Coverage     80.83%   81.19%   +0.36%     
- Complexity     4215     4335     +120     
============================================
  Files           404      421      +17     
  Lines         16977    17303     +326     
  Branches       1818     1829      +11     
============================================
+ Hits          13723    14049     +326     
+ Misses         2539     2537       -2     
- Partials        715      717       +2

Flag	Coverage Δ
ml-commons	`81.19% <97.08%> (+0.36%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

HenryL27 · 2023-11-29T18:34:04Z

@Zhangxunmt rebased for gradle/java/lombok update. Failing CI tests are known flaky tests, not introduced by this PR. Can you try again? Thanks

austintlee

LGTM

* add searchConversation Signed-off-by: HenryL27 <[email protected]> * add searchinteractions Signed-off-by: HenryL27 <[email protected]> * add searchConversationsITTests Signed-off-by: HenryL27 <[email protected]> * add searchInteractionsITTests Signed-off-by: HenryL27 <[email protected]> * add unit tests for storage-layer search Signed-off-by: HenryL27 <[email protected]> * add Search transport actions and tests Signed-off-by: HenryL27 <[email protected]> * add rest search actions Signed-off-by: HenryL27 <[email protected]> * add search rest actions Signed-off-by: HenryL27 <[email protected]> * Add singular get actions at storage layer Signed-off-by: HenryL27 <[email protected]> * Add OpenSearhMemoryHandler unit tests for singular get Signed-off-by: HenryL27 <[email protected]> * Add singular get transport layer Signed-off-by: HenryL27 <[email protected]> * add singular get rest actions Signed-off-by: HenryL27 <[email protected]> * fix async return value problem Signed-off-by: HenryL27 <[email protected]> * address esay PR comments Signed-off-by: HenryL27 <[email protected]> --------- Signed-off-by: HenryL27 <[email protected]> (cherry picked from commit adc7da0)

…ct#1504) * add searchConversation Signed-off-by: HenryL27 <[email protected]> * add searchinteractions Signed-off-by: HenryL27 <[email protected]> * add searchConversationsITTests Signed-off-by: HenryL27 <[email protected]> * add searchInteractionsITTests Signed-off-by: HenryL27 <[email protected]> * add unit tests for storage-layer search Signed-off-by: HenryL27 <[email protected]> * add Search transport actions and tests Signed-off-by: HenryL27 <[email protected]> * add rest search actions Signed-off-by: HenryL27 <[email protected]> * add search rest actions Signed-off-by: HenryL27 <[email protected]> * Add singular get actions at storage layer Signed-off-by: HenryL27 <[email protected]> * Add OpenSearhMemoryHandler unit tests for singular get Signed-off-by: HenryL27 <[email protected]> * Add singular get transport layer Signed-off-by: HenryL27 <[email protected]> * add singular get rest actions Signed-off-by: HenryL27 <[email protected]> * fix async return value problem Signed-off-by: HenryL27 <[email protected]> * address esay PR comments Signed-off-by: HenryL27 <[email protected]> --------- Signed-off-by: HenryL27 <[email protected]>

…1504) (#1720) * Add search and singular APIs to conversation memory (#1504) * add searchConversation Signed-off-by: HenryL27 <[email protected]> * add searchinteractions Signed-off-by: HenryL27 <[email protected]> * add searchConversationsITTests Signed-off-by: HenryL27 <[email protected]> * add searchInteractionsITTests Signed-off-by: HenryL27 <[email protected]> * add unit tests for storage-layer search Signed-off-by: HenryL27 <[email protected]> * add Search transport actions and tests Signed-off-by: HenryL27 <[email protected]> * add rest search actions Signed-off-by: HenryL27 <[email protected]> * add search rest actions Signed-off-by: HenryL27 <[email protected]> * Add singular get actions at storage layer Signed-off-by: HenryL27 <[email protected]> * Add OpenSearhMemoryHandler unit tests for singular get Signed-off-by: HenryL27 <[email protected]> * Add singular get transport layer Signed-off-by: HenryL27 <[email protected]> * add singular get rest actions Signed-off-by: HenryL27 <[email protected]> * fix async return value problem Signed-off-by: HenryL27 <[email protected]> * address esay PR comments Signed-off-by: HenryL27 <[email protected]> --------- Signed-off-by: HenryL27 <[email protected]> * fix apache http version Signed-off-by: HenryL27 <[email protected]> --------- Signed-off-by: HenryL27 <[email protected]>

…ct#1504) * add searchConversation Signed-off-by: HenryL27 <[email protected]> * add searchinteractions Signed-off-by: HenryL27 <[email protected]> * add searchConversationsITTests Signed-off-by: HenryL27 <[email protected]> * add searchInteractionsITTests Signed-off-by: HenryL27 <[email protected]> * add unit tests for storage-layer search Signed-off-by: HenryL27 <[email protected]> * add Search transport actions and tests Signed-off-by: HenryL27 <[email protected]> * add rest search actions Signed-off-by: HenryL27 <[email protected]> * add search rest actions Signed-off-by: HenryL27 <[email protected]> * Add singular get actions at storage layer Signed-off-by: HenryL27 <[email protected]> * Add OpenSearhMemoryHandler unit tests for singular get Signed-off-by: HenryL27 <[email protected]> * Add singular get transport layer Signed-off-by: HenryL27 <[email protected]> * add singular get rest actions Signed-off-by: HenryL27 <[email protected]> * fix async return value problem Signed-off-by: HenryL27 <[email protected]> * address esay PR comments Signed-off-by: HenryL27 <[email protected]> --------- Signed-off-by: HenryL27 <[email protected]>

HenryL27 requested review from b4sjoo, dhrubo-os, jngz-es, model-collapse, rbhavna, wujunshen, ylwu-amzn, zane-neo and Zhangxunmt as code owners October 12, 2023 18:02

HenryL27 had a problem deploying to ml-commons-cicd-env October 12, 2023 18:02 — with GitHub Actions Error

HenryL27 temporarily deployed to ml-commons-cicd-env October 12, 2023 18:03 — with GitHub Actions Inactive

HenryL27 had a problem deploying to ml-commons-cicd-env October 12, 2023 18:03 — with GitHub Actions Error

HenryL27 temporarily deployed to ml-commons-cicd-env October 12, 2023 18:03 — with GitHub Actions Inactive

HenryL27 had a problem deploying to ml-commons-cicd-env October 12, 2023 18:03 — with GitHub Actions Failure

saratvemulapalli mentioned this pull request Oct 12, 2023

[FEATURE] Singular and _search api for Conversational Memory #1268

Closed

austintlee reviewed Oct 16, 2023

View reviewed changes

HenryL27 had a problem deploying to ml-commons-cicd-env October 17, 2023 22:24 — with GitHub Actions Error

HenryL27 temporarily deployed to ml-commons-cicd-env October 17, 2023 22:24 — with GitHub Actions Inactive

HenryL27 had a problem deploying to ml-commons-cicd-env October 17, 2023 22:24 — with GitHub Actions Failure

HenryL27 mentioned this pull request Nov 9, 2023

[FEATURE] Memory Enhancement to Support more LLM applications #1614

Closed

Zhangxunmt reviewed Nov 20, 2023

View reviewed changes

HenryL27 added 2 commits November 29, 2023 09:30

add searchConversation

04e66b8

Signed-off-by: HenryL27 <[email protected]>

add searchinteractions

6303d23

Signed-off-by: HenryL27 <[email protected]>

HenryL27 added 11 commits November 29, 2023 09:30

add searchInteractionsITTests

16bd75b

Signed-off-by: HenryL27 <[email protected]>

add unit tests for storage-layer search

f6d1c0b

Signed-off-by: HenryL27 <[email protected]>

add Search transport actions and tests

33d0c3b

Signed-off-by: HenryL27 <[email protected]>

add rest search actions

e8b160a

Signed-off-by: HenryL27 <[email protected]>

add search rest actions

baf6619

Signed-off-by: HenryL27 <[email protected]>

Add singular get actions at storage layer

9c3a693

Signed-off-by: HenryL27 <[email protected]>

Add OpenSearhMemoryHandler unit tests for singular get

a630550

Signed-off-by: HenryL27 <[email protected]>

Add singular get transport layer

3f7404c

Signed-off-by: HenryL27 <[email protected]>

add singular get rest actions

a0c49c2

Signed-off-by: HenryL27 <[email protected]>

fix async return value problem

9b4c461

Signed-off-by: HenryL27 <[email protected]>

address esay PR comments

1f4bfdb

Signed-off-by: HenryL27 <[email protected]>

HenryL27 force-pushed the main branch from 09221dd to 1f4bfdb Compare November 29, 2023 17:40

HenryL27 had a problem deploying to ml-commons-cicd-env November 29, 2023 17:40 — with GitHub Actions Error

HenryL27 had a problem deploying to ml-commons-cicd-env November 29, 2023 17:40 — with GitHub Actions Failure

HenryL27 temporarily deployed to ml-commons-cicd-env November 29, 2023 17:41 — with GitHub Actions Inactive

HenryL27 had a problem deploying to ml-commons-cicd-env November 29, 2023 17:41 — with GitHub Actions Failure

Zhangxunmt approved these changes Nov 29, 2023

View reviewed changes

HenryL27 requested a review from austintlee November 30, 2023 00:51

austintlee approved these changes Nov 30, 2023

View reviewed changes

HenryL27 merged commit adc7da0 into opensearch-project:main Nov 30, 2023
8 of 12 checks passed

HenryL27 added the backport 2.x label Nov 30, 2023

opensearch-trigger-bot bot mentioned this pull request Nov 30, 2023

[Backport 2.x] Add search and singular APIs to conversation memory #1718

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add search and singular APIs to conversation memory #1504

Add search and singular APIs to conversation memory #1504

HenryL27 commented Oct 12, 2023 •

edited

Loading

HenryL27 commented Oct 12, 2023 •

edited

Loading

austintlee left a comment

austintlee Oct 16, 2023

austintlee Oct 16, 2023

HenryL27 Oct 17, 2023

HenryL27 commented Nov 1, 2023

HenryL27 commented Nov 13, 2023

Zhangxunmt left a comment

codecov bot commented Nov 29, 2023

HenryL27 commented Nov 29, 2023

austintlee left a comment

Add search and singular APIs to conversation memory #1504

Add search and singular APIs to conversation memory #1504

Conversation

HenryL27 commented Oct 12, 2023 • edited Loading

Description

Issues Resolved

Check List

HenryL27 commented Oct 12, 2023 • edited Loading

austintlee left a comment

Choose a reason for hiding this comment

austintlee Oct 16, 2023

Choose a reason for hiding this comment

austintlee Oct 16, 2023

Choose a reason for hiding this comment

HenryL27 Oct 17, 2023

Choose a reason for hiding this comment

HenryL27 commented Nov 1, 2023

HenryL27 commented Nov 13, 2023

Zhangxunmt left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 29, 2023

Codecov Report

HenryL27 commented Nov 29, 2023

austintlee left a comment

Choose a reason for hiding this comment

HenryL27 commented Oct 12, 2023 •

edited

Loading

HenryL27 commented Oct 12, 2023 •

edited

Loading