refactor(llm_clients): build the client keyword arguments in llm_clients

- build_llm_kwargs(config) replaces the graph's _get_provider_kwargs, with the retry and token coercion beside it

tests/test_llm_max_retries.py

@@ -11,7 +11,7 @@ import importlib
 import pytest
 
 import tradingagents.default_config as default_config_module
-from tradingagents.graph.trading_graph import TradingAgentsGraph, _coerce_max_retries
+from tradingagents.llm_clients.factory import _coerce_max_retries, build_llm_kwargs
 
 # --- coercion / validation -------------------------------------------------
 
@@ -44,36 +44,31 @@ def test_coerce_rejects_non_integers(bad):
 
 # --- forwarding into provider kwargs --------------------------------------
 
-def _bare_graph(config):
-    g = object.__new__(TradingAgentsGraph)
-    g.config = config
-    return g
-
 
 @pytest.mark.unit
 def test_not_forwarded_when_unset():
-    kwargs = _bare_graph({"llm_provider": "openai", "llm_max_retries": None})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": "openai", "llm_max_retries": None})
     assert "max_retries" not in kwargs
 
 
 @pytest.mark.unit
 @pytest.mark.parametrize("provider", ["openai", "anthropic", "google"])
 def test_forwarded_across_providers(provider):
-    kwargs = _bare_graph({"llm_provider": provider, "llm_max_retries": 6})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": provider, "llm_max_retries": 6})
     assert kwargs["max_retries"] == 6
 
 
 @pytest.mark.unit
 def test_forwarded_env_string_is_coerced():
     # env vars arrive as strings; the consumer coerces (like temperature)
-    kwargs = _bare_graph({"llm_provider": "openai", "llm_max_retries": "4"})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": "openai", "llm_max_retries": "4"})
     assert kwargs["max_retries"] == 4
 
 
 @pytest.mark.unit
 def test_invalid_config_value_fails_loudly():
     with pytest.raises(ValueError):
-        _bare_graph({"llm_provider": "openai", "llm_max_retries": -1})._get_provider_kwargs()
+        build_llm_kwargs({"llm_provider": "openai", "llm_max_retries": -1})
 
 
 # --- env overlay -----------------------------------------------------------

tests/test_llm_max_tokens.py

@@ -13,7 +13,7 @@ import importlib
 import pytest
 
 import tradingagents.default_config as default_config_module
-from tradingagents.graph.trading_graph import TradingAgentsGraph, _coerce_max_tokens
+from tradingagents.llm_clients.factory import _coerce_max_tokens, build_llm_kwargs
 
 # --- coercion / validation -------------------------------------------------
 
@@ -46,15 +46,10 @@ def test_coerce_rejects_non_integers(bad):
 
 # --- forwarding into provider kwargs (right key per provider) --------------
 
-def _bare_graph(config):
-    g = object.__new__(TradingAgentsGraph)
-    g.config = config
-    return g
-
 
 @pytest.mark.unit
 def test_not_forwarded_when_unset():
-    kwargs = _bare_graph({"llm_provider": "openai", "max_tokens": None})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": "openai", "max_tokens": None})
     assert "max_tokens" not in kwargs
     assert "max_output_tokens" not in kwargs
 
@@ -62,7 +57,7 @@ def test_not_forwarded_when_unset():
 @pytest.mark.unit
 @pytest.mark.parametrize("provider", ["openai", "anthropic", "deepseek", "openai_compatible"])
 def test_forwarded_as_max_tokens_for_non_google(provider):
-    kwargs = _bare_graph({"llm_provider": provider, "max_tokens": 8192})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": provider, "max_tokens": 8192})
     assert kwargs["max_tokens"] == 8192
     assert "max_output_tokens" not in kwargs
 
@@ -70,21 +65,21 @@ def test_forwarded_as_max_tokens_for_non_google(provider):
 @pytest.mark.unit
 def test_forwarded_as_max_output_tokens_for_google():
     # Gemini's kwarg name differs; forwarding plain max_tokens would be rejected.
-    kwargs = _bare_graph({"llm_provider": "google", "max_tokens": 8192})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": "google", "max_tokens": 8192})
     assert kwargs["max_output_tokens"] == 8192
     assert "max_tokens" not in kwargs
 
 
 @pytest.mark.unit
 def test_env_string_is_coerced():
-    kwargs = _bare_graph({"llm_provider": "openai", "max_tokens": "4096"})._get_provider_kwargs()
+    kwargs = build_llm_kwargs({"llm_provider": "openai", "max_tokens": "4096"})
     assert kwargs["max_tokens"] == 4096
 
 
 @pytest.mark.unit
 def test_invalid_value_fails_loudly():
     with pytest.raises(ValueError):
-        _bare_graph({"llm_provider": "openai", "max_tokens": 0})._get_provider_kwargs()
+        build_llm_kwargs({"llm_provider": "openai", "max_tokens": 0})
 
 
 # --- client-side allowlists carry the kwarg --------------------------------

tests/test_temperature_config.py

@@ -61,14 +61,11 @@ class TestTemperatureEnvOverlay:
 
 @pytest.mark.unit
 class TestProviderKwargsTemperature:
-    """_get_provider_kwargs float-coerces and forwards temperature, or omits it."""
+    """build_llm_kwargs float-coerces and forwards temperature, or omits it."""
 
     def _kwargs_for(self, temperature):
-        from tradingagents.graph.trading_graph import TradingAgentsGraph
+        from tradingagents.llm_clients import build_llm_kwargs
-        # Call the method without constructing the full graph.
+        return build_llm_kwargs({"llm_provider": "openai", "temperature": temperature})
-        graph = TradingAgentsGraph.__new__(TradingAgentsGraph)
-        graph.config = {"llm_provider": "openai", "temperature": temperature}
-        return TradingAgentsGraph._get_provider_kwargs(graph)
 
     def test_float_string_coerced(self):
         assert self._kwargs_for("0.3")["temperature"] == 0.3

tradingagents/graph/trading_graph.py

@@ -16,7 +16,7 @@ from tradingagents.dataflows.symbols import safe_ticker_component
 from tradingagents.dataflows.vendors.yahoo.market import get_closes
 from tradingagents.decision_log import TradingMemoryLog
 from tradingagents.default_config import DEFAULT_CONFIG
-from tradingagents.llm_clients import create_llm_client
+from tradingagents.llm_clients import build_llm_kwargs, create_llm_client
 from tradingagents.reporting import write_report_tree
 
 from .checkpointer import checkpoint_step, clear_checkpoint, get_checkpointer, thread_id
@@ -42,37 +42,6 @@ def _validate_trade_date(trade_date) -> str:
     return value
 
 
-def _coerce_max_retries(value):
-    """Validate an ``llm_max_retries`` value to a non-negative int.
-
-    Accepts an int or a numeric string (env vars arrive as strings). Rejects
-    booleans and negatives loudly so a misconfiguration fails at startup rather
-    than silently disabling retries.
-    """
-    if isinstance(value, bool):
-        raise ValueError(f"llm_max_retries must be an integer, not a boolean: {value!r}")
-    try:
-        n = int(value)
-    except (TypeError, ValueError) as exc:
-        raise ValueError(f"llm_max_retries must be an integer, got {value!r}") from exc
-    if n < 0:
-        raise ValueError(f"llm_max_retries must be >= 0, got {n}")
-    return n
-
-
-def _coerce_max_tokens(value):
-    """Validate a ``max_tokens`` value to a positive int (env vars are strings)."""
-    if isinstance(value, bool):
-        raise ValueError(f"max_tokens must be an integer, not a boolean: {value!r}")
-    try:
-        n = int(value)
-    except (TypeError, ValueError) as exc:
-        raise ValueError(f"max_tokens must be an integer, got {value!r}") from exc
-    if n <= 0:
-        raise ValueError(f"max_tokens must be > 0, got {n}")
-    return n
-
-
 class TradingAgentsGraph:
     """Main class that orchestrates the trading agents framework."""
 
@@ -103,7 +72,7 @@ class TradingAgentsGraph:
         os.makedirs(self.config["results_dir"], exist_ok=True)
 
         # Initialize LLMs with provider-specific thinking configuration
-        llm_kwargs = self._get_provider_kwargs()
+        llm_kwargs = build_llm_kwargs(self.config)
 
         # Add callbacks to kwargs if provided (passed to LLM constructor)
         if self.callbacks:
@@ -152,48 +121,6 @@ class TradingAgentsGraph:
         self._checkpointer_ctx = None
         self._resuming = False
 
-    def _get_provider_kwargs(self) -> dict[str, Any]:
-        """Get provider-specific kwargs for LLM client creation."""
-        kwargs = {}
-        provider = self.config.get("llm_provider", "").lower()
-
-        if provider == "google":
-            thinking_level = self.config.get("google_thinking_level")
-            if thinking_level:
-                kwargs["thinking_level"] = thinking_level
-
-        elif provider == "openai":
-            reasoning_effort = self.config.get("openai_reasoning_effort")
-            if reasoning_effort:
-                kwargs["reasoning_effort"] = reasoning_effort
-
-        elif provider == "anthropic":
-            effort = self.config.get("anthropic_effort")
-            if effort:
-                kwargs["effort"] = effort
-
-        # Sampling temperature is cross-provider: forward it whenever set.
-        # float() here so a value coming from a TRADINGAGENTS_TEMPERATURE env
-        # string ("0.2") works the same as a programmatic float.
-        temperature = self.config.get("temperature")
-        if temperature is not None and temperature != "":
-            kwargs["temperature"] = float(temperature)
-
-        # SDK retry budget is cross-provider. Forward it only when explicitly set
-        # so each provider keeps its own default (usually 2) otherwise (#1091).
-        max_retries = self.config.get("llm_max_retries")
-        if max_retries is not None and max_retries != "":
-            kwargs["max_retries"] = _coerce_max_retries(max_retries)
-
-        # Output-token cap is cross-provider, but Gemini names it
-        # ``max_output_tokens``; forward under the right key when set (#1204).
-        max_tokens = self.config.get("max_tokens")
-        if max_tokens is not None and max_tokens != "":
-            key = "max_output_tokens" if provider == "google" else "max_tokens"
-            kwargs[key] = _coerce_max_tokens(max_tokens)
-
-        return kwargs
-
     def _resolve_benchmark(self, ticker: str) -> str:
         """Pick the benchmark ticker for alpha calculation against ``ticker``.
 

tradingagents/llm_clients/__init__.py

@@ -1,4 +1,4 @@
 from .base_client import BaseLLMClient
-from .factory import create_llm_client
+from .factory import build_llm_kwargs, create_llm_client
 
-__all__ = ["BaseLLMClient", "create_llm_client"]
+__all__ = ["BaseLLMClient", "build_llm_kwargs", "create_llm_client"]

tradingagents/llm_clients/factory.py

@@ -1,4 +1,6 @@
 
+from typing import Any
+
 from .base_client import BaseLLMClient
 
 
@@ -52,3 +54,77 @@ def create_llm_client(
         return OpenAIClient(model, base_url, provider=provider_lower, **kwargs)
 
     raise ValueError(f"Unsupported LLM provider: {provider}")
+
+
+def _coerce_max_retries(value):
+    """Validate an ``llm_max_retries`` value to a non-negative int.
+
+    Accepts an int or a numeric string (env vars arrive as strings). Rejects
+    booleans and negatives loudly so a misconfiguration fails at startup rather
+    than silently disabling retries.
+    """
+    if isinstance(value, bool):
+        raise ValueError(f"llm_max_retries must be an integer, not a boolean: {value!r}")
+    try:
+        n = int(value)
+    except (TypeError, ValueError) as exc:
+        raise ValueError(f"llm_max_retries must be an integer, got {value!r}") from exc
+    if n < 0:
+        raise ValueError(f"llm_max_retries must be >= 0, got {n}")
+    return n
+
+
+def _coerce_max_tokens(value):
+    """Validate a ``max_tokens`` value to a positive int (env vars are strings)."""
+    if isinstance(value, bool):
+        raise ValueError(f"max_tokens must be an integer, not a boolean: {value!r}")
+    try:
+        n = int(value)
+    except (TypeError, ValueError) as exc:
+        raise ValueError(f"max_tokens must be an integer, got {value!r}") from exc
+    if n <= 0:
+        raise ValueError(f"max_tokens must be > 0, got {n}")
+    return n
+
+
+def build_llm_kwargs(config: dict) -> dict[str, Any]:
+    """Keyword arguments for ``create_llm_client`` from a TradingAgents config."""
+    kwargs = {}
+    provider = config.get("llm_provider", "").lower()
+
+    if provider == "google":
+        thinking_level = config.get("google_thinking_level")
+        if thinking_level:
+            kwargs["thinking_level"] = thinking_level
+
+    elif provider == "openai":
+        reasoning_effort = config.get("openai_reasoning_effort")
+        if reasoning_effort:
+            kwargs["reasoning_effort"] = reasoning_effort
+
+    elif provider == "anthropic":
+        effort = config.get("anthropic_effort")
+        if effort:
+            kwargs["effort"] = effort
+
+    # Sampling temperature is cross-provider: forward it whenever set.
+    # float() here so a value coming from a TRADINGAGENTS_TEMPERATURE env
+    # string ("0.2") works the same as a programmatic float.
+    temperature = config.get("temperature")
+    if temperature is not None and temperature != "":
+        kwargs["temperature"] = float(temperature)
+
+    # SDK retry budget is cross-provider. Forward it only when explicitly set
+    # so each provider keeps its own default (usually 2) otherwise (#1091).
+    max_retries = config.get("llm_max_retries")
+    if max_retries is not None and max_retries != "":
+        kwargs["max_retries"] = _coerce_max_retries(max_retries)
+
+    # Output-token cap is cross-provider, but Gemini names it
+    # ``max_output_tokens``; forward under the right key when set (#1204).
+    max_tokens = config.get("max_tokens")
+    if max_tokens is not None and max_tokens != "":
+        key = "max_output_tokens" if provider == "google" else "max_tokens"
+        kwargs[key] = _coerce_max_tokens(max_tokens)
+
+    return kwargs