From 21c712ca465adaf6d4bace72bbe203c648af056b Mon Sep 17 00:00:00 2001 From: Builder Date: Sun, 13 Sep 2026 12:29:06 +0800 Subject: [PATCH] fix(backtest): charge transaction costs on the two-way traded value (BUG-03) Every backtest cost was exactly half what the advertised A-share model prescribes. `CostModel.compute_costs` charges commission and slippage on the value it is given, and halves stamp tax on the assumption that half of that value is sells: commission_cost = abs_turnover * commission_rate stamp_cost = abs_turnover * stamp_tax_rate * 0.5 The `* 0.5` only holds for the *two-way* traded value (buys + sells). Both callers passed one-sided turnover, which is the convention this codebase documents -- portfolio/constraints.py:16, "Maximum one-sided turnover per rebalance". Measured, 30% one-sided turnover, commission 3bp / stamp 10bp / slippage 5bp: before compute_costs(0.30) = 3.90 bp after compute_costs(0.60) = 7.80 bp ratio 2.0000 Hand-check for the same inputs: commission 0.60*3bp + stamp 0.30*10bp + slippage 0.60*5bp = 7.80 bp, matching to 1e-12. A monthly strategy at 30% one-sided turnover was therefore undercharged by ~47 bp/year, which flatters every CAGR, Sharpe, Sortino and net-return figure this platform has produced. Fixed at both call sites: backtest/engine.py passes turnover * 2 backtest/capacity.py passes turnover * 2 The recorded turnover stays one-sided in both places -- it is a reported metric, not the base the cost model is fed. Mixing those two roles is what caused the defect; the comments now say which is which. Two tests: - test_cost_model.py pins the contract itself (two-way in, and one-sided input producing exactly half) - test_engine.py's rebalance test asserted 999_500, the half-charged figure, and now asserts 999_000 1318 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) --- backtest/capacity.py | 5 ++++- backtest/engine.py | 8 ++++++- tests/test_backtest/test_cost_model.py | 31 ++++++++++++++++++++++++++ tests/test_backtest/test_engine.py | 8 ++++++- 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/backtest/capacity.py b/backtest/capacity.py index 721d290..274395b 100644 --- a/backtest/capacity.py +++ b/backtest/capacity.py @@ -272,7 +272,10 @@ def _run_at_aum( # Standard turnover cost turnover = abs(trade_weights).sum() / 2 total_turnover += turnover - cost_rate = self.cost_model.compute_costs(turnover) + # Two-way traded value: compute_costs halves stamp tax on the + # assumption that half of its input is sells, so it needs + # buys + sells, not the one-sided turnover kept above. + cost_rate = self.cost_model.compute_costs(turnover * 2) capital -= cost_rate * capital # Extra impact cost diff --git a/backtest/engine.py b/backtest/engine.py index 64a9132..a4b0c00 100644 --- a/backtest/engine.py +++ b/backtest/engine.py @@ -224,7 +224,13 @@ def _simulate_pnl(self, returns: pd.DataFrame) -> None: turnover = (target_weights - current_weights).abs().sum() / 2 if turnover > 0: - rebalance_cost = self.cost_model.compute_costs(turnover) + # `turnover` is one-sided (the convention documented in + # portfolio/constraints.py), but CostModel.compute_costs + # charges commission and slippage on the value it is given + # and halves stamp tax on the assumption that half of it is + # sells -- which only holds for the *two-way* traded value. + # Passing one-sided turnover halved every component. + rebalance_cost = self.cost_model.compute_costs(turnover * 2) turnover_records.append((next_rdate, turnover)) current_weights = target_weights.copy() diff --git a/tests/test_backtest/test_cost_model.py b/tests/test_backtest/test_cost_model.py index c4049b9..ee21da6 100644 --- a/tests/test_backtest/test_cost_model.py +++ b/tests/test_backtest/test_cost_model.py @@ -1,9 +1,40 @@ """Tests for transaction cost model.""" import pandas as pd +import pytest from quant_platform.backtest.cost_model import CostModel +def test_compute_costs_scalar_input_is_the_two_way_traded_value(): + """The contract the backtest engine depends on (regression for BUG-03). + + `compute_costs` charges commission and slippage on the value it is given, + and halves stamp tax on the assumption that half of that value is sells. + That assumption only holds for the *two-way* traded value (buys + sells), + not for one-sided turnover. + + The engine used to pass one-sided turnover -- the convention documented in + portfolio/constraints.py -- which halved commission, stamp tax and slippage + together. On a 30%-one-sided-turnover monthly strategy that understated the + cost by ~47 bp/year. + """ + model = CostModel(commission=0.0003, stamp_tax=0.001, slippage=0.0005) + one_sided = 0.30 + two_way = one_sided * 2 + + expected = ( + two_way * 0.0003 # commission, both sides + + one_sided * 0.001 # stamp tax, sells only + + two_way * 0.0005 # slippage, both sides + ) + + assert model.compute_costs(two_way) == pytest.approx(expected) + # And feeding it one-sided is exactly the halving that was the defect. + assert model.compute_costs(two_way) == pytest.approx( + 2 * model.compute_costs(one_sided) + ) + + def test_cost_model_basic(): model = CostModel() turnover = pd.Series([100000, 50000], index=["A", "B"]) diff --git a/tests/test_backtest/test_engine.py b/tests/test_backtest/test_engine.py index d6dd2d7..0ef9d88 100644 --- a/tests/test_backtest/test_engine.py +++ b/tests/test_backtest/test_engine.py @@ -39,7 +39,13 @@ def test_initial_rebalance_charges_cost_and_records_turnover(self): engine.weights_history = {dates[1]: target} engine._simulate_pnl(returns) - assert engine.portfolio_values.iloc[-1] == pytest.approx(999_500.0) + # One-sided turnover is 0.5 (cash -> fully invested), so the two-way + # traded value is 1.0 and the cost is 1.0 * commission = 0.001. + # This used to assert 999_500 -- the half-charged figure produced by + # passing one-sided turnover to a cost model that expects two-way. + assert engine.portfolio_values.iloc[-1] == pytest.approx(999_000.0) assert engine.turnover_history is not None assert len(engine.turnover_history) == 1 + # The recorded turnover stays one-sided: it is a metric, not the base + # the cost model is fed. assert engine.turnover_history.iloc[0] == pytest.approx(0.5)