-
Notifications
You must be signed in to change notification settings - Fork 16
feat: [Orchestration] Convenience for grounding metadata params #659
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||
| import javax.annotation.Nonnull; | ||||||||||||||||||||||||||||
| import javax.annotation.Nullable; | ||||||||||||||||||||||||||||
| import lombok.Setter; | ||||||||||||||||||||||||||||
| import lombok.experimental.Accessors; | ||||||||||||||||||||||||||||
| import lombok.val; | ||||||||||||||||||||||||||||
|
|
@@ -31,6 +32,8 @@ public class Grounding implements GroundingProvider { | |||||||||||||||||||||||||||
| private List<GroundingModuleConfigConfigFiltersInner> filters = | ||||||||||||||||||||||||||||
| List.of(DocumentGroundingFilter.create().dataRepositoryType(DataRepositoryType.VECTOR)); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @Nullable private List<String> metadataParams = null; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @Setter(onMethod_ = {@Nonnull}) | ||||||||||||||||||||||||||||
| private TypeEnum documentGroundingService = TypeEnum.DOCUMENT_GROUNDING_SERVICE; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
@@ -60,6 +63,19 @@ public Grounding filters(@Nonnull final GroundingModuleConfigConfigFiltersInner. | |||||||||||||||||||||||||||
| return this; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Set which metadataParams are used in the grounding response. | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * @param metadataParams List of metadataParams to set. | ||||||||||||||||||||||||||||
| * @return The modified grounding configuration. | ||||||||||||||||||||||||||||
| * @since 1.13.0 | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| @Nonnull | ||||||||||||||||||||||||||||
| public Grounding metadataParams(@Nonnull final String... metadataParams) { | ||||||||||||||||||||||||||||
| this.metadataParams = List.of(metadataParams); | ||||||||||||||||||||||||||||
| return this; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Create a prompt with grounding parameters included in the message. | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
|
|
@@ -86,8 +102,10 @@ public GroundingModuleConfig createConfig() { | |||||||||||||||||||||||||||
| GroundingModuleConfigConfigPlaceholders.create() | ||||||||||||||||||||||||||||
| .input(List.of("userMessage")) | ||||||||||||||||||||||||||||
| .output("groundingContext")) | ||||||||||||||||||||||||||||
| .filters(filters); | ||||||||||||||||||||||||||||
| .filters(filters) | ||||||||||||||||||||||||||||
| .metadataParams(metadataParams); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // metadata_params field is not allowed for data repository type: `help.sap.com` | ||||||||||||||||||||||||||||
|
Comment on lines
+105
to
+108
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (minor) This looks a bit counter intuitive. Instead of setting
Suggested change
Then, you can also set the default value of field
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach unfortunately does not work since if you do not set metadata params explicitly, the generated code will use an empty list which already leads to an error form grounding service when using help.sap.com. I changed the default value of the field to |
||||||||||||||||||||||||||||
| if (filters.contains( | ||||||||||||||||||||||||||||
| DocumentGroundingFilter.create().dataRepositoryType(DataRepositoryType.HELP_SAP_COM))) { | ||||||||||||||||||||||||||||
| groundingConfigConfig.setMetadataParams(null); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -268,6 +268,28 @@ void testGrounding() throws IOException { | |
| postRequestedFor(urlPathEqualTo("/v2/completion")).withRequestBody(equalToJson(request))); | ||
| } | ||
|
|
||
| @Test | ||
| void testGroundingWithConvenience() throws IOException { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this test since the only grounding unit test I saw was only testing low-level code (not our convenience). |
||
| stubFor( | ||
| post(urlPathEqualTo("/v2/completion")) | ||
| .willReturn( | ||
| aResponse() | ||
| .withBodyFile("groundingResponse.json") | ||
| .withHeader("Content-Type", "application/json"))); | ||
|
|
||
| var dbFilters = DocumentGroundingFilter.create().dataRepositoryType(DataRepositoryType.VECTOR); | ||
|
|
||
| var groundingConfig = Grounding.create().metadataParams("foo", "bar").filters(dbFilters); | ||
| var prompt = groundingConfig.createGroundingPrompt("Hello, what do you know?"); | ||
| var configWithGrounding = config.withGrounding(groundingConfig); | ||
|
|
||
| final var response = client.chatCompletion(prompt, configWithGrounding); | ||
|
|
||
| final String request = fileLoaderStr.apply("groundingRequestMetadata.json"); | ||
| verify( | ||
| postRequestedFor(urlPathEqualTo("/v2/completion")).withRequestBody(equalToJson(request))); | ||
| } | ||
|
|
||
| @Test | ||
| void testGroundingWithHelpSapCom() throws IOException { | ||
| stubFor( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| { | ||
| "config" : { | ||
| "modules" : { | ||
| "prompt_templating" : { | ||
| "prompt" : { | ||
| "template" : [ { | ||
| "content" : "{{?userMessage}} Use the following information as additional context: {{?groundingContext}}", | ||
| "role" : "user" | ||
| } ], | ||
| "defaults" : { }, | ||
| "tools" : [ ] | ||
| }, | ||
| "model" : { | ||
| "name" : "gpt-4o", | ||
| "version" : "latest", | ||
| "params" : { | ||
| "max_tokens" : 50, | ||
| "temperature" : 0.1, | ||
| "frequency_penalty" : 0, | ||
| "presence_penalty" : 0, | ||
| "top_p" : 1, | ||
| "n" : 1 | ||
| }, | ||
| "timeout" : 600, | ||
| "max_retries" : 2 | ||
| } | ||
| }, | ||
| "grounding" : { | ||
| "type" : "document_grounding_service", | ||
| "config" : { | ||
| "filters" : [ { | ||
| "data_repositories" : [ "*" ], | ||
| "data_repository_type" : "vector", | ||
| "data_repository_metadata" : [ ], | ||
| "document_metadata" : [ ], | ||
| "chunk_metadata" : [ ] | ||
| } ], | ||
| "placeholders" : { | ||
| "input" : [ "userMessage" ], | ||
| "output" : "groundingContext" | ||
| }, | ||
| "metadata_params" : [ "foo", "bar" ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "placeholder_values" : { | ||
| "userMessage" : "Hello, what do you know?" | ||
| }, | ||
| "messages_history" : [ ] | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.