Support pass prompt to CreateAlertTool #452

qianheng-aws · 2024-10-23T09:28:21Z

Description

This PR enhance the CreateAlertTool with 2 features:

support pass prompt as parameter to override the default prompt.
use CLAUDE as its default model type if model_type in parameters is empty or unsupported.

Related Issues

Resolves #447

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.

Signed-off-by: Heng Qian <qianheng@amazon.com>

src/main/java/org/opensearch/agent/tools/CreateAlertTool.java

Signed-off-by: Heng Qian <qianheng@amazon.com>

yuye-aws

Approved. Can @zane-neo review?

yuye-aws · 2024-11-05T03:35:06Z

src/main/java/org/opensearch/agent/tools/CreateAlertTool.java

+    public enum ModelType {
+        CLAUDE,
+        OPENAI;
+
+        public static ModelType from(String value) {
+            if (value.isEmpty()) {
+                return ModelType.CLAUDE;
+            }
+            try {
+                return ModelType.valueOf(value.toUpperCase(Locale.ROOT));
+            } catch (Exception e) {
+                log.error("Wrong Model type, should be CLAUDE or OPENAI");
+                return ModelType.CLAUDE;
+            }
+        }
+    }


Wait a bit. The code is the same as the ModelType in PPL tool. Can you extract it outside the function for code reusage?

They are not exactly the same, this tool doesn't support FINETUNE model. If we want to reuse this method, we'd better refactor it as well and I think it should also applies to other tools like CreateAnomalyDetectorTool.

So I don't think it's a blocker for this PR, we can do such refactor work in a separate PR.

I see. Then the code is good.

xinyual · 2024-11-12T05:11:50Z

src/main/java/org/opensearch/agent/tools/CreateAlertTool.java

