fix: remove chunk_id property from Chunk class

chunk_id in the Chunk class executes actual logic to compute a chunk ID. This sort of logic should not live in the API spec.

Instead, the providers should be in charge of calling generate_chunk_id, and pass it to `Chunk`.

this removes the incorrect dependency between Provider impl and API impl

Signed-off-by: Charlie Doern <cdoern@redhat.com>
This commit is contained in:
Charlie Doern 2025-10-28 20:51:13 -04:00
parent b90c6a2c8b
commit 1f05e2e8b1
38 changed files with 40679 additions and 135 deletions

View file

@ -41,6 +41,7 @@ class TestRagQuery:
interleaved_content = MagicMock()
chunk = Chunk(
content=interleaved_content,
chunk_id="chunk1",
metadata={
"key1": "value1",
"token_count": 10,
@ -48,7 +49,6 @@ class TestRagQuery:
# Note this is inserted into `metadata` during MemoryToolRuntimeImpl().insert()
"document_id": "doc1",
},
stored_chunk_id="chunk1",
chunk_metadata=chunk_metadata,
)
@ -101,8 +101,8 @@ class TestRagQuery:
)
chunk1 = Chunk(
content="chunk from db1",
chunk_id="c1",
metadata={"vector_store_id": "db1", "document_id": "doc1"},
stored_chunk_id="c1",
chunk_metadata=chunk_metadata1,
)
@ -114,8 +114,8 @@ class TestRagQuery:
)
chunk2 = Chunk(
content="chunk from db2",
chunk_id="c2",
metadata={"vector_store_id": "db2", "document_id": "doc2"},
stored_chunk_id="c2",
chunk_metadata=chunk_metadata2,
)

View file

@ -26,6 +26,7 @@ from llama_stack.providers.utils.memory.vector_store import (
content_from_doc,
make_overlapped_chunks,
)
from llama_stack.providers.utils.vector_io.vector_utils import generate_chunk_id
DUMMY_PDF_PATH = Path(os.path.abspath(__file__)).parent / "fixtures" / "dummy.pdf"
# Depending on the machine, this can get parsed a couple of ways
@ -53,6 +54,7 @@ class TestChunk:
def test_chunk(self):
chunk = Chunk(
content="Example chunk content",
chunk_id=generate_chunk_id("test-doc", "Example chunk content"),
metadata={"key": "value"},
embedding=[0.1, 0.2, 0.3],
)
@ -63,6 +65,7 @@ class TestChunk:
chunk_no_embedding = Chunk(
content="Example chunk content",
chunk_id=generate_chunk_id("test-doc", "Example chunk content"),
metadata={"key": "value"},
)
assert chunk_no_embedding.embedding is None
@ -218,8 +221,8 @@ class TestVectorStoreWithIndex:
)
chunks = [
Chunk(content="Test 1", embedding=None, metadata={}),
Chunk(content="Test 2", embedding=None, metadata={}),
Chunk(content="Test 1", chunk_id=generate_chunk_id("test-doc", "Test 1"), embedding=None, metadata={}),
Chunk(content="Test 2", chunk_id=generate_chunk_id("test-doc", "Test 2"), embedding=None, metadata={}),
]
mock_inference_api.openai_embeddings.return_value.data = [
@ -254,8 +257,18 @@ class TestVectorStoreWithIndex:
)
chunks = [
Chunk(content="Test 1", embedding=[0.1, 0.2, 0.3], metadata={}),
Chunk(content="Test 2", embedding=[0.4, 0.5, 0.6], metadata={}),
Chunk(
content="Test 1",
chunk_id=generate_chunk_id("test-doc", "Test 1"),
embedding=[0.1, 0.2, 0.3],
metadata={},
),
Chunk(
content="Test 2",
chunk_id=generate_chunk_id("test-doc", "Test 2"),
embedding=[0.4, 0.5, 0.6],
metadata={},
),
]
await vector_store_with_index.insert_chunks(chunks)
@ -279,25 +292,47 @@ class TestVectorStoreWithIndex:
# Verify Chunk raises ValueError for invalid embedding type
with pytest.raises(ValueError, match="Input should be a valid list"):
Chunk(content="Test 1", embedding="invalid_type", metadata={})
Chunk(
content="Test 1",
chunk_id=generate_chunk_id("test-doc", "Test 1"),
embedding="invalid_type",
metadata={},
)
# Verify Chunk raises ValueError for invalid embedding type in insert_chunks (i.e., Chunk errors before insert_chunks is called)
with pytest.raises(ValueError, match="Input should be a valid list"):
await vector_store_with_index.insert_chunks(
[
Chunk(content="Test 1", embedding=None, metadata={}),
Chunk(content="Test 2", embedding="invalid_type", metadata={}),
Chunk(
content="Test 1", chunk_id=generate_chunk_id("test-doc", "Test 1"), embedding=None, metadata={}
),
Chunk(
content="Test 2",
chunk_id=generate_chunk_id("test-doc", "Test 2"),
embedding="invalid_type",
metadata={},
),
]
)
# Verify Chunk raises ValueError for invalid embedding element type in insert_chunks (i.e., Chunk errors before insert_chunks is called)
with pytest.raises(ValueError, match=" Input should be a valid number, unable to parse string as a number "):
await vector_store_with_index.insert_chunks(
Chunk(content="Test 1", embedding=[0.1, "string", 0.3], metadata={})
Chunk(
content="Test 1",
chunk_id=generate_chunk_id("test-doc", "Test 1"),
embedding=[0.1, "string", 0.3],
metadata={},
)
)
chunks_wrong_dim = [
Chunk(content="Test 1", embedding=[0.1, 0.2, 0.3, 0.4], metadata={}),
Chunk(
content="Test 1",
chunk_id=generate_chunk_id("test-doc", "Test 1"),
embedding=[0.1, 0.2, 0.3, 0.4],
metadata={},
),
]
with pytest.raises(ValueError, match="has dimension 4, expected 3"):
await vector_store_with_index.insert_chunks(chunks_wrong_dim)
@ -317,9 +352,14 @@ class TestVectorStoreWithIndex:
)
chunks = [
Chunk(content="Test 1", embedding=None, metadata={}),
Chunk(content="Test 2", embedding=[0.2, 0.2, 0.2], metadata={}),
Chunk(content="Test 3", embedding=None, metadata={}),
Chunk(content="Test 1", chunk_id=generate_chunk_id("test-doc", "Test 1"), embedding=None, metadata={}),
Chunk(
content="Test 2",
chunk_id=generate_chunk_id("test-doc", "Test 2"),
embedding=[0.2, 0.2, 0.2],
metadata={},
),
Chunk(content="Test 3", chunk_id=generate_chunk_id("test-doc", "Test 3"), embedding=None, metadata={}),
]
mock_inference_api.openai_embeddings.return_value.data = [