fix(chat): BYOK dialect defaults to Passthrough; dedicated Mistral dialect

C1 (decision): custom/BYOK ChatCompletions entries defaulted to the Kimi dialect, leaking Kimi-specific body mutations (thinking:{…} control, replayed reasoning_content, schema rewrites) to arbitrary third-party OpenAI-compatible servers. The default is now Passthrough (vanilla OpenAI semantics), with ONE exception mirroring Pi's base-url quirk sniffing: entries pointed at the house/Kimi coding endpoint keep the Kimi dialect. Registry platforms are unaffected (all declare explicitly). C2 (Pi mistral-conversations normalizer): Mistral requires tool-call ids of EXACTLY nine [a-zA-Z0-9] chars; even same-session synthesized UUIDs violate it. New ChatCompat::Mistral = StrictOpenAi behavior + the normalizer — strip non-alphanumerics, keep exact-9 ids, else FNV-1a → base36 (build-stable, deterministic across requests for prefix-cache stability) with collision retry; ONE map covers tool_calls[].id and tool_call_id so pairing survives. The mistral registry row and the persisted 'mistral' serde value both resolve to it (pre-rename Mistral sessions gain the contract automatically); Cerebras/NVIDIA stay on StrictOpenAi untouched. Verified: models+sampling-types+sampler+chat-state+shell all green, clippy clean.
2026-07-23 10:31:45 -04:00
parent d3c9380307
commit 301eb61de4
5 changed files with 247 additions and 15 deletions
@@ -70,9 +70,12 @@ pub enum PlatformChatCompat {
    Kimi,
    DeepSeek,
    Passthrough,
-    /// Strict OpenAI-compatible validator (Mistral, Cerebras) — strips
+    /// Strict OpenAI-compatible validator (Cerebras, NVIDIA) — strips
    /// `stream_options` and private fields.
    StrictOpenAi,
+    /// Mistral: StrictOpenAi plus its exactly-9-alphanumeric tool-call id
+    /// contract (foreign/OpenAI-style ids are deterministically remapped).
+    Mistral,
 }

 /// How a platform's API key rides requests (listing, validation, inference).
@@ -599,10 +602,12 @@ const MISTRAL_SPEC: PlatformSpec = PlatformSpec {
    wire_serves_metadata: false,
    wire_api: PlatformWireApi::ChatCompletions,
    listing: ListingDialect::OpenAi,
-    // Mistral's strict validator 422s on `stream_options`, and its reasoning
-    // models return array content — the StrictOpenAi dialect strips
-    // stream_options; the response deserializer handles arrays universally.
-    chat_compat: PlatformChatCompat::StrictOpenAi,
+    // Mistral's strict validator 422s on `stream_options`, its reasoning
+    // models return array content, and tool-call ids must be EXACTLY nine
+    // `[a-zA-Z0-9]` chars — the Mistral dialect strips stream_options and
+    // deterministically remaps non-conforming (foreign/OpenAI-style) ids;
+    // the response deserializer handles arrays universally.
+    chat_compat: PlatformChatCompat::Mistral,
    key_header: PlatformKeyHeader::Bearer,
    // The listing carries embed/moderation/OCR entries; keep tool-calling
    // chat models only.
@@ -51,6 +51,91 @@ pub(crate) fn adapt_chat_completions_body_for(
            strip_kigi_private_message_fields(body);
            strip_stream_options(body);
        }
+        kigi_sampling_types::ChatCompat::Mistral => {
+            strip_kigi_private_message_fields(body);
+            strip_stream_options(body);
+            normalize_mistral_tool_call_ids(body);
+        }
+    }
+}
+
+/// Mistral's validator requires tool-call ids of EXACTLY nine
+/// `[a-zA-Z0-9]` characters. Foreign backends mint arbitrary ids
+/// (OpenAI `call_…`, UUIDs, Anthropic `toolu_…`), so non-conforming ids
+/// are remapped deterministically — ported from Pi's
+/// `mistral-conversations.ts` normalizer: strip non-alphanumerics, keep
+/// the id when the result is already exactly nine chars, otherwise hash
+/// (FNV-1a → base36) down to nine, retrying with an attempt suffix on
+/// collision. ONE map serves `tool_calls[].id` and `tool_call_id` alike,
+/// so call/result pairing survives.
+fn normalize_mistral_tool_call_ids(body: &mut Value) {
+    const LEN: usize = 9;
+
+    fn derive(id: &str, attempt: u32) -> String {
+        let normalized: String = id.chars().filter(char::is_ascii_alphanumeric).collect();
+        if attempt == 0 && normalized.len() == LEN {
+            return normalized;
+        }
+        let seed_base = if normalized.is_empty() {
+            id
+        } else {
+            &normalized
+        };
+        let seed = if attempt == 0 {
+            seed_base.to_string()
+        } else {
+            format!("{seed_base}:{attempt}")
+        };
+        // FNV-1a (stable across builds, unlike std's DefaultHasher) → base36.
+        let mut hash: u64 = 0xcbf2_9ce4_8422_2325;
+        for b in seed.bytes() {
+            hash ^= u64::from(b);
+            hash = hash.wrapping_mul(0x0000_0100_0000_01b3);
+        }
+        let mut out = String::with_capacity(LEN);
+        let digits = b"0123456789abcdefghijklmnopqrstuvwxyz";
+        let mut h = hash;
+        while out.len() < LEN {
+            out.push(digits[(h % 36) as usize] as char);
+            h = h / 36 + 1; // +1 keeps the stream from collapsing to zeros
+        }
+        out
+    }
+
+    let Some(messages) = body.get_mut("messages").and_then(|m| m.as_array_mut()) else {
+        return;
+    };
+    let mut forward: std::collections::HashMap<String, String> = std::collections::HashMap::new();
+    let mut taken: std::collections::HashSet<String> = std::collections::HashSet::new();
+    let mut normalize = |id: &str| -> String {
+        if let Some(mapped) = forward.get(id) {
+            return mapped.clone();
+        }
+        let mut attempt = 0;
+        loop {
+            let candidate = derive(id, attempt);
+            if taken.insert(candidate.clone()) {
+                forward.insert(id.to_string(), candidate.clone());
+                return candidate;
+            }
+            attempt += 1;
+        }
+    };
+    for message in messages.iter_mut() {
+        if let Some(tool_calls) = message.get_mut("tool_calls").and_then(|t| t.as_array_mut()) {
+            for tc in tool_calls {
+                if let Some(id) = tc.get("id").and_then(|v| v.as_str()).map(str::to_owned) {
+                    tc["id"] = Value::String(normalize(&id));
+                }
+            }
+        }
+        if let Some(id) = message
+            .get("tool_call_id")
+            .and_then(|v| v.as_str())
+            .map(str::to_owned)
+        {
+            message["tool_call_id"] = Value::String(normalize(&id));
+        }
    }
 }

@@ -666,4 +751,56 @@ mod tests {
        assert_eq!(props["num"]["type"], json!("number"));
        assert_eq!(props["free"]["type"], json!("string"));
    }
+
+    /// Mistral dialect: exactly-nine `[a-zA-Z0-9]` tool-call ids. A
+    /// conforming id survives; foreign ids (OpenAI `call_…`, UUIDs) remap
+    /// deterministically; the SAME map serves `tool_calls[].id` and
+    /// `tool_call_id`, so pairing survives; distinct inputs never collide.
+    #[test]
+    fn mistral_dialect_normalizes_tool_call_ids_symmetrically() {
+        let mut body = serde_json::json!({
+            "messages": [
+                {"role": "assistant", "tool_calls": [
+                    {"id": "abc123XYZ", "type": "function", "function": {"name": "a", "arguments": "{}"}},
+                    {"id": "call_0123456789abcdef", "type": "function", "function": {"name": "b", "arguments": "{}"}}
+                ]},
+                {"role": "tool", "tool_call_id": "abc123XYZ", "content": "r1"},
+                {"role": "tool", "tool_call_id": "call_0123456789abcdef", "content": "r2"},
+            ],
+            "stream_options": {"include_usage": true}
+        });
+        adapt_chat_completions_body_for(kigi_sampling_types::ChatCompat::Mistral, &mut body);
+
+        let msgs = body["messages"].as_array().unwrap();
+        let ids: Vec<String> = msgs[0]["tool_calls"]
+            .as_array()
+            .unwrap()
+            .iter()
+            .map(|tc| tc["id"].as_str().unwrap().to_string())
+            .collect();
+        // Conforming id kept verbatim.
+        assert_eq!(ids[0], "abc123XYZ");
+        // Foreign id remapped to exactly nine alphanumerics.
+        assert_eq!(ids[1].len(), 9, "{ids:?}");
+        assert!(ids[1].chars().all(|ch| ch.is_ascii_alphanumeric()));
+        assert_ne!(ids[0], ids[1], "distinct inputs must not collide");
+        // Results carry the SAME mapped ids.
+        assert_eq!(msgs[1]["tool_call_id"].as_str().unwrap(), ids[0]);
+        assert_eq!(msgs[2]["tool_call_id"].as_str().unwrap(), ids[1]);
+        // StrictOpenAi base behavior rides along.
+        assert!(body.get("stream_options").is_none());
+
+        // Determinism: the same foreign id maps identically in a fresh body.
+        let mut body2 = serde_json::json!({
+            "messages": [
+                {"role": "tool", "tool_call_id": "call_0123456789abcdef", "content": "r"}
+            ]
+        });
+        adapt_chat_completions_body_for(kigi_sampling_types::ChatCompat::Mistral, &mut body2);
+        assert_eq!(
+            body2["messages"][0]["tool_call_id"].as_str().unwrap(),
+            ids[1],
+            "remap must be deterministic across requests (prefix-cache stability)"
+        );
+    }
 }
@@ -1260,15 +1260,21 @@ pub enum ChatCompat {
    DeepSeek,
    /// Leave the body as-is (OpenAI-style `reasoning_effort` passes through).
    Passthrough,
-    /// Strict OpenAI-compatible validators (Mistral, Cerebras) reject any
+    /// Strict OpenAI-compatible validators (Cerebras, NVIDIA) reject any
    /// out-of-schema request field with a 4xx (`additionalProperties:false`).
    /// kigi injects `stream_options.include_usage` on every streaming
    /// request, which such validators reject, so it is stripped (streaming
    /// usage falls back to token estimation). `reasoning_effort` passes
    /// through; private message fields are stripped like Passthrough.
-    /// (Serde alias `mistral` keeps sessions persisted before the rename.)
-    #[serde(alias = "mistral")]
    StrictOpenAi,
+    /// Mistral: [`Self::StrictOpenAi`] behavior plus its exactly-nine
+    /// `[a-zA-Z0-9]` tool-call id contract — foreign/OpenAI-style ids are
+    /// deterministically remapped on call+result in one shared map (the
+    /// Pi `mistral-conversations` normalizer). Serializes as `mistral`, so
+    /// sessions persisted before the StrictOpenAi rename (which carried
+    /// the `mistral` alias) resolve here — correct, they were Mistral
+    /// sessions.
+    Mistral,
 }

 pub const REASONING_EFFORT_META_KEY: &str = "reasoningEffort";
@@ -1731,9 +1737,15 @@ mod tests {
    /// persisted before the rename still deserialize.
    #[test]
    fn chat_compat_mistral_alias_deserializes_to_strict_openai() {
+        // `mistral` resolves to the dedicated Mistral dialect — including
+        // sessions persisted before the StrictOpenAi rename (they were
+        // Mistral sessions and now get the 9-char id contract too).
        let v: ChatCompat = serde_json::from_str("\"mistral\"").unwrap();
-        assert_eq!(v, ChatCompat::StrictOpenAi);
-        // New value round-trips as strict_open_ai.
+        assert_eq!(v, ChatCompat::Mistral);
+        assert_eq!(
+            serde_json::to_string(&ChatCompat::Mistral).unwrap(),
+            "\"mistral\""
+        );
        let v: ChatCompat = serde_json::from_str("\"strict_open_ai\"").unwrap();
        assert_eq!(v, ChatCompat::StrictOpenAi);
        assert_eq!(
@@ -4099,8 +4099,12 @@ pub fn sampling_config_for_model(
        &credentials.base_url,
    );
    let api_backend = info.api_backend.clone();
-    // Managed platform entries speak their registry dialect; BYOK/custom
-    // entries keep the historical Kimi body adaptation.
+    // Managed platform entries speak their registry dialect. BYOK/custom
+    // entries default to Passthrough (vanilla OpenAI semantics — the
+    // Kimi-specific body mutations `thinking:{…}` + replayed
+    // `reasoning_content` 400 on third-party OpenAI-compatible servers),
+    // EXCEPT entries pointed at the house/Kimi coding endpoint, which keep
+    // the historical Kimi dialect (mirrors Pi's base-url quirk sniffing).
    let chat_compat = info
        .id
        .as_deref()
@@ -4114,8 +4118,15 @@ pub fn sampling_config_for_model(
            kigi_models::PlatformChatCompat::StrictOpenAi => {
                kigi_sampling_types::ChatCompat::StrictOpenAi
            }
+            kigi_models::PlatformChatCompat::Mistral => kigi_sampling_types::ChatCompat::Mistral,
        })
-        .unwrap_or_default();
+        .unwrap_or_else(|| {
+            if crate::util::is_effective_coding_endpoint_url(&credentials.base_url) {
+                kigi_sampling_types::ChatCompat::Kimi
+            } else {
+                kigi_sampling_types::ChatCompat::Passthrough
+            }
+        });
    // Claude Pro/Max OAuth Messages adaptation: a managed key whose platform is
    // a generic-OAuth Messages provider (claude-pro-max) drives the OAuth
    // identity headers + "You are Claude Code" system prefix in the sampler.
@@ -5963,6 +5974,72 @@ reasoning_effort = "low"
            "agentType should always be in meta, defaulting to DEFAULT_AGENT_TYPE"
        );
    }
+    /// BYOK/custom entries (no managed platform key) default to the
+    /// Passthrough dialect — the historical Kimi default leaked
+    /// Kimi-specific body mutations (`thinking:{…}`, replayed
+    /// `reasoning_content`) to third-party OpenAI-compatible servers.
+    /// The one exception: entries pointed at the house/Kimi coding
+    /// endpoint keep the Kimi dialect (base-url detection, mirroring
+    /// Pi's quirk sniffing).
+    #[test]
+    fn byok_custom_entries_default_to_passthrough_except_house_endpoint() {
+        let make_cfg = |base_url: &str| {
+            let entry_cfg = ModelEntryConfig {
+                id: None, // BYOK: no managed platform key
+                model: "my-custom-model".to_string(),
+                base_url: base_url.to_string(),
+                name: None,
+                description: None,
+                max_completion_tokens: None,
+                temperature: None,
+                top_p: None,
+                api_key: None,
+                env_key: None,
+                api_backend: ApiBackend::default(),
+                auth_scheme: None,
+                extra_headers: IndexMap::new(),
+                context_window: NonZeroU64::new(200_000).unwrap(),
+                auto_compact_threshold_percent: None,
+                system_prompt_label: None,
+                api_base_url: None,
+                use_concise: true,
+                agent_type: default_agent_type(),
+                inference_idle_timeout_secs: None,
+                max_retries: None,
+                hidden: false,
+                supported_in_api: true,
+                reasoning_effort: None,
+                supports_reasoning_effort: false,
+                reasoning_efforts: Vec::new(),
+                capabilities: Vec::new(),
+                supports_backend_search: false,
+                compactions_remaining: None,
+                compaction_at_tokens: None,
+                show_model_fingerprint: false,
+                stream_tool_calls: None,
+                laziness_detector: LazinessDetectorPerModelConfig::default(),
+            };
+            let entry = ModelEntry::from_config_entry(&entry_cfg);
+            let creds = ResolvedCredentials {
+                api_key: Some("sk-byok".into()),
+                base_url: base_url.to_string(),
+                auth_type: kigi_chat_state::AuthType::ApiKey,
+                auth_scheme: Default::default(),
+            };
+            sampling_config_for_model(&entry, creds, None)
+        };
+        assert_eq!(
+            make_cfg("https://api.third-party.example/v1").chat_compat,
+            kigi_sampling_types::ChatCompat::Passthrough,
+            "third-party BYOK must get vanilla OpenAI semantics"
+        );
+        assert_eq!(
+            make_cfg("https://api.kimi.com/coding/v1").chat_compat,
+            kigi_sampling_types::ChatCompat::Kimi,
+            "the house coding endpoint keeps the Kimi dialect"
+        );
+    }
+
    /// Managed `{platform}/{model}` entries stamp `meta.provider` with the
    /// platform's display name so the client's model picker can say which
    /// connected provider each model belongs to. User-defined `[model.*]`
@@ -1841,8 +1841,9 @@ mod tests {
        let cfg = crate::agent::config::sampling_config_for_model(&model_entry, creds, None);
        assert_eq!(
            cfg.chat_compat,
-            kigi_sampling_types::ChatCompat::StrictOpenAi,
-            "mistral entries use the StrictOpenAi dialect (stream_options strip)"
+            kigi_sampling_types::ChatCompat::Mistral,
+            "mistral entries use the Mistral dialect (StrictOpenAi behavior \
+             plus the exactly-nine-alphanumeric tool-call id contract)"
        );
    }