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>
Remove unused imports (sys, ScenarioMatch, asyncio, StockPlaybook),
fix import ordering, and split long lines for ruff compliance.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace brain.decide() with scenario_engine.evaluate() in trading_cycle
and brain.decide_batch() with per-stock scenario evaluation in
run_daily_session. Initialize PreMarketPlanner, ScenarioEngine, and
PlaybookStore in run(). Add pre-market playbook generation on market
open (1 Gemini call per market per day), market_data enrichment from
scanner metrics (rsi, volume_ratio), portfolio_data for global rules,
scenario match notifications, and playbook lifecycle management.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review findings addressed:
- Finding 1 (ImportError): false positive — ContextLayer is re-exported from
src.context.store, import works correctly at runtime
- Finding 2 (timezone): generate_playbook() and build_cross_market_context()
now accept optional today parameter for market-local date injection
- Finding 3 (lint): removed unused imports (UTC, datetime, PlaybookStatus),
fixed line-too-long in prompt template
- Tests simplified: replaced date patching with direct today= parameter
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add playbooks table to src/db.py with UNIQUE(date, market) constraint
- PlaybookStore: save/load/delete, status management, match_count tracking,
list_recent with market filter, stats without full deserialization
- DayPlaybook JSON serialization via Pydantic model_dump_json/model_validate_json
- 23 tests, 100% coverage on playbook_store.py
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses second round of PR #102 review:
- _warn_missing_key(): logs each missing key only once per engine instance
to prevent log spam in high-frequency trading loops
- _build_match_details(): uses _safe_float() normalized values instead of
raw market_data to ensure consistent float types in logging/analysis
- Test: verify warning fires exactly once across repeated calls
- Test: verify match_details contains normalized float values
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ScenarioEngine evaluates pre-defined playbook scenarios against real-time
market data with sub-100ms execution (zero API calls). Supports condition
AND-matching, global portfolio rules, and first-match-wins priority.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add pre-market planner settings: PRE_MARKET_MINUTES, MAX_SCENARIOS_PER_STOCK,
PLANNER_TIMEOUT_SECONDS, DEFENSIVE_PLAYBOOK_ON_FAILURE, RESCAN_INTERVAL_SECONDS
- Change ENABLED_MARKETS default from KR to KR,US
- Remove static WATCHLISTS and STOCK_UNIVERSE dictionaries from main.py
- Replace watchlist-based trading with dynamic scanner-only stock discovery
- SmartVolatilityScanner is now the sole source of trading candidates
- Add active_stocks dict for scanner-discovered stocks per market
- Add smart_scanner parameter to run_daily_session()
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixed AttributeError exceptions in /status and /positions commands:
- Replaced invalid risk.calculate_pnl() with inline P&L calculation from balance dict
- Changed risk.circuit_breaker_threshold to risk._cb_threshold
- Replaced balance.stocks access with account summary from output2 dict
- Updated tests to match new account summary format
All 27 telegram command tests pass. Live bot testing confirms no errors.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove /start command as name doesn't match functionality, and fix
command parsing to handle @botname suffix for group chat compatibility.
Changes:
- Remove handle_start function and registration
- Remove /start from help command list
- Remove test_start_command_content test
- Strip @botname suffix from commands (e.g., /help@mybot → help)
Rationale:
- /start command name implies bot initialization, but it was just
showing help text (duplicate of /help)
- Better to have one clear /help command
- @botname suffix handling needed for group chats
Test:
- 27 tests pass (1 removed, 1 added for @botname handling)
- All existing functionality preserved
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add configuration options and comprehensive documentation for the new
bidirectional command feature.
Changes:
- Add TELEGRAM_COMMANDS_ENABLED to config.py
- Add TELEGRAM_POLLING_INTERVAL to config.py
- Add extensive "Bidirectional Commands" section to README.md
Documentation:
- Available commands table with descriptions
- Command usage examples with sample outputs
- Security section (Chat ID verification, authorization)
- Configuration options and .env examples
- How it works (long polling, authentication flow)
- Error handling and troubleshooting guide
Features:
- Optional command support (can disable while keeping notifications)
- Configurable polling interval
- Complete security documentation
- Troubleshooting guide for common issues
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add real-time status and portfolio monitoring via Telegram.
Changes:
- Implement /status handler (mode, markets, P&L, trading state)
- Implement /positions handler (holdings with grouping by market)
- Integrate with Broker API and RiskManager
- Add 5 comprehensive tests for status commands
Features:
- /status: Shows trading mode, enabled markets, pause state, P&L, circuit breaker
- /positions: Lists holdings grouped by market (domestic/overseas)
- Error handling: Graceful degradation on API failures
- Empty state: Handles portfolios with no positions
Integration:
- Uses broker.get_balance() for account data
- Uses risk.calculate_pnl() for P&L calculation
- Accesses pause_trading.is_set() for trading state
- Groups positions by market for better readability
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add pause/resume functionality for remote trading control via Telegram.
Changes:
- Add pause_trading Event to main.py
- Implement /stop handler (pause trading)
- Implement /resume handler (resume trading)
- Integrate pause logic into both daily and realtime trading loops
- Add 4 comprehensive tests for trading control
Features:
- /stop: Pauses all trading operations
- /resume: Resumes trading operations
- Idempotent: Handles repeated stop/resume gracefully
- Status feedback: Informs if already paused/active
- Works in both daily and realtime trading modes
Security:
- Commands verified by TelegramCommandHandler chat_id check
- Only authorized users can control trading
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add TelegramCommandHandler class with long polling, command routing,
and security features.
Changes:
- Add TelegramCommandHandler class to telegram_client.py
- Implement long polling with getUpdates API
- Add command registration and routing mechanism
- Implement chat ID verification for security
- Add comprehensive tests (16 tests)
- Coverage: 85% for telegram_client.py
Features:
- start_polling() / stop_polling() lifecycle management
- register_command() for handler registration
- Chat ID verification to prevent unauthorized access
- Error isolation (command failures don't crash system)
- Graceful handling of API errors and timeouts
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add send_message(text, parse_mode) method that can be used for both
notifications and command responses. Refactor _send_notification to
use the new method.
Changes:
- Add send_message() method with return value for success/failure
- Refactor _send_notification() to call send_message()
- Add comprehensive tests for send_message()
- Coverage: 93% for telegram_client.py
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Root cause analysis revealed 3 critical issues causing EGW00201 errors:
1. **Hash key bypass** - _get_hash_key() made API calls without rate limiting
- Every order made 2 API calls but only 1 was rate-limited
- Fixed by adding rate_limiter.acquire() to _get_hash_key()
2. **Scanner concurrent burst** - scan_market() launched all stocks via asyncio.gather
- All tasks queued simultaneously creating burst pressure
- Fixed by adding Semaphore(1) for fully serialized scanning
3. **RPS too aggressive** - 5.0 RPS exceeded KIS API's real ~2 RPS limit
- Lowered to 2.0 RPS (500ms interval) for maximum safety
Changes:
- src/broker/kis_api.py: Add rate limiter to _get_hash_key()
- src/analysis/scanner.py: Add semaphore-based concurrency control
- New max_concurrent_scans parameter (default 1, fully serialized)
- Wrap scan_stock calls with semaphore in _bounded_scan()
- Remove ineffective asyncio.sleep(0.2) from scan_stock()
- src/config.py: Lower RATE_LIMIT_RPS from 5.0 to 2.0
- tests/test_broker.py: Add 2 tests for hash key rate limiting
- tests/test_volatility.py: Add test for scanner concurrency limit
Results:
- EGW00201 errors: 10 → 0 (100% elimination)
- All 290 tests pass
- 80% code coverage maintained
- Scanner still handles unlimited stocks (just serialized for API safety)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Resolved conflict in src/main.py by using safe_float() from main
instead of float(...or '0') pattern.
Changes:
- src/main.py: Use safe_float() for consistent empty string handling
- All 16 tests pass including test_overseas_price_empty_string
