fix(backtest): quote one book, not two
The recommendation's "Book vs SPY" line read from portfolio_sim — the hold/target policy book — while the tiles directly above read from portfolio_monitor, the production ATR-trail book. Same SPY figure, different portfolio return, on one screen. It now reads the same production row the tiles do. Also removed, for the same reason: the "legacy exit diagnostic" comparing hold against the S/R target. Both are exits the production book replaced, so a recommendation between them could not lead to an action. And the fallback headline, which advised the fixed-hold exit whenever a report had no production row — a report that cannot describe the production baseline now states none. portfolio_sim stays in the report payload: scripts/run_backtest_snapshot.py and reports/compare_reports.py read it, and it is no longer surfaced in the UI. The test fixture now carries a production monitor whose numbers differ from its policy sim, so re-sourcing that line from the old place fails rather than passes unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3946,38 +3946,12 @@ def _build_recommendation(report: dict) -> dict:
|
|||||||
q = report.get("overall_qualified") or {}
|
q = report.get("overall_qualified") or {}
|
||||||
target_net = q.get("net_avg_r")
|
target_net = q.get("net_avg_r")
|
||||||
|
|
||||||
# Legacy diagnostic: target/stop race vs the best fixed hold.
|
# The best fixed hold, kept ONLY to pick the basis for the robustness check
|
||||||
|
# below. The hold-vs-target comparison itself is deliberately not reported:
|
||||||
|
# both are legacy exits the production book replaced with the ATR trail, so
|
||||||
|
# a recommendation between them could not lead to an action.
|
||||||
time_rows = [r for r in report.get("time_exit_sweep") or [] if r.get("net_avg_r") is not None]
|
time_rows = [r for r in report.get("time_exit_sweep") or [] if r.get("net_avg_r") is not None]
|
||||||
best_hold = max(time_rows, key=lambda r: r["net_avg_r"], default=None)
|
best_hold = max(time_rows, key=lambda r: r["net_avg_r"], default=None)
|
||||||
sim_rows = {
|
|
||||||
p.get("policy"): p
|
|
||||||
for p in (report.get("portfolio_sim") or {}).get("policies", [])
|
|
||||||
}
|
|
||||||
hold_sim = sim_rows.get("hold")
|
|
||||||
if best_hold is not None and target_net is not None:
|
|
||||||
if best_hold["net_avg_r"] > target_net + _EXIT_SWITCH_THRESHOLD:
|
|
||||||
text = (
|
|
||||||
f"Legacy exit diagnostic: hold {best_hold['hold_days']} trading days with the initial stop "
|
|
||||||
f"({best_hold['net_avg_r']:+.2f}R net/trade vs {target_net:+.2f}R for the S/R target exit)."
|
|
||||||
)
|
|
||||||
target_sim = sim_rows.get("target")
|
|
||||||
if (
|
|
||||||
hold_sim is not None and target_sim is not None
|
|
||||||
and hold_sim.get("cagr_pct") is not None and target_sim.get("cagr_pct") is not None
|
|
||||||
):
|
|
||||||
text += (
|
|
||||||
f" The simulated book agrees: {hold_sim['cagr_pct']:+.1f}% vs "
|
|
||||||
f"{target_sim['cagr_pct']:+.1f}% CAGR at similar drawdown."
|
|
||||||
)
|
|
||||||
items.append({"topic": "exit", "text": text})
|
|
||||||
else:
|
|
||||||
items.append({
|
|
||||||
"topic": "exit",
|
|
||||||
"text": (
|
|
||||||
f"Legacy exit diagnostic: keep the S/R target exit ({target_net:+.2f}R net/trade) — "
|
|
||||||
"no fixed hold beats it by a meaningful margin."
|
|
||||||
),
|
|
||||||
})
|
|
||||||
|
|
||||||
# Gate floors, judged under the hold exit (the ablation's Hold column).
|
# Gate floors, judged under the hold exit (the ablation's Hold column).
|
||||||
ablation = {r["variant"]: r for r in report.get("gate_ablation") or []}
|
ablation = {r["variant"]: r for r in report.get("gate_ablation") or []}
|
||||||
@@ -4025,17 +3999,20 @@ def _build_recommendation(report: dict) -> dict:
|
|||||||
),
|
),
|
||||||
})
|
})
|
||||||
|
|
||||||
# Book vs benchmark.
|
# Book vs benchmark — read from the SAME production monitor row the page
|
||||||
book = hold_sim or sim_rows.get("target")
|
# shows in its tiles. It used to read the hold/target policy sim, so the
|
||||||
if book is not None and book.get("spy_return_pct") is not None:
|
# recommendation quoted a different portfolio return than the tile directly
|
||||||
edge = book["total_return_pct"] - book["spy_return_pct"]
|
# above it, against an identical SPY figure. Those policies are legacy
|
||||||
|
# diagnostics; the production book is the ATR trail.
|
||||||
|
if production_row is not None and production_row.get("spy_return_pct") is not None:
|
||||||
|
edge = production_row["total_return_pct"] - production_row["spy_return_pct"]
|
||||||
verdict = "beats" if edge > 0 else "LAGS"
|
verdict = "beats" if edge > 0 else "LAGS"
|
||||||
items.append({
|
items.append({
|
||||||
"topic": "benchmark",
|
"topic": "benchmark",
|
||||||
"text": (
|
"text": (
|
||||||
f"Book vs SPY: {verdict} buy-and-hold by {edge:+.1f} points "
|
f"Book vs SPY: {verdict} buy-and-hold by {edge:+.1f} points "
|
||||||
f"({book['total_return_pct']:+.1f}% vs {book['spy_return_pct']:+.1f}%), "
|
f"({production_row['total_return_pct']:+.1f}% vs "
|
||||||
f"max drawdown −{book['max_drawdown_pct']:.1f}%."
|
f"{production_row['spy_return_pct']:+.1f}%)."
|
||||||
),
|
),
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -4072,16 +4049,10 @@ def _build_recommendation(report: dict) -> dict:
|
|||||||
),
|
),
|
||||||
})
|
})
|
||||||
|
|
||||||
if headline is None and hold_recommended:
|
# No fallback headline. It used to recommend the fixed-hold exit whenever the
|
||||||
cagr_note = (
|
# portfolio monitor was missing, which meant a report without a production
|
||||||
f" (~{hold_sim['cagr_pct']:.0f}% CAGR simulated)"
|
# row advised an exit the production book had already replaced. A report that
|
||||||
if hold_sim is not None and hold_sim.get("cagr_pct") is not None
|
# cannot describe the production baseline states no baseline.
|
||||||
else ""
|
|
||||||
)
|
|
||||||
headline = (
|
|
||||||
f"Trade the qualified list long-only; hold {best_hold['hold_days']} trading days "
|
|
||||||
f"with the initial ATR stop{cagr_note}."
|
|
||||||
)
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"headline": headline,
|
"headline": headline,
|
||||||
|
|||||||
@@ -1272,20 +1272,36 @@ def test_build_recommendation_reads_the_report():
|
|||||||
{"min_momentum_percentile": 60.0, "net_avg_r": 0.05, "total": 300},
|
{"min_momentum_percentile": 60.0, "net_avg_r": 0.05, "total": 300},
|
||||||
{"min_momentum_percentile": 0.0, "net_avg_r": -0.12, "total": 1000},
|
{"min_momentum_percentile": 0.0, "net_avg_r": -0.12, "total": 1000},
|
||||||
],
|
],
|
||||||
|
# Legacy policy book. Its numbers are deliberately DIFFERENT from the
|
||||||
|
# production monitor's below, so sourcing the benchmark line from here
|
||||||
|
# again would fail the assertion rather than pass unnoticed.
|
||||||
"portfolio_sim": {"policies": [
|
"portfolio_sim": {"policies": [
|
||||||
{"policy": "target", "cagr_pct": 23.7, "total_return_pct": 134.8,
|
{"policy": "target", "cagr_pct": 23.7, "total_return_pct": 134.8,
|
||||||
"spy_return_pct": 95.9, "max_drawdown_pct": 20.7},
|
"spy_return_pct": 95.9, "max_drawdown_pct": 20.7},
|
||||||
{"policy": "hold", "cagr_pct": 31.9, "total_return_pct": 203.6,
|
{"policy": "hold", "cagr_pct": 31.9, "total_return_pct": 203.6,
|
||||||
"spy_return_pct": 95.9, "max_drawdown_pct": 21.2},
|
"spy_return_pct": 95.9, "max_drawdown_pct": 21.2},
|
||||||
]},
|
]},
|
||||||
|
"portfolio_monitor": {
|
||||||
|
"production_strategy": "prod",
|
||||||
|
"runs": [{
|
||||||
|
"strategy": "prod", "lookback": "all", "lookback_label": "All history",
|
||||||
|
"cagr_pct": 40.0, "sharpe": 1.72, "max_drawdown_pct": 17.7,
|
||||||
|
"total_return_pct": 297.8, "spy_return_pct": 101.9,
|
||||||
|
}],
|
||||||
|
},
|
||||||
}
|
}
|
||||||
rec = bt._build_recommendation(report)
|
rec = bt._build_recommendation(report)
|
||||||
by_topic: dict[str, list[str]] = {}
|
by_topic: dict[str, list[str]] = {}
|
||||||
for item in rec["items"]:
|
for item in rec["items"]:
|
||||||
by_topic.setdefault(item["topic"], []).append(item["text"])
|
by_topic.setdefault(item["topic"], []).append(item["text"])
|
||||||
|
|
||||||
assert rec["headline"] is not None and "hold 30" in rec["headline"]
|
assert rec["headline"] is not None and "Production baseline" in rec["headline"]
|
||||||
assert any("hold 30 trading days" in t for t in by_topic["exit"])
|
# The hold-vs-target comparison is gone: both are exits the production book
|
||||||
|
# replaced, so a recommendation between them cannot lead to an action.
|
||||||
|
assert "exit" not in by_topic
|
||||||
|
# Benchmark must quote the SAME row the page's tiles show, not the policy sim.
|
||||||
|
assert "+297.8%" in by_topic["benchmark"][0]
|
||||||
|
assert "203.6" not in by_topic["benchmark"][0]
|
||||||
gate_texts = " | ".join(by_topic["gate"])
|
gate_texts = " | ".join(by_topic["gate"])
|
||||||
assert "confidence floor adds nothing" in gate_texts
|
assert "confidence floor adds nothing" in gate_texts
|
||||||
assert "keep the R:R floor" in gate_texts
|
assert "keep the R:R floor" in gate_texts
|
||||||
@@ -1810,3 +1826,24 @@ class TestPortfolioQualityMetrics:
|
|||||||
for key in ("sortino", "gain_to_pain", "profit_factor"):
|
for key in ("sortino", "gain_to_pain", "profit_factor"):
|
||||||
assert key in sim
|
assert key in sim
|
||||||
assert sim["sortino"] is None
|
assert sim["sortino"] is None
|
||||||
|
|
||||||
|
def test_build_recommendation_states_no_baseline_without_a_production_row():
|
||||||
|
"""A report with no portfolio monitor cannot describe the production book.
|
||||||
|
It used to fall back to recommending the fixed-hold exit — advice for a model
|
||||||
|
the production book had already replaced."""
|
||||||
|
report = {
|
||||||
|
"overall_qualified": {"net_avg_r": 0.13, "net_avg_r_ex_top5": 0.05},
|
||||||
|
"time_exit_sweep": [{"hold_days": 30, "net_avg_r": 0.50, "net_avg_r_ex_top5": 0.21}],
|
||||||
|
"portfolio_sim": {"policies": [
|
||||||
|
{"policy": "hold", "cagr_pct": 31.9, "total_return_pct": 203.6,
|
||||||
|
"spy_return_pct": 95.9, "max_drawdown_pct": 21.2},
|
||||||
|
]},
|
||||||
|
}
|
||||||
|
rec = bt._build_recommendation(report)
|
||||||
|
topics = {item["topic"] for item in rec["items"]}
|
||||||
|
|
||||||
|
assert rec["headline"] is None
|
||||||
|
# Nothing may be sourced from the legacy policy book.
|
||||||
|
assert "benchmark" not in topics
|
||||||
|
assert "exit" not in topics
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user