Skip to content

Commit 9f3fd5c

Browse files
committed
Fix models controller config handling and add regression test
1 parent c03159e commit 9f3fd5c

File tree

2 files changed

+147
-13
lines changed

2 files changed

+147
-13
lines changed

src/core/app/controllers/models_controller.py

Lines changed: 86 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -255,27 +255,100 @@ async def _list_models_impl(
255255
# Use the injected config service
256256
from src.core.config.app_config import AppConfig
257257

258-
if not isinstance(config, AppConfig):
259-
# Fallback to default config if we got a different config type
260-
config = AppConfig()
258+
# Determine which backend configuration views are available. Prefer the
259+
# injected configuration object but gracefully fall back to a default
260+
# AppConfig instance when the provided config does not expose a
261+
# compatible `backends` attribute. This preserves compatibility with
262+
# lightweight test doubles that only implement the public IConfig
263+
# interface while avoiding the previous behaviour of discarding the
264+
# injected configuration altogether.
265+
backend_views: list[Any] = []
266+
if hasattr(config, "backends"):
267+
try:
268+
backend_views.append(getattr(config, "backends"))
269+
except Exception as exc: # pragma: no cover - defensive guard
270+
logger.debug(
271+
"Unable to access backends on %s: %s",
272+
type(config).__name__,
273+
exc,
274+
exc_info=True,
275+
)
276+
else:
277+
logger.debug(
278+
"Configuration %s lacks 'backends' attribute; using fallback AppConfig",
279+
type(config).__name__,
280+
)
281+
282+
if not backend_views:
283+
fallback_config = AppConfig()
284+
backend_views.append(fallback_config.backends)
261285

262286
# Ensure backend service is at least resolved for DI side effects
263287
_ = backend_service
264288

265-
try:
266-
functional_backends = set(config.backends.functional_backends)
267-
except Exception as exc: # pragma: no cover - defensive guard
268-
logger.debug(
269-
"Unable to determine functional backends: %s", exc, exc_info=True
270-
)
271-
functional_backends = set()
289+
functional_backends: set[str] = set()
290+
for view in backend_views:
291+
try:
292+
candidates = getattr(view, "functional_backends")
293+
except AttributeError:
294+
logger.debug(
295+
"Backend configuration %s does not expose functional_backends",
296+
type(view).__name__,
297+
)
298+
continue
299+
except Exception as exc: # pragma: no cover - defensive guard
300+
logger.debug(
301+
"Error accessing functional_backends on %s: %s",
302+
type(view).__name__,
303+
exc,
304+
exc_info=True,
305+
)
306+
continue
307+
308+
# Support both property-based and plain attribute implementations.
309+
try:
310+
if callable(candidates):
311+
candidates = candidates()
312+
except Exception as exc: # pragma: no cover - defensive guard
313+
logger.debug(
314+
"Failed to invoke functional_backends on %s: %s",
315+
type(view).__name__,
316+
exc,
317+
exc_info=True,
318+
)
319+
continue
320+
321+
try:
322+
if isinstance(candidates, set) or isinstance(candidates, (list, tuple)):
323+
functional_backends.update(candidates)
324+
elif isinstance(candidates, dict):
325+
functional_backends.update(candidates.keys())
326+
elif candidates is None:
327+
continue
328+
else:
329+
functional_backends.update(set(candidates))
330+
except TypeError:
331+
logger.debug(
332+
"functional_backends on %s is not iterable", type(view).__name__
333+
)
334+
continue
272335

273336
# Iterate through dynamically discovered backend types from the registry
274337
for backend_type in backend_registry.get_registered_backends():
275338
backend_config: Any | None = None
276-
if config.backends:
277-
# Access backend config dynamically using getattr
278-
backend_config = getattr(config.backends, backend_type, None)
339+
for view in backend_views:
340+
candidate: Any | None = None
341+
if isinstance(view, dict):
342+
candidate = view.get(backend_type)
343+
else:
344+
try:
345+
candidate = getattr(view, backend_type, None)
346+
except AttributeError:
347+
candidate = None
348+
349+
if candidate is not None:
350+
backend_config = candidate
351+
break
279352

280353
has_credentials = False
281354
if isinstance(backend_config, dict):

tests/unit/test_models_endpoint.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,64 @@ def create_backend(
7272
model_ids = {model["id"] for model in result["data"]}
7373
assert "gemini-oauth-plan:gemini-2.5-pro" in model_ids
7474
assert created_backends == ["gemini-oauth-plan"]
75+
76+
77+
def test_model_listing_respects_injected_config(monkeypatch) -> None:
78+
"""Ensure the controller honours custom configurations supplied via DI."""
79+
80+
import asyncio
81+
from types import SimpleNamespace
82+
from unittest.mock import Mock
83+
84+
from src.core.app.controllers import models_controller
85+
from src.core.app.controllers.models_controller import _list_models_impl
86+
87+
monkeypatch.setattr(
88+
models_controller.backend_registry,
89+
"get_registered_backends",
90+
lambda: ["dummy"],
91+
)
92+
93+
class DummyBackend:
94+
def get_available_models(self) -> list[str]:
95+
return ["dummy-model"]
96+
97+
class DummyFactory:
98+
def __init__(self) -> None:
99+
self.created_with: list[tuple[str, object]] = []
100+
101+
def create_backend(self, backend_type: str, config_obj: object) -> DummyBackend:
102+
self.created_with.append((backend_type, config_obj))
103+
return DummyBackend()
104+
105+
class CustomBackends:
106+
def __init__(self) -> None:
107+
self.functional_backends = {"dummy"}
108+
self.dummy = SimpleNamespace(api_key="token")
109+
110+
class CustomConfig:
111+
def __init__(self) -> None:
112+
self.backends = CustomBackends()
113+
114+
def get(self, key: str, default: object | None = None) -> object | None:
115+
if key == "backends":
116+
return self.backends
117+
return default
118+
119+
def set(self, key: str, value: object) -> None:
120+
setattr(self, key, value)
121+
122+
factory = DummyFactory()
123+
config = CustomConfig()
124+
125+
result = asyncio.run(
126+
_list_models_impl(
127+
backend_service=Mock(),
128+
config=config, # type: ignore[arg-type]
129+
backend_factory=factory, # type: ignore[arg-type]
130+
)
131+
)
132+
133+
model_ids = {model["id"] for model in result["data"]}
134+
assert "dummy:dummy-model" in model_ids
135+
assert factory.created_with == [("dummy", config)]

0 commit comments

Comments
 (0)