From f5cb1eba3902aff3e734fdfc6c7a7242de4ce114 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Fri, 10 Jul 2026 01:30:53 +0000 Subject: [PATCH] harness(openai): native-tool/vision profile toggles + baked provider options Add three additive OpenAiModel builders so the OpenHuman host can cut its local-runtime providers (Ollama, LM Studio, MLX, OMLX, local-openai) over to the crate-native ChatModel without a capability regression: - with_native_tool_calling(bool): overrides profile.tool_calling (and clears parallel_tool_calls/streaming_tool_chunks when disabled). Local runtimes that reject the OpenAI `tools` parameter (HTTP 400) need this off so a harness embeds tool specs in the prompt instead of sending native tools. - with_vision(bool): overrides profile.modalities.image_in for text-only models. - with_default_provider_options(Value): bakes provider options (e.g. Ollama's {"options":{"num_ctx":8192}}) onto every request, merged under each request's own provider_options (per-call keys win). A pure merge_provider_options helper implements the policy and deliberately passes a non-object override through untouched so provider_extra_options still rejects it. Both invoke and stream go through translate_request, so the baked options and profile toggles apply to unary and streaming calls alike. Unit-tested; fmt + clippy -D warnings clean. --- src/harness/providers/openai/mod.rs | 3 +- src/harness/providers/openai/test.rs | 93 +++++++++++++++++++++++ src/harness/providers/openai/transport.rs | 84 +++++++++++++++++++- 3 files changed, 177 insertions(+), 3 deletions(-) diff --git a/src/harness/providers/openai/mod.rs b/src/harness/providers/openai/mod.rs index c84f0db..787eddf 100644 --- a/src/harness/providers/openai/mod.rs +++ b/src/harness/providers/openai/mod.rs @@ -75,7 +75,8 @@ use convert::*; use sse::*; #[cfg(test)] use transport::{ - auth_headers, effective_temperature, glob_match, merge_system_into_user, request_timeout, + auth_headers, effective_temperature, glob_match, merge_provider_options, + merge_system_into_user, request_timeout, }; #[cfg(test)] diff --git a/src/harness/providers/openai/test.rs b/src/harness/providers/openai/test.rs index 64e9d70..5ae6268 100644 --- a/src/harness/providers/openai/test.rs +++ b/src/harness/providers/openai/test.rs @@ -1382,3 +1382,96 @@ async fn sse_stream_recovers_tool_args_with_leaked_template_marker() { assert_eq!(calls[0].name, "composio_execute"); assert_eq!(calls[0].arguments, json!({ "q": 1 })); } +// `ChatModel::profile` is generic over `State`; pin `State = ()` so the concrete +// `OpenAiModel` handle disambiguates without a turbofish at every call site. +fn profile_of(model: &OpenAiModel) -> &crate::harness::model::ModelProfile { + ChatModel::<()>::profile(model).expect("openai models expose a profile") +} + +#[test] +fn with_native_tool_calling_false_clears_tool_profile_flags() { + let model = OpenAiModel::new("k") + .with_model("qwen2.5") + .with_native_tool_calling(false); + let profile = profile_of(&model); + assert!(!profile.tool_calling); + assert!(!profile.parallel_tool_calls); + assert!(!profile.streaming_tool_chunks); + + // Re-enabling restores native tool calling on the profile. + let re = OpenAiModel::new("k") + .with_model("qwen2.5") + .with_native_tool_calling(false) + .with_native_tool_calling(true); + assert!(profile_of(&re).tool_calling); +} + +#[test] +fn with_vision_toggles_image_in_modality() { + let off = OpenAiModel::new("k") + .with_model("qwen2.5") + .with_vision(false); + assert!(!profile_of(&off).modalities.image_in); + + let on = OpenAiModel::new("k") + .with_model("gpt-4.1-mini") + .with_vision(true); + assert!(profile_of(&on).modalities.image_in); +} + +#[test] +fn default_provider_options_are_baked_onto_every_request() { + let model = OpenAiModel::ollama() + .with_model("qwen2.5") + .with_default_provider_options(json!({ "options": { "num_ctx": 8192 } })); + + // A request with no provider_options still carries the baked options. + let request = ModelRequest::new(vec![Message::user("hi")]); + let value = serde_json::to_value(model.translate_request(&request).unwrap()).unwrap(); + assert_eq!(value["options"]["num_ctx"], json!(8192)); +} + +#[test] +fn request_provider_options_win_over_baked_defaults() { + let model = OpenAiModel::ollama() + .with_model("qwen2.5") + .with_default_provider_options( + json!({ "options": { "num_ctx": 8192 }, "keep_alive": "5m" }), + ); + + // The per-call `options` key overrides the baked one; unrelated baked keys + // (`keep_alive`) still flow through. + let request = ModelRequest::new(vec![Message::user("hi")]) + .with_provider_options(json!({ "options": { "num_ctx": 2048 } })); + let value = serde_json::to_value(model.translate_request(&request).unwrap()).unwrap(); + assert_eq!(value["options"]["num_ctx"], json!(2048)); + assert_eq!(value["keep_alive"], json!("5m")); +} + +#[test] +fn merge_provider_options_prefers_overrides_and_handles_nulls() { + let defaults = json!({ "a": 1, "b": 2 }); + let overrides = json!({ "b": 20, "c": 30 }); + assert_eq!( + merge_provider_options(&defaults, &overrides), + json!({ "a": 1, "b": 20, "c": 30 }) + ); + assert_eq!( + merge_provider_options(&serde_json::Value::Null, &overrides), + overrides + ); + assert_eq!( + merge_provider_options(&defaults, &serde_json::Value::Null), + defaults + ); + assert_eq!( + merge_provider_options(&serde_json::Value::Null, &serde_json::Value::Null), + serde_json::Value::Null + ); + // A non-null, non-object override is passed through untouched (not merged), so + // downstream validation still rejects it rather than the merge hiding it. + assert_eq!( + merge_provider_options(&defaults, &json!(["top_k", 40])), + json!(["top_k", 40]) + ); +} diff --git a/src/harness/providers/openai/transport.rs b/src/harness/providers/openai/transport.rs index dd3e2bb..10cdb90 100644 --- a/src/harness/providers/openai/transport.rs +++ b/src/harness/providers/openai/transport.rs @@ -59,8 +59,14 @@ pub struct OpenAiModel { provider: String, /// API base URL (no trailing slash); `/chat/completions` is appended. base_url: String, - /// Capability profile derived from the default model id. + /// Capability profile derived from the default model id, optionally adjusted + /// by [`Self::with_native_tool_calling`] / [`Self::with_vision`]. profile: ModelProfile, + /// Provider-specific options baked onto every request (e.g. a local model's + /// `{"options": {"num_ctx": 8192}}`). Merged under each request's own + /// `provider_options`, which win on key conflicts. `Value::Null` by default + /// (no baked options). See [`Self::with_default_provider_options`]. + default_provider_options: Value, } /// The auth headers `(name, value)` for a given [`AuthStyle`] + credential. @@ -272,6 +278,7 @@ impl OpenAiModel { provider: "openai".to_string(), base_url: DEFAULT_BASE_URL.to_string(), profile: derive_profile("openai", DEFAULT_MODEL), + default_provider_options: Value::Null, } } @@ -319,6 +326,48 @@ impl OpenAiModel { self } + /// Overrides whether this model advertises **native** tool calling on its + /// [`profile`](ChatModel::profile). Many self-hosted / local OpenAI-compatible + /// runtimes (Ollama and others) reject the OpenAI `tools` parameter with an + /// HTTP 400; passing `false` lets a harness detect that and embed tool specs + /// in the prompt instead of sending native `tools`. Disabling native tools + /// also clears `parallel_tool_calls` and `streaming_tool_chunks`, which are + /// meaningless without them. + /// + /// This mutates the derived profile, so apply it **after** + /// [`with_model`](Self::with_model) / [`with_provider`](Self::with_provider) + /// (which re-derive the profile). + pub fn with_native_tool_calling(mut self, enabled: bool) -> Self { + self.profile.tool_calling = enabled; + if !enabled { + self.profile.parallel_tool_calls = false; + self.profile.streaming_tool_chunks = false; + } + self + } + + /// Overrides whether this model advertises image input (vision) on its + /// [`profile`](ChatModel::profile). The OpenAI wire preset defaults to `true`; + /// pass `false` for text-only local/self-hosted models so a harness does not + /// route image content to an endpoint that cannot accept it. + /// + /// This mutates the derived profile, so apply it **after** + /// [`with_model`](Self::with_model) / [`with_provider`](Self::with_provider). + pub fn with_vision(mut self, enabled: bool) -> Self { + self.profile.modalities.image_in = enabled; + self + } + + /// Bakes provider-specific options onto every request (e.g. a local model's + /// `{"options": {"num_ctx": 8192}}`). These are merged **under** each + /// request's own [`ModelRequest::provider_options`], so a per-call option of + /// the same key wins. Reserved OpenAI fields are still stripped downstream by + /// [`provider_extra_options`]. Passing `Value::Null` clears the baked options. + pub fn with_default_provider_options(mut self, options: Value) -> Self { + self.default_provider_options = options; + self + } + /// Overrides the default model id. pub fn with_model(mut self, model: impl Into) -> Self { self.model = model.into(); @@ -655,7 +704,10 @@ impl OpenAiModel { seed: request.seed, stream: false, stream_options: None, - extra: provider_extra_options(&request.provider_options)?, + extra: provider_extra_options(&merge_provider_options( + &self.default_provider_options, + &request.provider_options, + ))?, }) } @@ -801,6 +853,34 @@ pub(super) fn request_timeout(timeout_ms: Option, streaming: bool) -> Optio } } +/// Merges baked `defaults` under a request's own `overrides` provider options. +/// +/// Keys present in `overrides` win over `defaults`. A `Null` on either side +/// contributes nothing; when neither side is an object the result is +/// `Value::Null`, so [`provider_extra_options`] short-circuits to an empty map. +/// +/// A **non-null, non-object** `overrides` is invalid caller input and is returned +/// untouched (never merged) so [`provider_extra_options`] still rejects it with +/// its clear validation error instead of the merge silently dropping it. Pure, so +/// the merge policy is unit-testable without a request. +pub(super) fn merge_provider_options(defaults: &Value, overrides: &Value) -> Value { + if !overrides.is_null() && !overrides.is_object() { + return overrides.clone(); + } + match (defaults.as_object(), overrides.as_object()) { + (None, None) => Value::Null, + (Some(base), None) => Value::Object(base.clone()), + (None, Some(over)) => Value::Object(over.clone()), + (Some(base), Some(over)) => { + let mut merged = base.clone(); + for (key, value) in over { + merged.insert(key.clone(), value.clone()); + } + Value::Object(merged) + } + } +} + /// Returns provider-specific top-level fields to flatten into the request body. /// /// Core OpenAI-compatible fields are intentionally reserved so normalized