chore!: Refactor embeddings out of VectorStoreWithIndex and into OpenAIVectorStoreMixin and make ChunkMetadata required. #4413

franciscojavierarceo · 2025-12-19T05:56:15Z

What does this PR do?

Move embedding generation responsibility from VectorStoreWithIndex.insert_chunks() to OpenAIVectorStoreMixin.openai_attach_file_to_vector_store() and make ChunkMetadata required.

Important call outs:

Add new EmbeddedChunk class that inherits from Chunk with embedding fields
Remove embedding fields from base Chunk and ChunkMetadata classes
Make ChunkMetadata required in Chunk class
Fix vector store file attachment error handling
Update all providers and tests to use EmbeddedChunk for vector operations

Test Plan

Updated tests

Resolves #2981

github-actions · 2025-12-19T05:56:57Z

✱ Stainless preview builds

This PR will update the llama-stack-client SDKs with the following commit message.

chore: Refactor embedding out of VectorStoreWithIndex and into OpenAI…

Edit this comment to update it. It will appear in the SDK's changelogs.

✅ llama-stack-client-node studio · code · diff

Your SDK built successfully.
generate ⚠️ → build ✅ → lint ✅ → test ✅
npm install https://pkg.stainless.com/s/llama-stack-client-node/95d5132cffa03bf650b3b048ae56555b6d61a7f9/dist.tar.gz

✅ llama-stack-client-kotlin studio · code · diff

Your SDK built successfully.
generate ⚠️ → lint ✅ → test ❗

⏳ llama-stack-client-python studio · code · diff

generate ⚠️ → build ⏳ → lint ⏳ → test ⏳

✅ llama-stack-client-go studio · code · diff

Your SDK built successfully.
generate ⚠️ → lint ❗ → test ❗
go get github.com/stainless-sdks/llama-stack-client-go@c4eed4b81913a556ac5b30b7c1900400cd88d49e

⏳ These are partial results; builds are still running.

This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push.
Last updated: 2025-12-25 03:16:05 UTC

cdoern

one question, otherwise lgtm!

cdoern · 2025-12-23T19:30:59Z

src/llama_stack/providers/utils/memory/openai_vector_store_mixin.py


+            # Get embedding model info from vector store metadata
+            store_info = self.openai_vector_stores[vector_store_id]
+            embedding_model = store_info["metadata"].get("embedding_model")


will store_info["metadata"] always be set? just want to check because we are calling .get on it unconditionally. I guess its ok because we are in a try except, but double checking :)

