-
Notifications
You must be signed in to change notification settings - Fork 43
fix: preserve tool_calls in OpenAI input messages #335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fix: preserve tool_calls in OpenAI input messages #335
Conversation
- Add helper function to format tool calls from both API responses and input messages - Handle multimodal content and tool calls in structured format - Maintain backwards compatibility for simple string content - Add test coverage for messages with tool calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 1 comment
# Third message should be the tool response | ||
assert props["$ai_input"][2] == { | ||
"role": "tool", | ||
"content": "72 degrees and sunny", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Missing tool_call_id
field validation in the tool message assertion - should verify this field is preserved
# Third message should be the tool response | |
assert props["$ai_input"][2] == { | |
"role": "tool", | |
"content": "72 degrees and sunny", | |
} | |
# Third message should be the tool response | |
assert props["$ai_input"][2] == { | |
"role": "tool", | |
"tool_call_id": "call_abc123", | |
"content": "72 degrees and sunny", | |
} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/ai/openai/test_openai.py
Line: 1245:1249
Comment:
**style:** Missing `tool_call_id` field validation in the tool message assertion - should verify this field is preserved
```suggestion
# Third message should be the tool response
assert props["$ai_input"][2] == {
"role": "tool",
"tool_call_id": "call_abc123",
"content": "72 degrees and sunny",
}
```
How can I resolve this? If you propose a fix, please make it concise.
- Extract and preserve tool_call_id from input messages - Keep simple string content for tool messages (with optional tool_call_id) - Update test to verify tool_call_id is preserved for tool responses 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
"content": content_value, | ||
} | ||
# Preserve tool_call_id for tool role messages | ||
if tool_call_id: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a tool_call_id
if we don't have any tool_calls
?
"content": content_items, | ||
} | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to change anything for the responses API format?
} | ||
) | ||
content.extend( | ||
_format_tool_calls_to_content(choice.message.tool_calls) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes it for the chat completions response API format. What about the responses API format?
assert props["$ai_tools"] == tools | ||
|
||
|
||
def test_input_messages_with_tool_calls(mock_client, mock_openai_response): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think that there are other tests that are testing things with tool_calls
? Maybe we can just put some extra assertions there?
We should probably also test some of the edge cases that you have in your code
Problem
Tool calls in input messages were being dropped during formatting, causing loss of conversation context when passing multi-turn conversations with tool usage to OpenAI's API.
Solution
Added proper handling to preserve
tool_calls
in input messages by:_format_tool_calls_to_content()
helper function that handles both object-based tool calls (from API responses) and dict-based tool calls (from user input)format_openai_input()
to detect and preserve tool calls alongside text contentChanges
Testing
test_input_messages_with_tool_calls()
to verify tool call preservation in multi-turn conversationsTested locally by sending some events using the fix branch of
posthog-python
:Previously the tool calls were being dropped in the assistant input messages that used
tool_calls
.(slack thread here for further context/background)