From 070f20b295ff69a1474d62534b7225019a530084 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Tue, 3 Nov 2026 12:06:46 -0500 Subject: [PATCH] feat(embeddings): record the embedding contract and refuse a mismatched config (#189) Stored vectors are only searchable with the backend, model, width and prefixes that built them, and nothing enforced that: changing [embedding] (or backend) without reembed_embeddings silently broke similarity search, and the per-row dim check missed same-width model or prefix swaps. - embedding_contract: one row (backend, model, dim, both prefixes). The server checks it once after migrations; the src/bin tools check it on their first embed. No row yet = record the running config. - On a mismatch every embed (embed_many, so queries and documents) fails with a message naming each changed field and the fix. Mixing vector spaces corrupts the corpus for good; refusing only pauses semantic search until the config is put back or the corpus re-embedded. - reembed_embeddings bypasses the guard and records the contract after a full run (all four tables, no --limit, not --dry-run). - A DB error in the check leaves embedding allowed and is logged. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 5 +- .../down.sql | 1 + .../up.sql | 12 + src/ai/embedding_contract.rs | 301 ++++++++++++++++++ src/ai/mod.rs | 3 + src/bin/reembed_embeddings.rs | 37 +++ src/database/schema.rs | 13 + src/main.rs | 3 + 8 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 migrations/2026-11-03-000000_add_embedding_contract/down.sql create mode 100644 migrations/2026-11-03-000000_add_embedding_contract/up.sql create mode 100644 src/ai/embedding_contract.rs diff --git a/CLAUDE.md b/CLAUDE.md index 28fcc5b..1a19348 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1004,7 +1004,10 @@ tests never touch the process environment. ADR-0003 records the split. Sections: `backend`, `unified_search_model`, `[llamacpp]` (`url`, `chat`, `vision`, `timeout_secs`, `[llamacpp.picker]` `include`/`exclude`), `[ollama]`, `[openrouter]`, `[embedding]` (`model`, `dim`, `query_prefix`, `document_prefix` — -the corpus contract: change any and run `reembed_embeddings`), `[tts]` +the corpus contract: change any and run `reembed_embeddings`; `src/ai/embedding_contract.rs` +stores it in `embedding_contract` on first start and refuses every embed, with a startup +error naming the changed fields, once the config no longer matches; a full +`reembed_embeddings` run records the new one), `[tts]` (`model`, `voice`, `ref_seconds`, `timeout_secs`), `[agentic]` (`max_iterations`, `chat_max_iterations`, `chat_default_num_ctx`, `chat_turn_timeout_secs`). diff --git a/migrations/2026-11-03-000000_add_embedding_contract/down.sql b/migrations/2026-11-03-000000_add_embedding_contract/down.sql new file mode 100644 index 0000000..b330a82 --- /dev/null +++ b/migrations/2026-11-03-000000_add_embedding_contract/down.sql @@ -0,0 +1 @@ +DROP TABLE embedding_contract; diff --git a/migrations/2026-11-03-000000_add_embedding_contract/up.sql b/migrations/2026-11-03-000000_add_embedding_contract/up.sql new file mode 100644 index 0000000..9a613da --- /dev/null +++ b/migrations/2026-11-03-000000_add_embedding_contract/up.sql @@ -0,0 +1,12 @@ +-- The embedding contract the stored vectors were built under (#189): one +-- row, written on first start (or by reembed_embeddings), compared with the +-- resolved [embedding] config at startup. +CREATE TABLE embedding_contract ( + id INTEGER PRIMARY KEY NOT NULL CHECK (id = 1), + backend TEXT NOT NULL, + model TEXT NOT NULL, + dim INTEGER NOT NULL, + query_prefix TEXT NOT NULL, + document_prefix TEXT NOT NULL, + recorded_at BIGINT NOT NULL +); diff --git a/src/ai/embedding_contract.rs b/src/ai/embedding_contract.rs new file mode 100644 index 0000000..dfa329b --- /dev/null +++ b/src/ai/embedding_contract.rs @@ -0,0 +1,301 @@ +//! The embedding contract (#189): what the stored vectors were built with. +//! +//! Stored vectors are only comparable with query vectors from the same +//! backend, model, width and prefixes. Changing `[embedding]` in llm.toml +//! (or `backend`) without running `reembed_embeddings` used to break +//! similarity search silently; the per-row dim check only caught a change +//! of width. The contract is one row in `embedding_contract`: +//! +//! - **Recorded** on first start (adopting the config the server is running +//! with, which is what built the existing corpus) and by a full +//! `reembed_embeddings` run, which is the one supported way to change it. +//! - **Checked** once per process: at server startup ([`check_at_startup`]), +//! or lazily on the first embed in the `src/bin` tools. +//! - **Enforced**: on a mismatch every embed fails with a message naming both +//! sides and the fix. Mixing vector spaces corrupts the corpus for good (new +//! rows can't be told apart from old ones), while refusing only pauses +//! semantic search and new embeddings until the config is put back or the +//! corpus re-embedded. +//! +//! `model_version` columns stay as per-row labels; this is the gate. + +use std::sync::OnceLock; +use std::sync::atomic::{AtomicBool, Ordering}; + +use diesel::prelude::*; +use diesel::sqlite::SqliteConnection; + +use crate::ai::config::{LlmConfig, LocalBackend}; +use crate::ai::ollama::EMBEDDING_MODEL; +use crate::database::schema::embedding_contract; + +/// Everything that must agree between the stored vectors and new embeds. +#[derive(Debug, Clone, PartialEq, Eq, Queryable, Selectable)] +#[diesel(table_name = embedding_contract)] +pub struct EmbeddingContract { + pub backend: String, + pub model: String, + pub dim: i32, + pub query_prefix: String, + pub document_prefix: String, +} + +impl EmbeddingContract { + /// The contract the resolved llm.toml implies. + pub fn current() -> Self { + Self::from_config(crate::ai::config::llm_config()) + } + + pub fn from_config(cfg: &LlmConfig) -> Self { + let (backend, model) = match cfg.backend { + LocalBackend::Llamacpp => ("llamacpp", cfg.embedding.model.clone()), + // Ollama always embeds with its fixed model (`embedding.model` + // names a llama-swap id and is ignored there). + LocalBackend::Ollama => ("ollama", EMBEDDING_MODEL.to_string()), + }; + Self { + backend: backend.into(), + model, + dim: cfg.embedding.dim as i32, + query_prefix: cfg.embedding.query_prefix.clone(), + document_prefix: cfg.embedding.document_prefix.clone(), + } + } + + /// The fields that differ, as `name: stored → current` (prefixes quoted, + /// since a trailing space or newline is the usual difference). + pub fn differences(&self, current: &Self) -> Vec { + let mut out = Vec::new(); + if self.backend != current.backend { + out.push(format!("backend: {} → {}", self.backend, current.backend)); + } + if self.model != current.model { + out.push(format!("model: {} → {}", self.model, current.model)); + } + if self.dim != current.dim { + out.push(format!("dim: {} → {}", self.dim, current.dim)); + } + if self.query_prefix != current.query_prefix { + out.push(format!( + "query_prefix: {:?} → {:?}", + self.query_prefix, current.query_prefix + )); + } + if self.document_prefix != current.document_prefix { + out.push(format!( + "document_prefix: {:?} → {:?}", + self.document_prefix, current.document_prefix + )); + } + out + } +} + +/// The stored contract, if one was recorded. +pub fn load(conn: &mut SqliteConnection) -> QueryResult> { + embedding_contract::table + .select(EmbeddingContract::as_select()) + .first(conn) + .optional() +} + +/// Record `contract` as the one the stored vectors follow (the single row). +pub fn record(conn: &mut SqliteConnection, contract: &EmbeddingContract) -> QueryResult<()> { + use embedding_contract::dsl as c; + let values = ( + c::backend.eq(&contract.backend), + c::model.eq(&contract.model), + c::dim.eq(contract.dim), + c::query_prefix.eq(&contract.query_prefix), + c::document_prefix.eq(&contract.document_prefix), + c::recorded_at.eq(chrono::Utc::now().timestamp()), + ); + diesel::insert_into(c::embedding_contract) + .values((c::id.eq(1), values)) + .on_conflict(c::id) + .do_update() + .set(values) + .execute(conn) + .map(|_| ()) +} + +#[derive(Debug, PartialEq, Eq)] +pub enum Verdict { + /// The stored contract matches the config. + Matches, + /// Nothing was stored; the config's contract was recorded. + Recorded, + /// The config no longer matches the stored vectors. + Mismatch(Vec), +} + +/// Compare `current` with the stored contract, recording it when none is. +pub fn check(conn: &mut SqliteConnection, current: &EmbeddingContract) -> QueryResult { + match load(conn)? { + None => { + record(conn, current)?; + Ok(Verdict::Recorded) + } + Some(stored) if stored == *current => Ok(Verdict::Matches), + Some(stored) => Ok(Verdict::Mismatch(stored.differences(current))), + } +} + +/// `Err(message)` when embeds must be refused; set once per process. +static STATE: OnceLock> = OnceLock::new(); +static BYPASS: AtomicBool = AtomicBool::new(false); + +fn mismatch_message(diffs: &[String]) -> String { + format!( + "The [embedding] config doesn't match the stored vectors ({}). Embedding is \ + refused so the corpus doesn't mix vector spaces. Run `reembed_embeddings` to \ + rebuild them under the new config (it records it when it finishes), or put \ + the old settings back in llm.toml.", + diffs.join("; ") + ) +} + +/// Run the check and turn it into the process state, logging the outcome. +/// A DB error leaves embedding allowed: the guard must not take search down +/// over its own bookkeeping. +fn evaluate(conn: &mut SqliteConnection) -> Result<(), String> { + let current = EmbeddingContract::current(); + match check(conn, ¤t) { + Ok(Verdict::Matches) => Ok(()), + Ok(Verdict::Recorded) => { + log::info!( + "Embedding contract recorded: backend={} model={} dim={} (stored vectors \ + are assumed to follow it; reembed_embeddings if they don't)", + current.backend, + current.model, + current.dim + ); + Ok(()) + } + Ok(Verdict::Mismatch(diffs)) => { + let message = mismatch_message(&diffs); + log::error!("{}", message); + Err(message) + } + Err(e) => { + log::warn!("Embedding contract check skipped: {}", e); + Ok(()) + } + } +} + +/// Server startup, after migrations: check the contract once and log it. +pub fn check_at_startup(conn: &mut SqliteConnection) { + let _ = STATE.get_or_init(|| evaluate(conn)); +} + +/// Called before every embed. Fails when the config doesn't match the stored +/// vectors. Processes that skipped [`check_at_startup`] (the `src/bin` +/// tools) check here on first use. +pub fn ensure() -> anyhow::Result<()> { + if BYPASS.load(Ordering::Relaxed) { + return Ok(()); + } + let state = STATE.get_or_init(|| { + let Ok(url) = dotenv::var("DATABASE_URL") else { + return Ok(()); + }; + match SqliteConnection::establish(&url) { + Ok(mut conn) => evaluate(&mut conn), + Err(e) => { + log::warn!("Embedding contract check skipped: {}", e); + Ok(()) + } + } + }); + state.clone().map_err(anyhow::Error::msg) +} + +/// For `reembed_embeddings` only: embed under the new config even though the +/// stored vectors don't follow it yet (that's what it's there to fix). +pub fn bypass() { + BYPASS.store(true, Ordering::Relaxed); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ai::config::EmbeddingConfig; + use crate::database::test::in_memory_db_connection; + + fn llamacpp(model: &str, dim: usize, query: &str, doc: &str) -> EmbeddingContract { + let cfg = LlmConfig { + backend: LocalBackend::Llamacpp, + embedding: EmbeddingConfig { + model: model.into(), + dim, + query_prefix: query.into(), + document_prefix: doc.into(), + }, + ..Default::default() + }; + EmbeddingContract::from_config(&cfg) + } + + #[test] + fn records_the_first_contract_then_matches_it() { + let mut conn = in_memory_db_connection(); + let c = llamacpp("embed", 768, "search_query: ", "search_document: "); + assert_eq!(check(&mut conn, &c).unwrap(), Verdict::Recorded); + assert_eq!(load(&mut conn).unwrap(), Some(c.clone())); + assert_eq!(check(&mut conn, &c).unwrap(), Verdict::Matches); + } + + #[test] + fn a_same_width_model_or_prefix_swap_is_a_mismatch() { + let mut conn = in_memory_db_connection(); + let stored = llamacpp("embed", 768, "search_query: ", "search_document: "); + record(&mut conn, &stored).unwrap(); + + let swapped = llamacpp("other-embed", 768, "search_query: ", ""); + let Verdict::Mismatch(diffs) = check(&mut conn, &swapped).unwrap() else { + panic!("expected a mismatch"); + }; + assert_eq!( + diffs, + vec![ + "model: embed → other-embed".to_string(), + "document_prefix: \"search_document: \" → \"\"".to_string(), + ] + ); + // A mismatch never overwrites the stored contract. + assert_eq!(load(&mut conn).unwrap(), Some(stored)); + } + + #[test] + fn record_replaces_the_single_row() { + let mut conn = in_memory_db_connection(); + record(&mut conn, &llamacpp("embed", 768, "", "")).unwrap(); + let qwen = llamacpp("qwen3-embed", 1024, "Instruct: find\nQuery: ", ""); + record(&mut conn, &qwen).unwrap(); + assert_eq!(load(&mut conn).unwrap(), Some(qwen.clone())); + assert_eq!(check(&mut conn, &qwen).unwrap(), Verdict::Matches); + } + + #[test] + fn ollama_uses_its_fixed_model_whatever_embedding_model_says() { + let cfg = LlmConfig { + backend: LocalBackend::Ollama, + embedding: EmbeddingConfig { + model: "ignored".into(), + ..Default::default() + }, + ..Default::default() + }; + let c = EmbeddingContract::from_config(&cfg); + assert_eq!(c.backend, "ollama"); + assert_eq!(c.model, EMBEDDING_MODEL); + } + + #[test] + fn the_mismatch_message_names_the_fix() { + let m = mismatch_message(&["dim: 768 → 1024".into()]); + assert!(m.contains("dim: 768 → 1024")); + assert!(m.contains("reembed_embeddings")); + } +} diff --git a/src/ai/mod.rs b/src/ai/mod.rs index a1c798c..a2f09b4 100644 --- a/src/ai/mod.rs +++ b/src/ai/mod.rs @@ -4,6 +4,7 @@ pub mod clip_client; pub mod config; pub mod daily_summary_job; pub mod date_info; +pub mod embedding_contract; pub mod face_client; pub mod gpu; pub mod handlers; @@ -122,6 +123,8 @@ pub async fn embed_many( llamacpp: Option<&LlamaCppClient>, texts: &[&str], ) -> anyhow::Result>> { + // Refuse to embed under a config the stored vectors weren't built with (#189). + embedding_contract::ensure()?; if local_backend_is_llamacpp() { if let Some(lc) = llamacpp { return ::generate_embeddings(lc, texts).await; diff --git a/src/bin/reembed_embeddings.rs b/src/bin/reembed_embeddings.rs index 936ba7c..ba2a4f0 100644 --- a/src/bin/reembed_embeddings.rs +++ b/src/bin/reembed_embeddings.rs @@ -25,10 +25,13 @@ use clap::Parser; use diesel::prelude::*; use diesel::sql_query; use diesel::sqlite::SqliteConnection; +use image_api::ai::embedding_contract::{self, EmbeddingContract}; use image_api::ai::{LocalLlm, strip_summary_boilerplate}; use image_api::bin_progress; use std::env; +const ALL_TABLES: [&str; 4] = ["summaries", "calendar", "search", "entities"]; + #[derive(Parser, Debug)] #[command(author, version, about = "Re-embed stored corpora via the configured local backend", long_about = None)] struct Args { @@ -284,6 +287,21 @@ async fn main() -> Result<()> { let mut conn = SqliteConnection::establish(&database_url) .with_context(|| format!("connecting to {}", database_url))?; + // This tool exists to move the corpus to the configured contract, so it + // embeds even while the stored one differs (#189). + embedding_contract::bypass(); + let current = EmbeddingContract::current(); + match embedding_contract::load(&mut conn) { + Ok(Some(stored)) if stored != current => { + println!( + "Stored embedding contract differs: {}", + stored.differences(¤t).join("; ") + ); + } + Ok(_) => {} + Err(e) => println!("Couldn't read the stored embedding contract: {}", e), + } + let llm = LocalLlm::from_config(); let model_version = llm.embedding_model_version(); println!("Embedding via '{}'", model_version); @@ -453,6 +471,25 @@ async fn main() -> Result<()> { report_similarity("entities", sims); } + // Every table re-embedded in full: the corpus now follows the configured + // contract, so record it and the server stops refusing embeds. + let full_run = + !args.dry_run && args.limit.is_none() && ALL_TABLES.iter().all(|t| tables.contains(t)); + if full_run { + embedding_contract::record(&mut conn, ¤t) + .context("recording the embedding contract")?; + println!( + "Recorded the embedding contract: backend={} model={} dim={}", + current.backend, current.model, current.dim + ); + } else if !args.dry_run { + println!( + "Embedding contract not recorded: only a full run (all of {}, no --limit) \ + leaves the whole corpus on the new config", + ALL_TABLES.join(", ") + ); + } + println!( "\n{}", if args.dry_run { diff --git a/src/database/schema.rs b/src/database/schema.rs index 9e40375..2eb3d0d 100644 --- a/src/database/schema.rs +++ b/src/database/schema.rs @@ -41,6 +41,18 @@ diesel::table! { } } +diesel::table! { + embedding_contract (id) { + id -> Integer, + backend -> Text, + model -> Text, + dim -> Integer, + query_prefix -> Text, + document_prefix -> Text, + recorded_at -> BigInt, + } +} + diesel::table! { enhance_jobs (id) { id -> Text, @@ -429,6 +441,7 @@ diesel::allow_tables_to_appear_in_same_query!( calendar_events, clip_failures, daily_conversation_summaries, + embedding_contract, enhance_jobs, entities, entity_facts, diff --git a/src/main.rs b/src/main.rs index 97a52bd..c92c38b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -69,6 +69,9 @@ fn main() -> std::io::Result<()> { // Load llm.toml (+ legacy env overrides) now, after logging is up, so // a bad file fails startup and its deprecation warnings are visible. ai::config::llm_config(); + // Stored vectors vs the [embedding] config (#189): records it on first + // start, refuses embeds (and logs why) on a mismatch. + ai::embedding_contract::check_at_startup(&mut connect()); // AppState construction loads (and seeds if needed) the libraries // table; we use that list to drive the initial thumbnail sweep.