From fe63e9f48664163e39d32167aad33a9d47e91203 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:08:54 -0400 Subject: [PATCH 1/4] fix(execution): reject invalid costs atomically Validate configured and custom execution costs before any fill or financial state mutation, including flip commissions and final execution prices. Closes #41. --- .pre-commit-config.yaml | 2 +- src/ml4t/backtest/accounting/gatekeeper.py | 8 +- src/ml4t/backtest/broker.py | 2 + src/ml4t/backtest/config.py | 47 +++- src/ml4t/backtest/core/execution_engine.py | 13 +- src/ml4t/backtest/core/order_book.py | 21 +- src/ml4t/backtest/datafeed.py | 1 - src/ml4t/backtest/execution/fill_executor.py | 90 ++++++-- src/ml4t/backtest/execution/schedule.py | 1 - src/ml4t/backtest/models.py | 39 ++++ src/ml4t/backtest/result.py | 1 - tests/contracts/test_execution_contracts.py | 2 +- tests/execution/test_rebalancer.py | 2 +- tests/execution/test_schedule.py | 2 +- tests/test_artifact_spec.py | 7 +- tests/test_broker.py | 2 +- tests/test_config_wiring.py | 2 +- tests/test_core.py | 2 +- tests/test_cost_validation.py | 228 +++++++++++++++++++ tests/test_datafeed_memory.py | 2 +- tests/test_equity_curve.py | 2 +- tests/test_result.py | 2 +- tests/test_strategy_templates.py | 2 +- 23 files changed, 423 insertions(+), 57 deletions(-) create mode 100644 tests/test_cost_validation.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c965d5d5..e275c6d0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.12.10 + rev: v0.14.10 hooks: - id: ruff-format - id: ruff diff --git a/src/ml4t/backtest/accounting/gatekeeper.py b/src/ml4t/backtest/accounting/gatekeeper.py index ab1696f7..9d68ac1f 100644 --- a/src/ml4t/backtest/accounting/gatekeeper.py +++ b/src/ml4t/backtest/accounting/gatekeeper.py @@ -6,7 +6,7 @@ from collections.abc import Callable -from ..models import CommissionModel +from ..models import CommissionModel, calculate_commission from ..types import Order, OrderSide from .account import AccountState @@ -137,7 +137,9 @@ def validate_order(self, order: Order, price: float) -> tuple[bool, str]: # Check for position reversal (long→short or short→long) # Delegate to policy's handle_reversal() method if self._is_reversal(current_qty, order_qty_delta): - commission = self.commission_model.calculate(order.asset, order.quantity, price) + commission = calculate_commission( + self.commission_model, order.asset, order.quantity, price + ) return self.account.policy.handle_reversal( asset=order.asset, current_quantity=current_qty, @@ -156,7 +158,7 @@ def validate_order(self, order: Order, price: float) -> tuple[bool, str]: # This is an opening order (new position or adding to existing) # Calculate commission to include in cost - commission = self.commission_model.calculate(order.asset, order.quantity, price) + commission = calculate_commission(self.commission_model, order.asset, order.quantity, price) # Use buffered cash (reserves cash_buffer_pct for safety margin) available = self._available_cash() diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 6db6ebf0..1fdcccb6 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -298,6 +298,8 @@ def from_config( VolumeShareSlippage, ) + config._validate_for_execution() + effective_commission_type = config.commission_type if effective_commission_type == CommissionType.NONE: if config.commission_per_share > 0: diff --git a/src/ml4t/backtest/config.py b/src/ml4t/backtest/config.py index c6aa3dc2..2b26c61e 100644 --- a/src/ml4t/backtest/config.py +++ b/src/ml4t/backtest/config.py @@ -22,6 +22,7 @@ from __future__ import annotations +import math import os from dataclasses import asdict, dataclass, field, replace from enum import Enum @@ -29,7 +30,6 @@ from typing import Any import yaml - from ml4t.specs.base import serialize_artifact_value from ml4t.specs.market_data import FeedSpec, TimestampSemantics @@ -535,7 +535,7 @@ def validate(self, warn: bool = True) -> list[str]: """ import warnings as _warnings - issues: list[str] = [] + issues: list[str] = self._execution_validation_errors() # Look-ahead bias warning if self.execution_mode == ExecutionMode.SAME_BAR: @@ -565,12 +565,6 @@ def validate(self, warn: bool = True) -> list[str]: "Verify this matches your broker's actual costs." ) - if self.slippage_spread < 0: - issues.append(f"slippage_spread ({self.slippage_spread}) must be >= 0") - - if any(spread < 0 for spread in self.slippage_spread_by_asset.values()): - issues.append("slippage_spread_by_asset values must all be >= 0") - if ( self.slippage_type == SlippageType.SPREAD and self.slippage_spread == 0.0 @@ -641,6 +635,43 @@ def validate(self, warn: bool = True) -> list[str]: return issues + def _execution_validation_errors(self) -> list[str]: + errors: list[str] = [] + cost_fields = ( + "commission_rate", + "commission_per_share", + "commission_per_trade", + "commission_minimum", + "slippage_rate", + "slippage_fixed", + "slippage_spread", + "stop_slippage_rate", + ) + for field_name in cost_fields: + value = getattr(self, field_name) + try: + valid = math.isfinite(value) and value >= 0.0 + except TypeError: + valid = False + if not valid: + errors.append(f"{field_name} ({value!r}) must be finite and >= 0") + + for asset, spread in self.slippage_spread_by_asset.items(): + try: + valid = math.isfinite(spread) and spread >= 0.0 + except TypeError: + valid = False + if not valid: + errors.append( + f"slippage_spread_by_asset[{asset!r}] ({spread!r}) must be finite and >= 0" + ) + return errors + + def _validate_for_execution(self) -> None: + errors = self._execution_validation_errors() + if errors: + raise ValueError("Invalid BacktestConfig: " + "; ".join(errors)) + def get_effective_account_settings(self) -> tuple[bool, bool]: """Get account settings as a tuple. diff --git a/src/ml4t/backtest/core/execution_engine.py b/src/ml4t/backtest/core/execution_engine.py index b612bb6e..86acf3f8 100644 --- a/src/ml4t/backtest/core/execution_engine.py +++ b/src/ml4t/backtest/core/execution_engine.py @@ -4,6 +4,7 @@ import copy +from ..models import calculate_commission from ..types import ExecutionMode, OrderSide, OrderStatus, OrderType, Position from .shared import is_exit_order @@ -239,8 +240,8 @@ def _validate_shadow_queue_order( and abs(new_qty) > 1e-12 and ((current_qty > 0 and new_qty < 0) or (current_qty < 0 and new_qty > 0)) ) - commission = broker.commission_model.calculate( - order.asset, order.quantity, validation_price + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, validation_price ) multiplier = broker.get_multiplier(order.asset) @@ -288,7 +289,9 @@ def _commit_shadow_queue_fill( shadow_positions[order.asset].quantity if order.asset in shadow_positions else 0.0 ) new_qty = current_qty + qty_delta - commission = broker.commission_model.calculate(order.asset, order.quantity, fill_price) + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, fill_price + ) shadow_cash += -qty_delta * fill_price * broker.get_multiplier(order.asset) - commission if abs(new_qty) <= 1e-12: @@ -586,7 +589,9 @@ def _passes_simple_cash_check(self, order, fill_price: float) -> bool: return True signed_qty = order.quantity if order.side is OrderSide.BUY else -order.quantity - commission = broker.commission_model.calculate(order.asset, order.quantity, fill_price) + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, fill_price + ) projected_cash = broker.cash - signed_qty * fill_price - commission return projected_cash >= 0.0 diff --git a/src/ml4t/backtest/core/order_book.py b/src/ml4t/backtest/core/order_book.py index 8ff7dcb5..e0ecfe68 100644 --- a/src/ml4t/backtest/core/order_book.py +++ b/src/ml4t/backtest/core/order_book.py @@ -4,6 +4,7 @@ from datetime import datetime +from ..models import calculate_commission from ..types import ExecutionMode, Order, OrderSide, OrderStatus, OrderType, Position from .shared import SubmitOrderOptions, is_exit_order @@ -339,16 +340,16 @@ def _passes_submission_precheck(self, order: Order) -> bool: if closed != 0.0: close_cash = (-closed) * signal_price shadow_cash += close_cash - closed_commission = broker.commission_model.calculate( - order.asset, abs(closed), signal_price + closed_commission = calculate_commission( + broker.commission_model, order.asset, abs(closed), signal_price ) shadow_cash -= closed_commission if opened != 0.0: open_cash = opened * signal_price shadow_cash -= open_cash - opened_commission = broker.commission_model.calculate( - order.asset, abs(opened), signal_price + opened_commission = calculate_commission( + broker.commission_model, order.asset, abs(opened), signal_price ) shadow_cash -= opened_commission @@ -419,8 +420,8 @@ def _passes_buying_power_check(self, order: Order) -> bool: if closed != 0.0: closed_value = (-closed) * signal_price shadow_cash += closed_value - closed_commission = broker.commission_model.calculate( - order.asset, abs(closed), signal_price + closed_commission = calculate_commission( + broker.commission_model, order.asset, abs(closed), signal_price ) shadow_cash -= closed_commission @@ -430,8 +431,8 @@ def _passes_buying_power_check(self, order: Order) -> bool: # This prevents credit-model inflation where short proceeds # artificially inflate shadow cash. shadow_cash -= abs(opened) * signal_price - opened_commission = broker.commission_model.calculate( - order.asset, abs(opened), signal_price + opened_commission = calculate_commission( + broker.commission_model, order.asset, abs(opened), signal_price ) shadow_cash -= opened_commission @@ -461,7 +462,9 @@ def _passes_margin_submission_precheck(self, order: Order, signal_price: float) old_qty, old_price, size, signal_price ) - commission = broker.commission_model.calculate(order.asset, order.quantity, signal_price) + commission = calculate_commission( + broker.commission_model, order.asset, order.quantity, signal_price + ) available_cash = self._submission_shadow_cash if broker.cash_buffer_pct > 0 and available_cash > 0: available_cash *= 1.0 - broker.cash_buffer_pct diff --git a/src/ml4t/backtest/datafeed.py b/src/ml4t/backtest/datafeed.py index 75ff7de8..6a7c74cb 100644 --- a/src/ml4t/backtest/datafeed.py +++ b/src/ml4t/backtest/datafeed.py @@ -9,7 +9,6 @@ from typing import Any import polars as pl - from ml4t.specs.market_data import FeedSpec diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index 1818c59d..6b36d42e 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -7,11 +7,13 @@ from __future__ import annotations +import math from dataclasses import dataclass from datetime import datetime from typing import TYPE_CHECKING from ..config import InitialHwmSource, ShareType +from ..models import calculate_commission, calculate_slippage from ..types import ( ExitReason, Fill, @@ -58,6 +60,8 @@ class FillContext: is_partial: bool price_source: str quote_context: dict[str, float | None] + close_commission: float | None = None + open_commission: float | None = None class FillExecutor: @@ -100,6 +104,8 @@ def execute(self, order: Order, base_price: float) -> bool: current_time = broker._current_time assert current_time is not None, "Cannot execute fill without current time" + self._validate_execution_price(base_price, source="base execution price") + available_size = broker.get_available_size(order.asset, order.side) # Get effective quantity (considering partial fills from previous bars) @@ -107,6 +113,7 @@ def execute(self, order: Order, base_price: float) -> bool: fill_quantity = effective_quantity # Apply execution limits (volume participation) + remaining_quantity = 0.0 if broker.execution_limits is not None: if order.order_id in broker._filled_this_bar: return False @@ -121,20 +128,18 @@ def execute(self, order: Order, base_price: float) -> bool: if broker.share_type == ShareType.INTEGER: fill_quantity = float(int(fill_quantity)) - if fill_quantity <= 0: + if not math.isfinite(fill_quantity) or fill_quantity < 0: + raise ValueError( + "Invalid execution quantity from " + f"{type(broker.execution_limits).__name__}: got {fill_quantity!r}" + ) + if fill_quantity == 0: return False - broker._filled_this_bar.add(order.order_id) - remaining_quantity = max(0.0, effective_quantity - fill_quantity) if broker.share_type == ShareType.INTEGER: remaining_quantity = float(int(remaining_quantity)) - if remaining_quantity > 0: - broker._partial_orders[order.order_id] = remaining_quantity - else: - broker._partial_orders.pop(order.order_id, None) - # Apply market impact if broker.market_impact_model is not None: is_buy = order.side == OrderSide.BUY @@ -144,21 +149,55 @@ def execute(self, order: Order, base_price: float) -> bool: available_size, is_buy, ) + self._validate_market_impact(impact, is_buy=is_buy) base_price = base_price + impact + self._validate_execution_price(base_price, source="market-impact execution price") # Calculate slippage - slippage = broker.slippage_model.calculate( + slippage = calculate_slippage( + broker.slippage_model, order.asset, fill_quantity, base_price, available_size, ) fill_price = base_price + slippage if order.side == OrderSide.BUY else base_price - slippage + self._validate_execution_price(fill_price, source="execution price") # Calculate commission - commission = broker.commission_model.calculate(order.asset, fill_quantity, fill_price) + commission = calculate_commission( + broker.commission_model, order.asset, fill_quantity, fill_price + ) quote_context = broker.get_quote_context(order.asset, order.side) + signed_qty = fill_quantity if order.side == OrderSide.BUY else -fill_quantity + close_commission = None + open_commission = None + position = broker.positions.get(order.asset) + if position is not None: + new_qty = position.quantity + signed_qty + is_flip = position.quantity > 0 > new_qty or position.quantity < 0 < new_qty + if is_flip: + close_commission = calculate_commission( + broker.commission_model, + order.asset, + abs(position.quantity), + fill_price, + ) + open_commission = calculate_commission( + broker.commission_model, + order.asset, + abs(new_qty), + fill_price, + ) + + if broker.execution_limits is not None: + broker._filled_this_bar.add(order.order_id) + if remaining_quantity > 0: + broker._partial_orders[order.order_id] = remaining_quantity + else: + broker._partial_orders.pop(order.order_id, None) + # Create fill record fill = Fill( order_id=order.order_id, @@ -196,7 +235,6 @@ def execute(self, order: Order, base_price: float) -> bool: order.filled_quantity = fill_quantity # Build fill context - signed_qty = fill_quantity if order.side == OrderSide.BUY else -fill_quantity ctx = FillContext( order=order, current_time=current_time, @@ -208,6 +246,8 @@ def execute(self, order: Order, base_price: float) -> bool: is_partial=is_partial, price_source=broker.execution_price.value, quote_context=quote_context, + close_commission=close_commission, + open_commission=open_commission, ) # Update position and get actual commission (may change for flips) @@ -241,6 +281,25 @@ def execute(self, order: Order, base_price: float) -> bool: return not is_partial + @staticmethod + def _validate_execution_price(value: float, *, source: str) -> None: + if not math.isfinite(value) or value <= 0.0: + raise ValueError(f"Invalid {source}: expected a finite positive number, got {value!r}") + + def _validate_market_impact(self, value: float, *, is_buy: bool) -> None: + model_name = type(self.broker.market_impact_model).__name__ + if not math.isfinite(value): + raise ValueError( + f"Invalid market impact from {model_name}: expected a finite adverse value, " + f"got {value!r}" + ) + wrong_direction = (is_buy and value < 0.0) or (not is_buy and value > 0.0) + if wrong_direction: + expected = ">= 0 for buys" if is_buy else "<= 0 for sells" + raise ValueError( + f"Invalid market impact from {model_name}: expected {expected}, got {value!r}" + ) + def _update_position(self, ctx: FillContext) -> float: """Update position based on fill. @@ -439,12 +498,11 @@ def _flip_position( broker = self.broker order = ctx.order - close_qty = abs(old_qty) - open_qty = abs(new_qty) - # Calculate separate commissions for close and open portions - close_commission = broker.commission_model.calculate(order.asset, close_qty, ctx.fill_price) - open_commission = broker.commission_model.calculate(order.asset, open_qty, ctx.fill_price) + assert ctx.close_commission is not None + assert ctx.open_commission is not None + close_commission = ctx.close_commission + open_commission = ctx.open_commission total_commission = close_commission + open_commission # Close the old position (include multiplier for futures) diff --git a/src/ml4t/backtest/execution/schedule.py b/src/ml4t/backtest/execution/schedule.py index 9f713537..a9b4f463 100644 --- a/src/ml4t/backtest/execution/schedule.py +++ b/src/ml4t/backtest/execution/schedule.py @@ -9,7 +9,6 @@ from typing import Any import polars as pl - from ml4t.specs.market_data import FeedSpec, TimestampSemantics from ..calendar import get_schedule diff --git a/src/ml4t/backtest/models.py b/src/ml4t/backtest/models.py index e7b7a0bc..ed8aeb4b 100644 --- a/src/ml4t/backtest/models.py +++ b/src/ml4t/backtest/models.py @@ -1,5 +1,6 @@ """Pluggable commission and slippage models.""" +import math from typing import Protocol, runtime_checkable # === Protocols === @@ -21,6 +22,44 @@ def calculate( ) -> float: ... +def calculate_commission( + model: CommissionModel, + asset: str, + quantity: float, + price: float, +) -> float: + value = model.calculate(asset, quantity, price) + return _validate_nonnegative_model_output("commission", model, value) + + +def calculate_slippage( + model: SlippageModel, + asset: str, + quantity: float, + price: float, + volume: float | None, +) -> float: + value = model.calculate(asset, quantity, price, volume) + return _validate_nonnegative_model_output("slippage", model, value) + + +def _validate_nonnegative_model_output(kind: str, model: object, value: float) -> float: + model_name = type(model).__name__ + try: + numeric_value = float(value) + except (TypeError, ValueError) as exc: + raise ValueError( + f"Invalid {kind} from {model_name}: expected a finite non-negative number, " + f"got {value!r}" + ) from exc + if not math.isfinite(numeric_value) or numeric_value < 0.0: + raise ValueError( + f"Invalid {kind} from {model_name}: expected a finite non-negative number, " + f"got {value!r}" + ) + return numeric_value + + # === Commission Models === diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 438a22ad..34f70419 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -28,7 +28,6 @@ from typing import TYPE_CHECKING, Any, Literal import polars as pl - from ml4t.specs.market_data import FeedSpec try: diff --git a/tests/contracts/test_execution_contracts.py b/tests/contracts/test_execution_contracts.py index 4819131c..e6e4dc3a 100644 --- a/tests/contracts/test_execution_contracts.py +++ b/tests/contracts/test_execution_contracts.py @@ -4,6 +4,7 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.config import ( BacktestConfig, @@ -15,7 +16,6 @@ from ml4t.backtest.engine import run_backtest from ml4t.backtest.strategy import Strategy from ml4t.backtest.types import ExecutionMode -from ml4t.specs.market_data import FeedSpec def _prices() -> pl.DataFrame: diff --git a/tests/execution/test_rebalancer.py b/tests/execution/test_rebalancer.py index 0359dda5..e58ef131 100644 --- a/tests/execution/test_rebalancer.py +++ b/tests/execution/test_rebalancer.py @@ -3,6 +3,7 @@ from datetime import datetime import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import ( Broker, @@ -12,7 +13,6 @@ from ml4t.backtest.execution.rebalancer import RebalanceConfig, TargetWeightExecutor from ml4t.backtest.execution.schedule import RebalanceSchedule from ml4t.backtest.models import NoCommission, NoSlippage -from ml4t.specs.market_data import FeedSpec class TestRebalanceConfig: diff --git a/tests/execution/test_schedule.py b/tests/execution/test_schedule.py index 9a578169..00bca0aa 100644 --- a/tests/execution/test_schedule.py +++ b/tests/execution/test_schedule.py @@ -5,13 +5,13 @@ from datetime import UTC, datetime import polars as pl +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.execution import ( RebalanceCadence, RebalanceSchedule, resolve_rebalance_timestamps, ) -from ml4t.specs.market_data import FeedSpec def _make_weekday_series(start: str, end: str) -> pl.Series: diff --git a/tests/test_artifact_spec.py b/tests/test_artifact_spec.py index c6a3f093..a37f7d25 100644 --- a/tests/test_artifact_spec.py +++ b/tests/test_artifact_spec.py @@ -2,13 +2,14 @@ from pathlib import Path +from ml4t.diagnostic.artifacts import dump_spec, load_market_data_spec, load_spec +from ml4t.engineer.artifacts import FeatureSpec, LabelSpec, PredictionSpec +from ml4t.specs import ArtifactKind, FeedSpec, MarketDataSpec, TimestampSemantics + from ml4t.backtest.spec_bridge import ( market_data_spec_to_feed_spec, market_data_spec_to_runtime_metadata, ) -from ml4t.diagnostic.artifacts import dump_spec, load_market_data_spec, load_spec -from ml4t.engineer.artifacts import FeatureSpec, LabelSpec, PredictionSpec -from ml4t.specs import ArtifactKind, FeedSpec, MarketDataSpec, TimestampSemantics def test_market_data_spec_from_mapping_normalizes_timestamp_semantics() -> None: diff --git a/tests/test_broker.py b/tests/test_broker.py index e6f660f1..735229fd 100644 --- a/tests/test_broker.py +++ b/tests/test_broker.py @@ -3,6 +3,7 @@ from datetime import datetime import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.broker import Broker from ml4t.backtest.config import ShareType @@ -16,7 +17,6 @@ OrderType, Position, ) -from ml4t.specs.market_data import FeedSpec @pytest.fixture diff --git a/tests/test_config_wiring.py b/tests/test_config_wiring.py index 041a2b26..2736fb96 100644 --- a/tests/test_config_wiring.py +++ b/tests/test_config_wiring.py @@ -12,6 +12,7 @@ from datetime import datetime import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import ( BacktestConfig, @@ -41,7 +42,6 @@ VolumeShareSlippage, ) from ml4t.backtest.types import OrderSide, Position -from ml4t.specs.market_data import FeedSpec # --------------------------------------------------------------------------- # Helpers diff --git a/tests/test_core.py b/tests/test_core.py index 85cdabdb..80fdac61 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -4,6 +4,7 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import ( Broker, @@ -23,7 +24,6 @@ SlippageType, ) from ml4t.backtest.models import PercentageCommission, VolumeShareSlippage -from ml4t.specs.market_data import FeedSpec # === Test Data Generators === diff --git a/tests/test_cost_validation.py b/tests/test_cost_validation.py new file mode 100644 index 00000000..4ab00d4c --- /dev/null +++ b/tests/test_cost_validation.py @@ -0,0 +1,228 @@ +from __future__ import annotations + +import copy +import math +from datetime import datetime, timedelta +from typing import Any + +import polars as pl +import pytest + +from ml4t.backtest import BacktestConfig, DataFeed, Engine, ExecutionMode, Strategy, run_backtest + + +class _NoOpStrategy(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + return None + + +class _BuyOnceStrategy(Strategy): + def __init__(self) -> None: + self.submitted = False + self.broker_snapshot: dict[str, Any] | None = None + + def on_data(self, timestamp, data, context, broker) -> None: + if self.submitted: + return + broker.submit_order("AAPL", 10) + self.submitted = True + self.broker_snapshot = _financial_snapshot(broker) + + +class _RoundTripStrategy(Strategy): + def __init__(self) -> None: + self.entered = False + self.exit_submitted = False + self.broker_snapshot: dict[str, Any] | None = None + + def on_data(self, timestamp, data, context, broker) -> None: + position = broker.get_position("AAPL") + if not self.entered: + broker.submit_order("AAPL", 10) + self.entered = True + elif position is not None and not self.exit_submitted: + broker.close_position("AAPL") + self.exit_submitted = True + self.broker_snapshot = _financial_snapshot(broker) + + +class _ConstantCommission: + def __init__(self, value: float) -> None: + self.value = value + + def calculate(self, asset: str, quantity: float, price: float) -> float: + return self.value + + +class _ConstantSlippage: + def __init__(self, value: float) -> None: + self.value = value + + def calculate( + self, + asset: str, + quantity: float, + price: float, + volume: float | None, + ) -> float: + return self.value + + +class _ConstantImpact: + def __init__(self, value: float) -> None: + self.value = value + + def calculate( + self, + quantity: float, + price: float, + volume: float | None, + is_buy: bool, + ) -> float: + return self.value + + +class _InvalidSellPriceImpact: + def __init__(self) -> None: + self.broker = None + self.broker_snapshot: dict[str, Any] | None = None + + def calculate( + self, + quantity: float, + price: float, + volume: float | None, + is_buy: bool, + ) -> float: + if not is_buy: + self.broker_snapshot = _financial_snapshot(self.broker) + return 0.0 if is_buy else -200.0 + + +def _prices() -> pl.DataFrame: + start = datetime(2024, 1, 2) + timestamps = [start + timedelta(days=offset) for offset in range(4)] + return pl.DataFrame( + { + "timestamp": timestamps, + "symbol": ["AAPL"] * len(timestamps), + "open": [100.0] * len(timestamps), + "high": [100.0] * len(timestamps), + "low": [100.0] * len(timestamps), + "close": [100.0] * len(timestamps), + "volume": [1_000_000.0] * len(timestamps), + } + ) + + +def _financial_snapshot(broker) -> dict[str, Any]: + return { + "cash": broker.cash, + "account_cash": broker.account.cash, + "positions": copy.deepcopy(broker.positions), + "account_positions": copy.deepcopy(broker.account.positions), + "orders": copy.deepcopy(broker.orders), + "pending_orders": copy.deepcopy(broker.pending_orders), + "fills": copy.deepcopy(broker.fills), + "trades": copy.deepcopy(broker.trades), + "partial_orders": copy.deepcopy(broker._partial_orders), + "filled_this_bar": copy.deepcopy(broker._filled_this_bar), + } + + +@pytest.mark.parametrize( + "field", + [ + "commission_rate", + "commission_per_share", + "commission_per_trade", + "commission_minimum", + "slippage_rate", + "slippage_fixed", + "slippage_spread", + "stop_slippage_rate", + ], +) +@pytest.mark.parametrize("value", [-0.01, math.nan, math.inf]) +def test_engine_rejects_invalid_builtin_cost_config(field: str, value: float) -> None: + config = BacktestConfig(**{field: value}) + + with pytest.raises(ValueError, match=field): + Engine(DataFeed(prices_df=_prices()), _NoOpStrategy(), config) + + +@pytest.mark.parametrize("value", [-0.01, math.nan, math.inf]) +def test_engine_rejects_invalid_asset_spread(value: float) -> None: + config = BacktestConfig(slippage_spread_by_asset={"AAPL": value}) + + with pytest.raises(ValueError, match=r"slippage_spread_by_asset\['AAPL'\]"): + Engine(DataFeed(prices_df=_prices()), _NoOpStrategy(), config) + + +def test_run_backtest_enforces_config_validation() -> None: + config = BacktestConfig(commission_per_trade=-10.0) + + with pytest.raises(ValueError, match=r"commission_per_trade.*-10\.0"): + run_backtest(_prices(), _NoOpStrategy(), config=config) + + +@pytest.mark.parametrize("value", [-1.0, math.nan, math.inf]) +def test_invalid_custom_commission_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + engine = Engine(DataFeed(prices_df=_prices()), strategy) + model = _ConstantCommission(value) + engine.broker.commission_model = model + engine.broker.gatekeeper.commission_model = model + + with pytest.raises(ValueError, match=r"commission.*_ConstantCommission"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +@pytest.mark.parametrize("value", [-1.0, math.nan, math.inf]) +def test_invalid_custom_slippage_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + engine = Engine(DataFeed(prices_df=_prices()), strategy) + engine.broker.slippage_model = _ConstantSlippage(value) + + with pytest.raises(ValueError, match=r"slippage.*_ConstantSlippage"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +@pytest.mark.parametrize("value", [-1.0, math.nan, math.inf]) +def test_invalid_custom_impact_is_fail_atomic(value: float) -> None: + strategy = _BuyOnceStrategy() + engine = Engine( + DataFeed(prices_df=_prices()), + strategy, + market_impact_model=_ConstantImpact(value), + ) + + with pytest.raises(ValueError, match=r"market impact.*_ConstantImpact"): + engine.run() + + assert strategy.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == strategy.broker_snapshot + + +def test_invalid_execution_price_is_fail_atomic() -> None: + strategy = _RoundTripStrategy() + impact = _InvalidSellPriceImpact() + engine = Engine( + DataFeed(prices_df=_prices()), + strategy, + BacktestConfig(execution_mode=ExecutionMode.NEXT_BAR), + market_impact_model=impact, + ) + impact.broker = engine.broker + + with pytest.raises(ValueError, match=r"execution price.*-100\.0"): + engine.run() + + assert impact.broker_snapshot is not None + assert _financial_snapshot(engine.broker) == impact.broker_snapshot diff --git a/tests/test_datafeed_memory.py b/tests/test_datafeed_memory.py index e6977517..423e2e0b 100644 --- a/tests/test_datafeed_memory.py +++ b/tests/test_datafeed_memory.py @@ -9,10 +9,10 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import BacktestConfig, DataFeed from ml4t.backtest.config import DataFrequency -from ml4t.specs.market_data import FeedSpec class TestDataFeedMemoryEfficiency: diff --git a/tests/test_equity_curve.py b/tests/test_equity_curve.py index 65223c8b..72bae584 100644 --- a/tests/test_equity_curve.py +++ b/tests/test_equity_curve.py @@ -3,11 +3,11 @@ from datetime import datetime, timedelta import polars as pl +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import BacktestConfig, DataFeed, Engine, Strategy from ml4t.backtest.analytics.equity import EquityCurve from ml4t.backtest.config import DataFrequency -from ml4t.specs.market_data import FeedSpec class TestEquityCurveAnnualization: diff --git a/tests/test_result.py b/tests/test_result.py index ece09e32..d1347538 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -10,6 +10,7 @@ import polars as pl import pytest +from ml4t.specs.market_data import FeedSpec from ml4t.backtest.config import BacktestConfig from ml4t.backtest.result import ( @@ -17,7 +18,6 @@ enrich_trades_with_signals, ) from ml4t.backtest.types import Fill, OrderSide, Trade -from ml4t.specs.market_data import FeedSpec @pytest.fixture diff --git a/tests/test_strategy_templates.py b/tests/test_strategy_templates.py index fdf7b27a..966cc7b2 100644 --- a/tests/test_strategy_templates.py +++ b/tests/test_strategy_templates.py @@ -4,6 +4,7 @@ import numpy as np import polars as pl +from ml4t.specs.market_data import FeedSpec from ml4t.backtest import BacktestConfig, DataFeed, Engine from ml4t.backtest.execution.schedule import RebalanceSchedule @@ -13,7 +14,6 @@ MomentumStrategy, SignalFollowingStrategy, ) -from ml4t.specs.market_data import FeedSpec def make_price_data( From 372f3c6b7e570f2bf9068eb5b07668d339381f1e Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:13:37 -0400 Subject: [PATCH 2/4] fix(accounting): reconcile partial-close costs Record partial exits as realized trades, allocate entry commission between exited and residual quantities, and enforce commission conservation with precision-derived tolerances. Closes #42. --- src/ml4t/backtest/execution/fill_executor.py | 45 +++++++- tests/contracts/test_book_parity_behaviors.py | 9 +- tests/helpers/invariants.py | 52 +++++---- tests/test_partial_close_accounting.py | 101 ++++++++++++++++++ 4 files changed, 178 insertions(+), 29 deletions(-) create mode 100644 tests/test_partial_close_accounting.py diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index 6b36d42e..c65aa6cd 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -252,6 +252,7 @@ def execute(self, order: Order, base_price: float) -> bool: # Update position and get actual commission (may change for flips) actual_commission = self._update_position(ctx) + fill.commission = actual_commission # Update cash (include multiplier for futures/derivatives) multiplier = broker.get_multiplier(order.asset) @@ -598,14 +599,52 @@ def _scale_position( else: # Short position pnl = (pos.entry_price - ctx.fill_price) * exited_qty * pos.multiplier - # Subtract proportional commission - # entry_commission is for the full position, so we take the proportional part - exit_portion_ratio = exited_qty / abs(pos.initial_quantity or old_qty) + # Allocate the current position's entry costs in proportion to the quantity + # removed. The residual cost remains attached to the residual position. + exit_portion_ratio = exited_qty / abs(old_qty) proportional_entry_commission = pos.entry_commission * exit_portion_ratio + pos.entry_commission -= proportional_entry_commission partial_exit_commission = ctx.commission total_commission = proportional_entry_commission + partial_exit_commission pnl -= total_commission + raw_pct = ( + (ctx.fill_price - pos.entry_price) / pos.entry_price if pos.entry_price else 0.0 + ) + pnl_pct = raw_pct if old_qty > 0 else -raw_pct + entry_quote = pos.context.get("entry_quote_context", {}) + exit_quote = ctx.quote_context + broker.trades.append( + Trade( + symbol=ctx.order.asset, + entry_time=pos.entry_time, + exit_time=ctx.current_time, + entry_price=pos.entry_price, + exit_price=ctx.fill_price, + quantity=math.copysign(exited_qty, old_qty), + pnl=pnl, + pnl_percent=pnl_pct, + bars_held=pos.bars_held, + fees=total_commission, + exit_slippage=ctx.slippage, + exit_reason=_get_exit_reason(ctx.order), + mfe=pos.max_favorable_excursion, + mae=pos.max_adverse_excursion, + entry_slippage=pos.entry_slippage, + multiplier=pos.multiplier, + entry_quote_mid_price=entry_quote.get("quote_mid_price"), + entry_bid_price=entry_quote.get("bid_price"), + entry_ask_price=entry_quote.get("ask_price"), + entry_spread=entry_quote.get("spread"), + entry_available_size=entry_quote.get("available_size"), + exit_quote_mid_price=exit_quote.get("quote_mid_price"), + exit_bid_price=exit_quote.get("bid_price"), + exit_ask_price=exit_quote.get("ask_price"), + exit_spread=exit_quote.get("spread"), + exit_available_size=exit_quote.get("available_size"), + ) + ) + # Record P&L event for trading stats broker._record_pnl_event(ctx.order.asset, pnl) diff --git a/tests/contracts/test_book_parity_behaviors.py b/tests/contracts/test_book_parity_behaviors.py index d7f19ecb..e5728a32 100644 --- a/tests/contracts/test_book_parity_behaviors.py +++ b/tests/contracts/test_book_parity_behaviors.py @@ -63,7 +63,6 @@ def on_data(self, timestamp, data, context, broker) -> None: self.msft_order_qty = order.quantity -@pytest.mark.no_invariant_check # Known: partial close during rebalance doesn't prorate entry commission def test_snapshot_value_freezes_targets_vs_incremental_recompute() -> None: start = datetime(2024, 1, 1) prices = pl.DataFrame( @@ -86,10 +85,14 @@ def test_snapshot_value_freezes_targets_vs_incremental_recompute() -> None: snapshot_strategy = _RebalanceByMode(RebalanceMode.SNAPSHOT) incremental_strategy = _RebalanceByMode(RebalanceMode.INCREMENTAL) - run_backtest(prices=prices, strategy=snapshot_strategy, config=cfg) - run_backtest(prices=prices, strategy=incremental_strategy, config=cfg) + snapshot_result = run_backtest(prices=prices, strategy=snapshot_strategy, config=cfg) + incremental_result = run_backtest(prices=prices, strategy=incremental_strategy, config=cfg) assert snapshot_strategy.msft_order_qty > incremental_strategy.msft_order_qty + for result in (snapshot_result, incremental_result): + fill_costs = sum(fill.commission for fill in result.fills) + reported_costs = sum(trade.fees for trade in result.trades) + assert reported_costs == pytest.approx(fill_costs, abs=1e-9) class _RotateSellThenBuy(Strategy): diff --git a/tests/helpers/invariants.py b/tests/helpers/invariants.py index e00d3514..bf0c735d 100644 --- a/tests/helpers/invariants.py +++ b/tests/helpers/invariants.py @@ -34,6 +34,7 @@ def assert_result_invariants( check_no_nan: bool = True, check_exit_reason_consistency: bool = True, check_fill_order_type_bounds: bool = True, + check_commission_allocation: bool = True, ) -> None: """Assert universal invariants on a BacktestResult. @@ -62,6 +63,14 @@ def assert_result_invariants( _check_exit_reason_consistency(result.trades) if check_fill_order_type_bounds: _check_fill_order_type_bounds(result) + if check_commission_allocation: + _check_commission_allocation(result) + + +def _accounting_tolerance(*values: float, operations: int = 1) -> float: + """Bound rounding error by the represented values and arithmetic operation count.""" + scale = max((abs(value) for value in values), default=0.0) + return max(1e-9, math.ulp(scale) * max(16, operations * 4)) def _check_equity_terminal( @@ -69,14 +78,7 @@ def _check_equity_terminal( initial_cash: float, closed_trades: list, ) -> None: - """Verify: initial_cash + sum(closed_pnl) + sum(open_pnl) ≈ final_value. - - When open positions exist, the tolerance is expanded because the open trade - PnL is computed from Position state which may not perfectly capture all - intermediate costs (especially in rebalancing with integer shares and high - commission rates). Multi-asset rebalancing with integer shares also creates - small rounding discrepancies in position PnL vs. cash-based equity tracking. - """ + """Verify: initial_cash + sum(closed_pnl) + sum(open_pnl) ≈ final_value.""" if not result.equity_curve: return @@ -88,20 +90,8 @@ def _check_equity_terminal( expected = initial_cash + closed_pnl + open_pnl diff = abs(expected - final_value) - # Base tolerance: relative to portfolio size - tol = max(_ABS_TOL, abs(final_value) * 1e-6) - - # Expand tolerance for total fill costs (commission + slippage on all fills) - total_fill_costs = sum(f.commission + f.slippage for f in result.fills) - if total_fill_costs > 0: - tol = max(tol, total_fill_costs * 0.05) # 5% of total costs - - # Expand tolerance for open positions: mark-to-market PnL from Position state - # can diverge slightly from cash-based equity tracking, especially with - # multi-asset rebalancing and integer share rounding. - if open_trades: - open_notional = sum(abs(t.quantity) * t.exit_price * t.multiplier for t in open_trades) - tol = max(tol, open_notional * 1e-4) # 0.01% of open notional + terms = [initial_cash, *(t.pnl for t in closed_trades), *(t.pnl for t in open_trades)] + tol = _accounting_tolerance(expected, final_value, *terms, operations=len(terms) + 1) assert diff <= tol, ( f"Equity terminal invariant violated: " @@ -118,7 +108,7 @@ def _check_pnl_decomposition(closed_trades: list) -> None: expected_net = gross - t.fees diff = abs(expected_net - t.pnl) - tol = max(_ABS_TOL, abs(gross) * 1e-6) + tol = _accounting_tolerance(gross, t.fees, expected_net, t.pnl, operations=2) assert diff <= tol, ( f"PnL decomposition invariant violated for trade {i} ({t.symbol}): " f"gross_pnl({gross:.6f}) - fees({t.fees:.6f}) = {expected_net:.6f} " @@ -295,3 +285,19 @@ def _check_fill_order_type_bounds(result: BacktestResult) -> None: f"Fill order-type bound violated for fill {i} ({f.asset}): " f"stop SELL filled at {f.price:.6f} > stop_price {stop_price:.6f}" ) + + +def _check_commission_allocation(result: BacktestResult) -> None: + """Verify every charged fill commission is allocated to a realized or open trade.""" + fill_commission = sum(fill.commission for fill in result.fills) + trade_commission = sum(trade.fees for trade in result.trades) + tol = _accounting_tolerance( + fill_commission, + trade_commission, + operations=len(result.fills) + len(result.trades), + ) + assert abs(fill_commission - trade_commission) <= tol, ( + "Commission allocation invariant violated: " + f"fills({fill_commission:.12f}) != trades({trade_commission:.12f}), " + f"diff={abs(fill_commission - trade_commission):.12f}, tol={tol:.12f}" + ) diff --git a/tests/test_partial_close_accounting.py b/tests/test_partial_close_accounting.py new file mode 100644 index 00000000..1afa0d70 --- /dev/null +++ b/tests/test_partial_close_accounting.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +from datetime import datetime, timedelta + +import pytest + +from ml4t.backtest import AssetClass, Broker, ContractSpec, OrderSide, OrderStatus +from ml4t.backtest.config import ShareType +from ml4t.backtest.models import NoSlippage, PercentageCommission + + +def _execute(broker: Broker, *, day: int, price: float, quantity: float) -> None: + timestamp = datetime(2024, 1, 1) + timedelta(days=day) + broker._update_time( + timestamp=timestamp, + prices={"TEST": price}, + opens={"TEST": price}, + highs={"TEST": price}, + lows={"TEST": price}, + volumes={"TEST": 1_000_000.0}, + signals={}, + ) + side = OrderSide.BUY if quantity > 0 else OrderSide.SELL + order = broker.submit_order("TEST", abs(quantity), side) + assert order is not None + broker._process_orders() + assert order.status == OrderStatus.FILLED + + +@pytest.mark.parametrize( + ("share_type", "contract_specs", "steps", "expected_closed", "expected_remaining"), + [ + ( + ShareType.INTEGER, + None, + [(100.0, 100.0), (110.0, -30.0), (90.0, -20.0)], + 2, + 50.0, + ), + ( + ShareType.FRACTIONAL, + None, + [(100.0, -10.5), (90.0, 3.25), (110.0, 2.0)], + 2, + -5.25, + ), + ( + ShareType.FRACTIONAL, + None, + [(100.0, 10.0), (110.0, 5.0), (120.0, -6.0)], + 1, + 9.0, + ), + ( + ShareType.INTEGER, + None, + [(100.0, 10.0), (120.0, -15.0)], + 1, + -5.0, + ), + ( + ShareType.INTEGER, + {"TEST": ContractSpec("TEST", AssetClass.FUTURE, multiplier=50.0)}, + [(4_000.0, 4.0), (4_010.0, -1.0)], + 1, + 3.0, + ), + ], +) +def test_fill_costs_are_conserved_across_realized_and_residual_positions( + share_type: ShareType, + contract_specs: dict[str, ContractSpec] | None, + steps: list[tuple[float, float]], + expected_closed: int, + expected_remaining: float, +) -> None: + broker = Broker( + initial_cash=10_000_000.0, + commission_model=PercentageCommission(0.01), + slippage_model=NoSlippage(), + allow_short_selling=True, + allow_leverage=True, + share_type=share_type, + contract_specs=contract_specs, + ) + + for day, (price, quantity) in enumerate(steps): + _execute(broker, day=day, price=price, quantity=quantity) + + position = broker.get_position("TEST") + assert position is not None + assert position.quantity == pytest.approx(expected_remaining) + assert len(broker.trades) == expected_closed + + fill_costs = sum(fill.commission for fill in broker.fills) + realized_costs = sum(trade.fees for trade in broker.trades) + residual_costs = position.entry_commission + assert realized_costs + residual_costs == pytest.approx(fill_costs, abs=1e-9) + + for trade in broker.trades: + assert trade.gross_pnl - trade.fees == pytest.approx(trade.pnl, abs=1e-9) From 1c70dde3efb508b9369875b36532d8fc932c351d Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:22:52 -0400 Subject: [PATCH 3/4] feat(results): preserve rejected orders Expose structured rejected-order records, aggregate counts, DataFrame conversion, and additive Parquet round trips while retaining original requested quantities. Closes #43. --- src/ml4t/backtest/engine.py | 12 ++- src/ml4t/backtest/export.py | 4 + src/ml4t/backtest/result.py | 95 ++++++++++++++++++++- src/ml4t/backtest/types.py | 25 ++++++ tests/benchmark/test_hotpath_benchmarks.py | 1 + tests/test_rejected_order_results.py | 99 ++++++++++++++++++++++ 6 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 tests/test_rejected_order_results.py diff --git a/src/ml4t/backtest/engine.py b/src/ml4t/backtest/engine.py index 47b57b06..db7a13e1 100644 --- a/src/ml4t/backtest/engine.py +++ b/src/ml4t/backtest/engine.py @@ -298,6 +298,8 @@ def _build_activity_metrics(self) -> dict[str, int | float]: ) max_open_positions = max((state[5] for state in self.portfolio_state), default=0) return { + "num_orders": len(self.broker.orders), + "num_rejected_orders": len(self.broker.get_rejected_orders()), "num_fills": 0, "num_rebalance_events": 0, "unique_symbols_traded": 0, @@ -335,6 +337,8 @@ def _build_activity_metrics(self) -> dict[str, int | float]: max_open_positions = max((state[5] for state in self.portfolio_state), default=0) return { + "num_orders": len(self.broker.orders), + "num_rejected_orders": len(self.broker.get_rejected_orders()), "num_fills": len(fills), "num_rebalance_events": len(rebalance_events), "unique_symbols_traded": len(traded_symbols), @@ -356,9 +360,14 @@ def _generate_results(self) -> BacktestResult: trades=[], equity_curve=[], fills=[], + rejected_orders=self.broker.get_rejected_orders(), predictions=self.feed.signals, portfolio_state=[], - metrics={"skipped_bars": self._skipped_bars}, + metrics={ + "skipped_bars": self._skipped_bars, + "num_orders": len(self.broker.orders), + "num_rejected_orders": len(self.broker.get_rejected_orders()), + }, config=self.config, ) @@ -475,6 +484,7 @@ def _generate_results(self) -> BacktestResult: trades=all_trades, # Includes both closed and open trades equity_curve=self.equity_curve, fills=self.broker.fills, + rejected_orders=self.broker.get_rejected_orders(), predictions=self.feed.signals, portfolio_state=self.portfolio_state, metrics=metrics, diff --git a/src/ml4t/backtest/export.py b/src/ml4t/backtest/export.py index 514d0393..19fc1894 100644 --- a/src/ml4t/backtest/export.py +++ b/src/ml4t/backtest/export.py @@ -130,6 +130,8 @@ def batch_export( record["total_commission"] = metrics.get("total_commission", 0.0) record["total_slippage"] = metrics.get("total_slippage", 0.0) record["num_fills"] = metrics.get("num_fills", 0) + record["num_orders"] = metrics.get("num_orders", 0) + record["num_rejected_orders"] = metrics.get("num_rejected_orders", 0) record["num_rebalance_events"] = metrics.get("num_rebalance_events", 0) record["unique_symbols_traded"] = metrics.get("unique_symbols_traded", 0) record["total_filled_notional"] = metrics.get("total_filled_notional", 0.0) @@ -217,6 +219,8 @@ def generate_json_report( "win_rate": metrics.get("win_rate", 0.0), "profit_factor": metrics.get("profit_factor", 0.0), "final_value": metrics.get("final_value", 0.0), + "num_orders": metrics.get("num_orders", 0), + "num_rejected_orders": metrics.get("num_rejected_orders", 0), } ) diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 34f70419..37c53ab5 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -35,7 +35,7 @@ except ImportError: # pragma: no cover - fallback for local editable edge cases __version__ = "0.0.0.dev0" from .analytics.annualization import should_session_align -from .types import Fill, OrderSide, Trade +from .types import Fill, Order, OrderSide, OrderStatus, OrderType, Trade if TYPE_CHECKING: from .analytics import EquityCurve, TradeAnalyzer @@ -56,6 +56,7 @@ class BacktestResult: trades: List of completed Trade objects equity_curve: List of (timestamp, portfolio_value) tuples fills: List of Fill objects (all order fills) + rejected_orders: Orders that reached the rejected terminal state predictions: Raw prediction DataFrame passed into the backtest (optional) metrics: Dictionary of computed performance metrics config: BacktestConfig used for the backtest (optional) @@ -74,12 +75,14 @@ class BacktestResult: portfolio_state: list[tuple[datetime, float, float, float, float, int]] = field( default_factory=list ) + rejected_orders: list[Order] = field(default_factory=list) # Cached DataFrames (computed on demand) _trades_df: pl.DataFrame | None = field(default=None, repr=False) _equity_df: pl.DataFrame | None = field(default=None, repr=False) _fills_df: pl.DataFrame | None = field(default=None, repr=False) _portfolio_state_df: pl.DataFrame | None = field(default=None, repr=False) + _rejected_orders_df: pl.DataFrame | None = field(default=None, repr=False) def _feed_spec(self) -> FeedSpec | None: if self.config is None: @@ -204,6 +207,38 @@ def to_fills_dataframe(self) -> pl.DataFrame: self._fills_df = pl.DataFrame(records, schema=self._fills_schema()) return self._fills_df + def to_rejected_orders_dataframe(self) -> pl.DataFrame: + """Convert rejected orders to a stable, machine-readable DataFrame.""" + if self._rejected_orders_df is not None: + return self._rejected_orders_df + if not self.rejected_orders: + return pl.DataFrame(schema=self._rejected_orders_schema()) + + records = [ + { + "order_id": order.order_id, + "symbol": order.asset, + "timestamp": order.created_at, + "requested_quantity": order.requested_quantity, + "side": order.side.value, + "order_type": order.order_type.value, + "limit_price": order.limit_price, + "stop_price": order.stop_price, + "trail_amount": order.trail_amount, + "parent_id": order.parent_id, + "rebalance_id": order.rebalance_id, + "status": order.status.value, + "rejection_code": order.rejection_code, + "rejection_reason": order.rejection_reason, + } + for order in self.rejected_orders + ] + self._rejected_orders_df = pl.DataFrame( + records, + schema=self._rejected_orders_schema(), + ) + return self._rejected_orders_df + def to_predictions_dataframe(self) -> pl.DataFrame: """Return the raw prediction DataFrame used as backtest input.""" if self.predictions is None: @@ -524,8 +559,8 @@ def to_parquet( Args: path: Directory path to write files include: Components to include. Default: all. - Options: ["trades", "fills", "predictions", "equity", "portfolio_state", - "daily_pnl", "metrics", "config", "spec"] + Options: ["trades", "fills", "rejected_orders", "predictions", "equity", + "portfolio_state", "daily_pnl", "metrics", "config", "spec"] compression: Parquet compression codec (default: "zstd") Returns: @@ -538,6 +573,7 @@ def to_parquet( include = [ "trades", "fills", + "rejected_orders", "predictions", "equity", "portfolio_state", @@ -559,6 +595,14 @@ def to_parquet( self.to_fills_dataframe().write_parquet(fills_path, compression=compression) written["fills"] = fills_path + if "rejected_orders" in include: + rejected_orders_path = path / "rejected_orders.parquet" + self.to_rejected_orders_dataframe().write_parquet( + rejected_orders_path, + compression=compression, + ) + written["rejected_orders"] = rejected_orders_path + if "predictions" in include and self.predictions is not None: predictions_path = path / "predictions.parquet" self.to_predictions_dataframe().write_parquet(predictions_path, compression=compression) @@ -725,6 +769,30 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: ) ) + rejected_orders: list[Order] = [] + rejected_orders_path = path / "rejected_orders.parquet" + if rejected_orders_path.exists(): + rejected_orders_df = pl.read_parquet(rejected_orders_path) + for row in rejected_orders_df.iter_rows(named=True): + rejected_orders.append( + Order( + order_id=row["order_id"], + asset=row["symbol"], + created_at=row["timestamp"], + requested_quantity=row["requested_quantity"], + quantity=row["requested_quantity"], + side=OrderSide(row["side"]), + order_type=OrderType(row["order_type"]), + limit_price=row.get("limit_price"), + stop_price=row.get("stop_price"), + trail_amount=row.get("trail_amount"), + parent_id=row.get("parent_id"), + rebalance_id=row.get("rebalance_id"), + status=OrderStatus(row["status"]), + rejection_reason=row.get("rejection_reason"), + ) + ) + predictions = None predictions_path = path / "predictions.parquet" if predictions_path.exists(): @@ -785,6 +853,7 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: fills=fills, predictions=predictions, portfolio_state=portfolio_state, + rejected_orders=rejected_orders, metrics=metrics, config=config, ) @@ -862,6 +931,26 @@ def _fills_schema() -> dict[str, pl.DataType]: "available_size": pl.Float64(), } + @staticmethod + def _rejected_orders_schema() -> dict[str, pl.DataType]: + """Schema for rejected order records added compatibly in v0.1.0.""" + return { + "order_id": pl.String(), + "symbol": pl.String(), + "timestamp": pl.Datetime(), + "requested_quantity": pl.Float64(), + "side": pl.String(), + "order_type": pl.String(), + "limit_price": pl.Float64(), + "stop_price": pl.Float64(), + "trail_amount": pl.Float64(), + "parent_id": pl.String(), + "rebalance_id": pl.String(), + "status": pl.String(), + "rejection_code": pl.String(), + "rejection_reason": pl.String(), + } + @staticmethod def _equity_schema() -> dict[str, pl.DataType]: """Schema for equity DataFrame.""" diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index b4e9ae76..24264fd5 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -164,6 +164,7 @@ class Order: filled_price: float | None = None filled_quantity: float = 0.0 rejection_reason: str | None = None # Reason if order was rejected + requested_quantity: float | None = None # Internal risk management fields (set by broker) _created_bar_index: int = 0 _signal_price: float | None = None # Close price at order creation time @@ -171,6 +172,30 @@ class Order: _exit_reason: ExitReason | None = None # Typed exit reason (preferred) _risk_fill_price: float | None = None # Stop/target price for risk exits + def __post_init__(self) -> None: + if self.requested_quantity is None: + self.requested_quantity = self.quantity + + @property + def rejection_code(self) -> str | None: + """Return a stable machine-readable category for the rejection reason.""" + if self.status is not OrderStatus.REJECTED: + return None + reason = (self.rejection_reason or "").lower() + if "rounds to zero" in reason: + return "quantity_rounds_to_zero" + if "no price" in reason: + return "price_unavailable" + if "fill check" in reason: + return "fill_check_failed" + if "short" in reason or "reversal not allowed" in reason: + return "account_restriction" + if "buying power" in reason or "margin" in reason: + return "insufficient_buying_power" + if "cash" in reason or "insufficient" in reason: + return "insufficient_cash" + return "order_validation_failed" + @dataclass class Position: diff --git a/tests/benchmark/test_hotpath_benchmarks.py b/tests/benchmark/test_hotpath_benchmarks.py index 0e7b0fe4..4f291176 100644 --- a/tests/benchmark/test_hotpath_benchmarks.py +++ b/tests/benchmark/test_hotpath_benchmarks.py @@ -175,6 +175,7 @@ def test_optimized_feed_matches_legacy_output(): @pytest.mark.benchmark +@pytest.mark.no_cover def test_optimized_feed_runtime_vs_legacy_baseline(): prices, signals = _build_benchmark_data(n_bars=3000, n_assets=20) diff --git a/tests/test_rejected_order_results.py b/tests/test_rejected_order_results.py new file mode 100644 index 00000000..d763f3eb --- /dev/null +++ b/tests/test_rejected_order_results.py @@ -0,0 +1,99 @@ +from __future__ import annotations + +from datetime import datetime + +import polars as pl + +from ml4t.backtest import BacktestConfig, Strategy, run_backtest +from ml4t.backtest.types import ExecutionMode + + +class _UnaffordableOrder(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + broker.submit_order("AAPL", 1_000_000.0) + + +class _NoOrders(Strategy): + def on_data(self, timestamp, data, context, broker) -> None: + pass + + +def _prices() -> pl.DataFrame: + return pl.DataFrame( + { + "timestamp": [datetime(2024, 1, 2)], + "asset": ["AAPL"], + "open": [100.0], + "high": [100.0], + "low": [100.0], + "close": [100.0], + "volume": [1_000_000.0], + } + ) + + +def _run(strategy: Strategy): + return run_backtest( + prices=_prices(), + strategy=strategy, + config=BacktestConfig( + initial_cash=10_000.0, + execution_mode=ExecutionMode.SAME_BAR, + ), + ) + + +def test_unaffordable_order_is_preserved_in_public_result() -> None: + result = _run(_UnaffordableOrder()) + + assert len(result.rejected_orders) == 1 + rejected = result.rejected_orders[0] + assert rejected.order_id + assert rejected.asset == "AAPL" + assert rejected.created_at == datetime(2024, 1, 2) + assert rejected.requested_quantity == 1_000_000.0 + assert rejected.status.value == "rejected" + assert rejected.rejection_code == "insufficient_cash" + assert rejected.rejection_reason + assert result.metrics["num_orders"] == 1 + assert result.metrics["num_rejected_orders"] == 1 + assert result.fills == [] + assert result.equity_curve[-1][1] == 10_000.0 + + +def test_no_orders_and_all_orders_rejected_are_distinguishable() -> None: + no_orders = _run(_NoOrders()) + all_rejected = _run(_UnaffordableOrder()) + + assert no_orders.metrics["num_orders"] == 0 + assert no_orders.metrics["num_rejected_orders"] == 0 + assert all_rejected.metrics["num_orders"] == 1 + assert all_rejected.metrics["num_rejected_orders"] == 1 + + +def test_rejected_orders_round_trip_through_result_artifact(tmp_path) -> None: + result = _run(_UnaffordableOrder()) + + frame = result.to_rejected_orders_dataframe() + assert frame.to_dicts() == [ + { + "order_id": result.rejected_orders[0].order_id, + "symbol": "AAPL", + "timestamp": datetime(2024, 1, 2), + "requested_quantity": 1_000_000.0, + "side": "buy", + "order_type": "market", + "limit_price": None, + "stop_price": None, + "trail_amount": None, + "parent_id": None, + "rebalance_id": None, + "status": "rejected", + "rejection_code": "insufficient_cash", + "rejection_reason": result.rejected_orders[0].rejection_reason, + } + ] + + result.to_parquet(tmp_path) + loaded = type(result).from_parquet(tmp_path) + assert loaded.to_rejected_orders_dataframe().to_dicts() == frame.to_dicts() From a2940c2e0db66a497665a7ecb92f8442563d30fc Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 31 Jul 2026 23:25:31 -0400 Subject: [PATCH 4/4] fix(risk): preserve liquidation causes Attach liquidation metadata before order submission and carry the category and detailed cause through fills, trades, DataFrames, and Parquet artifacts. Closes #44. --- src/ml4t/backtest/broker.py | 12 +++-- src/ml4t/backtest/execution/fill_executor.py | 5 ++ src/ml4t/backtest/result.py | 10 ++++ src/ml4t/backtest/types.py | 3 ++ tests/risk/test_portfolio_manager.py | 49 ++++++++++++++++++++ tests/test_result.py | 3 ++ 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/ml4t/backtest/broker.py b/src/ml4t/backtest/broker.py index 1fdcccb6..1e2fc551 100644 --- a/src/ml4t/backtest/broker.py +++ b/src/ml4t/backtest/broker.py @@ -1052,11 +1052,17 @@ def flatten_all_positions( liquidations: list[Order] = [] for asset in list(self.positions): - order = self.close_position(asset, order_type=order_type) + order = self.close_position( + asset, + order_type=order_type, + _options=SubmitOrderOptions( + eligible_in_next_bar_mode=True, + risk_exit_reason=reason, + exit_reason=ExitReason.RISK_LIQUIDATION, + ), + ) if order is None: continue - order._exit_reason = ExitReason.RISK_LIQUIDATION - order._risk_exit_reason = reason liquidations.append(order) return liquidations diff --git a/src/ml4t/backtest/execution/fill_executor.py b/src/ml4t/backtest/execution/fill_executor.py index c65aa6cd..3c1ef77e 100644 --- a/src/ml4t/backtest/execution/fill_executor.py +++ b/src/ml4t/backtest/execution/fill_executor.py @@ -221,6 +221,8 @@ def execute(self, order: Order, base_price: float) -> bool: bid_size=quote_context["bid_size"], ask_size=quote_context["ask_size"], available_size=quote_context["available_size"], + exit_reason=_get_exit_reason(order), + exit_reason_detail=order._risk_exit_reason, ) broker.fills.append(fill) @@ -461,6 +463,7 @@ def _close_position(self, ctx: FillContext, pos: Position, old_qty: float) -> No fees=total_commission, exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(order), + exit_reason_detail=order._risk_exit_reason, mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, @@ -527,6 +530,7 @@ def _flip_position( fees=total_close_commission, exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(order), + exit_reason_detail=order._risk_exit_reason, mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, @@ -628,6 +632,7 @@ def _scale_position( fees=total_commission, exit_slippage=ctx.slippage, exit_reason=_get_exit_reason(ctx.order), + exit_reason_detail=ctx.order._risk_exit_reason, mfe=pos.max_favorable_excursion, mae=pos.max_adverse_excursion, entry_slippage=pos.entry_slippage, diff --git a/src/ml4t/backtest/result.py b/src/ml4t/backtest/result.py index 37c53ab5..f2a0a00d 100644 --- a/src/ml4t/backtest/result.py +++ b/src/ml4t/backtest/result.py @@ -161,6 +161,7 @@ def to_trades_dataframe(self) -> pl.DataFrame: "total_slippage_cost": t.total_slippage_cost, "cost_drag": t.cost_drag, "exit_reason": t.exit_reason, + "exit_reason_detail": t.exit_reason_detail, "status": t.status, } ) @@ -201,6 +202,8 @@ def to_fills_dataframe(self) -> pl.DataFrame: "bid_size": fill.bid_size, "ask_size": fill.ask_size, "available_size": fill.available_size, + "exit_reason": fill.exit_reason, + "exit_reason_detail": fill.exit_reason_detail, } ) @@ -706,6 +709,8 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: fees=fees, exit_slippage=row.get("exit_slippage", row.get("slippage", 0.0)), exit_reason=row.get("exit_reason", "signal"), + exit_reason_detail=row.get("exit_reason_detail"), + status=row.get("status", "closed"), mfe=row["mfe"], mae=row["mae"], entry_slippage=row.get("entry_slippage", 0.0), @@ -766,6 +771,8 @@ def from_parquet(cls, path: str | Path) -> BacktestResult: bid_size=row.get("bid_size"), ask_size=row.get("ask_size"), available_size=row.get("available_size"), + exit_reason=row.get("exit_reason", ""), + exit_reason_detail=row.get("exit_reason_detail"), ) ) @@ -901,6 +908,7 @@ def _trades_schema() -> dict[str, pl.DataType]: "total_slippage_cost": pl.Float64(), "cost_drag": pl.Float64(), "exit_reason": pl.String(), + "exit_reason_detail": pl.String(), "status": pl.String(), # "closed" or "open" } @@ -929,6 +937,8 @@ def _fills_schema() -> dict[str, pl.DataType]: "bid_size": pl.Float64(), "ask_size": pl.Float64(), "available_size": pl.Float64(), + "exit_reason": pl.String(), + "exit_reason_detail": pl.String(), } @staticmethod diff --git a/src/ml4t/backtest/types.py b/src/ml4t/backtest/types.py index 24264fd5..fd7657f5 100644 --- a/src/ml4t/backtest/types.py +++ b/src/ml4t/backtest/types.py @@ -396,6 +396,8 @@ class Fill: bid_size: float | None = None ask_size: float | None = None available_size: float | None = None + exit_reason: str = "" + exit_reason_detail: str | None = None @dataclass @@ -428,6 +430,7 @@ class Trade: exit_slippage: float = 0.0 # Per-unit slippage on exit # Exit reason for trade analysis (cross-library API field) exit_reason: str = "signal" # ExitReason enum value as string + exit_reason_detail: str | None = None # Trade status: "closed" (actually exited) or "open" (mark-to-market at end) status: str = "closed" # MFE/MAE preserved from Position for trade analysis (shorter field names) diff --git a/tests/risk/test_portfolio_manager.py b/tests/risk/test_portfolio_manager.py index 80ef6d8a..1e214303 100644 --- a/tests/risk/test_portfolio_manager.py +++ b/tests/risk/test_portfolio_manager.py @@ -4,6 +4,7 @@ import pytest +from ml4t.backtest import BacktestResult from ml4t.backtest.broker import Broker from ml4t.backtest.models import NoCommission, NoSlippage from ml4t.backtest.risk.portfolio.limits import ( @@ -195,6 +196,54 @@ def test_update_liquidate_action_is_idempotent_with_broker(self): assert len(pending) == 1 assert pending[0]._exit_reason == ExitReason.RISK_LIQUIDATION + @pytest.mark.parametrize("immediate_fill", [True, False]) + def test_liquidation_cause_survives_fill_trade_and_artifact( + self, + immediate_fill: bool, + tmp_path, + ): + manager = RiskManager(limits=[MaxDrawdownLimit(max_drawdown=0.10)]) + manager.initialize(initial_equity=100000.0) + broker = Broker( + initial_cash=100000.0, + commission_model=NoCommission(), + slippage_model=NoSlippage(), + immediate_fill=immediate_fill, + ) + open_long_position(broker, "AAPL", 100.0, 150.0) + + manager.update(equity=85000.0, positions={"AAPL": 15000.0}, broker=broker) + manager.update(equity=84000.0, positions={"AAPL": 15000.0}, broker=broker) + if not immediate_fill: + broker._process_orders() + + liquidation_orders = [ + order for order in broker.orders if order._exit_reason == ExitReason.RISK_LIQUIDATION + ] + assert len(liquidation_orders) == 1 + detail = liquidation_orders[0]._risk_exit_reason + assert detail and "drawdown" in detail.lower() + + exit_fill = broker.fills[-1] + exit_trade = broker.trades[-1] + assert exit_fill.exit_reason == "risk_liquidation" + assert exit_fill.exit_reason_detail == detail + assert exit_trade.exit_reason == "risk_liquidation" + assert exit_trade.exit_reason_detail == detail + + result = BacktestResult( + trades=broker.trades, + equity_curve=[], + fills=broker.fills, + metrics={}, + ) + result.to_parquet(tmp_path) + loaded = BacktestResult.from_parquet(tmp_path) + assert loaded.fills[-1].exit_reason == "risk_liquidation" + assert loaded.fills[-1].exit_reason_detail == detail + assert loaded.trades[-1].exit_reason == "risk_liquidation" + assert loaded.trades[-1].exit_reason_detail == detail + def test_update_warn_action(self): """Test that warn action adds to warnings.""" limits = [MaxExposureLimit(max_exposure_pct=0.50, action="warn")] diff --git a/tests/test_result.py b/tests/test_result.py index d1347538..c8a88c83 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -202,6 +202,7 @@ def test_trades_dataframe_basic(self, backtest_result: BacktestResult): "total_slippage_cost", "cost_drag", "exit_reason", + "exit_reason_detail", "status", ] @@ -336,6 +337,8 @@ def test_fills_dataframe_basic(self, backtest_result: BacktestResult): "bid_size", "ask_size", "available_size", + "exit_reason", + "exit_reason_detail", ] assert df["rebalance_id"].to_list() == ["rebalance-1", "rebalance-1"]