Rust bug fixes & changes (#560)

Part 1 of Rust changes (have part 2 but don't have time to test it now).

This is mostly improving perf by reducing cloning and fixing some bugs +
making code more readable (avoiding early returns).

Author: nenad1002

sdk/rust/docs/api.md

@@ -149,7 +149,7 @@ pub struct Model { /* private fields */ }
 |--------|-----------|-------------|
 | `alias` | `fn alias(&self) -> &str` | Alias shared by all variants. |
 | `id` | `fn id(&self) -> &str` | Unique identifier of the selected variant. |
-| `variants` | `fn variants(&self) -> &[ModelVariant]` | All variants in this model. |
+| `variants` | `fn variants(&self) -> &[Arc<ModelVariant>]` | All variants in this model. |
 | `selected_variant` | `fn selected_variant(&self) -> &ModelVariant` | Currently selected variant. |
 | `select_variant` | `fn select_variant(&self, id: &str) -> Result<(), FoundryLocalError>` | Select a variant by id. |
 | `is_cached` | `async fn is_cached(&self) -> Result<bool, FoundryLocalError>` | Whether the selected variant is cached on disk. |

sdk/rust/src/catalog.rs

@@ -135,7 +135,7 @@ impl Catalog {
         self.update_models().await?;
         let s = self.lock_state()?;
         s.models_by_alias.get(alias).cloned().ok_or_else(|| {
-            let available: Vec<&String> = s.models_by_alias.keys().collect();
+            let available: Vec<&str> = s.models_by_alias.keys().map(|k| k.as_str()).collect();
             FoundryLocalError::ModelOperation {
                 reason: format!("Unknown model alias '{alias}'. Available: {available:?}"),
             }
@@ -152,7 +152,7 @@ impl Catalog {
         self.update_models().await?;
         let s = self.lock_state()?;
         s.variants_by_id.get(id).cloned().ok_or_else(|| {
-            let available: Vec<&String> = s.variants_by_id.keys().collect();
+            let available: Vec<&str> = s.variants_by_id.keys().map(|k| k.as_str()).collect();
             FoundryLocalError::ModelOperation {
                 reason: format!("Unknown variant id '{id}'. Available: {available:?}"),
             }
@@ -216,18 +216,17 @@ impl Catalog {
         for info in infos {
             let id = info.id.clone();
             let alias = info.alias.clone();
-            let variant = ModelVariant::new(
+            let variant = Arc::new(ModelVariant::new(
                 info,
                 Arc::clone(&self.core),
                 Arc::clone(&self.model_load_manager),
                 self.invalidator.clone(),
-            );
-            let variant_arc = Arc::new(variant.clone());
-            id_map.insert(id, variant_arc);
+            ));
+            id_map.insert(id, Arc::clone(&variant));
 
             alias_map_build
-                .entry(alias.clone())
-                .or_insert_with(|| Model::new(alias, Arc::clone(&self.core)))
+                .entry(alias)
+                .or_insert_with_key(|a| Model::new(a.clone(), Arc::clone(&self.core)))
                 .add_variant(variant);
         }
 

sdk/rust/src/configuration.rs

@@ -183,31 +183,24 @@ impl Configuration {
         let mut params = HashMap::new();
         params.insert("AppName".into(), app_name);
 
-        if let Some(v) = config.app_data_dir {
-            params.insert("AppDataDir".into(), v);
-        }
-        if let Some(v) = config.model_cache_dir {
-            params.insert("ModelCacheDir".into(), v);
-        }
-        if let Some(v) = config.logs_dir {
-            params.insert("LogsDir".into(), v);
-        }
-        if let Some(level) = config.log_level {
-            params.insert("LogLevel".into(), level.as_core_str().into());
-        }
-        if let Some(v) = config.web_service_urls {
-            params.insert("WebServiceUrls".into(), v);
-        }
-        if let Some(v) = config.service_endpoint {
-            params.insert("WebServiceExternalUrl".into(), v);
-        }
-        if let Some(v) = config.library_path {
-            params.insert("FoundryLocalCorePath".into(), v);
+        let optional_fields = [
+            ("AppDataDir", config.app_data_dir),
+            ("ModelCacheDir", config.model_cache_dir),
+            ("LogsDir", config.logs_dir),
+            ("LogLevel", config.log_level.map(|l| l.as_core_str().into())),
+            ("WebServiceUrls", config.web_service_urls),
+            ("WebServiceExternalUrl", config.service_endpoint),
+            ("FoundryLocalCorePath", config.library_path),
+        ];
+
+        for (key, value) in optional_fields {
+            if let Some(v) = value {
+                params.insert(key.into(), v);
+            }
         }
+
         if let Some(extra) = config.additional_settings {
-            for (k, v) in extra {
-                params.insert(k, v);
-            }
+            params.extend(extra);
         }
 
         Ok((Self { params }, config.logger))

sdk/rust/src/detail/core_interop.rs

@@ -137,25 +137,42 @@ impl<'a> StreamingCallbackState<'a> {
 
     /// Append raw bytes, decode as much valid UTF-8 as possible, and forward
     /// complete text to the callback. Any trailing incomplete multi-byte
-    /// sequence is kept in the buffer for the next call.
+    /// sequence is kept in the buffer for the next call. Invalid byte
+    /// sequences are skipped to prevent the buffer from growing unboundedly.
     fn push(&mut self, bytes: &[u8]) {
         self.buf.extend_from_slice(bytes);
-        let valid_up_to = match std::str::from_utf8(&self.buf) {
-            Ok(s) => {
-                (self.callback)(s);
-                s.len()
-            }
-            Err(e) => {
-                let n = e.valid_up_to();
-                if n > 0 {
-                    // SAFETY: `valid_up_to` guarantees this prefix is valid UTF-8.
-                    let valid = unsafe { std::str::from_utf8_unchecked(&self.buf[..n]) };
-                    (self.callback)(valid);
+        loop {
+            match std::str::from_utf8(&self.buf) {
+                Ok(s) => {
+                    if !s.is_empty() {
+                        (self.callback)(s);
+                    }
+                    self.buf.clear();
+                    break;
+                }
+                Err(e) => {
+                    let n = e.valid_up_to();
+                    if n > 0 {
+                        // SAFETY: `valid_up_to` guarantees this prefix is valid UTF-8.
+                        let valid = unsafe { std::str::from_utf8_unchecked(&self.buf[..n]) };
+                        (self.callback)(valid);
+                    }
+                    match e.error_len() {
+                        Some(err_len) => {
+                            // Definite invalid sequence — skip past it and
+                            // continue decoding the remainder.
+                            self.buf.drain(..n + err_len);
+                        }
+                        None => {
+                            // Incomplete multi-byte sequence at the end —
+                            // keep it for the next push.
+                            self.buf.drain(..n);
+                            break;
+                        }
+                    }
                 }
-                n
             }
-        };
-        self.buf.drain(..valid_up_to);
+        }
     }
 
     /// Flush any remaining bytes as lossy UTF-8 (called once after the native

sdk/rust/src/detail/model_load_manager.rs

@@ -34,27 +34,27 @@ impl ModelLoadManager {
             let encoded_id = urlencoding::encode(model_id);
             self.http_get(&format!("{base_url}/models/load/{encoded_id}"))
                 .await?;
-            return Ok(());
+        } else {
+            let params = json!({ "Params": { "Model": model_id } });
+            self.core
+                .execute_command_async("load_model".into(), Some(params))
+                .await?;
         }
-        let params = json!({ "Params": { "Model": model_id } });
-        self.core
-            .execute_command_async("load_model".into(), Some(params))
-            .await?;
         Ok(())
     }
 
     /// Unload a previously loaded model.
     pub async fn unload(&self, model_id: &str) -> Result<String> {
         if let Some(base_url) = &self.external_service_url {
             let encoded_id = urlencoding::encode(model_id);
-            return self
-                .http_get(&format!("{base_url}/models/unload/{encoded_id}"))
-                .await;
+            self.http_get(&format!("{base_url}/models/unload/{encoded_id}"))
+                .await
+        } else {
+            let params = json!({ "Params": { "Model": model_id } });
+            self.core
+                .execute_command_async("unload_model".into(), Some(params))
+                .await
         }
-        let params = json!({ "Params": { "Model": model_id } });
-        self.core
-            .execute_command_async("unload_model".into(), Some(params))
-            .await
     }
 
     /// Return the list of currently loaded model identifiers.
@@ -67,11 +67,11 @@ impl ModelLoadManager {
                 .await?
         };
 
-        if raw.trim().is_empty() {
-            return Ok(Vec::new());
-        }
-
-        let ids: Vec<String> = serde_json::from_str(&raw)?;
+        let ids: Vec<String> = if raw.trim().is_empty() {
+            Vec::new()
+        } else {
+            serde_json::from_str(&raw)?
+        };
         Ok(ids)
     }
 

sdk/rust/src/model.rs

@@ -19,7 +19,7 @@ use crate::openai::ChatClient;
 pub struct Model {
     alias: String,
     core: Arc<CoreInterop>,
-    variants: Vec<ModelVariant>,
+    variants: Vec<Arc<ModelVariant>>,
     selected_index: AtomicUsize,
 }
 
@@ -57,7 +57,7 @@ impl Model {
 
     /// Add a variant.  If the new variant is cached and the current selection
     /// is not, the new variant becomes the selected one.
-    pub(crate) fn add_variant(&mut self, variant: ModelVariant) {
+    pub(crate) fn add_variant(&mut self, variant: Arc<ModelVariant>) {
         self.variants.push(variant);
         let new_idx = self.variants.len() - 1;
         let current = self.selected_index.load(Relaxed);
@@ -70,17 +70,21 @@ impl Model {
 
     /// Select a variant by its unique id.
     pub fn select_variant(&self, id: &str) -> Result<()> {
-        if let Some(pos) = self.variants.iter().position(|v| v.id() == id) {
-            self.selected_index.store(pos, Relaxed);
-            return Ok(());
+        match self.variants.iter().position(|v| v.id() == id) {
+            Some(pos) => {
+                self.selected_index.store(pos, Relaxed);
+                Ok(())
+            }
+            None => {
+                let available: Vec<&str> = self.variants.iter().map(|v| v.id()).collect();
+                Err(FoundryLocalError::ModelOperation {
+                    reason: format!(
+                        "Variant '{id}' not found for model '{}'. Available: {available:?}",
+                        self.alias
+                    ),
+                })
+            }
         }
-        let available: Vec<String> = self.variants.iter().map(|v| v.id().to_string()).collect();
-        Err(FoundryLocalError::ModelOperation {
-            reason: format!(
-                "Variant '{id}' not found for model '{}'. Available: {available:?}",
-                self.alias
-            ),
-        })
     }
 
     /// Returns a reference to the currently selected variant.
@@ -89,7 +93,7 @@ impl Model {
     }
 
     /// Returns all variants that belong to this model.
-    pub fn variants(&self) -> &[ModelVariant] {
+    pub fn variants(&self) -> &[Arc<ModelVariant>] {
         &self.variants
     }
 
@@ -169,11 +173,11 @@ impl Model {
 
     /// Create a [`ChatClient`] bound to the selected variant.
     pub fn create_chat_client(&self) -> ChatClient {
-        ChatClient::new(self.id().to_string(), Arc::clone(&self.core))
+        ChatClient::new(self.id(), Arc::clone(&self.core))
     }
 
     /// Create an [`AudioClient`] bound to the selected variant.
     pub fn create_audio_client(&self) -> AudioClient {
-        AudioClient::new(self.id().to_string(), Arc::clone(&self.core))
+        AudioClient::new(self.id(), Arc::clone(&self.core))
     }
 }

sdk/rust/src/model_variant.rs

@@ -143,11 +143,11 @@ impl ModelVariant {
 
     /// Create a [`ChatClient`] bound to this variant.
     pub fn create_chat_client(&self) -> ChatClient {
-        ChatClient::new(self.info.id.clone(), Arc::clone(&self.core))
+        ChatClient::new(&self.info.id, Arc::clone(&self.core))
     }
 
     /// Create an [`AudioClient`] bound to this variant.
     pub fn create_audio_client(&self) -> AudioClient {
-        AudioClient::new(self.info.id.clone(), Arc::clone(&self.core))
+        AudioClient::new(&self.info.id, Arc::clone(&self.core))
     }
 }

sdk/rust/src/openai/audio_client.rs

@@ -116,9 +116,9 @@ pub struct AudioClient {
 }
 
 impl AudioClient {
-    pub(crate) fn new(model_id: String, core: Arc<CoreInterop>) -> Self {
+    pub(crate) fn new(model_id: &str, core: Arc<CoreInterop>) -> Self {
         Self {
-            model_id,
+            model_id: model_id.to_owned(),
             core,
             settings: AudioClientSettings::default(),
         }

sdk/rust/src/openai/chat_client.rs

@@ -132,9 +132,9 @@ pub struct ChatClient {
 }
 
 impl ChatClient {
-    pub(crate) fn new(model_id: String, core: Arc<CoreInterop>) -> Self {
+    pub(crate) fn new(model_id: &str, core: Arc<CoreInterop>) -> Self {
         Self {
-            model_id,
+            model_id: model_id.to_owned(),
             core,
             settings: ChatClientSettings::default(),
         }