In release v0.2.13 (https://github.com/llamastack/llama-stack/releases/tag/v0.2.13) I added the metadata object but now I'm forcing it to be required so the edge case is for users that are migrating data from <0.2.13 and upgrading to 0.4.0.

I think it's okay for those users to reingest the data. Now that the metadata and child fields are required, we won't have that concern going forward.

src/llama_stack/providers/utils/memory/openai_vector_store_mixin.py

mattf · 2025-12-24T12:13:36Z

src/llama_stack/providers/utils/memory/openai_vector_store_mixin.py

+                    if isinstance(data.embedding, list):
+                        c.embedding = data.embedding
+                    else:
+                        raise ValueError(f"Expected embedding to be a list, got {type(data.embedding)}")


this looks like a 500 instead of a 400

did you see a non-list here? that'd be an api violation. remove the isinstance check and let it fail, which will result in a 500.

mattf · 2025-12-24T12:15:47Z

src/llama_stack_api/vector_io.py

+    embedding: list[float]
+    chunk_metadata: ChunkMetadata


does this api need to be changed as part of the refactor?

mattf · 2025-12-24T12:20:22Z

src/llama_stack/providers/utils/memory/vector_store.py

+            chunk_embedding_model=embedding_model,
+            chunk_embedding_dimension=embedding_dimension,


where are these used?

mattf

looking better!

for later -

turn OpenAIVectorStoreMixin into a BaseModel

mattf · 2025-12-25T12:14:36Z

src/llama_stack/providers/utils/memory/openai_vector_store_mixin.py

+                    if isinstance(data.embedding, list):
+                        c.embedding = data.embedding
+                    else:
+                        raise ValueError(f"Expected embedding to be a list, got {type(data.embedding)}")


did you see a non-list here? that'd be an api violation. remove the isinstance check and let it fail, which will result in a 500.

mergify · 2026-01-06T09:24:40Z

This pull request has merge conflicts that must be resolved before it can be merged. @franciscojavierarceo please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

…VectorStoreMixin Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

franciscojavierarceo · 2026-01-06T15:06:45Z

@mattf you mind reviewing? I'd like to get this into the 0.4 and backport it into the 0.3.x releases.

leseb · 2026-01-06T15:11:07Z

@mattf you mind reviewing? I'd like to get this into the 0.4 and backport it into the 0.3.x releases.

we won't backport in 0.3.x if it's a breaking change / feature.

franciscojavierarceo · 2026-01-06T15:19:44Z

@leseb oh yeah that makes sense.

leseb

Would like to land this so we can cut 0.4.0 today. I believe all @mattf comments have been addressed. Also, any small miss can be addressed in a 0.4.x release quickly. Thanks!

raghotham · 2026-01-06T16:59:04Z

tests/unit/core/routers/test_vector_stores_abac.py

+                            chunk_id="c1",
+                            created_timestamp=1234567890,
+                            updated_timestamp=1234567890,
+                            chunk_embedding_model="test-model",


ChunkMetadata no longer has embedding info?

nope, moving it to the EmbeddedChunk as Chunks will no longer have embeddings.

It's an annoying change but it makes things much cleaner and explicit about when Chunks have embeddings and don't.

should you then instantiate ChunkMetadata with these params? or am i missing the fact that this is a failure test?

I missed cleaning these up 🤦. Removing them in this PR: #4454

raghotham

looks good

Thanks for the feedback Matt, I think I incorporated the changes. Given you're OOO and we have enough approvals, we want to land this for 0.4. Let me know if you have other followups, happy to make them in subsequent PRs.

meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Dec 19, 2025

franciscojavierarceo changed the title ~~chore: Refactor embedding out of VectorStoreWithIndex and into OpenAI…~~ chore: Refactor embedding out of VectorStoreWithIndex and into OpenAIVectorStoreMixin Dec 19, 2025

franciscojavierarceo changed the title ~~chore: Refactor embedding out of VectorStoreWithIndex and into OpenAIVectorStoreMixin~~ chore!: Refactor embedding out of VectorStoreWithIndex and into OpenAIVectorStoreMixin Dec 19, 2025

franciscojavierarceo force-pushed the migrate-embedding-to-vector-store-mixin branch 2 times, most recently from cd40891 to d34448b Compare December 22, 2025 21:18

franciscojavierarceo changed the title ~~chore!: Refactor embedding out of VectorStoreWithIndex and into OpenAIVectorStoreMixin~~ chore!: Refactor embeddings out of VectorStoreWithIndex and into OpenAIVectorStoreMixin. Make ChunkMetadata required. Dec 22, 2025

franciscojavierarceo changed the title ~~chore!: Refactor embeddings out of VectorStoreWithIndex and into OpenAIVectorStoreMixin. Make ChunkMetadata required.~~ chore!: Refactor embeddings out of VectorStoreWithIndex and into OpenAIVectorStoreMixin and make ChunkMetadata required. Dec 22, 2025

franciscojavierarceo marked this pull request as ready for review December 22, 2025 21:34

franciscojavierarceo requested review from ashwinb, bbrowning, cdoern, ehhuang, leseb, mattf and raghotham as code owners December 22, 2025 21:34

cdoern reviewed Dec 23, 2025

View reviewed changes

cdoern approved these changes Dec 23, 2025

View reviewed changes

mattf requested changes Dec 24, 2025

View reviewed changes

mattf mentioned this pull request Dec 24, 2025

feat(api): add file_processor API skeleton #4113

Merged

mattf previously requested changes Dec 25, 2025

View reviewed changes

franciscojavierarceo force-pushed the migrate-embedding-to-vector-store-mixin branch 4 times, most recently from 61faef4 to 7dd3abd Compare January 3, 2026 05:27

varshaprasad96 approved these changes Jan 5, 2026

View reviewed changes

mergify bot added the needs-rebase label Jan 6, 2026

franciscojavierarceo added 2 commits January 6, 2026 09:30

chore: Refactor embedding out of VectorStoreWithIndex and into OpenAI…

28aa7d1

…VectorStoreMixin Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

fixed metadata and delete unused test

cea5319

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

franciscojavierarceo added 5 commits January 6, 2026 09:30

updated tests to use real embeddings rather than mock values

fda5490

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

make embeddings required and fix tests

fd613f8

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

moving inference validation to init and updating adapters

62d7ebb

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

updated tests

53fcc5e

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

make EmbeddedChunk inherit Chunk

2611343

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

franciscojavierarceo force-pushed the migrate-embedding-to-vector-store-mixin branch from 9597d7c to 2611343 Compare January 6, 2026 15:00

mergify bot removed the needs-rebase label Jan 6, 2026

leseb approved these changes Jan 6, 2026

View reviewed changes

raghotham reviewed Jan 6, 2026

View reviewed changes

raghotham approved these changes Jan 6, 2026

View reviewed changes

Merge branch 'main' into migrate-embedding-to-vector-store-mixin

a521e1b

franciscojavierarceo merged commit 6aacfef into llamastack:main Jan 6, 2026
35 checks passed

		chunk_embedding_model=embedding_model,
		chunk_embedding_dimension=embedding_dimension,

chore!: Refactor embeddings out of VectorStoreWithIndex and into OpenAIVectorStoreMixin and make ChunkMetadata required. #4413

chore!: Refactor embeddings out of VectorStoreWithIndex and into OpenAIVectorStoreMixin and make ChunkMetadata required. #4413

Uh oh!

Conversation

franciscojavierarceo commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Test Plan

Resolves #2981

Uh oh!

github-actions bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✱ Stainless preview builds

Uh oh!

cdoern left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Jan 6, 2026

Uh oh!

franciscojavierarceo commented Jan 6, 2026

Uh oh!

leseb commented Jan 6, 2026

Uh oh!

franciscojavierarceo commented Jan 6, 2026

Uh oh!

leseb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raghotham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

franciscojavierarceo commented Dec 19, 2025 •

edited

Loading

github-actions bot commented Dec 19, 2025 •

edited

Loading