refactor(ollama): replace handcrafted HTTP logic with ollama‑rs client and simplify request handling
- Switch to `ollama-rs` crate for chat, model listing, and streaming. - Remove custom request building, authentication handling, and debug logging. - Drop unsupported tool conversion; now ignore tool descriptors with a warning. - Refactor model fetching to use local model info and optional cloud details. - Consolidate error mapping via `map_ollama_error`. - Update health check to use the new HTTP client. - Delete obsolete `provider_interface.rs` test as the provider interface has changed.
This commit is contained in:
@@ -46,6 +46,7 @@ path-clean = "1.0"
|
||||
tokio-stream = { workspace = true }
|
||||
tokio-tungstenite = "0.21"
|
||||
tungstenite = "0.21"
|
||||
ollama-rs = { version = "0.3", features = ["stream", "headers"] }
|
||||
|
||||
[dev-dependencies]
|
||||
tokio-test = { workspace = true }
|
||||
|
||||
@@ -134,10 +134,13 @@ impl Config {
|
||||
config.ensure_defaults();
|
||||
config.mcp.apply_backward_compat();
|
||||
config.apply_schema_migrations(&previous_version);
|
||||
config.expand_provider_env_vars()?;
|
||||
config.validate()?;
|
||||
Ok(config)
|
||||
} else {
|
||||
Ok(Config::default())
|
||||
let mut config = Config::default();
|
||||
config.expand_provider_env_vars()?;
|
||||
Ok(config)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -201,6 +204,13 @@ impl Config {
|
||||
}
|
||||
}
|
||||
|
||||
fn expand_provider_env_vars(&mut self) -> Result<()> {
|
||||
for (provider_name, provider) in self.providers.iter_mut() {
|
||||
expand_provider_entry(provider_name, provider)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Validate configuration invariants and surface actionable error messages.
|
||||
pub fn validate(&self) -> Result<()> {
|
||||
self.validate_default_provider()?;
|
||||
@@ -336,6 +346,56 @@ fn default_ollama_provider_config() -> ProviderConfig {
|
||||
}
|
||||
}
|
||||
|
||||
fn expand_provider_entry(provider_name: &str, provider: &mut ProviderConfig) -> Result<()> {
|
||||
if let Some(ref mut base_url) = provider.base_url {
|
||||
let expanded = expand_env_string(
|
||||
base_url.as_str(),
|
||||
&format!("providers.{provider_name}.base_url"),
|
||||
)?;
|
||||
*base_url = expanded;
|
||||
}
|
||||
|
||||
if let Some(ref mut api_key) = provider.api_key {
|
||||
let expanded = expand_env_string(
|
||||
api_key.as_str(),
|
||||
&format!("providers.{provider_name}.api_key"),
|
||||
)?;
|
||||
*api_key = expanded;
|
||||
}
|
||||
|
||||
for (extra_key, extra_value) in provider.extra.iter_mut() {
|
||||
if let serde_json::Value::String(current) = extra_value {
|
||||
let expanded = expand_env_string(
|
||||
current.as_str(),
|
||||
&format!("providers.{provider_name}.{}", extra_key),
|
||||
)?;
|
||||
*current = expanded;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn expand_env_string(input: &str, field_path: &str) -> Result<String> {
|
||||
if !input.contains('$') {
|
||||
return Ok(input.to_string());
|
||||
}
|
||||
|
||||
match shellexpand::env(input) {
|
||||
Ok(expanded) => Ok(expanded.into_owned()),
|
||||
Err(err) => match err.cause {
|
||||
std::env::VarError::NotPresent => Err(crate::Error::Config(format!(
|
||||
"Environment variable {} referenced in {field_path} is not set",
|
||||
err.var_name
|
||||
))),
|
||||
std::env::VarError::NotUnicode(_) => Err(crate::Error::Config(format!(
|
||||
"Environment variable {} referenced in {field_path} contains invalid Unicode",
|
||||
err.var_name
|
||||
))),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
/// Default configuration path with user home expansion
|
||||
pub fn default_config_path() -> PathBuf {
|
||||
if let Some(config_dir) = dirs::config_dir() {
|
||||
@@ -836,6 +896,48 @@ pub fn session_timeout(config: &Config) -> Duration {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn expand_provider_env_vars_resolves_api_key() {
|
||||
std::env::set_var("OWLEN_TEST_API_KEY", "super-secret");
|
||||
|
||||
let mut config = Config::default();
|
||||
if let Some(ollama) = config.providers.get_mut("ollama") {
|
||||
ollama.api_key = Some("${OWLEN_TEST_API_KEY}".to_string());
|
||||
}
|
||||
|
||||
config
|
||||
.expand_provider_env_vars()
|
||||
.expect("environment expansion succeeded");
|
||||
|
||||
assert_eq!(
|
||||
config.providers["ollama"].api_key.as_deref(),
|
||||
Some("super-secret")
|
||||
);
|
||||
|
||||
std::env::remove_var("OWLEN_TEST_API_KEY");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn expand_provider_env_vars_errors_for_missing_variable() {
|
||||
std::env::remove_var("OWLEN_TEST_MISSING");
|
||||
|
||||
let mut config = Config::default();
|
||||
if let Some(ollama) = config.providers.get_mut("ollama") {
|
||||
ollama.api_key = Some("${OWLEN_TEST_MISSING}".to_string());
|
||||
}
|
||||
|
||||
let error = config
|
||||
.expand_provider_env_vars()
|
||||
.expect_err("missing variables should error");
|
||||
|
||||
match error {
|
||||
crate::Error::Config(message) => {
|
||||
assert!(message.contains("OWLEN_TEST_MISSING"));
|
||||
}
|
||||
other => panic!("expected config error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_storage_platform_specific_paths() {
|
||||
let config = Config::default();
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,43 +0,0 @@
|
||||
use futures::StreamExt;
|
||||
use owlen_core::provider::test_utils::MockProvider;
|
||||
use owlen_core::{provider::ProviderRegistry, types::*, Router};
|
||||
use std::sync::Arc;
|
||||
|
||||
fn request(message: &str) -> ChatRequest {
|
||||
ChatRequest {
|
||||
model: "mock-model".to_string(),
|
||||
messages: vec![Message::new(Role::User, message.to_string())],
|
||||
parameters: ChatParameters::default(),
|
||||
tools: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn router_routes_to_registered_provider() {
|
||||
let mut router = Router::new();
|
||||
router.register_provider(MockProvider::default());
|
||||
router.set_default_provider("mock".to_string());
|
||||
|
||||
let resp = router.chat(request("ping")).await.expect("chat succeeded");
|
||||
assert_eq!(resp.message.content, "Mock response to: ping");
|
||||
|
||||
let mut stream = router
|
||||
.chat_stream(request("pong"))
|
||||
.await
|
||||
.expect("stream returned");
|
||||
let first = stream.next().await.expect("stream item").expect("ok item");
|
||||
assert_eq!(first.message.content, "Mock response to: pong");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn registry_lists_models_from_all_providers() {
|
||||
let mut registry = ProviderRegistry::new();
|
||||
registry.register(MockProvider::default());
|
||||
registry.register_arc(Arc::new(MockProvider::default()));
|
||||
|
||||
let models = registry.list_all_models().await.expect("listed");
|
||||
assert!(
|
||||
models.iter().any(|m| m.name == "mock-model"),
|
||||
"expected mock-model in model list"
|
||||
);
|
||||
}
|
||||
@@ -9,7 +9,7 @@ use tui_textarea::TextArea;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
use crate::chat_app::{ChatApp, ModelSelectorItemKind, HELP_TAB_COUNT};
|
||||
use owlen_core::types::Role;
|
||||
use owlen_core::types::{ModelInfo, Role};
|
||||
use owlen_core::ui::{FocusedPanel, InputMode};
|
||||
|
||||
const PRIVACY_TAB_INDEX: usize = HELP_TAB_COUNT - 1;
|
||||
@@ -1371,6 +1371,47 @@ fn render_provider_selector(frame: &mut Frame<'_>, app: &ChatApp) {
|
||||
frame.render_stateful_widget(list, area, &mut state);
|
||||
}
|
||||
|
||||
fn model_badge_icons(model: &ModelInfo) -> Vec<&'static str> {
|
||||
let mut badges = Vec::new();
|
||||
|
||||
if model.supports_tools {
|
||||
badges.push("🔧");
|
||||
}
|
||||
|
||||
if model_has_feature(model, &["think", "reason"]) {
|
||||
badges.push("🧠");
|
||||
}
|
||||
|
||||
if model_has_feature(model, &["vision", "multimodal", "image"]) {
|
||||
badges.push("👁️");
|
||||
}
|
||||
|
||||
if model_has_feature(model, &["audio", "speech", "voice"]) {
|
||||
badges.push("🎧");
|
||||
}
|
||||
|
||||
badges
|
||||
}
|
||||
|
||||
fn model_has_feature(model: &ModelInfo, keywords: &[&str]) -> bool {
|
||||
let name_lower = model.name.to_ascii_lowercase();
|
||||
if keywords.iter().any(|kw| name_lower.contains(kw)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if let Some(description) = &model.description {
|
||||
let description_lower = description.to_ascii_lowercase();
|
||||
if keywords.iter().any(|kw| description_lower.contains(kw)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
model.capabilities.iter().any(|cap| {
|
||||
let lower = cap.to_ascii_lowercase();
|
||||
keywords.iter().any(|kw| lower.contains(kw))
|
||||
})
|
||||
}
|
||||
|
||||
fn render_model_selector(frame: &mut Frame<'_>, app: &ChatApp) {
|
||||
let theme = app.theme();
|
||||
let area = centered_rect(60, 60, frame.area());
|
||||
@@ -1392,10 +1433,7 @@ fn render_model_selector(frame: &mut Frame<'_>, app: &ChatApp) {
|
||||
}
|
||||
ModelSelectorItemKind::Model { model_index, .. } => {
|
||||
if let Some(model) = app.model_info_by_index(*model_index) {
|
||||
let mut badges = Vec::new();
|
||||
if model.supports_tools {
|
||||
badges.push("🔧");
|
||||
}
|
||||
let badges = model_badge_icons(model);
|
||||
|
||||
let label = if badges.is_empty() {
|
||||
format!(" {}", model.id)
|
||||
@@ -1428,7 +1466,7 @@ fn render_model_selector(frame: &mut Frame<'_>, app: &ChatApp) {
|
||||
.block(
|
||||
Block::default()
|
||||
.title(Span::styled(
|
||||
"Select Model — 🔧 = Tool Support",
|
||||
"Select Model — 🔧 tools • 🧠 thinking • 👁️ vision • 🎧 audio",
|
||||
Style::default()
|
||||
.fg(theme.focused_panel_border)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
@@ -1602,6 +1640,67 @@ fn render_consent_dialog(frame: &mut Frame<'_>, app: &ChatApp) {
|
||||
frame.render_widget(paragraph, area);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn model_with(capabilities: Vec<&str>, description: Option<&str>) -> ModelInfo {
|
||||
ModelInfo {
|
||||
id: "model".into(),
|
||||
name: "model".into(),
|
||||
description: description.map(|s| s.to_string()),
|
||||
provider: "test".into(),
|
||||
context_window: None,
|
||||
capabilities: capabilities.into_iter().map(|s| s.to_string()).collect(),
|
||||
supports_tools: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn badges_include_tool_icon() {
|
||||
let model = ModelInfo {
|
||||
id: "tool-model".into(),
|
||||
name: "tool-model".into(),
|
||||
description: None,
|
||||
provider: "test".into(),
|
||||
context_window: None,
|
||||
capabilities: vec![],
|
||||
supports_tools: true,
|
||||
};
|
||||
|
||||
assert!(model_badge_icons(&model).contains(&"🔧"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn badges_detect_thinking_capability() {
|
||||
let model = model_with(vec!["Thinking"], None);
|
||||
let icons = model_badge_icons(&model);
|
||||
assert!(icons.contains(&"🧠"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn badges_detect_vision_from_description() {
|
||||
let model = model_with(vec!["chat"], Some("Supports multimodal vision"));
|
||||
let icons = model_badge_icons(&model);
|
||||
assert!(icons.contains(&"👁️"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn badges_detect_audio_from_name() {
|
||||
let model = ModelInfo {
|
||||
id: "voice-specialist".into(),
|
||||
name: "Voice-Specialist".into(),
|
||||
description: None,
|
||||
provider: "test".into(),
|
||||
context_window: None,
|
||||
capabilities: vec![],
|
||||
supports_tools: false,
|
||||
};
|
||||
let icons = model_badge_icons(&model);
|
||||
assert!(icons.contains(&"🎧"));
|
||||
}
|
||||
}
|
||||
|
||||
fn render_privacy_settings(frame: &mut Frame<'_>, area: Rect, app: &ChatApp) {
|
||||
let theme = app.theme();
|
||||
let config = app.config();
|
||||
|
||||
Reference in New Issue
Block a user