diff --git a/app/services/backtest_service.py b/app/services/backtest_service.py index 14a3817..eaf7b2e 100644 --- a/app/services/backtest_service.py +++ b/app/services/backtest_service.py @@ -4436,11 +4436,38 @@ async def run_and_store( async def get_backtest_report(db: AsyncSession) -> dict | None: - """Return the last cached backtest report, or None if never run.""" + """Return the last cached backtest report, or None if never run. + + The recommendation is **re-derived from the cached report** rather than + served as stored. It is a pure function of the numbers already in the + report — the payload's own note says it is derived from them on every run — + so recomputing costs nothing and keeps one class of bug out: + + A report cached by an older build carries that build's recommendation. After + a change to how the recommendation is sourced, the page would keep showing + the old one — quoting the legacy policy book, naming a rejected exit as + "recommended", and omitting ``basis_lookback``, which in turn let the + lookback selector default somewhere else. The result was the exact + tiles-disagree-with-recommendation contradiction this rebuild exists to + prevent, silently, until the next scheduled run happened to overwrite it. + + Re-deriving means a corrected recommendation appears on the first page load + after deploy instead of after the next backtest. + """ setting = await settings_store.get_setting(db, KEY_REPORT) if setting is None: return None try: - return json.loads(setting.value) + report = json.loads(setting.value) except (TypeError, ValueError): return None + if not isinstance(report, dict): + return None + try: + report["recommendation"] = _build_recommendation(report) + except Exception: + # Fail closed: drop it rather than fall back to the stored one, which is + # precisely the stale derivation this rebuild is here to replace. + logger.exception("Could not rebuild the backtest recommendation; omitting it") + report.pop("recommendation", None) + return report diff --git a/tests/unit/test_backtest_service.py b/tests/unit/test_backtest_service.py index 3a182df..b16303a 100644 --- a/tests/unit/test_backtest_service.py +++ b/tests/unit/test_backtest_service.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import math from datetime import date, timedelta from types import SimpleNamespace @@ -1862,3 +1863,58 @@ def test_build_recommendation_states_no_baseline_without_a_production_row(): assert "benchmark" not in topics assert "exit" not in topics + +async def test_cached_report_recommendation_is_rebuilt_on_read(session): + """A report cached by an older build carries that build's recommendation. + + Served verbatim, the page would show the legacy wording and no + basis_lookback — which let the lookback selector default elsewhere, putting + 3y tiles beside an all-history recommendation with no warning. This is the + shape of the report sitting in production right now. + """ + from app.services.admin_service import update_setting + + stale = { + "generated_at": "2026-08-12T05:00:00+00:00", + "tickers": 512, "candidates": 100, "qualified": 10, + "params": {"horizon_days": 30}, + "overall_qualified": {"net_avg_r": 0.13, "net_avg_r_ex_top5": 0.20}, + "portfolio_sim": {"policies": [ + {"policy": "hold", "cagr_pct": 31.9, "total_return_pct": 175.0, + "spy_return_pct": 101.9, "max_drawdown_pct": 23.7}, + ]}, + "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, + }], + }, + # What the old build stored: sourced from the policy book, and naming an + # exit the production book replaced. + "recommendation": { + "headline": "Trade the qualified list long-only; hold 30 trading days.", + "items": [ + {"topic": "benchmark", "text": "Book vs SPY: beats buy-and-hold by " + "+73.1 points (+175.0% vs +101.9%)."}, + {"topic": "robustness", "text": "Robustness: expectancy survives removing " + "the top 5% of winners (+0.20R net/trade " + "under the recommended 30d hold)."}, + ], + "note": "stale", + }, + } + await update_setting(session, bt.KEY_REPORT, json.dumps(stale)) + + report = await bt.get_backtest_report(session) + assert report is not None + rec = report["recommendation"] + + # Rebuilt: the basis is published, so the page cannot default elsewhere. + assert rec["basis_lookback"] == "all" + texts = " | ".join(i["text"] for i in rec["items"]) + # ...and it quotes the production book, not the policy sim it used to. + assert "+297.8%" in texts and "175.0" not in texts + assert "recommended 30d hold" not in texts + assert "Production baseline" in (rec["headline"] or "")