diff --git a/tests/test_memory_log.py b/tests/test_memory_log.py index 69fdb8047f3..71858895d00 100644 --- a/tests/test_memory_log.py +++ b/tests/test_memory_log.py @@ -8,6 +8,7 @@ from tradingagents.agents.managers.portfolio_manager import create_portfolio_manager from tradingagents.agents.schemas import PortfolioDecision, PortfolioRating from tradingagents.agents.utils.memory import TradingMemoryLog +from tradingagents.agents.utils.rating import RATING_REVIEW from tradingagents.graph.propagation import Propagator from tradingagents.graph.reflection import Reflector from tradingagents.graph.trading_graph import TradingAgentsGraph @@ -171,10 +172,12 @@ def test_rating_parsed_overweight(self, tmp_path): 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_rating_review_when_unparseable(self, tmp_path): + # A decision with no parseable rating is tagged REVIEW, not a silent Hold, + # so the log never conflates a parse failure with a genuine neutral call. 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.py b/tests/test_rating.py new file mode 100644 index 00000000000..2211ad5a186 --- /dev/null +++ b/tests/test_rating.py @@ -0,0 +1,101 @@ +"""Tests for the deterministic rating parser in ``tradingagents.agents.utils.rating``. + +Covers the contract from issue #1170: an unrecognised or missing rating must never +silently become ``Hold``; canonical, markdown-wrapped, and harmless punctuation +variants parse; localized/fullwidth punctuation is normalised or surfaced; and the +5-tier rating stays distinct from the 3-tier trade action. +""" + +import pytest + +from tradingagents.agents.utils.rating import ( + RATING_REVIEW, + RATINGS_5_TIER, + extract_rating, + parse_rating, +) + + +@pytest.mark.unit +class TestExtractRating: + def test_all_five_tiers_via_label(self): + for r in RATINGS_5_TIER: + assert extract_rating(f"Rating: {r}") == r + + def test_markdown_bold_value(self): + assert extract_rating("Rating: **Sell**\nExit immediately.") == "Sell" + + def test_markdown_bold_label(self): + assert extract_rating("**Rating**: Underweight\nTrim exposure.") == "Underweight" + + def test_rendered_pm_markdown_shape(self): + text = ( + "**Rating**: Buy\n\n" + "**Executive Summary**: Enter at $189-192.\n\n" + "**Investment Thesis**: AI capex cycle intact." + ) + assert extract_rating(text) == "Buy" + + def test_label_wins_over_prose(self): + text = "The sell thesis is weakened by guidance.\nRating: **Buy**\nEnter now." + assert extract_rating(text) == "Buy" + + # --- The fix: unparseable is explicit, never a silent Hold ----------------- + + def test_no_rating_returns_none(self): + assert extract_rating("No clear directional signal at this time.") is None + + def test_empty_returns_none(self): + assert extract_rating("") is None + assert extract_rating(" \n ") is None + + def test_review_sentinel_is_not_a_tier(self): + assert RATING_REVIEW not in RATINGS_5_TIER + + # --- Fullwidth / localized punctuation ------------------------------------- + + def test_fullwidth_colon_label(self): + assert extract_rating("Rating：Overweight") == "Overweight" + + def test_fullwidth_parentheses_after_rating(self): + assert extract_rating("Final rating: Sell（bearish）") == "Sell" + + def test_localized_label_with_english_rating(self): + # Non-canonical label (评级) but a recognisable English rating word. + assert extract_rating("评级：Overweight（超配）") == "Overweight" + + def test_fullwidth_colon_label_still_wins_over_prose(self): + text = "The sell thesis is weak.\nRating：Buy\nStrong fundamentals." + assert extract_rating(text) == "Buy" + + # --- Whole-word matching: no partial/substring false positives ------------- + + @pytest.mark.parametrize("text", [ + "The buyer is holding out for a better price.", + "A motivated seller emerged this quarter.", + "Overweighting the growth sleeve is under review.", # 'Overweighting' != 'Overweight' + ]) + def test_longer_words_do_not_match(self, text): + assert extract_rating(text) is None + + # --- Rating (5-tier) is not inferred as a trade action (3-tier) ------------ + + def test_rating_not_mapped_to_trade_action(self): + assert extract_rating("Rating: Overweight") == "Overweight" # not "Buy" + assert extract_rating("Rating: Underweight") == "Underweight" # not "Sell" + + +@pytest.mark.unit +class TestParseRatingBackwardCompat: + def test_parsed_value(self): + assert parse_rating("Rating: Sell\nExit.") == "Sell" + + def test_default_when_unparseable(self): + assert parse_rating("No clear directional signal.") == "Hold" + + def test_custom_default(self): + assert parse_rating("Plain prose.", default="Underweight") == "Underweight" + + def test_default_is_only_a_fallback_not_a_match(self): + # A real rating is returned even though the default differs. + assert parse_rating("Rating: Buy", default="Sell") == "Buy" diff --git a/tests/test_signal_processing.py b/tests/test_signal_processing.py index 92520a8d10e..8ccf6e8b027 100644 --- a/tests/test_signal_processing.py +++ b/tests/test_signal_processing.py @@ -10,7 +10,7 @@ import pytest -from tradingagents.agents.utils.rating import RATINGS_5_TIER, parse_rating +from tradingagents.agents.utils.rating import RATING_REVIEW, RATINGS_5_TIER, parse_rating from tradingagents.graph.signal_processing import SignalProcessor # --------------------------------------------------------------------------- @@ -84,6 +84,8 @@ def test_makes_no_llm_calls(self): llm.invoke.assert_not_called() llm.with_structured_output.assert_not_called() - def test_default_when_no_rating_present(self): + def test_review_when_no_rating_present(self): + # A decision with no parseable rating must surface as REVIEW, not a silent + # Hold — so downstream can tell a parse failure from a genuine neutral call. sp = SignalProcessor() - assert sp.process_signal("Plain prose without a recommendation.") == "Hold" + assert sp.process_signal("Plain prose without a recommendation.") == RATING_REVIEW diff --git a/tradingagents/agents/utils/memory.py b/tradingagents/agents/utils/memory.py index ff9e94579bf..c6548b487b7 100644 --- a/tradingagents/agents/utils/memory.py +++ b/tradingagents/agents/utils/memory.py @@ -3,7 +3,7 @@ import re from pathlib import Path -from tradingagents.agents.utils.rating import parse_rating +from tradingagents.agents.utils.rating import RATING_REVIEW, extract_rating class TradingMemoryLog: @@ -42,7 +42,9 @@ def store_decision( for line in raw.splitlines(): if line.startswith(f"[{trade_date} | {ticker} |") and line.endswith("| pending]"): return - rating = parse_rating(final_trade_decision) + # Tag with the parsed rating, or RATING_REVIEW when none is parseable — so a + # logged entry never conflates a parse failure with a genuine Hold. + rating = extract_rating(final_trade_decision) or RATING_REVIEW tag = f"[{trade_date} | {ticker} | {rating} | pending]" entry = f"{tag}\n\nDECISION:\n{final_trade_decision}{self._SEPARATOR}" with open(self._log_path, "a", encoding="utf-8") as f: diff --git a/tradingagents/agents/utils/rating.py b/tradingagents/agents/utils/rating.py index 234bc568d64..003168bc5f4 100644 --- a/tradingagents/agents/utils/rating.py +++ b/tradingagents/agents/utils/rating.py @@ -7,6 +7,12 @@ - The memory log (rating tag stored alongside each decision entry) Centralising it here avoids drift between those call sites. + +:func:`extract_rating` returns ``None`` when no rating can be recognised, so a +caller can tell a genuine ``Hold`` apart from a parse failure. :func:`parse_rating` +keeps the older "return a default" contract for callers that deliberately want +one. The 5-tier *rating* is intentionally kept separate from the 3-tier *trade +action* (Buy / Sell / Hold): a rating word is never mapped onto a trade action here. """ from __future__ import annotations @@ -18,31 +24,73 @@ "Buy", "Overweight", "Hold", "Underweight", "Sell", ) +# Sentinel for "no parseable rating". It is deliberately NOT a member of the +# 5-tier scale: a genuine neutral call is "Hold", whereas this marks "the text +# carried no recognisable rating" so downstream consumers can surface it for +# review instead of mistaking a parse failure for a real Hold. +RATING_REVIEW = "REVIEW" + _RATING_SET = {r.lower() for r in RATINGS_5_TIER} +# Fullwidth / CJK punctuation that commonly wraps a rating in localized output, +# normalised to its ASCII equivalent so the label match and the tokeniser see the +# rating cleanly (e.g. "Rating：Overweight", "Sell（bearish）"). Only punctuation is +# mapped; letters and content are untouched. +_PUNCT_NORMALIZE = str.maketrans({ + "：": ":", "－": "-", "‐": "-", + "（": "(", "）": ")", "［": "[", "］": "]", + "，": ",", "、": ",", "．": ".", "｜": "|", +}) + # 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) -def parse_rating(text: str, default: str = "Hold") -> str: - """Heuristically extract a 5-tier rating from prose text. +def _normalize(text: str) -> str: + return text.translate(_PUNCT_NORMALIZE) + + +def extract_rating(text: str) -> str | None: + """Heuristically extract a 5-tier rating, or ``None`` if none is recognised. + + After normalising harmless fullwidth/CJK punctuation, a two-pass strategy: - Two-pass strategy: - 1. Look for an explicit "Rating: X" label (tolerant of markdown bold). - 2. Fall back to the first 5-tier rating word found anywhere in the text. + 1. An explicit ``Rating: X`` label (tolerant of markdown bold) takes priority. + 2. Otherwise, the first 5-tier rating *word* found anywhere in the text. - Returns a Title-cased rating string, or ``default`` if no rating word appears. + Returns a Title-cased rating string, or ``None`` when the text contains no + recognisable rating — never a silent default. Matching is on whole alphabetic + tokens, so "buyer", "holding" or "seller" do not spuriously match. """ - for line in text.splitlines(): + if not text: + return None + norm = _normalize(text) + + # Pass 1: an explicit label anywhere wins over a bare rating word in prose. + for line in norm.splitlines(): m = _RATING_LABEL_RE.search(line) if m and m.group(1).lower() in _RATING_SET: return m.group(1).capitalize() - for line in text.splitlines(): - for word in line.lower().split(): - clean = word.strip("*:.,") - if clean in _RATING_SET: - return clean.capitalize() + # Pass 2: first standalone rating word. re.findall on [A-Za-z]+ splits on any + # non-letter — whitespace, ASCII or normalised fullwidth punctuation, or CJK — + # so a rating glued to a localized label ("评级:Overweight") is still found, + # while longer words ("buyer", "holding") are not partially matched. + for line in norm.splitlines(): + for token in re.findall(r"[A-Za-z]+", line): + if token.lower() in _RATING_SET: + return token.capitalize() + + return None + - return default +def parse_rating(text: str, default: str = "Hold") -> str: + """Extract a 5-tier rating, falling back to ``default`` when none is found. + + Backwards-compatible convenience wrapper over :func:`extract_rating`, for + callers that deliberately want a default. Prefer :func:`extract_rating` when a + parse failure must be distinguished from a genuine rating. + """ + rating = extract_rating(text) + return rating if rating is not None else default diff --git a/tradingagents/graph/signal_processing.py b/tradingagents/graph/signal_processing.py index 90fafd04b43..7d10d8eda72 100644 --- a/tradingagents/graph/signal_processing.py +++ b/tradingagents/graph/signal_processing.py @@ -14,7 +14,7 @@ from typing import Any -from tradingagents.agents.utils.rating import parse_rating +from tradingagents.agents.utils.rating import RATING_REVIEW, extract_rating class SignalProcessor: @@ -27,5 +27,12 @@ def __init__(self, quick_thinking_llm: Any = None): self.quick_thinking_llm = quick_thinking_llm def process_signal(self, full_signal: str) -> str: - """Return one of Buy / Overweight / Hold / Underweight / Sell.""" - return parse_rating(full_signal) + """Return one of Buy / Overweight / Hold / Underweight / Sell. + + When the decision text carries no parseable rating, return + :data:`~tradingagents.agents.utils.rating.RATING_REVIEW` rather than a + silent ``Hold`` — so a parse failure is visible to downstream consumers + instead of masquerading as a genuine neutral call. + """ + rating = extract_rating(full_signal) + return rating if rating is not None else RATING_REVIEW