refactor(ai_client): obliterate 5 legacy model-list wrappers (Phase 4)

Phase 4 (5 of 9 cruft sites obliterated):

OBLITERATED wrappers:
1. _reread_file_items (4 callers in _send_gemini + _send_gemini_cli + 2 others)
2. _list_anthropic_models (1 caller in list_models)
3. _list_gemini_models (1 caller in list_models)
4. _extract_gemini_thoughts (1 caller in _send_gemini)
5. _list_minimax_models (2 callers in _set_minimax_provider_result + set_provider)

Migration: each caller now uses the _result sibling directly with .ok check
+ .data extraction. The Result[T] error context (structured ErrorInfo) is now
propagated instead of dropped. _send_gemini gets .data with explicit .ok check.

Updated tests to assert OBLITERATED state (5 sub-track 5 tests inverted from
'_legacy_preserved' to '_legacy_obliterated'):
- tests/test_baseline_result.py: test_phase9_redo_modules_import_cleanly
- tests/tier2/phase10_invariant_test.py: _list_gemini_models removed from list
- tests/tier2/phase10_site1_test.py: _legacy_unchanged -> _legacy_obliterated
- tests/tier2/phase11_invariant_test.py: _extract/_list_minimax moved to obliterated
- tests/tier2/phase11_sites78_test.py: _legacy_preserved -> _legacy_obliterated
- tests/tier2/phase12_invariant_test.py: _list_anthropic moved to obliterated
- tests/tier2/phase12_site4_test.py: _legacy_preserved -> _legacy_obliterated
- tests/test_gemini_thinking_format.py: helper uses _result directly
- tests/test_cruft_removal.py: 5 new obliterated-wrappers invariant tests

Test result: 122/122 pass (31 baseline + 16 heuristic + 9 cruft + 5 thinking + 61 tier2).
Audit gate: src/ai_client.py --strict exits 0 (no new violations introduced).
Wrapper count: 9 -> 3 (Phase 5-6 remaining: rag_engine 1, gui_2 2).

tests/test_baseline_result.py

@@ -359,4 +359,7 @@ def test_phase9_redo_modules_import_cleanly():
  import src.ai_client
  # The legacy string-returning functions should still exist for backward compat.
  assert callable(getattr(src.ai_client, "set_provider", None))
- assert callable(getattr(src.ai_client, "_reread_file_items", None))
+ # _reread_file_items wrapper was OBLITERATED by cruft-removal Phase 4
+ assert not hasattr(src.ai_client, "_reread_file_items"), (
+  "_reread_file_items wrapper should be deleted; use _reread_file_items_result directly"
+ )

tests/test_cruft_removal.py

@@ -81,4 +81,43 @@ def test_audit_script_finds_zero_mcp_client_wrappers():
     )
     assert "src\\mcp_client.py" not in r.stdout, (
         f"expected 0 wrappers in src/mcp_client.py, but found:\n{r.stdout}"
+    )
+
+
+# ============ Phase 4 (ai_client wrappers) ============
+
+def test_phase4_reread_file_items_wrapper_obliterated():
+    """Phase 4 invariant: the legacy _reread_file_items wrapper is DELETED."""
+    from src import ai_client
+    assert not hasattr(ai_client, "_reread_file_items"), (
+        "_reread_file_items legacy wrapper must be OBLITERATED. "
+        "Callers must use _reread_file_items_result(...).ok directly."
+    )
+
+
+def test_phase4_list_anthropic_models_wrapper_obliterated():
+    from src import ai_client
+    assert not hasattr(ai_client, "_list_anthropic_models"), (
+        "_list_anthropic_models legacy wrapper must be OBLITERATED."
+    )
+
+
+def test_phase4_list_gemini_models_wrapper_obliterated():
+    from src import ai_client
+    assert not hasattr(ai_client, "_list_gemini_models"), (
+        "_list_gemini_models legacy wrapper must be OBLITERATED."
+    )
+
+
+def test_phase4_extract_gemini_thoughts_wrapper_obliterated():
+    from src import ai_client
+    assert not hasattr(ai_client, "_extract_gemini_thoughts"), (
+        "_extract_gemini_thoughts legacy wrapper must be OBLITERATED."
+    )
+
+
+def test_phase4_list_minimax_models_wrapper_obliterated():
+    from src import ai_client
+    assert not hasattr(ai_client, "_list_minimax_models"), (
+        "_list_minimax_models legacy wrapper must be OBLITERATED."
     )

tests/test_gemini_thinking_format.py

@@ -9,10 +9,18 @@ ThinkingSegment.
 from unittest.mock import MagicMock, patch
 from google.genai.types import Part, Content, Candidate, GenerateContentResponse
 
-from src.ai_client import _extract_gemini_thoughts
+from src.ai_client import _extract_gemini_thoughts_result
 from src.thinking_parser import parse_thinking_trace
 
 
+def _call(resp):
+    """Migration helper: _extract_gemini_thoughts wrapper was OBLITERATED (cruft-removal Phase 4).
+    Call _extract_gemini_thoughts_result(...).data directly; on failure return empty string.
+    """
+    result = _extract_gemini_thoughts_result(resp)
+    return result.data if result.ok else ""
+
+
 def test_extract_gemini_thoughts_returns_thinking_only() -> None:
  """The helper must return concatenated thought=True parts and ignore thought=False parts."""
  resp = GenerateContentResponse(
@@ -23,7 +31,7 @@ def test_extract_gemini_thoughts_returns_thinking_only() -> None:
    Part(text="visible text 2"),
   ]))]
  )
- thoughts = _extract_gemini_thoughts(resp)
+ thoughts = _call(resp)
  assert thoughts == "step 1 reasoningstep 2 reasoning"
 
 
@@ -32,7 +40,7 @@ def test_extract_gemini_thoughts_returns_empty_when_no_thoughts() -> None:
  resp = GenerateContentResponse(
   candidates=[Candidate(content=Content(parts=[Part(text="just visible")]))]
  )
- assert _extract_gemini_thoughts(resp) == ""
+ assert _call(resp) == ""
 
 
 def test_extract_gemini_thoughts_handles_missing_attributes() -> None:
@@ -40,9 +48,9 @@ def test_extract_gemini_thoughts_handles_missing_attributes() -> None:
  fake = MagicMock()
  fake.candidates = [MagicMock()]
  fake.candidates[0].content.parts = [MagicMock(thought=True, text="thinking text")]
- assert _extract_gemini_thoughts(fake) == "thinking text"
+ assert _call(fake) == "thinking text"
  fake.candidates = []
- assert _extract_gemini_thoughts(fake) == ""
+ assert _call(fake) == ""
 
 
 def test_gemini_thinking_segment_extractable_after_wrap() -> None:
@@ -53,7 +61,7 @@ def test_gemini_thinking_segment_extractable_after_wrap() -> None:
    Part(text="final answer"),
   ]))]
  )
- thoughts = _extract_gemini_thoughts(resp)
+ thoughts = _call(resp)
  wrapped = f"<thinking>\n{thoughts}\n</thinking>\n\nfinal answer"
  segments, response = parse_thinking_trace(wrapped)
  assert len(segments) == 1
@@ -64,7 +72,7 @@ def test_gemini_thinking_segment_extractable_after_wrap() -> None:
 
 def test_extract_gemini_thoughts_handles_none_resp() -> None:
  """Defensive: must not crash on None response."""
- assert _extract_gemini_thoughts(None) == ""
+ assert _call(None) == ""
 
 
 if __name__ == "__main__":

tests/tier2/phase10_invariant_test.py

@@ -48,16 +48,22 @@ def test_phase10_all_helpers_exist():
 
 
 def test_phase10_legacy_functions_preserved():
-    """All legacy functions must still be callable with original signatures."""
+    """Legacy functions preserved EXCEPT those OBLITERATED by cruft-removal Phase 4."""
     import src.ai_client
     legacy = [
-        "_list_gemini_models",
         "_send_gemini",
         "_send_gemini_cli",
         "run_tier4_analysis",
         "run_tier4_patch_callback",
         "run_tier4_patch_generation",
     ]
+    # _list_gemini_models wrapper was OBLITERATED by cruft-removal Phase 4
+    obliterated = ["_list_gemini_models"]
     for name in legacy:
         assert hasattr(src.ai_client, name), f"{name} legacy function missing"
-        assert callable(getattr(src.ai_client, name)), f"{name} not callable"
+        assert callable(getattr(src.ai_client, name)), f"{name} not callable"
+    for name in obliterated:
+        assert not hasattr(src.ai_client, name), (
+            f"{name} wrapper must be OBLITERATED (cruft-removal Phase 4); "
+            f"callers must use {name}_result directly"
+        )

tests/tier2/phase10_site1_test.py

