Prompt Management Create API #3779

rcho93 · 2025-04-26T00:21:10Z

Description

Implements an API to create prompt, and creates a new system index for prompt

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

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.

dhrubo-os · 2025-04-26T01:39:27Z

Apply spotless: ./gradlew spotlessApply

dhrubo-os · 2025-04-26T01:41:01Z

common/src/main/java/org/opensearch/ml/common/MLIndex.java

 import java.io.IOException;
 import java.io.UncheckedIOException;

 import org.opensearch.ml.common.utils.IndexUtils;

+import static org.opensearch.ml.common.CommonValue.*;


Don't use * in import. Disable wilecard import: https://www.baeldung.com/intellij-disable-wildcard-import

Got it. Thank you!

dhrubo-os · 2025-04-26T01:43:57Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+        if (tag != null) {
+            builder.field(TAG_FIELD, tag);
+        }
+        if (createTime != null) {


tenantid is missing here.

dhrubo-os · 2025-04-26T01:43:57Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+                .build();
+    }
+
+    /*public void update(MLCreatePromptInput updateContent) {


can we remove this?

I did not commit my changes frequently while working on this project. To avoid ending up with a large PR that would be difficult for reviewers to go through, I am creating multiple smaller PRs instead.
So, I am manually commenting out unrelated code so that each PR stays focused on a specific topic. I will be using this method for Update api.

…mpanied by text (opensearch-project#3755) * fix: handle model response when toolUse is not accompanied by text Signed-off-by: Pavan Yekbote <[email protected]> * feat: add test case for parseLLMOutput Signed-off-by: Pavan Yekbote <[email protected]> --------- Signed-off-by: Pavan Yekbote <[email protected]>

…ent empty response (opensearch-project#3756) * fix: expose max_iteration for react Signed-off-by: Pavan Yekbote <[email protected]> * fix: defaults for agent execution and differentiate between step and step result Signed-off-by: Pavan Yekbote <[email protected]> * fix: return react agent id in agent response to expose more details Signed-off-by: Pavan Yekbote <[email protected]> * spotless Signed-off-by: Pavan Yekbote <[email protected]> * fix: remove test prompt from react system prompt Signed-off-by: Pavan Yekbote <[email protected]> * refactor: rename parameters exposed to user to executor Signed-off-by: Pavan Yekbote <[email protected]> * fix: give user complete control over planner system prompt Signed-off-by: Pavan Yekbote <[email protected]> --------- Signed-off-by: Pavan Yekbote <[email protected]>

Signed-off-by: rithin-pullela-aws <[email protected]>

rcho93 · 2025-04-26T21:09:57Z

@ohltyler, Hey Tyler, This is my first draft PR for CREATE API for pormpt management:)

mingshl · 2025-04-26T22:00:49Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+
+@Getter
+@EqualsAndHashCode
+public class MLPrompt implements ToXContentObject, Writeable {


add java doc for every class and main method so that it's readable

If this is a new file, a header is needed. Check the other classes and copy the header in all new files.
/*

Copyright OpenSearch Contributors

SPDX-License-Identifier: Apache-2.0
*/

mingshl · 2025-04-26T22:01:16Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+    }
+
+    public MLPrompt(StreamInput input) throws IOException {
+        Version streamInputVersion = input.getVersion();


is the version a mandatory field?

Yes, I think the version should be a mandatory field, and should throw an error if the version field is not provided by the user.

can we do version has a default one if no version is required? I would assume when I don't put a version, it has an initial version.

Okay, I will make the version field optional. Thank you!

mingshl · 2025-04-26T22:02:10Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+            this.prompt = input.readMap(s -> s.readString(), s -> s.readString());
+        }
+        this.tag = input.readOptionalString();
+        tenantId = streamInputVersion.onOrAfter(VERSION_2_19_0) ? input.readOptionalString() : null;


this.tenantId?

mingshl · 2025-04-26T22:16:55Z

plugin/src/main/java/org/opensearch/ml/action/prompt/TransportCreatePromptAction.java

