Make better

backend/onyx/db/llm.py

@@ -270,10 +270,34 @@ def upsert_llm_provider(
         mc.name for mc in llm_provider_upsert_request.model_configurations
     }
 
+    default_model = fetch_default_llm_model(db_session)
+
+    # Build a lookup of requested visibility by model name
+    requested_visibility = {
+        mc.name: mc.is_visible
+        for mc in llm_provider_upsert_request.model_configurations
+    }
+
     # Delete removed models
     removed_ids = [
         mc.id for name, mc in existing_by_name.items() if name not in models_to_exist
     ]
+
+    # Prevent removing and hiding the default model
+    if default_model:
+        for name, mc in existing_by_name.items():
+            if mc.id == default_model.id:
+                if name not in models_to_exist:
+                    raise ValueError(
+                        f"Cannot remove the default model '{name}'. "
+                        "Please change the default model before removing."
+                    )
+                if not requested_visibility.get(name, True):
+                    raise ValueError(
+                        f"Cannot hide the default model '{name}'. "
+                        "Please change the default model before hiding."
+                    )
+
     if removed_ids:
         db_session.query(ModelConfiguration).filter(
             ModelConfiguration.id.in_(removed_ids)
@@ -538,7 +562,6 @@ def fetch_default_model(
         .options(selectinload(ModelConfiguration.llm_provider))
         .join(LLMModelFlow)
         .where(
-            ModelConfiguration.is_visible == True,  # noqa: E712
             LLMModelFlow.llm_model_flow_type == flow_type,
             LLMModelFlow.is_default == True,  # noqa: E712
         )

backend/onyx/server/manage/llm/api.py

@@ -65,6 +65,7 @@ from onyx.server.manage.llm.models import LLMProviderUpsertRequest
 from onyx.server.manage.llm.models import LLMProviderView
 from onyx.server.manage.llm.models import LMStudioFinalModelResponse
 from onyx.server.manage.llm.models import LMStudioModelsRequest
+from onyx.server.manage.llm.models import ModelConfigurationUpsertRequest
 from onyx.server.manage.llm.models import OllamaFinalModelResponse
 from onyx.server.manage.llm.models import OllamaModelDetails
 from onyx.server.manage.llm.models import OllamaModelsRequest
@@ -445,16 +446,17 @@ def put_llm_provider(
         not existing_provider or not existing_provider.is_auto_mode
     )
 
-    # Before the upsert, check if this provider currently owns the global
-    # CHAT default. The upsert may cascade-delete model_configurations
-    # (and their flow mappings), so we need to remember this beforehand.
-    was_default_provider = False
-    if existing_provider and transitioning_to_auto_mode:
-        current_default = fetch_default_llm_model(db_session)
-        was_default_provider = (
-            current_default is not None
-            and current_default.llm_provider_id == existing_provider.id
-        )
+    # When transitioning to auto mode, preserve existing model configurations
+    # so the upsert doesn't try to delete them (which would trip the default
+    # model protection guard). sync_auto_mode_models will handle the model
+    # lifecycle afterward — adding new models, hiding removed ones, and
+    # updating the default. This is safe even if sync fails: the provider
+    # keeps its old models and default rather than losing them.
+    if transitioning_to_auto_mode and existing_provider:
+        llm_provider_upsert_request.model_configurations = [
+            ModelConfigurationUpsertRequest.from_model(mc)
+            for mc in existing_provider.model_configurations
+        ]
 
     try:
         result = upsert_llm_provider(
@@ -468,7 +470,6 @@ def put_llm_provider(
 
             config = fetch_llm_recommendations_from_github()
             if config and llm_provider_upsert_request.provider in config.providers:
-                # Refetch the provider to get the updated model
                 updated_provider = fetch_existing_llm_provider_by_id(
                     id=result.id, db_session=db_session
                 )
@@ -478,20 +479,6 @@ def put_llm_provider(
                         updated_provider,
                         config,
                     )
-
-                    # If this provider was the default before the transition,
-                    # restore the default using the recommended model.
-                    if was_default_provider:
-                        recommended = config.get_default_model(
-                            llm_provider_upsert_request.provider
-                        )
-                        if recommended:
-                            update_default_provider(
-                                provider_id=updated_provider.id,
-                                model_name=recommended.name,
-                                db_session=db_session,
-                            )
-
                     # Refresh result with synced models
                     result = LLMProviderView.from_model(updated_provider)
 

backend/tests/external_dependency_unit/llm/test_llm_provider_auto_mode.py

@@ -1152,3 +1152,179 @@ class TestAutoModeTransitionsAndResync:
         finally:
             db_session.rollback()
             _cleanup_provider(db_session, provider_name)
+
+    def test_sync_updates_default_when_recommended_default_changes(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """When the provider owns the CHAT default and a sync arrives with a
+        different recommended default model (both models still in config),
+        the global default should be updated to the new recommendation.
+
+        Steps:
+        1. Create auto-mode provider with config v1: default=gpt-4o.
+        2. Set gpt-4o as the global CHAT default.
+        3. Re-sync with config v2: default=gpt-4o-mini (gpt-4o still present).
+        4. Verify the CHAT default switched to gpt-4o-mini and both models
+           remain visible.
+        """
+        config_v1 = _create_mock_llm_recommendations(
+            provider=LlmProviderNames.OPENAI,
+            default_model_name="gpt-4o",
+            additional_models=["gpt-4o-mini"],
+        )
+        config_v2 = _create_mock_llm_recommendations(
+            provider=LlmProviderNames.OPENAI,
+            default_model_name="gpt-4o-mini",
+            additional_models=["gpt-4o"],
+        )
+
+        try:
+            with patch(
+                "onyx.server.manage.llm.api.fetch_llm_recommendations_from_github",
+                return_value=config_v1,
+            ):
+                put_llm_provider(
+                    llm_provider_upsert_request=LLMProviderUpsertRequest(
+                        name=provider_name,
+                        provider=LlmProviderNames.OPENAI,
+                        api_key="sk-test-key-00000000000000000000000000000000000",
+                        api_key_changed=True,
+                        is_auto_mode=True,
+                        model_configurations=[],
+                    ),
+                    is_creation=True,
+                    _=_create_mock_admin(),
+                    db_session=db_session,
+                )
+
+            # Set gpt-4o as the global CHAT default
+            db_session.expire_all()
+            provider = fetch_existing_llm_provider(
+                name=provider_name, db_session=db_session
+            )
+            assert provider is not None
+            update_default_provider(provider.id, "gpt-4o", db_session)
+
+            default_before = fetch_default_llm_model(db_session)
+            assert default_before is not None
+            assert default_before.name == "gpt-4o"
+
+            # Re-sync with config v2 (recommended default changed)
+            db_session.expire_all()
+            provider = fetch_existing_llm_provider(
+                name=provider_name, db_session=db_session
+            )
+            assert provider is not None
+
+            changes = sync_auto_mode_models(
+                db_session=db_session,
+                provider=provider,
+                llm_recommendations=config_v2,
+            )
+            assert changes > 0, "Sync should report changes when default switches"
+
+            # Both models should remain visible
+            db_session.expire_all()
+            provider = fetch_existing_llm_provider(
+                name=provider_name, db_session=db_session
+            )
+            assert provider is not None
+            visibility = {
+                mc.name: mc.is_visible for mc in provider.model_configurations
+            }
+            assert visibility["gpt-4o"] is True
+            assert visibility["gpt-4o-mini"] is True
+
+            # The CHAT default should now be gpt-4o-mini
+            default_after = fetch_default_llm_model(db_session)
+            assert default_after is not None
+            assert (
+                default_after.name == "gpt-4o-mini"
+            ), f"Default should be updated to 'gpt-4o-mini', got '{default_after.name}'"
+
+        finally:
+            db_session.rollback()
+            _cleanup_provider(db_session, provider_name)
+
+    def test_sync_idempotent_when_default_already_matches(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """When the provider owns the CHAT default and it already matches the
+        recommended default, re-syncing should report zero changes.
+
+        This is a regression test for the bug where changes was unconditionally
+        incremented even when the default was already correct.
+        """
+        config = _create_mock_llm_recommendations(
+            provider=LlmProviderNames.OPENAI,
+            default_model_name="gpt-4o",
+            additional_models=["gpt-4o-mini"],
+        )
+
+        try:
+            with patch(
+                "onyx.server.manage.llm.api.fetch_llm_recommendations_from_github",
+                return_value=config,
+            ):
+                put_llm_provider(
+                    llm_provider_upsert_request=LLMProviderUpsertRequest(
+                        name=provider_name,
+                        provider=LlmProviderNames.OPENAI,
+                        api_key="sk-test-key-00000000000000000000000000000000000",
+                        api_key_changed=True,
+                        is_auto_mode=True,
+                        model_configurations=[],
+                    ),
+                    is_creation=True,
+                    _=_create_mock_admin(),
+                    db_session=db_session,
+                )
+
+            # Set gpt-4o (the recommended default) as global CHAT default
+            db_session.expire_all()
+            provider = fetch_existing_llm_provider(
+                name=provider_name, db_session=db_session
+            )
+            assert provider is not None
+            update_default_provider(provider.id, "gpt-4o", db_session)
+
+            # First sync to stabilize state
+            db_session.expire_all()
+            provider = fetch_existing_llm_provider(
+                name=provider_name, db_session=db_session
+            )
+            assert provider is not None
+            sync_auto_mode_models(
+                db_session=db_session,
+                provider=provider,
+                llm_recommendations=config,
+            )
+
+            # Second sync — default already matches, should be a no-op
+            db_session.expire_all()
+            provider = fetch_existing_llm_provider(
+                name=provider_name, db_session=db_session
+            )
+            assert provider is not None
+            changes = sync_auto_mode_models(
+                db_session=db_session,
+                provider=provider,
+                llm_recommendations=config,
+            )
+            assert changes == 0, (
+                f"Expected 0 changes when default already matches recommended, "
+                f"got {changes}"
+            )
+
+            # Default should still be gpt-4o
+            default_model = fetch_default_llm_model(db_session)
+            assert default_model is not None
+            assert default_model.name == "gpt-4o"
+
+        finally:
+            db_session.rollback()
+            _cleanup_provider(db_session, provider_name)

backend/tests/external_dependency_unit/llm/test_llm_provider_default_model_protection.py

@@ -0,0 +1,220 @@
+"""
+This should act as the main point of reference for testing that default model
+logic is consisten.
+
+ -
+"""
+
+from collections.abc import Generator
+from uuid import uuid4
+
+import pytest
+from sqlalchemy.orm import Session
+
+from onyx.db.llm import fetch_existing_llm_provider
+from onyx.db.llm import remove_llm_provider
+from onyx.db.llm import update_default_provider
+from onyx.db.llm import update_default_vision_provider
+from onyx.db.llm import upsert_llm_provider
+from onyx.llm.constants import LlmProviderNames
+from onyx.server.manage.llm.models import LLMProviderUpsertRequest
+from onyx.server.manage.llm.models import LLMProviderView
+from onyx.server.manage.llm.models import ModelConfigurationUpsertRequest
+
+
+def _create_test_provider(
+    db_session: Session,
+    name: str,
+    models: list[ModelConfigurationUpsertRequest] | None = None,
+) -> LLMProviderView:
+    """Helper to create a test LLM provider with multiple models."""
+    if models is None:
+        models = [
+            ModelConfigurationUpsertRequest(
+                name="gpt-4o", is_visible=True, supports_image_input=True
+            ),
+            ModelConfigurationUpsertRequest(
+                name="gpt-4o-mini", is_visible=True, supports_image_input=False
+            ),
+        ]
+    return upsert_llm_provider(
+        LLMProviderUpsertRequest(
+            name=name,
+            provider=LlmProviderNames.OPENAI,
+            api_key="sk-test-key-00000000000000000000000000000000000",
+            api_key_changed=True,
+            model_configurations=models,
+        ),
+        db_session=db_session,
+    )
+
+
+def _cleanup_provider(db_session: Session, name: str) -> None:
+    """Helper to clean up a test provider by name."""
+    provider = fetch_existing_llm_provider(name=name, db_session=db_session)
+    if provider:
+        remove_llm_provider(db_session, provider.id)
+
+
+@pytest.fixture
+def provider_name(db_session: Session) -> Generator[str, None, None]:
+    """Generate a unique provider name for each test, with automatic cleanup."""
+    name = f"test-provider-{uuid4().hex[:8]}"
+    yield name
+    db_session.rollback()
+    _cleanup_provider(db_session, name)
+
+
+class TestDefaultModelProtection:
+    """Tests that the default model cannot be removed or hidden."""
+
+    def test_cannot_remove_default_text_model(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """Removing the default text model from a provider should raise ValueError."""
+        provider = _create_test_provider(db_session, provider_name)
+        update_default_provider(provider.id, "gpt-4o", db_session)
+
+        # Try to update the provider without the default model
+        with pytest.raises(ValueError, match="Cannot remove the default model"):
+            upsert_llm_provider(
+                LLMProviderUpsertRequest(
+                    id=provider.id,
+                    name=provider_name,
+                    provider=LlmProviderNames.OPENAI,
+                    api_key="sk-test-key-00000000000000000000000000000000000",
+                    api_key_changed=True,
+                    model_configurations=[
+                        ModelConfigurationUpsertRequest(
+                            name="gpt-4o-mini", is_visible=True
+                        ),
+                    ],
+                ),
+                db_session=db_session,
+            )
+
+    def test_cannot_hide_default_text_model(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """Setting is_visible=False on the default text model should raise ValueError."""
+        provider = _create_test_provider(db_session, provider_name)
+        update_default_provider(provider.id, "gpt-4o", db_session)
+
+        # Try to hide the default model
+        with pytest.raises(ValueError, match="Cannot hide the default model"):
+            upsert_llm_provider(
+                LLMProviderUpsertRequest(
+                    id=provider.id,
+                    name=provider_name,
+                    provider=LlmProviderNames.OPENAI,
+                    api_key="sk-test-key-00000000000000000000000000000000000",
+                    api_key_changed=True,
+                    model_configurations=[
+                        ModelConfigurationUpsertRequest(
+                            name="gpt-4o", is_visible=False
+                        ),
+                        ModelConfigurationUpsertRequest(
+                            name="gpt-4o-mini", is_visible=True
+                        ),
+                    ],
+                ),
+                db_session=db_session,
+            )
+
+    def test_cannot_remove_default_vision_model(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """Removing the default vision model from a provider should raise ValueError."""
+        provider = _create_test_provider(db_session, provider_name)
+        # Set gpt-4o as both the text and vision default
+        update_default_provider(provider.id, "gpt-4o", db_session)
+        update_default_vision_provider(provider.id, "gpt-4o", db_session)
+
+        # Try to remove the default vision model
+        with pytest.raises(ValueError, match="Cannot remove the default model"):
+            upsert_llm_provider(
+                LLMProviderUpsertRequest(
+                    id=provider.id,
+                    name=provider_name,
+                    provider=LlmProviderNames.OPENAI,
+                    api_key="sk-test-key-00000000000000000000000000000000000",
+                    api_key_changed=True,
+                    model_configurations=[
+                        ModelConfigurationUpsertRequest(
+                            name="gpt-4o-mini", is_visible=True
+                        ),
+                    ],
+                ),
+                db_session=db_session,
+            )
+
+    def test_can_remove_non_default_model(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """Removing a non-default model should succeed."""
+        provider = _create_test_provider(db_session, provider_name)
+        update_default_provider(provider.id, "gpt-4o", db_session)
+
+        # Remove gpt-4o-mini (not default) — should succeed
+        updated = upsert_llm_provider(
+            LLMProviderUpsertRequest(
+                id=provider.id,
+                name=provider_name,
+                provider=LlmProviderNames.OPENAI,
+                api_key="sk-test-key-00000000000000000000000000000000000",
+                api_key_changed=True,
+                model_configurations=[
+                    ModelConfigurationUpsertRequest(
+                        name="gpt-4o", is_visible=True, supports_image_input=True
+                    ),
+                ],
+            ),
+            db_session=db_session,
+        )
+
+        model_names = {mc.name for mc in updated.model_configurations}
+        assert "gpt-4o" in model_names
+        assert "gpt-4o-mini" not in model_names
+
+    def test_can_hide_non_default_model(
+        self,
+        db_session: Session,
+        provider_name: str,
+    ) -> None:
+        """Hiding a non-default model should succeed."""
+        provider = _create_test_provider(db_session, provider_name)
+        update_default_provider(provider.id, "gpt-4o", db_session)
+
+        # Hide gpt-4o-mini (not default) — should succeed
+        updated = upsert_llm_provider(
+            LLMProviderUpsertRequest(
+                id=provider.id,
+                name=provider_name,
+                provider=LlmProviderNames.OPENAI,
+                api_key="sk-test-key-00000000000000000000000000000000000",
+                api_key_changed=True,
+                model_configurations=[
+                    ModelConfigurationUpsertRequest(
+                        name="gpt-4o", is_visible=True, supports_image_input=True
+                    ),
+                    ModelConfigurationUpsertRequest(
+                        name="gpt-4o-mini", is_visible=False
+                    ),
+                ],
+            ),
+            db_session=db_session,
+        )
+
+        model_visibility = {
+            mc.name: mc.is_visible for mc in updated.model_configurations
+        }
+        assert model_visibility["gpt-4o"] is True
+        assert model_visibility["gpt-4o-mini"] is False