diff --git a/.agents/feature-map/components/llm-providers.md b/.agents/feature-map/components/llm-providers.md index d14d190fff..bb864c60f8 100644 --- a/.agents/feature-map/components/llm-providers.md +++ b/.agents/feature-map/components/llm-providers.md @@ -23,7 +23,8 @@ An admin adds an LLM provider (OpenAI, Anthropic, Bedrock, Ollama, a custom OpenAI-compatible endpoint, and more) under **Admin > LLM**, enters credentials, and picks which of the provider's models are visible to end users. The admin then marks one model as the default for chat, and optionally separate defaults -for vision, contextual RAG, chat-naming, and Craft. +for vision, contextual RAG, chat naming, search selection, and Craft. +The search-selection designation is stored independently of the chat default. A user picks a model per chat session, or leaves it on the assistant's default, or leaves that on the workspace default. Whichever call actually reaches the @@ -58,6 +59,7 @@ Admin router, prefix `/admin/llm`: | POST | `/admin/llm/default` | `set_provider_as_default` | Sets the CHAT default. | | POST | `/admin/llm/default-vision` | `set_provider_as_default_vision` | Rejects a model that fails `model_supports_image_input`. | | POST/DELETE | `/admin/llm/default-chat-naming` | `set_provider_as_default_chat_naming` / `clear_default_chat_naming` | | +| POST/DELETE | `/admin/llm/default-search-selection` | `set_provider_as_default_search_selection` / `clear_default_search_selection` | Independent optional model for selecting and checking retrieved sections. | | POST/DELETE | `/admin/llm/default-craft` | `set_provider_as_default_craft` / `clear_default_craft` | Craft has no capability check; see ยง9. | | GET | `/admin/llm/built-in/options`, `/built-in/options/{provider_name}` | `fetch_llm_options`, `fetch_llm_provider_options` | The well-known-provider catalogue. | | GET | `/admin/llm/custom-provider-names` | `fetch_custom_provider_names` | Names for the custom provider option. | @@ -140,7 +142,7 @@ edit surface. It is a thin client over the endpoints above. one default per flow across the whole table, not per provider. It does not guarantee that a default exists. Seed a default separately. - **`LLMModelFlowType`** (`db/enums.py`): `CHAT`, `VISION`, `CONTEXTUAL_RAG`, - `REASONING`, `CHAT_NAMING`, `CRAFT`. Stored on `llm_model_flow` as + `REASONING`, `CHAT_NAMING`, `SEARCH_SELECTION`, `CRAFT`. Stored on `llm_model_flow` as `Enum(LLMModelFlowType, native_enum=False)`, which SQLAlchemy renders as a plain `VARCHAR` column with **no `CHECK` constraint** (`alembic/versions/f220515df7b4_add_flow_mapping_table.py:upgrade`, @@ -220,19 +222,33 @@ joins `ModelConfiguration` to `LLMModelFlow` and filters on `llm_model_flow_type == flow_type AND is_default == true`. Thin wrappers exist per flow: `fetch_default_llm_model` (CHAT), `fetch_default_vision_model`, `fetch_default_contextual_rag_model`, `fetch_default_chat_naming_model`, -`fetch_default_craft_model`. +`fetch_default_search_selection_model`, `fetch_default_craft_model`. Setting a new default goes through `db/llm.py:_update_default_model`, which clears the prior `is_default` row for that flow type and sets the new one in the same transaction, so the partial unique index is never violated mid-write. `update_default_provider` (CHAT), `update_default_vision_provider` (checks `model_supports_image_input` first), `update_default_chat_naming_provider`, -and `update_default_craft_provider` (creates the `LLMModelFlow` row first via -`create_new_flow_mapping__no_commit`, because nothing else ever populates a -CRAFT-flow row) all call it. `fetch_existing_models(db_session, flow_types)` +`update_default_search_selection_provider`, and +`update_default_craft_provider` all call it. +The naming, selection, and Craft setters share `_update_default_pointer_model`, which +validates the selected model and creates its pointer mapping through +`create_new_flow_mapping__no_commit`. `fetch_existing_models(db_session, flow_types)` returns every `ModelConfiguration` that has an `LLMModelFlow` row matching any flow type in the list. +CHAT_NAMING and SEARCH_SELECTION preserve `ModelConfiguration.is_visible`; they do +not add a model to the user picker. Provider edits can change a task-only default's visibility, but +cannot remove its model until the default is cleared. Auto-mode sync does not +restore visibility solely because a model holds either designation. Admin listings expose and +pin `default_chat_naming` and `default_search_selection`, including hidden models. +User and persona listings leave both descriptors empty and do not pin their models. + +Migration `c92b67e56ec4` converts draft FAST mappings into CHAT_NAMING mappings. +Existing canonical naming defaults take precedence. Duplicate mappings merge, +and model visibility remains unchanged. The downgrade does not reverse the +merge because original and converted naming rows cannot be distinguished. + ### 4.4 The LiteLLM wrapper: `LitellmLLM` `multi_llm.py:LitellmLLM` (subclass of `interfaces.py:LLM`) wraps the LiteLLM diff --git a/backend/alembic/versions/c92b67e56ec4_preserve_draft_fast_defaults_as_chat_.py b/backend/alembic/versions/c92b67e56ec4_preserve_draft_fast_defaults_as_chat_.py new file mode 100644 index 0000000000..360880c38a --- /dev/null +++ b/backend/alembic/versions/c92b67e56ec4_preserve_draft_fast_defaults_as_chat_.py @@ -0,0 +1,55 @@ +"""Preserve draft fast defaults as chat naming defaults + +Revision ID: c92b67e56ec4 +Revises: 1c7b6cce66ac +Create Date: 2026-10-14 16:53:14.197305 + +""" + +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "c92b67e56ec4" +down_revision = "1c7b6cce66ac" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute(""" + UPDATE llm_model_flow + SET is_default = FALSE + WHERE llm_model_flow_type = 'FAST' + AND is_default = TRUE + AND EXISTS ( + SELECT 1 FROM llm_model_flow + WHERE llm_model_flow_type = 'CHAT_NAMING' AND is_default = TRUE + ) + """) + op.execute(""" + UPDATE llm_model_flow AS naming + SET is_default = TRUE + FROM llm_model_flow AS fast + WHERE naming.llm_model_flow_type = 'CHAT_NAMING' + AND fast.llm_model_flow_type = 'FAST' + AND fast.is_default = TRUE + AND naming.model_configuration_id = fast.model_configuration_id + """) + op.execute(""" + DELETE FROM llm_model_flow AS fast + USING llm_model_flow AS naming + WHERE fast.llm_model_flow_type = 'FAST' + AND naming.llm_model_flow_type = 'CHAT_NAMING' + AND fast.model_configuration_id = naming.model_configuration_id + """) + op.execute(""" + UPDATE llm_model_flow + SET llm_model_flow_type = 'CHAT_NAMING' + WHERE llm_model_flow_type = 'FAST' + """) + + +def downgrade() -> None: + # Original naming rows cannot be distinguished from converted draft rows. + pass diff --git a/backend/onyx/db/enums.py b/backend/onyx/db/enums.py index 5b19e1fdf0..52ee24c41e 100644 --- a/backend/onyx/db/enums.py +++ b/backend/onyx/db/enums.py @@ -683,6 +683,7 @@ class LLMModelFlowType(str, PyEnum): CONTEXTUAL_RAG = "contextual_rag" REASONING = "reasoning" CHAT_NAMING = "chat_naming" + SEARCH_SELECTION = "search_selection" CRAFT = "craft" diff --git a/backend/onyx/db/llm.py b/backend/onyx/db/llm.py index 491f39f2ee..d14e85195e 100644 --- a/backend/onyx/db/llm.py +++ b/backend/onyx/db/llm.py @@ -477,12 +477,6 @@ def upsert_llm_provider( ] ) - # Every deployment default lives on a flow row pointing at a model, and - # _update_default_model__no_commit makes that model visible, so a model - # holding any default must stay present and visible. Checking only the chat - # default let an edit hide the model contextual RAG or Craft still resolves - # to, since neither resolver looks at is_visible. - # # Only the visible-to-hidden transition is refused, not the steady state. A # default can already sit on a hidden model โ€” sync_auto_mode_models hides # models dropped from the recommendations and re-points only the chat @@ -501,7 +495,12 @@ def upsert_llm_provider( f"Cannot remove the default model '{name}'. It is the default for: " f"{held}. Please change those defaults before removing." ) - if mc.is_visible and not requested_visibility.get(name, True): + if ( + held_flows + - {LLMModelFlowType.CHAT_NAMING, LLMModelFlowType.SEARCH_SELECTION} + and mc.is_visible + and not requested_visibility.get(name, True) + ): raise ValueError( f"Cannot hide the default model '{name}'. It is the default for: " f"{held}. Please change those defaults before hiding." @@ -1152,6 +1151,12 @@ def fetch_default_chat_naming_model( return fetch_default_model(db_session, LLMModelFlowType.CHAT_NAMING) +def fetch_default_search_selection_model( + db_session: Session, +) -> ModelConfiguration | None: + return fetch_default_model(db_session, LLMModelFlowType.SEARCH_SELECTION) + + def fetch_default_craft_model(db_session: Session) -> ModelConfiguration | None: return fetch_default_model(db_session, LLMModelFlowType.CRAFT) @@ -1372,38 +1377,19 @@ def update_default_vision_provider( ) -def update_default_chat_naming_provider( - provider_id: int, chat_naming_model: str, db_session: Session +def _update_default_pointer_model( + provider_id: int, + model_name: str, + db_session: Session, + flow_type: LLMModelFlowType, ) -> None: - provider = db_session.scalar( - select(LLMProviderModel).where( - LLMProviderModel.id == provider_id, - ) - ) - - if provider is None: - raise ValueError(f"LLM Provider with id={provider_id} does not exist") - - _update_default_model( - db_session=db_session, - provider_id=provider_id, - model=chat_naming_model, - flow_type=LLMModelFlowType.CHAT_NAMING, - ) - - -def update_default_craft_provider( - provider_id: int, model_name: str, db_session: Session -) -> None: - # CRAFT is a pointer flow, not a capability: nothing populates it during - # provider upsert, so the row has to be created before it can be defaulted. model_config = db_session.scalar( select(ModelConfiguration).where( ModelConfiguration.llm_provider_id == provider_id, ModelConfiguration.name == model_name, ) ) - if not model_config: + if model_config is None: raise ValueError( f"Model '{model_name}' is not a valid model for provider_id={provider_id}" ) @@ -1411,15 +1397,33 @@ def update_default_craft_provider( create_new_flow_mapping__no_commit( db_session=db_session, model_configuration_id=model_config.id, - flow_type=LLMModelFlowType.CRAFT, + flow_type=flow_type, ) db_session.flush() + _update_default_model(db_session, provider_id, model_name, flow_type) - _update_default_model( - db_session, - provider_id, - model_name, - LLMModelFlowType.CRAFT, + +def update_default_chat_naming_provider( + provider_id: int, chat_naming_model: str, db_session: Session +) -> None: + _update_default_pointer_model( + provider_id, chat_naming_model, db_session, LLMModelFlowType.CHAT_NAMING + ) + + +def update_default_craft_provider( + provider_id: int, model_name: str, db_session: Session +) -> None: + _update_default_pointer_model( + provider_id, model_name, db_session, LLMModelFlowType.CRAFT + ) + + +def update_default_search_selection_provider( + provider_id: int, model_name: str, db_session: Session +) -> None: + _update_default_pointer_model( + provider_id, model_name, db_session, LLMModelFlowType.SEARCH_SELECTION ) @@ -1463,6 +1467,18 @@ def update_no_default_contextual_rag_provider( db_session.commit() +def update_no_default_search_selection_provider(db_session: Session) -> None: + db_session.execute( + update(LLMModelFlow) + .where( + LLMModelFlow.llm_model_flow_type == LLMModelFlowType.SEARCH_SELECTION, + LLMModelFlow.is_default == True, # noqa: E712 + ) + .values(is_default=False) + ) + db_session.commit() + + def update_default_contextual_model( db_session: Session, enable_contextual_rag: bool, @@ -1603,12 +1619,6 @@ def sync_auto_mode_models( ) changes += 1 - # Reconcile the visibility of the models the config dropped. A model still - # holding a deployment default stays visible: only the chat default is - # re-pointed above, so hiding the rest would strand a default on a model the - # admin can no longer see or change. Every other write path keeps a default - # model visible. - # # Both statements test the default in SQL rather than from a snapshot read # here. A default assigned between the two would otherwise be missed, and # the model hidden anyway. They synchronize the session because sessions are @@ -1625,6 +1635,9 @@ def sync_auto_mode_models( .where( LLMModelFlow.model_configuration_id == ModelConfiguration.id, LLMModelFlow.is_default == True, # noqa: E712 + LLMModelFlow.llm_model_flow_type.not_in( + [LLMModelFlowType.CHAT_NAMING, LLMModelFlowType.SEARCH_SELECTION] + ), ) .exists() ) @@ -1846,9 +1859,6 @@ def update_model_configuration__no_commit( for flow_type in supported_flows if flow_type not in model_configuration.llm_model_flow_types } - # Only the capability-derived flows are reconciled here. The pointer flows โ€” - # CONTEXTUAL_RAG, CHAT_NAMING and CRAFT โ€” are set by their own endpoints and - # are implied by no supports_* field, so they are never in supported_flows. # Without this filter an ordinary provider update deletes them, and the # deployment default each one carries goes with it. reconciled_flows = { @@ -1916,7 +1926,11 @@ def _update_default_model__no_commit( ) new_default.is_default = True - model_config.is_visible = True + if flow_type not in { + LLMModelFlowType.CHAT_NAMING, + LLMModelFlowType.SEARCH_SELECTION, + }: + model_config.is_visible = True def _update_default_model( diff --git a/backend/onyx/server/manage/llm/api.py b/backend/onyx/server/manage/llm/api.py index 8693a6b856..450aeceacc 100644 --- a/backend/onyx/server/manage/llm/api.py +++ b/backend/onyx/server/manage/llm/api.py @@ -22,6 +22,7 @@ from onyx.db.llm import ( fetch_default_chat_naming_model, fetch_default_craft_model, fetch_default_llm_model, + fetch_default_search_selection_model, fetch_default_vision_model, fetch_existing_llm_provider_by_id, fetch_existing_llm_providers, @@ -38,9 +39,11 @@ from onyx.db.llm import ( update_default_chat_naming_provider, update_default_craft_provider, update_default_provider, + update_default_search_selection_provider, update_default_vision_provider, update_no_default_chat_naming_provider, update_no_default_craft_provider, + update_no_default_search_selection_provider, upsert_llm_provider, validate_persona_ids_exist, ) @@ -562,6 +565,7 @@ def list_llm_providers( default_text_model = fetch_default_llm_model(db_session) default_vision_model = fetch_default_vision_model(db_session) default_chat_naming_model = fetch_default_chat_naming_model(db_session) + default_search_selection_model = fetch_default_search_selection_model(db_session) default_craft_model = fetch_default_craft_model(db_session) # page_models is opt-in: the admin UI takes one page per provider plus the # defaults, callers that omit it still get every model. @@ -573,6 +577,7 @@ def list_llm_providers( default_text_model, default_vision_model, default_chat_naming_model, + default_search_selection_model, default_craft_model, ], ) @@ -613,6 +618,9 @@ def list_llm_providers( default_text=DefaultModel.from_model_config(default_text_model), default_vision=DefaultModel.from_model_config(default_vision_model), default_chat_naming=DefaultModel.from_model_config(default_chat_naming_model), + default_search_selection=DefaultModel.from_model_config( + default_search_selection_model + ), default_craft=DefaultModel.from_model_config(default_craft_model), ) @@ -891,24 +899,26 @@ def set_provider_as_default_vision( @admin_router.post("/default-chat-naming") def set_provider_as_default_chat_naming( default_model: DefaultModel, - _: User = Depends(require_permission(Permission.FULL_ADMIN_PANEL_ACCESS)), + _: User = Depends(require_permission(Permission.MANAGE_LLMS)), db_session: Session = Depends(get_session), ) -> None: - update_default_chat_naming_provider( - provider_id=default_model.provider_id, - chat_naming_model=default_model.model_name, - db_session=db_session, - ) + try: + update_default_chat_naming_provider( + provider_id=default_model.provider_id, + chat_naming_model=default_model.model_name, + db_session=db_session, + ) + except ValueError as e: + raise OnyxError(OnyxErrorCode.VALIDATION_ERROR, str(e)) from e invalidate_provider_listing_cache() @admin_router.delete("/default-chat-naming") def clear_default_chat_naming( - _: User = Depends(require_permission(Permission.FULL_ADMIN_PANEL_ACCESS)), + _: User = Depends(require_permission(Permission.MANAGE_LLMS)), db_session: Session = Depends(get_session), ) -> None: - """Clear the dedicated naming model; auto-naming falls back to the - session's model.""" + """Clear the naming override; use the session model instead.""" update_no_default_chat_naming_provider(db_session=db_session) invalidate_provider_listing_cache() @@ -937,6 +947,32 @@ def clear_default_craft( invalidate_provider_listing_cache() +@admin_router.post("/default-search-selection") +def set_provider_as_default_search_selection( + default_model: DefaultModel, + _: User = Depends(require_permission(Permission.MANAGE_LLMS)), + db_session: Session = Depends(get_session), +) -> None: + try: + update_default_search_selection_provider( + provider_id=default_model.provider_id, + model_name=default_model.model_name, + db_session=db_session, + ) + except ValueError as e: + raise OnyxError(OnyxErrorCode.VALIDATION_ERROR, str(e)) from e + invalidate_provider_listing_cache() + + +@admin_router.delete("/default-search-selection") +def clear_default_search_selection( + _: User = Depends(require_permission(Permission.MANAGE_LLMS)), + db_session: Session = Depends(get_session), +) -> None: + update_no_default_search_selection_provider(db_session=db_session) + invalidate_provider_listing_cache() + + @admin_router.get("/auto-config") def get_auto_config( _: User = Depends(require_permission(Permission.MANAGE_LLMS)), @@ -1134,7 +1170,6 @@ def list_llm_provider_basics( ) default_text_model = fetch_default_llm_model(db_session) default_vision_model = fetch_default_vision_model(db_session) - default_chat_naming_model = fetch_default_chat_naming_model(db_session) default_craft_model = fetch_default_craft_model(db_session) # Use centralized access control logic with persona=None since we're @@ -1155,7 +1190,6 @@ def list_llm_provider_basics( [ default_text_model, default_vision_model, - default_chat_naming_model, default_craft_model, ], ) @@ -1172,7 +1206,6 @@ def list_llm_provider_basics( providers=accessible_providers, default_text=DefaultModel.from_model_config(default_text_model), default_vision=DefaultModel.from_model_config(default_vision_model), - default_chat_naming=DefaultModel.from_model_config(default_chat_naming_model), default_craft=DefaultModel.from_model_config(default_craft_model), ) cache_provider_listing( diff --git a/backend/onyx/server/manage/llm/models.py b/backend/onyx/server/manage/llm/models.py index 1580baaa89..296853a567 100644 --- a/backend/onyx/server/manage/llm/models.py +++ b/backend/onyx/server/manage/llm/models.py @@ -692,6 +692,7 @@ class LLMProviderResponse(BaseModel, Generic[T]): default_text: DefaultModel | None = None default_vision: DefaultModel | None = None default_chat_naming: DefaultModel | None = None + default_search_selection: DefaultModel | None = None default_craft: DefaultModel | None = None @classmethod @@ -702,12 +703,14 @@ class LLMProviderResponse(BaseModel, Generic[T]): default_vision: DefaultModel | None = None, default_chat_naming: DefaultModel | None = None, default_craft: DefaultModel | None = None, + default_search_selection: DefaultModel | None = None, ) -> LLMProviderResponse[T]: return cls( providers=providers, default_text=default_text, default_vision=default_vision, default_chat_naming=default_chat_naming, + default_search_selection=default_search_selection, default_craft=default_craft, ) diff --git a/backend/tests/external_dependency_unit/llm/test_llm_provider_task_defaults.py b/backend/tests/external_dependency_unit/llm/test_llm_provider_task_defaults.py new file mode 100644 index 0000000000..2b64d9a811 --- /dev/null +++ b/backend/tests/external_dependency_unit/llm/test_llm_provider_task_defaults.py @@ -0,0 +1,546 @@ +from collections.abc import Generator +from datetime import datetime, timezone +from pathlib import Path +from uuid import uuid4 + +import pytest +from alembic.config import Config +from alembic.migration import MigrationContext +from alembic.operations import Operations +from alembic.script import ScriptDirectory +from sqlalchemy import select, text +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session +from sqlalchemy.schema import CreateSchema, DropSchema + +from onyx.db.engine.sql_engine import SqlEngine +from onyx.db.enums import LLMModelFlowType +from onyx.db.llm import ( + fetch_default_chat_naming_model, + fetch_default_craft_model, + fetch_default_flows_by_model_id, + fetch_default_llm_model, + fetch_default_search_selection_model, + fetch_existing_llm_provider_by_id, + remove_llm_provider, + sync_auto_mode_models, + sync_model_configurations, + update_default_chat_naming_provider, + update_default_craft_provider, + update_default_provider, + update_default_search_selection_provider, + update_no_default_chat_naming_provider, + update_no_default_search_selection_provider, + upsert_llm_provider, +) +from onyx.db.models import Base, LLMModelFlow, ModelConfiguration +from onyx.llm.well_known_providers.auto_update_models import ( + LLMProviderRecommendation, + LLMRecommendations, +) +from onyx.llm.well_known_providers.models import SimpleKnownModel +from onyx.server.manage.llm.models import ( + LLMProviderUpsertRequest, + LLMProviderView, + ModelConfigurationUpsertRequest, + SyncModelEntry, +) +from shared_configs.contextvars import CURRENT_TENANT_ID_CONTEXTVAR + + +@pytest.fixture +def db_session(test_schema_engine: Engine) -> Generator[Session, None, None]: + with Session(test_schema_engine, expire_on_commit=False) as session: + yield session + + +@pytest.fixture(scope="module") +def test_schema_engine() -> Generator[Engine, None, None]: + SqlEngine.init_engine(pool_size=10, max_overflow=5) + engine = SqlEngine.get_engine() + schema_name = f"task_defaults_test_{uuid4().hex}" + scoped_engine = engine.execution_options(schema_translate_map={None: schema_name}) + with scoped_engine.begin() as connection: + connection.execute(CreateSchema(schema_name)) + connection.execute(text(f'SET LOCAL search_path TO "{schema_name}", public')) + Base.metadata.create_all( + connection, + tables=[ + Base.metadata.tables[table_name] + for table_name in ( + "llm_provider", + "model_configuration", + "llm_model_flow", + "image_generation_config", + "llm_provider__persona", + "llm_provider__user_group", + "persona", + "user_group", + "user", + ) + ], + ) + tenant_token = CURRENT_TENANT_ID_CONTEXTVAR.set(schema_name) + try: + yield scoped_engine + finally: + CURRENT_TENANT_ID_CONTEXTVAR.reset(tenant_token) + with engine.begin() as connection: + connection.execute(DropSchema(schema_name, cascade=True)) + + +@pytest.fixture +def provider(db_session: Session) -> Generator[LLMProviderView, None, None]: + saved = upsert_llm_provider( + LLMProviderUpsertRequest( + name=f"test-naming-{uuid4().hex}", + provider="openai", + model_configurations=[ + ModelConfigurationUpsertRequest(name="gpt-5-mini", is_visible=True), + ModelConfigurationUpsertRequest(name="gpt-5-nano", is_visible=False), + ], + ), + db_session, + ) + try: + yield saved + finally: + db_session.rollback() + remove_llm_provider(db_session, saved.id) + + +def _update_provider( + db_session: Session, + provider: LLMProviderView, + *, + mini_visible: bool = True, + nano_visible: bool = False, +) -> None: + upsert_llm_provider( + LLMProviderUpsertRequest( + id=provider.id, + name=provider.name, + provider="openai", + model_configurations=[ + ModelConfigurationUpsertRequest( + name="gpt-5-mini", is_visible=mini_visible + ), + ModelConfigurationUpsertRequest( + name="gpt-5-nano", is_visible=nano_visible + ), + ], + ), + db_session, + ) + + +@pytest.mark.parametrize( + "model_name,is_visible", [("gpt-5-mini", True), ("gpt-5-nano", False)] +) +def test_setting_chat_naming_creates_one_mapping_and_preserves_visibility( + db_session: Session, + provider: LLMProviderView, + model_name: str, + is_visible: bool, +) -> None: + update_default_chat_naming_provider(provider.id, model_name, db_session) + update_default_chat_naming_provider(provider.id, model_name, db_session) + + default = fetch_default_chat_naming_model(db_session) + assert default is not None + assert (default.llm_provider_id, default.name, default.is_visible) == ( + provider.id, + model_name, + is_visible, + ) + mappings = db_session.scalars( + select(LLMModelFlow).where( + LLMModelFlow.model_configuration_id == default.id, + LLMModelFlow.llm_model_flow_type == LLMModelFlowType.CHAT_NAMING, + ) + ).all() + assert len(mappings) == 1 + assert LLMModelFlowType.CHAT in default.llm_model_flow_types + + +def test_search_selection_persists_hidden_mapping_and_resets_independently( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-mini", db_session) + update_default_search_selection_provider(provider.id, "gpt-5-nano", db_session) + update_default_search_selection_provider(provider.id, "gpt-5-nano", db_session) + db_session.expire_all() + + selection = fetch_default_search_selection_model(db_session) + assert selection is not None + assert (selection.name, selection.is_visible) == ("gpt-5-nano", False) + mappings = db_session.scalars( + select(LLMModelFlow).where( + LLMModelFlow.model_configuration_id == selection.id, + LLMModelFlow.llm_model_flow_type == LLMModelFlowType.SEARCH_SELECTION, + ) + ).all() + assert len(mappings) == 1 + assert mappings[0].is_default + + update_no_default_chat_naming_provider(db_session) + assert fetch_default_chat_naming_model(db_session) is None + assert fetch_default_search_selection_model(db_session) is selection + update_default_chat_naming_provider(provider.id, "gpt-5-mini", db_session) + update_no_default_search_selection_provider(db_session) + update_no_default_search_selection_provider(db_session) + assert fetch_default_search_selection_model(db_session) is None + naming = fetch_default_chat_naming_model(db_session) + assert naming is not None and naming.name == "gpt-5-mini" + + +def test_search_selection_survives_edit_discovery_and_auto_sync( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_search_selection_provider(provider.id, "gpt-5-nano", db_session) + _update_provider(db_session, provider) + sync_model_configurations( + db_session, + provider.id, + [ + SyncModelEntry( + name="gpt-5-nano", display_name="GPT 5 nano", supports_reasoning=True + ) + ], + ) + stored = fetch_existing_llm_provider_by_id(provider.id, db_session) + assert stored is not None + sync_auto_mode_models( + db_session, + stored, + LLMRecommendations( + version="test", + updated_at=datetime.now(timezone.utc), + providers={ + "openai": LLMProviderRecommendation( + default_model=SimpleKnownModel(name="gpt-5-mini") + ) + }, + ), + ) + selection = fetch_default_search_selection_model(db_session) + assert selection is not None + assert (selection.name, selection.is_visible) == ("gpt-5-nano", False) + assert LLMModelFlowType.REASONING in selection.llm_model_flow_types + + +def test_search_selection_can_be_hidden_but_not_removed( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_search_selection_provider(provider.id, "gpt-5-mini", db_session) + _update_provider(db_session, provider, mini_visible=False) + selection = fetch_default_search_selection_model(db_session) + assert selection is not None and not selection.is_visible + with pytest.raises(ValueError, match="Cannot remove the default model"): + upsert_llm_provider( + LLMProviderUpsertRequest( + id=provider.id, provider="openai", model_configurations=[] + ), + db_session, + ) + + +def test_invalid_search_selection_preserves_existing_defaults( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-mini", db_session) + update_default_search_selection_provider(provider.id, "gpt-5-nano", db_session) + with pytest.raises(ValueError, match="is not a valid model"): + update_default_search_selection_provider( + provider.id, "missing-model", db_session + ) + selection = fetch_default_search_selection_model(db_session) + naming = fetch_default_chat_naming_model(db_session) + assert selection is not None and selection.name == "gpt-5-nano" + assert naming is not None and naming.name == "gpt-5-mini" + + +def test_search_selection_shared_with_chat_cannot_be_hidden( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_search_selection_provider(provider.id, "gpt-5-mini", db_session) + update_default_provider(provider.id, "gpt-5-mini", db_session) + with pytest.raises(ValueError, match="Cannot hide the default model"): + _update_provider(db_session, provider, mini_visible=False) + + +def test_replacing_and_clearing_chat_naming_preserves_chat_and_search_selection( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_provider(provider.id, "gpt-5-mini", db_session) + update_default_search_selection_provider(provider.id, "gpt-5-mini", db_session) + update_default_chat_naming_provider(provider.id, "gpt-5-mini", db_session) + update_default_chat_naming_provider(provider.id, "gpt-5-nano", db_session) + + default = fetch_default_chat_naming_model(db_session) + assert default is not None + assert (default.name, default.is_visible) == ("gpt-5-nano", False) + assert ( + sum( + LLMModelFlowType.CHAT_NAMING in flows + for flows in fetch_default_flows_by_model_id(db_session).values() + ) + == 1 + ) + + update_no_default_chat_naming_provider(db_session) + update_no_default_chat_naming_provider(db_session) + + assert fetch_default_chat_naming_model(db_session) is None + for model in [ + fetch_default_llm_model(db_session), + fetch_default_search_selection_model(db_session), + ]: + assert model is not None + assert (model.llm_provider_id, model.name, model.is_visible) == ( + provider.id, + "gpt-5-mini", + True, + ) + + +def test_hidden_chat_naming_default_survives_provider_edit_and_discovery( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-nano", db_session) + + _update_provider(db_session, provider) + sync_model_configurations( + db_session, + provider.id, + [ + SyncModelEntry( + name="gpt-5-nano", display_name="GPT 5 nano", supports_reasoning=True + ) + ], + ) + + default = fetch_default_chat_naming_model(db_session) + assert default is not None + assert (default.name, default.is_visible) == ("gpt-5-nano", False) + assert { + LLMModelFlowType.CHAT, + LLMModelFlowType.REASONING, + LLMModelFlowType.CHAT_NAMING, + } <= set(default.llm_model_flow_types) + + +def test_chat_naming_only_default_can_be_hidden_but_shared_chat_default_cannot( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-mini", db_session) + _update_provider(db_session, provider, mini_visible=False) + default = fetch_default_chat_naming_model(db_session) + assert default is not None + assert (default.name, default.is_visible) == ("gpt-5-mini", False) + + update_default_provider(provider.id, "gpt-5-mini", db_session) + with pytest.raises(ValueError, match="Cannot hide the default model"): + _update_provider(db_session, provider, mini_visible=False) + + +def test_chat_naming_default_cannot_be_removed_during_provider_edit( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-nano", db_session) + with pytest.raises(ValueError, match="Cannot remove the default model"): + upsert_llm_provider( + LLMProviderUpsertRequest( + id=provider.id, + provider="openai", + model_configurations=[ + ModelConfigurationUpsertRequest(name="gpt-5-mini", is_visible=True) + ], + ), + db_session, + ) + + +def test_auto_sync_does_not_restore_hidden_chat_naming_only_default( + db_session: Session, provider: LLMProviderView +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-nano", db_session) + stored_provider = fetch_existing_llm_provider_by_id(provider.id, db_session) + assert stored_provider is not None + + sync_auto_mode_models( + db_session, + stored_provider, + LLMRecommendations( + version="1", + updated_at=datetime.now(timezone.utc), + providers={ + "openai": LLMProviderRecommendation( + default_model=SimpleKnownModel(name="gpt-5-mini") + ) + }, + ), + ) + + default = fetch_default_chat_naming_model(db_session) + assert default is not None + assert (default.name, default.is_visible) == ("gpt-5-nano", False) + + +@pytest.mark.parametrize("model_name", ["missing-model", ""]) +def test_invalid_selection_preserves_existing_chat_naming_default( + db_session: Session, provider: LLMProviderView, model_name: str +) -> None: + update_default_chat_naming_provider(provider.id, "gpt-5-nano", db_session) + with pytest.raises(ValueError, match="is not a valid model"): + update_default_chat_naming_provider(provider.id, model_name, db_session) + + default = fetch_default_chat_naming_model(db_session) + assert default is not None + assert (default.llm_provider_id, default.name, default.is_visible) == ( + provider.id, + "gpt-5-nano", + False, + ) + + +@pytest.mark.parametrize("model_name", ["gpt-5-mini", "gpt-5-nano"]) +def test_setting_craft_creates_one_mapping_and_makes_model_visible( + db_session: Session, provider: LLMProviderView, model_name: str +) -> None: + assert fetch_default_craft_model(db_session) is None + assert ( + db_session.scalars( + select(LLMModelFlow).where( + LLMModelFlow.llm_model_flow_type == LLMModelFlowType.CRAFT + ) + ).all() + == [] + ) + + update_default_craft_provider(provider.id, model_name, db_session) + update_default_craft_provider(provider.id, model_name, db_session) + db_session.rollback() + + default = fetch_default_craft_model(db_session) + assert default is not None + assert (default.llm_provider_id, default.name, default.is_visible) == ( + provider.id, + model_name, + True, + ) + mappings = db_session.scalars( + select(LLMModelFlow).where( + LLMModelFlow.llm_model_flow_type == LLMModelFlowType.CRAFT + ) + ).all() + assert [(flow.model_configuration_id, flow.is_default) for flow in mappings] == [ + (default.id, True) + ] + assert LLMModelFlowType.CHAT in default.llm_model_flow_types + + +@pytest.mark.parametrize("model_name", ["missing-model", ""]) +def test_invalid_selection_preserves_existing_craft_default( + db_session: Session, provider: LLMProviderView, model_name: str +) -> None: + update_default_craft_provider(provider.id, "gpt-5-nano", db_session) + with pytest.raises(ValueError) as exc_info: + update_default_craft_provider(provider.id, model_name, db_session) + + assert str(exc_info.value) == ( + f"Model '{model_name}' is not a valid model for provider_id={provider.id}" + ) + default = fetch_default_craft_model(db_session) + assert default is not None + assert (default.llm_provider_id, default.name, default.is_visible) == ( + provider.id, + "gpt-5-nano", + True, + ) + + +@pytest.mark.parametrize( + ("rows", "expected"), + [ + ( + [("gpt-5-mini", "FAST", False), ("gpt-5-nano", "FAST", True)], + {("gpt-5-mini", False), ("gpt-5-nano", True)}, + ), + ( + [ + ("gpt-5-mini", "CHAT_NAMING", True), + ("gpt-5-mini", "FAST", False), + ("gpt-5-nano", "FAST", True), + ], + {("gpt-5-mini", True), ("gpt-5-nano", False)}, + ), + ( + [ + ("gpt-5-mini", "CHAT_NAMING", False), + ("gpt-5-nano", "CHAT_NAMING", False), + ("gpt-5-nano", "FAST", True), + ], + {("gpt-5-mini", False), ("gpt-5-nano", True)}, + ), + ([], set()), + ([("gpt-5-mini", "CHAT_NAMING", True)], {("gpt-5-mini", True)}), + ( + [("gpt-5-mini", "CHAT_NAMING", True), ("gpt-5-mini", "FAST", True)], + {("gpt-5-mini", True)}, + ), + ], +) +def test_migrate_draft_fast_preserves_naming_and_search_defaults( + db_session: Session, + provider: LLMProviderView, + rows: list[tuple[str, str, bool]], + expected: set[tuple[str, bool]], +) -> None: + schema = CURRENT_TENANT_ID_CONTEXTVAR.get() + db_session.execute(text(f'SET LOCAL search_path TO "{schema}", public')) + models = db_session.scalars( + select(ModelConfiguration).where( + ModelConfiguration.llm_provider_id == provider.id + ) + ).all() + model_ids = {model.name: model.id for model in models} + visibility_before = {model.name: model.is_visible for model in models} + for name, flow, is_default in [*rows, ("gpt-5-nano", "SEARCH_SELECTION", True)]: + db_session.execute( + text( + "INSERT INTO llm_model_flow (model_configuration_id, llm_model_flow_type, is_default) " + "VALUES (:model_id, :flow, :is_default)" + ), + {"model_id": model_ids[name], "flow": flow, "is_default": is_default}, + ) + + config = Config() + config.set_main_option( + "script_location", str(Path(__file__).parents[3] / "alembic") + ) + revision = ScriptDirectory.from_config(config).get_revision("c92b67e56ec4") + assert revision is not None + with Operations.context(MigrationContext.configure(db_session.connection())): + revision.module.upgrade() + revision.module.upgrade() + revision.module.downgrade() + + db_session.expire_all() + assert ( + db_session.scalar( + text( + "SELECT COUNT(*) FROM llm_model_flow WHERE llm_model_flow_type = 'FAST'" + ) + ) + == 0 + ) + naming_rows = db_session.execute( + select(ModelConfiguration.name, LLMModelFlow.is_default) + .join(LLMModelFlow) + .where(LLMModelFlow.llm_model_flow_type == LLMModelFlowType.CHAT_NAMING) + ).tuples() + assert set(naming_rows) == expected + selection = fetch_default_search_selection_model(db_session) + assert selection is not None and selection.name == "gpt-5-nano" + assert {model.name: model.is_visible for model in models} == visibility_before diff --git a/backend/tests/unit/onyx/server/manage/llm/test_default_chat_naming.py b/backend/tests/unit/onyx/server/manage/llm/test_default_chat_naming.py new file mode 100644 index 0000000000..48564d0d47 --- /dev/null +++ b/backend/tests/unit/onyx/server/manage/llm/test_default_chat_naming.py @@ -0,0 +1,263 @@ +from collections.abc import Generator +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy.orm import Session + +from onyx.auth.users import current_user +from onyx.db.enums import LLMModelFlowType, Permission, PermissionAuthority +from onyx.db.llm import ModelConfigurationWindow +from onyx.db.models import LLMModelFlow, LLMProvider, ModelConfiguration, Persona, User +from onyx.error_handling.exceptions import register_onyx_exception_handlers +from onyx.server.manage.llm import api +from onyx.server.manage.llm.models import DefaultModel +from onyx.server.manage.llm.provider_cache import ProviderListingCacheLookup +from onyx.utils.sensitive import make_mock_sensitive_value + + +@pytest.fixture +def client() -> Generator[TestClient, None, None]: + app = FastAPI() + app.include_router(api.admin_router) + register_onyx_exception_handlers(app) + app.dependency_overrides[current_user] = lambda: User() + app.dependency_overrides[api.get_session] = lambda: MagicMock(spec=Session) + with TestClient(app) as test_client: + yield test_client + + +def test_manage_llms_permission_can_set_and_clear_chat_naming( + client: TestClient, +) -> None: + def permission_for_user(_: User, permission: Permission) -> PermissionAuthority: + return ( + PermissionAuthority.GLOBAL + if permission == Permission.MANAGE_LLMS + else PermissionAuthority.NONE + ) + + with ( + patch("onyx.auth.permissions.has_permission", side_effect=permission_for_user), + patch.object(api, "update_default_chat_naming_provider") as set_default, + patch.object(api, "update_no_default_chat_naming_provider") as clear_default, + patch.object(api, "invalidate_provider_listing_cache") as invalidate, + ): + response = client.post( + "/admin/llm/default-chat-naming", + json={"provider_id": 7, "model_name": "gpt-5-nano"}, + ) + assert (response.status_code, response.json()) == (200, None) + assert set_default.call_args.kwargs["provider_id"] == 7 + assert set_default.call_args.kwargs["chat_naming_model"] == "gpt-5-nano" + assert isinstance(set_default.call_args.kwargs["db_session"], MagicMock) + + response = client.delete("/admin/llm/default-chat-naming") + assert (response.status_code, response.json()) == (200, None) + assert isinstance(clear_default.call_args.kwargs["db_session"], MagicMock) + assert invalidate.call_count == 2 + + +@pytest.mark.parametrize("method", ["post", "delete"]) +def test_retired_fast_endpoints_are_not_available( + client: TestClient, method: str +) -> None: + response = client.request( + method, + "/admin/llm/default-fast", + json={"provider_id": 7, "model_name": "gpt-5-nano"}, + ) + assert response.status_code == 404 + + +@pytest.mark.parametrize("method", ["post", "delete"]) +def test_chat_naming_mutation_rejects_users_without_manage_llms( + client: TestClient, method: str +) -> None: + with ( + patch( + "onyx.auth.permissions.has_permission", + return_value=PermissionAuthority.NONE, + ), + patch.object(api, "update_default_chat_naming_provider") as set_default, + patch.object(api, "update_no_default_chat_naming_provider") as clear_default, + ): + response = client.request( + method, + "/admin/llm/default-chat-naming", + json={"provider_id": 7, "model_name": "gpt-5-nano"}, + ) + assert response.status_code == 403 + assert response.json()["error_code"] == "INSUFFICIENT_PERMISSIONS" + set_default.assert_not_called() + clear_default.assert_not_called() + + +def test_invalid_chat_naming_selection_returns_a_validation_error( + client: TestClient, +) -> None: + with ( + patch( + "onyx.auth.permissions.has_permission", + return_value=PermissionAuthority.GLOBAL, + ), + patch.object( + api, + "update_default_chat_naming_provider", + side_effect=ValueError("Unknown model"), + ), + patch.object(api, "invalidate_provider_listing_cache") as invalidate, + ): + response = client.post( + "/admin/llm/default-chat-naming", + json={"provider_id": 7, "model_name": "missing-model"}, + ) + assert response.status_code == 400 + assert response.json() == { + "error_code": "VALIDATION_ERROR", + "detail": "Unknown model", + } + invalidate.assert_not_called() + + +def _provider_with_chat_naming() -> tuple[ + LLMProvider, ModelConfiguration, ModelConfiguration +]: + provider = LLMProvider( + id=7, + name="Workspace provider", + provider="openai", + is_public=True, + is_auto_mode=False, + api_key=make_mock_sensitive_value("sk-test-workspace-key-123456789"), + custom_config={"api_key": "workspace-custom-secret"}, + groups=[], + personas=[], + ) + visible = ModelConfiguration( + id=70, + llm_provider=provider, + llm_provider_id=provider.id, + name="gpt-5-mini", + display_name="Chat model", + is_visible=True, + is_router=False, + max_input_tokens=16000, + llm_model_flows=[LLMModelFlow(llm_model_flow_type=LLMModelFlowType.CHAT)], + ) + hidden_chat_naming = ModelConfiguration( + id=71, + llm_provider=provider, + llm_provider_id=provider.id, + name="gpt-5-nano", + display_name="Utility model", + is_visible=False, + is_router=False, + max_input_tokens=8000, + llm_model_flows=[ + LLMModelFlow(llm_model_flow_type=LLMModelFlowType.CHAT), + LLMModelFlow( + llm_model_flow_type=LLMModelFlowType.CHAT_NAMING, is_default=True + ), + ], + ) + return provider, visible, hidden_chat_naming + + +@pytest.fixture +def listing_defaults() -> Generator[None, None, None]: + with ( + patch.object(api, "fetch_default_llm_model", return_value=None), + patch.object(api, "fetch_default_vision_model", return_value=None), + patch.object(api, "fetch_default_chat_naming_model", return_value=None), + patch.object(api, "fetch_default_craft_model", return_value=None), + patch.object(api, "fetch_default_search_selection_model", return_value=None), + patch.object(api, "has_global_permission", return_value=False), + patch.object(api, "fetch_user_group_ids", return_value=set()), + patch.object( + api, + "get_cached_provider_listing", + return_value=ProviderListingCacheLookup(response=None, version=None), + ), + patch.object(api, "cache_provider_listing"), + ): + yield + + +@pytest.mark.usefixtures("listing_defaults") +def test_admin_listing_pins_hidden_chat_naming_and_masks_credentials() -> None: + provider, visible, naming = _provider_with_chat_naming() + with ( + patch.object(api, "fetch_existing_llm_providers", return_value=[provider]), + patch.object(api, "fetch_default_chat_naming_model", return_value=naming), + patch.object( + api, + "fetch_model_configurations_page", + return_value={ + 7: ModelConfigurationWindow( + model_configurations=[visible], next_offset=1 + ) + }, + ), + ): + response = api.list_llm_providers( + include_image_gen=False, + page_models=True, + _=User(), + db_session=MagicMock(spec=Session), + ) + + assert response.default_chat_naming == DefaultModel( + provider_id=7, model_name="gpt-5-nano" + ) + assert "default_fast" not in response.model_dump() + assert [ + (model.name, model.is_visible) + for model in response.providers[0].model_configurations + ] == [("gpt-5-mini", True), ("gpt-5-nano", False)] + assert response.providers[0].next_model_configuration_offset == 1 + payload = response.model_dump_json() + assert "sk-test-workspace-key-123456789" not in payload + assert "workspace-custom-secret" not in payload + + +@pytest.mark.usefixtures("listing_defaults") +@pytest.mark.parametrize("persona_id", [None, 42]) +@pytest.mark.parametrize("provider_is_public", [False, True]) +def test_user_listings_do_not_publish_or_pin_chat_naming( + persona_id: int | None, provider_is_public: bool +) -> None: + provider, visible, naming = _provider_with_chat_naming() + provider.is_public = provider_is_public + with ( + patch.object(api, "fetch_existing_llm_providers", return_value=[provider]), + patch.object(api, "fetch_default_chat_naming_model", return_value=naming), + patch.object( + api, + "fetch_model_configurations_page", + return_value={ + 7: ModelConfigurationWindow( + model_configurations=[visible], next_offset=1 + ) + }, + ), + patch.object(api, "_fetch_persona_for_listing", return_value=Persona(id=42)), + ): + if persona_id is None: + response = api.list_llm_provider_basics(User(), MagicMock(spec=Session)) + else: + response = api.list_llm_providers_for_persona( + persona_id, User(), MagicMock(spec=Session) + ) + + assert response.default_chat_naming is None + assert "gpt-5-nano" not in response.model_dump_json() + assert [item.id for item in response.providers] == ( + [7] if provider_is_public else [] + ) + if provider_is_public: + assert [model.name for model in response.providers[0].model_configurations] == [ + "gpt-5-mini" + ] + assert response.providers[0].next_model_configuration_offset == 1 diff --git a/backend/tests/unit/onyx/server/manage/llm/test_default_search_selection.py b/backend/tests/unit/onyx/server/manage/llm/test_default_search_selection.py new file mode 100644 index 0000000000..f13a884ed8 --- /dev/null +++ b/backend/tests/unit/onyx/server/manage/llm/test_default_search_selection.py @@ -0,0 +1,183 @@ +from collections.abc import Generator +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy.orm import Session + +from onyx.auth.users import current_user +from onyx.db.enums import LLMModelFlowType, Permission, PermissionAuthority +from onyx.db.llm import ModelConfigurationWindow +from onyx.db.models import Persona, User +from onyx.error_handling.exceptions import register_onyx_exception_handlers +from onyx.server.manage.llm import api +from onyx.server.manage.llm.models import DefaultModel +from tests.unit.onyx.server.manage.llm.test_default_chat_naming import ( + _provider_with_chat_naming, +) +from tests.unit.onyx.server.manage.llm.test_default_chat_naming import ( + listing_defaults as listing_defaults, +) + + +@pytest.fixture +def client() -> Generator[TestClient, None, None]: + app = FastAPI() + app.include_router(api.admin_router) + register_onyx_exception_handlers(app) + app.dependency_overrides[current_user] = lambda: User() + app.dependency_overrides[api.get_session] = lambda: MagicMock(spec=Session) + with TestClient(app) as test_client: + yield test_client + + +def test_manage_llms_can_set_and_clear_search_selection(client: TestClient) -> None: + with ( + patch( + "onyx.auth.permissions.has_permission", + side_effect=lambda _, permission: ( + PermissionAuthority.GLOBAL + if permission == Permission.MANAGE_LLMS + else PermissionAuthority.NONE + ), + ), + patch.object(api, "update_default_search_selection_provider") as set_default, + patch.object( + api, "update_no_default_search_selection_provider" + ) as clear_default, + patch.object(api, "update_default_chat_naming_provider") as set_chat_naming, + patch.object( + api, "update_no_default_chat_naming_provider" + ) as clear_chat_naming, + patch.object(api, "invalidate_provider_listing_cache") as invalidate, + ): + response = client.post( + "/admin/llm/default-search-selection", + json={"provider_id": 7, "model_name": "gpt-5-mini"}, + ) + assert (response.status_code, response.json()) == (200, None) + assert set_default.call_args.kwargs["provider_id"] == 7 + assert set_default.call_args.kwargs["model_name"] == "gpt-5-mini" + response = client.delete("/admin/llm/default-search-selection") + assert (response.status_code, response.json()) == (200, None) + clear_default.assert_called_once() + assert invalidate.call_count == 2 + set_chat_naming.assert_not_called() + clear_chat_naming.assert_not_called() + + +@pytest.mark.parametrize("method", ["post", "delete"]) +def test_search_selection_requires_manage_llms(client: TestClient, method: str) -> None: + with ( + patch( + "onyx.auth.permissions.has_permission", + return_value=PermissionAuthority.NONE, + ), + patch.object(api, "update_default_search_selection_provider") as set_default, + patch.object( + api, "update_no_default_search_selection_provider" + ) as clear_default, + ): + response = client.request( + method, + "/admin/llm/default-search-selection", + json={"provider_id": 7, "model_name": "gpt-5-mini"}, + ) + assert response.status_code == 403 + set_default.assert_not_called() + clear_default.assert_not_called() + + +def test_invalid_search_selection_returns_validation_error(client: TestClient) -> None: + with ( + patch( + "onyx.auth.permissions.has_permission", + return_value=PermissionAuthority.GLOBAL, + ), + patch.object( + api, + "update_default_search_selection_provider", + side_effect=ValueError("Unknown model"), + ), + patch.object(api, "invalidate_provider_listing_cache") as invalidate, + ): + response = client.post( + "/admin/llm/default-search-selection", + json={"provider_id": 7, "model_name": "missing-model"}, + ) + assert response.status_code == 400 + assert response.json() == { + "error_code": "VALIDATION_ERROR", + "detail": "Unknown model", + } + invalidate.assert_not_called() + + +@pytest.mark.usefixtures("listing_defaults") +def test_admin_pins_hidden_search_selection_without_changing_chat_naming() -> None: + provider, visible, hidden = _provider_with_chat_naming() + hidden.llm_model_flows[-1].llm_model_flow_type = LLMModelFlowType.SEARCH_SELECTION + with ( + patch.object(api, "fetch_existing_llm_providers", return_value=[provider]), + patch.object(api, "fetch_default_chat_naming_model", return_value=visible), + patch.object(api, "fetch_default_search_selection_model", return_value=hidden), + patch.object( + api, + "fetch_model_configurations_page", + return_value={ + 7: ModelConfigurationWindow( + model_configurations=[visible], next_offset=1 + ) + }, + ), + ): + response = api.list_llm_providers( + include_image_gen=False, + page_models=True, + _=User(), + db_session=MagicMock(spec=Session), + ) + assert response.default_chat_naming == DefaultModel( + provider_id=7, model_name="gpt-5-mini" + ) + assert response.default_search_selection == DefaultModel( + provider_id=7, model_name="gpt-5-nano" + ) + assert [ + (model.name, model.is_visible) + for model in response.providers[0].model_configurations + ] == [("gpt-5-mini", True), ("gpt-5-nano", False)] + assert "sk-test-workspace-key-123456789" not in response.model_dump_json() + + +@pytest.mark.usefixtures("listing_defaults") +@pytest.mark.parametrize("persona_id", [None, 42]) +def test_user_listing_does_not_publish_search_selection(persona_id: int | None) -> None: + provider, visible, hidden = _provider_with_chat_naming() + with ( + patch.object(api, "fetch_existing_llm_providers", return_value=[provider]), + patch.object( + api, "fetch_default_search_selection_model", return_value=hidden + ) as fetch, + patch.object( + api, + "fetch_model_configurations_page", + return_value={ + 7: ModelConfigurationWindow( + model_configurations=[visible], next_offset=1 + ) + }, + ), + patch.object(api, "_fetch_persona_for_listing", return_value=Persona(id=42)), + ): + response = ( + api.list_llm_provider_basics(User(), MagicMock(spec=Session)) + if persona_id is None + else api.list_llm_providers_for_persona( + persona_id, User(), MagicMock(spec=Session) + ) + ) + assert response.default_search_selection is None + assert "gpt-5-nano" not in response.model_dump_json() + fetch.assert_not_called() diff --git a/backend/tests/unit/onyx/server/manage/llm/test_provider_cache.py b/backend/tests/unit/onyx/server/manage/llm/test_provider_cache.py index 3b777d9e3b..df02964eb6 100644 --- a/backend/tests/unit/onyx/server/manage/llm/test_provider_cache.py +++ b/backend/tests/unit/onyx/server/manage/llm/test_provider_cache.py @@ -51,6 +51,8 @@ def _make_response() -> LLMProviderResponse[LLMProviderDescriptor]: providers=[descriptor], default_text=DefaultModel(provider_id=1, model_name="gpt-5-mini"), default_vision=None, + default_chat_naming=DefaultModel(provider_id=1, model_name="gpt-5-nano"), + default_search_selection=DefaultModel(provider_id=1, model_name="gpt-5-mini"), ) diff --git a/backend/tests/unit/onyx/server/route_contract_manifest.json b/backend/tests/unit/onyx/server/route_contract_manifest.json index de3f6476d6..7286432397 100644 --- a/backend/tests/unit/onyx/server/route_contract_manifest.json +++ b/backend/tests/unit/onyx/server/route_contract_manifest.json @@ -7,8 +7,9 @@ "DELETE /admin/image-generation/config/{image_provider_id}": "admin", "DELETE /admin/image-generation/config/{image_provider_id}/default": "admin", "DELETE /admin/input_prompt/{input_prompt_id}": "admin", - "DELETE /admin/llm/default-chat-naming": "admin", + "DELETE /admin/llm/default-chat-naming": "manage:llms", "DELETE /admin/llm/default-craft": "manage:llms", + "DELETE /admin/llm/default-search-selection": "manage:llms", "DELETE /admin/llm/provider/{provider_id}": "manage:llms", "DELETE /admin/mcp/server/{server_id}": "manage:actions +scope", "DELETE /admin/oauth-config/{oauth_config_id}": "manage:actions +scope", @@ -456,8 +457,9 @@ "POST /admin/llm/bedrock/available-models": "manage:llms", "POST /admin/llm/bifrost/available-models": "manage:llms", "POST /admin/llm/default": "manage:llms", - "POST /admin/llm/default-chat-naming": "admin", + "POST /admin/llm/default-chat-naming": "manage:llms", "POST /admin/llm/default-craft": "manage:llms", + "POST /admin/llm/default-search-selection": "manage:llms", "POST /admin/llm/default-vision": "manage:llms", "POST /admin/llm/litellm/available-models": "manage:llms", "POST /admin/llm/lm-studio/available-models": "manage:llms",