Compare commits

..

4 Commits

Author SHA1 Message Date
31d6818130 fix: enable cross-job portfolio continuity in get_starting_holdings
Remove job_id filter from get_starting_holdings() SQL JOIN to enable
holdings continuity across jobs. This completes the cross-job portfolio
continuity fix started in the previous commit.

Root cause: get_starting_holdings() joined on job_id, preventing it from
finding previous day's holdings when queried from a different job. This
caused starting_position.holdings to be empty in API results for new jobs
even though starting_cash was correctly retrieved.

Changes:
- api/database.py: Remove job_id from JOIN condition in get_starting_holdings()
- tests/unit/test_database_helpers.py: Add test for cross-job holdings retrieval

Together with the previous commit fixing get_previous_trading_day(), this
ensures complete portfolio continuity (both cash and holdings) across jobs.
2025-11-07 16:38:33 -05:00
4638c073e3 fix: enable cross-job portfolio continuity in get_previous_trading_day
Remove job_id filter from get_previous_trading_day() SQL query to enable
portfolio continuity across jobs. Previously, new jobs would reset to
initial $10,000 cash instead of continuing from previous job's ending
position.

Root cause: get_previous_trading_day() filtered by job_id, while
get_current_position_from_db() correctly queries across all jobs.
This inconsistency caused starting_cash to default to initial_cash
when no previous day was found within the same job.

Changes:
- api/database.py: Remove job_id filter from SQL WHERE clause
- tests/unit/test_database_helpers.py: Add test for cross-job continuity

Fixes position tracking bug where subsequent jobs on consecutive dates
would not recognize previous day's holdings from different job.
2025-11-07 16:13:28 -05:00
96f61cf347 release: v0.4.2 - fix critical negative cash position bug
Remove debug logging and update CHANGELOG for v0.4.2 release.

Fixed critical bug where trades calculated from initial $10,000 capital
instead of accumulating, allowing over-spending and negative cash balances.

Key changes:
- Extract position dict from CallToolResult.structuredContent
- Enable MCP service logging for better debugging
- Update tests to match production MCP behavior

All tests passing. Ready for production release.
2025-11-07 15:41:28 -05:00
0eb5fcc940 debug: enable stdout/stderr for MCP services to diagnose parameter injection
MCP services were started with stdout/stderr redirected to DEVNULL, making
debug logs invisible. This prevented diagnosing why _current_position parameter
is not being received by buy() function.

Changed subprocess.Popen to redirect MCP service output to main process
stdout/stderr, allowing [DEBUG buy] logs to be visible in docker logs.

This will help identify whether:
1. _current_position is being sent by ContextInjector but not received
2. MCP HTTP transport filters underscore-prefixed parameters
3. Parameter serialization is failing

Related to negative cash bug where final position shows -$3,049.83 instead
of +$727.92 tracked by ContextInjector.
2025-11-07 14:56:48 -05:00
6 changed files with 118 additions and 21 deletions

View File

@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
## [0.4.2] - 2025-11-07
### Fixed
- **Critical:** Fixed negative cash position bug where trades calculated from initial capital instead of accumulating
- Root cause: MCP tools return `CallToolResult` objects with position data in `structuredContent` field, but `ContextInjector` was checking `isinstance(result, dict)` which always failed
- Impact: Each trade checked cash against initial $10,000 instead of cumulative position, allowing over-spending and resulting in negative cash balances (e.g., -$8,768.68 after 11 trades totaling $18,768.68)
- Solution: Updated `ContextInjector` to extract position dict from `CallToolResult.structuredContent` before validation
- Fix ensures proper intra-day position tracking with cumulative cash checks preventing over-trading
- Updated unit tests to mock `CallToolResult` objects matching production MCP behavior
- Locations: `agent/context_injector.py:95-109`, `tests/unit/test_context_injector.py:26-53`
- Enabled MCP service logging by redirecting stdout/stderr from `/dev/null` to main process for better debugging
- Previously, all MCP tool debug output was silently discarded
- Now visible in docker logs for diagnosing parameter injection and trade execution issues
- Location: `agent_tools/start_mcp_services.py:81-88`
### Fixed
- **Critical:** Fixed stale jobs blocking new jobs after Docker container restart
- Root cause: Jobs with status 'pending', 'downloading_data', or 'running' remained in database after container shutdown, preventing new job creation

View File

@@ -88,28 +88,17 @@ class ContextInjector:
# Update position state after successful trade
if request.name in ["buy", "sell"]:
# Debug: Log result type and structure
print(f"[DEBUG ContextInjector] Trade result type: {type(result)}")
print(f"[DEBUG ContextInjector] Trade result: {result}")
# Extract position dict from MCP result
# MCP tools return CallToolResult objects with structuredContent field
position_dict = None
if hasattr(result, 'structuredContent') and result.structuredContent:
position_dict = result.structuredContent
print(f"[DEBUG ContextInjector] Extracted from structuredContent: {position_dict}")
elif isinstance(result, dict):
position_dict = result
print(f"[DEBUG ContextInjector] Using result as dict: {position_dict}")
# Check if position dict is valid (not an error) and update state
if position_dict and "error" not in position_dict and "CASH" in position_dict:
# Update our tracked position with the new state
self._current_position = position_dict.copy()
print(f"[DEBUG ContextInjector] Updated _current_position: {self._current_position}")
else:
print(f"[DEBUG ContextInjector] Did NOT update _current_position - check failed")
print(f"[DEBUG ContextInjector] position_dict: {position_dict}")
print(f"[DEBUG ContextInjector] _current_position remains: {self._current_position}")
return result

View File

@@ -78,10 +78,11 @@ class MCPServiceManager:
env['PYTHONPATH'] = str(Path.cwd())
# Start service process (output goes to Docker logs)
# Enable stdout/stderr for debugging (previously sent to DEVNULL)
process = subprocess.Popen(
[sys.executable, str(script_path)],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
stdout=sys.stdout, # Redirect to main process stdout
stderr=sys.stderr, # Redirect to main process stderr
cwd=Path.cwd(), # Use current working directory (/app)
env=env # Pass environment with PYTHONPATH
)

View File

@@ -132,14 +132,11 @@ def _buy_impl(symbol: str, amount: int, signature: str = None, today_date: str =
# Step 1: Get current position
# Use injected position if available (for intra-day tracking),
# otherwise query database for starting position
print(f"[DEBUG buy] _current_position received: {_current_position}")
if _current_position is not None:
current_position = _current_position
next_action_id = 0 # Not used in new schema
print(f"[DEBUG buy] Using injected position: {current_position}")
else:
current_position, next_action_id = get_current_position_from_db(job_id, signature, today_date)
print(f"[DEBUG buy] Queried position from DB: {current_position}")
# Step 2: Get stock price
try:
@@ -192,8 +189,6 @@ def _buy_impl(symbol: str, amount: int, signature: str = None, today_date: str =
conn.commit()
print(f"[buy] {signature} bought {amount} shares of {symbol} at ${this_symbol_price}")
print(f"[DEBUG buy] Returning new_position: {new_position}")
print(f"[DEBUG buy] new_position keys: {list(new_position.keys())}")
return new_position
except Exception as e:

View File

@@ -611,6 +611,10 @@ class Database:
Handles weekends/holidays by finding actual previous trading day.
NOTE: Queries across ALL jobs for the given model to enable portfolio
continuity even when new jobs are created with overlapping date ranges.
The job_id parameter is kept for API compatibility but not used in the query.
Returns:
dict with keys: id, date, ending_cash, ending_portfolio_value
or None if no previous day exists
@@ -619,11 +623,11 @@ class Database:
"""
SELECT id, date, ending_cash, ending_portfolio_value
FROM trading_days
WHERE job_id = ? AND model = ? AND date < ?
WHERE model = ? AND date < ?
ORDER BY date DESC
LIMIT 1
""",
(job_id, model, current_date)
(model, current_date)
)
row = cursor.fetchone()
@@ -657,6 +661,9 @@ class Database:
def get_starting_holdings(self, trading_day_id: int) -> list:
"""Get starting holdings from previous day's ending holdings.
NOTE: Queries across ALL jobs for the given model to enable portfolio
continuity even when new jobs are created with overlapping date ranges.
Returns:
List of dicts with keys: symbol, quantity
Empty list if first trading day
@@ -667,7 +674,6 @@ class Database:
SELECT td_prev.id
FROM trading_days td_current
JOIN trading_days td_prev ON
td_prev.job_id = td_current.job_id AND
td_prev.model = td_current.model AND
td_prev.date < td_current.date
WHERE td_current.id = ?

View File

@@ -130,6 +130,44 @@ class TestDatabaseHelpers:
assert previous is not None
assert previous["date"] == "2025-01-17"
def test_get_previous_trading_day_across_jobs(self, db):
"""Test retrieving previous trading day from different job (cross-job continuity)."""
# Setup: Create two jobs
db.connection.execute(
"INSERT INTO jobs (job_id, status, config_path, date_range, models, created_at) VALUES (?, ?, ?, ?, ?, ?)",
("job-1", "completed", "config.json", "2025-10-07,2025-10-07", "deepseek-chat-v3.1", "2025-11-07T00:00:00Z")
)
db.connection.execute(
"INSERT INTO jobs (job_id, status, config_path, date_range, models, created_at) VALUES (?, ?, ?, ?, ?, ?)",
("job-2", "running", "config.json", "2025-10-08,2025-10-08", "deepseek-chat-v3.1", "2025-11-07T01:00:00Z")
)
# Day 1 in job-1
db.create_trading_day(
job_id="job-1",
model="deepseek-chat-v3.1",
date="2025-10-07",
starting_cash=10000.0,
starting_portfolio_value=10000.0,
daily_profit=214.58,
daily_return_pct=2.15,
ending_cash=123.59,
ending_portfolio_value=10214.58
)
# Test: Get previous day from job-2 on next date
# Should find job-1's record (cross-job continuity)
previous = db.get_previous_trading_day(
job_id="job-2",
model="deepseek-chat-v3.1",
current_date="2025-10-08"
)
assert previous is not None
assert previous["date"] == "2025-10-07"
assert previous["ending_cash"] == 123.59
assert previous["ending_portfolio_value"] == 10214.58
def test_get_ending_holdings(self, db):
"""Test retrieving ending holdings for a trading day."""
db.connection.execute(
@@ -224,6 +262,59 @@ class TestDatabaseHelpers:
assert holdings[0]["symbol"] == "AAPL"
assert holdings[0]["quantity"] == 10
def test_get_starting_holdings_across_jobs(self, db):
"""Test starting holdings retrieval across different jobs (cross-job continuity)."""
# Setup: Create two jobs
db.connection.execute(
"INSERT INTO jobs (job_id, status, config_path, date_range, models, created_at) VALUES (?, ?, ?, ?, ?, ?)",
("job-1", "completed", "config.json", "2025-10-07,2025-10-07", "deepseek-chat-v3.1", "2025-11-07T00:00:00Z")
)
db.connection.execute(
"INSERT INTO jobs (job_id, status, config_path, date_range, models, created_at) VALUES (?, ?, ?, ?, ?, ?)",
("job-2", "running", "config.json", "2025-10-08,2025-10-08", "deepseek-chat-v3.1", "2025-11-07T01:00:00Z")
)
# Day 1 in job-1 with holdings
day1_id = db.create_trading_day(
job_id="job-1",
model="deepseek-chat-v3.1",
date="2025-10-07",
starting_cash=10000.0,
starting_portfolio_value=10000.0,
daily_profit=214.58,
daily_return_pct=2.15,
ending_cash=329.825,
ending_portfolio_value=10666.135
)
db.create_holding(day1_id, "AAPL", 10)
db.create_holding(day1_id, "AMD", 4)
db.create_holding(day1_id, "MSFT", 8)
db.create_holding(day1_id, "NVDA", 12)
db.create_holding(day1_id, "TSLA", 1)
# Day 2 in job-2 (different job)
day2_id = db.create_trading_day(
job_id="job-2",
model="deepseek-chat-v3.1",
date="2025-10-08",
starting_cash=329.825,
starting_portfolio_value=10609.475,
daily_profit=-56.66,
daily_return_pct=-0.53,
ending_cash=33.62,
ending_portfolio_value=329.825
)
# Test: Day 2 should get Day 1's holdings from different job
holdings = db.get_starting_holdings(day2_id)
assert len(holdings) == 5
assert {"symbol": "AAPL", "quantity": 10} in holdings
assert {"symbol": "AMD", "quantity": 4} in holdings
assert {"symbol": "MSFT", "quantity": 8} in holdings
assert {"symbol": "NVDA", "quantity": 12} in holdings
assert {"symbol": "TSLA", "quantity": 1} in holdings
def test_create_action(self, db):
"""Test creating an action record."""
db.connection.execute(