fix(vector): guard dead-letter on etag, harden marker filter

Addresses round-2 review on PR #920:
- Only dead-letter a terminal failure when the file has an etag to
  content-address the marker; without one, fall back to the legacy per-user
  placeholder mark (an etagless marker is unmatchable). + test.
- _dead_letter_filter now also matches is_placeholder=True (redundant with
  dead_letter=True but lets Qdrant use the is_placeholder payload index).
- TODO(deck-349) documenting the dead-lettered-then-deleted orphan-marker leak
  (out of scope; needs a marker sweep or TTL field) per reviewer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

nextcloud_mcp_server/vector/dead_letter.py

@@ -25,6 +25,16 @@ filter, plus ``dead_letter=True`` so the orphan-placeholder sweep and the scanne
 can tell it apart from a volatile in-flight placeholder. Mirrors the fail-safe
 philosophy of ``sharing_state``: a Qdrant error never aborts ingest — a failed
 lookup degrades to "process normally", a failed write is logged, not raised.
+
+TODO(deck-349): a marker for a file that is dead-lettered and *then* deleted from
+Nextcloud can be orphaned. The processor's delete path clears it, but the
+scanner's grace-period deletion tracking only sees a file via its real indexed
+points (filtered by ``user_id``); a dead-lettered file has only this
+user-agnostic marker, so its disappearance enqueues no delete task and the marker
+is never reached. Not a correctness issue (search excludes ``is_placeholder=True``
+and the etag check means a stale marker never blocks new content), but it
+accumulates. A dedicated marker sweep or a TTL payload field would close it —
+tracked as a follow-up, not done here.
 """
 
 import logging
@@ -57,11 +67,17 @@ def _generate_dead_letter_id(doc_type: str, doc_id: str) -> str:
 
 
 def _dead_letter_filter(doc_id: str, doc_type: str) -> Filter:
-    """Match the dead-letter marker for one document (tenant-wide, no user_id)."""
+    """Match the dead-letter marker for one document (tenant-wide, no user_id).
+
+    Includes ``is_placeholder=True`` (redundant with ``dead_letter=True``, which
+    nothing else sets) so Qdrant can lean on the existing ``is_placeholder``
+    payload index.
+    """
     return Filter(
         must=[
             FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
             FieldCondition(key="doc_type", match=MatchValue(value=doc_type)),
+            FieldCondition(key="is_placeholder", match=MatchValue(value=True)),
             FieldCondition(key=DEAD_LETTER_KEY, match=MatchValue(value=True)),
         ]
     )

nextcloud_mcp_server/vector/processor.py

@@ -1068,7 +1068,7 @@ async def _index_document(
                         reason == "oversize"
                         or registry.next_available_tier(failing_tier, settings) is None
                     )
-                    if terminal:
+                    if terminal and doc_task.etag:
                         # No higher tier can run (e.g. structured timed out with
                         # OCR off), so retrying just re-burns the same failing
                         # parse. Dead-letter the document tenant-wide
@@ -1079,11 +1079,13 @@ async def _index_document(
                         # ping-pong the per-user "failed" mark could not: a file
                         # shared by N users has ONE user-agnostic placeholder
                         # whose user_id is overwritten by the last scanner, so
-                        # every other user re-queued it forever.
+                        # every other user re-queued it forever. Requires an etag
+                        # to content-address the marker; without one (rare) we
+                        # fall back to the legacy per-user mark below.
                         await mark_dead_letter(
                             doc_task.doc_id,
                             doc_task.doc_type,
-                            doc_task.etag or "",
+                            doc_task.etag,
                             escalation_tiers_signature(settings),
                             reason,
                             file_path=file_path,
@@ -1112,10 +1114,10 @@ async def _index_document(
                                 exc_info=True,
                             )
                     else:
-                        # A higher tier exists; parse failures don't escalate to
+                        # Either a higher tier exists (parse failures don't
-                        # it today, so keep the legacy per-user "failed"
+                        # escalate to it today) or there's no etag to
-                        # placeholder mark (not dead-lettered -- a future change
+                        # content-address a dead-letter marker. Keep the legacy
-                        # may route the failure to that tier).
+                        # per-user "failed" placeholder mark.
                         logger.warning(
                             "Permanent parse failure for %s (reason=%s); marking "
                             "failed and skipping index",

tests/unit/vector/test_dead_letter.py

@@ -151,6 +151,7 @@ class TestIsDeadLettered:
         assert conds == {
             "doc_id": "520189",
             "doc_type": "file",
+            "is_placeholder": True,
             dl.DEAD_LETTER_KEY: True,
         }
         assert "user_id" not in conds

tests/unit/vector/test_processor_dead_letter.py

@@ -170,6 +170,38 @@ async def test_oversize_failure_dead_letters_regardless_of_tier(mocker):
     spies.update_ph.assert_not_awaited()
 
 
+async def test_terminal_failure_without_etag_uses_legacy_mark(mocker):
+    """A terminal failure with no etag can't be content-addressed, so fall back to
+    the legacy per-user placeholder mark instead of writing an unmatchable marker."""
+    spies = _patch_common(mocker, ocr_enabled=False)
+    mocker.patch.object(
+        processor,
+        "_parse_pdf_tier",
+        AsyncMock(
+            return_value=ProcessingResult(
+                text="",
+                metadata={
+                    "parse_failed_reason": "timeout",
+                    "pipeline_tier": "structured",
+                },
+                processor="pymupdf",
+                success=False,
+                error="isolated parse failed (timeout)",
+            )
+        ),
+    )
+    task = _file_task()
+    task.etag = None  # no content key
+
+    result = await processor._index_document(
+        task, _nc_client(), MagicMock(), tier="structured"
+    )
+
+    assert result is False
+    spies.mark.assert_not_awaited()  # no unmatchable marker written
+    spies.update_ph.assert_awaited_once()  # legacy fallback
+
+
 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