fix(regime): decouple ratio gate from proxy history#659
Conversation
Add a regression test for an uninvested non-anchor symbol in the ratio basket, fetch aligned closes explicitly for the ratio gate, and widen create_limit_order kwargs typing so the ty pre-commit hook passes.
There was a problem hiding this comment.
Pull request overview
This PR adjusts regime-aware rebalancing to avoid coupling the ratio-gate computation to the proxy-series history, adds a regression test for an “uninvested rest symbol” scenario, and fixes a typing issue in order creation so static checking passes.
Changes:
- Update regime proxy-series helper to return only dates/series, and fetch aligned closes explicitly for ratio-gate evaluation.
- Add an async regression test covering a ratio basket that includes an uninvested non-anchor symbol.
- Widen
create_limit_orderkwargs typing to satisfyty/pre-commit checks.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
thetagang/trading_operations.py |
Widens kwargs typing in create_limit_order to satisfy static typing. |
thetagang/strategies/regime_engine.py |
Refactors proxy-series return shape and adds a separate aligned-close fetch for ratio gating. |
thetagang/portfolio_manager.py |
Updates proxy-series wrapper return type to match the regime engine change. |
tests/test_regime_rebalance.py |
Adds a regression test scenario for ratio gate with an uninvested “rest” symbol. |
Comments suppressed due to low confidence (1)
thetagang/strategies/regime_engine.py:96
- In
_get_regime_proxy_series,proxy_symbolsis derived fromweights_override, but_get_regime_aligned_closesis still called with the fullsymbolslist. This means the proxy-series calculation can fail (or do unnecessary IBKR history requests) due to a non-proxy symbol having missing/invalid history, even though that symbol isn't used in the weighted series. Consider fetching aligned closes forproxy_symbolshere (and only requiring history/validation for the symbols actually used in the proxy series).
proxy_symbols = list(weights_override.keys()) if weights_override else symbols
sorted_dates, aligned_closes = await self._get_regime_aligned_closes(
symbols,
lookback_days,
cooldown_days,
| _, aligned_closes = await self._get_regime_aligned_closes( | ||
| symbols, | ||
| regime_rebalance.lookback_days, | ||
| regime_rebalance.cooldown_days, | ||
| ) |
There was a problem hiding this comment.
aligned_closes for the ratio gate is fetched via a second _get_regime_aligned_closes() call, but the returned dates are discarded and the code indexes aligned_closes[*][idx] using idx from dates returned by _get_regime_proxy_series(). Since these are separate IBKR history requests, there’s no guarantee the two date sets/lengths match, which can lead to IndexError or misaligned ratio calculations. Use the dates returned with aligned_closes (or assert they match dates and fail deterministically) before indexing.
| _, aligned_closes = await self._get_regime_aligned_closes( | ||
| symbols, | ||
| regime_rebalance.lookback_days, | ||
| regime_rebalance.cooldown_days, | ||
| ) |
There was a problem hiding this comment.
Enabling ratio_gate now triggers an additional full historical-data fetch for all symbols (on top of the proxy-series history fetch). This increases runtime and the number of IBKR historical requests, which can hit pacing limits. Consider fetching aligned closes once and reusing them for both the proxy-series and ratio-gate computations (or otherwise sharing the fetched histories).
| _, aligned_closes = await self._get_regime_aligned_closes( | |
| symbols, | |
| regime_rebalance.lookback_days, | |
| regime_rebalance.cooldown_days, | |
| ) | |
| if "aligned_closes" not in locals(): | |
| dates, aligned_closes = await self._get_regime_aligned_closes( | |
| symbols, | |
| regime_rebalance.lookback_days, | |
| regime_rebalance.cooldown_days, | |
| ) |
| _, orders = await portfolio_manager.check_regime_rebalance_positions( | ||
| account_summary, portfolio_positions | ||
| ) | ||
|
|
||
| assert isinstance(orders, list) |
There was a problem hiding this comment.
This regression test only asserts isinstance(orders, list), which will pass even if the ratio-gate logic still regresses (e.g., incorrect orders emitted, or the ratio gate being bypassed). Since this PR changes regime/ratio gating behavior, strengthen the assertions to validate the expected order list (symbols + share deltas) for this scenario, so it actually detects future trading-behavior regressions.
Add a regression test for an uninvested non-anchor symbol in the ratio basket, fetch aligned closes explicitly for the ratio gate, and widen create_limit_order kwargs typing so the ty pre-commit hook passes.