Skip to content

Commit 88eb808

Browse files
BtXinclaude
andauthored
fix: register chunk UDF on runtime ctx; add libgomp1 to Docker image (#128)
* fix: register chunk UDF on runtime ctx; add libgomp1 to Docker image Two bugs caught running the -rag Docker variant end-to-end against a pgvector sidecar with the demo/rag/server pipelines: * Server: chunk() was only registered on the planning SessionContext (config.rs::setup_context_for_pipelines), not on the runtime SessionContext (server.rs::setup_app_state). Pipeline SQL passed startup validation, but `/ingest-chunked/execute` would fail at request time with "function chunk not found." Same shape as the previous candle/onnx cfg-gate fix in #120. * Docker: the runtime stage (debian:trixie-slim) was missing libgomp1. candle-core links against OpenMP on the CPU path, so the binary failed to load with `libgomp.so.1: cannot open shared object file`. This pre-existed the -rag variant — the previous -embedding image hit the same loader error on first start. Verified end-to-end after the fixes: chunk UDF registered on both planning + runtime contexts, POST /ingest-chunked landed 7 rows in Postgres, /search-hybrid returned the right RRF chunk first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: cross-UDF composition guards for chunk() × every embedding UDF Adds a new integration-test file that pairs chunk() with each embedding UDF skardi ships (candle, gguf, onnx_predict, remote_embed) on a single SessionContext and asserts the canonical chunk → embed → write SQL plans cleanly. Tests stop at ctx.sql(...) (no .collect()) so the embedding UDF body never runs and no real model needs to exist. Lives under crates/skardi/tests/ rather than chunking/mod.rs because the assertion is about cross-module composability, not chunk() internals. Each test is gated by #[cfg(feature = "<embedding>")] so it only compiles when the relevant UDF crate is in the build; the file itself is gated by #![cfg(feature = "chunking")]. All four pass under `cargo test -p skardi --features rag --test cross_udf_composition`. Guards the regression class fixed in #128 — UDF registered on the planning ctx but missing from the runtime ctx — across every embedding UDF, not just candle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6b48384 commit 88eb808

4 files changed

Lines changed: 137 additions & 0 deletions

File tree

Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ RUN apt-get update && apt-get install -y \
2929
libssl3t64 \
3030
libsqlite3-0 \
3131
zlib1g \
32+
libgomp1 \
3233
ca-certificates \
3334
&& rm -rf /var/lib/apt/lists/*
3435

crates/server/src/server.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use crate::auth::mode::AuthMode;
1717
use crate::config::ServerConfig;
1818
#[cfg(feature = "candle")]
1919
use crate::config::register_candle_udf;
20+
#[cfg(feature = "chunking")]
21+
use crate::config::register_chunk_udf;
2022
#[cfg(feature = "gguf")]
2123
use crate::config::register_gguf_udf;
2224
#[cfg(feature = "onnx")]
@@ -148,6 +150,9 @@ pub async fn setup_app_state(config: ServerConfig) -> Result<AppState> {
148150
// Register candle UDF (lazy — models loaded on first call from inline path)
149151
#[cfg(feature = "candle")]
150152
register_candle_udf(&mut session_ctx);
153+
// Register chunk UDF (text-splitter wrapper for inline ingestion)
154+
#[cfg(feature = "chunking")]
155+
register_chunk_udf(&mut session_ctx);
151156

152157
// Build auth layer and register auth.users / auth.sessions on the runtime SessionContext.
153158
let auth_layer = AuthLayer::build(&AuthMode::from_env()).await?;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,4 +667,8 @@ mod tests {
667667
assert!(slugs[0].starts_with("alice/chap1/p0"), "got {slugs:?}");
668668
assert!(slugs.iter().all(|s| s.starts_with("alice/chap1/p")));
669669
}
670+
671+
// Cross-UDF composition guards (chunk × candle/gguf/onnx/remote_embed)
672+
// live in `crates/skardi/tests/cross_udf_composition.rs` so chunking
673+
// module tests stay focused on chunk()'s own behaviour.
670674
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
//! Cross-UDF composition guards.
2+
//!
3+
//! Each test pairs `chunk()` with one embedding UDF and asserts both
4+
//! resolve and plan together on a single `SessionContext`. Tests stop at
5+
//! `ctx.sql(...)` (no `.collect()`) so the embedding UDF is never invoked
6+
//! and no real model needs to exist on disk.
7+
//!
8+
//! These guard the regression class fixed in #128 — a UDF registered on
9+
//! the planning context but missing from the runtime context would let
10+
//! pipeline SQL validate at startup but fail at request time. Catching
11+
//! that in unit tests would require simulating the server's two-context
12+
//! setup; this is the next-best thing: structural assertions that every
13+
//! shipping embedding UDF can coexist with `chunk()` on the same context.
14+
15+
#![cfg(feature = "chunking")]
16+
17+
use std::sync::Arc;
18+
19+
use datafusion::execution::FunctionRegistry;
20+
use datafusion::prelude::SessionContext;
21+
use skardi::model::ChunkingRegistry;
22+
23+
/// SQL fragment that all four tests reuse — chunks a literal markdown
24+
/// body and pipes the chunks through the paired embedding UDF.
25+
fn build_ctx() -> SessionContext {
26+
let mut ctx = SessionContext::new();
27+
Arc::new(ChunkingRegistry::new()).register_chunk_udf(&mut ctx);
28+
ctx
29+
}
30+
31+
#[cfg(feature = "candle")]
32+
#[tokio::test]
33+
async fn chunk_composes_with_candle() {
34+
use skardi::model::CandleModelRegistry;
35+
36+
let mut ctx = build_ctx();
37+
Arc::new(CandleModelRegistry::new()).register_candle_udf(&mut ctx);
38+
39+
assert!(ctx.udf("chunk").is_ok());
40+
assert!(ctx.udf("candle").is_ok());
41+
42+
let sql = "WITH src AS (SELECT 'doc body for chunking' AS body) \
43+
SELECT chunk_text, \
44+
candle('models/does/not/exist', chunk_text) AS embedding \
45+
FROM ( \
46+
SELECT UNNEST(chunk('markdown', body, 50)) AS chunk_text \
47+
FROM src \
48+
)";
49+
ctx.sql(sql)
50+
.await
51+
.expect("chunk + candle composition should plan cleanly");
52+
}
53+
54+
#[cfg(feature = "gguf")]
55+
#[tokio::test]
56+
async fn chunk_composes_with_gguf() {
57+
use skardi::model::GgufModelRegistry;
58+
59+
let mut ctx = build_ctx();
60+
Arc::new(GgufModelRegistry::new()).register_gguf_udf(&mut ctx);
61+
62+
assert!(ctx.udf("chunk").is_ok());
63+
assert!(ctx.udf("gguf").is_ok());
64+
65+
let sql = "WITH src AS (SELECT 'doc body for chunking' AS body) \
66+
SELECT chunk_text, \
67+
gguf('models/does/not/exist', chunk_text) AS embedding \
68+
FROM ( \
69+
SELECT UNNEST(chunk('markdown', body, 50)) AS chunk_text \
70+
FROM src \
71+
)";
72+
ctx.sql(sql)
73+
.await
74+
.expect("chunk + gguf composition should plan cleanly");
75+
}
76+
77+
#[cfg(feature = "onnx")]
78+
#[tokio::test]
79+
async fn chunk_composes_with_onnx_predict() {
80+
use skardi::model::OnnxModelRegistry;
81+
82+
let mut ctx = build_ctx();
83+
Arc::new(OnnxModelRegistry::new()).register_onnx_predict_udf(&mut ctx);
84+
85+
assert!(ctx.udf("chunk").is_ok());
86+
assert!(ctx.udf("onnx_predict").is_ok());
87+
88+
// onnx_predict takes a model path + numeric inputs. Pair it with the
89+
// chunk-text length so both UDFs land in the same query shape.
90+
let sql = "WITH src AS (SELECT 'doc body for chunking' AS body) \
91+
SELECT chunk_text, \
92+
onnx_predict('models/does/not/exist.onnx', \
93+
CAST(LENGTH(chunk_text) AS BIGINT)) AS score \
94+
FROM ( \
95+
SELECT UNNEST(chunk('markdown', body, 50)) AS chunk_text \
96+
FROM src \
97+
)";
98+
ctx.sql(sql)
99+
.await
100+
.expect("chunk + onnx_predict composition should plan cleanly");
101+
}
102+
103+
#[cfg(feature = "remote-embed")]
104+
#[tokio::test]
105+
async fn chunk_composes_with_remote_embed() {
106+
use skardi::model::RemoteEmbedRegistry;
107+
108+
let mut ctx = build_ctx();
109+
Arc::new(RemoteEmbedRegistry::new()).register_remote_embed_udf(&mut ctx);
110+
111+
assert!(ctx.udf("chunk").is_ok());
112+
assert!(ctx.udf("remote_embed").is_ok());
113+
114+
// We never `.collect()` — no API call fires, just a planner-level check
115+
// that both UDFs name-resolve and compose.
116+
let sql = "WITH src AS (SELECT 'doc body for chunking' AS body) \
117+
SELECT chunk_text, \
118+
remote_embed('openai', 'text-embedding-3-small', chunk_text) \
119+
AS embedding \
120+
FROM ( \
121+
SELECT UNNEST(chunk('markdown', body, 50)) AS chunk_text \
122+
FROM src \
123+
)";
124+
ctx.sql(sql)
125+
.await
126+
.expect("chunk + remote_embed composition should plan cleanly");
127+
}

0 commit comments

Comments
 (0)