Repository: tensorzero/tensorzero (language: rust)

Issue:
Reject all entries starting with `tensorzero::` during config validation
We should have a reserved namespace for things like synthetic evals, functions that are built-ins, variants that are built-ins, etc. We might also want to reject "none" for these (since "" is reserved in selectors in UI we need some value that's nullish but not the empty string)

---

* Functions
* Variants
* Models
* Providers
* Tools
* Metrics
* Embedding models
* Tag keys for inferences and feedback


Discussion / hints:
* Functions
* Variants
* Models
* Providers
* Tools
* Metrics
* Embedding Models

Anything else?
> We might also want to reject "none" for these

`tensorzero::null` / `tensorzero::none`?
> > We might also want to reject "none" for these
> 
> `tensorzero::null` / `tensorzero::none`?

Nah we want "none" for display reasons, i.e. in the metric selection UI. "tensorzero::none" doesn't accomplish the same thing. See #598 in `MetricSelector.tsx` for an example.
@virajmehta `<SelectItem value="tensorzero::none">None</SelectItem>`?
The namespace that matters in UI is display names

On Wed, Dec 25 2024 at 11:14 AM, Gabriel Bianconi < ***@***.*** > wrote:

> 
> 
> 
> 
> 
> 
> 
> @virajmehta ( https://github.com/virajmehta ) <SelectItem
> value="tensorzero::none">None</SelectItem> ?
> 
> 
> 
> —
> Reply to this email directly, view it on GitHub (
> https://github.com/tensorzero/tensorzero/issues/597#issuecomment-2561925328
> ) , or unsubscribe (
> https://github.com/notifications/unsubscribe-auth/ACGQ6BBNKZ6J7SJNU5LQZVT2HLDXDAVCNFSM6AAAAABUFLSRBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRRHEZDKMZSHA
> ).
> You are receiving this because you were mentioned. Message ID: <tensorzero/tensorzero/issues/597/2561925328
> @ github. com>
> 
> 
>
Ah you mean someone calling a metric "None" which would show up in the UI as a duplicate?

I think we can avoid that by displaying metric names (and other values) as monospace font, i.e. "None" vs "`None` (badge) (badge)". Which we probably should regardless of this point.

> * Functions
> * Variants
> * Models
> * Providers
> * Tools
> * Metrics
> * Embedding Models
> 
> Anything else?

tag keys across the board

Fix patch:
diff --git a/tensorzero_internal/src/config_parser.rs b/tensorzero_internal/src/config_parser.rs
index 20d79a0b6..1ed3ecda0 100644
--- a/tensorzero_internal/src/config_parser.rs
+++ b/tensorzero_internal/src/config_parser.rs
@@ -146,6 +146,14 @@ impl<'c> Config<'c> {
     fn validate(&mut self) -> Result<(), Error> {
         // Validate each function
         for (function_name, function) in &self.functions {
+            if function_name.starts_with("tensorzero::") {
+                return Err(ErrorDetails::Config {
+                    message: format!(
+                        "Function name cannot start with 'tensorzero::': {function_name}"
+                    ),
+                }
+                .into());
+            }
             function.validate(
                 &self.tools,
                 &mut self.models, // NOTE: in here there might be some models created using shorthand initialization
@@ -166,50 +174,43 @@ impl<'c> Config<'c> {
                 }
                 .into());
             }
+            if metric_name.starts_with("tensorzero::") {
+                return Err(ErrorDetails::Config {
+                    message: format!("Metric name cannot start with 'tensorzero::': {metric_name}"),
+                }
+                .into());
+            }
         }
 
         // Validate each model
         for (model_name, model) in self.models.iter() {
-            // Ensure that the model has at least one provider
-            if model.routing.is_empty() {
+            if model_name.starts_with("tensorzero::") {
                 return Err(ErrorDetails::Config {
-                    message: format!("`models.{model_name}`: `routing` must not be empty"),
+                    message: format!("Model name cannot start with 'tensorzero::': {model_name}"),
                 }
                 .into());
             }
+            model.validate(model_name)?;
+        }
 
-            // Ensure that routing entries are unique and exist as keys in providers
-            let mut seen_providers = std::collections::HashSet::new();
-            for provider in &model.routing {
-                if !seen_providers.insert(provider) {
-                    return Err(ErrorDetails::Config {
-                        message: format!(
-                            "`models.{model_name}.routing`: duplicate entry `{provider}`"
-                        ),
-                    }
-                    .into());
-                }
-
-                if !model.providers.contains_key(provider) {
-                    return Err(ErrorDetails::Config {
-                message: format!(
-                    "`models.{model_name}`: `routing` contains entry `{provider}` that does not exist in `providers`"
-                ),
-            }
-            .into());
+        for embedding_model_name in self.embedding_models.keys() {
+            if embedding_model_name.starts_with("tensorzero::") {
+                return Err(ErrorDetails::Config {
+                    message: format!(
+                        "Embedding model name cannot start with 'tensorzero::': {embedding_model_name}"
+                    ),
                 }
+                .into());
             }
+        }
 
-            // Validate each provider
-            for provider_name in model.providers.keys() {
-                if !seen_providers.contains(provider_name) {
-                    return Err(ErrorDetails::Config {
-                        message: format!(
-                    "`models.{model_name}`: Provider `{provider_name}` is not listed in `routing`"
-                ),
-                    }
-                    .into());
+        // Validate each tool
+        for tool_name in self.tools.keys() {
+            if tool_name.starts_with("tensorzero::") {
+                return Err(ErrorDetails::Config {
+                    message: format!("Tool name cannot start with 'tensorzero::': {tool_name}"),
                 }
+                .into());
             }
         }
         Ok(())
@@ -979,13 +980,8 @@ mod tests {
         config["models"]["gpt-3.5-turbo"]["routing"] =
             toml::Value::Array(vec!["openai".into(), "openai".into()]);
         let result = Config::load_from_toml(config, PathBuf::new());
-        assert_eq!(
-            result.unwrap_err(),
-            ErrorDetails::Config {
-                message: "`models.gpt-3.5-turbo.routing`: duplicate entry `openai`".to_string()
-            }
-            .into()
-        );
+        let error = result.unwrap_err().to_string();
+        assert!(error.contains("`models.gpt-3.5-turbo.routing`: duplicate entry `openai`"));
     }
 
     /// Ensure that the config validation fails when a routing entry does not exist in providers
@@ -994,13 +990,7 @@ mod tests {
         let mut config = get_sample_valid_config();
         config["models"]["gpt-3.5-turbo"]["routing"] = toml::Value::Array(vec!["closedai".into()]);
         let result = Config::load_from_toml(config, PathBuf::new());
-        assert_eq!(
-            result.unwrap_err(),
-            ErrorDetails::Config {
-                message: "`models.gpt-3.5-turbo`: `routing` contains entry `closedai` that does not exist in `providers`".to_string()
-            }
-            .into()
-        );
+        assert!(result.unwrap_err().to_string().contains("`models.gpt-3.5-turbo`: `routing` contains entry `closedai` that does not exist in `providers`"));
     }
 
     /// Ensure that the config loading fails when the system schema does not exist
@@ -1273,6 +1263,212 @@ mod tests {
         );
     }
 
+    /// Ensure that the config validation fails when a function name starts with `tensorzero::`
+    #[test]
+    fn test_config_validate_function_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // Rename an existing function to start with `tensorzero::`
+        let old_function_entry = config["functions"]
+            .as_table_mut()
+            .unwrap()
+            .remove("generate_draft")
+            .expect("Did not find function `generate_draft`");
+        config["functions"]
+            .as_table_mut()
+            .unwrap()
+            .insert("tensorzero::bad_function".to_string(), old_function_entry);
+
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+        assert_eq!(
+            result.unwrap_err(),
+            Error::new(ErrorDetails::Config {
+                message: "Function name cannot start with 'tensorzero::': tensorzero::bad_function"
+                    .to_string()
+            })
+        );
+    }
+
+    /// Ensure that the config validation fails when a metric name starts with `tensorzero::`
+    #[test]
+    fn test_config_validate_metric_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // Rename an existing metric to start with `tensorzero::`
+        let old_metric_entry = config["metrics"]
+            .as_table_mut()
+            .unwrap()
+            .remove("task_success")
+            .expect("Did not find metric `task_success`");
+        config["metrics"]
+            .as_table_mut()
+            .unwrap()
+            .insert("tensorzero::bad_metric".to_string(), old_metric_entry);
+
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+        assert_eq!(
+            result.unwrap_err(),
+            Error::new(ErrorDetails::Config {
+                message: "Metric name cannot start with 'tensorzero::': tensorzero::bad_metric"
+                    .to_string()
+            })
+        );
+    }
+
+    /// Ensure that the config validation fails when a model name starts with `tensorzero::`
+    #[test]
+    fn test_config_validate_model_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // Rename an existing model to start with `tensorzero::`
+        let old_model_entry = config["models"]
+            .as_table_mut()
+            .unwrap()
+            .remove("gpt-3.5-turbo")
+            .expect("Did not find model `gpt-3.5-turbo`");
+        config["models"]
+            .as_table_mut()
+            .unwrap()
+            .insert("tensorzero::bad_model".to_string(), old_model_entry);
+
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+        assert_eq!(
+            result.unwrap_err(),
+            Error::new(ErrorDetails::Config {
+                message:
+                    "Model name 'tensorzero::bad_model' contains a reserved prefix\nin `models`\n"
+                        .to_string()
+            })
+        );
+    }
+
+    /// Ensure that the config validation fails when an embedding model name starts with `tensorzero::`
+    #[test]
+    fn test_config_validate_embedding_model_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // Rename an existing embedding model to start with `tensorzero::`
+        let old_embedding_model_entry = config["embedding_models"]
+            .as_table_mut()
+            .unwrap()
+            .remove("text-embedding-3-small")
+            .expect("Did not find embedding model `text-embedding-3-small`");
+        config["embedding_models"].as_table_mut().unwrap().insert(
+            "tensorzero::bad_embedding_model".to_string(),
+            old_embedding_model_entry,
+        );
+
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+        assert_eq!(
+                result.unwrap_err(),
+                Error::new(ErrorDetails::Config {
+                    message:
+                        "Embedding model name cannot start with 'tensorzero::': tensorzero::bad_embedding_model"
+                            .to_string()
+                })
+            );
+    }
+
+    /// Ensure that the config validation fails when a tool name starts with `tensorzero::`
+    #[test]
+    fn test_config_validate_tool_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // Clone an existing tool and add a new one with tensorzero:: prefix
+        let old_tool_entry = config["tools"]
+            .as_table()
+            .unwrap()
+            .get("get_temperature")
+            .expect("Did not find tool `get_temperature`")
+            .clone();
+        config["tools"]
+            .as_table_mut()
+            .unwrap()
+            .insert("tensorzero::bad_tool".to_string(), old_tool_entry);
+
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+        assert_eq!(
+            result.unwrap_err(),
+            Error::new(ErrorDetails::Config {
+                message: "Tool name cannot start with 'tensorzero::': tensorzero::bad_tool"
+                    .to_string()
+            })
+        );
+    }
+
+    /// If you also want to confirm a variant name starting with `tensorzero::` fails
+    /// (only do this if your `function.validate` logic checks variant names):
+    #[test]
+    fn test_config_validate_variant_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // For demonstration, rename an existing variant inside `generate_draft`:
+        let old_variant_entry = config["functions"]["generate_draft"]["variants"]
+            .as_table_mut()
+            .unwrap()
+            .remove("openai_promptA")
+            .expect("Did not find variant `openai_promptA`");
+        config["functions"]["generate_draft"]["variants"]
+            .as_table_mut()
+            .unwrap()
+            .insert("tensorzero::bad_variant".to_string(), old_variant_entry);
+
+        // This test will only pass if your code actually rejects variant names with that prefix
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+
+        // Adjust the expected message if your code gives a different error shape for variants
+        // Or remove this test if variant names are *not* validated in that manner
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("tensorzero::bad_variant"));
+    }
+
+    /// Ensure that the config validation fails when a model provider's name starts with `tensorzero::`
+    #[test]
+    fn test_config_validate_model_provider_name_tensorzero_prefix() {
+        let mut config = get_sample_valid_config();
+
+        // Rename an existing provider to start with `tensorzero::`
+        let old_openai_provider = config["models"]["gpt-3.5-turbo"]["providers"]
+            .as_table_mut()
+            .unwrap()
+            .remove("openai")
+            .expect("Did not find provider `openai` under `gpt-3.5-turbo`");
+        config["models"]["gpt-3.5-turbo"]["providers"]
+            .as_table_mut()
+            .unwrap()
+            .insert("tensorzero::openai".to_string(), old_openai_provider);
+
+        // Update the routing entry to match the new provider name
+        let routing = config["models"]["gpt-3.5-turbo"]["routing"]
+            .as_array_mut()
+            .expect("Expected routing to be an array");
+        for entry in routing.iter_mut() {
+            if entry.as_str() == Some("openai") {
+                *entry = toml::Value::String("tensorzero::openai".to_string());
+            }
+        }
+
+        let base_path = PathBuf::new();
+        let result = Config::load_from_toml(config, base_path);
+
+        // Adjust this check to match how your code surfaces the "cannot start with `tensorzero::`" error
+        assert_eq!(
+            result.unwrap_err(),
+            Error::new(ErrorDetails::Config {
+                message: "`models.gpt-3.5-turbo.routing`: Provider name cannot start with 'tensorzero::': tensorzero::openai"
+                    .to_string()
+            })
+        );
+    }
+
     /// Ensure that get_templates returns the correct templates
     #[test]
     fn test_get_all_templates() {
diff --git a/tensorzero_internal/src/endpoints/feedback.rs b/tensorzero_internal/src/endpoints/feedback.rs
index 058f8e18b..5d69502a7 100644
--- a/tensorzero_internal/src/endpoints/feedback.rs
+++ b/tensorzero_internal/src/endpoints/feedback.rs
@@ -19,6 +19,8 @@ use crate::inference::types::{parse_chat_output, ContentBlock, ContentBlockOutpu
 use crate::tool::{DynamicToolParams, StaticToolConfig, ToolCall, ToolCallConfig};
 use crate::uuid_util::uuid_elapsed;
 
+use super::validate_tags;
+
 /// There is a potential issue here where if we write an inference and then immediately write feedback for it,
 /// we might not be able to find the inference in the database because it hasn't been written yet.
 ///
@@ -78,6 +80,7 @@ pub async fn feedback_handler(
     }): AppState,
     StructuredJson(params): StructuredJson<Params>,
 ) -> Result<Json<Value>, Error> {
+    validate_tags(&params.tags)?;
     // Get the metric config or return an error if it doesn't exist
     let feedback_metadata = get_feedback_metadata(
         &config,
diff --git a/tensorzero_internal/src/endpoints/inference.rs b/tensorzero_internal/src/endpoints/inference.rs
index 77e122182..f906588a7 100644
--- a/tensorzero_internal/src/endpoints/inference.rs
+++ b/tensorzero_internal/src/endpoints/inference.rs
@@ -36,6 +36,8 @@ use crate::tool::{DynamicToolParams, ToolCallConfig};
 use crate::uuid_util::validate_episode_id;
 use crate::variant::{InferenceConfig, Variant};
 
+use super::validate_tags;
+
 /// The expected payload is a JSON object with the following fields:
 #[derive(Debug, Deserialize)]
 #[serde(deny_unknown_fields)]
@@ -147,6 +149,7 @@ pub async fn inference(
 ) -> Result<InferenceOutput, Error> {
     // To be used for the Inference table processing_time measurements
     let start_time = Instant::now();
+    validate_tags(&params.tags)?;
     // Get the function config or return an error if it doesn't exist
     let function = config.get_function(&params.function_name)?.clone();
     let tool_config = function.prepare_tool_config(params.dynamic_tool_params, &config.tools)?;
diff --git a/tensorzero_internal/src/endpoints/mod.rs b/tensorzero_internal/src/endpoints/mod.rs
index fbeb5af78..ae2db4670 100644
--- a/tensorzero_internal/src/endpoints/mod.rs
+++ b/tensorzero_internal/src/endpoints/mod.rs
@@ -1,6 +1,34 @@
+use std::collections::HashMap;
+
+use crate::error::{Error, ErrorDetails};
+
 pub mod batch_inference;
 pub mod fallback;
 pub mod feedback;
 pub mod inference;
 pub mod openai_compatible;
 pub mod status;
+
+fn validate_tags(tags: &HashMap<String, String>) -> Result<(), Error> {
+    for tag_name in tags.keys() {
+        if tag_name.starts_with("tensorzero::") {
+            return Err(Error::new(ErrorDetails::InvalidRequest {
+                message: format!("Tag name cannot start with 'tensorzero::': {tag_name}"),
+            }));
+        }
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_validate_tags() {
+        let mut tags = HashMap::new();
+        assert!(validate_tags(&tags).is_ok());
+        tags.insert("tensorzero::test".to_string(), "test".to_string());
+        assert!(validate_tags(&tags).is_err());
+    }
+}
diff --git a/tensorzero_internal/src/function.rs b/tensorzero_internal/src/function.rs
index 4cf722bf0..67237ac2c 100644
--- a/tensorzero_internal/src/function.rs
+++ b/tensorzero_internal/src/function.rs
@@ -254,6 +254,14 @@ impl FunctionConfig {
     ) -> Result<(), Error> {
         // Validate each variant
         for (variant_name, variant) in self.variants() {
+            if variant_name.starts_with("tensorzero::") {
+                return Err(ErrorDetails::Config {
+                    message: format!(
+                        "Variant name cannot start with 'tensorzero::': {variant_name}"
+                    ),
+                }
+                .into());
+            }
             variant.validate(
                 self,
                 models,
diff --git a/tensorzero_internal/src/model.rs b/tensorzero_internal/src/model.rs
index 434fa477b..6c6838b40 100644
--- a/tensorzero_internal/src/model.rs
+++ b/tensorzero_internal/src/model.rs
@@ -166,8 +166,52 @@ impl ModelConfig {
         }))
     }
 
-    pub fn validate(&self) -> Result<(), Error> {
-        // Placeholder in case we want to add validation in the future
+    pub fn validate(&self, model_name: &str) -> Result<(), Error> {
+        // Ensure that the model has at least one provider
+        if self.routing.is_empty() {
+            return Err(ErrorDetails::Config {
+                message: format!("`models.{model_name}`: `routing` must not be empty"),
+            }
+            .into());
+        }
+
+        // Ensure that routing entries are unique and exist as keys in providers
+        let mut seen_providers = std::collections::HashSet::new();
+        for provider in &self.routing {
+            if provider.starts_with("tensorzero::") {
+                return Err(ErrorDetails::Config {
+                    message: format!("`models.{model_name}.routing`: Provider name cannot start with 'tensorzero::': {provider}"),
+                }
+                .into());
+            }
+            if !seen_providers.insert(provider) {
+                return Err(ErrorDetails::Config {
+                    message: format!("`models.{model_name}.routing`: duplicate entry `{provider}`"),
+                }
+                .into());
+            }
+
+            if !self.providers.contains_key(provider) {
+                return Err(ErrorDetails::Config {
+            message: format!(
+                "`models.{model_name}`: `routing` contains entry `{provider}` that does not exist in `providers`"
+            ),
+        }
+        .into());
+            }
+        }
+
+        // Validate each provider
+        for provider_name in self.providers.keys() {
+            if !seen_providers.contains(provider_name) {
+                return Err(ErrorDetails::Config {
+                    message: format!(
+                "`models.{model_name}`: Provider `{provider_name}` is not listed in `routing`"
+            ),
+                }
+                .into());
+            }
+        }
         Ok(())
     }
 }
@@ -862,10 +906,14 @@ impl TryFrom<(CredentialLocation, &str)> for Credential {
 }
 
 lazy_static! {
-    static ref RESERVED_MODEL_PREFIXES: Vec<String> = ProviderConfigHelper::VARIANTS
-        .iter()
-        .map(|&v| format!("{}::", v))
-        .collect();
+    static ref RESERVED_MODEL_PREFIXES: Vec<String> = {
+        let mut prefixes: Vec<String> = ProviderConfigHelper::VARIANTS
+            .iter()
+            .map(|&v| format!("{}::", v))
+            .collect();
+        prefixes.push("tensorzero::".to_string());
+        prefixes
+    };
 }
 
 const SHORTHAND_MODEL_PREFIXES: &[&str] = &[
@@ -916,7 +964,7 @@ impl ModelTable {
         // Try direct lookup (if it's blacklisted, it's not in the table)
         // If it's shorthand and already in the table, it's valid
         if let Some(model_config) = self.0.get(key) {
-            model_config.validate()?;
+            model_config.validate(key)?;
             return Ok(());
         }
 
@@ -940,7 +988,7 @@ impl ModelTable {
             // Remove the last two characters of the prefix to get the provider type
             let provider_type = &prefix[..prefix.len() - 2];
             let model_config = model_config_from_shorthand(provider_type, model_name)?;
-            model_config.validate()?;
+            model_config.validate(key)?;
             self.0.insert(key.into(), model_config);
             return Ok(());
         }


Test patch: