fix: upsert document vectors before pruning stale chunks
DocumentSyncService._index_vectors ran delete_by_filter on the
document's existing chunks FIRST and only then embedded; if the
embedding pass failed (Ollama down) the Paperless document was left
with zero vectors until the next successful sync - the same
zero-vector hazard already fixed for wiki pages in
VectorService.update_from_page.
Chunk ids are now deterministic uuid5 (document_{id}_chunk_{i}) so
re-upserting overwrites in place; new points are upserted first, then
stale points (including legacy random-uuid4 ones) are pruned via
scroll + delete_by_ids, and only after a successful upsert.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbFZyDvYksazX6nYQYZ67L
This commit is contained in:
@@ -6,10 +6,13 @@ Pins the Phase D fixes:
|
||||
that never ran, so fresh tenants had no collection at upsert time)
|
||||
- None entries from embed_batch are filtered out instead of poisoning
|
||||
the whole batch upsert (one failed chunk aborted the document)
|
||||
- raw client.delete/client.upsert calls are routed through the async
|
||||
wrapper methods (delete_by_filter / upsert_points)
|
||||
- delete-LAST reindex order (same as the wiki fix): new points are
|
||||
upserted with deterministic uuid5 ids BEFORE stale points are pruned,
|
||||
so a failed embedding pass can no longer leave a document with zero
|
||||
vectors (the old order ran delete_by_filter first)
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
@@ -24,8 +27,9 @@ TENANT_COLLECTION = "library_desk_llm_tester"
|
||||
def mock_qdrant():
|
||||
qdrant = MagicMock()
|
||||
qdrant.ensure_collection = AsyncMock()
|
||||
qdrant.delete_by_filter = AsyncMock(return_value=0)
|
||||
qdrant.upsert_points = AsyncMock(side_effect=lambda collection_name, points: len(points))
|
||||
qdrant.scroll_all_points = AsyncMock(return_value=[])
|
||||
qdrant.delete_by_ids = AsyncMock(side_effect=lambda collection_name, point_ids: len(point_ids))
|
||||
return qdrant
|
||||
|
||||
|
||||
@@ -75,14 +79,46 @@ class TestIndexVectors:
|
||||
assert result.success is True
|
||||
mock_qdrant.ensure_collection.assert_awaited_once_with(TENANT_COLLECTION)
|
||||
|
||||
async def test_delete_routed_through_wrapper(self, sync_service, mock_qdrant):
|
||||
async def test_point_ids_are_deterministic(self, sync_service, mock_qdrant):
|
||||
await sync_service.index_document(
|
||||
document_id=7, user=TENANT, content="hello world", title="Doc"
|
||||
)
|
||||
|
||||
mock_qdrant.delete_by_filter.assert_awaited_once_with(
|
||||
points = mock_qdrant.upsert_points.await_args.kwargs["points"]
|
||||
expected = str(uuid.uuid5(uuid.NAMESPACE_DNS, "document_7_chunk_0"))
|
||||
assert points[0]["id"] == expected
|
||||
|
||||
async def test_stale_chunks_pruned_after_upsert(self, sync_service, mock_qdrant):
|
||||
"""Old points not in the new set are deleted AFTER the new upsert."""
|
||||
call_order = []
|
||||
mock_qdrant.upsert_points = AsyncMock(
|
||||
side_effect=lambda collection_name, points: (
|
||||
call_order.append("upsert"), len(points))[1]
|
||||
)
|
||||
stale_id = str(uuid.uuid4()) # legacy random-uuid4 point
|
||||
kept_id = str(uuid.uuid5(uuid.NAMESPACE_DNS, "document_7_chunk_0"))
|
||||
mock_qdrant.scroll_all_points = AsyncMock(
|
||||
return_value=[{"id": stale_id, "payload": {}},
|
||||
{"id": kept_id, "payload": {}}]
|
||||
)
|
||||
mock_qdrant.delete_by_ids = AsyncMock(
|
||||
side_effect=lambda collection_name, point_ids: (
|
||||
call_order.append("delete"), len(point_ids))[1]
|
||||
)
|
||||
|
||||
await sync_service.index_document(
|
||||
document_id=7, user=TENANT, content="hello world", title="Doc"
|
||||
)
|
||||
|
||||
assert call_order == ["upsert", "delete"]
|
||||
mock_qdrant.delete_by_ids.assert_awaited_once_with(
|
||||
collection_name=TENANT_COLLECTION,
|
||||
point_ids=[stale_id],
|
||||
)
|
||||
mock_qdrant.scroll_all_points.assert_awaited_once_with(
|
||||
collection_name=TENANT_COLLECTION,
|
||||
filter_conditions={"doc_type": "document", "paperless_id": 7},
|
||||
with_payload=False,
|
||||
)
|
||||
|
||||
async def test_upsert_routed_through_wrapper(self, sync_service, mock_qdrant):
|
||||
@@ -134,3 +170,7 @@ class TestIndexVectors:
|
||||
assert result.success is False
|
||||
assert "embed" in (result.error or "").lower()
|
||||
mock_qdrant.upsert_points.assert_not_awaited()
|
||||
# Delete-last: a fully failed embedding pass must leave the old
|
||||
# vectors untouched (previously delete_by_filter ran first, leaving
|
||||
# the document with zero vectors until the next successful sync)
|
||||
mock_qdrant.delete_by_ids.assert_not_awaited()
|
||||
|
||||
Reference in New Issue
Block a user