From e499ce440792f8d7aab210bc0d55cd17cd214157 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Mon, 27 Jul 2026 20:17:18 +0530 Subject: [PATCH 1/2] =?UTF-8?q?fix(inference):=20local=20model=20reliabili?= =?UTF-8?q?ty=20=E2=80=94=20/api/tags=20fallback=20+=20real=20Ollama=20mod?= =?UTF-8?q?el=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scope is narrower than issue #5055 reads, because its two root-cause sub-issues already shipped and are on main: - #5053 (discovery endpoint): `model_discovery_api` / `endpoint_is_openai_v1` in inference/local/provider.rs already gate discovery by endpoint *type* rather than "is it localhost", wired into diagnostics.rs with a regression test. Left as-is. - #5017 (embedding verification): the setup probe already sends a real POST /v1/embeddings with the user's model and key and classifies seven outcomes (auth / unreachable / model-incompatible / dimension-mismatch / no-embeddings-API / no-model-loaded / generic), surfaced by EmbeddingsPanel.tsx and pinned by a mock-endpoint test that captures the auth header and request body. Left as-is. What was actually missing: 1. No 404 fallback. A `/v1/models` 404 now retries the host-rooted `/api/tags` exactly once, logging a warning, so a runtime that only speaks the Ollama listing still discovers its models. Three cases deliberately do NOT recover, because a permissive fallback would reintroduce #5053: a non-404 status never triggers it (a 500 is a server fault, not a wrong-endpoint signal); an empty fallback catalog is not a recovery (LM Studio answers unknown paths with `200 {"error": …}` and zero models); and when the fallback fails the caller sees the original /v1/models error rather than a second, more confusing one. The new `ollama_tags_fallback_url` strips a trailing `/v1` so the probe can never become the malformed `/v1/api/tags`. 2. The resolved discovery URL is now logged at DEBUG, so a wrong base URL is diagnosable from app logs without reproducing against the runtime's own request log. 3. `gemma4` is not a real Ollama namespace. `MVP_ALLOWED_CHAT_MODELS` carried `gemma4:e4b-it-q8_0` and the 16 GB+ preset shipped `gemma4:e4b` for chat and vision, so that tier offered a model no `ollama pull` could fetch — the "stale or unavailable default" failure mode in the issue. The intended model is Gemma 3n, published as `gemma3n:e4b-it-q8_0` (9.5 GB, which is what the tier's approx_download_gb was sized against). Verified against the public library: gemma3:1b-it-qat, gemma3:4b-it-qat, moondream:1.8b-v2-q4_K_S, bge-m3 and gemma3n:e4b-it-q8_0 all resolve; gemma4:* does not. This also closes a latent second bug: the preset (`gemma4:e4b`) and the allowlist (`gemma4:e4b-it-q8_0`) disagreed, so selecting the tier would have been silently redirected to gemma3:1b-it-qat by enforce_mvp_chat_allowlist. They now agree. The tier's `quantization` label moves "qat" -> "q8_0" to match what is actually pulled. It is a display string (effective_quantization) and takes no part in resolving the model tag. Tests added: four for the fallback (recovers, original error preserved when the fallback also fails, empty catalog is not a recovery, non-404 never probes twice — asserted with a hit counter), one for the fallback URL derivation, and three guards that no allowlist entry or preset can name the unpullable gemma4 namespace again. The Ollama path is untouched: the fallback lives only in list_lm_studio_models, which a genuine Ollama config never reaches because diagnostics() takes the /api/tags branch first. Fixes #5055 --- src/openhuman/inference/local/lm_studio.rs | 54 +++++++- .../inference/local/service/lm_studio.rs | 129 ++++++++++++++++- .../local/service/ollama_admin_tests.rs | 131 ++++++++++++++++++ src/openhuman/inference/model_ids.rs | 39 +++++- src/openhuman/inference/presets.rs | 48 ++++++- .../inference_local_admin_raw_coverage_e2e.rs | 12 +- ...provider_admin_round22_raw_coverage_e2e.rs | 4 +- 7 files changed, 392 insertions(+), 25 deletions(-) diff --git a/src/openhuman/inference/local/lm_studio.rs b/src/openhuman/inference/local/lm_studio.rs index cf5317402c..e24e6aa8d3 100644 --- a/src/openhuman/inference/local/lm_studio.rs +++ b/src/openhuman/inference/local/lm_studio.rs @@ -291,11 +291,28 @@ pub(crate) struct LmStudioNativeModelsResponse { /// Map a normalized `…/v1` base URL to the LM Studio native models endpoint /// `…/api/v0/models` (a sibling of `/v1`, served at the host root). pub(crate) fn lm_studio_native_models_url(v1_base_url: &str) -> String { - let root = v1_base_url + format!("{}/api/v0/models", host_root_of(v1_base_url)) +} + +/// Strip a trailing `/v1` so sibling endpoints served at the host root can be +/// derived from an OpenAI-compatible base URL. +pub(crate) fn host_root_of(v1_base_url: &str) -> &str { + v1_base_url .trim_end_matches('/') .trim_end_matches("/v1") - .trim_end_matches('/'); - format!("{root}/api/v0/models") + .trim_end_matches('/') +} + +/// Ollama-native `GET /api/tags` URL derived from an OpenAI-compatible base. +/// +/// Only used as the one-shot 404 fallback in +/// [`LocalAiService::list_lm_studio_models`](crate::openhuman::inference::local::service::LocalAiService): +/// some runtimes are reachable on an OpenAI-shaped base URL but expose only the +/// Ollama listing (e.g. plain Ollama configured with a `/v1` base). Discovery is +/// still chosen by provider type first — this is a recovery path, not a probe +/// order (GH #5055). +pub(crate) fn ollama_tags_fallback_url(v1_base_url: &str) -> String { + format!("{}/api/tags", host_root_of(v1_base_url)) } /// Resolve the context window LM Studio reports for `model_id` from a native @@ -336,6 +353,37 @@ mod tests { ); } + /// GH #5055: the `/api/tags` fallback URL is a sibling of `/v1` at the host + /// root. Appending to the `/v1` base would produce `/v1/api/tags` — the + /// exact malformed request LM Studio logs as `Unexpected endpoint or + /// method` (GH #5053). + #[test] + fn ollama_tags_fallback_url_is_host_rooted_not_v1_suffixed() { + assert_eq!( + ollama_tags_fallback_url("http://localhost:1234/v1"), + "http://localhost:1234/api/tags" + ); + assert_eq!( + ollama_tags_fallback_url("http://127.0.0.1:1234/v1/"), + "http://127.0.0.1:1234/api/tags" + ); + assert_eq!( + ollama_tags_fallback_url("https://lm.example.com/lmstudio/v1"), + "https://lm.example.com/lmstudio/api/tags" + ); + // A host-rooted base (no /v1) is left alone. + assert_eq!( + ollama_tags_fallback_url("http://localhost:11434"), + "http://localhost:11434/api/tags" + ); + for url in [ + ollama_tags_fallback_url("http://localhost:1234/v1"), + ollama_tags_fallback_url("http://localhost:1234/v1/"), + ] { + assert!(!url.contains("/v1/api/tags"), "malformed probe URL: {url}"); + } + } + #[test] fn context_window_prefers_loaded_then_max() { let resp: LmStudioNativeModelsResponse = serde_json::from_str( diff --git a/src/openhuman/inference/local/service/lm_studio.rs b/src/openhuman/inference/local/service/lm_studio.rs index 38163e344c..976931f63e 100644 --- a/src/openhuman/inference/local/service/lm_studio.rs +++ b/src/openhuman/inference/local/service/lm_studio.rs @@ -1,9 +1,10 @@ use crate::openhuman::config::Config; use crate::openhuman::inference::local::lm_studio::{ - apply_lm_studio_auth, lm_studio_base_url, LmStudioChatCompletionRequest, - LmStudioChatCompletionResponse, LmStudioChatMessage, LmStudioModelsResponse, + apply_lm_studio_auth, lm_studio_base_url, ollama_tags_fallback_url, + LmStudioChatCompletionRequest, LmStudioChatCompletionResponse, LmStudioChatMessage, + LmStudioModelsResponse, }; -use crate::openhuman::inference::local::ollama::OllamaModelTag; +use crate::openhuman::inference::local::ollama::{OllamaModelTag, OllamaTagsResponse}; use crate::openhuman::inference::model_ids; use super::LocalAiService; @@ -43,11 +44,15 @@ impl LocalAiService { ) -> Result, String> { let base = lm_studio_base_url(config); let url = format!("{base}/models"); + // GH #5055: log the *resolved* discovery URL so a wrong base URL is + // diagnosable from app logs alone, without reproducing against the + // runtime's own request log. tracing::debug!( target: "local_ai::lm_studio", %base, - %url, - "[local_ai:lm_studio] list_models: sending GET" + discovery_url = %url, + api = "openai_v1_models", + "[local_ai:lm_studio] list_models: resolved discovery URL — sending GET" ); let request = self @@ -78,6 +83,20 @@ impl LocalAiService { body = %diagnostic_body_snippet(&body), "[local_ai:lm_studio] list_models: non-success response" ); + + // GH #5055: a 404 on `/v1/models` means this host is reachable but + // does not serve the OpenAI catalog. Try the Ollama-native + // `/api/tags` exactly once before giving up, so a runtime that only + // speaks the Ollama listing still discovers its models. Discovery is + // still selected by provider *type* first (`model_discovery_api`); + // this is a recovery path, never a probe order, and it never runs + // for any status other than 404. + if status == reqwest::StatusCode::NOT_FOUND { + if let Some(models) = self.list_ollama_tags_fallback(config, &base).await { + return Ok(models); + } + } + return Err(format!( "lm studio models failed with status {}{}", status, @@ -120,6 +139,106 @@ impl LocalAiService { .collect()) } + /// One-shot Ollama-native `/api/tags` fallback for a `/v1/models` 404. + /// + /// Returns `Some(models)` only when the fallback actually produced a + /// catalog; every failure returns `None` so the caller surfaces the original + /// `/v1/models` error rather than a confusing second one. Logs at WARN on + /// entry because taking this path means the configured base URL and the + /// provider type disagree — the user should fix the configuration even + /// though discovery recovered (GH #5055). + async fn list_ollama_tags_fallback( + &self, + config: &Config, + base: &str, + ) -> Option> { + let fallback_url = ollama_tags_fallback_url(base); + tracing::warn!( + target: "local_ai::lm_studio", + %base, + discovery_url = %fallback_url, + api = "ollama_api_tags", + "[local_ai:lm_studio] list_models: /v1/models returned 404 — retrying once against \ + the Ollama-native /api/tags. Check the configured base URL: an OpenAI-compatible \ + runtime should serve /v1/models." + ); + + let request = self + .http + .get(&fallback_url) + .timeout(std::time::Duration::from_secs(5)); + let response = match apply_lm_studio_auth(request, config).send().await { + Ok(r) => r, + Err(e) => { + tracing::debug!( + target: "local_ai::lm_studio", + url = %fallback_url, + error = %e, + "[local_ai:lm_studio] /api/tags fallback request failed" + ); + return None; + } + }; + + let status = response.status(); + if !status.is_success() { + tracing::debug!( + target: "local_ai::lm_studio", + url = %fallback_url, + %status, + "[local_ai:lm_studio] /api/tags fallback returned non-success" + ); + return None; + } + + let body = match response.text().await { + Ok(b) => b, + Err(e) => { + tracing::debug!( + target: "local_ai::lm_studio", + url = %fallback_url, + error = %e, + "[local_ai:lm_studio] /api/tags fallback body read failed" + ); + return None; + } + }; + let payload: OllamaTagsResponse = match serde_json::from_str(&body) { + Ok(p) => p, + Err(e) => { + tracing::debug!( + target: "local_ai::lm_studio", + url = %fallback_url, + error = %e, + body = %diagnostic_body_snippet(&body), + "[local_ai:lm_studio] /api/tags fallback parse failed" + ); + return None; + } + }; + + // An empty catalog is not a recovery — fall through to the original + // /v1/models error so the user sees the real problem. LM Studio answers + // unknown paths with `200 {"error": …}` and no models (GH #5053), which + // would otherwise look like a successful discovery of zero models. + if payload.models.is_empty() { + tracing::debug!( + target: "local_ai::lm_studio", + url = %fallback_url, + "[local_ai:lm_studio] /api/tags fallback returned an empty catalog — not a recovery" + ); + return None; + } + + tracing::info!( + target: "local_ai::lm_studio", + url = %fallback_url, + model_count = payload.models.len(), + "[local_ai:lm_studio] recovered model discovery via the Ollama /api/tags fallback" + ); + Some(payload.models) + } + pub(in crate::openhuman::inference::local::service) async fn has_lm_studio_model( &self, config: &Config, diff --git a/src/openhuman/inference/local/service/ollama_admin_tests.rs b/src/openhuman/inference/local/service/ollama_admin_tests.rs index fb0ff10787..eb7bb4e588 100644 --- a/src/openhuman/inference/local/service/ollama_admin_tests.rs +++ b/src/openhuman/inference/local/service/ollama_admin_tests.rs @@ -1277,3 +1277,134 @@ async fn diagnostics_gates_models_by_context_window() { std::env::remove_var("OPENHUMAN_OLLAMA_BASE_URL"); } } + +// ── GH #5055: one-shot /api/tags fallback on a /v1/models 404 ─────────────── +// +// Discovery is still chosen by provider *type* (`model_discovery_api`); these +// cover the recovery path taken when the chosen OpenAI-compatible endpoint +// answers 404, so a runtime that only speaks the Ollama listing is not left +// with an empty catalog. + +/// A `/v1/models` 404 falls back to the host-rooted `/api/tags` exactly once +/// and returns that catalog. +#[tokio::test] +async fn v1_models_404_falls_back_to_ollama_api_tags() { + let _guard = crate::openhuman::inference::inference_test_guard(); + + let app = Router::new() + .route( + "/v1/models", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "404 page not found") }), + ) + .route( + "/api/tags", + get(|| async { + Json(json!({ + "models": [ + { "name": "local-model", "size": 42, "modified_at": "2026-01-01T00:00:00Z" } + ] + })) + }), + ); + let base = spawn_mock(app).await; + let config = lm_studio_config(&base); + let service = LocalAiService::new(&config); + + let models = service + .list_lm_studio_models(&config) + .await + .expect("the /api/tags fallback should recover discovery"); + + assert_eq!(models.len(), 1); + assert_eq!(models[0].name, "local-model"); +} + +/// When neither endpoint serves a catalog, the caller sees the original +/// `/v1/models` failure — not a second, more confusing error from the fallback. +#[tokio::test] +async fn v1_models_404_reports_the_original_error_when_fallback_also_fails() { + let _guard = crate::openhuman::inference::inference_test_guard(); + + let app = Router::new().route( + "/v1/models", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "404 page not found") }), + ); + let base = spawn_mock(app).await; + let config = lm_studio_config(&base); + let service = LocalAiService::new(&config); + + let err = service + .list_lm_studio_models(&config) + .await + .expect_err("no catalog anywhere must fail"); + + assert!( + err.contains("404"), + "expected the original /v1/models status, got: {err}" + ); +} + +/// LM Studio answers unknown paths with `200 {"error": …}` and no models +/// (GH #5053). An empty fallback catalog must NOT be treated as a recovery, +/// otherwise discovery silently "succeeds" with zero models. +#[tokio::test] +async fn empty_api_tags_fallback_is_not_treated_as_recovery() { + let _guard = crate::openhuman::inference::inference_test_guard(); + + let app = Router::new() + .route( + "/v1/models", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "404 page not found") }), + ) + .route("/api/tags", get(|| async { Json(json!({ "models": [] })) })); + let base = spawn_mock(app).await; + let config = lm_studio_config(&base); + let service = LocalAiService::new(&config); + + let err = service + .list_lm_studio_models(&config) + .await + .expect_err("an empty fallback catalog is not a recovery"); + + assert!(err.contains("404"), "got: {err}"); +} + +/// Any non-404 failure must NOT trigger the fallback: a 500 is a server fault, +/// not a wrong-endpoint signal, and probing a second path would mask it. +#[tokio::test] +async fn non_404_status_does_not_trigger_the_fallback() { + let _guard = crate::openhuman::inference::inference_test_guard(); + + let hits = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let hits_route = std::sync::Arc::clone(&hits); + let app = Router::new() + .route( + "/v1/models", + get(|| async { (axum::http::StatusCode::INTERNAL_SERVER_ERROR, "boom") }), + ) + .route( + "/api/tags", + get(move || { + let hits = std::sync::Arc::clone(&hits_route); + async move { + hits.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Json(json!({ "models": [{ "name": "should-not-be-used" }] })) + } + }), + ); + let base = spawn_mock(app).await; + let config = lm_studio_config(&base); + let service = LocalAiService::new(&config); + + let err = service + .list_lm_studio_models(&config) + .await + .expect_err("a 500 must surface, not fall back"); + + assert!(err.contains("500"), "got: {err}"); + assert_eq!( + hits.load(std::sync::atomic::Ordering::SeqCst), + 0, + "the /api/tags fallback must not run for a non-404 status" + ); +} diff --git a/src/openhuman/inference/model_ids.rs b/src/openhuman/inference/model_ids.rs index f5c03e3462..bc88534d50 100644 --- a/src/openhuman/inference/model_ids.rs +++ b/src/openhuman/inference/model_ids.rs @@ -19,7 +19,14 @@ pub(crate) const DEFAULT_OLLAMA_EMBED_MODEL: &str = "bge-m3"; /// Chat models allowed in the current local Ollama build. /// Any resolved chat model ID not listed here is redirected to `MVP_DEFAULT_CHAT_MODEL`. -const MVP_ALLOWED_CHAT_MODELS: &[&str] = &["gemma3:1b-it-qat", "gemma4:e4b-it-q8_0"]; +/// +/// Every id here must be pullable from the public Ollama library as written — +/// an entry that does not resolve makes the allowlist silently redirect the +/// user back to the default, or leaves them with a model that `ollama pull` +/// cannot fetch (GH #5055). `gemma4:*` was such an entry: there is no `gemma4` +/// namespace on Ollama. The intended model is Gemma 3n, published as +/// `gemma3n:e4b-it-q8_0`. +const MVP_ALLOWED_CHAT_MODELS: &[&str] = &["gemma3:1b-it-qat", "gemma3n:e4b-it-q8_0"]; const MVP_DEFAULT_CHAT_MODEL: &str = "gemma3:1b-it-qat"; /// Vision models allowed in MVP — only disabled (empty string) since the @@ -233,10 +240,10 @@ mod tests { } #[test] - fn chat_model_allows_requested_ollama_gemma4_q8() { + fn chat_model_allows_requested_ollama_gemma3n_q8() { let mut config = test_config(); - config.local_ai.chat_model_id = "gemma4:e4b-it-q8_0".to_string(); - assert_eq!(effective_chat_model_id(&config), "gemma4:e4b-it-q8_0"); + config.local_ai.chat_model_id = "gemma3n:e4b-it-q8_0".to_string(); + assert_eq!(effective_chat_model_id(&config), "gemma3n:e4b-it-q8_0"); } #[test] @@ -272,8 +279,30 @@ mod tests { config.local_ai.chat_model_id = "gemma3:270m-it-qat".to_string(); assert_eq!(effective_chat_model_id(&config), MVP_DEFAULT_CHAT_MODEL); - config.local_ai.chat_model_id = "gemma4:e4b".to_string(); + // Bare `gemma3n:e4b` is a real Ollama tag but is NOT the allowlisted + // quantization, so it still redirects to the default. + config.local_ai.chat_model_id = "gemma3n:e4b".to_string(); assert_eq!(effective_chat_model_id(&config), MVP_DEFAULT_CHAT_MODEL); + + // The retired `gemma4:*` namespace does not exist on Ollama at all. + config.local_ai.chat_model_id = "gemma4:e4b-it-q8_0".to_string(); + assert_eq!(effective_chat_model_id(&config), MVP_DEFAULT_CHAT_MODEL); + } + + /// GH #5055: every allowlisted chat model must be a real, pullable Ollama + /// id. `gemma4:*` shipped for a while and could never be pulled. + #[test] + fn mvp_chat_allowlist_has_no_unpullable_namespaces() { + for model in MVP_ALLOWED_CHAT_MODELS { + assert!( + !model.starts_with("gemma4:"), + "`{model}` is not a real Ollama model — there is no gemma4 namespace" + ); + assert!( + model.contains(':'), + "`{model}` must be a fully-qualified `:` id" + ); + } } #[test] diff --git a/src/openhuman/inference/presets.rs b/src/openhuman/inference/presets.rs index 0981ec6a53..a322a70d8a 100644 --- a/src/openhuman/inference/presets.rs +++ b/src/openhuman/inference/presets.rs @@ -169,11 +169,19 @@ pub fn all_presets() -> Vec { ModelPreset { tier: ModelTier::Ram16PlusGb, label: "16 GB+", - description: "Best local quality with Gemma 4 on higher-end devices.", - chat_model_id: "gemma4:e4b", - vision_model_id: "gemma4:e4b", + description: "Best local quality with Gemma 3n on higher-end devices.", + // GH #5055: was `gemma4:e4b`, which does not exist on the Ollama + // library — the tier shipped a model no `ollama pull` could fetch. + // Gemma 3n is the real publication of the "e4b" (effective-4B) + // variant, and `e4b-it-q8_0` (9.5 GB) is what `approx_download_gb` + // below was sized against. + chat_model_id: "gemma3n:e4b-it-q8_0", + vision_model_id: "gemma3n:e4b-it-q8_0", embedding_model_id: "nomic-embed-text:latest", - quantization: "qat", + // The other tiers ship QAT builds; this one is q8_0. The field is a + // display label (`effective_quantization`) and does not take part in + // resolving the model tag, but it should still match what is pulled. + quantization: "q8_0", vision_mode: VisionMode::Bundled, supports_screen_summary: true, target_ram_gb: 16, @@ -423,4 +431,36 @@ mod tests { apply_preset_to_config(&mut config, ModelTier::Ram16PlusGb); assert_eq!(vision_mode_for_config(&config), VisionMode::Bundled); } + + /// GH #5055: every preset must name a model that actually exists on the + /// Ollama library. The 16 GB+ tier shipped `gemma4:e4b` for chat and vision + /// — there is no `gemma4` namespace, so the tier offered a model that no + /// `ollama pull` could ever fetch. Guard the whole table, not just that row. + #[test] + fn presets_reference_no_unpullable_gemma4_namespace() { + for preset in all_presets() { + for (field, id) in [ + ("chat", preset.chat_model_id), + ("vision", preset.vision_model_id), + ("embedding", preset.embedding_model_id), + ] { + assert!( + !id.starts_with("gemma4:"), + "preset {:?} {field} model `{id}` is not a real Ollama model \ + — there is no gemma4 namespace", + preset.tier + ); + } + } + } + + /// The 16 GB+ tier's chat model must be the allowlisted Gemma 3n build, so + /// selecting the preset does not immediately get redirected back to the + /// default by `enforce_mvp_chat_allowlist`. + #[test] + fn high_tier_preset_uses_the_allowlisted_gemma3n_build() { + let preset = preset_for_tier(ModelTier::Ram16PlusGb).expect("16 GB+ preset"); + assert_eq!(preset.chat_model_id, "gemma3n:e4b-it-q8_0"); + assert_eq!(preset.vision_model_id, "gemma3n:e4b-it-q8_0"); + } } diff --git a/tests/raw_coverage/inference_local_admin_raw_coverage_e2e.rs b/tests/raw_coverage/inference_local_admin_raw_coverage_e2e.rs index dddaa8fc93..dd39563c09 100644 --- a/tests/raw_coverage/inference_local_admin_raw_coverage_e2e.rs +++ b/tests/raw_coverage/inference_local_admin_raw_coverage_e2e.rs @@ -96,7 +96,7 @@ async fn local_admin_covers_assets_diagnostics_downloads_and_ops_errors() { config.local_ai.runtime_enabled = true; config.local_ai.opt_in_confirmed = true; config.local_ai.base_url = Some(base.clone()); - config.local_ai.chat_model_id = "gemma4:e4b-it-q8_0".to_string(); + config.local_ai.chat_model_id = "gemma3n:e4b-it-q8_0".to_string(); config.local_ai.embedding_model_id = "bge-m3".to_string(); config.local_ai.vision_model_id = "missing-vision".to_string(); config.local_ai.selected_tier = Some("custom".to_string()); @@ -134,7 +134,7 @@ async fn local_admin_covers_assets_diagnostics_downloads_and_ops_errors() { .as_array() .unwrap() .iter() - .any(|issue| issue.as_str().unwrap().contains("gemma4:e4b-it-q8_0"))); + .any(|issue| issue.as_str().unwrap().contains("gemma3n:e4b-it-q8_0"))); let assets = service.assets_status(&config).await.expect("assets"); assert!(assets.ollama_available); @@ -175,7 +175,7 @@ async fn local_admin_covers_assets_diagnostics_downloads_and_ops_errors() { .lock() .expect("models") .iter() - .any(|m| m == "gemma4:e4b-it-q8_0")); + .any(|m| m == "gemma3n:e4b-it-q8_0")); let mut lm_config = config.clone(); lm_config.local_ai.provider = "lmstudio".to_string(); @@ -220,7 +220,7 @@ async fn local_admin_covers_assets_diagnostics_downloads_and_ops_errors() { .await .expect("ops progress") .value; - assert_eq!(ops_progress.chat.id, "gemma4:e4b-it-q8_0"); + assert_eq!(ops_progress.chat.id, "gemma3n:e4b-it-q8_0"); let ops_asset = local_ai_download_asset(&config, "embedding") .await @@ -529,7 +529,7 @@ async fn ollama_show(Json(body): Json) -> impl IntoResponse { .into_response(); } let context = match model { - "gemma4:e4b-it-q8_0" => 1024, + "gemma3n:e4b-it-q8_0" => 1024, "bge-m3" => 8192, _ => 4096, }; @@ -545,7 +545,7 @@ async fn ollama_show(Json(body): Json) -> impl IntoResponse { async fn ollama_pull(State(state): State, Json(body): Json) -> impl IntoResponse { let name = body["name"] .as_str() - .unwrap_or("gemma4:e4b-it-q8_0") + .unwrap_or("gemma3n:e4b-it-q8_0") .to_string(); state.ollama_models.lock().expect("models").push(name); let body = [ diff --git a/tests/raw_coverage/inference_provider_admin_round22_raw_coverage_e2e.rs b/tests/raw_coverage/inference_provider_admin_round22_raw_coverage_e2e.rs index 4d789f6b59..99a69967b1 100644 --- a/tests/raw_coverage/inference_provider_admin_round22_raw_coverage_e2e.rs +++ b/tests/raw_coverage/inference_provider_admin_round22_raw_coverage_e2e.rs @@ -295,7 +295,7 @@ async fn local_admin_covers_diagnostics_errors_assets_status_and_shutdown_with_f config.local_ai.runtime_enabled = true; config.local_ai.opt_in_confirmed = true; config.local_ai.base_url = Some(base.clone()); - config.local_ai.chat_model_id = "gemma4:e4b-it-q8_0".to_string(); + config.local_ai.chat_model_id = "gemma3n:e4b-it-q8_0".to_string(); config.local_ai.embedding_model_id = "all-minilm:latest".to_string(); config.local_ai.selected_tier = Some("custom".to_string()); config.local_ai.preload_embedding_model = true; @@ -325,7 +325,7 @@ async fn local_admin_covers_diagnostics_errors_assets_status_and_shutdown_with_f assert!(issues.iter().any(|issue| issue .as_str() .unwrap() - .contains("Chat model `gemma4:e4b-it-q8_0`"))); + .contains("Chat model `gemma3n:e4b-it-q8_0`"))); assert!(issues.iter().any(|issue| issue .as_str() .unwrap() From 53f3cdd0b0d12baeae663aa114d9352f5d09d02f Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Mon, 27 Jul 2026 20:49:08 +0530 Subject: [PATCH 2/2] fix(inference): accept a valid empty Ollama catalog, sharpen fallback tests (#5055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2 (Codex) — the /api/tags fallback rejected an EMPTY catalog, but `{"models":[]}` is exactly what a fresh Ollama with nothing pulled returns. Treating it as a failure hid a reachable runtime behind the original 404, so availability/bootstrap reported a degraded error and the UI could not offer the model-download action. The real LM Studio signal is an error envelope, not emptiness: it answers unknown paths with `200 {"error": …}` and no models (#5053). `OllamaTagsResponse` now carries an optional `error` field and the fallback branches on that, accepting an empty `models` array as a successful zero-model catalog. The field is `#[serde(default)]` and the type is only ever deserialized, so no construction sites change. Minor (CodeRabbit) — `v1_models_404_reports_the_original_error_when_fallback_also_fails` was not actually distinguishing the two errors: Axum's implicit fallback route also answers 404, so the assertion passed even if the implementation surfaced the /api/tags failure. The mock now returns a distinct 503 for /api/tags and the test asserts the 404 survives AND the 503 does not leak. Splits the old empty-catalog test into the two cases it was conflating: an error envelope is still not a recovery, an empty catalog now is. --- src/openhuman/inference/local/ollama.rs | 8 +++ .../inference/local/service/lm_studio.rs | 28 ++++++-- .../local/service/ollama_admin_tests.rs | 69 ++++++++++++++++--- 3 files changed, 91 insertions(+), 14 deletions(-) diff --git a/src/openhuman/inference/local/ollama.rs b/src/openhuman/inference/local/ollama.rs index 486c48e823..74252033d1 100644 --- a/src/openhuman/inference/local/ollama.rs +++ b/src/openhuman/inference/local/ollama.rs @@ -312,6 +312,14 @@ impl OllamaPullProgress { pub(crate) struct OllamaTagsResponse { #[serde(default)] pub models: Vec, + /// Set when the server answered with an error envelope rather than a + /// catalog. LM Studio replies to unknown paths with `200 {"error": …}` and + /// no `models` (GH #5053), which is indistinguishable from a real catalog + /// by `models` alone — a fresh Ollama with nothing pulled legitimately + /// returns `{"models":[]}`. Callers must branch on this field, not on + /// emptiness. + #[serde(default)] + pub error: Option, } #[derive(Debug, Clone, Deserialize, Serialize)] diff --git a/src/openhuman/inference/local/service/lm_studio.rs b/src/openhuman/inference/local/service/lm_studio.rs index 976931f63e..b1f49ecfcf 100644 --- a/src/openhuman/inference/local/service/lm_studio.rs +++ b/src/openhuman/inference/local/service/lm_studio.rs @@ -217,18 +217,34 @@ impl LocalAiService { } }; - // An empty catalog is not a recovery — fall through to the original - // /v1/models error so the user sees the real problem. LM Studio answers - // unknown paths with `200 {"error": …}` and no models (GH #5053), which - // would otherwise look like a successful discovery of zero models. - if payload.models.is_empty() { + // Reject on an explicit error envelope, NOT on emptiness. LM Studio + // answers unknown paths with `200 {"error": …}` and no models + // (GH #5053) — that is the case that must fall through to the original + // /v1/models error. A fresh Ollama with nothing pulled yet legitimately + // returns `{"models":[]}`, and treating that as a failure hid a + // reachable runtime behind a 404, so the UI could not offer the + // model-download action. + if let Some(error) = payload + .error + .as_deref() + .map(str::trim) + .filter(|e| !e.is_empty()) + { tracing::debug!( target: "local_ai::lm_studio", url = %fallback_url, - "[local_ai:lm_studio] /api/tags fallback returned an empty catalog — not a recovery" + error = %error, + "[local_ai:lm_studio] /api/tags fallback returned an error envelope — not a recovery" ); return None; } + if payload.models.is_empty() { + tracing::info!( + target: "local_ai::lm_studio", + url = %fallback_url, + "[local_ai:lm_studio] /api/tags fallback reached a reachable runtime with an empty catalog — recovering as zero models" + ); + } tracing::info!( target: "local_ai::lm_studio", diff --git a/src/openhuman/inference/local/service/ollama_admin_tests.rs b/src/openhuman/inference/local/service/ollama_admin_tests.rs index eb7bb4e588..0cfd8a786b 100644 --- a/src/openhuman/inference/local/service/ollama_admin_tests.rs +++ b/src/openhuman/inference/local/service/ollama_admin_tests.rs @@ -1325,10 +1325,28 @@ async fn v1_models_404_falls_back_to_ollama_api_tags() { async fn v1_models_404_reports_the_original_error_when_fallback_also_fails() { let _guard = crate::openhuman::inference::inference_test_guard(); - let app = Router::new().route( - "/v1/models", - get(|| async { (axum::http::StatusCode::NOT_FOUND, "404 page not found") }), - ); + // The fallback returns a DISTINCT status (503, not 404). Without that, + // Axum's implicit fallback route also answers 404 and the assertion below + // would pass even if the implementation surfaced the /api/tags failure. + let app = Router::new() + .route( + "/v1/models", + get(|| async { + ( + axum::http::StatusCode::NOT_FOUND, + "404 page not found: /v1/models", + ) + }), + ) + .route( + "/api/tags", + get(|| async { + ( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + "503 fallback unavailable", + ) + }), + ); let base = spawn_mock(app).await; let config = lm_studio_config(&base); let service = LocalAiService::new(&config); @@ -1342,13 +1360,17 @@ async fn v1_models_404_reports_the_original_error_when_fallback_also_fails() { err.contains("404"), "expected the original /v1/models status, got: {err}" ); + assert!( + !err.contains("503"), + "the fallback failure must not replace the original error, got: {err}" + ); } /// LM Studio answers unknown paths with `200 {"error": …}` and no models -/// (GH #5053). An empty fallback catalog must NOT be treated as a recovery, +/// (GH #5053). That ERROR ENVELOPE must not be treated as a recovery, /// otherwise discovery silently "succeeds" with zero models. #[tokio::test] -async fn empty_api_tags_fallback_is_not_treated_as_recovery() { +async fn error_envelope_api_tags_fallback_is_not_treated_as_recovery() { let _guard = crate::openhuman::inference::inference_test_guard(); let app = Router::new() @@ -1356,7 +1378,10 @@ async fn empty_api_tags_fallback_is_not_treated_as_recovery() { "/v1/models", get(|| async { (axum::http::StatusCode::NOT_FOUND, "404 page not found") }), ) - .route("/api/tags", get(|| async { Json(json!({ "models": [] })) })); + .route( + "/api/tags", + get(|| async { Json(json!({ "error": "Unexpected endpoint or method." })) }), + ); let base = spawn_mock(app).await; let config = lm_studio_config(&base); let service = LocalAiService::new(&config); @@ -1364,11 +1389,39 @@ async fn empty_api_tags_fallback_is_not_treated_as_recovery() { let err = service .list_lm_studio_models(&config) .await - .expect_err("an empty fallback catalog is not a recovery"); + .expect_err("an error envelope is not a recovery"); assert!(err.contains("404"), "got: {err}"); } +/// A fresh Ollama with nothing pulled answers `{"models":[]}`. That is a +/// REACHABLE runtime, not a failure: rejecting it hid the server behind the +/// original 404 so the UI could not offer the model-download action. +#[tokio::test] +async fn empty_api_tags_fallback_recovers_as_zero_models() { + let _guard = crate::openhuman::inference::inference_test_guard(); + + let app = Router::new() + .route( + "/v1/models", + get(|| async { (axum::http::StatusCode::NOT_FOUND, "404 page not found") }), + ) + .route("/api/tags", get(|| async { Json(json!({ "models": [] })) })); + let base = spawn_mock(app).await; + let config = lm_studio_config(&base); + let service = LocalAiService::new(&config); + + let models = service + .list_lm_studio_models(&config) + .await + .expect("a reachable runtime with no models is not an error"); + + assert!( + models.is_empty(), + "expected an empty catalog, got {models:?}" + ); +} + /// Any non-404 failure must NOT trigger the fallback: a 500 is a server fault, /// not a wrong-endpoint signal, and probing a second path would mask it. #[tokio::test]