Add default prompt to ppl tool #125

xinyual · 2024-01-12T07:24:53Z

Description

add default prompt to ppl tool

Issues Resolved

add default prompt to ppl tool. So customer don't need to add very low context prompt in their request body.

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.

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

zhichao-aws · 2024-01-12T07:33:19Z

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

@@ -373,4 +405,20 @@ private String parseOutput(String llmOutput, String indexName) {
        return ppl;
    }

+    private String loadDefaultPrompt(PPLModelType pplModelType) {


Now we read the resource every time when invoke creating PPLTool. It would be more efficient if we only read them once and store the prompts at static fields

Already move it to static.

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

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

codecov · 2024-01-12T07:41:11Z

Codecov Report

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

Comparison is base (c81746e) 80.82% compared to head (60f0c10) 80.83%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
.../main/java/org/opensearch/agent/tools/PPLTool.java	83.33%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #125      +/-   ##
============================================
+ Coverage     80.82%   80.83%   +0.01%     
- Complexity      193      195       +2     
============================================
  Files            13       13              
  Lines           975     1002      +27     
  Branches        130      133       +3     
============================================
+ Hits            788      810      +22     
- Misses          137      141       +4     
- Partials         50       51       +1

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

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

zane-neo · 2024-01-15T01:58:42Z

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

+        try {
+            defaultPromptDict = loadDefaultPromptDict();
+        } catch (IOException e) {
+            throw new RuntimeException("fail to load default prompt dict" + e.getMessage());


Is it possible that this would break the plugin initialization process with this error thrown?

You're right. Already change the logic to init the dict to an empty dict and log an error here.

zane-neo · 2024-01-15T02:05:40Z

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

-        this.contextPrompt = contextPrompt;
+        this.pplModelType = PPLModelType.from(pplModelType);
+        if (contextPrompt.isEmpty()) {
+            this.contextPrompt = this.defaultPromptDict.getOrDefault(this.pplModelType.toString(), "");


Is it possible this pplModelType.toString() produce NPE?

Since we init the modeltype with the default value, the model type should be an object or throw an error. So it shouldn't be a NPE.

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

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

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

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

xinyual · 2024-01-15T07:01:03Z

Force push since we decide not to add abstract prompt class this time.

zane-neo · 2024-01-16T02:58:24Z

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

+        FINETUNE;
+
+        public static PPLModelType from(String value) {
+            if (value.isEmpty()) {


Is value possibly be null?

No, when create ppltool object, we use (String) map.getOrDefault("model_type", "") So model type string will have default value.

* add default prompt for ppl tool Signed-off-by: xinyual <[email protected]> * fix Upper problem Signed-off-by: xinyual <[email protected]> * change wrong information Signed-off-by: xinyual <[email protected]> * remove uesless log Signed-off-by: xinyual <[email protected]> * add corresponding UTs Signed-off-by: xinyual <[email protected]> * apply spotless Signed-off-by: xinyual <[email protected]> * use locale instead Signed-off-by: xinyual <[email protected]> * move dict to static Signed-off-by: xinyual <[email protected]> * move dict to static Signed-off-by: xinyual <[email protected]> * replace throw error with error log Signed-off-by: xinyual <[email protected]> * add default value for PPL model type Signed-off-by: xinyual <[email protected]> * apply spotless Signed-off-by: xinyual <[email protected]> --------- Signed-off-by: xinyual <[email protected]> (cherry picked from commit 7e4c8d5) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add default prompt for ppl tool * fix Upper problem * change wrong information * remove uesless log * add corresponding UTs * apply spotless * use locale instead * move dict to static * move dict to static * replace throw error with error log * add default value for PPL model type * apply spotless --------- (cherry picked from commit 7e4c8d5) Signed-off-by: xinyual <[email protected]> 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>

…roject#129) * add default prompt for ppl tool * fix Upper problem * change wrong information * remove uesless log * add corresponding UTs * apply spotless * use locale instead * move dict to static * move dict to static * replace throw error with error log * add default value for PPL model type * apply spotless --------- (cherry picked from commit 7e4c8d5) Signed-off-by: xinyual <[email protected]> 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> Signed-off-by: yuye-aws <[email protected]>

xinyual added 5 commits January 12, 2024 15:11

add default prompt for ppl tool

3cf464c

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

fix Upper problem

c251ad3

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

change wrong information

9b89496

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

remove uesless log

d7bfdec

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

add corresponding UTs

3ea75ce

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

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

opensearch-trigger-bot bot added the backport 2.x label Jan 12, 2024

apply spotless

112a81c

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

zhichao-aws reviewed Jan 12, 2024

View reviewed changes

use locale instead

7caf914

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

xinyual added 2 commits January 12, 2024 16:16

move dict to static

820e53f

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

move dict to static

4e6caba

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

zane-neo reviewed Jan 15, 2024

View reviewed changes

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

replace throw error with error log

6f08725

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

zane-neo reviewed Jan 15, 2024

View reviewed changes

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

add default value for PPL model type

a8c673a

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

xinyual force-pushed the addDefaultPromptToPPLTool branch from cd2717b to a8c673a Compare January 15, 2024 06:56

apply spotless

60f0c10

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

zane-neo reviewed Jan 16, 2024

View reviewed changes

zane-neo approved these changes Jan 16, 2024

View reviewed changes

zane-neo merged commit 7e4c8d5 into opensearch-project:main Jan 16, 2024
13 checks passed

opensearch-trigger-bot bot mentioned this pull request Jan 16, 2024

[Backport 2.x] Add default prompt to ppl tool #129

Merged

zhichao-aws mentioned this pull request Jan 17, 2024

fix: add prompt field in RAGTool #130

Closed

5 tasks

xinyual mentioned this pull request Jan 18, 2024

[FEATURE] Extract output from previous Tool #137

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default prompt to ppl tool #125

Add default prompt to ppl tool #125

xinyual commented Jan 12, 2024

zhichao-aws Jan 12, 2024 •

edited

Loading

xinyual Jan 12, 2024

codecov bot commented Jan 12, 2024 •

edited

Loading

zane-neo Jan 15, 2024

xinyual Jan 15, 2024

zane-neo Jan 15, 2024

xinyual Jan 15, 2024

xinyual commented Jan 15, 2024

zane-neo Jan 16, 2024

xinyual Jan 16, 2024

Add default prompt to ppl tool #125

Add default prompt to ppl tool #125

Conversation

xinyual commented Jan 12, 2024

Description

Issues Resolved

Check List

zhichao-aws Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

xinyual Jan 12, 2024

Choose a reason for hiding this comment

codecov bot commented Jan 12, 2024 • edited Loading

Codecov Report

zane-neo Jan 15, 2024

Choose a reason for hiding this comment

xinyual Jan 15, 2024

Choose a reason for hiding this comment

zane-neo Jan 15, 2024

Choose a reason for hiding this comment

xinyual Jan 15, 2024

Choose a reason for hiding this comment

xinyual commented Jan 15, 2024

zane-neo Jan 16, 2024

Choose a reason for hiding this comment

xinyual Jan 16, 2024

Choose a reason for hiding this comment

zhichao-aws Jan 12, 2024 •

edited

Loading

codecov bot commented Jan 12, 2024 •

edited

Loading