refactor(ai_client): migrate L1594 _list_gemini_models to Result[T] (Phase 10 site 1)
The original function had a broken pattern: 'raise _classify_gemini_error(exc) from exc' which raises an ErrorInfo (not an Exception) — a runtime bug. Per TIER1_REVIEW 2026-06-20 directive: per-site decision. The body raised a structured error carrier (ErrorInfo), but the pattern was incorrect (ErrorInfo is not an Exception). Cleanest fix: full Result[T] migration. New helper: - _list_gemini_models_result(api_key: str) -> Result[list[str]] Returns Result(data=sorted_models) on success, Result(data=[], errors=[ErrorInfo]) on SDK/network failure. Legacy wrapper: - _list_gemini_models(api_key: str) -> list[str] Returns result.data (preserves original signature; callers don't see errors). Audit: ai_client BC 9 -> 8. Site L1594 (now shifted to L1609 due to helper insertion) no longer in INTERNAL_BROAD_CATCH.
This commit is contained in:
+18
-3
@@ -1581,7 +1581,15 @@ def _list_gemini_cli_models() -> list[str]:
|
||||
"gemini-2.5-flash-lite",
|
||||
]
|
||||
|
||||
def _list_gemini_models(api_key: str) -> list[str]:
|
||||
def _list_gemini_models_result(api_key: str) -> Result[list[str]]:
|
||||
"""List available Gemini models via google-genai SDK.
|
||||
|
||||
Returns the sorted list of Gemini model names. On SDK or network failure,
|
||||
returns Result(data=[], errors=[ErrorInfo(...)]). The legacy caller
|
||||
(_list_gemini_models) returns result.data directly (preserving original
|
||||
behavior); callers that need to surface errors should call this helper
|
||||
and inspect result.errors.
|
||||
"""
|
||||
try:
|
||||
genai = _require_warmed("google.genai")
|
||||
client = genai.Client(api_key=api_key)
|
||||
@@ -1590,9 +1598,16 @@ def _list_gemini_models(api_key: str) -> list[str]:
|
||||
name = m.name
|
||||
if name and name.startswith("models/"): name = name[len("models/"):]
|
||||
if name and "gemini" in name.lower(): models.append(name)
|
||||
return sorted(models)
|
||||
return Result(data=sorted(models))
|
||||
except Exception as exc:
|
||||
raise _classify_gemini_error(exc) from exc
|
||||
return Result(
|
||||
data=[],
|
||||
errors=[_classify_gemini_error(exc, source="ai_client._list_gemini_models_result")],
|
||||
)
|
||||
|
||||
|
||||
def _list_gemini_models(api_key: str) -> list[str]:
|
||||
return _list_gemini_models_result(api_key).data
|
||||
|
||||
def _ensure_gemini_client() -> None:
|
||||
global _gemini_client
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
"""Phase 10 invariant tests (RED).
|
||||
|
||||
Site 1 (L1594): _list_gemini_models_result helper must exist + return Result[list[str]].
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
from src.result_types import Result, ErrorInfo
|
||||
|
||||
|
||||
def test_phase10_site1_list_gemini_models_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_list_gemini_models_result"), \
|
||||
"_list_gemini_models_result helper missing from src.ai_client"
|
||||
|
||||
|
||||
def test_phase10_site1_list_gemini_models_result_returns_result():
|
||||
"""The helper must return a Result[list[str]]."""
|
||||
import src.ai_client
|
||||
fn = getattr(src.ai_client, "_list_gemini_models_result", None)
|
||||
assert fn is not None
|
||||
import inspect
|
||||
sig = inspect.signature(fn)
|
||||
# Should have a return annotation of Result
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_list_gemini_models_result return annotation must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase10_site1_list_gemini_models_legacy_unchanged():
|
||||
"""Legacy _list_gemini_models must still return list[str] (preserve signature)."""
|
||||
import src.ai_client
|
||||
fn = getattr(src.ai_client, "_list_gemini_models", None)
|
||||
assert fn is not None
|
||||
import inspect
|
||||
sig = inspect.signature(fn)
|
||||
assert "list[str]" in str(sig.return_annotation) or "list" in str(sig.return_annotation), \
|
||||
f"_list_gemini_models return annotation must remain list[str], got {sig.return_annotation}"
|
||||
Reference in New Issue
Block a user