Skip to content

Python: Require tool_call_id parameter for string-based tool messages in ChatHistory#12753

Merged
moonbox3 merged 5 commits into
microsoft:mainfrom
moonbox3:chat-history-tool-msg-handling
Jul 23, 2025
Merged

Python: Require tool_call_id parameter for string-based tool messages in ChatHistory#12753
moonbox3 merged 5 commits into
microsoft:mainfrom
moonbox3:chat-history-tool-msg-handling

Conversation

@moonbox3

@moonbox3 moonbox3 commented Jul 21, 2025

Copy link
Copy Markdown
Collaborator

Motivation and Context

The add_tool_message method of ChatHistory accepts string content without requiring a tool_call_id parameter. This design is flawed from an API design perspective. It created tool messages that:

  1. Violated chat completion protocols: most LLM APIs (OpenAI, etc.) require tool messages to have a tool_call_id that references a previous function call
  2. Broke conversation flow: tool messages should always be responses to specific tool calls, maintaining the call-response relationship
  3. Created malformed message sequences: messages without proper tool call IDs would fail when sent to language models

Description

This PR modifies the add_tool_message string overload to require a tool_call_id parameter, which makes sure we have proper tool call protocol compliance.

  1. The string overload now validates that tool_call_id is provided and raises a clear error if missing
  2. Tool messages now create FunctionResultContent objects with both id and call_id set to the provided tool_call_id
  3. Added support for an optional function_name parameter for better bookkeeping
  4. Provides informative error messages explaining why tool_call_id is required

Before:

# This would create an invalid tool message without tool_call_id
# when the `.to_dict()` method is called on `ChatMessageContent`
chat_history.add_tool_message("Function result")  # Creates broken message and losing pairing to the corresponding `FunctionCallContent`.

After:

# Now requires tool_call_id for proper protocol compliance
chat_history.add_tool_message("Function result")  # raises clear error due to missing `tool_call_id`
chat_history.add_tool_message("Function result", tool_call_id="call_123")  # works correctly

# Optional function name for better bookkeeping
chat_history.add_tool_message("Function result", tool_call_id="call_123", function_name="get_weather") # works as well

Contribution Checklist

@moonbox3 moonbox3 self-assigned this Jul 21, 2025
@moonbox3 moonbox3 requested a review from a team as a code owner July 21, 2025 04:28
@moonbox3 moonbox3 added the python Pull requests for the Python Semantic Kernel label Jul 21, 2025
@moonbox3

moonbox3 commented Jul 21, 2025

Copy link
Copy Markdown
Collaborator Author

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
contents
   chat_history.py181497%103, 108, 309–310
TOTAL26679458082% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
3659 22 💤 0 ❌ 0 🔥 1m 53s ⏱️

@mattiasu96

Copy link
Copy Markdown

Small comment, could be nice to also add an utility function to add function call messages?

At the moment the library provides convenience methods for user, system and tool result, it would be consistent with the rest to also have a function to explicitly manage tool call messages in the chat history (besides using the standard add_message function)

@moonbox3

Copy link
Copy Markdown
Collaborator Author

Small comment, could be nice to also add an utility function to add function call messages?

At the moment the library provides convenience methods for user, system and tool result, it would be consistent with the rest to also have a function to explicitly manage tool call messages in the chat history (besides using the standard add_message function)

Thanks for the feedback. We can look into this in the next iteration.

@TaoChenOSU TaoChenOSU added this pull request to the merge queue Jul 22, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 22, 2025
@moonbox3 moonbox3 enabled auto-merge July 23, 2025 03:01
@moonbox3 moonbox3 added this pull request to the merge queue Jul 23, 2025
Merged via the queue into microsoft:main with commit 5332c2e Jul 23, 2025
28 checks passed
@moonbox3 moonbox3 deleted the chat-history-tool-msg-handling branch July 23, 2025 04:14
jcruzmot-te pushed a commit to thousandeyes/aia-semantic-kernel that referenced this pull request Sep 15, 2025
… in ChatHistory (microsoft#12753)

### Motivation and Context

The `add_tool_message` method of `ChatHistory` accepts string content
without requiring a `tool_call_id` parameter. This design is flawed from
an API design perspective. It created tool messages that:

1. Violated chat completion protocols: most LLM APIs (OpenAI, etc.)
require tool messages to have a `tool_call_id` that references a
previous function call
2. Broke conversation flow: tool messages should always be responses to
specific tool calls, maintaining the call-response relationship
3. Created malformed message sequences: messages without proper tool
call IDs would fail when sent to language models

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

This PR modifies the `add_tool_message` string overload to require a
`tool_call_id` parameter, which makes sure we have proper tool call
protocol compliance.

1. The string overload now validates that `tool_call_id` is provided and
raises a clear error if missing
2. Tool messages now create `FunctionResultContent` objects with both
`id` and `call_id` set to the provided `tool_call_id`
3. Added support for an optional `function_name` parameter for better
bookkeeping
4. Provides informative error messages explaining why `tool_call_id` is
required
- Closes microsoft#12744 

Before:

```python
# This would create an invalid tool message without tool_call_id
# when the `.to_dict()` method is called on `ChatMessageContent`
chat_history.add_tool_message("Function result")  # Creates broken message and losing pairing to the corresponding `FunctionCallContent`.
```

After: 

```python
# Now requires tool_call_id for proper protocol compliance
chat_history.add_tool_message("Function result")  # raises clear error due to missing `tool_call_id`
chat_history.add_tool_message("Function result", tool_call_id="call_123")  # works correctly

# Optional function name for better bookkeeping
chat_history.add_tool_message("Function result", tool_call_id="call_123", function_name="get_weather") # works as well
```

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Bug: adding TOOL message to ChatHistory causes validation error in ChatMessageContent

4 participants