@@ -26,12 +26,10 @@ def test_phase10_site1_list_gemini_models_result_returns_result():
         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)."""
+def test_phase10_site1_list_gemini_models_legacy_obliterated():
+    """Legacy _list_gemini_models wrapper OBLITERATED (cruft-removal Phase 4)."""
     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}"
+    assert not hasattr(src.ai_client, "_list_gemini_models"), (
+        "_list_gemini_models legacy wrapper must be DELETED; "
+        "callers must use _list_gemini_models_result(...).ok directly."
+    )

tests/tier2/phase11_invariant_test.py

@@ -62,7 +62,7 @@ def test_phase11_all_helpers_exist():
 
 
 def test_phase11_legacy_functions_preserved():
-    """All legacy functions must still be callable."""
+    """All legacy functions must still be callable (some OBLITERATED by cruft-removal Phase 4)."""
     import src.ai_client
     legacy = [
         "_classify_anthropic_error",
@@ -71,10 +71,15 @@ def test_phase11_legacy_functions_preserved():
         "reset_session",
         "set_tool_preset",
         "set_bias_profile",
-        "_extract_gemini_thoughts",
-        "_list_minimax_models",
         "get_token_stats",
     ]
+    # OBLITERATED by cruft-removal Phase 4 (no backward compat):
+    obliterated = ["_extract_gemini_thoughts", "_list_minimax_models"]
     for name in legacy:
         assert hasattr(src.ai_client, name), f"{name} legacy function missing"
-        assert callable(getattr(src.ai_client, name)), f"{name} not callable"
+        assert callable(getattr(src.ai_client, name)), f"{name} not callable"
+    for name in obliterated:
+        assert not hasattr(src.ai_client, name), (
+            f"{name} wrapper must be OBLITERATED (cruft-removal Phase 4); "
+            f"callers must use {name}_result directly"
+        )

tests/tier2/phase11_sites78_test.py

@@ -46,7 +46,12 @@ def test_phase11_sites78_helpers_return_result():
             f"{name} return must be Result, got {sig.return_annotation}"
 
 
-def test_phase11_sites78_legacy_preserved():
+def test_phase11_sites78_legacy_obliterated():
+    """Legacy wrappers OBLITERATED (cruft-removal Phase 4)."""
     import src.ai_client
-    assert callable(getattr(src.ai_client, "_extract_gemini_thoughts", None))
-    assert callable(getattr(src.ai_client, "_list_minimax_models", None))
+    assert not hasattr(src.ai_client, "_extract_gemini_thoughts"), (
+        "_extract_gemini_thoughts legacy wrapper must be DELETED."
+    )
+    assert not hasattr(src.ai_client, "_list_minimax_models"), (
+        "_list_minimax_models legacy wrapper must be DELETED."
+    )

tests/tier2/phase12_invariant_test.py

@@ -43,14 +43,18 @@ def test_phase12_list_anthropic_models_result_exists():
 
 
 def test_phase12_legacy_functions_preserved():
-    """Legacy functions must still exist."""
+    """Legacy functions preserved EXCEPT _list_anthropic_models OBLITERATED by cruft-removal Phase 4."""
     import src.ai_client
-    for name in ("_load_credentials",
-                 "_list_anthropic_models",
-                 "_default_send",
-                 "_dashscope_call"):
-        assert hasattr(src.ai_client, name) or name == "_default_send", \
-            f"{name} legacy function missing"
-        # _default_send is nested; check via run_with_tool_loop
-    # The nested _default_send is part of run_with_tool_loop
-    assert callable(getattr(src.ai_client, "run_with_tool_loop", None))
+    # _default_send is nested inside run_with_tool_loop; check via getattr with fallback
+    preserved = ("_load_credentials", "_dashscope_call")
+    obliterated = ("_list_anthropic_models",)
+    for name in preserved:
+        assert hasattr(src.ai_client, name), f"{name} legacy function missing"
+    # The nested _default_send is part of run_with_tool_loop (not a top-level attr)
+    assert callable(getattr(src.ai_client, "run_with_tool_loop", None)), \
+        "run_with_tool_loop must exist (contains nested _default_send)"
+    for name in obliterated:
+        assert not hasattr(src.ai_client, name), (
+            f"{name} wrapper must be OBLITERATED (cruft-removal Phase 4); "
+            f"callers must use {name}_result directly"
+        )

tests/tier2/phase12_site4_test.py

@@ -27,15 +27,10 @@ def test_phase12_site4_helper_returns_result():
         f"_list_anthropic_models_result return must be Result, got {sig.return_annotation}"
 
 
-def test_phase12_site4_legacy_no_broken_raise():
-    """Legacy _list_anthropic_models must NOT raise _classify_anthropic_error result (the ErrorInfo-as-Exception bug)."""
-    import inspect
+def test_phase12_site4_legacy_obliterated():
+    """_list_anthropic_models wrapper OBLITERATED by cruft-removal Phase 4."""
     import src.ai_client
-    src_text = inspect.getsource(src.ai_client._list_anthropic_models)
-    assert "raise _classify_anthropic_error" not in src_text, \
-        "_list_anthropic_models legacy must NOT raise ErrorInfo as Exception"
-
-
-def test_phase12_site4_legacy_preserved():
-    import src.ai_client
-    assert callable(getattr(src.ai_client, "_list_anthropic_models", None))
+    assert not hasattr(src.ai_client, "_list_anthropic_models"), (
+        "_list_anthropic_models wrapper must be DELETED; "
+        "callers must use _list_anthropic_models_result(...).ok directly."
+    )