mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-19 01:50:51 -05:00
fix(indexing): never caption with a vision model nobody chose (#14965)
This commit is contained in:
1 parent
1d72e59f06
commit
1a1868531e
6 files changed
+92
-166
No files matched your search
@@ -840,9 +840,9 @@ def _get_image_summarization_llm(
|
||||
llm = get_default_llm_with_vision()
|
||||
if llm is None:
|
||||
logger.warning(
|
||||
"Image analysis is enabled but no vision-capable LLM is "
|
||||
"available — images will not be summarized. Configure a "
|
||||
"vision model in the admin LLM settings."
|
||||
"Image analysis is enabled but no usable captioning model is "
|
||||
"available — images will not be summarized. Check the captioning "
|
||||
"model under Index Settings."
|
||||
)
|
||||
return llm
|
||||
|
||||
|
||||
+33
-92
@@ -6,14 +6,13 @@ from sqlalchemy.orm import Session
|
||||
from onyx.auth.permissions import has_global_permission
|
||||
from onyx.configs.model_configs import GEN_AI_TEMPERATURE
|
||||
from onyx.db.engine.sql_engine import get_session_with_current_tenant
|
||||
from onyx.db.enums import LLMModelFlowType, Permission
|
||||
from onyx.db.enums import Permission
|
||||
from onyx.db.llm import (
|
||||
can_user_access_llm_provider,
|
||||
fetch_default_contextual_rag_model,
|
||||
fetch_default_llm_model,
|
||||
fetch_default_vision_model,
|
||||
fetch_existing_llm_provider,
|
||||
fetch_existing_models,
|
||||
fetch_model_configuration_by_id,
|
||||
fetch_user_group_ids,
|
||||
)
|
||||
@@ -245,105 +244,47 @@ def get_default_llm_with_vision(
|
||||
temperature: float | None = None,
|
||||
additional_headers: dict[str, str] | None = None,
|
||||
) -> LLM | None:
|
||||
"""Get an LLM that supports image input, with the following priority:
|
||||
1. Use the designated default vision provider if it exists and supports image input
|
||||
2. Fall back to the first LLM provider that supports image input
|
||||
"""The designated default vision model, or None.
|
||||
|
||||
Returns None if no providers exist or if no provider supports images.
|
||||
There is deliberately no fallback. With no default set, image captioning
|
||||
is off: picking an arbitrary image-capable model would spend money on a
|
||||
model nobody chose.
|
||||
"""
|
||||
with get_session_with_current_tenant() as db_session:
|
||||
default_model = fetch_default_vision_model(db_session)
|
||||
if default_model is None:
|
||||
logger.warning(
|
||||
"No default vision model is set — image summarization will be "
|
||||
"disabled. Pick a captioning model under Index Settings."
|
||||
)
|
||||
return None
|
||||
|
||||
def create_vision_llm(provider: LLMProviderView, model: str) -> LLM:
|
||||
"""Helper to create an LLM if the provider supports image input."""
|
||||
if not model_supports_image_input(
|
||||
default_model.name,
|
||||
default_model.llm_provider.provider,
|
||||
default_model.llm_provider.deployment_name,
|
||||
):
|
||||
logger.warning(
|
||||
"Default vision model %s (provider=%s) does not support image "
|
||||
"input — image summarization will be disabled",
|
||||
default_model.name,
|
||||
default_model.llm_provider.provider,
|
||||
)
|
||||
return None
|
||||
|
||||
logger.info(
|
||||
"Using default vision model: %s (provider=%s)",
|
||||
default_model.name,
|
||||
default_model.llm_provider.provider,
|
||||
)
|
||||
return llm_from_provider(
|
||||
model_name=model,
|
||||
llm_provider=provider,
|
||||
model_name=default_model.name,
|
||||
llm_provider=LLMProviderView.from_model(default_model.llm_provider),
|
||||
timeout=timeout,
|
||||
temperature=temperature,
|
||||
additional_headers=additional_headers,
|
||||
)
|
||||
|
||||
provider_map = {}
|
||||
with get_session_with_current_tenant() as db_session:
|
||||
# Try the default vision provider first
|
||||
default_model = fetch_default_vision_model(db_session)
|
||||
if default_model:
|
||||
if model_supports_image_input(
|
||||
default_model.name,
|
||||
default_model.llm_provider.provider,
|
||||
default_model.llm_provider.deployment_name,
|
||||
):
|
||||
logger.info(
|
||||
"Using default vision model: %s (provider=%s)",
|
||||
default_model.name,
|
||||
default_model.llm_provider.provider,
|
||||
)
|
||||
return create_vision_llm(
|
||||
LLMProviderView.from_model(default_model.llm_provider),
|
||||
default_model.name,
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
"Default vision model %s (provider=%s) does not support "
|
||||
"image input — falling back to searching all providers",
|
||||
default_model.name,
|
||||
default_model.llm_provider.provider,
|
||||
)
|
||||
|
||||
# Fall back to searching all providers
|
||||
models = fetch_existing_models(
|
||||
db_session=db_session,
|
||||
flow_types=[LLMModelFlowType.VISION, LLMModelFlowType.CHAT],
|
||||
)
|
||||
|
||||
if not models:
|
||||
logger.warning(
|
||||
"No LLM models with VISION or CHAT flow type found — "
|
||||
"image summarization will be disabled"
|
||||
)
|
||||
return None
|
||||
|
||||
for model in models:
|
||||
if model.llm_provider_id not in provider_map:
|
||||
provider_map[model.llm_provider_id] = LLMProviderView.from_model(
|
||||
model.llm_provider
|
||||
)
|
||||
|
||||
# Search for viable vision model followed by chat models
|
||||
# Sort models from VISION to CHAT priority
|
||||
sorted_models = sorted(
|
||||
models,
|
||||
key=lambda x: (
|
||||
LLMModelFlowType.VISION in x.llm_model_flow_types,
|
||||
LLMModelFlowType.CHAT in x.llm_model_flow_types,
|
||||
),
|
||||
reverse=True,
|
||||
)
|
||||
|
||||
for model in sorted_models:
|
||||
if model_supports_image_input(
|
||||
model.name, model.llm_provider.provider, model.llm_provider.deployment_name
|
||||
):
|
||||
logger.info(
|
||||
"Using fallback vision model: %s (provider=%s)",
|
||||
model.name,
|
||||
model.llm_provider.provider,
|
||||
)
|
||||
return create_vision_llm(
|
||||
provider_map[model.llm_provider_id],
|
||||
model.name,
|
||||
)
|
||||
|
||||
checked_models = [
|
||||
f"{m.name} (provider={m.llm_provider.provider})" for m in sorted_models
|
||||
]
|
||||
logger.warning(
|
||||
"No vision-capable model found among %d candidates: %s — "
|
||||
"image summarization will be disabled",
|
||||
len(sorted_models),
|
||||
", ".join(checked_models),
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def llm_from_provider(
|
||||
model_name: str,
|
||||
|
||||
@@ -159,6 +159,24 @@ class LLMProviderManager:
|
||||
if not verify_deleted:
|
||||
raise ValueError(f"LLM Provider {llm_provider.id} not found")
|
||||
|
||||
@staticmethod
|
||||
def set_default_vision(
|
||||
provider_id: int,
|
||||
user_performing_action: DATestUser,
|
||||
model_name: str,
|
||||
) -> None:
|
||||
"""Point image captioning at this provider's model. Without a default
|
||||
vision model, indexing skips captions entirely."""
|
||||
response = client.post(
|
||||
f"{API_SERVER_URL}/admin/llm/default-vision",
|
||||
json={
|
||||
"provider_id": provider_id,
|
||||
"model_name": model_name,
|
||||
},
|
||||
headers=user_performing_action.headers,
|
||||
)
|
||||
response.raise_for_status()
|
||||
|
||||
@staticmethod
|
||||
def get_default_model(
|
||||
user_performing_action: DATestUser | None = None,
|
||||
|
||||
@@ -35,10 +35,16 @@ def test_image_indexing(
|
||||
file_path=test_file_path, file_name=FILE_NAME, user_performing_action=admin_user
|
||||
)
|
||||
|
||||
LLMProviderManager.create(
|
||||
llm_provider = LLMProviderManager.create(
|
||||
name="test_llm",
|
||||
user_performing_action=admin_user,
|
||||
)
|
||||
assert llm_provider.default_model_name is not None
|
||||
LLMProviderManager.set_default_vision(
|
||||
llm_provider.id,
|
||||
user_performing_action=admin_user,
|
||||
model_name=llm_provider.default_model_name,
|
||||
)
|
||||
|
||||
SettingsManager.update_settings(
|
||||
DATestSettings(
|
||||
@@ -133,10 +139,16 @@ def test_docx_image_indexing(
|
||||
user_performing_action=admin_user,
|
||||
)
|
||||
|
||||
LLMProviderManager.create(
|
||||
llm_provider = LLMProviderManager.create(
|
||||
name="test_llm_docx",
|
||||
user_performing_action=admin_user,
|
||||
)
|
||||
assert llm_provider.default_model_name is not None
|
||||
LLMProviderManager.set_default_vision(
|
||||
llm_provider.id,
|
||||
user_performing_action=admin_user,
|
||||
model_name=llm_provider.default_model_name,
|
||||
)
|
||||
|
||||
SettingsManager.update_settings(
|
||||
DATestSettings(
|
||||
|
||||
@@ -20,6 +20,7 @@ from onyx.llm.well_known_providers.llm_provider_options import (
|
||||
from onyx.server.manage.llm.models import ModelConfigurationUpsertRequest
|
||||
from tests.integration.common_utils.constants import API_SERVER_URL
|
||||
from tests.integration.common_utils.http_client import client
|
||||
from tests.integration.common_utils.managers.llm_provider import LLMProviderManager
|
||||
from tests.integration.common_utils.managers.user import UserManager
|
||||
from tests.integration.common_utils.test_models import DATestUser
|
||||
|
||||
@@ -630,7 +631,7 @@ def test_delete_default_vision_provider_clears_vision_default(
|
||||
)
|
||||
assert vision_response.status_code == 200
|
||||
vision_provider = vision_response.json()
|
||||
_set_default_vision_provider(admin_user, vision_provider["id"], "gpt-4o")
|
||||
LLMProviderManager.set_default_vision(vision_provider["id"], admin_user, "gpt-4o")
|
||||
|
||||
# Verify vision default is set
|
||||
data = _get_providers_admin(admin_user)
|
||||
@@ -1375,21 +1376,6 @@ def _set_default_provider(
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def _set_default_vision_provider(
|
||||
admin_user: DATestUser, provider_id: int, vision_model: str | None = None
|
||||
) -> None:
|
||||
"""Utility function to set a provider as the default vision provider."""
|
||||
response = client.post(
|
||||
f"{API_SERVER_URL}/admin/llm/default-vision",
|
||||
json={
|
||||
"provider_id": provider_id,
|
||||
"model_name": vision_model,
|
||||
},
|
||||
headers=admin_user.headers,
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_multiple_providers_default_switching(
|
||||
reset: None, # noqa: ARG001
|
||||
) -> None: # noqa: ARG001
|
||||
@@ -1859,8 +1845,8 @@ def test_default_provider_and_vision_provider_selection(
|
||||
_set_default_provider(admin_user, provider_1["id"], provider_1_non_vision_model)
|
||||
|
||||
# Step 4: Set provider 2 with a specific vision model as the default vision provider
|
||||
_set_default_vision_provider(
|
||||
admin_user, provider_2["id"], provider_2_vision_model_1
|
||||
LLMProviderManager.set_default_vision(
|
||||
provider_2["id"], admin_user, provider_2_vision_model_1
|
||||
)
|
||||
|
||||
# Step 5: Verify via admin endpoint
|
||||
@@ -2211,8 +2197,8 @@ def test_all_three_provider_types_no_mixup(reset: None) -> None: # noqa: ARG001
|
||||
vision_provider = create_vision_response.json()
|
||||
|
||||
# Set as default vision provider
|
||||
_set_default_vision_provider(
|
||||
admin_user, vision_provider["id"], "gpt-4-vision-preview"
|
||||
LLMProviderManager.set_default_vision(
|
||||
vision_provider["id"], admin_user, "gpt-4-vision-preview"
|
||||
)
|
||||
|
||||
# Step 3: Create image generation config using clone mode from regular provider
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
"""
|
||||
Unit tests for vision model selection logging in get_default_llm_with_vision.
|
||||
Unit tests for get_default_llm_with_vision.
|
||||
|
||||
Verifies that operators get clear feedback about:
|
||||
1. Which vision model was selected and why
|
||||
2. When the default vision model doesn't support image input
|
||||
3. When no vision-capable model exists at all
|
||||
Only the designated default vision model is ever used. With none set, or with
|
||||
one that cannot take images, captioning is off. There is no fallback scan.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
@@ -19,13 +17,11 @@ def _make_mock_model(
|
||||
name: str = "gpt-4o",
|
||||
provider: str = "openai",
|
||||
provider_id: int = 1,
|
||||
flow_types: list[str] | None = None,
|
||||
) -> MagicMock:
|
||||
model = MagicMock()
|
||||
model.name = name
|
||||
model.llm_provider_id = provider_id
|
||||
model.llm_provider.provider = provider
|
||||
model.llm_model_flow_types = flow_types or []
|
||||
return model
|
||||
|
||||
|
||||
@@ -35,18 +31,19 @@ def _make_mock_model(
|
||||
@patch(f"{_FACTORY}.llm_from_provider")
|
||||
@patch(f"{_FACTORY}.LLMProviderView")
|
||||
@patch(f"{_FACTORY}.logger")
|
||||
def test_logs_when_using_default_vision_model(
|
||||
def test_uses_the_default_vision_model(
|
||||
mock_logger: MagicMock,
|
||||
mock_provider_view: MagicMock, # noqa: ARG001
|
||||
mock_llm_from: MagicMock, # noqa: ARG001
|
||||
mock_llm_from: MagicMock,
|
||||
mock_supports: MagicMock, # noqa: ARG001
|
||||
mock_fetch_default: MagicMock,
|
||||
mock_session: MagicMock, # noqa: ARG001
|
||||
) -> None:
|
||||
mock_fetch_default.return_value = _make_mock_model(name="gpt-4o", provider="azure")
|
||||
|
||||
get_default_llm_with_vision()
|
||||
result = get_default_llm_with_vision()
|
||||
|
||||
assert result is mock_llm_from.return_value
|
||||
mock_logger.info.assert_called_once()
|
||||
log_msg = mock_logger.info.call_args[0][0]
|
||||
assert "default vision model" in log_msg.lower()
|
||||
@@ -55,11 +52,11 @@ def test_logs_when_using_default_vision_model(
|
||||
@patch(f"{_FACTORY}.get_session_with_current_tenant")
|
||||
@patch(f"{_FACTORY}.fetch_default_vision_model")
|
||||
@patch(f"{_FACTORY}.model_supports_image_input", return_value=False)
|
||||
@patch(f"{_FACTORY}.fetch_existing_models", return_value=[])
|
||||
@patch(f"{_FACTORY}.llm_from_provider")
|
||||
@patch(f"{_FACTORY}.logger")
|
||||
def test_warns_when_default_model_lacks_vision(
|
||||
def test_returns_none_when_default_model_lacks_vision(
|
||||
mock_logger: MagicMock,
|
||||
mock_fetch_models: MagicMock, # noqa: ARG001
|
||||
mock_llm_from: MagicMock,
|
||||
mock_supports: MagicMock, # noqa: ARG001
|
||||
mock_fetch_default: MagicMock,
|
||||
mock_session: MagicMock, # noqa: ARG001
|
||||
@@ -71,58 +68,30 @@ def test_warns_when_default_model_lacks_vision(
|
||||
result = get_default_llm_with_vision()
|
||||
|
||||
assert result is None
|
||||
# Should have warned about the default model not supporting vision
|
||||
mock_llm_from.assert_not_called()
|
||||
warning_calls = [
|
||||
call
|
||||
for call in mock_logger.warning.call_args_list
|
||||
if "does not support" in str(call)
|
||||
]
|
||||
assert len(warning_calls) >= 1
|
||||
assert len(warning_calls) == 1
|
||||
|
||||
|
||||
@patch(f"{_FACTORY}.get_session_with_current_tenant")
|
||||
@patch(f"{_FACTORY}.fetch_default_vision_model", return_value=None)
|
||||
@patch(f"{_FACTORY}.fetch_existing_models", return_value=[])
|
||||
@patch(f"{_FACTORY}.llm_from_provider")
|
||||
@patch(f"{_FACTORY}.logger")
|
||||
def test_warns_when_no_models_exist(
|
||||
def test_returns_none_when_no_default_is_set(
|
||||
mock_logger: MagicMock,
|
||||
mock_fetch_models: MagicMock, # noqa: ARG001
|
||||
mock_llm_from: MagicMock,
|
||||
mock_fetch_default: MagicMock, # noqa: ARG001
|
||||
mock_session: MagicMock, # noqa: ARG001
|
||||
) -> None:
|
||||
"""No default means no captioning. Nothing else is tried."""
|
||||
result = get_default_llm_with_vision()
|
||||
|
||||
assert result is None
|
||||
mock_llm_from.assert_not_called()
|
||||
mock_logger.warning.assert_called_once()
|
||||
log_msg = mock_logger.warning.call_args[0][0]
|
||||
assert "no llm models" in log_msg.lower()
|
||||
|
||||
|
||||
@patch(f"{_FACTORY}.get_session_with_current_tenant")
|
||||
@patch(f"{_FACTORY}.fetch_default_vision_model", return_value=None)
|
||||
@patch(f"{_FACTORY}.fetch_existing_models")
|
||||
@patch(f"{_FACTORY}.model_supports_image_input", return_value=False)
|
||||
@patch(f"{_FACTORY}.LLMProviderView")
|
||||
@patch(f"{_FACTORY}.logger")
|
||||
def test_warns_when_no_model_supports_vision(
|
||||
mock_logger: MagicMock,
|
||||
mock_provider_view: MagicMock, # noqa: ARG001
|
||||
mock_supports: MagicMock, # noqa: ARG001
|
||||
mock_fetch_models: MagicMock,
|
||||
mock_fetch_default: MagicMock, # noqa: ARG001
|
||||
mock_session: MagicMock, # noqa: ARG001
|
||||
) -> None:
|
||||
mock_fetch_models.return_value = [
|
||||
_make_mock_model(name="text-model-1", provider="openai"),
|
||||
_make_mock_model(name="text-model-2", provider="azure", provider_id=2),
|
||||
]
|
||||
|
||||
result = get_default_llm_with_vision()
|
||||
|
||||
assert result is None
|
||||
warning_calls = [
|
||||
call
|
||||
for call in mock_logger.warning.call_args_list
|
||||
if "no vision-capable model" in str(call).lower()
|
||||
]
|
||||
assert len(warning_calls) == 1
|
||||
assert "no default vision model" in log_msg.lower()
|
||||
Reference in new issue
Block a user