+
+    private void indexPrompt(MLPrompt prompt, ActionListener<MLCreatePromptResponse> listener) {
+        log.info("prompt created, indexing into the prompt system index");
+        mlIndicesHandler.initMLPromptIndex(ActionListener.wrap(indexCreated -> {


when creating the index, do you have validation to check if the prompt id is existed? can you create a prompt twice with the same prompt id?

also thinking can user create two prompt with the same name? have you consider the scenarios?

I decided to make prompt create api consistent as the other create apis, like connector. I noticed that the connector_id is set to a document id that is generated when a new document is stored in index. So, I followed the same mechanism without including any validation checks on promptId, since we are not manually creating one. Naming as well, other ml-commons features such as connector and model don't have a unique name and do not throw any errors when existing name is used.

mingshl · 2025-04-26T22:17:31Z

common/src/main/resources/index-mappings/ml_prompt.json

+      "type": "text"
+    },
+    "prompt": {
+      "type": "flat_object"


are you trying add tags later?

brianf-aws

Exciting work so far lets keep it up! 😄

brianf-aws · 2025-04-28T18:21:09Z

ml-algorithms/src/main/java/org/opensearch/ml/engine/indices/MLIndicesHandler.java

@@ -119,6 +123,7 @@ public void initMLIndexIfAbsent(MLIndex index, ActionListener<Boolean> listener)
                log.debug("index:{} is already created", indexName);
                if (indexMappingUpdated.containsKey(indexName) && !indexMappingUpdated.get(indexName).get()) {
                    shouldUpdateIndex(indexName, index.getVersion(), ActionListener.wrap(r -> {
+                        log.info("r is " + r);


Now looking back is this a log for your dev purpose? If so lets delete it

brianf-aws · 2025-04-28T18:29:38Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+        out.writeOptionalString(name);
+        out.writeOptionalString(description);
+        if (prompt != null) {
+            out.writeBoolean(true);


Its not to clear what field this boolean pertains to I'm assuming its a update version sort of boolean can we refactor so we assign some variable affected by this to make it clear to the reader what is this related to?

brianf-aws · 2025-04-28T18:33:27Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

+            String description,
+            Map<String, String> prompt,
+            String tag,
+            boolean dryRun,


Whats your interpretation of dryRun?

brianf-aws · 2025-04-28T18:36:21Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

+        Version streamInputVersion = input.getVersion();
+        name = input.readOptionalString();
+        description = input.readOptionalString();
+        if (input.readBoolean()) {


updatePrompt = input.readBoolean();

if(updatePrompt){
// Read from streamInput logic
}

brianf-aws · 2025-04-28T18:37:16Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

+        tag = input.readOptionalString();
+        dryRun = input.readBoolean();
+        if (streamInputVersion.onOrAfter(VERSION_2_19_0)) {
+            input.readOptionalString();


Why do we read here and not use and not store does this advance to the next item?

brianf-aws · 2025-04-28T18:38:48Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptRequest.java

+    public ActionRequestValidationException validate() {
+        ActionRequestValidationException exception = null;
+        if (mlCreatePromptInput == null) {
+            exception = addValidationError("ML Prompt input can't be null", exception);


If this string gets used over "ML Prompt input can't be null lets put it in a file to reference later if you intend to reuse the string over other actions/methods.

brianf-aws · 2025-04-28T18:45:39Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

+
+    private static final Version MINIMAL_SUPPORTED_VERSION_FOR_CLIENT_CONFIG = CommonValue.VERSION_2_13_0;
+
+    public static final String DRY_RUN_PROMPT_NAME = "dryRunPrompt";


Walk me though this variable what are its implications I see this variable used a lot "dryRyn*"

brianf-aws · 2025-04-28T18:48:25Z

plugin/src/main/java/org/opensearch/ml/action/prompt/TransportCreatePromptAction.java

+            try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
+                prompt.setCreateTime(Instant.now());
+                prompt.setLastUpdateTime(Instant.now());
+                sdkClient


Lets do some refactoring to avoid callback hell

http://callbackhell.com/

Zhangxunmt · 2025-04-28T21:08:14Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+
+@Getter
+@EqualsAndHashCode
+public class MLPrompt implements ToXContentObject, Writeable {


If this is a new file, a header is needed. Check the other classes and copy the header in all new files.
/*

Copyright OpenSearch Contributors

SPDX-License-Identifier: Apache-2.0
*/

Zhangxunmt · 2025-04-28T21:48:22Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

+        this.name = name;
+        this.description = description;
+        this.prompt = prompt;
+        this.tag = tag;


Any of these shouldn't be null? I think at least the name shouldn't be null, so it's better to add validation here.

Good call, I think name and prompt should not be null and force user to specify fields additionally do we think we ought to make prompt not be empty as well (@Rycho)?

Zhangxunmt · 2025-04-28T21:49:12Z

plugin/src/main/java/org/opensearch/ml/rest/RestMLCreatePromptAction.java

+        if (!request.hasContent()) {
+            throw new IOException("Create Prompt request has empty body");
+        }
+        XContentParser parser = request.contentParser();
+        ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
+        MLCreatePromptInput mlCreatePromptInput = MLCreatePromptInput.parse(parser);


It's better to either add validation in here or in the MLCreatePromptInput

…ect#3785)

* initial commit for MCP server in OpenSearch Signed-off-by: zane-neo <[email protected]> * Make change to support register or remove tools across cluster Signed-off-by: zane-neo <[email protected]> * format code Signed-off-by: zane-neo <[email protected]> * fix UT failure caused by code change Signed-off-by: zane-neo <[email protected]> * format code Signed-off-by: zane-neo <[email protected]> * format code Signed-off-by: zane-neo <[email protected]> * add license header Signed-off-by: zane-neo <[email protected]> * fix notifications initialized not respond issue Signed-off-by: zane-neo <[email protected]> * fix minor issues and add UTs Signed-off-by: zane-neo <[email protected]> * Add more UTs Signed-off-by: zane-neo <[email protected]> --------- Signed-off-by: zane-neo <[email protected]>

…3795) (cherry picked from commit 3f503f1) Signed-off-by: Peter Zhu <[email protected]> Co-authored-by: Peter Zhu <[email protected]>

* Increment version to 3.1.0-SNAPSHOT Signed-off-by: opensearch-ci-bot <[email protected]> * Update build.gradle Signed-off-by: Peter Zhu <[email protected]> --------- Signed-off-by: opensearch-ci-bot <[email protected]> Signed-off-by: Peter Zhu <[email protected]> Co-authored-by: opensearch-ci-bot <[email protected]> Co-authored-by: Peter Zhu <[email protected]>

Signed-off-by: Mingshi Liu <[email protected]>

* support MCP session management Signed-off-by: zane-neo <[email protected]> * Addressing comments Signed-off-by: zane-neo <[email protected]> * add feature flag for mcp server and renaming mcp connector feature flag Signed-off-by: zane-neo <[email protected]> * Address critical comments in opensearch-project#3781 Signed-off-by: zane-neo <[email protected]> --------- Signed-off-by: zane-neo <[email protected]>

* upgrade http client to versoin align with core Signed-off-by: zane-neo <[email protected]> * upgrade httpclient-h2 to correct versiono Signed-off-by: zane-neo <[email protected]> * use placeholder approach Signed-off-by: zane-neo <[email protected]> --------- Signed-off-by: zane-neo <[email protected]>

brianf-aws

Almost there; I think we should tweak the exception messages but other than that looks good to me

brianf-aws · 2025-05-05T22:24:45Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

        }
+        if (!prompt.containsKey(PROMPT_FIELD_SYSTEM_PROMPT)) {
+            throw new IllegalArgumentException("Prompt prompt field require system prompt");


Similar flow for ones in same method

Suggested change

throw new IllegalArgumentException("Prompt prompt field require system prompt");

throw new IllegalArgumentException("MLPrompt prompt field requires {PROMPT_FIELD_SYSTEM_PROMPT} parameter");

brianf-aws · 2025-05-05T22:27:13Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

@@ -166,7 +179,7 @@ public static MLCreatePromptInput parse(XContentParser parser, boolean updatePro
                    break;
            }
        }
-        return new MLCreatePromptInput(name, description, prompt, tag, tenantId, updatePrompt);
+        return new MLCreatePromptInput(name, description, version, prompt, tags, tenantId);


Oh can we not use the builder here?

brianf-aws · 2025-05-05T22:41:17Z

plugin/src/main/java/org/opensearch/ml/action/prompt/TransportCreatePromptAction.java

-    private void handlePromptPutFailure(Exception cause, ActionListener<MLCreatePromptResponse> listener) {
-        log.error("Failed to save ML Prompt", cause);
+    private void handlePromptPutFailure(Exception cause, ActionListener<MLCreatePromptResponse> listener, String likelyCause) {
+        log.error("Failed to save ML Prompt: {}", likelyCause, cause);


Awesome!!! This makes the method more user friendly do you think we can resuse the method elswhere?

dhrubo-os · 2025-05-06T18:25:49Z

common/src/main/java/org/opensearch/ml/common/transport/prompt/MLCreatePromptInput.java

+    public static final String PROMPT_FIELD_USER_PROMPT = "user";
+    public static final String PROMPT_FIELD_SYSTEM_PROMPT = "system";
+
+    private static final Version MINIMAL_SUPPORTED_VERSION_FOR_CLIENT_CONFIG = CommonValue.VERSION_2_13_0;


Why do we need this here?

oops, just fixed this :)

dhrubo-os · 2025-05-06T18:27:45Z

common/src/main/java/org/opensearch/ml/common/MLPrompt.java

+        this.version = input.readOptionalString();
+        this.prompt = input.readMap(s -> s.readString(), s -> s.readString());
+        this.tags = input.readOptionalStringList();
+        this.tenantId = streamInputVersion.onOrAfter(VERSION_2_19_0) ? input.readOptionalString() : null;


We usually do this type of check for the backward compatibility for the older versions. So I don't think we need this type of check here, as multi-tenancy feature is already published.

brianf-aws · 2025-05-06T22:03:30Z

Hey Ryan Lets add UTs for this PR :)

Signed-off-by: seungwon cho <[email protected]>

dhrubo-os reviewed Apr 26, 2025

View reviewed changes

pyek-bot and others added 3 commits April 25, 2025 21:54

Clean up JSM from MCP (opensearch-project#3773)

89cb24a

Signed-off-by: rithin-pullela-aws <[email protected]>

rcho93 marked this pull request as ready for review April 26, 2025 21:04

rcho93 requested review from b4sjoo, mingshl, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27 and xinyual as code owners April 26, 2025 21:04

mingshl reviewed Apr 26, 2025

View reviewed changes

brianf-aws suggested changes Apr 28, 2025

View reviewed changes

Zhangxunmt reviewed Apr 28, 2025

View reviewed changes

pyek-bot and others added 7 commits April 28, 2025 16:22

[Bug] ListTools call does not return tool attributes (opensearch-proj…

ed4f09f

…ect#3785)

Remove beta1 qualifier (opensearch-project#3794) (opensearch-project#…

f6d750f

…3795) (cherry picked from commit 3f503f1) Signed-off-by: Peter Zhu <[email protected]> Co-authored-by: Peter Zhu <[email protected]>

add release note for 3.0 (opensearch-project#3792)

c51c5e4

Signed-off-by: Mingshi Liu <[email protected]>

brianf-aws approved these changes May 6, 2025

View reviewed changes

rcho93 requested a deployment to ml-commons-cicd-env-require-approval May 6, 2025 06:05 — with GitHub Actions Waiting

Zhangxunmt approved these changes May 6, 2025

View reviewed changes

dhrubo-os reviewed May 6, 2025

View reviewed changes

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 18:40 — with GitHub Actions Error

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 18:40 — with GitHub Actions Failure

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 18:40 — with GitHub Actions Error

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 18:40 — with GitHub Actions Failure

rcho93 requested a review from dhrubo-os May 6, 2025 18:42

rcho93 added 11 commits May 6, 2025 15:33

Create prompt API & System Index for prompt

e48657b

Signed-off-by: seungwon cho <[email protected]>

spotless is applied & disabled wildcard import

8a3d340

Signed-off-by: seungwon cho <[email protected]>

javadoc added & header added & minor code errors fixed

969e2ae

Signed-off-by: seungwon cho <[email protected]>

Apply spotless

c87eaee

Signed-off-by: seungwon cho <[email protected]>

apply spotless

ba34b6b

Signed-off-by: seungwon cho <[email protected]>

addressed comments

e99e4f5

Signed-off-by: seungwon cho <[email protected]>

apply spotless

88c9c6d

Signed-off-by: seungwon cho <[email protected]>

addresses comments

ad07ac4

Signed-off-by: seungwon cho <[email protected]>

apply spotless

e1a9253

Signed-off-by: seungwon cho <[email protected]>

addressed comments

0f0974f

Signed-off-by: seungwon cho <[email protected]>

solve gradle build issue

e97d7bb

Signed-off-by: seungwon cho <[email protected]>

rcho93 force-pushed the feature/prompt-management branch from 2b5c24a to e97d7bb Compare May 6, 2025 22:52

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 22:53 — with GitHub Actions Failure

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 22:53 — with GitHub Actions Error

rcho93 had a problem deploying to ml-commons-cicd-env-require-approval May 6, 2025 22:53 — with GitHub Actions Failure


		private static final Version MINIMAL_SUPPORTED_VERSION_FOR_CLIENT_CONFIG = CommonValue.VERSION_2_13_0;

		public static final String DRY_RUN_PROMPT_NAME = "dryRunPrompt";

	throw new IllegalArgumentException("Prompt prompt field require system prompt");
	throw new IllegalArgumentException("MLPrompt prompt field requires {PROMPT_FIELD_SYSTEM_PROMPT} parameter");

Prompt Management Create API #3779

Are you sure you want to change the base?

Prompt Management Create API #3779

Conversation

rcho93 commented Apr 26, 2025 • edited Loading

Description

Related Issues

Check List

dhrubo-os commented Apr 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcho93 commented Apr 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mingshl Apr 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianf-aws left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianf-aws left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianf-aws commented May 6, 2025

rcho93 commented Apr 26, 2025 •

edited

Loading

mingshl Apr 26, 2025 •

edited

Loading