trading/docs/plans/2026-06-04-meet-kevin-trade-execution-fix.md
Viktor Barzin 44132e9961
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
docs(kevin): trade-execution fix plan + go-live
Documents the six fixes (Phases 1-4), the new-only cold-start, the migration handling + pipeline migration-gap, and go-live verification. This commit (no ci skip) triggers the CI build + rollout of the Kevin trading fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-04 22:38:54 +00:00

80 lines
4.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Meet Kevin — trade-execution fix (go-live)
**Date:** 2026-06-04
**Status:** shipped (Phases 14 merged) → deploying
## Problem
Phase 2 "paper trading" was enabled (`TRADING_KEVIN_ENABLE_TRADING=true`) but
was a **silent no-op**: 89 Kevin signals reached the executor and every one was
dropped at sizing. The Alpaca paper account had **zero** Kevin orders (all 50
historical fills were `SIMPLE` orders from the normal signal-generator path).
Six distinct breaks sat between "infer a call from Kevin's video" and "place +
record + manage the trade":
1. Sizing read price only from `sentiment_context["current_price"]` (None for
Kevin) → `qty=0` → skip.
2. The bridge never put a price on the signal.
3. Orders were always built `SIMPLE``stop_loss_pct`/`take_profit_pct` were
ignored, so no brackets.
4. Persisted trades hardcoded `strategy_id=None` → unattributed, invisible to
the dashboard (which filters by Kevin's strategy id).
5. The exit scanner was orphaned — the cron was wired nowhere, so exits never
ran; it also lacked an "already-closed" guard.
6. No realized P&L was ever written, and bracket stop/take-profit legs that
fill at Alpaca never passed through our executor (close + P&L invisible).
## Fixes (TDD throughout)
| Phase | Change | Commit |
|---|---|---|
| 1 | `TradeSignal.current_price`; bridge sets it; sizing honors `target_dollars` and resolves price; BRACKET orders for LONG entries (EXIT stays SIMPLE) | `14407d3` |
| 2 | Persist `strategy_id` on the trade row + execution (record + dashboard) | `a8b0d33` |
| 3 | EXIT closes the full held position; realized P&L on close; wire exit scanner on its cron + offsetting-SELL guard | `52b3c76` |
| 4 | `broker.get_order(nested)` + `list_orders`; `Trade.broker_order_id` (migration `f6a7b8c9d0e1`); reconcile loop books bracket auto-closes + P&L idempotently, syncs status, logs drift | `82dc622` |
Design decision: sizing **honors the bridge's `target_dollars`** rather than
re-deriving in the executor — the bridge already applied conviction-weighting
and per-ticker headroom the executor can't see. Brackets trigger on entry
direction + presence of both pct legs (the bridge stamps pcts even on exits, so
the LONG guard is what keeps exits simple).
## Cold start — "new signals only"
The live state was already new-only at go-live: bridge cursor
`kevin:bridge:last_mention_id = 316` (= MAX mention id, all prior mentions
behind the cursor) and `kevin:deferred_signals` empty. No flush required. The
stale "deferred" backlog seen in `kevin_signal_bridge_state` was audit-row
*classification*, not queued orders.
## Migration handling + a latent gap
The Woodpecker pipeline (`.woodpecker.yml`) has **no migration step** — it runs
tests → builds images → patches the deployment. Migrations only run via the
TF-managed `trading-bot-migrations` Job (`alembic upgrade head`), which the
deploy does not trigger. Migration `f6a7b8c9d0e1` (nullable
`trades.broker_order_id` + index) is backward-compatible, so it was applied to
the live DB ahead of the rollout (atomic DDL + version stamp); the TF Job will
no-op on its next run.
**Recommendation (separate, not done here):** add a `run-migrations` step to the
pipeline (a K8s Job on the freshly-built image, before `update-deployment`) so
future migrations apply automatically on deploy. Requires `create jobs` RBAC for
the Woodpecker service account (infra repo).
## Guardrails (live)
Per-strategy caps already enforced in the risk manager: 10 trades/day, $20k/day
allocation, 20% equity-drawdown halt (permanent pause), 5% daily-loss circuit
(24h pause). Paper account only (`TRADING_PAPER_TRADING=true`).
## Go-live verification
1. CI green; `trading-bot-workers` 6/6 on the new image.
2. Next Kevin video → mention (id > 316) → bridge emits → executor places a
**BRACKET** paper order → `trades` row with `strategy_id = kevin`
dashboard `/meet-kevin/strategy` shows it; `#trading-bot` Slack posts the
entry. (First real trade occurs on Kevin's next upload — the watcher polls
every ~3h.)
3. On a stop/take-profit fill, the reconcile loop books the closing SELL +
realized P&L.