mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(memory_tree): route bucket-seal embeddings to local OpenAI-compatible backend (#3959)
This commit is contained in:
@@ -507,6 +507,46 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_embedder_routes_to_lmstudio_local_endpoint() {
|
||||
// #3781 regression at the factory/seal level: a configured local
|
||||
// OpenAI-compatible embeddings backend (LM Studio at localhost:1234,
|
||||
// registered as a `cloud_providers` slug) must drive bucket sealing —
|
||||
// the same way the LLM extractor already resolves the `lmstudio` slug —
|
||||
// and NOT fall through to the managed cloud budget (which 400s with
|
||||
// "Insufficient budget" and fails the seal job unrecoverably).
|
||||
use crate::openhuman::config::schema::cloud_providers::CloudProviderCreds;
|
||||
use crate::openhuman::memory_tree::health::{
|
||||
current_degraded_state, mark_semantic_recall_degraded, FailureCode,
|
||||
};
|
||||
let _guard = degraded_flag_lock();
|
||||
mark_semantic_recall_degraded(FailureCode::EmbeddingsUnconfigured);
|
||||
let (_tmp, mut cfg) = test_config();
|
||||
cfg.memory_tree.embedding_endpoint = None;
|
||||
cfg.memory_tree.embedding_model = None;
|
||||
cfg.embeddings_provider = None; // top-level workload routing: unset
|
||||
cfg.memory.embedding_provider = "lmstudio".to_string();
|
||||
cfg.memory.embedding_model = "bge-m3".to_string();
|
||||
cfg.cloud_providers = vec![CloudProviderCreds {
|
||||
id: "p_lmstudio".to_string(),
|
||||
slug: "lmstudio".to_string(),
|
||||
endpoint: "http://localhost:1234/v1".to_string(),
|
||||
..Default::default()
|
||||
}];
|
||||
let e = build_write_embedder(&cfg)
|
||||
.expect("factory must not error")
|
||||
.expect("lmstudio backend → Some(embedder), must NOT fall through to cloud");
|
||||
assert_eq!(
|
||||
e.name(),
|
||||
"custom",
|
||||
"must route to the local OpenAI-compatible endpoint, not the managed backend"
|
||||
);
|
||||
assert!(
|
||||
!current_degraded_state().semantic_recall,
|
||||
"a usable local provider must clear the degraded flag"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_embedder_routes_to_openai_when_memory_provider_is_openai() {
|
||||
// Same FR-015 routing, read path (`build_embedder_from_config`).
|
||||
|
||||
@@ -54,18 +54,90 @@ impl OpenAiCompatEmbedder {
|
||||
/// honours the `dimensions` param so 3-large complies.
|
||||
pub fn try_from_config(config: &Config) -> Result<Option<Self>> {
|
||||
let provider = config.memory.embedding_provider.trim();
|
||||
let (slug, label): (&str, &'static str) = if provider == "openai" {
|
||||
("openai", "openai")
|
||||
} else if provider == "custom" || provider.starts_with("custom:") {
|
||||
("custom", "custom")
|
||||
} else {
|
||||
// Not an OpenAI-compatible provider — let the caller fall through.
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
let model = config.memory.embedding_model.trim();
|
||||
let api_key = crate::openhuman::embeddings::resolve_api_key(config, provider);
|
||||
let custom_endpoint = provider.strip_prefix("custom:");
|
||||
// Decide which OpenAI-compatible endpoint to route to, if any:
|
||||
// * `openai` → OpenAI's hosted API.
|
||||
// * `custom` / `custom:` → an inline custom endpoint.
|
||||
// * any configured `cloud_providers` slug (e.g. `lmstudio`, `vllm`)
|
||||
// → that entry's endpoint, treated as a custom OpenAI-compatible
|
||||
// server.
|
||||
//
|
||||
// The third case is the #3781 fix. The chat/LLM factory already
|
||||
// resolves these slugs via `config.cloud_providers`
|
||||
// (`make_cloud_provider_by_slug`), so the memory_tree LLM extractor
|
||||
// honours a local `lmstudio` backend. The embedder, however, only knew
|
||||
// `openai`/`custom` — so a local LM Studio embeddings backend
|
||||
// (`[memory] embedding_provider = "lmstudio"`) was silently ignored and
|
||||
// bucket sealing fell through to the managed cloud budget, 400ing with
|
||||
// "Insufficient budget" and failing jobs as unrecoverable. Mirroring the
|
||||
// chat factory's slug resolution here gives sealing/ingest embeddings
|
||||
// the same local-endpoint parity the extractor already has.
|
||||
//
|
||||
// Anything else returns `Ok(None)` so the caller's resolution ladder
|
||||
// continues to the managed cloud default.
|
||||
let (slug, label, custom_endpoint): (&str, &'static str, Option<&str>) =
|
||||
if provider == "openai" {
|
||||
("openai", "openai", None)
|
||||
} else if provider == "custom" || provider.starts_with("custom:") {
|
||||
("custom", "custom", provider.strip_prefix("custom:"))
|
||||
} else {
|
||||
// Bare slug, tolerating a trailing `:model` for symmetry with the
|
||||
// top-level `embeddings_provider = "slug:model"` form.
|
||||
let bare = provider.split(':').next().unwrap_or(provider).trim();
|
||||
// Reserved / managed / native-API slugs are owned by other
|
||||
// branches of the resolution ladder (managed cloud, Voyage,
|
||||
// Cohere, native Ollama, deliberate opt-out) — never the
|
||||
// OpenAI-compatible adapter. Let the caller fall through.
|
||||
if bare.is_empty()
|
||||
|| matches!(
|
||||
bare,
|
||||
"managed" | "cloud" | "openhuman" | "voyage" | "cohere" | "ollama" | "none"
|
||||
)
|
||||
{
|
||||
return Ok(None);
|
||||
}
|
||||
match config
|
||||
.cloud_providers
|
||||
.iter()
|
||||
.find(|e| e.slug == bare)
|
||||
.map(|e| e.endpoint.trim())
|
||||
.filter(|ep| !ep.is_empty())
|
||||
{
|
||||
// A configured OpenAI-compatible provider (LM Studio, vLLM,
|
||||
// text-embeddings-inference, …) → route as `custom` against
|
||||
// its endpoint.
|
||||
Some(endpoint) => ("custom", "custom", Some(endpoint)),
|
||||
// Unknown slug, or one with no endpoint configured — fall
|
||||
// through to the managed cloud default rather than erroring.
|
||||
None => return Ok(None),
|
||||
}
|
||||
};
|
||||
|
||||
// Credential lookup keys on the bare slug (`embeddings:<slug>`); local
|
||||
// servers like LM Studio usually need no key, so an empty result is fine
|
||||
// and matches the existing `custom` behaviour. `resolve_api_key` already
|
||||
// normalises a `custom:<url>` argument down to the `custom` slug.
|
||||
let cred_slug = provider.split(':').next().unwrap_or(provider).trim();
|
||||
let api_key = crate::openhuman::embeddings::resolve_api_key(config, cred_slug);
|
||||
|
||||
// Model: prefer the explicit `embedding_model`; otherwise fall back to an
|
||||
// inline `slug:model` suffix on the provider string. The `custom:<url>`
|
||||
// form is exempt — its suffix is an endpoint URL, not a model name, so
|
||||
// splitting it would mis-route the URL as the model. Leave the model
|
||||
// empty in that case and let the endpoint default apply.
|
||||
let model = {
|
||||
let explicit = config.memory.embedding_model.trim();
|
||||
if !explicit.is_empty() {
|
||||
explicit
|
||||
} else if provider.starts_with("custom:") {
|
||||
""
|
||||
} else {
|
||||
provider
|
||||
.split_once(':')
|
||||
.map(|(_, m)| m.trim())
|
||||
.unwrap_or("")
|
||||
}
|
||||
};
|
||||
|
||||
let inner = crate::openhuman::embeddings::create_embedding_provider_with_credentials(
|
||||
slug,
|
||||
@@ -74,10 +146,15 @@ impl OpenAiCompatEmbedder {
|
||||
&api_key,
|
||||
custom_endpoint,
|
||||
)
|
||||
.with_context(|| format!("build {label} embedder for memory tree"))?;
|
||||
.with_context(|| {
|
||||
format!("build {label} embedder for memory tree (provider='{provider}')")
|
||||
})?;
|
||||
|
||||
log::debug!(
|
||||
"[memory_tree::embed::openai_compat] using {label} provider model={} dims={}",
|
||||
"[memory_tree::embed::openai_compat] using {label} provider (config='{}') \
|
||||
endpoint={:?} model={} dims={}",
|
||||
provider,
|
||||
custom_endpoint,
|
||||
model,
|
||||
EMBEDDING_DIM
|
||||
);
|
||||
@@ -157,4 +234,99 @@ mod tests {
|
||||
let e = got.expect("custom should build an adapter");
|
||||
assert_eq!(e.name(), "custom");
|
||||
}
|
||||
|
||||
/// When `embedding_model` is unset, a `custom:<url>` provider must NOT treat
|
||||
/// the endpoint URL suffix as an inline model name (CodeRabbit #3781). The
|
||||
/// adapter still builds; the model is simply left empty.
|
||||
#[test]
|
||||
fn some_for_custom_endpoint_does_not_use_url_as_model() {
|
||||
let (_tmp, mut cfg) = cfg_with_provider("custom:https://embed.example/v1");
|
||||
cfg.memory.embedding_model = String::new(); // force the inline fallback path
|
||||
let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error");
|
||||
let e = got.expect("custom endpoint with no model should still build");
|
||||
assert_eq!(e.name(), "custom");
|
||||
}
|
||||
|
||||
/// Build a `cloud_providers` entry the way AI Settings persists a local
|
||||
/// OpenAI-compatible server.
|
||||
fn lmstudio_entry(
|
||||
endpoint: &str,
|
||||
) -> crate::openhuman::config::schema::cloud_providers::CloudProviderCreds {
|
||||
crate::openhuman::config::schema::cloud_providers::CloudProviderCreds {
|
||||
id: "p_lmstudio_test".to_string(),
|
||||
slug: "lmstudio".to_string(),
|
||||
endpoint: endpoint.to_string(),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
/// #3781: a configured `lmstudio` slug (OpenAI-compatible, like LM Studio at
|
||||
/// localhost:1234) must resolve to its `cloud_providers` endpoint and route
|
||||
/// as a `custom` OpenAI-compatible embedder — NOT fall through to managed
|
||||
/// cloud. This is the headline bug: sealing ignored the local backend.
|
||||
#[test]
|
||||
fn some_for_configured_lmstudio_slug() {
|
||||
let (_tmp, mut cfg) = cfg_with_provider("lmstudio");
|
||||
cfg.memory.embedding_model = "bge-m3".to_string();
|
||||
cfg.cloud_providers = vec![lmstudio_entry("http://localhost:1234/v1")];
|
||||
|
||||
let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error");
|
||||
let e = got.expect("configured lmstudio slug must build an adapter, not fall through");
|
||||
assert_eq!(e.name(), "custom");
|
||||
}
|
||||
|
||||
/// The `slug:model` form (mirroring the top-level
|
||||
/// `embeddings_provider = "lmstudio:bge-m3"` shape) also resolves, taking the
|
||||
/// model from the inline suffix when `embedding_model` is unset.
|
||||
#[test]
|
||||
fn some_for_lmstudio_slug_with_inline_model() {
|
||||
let (_tmp, mut cfg) = cfg_with_provider("lmstudio:bge-m3");
|
||||
cfg.memory.embedding_model = String::new(); // force inline-suffix fallback
|
||||
cfg.cloud_providers = vec![lmstudio_entry("http://localhost:1234/v1")];
|
||||
|
||||
let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error");
|
||||
let e = got.expect("lmstudio:model slug must resolve");
|
||||
assert_eq!(e.name(), "custom");
|
||||
}
|
||||
|
||||
/// A custom slug with no matching `cloud_providers` entry must fall through
|
||||
/// (Ok(None)) so the caller's ladder continues to the managed default —
|
||||
/// rather than erroring or hijacking the resolution.
|
||||
#[test]
|
||||
fn none_for_unconfigured_custom_slug() {
|
||||
let (_tmp, cfg) = cfg_with_provider("lmstudio"); // no cloud_providers entry
|
||||
let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error");
|
||||
assert!(
|
||||
got.is_none(),
|
||||
"unconfigured slug should fall through, not build an adapter"
|
||||
);
|
||||
}
|
||||
|
||||
/// An entry that exists but has a blank endpoint is unusable → fall through.
|
||||
#[test]
|
||||
fn none_for_configured_slug_with_blank_endpoint() {
|
||||
let (_tmp, mut cfg) = cfg_with_provider("lmstudio");
|
||||
cfg.cloud_providers = vec![lmstudio_entry(" ")];
|
||||
let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error");
|
||||
assert!(got.is_none(), "blank endpoint should fall through");
|
||||
}
|
||||
|
||||
/// Reserved/managed/native slugs must keep falling through even if a stray
|
||||
/// `cloud_providers` entry exists for them — they are owned by other ladder
|
||||
/// branches, not the OpenAI-compatible adapter.
|
||||
#[test]
|
||||
fn reserved_slugs_still_fall_through() {
|
||||
use crate::openhuman::config::schema::cloud_providers::CloudProviderCreds;
|
||||
for p in ["managed", "cloud", "voyage", "cohere", "ollama", "none"] {
|
||||
let (_tmp, mut cfg) = cfg_with_provider(p);
|
||||
cfg.cloud_providers = vec![CloudProviderCreds {
|
||||
id: format!("p_{p}"),
|
||||
slug: p.to_string(),
|
||||
endpoint: "http://localhost:1234/v1".to_string(),
|
||||
..Default::default()
|
||||
}];
|
||||
let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error");
|
||||
assert!(got.is_none(), "{p} must fall through, got Some");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user