Improve NSQL UX and harden internal LLM tools (sampling SQL quoting, tool descriptions) (spiceai#10715)

* Improve NSQL defaults and add datetime tool

* feat: update tool descriptions and add current datetime function to snapshots

* refactor(tests): improve variable naming for clarity in tools_by_name test

* fix: remove dereference operator for tool_name in all_available_tools function

* feat: enhance tool descriptions for clarity and detail across multiple tools

Co-authored-by: Copilot <copilot@github.com>

* feat: update tool descriptions for improved clarity and detail in Spice.ai runtime

* feat: enhance tool descriptions for clarity and detail in snapshots

* feat: clarify SQL query description in SqlToolParams struct

* feat: update SQL query description for consistency in Spice.ai SQL Dialect

Co-authored-by: Copilot <copilot@github.com>

---------

Co-authored-by: Copilot <copilot@github.com>

Author: lukekim

crates/runtime/src/http/v1/nsql.rs

@@ -53,6 +53,7 @@ use arrow::array::RecordBatch;
 use itertools::Itertools;
 use llms::chat::nsql::{FailedAttempt, QueryGenerationContext, default::DefaultSqlGeneration};
 use serde::{Deserialize, Serialize};
+use spicepod::component::model::ModelType;
 use std::{sync::Arc, time::Duration};
 use tokio::sync::RwLock;
 use tracing::Span;
@@ -138,15 +139,15 @@ pub struct Request {
     /// The natural language query to be converted into SQL
     pub query: String,
 
-    /// The name of the model to use for SQL generation. Default: "nql"
-    #[serde(default = "default_model")]
-    pub model: String,
+    /// The name of the model to use for SQL generation. If omitted, Spice defaults to the only compatible LLM model configured in the Spicepod.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub model: Option<String>,
 
     /// If true, streams the response instead of waiting for completion
     #[serde(default)]
     pub stream: bool,
 
-    /// Whether sample data is included in the context for SQL generation. Default: true
+    /// Whether sample data is included in the context for SQL generation. Default: false
     #[serde(default = "default_sample_data_enabled")]
     pub sample_data_enabled: bool,
 
@@ -160,11 +161,7 @@ pub struct Request {
 }
 
 fn default_sample_data_enabled() -> bool {
-    true
-}
-
-fn default_model() -> String {
-    "nql".to_string()
+    false
 }
 
 /// Checks if the request is asking to only generate SQL.
@@ -193,9 +190,8 @@ fn return_sql_only(accept: Option<&TypedHeader<Accept>>) -> bool {
             Request = "application/json",
             example = json!({
                 "query": "Get the top 5 customers by total sales",
-                "model": "nql",
                 "stream": false,
-                "sample_data_enabled": true,
+                "sample_data_enabled": false,
                 "datasets": ["sales_data"],
                 "prompt_cache_key": "sales-dashboard"
             })
@@ -323,12 +319,18 @@ pub(crate) async fn handle_nsql_query(
 
     let Request {
         query,
-        model,
+        model: requested_model,
         sample_data_enabled,
         datasets,
         prompt_cache_key,
         ..
     } = payload;
+
+    let model = match resolve_nsql_model_name(requested_model, &rt).await {
+        Ok(model) => model,
+        Err((status, message)) => return (status, headers, message),
+    };
+
     let table_allowlist_opt = match table_allowlist(&model, &rt).await {
         Ok(ta) => ta,
         Err(e) => {
@@ -405,16 +407,20 @@ pub(crate) async fn handle_nsql_query(
         vec![]
     };
 
-    let models = llms.read().await;
-    let Some(nql_model) = models.get(&model) else {
-        return (
-            StatusCode::BAD_REQUEST,
-            headers,
-            format!("Model {model} not found"),
-        );
+    let nql_model = {
+        let models = llms.read().await;
+        let Some(nql_model) = models.get(&model) else {
+            return (
+                StatusCode::BAD_REQUEST,
+                headers,
+                format!("Model {model} not found"),
+            );
+        };
+        Arc::clone(nql_model)
     };
 
-    let sql_gen = nql_model.as_sql().unwrap_or(&DefaultSqlGeneration {});
+    let default_sql_generation = DefaultSqlGeneration {};
+    let sql_gen = nql_model.as_sql().unwrap_or(&default_sql_generation);
     // Tracks previously generated queries and associated errors to enable an efficient retry mechanism
     let mut sql_gen_ctx = QueryGenerationContext::default();
     let mut num_retries = 0;
@@ -555,6 +561,49 @@ pub(crate) async fn handle_nsql_query(
     }
 }
 
+async fn resolve_nsql_model_name(
+    requested_model: Option<String>,
+    rt: &Arc<Runtime>,
+) -> Result<String, (StatusCode, String)> {
+    if let Some(model) = requested_model {
+        return Ok(model);
+    }
+
+    let Some(app) = rt.read_app().await else {
+        return Err((
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "Unexpected internal error. App not prepared in runtime.".to_string(),
+        ));
+    };
+
+    resolve_nsql_model_name_from_app(app.as_ref())
+        .map_err(|message| (StatusCode::BAD_REQUEST, message))
+}
+
+fn resolve_nsql_model_name_from_app(app: &app::App) -> Result<String, String> {
+    let compatible_models = compatible_nsql_model_names(app);
+
+    match compatible_models.as_slice() {
+        [] => Err(
+            "No model specified and no compatible LLM model is configured. Add exactly one LLM model to the Spicepod or include the 'model' field in the request."
+                .to_string(),
+        ),
+        [model] => Ok(model.clone()),
+        models => Err(format!(
+            "No model specified and multiple compatible LLM models are configured ({}). Include the 'model' field in the request.",
+            models.join(", ")
+        )),
+    }
+}
+
+fn compatible_nsql_model_names(app: &app::App) -> Vec<String> {
+    app.models
+        .iter()
+        .filter(|model| model.model_type() == Some(ModelType::Llm))
+        .map(|model| model.name.clone())
+        .collect()
+}
+
 /// Construct a [`ResolvedTableAwareAllowlist`] based on the `App`'s `model.datasets`.
 async fn table_allowlist(
     model_name: &str,
@@ -591,3 +640,82 @@ async fn table_allowlist(
     };
     Ok(table_allowlist)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use app::AppBuilder;
+    use serde_json::json;
+    use spicepod::component::model::Model;
+
+    fn app_with_models(models: Vec<Model>) -> app::App {
+        let mut builder = AppBuilder::new("test");
+        for model in models {
+            builder = builder.with_model(model);
+        }
+        builder.build()
+    }
+
+    #[test]
+    fn request_defaults_to_no_model_and_no_sample_data() {
+        let request: Request = serde_json::from_value(json!({
+            "query": "show total sales"
+        }))
+        .expect("request should deserialize with omitted optional fields");
+
+        assert_eq!(request.model, None);
+        assert!(!request.sample_data_enabled);
+    }
+
+    #[test]
+    fn omitted_model_uses_single_compatible_model() {
+        let app = app_with_models(vec![Model::new("openai:gpt-4o-mini", "llm_model")]);
+
+        let model_name = resolve_nsql_model_name_from_app(&app)
+            .expect("single compatible model should be selected");
+
+        assert_eq!(model_name, "llm_model");
+    }
+
+    #[test]
+    fn omitted_model_ignores_non_llm_models() {
+        let app = app_with_models(vec![
+            Model::new("spiceai:my-org/my-app/models/runnable", "ml_model"),
+            Model::new("openai:gpt-4o-mini", "llm_model"),
+        ]);
+
+        let model_name = resolve_nsql_model_name_from_app(&app)
+            .expect("single compatible model should be selected");
+
+        assert_eq!(model_name, "llm_model");
+    }
+
+    #[test]
+    fn omitted_model_errors_when_no_compatible_model_exists() {
+        let app = app_with_models(vec![]);
+
+        let error = resolve_nsql_model_name_from_app(&app)
+            .expect_err("omitted model should fail without compatible models");
+
+        assert_eq!(
+            error,
+            "No model specified and no compatible LLM model is configured. Add exactly one LLM model to the Spicepod or include the 'model' field in the request."
+        );
+    }
+
+    #[test]
+    fn omitted_model_errors_when_multiple_compatible_models_exist() {
+        let app = app_with_models(vec![
+            Model::new("openai:gpt-4o-mini", "first_model"),
+            Model::new("openai:gpt-4o", "second_model"),
+        ]);
+
+        let error = resolve_nsql_model_name_from_app(&app)
+            .expect_err("omitted model should fail with multiple compatible models");
+
+        assert_eq!(
+            error,
+            "No model specified and multiple compatible LLM models are configured (first_model, second_model). Include the 'model' field in the request."
+        );
+    }
+}

crates/runtime/src/http/v1/tools.rs

@@ -72,8 +72,8 @@ pub(crate) struct SearchToolsQuery {
             status = 200,  body = [ListToolElement],
             description = "All tools available in the Spice runtime",
             example = json!([
-                {"name": "get_readiness", "description": "Retrieves the readiness status of all runtime components including registered datasets, models, and embeddings.", "parameters": null},
-                {"name": "list_datasets", "description": "List all SQL tables available.", "parameters": null}
+                {"name": "get_readiness", "description": "Report the readiness state of every Spice runtime component (datasets, accelerators, models, embeddings, catalogs).", "parameters": null},
+                {"name": "list_datasets", "description": "List every dataset, view, and catalog visible to this runtime.", "parameters": null}
             ])
         ),
         (
@@ -110,7 +110,7 @@ pub(crate) async fn list(Extension(rt): Extension<Arc<Runtime>>) -> Response {
             example = json!([
                 {"name": "tool_search", "description": "Search the Spice tool registry for tools relevant to the current task.", "parameters": {"type": "object"}},
                 {"name": "tool_invoke", "description": "Invoke one Spice tool returned by tool_search.", "parameters": {"type": "object"}},
-                {"name": "list_datasets", "description": "List all SQL tables available.", "parameters": null}
+                {"name": "list_datasets", "description": "List every dataset, view, and catalog visible to this runtime.", "parameters": null}
             ])
         ),
         (status = 400, description = "Searchable tool registry is not configured", body = serde_json::Value),

crates/runtime/src/model/params/mod.rs

@@ -57,7 +57,7 @@ pub const PARAM_WITH_DEPRE_LEN: usize = 52;
 pub const COMMON_MODEL_PARAMETERS: [ParameterSpec; PARAM_LEN] = [
     // Common parameters for all models
     ParameterSpec::runtime("tools")
-        .description("Which tools should be made available to the model. Set to 'auto' to automatically choose between direct tools and searchable discovery, 'all' to use built-in and Spicepod-configured tools directly, or 'search_registry' to require searchable tool discovery."),
+        .description("Which tools should be made available to the model. Set to 'auto' to automatically choose between direct tools and searchable discovery without data sampling tools, 'all' to use built-in and Spicepod-configured tools directly, or 'search_registry' to require searchable tool discovery."),
     ParameterSpec::runtime("tool_embedding_model")
         .description("Embedding model name to use for searchable tool discovery. tools: search_registry requires a model configured in the embeddings section and uses it when only one embedding model is configured; tools: auto falls back to direct tools if embeddings are unavailable."),
     ParameterSpec::runtime("system_prompt")
@@ -120,7 +120,7 @@ pub const COMMON_MODEL_PARAMETERS: [ParameterSpec; PARAM_LEN] = [
 pub const COMMON_MODEL_PARAMETERS_WITH_DEPRECATED: [ParameterSpec; PARAM_WITH_DEPRE_LEN] = [
     // Common parameters for all models
     ParameterSpec::runtime("tools")
-        .description("Which tools should be made available to the model. Set to 'auto' to automatically choose between direct tools and searchable discovery, 'all' to use built-in and Spicepod-configured tools directly, or 'search_registry' to require searchable tool discovery."),
+        .description("Which tools should be made available to the model. Set to 'auto' to automatically choose between direct tools and searchable discovery without data sampling tools, 'all' to use built-in and Spicepod-configured tools directly, or 'search_registry' to require searchable tool discovery."),
     ParameterSpec::runtime("tool_embedding_model")
         .description("Embedding model name to use for searchable tool discovery. tools: search_registry requires a model configured in the embeddings section and uses it when only one embedding model is configured; tools: auto falls back to direct tools if embeddings are unavailable."),
     ParameterSpec::runtime("system_prompt")

crates/runtime/src/model/tool_use.rs

@@ -46,6 +46,7 @@ use tracing::{Instrument, Span};
 
 use crate::Runtime;
 use crate::model::ModelContextExtension;
+use crate::tools::utils::tool_call_error_response;
 use llms::progress::Progress;
 use runtime_request_context::{AsyncMarker, RequestContext};
 
@@ -107,10 +108,17 @@ impl ToolUsingChat {
         &self,
         list_datasets: &Arc<dyn SpiceModelTool>,
     ) -> Result<Vec<ChatCompletionRequestMessage>, OpenAIError> {
-        let t_resp = list_datasets
-            .call("")
-            .await
-            .map_err(|e| OpenAIError::InvalidArgument(e.to_string()))?;
+        let t_resp = match list_datasets.call("").await {
+            Ok(resp) => resp,
+            Err(e) => {
+                let tool_name = list_datasets.name();
+                let error = e.to_string();
+                tracing::warn!(
+                    "Tool '{tool_name}' failed while creating initial tool-use messages: {error}"
+                );
+                tool_call_error_response(tool_name.as_ref(), error)
+            }
+        };
         Ok(vec![
             ChatCompletionRequestAssistantMessageArgs::default()
                 .tool_calls(vec![ChatCompletionMessageToolCalls::Function(
@@ -166,10 +174,7 @@ impl ToolUsingChat {
                             .content(e.to_string())
                             .to_jsonl(),
                     );
-                    Value::String(format!(
-                        "Failed to call the tool {}.\nAn error occurred: {e}",
-                        t.name()
-                    ))
+                    tool_call_error_response(t.name().as_ref(), e)
                 }
             },
             None => {