Skip to content

Commit 5332c2e

Browse files
authored
Python: Require tool_call_id parameter for string-based tool messages in ChatHistory (#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 #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 😄
1 parent d4f6703 commit 5332c2e

File tree

6 files changed

+3432
-3388
lines changed

6 files changed

+3432
-3388
lines changed

.github/workflows/python-test-coverage.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
cache-suffix: ${{ runner.os }}-${{ env.UV_PYTHON }}
3535
cache-dependency-glob: "**/uv.lock"
3636
- name: Install the project
37-
run: uv sync --all-extras --dev
37+
run: uv sync --all-extras --dev -U --prerelease=if-necessary-or-explicit
3838
- name: Test with pytest
3939
run: uv run --frozen pytest -q --junitxml=pytest.xml --cov=semantic_kernel --cov-report=term-missing:skip-covered --cov-report=xml:python-coverage.xml ./tests/unit
4040
- name: Upload coverage report

python/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ dependencies = [
3737
"numpy >= 1.25.0; python_version < '3.12'",
3838
"numpy >= 1.26.0; python_version >= '3.12'",
3939
# openai connector
40-
"openai >= 1.67",
40+
"openai >= 1.91.1",
4141
# openapi and swagger
4242
"openapi_core >= 0.18,<0.20",
4343
"websockets >= 13, < 16",

python/semantic_kernel/contents/chat_history.py

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
from semantic_kernel.contents.chat_message_content import ChatMessageContent
1414
from semantic_kernel.contents.const import CHAT_HISTORY_TAG, CHAT_MESSAGE_CONTENT_TAG
15+
from semantic_kernel.contents.function_result_content import FunctionResultContent
1516
from semantic_kernel.contents.kernel_content import KernelContent
1617
from semantic_kernel.contents.utils.author_role import AuthorRole
1718
from semantic_kernel.exceptions import ContentInitializationError, ContentSerializationError
@@ -154,15 +155,29 @@ def add_tool_message(self, content: str | list[KernelContent], **kwargs: Any) ->
154155
"""Add a tool message to the chat history.
155156
156157
Args:
157-
content: The content of the tool message, can be a string or a
158-
list of KernelContent instances that are turned into a single ChatMessageContent.
159-
**kwargs: Additional keyword arguments.
158+
content: The content of the tool message. If a string, tool_call_id must be provided
159+
as a keyword argument. If a list of KernelContent instances, they should contain
160+
properly configured FunctionResultContent objects.
161+
**kwargs: Additional keyword arguments. For string content, tool_call_id is required.
162+
Optionally one may provide function_name to specify the tool function name. The
163+
function_name is only used for bookkeeping purposes as part of ChatHistory and is
164+
not included in the call to the model.
160165
"""
161166
raise NotImplementedError
162167

163168
@add_tool_message.register
164169
def _(self, content: str, **kwargs: Any) -> None:
165-
"""Add a tool message to the chat history."""
170+
"""Add a tool message to the chat history.
171+
172+
Args:
173+
content: The result content of the tool call.
174+
**kwargs: Additional keyword arguments. 'tool_call_id' is required when using string content.
175+
"""
176+
if "tool_call_id" not in kwargs:
177+
raise ContentInitializationError(
178+
"tool_call_id is required when adding a tool message with string content. "
179+
"Tool messages must reference the specific tool call they respond to."
180+
)
166181
self.add_message(message=self._prepare_for_add(role=AuthorRole.TOOL, content=content, **kwargs))
167182

168183
@add_tool_message.register(list)
@@ -203,9 +218,21 @@ def _prepare_for_add(
203218
) -> dict[str, str]:
204219
"""Prepare a message to be added to the history."""
205220
kwargs["role"] = role
206-
if content:
221+
222+
if role == AuthorRole.TOOL and content and not items:
223+
tool_call_id = kwargs.pop("tool_call_id", None)
224+
function_name = kwargs.pop("function_name", "unknown")
225+
function_result_content = FunctionResultContent(
226+
function_name=function_name,
227+
result=content,
228+
id=tool_call_id, # Set both id and call_id for compatibility
229+
call_id=tool_call_id,
230+
**kwargs,
231+
)
232+
kwargs["items"] = [function_result_content]
233+
elif content:
207234
kwargs["content"] = content
208-
if items:
235+
elif items:
209236
kwargs["items"] = items
210237
return kwargs
211238

python/tests/unit/agents/openai_responses/test_openai_responses_thread_actions.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from openai.types.responses.response_output_message import ResponseOutputMessage
1212
from openai.types.responses.response_output_text import ResponseOutputText
1313
from openai.types.responses.response_stream_event import ResponseStreamEvent
14-
from openai.types.responses.response_text_delta_event import ResponseTextDeltaEvent
14+
from openai.types.responses.response_text_delta_event import Logprob, ResponseTextDeltaEvent
1515

1616
from semantic_kernel.agents.open_ai.openai_responses_agent import OpenAIResponsesAgent
1717
from semantic_kernel.agents.open_ai.responses_agent_thread_actions import ResponsesAgentThreadActions
@@ -250,6 +250,7 @@ async def __anext__(self):
250250
delta="Test partial content",
251251
content_index=0,
252252
item_id="fake-item-id",
253+
logprobs=[Logprob(token="test_token", logprob=0.3)],
253254
output_index=0,
254255
type="response.output_text.delta",
255256
sequence_number=0,

python/tests/unit/contents/test_chat_history.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,30 @@ def test_add_assistant_message_list(chat_history: ChatHistory):
108108
assert chat_history.messages[-1].role == AuthorRole.ASSISTANT
109109

110110

111+
def test_add_tool_message_raises_without_tool_call_id(chat_history: ChatHistory):
112+
content = "Tool message"
113+
with pytest.raises(ContentInitializationError):
114+
chat_history.add_tool_message(content)
115+
116+
111117
def test_add_tool_message(chat_history: ChatHistory):
112118
content = "Tool message"
113-
chat_history.add_tool_message(content)
114-
assert chat_history.messages[-1].content == content
119+
chat_history.add_tool_message(content, tool_call_id="call_123")
120+
121+
122+
def test_add_tool_message_to_dict_succeeds(chat_history: ChatHistory):
123+
content = "Tool message"
124+
chat_history.add_tool_message(content, tool_call_id="call_123", function_name="test_function")
115125
assert chat_history.messages[-1].role == AuthorRole.TOOL
116126

127+
msg = chat_history.messages[-1]
128+
assert isinstance(msg.items[0], FunctionResultContent)
129+
assert msg.items[0].function_name == "test_function"
130+
result = msg.to_dict()
131+
assert result["content"] == content
132+
assert result["role"] == AuthorRole.TOOL
133+
assert result["tool_call_id"] == "call_123"
134+
117135

118136
def test_add_tool_message_list(chat_history: ChatHistory):
119137
content = [FunctionResultContent(id="test", result="Tool message")]

0 commit comments

Comments
 (0)