fix: match shadow book to its pipeline's scan by run id, not timestamp
A manually triggered rr_scanner and the scheduled near-close pipeline are separate APScheduler jobs; max_instances=1 serialises a job only against itself, so they can overlap. A manual scan starting just before the pipeline can finish just after it began and overwrite the scan markers. Its completion timestamp is then later than the pipeline start, so the previous 'completed >= pipeline_start' check accepted its batch as though it were the pipeline's own -- exactly when the pipeline's scan may have failed. Replace the timestamp comparison with an exact run-id match. A new pipeline_run module holds a per-task run-id contextvar (separate module so the scanner and scheduler import it without a cycle). _run_pipeline binds a fresh id per invocation; scan_all_tickers stamps that id -- or a fresh one when run standalone -- into the scan markers, written with started/completed in a single commit. The shadow step requires the stored run id to equal its pipeline's id exactly, so a concurrent manual scan (its own id) or a failed pipeline scan (a prior run's id) can never be mistaken for it. Direct Admin triggers have no pipeline context and keep the freshness fallback. Known residual: the id match governs whether shadow proceeds; setup selection remains detected_at >= scan start, so a fully per-run setup isolation would need a run_id column on trade_setups (not required here). Tests cover the reported race (manual scan finishing last is refused), a failed pipeline scan, the id-match accept path, and contextvar propagation and non-leakage across tasks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,72 @@
|
||||
"""Pipeline run-id context and the scanner stamping it into scan markers."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from app.services import pipeline_run
|
||||
|
||||
|
||||
def test_no_run_id_by_default():
|
||||
assert pipeline_run.current() is None
|
||||
|
||||
|
||||
def test_bind_and_release_restore_previous():
|
||||
assert pipeline_run.current() is None
|
||||
token = pipeline_run.bind("run-1")
|
||||
try:
|
||||
assert pipeline_run.current() == "run-1"
|
||||
finally:
|
||||
pipeline_run.release(token)
|
||||
assert pipeline_run.current() is None
|
||||
|
||||
|
||||
def test_new_run_ids_are_unique():
|
||||
ids = {pipeline_run.new_run_id() for _ in range(100)}
|
||||
assert len(ids) == 100
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_id_propagates_to_awaited_coroutines():
|
||||
"""The scan and shadow steps are awaited inside the pipeline's task, so they
|
||||
must observe the id the pipeline bound."""
|
||||
|
||||
async def step() -> str | None:
|
||||
return pipeline_run.current()
|
||||
|
||||
token = pipeline_run.bind("run-42")
|
||||
try:
|
||||
assert await step() == "run-42"
|
||||
finally:
|
||||
pipeline_run.release(token)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_id_does_not_leak_into_an_independent_task():
|
||||
"""A manual scan is a separate APScheduler job, started independently of the
|
||||
pipeline. Modelled here as a task created before the bind: it captures its
|
||||
own context and never observes the id the pipeline binds afterwards."""
|
||||
seen: dict[str, str | None] = {}
|
||||
manual_started = asyncio.Event()
|
||||
let_manual_finish = asyncio.Event()
|
||||
|
||||
async def manual_job() -> None:
|
||||
manual_started.set()
|
||||
await let_manual_finish.wait()
|
||||
seen["manual"] = pipeline_run.current()
|
||||
|
||||
# Created with no id in context — the manual job predates the pipeline bind.
|
||||
task = asyncio.create_task(manual_job())
|
||||
await manual_started.wait()
|
||||
|
||||
token = pipeline_run.bind("pipeline")
|
||||
try:
|
||||
assert pipeline_run.current() == "pipeline"
|
||||
let_manual_finish.set()
|
||||
await task
|
||||
finally:
|
||||
pipeline_run.release(token)
|
||||
|
||||
assert seen["manual"] is None
|
||||
@@ -79,7 +79,13 @@ def _setup(
|
||||
)
|
||||
|
||||
|
||||
async def _mark_scan(session, *, started: datetime, completed: datetime | None = None):
|
||||
async def _mark_scan(
|
||||
session,
|
||||
*,
|
||||
started: datetime,
|
||||
completed: datetime | None = None,
|
||||
run_id: str = "scan-run",
|
||||
):
|
||||
"""Record a successful scan run so the shadow book has something to act on."""
|
||||
from app.services import rr_scanner_service as rr
|
||||
|
||||
@@ -90,6 +96,9 @@ async def _mark_scan(session, *, started: datetime, completed: datetime | None =
|
||||
await shadow_book_service.settings_store.upsert_setting(
|
||||
session, rr.KEY_LAST_SCAN_COMPLETED, completed.isoformat()
|
||||
)
|
||||
await shadow_book_service.settings_store.upsert_setting(
|
||||
session, rr.KEY_LAST_SCAN_RUN_ID, run_id
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
|
||||
@@ -285,40 +294,56 @@ class TestScanFreshness:
|
||||
|
||||
class TestPipelineScanBinding:
|
||||
@pytest.mark.asyncio
|
||||
async def test_fresh_manual_scan_before_pipeline_is_refused(self, session):
|
||||
"""A manual scan at 13:00 is still 'fresh' at 15:30, but the 15:30
|
||||
pipeline's own scan failed. Binding to the pipeline start rejects the
|
||||
13:00 batch — no successful scan happened in *this* pipeline pass."""
|
||||
async def test_pipeline_scan_run_id_match_is_accepted(self, session):
|
||||
"""The scan that stamped this pipeline's run id is the one to act on."""
|
||||
ids = await _seed(session, ["AAA"])
|
||||
pipeline_start = datetime.now(timezone.utc)
|
||||
manual_scan = pipeline_start - timedelta(hours=2, minutes=30)
|
||||
session.add(_setup(ids["AAA"], rank=0.9, detected=manual_scan))
|
||||
now = datetime.now(timezone.utc)
|
||||
session.add(_setup(ids["AAA"], rank=0.9, detected=now))
|
||||
await session.commit()
|
||||
await _mark_scan(session, started=manual_scan - timedelta(minutes=5),
|
||||
completed=manual_scan)
|
||||
await _mark_scan(session, started=now - timedelta(minutes=1),
|
||||
completed=now, run_id="pipeline-A")
|
||||
|
||||
summary = await shadow_book_service.open_shadow_positions(
|
||||
session, activation_config=_CONFIG, require_scan_after=pipeline_start
|
||||
session, activation_config=_CONFIG, expected_run_id="pipeline-A"
|
||||
)
|
||||
|
||||
assert summary["opened"] == 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_concurrent_manual_scan_finishing_last_is_refused(self, session):
|
||||
"""The reported race: a manual rr_scanner overlaps the near-close
|
||||
pipeline and writes the markers last. Its completion timestamp is later
|
||||
than the pipeline start, but its run id is not the pipeline's — so the
|
||||
shadow book must refuse, even though a timestamp check would accept."""
|
||||
ids = await _seed(session, ["AAA"])
|
||||
now = datetime.now(timezone.utc)
|
||||
session.add(_setup(ids["AAA"], rank=0.9, detected=now))
|
||||
await session.commit()
|
||||
# Pipeline expects "pipeline-A"; the manual scan's id won the last write.
|
||||
await _mark_scan(session, started=now - timedelta(minutes=1),
|
||||
completed=now, run_id="manual-999")
|
||||
|
||||
summary = await shadow_book_service.open_shadow_positions(
|
||||
session, activation_config=_CONFIG, expected_run_id="pipeline-A"
|
||||
)
|
||||
|
||||
assert summary["opened"] == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pipeline_scan_after_start_is_accepted(self, session):
|
||||
"""The pipeline's own scan completes just after the pipeline began."""
|
||||
async def test_pipeline_scan_failed_leaves_prior_run_id(self, session):
|
||||
"""If the pipeline's own scan failed, the stored id is a prior run's."""
|
||||
ids = await _seed(session, ["AAA"])
|
||||
pipeline_start = datetime.now(timezone.utc)
|
||||
scan_completed = pipeline_start + timedelta(minutes=1)
|
||||
session.add(_setup(ids["AAA"], rank=0.9, detected=scan_completed))
|
||||
now = datetime.now(timezone.utc)
|
||||
session.add(_setup(ids["AAA"], rank=0.9, detected=now))
|
||||
await session.commit()
|
||||
await _mark_scan(session, started=pipeline_start + timedelta(seconds=1),
|
||||
completed=scan_completed)
|
||||
await _mark_scan(session, started=now - timedelta(minutes=1),
|
||||
completed=now, run_id="yesterday")
|
||||
|
||||
summary = await shadow_book_service.open_shadow_positions(
|
||||
session, activation_config=_CONFIG, require_scan_after=pipeline_start
|
||||
session, activation_config=_CONFIG, expected_run_id="pipeline-today"
|
||||
)
|
||||
|
||||
assert summary["opened"] == 1
|
||||
assert summary["opened"] == 0
|
||||
|
||||
|
||||
class TestLongOnly:
|
||||
|
||||
Reference in New Issue
Block a user