mirror of
https://github.com/Xe138/AI-Trader.git
synced 2026-04-05 02:17:23 -04:00
Compare commits
5 Commits
v0.4.2-alp
...
v0.4.2
| Author | SHA1 | Date | |
|---|---|---|---|
| 96f61cf347 | |||
| 0eb5fcc940 | |||
| bee6afe531 | |||
| f1f76b9a99 | |||
| 277714f664 |
15
CHANGELOG.md
15
CHANGELOG.md
@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
## [Unreleased]
|
## [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
|
### Fixed
|
||||||
- **Critical:** Fixed stale jobs blocking new jobs after Docker container restart
|
- **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
|
- Root cause: Jobs with status 'pending', 'downloading_data', or 'running' remained in database after container shutdown, preventing new job creation
|
||||||
|
|||||||
@@ -88,9 +88,17 @@ class ContextInjector:
|
|||||||
|
|
||||||
# Update position state after successful trade
|
# Update position state after successful trade
|
||||||
if request.name in ["buy", "sell"]:
|
if request.name in ["buy", "sell"]:
|
||||||
# Check if result is a valid position dict (not an error)
|
# Extract position dict from MCP result
|
||||||
if isinstance(result, dict) and "error" not in result and "CASH" in result:
|
# MCP tools return CallToolResult objects with structuredContent field
|
||||||
|
position_dict = None
|
||||||
|
if hasattr(result, 'structuredContent') and result.structuredContent:
|
||||||
|
position_dict = result.structuredContent
|
||||||
|
elif isinstance(result, dict):
|
||||||
|
position_dict = result
|
||||||
|
|
||||||
|
# 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
|
# Update our tracked position with the new state
|
||||||
self._current_position = result.copy()
|
self._current_position = position_dict.copy()
|
||||||
|
|
||||||
return result
|
return result
|
||||||
|
|||||||
@@ -78,10 +78,11 @@ class MCPServiceManager:
|
|||||||
env['PYTHONPATH'] = str(Path.cwd())
|
env['PYTHONPATH'] = str(Path.cwd())
|
||||||
|
|
||||||
# Start service process (output goes to Docker logs)
|
# Start service process (output goes to Docker logs)
|
||||||
|
# Enable stdout/stderr for debugging (previously sent to DEVNULL)
|
||||||
process = subprocess.Popen(
|
process = subprocess.Popen(
|
||||||
[sys.executable, str(script_path)],
|
[sys.executable, str(script_path)],
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=sys.stdout, # Redirect to main process stdout
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=sys.stderr, # Redirect to main process stderr
|
||||||
cwd=Path.cwd(), # Use current working directory (/app)
|
cwd=Path.cwd(), # Use current working directory (/app)
|
||||||
env=env # Pass environment with PYTHONPATH
|
env=env # Pass environment with PYTHONPATH
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from agent.context_injector import ContextInjector
|
from agent.context_injector import ContextInjector
|
||||||
|
from unittest.mock import Mock
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
@@ -22,27 +23,34 @@ class MockRequest:
|
|||||||
self.args = args or {}
|
self.args = args or {}
|
||||||
|
|
||||||
|
|
||||||
|
def create_mcp_result(position_dict):
|
||||||
|
"""Create a mock MCP CallToolResult object matching production behavior."""
|
||||||
|
result = Mock()
|
||||||
|
result.structuredContent = position_dict
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
async def mock_handler_success(request):
|
async def mock_handler_success(request):
|
||||||
"""Mock handler that returns a successful position update."""
|
"""Mock handler that returns a successful position update as MCP CallToolResult."""
|
||||||
# Simulate a successful trade returning updated position
|
# Simulate a successful trade returning updated position
|
||||||
if request.name == "sell":
|
if request.name == "sell":
|
||||||
return {
|
return create_mcp_result({
|
||||||
"CASH": 1100.0,
|
"CASH": 1100.0,
|
||||||
"AAPL": 7,
|
"AAPL": 7,
|
||||||
"MSFT": 5
|
"MSFT": 5
|
||||||
}
|
})
|
||||||
elif request.name == "buy":
|
elif request.name == "buy":
|
||||||
return {
|
return create_mcp_result({
|
||||||
"CASH": 50.0,
|
"CASH": 50.0,
|
||||||
"AAPL": 7,
|
"AAPL": 7,
|
||||||
"MSFT": 12
|
"MSFT": 12
|
||||||
}
|
})
|
||||||
return {}
|
return create_mcp_result({})
|
||||||
|
|
||||||
|
|
||||||
async def mock_handler_error(request):
|
async def mock_handler_error(request):
|
||||||
"""Mock handler that returns an error."""
|
"""Mock handler that returns an error as MCP CallToolResult."""
|
||||||
return {"error": "Insufficient cash"}
|
return create_mcp_result({"error": "Insufficient cash"})
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -68,17 +76,17 @@ async def test_context_injector_injects_parameters(injector):
|
|||||||
"""Test that context parameters are injected into buy/sell requests."""
|
"""Test that context parameters are injected into buy/sell requests."""
|
||||||
request = MockRequest("buy", {"symbol": "AAPL", "amount": 10})
|
request = MockRequest("buy", {"symbol": "AAPL", "amount": 10})
|
||||||
|
|
||||||
# Mock handler that just returns the request args
|
# Mock handler that returns MCP result containing the request args
|
||||||
async def handler(req):
|
async def handler(req):
|
||||||
return req.args
|
return create_mcp_result(req.args)
|
||||||
|
|
||||||
result = await injector(request, handler)
|
result = await injector(request, handler)
|
||||||
|
|
||||||
# Verify context was injected
|
# Verify context was injected (result is MCP CallToolResult object)
|
||||||
assert result["signature"] == "test-model"
|
assert result.structuredContent["signature"] == "test-model"
|
||||||
assert result["today_date"] == "2025-01-15"
|
assert result.structuredContent["today_date"] == "2025-01-15"
|
||||||
assert result["job_id"] == "test-job-123"
|
assert result.structuredContent["job_id"] == "test-job-123"
|
||||||
assert result["trading_day_id"] == 1
|
assert result.structuredContent["trading_day_id"] == 1
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -132,7 +140,7 @@ async def test_context_injector_does_not_update_position_on_error(injector):
|
|||||||
|
|
||||||
# Verify position was NOT updated
|
# Verify position was NOT updated
|
||||||
assert injector._current_position == original_position
|
assert injector._current_position == original_position
|
||||||
assert "error" in result
|
assert "error" in result.structuredContent
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -146,7 +154,7 @@ async def test_context_injector_does_not_inject_position_for_non_trade_tools(inj
|
|||||||
|
|
||||||
async def verify_no_injection_handler(req):
|
async def verify_no_injection_handler(req):
|
||||||
assert "_current_position" not in req.args
|
assert "_current_position" not in req.args
|
||||||
return {"results": []}
|
return create_mcp_result({"results": []})
|
||||||
|
|
||||||
await injector(request, verify_no_injection_handler)
|
await injector(request, verify_no_injection_handler)
|
||||||
|
|
||||||
@@ -164,7 +172,7 @@ async def test_context_injector_full_trading_session_simulation(injector):
|
|||||||
async def handler1(req):
|
async def handler1(req):
|
||||||
# First trade should NOT have injected position
|
# First trade should NOT have injected position
|
||||||
assert req.args.get("_current_position") is None
|
assert req.args.get("_current_position") is None
|
||||||
return {"CASH": 1100.0, "AAPL": 7}
|
return create_mcp_result({"CASH": 1100.0, "AAPL": 7})
|
||||||
|
|
||||||
result1 = await injector(request1, handler1)
|
result1 = await injector(request1, handler1)
|
||||||
assert injector._current_position == {"CASH": 1100.0, "AAPL": 7}
|
assert injector._current_position == {"CASH": 1100.0, "AAPL": 7}
|
||||||
@@ -176,7 +184,7 @@ async def test_context_injector_full_trading_session_simulation(injector):
|
|||||||
# Second trade SHOULD have injected position from trade 1
|
# Second trade SHOULD have injected position from trade 1
|
||||||
assert req.args["_current_position"]["CASH"] == 1100.0
|
assert req.args["_current_position"]["CASH"] == 1100.0
|
||||||
assert req.args["_current_position"]["AAPL"] == 7
|
assert req.args["_current_position"]["AAPL"] == 7
|
||||||
return {"CASH": 50.0, "AAPL": 7, "MSFT": 7}
|
return create_mcp_result({"CASH": 50.0, "AAPL": 7, "MSFT": 7})
|
||||||
|
|
||||||
result2 = await injector(request2, handler2)
|
result2 = await injector(request2, handler2)
|
||||||
assert injector._current_position == {"CASH": 50.0, "AAPL": 7, "MSFT": 7}
|
assert injector._current_position == {"CASH": 50.0, "AAPL": 7, "MSFT": 7}
|
||||||
@@ -185,7 +193,7 @@ async def test_context_injector_full_trading_session_simulation(injector):
|
|||||||
request3 = MockRequest("buy", {"symbol": "GOOGL", "amount": 100})
|
request3 = MockRequest("buy", {"symbol": "GOOGL", "amount": 100})
|
||||||
|
|
||||||
async def handler3(req):
|
async def handler3(req):
|
||||||
return {"error": "Insufficient cash", "cash_available": 50.0}
|
return create_mcp_result({"error": "Insufficient cash", "cash_available": 50.0})
|
||||||
|
|
||||||
result3 = await injector(request3, handler3)
|
result3 = await injector(request3, handler3)
|
||||||
# Position should remain unchanged after failed trade
|
# Position should remain unchanged after failed trade
|
||||||
|
|||||||
Reference in New Issue
Block a user