fix(observability): address third review round

Remaining items from the PR #831 Claude review:

- processor span symmetry: add "vector_sync.total_chars" to the sparse
  embedding span (already on the dense span) and drop the redundant
  "embedding.batch_size" attribute from both spans — it always equalled
  vector_sync.chunk_count and would mislead once batching is split.
- metrics: document the deliberate "throughput counts only on full success"
  contract in record_document_parse (partial extractions flagged
  success=False are counted as a parse-error but never inflate
  pages/chars/bytes throughput).
- config: extract _detect_base_provider() -> (family, model) as the single
  source of truth for the provider-detection priority chain, shared by
  get_embedding_model_name() and get_embedding_provider_family(). Preserves
  the intentional gateway asymmetry (only the family method short-circuits).
- base.py: Optional[...] -> PEP 604 `... | None`; drop now-unused import.

Behavior unchanged (get_embedding_* outputs covered by test_config.py).

Refs Deck #175, PR #831.

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

nextcloud_mcp_server/config.py

@@ -917,16 +917,45 @@ class Settings:
         self.enable_multi_user_basic_auth = resolved_mode == "multi_user_basic"
         self.enable_login_flow = resolved_mode == "login_flow"
 
-    def get_embedding_model_name(self) -> str:
+    def _detect_base_provider(self) -> tuple[str, str]:
         """
-        Get the active embedding model name based on provider priority.
+        Resolve the ``(family, model)`` for the underlying embedding provider.
 
-        Priority order (same as ProviderRegistry):
+        Single source of truth for the provider-detection priority chain shared
+        by ``get_embedding_model_name`` and ``get_embedding_provider_family``:
         1. Bedrock - if AWS_REGION or BEDROCK_EMBEDDING_MODEL is set
         2. OpenAI - if OPENAI_API_KEY is set
         3. Mistral - if MISTRAL_API_KEY is set
         4. Ollama - if OLLAMA_BASE_URL is set
-        5. Simple - fallback (returns "simple-{dimension}")
+        5. Simple - fallback
+
+        Does NOT handle the gateway short-circuit — callers layer that on top
+        as needed (see the asymmetry note on ``get_embedding_model_name``).
+        """
+        if (
+            self.aws_region
+            or self.bedrock_embedding_model
+            or self.bedrock_generation_model
+        ):
+            return "bedrock", self.bedrock_embedding_model or "bedrock-default"
+
+        if self.openai_api_key:
+            return "openai", self.openai_embedding_model
+
+        if self.mistral_api_key:
+            return "mistral", self.mistral_embedding_model
+
+        if self.ollama_base_url:
+            return "ollama", self.ollama_embedding_model
+
+        return "simple", f"simple-{self.simple_embedding_dimension}"
+
+    def get_embedding_model_name(self) -> str:
+        """
+        Get the active embedding model name based on provider priority.
+
+        Priority order (same as ProviderRegistry): bedrock → openai → mistral →
+        ollama → simple (returns "simple-{dimension}").
 
         Returns:
             Active embedding model name
@@ -937,23 +966,7 @@ class Settings:
         # get_embedding_provider_family() short-circuits to the gateway-routed
         # family. Keep that asymmetry in mind before joining metrics/labels
         # derived from these two methods.
-        if (
-            self.aws_region
-            or self.bedrock_embedding_model
-            or self.bedrock_generation_model
-        ):
-            return self.bedrock_embedding_model or "bedrock-default"
-
-        if self.openai_api_key:
-            return self.openai_embedding_model
-
-        if self.mistral_api_key:
-            return self.mistral_embedding_model
-
-        if self.ollama_base_url:
-            return self.ollama_embedding_model
-
-        return f"simple-{self.simple_embedding_dimension}"
+        return self._detect_base_provider()[1]
 
     def get_embedding_provider_family(self) -> str:
         """
@@ -964,14 +977,9 @@ class Settings:
         *family* (e.g. "bedrock"), never the model name, to keep metric
         cardinality bounded.
 
