diff --git a/crates/kb-app/src/app.rs b/crates/kb-app/src/app.rs index 01266d8..8156b3e 100644 --- a/crates/kb-app/src/app.rs +++ b/crates/kb-app/src/app.rs @@ -1,24 +1,22 @@ -//! `App` — internal lifecycle struct (§7). +//! `App` — facade lifecycle struct (§7). //! -//! A single `App` represents one CLI invocation's worth of state: a -//! resolved `Config`, an open `SqliteStore`, and (when embeddings are -//! enabled) an `Embedder` + `LanceVectorStore`. Each public free -//! function on `kb-app` wraps `App::open(config)` once, runs the -//! requested op, and drops everything on return. -//! -//! The struct is `pub(crate)` because it is an internal seam: `kb-cli` -//! calls only the free functions on the crate root. `kb-tui` (P9) is -//! expected to hold one `App` for the session, at which point the -//! struct may need to be promoted to `pub`. Until then, keep it -//! private to insulate the wiring shape from downstream callers. +//! A single `App` represents one CLI invocation's (or one TUI +//! session's / one eval-runner suite's) worth of state: a resolved +//! `Config`, an open `SqliteStore`, and (when embeddings are enabled) +//! an `Embedder` + `LanceVectorStore`. Each public free function on +//! `kb-app` builds an `App` once, runs the requested op, and drops +//! everything on return; long-lived callers (kb-eval, the future P9 +//! TUI session) hold onto an `App` across many calls so the per-query +//! cost is just a method dispatch. //! //! ## Embedder + Vector store lifetime //! -//! `App::open` builds the SQLite store unconditionally. The embedder -//! and vector store are *lazy + memoized* — built on first call to -//! [`App::embedder`] / [`App::vector`] and cached in `OnceLock`s — so -//! a long-lived `App` (e.g., the P9 TUI session) pays the ~470 MB -//! ONNX init plus Lance reopen cost exactly once. +//! `App::open_with_config` builds the SQLite store unconditionally. +//! The embedder and vector store are *lazy + memoized* — built on +//! first call to [`App::embedder`] / [`App::vector`] and cached in +//! `OnceLock`s — so a long-lived `App` (kb-eval driving 50 queries, +//! the P9 TUI session) pays the ~470 MB ONNX init plus Lance reopen +//! cost exactly once. //! //! - `kb list` / `kb inspect` never need them. //! - `kb search --mode lexical` never needs them. @@ -27,7 +25,8 @@ //! Building eagerly would force every CLI invocation to load ~470 MB of //! ONNX weights, which is the dominant cold-start cost. The lazy //! pattern keeps the lexical-only paths instant; the memoization makes -//! the TUI's repeated searches cheap after the first. +//! the TUI's repeated searches and the eval runner's per-query loop +//! cheap after the first invocation. //! //! Embeddings can also be **disabled** workspace-wide via //! `config.models.embedding.provider = "none"` (or `dimensions = 0`); @@ -36,15 +35,26 @@ use std::sync::{Arc, OnceLock}; -use anyhow::{Context, Result}; +use anyhow::{Context, Result, anyhow}; -use kb_core::Embedder; +use kb_core::{ + Answer, Embedder, IndexVersion, LanguageModel, Retriever, SearchHit, SearchMode, + SearchQuery, VectorStore, +}; use kb_embed_local::FastembedEmbedder; +use kb_llm_local::OllamaLanguageModel; +use kb_rag::{AskOpts, RagPipeline}; +use kb_search::{HybridRetriever, LexicalRetriever, VectorRetriever}; use kb_store_sqlite::SqliteStore; use kb_store_vector::LanceVectorStore; -/// Internal facade state. See module docs for lifetime rules. -pub(crate) struct App { +/// Facade state — see module docs for lifetime rules. +/// +/// The struct is public so long-lived callers (kb-eval, the future P9 +/// TUI session) can construct one and reuse it across many search / +/// ask calls. The OnceLock-backed `embedder` / `vector` fields ensure +/// the cold-start cost is paid exactly once per instance. +pub struct App { pub(crate) config: kb_config::Config, pub(crate) sqlite: Arc, /// Memoized embedder — built lazily on first `embedder()` call when @@ -54,6 +64,11 @@ pub(crate) struct App { /// Memoized vector store — built lazily on first `vector()` call /// when embeddings are enabled. Same rationale as `embedder`. vector: OnceLock>, + /// Memoized LLM — built lazily on first `ask()` call. Sharing one + /// across the eval runner avoids re-handshaking the Ollama HTTP + /// client per query (cheap, but still measurable on a 50-query + /// suite). + llm: OnceLock>, } impl App { @@ -65,7 +80,7 @@ impl App { /// Downstream `LanceVectorStore::new` (called by [`Self::vector`]) /// internally drives a `tokio::Runtime::block_on`, which panics if /// invoked from inside another tokio runtime. - pub(crate) fn open(config: kb_config::Config) -> Result { + pub fn open_with_config(config: kb_config::Config) -> Result { let sqlite = SqliteStore::open(&config).context("kb-app: open SqliteStore")?; sqlite .run_migrations() @@ -75,9 +90,112 @@ impl App { sqlite: Arc::new(sqlite), embedder: OnceLock::new(), vector: OnceLock::new(), + llm: OnceLock::new(), }) } + /// Run a [`SearchQuery`] through the configured retriever stack and + /// return the top-k hits. + /// + /// Reuses any previously-built embedder / vector store on this `App` + /// — long-lived callers (kb-eval, future TUI) get amortized cost + /// across calls. + pub fn search(&self, query: SearchQuery) -> Result> { + match query.mode { + SearchMode::Lexical => { + let lex = LexicalRetriever::with_settings( + self.sqlite.clone(), + lexical_index_version(&self.config), + self.config.search.snippet_chars, + ); + lex.search(&query) + } + SearchMode::Vector => { + let (emb, vec_store) = self.require_embeddings()?; + let vec_iv = vector_index_version(emb.as_ref()); + let vec_dyn: Arc = vec_store; + let emb_dyn: Arc = emb; + let retr = VectorRetriever::with_settings( + vec_dyn, + emb_dyn, + self.sqlite.clone(), + vec_iv, + self.config.search.snippet_chars, + ); + retr.search(&query) + } + SearchMode::Hybrid => { + let lex = Arc::new(LexicalRetriever::with_settings( + self.sqlite.clone(), + lexical_index_version(&self.config), + self.config.search.snippet_chars, + )) as Arc; + let (emb, vec_store) = self.require_embeddings()?; + let vec_iv = vector_index_version(emb.as_ref()); + let vec_dyn: Arc = vec_store; + let emb_dyn: Arc = emb; + let vec_retr = Arc::new(VectorRetriever::with_settings( + vec_dyn, + emb_dyn, + self.sqlite.clone(), + vec_iv, + self.config.search.snippet_chars, + )) as Arc; + let hybrid = HybridRetriever::new(&self.config, lex, vec_retr); + hybrid.search(&query) + } + } + } + + /// Run a RAG `ask` against the configured retriever + LLM. Reuses + /// the memoized embedder / vector / LLM where applicable. + pub fn ask(&self, query: &str, opts: AskOpts) -> Result { + let retriever: Arc = match opts.mode { + SearchMode::Lexical => Arc::new(LexicalRetriever::with_settings( + self.sqlite.clone(), + lexical_index_version(&self.config), + self.config.search.snippet_chars, + )), + SearchMode::Vector => { + let (emb, vec_store) = self.require_embeddings()?; + let vec_iv = vector_index_version(emb.as_ref()); + let vec_dyn: Arc = vec_store; + let emb_dyn: Arc = emb; + Arc::new(VectorRetriever::with_settings( + vec_dyn, + emb_dyn, + self.sqlite.clone(), + vec_iv, + self.config.search.snippet_chars, + )) + } + SearchMode::Hybrid => { + let lex = Arc::new(LexicalRetriever::with_settings( + self.sqlite.clone(), + lexical_index_version(&self.config), + self.config.search.snippet_chars, + )) as Arc; + let (emb, vec_store) = self.require_embeddings()?; + let vec_iv = vector_index_version(emb.as_ref()); + let vec_dyn: Arc = vec_store; + let emb_dyn: Arc = emb; + let vec_retr = Arc::new(VectorRetriever::with_settings( + vec_dyn, + emb_dyn, + self.sqlite.clone(), + vec_iv, + self.config.search.snippet_chars, + )) as Arc; + Arc::new(HybridRetriever::new(&self.config, lex, vec_retr)) + } + }; + + let llm = self.llm()?; + let pipeline = + RagPipeline::new(self.config.clone(), retriever, llm, self.sqlite.clone()); + pipeline.ask(query, opts) + } + /// Returns `true` when the workspace has embeddings turned off /// (`provider = "none"` or `dimensions = 0`). Lexical-only mode. pub(crate) fn embeddings_disabled(&self) -> bool { @@ -123,4 +241,64 @@ impl App { let _ = self.vector.set(store.clone()); Ok(Some(self.vector.get().cloned().unwrap_or(store))) } + + /// Build (or reuse) the configured LLM. Currently always Ollama; + /// when a second provider lands this is the place to switch on + /// `config.models.llm.provider`. + fn llm(&self) -> Result> { + if let Some(l) = self.llm.get() { + return Ok(l.clone()); + } + let llm: Arc = Arc::new( + OllamaLanguageModel::new(&self.config) + .context("kb-app::ask: build OllamaLanguageModel")?, + ); + let _ = self.llm.set(llm.clone()); + Ok(self.llm.get().cloned().unwrap_or(llm)) + } + + /// Resolve the embedder + vector store, surfacing the user-friendly + /// "switch to --mode lexical" error when embeddings are disabled. + fn require_embeddings( + &self, + ) -> Result<( + Arc, + Arc, + )> { + let emb = self.embedder()?.ok_or_else(|| { + anyhow!( + "embeddings disabled (config.models.embedding.provider == \"none\" \ + or dimensions == 0); vector / hybrid search require embeddings — \ + switch to --mode lexical or enable an embedding provider in config.toml" + ) + })?; + let vec_store = self.vector()?.ok_or_else(|| { + anyhow!( + "vector store unavailable while embedder is configured — this should \ + not happen; check `kb doctor` and the data_dir permissions" + ) + })?; + Ok((emb, vec_store)) + } +} + +/// Compose a stable `IndexVersion` for the lexical retriever from +/// the active config. This token surfaces in `SearchHit.index_version` +/// and on snapshot tests; including the chunker version pins it to +/// the chunking policy in effect. +fn lexical_index_version(config: &kb_config::Config) -> IndexVersion { + IndexVersion(format!("lex:{}", config.chunking.chunker_version)) +} + +/// Compose a stable `IndexVersion` for the vector retriever. Tracks +/// `(embedding_model, embedding_version, dimensions)` so a model swap +/// flags drift via the existing index_version mismatch warning in +/// `HybridRetriever::new`. +fn vector_index_version(embedder: &dyn Embedder) -> IndexVersion { + IndexVersion(format!( + "vec:{}@{}:{}", + embedder.model_id().0, + embedder.model_version().0, + embedder.dimensions(), + )) } diff --git a/crates/kb-app/src/lib.rs b/crates/kb-app/src/lib.rs index 8903e6a..aefc030 100644 --- a/crates/kb-app/src/lib.rs +++ b/crates/kb-app/src/lib.rs @@ -43,22 +43,18 @@ use kb_chunk::MdHeadingV1Chunker; use kb_core::{ Answer, CanonicalDocument, Chunk, ChunkId, ChunkPolicy, ChunkerVersion, Chunker, DocFilter, DocSummary, DocumentId, DocumentStore, Embedder, EmbeddingInput, - EmbeddingKind, IndexVersion, IngestReport, LanguageModel, ParserVersion, RawAsset, - Retriever, SearchHit, SearchMode, SearchQuery, SourceConnector, SourceScope, - SourceUri, VectorRecord, VectorStore, + EmbeddingKind, IngestReport, ParserVersion, RawAsset, SearchHit, SearchQuery, + SourceConnector, SourceScope, SourceUri, VectorRecord, VectorStore, }; -use kb_llm_local::OllamaLanguageModel; use kb_normalize::build_canonical_document; use kb_parse_md::{BodyHints, parse_blocks, parse_frontmatter}; -use kb_rag::RagPipeline; -use kb_search::{HybridRetriever, LexicalRetriever, VectorRetriever}; use kb_source_fs::FsSourceConnector; mod app; pub mod doctor_signal; pub mod logging; -use app::App; +pub use app::App; /// Parser-version label persisted in `documents.parser_version` for /// every Markdown file ingested through the `kb-parse-md` pipeline. @@ -168,7 +164,7 @@ pub fn ingest_with_config( ) -> anyhow::Result { let started_instant = std::time::Instant::now(); - let app = App::open(config)?; + let app = App::open_with_config(config)?; // Walk the workspace. let connector = FsSourceConnector::new(&app.config) @@ -667,7 +663,7 @@ pub fn list_docs_with_config( config: kb_config::Config, filter: DocFilter, ) -> anyhow::Result> { - let app = App::open(config)?; + let app = App::open_with_config(config)?; app.sqlite.list_documents(&filter) } @@ -683,7 +679,7 @@ pub fn inspect_doc_with_config( config: kb_config::Config, id: &DocumentId, ) -> anyhow::Result { - let app = App::open(config)?; + let app = App::open_with_config(config)?; app.sqlite .get_document(id)? .ok_or_else(|| anyhow!("document not found: {} (try `kb list docs`)", id.0)) @@ -701,7 +697,7 @@ pub fn inspect_chunk_with_config( config: kb_config::Config, id: &ChunkId, ) -> anyhow::Result { - let app = App::open(config)?; + let app = App::open_with_config(config)?; app.sqlite .get_chunk(id)? .ok_or_else(|| anyhow!("chunk not found: {} (try `kb inspect doc `)", id.0)) @@ -715,101 +711,15 @@ pub fn search(query: SearchQuery) -> anyhow::Result> { } /// Test-only seam — kb-cli must call the public free function -/// ([`search`]), not this. +/// ([`search`]), not this. Builds a one-shot `App` and delegates to +/// [`App::search`]; long-lived callers should hold an `App` instance +/// directly to amortize the embedder / vector-store cold start. #[doc(hidden)] pub fn search_with_config( config: kb_config::Config, query: SearchQuery, ) -> anyhow::Result> { - let app = App::open(config)?; - - match query.mode { - SearchMode::Lexical => { - let lex = LexicalRetriever::with_settings( - app.sqlite.clone(), - lexical_index_version(&app.config), - app.config.search.snippet_chars, - ); - lex.search(&query) - } - SearchMode::Vector => { - let (emb, vec_store) = require_embeddings(&app)?; - let vec_iv = vector_index_version(emb.as_ref()); - let vec_dyn: Arc = vec_store; - let emb_dyn: Arc = emb; - let retr = VectorRetriever::with_settings( - vec_dyn, - emb_dyn, - app.sqlite.clone(), - vec_iv, - app.config.search.snippet_chars, - ); - retr.search(&query) - } - SearchMode::Hybrid => { - let lex = Arc::new(LexicalRetriever::with_settings( - app.sqlite.clone(), - lexical_index_version(&app.config), - app.config.search.snippet_chars, - )) as Arc; - let (emb, vec_store) = require_embeddings(&app)?; - let vec_iv = vector_index_version(emb.as_ref()); - let vec_dyn: Arc = vec_store; - let emb_dyn: Arc = emb; - let vec_retr = Arc::new(VectorRetriever::with_settings( - vec_dyn, - emb_dyn, - app.sqlite.clone(), - vec_iv, - app.config.search.snippet_chars, - )) as Arc; - let hybrid = HybridRetriever::new(&app.config, lex, vec_retr); - hybrid.search(&query) - } - } -} - -fn require_embeddings( - app: &App, -) -> anyhow::Result<( - Arc, - Arc, -)> { - let emb = app.embedder()?.ok_or_else(|| { - anyhow!( - "embeddings disabled (config.models.embedding.provider == \"none\" \ - or dimensions == 0); vector / hybrid search require embeddings — \ - switch to --mode lexical or enable an embedding provider in config.toml" - ) - })?; - let vec_store = app.vector()?.ok_or_else(|| { - anyhow!( - "vector store unavailable while embedder is configured — this should \ - not happen; check `kb doctor` and the data_dir permissions" - ) - })?; - Ok((emb, vec_store)) -} - -/// Compose a stable `IndexVersion` for the lexical retriever from -/// the active config. This token surfaces in `SearchHit.index_version` -/// and on snapshot tests; including the chunker version pins it to -/// the chunking policy in effect. -fn lexical_index_version(config: &kb_config::Config) -> IndexVersion { - IndexVersion(format!("lex:{}", config.chunking.chunker_version)) -} - -/// Compose a stable `IndexVersion` for the vector retriever. Tracks -/// `(embedding_model, embedding_version, dimensions)` so a model swap -/// flags drift via the existing index_version mismatch warning in -/// `HybridRetriever::new`. -fn vector_index_version(embedder: &dyn Embedder) -> IndexVersion { - IndexVersion(format!( - "vec:{}@{}:{}", - embedder.model_id().0, - embedder.model_version().0, - embedder.dimensions(), - )) + App::open_with_config(config)?.search(query) } // ── ask ────────────────────────────────────────────────────────────────── @@ -826,64 +736,15 @@ pub fn ask(query: &str, opts: AskOpts) -> anyhow::Result { } /// Test-only seam — kb-cli must call the public free function -/// ([`ask`]), not this. Mirrors the `*_with_config` pattern documented -/// at the top of this module. +/// ([`ask`]), not this. Builds a one-shot `App` and delegates to +/// [`App::ask`]. #[doc(hidden)] pub fn ask_with_config( config: kb_config::Config, query: &str, opts: AskOpts, ) -> anyhow::Result { - let app = App::open(config)?; - - let retriever: Arc = match opts.mode { - SearchMode::Lexical => Arc::new(LexicalRetriever::with_settings( - app.sqlite.clone(), - lexical_index_version(&app.config), - app.config.search.snippet_chars, - )), - SearchMode::Vector => { - let (emb, vec_store) = require_embeddings(&app)?; - let vec_iv = vector_index_version(emb.as_ref()); - let vec_dyn: Arc = vec_store; - let emb_dyn: Arc = emb; - Arc::new(VectorRetriever::with_settings( - vec_dyn, - emb_dyn, - app.sqlite.clone(), - vec_iv, - app.config.search.snippet_chars, - )) - } - SearchMode::Hybrid => { - let lex = Arc::new(LexicalRetriever::with_settings( - app.sqlite.clone(), - lexical_index_version(&app.config), - app.config.search.snippet_chars, - )) as Arc; - let (emb, vec_store) = require_embeddings(&app)?; - let vec_iv = vector_index_version(emb.as_ref()); - let vec_dyn: Arc = vec_store; - let emb_dyn: Arc = emb; - let vec_retr = Arc::new(VectorRetriever::with_settings( - vec_dyn, - emb_dyn, - app.sqlite.clone(), - vec_iv, - app.config.search.snippet_chars, - )) as Arc; - Arc::new(HybridRetriever::new(&app.config, lex, vec_retr)) - } - }; - - let llm: Arc = Arc::new( - OllamaLanguageModel::new(&app.config) - .context("kb-app::ask: build OllamaLanguageModel")?, - ); - - let pipeline = - RagPipeline::new(app.config.clone(), retriever, llm, app.sqlite.clone()); - pipeline.ask(query, opts) + App::open_with_config(config)?.ask(query, opts) } /// Run the doctor checks against the explicit config path the user diff --git a/crates/kb-config/src/lib.rs b/crates/kb-config/src/lib.rs index b2b1352..4620ff1 100644 --- a/crates/kb-config/src/lib.rs +++ b/crates/kb-config/src/lib.rs @@ -8,6 +8,9 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; +mod paths; +pub use paths::expand_path; + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Config { pub schema_version: u32, diff --git a/crates/kb-config/src/paths.rs b/crates/kb-config/src/paths.rs new file mode 100644 index 0000000..f36c6dc --- /dev/null +++ b/crates/kb-config/src/paths.rs @@ -0,0 +1,186 @@ +//! Shared path expansion helper. +//! +//! `Config::storage.*` fields are stored as raw template strings (e.g. +//! `${XDG_DATA_HOME:-~/.local/share}/kb`, `{data_dir}/runs`). Every +//! crate that turns one of those strings into a real filesystem path +//! needs to apply the same set of substitutions; this module is the +//! single source of truth so the behavior cannot drift. +//! +//! Substitutions, applied in order: +//! +//! 1. `{data_dir}` → caller-supplied `data_dir`. +//! - When the caller passes an empty `data_dir` (because they ARE +//! resolving `data_dir` itself), the substitution is a no-op so +//! a literal `{data_dir}` is left in place rather than producing +//! a `/{data_dir}/...` artifact. +//! 2. `${XDG_DATA_HOME:-}` (or the bare `${XDG_DATA_HOME}`) → +//! the env var if set + non-empty, else the default after `:-`. +//! Mimics POSIX shell's `${VAR:-default}` semantics. Mid-string +//! occurrences are supported; only the first match is replaced. +//! 3. Leading `~` / `~/...` → `$HOME`. Any non-leading `~` is left +//! literal (matches shell behavior — only the first segment expands). +//! +//! The result is a `PathBuf` regardless of whether all substitutions +//! were applicable; relative paths are kept relative to the caller's +//! CWD (not resolved here). + +use std::path::PathBuf; + +/// Expand storage-path templates. See module docs for the substitution +/// rules. +/// +/// Pass an empty `data_dir` when resolving `data_dir` itself; the +/// `{data_dir}` substitution becomes a no-op in that case so the +/// recursive shape (`data_dir = "${XDG_DATA_HOME:-…}/kb"`) resolves +/// without producing a literal `{data_dir}` token in the output. +pub fn expand_path(raw: &str, data_dir: &str) -> PathBuf { + let mut s = raw.to_string(); + + // 1. {data_dir} substitution (skipped when resolving data_dir + // itself; see module docs). + if !data_dir.is_empty() { + s = s.replace("{data_dir}", data_dir); + } + + // 2. ${XDG_DATA_HOME:-}: env override else default. + if let Some(start) = s.find("${XDG_DATA_HOME") { + if let Some(rel_end) = s[start..].find('}') { + let end = start + rel_end + 1; // include trailing '}' + let inner = &s[start + 2..end - 1]; // strip ${ and } + let replacement = match std::env::var("XDG_DATA_HOME") { + Ok(v) if !v.is_empty() => v, + _ => match inner.split_once(":-") { + Some((_, default)) => default.to_string(), + None => String::new(), + }, + }; + s.replace_range(start..end, &replacement); + } + } + + // 3. Leading `~` → $HOME. + if let Some(rest) = s.strip_prefix('~') { + if let Some(home) = std::env::var_os("HOME").map(PathBuf::from) { + return home.join(rest.trim_start_matches('/')); + } + } + + PathBuf::from(s) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex as StdMutex; + + /// `XDG_DATA_HOME` / `HOME` env mutations must be serialized so + /// concurrent test runs (cargo's default parallel runner) don't + /// observe each other's transient values. + static ENV_LOCK: StdMutex<()> = StdMutex::new(()); + + /// RAII guard: snapshots `XDG_DATA_HOME` on construction, restores + /// it on drop. + struct XdgGuard { + prior: Option, + } + + impl XdgGuard { + fn capture() -> Self { + Self { + prior: std::env::var("XDG_DATA_HOME").ok(), + } + } + } + + impl Drop for XdgGuard { + fn drop(&mut self) { + // SAFETY: edition 2024 marks set_var/remove_var unsafe + // because env mutation is not thread-safe. The ENV_LOCK + // guard at the call site prevents concurrent observation. + unsafe { + match &self.prior { + Some(v) => std::env::set_var("XDG_DATA_HOME", v), + None => std::env::remove_var("XDG_DATA_HOME"), + } + } + } + } + + #[test] + fn substitutes_data_dir_template() { + let p = expand_path("{data_dir}/runs", "/tmp/kbtest"); + assert_eq!(p, PathBuf::from("/tmp/kbtest/runs")); + } + + #[test] + fn data_dir_substitution_skipped_when_empty() { + // Empty `data_dir` is the "resolving data_dir itself" signal; + // the literal `{data_dir}` token must survive. + let p = expand_path("{data_dir}/runs", ""); + assert_eq!(p, PathBuf::from("{data_dir}/runs")); + } + + #[test] + fn passthrough_absolute_path() { + let p = expand_path("/abs/runs", "/ignored"); + assert_eq!(p, PathBuf::from("/abs/runs")); + } + + #[test] + fn xdg_data_home_set_replaces_var() { + let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = XdgGuard::capture(); + // SAFETY: lock held for the duration of this test. + unsafe { std::env::set_var("XDG_DATA_HOME", "/custom/path") }; + + let p = expand_path("${XDG_DATA_HOME:-~/.local/share}/kb", ""); + assert_eq!(p, PathBuf::from("/custom/path/kb")); + } + + #[test] + fn xdg_data_home_unset_uses_default() { + let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = XdgGuard::capture(); + // SAFETY: lock held for the duration of this test. + unsafe { std::env::remove_var("XDG_DATA_HOME") }; + + let home = std::env::var("HOME").expect("HOME must be set in tests"); + let expected = PathBuf::from(home).join(".local/share/kb"); + let p = expand_path("${XDG_DATA_HOME:-~/.local/share}/kb", ""); + assert_eq!(p, expected); + } + + #[test] + fn xdg_with_no_default_resolves_to_empty_when_unset() { + let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = XdgGuard::capture(); + // SAFETY: lock held for the duration of this test. + unsafe { std::env::remove_var("XDG_DATA_HOME") }; + + // No `:-default` clause, no env var → empty string substitution. + let p = expand_path("${XDG_DATA_HOME}/kb", ""); + assert_eq!(p, PathBuf::from("/kb")); + } + + #[test] + fn leading_tilde_expands_to_home() { + let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let home = std::env::var("HOME").expect("HOME must be set in tests"); + let p = expand_path("~/runs", ""); + assert_eq!(p, PathBuf::from(home).join("runs")); + } + + #[test] + fn data_dir_then_xdg_then_tilde_compose() { + // Order matters: substitute `{data_dir}` (which itself contains + // an unexpanded `${XDG_DATA_HOME}` and `~`), then the other two + // resolve the result. + let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = XdgGuard::capture(); + // SAFETY: lock held for the duration of this test. + unsafe { std::env::set_var("XDG_DATA_HOME", "/xdg/data") }; + + let p = expand_path("{data_dir}/runs", "/xdg/data/kb"); + assert_eq!(p, PathBuf::from("/xdg/data/kb/runs")); + } +} diff --git a/crates/kb-embed-local/src/lib.rs b/crates/kb-embed-local/src/lib.rs index 1d3e2d0..2d0796c 100644 --- a/crates/kb-embed-local/src/lib.rs +++ b/crates/kb-embed-local/src/lib.rs @@ -22,11 +22,11 @@ //! See `docs/superpowers/specs/2026-04-27-kb-final-form-design.md` //! §7.2 (Embedder), §6.4 ([models.embedding]), §9 (versioning). -use std::path::PathBuf; use std::sync::Mutex; use anyhow::{Context, Result}; use fastembed::{EmbeddingModel, InitOptions, TextEmbedding}; +use kb_config::expand_path; use kb_embed::{Embedder, EmbeddingInput, EmbeddingKind, EmbeddingModelId, EmbeddingVersion}; /// Subdirectory under `config.storage.model_dir` where the fastembed @@ -60,9 +60,8 @@ impl FastembedEmbedder { /// first `embed`). pub fn new(config: &kb_config::Config) -> Result { // 1. Resolve `{data_dir}/models/fastembed/` from the config - // templates. `kb-config` does not expose a public path - // resolver yet, so we hand-roll a tiny one mirroring - // kb-store-sqlite's `expand_data_dir`. + // templates. Goes through the shared `kb_config::expand_path` + // so every crate resolves storage paths identically. let data_dir = expand_path(&config.storage.data_dir, ""); let model_dir = expand_path(&config.storage.model_dir, &data_dir.to_string_lossy()); let cache_dir = model_dir.join(FASTEMBED_CACHE_SUBDIR); @@ -222,58 +221,6 @@ pub(crate) fn check_dim(model_dim: usize, cfg_dim: usize) -> Result<()> { Ok(()) } -/// Expand the limited template language `kb-config` uses for storage -/// paths. -/// -/// Supported substitutions, applied in order: -/// 1. `{data_dir}` → `data_dir` (caller-supplied resolved string). This -/// is a no-op when `data_dir` is empty (used by the recursive call -/// that resolves `data_dir` itself). -/// 2. `${XDG_DATA_HOME:-~/.local/share}` (and the bare -/// `${XDG_DATA_HOME}`) → env var if set, else the default after -/// `:-`. -/// 3. Leading `~` → `$HOME`. -/// -/// Mirrors `kb-store-sqlite::store::expand_data_dir`. Kept private to -/// this crate; promoting it to a public `kb-config` API is a separate -/// task (see task p3-2 risks: "don't expand kb-config's public API"). -fn expand_path(raw: &str, data_dir: &str) -> PathBuf { - let mut s = raw.to_string(); - - if !data_dir.is_empty() { - s = s.replace("{data_dir}", data_dir); - } - - // ${XDG_DATA_HOME:-~/.local/share}: respect env override, else fall - // back to the suffix after `:-`. - if let Some(start) = s.find("${XDG_DATA_HOME") { - if let Some(rel_end) = s[start..].find('}') { - let end = start + rel_end + 1; // include trailing '}' - let inner = &s[start + 2..end - 1]; // strip ${ and } - let replacement = match std::env::var("XDG_DATA_HOME") { - Ok(v) if !v.is_empty() => v, - _ => { - if let Some((_, default)) = inner.split_once(":-") { - default.to_string() - } else { - String::new() - } - } - }; - s.replace_range(start..end, &replacement); - } - } - - // Leading `~` → $HOME. - if let Some(rest) = s.strip_prefix('~') { - if let Some(home) = std::env::var_os("HOME").map(PathBuf::from) { - return home.join(rest.trim_start_matches('/')); - } - } - - PathBuf::from(s) -} - #[cfg(test)] mod tests { use super::*; @@ -354,80 +301,6 @@ mod tests { assert!(msg.contains("unsupported embedding model"), "msg={msg}"); } - // ── expand_path ────────────────────────────────────────────────── - - #[test] - fn expand_path_substitutes_data_dir_template() { - let p = expand_path("{data_dir}/models", "/tmp/kbtest"); - assert_eq!(p, PathBuf::from("/tmp/kbtest/models")); - } - - #[test] - fn expand_path_no_op_without_template() { - let p = expand_path("/abs/path", "/tmp/kbtest"); - assert_eq!(p, PathBuf::from("/abs/path")); - } - - // ── expand_path: XDG_DATA_HOME fallback ────────────────────────── - // - // These two tests mutate the process-wide `XDG_DATA_HOME` env var, - // which is unsafe under edition 2024 and racy under cargo's default - // parallel test runner. The shared `ENV_LOCK` serializes them; each - // test snapshots the prior value and restores it on exit. - - use std::sync::Mutex as StdMutex; - static ENV_LOCK: StdMutex<()> = StdMutex::new(()); - - /// RAII guard: snapshots `XDG_DATA_HOME` on construction, restores - /// it on drop. Pair with the `ENV_LOCK` guard for serial access. - struct XdgGuard { - prior: Option, - } - - impl XdgGuard { - fn capture() -> Self { - Self { - prior: std::env::var("XDG_DATA_HOME").ok(), - } - } - } - - impl Drop for XdgGuard { - fn drop(&mut self) { - // SAFETY: edition 2024 marks `set_var`/`remove_var` unsafe - // because env mutation is not thread-safe. Callers hold - // `ENV_LOCK` for the duration of the test, so no other - // thread observes the mutation. - unsafe { - match &self.prior { - Some(v) => std::env::set_var("XDG_DATA_HOME", v), - None => std::env::remove_var("XDG_DATA_HOME"), - } - } - } - } - - #[test] - fn expand_path_xdg_data_home_set() { - let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); - let _guard = XdgGuard::capture(); - // SAFETY: lock held for the duration of this test. - unsafe { std::env::set_var("XDG_DATA_HOME", "/custom/path") }; - - let p = expand_path("${XDG_DATA_HOME:-~/.local/share}/kb", ""); - assert_eq!(p, PathBuf::from("/custom/path/kb")); - } - - #[test] - fn expand_path_xdg_data_home_unset_falls_back_to_home() { - let _lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); - let _guard = XdgGuard::capture(); - // SAFETY: lock held for the duration of this test. - unsafe { std::env::remove_var("XDG_DATA_HOME") }; - - let home = std::env::var("HOME").expect("HOME must be set in tests"); - let expected = PathBuf::from(home).join(".local/share/kb"); - let p = expand_path("${XDG_DATA_HOME:-~/.local/share}/kb", ""); - assert_eq!(p, expected); - } + // expand_path tests live in `kb-config::paths`. The adapter imports + // it and trusts the upstream coverage rather than duplicating it. } diff --git a/crates/kb-eval/src/runner.rs b/crates/kb-eval/src/runner.rs index 5ebd340..7916654 100644 --- a/crates/kb-eval/src/runner.rs +++ b/crates/kb-eval/src/runner.rs @@ -3,8 +3,11 @@ use std::fs::File; use std::io::{BufWriter, Write}; use std::path::PathBuf; +use std::time::Instant; use anyhow::{Context, Result}; +use kb_app::App; +use kb_config::expand_path; use kb_core::{SearchFilters, SearchQuery}; use kb_store_sqlite::{EvalRunRow, SqliteStore}; use time::OffsetDateTime; @@ -12,6 +15,14 @@ use time::OffsetDateTime; use crate::loader::{load_golden_set, validate_against_db}; use crate::types::{EvalRun, EvalRunOpts, GoldenQuery, QueryResult}; +/// Convert a wall-clock duration since `start` into milliseconds clamped +/// to `u32::MAX`. The `QueryResult.elapsed_ms` and `eval_runs.duration_ms` +/// fields are `u32`; saturate (rather than wrap) so a stuck run never +/// reports a misleading sub-second duration. +fn elapsed_ms_u32(start: Instant) -> u32 { + start.elapsed().as_millis().min(u128::from(u32::MAX)) as u32 +} + /// Env var that overrides the default `fixtures/golden_queries.yaml` /// path. Resolved relative to the current working directory. const KB_EVAL_GOLDEN: &str = "KB_EVAL_GOLDEN"; @@ -32,15 +43,13 @@ pub fn run_eval(opts: &EvalRunOpts) -> Result { /// data_dir) and any future caller that wants to drive the runner /// against a non-default config. pub fn run_eval_with_config(cfg: &kb_config::Config, opts: &EvalRunOpts) -> Result { - let started = std::time::Instant::now(); + let started = Instant::now(); // ── 1. Load golden set ──────────────────────────────────────────────── + // + // `with_context` already names the path on error, so a separate + // `tracing::debug!` here would just be noise. let golden_path = resolve_golden_path(); - tracing::debug!( - target: "kb-eval", - path = %golden_path.display(), - "kb-eval: loading golden set" - ); let queries = load_golden_set(&golden_path).with_context(|| { format!( "load golden set from {} (override via KB_EVAL_GOLDEN)", @@ -69,9 +78,15 @@ pub fn run_eval_with_config(cfg: &kb_config::Config, opts: &EvalRunOpts) -> Resu serde_json::to_string(&config_snapshot_json).context("serialize config_snapshot_json")?; // ── 4. Per-query execution ──────────────────────────────────────────── + // + // Open one `App` for the whole suite. The embedder / vector store / + // LLM are memoized on the App, so a 50-query run pays the ~470 MB + // ONNX init + Lance reopen + Ollama handshake exactly once. + let app = App::open_with_config(cfg.clone()).context("open App for run_eval")?; + let mut per_query: Vec = Vec::with_capacity(queries.len()); for gq in &queries { - let qr = execute_query(cfg, gq, opts); + let qr = execute_query(&app, gq, opts); per_query.push(qr); } @@ -99,7 +114,7 @@ pub fn run_eval_with_config(cfg: &kb_config::Config, opts: &EvalRunOpts) -> Resu // ── 6. Mirror to runs_dir//per_query.jsonl ──────────────────── write_per_query_jsonl(cfg, &run_id, &per_query)?; - let duration_ms = started.elapsed().as_millis().min(u128::from(u32::MAX)) as u32; + let duration_ms = elapsed_ms_u32(started); tracing::info!( target: "kb-eval", run_id = %run_id, @@ -139,8 +154,8 @@ fn resolve_golden_path() -> PathBuf { /// Run one [`GoldenQuery`] through the kb-app facade. Errors are /// captured into `QueryResult.error` so the run continues. -fn execute_query(cfg: &kb_config::Config, gq: &GoldenQuery, opts: &EvalRunOpts) -> QueryResult { - let started = std::time::Instant::now(); +fn execute_query(app: &App, gq: &GoldenQuery, opts: &EvalRunOpts) -> QueryResult { + let started = Instant::now(); let search_query = SearchQuery { text: gq.query.clone(), @@ -149,7 +164,7 @@ fn execute_query(cfg: &kb_config::Config, gq: &GoldenQuery, opts: &EvalRunOpts) filters: SearchFilters::default(), }; - let (hits_top_k, mut error) = match kb_app::search_with_config(cfg.clone(), search_query) { + let (hits_top_k, mut error) = match app.search(search_query) { Ok(hits) => (hits, None), Err(e) => (Vec::new(), Some(format!("{e:#}"))), }; @@ -166,7 +181,7 @@ fn execute_query(cfg: &kb_config::Config, gq: &GoldenQuery, opts: &EvalRunOpts) seed: opts.seed, stream_sink: None, }; - match kb_app::ask_with_config(cfg.clone(), &gq.query, ask_opts) { + match app.ask(&gq.query, ask_opts) { Ok(ans) => Some(ans), Err(e) => { error = Some(format!("{e:#}")); @@ -177,15 +192,13 @@ fn execute_query(cfg: &kb_config::Config, gq: &GoldenQuery, opts: &EvalRunOpts) None }; - let elapsed_ms = started.elapsed().as_millis().min(u128::from(u32::MAX)) as u32; - QueryResult { query_id: gq.id.clone(), query: gq.query.clone(), mode: opts.mode, hits_top_k, answer, - elapsed_ms, + elapsed_ms: elapsed_ms_u32(started), error, } } @@ -231,7 +244,14 @@ fn write_per_query_jsonl( run_id: &str, per_query: &[QueryResult], ) -> Result<()> { - let runs_dir = expand_path(&cfg.storage.runs_dir, &cfg.storage.data_dir); + // `data_dir` may itself contain `${XDG_DATA_HOME:-…}` / `~` (the + // workspace-default does); resolve it before threading it into the + // `{data_dir}` substitution of `runs_dir`. + let resolved_data_dir = expand_path(&cfg.storage.data_dir, ""); + let runs_dir = expand_path( + &cfg.storage.runs_dir, + &resolved_data_dir.to_string_lossy(), + ); let run_dir = runs_dir.join(run_id); std::fs::create_dir_all(&run_dir) .with_context(|| format!("create run dir {}", run_dir.display()))?; @@ -248,83 +268,3 @@ fn write_per_query_jsonl( w.flush().context("flush per_query.jsonl")?; Ok(()) } - -/// Expand `{data_dir}` / `${XDG_DATA_HOME:-…}` / leading `~`. Mirror -/// of `kb-store-vector::paths::expand_path` and -/// `kb-store-sqlite::expand_data_dir` — kept private here because -/// `kb-config` does not (yet) expose a shared resolver helper. -fn expand_path(raw: &str, data_dir: &str) -> PathBuf { - let mut s = raw.to_string(); - - // First, resolve `data_dir` itself so any `{data_dir}` substitution - // points at an already-expanded base path. `data_dir` may contain - // `${XDG_DATA_HOME:-…}` and `~`; resolve them once and re-use the - // result. - let resolved_data_dir = expand_xdg_and_tilde(data_dir); - s = s.replace("{data_dir}", &resolved_data_dir); - - expand_xdg_and_tilde_path(&s) -} - -fn expand_xdg_and_tilde(raw: &str) -> String { - let s = expand_xdg(raw); - expand_tilde_str(&s) -} - -fn expand_xdg_and_tilde_path(raw: &str) -> PathBuf { - let s = expand_xdg_and_tilde(raw); - PathBuf::from(s) -} - -fn expand_xdg(raw: &str) -> String { - let mut s = raw.to_string(); - if let Some(start) = s.find("${XDG_DATA_HOME") { - if let Some(rel_end) = s[start..].find('}') { - let end = start + rel_end + 1; - let inner = &s[start + 2..end - 1]; - let replacement = match std::env::var("XDG_DATA_HOME") { - Ok(v) if !v.is_empty() => v, - _ => match inner.split_once(":-") { - Some((_, default)) => default.to_string(), - None => String::new(), - }, - }; - s.replace_range(start..end, &replacement); - } - } - s -} - -fn expand_tilde_str(raw: &str) -> String { - if let Some(rest) = raw.strip_prefix("~/") { - if let Some(home) = std::env::var_os("HOME") { - let mut p = PathBuf::from(home); - p.push(rest); - return p.to_string_lossy().into_owned(); - } - } - if raw == "~" { - if let Some(home) = std::env::var_os("HOME") { - return PathBuf::from(home).to_string_lossy().into_owned(); - } - } - raw.to_string() -} - -#[cfg(test)] -mod expand_tests { - use super::*; - use std::path::Path; - - #[test] - fn expand_substitutes_data_dir() { - let p = expand_path("{data_dir}/runs", "/tmp/kbtest"); - assert_eq!(p, Path::new("/tmp/kbtest/runs")); - } - - #[test] - fn expand_passthrough_absolute() { - let p = expand_path("/abs/runs", "/ignored"); - assert_eq!(p, Path::new("/abs/runs")); - } -} diff --git a/crates/kb-eval/tests/runner.rs b/crates/kb-eval/tests/runner.rs index eba63af..248c1fd 100644 --- a/crates/kb-eval/tests/runner.rs +++ b/crates/kb-eval/tests/runner.rs @@ -120,6 +120,18 @@ fn write_golden(dir: &Path, body: &str) -> PathBuf { path } +/// Bind a fresh ephemeral port, then release it. The returned URL +/// points at a port that was just freed; very likely still unbound +/// when the test issues its outbound connection a moment later, in +/// which case `connect()` fails fast with `ECONNREFUSED`. Beats +/// hard-coding port 1 which can timeout slowly on hardened hosts. +fn unreachable_endpoint() -> String { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + drop(listener); + format!("http://127.0.0.1:{port}") +} + fn lexical_opts() -> EvalRunOpts { EvalRunOpts { suite: "test".to_string(), @@ -212,10 +224,14 @@ fn runner_records_config_snapshot_with_versions() { #[test] fn runner_captures_per_query_error_when_rag_unreachable() { let env = RunEnv::new(); - // Point Ollama at a guaranteed-dead port so `ask_with_config` - // surfaces a connection error per query. + // Point Ollama at an unbound port so `ask_with_config` surfaces a + // connection error per query. We use bind-then-release rather than + // a hard-coded `:1` because port 1 is reserved-but-not-guaranteed- + // unbound (some hardened systems answer with ICMP unreachable + // instantly, others timeout slowly). TOCTOU race is theoretically + // possible but rare in practice and faster-failing than `:1`. let mut config = env.config.clone(); - config.models.llm.endpoint = "http://127.0.0.1:1".to_string(); + config.models.llm.endpoint = unreachable_endpoint(); let yaml = write_golden(env.data_dir().as_path(), "- id: q1\n query: ownership\n"); diff --git a/crates/kb-store-sqlite/src/store.rs b/crates/kb-store-sqlite/src/store.rs index b7f57d6..f0c1560 100644 --- a/crates/kb-store-sqlite/src/store.rs +++ b/crates/kb-store-sqlite/src/store.rs @@ -64,7 +64,7 @@ impl SqliteStore { /// temp_store=MEMORY), and create parent directories as needed. /// **Does not run migrations** — call [`Self::run_migrations`] next. pub fn open(config: &kb_config::Config) -> Result { - let data_dir = expand_data_dir(&config.storage.data_dir); + let data_dir = kb_config::expand_path(&config.storage.data_dir, ""); std::fs::create_dir_all(&data_dir) .with_context(|| format!("create data_dir {}", data_dir.display()))?; let db_path = data_dir.join(SQLITE_FILE); @@ -363,53 +363,3 @@ fn apply_pragmas(conn: &Connection) -> Result<()> { Ok(()) } -/// Expand the placeholders / `~` / env-vars used by `Config::storage.data_dir`. -/// -/// Supported substitutions, in order: -/// - `${XDG_DATA_HOME:-~/.local/share}` (and the bare `${XDG_DATA_HOME}`) -/// - leading `~` → `$HOME` -/// -/// If neither produces an absolute path, the input is returned as-is -/// (relative paths are kept relative to the caller's CWD). -fn expand_data_dir(raw: &str) -> PathBuf { - let mut s = raw.to_string(); - - // ${XDG_DATA_HOME:-~/.local/share}: respect the env override, else - // fall back to the suffix after `:-`. - if let Some(start) = s.find("${XDG_DATA_HOME") { - if let Some(rel_end) = s[start..].find('}') { - let end = start + rel_end + 1; // include trailing '}' - let inner = &s[start + 2..end - 1]; // strip ${ and } - let replacement = match std::env::var("XDG_DATA_HOME") { - Ok(v) if !v.is_empty() => v, - _ => { - // inner is e.g. `XDG_DATA_HOME:-~/.local/share`. - if let Some((_, default)) = inner.split_once(":-") { - default.to_string() - } else { - // No default supplied; mimic Bash and yield "". - String::new() - } - } - }; - s.replace_range(start..end, &replacement); - } - } - - // ~ at the front → $HOME (or `dirs::home_dir`). - if let Some(rest) = s.strip_prefix('~') { - if let Some(home) = std::env::var_os("HOME").map(PathBuf::from).or_else(dirs_home_fallback) - { - return home.join(rest.trim_start_matches('/')); - } - } - - PathBuf::from(s) -} - -/// Tiny shim to avoid pulling in the `dirs` crate as a direct dep — we -/// only fall back when `$HOME` is unset, which is exotic on the platforms -/// we target. Returns `None` so the caller keeps the literal `~`. -fn dirs_home_fallback() -> Option { - None -} diff --git a/crates/kb-store-vector/src/paths.rs b/crates/kb-store-vector/src/paths.rs index 87d2979..40508f2 100644 --- a/crates/kb-store-vector/src/paths.rs +++ b/crates/kb-store-vector/src/paths.rs @@ -1,51 +1,10 @@ //! Path expansion + table-name sanitization. //! -//! Mirrors `kb-store-sqlite::store::expand_data_dir` and -//! `kb-embed-local::expand_path` so the three crates resolve -//! `${XDG_DATA_HOME:-…}` / leading `~` / `{data_dir}` identically. A -//! shared helper would live in `kb-config`, but the task spec forbids -//! adding new types to `kb-config`, so we keep a private clone. - -use std::path::PathBuf; - -/// Expand `{data_dir}` → `data_dir`, `${XDG_DATA_HOME:-…}` → env or -/// default, leading `~` → `$HOME`. Pass an empty `data_dir` when -/// resolving `data_dir` itself (the `{data_dir}` substitution is a -/// no-op in that case). -pub(crate) fn expand_path(raw: &str, data_dir: &str) -> PathBuf { - let mut s = raw.to_string(); - - if !data_dir.is_empty() { - s = s.replace("{data_dir}", data_dir); - } - - // ${XDG_DATA_HOME:-~/.local/share}: env override, else default after `:-`. - if let Some(start) = s.find("${XDG_DATA_HOME") { - if let Some(rel_end) = s[start..].find('}') { - let end = start + rel_end + 1; - let inner = &s[start + 2..end - 1]; - let replacement = match std::env::var("XDG_DATA_HOME") { - Ok(v) if !v.is_empty() => v, - _ => { - if let Some((_, default)) = inner.split_once(":-") { - default.to_string() - } else { - String::new() - } - } - }; - s.replace_range(start..end, &replacement); - } - } - - if let Some(rest) = s.strip_prefix('~') { - if let Some(home) = std::env::var_os("HOME").map(PathBuf::from) { - return home.join(rest.trim_start_matches('/')); - } - } - - PathBuf::from(s) -} +//! `expand_path` lives in `kb-config` so `kb-store-vector`, +//! `kb-store-sqlite`, `kb-embed-local`, and `kb-eval` all resolve +//! `${XDG_DATA_HOME:-…}` / leading `~` / `{data_dir}` identically. This +//! module re-exports nothing; consumers within the crate `use +//! kb_config::expand_path` directly. /// Build the per-model Lance table name. Per design §6.3: /// `chunk_embeddings__.lance`. Model IDs may contain @@ -104,16 +63,4 @@ mod tests { "chunk_embeddings_BAAI_bge-small-en_384" ); } - - #[test] - fn expand_path_substitutes_data_dir() { - let p = expand_path("{data_dir}/lancedb", "/tmp/kbtest"); - assert_eq!(p, PathBuf::from("/tmp/kbtest/lancedb")); - } - - #[test] - fn expand_path_passthrough_absolute() { - let p = expand_path("/abs/dir", "/ignored"); - assert_eq!(p, PathBuf::from("/abs/dir")); - } } diff --git a/crates/kb-store-vector/src/store.rs b/crates/kb-store-vector/src/store.rs index 124c11c..99e3c6f 100644 --- a/crates/kb-store-vector/src/store.rs +++ b/crates/kb-store-vector/src/store.rs @@ -22,8 +22,10 @@ use serde_json::json; use time::OffsetDateTime; use tokio::runtime::{Builder as RuntimeBuilder, Runtime}; +use kb_config::expand_path; + use crate::arrow_batch::{build_batch, schema_for, schema_params_hash}; -use crate::paths::{expand_path, lance_table_name}; +use crate::paths::lance_table_name; /// Overfetch multiplier: when post-filtering Lance results against /// SQLite-side filters we ask for `2 * k` candidates so a moderately