fix(udf): PR #193 round-2 — merge-safe packs pin, view-string + list-literal support, chunk arg dedup

- packs pin: main gained the feishu pack while this branch was in
  flight, so the roster assertion would have gone red on the merged
  tree. Absorbed main; the test now asserts sortedness independently of
  the roster (survives future pack additions) plus the explicit
  completeness list including feishu.
- json_pack: Utf8View accepted for keys and values (DataFusion 52
  carries computed string expressions as view types — the filters.rs
  precedent); a const-folded make_array literal (ScalarValue::List)
  now encodes through the same utf8_list_value conversion as list
  columns, honoring the documented "column or literal" contract.
- json_pack coverage: every timestamp granularity pinned by value
  (seconds ×1000 checked, micro/nano floor, pre-epoch sign), the
  seconds-overflow refusal, List<Utf8> with null element and null
  list, the make_array literal spelling, non-Utf8 list elements
  refused with the type named, and view-typed keys/values byte-exact.
- chunking: the ~55 duplicated argument-parsing lines collapse into
  one parse_chunk_args shared by chunk and chunk_parts, so the two
  UDFs' argument contracts cannot drift; the shared text reader also
  accepts Utf8View columns and scalars.
- style: register_json_pack_udf imported via `use` in server
  config.rs/server.rs instead of inline crate paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: bakey

crates/server/src/config.rs

@@ -20,6 +20,7 @@ use skardi::sources::providers::seekdb::register_seekdb_tables;
 use skardi::sources::providers::sqlite::register_sqlite_tables;
 use skardi::sources::providers::sqlx::postgres::register_postgres_tables;
 use skardi::sources::sql_validator::{AdhocSqlPolicy, SqlValidatorConfig, validate_sql};
+use skardi::util::json_pack::register_json_pack_udf;
 use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
