From 14407d37dc3cb6d7aaf06351ab732b7cddcd33f5 Mon Sep 17 00:00:00 2001 From: Viktor Barzin Date: Thu, 4 Jun 2026 21:56:59 +0000 Subject: [PATCH] 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 --- services/kevin_signal_bridge/main.py | 1 + services/trade_executor/main.py | 38 +++- services/trade_executor/risk_manager.py | 57 +++--- shared/schemas/trading.py | 1 + .../services/kevin_signal_bridge/test_main.py | 59 ++++++ tests/services/test_trade_executor.py | 172 ++++++++++++++++++ 6 files changed, 302 insertions(+), 26 deletions(-) diff --git a/services/kevin_signal_bridge/main.py b/services/kevin_signal_bridge/main.py index 2216d0f..ac42a28 100644 --- a/services/kevin_signal_bridge/main.py +++ b/services/kevin_signal_bridge/main.py @@ -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 diff --git a/services/trade_executor/main.py b/services/trade_executor/main.py index a241e7d..abc3996 100644 --- a/services/trade_executor/main.py +++ b/services/trade_executor/main.py @@ -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() diff --git a/services/trade_executor/risk_manager.py b/services/trade_executor/risk_manager.py index 238dfbe..4e4e2a5 100644 --- a/services/trade_executor/risk_manager.py +++ b/services/trade_executor/risk_manager.py @@ -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 diff --git a/shared/schemas/trading.py b/shared/schemas/trading.py index 5c8c7d0..7cca4d5 100644 --- a/shared/schemas/trading.py +++ b/shared/schemas/trading.py @@ -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} diff --git a/tests/services/kevin_signal_bridge/test_main.py b/tests/services/kevin_signal_bridge/test_main.py index a926302..bc7c9c8 100644 --- a/tests/services/kevin_signal_bridge/test_main.py +++ b/tests/services/kevin_signal_bridge/test_main.py @@ -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( diff --git a/tests/services/test_trade_executor.py b/tests/services/test_trade_executor.py index 2ace605..bbaf84f 100644 --- a/tests/services/test_trade_executor.py +++ b/tests/services/test_trade_executor.py @@ -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 # ---------------------------------------------------------------------------