Skip to content

Python: fix(ollama): omit tools when tool_choice is none - #8987

Open
ankit kumar (ankit2235) wants to merge 4 commits into
microsoft:mainfrom
ankit2235:ollama-tool-choice-none
Open

ankit kumar (ankit2235) wants to merge 4 commits into
microsoft:mainfrom
ankit2235:ollama-tool-choice-none

Conversation

@ankit2235

@ankit2235 ankit kumar (ankit2235) commented Oct 2, 2026 •

Copy link
Copy Markdown

Motivation & Context

OllamaChatClient drops tool_choice but still sends tools, so tool_choice="none" has no effect. This also affects callers who never set it: when the function invocation limit is reached, the loop makes a final request with tool_choice="none" to get a text answer. With Ollama the tools are offered again, the model may return another tool call, and the caller gets the fallback text instead of a model answer.

Description & Review Guide

  • What are the major changes? _prepare_options now omits tools from the request when the tool mode is none. Ollama has no tool choice parameter, so this is the only way to honor it. BedrockChatClient handles none the same way. The two tool_choice docstrings are updated to match.
  • What is the impact of these changes? With tool_choice="none", Ollama is no longer offered tools. Other tool choices behave as before: the tools are sent and tool_choice itself is not forwarded. tool_choice now goes through validate_tool_mode, so an invalid value raises ContentError instead of being ignored.
  • What do you want reviewers to focus on? Whether omitting the tools is the behavior you want for Ollama, given tool_choice is documented as unsupported there. The tests are offline with a mocked AsyncClient.chat; I have not run this against a live Ollama server.

Related Issue

Fixes #8986

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

@ankit2235

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Fixes Ollama connector behavior so tool_choice="none" is honored by omitting tools from the request (since Ollama lacks a native tool_choice parameter), aligning behavior with other connectors and preventing extra tool-call loops.

Changes:

  • Update OllamaChatClient._prepare_options to drop tools when validated tool mode is none and to validate tool_choice via validate_tool_mode.
  • Update tool_choice docstrings to document the special handling of none.
  • Add unit tests covering omission/retention of tools and the function-invocation-limit final request behavior.
File Description
python/​packages/​ollama/​agent_framework_ollama/​_chat_client.py Validates tool_choice and omits tools when mode is none; updates option docs.
python/​packages/​ollama/​tests/​test_ollama_chat_client.py Adds tests to ensure tools is omitted for tool_choice="none" and on the final request after max iterations.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/ollama/agent_framework_ollama/_chat_client.py Outdated
Comment thread python/packages/ollama/tests/test_ollama_chat_client.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The public options type still rejects the newly supported tool-choice values.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity OllamaChatOptions rejects the supported none tool_choice mode

python/​packages/​ollama/​agent_framework_ollama/​_chat_client.py:221

The newly supported none mode is still rejected by the exported OllamaChatOptions type: tool_choice: None prevents typed callers from passing either {"tool_choice": "none"} or {"tool_choice": {"mode": "none"}}. Remove this override to inherit ChatOptions.tool_choice, which already accepts both forms. The class-level documentation can retain the explanation of Ollama's behavior.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The localized fix matches the function-calling contract, with focused regression coverage and no identified blocking issues.

Review effort: Balanced
Findings: None

This branch was successfully deployed

1 active deployment
github-app-auth — 6366d5ff Deployed Oct 7, 2026 by ankit2235 via add_label #24619
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: OllamaChatClient still sends tools when tool_choice is "none"

3 participants