From 65608831f88b06d35c7978d715e2c146839b700f Mon Sep 17 00:00:00 2001 From: Yijia-Xiao Date: Sat, 13 Jun 2026 21:11:25 +0000 Subject: [PATCH] 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. --- tests/conftest.py | 19 +++++ tests/test_vendor_routing.py | 102 +++++++++++++++++++++++++++ tradingagents/dataflows/interface.py | 49 +++++++++---- tradingagents/default_config.py | 5 +- 4 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 tests/test_vendor_routing.py diff --git a/tests/conftest.py b/tests/conftest.py index 506510cea..6930599a7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -35,6 +35,25 @@ def _dummy_api_keys(monkeypatch): 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() def mock_llm_client(): client = MagicMock() diff --git a/tests/test_vendor_routing.py b/tests/test_vendor_routing.py new file mode 100644 index 000000000..57e36a134 --- /dev/null +++ b/tests/test_vendor_routing.py @@ -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() diff --git a/tradingagents/dataflows/interface.py b/tradingagents/dataflows/interface.py index d073cb079..5897bbbb9 100644 --- a/tradingagents/dataflows/interface.py +++ b/tradingagents/dataflows/interface.py @@ -1,3 +1,4 @@ +import logging from typing import Annotated # Import from vendor-specific modules @@ -28,6 +29,8 @@ from .symbol_utils import NoMarketDataError # Configuration and routing logic from .config import get_config +logger = logging.getLogger(__name__) + # Tools organized by category TOOLS_CATEGORIES = { "core_stock_apis": { @@ -141,34 +144,43 @@ def route_to_vendor(method: str, *args, **kwargs): if method not in VENDOR_METHODS: 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()) - fallback_vendors = primary_vendors.copy() - for vendor in all_available_vendors: - if vendor not in fallback_vendors: - fallback_vendors.append(vendor) + + # The configured vendor list IS the chain: we do NOT silently fall back to + # vendors the user did not choose (#988/#289) — that returned data from an + # 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 first_error: Exception | None = None - for vendor in fallback_vendors: - if vendor not in VENDOR_METHODS[method]: - continue - + for vendor in vendor_chain: vendor_impl = VENDOR_METHODS[method][vendor] impl_func = vendor_impl[0] if isinstance(vendor_impl, list) else vendor_impl try: return impl_func(*args, **kwargs) 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: - 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 except Exception as e: - # A fallback vendor failing for an incidental reason (e.g. no API - # key configured) must not crash the call when another vendor - # already determined the symbol simply has no data. Remember the - # first error so a genuine primary-vendor failure still surfaces. + # Don't let one vendor's failure crash the call when another can + # serve it, but never swallow silently: a broken primary must be + # visible in the logs (#989), not hidden behind a fallback's verdict. + logger.warning("Vendor %r failed for %s: %s", vendor, method, e) if first_error is None: first_error = e continue @@ -178,6 +190,13 @@ def route_to_vendor(method: str, *args, **kwargs): # empty string, so the agent reports "unavailable" instead of inventing a # value. This takes precedence over incidental fallback errors. 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 canonical = last_no_data.canonical resolved = "" if canonical == sym else f" (resolved to '{canonical}')" diff --git a/tradingagents/default_config.py b/tradingagents/default_config.py index d9fd83f65..e195c62ee 100644 --- a/tradingagents/default_config.py +++ b/tradingagents/default_config.py @@ -97,7 +97,10 @@ DEFAULT_CONFIG = _apply_env_overrides({ "oil commodities supply chain energy", ], # 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": { "core_stock_apis": "yfinance", # Options: alpha_vantage, yfinance "technical_indicators": "yfinance", # Options: alpha_vantage, yfinance