Skip to content

Commit

Permalink
Python: Refactor completion integration tests (#7905)
Browse files Browse the repository at this point in the history
### Motivation and Context

<!-- 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.
-->
We currently have two integration test modules for testing our
completion services: one for chat completion and one for text
completion.

We have many use cases, including image inputs, tool calls outputs, and
others. They all live in single test modules. This makes the them hard
to maintain, especially when we are adding more connectors.

We are also only testing the output contents of the services, not the
types. Testing only the output contents can result in flaky tests
because the models don’t always return what we expect them to return.
Checking the types of the contents makes the tests more robust.

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
1. Create a base class to enforce the structure of the completion test
modules.
2. Create two new test modules for image content and function calling
contents.
3. Simply the original text completion test and chat completion test to
only test for the simplest scenarios.
4. No longer check if the services return specific words. They are
considered passing as long as they return something non empty.
5. Bug fixes on function calling for Azure AI Inference and the Google
connectors.


### 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 😄
  • Loading branch information
TaoChenOSU authored Aug 15, 2024
1 parent f9247da commit 2d31245
Show file tree
Hide file tree
Showing 11 changed files with 1,408 additions and 766 deletions.
5 changes: 4 additions & 1 deletion python/.cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
"serde",
"datamodel",
"vectorstoremodel",
"qdrant"
"qdrant",
"huggingface",
"pytestmark",
"contoso"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,19 +135,20 @@ async def get_chat_message_contents(
settings = self.get_prompt_execution_settings_from_settings(settings)
assert isinstance(settings, AzureAIInferenceChatPromptExecutionSettings) # nosec

kernel = kwargs.get("kernel")
if settings.function_choice_behavior is not None and (not kernel or not isinstance(kernel, Kernel)):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

if kernel and settings.function_choice_behavior:
self._verify_function_choice_behavior(settings)
self._configure_function_choice_behavior(settings, kernel)

if (
settings.function_choice_behavior is None
or not settings.function_choice_behavior.auto_invoke_kernel_functions
):
return await self._send_chat_request(chat_history, settings)

kernel = kwargs.get("kernel")
if not isinstance(kernel, Kernel):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

self._verify_function_choice_behavior(settings)
self._configure_function_choice_behavior(settings, kernel)

for request_index in range(settings.function_choice_behavior.maximum_auto_invoke_attempts):
completions = await self._send_chat_request(chat_history, settings)
chat_history.add_message(message=completions[0])
Expand All @@ -158,7 +159,7 @@ async def get_chat_message_contents(
results = await self._invoke_function_calls(
function_calls=function_calls,
chat_history=chat_history,
kernel=kernel,
kernel=kernel, # type: ignore
arguments=kwargs.get("arguments", None),
function_call_count=fc_count,
request_index=request_index,
Expand Down Expand Up @@ -248,6 +249,14 @@ async def get_streaming_chat_message_contents(
settings = self.get_prompt_execution_settings_from_settings(settings)
assert isinstance(settings, AzureAIInferenceChatPromptExecutionSettings) # nosec

kernel = kwargs.get("kernel")
if settings.function_choice_behavior is not None and (not kernel or not isinstance(kernel, Kernel)):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

if kernel and settings.function_choice_behavior:
self._verify_function_choice_behavior(settings)
self._configure_function_choice_behavior(settings, kernel)

if (
settings.function_choice_behavior is None
or not settings.function_choice_behavior.auto_invoke_kernel_functions
Expand All @@ -256,25 +265,24 @@ async def get_streaming_chat_message_contents(
async_generator = self._send_chat_streaming_request(chat_history, settings)
else:
# Auto invoke is required.
async_generator = self._get_streaming_chat_message_contents_auto_invoke(chat_history, settings, **kwargs)
async_generator = self._get_streaming_chat_message_contents_auto_invoke(
kernel, # type: ignore
kwargs.get("arguments"),
chat_history,
settings,
)

async for messages in async_generator:
yield messages

async def _get_streaming_chat_message_contents_auto_invoke(
self,
kernel: Kernel,
arguments: KernelArguments | None,
chat_history: ChatHistory,
settings: AzureAIInferenceChatPromptExecutionSettings,
**kwargs: Any,
) -> AsyncGenerator[list[StreamingChatMessageContent], Any]:
"""Get streaming chat message contents from the Azure AI Inference service with auto invoking functions."""
kernel = kwargs.get("kernel")
if not isinstance(kernel, Kernel):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

self._verify_function_choice_behavior(settings)
self._configure_function_choice_behavior(settings, kernel)

# mypy doesn't recognize the settings.function_choice_behavior is not None by the check above
request_attempts = settings.function_choice_behavior.maximum_auto_invoke_attempts # type: ignore

Expand All @@ -301,7 +309,7 @@ async def _get_streaming_chat_message_contents_auto_invoke(
function_calls=function_calls,
chat_history=chat_history,
kernel=kernel,
arguments=kwargs.get("arguments", None),
arguments=arguments,
function_call_count=len(function_calls),
request_index=request_index,
# mypy doesn't recognize the settings.function_choice_behavior is not None by the check above
Expand Down Expand Up @@ -412,8 +420,6 @@ def _get_metadata_from_response(self, response: ChatCompletions | StreamingChatC

def _verify_function_choice_behavior(self, settings: AzureAIInferenceChatPromptExecutionSettings):
"""Verify the function choice behavior."""
if not settings.function_choice_behavior:
raise ServiceInvalidExecutionSettingsError("Function choice behavior is required for tool calls.")
if settings.extra_parameters is not None and settings.extra_parameters.get("n", 1) > 1:
# Currently only OpenAI models allow multiple completions but the Azure AI Inference service
# does not expose the functionality directly. If users want to have more than 1 responses, they
Expand All @@ -427,7 +433,7 @@ def _configure_function_choice_behavior(
):
"""Configure the function choice behavior to include the kernel functions."""
if not settings.function_choice_behavior:
raise ServiceInvalidExecutionSettingsError("Function choice behavior is required for tool calls.")
return

settings.function_choice_behavior.configure(
kernel=kernel, update_settings_callback=update_settings_from_function_call_configuration, settings=settings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from semantic_kernel.contents.text_content import TextContent
from semantic_kernel.contents.utils.author_role import AuthorRole
from semantic_kernel.contents.utils.finish_reason import FinishReason
from semantic_kernel.functions.kernel_arguments import KernelArguments
from semantic_kernel.kernel import Kernel

if sys.version_info >= (3, 12):
Expand Down Expand Up @@ -116,18 +117,19 @@ async def get_chat_message_contents(
settings = self.get_prompt_execution_settings_from_settings(settings)
assert isinstance(settings, GoogleAIChatPromptExecutionSettings) # nosec

kernel = kwargs.get("kernel")
if settings.function_choice_behavior is not None and (not kernel or not isinstance(kernel, Kernel)):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

if kernel and settings.function_choice_behavior:
configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

if (
settings.function_choice_behavior is None
or not settings.function_choice_behavior.auto_invoke_kernel_functions
):
return await self._send_chat_request(chat_history, settings)

kernel = kwargs.get("kernel")
if not isinstance(kernel, Kernel):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

for request_index in range(settings.function_choice_behavior.maximum_auto_invoke_attempts):
completions = await self._send_chat_request(chat_history, settings)
chat_history.add_message(message=completions[0])
Expand All @@ -138,7 +140,7 @@ async def get_chat_message_contents(
results = await invoke_function_calls(
function_calls=function_calls,
chat_history=chat_history,
kernel=kernel,
kernel=kernel, # type: ignore
arguments=kwargs.get("arguments", None),
function_call_count=fc_count,
request_index=request_index,
Expand Down Expand Up @@ -224,6 +226,13 @@ async def get_streaming_chat_message_contents(
settings = self.get_prompt_execution_settings_from_settings(settings)
assert isinstance(settings, GoogleAIChatPromptExecutionSettings) # nosec

kernel = kwargs.get("kernel")
if settings.function_choice_behavior is not None and (not kernel or not isinstance(kernel, Kernel)):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

if kernel and settings.function_choice_behavior:
configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

if (
settings.function_choice_behavior is None
or not settings.function_choice_behavior.auto_invoke_kernel_functions
Expand All @@ -232,28 +241,29 @@ async def get_streaming_chat_message_contents(
async_generator = self._send_chat_streaming_request(chat_history, settings)
else:
# Auto invoke is required.
async_generator = self._get_streaming_chat_message_contents_auto_invoke(chat_history, settings, **kwargs)
async_generator = self._get_streaming_chat_message_contents_auto_invoke(
kernel, # type: ignore
kwargs.get("arguments"),
chat_history,
settings,
)

async for messages in async_generator:
yield messages

async def _get_streaming_chat_message_contents_auto_invoke(
self,
kernel: Kernel,
arguments: KernelArguments | None,
chat_history: ChatHistory,
settings: GoogleAIChatPromptExecutionSettings,
**kwargs: Any,
) -> AsyncGenerator[list[StreamingChatMessageContent], Any]:
"""Get streaming chat message contents from the Google AI service with auto invoking functions."""
kernel = kwargs.get("kernel")
if not isinstance(kernel, Kernel):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")
if not settings.function_choice_behavior:
raise ServiceInvalidExecutionSettingsError(
"Function choice behavior is required for auto invoking functions."
)

configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

for request_index in range(settings.function_choice_behavior.maximum_auto_invoke_attempts):
all_messages: list[StreamingChatMessageContent] = []
function_call_returned = False
Expand All @@ -277,7 +287,7 @@ async def _get_streaming_chat_message_contents_auto_invoke(
function_calls=function_calls,
chat_history=chat_history,
kernel=kernel,
arguments=kwargs.get("arguments", None),
arguments=arguments,
function_call_count=len(function_calls),
request_index=request_index,
function_behavior=settings.function_choice_behavior,
Expand Down
7 changes: 2 additions & 5 deletions python/semantic_kernel/connectors/ai/google/shared_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@
from semantic_kernel.contents.function_call_content import FunctionCallContent
from semantic_kernel.contents.function_result_content import FunctionResultContent
from semantic_kernel.contents.utils.author_role import AuthorRole
from semantic_kernel.exceptions.service_exceptions import (
ServiceInvalidExecutionSettingsError,
ServiceInvalidRequestError,
)
from semantic_kernel.exceptions.service_exceptions import ServiceInvalidRequestError
from semantic_kernel.functions.kernel_arguments import KernelArguments
from semantic_kernel.functions.kernel_function_metadata import KernelFunctionMetadata
from semantic_kernel.kernel import Kernel
Expand Down Expand Up @@ -119,6 +116,6 @@ def configure_function_choice_behavior(
):
"""Configure the function choice behavior to include the kernel functions."""
if not settings.function_choice_behavior:
raise ServiceInvalidExecutionSettingsError("Function choice behavior is required for tool calls.")
return

settings.function_choice_behavior.configure(kernel=kernel, update_settings_callback=callback, settings=settings)
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
ServiceInitializationError,
ServiceInvalidExecutionSettingsError,
)
from semantic_kernel.functions.kernel_arguments import KernelArguments
from semantic_kernel.kernel import Kernel

if sys.version_info >= (3, 12):
Expand Down Expand Up @@ -110,18 +111,19 @@ async def get_chat_message_contents(
settings = self.get_prompt_execution_settings_from_settings(settings)
assert isinstance(settings, VertexAIChatPromptExecutionSettings) # nosec

kernel = kwargs.get("kernel")
if settings.function_choice_behavior is not None and (not kernel or not isinstance(kernel, Kernel)):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

if kernel and settings.function_choice_behavior:
configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

if (
settings.function_choice_behavior is None
or not settings.function_choice_behavior.auto_invoke_kernel_functions
):
return await self._send_chat_request(chat_history, settings)

kernel = kwargs.get("kernel")
if not isinstance(kernel, Kernel):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

for request_index in range(settings.function_choice_behavior.maximum_auto_invoke_attempts):
completions = await self._send_chat_request(chat_history, settings)
chat_history.add_message(message=completions[0])
Expand All @@ -132,7 +134,7 @@ async def get_chat_message_contents(
results = await invoke_function_calls(
function_calls=function_calls,
chat_history=chat_history,
kernel=kernel,
kernel=kernel, # type: ignore
arguments=kwargs.get("arguments", None),
function_call_count=fc_count,
request_index=request_index,
Expand Down Expand Up @@ -217,6 +219,13 @@ async def get_streaming_chat_message_contents(
settings = self.get_prompt_execution_settings_from_settings(settings)
assert isinstance(settings, VertexAIChatPromptExecutionSettings) # nosec

kernel = kwargs.get("kernel")
if settings.function_choice_behavior is not None and (not kernel or not isinstance(kernel, Kernel)):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")

if kernel and settings.function_choice_behavior:
configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

if (
settings.function_choice_behavior is None
or not settings.function_choice_behavior.auto_invoke_kernel_functions
Expand All @@ -225,28 +234,29 @@ async def get_streaming_chat_message_contents(
async_generator = self._send_chat_streaming_request(chat_history, settings)
else:
# Auto invoke is required.
async_generator = self._get_streaming_chat_message_contents_auto_invoke(chat_history, settings, **kwargs)
async_generator = self._get_streaming_chat_message_contents_auto_invoke(
kernel, # type: ignore
kwargs.get("arguments"),
chat_history,
settings,
)

async for messages in async_generator:
yield messages

async def _get_streaming_chat_message_contents_auto_invoke(
self,
kernel: Kernel,
arguments: KernelArguments | None,
chat_history: ChatHistory,
settings: VertexAIChatPromptExecutionSettings,
**kwargs: Any,
) -> AsyncGenerator[list[StreamingChatMessageContent], Any]:
"""Get streaming chat message contents from the Google AI service with auto invoking functions."""
kernel = kwargs.get("kernel")
if not isinstance(kernel, Kernel):
raise ServiceInvalidExecutionSettingsError("Kernel is required for auto invoking functions.")
if not settings.function_choice_behavior:
raise ServiceInvalidExecutionSettingsError(
"Function choice behavior is required for auto invoking functions."
)

configure_function_choice_behavior(settings, kernel, update_settings_from_function_choice_configuration)

for request_index in range(settings.function_choice_behavior.maximum_auto_invoke_attempts):
all_messages: list[StreamingChatMessageContent] = []
function_call_returned = False
Expand All @@ -270,7 +280,7 @@ async def _get_streaming_chat_message_contents_auto_invoke(
function_calls=function_calls,
chat_history=chat_history,
kernel=kernel,
arguments=kwargs.get("arguments", None),
arguments=arguments,
function_call_count=len(function_calls),
request_index=request_index,
function_behavior=settings.function_choice_behavior,
Expand Down
Loading

0 comments on commit 2d31245

Please sign in to comment.