diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b7207951..72942f5dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,5 +57,5 @@ jobs: run: pip install "ruff>=0.15" - name: Lint the repository # The repo is fully clean under the strict select, so we lint everything - # (results/ and worklog/ are excluded via pyproject extend-exclude). + # (generated results/ is excluded via pyproject extend-exclude). run: ruff check . diff --git a/pyproject.toml b/pyproject.toml index b6e2974a7..a543311e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,7 +69,7 @@ filterwarnings = [ [tool.ruff] line-length = 100 target-version = "py310" -extend-exclude = ["results", "worklog"] +extend-exclude = ["results"] [tool.ruff.lint] # Standard "good defaults" rule set (pyflakes + pycodestyle + isort + bugbear + diff --git a/tests/test_alpha_vantage_hardening.py b/tests/test_alpha_vantage_hardening.py index d82ea8dc3..494f2bac6 100644 --- a/tests/test_alpha_vantage_hardening.py +++ b/tests/test_alpha_vantage_hardening.py @@ -3,7 +3,8 @@ Regressions for #990 (no request timeout -> can hang), #991 (invalid-key responses mislabeled as rate limits and silently treated as transient), and #1115 (fundamentals look-ahead filter never ran because the payload is a JSON -string, not a dict). +string, not a dict), and the date trim that keeps post-end_date bars out of a +historical run. """ import json @@ -11,6 +12,7 @@ import pytest import tradingagents.dataflows.alpha_vantage_common as av import tradingagents.dataflows.alpha_vantage_fundamentals as avf +import tradingagents.dataflows.alpha_vantage_stock as avs class _FakeResponse: @@ -94,3 +96,40 @@ def test_fundamentals_no_curr_date_passes_through(monkeypatch): def test_fundamentals_non_json_body_unchanged(monkeypatch): monkeypatch.setattr(avf, "_make_api_request", lambda fn, params: "not-json") assert avf.get_cashflow("AAPL", curr_date="2024-01-01") == "not-json" + + +# --------------------------------------------------------------------------- +# Date trim (see the rationale on the unguarded trim in alpha_vantage_common) +# --------------------------------------------------------------------------- + +_DAILY_CSV = ( + "timestamp,open,high,low,close,volume\n" + "2024-05-13,1,1,1,1,10\n" # after end_date -> must never be served + "2024-05-10,1,1,1,1,10\n" + "2024-05-09,1,1,1,1,10\n" +) + + +@pytest.mark.unit +def test_stock_data_is_trimmed_to_the_requested_window(monkeypatch): + monkeypatch.setattr(avs, "_make_api_request", lambda *a, **k: _DAILY_CSV) + out = avs.get_stock("IBM", "2024-05-09", "2024-05-10") + assert "2024-05-10" in out and "2024-05-09" in out + assert "2024-05-13" not in out, "bar after end_date leaked into the window" + + +@pytest.mark.unit +def test_unparseable_body_is_never_served_untrimmed(monkeypatch): + """The trim used to swallow the failure and return the whole body, putting + bars after end_date into a backtest. It must raise instead.""" + monkeypatch.setattr(avs, "_make_api_request", + lambda *a, **k: "timestamp,close\nnot-a-date,1\n") + + with pytest.raises(ValueError): + avs.get_stock("IBM", "2024-05-09", "2024-05-10") + + +@pytest.mark.unit +def test_empty_body_still_passes_through(monkeypatch): + monkeypatch.setattr(avs, "_make_api_request", lambda *a, **k: "") + assert avs.get_stock("IBM", "2024-05-09", "2024-05-10") == "" diff --git a/tests/test_fundamentals_lookahead.py b/tests/test_fundamentals_lookahead.py new file mode 100644 index 000000000..696b14872 --- /dev/null +++ b/tests/test_fundamentals_lookahead.py @@ -0,0 +1,130 @@ +"""Historical fundamentals must not leak a live company profile (#1300). + +Vendor "company overview" endpoints (yfinance ``Ticker.info``, Alpha Vantage +``OVERVIEW``) serve only present-day values: market cap, valuation multiples, +the 52-week range and TTM income all move with today's quote, and even name, +sector and industry shift when a company renames or is reclassified. None of it +carries a historical vintage, so emitting it into a run dated in the past puts +post-decision information into the analyst's context, in the same family as the +FRED (#1275), social (#1220) and memory (#1251) leaks. + +Both vendors withhold on one shared rule (``date_window.withhold_live_profile``) +so switching ``fundamental_data`` between them cannot reintroduce the leak. The +statement tools stay point-in-time by filtering on ``curr_date``, and a live run +is unchanged. All API access is mocked. +""" +from __future__ import annotations + +from unittest import mock + +import pytest + +from tradingagents.dataflows import alpha_vantage_fundamentals as av, date_window, y_finance + +_TODAY = "2026-09-07" +_PAST = "2024-05-10" + +# A profile payload mixing stable-looking identity fields with market-dependent ones. +_INFO = { + "longName": "Apple Inc.", + "sector": "Technology", + "industry": "Consumer Electronics", + "marketCap": 3_500_000_000_000, + "trailingPE": 34.2, + "fiftyTwoWeekHigh": 260.1, + "totalRevenue": 391_000_000_000, +} + +# Values that must never reach a historical run. +_LEAKY = ("3500000000000", "34.2", "260.1", "391000000000", + "Apple Inc.", "Technology", "Consumer Electronics") + + +def _yf(curr_date, info=_INFO, today=_TODAY): + with mock.patch.object(date_window, "get_current_date", return_value=today), \ + mock.patch.object(y_finance, "yf_retry", lambda fn: info), \ + mock.patch.object(y_finance.yf, "Ticker"): + return y_finance.get_fundamentals("AAPL", curr_date) + + +def _av(curr_date, today=_TODAY): + """Alpha Vantage path; the API call is mocked so a leak would be visible.""" + with mock.patch.object(date_window, "get_current_date", return_value=today), \ + mock.patch.object(av, "_make_api_request", + return_value="MarketCapitalization: 3500000000000") as req: + return av.get_fundamentals("AAPL", curr_date), req + + +@pytest.mark.unit +class TestYFinanceHistoricalRun: + def test_no_profile_value_survives(self): + out = _yf(_PAST) + for leaked in _LEAKY: + assert leaked not in out, f"leaked live-profile value {leaked!r}" + + def test_states_the_as_of_date_and_explains_itself(self): + # The analyst must be told why the figures are absent, so it does not + # read the gap as a real signal or fabricate around it. + out = _yf(_PAST) + assert f"Point-in-time as of: {_PAST}" in out + assert "withheld" in out + assert _PAST in out and _TODAY in out + + def test_no_wall_clock_retrieval_stamp(self): + # The old header stamped datetime.now(), which is what surfaced the leak. + assert "Data retrieved on:" not in _yf(_PAST) + + def test_the_request_is_not_even_made(self): + # The response would only be discarded; skipping it also avoids burning + # vendor quota on a call whose result cannot be used. + with mock.patch.object(date_window, "get_current_date", return_value=_TODAY), \ + mock.patch.object(y_finance.yf, "Ticker") as tk: + y_finance.get_fundamentals("AAPL", _PAST) + tk.assert_not_called() + + +@pytest.mark.unit +class TestAlphaVantageHistoricalRun: + """The same rule must hold for the other fundamentals vendor, or switching + data_vendors["fundamental_data"] would silently reintroduce the leak.""" + + def test_overview_is_withheld(self): + out, _ = _av(_PAST) + assert "3500000000000" not in out + assert "withheld" in out + assert f"Point-in-time as of: {_PAST}" in out + + def test_the_api_call_is_not_made(self): + _, req = _av(_PAST) + req.assert_not_called() + + def test_live_run_still_calls_the_api(self): + out, req = _av(_TODAY) + req.assert_called_once() + assert "3500000000000" in out + + +@pytest.mark.unit +class TestLiveRunUnchanged: + def test_yfinance_current_date_returns_the_full_profile(self): + out = _yf(_TODAY) + for value in _LEAKY: + assert value in out + assert "Data retrieved on:" in out + assert "withheld" not in out + + def test_yfinance_absent_curr_date_returns_the_full_profile(self): + out = _yf(None) + assert "Market Cap: 3500000000000" in out + assert "withheld" not in out + + +@pytest.mark.unit +class TestNoUsableFieldsStillRaises: + def test_stub_payload_raises_no_market_data(self): + # yfinance returns {"trailingPegRatio": None} for unknown symbols; on a + # live run that must stay a hard "no data", not a bare header. + from tradingagents.dataflows.symbol_utils import NoMarketDataError + + with pytest.raises(NoMarketDataError): + _yf(_TODAY, info={"trailingPegRatio": None}) diff --git a/tests/test_ohlcv_latest_bar.py b/tests/test_ohlcv_latest_bar.py index 7a26b0e00..cf9bca676 100644 --- a/tests/test_ohlcv_latest_bar.py +++ b/tests/test_ohlcv_latest_bar.py @@ -4,8 +4,12 @@ yfinance can return the newest in-range bar with a NaN close (an unsettled or glitched session). The old path parsed dates without normalizing timezone and dropped every NaN-close row before applying the curr_date cutoff, so the latest bar disappeared and the previous trading day looked like the latest. Now dates -are normalized, and a latest in-range bar with no close raises rather than -silently falling back. +are normalized before the cutoff, so the frame ends at the last settled bar +instead of carrying a fabricated close. + +Refusing the whole frame instead (the first attempt at #1201) reported a +tradable symbol as invalid or delisted (#1289), so only a range with no close +anywhere counts as no data and the staleness check judges the rest. """ from __future__ import annotations @@ -92,19 +96,46 @@ def _run_load(monkeypatch, tmp_path, frame, curr_date): def _fail_download(*a, **k): raise AssertionError("should use the seeded cache, not download") monkeypatch.setattr(su.yf, "download", _fail_download) - monkeypatch.setattr(su, "_assert_ohlcv_not_stale", lambda *a, **k: None) return su.load_ohlcv("AAPL", curr_date) @pytest.mark.unit -def test_latest_in_range_nan_close_raises_not_silent_fallback(monkeypatch, tmp_path): - # Newest bar (the curr_date) has no close -> raise, don't return Thursday. +def test_unsettled_latest_bar_is_served_as_the_last_settled_bar(monkeypatch, tmp_path): + # Newest bar (the curr_date) has no close: serve the last settled bar rather + # than reporting the whole symbol as unavailable (#1289). frame = pd.DataFrame({ "Date": ["2026-05-07", "2026-05-08"], "Open": [100.0, 101.0], "High": [101.0, 102.0], "Low": [99.0, 100.0], "Close": [100.5, float("nan")], "Volume": [1_000_000, 1_000_000], }) - with pytest.raises(NoMarketDataError, match="no closing price"): + out = _run_load(monkeypatch, tmp_path, frame, "2026-05-08") + assert out["Date"].iloc[-1] == pd.Timestamp("2026-05-07") + assert out["Close"].iloc[-1] == 100.5 + + +@pytest.mark.unit +def test_no_settled_bar_at_all_is_still_no_data(monkeypatch, tmp_path): + frame = pd.DataFrame({ + "Date": ["2026-05-07", "2026-05-08"], + "Open": [100.0, 101.0], "High": [101.0, 102.0], "Low": [99.0, 100.0], + "Close": [float("nan"), float("nan")], "Volume": [1_000_000, 1_000_000], + }) + with pytest.raises(NoMarketDataError, match="no bar in range has a closing price"): + _run_load(monkeypatch, tmp_path, frame, "2026-05-08") + + +@pytest.mark.unit +def test_serving_the_last_settled_bar_does_not_bypass_the_staleness_check( + monkeypatch, tmp_path +): + # Falling back must not resurrect a long-dead series: once the closeless + # tail is gone, the remaining bar is judged on its age like any other. + frame = pd.DataFrame({ + "Date": ["2026-01-05", "2026-05-08"], + "Open": [100.0, 101.0], "High": [101.0, 102.0], "Low": [99.0, 100.0], + "Close": [100.5, float("nan")], "Volume": [1_000_000, 1_000_000], + }) + with pytest.raises(NoMarketDataError, match="stale"): _run_load(monkeypatch, tmp_path, frame, "2026-05-08") diff --git a/tests/test_ollama_base_url.py b/tests/test_ollama_base_url.py index 5dd0f85c8..c54162da6 100644 --- a/tests/test_ollama_base_url.py +++ b/tests/test_ollama_base_url.py @@ -3,9 +3,20 @@ from __future__ import annotations import importlib +import re import pytest +# Rich colorizes console output and highlights numbers and URLs, which splits +# asserted substrings with escape codes ("port \x1b[1;33m11434"). Whether it +# does so depends on the ambient terminal, so strip the codes to keep these +# assertions independent of where the suite runs. +_ANSI = re.compile(r"\x1b\[[0-9;]*m") + + +def _console_out(capsys) -> str: + return _ANSI.sub("", capsys.readouterr().out) + @pytest.fixture(scope="module", autouse=True) def _resync_reloaded_modules(): @@ -122,7 +133,7 @@ def test_confirm_endpoint_shows_default(monkeypatch, capsys): import cli.utils as cli_utils importlib.reload(cli_utils) cli_utils.confirm_ollama_endpoint("http://localhost:11434/v1") - out = capsys.readouterr().out + out = _console_out(capsys) assert "http://localhost:11434/v1" in out assert "OLLAMA_BASE_URL" not in out # not from env assert "Note" not in out # no warnings for the canonical default @@ -133,7 +144,7 @@ def test_confirm_endpoint_marks_env_origin(monkeypatch, capsys): import cli.utils as cli_utils importlib.reload(cli_utils) cli_utils.confirm_ollama_endpoint("http://remote-host:11434/v1") - out = capsys.readouterr().out + out = _console_out(capsys) assert "http://remote-host:11434/v1" in out assert "OLLAMA_BASE_URL" in out @@ -144,7 +155,7 @@ def test_confirm_endpoint_warns_on_missing_scheme(monkeypatch, capsys): import cli.utils as cli_utils importlib.reload(cli_utils) cli_utils.confirm_ollama_endpoint("0.0.0.128") - out = capsys.readouterr().out + out = _console_out(capsys) assert "missing a scheme" in out assert "http://:11434/v1" in out @@ -155,7 +166,7 @@ def test_confirm_endpoint_warns_on_non_default_port_remote(monkeypatch, capsys): import cli.utils as cli_utils importlib.reload(cli_utils) cli_utils.confirm_ollama_endpoint("http://remote-host/v1") - out = capsys.readouterr().out + out = _console_out(capsys) assert "port 11434" in out @@ -165,7 +176,7 @@ def test_confirm_endpoint_quiet_on_local_no_port(monkeypatch, capsys): import cli.utils as cli_utils importlib.reload(cli_utils) cli_utils.confirm_ollama_endpoint("http://localhost/v1") - out = capsys.readouterr().out + out = _console_out(capsys) assert "Note" not in out # localhost is fine without explicit port diff --git a/tests/test_reddit_fallback.py b/tests/test_reddit_fallback.py index ec9a89fdd..a2bc2cc25 100644 --- a/tests/test_reddit_fallback.py +++ b/tests/test_reddit_fallback.py @@ -86,9 +86,9 @@ class TestRssParsing: assert posts[0]["created_utc"] > 0 assert "datacenter unit" in posts[0]["selftext"] - def test_malformed_xml_fails_open(self): + def test_malformed_xml_reports_unavailable(self): with patch.object(reddit, "urlopen", return_value=_resp(lambda: b"<>")): - assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) == [] + assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) is None @pytest.mark.unit @@ -139,7 +139,7 @@ class TestRss429Backoff: patch.object(reddit.time, "sleep"): posts = reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) assert op.call_count == 2 # one retry, then gives up cleanly - assert posts == [] + assert posts is None def test_retry_after_header_is_honoured(self): err = HTTPError("url", 429, "Too Many Requests", {"Retry-After": "12"}, None) @@ -166,7 +166,7 @@ class TestRss429Backoff: reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) slept.assert_called_once() (wait,), _ = slept.call_args - assert 4.0 <= wait <= 6.0 # 5s +/-20% jitter + assert 48.0 <= wait <= 72.0 # 60s +/-20% jitter @pytest.mark.unit @@ -174,9 +174,9 @@ class TestChunkedTransferErrorsHandled: """IncompleteRead/RemoteDisconnected come from http.client and are NOT OSErrors, so they were previously uncaught and crashed the pipeline (#1024).""" - def test_rss_incomplete_read_degrades_to_empty(self): + def test_rss_incomplete_read_reports_unavailable(self): with patch.object(reddit, "urlopen", return_value=_raise(http.client.IncompleteRead(b""))): - assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) == [] + assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) is None def test_json_incomplete_read_falls_back_to_rss(self): with patch.object(reddit, "urlopen", return_value=_raise(http.client.IncompleteRead(b""))), \ @@ -190,7 +190,7 @@ class TestChunkedTransferErrorsHandled: big = _resp(lambda: b"x" * 100) with patch.object(reddit, "_MAX_FEED_BYTES", 10), \ patch.object(reddit, "urlopen", return_value=big): - assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) == [] + assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) is None @pytest.mark.unit @@ -228,7 +228,7 @@ class TestCryptoSearchTerm: def _captured_ticker(self, ticker): seen = {} - def fake_fetch(t, sub, limit, timeout): + def fake_fetch(t, sub, limit, timeout, **kwargs): seen["ticker"] = t return [] @@ -241,3 +241,64 @@ class TestCryptoSearchTerm: def test_equity_passes_through(self): assert self._captured_ticker("NVDA") == "NVDA" + + +@pytest.mark.unit +class TestFailedFetchIsNotSilence: + """A throttled fetch must not be rendered as "no posts found" (#1295). + + Returning [] for both a failed request and a genuinely empty search made the + sentiment analyst read rate limiting as real silence ("r/stocks and + r/investing are silent"), which is a signal that was never observed. + """ + + _POST = { + "title": "NVDA pops", "score": None, "num_comments": None, + "created_utc": reddit._iso_to_timestamp("2026-05-20T14:30:00Z"), + "selftext": "", "source": "rss", + } + + def _run(self, results): + """Drive fetch_reddit_posts with a per-subreddit result sequence.""" + subs = tuple(f"s{i}" for i in range(len(results))) + with patch.object(reddit, "_fetch_subreddit", side_effect=list(results)): + return reddit.fetch_reddit_posts( + "NVDA", subreddits=subs, inter_request_delay=0 + ) + + def test_failed_subreddit_is_marked_unavailable_not_empty(self): + out = self._run([None, [self._POST]]) + assert "unavailable" in out + assert "no posts found" not in out.split("unavailable")[0] + + def test_all_sources_failing_does_not_claim_no_posts(self): + out = self._run([None, None]) + assert "Reddit unavailable" in out + assert "no Reddit posts found" not in out + + def test_mixed_failure_and_empty_only_claims_silence_for_searched_subs(self): + # s0 failed, s1 genuinely returned nothing: the "no posts" claim must + # cover only s1, with s0 reported separately as unavailable. + out = self._run([None, []]) + assert "r/s1" in out.split("unavailable (fetch failed)")[0] + assert "unavailable (fetch failed): r/s0" in out + + def test_genuine_empty_still_reports_no_posts(self): + out = self._run([[], []]) + assert "no Reddit posts found" in out + assert "unavailable" not in out + + def test_retry_is_not_spent_again_after_a_failure(self): + # The 60s back-off must be paid at most once per run, so subsequent + # subreddits are fetched with retry disabled rather than stalling. + seen = [] + + def record(t, sub, limit, timeout, _retry=True): + seen.append(_retry) + return None + + with patch.object(reddit, "_fetch_subreddit", side_effect=record): + reddit.fetch_reddit_posts( + "NVDA", subreddits=("a", "b", "c"), inter_request_delay=0 + ) + assert seen == [True, False, False] diff --git a/tests/test_structured_agents.py b/tests/test_structured_agents.py index 50566cdd4..ba3aa6f03 100644 --- a/tests/test_structured_agents.py +++ b/tests/test_structured_agents.py @@ -97,6 +97,43 @@ class TestNullishFloatCoercion: ) assert d.price_target is None + def test_percentage_answer_to_a_price_field_becomes_none(self): + # The Trader is asked for concrete levels and may answer a price field + # with a distance ("15%"), which failed the whole proposal (#1288). + # A percentage cannot be salvaged: 15% must not become a $15 stop. + for pct in ("15%", " 7.5% ", "-10%"): + p = TraderProposal( + action=TraderAction.BUY, + reasoning="x", + entry_price=pct, + stop_loss=pct, + ) + assert p.entry_price is None + assert p.stop_loss is None + + def test_human_formatted_price_is_reduced_to_its_number(self): + p = TraderProposal( + action=TraderAction.BUY, + reasoning="x", + entry_price="$1,234.50", + stop_loss="1,180", + ) + assert p.entry_price == 1234.50 + assert p.stop_loss == 1180.0 + + def test_one_bad_field_no_longer_fails_the_whole_proposal(self): + # Previously a single '15%' raised, forcing a free-text retry that lost + # the action and reasoning; now the rest of the proposal survives. + p = TraderProposal( + action=TraderAction.SELL, + reasoning="downgrade on margin compression", + entry_price="612.40", + stop_loss="15%", + ) + assert p.action is TraderAction.SELL + assert p.entry_price == 612.40 + assert p.stop_loss is None + @pytest.mark.unit class TestRenderResearchPlan: @@ -393,6 +430,21 @@ def _structured_sentiment_llm(captured: dict, report: SentimentReport | None = N @pytest.mark.unit class TestSentimentAnalystAgent: + @pytest.fixture(autouse=True) + def _stub_prefetched_sources(self, monkeypatch): + """Stub the sources the analyst pre-fetches before prompting. + + create_sentiment_analyst fetches news, StockTwits and Reddit itself, so + without this these tests hit the live network. A real Reddit 429 then + backs the fetcher off for a minute per subreddit, which is what turned + this file into a multi-minute hang. + """ + from tradingagents.agents.analysts import sentiment_analyst as sentiment + + monkeypatch.setattr(sentiment, "fetch_stocktwits_messages", lambda *a, **k: "st") + monkeypatch.setattr(sentiment, "fetch_reddit_posts", lambda *a, **k: "rd") + monkeypatch.setattr(sentiment.get_news, "func", lambda *a, **k: "news", raising=False) + def test_structured_path_produces_rendered_markdown(self): captured = {} report = SentimentReport( diff --git a/tradingagents/agents/schemas.py b/tradingagents/agents/schemas.py index 961cde79a..e933017d7 100644 --- a/tradingagents/agents/schemas.py +++ b/tradingagents/agents/schemas.py @@ -31,9 +31,23 @@ _NULLISH_FLOAT = {"", "none", "n/a", "na", "null", "nil", "-", "tbd", "unknown"} def _coerce_optional_float(value): - if isinstance(value, str) and value.strip().lower() in _NULLISH_FLOAT: + """Normalise an LLM-written optional numeric field before validation. + + Three shapes show up in practice: a placeholder string ("None", "N/A") in + place of an omitted value (#1058); a percentage where a price was asked for + ("15%", #1288); and a human-formatted price ("$1,234.50"). A percentage + cannot be salvaged into an absolute level -- reading "15%" as 15 would put a + stop at $15 on a $600 stock -- so it is dropped like a placeholder, leaving + one bad field to null out instead of failing the whole proposal. A formatted + price is reduced to its number. Anything else passes through to pydantic. + """ + if not isinstance(value, str): + return value + text = value.strip() + if text.lower() in _NULLISH_FLOAT or text.endswith("%"): return None - return value + cleaned = text.replace(",", "").lstrip("$€£¥").strip() + return cleaned or None # --------------------------------------------------------------------------- @@ -140,11 +154,19 @@ class TraderProposal(BaseModel): ) entry_price: float | None = Field( default=None, - description="Optional entry price target in the instrument's quote currency.", + description=( + "Optional entry price target as an absolute number in the instrument's " + "quote currency (e.g. 189.5), never a percentage or a range. Omit it " + "if you cannot state a specific level." + ), ) stop_loss: float | None = Field( default=None, - description="Optional stop-loss price in the instrument's quote currency.", + description=( + "Optional stop-loss as an absolute price in the instrument's quote " + "currency (e.g. 172.0), never a percentage. Convert a percentage " + "distance to the price level it implies, or omit it." + ), ) position_sizing: str | None = Field( default=None, diff --git a/tradingagents/agents/trader/trader.py b/tradingagents/agents/trader/trader.py index 402023bf1..dbf1f942e 100644 --- a/tradingagents/agents/trader/trader.py +++ b/tradingagents/agents/trader/trader.py @@ -50,6 +50,13 @@ def create_trader(llm): "You are a trading agent analyzing market data to make investment decisions. " "Based on your analysis, provide a specific recommendation to buy, sell, or hold. " + grounding + # Entry/stop are numeric price fields. Asking for concrete + # levels invites a percentage ("15%"), which is not a price + # and fails the structured parse (#1288). + + "State entry price and stop-loss as absolute price levels in the " + "instrument's quote currency (for example 189.5), never a percentage " + "or a range; convert a percentage distance to the price level it " + "implies, or omit the field if you cannot state a number. " + NO_EXTERNAL_TOOLS + get_language_instruction() ), diff --git a/tradingagents/dataflows/alpha_vantage_common.py b/tradingagents/dataflows/alpha_vantage_common.py index 237ff54ce..ecbe4dcec 100644 --- a/tradingagents/dataflows/alpha_vantage_common.py +++ b/tradingagents/dataflows/alpha_vantage_common.py @@ -128,24 +128,18 @@ def _filter_csv_by_date_range(csv_data: str, start_date: str, end_date: str) -> if not csv_data or csv_data.strip() == "": return csv_data - try: - # Parse CSV data - df = pd.read_csv(StringIO(csv_data)) + # Deliberately unguarded: TIME_SERIES_DAILY_ADJUSTED returns the full series + # up to today, so this trim is the only thing keeping bars after end_date out + # of a historical run. Swallowing a parse failure would serve the untrimmed + # body, and with it future prices. + df = pd.read_csv(StringIO(csv_data)) - # Assume the first column is the date column (timestamp) - date_col = df.columns[0] - df[date_col] = pd.to_datetime(df[date_col]) + # Assume the first column is the date column (timestamp) + date_col = df.columns[0] + df[date_col] = pd.to_datetime(df[date_col]) - # Filter by date range - start_dt = pd.to_datetime(start_date) - end_dt = pd.to_datetime(end_date) + start_dt = pd.to_datetime(start_date) + end_dt = pd.to_datetime(end_date) + filtered_df = df[(df[date_col] >= start_dt) & (df[date_col] <= end_dt)] - filtered_df = df[(df[date_col] >= start_dt) & (df[date_col] <= end_dt)] - - # Convert back to CSV string - return filtered_df.to_csv(index=False) - - except Exception as e: - # If filtering fails, return original data with a warning - print(f"Warning: Failed to filter CSV data by date range: {e}") - return csv_data + return filtered_df.to_csv(index=False) diff --git a/tradingagents/dataflows/alpha_vantage_fundamentals.py b/tradingagents/dataflows/alpha_vantage_fundamentals.py index 90c89f204..56525b3ff 100644 --- a/tradingagents/dataflows/alpha_vantage_fundamentals.py +++ b/tradingagents/dataflows/alpha_vantage_fundamentals.py @@ -1,6 +1,7 @@ import json from .alpha_vantage_common import _make_api_request +from .date_window import withhold_live_profile def _filter_reports_by_date(result, curr_date: str): @@ -31,13 +32,22 @@ def get_fundamentals(ticker: str, curr_date: str = None) -> str: """ Retrieve comprehensive fundamental data for a given ticker symbol using Alpha Vantage. + OVERVIEW serves only present-day values and carries no historical vintage, so + a past ``curr_date`` withholds it rather than leaking post-decision figures + into a backtest (#1300); the statement endpoints below stay point-in-time via + ``_filter_reports_by_date``. + Args: ticker (str): Ticker symbol of the company - curr_date (str): Current date you are trading at, yyyy-mm-dd (not used for Alpha Vantage) + curr_date (str): Analysis date, yyyy-mm-dd Returns: str: Company overview data including financial ratios and key metrics """ + withheld = withhold_live_profile(curr_date, ticker) + if withheld: + return withheld + params = { "symbol": ticker, } diff --git a/tradingagents/dataflows/alpha_vantage_indicator.py b/tradingagents/dataflows/alpha_vantage_indicator.py index dcfd17ab6..5dd61f387 100644 --- a/tradingagents/dataflows/alpha_vantage_indicator.py +++ b/tradingagents/dataflows/alpha_vantage_indicator.py @@ -1,5 +1,9 @@ +import logging + from .alpha_vantage_common import AlphaVantageNotConfiguredError, _make_api_request +logger = logging.getLogger(__name__) + def get_indicator( symbol: str, @@ -211,5 +215,5 @@ def get_indicator( # successful-looking error string. raise except Exception as e: - print(f"Error getting Alpha Vantage indicator data for {indicator}: {e}") + logger.warning("Alpha Vantage indicator %s failed: %s", indicator, e) return f"Error retrieving {indicator} data: {str(e)}" diff --git a/tradingagents/dataflows/date_window.py b/tradingagents/dataflows/date_window.py index 8b49dca26..3a7dc0206 100644 --- a/tradingagents/dataflows/date_window.py +++ b/tradingagents/dataflows/date_window.py @@ -13,6 +13,8 @@ from __future__ import annotations from datetime import datetime, timedelta, timezone +from .utils import get_current_date + def to_utc(dt: datetime) -> datetime: """Normalize a datetime to UTC-aware; a naive value is assumed to be UTC.""" @@ -28,3 +30,32 @@ def in_window(pub_dt: datetime | None, start_dt: datetime, end_dt: datetime) -> if pub_dt is not None: return to_utc(start_dt) <= to_utc(pub_dt) < end + timedelta(days=1) return end >= datetime.now(timezone.utc) - timedelta(days=1) + + +def withhold_live_profile(curr_date: str | None, label: str) -> str | None: + """Notice to serve instead of a live-only company profile, or None to serve it. + + Vendor "company overview" endpoints (yfinance ``Ticker.info``, Alpha Vantage + ``OVERVIEW``) carry no historical vintage — not even name, sector and + industry, which move when a company renames or is reclassified — so serving + one into a run dated in the past leaks post-decision information (#1300). + Every fundamentals vendor withholds on this rule, so switching between them + cannot reintroduce the leak. + """ + if not curr_date: + return None + today = get_current_date() + if curr_date >= today: + return None + return ( + f"# Company Fundamentals for {label}\n" + f"# Point-in-time as of: {curr_date}\n\n" + f"Profile fundamentals are withheld for this date. This vendor serves " + f"only present-day values ({today}) with no historical vintage: market " + f"cap, valuation multiples, the 52-week range and TTM income move with " + f"today's quote, and even the name, sector and industry reflect today " + f"rather than {curr_date} (companies rename and get reclassified). " + f"Serving them would put post-decision information into a {curr_date} " + f"analysis. Point-in-time fundamentals for {curr_date} are available " + f"from the balance sheet, income statement, and cash flow tools." + ) diff --git a/tradingagents/dataflows/reddit.py b/tradingagents/dataflows/reddit.py index f8f0e1755..502c84d68 100644 --- a/tradingagents/dataflows/reddit.py +++ b/tradingagents/dataflows/reddit.py @@ -10,6 +10,10 @@ off once (honouring ``Retry-After``). RSS lacks score / comment counts, so those posts are marked and the formatter omits the metrics rather than printing fake zeros. +A fetch that fails is reported as ````, never as "no posts found": +the two are different claims, and passing a rate-limited fetch off as silence +hands the sentiment analyst a signal that was never observed (#1295). + No API key required. Returns formatted plaintext blocks ready for prompt injection and degrades gracefully — returns a placeholder string rather than raising, so callers never special-case missing data. @@ -102,9 +106,12 @@ def _strip_html(content: str) -> str: return " ".join(html.unescape(text).split()) -# Headerless-429 backoff when Reddit gives no Retry-After. Jittered so several -# analyses sharing an IP don't retry in lockstep and re-collide on the limit. -_RETRY_FALLBACK_SECONDS = 5.0 +# Headerless-429 backoff when Reddit gives no Retry-After. Measured against +# /r/{sub}/search.rss, a retry still 429s at 8s, 10s and 30s of spacing and +# succeeds at 60s, so a shorter wait spends the one retry on a request that +# cannot succeed (#1295). Jittered so several analyses sharing an IP don't +# retry in lockstep and re-collide on the limit. +_RETRY_FALLBACK_SECONDS = 60.0 def _jitter(seconds: float, frac: float = 0.2) -> float: @@ -114,14 +121,18 @@ def _jitter(seconds: float, frac: float = 0.2) -> float: def _retry_after_seconds(exc: HTTPError) -> float | None: - """Seconds to wait from a 429's ``Retry-After`` header, capped at 30s. + """Seconds to wait from a 429's ``Retry-After`` header, capped at 60s. + + The cap matches ``_RETRY_FALLBACK_SECONDS``: honouring less than we would + wait on our own would spend the one retry on a request we already know is + too early. Returns ``None`` only when the header is absent or unparseable; a valid ``Retry-After: 0`` returns ``0.0`` (retry at once), not ``None``. """ try: val = exc.headers.get("Retry-After") if getattr(exc, "headers", None) else None - return min(float(val), 30.0) if val is not None else None + return min(float(val), 60.0) if val is not None else None except (ValueError, TypeError, AttributeError): return None @@ -149,13 +160,18 @@ def _fetch_subreddit_rss( limit: int, timeout: float, _retry: bool = True, -) -> list[dict]: +) -> list[dict] | None: """Default path: parse the public Atom search feed for a subreddit. Carries no score / comment counts, so those fields are left None and the post is tagged ``source="rss"`` for honest display. On a 429 (Reddit's per-IP rate limit) we back off once — honouring ``Retry-After`` when present — before giving up, so a transient burst doesn't blank the feed. + + Returns ``[]`` when the search ran and matched nothing, and ``None`` when + the fetch itself failed. The caller must keep these apart: rendering a + failed fetch as "no posts found" hands the sentiment analyst an absence of + discussion that was never observed (#1295). """ url = _RSS.format(sub=sub, qs=_search_qs(ticker, limit)) req = Request(url, headers={"User-Agent": _UA}) @@ -175,12 +191,12 @@ def _fetch_subreddit_rss( time.sleep(wait) return _fetch_subreddit_rss(ticker, sub, limit, timeout, _retry=False) logger.warning("Reddit RSS fetch failed for r/%s · %s: %s", sub, ticker, exc) - return [] + return None except (OSError, http.client.HTTPException, ET.ParseError) as exc: # OSError covers URLError/TimeoutError/connection resets; HTTPException # covers chunked-transfer errors (IncompleteRead/BadStatusLine, #1024). logger.warning("Reddit RSS fetch failed for r/%s · %s: %s", sub, ticker, exc) - return [] + return None posts = [] for entry in root.findall("atom:entry", _ATOM_NS)[:limit]: @@ -234,14 +250,15 @@ def _fetch_subreddit( sub: str, limit: int, timeout: float, -) -> list[dict]: - """Fetch one subreddit, RSS-first. + _retry: bool = True, +) -> list[dict] | None: + """Fetch one subreddit, RSS-first. ``None`` means the fetch failed. The JSON search endpoint is reliably WAF-blocked (403) for public clients, so we go straight to the RSS feed — which serves our identified User-Agent reliably — halving our request volume against Reddit's per-IP rate limit. """ - return _fetch_subreddit_rss(ticker, sub, limit, timeout) + return _fetch_subreddit_rss(ticker, sub, limit, timeout, _retry=_retry) def fetch_reddit_posts( @@ -267,13 +284,26 @@ def fetch_reddit_posts( # Crypto reaches us as a Yahoo pair (BTC-USD); search Reddit for the base # ("BTC") so the query actually matches discussion instead of near-nothing. ticker = crypto_base(ticker) or ticker + subreddits = list(subreddits) blocks = [] total_posts = 0 + unavailable = [] + allow_retry = True for i, sub in enumerate(subreddits): if i > 0 and inter_request_delay: time.sleep(_jitter(inter_request_delay)) - posts = _within_window(_fetch_subreddit(ticker, sub, limit_per_sub, timeout), - start_date, end_date) + fetched = _fetch_subreddit(ticker, sub, limit_per_sub, timeout, _retry=allow_retry) + if fetched is None: + # A failed fetch is not an absence of discussion, so it must not be + # rendered as "no posts found" (#1295). One failure also means the + # per-IP budget is likely gone, so skip the (now 60s) back-off on + # the remaining subreddits rather than stalling the run on retries + # that cannot succeed; #1286 tracks coordinating this properly. + allow_retry = False + unavailable.append(sub) + blocks.append(f"r/{sub}: ") + continue + posts = _within_window(fetched, start_date, end_date) total_posts += len(posts) if not posts: blocks.append(f"r/{sub}: ") @@ -306,8 +336,23 @@ def fetch_reddit_posts( blocks.append("\n".join(lines)) if total_posts == 0: - return ( + searched = [s for s in subreddits if s not in unavailable] + if not searched: + # Every source failed: claiming "no posts" here would assert a + # silence we never observed. + return ( + f"" + ) + summary = ( f"" + f"{', '.join(f'r/{s}' for s in searched)} in the past 7 days>" ) + if unavailable: + summary += ( + f"\n" + ) + return summary return "\n\n".join(blocks) diff --git a/tradingagents/dataflows/stockstats_utils.py b/tradingagents/dataflows/stockstats_utils.py index 94ba07ea8..1bdd02ab8 100644 --- a/tradingagents/dataflows/stockstats_utils.py +++ b/tradingagents/dataflows/stockstats_utils.py @@ -250,13 +250,20 @@ def load_ohlcv(symbol: str, curr_date: str) -> pd.DataFrame: # Filter to curr_date to prevent look-ahead bias in backtesting. data = data[data["Date"] <= curr_date_dt] - # Guard the latest in-range bar before dropping incomplete rows: a newest bar - # with no close is "not settled yet", not "does not exist". Silently dropping - # it would make the previous trading day look like the latest (#1201); raise - # instead so the router surfaces it rather than fabricating a fallback. + # A closeless newest bar is an unsettled session, not a symbol without data. + # _fill_price_gaps below drops it, here and mid-series alike, so the frame + # ends at the last settled bar; only a range with no close anywhere is no + # data (#1201, #1289). if not data.empty and pd.isna(data["Close"].iloc[-1]): - raise NoMarketDataError( - symbol, canonical, "latest in-range OHLCV bar has no closing price" + settled = data["Close"].notna().to_numpy().nonzero()[0] + if settled.size == 0: + raise NoMarketDataError( + symbol, canonical, "no bar in range has a closing price" + ) + logger.warning( + "%s: %d trailing bar(s) through %s have no closing price; using %s " + "as the latest close.", canonical, len(data) - settled[-1] - 1, + data["Date"].iloc[-1].date(), data["Date"].iloc[settled[-1]].date(), ) data = _fill_price_gaps(data) diff --git a/tradingagents/dataflows/utils.py b/tradingagents/dataflows/utils.py index b5d48d079..71806b47f 100644 --- a/tradingagents/dataflows/utils.py +++ b/tradingagents/dataflows/utils.py @@ -1,10 +1,5 @@ import re -from datetime import date, datetime, timedelta -from typing import Annotated - -import pandas as pd - -SavePathType = Annotated[str, "File path to save data. If None, data is not saved."] +from datetime import date # Tickers can contain letters, digits, dot, dash, underscore, caret # (index symbols like ^GSPC), equals (futures like GC=F), and plus @@ -42,34 +37,5 @@ def safe_ticker_component(value: str, *, max_len: int = 32) -> str: return value -def save_output(data: pd.DataFrame, tag: str, save_path: SavePathType = None) -> None: - if save_path: - data.to_csv(save_path, encoding="utf-8") - print(f"{tag} saved to {save_path}") - - def get_current_date(): return date.today().strftime("%Y-%m-%d") - - -def decorate_all_methods(decorator): - def class_decorator(cls): - for attr_name, attr_value in cls.__dict__.items(): - if callable(attr_value): - setattr(cls, attr_name, decorator(attr_value)) - return cls - - return class_decorator - - -def get_next_weekday(date): - - if not isinstance(date, datetime): - date = datetime.strptime(date, "%Y-%m-%d") - - if date.weekday() >= 5: - days_to_add = 7 - date.weekday() - next_weekday = date + timedelta(days=days_to_add) - return next_weekday - else: - return date diff --git a/tradingagents/dataflows/y_finance.py b/tradingagents/dataflows/y_finance.py index fdb49d52d..9e67ac591 100644 --- a/tradingagents/dataflows/y_finance.py +++ b/tradingagents/dataflows/y_finance.py @@ -1,3 +1,4 @@ +import logging from datetime import datetime from typing import Annotated @@ -5,6 +6,7 @@ import pandas as pd import yfinance as yf from dateutil.relativedelta import relativedelta +from .date_window import withhold_live_profile from .stockstats_utils import ( StockstatsUtils, _assert_ohlcv_not_stale, @@ -14,6 +16,8 @@ from .stockstats_utils import ( ) from .symbol_utils import NoMarketDataError, normalize_symbol +logger = logging.getLogger(__name__) + def get_YFin_data_online( symbol: Annotated[str, "ticker symbol of the company"], @@ -188,7 +192,7 @@ def get_stock_stats_indicators_window( except NoMarketDataError: raise # Unknown/delisted symbol — let the router emit the sentinel except Exception as e: - print(f"Error getting bulk stockstats data: {e}") + logger.warning("Bulk stockstats fetch failed, falling back per-day: %s", e) # Fallback to original implementation if bulk method fails ind_string = "" curr_date_dt = datetime.strptime(curr_date, "%Y-%m-%d") @@ -263,9 +267,7 @@ def get_stockstats_indicator( except NoMarketDataError: raise # Unknown/delisted symbol — let the router emit the sentinel except Exception as e: - print( - f"Error getting stockstats indicator data for indicator {indicator} on {curr_date}: {e}" - ) + logger.warning("Stockstats indicator %s failed on %s: %s", indicator, curr_date, e) return "" return str(indicator_value) @@ -273,10 +275,22 @@ def get_stockstats_indicator( def get_fundamentals( ticker: Annotated[str, "ticker symbol of the company"], - curr_date: Annotated[str, "current date (not used for yfinance)"] = None + curr_date: Annotated[str, "analysis date in YYYY-MM-DD format"] = None ): - """Get company fundamentals overview from yfinance.""" + """Get company fundamentals overview from yfinance. + + ``Ticker.info`` is a present-day snapshot with no historical vintage, so a + past ``curr_date`` withholds it through the shared point-in-time guard + (``date_window.withhold_live_profile``, #1300). + """ canonical = normalize_symbol(ticker) + + # Guard before the request: the response would only be discarded, and the + # answer does not depend on it. + withheld = withhold_live_profile(curr_date, canonical) + if withheld: + return withheld + try: ticker_obj = yf.Ticker(canonical) info = yf_retry(lambda: ticker_obj.info) @@ -315,10 +329,7 @@ def get_fundamentals( ("Free Cash Flow", info.get("freeCashflow")), ] - lines = [] - for label, value in fields: - if value is not None: - lines.append(f"{label}: {value}") + lines = [f"{label}: {v}" for label, v in fields if v is not None] # yfinance returns a stub dict (e.g. {"trailingPegRatio": None}) for # unknown symbols, so `info` is truthy but every field is empty. Treat diff --git a/tradingagents/llm_clients/model_catalog.py b/tradingagents/llm_clients/model_catalog.py index b058fc302..1d9ee802a 100644 --- a/tradingagents/llm_clients/model_catalog.py +++ b/tradingagents/llm_clients/model_catalog.py @@ -61,6 +61,23 @@ _QWEN_MODELS: dict[str, list[ModelOption]] = { # Shared model list for MiniMax's global and CN endpoints (same IDs). # Full official lineup per platform.minimax.io/docs/api-reference/text-openai-api. # M3 carries a 1M-token context window; the M2.x line is 204,800 tokens. +# Kimi (Moonshot). Source: platform.kimi.ai/docs/models. "Custom model ID" stays +# available for models newer than this list. The k2.7-code variants are omitted: +# they are coding specialists, not analysis models. +_KIMI_MODELS: dict[str, list[ModelOption]] = { + "quick": [ + ("Kimi K2.6 - 256K ctx, thinking modes, agent tasks", "kimi-k2.6"), + ("Kimi K3 - Flagship, 1M ctx", "kimi-k3"), + ("Custom model ID", "custom"), + ], + "deep": [ + ("Kimi K3 - Flagship, 1M ctx, native visual understanding", "kimi-k3"), + ("Kimi K2.6 - 256K ctx, thinking modes, agent tasks", "kimi-k2.6"), + ("Custom model ID", "custom"), + ], +} + + _MINIMAX_MODELS: dict[str, list[ModelOption]] = { "quick": [ ("MiniMax-M3 - Latest, 1M ctx, native multimodal", "MiniMax-M3"), @@ -151,6 +168,7 @@ MODEL_OPTIONS: ProviderModeOptions = { "glm-cn": _GLM_MODELS, # MiniMax: same model IDs across global (.io) and China (.com) regions, # so the two provider keys share one model list. + "kimi": _KIMI_MODELS, "minimax": _MINIMAX_MODELS, "minimax-cn": _MINIMAX_MODELS, # OpenRouter: fetched dynamically. Azure: any deployed model name. @@ -183,7 +201,6 @@ MODEL_OPTIONS: ProviderModeOptions = { # stale. The endpoint + key are wired by the provider; the user picks the # model their account has access to. "mistral": _CUSTOM_ONLY, - "kimi": _CUSTOM_ONLY, "groq": _CUSTOM_ONLY, "nvidia": _CUSTOM_ONLY, # Bedrock model IDs / cross-region inference profile IDs are user-specified.