From b01da6865ef085f704a841ffd50da1d0bc557280 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Thu, 18 Jun 2026 10:16:27 +0200 Subject: [PATCH] fix(b3b): PIT path uses full trading calendar for hold days; CLI omits candidate_top_k under --pit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../application/equity_backtest_runner.py | 8 +- src/fxhnt/cli.py | 3 +- .../test_equity_backtest_runner.py | 91 +++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/src/fxhnt/application/equity_backtest_runner.py b/src/fxhnt/application/equity_backtest_runner.py index c3833e3..4f710a6 100644 --- a/src/fxhnt/application/equity_backtest_runner.py +++ b/src/fxhnt/application/equity_backtest_runner.py @@ -137,7 +137,13 @@ class EquityBacktestRunner: union = sorted({s for syms in uni_by_ym.values() for s in syms}) adj = self._store.read_panel(union, "adjclose") - all_days = sorted({d for s in adj.values() for d in s}) + # Holding-period calendar uses the FULL trading calendar (`days` from + # trading_days_epoch(), the same calendar the rebalance dates are built + # from), NOT the union-only adjclose panel which is sparser. A day where + # the union has prices for OTHER names but a held name has a gap is still + # a real trading day: daily_returns_for_weights already hold-flats the + # gapped name, so iterating the full calendar matches the non-PIT path. + all_days = days # Delisting detection (return-booking survivorship fix), same as the other # paths: a name whose adjclose TERMINATES more than `delist_gap_days` before diff --git a/src/fxhnt/cli.py b/src/fxhnt/cli.py index 707b4c4..7e224a7 100644 --- a/src/fxhnt/cli.py +++ b/src/fxhnt/cli.py @@ -533,7 +533,8 @@ def backtest_equity( result = EquityBacktestRunner( store, n=n, cost_bps_per_turnover=cost_bps, borrow_annual=borrow_annual, pit=pit, trailing_window_months=trailing_window_months, - candidate_top_k=candidate_top_k, candidate_min_history=candidate_min_history, + candidate_top_k=candidate_top_k if not pit else None, + candidate_min_history=candidate_min_history, delisting_return=delisting_return, ).run() report = evaluate_constructions( diff --git a/tests/integration/test_equity_backtest_runner.py b/tests/integration/test_equity_backtest_runner.py index 2742f68..fbf4e9d 100644 --- a/tests/integration/test_equity_backtest_runner.py +++ b/tests/integration/test_equity_backtest_runner.py @@ -569,6 +569,97 @@ def test_runner_pit_path_deterministic(tmp_path): assert np.array_equal(a, b) +def _build_pit_warehouse_with_calendar_gap(path: str) -> DuckDbFeatureStore: + """PIT warehouse where the selected names (AAA/BBB/CCC, liquid) each SKIP one + interior calendar day (a per-name price gap), while an ILLIQUID name (CAL) — never + selected into any month's top-N — trades EVERY calendar day. The full trading + calendar (trading_days_epoch(), union of ALL names incl. CAL) is therefore strictly + denser than the union of only the SELECTED names' adjclose panels. On a gap day a + held name is hold-flat while another selected name still books, matching the + non-PIT path's full-calendar hold-flat convention.""" + store = DuckDbFeatureStore(path) + start = dt.date(2021, 1, 4) + n = 760 # ~3 trading years + drifts = {"AAA": 0.0012, "BBB": 0.0004, "CCC": -0.0008} + items = [] + members = [] + # Stagger the per-name gap day so that on each gap day OTHER selected names trade. + gap_index = {"AAA": 600, "BBB": 601, "CCC": 602} + for sym, dr in drifts.items(): + rows = [] + px = 100.0 + for i in range(n): + day = start + dt.timedelta(days=i) + px *= (1.0 + dr) + if i == gap_index[sym]: + continue # this name has NO bar on its gap day (price gap) + ts = (day - dt.date(1970, 1, 1)).days * _SPD + rows.append((ts, {"adjclose": px, "close": px, "volume": 1_000_000.0})) + items.append((sym, rows)) + members.append((sym, "NYSE", "2000-01-01", "2026-06-01")) + # CAL: tiny volume (never top-N) but present on EVERY day — defines the full calendar. + px = 50.0 + cal_rows = [] + for i in range(n): + day = start + dt.timedelta(days=i) + px *= 1.0001 + ts = (day - dt.date(1970, 1, 1)).days * _SPD + cal_rows.append((ts, {"adjclose": px, "close": px, "volume": 1.0})) + items.append(("CAL", cal_rows)) + members.append(("CAL", "NYSE", "2000-01-01", "2026-06-01")) + store.write_features_bulk(items) + store.upsert_membership(members) + return store + + +def test_runner_pit_path_uses_full_trading_calendar_for_hold_days(tmp_path, monkeypatch): + # FIX #1: the PIT holding-period calendar must come from the FULL trading calendar + # (trading_days_epoch(), union of ALL warehouse names) — NOT the sparser union of + # only the SELECTED names' adjclose panels. We compare the real run against a run + # that simulates the OLD union-derived calendar (monkeypatch trading_days_epoch to + # return only the selected-union days) and assert the full-calendar series is at + # least as long, plus determinism + finiteness. + import fxhnt.application.equity_backtest_runner as runner_mod + + kw = dict(n=3, pit=True, trailing_window_months=12, min_bars_window=60, + momentum_min_history=252, candidate_min_price=1.0) + + # Full-calendar run (post-fix behavior). + store = _build_pit_warehouse_with_calendar_gap(str(tmp_path / "pit_full.duckdb")) + full = EquityBacktestRunner(store, **kw).run() + store.close() + + # Determinism: same warehouse + settings -> bit-identical series. + store2 = _build_pit_warehouse_with_calendar_gap(str(tmp_path / "pit_full2.duckdb")) + full2 = EquityBacktestRunner(store2, **kw).run() + store2.close() + for c in ("long", "ls", "tilt"): + assert np.array_equal(full.returns_by_construction[c], full2.returns_by_construction[c]) + assert np.isfinite(full.returns_by_construction[c]).all() + + # Union-derived run (simulated OLD behavior): patch trading_days_epoch so the + # holding-period calendar collapses to the SELECTED-names union (drops the + # CAL-only days). The fixed runner reads `days` from trading_days_epoch(); the + # OLD code derived its calendar from the union-only adjclose panel, which here + # equals the union of AAA/BBB/CCC (CAL is never selected). + store3 = _build_pit_warehouse_with_calendar_gap(str(tmp_path / "pit_union.duckdb")) + real_tde = store3.trading_days_epoch + + def union_only_calendar() -> list[int]: + adj = store3.read_panel(["AAA", "BBB", "CCC"], "adjclose") + return sorted({d for s in adj.values() for d in s}) + + monkeypatch.setattr(store3, "trading_days_epoch", union_only_calendar) + union = EquityBacktestRunner(store3, **kw).run() + monkeypatch.setattr(store3, "trading_days_epoch", real_tde) + store3.close() + + # The full-calendar holding-period series is at least as long as the sparser + # union-derived one (it never drops a real trading day). + for c in ("long", "ls", "tilt"): + assert len(full.returns_by_construction[c]) >= len(union.returns_by_construction[c]) + + def test_evaluate_constructions_sr_variance_is_cross_construction_computed_once(monkeypatch): # FIX #1: sr_variance must be the variance of the per-construction IS Sharpes # ACROSS the constructions, computed ONCE and shared by every evaluate() call —