Repository navigation
Conversation
chat() always sent "tools": [] in the request body, even when the caller passed no tools: _copy_tools(None) yields an empty list and model_dump(exclude_none=True) keeps the key. On servers rendering Llama 3.x's chat template directly, the template's tools-is-not-none check treats the empty list as tools being available, routing the request into tool-calling mode. Normalize empty tool lists to absent at both chat() call sites (sync and async), so tools is only sent when tools are actually provided. Fixes ollama#767
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Tested this branch against the setup from #767: rkllama serving Llama 3.2 3B (RKLLM 1.3.0) on an RK3588, CPython 3.14.4, same script and server for both runs, only the library changed. The script calls ollama 0.6.3 (current release): This PR (0.6.4.dev2+g4ee9c5cfb): One note on impact for the description: on this server, LGTM from the affected setup. Thanks for the quick fix! And thank you for your work on this project. I couldn't have gotten the work on my project done without it. Repro script"""Before/after check for ollama-python #767 / PR #768.
Uses the ollama library directly (not asky, so no DropEmptyTools) against
rkllama, and reports two things per request:
- whether the request body sent to /api/chat contained a "tools" key
- whether the model answered with plain text or a JSON function call
Usage: python pr768_repro.py [host] [model]
"""
import json
import sys
from importlib.metadata import version
import httpx
from ollama import Client
HOST = sys.argv[1] if len(sys.argv) > 1 else "http://localhost:8080"
MODEL = sys.argv[2] if len(sys.argv) > 2 else "Llama-3.2-3B-Instruct-RKLLM1.3.0"
RUNS = 5
sent_tools = []
def log_request(request: httpx.Request) -> None:
if request.url.path.endswith("/api/chat"):
body = json.loads(request.content)
sent_tools.append(body.get("tools", "<absent>"))
client = Client(host=HOST, event_hooks={"request": [log_request]})
print(f"ollama-python {version('ollama')}, host {HOST}, model {MODEL}\n")
json_replies = 0
for i in range(1, RUNS + 1):
r = client.chat(
model=MODEL,
messages=[{"role": "user", "content": "Say hello in one short sentence."}],
options={"temperature": 0.2},
)
text = (r.message.content or "").strip()
is_call = bool(r.message.tool_calls) or text.startswith("{")
json_replies += is_call
print(f"{i}. tools sent: {sent_tools[-1]!s:9} reply: "
f"{'JSON CALL' if is_call else 'text '} {text[:60]!r}")
print(f"\n{json_replies}/{RUNS} replies were JSON function calls")Note: the script’s docstring mentions asky, the (unpublished) TUI where I hit this. It carries a client-side workaround, DropEmptyTools, so this test calls your library directly instead: the only things that changed between the two runs are 0.6.3 and this PR, on the same hardware that surfaced the issue. -HB |
|
Thanks for testing it on the affected setup, hungry-bogart - really useful confirmation. Good point on the impact description: accepted-but-silently-switched-into-tool-mode is worse than a rejection. I'll add that to the PR body. |
Fixes #767.
Every chat() request included "tools": [] in the JSON body, whether tools was omitted or passed as an empty list, because the copied list was passed through even when empty. Endpoints that reject an empty tools array fail on every chat call.
Both the sync and async chat() now pass tools=None when the copied list is empty, so the field is omitted from the request body.
Tests: removed the stale 'tools': [] expectations in the request-body tests that encoded the old behavior, and added two tests: tools=[] omits the field, and a function tool is still sent. Full suite passes (119 tests); ruff check and format clean.
Impact note from the issue reporter's verification on affected hardware (rkllama serving Llama 3.2 3B on RK3588): some servers do not reject "tools": [] - rkllama accepts it and the Llama 3.x template (tools is not none) silently switches the model into tool-calling mode, so every reply comes back as a JSON function call with a 200 status and no error. With the field omitted, the same setup answers with plain text (5/5 JSON calls before, 0/5 after).