Files
optimclaw/src/config/embeddings.rs
T
097a26ace6 fix: harden openai-compatible provider, approval replay, and embeddings defaults (#237)
* fix: harden openai-compatible tool flow and local defaults

* fix: close approval replay gaps and harden openai-compatible flow

* fix: address review feedback and code improvements (takeover #112)

- Make ChatCompletionResponse.id Optional<String> to handle providers
  that omit or null the field
- Propagate HTTP client builder errors instead of silently dropping
  timeout configuration (openai_compatible_chat, nearai_chat)
- Add EMBEDDING_DIMENSION env var with smart per-model defaults instead
  of hardcoding 768/1536 everywhere
- Remove duplicated dimension inference logic from main.rs

Co-Authored-By: panosAthDBX <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: harden src/llm/ module from crate audit findings

- Replace 9x .expect() on RwLock with graceful poison recovery
  (nearai.rs: 7, nearai_chat.rs: 2) — eliminates production panics
- Propagate HTTP client builder errors in nearai.rs instead of
  silently dropping timeout config (NearAiProvider::new now returns Result)
- Make nearai_chat ChatCompletionResponse.id Optional<String>
  (mirrors openai_compatible_chat.rs fix for providers that omit id)
- Make nearai_chat usage fields optional with defensive parse_usage()
  helper (was required u32 fields that crash on null/missing)
- Truncate error responses to 512 chars in nearai_chat.rs error
  messages to prevent log bloat and potential data leakage
- Delegate 4 missing LlmProvider methods in FailoverProvider
  (model_metadata, seed_response_chain, get_response_chain_id,
  calculate_cost) to last-used provider instead of trait defaults

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(llm): add RetryProvider, remove openai_compatible_chat, harden decorators

- Add composable RetryProvider decorator wrapping any LlmProvider with
  exponential backoff + jitter, respecting RateLimited retry_after hints
- Remove openai_compatible_chat.rs — replaced by rig adapter + RetryProvider
- Remove internal retry loop from nearai.rs (was causing double-retry
  with external RetryProvider, up to 16 attempts instead of 4)
- Remove internal retry loop from nearai_chat.rs (same issue)
- Wire RetryProvider into main.rs composition chain: each provider gets
  its own retry wrapper before failover
- Move normalize_tool_name to rig_adapter.rs for all rig-based providers
- Reconcile is_retryable() vs is_transient() error classification:
  ModelNotAvailable no longer retryable, Json no longer transient
- Fix unchecked Duration subtraction panic in circuit_breaker.rs
- Make failover.rs use shared is_retryable() from retry.rs
- Remove stale #[allow(dead_code)] on NearAiResponse::id (field is used)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review feedback — error handling, dimension validation, libSQL warning

- Replace response.text().await.unwrap_or_default() with proper error
  propagation in nearai.rs and nearai_chat.rs (4 call sites). Failures
  now return LlmError::RequestFailed with context instead of silently
  proceeding with an empty string.
- Add embedding dimension validation in OllamaEmbeddings::embed_batch():
  returns EmbeddingError if Ollama returns embeddings with a dimension
  that doesn't match the configured value.
- Add runtime warning when libSQL backend is used with non-1536 embedding
  dimension, since the libSQL schema uses F32_BLOB(1536) and cannot store
  different-dimension vectors.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: panosAthDbx <[email protected]>
Co-authored-by: panosAthDBX <[email protected]>
Co-authored-by: panosAthDBX <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Copilot <[email protected]>
2026-02-19 23:05:04 +00:00

204 lines
6.4 KiB
Rust

use secrecy::{ExposeSecret, SecretString};
use crate::config::helpers::optional_env;
use crate::error::ConfigError;
use crate::settings::Settings;
/// Embeddings provider configuration.
#[derive(Debug, Clone)]
pub struct EmbeddingsConfig {
/// Whether embeddings are enabled.
pub enabled: bool,
/// Provider to use: "openai", "nearai", or "ollama"
pub provider: String,
/// OpenAI API key (for OpenAI provider).
pub openai_api_key: Option<SecretString>,
/// Model to use for embeddings.
pub model: String,
/// Ollama base URL (for Ollama provider). Defaults to http://localhost:11434.
pub ollama_base_url: String,
/// Embedding vector dimension. Inferred from the model name when not set explicitly.
pub dimension: usize,
}
impl Default for EmbeddingsConfig {
fn default() -> Self {
let model = "text-embedding-3-small".to_string();
let dimension = default_dimension_for_model(&model);
Self {
enabled: false,
provider: "openai".to_string(),
openai_api_key: None,
model,
ollama_base_url: "http://localhost:11434".to_string(),
dimension,
}
}
}
/// Infer the embedding dimension from a well-known model name.
///
/// Falls back to 1536 (OpenAI text-embedding-3-small default) for unknown models.
fn default_dimension_for_model(model: &str) -> usize {
match model {
"text-embedding-3-small" => 1536,
"text-embedding-3-large" => 3072,
"text-embedding-ada-002" => 1536,
"nomic-embed-text" => 768,
"mxbai-embed-large" => 1024,
"all-minilm" => 384,
_ => 1536,
}
}
impl EmbeddingsConfig {
pub(crate) fn resolve(settings: &Settings) -> Result<Self, ConfigError> {
let openai_api_key = optional_env("OPENAI_API_KEY")?.map(SecretString::from);
let provider = optional_env("EMBEDDING_PROVIDER")?
.unwrap_or_else(|| settings.embeddings.provider.clone());
let model =
optional_env("EMBEDDING_MODEL")?.unwrap_or_else(|| settings.embeddings.model.clone());
let ollama_base_url = optional_env("OLLAMA_BASE_URL")?
.or_else(|| settings.ollama_base_url.clone())
.unwrap_or_else(|| "http://localhost:11434".to_string());
let dimension = optional_env("EMBEDDING_DIMENSION")?
.map(|s| s.parse::<usize>())
.transpose()
.map_err(|e| ConfigError::InvalidValue {
key: "EMBEDDING_DIMENSION".to_string(),
message: format!("must be a positive integer: {e}"),
})?
.unwrap_or_else(|| default_dimension_for_model(&model));
let enabled = optional_env("EMBEDDING_ENABLED")?
.map(|s| s.parse())
.transpose()
.map_err(|e| ConfigError::InvalidValue {
key: "EMBEDDING_ENABLED".to_string(),
message: format!("must be 'true' or 'false': {e}"),
})?
.unwrap_or(settings.embeddings.enabled);
Ok(Self {
enabled,
provider,
openai_api_key,
model,
ollama_base_url,
dimension,
})
}
/// Get the OpenAI API key if configured.
pub fn openai_api_key(&self) -> Option<&str> {
self.openai_api_key.as_ref().map(|s| s.expose_secret())
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::settings::{EmbeddingsSettings, Settings};
use std::sync::Mutex;
/// Serializes env-mutating tests to prevent parallel races.
static ENV_MUTEX: Mutex<()> = Mutex::new(());
/// Clear all embedding-related env vars.
fn clear_embedding_env() {
// SAFETY: Only called under ENV_MUTEX in tests. No other threads
// observe these vars while the lock is held.
unsafe {
std::env::remove_var("EMBEDDING_ENABLED");
std::env::remove_var("EMBEDDING_PROVIDER");
std::env::remove_var("EMBEDDING_MODEL");
std::env::remove_var("OPENAI_API_KEY");
}
}
#[test]
fn embeddings_disabled_not_overridden_by_openai_key() {
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
clear_embedding_env();
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
std::env::set_var("OPENAI_API_KEY", "sk-test-key-for-issue-129");
}
let settings = Settings {
embeddings: EmbeddingsSettings {
enabled: false,
..Default::default()
},
..Default::default()
};
let config = EmbeddingsConfig::resolve(&settings).expect("resolve should succeed");
assert!(
!config.enabled,
"embeddings should remain disabled when settings.embeddings.enabled=false, \
even when OPENAI_API_KEY is set (issue #129)"
);
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("OPENAI_API_KEY");
}
}
#[test]
fn embeddings_enabled_from_settings() {
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
clear_embedding_env();
let settings = Settings {
embeddings: EmbeddingsSettings {
enabled: true,
..Default::default()
},
..Default::default()
};
let config = EmbeddingsConfig::resolve(&settings).expect("resolve should succeed");
assert!(
config.enabled,
"embeddings should be enabled when settings say so"
);
}
#[test]
fn embeddings_env_override_takes_precedence() {
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
clear_embedding_env();
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::set_var("EMBEDDING_ENABLED", "true");
}
let settings = Settings {
embeddings: EmbeddingsSettings {
enabled: false,
..Default::default()
},
..Default::default()
};
let config = EmbeddingsConfig::resolve(&settings).expect("resolve should succeed");
assert!(
config.enabled,
"EMBEDDING_ENABLED=true env var should override settings"
);
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("EMBEDDING_ENABLED");
}
}
}