From bf77737d880d324fdc171c0cc2b5cdd7d12e2d64 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Sun, 26 Jul 2026 23:37:15 +0000 Subject: [PATCH] fix: address pre-push review findings --- backtesting/engine.py | 13 ++++++++++--- dashboard/persistence.py | 9 +++++---- dashboard/server.py | 3 ++- orchestrator.py | 4 +++- tests/test_backtest_cache.py | 2 ++ tests/test_backtest_data_quality.py | 8 ++++++++ tests/test_backtest_statistics.py | 16 ++++++++++++++++ tests/test_reliability_persistence.py | 22 ++++++++++++++++++++++ 8 files changed, 68 insertions(+), 9 deletions(-) diff --git a/backtesting/engine.py b/backtesting/engine.py index 427f9a6..3185c2e 100644 --- a/backtesting/engine.py +++ b/backtesting/engine.py @@ -24,6 +24,7 @@ ML_WEIGHTS_PATH = os.path.join(BASE_DIR, "config", "ml_weights.json") _BACKTEST_CACHE = {} _BACKTEST_CACHE_LOCK = threading.Lock() +_BACKTEST_CACHE_LIMIT = 4 # Score brackets matching the dashboard assessment levels BRACKETS = SCORE_BRACKETS @@ -422,7 +423,12 @@ def clear_backtest_cache(): def _add_return_statistics(stats, period, returns): """Add return summaries and a moving-block-bootstrap mean interval.""" - summary = summarize_returns(returns, block_size=min(30, len(returns)), n_resamples=400) + horizon_days = int(period.removesuffix("d")) + summary = summarize_returns( + returns, + block_size=min(horizon_days, len(returns)), + n_resamples=400, + ) stats[f"avg_{period}"] = summary["mean"] stats[f"median_{period}"] = summary["median"] stats[f"win_rate_{period}"] = summary["win_rate"] @@ -449,8 +455,9 @@ def run_backtest(ml_mode=False): result = _compute_backtest(ml_mode=ml_mode) with _BACKTEST_CACHE_LOCK: - _BACKTEST_CACHE.clear() _BACKTEST_CACHE[signature] = copy.deepcopy(result) + while len(_BACKTEST_CACHE) > _BACKTEST_CACHE_LIMIT: + _BACKTEST_CACHE.pop(next(iter(_BACKTEST_CACHE))) return copy.deepcopy(result) @@ -720,7 +727,7 @@ def _compute_backtest(ml_mode=False): # Include per-metric values (raw metric value, not score) metric_vals = d.get("metric_values", {}) if metric_vals: - entry["metrics"] = metric_vals + entry["metric_values"] = metric_vals chart_data.append(entry) if not ml_mode: diff --git a/dashboard/persistence.py b/dashboard/persistence.py index 58b2972..adf4c5e 100644 --- a/dashboard/persistence.py +++ b/dashboard/persistence.py @@ -19,7 +19,7 @@ except ImportError: # pragma: no cover - Windows fallback uses the process lock _LOCKS: dict[str, threading.RLock] = {} _LOCKS_GUARD = threading.Lock() -_METADATA_KEYS = {"observed_at", "source", "stale", "last_error"} +_METADATA_KEYS = {"observed_at", "source", "stale", "last_error", "error"} def _thread_lock(path: Path) -> threading.RLock: @@ -165,7 +165,7 @@ def append_daily_jsonl(path: str | os.PathLike[str], entry: dict[str, Any]) -> b return True -def _has_observation(payload: Any) -> bool: +def has_observation(payload: Any) -> bool: if not isinstance(payload, dict): return payload is not None return any(value is not None for key, value in payload.items() if key not in _METADATA_KEYS) @@ -180,7 +180,7 @@ def merge_observation( error: str | None = None, ) -> dict[str, Any]: """Annotate a fresh observation or retain the last-known-good value as stale.""" - if _has_observation(observed): + if has_observation(observed): merged = dict(observed) if isinstance(observed, dict) else {"value": observed} merged.update( observed_at=observed_at or datetime.now(timezone.utc).isoformat(), @@ -191,10 +191,11 @@ def merge_observation( return merged merged = dict(previous) if isinstance(previous, dict) else {} + observed_error = observed.get("error") if isinstance(observed, dict) else None merged.update( source=merged.get("source") or source, stale=True, - last_error=error or "metric was not observed", + last_error=observed_error or error or "metric was not observed", ) merged.setdefault("observed_at", None) return merged diff --git a/dashboard/server.py b/dashboard/server.py index 46644e2..69b6286 100644 --- a/dashboard/server.py +++ b/dashboard/server.py @@ -32,6 +32,7 @@ from scoring import engine from dashboard.persistence import ( append_daily_jsonl, atomic_write_json, + has_observation, load_json, load_jsonl_tail, merge_observation, @@ -293,7 +294,7 @@ def run_scrape(force_full=False): existing_cache.get(key), onchain.get(key), source=source, error="metric missing from scrape", ) - if successful_sources: + if successful_sources and any(has_observation(value) for value in onchain.values()): metrics["_onchain_timestamp"] = datetime.now(timezone.utc).isoformat() elif "_onchain_timestamp" in existing_cache: metrics["_onchain_timestamp"] = existing_cache["_onchain_timestamp"] diff --git a/orchestrator.py b/orchestrator.py index 7c6219d..c4347f3 100755 --- a/orchestrator.py +++ b/orchestrator.py @@ -413,7 +413,7 @@ def run_optimization_loop(callback=None, config_override=None): with open(results_local) as f: results = json.load(f) - current_score = results.get("cost_basis_improvement_pct", 0) + current_score = objective_score(results) signal_count = results.get("strong_buy_signal_count", 0) is_best = current_score > best_score and signal_count >= MIN_SIGNAL_COUNT @@ -427,6 +427,8 @@ def run_optimization_loop(callback=None, config_override=None): "iteration": iteration, "timestamp": datetime.now(timezone.utc).isoformat(), "cost_improvement": current_score, + "objective_improvement": current_score, + "objective": "equal_periodic_contribution_terminal_wealth", "signal_count": signal_count, "signal_frequency": results.get("signal_frequency_pct", 0), "r2_score": results.get("model_r2_score", 0), diff --git a/tests/test_backtest_cache.py b/tests/test_backtest_cache.py index 5e4294e..218dbae 100644 --- a/tests/test_backtest_cache.py +++ b/tests/test_backtest_cache.py @@ -27,9 +27,11 @@ def test_run_backtest_caches_by_input_file_signature(monkeypatch, tmp_path): second = engine.run_backtest() ml_first = engine.run_backtest(ml_mode=True) ml_second = engine.run_backtest(ml_mode=True) + classic_after_ml = engine.run_backtest() assert first == second == {"ml_mode": False, "calls": 1} assert ml_first == ml_second == {"ml_mode": True, "calls": 2} + assert classic_after_ml == first assert calls == [False, True] history.write_text('{"changed": true}') diff --git a/tests/test_backtest_data_quality.py b/tests/test_backtest_data_quality.py index 0383e49..4a0b6a3 100644 --- a/tests/test_backtest_data_quality.py +++ b/tests/test_backtest_data_quality.py @@ -51,3 +51,11 @@ def test_live_and_backtest_outputs_publish_panel_and_coverage_metadata(): "panel_count": 9, } assert metadata["staleness_days"] == backtest.METRIC_MAX_AGE_DAYS + + +def test_chart_data_exposes_metric_values_under_frontend_contract(): + result = backtest.run_backtest() + entries = [entry for entry in result["chart_data"] if entry.get("metric_values")] + + assert entries + assert all("metrics" not in entry for entry in entries) diff --git a/tests/test_backtest_statistics.py b/tests/test_backtest_statistics.py index 22d15c2..4cad8a9 100644 --- a/tests/test_backtest_statistics.py +++ b/tests/test_backtest_statistics.py @@ -43,3 +43,19 @@ def test_backtest_brackets_publish_bootstrap_confidence_intervals(): assert stats["median_90d"] == 2.5 assert stats["win_rate_90d"] == 50.0 assert stats["avg_90d_ci_low"] <= stats["avg_90d"] <= stats["avg_90d_ci_high"] + + +def test_long_horizon_returns_use_a_matching_dependence_block(monkeypatch): + observed = {} + + def fake_summary(values, *, block_size, n_resamples): + observed.update(block_size=block_size, n_resamples=n_resamples) + return { + "mean": 1.0, "median": 1.0, "win_rate": 100.0, + "mean_ci_low": 0.5, "mean_ci_high": 1.5, "n": len(values), + } + + monkeypatch.setattr(engine, "summarize_returns", fake_summary) + engine._add_return_statistics({}, "365d", [1.0] * 500) + + assert observed == {"block_size": 365, "n_resamples": 400} diff --git a/tests/test_reliability_persistence.py b/tests/test_reliability_persistence.py index 24b334a..7d0ad31 100644 --- a/tests/test_reliability_persistence.py +++ b/tests/test_reliability_persistence.py @@ -87,6 +87,28 @@ def test_merge_observation_records_metadata_for_fresh_value(): assert merged["last_error"] is None +def test_merge_observation_rejects_error_only_payload_as_fresh_data(): + old = { + "value": 1.25, + "observed_at": "2026-07-25T12:00:00+00:00", + "source": "lookintobitcoin", + "stale": False, + "last_error": None, + } + + merged = merge_observation( + old, + {"value": None, "error": "No data returned"}, + source="lookintobitcoin", + error="metric missing from scrape", + ) + + assert merged["value"] == 1.25 + assert merged["observed_at"] == old["observed_at"] + assert merged["stale"] is True + assert merged["last_error"] == "No data returned" + + @pytest.mark.parametrize("timestamp", [None, "", "not-a-time"]) def test_onchain_refresh_due_when_timestamp_is_missing_or_invalid(timestamp): assert onchain_refresh_due(timestamp, now=datetime(2026, 7, 26, tzinfo=timezone.utc)) is True