Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guardrails for remote model input and output #2209

Merged
merged 8 commits into from
Mar 19, 2024

Conversation

jngz-es
Copy link
Collaborator

@jngz-es jngz-es commented Mar 17, 2024

Description

Add guardrails for remote model input and output. We support two ways stop words and regex.

Example:

POST /_plugins/_ml/models/LHkGS44BStH5-WXNbXxT/_predict
{
  "parameters": {
    "prompt": "\n\nHuman:this is a test of <stop words>\n\nnAssistant:"
  }
}

The response
{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "guardrails triggered for user input"
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "guardrails triggered for user input"
  },
  "status": 400
}

Issues Resolved

[List any issues this PR will resolve]

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.

jngz-es added 3 commits March 17, 2024 00:00
Signed-off-by: Jing Zhang <[email protected]>
Signed-off-by: Jing Zhang <[email protected]>
Signed-off-by: Jing Zhang <[email protected]>
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 18, 2024 22:06 — with GitHub Actions Failure
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 18, 2024 22:06 — with GitHub Actions Failure
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 18, 2024 22:06 — with GitHub Actions Error
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 18, 2024 22:07 — with GitHub Actions Error
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 18, 2024 22:07 — with GitHub Actions Error
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 18, 2024 22:07 — with GitHub Actions Failure
Copy link
Member

@lezzago lezzago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to create more of a guardrails framework here. Down the road we would add many more different guardrail mechanisms as the LLM space with guardrails is new and constantly evolving. We should think with that in mind.

Comment on lines +25 to +27
public class Guardrail implements ToXContentObject {
public static final String STOP_WORDS_FIELD = "stop_words";
public static final String REGEX_FIELD = "regex";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guardrail should be a super class and stop words is a sub class for a type of guardrail and same with regex being a sub class of type guardrail. This would be easier to integrate new types of guardrails and create a framework around it.

Comment on lines +97 to +120
public Boolean validateRegexList(String input, List<Pattern> regexPatterns) {
for (Pattern pattern : regexPatterns) {
if (!validateRegex(input, pattern)) {
return false;
}
}
return true;
}

public Boolean validateRegex(String input, Pattern pattern) {
Matcher matcher = pattern.matcher(input);
return !matcher.matches();
}

public Boolean validateStopWords(String input, Map<String, List<String>> stopWordsIndices) {
for (Map.Entry entry : stopWordsIndices.entrySet()) {
if (!validateStopWordsSingleIndex(input, (String) entry.getKey(), (List<String>) entry.getValue())) {
return false;
}
}
return true;
}

public Boolean validateStopWordsSingleIndex(String input, String indexName, List<String> fieldNames) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These function should be a part of that specific guardrail model class and we just call a validate function on the different guardrail component and that will determine how to correctly validate the input.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, will refactor it in next pr.

@lezzago
Copy link
Member

lezzago commented Mar 19, 2024

I think we need to create more of a guardrails framework here. Down the road we would add many more different guardrail mechanisms as the LLM space with guardrails is new and constantly evolving. We should think with that in mind.

I am fine with these changes happening post this PR

@jngz-es jngz-es merged commit 2d401bc into opensearch-project:main Mar 19, 2024
4 of 10 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 19, 2024
* guardrails

Signed-off-by: Jing Zhang <[email protected]>

* update guardrails

Signed-off-by: Jing Zhang <[email protected]>

* bug fix

Signed-off-by: Jing Zhang <[email protected]>

* add some UT

Signed-off-by: Jing Zhang <[email protected]>

* change stop words search to unblocking way

Signed-off-by: Jing Zhang <[email protected]>

* add more UT

Signed-off-by: Jing Zhang <[email protected]>

* address comments

Signed-off-by: Jing Zhang <[email protected]>

* add latch countdown when catching exception

Signed-off-by: Jing Zhang <[email protected]>

---------

Signed-off-by: Jing Zhang <[email protected]>
(cherry picked from commit 2d401bc)
jngz-es added a commit that referenced this pull request Mar 19, 2024
* guardrails

Signed-off-by: Jing Zhang <[email protected]>

* update guardrails

Signed-off-by: Jing Zhang <[email protected]>

* bug fix

Signed-off-by: Jing Zhang <[email protected]>

* add some UT

Signed-off-by: Jing Zhang <[email protected]>

* change stop words search to unblocking way

Signed-off-by: Jing Zhang <[email protected]>

* add more UT

Signed-off-by: Jing Zhang <[email protected]>

* address comments

Signed-off-by: Jing Zhang <[email protected]>

* add latch countdown when catching exception

Signed-off-by: Jing Zhang <[email protected]>

---------

Signed-off-by: Jing Zhang <[email protected]>
(cherry picked from commit 2d401bc)

Co-authored-by: Jing Zhang <[email protected]>
@@ -265,7 +265,10 @@ public class CommonValue {
+ MLModel.CONNECTOR_FIELD
+ "\": {" + ML_CONNECTOR_INDEX_FIELDS + " }\n},"
+ USER_FIELD_MAPPING
+ " }\n"
+ " },\n"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you added comma here.

searchSourceBuilder.parseXContent(queryParser);
searchSourceBuilder.size(1); //Only need 1 doc returned, if hit.
searchRequest = new SearchRequest().source(searchSourceBuilder).indices(indexName);
context.restore();
Copy link
Collaborator

@ylwu-amzn ylwu-amzn Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why restore context here? I see already have restore in action listener in line 147

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.of("query", Map.of("percolate", Map.of("field", "query", "document", documentMap)));
CountDownLatch latch = new CountDownLatch(1);

try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the indexName is not system index ? I think we should only stash context for system index.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants