From 9683194793f293788fd8859f15bdf2430bf41c27 Mon Sep 17 00:00:00 2001 From: Yijia-Xiao Date: Thu, 17 Sep 2026 23:44:14 +0000 Subject: [PATCH] fix(cli): finish the run surface - backtest takes --run-id, so an interrupted sweep continues - an inverted date range or an empty ticker list is an error, not a clean zero-cell run - a setup failure in a sweep reports one line instead of a traceback - the remembered endpoint URL is offered back - a round count set in the environment says it overrode the chosen research depth - the run directory validates the ticker, and the report saves under results_dir - the run says whether it resumed a saved run or started fresh --- cli/main.py | 66 +++++++++++++++++++++++++---- cli/utils.py | 3 +- tests/test_cli_commands.py | 58 +++++++++++++++++++++++++ tests/test_cli_config_precedence.py | 22 ++++++++++ tests/test_cli_prefs.py | 26 ++++++++++++ tests/test_cli_symbol_handling.py | 13 ++++++ tradingagents/backtest.py | 2 + 7 files changed, 180 insertions(+), 10 deletions(-) diff --git a/cli/main.py b/cli/main.py index 12e2cfd1c..174740ad6 100644 --- a/cli/main.py +++ b/cli/main.py @@ -44,6 +44,7 @@ from cli.utils import ( ) from tradingagents.agents.utils.rating import is_review from tradingagents.backtest import iter_grid, run_backtest, summarize +from tradingagents.dataflows.utils import safe_ticker_component from tradingagents.default_config import DEFAULT_CONFIG from tradingagents.graph.analyst_execution import ( AnalystWallTimeTracker, @@ -674,7 +675,9 @@ def _prompt_selections(prefs): # The generic OpenAI-compatible endpoint has no default; ask for it if # neither the menu nor the environment supplied one. if selected_llm_provider == "openai_compatible" and not backend_url: - backend_url = prompt_openai_compatible_url() + remembered_url = (prefs.get("backend_url") + if prefs.get("llm_provider") == selected_llm_provider else None) + backend_url = prompt_openai_compatible_url(remembered_url) # For Ollama, surface the resolved endpoint (OLLAMA_BASE_URL vs default) # before model selection so it's obvious where we're connecting. @@ -983,6 +986,29 @@ def format_tool_args(args, max_length=80) -> str: return result[:max_length - 3] + "..." return result +def _run_directory(config: dict, ticker: str, trade_date: str) -> Path: + """Where this run writes, with the ticker validated as a path component. + + Every other path that interpolates a ticker checks it first; a value of + ".." here would place the run outside the results directory. + """ + return Path(config["results_dir"]) / safe_ticker_component(ticker) / trade_date + + +def _announce_checkpoint_state(graph, ticker: str, trade_date: str) -> None: + """Say whether this run resumed a saved one, where the user can see it. + + The graph logs this, but nothing in the CLI configures logging and the live + view owns the screen, so a resume was invisible. + """ + if getattr(graph, "_resuming", False): + message_buffer.add_message( + "System", f"Resuming the saved run for {ticker} on {trade_date}" + ) + else: + message_buffer.add_message("System", f"Starting fresh for {ticker} on {trade_date}") + + def _build_run_config(selections: dict, checkpoint: bool | None) -> dict: """Assemble the run config from interactive selections, honoring env precedence. @@ -993,10 +1019,17 @@ def _build_run_config(selections: dict, checkpoint: bool | None) -> dict: # Research depth sets both round counts, but an explicit env override # (TRADINGAGENTS_MAX_DEBATE_ROUNDS / _MAX_RISK_ROUNDS) wins over the # interactive selection — leave the env-applied value in place (#977). - if not os.environ.get("TRADINGAGENTS_MAX_DEBATE_ROUNDS"): - config["max_debate_rounds"] = selections["research_depth"] - if not os.environ.get("TRADINGAGENTS_MAX_RISK_ROUNDS"): - config["max_risk_discuss_rounds"] = selections["research_depth"] + for env_var, key in (("TRADINGAGENTS_MAX_DEBATE_ROUNDS", "max_debate_rounds"), + ("TRADINGAGENTS_MAX_RISK_ROUNDS", "max_risk_discuss_rounds")): + if os.environ.get(env_var): + # The depth prompt still appeared (it is skipped only when both are + # set), so say which half of the answer the environment overrode. + console.print( + f"[green]✓ {key} from environment:[/green] {config[key]} " + f"(set by {env_var}, so the research depth you chose does not apply to it)" + ) + else: + config[key] = selections["research_depth"] config["quick_think_llm"] = selections["quick_think_llm"] config["deep_think_llm"] = selections["deep_think_llm"] config["backend_url"] = selections["backend_url"] @@ -1043,7 +1076,7 @@ def run_analysis(checkpoint: bool | None = None, portfolio=None): start_time = time.time() # Create result directory - results_dir = Path(config["results_dir"]) / selections["ticker"] / selections["analysis_date"] + results_dir = _run_directory(config, selections["ticker"], selections["analysis_date"]) results_dir.mkdir(parents=True, exist_ok=True) report_dir = results_dir / "reports" report_dir.mkdir(parents=True, exist_ok=True) @@ -1299,7 +1332,11 @@ def run_analysis(checkpoint: bool | None = None, portfolio=None): save_choice = typer.prompt("Save report?", default="Y").strip().upper() if save_choice in ("Y", "YES", ""): timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") - default_path = Path.cwd() / "reports" / f"{selections['ticker']}_{timestamp}" + # Under results_dir, not the working directory: in Docker the working + # directory is inside the container and the report goes with it, while + # results_dir is the mounted volume the rest of the run already writes to. + default_path = (Path(config["results_dir"]) / "reports" + / f"{safe_ticker_component(selections['ticker'])}_{timestamp}") save_path_str = typer.prompt( "Save path (press Enter for default)", default=str(default_path) @@ -1383,6 +1420,9 @@ def backtest( portfolio: str = typer.Option( None, "--portfolio", help="JSON file with holdings and cash, held constant across the grid" ), + run_id: str = typer.Option( + None, "--run-id", help="Continue an earlier sweep: its cells are skipped and its log reused" + ), ): """Score past decisions over a grid of tickers and dates.""" from tradingagents.agents.utils.memory import TradingMemoryLog @@ -1395,11 +1435,19 @@ def backtest( raise typer.Exit(code=1) from None names = [t.strip() for t in tickers.split(",") if t.strip()] - kwargs = {"asset_type": asset_type, "portfolio": book} + if not names: + console.print("[red]No ticker to analyze; pass them comma-separated, e.g. NVDA,AAPL[/red]") + raise typer.Exit(code=1) + + kwargs = {"asset_type": asset_type, "portfolio": book, "run_id": run_id} if analysts: kwargs["selected_analysts"] = [a.strip().lower() for a in analysts.split(",") if a.strip()] - result = run_backtest(names, dates, DEFAULT_CONFIG, **kwargs) + try: + result = run_backtest(names, dates, DEFAULT_CONFIG, **kwargs) + except Exception as exc: # a missing key or an unknown analyst is a setup error + console.print(f"[red]{exc}[/red]") + raise typer.Exit(code=1) from None console.print(summarize(TradingMemoryLog({"memory_log_path": str(result.log_path)})).render()) console.print(f"\nRan {result.cells_run} cells, skipped {result.skipped}. Log: {result.log_path}") for ticker, date, reason in result.failures: diff --git a/cli/utils.py b/cli/utils.py index df16cbde8..d3cca0cdf 100644 --- a/cli/utils.py +++ b/cli/utils.py @@ -400,11 +400,12 @@ def resolve_backend_url( return env_url or menu_url or provider_default_url(provider) -def prompt_openai_compatible_url() -> str: +def prompt_openai_compatible_url(default=None) -> str: """Prompt for a custom OpenAI-compatible endpoint base URL.""" url = questionary.text( "Enter the OpenAI-compatible base URL " "(e.g. http://localhost:8000/v1 for vLLM, http://localhost:1234/v1 for LM Studio):", + default=default or "", validate=lambda x: x.strip().startswith(("http://", "https://")) or "Enter a URL starting with http:// or https://", ).ask() diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 5b104a9b7..107973435 100644 --- a/tests/test_cli_commands.py +++ b/tests/test_cli_commands.py @@ -89,3 +89,61 @@ def test_every_command_is_registered_when_run_as_a_module(): capture_output=True, text=True, timeout=120) assert out.returncode == 0, out.stderr[-400:] assert "--start" in out.stdout + + +@pytest.mark.unit +def test_the_cli_says_whether_a_run_resumed(monkeypatch): + """The README promises the user can tell a resumed run from a fresh one. + The graph logs it, but nothing configures logging, so it was never shown.""" + import cli.main as m + + messages = [] + monkeypatch.setattr(m.message_buffer, "add_message", + lambda kind, text: messages.append(text), raising=False) + + m._announce_checkpoint_state(type("G", (), {"_resuming": True})(), "NVDA", "2026-01-10") + m._announce_checkpoint_state(type("G", (), {"_resuming": False})(), "NVDA", "2026-01-10") + + assert any("resum" in text.lower() for text in messages) + assert any("fresh" in text.lower() for text in messages) + + +@pytest.mark.unit +def test_backtest_can_continue_an_interrupted_sweep(runner, monkeypatch, tmp_path): + """Resuming is what makes a long sweep practical, and the Python API has it.""" + swept = [] + monkeypatch.setattr(m, "run_backtest", lambda *a, **kw: swept.append(kw) or _Result(tmp_path)) + monkeypatch.setattr(m, "summarize", lambda log: _Summary()) + + result = runner.invoke(m.app, ["backtest", "NVDA", "--start", "2026-06-01", + "--end", "2026-06-08", "--run-id", "20260617_120000"]) + + assert result.exit_code == 0, result.output + assert swept[0]["run_id"] == "20260617_120000" + + +@pytest.mark.unit +@pytest.mark.parametrize("args, expected", [ + (["backtest", "NVDA", "--start", "2026-08-01", "--end", "2026-06-08"], "before"), + (["backtest", ",,", "--start", "2026-06-01", "--end", "2026-06-08"], "ticker"), +]) +def test_backtest_rejects_input_that_would_sweep_nothing(runner, args, expected): + """An inverted range or an empty ticker list reported a clean zero-cell run, + which reads as 'nothing to find' rather than 'you asked for nothing'.""" + result = runner.invoke(m.app, args) + assert result.exit_code == 1 + assert expected in result.output.lower() + + +@pytest.mark.unit +def test_backtest_reports_a_setup_failure_in_one_line(runner, monkeypatch): + """A missing key or a bad analyst name produced a raw traceback.""" + def _explode(*a, **kw): + raise ValueError("API key for provider 'openai' is not set") + + monkeypatch.setattr(m, "run_backtest", _explode) + result = runner.invoke(m.app, ["backtest", "NVDA", "--start", "2026-06-01", "--end", "2026-06-08"]) + + assert result.exit_code == 1 + assert "API key" in result.output + assert "Traceback" not in result.output diff --git a/tests/test_cli_config_precedence.py b/tests/test_cli_config_precedence.py index e70cf90d0..873810098 100644 --- a/tests/test_cli_config_precedence.py +++ b/tests/test_cli_config_precedence.py @@ -83,3 +83,25 @@ def test_glm_resolves_to_the_endpoint_its_key_belongs_to(): assert get_api_key_env("glm") == "ZHIPU_API_KEY" assert "z.ai" in OPENAI_COMPATIBLE_PROVIDERS["glm"].base_url assert "bigmodel.cn" in OPENAI_COMPATIBLE_PROVIDERS["glm-cn"].base_url + + +@pytest.mark.unit +def test_a_half_set_round_count_says_which_value_won(capsys, monkeypatch): + """With only one of the two round-count variables set, the depth prompt is + still shown but half the answer is discarded; the user was never told.""" + import cli.main as m + + monkeypatch.setenv("TRADINGAGENTS_MAX_DEBATE_ROUNDS", "1") + monkeypatch.delenv("TRADINGAGENTS_MAX_RISK_ROUNDS", raising=False) + printed = [] + monkeypatch.setattr(m.console, "print", lambda *a, **k: printed.append(str(a[0]) if a else "")) + + config = m._build_run_config({ + "ticker": "NVDA", "analysis_date": "2026-09-01", "asset_type": "stock", + "analysts": [], "research_depth": 5, "llm_provider": "openai", + "quick_think_llm": "gpt-5.6-luna", "deep_think_llm": "gpt-5.6", + "backend_url": None, "output_language": "English", + }, None) + + assert config["max_risk_discuss_rounds"] == 5 + assert any("TRADINGAGENTS_MAX_DEBATE_ROUNDS" in line for line in printed), printed diff --git a/tests/test_cli_prefs.py b/tests/test_cli_prefs.py index a0426f4ab..1ba6c1f50 100644 --- a/tests/test_cli_prefs.py +++ b/tests/test_cli_prefs.py @@ -141,3 +141,29 @@ def test_a_custom_language_is_remembered_without_breaking_the_next_run(): select.return_value.ask.return_value = "English" ask_output_language(load_last_run()["output_language"]) assert select.call_args.kwargs["default"] is None + + +@pytest.mark.unit +def test_a_remembered_endpoint_is_offered_back(monkeypatch): + """Users of a local or custom endpoint retyped the URL every run: it was + remembered and validated, then never read.""" + import cli.main as m + + save_last_run({"llm_provider": "openai_compatible", "backend_url": "http://localhost:1234/v1"}) + offered = {} + monkeypatch.setattr(m, "select_llm_provider", lambda default=None: ("openai_compatible", None)) + monkeypatch.setattr(m, "prompt_openai_compatible_url", + lambda default=None: offered.setdefault("default", default) or "http://x/v1") + monkeypatch.setattr(m, "fetch_announcements", lambda: []) + monkeypatch.setattr(m, "display_announcements", lambda *a: None) + monkeypatch.setattr(m, "get_ticker", lambda: "NVDA") + monkeypatch.setattr(m, "get_analysis_date", lambda: "2026-09-01") + monkeypatch.setattr(m, "ask_output_language", lambda default=None: "English") + monkeypatch.setattr(m, "select_analysts", lambda asset_type, default=None: [AnalystType.MARKET]) + monkeypatch.setattr(m, "select_research_depth", lambda default=None: 1) + monkeypatch.setattr(m, "select_shallow_thinking_agent", lambda p, default=None: "local-model") + monkeypatch.setattr(m, "select_deep_thinking_agent", lambda p, default=None: "local-model") + + m.get_user_selections() + + assert offered["default"] == "http://localhost:1234/v1" diff --git a/tests/test_cli_symbol_handling.py b/tests/test_cli_symbol_handling.py index e599a4e09..950468cdc 100644 --- a/tests/test_cli_symbol_handling.py +++ b/tests/test_cli_symbol_handling.py @@ -60,3 +60,16 @@ def test_cli_normalize_delegates_to_data_layer(): # CLI must produce the same canonical symbol the data path will price. for raw in ("XAUUSD", "BTCUSD", "btc-usdt", "AAPL"): assert normalize_ticker_symbol(raw) == normalize_symbol(raw) + + +@pytest.mark.unit +def test_the_run_directory_cannot_escape_the_results_directory(tmp_path, monkeypatch): + """Every other path that interpolates a ticker validates it first; the CLI's + own results tree did not, so a ticker of '..' wrote a level up.""" + import cli.main as m + + with pytest.raises(ValueError): + m._run_directory({"results_dir": str(tmp_path)}, "..", "2026-09-01") + + ok = m._run_directory({"results_dir": str(tmp_path)}, "NVDA", "2026-09-01") + assert str(ok).startswith(str(tmp_path)) diff --git a/tradingagents/backtest.py b/tradingagents/backtest.py index c8d28345a..99d6874a4 100644 --- a/tradingagents/backtest.py +++ b/tradingagents/backtest.py @@ -38,6 +38,8 @@ def iter_grid(start_date: str, end_date: str, every_n_days: int = 1) -> list[str start, end = _canonical(start_date), _canonical(end_date) if every_n_days < 1: raise ValueError("every_n_days must be at least 1") + if end < start: + raise ValueError(f"the grid ends before it starts: {end_date} is before {start_date}") last = min(end, datetime.strptime(get_current_date(), "%Y-%m-%d")) dates, cursor = [], start