Compare commits

...

3 Commits

Author SHA1 Message Date
bee6afe531 test: update ContextInjector tests to match production MCP behavior
Update unit tests to mock CallToolResult objects instead of plain dicts,
matching actual MCP tool behavior in production.

Changes:
- Add create_mcp_result() helper to create mock CallToolResult objects
- Update all mock handlers to return MCP result objects
- Update assertions to access result.structuredContent field
- Maintains test coverage while accurately reflecting production behavior

This ensures tests validate the actual code path used in production,
where MCP tools return CallToolResult objects with structuredContent
field containing the position dict.
2025-11-07 14:32:20 -05:00
f1f76b9a99 fix: extract position dict from CallToolResult.structuredContent
Fix negative cash bug where ContextInjector._current_position never updated.

Root cause: MCP tools return mcp.types.CallToolResult objects, not plain
dicts. The isinstance(result, dict) check always failed, preventing
_current_position from accumulating trades within a session.

This caused all trades to calculate from initial $10,000 position instead
of previous trade's ending position, resulting in negative cash balances
when total purchases exceeded $10,000.

Solution: Extract position dict from CallToolResult.structuredContent field
before validating. Maintains backward compatibility by handling both
CallToolResult objects (production) and plain dicts (unit tests).

Impact:
- Fixes negative cash positions (e.g., -$8,768.68 after 11 trades)
- Enables proper intra-day position tracking
- Validates sufficient cash before each trade based on cumulative position
- Trade tool responses now properly accumulate all holdings

Testing:
- All existing unit tests pass (handle plain dict results)
- Production logs confirm structuredContent extraction works
- Debug logging shows _current_position now updates after each trade
2025-11-07 14:24:48 -05:00
277714f664 debug: add comprehensive logging for position tracking bug investigation
Add debug logging to diagnose negative cash position issue where trades
calculate from initial $10,000 instead of accumulating.

Issue: After 11 trades, final cash shows -$8,768.68. Each trade appears
to calculate from $10,000 starting position instead of previous trade's
ending position.

Hypothesis: ContextInjector._current_position not updating after trades,
possibly due to MCP result type mismatch in isinstance(result, dict) check.

Debug logging added:
- agent/context_injector.py: Log MCP result type, content, and whether
  _current_position updates after each trade
- agent_tools/tool_trade.py: Log whether injected position is used vs
  DB query, and full contents of returned position dict

This will help identify:
1. What type is returned by MCP tool (dict vs other)
2. Whether _current_position is None on subsequent trades
3. What keys are present in returned position dicts

Related to issue where reasoning summary claims no trades executed
despite 4 sell orders being recorded.
2025-11-07 14:16:30 -05:00
3 changed files with 55 additions and 23 deletions

View File

@@ -88,9 +88,28 @@ class ContextInjector:
# Update position state after successful trade
if request.name in ["buy", "sell"]:
# Check if result is a valid position dict (not an error)
if isinstance(result, dict) and "error" not in result and "CASH" in result:
# 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 = result.copy()
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

@@ -132,11 +132,14 @@ 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:
@@ -189,6 +192,8 @@ 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

@@ -2,6 +2,7 @@
import pytest
from agent.context_injector import ContextInjector
from unittest.mock import Mock
@pytest.fixture
@@ -22,27 +23,34 @@ class MockRequest:
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):
"""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
if request.name == "sell":
return {
return create_mcp_result({
"CASH": 1100.0,
"AAPL": 7,
"MSFT": 5
}
})
elif request.name == "buy":
return {
return create_mcp_result({
"CASH": 50.0,
"AAPL": 7,
"MSFT": 12
}
return {}
})
return create_mcp_result({})
async def mock_handler_error(request):
"""Mock handler that returns an error."""
return {"error": "Insufficient cash"}
"""Mock handler that returns an error as MCP CallToolResult."""
return create_mcp_result({"error": "Insufficient cash"})
@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."""
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):
return req.args
return create_mcp_result(req.args)
result = await injector(request, handler)
# Verify context was injected
assert result["signature"] == "test-model"
assert result["today_date"] == "2025-01-15"
assert result["job_id"] == "test-job-123"
assert result["trading_day_id"] == 1
# Verify context was injected (result is MCP CallToolResult object)
assert result.structuredContent["signature"] == "test-model"
assert result.structuredContent["today_date"] == "2025-01-15"
assert result.structuredContent["job_id"] == "test-job-123"
assert result.structuredContent["trading_day_id"] == 1
@pytest.mark.asyncio
@@ -132,7 +140,7 @@ async def test_context_injector_does_not_update_position_on_error(injector):
# Verify position was NOT updated
assert injector._current_position == original_position
assert "error" in result
assert "error" in result.structuredContent
@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):
assert "_current_position" not in req.args
return {"results": []}
return create_mcp_result({"results": []})
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):
# First trade should NOT have injected position
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)
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
assert req.args["_current_position"]["CASH"] == 1100.0
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)
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})
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)
# Position should remain unchanged after failed trade