@@ -445,7 +446,7 @@ pub async fn load_server_config(args: CliArgs) -> Result<ServerConfig> {
     register_chunk_udf(&mut session_ctx);
     // Register json_pack UDF (SQL-side JSON encoding; the etl generator's
     // metadata/frontmatter serialization boundary)
-    skardi::util::json_pack::register_json_pack_udf(&mut session_ctx);
+    register_json_pack_udf(&mut session_ctx);
 
     // This auth layer is used only for SQL planning and is discarded after current function returns.
     // The live auth layer is built separately in setup_app_state.

crates/server/src/server.rs

@@ -8,6 +8,7 @@ use skardi::engine::datafusion::DataFusionEngine;
 use skardi::jobs::{JobExecutor, JobStore, SqliteJobStore};
 use skardi::sources::DataSourceType;
 use skardi::sources::sql_validator::AdhocSqlPolicy;
+use skardi::util::json_pack::register_json_pack_udf;
 use std::collections::HashMap;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
@@ -190,7 +191,7 @@ pub async fn setup_app_state(config: ServerConfig) -> Result<AppState> {
     register_chunk_udf(&mut session_ctx);
     // Register json_pack UDF (SQL-side JSON encoding; the etl generator's
     // metadata/frontmatter serialization boundary)
-    skardi::util::json_pack::register_json_pack_udf(&mut session_ctx);
+    register_json_pack_udf(&mut session_ctx);
 
     // Build auth layer and register auth.users / auth.sessions on the runtime SessionContext.
     let auth_layer = AuthLayer::build(&AuthMode::from_env()).await?;
@@ -594,6 +595,8 @@ spec:
             ctx_file: None,
             semantics_path: None,
             port: 8080,
+            query_audit_db: None,
+            query_audit_retention_days: None,
         };
         let config = crate::config::load_server_config(args)
             .await

crates/skardi/src/model/chunking/mod.rs

@@ -26,7 +26,8 @@
 use std::sync::Arc;
 
 use arrow::array::{
-    Array, ArrayRef, Int32Builder, ListBuilder, StringArray, StringBuilder, StructBuilder,
+    Array, ArrayRef, Int32Builder, ListBuilder, StringArray, StringBuilder, StringViewArray,
+    StructBuilder,
 };
 use arrow::datatypes::{DataType, Field, Fields};
 use datafusion::common::Result as DfResult;
@@ -142,35 +143,7 @@ impl ScalarUDFImpl for ChunkingUDF {
 
     fn invoke_with_args(&self, args: ScalarFunctionArgs) -> DfResult<ColumnarValue> {
         let args = args.args;
-
-        if args.len() < 3 || args.len() > 4 {
-            return Err(DataFusionError::Execution(format!(
-                "chunk expects 3 or 4 arguments (mode, text, size [, overlap]); got {}",
-                args.len()
-            )));
-        }
-
-        let mode = read_scalar_string("chunk", &args[0], "mode")?;
-        let size = read_scalar_usize("chunk", &args[2], "size")?;
-        if size == 0 {
-            return Err(DataFusionError::Execution(
-                "chunk: 'size' must be > 0".to_string(),
-            ));
-        }
-        let overlap = if args.len() == 4 {
-            read_scalar_usize("chunk", &args[3], "overlap")?
-        } else {
-            0
-        };
-        // text-splitter's ChunkConfig::with_overlap also rejects this; the explicit
-        // check exists so the error names both values instead of a generic message.
-        if overlap >= size {
-            return Err(DataFusionError::Execution(format!(
-                "chunk: 'overlap' ({overlap}) must be strictly less than 'size' ({size})"
-            )));
-        }
-
-        let texts = read_text_column("chunk", &args[1], "text")?;
+        let (mode, texts, size, overlap) = parse_chunk_args("chunk", &args)?;
 
         let array: ArrayRef = match mode.as_str() {
             "character" => {
@@ -257,33 +230,7 @@ impl ScalarUDFImpl for ChunkPartsUDF {
 
     fn invoke_with_args(&self, args: ScalarFunctionArgs) -> DfResult<ColumnarValue> {
         let args = args.args;
-
-        if args.len() < 3 || args.len() > 4 {
-            return Err(DataFusionError::Execution(format!(
-                "chunk_parts expects 3 or 4 arguments (mode, text, size [, overlap]); got {}",
-                args.len()
-            )));
-        }
-
-        let mode = read_scalar_string("chunk_parts", &args[0], "mode")?;
-        let size = read_scalar_usize("chunk_parts", &args[2], "size")?;
-        if size == 0 {
-            return Err(DataFusionError::Execution(
-                "chunk_parts: 'size' must be > 0".to_string(),
-            ));
-        }
-        let overlap = if args.len() == 4 {
-            read_scalar_usize("chunk_parts", &args[3], "overlap")?
-        } else {
-            0
-        };
-        if overlap >= size {
-            return Err(DataFusionError::Execution(format!(
-                "chunk_parts: 'overlap' ({overlap}) must be strictly less than 'size' ({size})"
-            )));
-        }
-
-        let texts = read_text_column("chunk_parts", &args[1], "text")?;
+        let (mode, texts, size, overlap) = parse_chunk_args("chunk_parts", &args)?;
 
         let array: ArrayRef = match mode.as_str() {
             "character" => {
@@ -349,6 +296,46 @@ where
 // Argument decoding helpers
 // =============================================================================
 
+/// Parse the shared `(mode, text, size [, overlap])` argument contract —
+/// one implementation for `chunk` and `chunk_parts`, so the two UDFs'
+/// argument semantics cannot drift (identical arity, literal rules, and
+/// bounds; only the element type downstream differs). `udf` names the
+/// caller in every diagnostic.
+fn parse_chunk_args<'a>(
+    udf: &str,
+    args: &'a [ColumnarValue],
+) -> DfResult<(String, Vec<Option<&'a str>>, usize, usize)> {
+    if args.len() < 3 || args.len() > 4 {
+        return Err(DataFusionError::Execution(format!(
+            "{udf} expects 3 or 4 arguments (mode, text, size [, overlap]); got {}",
+            args.len()
+        )));
+    }
+
+    let mode = read_scalar_string(udf, &args[0], "mode")?;
+    let size = read_scalar_usize(udf, &args[2], "size")?;
+    if size == 0 {
+        return Err(DataFusionError::Execution(format!(
+            "{udf}: 'size' must be > 0"
+        )));
+    }
+    let overlap = if args.len() == 4 {
+        read_scalar_usize(udf, &args[3], "overlap")?
+    } else {
+        0
+    };
+    // text-splitter's ChunkConfig::with_overlap also rejects this; the explicit
+    // check exists so the error names both values instead of a generic message.
+    if overlap >= size {
+        return Err(DataFusionError::Execution(format!(
+            "{udf}: 'overlap' ({overlap}) must be strictly less than 'size' ({size})"
+        )));
+    }
+
+    let texts = read_text_column(udf, &args[1], "text")?;
+    Ok((mode, texts, size, overlap))
+}
+
 fn read_scalar_string(udf: &str, arg: &ColumnarValue, name: &str) -> DfResult<String> {
     match arg {
         ColumnarValue::Scalar(ScalarValue::Utf8(Some(s)))
@@ -397,7 +384,22 @@ fn read_text_column<'a>(
     name: &str,
 ) -> DfResult<Vec<Option<&'a str>>> {
     match arg {
+        // Utf8View included alongside the classic layouts: DataFusion 52
+        // carries computed string expressions as view arrays/scalars (the
+        // same reality open_connector/filters.rs handles), and a text
+        // column fed through a CAST or concat must not fail the split.
         ColumnarValue::Array(arr) => {
+            if let Some(view_arr) = arr.as_any().downcast_ref::<StringViewArray>() {
+                return Ok((0..view_arr.len())
+                    .map(|i| {
+                        if view_arr.is_null(i) {
+                            None
+                        } else {
+                            Some(view_arr.value(i))
+                        }
+                    })
+                    .collect());
+            }
             let str_arr = arr.as_any().downcast_ref::<StringArray>().ok_or_else(|| {
                 DataFusionError::Execution(format!("{udf}: '{name}' must be a Utf8 column"))
             })?;
@@ -412,10 +414,11 @@ fn read_text_column<'a>(
                 .collect())
         }
         ColumnarValue::Scalar(ScalarValue::Utf8(Some(s)))
-        | ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(s))) => Ok(vec![Some(s.as_str())]),
-        ColumnarValue::Scalar(ScalarValue::Utf8(None) | ScalarValue::LargeUtf8(None)) => {
-            Ok(vec![None])
-        }
+        | ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(s)))
+        | ColumnarValue::Scalar(ScalarValue::Utf8View(Some(s))) => Ok(vec![Some(s.as_str())]),
+        ColumnarValue::Scalar(
+            ScalarValue::Utf8(None) | ScalarValue::LargeUtf8(None) | ScalarValue::Utf8View(None),
+        ) => Ok(vec![None]),
         _ => Err(DataFusionError::Execution(format!(
             "{udf}: '{name}' must be Utf8"
         ))),

crates/skardi/src/sources/providers/open_connector/source_pack.rs

@@ -244,7 +244,16 @@ mod tests {
         // backing map is unordered, so the sort here is load-bearing.
         let registry = SourcePackRegistry::builtins().expect("embedded assets parse");
         let names: Vec<&str> = registry.packs().map(|p| p.name).collect();
-        assert_eq!(names, vec!["github", "mock", "notion", "slack"]);
+        // Sortedness, asserted independently of the roster so THIS pin
+        // survives future pack additions untouched…
+        assert!(
+            names.windows(2).all(|w| w[0] < w[1]),
+            "packs() must iterate name-sorted with no duplicates: {names:?}"
+        );
+        // …and completeness as an explicit roster, the one line a new pack
+        // must extend (a stale list here means the generator's coverage
+        // listing silently omits the newcomer).
+        assert_eq!(names, vec!["feishu", "github", "mock", "notion", "slack"]);
     }
 
     #[test]