Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Replace JSON vector store with SQLite#6438

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
MackinnonBuck merged 12 commits intomainfrommbuck/replace-json-vector-store
May 20, 2025

Conversation

@MackinnonBuck
Copy link
Member

@MackinnonBuckMackinnonBuck commentedMay 13, 2025•
edited
Loading

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

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

Microsoft Reviewers:Open in CodeFlow

@stephentoub
Copy link
Member

stephentoub commentedMay 13, 2025•
edited
Loading

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?

roji reacted with thumbs up emoji

@jeffhandleyjeffhandley added the * NO MERGE *Do not merge this PR as long as this label is present. labelMay 14, 2025
@jeffhandley
Copy link
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.

MackinnonBuck reacted with thumbs up emoji

@jeffhandleyjeffhandley requested a review fromrojiMay 14, 2025 07:07
Copy link
Member

@jeffhandleyjeffhandley left a comment

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.

Copy link
Member

@rojiroji left a comment

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.

jeffhandley reacted with heart emoji
@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. labelMay 14, 2025
Copy link
Member

@SteveSandersonMSSteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Looks superb - great job!

roji reacted with thumbs up emoji
@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. labelMay 14, 2025
@MackinnonBuckMackinnonBuck requested a review fromrojiMay 19, 2025 21:22
Copy link
Member

@jeffhandleyjeffhandley left a comment

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!

MackinnonBuck reacted with heart emoji
Copy link
Member

@rojiroji left a comment

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 added2 commitsMay 20, 2025 09:07
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
@MackinnonBuckMackinnonBuck requested a review fromrojiMay 20, 2025 16:54
Copy link
Member

@rojiroji left a comment

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!

MackinnonBuck reacted with heart emoji
@MackinnonBuckMackinnonBuck removed the * NO MERGE *Do not merge this PR as long as this label is present. labelMay 20, 2025
@MackinnonBuckMackinnonBuck merged commit712d9aa intomainMay 20, 2025
6 checks passed
@MackinnonBuckMackinnonBuck deleted the mbuck/replace-json-vector-store branchMay 20, 2025 18:17
joperezr added a commit to joperezr/extensions that referenced this pull requestMay 21, 2025
Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJun 20, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@SteveSandersonMSSteveSandersonMSSteveSandersonMS approved these changes

@jeffhandleyjeffhandleyjeffhandley approved these changes

@rojirojiroji approved these changes

+2 more reviewers

@westey-mwestey-mwestey-m left review comments

@RussKieRussKieRussKie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@MackinnonBuckMackinnonBuck

Labels

area-ai-templatesMicrosoft.Extensions.AI.Templates

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@MackinnonBuck@stephentoub@jeffhandley@SteveSandersonMS@roji@RussKie@westey-m
[8]ページ先頭
©2009-2025 Movatter.jp