[ankit kumar (ankit2235)]

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 handles tool_choice explicitly. Ollama has no tool choice parameter, so none is honored by omitting tools from the request (as BedrockChatClient does), and modes Ollama cannot enforce (required, with or without required_function_name, and allowed_tools) raise ChatClientInvalidRequestException instead of silently behaving like auto. tools is now set only in the dedicated tools section, and OllamaChatOptions no longer overrides tool_choice as None, so typed callers can pass "none". The class docstring is updated to match.
• What is the impact of these changes? With tool_choice="none", including the final request after the function invocation limit is reached, Ollama is no longer offered tools. No tool_choice or auto behaves as before. required and allowed_tools now raise ChatClientInvalidRequestException instead of being ignored, and an invalid tool_choice value raises ContentError.
• What do you want reviewers to focus on? 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.

[ankit kumar (ankit2235)]

@microsoft-github-policy-service agree

[Copilot]

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

Open (2)

• run_options.pop(\"tools\", None) works, but it makes the control flow less explicit because… · New
• This PR changes behavior so tool_choice is now validated via validate_tool_mode (invalid values… · New

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.

[Copilot]

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)

• This PR changes behavior so tool_choice is now validated via validate_tool_mode (invalid values…
• run_options.pop(\"tools\", None) works, but it makes the control flow less explicit because…

Previously missed (1)

In code that hasn't changed since last review

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]

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

[Copilot]

🟢 Approval recommended

The focused implementation and regression coverage support approval, with only a non-blocking PR description correction remaining.

1 open finding

• Disclose new exceptions for required and allowed_tools choices · New

🧠 Review effort: Balanced

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[Copilot]

🟢 Approval recommended

The focused fix has targeted regression coverage and no unresolved blocking findings.

0 open findings

1 resolved since last review

• Disclose new exceptions for required and allowed_tools choices

🧠 Review effort: Balanced

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.