fix: Ollama tool property types must be scalar

[gadenbuie]

Problem

Ollama currently doesn't support arrays in the tools.function.parameters.properties.type field, instead it expects a scalar. ollama/ollama#5990

In ellmer, this means that using type_*(required = FALSE) results in the following warning:

pkgload::load_all()
#> ℹ Loading ellmer

tool_current_time <- tool(
  \(tz = "UTC") strftime(Sys.time(), "%F %T", tz = tz),
  .name = "get_current_time",
  .description = "Get the current date and time.",
  tz = type_string(
    "The IANA-recognized timezone, defaults to 'UTC'",
    required = FALSE
  ),
)

chat <- chat_ollama(model="qwen2.5:7b", echo="all")
chat$register_tool(tool_current_time)

chat$chat("Where was R invented?")
#> > Where was R invented?
#> Error in `req_perform_connection()` at ellmer/R/httr2.R:34:3:
#> ! HTTP 400 Bad Request.
#> • json: cannot unmarshal array into Go struct field .tools.function.parameters.properties.type of type string

Edit: This behavior happens in the OpenAI-compatible API, Ollama's API supports required as a separate field.

After

This PR updates the as_json method for ProviderOpenAI, TypeObject to avoid add "null" to the property.type when the provider is an Ollama model.

pkgload::load_all()
#> ℹ Loading ellmer

tool_current_time <- tool(
  \(tz = "UTC") strftime(Sys.time(), "%F %T", tz = tz),
  .name = "get_current_time",
  .description = "Get the current date and time.",
  tz = type_string(
    "The IANA-recognized timezone, defaults to 'UTC'",
    required = FALSE
  ),
)

chat <- chat_ollama(model="qwen2.5:7b", echo="all")
chat$register_tool(tool_current_time)

chat$chat("Where was R invented?")
#> > Where was R invented?
#> < R was primarily developed at the University of Auckland in New Zealand by 
#> < Ross Ihaka and Robert Gentleman in the late 1990s. However, it has since 
#> < grown to be a widely used language and environment for statistical computing 
#> < and graphics, with contributions from around the world.
#> <
chat$chat("What's the current time in the place where the R language was invented?")
#> > What's the current time in the place where the R language was invented?
#> < 
#> < [tool request (call_ynzxn614)]: get_current_time(tz = "Pacific/Auckland")
#> > [tool result  (call_ynzxn614)]: 2025-02-28 02:22:02
#> < The current time in Auckland, New Zealand (which we're approximating as the 
#> < place where R was invented) is February 28, 2025, at 02:22:02 UTC. If you 
#> < need the time in a different timezone or specific date format, let me know!
#> <

[gadenbuie]

This feels safe but a little hacky. chat_ollama() calls chat_openai() and sets api_key = "ollama". I briefly looked into adding a new ProviderOllama that inherits from ProviderOpenAI until I realized I could simply check here if the provider is Ollama. If you'd rather the ProviderOllama approach, I could go that way.

[hadley]

Yeah, I'd rather make a single method(as_json, list(ProviderOllama, TypeObject)).

And should this warn to let the user know it isn't supported?

[gadenbuie]

Just a little more context: the reason I backed out of the ProviderOllama approach is because it involves untangling Ollama and OpenAI all the way up to chat_ollama(). I'm up for it, but just wanted to call out that it's a bigger change than adding a method.

I agree, a warning would make sense but should probably come from the $register_tool() call.

[gadenbuie]

I implemented ProviderOllama (fb07118) but while reading Ollama docs, I realized that Ollama is expecting the required properties in a separate array adjacent to properties (8ee8a59).

OpenAI, on the other hand, requires all properties to appear in required, so we still need a separate provider for Ollama.

[hadley]

Thanks, that looks exactly like what I was expecting 😄

[hadley]

You might take a quick look at some of the other provider methods for as_json(, TypeObject) just in case someone else uses the same pattern (which looks vaguely familiar to me)

[gadenbuie]

You're right, it's basically the same as for ProviderGroq. I refactored to match the code style. b094638

[hadley]

Would you mind adding a test, even if it's not going to run on CI?

[gadenbuie]

Tests added in d3b42a1. I added one test around the shape of the tool def and one smoke test that chat_ollama() still works when a tool with optional arguments is registered.

[hadley]

Thanks! Just needs a news bullet and then I'll merge.

[gadenbuie]

I added one already 😊 (but I may have put it in the wrong place)

[hadley]

FWIW our convention is to put new bullets at the top, but it doesn't really matter (and will get tweaked prior to releae)