diff --git a/rdagent/components/backtesting/backtest_engine.py b/rdagent/components/backtesting/backtest_engine.py index bc149dfd..69c661f7 100644 --- a/rdagent/components/backtesting/backtest_engine.py +++ b/rdagent/components/backtesting/backtest_engine.py @@ -72,7 +72,7 @@ class BacktestMetrics: class FactorBacktester: def __init__(self): self.metrics = BacktestMetrics() - self.results_path = Path(__file__).parent.parent.parent / "results" / "backtests" + self.results_path = Path(__file__).parent.parent.parent.parent / "results" / "backtests" self.results_path.mkdir(parents=True, exist_ok=True) def run_backtest( diff --git a/rdagent/components/backtesting/results_db.py b/rdagent/components/backtesting/results_db.py index b745c299..e727c3b6 100644 --- a/rdagent/components/backtesting/results_db.py +++ b/rdagent/components/backtesting/results_db.py @@ -330,13 +330,13 @@ class ResultsDatabase: worst_drawdown = all_results['max_drawdown'].min() if total_runs > 0 and all_results['max_drawdown'].notna().any() else None # Scan factors directory for JSON files - factors_dir = Path(__file__).parent.parent.parent / "results" / "factors" + factors_dir = Path(__file__).parent.parent.parent.parent / "results" / "factors" json_factor_files = 0 if factors_dir.exists(): json_factor_files = len(list(factors_dir.glob("*.json"))) # Scan failed runs - failed_dir = Path(__file__).parent.parent.parent / "results" / "failed_runs" + failed_dir = Path(__file__).parent.parent.parent.parent / "results" / "failed_runs" failed_runs_file = failed_dir / "failed_runs.json" failed_runs_count = 0 failed_runs_data = [] diff --git a/rdagent/scenarios/qlib/developer/factor_runner.py b/rdagent/scenarios/qlib/developer/factor_runner.py index d544c134..e5eba5d0 100644 --- a/rdagent/scenarios/qlib/developer/factor_runner.py +++ b/rdagent/scenarios/qlib/developer/factor_runner.py @@ -143,6 +143,8 @@ class QlibFactorRunner(CachedRunner[QlibFactorExperiment]): ) IC_max.index = pd.MultiIndex.from_product([range(SOTA_feature.shape[1]), range(new_feature.shape[1])]) IC_max = IC_max.unstack().max(axis=0) + if not hasattr(IC_max, "index"): + return new_feature return new_feature.iloc[:, IC_max[IC_max < 0.99].index] def develop(self, exp: QlibFactorExperiment) -> QlibFactorExperiment: @@ -757,19 +759,19 @@ class QlibFactorRunner(CachedRunner[QlibFactorExperiment]): db_file = db_path / "backtest_results.db" # Parallel run isolation: use run-specific subdirectory if PARALLEL_RUN_ID is set - run_id = os.getenv("PARALLEL_RUN_ID", "0") - if run_id != "0": + parallel_run_id = os.getenv("PARALLEL_RUN_ID", "0") + if parallel_run_id != "0": # For parallel runs, save to isolated results directory - isolated_db_path = project_root / "results" / "runs" / f"run{run_id}" / "db" + isolated_db_path = project_root / "results" / "runs" / f"run{parallel_run_id}" / "db" isolated_db_path.mkdir(parents=True, exist_ok=True) db_file = isolated_db_path / "backtest_results.db" # Save to database db = ResultsDatabase(db_path=str(db_file)) - run_id = db.add_backtest(factor_name=factor_name[:100], metrics=metrics) + db_run_id = db.add_backtest(factor_name=factor_name[:100], metrics=metrics) logger.info( f"Factor result saved to DB: {factor_name[:60]} " - f"(IC={metrics.get('ic')}, Sharpe={metrics.get('sharpe_ratio')}, run_id={run_id})", + f"(IC={metrics.get('ic')}, Sharpe={metrics.get('sharpe_ratio')}, run_id={db_run_id})" ) # Extract factor code and description from experiment @@ -777,7 +779,7 @@ class QlibFactorRunner(CachedRunner[QlibFactorExperiment]): # Also write a JSON summary to results/factors/ for file-based access self._save_factor_json( - factor_name, metrics, run_id, + factor_name, metrics, db_run_id, factor_code=factor_code, factor_description=factor_description, exp=exp, @@ -963,12 +965,17 @@ class QlibFactorRunner(CachedRunner[QlibFactorExperiment]): shutil.copy(str(full_data), str(tmp / "intraday_pv.h5")) ret = subprocess.run( - ["sys.executable", "factor.py"], + [sys.executable, "factor.py"], cwd=str(tmp), capture_output=True, timeout=300, + check=False, ) if ret.returncode != 0: + logger.warning( + f"Full-data factor run failed (exit {ret.returncode}): " + f"{ret.stderr[:500] if ret.stderr else '(no stderr)'}" + ) # Fall back to debug-data result if full-data run fails result_h5 = workspace_path / "result.h5" if not result_h5.exists(): diff --git a/rdagent/scenarios/qlib/developer/model_runner.py b/rdagent/scenarios/qlib/developer/model_runner.py index 61ddafc2..44a787ef 100644 --- a/rdagent/scenarios/qlib/developer/model_runner.py +++ b/rdagent/scenarios/qlib/developer/model_runner.py @@ -203,12 +203,14 @@ class QlibModelRunner(CachedRunner[QlibModelExperiment]): # Save to database db = ResultsDatabase() - run_id = db.add_backtest(factor_name=factor_name[:100], metrics=metrics) - logger.info( - f"Model result saved to DB: {factor_name[:50]} " - f"(IC={metrics.get('ic')}, Sharpe={metrics.get('sharpe_ratio')}, run_id={run_id})" - ) - db.close() + try: + run_id = db.add_backtest(factor_name=factor_name[:100], metrics=metrics) + logger.info( + f"Model result saved to DB: {factor_name[:50]} " + f"(IC={metrics.get('ic')}, Sharpe={metrics.get('sharpe_ratio')}, run_id={run_id})" + ) + finally: + db.close() except Exception as e: logger.warning(f"Database save failed for model {getattr(exp.hypothesis, 'hypothesis', 'unknown')}: {e}") diff --git a/rdagent/utils/env.py b/rdagent/utils/env.py index 92650bcd..981408cb 100644 --- a/rdagent/utils/env.py +++ b/rdagent/utils/env.py @@ -923,7 +923,11 @@ def _prepare_conda_env(env_name: str, requirements_file: Path, python_version: s """ # 1. Create conda environment if not exists env_list = subprocess.run(["conda", "env", "list"], capture_output=True, text=True, check=False) - env_exists = any(line.split()[0] == env_name for line in env_list.stdout.splitlines() if line and not line.startswith("#")) + env_exists = any( + line.split()[0] == env_name + for line in env_list.stdout.splitlines() + if line and not line.startswith("#") and len(line.split()) > 0 + ) if not env_exists: print(f"[yellow]Creating conda env '{env_name}' (Python {python_version})...[/yellow]") subprocess.check_call(["conda", "create", "-y", "-n", env_name, f"python={python_version}"]) @@ -1189,7 +1193,7 @@ class DockerEnv(Env[DockerConf]): with Progress(SpinnerColumn(), TextColumn("{task.description}")) as p: task = p.add_task("[cyan]Building image...") for part in resp_stream: - lines = part.decode("utf-8").split("\r\n") + lines = part.decode("utf-8", errors="replace").split("\r\n") for line in lines: if line.strip(): status_dict = json.loads(line) diff --git a/scripts/predix_parallel.py b/scripts/predix_parallel.py index ec3e0fc0..2147c944 100644 --- a/scripts/predix_parallel.py +++ b/scripts/predix_parallel.py @@ -230,16 +230,20 @@ class ParallelRunner: log_path = self.project_root / run_state.log_file log_f = open(log_path, "a", encoding="utf-8") - # Start subprocess - run_state.process = subprocess.Popen( - cmd, - env=env, - cwd=str(self.project_root), - stdout=log_f, - stderr=subprocess.STDOUT, - ) - run_state.status = "running" - run_state.start_time = datetime.now() + try: + # Start subprocess + run_state.process = subprocess.Popen( + cmd, + env=env, + cwd=str(self.project_root), + stdout=log_f, + stderr=subprocess.STDOUT, + ) + run_state.status = "running" + run_state.start_time = datetime.now() + except Exception: + log_f.close() + raise console.print( f"[dim] ▶️ Run {run_state.run_id} started (PID: {run_state.process.pid}, " diff --git a/scripts/predix_rebacktest_strategies.py b/scripts/predix_rebacktest_strategies.py index aced3f01..ac9e91e3 100644 --- a/scripts/predix_rebacktest_strategies.py +++ b/scripts/predix_rebacktest_strategies.py @@ -16,7 +16,8 @@ def load_factors(names, vdir): if df is not None and len(df.columns) > 0: dfs[n] = df.iloc[:, 0] break - except: pass + except Exception: + pass return dfs def fix_code(code, available): @@ -79,7 +80,7 @@ except Exception as e: if r.returncode != 0: return None sig = pd.read_pickle(str(tdp / "s.pkl")) - except: + except Exception: return None fwd = df.mean(axis=1).shift(-96).dropna() @@ -124,7 +125,7 @@ def main(count=None): d = json.load(open(f)) if isinstance(d, dict) and 'strategy_name' in d: files.append(f) - except: pass + except Exception: pass if count: files = files[:count] print(f"Re-evaluating {len(files)} strategies...\n") @@ -147,7 +148,7 @@ def main(count=None): with open(f, 'w') as out: json.dump(data, out, indent=2, ensure_ascii=False) updated += 1 results.append({'name':data['strategy_name'], **bt}) - except: + except Exception: pass p.update(task, advance=1) diff --git a/test/local/test_bug_fixes.py b/test/local/test_bug_fixes.py new file mode 100644 index 00000000..732f550e --- /dev/null +++ b/test/local/test_bug_fixes.py @@ -0,0 +1,281 @@ +""" +Tests for bug fixes in strategy_orchestrator, factor_runner, backtest_engine, +results_db, model_runner, optuna_optimizer, env, and related modules. + +Verifies: +- strategy_orchestrator.py compiles (IndentationError was fixed) +- factor_runner.py uses sys.executable (variable), not literal string +- backtest_engine.py path depth is 4 (not 3) .parent hops +- results_db.py path depth for factors/failed dirs is 4 (not 3) +- model_runner.py DB connection closed via try/finally +- factor_runner.py variable shadowing eliminated (run_id vs db_run_id) +- optuna_optimizer.py no longer shadows imported logger +- env.py Docker build output handles non-UTF-8 bytes +- env.py conda env list parsing guards against empty lines +- strategy_orchestrator.py exec() exception logged at ERROR level +- strategy_orchestrator.py template validation warns on unreplaced {{...}} +- factor_runner.py IC_max guard against scalar (AttributeError) +- predix_parallel.py handle leak on Popen failure +- predix_rebacktest_strategies.py bare except replaced with except Exception +""" + +import ast +import inspect +import os +import sys +from pathlib import Path +from unittest.mock import patch + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.parent + + +# ── Fix 1: strategy_orchestrator.py IndentationError ────────────────────── + + +class TestStrategyOrchestratorSyntax: + def test_file_compiles(self): + """Bug: IndentationError at line 764 prevented the entire file from importing.""" + import py_compile + py_compile.compile( + str(REPO_ROOT / "rdagent/components/coder/strategy_orchestrator.py"), + doraise=True, + ) + + def test_module_imports(self): + """Verify StrategyOrchestrator can be imported after syntax fix.""" + from rdagent.components.coder.strategy_orchestrator import StrategyOrchestrator + assert StrategyOrchestrator is not None + + +# ── Fix 2: factor_runner.py literal "sys.executable" ────────────────────── + + +class TestFactorRunnerSysExecutable: + def test_not_literal_string(self): + """Bug: ["sys.executable", ...] was a literal string, not the variable.""" + source = (REPO_ROOT / "rdagent/scenarios/qlib/developer/factor_runner.py").read_text() + # The fix should NOT contain the quoted literal 'sys.executable' + assert '"sys.executable"' not in source, ( + "factor_runner.py still contains literal string 'sys.executable' — " + "should be sys.executable (variable)" + ) + # Should use sys.executable (variable, part of a list) + assert "sys.executable" in source + + def test_subprocess_run_with_check_false(self): + """Bug: subprocess.run without explicit check=False.""" + source = (REPO_ROOT / "rdagent/scenarios/qlib/developer/factor_runner.py").read_text() + # The fix added check=False to the full-data factor run + assert 'check=False' in source, ( + "subprocess.run should have explicit check=False for full-data factor run" + ) + + +# ── Fix 3: backtest_engine.py path depth ───────────────────────────────── + + +class TestBacktestEnginePathDepth: + def test_path_depth_is_4(self): + """Bug: 3 .parent hops ended at rdagent/ instead of repo root.""" + from rdagent.components.backtesting.backtest_engine import FactorBacktester + + fb = FactorBacktester() + results = fb.results_path + + # results_path should be under the repo root, not under rdagent/ + assert REPO_ROOT in results.parents or results.parent == REPO_ROOT / "results", ( + f"results_path={results} is not under repo root {REPO_ROOT}. " + "Path depth may still be wrong." + ) + # The path should NOT be inside rdagent/ + assert "rdagent/results" not in str(results).replace(str(REPO_ROOT), ""), ( + f"results_path={results} appears to be inside rdagent/ directory" + ) + + +# ── Fix 4: results_db.py path depth ────────────────────────────────────── + + +class TestResultsDBPathDepth: + def test_factors_dir_depth(self): + """Bug: 3 .parent hops from results_db.py ended at rdagent/.""" + from rdagent.components.backtesting.results_db import ResultsDatabase + source = inspect.getsource(ResultsDatabase.generate_results_summary) + + # After fix, should use .parent.parent.parent.parent (4 hops) + assert ".parent.parent.parent.parent" in source, ( + "ResultsDatabase.generate_results_summary should use 4 .parent hops, not 3" + ) + + +# ── Fix 5: model_runner.py DB close ────────────────────────────────────── + + +class TestModelRunnerDBClose: + def test_try_finally_for_db_close(self): + """Bug: db.close() was after add_backtest, not in finally block.""" + source = (REPO_ROOT / "rdagent/scenarios/qlib/developer/model_runner.py").read_text() + + # After fix, db.close() should be in a finally block or try/finally context + assert "finally:" in source, ( + "model_runner.py should use try/finally to close DB connection" + ) + assert "db.close()" in source + + +# ── Fix 6: factor_runner.py variable shadowing ─────────────────────────── + + +class TestFactorRunnerShadowing: + def test_no_run_id_shadowing(self): + """Bug: run_id was reassigned from parallel run ID to DB row ID.""" + source = (REPO_ROOT / "rdagent/scenarios/qlib/developer/factor_runner.py").read_text() + + # After fix, parallel_run_id and db_run_id are separate variables + assert "parallel_run_id" in source, ( + "factor_runner.py should use parallel_run_id for parallel run isolation" + ) + assert "db_run_id" in source, ( + "factor_runner.py should use db_run_id for DB row ID" + ) + + +# ── Fix 7: optuna_optimizer.py logger shadowing ────────────────────────── + + +class TestOptunaLoggerShadowing: + def test_logger_not_reassigned(self): + """Bug: logger was reassigned from rdagent_logger to raw logging.getLogger.""" + source = (REPO_ROOT / "rdagent/components/coder/optuna_optimizer.py").read_text() + + # After fix, the module-level logger should be rdagent_logger + assert "from rdagent.log import rdagent_logger as logger" in source + # The second assignment should use a different name + assert "_optuna_logger" in source, ( + "optuna_optimizer.py should not shadow the rdagent logger" + ) + + +# ── Fix 8: env.py UnicodeDecodeError ───────────────────────────────────── + + +class TestEnvUnicodeDecode: + def test_decode_with_errors_replace(self): + """Bug: part.decode('utf-8') could raise UnicodeDecodeError.""" + source = (REPO_ROOT / "rdagent/utils/env.py").read_text() + + # After fix, decode uses errors="replace" + assert 'decode("utf-8", errors="replace")' in source, ( + "env.py should handle non-UTF-8 Docker build output with errors='replace'" + ) + + +# ── Fix 9: env.py conda env list parsing ───────────────────────────────── + + +class TestEnvCondaParsing: + def test_guard_against_empty_lines(self): + """Bug: line.split()[0] crashed on empty tokens.""" + source = (REPO_ROOT / "rdagent/utils/env.py").read_text() + + # After fix, guards against empty split results + assert "len(line.split()) > 0" in source, ( + "env.py should guard against empty split results in conda env list parsing" + ) + + +# ── Fix 10: strategy_orchestrator.py exec() logging ────────────────────── + + +class TestStrategyOrchExecLogging: + def test_error_logging_in_exec_handler(self): + """Bug: exec() exception was silently swallowed.""" + source = (REPO_ROOT / "rdagent/components/coder/strategy_orchestrator.py").read_text() + + # After fix, logger.error is called inside the except block + assert "logger.error" in source, ( + "strategy_orchestrator.py should log exec() errors at ERROR level" + ) + + +# ── Fix 11: strategy_orchestrator.py template validation ───────────────── + + +class TestStrategyOrchTemplateValidation: + def test_unreplaced_template_warning(self): + """Bug: no validation that {{...}} placeholders were replaced.""" + source = (REPO_ROOT / "rdagent/components/coder/strategy_orchestrator.py").read_text() + + # After fix, warns on unreplaced template variables + assert "Unreplaced template variables" in source, ( + "strategy_orchestrator.py should warn on unreplaced {{...}} placeholders" + ) + + +# ── Fix 12: factor_runner.py IC_max guard ──────────────────────────────── + + +class TestFactorRunnerICMaxGuard: + def test_hasattr_guard(self): + """Bug: IC_max[...].index failed with AttributeError on scalar result.""" + source = (REPO_ROOT / "rdagent/scenarios/qlib/developer/factor_runner.py").read_text() + + # After fix, guards against scalar IC_max result + assert 'hasattr(IC_max, "index")' in source, ( + "factor_runner.py should guard IC_max.index access with hasattr" + ) + + +# ── Fix 13: predix_parallel.py handle leak ─────────────────────────────── + + +class TestParallelRunnerHandleLeak: + def test_log_f_close_on_popen_failure(self): + """Bug: open() file handle leaked if Popen failed.""" + source = (REPO_ROOT / "scripts/predix_parallel.py").read_text() + + # After fix, log_f.close() is called before re-raise + assert "log_f.close()" in source, ( + "predix_parallel.py should close log file handle on Popen failure" + ) + + +# ── Fix 14: predix_rebacktest_strategies.py bare except ────────────────── + + +class TestRebacktestBareExcept: + def test_not_bare_except(self): + """Bug: bare except: pass swallowed all errors including SystemExit.""" + source = (REPO_ROOT / "scripts/predix_rebacktest_strategies.py").read_text() + + # After fix, should use except Exception, not bare except + assert "except Exception:" in source + assert "except:" not in source, ( + "predix_rebacktest_strategies.py should not use bare except:" + ) + + +# ── Integration: import checks ─────────────────────────────────────────── + + +class TestAllImportsDontCrash: + def test_strategy_orchestrator_imports(self): + """Verify all fixed modules import without errors.""" + import rdagent.components.coder.strategy_orchestrator # noqa: F401 + + def test_factor_runner_imports(self): + import rdagent.scenarios.qlib.developer.factor_runner # noqa: F401 + + def test_backtest_engine_imports(self): + import rdagent.components.backtesting.backtest_engine # noqa: F401 + + def test_results_db_imports(self): + import rdagent.components.backtesting.results_db # noqa: F401 + + def test_model_runner_imports(self): + import rdagent.scenarios.qlib.developer.model_runner # noqa: F401 + + def test_optuna_optimizer_imports(self): + import rdagent.components.coder.optuna_optimizer # noqa: F401