fix(vector): clear dead-letter marker on delete, treat oversize as terminal
Addresses round-1 review on PR #920: - Delete path now clears the file's dead-letter marker after release_document_for_user (whose principal-based filter misses the user-agnostic, principal-less marker), preventing orphan-marker accumulation for dead-lettered-then-deleted files. - Oversize PDFs (rejected by the pre-parse size guard, no pipeline_tier stamped) are now treated as terminal regardless of failing_tier -- no tier can parse an oversize file -- so they dead-letter instead of falling to the legacy per-user mark on the inline path. - Gate the success-path clear on a non-empty etag (an etag-less file can never have a marker, mirroring is_dead_lettered's early return). - dead_letter.py: payload typed dict[str, Any] (CLAUDE.md). Tests: oversize-terminal dead-letter and delete-path marker clear. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
8c9339501e
commit
cd348b3233
@@ -30,6 +30,7 @@ lookup degrades to "process normally", a failed write is logged, not raised.
|
||||
import logging
|
||||
import time
|
||||
import uuid
|
||||
from typing import Any
|
||||
|
||||
from qdrant_client import models
|
||||
from qdrant_client.models import FieldCondition, Filter, MatchValue, PointStruct
|
||||
@@ -88,7 +89,7 @@ async def mark_dead_letter(
|
||||
settings = get_settings()
|
||||
dimension = get_embedding_service().get_dimension()
|
||||
|
||||
payload: dict = {
|
||||
payload: dict[str, Any] = {
|
||||
"doc_id": doc_id,
|
||||
"doc_type": doc_type,
|
||||
"is_placeholder": True,
|
||||
|
||||
@@ -626,6 +626,13 @@ async def process_document(
|
||||
await release_document_for_user(
|
||||
doc_task.doc_id, doc_task.doc_type, doc_task.user_id
|
||||
)
|
||||
# Drop any dead-letter marker for the file too: release only
|
||||
# removes it when the last reader leaves (its filter misses the
|
||||
# user-agnostic, principal-less marker), so without this a
|
||||
# dead-lettered-then-deleted file would leave an orphan marker
|
||||
# accumulating in Qdrant. Only files are ever dead-lettered.
|
||||
if doc_task.doc_type == "file":
|
||||
await clear_dead_letter(doc_task.doc_id, doc_task.doc_type)
|
||||
logger.info(
|
||||
"Deleted %s_%s for %s",
|
||||
doc_task.doc_type,
|
||||
@@ -1052,8 +1059,14 @@ async def _index_document(
|
||||
failing_tier = tier or result.metadata.get(
|
||||
"pipeline_tier", TIER_LADDER[0]
|
||||
)
|
||||
# An oversize PDF is rejected by the pre-parse size guard
|
||||
# before any tier runs (no pipeline_tier stamped on the inline
|
||||
# path) and no tier can ever parse it, so it is terminal
|
||||
# regardless of failing_tier. Otherwise, terminal == no higher
|
||||
# tier can run.
|
||||
terminal = (
|
||||
registry.next_available_tier(failing_tier, settings) is None
|
||||
reason == "oversize"
|
||||
or registry.next_available_tier(failing_tier, settings) is None
|
||||
)
|
||||
if terminal:
|
||||
# No higher tier can run (e.g. structured timed out with
|
||||
@@ -1594,9 +1607,9 @@ async def _index_document(
|
||||
# A successful (re-)index supersedes any prior terminal failure: clear a
|
||||
# stale dead-letter marker (e.g. the file was fixed/replaced, or a new
|
||||
# escalation tier finally parsed it) so it isn't left behind. Only files are
|
||||
# ever dead-lettered (the mark lives in the file branch), so skip the extra
|
||||
# Qdrant round-trip for the other doc types on the hot indexing path.
|
||||
if doc_task.doc_type == "file":
|
||||
# ever dead-lettered, and only with a non-empty etag (is_dead_lettered
|
||||
# early-returns without one), so skip the extra Qdrant round-trip otherwise.
|
||||
if doc_task.doc_type == "file" and doc_task.etag:
|
||||
await clear_dead_letter(doc_task.doc_id, doc_task.doc_type)
|
||||
|
||||
# Delete placeholder before writing real vectors
|
||||
|
||||
@@ -139,3 +139,54 @@ async def test_non_terminal_failure_keeps_legacy_mark(mocker):
|
||||
spies.update_ph.assert_awaited_once() # legacy per-user failed mark
|
||||
spies.mark.assert_not_awaited() # NOT dead-lettered (structured can still run)
|
||||
spies.dead_metric.assert_not_called()
|
||||
|
||||
|
||||
async def test_oversize_failure_dead_letters_regardless_of_tier(mocker):
|
||||
"""An oversize PDF is terminal at any tier (no tier can parse it) -> dead-letter
|
||||
even though a higher tier (structured) is nominally available above 'fast'."""
|
||||
spies = _patch_common(mocker, ocr_enabled=False)
|
||||
mocker.patch.object(
|
||||
processor,
|
||||
"_parse_pdf_tier",
|
||||
AsyncMock(
|
||||
return_value=ProcessingResult(
|
||||
text="",
|
||||
metadata={"parse_failed_reason": "oversize"},
|
||||
processor="size_guard",
|
||||
success=False,
|
||||
error="PDF exceeds size cap",
|
||||
)
|
||||
),
|
||||
)
|
||||
|
||||
result = await processor._index_document(
|
||||
_file_task(), _nc_client(), MagicMock(), tier="fast"
|
||||
)
|
||||
|
||||
assert result is False
|
||||
spies.mark.assert_awaited_once()
|
||||
assert spies.mark.await_args.args[4] == "oversize" # reason
|
||||
spies.dead_metric.assert_called_once_with("oversize")
|
||||
spies.update_ph.assert_not_awaited()
|
||||
|
||||
|
||||
async def test_delete_clears_dead_letter_marker(mocker):
|
||||
"""Deleting a file must also drop its dead-letter marker, else a
|
||||
dead-lettered-then-deleted file leaves an orphan accumulating in Qdrant
|
||||
(release_document_for_user's filter misses the user-agnostic marker)."""
|
||||
mocker.patch.object(processor, "get_qdrant_client", AsyncMock())
|
||||
mocker.patch.object(processor, "release_document_for_user", AsyncMock())
|
||||
clear = mocker.patch.object(processor, "clear_dead_letter", AsyncMock())
|
||||
|
||||
task = DocumentTask(
|
||||
user_id="Demo-User",
|
||||
doc_id="520189",
|
||||
doc_type="file",
|
||||
operation="delete",
|
||||
modified_at=0,
|
||||
file_path="/Plans/big.pdf",
|
||||
etag="etag-1",
|
||||
)
|
||||
await processor.process_document(task, MagicMock(), max_retries=1)
|
||||
|
||||
clear.assert_awaited_once_with("520189", "file")
|
||||
|
||||
Reference in New Issue
Block a user