diff --git a/tests/test_cli_decision_log.py b/tests/test_cli_decision_log.py index b5bebb260..63dfee7ac 100644 --- a/tests/test_cli_decision_log.py +++ b/tests/test_cli_decision_log.py @@ -73,6 +73,10 @@ class _FakeGraph: self.calls.append(("create_run_state", ticker, trade_date)) return {"messages": [], "company_of_interest": ticker} + def process_signal(self, text): + from tradingagents.graph.signal_processing import SignalProcessor + return SignalProcessor.process_signal(None, text) + def record_decision(self, ticker, trade_date, final_state): self.calls.append(("record_decision", ticker, trade_date, final_state.get("final_trade_decision"))) diff --git a/tests/test_memory_log.py b/tests/test_memory_log.py index 78f615297..fdf337180 100644 --- a/tests/test_memory_log.py +++ b/tests/test_memory_log.py @@ -193,10 +193,14 @@ class TestTradingMemoryLogCore: log.store_decision("AAPL", "2026-01-11", DECISION_OVERWEIGHT) assert log.load_entries()[0]["rating"] == "Overweight" - def test_rating_fallback_hold(self, tmp_path): + def test_an_unreadable_decision_is_tagged_for_review(self, tmp_path): + """Not a Hold: a fabricated rating is quoted back to the next run as a + call that was never made, and counted in the backtest figures.""" + from tradingagents.agents.utils.rating import RATING_REVIEW + log = make_log(tmp_path) log.store_decision("MSFT", "2026-01-12", DECISION_NO_RATING) - assert log.load_entries()[0]["rating"] == "Hold" + assert log.load_entries()[0]["rating"] == RATING_REVIEW def test_rating_priority_over_prose(self, tmp_path): """'Rating: X' label wins even when an opposing rating word appears earlier in prose.""" diff --git a/tests/test_rating_integrity.py b/tests/test_rating_integrity.py new file mode 100644 index 000000000..8d8fcb0cf --- /dev/null +++ b/tests/test_rating_integrity.py @@ -0,0 +1,211 @@ +"""A decision is recorded as the call that was made, or as needing review. + +Two readers used to disagree about the same text: the signal said REVIEW while +the memory log wrote a fabricated Hold. Worse, prose that argued against a Buy +before concluding Underweight was read as Buy, because the parser took the first +rating word anywhere in the document. A wrong direction is worse than no +direction, so an unclear decision is REVIEW everywhere. +""" + +from __future__ import annotations + +import pytest + +from tradingagents.agents.utils.rating import RATING_REVIEW, extract_rating, parse_rating + +INVERTED = ("The aggressive analyst pushed hard for a Buy on the AI backlog, but the " + "conservative case on margin compression carried the debate. " + "Final rating — Underweight. Trim to half weight over the next two weeks.") +REFUSAL = "I'm sorry, I can't provide a rating for this security." + + +@pytest.mark.unit +@pytest.mark.parametrize("separator", [":", "-", "—", "–", "：", ": **"]) +def test_the_labelled_rating_wins_whatever_separates_it(separator): + text = f"Buy arguments were raised and rejected.\n\nRating{separator}Underweight\n\nTrim." + assert extract_rating(text) == "Underweight" + + +@pytest.mark.unit +def test_a_rating_argued_against_is_not_read_as_the_decision(): + assert extract_rating(INVERTED) == "Underweight" + + +@pytest.mark.unit +def test_prose_naming_several_ratings_without_a_label_needs_review(): + """Nothing in the text says which one is the call, so guessing risks + reporting the opposite of the decision.""" + text = "The bull wants Buy, the bear wants Sell, and the committee was split." + assert extract_rating(text) is None + + +@pytest.mark.unit +def test_prose_naming_one_rating_is_taken_as_the_call(): + assert extract_rating("On balance we stay Underweight until margins recover.") == "Underweight" + + +@pytest.mark.unit +def test_a_refusal_has_no_rating_and_is_not_defaulted(): + assert extract_rating(REFUSAL) is None + assert parse_rating(REFUSAL) == RATING_REVIEW + + +@pytest.mark.unit +def test_the_scale_quoted_in_a_prompt_does_not_become_the_rating(): + """A free-text answer that echoes the rating scale was read as the first + tier listed in it.""" + text = ("**Rating Scale**: Buy, Overweight, Hold, Underweight, Sell.\n\n" + "**Rating**: Sell\n\nExit the position.") + assert extract_rating(text) == "Sell" + + +# --- the readers agree ------------------------------------------------------ + +@pytest.mark.unit +def test_the_memory_log_records_review_rather_than_a_tradeable_hold(tmp_path): + from tradingagents.agents.utils.memory import TradingMemoryLog + + log = TradingMemoryLog({"memory_log_path": str(tmp_path / "m.md")}) + log.store_decision("NVDA", "2026-01-05", REFUSAL) + + entry = log.load_entries()[0] + assert entry["rating"] == RATING_REVIEW + + +@pytest.mark.unit +def test_the_signal_and_the_log_agree_on_the_same_decision(tmp_path): + from tradingagents.agents.utils.memory import TradingMemoryLog + from tradingagents.graph.signal_processing import SignalProcessor + + log = TradingMemoryLog({"memory_log_path": str(tmp_path / "m.md")}) + for text in (INVERTED, REFUSAL, "**Rating**: Buy\n\nAccumulate."): + log.store_decision("NVDA", f"2026-01-0{len(log.load_entries()) + 1}", text) + + signals = [SignalProcessor.process_signal(None, text) + for text in (INVERTED, REFUSAL, "**Rating**: Buy\n\nAccumulate.")] + assert [e["rating"] for e in log.load_entries()] == signals + + +@pytest.mark.unit +def test_an_unscored_decision_is_left_out_of_the_backtest_figures(tmp_path): + """REVIEW has no direction, so it cannot count for or against the system.""" + from tradingagents.agents.utils.memory import TradingMemoryLog + from tradingagents.backtest import summarize + + log = TradingMemoryLog({"memory_log_path": str(tmp_path / "m.md")}) + log.store_decision("NVDA", "2026-01-05", "**Rating**: Buy\n\nx") + log.update_with_outcome("NVDA", "2026-01-05", 0.1, 0.04, 5, "note", "2026-02-01") + log.store_decision("AAPL", "2026-01-05", REFUSAL) + log.update_with_outcome("AAPL", "2026-01-05", 0.1, 0.04, 5, "note", "2026-02-01") + + summary = summarize(log) + assert set(summary.by_rating) == {"Buy"} + + +@pytest.mark.unit +def test_the_cli_says_when_a_run_produced_no_usable_rating(monkeypatch, tmp_path, capsys): + """The CLI is the primary entry point; an unreadable decision must be + visible there, not only in the log.""" + import cli.main as m + from cli.models import AnalystType + + printed = [] + + class _Graph: + graph = propagator = None + + def create_run_state(self, *a, **k): + return {"messages": []} + + def record_decision(self, *a, **k): + pass + + def process_signal(self, text): + from tradingagents.graph.signal_processing import SignalProcessor + return SignalProcessor.process_signal(None, text) + + def get_graph_args(self, callbacks=None): + return {} + + def begin_checkpoint(self, *a, **k): + return None + + def checkpoint_input(self, state): + return state + + def clear_checkpoint_on_success(self, *a, **k): + pass + + def end_checkpoint(self): + pass + + def stream(self, *a, **k): + yield {"messages": [], "final_trade_decision": REFUSAL} + + fake = _Graph() + fake.graph = fake + fake.propagator = fake + monkeypatch.setattr(m, "TradingAgentsGraph", lambda *a, **k: fake) + monkeypatch.setattr(m, "create_layout", lambda: None) + monkeypatch.setattr(m, "update_display", lambda *a, **k: None) + monkeypatch.setattr(m, "Live", type("L", (), {"__init__": lambda s, *a, **k: None, + "__enter__": lambda s: s, + "__exit__": lambda s, *a: False})) + monkeypatch.setattr(m.console, "print", lambda *a, **k: printed.append(" ".join(str(x) for x in a))) + monkeypatch.setattr(m, "display_complete_report", lambda *a, **k: None) + monkeypatch.setattr(m.typer, "prompt", lambda *a, **k: "N") + monkeypatch.setattr(m, "get_user_selections", lambda: { + "ticker": "NVDA", "analysis_date": "2026-01-10", + "analysts": [AnalystType.MARKET], "asset_type": "stock", + }) + monkeypatch.setattr(m, "_build_run_config", lambda s, c: { + "data_cache_dir": str(tmp_path / "c"), "results_dir": str(tmp_path / "r")}) + + m.run_analysis() + + assert any("review" in line.lower() for line in printed), printed[-5:] + + +@pytest.mark.unit +@pytest.mark.parametrize("module, factory, must_name", [ + ("tradingagents.agents.managers.portfolio_manager", "create_portfolio_manager", "Rating"), + ("tradingagents.agents.managers.research_manager", "create_research_manager", "Recommendation"), + ("tradingagents.agents.trader.trader", "create_trader", "Action"), +]) +def test_a_decision_prompt_states_the_shape_of_its_answer(module, factory, must_name): + """The field descriptions live in the schema, which a provider without + structured output never sees. Without the format in the prompt body, the + fallback answer is prose nobody can read a rating from.""" + import importlib + + from langchain_core.messages import AIMessage + + mod = importlib.import_module(module) + seen = [] + + class _LLM: + def invoke(self, prompt, *a, **k): + seen.append(prompt if isinstance(prompt, str) else str(prompt)) + return AIMessage("**Rating**: Hold\n\nnothing to do") + + def with_structured_output(self, *a, **k): + raise NotImplementedError # force the free-text path + + state = { + "company_of_interest": "NVDA", "trade_date": "2026-08-14", "asset_type": "stock", + "instrument_context": "", "market_report": "M", "sentiment_report": "S", + "news_report": "N", "fundamentals_report": "F", "investment_plan": "P", + "trader_investment_plan": "T", "past_context": "", "portfolio_context": "", + "investment_debate_state": {"bull_history": "b", "bear_history": "r", "history": "h", + "current_response": "", "judge_decision": "", "count": 2}, + "risk_debate_state": {"history": "h", "latest_speaker": "", "count": 3, + "aggressive_history": "", "conservative_history": "", "neutral_history": "", + "current_aggressive_response": "", "current_conservative_response": "", + "current_neutral_response": "", "judge_decision": ""}, + } + getattr(mod, factory)(_LLM())(state) + + prompt = " ".join(seen) + assert "## Output" in prompt, "no output-format section in the prompt" + section = prompt.split("## Output", 1)[1] + assert f"**{must_name}**" in section, section[:300] diff --git a/tests/test_signal_processing.py b/tests/test_signal_processing.py index 705c81183..bcb7ccdec 100644 --- a/tests/test_signal_processing.py +++ b/tests/test_signal_processing.py @@ -56,8 +56,9 @@ class TestParseRating: ) assert parse_rating(text) == "Sell" - def test_no_rating_returns_default(self): - assert parse_rating("No clear directional signal at this time.") == "Hold" + def test_no_rating_is_flagged_for_review_not_defaulted(self): + # A decision nobody can read is not a Hold; recording one invents a call. + assert parse_rating("No clear directional signal at this time.") == RATING_REVIEW def test_no_rating_custom_default(self): assert parse_rating("Plain prose.", default="Underweight") == "Underweight" @@ -116,9 +117,9 @@ class TestExtractRating: # substrings inside larger words must not match assert extract_rating("The buyer was holding shares.") is None - def test_parse_rating_keeps_silent_default_for_compat(self): - # parse_rating (used by the memory log) intentionally keeps Hold default. - assert parse_rating("No rating here.") == "Hold" + def test_parse_rating_defaults_to_review(self): + # The memory log tags an unreadable decision REVIEW, never a tradeable rating. + assert parse_rating("No rating here.") == RATING_REVIEW assert parse_rating("No rating here.", default="Underweight") == "Underweight" diff --git a/tradingagents/agents/utils/rating.py b/tradingagents/agents/utils/rating.py index 1a828ea21..daaa3a492 100644 --- a/tradingagents/agents/utils/rating.py +++ b/tradingagents/agents/utils/rating.py @@ -8,10 +8,10 @@ The same five-tier scale (Buy, Overweight, Hold, Underweight, Sell) is used by: Centralising it here avoids drift between those call sites. -``extract_rating`` returns ``None`` when no rating can be found, so the graph can -surface an explicit ``REVIEW`` signal instead of a fabricated ``Hold`` (#1170). -``parse_rating`` keeps the legacy silent-default behaviour for callers (e.g. the -memory log) that need a rating string regardless. +``extract_rating`` returns ``None`` when no rating can be found, and every +caller turns that into ``REVIEW`` rather than a tradeable position: a decision +nobody can read is not a Hold, and a Hold recorded in its place is quoted back to +the next run as a call that was never made (#1170). """ from __future__ import annotations @@ -32,9 +32,13 @@ RATING_REVIEW = "REVIEW" _RATING_SET = {r.lower() for r in RATINGS_5_TIER} -# Matches "Rating: X" / "rating - X" / "Rating: **X**" — tolerates markdown -# bold wrappers and either a colon or hyphen separator. -_RATING_LABEL_RE = re.compile(r"rating.*?[:\-][\s*]*(\w+)", re.IGNORECASE) +# Matches "Rating: X" / "rating - X" / "Rating — **X**" — tolerates markdown +# bold wrappers and any dash or colon a model writes as the separator. +_RATING_LABEL_RE = re.compile(r"rating\b[^:\-\u2010-\u2015]*[:\-\u2010-\u2015][\s*]*(\w+)", + re.IGNORECASE) + +# A line presenting the scale rather than a decision ("Rating Scale: Buy, ..."). +_RATING_SCALE_RE = re.compile(r"rating\s*(scale|options|legend)", re.IGNORECASE) # Standalone 5-tier word anywhere (word boundaries so "Buyer"/"Holding" don't match). _RATING_WORD_RE = re.compile( @@ -54,25 +58,31 @@ def extract_rating(text: str) -> str | None: return None norm = unicodedata.normalize("NFKC", text) + # The labelled rating, taking the last one written: a decision states its + # rating after discussing the alternatives. Lines presenting the scale + # itself are a legend the model echoed, not a call. + labelled = None for line in norm.splitlines(): + if _RATING_SCALE_RE.search(line): + continue m = _RATING_LABEL_RE.search(line) if m and m.group(1).lower() in _RATING_SET: - return m.group(1).capitalize() + labelled = m.group(1).capitalize() + if labelled: + return labelled - m = _RATING_WORD_RE.search(norm) - if m: - return m.group(1).capitalize() - - return None + # No label. A single rating word in the text is the call; several are an + # argument, and picking one of them reports a direction nobody decided -- + # prose that rejects a Buy before concluding Underweight read as Buy. + named = {m.group(1).capitalize() for m in _RATING_WORD_RE.finditer(norm)} + return named.pop() if len(named) == 1 else None -def parse_rating(text: str, default: str = "Hold") -> str: - """Extract a 5-tier rating, falling back to ``default`` when none is found. +def parse_rating(text: str, default: str = RATING_REVIEW) -> str: + """Extract a 5-tier rating, or ``REVIEW`` when the decision has none. - Legacy convenience wrapper: it always returns a rating string, so an - unparseable decision silently becomes ``default`` (``Hold``). Callers that - must distinguish "no rating" from a real Hold should use - :func:`extract_rating` (or the graph's REVIEW-surfacing signal) instead. + For callers that need a string for every decision, such as the memory log's + entry tag. The default is the review sentinel, never a tradeable rating. """ rating = extract_rating(text) return rating if rating is not None else default diff --git a/tradingagents/backtest.py b/tradingagents/backtest.py index f6d9a4e23..62ef81ef7 100644 --- a/tradingagents/backtest.py +++ b/tradingagents/backtest.py @@ -22,6 +22,7 @@ from datetime import datetime, timedelta from pathlib import Path from tradingagents.agents.utils.memory import TradingMemoryLog +from tradingagents.agents.utils.rating import RATING_REVIEW from tradingagents.dataflows.utils import get_current_date, safe_ticker_component from tradingagents.graph.trading_graph import TradingAgentsGraph @@ -92,9 +93,11 @@ class BacktestSummary: resolved: int pending: int by_rating: dict[str, RatingScore] + unscored: int = 0 def render(self) -> str: - lines = [f"Resolved cells: {self.resolved} · pending: {self.pending}"] + lines = [f"Resolved cells: {self.resolved} · pending: {self.pending}" + + (f" · unscored: {self.unscored}" if self.unscored else "")] for rating, score in self.by_rating.items(): lines.append( f"- {rating}: n={score.count}, beat the benchmark " @@ -163,7 +166,10 @@ def run_backtest( def summarize(memory_log: TradingMemoryLog) -> BacktestSummary: """Score the settled decisions in a log, by rating.""" entries = memory_log.load_entries() - resolved = [(e, _alpha(e)) for e in entries if not e["pending"]] + # A decision with no readable rating has no direction, so it can neither + # count for nor against the system; it is reported as unscored instead. + resolved = [(e, _alpha(e)) for e in entries + if not e["pending"] and e["rating"] != RATING_REVIEW] resolved = [(e, a) for e, a in resolved if a is not None] by_rating: dict[str, RatingScore] = {} for rating in dict.fromkeys(e["rating"] for e, _ in resolved): @@ -173,5 +179,7 @@ def summarize(memory_log: TradingMemoryLog) -> BacktestSummary: hit_rate=sum(a > 0 for a in alphas) / len(alphas), mean_alpha=sum(alphas) / len(alphas), ) - return BacktestSummary(resolved=len(resolved), pending=len(entries) - len(resolved), - by_rating=by_rating) + unscored = sum(1 for e in entries if e["rating"] == RATING_REVIEW) + return BacktestSummary(resolved=len(resolved), + pending=len(entries) - len(resolved) - unscored, + by_rating=by_rating, unscored=unscored)