From 8db41f6bca3c4ab2c9b89df62a55cf776e900e8c Mon Sep 17 00:00:00 2001 From: Yijia-Xiao Date: Sun, 30 Aug 2026 07:03:06 +0000 Subject: [PATCH] fix(memory): gate past-context lessons to point-in-time in backtests - get_past_context returned every resolved lesson regardless of the run date, so a historical run could learn from an outcome that had not happened yet - record each resolved entry's resolution date (the last price bar used) and filter get_past_context(as_of=trade_date) on it for a historical run; a current-date run passes None so live behavior and pre-migration entries (no stored resolution date, conservatively excluded from backtests) are unaffected #1251 --- tests/test_memory_log.py | 34 +++++---- tests/test_memory_pointintime.py | 95 ++++++++++++++++++++++++ tests/test_symbol_normalization_paths.py | 7 +- tradingagents/agents/utils/memory.py | 55 +++++++++++--- tradingagents/graph/trading_graph.py | 33 ++++++-- 5 files changed, 192 insertions(+), 32 deletions(-) create mode 100644 tests/test_memory_pointintime.py diff --git a/tests/test_memory_log.py b/tests/test_memory_log.py index 69fdb8047..34bf0c4d1 100644 --- a/tests/test_memory_log.py +++ b/tests/test_memory_log.py @@ -54,9 +54,14 @@ def _resolve_entry(log, ticker, date, decision, reflection="Good call."): log.update_with_outcome(ticker, date, 0.05, 0.02, 5, reflection) -def _price_df(prices): - """Minimal DataFrame matching yfinance .history() output shape.""" - return pd.DataFrame({"Close": prices}) +def _price_df(prices, start="2026-01-05"): + """Minimal DataFrame matching yfinance .history() output shape. + + Uses a DatetimeIndex like real yfinance output, so resolution-date + extraction (stock.index[actual_days]) works (#1251). + """ + idx = pd.date_range(start=start, periods=len(prices), freq="D") + return pd.DataFrame({"Close": prices}, index=idx) def _make_pm_state(past_context=""): @@ -496,30 +501,32 @@ class TestDeferredReflection: m.history.return_value = _price_df(spy_prices if sym == "SPY" else stock_prices) return m mock_ticker_cls.side_effect = _make_ticker - raw, alpha, days = TradingAgentsGraph._fetch_returns(mock_graph, "NVDA", "2026-01-05") + raw, alpha, days, resolved = TradingAgentsGraph._fetch_returns(mock_graph, "NVDA", "2026-01-05") assert raw is not None and alpha is not None and days is not None assert isinstance(raw, float) and isinstance(alpha, float) and isinstance(days, int) assert days == 5 + # resolution date = the bar `days` sessions after the trade date (#1251) + assert resolved == "2026-01-10" def test_fetch_returns_too_recent(self): - """Only 1 data point available → returns (None, None, None), no crash.""" + """Only 1 data point available → returns all-None, no crash.""" mock_graph = MagicMock(spec=TradingAgentsGraph) with patch("yfinance.Ticker") as mock_ticker_cls: m = MagicMock() m.history.return_value = _price_df([100.0]) mock_ticker_cls.return_value = m - raw, alpha, days = TradingAgentsGraph._fetch_returns(mock_graph, "NVDA", "2026-04-19") - assert raw is None and alpha is None and days is None + raw, alpha, days, resolved = TradingAgentsGraph._fetch_returns(mock_graph, "NVDA", "2026-04-19") + assert (raw, alpha, days, resolved) == (None, None, None, None) def test_fetch_returns_delisted(self): - """Empty DataFrame → returns (None, None, None), no crash.""" + """Empty DataFrame → returns all-None, no crash.""" mock_graph = MagicMock(spec=TradingAgentsGraph) with patch("yfinance.Ticker") as mock_ticker_cls: m = MagicMock() m.history.return_value = pd.DataFrame({"Close": []}) mock_ticker_cls.return_value = m - raw, alpha, days = TradingAgentsGraph._fetch_returns(mock_graph, "XXXXXFAKE", "2026-01-10") - assert raw is None and alpha is None and days is None + raw, alpha, days, resolved = TradingAgentsGraph._fetch_returns(mock_graph, "XXXXXFAKE", "2026-01-10") + assert (raw, alpha, days, resolved) == (None, None, None, None) def test_fetch_returns_spy_shorter_than_stock(self): """SPY having fewer rows than the stock must not raise IndexError.""" @@ -532,9 +539,10 @@ class TestDeferredReflection: m.history.return_value = _price_df(spy_prices if sym == "SPY" else stock_prices) return m mock_ticker_cls.side_effect = _make_ticker - raw, alpha, days = TradingAgentsGraph._fetch_returns(mock_graph, "NVDA", "2026-01-05") + raw, alpha, days, resolved = TradingAgentsGraph._fetch_returns(mock_graph, "NVDA", "2026-01-05") assert raw is not None and alpha is not None and days is not None assert days == 2 + assert resolved == "2026-01-07" # 2 sessions after the trade date # TradingAgentsGraph._resolve_benchmark — picks index for alpha calc @@ -641,7 +649,7 @@ class TestDeferredReflection: log.store_decision("AAPL", "2026-01-10", DECISION_BUY) mock_graph = MagicMock(spec=TradingAgentsGraph) mock_graph.memory_log = log - mock_graph._fetch_returns = MagicMock(return_value=(0.05, 0.02, 5)) + mock_graph._fetch_returns = MagicMock(return_value=(0.05, 0.02, 5, "2026-01-12")) TradingAgentsGraph._resolve_pending_entries(mock_graph, "NVDA") mock_graph._fetch_returns.assert_not_called() assert len(log.get_pending_entries()) == 1 @@ -655,7 +663,7 @@ class TestDeferredReflection: mock_graph = MagicMock(spec=TradingAgentsGraph) mock_graph.memory_log = log mock_graph.reflector = mock_reflector - mock_graph._fetch_returns = MagicMock(return_value=(0.05, 0.02, 5)) + mock_graph._fetch_returns = MagicMock(return_value=(0.05, 0.02, 5, "2026-01-12")) TradingAgentsGraph._resolve_pending_entries(mock_graph, "NVDA") assert log.get_pending_entries() == [] entries = log.load_entries() diff --git a/tests/test_memory_pointintime.py b/tests/test_memory_pointintime.py new file mode 100644 index 000000000..8d312c7c5 --- /dev/null +++ b/tests/test_memory_pointintime.py @@ -0,0 +1,95 @@ +"""Memory-log lessons must be point-in-time safe in a backtest (#1251). + +get_past_context previously returned every resolved lesson regardless of the run +date, so a historical run could learn from an outcome that had not happened yet. +Resolved entries now record the date their outcome became known (``resolved:``), +and get_past_context(as_of=...) filters on it. Legacy entries without a +resolution date are excluded from a point-in-time query (conservative migration). +""" +from __future__ import annotations + +import pytest + +from tradingagents.agents.utils.memory import TradingMemoryLog + + +def _log(tmp_path): + return TradingMemoryLog({"memory_log_path": str(tmp_path / "mem.md")}) + + +def _resolve(log, ticker, date, resolution_date, reflection): + log.store_decision(ticker, date, f"Rating: Buy\n{reflection}") + log.update_with_outcome( + ticker, date, 0.05, 0.02, 5, reflection, resolution_date=resolution_date, + ) + + +@pytest.mark.unit +def test_resolution_date_is_stored_and_parsed(tmp_path): + log = _log(tmp_path) + _resolve(log, "NVDA", "2026-01-05", "2026-01-10", "outcome known 01-10") + entry = log.load_entries()[0] + assert entry["resolved"] == "2026-01-10" + assert "resolved:2026-01-10" in (tmp_path / "mem.md").read_text() + + +@pytest.mark.unit +def test_as_of_excludes_lessons_resolved_after_the_run_date(tmp_path): + log = _log(tmp_path) + # Decision on 01-05, outcome only known on 01-10. + _resolve(log, "NVDA", "2026-01-05", "2026-01-10", "great trade") + + # A run as-of 01-07 must NOT see it (the outcome was still in the future). + assert log.get_past_context("NVDA", as_of="2026-01-07") == "" + # A run as-of 01-10 (and later) sees it. + assert "great trade" in log.get_past_context("NVDA", as_of="2026-01-10") + assert "great trade" in log.get_past_context("NVDA", as_of="2026-02-01") + + +@pytest.mark.unit +def test_no_as_of_is_unfiltered_live_behavior(tmp_path): + log = _log(tmp_path) + _resolve(log, "NVDA", "2026-01-05", "2026-01-10", "great trade") + # Live run (no as_of): unchanged behavior, lesson is shown. + assert "great trade" in log.get_past_context("NVDA") + + +@pytest.mark.unit +def test_legacy_entry_without_resolution_date_excluded_in_backtest(tmp_path): + log = _log(tmp_path) + # Simulate a pre-migration resolved entry: no resolution_date recorded. + log.store_decision("NVDA", "2026-01-05", "Rating: Buy\nlegacy lesson") + log.update_with_outcome("NVDA", "2026-01-05", 0.05, 0.02, 5, "legacy lesson") + entry = log.load_entries()[0] + assert entry["resolved"] is None + + # Conservative: excluded from a point-in-time query (can't prove it was known)... + assert log.get_past_context("NVDA", as_of="2026-06-01") == "" + # ...but still available on a live (unfiltered) run. + assert "legacy lesson" in log.get_past_context("NVDA") + + +@pytest.mark.unit +def test_cross_ticker_lessons_are_also_gated(tmp_path): + log = _log(tmp_path) + _resolve(log, "AAPL", "2026-01-05", "2026-01-10", "cross lesson") + # Querying a different ticker as-of before resolution: no cross lesson leaks. + assert log.get_past_context("NVDA", as_of="2026-01-07") == "" + assert "cross lesson" in log.get_past_context("NVDA", as_of="2026-01-10") + + +@pytest.mark.unit +def test_memory_as_of_gates_historical_but_not_live(): + # The graph filters only for a past trade date; a current-date run passes + # None so live behavior and legacy entries are unaffected (#1251). + from datetime import datetime, timedelta + + from tradingagents.graph.trading_graph import TradingAgentsGraph + + g = object.__new__(TradingAgentsGraph) + past = "2024-01-01" + today = datetime.now().strftime("%Y-%m-%d") + future = (datetime.now() + timedelta(days=30)).strftime("%Y-%m-%d") + assert g._memory_as_of(past) == past # backtest -> filter on the trade date + assert g._memory_as_of(today) is None # live -> no filter + assert g._memory_as_of(future) is None # future-dated run -> no filter diff --git a/tests/test_symbol_normalization_paths.py b/tests/test_symbol_normalization_paths.py index 1f4d9c88d..1e0bac6bf 100644 --- a/tests/test_symbol_normalization_paths.py +++ b/tests/test_symbol_normalization_paths.py @@ -41,18 +41,21 @@ def test_fetch_returns_normalizes_symbol(monkeypatch): queried.append(symbol) def history(self, *args, **kwargs): - return pd.DataFrame({"Close": [100.0, 101.0, 102.0, 103.0, 104.0, 105.0, 106.0]}) + prices = [100.0, 101.0, 102.0, 103.0, 104.0, 105.0, 106.0] + idx = pd.date_range(start="2025-01-02", periods=len(prices), freq="D") + return pd.DataFrame({"Close": prices}, index=idx) monkeypatch.setattr(tg.yf, "Ticker", FakeTicker) # _fetch_returns does not use ``self``; call unbound to avoid building the graph. - raw, alpha, days = TradingAgentsGraph._fetch_returns( + raw, alpha, days, resolved = TradingAgentsGraph._fetch_returns( None, "XAUUSD", "2025-01-02", holding_days=5, benchmark="SPY" ) assert queried[0] == "GC=F" # stock symbol normalized (#984) assert queried[1] == "SPY" # benchmark left as the canonical symbol assert raw is not None and days is not None + assert resolved == "2025-01-07" # resolution date recorded (#1251) def test_news_lookup_normalizes_symbol(monkeypatch): diff --git a/tradingagents/agents/utils/memory.py b/tradingagents/agents/utils/memory.py index ff9e94579..30b16339b 100644 --- a/tradingagents/agents/utils/memory.py +++ b/tradingagents/agents/utils/memory.py @@ -67,9 +67,21 @@ class TradingMemoryLog: """Return entries with outcome:pending (for Phase B).""" return [e for e in self.load_entries() if e.get("pending")] - def get_past_context(self, ticker: str, n_same: int = 5, n_cross: int = 3) -> str: - """Return formatted past context string for agent prompt injection.""" + def get_past_context( + self, ticker: str, n_same: int = 5, n_cross: int = 3, as_of: str | None = None + ) -> str: + """Return formatted past context string for agent prompt injection. + + When ``as_of`` (yyyy-mm-dd) is given, only lessons whose outcome was + already known by that date are included — an entry is kept only if it + stores a resolution date (``resolved:...``) that is on or before + ``as_of``. This keeps a historical/backtest run from learning from + outcomes that had not happened yet (#1251). ``as_of=None`` disables the + filter, so live runs and pre-migration entries are unaffected. + """ entries = [e for e in self.load_entries() if not e.get("pending")] + if as_of is not None: + entries = [e for e in entries if e.get("resolved") and e["resolved"] <= as_of] if not entries: return "" @@ -104,12 +116,14 @@ class TradingMemoryLog: alpha_return: float, holding_days: int, reflection: str, + resolution_date: str | None = None, ) -> None: """Replace pending tag and append REFLECTION section using atomic write. Finds the first pending entry matching (trade_date, ticker), updates - its tag with return figures, and appends a REFLECTION section. Uses - a temp-file + os.replace() so a crash mid-write never corrupts the log. + its tag with return figures (and the ``resolution_date`` the outcome + became known), and appends a REFLECTION section. Uses a temp-file + + os.replace() so a crash mid-write never corrupts the log. """ if not self._log_path or not self._log_path.exists(): return @@ -140,9 +154,8 @@ class TradingMemoryLog: # Parse rating from the existing pending tag fields = [f.strip() for f in tag_line[1:-1].split("|")] rating = fields[2] - new_tag = ( - f"[{trade_date} | {ticker} | {rating}" - f" | {raw_pct} | {alpha_pct} | {holding_days}d]" + new_tag = self._resolved_tag( + trade_date, ticker, rating, raw_pct, alpha_pct, holding_days, resolution_date ) rest = "\n".join(lines[1:]) new_blocks.append( @@ -194,9 +207,9 @@ class TradingMemoryLog: rating = fields[2] raw_pct = f"{upd['raw_return']:+.1%}" alpha_pct = f"{upd['alpha_return']:+.1%}" - new_tag = ( - f"[{trade_date} | {ticker} | {rating}" - f" | {raw_pct} | {alpha_pct} | {upd['holding_days']}d]" + new_tag = self._resolved_tag( + trade_date, ticker, rating, raw_pct, alpha_pct, + upd["holding_days"], upd.get("resolution_date"), ) rest = "\n".join(lines[1:]) new_blocks.append( @@ -217,6 +230,21 @@ class TradingMemoryLog: # --- Helpers --- + @staticmethod + def _resolved_tag( + trade_date, ticker, rating, raw_pct, alpha_pct, holding_days, resolution_date + ) -> str: + """Build a resolved entry tag, recording the outcome's known-by date. + + ``resolution_date`` (the date of the last price bar used for the return) + is the point-in-time cutoff a later run filters on (#1251). Omitted when + unavailable, keeping the legacy 6-field tag. + """ + tag = f"[{trade_date} | {ticker} | {rating} | {raw_pct} | {alpha_pct} | {holding_days}d" + if resolution_date: + tag += f" | resolved:{resolution_date}" + return tag + "]" + def _apply_rotation(self, blocks: list[str]) -> list[str]: """Drop oldest resolved blocks when their count exceeds max_entries. @@ -264,6 +292,12 @@ class TradingMemoryLog: fields = [f.strip() for f in tag_line[1:-1].split("|")] if len(fields) < 4: return None + # Optional trailing "resolved:YYYY-MM-DD" field records when the outcome + # became known, for point-in-time filtering (#1251). + resolved = None + for f in fields[6:]: + if f.startswith("resolved:"): + resolved = f[len("resolved:"):].strip() entry = { "date": fields[0], "ticker": fields[1], @@ -272,6 +306,7 @@ class TradingMemoryLog: "raw": fields[3] if fields[3] != "pending" else None, "alpha": fields[4] if len(fields) > 4 else None, "holding": fields[5] if len(fields) > 5 else None, + "resolved": resolved, } body = "\n".join(lines[1:]).strip() decision_match = self._DECISION_RE.search(body) diff --git a/tradingagents/graph/trading_graph.py b/tradingagents/graph/trading_graph.py index fefbd2342..2ae1e5bf9 100644 --- a/tradingagents/graph/trading_graph.py +++ b/tradingagents/graph/trading_graph.py @@ -272,7 +272,7 @@ class TradingAgentsGraph: def _fetch_returns( self, ticker: str, trade_date: str, holding_days: int = 5, benchmark: str = "SPY", - ) -> tuple[float | None, float | None, int | None]: + ) -> tuple[float | None, float | None, int | None, str | None]: """Fetch raw and alpha return for ticker over holding_days from trade_date. ``benchmark`` is the index used as the alpha baseline (resolved by the @@ -294,7 +294,7 @@ class TradingAgentsGraph: bench = yf.Ticker(benchmark).history(start=trade_date, end=end_str) if len(stock) < 2 or len(bench) < 2: - return None, None, None + return None, None, None, None actual_days = min(holding_days, len(stock) - 1, len(bench) - 1) raw = float( @@ -306,13 +306,16 @@ class TradingAgentsGraph: / bench["Close"].iloc[0] ) alpha = raw - bench_ret - return raw, alpha, actual_days + # The date of the last price bar used is when this outcome became + # known — the point-in-time cutoff for injecting the lesson (#1251). + resolution_date = stock.index[actual_days].strftime("%Y-%m-%d") + return raw, alpha, actual_days, resolution_date except Exception as e: logger.warning( "Could not resolve outcome for %s on %s vs %s (will retry next run): %s", ticker, trade_date, benchmark, e, ) - return None, None, None + return None, None, None, None def _resolve_pending_entries(self, ticker: str) -> None: """Resolve pending log entries for ticker at the start of a new run. @@ -331,7 +334,7 @@ class TradingAgentsGraph: benchmark = self._resolve_benchmark(ticker) updates = [] for entry in pending: - raw, alpha, days = self._fetch_returns( + raw, alpha, days, resolution_date = self._fetch_returns( ticker, entry["date"], benchmark=benchmark, ) if raw is None: @@ -349,6 +352,7 @@ class TradingAgentsGraph: "alpha_return": alpha, "holding_days": days, "reflection": reflection, + "resolution_date": resolution_date, }) if updates: @@ -366,6 +370,17 @@ class TradingAgentsGraph: identity = resolve_instrument_identity(ticker) return build_instrument_context(ticker, asset_type, identity) + def _memory_as_of(self, trade_date) -> str | None: + """Point-in-time cutoff for past-context lessons (#1251). + + A historical/backtest run (trade date before today) filters lessons to + those already resolved by the trade date. A current-date run returns + None, disabling the filter so live behavior and pre-migration entries + (which have no stored resolution date) are unaffected. + """ + td = str(trade_date) + return td if td < datetime.now().strftime("%Y-%m-%d") else None + def _run_signature(self, asset_type: str) -> str: """Graph-shape inputs that must invalidate a checkpoint if changed. @@ -476,8 +491,12 @@ class TradingAgentsGraph: checkpoint_thread_id: str | None = None): """Execute the graph and write the resulting state to disk and memory log.""" # Initialize state — inject memory log context for PM and the - # deterministically resolved instrument identity for all agents. - past_context = self.memory_log.get_past_context(company_name) + # deterministically resolved instrument identity for all agents. On a + # historical run, gate lessons to those whose outcome was known by the + # trade date so a backtest can't learn from the future (#1251). + past_context = self.memory_log.get_past_context( + company_name, as_of=self._memory_as_of(trade_date) + ) instrument_context = self.resolve_instrument_context(company_name, asset_type) init_agent_state = self.propagator.create_initial_state( company_name,