fix: address pre-push review findings
This commit is contained in:
+10
-3
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
+2
-1
@@ -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"]
|
||||
|
||||
+3
-1
@@ -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),
|
||||
|
||||
@@ -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}')
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user