refactor(app_controller): migrate _fetch_models.do_fetch to per-provider Result (Phase 6 Group 6.4)

Replaces per-provider logging.debug body with _list_models_for_provider_result SDK-boundary helper. Aggregates per-provider failures into self._model_fetch_errors and returns Result with aggregated errors. Stderr summary on partial failure. The SDK boundary (ai_client.list_models call) is the canonical place to catch vendor exceptions and convert to ErrorInfo(kind=NETWORK), per error_handling.md §'Boundary Types'. Audit: INTERNAL_SILENT_SWALLOW for src/app_controller.py: 23 -> 22.
2026-06-19 15:56:53 -04:00
parent fd91c83a0c
commit 50750f3183
2 changed files with 106 additions and 10 deletions
@@ -845,6 +845,11 @@ class AppController:
  self._inject_preview_error:           Optional[ErrorInfo] = None
  self._mcp_config_parse_error:         Optional[ErrorInfo] = None
  self._save_project_error:             Optional[ErrorInfo] = None
+  # --- Per-provider model-fetch errors (Phase 6: Group 6.4) ---
+  # Aggregated per-provider failures from _fetch_models. The map is the
+  # durable data plane for sub-track 4 GUI to display in the models panel;
+  # stderr write IS the visible-but-incomplete drain (full drain = GUI).
+  self._model_fetch_errors:             Dict[str, ErrorInfo] = {}
  # --- Shared background pool + proactive warmup (startup_speedup_20260606) ---
  self._io_pool = make_io_pool()
  _install_sigint_exit_handler(self)
@@ -3322,6 +3327,27 @@ class AppController:

 #region: AI Settings