-            );
+        modelType = String.valueOf(ModelType.from(modelType));
+        if (prompt.isEmpty()) {
+            if (!promptDict.containsKey(modelType)) {


The model type is enumerated, do we need to judge whether promptDict contains the model type?

This is the right line for identify whether promptDict contains that model. It will throw exception if not having.

yuye-aws

This PR looks good to me. Pinging @xinyual to review again.

yuye-aws · 2024-11-14T06:43:26Z

#457 @qianheng-aws This PR gets merged. Please update accordingly.

…lImprove

qianheng-aws · 2024-11-14T11:34:35Z

#457 @qianheng-aws This PR gets merged. Please update accordingly.

Should this PR be the same as that change? I think there is no difference functionally. We already support empty model_type and return a default one.

yuye-aws · 2024-11-15T07:48:04Z

#457 @qianheng-aws This PR gets merged. Please update accordingly.

Should this PR be the same as that change? I think there is no difference functionally. We already support empty model_type and return a default one.

Well. I mean you can make the following code snippet public. Then, both CreateAlertTool and CreateAnomalyDetectorTool can import it.

    public enum ModelType {
        CLAUDE,
        OPENAI;

        public static ModelType from(String value) {
            if (value.isEmpty()) {
                return ModelType.CLAUDE;
            }
            try {
                return ModelType.valueOf(value.toUpperCase(Locale.ROOT));
            } catch (Exception e) {
                log.error("Wrong Model type, should be CLAUDE or OPENAI");
                return ModelType.CLAUDE;
            }
        }
    }

qianheng-aws · 2024-11-19T03:17:11Z

#457 @qianheng-aws This PR gets merged. Please update accordingly.

Should this PR be the same as that change? I think there is no difference functionally. We already support empty model_type and return a default one.

Well. I mean you can make the following code snippet public. Then, both CreateAlertTool and CreateAnomalyDetectorTool can import it.
    public enum ModelType {
        CLAUDE,
        OPENAI;

        public static ModelType from(String value) {
            if (value.isEmpty()) {
                return ModelType.CLAUDE;
            }
            try {
                return ModelType.valueOf(value.toUpperCase(Locale.ROOT));
            } catch (Exception e) {
                log.error("Wrong Model type, should be CLAUDE or OPENAI");
                return ModelType.CLAUDE;
            }
        }
    }

I think that change should be made in the PR for refactoring code reuse. There is no help make it public for now.

xinyual · 2024-11-19T03:45:19Z

@qianheng-aws Seems CI/CD fails. Can you fix this?

…olImprove

Signed-off-by: Heng Qian <qianheng@amazon.com>

yuye-aws · 2024-11-21T06:56:16Z

@qianheng-aws Spotless apply plz.

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws · 2024-11-21T08:04:29Z

Only 1 flakey test failure, which is unrelated as well:

71 tests completed, 1 failed
Tests with failures:

org.opensearch.integTest.NeuralSparseSearchToolIT.testNeuralSparseSearchToolInFlowAgent_withNestedIndex

yuye-aws · 2024-11-21T10:31:09Z

Pinging @xinyual to review this PR.

yuye-aws · 2024-11-22T04:13:37Z

@xinyual Plz attach backport-2.x label to this PR.

* Support pass prompt to CreateAlertTool Signed-off-by: Heng Qian <qianheng@amazon.com> * Use Claude as ModelType if passed-in modelType is empty Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix UT Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix spotlessApply Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix spotlessApply Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> Co-authored-by: zane-neo <zaniu@amazon.com> (cherry picked from commit a95cfae) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Support pass prompt to CreateAlertTool * Use Claude as ModelType if passed-in modelType is empty * Fix UT * Fix spotlessApply * Fix spotlessApply --------- (cherry picked from commit a95cfae) Signed-off-by: Heng Qian <qianheng@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: zane-neo <zaniu@amazon.com>

Support pass prompt to CreateAlertTool

2ff4650

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws requested review from b4sjoo, dhrubo-os, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, xinyual, amitgalitz, jackiehanyang, owaiskazi19, ohltyler, joshpalis, dbwiddis, kaituo, lezzago, eirsep, sbcd90, zhichao-aws and joshuali925 as code owners October 23, 2024 09:28

yuye-aws reviewed Oct 29, 2024

View reviewed changes

src/main/java/org/opensearch/agent/tools/CreateAlertTool.java Outdated Show resolved Hide resolved

yuye-aws mentioned this pull request Oct 29, 2024

[FEATURE] Support other model types in CreateAlertTool and CreateAnomalyDetectorTool #447

Closed

Use Claude as ModelType if passed-in modelType is empty

bfe59b7

Signed-off-by: Heng Qian <qianheng@amazon.com>

yuye-aws approved these changes Nov 4, 2024

View reviewed changes

yuye-aws reviewed Nov 5, 2024

View reviewed changes

xinyual reviewed Nov 12, 2024

View reviewed changes

yuye-aws mentioned this pull request Nov 12, 2024

CreateAnomalyDetectorTool supports empty model_type #457

Merged

5 tasks

yuye-aws approved these changes Nov 14, 2024

View reviewed changes

Merge remote-tracking branch 'refs/remotes/origin/main' into alertToo…

e76e848

…lImprove

Merge branch 'main' into alertToolImprove

a1c34f2

zane-neo and others added 3 commits November 21, 2024 13:49

Merge branch 'main' into alertToolImprove

d08081d

Merge remote-tracking branch 'qianheng/alertToolImprove' into alertTo…

585e6d5

…olImprove

Fix UT

f779fd9

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws added 2 commits November 21, 2024 15:13

Fix spotlessApply

da90a27

Signed-off-by: Heng Qian <qianheng@amazon.com>

Fix spotlessApply

a49a868

Signed-off-by: Heng Qian <qianheng@amazon.com>

xinyual approved these changes Nov 22, 2024

View reviewed changes

yuye-aws approved these changes Nov 22, 2024

View reviewed changes

xinyual added the backport 2.x label Nov 22, 2024

xinyual merged commit a95cfae into opensearch-project:main Nov 22, 2024
8 of 9 checks passed

opensearch-trigger-bot bot mentioned this pull request Nov 22, 2024

[Backport 2.x] Support pass prompt to CreateAlertTool #469

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support pass prompt to CreateAlertTool #452

Support pass prompt to CreateAlertTool #452

qianheng-aws commented Oct 23, 2024 •

edited

Loading

yuye-aws left a comment

yuye-aws Nov 5, 2024

qianheng-aws Nov 12, 2024 •

edited

Loading

yuye-aws Nov 12, 2024

xinyual Nov 12, 2024

qianheng-aws Nov 12, 2024 •

edited

Loading

yuye-aws left a comment

yuye-aws commented Nov 14, 2024

qianheng-aws commented Nov 14, 2024 •

edited

Loading

yuye-aws commented Nov 15, 2024

qianheng-aws commented Nov 19, 2024

xinyual commented Nov 19, 2024

yuye-aws commented Nov 21, 2024

qianheng-aws commented Nov 21, 2024

yuye-aws commented Nov 21, 2024

yuye-aws commented Nov 22, 2024

Support pass prompt to CreateAlertTool #452

Support pass prompt to CreateAlertTool #452

Conversation

qianheng-aws commented Oct 23, 2024 • edited Loading

Description

Related Issues

Check List

yuye-aws left a comment

Choose a reason for hiding this comment

yuye-aws Nov 5, 2024

Choose a reason for hiding this comment

qianheng-aws Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

yuye-aws Nov 12, 2024

Choose a reason for hiding this comment

xinyual Nov 12, 2024

Choose a reason for hiding this comment

qianheng-aws Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

yuye-aws left a comment

Choose a reason for hiding this comment

yuye-aws commented Nov 14, 2024

qianheng-aws commented Nov 14, 2024 • edited Loading

yuye-aws commented Nov 15, 2024

qianheng-aws commented Nov 19, 2024

xinyual commented Nov 19, 2024

yuye-aws commented Nov 21, 2024

qianheng-aws commented Nov 21, 2024

yuye-aws commented Nov 21, 2024

yuye-aws commented Nov 22, 2024

qianheng-aws commented Oct 23, 2024 •

edited

Loading

qianheng-aws Nov 12, 2024 •

edited

Loading

qianheng-aws Nov 12, 2024 •

edited

Loading

qianheng-aws commented Nov 14, 2024 •

edited

Loading