Add kill switch liquidation signal handling in strategy executor#912
Add kill switch liquidation signal handling in strategy executor#912
Conversation
danphilos
had a problem deploying
to
unit-tests
GitHub Actions
Failure
- Poll DynamoDB for "liquidating" status during trading iterations - Execute sell_all() when liquidation signal detected - Update DynamoDB status to "liquidated" after positions closed - Add _check_kill_switch_signal() and _handle_kill_switch_liquidation() - Check for signal in main trading loop and iteration start When bot_manager sets status to "liquidating", the bot gracefully closes all positions before being terminated by the kill switch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
When kill switch liquidation triggers, the bot now: 1. Sets stop_event to signal executor shutdown 2. Calls broker.cleanup_streams() to properly close websocket connections This prevents "cannot schedule new futures after shutdown" errors that occurred when the Alpaca websocket stream kept trying to reconnect while the executor was shutting down. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
danphilos
force-pushed
the
fix-kill-switch
branch
from
b463458 to
fcb9035
Compare
📝 WalkthroughWalkthroughAdds a DynamoDB-based kill switch to StrategyExecutor (remote liquidation checks, status updates, and liquidation workflow) and comments out an unused Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Executor as StrategyExecutor
participant DynamoDB as DynamoDB (Kill Switch)
participant Broker as Broker / Trading API
participant Streams as StreamManager
rect rgb(240,248,255)
Executor->>DynamoDB: _check_kill_switch_signal()
end
alt Kill signal present
rect rgb(255,240,240)
DynamoDB-->>Executor: signal=true
Executor->>Broker: liquidate_all_positions()
Broker-->>Executor: positions_sold
Executor->>DynamoDB: _update_status_to_liquidated()
DynamoDB-->>Executor: status_confirmed
Executor->>Streams: cleanup_streams()
Executor->>Executor: stop_executor()
end
else No signal / DynamoDB error
rect rgb(240,255,240)
DynamoDB-->>Executor: no_signal / error
Executor->>Executor: continue_trading()
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (4.0.4)lumibot/tools/thetadata_helper.py************* Module pylintrc ... [truncated 146300 characters] ... 415" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lumibot/strategies/strategy_executor.py (3)
33-40: UnusedClientErrorimport.
ClientErroris imported but all exception handlers use genericException. Consider either removing the unused import or usingClientErrorfor more specific DynamoDB error handling.Option 1: Remove unused import
try: import boto3 - from botocore.exceptions import ClientError BOTO3_AVAILABLE = True except ImportError: BOTO3_AVAILABLE = FalseOption 2: Use ClientError for specific handling
# In _check_kill_switch_signal and other methods: except ClientError as e: error_code = e.response.get('Error', {}).get('Code', '') if error_code == 'ResourceNotFoundException': # Table doesn't exist - log once and disable further checks pass # ... handle other specific cases except Exception as e: # Fallback for non-AWS errors
127-138: Silent exception swallowing loses diagnostic information.When DynamoDB resource creation fails, the exception is silently discarded. This could make it difficult to diagnose connection or credential issues in production. Consider logging a warning.
Proposed fix
if self._kill_switch_bot_id and BOTO3_AVAILABLE: try: self._kill_switch_dynamodb = boto3.resource( "dynamodb", region_name=self._kill_switch_region ) self._kill_switch_table_name = f"{self._kill_switch_environment}-bots" except Exception as e: # Log but don't fail - kill switch is optional - pass + logger.warning(f"Failed to initialize DynamoDB for kill switch: {e}")
21-22: Duplicate import ofAsset.
Assetis imported twice fromlumibot.entities(lines 21 and 22). This appears to be a pre-existing issue or an artifact from the changes.Proposed fix
from lumibot.entities import Asset, Order -from lumibot.entities import Asset
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lumibot/strategies/strategy_executor.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Always useBACKTESTING_DATA_SOURCEenvironment variable to override data source selection; set it to 'thetadata', 'yahoo', or 'none'
Only use Data Downloader athttp://data-downloader.lumiwealth.com:8080for backtests; avoid hard-coded IPs as they can change on redeploy
Always compare ThetaData vs Yahoo prices for validation; Yahoo is the gold standard for split-adjusted prices
UseBACKTESTING_DATA_SOURCE=yahoofor dividend-sensitive strategies as ThetaData has known phantom dividend data quality issues
Files:
lumibot/strategies/strategy_executor.py
🧬 Code graph analysis (1)
lumibot/strategies/strategy_executor.py (2)
lumibot/strategies/strategy.py (3)
log_message(357-403)sell_all(2173-2199)cancel_open_orders(2085-2132)lumibot/brokers/broker.py (2)
sell_all(1577-1602)cancel_open_orders(1521-1553)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
lumibot/strategies/strategy_executor.py (5)
203-240: LGTM!The kill switch signal check is well-structured with appropriate early returns and non-fatal error handling. The
_liquidation_triggeredflag correctly prevents duplicate liquidation attempts.
241-272: LGTM!The status update implementation correctly uses
ExpressionAttributeNamesto avoid reserved word conflicts, and the error handling appropriately logs failures without blocking the shutdown flow.
273-315: Verify behavior on partialsell_allfailure.The implementation correctly prioritizes updating the status to "liquidated" even if
sell_allfails, which signals that liquidation was attempted. However, ifsell_allpartially fails (e.g., some orders rejected), some positions may remain open while status shows "liquidated".Consider whether
bot_managershould verify that positions are actually closed, or if the current best-effort approach is acceptable for your use case.
1038-1047: LGTM!Placing the kill switch check at the start of each trading iteration ensures responsive handling. The early return with proper state cleanup (
_in_trading_iteration = False) is correct.
2041-2049: LGTM!The pre-session kill switch check provides coverage for signals received outside of trading iterations (e.g., before market opens). This complements the per-iteration check nicely for comprehensive signal handling.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lumibot/tools/thetadata_helper.py (1)
3594-3605: Remove unused index helper variables instead of commenting out their consumerCommenting out
is_index_assetfixed one Ruff F841, butasset_type_value,symbol_upper, andindex_symbolsare now also unused and still fail lint/CI. Since this function doesn’t actually need index-specific handling today, it’s cleaner to drop the whole unused block (and avoid commented-out code).Proposed fix to satisfy Ruff F841 and simplify get_missing_dates
- asset_type_value = str(getattr(asset, "asset_type", "")).lower() - symbol_upper = str(getattr(asset, "symbol", "") or "").upper() - index_symbols = { - "SPX", "SPXW", - "RUT", "RUTW", - "VIX", "VIXW", - "NDX", "NDXP", - "XSP", "DJX", "OEX", "XEO", - } - # TODO: Unused variable - commenting out to fix Ruff F841 - # is_index_asset = asset_type_value == "index" or symbol_upper in index_symbols + # NOTE: Index-style assets currently reuse the same missing-date logic as other types. + # If index-specific behavior is needed here in the future, reintroduce an `is_index_asset` + # helper and ensure it’s actually used so Ruff F841 does not trigger.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lumibot/tools/thetadata_helper.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Always useBACKTESTING_DATA_SOURCEenvironment variable to override data source selection; set it to 'thetadata', 'yahoo', or 'none'
Only use Data Downloader athttp://data-downloader.lumiwealth.com:8080for backtests; avoid hard-coded IPs as they can change on redeploy
Always compare ThetaData vs Yahoo prices for validation; Yahoo is the gold standard for split-adjusted prices
UseBACKTESTING_DATA_SOURCE=yahoofor dividend-sensitive strategies as ThetaData has known phantom dividend data quality issues
Files:
lumibot/tools/thetadata_helper.py
lumibot/tools/thetadata_helper.py
📄 CodeRabbit inference engine (CLAUDE.md)
lumibot/tools/thetadata_helper.py: Add idempotency checks and markers to functions that are called multiple times per backtest (e.g.,_apply_corporate_actions_to_frame()with_split_adjustedcolumn marker)
Implement deduplication of dividend events by ex_date in_normalize_dividend_events()to prevent multiple applications of the same dividend
Filter special distributions withless_amount > 0in dividend processing to exclude non-dividend events like return of capital
Convert ThetaData option strike queries using_get_option_query_strike()to account for split adjustments (GOOG 20:1, AAPL 4:1, TSLA 3:1)
Files:
lumibot/tools/thetadata_helper.py
{lumibot/tools/thetadata_helper.py,lumibot/backtesting/thetadata_backtesting_pandas.py}
📄 CodeRabbit inference engine (CLAUDE.md)
Filter zero-price OHLC rows when loading from cache and when receiving new data from ThetaData to prevent ZeroDivisionError
Files:
lumibot/tools/thetadata_helper.py
🧠 Learnings (2)
📚 Learning: 2026-01-05T06:15:00.886Z
Learnt from: CR
Repo: Lumiwealth/lumibot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-05T06:15:00.886Z
Learning: Applies to lumibot/tools/thetadata_helper.py : Add idempotency checks and markers to functions that are called multiple times per backtest (e.g., `_apply_corporate_actions_to_frame()` with `_split_adjusted` column marker)
Applied to files:
lumibot/tools/thetadata_helper.py
📚 Learning: 2026-01-05T06:15:00.886Z
Learnt from: CR
Repo: Lumiwealth/lumibot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-05T06:15:00.886Z
Learning: Applies to lumibot/tools/thetadata_helper.py : Implement deduplication of dividend events by ex_date in `_normalize_dividend_events()` to prevent multiple applications of the same dividend
Applied to files:
lumibot/tools/thetadata_helper.py
🪛 GitHub Actions: LumiBot CI/CD
lumibot/tools/thetadata_helper.py
[error] 3612-3614: F841 Local variable 'asset_type_value' is assigned to but never used; F841 Local variable 'symbol_upper' is assigned to but never used; F841 Local variable 'index_symbols' is assigned to but never used. Remove assignments.
[error] 3612-3614: Three unused variable assignments detected by Ruff: asset_type_value, symbol_upper, index_symbols. Remove them or use them.
[error] 3612-3614: Ruff check failed with 3 linting errors. No fixes available (use --unsafe-fixes to reveal hidden fixes).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
When bot_manager sets status to "liquidating", the bot gracefully closes all positions before being terminated by the kill switch.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.