Add semantic kernel vector store approaches #106

milderhc · 2024-12-04T22:09:32Z

Purpose

Adding vector store functionality to the Ask and Chat sections. Using hybrid search + semantic ranker, query captions are not available as the semantic kernel maps the documents and returns them without that extra info.
Updated Azure AI Search SDK version

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

Get the code

git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install

Test the code

What to Check

Verify that the following are valid

...

Other Information

…openai-demo-java into sk-vector-stores

dantelmomsft · 2024-12-09T15:51:21Z

...nai/samples/rag/ask/approaches/semantickernel/JavaSemanticKernelWithVectorStoreApproach.java

+    String indexName;
+
+    @Value("${openai.chatgpt.deployment}")
+    private String gptChatDeploymentModelId;


not required. we can delete this

dantelmomsft · 2024-12-09T15:52:26Z

...nai/samples/rag/ask/approaches/semantickernel/JavaSemanticKernelWithVectorStoreApproach.java

+    private String gptChatDeploymentModelId;
+
+    @Value("${openai.embedding.deployment}")
+    private String embeddingDeploymentModelId;


not required. we can delete this

dantelmomsft · 2024-12-09T15:54:11Z

...amples/rag/chat/approaches/semantickernel/JavaSemanticKernelWithVectorStoreChatApproach.java

+    @Value("${cognitive.search.index}")
+    String indexName;
+    @Value("${openai.chatgpt.deployment}")
+    private String gptChatDeploymentModelId;


not required. we can delete this.

dantelmomsft · 2024-12-09T15:54:20Z

...amples/rag/chat/approaches/semantickernel/JavaSemanticKernelWithVectorStoreChatApproach.java

+    @Value("${openai.chatgpt.deployment}")
+    private String gptChatDeploymentModelId;
+    @Value("${openai.embedding.deployment}")
+    private String embeddingDeploymentModelId;


not required. we can delete this.

dantelmomsft · 2024-12-09T15:56:04Z

...amples/rag/chat/approaches/semantickernel/JavaSemanticKernelWithVectorStoreChatApproach.java

+        Kernel semanticKernel = buildSemanticKernel();
+
+        // STEP 1: Create Azure AI Search client
+        SearchIndexAsyncClient client = new SearchIndexClientBuilder()


Can we move this instance configuration in com.microsoft.openai.samples.rag.config.AzureAISearchConfiguration and let spring initialize it through constructor injection?

dantelmomsft · 2024-12-09T15:56:16Z

...nai/samples/rag/ask/approaches/semantickernel/JavaSemanticKernelWithVectorStoreApproach.java

+        Kernel semanticKernel = buildSemanticKernel(options);
+
+        // STEP 1: Create Azure AI Search client
+        SearchIndexAsyncClient client = new SearchIndexClientBuilder()


Can we move this instance configuration in com.microsoft.openai.samples.rag.config.AzureAISearchConfiguration and let spring initialize it through constructor injection?

dantelmomsft · 2024-12-09T15:58:57Z

...amples/rag/chat/approaches/semantickernel/JavaSemanticKernelWithVectorStoreChatApproach.java

+        );
+
+        // STEP 3: Retrieve relevant documents using keywords extracted from the chat history
+        String conversationString = ChatGPTUtils.formatAsChatML(questionOrConversation.toOpenAIChatMessages());


I think this is a duplication. let's use SK ChatHistory we have in conversation variable at line 72

dantelmomsft · 2024-12-09T16:04:19Z

.../microsoft/openai/samples/rag/retrieval/semantickernel/AzureAISearchVectorStoreApproach.java

+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class AzureAISearchVectorStoreApproach {


Can we change the class name with something which doesn't create confusion with JavaSemanticKernelWithVector..Approach.java name?.. Maybe something like AzureAISearchVectorStoreUtils ?

dantelmomsft · 2024-12-09T16:05:25Z

.../microsoft/openai/samples/rag/retrieval/semantickernel/AzureAISearchVectorStoreApproach.java

+
+    private static final String EMBEDDING_FIELD_NAME = "embedding";
+
+    public static class MemoryRecord {


what if we call this DocumentRecord ?

dantelmomsft · 2024-12-09T16:13:13Z

app/frontend/src/pages/chat/Chat.tsx

@milderhc what if we make the SK vector store (JavaSemanticKernelWithVectorStoreChatApproach) option the default one for semantic kernel? I think this will provide a better message about simplifying RAG implementation with semantic kernel compared to the one using SK chains. JavaSemanticKernelChainsChatApproach can still be there in repo for exploration purpose but not should not be addressable from the UI

milderhc added 3 commits December 4, 2024 13:58

Add semantic kernel vector store approaches

c57efea

Merge branch 'main' of https://github.com/Azure-Samples/azure-search-…

80a5d96

…openai-demo-java into sk-vector-stores

Update README.md

6d19745

milderhc requested a review from brunoborges as a code owner December 4, 2024 22:09

dantelmomsft reviewed Dec 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add semantic kernel vector store approaches #106

Add semantic kernel vector store approaches #106

milderhc commented Dec 4, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024

dantelmomsft Dec 9, 2024


		private static final String EMBEDDING_FIELD_NAME = "embedding";

		public static class MemoryRecord {

Add semantic kernel vector store approaches #106

Are you sure you want to change the base?

Add semantic kernel vector store approaches #106

Conversation

milderhc commented Dec 4, 2024

Purpose

Does this introduce a breaking change?

Pull Request Type

How to Test

What to Check

Other Information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment