mirror of
https://github.com/TauricResearch/TradingAgents.git
synced 2026-06-16 21:06:15 +03:00
fix(data): respect the configured vendor chain and log vendor failures
The router silently extended every request to all available vendors regardless of config, so an explicit single-vendor choice still fell back to others and returned data from an unexpected source (#988, #289), and serious primary-vendor errors were swallowed without a trace (#989). The configured vendor list is now the exact chain (list several for ordered fallback; "default" uses all), unknown vendors raise, and swallowed vendor errors are logged. Adds an autouse config isolation fixture so vendor config can't leak between tests.
This commit is contained in:
@@ -35,6 +35,25 @@ def _dummy_api_keys(monkeypatch):
|
|||||||
monkeypatch.setenv(env_var, os.environ.get(env_var, "placeholder"))
|
monkeypatch.setenv(env_var, os.environ.get(env_var, "placeholder"))
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _isolate_config():
|
||||||
|
"""Reset the global dataflows config before and after each test.
|
||||||
|
|
||||||
|
``set_config`` merges (it never clears keys absent from the override), so a
|
||||||
|
test that sets e.g. ``tool_vendors`` would otherwise leak into later tests
|
||||||
|
and make routing behavior order-dependent. Replace the global outright so
|
||||||
|
every test starts from a clean DEFAULT_CONFIG.
|
||||||
|
"""
|
||||||
|
import copy
|
||||||
|
|
||||||
|
import tradingagents.dataflows.config as config_module
|
||||||
|
import tradingagents.default_config as default_config
|
||||||
|
|
||||||
|
config_module._config = copy.deepcopy(default_config.DEFAULT_CONFIG)
|
||||||
|
yield
|
||||||
|
config_module._config = copy.deepcopy(default_config.DEFAULT_CONFIG)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture()
|
@pytest.fixture()
|
||||||
def mock_llm_client():
|
def mock_llm_client():
|
||||||
client = MagicMock()
|
client = MagicMock()
|
||||||
|
|||||||
102
tests/test_vendor_routing.py
Normal file
102
tests/test_vendor_routing.py
Normal file
@@ -0,0 +1,102 @@
|
|||||||
|
"""Vendor router must respect the configured chain and never silently hide a
|
||||||
|
broken primary.
|
||||||
|
|
||||||
|
Regressions for #988 (explicit single-vendor config still fell back to others),
|
||||||
|
#289 (fallback ran for unchosen vendors), and #989 (serious primary failures
|
||||||
|
were swallowed without a trace).
|
||||||
|
"""
|
||||||
|
import copy
|
||||||
|
import logging
|
||||||
|
import unittest
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
import tradingagents.dataflows.config as config_module
|
||||||
|
import tradingagents.default_config as default_config
|
||||||
|
from tradingagents.dataflows import interface
|
||||||
|
from tradingagents.dataflows.config import set_config
|
||||||
|
from tradingagents.dataflows.symbol_utils import NoMarketDataError
|
||||||
|
|
||||||
|
|
||||||
|
def _reset_config():
|
||||||
|
# Hard reset: set_config() merges, so empty DEFAULT dicts (e.g. tool_vendors)
|
||||||
|
# don't clear keys leaked by other tests. Replace the global outright.
|
||||||
|
config_module._config = copy.deepcopy(default_config.DEFAULT_CONFIG)
|
||||||
|
|
||||||
|
|
||||||
|
def _no_data(symbol, *a, **k):
|
||||||
|
raise NoMarketDataError(symbol, symbol, "no rows")
|
||||||
|
|
||||||
|
|
||||||
|
def _returns(value):
|
||||||
|
def impl(symbol, *a, **k):
|
||||||
|
return value
|
||||||
|
return impl
|
||||||
|
|
||||||
|
|
||||||
|
def _raises(exc):
|
||||||
|
def impl(symbol, *a, **k):
|
||||||
|
raise exc
|
||||||
|
return impl
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class VendorRoutingTests(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
_reset_config()
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
_reset_config()
|
||||||
|
|
||||||
|
def _route(self, vendors_for_get_stock_data):
|
||||||
|
return mock.patch.dict(
|
||||||
|
interface.VENDOR_METHODS,
|
||||||
|
{"get_stock_data": vendors_for_get_stock_data},
|
||||||
|
clear=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_explicit_single_vendor_does_not_fall_back(self):
|
||||||
|
# #988: with yfinance pinned, a healthy alpha_vantage must NOT be used.
|
||||||
|
set_config({"data_vendors": {"core_stock_apis": "yfinance"}})
|
||||||
|
av = mock.Mock(side_effect=_returns("AV_DATA"))
|
||||||
|
with self._route({"yfinance": _no_data, "alpha_vantage": av}):
|
||||||
|
result = interface.route_to_vendor("get_stock_data", "FAKE", "2026-01-01", "2026-01-10")
|
||||||
|
self.assertIn("NO_DATA_AVAILABLE", result)
|
||||||
|
av.assert_not_called() # the unchosen vendor was never tried
|
||||||
|
|
||||||
|
def test_explicit_multi_vendor_falls_back_within_chain(self):
|
||||||
|
# Listing both vendors opts in to ordered fallback.
|
||||||
|
set_config({"data_vendors": {"core_stock_apis": "yfinance,alpha_vantage"}})
|
||||||
|
with self._route({"yfinance": _no_data, "alpha_vantage": _returns("AV_DATA")}):
|
||||||
|
result = interface.route_to_vendor("get_stock_data", "AAPL", "2026-01-01", "2026-01-10")
|
||||||
|
self.assertEqual(result, "AV_DATA")
|
||||||
|
|
||||||
|
def test_primary_error_is_logged_not_masked(self):
|
||||||
|
# #989: primary errors + fallback no-data -> NO_DATA, but the failure
|
||||||
|
# must be visible in logs (broken primary not hidden).
|
||||||
|
set_config({"data_vendors": {"core_stock_apis": "yfinance,alpha_vantage"}})
|
||||||
|
with self._route({"yfinance": _raises(ValueError("boom")), "alpha_vantage": _no_data}):
|
||||||
|
with self.assertLogs("tradingagents.dataflows.interface", level="WARNING") as cm:
|
||||||
|
result = interface.route_to_vendor("get_stock_data", "AAPL", "2026-01-01", "2026-01-10")
|
||||||
|
self.assertIn("NO_DATA_AVAILABLE", result)
|
||||||
|
joined = "\n".join(cm.output)
|
||||||
|
self.assertIn("boom", joined) # the real error surfaced in logs
|
||||||
|
self.assertIn("yfinance", joined)
|
||||||
|
|
||||||
|
def test_unknown_configured_vendor_raises(self):
|
||||||
|
set_config({"data_vendors": {"core_stock_apis": "bogus_vendor"}})
|
||||||
|
with self.assertRaises(ValueError) as ctx:
|
||||||
|
interface.route_to_vendor("get_stock_data", "AAPL", "2026-01-01", "2026-01-10")
|
||||||
|
self.assertIn("bogus_vendor", str(ctx.exception))
|
||||||
|
|
||||||
|
def test_default_sentinel_uses_all_vendors(self):
|
||||||
|
# No explicit choice ("default") keeps the resilient full-chain behavior.
|
||||||
|
set_config({"data_vendors": {"core_stock_apis": "default"}})
|
||||||
|
with self._route({"yfinance": _no_data, "alpha_vantage": _returns("AV_DATA")}):
|
||||||
|
result = interface.route_to_vendor("get_stock_data", "AAPL", "2026-01-01", "2026-01-10")
|
||||||
|
self.assertEqual(result, "AV_DATA")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import logging
|
||||||
from typing import Annotated
|
from typing import Annotated
|
||||||
|
|
||||||
# Import from vendor-specific modules
|
# Import from vendor-specific modules
|
||||||
@@ -28,6 +29,8 @@ from .symbol_utils import NoMarketDataError
|
|||||||
# Configuration and routing logic
|
# Configuration and routing logic
|
||||||
from .config import get_config
|
from .config import get_config
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
# Tools organized by category
|
# Tools organized by category
|
||||||
TOOLS_CATEGORIES = {
|
TOOLS_CATEGORIES = {
|
||||||
"core_stock_apis": {
|
"core_stock_apis": {
|
||||||
@@ -141,34 +144,43 @@ def route_to_vendor(method: str, *args, **kwargs):
|
|||||||
if method not in VENDOR_METHODS:
|
if method not in VENDOR_METHODS:
|
||||||
raise ValueError(f"Method '{method}' not supported")
|
raise ValueError(f"Method '{method}' not supported")
|
||||||
|
|
||||||
# Build fallback chain: primary vendors first, then remaining available vendors
|
|
||||||
all_available_vendors = list(VENDOR_METHODS[method].keys())
|
all_available_vendors = list(VENDOR_METHODS[method].keys())
|
||||||
fallback_vendors = primary_vendors.copy()
|
|
||||||
for vendor in all_available_vendors:
|
# The configured vendor list IS the chain: we do NOT silently fall back to
|
||||||
if vendor not in fallback_vendors:
|
# vendors the user did not choose (#988/#289) — that returned data from an
|
||||||
fallback_vendors.append(vendor)
|
# unexpected source and caused cross-vendor inconsistencies. For multi-vendor
|
||||||
|
# fallback, list them in order, e.g. data_vendors="yfinance,alpha_vantage".
|
||||||
|
# The "default" sentinel (no explicit config) uses all available vendors.
|
||||||
|
explicit = [v for v in primary_vendors if v and v != "default"]
|
||||||
|
if explicit:
|
||||||
|
vendor_chain = [v for v in explicit if v in VENDOR_METHODS[method]]
|
||||||
|
if not vendor_chain:
|
||||||
|
raise ValueError(
|
||||||
|
f"Configured vendor(s) {explicit} not available for '{method}'. "
|
||||||
|
f"Available: {all_available_vendors}."
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
vendor_chain = all_available_vendors
|
||||||
|
|
||||||
last_no_data: NoMarketDataError | None = None
|
last_no_data: NoMarketDataError | None = None
|
||||||
first_error: Exception | None = None
|
first_error: Exception | None = None
|
||||||
for vendor in fallback_vendors:
|
for vendor in vendor_chain:
|
||||||
if vendor not in VENDOR_METHODS[method]:
|
|
||||||
continue
|
|
||||||
|
|
||||||
vendor_impl = VENDOR_METHODS[method][vendor]
|
vendor_impl = VENDOR_METHODS[method][vendor]
|
||||||
impl_func = vendor_impl[0] if isinstance(vendor_impl, list) else vendor_impl
|
impl_func = vendor_impl[0] if isinstance(vendor_impl, list) else vendor_impl
|
||||||
|
|
||||||
try:
|
try:
|
||||||
return impl_func(*args, **kwargs)
|
return impl_func(*args, **kwargs)
|
||||||
except AlphaVantageRateLimitError:
|
except AlphaVantageRateLimitError:
|
||||||
continue # Rate limits: try the next vendor
|
logger.warning("Vendor %r rate-limited for %s; trying next vendor.", vendor, method)
|
||||||
|
continue
|
||||||
except NoMarketDataError as e:
|
except NoMarketDataError as e:
|
||||||
last_no_data = e # No data here; another vendor may have it
|
last_no_data = e # No data here; another configured vendor may have it
|
||||||
continue
|
continue
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
# A fallback vendor failing for an incidental reason (e.g. no API
|
# Don't let one vendor's failure crash the call when another can
|
||||||
# key configured) must not crash the call when another vendor
|
# serve it, but never swallow silently: a broken primary must be
|
||||||
# already determined the symbol simply has no data. Remember the
|
# visible in the logs (#989), not hidden behind a fallback's verdict.
|
||||||
# first error so a genuine primary-vendor failure still surfaces.
|
logger.warning("Vendor %r failed for %s: %s", vendor, method, e)
|
||||||
if first_error is None:
|
if first_error is None:
|
||||||
first_error = e
|
first_error = e
|
||||||
continue
|
continue
|
||||||
@@ -178,6 +190,13 @@ def route_to_vendor(method: str, *args, **kwargs):
|
|||||||
# empty string, so the agent reports "unavailable" instead of inventing a
|
# empty string, so the agent reports "unavailable" instead of inventing a
|
||||||
# value. This takes precedence over incidental fallback errors.
|
# value. This takes precedence over incidental fallback errors.
|
||||||
if last_no_data is not None:
|
if last_no_data is not None:
|
||||||
|
if first_error is not None:
|
||||||
|
# A vendor also hit a real error; surface it in logs so the no-data
|
||||||
|
# verdict can't hide a broken primary (network/auth/etc.).
|
||||||
|
logger.warning(
|
||||||
|
"Returning NO_DATA for %s, but a vendor errored earlier: %s",
|
||||||
|
method, first_error,
|
||||||
|
)
|
||||||
sym = last_no_data.symbol
|
sym = last_no_data.symbol
|
||||||
canonical = last_no_data.canonical
|
canonical = last_no_data.canonical
|
||||||
resolved = "" if canonical == sym else f" (resolved to '{canonical}')"
|
resolved = "" if canonical == sym else f" (resolved to '{canonical}')"
|
||||||
|
|||||||
@@ -97,7 +97,10 @@ DEFAULT_CONFIG = _apply_env_overrides({
|
|||||||
"oil commodities supply chain energy",
|
"oil commodities supply chain energy",
|
||||||
],
|
],
|
||||||
# Data vendor configuration
|
# Data vendor configuration
|
||||||
# Category-level configuration (default for all tools in category)
|
# Category-level configuration (default for all tools in category).
|
||||||
|
# The configured value is the exact vendor chain — requests are NOT silently
|
||||||
|
# routed to vendors you didn't choose. For ordered fallback, list several,
|
||||||
|
# e.g. "yfinance,alpha_vantage". "default" uses all available vendors.
|
||||||
"data_vendors": {
|
"data_vendors": {
|
||||||
"core_stock_apis": "yfinance", # Options: alpha_vantage, yfinance
|
"core_stock_apis": "yfinance", # Options: alpha_vantage, yfinance
|
||||||
"technical_indicators": "yfinance", # Options: alpha_vantage, yfinance
|
"technical_indicators": "yfinance", # Options: alpha_vantage, yfinance
|
||||||
|
|||||||
Reference in New Issue
Block a user