opensource/dify
2
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-09-09 05:41:00 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(vdb-weaviate): backward-compatible cleanup + preserve positional alignment (#41915)

Browse Source 
Co-authored-by: Taranum01 <taranumwasu@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
Taranum Wasu 2026-09-08 01:55:08 +00:00 committed by GitHub
parent eeff2c01b1
commit 216180c7fd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 261 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
api/providers/vdb/vdb-weaviate/src/dify_vdb_weaviate/weaviate_vector.py
View File

@ -278,18 +278,38 @@ class WeaviateVector(BaseVector):
logger.warning("Could not add property %s: %s", prop.name, e)
@override
def _get_uuids(self, documents: list[Document]) -> list[str]:
def _get_uuids(self, texts: list[Document]) -> list[str]:
"""
Generates deterministic UUIDs for documents based on their content.
Generate canonical Weaviate object ids for each document.
Uses UUID5 with URL namespace to ensure consistent IDs for identical content.
Issue #41714: insert and the cleanup path
(``batch_clean_document_task`` → ``index_processor.clean`` →
``vector.delete_by_ids``) must agree on the object id, otherwise
``delete_by_id`` silently no-ops and the cleanup task reports
success while the objects stay behind as orphans.
Use the same source the rest of the VDB stack uses
(``VectorBase._get_uuids``): the ``doc_id`` stored in each
document's metadata. ``doc_id`` is the segment's
``index_node_id`` that ``delete_by_ids`` already passes in.
Preserve the parent caller's positional alignment: a missing
``doc_id`` is replaced with a freshly generated ``uuid4`` rather
than dropping the slot, so the parallel ``objs`` list in
``add_texts`` keeps matching the input documents one-for-one.
"""
URL_NAMESPACE = _uuid.UUID("6ba7b811-9dad-11d1-80b4-00c04fd430c8")
uuids = []
for doc in documents:
uuid_val = _uuid.uuid5(URL_NAMESPACE, doc.page_content)
uuids.append(str(uuid_val))
uuids: list[str] = []
for doc in texts:
doc_id = (doc.metadata or {}).get("doc_id")
if isinstance(doc_id, str) and doc_id:
uuids.append(doc_id)
else:
# Fallback only when the caller didn't supply a doc_id.
# Random uuid4 keeps the object uniquely addressable; the
# cleanup path can't find it without doc_id, but that's
# only acceptable for tests / fixtures that don't exercise
# cleanup.
uuids.append(str(_uuid.uuid4()))
return uuids
@ -380,17 +400,40 @@ class WeaviateVector(BaseVector):
Deletes objects by their UUID identifiers.
Silently ignores 404 errors for non-existent IDs.
Issue #41714: cleanup task (and any other caller) passes
``index_node_id`` (a UUID) here. New objects inserted after this
fix use the segment's ``index_node_id`` as their Weaviate UUID, so
the call lands cleanly. **Pre-existing** objects written by the
old UUID5 path still don't match — fall through to a
backward-compatible ``doc_id``-metadata filter so they are
cleaned up too, and a legacy object whose Weaviate UUID differs
from its ``doc_id`` is still reaped.
"""
if not self._client.collections.exists(self._collection_name):
return
col = self._client.collections.use(self._collection_name)
# 1. Best-effort direct delete by UUID for the fresh-write path.
# 404s mean the object was never written (or was written under
# a different UUID), which is expected for legacy rows.
for uid in ids:
try:
col.data.delete_by_id(uid)
except UnexpectedStatusCodeError as e:
if getattr(e, "status_code", None) != 404:
if e.status_code != 404:
raise
# 2. Backward-compatible catch-up: the legacy UUID5 path (or any
# other source) wrote rows whose Weaviate UUID no longer
# matches ``index_node_id``. Reap them by the ``doc_id``
# metadata so a delete pass never leaves orphans behind.
if ids:
try:
col.data.delete_many(where=Filter.by_property("doc_id").contains_any(ids))
except UnexpectedStatusCodeError as e:
if e.status_code != 404:
raise
@override

208
api/providers/vdb/vdb-weaviate/tests/unit_tests/test_doc_id_cleanup_41714.py Normal file
View File

@ -0,0 +1,208 @@
"""Regression tests for issue #41714.
The pre-fix code generated Weaviate object ids via
``uuid5(URL_NAMESPACE, page_content)`` (UUID v5 from content). The
cleanup path (``batch_clean_document_task`` -> ``index_processor.clean``
-> ``vector.delete_by_ids``) passes the segment's ``index_node_id``
(random UUID v4 from the database), so the two never agreed and
``delete_by_id`` silently no-op'd on every cleanup pass.
The fix:
1. ``_get_uuids`` now uses the segment's ``doc_id`` (== ``index_node_id``)
from each document's metadata. New objects line up with the
cleanup path one-for-one.
2. Documents without a ``doc_id`` keep a stable slot via a freshly
generated ``uuid4`` so the parallel ``objs`` list in ``add_texts``
never goes out of alignment with the input documents.
3. ``delete_by_ids`` keeps the best-effort direct-UUID delete (the
fresh-write path), then falls through to a ``doc_id``-metadata
``contains_any`` filter so the legacy UUID5 objects are reaped
too -- backwards-compatible with the rows already in production.
"""
from unittest.mock import MagicMock, patch
from dify_vdb_weaviate import weaviate_vector as weaviate_vector_module
from dify_vdb_weaviate.weaviate_vector import WeaviateVector
from core.rag.models.document import Document
class _FakeUnexpectedStatusCodeError(Exception):
"""Stand-in for ``weaviate.exceptions.UnexpectedStatusCodeError``.
The real class has a read-only ``status_code`` property; the
production code only reads ``.status_code`` off the exception, so
a plain attribute is enough. We patch the module-level reference
in the production module so the ``except`` clause catches ours.
"""
def __init__(self, status_code: int) -> None:
super().__init__(f"status={status_code}")
self.status_code = status_code
def _make_vector() -> tuple[WeaviateVector, MagicMock]:
"""Build a ``WeaviateVector`` without invoking ``__init__`` so we
never reach ``_init_client`` (which would try to connect).
Only ``_client`` and ``_collection_name`` are touched by the
production code paths exercised here (``_get_uuids`` and
``delete_by_ids``), so the rest of the instance dict is left bare.
Returns the ``WeaviateVector`` together with its mocked client so
tests can reach ``client.collections.use(...).data`` (the same
path the production code walks) without pyrefly tripping over
chained ``.return_value`` access through the dynamic ``_client``
instance attribute.
"""
# pyrefly cannot resolve chained ``.return_value`` access through
# dynamically-attached attributes (it infers ``v._client`` as the
# missing-on-class attribute and falls through to the stdlib
# ``collections`` module), so keep the mocked client in a local
# variable whose type pyrefly can see.
client: MagicMock = MagicMock()
client.collections.exists.return_value = True
client.collections.use.return_value = client.data
v = WeaviateVector.__new__(WeaviateVector)
v._collection_name = "Test_Collection"
v._client = client
return v, client
def _collection(client: MagicMock) -> MagicMock:
"""Resolve ``client.collections.use().data`` as a typed MagicMock.
Centralised so the same ``client.data`` instance is reused -- the
production code reaches the Weaviate collection's ``data`` object
via ``v._client.collections.use(name).data``; we mirror that path
so ``delete_by_id`` / ``delete_many`` side effects wired on the
returned mock are observed by the production calls.
"""
col: MagicMock = client.collections.use.return_value
return col.data
class TestGetUuidsUsesDocId:
"""_get_uuids must return one id per input document, positionally aligned."""
def test_doc_id_passes_through(self) -> None:
v, _client = _make_vector()
docs = [Document(page_content="x", metadata={"doc_id": "doc-aaa-1"})]
uuids = v._get_uuids(docs)
assert uuids == ["doc-aaa-1"]
def test_missing_doc_id_falls_back_to_fresh_uuid4(self) -> None:
"""A document without ``doc_id`` still needs a slot in the
returned list so the parallel ``objs`` list in ``add_texts``
never goes out of alignment with the input documents.
"""
v, _client = _make_vector()
docs = [
Document(page_content="x", metadata={"doc_id": "doc-aaa-1"}),
Document(page_content="y", metadata={}),
Document(page_content="z", metadata={"doc_id": "doc-ccc-3"}),
]
uuids = v._get_uuids(docs)
assert uuids[0] == "doc-aaa-1"
assert uuids[2] == "doc-ccc-3"
# The middle slot is filled, not dropped.
assert len(uuids) == 3
assert uuids[1] != ""
# And the fallback is a valid UUID4.
import uuid as _uuid
parsed = _uuid.UUID(uuids[1])
assert parsed.version == 4
def test_all_doc_ids_present(self) -> None:
v, _client = _make_vector()
docs = [
Document(page_content="x", metadata={"doc_id": "a"}),
Document(page_content="y", metadata={"doc_id": "b"}),
Document(page_content="z", metadata={"doc_id": "c"}),
]
assert v._get_uuids(docs) == ["a", "b", "c"]
class TestDeleteByIdsBackwardCompatible:
"""#41714 backwards-compatibility: legacy UUID5 objects whose Weaviate
UUID no longer matches ``index_node_id`` are reaped via the
``doc_id``-metadata filter."""
def test_legacy_object_with_matching_doc_id_is_reaped(self) -> None:
"""A legacy object whose ``doc_id`` matches the supplied
``index_node_id`` must be reaped even though its Weaviate UUID
no longer matches.
"""
v, client = _make_vector()
col = _collection(client)
col.delete_by_id.side_effect = _FakeUnexpectedStatusCodeError(404)
with patch.object(weaviate_vector_module, "UnexpectedStatusCodeError", _FakeUnexpectedStatusCodeError):
v.delete_by_ids(["doc-aaa-1"])
# Best-effort direct delete ran for each id.
assert col.delete_by_id.call_count == 1
# And the metadata catch-up filter ran with the same ids.
col.delete_many.assert_called_once()
kwargs = col.delete_many.call_args.kwargs
where = kwargs["where"]
# Filter.by_property("doc_id").contains_any(["doc-aaa-1"])
assert "doc_id" in str(where)
assert "doc-aaa-1" in str(where)
def test_fresh_object_is_reaped_by_direct_delete(self) -> None:
"""A fresh object whose Weaviate UUID equals the supplied
``index_node_id`` is reaped by the direct delete, and the
metadata-filter pass is a redundant no-op (no rows match).
"""
v, client = _make_vector()
col = _collection(client)
v.delete_by_ids(["doc-aaa-1"])
col.delete_by_id.assert_called_once_with("doc-aaa-1")
# The metadata filter always runs, but it's a no-op when the row
# is already gone (Weaviate's ``delete_many`` returns an empty
# result set against the cleared UUID).
col.delete_many.assert_called_once()
where = col.delete_many.call_args.kwargs["where"]
assert "doc_id" in str(where)
assert "doc-aaa-1" in str(where)
def test_non_404_error_propagates(self) -> None:
"""A non-404 (e.g. 500) error on direct delete must propagate,
not be swallowed as if it were a 404.
"""
import pytest
v, client = _make_vector()
col = _collection(client)
col.delete_by_id.side_effect = _FakeUnexpectedStatusCodeError(500)
with (
patch.object(weaviate_vector_module, "UnexpectedStatusCodeError", _FakeUnexpectedStatusCodeError),
pytest.raises(_FakeUnexpectedStatusCodeError),
):
v.delete_by_ids(["doc-aaa-1"])
def test_delete_many_404_is_swallowed(self) -> None:
"""The metadata-filter 404 (column doesn't exist) is also
swallowed -- the schema may not have been migrated yet.
"""
v, client = _make_vector()
col = _collection(client)
col.delete_by_id.side_effect = _FakeUnexpectedStatusCodeError(404)
col.delete_many.side_effect = _FakeUnexpectedStatusCodeError(404)
# Neither path raises.
with patch.object(weaviate_vector_module, "UnexpectedStatusCodeError", _FakeUnexpectedStatusCodeError):
v.delete_by_ids(["doc-aaa-1"])