fix(deriver): harden create_documents candidate hoist and test isolation
Skip empty embeddings on the external-store path, isolate per-document resolve failures, and keep replacement times_derived in the in-batch ledger. Patch get_external_vector_store in the hoist test and cover in-loop SQLAlchemyError abort.
This commit is contained in:
parent
f34f3e7f55
commit
8f6ba91710
|
|
@ -544,7 +544,7 @@ async def create_documents(
|
|||
|
||||
async def _resolve_candidates(index: int, doc: schemas.DocumentCreate) -> None:
|
||||
filters = _semantic_dup_filters(doc)
|
||||
if filters is None:
|
||||
if filters is None or not doc.embedding:
|
||||
return
|
||||
semantic_candidates[index] = await query_external_vector_document_ids(
|
||||
workspace_name=workspace_name,
|
||||
|
|
@ -557,7 +557,8 @@ async def create_documents(
|
|||
)
|
||||
|
||||
await asyncio.gather(
|
||||
*(_resolve_candidates(i, doc) for i, doc in enumerate(documents))
|
||||
*(_resolve_candidates(i, doc) for i, doc in enumerate(documents)),
|
||||
return_exceptions=True,
|
||||
)
|
||||
|
||||
# exact-content dedup (independent of `deduplicate`): pre-fetch
|
||||
|
|
@ -674,6 +675,7 @@ async def create_documents(
|
|||
existing_dup.id, existing_dup.times_derived
|
||||
)
|
||||
doc.times_derived = max(doc.times_derived, current_td + 1)
|
||||
pending_times_derived[existing_dup.id] = doc.times_derived
|
||||
row_ops.append(_DocumentRowOp("replace", existing_dup.id))
|
||||
semantic_dup_replaced_count += 1
|
||||
elif (
|
||||
|
|
@ -1312,7 +1314,7 @@ async def _semantic_dup_decision(
|
|||
filters=filters,
|
||||
max_distance=_SEMANTIC_DUP_MAX_DISTANCE,
|
||||
top_k=_SEMANTIC_DUP_TOP_K,
|
||||
embedding=doc.embedding,
|
||||
embedding=doc.embedding or None,
|
||||
)
|
||||
|
||||
if not similar_docs:
|
||||
|
|
|
|||
|
|
@ -1499,7 +1499,7 @@ class TestCreateDocumentsErrorHandling:
|
|||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_db_error_in_loop_aborts_batch(
|
||||
async def test_db_error_on_row_update_flush_aborts_batch(
|
||||
self,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[models.Workspace, models.Peer],
|
||||
|
|
@ -1558,6 +1558,60 @@ class TestCreateDocumentsErrorHandling:
|
|||
assert [d.content for d in docs] == ["existing fact"]
|
||||
assert docs[0].times_derived == 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_db_error_in_loop_aborts_batch(
|
||||
self,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[models.Workspace, models.Peer],
|
||||
):
|
||||
"""A DB error during per-document classification raises and commits nothing."""
|
||||
test_workspace, test_peer = sample_data
|
||||
test_peer2, test_session = await self._setup(
|
||||
db_session, test_workspace, test_peer
|
||||
)
|
||||
workspace_name = test_workspace.name
|
||||
observer = test_peer.name
|
||||
observed = test_peer2.name
|
||||
session_name = test_session.name
|
||||
|
||||
class FakePGError(Exception):
|
||||
sqlstate: str = "40P01"
|
||||
|
||||
deadlock = OperationalError("SELECT documents", {}, FakePGError())
|
||||
with (
|
||||
patch(
|
||||
"src.crud.document._semantic_dup_decision",
|
||||
AsyncMock(side_effect=deadlock),
|
||||
),
|
||||
pytest.raises(OperationalError),
|
||||
):
|
||||
await crud.create_documents(
|
||||
db_session,
|
||||
[
|
||||
self._doc("a brand new fact", session_name),
|
||||
self._doc("another new fact", session_name),
|
||||
],
|
||||
workspace_name=workspace_name,
|
||||
observer=observer,
|
||||
observed=observed,
|
||||
deduplicate=True,
|
||||
)
|
||||
|
||||
docs = (
|
||||
(
|
||||
await db_session.execute(
|
||||
select(models.Document).where(
|
||||
models.Document.workspace_name == workspace_name,
|
||||
models.Document.observer == observer,
|
||||
models.Document.observed == observed,
|
||||
)
|
||||
)
|
||||
)
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
assert docs == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_per_document_error_still_skips_only_that_document(
|
||||
self,
|
||||
|
|
@ -1671,6 +1725,10 @@ class TestExternalCandidateHoist:
|
|||
"src.crud.document.query_external_vector_document_ids",
|
||||
side_effect=fake_resolve,
|
||||
),
|
||||
patch(
|
||||
"src.crud.document.get_external_vector_store",
|
||||
return_value=None,
|
||||
),
|
||||
):
|
||||
result = await crud.create_documents(
|
||||
db_session,
|
||||
|
|
|
|||
Loading…
Reference in New Issue