Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions application/execution_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from dataclasses import dataclass
from typing import Any

from quant_platform_kit.common.order_status import compute_confirmed_sell_release_value
from quant_platform_kit.common.models import OrderIntent
from quant_platform_kit.common.ports import ExecutionPort, MarketDataPort
try:
Expand Down Expand Up @@ -573,6 +574,7 @@ def execute_value_target_plan(
safe_haven_cash_substitute_threshold_usd: float = DEFAULT_SAFE_HAVEN_CASH_SUBSTITUTE_THRESHOLD_USD,
cash_only_execution: bool = True,
notional_buy_execution: bool = False,
fetch_order_status=None,
) -> ExecutionCycleResult:
del dry_run_only # ExecutionPort owns preview vs live submission.
plan = substitute_small_safe_haven_targets_with_cash(
Expand Down Expand Up @@ -628,6 +630,7 @@ def execute_value_target_plan(
skipped: list[dict[str, Any]] = []
reference_prices: dict[str, float] = {}
pending_sell_release_symbols: list[str] = []
submitted_sell_orders: list[dict[str, Any]] = []
sell_submitted = False

tradable_deltas: list[tuple[str, float, float]] = []
Expand Down Expand Up @@ -687,21 +690,35 @@ def execute_value_target_plan(
max_notional_usd=max_order_notional_usd,
)
)
submitted_sell_orders.append(submitted[-1])
pending_sell_release_symbols.append(symbol)
sell_submitted = True
investable_cash += quantity * sell_limit_price
continue

confirmed_sell_release_value = compute_confirmed_sell_release_value(
submitted_sell_orders=submitted_sell_orders,
fetch_order_status=fetch_order_status,
Comment on lines +698 to +700

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require fetched fills before releasing sell proceeds

Here the release calculation receives the just-submitted sell order, so when get_order_status cannot find the order yet (common immediately after submitting a live limit sell, and also for preview/dry-run payloads), the pinned helper falls back to the submission payload/order dict. Firstrade submission payloads contain the requested shares/limit_price while still pending, which can be credited as released cash and allow the later buy loop to spend proceeds from an unfilled sell; only fetched fill data (or a status explicitly confirmed as filled) should be used for this value.

Useful? React with 👍 / 👎.

)
investable_cash = max(
0.0,
float(execution.get("investable_cash") or portfolio.get("liquid_cash") or 0.0)
+ confirmed_sell_release_value,
)

buy_deltas = [item for item in tradable_deltas if item[1] > 0]
_buys_blocked_reason: str | None = None
if cash_only_execution and buy_deltas and pending_sell_release_symbols:
estimated_buy_cost = 0.0
requires_pending_sell_release = False
for symbol, delta_value, price in buy_deltas:
buy_budget = min(float(delta_value), investable_cash)
if order_notional_cap is not None:
buy_budget = min(buy_budget, order_notional_cap)
if notional_buy_execution:
if buy_budget >= MIN_NOTIONAL_BUY_USD:
estimated_buy_cost += buy_budget
elif float(delta_value) >= MIN_NOTIONAL_BUY_USD:
requires_pending_sell_release = True
else:
limit_price = _limit_buy_price(
symbol, price, limit_buy_premium, limit_buy_premium_by_symbol
Expand All @@ -718,7 +735,9 @@ def execute_value_target_plan(
)
if quantity > 0:
estimated_buy_cost += quantity * limit_price
if estimated_buy_cost > investable_cash:
elif float(delta_value) > 0.0 and limit_price > max(0.0, buy_budget):
requires_pending_sell_release = True
if estimated_buy_cost > investable_cash or requires_pending_sell_release:
Comment on lines +738 to +740

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow affordable buys after zero-quantity sells

When a negative delta floors to zero shares, this function still adds that symbol to pending_sell_release_symbols above even though no sell order was submitted. In that state, any later buy whose limit price is above its current budget sets requires_pending_sell_release here, and the condition below clears every buy_delta, so buys that were fully affordable from existing cash are skipped as pending_sell_release even though no sell can ever release funds. Limit this check to actually submitted pending sells, or skip only the buy that needs unconfirmed proceeds.

Useful? React with 👍 / 👎.

_buys_blocked_reason = "pending_sell_release"
for symbol, _delta_value, _price in buy_deltas:
skipped.append(
Expand Down
101 changes: 101 additions & 0 deletions application/firstrade_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from time import time
from typing import Any, Callable

from application.account_payload_utils import flatten_values, float_or_none
from application.state_persistence import GcsStateStore


Expand Down Expand Up @@ -447,6 +448,68 @@ def get_positions(self, account: str) -> dict[str, Any]:
_, account_data = self.require_connected()
return dict(account_data.get_positions(account))

def get_orders(self, account: str, *, per_page: int = 0) -> list[dict[str, Any]]:
_, account_data = self.require_connected()
payload = account_data.get_orders(account, per_page=per_page)
if isinstance(payload, list):
return [dict(row) for row in payload if isinstance(row, dict)]
if isinstance(payload, dict):
for key in ("items", "orders", "data", "result"):
value = payload.get(key)
if isinstance(value, list):
return [dict(row) for row in value if isinstance(row, dict)]
return []

def get_order_status(self, account: str, order_id: str) -> dict[str, Any] | None:
normalized_order_id = str(order_id or "").strip()
if not normalized_order_id:
return None
for row in self.get_orders(account):
if not _payload_contains_order_id(row, normalized_order_id):
continue
status = _first_text_from_payload(
row,
"status",
"order_status",
"state",
"status_description",
"description",
)
executed_qty = _first_numeric_from_payload(
row,
"executed_qty",
"executed_quantity",
"filled_quantity",
"filled_qty",
"filled",
"filled_shares",
"executed_shares",
"quantity_filled",
"quantity",
"shares",
"qty",
Comment on lines +488 to +490

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep requested shares out of executed quantity

Even when get_order_status does find an order-status row, these fallback aliases treat the order's requested quantity/shares as executed_qty. For a live sell that is still open or pending, Firstrade status rows commonly still include the requested share count, and the release helper accepts a positive executed quantity even without a confirmed fill status; this can again credit unfilled sell proceeds and submit buys against cash that has not settled into the account.

Useful? React with 👍 / 👎.

)
executed_price = _first_numeric_from_payload(
row,
"executed_price",
"average_fill_price",
"avg_fill_price",
"avg_price",
"average_price",
"fill_price",
"filled_price",
"price",
"limit_price",
)
return {
"status": status or "",
"executed_qty": max(0.0, float(executed_qty or 0.0)),
"executed_price": max(0.0, float(executed_price or 0.0)),
"broker_order_id": normalized_order_id,
"raw_payload": dict(row),
}
return None

def get_quote(self, account: str, symbol: str) -> dict[str, Any]:
session, _ = self.require_connected()
quote_factory = self._quote_factory
Expand Down Expand Up @@ -533,3 +596,41 @@ def place_stock_order(
notional=notional,
)
)


def _sanitize_payload_key(value: Any) -> str:
return "".join(ch for ch in str(value or "").lower() if ch.isalnum())


def _first_payload_value(payload: Any, *candidate_keys: str) -> Any:
flattened = flatten_values(payload)
candidates = {_sanitize_payload_key(key) for key in candidate_keys}
for key, value in flattened.items():
if _sanitize_payload_key(key.rsplit(".", 1)[-1]) in candidates:
return value
Comment on lines +608 to +610

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor candidate priority when reading order fields

This helper receives candidates in priority order, but it returns whichever matching key appears first in Firstrade's payload. When an order-status row includes both a generic field such as description/limit_price and the real status/avg_price, an earlier generic key shadows the fill field, so confirmed fills can be treated as unfilled or released at the wrong price. Iterate candidate keys by priority instead of payload order so the explicit fill/status fields win.

Useful? React with 👍 / 👎.

return None


def _first_text_from_payload(payload: Any, *candidate_keys: str) -> str | None:
value = _first_payload_value(payload, *candidate_keys)
text = str(value or "").strip()
return text or None


def _first_numeric_from_payload(payload: Any, *candidate_keys: str) -> float | None:
return float_or_none(_first_payload_value(payload, *candidate_keys))


def _payload_contains_order_id(payload: Any, order_id: str) -> bool:
normalized_order_id = str(order_id or "").strip()
if not normalized_order_id:
return False
for key, value in flatten_values(payload).items():
key_normalized = _sanitize_payload_key(key)
if "order" not in key_normalized:
continue
if not any(token in key_normalized for token in ("id", "number", "orderno")):
continue
if str(value or "").strip() == normalized_order_id:
return True
return False
1 change: 1 addition & 0 deletions application/rebalance_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ def log_message(message: str) -> None:
safe_haven_cash_substitute_threshold_usd=settings.safe_haven_cash_substitute_threshold_usd,
cash_only_execution=settings.cash_only_execution,
notional_buy_execution=notional_buy_execution_enabled(settings.strategy_profile),
fetch_order_status=lambda broker_order_id: client.get_order_status(account, broker_order_id),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle order-status lookup failures after sells

When client.get_order_status raises (for example, the Firstrade order-status endpoint times out or returns an unexpected payload), that exception bubbles out of execute_value_target_plan after sell orders may already have been submitted. In that scenario the run aborts before producing the submitted/skipped order summary or completed strategy-run state; the safer behavior for an unavailable status lookup is to treat the confirmed release as zero and skip dependent buys instead of crashing the cycle.

Useful? React with 👍 / 👎.

)
submitted_orders = list(execution_result.submitted_orders)
skipped_orders = list(execution_result.skipped_orders)
Expand Down
15 changes: 15 additions & 0 deletions application/runtime_broker_adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from application.account_payload_utils import (
first_numeric_by_keywords,
flatten_values,
float_or_none,
get_first,
iter_position_rows,
Expand Down Expand Up @@ -63,6 +64,19 @@ def _utcnow() -> datetime:
)


def _extract_broker_order_id(payload) -> str | None:
for key, value in flatten_values(payload).items():
normalized = "".join(ch for ch in str(key or "").lower() if ch.isalnum())
if "order" not in normalized:
continue
if not any(token in normalized for token in ("id", "number", "orderno")):
continue
text = str(value or "").strip()
if text:
return text
return None


def _market_date(value: datetime) -> date:
normalized = value if value.tzinfo is not None else value.replace(tzinfo=timezone.utc)
return normalized.astimezone(_NEW_YORK_TZ).date()
Expand Down Expand Up @@ -326,6 +340,7 @@ def submit(order_intent) -> ExecutionReport:
side=request.side,
quantity=float(request.quantity or request.notional_usd or 0),
status="previewed" if not self.live_orders else "submitted",
broker_order_id=_extract_broker_order_id(raw),
raw_payload=raw,
)

Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ dependencies = [
"pytest",
"pytz",
"requests",
"quant-platform-kit @ git+https://github.com/QuantStrategyLab/QuantPlatformKit.git@69a0256934d081b5ef309a885384b9eb9f62cf90",
"quant-platform-kit @ git+https://github.com/QuantStrategyLab/QuantPlatformKit.git@2381aa4577e9fd6329053a73a1c888929170eaf3",
"us-equity-strategies @ git+https://github.com/QuantStrategyLab/UsEquityStrategies.git@17ddb86c72d44b2c7b78ba7a10d8f71b21180166",
]
license = "MIT"
Expand Down Expand Up @@ -82,5 +82,5 @@ show_missing = true

[tool.uv]
override-dependencies = [
"quant-platform-kit @ git+https://github.com/QuantStrategyLab/QuantPlatformKit.git@69a0256934d081b5ef309a885384b9eb9f62cf90",
"quant-platform-kit @ git+https://github.com/QuantStrategyLab/QuantPlatformKit.git@2381aa4577e9fd6329053a73a1c888929170eaf3",
]
2 changes: 1 addition & 1 deletion qsl.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ upgrade_ring = "ring_d"
allow_legacy = false

[qsl.requires]
quant_platform_kit = "69a0256934d081b5ef309a885384b9eb9f62cf90"
quant_platform_kit = "2381aa4577e9fd6329053a73a1c888929170eaf3"
us_equity_strategies = "17ddb86c72d44b2c7b78ba7a10d8f71b21180166"

[qsl.compat]
Expand Down
46 changes: 46 additions & 0 deletions tests/test_execution_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def submit_order(self, order_intent) -> ExecutionReport:
side=order_intent.side,
quantity=order_intent.quantity,
status="previewed",
broker_order_id=f"OID-{len(self.orders)}",
raw_payload={
"limit_price": order_intent.limit_price,
"max_notional_usd": order_intent.metadata.get("max_notional_usd"),
Expand Down Expand Up @@ -139,6 +140,11 @@ def test_execute_value_target_plan_tops_up_existing_whole_share_when_target_roun
execution_port=execution_port,
dry_run_only=True,
limit_buy_premium=1.0,
fetch_order_status=lambda broker_order_id: {
"status": "Filled" if broker_order_id else "",
"executed_qty": 3.0,
"executed_price": 40.0,
},
)

assert result.action_done is True
Expand All @@ -148,6 +154,41 @@ def test_execute_value_target_plan_tops_up_existing_whole_share_when_target_roun
]


def test_execute_value_target_plan_defers_buy_until_sell_release_is_confirmed():
execution_port = FakeExecutionPort()
result = execute_value_target_plan(
plan={
"allocation": {
"targets": {"SOXL": 0.0, "SOXX": 260.0},
"risk_symbols": ("SOXL", "SOXX"),
},
"portfolio": {
"market_values": {"SOXL": 120.0, "SOXX": 200.0},
"quantities": {"SOXL": 3.0, "SOXX": 2.0},
"sellable_quantities": {"SOXL": 3.0, "SOXX": 2.0},
"liquid_cash": 10.0,
},
"execution": {"current_min_trade": 10.0, "investable_cash": 10.0},
},
market_data_port=FakeMarketDataPort({"SOXL": 40.0, "SOXX": 100.0}),
execution_port=execution_port,
dry_run_only=True,
limit_buy_premium=1.0,
)

assert result.action_done is True
assert [(order.side, order.symbol, order.quantity) for order in execution_port.orders] == [
("sell", "SOXL", 3.0),
]
assert result.skipped_orders == (
{
"symbol": "SOXX",
"reason": "pending_sell_release",
"pending_sell_symbols": ["SOXL"],
},
)


def test_execute_value_target_plan_skips_when_cap_cannot_buy_one_share():
execution_port = FakeExecutionPort()
result = execute_value_target_plan(
Expand Down Expand Up @@ -289,6 +330,11 @@ def test_execute_value_target_plan_projects_unbuyable_value_target_to_zero():
dry_run_only=True,
max_order_notional_usd=1000.0,
safe_haven_cash_substitute_threshold_usd=1000.0,
fetch_order_status=lambda broker_order_id: {
"status": "Filled" if broker_order_id else "",
"executed_qty": 1.0,
"executed_price": 536.88,
},
)

assert result.action_done is True
Expand Down
41 changes: 41 additions & 0 deletions tests/test_firstrade_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def get_account_balances(self, account):
def get_positions(self, _account):
return {"items": [{"symbol": "SPY", "quantity": "1", "market_value": "500"}]}

def get_orders(self, _account, per_page=0):
del per_page
return []


class FakeOrder:
def __init__(self, _session):
Expand Down Expand Up @@ -151,6 +155,43 @@ def test_client_order_preview_uses_dry_run_by_default():
assert response["price"] == 5.0


def test_get_order_status_normalizes_matching_order_payload():
class OrdersAccountData(FakeAccountData):
def get_orders(self, _account, per_page=0):
del per_page
return [
{
"order_id": "OID-123",
"status": "Filled",
"filled_quantity": "3",
"avg_price": "101.25",
}
]

credentials = FirstradeCredentials(username="user", password="pass")
client = FirstradeBrokerClient(
credentials,
session_factory=FakeSession,
account_data_factory=OrdersAccountData,
order_factory=FakeOrder,
).connect()

status = client.get_order_status("12345678", "OID-123")

assert status == {
"status": "Filled",
"executed_qty": 3.0,
"executed_price": 101.25,
"broker_order_id": "OID-123",
"raw_payload": {
"order_id": "OID-123",
"status": "Filled",
"filled_quantity": "3",
"avg_price": "101.25",
},
}


def test_get_balances_includes_account_list_total_value():
class BalancesWithoutTotalAccountData(FakeAccountData):
account_balances = {"12345678": "$987.65"}
Expand Down
3 changes: 3 additions & 0 deletions tests/test_rebalance_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ def get_quote(self, _account, symbol):
def get_ohlc(self, _symbol, _range):
return [(1700000000000 + index * 86400000, 9, 11, 8, 10 + index, 1000) for index in range(5)]

def get_order_status(self, _account, _order_id):
return None

def place_stock_order(self, request, dry_run=True, explicit_live_ack=False):
self.orders.append((request, dry_run, explicit_live_ack))
return {
Expand Down
Loading
Loading