-        Priority mirrors ``get_embedding_model_name`` / ProviderRegistry:
-        1. Gateway - if EMBEDDING_PROVIDER=gateway (family from the model prefix,
-           e.g. "mistral/mistral-embed" -> "mistral")
-        2. Bedrock - if AWS_REGION or BEDROCK_EMBEDDING_MODEL is set
-        3. OpenAI - if OPENAI_API_KEY is set
-        4. Mistral - if MISTRAL_API_KEY is set
-        5. Ollama - if OLLAMA_BASE_URL is set
-        6. Simple - fallback
+        Gateway short-circuits to the gateway-routed family (from the model
+        prefix, e.g. "mistral/mistral-embed" -> "mistral"); otherwise the family
+        comes from the shared ``_detect_base_provider`` priority chain.
 
         Returns:
             Provider family: gateway-routed family | bedrock | openai | mistral
@@ -981,23 +989,7 @@ class Settings:
             model = self.embedding_gateway_model or ""
             return model.split("/", 1)[0] if "/" in model else "gateway"
 
-        if (
-            self.aws_region
-            or self.bedrock_embedding_model
-            or self.bedrock_generation_model
-        ):
-            return "bedrock"
-
-        if self.openai_api_key:
-            return "openai"
-
-        if self.mistral_api_key:
-            return "mistral"
-
-        if self.ollama_base_url:
-            return "ollama"
-
-        return "simple"
+        return self._detect_base_provider()[0]
 
     def get_collection_name(self) -> str:
         """

nextcloud_mcp_server/document_processors/base.py

@@ -2,7 +2,7 @@
 
 from abc import ABC, abstractmethod
 from collections.abc import Awaitable, Callable
-from typing import Any, Optional
+from typing import Any
 
 from pydantic import BaseModel
 
@@ -22,7 +22,7 @@ class ProcessingResult(BaseModel):
     success: bool = True
     """Whether processing succeeded"""
 
-    error: Optional[str] = None
+    error: str | None = None
     """Error message if processing failed"""
 
 
@@ -83,11 +83,10 @@ class DocumentProcessor(ABC):
         self,
         content: bytes,
         content_type: str,
-        filename: Optional[str] = None,
-        options: Optional[dict[str, Any]] = None,
-        progress_callback: Optional[
-            Callable[[float, Optional[float], Optional[str]], Awaitable[None]]
-        ] = None,
+        filename: str | None = None,
+        options: dict[str, Any] | None = None,
+        progress_callback: Callable[[float, float | None, str | None], Awaitable[None]]
+        | None = None,
     ) -> ProcessingResult:
         """Process a document and extract text.
 

nextcloud_mcp_server/observability/metrics.py

@@ -533,6 +533,10 @@ def record_document_parse(
         processor=processor, tier=tier, status=status
     ).observe(duration)
     document_parse_total.labels(processor=processor, tier=tier, status=status).inc()
+    # Throughput counters (pages/chars/bytes) accrue only on a full success.
+    # A partial extraction flagged success=False is recorded above as a
+    # parse-error but is intentionally excluded here so low-confidence output
+    # never inflates pipeline throughput.
     if status == "success":
         if pages > 0:
             document_pages_processed_total.labels(processor=processor, tier=tier).inc(

nextcloud_mcp_server/vector/processor.py

@@ -631,7 +631,6 @@ async def _index_document(
                 "embedding.kind": "dense",
                 "embedding.provider": provider,
                 "embedding.model": settings.get_embedding_model_name(),
-                "embedding.batch_size": len(chunk_texts),
             },
         ):
             embedding_service = get_embedding_service()
@@ -659,9 +658,9 @@ async def _index_document(
             "vector_sync.embed_sparse",
             attributes={
                 _ATTR_CHUNK_COUNT: len(chunk_texts),
+                "vector_sync.total_chars": total_chars,
                 "embedding.kind": "sparse",
                 "embedding.provider": "bm25",
-                "embedding.batch_size": len(chunk_texts),
             },
         ):
             bm25_service = await get_bm25_service()