Address review: fix Awaitable double-await, default value, phantom imports

- StreamObserverProtocol.__call__: returns None (async def already wraps in Awaitable)
- PermissionedQueryManagerProtocol.visible_to_user: default is None not Ellipsis
- Drop unused Awaitable and Optional imports from protocols
- Remove phantom protocol imports + multi-line comment blocks from tool_factory, base_component, Managers (rely on canonical opencontractserver.types.protocols)
- Collapse multi-line re-export comment in core_vector_stores
- Type _process_single_upload/_fail_upload upload_id as UUID (matches model)
- Type agents/admin.get_queryset return as QuerySet[AgentActionResult]
- Type UserFeedbackManager.get_or_none *args/**kwargs as Any
- Document why StreamObserverProtocol is not @runtime_checkable

Author: JSv4

opencontractserver/agents/admin.py

@@ -1,6 +1,5 @@
-from typing import Any
-
 from django.contrib import admin
+from django.db.models import QuerySet
 from django.http import HttpRequest
 from django.utils.html import format_html
 from guardian.admin import GuardedModelAdmin
@@ -268,7 +267,7 @@ def execution_metadata_display(self, obj: AgentActionResult) -> str:
 
     execution_metadata_display.short_description = "Execution Metadata"
 
-    def get_queryset(self, request: HttpRequest) -> Any:
+    def get_queryset(self, request: HttpRequest) -> QuerySet[AgentActionResult]:
         """Optimize queryset with select_related."""
         return (
             super()

opencontractserver/llms/tools/tool_factory.py

@@ -6,14 +6,9 @@
 from typing import Any, Callable, Optional
 
 from opencontractserver.llms.types import AgentFramework
-from opencontractserver.types.protocols import ToolProtocol  # noqa: F401
 
 logger = logging.getLogger(__name__)
 
-# ``ToolProtocol`` mirrors the public surface of :class:`CoreTool` so that
-# framework adapters (e.g. PydanticAI) can accept any object exposing the
-# same ``name``/``description``/``parameters``/``requires_approval`` API.
-
 
 def build_inject_params_for_context(
     tool: "CoreTool",

opencontractserver/llms/vector_stores/core_vector_stores.py

@@ -1103,10 +1103,7 @@ async def async_search(self, query: VectorSearchQuery) -> list[VectorSearchResul
         return results
 
 
-# ``VectorStoreProtocol`` is imported above so that downstream callers can
-# annotate parameters as ``VectorStoreProtocol`` and mypy will accept any
-# class implementing the same surface as :class:`CoreAnnotationVectorStore`.
-# Re-export to make the dependency explicit for static analysis tools.
+# Re-exported so downstream callers can annotate against the protocol.
 __all__ = [
     "CoreAnnotationVectorStore",
     "VectorSearchQuery",

opencontractserver/pipeline/base/base_component.py

@@ -5,16 +5,8 @@
 
 from django.conf import settings
 
-from opencontractserver.types.protocols import PipelineComponentProtocol  # noqa: F401
-
 logger = logging.getLogger(__name__)
 
-# ``PipelineComponentProtocol`` (re-exported here) describes the duck-typed
-# surface that registry consumers rely on (``title``, ``description``,
-# ``author``, ``dependencies``). Concrete subclasses of
-# :class:`PipelineComponentBase` (parsers, embedders, thumbnailers,
-# post-processors) all satisfy that protocol structurally.
-
 
 class PipelineComponentBase(ABC):
     """

opencontractserver/shared/Managers.py

@@ -12,14 +12,6 @@
     PermissionQuerySet,
     UserFeedbackQuerySet,
 )
-from opencontractserver.types.protocols import (  # noqa: F401
-    PermissionedQueryManagerProtocol,
-)
-
-# ``PermissionedQueryManagerProtocol`` (in ``opencontractserver.types.protocols``)
-# describes the public ``visible_to_user(user)`` contract that every manager
-# below honours. Importing it here makes the dependency explicit so callers
-# typing on the protocol get static checks for free.
 
 if TYPE_CHECKING:
     from django.contrib.auth import get_user_model
@@ -247,7 +239,7 @@ def visible_to_user(self, user: User) -> QuerySet:
         """Delegate to the queryset's visible_to_user method"""
         return self.get_queryset().visible_to_user(user)
 
-    def get_or_none(self, *args, **kwargs) -> Optional[Any]:
+    def get_or_none(self, *args: Any, **kwargs: Any) -> Optional[Any]:
         try:
             return self.get(*args, **kwargs)
         except self.model.DoesNotExist:

opencontractserver/types/protocols.py

@@ -20,7 +20,6 @@
 
 from __future__ import annotations
 
-from collections.abc import Awaitable
 from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable
 
 if TYPE_CHECKING:
@@ -149,7 +148,7 @@ class PermissionedQueryManagerProtocol(Protocol):
     accept ``None`` (treated as anonymous) still satisfy the protocol.
     """
 
-    def visible_to_user(self, user: Any | None = ...) -> QuerySet[Any]:
+    def visible_to_user(self, user: Any | None = None) -> QuerySet[Any]:
         """Return a queryset filtered to objects visible to ``user``."""
         ...
 
@@ -165,9 +164,13 @@ class StreamObserverProtocol(Protocol):
     Mirrors :class:`opencontractserver.llms.types.StreamObserver` so it
     can be referenced from non-LLM code (notifications, websockets)
     without importing ``llms.types``.
+
+    Not ``@runtime_checkable``: ``isinstance`` checks against an
+    async-only ``__call__`` protocol cannot distinguish sync from async
+    callables, so we leave verification to the static type checker.
     """
 
-    async def __call__(self, event: Any) -> Awaitable[None]: ...
+    async def __call__(self, event: Any) -> None: ...
 
 
 __all__ = [

opencontractserver/worker_uploads/tasks.py

@@ -16,6 +16,7 @@
 import re
 from datetime import timedelta
 from typing import Any
+from uuid import UUID
 
 from celery import shared_task
 from django.conf import settings
@@ -180,7 +181,7 @@ def recover_stalled_uploads() -> dict[str, int]:
     return {"recovered": count}
 
 
-def _process_single_upload(upload_id: Any) -> None:
+def _process_single_upload(upload_id: UUID) -> None:
     """
     Process one WorkerDocumentUpload: create Document, annotations,
     embeddings, and add to the target corpus.
@@ -487,7 +488,7 @@ def _get_vector_field(dimension: int) -> str | None:
     return _VECTOR_FIELD_MAP.get(dimension)
 
 
-def _fail_upload(upload_id: Any, error_message: str) -> None:
+def _fail_upload(upload_id: UUID, error_message: str) -> None:
     """Mark an upload as FAILED and clean up its staging file."""
     upload = WorkerDocumentUpload.objects.filter(id=upload_id).first()
     if upload is None: