feat(kevin-exec): size from target_dollars, propagate price, bracket entries
Kevin signals never placed orders: the executor sized only from sentiment_context["current_price"] (None for Kevin) so qty=0, and orders were always built SIMPLE (stop/take pcts ignored). - TradeSignal gains `current_price`; the bridge now sets it on publish - risk_manager honors `target_dollars` directly (no strength re-scale) and resolves price from current_price then sentiment_context - executor builds BRACKET orders for LONG entries carrying stop/take pcts; EXIT/SELL signals stay SIMPLE (the bridge sets pcts even on exits) [ci skip] Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
7d1d4464c9
commit
14407d37dc
6 changed files with 302 additions and 26 deletions
|
|
@ -129,6 +129,7 @@ class KevinBridge:
|
|||
target_dollars=decision.target_dollars,
|
||||
stop_loss_pct=Decimal(str(self.config.kevin_stop_loss_pct)),
|
||||
take_profit_pct=Decimal(str(self.config.kevin_take_profit_pct)),
|
||||
current_price=current_price,
|
||||
)
|
||||
|
||||
# Persist to signals table FIRST so downstream FK
|
||||
|
|
|
|||
|
|
@ -56,6 +56,38 @@ async def _next_market_open(broker: AlpacaBroker) -> datetime:
|
|||
return next_open.astimezone(timezone.utc)
|
||||
|
||||
|
||||
def _build_order_request(
|
||||
signal: TradeSignal,
|
||||
side: OrderSide,
|
||||
qty: float,
|
||||
risk_manager: RiskManager,
|
||||
) -> OrderRequest:
|
||||
"""Build a BRACKET order for LONG entries that carry both stop/take
|
||||
percentages; otherwise a SIMPLE order.
|
||||
|
||||
The bridge stamps stop/take pcts even on EXIT signals, so the
|
||||
LONG-direction guard is what keeps exits SIMPLE.
|
||||
"""
|
||||
is_entry = signal.direction == SignalDirection.LONG
|
||||
if (
|
||||
is_entry
|
||||
and signal.stop_loss_pct is not None
|
||||
and signal.take_profit_pct is not None
|
||||
):
|
||||
entry = risk_manager._resolve_price(signal)
|
||||
stop_loss_price = round(entry * (1 - float(signal.stop_loss_pct)), 2)
|
||||
take_profit_price = round(entry * (1 + float(signal.take_profit_pct)), 2)
|
||||
return OrderRequest(
|
||||
ticker=signal.ticker,
|
||||
side=side,
|
||||
qty=float(qty),
|
||||
order_class="bracket",
|
||||
take_profit_price=take_profit_price,
|
||||
stop_loss_price=stop_loss_price,
|
||||
)
|
||||
return OrderRequest(ticker=signal.ticker, side=side, qty=float(qty))
|
||||
|
||||
|
||||
async def process_signal(
|
||||
signal: TradeSignal,
|
||||
risk_manager: RiskManager,
|
||||
|
|
@ -133,11 +165,7 @@ async def process_signal(
|
|||
|
||||
# --- Step 3: create order ---
|
||||
side = OrderSide.BUY if signal.direction == SignalDirection.LONG else OrderSide.SELL
|
||||
order_request = OrderRequest(
|
||||
ticker=signal.ticker,
|
||||
side=side,
|
||||
qty=float(qty),
|
||||
)
|
||||
order_request = _build_order_request(signal, side, qty, risk_manager)
|
||||
|
||||
# --- Step 4: submit order ---
|
||||
start = time.monotonic()
|
||||
|
|
|
|||
|
|
@ -8,7 +8,6 @@ from __future__ import annotations
|
|||
|
||||
import logging
|
||||
from datetime import datetime, timedelta
|
||||
from decimal import Decimal
|
||||
from zoneinfo import ZoneInfo
|
||||
|
||||
from redis.asyncio import Redis
|
||||
|
|
@ -16,7 +15,7 @@ from redis.asyncio import Redis
|
|||
from services.trade_executor.config import TradeExecutorConfig
|
||||
from shared.broker.base import BaseBroker
|
||||
from shared.constants.kevin import KEVIN_STRATEGY_UUID
|
||||
from shared.schemas.trading import AccountInfo, PositionInfo, SignalDirection, TradeSignal
|
||||
from shared.schemas.trading import AccountInfo, TradeSignal
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -185,14 +184,22 @@ class RiskManager:
|
|||
) -> float:
|
||||
"""Calculate the number of shares to buy/sell.
|
||||
|
||||
Uses fixed-fractional sizing: ``equity * max_position_pct``
|
||||
gives the maximum dollar value per position, then scales by
|
||||
signal strength.
|
||||
Two sizing modes:
|
||||
|
||||
* **Pre-computed (Kevin)** — when ``signal.target_dollars`` is set,
|
||||
honor it directly as the position notional. The bridge has already
|
||||
applied conviction-weighting and per-ticker headroom, so we do NOT
|
||||
re-scale by strength.
|
||||
* **Fixed-fractional (legacy)** — ``equity * max_position_pct`` gives
|
||||
the max dollar value per position, then scales by signal strength.
|
||||
|
||||
Price is resolved from ``signal.current_price`` first, falling back to
|
||||
``signal.sentiment_context["current_price"]``.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
signal:
|
||||
The trade signal (includes current price via strength).
|
||||
The trade signal carrying the price and (optionally) sizing.
|
||||
account:
|
||||
Current account info (equity, buying power).
|
||||
|
||||
|
|
@ -201,26 +208,34 @@ class RiskManager:
|
|||
float
|
||||
Number of shares (whole shares).
|
||||
"""
|
||||
if signal.strength <= 0 or account.equity <= 0:
|
||||
if account.equity <= 0:
|
||||
return 0.0
|
||||
|
||||
position_value = account.equity * self.config.max_position_pct
|
||||
position_value *= signal.strength
|
||||
|
||||
# Need a price to compute qty — use the signal's embedded price
|
||||
# or fall back to getting it from the snapshot. For simplicity
|
||||
# the executor will pass the current price through the signal's
|
||||
# sentiment_context or fetch it directly.
|
||||
current_price = 0.0
|
||||
if signal.sentiment_context and "current_price" in signal.sentiment_context:
|
||||
current_price = float(signal.sentiment_context["current_price"])
|
||||
|
||||
if current_price <= 0:
|
||||
price = self._resolve_price(signal)
|
||||
if price <= 0:
|
||||
logger.warning("No current price for %s, cannot size position", signal.ticker)
|
||||
return 0.0
|
||||
|
||||
qty = position_value / current_price
|
||||
return max(int(qty), 0)
|
||||
if signal.target_dollars is not None and signal.target_dollars > 0:
|
||||
position_value = float(signal.target_dollars)
|
||||
else:
|
||||
if signal.strength <= 0:
|
||||
return 0.0
|
||||
position_value = account.equity * self.config.max_position_pct * signal.strength
|
||||
|
||||
return max(int(position_value / price), 0)
|
||||
|
||||
@staticmethod
|
||||
def _resolve_price(signal: TradeSignal) -> float:
|
||||
"""Resolve the sizing/entry price: prefer the dedicated
|
||||
``current_price`` field, fall back to ``sentiment_context``."""
|
||||
if signal.current_price is not None and signal.current_price > 0:
|
||||
return float(signal.current_price)
|
||||
if signal.sentiment_context and "current_price" in signal.sentiment_context:
|
||||
ctx_price = float(signal.sentiment_context["current_price"])
|
||||
if ctx_price > 0:
|
||||
return ctx_price
|
||||
return 0.0
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Internal helpers
|
||||
|
|
|
|||
|
|
@ -125,6 +125,7 @@ class TradeSignal(BaseModel):
|
|||
target_dollars: Decimal | None = None
|
||||
stop_loss_pct: Decimal | None = None
|
||||
take_profit_pct: Decimal | None = None
|
||||
current_price: Decimal | None = None
|
||||
|
||||
model_config = {"from_attributes": True}
|
||||
|
||||
|
|
|
|||
|
|
@ -118,6 +118,65 @@ async def test_bridge_kill_switch_on_publishes_to_stream():
|
|||
cursor.advance.assert_awaited_with(1)
|
||||
|
||||
|
||||
async def test_bridge_attaches_current_price_to_signal():
|
||||
"""The bridge must propagate broker.get_latest_price onto the published
|
||||
TradeSignal so the executor can size the position."""
|
||||
config = MagicMock(
|
||||
kevin_enable_trading=True,
|
||||
kevin_stop_loss_pct=0.08,
|
||||
kevin_take_profit_pct=0.20,
|
||||
kevin_avoid_blocks_days=7,
|
||||
)
|
||||
cursor = AsyncMock()
|
||||
cursor.last_seen_id.return_value = 0
|
||||
publisher = AsyncMock()
|
||||
publisher.publish.return_value = "1234-0"
|
||||
aggregator = AsyncMock()
|
||||
aggregator.fetch_pending.return_value = [
|
||||
MagicMock(
|
||||
id=1,
|
||||
symbol="NVDA",
|
||||
action=MagicMock(value="buy"),
|
||||
conviction=Decimal("0.8"),
|
||||
effective_conviction=Decimal("0.8"),
|
||||
time_horizon=MagicMock(value="weeks"),
|
||||
),
|
||||
]
|
||||
strategy = AsyncMock()
|
||||
from shared.schemas.kevin import KevinDecisionType
|
||||
strategy.evaluate_mention.return_value = MagicMock(
|
||||
decision=KevinDecisionType.OPEN_LONG,
|
||||
symbol="NVDA",
|
||||
target_dollars=Decimal("3000"),
|
||||
holding_days=10,
|
||||
effective_conviction=Decimal("0.8"),
|
||||
rationale="ok",
|
||||
)
|
||||
audit_writer = AsyncMock()
|
||||
broker = AsyncMock()
|
||||
broker.is_asset_tradable.return_value = True
|
||||
broker.get_latest_price.return_value = Decimal("123.45")
|
||||
broker.get_account.return_value = MagicMock(
|
||||
equity=Decimal("100000"), cash=Decimal("100000")
|
||||
)
|
||||
broker.get_positions.return_value = []
|
||||
|
||||
bridge = KevinBridge(
|
||||
config=config,
|
||||
cursor=cursor,
|
||||
publisher=publisher,
|
||||
aggregator=aggregator,
|
||||
strategy=strategy,
|
||||
audit_writer=audit_writer,
|
||||
broker=broker,
|
||||
)
|
||||
await bridge.process_one_pass()
|
||||
|
||||
publisher.publish.assert_awaited_once()
|
||||
published_signal = publisher.publish.call_args[0][0]
|
||||
assert published_signal.current_price == Decimal("123.45")
|
||||
|
||||
|
||||
async def test_bridge_advances_cursor_only_after_publish():
|
||||
"""Race-condition guard: cursor must NOT advance if publish raises."""
|
||||
config = MagicMock(
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ position sizing) and the end-to-end executor flow with a mocked broker.
|
|||
from __future__ import annotations
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from decimal import Decimal
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from zoneinfo import ZoneInfo
|
||||
|
||||
|
|
@ -64,6 +65,31 @@ def _make_signal(
|
|||
)
|
||||
|
||||
|
||||
def _make_kevin_signal(
|
||||
ticker: str = "NVDA",
|
||||
direction: SignalDirection = SignalDirection.LONG,
|
||||
strength: float = 0.8,
|
||||
current_price: Decimal | None = Decimal("100"),
|
||||
target_dollars: Decimal | None = Decimal("2000"),
|
||||
stop_loss_pct: Decimal | None = Decimal("0.08"),
|
||||
take_profit_pct: Decimal | None = Decimal("0.20"),
|
||||
) -> TradeSignal:
|
||||
"""A Kevin-style signal: price on the new ``current_price`` field,
|
||||
pre-computed ``target_dollars``, and stop/take percentages — but NO
|
||||
``sentiment_context``."""
|
||||
return TradeSignal(
|
||||
ticker=ticker,
|
||||
direction=direction,
|
||||
strength=strength,
|
||||
strategy_sources=["kevin:buy:0.8"],
|
||||
current_price=current_price,
|
||||
target_dollars=target_dollars,
|
||||
stop_loss_pct=stop_loss_pct,
|
||||
take_profit_pct=take_profit_pct,
|
||||
timestamp=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
|
||||
def _make_account(equity: float = 100_000.0) -> AccountInfo:
|
||||
return AccountInfo(
|
||||
equity=equity,
|
||||
|
|
@ -373,6 +399,77 @@ class TestPositionSizingRespectsMaxPct:
|
|||
assert qty == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Position sizing — Kevin path (target_dollars + current_price field)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPositionSizingHonorsTargetDollars:
|
||||
"""When the signal carries ``target_dollars`` (Kevin's pre-computed
|
||||
sizing), use it directly and ignore signal strength."""
|
||||
|
||||
def test_target_dollars_drives_qty(self):
|
||||
config = _make_config(max_position_pct=0.05)
|
||||
broker = _mock_broker()
|
||||
rm = RiskManager(config, broker)
|
||||
|
||||
signal = _make_kevin_signal(
|
||||
target_dollars=Decimal("2000"), current_price=Decimal("100")
|
||||
)
|
||||
account = _make_account(equity=100_000)
|
||||
|
||||
qty = rm.calculate_position_size(signal, account)
|
||||
# 2000 / 100 = 20 shares — NOT scaled by strength/max_position_pct.
|
||||
assert qty == 20
|
||||
|
||||
def test_target_dollars_ignores_strength(self):
|
||||
"""qty must be identical regardless of strength when target_dollars is set."""
|
||||
config = _make_config(max_position_pct=0.05)
|
||||
broker = _mock_broker()
|
||||
rm = RiskManager(config, broker)
|
||||
account = _make_account(equity=100_000)
|
||||
|
||||
low = rm.calculate_position_size(
|
||||
_make_kevin_signal(
|
||||
strength=0.1, target_dollars=Decimal("2000"), current_price=Decimal("100")
|
||||
),
|
||||
account,
|
||||
)
|
||||
high = rm.calculate_position_size(
|
||||
_make_kevin_signal(
|
||||
strength=1.0, target_dollars=Decimal("2000"), current_price=Decimal("100")
|
||||
),
|
||||
account,
|
||||
)
|
||||
assert low == high == 20
|
||||
|
||||
|
||||
class TestPositionSizingReadsCurrentPriceField:
|
||||
"""Sizing must read the new ``current_price`` field when
|
||||
``sentiment_context`` is absent (the legacy price source)."""
|
||||
|
||||
def test_current_price_field_used_when_no_sentiment_context(self):
|
||||
config = _make_config(max_position_pct=0.05)
|
||||
broker = _mock_broker()
|
||||
rm = RiskManager(config, broker)
|
||||
|
||||
# Legacy fixed-fractional path (no target_dollars) but price lives
|
||||
# on the new field, not in sentiment_context.
|
||||
signal = _make_kevin_signal(
|
||||
strength=1.0,
|
||||
current_price=Decimal("100"),
|
||||
target_dollars=None,
|
||||
stop_loss_pct=None,
|
||||
take_profit_pct=None,
|
||||
)
|
||||
assert signal.sentiment_context is None
|
||||
account = _make_account(equity=100_000)
|
||||
|
||||
qty = rm.calculate_position_size(signal, account)
|
||||
# 100k * 0.05 * 1.0 = 5000 / 100 = 50 shares.
|
||||
assert qty == 50
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Executor flow — approved signal
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -411,6 +508,81 @@ class TestExecutorFlowApproved:
|
|||
counters["trades_executed"].add.assert_called_once_with(1)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Executor flow — bracket vs simple order construction
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestExecutorBracketOrders:
|
||||
"""LONG entries with both stop/take pcts become BRACKET orders;
|
||||
EXIT signals (or signals missing a pct) stay SIMPLE."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_long_entry_with_pcts_builds_bracket(self):
|
||||
config = _make_config()
|
||||
broker = _mock_broker(positions=[], account=_make_account(100_000))
|
||||
publisher = AsyncMock()
|
||||
publisher.publish = AsyncMock(return_value=b"1-0")
|
||||
counters = {
|
||||
"trades_executed": MagicMock(),
|
||||
"rejections": MagicMock(),
|
||||
"fill_latency": MagicMock(),
|
||||
}
|
||||
|
||||
signal = _make_kevin_signal(
|
||||
ticker="NVDA",
|
||||
direction=SignalDirection.LONG,
|
||||
current_price=Decimal("100"),
|
||||
target_dollars=Decimal("2000"),
|
||||
stop_loss_pct=Decimal("0.08"),
|
||||
take_profit_pct=Decimal("0.20"),
|
||||
)
|
||||
|
||||
with patch.object(RiskManager, "check_risk", return_value=(True, "approved")):
|
||||
await process_signal(signal, RiskManager(config, broker), broker, publisher, counters)
|
||||
|
||||
broker.submit_order.assert_called_once()
|
||||
order_arg = broker.submit_order.call_args[0][0]
|
||||
assert order_arg.order_class == "bracket"
|
||||
assert order_arg.side == OrderSide.BUY
|
||||
# entry=100 → stop=100*(1-0.08)=92.0, take=100*(1+0.20)=120.0
|
||||
assert order_arg.stop_loss_price == 92.0
|
||||
assert order_arg.take_profit_price == 120.0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exit_signal_with_pcts_stays_simple(self):
|
||||
"""The bridge stamps stop/take pcts even on EXIT signals; the
|
||||
direction guard must keep the resulting SELL order SIMPLE."""
|
||||
config = _make_config()
|
||||
broker = _mock_broker(positions=[], account=_make_account(100_000))
|
||||
publisher = AsyncMock()
|
||||
publisher.publish = AsyncMock(return_value=b"1-0")
|
||||
counters = {
|
||||
"trades_executed": MagicMock(),
|
||||
"rejections": MagicMock(),
|
||||
"fill_latency": MagicMock(),
|
||||
}
|
||||
|
||||
signal = _make_kevin_signal(
|
||||
ticker="NVDA",
|
||||
direction=SignalDirection.EXIT,
|
||||
current_price=Decimal("100"),
|
||||
target_dollars=Decimal("2000"),
|
||||
stop_loss_pct=Decimal("0.08"),
|
||||
take_profit_pct=Decimal("0.20"),
|
||||
)
|
||||
|
||||
with patch.object(RiskManager, "check_risk", return_value=(True, "approved")):
|
||||
await process_signal(signal, RiskManager(config, broker), broker, publisher, counters)
|
||||
|
||||
broker.submit_order.assert_called_once()
|
||||
order_arg = broker.submit_order.call_args[0][0]
|
||||
assert order_arg.order_class == "simple"
|
||||
assert order_arg.side == OrderSide.SELL
|
||||
assert order_arg.take_profit_price is None
|
||||
assert order_arg.stop_loss_price is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Executor flow — rejected signal
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue