Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def _inner_get_response(
messages: Sequence[Message],
options: Mapping[str, Any],
stream: bool = False,
**kwargs: Any,
**kwargs: Any, # noqa: ARG002 — accepted for interface compatibility; not forwarded to Ollama
) -> Awaitable[ChatResponse] | ResponseStream[ChatResponseUpdate, ChatResponse]:
if stream:
# Streaming mode
Expand All @@ -360,7 +360,6 @@ async def _stream() -> AsyncIterable[ChatResponseUpdate]:
response_object: AsyncIterable[OllamaChatResponse] = await self.client.chat( # type: ignore[misc]
stream=True,
**options_dict,
**kwargs,
)
Comment thread
max-montes marked this conversation as resolved.
except Exception as ex:
raise ChatClientException(f"Ollama streaming chat request failed : {ex}", ex) from ex
Expand All @@ -378,7 +377,6 @@ async def _get_response() -> ChatResponse:
response: OllamaChatResponse = await self.client.chat( # type: ignore[misc]
stream=False,
**options_dict,
**kwargs,
)
Comment thread
max-montes marked this conversation as resolved.
except Exception as ex:
raise ChatClientException(f"Ollama chat request failed : {ex}", ex) from ex
Expand Down
56 changes: 56 additions & 0 deletions python/packages/ollama/tests/test_ollama_chat_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,62 @@ async def test_cmc(
assert result.text == "test"


@patch.object(AsyncClient, "chat", new_callable=AsyncMock)
async def test_cmc_ignores_unsupported_kwargs(
mock_chat: AsyncMock,
ollama_unit_test_env: dict[str, str],
chat_history: list[Message],
mock_chat_completion_response: OllamaChatResponse,
) -> None:
"""Verify that unsupported kwargs (e.g. allow_multiple_tool_calls) are
silently filtered out and never forwarded to ollama.AsyncClient.chat().

Regression test for: https://github.com/microsoft/agent-framework/issues/4402
"""
mock_chat.return_value = mock_chat_completion_response
chat_history.append(Message(text="hello world", role="user"))

ollama_client = OllamaChatClient()
result = await ollama_client.get_response(
messages=chat_history,
allow_multiple_tool_calls=True,
)

assert result.text == "test"
mock_chat.assert_called_once()
call_kwargs = mock_chat.call_args.kwargs
assert "allow_multiple_tool_calls" not in call_kwargs


@patch.object(AsyncClient, "chat", new_callable=AsyncMock)
async def test_cmc_streaming_ignores_unsupported_kwargs(
mock_chat: AsyncMock,
ollama_unit_test_env: dict[str, str],
chat_history: list[Message],
mock_streaming_chat_completion_response: AsyncStream[OllamaChatResponse],
) -> None:
"""Verify that unsupported kwargs are filtered in streaming mode too.

Regression test for: https://github.com/microsoft/agent-framework/issues/4402
"""
mock_chat.return_value = mock_streaming_chat_completion_response
chat_history.append(Message(text="hello world", role="user"))

ollama_client = OllamaChatClient()
result = ollama_client.get_response(
messages=chat_history,
stream=True,
allow_multiple_tool_calls=True,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is no longer a valid pattern, one should always use options and we have a PR open to remove kwargs in most places(#4581), I think we can close this PR for now

)

async for chunk in result:
assert chunk.text == "test"

mock_chat.assert_called_once()
call_kwargs = mock_chat.call_args.kwargs
assert "allow_multiple_tool_calls" not in call_kwargs


@patch.object(AsyncClient, "chat", new_callable=AsyncMock)
async def test_cmc_reasoning(
mock_chat: AsyncMock,
Expand Down