fix: harden shadow book against book leakage (review of ba2df8b)

Review of the shadow book found seven ways the two books could leak into each other; all are fixed here. The most serious silently invalidated the comparison the shadow book exists to make. - Shadow holdings no longer suppress the manual candidate list. The open-trade exclusion filtered on any book, so shadow taking the top-ranked names removed exactly those from the user's list and alerts, confining the discretionary book to leftovers. Scoped to the manual book. Closed-trade alerts and paper-book equity were leaking the same way and are likewise scoped. - Shadow sizing now matches _simulate_portfolio: min(1% risk, 20% notional cap, available cash) from marked equity, plus the sub- dust guard. Previously risk-only from realized equity, so a tight stop produced a multiples-of-equity leveraged position the strategy would never take. - Shadow only trades setups from the scan that just ran (<6h old) with one setup per ticker. A failed or disabled scan step could otherwise open positions from a prior session at stale prices. - Gate-reset transitions are observed for both books, so a shadow stop-out completes fail -> requalify instead of staying locked forever. - Manual list/close endpoints default to the manual book and reject hand-closing shadow trades; the performance endpoint is scoped to the caller so 'your picks' is not every user's book. - run_shadow_book is registered as a paused job so Admin can trigger it. Also anchors three pre-existing paper-trade tests (and the new alpaca window test) on the UTC date. They build fixtures from the local date but the service stamps opened_at in UTC, so they failed only between 00:00 and 02:00 in a UTC+hh timezone -- latent on ba2df8b, exposed by the clock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 09:11:14 +02:00
parent ba2df8b9fd
commit 247a92a89f
11 changed files with 434 additions and 66 deletions
@@ -0,0 +1,181 @@
+"""The two books must not leak into each other.
+
+Regression cover for the review of ba2df8b. Each test here pins a way the
+shadow book could quietly corrupt the discretionary record — or be corrupted
+by it — which would invalidate the comparison the shadow book exists to make.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime, timedelta, timezone
+
+import pytest
+
+from app.exceptions import ValidationError
+from app.models.paper_trade import PaperTrade
+from app.models.ticker import Ticker
+from app.models.user import User
+from app.services import paper_trade_service as pts
+from app.services.trade_policy import (
+    MANUAL_BOOK,
+    SHADOW_BOOK,
+    get_reentry_gate_locks,
+    observe_reentry_gate_transitions,
+)
+
+
+@pytest.fixture
+async def session():
+    from tests.conftest import _test_session_factory
+
+    async with _test_session_factory() as session:
+        yield session
+
+
+async def _seed(session) -> int:
+    session.add(User(id=1, username="owner", password_hash="x"))
+    session.add(Ticker(id=1, symbol="AAA", name="AAA"))
+    await session.commit()
+    return 1
+
+
+def _trade(*, book, status="open", close_reason=None, closed_at=None, user_id=1):
+    now = datetime.now(timezone.utc)
+    return PaperTrade(
+        user_id=user_id,
+        ticker_id=1,
+        direction="long",
+        entry_price=100.0,
+        shares=10.0,
+        stop_loss=95.0,
+        target=115.0,
+        status=status,
+        opened_at=now - timedelta(days=3),
+        closed_at=closed_at,
+        close_price=95.0 if status == "closed" else None,
+        close_reason=close_reason,
+        book=book,
+    )
+
+
+class TestManualEndpoints:
+    @pytest.mark.asyncio
+    async def test_list_excludes_shadow_by_default(self, session):
+        """Open Positions must not silently mix the autonomous book in."""
+        await _seed(session)
+        session.add_all([_trade(book=MANUAL_BOOK), _trade(book=SHADOW_BOOK)])
+        await session.commit()
+
+        rows = await pts.list_trades(session, user_id=1)
+
+        assert len(rows) == 1
+
+    @pytest.mark.asyncio
+    async def test_list_can_span_both_books_deliberately(self, session):
+        await _seed(session)
+        session.add_all([_trade(book=MANUAL_BOOK), _trade(book=SHADOW_BOOK)])
+        await session.commit()
+
+        rows = await pts.list_trades(session, user_id=1, book=None)
+
+        assert len(rows) == 2
+
+    @pytest.mark.asyncio
+    async def test_shadow_trades_cannot_be_closed_by_hand(self, session):
+        """A hand-closed shadow trade is no longer what the strategy would do."""
+        await _seed(session)
+        trade = _trade(book=SHADOW_BOOK)
+        session.add(trade)
+        await session.commit()
+
+        with pytest.raises(ValidationError, match="exit policy"):
+            await pts.close_trade(session, 1, trade.id, 110.0)
+
+
+class TestGateResetCycle:
+    @pytest.mark.asyncio
+    async def test_shadow_stop_completes_fail_then_requalify(self, session):
+        """The full cycle must advance for the shadow book, or a stopped ticker
+        stays locked out forever."""
+        await _seed(session)
+        closed = datetime.now(timezone.utc) - timedelta(days=2)
+        session.add(
+            _trade(book=SHADOW_BOOK, status="closed", close_reason="stop", closed_at=closed)
+        )
+        await session.commit()
+
+        assert 1 in await get_reentry_gate_locks(session, book=SHADOW_BOOK)
+
+        # Day 1: ticker no longer qualifies → failure observed.
+        await observe_reentry_gate_transitions(
+            session,
+            evaluated_ticker_ids=[1],
+            qualified_ticker_ids=[],
+            observed_at=closed + timedelta(days=1),
+            book=SHADOW_BOOK,
+        )
+        assert 1 in await get_reentry_gate_locks(session, book=SHADOW_BOOK)
+
+        # Day 2: qualifies again → lock releases.
+        await observe_reentry_gate_transitions(
+            session,
+            evaluated_ticker_ids=[1],
+            qualified_ticker_ids=[1],
+            observed_at=closed + timedelta(days=2),
+            book=SHADOW_BOOK,
+        )
+        assert 1 not in await get_reentry_gate_locks(session, book=SHADOW_BOOK)
+
+    @pytest.mark.asyncio
+    async def test_observing_one_book_does_not_move_the_other(self, session):
+        await _seed(session)
+        closed = datetime.now(timezone.utc) - timedelta(days=2)
+        session.add_all(
+            [
+                _trade(book=SHADOW_BOOK, status="closed", close_reason="stop", closed_at=closed),
+                _trade(book=MANUAL_BOOK, status="closed", close_reason="stop", closed_at=closed),
+            ]
+        )
+        await session.commit()
+
+        await observe_reentry_gate_transitions(
+            session,
+            evaluated_ticker_ids=[1],
+            qualified_ticker_ids=[],
+            observed_at=closed + timedelta(days=1),
+            book=SHADOW_BOOK,
+        )
+        await observe_reentry_gate_transitions(
+            session,
+            evaluated_ticker_ids=[1],
+            qualified_ticker_ids=[1],
+            observed_at=closed + timedelta(days=2),
+            book=SHADOW_BOOK,
+        )
+
+        # Shadow released; manual never observed, so it stays locked.
+        assert 1 not in await get_reentry_gate_locks(session, book=SHADOW_BOOK)
+        assert 1 in await get_reentry_gate_locks(session, book=MANUAL_BOOK)
+
+
+class TestPerformanceScoping:
+    @pytest.mark.asyncio
+    async def test_manual_side_is_scoped_to_the_caller(self, session):
+        """'Your picks' must not aggregate another user's discretionary book."""
+        await _seed(session)
+        session.add(User(id=2, username="other", password_hash="x"))
+        await session.commit()
+        session.add_all(
+            [
+                _trade(book=MANUAL_BOOK, user_id=1),
+                _trade(book=MANUAL_BOOK, user_id=2),
+                _trade(book=SHADOW_BOOK, user_id=1),
+            ]
+        )
+        await session.commit()
+
+        mine = await pts.list_trades(session, user_id=1)
+        theirs = await pts.list_trades(session, user_id=2)
+
+        assert len(mine) == 1
+        assert len(theirs) == 1