fix(phase8/task46/h4): graceful skip for default chat collection without embedding model

aperag/domains/conversation/service/chat_collection_service.py

@@ -112,13 +112,27 @@ async def _get_default_embedding_model(self, user_id: str) -> Optional[ModelSpec
             logger.error(f"Failed to get default embedding model for user {user_id}: {e}")
             return None
 
-    async def create_user_chat_collection(self, user_id: str) -> KnowledgeBaseCollectionView:
-        """Create chat collection for user"""
+    async def create_user_chat_collection(self, user_id: str) -> Optional[KnowledgeBaseCollectionView]:
+        """Create chat collection for user.
+
+        Returns ``None`` when no embedding model is configured for the user
+        (graceful skip — the user can configure a provider later and create
+        a chat collection on demand). This keeps registration / first-login
+        flows free of provider-required failures and avoids Celery retry
+        spam in provider-independent smoke environments.
+        """
         # Get default embedding model
         embedding_model = await self._get_default_embedding_model(user_id)
 
         if not embedding_model:
-            raise ValueError("No suitable embedding model found for chat collection")
+            logger.info(
+                "Skipping default chat collection creation for user %s: "
+                "no embedding model configured. Configure an LLM provider "
+                "with default_for_embedding or enable_for_collection tag to "
+                "enable default chat collection.",
+                user_id,
+            )
+            return None
 
         # Create collection config
         config = CollectionConfig(
@@ -164,8 +178,12 @@ async def _mark_as_chat_collection(session):
         logger.info(f"Created chat collection {collection.id} for user {user_id}")
         return collection
 
-    async def initialize_user_chat_collection(self, user_id: str) -> KnowledgeBaseCollectionView:
-        """Initialize chat collection for user during registration"""
+    async def initialize_user_chat_collection(self, user_id: str) -> Optional[KnowledgeBaseCollectionView]:
+        """Initialize chat collection for user during registration.
+
+        Returns ``None`` when no embedding model is configured (graceful
+        skip — see ``create_user_chat_collection`` for rationale).
+        """
         existing_collection = await self.get_user_chat_collection(user_id)
         if existing_collection:
             logger.info(f"User {user_id} already has chat collection {existing_collection.id}")

aperag/domains/conversation/service/chat_document_service.py

@@ -58,6 +58,20 @@ async def upload_chat_document(self, chat_id: str, user_id: str, file: UploadFil
         if not collection:
             # Create if missing (fallback)
             collection = await chat_collection_service.create_user_chat_collection(user_id)
+            if collection is None:
+                # Graceful skip path: user has no embedding model configured,
+                # so we cannot create / use a default chat collection. Surface
+                # a 400 to the caller — chat document upload genuinely
+                # requires an embedding model, unlike registration.
+                raise HTTPException(
+                    status_code=400,
+                    detail=(
+                        "No embedding model is configured. Configure an LLM "
+                        "provider with the default_for_embedding or "
+                        "enable_for_collection tag before uploading chat "
+                        "documents."
+                    ),
+                )
 
         # Prepare document metadata (without message_id initially)
         doc_metadata = {

aperag/tasks/collection.py

@@ -139,7 +139,23 @@ def _initialize_vector_databases(self, collection_id: str, collection) -> None:
         the connector). We still call through so new deployments get their
         global collection primed at cluster-creation time rather than on the
         first user upload.
+
+        Mirrors ``_initialize_fulltext_index``'s ``enable_fulltext`` skip:
+        a collection with ``enable_vector=False`` does not require any
+        embedding lookup, so we short-circuit before resolving the
+        embedding provider. Without this guard, provider-independent
+        collections (smoke tests, KG-only tenants) trigger a NoneType
+        ``model_service_provider`` access in ``base_embedding`` and a
+        Celery retry storm.
         """
+        config = parseCollectionConfig(collection.config)
+        if not config.enable_vector:
+            logger.info(
+                "Skipping vector database initialization for collection %s because enable_vector=false",
+                collection_id,
+            )
+            return
+
         # Get embedding service
         _, vector_size = get_collection_embedding_service_sync(collection)
 

tests/unit_test/chat/test_chat_collection_graceful_skip.py

@@ -0,0 +1,82 @@
+from types import SimpleNamespace
+
+import pytest
+
+from aperag.domains.conversation.service.chat_collection_service import ChatCollectionService
+
+
+class _FakeUserDbOps:
+    """Fake db_ops that always reports the user has no chat collection yet."""
+
+    def __init__(self):
+        self.queried_user_ids = []
+
+    async def query_user_by_id(self, user_id):
+        self.queried_user_ids.append(user_id)
+        return SimpleNamespace(id=user_id, chat_collection_id=None)
+
+    async def query_collection_by_id(self, collection_id):
+        return None
+
+
+@pytest.mark.asyncio
+async def test_create_user_chat_collection_returns_none_when_no_embedding_model():
+    """No provider configured → graceful skip (return None), no ValueError."""
+    service = ChatCollectionService()
+    service.db_ops = _FakeUserDbOps()
+
+    async def _no_embedding(_user_id):
+        return None
+
+    service._get_default_embedding_model = _no_embedding
+
+    result = await service.create_user_chat_collection("user-no-provider")
+
+    assert result is None
+
+
+@pytest.mark.asyncio
+async def test_initialize_user_chat_collection_returns_none_when_no_embedding_model():
+    """Registration flow tolerates missing provider — returns None."""
+    service = ChatCollectionService()
+    service.db_ops = _FakeUserDbOps()
+
+    async def _no_embedding(_user_id):
+        return None
+
+    service._get_default_embedding_model = _no_embedding
+
+    result = await service.initialize_user_chat_collection("user-no-provider")
+
+    assert result is None
+
+
+@pytest.mark.asyncio
+async def test_initialize_user_chat_collection_returns_existing_when_present():
+    """Existing chat collection takes precedence — graceful skip path is unreachable."""
+    service = ChatCollectionService()
+
+    existing_collection = SimpleNamespace(id="coll-existing", status="ACTIVE")
+
+    class _ExistingChatOps:
+        async def query_user_by_id(self, _user_id):
+            return SimpleNamespace(id="user-1", chat_collection_id="coll-existing")
+
+        async def query_collection_by_id(self, collection_id):
+            assert collection_id == "coll-existing"
+            return existing_collection
+
+    service.db_ops = _ExistingChatOps()
+
+    embedding_calls = []
+
+    async def _no_embedding(user_id):
+        embedding_calls.append(user_id)
+        return None
+
+    service._get_default_embedding_model = _no_embedding
+
+    result = await service.initialize_user_chat_collection("user-1")
+
+    assert result is existing_collection
+    assert embedding_calls == [], "embedding lookup should be skipped when collection exists"

tests/unit_test/tasks/test_collection_init_skip.py

@@ -0,0 +1,73 @@
+"""Tests for ``CollectionTask`` graceful skip paths in ``_initialize_vector_databases``.
+
+Mirrors the existing ``_initialize_fulltext_index`` ``enable_fulltext`` skip
+behavior: a collection with ``enable_vector=false`` must not trigger an
+embedding provider lookup, otherwise provider-independent collections cause
+a Celery retry storm via ``model_service_provider`` ``NoneType`` access.
+"""
+
+import json
+from types import SimpleNamespace
+from unittest.mock import patch
+
+from aperag.tasks.collection import CollectionTask
+
+
+def _collection(*, enable_vector: bool, embedding=None):
+    config = {
+        "source": "system",
+        "enable_vector": enable_vector,
+        "enable_fulltext": False,
+        "enable_knowledge_graph": False,
+        "enable_summary": False,
+        "enable_vision": False,
+    }
+    if embedding is not None:
+        config["embedding"] = embedding
+    return SimpleNamespace(id="coll-1", config=json.dumps(config), user="user-1")
+
+
+def test_initialize_vector_databases_skips_when_enable_vector_false():
+    """enable_vector=false collection skips embedding lookup entirely."""
+    task = CollectionTask()
+    collection = _collection(enable_vector=False)
+
+    with (
+        patch("aperag.tasks.collection.get_collection_embedding_service_sync") as mock_emb,
+        patch("aperag.tasks.collection.get_vector_db_connector") as mock_vdb,
+    ):
+        task._initialize_vector_databases("coll-1", collection)
+
+    mock_emb.assert_not_called()
+    mock_vdb.assert_not_called()
+
+
+def test_initialize_vector_databases_resolves_embedding_when_enable_vector_true():
+    """enable_vector=true collection resolves embedding provider as before."""
+    task = CollectionTask()
+    collection = _collection(
+        enable_vector=True,
+        embedding={
+            "model": "text-embedding-3-small",
+            "model_service_provider": "openai",
+            "custom_llm_provider": "openai",
+        },
+    )
+
+    fake_connector = SimpleNamespace(connector=SimpleNamespace(ensure_collection=lambda: None))
+    with (
+        patch(
+            "aperag.tasks.collection.get_collection_embedding_service_sync",
+            return_value=(SimpleNamespace(), 1536),
+        ) as mock_emb,
+        patch(
+            "aperag.tasks.collection.get_vector_db_connector",
+            return_value=fake_connector,
+        ) as mock_vdb,
+    ):
+        task._initialize_vector_databases("coll-1", collection)
+
+    mock_emb.assert_called_once_with(collection)
+    assert mock_vdb.call_count == 1
+    _, kwargs = mock_vdb.call_args
+    assert kwargs["vector_size"] == 1536