Skip to content

Replace JSON vector store with SQLite - #6438

Merged
MackinnonBuck merged 12 commits into
mainfrom
mbuck/replace-json-vector-store
May 20, 2025
Merged

Replace JSON vector store with SQLite#6438
MackinnonBuck merged 12 commits into
mainfrom
mbuck/replace-json-vector-store

Conversation

@MackinnonBuck

@MackinnonBuckMackinnonBuck commented May 13, 2025

Copy link
Copy Markdown
Member

This PR removes the JSON vector store and replaces it with a SQLite implementation using Microsoft.SemanticKernel.Connectors.SqliteVec.

Since Microsoft.SemanticKernel.Connectors.SqliteVec isn't shipping publicly yet, I've built Microsoft.SemanticKernel locally and included the generated .nupkgs directly in the repo. This change (41bd61e) will get reverted after the updated Microsoft.SemanticKernel packages ship.

Microsoft Reviewers: Open in CodeFlow

@MackinnonBuck
MackinnonBuck requested review from a team as code ownersMay 13, 2025 23:19
@github-actionsgithub-actionsBot added the area-ai-templates Microsoft.Extensions.AI.Templates label May 13, 2025
@stephentoub

stephentoub commented May 13, 2025

Copy link
Copy Markdown
Member

I've built Microsoft.SemanticKernel locally and included the generated .nupkgs directly in the repo. This change (41bd61e) will get reverted after the updated Microsoft.SemanticKernel packages ship.

This will add over a mb of binaries to the git history. Are we ok with that? Or did you mean you'll update the pr before it merges?

@jeffhandleyjeffhandley added the * NO MERGE * Do not merge this PR as long as this label is present. label May 14, 2025
@jeffhandley

Copy link
Copy Markdown
Member

I applied * NO MERGE *Do not merge this PR as long as this label is present. . We will wait to merge this until the package is published and we can reference it.

@jeffhandley
jeffhandley requested a review from rojiMay 14, 2025 07:07

@jeffhandleyjeffhandley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great to me, @MackinnonBuck. Nice work! Let's get a review from at least one of @roji or @SteveSandersonMS though, and wait for the NuGet packages to be available before merging.

@rojiroji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for pinging me, see below for a few comments.

Note that there will still be a few API changes for GA; some of my comments can already be addressed now, but others can't. So we can do another sync/iteration on this PR (hopefully by Friday everything will be in), and in general only merge after the GA nugets are fully available on nuget.org.

@dotnet-policy-servicedotnet-policy-serviceBot added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label May 14, 2025

@SteveSandersonMSSteveSandersonMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks superb - great job!

@dotnet-policy-servicedotnet-policy-serviceBot removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label May 14, 2025
@MackinnonBuck
MackinnonBuck requested a review from rojiMay 19, 2025 21:22

@jeffhandleyjeffhandley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just the patch version bum in my comments, and then once @roji approves we're good to go. Nice work, @MackinnonBuck!

Comment threadsrc/ProjectTemplates/GeneratedContent.targets Outdated

@rojiroji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great @MackinnonBuck! See the one long comment below about doing automatic embedding generation for upsert as well, not just for search. Other than that I think it's ready for merging.

MackinnonBuckand others added 2 commits May 20, 2025 09:07
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
@MackinnonBuck
MackinnonBuck requested a review from rojiMay 20, 2025 16:54

@rojiroji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great @MackinnonBuck, well done!

@MackinnonBuckMackinnonBuck removed the * NO MERGE * Do not merge this PR as long as this label is present. label May 20, 2025
@MackinnonBuck
MackinnonBuck merged commit 712d9aa into mainMay 20, 2025
@MackinnonBuck
MackinnonBuck deleted the mbuck/replace-json-vector-store branch May 20, 2025 18:17
joperezr added a commit to joperezr/extensions that referenced this pull request May 21, 2025
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 20, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-ai-templatesMicrosoft.Extensions.AI.Templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@MackinnonBuck@stephentoub@jeffhandley@SteveSandersonMS@roji@RussKie@westey-m