From 2eefa5edfd76573f39091357f970ebb95d585a1b Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 7 Apr 2026 06:51:49 -0600 Subject: [PATCH] Deprecate /api/options/models, add /api/v1/config/models/search endpoint (#13799) Co-authored-by: openhands --- .../app_server/config_api/config_models.py | 32 +++ .../app_server/config_api/config_router.py | 94 ++++++++ openhands/app_server/v1_router.py | 2 + openhands/server/routes/public.py | 10 +- tests/unit/app_server/test_config_router.py | 206 ++++++++++++++++++ 5 files changed, 343 insertions(+), 1 deletion(-) create mode 100644 openhands/app_server/config_api/config_models.py create mode 100644 openhands/app_server/config_api/config_router.py create mode 100644 tests/unit/app_server/test_config_router.py diff --git a/openhands/app_server/config_api/config_models.py b/openhands/app_server/config_api/config_models.py new file mode 100644 index 0000000000..bccd654989 --- /dev/null +++ b/openhands/app_server/config_api/config_models.py @@ -0,0 +1,32 @@ +"""Config-related models for OpenHands App Server V1 API.""" + +from pydantic import BaseModel, Field + + +class LLMModel(BaseModel): + """LLM Model object for API responses. + + Attributes: + name: The model name. + verified: Whether the model is verified by OpenHands. + """ + + provider: str | None = Field( + default=None, description='The name of the provider for this model' + ) + name: str = Field(description='The name of this model') + verified: bool = Field( + default=False, description='Whether the model is verified by OpenHands' + ) + + +class LLMModelPage(BaseModel): + """Paginated response for LLM models. + + Attributes: + items: List of LLM models in the current page. + next_page_id: ID for the next page, or None if there are no more pages. + """ + + items: list[LLMModel] + next_page_id: str | None = None diff --git a/openhands/app_server/config_api/config_router.py b/openhands/app_server/config_api/config_router.py new file mode 100644 index 0000000000..29df9e870c --- /dev/null +++ b/openhands/app_server/config_api/config_router.py @@ -0,0 +1,94 @@ +"""Config router for OpenHands App Server V1 API. + +This module provides V1 API endpoints for configuration, including model search +with pagination support. +""" + +from typing import Annotated + +from fastapi import APIRouter, Depends, Query + +from openhands.app_server.config_api.config_models import LLMModel, LLMModelPage +from openhands.app_server.utils.dependencies import get_dependencies +from openhands.app_server.utils.paging_utils import ( + paginate_results, +) +from openhands.sdk.llm.utils.verified_models import VERIFIED_MODELS +from openhands.server.routes.public import get_llm_models_dependency +from openhands.utils.llm import ModelsResponse + +# We use the get_dependencies method here to signal to the OpenAPI docs that this endpoint +# is protected. The actual protection is provided by SetAuthCookieMiddleware +router = APIRouter( + prefix='/config', + tags=['Config'], + dependencies=get_dependencies(), +) + + +@router.get('/models/search') +async def search_models( + page_id: Annotated[ + str | None, + Query(title='Optional next_page_id from the previously returned page'), + ] = None, + limit: Annotated[ + int, + Query(title='The max number of results in the page', gt=0, le=100), + ] = 50, + query: Annotated[ + str | None, + Query(title='Filter models by name (case-insensitive substring match)'), + ] = None, + verified__eq: Annotated[ + bool | None, + Query(title='Filter by verified status (true/false, omit for all)'), + ] = None, + models: ModelsResponse = Depends(get_llm_models_dependency), +) -> LLMModelPage: + """Search for LLM models with pagination and filtering. + + Returns a paginated list of models that can be filtered by name + (contains) and verified status. + """ + filtered_models = _get_all_models_with_verified(models) + + if query is not None: + query_lower = query.lower() + filtered_models = [m for m in filtered_models if query_lower in m.name.lower()] + + if verified__eq is not None: + filtered_models = [m for m in filtered_models if m.verified == verified__eq] + + # Apply pagination + items, next_page_id = paginate_results(filtered_models, page_id, limit) + + return LLMModelPage(items=items, next_page_id=next_page_id) + + +def _get_verified_models() -> set[str]: + verified_models = set() + for provider, models in VERIFIED_MODELS.items(): + for name in models: + verified_models.add(f'{provider}/{name}') + return verified_models + + +def _get_all_models_with_verified(models: ModelsResponse) -> list[LLMModel]: + verified_models = _get_verified_models() + results = [] + for model_name in models.models: + verified = model_name in verified_models + parts = model_name.split('/', 1) + if len(parts) == 2: + provider, name = parts + else: + provider = None + name = parts[0] + result = LLMModel( + provider=provider, + name=name, + verified=verified, + ) + results.append(result) + return results diff --git a/openhands/app_server/v1_router.py b/openhands/app_server/v1_router.py index 292b77b2e1..eae32aa933 100644 --- a/openhands/app_server/v1_router.py +++ b/openhands/app_server/v1_router.py @@ -1,6 +1,7 @@ from fastapi import APIRouter from openhands.app_server.app_conversation import app_conversation_router +from openhands.app_server.config_api.config_router import router as config_router from openhands.app_server.event import event_router from openhands.app_server.event_callback import ( webhook_router, @@ -33,3 +34,4 @@ router.include_router(skills_router.router) router.include_router(webhook_router.router) router.include_router(web_client_router.router) router.include_router(git_router) +router.include_router(config_router) diff --git a/openhands/server/routes/public.py b/openhands/server/routes/public.py index 6c8067f87c..3ceb3f0bbd 100644 --- a/openhands/server/routes/public.py +++ b/openhands/server/routes/public.py @@ -29,10 +29,18 @@ async def get_llm_models_dependency(request: Request) -> ModelsResponse: return get_supported_llm_models(config) -@app.get('/models') +@app.get('/models', deprecated=True) async def get_litellm_models( models: ModelsResponse = Depends(get_llm_models_dependency), ) -> ModelsResponse: + """Get all supported LLM models. + + .. deprecated:: + This endpoint is deprecated. Use `/api/v1/config/models/search` instead. + + Returns: + ModelsResponse: Response containing models, verified_models, verified_providers, and default_model. + """ return models diff --git a/tests/unit/app_server/test_config_router.py b/tests/unit/app_server/test_config_router.py new file mode 100644 index 0000000000..8bd5eeb850 --- /dev/null +++ b/tests/unit/app_server/test_config_router.py @@ -0,0 +1,206 @@ +"""Unit tests for the config_models and config_router. + +This module tests the config router endpoints, +focusing on the search_models endpoint for LLM models. +""" + +import pytest +from fastapi import FastAPI, status +from fastapi.testclient import TestClient + +from openhands.app_server.config_api.config_models import LLMModel +from openhands.app_server.config_api.config_router import ( + _get_all_models_with_verified, + router, +) +from openhands.app_server.utils.dependencies import check_session_api_key +from openhands.app_server.utils.paging_utils import encode_page_id, paginate_results +from openhands.server.shared import config +from openhands.utils.llm import get_supported_llm_models + + +class TestLLMModel: + """Test suite for LLMModel.""" + + def test_create_model_with_name_and_verified(self): + """Test that LLMModel can be created with name and verified.""" + model = LLMModel(provider='openai', name='gpt-4', verified=True) + + assert model.provider == 'openai' + assert model.name == 'gpt-4' + assert model.verified is True + + def test_create_model_with_default_verified_false(self): + """Test that verified defaults to False.""" + model = LLMModel(provider='openai', name='gpt-4') + + assert model.provider == 'openai' + assert model.name == 'gpt-4' + assert model.verified is False + + +class TestPagination: + """Test suite for pagination helper function.""" + + def test_returns_first_page_when_no_page_id(self): + """Test that first page is returned when no page_id is provided.""" + models = [ + LLMModel(provider='openai', name='gpt-4', verified=True), + LLMModel(provider='anthropic', name='claude-3', verified=True), + LLMModel(provider='openai', name='gpt-3.5', verified=False), + ] + + result, next_page_id = paginate_results(models, None, 2) + + assert len(result) == 2 + assert next_page_id == encode_page_id(2) + + def test_returns_second_page_when_page_id_provided(self): + """Test that correct page is returned when page_id is provided.""" + models = [ + LLMModel(provider='openai', name='gpt-4', verified=True), + LLMModel(provider='anthropic', name='claude-3', verified=True), + LLMModel(provider='openai', name='gpt-3.5', verified=False), + ] + encoded_page_id = encode_page_id(2) + + result, next_page_id = paginate_results(models, encoded_page_id, 2) + + assert len(result) == 1 + assert result[0].provider == 'openai' + assert result[0].name == 'gpt-3.5' + assert next_page_id is None + + +class TestGetAllModelsWithVerified: + """Test suite for _get_all_models_with_verified function.""" + + def test_returns_list_of_llm_models(self): + """Test that function returns list of LLMModel objects.""" + models = _get_all_models_with_verified(get_supported_llm_models(config)) + + assert isinstance(models, list) + assert all(isinstance(m, LLMModel) for m in models) + + def test_models_verified_mix(self): + """Test that models contains a mix of verified and unverified.""" + models = _get_all_models_with_verified(get_supported_llm_models(config)) + + assert any(m.verified is True for m in models) + assert any(m.verified is False for m in models) + + +@pytest.fixture +def test_client(): + """Create a test client with the actual config router and mocked dependencies. + + We override check_session_api_key to bypass auth checks. + This allows us to test the actual Query parameter validation in the router. + """ + app = FastAPI() + app.include_router(router) + + # Override the auth dependency to always pass + app.dependency_overrides[check_session_api_key] = lambda: None + + client = TestClient(app, raise_server_exceptions=False) + yield client + + # Clean up + app.dependency_overrides.clear() + + +class TestSearchModelsEndpoint: + """Test suite for /models/search endpoint.""" + + def test_returns_200_with_paginated_results(self, test_client): + """Test that endpoint returns 200 with paginated results.""" + response = test_client.get('/config/models/search') + + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert 'items' in data + assert 'next_page_id' in data + + def test_respects_limit_parameter(self, test_client): + """Test that limit parameter is respected.""" + response = test_client.get('/config/models/search', params={'limit': 2}) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert len(data['items']) <= 2 + + def test_filters_by_query_name_contains(self, test_client): + """Test that query parameter filters by name (case-insensitive).""" + response = test_client.get('/config/models/search', params={'query': 'gpt'}) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + for item in data['items']: + assert 'gpt' in item['name'].lower() + + def test_filters_by_verified_eq_true(self, test_client): + """Test that verified__eq=true filters to verified models only.""" + response = test_client.get( + '/config/models/search', params={'verified__eq': True} + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + for item in data['items']: + assert item['verified'] is True + + def test_filters_by_verified_eq_false(self, test_client): + """Test that verified__eq=false filters to non-verified models only.""" + # Since all models from _SDK_VERIFIED_MODELS are verified, + # we expect empty results when filtering for non-verified + response = test_client.get( + '/config/models/search', params={'verified__eq': False} + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + for item in data['items']: + assert item['verified'] is False + + def test_combines_query_and_verified_filters(self, test_client): + """Test that query and verified filters are combined.""" + response = test_client.get( + '/config/models/search', params={'query': 'gpt', 'verified__eq': True} + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + for item in data['items']: + assert 'gpt' in item['name'].lower() + assert item['verified'] is True + + def test_pagination_with_page_id(self, test_client): + """Test that pagination works with page_id.""" + # First request - get first page + response1 = test_client.get('/config/models/search', params={'limit': 1}) + data1 = response1.json() + + # If there's a next page, test it + if data1.get('next_page_id'): + response2 = test_client.get( + '/config/models/search', + params={'limit': 1, 'page_id': data1['next_page_id']}, + ) + data2 = response2.json() + + assert response2.status_code == status.HTTP_200_OK + # The items should be different + assert data1['items'][0]['name'] != data2['items'][0]['name'] + + def test_invalid_limit_parameter_returns_422(self, test_client): + """Test that invalid limit parameter returns 422.""" + response = test_client.get('/config/models/search', params={'limit': 0}) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + + def test_limit_exceeds_max_returns_422(self, test_client): + """Test that limit exceeding max returns 422.""" + response = test_client.get('/config/models/search', params={'limit': 101}) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY