diff --git a/CHANGELOG.md b/CHANGELOG.md index 6105b42..b3ffe5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.4.0] - 2025-11-04 +## [0.4.1] - 2025-11-05 + +### Fixed +- Fixed Pydantic validation error for tool_calls when using DeepSeek and other AI providers that return `args` as JSON strings instead of dictionaries. Added `ToolCallArgsParsingWrapper` that monkey-patches ChatOpenAI's `_create_chat_result` method to parse string arguments before AIMessage construction. + +## [0.4.0] - 2025-11-05 ### BREAKING CHANGES @@ -130,6 +135,49 @@ New `/results?reasoning=full` returns: - Test coverage increased with 36+ new comprehensive tests - Documentation updated with complete API reference and database schema details +### Fixed +- **Critical:** Intra-day position tracking for sell-then-buy trades (e20dce7) + - Sell proceeds now immediately available for subsequent buy orders within same trading session + - ContextInjector maintains in-memory position state during trading sessions + - Position updates accumulate after each successful trade + - Enables agents to rebalance portfolios (sell + buy) in single session + - Added 13 comprehensive tests for position tracking +- **Critical:** Tool message extraction in conversation history (462de3a, abb9cd0) + - Fixed bug where tool messages (buy/sell trades) were not captured when agent completed in single step + - Tool extraction now happens BEFORE finish signal check + - Reasoning summaries now accurately reflect actual trades executed + - Resolves issue where summarizer saw 0 tools despite multiple trades +- Reasoning summary generation improvements (6d126db) + - Summaries now explicitly mention specific trades executed (symbols, quantities, actions) + - Added TRADES EXECUTED section highlighting tool calls + - Example: 'sold 1 GOOGL and 1 AMZN to reduce exposure' instead of 'maintain core holdings' +- Final holdings calculation accuracy (a8d912b) + - Final positions now calculated from actions instead of querying incomplete database records + - Correctly handles first trading day with multiple trades + - New `_calculate_final_position_from_actions()` method applies all trades to calculate final state + - Holdings now persist correctly across all trading days + - Added 3 comprehensive tests for final position calculation +- Holdings persistence between trading days (aa16480) + - Query now retrieves previous day's ending position as current day's starting position + - Changed query from `date <=` to `date <` to prevent returning incomplete current-day records + - Fixes empty starting_position/final_position in API responses despite successful trades + - Updated tests to verify correct previous-day retrieval +- Context injector trading_day_id synchronization (05620fa) + - ContextInjector now updated with trading_day_id after record creation + - Fixes "Trade failed: trading_day_id not found in runtime config" error + - MCP tools now correctly receive trading_day_id via context injection +- Schema migration compatibility fixes (7c71a04) + - Updated position queries to use new trading_days schema instead of obsolete positions table + - Removed obsolete add_no_trade_record_to_db function calls + - Fixes "no such table: positions" error + - Simplified _handle_trading_result logic +- Database referential integrity (9da65c2) + - Corrected Database default path from "data/trading.db" to "data/jobs.db" + - Ensures all components use same database file + - Fixes FOREIGN KEY constraint failures when creating trading_day records +- Debug logging cleanup (1e7bdb5) + - Removed verbose debug logging from ContextInjector for cleaner output + ## [0.3.1] - 2025-11-03 ### Fixed diff --git a/agent/base_agent/base_agent.py b/agent/base_agent/base_agent.py index 7314d11..3233eb4 100644 --- a/agent/base_agent/base_agent.py +++ b/agent/base_agent/base_agent.py @@ -33,6 +33,7 @@ from tools.deployment_config import ( from agent.context_injector import ContextInjector from agent.pnl_calculator import DailyPnLCalculator from agent.reasoning_summarizer import ReasoningSummarizer +from agent.chat_model_wrapper import ToolCallArgsParsingWrapper # Load environment variables load_dotenv() @@ -208,10 +209,10 @@ class BaseAgent: # Create AI model (mock in DEV mode, real in PROD mode) if is_dev_mode(): from agent.mock_provider import MockChatModel - self.model = MockChatModel(date="2025-01-01") # Date will be updated per session + base_model = MockChatModel(date="2025-01-01") # Date will be updated per session print(f"🤖 Using MockChatModel (DEV mode)") else: - self.model = ChatOpenAI( + base_model = ChatOpenAI( model=self.basemodel, base_url=self.openai_base_url, api_key=self.openai_api_key, @@ -219,6 +220,10 @@ class BaseAgent: timeout=30 ) print(f"🤖 Using {self.basemodel} (PROD mode)") + + # Wrap model to fix tool_calls args parsing + self.model = ToolCallArgsParsingWrapper(model=base_model) + print(f"✅ Applied tool_calls args parsing wrapper") except Exception as e: raise RuntimeError(f"❌ Failed to initialize AI model: {e}") @@ -541,7 +546,7 @@ Summary:""" # Update mock model date if in dev mode if is_dev_mode(): - self.model.date = today_date + self.model.wrapped_model.date = today_date # Get job_id from context injector job_id = self.context_injector.job_id if self.context_injector else get_config_value("JOB_ID") diff --git a/agent/chat_model_wrapper.py b/agent/chat_model_wrapper.py new file mode 100644 index 0000000..d326e29 --- /dev/null +++ b/agent/chat_model_wrapper.py @@ -0,0 +1,98 @@ +""" +Chat model wrapper to fix tool_calls args parsing issues. + +Some AI providers (like DeepSeek) return tool_calls.args as JSON strings instead +of dictionaries, causing Pydantic validation errors. This wrapper monkey-patches +the model to fix args before AIMessage construction. +""" + +import json +from typing import Any, List, Optional, Dict +from functools import wraps +from langchain_core.messages import AIMessage, BaseMessage + + +class ToolCallArgsParsingWrapper: + """ + Wrapper around ChatOpenAI that fixes tool_calls args parsing. + + This fixes the Pydantic validation error: + "Input should be a valid dictionary [type=dict_type, input_value='...', input_type=str]" + + Works by monkey-patching _create_chat_result to parse string args before + AIMessage construction. + """ + + def __init__(self, model: Any, **kwargs): + """ + Initialize wrapper around a chat model. + + Args: + model: The chat model to wrap (should be ChatOpenAI instance) + **kwargs: Additional parameters (ignored, for compatibility) + """ + self.wrapped_model = model + self._patch_model() + + def _patch_model(self): + """Monkey-patch the model's _create_chat_result to fix tool_calls args""" + if not hasattr(self.wrapped_model, '_create_chat_result'): + # Model doesn't have this method (e.g., MockChatModel), skip patching + return + + original_create_chat_result = self.wrapped_model._create_chat_result + + @wraps(original_create_chat_result) + def patched_create_chat_result(response: Any, generation_info: Optional[Dict] = None): + """Patched version that fixes tool_calls args before AIMessage construction""" + # Fix tool_calls in the response dict before passing to original method + response_dict = response if isinstance(response, dict) else response.model_dump() + + if 'choices' in response_dict: + for choice in response_dict['choices']: + if 'message' in choice and 'tool_calls' in choice['message']: + tool_calls = choice['message']['tool_calls'] + if tool_calls: + for tool_call in tool_calls: + if 'function' in tool_call and 'arguments' in tool_call['function']: + args = tool_call['function']['arguments'] + # Parse string arguments to dict + if isinstance(args, str): + try: + tool_call['function']['arguments'] = json.loads(args) + except json.JSONDecodeError: + # Keep as string if parsing fails + pass + + # Call original method with fixed response + return original_create_chat_result(response_dict, generation_info) + + # Replace the method + self.wrapped_model._create_chat_result = patched_create_chat_result + + @property + def _llm_type(self) -> str: + """Return identifier for this LLM type""" + if hasattr(self.wrapped_model, '_llm_type'): + return f"wrapped-{self.wrapped_model._llm_type}" + return "wrapped-chat-model" + + def __getattr__(self, name: str): + """Proxy all other attributes/methods to the wrapped model""" + return getattr(self.wrapped_model, name) + + def bind_tools(self, tools: Any, **kwargs): + """ + Bind tools to the wrapped model. + + Since we patch the model in-place, we can just delegate to the wrapped model. + """ + return self.wrapped_model.bind_tools(tools, **kwargs) + + def bind(self, **kwargs): + """ + Bind settings to the wrapped model. + + Since we patch the model in-place, we can just delegate to the wrapped model. + """ + return self.wrapped_model.bind(**kwargs) diff --git a/tests/unit/test_chat_model_wrapper.py b/tests/unit/test_chat_model_wrapper.py new file mode 100644 index 0000000..7bc9f26 --- /dev/null +++ b/tests/unit/test_chat_model_wrapper.py @@ -0,0 +1,216 @@ +""" +Unit tests for ChatModelWrapper - tool_calls args parsing fix +""" + +import json +import pytest +from unittest.mock import Mock, AsyncMock +from langchain_core.messages import AIMessage +from langchain_core.outputs import ChatResult, ChatGeneration + +from agent.chat_model_wrapper import ToolCallArgsParsingWrapper + + +class TestToolCallArgsParsingWrapper: + """Tests for ToolCallArgsParsingWrapper""" + + @pytest.fixture + def mock_model(self): + """Create a mock chat model""" + model = Mock() + model._llm_type = "mock-model" + return model + + @pytest.fixture + def wrapper(self, mock_model): + """Create a wrapper around mock model""" + return ToolCallArgsParsingWrapper(model=mock_model) + + def test_fix_tool_calls_with_string_args(self, wrapper): + """Test that string args are parsed to dict""" + # Create message with tool_calls where args is a JSON string + message = AIMessage( + content="", + tool_calls=[ + { + "name": "buy", + "args": '{"symbol": "AAPL", "amount": 10}', # String, not dict + "id": "call_123" + } + ] + ) + + fixed_message = wrapper._fix_tool_calls(message) + + # Check that args is now a dict + assert isinstance(fixed_message.tool_calls[0]['args'], dict) + assert fixed_message.tool_calls[0]['args'] == {"symbol": "AAPL", "amount": 10} + + def test_fix_tool_calls_with_dict_args(self, wrapper): + """Test that dict args are left unchanged""" + # Create message with tool_calls where args is already a dict + message = AIMessage( + content="", + tool_calls=[ + { + "name": "buy", + "args": {"symbol": "AAPL", "amount": 10}, # Already a dict + "id": "call_123" + } + ] + ) + + fixed_message = wrapper._fix_tool_calls(message) + + # Check that args is still a dict + assert isinstance(fixed_message.tool_calls[0]['args'], dict) + assert fixed_message.tool_calls[0]['args'] == {"symbol": "AAPL", "amount": 10} + + def test_fix_tool_calls_with_invalid_json(self, wrapper): + """Test that invalid JSON string is left unchanged""" + # Create message with tool_calls where args is an invalid JSON string + message = AIMessage( + content="", + tool_calls=[ + { + "name": "buy", + "args": 'invalid json {', # Invalid JSON + "id": "call_123" + } + ] + ) + + fixed_message = wrapper._fix_tool_calls(message) + + # Check that args is still a string (parsing failed) + assert isinstance(fixed_message.tool_calls[0]['args'], str) + assert fixed_message.tool_calls[0]['args'] == 'invalid json {' + + def test_fix_tool_calls_no_tool_calls(self, wrapper): + """Test that messages without tool_calls are left unchanged""" + message = AIMessage(content="Hello, world!") + fixed_message = wrapper._fix_tool_calls(message) + + assert fixed_message == message + + def test_generate_with_string_args(self, wrapper, mock_model): + """Test _generate method with string args""" + # Create a response with string args + original_message = AIMessage( + content="", + tool_calls=[ + { + "name": "buy", + "args": '{"symbol": "MSFT", "amount": 5}', + "id": "call_456" + } + ] + ) + + mock_result = ChatResult( + generations=[ChatGeneration(message=original_message)] + ) + mock_model._generate.return_value = mock_result + + # Call wrapper's _generate + result = wrapper._generate(messages=[], stop=None, run_manager=None) + + # Check that args is now a dict + fixed_message = result.generations[0].message + assert isinstance(fixed_message.tool_calls[0]['args'], dict) + assert fixed_message.tool_calls[0]['args'] == {"symbol": "MSFT", "amount": 5} + + @pytest.mark.asyncio + async def test_agenerate_with_string_args(self, wrapper, mock_model): + """Test _agenerate method with string args""" + # Create a response with string args + original_message = AIMessage( + content="", + tool_calls=[ + { + "name": "sell", + "args": '{"symbol": "GOOGL", "amount": 3}', + "id": "call_789" + } + ] + ) + + mock_result = ChatResult( + generations=[ChatGeneration(message=original_message)] + ) + mock_model._agenerate = AsyncMock(return_value=mock_result) + + # Call wrapper's _agenerate + result = await wrapper._agenerate(messages=[], stop=None, run_manager=None) + + # Check that args is now a dict + fixed_message = result.generations[0].message + assert isinstance(fixed_message.tool_calls[0]['args'], dict) + assert fixed_message.tool_calls[0]['args'] == {"symbol": "GOOGL", "amount": 3} + + def test_invoke_with_string_args(self, wrapper, mock_model): + """Test invoke method with string args""" + original_message = AIMessage( + content="", + tool_calls=[ + { + "name": "buy", + "args": '{"symbol": "NVDA", "amount": 20}', + "id": "call_999" + } + ] + ) + + mock_model.invoke.return_value = original_message + + # Call wrapper's invoke + result = wrapper.invoke(input=[]) + + # Check that args is now a dict + assert isinstance(result.tool_calls[0]['args'], dict) + assert result.tool_calls[0]['args'] == {"symbol": "NVDA", "amount": 20} + + @pytest.mark.asyncio + async def test_ainvoke_with_string_args(self, wrapper, mock_model): + """Test ainvoke method with string args""" + original_message = AIMessage( + content="", + tool_calls=[ + { + "name": "sell", + "args": '{"symbol": "TSLA", "amount": 15}', + "id": "call_111" + } + ] + ) + + mock_model.ainvoke = AsyncMock(return_value=original_message) + + # Call wrapper's ainvoke + result = await wrapper.ainvoke(input=[]) + + # Check that args is now a dict + assert isinstance(result.tool_calls[0]['args'], dict) + assert result.tool_calls[0]['args'] == {"symbol": "TSLA", "amount": 15} + + def test_bind_tools_returns_wrapper(self, wrapper, mock_model): + """Test that bind_tools returns a new wrapper""" + mock_bound = Mock() + mock_model.bind_tools.return_value = mock_bound + + result = wrapper.bind_tools(tools=[], strict=True) + + # Check that result is a wrapper around the bound model + assert isinstance(result, ToolCallArgsParsingWrapper) + assert result.wrapped_model == mock_bound + + def test_bind_returns_wrapper(self, wrapper, mock_model): + """Test that bind returns a new wrapper""" + mock_bound = Mock() + mock_model.bind.return_value = mock_bound + + result = wrapper.bind(max_tokens=100) + + # Check that result is a wrapper around the bound model + assert isinstance(result, ToolCallArgsParsingWrapper) + assert result.wrapped_model == mock_bound