mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(inference/ollama): use prompt-guided tool calling so skills work on Ollama (#3098 sub-issue 3) (#3229)
This commit is contained in:
@@ -80,6 +80,14 @@ pub struct OpenAiCompatibleProvider {
|
||||
/// carries an `@<temp>` suffix (e.g. `"openai:gpt-4o@0.2"`). The
|
||||
/// `temperature_unsupported_models` glob filter still applies after.
|
||||
pub(crate) temperature_override: Option<f64>,
|
||||
/// Value reported by `capabilities().native_tool_calling`. Defaults to
|
||||
/// `true` because most OpenAI-compatible providers (OpenAI, Anthropic
|
||||
/// adapters, GLM, Groq, Mistral, OpenHuman backend, …) implement the
|
||||
/// `tools` parameter correctly. The factory flips this to `false` for
|
||||
/// Ollama (sub-issue 3 of #3098), whose OpenAI-compat endpoint returns
|
||||
/// HTTP 400 on `tools` for many models — making prompt-guided text
|
||||
/// tool specs the only path that works across the Ollama model zoo.
|
||||
native_tool_calling: bool,
|
||||
}
|
||||
|
||||
/// How the provider expects the API key to be sent.
|
||||
@@ -195,9 +203,20 @@ impl OpenAiCompatibleProvider {
|
||||
emit_openhuman_thread_id: false,
|
||||
temperature_unsupported_models: Vec::new(),
|
||||
temperature_override: None,
|
||||
native_tool_calling: true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Toggle whether this provider advertises native (OpenAI-style) tool
|
||||
/// calling to the agent harness. The default is `true`; set to `false`
|
||||
/// for providers whose `/v1/chat/completions` endpoint rejects the
|
||||
/// `tools` parameter — the harness will then embed tool specs in the
|
||||
/// system prompt and parse calls out of the response text instead.
|
||||
pub fn with_native_tool_calling(mut self, enabled: bool) -> Self {
|
||||
self.native_tool_calling = enabled;
|
||||
self
|
||||
}
|
||||
|
||||
/// Set the list of model glob patterns for which temperature must be
|
||||
/// omitted from request bodies. Called by the provider factory to
|
||||
/// propagate `config.temperature_unsupported_models`.
|
||||
@@ -1248,7 +1267,7 @@ impl OpenAiCompatibleProvider {
|
||||
impl Provider for OpenAiCompatibleProvider {
|
||||
fn capabilities(&self) -> crate::openhuman::inference::provider::traits::ProviderCapabilities {
|
||||
crate::openhuman::inference::provider::traits::ProviderCapabilities {
|
||||
native_tool_calling: true,
|
||||
native_tool_calling: self.native_tool_calling,
|
||||
vision: false,
|
||||
}
|
||||
}
|
||||
@@ -1940,7 +1959,12 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
}
|
||||
|
||||
fn supports_native_tools(&self) -> bool {
|
||||
true
|
||||
// Must mirror `capabilities().native_tool_calling`. Both signals are
|
||||
// read by the agent harness (`traits.rs:415`) to decide between an
|
||||
// OpenAI-style `tools` array and the prompt-guided text fallback;
|
||||
// letting them disagree would defeat `with_native_tool_calling(false)`
|
||||
// for the Ollama branch of sub-issue 3 of #3098.
|
||||
self.native_tool_calling
|
||||
}
|
||||
|
||||
fn supports_streaming(&self) -> bool {
|
||||
|
||||
@@ -1303,6 +1303,69 @@ fn capabilities_reports_native_tool_calling() {
|
||||
assert!(caps.native_tool_calling);
|
||||
}
|
||||
|
||||
// Sub-issue 3 of #3098: Ollama's OpenAI-compat endpoint silently rejects the
|
||||
// `tools` parameter for many models, so we must let the factory opt the
|
||||
// Ollama provider out of native tool calling. The agent harness then falls
|
||||
// back to prompt-guided tool specs (embedded in the system prompt) which
|
||||
// any chat model can follow. The builder defaults to enabled so cloud
|
||||
// providers (OpenAI, BYOK slugs, OpenHuman backend) are unaffected.
|
||||
|
||||
#[test]
|
||||
fn with_native_tool_calling_false_disables_capability() {
|
||||
let p = make_provider("test", "https://example.com", None).with_native_tool_calling(false);
|
||||
let caps = <OpenAiCompatibleProvider as Provider>::capabilities(&p);
|
||||
assert!(
|
||||
!caps.native_tool_calling,
|
||||
"capabilities() must mirror the builder override; this is the gate the agent harness uses to decide between native vs prompt-guided tool specs"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn with_native_tool_calling_true_preserves_default() {
|
||||
let p = make_provider("test", "https://example.com", None).with_native_tool_calling(true);
|
||||
let caps = <OpenAiCompatibleProvider as Provider>::capabilities(&p);
|
||||
assert!(caps.native_tool_calling);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn with_native_tool_calling_is_idempotent() {
|
||||
let p = make_provider("test", "https://example.com", None)
|
||||
.with_native_tool_calling(false)
|
||||
.with_native_tool_calling(false);
|
||||
let caps = <OpenAiCompatibleProvider as Provider>::capabilities(&p);
|
||||
assert!(!caps.native_tool_calling);
|
||||
}
|
||||
|
||||
/// `supports_native_tools()` is the gate the agent harness reads
|
||||
/// (`traits.rs:415`) when deciding whether to send tools natively or
|
||||
/// inject them into the prompt. It MUST agree with
|
||||
/// `capabilities().native_tool_calling`; otherwise
|
||||
/// `with_native_tool_calling(false)` silently fails to switch to
|
||||
/// prompt-guided and Ollama still receives a `tools` array (the exact
|
||||
/// regression sub-issue 3 of #3098 was meant to fix).
|
||||
#[test]
|
||||
fn supports_native_tools_mirrors_capabilities_flag() {
|
||||
let default = make_provider("test", "https://example.com", None);
|
||||
assert_eq!(
|
||||
default.supports_native_tools(),
|
||||
<OpenAiCompatibleProvider as Provider>::capabilities(&default).native_tool_calling,
|
||||
"default provider: the two capability signals must match"
|
||||
);
|
||||
assert!(default.supports_native_tools(), "default must remain true");
|
||||
|
||||
let opted_out =
|
||||
make_provider("test", "https://example.com", None).with_native_tool_calling(false);
|
||||
assert_eq!(
|
||||
opted_out.supports_native_tools(),
|
||||
<OpenAiCompatibleProvider as Provider>::capabilities(&opted_out).native_tool_calling,
|
||||
"after with_native_tool_calling(false): the two capability signals must match"
|
||||
);
|
||||
assert!(
|
||||
!opted_out.supports_native_tools(),
|
||||
"after with_native_tool_calling(false), supports_native_tools must report false so the harness picks the prompt-guided fallback"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_specs_convert_to_openai_format() {
|
||||
let specs = vec![crate::openhuman::tools::ToolSpec {
|
||||
|
||||
@@ -731,16 +731,24 @@ fn make_ollama_provider(
|
||||
// Ollama does not expose the Responses API (/v1/responses) — passing
|
||||
// `false` prevents a guaranteed-404 fallback attempt and the Sentry
|
||||
// noise it would generate (TAURI-RUST-59Y).
|
||||
let p = make_openai_compatible_provider_with_config(
|
||||
//
|
||||
// Ollama also rejects the OpenAI-style `tools` parameter for many models
|
||||
// (HTTP 400 "unsupported parameter: tools"), so we disable
|
||||
// `native_tool_calling` on the provider directly. The agent harness
|
||||
// then embeds tool specs in the system prompt and parses tool calls
|
||||
// out of the response text — a format any chat model can follow.
|
||||
// Skills that depend on tool invocations now work over Ollama
|
||||
// (sub-issue 3 of #3098).
|
||||
let provider = OpenAiCompatibleProvider::new_no_responses_fallback(
|
||||
"ollama",
|
||||
&endpoint,
|
||||
"",
|
||||
None,
|
||||
CompatAuthStyle::None,
|
||||
&config.temperature_unsupported_models,
|
||||
temperature_override,
|
||||
false,
|
||||
)?;
|
||||
Ok((p, model.to_string()))
|
||||
)
|
||||
.with_temperature_unsupported_models(config.temperature_unsupported_models.clone())
|
||||
.with_temperature_override(temperature_override)
|
||||
.with_native_tool_calling(false);
|
||||
Ok((Box::new(provider), model.to_string()))
|
||||
}
|
||||
|
||||
/// Build an LM Studio local provider.
|
||||
|
||||
@@ -210,6 +210,48 @@ fn ollama_prefix() {
|
||||
assert_eq!(model, "llama3.1:8b");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ollama_provider_opts_out_of_native_tool_calling() {
|
||||
// Sub-issue 3 of #3098: Ollama's OpenAI-compat endpoint returns HTTP 400
|
||||
// for many models when a `tools` array is sent (the existing detection
|
||||
// path matches "unsupported parameter: tools"). The retry logic strips
|
||||
// tools entirely, which silently breaks any skill or workflow that
|
||||
// depends on tool calls. The factory must build the Ollama provider
|
||||
// with native tool calling disabled so the agent harness uses the
|
||||
// prompt-guided text format from the first request.
|
||||
let config = Config::default();
|
||||
let (provider, _model) = create_chat_provider_from_string("chat", "ollama:llama3.2", &config)
|
||||
.expect("ollama:<model> must build");
|
||||
let caps = provider.capabilities();
|
||||
assert!(
|
||||
!caps.native_tool_calling,
|
||||
"ollama provider must report native_tool_calling=false so the agent harness emits prompt-guided tool specs instead of an OpenAI-style `tools` array"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lmstudio_provider_keeps_native_tool_calling_enabled() {
|
||||
// LM Studio's OpenAI-compat endpoint supports the `tools` parameter for
|
||||
// models that expose function calling. Only Ollama gets opted out by
|
||||
// default — the LM Studio path stays on the native schema.
|
||||
let mut config = Config::default();
|
||||
config.local_ai.base_url = Some("http://127.0.0.1:1234".to_string());
|
||||
let (provider, _model) =
|
||||
create_chat_provider_from_string("chat", "lmstudio:google/gemma-4-e4b", &config)
|
||||
.expect("lmstudio:<model> must build");
|
||||
assert!(
|
||||
provider.capabilities().native_tool_calling,
|
||||
"lmstudio provider must keep native_tool_calling=true; only the ollama branch opts out"
|
||||
);
|
||||
}
|
||||
|
||||
// Note: a BYOK-cloud regression test (e.g. `openai:gpt-4o` keeps
|
||||
// native_tool_calling=true) would need an `AuthService` with the slug's API
|
||||
// key seeded. The unit test
|
||||
// `with_native_tool_calling_true_preserves_default` in compatible_tests.rs
|
||||
// already pins that the builder leaves the default in place when not
|
||||
// called, which is what every non-Ollama factory path relies on.
|
||||
|
||||
#[test]
|
||||
fn lmstudio_prefix() {
|
||||
let mut config = Config::default();
|
||||
|
||||
Reference in New Issue
Block a user