Possible bugs in both tools examples (or at least something to document) #211

New Issue

2026-02-15T16:28:47-05:00

yindo commented

2026-02-15 16:28:47 -05:00

Originally created by @jdalegonzalez on GitHub (Feb 4, 2025).

Originally assigned to: @ParthSareen on GitHub.

The current code in the tools examples looks like:

if response.message.tool_calls:
  # There may be multiple tool calls in the response
  for tool in response.message.tool_calls:
    # Ensure the function is available, and then call it
    if function_to_call := available_functions.get(tool.function.name):
      print('Calling function:', tool.function.name)
      print('Arguments:', tool.function.arguments)
      output = function_to_call(**tool.function.arguments)
      print('Function output:', output)
    else:
      print('Function', tool.function.name, 'not found')

# Only needed to chat with the model using the tool call results
if response.message.tool_calls:
  # Add the function response to messages for the model to use
  messages.append(response.message)
  messages.append({'role': 'tool', 'content': str(output), 'name': tool.function.name})

  # Get final response from model with function outputs
  final_response = chat('llama3.1', messages=messages)
  print('Final response:', final_response.message.content)

else:
  print('No tool calls returned from model')

This code raises the following questions:

Why test "if response.message.tool_calls" twice? Could this be refactored to only need one test? Something like

if response.message.tool_calls:
   messages.append(response.message)
   # There may be multiple calls in the response
   for tool in ...
else
   print('No tool calls returned from model')

If that refactor isn't OK, it would be good to document why testing .message.tool_calls twice is necessary
If there are multiple tool calls, isn't the output getting thrown away for every tool call except the last one? If that's OK, why is it OK? Should the loop actually be something like:

 for tool in response.message.tool_calls:
    # Ensure the function is available, and then call it
    if function_to_call := available_functions.get(tool.function.name):
      print('Calling function:', tool.function.name)
      print('Arguments:', tool.function.arguments)
      output = function_to_call(**tool.function.arguments)
      print('Function output:', output)
      messages.append({'role': 'tool', 'content': str(output), 'name': tool.function.name})
    else:
      print('Function', tool.function.name, 'not found')

If it's OK for output to get tossed, documenting why that's OK would be helpful.

For what it's worth, I'm happy to make the refactors and submit a pull request, I just didn't know if I was missing something. (Since I'm VERY new to ollama, tools, etc...)

Originally created by @jdalegonzalez on GitHub (Feb 4, 2025). Originally assigned to: @ParthSareen on GitHub. The current code in the tools examples looks like: ``` if response.message.tool_calls: # There may be multiple tool calls in the response for tool in response.message.tool_calls: # Ensure the function is available, and then call it if function_to_call := available_functions.get(tool.function.name): print('Calling function:', tool.function.name) print('Arguments:', tool.function.arguments) output = function_to_call(**tool.function.arguments) print('Function output:', output) else: print('Function', tool.function.name, 'not found') # Only needed to chat with the model using the tool call results if response.message.tool_calls: # Add the function response to messages for the model to use messages.append(response.message) messages.append({'role': 'tool', 'content': str(output), 'name': tool.function.name}) # Get final response from model with function outputs final_response = chat('llama3.1', messages=messages) print('Final response:', final_response.message.content) else: print('No tool calls returned from model') ``` This code raises the following questions: - Why test "if response.message.tool_calls" twice? Could this be refactored to only need one test? Something like ``` if response.message.tool_calls: messages.append(response.message) # There may be multiple calls in the response for tool in ... else print('No tool calls returned from model') ``` - If that refactor isn't OK, it would be good to document why testing .message.tool_calls twice is necessary - If there are multiple tool calls, isn't the output getting thrown away for every tool call except the last one? If that's OK, why is it OK? Should the loop actually be something like: ``` for tool in response.message.tool_calls: # Ensure the function is available, and then call it if function_to_call := available_functions.get(tool.function.name): print('Calling function:', tool.function.name) print('Arguments:', tool.function.arguments) output = function_to_call(**tool.function.arguments) print('Function output:', output) messages.append({'role': 'tool', 'content': str(output), 'name': tool.function.name}) else: print('Function', tool.function.name, 'not found') ``` - If it's OK for output to get tossed, documenting why that's OK would be helpful. For what it's worth, I'm happy to make the refactors and submit a pull request, I just didn't know if I was missing something. (Since I'm VERY new to ollama, tools, etc...)

yindo commented

2026-02-15 16:28:47 -05:00

@ParthSareen commented on GitHub (Feb 10, 2025):

Yeah, on second look this reads a bit confusing. There should be a way to clean it up - the second if is to mainly show how it can be fed back into messages and the LLM. I'll document something and clean this up a bit

@ParthSareen commented on GitHub (Feb 10, 2025): Yeah, on second look this reads a bit confusing. There should be a way to clean it up - the second if is to mainly show how it can be fed back into `messages` and the LLM. I'll document something and clean this up a bit

yindo commented

2026-02-15 16:28:48 -05:00