Venice: add auto-recover for optional params and surface actual error message on 400s

CHANGELOG.md

@@ -1,3 +1,10 @@
+# (2026-06-23) Version 1.23.1
+
+- (**Bugfix**) The [Venice](https://venice.ai) provider now auto-recovers when a model rejects an optional knob it does not support. Venice's request body is strict (`additionalProperties: false`), so a model that lacks `prompt_cache_retention`, `reasoning_effort`, or `prompt_cache_key` rejected the whole request with a `400 Bad Request` — breaking agent creation and every reply. baibot now drops the unsupported field and retries, remembering the rejection per model so later requests skip it without a wasted round-trip. Only these meaning-preserving fields are dropped; sampling knobs that change the output (`temperature`, `top_p`, the penalties) are never silently removed and still surface as an error.
+
+- (**Improvement**) When Venice rejects a request with a `400 Bad Request`, baibot now surfaces Venice's actual error message (e.g. `Extra inputs are not permitted, field: 'prompt_cache_retention'`) instead of a generic "configuration does not result in a working agent". This makes agent-creation failures self-explanatory. Other error statuses keep their bodies redacted, since those can carry account or rate-limit details.
+
+
 # (2026-06-23) Version 1.23.0
 
 - (**Feature**) The [Venice](https://venice.ai) provider now accepts file inputs (PDF, DOCX, and other documents, up to 25MB), the same way it already handled images. This makes Venice the second provider after OpenAI to accept files; the others (Anthropic and the OpenAI-compatible providers) skip them. See the [text-generation feature docs](./docs/features.md#-text-generation).

Cargo.lock

@@ -315,7 +315,7 @@ dependencies = [
 
 [[package]]
 name = "baibot"
-version = "1.23.0"
+version = "1.23.1"
 dependencies = [
  "anthropic",
  "anyhow",

Cargo.toml

@@ -7,7 +7,7 @@ license = "AGPL-3.0-or-later"
 readme = "README.md"
 keywords = ["matrix", "chat", "bot", "AI", "LLM"]
 include = ["/etc/assets/baibot-torso-768.png", "/src", "/README.md", "/CHANGELOG.md", "/LICENSE"]
-version = "1.23.0"
+version = "1.23.1"
 edition = "2024"
 
 [lib]

src/agent/provider/venice/chat.rs

@@ -19,6 +19,7 @@ use super::wire::{ChatCompletionRequest, ChatCompletionResponse, WebSearchCitati
 pub async fn generate_text(
     config: &Config,
     http: &reqwest::Client,
+    unsupported: &super::recovery::UnsupportedFieldsCache,
     conversation: LLMConversation,
     params: TextGenerationParams,
 ) -> anyhow::Result<TextGenerationResult> {
@@ -98,7 +99,7 @@ pub async fn generate_text(
             vp
         });
 
-    let request = ChatCompletionRequest {
+    let mut request = ChatCompletionRequest {
         model: text_generation_config.model_id.clone(),
         messages,
         temperature: Some(temperature),
@@ -117,40 +118,82 @@ pub async fn generate_text(
 
     let url = format!("{}/chat/completions", config.base_url.trim_end_matches('/'));
 
-    tracing::trace!(
-        model = text_generation_config.model_id,
-        messages_count = request.messages.len(),
-        "Sending Venice chat completion API request"
-    );
+    let model_id = text_generation_config.model_id.clone();
 
-    let response = http
-        .post(&url)
-        .bearer_auth(&config.api_key)
-        .json(&request)
-        .send()
-        .await?;
+    // Proactively drop fields this model has already rejected earlier in this process, so a known
+    // mismatch costs zero wasted round-trips after the first discovery. Venice's body is
+    // `additionalProperties: false`, so sending a known-unsupported field would 400 again.
+    for field in unsupported.known_for(&model_id) {
+        super::recovery::strip_droppable_field(&mut request, &field);
+    }
 
-    let status = response.status();
-    if !status.is_success() {
-        // Log the body server-side for debugging (Venice explains a rejected strict body there),
-        // but keep it OUT of the returned error: that error surfaces in the Matrix room, and the
-        // body can carry account / rate-limit details that shouldn't reach room members.
+    // Send with bounded auto-recovery. When Venice 400s because a model does not support an optional
+    // knob, it names the field (`field: '...'`); if that field is one we may safely drop, we strip
+    // it, remember the rejection for this model, and retry. The loop is bounded: each retry clears a
+    // distinct droppable field (strip returns false once it is gone), so after at most
+    // `DROPPABLE_FIELDS.len()` retries the request either succeeds or surfaces the error.
+    let response = loop {
+        tracing::trace!(
+            model = model_id,
+            messages_count = request.messages.len(),
+            "Sending Venice chat completion API request"
+        );
+
+        let response = http
+            .post(&url)
+            .bearer_auth(&config.api_key)
+            .json(&request)
+            .send()
+            .await?;
+
+        let status = response.status();
+        if status.is_success() {
+            break response;
+        }
+
+        // Always log the body server-side: Venice explains a rejected strict body there.
         let body = response.text().await.unwrap_or_default();
         tracing::warn!(%status, body, "Venice chat completion request failed");
 
+        // Recover from a strict-body 400 over an unsupported optional knob: strip the named field
+        // and retry. Only fields in `DROPPABLE_FIELDS` are eligible, so a meaning-bearing knob (a
+        // sampling parameter) is never silently dropped; that case falls through to surface below.
+        if status == reqwest::StatusCode::BAD_REQUEST
+            && let Some(field) = super::recovery::parse_rejected_field(&body)
+            && super::recovery::strip_droppable_field(&mut request, &field)
+        {
+            unsupported.record(&model_id, &field);
+            tracing::info!(
+                model = model_id,
+                field,
+                "Venice rejected an unsupported field; dropping it and retrying"
+            );
+            continue;
+        }
+
         // A 413 almost always means an attached file pushed the request past Venice's size limit.
-        // Surface a clear, actionable message rather than the opaque status; the raw body still
-        // stays out of the room for the reason above.
         if status == reqwest::StatusCode::PAYLOAD_TOO_LARGE {
             return Err(anyhow::anyhow!(
                 "The request was too large for Venice, most likely an attached file over the 25MB limit."
             ));
         }
 
+        // A 400 is a complaint about the request baibot built, so the body is safe and useful to
+        // surface: it tells the operator (e.g. at agent-create time) exactly which field or value
+        // Venice rejected, instead of an opaque status. Other statuses keep the body OUT of the
+        // returned error, since it can carry account / rate-limit details that shouldn't reach the
+        // room.
+        if status == reqwest::StatusCode::BAD_REQUEST {
+            return Err(anyhow::anyhow!(
+                "Venice rejected the request (400 Bad Request): {}",
+                super::recovery::extract_error_message(&body)
+            ));
+        }
+
         return Err(anyhow::anyhow!(
             "Venice chat completion request failed with status {status}"
         ));
-    }
+    };
 
     let response: ChatCompletionResponse = response.json().await?;
 

src/agent/provider/venice/controller.rs

@@ -13,11 +13,16 @@ use crate::conversation::llm::{
 
 use super::super::ControllerTrait;
 use super::config::Config;
+use super::recovery::UnsupportedFieldsCache;
 
 #[derive(Debug, Clone)]
 pub struct Controller {
     config: Config,
     http: reqwest::Client,
+    // Per-model record of chat fields this Venice deployment has rejected as unsupported, learned at
+    // runtime. `Arc`-backed inside, so the `Clone` derive shares one cache across all clones of an
+    // agent's controller.
+    unsupported_fields: UnsupportedFieldsCache,
 }
 
 impl Controller {
@@ -30,7 +35,11 @@ impl Controller {
             .build()
             .unwrap_or_else(|_| reqwest::Client::new());
 
-        Self { config, http }
+        Self {
+            config,
+            http,
+            unsupported_fields: UnsupportedFieldsCache::default(),
+        }
     }
 }
 
@@ -62,7 +71,14 @@ impl ControllerTrait for Controller {
         conversation: LLMConversation,
         params: TextGenerationParams,
     ) -> anyhow::Result<TextGenerationResult> {
-        super::chat::generate_text(&self.config, &self.http, conversation, params).await
+        super::chat::generate_text(
+            &self.config,
+            &self.http,
+            &self.unsupported_fields,
+            conversation,
+            params,
+        )
+        .await
     }
 
     async fn speech_to_text(

src/agent/provider/venice/mod.rs

@@ -3,6 +3,7 @@ mod chat;
 mod config;
 mod controller;
 mod images;
+mod recovery;
 mod utils;
 mod wire;
 

src/agent/provider/venice/recovery.rs

@@ -0,0 +1,254 @@
+//! Auto-recovery for Venice's strict request bodies.
+//!
+//! Venice's `/chat/completions` body is `additionalProperties: false`, so a model that does not
+//! support an optional knob rejects the whole request with a 400 instead of ignoring the field.
+//! Some knobs are documented as model-specific ("for supported models") and are pure
+//! optimization/tuning hints: dropping them changes nothing about the answer, only loses the
+//! optimization. When such a field is the reason for a 400, we strip it and retry, then remember
+//! the rejection per model so later requests skip the field (and the wasted round-trip) entirely.
+//!
+//! Universal sampling knobs (`temperature`, `top_p`, the penalties, `max_completion_tokens`) are
+//! deliberately NOT recoverable here: dropping one silently changes the model's output, so a model
+//! that rejects one is a real configuration problem the operator must see, not paper over.
+
+use std::collections::{HashMap, HashSet};
+use std::sync::{Arc, OnceLock, RwLock};
+
+use regex::Regex;
+
+use super::wire::ChatCompletionRequest;
+
+/// Top-level chat-completion fields baibot may drop to recover from a 400. Each is documented by
+/// Venice as model-specific or as a routing hint, and is meaning-preserving to omit (Venice falls
+/// back to its server-side default):
+/// - `prompt_cache_retention` — "extends retention ... for supported models" (cache TTL only)
+/// - `reasoning_effort` — "control reasoning effort level for supported models"
+/// - `prompt_cache_key` — cache-routing hint; dropping it only forfeits a cache-hit optimization
+///
+/// Recovery operates on TOP-LEVEL request fields only. Sub-fields inside the `venice_parameters`
+/// bag (`disable_thinking`, `enable_e2ee`, `character_slug`, …) are intentionally absent: a model
+/// that rejects one surfaces a clear 400 to the operator rather than being auto-stripped. Adding a
+/// new bag field does not extend recovery to it; only a name listed here is droppable.
+pub(super) const DROPPABLE_FIELDS: &[&str] = &[
+    "prompt_cache_retention",
+    "prompt_cache_key",
+    "reasoning_effort",
+];
+
+/// Per-model record of fields a Venice model has rejected as unsupported, learned at runtime from
+/// 400 responses. The `Arc` is shared across `Controller` clones, so a rejection learned once is
+/// seen by every clone of the same agent. The cache is process-lived only: a restart re-learns on
+/// the first request to each model, which costs one extra round-trip and nothing else, so there is
+/// no persistence to keep in sync with config changes.
+#[derive(Debug, Clone, Default)]
+pub(super) struct UnsupportedFieldsCache {
+    inner: Arc<RwLock<HashMap<String, HashSet<String>>>>,
+}
+
+impl UnsupportedFieldsCache {
+    /// Fields already known unsupported for `model_id`. Returns an empty set on an unknown model or
+    /// a poisoned lock, so a cache failure degrades to "strip nothing proactively" rather than
+    /// breaking the request path.
+    pub(super) fn known_for(&self, model_id: &str) -> HashSet<String> {
+        self.inner
+            .read()
+            .ok()
+            .and_then(|map| map.get(model_id).cloned())
+            .unwrap_or_default()
+    }
+
+    /// Records that `model_id` rejected `field`. A poisoned lock is ignored: failing to memoize only
+    /// means the next request re-discovers the rejection, never a wrong result.
+    pub(super) fn record(&self, model_id: &str, field: &str) {
+        if let Ok(mut map) = self.inner.write() {
+            map.entry(model_id.to_owned())
+                .or_default()
+                .insert(field.to_owned());
+        }
+    }
+}
+
+/// Parses the offending field name out of a Venice 400 body. A field the schema does not allow is
+/// reported as `... field: 'prompt_cache_retention', value: '...'`, so this matches the `field: '..'`
+/// marker wherever it sits in the message. Returns `None` when the body carries no such marker (a
+/// different 400 class, e.g. a missing required field), so the caller surfaces that error instead.
+///
+/// Returns only the FIRST `field: '..'` match by design. If Venice ever names several rejected
+/// fields in one body, the retry loop strips this one, retries, and rediscovers the next on the
+/// following 400 — bounded and correct. Do not switch to `captures_iter` to "batch" them without
+/// re-checking the loop's per-field termination bound in `chat.rs`.
+pub(super) fn parse_rejected_field(body: &str) -> Option<String> {
+    static RE: OnceLock<Regex> = OnceLock::new();
+    let re =
+        RE.get_or_init(|| Regex::new(r"field: '([^']+)'").expect("rejected-field regex is valid"));
+    re.captures(body)
+        .map(|caps| caps[1].to_owned())
+        .filter(|field| !field.is_empty())
+}
+
+/// Clears `field` from the request when it is one baibot may safely drop and it is currently set.
+/// Returns `true` only when a value was actually removed, which is what bounds the retry loop: once
+/// a field is `None`, a repeat rejection for the same name returns `false` and the caller stops
+/// instead of retrying forever. A field outside [`DROPPABLE_FIELDS`] always returns `false`, so a
+/// meaning-bearing knob is never silently dropped.
+pub(super) fn strip_droppable_field(request: &mut ChatCompletionRequest, field: &str) -> bool {
+    if !DROPPABLE_FIELDS.contains(&field) {
+        return false;
+    }
+    match field {
+        "prompt_cache_retention" => request.prompt_cache_retention.take().is_some(),
+        "prompt_cache_key" => request.prompt_cache_key.take().is_some(),
+        "reasoning_effort" => request.reasoning_effort.take().is_some(),
+        _ => false,
+    }
+}
+
+/// Pulls a human-readable message out of a Venice error body for surfacing in the room. Venice's
+/// usual envelope is `{"error": "..."}`; some OpenAI-compatible paths nest `{"error": {"message":
+/// "..."}}`. Falls back to the trimmed raw body (length-capped so a large body cannot flood the
+/// room) and finally to a fixed string for an empty body, so the caller always has something to
+/// show.
+pub(super) fn extract_error_message(body: &str) -> String {
+    let trimmed = body.trim();
+    if trimmed.is_empty() {
+        return "no response body".to_owned();
+    }
+
+    if let Ok(value) = serde_json::from_str::<serde_json::Value>(trimmed) {
+        if let Some(msg) = value.get("error").and_then(|e| e.as_str()) {
+            return msg.to_owned();
+        }
+        if let Some(msg) = value
+            .get("error")
+            .and_then(|e| e.get("message"))
+            .and_then(|m| m.as_str())
+        {
+            return msg.to_owned();
+        }
+    }
+
+    const MAX: usize = 500;
+    if trimmed.chars().count() > MAX {
+        trimmed.chars().take(MAX).collect::<String>() + "…"
+    } else {
+        trimmed.to_owned()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn full_request() -> ChatCompletionRequest {
+        ChatCompletionRequest {
+            model: "venice-uncensored".to_owned(),
+            messages: vec![],
+            temperature: Some(0.7),
+            max_completion_tokens: Some(1024),
+            top_p: None,
+            frequency_penalty: None,
+            presence_penalty: None,
+            repetition_penalty: None,
+            reasoning_effort: Some("high".to_owned()),
+            prompt_cache_key: Some("cafef00d".to_owned()),
+            prompt_cache_retention: Some("24h".to_owned()),
+            venice_parameters: None,
+        }
+    }
+
+    #[test]
+    fn parses_the_rejected_field_from_a_real_venice_body() {
+        let body = r#"{"error":"Extra inputs are not permitted, field: 'prompt_cache_retention', value: 'default'","request_id":"qM_DmKSXKF07wRxmQJ-hc"}"#;
+        assert_eq!(
+            parse_rejected_field(body).as_deref(),
+            Some("prompt_cache_retention")
+        );
+    }
+
+    #[test]
+    fn returns_no_field_when_the_body_has_no_field_marker() {
+        // A different 400 class (e.g. a genuinely malformed request) carries no `field: '..'`
+        // marker, so there is nothing to strip and the caller must surface the error instead.
+        assert_eq!(parse_rejected_field(r#"{"error":"Invalid request"}"#), None);
+        assert_eq!(parse_rejected_field(""), None);
+    }
+
+    #[test]
+    fn strips_a_droppable_field_once_then_reports_no_progress() {
+        let mut request = full_request();
+
+        // First strip clears the field and reports progress, so the caller retries.
+        assert!(strip_droppable_field(
+            &mut request,
+            "prompt_cache_retention"
+        ));
+        assert!(request.prompt_cache_retention.is_none());
+
+        // A repeat rejection for the same (now absent) field reports no progress: this is what
+        // stops the retry loop instead of spinning forever.
+        assert!(!strip_droppable_field(
+            &mut request,
+            "prompt_cache_retention"
+        ));
+    }
+
+    #[test]
+    fn refuses_to_strip_a_meaning_bearing_field() {
+        let mut request = full_request();
+
+        // `temperature` is universal and changes the output; a rejection for it must surface, never
+        // be silently dropped. The whole droppable set is the only thing strip will touch.
+        assert!(!strip_droppable_field(&mut request, "temperature"));
+        assert_eq!(request.temperature, Some(0.7));
+
+        assert!(!strip_droppable_field(
+            &mut request,
+            "max_completion_tokens"
+        ));
+        assert_eq!(request.max_completion_tokens, Some(1024));
+
+        for field in DROPPABLE_FIELDS {
+            assert!(
+                strip_droppable_field(&mut full_request(), field),
+                "every advertised droppable field must actually be strippable: {field}"
+            );
+        }
+    }
+
+    #[test]
+    fn cache_records_per_model_and_isolates_models() {
+        let cache = UnsupportedFieldsCache::default();
+        assert!(cache.known_for("venice-uncensored").is_empty());
+
+        cache.record("venice-uncensored", "prompt_cache_retention");
+        cache.record("venice-uncensored", "reasoning_effort");
+
+        let known = cache.known_for("venice-uncensored");
+        assert!(known.contains("prompt_cache_retention"));
+        assert!(known.contains("reasoning_effort"));
+
+        // A rejection learned for one model must not leak to another.
+        assert!(cache.known_for("kimi-k2-5").is_empty());
+    }
+
+    #[test]
+    fn extracts_a_human_message_from_error_envelopes() {
+        assert_eq!(
+            extract_error_message(r#"{"error":"Extra inputs are not permitted","request_id":"x"}"#),
+            "Extra inputs are not permitted"
+        );
+
+        // OpenAI-style nested envelope.
+        assert_eq!(
+            extract_error_message(r#"{"error":{"message":"context length exceeded"}}"#),
+            "context length exceeded"
+        );
+
+        // Unknown shape falls back to the raw body; empty falls back to a fixed string.
+        assert_eq!(
+            extract_error_message("plain text failure"),
+            "plain text failure"
+        );
+        assert_eq!(extract_error_message("   "), "no response body");
+    }
+}