This commit implements Task 1 from the schema migration plan:
- Trade tools (buy/sell) now write to actions table instead of old positions table
- Added trading_day_id parameter to buy/sell functions
- Updated ContextInjector to inject trading_day_id
- Updated RuntimeConfigManager to include TRADING_DAY_ID in config
- Removed P&L calculation from trade functions (now done at trading_days level)
- Added tests verifying correct behavior with new schema
Changes:
- agent_tools/tool_trade.py: Modified _buy_impl and _sell_impl to write to actions table
- agent/context_injector.py: Added trading_day_id parameter and injection logic
- api/model_day_executor.py: Updated to read trading_day_id from runtime config
- api/runtime_manager.py: Added trading_day_id to config initialization
- tests/unit/test_trade_tools_new_schema.py: New tests for new schema compliance
All tests passing.
- Created comprehensive E2E test in tests/e2e/test_full_simulation_workflow.py
- Tests new trading_days schema with manually populated data
- Verifies database helper methods work correctly
- Tests Results API structure and filtering
- Validates holdings chain across multiple days
- Checks daily P&L calculation and storage
- Verifies reasoning summary/full retrieval
- Fixed database index creation for backward compatibility with old schema
- Added migration script for cleaning old positions table
- Test uses dependency override to ensure API uses correct database
NOTE: Test does not run full simulation since model_day_executor
has not yet been migrated to new schema. Instead directly populates
trading_days table and validates API layer works correctly.
Test verifies Task 9 requirements from implementation plan.
Implements new /results endpoint with day-centric data structure:
- Returns starting_position, daily_metrics, trades, and final_position
- Supports reasoning levels: none (default), summary, full
- Uses database helper methods from trading_days schema
- Replaces old positions-based endpoint
Changes:
- Created api/routes/results_v2.py with new endpoint
- Registered router in api/main.py
- Removed old /results endpoint (positions table)
- Added comprehensive integration tests
All tests pass.
Critical fixes:
1. Fixed api/database.py import - use get_db_path() instead of non-existent get_database_path()
2. Fixed state management - use database queries instead of reading from position.jsonl file
3. Fixed action counting - track during trading loop execution instead of retroactively from conversation history
4. Completed integration test to verify P&L calculation works correctly
Changes:
- agent/base_agent/base_agent.py:
* Updated _get_current_portfolio_state() to query database via get_current_position_from_db()
* Added today_date and job_id parameters to method signature
* Count trade actions during trading loop instead of post-processing conversation history
* Removed obsolete action counting logic
- api/database.py:
* Fixed import to use get_db_path() from deployment_config
* Pass correct default database path "data/trading.db"
- tests/integration/test_agent_pnl_integration.py:
* Added proper mocks for dev mode and MCP client
* Mocked get_current_position_from_db to return test data
* Added comprehensive assertions to verify trading_day record fields
* Test now actually validates P&L calculation integration
Test results:
- All unit tests passing (252 passed)
- All P&L integration tests passing (8 passed)
- No regressions detected
- Add PRAGMA foreign_keys = ON at the beginning of create_trading_days_schema()
- Create jobs table if it doesn't exist as a prerequisite for the foreign key constraint
- Ensures referential integrity is properly enforced for the trading_days table
Reduce visual noise during development by logging healthcheck requests
at DEBUG level when DEPLOYMENT_MODE=DEV. Production mode continues to
log healthchecks at INFO level for proper observability.
Changes:
- Modified /health endpoint to check deployment mode
- DEV mode: logs at DEBUG level (only visible with DEBUG logging)
- PROD mode: logs at INFO level (maintains current behavior)
Co-Authored-By: Claude <noreply@anthropic.com>
- Removed call to _write_results_to_db() in execute_async()
- Deleted entire _write_results_to_db() method (lines 435-531)
- Deleted helper method _calculate_portfolio_value() (lines 533-557)
- Position tracking now exclusively handled by trade tools
This method was calling non-existent methods (get_positions(), get_last_trade(),
get_current_prices()) on BaseAgent, resulting in corrupt records with cash=0
and holdings=[]. Removal fixes bugs where cash resets to initial value and
positions are lost over weekends.
Previously, when re-running a job with some model-days already completed:
- _prepare_data() marked them as "skipped" with error="Already completed"
- But _execute_date() didn't check the skip list before launching executors
- ModelDayExecutor would start, change status to "running", and never complete
- Job would hang with status="running" and pending count > 0
Fixed by:
- _prepare_data() now returns completion_skips: {model: {dates}}
- _execute_date() receives completion_skips and filters out already-completed models
- Skipped model-days are not submitted to ThreadPoolExecutor
- Job completes correctly, skipped model-days remain with status="skipped"
This ensures idempotent job behavior - re-running a job only executes
model-days that haven't completed yet.
Fixes#73
Add instrumentation at component boundaries to trace where ContextInjector values become None:
- ModelDayExecutor: Log ContextInjector creation and set_context() invocation
- BaseAgent.set_context(): Log entry, client creation, tool reload, completion
- Includes object IDs to verify instance identity across boundaries
Part of systematic debugging investigation for issue #TBD.
Critical fixes for ContextInjector and database concurrency:
1. ContextInjector Not Working:
- Made set_context() async to reload tools after recreating MCP client
- Tools from old client (without interceptor) were still being used
- Now tools are reloaded from new client with interceptor active
- This ensures buy/sell calls properly receive injected parameters
2. Database Locking:
- Closed main connection before _write_results_to_db() opens new one
- SQLite doesn't handle concurrent write connections well
- Prevents "database is locked" error during position writes
Changes:
- agent/base_agent/base_agent.py:
- async def set_context() instead of def set_context()
- Added: self.tools = await self.client.get_tools()
- api/model_day_executor.py:
- await agent.set_context(context_injector)
- conn.close() before _write_results_to_db()
Root Cause:
When recreating the MCP client with tool_interceptors, the old tools
were still cached in self.tools and being passed to the AI agent.
The interceptor was never invoked, so job_id/signature/date were missing.
This commit migrates the system to database-only position storage,
eliminating file-based position.jsonl dependencies and fixing
ContextInjector initialization timing issues.
Key Changes:
1. ContextInjector Lifecycle Refactor:
- Remove ContextInjector creation from BaseAgent.__init__()
- Add BaseAgent.set_context() method for post-initialization injection
- Update ModelDayExecutor to create ContextInjector with correct trading day date
- Ensures ContextInjector receives actual trading date instead of init_date
- Includes session_id injection for proper database linking
2. Database Position Functions:
- Implement get_today_init_position_from_db() for querying previous positions
- Implement add_no_trade_record_to_db() for no-trade day handling
- Both functions query SQLite directly (positions + holdings tables)
- Handle first trading day case with initial cash return
- Include comprehensive error handling and logging
3. System Integration:
- Update get_agent_system_prompt() to use database queries
- Update _handle_trading_result() to write no-trade records to database
- Remove dependencies on position.jsonl file reading/writing
- Use deployment_config for automatic prod/dev database resolution
Data Flow:
- ModelDayExecutor creates runtime config and trading session
- Agent initialized without context
- ContextInjector created with (signature, date, job_id, session_id)
- Context injected via set_context()
- System prompt queries database for yesterday's position
- Trade tools write directly to database
- No-trade handler creates database records
Fixes:
- ContextInjector no longer receives None values
- No FileNotFoundError for missing position.jsonl files
- Database is single source of truth for position tracking
- Session linking maintained across all position records
Design: docs/plans/2025-02-11-database-position-tracking-design.md
Changed action_type from 'init' to 'no_trade' in _initialize_starting_position()
to comply with database CHECK constraint that only allows 'buy', 'sell', or 'no_trade'.
Fixes sqlite3.IntegrityError during position initialization.
Complete rewrite of position management in MCP trade tools:
**Trade Tools (agent_tools/tool_trade.py)**
- Replace file-based position.jsonl reads with SQLite queries
- Add get_current_position_from_db() to query positions and holdings tables
- Rewrite buy() and sell() to write directly to database
- Calculate portfolio value and P&L metrics in tools
- Accept job_id and session_id parameters via ContextInjector
- Return errors with proper context for debugging
- Use deployment-aware database path resolution
**Context Injection (agent/context_injector.py)**
- Add job_id and session_id to constructor
- Inject job_id and session_id into buy/sell tool calls
- Support optional parameters (None in standalone mode)
**BaseAgent (agent/base_agent/base_agent.py)**
- Read JOB_ID from runtime config
- Pass job_id to ContextInjector during initialization
- Enable automatic context injection for API mode
**ModelDayExecutor (api/model_day_executor.py)**
- Add _initialize_starting_position() method
- Create initial position record before agent runs
- Load initial_cash from config
- Update context_injector.session_id after session creation
- Link positions to sessions automatically
**Architecture Changes:**
- Eliminates file-based position tracking entirely
- Single source of truth: SQLite database
- Positions automatically linked to trading sessions
- Concurrent execution safe (no file system conflicts)
- Deployment mode aware (prod vs dev databases)
This completes the migration to database-only position storage.
File-based position.jsonl is no longer used or created.
Fixes context injection errors in concurrent simulations.
Fixed critical bug where ModelDayExecutor._initialize_agent() created
a BaseAgent but never called agent.initialize(), leaving self.model=None.
This caused 'NoneType' object has no attribute 'bind' error when
run_trading_session() tried to create the langchain agent.
Changes:
- Made _initialize_agent() async
- Added await agent.initialize() call
- Updated call site to await async function
Now properly initializes MCP client, tools, and AI model before
executing trading sessions, matching the working CLI mode pattern.
Fixes startup error 'no such column: session_id' that occurs when
_create_indexes() tries to create indexes on columns that don't exist yet.
The issue occurred when initializing a database from scratch:
1. _migrate_schema() adds session_id column to positions table
2. _create_indexes() tries to create index on session_id
3. But on fresh databases, positions table was created without session_id
4. Migration runs after table creation, before index creation
5. Index creation fails because column doesn't exist yet
Solution: Check if columns exist before creating indexes on them.
This ensures the database can be initialized both:
- Fresh (CREATE TABLE without session_id, then ALTER TABLE, then CREATE INDEX)
- Migrated (ALTER TABLE adds column, then CREATE INDEX)
Tested: All 21 database tests passing
Complete implementation of reasoning logs retrieval system that
replaces JSONL file-based logging with database-only storage.
Database Changes:
- Add trading_sessions table (one record per model-day)
- Add reasoning_logs table (conversation history with summaries)
- Add session_id column to positions table
- Add indexes for query performance
Agent Changes:
- Add conversation history tracking to BaseAgent
- Add AI-powered summary generation using same model
- Remove JSONL logging code (_log_message, _setup_logging)
- Preserve in-memory conversation tracking
ModelDayExecutor Changes:
- Create trading session at start of execution
- Store reasoning logs with AI-generated summaries
- Update session summary after completion
- Link positions to sessions via session_id
API Changes:
- Add GET /reasoning endpoint with filters (job_id, date, model)
- Support include_full_conversation parameter
- Return both summaries and full conversation on demand
- Include deployment mode info in responses
Documentation:
- Add complete API reference for GET /reasoning
- Add design document with architecture details
- Add implementation guide with step-by-step tasks
- Update Python and TypeScript client examples
Testing:
- Add 6 tests for conversation history tracking
- Add 4 tests for summary generation
- Add 5 tests for model_day_executor integration
- Add 8 tests for GET /reasoning endpoint
- Add 9 integration tests for E2E flow
- Update existing tests for schema changes
All 32 new feature tests passing. Total: 285 tests passing.
- Add Pydantic models for reasoning API responses
- Implement GET /reasoning with job_id, date, model filters
- Support include_full_conversation parameter
- Add comprehensive unit tests (8 tests)
- Return deployment mode info in responses
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Add _create_trading_session() method to create session records
- Add async _store_reasoning_logs() to store conversation with AI summaries
- Add async _update_session_summary() to generate overall session summary
- Modify execute() -> execute_async() with async workflow
- Add execute_sync() wrapper and keep execute() as sync entry point
- Update _write_results_to_db() to accept and use session_id parameter
- Modify positions INSERT to include session_id foreign key
- Remove old reasoning_logs code block (obsolete schema)
- Add comprehensive unit tests for all new functionality
All tests pass. Session-based reasoning storage now integrated.
- Fix async call in model_day_executor.py by wrapping with asyncio.run()
Resolves RuntimeWarning where run_trading_session coroutine was never awaited
- Remove register_agent() call in API mode to prevent file-based position storage
Position data is now stored exclusively in SQLite database (jobs.db)
- Update test mocks to use AsyncMock for async run_trading_session method
This fixes production deployment issues:
1. Trading sessions now execute properly (async bug)
2. No position files created, database-only storage
3. All tests pass
Closes issue with no trades being executed in production
The FastAPI app now checks for /tmp/runtime_config.json (created by
entrypoint.sh config merger) before falling back to the default config.
This allows user-provided configs mounted at /app/user-configs/ to
properly override default values in containerized deployments.
Fixes issue where custom user configs were not being applied.
Cleaned up all diagnostic print statements added during debugging.
The root cause (non-idempotent get_db_path) has been fixed, so the
extensive instrumentation is no longer needed.
Changes:
- Removed all diagnostic prints from api/main.py (lifespan and module-level)
- Removed all diagnostic prints from api/database.py (get_db_connection and initialize_dev_database)
- Kept essential user-facing messages (PRESERVE_DEV_DATA notice, database creation messages)
All 28 integration tests pass.
Enhanced diagnostics to trace database path resolution and table existence
at connection time. This will help identify if get_db_connection() is
resolving paths correctly and accessing the right database file.
Added diagnostics to:
- get_db_connection(): Show input path, resolved path, file existence, and tables found
- initialize_dev_database(): Verify tables exist after creation
This will reveal whether the path resolution is working correctly or if
there's a timing/caching issue with database file access.
Following systematic debugging methodology after 5 failed fix attempts.
Adding extensive print-based diagnostics to trace execution flow in Docker.
Instrumentation added to:
- api/main.py: Module import, app creation, lifespan function, module-level init
- api/database.py: initialize_dev_database() entry/exit and decision points
This diagnostic version will help identify:
1. Whether module-level code executes in Docker
2. Which initialization layer is failing
3. Database paths being resolved
4. Environment variable values
Tests confirmed passing with diagnostic logging.
Add database initialization at module load time to ensure it runs
regardless of how uvicorn handles the lifespan context manager.
Issue: The lifespan function wasn't being triggered consistently when
uvicorn loads the app module, causing "no such table: jobs" errors.
Solution: Initialize database when the module is imported (after app
creation), providing a reliable fallback that works in all deployment
scenarios.
This provides defense-in-depth:
1. Lifespan function (ideal path)
2. Module-level initialization (fallback/guarantee)
Both paths check deployment mode and call the appropriate init function.
Move database initialization logic from shell script to Python application
lifespan, following separation of concerns and improving maintainability.
Benefits:
- Single source of truth for database initialization (api/main.py lifespan)
- Better testability - Python code vs shell scripts
- Clearer logging with structured messages
- Easier to debug and maintain
- Infrastructure (entrypoint.sh) focuses on service orchestration
- Application (api/main.py) owns its data layer
Changes:
- Removed database init from entrypoint.sh
- Enhanced lifespan function with detailed logging
- Simplified entrypoint script (now 4 steps instead of 5)
- All tests pass (28/28 API endpoint tests)
- Fix lifespan function to access db_path from create_app scope via closure
- Prevents "no such table: jobs" error by ensuring database initialization runs
- Previous version tried to access app.state.db_path before it was set
The issue was that app.state is set after FastAPI instantiation, but the
lifespan function needs the db_path during startup. Using closure allows
the lifespan function to capture db_path from the create_app function scope.
- Add database initialization to API lifespan event handler
- DEV mode: Reset database on startup (unless PRESERVE_DEV_DATA=true)
- PROD mode: Ensure database schema exists
- Migrate from deprecated @app.on_event to modern lifespan context manager
- Fixes 400 error "Another simulation job is already running" on fresh container starts
This ensures the dev database is reset when the API server starts in dev mode,
preventing stale "running" or "pending" jobs from blocking new job creation.
- Add 'skipped' to terminal states in update_job_detail_status()
- Ensures skipped dates properly:
- Update status and completed_at timestamp
- Store skip reason in error field
- Trigger job completion checks
- Add comprehensive test suite (11 tests) covering:
- Database schema validation
- Job completion with skipped dates
- Progress tracking with skip counts
- Multi-model skip handling
- Skip reason storage
Bug was discovered via TDD - created tests first, which revealed
that skipped status wasn't being handled in the terminal state
block at line 397.
All 11 tests passing.
Implement skip status tracking to fix jobs hanging when dates are
filtered out. Jobs now properly complete when all model-days reach
terminal states (completed/failed/skipped).
Changes:
- database.py: Add 'skipped' status to job_details CHECK constraint
- job_manager.py: Update completion logic to count skipped as done
- job_manager.py: Add skipped count to progress tracking
- simulation_worker.py: Implement skip tracking with per-model granularity
- simulation_worker.py: Add _filter_completed_dates_with_tracking()
- simulation_worker.py: Add _mark_skipped_dates()
- simulation_worker.py: Update _prepare_data() to use skip tracking
- simulation_worker.py: Improve warning messages to distinguish skip types
Skip reasons:
- "Already completed" - Position data exists from previous job
- "Incomplete price data" - Missing prices (weekends/holidays/future)
The implementation correctly handles multi-model scenarios where different
models have different completion states for the same date.
When the trigger simulation API receives an empty models list ([]),
it now correctly falls back to enabled models from config instead
of running with no models.
Changes:
- Update condition to check for both None and empty list
- Add test case for empty models list behavior
- Update API documentation to clarify this behavior
All 28 integration tests pass.
Remove complex table recreation logic since the server hasn't been
deployed yet. For existing databases, simply delete and recreate.
The dev database is already recreated on startup by design.
Co-Authored-By: Claude <noreply@anthropic.com>
Move data preparation to background worker:
- Fast endpoint response (<1s)
- No blocking downloads
- Worker handles data download and filtering
- Maintains backwards compatibility
Co-Authored-By: Claude <noreply@anthropic.com>
Call _prepare_data before executing trades:
- Download missing data if needed
- Filter completed dates
- Store warnings
- Handle empty date scenarios
Co-Authored-By: Claude <noreply@anthropic.com>
Orchestrate data preparation phase:
- Check missing data
- Download if needed
- Filter completed dates
- Update job status
Co-Authored-By: Claude <noreply@anthropic.com>
Critical fixes identified in code review:
1. Add warnings column migration to _migrate_schema()
- Checks if warnings column exists in jobs table
- Adds column via ALTER TABLE if missing
- Ensures existing databases get new column on upgrade
2. Document CHECK constraint limitation
- Added docstring explaining ALTER TABLE cannot add CHECK constraints
- Notes that "downloading_data" status requires fresh DB or manual migration
3. Add comprehensive migration tests
- test_migration_adds_warnings_column: Verifies warnings column migration
- test_migration_adds_simulation_run_id_column: Tests existing migration
- Both tests include cleanup to prevent cross-test contamination
4. Update test fixtures and expectations
- Updated clean_db fixture to delete from all 9 tables
- Fixed table count assertions (6 -> 9 tables)
- Updated expected columns in schema tests
All 21 database tests now pass.
Add support for:
- downloading_data job status for visibility during data prep
- warnings TEXT column for storing job-level warnings (JSON array)
Co-Authored-By: Claude <noreply@anthropic.com>
- Add FastAPI @app.on_event("startup") handler to display warning
- Previously only appeared when running directly (not via uvicorn)
- Add DEPLOYMENT_MODE and PRESERVE_DEV_DATA to docker-compose.yml
- Update CHANGELOG.md with fix documentation
Fixes issue where dev mode banner wasn't visible in Docker logs
because uvicorn imports app without executing __main__ block.
BREAKING CHANGE: end_date is now required and cannot be null/empty
New Features:
- Resume mode: Set start_date to null to continue from last completed date per model
- Idempotent by default: Skip already-completed dates with replace_existing=false
- Per-model independence: Each model resumes from its own last completed date
- Cold start handling: If no data exists in resume mode, runs only end_date as single day
API Changes:
- start_date: Now optional (null enables resume mode)
- end_date: Now REQUIRED (cannot be null or empty string)
- replace_existing: New optional field (default: false for idempotent behavior)
Implementation:
- Added JobManager.get_last_completed_date_for_model() method
- Added JobManager.get_completed_model_dates() method
- Updated create_job() to support model_day_filter for selective task creation
- Fixed bug with start_date=None in price data checks
Documentation:
- Updated API_REFERENCE.md with complete examples and behavior matrix
- Updated QUICK_START.md with resume mode examples
- Updated docs/user-guide/using-the-api.md
- Added CHANGELOG_NEW_API.md with migration guide
- Updated all integration tests for new schema
- Updated client library examples (Python, TypeScript)
Migration:
- Old: {"start_date": "2025-01-16"}
- New: {"start_date": "2025-01-16", "end_date": "2025-01-16"}
- Resume: {"start_date": null, "end_date": "2025-01-31"}
See CHANGELOG_NEW_API.md for complete details.
Add comprehensive warning display when server starts in development mode
to ensure users are aware of simulated AI calls and data handling.
Changes:
- Add log_dev_mode_startup_warning() function in deployment_config.py
- Display warning on main.py startup when DEPLOYMENT_MODE=DEV
- Display warning on API server startup (api/main.py)
- Warning shows AI simulation status and data persistence behavior
- Provides clear instructions for switching to PROD mode
The warning is highly visible and informs users that:
- AI API calls are simulated (no costs incurred)
- Data may be reset between runs (based on PRESERVE_DEV_DATA)
- System is using isolated dev database and paths
Co-Authored-By: Claude <noreply@anthropic.com>