+ def _list_models_for_provider_result(self, p: str) -> "Result[List[str]]":
+  """SDK boundary (Phase 6 Group 6.4): wrap ai_client.list_models(p).
+  Returns Result[List[str]]. On failure: any SDK exception
+  (OSError/ValueError/TypeError/KeyError/AttributeError/RuntimeError/etc.)
+  -> ErrorInfo(kind=NETWORK, original=e).
+
+  Per the styleguide §"Boundary Types", SDK calls are the canonical
+  boundary: catch vendor exceptions and convert to ErrorInfo. The
+  per-provider accumulation in self._model_fetch_errors makes the
+  aggregated failures visible to sub-track 4 GUI."""
+  try:
+   models = ai_client.list_models(p)
+   return Result(data=list(models) if models else [])
+  except Exception as e:
+   return Result(data=[], errors=[ErrorInfo(
+    kind=ErrorKind.NETWORK,
+    message=str(e),
+    source=f"app_controller._list_models_for_provider_result[{p}]",
+    original=e,
+   )])
+
 def _fetch_models(self, provider: str) -> None:
  """
  [C: src/gui_2.py:App.run]
@@ -3336,12 +3362,16 @@ class AppController:
  self.ai_status = "fetching models..."

  def do_fetch() -> Result[None]:
+   aggregated_errors: List[ErrorInfo] = []
   try:
    for p in ai_client.PROVIDERS:
-     try:
-      self.all_available_models[p] = ai_client.list_models(p)
-     except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError, RuntimeError) as e:
-      logging.getLogger(__name__).debug("list_models failed for %s: %s", p, e, extra={"source": "app_controller._fetch_models.do_fetch.per_provider"})
+     result = self._list_models_for_provider_result(p)
+     if result.ok:
+      self.all_available_models[p] = result.data
+     else:
+      err = result.errors[0]
+      self._model_fetch_errors[p] = err
+      aggregated_errors.append(err)
      self.all_available_models[p] = []

    models_list = self.all_available_models.get(provider, [])
@@ -3351,17 +3381,22 @@ class AppController:
     ai_client.set_provider(self._current_provider, self.current_model)
    if self.ai_status == "fetching models...":
     self.ai_status = f"models loaded: {len(models_list)}"
+    if aggregated_errors:
+     summary = f"{len(aggregated_errors)} provider(s) failed: " + "; ".join(e.message for e in aggregated_errors)
+     sys.stderr.write(f"model fetch partial failure: {summary}\n")
+     sys.stderr.flush()
+     return Result(data=None, errors=aggregated_errors)
    return OK
-   except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError, RuntimeError) as e:
-    logging.getLogger(__name__).debug("model fetch error: %s", e, extra={"source": "app_controller._fetch_models.do_fetch"})
-    if self.ai_status == "fetching models...":
-     self.ai_status = f"model fetch error: {e}"
-    return Result(data=None, errors=[ErrorInfo(
+   except Exception as e:
+    err = ErrorInfo(
     kind=ErrorKind.INTERNAL,
     message=str(e),
     source="app_controller._fetch_models.do_fetch",
     original=e,
-    )])
+    )
+    if self.ai_status == "fetching models...":
+     self.ai_status = f"model fetch error: {e}"
+    return Result(data=None, errors=[err])
  self.submit_io(do_fetch)

 def _apply_preset(self, name: str, scope: str) -> None:
@@ -405,3 +405,64 @@ def test_save_active_project_stores_error_on_save_failure():
 assert isinstance(ctrl._save_project_error, ErrorInfo)
 assert "denied" in ctrl._save_project_error.message
 assert "save error" in ctrl.ai_status
+
+
+# --- Phase 6: Group 6.4 (SDK boundary in _fetch_models) ---
+
+def test_list_models_for_provider_result_returns_ok_on_success():
+ """
+ SDK boundary (Phase 6 Group 6.4): _list_models_for_provider_result wraps
+ ai_client.list_models(p) and returns Result[list] on success.
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ with patch("src.app_controller.ai_client.list_models", return_value=["model-a", "model-b"]):
+  result = ctrl._list_models_for_provider_result("gemini")
+ assert isinstance(result, Result)
+ assert result.ok is True
+ assert result.data == ["model-a", "model-b"]
+
+
+def test_list_models_for_provider_result_returns_error_on_sdk_failure():
+ """
+ SDK boundary: _list_models_for_provider_result converts SDK exceptions
+ to ErrorInfo(original=e) with NETWORK kind (the standard SDK boundary kind).
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ with patch("src.app_controller.ai_client.list_models", side_effect=RuntimeError("network unreachable")):
+  result = ctrl._list_models_for_provider_result("gemini")
+ assert isinstance(result, Result)
+ assert result.ok is False
+ assert len(result.errors) == 1
+ assert isinstance(result.errors[0], ErrorInfo)
+ assert "network unreachable" in result.errors[0].message
+ assert result.errors[0].kind == ErrorKind.NETWORK
+ assert result.errors[0].original is not None
+
+
+def test_fetch_models_aggregates_per_provider_errors():
+ """
+ The _fetch_models.do_fetch wrapper accumulates per-provider failures in
+ self._model_fetch_errors and returns a Result that carries the aggregated
+ errors. The legacy wrapper (do_fetch itself) is internal; the public API
+ is the side effect (self.all_available_models gets a [] entry per failed provider).
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ # Make the SDK return an error for "gemini" and succeed for "anthropic"
+ def fake_list_models(p):
+  if p == "gemini":
+   raise RuntimeError("gemini api down")
+  return [f"{p}-model"]
+ with patch("src.app_controller.ai_client.list_models", side_effect=fake_list_models):
+  with patch("src.app_controller.ai_client.PROVIDERS", new=["gemini", "anthropic"]):
+   # do_fetch is the inner function; we need to access it. Easiest: call _fetch_models
+   # and inspect the resulting side effect on all_available_models.
+   ctrl._fetch_models("anthropic")
+ # Per-provider errors should be accumulated in self._model_fetch_errors
+ assert "gemini" in ctrl._model_fetch_errors
+ assert isinstance(ctrl._model_fetch_errors["gemini"], ErrorInfo)
+ assert "gemini api down" in ctrl._model_fetch_errors["gemini"].message
+ # The gemini entry should have an empty list (per-provider failure placeholder)
+ assert ctrl.all_available_models.get("gemini") == []  # NOTE: do_fetch may not have run yet if deferred