From a94a411b0e404e49833c4aada1616495f1696e62 Mon Sep 17 00:00:00 2001 From: Yijia-Xiao Date: Thu, 24 Sep 2026 08:22:23 +0000 Subject: [PATCH] fix(agents): carry the Portfolio Manager's rating through the run (#1383) - the typed rating is the state's final_rating; propagate returns it, and the memory log tag, the state log and the CLI review check read it - the decision text is parsed only when the Portfolio Manager answered in free text - TradingAgentsGraph.process_signal is removed --- cli/run.py | 4 +- tests/test_cli_decision_log.py | 6 +-- tests/test_cli_display.py | 2 +- tests/test_graph_end_to_end.py | 6 ++- tests/test_memory_log.py | 2 +- tests/test_portfolio_context.py | 3 +- tests/test_rating_integrity.py | 18 +++++-- tests/test_signal_processing.py | 17 ------ .../agents/managers/portfolio_manager.py | 34 ++++++------ tradingagents/agents/rating.py | 12 ++++- tradingagents/agents/state.py | 1 + tradingagents/agents/structured.py | 54 ++++++++++--------- tradingagents/graph/trading_graph.py | 12 ++--- tradingagents/memory.py | 9 +++- 14 files changed, 93 insertions(+), 87 deletions(-) diff --git a/cli/run.py b/cli/run.py index 08896cf2e..7d0bda236 100644 --- a/cli/run.py +++ b/cli/run.py @@ -23,7 +23,7 @@ from cli.display import ( ) from cli.selections import get_user_selections from cli.stats_handler import StatsCallbackHandler -from tradingagents.agents.rating import is_review +from tradingagents.agents.rating import is_review, run_rating from tradingagents.dataflows.symbols import safe_ticker_component from tradingagents.default_config import DEFAULT_CONFIG from tradingagents.graph.analyst_execution import ( @@ -358,7 +358,7 @@ def run_analysis(checkpoint: bool | None = None, portfolio=None): # A decision nobody can read is not a position. Say so here rather than # leaving the run to look like a normal result. - if is_review(graph.process_signal(final_state.get("final_trade_decision", ""))): + if is_review(run_rating(final_state)): console.print( "[yellow]No rating could be read from the final decision, so this run " "is recorded for review rather than as a position. Re-run, or read the " diff --git a/tests/test_cli_decision_log.py b/tests/test_cli_decision_log.py index d8cf89da3..d6d01e784 100644 --- a/tests/test_cli_decision_log.py +++ b/tests/test_cli_decision_log.py @@ -76,10 +76,6 @@ class _FakeGraph: self.calls.append(("create_run_state", ticker, trade_date)) return {"messages": [], "company_of_interest": ticker} - def process_signal(self, text): - from tradingagents.agents.rating import parse_rating - return parse_rating(text) - def record_decision(self, ticker, trade_date, final_state): self.calls.append(("record_decision", ticker, trade_date, final_state.get("final_trade_decision"))) @@ -101,7 +97,7 @@ class _FakeGraph: def stream(self, graph_input, **kwargs): yield {"messages": [], "market_report": "M"} - yield {"messages": [], "final_trade_decision": "Rating: Buy\n\nBuy NVDA."} + yield {"messages": [], "final_trade_decision": "Rating: Buy\n\nBuy NVDA.", "final_rating": "Buy"} class _NullLive: diff --git a/tests/test_cli_display.py b/tests/test_cli_display.py index 52d419e07..8adb45ea1 100644 --- a/tests/test_cli_display.py +++ b/tests/test_cli_display.py @@ -46,7 +46,7 @@ def _state(ticker, final="评级: 买入"): "company_of_interest": ticker, "trade_date": "2026-09-01", "market_report": "市场", "sentiment_report": "情绪", "news_report": "新闻", "fundamentals_report": "基本面", "investment_plan": "计划", - "trader_investment_plan": "交易计划", "final_trade_decision": final, + "trader_investment_plan": "交易计划", "final_trade_decision": final, "final_rating": "REVIEW", "investment_debate_state": {"bull_history": "", "bear_history": "", "history": "", "current_response": "", "judge_decision": "", "count": 0}, "risk_debate_state": {"aggressive_history": "", "conservative_history": "", diff --git a/tests/test_graph_end_to_end.py b/tests/test_graph_end_to_end.py index 2bd846b89..7b791220c 100644 --- a/tests/test_graph_end_to_end.py +++ b/tests/test_graph_end_to_end.py @@ -34,8 +34,10 @@ STRUCTURED = { schemas.ResearchPlan: schemas.ResearchPlan( recommendation=schemas.PortfolioRating.OVERWEIGHT, rationale="r", strategic_actions="a"), schemas.TraderProposal: schemas.TraderProposal(action=schemas.TraderAction.BUY, reasoning="r"), + # The thesis quotes another party's rating; the decision is still the PM's own. schemas.PortfolioDecision: schemas.PortfolioDecision( - rating=schemas.PortfolioRating.OVERWEIGHT, executive_summary="s", investment_thesis="t"), + rating=schemas.PortfolioRating.OVERWEIGHT, executive_summary="s", + investment_thesis="Street consensus rating: Buy (28 of 35 analysts)."), schemas.SentimentReport: schemas.SentimentReport( overall_band=schemas.SentimentBand.NEUTRAL, overall_score=5.0, confidence="low", narrative="n"), } @@ -126,7 +128,7 @@ def test_a_full_run_reaches_a_logged_decision(tmp_path, monkeypatch, offline, st state, signal = graph.propagate("NVDA", TRADE_DATE) - assert signal == "Overweight" + assert signal == state["final_rating"] == "Overweight" for key in ("market_report", "sentiment_report", "news_report", "fundamentals_report", "investment_plan", "trader_investment_plan", "final_trade_decision"): assert state[key].strip(), key diff --git a/tests/test_memory_log.py b/tests/test_memory_log.py index 090c3af9a..5ceb01ac8 100644 --- a/tests/test_memory_log.py +++ b/tests/test_memory_log.py @@ -898,6 +898,7 @@ class TestLegacyRemoval: fake_state = { "final_trade_decision": "Rating: Buy\nBuy NVDA.", + "final_rating": "Buy", "company_of_interest": "NVDA", "trade_date": "2026-01-10", "market_report": "", @@ -924,7 +925,6 @@ class TestLegacyRemoval: mock_graph.graph.invoke.return_value = fake_state mock_graph.propagator.create_initial_state.return_value = fake_state mock_graph.propagator.get_graph_args.return_value = {} - mock_graph.process_signal.return_value = "Buy" # Bind the real _run_graph so propagate's call to self._run_graph executes # the actual write path instead of the auto-MagicMock. mock_graph._run_graph = functools.partial( diff --git a/tests/test_portfolio_context.py b/tests/test_portfolio_context.py index 3da81b5fe..228489671 100644 --- a/tests/test_portfolio_context.py +++ b/tests/test_portfolio_context.py @@ -174,9 +174,8 @@ def test_completed_run_clears_the_checkpoint_it_wrote(tmp_path, monkeypatch): graph.debug = False graph._resuming = False graph.propagator.get_graph_args = lambda callbacks=None: {} - graph.process_signal = lambda d: "Hold" graph._log_state = lambda *a, **k: None - graph.graph = type("G", (), {"invoke": lambda self, i, **k: {"final_trade_decision": "Rating: Hold\n\nx"}})() + graph.graph = type("G", (), {"invoke": lambda self, i, **k: {"final_trade_decision": "Rating: Hold\n\nx", "final_rating": "Hold"}})() book = PortfolioContext.model_validate(HOLDING) written = graph._run_signature("stock", book) # what begin_checkpoint keys on diff --git a/tests/test_rating_integrity.py b/tests/test_rating_integrity.py index 692ad918a..8fcadac4a 100644 --- a/tests/test_rating_integrity.py +++ b/tests/test_rating_integrity.py @@ -121,10 +121,6 @@ def test_the_cli_says_when_a_run_produced_no_usable_rating(monkeypatch, tmp_path def record_decision(self, *a, **k): pass - def process_signal(self, text): - from tradingagents.agents.rating import parse_rating - return parse_rating(text) - def get_graph_args(self, callbacks=None): return {} @@ -141,7 +137,7 @@ def test_the_cli_says_when_a_run_produced_no_usable_rating(monkeypatch, tmp_path pass def stream(self, *a, **k): - yield {"messages": [], "final_trade_decision": REFUSAL} + yield {"messages": [], "final_trade_decision": REFUSAL, "final_rating": RATING_REVIEW} fake = _Graph() fake.graph = fake @@ -210,3 +206,15 @@ def test_a_decision_prompt_states_the_shape_of_its_answer(module, factory, must_ 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] + + +@pytest.mark.unit +def test_a_state_without_the_typed_rating_reads_it_from_the_decision(): + """A run finished by an older version and resumed from its checkpoint has + no final_rating; every reader falls back the same way instead of one + raising and another reporting REVIEW.""" + from tradingagents.agents.rating import run_rating + + assert run_rating({"final_rating": "Hold", "final_trade_decision": "**Rating**: Buy"}) == "Hold" + assert run_rating({"final_trade_decision": "**Rating**: Sell\n\nExit."}) == "Sell" + assert run_rating({}) == RATING_REVIEW diff --git a/tests/test_signal_processing.py b/tests/test_signal_processing.py index 4a0440c6f..7976b8239 100644 --- a/tests/test_signal_processing.py +++ b/tests/test_signal_processing.py @@ -75,20 +75,3 @@ class TestExtractRating: # 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" - - -@pytest.mark.unit -class TestGraphSignalContract: - """The graph-facing signal (TradingAgentsGraph.process_signal) honors the - documented "5-tier or REVIEW" contract, not just the parser in isolation.""" - - def _bare_graph(self): - from tradingagents.graph.trading_graph import TradingAgentsGraph - g = object.__new__(TradingAgentsGraph) - return g - - def test_graph_surfaces_review(self): - assert self._bare_graph().process_signal("no rating in here") == RATING_REVIEW - - def test_graph_returns_rating(self): - assert self._bare_graph().process_signal("**Rating**: Sell") == "Sell" diff --git a/tradingagents/agents/managers/portfolio_manager.py b/tradingagents/agents/managers/portfolio_manager.py index 442fbb5d6..228758221 100644 --- a/tradingagents/agents/managers/portfolio_manager.py +++ b/tradingagents/agents/managers/portfolio_manager.py @@ -1,11 +1,11 @@ """Portfolio Manager: synthesises the risk-analyst debate into the final decision. Uses LangChain's ``with_structured_output`` so the LLM produces a typed -``PortfolioDecision`` directly, in a single call. The result is rendered -back to markdown for storage in ``final_trade_decision`` so memory log, -CLI display, and saved reports continue to consume the same shape they do -today. When a provider does not expose structured output, the agent falls -back gracefully to free-text generation. +``PortfolioDecision`` directly, in a single call. Its rating is the run's +``final_rating``, and the decision is rendered to markdown as +``final_trade_decision`` for the memory log, CLI display and saved reports. +When a provider does not expose structured output, the agent falls back to +free-text generation and the rating is read from that text. """ from __future__ import annotations @@ -15,12 +15,9 @@ from tradingagents.agents.context import ( get_language_instruction, get_portfolio_context_from_state, ) +from tradingagents.agents.rating import parse_rating from tradingagents.agents.schemas import PortfolioDecision, render_pm_decision -from tradingagents.agents.structured import ( - NO_EXTERNAL_TOOLS, - bind_structured, - invoke_structured_or_freetext, -) +from tradingagents.agents.structured import NO_EXTERNAL_TOOLS, bind_structured, invoke_structured def create_portfolio_manager(llm): @@ -78,13 +75,15 @@ Write these sections, in this order, starting with the rating on its own line: {NO_EXTERNAL_TOOLS}{get_language_instruction()}""" - final_trade_decision = invoke_structured_or_freetext( - structured_llm, - llm, - prompt, - render_pm_decision, - "Portfolio Manager", - ) + # The typed rating is the decision; the rendered text only carries it. + # Read back from text, a rating the thesis quotes could replace it. + decision = invoke_structured(structured_llm, prompt, "Portfolio Manager") + if decision is not None: + final_trade_decision = render_pm_decision(decision) + final_rating = decision.rating.value + else: + final_trade_decision = llm.invoke(prompt).content + final_rating = parse_rating(final_trade_decision) new_risk_debate_state = { "judge_decision": final_trade_decision, @@ -102,6 +101,7 @@ Write these sections, in this order, starting with the rating on its own line: return { "risk_debate_state": new_risk_debate_state, "final_trade_decision": final_trade_decision, + "final_rating": final_rating, } return portfolio_manager_node diff --git a/tradingagents/agents/rating.py b/tradingagents/agents/rating.py index daaa3a492..37a6db8d6 100644 --- a/tradingagents/agents/rating.py +++ b/tradingagents/agents/rating.py @@ -2,8 +2,7 @@ The same five-tier scale (Buy, Overweight, Hold, Underweight, Sell) is used by: - The Research Manager (investment plan recommendation) -- The Portfolio Manager (final position decision) -- The signal processor (rating extracted for downstream consumers) +- The Portfolio Manager (final position decision; its free-text fallback is read here) - The memory log (rating tag stored alongside each decision entry) Centralising it here avoids drift between those call sites. @@ -88,6 +87,15 @@ def parse_rating(text: str, default: str = RATING_REVIEW) -> str: return rating if rating is not None else default +def run_rating(final_state: dict) -> str: + """A finished run's rating: the Portfolio Manager's own, else read from its decision. + + The fallback serves a state without ``final_rating``, such as a run an older + version completed and a checkpoint hands back unchanged. + """ + return final_state.get("final_rating") or parse_rating(final_state.get("final_trade_decision", "")) + + def is_review(signal: str) -> bool: """Whether a signal is the non-tradeable REVIEW sentinel (#1170).""" return signal == RATING_REVIEW diff --git a/tradingagents/agents/state.py b/tradingagents/agents/state.py index 0d6df58cc..7faa545e5 100644 --- a/tradingagents/agents/state.py +++ b/tradingagents/agents/state.py @@ -73,5 +73,6 @@ class AgentState(MessagesState): RiskDebateState, "Current state of the debate on evaluating risk" ] final_trade_decision: Annotated[str, "Final decision made by the Risk Analysts"] + final_rating: Annotated[str, "The Portfolio Manager's 5-tier rating, or REVIEW when it has none"] past_context: Annotated[str, "Memory log context injected at run start (same-ticker decisions + cross-ticker lessons)"] portfolio_context: Annotated[str, "Caller-supplied holdings and cash, rendered at run start; empty when not provided"] diff --git a/tradingagents/agents/structured.py b/tradingagents/agents/structured.py index 8e94132c9..8992e5351 100644 --- a/tradingagents/agents/structured.py +++ b/tradingagents/agents/structured.py @@ -56,6 +56,31 @@ def bind_structured(llm: Any, schema: type[T], agent_name: str) -> Any | None: return None +def invoke_structured(structured_llm: Any | None, prompt: Any, agent_name: str) -> T | None: + """Run the structured call; ``None`` when there is none or it fails. + + ``prompt`` is whatever the underlying LLM accepts (a string for chat + invocations, a list of message dicts for chat models that take that + shape), so a caller can forward the same value to its free-text fallback. + """ + if structured_llm is None: + return None + try: + result = structured_llm.invoke(prompt) + if result is None: + # A thinking model can answer in plain text instead of calling + # the tool, leaving the parser with nothing to return. Treat it + # as a structured miss and fall back, with a clear reason. + raise ValueError("structured output returned no parsed result") + return result + except Exception as exc: + logger.warning( + "%s: structured-output invocation failed (%s); retrying once as free text", + agent_name, exc, + ) + return None + + def invoke_structured_or_freetext( structured_llm: Any | None, plain_llm: Any, @@ -63,27 +88,8 @@ def invoke_structured_or_freetext( render: Callable[[T], str], agent_name: str, ) -> str: - """Run the structured call and render to markdown; fall back to free-text on any failure. - - ``prompt`` is whatever the underlying LLM accepts (a string for chat - invocations, a list of message dicts for chat models that take that - shape). The same value is forwarded to the free-text path so the - fallback sees the same input the structured call did. - """ - if structured_llm is not None: - try: - result = structured_llm.invoke(prompt) - if result is None: - # A thinking model can answer in plain text instead of calling - # the tool, leaving the parser with nothing to return. Treat it - # as a structured miss and fall back, with a clear reason. - raise ValueError("structured output returned no parsed result") - return render(result) - except Exception as exc: - logger.warning( - "%s: structured-output invocation failed (%s); retrying once as free text", - agent_name, exc, - ) - - response = plain_llm.invoke(prompt) - return response.content + """Run the structured call and render to markdown; fall back to free-text on any failure.""" + result = invoke_structured(structured_llm, prompt, agent_name) + if result is not None: + return render(result) + return plain_llm.invoke(prompt).content diff --git a/tradingagents/graph/trading_graph.py b/tradingagents/graph/trading_graph.py index 6717ef397..9fe826977 100644 --- a/tradingagents/graph/trading_graph.py +++ b/tradingagents/graph/trading_graph.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Any from tradingagents.agents.context import build_instrument_context, resolve_instrument_identity -from tradingagents.agents.rating import parse_rating +from tradingagents.agents.rating import run_rating from tradingagents.dataflows.config import run_config, set_config from tradingagents.dataflows.date_window import get_current_date from tradingagents.dataflows.symbols import safe_ticker_component @@ -295,7 +295,8 @@ class TradingAgentsGraph: logger.warning("No final decision for %s on %s; nothing logged", company_name, trade_date) return self.memory_log.store_decision( - ticker=company_name, trade_date=trade_date, final_trade_decision=decision + ticker=company_name, trade_date=trade_date, final_trade_decision=decision, + rating=run_rating(final_state), ) def _run_graph(self, company_name, trade_date, asset_type: str = "stock", @@ -341,7 +342,7 @@ class TradingAgentsGraph: # Clear checkpoint on successful completion to avoid stale state. self.clear_checkpoint_on_success(company_name, trade_date, asset_type, portfolio) - return final_state, self.process_signal(final_state["final_trade_decision"]) + return final_state, run_rating(final_state) def _log_state(self, trade_date, final_state): """Write a run's final state to JSON under the run's own ticker.""" @@ -373,6 +374,7 @@ class TradingAgentsGraph: }, "investment_plan": final_state["investment_plan"], "final_trade_decision": final_state["final_trade_decision"], + "final_rating": run_rating(final_state), } # A ticker that would escape the results directory is rejected. @@ -384,7 +386,3 @@ class TradingAgentsGraph: with open(log_path, "w", encoding="utf-8") as f: # Reports can be in any language and this file is read by a person. json.dump(entry, f, indent=4, ensure_ascii=False) - - def process_signal(self, full_signal): - """The decision's 5-tier rating, or REVIEW when it has none.""" - return parse_rating(full_signal) diff --git a/tradingagents/memory.py b/tradingagents/memory.py index ca5afb47f..1b5ab6039 100644 --- a/tradingagents/memory.py +++ b/tradingagents/memory.py @@ -32,8 +32,13 @@ class TradingMemoryLog: ticker: str, trade_date: str, final_trade_decision: str, + rating: str | None = None, ) -> None: - """Append pending entry at end of propagate(). No LLM call.""" + """Append pending entry at end of propagate(). No LLM call. + + ``rating`` is the decision's own rating when the caller has it; without + one it is read from the decision text. + """ if not self._log_path: return # Idempotency guard: fast raw-text scan instead of full parse. Any entry @@ -45,7 +50,7 @@ class TradingMemoryLog: for line in raw.splitlines(): if line.startswith(f"[{trade_date} | {ticker} |") and line.endswith("]"): return - rating = parse_rating(final_trade_decision) + rating = rating or parse_rating(final_trade_decision) 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: