Adding query planning tool search template validation and integration tests #4177

joshpalis · 2025-09-17T20:59:24Z

Description

Adds field validation for QueryPlanningTool field search_templates to require both template_id and template_description
Adds null check for StoredScript response, defaults to default search template if script response is null
Adds integration tests for Search template field

Additionally includes changes from #4176, will rebase once its 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.

mingshl · 2025-09-17T21:07:07Z

ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java

    private static final String DEFAULT_SYSTEM_PROMPT =
        "You are an OpenSearch Query DSL generation assistant, translating natural language questions to OpenSeach DSL Queries";
+
+    private static final Logger logger = LogManager.getLogger(QueryPlanningTool.class);


can you use the same logger like the rest of the repo?

ml-commons/common/src/main/java/org/opensearch/ml/common/utils/ToolUtils.java

Line 26 in 281c430

import lombok.extern.log4j.Log4j2;

Ill remove the logger, left over from debugging

mingshl · 2025-09-17T21:08:02Z

ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java

+        private void validateTemplateFields(Map<String, String> template) {
+            // Validate templateId
+            String templateId = template.get(TEMPLATE_ID_FIELD);
+            if (templateId == null || templateId.trim().isEmpty()) {


is it the same validation of search template or similar to opensearch core?

This validation is specifically for the query planning tool configuration, it ensures that each entry of the search_templates field contains both template_id and template_description

rithin-pullela-aws · 2025-09-17T21:12:02Z

ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java

+
+            // Validate templateDescription
+            String templateDescription = template.get(TEMPLATE_DESCRIPTION_FIELD);
+            if (templateDescription == null || templateDescription.trim().isEmpty()) {


nit: can we use .isBlank() instead of .trim().isEmpty()
isBalnk() is more efficient

rithin-pullela-aws · 2025-09-17T21:13:17Z

plugin/build.gradle

+    zipArchive("org.opensearch.plugin:opensearch-job-scheduler:${opensearch_build}")
+    zipArchive("org.opensearch.plugin:opensearch-knn:${opensearch_build}")
+    zipArchive("org.opensearch.plugin:neural-search:${opensearch_build}")


Can we remove these?

ah yeah, left over from debugging, Ill remove in the next commit

rithin-pullela-aws · 2025-09-17T21:13:51Z

plugin/build.gradle

+    plugin(provider(new Callable<RegularFile>(){
+            @Override
+            RegularFile call() throws Exception {
+                return new RegularFile() {
+                    @Override
+                    File getAsFile() {
+                        return configurations.zipArchive.asFileTree.matching{include "**/opensearch-knn-${opensearch_build}.zip"}.getSingleFile()
+                    }
+                }
+            }
+        }))
+    plugin(provider(new Callable<RegularFile>(){
+            @Override
+            RegularFile call() throws Exception {
+                return new RegularFile() {
+                    @Override
+                    File getAsFile() {
+                        return configurations.zipArchive.asFileTree.matching{include "**/neural-search-${opensearch_build}.zip"}.getSingleFile()
+                    }
+                }
+            }
+        }))


Can we remove these too

Why do we need to have these plugins in the integTest setup?

… tests Signed-off-by: Joshua Palis <jpalis@amazon.com>

rithin-pullela-aws

LGTM!

codecov · 2025-09-18T18:04:58Z

Codecov Report

❌ Patch coverage is 47.36842% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.00%. Comparing base (7bf4b54) to head (c32fbe4).

Files with missing lines	Patch %	Lines
.../opensearch/ml/engine/tools/QueryPlanningTool.java	47.36%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4177      +/-   ##
============================================
- Coverage     82.02%   82.00%   -0.03%     
+ Complexity     9195     9193       -2     
============================================
  Files           789      789              
  Lines         39541    39557      +16     
  Branches       4387     4390       +3     
============================================
+ Hits          32432    32437       +5     
- Misses         5219     5228       +9     
- Partials       1890     1892       +2

Flag	Coverage Δ
ml-commons	`82.00% <47.36%> (-0.03%)`	⬇️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Joshua Palis <jpalis@amazon.com>

mingshl · 2025-09-19T19:56:55Z

you need to meet the codecov for your if else condition

joshpalis requested review from b4sjoo, dhrubo-os, mingshl, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27, sam-herman, xinyual and pyek-bot as code owners September 17, 2025 20:59

joshpalis requested a deployment to ml-commons-cicd-env-require-approval September 17, 2025 21:00 — with GitHub Actions Waiting

joshpalis force-pushed the integ branch from 326e7ba to 251879a Compare September 17, 2025 21:00

joshpalis requested a deployment to ml-commons-cicd-env-require-approval September 17, 2025 21:02 — with GitHub Actions Waiting

mingshl reviewed Sep 17, 2025

View reviewed changes

joshpalis force-pushed the integ branch from 251879a to 5474d1b Compare September 17, 2025 21:12

joshpalis requested a deployment to ml-commons-cicd-env-require-approval September 17, 2025 21:13 — with GitHub Actions Waiting

rithin-pullela-aws reviewed Sep 17, 2025

View reviewed changes

Adding query planning tool search template validation and integration…

04c705d

… tests Signed-off-by: Joshua Palis <jpalis@amazon.com>

joshpalis force-pushed the integ branch from 353a221 to 04c705d Compare September 17, 2025 23:57

joshpalis temporarily deployed to ml-commons-cicd-env-require-approval September 17, 2025 23:58 — with GitHub Actions Inactive

rithin-pullela-aws approved these changes Sep 18, 2025

View reviewed changes

joshpalis requested a deployment to ml-commons-cicd-env-require-approval September 18, 2025 18:05 — with GitHub Actions Waiting

mingshl previously approved these changes Sep 18, 2025

View reviewed changes

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 18, 2025 23:21 — with GitHub Actions Error

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 18, 2025 23:21 — with GitHub Actions Failure

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 18, 2025 23:21 — with GitHub Actions Error

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 18, 2025 23:21 — with GitHub Actions Failure

ylwu-amzn previously approved these changes Sep 18, 2025

View reviewed changes

Merge branch 'main' into integ

e33813a

Signed-off-by: Joshua Palis <jpalis@amazon.com>

joshpalis dismissed stale reviews from ylwu-amzn and mingshl via e33813a September 19, 2025 00:42

joshpalis force-pushed the integ branch from 1e18173 to e33813a Compare September 19, 2025 00:42

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 19, 2025 00:43 — with GitHub Actions Error

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 19, 2025 00:43 — with GitHub Actions Failure

Merge branch 'main' into integ

c32fbe4

joshpalis deployed to ml-commons-cicd-env-require-approval September 19, 2025 17:32 — with GitHub Actions Active

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 19, 2025 17:32 — with GitHub Actions Error

joshpalis had a problem deploying to ml-commons-cicd-env-require-approval September 19, 2025 17:32 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding query planning tool search template validation and integration tests #4177

Adding query planning tool search template validation and integration tests #4177

joshpalis commented Sep 17, 2025

Uh oh!

mingshl Sep 17, 2025

Uh oh!

joshpalis Sep 17, 2025

Uh oh!

mingshl Sep 17, 2025

Uh oh!

joshpalis Sep 17, 2025

Uh oh!

rithin-pullela-aws Sep 17, 2025

Uh oh!

rithin-pullela-aws Sep 17, 2025

Uh oh!

joshpalis Sep 17, 2025

Uh oh!

rithin-pullela-aws Sep 17, 2025

Uh oh!

owaiskazi19 Sep 17, 2025

Uh oh!

joshpalis Sep 17, 2025

Uh oh!

rithin-pullela-aws left a comment

Uh oh!

codecov bot commented Sep 18, 2025 •

edited

Loading

Uh oh!

mingshl commented Sep 19, 2025

Uh oh!

Uh oh!

Adding query planning tool search template validation and integration tests #4177

Are you sure you want to change the base?

Adding query planning tool search template validation and integration tests #4177

Conversation

joshpalis commented Sep 17, 2025

Description

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rithin-pullela-aws left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mingshl commented Sep 19, 2025

Uh oh!

Uh oh!

codecov bot commented Sep 18, 2025 •

edited

Loading