Add execution layer: order lifecycle management#32
Conversation
Track order lifecycle (submitted/filled/cancelled) across ticks by diffing open order IDs, seeding from 7-day history for immediate fill rate. Adds gamma runtime service client for cross-engine price visibility. Both fill rate and gamma snapshot are persisted to blob state logs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ossover alerts Introduces trading_system/execution/ — sits between strategy signals and the broker to handle multi-source price discovery (Twelve Data + Gamma + Alpaca + broker), fill monitoring with crossover detection, staggered repricing within a 0.3% slippage budget, and Slack escalation (@jason Bian) when budget is exhausted. Wired into main.py init/run_once/process_signal and blob logger. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Need to confirm fillrate strat |
Code Review — PR #32Overall: Ambitious execution layer adding order lifecycle management between strategy signals and the broker. The architecture (price discovery → fill monitoring → slippage control → escalation) is well-structured. Concerns
Generated by Claude Code |
|
Stale PR notice — This PR has been open for 44 days with no new commits since Feb 27. The prior review flagged the large surface area (6 new files + Generated by Claude Code |
|
Stale — 8 weeks, no activity since Feb 27. This execution layer PR has 6 new files and unchecked live-run test items. With PR #53 (Executor pattern) and PR #55 (IBKR client) also open, the execution architecture is fragmenting across multiple stale branches. Recommend deciding on a single execution approach, closing superseded PRs, and rebasing the chosen one. Generated by Claude Code |
Code Deletion Opportunities — PR #32Focused follow-up on code that can be removed or consolidated. 1. Delete
|
IamJasonBian
left a comment
There was a problem hiding this comment.
Stale — superseded by PR #53
PR #53 ("Executor pattern, RobinhoodClient, deferred task scheduler") restructures the execution layer with a cleaner Brokerage ABC and Executor pattern that replaces the approach in this PR. The two PRs conflict heavily — merging both would require significant reconciliation.
Recommendation: If the Executor pattern in #53 is the direction forward, close this PR. If there are specific pieces worth keeping (e.g., the PriceDiscovery multi-source aggregation or SlippageController), extract them into #53 or a follow-up, then close this.
Also note: the alpaca-py dependency added here is promoted from optional to required — verify that's intentional since not all environments may have Alpaca credentials.
Generated by Claude Code
Summary
trading_system/execution/— an execution layer between strategy signals and the broker that handles multi-source price discovery (Twelve Data + Gamma + Alpaca + broker), fill monitoring with crossover detection, staggered repricing within a 0.3% slippage budget, and Slack escalation (@jason Bian) when budget is exhaustedExecutionManagerintomain.pyinit,run_once(), andprocess_signal()— orders are registered for lifecycle tracking after placementexecution_summaryto blob logger for state persistence,send_crossover_alert()to Slack utils, andalpaca-pydependency for Alpaca price feedNew files
execution/__init__.pyexecution/price_discovery.pyexecution/fill_monitor.pyexecution/slippage_controller.pyexecution/execution_manager.pyexecution/order_types.pyTest plan
python -m trading_system.main -v --ticker BTCdry run completes cleanlyexecution_summarykey present in state log JSON with pending contextsregister_order_idattaches broker IDs to contexts🤖 Generated with Claude Code