Skip to content

Commit

Permalink
Python: FunctionResultContent hash fix to handle lists/sets (#9978)
Browse files Browse the repository at this point in the history
### Motivation and Context

Currently, the hash function in `FunctionResultContent` does not
properly handle types of list or set. Adding the proper handling to turn
these into a tuple, and then the hash function works properly.

<!-- 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:
- Makes sure the FunctionResultContent `__hash__` method can properly
handle mutable types like lists or sets
- Adds tests to ensure the new behavior works as expected
- Ensures the create message content method can handle lists or sets
when generating the content and the model can only accept a string.
- Fixes #9977

<!-- 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 😄

---------

Co-authored-by: Eduard van Valkenburg <[email protected]>
  • Loading branch information
moonbox3 and eavanvalkenburg authored Dec 16, 2024
1 parent 62a50f3 commit b427208
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,36 @@ def get_message_contents(message: "ChatMessageContent") -> list[dict[str, Any]]:
for content in message.items:
match content:
case TextContent():
contents.append({"type": "text", "text": content.text})
# Make sure text is a string
final_text = content.text
if not isinstance(final_text, str):
if isinstance(final_text, (list, tuple)):
final_text = " ".join(map(str, final_text))
else:
final_text = str(final_text)

contents.append({"type": "text", "text": final_text})

case ImageContent():
if content.uri:
contents.append(content.to_dict())

case FileReferenceContent():
contents.append({
"type": "image_file",
"image_file": {"file_id": content.file_id},
})

case FunctionResultContent():
contents.append({"type": "text", "text": content.result})
final_result = content.result
match final_result:
case str():
contents.append({"type": "text", "text": final_result})
case list() | tuple():
contents.append({"type": "text", "text": " ".join(map(str, final_result))})
case _:
contents.append({"type": "text", "text": str(final_result)})

return contents


Expand Down
17 changes: 16 additions & 1 deletion python/semantic_kernel/contents/function_result_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
if TYPE_CHECKING:
from semantic_kernel.contents.chat_message_content import ChatMessageContent
from semantic_kernel.contents.function_call_content import FunctionCallContent
from semantic_kernel.contents.streaming_chat_message_content import StreamingChatMessageContent
from semantic_kernel.functions.function_result import FunctionResult

TAG_CONTENT_MAP = {
Expand Down Expand Up @@ -157,6 +158,12 @@ def to_chat_message_content(self) -> "ChatMessageContent":

return ChatMessageContent(role=AuthorRole.TOOL, items=[self])

def to_streaming_chat_message_content(self) -> "StreamingChatMessageContent":
"""Convert the instance to a StreamingChatMessageContent."""
from semantic_kernel.contents.streaming_chat_message_content import StreamingChatMessageContent

return StreamingChatMessageContent(role=AuthorRole.TOOL, choice_index=0, items=[self])

def to_dict(self) -> dict[str, str]:
"""Convert the instance to a dictionary."""
return {
Expand Down Expand Up @@ -187,4 +194,12 @@ def serialize_result(self, value: Any) -> str:

def __hash__(self) -> int:
"""Return the hash of the function result content."""
return hash((self.tag, self.id, self.result, self.name, self.function_name, self.plugin_name, self.encoding))
return hash((
self.tag,
self.id,
tuple(self.result) if isinstance(self.result, list) else self.result,
self.name,
self.function_name,
self.plugin_name,
self.encoding,
))
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from html import unescape
from typing import TYPE_CHECKING, Any

import yaml
import yaml # type: ignore
from pydantic import Field, ValidationError, model_validator

from semantic_kernel.connectors.ai.chat_completion_client_base import ChatCompletionClientBase
Expand Down
2 changes: 2 additions & 0 deletions python/tests/unit/agents/test_open_ai_assistant_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,8 @@ def test_get_message_contents(azure_openai_assistant_agent: AzureAssistantAgent,
ImageContent(role=AuthorRole.ASSISTANT, content="test message", uri="http://image.url"),
TextContent(role=AuthorRole.ASSISTANT, text="test message"),
FileReferenceContent(role=AuthorRole.ASSISTANT, file_id="test_file_id"),
TextContent(role=AuthorRole.USER, text="test message"),
FunctionResultContent(role=AuthorRole.ASSISTANT, result=["test result"], id="test_id"),
]

result = get_message_contents(message)
Expand Down
33 changes: 31 additions & 2 deletions python/tests/unit/contents/test_function_result_content.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright (c) Microsoft. All rights reserved.


from typing import Any
from unittest.mock import Mock

import pytest
Expand All @@ -15,6 +14,26 @@
from semantic_kernel.functions.kernel_function_metadata import KernelFunctionMetadata


class CustomResultClass:
"""Custom class for testing."""

def __init__(self, result):
self.result = result

def __str__(self) -> str:
return self.result


class CustomObjectWithList:
"""Custom class for testing."""

def __init__(self, items):
self.items = items

def __str__(self):
return f"CustomObjectWithList({self.items})"


def test_init():
frc = FunctionResultContent(id="test", name="test-function", result="test-result", metadata={"test": "test"})
assert frc.name == "test-function"
Expand Down Expand Up @@ -50,6 +69,11 @@ def test_init_from_names():
ChatMessageContent(role="user", content="Hello world!"),
ChatMessageContent(role="user", items=[ImageContent(uri="https://example.com")]),
ChatMessageContent(role="user", items=[FunctionResultContent(id="test", name="test", result="Hello world!")]),
[1, 2, 3],
[{"key": "value"}, {"another": "item"}],
{"a", "b"},
CustomResultClass("test"),
CustomObjectWithList(["one", "two", "three"]),
],
ids=[
"str",
Expand All @@ -60,9 +84,14 @@ def test_init_from_names():
"ChatMessageContent",
"ChatMessageContent-ImageContent",
"ChatMessageContent-FunctionResultContent",
"list",
"list_of_dicts",
"set",
"CustomResultClass",
"CustomObjectWithList",
],
)
def test_from_fcc_and_result(result: Any):
def test_from_fcc_and_result(result: any):
fcc = FunctionCallContent(
id="test", name="test-function", arguments='{"input": "world"}', metadata={"test": "test"}
)
Expand Down

0 comments on commit b427208

Please sign in to comment.