Skip to content

Add max_tool_calls parameter for automatic tool execution#629

Open
HamzaYslmn wants to merge 1 commit intoollama:mainfrom
HamzaYslmn:main
Open

Add max_tool_calls parameter for automatic tool execution#629
HamzaYslmn wants to merge 1 commit intoollama:mainfrom
HamzaYslmn:main

Conversation

@HamzaYslmn
Copy link
Copy Markdown

This pull request introduces automatic tool execution for chat interactions in the ollama client, allowing tools to be executed in a loop up to a specified maximum number of times via a new max_tool_calls parameter. It updates both synchronous and asynchronous chat methods, enhances documentation and examples, and adds internal logic for executing tool calls. The changes improve usability for tool-augmented chat workflows and provide clearer guidance for both manual and automated tool handling.

Automatic tool execution support:

  • Added a max_tool_calls parameter to both sync and async chat methods in ollama/_client.py. When set, the client will automatically execute tool calls in a loop, feeding tool outputs back to the model, up to the specified number of iterations. If not set or set to None, manual tool handling remains the default. [1] [2] [3] [4] [5] [6]
  • Implemented the auto tool execution loop logic in both sync and async chat methods, including error handling for exceeding the maximum number of tool calls. [1] [2] [3] [4]

Internal utilities:

  • Added the _exec_tool helper function to execute a tool call and serialize the result, handling errors if the tool is not found.

Documentation and examples:

  • Updated docstrings and example usages in ollama/_client.py to explain the new max_tool_calls parameter and demonstrate both manual and automatic tool execution patterns. [1] [2]
  • Enhanced the examples/async-tools.py script to showcase both auto and manual tool handling, and switched the example model to qwen3.5:4b. [1] [2]
Copy link
Copy Markdown

@guicybercode guicybercode left a comment

Choose a reason for hiding this comment

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

Overview

This diff adds function calling/tool support and several new parameters (think, logprobs, top_logprobs) to the chat() method in both sync and async variants. The changes appear to align the Python client with newer Ollama API capabilities.


Strengths

1. Consistent API Design

  • Both sync (def chat) and async (async def chat) methods receive identical parameter additions and documentation updates.
  • Method overloads are properly maintained for type-checking with stream variations.

2. Tool Handling Abstraction

tools=list(_copy_tools(tools)),
  • Delegating tool normalization to _copy_tools() keeps the main chat() method clean.
  • Supports multiple input types: Mapping, Tool, or Callable → good developer ergonomics.

3. Improved Documentation

  • Added practical example showing how to define and pass a tool function.
  • Docstring references Google style guide for docstring conventions 👍.

4. Type Safety

  • Proper use of Optional[Union[...]] and JsonSchemaValue for the format parameter.
  • Return types correctly distinguish between ChatResponse and iterator variants based on stream.

Areas for Improvement

1. Missing Parameter Documentation

The new parameters think, logprobs, and top_logprobs are added to the signature and payload but not documented in the docstring:

# Add to docstring:
#   think: Enable reasoning/thinking mode (if supported by model).
#   logprobs: Include log probabilities in the response.
#   top_logprobs: Number of top log probabilities to return (requires logprobs=True).

2. Tool Error Handling

The _copy_tools() helper converts callables via convert_function_to_tool(), but there's no visible error handling for:

  • Functions with unsupported signatures
  • Missing type hints on parameters
  • Non-JSON-serializable return types

Recommendation: Add validation with clear error messages:

try:
    yield convert_function_to_tool(unprocessed_tool)
except (TypeError, ValueError) as e:
    raise RequestError(f"Failed to convert tool '{unprocessed_tool.__name__}': {e}")

3. Mutable Default Argument Risk (Minor)

While not directly visible here, ensure messages and tools aren't using mutable defaults elsewhere. The use of _copy_messages() and _copy_tools() mitigates this, but explicit copying in the public API is a good pattern to maintain.

4. Async/Sync Code Duplication

The sync and async chat() implementations are nearly identical aside from await. Consider:

  • Using a shared private helper method to reduce duplication
  • Or leveraging a decorator pattern for request handling

5. Example Code in Docstring

The example shows:

client.chat(model='llama3.2', tools=[add_two_numbers], messages=[...])

But doesn't show the full tool-calling workflow (model response → tool execution → follow-up message). Consider linking to a full tutorial or expanding the example.


Potential Bugs / Edge Cases

Issue Severity Recommendation
tools=None passed to list(_copy_tools(None)) Low Ensure _copy_tools handles None gracefully (returns empty list)
top_logprobs without logprobs=True Medium Add validation: if top_logprobs is set, logprobs must be True
Tool name collisions Low Document that tool.function.name must be unique per request

Style & Maintainability

  • ✅ Follows existing code style (type hints, docstring format)
  • ✅ Uses model_dump(exclude_none=True) to keep payloads clean
  • ⚠️ Consider adding # type: ignore comments only where strictly necessary (none visible here)

Suggestions for Future Enhancements

  1. Add a Tool TypedDict/Pydantic model in the public API for explicit tool definition (beyond auto-conversion from functions).
  2. Support tool choice parameters (e.g., tool_choice="auto" | "none" | {"type": "function", "function": {"name": "..."}}) if the backend supports it.
  3. Add integration tests for:
    • Tool calling round-trip (sync + async)
    • Error cases (invalid tool schema, model doesn't support tools)
  4. Deprecation path: If older Ollama versions don't support these params, add version checking with helpful warnings.

Final Verdict: Approve with Minor Revisions

The changes are well-structured, type-safe, and improve the library's functionality significantly. Before merging:

  1. ✅ Document the three new parameters in docstrings
  2. ✅ Add validation for logprobs/top_logprobs dependency
  3. ✅ Ensure _copy_tools(None) returns [] and test edge cases
  4. ✅ (Optional) Refactor sync/async duplication if maintainability becomes a concern
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants