feat(qdrant): add tenant-scoped point storage for ingestion
Why: - Ingested chunks need to become searchable Qdrant points before the upload response returns, with tenant/domain isolation and a safe re-ingestion story per ADR-0001/0017. Changes: - src/application/points/: index_chunks() is the sole entry point, owning payload construction, batched/bounded-concurrency upserts (upsert_concurrency semaphore), and a soft-delete sweep for points a shorter re-ingestion leaves behind. The sweep runs only after every upsert in the attempt succeeds, so a failed attempt can leave a stale prefix but never removes content from a working index. - PointStorage port (application/ports/) + QdrantPointStorage adapter (infrastructure/qdrant/points.py), keeping the qdrant_client SDK out of application code per ADR-0015. - FakePointStorage test double for exercising the ordering/idempotency guarantees without a real Qdrant.
This commit is contained in:
Ali Zarinkolah
@@ -1,10 +1,13 @@
|
||||
"""Hand-written fakes for narrow application-owned ports (ADR-0016)."""
|
||||
|
||||
import asyncio
|
||||
import uuid
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime
|
||||
|
||||
from src.application.ingestion.models import SparseVector
|
||||
from src.application.points.models import ChunkPoint
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -61,3 +64,75 @@ class FakeSparseEmbedder:
|
||||
raise RuntimeError("simulated embedder failure")
|
||||
return [SparseVector(indices=[], values=[]) for _ in texts]
|
||||
|
||||
|
||||
@dataclass
|
||||
class FakePointStorage:
|
||||
"""An in-memory `PointStorage`.
|
||||
|
||||
`points` is keyed by point id, so a re-upsert of the same deterministic id
|
||||
overwrites rather than accumulating — the property a test asserting "a
|
||||
retry produces no duplicate points" needs the fake to actually model.
|
||||
|
||||
`fail_on_batch` fails the Nth (0-based) upsert batch, which is how a test
|
||||
checks that the soft-delete sweep never runs after a partial failure.
|
||||
"""
|
||||
|
||||
points: dict[str, ChunkPoint] = field(default_factory=dict)
|
||||
upsert_batches: list[int] = field(default_factory=list)
|
||||
deactivate_calls: list[dict[str, object]] = field(default_factory=list)
|
||||
fail_on_batch: int | None = None
|
||||
fail_deactivate: bool = False
|
||||
max_in_flight: int = 0
|
||||
_in_flight: int = 0
|
||||
|
||||
async def upsert_points(self, points: Sequence[ChunkPoint]) -> None:
|
||||
self._in_flight += 1
|
||||
self.max_in_flight = max(self.max_in_flight, self._in_flight)
|
||||
try:
|
||||
# Yield so concurrent batches actually overlap; without this the
|
||||
# in-flight ceiling is trivially 1 and the bound goes untested.
|
||||
await asyncio.sleep(0)
|
||||
index = len(self.upsert_batches)
|
||||
self.upsert_batches.append(len(points))
|
||||
if self.fail_on_batch is not None and index == self.fail_on_batch:
|
||||
raise RuntimeError("simulated point storage failure")
|
||||
for point in points:
|
||||
self.points[str(point.point_id)] = point
|
||||
finally:
|
||||
self._in_flight -= 1
|
||||
|
||||
async def deactivate_points_from_index(
|
||||
self,
|
||||
*,
|
||||
tenant_id: uuid.UUID,
|
||||
file_id: uuid.UUID,
|
||||
from_chunk_index: int,
|
||||
deleted_at: datetime,
|
||||
updated_by: str,
|
||||
) -> int:
|
||||
self.deactivate_calls.append(
|
||||
{
|
||||
"tenant_id": tenant_id,
|
||||
"file_id": file_id,
|
||||
"from_chunk_index": from_chunk_index,
|
||||
"updated_by": updated_by,
|
||||
}
|
||||
)
|
||||
if self.fail_deactivate:
|
||||
raise RuntimeError("simulated deactivate failure")
|
||||
|
||||
def is_stale(point: ChunkPoint) -> bool:
|
||||
chunk_index = point.payload.get("chunk_index")
|
||||
return (
|
||||
point.payload.get("file_id") == str(file_id)
|
||||
and point.payload.get("tenant_id") == str(tenant_id)
|
||||
and point.payload.get("is_active") is True
|
||||
and isinstance(chunk_index, int)
|
||||
and chunk_index >= from_chunk_index
|
||||
)
|
||||
|
||||
stale = [point for point in self.points.values() if is_stale(point)]
|
||||
for point in stale:
|
||||
point.payload["is_active"] = False
|
||||
point.payload["deleted_at"] = deleted_at.isoformat()
|
||||
return len(stale)
|
||||
|
||||
182
tests/integration/qdrant/test_points.py
Normal file
182
tests/integration/qdrant/test_points.py
Normal file
@@ -0,0 +1,182 @@
|
||||
"""`QdrantPointStorage` against a real Qdrant (ADR-0001, ADR-0002).
|
||||
|
||||
Reads here go through the raw client rather than the port: `PointStorage` is
|
||||
deliberately write-only, because point reads are plan 002's `/v1/points`
|
||||
surface. The reads below are the test's own verification, not a preview of an
|
||||
API this slice ships.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import UTC, datetime
|
||||
|
||||
import pytest
|
||||
from qdrant_client import AsyncQdrantClient, models
|
||||
|
||||
from src.application.ingestion.chunking import chunk_id_for
|
||||
from src.application.ingestion.models import SparseVector
|
||||
from src.application.points.models import ChunkPoint
|
||||
from src.config import QdrantSettings
|
||||
from src.infrastructure.qdrant.collection import (
|
||||
DENSE_NOMIC_DIMENSIONS,
|
||||
DENSE_OPENAI_DIMENSIONS,
|
||||
ensure_chunks_collection,
|
||||
)
|
||||
from src.infrastructure.qdrant.points import QdrantPointStorage
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.qdrant,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
|
||||
def _point(tenant_id: uuid.UUID, file_id: uuid.UUID, chunk_index: int) -> ChunkPoint:
|
||||
return ChunkPoint(
|
||||
point_id=chunk_id_for(file_id, chunk_index),
|
||||
dense={
|
||||
"dense_nomic": [0.1] * DENSE_NOMIC_DIMENSIONS,
|
||||
"dense_openai": [0.2] * DENSE_OPENAI_DIMENSIONS,
|
||||
},
|
||||
sparse=SparseVector(indices=[1, 2], values=[0.5, 0.25]),
|
||||
payload={
|
||||
"tenant_id": str(tenant_id),
|
||||
"domain": "fire",
|
||||
"file_id": str(file_id),
|
||||
"chunk_id": str(chunk_id_for(file_id, chunk_index)),
|
||||
"chunk_index": chunk_index,
|
||||
"order_id": float(chunk_index + 1),
|
||||
"content": f"chunk {chunk_index}",
|
||||
"is_active": True,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
async def _storage(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> QdrantPointStorage:
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
return QdrantPointStorage(qdrant_client, collection=qdrant_settings.collection)
|
||||
|
||||
|
||||
async def _count_for_tenant(
|
||||
client: AsyncQdrantClient, collection: str, tenant_id: uuid.UUID
|
||||
) -> int:
|
||||
result = await client.count(
|
||||
collection_name=collection,
|
||||
count_filter=models.Filter(
|
||||
must=[
|
||||
models.FieldCondition(
|
||||
key="tenant_id", match=models.MatchValue(value=str(tenant_id))
|
||||
)
|
||||
]
|
||||
),
|
||||
exact=True,
|
||||
)
|
||||
return result.count
|
||||
|
||||
|
||||
async def test_upsert_points_stores_points_readable_under_the_owning_tenant_filter(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
|
||||
await storage.upsert_points([_point(tenant_id, file_id, i) for i in range(3)])
|
||||
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, tenant_id) == 3
|
||||
|
||||
|
||||
async def test_upsert_points_are_invisible_to_another_tenants_filter(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""The Qdrant-level form of "cross-tenant access finds nothing" (ADR-0002)."""
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
owner, other, file_id = uuid.uuid4(), uuid.uuid4(), uuid.uuid4()
|
||||
|
||||
await storage.upsert_points([_point(owner, file_id, i) for i in range(3)])
|
||||
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, other) == 0
|
||||
|
||||
|
||||
async def test_upsert_points_is_idempotent_for_deterministic_ids(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
points = [_point(tenant_id, file_id, i) for i in range(4)]
|
||||
|
||||
await storage.upsert_points(points)
|
||||
await storage.upsert_points(points)
|
||||
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, tenant_id) == 4
|
||||
|
||||
|
||||
async def test_deactivate_points_from_index_soft_deletes_only_the_tail(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
await storage.upsert_points([_point(tenant_id, file_id, i) for i in range(5)])
|
||||
|
||||
deactivated = await storage.deactivate_points_from_index(
|
||||
tenant_id=tenant_id,
|
||||
file_id=file_id,
|
||||
from_chunk_index=2,
|
||||
deleted_at=datetime.now(UTC),
|
||||
updated_by="api_key:test",
|
||||
)
|
||||
|
||||
assert deactivated == 3
|
||||
records, _ = await qdrant_client.scroll(
|
||||
collection_name=qdrant_settings.collection,
|
||||
scroll_filter=models.Filter(
|
||||
must=[models.FieldCondition(key="file_id", match=models.MatchValue(value=str(file_id)))]
|
||||
),
|
||||
limit=10,
|
||||
with_payload=True,
|
||||
)
|
||||
by_index = {
|
||||
record.payload["chunk_index"]: record.payload["is_active"]
|
||||
for record in records
|
||||
if record.payload is not None
|
||||
}
|
||||
assert by_index == {0: True, 1: True, 2: False, 3: False, 4: False}
|
||||
# Soft delete, not removal -- the points stay for audit (ADR-0002).
|
||||
assert await _count_for_tenant(qdrant_client, qdrant_settings.collection, tenant_id) == 5
|
||||
|
||||
|
||||
async def test_deactivate_points_from_index_does_not_touch_another_tenants_points(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
"""A file_id alone is never authority to mutate (ADR-0002)."""
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
owner, other, file_id = uuid.uuid4(), uuid.uuid4(), uuid.uuid4()
|
||||
await storage.upsert_points([_point(owner, file_id, i) for i in range(3)])
|
||||
|
||||
deactivated = await storage.deactivate_points_from_index(
|
||||
tenant_id=other,
|
||||
file_id=file_id,
|
||||
from_chunk_index=0,
|
||||
deleted_at=datetime.now(UTC),
|
||||
updated_by="api_key:intruder",
|
||||
)
|
||||
|
||||
assert deactivated == 0
|
||||
|
||||
|
||||
async def test_deactivate_points_from_index_returns_zero_when_nothing_is_stale(
|
||||
qdrant_client: AsyncQdrantClient, qdrant_settings: QdrantSettings
|
||||
) -> None:
|
||||
storage = await _storage(qdrant_client, qdrant_settings)
|
||||
tenant_id, file_id = uuid.uuid4(), uuid.uuid4()
|
||||
await storage.upsert_points([_point(tenant_id, file_id, i) for i in range(3)])
|
||||
|
||||
deactivated = await storage.deactivate_points_from_index(
|
||||
tenant_id=tenant_id,
|
||||
file_id=file_id,
|
||||
from_chunk_index=3,
|
||||
deleted_at=datetime.now(UTC),
|
||||
updated_by="api_key:test",
|
||||
)
|
||||
|
||||
assert deactivated == 0
|
||||
0
tests/unit/application/points/__init__.py
Normal file
0
tests/unit/application/points/__init__.py
Normal file
227
tests/unit/application/points/test_indexing.py
Normal file
227
tests/unit/application/points/test_indexing.py
Normal file
@@ -0,0 +1,227 @@
|
||||
"""`index_chunks`: payload correctness, bounded batching, and the ordering
|
||||
that keeps a failed attempt from damaging a working index (ADR-0001, ADR-0017).
|
||||
"""
|
||||
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
from anyio import CapacityLimiter
|
||||
|
||||
from src.application.ingestion.chunking import chunk_id_for
|
||||
from src.application.ingestion.errors import PointIndexingError
|
||||
from src.application.ingestion.models import Chunk, ContentType, EmbeddedChunk, SparseVector
|
||||
from src.application.points import index_chunks
|
||||
from src.config import QdrantSettings
|
||||
from tests.fakes import FakeDenseEmbedder, FakePointStorage, FakeSparseEmbedder
|
||||
|
||||
pytestmark = [pytest.mark.unit, pytest.mark.asyncio]
|
||||
|
||||
_TENANT_ID = uuid.UUID("11111111-1111-1111-1111-111111111111")
|
||||
_FILE_ID = uuid.UUID("22222222-2222-2222-2222-222222222222")
|
||||
_API_KEY_ID = uuid.UUID("33333333-3333-3333-3333-333333333333")
|
||||
_ACTOR = f"api_key:{_API_KEY_ID}"
|
||||
|
||||
|
||||
def _embedded(count: int) -> list[EmbeddedChunk]:
|
||||
chunks = [
|
||||
Chunk(
|
||||
chunk_id=chunk_id_for(_FILE_ID, index),
|
||||
chunk_index=index,
|
||||
order_id=float(index + 1),
|
||||
content=f"chunk {index}",
|
||||
content_type=ContentType.PARAGRAPH,
|
||||
token_count=2,
|
||||
character_count=7,
|
||||
)
|
||||
for index in range(count)
|
||||
]
|
||||
for position, chunk in enumerate(chunks):
|
||||
if position > 0:
|
||||
chunk.previous_chunk_id = chunks[position - 1].chunk_id
|
||||
if position < len(chunks) - 1:
|
||||
chunk.next_chunk_id = chunks[position + 1].chunk_id
|
||||
return [
|
||||
EmbeddedChunk(
|
||||
chunk=chunk,
|
||||
dense={"dense_nomic": [0.0] * 4, "dense_openai": [1.0] * 4},
|
||||
sparse=SparseVector(indices=[7], values=[0.5]),
|
||||
)
|
||||
for chunk in chunks
|
||||
]
|
||||
|
||||
|
||||
async def _index(
|
||||
storage: FakePointStorage,
|
||||
*,
|
||||
count: int = 3,
|
||||
settings: QdrantSettings | None = None,
|
||||
domain: str = "fire",
|
||||
):
|
||||
return await index_chunks(
|
||||
_embedded(count),
|
||||
storage=storage,
|
||||
tenant_id=_TENANT_ID,
|
||||
domain=domain,
|
||||
file_id=_FILE_ID,
|
||||
source_filename="policy.docx",
|
||||
source_type="docx",
|
||||
actor=_ACTOR,
|
||||
dense_embedders=[
|
||||
FakeDenseEmbedder(name="dense_nomic", model_version="nomic-embed-text-v2-moe"),
|
||||
FakeDenseEmbedder(name="dense_openai", model_version="text-embedding-3-large"),
|
||||
],
|
||||
sparse_embedder=FakeSparseEmbedder(model_version="bm25-fa_norm_stop"),
|
||||
settings=settings or QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
)
|
||||
|
||||
|
||||
async def test_index_chunks_writes_every_adr_0001_payload_field() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=3)
|
||||
|
||||
payload = storage.points[str(chunk_id_for(_FILE_ID, 1))].payload
|
||||
assert payload["tenant_id"] == str(_TENANT_ID)
|
||||
assert payload["domain"] == "fire"
|
||||
assert payload["file_id"] == str(_FILE_ID)
|
||||
assert payload["chunk_id"] == str(chunk_id_for(_FILE_ID, 1))
|
||||
assert payload["content"] == "chunk 1"
|
||||
assert payload["content_type"] == "paragraph"
|
||||
assert payload["source_filename"] == "policy.docx"
|
||||
assert payload["source_type"] == "docx"
|
||||
assert payload["order_id"] == 2.0
|
||||
assert payload["chunk_index"] == 1
|
||||
assert payload["previous_chunk_id"] == str(chunk_id_for(_FILE_ID, 0))
|
||||
assert payload["next_chunk_id"] == str(chunk_id_for(_FILE_ID, 2))
|
||||
assert payload["is_active"] is True
|
||||
assert payload["deleted_at"] is None
|
||||
assert payload["created_by"] == _ACTOR
|
||||
assert payload["updated_by"] == _ACTOR
|
||||
assert payload["version"] == 1
|
||||
assert payload["created_at"] == payload["updated_at"]
|
||||
assert isinstance(payload["content_hash"], str)
|
||||
# Sorted, so wiring order cannot change the value (ADR-0001).
|
||||
assert payload["embedding_model_version"] == (
|
||||
"bm25-fa_norm_stop+nomic-embed-text-v2-moe+text-embedding-3-large"
|
||||
)
|
||||
|
||||
|
||||
async def test_index_chunks_uses_null_neighbours_at_the_file_ends() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=3)
|
||||
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 0))].payload["previous_chunk_id"] is None
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 2))].payload["next_chunk_id"] is None
|
||||
|
||||
|
||||
async def test_index_chunks_derives_tenant_and_domain_from_the_caller_not_the_chunk() -> None:
|
||||
"""Tenant identity is server-derived; nothing in the chunk can assert it."""
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=1, domain="car")
|
||||
|
||||
payload = storage.points[str(chunk_id_for(_FILE_ID, 0))].payload
|
||||
assert payload["tenant_id"] == str(_TENANT_ID)
|
||||
assert payload["domain"] == "car"
|
||||
|
||||
|
||||
async def test_index_chunks_uses_deterministic_point_ids() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
result = await _index(storage, count=4)
|
||||
|
||||
assert result.points_upserted == 4
|
||||
assert set(storage.points) == {str(chunk_id_for(_FILE_ID, i)) for i in range(4)}
|
||||
|
||||
|
||||
async def test_index_chunks_repeated_run_produces_no_duplicate_points() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
await _index(storage, count=4)
|
||||
await _index(storage, count=4)
|
||||
|
||||
assert len(storage.points) == 4
|
||||
|
||||
|
||||
async def test_index_chunks_batches_at_the_configured_size() -> None:
|
||||
storage = FakePointStorage()
|
||||
settings = QdrantSettings(upsert_batch_size=2, upsert_concurrency=4)
|
||||
|
||||
await _index(storage, count=5, settings=settings)
|
||||
|
||||
assert storage.upsert_batches == [2, 2, 1]
|
||||
|
||||
|
||||
async def test_index_chunks_bounds_in_flight_batches() -> None:
|
||||
storage = FakePointStorage()
|
||||
settings = QdrantSettings(upsert_batch_size=1, upsert_concurrency=2)
|
||||
|
||||
await _index(storage, count=8, settings=settings)
|
||||
|
||||
assert len(storage.upsert_batches) == 8
|
||||
assert storage.max_in_flight <= 2
|
||||
|
||||
|
||||
async def test_index_chunks_soft_deletes_only_points_past_the_new_chunk_count() -> None:
|
||||
storage = FakePointStorage()
|
||||
await _index(storage, count=5)
|
||||
|
||||
result = await _index(storage, count=2)
|
||||
|
||||
assert result.points_soft_deleted == 3
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 1))].payload["is_active"] is True
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 2))].payload["is_active"] is False
|
||||
assert storage.points[str(chunk_id_for(_FILE_ID, 4))].payload["is_active"] is False
|
||||
|
||||
|
||||
async def test_index_chunks_does_not_soft_delete_when_an_upsert_batch_fails() -> None:
|
||||
"""A failed attempt must never remove content from a working index."""
|
||||
storage = FakePointStorage()
|
||||
await _index(storage, count=5)
|
||||
storage.deactivate_calls.clear()
|
||||
storage.fail_on_batch = 1
|
||||
|
||||
with pytest.raises(PointIndexingError):
|
||||
await _index(storage, count=2, settings=QdrantSettings(upsert_batch_size=1))
|
||||
|
||||
assert storage.deactivate_calls == []
|
||||
assert all(point.payload["is_active"] is True for point in storage.points.values())
|
||||
|
||||
|
||||
async def test_index_chunks_raises_point_indexing_error_when_a_batch_fails() -> None:
|
||||
storage = FakePointStorage(fail_on_batch=0)
|
||||
|
||||
with pytest.raises(PointIndexingError, match="upserting"):
|
||||
await _index(storage, count=2)
|
||||
|
||||
|
||||
async def test_index_chunks_raises_point_indexing_error_when_the_sweep_fails() -> None:
|
||||
storage = FakePointStorage(fail_deactivate=True)
|
||||
|
||||
with pytest.raises(PointIndexingError, match="soft-deleting"):
|
||||
await _index(storage, count=2)
|
||||
|
||||
|
||||
async def test_index_chunks_on_empty_input_touches_no_storage() -> None:
|
||||
storage = FakePointStorage()
|
||||
|
||||
result = await index_chunks(
|
||||
[],
|
||||
storage=storage,
|
||||
tenant_id=_TENANT_ID,
|
||||
domain="fire",
|
||||
file_id=_FILE_ID,
|
||||
source_filename="empty.csv",
|
||||
source_type="csv",
|
||||
actor=_ACTOR,
|
||||
dense_embedders=[FakeDenseEmbedder(name="dense_nomic")],
|
||||
sparse_embedder=FakeSparseEmbedder(),
|
||||
settings=QdrantSettings(),
|
||||
thread_limiter=CapacityLimiter(2),
|
||||
)
|
||||
|
||||
assert result.points_upserted == 0
|
||||
assert storage.upsert_batches == []
|
||||
assert storage.deactivate_calls == []
|
||||
Reference in New Issue
Block a user