Skip to content

Add execution layer: order lifecycle management#32

Open
IamJasonBian wants to merge 3 commits into
mainfrom
IamJasonBian/execution-layer
Open

Add execution layer: order lifecycle management#32
IamJasonBian wants to merge 3 commits into
mainfrom
IamJasonBian/execution-layer

Conversation

@IamJasonBian

Copy link
Copy Markdown
Owner

Summary

  • Adds 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 exhausted
  • Wires ExecutionManager into main.py init, run_once(), and process_signal() — orders are registered for lifecycle tracking after placement
  • Adds execution_summary to blob logger for state persistence, send_crossover_alert() to Slack utils, and alpaca-py dependency for Alpaca price feed

New files

File Purpose
execution/__init__.py Package exports
execution/price_discovery.py Aggregates prices from 4 sources, median consensus, outlier flagging
execution/fill_monitor.py Detects fills, cancellations, crossovers (0.1% noise threshold)
execution/slippage_controller.py 3-step repricing: 0.05% → 0.15% → 0.30% cumulative
execution/execution_manager.py Orchestrates submit → monitor → reprice → escalate
execution/order_types.py SmartOrderType enum + TWAP stub

Test plan

  • python -m trading_system.main -v --ticker BTC dry run completes cleanly
  • Price discovery cross-check fires divergence warning (1.24% for BTC stop-limit vs consensus)
  • execution_summary key present in state log JSON with pending contexts
  • Live run: verify register_order_id attaches broker IDs to contexts
  • Multi-tick run: verify crossover detection triggers repricing and Slack escalation

🤖 Generated with Claude Code

Jason Bian and others added 3 commits February 26, 2026 17:56
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>
@IamJasonBian

Copy link
Copy Markdown
Owner Author

Need to confirm fillrate strat

Copy link
Copy Markdown
Owner Author

Code Review — PR #32

Overall: 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

  1. Large surface area — 6 new files + integration into main.py. This PR has been open since Feb 27 and the base is stale. The longer it sits, the harder the merge. Consider whether it can be broken into smaller PRs (e.g., price discovery first, then fill monitoring, then slippage control).

  2. alpaca-py is now a hard dependency — Moved from optional comment to required in requirements.txt. This means all environments (including publish-only Render deploys from PR Add --publish flag for Render publish-only deployment #52) must install it. Verify this is acceptable.

  3. Test plan has unchecked items — The last two items (live run: register_order_id, multi-tick: crossover detection) are still unchecked. Are these blocking or nice-to-have?

  4. Stale base — Base SHA is from Feb 27. Significant changes have landed since (drift comparator, PDT gate, etc.). Recommend rebasing before review to ensure compatibility.


Generated by Claude Code

@IamJasonBian IamJasonBian added the stale label Mar 27, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

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 + main.py integration), a stale base SHA, unchecked live-run test items, and alpaca-py becoming a hard dependency that affects publish-only deploys in #52. Given the size and age, consider splitting into smaller PRs (price discovery → fill monitor → slippage control) or rebasing and completing the live-run test plan.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

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

Copy link
Copy Markdown
Owner Author

Code Deletion Opportunities — PR #32

Focused follow-up on code that can be removed or consolidated.

1. Delete order_types.py (15 lines → 0)

SmartOrderType enum and should_use_twap() are exported in execution/__init__.py but never referenced by any caller. should_use_twap() is hardcoded to return False. The entire file is dead scaffolding — delete it and remove the __init__.py exports.

2. Collapse blob_logger.py parameter sprawl

The same 4 new kwargs (fill_rate, execution_log, gamma_snapshot, execution_summary) are threaded through 4 function signatures (_serialize_state, _log_local, _log_remote, log_state_to_blob). Each function just passes them down unchanged. Replace with a single extras: dict | None = None parameter:

def log_state_to_blob(state_manager, live=False, ..., **extras):
    ...
    snapshot.update(extras)

This removes ~30 lines of boilerplate parameter forwarding and makes adding future fields a zero-diff change.

3. OrderBook overlaps with ExecutionManager

OrderBook.update() diffs open orders across ticks to detect fills/cancellations. ExecutionManager.check_pending_orders() does the same thing for its own _pending contexts. Both maintain parallel state about the same orders. Pick one tracking mechanism — OrderBook is simpler and more general. The ExecutionManager could consume OrderBook.update() tick summaries instead of maintaining its own fill detection.

4. send_crossover_alert is a one-liner wrapper

def send_crossover_alert(message):
    tagged = f"@Jason Bian {message}"
    return send_slack_alert(tagged, emoji=":rotating_light:")

This doesn't justify a new function — inline the two lines at the single call site in ExecutionManager._escalate().

Net reduction potential: ~60 lines deletable, ~30 lines consolidatable


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants