From 122196b718bd29d86cec23c12855c4a4a036c493 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Thu, 4 Jun 2026 19:08:08 +0530 Subject: [PATCH] fix(inference,ai-settings): prevent + handle ollama embedding-model-as-chat (Sentry TAURI-RUST-4P6) (#3359) (#3360) --- .../components/settings/panels/AIPanel.tsx | 17 ++- .../panels/__tests__/aiRouting.test.ts | 70 ++++++++- .../components/settings/panels/aiRouting.ts | 47 ++++++ app/src/services/api/aiSettingsApi.ts | 3 +- app/src/utils/tauriCommands/localAi.ts | 7 + src/openhuman/inference/local/ollama.rs | 85 ++++++++++- .../inference/local/service/ollama_admin.rs | 89 +++++++----- .../inference/provider/compatible.rs | 84 +++++++++++ .../inference/provider/compatible_tests.rs | 135 ++++++++++++++++++ .../inference/provider/config_rejection.rs | 28 ++++ 10 files changed, 517 insertions(+), 48 deletions(-) diff --git a/app/src/components/settings/panels/AIPanel.tsx b/app/src/components/settings/panels/AIPanel.tsx index 49ec9e120..6247a46bb 100644 --- a/app/src/components/settings/panels/AIPanel.tsx +++ b/app/src/components/settings/panels/AIPanel.tsx @@ -54,7 +54,7 @@ import { ConfirmationModal } from '../../intelligence/ConfirmationModal'; import SettingsHeader from '../components/SettingsHeader'; import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; import { ClaudeCodeStatusCard } from './ai/ClaudeCodeStatusCard'; -import { routingWithProviderRemoved } from './aiRouting'; +import { routingWithProviderRemoved, toSelectableChatModels } from './aiRouting'; import { authStyleForBuiltinCloudProvider, BUILTIN_CLOUD_PROVIDER_META, @@ -471,14 +471,13 @@ function useOllamaStatus() { } function useInstalledModels(snapshot: LocalProviderSnapshot | null): OllamaModel[] { - return useMemo(() => { - const list = snapshot?.installedModels ?? []; - return list.map(m => ({ - id: m.name, - sizeBytes: m.size ?? 0, - family: m.name.split(/[:/]/, 1)[0] ?? 'model', - })); - }, [snapshot]); + // Hide embedding-only models (e.g. `bge-m3`) from every LLM/chat workload + // picker — both consumers of this hook (CustomRoutingDialog and + // GlobalOwnModelSelector) route a chat model, never the embedder (which is + // configured separately in EmbeddingsPanel). Selecting an embedding model as + // chat 400s every turn on Ollama (TAURI-RUST-4P6). Filter + map live in the + // pure, unit-tested `toSelectableChatModels` helper. + return useMemo(() => toSelectableChatModels(snapshot?.installedModels ?? []), [snapshot]); } // ───────────────────────────────────────────────────────────────────────────── diff --git a/app/src/components/settings/panels/__tests__/aiRouting.test.ts b/app/src/components/settings/panels/__tests__/aiRouting.test.ts index 3eae75f1a..aae177ce9 100644 --- a/app/src/components/settings/panels/__tests__/aiRouting.test.ts +++ b/app/src/components/settings/panels/__tests__/aiRouting.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest'; import type { CloudProvider, ProviderRef, RoutingMap } from '../AIPanel'; -import { routingWithProviderRemoved } from '../aiRouting'; +import { + isChatSelectableLocalModel, + routingWithProviderRemoved, + toSelectableChatModels, +} from '../aiRouting'; const WORKLOADS = [ 'chat', @@ -100,3 +104,67 @@ describe('routingWithProviderRemoved', () => { expect(Object.keys(next).sort()).toEqual([...WORKLOADS].sort()); }); }); + +describe('isChatSelectableLocalModel (TAURI-RUST-4P6)', () => { + it('hides models the core flagged embedding-only (chat_capable=false)', () => { + expect(isChatSelectableLocalModel({ chat_capable: false })).toBe(false); + }); + + it('keeps chat-capable models (chat_capable=true)', () => { + expect(isChatSelectableLocalModel({ chat_capable: true })).toBe(true); + }); + + it('keeps models with unknown capability — fail-open', () => { + // null / undefined / missing → unknown, must stay visible so an older + // Ollama or an /api/show miss never hides a usable chat model. + expect(isChatSelectableLocalModel({ chat_capable: null })).toBe(true); + expect(isChatSelectableLocalModel({ chat_capable: undefined })).toBe(true); + expect(isChatSelectableLocalModel({})).toBe(true); + }); + + it('filters an installed-model list down to chat-capable + unknown', () => { + const models = [ + { name: 'llama3', chat_capable: true }, + { name: 'bge-m3', chat_capable: false }, + { name: 'mystery', chat_capable: null }, + ]; + expect(models.filter(isChatSelectableLocalModel).map(m => m.name)).toEqual([ + 'llama3', + 'mystery', + ]); + }); +}); + +describe('toSelectableChatModels (TAURI-RUST-4P6)', () => { + it('drops embedding-only models and maps the rest to picker shape', () => { + const out = toSelectableChatModels([ + { name: 'llama3:8b', size: 4_700_000_000, chat_capable: true }, + { name: 'bge-m3:latest', size: 1_200_000_000, chat_capable: false }, + { name: 'mystery', size: 100, chat_capable: null }, + ]); + expect(out).toEqual([ + { id: 'llama3:8b', sizeBytes: 4_700_000_000, family: 'llama3' }, + { id: 'mystery', sizeBytes: 100, family: 'mystery' }, + ]); + }); + + it('derives family from the name before the first `:` or `/` separator', () => { + const out = toSelectableChatModels([ + { name: 'qwen2.5:14b', chat_capable: true }, + { name: 'library/phi3', chat_capable: true }, + ]); + expect(out.map(m => m.family)).toEqual(['qwen2.5', 'library']); + }); + + it('defaults sizeBytes to 0 when size is null/undefined', () => { + const out = toSelectableChatModels([ + { name: 'a', size: null, chat_capable: true }, + { name: 'b', chat_capable: true }, + ]); + expect(out.map(m => m.sizeBytes)).toEqual([0, 0]); + }); + + it('returns an empty list for no installed models', () => { + expect(toSelectableChatModels([])).toEqual([]); + }); +}); diff --git a/app/src/components/settings/panels/aiRouting.ts b/app/src/components/settings/panels/aiRouting.ts index 6f86c3fa0..40bd29124 100644 --- a/app/src/components/settings/panels/aiRouting.ts +++ b/app/src/components/settings/panels/aiRouting.ts @@ -49,3 +49,50 @@ export function routingWithProviderRemoved( return Object.fromEntries(scrubbed) as RoutingMap; } + +/** + * Whether a locally-installed Ollama model may be offered as a chat/LLM model + * in the settings pickers. + * + * Embedding-only models (e.g. `bge-m3`) cannot serve chat — Ollama 400s every + * turn with `"" does not support chat`, which flooded Sentry + * (TAURI-RUST-4P6). The core reports `chat_capable: false` only when it is + * confident a model is embedding-only (from `/api/show` `capabilities`); a + * `null`/`undefined` value means unknown (older Ollama, or an `/api/show` + * miss) and stays selectable — fail-open, never hide a usable model. The + * embedding model is configured in a separate panel, so hiding these here + * never blocks embedding selection. + */ +export function isChatSelectableLocalModel(model: { chat_capable?: boolean | null }): boolean { + return model.chat_capable !== false; +} + +/** A locally-installed model mapped to the picker shape consumed by the LLM/chat selectors. */ +export interface SelectableChatModel { + id: string; + sizeBytes: number; + family: string; +} + +/** + * Filter the installed-model list down to chat-selectable models (hiding + * embedding-only ones via {@link isChatSelectableLocalModel}) and map each to + * the `{ id, sizeBytes, family }` picker shape. + * + * Pure so the filter + map wiring is unit-testable without rendering the panel + * — both picker consumers (`CustomRoutingDialog` + `GlobalOwnModelSelector`) + * route a chat model, never the embedder (configured separately in + * `EmbeddingsPanel`). Selecting an embedding model as chat 400s every turn on + * Ollama (TAURI-RUST-4P6). + */ +export function toSelectableChatModels( + installed: readonly { name: string; size?: number | null; chat_capable?: boolean | null }[] +): SelectableChatModel[] { + return installed + .filter(isChatSelectableLocalModel) + .map(m => ({ + id: m.name, + sizeBytes: m.size ?? 0, + family: m.name.split(/[:/]/, 1)[0] ?? 'model', + })); +} diff --git a/app/src/services/api/aiSettingsApi.ts b/app/src/services/api/aiSettingsApi.ts index 52a4e1095..89c89eba6 100644 --- a/app/src/services/api/aiSettingsApi.ts +++ b/app/src/services/api/aiSettingsApi.ts @@ -32,6 +32,7 @@ import { openhumanUpdateModelSettings, } from '../../utils/tauriCommands/config'; import { + type InstalledModelInfo, type LocalAiDiagnostics, type LocalAiStatus, type ModelPresetResult, @@ -432,7 +433,7 @@ export interface LocalProviderSnapshot { status: LocalAiStatus | null; diagnostics: LocalAiDiagnostics | null; presets: PresetsResponse | null; - installedModels: Array<{ name: string; size?: number | null }>; + installedModels: InstalledModelInfo[]; } export async function loadLocalProviderSnapshot(): Promise { diff --git a/app/src/utils/tauriCommands/localAi.ts b/app/src/utils/tauriCommands/localAi.ts index b4711adf4..c7cccb3e5 100644 --- a/app/src/utils/tauriCommands/localAi.ts +++ b/app/src/utils/tauriCommands/localAi.ts @@ -190,6 +190,13 @@ export interface InstalledModelInfo { /** Native context window in tokens, or null when `/api/show` didn't report it. */ context_length?: number | null; eligibility?: ModelContextEligibility | null; + /** + * Whether the model can serve chat/completions (from Ollama `/api/show` + * `capabilities`). `false` = embedding-only model that must be hidden from + * the chat-model picker; `null` = unknown (older Ollama / `/api/show` miss), + * treated as visible (fail-open). See Sentry TAURI-RUST-4P6. + */ + chat_capable?: boolean | null; } export interface LocalAiDiagnostics { diff --git a/src/openhuman/inference/local/ollama.rs b/src/openhuman/inference/local/ollama.rs index 998bd2026..721789dcb 100644 --- a/src/openhuman/inference/local/ollama.rs +++ b/src/openhuman/inference/local/ollama.rs @@ -275,6 +275,18 @@ pub(crate) struct OllamaModelTag { pub modified_at: Option, } +/// Resolved per-model signals from one Ollama `POST /api/show` round-trip. +/// +/// Both fields are `None` when `/api/show` failed or omitted the data: +/// `context_length` → an `Unknown` memory-layer eligibility verdict; +/// `chat_capable` → "keep visible" in the chat picker (fail-open). See +/// [`OllamaShowResponse::chat_capability`] and Sentry TAURI-RUST-4P6. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct OllamaModelShow { + pub context_length: Option, + pub chat_capable: Option, +} + /// Request body for Ollama `POST /api/show`. #[derive(Debug, Serialize)] pub(crate) struct OllamaShowRequest { @@ -294,9 +306,11 @@ pub(crate) struct OllamaShowRequest { pub(crate) struct OllamaShowResponse { #[serde(default)] pub model_info: serde_json::Map, - // Present in the Ollama API response; retained to document the wire shape, not yet consumed. + /// Capability tags Ollama advertises for the model (e.g. `"completion"`, + /// `"tools"`, `"vision"`, `"embedding"`, `"insert"`). Consumed by + /// [`OllamaShowResponse::chat_capability`] to keep embedding-only models + /// out of the chat-model picker (Sentry TAURI-RUST-4P6). #[serde(default)] - #[allow(dead_code)] pub capabilities: Vec, } @@ -306,6 +320,43 @@ impl OllamaShowResponse { pub(crate) fn context_length(&self) -> Option { context_length_from_model_info(&self.model_info) } + + /// Whether this model can serve chat/completions, from its `capabilities`. + pub(crate) fn chat_capability(&self) -> Option { + ollama_chat_capability(&self.capabilities) + } +} + +/// Classify whether an Ollama model can serve chat/completions from its +/// `/api/show` `capabilities` list. +/// +/// Ollama tags text-generation models with `"completion"` (and newer builds +/// also `"chat"`); embedding models are tagged `"embedding"` only. We only +/// declare a model **not** chat-capable when we are confident it is +/// embedding-only — capabilities is non-empty, carries an embedding marker, +/// and carries no completion/chat marker. Anything ambiguous returns `None` +/// (unknown): +/// * empty / absent capabilities (older Ollama, or an `/api/show` miss); +/// * a tag set we don't recognise (e.g. `["insert"]` only). +/// Callers treat `None` as "keep visible" — fail-open, never hide a model +/// that might be usable for chat. Mirrors the non-rejecting `Unknown` arm of +/// [`super::model_requirements::ContextEligibility`]. See Sentry TAURI-RUST-4P6. +pub(crate) fn ollama_chat_capability(capabilities: &[String]) -> Option { + if capabilities.is_empty() { + return None; + } + let has = |needle: &str| { + capabilities + .iter() + .any(|c| c.trim().eq_ignore_ascii_case(needle)) + }; + if has("completion") || has("chat") { + Some(true) + } else if has("embedding") || has("embed") { + Some(false) + } else { + None + } } /// Extract `.context_length` from an Ollama `model_info` map. @@ -418,6 +469,36 @@ pub(crate) fn ns_to_tps(tokens: f32, duration_ns: u64) -> Option { mod tests { use super::*; + #[test] + fn chat_capability_classifies_embedding_completion_and_unknown() { + let owned = |xs: &[&str]| xs.iter().map(|s| s.to_string()).collect::>(); + + // Embedding-only model (bge-m3) → not chat-capable. TAURI-RUST-4P6. + assert_eq!(ollama_chat_capability(&owned(&["embedding"])), Some(false)); + // Chat/completion models → chat-capable. + assert_eq!(ollama_chat_capability(&owned(&["completion"])), Some(true)); + assert_eq!( + ollama_chat_capability(&owned(&["completion", "tools", "vision"])), + Some(true) + ); + assert_eq!(ollama_chat_capability(&owned(&["chat"])), Some(true)); + // A model exposing BOTH stays chat-capable (completion wins). + assert_eq!( + ollama_chat_capability(&owned(&["embedding", "completion"])), + Some(true) + ); + // Unknown / fail-open: empty, or a tag set we don't recognise → None + // (caller keeps the model visible). + assert_eq!(ollama_chat_capability(&[]), None); + assert_eq!(ollama_chat_capability(&owned(&["insert"])), None); + // Case / whitespace tolerant. + assert_eq!( + ollama_chat_capability(&owned(&[" Embedding "])), + Some(false) + ); + assert_eq!(ollama_chat_capability(&owned(&["COMPLETION"])), Some(true)); + } + #[test] fn pull_progress_aggregates_layered_download_events() { let mut progress = OllamaPullProgress::default(); diff --git a/src/openhuman/inference/local/service/ollama_admin.rs b/src/openhuman/inference/local/service/ollama_admin.rs index de2b0e760..bf40cb2e9 100644 --- a/src/openhuman/inference/local/service/ollama_admin.rs +++ b/src/openhuman/inference/local/service/ollama_admin.rs @@ -11,9 +11,9 @@ use crate::openhuman::inference::local::model_requirements::{ evaluate_context, ContextEligibility, MIN_CONTEXT_TOKENS, }; use crate::openhuman::inference::local::ollama::{ - ollama_base_url, ollama_base_url_from_config, validate_ollama_url, OllamaModelTag, - OllamaPullEvent, OllamaPullProgress, OllamaPullRequest, OllamaShowRequest, OllamaShowResponse, - OllamaTagsResponse, + ollama_base_url, ollama_base_url_from_config, validate_ollama_url, OllamaModelShow, + OllamaModelTag, OllamaPullEvent, OllamaPullProgress, OllamaPullRequest, OllamaShowRequest, + OllamaShowResponse, OllamaTagsResponse, }; use crate::openhuman::inference::local::process_util::apply_no_window; use crate::openhuman::inference::local::provider::{provider_from_config, LocalAiProvider}; @@ -890,22 +890,24 @@ impl LocalAiService { let embedding_found = has(&expected_embedding); let vision_found = has(&expected_vision); - // Per-model native context window vs the memory-layer minimum. - // `/api/show` is one bounded round-trip per installed model, - // fetched concurrently and only on this diagnostics path. - let model_eligibilities: Vec = if healthy { + // Per-model native context window (vs the memory-layer minimum) and + // chat-capability. `/api/show` is one bounded round-trip per installed + // model, fetched concurrently and only on this diagnostics path; the + // single call yields both signals. + let model_shows: Vec = if healthy { futures_util::future::join_all( models .iter() - .map(|m| self.fetch_model_context_at(&base_url, &m.name)), + .map(|m| self.fetch_model_show_at(&base_url, &m.name)), ) .await - .into_iter() - .map(evaluate_context) - .collect() } else { Vec::new() }; + let model_eligibilities: Vec = model_shows + .iter() + .map(|s| evaluate_context(s.context_length)) + .collect(); let installed_models: Vec = models .iter() @@ -919,12 +921,17 @@ impl LocalAiService { } _ => None, }; + // `chat_capable: false` → embedding-only model the chat picker + // must hide; `null`/`true` → keep visible (fail-open). + // TAURI-RUST-4P6. + let chat_capable = model_shows.get(i).and_then(|s| s.chat_capable); serde_json::json!({ "name": m.name, "size": m.size, "modified_at": m.modified_at, "context_length": context_length, "eligibility": eligibility, + "chat_capable": chat_capable, }) }) .collect(); @@ -1127,15 +1134,19 @@ impl LocalAiService { Ok(payload.models) } - /// Fetch a model's native context window via Ollama `POST /api/show`. + /// Fetch a model's native context window and chat-capability via Ollama + /// `POST /api/show`. /// - /// Returns `None` on any failure (unreachable, non-2xx, parse error, or - /// the metadata key is absent) — the caller maps that to an `Unknown` - /// eligibility verdict rather than a hard rejection. One bounded HTTP - /// round-trip per model; only ever invoked from the diagnostics path. - async fn fetch_model_context_at(&self, base_url: &str, model: &str) -> Option { + /// Both fields default to `None` on any failure (unreachable, non-2xx, + /// parse error, or the metadata key is absent) — the caller maps a `None` + /// context to an `Unknown` eligibility verdict, and a `None` chat-capable + /// to "keep visible" (fail-open). One bounded HTTP round-trip per model; + /// only ever invoked from the diagnostics path. The single round-trip + /// yields both signals (context for the memory-layer gate, capability for + /// the chat-picker filter — TAURI-RUST-4P6). + async fn fetch_model_show_at(&self, base_url: &str, model: &str) -> OllamaModelShow { let url = format!("{}/api/show", base_url.trim_end_matches('/')); - let resp = self + let resp = match self .http .post(&url) .json(&OllamaShowRequest { @@ -1144,41 +1155,49 @@ impl LocalAiService { .timeout(std::time::Duration::from_secs(5)) .send() .await - .inspect_err(|e| { + { + Ok(resp) => resp, + Err(e) => { tracing::debug!( target: "local_ai::ollama_admin", %url, model, error = %e, - "[local_ai:ollama_admin] fetch_model_context: request failed" + "[local_ai:ollama_admin] fetch_model_show: request failed" ); - }) - .ok()?; + return OllamaModelShow::default(); + } + }; let status = resp.status(); if !status.is_success() { tracing::debug!( target: "local_ai::ollama_admin", %url, model, %status, - "[local_ai:ollama_admin] fetch_model_context: non-success response" + "[local_ai:ollama_admin] fetch_model_show: non-success response" ); - return None; + return OllamaModelShow::default(); } - let parsed: OllamaShowResponse = resp - .json() - .await - .inspect_err(|e| { + let parsed: OllamaShowResponse = match resp.json().await { + Ok(parsed) => parsed, + Err(e) => { tracing::debug!( target: "local_ai::ollama_admin", %url, model, error = %e, - "[local_ai:ollama_admin] fetch_model_context: JSON parse failed" + "[local_ai:ollama_admin] fetch_model_show: JSON parse failed" ); - }) - .ok()?; - let ctx = parsed.context_length(); + return OllamaModelShow::default(); + } + }; + let show = OllamaModelShow { + context_length: parsed.context_length(), + chat_capable: parsed.chat_capability(), + }; tracing::debug!( target: "local_ai::ollama_admin", - model, context_length = ?ctx, - "[local_ai:ollama_admin] fetch_model_context: resolved" + model, + context_length = ?show.context_length, + chat_capable = ?show.chat_capable, + "[local_ai:ollama_admin] fetch_model_show: resolved" ); - ctx + show } async fn lm_studio_diagnostics(&self, config: &Config) -> Result { diff --git a/src/openhuman/inference/provider/compatible.rs b/src/openhuman/inference/provider/compatible.rs index 36ae2a545..5ea32f7c7 100644 --- a/src/openhuman/inference/provider/compatible.rs +++ b/src/openhuman/inference/provider/compatible.rs @@ -196,6 +196,62 @@ impl OpenAiCompatibleProvider { } } + /// Build an actionable error for a model that lacks the chat capability — + /// e.g. an *embedding* model (Ollama `bge-m3`) selected as the chat model. + /// Ollama returns `400 "" does not support chat`; we replace the + /// opaque upstream JSON with concrete remediation. See Sentry + /// TAURI-RUST-4P6. + /// + /// The phrase `does not support chat` is preserved verbatim so the + /// re-reported error still matches + /// [`super::config_rejection::is_provider_config_rejection_message`] and + /// stays demoted from Sentry. + fn not_chat_capable_model_message(&self, model: &str, sanitized: &str) -> String { + format!( + "{name} API error: model '{model}' does not support chat — it \ + appears to be an embedding or non-chat model. Assign a \ + chat-capable model to this provider (e.g. in Settings → AI), or \ + pick a different model. Provider detail: {sanitized}", + name = self.name, + ) + } + + /// Detect a model rejected because it has no chat capability. Unlike the + /// completion-only base model (which 404s), an embedding model picked as + /// the chat model is rejected by Ollama with a **400/422** carrying + /// `"" does not support chat`, so it bypasses + /// [`is_completion_only_model_404`]. Match is tight (the exact phrase) so + /// ordinary 400s keep their normal handling. See Sentry TAURI-RUST-4P6. + fn is_not_chat_capable_model(status: reqwest::StatusCode, error: &str) -> bool { + if !matches!( + status, + reqwest::StatusCode::BAD_REQUEST | reqwest::StatusCode::UNPROCESSABLE_ENTITY + ) { + return false; + } + error.to_lowercase().contains("does not support chat") + } + + /// Guard shared by every chat-completions error handler: if the body shows + /// a non-chat-capable model (embedding model picked as chat), return the + /// actionable error so the caller fails fast with concrete remediation + /// instead of surfacing the opaque upstream JSON. `None` means "not this + /// case — proceed with normal fallback/enrich". See Sentry TAURI-RUST-4P6. + fn not_chat_capable_guard( + &self, + status: reqwest::StatusCode, + sanitized: &str, + model: &str, + ) -> Option { + if Self::is_not_chat_capable_model(status, sanitized) { + Some(anyhow::anyhow!( + self.not_chat_capable_model_message(model, sanitized) + )) + } else { + None + } + } + /// Create a provider with a custom User-Agent header. /// /// Some providers (for example Kimi Code) require a specific User-Agent @@ -1535,6 +1591,13 @@ impl Provider for OpenAiCompatibleProvider { return Err(err); } + // An embedding / non-chat model rejected with 400 "does not + // support chat" (e.g. Ollama bge-m3 picked as the chat model) — + // fail fast with actionable guidance. See Sentry TAURI-RUST-4P6. + if let Some(err) = self.not_chat_capable_guard(status, &sanitized, model) { + return Err(err); + } + if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback { return self .chat_via_responses(credential, &fallback_messages, model) @@ -1736,7 +1799,21 @@ impl Provider for OpenAiCompatibleProvider { return Err(anyhow::anyhow!("{enriched}")); } + // `api_error` reads the body and runs the shared classification + // (SessionExpired publish, config-rejection demotion, Sentry-report + // decision). For a non-chat-capable model (embedding model picked + // as chat → 400 "does not support chat") it already demotes the + // event, but its message is the opaque upstream JSON. Upgrade that + // to the actionable "assign a chat-capable model" copy — which + // still carries the phrase, so it stays demoted on any re-report. + // See Sentry TAURI-RUST-4P6. let err = super::api_error(&self.name, response).await; + let err_str = err.to_string(); + if Self::is_not_chat_capable_model(status, &err_str) { + return Err(anyhow::anyhow!( + self.not_chat_capable_model_message(model, &err_str) + )); + } let enriched = self.enrich_404_message(format!("{err:#}"), status); return Err(anyhow::anyhow!("{enriched}")); } @@ -2113,6 +2190,13 @@ impl Provider for OpenAiCompatibleProvider { return Err(err); } + // An embedding / non-chat model rejected with 400 "does not + // support chat" (e.g. Ollama bge-m3 picked as the chat model) — + // fail fast with actionable guidance. See Sentry TAURI-RUST-4P6. + if let Some(err) = self.not_chat_capable_guard(status, &sanitized, model) { + return Err(err); + } + if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback { return self .chat_via_responses(credential, &effective_messages, model) diff --git a/src/openhuman/inference/provider/compatible_tests.rs b/src/openhuman/inference/provider/compatible_tests.rs index a1f3de390..d1525eabb 100644 --- a/src/openhuman/inference/provider/compatible_tests.rs +++ b/src/openhuman/inference/provider/compatible_tests.rs @@ -2579,6 +2579,141 @@ async fn completion_only_404_fails_fast_without_responses_fallback() { ); } +// ── TAURI-RUST-4P6: embedding model picked as chat model → 400 ─────────────── + +#[test] +fn not_chat_capable_detected_from_ollama_400() { + // Verbatim Ollama wire body when an embedding model (bge-m3) is used as + // the chat model. Sentry issue 5338. + let body = r#"{"error":{"message":"\"bge-m3:latest\" does not support chat","type":"invalid_request_error","param":null,"code":null}}"#; + assert!(OpenAiCompatibleProvider::is_not_chat_capable_model( + reqwest::StatusCode::BAD_REQUEST, + body + )); + // Some compatible backends use 422 for the same class. + assert!(OpenAiCompatibleProvider::is_not_chat_capable_model( + reqwest::StatusCode::UNPROCESSABLE_ENTITY, + body + )); +} + +#[test] +fn not_chat_capable_requires_4xx_status() { + // The exact phrase under a non-4xx status is not this case — let other + // handling deal with 404/5xx so we don't shadow real failures. + let body = "\"bge-m3:latest\" does not support chat"; + assert!(!OpenAiCompatibleProvider::is_not_chat_capable_model( + reqwest::StatusCode::NOT_FOUND, + body + )); + assert!(!OpenAiCompatibleProvider::is_not_chat_capable_model( + reqwest::StatusCode::INTERNAL_SERVER_ERROR, + body + )); +} + +#[test] +fn not_chat_capable_ignores_unrelated_400() { + // An ordinary 400 with no "does not support chat" phrase must keep its + // normal enrich/handling path. + assert!(!OpenAiCompatibleProvider::is_not_chat_capable_model( + reqwest::StatusCode::BAD_REQUEST, + "invalid temperature: only 1 is allowed for this model" + )); +} + +#[test] +fn not_chat_capable_message_names_model_remediation_and_keeps_phrase() { + let p = make_provider("ollama", "http://127.0.0.1:11434/v1", None); + let msg = p.not_chat_capable_model_message( + "bge-m3:latest", + r#"{"error":{"message":"\"bge-m3:latest\" does not support chat"}}"#, + ); + assert!(msg.contains("bge-m3:latest"), "names the model: {msg}"); + assert!( + msg.contains("chat-capable model"), + "states the remediation: {msg}" + ); + // CRITICAL: the actionable rewrite must still carry the upstream phrase so + // the re-reported error stays demoted by the config-rejection classifier + // (otherwise the 36.6k Sentry events come back). See TAURI-RUST-4P6. + assert!( + msg.to_lowercase().contains("does not support chat"), + "must preserve the classifier anchor phrase: {msg}" + ); + assert!( + super::super::is_provider_config_rejection_message(&msg), + "enriched message must classify as a provider config-rejection: {msg}" + ); +} + +#[test] +fn not_chat_capable_guard_fires_only_on_signature() { + let p = make_provider("ollama", "http://127.0.0.1:11434/v1", None); + let hit = p.not_chat_capable_guard( + reqwest::StatusCode::BAD_REQUEST, + "\"bge-m3:latest\" does not support chat", + "bge-m3:latest", + ); + assert!(hit + .expect("guard should fire on the does-not-support-chat 400") + .to_string() + .contains("bge-m3:latest")); + // Unrelated 400 → None (normal handling preserved). + assert!(p + .not_chat_capable_guard( + reqwest::StatusCode::BAD_REQUEST, + "rate limit exceeded", + "bge-m3:latest" + ) + .is_none()); +} + +#[tokio::test] +async fn not_chat_capable_400_fails_fast_with_actionable_message() { + // End-to-end over the wire: an embedding model used as chat 400s, and the + // guard must short-circuit with the actionable message rather than the + // opaque upstream JSON. TAURI-RUST-4P6. + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with(ResponseTemplate::new(400).set_body_json(serde_json::json!({ + "error": { + "message": "\"bge-m3:latest\" does not support chat", + "type": "invalid_request_error", + "param": null, + "code": null + } + }))) + .mount(&server) + .await; + + let provider = OpenAiCompatibleProvider::new( + "ollama", + &format!("{}/v1", server.uri()), + // Dummy key only to clear the pre-flight credential gate; the mock + // ignores auth. Real Ollama is keyless, but the provider requires a + // non-empty credential before dispatching. + Some("x"), + AuthStyle::Bearer, + ); + + let err = provider + .chat_with_history(&[ChatMessage::user("hello")], "bge-m3:latest", 0.0) + .await + .expect_err("embedding-model-as-chat must error"); + let msg = err.to_string(); + assert!( + msg.contains("bge-m3:latest") && msg.contains("chat-capable model"), + "expected actionable does-not-support-chat message, got: {msg}" + ); + // And it must remain demotable — Sentry suppression depends on it. + assert!( + super::super::is_provider_config_rejection_message(&msg), + "bubbled error must classify as config-rejection, got: {msg}" + ); +} + // ── #3205: multimodal [IMAGE:] markers → OpenAI image_url content parts ───────── const TEST_PNG_DATA_URI: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="; diff --git a/src/openhuman/inference/provider/config_rejection.rs b/src/openhuman/inference/provider/config_rejection.rs index 9f2aa689e..bdc3b305f 100644 --- a/src/openhuman/inference/provider/config_rejection.rs +++ b/src/openhuman/inference/provider/config_rejection.rs @@ -236,6 +236,20 @@ pub fn is_provider_config_rejection_message(body: &str) -> bool { // harmless (`.any()` short-circuits) and kept so each Sentry // family stays self-documenting. "does not support tools", + // TAURI-RUST-4P6 (~36.6k events / 2 users) — user picked an + // *embedding* model (Ollama `bge-m3:latest`, OpenHuman's default + // memory-tree embed model) as their chat model. Ollama rejects every + // chat turn with `{"error":{"message":"\"bge-m3:latest\" does not + // support chat","type":"invalid_request_error",...}}` on a 400. Same + // user-state class as `does not support tools`: the model lacks the + // chat capability and the user must pick a chat-capable model — Sentry + // has no remediation. The 400 status bypasses the + // `completion_only_404_guard` (404-only), so without this phrase the + // raw body re-reports every turn. The companion `not_chat_capable_guard` + // in `compatible.rs` rewrites the opaque upstream JSON into an + // actionable "assign a chat-capable model" message that still carries + // this substring, so it stays demoted. + "does not support chat", ]; let lower = body.to_ascii_lowercase(); @@ -358,6 +372,20 @@ mod tests { "TAURI-RUST-2F", r#"cloud streaming API error (400 Bad Request): {"error":{"message":"The `reasoning_content` in the thinking mode must be passed back to the API.","type":"invalid_request_error","param":null,"code":"invalid_request_error"}}"#, ), + // TAURI-RUST-4P6 — user picked an embedding model + // (`bge-m3:latest`) as their Ollama chat model. Ollama 400s every + // chat turn. Verbatim wire body from Sentry issue 5338. + ( + "TAURI-RUST-4P6", + r#"ollama API error (400 Bad Request): {"error":{"message":"\"bge-m3:latest\" does not support chat","type":"invalid_request_error","param":null,"code":null}}"#, + ), + // Same shape after `not_chat_capable_guard` (compatible.rs) + // rewrites it into the actionable message — must still classify so + // the re-reported error stays demoted. + ( + "TAURI-RUST-4P6-enriched", + "ollama API error: model 'bge-m3:latest' does not support chat — it appears to be an embedding or non-chat model. Assign a chat-capable model to this provider (e.g. in Settings → AI), or pick a different model.", + ), ] { assert!( is_provider_config_rejection_message(body),