diff --git a/tests/test_llm_max_retries.py b/tests/test_llm_max_retries.py index 632060a5e..a32416127 100644 --- a/tests/test_llm_max_retries.py +++ b/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 ----------------------------------------------------------- diff --git a/tests/test_llm_max_tokens.py b/tests/test_llm_max_tokens.py index 87cddba52..85e8760bb 100644 --- a/tests/test_llm_max_tokens.py +++ b/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 -------------------------------- diff --git a/tests/test_temperature_config.py b/tests/test_temperature_config.py index 758f80822..502182fd1 100644 --- a/tests/test_temperature_config.py +++ b/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 - # Call the method without constructing the full graph. - graph = TradingAgentsGraph.__new__(TradingAgentsGraph) - graph.config = {"llm_provider": "openai", "temperature": temperature} - return TradingAgentsGraph._get_provider_kwargs(graph) + from tradingagents.llm_clients import build_llm_kwargs + return build_llm_kwargs({"llm_provider": "openai", "temperature": temperature}) def test_float_string_coerced(self): assert self._kwargs_for("0.3")["temperature"] == 0.3 diff --git a/tradingagents/graph/trading_graph.py b/tradingagents/graph/trading_graph.py index 7a0c77ced..e896b9152 100644 --- a/tradingagents/graph/trading_graph.py +++ b/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``. diff --git a/tradingagents/llm_clients/__init__.py b/tradingagents/llm_clients/__init__.py index e528eabef..fafb5e2dd 100644 --- a/tradingagents/llm_clients/__init__.py +++ b/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"] diff --git a/tradingagents/llm_clients/factory.py b/tradingagents/llm_clients/factory.py index 02c1cf428..14d7489b2 100644 --- a/tradingagents/llm_clients/factory.py +++ b/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