From cbd2011900bb47fda2d44ff5eba01915b24349ee Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:25:53 -0700 Subject: [PATCH] refactor(acp): move picker assembly into model_catalog.build_model_state; server keeps only the fallback row --- acp_adapter/model_catalog.py | 55 ++++++++++++++++++++- acp_adapter/server.py | 59 ++--------------------- tests/acp/test_named_provider_catalogs.py | 7 +-- 3 files changed, 63 insertions(+), 58 deletions(-) diff --git a/acp_adapter/model_catalog.py b/acp_adapter/model_catalog.py index 926ba38901..3d5acf3165 100644 --- a/acp_adapter/model_catalog.py +++ b/acp_adapter/model_catalog.py @@ -7,7 +7,7 @@ import os from dataclasses import dataclass, field from typing import Callable -from acp.schema import ModelInfo +from acp.schema import ModelInfo, SessionModelState logger = logging.getLogger("acp_adapter.server") @@ -250,3 +250,56 @@ class _ModelCatalog: if named_slug == normalized_provider and named_model == self.current_model: named_parts.append("current") self.add(named_slug, named_model, named_model, " • ".join(part for part in named_parts if part)) + + +def build_model_state(model: str, provider: str, base_url: str) -> SessionModelState | None: + """Picker state from the shared inventory + named endpoints; ``None`` when nothing is listable + (caller falls back to a single current-model row). Raises on inventory failure.""" + from hermes_cli.inventory import build_models_payload, load_picker_context + from hermes_cli.models import normalize_provider, provider_label + + normalized_provider = normalize_provider(provider) + context = load_picker_context().with_overrides( + current_provider=normalized_provider, current_model=model, current_base_url=base_url, + ) + payload = build_models_payload( + context, explicit_only=True, include_unconfigured=False, picker_hints=False, + canonical_order=True, pricing=False, capabilities=False, refresh=False, + probe_custom_providers=False, probe_current_custom_provider=False, + max_models=ACP_MAX_MODELS_PER_PROVIDER, + ) + + cat = _ModelCatalog( + normalize_provider=normalize_provider, current_model=model, + current_choice_provider=str(provider or "").strip().lower(), + current_base_url=base_url.strip().rstrip("/").lower(), + ) + cat.add_inventory_rows(payload.get("providers") or [], provider_label) + cat.add_named_catalogs(_named_custom_provider_catalogs(), normalized_provider) + available_models = cat.models + + def empty_applies(provider_id: str) -> bool: + return _empty_catalog_applies(provider_id, cat.empty_authoritative, normalize_provider) + + if cat.empty_authoritative: + available_models = [m for m in available_models if not empty_applies(_choice_provider(m.model_id))] + + current_is_empty = empty_applies(cat.current_choice_provider) + if current_is_empty: + available_models = [m for m in available_models if " • current" not in str(m.description or "")] + current_model_id = "" if current_is_empty else encode_model_choice(cat.current_choice_provider, model) + if current_model_id and current_model_id not in {item.model_id for item in available_models}: + provider_name = provider_label(normalized_provider) + available_models.insert(0, ModelInfo( + model_id=current_model_id, name=f"{provider_name} · {model}", + description=f"Provider: {provider_name} • current", + )) + + if not available_models and current_is_empty: + return SessionModelState(available_models=[], current_model_id="") + if available_models: + return SessionModelState( + available_models=available_models, + current_model_id=current_model_id if current_model_id or current_is_empty else available_models[0].model_id, + ) + return None diff --git a/acp_adapter/server.py b/acp_adapter/server.py index 3843ccbc27..2d93d00389 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -31,9 +31,8 @@ from acp_adapter.events import ( _build_plan_update_from_todo_result, make_message_cb, make_step_cb, make_thinking_cb, make_tool_progress_cb, ) -from acp_adapter.model_catalog import ( - ACP_MAX_MODELS_PER_PROVIDER, _ModelCatalog, _choice_provider, _empty_catalog_applies, encode_model_choice, - _named_custom_provider_catalogs, +from acp_adapter.model_catalog import ( # noqa: F401 (ACP_MAX_MODELS_PER_PROVIDER re-exported for tests) + ACP_MAX_MODELS_PER_PROVIDER, build_model_state, encode_model_choice, ) from acp_adapter.permissions import make_approval_callback from acp_adapter.provenance import session_provenance_meta @@ -301,58 +300,10 @@ class HermesACPAgent(acp.Agent): as ``hermes model``/TUI/dashboard) so the selector isn't just the current curated list.""" model = str(state.model or getattr(state.agent, "model", "") or "").strip() provider = getattr(state.agent, "provider", None) or detect_provider() or "openrouter" - try: - from hermes_cli.inventory import build_models_payload, load_picker_context - from hermes_cli.models import normalize_provider, provider_label - - normalized_provider = normalize_provider(provider) - context = load_picker_context().with_overrides( - current_provider=normalized_provider, current_model=model, - current_base_url=str(getattr(state.agent, "base_url", "") or ""), - ) - payload = build_models_payload( - context, explicit_only=True, include_unconfigured=False, picker_hints=False, - canonical_order=True, pricing=False, capabilities=False, refresh=False, - probe_custom_providers=False, probe_current_custom_provider=False, - max_models=ACP_MAX_MODELS_PER_PROVIDER, - ) - - cat = _ModelCatalog( - normalize_provider=normalize_provider, current_model=model, - current_choice_provider=str(provider or "").strip().lower(), - current_base_url=str(getattr(state.agent, "base_url", "") or "").strip().rstrip("/").lower(), - ) - cat.add_inventory_rows(payload.get("providers") or [], provider_label) - cat.add_named_catalogs(_named_custom_provider_catalogs(), normalized_provider) - available_models = cat.models - - def empty_applies(provider_id: str) -> bool: - return _empty_catalog_applies(provider_id, cat.empty_authoritative, normalize_provider) - - if cat.empty_authoritative: - available_models = [m for m in available_models if not empty_applies(_choice_provider(m.model_id))] - - current_is_empty = empty_applies(cat.current_choice_provider) - if current_is_empty: - available_models = [m for m in available_models if " • current" not in str(m.description or "")] - current_model_id = "" if current_is_empty else encode_model_choice(cat.current_choice_provider, model) - if current_model_id and current_model_id not in {item.model_id for item in available_models}: - provider_name = provider_label(normalized_provider) - available_models.insert(0, ModelInfo( - model_id=current_model_id, name=f"{provider_name} · {model}", - description=f"Provider: {provider_name} • current", - )) - - if not available_models and current_is_empty: - return SessionModelState(available_models=[], current_model_id="") - if available_models: - return SessionModelState( - available_models=available_models, - current_model_id=( - current_model_id if current_model_id or current_is_empty else available_models[0].model_id - ), - ) + picker = build_model_state(model, provider, str(getattr(state.agent, "base_url", "") or "")) + if picker is not None: + return picker except Exception: logger.debug("Could not build ACP model state", exc_info=True) diff --git a/tests/acp/test_named_provider_catalogs.py b/tests/acp/test_named_provider_catalogs.py index cb1a8404fd..49abc02b80 100644 --- a/tests/acp/test_named_provider_catalogs.py +++ b/tests/acp/test_named_provider_catalogs.py @@ -12,7 +12,8 @@ from unittest.mock import patch import pytest -from acp_adapter.server import HermesACPAgent, _named_custom_provider_catalogs +from acp_adapter.model_catalog import _named_custom_provider_catalogs +from acp_adapter.server import HermesACPAgent from acp_adapter.session import SessionManager from acp.schema import SessionModelState @@ -193,7 +194,7 @@ class TestModelStateIncludesNamedProviders: acp_agent = HermesACPAgent(session_manager=manager) with patch( - "acp_adapter.server._named_custom_provider_catalogs", + "acp_adapter.model_catalog._named_custom_provider_catalogs", return_value=[("custom:ollama", "Ollama", [])], ): resp = await acp_agent.new_session(cwd="/tmp") @@ -215,7 +216,7 @@ class TestModelStateIncludesNamedProviders: acp_agent = HermesACPAgent(session_manager=manager) with patch( - "acp_adapter.server._named_custom_provider_catalogs", + "acp_adapter.model_catalog._named_custom_provider_catalogs", return_value=[ ( "custom:bedrock-mantle",