fix: PR review — DB reload, market-local date, market-scoped scan_candidates
Some checks failed
CI / test (pull_request) Has been cancelled
Some checks failed
CI / test (pull_request) Has been cancelled
Address PR #110 review findings: 1. High — Realtime mode now loads playbook from DB before calling Gemini, preventing duplicate API calls on process restart (4/day budget). 2. Medium — Pass market-local date (via market.timezone) to generate_playbook() and _empty_playbook() instead of date.today(). 3. Medium — scan_candidates restructured from {stock_code: candidate} to {market_code: {stock_code: candidate}} to prevent KR/US symbol collision. New test: test_scan_candidates_market_scoped verifies cross-market isolation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
113
src/main.py
113
src/main.py
@@ -10,7 +10,7 @@ import argparse
|
|||||||
import asyncio
|
import asyncio
|
||||||
import logging
|
import logging
|
||||||
import signal
|
import signal
|
||||||
from datetime import UTC, date, datetime
|
from datetime import UTC, datetime
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from src.analysis.scanner import MarketScanner
|
from src.analysis.scanner import MarketScanner
|
||||||
@@ -89,7 +89,7 @@ async def trading_cycle(
|
|||||||
telegram: TelegramClient,
|
telegram: TelegramClient,
|
||||||
market: MarketInfo,
|
market: MarketInfo,
|
||||||
stock_code: str,
|
stock_code: str,
|
||||||
scan_candidates: dict[str, ScanCandidate],
|
scan_candidates: dict[str, dict[str, ScanCandidate]],
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Execute one trading cycle for a single stock."""
|
"""Execute one trading cycle for a single stock."""
|
||||||
cycle_start_time = asyncio.get_event_loop().time()
|
cycle_start_time = asyncio.get_event_loop().time()
|
||||||
@@ -148,7 +148,8 @@ async def trading_cycle(
|
|||||||
}
|
}
|
||||||
|
|
||||||
# Enrich market_data with scanner metrics for scenario engine
|
# Enrich market_data with scanner metrics for scenario engine
|
||||||
candidate = scan_candidates.get(stock_code)
|
market_candidates = scan_candidates.get(market.code, {})
|
||||||
|
candidate = market_candidates.get(stock_code)
|
||||||
if candidate:
|
if candidate:
|
||||||
market_data["rsi"] = candidate.rsi
|
market_data["rsi"] = candidate.rsi
|
||||||
market_data["volume_ratio"] = candidate.volume_ratio
|
market_data["volume_ratio"] = candidate.volume_ratio
|
||||||
@@ -315,8 +316,8 @@ async def trading_cycle(
|
|||||||
|
|
||||||
# 6. Log trade with selection context
|
# 6. Log trade with selection context
|
||||||
selection_context = None
|
selection_context = None
|
||||||
if stock_code in scan_candidates:
|
if stock_code in market_candidates:
|
||||||
candidate = scan_candidates[stock_code]
|
candidate = market_candidates[stock_code]
|
||||||
selection_context = {
|
selection_context = {
|
||||||
"rsi": candidate.rsi,
|
"rsi": candidate.rsi,
|
||||||
"volume_ratio": candidate.volume_ratio,
|
"volume_ratio": candidate.volume_ratio,
|
||||||
@@ -386,10 +387,11 @@ async def run_daily_session(
|
|||||||
|
|
||||||
logger.info("Starting daily trading session for %d markets", len(open_markets))
|
logger.info("Starting daily trading session for %d markets", len(open_markets))
|
||||||
|
|
||||||
today = date.today()
|
|
||||||
|
|
||||||
# Process each open market
|
# Process each open market
|
||||||
for market in open_markets:
|
for market in open_markets:
|
||||||
|
# Use market-local date for playbook keying
|
||||||
|
market_today = datetime.now(market.timezone).date()
|
||||||
|
|
||||||
# Dynamic stock discovery via scanner (no static watchlists)
|
# Dynamic stock discovery via scanner (no static watchlists)
|
||||||
candidates_list: list[ScanCandidate] = []
|
candidates_list: list[ScanCandidate] = []
|
||||||
try:
|
try:
|
||||||
@@ -406,13 +408,13 @@ async def run_daily_session(
|
|||||||
logger.info("Processing market: %s (%d stocks)", market.name, len(watchlist))
|
logger.info("Processing market: %s (%d stocks)", market.name, len(watchlist))
|
||||||
|
|
||||||
# Generate or load playbook (1 Gemini API call per market per day)
|
# Generate or load playbook (1 Gemini API call per market per day)
|
||||||
playbook = playbook_store.load(today, market.code)
|
playbook = playbook_store.load(market_today, market.code)
|
||||||
if playbook is None:
|
if playbook is None:
|
||||||
try:
|
try:
|
||||||
playbook = await pre_market_planner.generate_playbook(
|
playbook = await pre_market_planner.generate_playbook(
|
||||||
market=market.code,
|
market=market.code,
|
||||||
candidates=candidates_list,
|
candidates=candidates_list,
|
||||||
today=today,
|
today=market_today,
|
||||||
)
|
)
|
||||||
playbook_store.save(playbook)
|
playbook_store.save(playbook)
|
||||||
try:
|
try:
|
||||||
@@ -436,7 +438,7 @@ async def run_daily_session(
|
|||||||
)
|
)
|
||||||
except Exception as notify_exc:
|
except Exception as notify_exc:
|
||||||
logger.warning("Playbook failed notification error: %s", notify_exc)
|
logger.warning("Playbook failed notification error: %s", notify_exc)
|
||||||
playbook = PreMarketPlanner._empty_playbook(today, market.code)
|
playbook = PreMarketPlanner._empty_playbook(market_today, market.code)
|
||||||
|
|
||||||
# Collect market data for all stocks from scanner
|
# Collect market data for all stocks from scanner
|
||||||
stocks_data = []
|
stocks_data = []
|
||||||
@@ -849,8 +851,8 @@ async def run(settings: Settings) -> None:
|
|||||||
settings=settings,
|
settings=settings,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Track scan candidates for selection context logging
|
# Track scan candidates per market for selection context logging
|
||||||
scan_candidates: dict[str, ScanCandidate] = {} # stock_code -> candidate
|
scan_candidates: dict[str, dict[str, ScanCandidate]] = {} # market -> {stock_code -> candidate}
|
||||||
|
|
||||||
# Active stocks per market (dynamically discovered by scanner)
|
# Active stocks per market (dynamically discovered by scanner)
|
||||||
active_stocks: dict[str, list[str]] = {} # market_code -> [stock_codes]
|
active_stocks: dict[str, list[str]] = {} # market_code -> [stock_codes]
|
||||||
@@ -1021,9 +1023,10 @@ async def run(settings: Settings) -> None:
|
|||||||
candidates
|
candidates
|
||||||
)
|
)
|
||||||
|
|
||||||
# Store candidates for selection context logging
|
# Store candidates per market for selection context logging
|
||||||
for candidate in candidates:
|
scan_candidates[market.code] = {
|
||||||
scan_candidates[candidate.stock_code] = candidate
|
c.stock_code: c for c in candidates
|
||||||
|
}
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"Smart Scanner: Found %d candidates for %s: %s",
|
"Smart Scanner: Found %d candidates for %s: %s",
|
||||||
@@ -1032,43 +1035,61 @@ async def run(settings: Settings) -> None:
|
|||||||
[f"{c.stock_code}(RSI={c.rsi:.0f})" for c in candidates],
|
[f"{c.stock_code}(RSI={c.rsi:.0f})" for c in candidates],
|
||||||
)
|
)
|
||||||
|
|
||||||
# Generate playbook on first scan (1 Gemini call per market)
|
# Get market-local date for playbook keying
|
||||||
|
market_today = datetime.now(
|
||||||
|
market.timezone
|
||||||
|
).date()
|
||||||
|
|
||||||
|
# Load or generate playbook (1 Gemini call per market per day)
|
||||||
if market.code not in playbooks:
|
if market.code not in playbooks:
|
||||||
try:
|
# Try DB first (survives process restart)
|
||||||
pb = await pre_market_planner.generate_playbook(
|
stored_pb = playbook_store.load(market_today, market.code)
|
||||||
market=market.code,
|
if stored_pb is not None:
|
||||||
candidates=candidates,
|
playbooks[market.code] = stored_pb
|
||||||
|
logger.info(
|
||||||
|
"Loaded existing playbook for %s from DB"
|
||||||
|
" (%d stocks, %d scenarios)",
|
||||||
|
market.code,
|
||||||
|
stored_pb.stock_count,
|
||||||
|
stored_pb.scenario_count,
|
||||||
)
|
)
|
||||||
playbook_store.save(pb)
|
else:
|
||||||
playbooks[market.code] = pb
|
|
||||||
try:
|
try:
|
||||||
await telegram.notify_playbook_generated(
|
pb = await pre_market_planner.generate_playbook(
|
||||||
market=market.code,
|
market=market.code,
|
||||||
stock_count=pb.stock_count,
|
candidates=candidates,
|
||||||
scenario_count=pb.scenario_count,
|
today=market_today,
|
||||||
token_count=pb.token_count,
|
|
||||||
)
|
)
|
||||||
|
playbook_store.save(pb)
|
||||||
|
playbooks[market.code] = pb
|
||||||
|
try:
|
||||||
|
await telegram.notify_playbook_generated(
|
||||||
|
market=market.code,
|
||||||
|
stock_count=pb.stock_count,
|
||||||
|
scenario_count=pb.scenario_count,
|
||||||
|
token_count=pb.token_count,
|
||||||
|
)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.warning(
|
||||||
|
"Playbook notification failed: %s", exc
|
||||||
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.warning(
|
logger.error(
|
||||||
"Playbook notification failed: %s", exc
|
"Playbook generation failed for %s: %s",
|
||||||
|
market.code, exc,
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
try:
|
||||||
logger.error(
|
await telegram.notify_playbook_failed(
|
||||||
"Playbook generation failed for %s: %s",
|
market=market.code,
|
||||||
market.code, exc,
|
reason=str(exc)[:200],
|
||||||
)
|
)
|
||||||
try:
|
except Exception:
|
||||||
await telegram.notify_playbook_failed(
|
pass
|
||||||
market=market.code,
|
playbooks[market.code] = (
|
||||||
reason=str(exc)[:200],
|
PreMarketPlanner._empty_playbook(
|
||||||
|
market_today, market.code
|
||||||
|
)
|
||||||
)
|
)
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
playbooks[market.code] = (
|
|
||||||
PreMarketPlanner._empty_playbook(
|
|
||||||
date.today(), market.code
|
|
||||||
)
|
|
||||||
)
|
|
||||||
else:
|
else:
|
||||||
logger.info(
|
logger.info(
|
||||||
"Smart Scanner: No candidates for %s — no trades", market.name
|
"Smart Scanner: No candidates for %s — no trades", market.name
|
||||||
@@ -1096,7 +1117,9 @@ async def run(settings: Settings) -> None:
|
|||||||
# Get playbook for this market
|
# Get playbook for this market
|
||||||
market_playbook = playbooks.get(
|
market_playbook = playbooks.get(
|
||||||
market.code,
|
market.code,
|
||||||
PreMarketPlanner._empty_playbook(date.today(), market.code),
|
PreMarketPlanner._empty_playbook(
|
||||||
|
datetime.now(market.timezone).date(), market.code
|
||||||
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
# Retry logic for connection errors
|
# Retry logic for connection errors
|
||||||
|
|||||||
@@ -792,7 +792,7 @@ class TestScenarioEngineIntegration:
|
|||||||
telegram=mock_telegram,
|
telegram=mock_telegram,
|
||||||
market=mock_market,
|
market=mock_market,
|
||||||
stock_code="005930",
|
stock_code="005930",
|
||||||
scan_candidates={"005930": candidate},
|
scan_candidates={"KR": {"005930": candidate}},
|
||||||
)
|
)
|
||||||
|
|
||||||
# Verify evaluate was called
|
# Verify evaluate was called
|
||||||
@@ -810,6 +810,48 @@ class TestScenarioEngineIntegration:
|
|||||||
assert "portfolio_pnl_pct" in portfolio_data
|
assert "portfolio_pnl_pct" in portfolio_data
|
||||||
assert "total_cash" in portfolio_data
|
assert "total_cash" in portfolio_data
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_scan_candidates_market_scoped(
|
||||||
|
self, mock_broker: MagicMock, mock_market: MagicMock, mock_telegram: MagicMock,
|
||||||
|
) -> None:
|
||||||
|
"""Test scan_candidates uses market-scoped lookup, ignoring other markets."""
|
||||||
|
from src.analysis.smart_scanner import ScanCandidate
|
||||||
|
|
||||||
|
engine = MagicMock(spec=ScenarioEngine)
|
||||||
|
engine.evaluate = MagicMock(return_value=_make_hold_match())
|
||||||
|
|
||||||
|
# Candidate stored under US market — should NOT be found for KR market
|
||||||
|
us_candidate = ScanCandidate(
|
||||||
|
stock_code="005930", name="Overlap", price=100,
|
||||||
|
volume=500000, volume_ratio=5.0, rsi=15.0,
|
||||||
|
signal="oversold", score=90.0,
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("src.main.log_trade"):
|
||||||
|
await trading_cycle(
|
||||||
|
broker=mock_broker,
|
||||||
|
overseas_broker=MagicMock(),
|
||||||
|
scenario_engine=engine,
|
||||||
|
playbook=_make_playbook(),
|
||||||
|
risk=MagicMock(),
|
||||||
|
db_conn=MagicMock(),
|
||||||
|
decision_logger=MagicMock(),
|
||||||
|
context_store=MagicMock(get_latest_timeframe=MagicMock(return_value=None)),
|
||||||
|
criticality_assessor=MagicMock(
|
||||||
|
assess_market_conditions=MagicMock(return_value=MagicMock(value="NORMAL")),
|
||||||
|
get_timeout=MagicMock(return_value=5.0),
|
||||||
|
),
|
||||||
|
telegram=mock_telegram,
|
||||||
|
market=mock_market, # KR market
|
||||||
|
stock_code="005930",
|
||||||
|
scan_candidates={"US": {"005930": us_candidate}}, # Wrong market
|
||||||
|
)
|
||||||
|
|
||||||
|
# Should NOT have rsi/volume_ratio because candidate is under US, not KR
|
||||||
|
market_data = engine.evaluate.call_args[0][2]
|
||||||
|
assert "rsi" not in market_data
|
||||||
|
assert "volume_ratio" not in market_data
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_scenario_engine_called_without_scanner_data(
|
async def test_scenario_engine_called_without_scanner_data(
|
||||||
self, mock_broker: MagicMock, mock_market: MagicMock, mock_telegram: MagicMock,
|
self, mock_broker: MagicMock, mock_market: MagicMock, mock_telegram: MagicMock,
|
||||||
|
|||||||
Reference in New Issue
Block a user